Files
reliquary/docs/workpad/reports/2026-10-16-18-headless-forks-phase2-review.md
T
2026-10-16 17:57:28 +02:00

24 KiB
Raw Blame History

Phase 2 review: headless forks (bookkeeping)

  • Reviewer model id: claude-opus-5-5[1m]. Fresh: did not plan, build or fix this phase.
  • Date: 2026-10-16 17:55 +0200 (date)
  • Scope: Phase 2, commits 571a3d6..3b1de6c on top of Phase 1 (60fdea0). Code fff43a0 (2.1), 5fda2e2 (2.2), 4b92b51 (2.3); the rest is docs. Inputs: the build plan §4 and §6 Phase 2, the plan §4 and §7 Phase 2, design §5–§6, the Phase 2 contract digest, the run log's Phase 2 sections, and the Phase 1 re-check.
  • Verdict: the phase works as built and its smoke numbers check out against the raw streams. There are 0 Critical, 2 Important and 14 Minor findings. Both Important findings are contract gaps that the Athanor-written digest left open: what delta_tokens counts, and which phase a ping belongs to.

Gates I ran

Gate Result
G-unit uv run plugins/spire/skills/forks/tests/test_forks.py (run once) exit 0, Ran 130 tests in 130.444s, OK, no skips. One ResourceWarning (unclosed BufferedReader, from test_tick_runs_in_wait_and_supervisor, test_forks.py:2963)
G-where uv run plugins/spire/skills/forks/forks.py where exit 0. stdout is one line holding one JSON object: ok: true, spawn: cim-hidden, binary 2.1.299 via CLAUDE_CODE_EXECPATH, warnings: []
Smoke figures against the raw streams Script over …\headless-forks-p0\smoke2\…-forks\. Checked for all 7 jobs: ledger tokens = result tokens = the raw result.usage. cost_delta_usd = raw total_cost_usd − entry cost_before. First-event cache_read against base_context gives the stated verdicts. Phase sums are 1: $0.309559 and 2: $0.145025. Pings (phase null) come to $0.013911. Ledger total $0.468495. The run log's figures match.
delta_tokens probe listed_notes on a brief that names the build plan, the run log and plugins/spire/agents/athanor.md (I1)
Leftover processes (17:54, by command line: forks-test-, _supervise, poscontrol) none
Commit hygiene 4b92b51..3b1de6c touches only the run log, so no code changed after the phase-end G-unit run. There are no attribution trailers in 60fdea0..3b1de6c.

I made no live claude calls.

Important

I1. delta_tokens counts every .md file a brief names, and no rebase can clear it

forks.py:2302-2321 (_NOTE_REF, listed_notes) and :2484-2486 (due_flags).

  • What it counts. "Notes listed in the brief" is implemented as every token ending in .md that resolves to an existing file, first against the base's cwd and then against the state dir. That includes the build plan, the run log, the design, and files the fork is told to edit.
  • Probe. A brief that names docs/workpad/plans/2026-10-16-02-headless-forks-build.md, the run log and plugins/spire/agents/athanor.md gives 124,721 chars, which is delta_tokens:31180 against a threshold of 12,000. The run log alone is 84,641 chars, or 21k tokens.
  • Failure scenario. The Athanor's briefs normally cite the build plan and the run log. Every fork then returns rebase_due: true. The reason does not depend on the base, so after rebase --as B2 the next fork with the same kind of brief reports it again. The procedure obeys flags ("flags, not judgment"; design §5 says to rebase before forking), so each fork is preceded by a rebase. After four of them, retrain_due: rebases:4 forces a re-train.
  • Fix direction.
    • Count only handoff notes: files under the state dir's notes/, or notes whose mtime is after the base's trained_at. Never count files the brief tells the fork to edit.
    • Pin the rule in the Phase 3 text.
    • Add a test where a brief names a large non-note document and the flag stays off.
  • Digest. The digest repeats the design's phrase "notes listed in the brief" without defining it. The builder flagged the reading (2.2).

I2. Pings have phase: null, so no phase table ever shows them

forks.py:2951-2958 (the ping entry carries no phase or block) and :3573-3575 (usage --phase filters on equality).

  • What the contract says. Design §6 has the phase close carry usage --phase <n> --format md with "per-kind totals (train, rebase, fork, continue, ping)". Plan 2.3's done-when asks for "per-kind totals … and a priced total".
  • What happens. A ping row can never appear in a phase table, and the phase's priced total leaves out keep-warm cost.
  • Smoke evidence.
    • Phase 1 is $0.3096 and phase 2 is $0.1450, together $0.4546.
    • The ledger holds $0.4685.
    • The $0.0139 of the two pings appears in no phase table. Only the whole-run usage shows it.
    • On a 250k base each ping is about 0.1 × prefix, so a multi-hour phase under-reports by several pings.
  • Fix direction. A ping takes its base's phase: base.phase, or else the highest phase of the base's forks. A simpler alternative: --phase N also includes pings on bases whose phase is N.
  • Digest. The digest leaves the ping's phase open. The builder flagged this (2.1 "block/phase null", 2.4 first flag).

Minor

  1. A re-train never supersedes the old base (cmd_train, forks.py:2588-2627). Plan §4.3's state machine says active ─► superseded (rebase / re-train), but nothing in the helper sets superseded on a re-train.
    • The abandoned base stays active and can be forked without --force.
    • The ping rule's activity condition is run-wide (run_active, :2864-2878), so the forks of the new base keep pinging the old one, up to break_even pings.
    • The cost is bounded at about 1.9 × the old prefix, which is one cold start.
    • Fix: train --supersedes <base>, or have the Phase 3 text expire the old base.
    • To the brief's question: superseded bases are never pinged (status gate, :2893), and a ping already in flight when a base is superseded ends normally and is not followed by another.
  2. --warm-margin-s is not checked against the TTL (:1355-1356, :1881). When margin ≥ TTL, a base is expiring right after every hit. A ping then fires on every tick, every 30 s from wait, until the cap.
    • Probe: a base with inferred TTL 300 and warm_margin_s 600 reads expiring 1 s after its hit.
    • This happens without anyone choosing it when a margin tuned for 1 h meets a 5-minute downgrade.
    • The cap bounds the waste at about 1.9 × prefix per real call.
    • Fix: cap the margin below the TTL, for example at ttl/2.
  3. A failed or missed ping is retried on the next tick with no backoff (:1525-1528, :2881-2907).
    • Example: a usage-limit 429. Each ping clears ping_in_flight, leaves last_hit alone, and the next tick launches again: a CIM spawn plus claude every 30–60 s through the expiring window, up to break_even.
    • Counting failed pings toward the cap is what bounds this. That is good, but a short backoff after ping_missed would be better.
  4. A partial ping that re-wrote most of the prefix carries no flag (:1518-1528). A ping that read only the shared system prefix (P0.6's case) is really a cold write of the base. cold_start is set for fork, continue and rebase only, so usage's cold_starts misses it. The next fork then shows a clean hit.
  5. A continue's cold_start follows the base's warmth, which a continue never refreshes (I3) (:1518-1520, :2738).
    • Scenario: a continue reads its fork's own warm prefix and gets hit, but the base went cold meanwhile (pings off, or a 5 m TTL). The continue is still flagged cold_start and counted in usage.
    • The rule dates from Phase 1. It now feeds the Phase 2 totals.
  6. The supervisor's synchronous tick can break M3's bound on the loop body (:3171-3173 against SupClock.tick, :3010-3019).
    • The Phase 1 re-check accepted CLOCK_JUMP_S = 60 s because the loop body stays under about 45 s.
    • A tick now adds a claude --version (60 s timeout), a lock wait (up to 45 s), the CIM spawn (120 s timeout) and a second lock.
    • A slow tick then reads as a host suspend:
      • a spurious clock_jump flag;
      • the gap is excluded from max_minutes;
      • the orphan watchdog is re-armed.
    • It is unlikely, but cheap to avoid: run the tick in a thread, or credit tick time back to the clock.
  7. The hand-back seam with pings (:3243-3245, :3252-3253, :3337-3340).
    • wait --all excludes pings, and wait <ping> exits 2, so no command waits on a ping.
    • list.running includes pings, though.
    • The last wait --all before hand-back ticks at its start. It can launch a ping and still return 0, and list then shows a running job. The procedure's "nothing running at hand-back" and plan 4.2's trial criterion would read that as a leak.
    • The Phase 3 text must say how to treat pings there (ignore them: they end within PING_MAX_S), or list should report them separately.
  8. init after bases exist (:2146-2177). The question was whether it can silently change a live base's fingerprint. No.
    • Every base launches from its own launch record (M10).
    • A record-less base from before 2.2 is backfilled from the old config before the config is replaced.
    • fingerprint hashes are never compared, only recorded.
    • The concern is elsewhere: every init flag has a default, so a partial re-run (as the smoke's init --warm-margin-s 3500) silently reverts any omitted run-wide knob: pings, parallel_cap, thresholds, ttl_s, warm_margin_s, orphan_after. The exit is 0, with the reverts listed in changed_fields.
    • This is a refusal downgraded by the digest, not by the build plan. It is acceptable for launch fields, but the Phase 3 text should forbid re-running init (A7 already says so for successors). Alternatively, init could keep omitted values.
  9. A test that cannot fail for its stated reason. UsageTests.test_r1_waits_for_the_registry_lock (test_forks.py:2187-2202) asserts that usage blocks on a held lock. But cmd_usage takes the lock in touch_orchestrator (forks.py:3565) before reconcile_ledger, so a reconcile that took no lock would pass too.
    • Fix: call reconcile_ledger directly, as test_r1_nothing_missing_takes_no_lock does, or set SPIRE_FORKS_CHILD for the call.
  10. The ResourceWarning, confirmed (handoff flag). Supervisor.run opens claude with stdout=PIPE (:3103-3105) and never closes p.stdout after reader.join (:3181-3185).
    • In the detached supervisor this is harmless: the process exits right after.
    • In-process it leaks a handle, which is what the test run shows.
    • Fix: close p.stdout after the join when the reader has ended.
  11. R1 can leave the ledger and the results disagreeing (no double count). Scenario: a result file exists but its registry entry still reads running (a kill between the result write and the registry write). reconcile_ledger (:3410-3444) appends its line from that result. A later reap_lost or kill then finalises the entry again (:1638-1662), overwrites the result with lost/killed, and the ledger append is skipped.
    • The ledger keeps the first status. Tokens and cost come from the same stream, so totals do not drift.
    • Idempotency itself holds:
      • the fast path takes no lock;
      • the slow path re-reads the ledger under the lock;
      • ledger_append dedupes on job id;
      • a second usage adds nothing (tested).
  12. train --tools does not re-derive allowed_tools (:2570-2573; the test codifies it at test_forks.py:3120). init derives allowed = tools. Under dontAsk, a tool added by a train --tools override is therefore denied, and the result reads needs_attention.
  13. Two sequential forks can run at once under parallel_cap 2. Their head_at_end ranges then overlap, so each Commit cell can list the other's commits (:3516-3525). The builder flagged it. The Phase 3 text should run sequential forks one at a time.
  14. Cosmetic or untested.
    • list computes warmth for superseded and retired bases. The smoke's superseded B1 shows expiring (:3326).
    • Untested: a ping launched by a supervisor's own tick, end to end (the env and interpreter path read correct); defining: for fields other than model; rebase_conflict. The last one is unreachable in practice: train refuses a pending name and rebase refuses an existing one.
    • The M8 desktop-wide diff fails if the user opens a console during the ~1 s sample.

The brief's hunt list

  • TTL inference.
    • infer_ttl gives 3600 when 1 h tokens were written, 300 when only 5 m tokens were written, and otherwise keeps the previous value, falling back to the config. It runs after every call with a result, ping and continue included.
    • The deviation from the digest (a read-only call keeps 300 instead of reverting to 3600) is right: reverting would show a 5-minute base as warm for an hour. It recovers on the next 1 h write.
  • Does a ping refresh the prefix it claims to? Yes.
    • Its argv is the base's record plus --resume <base> --fork-session --session-id <new> --max-turns 1. That request reads the base prefix.
    • Smoke: j0006 read 25,277 of B2's 25,279. j0007 read 25,719, which includes j0006's identical suffix.
    • last_hit is set to the ping's first event at finalise, only for an ok ping with hit or partial.
    • Caveat: Minor 4.
  • The cap math.
    • break_even is computed in integer hundredths: (200−10)//10 = 19 and (125−10)//10 = 11. Correct, and tested.
    • Pings since the last real call are counted by job number. Every status counts, and the count is per base. A new base created by a rebase counts its pings from creation, because the rebase job belongs to the old base.
  • A ping storm from concurrent tickers. Not possible.
    • The decision, the job entry and ping_in_flight are written in one locked write.
    • The unlocked first look is redone under the lock.
    • The finaliser clears the mark and sets last_hit in one write.
    • test_two_tickers_one_ping forces both tickers past the unlocked look with a barrier, so it would fail without the locked re-check.
    • What remains is retry without backoff (Minor 3) and margin ≥ TTL (Minor 2).
  • Cost leaks from pings on superseded bases. None, because of the status gate. The leak is through bases a re-train abandons (Minor 1).
  • Rebase, old/new switch.
    • The new base is created only when the job is ok, in _rebase_succeeded (:1599-1636).
    • On failed, killed, timeout or lost, B stays active and NEW never exists. This is tested for is_error and kill.
    • Smoke: the new base's cost_cum_usd is the rebase's cumulative total, $0.2435012, and that is exactly cost_before of fork j0005.
  • last_hit on a failed rebase. It is refreshed from the rebase's first assistant event, both live (bump_last_hit) and at finalise. That is correct: the first request read B's prefix whatever happened afterwards. An error before any assistant event refreshes nothing.
  • init after bases exist. It cannot change a live base's fingerprint (Minor 8).
  • R1. Idempotent, and no double count (Minor 11 for the one inconsistency).
  • Usage numbers. They match the ledger, the result files and the raw result events for all 7 smoke calls. The md totals equal the sums.
  • M13 cygpath.
    • Correct: drive forms skip it, and /tmp/x goes through cygpath -w.
    • With no cygpath, or a failing one, the command exits 2 naming the path, and nothing is written on the cwd's drive (tested end to end through init --run /tmp/…).
    • //server paths are untouched.
    • In contexts without Git Bash on PATH, such as a supervisor or PowerShell, rooted paths are refused rather than guessed. That is the right failure.

Weakened checks and tests that cannot fail

I went through every assertion line the Phase 2 diff removes from test_forks.py:

  • /tmp/x → G:\tmp\x was removed. M13 forbids that mapping, and two new tests cover the input.
  • test_changed_values_after_base_refused became …_allowed. This is the digest's specified change; the test still checks that the base keeps its record and that identical values stay byte-equal.
  • test_phase2_not_implemented was removed: the commands are implemented, and test_commands_need_init gains them.
  • The window assertions moved into test_no_visible_window, which is wider and has a positive control. The assertGreater(len(all_windows), 0) became a skip, as M8 specified.
  • The running[] key turns became messages (M16).
  • In test_train_fork_continue_end_to_end the note is now written after the launch (M11), with its assertions unchanged.

Nothing was loosened. The billed-error test (test_error_after_requested_model_billed) fails if the result reads model_refused. M8's positive control proves the detector sees a console window.

There is one test that cannot fail for its stated reason (Minor 9).

Handoff flags, one by one

# Block Flag Answer
1 digest Athanor-written. Check it against the build plan It is consistent with the build plan except in three places. (a) It puts rebase in parallel_cap (CAPPED_KINDS, forks.py:159), while plan §4.3's invariant names fork/continue. That is stricter and harmless, but the run log says "none against the build plan". (b) It leaves the ping's phase open (I2). (c) It leaves "notes listed in the brief" undefined (I1). Replacing init's refusal is the digest's own decision, not the coordinator's (Minor 8). The coordinator items R1, M6, M7, M8, M10, M11, M13, M16 and the billed-error test are all implemented as decided.
2 2.1 A read-only call keeps the previous TTL even when it is 300 Accepted (see the hunt list)
3 2.1 messages also in wait running[] and list running rows Accepted. The Phase 3 text must use messages there and turns in results
4 2.1 Pings use the base's record Accepted. 2.2 finished M10 for every launch
5 2.1 wait ticks at its start Accepted. Note Minor 7: the last wait before hand-back can launch a ping
6 2.1 Naming a ping job in wait exits 2 Accepted as such, but nothing can wait on a ping (Minor 7)
7 2.1 Activity = last_wait_at or a fork/continue in the last 600 s; a hand tick is not activity Matches design §5. Activity is run-wide (Minor 1)
8 2.1 Pings counted by job number, every status counts Accepted. Counting failures is what bounds Minor 3
9 2.1 break_even uses the 1 h ratio above 300 s Accepted
10 2.1 Ping wall limit 300 s; block/phase null The wall limit is fine. Phase null is I2
11 2.1 Binary check before the lock; a missing binary skips the base Accepted. Tested (binary_missing why, no job)
12 2.1 The supervisor's tick pauses its loop 1–3 s The worst case is much longer and breaks M3's bound (Minor 6)
13 2.1 No N3 check inside tick_pass Accepted. tick is refused in a fork, a fork's wait skips the tick, and the supervisor's ticks are the design's own
14 2.1 A supervisor ticks first after 60 s Accepted. wait covers short jobs
15 2.1 A ping's last_hit is set at ping end Accepted. ping_in_flight covers the gap. The timestamp is the first event, not the end
16 2.1 TTL is inferred from failed calls too Accepted: billed tokens are real writes
17 2.1 M8 positive control is minimised; the desktop diff can flake Accepted, Minor 14
18 2.1 ResourceWarning Confirmed, Minor 10
19 2.1 Untested: a ping from a supervisor's own tick Still untested (Minor 14). By reading, the env (child env, scrubbed) and interpreter path are right
20 2.2 The new base records the binary the rebase ran under Correct, tested (b4 against the current stub)
21 2.2 rebase returns train's shape plus parent Accepted
22 2.2 phase_boundary follows the digest's "or" literally Accepted. Repeating until a rebase fits design §5's "the first fork of a new phase comes from a rebased base"
23 2.2 A rebased base's phase is --phase, not inherited Accepted. Without --phase the base's own side is unknown, so the Phase 3 text should always pass --phase to rebase
24 2.2 delta_tokens counts the *.md paths in the brief I1
25 2.2 lines includes uncommitted changes Accepted (design: git diff --shortstat)
26 2.2 "Last call" for runtime_drift Accepted. A rebase clears it: the new base's reference is the rebase's own init
27 2.2 The defining: field set (effort and allow/deny lists are not defining) Matches design §5's list
28 2.2 Transcript read from the tail; only a synthetic test Accepted. It falls back to the stored context
29 2.2 list computes flags for active bases only Accepted. The warmth is still shown for others (Minor 14)
30 2.2 train re-validates the whole record Accepted. --tools without allowed_tools is Minor 12. There is still no way to clear the config's agent
31 2.2 A rebase's first event refreshes the old base's last_hit even if the rebase fails Correct (see the hunt list)
32 2.2 NEW's training cost is the rebase's cost delta; its forks' basis is the rebase's cumulative total Verified in the smoke (j0005.cost_before = 0.2435012)
33 2.2 Rebases are not activity Accepted. A wait loop over a rebase is activity anyway
34 2.2 M11 has 2 s slack Accepted
35 2.2 M7 hashes a constant empty server set Accepted. The helper writes that file itself
36 2.2 M13: paths in a brief that cygpath cannot convert are skipped Accepted
37 2.2 rebase_conflict untested Unreachable in practice (Minor 14)
38 2.2 Untested defining: fields, train --cwd/--agent/--binary, pre-2.2 bases Still untested. The code paths are shared with the tested model case
39 2.3 The whole-run md uses the phase layout Accepted
40 2.3 Killed/lost calls count as calls with null sums Accepted. Consistent between json and md
41 2.3 The md phase table omits zero-call kinds Accepted. The json always has all five kinds
42 2.3 R1 also reconciles a result whose registry entry still reads running No double count, but the ledger can keep a status the result no longer has (Minor 11)
43 2.3 usage stamps the orchestrator call (not from a fork) Accepted
44 2.3 head_at_end can include another fork's commit, so Commit is shown for sequential forks only Two sequential forks can still overlap (Minor 13)
45 2.3 Commit lists 7-character hashes Accepted
46 2.4 Pings are left out of usage --phase I2
47 2.4 The superseded B1 shows expiring Cosmetic, Minor 14. No ping went to it (status gate)
48 2.4 The partial-ping refresh was not reproduced live Accepted. The unit test covers it (test_ping_end_to_end_through_wait)
49 2.4 The smoke used init --warm-margin-s instead of a 54-minute wait Accepted for the smoke. The tunable needs a guard (Minor 2)

Built but not asked for (on record only)

  • init --warm-margin-s and the config key warm_margin_s. The digest added it for the smoke; it is not in the build plan.
  • The flag ping_missed. list base rows gain ttl_s, margin_s, ttl_from and ping_in_flight, and bases store ping_in_flight_at.
  • wait refuses ping job ids and ticks at its start. wait --all excludes pings.
  • messages in wait running[] and in list running rows. This changes the Phase 1 digest's shape.
  • rebase additions:
    • the options --max-minutes and --max-budget-usd;
    • the refusals "the base is being rebased", and on train, "a running rebase creates this base";
    • rebase_conflict;
    • parent in the return.
  • init output gains bases, and init backfills record-less bases.
  • Job-entry fields created_ts, head_at_end, runtime_drift, rebase_as and new_launch.
  • usage json additions:
    • reconciled, cache_hits, cache_verdicts and scope;
    • ledger lines marked "reconciled": true;
    • --format md written as UTF-8 bytes.
  • Test hooks CLAUDE_PROJECTS_DIR, TRANSCRIPT_TAIL_BYTES and find_cygpath.
  • Stub directives billed, ttl=5m, nover= and skill=, and the stub now honours --max-turns.