daemon stability wave 2: health you can see, someone else's Mac - #130
Merged
Merged
Conversation
…e can't cause a duplicate delivery (S096)
…launchd-frozen snapshot (S046)
…d of a phantom record (S051)
…can't surface as a failed post (S052)
… and report truncation at 100 files (S053, S086)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s on refresh (S057)
…-043/S050/S054/S083-085/S092)
…/gitq/deck) on a same-named PATH collision (S066)
…e predicate (S075)
…ULL-keyed presence row (S076)
…ry retire, not just createTree's path (S078)
…t + supervision design
… a configured root that is an ancestor of the repo (S079)
… (S054) api-server captured the token once at boot while the secrets handler called loadOrCreateApiToken() fresh on every request; an external rotation or an unwritable token dir left the two permanently disagreeing about the current token. getApiToken/reloadApiToken share one in-memory cache between both consumers, and a persist failure now logs a warning instead of failing silently.
…pid after binds Boot failures on the prod path used to leave a live-pid zombie: a stale rt.pid could get written before the socket/API binds even attempted, so a failed boot exited without ever removing it. runDaemon() now wraps its body in try/catch (log.fatal + flush + exit 1 on any failure), and rt.pid is only written once both servers.socket and servers.api are assigned. installCrashHandlers gains an opts.booting predicate: unhandledRejection is fatal + exit 1 while true (boot phase, nothing worth staying up for), and logs only (today's behavior) once bootPhase flips to "ready" right before "daemon ready".
…rly instead of racing a retry into a second pane (S087)
…k that the sync path enforces (S088)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review fix (round 1): runDaemon() now catches and exit(1)s internally on boot failure, so startDaemon()'s outer try/catch never runs, and its JSDoc claiming otherwise was misleading. Replaced with a one-line comment stating the real constraint.
…eplenish don't race git ref locks (S089)
…t (S005/S006/S040/S041/S084)
…per-chunk, so a split multibyte char survives (S095)
…scope side effect Hoists redirectNativeStderr() to the first executable statement and installCrashHandlers() to right after the logger resolves, both before createEventsBus, cron, sweep timers, and home-snapshot construction. A pre-startDaemon throw (e.g. a corrupt events.db) now lands in daemon-stderr.log instead of vanishing down a discarded fd 2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…linked or case-variant root matches lsof (S097)
…es (R043) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…069 part 1) Also updates the two other BranchCacheStore implementers (lib/daemon.ts's delegating facade, fake-cache-store.ts's test double) and one exact-key-set test assertion so tsc stays clean; store PK/upsert/delete/gc behavior is unchanged.
…fence (S069 part 1) get/getByBranch on the BranchCacheStore interface forced an edit to lib/daemon.ts's cache facade, which is under a write fence owned by the p2-health lane. Drop get entirely (Task 10 consumers will use entries[composeKey(identity, branch)] directly); reshape getByBranch into a free function over an entries map instead of a store method, so the interface -- and daemon.ts's facade -- do not change. Reverts the daemon.ts and fake-cache-store.ts edits from the prior commit.
…-now items Applies the final whole-branch review's approved fix wave for the rt daemon health feature: maxLagMs now decays as a windowed max instead of a lifetime high-water mark (and currentlyStalled drops its redundant maxLagMs OR-leg), the health degraded threshold for event-loop lag is named instead of hardcoded, one branch-introduced em dash is fixed, a process-citation is dropped from a comment, the daemon log-level resolver validates against pino's level set, and the heartbeat reader now shape-guards against a partial-but-valid JSON object.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…consumers (S069) Consumers updated: store put (keys off entry.repoName), enrich (cold-start sets identity from remoteUrl), notifier (composite fired-state), worktree-reconciler (branchOf/mrKey), freshness (composeKey lookups), handlers/cache (bare-branch read contract + optional repoIdentity), handlers/system-processes and handlers/worktree (repo-scoped lookups), status/data (branchOf display). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…or/identity check Two earlier-committed behavior changes broke four consumer test fixtures: the dev-mode wrapper detector now requires a real marker (# mattstack-dev-mode or RT_LAUNCH_CWD) instead of treating any file's presence as dev mode, and commitInitialUserRepo now checks git identity before committing. Production code is unchanged; fixtures now plant a recognized wrapper and answer the git config user.name/email probes, matching the pattern already used in home-snapshot.test.ts's defaultResponders.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 21 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 72 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (93)
Comment |
m4ttheweric
added a commit
that referenced
this pull request
Sep 17, 2026
* agent-herdr: resolve herdr via PATH probe instead of hardcoded ~/.local/bin (S002) * notifier: retry the post-push queue removal so a busy-swallowed delete can't cause a duplicate delivery (S096) * cron: pass the daemon's resolved process.env to spawn instead of the launchd-frozen snapshot (S046) * agent handler: return ok:false when herdr dedups the tab label instead of a phantom record (S051) * chat handlers: guard emit/notify after the message commit so a throw can't surface as a failed post (S052) * discussions:diffs: bound the GitLab fetch with a timeout+abort signal and report truncation at 100 files (S053, S086) * plan: Phase 0 honest-supervision implementation plan Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * hooks-guard: handle fs.watch error events and reconcile stale watchers on refresh (S057) * plan: Phase 3 trust-boundary implementation plan (S005/S006/S010/S040-043/S050/S054/S083-085/S092) * deps links: never auto-unlink a DEFAULT_EXPOSED tool (rt/fast-browser/gitq/deck) on a same-named PATH collision (S066) * age-key: give the keychain spawn a bounded, distinguishable timeout (S070) * presence-store: honor the tail heartbeat in the offline rule and prune predicate (S075) * chat:sign-in: reject a missing/empty sessionId instead of storing a NULL-keyed presence row (S076) * worktree trash: exclude .worktrees/ from the repo's git status on every retire, not just createTree's path (S078) * docs: sketch daemon supervision verdicts + exit-code semantics * docs: retire stale daemon-runner-health.md, point at the current audit + supervision design * daemon: cap request body size at 1 MiB on both servers (S092) * worktree trash reap: require a plausible rt-written epoch, and refuse a configured root that is an ancestor of the repo (S079) * daemon: one shared api-token cache for api-server and secrets handler (S054) api-server captured the token once at boot while the secrets handler called loadOrCreateApiToken() fresh on every request; an external rotation or an unwritable token dir left the two permanently disagreeing about the current token. getApiToken/reloadApiToken share one in-memory cache between both consumers, and a persist failure now logs a warning instead of failing silently. * daemon: boot failure is fatal (exit 1), gated by boot-phase flag; rt.pid after binds Boot failures on the prod path used to leave a live-pid zombie: a stale rt.pid could get written before the socket/API binds even attempted, so a failed boot exited without ever removing it. runDaemon() now wraps its body in try/catch (log.fatal + flush + exit 1 on any failure), and rt.pid is only written once both servers.socket and servers.api are assigned. installCrashHandlers gains an opts.booting predicate: unhandledRejection is fatal + exit 1 while true (boot phase, nothing worth staying up for), and logs only (today's behavior) once bootPhase flips to "ready" right before "daemon ready". * pane:spawn: check the caller's abort signal between steps and stop early instead of racing a retry into a second pane (S087) * mr:by-branch: apply the same demand-scope gate to the forge write-back that the sync path enforces (S088) * plan: Phase 1 event-loop implementation plan (RT-78, items 1.1-1.5) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * daemon: drop startDaemon's dead-code catch and its now-stale JSDoc Review fix (round 1): runDaemon() now catches and exit(1)s internally on boot failure, so startDaemon()'s outer try/catch never runs, and its JSDoc claiming otherwise was misleading. Replaced with a one-line comment stating the real constraint. * worktree create: serialize createTree per repoPath so provision and replenish don't race git ref locks (S089) * daemon: rt.trustedBrowserOrigins allowlist + needsToken invert-default (S005/S006/S040/S041/S084) * herdr client: accumulate raw socket bytes and decode once instead of per-chunk, so a split multibyte char survives (S095) * daemon: install stderr redirect + crash handlers before every module-scope side effect Hoists redirectNativeStderr() to the first executable statement and installCrashHandlers() to right after the logger resolves, both before createEventsBus, cron, sweep timers, and home-snapshot construction. A pre-startDaemon throw (e.g. a corrupt events.db) now lands in daemon-stderr.log instead of vanishing down a discarded fd 2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * port-scanner: canonicalize repo/worktree paths once per scan so a symlinked or case-variant root matches lsof (S097) * rt-paths: only migrate a legacy ~/.rt that carries an actual rt signature (S099) * daemon: fix needsToken to never gate OPTIONS preflight (review fix) Splitting OPTIONS out of the GET/HEAD bucket restores the pre-existing guarantee that a CORS preflight is never gated, regardless of path, matching this file's own docblock and existing test coverage. * daemon-client: return timed-out/refused attribution per query instead of shared module flags (S081) * daemon: default-deny CORS and gate /ws on origin/token (S005/S006) * runCapture: race reads against the deadline so a pipe-holding grandchild can't wedge it (S023, S024) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * daemon-client: bounded-poll rt.sock after a restart instead of one 300ms retry (S082) * runs prune: reap expired run dirs with a detached rm -rf, never a sync recursive rmSync (S100) * daemon: broadcast() drops dead/backpressured ws clients instead of silently dropping frames (S042) * worktree reconciler: queue a follow-up pass for a kick arriving after the loop has started, instead of dropping it (S065) * events.db: corruption quarantine + busy_timeout/synchronous pragmas; guard sweep timers createEventsBus now mirrors state/db.ts's corruption quarantine (rename to events.db.corrupt-<ISO> + -wal/-shm sidecars, warn, recreate empty) and sets busy_timeout=250 / synchronous=NORMAL on open. events.db is a bounded-retention journal, so total loss on quarantine is harmless. Added safeInterval/safeTimeout (lib/daemon/safe-timers.ts) and wrapped the two eventsBus.sweep() timers in lib/daemon.ts with them, so a synchronous sqlite throw mid-tick (e.g. SQLITE_FULL) warns instead of becoming an uncaughtException that exits the daemon. Also fixes the near-vacuous e2e assertion carried from Task 3: the corrupt-events.db test now asserts the quarantine file exists AND the daemon actually boots and serves (a live rt events emit round trip), instead of existsSync(stderrLog) || quarantined, which was always true. * runCapture: stop clearing the SIGKILL timer before it fires, strengthen SIGKILL test Review found the finally block cleared killTimer in the same tick the deadline promise resolved, so a SIGTERM-ignoring child was never actually SIGKILLed even though runCapture's own promise settled on time. Leave killTimer running past the finally; it is already try/catch guarded so it is a no-op once the child has exited. Also capture and clear the deadline promise's own timer handle so it doesn't outlive the fast path. Strengthened the SIGKILL escalation test to assert the child process is actually gone (via its recorded pid) rather than only asserting runCapture's promise resolved on schedule, since the old assertion passed regardless of whether the kill worked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chat:post/chat:dm: validate room/handle/body shape and refuse an unknown room instead of a silent black hole (R010) * daemon: pathParam() helper -- malformed %-encoding is a 400, not a logged 500 (S083) Adds a shared pathParam() decode helper and wires it into the three parameterized routes (/api/cache/:branch, /api/hooks/:repo/repair, /api/runs/:repo/:runId), replacing each route's hand-rolled decodeURIComponent. A malformed path segment now returns a clean 400 instead of falling through to the outer catch's logged 500 (cache and hooks routes) or the generic 404 (runs route). * validate chat:join wakeOn and agent:start/resume surface against their enums (R033) * source-guards: update the boot-failure-exits guard for runDaemon owning the catch Review fix (round 2): the guard tested startDaemon's body for catch + process.exit(1), which fix-round-1 made stale by moving the catch-and-exit into runDaemon itself (startDaemon is now a thin await runDaemon()). Rewrote the guard to assert the invariant at its real location: runDaemon's body still owns catch + process.exit(1), and startDaemon just awaits it. Fixed the stale "startDaemon is a thin catch-and-exit wrapper" comment in the neighboring test to match. * git-async: 5-min timeout for checkout/merge/stash/status so a large-repo checkout isn't killed half-applied (S104) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chat:read/chat:messages: clamp limit into [1,500] instead of reaching SQLite's unlimited negative LIMIT (R034) * daemon: coerce REST GET query params to number/boolean at the seam (S085) * git-async: bump the 3 inline runGit(status/stash pop) call sites to MUTATING_TIMEOUT_MS Closes the S104 gap left by fd935d7: freshenOne's stash-pop reapply and autoReturnMain's two status --porcelain checks called runGit directly rather than through the helpers, so they were still on the 60s default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * discussions-diffs test: fix fetch cast to satisfy tsc * daemon: log the EADDRINUSE port holder and throw a typed ApiPortInUseError * cache-refresh: async git + grant-gated doppler loop; add listWorktreeRootsAsync (S008, S045, S021) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * daemon: bind API before socket; register rt.apiPort setting + resolveApiPort() Binding the API server first means a fatal API-bind failure never strands a socket-bound zombie behind it. rt.apiPort is the escape-hatch setting the api-server sibling consumes at bind time; resolveApiPort() resolves it lazily (env > setting > 9401) without disturbing the existing API_PORT const api-server.ts already imports. * daemon: standalone git-ref validator for option-injection guard (worktree.ts wiring documented, not wired here) * daemon: standalone credential-redaction utility (freshness.ts wiring documented, not wired here) * freshness: async, cached getRemoteUrl via runCapture (R032) getRemoteUrl used execSync on the daemon thread inside every forge handler and the freshness reconcile loop. Switch to runCapture (5s timeout) and cache per repoPath for the process lifetime, since remotes rarely change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * worktree-process-kill: async lsof/ps via runCapture (S015, S016) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * home-snapshot: lazy daemon-flavored state.db; getStateDb re-applies a stronger flavor timeout startHomeSnapshot no longer resolves getStateDb() eagerly at construction with the default cli flavor (5000ms busy_timeout). db access is a thunk (resolveDb) defaulting to getStateDb("daemon"), first invoked inside init() after its await (past module-scope construction, so the daemon's own openBranchCacheStore() opens the singleton daemon-flavored first). getStateDb() also now re-applies PRAGMA busy_timeout when a caller requests a stronger (shorter) flavor than the already-open singleton holds, hardening against any other future eager-cli-then-daemon ordering bug. * docs: the :9401 trust boundary model and the S010/S050/S043 sibling wiring notes * repo-index: async observed-main-path on the endpoint:claim resolve path (S098) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix rt-paths lint and registry test regressions on job/p3-trust-boundary redact-credentials.ts: reword the doc comment so it no longer spells out a literal .rt-prefixed path (rt-paths.test.ts bans that outside lib/rt-paths.ts); the rationale is unchanged. registry.test.ts: add the rt.trustedBrowserOrigins key (introduced by an earlier task on this branch) to the expected suiteKeys list and bump the length assertion from 42 to 43. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * state: isBusyError matches SQLITE_BUSY_*; read-then-write daemon txns use BEGIN IMMEDIATE joinRoom/archiveRoom/readUnread (chat-store.ts), dmRoomFor (dm-store.ts), and drainNotificationQueue (notifier-store.ts) read before they write inside a db.transaction; bun:sqlite's plain transaction() defers BEGIN, so a concurrent writer can produce SQLITE_BUSY_SNAPSHOT that busy_timeout cannot absorb. .immediate() takes the write lock up front instead. presence-store.ts:signIn has the same shape but is a sibling-owned write-fence file; left untouched as a documented follow-up. * repo-index: fix write-back regression in resolveIndexPathForIdentity setIndexPath is a bare KV write; it skipped the moved-repo guard, the repos.json compat mirror, and the unopenable-db degrade that writeIndexRow (and thus updateRepoIndex) provides. Swap to writeIndexRow, which takes the already-resolved async main path and preserves all three, with no sync git reintroduced. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * gate: fail on sync-exec anywhere in the daemon import graph (1.3) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * state.db: isolate each legacy importer in a SAVEPOINT so one bad file cannot wedge migration A throwing legacy importer previously rolled back the whole v0->v1 migration, leaving user_version at 0 and repeating the identical throw on every later openStateDb call (daemon boot + every CLI command) with no self-heal. Wrap each LEGACY_IMPORTS entry in its own SAVEPOINT so one importer's throw only rolls back its own writes; warn with the file and error, still push it to consumed so it gets renamed .migrated, and let the schema DDL and every other importer land and reach SCHEMA_VERSION. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * daemon: add supervision-state (boot attempts, failures, last-exit) kv + breadcrumb * refresh: whole-cycle deadline clears the coalesce latch; cap RepoWatch.pending (S007) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * daemon: wire supervision-state breadcrumbs + kv records into boot/shutdown * fix(refresh): clear the deadline timer on every settle path, not just on wedge Promise.race never cancels the losing branch. When impl won (every successful refresh), the deadline setTimeout still fired ~4 min later and called onTimeout, logging a spurious "cache refresh timed out" warn and leaking a ref'd timer per cycle. Capture the timer handle and clearTimeout it in the shared .finally, which runs on every settle path; clearing an already-fired timer is a no-op. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * freshness: rebuild the provider cache when gitlabToken rotates (S048, S049) Key the providers map on a token fingerprint so a rotated gitlabToken rebuilds the GitLabProvider on the next ensureProvider/getRepoContext call instead of waiting for a daemon restart. A token mismatch drops the stale watch and resets the userIdResolved latch so the next reconcile re-authenticates. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * daemon: lazy-resolve trusted-origin allowlist, drop finding-ID comments Fix A: getTrustedBrowserOrigins() did a synchronous settings-store read on every single :9401 request, even the ~100% with no Origin header (CLI, tray, rt-client from Bun/Node) that never needed it. Added resolveOriginTrust() in api-auth.ts, which only resolves the allowlist when an Origin header is present, and wired both api-server.ts call sites (the /ws gate and the CORS/token gate) through it. Fix B: stripped leftover audit finding-ID citations (S005/S006/S040/ S041/S042/S054/S083/S085) from production comments in api-auth.ts and api-server.ts, keeping the underlying technical rationale. Fix C: reworded two stale "CORS is *" comments to describe the actual current default-deny CORS model (a trusted Origin gets its response echoed back; the local token, not CORS, is what stops a page from firing a mutating request in the first place). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: name the S084 GET-route known-gap ruling in the trust-boundary note * freshness: close the two remaining stale-token paths (S048/S049 review fix) getRepoContext served a cached provider without checking its token, so poll-mode repos and already-cached forge-handler providers kept a rotated token forever. reconcileFreshnessImpl skipped ensureProvider entirely for repos with a live watch, so a running watcher never rebuilt after rotation either. Both now compare the cached token against loadSecrets() before reuse and drop the stale watch/provider on a mismatch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * system-process-scanner: a failed lsof preserves runaway windows (S061) gather() returned [] on both a real empty scan and an lsof/ps failure, so scan() pruned every tracked pid on a transient failure, resetting firstSeen and the runaway sample window. getAllRepoPids and gather now return null on scan failure (distinct from an empty result), and scan/refresh early-return the prior lastResult before touching tracked state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * runs: mtime-memoize finished-run summaries; back off herdr probe (S101, S038, S039) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * daemon status: alive-not-serving / parked / boot-failed / crash-looping verdicts rt daemon status and the ping handler now read Task 9's breadcrumb file and supervision kv to distinguish a live-but-not-serving daemon (booting/wedged/ quarantined), a flavor standoff (parked), a single boot failure, and a crash loop, on top of the existing not-installed/running/degraded/not-running verdicts. pidAlive uses lsof scoped to RT_DIR instead of a system-wide pgrep, since the brief's suggested pgrep pattern false-positives against any real rt daemon running under a different HOME on the same machine. * runs: scope summary-cache key by runsRoot; poller: test the backoff recovery path Coordinator review follow-up on task 11: the mtime cache key could theoretically collide across different RT_RUNS_ROOT values sharing a process, and the backoff's consecutiveFailures reset on a successful probe had no test proving it re-enables per-tick probing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pollers: gate the 10s/30s scans on recent consumer demand (S058, S093) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * daemon status: exclude the calling process from probePidAlive's lsof fallback readSupervisionState() opens a bun:sqlite handle on state.db (inside RT_DIR) right before probePidAlive runs, so lsof +D RT_DIR was reporting the calling CLI process itself as a live holder of the directory -- a dead daemon with no rt.pid and no live breadcrumb pid could self-match and misclassify as alive-not-serving/parked. Filter process.pid out of the lsof result and add a regression test that opens state.db and asserts the fallback returns false with no daemon-related pids present. * demand-tracker: document command-only demand-stamp contract; subprocess: unref SIGKILL timer Notes the reviewer-flagged gap that WS/SSE topic subscriptions don't stamp demand, only wrapped command handlers do. Unrefs the belt-and-suspenders SIGKILL timer in runCapture so a timed-out call can't hold a short-lived rt-client CLI process open for up to 2s; the daemon still delivers the SIGKILL since it stays alive. Applied identically to lib/subprocess.ts and its rt-client mirror, and rebuilt rt-client's dist/ to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * logs: rotate daemon-stderr.log on open; hide stale crash block by mtime daemon-stderr.log grew unbounded and rt daemon logs always showed its stale contents as "most recent crash". redirectNativeStderr now renames a non-empty file to a dated daemon-stderr.<yyyy-MM-dd>.log (deduped with a .N suffix) before reopening, matching log-janitor's LOG_FILE_PATTERN so pruneLogs sweeps it for free. showLogs now gates the native-stderr block on mtime vs. the running daemon's startedAt (from ping), via a new pure nativeStderrDisplay helper, and stamps the mtime in the header. * daemon: bare-signal exit is non-zero (launchd respawns); shutdown verb stays exit 0 launchd's KeepAlive.SuccessfulExit:false only respawns on a non-zero exit. Reserve exit 0 for the intentional shutdown verb; a bare SIGTERM/SIGINT/SIGHUP (external kill, memory pressure) now exits 1 so launchd relaunches. The sanctioned stop path (SMAppService.unregister) never goes through this signal path, so this does not affect intended stops. Refactors installSignalHandlers' inline gracefulExit into a testable makeGracefulExit(deps) that injects exit/recordCleanExit/wasVerbShutdown. * daemon: ownership-aware socket/pid unlink; eviction waits for pid death then SIGKILL * daemon CLI: uninstall guards on liveness; start escalates to kickstart; attemptRestart re-probes uninstall() no longer deletes rt.sock/rt.pid/daemon.json when a failed or absent tray /daemon/stop leaves the daemon actually still alive (isDaemonProcessRunning() or probeSocketHolder() says so) — it now prints the real remedy (launchctl bootout) and leaves the files in place instead of orphaning a live daemon. start() now escalates to the /daemon/restart (kickstart) route when the tray acks /daemon/start but the socket never comes up through the existing poll, since SMAppService can register a job that never actually launches. attemptRestart() in lib/daemon-client.ts re-probes isDaemonRunning() after the tray ack instead of trusting the POST response alone, so daemonQuery's retry logic stops treating a merely-accepted request as a real restart. Audited commands/settings.ts's dev-mode toggle for the same gap: it never calls cleanupDaemonFiles()/markDaemonUninstalled() at all (it goes through the tray's /flavor/retire route instead), so there's nothing to fix there. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * daemon: redirect stderr after legacy-dir migration; set shuttingDownViaVerb at verb receipt redirectNativeStderr() ran before migrateLegacyRtDir(), so its mkdirSync of the new rt dir made every real ~/.rt migration report a false "conflict". Move it after the migration check, still ahead of every other module-scope side effect. shuttingDownViaVerb was only set inside the shutdown verb's 100ms delayed cleanup, so a bare SIGTERM arriving in that window read it as unset and exited 1, causing launchd to respawn a daemon that was told to stop. Set it at verb receipt instead. * daemon: drop unused clearBreadcrumb Production only ever writes or reads the boot breadcrumb (it gets overwritten with "ready" on successful boot); clearBreadcrumb had no callers outside its own test. Removed the export and the now-unused unlinkSync import; the test that only covered clearBreadcrumb is gone, and the remaining "no breadcrumb written" test does its own file cleanup instead of relying on the removed API. * rt-client: soften the rt.apiPort description to match reality resolveApiPort() has no callers yet -- api-server.ts still binds the API_PORT const -- so the setting currently does nothing when set. The old description promised it as an "escape hatch when 9401 is held", which is not true until the sibling wiring lands. Rebuilt dist/ (not committed, gitignored). * docs: scrub em/en dashes, fix daemon-supervision-design phase-order drift Owner rule forbids em/en dashes in committed text. Rephrased them out of daemon-supervision-design.md and the runner-health redirect title; mechanical ellipsis substitution for the large historical plan doc, where careful rephrase wasn't warranted. Also fixed two places daemon-supervision-design.md had drifted from the implementation: the breadcrumb BootPhase order is actually start -> events-db -> state-db -> api -> socket -> ready (no "crash-handlers" phase, API binds before socket), and the alive-not-serving liveness fallback is `lsof +D RT_DIR` scoped to this HOME and filtered to exclude the caller's own pid, not a system-wide `pgrep -f`. * chore: scrub em/en dashes from branch-added comments and strings (owner rule) * I5: wire the seams the four lanes documented but could not cross (a) daemon.ts catches ApiPortInUseError from startApiServer and parks with exponential backoff (withApiPortParkRetry); api-server.ts binds via resolveApiPort() instead of the hardcoded API_PORT constant. (b) S055: handlers/status.ts's "repos" command uses listWorktreesAsync instead of the sync execSync-based listWorktrees; git-worktrees.ts and handlers/status.ts drop out of the no-daemon-sync-exec allowlist. (c) S010: worktree:provision validates the resolved branch with validateGitRef before any runGit call (covers divergence() too). (d) S050: freshness.ts's three remote-URL log/error interpolations run through redactCredentials. (e) S022: resolveUserIdAcrossTracking resolves userId for any branches/project-mrs tracked repo regardless of mode, called from cache-refresh.ts before checkAndNotify so poll-only users get self-authored-transition notifications from cycle 1. (f) S073: presence-store.ts's signIn transaction uses .immediate(), matching the chat-store/dm-store/notifier-store siblings. (g) rt.apiPort's settings description now reflects that it's wired. * e2e: rewrite the 3 API-port-squat daemon tests for I5(a)'s park-retry They asserted the pre-fix crash-on-EADDRINUSE behavior (fatal exit, boot-failed/crash-looping). I5(a) makes this recoverable instead: withApiPortParkRetry parks and retries with backoff rather than crashing, so the daemon now boots successfully once the squatted port frees. Rewritten to assert the new contract: alive (not exited) while parked, no rt.sock/rt.pid until bind succeeds, status never falsely reports "running" while parked, and both recover once the port frees. * spec: p2-health daemon health model design Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * spec: p6-portability design (Phase 6 portability) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * spec: fix degraded-vs-alive-not-serving branch, pin thresholds, scope R012 watcher-close out Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * spec: apply reviewer edits (S069 read contract + 3 sites, bounded-prefix wrapper read, interactive PATH overlay, S071/doppler notes) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * plan: p2-health implementation plan (16 TDD tasks) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * plan: p6-portability implementation plan (10 tasks) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * plan: apply reviewer fixes (loop-monitor test/alloc, CacheRefresherDeps widen, seq in ping, second fetch tag) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * add lib/daemon/health.ts: pure computeHealth + thresholds * add lib/daemon/heartbeat-file.ts: atomic-rename heartbeat * plan: apply reviewer fixes (bounded read in links.ts, widen disabledReason, overlay garbage warn, sops spawn-injection test) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * add lib/daemon/loop-monitor.ts: drift monitor + heartbeat cadence * rt-client: register rt.daemonPath machine setting (6.1) * daemon-status: heartbeat-stale 'stalled' detail + degraded eventLoop * daemon status: render health/stall lines; add non-restarting pingDaemon * user-path: async fish-aware killable PATH probe + rt.daemonPath override (S013/S014/S062) * daemon-logger: stream error listener + loggerDegraded + crash-handler raw-write fallback * daemon-logger: drop em dashes from new comments (style) * user-path: reword overlay comment to drop process-artifact reference * daemon: await async resolveUserPath; drop user-path sync-exec allowlist (6.1) * daemon-logger: rt.logLevel resolution, stderr->warn demotion, 50m size cap, recovered-error counter * home: stable machine-key at init, data-preserving freeze of existing stores (S071) * log-janitor: onError callback; daemon logs prune failures at warn * plan: Task 14 imports setSettingsWarnSink from local ./settings/resolve.ts barrel Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * rt-client: rt.logLevel registry row + injectable deduped settings warn sink (dist rebuilt, no bump) * dev-mode: marker-based wrapper detection, bounded read, legacy fallback (S020/S067) * handleCommand: reqId + caller tag + per-(cmd,error) suppression + slow-command info + currentCmd * setup: arm64/unsupported-arch row at setup (R051) * unknown-command envelope (code+version); transports send X-RT-Client (dist rebuilt, no bump) * servers: thread X-RT-Client into payload._client; advertise it in CORS * plan: split HandlerContext type additions into the tasks that provide their values (13/14/15) to avoid a tsc-red window Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ctx: extend refreshStatusRef with cycle outcome; export apiWsClientCount * cache-refresh: remove em dash from applyRefreshOutcome docstring * dev-mode: fix rt-health/steps-a consumer tests for the new wrapper detector (S020/S067) * home-snapshot: diagnose not-provisioned and missing git identity (S090/R043) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * daemon: wire loop monitor + heartbeat + health sampler; surface health/metrics/eventLoop in status/tray:status/ping status-identity.test.ts's fakeCtx() gained getHealth/heartbeatSeq stubs to match the widened HandlerContext. * add rt daemon log-level: live level set/show via daemon:log-level verb * fix: remove em dashes from log-level comments * home-snapshot: gate janitor-zone commit on git identity; drop em dashes (R043) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * secrets: timeout + SecretsTimeoutError on the sops spawn (S070 sops half) * e2e: assert additive health/metrics/eventLoop + heartbeat file * branch-cache: add composeKey/branchOf/identityOf + get/getByBranch (S069 part 1) Also updates the two other BranchCacheStore implementers (lib/daemon.ts's delegating facade, fake-cache-store.ts's test double) and one exact-key-set test assertion so tsc stays clean; store PK/upsert/delete/gc behavior is unchanged. * registry.test: add rt.logLevel to migrated-key fixtures (24 keys) * branch-cache: make get/getByBranch free functions; restore daemon.ts fence (S069 part 1) get/getByBranch on the BranchCacheStore interface forced an edit to lib/daemon.ts's cache facade, which is under a write fence owned by the p2-health lane. Drop get entirely (Task 10 consumers will use entries[composeKey(identity, branch)] directly); reshape getByBranch into a free function over an entries map instead of a store method, so the interface -- and daemon.ts's facade -- do not change. Reverts the daemon.ts and fake-cache-store.ts edits from the prior commit. * fix: windowed maxLag recovery + named loop-lag threshold + review fix-now items Applies the final whole-branch review's approved fix wave for the rt daemon health feature: maxLagMs now decays as a windowed max instead of a lifetime high-water mark (and currentlyStalled drops its redundant maxLagMs OR-leg), the health degraded threshold for event-loop lag is named instead of hardcoded, one branch-introduced em dash is fixed, a process-citation is dropped from a comment, the daemon log-level resolver validates against pino's level set, and the heartbeat reader now shape-guards against a partial-but-valid JSON object. * fix: drop em dashes in arch-row comment and test describe (no-em-dashes) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * branch-cache: flip to composite ${identity}:${branch} key; scope all consumers (S069) Consumers updated: store put (keys off entry.repoName), enrich (cold-start sets identity from remoteUrl), notifier (composite fired-state), worktree-reconciler (branchOf/mrKey), freshness (composeKey lookups), handlers/cache (bare-branch read contract + optional repoIdentity), handlers/system-processes and handlers/worktree (repo-scoped lookups), status/data (branchOf display). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: adapt dev-mode + home-init consumer fixtures to stricter detector/identity check Two earlier-committed behavior changes broke four consumer test fixtures: the dev-mode wrapper detector now requires a real marker (# mattstack-dev-mode or RT_LAUNCH_CWD) instead of treating any file's presence as dev mode, and commitInitialUserRepo now checks git identity before committing. Production code is unchanged; fixtures now plant a recognized wrapper and answer the git config user.name/email probes, matching the pattern already used in home-snapshot.test.ts's defaultResponders. * docs: regenerate command reference for wave-1 daemon flags and verbs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * enrich: repo-scope daemon cache:read; SecretsTimeoutError.name (final review #1/#2) enrichBranches's daemon-first path now passes repoIdentity on cache:read, so a branch name shared across two tracked repos (main/master) no longer risks a suffix-match cross-repo hit. SecretsTimeoutError now sets its name, mirroring AgeKeyTimeoutError. * ci: retrigger checks after docs regeneration * docs: regenerate command reference after wave-2 merge (J5) * chat-handlers.test: drop test for chat:unread-waking, a verb main's delivery-v2 removed --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
m4ttheweric
added a commit
that referenced
this pull request
Sep 26, 2026
…cussion check (#130) Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Daemon stability wave 2: health you can see, someone else's Mac
Stacked on #127. Executes phases 2 and 6 of
docs/daemon-stability-audit-2026-08.md: each phase ran as a design lane (brainstorm, spec, plan, subagent-driven TDD) with an independent Opus review of both the spec and the plan before execution.What changed
Phase 2, health you can see (RT-79; spec
docs/superpowers/specs/2026-08-28-p2-health-design.md)ok/degraded/unhealthy, naming the failing subsystem) plus memory, uptime, watcher and client metrics onstatus,tray:status, andping(R011, R012)rt.logLevelsetting plus a livert daemon log-levelverb; log size cap, per-(cmd,error) suppression, pino stream error listener, stderr demotion, deduped resolver warn sink (R004, S031, S032, S033, R005)Phase 6, someone else's Mac (RT-83; spec
docs/superpowers/specs/2026-08-28-p6-portability-design.md)-lcbase plus a bounded best-effort-ilcoverlay in a killable process group,rt.daemonPathoverride, missing-tool warning; the sync-exec gate loses itsuser-path.tsallowlist entry (S013, S014, S062)rt home initvia hardware UUID, data-preserving for existing machines (S071)branch_cachekeyedidentity:branchwith every consumer repo-scoped and bare-branch lookups preserved (S069)Follow-up
summaryCacheeviction and the two-layer restart wait indaemon-client.tsremain parked minors from wave 1Checklist
rt.logLevel,rt.daemonPath) default to prior behavior.🤖 Generated with Claude Code