Files
reliquary/docs/workpad/reports/2026-10-16-22-gremlins-code-verdict.md

16 KiB
Raw Permalink Blame History

Gremlins helper: code verdict

  • Date: 2026-10-16
  • Reviewer model: claude-opus-5-5
  • Timebox: 25 minutes, done by one agent with no sub-agents
  • Target: .claude/worktrees/headless-forks/plugins/spire/skills/gremlins/ on branch headless-forks at ab1d945
    • gremlins.py: 4,155 lines
    • tests/test_gremlins.py: 4,235 lines, about 180 tests
    • SKILL.md: 120 lines
    • preamble.md and three templates: 69 lines in total

All line references below point to gremlins.py unless another file is named.

1. Verdict

The code is careful, consistent and well tested, and it works: the live happy path ran cleanly, with a cache hit on the fork. It is still over-built by about four to five times. Roughly two thirds of the lines run and police detached processes: a PowerShell/CIM detach, heartbeats, reaping of lost jobs, an orphan watchdog, clock-jump detection, a tree kill that guards against pid reuse, and breaking of stale locks. The rest is mostly a layer of heuristic rules (pings, rebase and re-train thresholds, fingerprint drift) that the requirement never asked for.

Rewrite it small, keeping the core. Do not throw it away. Keep the stream parsing, the usage and cache accounting, the env scrub, the argv builder and the templates. Replace the supervisor stack with one foreground call that the orchestrator puts in the background through the Bash tool.

2. What it contains

Part Lines Rough size
Constants and tunables (about 60 named constants, many "so tests can shorten them") 1–178 180
Small helpers: emit, clock, ISO times, sha, fingerprint hash 180–265 85
Platform seam: Platform base, WindowsPlatform (ctypes, CIM detach, Toolhelp32, taskkill, EnumWindows), PosixPlatform (gated off) 267–734 470
Platform gate and state-dir layout 737–822 85
Atomic IO with retries, RegistryLock (breaks stale locks), ledger append 824–1060 235
Process records (pid + ctime), process_tree, kill_tree, snapshot_survivor, git 1062–1228 170
Env scrub, binary resolution, where sub-reports, model admission 1230–1452 225
StreamState, runtime fingerprint, usage_from_result, API-error mapping, cache verdict, TTL, warmth 1454–1691 240
Job finalising: _conclude, _rebase_succeeded, finalize_job, orchestrator stamps, reap_lost, kill_job 1693–2107 415
argparse CLI 2109–2313 205
where and init (build_launch, binary re-resolution, config diff) 2315–2598 285
Launch helpers: brief, binary check, argv, delta header 2599–2700 100
Rebase and re-train heuristics: git delta counts, ## Changes parsing, transcript tail read, drift checks, due_flags 2700–2957 260
train, spawn, continue, rebase commands 2959–3291 335
Keep-warm pings and tick 3293–3476 185
SupClock and Supervisor (the detached _supervise process) 3478–3705 230
wait, list, kill, expire 3707–3863 158
usage (ledger reconcile, aggregate, markdown tables) 3865–4096 230
Dispatch and main 4097–4155 60

The code makes about 100 references to review-finding IDs (M7, N4, C16, I2, A19, Minor 13 and others; 49 distinct) and about 30 to "design N", "plan N" or "digest §N". The code reads like layers of review fixes, and those IDs mean nothing without the old review documents.

3. Keep (as it is, or nearly)

  • StreamState (1459–1522). This is a small, correct stream-json reader: init event, first and last usage, result, and the model-restriction warning.
  • usage_from_result (1573–1601). It computes tokens with the 1h/5m split, models_used from the modelUsage delta, and the cost delta against the session's cumulative total. This is the real know-how for usage tracking on forks.
  • cache_verdict, infer_ttl, wrote_only_5m, base_warmth (1640–1690). This is exactly the "is the base's cache still warm" requirement, in about 50 lines.
  • api_error_reason (1604–1637). It is a table-driven mapping of failure reasons and is already shaped well.
  • The SCRUB_* constants and scrubbed_env (89–114, 1235–1282). They hold the empirically gathered list of Desktop and SDK env vars that must not leak into a child claude -p. This is hard-won knowledge.
  • claude_argv (2652–2666) and the fork argv: --resume <base> --fork-session --session-id <uuid>.
  • Small helpers: atomic_write_bytes and _retry (829–866), which handle Windows sharing violations.
  • templates/*.md and preamble.md. These are short and to the point. The default sequential commit_rule (3152) says "This gremlin commits" even when the brief says not to. The smoke gremlin noticed the conflict and followed the brief. Make the rule an explicit flag.
  • Ledger plus the usage --format md tables (3877–4096), but trimmed by half (see below).

4. Delete or simplify

4a. The detached supervisor and everything that exists because of it

The whole stack exists so that launches can return at once (SKILL.md:70). It is then needed to stop jobs from leaking once nobody watches them. The Bash tool already moves a call past its timeout into the background and sends a completion notice, and run_in_background does the same on purpose. The orchestrator can run gremlins.py spawn … as a blocking call in the background and read the result JSON from the notice. Without detach, the following code goes away:

  • Detach. WindowsPlatform.detach with its PowerShell/CIM hidden window (356–362, 476–495), detach_supervisor (1745), supervisor_interpreter (1398), and the detach_failed reason.
  • The supervisor. Supervisor (3517–3684) and cmd_supervise (3687–3704). The run loop polls every 0.25 s and handles heartbeat, stall, orphan check, keep-warm tick and max time in one while loop (3613–3659). os.environ.clear() (3579) mutates its own process environment to reach the child's PATH.
  • SupClock (3481–3514). It detects host suspend so that suspends are not counted toward max_minutes and so the orphan watchdog can be re-armed. This is a special case that exists only to patch another special case (the watchdog).
  • The orphan watchdog (3639–3651) and the stamps behind it: stamp_orchestrator and touch_orchestrator (2009–2030). in_child checks guard both functions, so a gremlin cannot keep its own watchdog alive (that is the M4 fix). Again, this is a patch on a patch.
  • Heartbeats and reaping. reap_lost, last_alive_at, LOST_AFTER_S and the lost status (2040–2073).
  • wait (3720–3790). Its delivered_at/settled_at double bookkeeping, --any, exit codes 3 and 4, and the 540/570 s caps all work around the Bash timeout. With completion notices there is nothing left to wait on. A plain result <job> read would do.
  • The pid-reuse-safe tree kill. process_tree uses dead-root bounds and ALIVE_UNTIL_SLACK_S (1099–1151). snapshot_survivor is "C16 as narrowed by A2" (1192–1205). kill_tree sweeps and verifies for 10 s (1161–1189). If claude is a child of the foreground helper, taskkill /T /F /PID <child> on Windows (or killpg on POSIX) while the helper is alive covers it. Python's subprocess with a Job Object is the robust variant.
  • RegistryLock (881–1005, 125 lines). It breaks stale locks by renaming the lock aside and putting it back if the content changed. Platform.restore_no_replace exists only for this. Writes per job (one jobs/<id>.json per call) plus append-only ledger lines mostly remove the shared read-modify-write. What remains needs, at most, a 20-line O_EXCL lock with a pid check.
  • About 60 % of the platform seam. Platform has 14 abstract methods for one tested platform. PosixPlatform (604–727) is 125 lines behind a closed gate, plus UNSUPPORTED_OK_ENV (127–129, 745). visible_windows (338, 583–601) is used only by tests. Without detach the real differences come down to path normalisation and the kill call.

4b. Heuristics nobody asked for

  • The ping machinery (3293–3476, 185 lines). It covers pings_since_real_call, ping_phase, run_active with its 10-minute activity window, a break_even cap computed from price ratios, and ping_in_flight marks. tick_pass decides twice, outside and inside the lock, and so needs a special why="eligible_since_this_tick_began" (3424). Every supervisor ticks every 60 s and every wait every 30 s. With a 1 h TTL, list already reports warm_until. A plain ping --base B, called when the orchestrator sees expiring, is about 20 lines.

  • The rebase and re-train flags. due_flags (2910–2956) combines five thresholds (rebase_commits, rebase_lines, rebase_delta_tokens, max_base_tokens, max_rebases) with defining_drift (2859–2888), binary_state drift by version, size and mtime (2891–2907), runtime drift from the init event (1548–1570, 2847), and a transcript tail read (2778–2830). They are all advisory (the docstring says only binary_missing stops a launch), yet each one is an init flag (2217–2218). Keep phase_boundary and max_base_tokens, and drop the rest.

  • The _conclude god function (1752–1904, 150 lines). It mixes four things:

    • the status decision: the if-chain at 1771–1790, which should be a classify(ss, exit_code) function returning (status, reason, detail)
    • eight flag rules (1793–1824), which should be a table of (flag, predicate)
    • two near-duplicate dict literals, the result and the ledger line (1833–1851)
    • the base state machine for train, rebase, ping and supersede (1867–1903)

    Split it into classify, a flags table, record(), and a separate base-transition function.

  • The race rule for "rebase finishes after a retrain superseded it". _rebase_target_free, the old_status branch of _rebase_succeeded and the parent_inactive flag (1819–1824, 1907–1963) handle it. Refusing train --supersedes X while a rebase of X runs would replace all of this with a single check.

  • Flags and options that are not needed:

    • --method has one choice, ("fork",) (164, 2256). It still sets method on every job, result and ledger row.
    • launch_overrides (2241–2249) adds hidden flags on spawn, continue and rebase only to refuse them. argparse already rejects unknown flags.
    • PHASE2_COMMANDS = () (119, 4120) is dead.
    • train re-accepts every launch field as a per-base override (2227–2236, train_launch 3015–3036). That forces the "override or config" logic in defining_drift (2866–2870). One launch config per run is enough, and a new run can change it.
    • --ttl-s, --warm-margin-s with its runtime caps (2212, 1681, SKILL.md:30), --stall-after-s, --orphan-after, --pings on|off and --parallel-cap are tunables without a user.
    • On an init re-run, rerun_binary (2458–2475) re-resolves the binary by its original source.
    • model_admission (1426–1452) is "permissive at init". It reads settings and enforces nothing.
  • where. Its env-scrub section lists "unknown" vars and a drop_path_entries prose rule. That is useful once when porting, and noise on every skill load (the SKILL.md front matter injects it).

  • The tests. 4,235 lines, more than the code. Most of them pin the machinery above (lock races, pid reuse, clock jumps, ping caps). They shrink with it.

5. The minimal version

One file, about 600–800 lines, standard library only, run in the foreground:

Part Lines
Config and state dir: run.json (launch config + bases), jobs/<id>.json, ledger.jsonl, notes/, raw/ 80
Env scrub + argv builder (kept code) 120
Stream reader + usage + cache verdict + warmth (kept code) 220
Commands 250
Windows path norm + child-tree kill (taskkill / killpg) 60
argparse 60
Tests about 500

The commands:

  • init --run R --model M [--effort] [--agent] [--cwd] writes the launch config.
  • train --base B --brief F runs claude -p … --session-id <uuid> in the foreground and streams it to raw/<job>.jsonl. On exit it writes the result and a ledger line, marks the base active, and records last_hit, context_tokens and the TTL.
  • spawn --base B --block X --brief F does the same with --resume <base> --fork-session. The prompt is the spawn template plus a delta header. The result carries cache, base_state at launch, and the note path.
  • continue --job J --brief F resumes the gremlin session (no fork).
  • ping --base B is a one-turn fork with --max-turns 1, only when asked.
  • list shows bases with base_state and warm_until, and jobs with their status.
  • usage [--block|--phase] [--format md].
  • kill --job J kills the recorded child pid tree.

The happy path from the orchestrator (Athanor), each call in Bash with run_in_background: true. The completion notice carries the result JSON.

G init R --model claude-opus-5-5[1m]
G train R --base B1 --brief train.md          # background; notice → {status, session, cost, ...}
G spawn R --base B1 --block 1.1 --brief b.md  # background; notice → {status, cache: hit, note, ...}
G spawn R --base B1 --block 1.2 --brief c.md  # parallel = just two background calls
G usage R --phase 1 --format md

Rebase is just "spawn, and register its session as a new base". It is about 30 lines inside spawn (--as NEW), not a separate state machine.

One open check before the rewrite: does TaskStop on a backgrounded Git Bash call kill the claude.exe grandchild on Windows? If it does not, keep kill --job. Either way the helper should kill its own child on SIGTERM or SIGBREAK.

6. Usable right now?

Yes, on Windows, for the happy path. I ran it from the worktree root and in a scratch git repo at C:\Users\g4b\AppData\Local\Temp\claude\G--dev-projects-reliquary\42c87f82-…\scratchpad\verdict-smoke, following SKILL.md's call sequence.

Step Result
where Exit 0 in 0.25 s, ok: true. Platform windows (supported), binary 2.1.299 found through CLAUDE_CODE_EXECPATH, uv 0.5.13, git found. 13 "unknown" CLAUDE_* vars listed, no warnings.
init --model claude-sonnet-5-5 --cwd <scratch> Exit 0. The model was accepted (model_admission: "no availableModels list"). Effort high, permission mode dontAsk, empty strict MCP config, preamble hashed.
train --base B1 --phase 1 --brief <one line> Exit 0 in 0.6 s; returned job j0001 and a session id.
wait --all (Bash timeout 600000) Exit 0 after 2 s. status: ok, 5.3 s wall, 2 turns, models_used: [claude-sonnet-5-5], 16,731 tokens written to the 1h cache, $0.0706.
spawn --base B1 --block 1.1 --phase 1 --brief <one line> Exit 0. base_state: warm, no rebase or re-train due.
wait --all Exit 0 after 12 s. status: ok, cache: hit (69,943 tokens read from cache, 1,530 written), 10.6 s wall, $0.0195. notes/1.1.md was written in the right format, stamped claude-sonnet-5-5.
usage (json) and usage --phase 1 --format md Both correct. Total $0.0902, cache-hit ratio 1/1, 0 cold starts. The md table renders well.
list B1 active, warm_until 1 h out, nothing running.
expire --all Exit 0: retired: [B1], killed: [], ok: true.
Process check afterwards No _supervise, --fork-session or run-gremlins processes left.

Nothing broke. Minor findings:

  • The default commit rule conflicts with a no-commit brief. The spawn template's default rule (sequential) told the gremlin to commit, while the brief said not to. The gremlin flagged the conflict in its note and followed the brief.
  • Rounding differs between ledger fields. cost_cumulative_usd (0.0706285) and cost_delta_usd (0.070628) are rounded differently in the ledger.
  • Paths not exercised. kill, pings, continue, rebase, failure paths and long runs over the Bash timeout were not run. expire had nothing to kill.
  • Platforms. The tool works only on Windows; Linux and macOS are gated off by design.