Skip to content

fix(cli): make scaffold commit messaging explicit about pushing to default branch #2198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
waynesun09 merged 1 commit into from
Jun 12, 2026
Merged

fix(cli): make scaffold commit messaging explicit about pushing to default branch #2198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 37 additions & 15 deletions internal/cli/admin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -994,15 +994,49 @@ func runPerRepoInstall(ctx context.Context, c perRepoInstallConfig) error {
"FULLSEND_GCP_WIF_PROVIDER": inferenceWIFProvider,
}

printer.StepStart("Writing per-repo scaffold files")
if err := applyPerRepoScaffold(ctx, client, printer, owner, repo, files, repoVars, repoSecrets); err != nil {
return err
}

if vendorBinary {
if err := acquireAndVendorFullsendBinary(ctx, client, printer, owner, repo, fullsendBinary); err != nil {
return fmt.Errorf("vendoring binary: %w", err)
}
} else {
if err := removeStaleVendoredBinary(ctx, client, printer, owner, repo, layers.VendoredBinaryPathPerRepo); err != nil {
return err
}
}

printer.Blank()
printer.StepDone(fmt.Sprintf("Per-repo installation complete for %s/%s", owner, repo))
return nil
}

@fullsend-ai-review fullsend-ai-review Bot Jun 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] redundant API call

applyPerRepoScaffold calls client.GetRepo() solely to retrieve DefaultBranch for a display string, but client.CommitFiles() internally also calls GET /repos/{owner}/{repo} to resolve the default branch for the actual commit. Same pattern in workflows.go.

Suggested fix: Consider refactoring CommitFiles to accept or return the branch name to avoid the extra round-trip.


// applyPerRepoScaffold commits scaffold files to the repo's default branch
// and configures the repository variables and secrets needed for fullsend.
func applyPerRepoScaffold(ctx context.Context, client forge.Client, printer *ui.Printer,
owner, repo string, files []forge.TreeFile,
repoVars, repoSecrets map[string]string) error {

targetRepo, err := client.GetRepo(ctx, owner, repo)
if err != nil {
return fmt.Errorf("getting repo info: %w", err)
}
printer.StepStart(fmt.Sprintf("Committing scaffold files to %s/%s (%s branch)",
owner, repo, targetRepo.DefaultBranch))
committed, err := client.CommitFiles(ctx, owner, repo,
fmt.Sprintf("chore: initialize fullsend-%s per-repo installation", version), files)
if err != nil {
printer.StepFail("Failed to write scaffold files")
printer.StepFail("Failed to commit scaffold files")
return fmt.Errorf("committing scaffold files: %w", err)
}
if committed {
printer.StepDone(fmt.Sprintf("Wrote %d files", len(files)))
noun := "files"
if len(files) == 1 {
noun = "file"
}
printer.StepDone(fmt.Sprintf("Pushed %d %s to %s", len(files), noun, targetRepo.DefaultBranch))
} else {
printer.StepDone("Scaffold up to date")
}
Expand All @@ -1025,18 +1059,6 @@ func runPerRepoInstall(ctx context.Context, c perRepoInstallConfig) error {
}
printer.StepDone(fmt.Sprintf("Set %d repository secrets", len(repoSecrets)))

if vendorBinary {
if err := acquireAndVendorFullsendBinary(ctx, client, printer, owner, repo, fullsendBinary); err != nil {
return fmt.Errorf("vendoring binary: %w", err)
}
} else {
if err := removeStaleVendoredBinary(ctx, client, printer, owner, repo, layers.VendoredBinaryPathPerRepo); err != nil {
return err
}
}

printer.Blank()
printer.StepDone(fmt.Sprintf("Per-repo installation complete for %s/%s", owner, repo))
return nil
}

Expand Down
136 changes: 136 additions & 0 deletions internal/cli/admin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1910,3 +1910,139 @@ func TestInstallCmd_SkipMintCheckStillValidatesWIFProvider(t *testing.T) {
require.Error(t, err)
assert.Contains(t, err.Error(), "--inference-wif-provider must be a full WIF provider resource name")
}

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

files := []forge.TreeFile{
{Path: ".github/workflows/fullsend.yaml", Content: []byte("workflow"), Mode: "100644"},
{Path: ".fullsend/config.yaml", Content: []byte("config"), Mode: "100644"},
}
repoVars := map[string]string{
"FULLSEND_MINT_URL": "https://mint.example.run.app",
"FULLSEND_GCP_REGION": "global",
forge.PerRepoGuardVar: "true",
}
repoSecrets := map[string]string{
"FULLSEND_GCP_PROJECT_ID": "my-project",
"FULLSEND_GCP_WIF_PROVIDER": "projects/123/locations/global/workloadIdentityPools/pool/providers/prov",
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, repoVars, repoSecrets)
require.NoError(t, err)

require.Len(t, client.CommittedFiles, 1)
assert.Equal(t, "acme", client.CommittedFiles[0].Owner)
assert.Equal(t, "widget", client.CommittedFiles[0].Repo)
assert.Len(t, client.CommittedFiles[0].Files, 2)

varNames := make(map[string]string)
for _, v := range client.Variables {
varNames[v.Name] = v.Value
}
assert.Equal(t, "https://mint.example.run.app", varNames["FULLSEND_MINT_URL"])
assert.Equal(t, "global", varNames["FULLSEND_GCP_REGION"])
assert.Equal(t, "true", varNames[forge.PerRepoGuardVar])

secretNames := make(map[string]string)
for _, s := range client.CreatedSecrets {
secretNames[s.Name] = s.Value
}
assert.Equal(t, "my-project", secretNames["FULLSEND_GCP_PROJECT_ID"])
assert.Contains(t, secretNames, "FULLSEND_GCP_WIF_PROVIDER")
}

func TestApplyPerRepoScaffold_GetRepoError(t *testing.T) {
client := forge.NewFakeClient()
client.Errors["GetRepo"] = errors.New("not found")
printer := ui.New(&bytes.Buffer{})

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", nil, nil, nil)
require.Error(t, err)
assert.Contains(t, err.Error(), "getting repo info")
}

func TestApplyPerRepoScaffold_CommitFilesError(t *testing.T) {
client := forge.NewFakeClient()
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
client.Errors["CommitFiles"] = errors.New("permission denied")
printer := ui.New(&bytes.Buffer{})

files := []forge.TreeFile{
{Path: ".fullsend/config.yaml", Content: []byte("cfg"), Mode: "100644"},
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil)
require.Error(t, err)
assert.Contains(t, err.Error(), "committing scaffold files")
}

func TestApplyPerRepoScaffold_Idempotent(t *testing.T) {
client := forge.NewFakeClient()
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
noChange := false
client.CommitFilesChanged = &noChange
var buf bytes.Buffer
printer := ui.New(&buf)

files := []forge.TreeFile{
{Path: ".fullsend/config.yaml", Content: []byte("cfg"), Mode: "100644"},
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, map[string]string{"K": "V"}, map[string]string{"S": "secret"})
require.NoError(t, err)
assert.Contains(t, buf.String(), "up to date")
assert.Len(t, client.Variables, 1, "variables should still be set even when files are unchanged")
assert.Len(t, client.CreatedSecrets, 1, "secrets should still be set even when files are unchanged")
}

func TestApplyPerRepoScaffold_NonMainBranch(t *testing.T) {
client := forge.NewFakeClient()
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "develop"}}
var buf bytes.Buffer
printer := ui.New(&buf)

files := []forge.TreeFile{
{Path: ".fullsend/config.yaml", Content: []byte("cfg"), Mode: "100644"},
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil)
require.NoError(t, err)
assert.Contains(t, buf.String(), "acme/widget (develop branch)")
assert.Contains(t, buf.String(), "Pushed 1 file to develop")
}

func TestApplyPerRepoScaffold_CreateVariableError(t *testing.T) {
client := forge.NewFakeClient()
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
client.Errors = map[string]error{
"CreateOrUpdateRepoVariable": errors.New("rate limited"),
}
printer := ui.New(&bytes.Buffer{})

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", nil, map[string]string{"K": "V"}, nil)
require.Error(t, err)
assert.Contains(t, err.Error(), "setting repo variable")
}

func TestApplyPerRepoScaffold_CreateSecretError(t *testing.T) {
client := forge.NewFakeClient()
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
client.Errors = map[string]error{
"CreateRepoSecret": errors.New("forbidden"),
}
printer := ui.New(&bytes.Buffer{})

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", nil, nil, map[string]string{"S": "V"})
require.Error(t, err)
assert.Contains(t, err.Error(), "setting repo secret")
}
33 changes: 3 additions & 30 deletions internal/cli/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func runGitHubSetupPerRepo(ctx context.Context, client forge.Client, printer *ui
printer.StepInfo("Dry run — no changes will be made")
printer.Blank()
for _, f := range files {
printer.StepDone(fmt.Sprintf("Would write: %s (%d bytes)", f.Path, len(f.Content)))
printer.StepDone(fmt.Sprintf("Would commit: %s (%d bytes)", f.Path, len(f.Content)))
}
printer.Blank()
printer.StepInfo("Would set repository variables:")
Expand All @@ -286,36 +286,9 @@ func runGitHubSetupPerRepo(ctx context.Context, client forge.Client, printer *ui
}
printer.Blank()

printer.StepStart("Writing per-repo scaffold files")
committed, err := client.CommitFiles(ctx, owner, repo,
fmt.Sprintf("chore: initialize fullsend-%s per-repo installation", version), files)
if err != nil {
printer.StepFail("Failed to write scaffold files")
return fmt.Errorf("committing scaffold files: %w", err)
}
if committed {
printer.StepDone(fmt.Sprintf("Wrote %d files", len(files)))
} else {
printer.StepDone("Scaffold up to date")
}

printer.StepStart("Configuring repository variables")
for _, name := range sortedStringMapKeys(repoVars) {
if err := client.CreateOrUpdateRepoVariable(ctx, owner, repo, name, repoVars[name]); err != nil {
printer.StepFail(fmt.Sprintf("Failed to set variable %s", name))
return fmt.Errorf("setting repo variable %s: %w", name, err)
}
}
printer.StepDone(fmt.Sprintf("Set %d repository variables", len(repoVars)))

printer.StepStart("Configuring repository secrets")
for _, name := range sortedStringMapKeys(repoSecrets) {
if err := client.CreateRepoSecret(ctx, owner, repo, name, repoSecrets[name]); err != nil {
printer.StepFail(fmt.Sprintf("Failed to set secret %s", name))
return fmt.Errorf("setting repo secret %s: %w", name, err)
}
if err := applyPerRepoScaffold(ctx, client, printer, owner, repo, files, repoVars, repoSecrets); err != nil {
return err
}
printer.StepDone(fmt.Sprintf("Set %d repository secrets", len(repoSecrets)))

if cfg.vendorBinary {
if err := acquireAndVendorFullsendBinary(ctx, client, printer, owner, repo, cfg.fullsendBinary); err != nil {
Expand Down
4 changes: 4 additions & 0 deletions internal/cli/github_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,7 @@ func TestParseTarget_Repo(t *testing.T) {
func TestRunGitHubSetupPerRepo(t *testing.T) {
t.Setenv("GH_TOKEN", "test-token")
client := forge.NewFakeClient()
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
client.TokenScopes = []string{"repo", "workflow"}
printer := ui.New(&discardWriter{})

Expand Down Expand Up @@ -625,6 +626,7 @@ func TestRunGitHubSetupPerRepo_DryRun(t *testing.T) {
func TestRunGitHubSetupPerRepo_ReusesExistingSecrets(t *testing.T) {
t.Setenv("GH_TOKEN", "test-token")
client := forge.NewFakeClient()
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
client.TokenScopes = []string{"repo", "workflow"}
// Pre-populate secrets as if a previous run stored them.
client.Secrets = map[string]bool{
Expand Down Expand Up @@ -661,6 +663,7 @@ func TestRunGitHubSetupPerRepo_ReusesExistingSecrets(t *testing.T) {
func TestRunGitHubSetupPerRepo_PartialReuse_ProjectOnly(t *testing.T) {
t.Setenv("GH_TOKEN", "test-token")
client := forge.NewFakeClient()
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
client.TokenScopes = []string{"repo", "workflow"}
// Only the project secret exists; WIF is provided via flag.
client.Secrets = map[string]bool{
Expand Down Expand Up @@ -720,6 +723,7 @@ func TestRunGitHubSetupPerRepo_MissingWIFNoExistingSecret(t *testing.T) {
func TestRunGitHubSetupPerRepo_PartialReuse_WIFOnly(t *testing.T) {
t.Setenv("GH_TOKEN", "test-token")
client := forge.NewFakeClient()
client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}}
client.TokenScopes = []string{"repo", "workflow"}
// Only the WIF secret exists; project is provided via flag.
client.Secrets = map[string]bool{
Expand Down
15 changes: 12 additions & 3 deletions internal/layers/workflows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,24 @@ func (l *WorkflowsLayer) Install(ctx context.Context) error {
Mode: "100644",
})

l.ui.StepStart("Writing scaffold files")
cfgRepo, err := l.client.GetRepo(ctx, l.org, forge.ConfigRepoName)
if err != nil {
return fmt.Errorf("getting config repo info: %w", err)
}
l.ui.StepStart(fmt.Sprintf("Committing scaffold files to %s/%s (%s branch)",
l.org, forge.ConfigRepoName, cfgRepo.DefaultBranch))
committed, err := l.client.CommitFiles(ctx, l.org, forge.ConfigRepoName,
fmt.Sprintf("chore: update fullsend-%s scaffold", l.version), files)
if err != nil {
l.ui.StepFail("Failed to write scaffold files")
l.ui.StepFail("Failed to commit scaffold files")
return fmt.Errorf("committing scaffold files: %w", err)
}
if committed {
l.ui.StepDone(fmt.Sprintf("Wrote %d files", len(files)))
noun := "files"
if len(files) == 1 {
noun = "file"
}
l.ui.StepDone(fmt.Sprintf("Pushed %d %s to %s", len(files), noun, cfgRepo.DefaultBranch))
} else {
l.ui.StepDone("Scaffold up to date")
}
Expand Down
10 changes: 10 additions & 0 deletions internal/layers/workflows_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ import (

func newWorkflowsLayer(t *testing.T, client *forge.FakeClient) (*WorkflowsLayer, *bytes.Buffer) {
t.Helper()
if client.Repos == nil {
client.Repos = []forge.Repository{{
FullName: "test-org/" + forge.ConfigRepoName,
DefaultBranch: "main",
}}
}
var buf bytes.Buffer
printer := ui.New(&buf)
layer := NewWorkflowsLayer("test-org", client, printer, "admin-user", "test-version")
Expand Down Expand Up @@ -123,6 +129,10 @@ func TestWorkflowsLayer_Install_ManagedHeaders(t *testing.T) {

func TestWorkflowsLayer_Install_Error(t *testing.T) {
client := &forge.FakeClient{
Repos: []forge.Repository{{
FullName: "test-org/" + forge.ConfigRepoName,
DefaultBranch: "main",
}},
Errors: map[string]error{
"CommitFiles": errors.New("write failed"),
},
Expand Down
Loading