From 381d7f6a83bdf0b959d5e605a321323fcaea9a82 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 25 Oct 2026 19:37:34 -0400 Subject: [PATCH 1/2] fix(source-control): bind a review re-run after a credential or credit failure A review lane's second attempt bound only after a usage or publish rate limit, so a re-run after a rejected Claude credential (failure-class=auth) or a spent key (credit-stop) stayed rerun-not-allowed and held the pull request. Attempt 1 left no verdict in those cases too, so the re-run cannot trade one review for another. rerun_binds adds them for binding only; the merge gate's automatic re-run still keys on rate_limited, so it cannot spend the one attempt before the secret or credit is fixed. Co-Authored-By: Claude Opus 5.5 --- .../fix-rerun-binds-infra-failure-3c71e0a2.md | 7 ++++++ .../skills/babysit-prs/reference/safety.md | 5 +++- .../scripts/babysit_review_evidence.py | 19 ++++++++++++++- .../tests/test_babysit_review_evidence.py | 24 +++++++++++++++++++ 4 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 .changes/source-control/fix-rerun-binds-infra-failure-3c71e0a2.md diff --git a/.changes/source-control/fix-rerun-binds-infra-failure-3c71e0a2.md b/.changes/source-control/fix-rerun-binds-infra-failure-3c71e0a2.md new file mode 100644 index 0000000000..a140ce1717 --- /dev/null +++ b/.changes/source-control/fix-rerun-binds-infra-failure-3c71e0a2.md @@ -0,0 +1,7 @@ +--- +bump: patch +--- + +### Fixed + +- `/source-control:babysit-prs` binds a review lane's second attempt after the first failed on a rejected Claude credential (`failure-class=auth`) or a spent key (`failure-class=credit-stop`), as it already did after a usage limit. Such a re-run was refused as `rerun-not-allowed`, so a pull request whose review was re-run after a credential outage stayed held. The merge gate still re-runs a review only after a rate limit, so a re-run before the secret or credit is fixed cannot spend the one allowed attempt. diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index 8d7a844a3d..9ef4abe3c9 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -856,7 +856,10 @@ and the run is on its first attempt. Five hours is the plan's session-limit wind has reset whenever it started. A weekly limit outlasts it, and the rerun then fails again and stays failed, because an attempt above one is never rerun: at most one gate rerun per check per head. Any other failure class is never rerun, and a check-only run never reruns. The rerun only restarts the -review; the PR still merges only once the check reports SUCCESS. +review; the PR still merges only once the check reports SUCCESS. The review evidence also binds a +second attempt after a `class=auth` or `class=credit-stop` failure (a rejected credential or a spent +key, which left no verdict), but the gate never starts that rerun: a person or session reruns it once +the secret or credit is fixed, since a rerun before then would spend the one attempt. - **Pointer**: the `class=` contract in ci-workflows [`.github/actions/report-lane-outcome/action.yml`](https://github.com/melodic-software/ci-workflows/blob/main/.github/actions/report-lane-outcome/action.yml), diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py index dc3319fe20..360f1243fc 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py @@ -98,6 +98,12 @@ # The reconciler re-runs any failed lane run once; attempt 2 binds only when # attempt 1 failed on a usage limit. RATE_LIMIT_CLASS_RE = re.compile(r"\bclass=rate-limit\b") +# Or on a rejected credential or a spent key (run-claude-cli's lane-class): +# like a usage limit, attempt 1 then left no verdict, so a re-run cannot trade +# one review for another. These bind a re-run but do not start one +# (rate_limited), since re-running before the secret or credit is fixed would +# spend the one allowed attempt. +INFRA_CLASS_RE = re.compile(r"\bclass=(?:auth|credit-stop)\b") # Or when the review's own post hit GitHub's API rate limit: the message opens # with this lane's publish-review failure line (ci-workflows publish.cjs `fail`) # whose detail, the request error's message, opens with GitHub's primary or @@ -376,7 +382,7 @@ def _bound_record(lane: str, sha: str, lane_runs: list[Mapping[str, Any]]) -> _R raise Unbound("rerun-not-allowed") if first is None: raise Unbound("evidence-missing") - if first.get("conclusion") != "failure" or not rate_limited( + if first.get("conclusion") != "failure" or not rerun_binds( lane, _lane_job(lane, newest["id"], first) ): raise Unbound("rerun-not-allowed") @@ -447,6 +453,17 @@ def limited(message: str) -> bool: ) +def rerun_binds(lane: str, job: Mapping[str, Any]) -> bool: + """Whether attempt 1's lane `job` lets attempt 2 bind: a rate limit, or a + rejected credential or spent key (INFRA_CLASS_RE).""" + return rate_limited(lane, job) or any( + isinstance(a, Mapping) + and a.get("annotation_level") == "failure" + and INFRA_CLASS_RE.search(str(a.get("message") or "")) + for a in job["annotations"] + ) + + def _record(lane: str, sha: str, job: Mapping[str, Any]) -> _Record | None: """The job's one completion record for `sha`, None when it left none.""" found = [ diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py index 7e6d6bb14b..3e0495d6e8 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py @@ -564,6 +564,30 @@ def test_second_attempt_after_a_rate_limit_failure(self) -> None: result = self.bind([self.lane_run(first, self.ok(2))]) self.assertTrue(result["claude-review"].bound) + def test_second_attempt_after_a_credential_or_credit_failure(self) -> None: + for cls, why in ( + ( + "auth", + "the Claude credential was rejected; the owner replaces the secret", + ), + ("credit-stop", "the key ran out of credits"), + ): + with self.subTest(cls=cls): + failed = dict( + OTHER_FAILURE, + message=f"claude-review: failure-class={cls}: {why}", + ) + first = attempt("claude-review", S, 1, [failed], "failure") + result = self.bind([self.lane_run(first, self.ok(2))]) + self.assertTrue(result["claude-review"].bound) + + def test_a_credential_failure_is_not_a_rate_limit(self) -> None: + failed = dict( + OTHER_FAILURE, + message="claude-review: failure-class=auth: the Claude credential was rejected", + ) + self.assertFalse(ev.rate_limited("claude-review", {"annotations": [failed]})) + def test_second_attempt_after_a_publish_rate_limit_failure(self) -> None: first = attempt("claude-review", S, 1, [PUBLISH_RATE_LIMIT], "failure") result = self.bind([self.lane_run(first, self.ok(2))]) From 7ddf6f9918208348ecb918fc153ff4f12ecf21b2 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:52:56 -0400 Subject: [PATCH 2/2] fix(source-control): bind only the lane's own failure-level auth line A rerun now binds after attempt 1 failed only when a failure-level annotation opens with `: failure-class=auth: `, matched like the publish rate-limit line, so `class=auth-x`, a quoted class in detail text, another lane's line, or publish.cjs's warning-level ` key: class=auth` cannot bind. credit-stop is dropped: publish.cjs reports a spent key as rate-limit, which already binds. rate_limited and rerun_binds share one failure-annotation scan. Co-Authored-By: Claude Opus 5.5 --- .../fix-rerun-binds-infra-failure-3c71e0a2.md | 2 +- .../skills/babysit-prs/reference/safety.md | 7 +- .../scripts/babysit_review_evidence.py | 47 +++++++------ .../tests/test_babysit_review_evidence.py | 67 ++++++++++++++----- 4 files changed, 81 insertions(+), 42 deletions(-) diff --git a/.changes/source-control/fix-rerun-binds-infra-failure-3c71e0a2.md b/.changes/source-control/fix-rerun-binds-infra-failure-3c71e0a2.md index a140ce1717..a2f87aea4f 100644 --- a/.changes/source-control/fix-rerun-binds-infra-failure-3c71e0a2.md +++ b/.changes/source-control/fix-rerun-binds-infra-failure-3c71e0a2.md @@ -4,4 +4,4 @@ bump: patch ### Fixed -- `/source-control:babysit-prs` binds a review lane's second attempt after the first failed on a rejected Claude credential (`failure-class=auth`) or a spent key (`failure-class=credit-stop`), as it already did after a usage limit. Such a re-run was refused as `rerun-not-allowed`, so a pull request whose review was re-run after a credential outage stayed held. The merge gate still re-runs a review only after a rate limit, so a re-run before the secret or credit is fixed cannot spend the one allowed attempt. +- `/source-control:babysit-prs` binds a review lane's second attempt after the first failed on a rejected Claude credential (the lane's own `failure-class=auth` failure line), as it already did after a usage limit or a spent key, which the lane reports as `rate-limit`. Such a re-run was refused as `rerun-not-allowed`, so a pull request whose review was re-run after a credential outage stayed held. The merge gate still re-runs a review only after a rate limit, so a re-run before the secret is fixed cannot spend the one allowed attempt. diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index 9ef4abe3c9..c02da8e00b 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -857,9 +857,10 @@ has reset whenever it started. A weekly limit outlasts it, and the rerun then fa failed, because an attempt above one is never rerun: at most one gate rerun per check per head. Any other failure class is never rerun, and a check-only run never reruns. The rerun only restarts the review; the PR still merges only once the check reports SUCCESS. The review evidence also binds a -second attempt after a `class=auth` or `class=credit-stop` failure (a rejected credential or a spent -key, which left no verdict), but the gate never starts that rerun: a person or session reruns it once -the secret or credit is fixed, since a rerun before then would spend the one attempt. +second attempt after the lane's own `: failure-class=auth:` failure line (a rejected +credential, which left no verdict), but the gate never starts that rerun: a person or session reruns +it once the secret is fixed, since a rerun before then would spend the one attempt. A spent key +already reports as `rate-limit`. - **Pointer**: the `class=` contract in ci-workflows [`.github/actions/report-lane-outcome/action.yml`](https://github.com/melodic-software/ci-workflows/blob/main/.github/actions/report-lane-outcome/action.yml), diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py index 360f1243fc..358146eec6 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py @@ -98,12 +98,13 @@ # The reconciler re-runs any failed lane run once; attempt 2 binds only when # attempt 1 failed on a usage limit. RATE_LIMIT_CLASS_RE = re.compile(r"\bclass=rate-limit\b") -# Or on a rejected credential or a spent key (run-claude-cli's lane-class): -# like a usage limit, attempt 1 then left no verdict, so a re-run cannot trade -# one review for another. These bind a re-run but do not start one -# (rate_limited), since re-running before the secret or credit is fixed would -# spend the one allowed attempt. -INFRA_CLASS_RE = re.compile(r"\bclass=(?:auth|credit-stop)\b") +# Or on a rejected Claude credential: this lane's publish-review failure line +# (ci-workflows publish.cjs `fail`) with class `auth`. Like a usage limit, +# attempt 1 then left no verdict, so a re-run cannot trade one review for +# another. It binds a re-run but does not start one (rate_limited), since a +# re-run before the secret is fixed would spend the one allowed attempt. A +# spent key needs no entry: publish.cjs reports it as `rate-limit`. +AUTH_FAILURE_RE = re.compile(r"(?P[\w-]+): failure-class=auth: ") # Or when the review's own post hit GitHub's API rate limit: the message opens # with this lane's publish-review failure line (ci-workflows publish.cjs `fail`) # whose detail, the request error's message, opens with GitHub's primary or @@ -438,29 +439,33 @@ def _lane_job(lane: str, run_id: int, attempt: Mapping[str, Any]) -> Mapping[str return job -def rate_limited(lane: str, job: Mapping[str, Any]) -> bool: - def limited(message: str) -> bool: - publish = PUBLISH_RATE_LIMIT_RE.match(message) - return bool(RATE_LIMIT_CLASS_RE.search(message)) or ( - publish is not None and publish["lane"] == lane - ) +def _failures(job: Mapping[str, Any]) -> list[str]: + """The messages of the job's failure-level annotations.""" + return [ + str(a.get("message") or "") + for a in job["annotations"] + if isinstance(a, Mapping) and a.get("annotation_level") == "failure" + ] + +def _lane_line(pattern: re.Pattern[str], lane: str, message: str) -> bool: + """Whether `message` opens with `pattern` for this lane.""" + found = pattern.match(message) + return found is not None and found["lane"] == lane + + +def rate_limited(lane: str, job: Mapping[str, Any]) -> bool: return any( - isinstance(a, Mapping) - and a.get("annotation_level") == "failure" - and limited(str(a.get("message") or "")) - for a in job["annotations"] + RATE_LIMIT_CLASS_RE.search(m) or _lane_line(PUBLISH_RATE_LIMIT_RE, lane, m) + for m in _failures(job) ) def rerun_binds(lane: str, job: Mapping[str, Any]) -> bool: """Whether attempt 1's lane `job` lets attempt 2 bind: a rate limit, or a - rejected credential or spent key (INFRA_CLASS_RE).""" + rejected credential (AUTH_FAILURE_RE).""" return rate_limited(lane, job) or any( - isinstance(a, Mapping) - and a.get("annotation_level") == "failure" - and INFRA_CLASS_RE.search(str(a.get("message") or "")) - for a in job["annotations"] + _lane_line(AUTH_FAILURE_RE, lane, m) for m in _failures(job) ) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py index 3e0495d6e8..b6df035408 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py @@ -67,6 +67,13 @@ def record(lane: str, sha: str, tail: str) -> dict[str, Any]: "title": "claude-review failed", "message": "failure-class=incomplete: no completed review was found on this head", } +# publish-review's failure line (ci-workflows publish.cjs `fail`) for a +# rejected Claude credential. +AUTH_FAILURE = dict( + OTHER_FAILURE, + message="claude-review: failure-class=auth:" + " the Claude credential was rejected; the owner replaces the secret", +) # publish-review's failure line (ci-workflows publish.cjs `fail`) as a run # recorded it when the review's own post hit GitHub's API rate limit. @@ -564,29 +571,55 @@ def test_second_attempt_after_a_rate_limit_failure(self) -> None: result = self.bind([self.lane_run(first, self.ok(2))]) self.assertTrue(result["claude-review"].bound) - def test_second_attempt_after_a_credential_or_credit_failure(self) -> None: - for cls, why in ( - ( - "auth", - "the Claude credential was rejected; the owner replaces the secret", - ), - ("credit-stop", "the key ran out of credits"), + def test_second_attempt_after_a_credential_failure(self) -> None: + first = attempt("claude-review", S, 1, [AUTH_FAILURE], "failure") + result = self.bind([self.lane_run(first, self.ok(2))]) + self.assertTrue(result["claude-review"].bound) + + def test_a_warning_level_auth_does_not_bind(self) -> None: + # publish.cjs warns ` key: class=auth` when the key leg's + # credential is refused, whatever the run then failed on; only a + # failure-level line names why the run failed. + timeout = dict( + OTHER_FAILURE, + message="claude-review: failure-class=timeout:" + " the review leg reached its time limit", + ) + for message in ( + "claude-review key: class=auth: the API refused the key;" + " the owner must replace the secret.", + AUTH_FAILURE["message"], + ): + with self.subTest(message=message): + warning = dict( + AUTH_FAILURE, annotation_level="warning", message=message + ) + first = attempt("claude-review", S, 1, [warning, timeout], "failure") + result = self.bind([self.lane_run(first, self.ok(2))]) + self.assertUnbound(result["claude-review"], "rerun-not-allowed") + + def test_an_auth_lookalike_does_not_bind(self) -> None: + for message in ( + "claude-review: failure-class=auth-x: something else", + "claude-review: failure-class=other: an unclassified failure" + " (detail quoting failure-class=auth: rejected)", + "security-review: failure-class=auth: another lane's credential", ): - with self.subTest(cls=cls): - failed = dict( - OTHER_FAILURE, - message=f"claude-review: failure-class={cls}: {why}", + with self.subTest(message=message): + first = attempt( + "claude-review", + S, + 1, + [dict(AUTH_FAILURE, message=message)], + "failure", ) - first = attempt("claude-review", S, 1, [failed], "failure") result = self.bind([self.lane_run(first, self.ok(2))]) - self.assertTrue(result["claude-review"].bound) + self.assertUnbound(result["claude-review"], "rerun-not-allowed") def test_a_credential_failure_is_not_a_rate_limit(self) -> None: - failed = dict( - OTHER_FAILURE, - message="claude-review: failure-class=auth: the Claude credential was rejected", + self.assertFalse( + ev.rate_limited("claude-review", {"annotations": [AUTH_FAILURE]}) ) - self.assertFalse(ev.rate_limited("claude-review", {"annotations": [failed]})) def test_second_attempt_after_a_publish_rate_limit_failure(self) -> None: first = attempt("claude-review", S, 1, [PUBLISH_RATE_LIMIT], "failure")