test(autonudge): pin the gate premise the paused-loop tests rely on

[bolichen97]

Problem / Motivation

main's five monitor_update paused-loop tests in
test/test_autonudge_stop_auth.py went red for several hours today, and the
failure told nobody why. All five reported the same thing:

AssertionError: assert 'merged' in 'monitor_update cannot apply legacy fields
to a structured monitor: max_cycles'

That string is a production refusal, so the batch reads as a bug in the
wording under test. The actual cause was one level up: monitor_update splits
on record kind before it reads a single bound, and
is_structured_monitor_loop decides that kind with
monitor is not None and not gate. The tests' _FakeLoop predated gate, so
the moment they gave it the monitor holding the terminal state they are about,
it classified as a controller record and the legacy branch under test became
unreachable. #8317 has since fixed the doubles.

The premise those five now rest on — gate=True puts a probe-state double on
the legacy side — is stated in _FakeLoop's docstring and executed nowhere.

Why it matters

The premise is load-bearing for five tests and invisible in all five. Nothing
observes it, so the next edit to is_structured_monitor_loop reproduces exactly
the diagnosis cost that just happened: five assertion failures against a refusal
string that names neither the flag nor the routing, in a file whose diff is
clean. It cost this repo a red shard inherited by every PR on that tip, plus two
independent agents reaching the same diagnosis from scratch.

It is also a live risk rather than a hypothetical one, because the classification
straddles two subsystems that ship separately: the discriminator lives in
autonudge.py, the split that reads it in dashboard/session_directive_apply.py,
and the tests that depend on it name neither in their own diff. That is precisely
the pairing no path-based or file-overlap check can make — it is only visible
through the predicate.

What changed (motivation → approach → change)

Goal: make the premise fail as itself, so the batch carries its own cause.

Approach considered and rejected: restating the invariant in more prose next
to each of the five. Prose is what is already there, and it is what did not fire.

The change: one test,
test_a_probe_state_double_is_a_legacy_loop_only_while_it_carries_the_gate,
asserting the three classifications the doubles rely on against the production
predicate itself:

double	is_structured_monitor_loop
monitor=_FakeMonitor(), gate=True — gated prompt loop	False (legacy)
monitor=_FakeMonitor() — controller record	True (structured)
no monitor — plain prompt loop	False (legacy)

Those three are the invariant production already enforces in both directions: the
prompt-loop arming path (AutoNudgeService.add → _add_unserialized) attaches a
monitor only if gate; add_monitor builds its controller record on the
dataclass default (gate=False); and _monitor_tick_is_quiet — the only
writer of monitor.terminal_pending and of the outcome /
MONITOR_TERMINAL_REASON settlement these tests assert on — returns early on
if not loop.gate. It is also what the spec says, in
docs/system-specs/modules/learn-cron-dashboard.md: "A prompt-driven zero-token
gate may also carry probe state, but it remains a legacy loop (gate=true) on
the AutoNudge control surfaces; only controller records (gate=false) use the
structured monitor APIs". Nothing documented changes, so no spec edit is owed.

Test-only. No production line moves.

Tests

• The one test above. test/test_autonudge_stop_auth.py: 51 passed.
• Verified it fires. Dropping the gate term from
  is_structured_monitor_loop (return getattr(loop, "monitor", None) is not None) takes the module from 5 failed, 45 passed to 6 failed, 45 passed —
  the sixth being this test, which names the predicate and the flag rather than a
  refusal string. Reverted after measuring; the production file is untouched in
  this diff.

Manual verification

N/A — the change is a test, and the suite is the observation. The mutation check
above is the substantive verification.

Related Issues

no linked issue: this is the harvest of a main regression already fixed by #8317,
not a tracked defect of its own.

Pattern harvest

Rule candidate: review-prompt
Pattern: a hand-rolled test double whose attribute set predates a production
discriminator silently answers that discriminator's default. The test then
routes to the wrong branch and asserts against the other branch's error
string — so the failure names a real production refusal and reads as a bug in the
code under test, while the double is the last place anyone looks.

A checkable form, for a reviewer or a review prompt: when a diff adds a field
that a getattr(x, "field", default) predicate splits on, the review has to look
for doubles of x in the test tree, not only for callers in src/.

And the reason this PR exists rather than a comment: an invariant a test
depends on but does not assert is not covered. When N tests share an unasserted
premise, a break in it costs N confusing failures and zero informative ones, so
the premise earns its own assertion against the production symbol.

Not a lint rule: the trigger is "this double stands in for that dataclass", which
nothing in the source states.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|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) — N/A: the spec already states the
  gate discriminator this pins; no documented behaviour changes.
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

A real diagnosed failure mode, closed with one proportionate assertion against the production predicate — mutation-verified, test-only, no new surface.

[DESIGN-REVIEWED] 60ad3b1

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] 60ad3b1

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

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

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 60ad3b17c29b9cf223191d9a884175c151fc70cd and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 60ad3b1

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

[bolichen97]

CI note — the one red check is a repo-wide npm-registry timeout, not this diff. It needs a maintainer decision, not a change here.

Dependency Audit / Audit Production Dependencies has failed on six consecutive attempts on this unchanged head, always fail-closed on a tool timeout and never on an advisory:

ERROR: production dependency audit failed closed: npm audit timed out after 120s for website/package-lock.json

(five attempts named website/package-lock.json, one named site/package-lock.json.)

It is failing across the repository, not on this PR. A sample of open PRs between 01:45 and 01:51 UTC shows the same job completed/failure on #8275, #5539, #7955, #8171, #7235, #8282, #7308, #7913, #8180, #7163, #7892, #8355 and #7777 — thirteen unrelated heads with unrelated diffs. One run succeeded at 01:26 (#8315), so the condition is intermittent and currently trending worse.

Why it cannot be this diff, independently of that. The gate fails closed because npm audit did not finish inside 120s, so the log reaches no vulnerability at all. This PR adds 22 lines to one Python test file (test/test_autonudge_stop_auth.py) and touches no manifest, lockfile, or dependency. Which lockfile times out even varies between attempts on the same unchanged head, which a diff-caused failure would not.

What I did and deliberately did not do. I re-ran the job five times rather than work around it. I am not adding a .vulnerability-exceptions.json entry: that mechanism is scoped to a vulnerability that cannot yet be remediated, and using it to paper over a tool timeout would silence the gate for a real advisory later. Nor am I touching the gate itself from this PR — a change to a security gate's timeout belongs in its own PR with a maintainer on it.

The question for a maintainer: website/package-lock.json has been bumped several times on main in the last few days (#8041, #7766, #7767), so the audited production tree grew while the ceiling in scripts/check_npm_audit.py stayed at 120s. Should that ceiling be raised, or the audit given a bounded retry, given it is currently the single blocking readiness item on a large share of the open-PR fleet?

Everything else on 94a2504 is green — 54 successful checks, no other failure — and all three blocking review lanes reported no findings: Design Review PASS, Opus 4.8 no blocking findings, GPT 5.6 no blocking findings.

[bolichen97]

Resolved without a maintainer decision — main answered the question, and a rebase picks it up.

My earlier note asked whether the npm audit ceiling should be raised or the audit given a bounded retry. Both landed on main in the meantime, so the question is withdrawn rather than still open.

e786b086d (#8362, ci: run the npm audit gate on releases only; retry transient faults) did three things:

• AUDIT_TIMEOUT_SECONDS is now 180, not 120.
• scripts/check_npm_audit.py retries transient faults under a total budget (AUDIT_TOTAL_BUDGET_SECONDS), so a slow registry no longer fails the gate on a single attempt.
• dependency-vulnerability.yml is now workflow_call / workflow_dispatch only, invoked from nightly.yml and release.yml — it no longer runs per-PR at all. The workflow comment states the reasoning: the job gates whether a release proceeds, and a per-PR Actions cache would be an input a lower-trust context could influence.

That is why a re-run could never have cleared this PR: a re-run replays the original merge-ref snapshot, so it re-tested a base that predated the fix. The remedy was a rebase, not another attempt.

What I did. Rebased onto main (390ac8d2f119), which brings e786b086d into the base, and force-pushed with a SHA-pinned lease: 94a250427 → 60ad3b17c. The diff is byte-identical — the same 22 added lines in test/test_autonudge_stop_auth.py, no production file touched. No .vulnerability-exceptions.json entry was added and the gate itself was not modified from this PR, exactly as stated before.

Local verification on the new base. Targeted tests test/test_autonudge_stop_auth.py 51 passed; isort / flake8 clean; mypy --platform linux src/kiro_crew clean across 1281 files; the black and subprocess-encoding ratchets pass in scope; the harness-parity and brand-name gates pass on the added lines. The mutation claim in the commit message re-verified on this base: dropping the gate term from is_structured_monitor_loop fails 6 tests, and the sixth is the new one, which names the predicate.

The three review lanes stamped at 94a250427 (GPT 5.6, Opus 4.8, Design Review — all zero findings) will re-run against the new head; the content they judged has not changed.

_{[operator: bolichen97]}