feat: automatic lossless post-reboot recovery of worker sessions - #311
feat: automatic lossless post-reboot recovery of worker sessions#311kwliang1 wants to merge 1 commit into
Conversation
| export async function reattachWorktree(repoDir: string, worktreePath: string, branch: string): Promise<ReattachResult> { | ||
| // Branch must exist to reattach — otherwise there's nothing to preserve. | ||
| try { | ||
| await execAsync('git', ['-C', repoDir, 'rev-parse', '--verify', branch], { timeout: 5_000 }) |
There was a problem hiding this comment.
Should-fix (correctness): rev-parse --verify runs outside the repo lock, but worktree add runs inside it. Between the two, a concurrent killSession (or external prune) can delete the branch. When that happens, worktree add fails → returns 'failed' (treated as transient/retryable), but the branch is truly gone — the 'branch-gone' path (which correctly sets suppressAutoRecover) is never reached.
The session stays in limbo: not recovered, not marked gone, retried every boot forever.
Fix: Either move the rev-parse inside the withRepoLock call, or on worktree add failure re-check whether the branch still exists and return 'branch-gone' if it doesn't.
Flagged by both reviewers.
| // builders in their post-sweep state (awaiting_pm ones marked suppressAutoRecover, | ||
| // others killed) rather than racing it. | ||
| const factorySweep = sweepOrphanedBuilders().catch(err => { | ||
| process.stderr.write(`daemon: factory sweep failed: ${err}\n`) |
There was a problem hiding this comment.
Should-fix (reliability): The .catch() here resolves the promise (returns Promise<void>), so await factorySweep at line 348 always succeeds — even if the sweep threw mid-way. autoRecoverAfterBoot then runs against a registry in a potentially inconsistent state (some builders swept, some not).
Either re-throw after logging so auto-recover is skipped on sweep failure, or explicitly document that partial-sweep + auto-recover is accepted.
| // ticket:SHA-256) — that remains the accepted ticket-dedup coarseness documented on | ||
| // workKey (bounded, surfaced in the recovery summary, manually recoverable). | ||
| function ticketKey(info: Partial<Pick<SessionInfo, 'topic' | 'description'>>): string | null { | ||
| for (const field of [info.topic, info.description]) { |
There was a problem hiding this comment.
Nit (correctness tradeoff — flagged by both reviewers): The ^ anchor means only the first token of topic/description matches. A topic like "Fix login — BANK-1750" won't extract the ticket, so its sibling "BANK-1750 classifier fix" gets deduped but this one doesn't — two sessions survive.
The tests confirm this is intentional (avoids false positives like UTF-8), and the collapse is lossless (skipped sessions stay recoverable). Worth a brief code comment noting the false-negative tradeoff since it's a common phrasing pattern.
| // finally guarantees release on every exit (success, throw, total-failure); once the | ||
| // survivor's record persists its worktreeBranch, pickSessionName's reservation takes over. | ||
| const reservedName = worktree | ||
| ? (worktree.branch.startsWith('wt/') ? worktree.branch.slice(3).split('-')[0] : deadInfo!.tmuxName) |
There was a problem hiding this comment.
Nit (safety): The deadInfo! non-null assertion is safe because worktree is only truthy when deadInfo?.worktreeRepo && deadInfo.worktreePath — but this coupling is non-obvious. A brief inline note (or a deadInfo! → deadInfo ?? unreachable() guard) would make the invariant explicit.
| // for every session, and each doSpawnSession deletes the dead record before it fails. | ||
| // Bail before touching anything so a broken boot can't erase the whole dead fleet. | ||
| if (!process.env.SPAWN_CWD) { | ||
| process.stderr.write('daemon: auto-recover: SPAWN_CWD unset — skipping to avoid destroying dead records on a broken boot\n') |
There was a problem hiding this comment.
Nit (observability): On a broken boot (SPAWN_CWD unset), this only logs to stderr — no Discord DM or channel notification. Every other auto-recovery outcome posts a summary. The operator may not notice the skip for hours. Consider posting a brief alert via gateway.sendDM or safeSend here too.
34b57da to
cec823a
Compare
|
Addressed all 6 review comments in
Verified: all 3 entry points build, affected suites 100/100, and a 2-round independent review pass (correctness + data-loss) on the delta came back clean — including a check that |
| daemon_commands_global --> daemon_bridge-transport | ||
| daemon_commands_global --> daemon_session-lifecycle | ||
| daemon_commands_global --> daemon_util | ||
| daemon_commands_global --> daemon_pr-watch |
There was a problem hiding this comment.
Should-fix (completeness): This PR adds a global→pr-watch edge, but also adds import { checkUnpushedCommits, reattachWorktree } from '../worktree-manager.js' in global.ts:9. The worktree-manager node is absent from the topology entirely — run bun scripts/gen-topology.ts to regenerate and pick up both new edges (and the missing node if needed). Per CLAUDE.md: "Regenerate after any import change."
|
|
||
| // Surface an auto-recovery message to the operator: DM the access allowlist if set, | ||
| // else fall back to the default channel so the run is visible. | ||
| async function notifyOperator(text: string): Promise<void> { |
There was a problem hiding this comment.
Should-fix (reliability): notifyOperator is async and callers await it, but the DM loop uses void gateway.sendDM(...) — fire-and-forget. The function resolves before any DM lands, so await notifyOperator(...) in postAutoRecoverySummary gives a false sense of sequencing. Either collect promises and await Promise.allSettled(...) so the caller knows DMs were attempted, or drop the async and make it explicitly fire-and-forget (matching the else branch's void safeSend pattern). Current shape is an async function that doesn't actually await its work.
| // stale-cleanup `git branch -D wt/<name>` destroy that preserved branch and any | ||
| // unpushed commits on it. | ||
| for (const s of this.sessions.values()) { | ||
| if (s.worktreeBranch?.startsWith('wt/')) used.add(s.worktreeBranch.slice(3).split('-')[0]) |
There was a problem hiding this comment.
Nit: This s.worktreeBranch.slice(3).split('-')[0] pattern is duplicated in daemon/commands/global.ts:385. If the branch naming convention ever changes, both must update in sync. Consider a small shared helper (e.g. baseNameFromBranch(branch: string): string) to keep it DRY.
| if (existsSync(legacyPlist) && legacyPlist !== dest) { | ||
| try { execSync(`launchctl unload ${shq(legacyPlist)} 2>/dev/null`, { stdio: 'pipe' }) } catch {} | ||
| try { unlinkSync(legacyPlist) } catch {} | ||
| console.log(`removed legacy watchdog (com.hydra.watchdog) — superseded by ${label}`) |
There was a problem hiding this comment.
Nit (correctness): This log line runs unconditionally after the try/catch blocks. If the file exists (per the existsSync guard) but unlinkSync fails (e.g. permissions), the user sees "removed" but the file remains. Consider checking existsSync again after the delete, or moving the log inside a success path.
cec823a to
0ec0b1c
Compare
|
Addressed all 4 in
Verified: all 3 entry points build, affected suites 100/100, import-cycle check clean, and a 2-round independent review pass (correctness + data-loss/regression) on the delta came back clean. |
| // Load-bearing: pickSessionName reserves this token and createWorktree's stale | ||
| // `branch -D wt/<name>` targets it, so recoverOne's name reservation must derive it identically. | ||
| // Callers must first confirm the branch starts with `wt/`. | ||
| export function baseNameFromBranch(branch: string): string { |
There was a problem hiding this comment.
Should-fix (correctness): branch.slice(3).split('-')[0] extracts the base name by splitting on -, but if session names ever contain hyphens (e.g. my-feature → branch wt/my-feature-suffix), this returns "my" instead of "my-feature". That means pickSessionName reserves the wrong token, and a new spawn could draw the unprotected name and branch -D the preserved branch.
If session names are guaranteed hyphen-free (word-list only), this works — but the invariant isn't documented, and worktreeBranchSuffix implies - is the delimiter between name and suffix. Consider either: (a) documenting the no-hyphen invariant on pickSessionName, or (b) using a separator that can't appear in names (e.g. --) so the split is unambiguous.
Flagged by both reviewers.
| // Create worktree | ||
| try { | ||
| await execAsync('git', ['-C', repoDir, 'worktree', 'add', '-b', branchName, wtDir, base], { timeout: 15_000 }) | ||
| process.stderr.write(`daemon: worktree: created ${wtDir} (branch ${branchName}) from ${base}\n`) |
There was a problem hiding this comment.
Nit (correctness): The initial rev-parse --verify branch runs outside withRepoLock. If a concurrent branch -D deletes the branch between this check and the lock acquisition, the unlocked rev-parse succeeds, then worktree add fails inside the lock. The in-lock re-verify (lines 127-132) correctly catches this — good.
However, the reverse race: if the unlocked rev-parse fails due to a concurrent delete, the code returns 'branch-gone' after only a repo-reachability heuristic. A race where the branch is deleted between the branch rev-parse and the --git-dir check would produce a false terminal 'branch-gone', permanently suppressing recovery. Moving the initial rev-parse inside the lock would close this gap cleanly. Low probability in practice since concurrent branch -D on the same branch is rare.
| }> { | ||
| // Reserve keys owned by tmux-alive sessions that are NOT themselves being recovered. | ||
| // Excluding candidates is essential: the manual path's candidate filter is `!isAlive`, | ||
| // so a deadAt-set-but-tmux-alive session is a candidate yet also tmux-alive — without |
There was a problem hiding this comment.
Nit (clarity): Promise.allSettled never sees a rejected status here because the inner .catch() converts every rejection to a fulfillment. Promise.all would behave identically. Not a bug — the sends are attempted and failures logged — but allSettled implies rejection-tolerance that isn't actually exercised. Minor.
| test('empty session → null', () => { | ||
| expect(workKey({})).toBeNull() | ||
| }) | ||
| }) |
There was a problem hiding this comment.
Nit (coverage): workKey is well-tested, but dedupForRecovery itself — the function that picks winners/losers, handles sibling watches, and interacts with live sessions — has no unit tests. That's the higher-risk logic. Consider adding integration tests with a mocked registry for the dedup path (live-key reservation, sibling watch handoff, sort-by-lastActive tiebreaking).
0ec0b1c to
a7b31ae
Compare
|
Addressed all 4 in
Verified: 3 entry points build, |
| export async function reattachWorktree(repoDir: string, worktreePath: string, branch: string): Promise<ReattachResult> { | ||
| // Branch must exist to reattach — otherwise there's nothing to preserve. | ||
| try { | ||
| await execAsync('git', ['-C', repoDir, 'rev-parse', '--verify', branch], { timeout: 5_000 }) |
There was a problem hiding this comment.
[bump] Nit (TOCTOU — prior reviews #3839252297 + #3839992553, no reply): rev-parse --verify branch still runs outside withRepoLock. A concurrent destroyWorktree can delete the branch between this check and the locked worktree add. The inner catch re-verifies and returns branch-gone, so the race is handled — but the outer check can still produce a misleading branch-gone result if the repo is reachable but a concurrent op deleted the branch between the two rev-parse calls (the --git-dir check passes, rev-parse for branch fails → branch-gone even though a lock would have prevented the deletion).
If this is the accepted design (optimization: skip the lock when branch is absent), a one-line comment on the outer rev-parse noting "best-effort pre-check; canonical result determined inside the lock" would close this.
There was a problem hiding this comment.
Fixed definitively in 033cfba (sorry for not replying on the earlier threads — I'd landed the in-lock re-verify but left this initial rev-parse --verify outside the lock). Moved the whole verify→prune→add sequence INSIDE withRepoLock, so no daemon worktree op (createWorktree/destroyWorktree — both lock on the same repoDir) can delete the branch between the check and the add. The internal TOCTOU is now closed; an external git branch -D is still classified correctly by the post-add re-verify.
| } | ||
|
|
||
| const deadInfos = [...registry.values()].filter(info => | ||
| !!info.deadAt |
There was a problem hiding this comment.
Should-fix (completeness — flagged by both reviewers): autoRecoverAfterBoot filters on !!info.deadAt, but after a hard reboot (daemon killed, not graceful shutdown), sessions that were alive at crash time never had deadAt stamped. On the first boot after such a crash, those sessions are in the registry as alive (no deadAt) but their tmux is gone (!tmuxHasSession would be true). They pass the tmux check but fail the deadAt gate, so auto-recovery skips them.
They'd be caught by the session-health timer, marked dead, and recovered on the next boot — but that defeats the "post-reboot" promise for the most common crash scenario.
Consider stamping deadAt on sessions with missing tmux at the top of autoRecoverAfterBoot before filtering, or relaxing the deadAt requirement when !tmuxHasSession is already checked.
There was a problem hiding this comment.
Respectfully, I think this one's already handled — loadPersisted (sessions.ts:427-449) runs in the SessionRegistry constructor (before autoRecoverAfterBoot) and stamps info.deadAt = info.deadAt ?? Date.now() on EVERY tmux-gone non-guest session at boot (only thread_guests are pruned earlier). So a hard-reboot survivor that was alive at crash time has deadAt set by the time the !!info.deadAt gate runs — it's recovered, not skipped. I added a comment on the filter documenting that loadPersisted guarantee since the invariant is non-obvious. Happy to reconsider if you're seeing a path where loadPersisted doesn't stamp it.
| // auto-recover filter (which is thread_owner-only + excludes parked/ephemeral/headless): | ||
| // a user may deliberately recover any dead non-guest session. Codex is excluded — the | ||
| // recoverOne cascade would relaunch it as Claude (codex reconnects via its own path). | ||
| function findDeadSessions(): SessionInfo[] { |
There was a problem hiding this comment.
Nit (clarity): findDeadSessions intentionally does NOT filter on suppressAutoRecover — manual recover should still work on parked/branch-gone sessions. This is correct but non-obvious since autoRecoverAfterBoot does filter on it. A one-line comment like // suppressAutoRecover not checked — manual recovery overrides would prevent a future reader from "fixing" this.
There was a problem hiding this comment.
Done in 033cfba — added: // suppressAutoRecover intentionally NOT checked — it gates only AUTOMATIC boot recovery … an explicit manual \recover` overrides it.`
a7b31ae to
033cfba
Compare
| const deadInfos = [...registry.values()].filter(info => | ||
| // deadAt reliably catches hard-reboot survivors: loadPersisted (sessions.ts, in the registry | ||
| // constructor — runs before this) stamps deadAt on EVERY tmux-gone non-guest session at boot, | ||
| // so a session that was alive at crash time (never gracefully stamped) is already deadAt here. |
There was a problem hiding this comment.
Should-fix (architecture — sp-reviewer): recoverOne + dedupForRecovery + workKey + autoRecoverAfterBoot + notifyOperator + helpers = ~220 new lines in commands/global.ts, plus two new cross-layer imports (worktree-manager core → commands, pr-watch domain → commands). The commands layer was a thin orchestration shell; this PR makes it the widest node in the topology graph.
Structural alternative: extract a daemon/recovery.ts at the lifecycle layer (where session-lifecycle.ts already imports both worktree-manager and pr-watch). handleRecoverIntercept becomes a thin call into it, and autoRecoverAfterBoot moves there entirely. Eliminates the two new edges from commands into core/domain.
There was a problem hiding this comment.
Done in f3e73a8 — extracted the recovery engine (recoverOne cascade, workKey/dedupForRecovery, autoRecoverAfterBoot, notifyOperator, and the manual recover handler) into a new daemon/recovery.ts at the lifecycle layer. commands/global.ts drops ~480 lines and no longer imports worktree-manager or pr-watch — those cross-layer imports now live at the lifecycle layer alongside session-lifecycle. Verified byte-faithful move (mechanical diff shows only the intended skipReason log change) + rewired router/daemon/test importers; topology regenerated.
| clearFactoryIdentity(info) | ||
| info.deadAt = info.deadAt ?? Date.now() | ||
| info.suppressAutoRecover = true | ||
| registry.set(info.sessionId, info) |
There was a problem hiding this comment.
Should-fix (fragility — sp-reviewer): killSession deletes the record, then this block re-inserts the same info object reference with mutated fields. Any field killSession nullified or mutated on the object during cleanup is silently carried into the re-persisted record. Today killSession doesn't mutate fields before registry.delete, but the coupling is implicit — a future killSession change that clears a field before deletion would corrupt these re-persisted records.
Consider a skipRegistryDelete option on killSession (parallel to skipWorktreeDestroy) so the record is never removed in the first place, rather than delete-then-reinsert.
There was a problem hiding this comment.
Fixed in f3e73a8: the sweep now snapshots preserved = {...info} BEFORE killSession and re-persists from that pristine copy, so a future killSession that mutates fields pre-delete can't corrupt the record. One nuance a reviewer caught: killSession's ONE current pre-delete mutation is discovering claudeSessionId from the live pane — so I also carry that single field forward (preserved.claudeSessionId ??= info.claudeSessionId) to keep a later recover's full-context tier-1 resume, while every other field stays pristine.
| // so they run one-at-a-time; different repos still run concurrently. | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| const repoLocks = new Map<string, Promise<unknown>>() |
There was a problem hiding this comment.
Nit (resource — ts-reviewer): repoLocks entries are never removed. Each settled promise is small, but the map pins closed-over values from the last fn call. Low practical risk with a handful of repos, but a one-liner cleanup when the chain settles empty (if (repoLocks.get(repoDir) === settled) repoLocks.delete(repoDir)) would keep it tidy.
There was a problem hiding this comment.
Done in f3e73a8 — withRepoLock now self-deletes a repo's entry once its chain settles, guarded by if (repoLocks.get(repoDir) === tail) so a newer chained op (which owns its own cleanup) is never clobbered. No lost mutual-exclusion.
| skipped: Array<{ info: SessionInfo; reason: string }> | ||
| siblingWatches: Map<string, WatchEntry[]> | ||
| }> { | ||
| // Reserve keys owned by tmux-alive sessions that are NOT themselves being recovered. |
There was a problem hiding this comment.
Nit (observability — ts-reviewer): The outer try {} catch {} around checkUnpushedCommits swallows everything silently, including non-git errors (OOM, TypeError). checkUnpushedCommits already returns -1 for "branch exists but count failed", so this catch handles only truly unexpected errors — a process.stderr.write here would help debuggability without changing control flow.
There was a problem hiding this comment.
Done in f3e73a8 — skipReason's catch now logs to stderr (checkUnpushedCommits already maps expected git failures to -1, so a throw here is genuinely unexpected) and still falls through to the base reason.
Revive dead worker sessions on daemon boot (opt-in HYDRA_AUTO_RECOVER=1) without losing their dashboard links or unpushed worktree commits. - Lossless respawn: carry artifacts/contextLinks/description across the replace; recovery kills pass skipWorktreeDestroy and reuse the on-disk worktree in place (reattach if the dir was pruned; distinguish a gone branch from a transiently-unreachable repo). - Auto worker-recovery at boot: read dead thread_owner workers, dedup by the real work they own (PR-from-artifacts -> leading ticket, both coarse but lossless), revive via resume->fork->respawn, DM a summary. Idempotent and single-flight; excludes factory/codex/parked/ephemeral/headless. - Preserved-branch protection: reserve the branch BASE name (not tmuxName, which diverges after a prior recovery) across the cascade; per-repo worktree-op serialization; checkUnpushedCommits fail-safe (-1 = unknown). - Cross-subsystem guards so a preserved branch is never destroyed on a timer/boot path: phase-budget reaper and factory sweep both skip dead records; factory sweep preserves a mid-build builder's branch + watches; pollPr freezes a dead-owner watch (no cursor advance) until revival; terminal branch-gone records drop their watches. - suppressAutoRecover clears when a live session replies (an adopted awaiting_pm builder stays auto-recoverable). - Re-verify guard injected into every recovered session (post-crash state is unknown before any write/push/deploy). - hydra install supersedes the legacy com.hydra.watchdog plist. Config-dir-agnostic; the ~/.claude reconciliation is a per-machine ops step, called out in the PR description. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
033cfba to
f3e73a8
Compare
| // | ||
| // Manual `recover [name]` (handleRecoverIntercept, router-dispatched) and automatic post-reboot | ||
| // recovery (autoRecoverAfterBoot, boot-wired in daemon.ts) share the recoverOne cascade, | ||
| // work-key dedup, and the single-flight guard here. |
There was a problem hiding this comment.
Nit (architecture — flagged by sp-reviewer): recovery.ts sits at the lifecycle layer but imports two domain-layer modules: access.ts (line 7, loadAccess) and pr-watch.ts (line 4, parsePrUrl/getWatchesBySession/restoreWatches/unwatchBySession). These are downward-layer edges.
Consistent with the existing session-lifecycle → access and session-lifecycle → pr-watch edges on main, so not blocking — but this PR doubles the lifecycle→domain coupling. If you're ever looking to tighten the topology: the loadAccess call could be injected from the caller in daemon.ts (entry layer), and pr-watch could be promoted to lifecycle since both lifecycle modules now depend on it.
There was a problem hiding this comment.
Agreed it's a nit, and I'm going to leave it as-is. Both edges are inherent to the recovery engine: loadAccess selects the DM recipients for the post-recovery summary, and the pr-watch helpers (getWatchesBySession/restoreWatches/unwatchBySession/parsePrUrl) are the watch snapshot/restore/dedup that make recovery lossless. They mirror the existing session-lifecycle → access/→ pr-watch edges (recovery is a sibling of session-lifecycle at the lifecycle layer), so this moves the imports out of the commands layer (worse) rather than creating a new class of edge. Eliminating them would need DI or relayering pr-watch/access — more churn than the nit warrants. Happy to revisit if you later want a formal lifecycle↛domain rule enforced.
2 participants
Problem
On the 2026-08-19 laptop reboot: the daemon/bridge came back, but the
slack-bytetmux session did not stay up and all 12 live worker sessions died with no automatic recovery. Manual respawn brought them back but lost each session's dashboard links (PR / claude.ai artifact URLs).Root cause (verified against current code)
A. slack-byte didn't restart — config split + crash-loop. The loaded launchd job
com.hydra.watchdogruns the deprecatedwatchdog.shwithCLAUDE_CONFIG_DIR=~/.claude-byte, while the daemon is run manually with~/.claude. The.claude-bytebyte crash-loops (weeks ofBot session 'slack-byte' missing … revivingevery 120s). The maintained CLI path (hydra up/watchdog/install,cli/lifecycle.ts) already fixes the model/auth bugs the deprecatedstart-byte.shhas — it just wasn't installed, andhydra installwrites a differently-labeled plist that would coexist with the legacy one.B. Workers weren't recovered; manual respawn lost links.
loadPersistedkeeps dead records (withartifacts[]) — it stampsdeadAt, doesn't prune — but nothing revives them at boot. And respawning onto a dead thread rankillSession, whichregistry.deleted the record (droppingartifacts[]) anddestroyWorktreed the on-disk worktree.Fix (repo code — helps every user)
session-lifecycle.ts):doSpawnSessionsnapshots the replaced record'sartifacts/contextLinks/descriptionbeforekillSessionand re-applies them to the new record (+ resetsartifactsBackfilledso the boot history-rescan self-heals anything missed). Fixes manualrecover/respawntoo.session-lifecycle.ts,worktree-manager.ts):killSession(…, {skipWorktreeDestroy}); recovery reuses the dead session's on-disk worktree in place (keeps unpushed work, and lets--resumefind the transcript under the same CWD). If the dir is gone but the branch survives,reattachWorktreere-materializes it (distinguishing a pruned branch from a transiently-unreachable repo, so a boot-time hiccup never orphans a branch). The preserved branch is fenced against a concurrent spawn's stalegit branch -Dby reserving its base name across the whole cascade — keyed off the worktree branch, not the tmux name, since a re-recovered session carries a fresh tmux name but its original branch. Same-repo worktree ops are serialized (withRepoLock). Two cross-subsystem boot/timer paths that would otherwise destroy a preserved branch are closed the same way:phase-budget.ts) is gated on liveness — it never arms for or reaps a dead record (doing so wouldkillSessionwithoutskipWorktreeDestroy~5 min after boot).factory.tssweepOrphanedBuilders) now preserves a swept mid-build builder's worktree branch when it holds unpushed (or unverifiable) commits — re-persisting a dead,suppressAutoRecoverthread_ownerrecord (sopickSessionNamekeeps reserving the branch name and it staysrecover-able) and snapshotting/restoring its PR watches (cursors intact), instead ofbranch -D-ing the branch and dropping the watches. (Factory builders are build sessions with worktrees too; a reboot mid-building/reviewingpreviously discarded their local commits.)daemon/recovery.ts, wired indaemon.ts): opt-inHYDRA_AUTO_RECOVER=1. Reads dead workers, dedups by real work (PR/ticket), not per-thread (a naive per-thread respawn can put two sessions on one PR), revives each via the existing resume→fork→respawn cascade, then DMs arecovered N / skipped Msummary so duplicates are easy to spot/kill. Idempotent (skips already-live tmux; shares therecoveryInProgressguard). The recovery engine (recoverOnecascade, work-key dedup,autoRecoverAfterBoot, and the manualrecoverhandler) lives in its owndaemon/recovery.tsat the lifecycle layer — keeping the commands layer a thin shell and the worktree-manager/pr-watch imports out of it.hydra installsupersedes the legacy watchdog (cli/lifecycle.ts): removescom.hydra.watchdogso there's a single watchdog, not two fighting.Ops runbook (per-machine — not repo code)
CLAUDE_CONFIG_DIRlives in each machine's~/.claude/channels/<platform>/.env, not the repo. To make slack-byte restart reliably on reboot on Kevin's machine: reconcile everything to~/.claude(the config the working manual daemon already uses), thenhydra install slack(installs the CLI watchdog + removes the legacy one). Note for other users: pick whichever single config dir your daemon actually runs under — recovery--resumeneeds the transcript under that same dir.Safety
SPAWN_CWDunset), auto-recovery bails before touching any record (so it can't erase the dead fleet) and DMs the operator that it skipped — no silent stderr-only skip.pr-watches.json; a recovered session's PR watches (with their seen-cursors) are snapshotted before the kill and restored onto the survivor. A watch whose owner record is present-but-dead is frozen (not polled) until the owner is revived, so the seen-cursors aren't advanced past feedback that no live agent consumed — the revived session re-notifies from the as-of-death cursor. A terminal dead record (branch gone → never revived) drops its watches instead of freezing them forever.suppressAutoRecover(the flag that keeps a parkedawaiting_pmbuilder / branch-gone / mid-build-preserved record from auto-reviving) clears itself the moment a live session produces a reply — so anawaiting_pmbuilder adopted as a working thread stays auto-recoverable across a later reboot, while dead suppressed records (which never reply) keep their suppression.preserveWorktreeflag that only the recovery path sets — the sharedtryResumehelper is also used by the manualresumecommand, which keeps its prior destroy-and-respawn semantics (no behavior change forresume).Testing
tsc --noEmit: 0 errors in any file this PR touches; unrelated pre-existing errors unchanged.recovery-dedup.test.ts(14 cases):workKeyPR/ticket keying (the "two sessions on one PR" regression),dedupForRecoverywinner/loser + most-recently-active tiebreak + null-key uniqueness + sibling-watch handoff (cursors preserved), and aSESSION_CATALOGhyphen-free invariant guardingbaseNameFromBranch.bunfig.toml[test] preloadpointsHYDRA_STATE_DIRat a throwaway temp dir, so suites that import daemon modules never read or clobber the developer's live~/.claudestate.protocol-scenarios/protocol-notifications/build/review harness tests (bun-vs-jest timer gap); they fail identically on plainmainand are untouched by this PR.hydra restart slackby the operator will exercise the boot path.