fix(session-lock): never elect or count Claude daemon infrastructure as the session holder - #2388
Open
Kallas95 wants to merge 4 commits into
Open
fix(session-lock): never elect or count Claude daemon infrastructure as the session holder#2388Kallas95 wants to merge 4 commits into
Kallas95 wants to merge 4 commits into
Conversation
…e session holder The session-lock election could pick the long-lived PPID-1 Claude daemon (claude daemon run, --bg-pty-host, --bg-spare) as the session pid when a hook fired inside the daemon worker chain. A lock naming the daemon never went stale after the real session died, so every later terminal-launched session was refused and started read-only forever. Election now skips daemon-infrastructure pids and returns the topmost real session process below the chain, failing when the whole run is daemon infrastructure. Holder liveness now rejects a live daemon pid, so a legacy lock naming the daemon classifies as stale and is recoverable through the normal takeover path. Membership still walks through the daemon chain unchanged.
…ection and liveness guarantees
…the infra token scan
4 tasks
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.
Intent
Fix a session-lock poisoning bug in bin/fm-session-lock-lib.sh: the lock can elect the Claude Code daemon as the session pid, producing a lock that is never recoverable.
Incident evidence (2026-08-14): state/.lock contained 41273 = /Users/max/.local/bin/claude daemon run ... (PPID 1, long-lived). It was written by a hook running inside Claude Code's daemon worker chain (hook shell -> claude bg-spare -> claude bg-pty-host -> claude daemon run, all contiguous claude matches) during a /clear + window-close episode. fm_harness_ancestry_pid (the WRITE-side election) picks the OUTERMOST pid of the contiguous claude run, which in that chain is the daemon itself. fm_harness_pid_alive (the liveness test) accepted 41273 as a live harness, so the lock never went stale after the real session died. Every later terminal-launched session (ancestry = its own single claude pid) was refused and started read-only, forever, until the captain authorized a manual rm state/.lock.
Required fix - three distinct properties, do not conflate them:
Acceptance criteria:
Delivery route for this repo: push ONLY the branch fm/fm-lock-daemon-pid to the fork https://github.com/Kallas95/firstmate (leave the worktree's remotes unchanged; push by explicit URL), then open the upstream PR against kunchenguid/firstmate:main with gh-axi. Local no-mistakes validation green + upstream PR open is this repo's terminal state; the upstream maintainer merges.
What Changed
fm_harness_ancestry_pid(the write-side election inbin/fm-session-lock-lib.sh) now skips Claude daemon-infrastructure pids —claude daemon run,--bg-pty-host,--bg-spare, recognized only in the leading argv command/flag tokens (including node/python-wrapped installs) and gated on the process already classifying as Claude — electing the topmost real session pid below the daemon chain, and failing instead of writing a doomed lock when the entire contiguous run is daemon infrastructure.fm_harness_pid_aliveno longer counts a live daemon-infrastructure pid as a live harness holder, so a legacy lock naming the daemon classifies as stale and stays recoverable through the normalbin/fm-lock.shtakeover path; the printed ancestry is untouched, so membership (fm_session_lock_owned_by_self) still walks through the daemon worker chain unchanged.pstests totests/fm-session-lock-ancestry.test.sh(election skips the daemon chain, election fails on an infrastructure-only run, liveness rejects a live daemon pid, free-text marker phrases don't misclassify a real session, node-wrapped daemons are recognized, membership through a daemon-parented chain, non-Claude harnesses untouched) and updateddocs/verification/supervision.mdto match the new election/liveness contract.Note: the same daemon-chain identity issue is the suspected cause of the upstream-known fm-claude-autoarm-inert behavior (Stop auto-arm never claiming the home). That is deliberately not fixed here — this change keeps its scope to the session-lock library.
Risk Assessment
✅ Low: All three required properties (election, liveness, membership) are implemented exactly as the intent and the two captain-directed fix rounds prescribe, verified by behavioral tests over deterministic fake-ps tables and real orphaned process trees, with shellcheck clean and only one speculative, unevidenced residual argv shape remaining.
Testing
Suite ciblée fm-session-lock-ancestry (étendue par ce changement) exécutée via le runner canonique : 14/14 ok, couvrant les cinq critères unitaires exigés et trois e2e sur de vrais arbres de processus ; rejeu manuel de l'incident avec de vrais processus (sans ps simulé) prouvant fail-before/pass-after au niveau CLI — base : verrou démon « held » et reprise refusée à jamais, cible : « stale » puis reprise normale réélisant la vraie session — plus contre-épreuve qu'une session vivante ordinaire reste « held » ; suite voisine fm-claude-stop-autoarm verte ; fixture nettoyée, arbre propre. Pas de capture visuelle : changement purement CLI/bibliothèque shell, la transcription CLI est la surface utilisateur réelle.
Evidence: Transcription CLI : rejeu de l'incident, base vs cible (status + acquisition)
state/.lock = 39109 (processus RÉEL vivant, PPID 1: .../hbin/claude daemon run) BASE bin/fm-lock.sh status → lock: held by live harness pid 39109 (le bug) CIBLE bin/fm-lock.sh status → lock: stale (pid 39109 dead or not a harness) BASE acquisition → error: another live firstmate session holds the lock (pid 39109); operate read-only (exit 1) CIBLE acquisition → lock acquired: harness pid 42755 ; .lock = 42755 (vraie session claude -p élue par l'ascendance réelle) Contre-épreuve (claude session ordinaire vivante, pid 39110) → lock: held by live harness pid 39110Evidence: Sortie complète de la suite fm-session-lock-ancestry (14 ok)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-session-lock-lib.sh:140- fm_harness_daemon_infra matches the markers ' daemon run ', ' --bg-pty-host ', ' --bg-spare ' as space-delimited substrings anywhere in the full argv. A real claude session whose argv embeds prompt/brief text containing one of these phrases is misclassified as daemon infrastructure: its own election fails (read-only start) and, worse, fm_harness_pid_alive denies it as a live holder, so a competing session can classify its lock as stale and take over a live session's home — the inverse of the incident. This is concretely plausible in this fleet: the live process table shows crewmate sessions launched asclaude ... "-cFIRSTMATE_OP: v1 launch-brief: ..."with multi-KB task text in argv, and this very task's brief contains 'claude daemon run' verbatim. Mitigations in practice: interactive primary sessions have short argv, and the autoarm's primary-scope gate excludes crewmates, so real reachability is narrow. The substring-over-args identification is exactly what the authoritative intent mandates ('claude processes whose args contain ...'), so any tightening (e.g. matching markers only among the leading option/subcommand tokens, before any -p/-c payload) is an intent-level design decision for the author, not an auto-fix.bin/fm-session-lock-lib.sh:185- The daemon-infrastructure predicate is applied to every harness's argv, not only Claude's (fm_harness_pid_alive and the election loop), while the intent and the code comment scope it to claude processes. Acceptance guarantee (e) 'non-Claude harnesses are untouched' therefore holds by the documented assumption that the markers never appear in pi/codex/opencode argv, not structurally. FM_HARNESS_IS_CLAUDE is freshly set right before the check in fm_harness_pid_alive, so gating there would be trivial if ever desired. Deliberate, documented tradeoff — no action needed.🔧 Fix: restrict daemon-infra markers to leading argv tokens of Claude
2 issues (1 warning, 1 info) still open:
bin/fm-session-lock-lib.sh:151- fm_harness_daemon_infra strips exactly one leading argv token before matching markers, but fm_harness_process_matches (lines 75-81) deliberately identifies interpreter-wrapped Claude ('node …/claude-code/cli.js …', comm=node) as a Claude harness with FM_HARNESS_IS_CLAUDE=1. An npm-installed Claude Code daemon presents as 'node /…/@anthropic-ai/claude-code/cli.js daemon run …' (shebang rewrite), so the marker sits after TWO command tokens: the script path is a bare word, the parser returns not-infra, and the daemon chain is again electable as the outermost pid and counted as a live holder — the exact incident failure (never-stale poisoned lock) remains reachable on interpreter-wrapped installs. Round 1's substring match covered this shape; the round-2 leading-token hardening dropped it. Fix at the same boundary: when the first argv token's basename is a bare interpreter (mirroring the node/python clause in fm_harness_process_matches), skip one extra leading token (the script path) before applying the identical leading-token rules; this stays within the captain's 'executable/subcommand/flag portion, never free text' prescription and needs a matching mocked-ps unit case.bin/fm-session-lock-lib.sh:155- ps -o args= carries no argv boundaries, so two residual misclassifications are irreducible: a real session launched asclaude 'daemon run …'(prompt as the first positional token) matches the 'daemon run' subcommand rule, andclaude -p --bg-spare …(marker token in a flag's value position, preceded only by dash tokens) matches the flag scan. Both shapes are byte-identical to genuine daemon argv in the flattened view, so no parser over ps output can distinguish them. This is a far narrower residual than the round-1 substring rule (free text behind any bare word or after--can never match) and is an acceptable documented tradeoff; no action needed.🔧 Fix: recognize interpreter-wrapped Claude daemons in the infra token scan
1 info still open:
bin/fm-session-lock-lib.sh:157- fm_harness_daemon_infra strips exactly one preamble token after a bare interpreter, so a daemon whose argv carries interpreter flags before the script path (e.g. 'node --max-old-space-size=4096 /…/cli.js daemon run', reachable only via inherited execArgv on fork) would evade the infra scan while still classifying as Claude, making it electable and countable as a live holder. No evidence exists that any real Claude Code install produces this shape (the shebang shim adds no flags; NODE_OPTIONS never appears in argv), so this is a documented residual, not a reachable defect. If ever observed, the fix is local: skip leading dash tokens between the interpreter and the first bare word (the script path) before applying the existing leading-token rules.✅ **Test** - passed
✅ No issues found.
bin/fm-test-run.sh tests/fm-session-lock-ancestry.test.sh— 14/14 ok : élection qui saute l'infra démon, échec d'élection si tout est infra, liveness rejetant unclaude daemon runvivant, appartenance à travers la chaîne démon, harnais non-Claude intacts, texte libre argv, démon enveloppé node, plus 3 e2e réels du Stop auto-armRejeu manuel de l'incident avec un vrai processus vivant PPID 1 d'argv<chemin>/claude daemon runinscrit dansstate/.lock:bin/fm-lock.sh statusbase (6789876, extraite viagit archive) →held by live harness pid, cible →stale; acquisition base → refusée « operate read-only » (l'incident), acquisition cible → reprise et réélection de la vraie session Claude (pid 42755) via l'ascendance réelle; contre-épreuve : session claude ordinaire vivante → toujoursheldbin/fm-test-run.sh tests/fm-claude-stop-autoarm.test.sh— suite voisine consommant le même contrat d'identité, passe intégralement✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.