The post-merge review's nine findings, fixed as directed

[brillout]

Implements every direction from the review round on #1536, one commit per instruction, in the order given.

What each commit does

1. handoff: local holds — removeProjectWorktree keeps a publish-nothing session's unpushed checkout (the remote-less fallback, no new mechanism) instead of pushing it to make removal possible. The armed handoff is read off the agent's meta (written as its first event), so a boot death keeps the recoverable default, and an already-pushed branch still lets the checkout go. Covers teardown, the sweep, and the dashboard's Remove alike, since they share the one implementation.
2. Footer — the framework "<what to build>" line D4 orphaned is gone; a test pins the absence.
3. Spec cleanup — readAgentSpec consumes the mkdtemp directory with the file, and both spawners remove a spec whose child never ran. Only a directory carrying the module's own prefix is removed whole, so a hand-written --agent <path> spec can't take the user's directory with it.
4. Stale claim comment — the daemon-services.ts line claiming the pinned entry "outlives the agent's local process" is replaced with what holds (the pushed drain lock for tickets), and auto-pm.SPEC.md now owns the ticketless hand-off window as accepted-not-closed, with the rationale on record.
5. Shutdown gap — onStart refuses once stopAgents has run, checked at entry and re-checked right before the spawn (the framework: auto PM has no stopped flag, so quiesce cannot stop an in-flight sweep from spawning an orphan run #983 shape), cleaning up the spec it wrote when the late check refuses. No shutdown reordering.
6. Dead surface deleted — preset:/event: (keys, resolution, echo, chips — absence pinned beside the other retired spellings), the six transport-less option fields plus the modes event kind and OPEN_LOOP_MODES, the unreachable "not enabled on this server" guards, and the control.SPEC.md sentence that contradicted context.SPEC.md. The surviving option docs stop naming D4's flags.
7. Sweep — hasRemote asked once per project (no per-checkout probe-and-push on a repo that can never push), and each kept checkout's line said once, not every ten minutes — re-announced only if the checkout reappears after a removal or a daemon restart.

Behaviour changes, called out

• A handoff: local session's checkout now stays on disk (visible on the Remove list) until its branch reaches the remote by someone's explicit act, or Delete throws it away. Remove refuses for it the same way it refuses on a remote-less repo. The lifecycle e2e story pins the new contract (checkout kept, nothing on origin); the publish story runs push-armed so it keeps pinning the click-vs-teardown ref collision.
• An unknown preset:/event: value in a committed yml no longer errors — the keys are unknown now, and unknown keys are ignored (the zero-migration rule, pinned in config.test.ts).

Verification

Both typechecks, a full build, 1466 node tests / 771 dashboard tests (81 files), all green on the head. The three files spanning multiple commits are byte-identical between the tested tree and the final commit.

🤖 Generated with Claude Code

https://claude.ai/code/session_0148SehcEqApqZTvG5wPzJoj

---

Generated by Claude Code

[brillout]

Merge conflict is being fixed.

@suleimansh I'll let you merge if you think this is good.

[suleimansh]

Reviewed all seven commits — spot-checks plus a high-effort review pass. The nine directions are implemented as directed, and the deletions all trace clean: every removed surface verifiably had no remaining consumer, and the removed guards were unreachable since D3 made the context accessor throw.

Four things worth fixing before this merges, all small, all in the keep-checkout half:

1. The handoff: local guard fails open on an unreadable meta (worktrees.ts:149). findAgent(...).catch(() => undefined) collapses a read failure into "no meta", and meta?.handoff && !meta.handoff.push then falls through to pushAgentBranch — publishing the exact branch the rung refuses, then deleting the checkout. Distinguish "meta read fine but has no handoff" (boot death — push, as intended) from "meta could not be read" (keep, retry next sweep).
2. commitPendingWork runs before the publish-nothing refusal (worktrees.ts:134). A kept handoff: local checkout is now a place someone works — and every 10-minute sweep pass commits their WIP as "[The Framework] uncommitted changes" before reaching the refusal. Move the meta check above the commit.
3. The shutdown re-check leaks the workspace it just allocated (daemon-runtime.ts:748). Between the entry check and the pre-spawn re-check, allocateWorkspace created a worktree + branch; the refusal removes only the spec. The orphan has no agent.json, so the next boot's sweep — with finding 1's fail-open — pushes an empty tf-agent-* junk branch to origin to reclaim it. Move the re-check above allocation, or tear the workspace down on refusal.
4. A child that spawns and dies before consuming its spec leaks it (cli.ts:1357 + the daemon exit handler). Cleanup fires only on the spawn error event; a boot crash leaves framework-session-*/session.json — full prompt plus any device token — on disk per attempt. removeAgentSpec is idempotent, so also calling it on nonzero exit is a one-liner per site.

Now or as follow-ups, your call:

• removeAgentSpec trusts the framework-session- name prefix as ownership — a user's own directory with that name gets rm -rf'd whole (agent-spec.ts:66); checking the parent is the configured spec dir closes it.
• The kept-checkout announce-dedup keys on agentId only, so a changed keep-reason (say you add a remote and pushes start failing on auth) never logs (merged-worktrees.ts:147). Key on id+reason.
• Kept handoff: local checkouts re-pay the full probe-and-push cycle every tick (merged-worktrees.ts:86), and the one-boolean handoff read goes through findAgent's full archive scan (worktrees.ts:149) — the batching this PR gave the no-remote case fits both.
• repoHasRemote duplicates the inline probe in agent-handoff.ts:277 — point the latter at the new helper.
• FEATURES-SPEC.md rows 77/97 don't reflect the permanent-keep behavior; AGENTS.md wants the feature list current.

[brillout]

All four blockers fixed and pushed — three commits — plus four of the five your-call items; one deferred, named below.

1. Fail-open meta read — the read is strict now, and it had to leave findAgent: the store's list reads are forgiving at every level (down to nodeFs.readdir returning [] on any error), so "could not be read" was structurally invisible through them. removeProjectWorktree reads the live agent.json, else the archived record, directly: ENOENT is a boot death and keeps the recoverable default; any other read or parse failure refuses with the error and a later pass retries. Test writes a corrupt agent.json and pins keep-plus-nothing-pushed.

2. Commit before the refusal — the handoff decides first now, and a publish-nothing checkout goes only from a clean tree on a pushed tip (a new worktreeClean — the status probe without the commit). The keep test now also pins that the dirty edit stays uncommitted and the branch gains no commit. This closes the wider hole too: previously a local-handoff checkout with a hand-pushed tip plus new WIP would have been committed and pushed by the old commit-first ordering.

3. Leaked workspace on shutdown refusal — kept the late re-check (it covers the awaited allocation+spec span the entry check can't) and made the refusal take everything back: spec, fresh worktree, and its branch. A continuation's checkout is the agent's own and stays. Test lands the stop through the preflight seam — deterministic, no race — and pins no branches/ entry, no tf-agent-* branch, empty spec home.

4. Spec left by a child that died unread — both spawners also remove the spec on exit now, a no-op after a consumed one. Tested against the dying stub with a scoped spec home.

Your-call items: took the removeAgentSpec ownership check (prefix AND directly-in-the-spec-home; a same-named user directory elsewhere loses only the file — tested), the reason-keyed announce dedup (changed reason logs again — tested), the repoHasRemote reuse in the handoff read, and the FEATURES-SPEC rows (77 also predated E5 — it still said failed/stopped keeps for inspection). Deferred: the batching/perf item — skipping the probe cycle for known-local checkouts wants the sweep to read metas before offering rows, which reshapes removeMergedWorktrees's seams; cheap once #1582's queue work touches that area anyway, and the announce-dedup already removed the visible symptom.

Verified on the head: both typechecks, a full build, 1483 node / 771 dashboard (81 files).

---

Generated by Claude Code