Skip to content

Commit 9be2798

Browse files
committed
fix(cli): fall back to PR when default branch is protected
When `fullsend github setup owner/repo` or `fullsend admin install owner/repo` encounters a protected default branch (422 on ref update), the CLI now falls back to creating a feature branch and opening a PR with the scaffold files instead of failing. Variables and secrets are set regardless of which path is taken. - Add ErrBranchProtected sentinel and CommitFilesToBranch to forge.Client - Refactor CommitFiles to detect 422 as branch protection failure - Add PR-based fallback in applyPerRepoScaffold Signed-off-by: Wayne Sun <gsun@redhat.com>
1 parent fd5b754 commit 9be2798

5 files changed

Lines changed: 194 additions & 19 deletions

File tree

internal/cli/admin.go

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,15 +1023,44 @@ func applyPerRepoScaffold(ctx context.Context, client forge.Client, printer *ui.
10231023
if err != nil {
10241024
return fmt.Errorf("getting repo info: %w", err)
10251025
}
1026+
commitMsg := fmt.Sprintf("chore: initialize fullsend-%s per-repo installation", version)
10261027
printer.StepStart(fmt.Sprintf("Committing scaffold files to %s/%s (%s branch)",
10271028
owner, repo, targetRepo.DefaultBranch))
1028-
committed, err := client.CommitFiles(ctx, owner, repo,
1029-
fmt.Sprintf("chore: initialize fullsend-%s per-repo installation", version), files)
1030-
if err != nil {
1029+
committed, err := client.CommitFiles(ctx, owner, repo, commitMsg, files)
1030+
if err != nil && forge.IsBranchProtected(err) {
1031+
printer.StepWarn("Default branch is protected — creating scaffold PR instead")
1032+
1033+
const scaffoldBranch = "fullsend/scaffold-install"
1034+
if branchErr := client.CreateBranch(ctx, owner, repo, scaffoldBranch); branchErr != nil {
1035+
if !strings.Contains(branchErr.Error(), "already exists") {
1036+
printer.StepFail("Failed to create scaffold branch")
1037+
return fmt.Errorf("creating scaffold branch: %w", branchErr)
1038+
}
1039+
}
1040+
1041+
if _, commitErr := client.CommitFilesToBranch(ctx, owner, repo, scaffoldBranch, commitMsg, files); commitErr != nil {
1042+
printer.StepFail("Failed to commit scaffold files to branch")
1043+
return fmt.Errorf("committing scaffold files to branch: %w", commitErr)
1044+
}
1045+
1046+
prBody := fmt.Sprintf("This PR adds the fullsend scaffold files for per-repo installation.\n\n"+
1047+
"The default branch (%s) has branch protection rules that prevent direct pushes, "+
1048+
"so these files are delivered via PR instead.\n\n"+
1049+
"Merge this PR to activate fullsend workflows.", targetRepo.DefaultBranch)
1050+
proposal, prErr := client.CreateChangeProposal(ctx, owner, repo,
1051+
"chore: initialize fullsend per-repo installation", prBody,
1052+
scaffoldBranch, targetRepo.DefaultBranch)
1053+
if prErr != nil {
1054+
printer.StepFail("Failed to create scaffold PR")
1055+
return fmt.Errorf("creating scaffold PR: %w", prErr)
1056+
}
1057+
1058+
printer.StepDone(fmt.Sprintf("Created PR #%d: %s", proposal.Number, proposal.URL))
1059+
printer.StepInfo("Merge the PR to activate fullsend workflows")
1060+
} else if err != nil {
10311061
printer.StepFail("Failed to commit scaffold files")
10321062
return fmt.Errorf("committing scaffold files: %w", err)
1033-
}
1034-
if committed {
1063+
} else if committed {
10351064
noun := "files"
10361065
if len(files) == 1 {
10371066
noun = "file"

internal/cli/admin_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2046,3 +2046,76 @@ func TestApplyPerRepoScaffold_CreateSecretError(t *testing.T) {
20462046
require.Error(t, err)
20472047
assert.Contains(t, err.Error(), "setting repo secret")
20482048
}
2049+
2050+
func TestApplyPerRepoScaffold_ProtectedBranchFallback(t *testing.T) {
2051+
client := forge.NewFakeClient()
2052+
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
2053+
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
2054+
var buf bytes.Buffer
2055+
printer := ui.New(&buf)
2056+
2057+
files := []forge.TreeFile{
2058+
{Path: ".github/workflows/fullsend.yaml", Content: []byte("workflow"), Mode: "100644"},
2059+
}
2060+
repoVars := map[string]string{"K": "V"}
2061+
repoSecrets := map[string]string{"S": "secret"}
2062+
2063+
err := applyPerRepoScaffold(context.Background(), client, printer,
2064+
"acme", "widget", files, repoVars, repoSecrets)
2065+
require.NoError(t, err)
2066+
2067+
require.Len(t, client.CreatedBranches, 1)
2068+
assert.Equal(t, "acme/widget/fullsend/scaffold-install", client.CreatedBranches[0])
2069+
2070+
require.Len(t, client.CommittedFilesToBranch, 1)
2071+
assert.Equal(t, "fullsend/scaffold-install", client.CommittedFilesToBranch[0].Branch)
2072+
assert.Len(t, client.CommittedFilesToBranch[0].Files, 1)
2073+
2074+
require.Len(t, client.CreatedProposals, 1)
2075+
assert.Contains(t, client.CreatedProposals[0].Title, "fullsend")
2076+
2077+
output := buf.String()
2078+
assert.Contains(t, output, "protected")
2079+
assert.Contains(t, output, "PR #1")
2080+
assert.Contains(t, output, "Merge the PR")
2081+
}
2082+
2083+
func TestApplyPerRepoScaffold_ProtectedBranch_ExistingBranch(t *testing.T) {
2084+
client := forge.NewFakeClient()
2085+
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
2086+
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
2087+
client.Errors["CreateBranch"] = fmt.Errorf("already exists")
2088+
var buf bytes.Buffer
2089+
printer := ui.New(&buf)
2090+
2091+
files := []forge.TreeFile{
2092+
{Path: ".fullsend/config.yaml", Content: []byte("cfg"), Mode: "100644"},
2093+
}
2094+
2095+
err := applyPerRepoScaffold(context.Background(), client, printer,
2096+
"acme", "widget", files, nil, nil)
2097+
require.NoError(t, err)
2098+
2099+
require.Len(t, client.CommittedFilesToBranch, 1, "should proceed despite branch existing")
2100+
require.Len(t, client.CreatedProposals, 1)
2101+
}
2102+
2103+
func TestApplyPerRepoScaffold_ProtectedBranch_StillSetsVarsAndSecrets(t *testing.T) {
2104+
client := forge.NewFakeClient()
2105+
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
2106+
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
2107+
printer := ui.New(&bytes.Buffer{})
2108+
2109+
files := []forge.TreeFile{
2110+
{Path: ".fullsend/config.yaml", Content: []byte("cfg"), Mode: "100644"},
2111+
}
2112+
repoVars := map[string]string{"FULLSEND_MINT_URL": "https://mint.example.run.app"}
2113+
repoSecrets := map[string]string{"FULLSEND_GCP_PROJECT_ID": "my-project"}
2114+
2115+
err := applyPerRepoScaffold(context.Background(), client, printer,
2116+
"acme", "widget", files, repoVars, repoSecrets)
2117+
require.NoError(t, err)
2118+
2119+
assert.Len(t, client.Variables, 1, "variables should be set even with PR fallback")
2120+
assert.Len(t, client.CreatedSecrets, 1, "secrets should be set even with PR fallback")
2121+
}

internal/forge/fake.go

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,12 @@ type CommitFilesRecord struct {
9595
Files []TreeFile
9696
}
9797

98+
// CommitFilesToBranchRecord records a CommitFilesToBranch call.
99+
type CommitFilesToBranchRecord struct {
100+
Owner, Repo, Branch, Message string
101+
Files []TreeFile
102+
}
103+
98104
// FakeClient is a thread-safe test double for forge.Client.
99105
// Pre-populate its fields to control return values, and inspect
100106
// recorder slices after the test to verify which calls were made.
@@ -175,8 +181,9 @@ type FakeClient struct {
175181
MinimizedComments []MinimizedCommentRecord
176182
CreatedReviews []ReviewRecord
177183
DismissedReviews []DismissedReviewRecord
178-
CommittedFiles []CommitFilesRecord
179-
DeletedComments []int // comment IDs
184+
CommittedFiles []CommitFilesRecord
185+
CommittedFilesToBranch []CommitFilesToBranchRecord
186+
DeletedComments []int // comment IDs
180187

181188
// internal counters
182189
proposalCounter int
@@ -448,6 +455,33 @@ func (f *FakeClient) CommitFiles(_ context.Context, owner, repo, message string,
448455
return changed, nil
449456
}
450457

458+
func (f *FakeClient) CommitFilesToBranch(_ context.Context, owner, repo, branch, message string, files []TreeFile) (bool, error) {
459+
f.mu.Lock()
460+
defer f.mu.Unlock()
461+
462+
if e := f.err("CommitFilesToBranch"); e != nil {
463+
return false, e
464+
}
465+
466+
f.CommittedFilesToBranch = append(f.CommittedFilesToBranch, CommitFilesToBranchRecord{
467+
Owner: owner,
468+
Repo: repo,
469+
Branch: branch,
470+
Message: message,
471+
Files: files,
472+
})
473+
474+
if f.FileContents == nil {
475+
f.FileContents = make(map[string][]byte)
476+
}
477+
for _, file := range files {
478+
f.FileContents[owner+"/"+repo+"/"+file.Path] = file.Content
479+
}
480+
481+
changed := f.CommitFilesChanged == nil || *f.CommitFilesChanged
482+
return changed, nil
483+
}
484+
451485
func (f *FakeClient) CreateBranch(_ context.Context, owner, repo, branchName string) error {
452486
f.mu.Lock()
453487
defer f.mu.Unlock()

internal/forge/forge.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,15 @@ func IsNotFound(err error) bool {
2424
return errors.Is(err, ErrNotFound)
2525
}
2626

27+
// ErrBranchProtected indicates that a ref update failed because the
28+
// target branch has protection rules that prevent direct pushes.
29+
var ErrBranchProtected = errors.New("branch is protected")
30+
31+
// IsBranchProtected reports whether err indicates a branch protection failure.
32+
func IsBranchProtected(err error) bool {
33+
return errors.Is(err, ErrBranchProtected)
34+
}
35+
2736
// Repository represents a repository on a git forge.
2837
type Repository struct {
2938
ID int64
@@ -186,6 +195,11 @@ type Client interface {
186195
// and it returns (false, nil).
187196
CommitFiles(ctx context.Context, owner, repo, message string, files []TreeFile) (committed bool, err error)
188197

198+
// CommitFilesToBranch atomically commits multiple files to a specific
199+
// branch. Like CommitFiles, it is idempotent: if all files already
200+
// have the expected content, no commit is created.
201+
CommitFilesToBranch(ctx context.Context, owner, repo, branch, message string, files []TreeFile) (committed bool, err error)
202+
189203
// Branch operations
190204
CreateBranch(ctx context.Context, owner, repo, branchName string) error
191205
CreateFileOnBranch(ctx context.Context, owner, repo, branch, path, message string, content []byte) error

internal/forge/github/github.go

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -599,12 +599,15 @@ func isTransientStatus(code int) bool {
599599
// CommitFiles atomically commits multiple files to the default branch
600600
// using the Git Trees/Blobs/Commits API. Returns (false, nil) when
601601
// all files already match the current tree (idempotent).
602+
//
603+
// Returns forge.ErrBranchProtected (wrapped) when the ref update fails
604+
// with a 422, which indicates branch protection rules prevent direct pushes.
602605
func (c *LiveClient) CommitFiles(ctx context.Context, owner, repo, message string, files []forge.TreeFile) (bool, error) {
603606
if len(files) == 0 {
604607
return false, nil
605608
}
606609

607-
// 1. Get default branch name.
610+
// Get default branch name.
608611
repoResp, err := c.get(ctx, fmt.Sprintf("/repos/%s/%s", owner, repo))
609612
if err != nil {
610613
return false, fmt.Errorf("get repo: %w", err)
@@ -616,12 +619,34 @@ func (c *LiveClient) CommitFiles(ctx context.Context, owner, repo, message strin
616619
return false, fmt.Errorf("decode repo info: %w", err)
617620
}
618621

619-
// 2. Get current commit SHA from the branch ref.
620-
// Wrapped in retryOnTransient for freshly-created repos where the
621-
// branch ref may not be materialized yet (async auto_init).
622+
committed, err := c.commitFilesTo(ctx, owner, repo, repoInfo.DefaultBranch, message, files)
623+
if err != nil {
624+
var apiErr *APIError
625+
if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusUnprocessableEntity {
626+
return false, fmt.Errorf("%w: %w", forge.ErrBranchProtected, err)
627+
}
628+
return false, err
629+
}
630+
return committed, nil
631+
}
632+
633+
// CommitFilesToBranch atomically commits multiple files to a specific
634+
// branch. Like CommitFiles, it is idempotent.
635+
func (c *LiveClient) CommitFilesToBranch(ctx context.Context, owner, repo, branch, message string, files []forge.TreeFile) (bool, error) {
636+
if len(files) == 0 {
637+
return false, nil
638+
}
639+
return c.commitFilesTo(ctx, owner, repo, branch, message, files)
640+
}
641+
642+
// commitFilesTo is the shared implementation for CommitFiles and
643+
// CommitFilesToBranch. It commits files to the specified branch using
644+
// the Git Trees/Blobs/Commits API.
645+
func (c *LiveClient) commitFilesTo(ctx context.Context, owner, repo, branch, message string, files []forge.TreeFile) (bool, error) {
646+
// 1. Get current commit SHA from the branch ref.
622647
var commitSHA string
623648
if err := c.retryOnTransient(ctx, "get branch ref", func() error {
624-
refResp, refErr := c.get(ctx, fmt.Sprintf("/repos/%s/%s/git/ref/heads/%s", owner, repo, repoInfo.DefaultBranch))
649+
refResp, refErr := c.get(ctx, fmt.Sprintf("/repos/%s/%s/git/ref/heads/%s", owner, repo, branch))
625650
if refErr != nil {
626651
return fmt.Errorf("get branch ref: %w", refErr)
627652
}
@@ -639,7 +664,7 @@ func (c *LiveClient) CommitFiles(ctx context.Context, owner, repo, message strin
639664
return false, err
640665
}
641666

642-
// 3. Get the current commit to find its tree SHA.
667+
// 2. Get the current commit to find its tree SHA.
643668
cResp, err := c.get(ctx, fmt.Sprintf("/repos/%s/%s/git/commits/%s", owner, repo, commitSHA))
644669
if err != nil {
645670
return false, fmt.Errorf("get commit: %w", err)
@@ -654,7 +679,7 @@ func (c *LiveClient) CommitFiles(ctx context.Context, owner, repo, message strin
654679
}
655680
baseTreeSHA := commitObj.Tree.SHA
656681

657-
// 4. Get the full recursive tree to compare existing blobs.
682+
// 3. Get the full recursive tree to compare existing blobs.
658683
treeResp, err := c.get(ctx, fmt.Sprintf("/repos/%s/%s/git/trees/%s?recursive=1", owner, repo, baseTreeSHA))
659684
if err != nil {
660685
return false, fmt.Errorf("get tree: %w", err)
@@ -683,7 +708,7 @@ func (c *LiveClient) CommitFiles(ctx context.Context, owner, repo, message strin
683708
existing[entry.Path] = blobInfo{sha: entry.SHA, mode: entry.Mode}
684709
}
685710

686-
// 5. Compute expected blob SHAs and filter to changed files.
711+
// 4. Compute expected blob SHAs and filter to changed files.
687712
var changedEntries []map[string]string
688713
for _, f := range files {
689714
expectedSHA := blobSHA(f.Content)
@@ -702,7 +727,7 @@ func (c *LiveClient) CommitFiles(ctx context.Context, owner, repo, message strin
702727
return false, nil
703728
}
704729

705-
// 6. Create new tree with base_tree + changed entries.
730+
// 5. Create new tree with base_tree + changed entries.
706731
treePayload := map[string]any{
707732
"base_tree": baseTreeSHA,
708733
"tree": changedEntries,
@@ -718,7 +743,7 @@ func (c *LiveClient) CommitFiles(ctx context.Context, owner, repo, message strin
718743
return false, fmt.Errorf("decode new tree: %w", err)
719744
}
720745

721-
// 7. Create commit with new tree and old commit as parent.
746+
// 6. Create commit with new tree and old commit as parent.
722747
commitPayload := map[string]any{
723748
"message": message,
724749
"tree": newTree.SHA,
@@ -735,11 +760,11 @@ func (c *LiveClient) CommitFiles(ctx context.Context, owner, repo, message strin
735760
return false, fmt.Errorf("decode new commit: %w", err)
736761
}
737762

738-
// 8. Update branch ref to point to new commit.
763+
// 7. Update branch ref to point to new commit.
739764
refPayload := map[string]string{
740765
"sha": newCommit.SHA,
741766
}
742-
refUpdateResp, err := c.patch(ctx, fmt.Sprintf("/repos/%s/%s/git/refs/heads/%s", owner, repo, repoInfo.DefaultBranch), refPayload)
767+
refUpdateResp, err := c.patch(ctx, fmt.Sprintf("/repos/%s/%s/git/refs/heads/%s", owner, repo, branch), refPayload)
743768
if err != nil {
744769
return false, fmt.Errorf("update ref: %w", err)
745770
}

0 commit comments

Comments
 (0)