reliquary/docs/workpad/reports/2026-10-16-16-headless-forks-phase1-review.md

Phase 1 review: headless forks (spire:forks helper core)

• Reviewer model id: claude-opus-5-5[1m] (fresh; did not plan, attack, build or smoke this phase)
• Date: 2026-10-16 16:08 +0200 (date)
• Scope: Phase 1 commits 670401d..cb65d88 on headless-forks, plus the Phase 0 flags (7421d3a, 83f6bac; Phase 0 had no separate review)
• Read: build plan plans/2026-10-16-02-headless-forks-build.md (§4, §6 Phase 0/1), plan plans/2026-10-16-01-headless-forks.md (§2, §3, §4, §7), design designs/2026-10-16-01-headless-forks.md (§2–§7), preflight report reports/2026-10-16-09-headless-forks-preflight.md, run log reports/2026-10-16-08-headless-forks-run.md, contract digest plans/2026-10-16-12-headless-forks-phase1-contract.md, and all of plugins/spire/skills/forks/ (forks.py, preamble.md, templates/, tests/test_forks.py).
• Counts: Critical 0 · Important 3 · Minor 16

Gates I ran

Gate	Result
G-unit uv run plugins/spire/skills/forks/tests/test_forks.py	exit 0, Ran 69 tests in 52.473s, OK
G-where uv run plugins/spire/skills/forks/forks.py where	exit 0; stdout is one JSON object, ok: true, spawn: cim-hidden, binary 2.1.299 via CLAUDE_CODE_EXECPATH; uv's line on stderr
Scope fence	git diff --stat 83f6bac..cb65d88: only the 5 files under plugins/spire/skills/forks/ plus the run log and the digest. No attribution lines in any Phase 1 commit body.
Leftover processes after my runs	none (tasklist python.exe: none)

Probes I ran (scratchpad, read-only, no claude calls):

• p1review_refused.py: _conclude on an is_error result whose modelUsage did not grow → failed model_refused (finding I1).
• p1review_tree.py: process_tree() on dead-root records whose pid equals a live process's stale parent pid. On this host right now, 5 such pid values exist. One claims csrss.exe/winlogon.exe/dwm.exe, one claims dirigent-watchdog.exe/dirigent-web.exe, and one claims OneDrive.Sync.Service.exe (finding I2; the probe kills nothing).
• claude --help (2.1.299, no API call): --effort choices low, medium, high, xhigh, max; --permission-mode choices acceptEdits, auto, bypassPermissions, manual, dontAsk, plan (finding M5).
• Spot check of Phase 0 numbers against raw files: raw/p06/state.json (ping A 0.91615, fork A 1.00041, fork B 0.08042, cost Δs) and raw/p02/seq1.summary.json (first read 182,202, result usage 14 / 1,448 / 3,113 / 1,288,193, cost 1.9413766). Both match the preflight report. raw/p01/r3-wincontrol/check-*.json matches the positive-control row too.

---

Findings

Critical

None.

Important

I1. Any error that happens before the model answers is reported as model_refused. forks.py:1200

if ss and (ss.refused or (res and requested not in models_used)):
    status, reason = "failed", "model_refused"

A call whose request is rejected still emits a result event (is_error: true). Examples: the plan usage limit, an overload after retries, an API error on the first request, a prompt that is too long. For a fork, the restored modelUsage is cumulative, so nothing grows and models_used is []. The requested id is then "not in" an empty list. My probe confirms it: failed model_refused models_used [], requested claude-opus-5-5.

Failure scenario: a fork launched near the subscription limit (R11) gets a 429. The result reads model_refused, and the Phase 3 procedure will route that reason to "the user must add the id to availableModels". That is the wrong remedy for the most likely error on a subscription.

Source: the contract digest. The preflight report states the rule as "that event, or a modelUsage key ≠ the requested id", which means a substitution: some other key grew. The digest (§Supervision, "Model refused") loosened it to "models_used not containing the requested id".

Fix direction:

• Infer a refusal from modelUsage only when models_used is non-empty and lacks the requested id.
• Otherwise fall through to is_error / exit_code, and consider carrying result.api_error_status (present in the real result keys, raw/p02/seq1.summary.json) into reason_detail.

Test gap: the stub's error directive still bills the requested model, so no test covers this case.

I2. Walking the tree of a dead root can claim and kill unrelated processes. forks.py:622-652 (process_tree). It is reached through reap_lost (:1355-1356, called by wait, list, fork, continue) and kill_job (:1383) whenever the recorded supervisor or claude is already dead.

When the root is dead, proc_ctime(root) is None, so the reuse guard at :641 does not fire. Every live process whose parent pid equals the recorded pid is then taken as a child if it was created at or after the root's recorded creation time. The children of a dead process keep its pid as their ppid. So the walk also matches this sequence: the pid was reused by a later process, that process started something and exited. The time guard does not separate the two cases, because the later processes are by definition newer.

Failure scenario:

1. The host reboots (Windows Update), or the supervisor crashes, while a fork runs. The registry still says running.
2. The next list/wait/fork (a successor Athanor's first call) reaps the job and calls kill_tree(rt["claude"]).
3. If any live process's dead parent had that pid, it and its whole subtree are TerminateProcessed.

After a reboot every process is newer than the record, so the time guard filters nothing. My read-only probe found 5 such pid values on this host now. They claim, among others:

• dwm.exe/winlogon.exe (pid 5572's orphans);
• the Dirigent watchdog and web server;
• the OneDrive sync service.

The chance per event is low, because the recorded pid has to match one of a handful of stale ppids. The damage is severe. The pid-reuse guard is the item 1.2 flagged as untested.

Fix direction: when the root is dead, bound descendants by the last time the job was known alive: child ctime ≤ the job file's heartbeat_at (or ended_at). Alternatively, have the supervisor record the claude subtree (pid + ctime) on each heartbeat, and kill only recorded pairs once the root is gone. The supervisor's immediate post-exit sweep (:2183) is safe as it is (milliseconds after exit).

I3. A continue moves the base's last_hit forward, so warmth is overstated.

• forks.py:2144: hit_written = e["kind"] == "train", so continue bumps the base at its first event.
• :1277: _conclude bumps it again with ss.first_at for every non-train kind.

Design §5 states the conservative assumption on purpose: "only a request that reads exactly the base prefix (a fork's first request) refreshes it; later requests in a fork read longer prefixes". A continue's first request reads the fork's own, longer prefix. The smoke shows exactly that: 27,192 read against a base of 23,676. That hits the fork's cache entry, not necessarily the base's.

Failure scenario:

1. Base trained at 10:00; fork S1 at 10:05 (refreshes the base); S1 ends 10:20.
2. continue S1 at 10:50 reads S1's warm prefix, and the helper sets the base's last_hit to 10:50.
3. The real base entry expires at about 11:05. The helper reports the base warm until 11:44.
4. Phase 2's ping rule fires only in expiring, so it skips the ping. The next fork at 11:10 cold-starts at about 1.9 × prefix: $0.69 against $0.09 in P0.6.

The run log lists this as a choice ("A continue refreshes its base's last_hit"). The digest's "or any of its forks" left it open, but the design did not.

Fix: bump the base only for fork (and Phase 2 pings), or only when the first cache_read is within the base context plus a small margin.

Minor

M1. A kill during finalisation can leave a stale lock and a duplicate ledger line. (flagged by 1.2) forks.py:1297-1307, :1377-1390.

Scenario: kill_job sets kill_requested. Meanwhile claude exits on its own, and the supervisor enters finalize_job. taskkill /T lands between ledger_append and write_registry. The lock is then stale for 30 s. kill_job's own finalize_job breaks the lock, still sees running, and appends a second ledger line (ok, then killed). The window is milliseconds.

Why it matters later: Phase 2's usage would double-count the cost.

Fix: write the registry status before the ledger line (the registry is the once-only guard), and/or make usage dedupe by job.

M2. The stale-lock break has a race and a hot loop. forks.py:414-423.

• Two waiters can both judge the lock stale. One removes it and recreates it. The other's os.remove can then delete the new lock, because check-then-remove is not atomic. (Flagged; the window is very narrow.)
• The stale branch continues with no sleep and no deadline check. If os.remove keeps raising PermissionError (an antivirus or sync client holding the file, or a read-only attribute), acquire spins at 100 % CPU forever and the call never prints its JSON.

Fix:

• Add the deadline check and the sleep to that branch.
• Treat a lock as stale only when its holder's pid + ctime is dead (the content already records both), or when its age is far beyond any critical section.

M3. N1 and max_minutes use wall-clock time, so a host suspend kills every running fork as orphaned. forks.py:2155, :2173.

This run's log records two multi-hour clock jumps: 10:13→12:16 in Phase 0, and a 3.3 h span in 1.2. After a resume, each supervisor sees now - last_orchestrator_call_at > 1200 within 1 s and kills its tree before the Athanor's wait can touch the stamp. The same happens if the Athanor's own turn stalls for more than about 10 min between waits (API backoff).

Fix (cheap): if the supervisor's loop observes a tick gap far larger than SUP_TICK_S (say > 60 s), reset the orphan baseline to "now" and grant one more orphan_after.

Phase 3 text: say that kill/expire do not refresh the stamp.

M4. N3 leaves two gaps. forks.py:119 (CHILD_REFUSED), :2248, :2302.

• A fork may still run kill, expire --all and init. expire --all from a fork kills every sibling and itself.
• list and wait from a fork refresh last_orchestrator_call_at. A fork whose orchestrator died can therefore keep its own watchdog alive by polling list.

The build plan lists only where, list, usage and wait as allowed.

Fix:

• Refuse kill, expire and init under SPIRE_FORKS_CHILD.
• Skip the orchestrator stamp when the marker is set.

M5. The --permission-mode value set is stale against 2.1.299. forks.py:64. (Answers the 1.1 flag.)

2.1.299 offers acceptEdits, auto, bypassPermissions, manual, dontAsk, plan. The helper accepts default, which the binary does not list, and refuses manual, which the binary has. --effort matches. The default dontAsk is unaffected.

M6. The runtime fingerprint misses two prefix-relevant changes. forks.py:1027-1037.

• skills are not compared. The real system/init carries them (raw/p02/seq1.summary.json). Skill is in --tools, so the skill list is part of the prefix. A user or project skill added mid-run gives a cache miss with fingerprint: ok, which the build plan's fallback reads as a "fingerprint bug".
• Plugins without a version compare as name@. In the real init that is skill-creator and the built-ins. An in-place update of such a plugin is not seen.

Fix: add sorted skills, and use the plugin path's last segment when there is no version.

M7. The fingerprint hash contains the per-run mcp-empty.json path. forks.py:1676. (Answers the 1.1 flag.)

It does not affect the cache: the request carries no MCP servers either way. Within a run it is stable, including for a successor that keeps the same run log. The hash cannot be compared across runs, though, so it is no use for the trial's cross-run comparisons.

Fix: hash {"strict": true, "servers": {}} (the content) and keep the path outside the hash.

M8. The "no visible window" unit check is narrower than P0.1's detector. test_forks.py:1366-1371.

It only looks at windows whose pid is in the job's tree. P0.1's detector also flagged any visible ConsoleWindowClass / CASCADIA_HOSTING_WINDOW_CLASS window. That matters because the real Windows Terminal window belongs to WindowsTerminal.exe (pid 19872 in r3-wincontrol), not to the tree.

The tree-scoped check is not blind: in the positive control the supervisor itself owned a visible PseudoConsoleWindow. But the suite has no positive control of its own. It also hard-fails on a host without a desktop (assertGreater(len(all_windows), 0)).

Fix:

• Diff the visible Console/CASCADIA windows before and during the job, alongside the pid check.
• Skip, rather than fail, when no desktop is present.

M9. ANTHROPIC_BASE_URL sits on neither scrub list. forks.py:106-109. (Answers the 1.1 flag.)

On this host its value is the default https://api.anthropic.com (host part checked), so keeping it is harmless here. Elsewhere it is an auth/billing-path variable of the class design §4 drops. Dropping it would break hosts that need a gateway, though.

Fix:

• Put it on the keep list explicitly.
• Have where warn when it is set to anything but the default.
• Optionally record its value per base, so a change shows as drift.

M10. train refuses launch-field overrides, and the stated reason misreads the plan. forks.py:1504-1508, :1950, :2022.

1.2's note says "the plan's invariant and the digest do not" allow them. Plan §4.3 says the opposite: "Only train and init accept launch fields". Design §3.4 gives train "optional launch overrides".

Consequences:

• Every base in a run has the init config's fields.
• Forks build their argv from cfg["launch"], not from a per-base record. That is consistent today, because init refuses changes once a base exists.
• Phase 2's retrain_due reason "the requested model, cwd, tools, … differs from the base" has no input path.

The Phase 2 plan should decide this explicitly. It is not a defect for Phase 1.

M11. Note detection only checks that the file exists. forks.py:1239-1241. A second fork of the same block (a re-run after a failure), or a continue, reports notes/<block>.md even if this call did not write it. Compare the file's mtime against the job's start.

M12. kill reports two results the Phase 3 text must handle. forks.py:1396.

• {"killed": true, "status": "ok"} when the supervisor finished first, after kill_requested was set.
• Survivors give ok: false with exit 0.

The Phase 3 text must key on ok and status, not on killed or the exit code.

M13. Git Bash rooted paths that are not drive forms land on the cwd's drive. forks.py:252-253. (Answers the 1.1 flag.)

--run /tmp/x-run.md from Git Bash creates G:\tmp\x-run-forks without a word; Git Bash means %TEMP%. For --brief the failure is loud ("cannot read brief"), for --run it is silent.

Fix: use cygpath -w when it is on PATH, or refuse rooted paths that are not drive forms.

M14. POSIX kill does not reach claude. forks.py:2127, :703, :604-619.

• claude gets its own session (start_new_session=True), so killpg(<supervisor>) misses it. Only the per-pid sweep catches it.
• Where /proc is missing (macOS), process_table() returns [], so kill kills only the supervisor and leaves claude running.

The path is untested and declared so. SKILL.md (Phase 3) should say "Windows-tested; POSIX untested".

M15. C8 admission takes the union of the three settings files. forks.py:920-945.

The build plan (A14) noted that the merge of these files is unknown. P0.5 also showed that project permissions.allow is ignored in untrusted dirs. The smoke ran in the untrusted scratch clone and was admitted by the clone's project list, so whether project availableModels applies there is untested. The runtime warning check backstops it. Record the rule as "permissive at init, enforced at runtime" in SKILL.md.

M16. Smaller items.

• turns means a message count in jobs/<job>.json and num_turns in the result (flagged by 1.3): rename the job-file field.
• The post-exit sweep (:2183) also kills daemons a fork's commit may start (gpg-agent, git fsmonitor--daemon). They restart on demand, so this is acceptable, but SKILL.md should say it.
• continue sends the brief with no frame (:2049), so the Phase 3 text must tell the Athanor to put the commit rule and the note path into continue briefs.

---

Handoff flags, one by one

Phase 0 (handed over by Igor)

Flag	Answer
P0.6 started before P0.1 (c) finished	Accepted. (c) passed later (08:39–08:45). P0.6 ran on rung 3, which survived, so its result stands.
--no-hardlinks clone	Accepted: same content, and hardlinks cannot cross G:→C:.
Six tool-less calls outside the clone	A deviation from the letter of plan decision 4, with a real reason: trust, because untrusted dirs ignore project settings. With --tools "" they cannot edit anything. They left transcripts under the reliquary slug. No action. See M15 on what trust means for C8.
Separate scratch dirs for --agent and P0.8	Accepted. It kept P0.2's prefix clean. Its cost is that P0.8 ran untrusted (below).
(d) produced with TaskStop, because the Bash tool now backgrounds at timeout	Accepted. C2's timeout: 600000 rule stays. A2's failure mode is now "backgrounded, notification to the subagent". Recommend one line on it in the Phase 3 SKILL.md.
Effort case run inside P0.2	Accepted. Effort is kept in the fingerprint and is not cache-defining (182,202 read).
r3-main/r3-tkill ran PATH claude 2.1.283	Closed by the helper: claude_argv always uses the stored absolute path (:1801), and the env pins CLAUDE_CODE_EXECPATH (:788). The smoke shows 2.1.299 on all five streams.
P0.8 in an untrusted dir	Open for Phase 4, as planned. SKILL.md needs the allow-rule note.
What effort changes is unobservable	Accepted; informational.
A backgrounded command left an orphan	Addressed: the preamble forbids run_in_background (preamble.md:6), the supervisor sweeps descendants after exit (:2183), and kill/expire sweep the tree. See M16 for the sweep's breadth.
P0.6 ping read 0.916	Still a hit under C5, and the numbers match raw/p06/state.json. For Phase 2: the re-created part (7,463) is about the size of the shared system/tool prefix. On a small base (the smoke's 23,676) the same loss gives about 0.68, so a ping would read partial. The Phase 2 ping verdict must not treat that as a failed refresh.
Visible windows before the fix, and the retest	The fix is in the helper: CIM ShowWindow=0 (:141-145) and CREATE_NO_WINDOW on every child (:719, :2126). The smoke checked it by hand. The unit check is narrower than P0.1's detector (M8).
$P0\raw\p01\ not cleaned; r3-kill2 unobserved	No effect on the verdict (r3-kill5 passes). Accepted.
Clock jump	Relevant to N1: see M3.
Phase 0 open questions	U4 moot: confirmed by the P0.5 rows. Rung 3 with PowerShell: accepted by the coordinator, launch cost acceptable. Preamble line: done. Bash backgrounding: Phase 3 line.

Block 1.1

Flag / deviation	Answer
Argparse sets declared early	Fine; 1.2 kept them.
N3 guard in the dispatch	Fine. Gaps: M4.
--help as JSON, exit 0	Fine; keeps the stdout invariant. Unplanned (listed below).
Internal error → exit 2 with JSON	Fine. The contract has no other code for it.
Extra init flags; nested thresholds; binary_resolved_by, model_admission	Fine. They are excluded from the change comparison (:1720), which is correct.
where --binary and extra fields	Fine.
Two extra preamble lines	Fine. Both are within the contract's intent (deny list; headless). The preamble's pointer plugins/spire/skills/implementation/references/run-log.md will not resolve in other projects (the plugin lives in the cache); Phase 3 should give the format inline or by ${CLAUDE_PLUGIN_ROOT}.
Effort / permission-mode value sets	Effort matches 2.1.299. Permission modes do not: M5.
Per-run mcp path in the hash	M7 (cosmetic).
Comma split outside parentheses	Fine; tested (Bash(a,b)).
Race window when breaking a stale lock	Real but narrow; a hot loop sits next to it: M2.
Invalid settings skipped; [] admits nothing	Acceptable; the runtime check backstops it. M15.
ANTHROPIC_BASE_URL kept and listed unknown	M9. It is the default endpoint on this host.
Access denied treated as alive	Acceptable for liveness. Combined with dead-root walking it is part of the I2 risk class: a reused pid that cannot be opened keeps a job from being reaped as lost, and wait returns 3 until the Athanor kills it. Rare.
/tmp-style paths	M13.
Permissive model regex (claude-sonnet)	Acceptable. Claude Code would then fail the call, which I1 currently mislabels as model_refused.
preamble.md written in 1.1	Fine (init hashes it).

Block 1.2

Flag / deviation	Answer
Float --max-minutes on fork/continue	Fine.
Lost reaper before the cap check; it kills the leftover tree	Running it before the cap check is good. Killing the leftover tree is right in intent but unsafe for dead roots: I2.
Immediate kill on the restriction warning	Good: saves spend, and the result is still model_refused.
Descendant sweep after exit	Good, and safe at that moment (ms after exit). Breadth: M16.
No overrides on train	The stated reason misreads plan §4.3: M10.
wait --all "settled" semantics	Correct against C4: mixed case 3 → 4 → 0, every result delivered once, the failed job counted in the 4 without being returned twice. Traced and covered by test_wait_exit_codes.
Reason vocabulary; timeout has reason: null	Consistent with the digest (null unless failed/killed/lost). Fine.
Precedence of failure reasons	Wrong in one branch: I1.
Null usage without a result	Fine.
Train last_hit from its last event	Fine: the full prefix is written there.
Continue: no template, parent session = base, cost basis	Cost basis correct (smoke: Δ 0.0955 = 0.39614 − 0.30066). No template: M16. Moving the base's last_hit: I3.
Note detection	M11.
Failed-base name reuse	Fine.
kill of a finished job	Fine. Report shape: M12.
Kill/finalise race (stale lock, duplicate ledger line)	M1.
last_orchestrator_call_at only at wait start/end	Fine: the worst gap is 570 s plus the Athanor's turn. Suspend case: M3.
Stall measured on any stdout line	Fine.
pid-reuse guard untested	The guard works for a live reused pid. It fails for a dead root: I2.
spawn_failed only visible in wait	Acceptable: the next wait returns it at once with exit 4. Phase 3 text should mention it.
Not tested: POSIX, --agent drift, pid reuse	POSIX: M14. --agent drift: the code path (:1789-1795) mirrors the tested preamble path, so it is low risk. pid reuse: I2.

Block 1.3

Flag	Answer
turns has two meanings	M16.
continue cache verdict against the base context; parent_session = base	The verdict is defensible (it reads at least the base prefix). The real problem is last_hit: I3.
Survival across the Athanor's turn end is untested	Still open. Parentage WmiPrvSE.exe (smoke, unit test) makes a tree-kill from the agent impossible, so the expectation holds, but it is unverified. Phase 4.
Small smoke base	Accepted. P0.2 covered 182k without the helper. The helper's arithmetic is the same (RuleTests uses the P0.2 seq-1 numbers).

The contract digest (Athanor-written), checked against the build plan

Consistent with the build plan on C1, C3(a–c), C4, C5, C6 (the CIM command matches the preflight), C7, N1, N2, N3, A18 and A19. Where it diverges, or decides something the plan left open:

• Model refused. It loosens the preflight's rule. This is the source of I1.
• Warmth. "last_hit … any of its forks" does not carry over design §5's "first request of a fork only". This is the source of I3.
• C3(b). It adds kill as a second writer of jobs/<job>.json when the supervisor is dead. Harmless (the supervisor is dead then), but it is a deviation from C3(b)'s "only writer".
• C8. It turns P0.5's rule into a union over three files without saying so. M15.
• train launch fields. It silently drops design §3.4's optional overrides on train. 1.2 then cited the plan for the refusal. M10.
• Additions with a stated source. The digest adds CLAUDE_CODE_EXECPATH and SPIRE_FORKS_CHILD as pinned vars, the raw/<job>.env.json sidecar, and the preamble content. Each traces to a coordinator decision or the preflight report. The env file holds CLAUDE_CODE_USER_EMAIL, account and organisation UUIDs. It is under raw/ and so gitignored. Fine.

---

Tests that cannot fail, weakened checks

• No assertion was loosened between 1.1 and 1.2.
• The launch tests are real: the CIM spawn, a separate supervisor process, and the real taskkill and sweep. They can fail.
• The orphan test injects the clock only into the stamp. The supervisor compares against its real clock, so it really exercises N1.
• test_lost kills a real supervisor.
• The stub paths hide one real behaviour. The stub's error directive bills the requested model, so the empty-delta error path behind I1 is never exercised. The stub claude is also a .cmd (arguments pass through cmd.exe), while the real binary is an .exe. That makes the tests stricter on quoting, not weaker.
• Weaker check: the visible-window assertion has no positive control and is pid-scoped (M8).
• Untested: the tree walk under pid reuse (I2) and the kill/finalise race (M1).

What was built that the plan did not ask for (on record, not findings)

• Output and errors: --help as a JSON object; internal errors as JSON with exit 2.
• where: --binary; and python.supervisor_interpreter, platform, scrub.drop_patterns, scrub.keep_rule in its output.
• init: --ttl-s, --stall-after-s and one flag per threshold; the config fields binary_resolved_by and model_admission.
• Tool lists: comma splitting outside parentheses for --allowed-tools / --disallowed-tools.
• Preamble: the lines "nobody can answer a question mid-run" and "never push, rebase or reset".
• Lost reaper: it also runs in fork/continue, and it kills the leftover claude tree.
• Model restriction: an immediate kill on the warning.
• Supervisor sweep: descendants of claude are swept after a normal exit.
• wait: "delivered" and "settled" marks (delivered_at, settled_at in the registry); --any semantics.
• kill: returns snapshot, survivors, taskkill_rc, ok.
• list: returns lost_now and per-base rebase_due/retrain_due/reasons.
• Registry job fields: reason_detail, forced, brief_source, warm_until_at_launch, head_at_launch, kill_requested.
• fork: --max-minutes accepts a float.
• Train template: forbids code changes during training.
• Tests: a module-end leftover-process check (tearDownModule), done by command line through CIM.

Open, not findings

• Supervisor survival past the Athanor's hand-back (Phase 4).
• A trusted-workspace P0.8 (Phase 4).
• A large base through the helper.
• stalled and lost with a real claude: so far only with the stub.