Files
reliquary/docs/workpad/reports/2026-10-16-19-headless-forks-phase2-recheck.md
2026-10-16 18:22:27 +02:00

15 KiB
Raw Permalink Blame History

Phase 2 re-check: headless forks (fix pass)

  • Reviewer model id: claude-opus-5-5[1m]. Fresh: did not plan, build, review or fix this phase.
  • Date: 2026-10-16 18:21 +0200 (date)
  • Scope: only the findings the fix pass took on from reports/2026-10-16-18-headless-forks-phase2-review.md: I1, I2 and Minors 1, 2, 8, 9, 10, 11. Fix commits d2d5104 (code and tests), ba463fc and 94c1d8c (run log, trunk notes), diffed against 9564c22. Also checked: the fixer's re-check flags, the new --delta-note and train --supersedes surfaces, and the changed tests for weakened assertions.
  • Verdict: all eight are resolved. No Critical or Important finding survives. The fixer's flags are acceptable as Minor. I found five new Minors in the new surfaces (N1–N5 below). None blocks Phase 3. Two of them need a line in the Phase 3 text.

Gates I ran

Gate Result
G-unit uv run plugins/spire/skills/forks/tests/test_forks.py (run once, 18:15–18:18) exit 1. Ran 137 tests in 150.191s, FAILED (failures=1). The failure is LaunchTests.test_refusals, test_forks.py:2870: kill returned ok: false, which means a process from the snapshot was still alive after KILL_VERIFY_S. Neither that test nor kill_job/kill_tree (forks.py:861-895, :1756-1786) is in the fix diff. Run alone, the test passed 3 of 3 times (3.8 s each). I count it as a load flake from Phase 1 code, outside this re-check (see "Outside scope"). There was no ResourceWarning line anywhere in the log.
G-where uv run plugins/spire/skills/forks/forks.py where exit 0. One JSON line: ok: true, spawn: cim-hidden, binary 2.1.299 via CLAUDE_CODE_EXECPATH, warnings: []
New and changed tests against the unfixed helper I ran the 13 touched tests in a scratch copy: forks.py, preamble.md and templates/ from 9564c22, plus the HEAD test file. Result: failures=9, errors=3. Each fails at the assertion for its finding (details below). test_tick_runs_in_wait_and_supervisor passes there but prints the ResourceWarning (Minor 10). test_ping_end_to_end_through_wait passes there too, as expected: it only moved its setup to a margin the new rule accepts.
Mutants of the fixed helper I made three mutants of the fixed helper and ran the matching test against each. All three were killed: (1) reconcile_ledger without the lock fails test_r1_waits_for_the_registry_lock ([['j0010']] != []). (2) new_to_base ignoring trained_at fails test_delta_tokens_counts_only_what_the_base_does_not_know at :3299. (3) Superseding a named base of any status fails test_train_supersedes at :3351 ('superseded' != 'failed').
Smoke state, copied to scratch, through the fixed helper usage exits 0, status_from_result: [] (no false positive on real data), reconciled: [], total $0.468495, unchanged. usage --phase 2 --format md gives the same $0.1450 as before. The two old pings keep phase: null, as flagged. list: B1 superseded, B2 active, margin_s 1800, which is the smoke's 3500 capped at 3600/2. No reasons.
Leftover processes (18:20, by command line: forks-test-, _supervise, poscontrol, claude-stub) none, apart from my own query
Commit hygiene Code changed only in d2d5104; ba463fc and 94c1d8c touch only the run log and trunk notes. No attribution trailers in 9564c22..HEAD.

I made no live claude calls.

Findings, one by one

I1. delta_tokens counts every .md file a brief names: resolved

  • What was done. forks.py:2360-2415 and :2573. _NOTE_REF and listed_notes are gone. delta_tokens is now chars/4 of three parts:
    • the delta header;
    • the --delta-note files whose mtime is after the base's trained_at (new_to_base, :2400);
    • the body of a ## Changes since base section (changes_section, :2370).
  • The review's failure scenario is gone. A brief that names the build plan, the run log and files to edit adds nothing. Because the mtime gate compares against trained_at, a rebase clears the reason. The review's three-part fix direction is met:
    • count only real deltas;
    • the rule is routed to the Phase 3 text (successor boot, run log :514);
    • a test where a large named document leaves the flag off (test_forks.py:3258, and :3195 in test_due_reasons_each_alone).
  • Test strength. On 9564c22, :3274 ("named only: nothing") fails, and so does :3195. The trained_at mutant fails at :3299.
  • Fixer flags:
    • Whole-second trained_at: acceptable. _iso truncates (isoformat(timespec="seconds"), forks.py:221), so trained_at is never later than the true end. A note written in the same second, before the end, counts as new. That is the safe side: at worst it raises a spurious advisory flag.
    • Reused ## Changes since base section: acceptable as Minor. It is a brief-writing rule, and it is routed to the Phase 3 text (run log :486, :514).

I2. Pings have phase: null: resolved

  • What was done. ping_phase (forks.py:2959) takes the phase of the latest train, fork, continue or rebase job on the base that has one. If none has one, it takes the base's own phase, else null. tick_pass records it on the entry (:3070), so the result, the ledger and usage --phase N all carry it.
  • Test strength. test_forks.py:1926 checks the fallbacks, the json kinds.ping, the md ping and total rows and Priced total, and that --phase 1 has no rows. On 9564c22 it errors (ping_phase does not exist). The md and json assertions would also fail if the phase were dropped at finalise.
  • Fixer flag (old pings keep null; a rebased base takes its forks' or its own phase): acceptable. Only the smoke's two pings are affected, as confirmed on the copy. The rule that rebase always gets --phase is already routed to the Phase 3 text.

Minor 1. A re-train never supersedes the old base: resolved

  • What was done. train --supersedes BASE (repeatable, :1917).
    • Unknown names and the base's own name are refused with exit 2, before and under the lock (:2686-2709).
    • On an ok training, each named base that is active becomes superseded with superseded_by and superseded_at (:1598-1604). A failed training changes nothing.
  • Test. test_forks.py:3321 covers the refusals, the failed training, the ok training, the ping decision status:superseded and fork-needs---force. The status-gate mutant is killed.
  • One new edge case is listed as N4.

Minor 2. --warm-margin-s is not checked against the TTL: resolved

  • What was done.
    • init refuses an effective margin ≥ the effective ttl_s with exit 2 and writes nothing (:2184-2187). This includes a re-run that lowers ttl_s under the current margin.
    • base_warmth caps a configured margin at the base's TTL/2 (:1357). The review's probe (TTL 300, margin 600) now reads warm with margin_s 150 (test_forks.py:1702), and the smoke copy shows 3500 → 1800.
  • Fixer flag (the smoke's --warm-margin-s 3500 acts as 1800, at most 1800 at 1 h): acceptable. It is in the successor-boot list.

Minor 8. init re-run reverts omitted settings: resolved

  • What was done.
    • Every init flag defaults to None. On a re-run, an omitted flag keeps its current value (build_init_config, :2121-2188).
    • The first init still needs --model; without it the exit is 2 with error: usage, as before.
    • test_forks.py:724 shows a --warm-margin-s-only re-run with changed_fields == ["warm_margin_s"], and launch fields, thresholds and run-wide knobs kept.
  • Fixer flags:
    • --tools alone keeps the old allowed_tools: acceptable as Minor. Note that this is a behaviour change: before the fix, a re-run with --tools re-derived allowed_tools. It now matches train (review Minor 12), and "pass --allowed-tools with --tools" is in the Phase 3 list.
    • Cannot clear warm_margin_s or the agent back to none: acceptable. "Never re-run init to reset" is in the Phase 3 list.
    • Exit 2 on a concurrent config change (:2208-2209): consistent. Every refusal in the CLI is a ForksError with the default EXIT_USAGE (2), and there is no separate "busy" code. Only init writes reg["config"] (:2228 is the only writer), so the check fires only on a concurrent init, not on a supervisor's registry write. This path is untested (N5).
  • One unflagged effect of the same change is listed as N3.

Minor 9. A test that cannot fail for its stated reason: resolved

test_forks.py:2310 now calls reconcile_ledger directly in a thread. It asserts that the line is absent while the lock is held, and present exactly once after. My lock-free mutant fails it. Stronger than before.

Minor 10. The ResourceWarning: resolved

Supervisor.run closes p.stdout after reader.join when the reader has ended (:3301-3303). My full run printed no warning line; the unfixed helper prints it for the same test. The pipe is left open when the reader outlives READER_JOIN_S (fixer flag). That is acceptable: the detached supervisor exits right after.

Minor 11. R1 ledger and results can disagree: resolved

  • What was done. The ledger stays append-only. usage takes status, flags and reason from results/<job>.json when they differ from the ledger line, keeps the ledger's tokens and cost, and lists the job in the new status_from_result (:3686-3704, :3721).
  • Why the comparison is sound. The ledger line and the result are built from the same status, flags and reason (:1538-1556), so a normal job never differs. The smoke copy confirms [].
  • Test. test_forks.py:2330.
  • Fixer flag (the md block table does not show status): acceptable.

The new surfaces

  • fork/rebase --delta-note PATH.
    • All refusals exit 2 with one JSON line, before any write:
      • a missing file gives cannot read delta note, the same as cannot read brief;
      • '' gives empty path;
      • a directory gives cannot read delta note after about 2 s of PermissionError retries.
    • Notes are read before the lock and before the job entry (:2746, :2877).
    • continue does not take the flag, so argparse gives error: usage, exit 2.
  • train --supersedes.
    • The refusal payload {"error": "unknown base", "bases": [...], "option"} matches expire's (:3481).
    • fork and rebase use base plus bases = the known list, so the same key means a different list there. Cosmetic.
    • The train return and its result do not say which bases were superseded; list shows it.

New Minors in the fix surfaces

  • N1. A # line inside a fenced code block ends the ## Changes since base section (forks.py:2367, :2377).
    • Probe: a section holding a bash fence with # comment yields only the text up to the comment ('A\n```bash').
    • Failure: a large delta written as shell snippets under-counts, so delta_tokens stays off. The flag is advisory and the brief still reaches the fork, so only the rebase advice is lost.
    • Fix: skip fenced blocks when looking for the next heading, or say in the Phase 3 text that the section must not contain level-1/2-looking lines.
  • N2. The mtime rule is a proxy for "the base knows it" (:2400-2404).
    • A note written before a rebase or training ended, but not given to it (not attached and not in its brief), counts as known. Example: a parallel fork's note lands during a rebase.
    • Failure: an under-count of the advisory flag only. The note is still put into the prompt in full.
    • Acceptable. The Phase 3 text should attach to a rebase every note written since the base's trained_at.
  • N3. An init re-run no longer re-resolves the binary (:2155-2156). Without --binary, a re-run keeps the stored path, where it used to follow $CLAUDE_CODE_EXECPATH.
    • The where output shows a versioned path (…\claude-code\2.1.299\…\claude.exe).
    • Failure: after a Claude Code update that leaves the old version on disk, a re-run (for example to change --pings) silently keeps 2.1.299 for new trains, and changed_fields shows nothing. If the old file is gone, the re-run exits 2 with claude binary not found and the old path, which is loud.
    • This is consistent with "omitted keeps", and the Phase 3 text forbids re-running init anyway. It should be recorded with the other Minor 8 flags.
  • N4. A rebase already running on a base that a re-train supersedes still creates an active child (_rebase_succeeded, :1612-1643, which does not check the old base's status).
    • Scenario: rebase --base B1 --as B1r is running, and train --base B3 --supersedes B1 ends ok first. When the rebase then ends ok:
      • B1r is created active and gets pings;
      • B1's superseded_by is overwritten to B1r.
    • The cost is bounded by the ping cap. It needs two overlapping lineage operations, so it is rare.
    • Fix: _rebase_succeeded creates the new base as superseded when the old one is no longer active, or the Phase 3 text says not to re-train while a rebase runs.
  • N5. Two new refusal paths are untested: init's "the config changed while init ran" (:2208-2209), and train --supersedes's re-check under the lock (:2709, effectively unreachable because bases are never deleted). By reading, both are correct.

Weakened checks and tests that cannot fail

I went through every assertion line the fix diff removes or changes in test_forks.py:

  • test_boundaries_1h_and_5m: the 5-minute case with margin 900 changed from "expiring from the hit" to the capped schedule (warm at 0 and 149 s, expiring at 150 and 299 s, cold at 300 s, margin_s 150). This is the specified change and it has more points than before. The 1 h cases are unchanged.
  • test_ping_rule_states_status_and_pings_off: "pings at T0+100" became "pings at T0+1800, and state:warm at T0+1799". Stricter: it now pins the boundary from both sides.
  • test_ping_end_to_end_through_wait: the margin went from 3600 to 1800 and the last hit from 100 s to 2000 s back. The base is in the same expiring state, and every assertion is unchanged.
  • test_due_reasons_each_alone: "a brief naming the big note is due" became "not due". The due case moved to --delta-note, and its three assertions are unchanged. This is the I1 change.
  • test_r1_waits_for_the_registry_lock: stronger (Minor 9).

Nothing was loosened. Every new test fails on 9564c22, or against a targeted mutant.

Built but not asked for (on record only)

These came from the fix pass beyond the review's literal fix directions:

  • the ## Changes since base section, alongside --delta-note (the Athanor built both options);
  • --delta-note on rebase, which carries the notes into the rebase prompt;
  • the ### Attached delta notes prompt section;
  • delta_notes on job entries;
  • supersedes on the training base;
  • status_from_result in the usage json;
  • init's concurrent-change refusal;
  • the change to a first init without --model, which is now a ForksError("usage") from the helper rather than from argparse (same shape, same exit).

Outside scope (on record)

  • test_refusals flaked once under full-suite load (test_forks.py:2870, kill ok: false), and passed 3 of 3 times alone.
    • The code involved is Phase 1 code that the fix did not touch.
    • It is the first failure of this test in the recorded runs: the Phase 2 close, the review and the fix pass were all green.
    • If it recurs, find out which snapshot pid survives KILL_VERIFY_S. For the product, it is a reminder that kill can report ok: false under load (the Phase 3 text already keys on ok and status).