Skip to content

fix(work-resolve-issue): skip an issue an open pull request already closes - #10726

Merged
kyle-sexton merged 5 commits into
mainfrom
fix/resolve-issue-skip-open-pr
Oct 28, 2026
Merged

kyle-sexton merged 5 commits into
mainfrom
fix/resolve-issue-skip-open-pr

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: lane duplicate work, #10698 built for #10694 while #10686 already closed it

Summary

The issue lane opened a second PR for an issue that an open PR already closed. The earlier-work check (report-outcome.mjs --find-prior) already queries the issue's open closing PRs and stops the lane on one a listed human or the lanes App opened, but it runs only before the model. In run 40732936415 it logged "no earlier run ... the model runs" at 17:11:21Z. #10686 gained its closing reference at 17:13:25Z, and the lane opened #10698 at 17:19:30Z. That second PR also ran the review lanes and pr-verify.

Fix

  • report-outcome.mjs reads the issue's open closing PRs again just before it opens a PR (a done run's or a reuse's). It skips this read when the lanes App already has a PR open from the branch, which happens when the report is rerun. One GraphQL call: closedByPullRequestsReferences(includeClosedPrs: false), which GitHub's GraphQL reference describes as "List of open pull requests referenced from this issue".
  • It applies the same trust rule as --find-prior: only a PR the lanes App or a listed human opened counts, so an outsider's closing reference cannot stop the lane. The rule is now one shared function, trustedCloser. When such a PR exists, the script opens nothing, logs that PR's number, and stops on the new reason closed-by-open-pr. That reason routes to retry in stop-routes.json, the same route as pr-head-moved, also a race. agent-ready stays on, and while that PR is open the next run stops in --find-prior before any model turn.
  • A failed read throws to main's catch and opens no PR, as the script's other required reads already do.
  • What this does not fix: the model run itself is still spent when the closing reference lands mid-run. The pre-build check was already in place and lost the race.

Verification

  • node --test in .github/scripts/work-resolve-issue: 371 passed, 0 failed. Against the base report-outcome.mjs, 2 of the new tests fail: a listed human's open closing PR still opened a PR, and an unreadable closing list still opened one.
  • The guard tests pass. With no closers, or with only an outsider's or another repository's PR closing the issue, the PR still opens. The lanes App's own PR from the branch is kept, and the closers are not read.
  • python3 plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py: 14 tests OK. scripts/check-changelog-fragments.sh --check and scripts/check-comment-hygiene.sh pass.
  • Left out: no test drives the reuse path through the new check. The reuse path goes through the same open-pr branch.

Related

  • #10686 and #10698, the duplicate pair.
  • pr-pipeline.schema.json is unchanged: it has no enum of stop names, and its skip-reason enum is for the read runner's skip, which this path does not use.

🤖 Generated with Claude Code

kyle-sexton and others added 3 commits October 28, 2026 15:27
…loses

The earlier-work check (report-outcome --find-prior) already stops on a
trusted open closing PR, but it runs before the model. A closing reference
that lands while the model runs is missed: PR 10686 gained a closing
reference to issue 10694 at 17:13, after the check read none at 17:11, and
the lane opened PR 10698 at 17:19. report-outcome now reads the open
closing PRs again just before it opens a PR and stops closed-by-open-pr
(route retry) when one closes the issue.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 28, 2026 19:52
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 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 ✅ Completed 2026-10-28T19:55:09.429296Z 943b162 Draft marked ready
ℹ️ 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

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

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

Incremental security review of .github/scripts/work-resolve-issue/report-outcome.mjs (a trusted path) since 943b162. I found no security issues.

The change only moves trustedCloser and its comment from above findPrior to just after readBranch. This also puts the doc comment for findPrior back directly above that function. The function body is the same on both sides:

  • It matches the repository case-insensitively.
  • It accepts a Bot author only when its login equals the lanes App login, or equals that login with [bot] appended.
  • It accepts a User author only when their databaseId is in the allowlist.

The trust rule is unchanged, so no trust control is widened. trustedCloser is a const arrow function. It is now defined earlier in the module, so it cannot be used before its definition.

I did not re-review the other four files in the PR. They are unchanged since 943b162 and were reviewed then.

Spend: none. This change only adds an existing-style GraphQL read.

critical-findings: 0

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

claude-review: 1 inline finding(s) on 943b16209d0250358f45b81dbc380a87c600fae6; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

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

Incremental review of .github/scripts/work-resolve-issue/report-outcome.mjs since 943b162. The change moves trustedCloser above the doc comment for findPrior. The findPrior comment ("Whether an earlier run already did the issue's work…") sits directly on findPrior again, where the earlier placement had separated it. The function body is the same. It is still defined once, and both callers (lines 1177 and 1796) resolve it. I found no behavior change and no new issues. I did not run the tests; this review was by reading the diff only.

The trusted-path match on this file alone is not a hold. This change spends no billed resources.

kyle-sexton and others added 2 commits October 28, 2026 16:04
…ndPrior

trustedCloser sat between findPrior's contract comment and the function,
so the comment read as trustedCloser's. Move trustedCloser above it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 05d5170 Oct 28, 2026
40 checks passed
@kyle-sexton
kyle-sexton deleted the fix/resolve-issue-skip-open-pr branch October 28, 2026 20:17
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