Skip to content

Fix stacked PR reviews to use PR base, not main - #59

Merged
Octember merged 3 commits into
mainfrom
fix/stacked-pr-base
Aug 24, 2026
Merged

Fix stacked PR reviews to use PR base, not main#59
Octember merged 3 commits into
mainfrom
fix/stacked-pr-base

Conversation

@Octember

Copy link
Copy Markdown
Owner

Summary

  • Diff stacked PRs via GitHub compare (baseRefOid...headRefOid) instead of gh pr diff against the default-branch checkout.
  • Check out a detached worktree at the PR head SHA so Codex reads the same tree the diff describes.
  • Trust ~/.stupify/worktrees in Codex config during VM setup so sandbox reads work outside the main repo checkout.

Test plan

  • bun run typecheck
  • bun test
  • Dry-run review on a stacked PR whose base is a feature branch (not main)

Made with Cursor

Octember and others added 2 commits August 23, 2026 22:14
Compare baseRefOid..headRefOid for the diff and run codex in a detached head worktree so file reads match stacked branches.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

@exe-dev-github-integration exe-dev-github-integration 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.

manual reviews are broken in two places 👇

Comment thread src/sweep/review-one.ts
Comment thread src/sweep/review-one.ts Outdated
Request baseRefOid/baseRefName from gh pr view, and only prepare a head worktree when cwd is the target repo.

Co-authored-by: Cursor <cursoragent@cursor.com>

@exe-dev-github-integration exe-dev-github-integration 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.

stacked bases expose two more correctness gaps 👇

Comment thread src/sweep/sweep.ts

// Fetch the diff once, here in the runner — codex reviews it from the prompt with no network/gh of its own.
const read = getDiff(cfg, pr.number)
const read = getDiff(cfg, pr)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 src/sweep/sweep.ts:121 · bug · conf 0.98
The diff now depends on baseRefOid, but dedup/failure state is still keyed only by headRefOid; when a stacked base advances without changing this PR's head, the sweep skips the changed compare entirely.
→ Fix: key the existing review marker and local attempt state by the base+head compare identity, not head alone.

Comment thread src/sweep/worktree.ts
rmSync(dir, { recursive: true, force: true })
exec('git', ['worktree', 'prune'], { cwd: repoDir })
if (
!exec('git', ['fetch', '-q', 'origin', pr.baseRefOid, pr.headRefOid], { cwd: repoDir }).ok ||

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 src/sweep/worktree.ts:20 · footgun · conf 0.93
Default pool workers call git fetch concurrently against the same checkout, contending on shared fetch metadata such as FETCH_HEAD; one preparation can fail and throttle an otherwise valid PR.
→ Fix: pass Git's existing --no-write-fetch-head flag, or perform the fetches in the existing serial candidate-collection phase.

@Octember
Octember merged commit b35cd9b into main Aug 24, 2026
1 check passed
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