protocol, cli: pin the frozen-surface corners a pre-release review left open #354

Merged

natsukium merged 7 commits from protocol/issue-347-cheap-before-freeze into main 2026-09-22 22:48:40 +09:00

Conversation 0 Commits 7 Files changed 6 +299 -24

natsukium commented 2026-09-22 21:18:49 +09:00

Owner

Copy link

Closes #347.

Items 1, A, B and C of the issue were moved to #351 by the 2026-09-22 triage; this PR carries items 2–6 only.

What changed

Item 2 — the _UNSPECIFIED mode branch. The handshake description in docs/reference/ipc.md now names which side
of the unknown-mode split each value falls on: a nonzero value the daemon does not define is the newer peer's and earns
Refused { UnknownMode }, while the CONNECTION_MODE_UNSPECIFIED sentinel 0 is malformed and takes the corruption
path.

No new test was needed. The behavior the issue asked to pin is already pinned:
an_unreadable_hello_that_is_not_a_future_mode_closes_unanswered in crates/felis-daemon/src/serve/tests.rs drives a
hand-encoded Hello with mode 0 and asserts the daemon closes without writing a frame, beside the value-9 test that
pins the typed refusal. The issue's premise ("whether the daemon takes the typed-refusal path or the corruption path is
unpinned") holds for the documentation, not for the tests.

Item 3 — the ConnEnd.count criterion. The End { stream_id, count } bullet now sorts a new frame by the request:
only the values it asked for are results, while an acknowledgement, a loss marker or a family trailer escorts them
without joining the count, a trailer included when it carries data End has no field for. That classifies all three
precedents the bullet names, which asking whether a family-agnostic terminal would carry the frame did not.

Item 4 — error.kind may not narrow. The "Frozen for v:1" paragraph in docs/reference/cli.md adds "or
narrowed", with the clause that says what that forbids.

Item 5 — --timeout 0. 0 sets a deadline that has already passed, so nothing is waited for and the verb exits
1 under timeout. That is what the code does today, on both flags the CLI epoch freezes:

• sessions send --wait --timeout 0: neither the admission confirmation nor the mark watch gets a chance to finish.
  Stated in the clap help and in the send bullet of docs/reference/cli.md, pinned by
  send_wait_timeout_zero_gives_up_at_once.
• notifications subscribe --once --timeout 0: the subscription waits for no notification, and the stream's terminal
  is an error object of kind timeout. Stated in that flag's clap help and its cli.md bullet, pinned by
  notifications_once_timeout_zero_gives_up_at_once.

The help-page insta snapshot was regenerated for both.

Both waits check the deadline before reaching for a frame rather than only racing it. tokio::time::timeout_at polls
its inner future before an expired timer, so a daemon that answered the subscribe with its ack and a notification in
one flush left --timeout 0 printing that notification and exiting 0. drain_frames (the mark watch) already
guarded this way; the notification loop and the admission confirmation now do too.
notifications_timeout_zero_outranks_an_already_readable_notification pins it with a fake daemon that puts both
frames in the reader's read-ahead before the CLI polls, and fails on the pre-fix code.

No schema change: felis-cli-v1.schema.json mentions timeout only as an error.kind token, and
SessionsSendParams in the bridge schema carries session, text and raw — the bridge has no wait, so it has no
timeout input to annotate.

Item 6 — why relay payload version 0 is malformed. The reference now gives the reason the explanation doc holds:
the version word is how a writer that predates it is told apart, and such a writer puts its entry count where the
version belongs with a zero high half.

Verified

• just fmt-check — clean
• just lint (clippy, workspace, all targets, all features) — clean
• just prose-check — 0 hits
• just docs-links — 1234 OK, 0 errors
• cargo nextest run --all-features -p felis-daemon -p felis-cli — 918 passed, 7 skipped
• Reverting the deadline guard fails notifications_timeout_zero_outranks_an_already_readable_notification 50 times
  out of 50; with the guard it passes 50 out of 50.
• doc-cascade sweep over --timeout, UnknownMode and the count criterion across docs/, skills/, README.md:
  no other page restates them.

Closes #347. Items 1, A, B and C of the issue were moved to #351 by the 2026-09-22 triage; this PR carries items 2–6 only. ## What changed **Item 2 — the `_UNSPECIFIED` mode branch.** The handshake description in `docs/reference/ipc.md` now names which side of the unknown-mode split each value falls on: a nonzero value the daemon does not define is the newer peer's and earns `Refused { UnknownMode }`, while the `CONNECTION_MODE_UNSPECIFIED` sentinel `0` is malformed and takes the corruption path. No new test was needed. The behavior the issue asked to pin is already pinned: `an_unreadable_hello_that_is_not_a_future_mode_closes_unanswered` in `crates/felis-daemon/src/serve/tests.rs` drives a hand-encoded `Hello` with mode `0` and asserts the daemon closes without writing a frame, beside the value-9 test that pins the typed refusal. The issue's premise ("whether the daemon takes the typed-refusal path or the corruption path is unpinned") holds for the documentation, not for the tests. **Item 3 — the `ConnEnd.count` criterion.** The `End { stream_id, count }` bullet now sorts a new frame by the request: only the values it asked for are results, while an acknowledgement, a loss marker or a family trailer escorts them without joining the count, a trailer included when it carries data `End` has no field for. That classifies all three precedents the bullet names, which asking whether a family-agnostic terminal would carry the frame did not. **Item 4 — `error.kind` may not narrow.** The "Frozen for `v:1`" paragraph in `docs/reference/cli.md` adds "or narrowed", with the clause that says what that forbids. **Item 5 — `--timeout 0`.** `0` sets a deadline that has already passed, so nothing is waited for and the verb exits `1` under `timeout`. That is what the code does today, on both flags the CLI epoch freezes: - `sessions send --wait --timeout 0`: neither the admission confirmation nor the mark watch gets a chance to finish. Stated in the clap help and in the `send` bullet of `docs/reference/cli.md`, pinned by `send_wait_timeout_zero_gives_up_at_once`. - `notifications subscribe --once --timeout 0`: the subscription waits for no notification, and the stream's terminal is an error object of kind `timeout`. Stated in that flag's clap help and its cli.md bullet, pinned by `notifications_once_timeout_zero_gives_up_at_once`. The help-page insta snapshot was regenerated for both. Both waits check the deadline before reaching for a frame rather than only racing it. `tokio::time::timeout_at` polls its inner future before an expired timer, so a daemon that answered the subscribe with its ack and a notification in one flush left `--timeout 0` printing that notification and exiting `0`. `drain_frames` (the mark watch) already guarded this way; the notification loop and the admission confirmation now do too. `notifications_timeout_zero_outranks_an_already_readable_notification` pins it with a fake daemon that puts both frames in the reader's read-ahead before the CLI polls, and fails on the pre-fix code. No schema change: `felis-cli-v1.schema.json` mentions `timeout` only as an `error.kind` token, and `SessionsSendParams` in the bridge schema carries `session`, `text` and `raw` — the bridge has no wait, so it has no timeout input to annotate. **Item 6 — why relay payload version `0` is malformed.** The reference now gives the reason the explanation doc holds: the version word is how a writer that predates it is told apart, and such a writer puts its entry count where the version belongs with a zero high half. ## Verified - `just fmt-check` — clean - `just lint` (clippy, workspace, all targets, all features) — clean - `just prose-check` — 0 hits - `just docs-links` — 1234 OK, 0 errors - `cargo nextest run --all-features -p felis-daemon -p felis-cli` — 918 passed, 7 skipped - Reverting the deadline guard fails `notifications_timeout_zero_outranks_an_already_readable_notification` 50 times out of 50; with the guard it passes 50 out of 50. - doc-cascade sweep over `--timeout`, `UnknownMode` and the count criterion across `docs/`, `skills/`, `README.md`: no other page restates them.

natsukium added 1 commit 2026-09-22 21:18:50 +09:00

protocol, cli: pin the frozen-surface corners a pre-release review left open ... 89345460d0

Four wire and CLI corners were decided in the code but stated nowhere a
reader of the contract could find, and each one costs a major to change
once v0.1.0 freezes them.

The handshake's unknown-mode rule ran on the mode number without saying
so: a nonzero value the daemon does not define is a newer peer and earns
the typed refusal, while the UNSPECIFIED sentinel is malformed and takes
the corruption path. Whoever adds the first post-baseline mode needs to
know which branch it lands on.

The item/trailer criterion behind ConnEnd.count lived only in the region
stream's discussion, so the next family adding a trailer had to re-derive
it from two precedents.

The CLI epoch promised that error.kind tokens are never respelled or
remapped, which still left a new token free to take over part of an
existing one's meaning. Narrowing breaks a consumer exactly the way
respelling does.

`sessions send --timeout 0` had a meaning the code gave it and no
document held: a deadline already past, so both waits give up at once
under `timeout`. Leaving it unstated would have made any later reading of
0 as "unbounded" or as a usage error a breaking change.

The relay block's version-0 rule was stated without its reason, which a
reference-only reader implementing another relay cannot supply: the
version word is how a writer that predates it is told apart, and such a
writer puts a zero high half where the version belongs.

Assisted-by: Claude Code

natsukium added 1 commit 2026-09-22 21:21:53 +09:00

cli: define notifications subscribe --timeout 0 the way sessions send defines it ... 52a0b8a7a6

The two verbs compute their deadline the same way, so `0` already means
the same thing in both: an instant already past, which the first wait
compares against and gives up on. For the subscription that means no
notification is waited for at all and the stream's terminal is an error
object of kind `timeout`.

Both flags are frozen by the same CLI epoch, so leaving one of them
unstated would make any later reading of 0 — unbounded, or a usage error
— a breaking change on a surface that cannot take one.

Assisted-by: Claude Code

natsukium added 1 commit 2026-09-22 21:46:29 +09:00

cli: check an elapsed deadline before the wait, not only against it ... 5e40e7b157

`tokio::time::timeout_at` polls its inner future before it looks at an
expired timer, so a wait wrapped in it returns whatever is already
readable no matter how long ago the deadline passed. A daemon that
answered the subscribe with its ack and a notification in one flush
therefore left `notifications subscribe --once --timeout 0` printing that
notification and exiting 0, against a contract that says the verb gives
up at once.

Both waits now test the deadline themselves before reaching for a frame,
which is what the mark watch in `drain_frames` already did. That makes
`--timeout 0` mean one thing across the two verbs the CLI epoch freezes:
the deadline is checked before any wait, and a frame that happens to be
readable is not consumed.

The new test's daemon writes the ack and the event in a single flush so
both are in the reader's read-ahead before the CLI polls; a silent-session
test cannot reach the path, because it is the readable frame that does the
damage.

Assisted-by: Claude Code

natsukium added 3 commits 2026-09-22 22:16:40 +09:00

cli: make the read-ahead timeout test decide the guard rather than race it ... b58c5cd3d0

The daemon half answered the subscribe only after reading it, which left
the notification arriving while the CLI was already polling. `FrameReader`
then took the ack and the event in separate `read_buf` calls, and the
event stayed in the kernel, where an unregistered readiness makes the
first poll pending. The wait timed out for the wrong reason, so the test
reported the guard present when it was not: against sources without it,
one run in fifty still passed.

Answering `Hello` with `Welcome`, the ack and the event in one write
lands all three in the reader's buffer in a single `read_buf`, so the
event is decoded before the stream is opened and the wait's first poll
can only be ready. Reverting the guard now fails the test fifty times
out of fifty.

Assisted-by: Claude Code

docs: sort ConnEnd.count by what the request asked for, not by the terminal ... 76011ee5c3

Asking whether a family-agnostic terminal would carry a frame sorts none
of the three precedents the bullet names. `RegionMsg::RowsDone` is a
trailer precisely because `exit_code` is data that terminal has no field
for, and `Subscribed` and `Lagged` carry family-specific data of their
own; by that reading all three would count, which is the opposite of the
rule.

The request settles it directly: its result values are the count, and an
acknowledgement, a loss marker or a family trailer escorts them without
joining them, whatever data the trailer has to carry.

Assisted-by: Claude Code

docs: say mode 0 is unwritable by contract, not by the schema ... e151a87a48

`CONNECTION_MODE_UNSPECIFIED = 0` is protobuf's default for the field,
so a sender generated from the schema can reach the wire with it by
simply leaving the mode unset. Claiming the schema prevents it confuses
what the schema can represent with what the protocol allows, and a
reader implementing another client would be entitled to disbelieve the
rule the sentence is there to support.

What holds is the protocol's own requirement: naming a mode is
mandatory, so a sender that omits it is not conforming, which is why the
value is malformed rather than the mark of a newer peer.

Assisted-by: Claude Code

natsukium added 1 commit 2026-09-22 22:27:25 +09:00

Merge remote-tracking branch 'origin/main' into protocol/issue-347-cheap-before-freeze e775c4b040

natsukium merged commit 13560a9ac7 into main 2026-09-22 22:48:40 +09:00

natsukium deleted branch protocol/issue-347-cheap-before-freeze 2026-09-22 22:48:40 +09:00

natsukium referenced this pull request from a commit 2026-09-22 22:48:41 +09:00

protocol, cli: pin the frozen-surface corners a pre-release review left open (#354)

Commenting is not possible because the repository is archived.

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

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!354

No description provided.