Preserve unique work across archive cleanup

[zionts]

Summary

• fail closed on advisory bootstrap manifests unless an internal trusted attestation matches worktree, HEAD, producer version, and exact manifest bytes
• revalidate after archive hooks, reject staged tracked dirt, and only mark archived after verified physical removal
• isolate GC snapshots with a scratch Git index and recheck registration, lock, HEAD, live CWD, and status before removal
• keep ignored paths outside the safety boundary, so the fail-closed gate stays usable rather than being routed around

Where the boundary sits

Fail-closed applies to everything Git tracks or reports as untracked. It does not extend to ignored paths, and that line is deliberate.

An earlier revision folded ignored files in. Measured on this repository's own worktree — 11,453 ignored files, 668 MB — that produced three effects, none of them more safety:

• Non-force archive became impossible for any worktree that had ever been built. The app never passes force: true anywhere; --force exists only in the CLI. The primary interface would have offered an action that always failed.
• The refusal message joined every offending path into one string and handed it to a GUI alert through an RPC error.
• GC committed the entire build tree into refs/tbd/snapshots/…. Snapshot refs stay reachable, so git gc never prunes them — the repository would grow by hundreds of megabytes per reaped worktree while preserving no work. This also silently reversed a pre-existing assertion on main that ignored bytes stay out of a snapshot.

.gitignore is the user's own standing declaration that those bytes are reproducible, and it is the only signal Git offers — nothing distinguishes a build tree from an ignored file someone would want back.

Excluding them costs nothing this design was built to buy. Bootstrap scaffolding — .agents, .codex, .Codex, hooks, AGENTS.md — is untracked by convention rather than ignored, so it still arrives as ?? and remains subject to the full provenance check. An ignored file has been removed along with its worktree since long before this PR; that behavior is unchanged, not newly introduced.

The alternative was not a stricter system but a bypassed one: an archive that always refuses trains every user onto --force, which skips the unpublished-commit check this work exists to enforce.

blockingSummary now caps each category at twenty paths regardless.

Correction to an earlier description

A previous version of this description said it "wire[s] known-published state into auto-archive", which read as though auto-archive trusts a known-published shortcut. It does not. Auto-archive always re-verifies via isHeadReachableFromAnyRemote, and mergedTransitionDoesNotWaivePublicationChecks pins that. knownPublished: true is passed only by GC, where it is a no-op because requiresPreservation never consults headIsPublished.

Validation

• swift build --target TBDDaemonLib
• swiftlint --strict — 0 violations across 643 files
• swift-format lint --strict for the changed classifier and test files
• Full swift test build succeeds locally under the 6.3.2 toolchain; hosted CI remains the authoritative run
• An independent 14-assertion harness validated the git-level premises the boundary rests on, using plain git in a scratch repo rather than TBD code: ignored paths absent from status --porcelain -uall; bootstrap families present as per-file ??; staged (M ) and staged-plus-unstaged (MM) tracked dirt still reported; git add -A into a scratch GIT_INDEX_FILE omitting ignored paths and leaving the real index untouched; publication flipping correctly across commit/push; merged HEAD still remote-reachable; and an in-worktree git branch -m observable via git worktree list once paths are symlink-resolved.

No archive, worktree removal, restart, deployment, or live-desk mutation was performed.

Known gap: GUI recovery for genuinely dirty worktrees

Before this PR, non-force archive from the app essentially always succeeded, because git worktree remove --force ran unconditionally — the silent-data-loss bug this PR fixes. After it, a worktree with real (non-ignored) uncommitted or unpublished content is refused, and the app's own Archive action never forces. A GUI-only user who wants to discard such a worktree has no targeted in-app override.

Correction to an earlier revision of this description. It claimed the app never passes force: true anywhere. That is wrong, and the claim came from a grep for the literal force: true that missed a computed argument. RepoSectionView.swift:157 passes force: activeWorktreeCount > 0 to "Remove repo from list", and handleRepoRemove then cascades archiveWorktree(force: true) over every active worktree in the repo — skipping the classifier preflight, the revalidation, and everything else this PR adds. Its confirmation text ("Your git repository and files on disk are not touched") is materially wrong for that path, since the worktree directories are force-removed.

That path is pre-existing and its blanket force is untouched here, and it is not a usable override for archiving one worktree — it is unlabelled, repo-wide, and all-or-nothing. But it does mean the escape hatch is not CLI-only as previously stated. The mis-worded dialog is fixed in this PR; only the cascade's force semantics are left alone.

Failing closed is still the intended direction — it fails safe, and the refusal is legible. What was missing was a route out, so archiveUnsafe now names it: the message carries the blocking summary and tbd worktree archive <name> --force. Both the CLI and the app alert render that description.

A proper in-app affordance — an "Archive anyway" confirmation on the refusal alert — is deliberately not in this PR. It is a UI decision about how prominent a total-bypass control should be, and it belongs to a spec rather than to a fix that is already changing the deletion gate. Tracked as follow-up.

Writers are silenced before the final check

An earlier revision of this PR moved terminal teardown from phase 1 to after removal. That left a live Claude/Codex agent running through the archive hook, the final revalidation, and git worktree remove --force — so it could create a file after the check returned and before removal executed, and forced removal would discard it with nothing having observed it. On main no live writer remained by the time removal ran, so the window was newly introduced here.

Terminals are now captured and killed first, immediately after phase 1 has already gated eligibility. captureThenKillWindow only touches tmux and the history rows — never the directory — so it is free to run that early, and the terminal rows themselves still survive until removal is verified.

Ordering alone was never the property. The property is that nothing can write to the worktree between the last check and the removal. Publishing archived-final state is what waits on physical absence; silencing writers is a precondition for a safe removal and belongs before it. The spec now draws that distinction, and a test pins the whole sequence through the tmux dry-run recorder: classify, kill, classify, remove.

Availability, and where the gate deliberately relaxes

A gate that refuses ordinary cases is not stricter, it is routed around. Three such cases were closed:

• Repos with no remote. The publication check ran git branch -r --contains HEAD, empty when no remote is configured, so a spotless fully committed worktree in a local-only repo was refused forever. Eligibility now asks whether HEAD survives removal — a remote-tracking branch where one exists, otherwise "some local branch contains HEAD". git worktree remove keeps the branch, so those commits stay reachable. Detached HEAD on no branch still blocks; content eligibility is untouched, so deleting a remote waves nothing through; any git failure still fails toward preservation. Worth stating plainly: this check protects discoverability, not reachability — archive keeps the branch either way.
• A failing archive hook still stops the archive, but now throws archiveHookFailed naming the hook, rather than a safety refusal. Reporting a broken user script as "unique unpublished work, use --force" sends an ordinary scripting bug through the one flag that skips every content and publication check.
• The refusal message said tbd worktree archive <name> --force, which nobody can run. It now names the worktree.

Plus two wording fixes: the common no-manifest refusal reason now reads "uncommitted or untracked content" instead of naming a bootstrap attestation the user has never encountered, and "Remove repo from list" no longer claims files on disk are untouched while force-archiving every active worktree.

Explicitly deferred

Non-blocking archive via a persisted .archiving status. The archive RPC is synchronous through hook, revalidation and removal, so the row can sit inert for up to ~60s. The proposed fix — a new WorktreeStatus case, a migration, recovery for rows stuck in .archiving, and daemon/app decode skew — is a shared-model change, which CLAUDE.md requires to land with a spec and a recovery story in one commit. It is deferred to its own spec and PR rather than stacked onto a safety change that is already four review rounds deep. The cost of deferring is UX latency, not data loss.

The repo-removal cascade's blanket force. RepoSectionView passes force: activeWorktreeCount > 0, and handleRepoRemove force-archives every active worktree, skipping this PR's gate entirely. Pre-existing and untouched here; only its misleading confirmation text is corrected. Routing it through the classifier changes the behavior of a path this PR never touched and deserves its own decision.

Exemption claim. No feature flag. This is a bug fix under CLAUDE.md — archive was calling git worktree remove --force unconditionally and destroying unpublished work — so the default-off flag rule does not apply. The availability shift is real, which is why the three cases above were closed rather than left to --force.

[tbd-claude-reviewer]

Claude finished @zionts's task in 3m 10s —— View job

---

✅ Approve

Re-review after the latest push. Both surviving findings from the prior automated review round (07:47:15Z) are fixed and tested by 7482ac2 ("fix: harden archive failure handling") + 3c14b46 ("docs: align archive ordering spec").

Verified fixes

1. Archive hook non-zero exit is now detected (was HIGH)
WorktreeLifecycle+Archive.swift:123-143 now destructures (succeeded, output) from hooks.execute and throws archiveHookFailed when succeeded == false, carrying bounded hook output as the error detail. Confirmed against HookResolver.execute's actual signature (Sources/TBDShared/HookResolver.swift:90-93), which returns (Bool, String) and only throws on process-launch failure — the previous code discarded the Bool, so this was the real gap.

The hook now runs before terminal capture/kill (previously kill came first) — a deliberate reorder: a hook failure is a recoverable precondition failure and should leave terminals/sessions intact for diagnosis rather than already dead. Covered by the new test nonzeroArchiveHookBlocksBeforeTerminalTeardown (zero kill-window commands issued, worktree stays .active, terminal row and directory survive), and the ordering test now pins ["classify", "hook", "kill", "classify", "remove"]. Force-archive still skips the hook (if !force at line 107) and still kills terminals unconditionally, matching existing force-path tests.

2. Dead terminals / no failure signal after terminal teardown (was MEDIUM)
WorktreeLifecycle+Archive.swift:183-251 wraps the final-classify → remove → path-check → DB cleanup → db.worktrees.archive sequence in a do/catch. On any failure there, it best-effort deletes the (now-dead) terminal/tab rows, broadcasts .terminalRemoved for each so the UI doesn't show dead panes as live, and rethrows as a new archiveInterruptedAfterTerminalStop error with a clear description ("...The worktree may still be active; inspect it before restarting work or retrying archive"). The worktree row is never flipped to .archived on this path.

AutoArchiveOnMergeCoordinator.swift's catch block now persists and broadcasts an .error notification when handleMergedTransition fails after identifying an active worktree, closing the silent-failure gap on the auto-archive path specifically called out by the prior review. Verified the db.notifications.create/unread call sites match NotificationStore's real signatures (Sources/TBDDaemon/Database/NotificationStore.swift:69-95). New test failedAutoArchivePersistsAndBroadcastsError drives this through a no-op worktreeRemover (so the post-removal path-existence check fails) and asserts one persisted .error notification and one matching broadcast delta.

The RPC path (handleWorktreeArchive) already runs completeArchiveWorktree synchronously from the original PR commit, so a human-triggered archive that hits this failure surfaces directly as an RPC error — no additional wiring was needed there.

Bonus, not required: ArchiveSafetyClassifier.regularFileData was rewritten to use openat(..., O_NOFOLLOW)-chained descriptors instead of FileManager.attributesOfItem + Data(contentsOf:), closing the TOCTOU gap a prior review round flagged as Low/dropped. New tests cover symlinked-component rejection and a race simulated via an afterOpen test seam, proving the read stays pinned to the originally opened descriptor.

Spec doc (docs/specs/2026-08-01-archive-bootstrap-provenance-design.md) was updated in the same round and accurately reflects the new classify → hook → kill → classify → remove order and failure semantics.

Self-directed check (not from prior rounds)

Traced whether the new catch block's cleanup could double-run against a partially-successful try block (e.g., db.worktrees.archive throwing after terminals/tabs deletes already succeeded). The re-run deletes are idempotent no-ops and the re-broadcast .terminalRemoved for already-absent ids is harmless for delta application. The underlying ordering (remove → verify gone → delete terminals/tabs → archive) predates this round; this round only adds catch-based cleanup/signaling around it — not a new defect, not flagged.

No new High/Medium issues found in the delta. 1 self-generated item investigated and dismissed as not a new defect (see above) — not counted as filtered "invalid feedback" since it was never drafted as a finding.

Review diagnostics

No tool calls failed or were denied. Static review only (ubuntu-latest runner, no macOS Swift build available) — relied on reading source directly and cross-checking call-site signatures against their definitions (HookResolver.execute, NotificationStore.create/unread) to confirm the new code compiles as intended.

[cheapsteak]

Review: policy is right, execution has two gaps, and the RPC shouldn't block

The core fix — never publish archived-final state before verified physical removal, and refuse to delete work that exists nowhere else — is correct and well-tested. We researched how comparable tools handle this (details at the bottom): the refuse-unless-clean-and-pushed policy is at the strict end of the field but matches the strongest precedent almost check-for-check, so the policy needs no softening. The findings below are about execution.

1. Make archive non-blocking with a persisted archiving state (High, UX/architecture)

handleWorktreeArchive now blocks through the 60s hook timeout plus git removal, and the worktreeArchived broadcast fires only at the end — the user clicks Archive and the row sits inert, apparently ignoring them, for up to a minute. This also runs against the repo's own precedent: handleWorktreeRevive was made non-blocking specifically because blocking an RPC on a hook "would starve the RPC connection" (RPCRouter+WorktreeHandlers.swift:232).

The safety design doesn't require a waiting caller — it requires ordering (hook → revalidate → remove → verify gone → only then archived-final) and a failure channel. Both exist without blocking:

• beginArchiveWorktree stays synchronous and cheap: run the classifier, and throw archiveUnsafe immediately on refusal (the common case still surfaces as a direct RPC error — nothing about the gate is weakened). On eligibility, flip the row to a new persisted .archiving status, broadcast it, return.
• Completion runs in a background task with the PR's exact internal ordering unchanged. On success: archived-final flip + worktreeArchived broadcast. On refusal/failure: revert to .active and broadcast a persisted notification carrying the blocking summary.

A persisted .archiving state (rather than an optimistic client-side one) buys crash recovery (+Recovery already resolves rows stuck in .creating — same pattern), concurrency gating (other ops bounce off the status instead of racing), and multi-client consistency via the existing delta channel. Notes: adding a WorktreeStatus case is a shared-model change with daemon/app skew implications (a stale app can't decode the new case), and recovery for .archiving rows should ship in the same change. The CLI can keep the synchronous all-in-one path — it genuinely wants to wait.

2. Live terminals can write during the "final" safety window (High, race)

The new order is hook → re-classify → git worktree remove --force → then capture/kill terminals. The comment on the re-classify says nothing "can mutate or race the content attested by this check" — but the biggest mutator, a still-running agent whose cwd is the worktree, is alive through both the re-classify and the removal. A file written in that window is destroyed unsnapshotted. GC got exactly this defense (before/after status compare + recheck) in this same PR; archive didn't. Moving to the .archiving design makes the fix natural: after begin flips the status, quiesce/kill terminals, then run hook → re-classify → remove. A refusal at the first (common) gate still preserves terminals; a refusal after quiesce costs tmux sessions but never bytes.

3. Repos with no remote can never non-force archive (High, availability)

Eligibility requires isHeadReachableFromAnyRemote, and empty output or git failure returns false. A repo with no remotes configured refuses every archive forever, even fully committed clean worktrees — training exactly the --force habit this PR argues against. Worth noting the unpushed-HEAD check protects discoverability, not commit reachability: archive keeps the branch, so commits survive removal regardless (plain git refuses only on dirty for this reason). That justifies the check, but also justifies a fallback: when git remote lists nothing, fall back to isReachableFromAnyBranch.

4. Smaller findings

• Hook failure now blocks archive entirely (try? → try). A chronically broken user hook leaves only --force, which skips the hook and all safety — strictly more dangerous than before. Consider distinguishing hook failure from safety refusal. Either way this behavior change belongs in the PR description.
• No flag, and no exemption claim. Arguably a bug fix (exempt per CLAUDE.md), but the availability shift is large — dirty worktrees are the norm in agent workflows. The description should either name a flag or explicitly claim the bug-fix exemption.
• knownPublished on the lifecycle methods is never true in production (GC calls the classifier directly) — could be dropped from that API.
• The refusal message says literally tbd worktree archive <name> --force; the actual name is available at both throw sites.
• The common-case finding reason ("no matching trusted out-of-worktree bootstrap attestation") is confusing for plain uncommitted changes; consider a plain reason when no manifest exists.
• New files use 2-space indent; the codebase uses 4-space.

---

Appendix: what comparable tools do

Surveyed: Claude Code (binary v2.1.220, via strings), Codex CLI, OpenCode, Crystal, claude-squad, ccmanager, vibe-kanban (source), plain git (empirically), GitButler/jj (docs).

Tool	Dirty check	Unpushed check	What happens
plain git worktree remove	refuses	none	--force overrides; branch always survives
Claude Code (headless job cleanup)	refuses ("has uncommitted changes, kept")	refuses ("commits that are not pushed anywhere")	keeps in place; force override
Claude Code (interactive exit)	state-aware prompt	commits-ahead check	explicit discard_changes opt-in
Codex CLI	—	—	doesn't manage worktrees
OpenCode	UI warning only	none	server force-deletes worktree and branch -Ds
Crystal	none	none	generic confirm → worktree remove --force
claude-squad (kill)	none	none	force remove + branch -D — unpushed commits destroyed
claude-squad (pause)	auto-commits	n/a	preserve-by-commit, keeps branch
ccmanager	warns, click-through	none	two-step confirm, then force
vibe-kanban	none at delete time	none	auto-commits after every agent run; worktree is disposable
GitButler / jj	n/a	n/a	snapshot everything, allow anything, undoable

Takeaways:

• This PR's policy mirrors Claude Code's own headless deletion path check-for-check (dirty → keep, unpushed-to-any-remote → keep, force override) — the right comparable, since both are non-interactive daemon-side destruction paths. It is stricter than every open-source worktree manager surveyed; several of those silently destroy uncommitted work (and two delete the branch, making unpushed commits genuinely unreachable).
• The road not taken is preserve-rather-than-refuse (claude-squad pause and vibe-kanban auto-commit before removal; GitButler/jj snapshot everything). TBD already has snapshot machinery on the GC reap path but deliberately refuses in place on the archive path. That's defensible — refusal leaves the user's state untouched rather than creating machine-made commits, and it's what Claude Code chose too — but the spec should state the choice explicitly, since a snapshot-then-archive middle path would dissolve most of the refusal gate's UX friction and is the obvious follow-up if the gate proves too aggressive in practice.

---

Generated by Claude Code

[tbd-claude-reviewer]

Claude finished @zionts's task in 10m 39s —— View job

---

🧌 Changes requested

The overall policy (fail-closed archive gate, GC snapshot isolation, .gitignore boundary exclusion) is sound and well-argued, and the prior human review round's concerns (non-blocking RPC deferred to follow-up, terminal-write race, no-remote availability) check out against the current code. Two execution gaps survived independent verification:

1. HIGH — Archive hook non-zero exit is never detected as a failure (Sources/TBDDaemon/Lifecycle/WorktreeLifecycle+Archive.swift:136)

The PR's stated fix — hook failure now blocks archive via try instead of try? — doesn't work for the common case. HookResolver.execute (Sources/TBDShared/HookResolver.swift:90-134) resumes with .success((terminationStatus == 0, output)) regardless of exit code; it only throws when the process fails to launch. The archive call site discards the returned Bool (_ = try await hooks.execute(...)), so a hook script that runs and exits non-zero (e.g. set -e, explicit exit 1 — the overwhelmingly common failure mode) is silently treated as success, and the archive proceeds through revalidation and removal. Confirmed this is the only call site of hooks.execute in Sources/, and the only test naming archiveHookFailed (ArchiveSafetyClassifierTests.swift:362) constructs the error directly rather than driving a real failing script through completeArchiveWorktree.

2. MEDIUM — Failure after terminal-kill leaves the worktree active with dead terminals and no signal on auto-archive (Sources/TBDDaemon/Lifecycle/WorktreeLifecycle+Archive.swift:112)

Terminals are killed first, before the hook/revalidate/remove sequence that can still throw. Any failure there (including #1 above) leaves the worktree .active in the DB with terminal rows present but their tmux windows already dead. On the RPC path a human sees the error directly; on the merge-triggered auto-archive path, AutoArchiveOnMergeCoordinator.handleMergedTransition (AutoArchiveOnMergeCoordinator.swift:67-70) only logs and returns false — no persisted/broadcast notification, unlike the success path. A user could end up with a worktree that looks active but has no working terminals, discoverable only via the daemon log.

1 item from the correctness pass was filtered out as not rising to HIGH/MEDIUM (see disposition below).

Minor items

• ArchiveSafetyClassifier.regularFileData (ArchiveSafetyClassifier.swift:313) has a TOCTOU gap between symlink-safety lstat checks and the subsequent Data(contentsOf:) read — a concurrent local process could swap in a symlink between check and read. Low real-world severity given the single-user local-daemon trust model and that terminals are already killed by this point.

Finding dispositions

• correctness-1 — kept (HIGH). Independently verified against HookResolver.execute source.
• correctness-2 — kept (MEDIUM). Independently verified against AutoArchiveOnMergeCoordinator.swift.
• correctness-3 — dropped. Real but low-confidence/low-severity TOCTOU noted in Minor items instead of counted as a surviving finding.

No conventions findings were filed (0 filed, 0 dropped) — the specialist checked the default-off-flag exemption claim, TUI-scraping surface, public-repo/multi-tenant leakage, DB migration triple-update, and theory-placement on the 20-path cap / 60s timeout / .gitignore boundary, and found each either not applicable or already documented in the committed spec (docs/specs/2026-08-01-archive-bootstrap-provenance-design.md).

Review diagnostics

No tool calls failed or were denied for either specialist or the orchestrator. Both specialists completed their reviews and wrote valid findings files; the orchestrator additionally re-verified the surviving HIGH/MEDIUM findings directly against source before merging.

[tbd-claude-reviewer]

Review in progress

• Read PR discussion context
• Inspect diff scope (git diff origin/main...HEAD)
• Spawn correctness specialist
• Spawn conventions specialist
• Merge findings and write review-result.json
• Post final review comment

View job run

[tbd-claude-reviewer]

Claude finished @zionts's task in 10m 2s —— View job

---

✅ Looks good

Both specialists (correctness, conventions) returned empty findings arrays — no surviving HIGH/MEDIUM/MINOR items.

Correctness ran the mandatory premise audit for this guard/safety-shaped change (fail-closed archive gate, GC snapshot isolation) and independently re-verified the two HIGH/MEDIUM findings from the prior automated review round against the current HEAD:

• Hook non-zero exit detection — now caught. Sources/TBDDaemon/Lifecycle/WorktreeLifecycle+Archive.swift:123-148 captures (succeeded, output) from hooks.execute and throws archiveHookFailed when succeeded is false, rather than discarding the tuple.
• Post-terminal-kill failure leaving silent inconsistent state — now handled. WorktreeLifecycle+Archive.swift wraps the post-kill removal/revalidation in a do/catch that throws archiveInterruptedAfterTerminalStop (declared WorktreeLifecycle.swift:17), and AutoArchiveOnMergeCoordinator persists+broadcasts an error notification on this path, covered by AutoArchiveTriggerTests.failedAutoArchivePersistsAndBroadcastsError (Tests/TBDDaemonTests/AutoArchiveTriggerTests.swift:176).

Both fixes were spot-checked directly against source by the orchestrator (grep/read), not taken on the specialist's word alone.

Ten additional premises from the PR description/spec (GC never has a live trustedAttestation in production, the scratch GIT_INDEX_FILE is per-Process not global setenv, the classify → hook → kill → classify → remove ordering is asserted by test, merge-triggered archive never infers publication from the merge event, RepoSectionView's corrected confirmation text is now factually accurate, etc.) were each verified with file:line evidence and held up. No fail-open path was found in ArchiveSafetyClassifier.classify.

Conventions checked all five assigned areas and found no violations: the bug-fix exemption from the default-off-flag rule holds (every destructive surface touched was already autonomous/destructive pre-PR; the diff only tightens safety); no new TUI screen-scraping was added (captureThenKillWindow is pre-existing and doesn't parse screen text for safety decisions); no private/org-specific context leaked into the diff (test fixtures consistently use acme placeholders); Database.swift is untouched so the migration triple-update rule doesn't apply, and the committed spec accurately reflects the shipped code; the .gitignore-boundary theory, the 20-path cap, and the hook timeout are all documented as deliberate choices in the spec (the hook timeout value itself is unchanged from pre-PR code).

0 findings filtered out as invalid during merge (none were reported to filter).

Finding dispositions

No specialist findings were reported — nothing to dispose of.

Review diagnostics

No tool calls failed or were denied for either specialist or the orchestrator. Both specialists completed their reviews and wrote valid findings files; the orchestrator additionally re-verified the two carried-over HIGH/MEDIUM findings from the prior round directly against source before concluding they no longer apply.