Session-lock fork-handover fix (panel-hardened) - #2
Merged
Merged
Conversation
* fix(herdr): place workers in the launching agent's exact workspace Herdr enforces no workspace-label uniqueness, and spawn resolved its container by taking the FIRST workspace whose label matched the home label. With two workspaces both labeled "firstmate", a worker launched from the second one was created in the first, so it appeared in a different space than the Firstmate the captain was watching. Reproduced end to end on Herdr 0.7.5 protocol 17 by running the real bin/fm-spawn.sh inside a launcher pane in the second "firstmate" workspace: the worker landed in w1 while its launcher was in w2, with an unrelated third workspace focused throughout, which also rules out any dependence on the focused workspace. Placement now binds to the launching process's own Herdr identity. Herdr injects HERDR_PANE_ID, HERDR_SESSION, and HERDR_SOCKET_PATH into every process it manages a pane for, and fm_backend_herdr_launcher_identity resolves that pane's current owning tab and workspace live from Herdr, cross-checking the pane against its tab and confirming the workspace exists exactly once in the session. The injected HERDR_TAB_ID and HERDR_WORKSPACE_ID are creation-time snapshots and are deliberately not read as current identity. Labels are no longer placement authority. A claimed parent identity that is unreadable, contradictory, stale, or from another named session or Herdr server stops the spawn before any worker endpoint exists, rather than degrading to a label search. A launcher with no Herdr ancestry has no workspace to inherit and keeps the per-home labeled container, which must now resolve to exactly one workspace; two same-labeled candidates refuse instead of adopting either. A --secondmate launch keeps standing up that home's own workspace by design. With presentation spaces enabled, the projected child is created and bound under that same exact parent and anchors its ordering on it, so a duplicated home label no longer makes the layout ambiguous. Projection, focus restoration, restart binding, and quarantine rules are unchanged, and children are never collapsed into the parent. tmux, Zellij, cmux, Orca, and the away-mode daemon terminal were each inspected and are not affected: none resolves a container by searching mutable labels. tests/fm-backend-herdr-launcher-workspace-e2e.test.sh drives the real spawn and teardown against an isolated Herdr lab, with its headline case running fm-spawn.sh inside a real Herdr pane so the identity comes from Herdr's own injection. The refusal matrix and the ordering anchor are covered deterministically in tests/fm-backend-herdr.test.sh. Eight existing real-Herdr suites inherited the developer terminal's own Herdr pane into their isolated lab sessions, which the new cross-session check correctly refuses. tests/herdr-test-safety.sh now owns herdr_forget_inherited_pane and those suites call it, so what they assert no longer depends on where they were launched from. Two unrelated fixes found along the way. tests/fm-secondmate-harness.test.sh had the same class of environment leak through CLAUDECODE, which outranks PI_CODING_AGENT in bin/fm-harness.sh and made its pi-signed ancestry case resolve "claude" whenever the suite ran inside Claude Code. And fm-spawn.sh's usage() printed a fixed line range that had already been truncating its own help mid-sentence. * no-mistakes(review): Enforce exact Herdr launcher and projection identity * no-mistakes(document): Document exact Herdr launcher workspace placement
FM_HARNESS_RE matched the bare command name "claude", but Claude Code names its shared background daemon and the daemon's pooled workers "claude" too. The daemon is parented to init, is shared by every session of the user, and outlives all of them, so when a hook or tool call was hosted by a pooled spare the ancestry walk resolved the daemon's pid as that session's identity. That broke identity in both directions. A session-start fm-lock.sh call hosted by a pooled spare published the daemon's pid into state/.lock, so two different sessions each verified ownership of a lock the other wrote, and because the daemon never dies no later session could see that lock go stale: fm-lock.sh refused acquisition permanently and the home stayed read-only. With a daemon pid in the lock, fm-claude-stop-autoarm.sh found ownership false but the recorded owner alive, so it exited 0 on every Stop for the whole session and nothing routinely armed the watcher. The walk now classifies process shapes through one shared owner, fm_harness_session_match, used by both the ancestry walk and the holder-liveness check so they cannot disagree about the same pid. The daemon (matched by its "daemon" subcommand, not by one --origin value), bg-pty-host, and bg-spare are rejected as non-session shapes. Claude Code's versioned executable is now recognized as a session, so resumed, forked, and app-hosted sessions resolve their own per-session pid instead of the daemon-owned pty host above them. Infrastructure met before any session match is skipped and the walk continues, which preserves the nested bg-spare chain fixed in kunchenguid#1206; infrastructure met after a match ends the contiguous run. When every candidate is infrastructure the walk resolves nothing and every caller fails closed, because a pooled worker's ancestry and inherited environment both describe the daemon and carry no evidence of the session that claimed it. A lock recorded against infrastructure is now a reclaimable owner, so an already-wedged home recovers through the existing stale-owner path. Every caller of the shared predicates was reviewed: all six call sites in fm-claude-stop-autoarm.sh and fm-lock.sh were affected and are fixed by this change. fm-harness.sh answers harness kind rather than session identity and is unaffected; fm-sessionstart-nudge.sh classifies no shape and was exposed only through lock content.
…oned exec and fail-closed inertness
A Claude Code mid-conversation fork (observed live 2026-07-29 16:51, Claude Code 2.1.220) replaces the session process: a new pid carrying a new --session-id continues the working conversation while the pre-fork pid stays alive as an idle interactive process. The lock still recorded the pre-fork pid, so the working session failed fm_session_lock_owned_by_self forever, the Stop auto-arm stayed correctly-but-silently inert, and nothing noticed that supervision was down: the working session neither owned its home nor demoted. Identity now has two layers. state/.lock keeps the owning pid; a new state/.lock-session sidecar records the owning harness SESSION id when a trusted source proves it - the Stop-payload session_id hint exported by the auto-arm, or the CLAUDE_CODE_SESSION_ID/CLAUDE_PID env pair Claude Code plants in every tool shell, accepted only when CLAUDE_PID names exactly the ancestry-resolved pid so a daemon-inherited or outer-session environment can never leak in. Command-line text is never an identity source because prompts are argv-visible. fm_session_lock_relation classifies a non-owned holder as self, same-session, live-other, or stale. A same-session holder is the same logical session in a replaced process, so fm-lock.sh re-keys the pid and the auto-arm recovers through the existing fm-lock.sh delegation, after the unchanged AFK and need gates. A live-other holder is never armed over, reclaimed, inherited, or forced - but the auto-arm no longer goes silent: once per distinct holder it wakes the model (exit 2, deduped via state/.claude-autoarm-foreign-lock) with the real diagnosis, and the fm-lock.sh refusal now names the holder's pid, session, start time, and terminal, plus the exact recovery action when this process's own argv shows it was forked from the recorded session. Dead-holder reclaim, daemon-shape rejection, lock-refused read-only, and the no-force rule are unchanged; non-Claude harnesses resolve no session id and keep the exact pid-only contract. Also fixes a latent test-hermeticity bug in the cherry-picked shared-daemon reclaim test: a single-command fake-claude body was tail-exec-collapsed by bash, so the ancestry walk escaped to whatever real session hosted the test run (it only passed locally because a real Claude session sat above the runner; CI has none).
…key scope honestly Independent review confirmed against the verification data that Claude Code's --fork-session mints the successor a NEW session id (cfaf5775 -> 42ed4142 in docs/verification/supervision.md), so same-session re-keying covers only successions that KEEP their session id; the fork successor's path is the loud foreign-owner notice while the pre-fork process lives, then the ordinary stale reclaim the moment it exits. That split is deliberate - fork lineage cannot distinguish an abandoned live holder from an idle working one, so automatic takeover from any live different-session holder would break the never-inherit boundary - but the lib, hook, and doc wording previously blurred the two legs. State the coverage split explicitly in fm-session-lock-lib.sh, fm-lock.sh, fm-claude-stop-autoarm.sh, docs/watcher-continuity.md, and the verification record, and add the missing end-to-end fork test: sidecar holds the pre-fork id, the Stop payload proves a different id, the hook emits exactly the foreign notice naming the holder and its session while mutating nothing, and the firing after the pre-fork process exits reclaims, re-keys to the successor's own id, and arms. Also make the two foreign-owner tests hermetic: their single-command fake-claude bodies were tail-exec-collapsed by bash, so the hook's ancestry escaped to whatever real session hosted the run; in CI, where no real session exists, classification would have failed closed and the tests would have failed.
…ession re-key Third-review TOCTOU findings, dispositioned by evidence. A same-session re-key rewrites the sidecar with a byte-identical value, so skip the clear-then-rewrite entirely in that case: the recorded identity is then never even transiently absent, which removes both the crash window that degraded a re-keyed lock to pid-only and the false foreign-owner notice a concurrent auto-arm firing could emit while the sidecar was cleared. Every other acquisition keeps the clear-first order deliberately: the reviewer-proposed alternative (retain the old sidecar until the new one lands) would let a crash leave the previous owner's identity beside the new owner's pid, and a relaunch successor of that PREVIOUS session could then prove same-session against a live foreign holder and steal the lock - clear-first makes every crash degrade toward pid-only, the safe direction. A sidecar write failure now also warns on stderr with the concrete consequence (a replacement process for this session cannot take over until this process exits) instead of degrading silently, and the lib header notes that alternating re-keys between two live incarnations of one session stay inside that session and settle once the superseded incarnation idles.
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
2 participants
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.
Landing on our fork per captain decision.