fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

CB-584: session resume is built end-to-end but connected at neither end — persist agentSessionId and expose it on bridge_spawn #65

New Issue
Closed
opened 2026-08-15 13:03:50 +02:00 by ltms · 2 comments
ltms commented 2026-08-15 13:03:50 +02:00
Owner
Copy Link
Copy Source

The gap

Everything needed to resume a member's conversation already exists, except the two links that would make it reachable.

Piece State
SpawnRequest.sessionName and SpawnRequest.resumeSessionId exists
ClaudeCodeLauncher passes --session-id, so the id is known before the peer writes anything exists
OpenCodeLauncher.SessionAwareHandle.agentSessionId() resolves it via session discovery exists
PeerHandle.agentSessionId() exists, but see the defect below
Capability.SESSION_RESUME declared
MemberSession records it no
bridge_spawn accepts a session name or a resume id no

MemberSession carries paneId, terminalId, profile, role, cwd, ownerTerminal, timestamps, turnCount, state, worktree, branch, charterReceipt — and not agentSessionId. So the moment a session is released, the id the adapter already knew is discarded. Nothing can ever resume it.

And bridge_spawn exposes neither sessionName nor resumeSessionId, so even a stored id would be unreachable from the only surface a lead can call.

Why it matters, and how it pairs with CB-578 stage C

Stage C (in flight) snapshots a dirty worktree into refs/wip/<branch> so a member's files survive its death. That is the right fix for the files, and it has a stated limit: the member's reasoning is gone. A re-dispatch is a fresh member re-reading a brief, not a continuation.

This ticket is the other half. If the agent session id survives with the worktree, a replacement member can resume the actual conversation rather than start cold. Issue #50 already flagged this as worth its own ticket:

codex resume --last exists and may give opencode-backed members a real context resume rather than a re-brief. Untested. Out of scope here, worth its own ticket.

The two together turn "the work is lost" into "the work and the thread both survive".

Defect to fix on the way — this is the tenth instance

PeerHandle.agentSessionId() is a default method returning null:

default String agentSessionId() {
    return null;
}

This is the exact defect fixed in CB-571 today, one method below it in the same file. There, charterReceipt() was a default returning null, OpenCodeLauncher's wrapping handle forgot to override it, and the feature was silently half-present — sol and terra showed no receipt while Claude Code members did, with nothing failing.

agentSessionId() is currently overridden by both implementations, so it is not broken today. But it is the same landmine: a new adapter that forgets it answers null, and the only symptom is that resume quietly stops working for that backend. Delete the default and let the compiler ask the question, as CB-571 did.

This repo has now shipped a feature silently switched off by a defaulted dependency nine times. This would be the tenth if left.

Scope

  1. Record agentSessionId on MemberSession, populated from PeerHandle at acquire.
  2. Delete the default from PeerHandle.agentSessionId().
  3. Expose it on the roster (bridge_list, GET /members) so a lead can see which conversation a member holds.
  4. Accept sessionName and resumeSessionId on bridge_spawn, threading them into the existing SpawnRequest fields.
  5. Carry the session id alongside the worktree, branch and snapshot ref in a failed ticket's detail (CB-578 stage C criterion 10 adds the other three — extend rather than duplicate).
  6. Refuse a resumeSessionId for a profile whose adapter does not declare Capability.SESSION_RESUME, with a message saying so. Silently ignoring it would look like a resume and deliver a cold session — worse than refusing.

Acceptance criteria

  1. A spawned member's agentSessionId is recorded on its MemberSession and visible in the roster.
  2. bridge_spawn{resumeSessionId} starts a member on that prior conversation, for an adapter that declares SESSION_RESUME.
  3. bridge_spawn{resumeSessionId} on an adapter that does not declare it is refused with a reason naming the capability — never silently ignored.
  4. PeerHandle.agentSessionId() has no default; every implementation answers explicitly.
  5. A failed ticket carries the session id along with worktree, branch and snapshot ref.
  6. A spawn that passes neither field behaves exactly as today.

Not in scope

Whether a resumed session is useful after a usage-limit refusal is untested. codex resume --last and claude --resume may or may not restore something worth having. Prove it with one live spawn before building policy on top of it.

## The gap Everything needed to resume a member's *conversation* already exists, except the two links that would make it reachable. | Piece | State | |---|---| | `SpawnRequest.sessionName` and `SpawnRequest.resumeSessionId` | exists | | `ClaudeCodeLauncher` passes `--session-id`, so the id is known **before** the peer writes anything | exists | | `OpenCodeLauncher.SessionAwareHandle.agentSessionId()` resolves it via session discovery | exists | | `PeerHandle.agentSessionId()` | exists, but see the defect below | | `Capability.SESSION_RESUME` | declared | | **`MemberSession` records it** | **no** | | **`bridge_spawn` accepts a session name or a resume id** | **no** | `MemberSession` carries `paneId`, `terminalId`, `profile`, `role`, `cwd`, `ownerTerminal`, timestamps, `turnCount`, `state`, `worktree`, `branch`, `charterReceipt` — and **not** `agentSessionId`. So the moment a session is released, the id the adapter already knew is discarded. Nothing can ever resume it. And `bridge_spawn` exposes neither `sessionName` nor `resumeSessionId`, so even a stored id would be unreachable from the only surface a lead can call. ## Why it matters, and how it pairs with CB-578 stage C Stage C (in flight) snapshots a dirty worktree into `refs/wip/<branch>` so a member's **files** survive its death. That is the right fix for the files, and it has a stated limit: the member's **reasoning is gone**. A re-dispatch is a fresh member re-reading a brief, not a continuation. This ticket is the other half. If the agent session id survives with the worktree, a replacement member can resume the actual conversation rather than start cold. Issue #50 already flagged this as worth its own ticket: > `codex resume --last` exists and may give opencode-backed members a real context resume rather than a re-brief. Untested. Out of scope here, worth its own ticket. The two together turn "the work is lost" into "the work and the thread both survive". ## Defect to fix on the way — this is the tenth instance `PeerHandle.agentSessionId()` is a `default` method returning `null`: ```java default String agentSessionId() { return null; } ``` This is the **exact** defect fixed in CB-571 today, one method below it in the same file. There, `charterReceipt()` was a `default` returning `null`, `OpenCodeLauncher`'s wrapping handle forgot to override it, and the feature was silently half-present — `sol` and `terra` showed no receipt while Claude Code members did, with nothing failing. `agentSessionId()` is currently overridden by both implementations, so it is not broken *today*. But it is the same landmine: a new adapter that forgets it answers `null`, and the only symptom is that resume quietly stops working for that backend. Delete the `default` and let the compiler ask the question, as CB-571 did. This repo has now shipped a feature silently switched off by a defaulted dependency **nine** times. This would be the tenth if left. ## Scope 1. Record `agentSessionId` on `MemberSession`, populated from `PeerHandle` at acquire. 2. Delete the `default` from `PeerHandle.agentSessionId()`. 3. Expose it on the roster (`bridge_list`, `GET /members`) so a lead can see which conversation a member holds. 4. Accept `sessionName` and `resumeSessionId` on `bridge_spawn`, threading them into the existing `SpawnRequest` fields. 5. Carry the session id alongside the worktree, branch and snapshot ref in a failed ticket's detail (CB-578 stage C criterion 10 adds the other three — extend rather than duplicate). 6. Refuse a `resumeSessionId` for a profile whose adapter does not declare `Capability.SESSION_RESUME`, with a message saying so. Silently ignoring it would look like a resume and deliver a cold session — worse than refusing. ## Acceptance criteria 1. A spawned member's `agentSessionId` is recorded on its `MemberSession` and visible in the roster. 2. `bridge_spawn{resumeSessionId}` starts a member on that prior conversation, for an adapter that declares `SESSION_RESUME`. 3. `bridge_spawn{resumeSessionId}` on an adapter that does not declare it is refused with a reason naming the capability — never silently ignored. 4. `PeerHandle.agentSessionId()` has no `default`; every implementation answers explicitly. 5. A failed ticket carries the session id along with worktree, branch and snapshot ref. 6. A spawn that passes neither field behaves exactly as today. ## Not in scope Whether a resumed session is *useful* after a usage-limit refusal is untested. `codex resume --last` and `claude --resume` may or may not restore something worth having. Prove it with one live spawn before building policy on top of it.
ltms added the ready-to-delegatesilent-default labels 2026-08-15 13:10:17 +02:00
ltms commented 2026-08-15 16:01:48 +02:00
Author
Owner
Copy Link
Copy Source

Merged to main at e01563a. My own build on the merged tree: 778 tests, 0 failures, BUILD SUCCESS, exit 0 (unpiped). CI green on the PR head (run 1196).

Criteria 1-4 and 6 are met. Criterion 5 (carrying the session id on a failed ticket's ReleaseDetail) was deliberately left out of the brief, because SessionManager.release()/ReleaseDetail had just been changed by CB-578 stage C and I would not put two workers in the same method. It remains open — I am tracking it here rather than closing the issue as if it were done.

Two things worth recording.

PeerLauncher.capabilitiesFor(profileName) is new interface surface, and it is correct. The reasoning holds: the existing capabilities() returns the union across every configured adapter, so on a mixed fleet — Claude Code plus opencode — it would happily approve a resume that then lands on an adapter which cannot do one. A per-profile answer is the only honest check. Going outside the brief's file list for this was the right call, and it was flagged rather than slipped in.

Importantly, capabilitiesFor is a required interface method, not a default. Given this ticket was partly about deleting a default that silently returned null, adding a new one would have been the tenth instance of the same trap in the same change. It was not.

Requiring an explicit profile alongside resumeSessionId is the right refusal. A resumed conversation belongs to the backend that minted its id, and an unqualified spawn is routed by placement at spawn time, so there is no adapter to check the capability against in advance. Refusing beats approximating.

The honesty note is accepted and matches the ticket's own "not in scope": this is plumbing, no live resume was run, and whether a resumed session is useful after a usage-limit refusal is still unproven. That proof needs one live spawn and should happen before anything is built on top of it.

Merged to `main` at `e01563a`. My own build on the merged tree: **778 tests, 0 failures, BUILD SUCCESS, exit 0** (unpiped). CI green on the PR head (run 1196). Criteria 1-4 and 6 are met. Criterion 5 (carrying the session id on a failed ticket's `ReleaseDetail`) was deliberately left out of the brief, because `SessionManager.release()`/`ReleaseDetail` had just been changed by CB-578 stage C and I would not put two workers in the same method. **It remains open** — I am tracking it here rather than closing the issue as if it were done. Two things worth recording. **`PeerLauncher.capabilitiesFor(profileName)` is new interface surface, and it is correct.** The reasoning holds: the existing `capabilities()` returns the union across every configured adapter, so on a mixed fleet — Claude Code plus opencode — it would happily approve a resume that then lands on an adapter which cannot do one. A per-profile answer is the only honest check. Going outside the brief's file list for this was the right call, and it was flagged rather than slipped in. Importantly, `capabilitiesFor` is a **required** interface method, not a `default`. Given this ticket was partly about deleting a `default` that silently returned `null`, adding a new one would have been the tenth instance of the same trap in the same change. It was not. **Requiring an explicit `profile` alongside `resumeSessionId` is the right refusal.** A resumed conversation belongs to the backend that minted its id, and an unqualified spawn is routed by placement at spawn time, so there is no adapter to check the capability against in advance. Refusing beats approximating. The honesty note is accepted and matches the ticket's own "not in scope": this is plumbing, no live resume was run, and whether a resumed session is *useful* after a usage-limit refusal is still unproven. That proof needs one live spawn and should happen before anything is built on top of it.
ltms added this to the 1.1 — single-host close-out milestone 2026-08-16 16:49:37 +02:00
ltms closed this issue 2026-08-16 18:30:13 +02:00
ltms commented 2026-08-16 18:30:34 +02:00
Author
Owner
Copy Link
Copy Source

Merged as 27bbd11 (PR #101). Closing.

The worker corrected my brief, and it was right to. I briefed all six scope items from this issue's text. Items 1–4 and 6 were already shipped in 5d5b3bd, which explicitly left item 5 for follow-up. So the issue description had gone stale and my brief carried the staleness forward. The worker checked the code instead of taking the brief at face value, did only item 5 plus the doc row, and said clearly where it diverged and why.

I verified that scope-down myself rather than accepting it:

  • MemberSession already has agentSessionId (line 49).
  • PeerHandle.agentSessionId() is already abstract — no default, so the compiler asks every adapter.
  • BridgeMcp already accepts sessionName and resumeSessionId and threads both into SpawnRequest.

So the remaining work really was item 5, and it is now done: ReleaseDetail carries agentSessionId, populated at the single construction site in release(), and the abandon reason appends it when present. Widening the record rather than adding an optional accessor is the right shape — the compiler now forces every construction site, which is the same lesson this issue was written around.

Verified here: trial merge onto main builds Tests run: 830, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS, unpiped. The new test asserts the id actually arrives on ReleaseDetail rather than merely that the field compiles.

Two follow-ups I handled, since a worker cannot:

  1. The CLAUDE.md edit touches the intent→tool table, which lives inside the canonical block. That block must stay byte-identical with the wiki template, and it had drifted. Re-synced; the check now prints in sync: True.
  2. Added the Features entry — what it is, that it needs no knob, why it exists (stage C saves the files, this saves the thread), and that a missing agentSessionId= is the normal case rather than a fault.

On the tenth silently-defaulted dependency: the worker looked and found none, and drew the right distinction — sessionName(), terminalId() and profile() are still default null on PeerHandle, but those are legitimately optional, unlike agentSessionId() and charterReceipt() where every implementation must answer. That is the correct test to apply, so I am satisfied nothing was missed.

Merged as 27bbd11 (PR #101). Closing. **The worker corrected my brief, and it was right to.** I briefed all six scope items from this issue's text. Items 1–4 and 6 were already shipped in `5d5b3bd`, which explicitly left item 5 for follow-up. So the issue description had gone stale and my brief carried the staleness forward. The worker checked the code instead of taking the brief at face value, did only item 5 plus the doc row, and said clearly where it diverged and why. I verified that scope-down myself rather than accepting it: - `MemberSession` already has `agentSessionId` (line 49). - `PeerHandle.agentSessionId()` is already abstract — no `default`, so the compiler asks every adapter. - `BridgeMcp` already accepts `sessionName` and `resumeSessionId` and threads both into `SpawnRequest`. So the remaining work really was item 5, and it is now done: `ReleaseDetail` carries `agentSessionId`, populated at the single construction site in `release()`, and the abandon reason appends it when present. Widening the record rather than adding an optional accessor is the right shape — the compiler now forces every construction site, which is the same lesson this issue was written around. **Verified here:** trial merge onto `main` builds `Tests run: 830, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`, unpiped. The new test asserts the id actually arrives on `ReleaseDetail` rather than merely that the field compiles. **Two follow-ups I handled, since a worker cannot:** 1. The `CLAUDE.md` edit touches the intent→tool table, which lives inside the canonical block. That block must stay byte-identical with the wiki template, and it had drifted. Re-synced; the check now prints `in sync: True`. 2. Added the Features entry — what it is, that it needs no knob, why it exists (stage C saves the files, this saves the thread), and that a missing `agentSessionId=` is the normal case rather than a fault. **On the tenth silently-defaulted dependency:** the worker looked and found none, and drew the right distinction — `sessionName()`, `terminalId()` and `profile()` are still `default null` on `PeerHandle`, but those are legitimately optional, unlike `agentSessionId()` and `charterReceipt()` where every implementation must answer. That is the correct test to apply, so I am satisfied nothing was missed.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label ready-to-delegate silent-default
Milestone
No items
No Milestone 1.1 — single-host close-out
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#65
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?