fix(bin): defer contribution unavailable wakes until a failure repeats - #6017
Open
dhanu-covlant wants to merge 3 commits into
Open
dhanu-covlant wants to merge 3 commits into
dhanu-covlant wants to merge 3 commits into
Conversation
|
…, and the full contribution test suite passes. I left ci-1 and ci-2 alone, as instructed. Those two checks report `action_required`, which needs someone to approve the workflow runs; no code change can clear them. - **ci-3, `bin/fm-contributions.jq:15`:** the old check turned a stored `false` into 0, so it counted as valid. The check now allows a missing or null `failure_streak`; any value that is present must be a whole number from 0 to 2. I ran the validator on sample records: `null`, `1` and `2` pass, while `false`, `1.5` and `3` are now rejected. - **ci-4, `tests/fm-contributions.test.sh`:** I added `test_legacy_stored_error_counts_as_reported` and put it in the test runner list. It starts from an older record that has an error but no `failure_streak`, runs a poll against a failing forge, and checks two things: nothing is printed, and the record is saved with the same error, the new `checked_at` and `failure_streak == 2`. **Checks run:** - `bash tests/fm-contributions.test.sh` passes every test, including the new one. - To confirm the new test catches a real regression, I temporarily changed the poll so a missing streak counts as 0 instead of 2. The new test failed ("a legacy stored error did not migrate to streak 2") and all other tests still passed. I then restored the file; only the two intended files differ from the original. The changes are not committed. Pushing and rechecking CI are left to the outer executor
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
Fix the repeated contribution-observer noise: the contribution poll keeps waking with
observation unavailablefor older Codeparser and Sentinel PRs even when there is no pending action. The recurring failure is transient and self-clearing, so the goal is to stop momentary read inconsistencies from interrupting supervision while preserving escalation for persistent observation loss.What Changed
bin/fm-contributions.shpoll now saves afailure_streakon each failed record. The first failed forge observation is saved quietly withfailure_streak=1. Thecontributions: observation unavailableline prints only when the next consecutive failure moves the streak to 2. Later failures stay at 2 and print nothing. A successful read, or settling a merged or closed contribution, clears the streak and ends the episode.failure_streak(an older record) counts as already reported, so it does not wake supervision again.bin/fm-contributions.jqacceptsfailure_streakonly as an integer from 0 to 2, and the script's header comment now describes the new rules.tests/fm-contributions.test.shhas new tests: one failure that clears on the next read without printing anything, a persistent failure that prints once after two failures and again only after a recovery, and a late-joining owner that shares the episode. Existing tests now expect a first failure to be saved quietly withfailure_streak == 1.🤖 Generated with Claude Code
Risk Assessment
✅ Low: The change is a small, clearly bounded debounce and the intent explicitly allows it: a first failure is now recorded silently, and the unavailable line prints once on the second consecutive failure. Success, final settlement, shared owners, late owners and legacy error records each reset or carry the streak correctly. The watcher runs checks every 300s, so a single momentary failure is absorbed while a persistent outage still wakes supervision within one extra check interval.
Testing
I ran the focused contribution test file, which passed. I then ran seven scenarios live against real GitHub through
fm-contributions.sh pollin a disposable lab home: a healthy read, a failure that clears on the next poll, a failure that persists, recovery followed by a new episode, a stored record from beforefailure_streakexisted, a URL shared with a task added mid-episode, and a tampered streak value. All seven matched the intended rules: the first failure is recorded without a wake, the second failure in a row wakes exactly once, and a good read clears the episode. The same drive on the base commit reproduced the reported noise (it woke on the first failure, even when the next read succeeded). This is a CLI change with no UI, so the evidence is poll output transcripts rather than screenshots. Lab homes and base-commit temp copies were removed, and the worktree is clean.Evidence: Live poll transcript (fix) — real gh against #6016, scenarios S1–S7
Source: Live poll transcript (fix) — real gh against kunchenguid/firstmate#6016, scenarios S1–S7
Evidence: Live poll transcript (base 29213a0) — reproduces a wake on the first failure that then clears
Source: Live poll transcript (base 29213a09) — reproduces a wake on the first failure that then clears
Evidence: Live drive script used for both transcripts
Source: Live drive script used for both transcripts
Evidence: Output of the contribution test file
Source: Output of the contribution test file
Evidence: Key before/after lines
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-contributions.sh:394- The legacy-migration branch has no test. In the prior-streak query,(.failure_streak // 2)treats a saved record that has an error but nofailure_streakas already reported. That is the case for every error record written before this change. Without that default, the first poll after deployment would re-wake each existing outage once. No test seeds such a record and checks that the next failure stays silent and writesfailure_streak == 2. Add one behavioural poll test that starts from a pre-change error record.✅ **Test** - passed
✅ No issues found.
bash tests/fm-contributions.test.sh(only the contribution test file; every case passed, including the newtest_transient_forge_failure_recovers_without_wake,test_unavailable_forge_wakes_once_after_two_consecutive_failuresandtest_late_owner_keeps_failure_episode_suppressed)Live drivelive-drive.sh <worktree>: minted a lab home withbin/fm-lab-home.sh create, added a backlog row linking feat(spawn): add opt-in worker launch command that leads the launch prompt #6016, and ran the realbin/fm-contributions.sh pollwith the real authenticatedgh. Genuine read failures came from a process-localGH_TOKEN=invalid...env var; the credential store was not touched.Live S1: healthy real read records the observation, no outputLive S2: one failure, then recovery → no wake, streak 1, then clearedLive S3: four failures in a row → one wake on the 2nd, silent on the 3rd and 4th, streak held at 2Live S4: recovery ends the episode; a new episode again waits for its 2nd failureLive S5: a stored error with no failure_streak (older record format) → the next failure stays quiet (streak 2)Live S6: a second task sharing the URL, added mid-episode → one shared wake, both records at streak 2, then quietLive S7 (adversarial): stored failure_streak=7 → the record is reported unreadable and no write happensBaseline: the same live drive againstgit archive 29213a09(the pre-fix code) to reproduce the reported noise✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.