24 KiB
24 KiB
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..3b1de6con top of Phase 1 (60fdea0). Codefff43a0(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_tokenscounts, 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
.mdthat 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 andplugins/spire/agents/athanor.mdgives 124,721 chars, which isdelta_tokens:31180against 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 afterrebase --as B2the 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:4forces a re-train. - Fix direction.
- Count only handoff notes: files under the state dir's
notes/, or notes whose mtime is after the base'strained_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.
- Count only handoff notes: files under the state dir's
- 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 mdwith "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
usageshows 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 Nalso 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
- A re-train never supersedes the old base (
cmd_train,forks.py:2588-2627). Plan §4.3's state machine saysactive ─► superseded (rebase / re-train), but nothing in the helper setssupersededon a re-train.- The abandoned base stays
activeand 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 tobreak_evenpings. - 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 textexpirethe 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.
- The abandoned base stays
--warm-margin-sis not checked against the TTL (:1355-1356,:1881). When margin ≥ TTL, a base isexpiringright after every hit. A ping then fires on every tick, every 30 s fromwait, until the cap.- Probe: a base with inferred TTL 300 and
warm_margin_s600 readsexpiring1 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.
- Probe: a base with inferred TTL 300 and
- 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, leaveslast_hitalone, and the next tick launches again: a CIM spawn plusclaudeevery 30–60 s through the expiring window, up tobreak_even. - Counting failed pings toward the cap is what bounds this. That is good, but a short backoff after
ping_missedwould be better.
- Example: a usage-limit 429. Each ping clears
- A
partialping 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_startis set for fork, continue and rebase only, sousage'scold_startsmisses it. The next fork then shows a cleanhit. - A
continue'scold_startfollows 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 flaggedcold_startand counted inusage. - The rule dates from Phase 1. It now feeds the Phase 2 totals.
- Scenario: a continue reads its fork's own warm prefix and gets
- The supervisor's synchronous tick can break M3's bound on the loop body (
:3171-3173againstSupClock.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_jumpflag; - the gap is excluded from
max_minutes; - the orphan watchdog is re-armed.
- a spurious
- It is unlikely, but cheap to avoid: run the tick in a thread, or credit tick time back to the clock.
- The Phase 1 re-check accepted
- The hand-back seam with pings (
:3243-3245,:3252-3253,:3337-3340).wait --allexcludes pings, andwait <ping>exits 2, so no command waits on a ping.list.runningincludes pings, though.- The last
wait --allbefore hand-back ticks at its start. It can launch a ping and still return 0, andlistthen 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), orlistshould report them separately.
initafter bases exist (:2146-2177). The question was whether it can silently change a live base's fingerprint. No.- Every base launches from its own
launchrecord (M10). - A record-less base from before 2.2 is backfilled from the old config before the config is replaced.
fingerprinthashes are never compared, only recorded.- The concern is elsewhere: every
initflag has a default, so a partial re-run (as the smoke'sinit --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 inchanged_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,initcould keep omitted values.
- Every base launches from its own
- A test that cannot fail for its stated reason.
UsageTests.test_r1_waits_for_the_registry_lock(test_forks.py:2187-2202) asserts thatusageblocks on a held lock. Butcmd_usagetakes the lock intouch_orchestrator(forks.py:3565) beforereconcile_ledger, so a reconcile that took no lock would pass too.- Fix: call
reconcile_ledgerdirectly, astest_r1_nothing_missing_takes_no_lockdoes, or setSPIRE_FORKS_CHILDfor the call.
- Fix: call
- The
ResourceWarning, confirmed (handoff flag).Supervisor.runopensclaudewithstdout=PIPE(:3103-3105) and never closesp.stdoutafterreader.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.stdoutafter the join when the reader has ended.
- 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 laterreap_lostorkillthen finalises the entry again (:1638-1662), overwrites the result withlost/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_appenddedupes on job id;- a second
usageadds nothing (tested).
train --toolsdoes not re-deriveallowed_tools(:2570-2573; the test codifies it attest_forks.py:3120).initderivesallowed = tools. UnderdontAsk, a tool added by atrain --toolsoverride is therefore denied, and the result readsneeds_attention.- Two sequential forks can run at once under
parallel_cap2. Theirhead_at_endranges 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. - Cosmetic or untested.
listcomputes warmth for superseded and retired bases. The smoke's superseded B1 showsexpiring(: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 thanmodel;rebase_conflict. The last one is unreachable in practice:trainrefuses a pending name andrebaserefuses 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_ttlgives 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 aresult, 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:
j0006read 25,277 of B2's 25,279.j0007read 25,719, which includes j0006's identical suffix. last_hitis set to the ping's first event at finalise, only for anokping withhitorpartial.- Caveat: Minor 4.
- Its argv is the base's record plus
- The cap math.
break_evenis 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_flightare written in one locked write. - The unlocked first look is redone under the lock.
- The finaliser clears the mark and sets
last_hitin one write. test_two_tickers_one_pingforces 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).
- The decision, the job entry and
- 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,timeoutorlost, B staysactiveand NEW never exists. This is tested foris_errorand kill. - Smoke: the new base's
cost_cum_usdis the rebase's cumulative total, $0.2435012, and that is exactlycost_beforeof forkj0005.
- The new base is created only when the job is
last_hiton 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.initafter 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
resultevents for all 7 smoke calls. The md totals equal the sums. - M13
cygpath.- Correct: drive forms skip it, and
/tmp/xgoes throughcygpath -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 throughinit --run /tmp/…). //serverpaths 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.
- Correct: drive forms skip it, and
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\xwas removed. M13 forbids that mapping, and two new tests cover the input.test_changed_values_after_base_refusedbecame…_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_implementedwas removed: the commands are implemented, andtest_commands_need_initgains them.- The window assertions moved into
test_no_visible_window, which is wider and has a positive control. TheassertGreater(len(all_windows), 0)became a skip, as M8 specified. - The
running[]keyturnsbecamemessages(M16). - In
test_train_fork_continue_end_to_endthe 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-sand the config keywarm_margin_s. The digest added it for the smoke; it is not in the build plan.- The flag
ping_missed.listbase rows gainttl_s,margin_s,ttl_fromandping_in_flight, and bases storeping_in_flight_at. waitrefuses ping job ids and ticks at its start.wait --allexcludes pings.messagesinwaitrunning[]and inlistrunning rows. This changes the Phase 1 digest's shape.rebaseadditions:- the options
--max-minutesand--max-budget-usd; - the refusals "the base is being rebased", and on
train, "a running rebase creates this base"; rebase_conflict;parentin the return.
- the options
initoutput gainsbases, andinitbackfills record-less bases.- Job-entry fields
created_ts,head_at_end,runtime_drift,rebase_asandnew_launch. usagejson additions:reconciled,cache_hits,cache_verdictsandscope;- ledger lines marked
"reconciled": true; --format mdwritten as UTF-8 bytes.
- Test hooks
CLAUDE_PROJECTS_DIR,TRANSCRIPT_TAIL_BYTESandfind_cygpath. - Stub directives
billed,ttl=5m,nover=andskill=, and the stub now honours--max-turns.