feat(dashboard): trigger a goal loop's next nudge from the popover

[chenmingwei23]

Problem / Motivation

A goal loop's interval is an idle gap, not a poll. When the operator already
knows the thing being waited on has changed -- CI finished, a review landed, a
deploy went out -- there was no way to tell the loop to look now. The autonudge
HTTP surface was list / get / start / update / delete, and the MCP surface has
only autonudge_stop, so nothing anywhere could bring a cycle forward.

The two workarounds both cost something. Sitting out the remaining gap wastes
it. Pasting the check instruction into the composer by hand duplicates a prompt
the loop already holds and does not reset the timer, so the scheduled nudge
fires right afterwards and the agent runs the same check twice.

Why it matters

The loop becomes responsive to what the operator knows, instead of only to the
clock. Concretely: on a 300s interval, learning that a PR just went green no
longer means either a five-minute wait or a duplicated turn.

What changed (motivation -> approach -> change)

One button in the goal loop popover, and one route behind it:
POST /api/autonudge/{loop_id}/fire.

The mechanism reuses the scheduled path rather than adding a second one.
AutoNudgeService.fire_now moves the loop's deadline to now and re-arms through
_arm_timer, so the cycle runs inside the ordinary _timer body. Calling
_run_fire_cycle directly would have needed the whole terminal ladder restated
at a second site, and a second copy of a five-condition gate is a divergence
waiting to happen. So the stop sentinel, the cycle cap, the wall-clock budget,
the approval-stall stop and the probe gate all still apply, and delivery goes
through the one _on_fire path.

What it does to the schedule is nothing that is not already true of a
scheduled fire, which is the property worth stating because a manual trigger
that silently shifted the interval would be a defect wearing a feature's
clothes. A delivered cycle already clears next_due_ts, and the re-arm then
starts a fresh full interval -- for a dashboard slot via notify_turn_complete
-> _arm_from_deadline. So the countdown reset the issue asks for is
inherited, not implemented: the next automatic nudge lands one full
idle_secs after the manual turn ends. Deadline preservation is untouched --
notify_user_input still cancels only the task and leaves next_due_ts alone,
so a user turn defers this cycle rather than cancelling it.

delay=0.0 rather than _arm_from_deadline's 10s _OVERDUE_REARM_SECS beat.
That beat exists so an elapsed deadline does not ambush a user mid-conversation;
a manual trigger is the user asking, so the condition it protects against is
absent. next_due_ts is still written to now, so a gateway restart between the
press and the fire resumes with an overdue deadline and takes the beat --
correct there, because by then nobody is pressing anything.

What the usability round changed

A blind reader of the screenshots could not tell the paused frame was the same
loop, so the schedule line now SAYS Paused where the button would be -- the
state is the reason for the absence, so it occupies the space the absence
leaves. The refusal reads "the agent is still working" rather than "mid-turn",
which was this codebase's vocabulary and not a user's. And a FOURTH frame now
photographs the successful press: the PR claimed the schedule line flips to
"due" in place and nothing showed it, which was an unbacked claim in my own
description. The Paused string is one new key, and no value was coined -- each
locale's is its own existing word for paused, taken from its own translation of
state_paused.

Declined: a title naming the cycle cost. It is UX's named smallest fix and my
earlier reason (the header readout advances) was fairly rebutted as post-hoc --
but a title is unreachable by touch and keyboard, and I will not hand-author
eleven translations of a new sentence I cannot verify. A maintainer who wants it
should have it as visible text, not a tooltip.

The new deadline is durable before the press is acknowledged

fire_now is async and awaits its persist, and on a write failure it puts
the previous deadline back and arms NOTHING (503 autonudge_not_persisted).
The rest of this class uses the detached _persist_soon, but that helper scopes
itself in its own docstring to "sync callers ... that cannot await
_persist_locked themselves" and accepts that "a lost write degrades to a fresh
full countdown after restart". That trade is right for the turn-lifecycle hooks,
which assign a deadline nobody asked for and can re-derive it. It is wrong for a
button: a crash in the window before the detached write flushed reloads the OLD
future deadline, so the press reported 200 and the cycle it promised never
happens. It is also wrong specifically because this route now writes a
critical=True audit saying the fire was invoked -- leaving the deadline
best-effort would let the durable record and the durable state disagree about the
same press. Awaiting is this module's own pattern for mutations entering from
async code; the staged-monitor paths all await theirs.

The deadline write is gone, and that is the fix

fire_now has NO suspension point: async def for the caller's convenience,
but nothing inside awaits, so the guards and the arm are atomic with respect to
the event loop. _arm_timer(delay=0.0) is what brings the cycle forward --
_timer sleeps the delay it is given and fires WITHOUT consulting
next_due_ts.

This shape was reached the hard way, and the history belongs in the record
because it is the argument for the shape. An earlier revision wrote
next_due_ts = time.time() and awaited a durable persist before arming, so a
restart between the press and the fire would resume overdue. That await was a
suspension window, and this module has several writers to next_due_ts that
hold no lock at all while writing it -- the quiet-tick re-arm on the
gated-wake branch (autonudge.py:3979) is one. Five successive review rounds
each found the next window, and each was real: a concurrent remove arming a
stale object, a cancelled caller abandoning the write, a countdown entering
_firing mid-persist, a quiet tick overwriting the deadline, then the refused
path leaving its own value durably committed. Every fix was correct and every
one created the next. The window is not closable at this call site, because the
racing writers take no lock to share.

So the write is gone rather than guarded a sixth time. What is given up is
exactly the restart cosmetics it bought: a crash between the press and the fire
resumes the loop on its own schedule instead of overdue, and the operator
presses again. That is the same degradation _persist_soon documents as
acceptable for every other deadline assignment in this class -- "a lost write
degrades to a fresh full countdown after restart, never a premature or dropped
fire" -- and eleven other callers already accept it.

Two consequences of removing the write, both taken. The route now returns the
loop UNCHANGED, so rendering the response verbatim would leave the countdown
showing the very cycle the press superseded -- the one visible confirmation a
press has. The component supplies the armed deadline instead, which is not a
fiction (the cycle IS armed to run now) and is reconciled by the delivery's
autonudge_state frame. The screenshot fixture was corrected to match: it used
to hand the frame a pre-moved deadline, which would have photographed something
the real route cannot produce. It now returns an unchanged loop, so the frame
proves the CLIENT produces the "due" state -- 20/20 with the change, 19/20
without it.

The tests' service directory is now SESSION-scoped rather than per-test.
_persist_soon dispatches through run_in_executor, and a thread already
inside _write_state cannot be stopped -- cancelling the awaiting task does not
reach it -- so a late write could land after a per-test directory was torn down
and recreate it. An earlier revision tried draining those writers at teardown;
that narrows the window but cannot close the cancel branch, and the review that
pressed on it was right.

The countdown reset issue #8212 asks for is unaffected, because it never came
from this write: a delivered cycle clears next_due_ts in _run_fire_cycle and
the re-arm starts a fresh full interval. The audit-or-deny gate is unaffected
too -- a press still spends a model turn, so it is still recorded before the
fire.

The invariant is asserted mechanically rather than described. A coroutine with
no await completes on its first send, so the test drives the raw coroutine
and fails if it ever yields -- await svc.fire_now(...) would pass either way
and so cannot distinguish them. Reintroducing an await fails that test with a
message naming the five races it reopens. Net effect on this PR: eight tests
and roughly 230 lines of guard code deleted, not added.

Where the button sits, and why not in the action row

On the schedule line, beside the countdown it acts on, not in the Stop/Save
row below. website/AUTOSDE.yaml:230 (max-two-buttons-per-row, blocking: true) holds a row to two controls and names this exact escape in its own text:
the third action "goes into an overflow DropdownMenu ... or leaves the
row". Leaving is cheaper than a menu for one action, and it is better on the
merits anyway: this control changes the countdown printed beside it, while Stop
and Save act on the loop's configuration. It is a one-button group, so the cap
is satisfied structurally rather than by happening to be under it today.

A successful press does NOT close the popover

Unlike Save and Stop. The route deliberately sends no body, so the nudge fired
is whatever the loop currently holds -- which means a user who edited the
textarea and pressed this gets the armed message. Closing on top of that would
also drop the unsaved edit, with no dirty guard (drafts are not persisted while
a loop exists), so a press after an edit would cost the user their text as well
as spending a turn on the old prompt. Staying open keeps the edit, keeps Save
reachable, and makes the outcome visible in place: the schedule line flips to
"due", and the header's cycle readout advances a moment later when the delivered
fire broadcasts autonudge_state -- which is also where the press's cost
against the cycle cap becomes observable.

The two questions the issue deferred are answered from the tree, not chosen here

Does a manual trigger count against max_cycles? Yes. _timer documents
cycle_count as counting delivered turns -- "a floor delivery, a fallback
and a follow-up are all delivered turns that advance it". A manual nudge
delivers one and spends a model turn exactly as a scheduled one does. Reusing
_run_fire_cycle makes it count with no manual-vs-scheduled branch to get
wrong, and not counting it would let a press drive unbounded turns past a cap
that exists to bound cost. The issue itself says either behaviour is workable.

Turn in flight: refused, not queued. The fire path already made that call,
with its reason written at the site: queueing "would stack identical 3KB+ nudges
and blow up the context window" (slack/gateway.py:6134). The route surfaces
that as a 409 session_busy using the repository's canonical predicate,
slot.running or slot._in_stage_execution, read exactly as the cron-injection
handler reads it -- slot.running alone is False between the stages of a
multi-stage plan. The two consumers of that predicate diverge deliberately: the
cron path queues, this one refuses.

Deviation from the issue's own wording, stated before anyone asks. The issue
weighed "queue the nudge, or disable until the turn ends". This ships a third
form: the button stays enabled and the press is refused with a visible reason.
Refusing is the recorded decision above; pre-disabling would additionally need
a busy signal threaded from ChatInput into the popover, a file ~26 open PRs
touch, for an affordance the 409 already communicates. If a maintainer wants the
disabled-button form instead, say so and it is a small follow-up -- which is why
this carries Refs and not a closing trailer.

Refusals, each load-bearing rather than defensive

Condition	Answer	Why
loop not registered	404	nothing to fire
loop not active	409	every terminal bound leaves the loop inactive, so one condition covers all of them without restating the list; a press must not buy a turn past a bound the user armed
loop mid-fire	409	_arm_timer cancels the existing timer, and inside the fire window that task may be parked on _persist_locked() writing the delivered cycle -- the same window notify_turn_complete defers around, and the same 409 the sibling POST /api/crons/{id}/run gives for a run already in flight
structured monitor	409	those records are driven by _on_monitor_tick and owned by the monitor API; same code PATCH uses

Every outcome is audited, and the FIRE is audit-or-deny. Two review rounds
landed on this and the split is now the one the repository already draws:

• The fire is gated on a critical=True write that lands BEFORE fire_now
  arms anything, awaited through asyncio.to_thread. This matters because the
  default SEL path only ENQUEUES, and on the event loop an enqueue failure drops
  the event with a warning -- so a pre-audit that is not critical is a hope,
  not a gate, and a model turn could run with no record that anything asked for
  it. The write goes off-loop because a synchronous flush on the loop would
  freeze every session's turn. An unwritable sink returns 503
  audit_unavailable with nothing armed. This is not a new invention: it is
  the shape this subsystem's own autonudge_authz already uses for
  monitor_update and monitor_stop, and the 503 mirrors
  handlers/cron.py's existing audit_unavailable refusal for a grant it could
  not record.
• The refusals emit a best-effort denied event. The first revision audited
  only after fire_now returned, so the four guards denied requests and left no
  event at all -- a real hole. Making them critical would trade an audit-sink
  problem for a different failure while preventing nothing, since the request is
  refused either way; that is the disposition messaging/identity states for a
  deny.
• The terminal event after the fire stays best-effort, because by then the
  timer is armed and the invoked record has landed, so raising would replace a
  real result with a logging error.

Authentication is the same as the POST / PATCH / DELETE siblings on this
path: no new trust boundary, and this route is strictly less powerful than
the POST beside it, which arms a loop that can spend turns until a bound stops
it. Audited like DELETE, on both outcomes, because a delivered cycle spends a
model turn. Both refusal arms carry a literal status beside their code rather
than a computed one, because the error-code contract caps dynamic statuses --
computing one is how that ratchet gets defeated while looking like ordinary
refactoring.

Documentation surfaces -- both of them

The route is added to its owning spec,
docs/system-specs/modules/learn-cron-dashboard.md, whose AutoNudge section
enumerates this HTTP surface (AGENTS.md:133 requires the owning spec in the SAME
commit, and the first revision did not do it while its checklist claimed
otherwise -- that was wrong and is fixed). It is also added to the Monitor
loops row of docs/feature-map/README.md, which is a second, independent
enumeration I missed on the first pass for the same reason I missed the first
one: I fixed the doc surface I knew about and assumed it was the doc surface.

The local gate cannot catch this, and that is worth stating rather than
hiding. scripts/check_feature_map.py is opt-in (it no-ops without
FEATURE_MAP_BASE_REF), and even in diff-scoped mode it passes here, because it
asserts that a touched file appears somewhere in the map -- and
handlers/autonudge.py already did. Route-level completeness is enforced only
by the AUTOSDE feature-map-correctness rule, so this class of miss is
review-caught by design, not gate-caught. Verified by running the gate both
before and after the fix: rc=0 either way.

Collision note: #5186, #8919 and #8936 all edit the spec paragraph, so
whichever lands second inherits a conflict there; the insertion is kept to one
clause plus its rationale to make that resolution mechanical.

i18n

One i18n key, components.autoNudgePopover.trigger_nudge, in en.manual.json
plus all eleven translated catalogues (en.json is generated and was not
hand-edited; en-XA regenerated). No value was coined: each is that locale's
own imperative button form (from stop_loop / start_loop in the same
object) applied to that locale's own word for "nudge" (from
seconds_between_nudges in the same object).

Tests

New: test/test_autonudge_manual_fire.py (20 tests) and 10 added to
website/src/test/AutoNudgePopover.test.tsx (41 in that file now).

Both properties that matter more than the button are asserted rather than
described. The schedule claim has its own test
(test_the_next_automatic_nudge_is_a_full_interval_after_the_manual_turn_ends),
and the bounds get one test per gate -- the cap and the sentinel separately,
because a single test is only proven to the first gate that fires.

Mutations, each hand-applied with its anchor asserted unique first, each
reddening a distinct set with a distinct message:

mutation	reddened	observable
delete the timer arm	4	KeyError: 'lp-1' -- no task armed
neuter the not-active refusal	1	assert NudgeLoop(...) is None
neuter the mid-fire refusal	1	same shape, different test
arm before refusing mid-fire	1	Task cancelling -- the second observable of that test, which the first assertion would otherwise have short-circuited
delete the deadline move	1	assert 9999999999.0 != 9999999999.0 -- the assertion added because that write was otherwise unobservable
drop the in-stage half of the busy predicate	1	assert 200 == 409
drop the structured-monitor refusal	1	assert 200 == 409
send a body on the fire POST	1	the serialized message to be undefined
gate the schedule-row button on the loop rather than its active flag	1	expected <button> to be null
close the popover on success again	2	the close callback fired, and the typed edit was lost
drop the audit on any ONE of the four early guards	1 each	the parametrized case's own label -- feature disabled: one event, unknown loop: one event, structured monitor: one event -- so each guard is proven independently rather than only the first
record the busy refusal as success	1	assert 'success' == 'denied'
stop recording the loop id in the audit metadata	1	assert '' == 'lp-1'
pin the popover back to 420px	2 capture assertions	popover spans x=0..420 in a 320px viewport, trigger spans x=310..403
downgrade the pre-fire audit to best-effort	2	assert 200 == 503, and the order list showing critical=False
let the audit gate fail OPEN on a sink error	1	assert 200 == 503
fire BEFORE the audit gate	2	fired despite an unwritable audit sink, and ['fire', 'audit:invoked...']
leave the trigger enabled after a successful press	1 test + 1 capture assertion	04 the trigger is now disabled fails: nothing says the cycle is already armed
label a paused loop's primary button Save again	1	Start loop absent, and the active case asserted too so the fix cannot just move the confusion
drop the client-side armed deadline	1 capture assertion	04 the line now reads due fails: the countdown keeps showing the superseded cycle
stop creating the stop-sentinel file	1	the sentinel test, verified after a first control that changed both sides and proved nothing
reintroduce an await in fire_now	1	fire_now suspended. Any await between the guards and the arm reopens ...
move the button back into the action row	5	to have a length of 2 but got 3 -- the placement assertion; the other four are locator ambiguity because this mutation is additive rather than a clean move, so they are the mutation's shape and not evidence about those tests

A vacuity in my own test, found by mutation and fixed. The first version of
the "close the popover" mutation reddened only one test, not the
edit-survives one -- because the test helper hardcoded open={true}, so the
harness pinned the popover's presence and a close could not be observed. The
helper now drives open as a controlled prop the way the real parent does, and
the same mutation reddens both.

A change I could not justify by measurement, and said so instead of quietly
keeping it. I added flex-wrap to the schedule row alongside the width cap,
expecting it to be the thing that keeps the trigger on screen. Mutation says
otherwise: pinning the shell back to 420px reddens the narrow frame, while
removing the wrap changes nothing at 320px. I also wrote an assertion on the text
column to justify it, and that assertion FAILED in the shipping state too (161px
either way against a threshold I had picked at 180px) -- a gate that is simply
wrong, so it is deleted rather than tuned until it passed. The wrap is kept for
string length, since shrink-0 protects the button and a longer localized
countdown has only this row to give, and its comment now says that instead of
claiming to be the fix.

A test with an escape hatch, found by mutation. The first version of the
remove-race test branched on the returned status and asserted final state --
but "the loop is gone AND a timer was armed" is true both when the fire
legitimately armed first and when it resurrected a removed loop, so removing the
lock did not redden it. The second version asserted ORDER, and still did not
redden: one sleep(0) does not let the removal commit, because the lock
acquisition is several awaits deep, so the dangerous interleaving was never
reached. Only the third version -- draining until the removal actually lands --
measures anything. Two passing mutations in a row on the same assertion, each of
which I had to treat as a MISSING measurement rather than a green one.

A mutation that passed and therefore proved nothing. My first ordering
mutation assigned svc.fire_now as a reference instead of moving the call, so it
changed no behaviour and the suite passed. A passing mutation is a MISSING
measurement, not a green one; re-done as a real swap, it reddens both tests.

A second self-inflicted miss worth recording. My mutation harness restores
the file from HEAD, which is only correct when the code under test is
committed. It was not, so the first mutation round silently reverted the audit
fix itself; the tests then failed, which is how I noticed. That accident doubled
as a whole-fix mutation (both new tests fail with the fix absent), but the
sequence is commit-then-mutate and I ran it out of order.

The predicted doc collision happened, and both sides are kept. #8936 landed
on main and edited the same Monitor loops row and the same spec paragraph this
change touches -- the collision this description has flagged since the first
revision. Resolved by taking main's line and re-applying my insertion into it,
not by choosing a side: the feature-map row keeps main's "also surfaced read-only
on the Crew Members drawer" wording AND gains the fire route, and the spec
paragraph keeps main's new prose AND my clause, re-applied verbatim from my own
side rather than retyped. Both resolutions assert their anchors and assert that a
sample of main's new text survived, so silently dropping the other side would
fail rather than merge.

Rebased onto 36ffa67ea to pick up someone else's test fix. Shard 4 was
failing on two tests in test_snapshot.py::TestNotificationCopyWhenNoLiveFileExists
-- and on six other open PRs at the same time, two of them on the identical tests
with identical assertion messages, which is what established it was not this
diff. fix(test): scope the notification-copy ordering triggers to the destination open (#8992) had already landed on main but this branch was based before it. The
rebase is clean: one commit, the same 27 files, and main's 17-file delta has ZERO
overlap with them (verified with an injected canary so the detector is known to
fire). That class now passes 26/26 and the suite 130/130 locally.

Two CI reds on an earlier head, neither attributable to this diff, both
checked rather than assumed. Coverage Gate is derived -- its own log reads
backend-test=failure -- failing closed -- so it was one failure, not two. That
one failure was test_snapshot.py::...test_a_FRESH_gateway_still_orders_the_copy_against_a_delivery,
a race detector with a 1.0s window: it passes 129/129 locally, was introduced by
#8576 BEFORE this branch's base, and drives
dashboard_state._notification_io_pool -- a module singleton in a file this diff
does not touch, and a different pool from the loop default executor the new
asyncio.to_thread uses. Separately, test_autonudge_reconciler.py errors 17
times locally on fixture 'event_loop' not found; that reproduces with 17
identical errors on a pristine base worktree with these files absent, so it is an
inherited harness gap and not part of the CI failing set.

Gates run locally, exit codes read without a pipe: black gate (24 files in
scope) 0, flake8 0, isort 0, error-code contract 0, docs-lint.sh 0, brand-name
gate 0, the feature-map gate 0 in diff-scoped mode, 233 neighbouring autonudge
tests 0, the 266-test test_sel.py suite 0, the 129-test test_snapshot.py suite 0, the i18n chain 0 including its diff-scoped zero-tolerance checks
([added-lines] 0, [vs-base] 0, [unit-added-lines] 0), tsc --noEmit 0,
eslint 0, vite build 0, merge-tree clean. mypy reports 4 errors in
transcribe.py and cloudwatch.py -- identical count and files on a pristine
base worktree in the same harness, and none of the three touched src/ files
appears anywhere in its output, so they are inherited.

Manual verification

website/scripts/capture-goal-trigger-nudge-8212.mjs drives the real built SPA
behind serveDist with every /api/** call answered from fixtures, and
asserts as well as photographs -- a PNG cannot fail, so it exits non-zero
unless each frame renders what this PR claims. 10 assertions, all passing,
including that the Stop/Save row holds exactly two buttons and that the typed
edit survives a refusal. Labels are read from the catalogue, so a key rename
breaks the capture loudly instead of silently photographing the wrong element.

Screenshots / video

Armed loop -- the button sits on the schedule line beside the countdown it
acts on, and the action row below is back to two controls.

Paused loop -- the control frame. The button is absent, because every terminal
bound leaves the loop inactive and the server refuses to fire one, so a button
there could only ever produce a 409. Without this frame the one above proves
only that a button can render, not that it renders when it should.

Refused during a live turn -- the reason names the outcome and the next step,
the popover stays open, and the typed-but-unsaved goal is still there. No still
of a button can show any of that, which is why it is photographed rather than
argued.

A successful press -- the claim that had no evidence until this round. The
schedule line reads "Next cycle due, fires after the current turn", the popover
is still open, and the unsaved edit is still in the box.

The 320px floor. The trigger wraps below the schedule line, fully inside the
viewport and pressable; nothing is clipped.

The press now acknowledges itself

A successful press left the button re-enabled and unchanged, so the only signal
was the schedule line's wording -- and a usability reader would not press it
again because they could not tell whether that would double the nudge. It would
not: the cycle is already armed. The button is now disabled while a cycle is due,
derived from the same countdown the schedule line renders so the two can never
disagree, and with no new string. Frame 4 asserts the disabled state, and the
capture drops to 20/21 without it.

Two further UX notes are declined with reasons rather than silently. "Stop loop"
being a DELETE with no confirm is real, but it is PRE-EXISTING behaviour that
this change only made more visible by putting "Paused" beside it; hiding or
relabelling a destructive control is its own change with its own review, not a
rider on a trigger button. And a "Save & start loop" label to disambiguate resume
would need a new key in thirteen catalogues for a nuance the existing
start_loop string already carries adequately.

Three advisory findings taken, one of them load-bearing

All six readiness conjuncts held on the previous head with every reviewer lane
green -- and three of those reviewers said CONCERNS behind the green, one of them
correctly. The spec paragraph in this same commit still described the
superseded design: it said fire_now "moves next_due_ts to now" and that the
value "is still written, so a restart ... resumes overdue", while the shipped
code deliberately writes nothing. The spec is the mandated first read for this
subsystem, so a maintainer would have reasoned from restart semantics the code
abandoned -- the exact property five rounds fought over. Rewritten to the
write-free design, with assertions that the superseded phrases are gone rather
than merely that new ones are present.

A paused loop had no visible way out. The primary button silently PATCHes
active: true but read "Save", so a reader found no resume control and called
both "Paused" and "Stop loop" risky. It now reads Start loop when the loop is
inactive, gated on active rather than on existence -- which was the bug -- and
it reuses the start_loop key the no-loop case already uses, so no catalogue
gains a string.

A successful press used to relocate the button under the cursor, because the
longer "due" wording made the row wrap. The schedule block now stacks in every
state, so the button occupies its own line at all widths and the narrow frame and
the desktop frame are one layout instead of two to keep in sync.

Not taken: First Principles notes that structured monitors share the root cause
and are refused toward a monitor API that has no fire route. That is accurate --
a monitor fire runs through _on_monitor_tick and is genuinely a larger change --
so it is deferred rather than smuggled in here.

Other suggestions / review dispositions

Taken from the SEVENTH round (GPT 5.6, one blocking, fenced and upheld): the
pre-persist _firing guard and the post-persist re-arm straddle a suspension
window the mutation lock does not cover, because the timer does not take that
lock. Correct, and mine.

The pattern is worth a maintainer's attention more than any single finding
here. Rounds 4 through 7 were each caused by the fix before them: making the
deadline durable added an await; the await opened a remove race; closing that
with a transaction still missed the cancellation shield; and the transaction does
not cover the timer, which is this round. Every finding was real and none was
overridden -- but the durability requirement that started the chain is the one
the adjudicator itself judged advisory, and four concurrency changes to a hot
path is a larger diff than the feature warrants. A maintainer may reasonably
prefer the synchronous _persist_soon form from the second revision, which has
none of these windows because it has no suspension point, and accept the
documented "a lost write degrades to a fresh full countdown" trade that the other
eleven callers in this file already accept.

Taken from the SIXTH round (GPT 5.6, two blocking, both upheld): the shielded-task
contract this file already uses, and the 320px floor. Both were mine, and the
first is the same shape as the round before it -- I fixed a concurrency hazard by
adding a transaction and still missed the cancellation shield its two siblings
carry a few hundred lines away. Six rounds in, the pattern in this PR is that
every concurrency fix has needed the next one; the reviewer has been right each
time and none of these was overridden.

Taken from the FIFTH round (GPT 5.6, two blocking, both mine and both caused by
the previous rounds' fixes): the durable-persist await opened a remove race, and
the refusal audits ran a possibly-initializing SEL call on the event loop. Worth
stating plainly, because it is the honest shape of this PR's history: rounds 3,
4 and 5 were each created by the fix before them, and the round-4 change --
which the adjudicator itself thought advisory -- is what introduced the
upheld-blocking race. The property Opus verified in round 1, that the guards are
synchronous with no awaits between check and mutate, is exactly what I broke and
have now restored by other means.

Taken from the FOURTH round (GPT 5.6, blocking, security-class): the new deadline
was acknowledged with a 200 before it was durable. Worth recording that the
bot's OWN adjudicator argued this should be advisory -- it found the crash window
sub-millisecond and the degradation self-correcting, and concluded the finding
"clears the FLAG bar rather than the unbounded-harm bar the fence assumed" -- but
the security fence withholds such findings from adjudication, so the blocking
verdict stood. I fixed it rather than seeking an override, because checking it at
source turned up an argument neither the finding nor the adjudication made: the
previous round's critical=True audit means a lost deadline write now leaves a
durable record of a fire that never happened. My first instinct -- that awaiting
would diverge from the subsystem -- was wrong, and _persist_soon's own
docstring is what corrected it.

Taken from the THIRD round (GPT 5.6, blocking, security-class and fenced): the
fire ran before its audit was guaranteed. This one was a direct consequence of
the previous round's fix -- adding the refusal audits put a non-critical write on
the success path too -- and the finding is correct: critical is what separates
an audit gate from an audit hope. Adopted in the repository's established form
rather than invented, and the ordering, the durability and the fail-closed
direction are each pinned by their own mutation.

Taken from the SECOND round (GPT 5.6, both blocking, both mine): the missing
feature-map row, and the four early refusals that denied a request without
emitting a SEL event. Neither was a false positive and neither is overridden.

Taken from the first round: the AUTOSDE row cap (blocking, GPT), the silent loss of an
unsaved edit and the jargon refusal string (UX, advisory), and the missing spec
route (Design, advisory -- and an AGENTS.md MUST I had violated).

Declined, with the mechanism rather than a preference. Design suggests
dropping the busy pre-check entirely and letting the existing
refuse-and-rearm-with-backoff machinery deliver the nudge after the turn ends.
It is a real argument -- the "3KB+" reason is about injection-queueing rather
than deadline-arming -- but it costs two things. The press would return 200 and
visibly do nothing, which First Principles independently named as a harm in this
same round; and the refused fire re-arms via _arm_timer(delay=backoff), which
does not touch next_due_ts, so the popover would read "due" while the real
next attempt is a backoff away -- a displayed deadline that is wrong. A visible
409 costs one pre-check and lies about nothing.

Also declined: a title on the button naming the cycle cost. Leaving the
popover open already makes that observable -- the header's cycle readout advances
when the fire delivers -- so a new key in twelve catalogues to restate it is not
earned.

Residue, named rather than left implicit. The refusal is still English prose
rendered verbatim, as all 13 error bodies in handlers/autonudge.py are. Per
RFC 9457 the code is the contract and error is advisory; localizing from the
code is the right fix and no autonudge consumer implements it, so that belongs to
all 13 at once rather than to a one-off divergence here.

Related Issues

Refs #8212

Not related, recorded so a reader does not conflate them: #8926 is a timeliness
defect in the observation-gated wake path. This change does not touch
_monitor_tick_is_quiet, and the popover never addresses a structured monitor
(api_autonudge_get filters them out, and the new route refuses one).

Checklist

• At most two commits (one), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated -- the owning spec carries the new route
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Reusing the scheduled _timer path via a write-free, no-await fire_now is the right shape: every terminal bound applies with zero duplicated gating.

Suggestions

• The fire's audit-or-deny gate and refusal audits live in the HTTP handler, unlike authorize_and_add_nudge/authorize_and_update_nudge, which the spec places in transport-agnostic autonudge_authz.py precisely so "no future non-HTTP caller can bypass them" — since the description already defers a monitor/MCP fire surface, an authorize_and_fire_nudge chokepoint beside its siblings would let that future caller inherit the gate instead of re-implementing it.

[DESIGN-REVIEWED] 05bf62d

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 05bf62d0cefa1259f5bfac0ab542140622c2caf9 and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- website/src/components/AutoNudgePopover.tsx:238 -- direct fetch(...) violates use-react-query and can overwrite a newer autonudge_state frame with the older POST payload -> Fix: use useMutation and apply onChange only if latest.current.loop === loop.
[GPT-REVIEWED] 05bf62d

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

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 05bf62d0cefa1259f5bfac0ab542140622c2caf9 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All the claimed existing mechanisms check out: the two-term busy predicate has 20+ consumers, the queue-refusal rationale exists verbatim at slack/gateway.py:6135, AUTONUDGE_LOOPS_QUERY_KEY is a real shared key with three prior consumers, and both cited AUTOSDE rules (320px floor, max-two-buttons-per-row at :230) exist. Screenshot dirs and capture scripts follow a repo convention with 1164 and 423 siblings respectively. Final review:

First-Principles-Verdict: PASS

Every item traces to issue #8212, a named usability report, or a documented rule; the one deep decision — deleting the deadline write — removes a race class instead of guarding it.

What this change ships

Intent: let an operator who already knows the watched thing changed run a goal loop's next cycle now instead of waiting out the idle gap — an ADDITION.

1. "Trigger nudge" button on the goal popover's schedule line, active loops only — justified (issue Add a "Trigger nudge" button to the goal loop popover #8212; no out-of-band fire existed anywhere)
2. New POST /api/autonudge/{loop_id}/fire + fire_now, reusing the ordinary timer path so every terminal gate still applies — justified; 1 consumer, singular form
3. Press visibly refused (409) when mid-turn, mid-fire, paused, or a structured monitor — justified (reuses the fire path's recorded refuse-don't-queue decision)
4. The fire is audit-or-deny; refusals audited best-effort — justified (agent/audit boundary, matches existing SEL split)
5. Paused loops now say "Paused" where the button would be (one i18n key) — justified (named blind-reader report)
6. Primary button reads "Start loop" instead of "Save" on a paused loop — rides along; named failing reader, existing key reused
7. Button disables once a cycle is due — justified (press acknowledgment without a new string)
8. Popover width viewport-capped below 420px — derived (AUTOSDE 320px floor, website/AUTOSDE.yaml:90)
9. Countdown flips to "due" client-side; shared loop cache invalidated — mechanism-level given the deleted server write
10. Capture script, 5 PNGs, backend/frontend tests, same-commit spec updates — repo convention (423 sibling capture scripts) and the AGENTS.md same-commit rule

Watch

• The named root cause — writers to next_due_ts that hold no lock (autonudge.py:3979 is the one cited) — is deferred, not fixed; the PR deletes its own write rather than serializing those writers. Accepted-and-deferred: the general fix is genuinely larger than this change, but the sibling writers remain for the next feature to trip on.
• The intent file was truncated at 8000 bytes, so whether items 6–8 are declared in the description tail could not be verified; each has a named justification regardless.

[FIRST-PRINCIPLES-REVIEWED] 05bf62d

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

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

All evidence is in hand — blind read, five screenshots, the diff, and the PR body (no recordings, but the popover closes on stop/save so the active↔paused flip never happens on screen). Reconciliation: the primary "Trigger nudge" button was read correctly and the reader would use it; the residual findings are vocabulary drift and the post-press disabled state, all grounded in the reader's words.

UX-Verdict: CONCERNS

The button works and reads correctly, but three nouns for one lifecycle — nudge/cycle, stop/paused/start — leave a cold reader guessing at every state change.

Watch

• "Trigger nudge"'s success feedback is the adjacent line flipping to "Next cycle due…" — a different noun; reader: "I can't tell if one nudge equals one cycle." Every press, comprehension friction, persistent. Fix: one noun — button "Run next cycle now", or schedule line speaks of nudges.
• The post-press disabled state doesn't read as disabled: shot-04 shows the button with a bright accent ring, near-identical to enabled shot-03; reader: "clicking would try to send again, or maybe nothing … no idea which — hesitant, since I might send the same reminder twice" — the exact question disabled was added to answer. Every successful press, persistent. Fix: on cycleAlreadyDue swap the label to the state ("Cycle queued") or drop the focus ring and mute harder than opacity-50.
• Pressing "Stop loop" yields a state labeled "Paused" (new loop_paused key), resumed by "Start loop", with "Stop loop" still offered on the paused frame; reader: "no idea whether 'paused' is different from 'stopped'" and, on the paused frame's Stop loop, "I can't tell what state I'd end up in." Fix: one term pair ("Paused"/"Resume loop" or "Stopped"/"Start loop") and hide "Stop loop" when the loop is already inactive.

[UX-REVIEWED] 05bf62d

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

I have enough to validate. Let me work through both candidates against the code I've now opened.

Candidate 1 — button disabled={saving || cycleAlreadyDue} where cycleAlreadyDue is true whenever countdownText is the "due" string.

Tracing nextCycle (autoNudgeLoop.ts:92-98): an active loop reads 'due' only when next_due_ts > 0 and next_due_ts - nowTs <= 0. In normal operation the server timer fires at that same deadline, clears next_due_ts to 0 on delivery, and re-arms a fresh interval, so "due" is transient. The persistent-"due" cases the candidate names are: (i) clock skew — bounded and transient, and (ii) a deferred/overdue fire — but a deferred fire happens precisely when the session has a turn in flight, and in that state the fire route itself returns session_busy (handler line 292), so a disabled button is consistent, not a lost affordance. The "genuinely overdue and un-armed" state relies on _reconcile_forever behavior the candidate never opened. (a) is unestablished (the candidate's own note concedes the blast radius is unverified), and the outcome is a UX-heuristic judgment, not a wrong observable. Does not reach 80. Dropped.

Candidate 2 — onChange({ ...data.loop, next_due_ts: Date.now() / 1000 }) optimistically reads "due"/disabled, allegedly stuck if the armed cycle is later refused by _on_fire.

In the success case this is the intended acknowledgment the PR designs for (the disabled state is deliberate; screenshot frame 4 pins it). The alleged "stuck" case requires a turn to start in the tiny window between the delay=0.0 arm and _timer running, _on_fire refusing, AND no reconciling event on the backoff re-arm — which the candidate explicitly did not trace ("if one is, this is a non-issue"). Even granting it, the state is transient (reconciled by the next autonudge_state frame or on reopen, and invalidateQueries is called), bounded by backoff — minor UX, not crash/data-loss/security. (b) and (c) rest on unopened backend behavior. Does not reach 80. Dropped.

Neither candidate survives falsification, and I found no separate grounded defect in the handler (fire_now's status mapping, the audit-or-deny ordering, and the busy affordance are all internally consistent and covered by the added tests).

No findings.

[OPUS-REVIEWED] 05bf62d

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

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

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] a2e1e29

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

[bolichen97]

Tech Lead review: approved.

Authorization scoping — the new POST /api/autonudge/{loop_id}/fire sits on the same dashboard-token-gated path as its existing POST/PATCH/DELETE siblings and introduces no new trust boundary. Cross-user triggering is not representable here for the same reason it is not for the siblings: autonudge_authz documents that ownership is not checked by loop id ("the REST route is user-token gated for the dashboard UI"), and identity-bearing callers resolve the id from their own binding key. The route is strictly less powerful than the POST beside it, which arms a loop that spends turns until a bound stops it, and it cannot buy a turn past a bound: fire_now re-arms through _arm_timer so the stop sentinel, cycle cap, wall-clock budget, approval-stall stop and probe gate all still apply, and not loop.active covers every non-removing terminal bound in one condition.

Scope — proportionate. Of +1759/-12 across 27 files, production code is autonudge.py +89 (mostly docstring), handlers/autonudge.py +200 (mostly docstring), server.py +2 route registration, and AutoNudgePopover.tsx +144/-7; the rest is 744 lines of new backend tests, a 353-line capture script, 197 test lines, and two-line locale additions. No unrelated surface touched.

Audit — the fire is audit-or-deny on a critical=True write awaited off-loop before anything is armed (503 audit_unavailable, nothing armed), refusals emit best-effort denied events, and every exit routes through one of the two helpers so a later guard cannot silently skip the record.

Unresolved blocking findings — none. All 66 check-runs are success/skipped with zero failures on 05bf62d, all five reviewer lanes are green with 0 annotations, and no human review requested changes. The residual notes are advisory: Design (PASS) would prefer an authorize_and_fire_nudge chokepoint in autonudge_authz for a future non-HTTP caller — a reasonable follow-up, not a defect for the HTTP surface this PR ships; UX (CONCERNS) flags nudge/cycle vocabulary drift; GPT's non-blocking note on the raw fetch is mitigated in the diff by the AUTONUDGE_LOOPS_QUERY_KEY invalidation, and is style debt worth a follow-up rather than a block.

i18n — compliant. Two keys (trigger_nudge, loop_paused) added to en.manual.json plus all eleven translated catalogues with en-XA regenerated; en.json is generated and correctly not hand-edited. The paused-state and Start loop labels reuse existing keys, so no catalogue gains an unverifiable hand-authored sentence. The diff-scoped zero-tolerance i18n checks pass.

Merging is still gated on the kirocrew-ux-reviewers team approval required by the UX ruleset, since website/src/components/AutoNudgePopover.tsx is a non-test, non-locale website/src/** file.