From 20b049e400ed680eac7de7fd7cddbf045f775b2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabor=20K=C3=B6rber?= Date: Fri, 16 Oct 2026 16:10:16 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=9D=20Headless=20forks=20Phase=201=20r?= =?UTF-8?q?eview?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...6-10-16-16-headless-forks-phase1-review.md | 326 ++++++++++++++++++ 1 file changed, 326 insertions(+) create mode 100644 docs/workpad/reports/2026-10-16-16-headless-forks-phase1-review.md diff --git a/docs/workpad/reports/2026-10-16-16-headless-forks-phase1-review.md b/docs/workpad/reports/2026-10-16-16-headless-forks-phase1-review.md new file mode 100644 index 0000000..cc62480 --- /dev/null +++ b/docs/workpad/reports/2026-10-16-16-headless-forks-phase1-review.md @@ -0,0 +1,326 @@ +# 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` + +```python +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 `TerminateProcess`ed. + +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 `continue`s 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/.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()` 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/.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/.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/.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.