feat: expose session monitors to agents

[kyleseaman]

Stacked change: PR 5 of 8

Stack: #5180 → #5181 → #5182 → #5183 → #5184 → #5185 → #5186 → #5305
Base: main (#5180–#5183 merged)
Next: #5185

Problem / Motivation

Agents and sessions cannot create or manage structured monitors, and the runtime has no authoritative controller that applies bounded decisions consistently across transports.

Why it matters

A model-only polling loop cannot provide zero-turn unchanged probes, restart-safe claims, exact completion accounting, strict caller ownership, or durable terminal evidence.

What changed (motivation → approach → change)

• Add stateless session-scoped monitor_watch, monitor_inspect, monitor_update, and monitor_stop MCP tools.
• Apply mutations through authenticated session directives and a durable controller with generation-safe updates and bounded retry/evidence deadlines.
• Audit disabled, unsupported, and authorization-refused structured create, update, and stop directives as denied rather than successful applications.
• Add typed DISPATCHED, BUSY, and UNAVAILABLE delivery across dashboard, Slack, and Discord, including an authoritative non-waiting session claim that carries the gateway-authorized Discord generation through dispatch and refuses a concurrent !new; authorization refusal before provider entry returns UNAVAILABLE without recording or persisting a successful session; after the hook-bearing turn is accepted, later Discord errors or an outer timeout remain DISPATCHED for bounded evidence recovery.
• Recheck runtime before a BUSY redispatch and preserve terminal outcomes against late delivery or evidence-expiry callbacks.
• Distinguish genuine provider end_turn completion from ACP compatibility-synthesized terminal events, so ordinary successful turns settle and charge exactly once while fabricated recovery evidence remains excluded.
• Preserve a raw Slack completion that arrives before timeout or shutdown cancellation so its disposition and spend are recorded exactly once before cancellation propagates.
• Treat Slack and Discord shutdown refusals before provider entry as transient BUSY so the durable claim retries after restart.
• Revalidate the persisted wake claim before transport handoff and again at each final turn-start boundary; dashboard waits for its unattended-turn permit before appending or running the wake, so a stop while queued cannot leave a stale instruction or start an action.
• Report a dashboard handoff as DISPATCHED only after the background permit and final authorization are accepted; a full-queue timeout returns BUSY, and an already-dispatched claim cannot be admitted twice.
• Restart a terminal monitor with compare-and-replace on the exact monitor ID and configuration generation read by the request, so concurrent restart or edit races return 409 instead of replacing the winner.
• Run the synchronous session-shutdown gate in the same yield-free Slack and Discord stream-start span, before accepting monitor completion correlation, so teardown cannot miss a newly opened turn.
• Route structured Slack wakes through the authenticated directive-aware channel driver so monitor updates and stops apply before raw completion accounting.
• Refuse structured watch creation from Webex before persistence while retaining its finite legacy prompt-loop path.
• Refuse structured monitor creation and edits in crew/member and temporary/incognito session modes, rechecking at final admission while preserving owner Stop.
• Summarize [monitor wake] turns without feeding those operational wakes into memory consolidation.
• Expose owner-only browser monitor REST routes, strict-internal session inspection, and safe public projections while preserving legacy AutoNudge contracts.
• Audit a missing or unsupported strict session binding as a denied session-monitor read before returning 401.
• Keep structured state out of legacy reads and generic WebSocket broadcasts, and require the dashboard owner before a legacy DELETE can stop a structured monitor.
• Resolve critical SEL sinks entirely off the event loop and prevent legacy creates from replacing an in-flight structured wake.
• Retain in-flight correlation when an action stops its own monitor so the final raw completion is charged exactly once.
• Refuse replacement while a wake is in flight so the original monitor ID remains completion-accountable.
• Keep controller, provider, and pull-request parser imports behind runtime boundaries so a disabled gateway boot stays cold.
• Preserve unsupported future-version monitor records as opaque, inactive state and never arm them for execution.
• Commit monitor replacement only after its atomic snapshot succeeds, leaving the previous record and timer running when persistence fails.
• Publish an already-durable staged transition before propagating cancellation, so live state and restart state cannot diverge after a drained write.
• Make dashboard monitor creation atomically create-only under the service lock, while retaining explicit agent replacement semantics.
• Clear prior comparison, decision, and wake baselines when a target or objective changes, so the new identity never displays the old decision.
• Stage every structured transition—including configuration, session-close, wake claims, handoff, completion, evidence recovery, and controller deactivation—and fsync its replacement before publishing live state or changing timers.
• Persist a user stop before mutating live state or cancelling its timer, so a failed disk write cannot resurrect the monitor after restart.
• Keep legacy AutoNudge integrations compatible when they implement the established service surface without structured lookup helpers.
• Preserve accepted completion evidence across a failed session-close rollback so the original turn remains exactly-once accountable.

Tests

• Controller, scheduler, restart, deduplication, budget, terminal-transition, and guarded dashboard-dispatch coverage
• MCP schema, strict session identity and binding-denial audit, directive application, authorization, audit, and REST routes
• Dashboard, Slack, Discord, session-close, shutdown-boundary, busy-race, refusal, stop-before-completion, unsupported-version and failed-replacement persistence, and legacy compatibility paths

Manual verification

N/A — controller, API, session, and transport contracts are covered with deterministic integration tests; no live channel credentials are required.

Related Issues

N/A — implements the live tool/controller layer specified in #5180 and depends on #5183.

Checklist

• Commit count satisfies the stacked PR hygiene limit
• Existing tests pass and new tests cover the new behavior
• Self-review completed; code follows project style guidelines
• Documentation updated where applicable
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

N/A — the repository template does not yet supply final CLA wording.

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 7b90924

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

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Reviewed: RFC (in base, extended here), monitoring/controller.py, mcp_tools/control.py, autonudge.py, session_directive_apply.py, and the Slack/Discord/dashboard dispatch changes.

Design-Verdict: CONCERNS

Sound RFC-driven controller design, but the new exactly-once machinery is poured into two already-huge legacy modules instead of the monitoring package it belongs to.

Watch

• The structured monitor's durable state machine (~25 new methods, ~900 lines: apply_monitor_probe, record_monitor_*, close-retire/rollback) lands inside autonudge.py (now ~4,000 lines) behind the controller's _Service protocol. Sharing the NudgeLoop slot is what enforces one-automation-per-session, but every stack PR deepens the entanglement the RFC's own migration plan (legacy loop retirement) will have to unwind — consider hosting the monitor methods in monitoring/ and composing them into the service now, while the seam is fresh.
• The claim-revalidation → shutdown-gate → typed-result choreography is re-implemented per transport as nested closures in the 6,000-line gateway orchestrator (_fire_dashboard_nudge alone gains seven inner functions). The controller keeps state authoritative, but where each transport places its final revalidation is per-transport convention defended only by that transport's tests; a fourth transport (Webex is refused today) must re-derive it.

Suggestions

• During rollout the agent sees both monitor_start (legacy, one turn per cycle) and monitor_watch (zero-turn probes) with sibling names, and monitor_stop answers "no structured monitor" on a legacy loop — bias the monitor_start description toward "prefer monitor_watch for GitHub PRs" so the LLM doesn't buy the cost the RFC exists to remove.

[DESIGN-REVIEWED] 7b90924

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 7b90924eca7bb132721143cdf9ce0e2a6f3185e0 — 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.

I've completed the review pass: intent file, full source diff, the RFC milestone the stack implements, and consumer-count greps for the new surface. Final review follows.

First-Principles-Verdict: CONCERNS

Six browser /api/monitors* routes ship with zero consumers — the RFC assigns dashboard controls to the next slice — plus two undeclared behavior riders.

What this change ships

Intent: let an agent arm a server-probed PR monitor that wakes its session only on actionable change — an ADDITION, PR 4 of the recorded RFC (docs/request-for-change/rfc-token-efficient-monitors.md:509 even prescribes this exact title).

1. Four agent tools: monitor_watch / inspect / update / stop — justified (RFC PR 4)
2. Live probe controller; unchanged/retry/terminal probes cost zero turns — justified
3. Typed DISPATCHED/BUSY/UNAVAILABLE delivery with durable evidence deadlines on dashboard/Slack/Discord — justified
4. [Monitor wake] injected turn, excluded from memory consolidation and summary counts — justified
5. Six browser /api/monitors* routes incl. restart — declared, zero consumers
6. Agent monitor_start and browser create become create-only (409 on replace) — changed default, obliquely declared
7. Legacy autonudge reads/PATCH exclude structured records; DELETE owner-gated — justified
8. Webex, crew/member, incognito/temporary sessions refused — justified
9. Cumulative provider-error budget stop (provider_error_budget) — undeclared
10. Malformed monitor records replaced by a quarantine row that discards the raw payload — undeclared (reverses the earlier preserve-for-inspection behavior)
    (more items exist — non-waiting wait_if_busy claim, synthetic_completion, black-baseline prune; these are declared or trivial)

Watch

• Grepped api/monitors under website/: 0 hits; only registration and tests reference the six browser routes. RFC PR 5 (feat: show bounded monitors in the dashboard #5185, "Truthful dashboard monitor experience") owns the browser controls — these routes land one slice early with nothing calling them.
• Items 9 and 10 change behavior of the already-shipped substrate and appear nowhere in the 42-bullet description; the spec edits are their only trace.

Subtractions

• Drop the budgets= parameter (and its mutual-exclusion guard) from AutoNudgeService.update_monitor — both production callers (autonudge_authz.py:119 via handlers and directives) pass only budget_patch; 0 consumers of the budgets arm.
• Delete _serialize_monitor (dashboard/handlers/autonudge.py:114) — a byte-identical alias of _serialize at 7 call sites; use _serialize.
• Defer the six /api/monitors* route registrations (dashboard/server.py:1481-1486) to feat: show bounded monitors in the dashboard #5185, which ships their only consumer.

[FIRST-PRINCIPLES-REVIEWED] 7b90924

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] 7b90924

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

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

[kyleseaman]

Fixed the two newly actionable failures in the PR that owns the behavior: Slack legacy monitor completion now rejects synthetic timeout evidence before accounting, and the session-close restoration regression uses the public accepted/dispatched transitions. Focused verification: 195 monitor tests passed; black, subprocess-encoding, isort, flake8, Linux mypy, SDK-boundary, docs, harness-parity, and brand gates passed.

[kyleseaman]

Fixed in a6d95f73a: a failed compatibility stop now raises the directive denial path, so the SEL terminal event is denied rather than success. The complete directive-apply suite passes.

[kyleseaman]

Fixed the current backend shard failure at this owning layer. The cancellation regression now uses authoritative max_tokens completion evidence (and expects a failure disposition) instead of ambiguous end_turn, which is intentionally ignored. Focused composed-stack verification passes.

[kyleseaman]

Addressed the current completion-accounting blocker in b793000. Genuine provider end_turn events now settle successful monitor turns; the ACP compatibility path marks fabricated end_turn terminals with explicit synthetic_completion provenance, which dashboard and channel accounting reject. Focused dashboard/driver/ACP propagation tests pass (23 tests across the relevant selections).

[bolichen97]

Review: session monitors + controller

Reviewed origin/token-monitors-github-probe...origin/token-monitors-tools (59 files, +8285/−613) at 52293ce01. The PR description asserts 30+ concurrency/restart/exactly-once invariants, so the review targeted those specifically and verified findings by executing the real AutoNudgeService, MonitorController, apply_session_directive and the transport seams rather than by reading. The existing suite is green (293 passed across the 9 monitor/autonudge/slack modules; 256 across the controller set), so every item below is an untested path.

The two most serious are the same failure mode reached two ways: MonitorController.tick has no branch for wake_in_flight=True, wake_delivery=None, and that state is both persisted by restore_monitor_after_failed_session_close and left behind by any exception escaping _dispatch_claimed. In both cases the monitor reports active forever while doing nothing — and the first survives restart, which defeats the restart-durability invariant the description leads with.

Blocking

1. monitoring/controller.py:142 — an in-flight claim whose delivery marker is still None never expires and never re-dispatches; the monitor is wedged forever, across restarts. Expiry is gated on wake_delivery is DISPATCHED, but Slack/Discord call mark_accepted() at turn start while record_monitor_dispatched only runs after the whole turn returns, so wake_delivery is None for the entire action turn (up to _NUDGE_TURN_TIMEOUT=1800s) while _accepted_monitor_turns is set. Close the slot in that window → _retain_accepted_terminal_completion retains wake_in_flight=True + completion_evidence_deadline=stopped_at+7260 with wake_delivery=None; if history persistence then fails, restore_monitor_after_failed_session_close (autonudge.py:1785) clears outcome, sets active=True, and schedules the BUSY retry cadence but never sets wake_delivery = BUSY. tick() then matches no branch → return NO_CHANGE. Verified end-to-end: after restore, state is active=True wake_in_flight=True delivery=None evid_deadline=7380 next_due=140, and tick returns no_change at now=140, 200, 7381 and 1e9 — never calling record_monitor_completion_evidence_unavailable, never probing, never re-dispatching. _timer's finally re-arms from the past next_due_ts every _OVERDUE_REARM_SECS=10s forever (measured 44 tick invocations per wall second with the interval compressed 500×); on restart _load sets next_due_ts=completion_evidence_deadline and the same no-op loop resumes. Flipping the same state to DISPATCHED expires correctly on the first tick, which isolates the gate as the cause. Also reproduced straight from a persisted record with wake_in_flight:true and no wake_delivery key. Every escape is closed: update_monitor(target=…) → refused ("target or objective cannot change while a wake is in flight"), re-arm → refused, api_monitor_restart → refused ("only terminal monitors can restart"). One line closes it: staged_state.wake_delivery = MonitorDispatchResult.BUSY in the autonudge.py:1785 branch.

2. monitoring/controller.py:206 — an exception escaping _dispatch_claimed after the claim is persisted leaves the monitor wedged with no timer at all. except Exception does not catch CancelledError, and the TypeError at line 229 is outside the try. apply_monitor_probe persists the claim as wake_in_flight=True, wake_delivery=None, next_due_ts=0.0; if _cancel_timer fires from a concurrent stop/close mid-handoff, or a dispatcher returns a non-MonitorDispatchResult, none of record_monitor_dispatch_failure/_busy/_dispatched runs, and _timer's finally declines to re-arm because it requires next_due_ts > 0 (autonudge.py:2544-2549). Verified: a dispatcher returning True → TypeError: monitor dispatcher returned an untyped result, state active=True wake_in_flight=True delivery=None outcome=None next_due_ts=0.0, _arm_timer never called, tick returns no_change at now=300 and 1e9. Same for the CancelledError variant. The monitor shows active in the dashboard and monitor_inspect but is dead — no timer, no probe, no expiry, no stop reason; only a process restart retires it, and then only as BLOCKED/completion_evidence_unavailable. A wake_delivery is None fallback in tick plus mapping the contract violation to UNAVAILABLE closes both this and finding 1.

3. dashboard/session_directive_apply.py:283 — _monitor_watch omits replace_existing=False, so the agent's monitor_watch silently destroys a user-created legacy babysit loop on the same slot. The browser's POST /api/monitors refuses the identical situation with 409 (api_monitor_create passes replace_existing=False, handlers/autonudge.py:369), but the directive path inherits authorize_and_add_nudge's default True, and _add_monitor_locked's only replacement guard is existing_monitor is not None and existing_monitor.wake_in_flight — a legacy loop has monitor is None, so it is removed unconditionally. Verified: with a user loop "keep an eye on the deploy and page me" armed on chat-1, one monitor_watch directive returns "Structured monitor 20236018 started on this session." and the legacy loop is gone, with no error, no warning and no audit of the destruction. _monitor_start has the same shape in reverse — the legacy monitor_start tool (still promoted by agent.py:4867/5193 and the prepare-pr/pipeline-conductor skills) removes a structured monitor's durable record from memory and disk with emit=False, so no removed WS event fires and the retained terminal outcome is unrecoverable. These are the only two authorize_and_add_nudge call sites left on the default; every other route in this PR was hardened.

4. monitoring/decision.py:78 — the cumulative provider-error budget is tracked, surfaced, and never enforced. monitor_budget_reason checks runtime, agent turns and tokens only; the sole provider bound is consecutive_provider_errors + 1 >= max_provider_errors in _provider_error_decision. Verified: provider_error_count=10_000, consecutive_provider_errors=0, max_provider_errors=3 → monitor_budget_reason() == '' and decide_monitor(...) == retry_provider. A flapping target (error, success, error, success…) polls for the full max_runtime_secs — 48 probes at the default cadence, 960 at MIN_MONITOR_CADENCE_SECS=15 — while monitor_inspect reports provider_error_count far above the max_provider_errors the operator set.

5. slack/gateway.py:5596 — the Slack exception path reports DISPATCHED whenever _turn_started is set, even though the accepted-check immediately above proves the turn never reached the provider. _turn_started = True is assigned at line 5434, before TurnDriver is even constructed. Verified by monkeypatching gw.build_tool_gate to raise: _fire_slack_nudge(loop, "[Monitor wake] x") returns DISPATCHED with accepted == False. record_monitor_dispatched then bumps wake_count 0→1 and arms completion_evidence_deadline = now + 7260; no completion can ever arrive, so the monitor sits idle ~2h and is retired BLOCKED/completion_evidence_unavailable instead of retried as BUSY in 15s. The sibling except asyncio.TimeoutError branch (5581-5585) keys off .accepted and does the right thing — the two paths disagree. Note the 7260s constant is justified as "the normal turn ceiling is two hours" while Slack/Discord monitor turns are capped at _NUDGE_TURN_TIMEOUT = 1800.0, so the dead window is 4× the surface's own ceiling.

6. discord/transport_dispatch.py:308 — Discord fails in the opposite direction: monitor_result is seeded UNAVAILABLE, so any transient pre-acceptance failure permanently retires the monitor. A transient _render_config read error, an attachment/typing failure, a governance deny, or the generic except Exception at 888 before mark_accepted all leave the seed in place; record_monitor_dispatch_failure then sets active=False, outcome=TARGET_UNAVAILABLE, stopped_reason=session_unavailable and cancels the timer. Verified: record_monitor_dispatch_failure → outcome=TARGET_UNAVAILABLE active=False, versus record_monitor_dispatch_busy → outcome=None active=True next_due=+15s. Slack maps the same class to BUSY, so at least one of the two surfaces is wrong — a one-shot embedding hiccup permanently kills a 4-hour monitor. Also: asyncio.TimeoutError is TimeoutError and is an Exception subclass, so _fire_discord_nudge:5751's bare except Exception maps a 1800s turn timeout to UNAVAILABLE too, where Slack returns BUSY.

7. slack/gateway.py:6183 — the MonitorDispatchResult return contract is enforced only by bare assert isinstance(...), which python -O strips. Lines 6183/6187/6192 are the only runtime enforcement that each _fire_*_nudge returned the enum on the monitor path, and _delivery_result returns a plain bool whenever wake_message is None (_fire_discord_nudge:5750, _fire_slack_nudge:5582,5600 do return bools). Under PYTHONOPTIMIZE=1 a bool reaches _dispatch_claimed, matches none of the three is comparisons, and hits the TypeError at controller.py:229 — i.e. finding 2's permanent wedge. Because every str, Enum member is truthy, a mode confusion in the other direction would read BUSY/UNAVAILABLE as "delivered" and inflate cycle_count. This needs an explicit if not isinstance(...) mapping to UNAVAILABLE, not an assert — the file itself says so 300 lines earlier ("Guard (not assert): stripped under -O", gateway.py:5868).

Should fix

8. autonudge.py:2228 — completion_evidence_deadline is anchored on the tick-start now, not on when dispatch actually returned, so the documented +60s margin is consumed by up to 1800s of in-dispatch time. Verified: tick at 1000, turn actually starts at 2800 (1800s queued), deadline 8260 → only 5460s of evidence window for a turn whose own ceiling is 7200s, expiring 1740s before the turn can be cancelled by its own timeout. The timeout then clears the claim, and the authoritative completion that arrives afterwards hits the not state.wake_in_flight guard at :1938 and is silently dropped — agent_turns/input_tokens/output_tokens all stay 0 for a turn that really ran and spent 180k/40k tokens. (Double-charging is correctly prevented; the failure mode is zero-charge.)

9. autonudge.py:1952 — record_monitor_turn_completion sets wake_in_flight = False but never resets wake_delivery, leaving (False, DISPATCHED) — the exact combination the other six writers deliberately reset to None (1683, 1866, 1917, 2038, 2274, 2455). monitor_state_public_dict publishes it verbatim, so any inspect between completion and the next wake shows an idle monitor whose delivery reads DISPATCHED. Verified against the real service. wake_in_flight and wake_delivery are one state machine split across two fields with ten writer sites; adding a CLAIMED member and re-exposing wake_in_flight as @property: self.wake_delivery is not None makes the desynced state unrepresentable and keeps all 28 read sites working.

10. monitoring/models.py:471 — a quarantined record is never actually persisted, so it re-quarantines forever and pays a full fsync'd store rewrite on every boot. _load builds quarantine_monitor_state(...) (outcome=BLOCKED, stopped_reason='invalid_monitor_record') and sets _store_dirty, but monitor_state_to_dict short-circuits on _raw_payload is not None and returns the original malformed payload verbatim. Verified with created_ts='not-a-number': in-memory outcome=BLOCKED, on disk outcome=None, stopped_reason=None, payload byte-identical to the original; second boot re-quarantines with a WARNING+traceback and rewrites the same bad bytes. monitor_state_public_dict reports BLOCKED to the browser while disk records no terminal state at all. Same short-circuit means MONITOR_STOP_UNSUPPORTED_VERSION set at :629 is never durable.

11. autonudge.py:624 — the unsupported-version branch is an elif head, so such a record keeps active=True forever and no supported API can clear it. It skips both the terminal-record repair (active=False, wake_in_flight=False, next_due_ts=0) and the unwired-monitor guard. Verified with version:99: active=True next_due_ts=123.0 outcome=BLOCKED. _arm_from_deadline bails on the version mismatch so it never fires, but stop_monitor returns unchanged (outcome already set), api_monitor_restart 409s, update_monitor returns None — the dashboard shows a permanently "active" monitor that never runs and cannot be removed. Related: _load:642 reads loop.next_due_ts before _repair_number sanitizes it at :691, so a BUSY claim with an out-of-range persisted deadline is wrongly retired as completion_evidence_unavailable even though BUSY proves no action turn started — contradicting the branch's own comment.

12. autonudge.py:1698 — every monitor tick fsyncs the entire store, including NO_CHANGE ticks that move only probe_count/last_probe_at/next_probe_at. The persist call is outside every decision branch, and _monitor_snapshot_with_replacement serializes all loops, so N monitors ticking each rewrite all N records — O(N²) bytes per cadence period, under the global service lock. Measured on APFS SSD: 0.69 ms and 5.3 KB with 1 monitor; 3.54 ms and 274,866 bytes with 25 monitors — at MIN_MONITOR_CADENCE_SECS=15 that is 25 fsyncs + 6.7 MB per 15s, ~144,000 fsyncs and ~40 GB/day for cadence bookkeeping, and on a spinning disk or network volume an fsync is 10–50 ms taken under the lock, so legacy nudge loops block behind it. self._store_dirty already exists but is consulted only for the load-repair flush. Related: BUSY retry is a flat 15s with no backoff and re-persists byte-identical state — one long user turn produces ~480 retries (~126 MiB, 480 fsyncs) carrying zero new information, while the provider-retry path in the same file already has exponential backoff.

13. mcp_tools/control.py:1157 — monitor_update resolves its session with the wider _autonudge_binding_key while monitor_watch/monitor_inspect/monitor_stop all use _structured_monitor_binding_key. Verified: binding_key_for('webex:room-1') == 'webex:room-1' but structured_monitor_binding_key_for(...) is None. So a Webex session is refused by three tools in one step with a clear message and a denied SEL record, but monitor_update admits it, encodes a directive carrying structured-only keys, tells the agent "Monitor-loop update requested for this session", and only then raises _DirectiveDenied in the consumer. Separately, monitor_watch/monitor_stop skip their refusal and their denied SEL event when strict resolution yields an empty key (... is None and sk), contradicting the guarantee this PR added to docs/architecture/mcp.md; monitor_inspect (which drops the and sk) is the only one that behaves as documented.

14. dashboard/session_directive_apply.py:166 — monitor_stop's reason is schema-validated, stripped, redacted and encoded into the directive, then discarded. The consumer calls _monitor_stop(session_key) with no args (the function takes no args parameter), and stop_monitor writes the constant MONITOR_STOP_USER unconditionally. The SEL metadata is {"caller": …} only. So an agent calling monitor_stop(reason="PR merged, review complete") leaves an operator-visible terminal record reading only user_stop, and the reason is unrecoverable from both monitor_inspect and the audit trail. Every sibling directive (_monitor_start/_monitor_watch/_monitor_update) receives args.

15. monitoring/models.py:46 — MONITOR_PUBLIC_FIELDS omits created_ts and last_wake_reason_code, so neither the dashboard nor monitor_inspect can show elapsed runtime or why the last wake fired. Verified by diffing the dataclass against the tuple: declared - public == ['created_ts', 'last_wake_reason_code']. monitor_budget_reason stops a monitor on now - state.created_ts >= budgets.max_runtime_secs — the bound most likely to end a monitor — yet an agent sees max_runtime_secs: 14400 with no way to compute time remaining, then gets stopped mid-objective with stopped_reason: runtime_budget and no warning. _compact_monitor_inspection drops 11 more fields including wake_delivery, so the agent cannot tell whether its wake was DISPATCHED or is spinning in the 15s BUSY loop — the exact distinction the new typed result was introduced to make durable. Both lists are hand-maintained and can be derived (tuple(f.name for f in fields(MonitorState) if f.name not in {'extra_fields','_raw_payload'})).

Notes, lower severity

autonudge.py:2365/2398 — notify_user_input cancels the timer of a terminal-but-awaiting-evidence monitor and notify_turn_complete's not loop.active guard then refuses to re-arm it, so the bounded evidence window is dropped and the claim leaks for the process lifetime (verified: claim leaked: True); notify_turn_complete also has no monitor branch at all, so _arm_from_deadline's min(remaining, loop.idle_secs) clamp silently reinterprets the 7260s evidence deadline as a cadence-length poll, and can re-arm a monitor the controller deliberately disarmed at next_due_ts == 0. · handlers/autonudge.py:262/273 filter structured monitors out of the legacy GET while _observer still broadcasts them on the same autonudge_state channel, so the AutoNudge popover on such a slot shows an empty form, 409s on Save (autonudge_not_armed), then 409s again on PATCH (structured_monitor_requires_monitor_api) — both dead ends, and no /api/monitors consumer exists in website/ at this layer. · handlers/autonudge.py:633 answers {"ok": true} for a DELETE that removes nothing once terminal, and with create/PATCH/POST /api/autonudge all refusing, the slot can never be retargeted from the browser. · handlers/autonudge.py:567's new replace_existing=False also 409s over an inactive legacy tombstone, contradicting its own docstring and the documented "start or replace" contract. · gateway.py:6106 — the acceptance callback re-reads state.last_wake_fingerprint lazily while the hook captured it eagerly, making mark_monitor_turn_accepted's fingerprint guard a tautology and allowing the accepted marker and the completing hook to name different fingerprints. · models.py:208 validates the agent-turn ceiling against DEFAULT_MONITOR_AGENT_TURNS instead of MAX_MONITOR_AGENT_TURNS (both 8 today, so invisible). · token_usage_known is write-only with zero readers, and it records the one condition that voids max_tokens — on the ACP seam max_tokens can never fire. · mark_monitor_action_in_flight has no production caller: 47 lines duplicating the most safety-critical transition in the PR, with a subtly different precondition. · controller.py:265 re-implements security.redact_and_truncate, and the 4096-char cap is a blind suffix chop applied after redaction, so the only content it can delete is the trailing operator Next action: line (verified: instruction entirely gone at len 4096) while provider-controlled values are preserved in full. · controller.py:11 imports MONITOR_WAKE_PREFIX from dashboard.state, dragging 18 dashboard modules (172 total, 0.47s) into any import of the "transport-independent" monitoring package. · _write_state fsyncs the temp file but never the containing directory after replace_with_retry, so the rename itself is not durable — the "fsync before publish" invariant holds for file contents only. · One bound has three owners with two semantics (validation.py rejects, _bounded_int rejects, add_monitor/update_monitor silently clamp using a different constant pair), and MONITOR_UPDATE_SCHEMA admits max_runtime_secs=0 that MonitorBudgets then rejects two layers down. · Four new copies of the owner-gate / internal-secret-gate / redact-then-truncate helpers where canonical ones exist; the _require_monitor_owner copy writes a client-chosen X-Session-Key header into the audit caller field on a denied request and emits dashboard_owner_required instead of the standard owner_only. · Conventions: the new [Monitor wake] prefix was added to dashboard/state.py but not to session_summary._INJECTED_PREFIXES, so a monitor wake is counted as user intent by the chat-summary/intent path; code-style.md's owning-module index gained no monitoring/ row for 11 new limits; AGENTS.md's own injected-envelope list still names three envelopes; and two black-baseline prunes ride inside feature commits (both files verified genuinely black-clean, so the prune is correct — just not its own commit).

---

_{Execution-verified AI-assisted review (Claude Code) across 10 parallel angles plus a gap sweep, run against a local checkout at 52293ce01. Nothing was modified; nothing was posted anywhere else. Confirmed as not problems, so please don't re-spend on them: generation-safe compare-and-replace is sound (update_monitor bumps config_generation exactly when target/objective change, refuses that change while in-flight, and apply_monitor_probe rejects a stale generation, so the wake envelope's target always matches the probed target); double-charging is correctly prevented by the wake_in_flight guard under the lock; synthetic_completion is plumbed through every terminal-event producer, so removing TURN_STOP_REASON_END_TURN from _SYNTHETIC_STOP_REASONS leaves no unflagged synthetic path; _accepted_monitor_turns is correctly bounded; the wake path correctly skips compose_nudge_body/render_snapshot; MCP statelessness holds for all three new tools; and no dispatcher can leak a bool into _dispatch_claimed while asserts are enabled.}

[kyleseaman]

Addressed the current review feedback and restacked this branch in 41ef44c. Monitor dispatch recovery is bounded and completion provenance remains intact. Focused backend verification for this slice passes, and the composed upper-stack monitor suite passes 234/234.

[kyleseaman]

Addressed the current GPT blocker in 11966da. Supplemental provider failures now advance both total and consecutive provider-error accounting while preserving the readable canonical observation, matching the shadow path. Added a bounded-streak regression; the focused monitor controller suite passes 46/46. This PR is also consolidated to the two-commit hygiene limit.

[bolichen97]

Audit note — this PR is one slice of a declared branch stack, not a duplicate

A duplicate-detection sweep flagged #5184 / #5185 / #5186 / #5305 as overlapping at FULL coverage. That is a stacking artifact, and it is worth stating plainly so nobody acts on it: each branch physically contains the previous one, so the shared code is inherited and each PR's review diff overstates what it actually authored.

Proved mechanically, not by reading code:

• feat: expose session monitors to agents #5184 → feat: show bounded monitors in the dashboard #5185 — The claim of FULL/complete overlap is a pure stacking artifact, provable mechanically. git merge-base --is-ancestor pr/5184 pr/5185 returns true, and git log --oneline pr/5185..pr/5184 is EMPTY: feat: expose session monitors to agents #5184 has zero commits that feat: show bounded monitors in the dashboard #5185 does not already contain. feat: show bounded monitors in the dashboard #5185 = feat: expose session monitors to agents #5184's head plus exactly two further commits (4c7b130 "feat: show bounded monitors in the dashboard", 331c07d "fix: preserve monitor dashboard control"). Both branches share the identical merge base dc35c19 ("feat: add github pull request monitor probe (feat: add github pull request monitor probe #5183)", on main), so feat: show bounded monitors in the dashboard #5185's merge-base diff reproduces feat: expose session monitors to agents #5184's diff verbatim: all 61 files in git diff dc35c192f...pr/5184 --name-only are present in feat: show bounded monitors in the dashboard #5185's 107-file diff, with zero exceptions (comm -23 is empty), and src/kiro_crew/monitoring/controller.py appears as the same +278/-0 new file on both sides. That inherited body is 100% of the claimed overlap.

The two PRs are also cleanly split by surface, which is why they are complementary rather than redundant. #5184 is backend-only: 26 files under src/kiro_crew/ (monitoring/controller.py, mcp_tools/control.py, validation.py, session_directive.py, dashboard/handlers/autonudge.py, slack/gateway.py, discord/gateway.py, ...) and ZERO files under website/. #5185's own delta (git diff pr/5184..pr/5185) is 55 files, +4328/-278, of which 44 are under website/ — the dashboard consumer for the six /api/monitors routes #5184 ships with no consumer (the exact point the first-principles review bot raised on #5184): new website/src/components/SessionAutomationPopover.tsx (+587), new website/src/monitoring/automation.ts (+400, exporting normalizeAutomationRecord/deriveAutomationStatus/automationForSlot), new website/src/monitoring/contract.json (+36), MonitorRadar.tsx (+25), useWebSocket.ts (+235), chatSlice.ts, i18n keys across 13 locale files, and ~1,300 lines of new frontend tests (SessionAutomationPopover.test.tsx +480, automationNormalizer.test.ts +238, monitorFixtures.ts +77).

The 11 non-website files in #5185's own delta are strictly ADDITIVE refinements in service of that frontend, not a competing design: monitoring/models.py gains monitor_frontend_contract() (which is what contract.json is generated from), a _public_github_observation() bounded projection over the public boundary, and two new MonitorState fields (last_observation_status, last_observation_reason_code) with their post_init validation and monitor_state_from_dict enum coercion; autonudge.py gains 4 lines populating those two fields; mcp_tools/control.py adds the same two keys to _compact_monitor_inspection's field tuple. Because #5185 sits downstream, it is structurally impossible for it to present an incompatible design for #5184's requirement — it edits #5184's own lines as a successor, not as an alternative.

Neither has landed: 550ea7b, fc82d95, 4c7b130 and 331c07d are all absent from origin/main; main carries monitoring/{init,completion,decision,github_pull_request,models,shadow}.py but NOT controller.py, and website/src/monitoring/ and SessionAutomationPopover.tsx do not exist on main. Ironically the first-pass record itself, the cached PR data, states in prose "Stacked descendant (PR 6). Its branch is #5184's head plus 4c7b130 + 331c07d; the shared code is inherited, which per the brief is a stacked branch, not duplicate" — the FULL/complete_coverage=true coding of that same relation is what the scan surfaced as a duplicate signal.

• feat: expose session monitors to agents #5184 → refactor: route babysit through structured monitors #5186 — The FULL-overlap claim is a pure stacking artifact, and the first-pass note itself already said so ("Stacked descendant, not duplication", recommendation KEEP) -- the FULL/complete_coverage flags were never a duplication finding.

Proof of stacking: git merge-base origin/main pr/5184 == git merge-base origin/main pr/5186 == dc35c19, and git merge-base --is-ancestor pr/5184 pr/5186 returns YES. git log --oneline dc35c192f..pr/5186 is 7ce67a7, 04e9461, 331c07d, 4c7b130, fc82d95, 550ea7b -- the last two ARE #5184's entire branch (550ea7b "feat: expose session monitors to agents", fc82d95 "fix: preserve bounded monitor recovery"), and 4c7b130/331c07d53 are #5185's. So the merge-base-vs-main diff for #5186 (118 files, +14347/-1684) contains 100% of #5184 by construction. One REST call confirmed intent: #5186 is head token-monitors-babysit onto base token-monitors-dashboard, and its body declares "PR 7 of 8 / Base: #5185 / Next: #5305"; #5184's body declares "PR 5 of 8 / Base: #5183 / Next: #5185". GitHub therefore renders only #5186's own delta. The maintainer reviews it the same way: bolichen97's 2026-09-01 review on #5184 states it reviewed origin/token-monitors-github-probe...origin/token-monitors-tools -- the slice, not the range to main.

#5186's own delta (git diff 331c07d53..7ce67a772) is 25 files, +1158/-751, and none of it duplicates #5184: (1) babysit/SKILL.md rewritten by 744 lines to point at monitor_watch instead of prompt-owned polling -- a file #5184 never touches; (2) a new shipped user doc src/kiro_crew/docs/monitoring.md (+85) with its docs/README.md and docs/index.md rows; (3) src/kiro_crew/config/prompt.md guidance; (4) a behavioural policy change in validation.py banning unbounded loops -- MONITOR_START_SCHEMA and MONITOR_UPDATE_SCHEMA move max_cycles/max_runtime_secs from min_val=0 to min_val=1; (5) a new AutoNudgeService.stop_monitor_if_budget_exhausted plus its _Service Protocol row and the if await self._service.stop_monitor_if_budget_exhausted(loop.id, now=now): return MonitorDecision.STOP_BUDGET call site in MonitorController.tick; (6) an evidence_scope FieldSpec on MONITOR_WATCH_SCHEMA and monitor_start's demotion to "Legacy fallback for targets or objectives unsupported by monitor_watch"; (7) slack/gateway.py terminal-outcome notifications keyed on (loop.id, outcome, stopped_at) for SUCCESS/BLOCKED/BUDGET/TARGET_UNAVAILABLE; (8) a new test module test/test_babysit_monitor_scenarios.py (+476) plus four more test files #5184 does not touch.

Per-file judgment of the 14 shared paths: every one is a file #5184 creates or extends and #5186 then builds further on. src/kiro_crew/monitoring/controller.py does not exist on origin/main at all (git ls-tree of the monitoring package shows only init.py, completion.py, decision.py, github_pull_request.py, models.py, shadow.py) -- #5184 creates it, #5186 adds 9 lines to it. mcp_tools/control.py: #5184 introduces the monitor_watch schema and #5186 edits the schema #5184 authored. That is a build-on-top dependency, the opposite of two implementations of one behaviour.

Deciding question: if #5184 merged, would #5186 still carry real, wanted work? Yes -- the whole babysit-skill migration, the shipped monitoring doc, the unbounded-loop ban, the pre-probe budget stop, evidence_scope, and the Slack terminal notifications. The reverse direction is not a redundancy signal: #5186 "contains" #5184 only because it is branched off it.

• feat: expose session monitors to agents #5184 → feat: monitor pull requests across source providers #5305 — pr/5184 (fc82d95) is a STRICT GIT ANCESTOR of pr/5305 (9a6f511): git merge-base --is-ancestor refs/remotes/pr/5184 refs/remotes/pr/5305 succeeds, and git log pr/5305..pr/5184 is EMPTY. 5305's branch was cut downstream of 5184's, so 5184's two commits (550ea7b "feat: expose session monitors to agents", fc82d95 "fix: preserve bounded monitor recovery") are physically inside 5305's history. That, and nothing else, is what the first-pass "FULL, complete overlap" claim measured. It is the textbook artifact the brief names in item 4.

The task premise that both are "open against main" is FACTUALLY WRONG for 5305. Its GitHub base ref is token-monitors-babysit (= 7ce67a7, #5186's branch head), not main. 5305 therefore cannot merge before its ancestors; there is no scenario in which it lands and strands 5184. Both bodies declare the same chain in prose: "Stack: #5180 -> #5181 -> #5182 -> #5183 -> #5184 -> #5185 -> #5186 -> #5305", with 5184 self-labelled "PR 5 of 8" / "Base: #5183" and 5305 "PR 8 of 8" / "Base: #5186". The remote branch chain matches exactly: token-monitors-github-probe (ca6fc25) -> token-monitors-tools (fc82d95 = pr/5184) -> token-monitors-dashboard (331c07d) -> token-monitors-babysit (7ce67a7) -> token-monitors-source-providers (9a6f511 = pr/5305).

On its OWN range (7ce67a7..9a6f511) 5305 is 77 files / +5,642 / -432 — matching GitHub's own 77 / +5605 / -432 for the PR. That range EXTENDS the four files 5184 created (src/kiro_crew/monitoring/controller.py, test/test_monitor_controller.py, test/test_monitor_directive_apply.py, test/test_monitor_mcp.py); it does not reimplement any of them. It also ADDS six modules 5184 has no counterpart for at all: monitoring/pull_request.py (the provider-neutral review-readiness contract), monitoring/targets.py (strict target parsers), monitoring/gitlab_merge_request.py, monitoring/azure_devops_pull_request.py, monitoring/bitbucket_pull_request.py, monitoring/provider_cli.py — plus dashboard/handlers/source_providers.py and a whole frontend surface (SessionAutomationPopover.tsx +626, monitoring/automation.ts +455, MonitorRadar.tsx) across 12 locale catalogs.

That is a capability 5184 STRUCTURALLY CANNOT DELIVER: 5184's MonitorController drives only the single GitHub probe already on main (monitoring/github_pull_request.py, landed by the stack's merged #5183 = dc35c19, which is 5184's merge base). GitLab / Azure DevOps / Bitbucket are different addressing schemes and different providers. Conversely 5184 is the substrate 5305 consumes — MonitorController, the monitor_watch/inspect/update/stop MCP tools, the owner-gated REST routes, format_monitor_wake, MonitorDispatchResult. Neither can be closed without destroying real, wanted work: closing 5184 deletes the controller 5305 imports; closing 5305 deletes four provider probes, the neutral readiness contract, and the entire multi-provider dashboard surface.

The maintainer agrees in practice. bolichen97 reviewed each slice on its own per-slice range — 5184 as origin/token-monitors-github-probe...origin/token-monitors-tools (59 files, +8285/-613) and 5305 as origin/token-monitors-babysit...origin/token-monitors-source-providers (73 files, +4763/-384). Nobody has ever suggested either is redundant; both carry live execution-verified review findings to fix, which is a request to iterate, not to close.

Consequence for review

Land the stack in order. Reviewing a later slice before its base lands means reviewing the base's code again, and the FULL-overlap signal a keyword or file-overlap sweep produces here means nothing.

Separately, #5184 vs #7634 (the zero-token-probe monitor gate) was adjudicated as independent — same monitor area, different behaviour — so neither blocks the other.

---

_{From a repository-wide duplicate/overlap audit of every pull request open against main, re-run against the current tree (origin/main 680baf9448dc). This PR is not being closed — the note is informational. If the reading is wrong, please correct the reasoning rather than just the conclusion.}

[bolichen97]

Review — head fc82d950076d276600d991bcaa66f88ba67b8385 (PR 5 of 8)

Re-reviewed the current head against the 2026-09-01 execution-verified review. Verdict: mergeable as the first slice of the stack — every blocking finding from that review is fixed on this head, CI is 62/62 green, and the only remaining gate is a human approval (the earlier approval was auto-dismissed by later pushes). Focused suites pass locally (1011 passed in the monitor/autonudge/directive set).

Prior blocking findings — verified fixed

• Controller wedge (wake_in_flight=True, wake_delivery=None) closed both ways: restore_monitor_after_failed_session_close now stamps MonitorDispatchResult.BUSY (src/kiro_crew/autonudge.py:1821), and _dispatch_claimed maps CancelledError → BUSY, unexpected exceptions → BUSY, untyped results → UNAVAILABLE (src/kiro_crew/monitoring/controller.py:210-222).
• Both agent directives pass replace_existing=False (src/kiro_crew/dashboard/session_directive_apply.py:238,292).
• Cumulative provider_error_count is enforced in monitor_budget_reason (src/kiro_crew/monitoring/decision.py:85).
• Slack/Discord DISPATCHED vs BUSY parity; isinstance guards replace the asserts (src/kiro_crew/slack/gateway.py:6197-6210).
• record_monitor_turn_completion resets wake_delivery to None; MONITOR_PUBLIC_FIELDS carries created_ts / last_wake_reason_code; monitor_stop's reason is persisted as user_stop_reason.

Leftovers (should-fix, not blocking)

1. src/kiro_crew/mcp_tools/control.py:1045 / :1171 — monitor_watch and monitor_stop still skip both the refusal and the denied SEL record when the strict key is empty (... is None and sk), and monitor_update (:1194) still resolves with the wider _autonudge_binding_key. monitor_inspect is the only tool that behaves as docs/architecture/mcp.md documents.
2. src/kiro_crew/mcp_tools/control.py:368 — monitor_start's description still says "One loop per session; starting a new one replaces the old", but the applier is now create-only. A model re-arming after compaction believes it replaced the loop; the refusal is visible only if it re-reads its own prior result.
3. src/kiro_crew/session_summary.py:40 — _INJECTED_PREFIXES lacks [monitor wake]. Dashboard wakes are role nudge (safe), but Slack/Discord wakes land as role user, so the summary/intent path counts an unattended wake as a user goal.

Integration with sessions that are not plain chats

• Crew-mode sessions. src/kiro_crew/dashboard/session_control.py:1110 refuses to deliver a turn into slot.mode == "crew" because a crew session's ingress is a durable queue entry, not a turn. The monitor admission gate (src/kiro_crew/autonudge_authz.py:503) and _fire_dashboard_nudge check only slot identity + _closing, never mode, so a monitor_watch armed from an orchestrator slot fires [Monitor wake] as a raw turn, bypassing state.crew.ingest. Same for member slots. Legacy nudges had this gap already; this PR widens the surface without closing it. Suggest refusing at admission and retiring on mode mismatch the way retire_monitor_for_session_close does.
• Conductor-style agents. The bundled goal-conductor / pipeline-conductor skills patrol their own session with monitor_start — that is the primary, correct use of the legacy loop, not a fallback. Two consequences of the create-only flip: re-arming now refuses instead of replacing (the skills should say "use monitor_update"), and the default caps (_MONITOR_DEFAULT_MAX_CYCLES=24, 4h runtime) end a long-horizon patrol silently — pre-existing, but this stack is where the skills got rewritten.
• Memory. src/kiro_crew/dashboard/chat_runner.py:9377 runs _maybe_consolidate after every completed turn with no automation exclusion, so each wake turn writes facts/episodes indistinguishable from user work. Any workspace/persona-scoped memory layer will need an origin facet for these turns (e.g. job_type=monitor, source=monitor_wake + monitor id), the [Monitor wake] prefix above, and a decision about wake_instructions — an agent-authored standing instruction re-injected every wake, i.e. a de-facto directive that lives in the monitor record rather than the memory store. The wake path also never consults slot.memory_mode (persistent|incognito|temporary), and the monitor record (target + instructions) is fsynced to disk regardless.

_{Execution-verified AI-assisted review against a local checkout at fc82d950; nothing modified. Findings name file:line so they can be checked directly — please push back where one misreads intent.}

[bolichen97]

Approving the current head after the re-review above: all prior blocking findings verified fixed, CI green; leftovers are tracked as follow-ups.

[bolichen97]

@kyleseaman Ready to land #5184 — all prior blocking findings are fixed on fc82d950 and CI is green — but the merge is blocked by branch protection: "New changes require approval from someone other than bolichen97 because they were the last pusher." I force-pushed the 19:40 restack of token-monitors-tools, so my approval on this head does not count.

Could you re-push the branch so you are the last pusher? Same content is fine, e.g.

git checkout token-monitors-tools
git commit --amend --no-edit
git push --force-with-lease origin token-monitors-tools

CI reruns (~30–60 min); I will re-approve the new head and merge, then continue with #5185 → #5186 → #5305 (those three you pushed last, so my approvals there are valid). No need to rebase onto main in the same push unless you want to — any rebase re-runs the readiness lane the same way.

[kyleseaman]

@bolichen97 Re-pushed the content-identical #5184 head as kyleseaman at 31f8cbf. Restacked and pushed #5185, #5186, and #5305 bottom-up as well so the parent chain remains linear. Fresh CI is running.

[kyleseaman]

@bolichen97 Re-pushed by kyleseaman at d5686eb1f, rebased cleanly onto current main (5081f18f1). I also folded the remaining session-integrity follow-ups into this head; the rest of the active stack was restacked and pushed bottom-up. Fresh CI is running, and the current review-thread audit shows no unresolved non-outdated threads.