Skip to content

Commit 08e7c62

Browse files
waynesun09waynesun09
authored
Merge pull request #2594 from fullsend-ai/agent/2592-fork-scaffold-pr
fix(#2592): default to fork-based scaffold PR for non-owner users
2 parents 02977ab + 60744d6 commit 08e7c62

12 files changed

Lines changed: 986 additions & 23 deletions

File tree

‎internal/cli/admin.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1043,7 +1043,7 @@ func applyPerRepoScaffold(ctx context.Context, client forge.Client, printer *ui.
10431043
"Merge this PR to activate fullsend workflows."
10441044
if _, err := layers.CommitScaffoldFiles(ctx, client, printer,
10451045
owner, repo, targetRepo.DefaultBranch,
1046-
commitMsg, "chore: initialize fullsend per-repo installation", prBody, files, direct); err != nil {
1046+
commitMsg, "chore: initialize fullsend per-repo installation", prBody, files, direct, os.Stdin); err != nil {
10471047
return err
10481048
}
10491049

‎internal/cli/admin_test.go‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1806,7 +1806,7 @@ func TestRunAnalyze_WithFakeClient(t *testing.T) {
18061806

18071807
func TestRunInstall_RequiresAgentCredsWhenMintEnabled(t *testing.T) {
18081808
client := forge.NewFakeClient()
1809-
client.AuthenticatedUser = "testuser"
1809+
client.AuthenticatedUser = "testorg"
18101810
discovered := []forge.Repository{
18111811
{Name: forge.ConfigRepoName, FullName: "testorg/" + forge.ConfigRepoName},
18121812
}
@@ -1829,7 +1829,7 @@ func TestRunInstall_RequiresAgentCredsWhenMintEnabled(t *testing.T) {
18291829
func TestRunInstall_WithSkipMintCheck(t *testing.T) {
18301830
cfg := setupTestConfig(map[string]bool{"myrepo": false})
18311831
client := setupTestClient("testorg", cfg, []string{"myrepo"})
1832-
client.AuthenticatedUser = "testuser"
1832+
client.AuthenticatedUser = "testorg"
18331833

18341834
var agentCreds []layers.AgentCredentials
18351835
for _, role := range config.DefaultAgentRoles() {
@@ -1854,7 +1854,7 @@ func TestRunInstall_WithSkipMintCheck(t *testing.T) {
18541854
func TestRunInstall_DiscoversRepos(t *testing.T) {
18551855
cfg := setupTestConfig(map[string]bool{"myrepo": false})
18561856
client := setupTestClient("testorg", cfg, []string{"myrepo"})
1857-
client.AuthenticatedUser = "testuser"
1857+
client.AuthenticatedUser = "testorg"
18581858

18591859
var agentCreds []layers.AgentCredentials
18601860
for _, role := range config.DefaultAgentRoles() {
@@ -1880,7 +1880,7 @@ func TestRunInstall_DiscoversRepos(t *testing.T) {
18801880

18811881
func TestRunInstall_InvalidEnabledRepo(t *testing.T) {
18821882
client := forge.NewFakeClient()
1883-
client.AuthenticatedUser = "testuser"
1883+
client.AuthenticatedUser = "testorg"
18841884
discovered := []forge.Repository{
18851885
{Name: "myrepo", FullName: "testorg/myrepo"},
18861886
}
@@ -1902,7 +1902,7 @@ func TestRunInstall_InvalidEnabledRepo(t *testing.T) {
19021902
func TestRunInstall_WithVendorAndSkipMint(t *testing.T) {
19031903
cfg := setupTestConfig(map[string]bool{"myrepo": false})
19041904
client := setupTestClient("testorg", cfg, []string{"myrepo"})
1905-
client.AuthenticatedUser = "testuser"
1905+
client.AuthenticatedUser = "testorg"
19061906

19071907
var agentCreds []layers.AgentCredentials
19081908
for _, role := range config.DefaultAgentRoles() {
@@ -2317,6 +2317,7 @@ func TestInstallCmd_SkipMintCheckStillValidatesWIFProvider(t *testing.T) {
23172317

23182318
func TestApplyPerRepoScaffold(t *testing.T) {
23192319
client := forge.NewFakeClient()
2320+
client.AuthenticatedUser = "acme"
23202321
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
23212322
printer := ui.New(&bytes.Buffer{})
23222323

@@ -2363,6 +2364,7 @@ func TestApplyPerRepoScaffold(t *testing.T) {
23632364

23642365
func TestApplyPerRepoScaffold_GetRepoError(t *testing.T) {
23652366
client := forge.NewFakeClient()
2367+
client.AuthenticatedUser = "acme"
23662368
client.Errors["GetRepo"] = errors.New("not found")
23672369
printer := ui.New(&bytes.Buffer{})
23682370

@@ -2374,6 +2376,7 @@ func TestApplyPerRepoScaffold_GetRepoError(t *testing.T) {
23742376

23752377
func TestApplyPerRepoScaffold_CommitFilesError(t *testing.T) {
23762378
client := forge.NewFakeClient()
2379+
client.AuthenticatedUser = "acme"
23772380
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
23782381
client.Errors["CommitFiles"] = errors.New("permission denied")
23792382
printer := ui.New(&bytes.Buffer{})
@@ -2392,6 +2395,7 @@ func TestApplyPerRepoScaffold_CommitFilesError(t *testing.T) {
23922395

23932396
func TestApplyPerRepoScaffold_Idempotent(t *testing.T) {
23942397
client := forge.NewFakeClient()
2398+
client.AuthenticatedUser = "acme"
23952399
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
23962400
noChange := false
23972401
client.CommitFilesChanged = &noChange
@@ -2415,6 +2419,7 @@ func TestApplyPerRepoScaffold_Idempotent(t *testing.T) {
24152419

24162420
func TestApplyPerRepoScaffold_DefaultPR_NoChanges(t *testing.T) {
24172421
client := forge.NewFakeClient()
2422+
client.AuthenticatedUser = "acme"
24182423
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
24192424
client.Errors = map[string]error{
24202425
"CreateChangeProposal": fmt.Errorf("PR: %w", forge.ErrNoChanges),
@@ -2434,6 +2439,7 @@ func TestApplyPerRepoScaffold_DefaultPR_NoChanges(t *testing.T) {
24342439

24352440
func TestApplyPerRepoScaffold_NonMainBranch(t *testing.T) {
24362441
client := forge.NewFakeClient()
2442+
client.AuthenticatedUser = "acme"
24372443
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "develop"}}
24382444
var buf bytes.Buffer
24392445
printer := ui.New(&buf)
@@ -2451,6 +2457,7 @@ func TestApplyPerRepoScaffold_NonMainBranch(t *testing.T) {
24512457

24522458
func TestApplyPerRepoScaffold_CreateVariableError(t *testing.T) {
24532459
client := forge.NewFakeClient()
2460+
client.AuthenticatedUser = "acme"
24542461
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
24552462
client.Errors = map[string]error{
24562463
"CreateOrUpdateRepoVariable": errors.New("rate limited"),
@@ -2465,6 +2472,7 @@ func TestApplyPerRepoScaffold_CreateVariableError(t *testing.T) {
24652472

24662473
func TestApplyPerRepoScaffold_CreateSecretError(t *testing.T) {
24672474
client := forge.NewFakeClient()
2475+
client.AuthenticatedUser = "acme"
24682476
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
24692477
client.Errors = map[string]error{
24702478
"CreateRepoSecret": errors.New("forbidden"),
@@ -2479,6 +2487,7 @@ func TestApplyPerRepoScaffold_CreateSecretError(t *testing.T) {
24792487

24802488
func TestApplyPerRepoScaffold_ProtectedBranchFallback(t *testing.T) {
24812489
client := forge.NewFakeClient()
2490+
client.AuthenticatedUser = "acme"
24822491
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
24832492
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
24842493
var buf bytes.Buffer
@@ -2512,6 +2521,7 @@ func TestApplyPerRepoScaffold_ProtectedBranchFallback(t *testing.T) {
25122521

25132522
func TestApplyPerRepoScaffold_ProtectedBranch_ExistingBranch(t *testing.T) {
25142523
client := forge.NewFakeClient()
2524+
client.AuthenticatedUser = "acme"
25152525
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
25162526
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
25172527
client.Errors["CreateBranch"] = fmt.Errorf("branch: %w", forge.ErrAlreadyExists)
@@ -2532,6 +2542,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_ExistingBranch(t *testing.T) {
25322542

25332543
func TestApplyPerRepoScaffold_ProtectedBranch_StillSetsVarsAndSecrets(t *testing.T) {
25342544
client := forge.NewFakeClient()
2545+
client.AuthenticatedUser = "acme"
25352546
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
25362547
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
25372548
printer := ui.New(&bytes.Buffer{})
@@ -2552,6 +2563,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_StillSetsVarsAndSecrets(t *testing
25522563

25532564
func TestApplyPerRepoScaffold_ProtectedBranch_CreateBranchFails(t *testing.T) {
25542565
client := forge.NewFakeClient()
2566+
client.AuthenticatedUser = "acme"
25552567
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
25562568
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
25572569
client.Errors["CreateBranch"] = fmt.Errorf("forbidden")
@@ -2569,6 +2581,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_CreateBranchFails(t *testing.T) {
25692581

25702582
func TestApplyPerRepoScaffold_ProtectedBranch_CommitToBranchFails(t *testing.T) {
25712583
client := forge.NewFakeClient()
2584+
client.AuthenticatedUser = "acme"
25722585
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
25732586
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
25742587
client.Errors["CommitFilesToBranch"] = fmt.Errorf("server error")
@@ -2586,6 +2599,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_CommitToBranchFails(t *testing.T)
25862599

25872600
func TestApplyPerRepoScaffold_ProtectedBranch_ScaffoldBranchAlsoProtected(t *testing.T) {
25882601
client := forge.NewFakeClient()
2602+
client.AuthenticatedUser = "acme"
25892603
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
25902604
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
25912605
client.Errors["CommitFilesToBranch"] = fmt.Errorf("%w: scaffold branch also protected", forge.ErrBranchProtected)
@@ -2604,6 +2618,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_ScaffoldBranchAlsoProtected(t *tes
26042618

26052619
func TestApplyPerRepoScaffold_ProtectedBranch_CreatePRFails(t *testing.T) {
26062620
client := forge.NewFakeClient()
2621+
client.AuthenticatedUser = "acme"
26072622
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
26082623
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
26092624
client.Errors["CreateChangeProposal"] = fmt.Errorf("forbidden")
@@ -2621,6 +2636,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_CreatePRFails(t *testing.T) {
26212636

26222637
func TestApplyPerRepoScaffold_ProtectedBranch_DuplicatePR(t *testing.T) {
26232638
client := forge.NewFakeClient()
2639+
client.AuthenticatedUser = "acme"
26242640
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
26252641
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
26262642
client.Errors["CreateChangeProposal"] = fmt.Errorf("pr: %w", forge.ErrAlreadyExists)
@@ -2752,6 +2768,7 @@ func TestLoadKnownSlugs_HardError_ReturnsNil(t *testing.T) {
27522768

27532769
func TestApplyPerRepoScaffold_ProtectedBranch_BranchUpToDate(t *testing.T) {
27542770
client := forge.NewFakeClient()
2771+
client.AuthenticatedUser = "acme"
27552772
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
27562773
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
27572774
client.Errors["CreateChangeProposal"] = fmt.Errorf("PR: %w", forge.ErrAlreadyExists)

‎internal/cli/github_test.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -626,6 +626,7 @@ func TestParseTarget_Repo(t *testing.T) {
626626
func TestRunGitHubSetupPerRepo(t *testing.T) {
627627
t.Setenv("GH_TOKEN", "test-token")
628628
client := forge.NewFakeClient()
629+
client.AuthenticatedUser = "acme"
629630
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
630631
client.TokenScopes = []string{"repo", "workflow"}
631632
printer := ui.New(&discardWriter{})
@@ -754,6 +755,7 @@ func TestRunGitHubSetupPerRepo_DryRun(t *testing.T) {
754755
func TestRunGitHubSetupPerRepo_ReusesExistingSecrets(t *testing.T) {
755756
t.Setenv("GH_TOKEN", "test-token")
756757
client := forge.NewFakeClient()
758+
client.AuthenticatedUser = "acme"
757759
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
758760
client.TokenScopes = []string{"repo", "workflow"}
759761
// Pre-populate secrets as if a previous run stored them.
@@ -791,6 +793,7 @@ func TestRunGitHubSetupPerRepo_ReusesExistingSecrets(t *testing.T) {
791793
func TestRunGitHubSetupPerRepo_PartialReuse_ProjectOnly(t *testing.T) {
792794
t.Setenv("GH_TOKEN", "test-token")
793795
client := forge.NewFakeClient()
796+
client.AuthenticatedUser = "acme"
794797
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
795798
client.TokenScopes = []string{"repo", "workflow"}
796799
// Only the project secret exists; WIF is provided via flag.
@@ -819,6 +822,7 @@ func TestRunGitHubSetupPerRepo_PartialReuse_ProjectOnly(t *testing.T) {
819822

820823
func TestRunGitHubSetupPerRepo_MissingFlagNoExistingSecret(t *testing.T) {
821824
client := forge.NewFakeClient()
825+
client.AuthenticatedUser = "acme"
822826
printer := ui.New(&discardWriter{})
823827

824828
// No existing secrets and no flags — should fail.
@@ -834,6 +838,7 @@ func TestRunGitHubSetupPerRepo_MissingFlagNoExistingSecret(t *testing.T) {
834838

835839
func TestRunGitHubSetupPerRepo_MissingWIFNoExistingSecret(t *testing.T) {
836840
client := forge.NewFakeClient()
841+
client.AuthenticatedUser = "acme"
837842
printer := ui.New(&discardWriter{})
838843

839844
// Project flag provided, WIF missing with no existing secret.
@@ -851,6 +856,7 @@ func TestRunGitHubSetupPerRepo_MissingWIFNoExistingSecret(t *testing.T) {
851856
func TestRunGitHubSetupPerRepo_PartialReuse_WIFOnly(t *testing.T) {
852857
t.Setenv("GH_TOKEN", "test-token")
853858
client := forge.NewFakeClient()
859+
client.AuthenticatedUser = "acme"
854860
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
855861
client.TokenScopes = []string{"repo", "workflow"}
856862
// Only the WIF secret exists; project is provided via flag.
@@ -879,6 +885,7 @@ func TestRunGitHubSetupPerRepo_PartialReuse_WIFOnly(t *testing.T) {
879885

880886
func TestRunGitHubSetupPerRepo_SecretCheckError(t *testing.T) {
881887
client := forge.NewFakeClient()
888+
client.AuthenticatedUser = "acme"
882889
client.Errors = map[string]error{
883890
"RepoSecretExists": fmt.Errorf("API rate limit exceeded"),
884891
}

‎internal/forge/fake.go‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,15 @@ type FakeClient struct {
122122
VariablesExist map[string]bool // key: "owner/repo/name"
123123
VariableValues map[string]string // key: "owner/repo/name"
124124

125+
// ForkOwner controls the return value of CreateFork. When non-empty,
126+
// CreateFork returns this value as the fork owner login. When empty,
127+
// CreateFork uses AuthenticatedUser.
128+
ForkOwner string
129+
130+
// ExistingForks maps "owner/repo" to the fork owner login returned
131+
// by FindExistingFork. Entries simulate an already-existing fork.
132+
ExistingForks map[string]string
133+
125134
// App client IDs for GetAppClientID
126135
AppClientIDs map[string]string // key: app slug → client ID
127136

@@ -143,6 +152,10 @@ type FakeClient struct {
143152
// Error injection: key is method name, value is error to return.
144153
Errors map[string]error
145154

155+
// CreateBranchErrors injects per-repo errors for CreateBranch.
156+
// Key is "owner/repo", checked before the generic Errors map.
157+
CreateBranchErrors map[string]error
158+
146159
// Issue comments for ListIssueComments / UpdateIssueComment.
147160
IssueComments map[string][]IssueComment // key: "owner/repo/number"
148161
OpenIssues map[string][]Issue // key: "owner/repo"
@@ -188,7 +201,8 @@ type FakeClient struct {
188201
DismissedReviews []DismissedReviewRecord
189202
CommittedFiles []CommitFilesRecord
190203
CommittedFilesToBranch []CommitFilesToBranchRecord
191-
DeletedComments []int // comment IDs
204+
CreatedForks []string // "owner/repo"
205+
DeletedComments []int // comment IDs
192206

193207
// internal counters
194208
proposalCounter int
@@ -317,6 +331,38 @@ func (f *FakeClient) DeleteRepo(_ context.Context, owner, repo string) error {
317331
return nil
318332
}
319333

334+
func (f *FakeClient) FindExistingFork(_ context.Context, owner, repo string) (string, string, error) {
335+
f.mu.Lock()
336+
defer f.mu.Unlock()
337+
338+
if e := f.err("FindExistingFork"); e != nil {
339+
return "", "", e
340+
}
341+
342+
if f.ExistingForks != nil {
343+
if forkOwner, ok := f.ExistingForks[owner+"/"+repo]; ok {
344+
return forkOwner, repo, nil
345+
}
346+
}
347+
return "", "", nil
348+
}
349+
350+
func (f *FakeClient) CreateFork(_ context.Context, owner, repo string) (string, string, error) {
351+
f.mu.Lock()
352+
defer f.mu.Unlock()
353+
354+
if e := f.err("CreateFork"); e != nil {
355+
return "", "", e
356+
}
357+
358+
f.CreatedForks = append(f.CreatedForks, owner+"/"+repo)
359+
360+
if f.ForkOwner != "" {
361+
return f.ForkOwner, repo, nil
362+
}
363+
return f.AuthenticatedUser, repo, nil
364+
}
365+
320366
func (f *FakeClient) CreateFile(_ context.Context, owner, repo, path, message string, content []byte) error {
321367
f.mu.Lock()
322368
defer f.mu.Unlock()
@@ -517,6 +563,11 @@ func (f *FakeClient) CreateBranch(_ context.Context, owner, repo, branchName str
517563
f.mu.Lock()
518564
defer f.mu.Unlock()
519565

566+
if f.CreateBranchErrors != nil {
567+
if e, ok := f.CreateBranchErrors[owner+"/"+repo]; ok {
568+
return e
569+
}
570+
}
520571
if e := f.err("CreateBranch"); e != nil {
521572
return e
522573
}
@@ -581,6 +632,8 @@ func (f *FakeClient) CreateChangeProposal(_ context.Context, owner, repo, title,
581632
URL: fmt.Sprintf("https://forge.example.com/%s/%s/pull/%d", owner, repo, f.proposalCounter),
582633
Title: title,
583634
Number: f.proposalCounter,
635+
Head: head,
636+
Base: base,
584637
}
585638
f.CreatedProposals = append(f.CreatedProposals, cp)
586639
return &cp, nil

0 commit comments

Comments
 (0)