Skip to content

feat(#6002): add --signoff flag to github setup command #6004

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
maruiz93 merged 7 commits into from
Aug 13, 2026
Merged

feat(#6002): add --signoff flag to github setup command #6004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cbc21c9
feat(#6002): add --signoff flag to github setup command
fullsend-ai-coder[bot] Aug 7, 2026
67803a5
fix: address review feedback on PR #6004
fullsend-ai-coder[bot] Aug 10, 2026
3821964
fix(#6002): strip angle brackets from name in FormatSignOffTrailer
fullsend-ai-coder[bot] Aug 10, 2026
ffe0022
fix: strip angle brackets from email field in FormatSignOffTrailer
fullsend-ai-coder[bot] Aug 10, 2026
eb0b74c
fix: resolve signoff identity before dry-run early return
fullsend-ai-coder[bot] Aug 11, 2026
d4bf64b
fix: add post-sanitization emptiness guard in FormatSignOffTrailer
fullsend-ai-coder[bot] Aug 11, 2026
8f9a921
fix: trim whitespace in FormatSignOffTrailer inputs
maruiz93 Aug 13, 2026
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
18 changes: 14 additions & 4 deletions internal/cli/admin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1130,7 +1130,7 @@ func runPerRepoInstall(ctx context.Context, c perRepoInstallConfig) error {
"FULLSEND_GCP_PROJECT_ID": inferenceProject,
"FULLSEND_GCP_WIF_PROVIDER": inferenceWIFProvider,
}
if err := applyPerRepoScaffold(ctx, client, printer, owner, repo, vendorFiles, repoVars, repoSecrets, c.Direct); err != nil {
if err := applyPerRepoScaffold(ctx, client, printer, owner, repo, vendorFiles, repoVars, repoSecrets, scaffoldOptions{direct: c.Direct}); err != nil {
return err
}
}
Expand Down Expand Up @@ -1210,11 +1210,18 @@ func (a *gcfProvisionerAdapter) DeleteWIFProvider(ctx context.Context, repo stri
return a.provisioner.DeleteWIFProvider(ctx, providerID)
}

// scaffoldOptions holds optional behavioral modifiers for applyPerRepoScaffold,
// keeping the function signature stable as new options are added.
type scaffoldOptions struct {
direct bool // push directly to the default branch instead of creating a PR
signOffTrailer string // e.g. "Signed-off-by: Name <email>"; appended to the commit message when non-empty
}

// 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, direct bool) error {
repoVars, repoSecrets map[string]string, opts scaffoldOptions) error {

targetRepo, err := client.GetRepo(ctx, owner, repo)
if err != nil {
Expand All @@ -1227,7 +1234,10 @@ func applyPerRepoScaffold(ctx context.Context, client forge.Client, printer *ui.
// BuildScaffoldPRMetadata will use the guard variable to distinguish
// fresh installs from upgrades without version information.
meta := repos.BuildScaffoldPRMetadata(ctx, client, owner, repo, "")
if direct {
if opts.signOffTrailer != "" {
meta.CommitMsg += "\n\n" + opts.signOffTrailer
}
if opts.direct {
printer.StepStart(fmt.Sprintf("Committing scaffold files to %s/%s (%s branch)",
owner, repo, targetRepo.DefaultBranch))
} else {
Expand All @@ -1236,7 +1246,7 @@ func applyPerRepoScaffold(ctx context.Context, client forge.Client, printer *ui.
}
if _, err := layers.CommitScaffoldFiles(ctx, client, printer,
owner, repo, targetRepo.DefaultBranch,
meta, files, direct, os.Stdin); err != nil {
meta, files, opts.direct, os.Stdin); err != nil {
return err
}

Expand Down
75 changes: 58 additions & 17 deletions internal/cli/admin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2657,7 +2657,7 @@ func TestApplyPerRepoScaffold(t *testing.T) {
}

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

require.Len(t, client.CommittedFilesToBranch, 1)
Expand All @@ -2683,14 +2683,55 @@ func TestApplyPerRepoScaffold(t *testing.T) {
assert.Contains(t, secretNames, "FULLSEND_GCP_WIF_PROVIDER")
}

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

files := []forge.TreeFile{
{Path: ".fullsend/config.yaml", Content: []byte("config"), Mode: "100644"},
}
trailer := "Signed-off-by: Test User <test@example.com>"

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil, scaffoldOptions{direct: true, signOffTrailer: trailer})
require.NoError(t, err)

require.NotEmpty(t, client.CommittedFiles)
commitMsg := client.CommittedFiles[0].Message
assert.Contains(t, commitMsg, "chore: initialize fullsend per-repo installation")
assert.Contains(t, commitMsg, "Signed-off-by: Test User <test@example.com>")
}

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

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

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil, scaffoldOptions{direct: true})
require.NoError(t, err)

require.NotEmpty(t, client.CommittedFiles)
commitMsg := client.CommittedFiles[0].Message
assert.Equal(t, "chore: initialize fullsend per-repo installation", commitMsg)
}

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

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", nil, nil, nil, false)
"acme", "widget", nil, nil, nil, scaffoldOptions{})
require.Error(t, err)
assert.Contains(t, err.Error(), "getting repo info")
}
Expand All @@ -2707,7 +2748,7 @@ func TestApplyPerRepoScaffold_CommitFilesError(t *testing.T) {
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil, true)
"acme", "widget", files, nil, nil, scaffoldOptions{direct: true})
require.Error(t, err)
assert.Contains(t, err.Error(), "committing scaffold files")
assert.Empty(t, client.CreatedBranches, "should not attempt fallback for generic error")
Expand All @@ -2731,7 +2772,7 @@ func TestApplyPerRepoScaffold_Idempotent(t *testing.T) {
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, map[string]string{"K": "V"}, map[string]string{"S": "secret"}, false)
"acme", "widget", files, map[string]string{"K": "V"}, map[string]string{"S": "secret"}, scaffoldOptions{})
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")
Expand All @@ -2753,7 +2794,7 @@ func TestApplyPerRepoScaffold_DefaultPR_NoChanges(t *testing.T) {
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, map[string]string{"K": "V"}, nil, false)
"acme", "widget", files, map[string]string{"K": "V"}, nil, scaffoldOptions{})
require.NoError(t, err)
assert.Contains(t, buf.String(), "up to date")
}
Expand All @@ -2770,7 +2811,7 @@ func TestApplyPerRepoScaffold_NonMainBranch(t *testing.T) {
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil, true)
"acme", "widget", files, nil, nil, scaffoldOptions{direct: true})
require.NoError(t, err)
assert.Contains(t, buf.String(), "acme/widget (develop branch)")
assert.Contains(t, buf.String(), "Pushed 1 file to develop")
Expand All @@ -2786,7 +2827,7 @@ func TestApplyPerRepoScaffold_CreateVariableError(t *testing.T) {
printer := ui.New(&bytes.Buffer{})

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", nil, map[string]string{"K": "V"}, nil, false)
"acme", "widget", nil, map[string]string{"K": "V"}, nil, scaffoldOptions{})
require.Error(t, err)
assert.Contains(t, err.Error(), "setting repo variable")
}
Expand All @@ -2801,7 +2842,7 @@ func TestApplyPerRepoScaffold_CreateSecretError(t *testing.T) {
printer := ui.New(&bytes.Buffer{})

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", nil, nil, map[string]string{"S": "V"}, false)
"acme", "widget", nil, nil, map[string]string{"S": "V"}, scaffoldOptions{})
require.Error(t, err)
assert.Contains(t, err.Error(), "setting repo secret")
}
Expand All @@ -2821,7 +2862,7 @@ func TestApplyPerRepoScaffold_ProtectedBranchFallback(t *testing.T) {
repoSecrets := map[string]string{"S": "secret"}

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

require.Len(t, client.CreatedBranches, 1)
Expand Down Expand Up @@ -2854,7 +2895,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_ExistingBranch(t *testing.T) {
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil, true)
"acme", "widget", files, nil, nil, scaffoldOptions{direct: true})
require.NoError(t, err)

require.Len(t, client.CommittedFilesToBranch, 1, "should proceed despite branch existing")
Expand All @@ -2875,7 +2916,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_StillSetsVarsAndSecrets(t *testing
repoSecrets := map[string]string{"FULLSEND_GCP_PROJECT_ID": "my-project"}

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

assert.Len(t, client.Variables, 1, "variables should be set even with PR fallback")
Expand All @@ -2895,7 +2936,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_CreateBranchFails(t *testing.T) {
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil, true)
"acme", "widget", files, nil, nil, scaffoldOptions{direct: true})
require.Error(t, err)
assert.Contains(t, err.Error(), "creating scaffold branch")
}
Expand All @@ -2913,7 +2954,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_CommitToBranchFails(t *testing.T)
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil, true)
"acme", "widget", files, nil, nil, scaffoldOptions{direct: true})
require.Error(t, err)
assert.Contains(t, err.Error(), "committing scaffold files to branch")
}
Expand All @@ -2931,7 +2972,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_ScaffoldBranchAlsoProtected(t *tes
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil, true)
"acme", "widget", files, nil, nil, scaffoldOptions{direct: true})
require.Error(t, err)
assert.Contains(t, err.Error(), "is protected")
assert.Contains(t, err.Error(), "configure branch protection")
Expand All @@ -2950,7 +2991,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_CreatePRFails(t *testing.T) {
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil, true)
"acme", "widget", files, nil, nil, scaffoldOptions{direct: true})
require.Error(t, err)
assert.Contains(t, err.Error(), "creating scaffold PR")
}
Expand All @@ -2969,7 +3010,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_DuplicatePR(t *testing.T) {
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil, true)
"acme", "widget", files, nil, nil, scaffoldOptions{direct: true})
require.NoError(t, err)

output := buf.String()
Expand Down Expand Up @@ -3136,7 +3177,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_BranchUpToDate(t *testing.T) {
}

err := applyPerRepoScaffold(context.Background(), client, printer,
"acme", "widget", files, nil, nil, true)
"acme", "widget", files, nil, nil, scaffoldOptions{direct: true})
require.NoError(t, err)

assert.Contains(t, buf.String(), "up to date")
Expand Down
35 changes: 33 additions & 2 deletions internal/cli/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ type githubSetupConfig struct {
runtime string
Comment thread
maruiz93 marked this conversation as resolved.
Comment thread
maruiz93 marked this conversation as resolved.
configPreset string // --config: local path or HTTPS URL to a preset
configHash string // --config-hash: SHA-256 hex digest for preset validation
signoff bool // --signoff: add Signed-off-by trailer to scaffold commits
Comment thread
maruiz93 marked this conversation as resolved.

// changedFlags records which flags were explicitly set on the
// command line (populated by RunE before calling the setup
Expand Down Expand Up @@ -116,7 +117,7 @@ values (mint URL, WIF provider, project ID) are provided as flags.`,

_, _, isRepoTarget := parseTarget(cfg.target)
if !isRepoTarget {
for _, name := range []string{"config", "config-hash"} {
for _, name := range []string{"config", "config-hash", "signoff"} {
if cmd.Flags().Changed(name) {
return fmt.Errorf("--%s is only valid for per-repo setup (fullsend github setup <owner/repo>)", name)
}
Expand Down Expand Up @@ -191,6 +192,7 @@ values (mint URL, WIF provider, project ID) are provided as flags.`,
addVendorFlags(cmd, &cfg.vendor, &cfg.fullsendBinary, &cfg.fullsendSource)
cmd.Flags().StringVar(&cfg.configPreset, "config", "", "local file path or HTTPS URL to a vendor preset (committed as .fullsend/config.base.yaml)")
cmd.Flags().StringVar(&cfg.configHash, "config-hash", "", "SHA-256 hex digest to validate the preset content")
Comment thread
maruiz93 marked this conversation as resolved.
Comment thread
maruiz93 marked this conversation as resolved.
cmd.Flags().BoolVar(&cfg.signoff, "signoff", false, "add Signed-off-by trailer to scaffold commits (requires GitHub user identity)")

return cmd
}
Expand Down Expand Up @@ -402,12 +404,41 @@ func runGitHubSetupPerRepo(ctx context.Context, client forge.Client, printer *ui
repoSecrets["FULLSEND_GCP_WIF_PROVIDER"] = cfg.inferenceWIFProvider
}

// Resolve Signed-off-by trailer when --signoff is set.
//
// Identity resolution runs before the dry-run early return so that
// --dry-run --signoff validates the token's identity up front instead
// of silently skipping the check.
//
// Unlike sync-scaffold (which gracefully degrades when identity is
// unavailable), setup uses an explicit opt-in flag and hard-fails.
// The user explicitly requested DCO sign-off; silently omitting the
// trailer would cause the DCO check to fail with a confusing error.
var signOffTrailer string
if cfg.signoff {
id, idErr := client.GetAuthenticatedUserIdentity(ctx)
if idErr != nil {
Comment thread
maruiz93 marked this conversation as resolved.
return fmt.Errorf("--signoff requires a GitHub user identity (name and email) — this is not available for GitHub App tokens: %w", idErr)
}
if id.Name == "" || id.Email == "" {
return fmt.Errorf("--signoff requires a GitHub user identity with both name and email set (got name=%q, email=%q)", id.Name, id.Email)
}
trailer, trailerErr := id.SignOffTrailer()
if trailerErr != nil {
return fmt.Errorf("--signoff: %w", trailerErr)
}
signOffTrailer = trailer
}

if cfg.dryRun {
printer.StepInfo("Dry run — no changes will be made")
printer.Blank()
for _, f := range files {
printer.StepDone(fmt.Sprintf("Would commit: %s (%d bytes)", f.Path, len(f.Content)))
}
if signOffTrailer != "" {
printer.StepDone(fmt.Sprintf("Would add trailer: %s", signOffTrailer))
}
printer.Blank()
printer.StepInfo("Would set repository variables:")
for _, name := range maputil.SortedKeys(repoVars) {
Expand Down Expand Up @@ -441,7 +472,7 @@ func runGitHubSetupPerRepo(ctx context.Context, client forge.Client, printer *ui
}
}

if err := applyPerRepoScaffold(ctx, client, printer, owner, repo, files, repoVars, repoSecrets, cfg.direct); err != nil {
if err := applyPerRepoScaffold(ctx, client, printer, owner, repo, files, repoVars, repoSecrets, scaffoldOptions{direct: cfg.direct, signOffTrailer: signOffTrailer}); err != nil {
return err
}

Expand Down
Loading
Loading