Skip to content

fix(pr-pipeline): start fresh instead of resuming a stale lane branch with no pull request - #9463

Merged
kyle-sexton merged 2 commits into
mainfrom
fix/resolve-issue-stale-branch-resume
Oct 23, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
fix/resolve-issue-stale-branch-resume

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

No related issue: lane stop F23 (Issue Triage), run 39979677366

Summary

Issue #7152 was re-admitted and work-resolve-issue resumed work-resolve-issue/issue-7152-39105891323-1, a branch from an earlier stopped run that was 1 commit ahead of main and 865 behind (run 39979677366). The model committed nothing new, the runner's whole-branch check (check-changelog-fragments.sh --check) failed over the stale base, and the issue went to needs-human with no PR. A resume now requires the branch to be zero commits behind the default branch. A stale branch starts fresh from main instead.

Fix

  • findPrior returns run (with a stale note naming the branch and its behind count) instead of resume when the marked run's branch is behind the default branch. The model then works from the trusted default tip, and the apply job writes a new issue-<n>-<run>-<attempt> branch. The old branch is left in place; nothing deletes it.
  • findPriorMain reads behind_by from the GET /compare call it already makes for each lane branch, so the change adds no API reads. A behind_by that is not a whole number fails the step, the same way ahead_by does.
  • The outcome comments now say a restart continues from the branch only while it is not behind the default branch. The issue-prior contract in docs/conventions/pr-pipeline/pr-run-activity.md and a pr-pipeline changes fragment are updated to match.
  • No change for an open lanes-App PR: findPrior already stops the run when one is open, and pr-unblock's resolve-conflicts path merges main into PR branches. No new write power, and the runner's whole-branch check is unchanged.
  • Consequence: main moves fast, so most turn-cap or stopped resumes will now start fresh rather than continue. If partial-work continuation matters more, a behind-count threshold is the alternative.

Verification

  • node --test .github/scripts/work-resolve-issue/*.test.mjs: 257 pass, 0 fail. New tests cover: a stopped run's branch behind main (1, 865) starts fresh; a malformed behind_by fails the step; an open lanes-App PR on a behind branch still stops; a later re-admission does not fall back to the stale branch; --find-prior emits no resume outputs for a behind branch. Existing tests still cover a fresh issue and a resume at distance 0.
  • actionlint on pr-run-activity-read.yml: clean.
  • scripts/check-changelog-fragments.sh --check: valid. scripts/check-affected.sh --tests: 10 passed, 4 skipped.
  • Left out: a live lane run. That happens on the next re-admission.

Related

N/A

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits October 23, 2026 12:40
… with no pull request

findPrior resumed a stopped run's lane branch whatever its distance from
main, so a re-admitted issue continued a branch 865 commits behind and the
runner's whole-branch checks failed over the stale base. A resume now needs
the branch to be zero commits behind the default branch, read from the
behind_by of the compare call find-prior already makes. A branch behind it
starts fresh from the default tip under a new lane branch; the old branch
is left alone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Address the ready-phase review: fail the issue-prior step on a compare
answer with no whole-number behind_by, as for ahead_by; log one line for a
stale branch instead of two that contradict; word the outcome comments so a
restart continues from the branch only while it is not behind the default
branch; update the issue-prior contract doc; add the changes fragment; list
funding among the resumable outcomes in the workflow comment.

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

chatgpt-codex-connector Bot commented Oct 23, 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-23T16:50:26.273202Z f731abf 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

Copy link
Copy Markdown
Contributor

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

I found no security issues in this pull request, so there are no inline comments and no hold.

What changed: A resume from an earlier run's lane branch now needs behind_by === 0. A branch that is behind the default branch starts fresh from the default tip, and the old branch is left in place. findPriorMain reads behind_by from the compare call it already makes, and fails the step if it isn't a non-negative safe integer.

Why it is not a finding:

  • Trust boundary: The change only narrows when a resume happens. A stale branch now falls back to the trusted default-branch tip, and the lane-App author, web-flow committer, verified-commit and PR checks on resume candidates are unchanged.
  • Early exits: The new stale early return sits after the existing stop and reuse decisions in findPrior, so open lanes-App PRs still stop the run. A new test covers that.
  • Input validation: behind_by is checked the same way as ahead_by, and a bad value fails closed with no model run. Tests cover undefined, a string, null, a non-integer and a negative.
  • Injection surface: stale.branch and behindBy appear only in a console.log line. They are not written to GITHUB_OUTPUT, where a crafted value could add output lines. The stale case writes only the existing run outputs (proceed, skip-reason, reuse-branch) and no resume outputs. The branch name has already matched the lane-branch pattern, and the count is a validated integer.
  • Instruction surface: I found no guardrail removed. The outcome-comment edits and the docs/conventions/pr-pipeline/pr-run-activity.md edit only describe the new behaviour. The workflow change is comment-only, so it widens no permissions: and no trigger.
  • Manifests: No plugin.json or marketplace.json is touched, so there are no manifest <field> lines.
  • PR context: I found no embedded directives aimed at the reviewer in the PR description.
  • Instruction exception register: I did not read it. The diff removes no rule that a hook, deny rule or validator enforces, and the one control it adjusts is tightened. I recognised that by consequence and did not consult the protected-class list.

critical-findings: 0

@github-actions

Copy link
Copy Markdown
Contributor

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

No blocking or advisory findings on head f731abf.

I read .claude-lane/pr.diff and .claude-lane/pr-context.md. I did not run the tests, and the PR's own verification claims (257 passing, actionlint clean) are the author's and unverified by me.

  • Code change: findPrior returns { action: "run", stale } when the resumable lane branch has behindBy !== 0. findPriorMain reads behind_by from the existing compare call and fails closed on a value that is not a non-negative safe integer, matching the existing ahead_by check. The stale branch is taken after the open-PR stop check, so a lanes-App PR open from a behind branch still stops the run. No resume outputs are emitted for a stale branch (runs stays true for the run action).
  • Tests: The new tests cover a behind count of 1 and 865, a lanes-App PR on a behind branch, re-admission after a fresh run, the --find-prior outputs, and a malformed behind_by on the fail-closed path. They also cover a resume at distance 0. The existing test fixtures were updated to carry behindBy and behind_by.
  • Docs and comments: The outcome comments, the workflow comment and the issue-prior convention doc all describe the same rule.
  • Closing issue: None is stated, so there are no criteria to check. The PR context text contains no embedded instructions.
  • Plugin manifests: The diff changes no plugin.json or marketplace.json, so there are no manifest lines. The changelog fragment is bump: patch, which fits a bug fix.
  • Hold: None of the classes apply. The diff adds no model or API spend, touches no trusted-actor or CODEOWNERS paths, and has no critical security finding.

One tradeoff the author already states: resuming now needs zero commits behind, so on a fast-moving default branch most stopped or turn-cap restarts will start fresh and drop their partial work. That is a design choice, not a defect.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit d345b56 Oct 23, 2026
83 of 95 checks passed
@kyle-sexton
kyle-sexton deleted the fix/resolve-issue-stale-branch-resume branch October 23, 2026 17:05
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