fix: detect no-mistakes pipeline stalls#893
Open
gizm0duck wants to merge 7 commits into
Open
Conversation
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
fm-crew-state.sh --stall-snapshotparsing and per-task detector state to track attributed run/step status, elapsed time, status-file progress, and alert deduplication.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
checkwakes 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
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 attributedfixingrun without an active step row, the new fallback emits duration0; lines 139–145 then reset its effective start tonowon 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 itsadvanced/stalledsuppression marker beforefm-watch.shdurably 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 valueawaiting_agent: parked 2m10sis parsed as2m10after only stripping the finals, fails the numeric check, and becomes0. 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-levelstatus: running; this fallback emitsrunningas the snapshot status, sonm_stall_check_tasktreats it as a 15-minute active step (and with noawaiting_agentage, 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 invalidFM_NM_STALL_PARKED_SECSfalls 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 onis_gate.🔧 Fix: Restore parked stall default fallback
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-nm-stall.test.shbash tests/fm-crew-state.test.shbash 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.