test(#5207): extend scm.Driver with fork methods - #5213
Conversation
Add three fork-related methods to the scm.Driver interface and implement them in the GitHub v1 driver: - CreateFork: idempotent fork creation that reuses an existing fork via FindExistingFork before calling CreateFork on the forge client. - CommitFileToFork: commits a file to a branch on the fork by delegating to CreateOrUpdateFileOnBranch. - CreateForkChangeProposal: opens a cross-fork PR using forkOwner:headBranch as the head ref. Unit tests verify idempotent fork creation, correct cross-fork PR head format, file commit targeting, and error propagation for all three methods. Note: pre-commit could not run in the sandbox (exit 3, HTTP 403 on git fetch during hook init). The post-script runs pre-commit authoritatively on the runner. Closes #5207
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
Site previewPreview: https://071e98e4-site.fullsend-ai.workers.dev Commit: |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 5:26 PM UTC · Completed 5:35 PM UTC |
Review — approvePR: #5213 — SummaryClean, well-scoped PR that implements exactly what issue #5207 authorizes. The implementation correctly uses Dimensions reviewed
Findings[low · documentation-staleness] Suggested update: "Issues, comments, labels (via GetIssue), file commits, branches, change proposals, forks" Labels: PR extends behaviour test driver infrastructure with fork-related SCM methods |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
fullsend-ai-review
Bot
added
ready-for-merge
ifireball
left a comment
ifireball
left a comment
There was a problem hiding this comment.
/fs-fix the signature and functionality of CreateFork metohd are not fit for purpose, we need to create a fork within the same org as the repo we are forking from with a given name, the implemented function seems to create the fork within the calling user's account (impossible when called with an app token), and return the name of the created fork.
|
/fs-fix the signature and functionality of CreateFork metohd are not fit for purpose, we need to create a fork within the same org as the repo we are forking from with a given name, the implemented function seems to create the fork within the calling user's account (impossible when called with an app token), and return the name of the created fork. |
|
🤖 Finished Fix · ✅ Success · Started 5:54 PM UTC · Completed 6:06 PM UTC |
The scm.Driver.CreateFork method previously forked under the authenticated user's account (impossible with app tokens). Change the signature to accept a forkName and create the fork within the same org as the source repository via a new forge.Client.CreateForkInOrg method.
🔧 Fix agent — iteration 1 (human-triggered)Changed scm.Driver.CreateFork to create forks within the same org as the source repo with a caller-specified name, instead of forking under the authenticated user's account. Added forge.Client.CreateForkInOrg to the interface with implementations in FakeClient and LiveClient. The new method passes 'organization' and 'name' fields to the GitHub fork API. Updated tests to verify the new signature and delegation. Fixed (1):
Tests: passed Decision points
Updated by fullsend fix agent |
|
/fs-fix what happens if the named for repo exists but is not a fork of the original repo, do we detect that and raise an error accordingly? |
|
🤖 Finished Fix · ✅ Success · Started 6:16 PM UTC · Completed 6:29 PM UTC |
When CreateForkInOrg targets a name that already exists but is not a
fork of the source repository, return forge.ErrNotFork so callers can
handle the collision gracefully instead of receiving a generic 422.
The GitHub implementation pre-checks with GET /repos/{org}/{name}
before calling the fork API. The fake client mirrors this by checking
its Repos slice. Three new test cases cover: non-fork repo, fork of
a different source, and idempotent fork of the correct source.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔧 Fix agent — iteration 2 (human-triggered)Added non-fork name collision detection to CreateForkInOrg. When a repository with the requested fork name already exists but is not a fork of the source repository, CreateForkInOrg now returns forge.ErrNotFork instead of proceeding to a generic GitHub 422 error. The GitHub implementation pre-checks the target repo with GET /repos/{org}/{name} before calling the fork API. Added ErrNotFork sentinel error and IsNotFork helper to forge.go, updated the FakeClient to mirror this behavior using its Repos slice and a new ForkParents map, and added three test cases covering: non-fork repo collision, fork of a different source, and idempotent fork of the correct source. Fixed (1):
Tests: passed Decision points
Updated by fullsend fix agent |
|
/fs-fix please get the coverage for the PR to 80% or above |
|
🤖 Finished Fix · ✅ Success · Started 6:52 PM UTC · Completed 7:03 PM UTC |
Add LiveClient httptest-based tests for CreateForkInOrg covering: - successful fork creation when no existing repo - ErrNotFork when existing non-fork repo has same name - ErrNotFork when existing fork has different parent - idempotent return for existing fork of correct source - fallthrough to fork creation on non-200 pre-check - API error during fork creation Add FakeClient tests for CreateForkInOrg and IsNotFork sentinel error to bring patch coverage above 80%.
🔧 Fix agent — iteration 3 (human-triggered)Added 203 lines of test code across 2 files to cover the CreateForkInOrg LiveClient implementation, the ErrNotFork sentinel error, and the FakeClient CreateForkInOrg method. The LiveClient tests use httptest servers to exercise all 6 code paths in CreateForkInOrg (pre-check non-fork collision, pre-check wrong-parent fork, pre-check correct fork idempotent return, pre-check non-200 fallthrough, successful creation, and API error). The FakeClient tests verify the same scenarios using the test double. All tests pass. Fixed (3):
Tests: passed Updated by fullsend fix agent |
|
🤖 Finished Retro · ✅ Success · Started 7:21 PM UTC · Completed 7:31 PM UTC |
|
PR #5213 added three fork-related methods to the scm.Driver interface. The code agent's initial implementation delegated to
Proposals filed
|
1 participant
Add three fork-related methods to the scm.Driver interface and implement them in the GitHub v1 driver:
existing fork via FindExistingFork before calling
CreateFork on the forge client.
fork by delegating to CreateOrUpdateFileOnBranch.
forkOwner:headBranch as the head ref.
Unit tests verify idempotent fork creation, correct cross-fork PR head format, file commit targeting, and error propagation for all three methods.
Note: pre-commit could not run in the sandbox (exit 3, HTTP 403 on git fetch during hook init). The post-script runs pre-commit authoritatively on the runner.
Closes #5207
Post-script verification
agent/5207-scm-driver-fork-methods)83397e5355462dea996845ed2a1ec019d32f30ac..HEAD)