test: tolerate absent triage wake queues - #2845
Conversation
…queue The exited-declared-pause case counts queued stale wakes by handing state/.wake-queue straight to awk. A watcher that queues nothing never creates that file, and awk aborts on a missing path before its END rule runs, so the count collapses to the empty string. The next comparison then fails as an integer-expression error and surfaces as a wake flood with no number, hiding the real contract breach the following grep names. Read the queue the way the drain-count assertion at the end of this file already does: silence awk's open error and default an absent queue to zero. Applied to all four counts in this case, including the live external-decision gate pair whose queue an acknowledged drain can also leave behind. An absent queue now reports "did not use the bounded paused recheck", while a genuine flood still fails with its real count. Fixes kunchenguid#2628
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Reviews (3): Last reviewed commit: "no-mistakes: apply CI fixes" | Re-trigger Greptile |
|
Speaking as Kun's firstmate: Scheduled 3:10am PT 8/23 pass. Main reconfirmed VISION (inspected Class: corrective (test-only). The named fix treats a missing optional queue as zero wakes and keeps genuine-flood counts discriminating. Extra CI-fix commits are also test-only. Security: none. Test fixtures only. No Overlap / HOLD: not spawn/teardown, no CI / NM: HEAD Workflows: already approved (CI completed SUCCESS on this HEAD). Run IDs: 32628496185 (CI), 32628496212 (Require no-mistakes), 32629332941 (Require no-mistakes). No pending first-time-fork approval. Land-eligible rec: NO (NM attestation does not match THIS HEAD; extra out-of-issue test file). Captain-flag NOW: no. Author can re-run no-mistakes so the body attestation matches |
2 participants
Fixes #2628
Additional commits from the validation pipeline
CI on this branch surfaced two failures unrelated to the queue-read fix described below. The validation pipeline diagnosed and fixed both, so this branch also carries:
tests/fm-pi-watch-extension.test.sh: the session-transition case identified its arm child by PID alone, so under CI load a recycled PID could make a dead generation look alive, or make one live process count twice. Replaced with a unique per-arm lifetime marker that the child creates and removes on exit, preserving the overlapping-generation detection the case exists to prove. This is a pre-existing, CI-only, load-sensitive flake, not a consequence of the change below.tests/fm-watch-triage.test.shis untouched by both.Intent
Fix GitHub issue #2628: the fm-watch-triage test suite's exited-declared-pause case fails when state/.wake-queue is absent.
Reported defect: running tests/fm-watch-triage.test.sh reaches the exited-declared-pause case without state/.wake-queue existing. Two awk reads then fail with "can't open file" for exited-declared-pause/state/.wake-queue, and the suite reports 'not ok - dead-agent declared pause flooded stale wakes across six unchanged polls' with no number in the message. The reporter's stated expectation: the test should either create the bounded paused-recheck wake queue it asserts against, or handle an absent queue without producing an empty integer comparison. The reporter also scoped this as reproducing independently of the fm-batch-tooling changes, to be fixed on its own.
Root cause established by direct reproduction, not inference: awk aborts on a missing input path before its END rule runs, so 'awk ... END { print n + 0 } $state/.wake-queue' prints nothing and exits 2 instead of printing 0. The count variable collapses to the empty string, and the following '[ "$wakes" -le 1 ]' fails as a bash 'integer expression expected' error rather than a comparison. That error takes the '|| fail' branch, so an absent queue is misreported as a wake flood of an unprintable count - masking the real contract breach that the very next grep names correctly ('did not use the bounded paused recheck'). A watcher that correctly absorbs every poll and queues nothing never creates .wake-queue at all, so the file is legitimately optional here.
Chosen resolution and why: of the reporter's two options, take the second (tolerate an absent queue) rather than pre-seeding a fixture queue. Seeding a queue file would pre-empt the very thing the case measures - how many stale wakes the watcher itself enqueued - whereas absent-tolerance is already this suite's own established convention for reading a possibly-absent queue: the drain-count assertions at the end of the same file use 'awk ... "$state/.wake-queue" 2>/dev/null || echo 0', and the sibling fm-procevent suites read the queue with '2>/dev/null' for the same reason. The fix therefore reuses the existing in-repo idiom instead of introducing a new helper or abstraction.
Scope decision: the ticket names the two reads in the exited-declared-pause fixture, but the identical unguarded pair appears again later in the same test function for the live external-decision-gate fixture, whose queue an acknowledged drain can also leave behind. All four reads were fixed together, since patching only the two the ticket names would leave the same latent crash in a sibling read one screen down. Deliberately NOT changed: the awk read of state/.watch-triage.log later in the file, which looks similar but reads a file that same test creates itself with 3001 lines and so can never be absent - guarding it would be unrelated scope creep.
Behavior preserved, verified by a focused before/after harness: with the queue absent the case now reports the accurate 'did not use the bounded paused recheck' diagnostic instead of two bogus integer-comparison failures; with a genuine two-wake flood present the guard still goes RED with its real counts (2), so the assertion's discriminating power is intact and this is not a weakened test. Also confirmed the exact reported symptom reproduces before the change: the failure line prints with the count missing, verbatim as filed.
Constraints honored: test-file-only change, no production watcher or classifier behavior touched, no new dependency or abstraction added, and an explanatory comment left at the first site so a future reader knows why the queue is optional and what the unguarded form used to misreport. Full suite verified green locally (60 ok, 0 not ok) and shellcheck -x clean on the changed file.
Deliver as a pull request that closes issue #2628 and nothing else.
What Changed
.wake-queueas zero wakes in declared-pause and live decision-gate assertions.Risk Assessment
✅ Low: Captain, this well-bounded test-only change correctly treats an absent optional wake queue as zero while preserving detection of genuine wake floods and required queue contents.
Testing
After reviewing the base-to-target test-only diff, I ran the canonical affected watcher suite and a focused before/after CLI harness. The suite passed, the reported empty-integer failure reproduced with the base expression, the fixed expression returned zero for an absent queue and preserved the accurate contract diagnostic, and a real two-wake flood remained detectable with its correct count.
Evidence: Targeted fm-watch-triage test transcript
Source: Targeted fm-watch-triage test transcript
Evidence: Missing-queue before/after and genuine-flood evidence
Source: Missing-queue before/after and genuine-flood evidence
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
Inspectedgit diff 8714c9a78c1b4355782fcb9ce1ccf14337478268..e328ebb9a0b8f31d2d722463bc241eea70c47a8e -- tests/fm-watch-triage.test.shRanbin/fm-test-run.sh tests/fm-watch-triage.test.shExecuted a focused shell harness reproducing the base missing-file failure, the fixed absent-queue behavior, and a genuine two-entry floodRangit status --shortto confirm testing left no transient worktree changes✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.