Skip to content

fix(source-control): re-run an AI review check that failed on its runner install - #10439

Merged
melodic-automation-lanes[bot] merged 1 commit into
mainfrom
work-resolve-issue/issue-9073-40452550361-1
Oct 27, 2026
Merged

melodic-automation-lanes[bot] merged 1 commit into
mainfrom
work-resolve-issue/issue-9073-40452550361-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #9073

Summary

Opened by the work-resolve-issue lane. The run's own summary:

The babysit merge gate now re-runs an AI review check right away when it failed with the lane's failure-class=install-failed line, with no five-hour wait. Before, a timed-out bubblewrap or CLI install left the lane red until a person re-ran it. install-failed is now defined once in babysit_review_evidence: rerun_binds accepts it, and the gate reads the classes it re-runs (GATE_RERUN_CLASSES) from the same table. The attempt-1-only, one-re-run-per-check-per-head and pinned-live-head rules are unchanged, and a credential failure is still left to a person, even when it appears beside a class the gate re-runs.

Not verified

  • The Python unit tests (test_babysit_merge_rate_limit_rerun.py, test_babysit_review_evidence.py, test_babysit_merge_skip.py) were not executed: this lane runs no interpreter, and check-affected runs without --tests. They were traced by hand and by the review subagent, and are left to the PR's CI affe
  • Person check from the decision: over a week of review-lane runs, no lane stays red on the install step without a re-run
  • typos and the validate-plugins/changed-skills sub-checks that check-affected skipped here are left to the PR's CI

Fix

  • plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py: adds INSTALL_FAILED and INSTALL_FAILURE_RE (publish.cjs <lane>: failure-class=install-failed: line), a RERUN_CLASSES table (rate-limit, publish-rate-limit, auth, install-failed), GATE_RERUN_CLASSES (the subset the
  • plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py: rerun_rate_limited_review classifies the annotations before the wait, re-runs only when every matched class is in GATE_RERUN_CLASSES, waits five hours unless every matched class is in NO_WAIT_RERUN_CLASSES (install-failed), and name
  • plugins/source-control/skills/babysit-prs/scripts/babysit_merge_skip.py: STABLE_RERUNS keys on the refusal reason's prefix failure class is not
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_rate_limit_rerun.py: tests that install-failed re-runs at once on both lanes, is re-run only once, waits when it appears beside a rate limit, is refused beside a credential failure or as a lookalike, that auth stays refused,
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_review_evidence.py: adds the INSTALL_FAILURE fixture and tests that attempt 2 binds after an install failure, that lookalikes and a third attempt do not, and that each gate class binds; the gate-binds test now asserts GATE_RERUN_CL
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_skip.py: the stable-rerun fixture uses the new refusal reason
  • plugins/source-control/skills/babysit-prs/reference/safety.md: the gate re-run rule names install-failed beside rate-limit and publish-failed, with no wait for it; adds the ci-workflows pointer; As of 2026-10-27
  • .changes/source-control/fix-9073-rerun-install-failed-review-5d3a9c1e.md: patch changelog fragment (written by hand: new-changelog-fragment refuses a detached HEAD)

Verification

The runner ran each check from the base copy against 99aeb6f:

  • scripts/check-changelog-fragments.sh --check: passed

The run reports:

  • read https: //github.com/melodic-software/ci-workflows/pull /893: merged 2026-10-26 as 3180217; the lane reports failure-class=install-failed when its Isolate the runner step fails or is cancelled, where it used to report no-outcome
  • read https: //github.com/melodic-software/ci-workflows/pull /893/files: pr-review-cli.yml legs fall back to 'install-failed' on steps.isolate failure/cancelled; publish.cjs gains a WHY entry for install-failed
  • read https: //raw.githubusercontent.com/melodic-software/ci-workflows/main/.github/actions/publish-review/publish.cjs and the copy at pin 30b251532b2e9f7e0c4215e1dee5be0d3c688743: fail() writes ${lane}: failure-class=${failureClass}: ${WHY} (plus (detail)) via core.setFailed; lane is env.LANE_I
  • read https: //raw.githubusercontent.com/melodic-software/ci-workflows/30b251532b2e9f7e0c4215e1dee5be0d3c688743/.github/workflows/pr-review-cli.yml: has id: isolate and the install-failed fallback; the publish step sets LANE_ID to claude-review or claude-security-review. This repo's pr-review-hoste
  • read https: //raw.githubusercontent.com/melodic-software/ci-workflows/main/.github/workflows/pr-review-security.yml: the older reusable writes failure-class=install-failed: <why> with no lane prefix; neither caller here uses it
  • .lane/checks/bin/check-changelog-fragments: all 9 fragments valid
  • .lane/checks/bin/check-affected: 11 passed, 6 skipped (shellcheck: no changed shell; typos not installed; validate-plugins and changed-skills sub-checks that run code from the checked tree), over 8 changed files, ruff included
  • cant-fail-scan --check on test_babysit_merge_rate_limit_rerun.py, test_babysit_review_evidence.py and test_babysit_merge_skip.py: PASS, no gating findings (3 advisory findings on existing tests this change did not touch)
  • fresh-context review subagent over the whole diff: no correctness bugs. Two notes fixed: a credential failure beside install-failed is now refused, and the skip fixture uses the new reason
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/40452550361

Left out: the items under Not verified.

Related

#9073

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 99aeb6fd7dc132fa898b41650164be9084347249: 0 finding(s), 0 as inline comments.

No security findings.

critical-findings: 0

I read the whole diff and the full head versions of babysit_review_evidence.py and babysit_merge.py. Both are trusted paths. I could not run the tests.

What the change does. It adds install-failed as a failure class after which attempt 2 binds in rerun_binds, and as a class the merge gate re-runs at once with no five-hour wait.

Why I treat it as not widening a merge hold.

  • Match is anchored and lane-checked. The new matcher, INSTALL_FAILURE_RE, goes through _lane_line. It uses .match at the message start and requires the lane to equal the job's own lane. Another lane's line, a quoted line and install-failed-x do not match. The new tests cover each of these.
  • A second attempt still has to earn a pass. A re-run only binds when attempt 1 concluded failure and max(attempts) <= 2. The gate repeats the attempt-1 check before it posts the re-run. Attempt 2 must still succeed with its own verdict record on the same head. The one-re-run, attempt-1-only and pinned-live-head rules are unchanged.
  • The auth class stays with a person. The gate refuses when the annotations name any class outside GATE_RERUN_CLASSES, so a credential failure beside install-failed is refused. Previously rate_limited could re-run a job that carried both a rate-limit line and an auth line. The gate is now stricter there.
  • Forgery adds no new capability. A job step that writes a failure annotation with the exact lane prefix could already forge class=rate-limit (an unanchored regex) or the publish-failed and auth lines. The new class gives the same single re-run, with the five-hour wait skipped. The re-run reviews the same head and its result is judged on its own, so I did not report this.
  • The refusal-reason prefix is safe. STABLE_RERUNS now matches the prefix "failure class is not ". The new refusal reason opens with it, and nothing else in the diff produces a reason with that prefix.

Cost. The extra re-run spends only prepaid or subscription usage, so I set no hold and did not count it as a finding.

Manifests. The diff changes no plugin manifest.

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 99aeb6fd7dc132fa898b41650164be9084347249: 0 finding(s), 0 as inline comments.

No blocking findings. I read the full diff and PR context and traced the logic by hand. I ran no tests, so the PR's "Not verified" claims stay author-claimed.

Checked:

  • rerun_rate_limited_review (babysit_merge.py) classifies annotations before the wait. It re-runs only when every matched class is in GATE_RERUN_CLASSES, so auth beside install-failed is refused.
  • The five-hour wait is skipped only when every matched class is in NO_WAIT_RERUN_CLASSES, so install-failed beside a rate limit still waits.
  • The attempt-1-only, one-re-run-per-head, pinned-head and job-binding checks still run before the re-run is posted.
  • rerun_binds now delegates to rerun_classes over the RERUN_CLASSES table, which holds the previous three classes plus install-failed. INSTALL_FAILURE_RE is anchored with match, so lookalikes (another lane, a quoted line, install-failed-x) do not bind. No callers of the removed rate_limited remain.
  • STABLE_RERUNS keys on the failure class is not prefix, which the new refusal reason starts with, and the skip fixture matches it.

Closing issue #9073, acceptance criteria:

  • Re-run on install-failed on attempt 1 with no wait, every other class still refused: met.
  • Test for one re-run, no second attempt and an unlisted class refused: met.
  • safety.md names install-failed beside rate-limit and publish-failed: met.
  • The ci-workflows#893 release and the repin of pr-review-hosted.yml and pr-review-security-hosted.yml: not part of this diff. The issue assigns it to a separate repin PR, and the gate has no Isolate the runner failure to re-run until that lands.
  • The last criterion in the context file is truncated and could not be read. It is not marked met.

Manifests: no plugin manifest changed, so no manifest lines apply. The fragment's bump: patch fits a bug fix.

Trusted paths: babysit_merge.py and babysit_review_evidence.py changed and were read in full. The change adds one class to the gate's allow-list. The re-run still runs under the owner's token on the pinned live head, and auth stays excluded.

Spend: the change causes at most one extra review-lane re-run per failed check per head, which uses prepaid Actions and model usage. It is not a hold.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head 99aeb6fd7dc132fa898b41650164be9084347249. A person must clear this reason, so the pr-merge lane added the needs-human label: fix the reason, then remove the label, and the next scheduled run judges it again.

it changes plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py, a hold path

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026

@kyle-sexton kyle-sexton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Owner approval of head 99aeb6f.

@kyle-sexton kyle-sexton removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head 99aeb6fd7dc132fa898b41650164be9084347249.

@melodic-automation-lanes
melodic-automation-lanes Bot added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 8ccaa19 Oct 27, 2026
75 of 77 checks passed
@melodic-automation-lanes
melodic-automation-lanes Bot deleted the work-resolve-issue/issue-9073-40452550361-1 branch October 27, 2026 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pr-review: the bubblewrap install step times out at 1 minute and fails review lanes with no outcome

1 participant