Files
reliquary/docs/workpad/reports/2026-10-16-19-gremlins-phase3-digest-check.md
T

14 KiB
Raw Blame History

Pre-build check: Phase 3 contract digest (spire:gremlins)

  • Checker model id: claude-opus-5-5[1m]. Fresh: did not write the digest and will not build or review Phase 3.
  • Date: 2026-10-16
  • Checked: docs/workpad/plans/2026-10-16-18-gremlins-phase3-contract.md at 5092ed8, against the goal on main (docs/workpad/goals/headless-forks.md), build plan §4 and §6 Phase 3, the Phase 2 re-check (N1–N5, the flake), the run log's routed items, the Phase 1 review and re-check, and plugins/spire/skills/forks/forks.py / tests/test_forks.py as they are now. No tests were run and no live calls were made.
  • Verdict: 1 BLOCKER, 12 ADJUST, the rest OK. The interface list is complete against the code. The rename map has no gap that breaks state. The gate tests, C16 and the release wording need changes before the build.

BLOCKER

B1. GateTests cannot work on the Windows host as specified (§4 "Tests"). They patch gremlins.PLATFORM to a PosixPlatform. Every path then goes through norm_path → PLATFORM.norm_path → PosixPlatform, and §3.4/C8 makes that refuse drive forms.

  • where --binary C:\…\claude-stub.cmd refuses in resolve_binary (forks.py:1004), and so does script_path() (:1067). Today cmd_where does not catch either (:2006, :2023).
  • init --run C:\… refuses in state_dir_for (:340) before any gate check.
  • What happens:
    • The override half ("where exits 0", "init proceeds") fails on Windows.
    • The closed init case ("exits 2 and creates no state dir") passes for the wrong reason, the path refusal. So it is a test that cannot fail on the gate.
    • The builder has to improvise the fix, which is how earlier digests produced Important findings.
  • Fix: the fake platform is a WindowsPlatform subclass, or PLATFORM with supported and describe patched. It has supported = False, is_wsl() = True and describe() = {name: "posix", sys_platform: "linux", wsl: true, …}, so path handling stays Windows. init asserts error == "platform not supported". The gate check is the first thing in init, before state_dir_for and any norm_path. Keep the pure PosixPlatform().supported is False unit tests as written.

ADJUST

A1. The gate leaves launches open when a registry exists (§4, C11; §7.2 intake). "Every other command needs init" is only true for a fresh run. The state dir is committed at phase close (§7.2 commit ownership: everything except raw/ and locks). §7.2 intake says: if the run log has a Gremlins: line, run list and adopt, without where. On Linux, a successor Athanor adopting a run that was started on Windows would then run spawn, rebase, continue and train. wait and tick would launch pings. All of this happens on a closed platform. The sessions are not on that host, so the jobs fail, but each failure costs a launch, and pings keep retrying (Phase 2 Minor 3).

  • Fix:
    • Refuse on a closed gate in the dispatcher for train, spawn, continue, rebase and tick, in the same place as the N3 refusal.
    • The tick inside wait and the supervisor launches no pings on a closed gate.
    • The intake runs where first, then adopts.
    • Add one GateTests case: a registry present, gate closed, spawn exits 2.

A2. C16 as written hides real survivors (§5.3). The rule is "a snapshot pid that is gone from the table or now carries another name is dead". It fails in three cases:

  • (i) process_table() returns []. On macOS that is always (no /proc); on Windows it happens when Toolhelp fails (:750-752). Every snapshot pid then counts as "gone", so kill reports survivors: [], ok: true.

  • (ii) The snapshot name is None. The root's name is names.get(root) (:805), so a root missing from the table gets None, and None never matches a name.

  • (iii) On POSIX, comm changes on exec, for example an sh wrapper that execs python or claude. A live process then reads "renamed".

    The defect that is actually observed is narrower: access denied makes pid_alive return True without a creation-time check (:719-721).

  • Fix: drop a snapshot pid only if one of these holds:

    • the table is non-empty and lacks the pid;
    • the snapshot name is known, the table name differs, and proc_ctime(pid) is None (the unverifiable case, i.e. class (a)).

    In every other case record_alive decides, as today.

  • Read the table through the module-level process_table (ProcessTreeTests patch module names).

  • Tests: an empty table keeps the survivor; a snapshot name None keeps it; a name differing while the ctime is verifiable leaves the decision to record_alive.

A3. The third N3 test in §5.1 contradicts itself. "Stored file deleted, env unset, PATH without claude → stored path kept with the 'could not re-resolve' warning (then build_launch refuses…)". build_init_config calls build_launch → binary_info, which raises claude binary not found (:2162, :1025-1026). The warning can never appear in the output.

  • Fix: split it into two tests.
    • Stored file present, env unset, PATH without claude: path kept, warning shown, exit 0.
    • Stored file deleted: exit 2, claude binary not found (today's loud failure).

A4. C14 can switch installs silently. If the stored binary_resolved_by is CLAUDE_CODE_EXECPATH and a re-run comes from a shell without it, resolve_binary(None) falls through to the PATH claude (:1005-1010). That can be another install, with another version or auth. A warning is given, but it still crosses sources.

  • Fix: re-resolve only through the stored source (EXECPATH from EXECPATH, PATH from PATH). If that source gives nothing, keep the stored path and warn.

A5. Path matrix row 3 (C:, C:x) contradicts "as today" (§3.4). On today's Windows branch, ntpath.join(base, "C:x") returns C:x. Its rest does not start with \, so the result is ForksError("cannot normalise path") (:322-332). C: goes the same way. Also, "assert absolute with drive C:" cannot be pure string logic on a POSIX host (WSL runs the matrix), because ntpath has no per-drive cwd there.

  • Fix: row 3 on Windows is refused, as today.

A6. The C18 wording fails G-text 1 (§7.2 vs §9.2.1). §7.2 proposes "A fresh agent reads the code the phase touches and writes the trunk notes". G-text 1 requires that phrase to be absent from athanor.md.

  • Fix: reword, for example "a fresh agent reads the code the blocks change and writes the trunk notes".

A7. "Windows-only in 0.3.0" contradicts the goal's "Amended … release gating". 0.3.0 is merged and released only after the Linux and macOS Igors lift their gates, so the 0.3.0 that ships is not Windows-only. The phrase is in four places:

  • the SKILL.md description (§7.1);

  • the README row (§9.1);

  • the gate reason (§4, "spire:gremlins 0.3.0 runs on Windows only");

  • the release commit body (§9.4).

    Porters can change the first three. They cannot change a commit body without rewriting a pushed history.

  • Fix: use version-free wording.

    • Text: "Windows-tested; other platforms fail closed until tested (see Platforms)".
    • Gate reason: "spire:gremlins is not tested on <sys_platform><, WSL> yet; …".
    • Changelog: "tested on Windows; Linux and macOS gated pending the platform handoff".
    • Handoff §8.8 lists every place the porter updates.

A8. Three routed text items are missing from §7.7.

  • (a) preamble.md:9 points gremlins at plugins/spire/skills/implementation/references/run-log.md. The Phase 1 review ("Two extra preamble lines" row) says this does not resolve in other projects, because the plugin lives in the cache. Phase 3 should give the format inline or by an absolute path. §2.6 says "keep the other rules", so this would ship broken.
    • Fix: inline the handoff-note format in preamble.md. Or have the helper put the absolute path into the templates (it knows script_path()). The preamble hash changes in 3.1 anyway.
  • (b) stalled together with clock_jump is not a hung gremlin (Phase 1 re-check, M3 builder flag).
  • (c) The remedy depends on the reason. Only model_refused points at availableModels. usage_limit, overloaded, api_error and prompt_too_long are not a settings problem (Phase 1 review, the 429 → model_refused finding). SKILL §6 lists the reasons but no action for each.

A9. Vacuous negative tests if the rename misses them (§2.7). test_forks.py:155, :934, :1011 and :1079 assert assertFalse(os.path.exists(… "-forks")). After the rename the state dir is -gremlins, so any line left unchanged can never fail. The residue grep shows them, but nothing in §2.8 says they are wrong.

  • Fix: name these lines in §2.7 and derive the path from gremlins.state_dir_for(run), not a literal suffix.

A10. The §2.8 residue categories miss legitimate hits outside the skill. These are:

  • igor/SKILL.md:66 (the D4 follow-up, out of scope);
  • igor/references/mindset.md:11 and igor/references/harness-notes.md:3;
  • implementation/references/superpowers.md:36 (repository forks).
  • Fix: add a category "Igor text (D4) and non-session meanings: leave untouched", so 3.1 neither edits these lines nor reports them as residue.

A11. A closed-gate where does not always "still fill everything else" (§4). cmd_where (:2006) does not catch a path refusal from resolve_binary. On WSL with a Windows CLAUDE_CODE_EXECPATH (via WSLENV), or with --binary C:\…, §3.4 now refuses. The output is then exit 2 with a path error, without the gate reason and without the report the porters need.

  • Fix: catch GremlinsError from resolve_binary into binary.error and reasons.

A12. platform.override is undefined while the env is set on a supported platform. C13 sets the env for every test, so the question comes up in every test. §4 says that with the override, where reports override: true.

  • Fix:
    • Define override = env set and not PLATFORM.supported. Windows CliTests then assert override: false even with the env set.
    • PosixPlatform.describe() reads self.supported instead of a literal False, so lifting the gate is one change (§4 "Lifting the gate").
    • platform_gate() reads PLATFORM.describe(), never sys.platform, so it passes the §3.7 seam grep.

OK, with notes

  • Interface (§3.2): complete against the code. These are every OS-specific site in forks.py, and §3.1–§3.3 places each one:

    • IS_WINDOWS/SPAWN_METHOD/_NOWIN :47-52;
    • cygpath :276-296;
    • the lock restore :500-503;
    • the ctypes block :630-664;
    • proc_ctime/epoch_to_ctime/pid_alive/process_table/terminate :667-853;
    • _taskkill/kill_tree :856-895;
    • _run :901;
    • _powershell/spawn_supervisor :1421-1460;
    • where :2028-2030;
    • Supervisor.run :3223-3224.

    The transcript_path slug and the scrub lists are neutral code that was only observed on Windows. They belong in the handoff, where §8 puts them. Do not name any local variable platform: the module imports platform (:2003).

  • Rename map: no gap that breaks state. No -forks state dir is tracked in git, and nothing outside the skill references skills/forks. The base key "forks" has three writers (:1637, :2716, :2782) plus LIST_BASE_KEYS :3431; all are covered by §2.4. run_for_state :2935 is covered by §2.1. The preamble text change alters the append_system_prompt hash (a defining field). That is harmless, because no base from before the rename is reused (C6).

  • C1–C7, C9, C10, C19, C20: OK. For C20, see the A7 wording.

  • C8: OK. The POSIX refusal of a backslash is a behaviour change, and the handoff (§8.4) already lists it.

  • C11: OK in itself. It is not enough on its own: see A1.

  • C12: OK. Naming: SPIRE_GREMLINS_UNSUPPORTED_OK has a plural prefix, SPIRE_GREMLIN_CHILD and SPIRE_GREMLIN_DETACH_* have a singular one. Pick one before 3.2 writes them.

  • C13: OK. It also means that a green suite on Linux says nothing about the gate; only GateTests cover it. The handoff should say so.

  • C15: OK.

    • rebase refuses a base that is not active, at launch and again under the lock (:2873, :2893).
    • Only one rebase per base runs at a time (:2901).
    • So "not active at finalise" can only mean "changed during the run".
    • _conclude runs inside finalize_job's lock with the same registry read as _rebase_succeeded (:1656-1665, :1607). Computing parent_inactive there is therefore consistent.
    • Set the flag only when NEW is actually created, not on the rebase_conflict early return (:1622-1624).
  • C16 (b): raising KILL_VERIFY_S to 30 s is fine for kill. expire kills its jobs one after another, though, so several stuck jobs × 30 s can pass the Bash tool's default 2 min. The tool then backgrounds the call (P0.1(d)). The SKILL text should give kill/expire a long Bash timeout.

  • C17: acceptable. The stub tests exercise the real CIM detach after the rename and the seam refactor. The release commit is not a merge. Phase 4 must smoke the renamed helper before anything ships.

  • C18: a reasonable reading of "the 0.2.2 shape" against "the Athanor does not read code". Wording: see A6.

  • Push: the goal says the branch "gets pushed". The digest says no push, which follows Igor's brief (precedence 1). Pushing is a post-build user step.

  • The two flagged risks (an ! injection exiting 2 on a closed platform; installing uv in WSL only through the permission prompt): correctly flagged, nothing to add.

  • The flake plan (§5.3): the bounded investigation and "never two suites at once" are right. Only the C16 rule needs A2.