natsukium/felis
Watch
1
Code Issues 27 Pull requests 1 Projects Releases Packages Wiki Activity Actions

[v0.1/P0] Add aggregate daemon admission and handshake limits #14

New issue
Closed
opened 2026-09-03 16:17:59 +09:00 by natsukium · 1 comment
natsukium commented 2026-09-03 16:17:59 +09:00
Owner
Copy link

Parent: #12 (P0.1). Supersedes the aggregate-budget part of #10.

Why

Per-resource ceilings do not compose into a process budget. Silent same-UID peers can also retain a task and file descriptor indefinitely before completing the handshake.

Scope

  • Add one daemon admission object backed by owned permits released on drop.
  • Account for connection phase, sessions and PTY threads, grid/scrollback, images, decode work, subscribers, queued subscriber bytes, and pending PTY input.
  • Cap connections immediately after accept.
  • Apply distinct deadlines to the preface, Hello, and first operation.
  • Reject before entering a long-lived phase when no permit is available.

Acceptance criteria

  • The product of admitted resources fits a documented daemon-wide budget.
  • Silent peers are bounded by both count and time.
  • Every permit is released on success, refusal, timeout, disconnect, and task failure.
  • Capacity failures are typed at_capacity/refused outcomes rather than allocation failures.
  • Multi-session stress tests assert the aggregate bound.
  • daemon status, spec, security model, and architecture docs describe the same accounting model.
Parent: #12 (P0.1). Supersedes the aggregate-budget part of #10. ## Why Per-resource ceilings do not compose into a process budget. Silent same-UID peers can also retain a task and file descriptor indefinitely before completing the handshake. ## Scope - Add one daemon admission object backed by owned permits released on drop. - Account for connection phase, sessions and PTY threads, grid/scrollback, images, decode work, subscribers, queued subscriber bytes, and pending PTY input. - Cap connections immediately after `accept`. - Apply distinct deadlines to the preface, `Hello`, and first operation. - Reject before entering a long-lived phase when no permit is available. ## Acceptance criteria - [ ] The product of admitted resources fits a documented daemon-wide budget. - [ ] Silent peers are bounded by both count and time. - [ ] Every permit is released on success, refusal, timeout, disconnect, and task failure. - [ ] Capacity failures are typed `at_capacity`/`refused` outcomes rather than allocation failures. - [ ] Multi-session stress tests assert the aggregate bound. - [ ] `daemon status`, spec, security model, and architecture docs describe the same accounting model.
natsukium commented 2026-09-03 20:35:10 +09:00
Author
Owner
Copy link

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.

  • No connection cap. crates/felis-daemon/src/serve.rs:246-266: the accept loop spawns handle_connection for every accepted stream with no count, permit, or gauge. DaemonCaps (serve.rs:141-165) carries only idle, max_sessions, agent, endpoint.
  • No handshake deadlines. exchange_bootstrap (serve.rs:1718-1753) awaits read_client_bootstrap with no timeout; handshake (serve.rs:1913-1935) awaits reader.next_frame() for Hello with no timeout; wait_for_attach (serve.rs:424-427) awaits the first operation with no timeout. grep timeout across felis-daemon/src and felis-transport/src finds only test code (parse_sink.rs, lib.rs:428, framing.rs:355). A silent same-UID peer holds a tokio task, a fd, a FrameReader (READ_CHUNK 8 KiB) and a BufWriter (WRITE_BUF_CAPACITY) forever.
  • Per-resource ceilings exist but do not compose. Sessions: MAX_SESSIONS = 256 (pool.rs:46) enforced via try_reserve + RAII SessionSlot (serve.rs:485-506, pool.rs:389-400). Per session: grid 512 MiB budget (REQ-605a, messages.rs:97), image store 256 MiB (pool.rs:38), decode reassembly 64 MiB. Per subscriber: SUBSCRIBER_BUFFER_CAP = 512 MiB (session_task.rs:46), eviction not backpressure. Subscriber count per session is unbounded; connections are unbounded. So the daemon-wide product is 256 × ~832 MiB + connections × 512 MiB + pending PTY input (unbounded, #15).
  • Status reports five rows, none for connections (serve.rs:928-1005): sessions, image-store bytes, in-flight decodes, decode bytes, deepest subscriber queue.
  • Typed capacity outcomes exist only for sessions: AttachFailure::SessionLimitReached → CLI at_capacity (CHANGELOG Unreleased). RefusalReason has a single variant Role (felis-protocol/src/messages/conn.rs:84-88).

One thing the issue understates: the recorded decision at docs/reference/ipc.md:1802-1816 and docs/explanation/architecture/session-lifecycle.md:1049-1058 is that subscriber outboxes are unbounded with a gauge and evicted on volume, never on time, because a bounded channel would apply one slow mirror's backpressure to the session task serving everyone. #14 must account for those bytes, not replace the eviction policy.

Verdict

accept-with-changes. The scope list ("one admission object" accounting for connection phase, sessions, PTY threads, grid/scrollback, images, decode, subscribers, queued bytes, pending input) over-reaches for what is missing. Grid, image, and decode budgets are already per-session constants whose aggregate is max_sessions × constant; re-accounting them through a second object adds a lock on the hot path for no new bound. What is actually unbounded is (a) connection count, (b) time spent pre-attach, and (c) pending PTY input (#15). Do these, and document the product of the fixed per-session constants as the daemon-wide budget rather than enforcing it twice.

Principle check: principle 3 test ("changing a client config requires a daemon restart") is untouched; control-surfaces.md:173-178 records that limits are compiled in and the daemon reads no config; keep it that way (no --max-connections flag; a DaemonCaps field for embedders/tests, as max_sessions already is).

Approach

felis-daemon

  • DaemonCaps gains max_connections: usize (compiled default MAX_CONNECTIONS = 1024 in pool.rs, 4× sessions: every window plus a bridge, an observer, and a felis sessions call per session) and handshake_deadlines: HandshakeDeadlines { preface: 2 s, hello: 5 s, first_op: 30 s }.
  • Admission: Arc<tokio::sync::Semaphore> sized max_connections, held in the serve loop; try_acquire_owned() at serve.rs:247 before tokio::spawn. The OwnedSemaphorePermit moves into the task and is dropped on every exit path (success, refusal, timeout, disconnect, panic, because tokio drops the future's captures on task failure). This is the "owned permits released on drop" the issue asks for; SessionSlot already has that shape for sessions.
  • Over-cap refusal: still run the preface (fixed 8-byte exchange, cheap) and read Hello under the deadline, then write ConnMsg::Refused { reason: RefusalReason::AtCapacity, detail: "the daemon is at N of M connections" } and close. Add RefusalReason::AtCapacity (proto enum tail value; a minor addition under #50's ledger, and swept into #30's 2.0 reset). Reading Hello for an over-cap peer is bounded by the deadline and by MAX_READAHEAD (framing.rs:51), so it costs no allocation beyond the read buffer. Rationale to record: closing the socket without a frame would make "daemon busy" indistinguishable from "daemon crashed" to felis and the bridge, which must surface at_capacity (retry) rather than daemon (exit 2).
  • Deadlines: wrap read_client_bootstrap (preface + carrier block), handshake's first next_frame, and wait_for_attach's first next_frame in tokio::time::timeout; a timeout is a new ConnError::HandshakeTimeout { phase } logged at debug (a peer that says nothing is not worth a warn). Only the first pre-attach frame is timed: a bridge (Ops mode) legitimately idles between verbs after its first operation, and a Notify observer idles for hours.
  • Status: add ResourceKind::Connections (count, limit max_connections, scope Daemon) to daemon_status (serve.rs:965-1004); used = max_connections - semaphore.available_permits().
  • Subscriber count: with a connection cap, subscribers ≤ connections, so no separate per-session subscriber cap; document the bound connections × SUBSCRIBER_BUFFER_CAP as the queued-bytes ceiling instead of adding a second gauge.
  • Pending PTY input: owned by #15; #14's DaemonCaps should reserve the field name (pty_input_budget) so #15 slots in without a second config surface.

felis-client-core / felis-cli: map ConnMsg::Refused { AtCapacity } during dial to ConnectError::AtCapacity → CLI/bridge at_capacity error kind (already exists for sessions, skills/felis/SKILL.md:234).

Tests

  • serve/tests.rs: DaemonCaps { max_connections: 2, .. } via handle_stream with in-memory duplex streams: third dial gets Refused(AtCapacity); after one disconnect the next dial succeeds (permit released); a handler that returns Err mid-handshake releases; tokio::time::pause + advance to prove each of the three deadlines fires and releases the permit; a peer that sends the preface and stops is cut at hello; a peer that completes Hello and stops is cut at first_op; a bridge idle after one op is not cut.
  • Stress: 16 sessions × N mirrors with max_connections small enough to refuse; assert daemon status rows connections.used ≤ limit throughout and equal to zero after teardown (proves every permit returned).

Docs cascade

  • docs/reference/spec.md: new REQ-916 (connection cap + three deadlines + typed refusal); REQ-915 gains a sentence that the daemon-wide budget is max_sessions × (grid + image + decode) + max_connections × SUBSCRIBER_BUFFER_CAP + the #15 input budget; REQ-1102 row list gains connections.
  • docs/reference/ipc.md: "Handshake" section gets a deadline table; "Backpressure" gets a paragraph that the pre-attach phase is time-bounded while steady state is volume-bounded (keeps the recorded "cut on volume, never on time" for subscribers accurate by scoping it).
  • docs/explanation/security-model.md "Daemon IPC": bullets for the connection cap and silent-peer bound, with the rejected alternative (per-session subscriber cap) and "Revisit if a mirror count above max_connections / max_sessions appears".
  • docs/explanation/architecture/session-lifecycle.md (admission list at :56-64), docs/explanation/architecture/control-surfaces.md "Diagnostic verbs" (new row), docs/reference/cli.md "Daemon status".
  • CHANGELOG.md Unreleased: new limit (1024 connections, typed at_capacity on connect), handshake deadlines, new status row.
  • skills/felis/SKILL.md §"Is the daemon healthy?": new row; note that at_capacity can now arrive at connect time, not only at spawn.

Dependencies

  • Independent of #13/#16/#49. Coordinate with #15 (input budget field) and #26 (status scope semantics: the connections row is Daemon-scoped, uncontroversial). The RefusalReason::AtCapacity and ResourceKind::Connections additions are wire edits that must precede #30's 2.0 reset and appear in #50's send-authorization ledger. #12's order (#14 before #15/#16/#49) still holds.

Risk/effort

M. Semaphore + three timeouts + one enum value is S; the tests and the six-document cascade are where the time goes. Main risk: a deadline too tight for the SSH relay path. The relay (felis-daemon relay) dials the remote daemon locally after sshd has already authenticated, so the preface deadline starts only once the local connect lands; keep preface at 2 s regardless, but verify with the relay integration test before choosing numbers.

Labels

Keep priority/P0, release/v0.1.0. An unbounded, un-timed pre-attach phase reachable by any same-UID process is a daemon-availability hole the security model (security-model.md:269, "second-largest attack surface") already claims to close.

Review amendments (round 1)

  • Over-cap peers must not get an unbounded task. Acquire the connection permit (try_acquire_owned) in the accept loop before tokio::spawn. When no permit is available, the refusal path is itself bounded by a second, small semaphore (MAX_REFUSALS_IN_FLIGHT, e.g. 16): with a refusal permit, spawn a task that runs only the fixed preface exchange and reads Hello under the handshake deadline, then writes Refused { AtCapacity } and closes; without one, drop the socket in the accept loop without writing anything. Silent peers can therefore hold at most max_connections + MAX_REFUSALS_IN_FLIGHT tasks and fds. The connections status row counts admitted permits only. Test: with max_connections = 1 and MAX_REFUSALS_IN_FLIGHT = 1, two silent extra dials leave exactly one refusal task alive and the third is closed immediately.
## 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. - **No connection cap.** `crates/felis-daemon/src/serve.rs:246-266`: the accept loop spawns `handle_connection` for every accepted stream with no count, permit, or gauge. `DaemonCaps` (`serve.rs:141-165`) carries only `idle`, `max_sessions`, `agent`, `endpoint`. - **No handshake deadlines.** `exchange_bootstrap` (`serve.rs:1718-1753`) awaits `read_client_bootstrap` with no timeout; `handshake` (`serve.rs:1913-1935`) awaits `reader.next_frame()` for `Hello` with no timeout; `wait_for_attach` (`serve.rs:424-427`) awaits the first operation with no timeout. `grep timeout` across `felis-daemon/src` and `felis-transport/src` finds only test code (`parse_sink.rs`, `lib.rs:428`, `framing.rs:355`). A silent same-UID peer holds a tokio task, a fd, a `FrameReader` (`READ_CHUNK` 8 KiB) and a `BufWriter` (`WRITE_BUF_CAPACITY`) forever. - **Per-resource ceilings exist but do not compose.** Sessions: `MAX_SESSIONS = 256` (`pool.rs:46`) enforced via `try_reserve` + RAII `SessionSlot` (`serve.rs:485-506`, `pool.rs:389-400`). Per session: grid 512 MiB budget (REQ-605a, `messages.rs:97`), image store 256 MiB (`pool.rs:38`), decode reassembly 64 MiB. Per subscriber: `SUBSCRIBER_BUFFER_CAP = 512 MiB` (`session_task.rs:46`), eviction not backpressure. Subscriber *count* per session is unbounded; connections are unbounded. So the daemon-wide product is `256 × ~832 MiB + connections × 512 MiB + pending PTY input (unbounded, #15)`. - **Status reports five rows, none for connections** (`serve.rs:928-1005`): sessions, image-store bytes, in-flight decodes, decode bytes, deepest subscriber queue. - **Typed capacity outcomes exist only for sessions**: `AttachFailure::SessionLimitReached` → CLI `at_capacity` (CHANGELOG Unreleased). `RefusalReason` has a single variant `Role` (`felis-protocol/src/messages/conn.rs:84-88`). One thing the issue understates: the recorded decision at `docs/reference/ipc.md:1802-1816` and `docs/explanation/architecture/session-lifecycle.md:1049-1058` is that subscriber outboxes are *unbounded with a gauge and evicted on volume, never on time*, because a bounded channel would apply one slow mirror's backpressure to the session task serving everyone. #14 must account for those bytes, not replace the eviction policy. ## Verdict **accept-with-changes.** The scope list ("one admission object" accounting for connection phase, sessions, PTY threads, grid/scrollback, images, decode, subscribers, queued bytes, pending input) over-reaches for what is missing. Grid, image, and decode budgets are already per-session constants whose aggregate is `max_sessions × constant`; re-accounting them through a second object adds a lock on the hot path for no new bound. What is actually unbounded is (a) connection count, (b) time spent pre-attach, and (c) pending PTY input (#15). Do these, and *document* the product of the fixed per-session constants as the daemon-wide budget rather than enforcing it twice. Principle check: principle 3 test ("changing a client config requires a daemon restart") is untouched; `control-surfaces.md:173-178` records that limits are compiled in and the daemon reads no config; keep it that way (no `--max-connections` flag; a `DaemonCaps` field for embedders/tests, as `max_sessions` already is). ## Approach **felis-daemon** - `DaemonCaps` gains `max_connections: usize` (compiled default `MAX_CONNECTIONS = 1024` in `pool.rs`, 4× sessions: every window plus a bridge, an observer, and a `felis sessions` call per session) and `handshake_deadlines: HandshakeDeadlines { preface: 2 s, hello: 5 s, first_op: 30 s }`. - Admission: `Arc<tokio::sync::Semaphore>` sized `max_connections`, held in the serve loop; `try_acquire_owned()` at `serve.rs:247` **before** `tokio::spawn`. The `OwnedSemaphorePermit` moves into the task and is dropped on every exit path (success, refusal, timeout, disconnect, panic, because tokio drops the future's captures on task failure). This is the "owned permits released on drop" the issue asks for; `SessionSlot` already has that shape for sessions. - Over-cap refusal: still run the preface (fixed 8-byte exchange, cheap) and read `Hello` under the deadline, then write `ConnMsg::Refused { reason: RefusalReason::AtCapacity, detail: "the daemon is at N of M connections" }` and close. Add `RefusalReason::AtCapacity` (proto enum tail value; a minor addition under #50's ledger, and swept into #30's 2.0 reset). Reading `Hello` for an over-cap peer is bounded by the deadline and by `MAX_READAHEAD` (`framing.rs:51`), so it costs no allocation beyond the read buffer. Rationale to record: closing the socket without a frame would make "daemon busy" indistinguishable from "daemon crashed" to `felis` and the bridge, which must surface `at_capacity` (retry) rather than `daemon` (exit 2). - Deadlines: wrap `read_client_bootstrap` (preface + carrier block), `handshake`'s first `next_frame`, and `wait_for_attach`'s *first* `next_frame` in `tokio::time::timeout`; a timeout is a new `ConnError::HandshakeTimeout { phase }` logged at `debug` (a peer that says nothing is not worth a `warn`). Only the first pre-attach frame is timed: a bridge (`Ops` mode) legitimately idles between verbs after its first operation, and a `Notify` observer idles for hours. - Status: add `ResourceKind::Connections` (count, limit `max_connections`, scope `Daemon`) to `daemon_status` (`serve.rs:965-1004`); `used` = `max_connections - semaphore.available_permits()`. - Subscriber count: with a connection cap, subscribers ≤ connections, so no separate per-session subscriber cap; document the bound `connections × SUBSCRIBER_BUFFER_CAP` as the queued-bytes ceiling instead of adding a second gauge. - Pending PTY input: owned by #15; #14's `DaemonCaps` should reserve the field name (`pty_input_budget`) so #15 slots in without a second config surface. **felis-client-core / felis-cli**: map `ConnMsg::Refused { AtCapacity }` during dial to `ConnectError::AtCapacity` → CLI/bridge `at_capacity` error kind (already exists for sessions, `skills/felis/SKILL.md:234`). **Tests** - `serve/tests.rs`: `DaemonCaps { max_connections: 2, .. }` via `handle_stream` with in-memory duplex streams: third dial gets `Refused(AtCapacity)`; after one disconnect the next dial succeeds (permit released); a handler that returns `Err` mid-handshake releases; `tokio::time::pause` + advance to prove each of the three deadlines fires and releases the permit; a peer that sends the preface and stops is cut at `hello`; a peer that completes `Hello` and stops is cut at `first_op`; a bridge idle *after* one op is not cut. - Stress: 16 sessions × N mirrors with `max_connections` small enough to refuse; assert `daemon status` rows `connections.used ≤ limit` throughout and equal to zero after teardown (proves every permit returned). **Docs cascade** - `docs/reference/spec.md`: new REQ-916 (connection cap + three deadlines + typed refusal); REQ-915 gains a sentence that the daemon-wide budget is `max_sessions × (grid + image + decode)` + `max_connections × SUBSCRIBER_BUFFER_CAP` + the #15 input budget; REQ-1102 row list gains connections. - `docs/reference/ipc.md`: "Handshake" section gets a deadline table; "Backpressure" gets a paragraph that the *pre-attach* phase is time-bounded while steady state is volume-bounded (keeps the recorded "cut on volume, never on time" for subscribers accurate by scoping it). - `docs/explanation/security-model.md` "Daemon IPC": bullets for the connection cap and silent-peer bound, with the rejected alternative (per-session subscriber cap) and "Revisit if a mirror count above `max_connections / max_sessions` appears". - `docs/explanation/architecture/session-lifecycle.md` (admission list at `:56-64`), `docs/explanation/architecture/control-surfaces.md` "Diagnostic verbs" (new row), `docs/reference/cli.md` "Daemon status". - `CHANGELOG.md` Unreleased: new limit (1024 connections, typed `at_capacity` on connect), handshake deadlines, new status row. - `skills/felis/SKILL.md` §"Is the daemon healthy?": new row; note that `at_capacity` can now arrive at connect time, not only at spawn. ## Dependencies - Independent of #13/#16/#49. Coordinate with **#15** (input budget field) and **#26** (status scope semantics: the connections row is `Daemon`-scoped, uncontroversial). The `RefusalReason::AtCapacity` and `ResourceKind::Connections` additions are wire edits that must precede **#30**'s 2.0 reset and appear in **#50**'s send-authorization ledger. #12's order (#14 before #15/#16/#49) still holds. ## Risk/effort **M.** Semaphore + three timeouts + one enum value is S; the tests and the six-document cascade are where the time goes. Main risk: a deadline too tight for the SSH relay path. The relay (`felis-daemon relay`) dials the *remote* daemon locally after sshd has already authenticated, so the preface deadline starts only once the local connect lands; keep `preface` at 2 s regardless, but verify with the relay integration test before choosing numbers. ## Labels Keep `priority/P0`, `release/v0.1.0`. An unbounded, un-timed pre-attach phase reachable by any same-UID process is a daemon-availability hole the security model (`security-model.md:269`, "second-largest attack surface") already claims to close. ## Review amendments (round 1) - **Over-cap peers must not get an unbounded task.** Acquire the connection permit (`try_acquire_owned`) in the accept loop *before* `tokio::spawn`. When no permit is available, the refusal path is itself bounded by a second, small semaphore (`MAX_REFUSALS_IN_FLIGHT`, e.g. 16): with a refusal permit, spawn a task that runs only the fixed preface exchange and reads `Hello` under the handshake deadline, then writes `Refused { AtCapacity }` and closes; without one, drop the socket in the accept loop without writing anything. Silent peers can therefore hold at most `max_connections + MAX_REFUSALS_IN_FLIGHT` tasks and fds. The `connections` status row counts admitted permits only. Test: with `max_connections = 1` and `MAX_REFUSALS_IN_FLIGHT = 1`, two silent extra dials leave exactly one refusal task alive and the third is closed immediately.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
ci/darwin-smoke
No results found.
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
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#14
No description provided.