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

[Post-v0.1/P2] Preserve sessions across daemon replacement #32

New issue
Open
opened 2026-09-03 16:18:10 +09:00 by natsukium · 1 comment
natsukium commented 2026-09-03 16:18:10 +09:00
Owner
Copy link

Follow-up from #12 (P2); not a v0.1.0 blocker.

Why

Because the daemon owns PTY masters, restarting it loses sessions. Solving that requires socket handoff, a drain broker, or per-session workers; weakening daemon ownership would discard the product's distinguishing lifecycle model.

Scope

  • Gather an observed workflow that requires zero-loss daemon replacement.
  • Compare handoff/broker/worker designs without moving PTY ownership into the client.
  • Define authentication, rollback, partial-transfer, and version-skew behavior before implementation.

Acceptance criteria

  • A principle check and design record justify the chosen architecture.
  • Sessions and PTY state survive a tested daemon binary replacement.
  • Failed upgrades preserve or explicitly terminate the old daemon without split ownership.
  • Until this lands, user docs continue to state that sessions survive client/window loss, not daemon loss.
Follow-up from #12 (P2); not a v0.1.0 blocker. ## Why Because the daemon owns PTY masters, restarting it loses sessions. Solving that requires socket handoff, a drain broker, or per-session workers; weakening daemon ownership would discard the product's distinguishing lifecycle model. ## Scope - Gather an observed workflow that requires zero-loss daemon replacement. - Compare handoff/broker/worker designs without moving PTY ownership into the client. - Define authentication, rollback, partial-transfer, and version-skew behavior before implementation. ## Acceptance criteria - [ ] A principle check and design record justify the chosen architecture. - [ ] Sessions and PTY state survive a tested daemon binary replacement. - [ ] Failed upgrades preserve or explicitly terminate the old daemon without split ownership. - [ ] Until this lands, user docs continue to state that sessions survive client/window loss, not daemon loss.
natsukium commented 2026-09-03 20:35:16 +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, but the issue under-states how much is already decided and built.

  • The daemon owns the PTY masters and all session state in RAM: crates/felis-daemon/src/pool.rs:86 builds each session's Grid::new(rows, cols) inside the pool, and docs/explanation/architecture/session-lifecycle.md:1091-1095 ("Daemon updates") states that a stop closes the PTY masters and every RAM-only state drops. docs/how-to/update-felis.md ("Drain your sessions", "Restart the daemon onto it") is the user-facing procedure: pkill felis-daemon.
  • The product claim the issue wants preserved is already what the docs say: docs/tutorials/first-session.md:132 ("sessions outlive their windows"), README.md:208, docs/tutorials/drive-without-a-window.md:11. No doc claims daemon-loss survival.
  • What the issue misses: a working session-preserving in-place upgrade existed and was deliberately removed in e0994073 (2026-08-28, "feat!: drop the in-place daemon upgrade from the initial release"). It removed crates/felis-daemon/src/upgrade/{dump,fd,reexec,restore,strand}.rs (≈1600 lines), the felis-daemon upgrade verb, ConnMsg::UpgradeDaemon/UpgradeStarting, the quiesce/thaw machinery, the transport fd-inheritance hooks, and felis-protocol's postcard feature. CHANGELOG.md:900-908 records the user-facing removal.
  • The design record the issue asks for already exists: session-lifecycle.md:1109-1137 records drain-as-manual-procedure (with the create-vs-stop race documented, not closed), "old-daemon-serves-until-empty (socket handoff or broker) is explicitly deferred: it needs a cross-platform handoff design v1 does not carry", and the re-exec rejection with its rationale (a private versioned dump schema is a second compatibility-bearing serialization surface; fd-inheritance hygiene; impossible on Windows) and a Revisit if clause: "long-lived sessions become precious enough in practice that losing them on update is a reported pain". The commit message adds the compatibility framing that matters for this issue: the upgrade "can return later as a purely additive feature, since it only ever matters between adjacent versions that both ship it."
  • Side finding (not this issue, but found while verifying it): CLAUDE.md "Workspace policy" still lists felis-daemon's upgrade::fd funnels among the audited #[allow(unsafe_code)] sites. That module no longer exists (e5b5c630 wrote the sentence on 2026-08-21; e0994073 removed the module a week later). grep -rn upgrade crates/felis-daemon/src finds only Weak::upgrade calls. Worth a docs:/chore: one-liner.

Verdict

defer (keep post-v0.1.0, P2) — with one scope correction: the issue's "compare handoff/broker/worker designs" step must start from the removed re-exec implementation at e0994073^ and the recorded rejection, not from a blank page. The principle question is already settled: none of the three designs moves PTY ownership out of the daemon, so principle 3 ("the daemon owns state") is not in play; the open question is upkeep cost versus demand, which is exactly what the Revisit clause names.

Deferral does not get more expensive because of the v0.1.0 freeze:

  • The freeze covers the preface, protobuf schema, row codec, and evolution rules (#12 "Contract freeze boundary"). Every design here needs at most one additive wire addition (a correlated Ops request to trigger the upgrade, or nothing at all for a socket handoff). The evolution rules being frozen by #30/#47 allow exactly that (an additive minor may add oneof variants; docs/reference/ipc.md:202-205).
  • The dump schema is by construction not part of the public contract: it only has to be readable by the adjacent version, and the removed design already had a decode-or-cold-start policy (session-lifecycle.md:1135-1137). So freezing the wire does not freeze anything the upgrade would later depend on.
  • The one thing to watch at freeze time is that #30's renumbering does not reserve the field numbers the old UpgradeDaemon/UpgradeStarting arms used in a way that blocks re-adding them — it should not, since new variants get new numbers, but the e0994073 message notes ConnMsg was repacked dense, so a future re-add is a new tag, never a revival of the old one.

What does change after the tag is the observability of demand: the Revisit trigger ("reported pain") can only fire once there are users on a tagged release. That is an argument for deferral, not against.

Approach

Eventual approach, when the Revisit clause fires:

  1. Restore the re-exec model as an additive feature, not a handoff/broker. Reason: the removed design was working (e0994073 message: "worked"), needs no second process or proxy in the socket path, and the cross-platform objection ("Windows cannot have it at all") is answered by making it a Unix-only operation that replies Unsupported elsewhere — the same shape felis-pty's ConPTY backend already takes for Unix-only behavior. A broker in front of two daemons would put a proxy in the preface path and is the design the docs already defer for lacking a cross-platform story; per-session workers would multiply the process model for every session, upgrade or not.
  2. Wire: one correlated OpsMsg::UpgradeDaemon { successor_path } → reply with Accepted/Refused { reason } in the #20/#47/#48 shapes (correlated request, exact per-arm identity). No pre-handshake arm this time: the removed pair was the only cross-generation path and its removal is what let every frame sit behind the generation gate (e0994073). Additive minor.
  3. Daemon (crates/felis-daemon): quiesce each session (stop the PTY drain, flush outboxes), dump structured state with a version tag bound to the build id (#17's build identity), pass the listen fd and PTY masters to the successor via the env-carried fd:pid manifest, execve; successor decodes-or-cold-starts. This is the recorded design (session-lifecycle.md:1135-1137), re-audited against the #[allow(unsafe_code)] policy in clippy.toml/CLAUDE.md (the upgrade::fd funnels come back as an audited site — and the stale CLAUDE.md sentence becomes true again).
  4. Failure behavior (acceptance "failed upgrades preserve or explicitly terminate the old daemon"): a successor that cannot decode the dump cold-starts and SIGKILLs the stranded children via the manifest (the removed strand.rs), so ownership is never split; a successor that fails to exec leaves the old daemon thawing its sessions. Both are integration-tested.
  5. CLI: felis daemon upgrade [--binary PATH] as a Point verb (--format json → {"v":1, "from": build, "to": build, "sessions": N}), classified in docs/reference/cli.md "Machine output" per #35.
  6. Docs cascade: session-lifecycle.md "Daemon updates" (flip the rejection into a decision record with the failure modes above), docs/how-to/update-felis.md (new primary path; pkill stays as the fallback and the Windows path), docs/reference/ipc.md (new Ops arm), docs/reference/cli.md, docs/reference/spec.md (a REQ for "sessions survive an in-place upgrade between adjacent versions"), CHANGELOG.md, skills/felis (the upgrade verb), the tutorial claim (first-session.md:132) may widen to "sessions outlive windows and daemon upgrades".
  7. Tests: felis-daemon integration test: serve → spawn session with scrollback → UpgradeDaemon to the same binary → reconnect → assert session id, grid, scrollback, image store intact; a corrupted-dump test asserting cold start + explicit child termination; a Windows build asserting the Unsupported reply. Reuse the harness style of crates/felis-daemon/tests/real_app_harness.rs (which e0994073 touched, so it once hosted this).

Dependencies

  • #25 (portable daemon stop/drain operations, v0.1.0 P1) — the quiesce/refuse-new semantics an upgrade needs are the same ones a drain op defines; land #25 first and build the upgrade's quiesce on it.
  • #22 (reconnect the same session after transport loss, v0.1.0 P1) — gives the client half for free: a window that re-dials and re-attaches by session id after the socket blips is exactly what an exec-over-the-same-fd upgrade needs on the client side.
  • #17 (build identity) — the dump version tag should be the build id, not a hand-maintained number.
  • #20/#47/#48 — fix the correlated-Ops shape the new arm follows.
  • Recommended order in #12 still holds; this issue sits after #31.

Risk/effort

L. Main risk is the one the removal named: a private serialization of grid/image-store/parser state that every later change to those types must keep adjacent-readable, or knowingly break. Mitigation: decode-or-cold-start (never refuse the upgrade, only downgrade it to "lose sessions, with an explicit log line"), plus a CI test that upgrades HEAD to HEAD.

Labels

Keep priority/P2, post-v0.1.0. Not a closure dependency for #12. Suggest editing the issue body to cite e0994073 and session-lifecycle.md "Daemon updates" as the starting point, and to drop the "gather an observed workflow" bullet in favor of the doc's existing Revisit trigger (reported pain from users on a tagged release), so the issue is actionable the day that trigger fires.

Review amendments (round 1)

  • Cross-platform non-goal. docs/explanation/non-goals.md "Cross-platform constraints" rejects features gated to one OS. A Unix-only daemon upgrade returning Unsupported on Windows is such a gate. Before implementation, the design must either be uniform (a handoff that does not rely on exec/fd passing, e.g. a socket-level state transfer between old and new daemon processes) or record a decision lifting the non-goal with its rationale. This does not affect v0.1.0 (the issue stays deferred).
## 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, but the issue under-states how much is already decided and built. - The daemon owns the PTY masters and all session state in RAM: `crates/felis-daemon/src/pool.rs:86` builds each session's `Grid::new(rows, cols)` inside the pool, and `docs/explanation/architecture/session-lifecycle.md:1091-1095` ("Daemon updates") states that a stop closes the PTY masters and every RAM-only state drops. `docs/how-to/update-felis.md` ("Drain your sessions", "Restart the daemon onto it") is the user-facing procedure: `pkill felis-daemon`. - The product claim the issue wants preserved is already what the docs say: `docs/tutorials/first-session.md:132` ("sessions outlive their windows"), `README.md:208`, `docs/tutorials/drive-without-a-window.md:11`. No doc claims daemon-loss survival. - What the issue misses: a working session-preserving in-place upgrade **existed and was deliberately removed** in `e0994073` (2026-08-28, "feat!: drop the in-place daemon upgrade from the initial release"). It removed `crates/felis-daemon/src/upgrade/{dump,fd,reexec,restore,strand}.rs` (≈1600 lines), the `felis-daemon upgrade` verb, `ConnMsg::UpgradeDaemon`/`UpgradeStarting`, the quiesce/thaw machinery, the transport fd-inheritance hooks, and felis-protocol's postcard feature. `CHANGELOG.md:900-908` records the user-facing removal. - The design record the issue asks for already exists: `session-lifecycle.md:1109-1137` records drain-as-manual-procedure (with the create-vs-stop race documented, not closed), "old-daemon-serves-until-empty (socket handoff or broker) is explicitly deferred: it needs a cross-platform handoff design v1 does not carry", and the re-exec rejection with its rationale (a private versioned dump schema is a second compatibility-bearing serialization surface; fd-inheritance hygiene; impossible on Windows) and a **Revisit if** clause: "long-lived sessions become precious enough in practice that losing them on update is a reported pain". The commit message adds the compatibility framing that matters for this issue: the upgrade "can return later as a purely additive feature, since it only ever matters between adjacent versions that both ship it." - Side finding (not this issue, but found while verifying it): `CLAUDE.md` "Workspace policy" still lists `felis-daemon`'s `upgrade::fd` funnels among the audited `#[allow(unsafe_code)]` sites. That module no longer exists (`e5b5c630` wrote the sentence on 2026-08-21; `e0994073` removed the module a week later). `grep -rn upgrade crates/felis-daemon/src` finds only `Weak::upgrade` calls. Worth a `docs:`/`chore:` one-liner. ## Verdict **defer** (keep post-v0.1.0, P2) — with one scope correction: the issue's "compare handoff/broker/worker designs" step must start from the removed re-exec implementation at `e0994073^` and the recorded rejection, not from a blank page. The principle question is already settled: none of the three designs moves PTY ownership out of the daemon, so principle 3 ("the daemon owns state") is not in play; the open question is upkeep cost versus demand, which is exactly what the Revisit clause names. Deferral does not get more expensive because of the v0.1.0 freeze: - The freeze covers the preface, protobuf schema, row codec, and evolution rules (#12 "Contract freeze boundary"). Every design here needs at most one **additive** wire addition (a correlated `Ops` request to trigger the upgrade, or nothing at all for a socket handoff). The evolution rules being frozen by #30/#47 allow exactly that (an additive minor may add oneof variants; `docs/reference/ipc.md:202-205`). - The dump schema is by construction *not* part of the public contract: it only has to be readable by the adjacent version, and the removed design already had a decode-or-cold-start policy (`session-lifecycle.md:1135-1137`). So freezing the wire does not freeze anything the upgrade would later depend on. - The one thing to watch at freeze time is that #30's renumbering does not *reserve* the field numbers the old `UpgradeDaemon`/`UpgradeStarting` arms used in a way that blocks re-adding them — it should not, since new variants get new numbers, but the `e0994073` message notes `ConnMsg` was repacked dense, so a future re-add is a new tag, never a revival of the old one. What *does* change after the tag is the observability of demand: the Revisit trigger ("reported pain") can only fire once there are users on a tagged release. That is an argument for deferral, not against. ## Approach Eventual approach, when the Revisit clause fires: 1. **Restore the re-exec model as an additive feature**, not a handoff/broker. Reason: the removed design was working (`e0994073` message: "worked"), needs no second process or proxy in the socket path, and the cross-platform objection ("Windows cannot have it at all") is answered by making it a Unix-only operation that replies `Unsupported` elsewhere — the same shape `felis-pty`'s ConPTY backend already takes for Unix-only behavior. A broker in front of two daemons would put a proxy in the preface path and is the design the docs already defer for lacking a cross-platform story; per-session workers would multiply the process model for every session, upgrade or not. 2. **Wire**: one correlated `OpsMsg::UpgradeDaemon { successor_path }` → reply with `Accepted`/`Refused { reason }` in the #20/#47/#48 shapes (correlated request, exact per-arm identity). No pre-handshake arm this time: the removed pair was the only cross-generation path and its removal is what let every frame sit behind the generation gate (`e0994073`). Additive minor. 3. **Daemon** (`crates/felis-daemon`): quiesce each session (stop the PTY drain, flush outboxes), dump structured state with a version tag bound to the build id (`#17`'s build identity), pass the listen fd and PTY masters to the successor via the env-carried `fd:pid` manifest, `execve`; successor decodes-or-cold-starts. This is the recorded design (`session-lifecycle.md:1135-1137`), re-audited against the `#[allow(unsafe_code)]` policy in `clippy.toml`/`CLAUDE.md` (the `upgrade::fd` funnels come back as an audited site — and the stale CLAUDE.md sentence becomes true again). 4. **Failure behavior** (acceptance "failed upgrades preserve or explicitly terminate the old daemon"): a successor that cannot decode the dump cold-starts and `SIGKILL`s the stranded children via the manifest (the removed `strand.rs`), so ownership is never split; a successor that fails to exec leaves the old daemon thawing its sessions. Both are integration-tested. 5. **CLI**: `felis daemon upgrade [--binary PATH]` as a Point verb (`--format json` → `{"v":1, "from": build, "to": build, "sessions": N}`), classified in `docs/reference/cli.md` "Machine output" per #35. 6. **Docs cascade**: `session-lifecycle.md` "Daemon updates" (flip the rejection into a decision record with the failure modes above), `docs/how-to/update-felis.md` (new primary path; `pkill` stays as the fallback and the Windows path), `docs/reference/ipc.md` (new Ops arm), `docs/reference/cli.md`, `docs/reference/spec.md` (a REQ for "sessions survive an in-place upgrade between adjacent versions"), `CHANGELOG.md`, `skills/felis` (the upgrade verb), the tutorial claim (`first-session.md:132`) may widen to "sessions outlive windows and daemon upgrades". 7. **Tests**: felis-daemon integration test: serve → spawn session with scrollback → `UpgradeDaemon` to the *same* binary → reconnect → assert session id, grid, scrollback, image store intact; a corrupted-dump test asserting cold start + explicit child termination; a Windows build asserting the `Unsupported` reply. Reuse the harness style of `crates/felis-daemon/tests/real_app_harness.rs` (which `e0994073` touched, so it once hosted this). ## Dependencies - **#25** (portable daemon stop/drain operations, v0.1.0 P1) — the quiesce/refuse-new semantics an upgrade needs are the same ones a drain op defines; land #25 first and build the upgrade's quiesce on it. - **#22** (reconnect the same session after transport loss, v0.1.0 P1) — gives the client half for free: a window that re-dials and re-attaches by session id after the socket blips is exactly what an exec-over-the-same-fd upgrade needs on the client side. - **#17** (build identity) — the dump version tag should be the build id, not a hand-maintained number. - **#20/#47/#48** — fix the correlated-Ops shape the new arm follows. - Recommended order in #12 still holds; this issue sits after #31. ## Risk/effort **L.** Main risk is the one the removal named: a private serialization of grid/image-store/parser state that every later change to those types must keep adjacent-readable, or knowingly break. Mitigation: decode-or-cold-start (never refuse the upgrade, only downgrade it to "lose sessions, with an explicit log line"), plus a CI test that upgrades HEAD to HEAD. ## Labels Keep `priority/P2`, post-v0.1.0. Not a closure dependency for #12. Suggest editing the issue body to cite `e0994073` and `session-lifecycle.md` "Daemon updates" as the starting point, and to drop the "gather an observed workflow" bullet in favor of the doc's existing Revisit trigger (reported pain from users on a tagged release), so the issue is actionable the day that trigger fires. ## Review amendments (round 1) - **Cross-platform non-goal.** `docs/explanation/non-goals.md` "Cross-platform constraints" rejects features gated to one OS. A Unix-only `daemon upgrade` returning `Unsupported` on Windows is such a gate. Before implementation, the design must either be uniform (a handoff that does not rely on `exec`/fd passing, e.g. a socket-level state transfer between old and new daemon processes) or record a decision lifting the non-goal with its rationale. This does not affect v0.1.0 (the issue stays deferred).
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#32
No description provided.