autosolve: Sync fork with upstream before pushing branches#30
Open
fantapop wants to merge 1 commit into
Open
Conversation
fantapop
force-pushed
the
autosolve-bwrap-sandbox
branch
from
7d08083 to
465aa67
Compare
fantapop
force-pushed
the
autosolve-fork-sync
branch
from
863edec to
d620cae
Compare
There was a problem hiding this comment.
Pull request overview
This PR reduces push-time workflow-scope failures by syncing the bot’s fork default branch with upstream immediately before pushing a feature branch, and makes the auth boundary explicit by splitting upstream vs fork GitHub clients.
Changes:
- Add
SyncForkto thegithub.Clientinterface and implement it via GitHub’smerge-upstreamAPI in theghCLI client. - Split
implement.Run’s single GitHub client intoupstreamClientandforkClient, routing fork-only operations (branch existence + fork sync) throughforkClient. - Update CLI wiring and tests to use the two-client model and assert the fork sync happens before pushing.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| autosolve/internal/implement/implement.go | Split GitHub client usage and sync fork base branch with upstream before pushing. |
| autosolve/internal/implement/implement_test.go | Update mocks/call sites and assert SyncFork is invoked for successful runs. |
| autosolve/internal/github/github.go | Extend GitHub client interface and add SyncFork implementation using gh api. |
| autosolve/cmd/autosolve/main.go | Construct and pass separate upstream vs fork GitHub clients using different tokens. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fantapop
added a commit
to fantapop/cockroachdb-actions
that referenced
this pull request
May 14, 2026
Address Copilot review feedback on PR cockroachdb#30: - github.Client: wrap the underlying exec error with %w (so exit status survives) and append captured stderr only when present, via a new cmdError helper. Apply to SyncFork, BranchExists, and CreateLabel — previously each dropped err and used stderr alone, which loses signal when stderr is empty. - implement_test: add TestRun_SyncForkFailureAbortsBeforePR covering the new failure path. Asserts Run returns the underlying sync error, that CreatePR is never attempted afterward, and that outputs are written with status=FAILED so subsequent workflow steps see the failure. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
fantapop
force-pushed
the
autosolve-fork-sync
branch
from
c732013 to
bf6b9cf
Compare
fantapop
force-pushed
the
autosolve-bwrap-sandbox
branch
from
465aa67 to
967cc12
Compare
fantapop
force-pushed
the
autosolve-fork-sync
branch
3 times, most recently
from
1a3c0bb to
f0ba446
Compare
fantapop
force-pushed
the
autosolve-fork-sync
branch
from
f0ba446 to
4977036
Compare
Comment on lines
+223
to
+242
| // Sync the fork's base branch with the upstream we have checked | ||
| // out, so the upcoming branch push only transmits the bot's | ||
| // commit. Without this, a stale fork would force git to relay | ||
| // every upstream commit it lacks, and any of those that touched | ||
| // .github/workflows/ would trip GitHub's per-commit | ||
| // workflow-scope check on a PAT that only holds `repo`. | ||
| // | ||
| // The sync source is whatever the workflow checked out as | ||
| // origin (typically github.repository), not GitHub's tracked | ||
| // fork-parent metadata — this matters when the bot's fork was | ||
| // created from a different repo than the one the PR targets. | ||
| action.LogNotice(fmt.Sprintf("Syncing fork's %s with origin", cfg.PRBaseBranch)) | ||
| if err := gitClient.Fetch("origin", cfg.PRBaseBranch); err != nil { | ||
| _ = writeOutputs("FAILED", "", "", resultText, &tracker) | ||
| return fmt.Errorf("PR creation failed: fetching origin/%s: %w", cfg.PRBaseBranch, err) | ||
| } | ||
| if err := gitClient.Push("--force", "fork", "FETCH_HEAD:refs/heads/"+cfg.PRBaseBranch); err != nil { | ||
| _ = writeOutputs("FAILED", "", "", resultText, &tracker) | ||
| return fmt.Errorf("PR creation failed: syncing fork's %s: %w", cfg.PRBaseBranch, err) | ||
| } |
Comment on lines
+35
to
+37
| // one for fork operations (syncing the bot's fork with upstream, looking | ||
| // up branches on the fork). Keeping them separate makes the auth boundary | ||
| // visible at every call site. |
Comment on lines
+31
to
+32
| // (needs pull_requests:write); forkClient targets the bot's fork (needs | ||
| // contents:write to push branches and sync the fork's default branch). |
Push the local origin tracking ref directly to the fork's base branch right before pushing the feature branch: git push --force fork refs/remotes/origin/<base>:refs/heads/<base> Without this, a stale fork forces git to relay every upstream commit it lacks. actions/checkout already brought refs/remotes/origin/<base> down from the workflow's repo, so it's already current and a re-fetch would only require origin auth that the workflow may not have configured. The sync source is whatever the workflow checked out as origin (typically github.repository), not GitHub's tracked fork-parent metadata — this matters when the bot's fork was created from a different repo than the one the PR targets. Splits the single ghClient parameter on implement.Run into upstreamClient and forkClient so the auth boundary is visible at every call site. Routes the existing BranchExists check on the fork through forkClient as well, fixing a latent issue with private forks. TestRun_SyncForkFailureAbortsBeforePR covers the failure path: Run returns the underlying sync error, CreatePR is not attempted, and outputs are written with status=FAILED. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
fantapop
force-pushed
the
autosolve-fork-sync
branch
from
4977036 to
edfc825
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Push the local origin tracking ref directly to the fork's base branch right before pushing the feature branch:
Without this, a stale fork forces git to relay every upstream commit it lacks.
actions/checkoutalready broughtrefs/remotes/origin/<base>down from the workflow's repo, so it's already current and a re-fetch would only require origin auth that the workflow may not have configured. The sync source is whatever the workflow checked out asorigin(typicallygithub.repository), not GitHub's tracked fork-parent metadata — this matters when the bot's fork was created from a different repo than the one the PR targets.Splits the single
ghClientparameter onimplement.RunintoupstreamClientandforkClientso the auth boundary is visible at every call site. Routes the existingBranchExistscheck on the fork throughforkClientas well, fixing a latent issue with private forks.