transport: gate every send on the connection's effective minor #177

Merged
natsukium merged 9 commits from fix/minor-send-authorization-50 into main 2026-09-07 15:18:25 +09:00
Owner

Fixes #50.

Send authorization for protocol minors now lives in one place. The minor ledger became executable metadata in
felis-protocol (minor.rs): every gated addition declares the minor it requires, and the writer consults that
metadata instead of each call site remembering the rule. FrameWriter carries the connection's effective minor
and refuses — or degrades — any frame that would emit an addition the peer never negotiated, so the daemon's
outbound frames, the client's outgoing queue, and the CLI bridge all pass through the same gate.

The daemon degrades a reply to the peer's minor before sending it, the client omits an attach field an older
daemon would ignore, and the gate reaches the values nested inside an arm rather than only the top-level arm.
A ledger-coverage test holds the prose ledger and the executable metadata to each other in both directions.

felis-client-core's check_scope_minor / check_arm_minor and the client-side daemon_status refusal stay:
they are the explicit local refusals the issue asks for and the CLI's exit-code mapping keys on
ConnectError::MinorTooOld. The writer is the backstop behind them, not a replacement.

Doc cascade: docs/reference/ipc.md (the ledger and the send-authorization rule),
docs/explanation/architecture/ipc.md (why authorization is centralized on the writer),
docs/reference/row-codec.md, and docs/reference/testing.md (the ledger-coverage test).

Verified: just check green (fmt, clippy, nextest, deny, proto-compat); reviewed by pi luna (round 1) + pi luna + pi sol; docs proofread by Gemini gemini-3.8-flash-high

Follow-ups

  • #164 [v0.1/P2] Exercise every minor-ledger row with a mixed-minor conversation
  • #176 protocol: FrameWriter::new defaults to this build's own PROTOCOL_MINOR

Deferred

  • A trybuild compile-fail test proving FrameWriter::write_frame is unreachable from a downstream crate —
    pub(crate) already makes rustc the check for every consumer; a trybuild harness would add a dev-dependency
    and a test kind the workspace does not have.
  • felis-client-core's check_scope_minor / check_arm_minor and the client-side daemon_status refusal were
    kept rather than deleted in favor of the writer gate — they are the explicit local refusals criterion 3 asks
    for, and the CLI's exit-code mapping keys on ConnectError::MinorTooOld; the writer is now the backstop
    behind them, not a replacement.
  • The mixed-minor golden conversation matrix (acceptance criterion 4) is not implemented — a pre-approved scope
    reduction from the issue's own triage plan, filed as #164.
  • No mixed-minor golden conversation coverage, which is what let the sender bugs found in review through —
    duplicate of the matrix finding, same pre-agreed deferral to #164.
  • The mechanism was built on the live development-era ledger, not the clean protocol-2.0 baseline the issue
    asked for — outside this branch's control since dependency issue #30 was closed without landing.
  • The new per-target omission paths lack boundary-precision coverage — the untested per-row minor boundaries
    across these fallback paths are the same territory #164 already tracks; bespoke boundary tests here would be
    hardening beyond what this PR's acceptance criteria require.
Fixes #50. Send authorization for protocol minors now lives in one place. The minor ledger became executable metadata in `felis-protocol` (`minor.rs`): every gated addition declares the minor it requires, and the writer consults that metadata instead of each call site remembering the rule. `FrameWriter` carries the connection's effective minor and refuses — or degrades — any frame that would emit an addition the peer never negotiated, so the daemon's outbound frames, the client's outgoing queue, and the CLI bridge all pass through the same gate. The daemon degrades a reply to the peer's minor before sending it, the client omits an attach field an older daemon would ignore, and the gate reaches the values nested inside an arm rather than only the top-level arm. A ledger-coverage test holds the prose ledger and the executable metadata to each other in both directions. `felis-client-core`'s `check_scope_minor` / `check_arm_minor` and the client-side `daemon_status` refusal stay: they are the explicit local refusals the issue asks for and the CLI's exit-code mapping keys on `ConnectError::MinorTooOld`. The writer is the backstop behind them, not a replacement. Doc cascade: `docs/reference/ipc.md` (the ledger and the send-authorization rule), `docs/explanation/architecture/ipc.md` (why authorization is centralized on the writer), `docs/reference/row-codec.md`, and `docs/reference/testing.md` (the ledger-coverage test). Verified: just check green (fmt, clippy, nextest, deny, proto-compat); reviewed by pi luna (round 1) + pi luna + pi sol; docs proofread by Gemini gemini-3.8-flash-high ## Follow-ups - #164 [v0.1/P2] Exercise every minor-ledger row with a mixed-minor conversation - #176 protocol: FrameWriter::new defaults to this build's own PROTOCOL_MINOR ## Deferred - A trybuild compile-fail test proving `FrameWriter::write_frame` is unreachable from a downstream crate — `pub(crate)` already makes rustc the check for every consumer; a trybuild harness would add a dev-dependency and a test kind the workspace does not have. - `felis-client-core`'s `check_scope_minor` / `check_arm_minor` and the client-side `daemon_status` refusal were kept rather than deleted in favor of the writer gate — they are the explicit local refusals criterion 3 asks for, and the CLI's exit-code mapping keys on `ConnectError::MinorTooOld`; the writer is now the backstop behind them, not a replacement. - The mixed-minor golden conversation matrix (acceptance criterion 4) is not implemented — a pre-approved scope reduction from the issue's own triage plan, filed as #164. - No mixed-minor golden conversation coverage, which is what let the sender bugs found in review through — duplicate of the matrix finding, same pre-agreed deferral to #164. - The mechanism was built on the live development-era ledger, not the clean protocol-2.0 baseline the issue asked for — outside this branch's control since dependency issue #30 was closed without landing. - The new per-target omission paths lack boundary-precision coverage — the untested per-row minor boundaries across these fallback paths are the same territory #164 already tracks; bespoke boundary tests here would be hardening beyond what this PR's acceptance criteria require.
The ledger was prose, so every minor addition needed its own hand-written
send-side branch and a new one could be forgotten without any check
noticing: a schema stays additively compatible while the sender puts a
value on the wire that costs an older peer its connection.

Each addition now answers what it costs to send. Arms already carried
`since_minor`; closed enums that grow values gain one, row-codec versions
get `ROW_CODEC_SINCE`, and a message folds its arm, its present fields and
the values it names into one `Requires` for a send boundary to check.
`Downgrade` is the other half: a value with a documented older equivalent
maps down, and one without says so instead of degrading into a weaker
truth an older peer would misread.

Fields carry no runtime table of their own, since protobuf tolerates an
unknown field on receive and the gate is the sender omitting it. The
ledger-coverage test is what holds the prose table and the metadata
together, failing with the identifier a row does not name.

Refs #50

Assisted-by: Claude Code
`ConnectionDriver` and `FrameWriter` knew nothing about a message's
minimum minor, so authorization lived in feature-specific branches at the
call sites and a new send site without one could put an addition on the
wire that an older peer decodes fatally.

The writer now carries the negotiated minor and checks it: `send`,
`send_unflushed` and `send_correlated` refuse a message whose `Requires`
outranks the connection, naming the addition, and nothing was written so
the connection survives the refusal. `write_frame` is private behind that
gate, with an unchecked entry point only under `test-util` for the tests
and the bench that must emit bytes no domain message produces.

Pre-encoded bodies get `CheckedFrame`, whose private fields can only come
from the checked encoder: the daemon's fan-out and the client's send queue
hand bytes to a writer that never sees the message, so the requirement has
to travel with them rather than be re-derived.

An unnegotiated writer defaults to this build's own minor, which is what a
test harness or a bench talking to itself speaks.

Refs #50

Assisted-by: Claude Code
The fan-out wrote pre-encoded bodies straight to the frame writer, so a
`Grid`, `Image`, `Push` or `Ops` body reached the wire without anyone
asking what minor it needed. It now encodes a `CheckedFrame`, which the
pump cannot construct itself, so the pump's write is authorized by the
same rule a direct send is.

The four per-reason refusal helpers collapse into the `AttachFailure`
down-map they were each restating: a reason is asked what it may be called
at this connection's minor, which is one rule the ledger owns instead of
four copies that can drift from it.

Refs #50

Assisted-by: Claude Code
An `OutgoingFrame` held a kind and opaque bytes, and the writer task wrote
them unchecked: the queue is where the client encodes, so by the time a
frame reaches the carrier there is no message left to ask what minor it
needs. It now holds a `CheckedFrame` and the writer sends through the
gate, which also puts the queue's own limit check on the encoder rather
than beside it.

`outgoing` moves behind the `native` feature with the transport it now
needs; the wasm build has no carrier to queue for.

Refs #50

Assisted-by: Claude Code
The ledger table now has a machine-checked counterpart beside each
definition, so the reference page states how a row must name its
additions and what fails when it does not, and the explanation twin
carries the decision: gate on send, because a receiver can see that a
value is unknown but not what should have been sent instead.

The distinction between a daemon's own minor and a connection's
effective one was implicit in the status reply's field docs and is now
stated where the contract is.

Refs #50

Assisted-by: Claude Code
`MinorGated::requires` answered the bare arm requirement for
`SessionMsg::Attached` / `Created` and ignored the `SwitchTarget` on
`OpsMsg::Switch` and the `RetargetTarget` on `PushMsg::RetargetHost`, so
a roster row's post-baseline fields and a relayed `SpawnArgs.env_base`
reached a peer below their minor with no authorization behind them --
the hazard the gate exists to close.

The coverage test could not have caught it: it compared only arms, enum
values and row codecs against the prose ledger, and built its enum list
from hand-written arrays that a new variant does not have to appear in.
Fields now carry their requirement in one registry the gate and the test
both read, and the enum walk starts from the generated wire enum, so a
value added to the schema forces the match to name it.

Refs #50
Every reply builder stamped the current minor's fields unconditionally,
so the send gate answered a peer below them with `MinorTooOld` and the
connection died. The ledger promises the opposite for each of these: a
roster row's `attachments` and `sequence` read as absent, `draining`
reads `false`, and an operation id degrades to no value at all. A window
that someone else switches must keep its connection, not lose it over an
id it never asked for, so the push fan-out consults each subscriber's
own effective minor rather than the daemon's.

Refs #50
A daemon below minor 3 ignores `live_only` and attaches anyway, and one
below minor 4 ignores a relayed `env_base`, but the send gate refused
the whole message instead: an automatic attach or a carrier retarget
against such a daemon failed outright, where the ledger's fallback is
the older behavior.

Refs #50
protocol: hold the ledger to the metadata in both directions
Some checks failed
darwin / build felis (aarch64-darwin) (pull_request) Successful in 2m6s
bench / Criterion regression gate (pull_request) Successful in 2m12s
windows / cargo nextest (Windows) (pull_request) Successful in 7m23s
pr / frontend smoke (x86_64-linux) (pull_request) Successful in 2m40s
pr / publish felis (x86_64-linux) (pull_request) Has been skipped
windows / cargo clippy (Windows cross) (pull_request) Failing after 13s
bench / Criterion full-suite snapshot (pull_request) Has been skipped
fuzz / cargo fuzz nightly long-run (pull_request) Has been skipped
fuzz / cargo fuzz smoke (per target) (pull_request) Successful in 1m15s
pr / nix flake check (pull_request) Successful in 1m7s
pr / cargo build / clippy / test / deny (pull_request) Successful in 1m39s
pr / wire schema is compatible with the base (pull_request) Successful in 8s
windows / package felis (x86_64-pc-windows-msvc) (pull_request) Has been cancelled
windows / frontend smoke (Windows) (pull_request) Has been cancelled
2ec23c2a87
The coverage test only asked whether every executable addition was named
in the prose ledger. A row was free to claim an addition that no arm,
field or enum value declares, which is the scattered-branch failure the
executable ledger exists to remove: a minor documented as authorized
with nothing behind it on the send gate reads, to a reviewer, exactly
like one that is.

So the check now also reads each row's identifiers back. Only the three
shapes the ledger spells with a qualifier are read as an addition, since
a row's prose names bare types and command lines it must stay free to
name, and a post-baseline row that names none of them is a row whose
addition cannot be checked at all.

The testing table also claimed the send-gate tests refuse each of the
three example additions through every writer path and pass at their
minor; they cover one path per example, and only the enum value passes
at its own.

Refs #50
natsukium force-pushed fix/minor-send-authorization-50 from 2ec23c2a87
Some checks failed
darwin / build felis (aarch64-darwin) (pull_request) Successful in 2m6s
bench / Criterion regression gate (pull_request) Successful in 2m12s
windows / cargo nextest (Windows) (pull_request) Successful in 7m23s
pr / frontend smoke (x86_64-linux) (pull_request) Successful in 2m40s
pr / publish felis (x86_64-linux) (pull_request) Has been skipped
windows / cargo clippy (Windows cross) (pull_request) Failing after 13s
bench / Criterion full-suite snapshot (pull_request) Has been skipped
fuzz / cargo fuzz nightly long-run (pull_request) Has been skipped
fuzz / cargo fuzz smoke (per target) (pull_request) Successful in 1m15s
pr / nix flake check (pull_request) Successful in 1m7s
pr / cargo build / clippy / test / deny (pull_request) Successful in 1m39s
pr / wire schema is compatible with the base (pull_request) Successful in 8s
windows / package felis (x86_64-pc-windows-msvc) (pull_request) Has been cancelled
windows / frontend smoke (Windows) (pull_request) Has been cancelled
to 939db56bdf
All checks were successful
darwin / build felis (aarch64-darwin) (pull_request) Successful in 50s
bench / Criterion regression gate (pull_request) Successful in 2m5s
bench / Criterion full-suite snapshot (pull_request) Has been skipped
fuzz / cargo fuzz nightly long-run (pull_request) Has been skipped
fuzz / cargo fuzz smoke (per target) (pull_request) Successful in 1m6s
pr / frontend smoke (x86_64-linux) (pull_request) Successful in 49s
pr / publish felis (x86_64-linux) (pull_request) Has been skipped
windows / cargo clippy (Windows cross) (pull_request) Successful in 13s
pr / nix flake check (pull_request) Successful in 29s
pr / cargo build / clippy / test / deny (pull_request) Successful in 1m41s
pr / wire schema is compatible with the base (pull_request) Successful in 10s
windows / cargo nextest (Windows) (pull_request) Successful in 9m4s
windows / frontend smoke (Windows) (pull_request) Successful in 2m39s
windows / package felis (x86_64-pc-windows-msvc) (pull_request) Has been skipped
2026-09-07 14:59:39 +09:00
Compare
natsukium deleted branch fix/minor-send-authorization-50 2026-09-07 15:18:25 +09:00
Sign in to join this conversation.
No description provided.