feat: add durable monitor decisions

[kyleseaman]

Stack: #5171 → #5172 → #5173 → #5174 → #5175 → #5176 → #5177

Position: 2 of 7. Base: #5171. Next: #5173.

Adds typed durable monitor state, bounded decisions, outcomes, and fail-closed persistence while preserving legacy AutoNudge behavior.

Verification: monitor decision, persistence, deadline, and AutoNudge suites.

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 1f1f2c0

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

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Sound fail-closed substrate, but decide_monitor's fingerprint dedup shadows terminal statuses — it silently assumes fingerprints encode status, a contract nowhere stated.

Watch

• In decide_monitor, the generic dedup (observation.fingerprint == state.last_fingerprint → NO_CHANGE) runs before the SUCCESS/BLOCKED branches. If a provider adapter's fingerprint doesn't change when status does (e.g. same head SHA, checks flip pending→passing), a met objective returns NO_CHANGE forever and the monitor probes until STOP_BUDGET — wrong terminal outcome, wasted probes. Either classify terminal statuses before the dedup, or document "fingerprint must incorporate status" as a hard contract on MonitorObservation so the later adapter slices can't violate it invisibly.
• Retention is asymmetric: a future-version monitor is preserved byte-exact (_raw_payload survives rewrite), but a malformed current-version monitor drops its whole loop from _loops, so the next unrelated save deletes the record from disk permanently. The doc frames this as "skipped", but skipped-in-memory becomes destroyed-on-disk; if inspection matters for version 99, it plausibly matters for a corrupt version 1 too.

Suggestions

• wake_in_flight is persisted, validated, and never consulted by the policy this PR ships; if Task4 owns it, say so in the spec, otherwise it's schema surface with no reader.

[DESIGN-REVIEWED] 1f1f2c0

[kyleseaman]

Replaced by #5181 after renaming the branch to remove the prefix.

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 1f1f2c017c5b58e2e03175988aa7feb3b8bd5689 — 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 read the contract, the intent file, and the patch, and grepped the repo to count consumers of each new surface. Here is the review.

First-Principles-Verdict: CONCERNS

A declared substrate PR whose centerpiece, decide_monitor, plus several state fields and enum variants, has zero non-test consumers anywhere in src/.

What this change ships

Intent: let a session monitor survive restarts with hard budgets, without letting it fire until its controller exists — an ADDITION (substrate, PR 2 of a 7-stack).

1. A loop can persist a typed monitor record across restarts — justified (declared)
2. Any monitor-carrying loop is deactivated at load/update/arm/fire, never nudges — justified fail-closed (unattended-turn boundary)
3. A future-version monitor is kept inert, blocked, byte-preserved on rewrite — justified, though it defends records nothing can yet produce
4. A malformed or null monitor drops its whole loop at load — justified fail-closed
5. Pure decide_monitor policy (budgets, error classes, fingerprint dedup) — zero consumers
6. New defaults: 14,400s/8 turns/250k tokens/3 errors, 300s cadence — declared, but no producer or scheduler reads them
7. Decision/outcome/error vocabulary (7+6+5 variants) — several variants zero consumers
8. Cosmetic reflow of unrelated autonudge.py lines — rides along
9. Spec section updated in same commit — justified (AGENTS.md mandate)

Watch

• decide_monitor has 0 src/ consumers (grepped decide_monitor: definition + tests only). The description declares later slices will wire it, so this is accepted-and-deferred, not blocked — but until feat: add github pull request monitor probe #5174 lands, 84 lines of policy plus 229 test lines are unfalsifiable against any real caller.
• Nothing in this PR (or the repo) constructs a MonitorState except the loader reading hand-authored JSON (grepped MonitorState(: models.py only). The five fail-closed guard sites in autonudge.py (load, update, _arm_from_deadline, _timer, _run_fire_cycle) all exist because the field is attached one slice before its controller; sequencing the attachment with the controller PR would delete the need for most of them.

Subtractions

• Defer src/kiro_crew/monitoring/decision.py + test/test_monitor_decision.py to the slice that calls it — 0 consumers (grep decide_monitor in src/: 1 definition, 0 callers).
• Drop MonitorState.wake_in_flight and next_probe_at — 0 reads outside their own validation (grep in src/: models.py definition/validation only); add each with its reader.
• Drop MonitorOutcome.USER_STOP / SESSION_CLOSE / TARGET_UNAVAILABLE and MonitorObservation.reason_code / summary — 0 constructors/readers in src/ (grep: models.py only); reintroduce with the code that sets them.

[FIRST-PRINCIPLES-REVIEWED] 1f1f2c0