Skip to content

fix: detect no-mistakes pipeline stalls#893

Open
gizm0duck wants to merge 7 commits into
kunchenguid:mainfrom
gizm0duck:fm/nm-wedge-detector-w4
Open

fix: detect no-mistakes pipeline stalls#893
gizm0duck wants to merge 7 commits into
kunchenguid:mainfrom
gizm0duck:fm/nm-wedge-detector-w4

Conversation

@gizm0duck

Copy link
Copy Markdown

Intent

Build automatic watcher-side no-mistakes stall detection in firstmate so the owning supervisor wakes when an attributed pipeline run is silently wedged. Detect running or fixing steps with no step transition or crew status progress beyond a configurable 15-minute default, awaiting_approval or fix_review gates beyond a configurable 8-minute default, and run step/status advancement with no crew status-file write. Reuse the existing watcher slow-check machinery and shared classifier rather than a daemon or parallel wake path, preserve home-local task ownership, name run id, step, and task in the wake, and add focused synthetic coverage plus concise architecture documentation.

What Changed

  • Add watcher slow-check detection for stalled active no-mistakes steps, overdue approval/review gates, and run advancement without a matching crew status-file update; enqueue durable, ownership-local wakes that name the run, step, and task.
  • Add fm-crew-state.sh --stall-snapshot parsing and per-task detector state to track attributed run/step status, elapsed time, status-file progress, and alert deduplication.
  • Document configurable active and parked thresholds, and add focused crew-state and watcher stall-detection coverage.

Risk Assessment

✅ Low: Captain, the change is bounded to watcher-side stall classification, preserves home-local ownership, and the reviewed paths satisfy the stated detection criteria.

Testing

On the clean target checkout, focused synthetic coverage passed for active, parked, silent-advance, threshold, deduplication, and shared run-step parsing behavior. A live watcher verification then showed the real watcher emitting and durably queuing named check wakes for a 901-second running step, a 481-second fix-review gate, and a run advance without a crew status-file write; the transcript is attached.

Evidence: Live watcher wake and durable-queue transcript

=== active ===
watcher output: check: no-mistakes stall: run run-active step review task task (running unchanged for 901s) 
durable queue: 1784783738	1	check	no-mistakes-stall:task	check: no-mistakes stall: run run-active step review task task (running unchanged for 901s) 

=== parked ===
watcher output: check: no-mistakes stall: run run-parked step fix-review task task (fix_review unchanged for 481s) 
durable queue: 1784783739	1	check	no-mistakes-stall:task	check: no-mistakes stall: run run-parked step fix-review task task (fix_review unchanged for 481s) 

=== silent ===
watcher output: check: no-mistakes silent advance: run run-silent step review task task (run changed without a crew status write) 
durable queue: 1784783740	1	check	no-mistakes-stall:task	check: no-mistakes silent advance: run run-silent step review task task (run changed without a crew status write) 

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 3 issues found → auto-fixed (5) ✅
  • 🚨 bin/fm-classify-lib.sh:129 - Required behavior: “run step/status advancement with no crew status-file write.” The snapshot includes elapsed duration, which changes on every poll; this condition therefore treats an unchanged long-running step or gate as a silent advance, repeatedly wakes it, and bypasses the later stall-age check. Compare only stable run/step/status identity here, keeping duration solely for age calculation.
  • 🚨 bin/fm-crew-state.sh:360 - Required behavior: “Detect running or fixing steps with no step transition or crew status progress beyond a configurable 15-minute default.” For an attributed fixing run without an active step row, the new fallback emits duration 0; lines 139–145 then reset its effective start to now on every check, so it can never exceed the threshold. Preserve a first-seen/start timestamp when the CLI provides no step duration.
  • ⚠️ bin/fm-classify-lib.sh:130 - The detector writes its advanced/stalled suppression marker before fm-watch.sh durably appends the wake. If queue append fails, a restart sees the marker and suppresses the only escalation. Commit detector state only after successful queue append, or retain a pending marker that is retried.

🔧 Fix: Fix no-mistakes stall detection state handling
2 errors still open:

  • 🚨 bin/fm-crew-state.sh:323 - Required criterion: “awaiting_approval or fix_review gates beyond a configurable 8-minute default.” The fixture-format value awaiting_agent: parked 2m10s is parsed as 2m10 after only stripping the final s, fails the numeric check, and becomes 0. A gate already waiting longer than eight minutes is therefore timed from this watcher's first observation instead of its actual gate age, delaying escalation after watcher restart.
  • 🚨 bin/fm-classify-lib.sh:155 - The required 8-minute gate condition is independent of the active-step clause’s “no step transition or crew status progress.” This shared anchor resets a parked gate’s age to the latest crew-status-file write, so recurring status writes can defer an unchanged awaiting_approval/fix_review gate indefinitely rather than surfacing it once the gate itself exceeds the parked threshold.

🔧 Fix: Fix parked gate stall timing
1 error still open:

  • 🚨 bin/fm-classify-lib.sh:148 - Required criterion: “Detect running or fixing steps with no step transition or crew status progress beyond a configurable 15-minute default.” On the first slow check of an already-15+-minute active step, the new marker branch writes state then returns at this hunk ([ "$is_gate" = 1 ] || return 0), deferring the wake until the next slow-check interval. With the default 300-second cadence, a known stalled step can remain silent for roughly 20 minutes; evaluate the current duration/status age on that first observation instead.

🔧 Fix: Fix immediate active-step stall detection
1 error still open:

  • 🚨 bin/fm-crew-state.sh:355 - Required criterion: “awaiting_approval or fix_review gates beyond a configurable 8-minute default.” A supported parked scalar-gate run can retain top-level status: running; this fallback emits running as the snapshot status, so nm_stall_check_task treats it as a 15-minute active step (and with no awaiting_agent age, first-seen timing) instead of the parked-gate path. Emit a gate status for this format or otherwise classify it with the 8-minute threshold.

🔧 Fix: Classify scalar parked gates as awaiting approval
1 warning still open:

  • ⚠️ bin/fm-classify-lib.sh:129 - An invalid FM_NM_STALL_PARKED_SECS falls back to the active-step default (900s), delaying parked-gate detection by seven minutes instead of restoring its documented 480s default. Select the fallback based on is_gate.

🔧 Fix: Restore parked stall default fallback
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-nm-stall.test.sh
  • bash tests/fm-crew-state.test.sh
  • bash tests/fm-watch-triage.test.sh (relevant watcher regression paths exercised)
  • bash tests/fm-wake-queue.test.sh (relevant queue regression paths exercised)
  • bash /var/folders/yh/09t9xt194zzg62t_x4f7llrr0000gn/T/no-mistakes-evidence/01KY6MPV7FETA6F8JJZFD0VM1X/run-watcher-stall-e2e.sh
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant