fix(server): the turn after a delete lands in the chat it was sent to (#730)

[edspencer]

Fixes #730.

The actual root cause — not the one in the issue

The issue's original guess (deleteSession clears the agent's current-session slot) was already retracted in a correction comment, and the correction's lead — the directory-wide discovery-cache invalidation — turns out to be a red herring too. Here is what is really happening, confirmed by reproducing it at the integration tier and reading herdctl's own log line:

Session for keeper-<slug> is invalid: CLI session file not found for session B. Clearing stale session.
No valid session for keeper-<slug> (expired or not found), starting fresh

Note B — the deleted chat — on a turn that explicitly asked to resume A.

herdctl keeps one agent-level session pointer per agent at <stateDir>/sessions/<agent>.json, rewritten after every batch turn, so it always names that project's most-recently-active chat. It exists for exactly one purpose: resume that session when a caller asks for a turn without naming one. Paddock's chat sends always name one, so Paddock never reads it.

It still reaches us, through how JobExecutor reacts to it going stale (dist/runner/job-executor.js:131-289):

const hadAgentSession = (await getSessionInfo(sessionsDir, agent.qualifiedName)) !== null;
const existingSession  = await getSessionInfo(sessionsDir, agent.qualifiedName, { timeout, runtime, ... });
// …
let adopt = !hadAgentSession && (currentRuntimeType !== "cli" || dockerEnabled);

Deleting B leaves the pointer naming a file that no longer exists. On the next turn the timeout-aware read judges it file_not_found, clears it, and returns null (dist/state/session.js:92-106). So existingSession is null while hadAgentSession is true, and the adoption branch — the one that would have honored the caller's explicit resume: A — is gated shut. effectiveResume stays undefined and the turn starts fresh.

The gate asks "did this agent have a pointer?" when the question is "was the thing that just got cleared the session the caller asked for?" Those coincide only when the pointer named the caller's chat.

Every rule the reproduction established falls out of this, which is what convinced me it is the whole mechanism and not a partial one:

Observed	Why
only the most-recently-active chat's deletion breaks anything	the pointer is rewritten every turn, so it only ever names that one
one-shot — the second send works	the fresh turn rewrites the pointer, and existingSession.session_id !== options.resume then correctly trusts the caller
archive does not trigger it	it removes nothing, so nothing dangles
promote does	it deletes the source transcript by the same call
scoped per project	one pointer per keeper agent
"it worked after a restart" (the note at herdctl.ts:1525)	a restart is just a run where no pointer exists yet, so adopt is free

Where the fix belongs

Upstream. Filed as edspencer/herdctl#448 with the one-line repair: capture the pointer's id, not just its existence, so hadAgentSession means "the pointer named the session the caller asked for". Any consumer whose transcripts vanish by some other route — a manual rm, a retention sweep, a crashed write — hits the same misclassification, and Paddock cannot fix that from here.

What this PR does instead

The thin, well-understood guard the brief allows for, and it is narrower than the upstream fix while being true regardless of it: when Paddock removes a transcript, it drops the agent-level pointer if that pointer names it.

That file is already dangling by the time we reach it, and herdctl would clear it at the very next turn anyway — clearing it at the moment we make it dangle is semantically a no-op that simply moves the clear out of the code path that misreads it. The next turn then sees a clean "this agent owns no session" and adopts the caller's explicit resume, which is the same path a process restart took. A pointer naming a different chat is live state and is left alone; the whole thing is best-effort, so failing to tidy can never fail the delete the user asked for.

Called from all four places Paddock removes a transcript from an agent's working directory: the own delete path, promoteSession, and the two import-undo paths that delete a copy an import placed. Under claude.transcripts: host nothing is removed, so there is nothing to drop.

Scope

driveMode: batch only. The SDK session path (driveMode: session, the default) never consults the pointer when the caller names a session, so it is structurally immune — worth stating plainly, since the issue does not record which mode the reproducing instance ran. If the report came from a session-mode instance then there is a second mechanism still at large, and I would want to know that; everything in the report is reproduced exactly by this one.

Tests

packages/server/test/integration/resume-after-delete.test.ts — server-integration tier (startTestApp + real FleetManager + real CLI runtime + the fake claude), per §5.3 item 0a. No browser: the whole bug is server-side session resolution.

• a turn sent to A after deleting B lands in A, with A's history behind it (the fake keeper still recalls the codeword), and no stray chat appears
• same after promoting B away
• a control: deleting an older chat, and archiving, change nothing

The first two fail on main (expected '907e4670-…' to be '28e6602a-…') and the control passes there — so the control is pinning the boundary, not just passing vacuously. There was previously no test anywhere that sent a turn after a delete; delete-chat-host-transcripts.test.ts covers the deletion and stops.

npm test (2,901 tests) and npm run typecheck green.

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying paddock with    Cloudflare Pages

Latest commit:	7e7e927
Status:	✅  Deploy successful!
Preview URL:	https://53abc5ec.paddock-7u2.pages.dev
Branch Preview URL:	https://fix-730-resume-after-delete.paddock-7u2.pages.dev

View logs

[edspencer]

Answering your scope question directly: the reproduction ran on batch.

I'm the author of the report, so I can settle this rather than leave it hanging.

The QA instance was a pm-run wrapper that explicitly exported PADDOCK_DRIVE_MODE=batch in order to reach the fake claude (the CLI runtime is the only one that spawns from PATH). I verified it at the time in /proc/<pid>/environ, and the audit write-up records it in its method section. Every reproduction in #730 — all 8+ across 5 probes, including the browser one — was on batch.

So your caveat is exactly right, and I'd go further: there is no evidence of a second mechanism, because there is no session-mode observation at all. Your analysis explains 100% of what I saw, and I'd treat "session mode is structurally immune" as unrefuted rather than as something my report contradicts.

What that means for severity, which I think is worth stating on the ticket

session is the default drive mode. If the pointer is genuinely never consulted when the caller names a session, then default-configuration users were never exposed to this, and the real-world blast radius is projects explicitly set to driveMode: batch (plus the sweeper/trigger paths, which are always CLI). That is materially narrower than "critical data loss for everyone", which is how I pitched it — and the correction is yours, not mine.

The fix is still worth having: batch is a supported, documented per-project mode, and the upstream misclassification you filed as edspencer/herdctl#448 bites any consumer whose transcript vanishes by any route (manual rm, retention sweep, crashed write). But I'd rather the ticket carry an accurate scope than an inflated one.

The uncomfortable general point

This is the third time in this workstream that a finding has turned out to be shaped by the harness rather than by the product: the sub-agent bug (#725) only reproduces in a state the fake couldn't previously produce; the mid-turn fork/revert transcripts are invalid in a way only the real Messages API would reject; and now #730's mechanism is batch-only while the default is session. All three are the batch-vs-session blindness #717 was filed about. I'll carry that caveat explicitly into the remaining fix waves rather than re-learn it each time.

Suggested follow-up, not for this PR: a deliberate check of whether the delete→resume sequence is truly clean on session mode with a real credential. If it is, we can say so on the issue and close the question; if it isn't, there's a second mechanism and we'd want to know. That's precisely a test:qa:live item under #717 rather than something this PR should carry.

Nothing blocking here — the root-cause analysis is more complete than what I filed, the upstream/downstream split is the right call, and the control test pinning the boundary is the detail that convinced me this is the whole mechanism. Thank you for not building on my wrong guess.