[v0.1/P1] Centralize effective-minor send authorization #50

Closed
opened 2026-09-03 16:43:17 +09:00 by natsukium · 1 comment
Owner

Problem

The effective minor is documented as a hard send-side authorization rule, but the implementation enforces it through scattered feature-specific branches.

Current examples include:

  • client-side check_scope_minor for narrowed switch scope;
  • client-side daemon_status refusal;
  • daemon-side geometry_refusal, exited_refusal, and session_limit_refusal downgrade helpers;
  • prose-only ledger entries for oneof arms, fields, enum values, and row-codec versions.

ConnectionDriver and FrameWriter do not know an outgoing message's minimum minor. Adding one new send site without a branch can send an unknown oneof arm or enum value to an older peer and force a connection-fatal decode. #19's protobuf breaking check cannot detect this behavioral error because the schema can remain additively compatible.

Required change

Make the minor ledger executable metadata and route outgoing additions through one checked send boundary on both peers. A feature's fallback must be explicit: omit a field, down-map to an older typed value, refuse locally, or do not emit the message.

Acceptance criteria

  • Every post-baseline frame kind, oneof arm, closed-enum value, behavior-changing field, and row-codec version has machine-readable since_minor metadata next to its protocol definition.
  • Client and daemon send paths receive the negotiated effective minor and cannot emit an unauthorized addition through the ordinary API.
  • A downgrade/omission is explicit and tested; there is no implicit protobuf-default fallback where the old meaning would be unsafe (for example, narrow scope degrading to broadcast).
  • Mixed-minor golden conversations exercise every ledger row in both directions and assert that the older peer sees only shapes it defines.
  • CI checks that the prose ledger and executable metadata cover the same additions.
  • The distinction between the daemon's own reported minor and a connection's effective minor remains explicit.
  • Build this mechanism against the clean protocol-2.0 baseline from #30 rather than carrying the development-era minor history forward accidentally.

Why this is separate

#19 checks schema compatibility against the CI base. This issue checks runtime sender authorization and downgrade behavior, which structural protobuf tooling does not prove.

## Problem The effective minor is documented as a hard send-side authorization rule, but the implementation enforces it through scattered feature-specific branches. Current examples include: - client-side `check_scope_minor` for narrowed switch scope; - client-side `daemon_status` refusal; - daemon-side `geometry_refusal`, `exited_refusal`, and `session_limit_refusal` downgrade helpers; - prose-only ledger entries for oneof arms, fields, enum values, and row-codec versions. `ConnectionDriver` and `FrameWriter` do not know an outgoing message's minimum minor. Adding one new send site without a branch can send an unknown oneof arm or enum value to an older peer and force a connection-fatal decode. #19's protobuf breaking check cannot detect this behavioral error because the schema can remain additively compatible. ## Required change Make the minor ledger executable metadata and route outgoing additions through one checked send boundary on both peers. A feature's fallback must be explicit: omit a field, down-map to an older typed value, refuse locally, or do not emit the message. ## Acceptance criteria - Every post-baseline frame kind, oneof arm, closed-enum value, behavior-changing field, and row-codec version has machine-readable `since_minor` metadata next to its protocol definition. - Client and daemon send paths receive the negotiated effective minor and cannot emit an unauthorized addition through the ordinary API. - A downgrade/omission is explicit and tested; there is no implicit protobuf-default fallback where the old meaning would be unsafe (for example, narrow scope degrading to broadcast). - Mixed-minor golden conversations exercise every ledger row in both directions and assert that the older peer sees only shapes it defines. - CI checks that the prose ledger and executable metadata cover the same additions. - The distinction between the daemon's own reported minor and a connection's effective minor remains explicit. - Build this mechanism against the clean protocol-2.0 baseline from #30 rather than carrying the development-era minor history forward accidentally. ## Why this is separate #19 checks schema compatibility against the CI base. This issue checks runtime sender authorization and downgrade behavior, which structural protobuf tooling does not prove.
Author
Owner

Triage plan (2026-09-03)

Source-grounded triage against main at 69076d42, reviewed through seven rounds of an independent reviewer (pi sol/luna) until it passed with no findings. The dependency order that supersedes the tracker's is posted on #12. Where a "Review amendments" section below conflicts with an earlier section, the amendment is the decision.

Claim check

Accurate against HEAD.

  • The contract is stated as hard and send-side: docs/reference/ipc.md:1689-1708, docs/explanation/architecture/ipc.md:850-881, docs/reference/spec.md:54 (REQ-104), preface.rs:35-39, and minor_defines (preface.rs:264-270) exists as "the send-side gate every minor addition asks".
  • Enforcement is scattered, exactly as listed: client check_scope_minor (crates/felis-client-core/src/connector.rs:138-156, used at :952, :971, and by the bridge cli_bridge.rs:2067-2085); client daemon_status (:996-1005); daemon geometry_refusal, session_limit_refusal, exited_refusal (crates/felis-daemon/src/serve.rs:1012-1039, used at :475, :496, :594). minor_defines itself has no non-test caller (grep: only preface.rs:584-586). MINOR_ENV_BASE (preface.rs:72) is declared but the client's env_base send path does not gate on it (grep MINOR_ENV_BASE finds only the declaration), which is a live instance of the "one new send site without a branch" hazard; harmless only because an older daemon ignores the unknown field.
  • ConnectionDriver holds no minor (driver.rs:176-184); FrameWriter (framing.rs) holds none; the minor lives on Connection.effective_minor (connector.rs:170) and ConnParams.effective_minor (serve.rs:172-178) as a plain u16 that each feature reads by hand.
  • Ledger rows are prose only (docs/reference/ipc.md:1745-1752; proto comments at felis.proto:209,215,222,523,578,1097,1155,1204). #19's buf breaking (buf.yaml WIRE_JSON) cannot see any of this.
  • Mixed-minor tests exist per feature (connector.rs:2077-2164, cli_bridge.rs:2067-2085, serve/tests.rs around geometry) but no matrix.

One thing the issue underestimates: after #30, the ledger has one row (minor 0) and every gate above is deleted. The mechanism will ship with zero live rows, so its tests must run on a synthetic fixture.

Verdict

accept-with-changes. Build the mechanism, but size it to the 2.0 baseline: an executable since_minor per arm and per closed-enum value, one send gate that reads it, an explicit Downgrade hook for the enum-value case, and a test fixture that proves the gate with a synthetic addition, since at 2.0 there is nothing real to gate. Drop the "every ledger row exercised in both directions" matrix from the v0.1.0 scope: there are no rows; keep it as the template the first post-2.0 minor must fill (record that in the explanation as the review-gate step).

Approach

Metadata (felis-protocol, on top of #47's ArmMeta):

  1. since_minor: u16 per arm (already a column in #47). For closed enums that grow values (AttachFailure, StreamErrorReason, RefusalReason, ResourceKind, ...) add fn since_minor(self) -> u16 via the existing data_free_enum! macro (convert.rs:141-167) extended with an optional @since per variant (default 0). Fields that change behavior get no runtime metadata: a field is protobuf-tolerant on receive, so its gate is the sender choosing to omit it; record such fields in the ledger only, with the test in step 5 checking the count. Row-codec versions: felis-grid's writer already carries a version tag (docs/reference/row-codec.md); expose ROW_CODEC_SINCE: &[(u8 /*codec*/, u16 /*minor*/)] in felis-protocol/src/row.rs.
  2. felis.proto: since=N in the arm: comment line (#47) and a // since minor N on enum values; the schema-walking test asserts they match the Rust metadata.

Send gate (felis-transport):

  1. FrameWriter gains effective_minor: u16 set once after the preface (daemon: serve.rs:302-305 after exchange_bootstrap; client: handshake_over after :302; from_halves takes it). send::<M: Directed> / send_correlated check msg.meta().since_minor <= effective_minor and return a typed TransportError::MinorTooOld { arm, needs, effective } (never panic; the daemon logs and drops the connection, the client surfaces ConnectError::MinorTooOld, which already exists at connector.rs:149). Every write in the workspace already goes through FrameWriter::send* (grep write_frame( callers: connector.rs:315 for Hello, which is minor 0). The daemon's fan-out path (session_task.rs OutEvent → writer) goes through the same writer, so Grid/Image arms are covered too.
  2. Enum values: a trait Downgrade { fn for_minor(self, effective: u16) -> Option<Self>; } implemented per closed enum (returns Some(older value) when a documented down-map exists, None when the sender must refuse locally). At 2.0 every impl is the identity; the three serve.rs:1012-1039 helpers are the shape the first real impl takes, and are deleted by #30. The writer cannot see enum values inside a body, so the call is explicit at the construction site; the executable ledger makes forgetting it visible: step 5.

Tests and CI:

  1. A felis-protocol test parses the ledger table in docs/reference/ipc.md (rows after | Minor |) and asserts: number of rows == max since_minor over arms/values/codecs + 1, and every arm/value with since_minor > 0 appears by name in its row. Same pattern as the_protocol_version_matches_the_schema (preface.rs:758-777). This is the "CI checks prose and metadata cover the same additions" criterion, run by cargo test in pr.yml.
  2. Driver/writer tests with a synthetic fixture: a #[cfg(test)] family with an arm at since_minor = 1 (or a test-only ArmMeta override) proving FrameWriter refuses it at effective 0 and passes at 1, in both Sides; a Downgrade fixture proving the explicit-refuse and down-map branches. Keep the existing per-feature tests until #30 deletes their subjects.
  3. The daemon's reported minor vs the connection's effective minor stay distinct: felis daemon status prints PROTOCOL_MINOR (CHANGELOG entry for minor 5; skills/felis/SKILL.md:220), the connection carries effective_minor; add a doc sentence, no code.

Docs: docs/reference/ipc.md:1738-1752 ledger gets a lead-in stating the table is machine-checked and what each row must name (arm/value/codec identifiers as they appear in felis.proto); docs/explanation/architecture/ipc.md:883-899 "The ledger is the review gate" gains the executable half and the rejected alternative (gating in ConnectionDriver::decode on the receive side, rejected because the receiver cannot know what the sender should have withheld; buf breaking, rejected as structural only); docs/reference/testing.md names the synthetic-fixture test. CHANGELOG: none (no user-visible change at 2.0 beyond error strings). skills/felis: none.

Dependencies

After #47 (metadata home) and #30 (clean baseline, so the mechanism is not built around minor 1-5 gates that #30 deletes). Order in #12 (item 10: #50 then #29) and #52 (item 4, before #30) conflict; #12's reading is right for the mechanism (build on 2.0), while the deletion of the old gates is #30's. Recommend: #30 lands first, then #50 adds the mechanism on the 2.0 tree, then #29's golden conversations. #19 is independent.

Risk/effort

M (2 days). Main risk: the enum-value gate is by construction not enforceable in the writer (values are inside the body), so the ledger-vs-metadata test in step 5 is the only thing that catches a forgotten Downgrade call; make that test's failure message name the value and the call site pattern.

Labels

Keep priority/P1, release/v0.1.0 for the gate and metadata (the freeze boundary includes "evolution rules"). If the release date is under pressure, the mixed-minor golden matrix (criterion 4) can move to post-v0.1 without weakening the contract: at 2.0 there is nothing to exercise.

Review amendments (round 1)

  • The "every write goes through send*" claim is wrong. Production callers of FrameWriter::write_frame at HEAD: felis-daemon/src/serve.rs:1421 (write_event_frame, the fan-out of pre-encoded OutEvent bodies), serve.rs:1899 (ConnMsg writes), felis-client/src/main.rs:1738 (the client's outgoing queue writer), felis-client-core/src/connector.rs:316 (Hello). Correction: make write_frame pub(crate) in felis-transport (tests use a #[doc(hidden)] write_frame_unchecked), and route all four through the authorized send* boundary. The fan-out path carries only (kind, body), so streaming::body_for must also return the arm's ArmMeta (or at least since_minor) alongside the body; the client queue item carries the same. The workspace compiles only when no production caller bypasses the gate, which is the enforcement.

Review amendments (round 2)

  • Authorization covers fields and enum values, not only arms. Every outgoing domain message implements AuthorizeForMinor (a recursive fn check(&self, effective: u16) -> Result<(), MinorViolation>) derived by the same schema-walking metadata as ArmMeta: it checks the arm's since_minor, every present field with a since_minor above 0 (either Some/non-default), every closed-enum value's since_minor, and the row-codec version. The ordinary writer API (send, send_unflushed, send_correlated) calls it before encoding; a violation is TransportError::MinorTooOld { path, needs, effective }. Downgrade is no longer optional call-site work: a sender that wants to talk to an older minor either downgrades through the trait or gets the typed error.
  • No production bypass. write_frame is pub(crate); the unchecked entry point exists only under a cfg(feature = "test-util") (or cfg(test)) gate, never as a #[doc(hidden)] public item. The fan-out path (serve.rs:1421) and the client queue (felis-client/src/main.rs:1738) carry a CheckedFrame newtype that can only be constructed by the checked encoder (private constructor in felis-transport), so pre-encoded bodies are unforgeable. streaming::body_for returns a CheckedFrame.
  • Test that proves it: a synthetic since_minor = 1 field and enum value in the test fixture; sending them at effective minor 0 through every public writer method fails with MinorTooOld; the same fixture at effective minor 1 succeeds; a compile-fail test (trybuild) shows write_frame is unreachable from a downstream crate.
## Triage plan (2026-09-03) Source-grounded triage against `main` at `69076d42`, reviewed through seven rounds of an independent reviewer (`pi` sol/luna) until it passed with no findings. The dependency order that supersedes the tracker's is posted on #12. Where a "Review amendments" section below conflicts with an earlier section, the amendment is the decision. ## Claim check Accurate against HEAD. - The contract is stated as hard and send-side: `docs/reference/ipc.md:1689-1708`, `docs/explanation/architecture/ipc.md:850-881`, `docs/reference/spec.md:54` (REQ-104), `preface.rs:35-39`, and `minor_defines` (`preface.rs:264-270`) exists as "the send-side gate every minor addition asks". - Enforcement is scattered, exactly as listed: client `check_scope_minor` (`crates/felis-client-core/src/connector.rs:138-156`, used at `:952`, `:971`, and by the bridge `cli_bridge.rs:2067-2085`); client `daemon_status` (`:996-1005`); daemon `geometry_refusal`, `session_limit_refusal`, `exited_refusal` (`crates/felis-daemon/src/serve.rs:1012-1039`, used at `:475`, `:496`, `:594`). `minor_defines` itself has no non-test caller (grep: only `preface.rs:584-586`). `MINOR_ENV_BASE` (`preface.rs:72`) is declared but the client's `env_base` send path does not gate on it (grep `MINOR_ENV_BASE` finds only the declaration), which is a live instance of the "one new send site without a branch" hazard; harmless only because an older daemon ignores the unknown field. - `ConnectionDriver` holds no minor (`driver.rs:176-184`); `FrameWriter` (`framing.rs`) holds none; the minor lives on `Connection.effective_minor` (`connector.rs:170`) and `ConnParams.effective_minor` (`serve.rs:172-178`) as a plain `u16` that each feature reads by hand. - Ledger rows are prose only (`docs/reference/ipc.md:1745-1752`; proto comments at `felis.proto:209,215,222,523,578,1097,1155,1204`). #19's `buf breaking` (`buf.yaml` `WIRE_JSON`) cannot see any of this. - Mixed-minor tests exist per feature (`connector.rs:2077-2164`, `cli_bridge.rs:2067-2085`, `serve/tests.rs` around geometry) but no matrix. One thing the issue underestimates: after #30, the ledger has **one row** (minor 0) and every gate above is deleted. The mechanism will ship with zero live rows, so its tests must run on a synthetic fixture. ## Verdict **accept-with-changes.** Build the mechanism, but size it to the 2.0 baseline: an executable `since_minor` per arm and per closed-enum value, one send gate that reads it, an explicit `Downgrade` hook for the enum-value case, and a test fixture that proves the gate with a synthetic addition, since at 2.0 there is nothing real to gate. Drop the "every ledger row exercised in both directions" matrix from the v0.1.0 scope: there are no rows; keep it as the *template* the first post-2.0 minor must fill (record that in the explanation as the review-gate step). ## Approach Metadata (`felis-protocol`, on top of #47's `ArmMeta`): 1. `since_minor: u16` per arm (already a column in #47). For closed enums that grow values (`AttachFailure`, `StreamErrorReason`, `RefusalReason`, `ResourceKind`, ...) add `fn since_minor(self) -> u16` via the existing `data_free_enum!` macro (`convert.rs:141-167`) extended with an optional `@since` per variant (default 0). Fields that change behavior get no runtime metadata: a *field* is protobuf-tolerant on receive, so its gate is the sender choosing to omit it; record such fields in the ledger only, with the test in step 5 checking the count. Row-codec versions: `felis-grid`'s writer already carries a version tag (`docs/reference/row-codec.md`); expose `ROW_CODEC_SINCE: &[(u8 /*codec*/, u16 /*minor*/)]` in `felis-protocol/src/row.rs`. 2. `felis.proto`: `since=N` in the `arm:` comment line (#47) and a `// since minor N` on enum values; the schema-walking test asserts they match the Rust metadata. Send gate (`felis-transport`): 3. `FrameWriter` gains `effective_minor: u16` set once after the preface (daemon: `serve.rs:302-305` after `exchange_bootstrap`; client: `handshake_over` after `:302`; `from_halves` takes it). `send::<M: Directed>` / `send_correlated` check `msg.meta().since_minor <= effective_minor` and return a typed `TransportError::MinorTooOld { arm, needs, effective }` (never panic; the daemon logs and drops the connection, the client surfaces `ConnectError::MinorTooOld`, which already exists at `connector.rs:149`). Every write in the workspace already goes through `FrameWriter::send*` (grep `write_frame(` callers: `connector.rs:315` for `Hello`, which is minor 0). The daemon's fan-out path (`session_task.rs` `OutEvent` → writer) goes through the same writer, so `Grid`/`Image` arms are covered too. 4. Enum values: a `trait Downgrade { fn for_minor(self, effective: u16) -> Option<Self>; }` implemented per closed enum (returns `Some(older value)` when a documented down-map exists, `None` when the sender must refuse locally). At 2.0 every impl is the identity; the three `serve.rs:1012-1039` helpers are the shape the first real impl takes, and are deleted by #30. The writer cannot see enum values inside a body, so the call is explicit at the construction site; the executable ledger makes forgetting it visible: step 5. Tests and CI: 5. A `felis-protocol` test parses the ledger table in `docs/reference/ipc.md` (rows after `| Minor |`) and asserts: number of rows == max `since_minor` over arms/values/codecs + 1, and every arm/value with `since_minor > 0` appears by name in its row. Same pattern as `the_protocol_version_matches_the_schema` (`preface.rs:758-777`). This is the "CI checks prose and metadata cover the same additions" criterion, run by `cargo test` in `pr.yml`. 6. Driver/writer tests with a synthetic fixture: a `#[cfg(test)]` family with an arm at `since_minor = 1` (or a test-only `ArmMeta` override) proving `FrameWriter` refuses it at effective 0 and passes at 1, in both `Side`s; a `Downgrade` fixture proving the explicit-refuse and down-map branches. Keep the existing per-feature tests until #30 deletes their subjects. 7. The daemon's reported minor vs the connection's effective minor stay distinct: `felis daemon status` prints `PROTOCOL_MINOR` (CHANGELOG entry for minor 5; `skills/felis/SKILL.md:220`), the connection carries `effective_minor`; add a doc sentence, no code. Docs: `docs/reference/ipc.md:1738-1752` ledger gets a lead-in stating the table is machine-checked and what each row must name (arm/value/codec identifiers as they appear in `felis.proto`); `docs/explanation/architecture/ipc.md:883-899` "The ledger is the review gate" gains the executable half and the rejected alternative (gating in `ConnectionDriver::decode` on the receive side, rejected because the receiver cannot know what the sender should have withheld; `buf breaking`, rejected as structural only); `docs/reference/testing.md` names the synthetic-fixture test. CHANGELOG: none (no user-visible change at 2.0 beyond error strings). `skills/felis`: none. ## Dependencies After #47 (metadata home) and #30 (clean baseline, so the mechanism is not built around minor 1-5 gates that #30 deletes). Order in #12 (item 10: #50 then #29) and #52 (item 4, before #30) conflict; #12's reading is right for the *mechanism* (build on 2.0), while the *deletion* of the old gates is #30's. Recommend: #30 lands first, then #50 adds the mechanism on the 2.0 tree, then #29's golden conversations. #19 is independent. ## Risk/effort **M** (2 days). Main risk: the enum-value gate is by construction not enforceable in the writer (values are inside the body), so the ledger-vs-metadata test in step 5 is the only thing that catches a forgotten `Downgrade` call; make that test's failure message name the value and the call site pattern. ## Labels Keep `priority/P1`, `release/v0.1.0` for the gate and metadata (the freeze boundary includes "evolution rules"). If the release date is under pressure, the mixed-minor golden matrix (criterion 4) can move to post-v0.1 without weakening the contract: at 2.0 there is nothing to exercise. ## Review amendments (round 1) - **The "every write goes through `send*`" claim is wrong.** Production callers of `FrameWriter::write_frame` at HEAD: `felis-daemon/src/serve.rs:1421` (`write_event_frame`, the fan-out of pre-encoded `OutEvent` bodies), `serve.rs:1899` (`ConnMsg` writes), `felis-client/src/main.rs:1738` (the client's outgoing queue writer), `felis-client-core/src/connector.rs:316` (`Hello`). Correction: make `write_frame` `pub(crate)` in felis-transport (tests use a `#[doc(hidden)]` `write_frame_unchecked`), and route all four through the authorized `send*` boundary. The fan-out path carries only `(kind, body)`, so `streaming::body_for` must also return the arm's `ArmMeta` (or at least `since_minor`) alongside the body; the client queue item carries the same. The workspace compiles only when no production caller bypasses the gate, which is the enforcement. ## Review amendments (round 2) - **Authorization covers fields and enum values, not only arms.** Every outgoing domain message implements `AuthorizeForMinor` (a recursive `fn check(&self, effective: u16) -> Result<(), MinorViolation>`) derived by the same schema-walking metadata as `ArmMeta`: it checks the arm's `since_minor`, every *present* field with a `since_minor` above 0 (either `Some`/non-default), every closed-enum value's `since_minor`, and the row-codec version. The ordinary writer API (`send`, `send_unflushed`, `send_correlated`) calls it before encoding; a violation is `TransportError::MinorTooOld { path, needs, effective }`. `Downgrade` is no longer optional call-site work: a sender that wants to talk to an older minor either downgrades through the trait or gets the typed error. - **No production bypass.** `write_frame` is `pub(crate)`; the unchecked entry point exists only under a `cfg(feature = "test-util")` (or `cfg(test)`) gate, never as a `#[doc(hidden)]` public item. The fan-out path (`serve.rs:1421`) and the client queue (`felis-client/src/main.rs:1738`) carry a `CheckedFrame` newtype that can only be constructed by the checked encoder (private constructor in felis-transport), so pre-encoded bodies are unforgeable. `streaming::body_for` returns a `CheckedFrame`. - **Test that proves it:** a synthetic `since_minor = 1` field and enum value in the test fixture; sending them at effective minor 0 through every public writer method fails with `MinorTooOld`; the same fixture at effective minor 1 succeeds; a compile-fail test (`trybuild`) shows `write_frame` is unreachable from a downstream crate.
Sign in to join this conversation.
No description provided.