Skip to content

Commit c03bd36

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 c03bd36

8 files changed

Lines changed: 442 additions & 45 deletions

File tree

internal/cli/admin.go

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,22 +1023,17 @@ 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 {
1031-
printer.StepFail("Failed to commit scaffold files")
1032-
return fmt.Errorf("committing scaffold files: %w", err)
1033-
}
1034-
if committed {
1035-
noun := "files"
1036-
if len(files) == 1 {
1037-
noun = "file"
1038-
}
1039-
printer.StepDone(fmt.Sprintf("Pushed %d %s to %s", len(files), noun, targetRepo.DefaultBranch))
1040-
} else {
1041-
printer.StepDone("Scaffold up to date")
1029+
prBody := fmt.Sprintf("This PR adds the fullsend scaffold files for per-repo installation.\n\n"+
1030+
"The default branch (%s) has branch protection rules that prevent direct pushes, "+
1031+
"so these files are delivered via PR instead.\n\n"+
1032+
"Merge this PR to activate fullsend workflows.", targetRepo.DefaultBranch)
1033+
if err := layers.CommitScaffoldFiles(ctx, client, printer,
1034+
owner, repo, targetRepo.DefaultBranch,
1035+
commitMsg, "chore: initialize fullsend per-repo installation", prBody, files); err != nil {
1036+
return err
10421037
}
10431038

10441039
printer.StepStart("Configuring repository variables")

internal/cli/admin_test.go

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1980,6 +1980,8 @@ func TestApplyPerRepoScaffold_CommitFilesError(t *testing.T) {
19801980
"acme", "widget", files, nil, nil)
19811981
require.Error(t, err)
19821982
assert.Contains(t, err.Error(), "committing scaffold files")
1983+
assert.Empty(t, client.CreatedBranches, "should not attempt fallback for generic error")
1984+
assert.Empty(t, client.CreatedProposals, "should not attempt fallback for generic error")
19831985
}
19841986

19851987
func TestApplyPerRepoScaffold_Idempotent(t *testing.T) {
@@ -2046,3 +2048,169 @@ func TestApplyPerRepoScaffold_CreateSecretError(t *testing.T) {
20462048
require.Error(t, err)
20472049
assert.Contains(t, err.Error(), "setting repo secret")
20482050
}
2051+
2052+
func TestApplyPerRepoScaffold_ProtectedBranchFallback(t *testing.T) {
2053+
client := forge.NewFakeClient()
2054+
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
2055+
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
2056+
var buf bytes.Buffer
2057+
printer := ui.New(&buf)
2058+
2059+
files := []forge.TreeFile{
2060+
{Path: ".github/workflows/fullsend.yaml", Content: []byte("workflow"), Mode: "100644"},
2061+
}
2062+
repoVars := map[string]string{"K": "V"}
2063+
repoSecrets := map[string]string{"S": "secret"}
2064+
2065+
err := applyPerRepoScaffold(context.Background(), client, printer,
2066+
"acme", "widget", files, repoVars, repoSecrets)
2067+
require.NoError(t, err)
2068+
2069+
require.Len(t, client.CreatedBranches, 1)
2070+
assert.Equal(t, "acme/widget/fullsend/scaffold-install", client.CreatedBranches[0])
2071+
2072+
require.Len(t, client.CommittedFilesToBranch, 1)
2073+
assert.Equal(t, "fullsend/scaffold-install", client.CommittedFilesToBranch[0].Branch)
2074+
assert.Len(t, client.CommittedFilesToBranch[0].Files, 1)
2075+
2076+
require.Len(t, client.CreatedProposals, 1)
2077+
assert.Contains(t, client.CreatedProposals[0].Title, "fullsend")
2078+
2079+
output := buf.String()
2080+
assert.Contains(t, output, "protected")
2081+
assert.Contains(t, output, "PR #1")
2082+
assert.Contains(t, output, "Merge the PR")
2083+
}
2084+
2085+
func TestApplyPerRepoScaffold_ProtectedBranch_ExistingBranch(t *testing.T) {
2086+
client := forge.NewFakeClient()
2087+
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
2088+
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
2089+
client.Errors["CreateBranch"] = fmt.Errorf("already exists")
2090+
var buf bytes.Buffer
2091+
printer := ui.New(&buf)
2092+
2093+
files := []forge.TreeFile{
2094+
{Path: ".fullsend/config.yaml", Content: []byte("cfg"), Mode: "100644"},
2095+
}
2096+
2097+
err := applyPerRepoScaffold(context.Background(), client, printer,
2098+
"acme", "widget", files, nil, nil)
2099+
require.NoError(t, err)
2100+
2101+
require.Len(t, client.CommittedFilesToBranch, 1, "should proceed despite branch existing")
2102+
require.Len(t, client.CreatedProposals, 1)
2103+
}
2104+
2105+
func TestApplyPerRepoScaffold_ProtectedBranch_StillSetsVarsAndSecrets(t *testing.T) {
2106+
client := forge.NewFakeClient()
2107+
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
2108+
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
2109+
printer := ui.New(&bytes.Buffer{})
2110+
2111+
files := []forge.TreeFile{
2112+
{Path: ".fullsend/config.yaml", Content: []byte("cfg"), Mode: "100644"},
2113+
}
2114+
repoVars := map[string]string{"FULLSEND_MINT_URL": "https://mint.example.run.app"}
2115+
repoSecrets := map[string]string{"FULLSEND_GCP_PROJECT_ID": "my-project"}
2116+
2117+
err := applyPerRepoScaffold(context.Background(), client, printer,
2118+
"acme", "widget", files, repoVars, repoSecrets)
2119+
require.NoError(t, err)
2120+
2121+
assert.Len(t, client.Variables, 1, "variables should be set even with PR fallback")
2122+
assert.Len(t, client.CreatedSecrets, 1, "secrets should be set even with PR fallback")
2123+
}
2124+
2125+
func TestApplyPerRepoScaffold_ProtectedBranch_CreateBranchFails(t *testing.T) {
2126+
client := forge.NewFakeClient()
2127+
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
2128+
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
2129+
client.Errors["CreateBranch"] = fmt.Errorf("forbidden")
2130+
printer := ui.New(&bytes.Buffer{})
2131+
2132+
files := []forge.TreeFile{
2133+
{Path: ".fullsend/config.yaml", Content: []byte("cfg"), Mode: "100644"},
2134+
}
2135+
2136+
err := applyPerRepoScaffold(context.Background(), client, printer,
2137+
"acme", "widget", files, nil, nil)
2138+
require.Error(t, err)
2139+
assert.Contains(t, err.Error(), "creating scaffold branch")
2140+
}
2141+
2142+
func TestApplyPerRepoScaffold_ProtectedBranch_CommitToBranchFails(t *testing.T) {
2143+
client := forge.NewFakeClient()
2144+
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
2145+
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
2146+
client.Errors["CommitFilesToBranch"] = fmt.Errorf("server error")
2147+
printer := ui.New(&bytes.Buffer{})
2148+
2149+
files := []forge.TreeFile{
2150+
{Path: ".fullsend/config.yaml", Content: []byte("cfg"), Mode: "100644"},
2151+
}
2152+
2153+
err := applyPerRepoScaffold(context.Background(), client, printer,
2154+
"acme", "widget", files, nil, nil)
2155+
require.Error(t, err)
2156+
assert.Contains(t, err.Error(), "committing scaffold files to branch")
2157+
}
2158+
2159+
func TestApplyPerRepoScaffold_ProtectedBranch_CreatePRFails(t *testing.T) {
2160+
client := forge.NewFakeClient()
2161+
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
2162+
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
2163+
client.Errors["CreateChangeProposal"] = fmt.Errorf("forbidden")
2164+
printer := ui.New(&bytes.Buffer{})
2165+
2166+
files := []forge.TreeFile{
2167+
{Path: ".fullsend/config.yaml", Content: []byte("cfg"), Mode: "100644"},
2168+
}
2169+
2170+
err := applyPerRepoScaffold(context.Background(), client, printer,
2171+
"acme", "widget", files, nil, nil)
2172+
require.Error(t, err)
2173+
assert.Contains(t, err.Error(), "creating scaffold PR")
2174+
}
2175+
2176+
func TestApplyPerRepoScaffold_ProtectedBranch_DuplicatePR(t *testing.T) {
2177+
client := forge.NewFakeClient()
2178+
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
2179+
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
2180+
client.Errors["CreateChangeProposal"] = fmt.Errorf("already exists")
2181+
var buf bytes.Buffer
2182+
printer := ui.New(&buf)
2183+
2184+
files := []forge.TreeFile{
2185+
{Path: ".fullsend/config.yaml", Content: []byte("cfg"), Mode: "100644"},
2186+
}
2187+
2188+
err := applyPerRepoScaffold(context.Background(), client, printer,
2189+
"acme", "widget", files, nil, nil)
2190+
require.NoError(t, err)
2191+
2192+
output := buf.String()
2193+
assert.Contains(t, output, "already exists")
2194+
assert.Contains(t, output, "Merge the PR")
2195+
}
2196+
2197+
func TestApplyPerRepoScaffold_ProtectedBranch_BranchUpToDate(t *testing.T) {
2198+
client := forge.NewFakeClient()
2199+
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
2200+
client.Errors["CommitFiles"] = fmt.Errorf("%w: github api: 422", forge.ErrBranchProtected)
2201+
noChange := false
2202+
client.CommitFilesChanged = &noChange
2203+
var buf bytes.Buffer
2204+
printer := ui.New(&buf)
2205+
2206+
files := []forge.TreeFile{
2207+
{Path: ".fullsend/config.yaml", Content: []byte("cfg"), Mode: "100644"},
2208+
}
2209+
2210+
err := applyPerRepoScaffold(context.Background(), client, printer,
2211+
"acme", "widget", files, nil, nil)
2212+
require.NoError(t, err)
2213+
2214+
assert.Empty(t, client.CreatedProposals, "should not create PR when branch files are unchanged")
2215+
assert.Contains(t, buf.String(), "up to date")
2216+
}

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

0 commit comments

Comments
 (0)