[javenciu]

Problem / Motivation

Goal: test_a_second_boot_touches_only_known_module_globals gives the same verdict whichever test ran before it and whichever boot first resolves the scratch-root override.

The test diffs every loaded module global of the package between a baseline taken after the first boot (before = _module_globals() at test/integration/test_boot_smoke.py:553) and a snapshot taken after the second (after = _module_globals() at test/integration/test_boot_smoke.py:556). config.paths._scratch_root_override_pin (src/kiro_crew/config/paths.py:372) is _UNSET until the first scratch_root() call. scratch_root is defined at src/kiro_crew/agent_scratch.py:185 and reads the pin through valid_scratch_root_override (src/kiro_crew/config/paths.py:375). The first caller is allocate_scratch (src/kiro_crew/agent_scratch.py:399), which a session spawn runs on an executor thread (src/kiro_crew/acp/launch.py:936). When the first resolve lands between the two snapshots, _UNSET becomes ('', None) and the test fails naming ('kiro_crew.config.paths', '_scratch_root_override_pin'). The boots in this test only request /api/health and nothing in the test body starts a session. I did not see a resolve land inside the window in local runs (the whole file in file order, and the file after test_streaming.py, which does spawn sessions), so the reproduction below forces one. The rootdir conftest.py resets the pin to _UNSET at the start of every test (conftest.py:4088), so an earlier test does not hand this one a set pin.

Why it matters

The test is the witness that keeps the integration harness's reset list honest. A failure that depends on timing makes a red Integration run look like a real home-bound global and sends the author hunting for a global no boot touched.

Not a goal

• No change to config/paths.py: the pin is a deliberate process-lifetime value (its docstring explains the time-of-use reason) and stays lazy.
• No row in _KNOWN_SECOND_BOOT_CHANGES for the pin: the witness keeps comparing it, so a boot that re-pins it to a different value still fails here.
• No change to test/integration/conftest.py or the rootdir conftest.py.

What changed (motivation → approach → change)

Symptom: the witness is red only when the pin's first resolve falls between its two snapshots. Cause: the pin is resolved lazily and the test does not control when. Change: the test resolves the override itself before the first boot, so both snapshots hold ('', None) and a later resolve hits the pinned value without changing it. The test's own monkeypatch records the previous value and restores it, so the test leaves the pin as it found it.

The edit is inside the test body only (test/integration/test_boot_smoke.py, one hunk, +10/-1): a local import of kiro_crew.config.paths, a monkeypatch.setattr of the pin to _UNSET, and one call to valid_scratch_root_override().

Rejected: a row for the pin in _KNOWN_SECOND_BOOT_CHANGES, the way #19168 listed the umask cache. The pin is the value the scratch-root symlink defence freezes (src/kiro_crew/config/paths.py:362 onward), so a row would let the witness ignore a boot that repoints it. Pinning it in the test keeps the comparison; the mutant run below shows the pinned form still catches a repoint.

Overlap with open PRs, read with gh pr view: #17666 changes only the five kiro_crew.security.argv_floor rows of _KNOWN_SECOND_BOOT_CHANGES in this file (hunk @@ -436,11 +436,11 @@); #15546 adds a test after line 150 of this file (@@ -150,6 +150,31 @@) and edits test/integration/conftest.py. This hunk (@@ -533,12 +533,21 @@) shares no line with either.

Backwards compatibility

Compatible: test-only change; no production code, fixture or other test changes.

Removes nothing: the witness still compares the pin across the two boots.

Tests

In short: the changed test is green on the reproductions where the unmodified test is red.

The first resolve was forced to land inside a chosen boot with a throwaway pytest plugin (an executor-thread call to valid_scratch_root_override() while that boot is live, as allocate_scratch makes it). Commands run from the repo root with KIROCREW_INTEGRATION=1, -n0, -p no:randomly. The red runs are on unmodified origin/main at 3665e4772f; the green runs are on that commit plus this change, then repeated on the rebased head over origin/main at 2f5c7d58b9 where noted.

• Unmodified test, first resolve inside the second boot: red 3 of 3, AssertionError: a boot changed module globals the harness neither restores nor documents ... [('kiro_crew.config.paths', '_scratch_root_override_pin')].
• Unmodified test, first resolve inside the first boot: green 3 of 3.
• Changed test, first resolve inside the second boot: green 3 of 3. Inside the first boot: green 3 of 3. No forced resolve: green 3 of 3. Re-run on the rebased head (2f5c7d58b9), first resolve inside the second boot: green 3 of 3, whole test_boot_smoke.py: 15 passed.
• Mutant, changed test: a second boot that overwrites the pin with a different value turns it red 3 of 3, naming ('kiro_crew.config.paths', '_scratch_root_override_pin'), so the pinned form still compares the pin.
• Restore: after full teardown of the changed test the pin reads _UNSET again.
• Leak check, one process at -n0, test_boot_smoke.py first, then test_taskrunner_atomic_persistence.py, test_taskrunner_execution_capture_async.py, test_unattended_slot_guardrails.py, test_wheel_apply.py: 127 passed.
• Neighbours, the whole test/integration directory in one process at -n0: 63 passed.
• Ratchets (test_wait_discipline_ratchet.py, test_flake_pattern_ratchet.py, test_pid_reader_ratchet.py, test_tmpdir_helpers.py, test_dir_fd_create_race_ratchet.py): 127 passed.
• black --check, isort --check-only, flake8, mypy on the file, scripts/check_agent_sdk_boundary.py, scripts/check_comment_history.py: exit 0.

Not shown red by test order alone: the rootdir conftest.py resets the pin to _UNSET at the start of every test, so a previous test does not leave it set at the start of this one. The red needs the first resolve to land between the two snapshots.

Manual verification

N/A — the change is a test and the reproductions above are the verification.

Related Issues

None.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

First Principles Review (Opus 5.5) — ⏭️ skipped

Revision d696816c01a1c57742155ca87eeba4ba5f98813c ships no reviewable capability (docs, tests or generated files only), so there is nothing to inventory. Advisory — does not block merge.

[github-actions]

UX Review (Opus 5.5) — ⏭️ skipped

Revision d696816c01a1c57742155ca87eeba4ba5f98813c touches no user-facing surface (no changes under website/ or committed screenshots), so the UX review was skipped. Advisory — does not block merge.

[github-actions]

Design Review (Opus 5.5) — ✅ PASS

Design-level review of d696816c01a1c57742155ca87eeba4ba5f98813c — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Nothing to check.

[DESIGN-REVIEWED] d696816

Description read

[DESCRIPTION-READ] 0dd01c3596c5d3350fbb7d8f8e76b95489d0addbcdabf86faf59d4f23cedc1f6

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

[github-actions]

Opus 5.5 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] d696816

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

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

[github-actions]

GPT 6.1 Review — ✅ no blocking findings

GPT 6.1 completed its review of d696816c01a1c57742155ca87eeba4ba5f98813c and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] d696816

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

[javenciu]

Closing and reopening so CI runs on hosted runners: same-repo runs from this account queue on the fleet label without a runner (details on #19683).

[KiroCrewCaptain]

Tier 1 auto-approve: test (1 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, security checklist all-NO, no reviewer-lane findings, readiness signal fresh for this head SHA, declared category matches the diff. Category: one integration test pins the scratch-root override before the boot diff, so the two snapshots compare like with like.