feat(orchestrator): startup hygiene — stale per-PID files + orphan detection - #8
Open
evannadeau wants to merge 2 commits into
Open
feat(orchestrator): startup hygiene — stale per-PID files + orphan detection#8evannadeau wants to merge 2 commits into
evannadeau wants to merge 2 commits into
Conversation
Per-PID active-session-<pid> files (introduced in 0.30.19+) make session_id lookup race-free under concurrent sessions, but nothing has been reaping them when the owning claude process exits. On a developer machine with many short-lived sessions per day, they accumulate indefinitely — 8 stale files observed in one project on 2026-05-13, from claude PIDs long since dead. The files are cosmetic in the sense that the legacy single `active-session` file remains the primary lookup, but a slow directory listing eventually becomes a real cost on a hot-spot workstation. This patch adds a startup sweep that walks `<project>/.orchestrator-state/`, matches files of shape `active-session-<pid>`, probes liveness via `process.kill(pid, 0)`, and unlinks dead-PID entries. The probe is cross-platform via Node's API. Cheap, idempotent, race-safe (we only unlink files whose PID is verifiably gone). Lost races with concurrent sessions are tolerated — next startup retries. Runs once at MCP startup, unconditionally (even when the no-claude-ancestor branch is about to exit, so future startups benefit). Tested: bun run typecheck clean, bun test 516 pass / 0 fail. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Complements the existing orphan-bun watchdog (which catches "parent dies while I'm alive" cases for the current process). The watchdog only protects processes that LOADED the watchdog code - older bun processes whose in-memory bytecode predates a fix do not benefit from that fix, and can survive forever if their original parent claude died without triggering whatever watchdog they happen to be running. Concretely: on a developer machine that pulls plugin updates, an MCP process loaded at time T1 may still be alive after the on-disk `dist/server.js` is rebuilt at T2 > T1. If the parent claude that spawned T1's bun dies after T2, the T1 bun's in-memory watchdog code is the version from T1 - any later improvements to watchdog detection are invisible to it. We observed this 2026-05-13: an orphan bun survived ~30 minutes across multiple watchdog tick intervals before manual cleanup via `kill -9`. This patch adds a startup-time scan (Linux only) that walks /proc for bun processes whose cmdline references `orchestrator/dist/server.js` and whose parent chain contains no live `claude` process within 8 hops. Suspects are logged with diagnostic guidance; we do NOT auto-kill, because sibling MCPs may co-own infrastructure shared across live sessions (the python sidecar is deliberately shared via `.sidecar-port`). Detection surfaces the issue; the operator decides. Windows is unchanged - killOlderDuplicateMcps already handles a related case (siblings sharing our parent claude). Pure orphans on Windows are rare because parent death typically reaps children. Tested: bun run typecheck clean, bun test 516 pass / 0 fail. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Merged
4 tasks
evannadeau
added a commit
to evannadeau/claude-plugins
that referenced
this pull request
May 31, 2026
…reaper regression (#4) Two related fixes that share a root cause: a fresh single-claude launch can end up with the MCP server's selfSession.session_id drifted from the harness session_id, and nothing reaps the per-PID active-session-<pid> files that feed the drift. Bug 1 (phantom-sibling false positive): On a cold start with no /resume, if a stale active-session-<pid> file from a prior session collides on PID with the new claude process (or the legacy single-file fallback wins a race), the MCP self-registers in agent_channel.db under an id8 that is NOT the harness session_id. The every-turn cross-session injection then compares against the harness id only and reports the MCP's own row as "1 sibling session active" for the whole session lifetime - heartbeat keeps advancing so the bug never self-clears. Fix: live_sessions.ts gains a process-wide self-id filter. server.ts registers the MCP's selfSession id at startAgentChannel time AND on the first explicit session_id observed via resolveSessionId. getLive- OtherSessionIds excludes BOTH the caller id and the registered self id, preventing the phantom even when the two disagree. Bug 2 (stale per-PID active-session file reaper): The per-PID active-session-<pid> scheme (0.30.19+) makes session_id lookup race-free under concurrent sessions, but nothing reaps the files when the owning claude process exits. They accumulate indefinitely - one project hit 30 stale files in ~12 days. Worse, PID reuse hands the fallback resolver a session_id that is no longer live, feeding directly into Bug 1. Fix: extract reapStaleActiveSessionFiles into its own engine module (testable in isolation with an injectable liveness probe) and wire it at MCP server startup. Cheap, idempotent, race-safe via process.kill(pid, 0). Originally proposed upstream as spawnbox-dev/claude-plugins PR SpawnBox-dev#8; that PR never merged so this fork carries the fix. Tests: - tests/engine/live_sessions_phantom_self.test.ts (3 cases) - covers drift, genuine siblings still surface, idempotent self-id updates. - tests/engine/startup_hygiene.test.ts (6 cases) - covers reap of dead- PID files, non-PID files ignored, missing dir no-op, empty dir no-op, real process.kill probe smoke test, pid<=0 rejection. - Full suite: 599 pass / 0 fail (was 590 baseline). - tsc --noEmit clean. Version: 0.30.52 -> 0.30.53. Co-authored-by: Evan Nadeau <1878498+evannadeau@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
SpawnBox-dev
pushed a commit
that referenced
this pull request
Aug 8, 2026
…e vector (0.46.0) Semantic search did not work on this corpus, and it is not clear it ever has. Measured 2026-08-08 against the live 7148-note KB. THE EVIDENCE. Five paraphrase probes - same idea, deliberately different vocabulary - ranked the correct note #205, #967, #1206, #1688 and #3247 out of 7148. The control, querying by each note's OWN words, returned #1 every time, so the data was findable and only meaning-matching was broken. That is the one capability a 1.8GB embedding model exists to provide. WHAT IT WAS NOT. The pipeline is correct: self-retrieval returns rank #1 at score 1.000 and stored vectors are unit-normalised. Anisotropy was the obvious suspect - unrelated notes scored 0.73-0.79 cosine against each other, where healthy is ~0.1-0.3 - so mean-centering was tried, the standard fix. It repaired the score collapse (0.79 -> 0.33) and made every single rank WORSE (mean 1463 -> 2678). Hypothesis refuted and recorded as such. WHAT IT WAS, in two parts. 1. TRUNCATION. sidecar/embed_server.py caps the tokenizer at 512 tokens (~2000 chars) while 3402 of 7148 notes are longer; the largest note had ~97% of its text discarded before its vector was computed. bge-m3 supports 8192 tokens, so the model was chosen for long context and configured to use 6% of it. 2. DILUTION, which is the bigger half. Even VERBATIM text from inside the window failed to retrieve its own note (ranks #17-#1961). One 1024-dim vector cannot represent a 5000-word note covering twenty separate claims; every specific idea averages into the document mean. That also explains the 0.73-0.79 similarity between unrelated notes - all document averages look alike. THE FIX. Embed PASSAGES. A note is split into ~1500-char overlapping chunks (mcp/engine/chunking.ts) and each is embedded into the new note_chunks table; a note's vector score becomes its BEST-matching chunk. Max-pool, not mean - averaging would re-introduce the dilution being fixed. Chunks sit comfortably inside the existing 512-token window, so this needed no change to the Python sidecar; the truncation limit simply stops mattering. VALIDATED BEFORE SHIPPING. Controlled A/B on a 300-note subset, same model, same probes, only the representation varying: chunked max-pool won 5/5 probes, mean rank 63.2 -> 32.0. Honest limits: the probes are adversarial (near-zero lexical overlap), the remaining ranks are #8-#98 of 300 rather than top-3, and the vector leg is RRF-fused with BM25 in practice, so it contributes rather than dominates. This is a real improvement, not a silver bullet. MIGRATION IS DELIBERATE, NOT AUTOMATIC. New and edited notes are chunked on write. Existing notes get chunks via backfillChunks(), which NOTHING calls on its own - a test asserts server.ts never references it. That is the direct lesson of 0.44.0/0.45.1, where a sweep wired into startup halted this machine twice in one afternoon. It is resumable by construction (population = notes with no chunk rows, recomputed per call), takes an optional limit so it can be done in sessions, and processes longest notes first so an interrupted run still delivers the biggest wins. Until a note is backfilled it keeps scoring off its note-level vector, so search degrades rather than dropping notes. `embeddings` is intentionally retained: the near-duplicate gate and auto-linker ask a whole-document question, which is what that vector answers. Guards: tests/engine/chunking.test.ts (8), tests/engine/chunked-retrieval.test.ts (13), including wiring assertions for max-pool, the no-chunks fallback, and the opt-in-only property. Suite 1082 pass / 0 fail. Also fixes a test fixture that returned one vector regardless of input - it only worked while embedIfAvailable sent exactly one text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MD4kPkrZLWbUxe4arhdwii
SpawnBox-dev
pushed a commit
that referenced
this pull request
Aug 11, 2026
…sed, and a test a comment could pass was hiding a stale base rate SA-d4db6493 nominated this as worth more than any note reorganisation, after being the SECOND first-person report of the same fork. Confirmed against both sources before touching anything. === THE FORK, and it is the alert's fault rather than the reader's Note e24d8156 is the liveness-triage method. Firing #11 (2026-08-11) recorded a reader skipping its two most important steps. Reading the alert text explains why: BOTH OMISSIONS WERE PRESCRIBED BY THE ALERT. * The alert said "sample twice a few seconds apart". Firing #10 had already MEASURED a 20-second frozen window on a session that was provably running - it was executing the command doing the measuring - and revised the method to three-plus points across 40-60s with an INDEPENDENT control. The alert never got that revision. The reader followed the alert and got a right answer from an instrument that could not have told them they were wrong. * "Did they post to the channel after the alert?" - free, decisive, and the reader already holds the messages - was the THIRD bullet, framed as one branch among several. It is now first, and says why it gets skipped: it does not FEEL like evidence because it is not a measurement, and it outranks one. Two readers with the method note available followed the alert instead (firings #8 and #11). A method note cannot compete with the instructions printed at the moment of the alarm. === THE SIBLING THAT NEVER GOT THE FIX The EGRESS alert hardcoded "BASE RATE: 0 of the last 8 firings" - stale (the note records 0 real faults in 11). The INGRESS alert had already removed exactly this at 0.34.0 with the reasoning written out: "a stated base rate that can go stale is worse than no stated base rate - it carries the authority of a measurement with the durability of a comment." That lesson was applied to one alert and never to its sibling. Same shape as the step-1 gap, and the same shape as a5f4f5a1's grep sweep: the fix landed where attention was and stopped there. Replaced with the calibration stated qualitatively plus a pointer to the note for the live count. Deliberately NOT re-derived as a number - any number printed in an alert rots the moment the next firing lands. === A TEST THAT A COMMENT COULD PASS Found while fixing the above. The guard read RAW SOURCE - comments included - and sliced a fixed 2600 characters from the marker. Two failures: 1. Adding rationale comments inside the alert pushed real text out of the window, failing an assertion about content that was still present. 2. Worse: when the hardcoded base rate was removed, the "prints the base rate" test KEPT PASSING, because the phrase survived in the comment explaining its removal. The assertion was satisfied by prose no reader will ever see. A test that a comment can pass is not testing the alert, it is testing the file. Now strips comment lines and anchors to the end of the template rather than counting characters. One assertion changed from pinning a literal count to asserting the reader gets calibrated - the intent 0.44.1 actually had - plus a new regression test that FAILS if any "N of the last M" is reintroduced. Suite 1249 pass / 0 fail (the orient-auto-retro flake did not fire this run). Typecheck clean. Runtime reports 0.67.0. Not live-verified: the fleet runs 0.64.0 (62d10892 - 0.65.0 never propagated), so this text will not reach a reader until a reload. Refs e24d8156, ed971934 (the open thread that proposed this three days ago), a5f4f5a1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MD4kPkrZLWbUxe4arhdwii
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
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.
Summary
Two additive startup-time hygiene improvements in
plugins/orchestrator/mcp/server.ts. Both run once per MCP startup, both are detection/cleanup (never auto-kill), both are no-ops in the steady state.1. Reap stale per-PID
active-session-<pid>files (b7f43b7)Per-PID
active-session-<pid>files (introduced in 0.30.19+) make session_id lookup race-free under concurrent sessions, but nothing has been reaping them when the owning claude process exits. On a developer machine with many short-lived sessions per day, they accumulate indefinitely — 8 stale files observed in one project on 2026-05-13, from claude PIDs long since dead.They are cosmetic — the legacy single
active-sessionfile remains the primary lookup — but a slow directory listing eventually becomes a real cost.This patch adds a startup sweep that walks
<project>/.orchestrator-state/, matches files of shapeactive-session-<pid>, probes liveness viaprocess.kill(pid, 0), and unlinks dead-PID entries. Cross-platform; cheap; idempotent; race-safe (only unlinks PIDs verified gone). Lost races with concurrent sessions tolerated — next startup retries.2. Warn about likely-orphan sibling MCPs (
a28388e)Complements the existing orphan-bun watchdog (which catches "parent dies while I'm alive" for the current process). The watchdog only protects processes that LOADED the watchdog code — older bun processes whose in-memory bytecode predates a fix do not benefit, and can survive forever if their original parent claude died without triggering whatever watchdog they happen to be running.
Concretely observed 2026-05-13: an orphan bun survived ~30 minutes across multiple watchdog tick intervals before manual
kill -9. The on-diskdist/server.jshad been rebuilt while the orphan was running, so any subsequent watchdog improvements were invisible to it.This patch adds a startup-time scan (Linux only) that walks
/procfor bun processes whose cmdline referencesorchestrator/dist/server.jsand whose parent chain contains no liveclaudeprocess within 8 hops. Suspects are logged with diagnostic guidance:Detection only — does NOT auto-kill. Sibling MCPs may co-own infrastructure shared across live sessions (the python sidecar is deliberately shared via
.sidecar-port— killing an unrelated bun could take down a live session's embeddings). The operator decides whether to clean up.Windows is unchanged —
killOlderDuplicateMcpsalready handles a related case (siblings sharing our parent claude). Pure orphans on Windows are rare because parent death typically reaps children.Why "detection, not auto-kill"
The sidecar reuse pattern at
startSidecar()line 338–350 deliberately shares the python embedding server across MCPs. An auto-kill could take down a sidecar that a live session depends on. Surfacing the problem at startup gives the operator the information without the risk.Files changed
plugins/orchestrator/mcp/server.ts— 2 new functions + 1 startup block wiring them inplugins/orchestrator/dist/server.js— rebuilt viabun run buildImports updated: added
readdirSyncandunlinkSyncto the existingnode:fsimport line.Tested
bun run typecheck— cleanbun test— 516 pass / 0 fail / 38 files / 1207 assertions (no test changes)active-session-<pid>files in a real workspace were correctly identified as dead and could be removed by the same logic the reaper applies.Test plan
.orchestrator-state/directory — both functions should be silent no-ops.active-session-<pid>files — verify the startup log line reports the reap count and the files are gone.claudeancestor).🤖 Generated with Claude Code