natsukium/felis
Archived
Watch
1
Code Issues 23 Pull requests Projects Releases 6 Packages Wiki Activity Actions

Add a correlated input-admission fence for sessions send #306

Closed
opened 2026-09-21 12:09:06 +09:00 by natsukium · 3 comments
natsukium commented 2026-09-21 12:09:06 +09:00
Owner
Copy link

Problem

felis sessions send needs to keep its connection alive until the daemon has admitted every preceding input frame. It currently establishes that ordering by issuing a second Ops::List query and waiting for the roster reply. The IPC reference documents the mechanism only as "reads a query back across the same connection".

This works because the inbound pump reads and admits frames in socket order before it handles the later query. The query itself is unrelated to input, however:

  • it materializes and returns a roster the caller does not need;
  • an external client has to know that an arbitrary later query doubles as an input-admission barrier;
  • the completion point is easy to overstate as child consumption, although it means only that the daemon has retained responsibility for the bytes.

This is not a v0.1 blocker because the existing ordering is functional and documented. A typed fence would make the guarantee explicit when the IPC next grows.

Proposed contract

Add a correlated attached-session request/reply such as InputFence / InputAccepted.

The reply should mean:

Every input frame preceding this fence on this connection has passed the session input budget and has been accepted into the session/PTY writer path. The child need not have read the bytes yet.

Processing the fence through the session actor after earlier input commands would give it a stronger and easier-to-test meaning than the current roster reply while preserving bounded backpressure. sessions send can use the fence at an effective minor that defines it and retain the roster-query fallback for an older daemon.

Acceptance criteria

  • The reply's completion boundary distinguishes admission, PTY write completion, and child consumption.
  • The fence cannot overtake earlier input from the same connection.
  • A client may close after the reply without causing admitted bytes to be abandoned.
  • Waiting for input budget remains interruptible by peer hangup and session termination.
  • sessions send --timeout covers the fence wait without changing its documented exit classes.
  • Mixed-minor behavior is defined; newer clients retain a safe fallback for v0.1 daemons.
  • Tests cover an empty fence, input followed by a fence, a budget-stalled input, session exit, and disconnect after acknowledgement.
  • IPC and CLI references state the same guarantee.

Principle check

  1. Add only what earns its place — Pass. sessions send is an existing consumer, and the proposal replaces an unrelated roster operation with a typed IPC contract.
  2. Render everything, fast — Pass. The path is input admission, not rendering.
  3. The daemon owns state, the client owns pixels — Pass. The daemon acknowledges the queue and budget it owns.
  4. Explicit over heuristic — Pass. The acknowledgement names its completion point directly.

Evidence

  • crates/felis-cli/src/cli_sessions.rs (cmd_send post-write roster query)
  • crates/felis-daemon/src/serve.rs (pump_inbound input reservation and Ops routing)
  • crates/felis-daemon/src/serve/session_task.rs (SessionCmd::Input)
  • crates/felis-daemon/src/serve/streaming.rs (write_pty_reserved)
  • docs/reference/ipc.md "Client → daemon: the input budget"
  • docs/reference/cli.md sessions send
## Problem `felis sessions send` needs to keep its connection alive until the daemon has admitted every preceding input frame. It currently establishes that ordering by issuing a second `Ops::List` query and waiting for the roster reply. The IPC reference documents the mechanism only as "reads a query back across the same connection". This works because the inbound pump reads and admits frames in socket order before it handles the later query. The query itself is unrelated to input, however: - it materializes and returns a roster the caller does not need; - an external client has to know that an arbitrary later query doubles as an input-admission barrier; - the completion point is easy to overstate as child consumption, although it means only that the daemon has retained responsibility for the bytes. This is not a v0.1 blocker because the existing ordering is functional and documented. A typed fence would make the guarantee explicit when the IPC next grows. ## Proposed contract Add a correlated attached-session request/reply such as `InputFence` / `InputAccepted`. The reply should mean: > Every input frame preceding this fence on this connection has passed the session input budget and has been accepted into the session/PTY writer path. The child need not have read the bytes yet. Processing the fence through the session actor after earlier input commands would give it a stronger and easier-to-test meaning than the current roster reply while preserving bounded backpressure. `sessions send` can use the fence at an effective minor that defines it and retain the roster-query fallback for an older daemon. ## Acceptance criteria - [ ] The reply's completion boundary distinguishes admission, PTY write completion, and child consumption. - [ ] The fence cannot overtake earlier input from the same connection. - [ ] A client may close after the reply without causing admitted bytes to be abandoned. - [ ] Waiting for input budget remains interruptible by peer hangup and session termination. - [ ] `sessions send --timeout` covers the fence wait without changing its documented exit classes. - [ ] Mixed-minor behavior is defined; newer clients retain a safe fallback for v0.1 daemons. - [ ] Tests cover an empty fence, input followed by a fence, a budget-stalled input, session exit, and disconnect after acknowledgement. - [ ] IPC and CLI references state the same guarantee. ## Principle check 1. **Add only what earns its place — Pass.** `sessions send` is an existing consumer, and the proposal replaces an unrelated roster operation with a typed IPC contract. 2. **Render everything, fast — Pass.** The path is input admission, not rendering. 3. **The daemon owns state, the client owns pixels — Pass.** The daemon acknowledges the queue and budget it owns. 4. **Explicit over heuristic — Pass.** The acknowledgement names its completion point directly. ## Evidence - `crates/felis-cli/src/cli_sessions.rs` (`cmd_send` post-write roster query) - `crates/felis-daemon/src/serve.rs` (`pump_inbound` input reservation and `Ops` routing) - `crates/felis-daemon/src/serve/session_task.rs` (`SessionCmd::Input`) - `crates/felis-daemon/src/serve/streaming.rs` (`write_pty_reserved`) - `docs/reference/ipc.md` "Client → daemon: the input budget" - `docs/reference/cli.md` `sessions send`
natsukium commented 2026-09-21 13:02:48 +09:00
Author
Owner
Copy link

Direction agreed 2026-09-21 after three sol design-review rounds (final verdict PASS). Full plan and batch order: see the comment on #12.

  • #306 — deferred; the roster-query barrier is functional and documented,
    and #309 preserves it for structured keys through the budget reservation.
Direction agreed 2026-09-21 after three `sol` design-review rounds (final verdict PASS). Full plan and batch order: see the comment on #12. - **#306** — deferred; the roster-query barrier is functional and documented, and #309 preserves it for structured keys through the budget reservation.
natsukium commented 2026-09-21 23:18:40 +09:00
Author
Owner
Copy link

The final pre-freeze interface audit makes this a v0.1/P1 item rather than a post-release cleanup.

There is no published daemon that requires the Ops::List fallback. Shipping that fallback would permanently couple input admission to an unrelated roster query and would make every external implementation learn an accidental ordering trick. Land the typed InputFence / acknowledgement in protocol 1.0 and remove the roster-query barrier rather than carrying mixed-minor fallback code into the first baseline.

This also resolves finding 1 in #326: SessionMsg correlation has a concrete use, so retain the field-100 correlation envelope instead of removing it.

The completion boundary in this issue remains correct: acknowledgement means daemon admission/ownership, not child consumption.

The final pre-freeze interface audit makes this a v0.1/P1 item rather than a post-release cleanup. There is no published daemon that requires the `Ops::List` fallback. Shipping that fallback would permanently couple input admission to an unrelated roster query and would make every external implementation learn an accidental ordering trick. Land the typed `InputFence` / acknowledgement in protocol 1.0 and remove the roster-query barrier rather than carrying mixed-minor fallback code into the first baseline. This also resolves finding 1 in #326: `SessionMsg` correlation has a concrete use, so retain the field-100 correlation envelope instead of removing it. The completion boundary in this issue remains correct: acknowledgement means daemon admission/ownership, not child consumption.
natsukium commented 2026-09-22 22:32:55 +09:00
Author
Owner
Copy link

Triage plan (2026-09-22)

Direction reviewed through nine sol design rounds (final verdict PASS). The plan below is the agreed shape; the PR implements it as written and records deviations in its body.

Wire (crates/felis-protocol/proto/felis.proto):

  • SessionMsg gains SessionInputFence input_fence = 8 with (felis.v1.arm) = TO_DAEMON, CORRELATION_REQUEST_OPENER, modes WINDOW+OPS, phase ATTACHED, since 0; and SessionInputAccepted input_accepted = 9 = TO_CLIENT, CORRELATION_REQUEST_REPLY. Both messages are empty. The field-100 correlation slot comment ("no arm uses it") and the ipc.md correlation table Session row (ipc.md:389) change to name this pair.
  • Domain: messages/session.rs gains the two variants with CorrelationClass::RequestOpener / RequestReply in Directed::ARMS; convert/session.rs maps them; minor.rs MinorGated for SessionMsg folds the new arms through arm_requires (since 0, so no ledger row).

Meaning of InputAccepted: every Input frame that preceded the fence on this connection has been processed by the session actor. For the byte-carrying arms (KeyBytes, Paste, Key, Mouse) the bytes they encoded to were admitted against the input budget and queued to the PTY writer. The other arms (Resize, FocusChange, ColorScheme, viewport, frame pull, prompt jump) carry no budget reservation; the reports some of them write to the PTY under DECSET 1004 / 2031 / 2048 are unreserved writes that the actor attempts before it reaches the fence (write_pty returns success even when PtyWriter answers WriteOutcome::Dropped on a saturated gauge), so they are ordered ahead of the reply but best-effort: a report can be dropped and the fence still completes. The reference says exactly that: attempt order is guaranteed, delivery of generated reports is not. It does not mean the child read anything. Tests: a resize before the fence (no report); a focus change with DECSET 1004 enabled before the fence on an unsaturated writer, asserting that the report was enqueued (WriteOutcome::Queued) before the fence reply was enqueued to the outbox, and that the report is eventually read from the PTY (no ordering against the client's receipt of the reply, which travels independently through the subscriber outbox and can arrive first); the same on a saturated writer, asserting the fence still completes while the report was dropped.

Daemon:

  • serve.rs::route_frame MessageKind::Session arm: InputFence becomes Route::Cmd(SessionCmd::InputFence { sub, request: delivered.request() }).
  • session_task.rs: new SessionCmd::InputFence { sub, request }; the actor answers with OutEvent::Session { msg: SessionMsg::InputAccepted, correlation: Correlation::request(request) } through push_to, the handle_region pattern. OutEvent has no Session variant: add OutEvent::Session { msg, correlation } and its encoding arm in streaming::body_for (serve/streaming.rs around line 916, the exhaustive match that write_out_event delegates to) using CheckedFrame::encode_correlated.
  • Ordering argument, to be pinned by tests: pump_inbound acquires the input-budget permit for an Input frame before it forwards SessionCmd::Input on the actor channel, and it does not read the next frame until then. A fence therefore cannot overtake a budget-stalled input: FrameReader may have read the fence's bytes ahead into its buffer (wait_for_hangup keeps bounded read-ahead going while admission stalls), but the fence is not taken from the reader or dispatched to the session actor until the preceding input's reservation succeeds and that input is forwarded. Once forwarded, the actor processes commands in channel order, so by the time it sees the fence it has run handle_admitted_input for every earlier input.
  • Interruptibility: the budget wait is already raced against wait_for_hangup and cmd.closed(); a session exit closes the channel and the connection ends, which the client reports as session_ended/daemon_lost as today.

Structural JSON (felis_json: 1): the format carries no correlation envelope (json_v1::Frame::from_body decodes the body only and into_body uses codec::encode), and ipc.md says the families the bridge serves belong to the CLI output contract instead. The fence pair is a correlated request/reply driven by the CLI and bridge, so it is excluded from felis_json rather than added: the SessionMsg → SessionJson conversion in crates/felis-grid/src/json_v1/session.rs maps InputFence and InputAccepted to the error the format already uses for a body outside it (find the variant of JsonError for an unrepresentable body, or add one), a test pins that both arms are refused with that error, and the ipc.md felis-json section lists the two arms as outside the format with the reason (no correlation slot). v1 stays unchanged: no schema or golden regeneration.

Protocol fixtures: session_cases() in crates/felis-protocol/src/messages/session.rs (around lines 270-433) is exhaustive and session_cases_cover_every_variant fails without the new arms; add both, with the correlation-class assertions the other correlated families' fixtures make (request opener / request reply), and a codec round trip through encode_correlated with a nonzero request id.

Client-core: connector.rs gains input_fence() beside list_sessions(), a correlated_request over a FenceReq shaped like ListReq.

CLI: cli_sessions.rs::cmd_send replaces roster(&mut conn) at the barrier with conn.input_fence(); the timeout and error-kind mapping are unchanged (timeout on elapsed deadline, daemon_lost on failure). No roster fallback: no daemon is published, so the fence is part of the base schema. The doc comment at cli_sessions.rs:49 loses the "List keeps the connection open" role for send.

Bridge: cli_bridge.rs Link::send_input (around line 2585) has its own copy of the barrier (self.request(&OpsMsg::List) after the input). It switches to the fence through the same correlated-request path (self.request(&SessionMsg::InputFence)). Link::decode today decodes every correlated SessionMsg to None, which dispatch_payload reports as a droppable frame, so the reply would be dropped as written: add DaemonMsg::Session(SessionMsg), route MessageKind::Session through it in Link::decode, and have send_input match SessionMsg::InputAccepted (any other session reply on that correlation is a protocol error). Bridge tests prove sessions.send waits for InputAccepted and that no Ops::List frame follows the input (the fake-daemon harness in the bridge tests reads the next frame after the input and asserts it is the fence).

Tests:

  • crates/felis-cli/tests/cli_sessions.rs rogue_daemon: the frame after the input is now the fence.
  • Existing send_waits_for_admission_rather_than_dropping_its_input and send_wait_timeout_bounds_the_admission_of_a_wedged_child keep passing unchanged in intent.
  • Daemon (serve/tests.rs or a new integration test): empty fence on a fresh attach answers immediately; input then fence answers after the input reached the writer; a budget-saturated input followed by a fence is not answered until the budget frees (the fence never overtakes); a client that closes after InputAccepted does not lose the admitted bytes (the child reads them).
  • Session exit: the actor's run_session select is biased toward commands, so a fence that is already queued when the PTY reaches EOF may legitimately be answered before the exit. The guaranteed boundary is actor-channel closure: once the session has ended (the client has seen SessionExited, or the actor channel is closed), a fence gets no reply and the connection ends. The test pins that boundary: send SessionExited to be observed first (or close the actor), then send the fence, and assert the connection ends without InputAccepted. No new lifecycle rule; the reference states that a fence answered during exit means only that the bytes were admitted before the session ended.

Docs: ipc.md Session (kind = 4) arm list, correlation table row, the input-budget paragraph (ipc.md:1824 "reads a query back"), and the SessionMsg proto comments; cli.md send bullet (the admission sentence); docs/explanation/architecture/ipc.md wherever the roster barrier is discussed; skills/felis/SKILL.md if it describes the barrier.

## Triage plan (2026-09-22) Direction reviewed through nine `sol` design rounds (final verdict PASS). The plan below is the agreed shape; the PR implements it as written and records deviations in its body. Wire (`crates/felis-protocol/proto/felis.proto`): - `SessionMsg` gains `SessionInputFence input_fence = 8` with `(felis.v1.arm)` = TO_DAEMON, CORRELATION_REQUEST_OPENER, modes WINDOW+OPS, phase ATTACHED, since 0; and `SessionInputAccepted input_accepted = 9` = TO_CLIENT, CORRELATION_REQUEST_REPLY. Both messages are empty. The field-100 correlation slot comment ("no arm uses it") and the ipc.md correlation table Session row (ipc.md:389) change to name this pair. - Domain: `messages/session.rs` gains the two variants with `CorrelationClass::RequestOpener` / `RequestReply` in `Directed::ARMS`; `convert/session.rs` maps them; `minor.rs` `MinorGated for SessionMsg` folds the new arms through `arm_requires` (since 0, so no ledger row). Meaning of `InputAccepted`: every `Input` frame that preceded the fence on this connection has been processed by the session actor. For the byte-carrying arms (`KeyBytes`, `Paste`, `Key`, `Mouse`) the bytes they encoded to were admitted against the input budget and queued to the PTY writer. The other arms (`Resize`, `FocusChange`, `ColorScheme`, viewport, frame pull, prompt jump) carry no budget reservation; the reports some of them write to the PTY under DECSET 1004 / 2031 / 2048 are unreserved writes that the actor attempts before it reaches the fence (`write_pty` returns success even when `PtyWriter` answers `WriteOutcome::Dropped` on a saturated gauge), so they are ordered ahead of the reply but best-effort: a report can be dropped and the fence still completes. The reference says exactly that: attempt order is guaranteed, delivery of generated reports is not. It does not mean the child read anything. Tests: a resize before the fence (no report); a focus change with DECSET 1004 enabled before the fence on an unsaturated writer, asserting that the report was enqueued (`WriteOutcome::Queued`) before the fence reply was enqueued to the outbox, and that the report is eventually read from the PTY (no ordering against the client's receipt of the reply, which travels independently through the subscriber outbox and can arrive first); the same on a saturated writer, asserting the fence still completes while the report was dropped. Daemon: - `serve.rs::route_frame` `MessageKind::Session` arm: `InputFence` becomes `Route::Cmd(SessionCmd::InputFence { sub, request: delivered.request() })`. - `session_task.rs`: new `SessionCmd::InputFence { sub, request }`; the actor answers with `OutEvent::Session { msg: SessionMsg::InputAccepted, correlation: Correlation::request(request) }` through `push_to`, the `handle_region` pattern. `OutEvent` has no `Session` variant: add `OutEvent::Session { msg, correlation }` and its encoding arm in `streaming::body_for` (`serve/streaming.rs` around line 916, the exhaustive match that `write_out_event` delegates to) using `CheckedFrame::encode_correlated`. - Ordering argument, to be pinned by tests: `pump_inbound` acquires the input-budget permit for an `Input` frame before it forwards `SessionCmd::Input` on the actor channel, and it does not read the next frame until then. A fence therefore cannot overtake a budget-stalled input: `FrameReader` may have read the fence's bytes ahead into its buffer (`wait_for_hangup` keeps bounded read-ahead going while admission stalls), but the fence is not taken from the reader or dispatched to the session actor until the preceding input's reservation succeeds and that input is forwarded. Once forwarded, the actor processes commands in channel order, so by the time it sees the fence it has run `handle_admitted_input` for every earlier input. - Interruptibility: the budget wait is already raced against `wait_for_hangup` and `cmd.closed()`; a session exit closes the channel and the connection ends, which the client reports as `session_ended`/`daemon_lost` as today. Structural JSON (`felis_json: 1`): the format carries no correlation envelope (`json_v1::Frame::from_body` decodes the body only and `into_body` uses `codec::encode`), and ipc.md says the families the bridge serves belong to the CLI output contract instead. The fence pair is a correlated request/reply driven by the CLI and bridge, so it is excluded from `felis_json` rather than added: the `SessionMsg` → `SessionJson` conversion in `crates/felis-grid/src/json_v1/session.rs` maps `InputFence` and `InputAccepted` to the error the format already uses for a body outside it (find the variant of `JsonError` for an unrepresentable body, or add one), a test pins that both arms are refused with that error, and the ipc.md `felis-json` section lists the two arms as outside the format with the reason (no correlation slot). v1 stays unchanged: no schema or golden regeneration. Protocol fixtures: `session_cases()` in `crates/felis-protocol/src/messages/session.rs` (around lines 270-433) is exhaustive and `session_cases_cover_every_variant` fails without the new arms; add both, with the correlation-class assertions the other correlated families' fixtures make (request opener / request reply), and a codec round trip through `encode_correlated` with a nonzero request id. Client-core: `connector.rs` gains `input_fence()` beside `list_sessions()`, a `correlated_request` over a `FenceReq` shaped like `ListReq`. CLI: `cli_sessions.rs::cmd_send` replaces `roster(&mut conn)` at the barrier with `conn.input_fence()`; the timeout and error-kind mapping are unchanged (`timeout` on elapsed deadline, `daemon_lost` on failure). No roster fallback: no daemon is published, so the fence is part of the base schema. The doc comment at cli_sessions.rs:49 loses the "List keeps the connection open" role for `send`. Bridge: `cli_bridge.rs` `Link::send_input` (around line 2585) has its own copy of the barrier (`self.request(&OpsMsg::List)` after the input). It switches to the fence through the same correlated-request path (`self.request(&SessionMsg::InputFence)`). `Link::decode` today decodes every correlated `SessionMsg` to `None`, which `dispatch_payload` reports as a droppable frame, so the reply would be dropped as written: add `DaemonMsg::Session(SessionMsg)`, route `MessageKind::Session` through it in `Link::decode`, and have `send_input` match `SessionMsg::InputAccepted` (any other session reply on that correlation is a protocol error). Bridge tests prove `sessions.send` waits for `InputAccepted` and that no `Ops::List` frame follows the input (the fake-daemon harness in the bridge tests reads the next frame after the input and asserts it is the fence). Tests: - `crates/felis-cli/tests/cli_sessions.rs` `rogue_daemon`: the frame after the input is now the fence. - Existing `send_waits_for_admission_rather_than_dropping_its_input` and `send_wait_timeout_bounds_the_admission_of_a_wedged_child` keep passing unchanged in intent. - Daemon (`serve/tests.rs` or a new integration test): empty fence on a fresh attach answers immediately; input then fence answers after the input reached the writer; a budget-saturated input followed by a fence is not answered until the budget frees (the fence never overtakes); a client that closes after `InputAccepted` does not lose the admitted bytes (the child reads them). - Session exit: the actor's `run_session` select is biased toward commands, so a fence that is already queued when the PTY reaches EOF may legitimately be answered before the exit. The guaranteed boundary is actor-channel closure: once the session has ended (the client has seen `SessionExited`, or the actor channel is closed), a fence gets no reply and the connection ends. The test pins that boundary: send `SessionExited` to be observed first (or close the actor), then send the fence, and assert the connection ends without `InputAccepted`. No new lifecycle rule; the reference states that a fence answered during exit means only that the bytes were admitted before the session ended. Docs: ipc.md Session (kind = 4) arm list, correlation table row, the input-budget paragraph (ipc.md:1824 "reads a query back"), and the `SessionMsg` proto comments; cli.md `send` bullet (the admission sentence); `docs/explanation/architecture/ipc.md` wherever the roster barrier is discussed; `skills/felis/SKILL.md` if it describes the barrier.
Commenting is not possible because the repository is archived.
No Branch/Tag specified
Branches Tags
main
v0.1.0-rc.9
v0.1.0-rc.8
v0.1.0-rc.7
v0.1.0-rc.6
v0.1.0-rc.5
v0.1.0-rc.4
v0.1.0-rc.3
v0.1.0-rc.2
v0.1.0-rc.1
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#306
No description provided.