Skip to content

fix(pr-pipeline): check a resumed issue run over its whole branch - #8567

Merged
kyle-sexton merged 5 commits into
mainfrom
fix/resume-checks-whole-branch
Oct 20, 2026
Merged

kyle-sexton merged 5 commits into
mainfrom
fix/resume-checks-whole-branch

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Refs: #8425

Summary

A resumed work-resolve-issue run whose work was already committed reported done with no new commit, and the runner stopped it for a person (issue-lane run 39386636889, #8530): run-checks only checked the run's own lane branch, which a resume with no new commit does not have, so report-outcome had no check over the work and opened no PR. Every resume whose work was already done stopped the same way; resumed admissions (#8425) are held on this.

Fix

  • run-checks (.github/workflows/work-resolve-issue.yml, "Pick the lane branch") checks the resumed branch when the run kept no branch, but only when lane.json's resume equals the runner-computed needs.resolve-issue.outputs.resume-branch and matches ^work-resolve-issue/issue-<this issue>-<n>-<n>$; it reaches checkout only through with: ref.
  • pr-run-activity-write.yml exposes resume-branch as a workflow_call output (no trigger, input or permission change).
  • report-outcome.mjs opens a PR from the resumed branch only if the check results name that branch, the branch is still at the checked tip, at least one check ran and none failed, and every commit ahead of the default branch is the lanes App's (a truncated commit list fails). The compare uses main...<checked tip>; after opening (or reusing an open App PR), a head SHA other than the checked tip closes the PR and hands the issue back.

Security pass: GitHub Security & Cost passed 84b0a4c and the F1/F2 hardening in 99c3929. Context recorded by that review: the App-commit check proves only "not a human's branch", since the model also holds the App token.

Verification

  • report-outcome.test.mjs 182/182 (new tests failed first), with callers.test.mjs 197/197; actionlint clean on both workflows; pre-push check-affected passed.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT

kyle-sexton and others added 3 commits October 20, 2026 03:41
A resumed work-resolve-issue run that finished the inherited work with no
commit of its own kept no branch, so run-checks found nothing to check and
report-outcome stopped every such run for a person.

run-checks now falls back to the branch the run resumed (lane.json resume)
and runs the activity's checks over that whole branch. report-outcome opens
the PR from it only when the results name that branch, at least one check
ran and none failed, the branch is still at the tip checked, and every
commit on it ahead of the default branch is verified, authored by the lanes
App and committed by web-flow. The PR body's Verification section records
that; any other case still hands back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT
…ch and checked tip

lane.json can be rewritten by code the model started, so run-checks now
checks its resume branch only when it equals the resume-branch issue-prior
chose before the mint, exported as a new output of pr-run-activity-write.
report-outcome counts results only for the branch they name, so a mismatch
leaves no results and the run stops.

The branch name can move after the tip check, so report-outcome reads the
compare at the checked SHA and closes a new PR whose head is not that SHA,
handing the issue back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 20, 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-20T08:09:27.666579Z 0296dbc 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0296dbceeb

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

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

Incremental review of 1020824 (since 87fb50e)

No blocking findings. I read the three changed files and the surrounding code in post-statuses.mjs and report-outcome.mjs.

I did not run the tests. The sandbox refused node --test for post-statuses.test.mjs and report-outcome.test.mjs (approval required). Treat the test results as unverified by this lane.

What the change does. A resumed run's statuses now land on the checked tip only when the same gate report-outcome applies to opening that PR passes. That gate has three parts:

  • A check ran (checksPassed).
  • No check failed.
  • Every commit ahead of the default branch is the lanes App's (listedCommits and allLaneCommits).

The helpers are exported from report-outcome.mjs and reused. The compare call in report-outcome.mjs main and the new one in post-statuses.mjs agree:

  • Both compare base...checks.tip and take default_branch from GET /repos/{repo}.
  • Both use the LANES_BOT_LOGIN default melodic-automation-lanes[bot].
  • Neither workflow job sets LANES_BOT_LOGIN, so both use the default.
  • checks.tip goes into the compare URL unencoded, but readChecks has already checked it against ^[0-9a-f]{40}$.
  • A truncated listing returns null, so allLaneCommits rejects it and the gate fails closed.

Notes, none blocking:

  • In post-statuses.mjs (around lines 143–152), an API error on the default-branch or compare call now throws and the step exits 1. That only happens for a resumed run whose checks passed. The failure is closed, which is consistent with the gate.
  • In report-outcome.mjs (around lines 794–800), the comment above allLaneCommits ("A resume carries every commit… passes none") now sits above the new listedCommits comment and no longer sits directly above its function. Move it back down to allLaneCommits.
  • The new test matrix covers a failing check, no check, a non-App commit, a non-matching resume and no results. It does not cover a truncated listing (total_commits larger than commits.length) or an unverified commit. Both are laneCommit and listedCommits behaviours already tested through report-outcome, so I'd call this optional.

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

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

No security findings in the incremental change since 87fb50e. I reviewed post-statuses.mjs, post-statuses.test.mjs and report-outcome.mjs from the incremental diff and the surrounding code. I did not consult the instruction exception register, because the diff changes no standing instructions.

The change tightens a gate. A resumed run's commit statuses are now posted on the resumed branch's checked tip only when all of these hold:

  • lane.json's resume equals the runner's RESUME_BRANCH.
  • A check ran and none failed (checksPassed).
  • Every commit between the default branch and the tip is verified, authored by the lanes App bot, and committed by web-flow (allLaneCommits).

Points checked:

  • Compare URL: checks.tip is validated by readChecks as 40 lowercase hex characters, and readChecks also requires the branch to match, so it can't inject into the compare path. The base is encodeURIComponent-encoded.
  • Truncated listings: listedCommits returns null when the compare listing is cut short, and allLaneCommits then fails closed.
  • API failures: the new github() calls throw on a non-OK response. The error goes to the existing top-level catch, so it fails the job rather than posting statuses.
  • Bot login: LANES_BOT_LOGIN comes from workflow env and defaults to melodic-automation-lanes[bot].
  • Ordering: moving the repo and github setup earlier changes no behavior.
  • Refactor: extracting checksPassed and listedCommits keeps report-outcome's semantics.
  • Tests: they cover the new rejection cases (failing check, no check run, non-App commit).

…'s checked tip

A resumed run that committed nothing has its PR opened from the branch it
resumed, but post-statuses found no branch of the run and posted nothing, so
the earlier run's failing pipeline/resolve-issue status stayed on the PR head
and held the merge gate. post-statuses now posts on the tip run-checks
checked, when lane.json's resume equals the runner's resume-branch and the
check results name that branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT
…come's gate

post-statuses took the resumed branch's checked tip whenever the check
results parsed for it, so a failing or empty check list, or a branch with a
commit not the lanes App's, still got the run's statuses although
report-outcome opened no PR. Both scripts now share report-outcome's gate:
checksPassed (a check ran, none failed) and allLaneCommits over the compare
at the checked tip.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xk7Lz7d2twxvA3LfRxnLkT
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 3dc02a5 Oct 20, 2026
25 checks passed
@kyle-sexton
kyle-sexton deleted the fix/resume-checks-whole-branch branch October 20, 2026 08:40
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