Skip to content

fix(maintenance): count a runner-loss annotation only on a failed job cut before the judge step - #9871

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/reconcile-hold-judge-guard
Oct 24, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
fix/reconcile-hold-judge-guard

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: hardens the annotation read added in #9813.

Summary

#9813 let the lost-runs reconciler read a cut verify job's annotations and treat a "lost communication with the server" failure annotation as proof the runner was lost, so it can re-run that run. The annotation can come from text a step prints, though. The judge step's log can echo model or pull request text, so a steered judge could print a line that reads as a runner loss and get its run re-run. A job that did not conclude failure could also count.

Fix

  • holdReason counts a runner-loss annotation only on a job whose steps did not list in full and that concluded failure (.github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs:238).
  • A cut step list that reaches the judge step or its predecessor, JUDGE_PREDECESSOR = "Write the prompt and schema" (reconcile.mjs:115), stays held as "after its judge step may have started" (reconcile.mjs:251), rather than read as a lost runner.

Verification

  • node --test .github/scripts/maintenance-reconcile-lost-runs/*.test.mjs: 85 tests, 85 pass.
  • New test a runner-loss annotation counts only on a failed job whose cut list stops before the judge step's predecessor (reconcile.test.mjs:539). It checks that a cut before the predecessor is re-run, that a cut reaching the predecessor or the judge step is held and not recorded, and that a cancelled job is not re-run.
  • pr-verify still names its judge step as the reconciler reads it now also asserts that JUDGE_PREDECESSOR is the step right before JUDGE_STEP in pr-verify.yml (reconcile.test.mjs:437), so renaming either step fails the test.

Related

  • #9813: added the annotation read this change narrows.

🤖 Generated with Claude Code

… cut before the judge step

A judge step that echoes model or PR text can emit a failure-level annotation
that reads as a lost runner. holdReason now counts the annotation only when the
job concluded failure, and holds a cut list that reaches the judge step's
predecessor.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review 🔄 Running since 2026-10-24T21:08:12.511815Z ac0a983 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

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

No findings. I read the full diff (reconcile.mjs and reconcile.test.mjs), the PR context, and the surrounding holdReason/withAnnotations code. I did not run the tests; the "85 of 85 pass" figure is the author's claim and I have not verified it.

  • Closing issue: none. The PR states "No related issue", so there are no acceptance criteria to check.
  • Trusted paths: no changed file matches a listed glob.
  • Manifests: no plugin.json or marketplace.json changes, so there are no manifest <field> lines.
  • Cost: no billed resources are involved, and no hold is needed.
  • holdReason change (reconcile.mjs:234-252):
    • Only a cut job that concluded failure and carries a runner-loss annotation is accepted. Any other cut job still throws, so the next pass reads it again.
    • A cut list that reaches JUDGE_PREDECESSOR is held. A cut list that reaches JUDGE_STEP is already held by the existing check.
    • Together these hold the case where the judge step's log echoes text that reads as a runner loss.
  • Workflow assertion: pr-verify.yml defines both steps twice (lines 224/258 and 549/581). In both jobs "Write the prompt and schema" immediately precedes "Judge the pull request". The new assertion only checks the first occurrence via indexOf. That is fine today, but a rename in only the second job would pass the test while the reconciler's constant stopped matching it.
  • Tests: the new test covers a cut before the predecessor (re-run), a cut reaching the predecessor or the judge step (held and unrecorded), and a cancelled job in both positions (throws, so not re-run).

@github-actions

Copy link
Copy Markdown
Contributor

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

No security findings on PR #9871. critical-findings: 0

The change touches .github/scripts/maintenance-reconcile-lost-runs/reconcile.mjs and its test. It tightens the lost-runner check in holdReason:

  • An annotation that reads as a runner loss now counts only on a cut job that concluded failure. Before, any job with a cut step list could count (reconcile.mjs:238-239).
  • A cut step list that contains JUDGE_PREDECESSOR ("Write the prompt and schema") is held as "after its judge step may have started" (reconcile.mjs:250). The judge step itself was already held by the existing check at reconcile.mjs:245.
  • The held outcomes are the more conservative direction. A job that is not failure now throws and is read again on the next pass. A cut list that reaches the predecessor is no longer re-run.
  • Residual edge, not reported: a cut job whose step list is empty or non-array and that has a runner_id still passes on its annotation alone, because the predecessor check finds no steps to match. An echoed annotation needs a step that ran, so a stale read would have to coincide with a judge step that echoed text. That depends on API read timing, was already possible before this PR, and is not a new exploit path.
  • The new test asserts that JUDGE_PREDECESSOR is the step right before JUDGE_STEP in pr-verify.yml, so a rename of either step fails the test instead of silently weakening the guard.

I changed no trusted-path files, no plugin manifests, and no instruction surfaces. Nothing here draws on paid usage. The PR description is data. It contains no embedded instructions.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit 47cc5b3 Oct 24, 2026
36 checks passed
@kyle-sexton
kyle-sexton deleted the fix/reconcile-hold-judge-guard branch October 24, 2026 21:32
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.

1 participant