Skip to content

fix(server): generate PR diffs from the merge base - #618

Merged
sak0a merged 2 commits into
mainfrom
ryco/agent-control-d93b6878-2372-4305-92a8-1
Sep 23, 2026
Merged

sak0a merged 2 commits into
mainfrom
ryco/agent-control-d93b6878-2372-4305-92a8-1

Conversation

@sak0a

@sak0a sak0a commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

When the PR base advances independently, a two-dot tree diff includes reversed base-only changes in the context sent to generatePrContent. A stale or missing local base can also inflate the context or fail despite an available tracking branch.

Generate stat and patch from a single explicitly resolved common ancestor, while retaining two-dot commit-log semantics. Reject histories with no common ancestor or multiple merge bases with actionable recovery guidance instead of generating misleading content. Invalid refs and other Git failures remain GitCommandErrors; existing truncation limits and markers are preserved.

For unqualified base branches, prefer an existing tracking ref from the intended repository (the head remote for same-repository PRs, origin for the existing fork workflow). Keep the PR target branch name unchanged and preserve explicit refs and local-only/custom base branches. This follows the merged upstream correction at pingdotgg/t3code#13170 and handles additional edge cases in Ryco.

Validation with pinned Bun 1.4.0 and bun install --frozen-lockfile:

  • Original divergence reproduced before the fix: 3 files / 2 insertions / 2 deletions became only the feature file / 1 insertion.
  • Real criss-cross history reproduced two common ancestors and an implicit Git diff that overstated the delta by one file; generation now rejects ambiguity. Unrelated histories now explain fetching/deepening history and choosing or transplanting onto the correct base.
  • With tracking selection disabled, stale/custom/fork fixtures leaked a base commit and missing-local-base failed; local-only and explicit-local cases passed. All pass with the fix, including selection of the target repository instead of a fork's stale base.
  • 46 focused tests passed (34.05s total), including existing PR workflows, ancestor/diverged/empty ranges, invalid refs and 59,000-byte patch truncation plus its marker.
  • Server typecheck, format/check and lint for changed files, and git diff --check passed.
bun run --cwd apps/server test src/git/GitManager.test.ts src/vcs/GitVcsDriverCore.test.ts -t 'PR|readRangeContext'

Verification uses real Git in isolated temporary repositories; text generation and GitHub calls are faked. No dev server, browser session or live model-generated PR was exercised for this backend-only change, and full repository suites were not run. Tracking selection uses available conventional refs/remotes/<remote>/<base> refs; it does not force a new fetch or add custom fetch-refspec destination mapping.

The reported concurrent-destination worktree test flake was reproduced separately: both additions can fail with HEAD/index-lock errors. The same failure reproduced using only two concurrent git worktree add processes, without Ryco or PR range handling. This independent pre-existing issue remains; no worktree implementation or test semantics are changed here.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@github-actions github-actions Bot added size:S vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Sep 23, 2026
@github-actions github-actions Bot added size:M and removed size:S labels Sep 23, 2026
@sak0a
sak0a merged commit 288bdf8 into main Sep 23, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant