diff --git a/docs/workpad/reports/2026-10-16-08-headless-forks-run.md b/docs/workpad/reports/2026-10-16-08-headless-forks-run.md index bc411f9..d54b8d0 100644 --- a/docs/workpad/reports/2026-10-16-08-headless-forks-run.md +++ b/docs/workpad/reports/2026-10-16-08-headless-forks-run.md @@ -193,3 +193,40 @@ Gate after 1.1 (Athanor, 12:29): G-unit `Ran 52 tests … OK`, exit 0; G-where e - Spend (Phase 1): live smoke $0.49 client estimate (`total_cost_usd` of the four completed calls' deltas; the killed fork reported none). Unit tests spend nothing (stub). Subagent tokens: 1.1 211,075; 1.2 337,053 (harness totals); the Athanor's own unknown. - Processes at hand-back: none from this phase (command-line sweep at 15:54, and the suite's own teardown check). - Status: provisional until review, fix pass and re-check close. + +### Phase 1 fix pass + +- Model: `claude-opus-5-5[1m]` (fixer, own tools, no agents dispatched) +- Start / end: Fri Oct 16 16:11:40 WEDT 2026 / Fri Oct 16 16:27:01 WEDT 2026 (`date`; this note and its commit followed) +- Review: `docs/workpad/reports/2026-10-16-16-headless-forks-phase1-review.md`. Fixed: I1, I2, I3, M1, M2, M3, M4, M5, M9. Not touched (deferred by the Athanor): M6, M7, M8, M10–M16. +- Done, per finding (each test fails on the unfixed `20b049e` helper; checked by running the new tests against it in a scratch copy: 18 failures, 11 errors, 0 passes): + - **I1** — `_conclude`: `model_refused` only on the restriction warning, or when `models_used` is non-empty and lacks the requested id. New `api_error_reason(result)` sets `usage_limit` (429, or "usage limit"/"rate limit"/"hit your limit"/"API Error: 429"), `overloaded` (529 / "overloaded"), `prompt_too_long`, `api_error` (any other status, or "API Error" text); `reason_detail` = `api_error_status : `. Precedence: model refused > no result > API error > `is_error` > non-zero exit. Stub: `error` now bills nothing (no assistant events, empty `modelUsage` delta); new `apierr=` and `errtext=`. Tests: `RuleTests.test_api_error_reason`, `LaunchTests.test_api_errors_are_not_model_refused` (each new reason end to end + substitution still `model_refused`), `LaunchTests.test_failures_flags_and_drift` (its `error` case is now the empty-delta path and still reads `is_error`). + - **I2** — `process_tree(rec, alive_until)`: for a dead root, a direct child is taken only if its creation time is after the root's recorded one and at most `alive_until` + 1 s (the job-file times are whole-second ISO). No recorded ctime or no `alive_until` → a dead root claims nothing. Below a claimed (live, pid + ctime verified) child the old rule applies. The live-root path is unchanged. New `epoch_to_ctime` (Windows FILETIME; POSIX `/proc/stat` btime × CLK_TCK) and `last_alive_at(rt)` (later of `heartbeat_at`, `ended_at`). `kill_tree` passes `alive_until` through; `reap_lost` and `kill_job` pass the job file's last-alive time; the supervisor's own calls pass `time.time()`. Tests: `ProcessTreeTests` (fake process table, `process_table`/`proc_ctime`/`pid_alive` patched, nothing real touched): `test_dead_root_claims_only_children_from_its_lifetime` (reused-pid child after the heartbeat and a pre-job process with the dead pid as stale ppid are not claimed, the genuine child and its later child are), `test_dead_root_without_bounds_claims_nothing`, `test_live_root_unchanged`, `test_reap_lost_bounds_by_heartbeat`. + - **I3** — only a fork's first assistant event (supervisor, `hit_written = kind != "fork"`) and the training's last event (`_conclude`) refresh the base's `last_hit`; a continue no longer does. Test: `LaunchTests.test_continue_leaves_base_last_hit` (also checks a following fork still refreshes it). + - **M3** — new `SupClock`: a tick gap > `CLOCK_JUMP_S` (60 s; the tick is 0.25 s) is a jump; gap minus one tick is left out of the `max_minutes` elapsed time, and the orphan watchdog counts from the later of the last orchestrator call and the jump (one more `orphan_after`). A jump is logged to `sup.log` (stderr) and adds flag `clock_jump`. Tests: `RuleTests.test_sup_clock_jump`; `LaunchTests.test_clock_jump_not_orphaned_or_timed_out` (a real `Supervisor` run in-process under an injected `_now` that jumps 2 h one second into a 3 s stub call, `orphan_after` 600, `--max-minutes 1`: ends `ok` with `clock_jump`, jump logged; the unfixed helper ends it `timeout`). + - **M2** — a lock is stale only when its holder pid + ctime is dead, or it is older than `LOCK_ABANDONED_S` = 300 s (every critical section is a few small JSON read-modify-writes: ms, at worst seconds of `PermissionError` retries; 300 s is two orders above that and only matters for a holder that cannot be judged). Breaking: rename the lock to a unique `registry..broken.lock`, delete it only if its content is the one judged stale, else put it back without overwriting (Windows `rename`, POSIX `link` + `unlink`). A failed break falls through to the deadline check and the sleep. `LOCK_STALE_S` is gone. Tests: `LockTests.test_unbreakable_stale_lock_respects_deadline` (rename/remove of the lock raise `PermissionError`: returns "registry locked" within the 0.5 s deadline; the unfixed helper spins), `test_fresh_lock_taken_during_break_survives` (a fresh lock swapped in at the moment of the break survives; no `.broken.lock` left), `test_dead_holder_lock_broken_at_once`, `test_abandoned_lock_broken_by_age`. + - **M1** — `finalize_job` writes the result, then the registry (the once-only guard), then the ledger line; `ledger_append` skips a job that already has a line (under the lock) and starts a new line after a torn last line. Test: `AtomicTests.test_ledger_append_dedupes_by_job`. + - **M4** — `CHILD_REFUSED` adds `kill`, `expire`, `init` (exit 2, message names the command and "inside a fork"). `stamp_orchestrator`/`touch_orchestrator` do nothing when `SPIRE_FORKS_CHILD` is set (all five stamp sites go through them). Tests: `CliTests.test_child_refuses_kill_expire_init`, `CliTests.test_child_does_not_stamp_orchestrator`. + - **M5** — `PERMISSION_MODES` = `acceptEdits, auto, bypassPermissions, manual, dontAsk, plan`; `plan` still refused; `default` is now "unknown permission mode" (its `allowed` list omits `plan`). Test: `InitTests.test_permission_modes_match_2_1_299`. + - **M9** — `ANTHROPIC_BASE_URL` on `SCRUB_KEEP`. `where` always carries `warnings` (list); one entry when the variable is set and, trailing slash and case ignored, not `https://api.anthropic.com`; exit stays 0. Test: `CliTests.test_where_warns_on_base_url` (unset, default, default with slash, a gateway URL). +- Behaviour changes to existing tests: + - `LockTests.test_stale_lock_broken` → `test_dead_holder_lock_broken_at_once`: the lock is fresh but its holder is dead (a dead-holder lock is now broken at once, no 30 s wait). The old fixture's `pid: 1` without ctime would be "alive" on POSIX. + - `test_garbage_stale_lock_broken_by_mtime` and `test_stale_under_injected_clock_and_safe_release`: the age bound is `LOCK_ABANDONED_S` (300 s) instead of `LOCK_STALE_S` (30 s). + - `test_fresh_lock_not_broken`: the holder is this live process (pid 1 is dead on Windows, which now means stale); it also checks a fresh garbage lock is not broken. + - `ScrubTests.test_unknown`: `ANTHROPIC_BASE_URL` is no longer in `scrub.unknown` (it is `keep`). + - Stub `error` directive: bills nothing (previously billed three turns to the requested model). `test_failures_flags_and_drift` expectations unchanged. + - The `timeout` `reason_detail` reads "run time over max_minutes" (was "wall time"); no test asserts it. +- Gates: + - G-unit `uv run plugins/spire/skills/forks/tests/test_forks.py` → exit 0, `Ran 86 tests in 68.942s` / `OK` (16:25:33–16:26:42; was 69 tests / 52 s). + - G-where `uv run plugins/spire/skills/forks/forks.py where` → exit 0, one line, one JSON object, `ok: true`, `spawn: cim-hidden`, binary 2.1.299 via `CLAUDE_CODE_EXECPATH`, `warnings: []`. + - Leftover processes: none (16:27, `Get-CimInstance Win32_Process` by command line: the tests' temp prefix, `_supervise`, the scratch copy of the old helper); also the suite's `tearDownModule`. + - No live `claude` calls (stub only). +- Flagged for the re-check: + - I2: children a dead root spawned in the last heartbeat interval (up to 10 s before it died) are not claimed. Missing a few is the price of never killing an unrelated process. The supervisor's post-exit sweep is unaffected (it passes `time.time()`). + - I2: on POSIX without `/proc/stat`, `epoch_to_ctime` is None, so a dead root claims nothing there. + - M2: one residual case. Waiter A renames a lock aside, finds a fresh lock L2 (not the stale one), and tries to put it back. If a third waiter C created a lock in that microsecond gap, the restore fails: L2 stays aside (logged, named `registry..broken.lock`, matched by `.gitignore`'s `*.lock`) and both L2's holder and C believe they hold the lock. The old code could delete a fresh lock outright in the two-waiter case. `release` is still check-then-remove (token read, then `os.remove`), as before; not in this pass. + - M3: the stall measure (`quiet`) still uses the wall clock, so a suspend also sets `stalled` (a flag, no kill). `wall_s` in the result stays wall time, suspend included. A backwards clock jump is not treated as a jump. + - M3: `kill`/`expire` still do not refresh the orchestrator stamp. That is for the Phase 3 text, as the review says. + - I1: the text patterns are a heuristic over `result`/`error`/`errors`; the status code wins when present. A result with `is_error: false` and no `api_error_status` is never classified. + - M9: an empty `ANTHROPIC_BASE_URL` counts as "set, not default" and warns. +- Commits: `7b9b748` (code and tests), then this note's commit.