fix(protocol): reject preface acceptance for an unoffered major #56

Merged

natsukium merged 8 commits from fix/preface-accept-major-45 into main 2026-09-04 01:29:44 +09:00

Conversation 0 Commits 8 Files changed 13 +587 -34

natsukium commented 2026-09-03 18:41:04 +09:00

Owner

Copy link

Fixes #45.

The client offered exactly PROTOCOL_MAJOR but handshake_over accepted any DaemonPreface::Accept regardless of its major field.

• handshake_over now rejects Accept.major != PROTOCOL_MAJOR before constructing a framed Connection
• DaemonPreface::accept selects major and minor together via daemon_minor_for
• docs/reference/ipc.md states the echo invariant
• test covers wrong-major case

Verified: fmt, clippy, nextest (2960 passed)

Fixes #45. The client offered exactly PROTOCOL_MAJOR but handshake_over accepted any DaemonPreface::Accept regardless of its major field. - handshake_over now rejects Accept.major != PROTOCOL_MAJOR before constructing a framed Connection - DaemonPreface::accept selects major and minor together via daemon_minor_for - docs/reference/ipc.md states the echo invariant - test covers wrong-major case Verified: fmt, clippy, nextest (2960 passed)

natsukium added 1 commit 2026-09-03 18:41:04 +09:00

fix(protocol): reject preface acceptance for an unoffered major ... f94bb78d60

The client offered exactly PROTOCOL_MAJOR but handshake_over accepted
any DaemonPreface::Accept regardless of its major field. A faulty
daemon could therefore accept major 9 after an offer of 1 and the
client would decode frames with the wrong schema, surfacing only as a
later protobuf failure. The negotiation layer must validate that the
accept echoes the offered major before a framed Connection is built,
so schema skew fails loud at the preface boundary and frames are never
decoded under the wrong major. The daemon's selected schema and its
advertised minor are also now one decision (daemon_minor_for) so a
future side-by-side major decoder cannot accidentally pair an old
schema with the current minor.

Fixes #45

Assisted-by: Pi Coding Agent:Muse Spark 1.2

natsukium added 7 commits 2026-09-03 23:54:30 +09:00

fix(protocol): own exact preface negotiation in felis-protocol ... 353e195e52

The wrong-major check lived only in the client connector, as an ad hoc
comparison against PROTOCOL_MAJOR, while the daemon still built its
accept through an API that took any major and always advertised the
global minor. Both halves of the invariant now live in the
transport-free crate so every connector and the daemon call one rule:

- DaemonPreface::select(ClientPreface) replaces accept(major). The
  accepted major and the minor advertised for it come out of one match
  arm, so a side-by-side decoder for an older major (a deprecation
  window) adds an arm with that major's own minor instead of inheriting
  PROTOCOL_MINOR; supports_major is derived from select so the refusal
  range and the reply cannot disagree. The daemon reads the effective
  minor back out of the reply it wrote rather than from the global.
- confirm_accept(offered, reply) -> Result<effective minor,
  NegotiationError> is the client half: an accept naming an unoffered
  major is its own error class, distinct from the two refusal statuses,
  because a peer that answers that way is not running felis's
  negotiation and the frames after it would decode under a schema the
  two never agreed on. The connector maps it to
  ConnectError::AcceptedUnofferedMajor, non-transient, before any
  FrameReader exists.

The decoder stays lenient on purpose (the status word alone
discriminates the reply); the comparison is one layer up.

Tests: the select table and confirm_accept cases in felis-protocol;
duplex tests in felis-transport for a wrong accepted major, the
offered major with an older/equal/newer daemon minor, a refusal and an
unknown status; the connector test now reads the fake daemon's side
to EOF and asserts no Hello was written after the bad accept.

Refs #45

Assisted-by: Claude Code

fix(cli): name an accept of an unoffered major in version and doctor ... b60f1fc6a9

Both verbs already tell a major-skewed daemon apart from an absent one,
because a daemon that answered is the case the report exists to expose.
An accept naming a major this build never offered fell through to the
"not running" arm (a warn in doctor), which hides a running peer that
failed negotiation and points the operator at the wrong fix. `felis
version` now prints the accepted and offered majors, and `felis doctor`
fails the daemon row the way it does for a major break.

Refs #45

Assisted-by: Claude Code

docs: record the exact-negotiation preface invariant as REQ-104d ... b61d7da0c7

The preface docs argued for a self-describing reply but never said
that an accept has to name the offered major, so a reader could take
"the decoder never compares the words against what it sent" as
license for a lenient negotiator. The reference now states the
invariant and the corruption behavior (close before any frame, an
error class distinct from a refusal), spec.md sources it as its own
requirement rather than widening REQ-104, and the explanation records
why leniency is rejected: it moves schema skew to a protobuf failure
after the preface, the failure the frozen layer exists to raise
before a frame is read. The proto header carries the same clause,
which regenerates into the committed codegen. The CHANGELOG entry
covers the new error string and the version/doctor wording.

Refs #45

Assisted-by: Claude Code

fix(cli): render the unoffered-major doctor row on one line and pin it ... bd0b5625d3

The doctor detail for an accept naming a major this build never
offered wrapped its format literal without a line continuation, so
the human table and the JSON `detail` both carried the source
indentation inside the message. rustfmt leaves string contents alone
and no test rendered the row, which is why the gate stayed green;
the row now has a test that dials a fake daemon and compares the
whole detail, so the wording is pinned the way `felis version`'s is.

The connector's negative test for the same accept now bounds the
dial: without the check, a lenient client writes `Hello` and waits
for a `Welcome` the fake daemon withholds until the client closes,
so a regression hung to nextest's slow-timeout instead of failing
the assertion that no frame follows a bad accept.

Refs #45

Assisted-by: Claude Code

docs: keep the exact-negotiation twins in their lanes ... 64e65d0709

The reference's "Version preface" argued why an accept naming an
unoffered major is its own error class and carried the only RFC
keyword on the page; the normative MUST already lives in REQ-104d,
so the reference now states the resulting facts and the argument
sits only in the explanation, which no longer retells the reference's
operational facts before its rejected alternative. The decision gains
the *Revisit if* trigger its neighbors carry, since the daemon-side
pairing exists for a deprecation window that has not arrived.

The doctor row in the CLI reference drops the em dash new prose
defaults away from.

The page's other skew catalog -- the "Handshake" section's table of
conditions and where each is reported -- enumerates the same preface
skew this commit otherwise touches but had no row for an accept
naming an unoffered major, so a reader consulting that table (the
place the explanation twin links to as "where skew is enumerated")
would not find this condition even though every other surface for it
was updated in the same change. Added the missing row here rather
than as a separate commit, since both edits keep the same page's
surfaces for this invariant current.

Refs #45

Assisted-by: Claude Code

docs(manual): state the unoffered-major version line, not argue it ... 15b6e080de

The CLI reference explained why an accept naming an unoffered major is
neither "not running" nor a refusal; that argument is owned by the
explanation twin (ipc.md "Handshake bootstrap", REQ-104d) and a
reference page carries only the string, the condition that produces
it, and the link. The round-1 twin fix stopped at the IPC reference
and left this sentence behind.

Refs #45

Assisted-by: Claude Code

docs(changelog): file the unoffered-major accept under Fixed ... 6a44c38302

Every commit on the branch is a fix and the issue is a defect (a
lenient client decoded frames under a schema it never agreed to), yet
the entry sat under Changed. The Wire bug-fix rows already live under
Fixed, so a reader scanning Fixed for wire defects would not have
found this one.

Refs #45

Assisted-by: Claude Code

natsukium merged commit 6a44c38302 into main 2026-09-04 01:29:44 +09:00

natsukium deleted branch fix/preface-accept-major-45 2026-09-04 01:29:44 +09:00

natsukium referenced this pull request 2026-09-07 16:09:12 +09:00

[v0.1/Proto Review] Freeze protobuf and IPC contracts (7 issues) #52

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

priority/P0

Must be resolved before the v0.1.0 tag

  

priority/P1

Must be settled before the first public contract freezes

  

priority/P2

Explicitly deferred until after v0.1.0

  

release/v0.1.0

Work tracked for the first tagged release

  

status/blocked

Cannot start now: waits on an upstream release, another issue, or a recorded revisit trigger

  

status/planned

Triaged with a source-grounded plan and a passed pi review; ready to implement

  

type/bug

Shipped behavior that contradicts the docs or the code's own intent; needs a product code change

  

type/design

An open scope or trade-off question; needs a recorded decision in the owning explanation doc before code

  

type/test-gap

Documented or reviewed behavior that no test pins; usually split from a review finding, needs no product code change

  

type/tracker

Parent issue that tracks child work

No labels

priority/P0

priority/P1

priority/P2

release/v0.1.0

status/blocked

status/planned

type/bug

type/design

type/test-gap

type/tracker

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

natsukium/felis!56

No description provided.