fix(spec-builder): resolve the guard's live state dir per test, not at import

[chenmingwei23]

What is the problem?

main is red. test/test_lazy_data_home_paths.py::TestNoImportTimePathResolution::test_no_module_level_path_constants — the #874 ratchet — fails on Backend Tests (3.10, 2), (3.12, 2) and (Windows) (2), with Coverage Gate cascading (Coverage Combine is skipped when a shard fails) and PR Readiness red on top:

AssertionError: Data-home path resolved at import time (issue #874). Convert each to an
override hook + accessor -- see the module docstring of this file for the pattern:
    apps/builtins/spec_builder/tests/test_routes.py:88  [module] _REAL_STATE_DIR = ..._state_dir()

The offending line is one module-level capture in the Spec Builder test file, landed by #518 (af8774b8b):

#: Captured at import, before any test can monkeypatch the module attributes.
_REAL_STATE_DIR = routes._state_dir()

Why this issue matters to the user

Because the red is on main, every open PR inherits it through its merge commit and cannot reach readiness: passed — including PRs that touch no Python at all. It was first observed on #1530, a docs-only PR whose branch does not even contain the offending file. Two PRs (#1492, #1509) have since merged through the red, so the queue is now landing changes with a red aggregate gate, which is exactly the state the readiness signal exists to prevent.

The ratchet itself is not noise. _state_dir() reads the data home on every call, so freezing its result at import pins whichever KIROCREW_HOME was active when collection imported the module. That is the #874 class of bug — a pod writes into the real data home instead of its isolated one — and this repository has already paid for it with real state pollution.

How our fix solves it

Symptom: three shards plus two aggregate gates red on main. → Root cause: the guard fixture needs the unpatched state dir, and the author got it by resolving at import, because _redirect_state monkeypatches routes._STATE_DIR before each test body runs. Import time is the only moment that is obviously "before the patch" — but it is also the one moment the data home may not be final yet. → Change: resolve it inside the fixture, before it installs the redirect. That is still unpatched, and it is now per-test and live.

src/kiro_crew/apps/builtins/spec_builder/tests/test_routes.py:

• Deleted the module-level _REAL_STATE_DIR.
• _live_state_snapshot() now takes the directory as a parameter instead of reading a module global.
• _never_touch_the_real_state resolves routes._state_dir() as its first statement — before _redirect_state runs — and passes that value to the before/after snapshots and the failure message.

The guard keeps its original semantics (whole-directory names + mtimes, compared across the test) and gains one property: under a KIROCREW_HOME override it now watches the sandbox's state dir rather than a path frozen from the ambient home, so it protects the directory the test could actually pollute.

No production code changed. routes.py was already compliant — its _STATE_DIR / _INDEX_PATH / _DELETED_PATH / _SETTINGS_PATH are None-defaulted override hooks with per-call accessors; only the test file froze a path.

What tests we did

• test/test_lazy_data_home_paths.py — 16 passed, including the ratchet that was red. This is the check that unblocks main.
• src/kiro_crew/apps/builtins/spec_builder/tests/ — 268 passed (whole Spec Builder suite, -n auto --dist loadgroup). Combined run of both: 284 passed.
• Updated test_state_guard_watches_the_whole_directory, the drift test that pins the guard's shape. It previously asserted on the identifiers this PR removes, so leaving it alone would have made it pass vacuously. It now pins the invariants that matter: the snapshot is taken against the fixture-resolved dir, the dir is resolved via routes._state_dir() inside the fixture, the snapshot iterates the whole directory, and no module-level _REAL_STATE_DIR = reappears.
• isort --check-only, flake8, and mypy on the changed file / its package: clean.

Manual verification

Mutation-tested the guard, because a "fix" that quietly disarms a safety net is worse than the red it removes. Appended a throwaway test to test_routes.py that writes leak.json directly into config_dir()/workspace/spec-builder (bypassing the redirect), ran it under KIROCREW_HOME=$(mktemp -d), and confirmed the fixture still fails the run:

E  AssertionError: a test wrote to the live state dir: /tmp/tmp.73UZ6TyCqJ/workspace/spec-builder

Note the reported path is the sandbox home, which is the behaviour improvement described above. The probe was removed afterwards; git status --porcelain shows only the one intended file.

Two earlier mutation attempts did not trip the guard, and are worth recording because they look like gaps and are not: dropping the _DELETED_PATH redirect, and dropping the _STATE_DIR redirect. Neither leaks, because _deleted_path() derives from _state_dir() and the remaining explicit *_PATH redirects still point at the tmp dir. Only a write that bypasses every hook reaches the live directory — which is what the final probe does.

Any other suggestions on the work

The three-shard fan-out plus Coverage Gate plus PR Readiness makes one line of test code look like five independent failures, and the --log-failed archive was empty for these jobs so the real assertion was only reachable by pulling the raw job log. Surfacing the first failing test name in the readiness summary would have cut the triage on this from ~20 minutes to seconds. Filing separately rather than widening this fix.

Fixes #1537

[github-actions]

Design Review (Fable 5) — ✅ PASS

Advisory design-level review of 4f64e5b854a859985e609bc9997006d791923d63 — updated in place on each push; does not block merge.

Design-Verdict: PASS

Root-cause fix at the right layer: the guard resolves the live dir per test before its own redirect, restoring the #874 invariant without touching production code.

[DESIGN-REVIEWED] 4f64e5b

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 4f64e5b854a859985e609bc9997006d791923d63 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 4f64e5b

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 4f64e5b854a859985e609bc9997006d791923d63: <one-sentence reason>

[github-actions]

Opus 5 Review — ✅ no blocking findings

Reviewed 4f64e5b854a859985e609bc9997006d791923d63 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 4f64e5b

Verdict parsed from the review's SHA-scoped output markers for commit 4f64e5b854a859985e609bc9997006d791923d63.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 4f64e5b854a859985e609bc9997006d791923d63: <one-sentence reason>

[chenmingwei23]

Superseded by #1543, which landed the same fix from the other direction: _REAL_STATE_DIR becomes Path | None = None at module level and is resolved on first use inside _live_state_snapshot(). The ratchet is satisfied and main is unblocked, so this PR has no remaining value — closing rather than resolving the conflict.

For the record, the one behavioural difference and why it does not matter here. #1543 caches the resolved dir in a module global for the process lifetime; this PR resolved it per test inside the fixture, which would additionally honour a later KIROCREW_HOME change. That difference is inert for this suite: test/conftest.py's _isolate_kirocrew_home autouse fixture (which pins a per-test home) is rooted at test/ and does not reach src/kiro_crew/apps/builtins/spec_builder/tests/, so the home is constant for the whole session there and the cached value equals the per-test value. If that suite ever gains per-test home isolation, the cache would start watching a stale directory and the guard would silently stop protecting anything — worth remembering, not worth a PR today.

Thanks to whoever picked it up.