fix(watchdog): stop one bad dispatch from killing the whole scan

[MarkusNeusinger]

Follow-up to #10179. That PR stopped implementation PRs from entering the review dead-end; this one stops them from staying there. Three verified holes in the pipeline's only safety net.

1. A failed dispatch aborted the entire scan — and this is armed right now

dispatch() called gh workflow run bare under set -euo pipefail. One unroutable dispatch therefore killed the whole run, and every PR after it went unscanned — the safety net failing silently, in the one place where nothing else is watching.

Five of the last fourteen scheduled scans died exactly this way (30749881831, 30781315973, 30803034385, 30824022333, 30872621465):

could not create workflow dispatch event: HTTP 422: Cannot trigger a
'workflow_dispatch' on a disabled workflow (.../workflows/265430494/dispatches)
##[error]Process completed with exit code 1.

Workflow 265430494 is .github/workflows/daily-regen.yml, state disabled_manually. The watchdog's Section C cron-liveness rescue keeps trying to revive it and cannot. Worse, the → dispatching line was printed before the attempt, so the log claimed rescues that never happened — which is why Section C reported "dispatching" on 08-03 and 08-04 while daily-regen stayed down.

This is not historical. daily-regen last ran 2026-08-05T16:51Z and LIVENESS_HOURS=10, so from ~02:51Z the gap trips again and the next scheduled scan (0 */6 * * *) dies with it.

Dispatch failures are now counted and surfaced, never fatal; a disabled target is named as the non-transient condition it is; and the log line follows the attempt instead of preceding it.

2. ai-rejected + ai-attempt-N matched no case at all

Case 2 excluded any verdict label, Case 4 excluded any attempt label — so that pair fell through everything. It is exactly the state impl-review.yml leaves behind when its impl-repair dispatch fails; the file's own comment documents the escape hatch (drop ai-rejected so Case 2 picks it up) which only works if that API call also succeeds. PR #9949 sat in this state for ten days.

Case 2 now excludes only ai-approved (Case 3's business). The existing age > STALE_SEC guard keeps it from racing an in-flight repair, and the watchdog:repair-rescued-N marker keeps it one-shot.

3. ai-review-rescued was never cleared

Case 1 writes it; nothing removes it. So it meant "this PR was ever rescued" instead of "this failure streak was already rescued once". Any PR that failed review twice in its lifetime was permanently outside automation — 9 of the 11 PRs stuck on 2026-08-05 carried this pair, and both impl-review-retry.yml and watchdog Case 1 refused them.

A successful review now clears it together with ai-review-failed, restoring the intended per-streak semantics. Done via the REST API: gh pr edit --remove-label fails on this repo with GraphQL: Projects (classic) is being deprecated ... (repository.pullRequest.projectCards) — hit while cleaning up these very PRs.

Verification

Workflow changes have no verification loop in this repo, so dispatch() is extracted verbatim from the YAML and exercised against a stubbed gh, and the Case 2 guard is replicated exactly as written:

--- (c) a failed dispatch must not abort the scan ---
PASS  disabled workflow: scan survives               rc=0
PASS  disabled workflow: reported as such            rc=0
PASS  transient error: scan survives                 rc=0
PASS  success: no failures counted                   rc=0
PASS  success: claims dispatched only after the fact rc=0
--- (b) Case 2 must now cover ai-rejected + ai-attempt-N ---
PASS  the #9949 dead-end is now caught               match
PASS  multi-attempt dead-end caught                  match
PASS  original Case 2 shape still caught             match
PASS  ai-approved still left to Case 3               skip
PASS  no attempt label -> left to Case 4             skip
PASS  no score label -> not Case 2                   skip

ALL CASES PASS

yaml.safe_load clean on both changed workflows.

Hole 3's fix rides in impl-review.yml rather than the watchdog, because a successful review is the only event that can truthfully say the failure streak ended. Its effect was observed live: after #10179 merged, #10130 and #10003 were resumed and both reviews now score 0 → ai-rejected → impl-repair dispatched (runs at 21:38), the hand-off that had not fired in ten days.

Open question for the maintainer, deliberately not actioned here: daily-regen.yml was disabled manually a few hours ago — plausibly to stop the flood of stranded PRs that #10179 has now fixed at the source. Re-enabling it is your call; this PR only makes the watchdog survive the disabled state instead of dying on it.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR hardens the GitHub Actions “watchdog” safety net so a single broken dispatch or a previously-rescued review state can’t strand implementation PRs in a dead-end, continuing the recovery work from #10179.

Changes:

• Make watchdog dispatch() non-fatal under set -euo pipefail, and improve logging for disabled-workflow vs transient dispatch errors.
• Fix watchdog Case 2 matching so ai-rejected + ai-attempt-N no longer falls through all cases.
• Clear ai-review-failed and ai-review-rescued after a successful review in impl-review.yml via REST label deletion (avoids gh pr edit --remove-label GraphQL failure).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
CHANGELOG.md	Adds an [Unreleased] Fixed entry documenting the three pipeline/watchdog fixes.
.github/workflows/watchdog-stuck-jobs.yml	Makes dispatch failures non-fatal and closes the “ai-rejected + ai-attempt-N” case gap.
.github/workflows/impl-review.yml	Clears “rescued/failed” labels on successful review using gh api label deletion.

[MarkusNeusinger]

Correct, and the gap was real: the counter was incremented and never read, so "counted and reported" only held for the inline per-failure warnings. Fixed in 5a8f0e4 — the scan's closing line now states the tally.

This matters beyond tidiness. Without it, a scan that dispatched nothing successfully reads exactly like a quiet, healthy one — and an always-green watchdog that silently rescues nothing is the precise failure mode this PR exists to fix.

if (( DISPATCH_FAILURES > 0 )); then
  echo "::warning::Watchdog scan complete — ${DISPATCH_FAILURES} dispatch(es) failed (see warnings above)"
else
  echo "::notice::Watchdog scan complete — all dispatches succeeded"
fi

Deliberately not exit 1 on a non-zero tally: aborting the scan is what took it down in the first place, and a red run would bury the summary behind a failed step.

Your comment also surfaced an adjacent risk worth pinning down rather than reasoning about. The counter only works if the increments happen in the current shell. The real scans are while read ... done < <(jq -c '.[]') — process substitution, so the body does run in the current shell — but had they been jq | while read pipelines, every increment would have landed in a subshell and the tally would have printed 0 forever, which is worse than no tally at all. Two cases now assert it against the real loop form:

--- the counter must survive the scan loop and be reported ---
PASS  counter survives while/process-substitution    3 failures reported
PASS  healthy scan reports success, not silence      ok

Suite is 25 cases, all passing.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[MarkusNeusinger]

Scope extended after maintainer input

The maintainer confirmed that daily-regen is switched off and on deliberately, to manage the monthly token budget. That makes the original framing of this PR insufficient.

Making the failed dispatch non-fatal (commit 1) stops the watchdog from dying, but Section C would still, on every single scan for as long as the workflow stays off:

• log ::warning::daily-regen: no new run on main for ...h — schedule starved, re-dispatching — a false report about an intentional state
• attempt a dispatch that cannot succeed (a disabled workflow rejects workflow_dispatch with HTTP 422)
• burn a counted dispatch failure

That is exactly the recurring noise that hid the real breakage in the first place.

Section C now reads the workflow state before deciding:

State	Behaviour
disabled_manually	skip quietly — intentional operator decision, not starvation
disabled_inactivity	skip with a warning — GitHub's 60-day auto-disable is worth flagging, but is equally undispatchable
active	unchanged: quiet-window check, then the 10 h gap check
unreadable	falls through to the existing logic, so a transient API blip cannot silently disable the rescue

Test suites extended from 11 to 23 cases. The 12 new ones cover the full workflow-state × quiet-window × gap matrix and pin that active behaviour — including the 10 h boundary — is untouched:

--- the maintainer's budget toggle must be respected ---
PASS  disabled_manually, long gap -> no false rescue       skip-intentional
PASS  disabled_manually, short gap                         skip-intentional
PASS  disabled_manually during quiet window                skip-intentional
--- GitHub's own inactivity disable is flagged, not dispatched ---
PASS  disabled_inactivity -> warn, no dispatch             skip-undispatchable
--- active behaviour must be unchanged ---
PASS  active, gap 200h -> rescue                           rescue
PASS  active, gap 11h -> rescue                            rescue
PASS  active, gap 10h -> healthy (boundary)                healthy
PASS  active, gap 4h -> healthy                            healthy
PASS  active, quiet window 17 -> skip                      skip-quiet
PASS  active, quiet window 21 -> skip                      skip-quiet
PASS  active, hour 22 -> rescue again                      rescue
PASS  unknown state falls through to normal logic          rescue

ALL CASES PASS

The open question in the PR description is resolved: daily-regen stays disabled, and that is now a state the watchdog understands rather than one it fights.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

.github/workflows/watchdog-stuck-jobs.yml:97

• DISPATCH_FAILURES is incremented but never surfaced in logs, so it currently adds state without changing behavior. If the intent is to make failures visible, include the running count in the warning output (or otherwise report it) so operators can see how many dispatches failed in a run.

            err=$(printf '%s' "$err" | head -c 300 | tr '\n' ' ')
            if [[ "$err" == *"disabled workflow"* ]]; then
              # Not transient and not something a retry fixes — a human turned
              # the workflow off. Report it plainly and keep scanning.
              echo "::warning::${label} → SKIPPED, target workflow is disabled: ${err}"

CHANGELOG.md:141

• The changelog entry says the safety net had "three" holes, but the bullet enumerates (1)–(4). This reads like an internal inconsistency in release notes; either adjust the count in the headline or drop the fourth numbered item from this bullet.

- **The pipeline's safety net had three holes, and one of them was armed** — follow-up to #10179,

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

CHANGELOG.md:143

• This changelog entry says the safety net had “three holes” but then enumerates (1)–(4). That internal inconsistency makes the release note hard to trust. Either adjust the headline count or merge/remove an item so the count matches the list.

- **The pipeline's safety net had three holes, and one of them was armed** — follow-up to #10179,
  which stopped PRs *entering* the dead-end; this stops them *staying* there.
  (1) `watchdog-stuck-jobs.yml` called `gh workflow run` bare under `set -euo pipefail`, so a

[MarkusNeusinger]

Confirmed, and this is the best catch of the review — the PR was reintroducing the exact failure mode it exists to fix, in the error path where it would be hardest to notice. Fixed in the latest commit.

One correction to the mechanism, because it changes how the regression test has to be written: the threshold is the pipe buffer, not the 300-byte limit. head -c 300 exits immediately, but printf only takes SIGPIPE once its write no longer fits in the ~64 KiB pipe buffer — below that it completes and exits 0 before head closes the read end. Measured on this runner, not inferred:

--- message 60000 bytes, current form ---   SURVIVED len=300   rc=0
--- message 70000 bytes, current form ---                      rc=141
--- message 200000 bytes, current form ---                     rc=141
--- 200000 bytes, proposed pure-bash form --- SURVIVED len=300  rc=0

That is exactly why the existing suite missed it: every stub error was short. A test using a 300-byte message would have passed against the buggy code and proved nothing.

Truncation is now pure parameter expansion — forks nothing, cannot fail:

err=${err//$'\n'/ }
err=${err:0:300}

Newline stripping is kept deliberately: ::warning:: is line-oriented, so a raw newline leaks everything after it into the plain log.

Four regression cases added (29 total), and each was checked to fail against the pre-fix form before being accepted, so they pin the behaviour instead of merely accompanying it:

against the FIXED code:                          against the BUGGY form:
PASS  100 KiB error: scan survives                FAIL  rc=141 (want 0)
PASS  100 KiB error: message truncated            FAIL  rc=141 (want 0)
PASS  multi-line huge error: scan survives        FAIL  rc=141 (want 0)
PASS  huge multi-line error stays one log line    FAIL  0 lines

Worth noting for the record: the pre-existing gh_retry helper in impl-review.yml uses head -c 500 /tmp/gh_retry.err | tr ..., which is safe — it reads from a file, so there is no upstream writer to receive SIGPIPE. Only the printf | form I introduced was vulnerable.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

CHANGELOG.md:141

• The changelog headline says the safety net had “three holes”, but the entry then enumerates (1)–(4). Please align the headline with the actual list (e.g., change it to “four holes”) to avoid a confusing release note.

- **The pipeline's safety net had three holes, and one of them was armed** — follow-up to #10179,