Skip to content

Commit e330649

Browse files
authored
Refactor bootstrap profile runner into focused modules (#46855)
1 parent 7666650 commit e330649

12 files changed

Lines changed: 2317 additions & 1457 deletions
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# ADR-46855: Refactor Bootstrap Profile Runner into Focused Modules
2+
3+
**Date**: 2026-07-20
4+
**Status**: Draft
5+
**Deciders**: Unknown
6+
7+
---
8+
9+
### Context
10+
11+
`pkg/cli/bootstrap_profile_runner.go` had grown past a healthy size threshold and mixed four distinct concerns in a single file: top-level orchestration, repository variable/secret/Copilot-auth mutations, GitHub App manifest-registration flow, git commit-and-push operations, and shared parsing/prompt/network helpers. This made the file hard to navigate, review, and test in isolation. Individual logical units (e.g., the GitHub App browser callback server, the git push helper) could not be exercised by focused unit tests without pulling in all adjacent concerns. The existing monolithic test file mirrored this problem, exercising only the top-level control flow rather than the extracted logic paths.
12+
13+
### Decision
14+
15+
We will decompose `bootstrap_profile_runner.go` into four focused files within the same `cli` package, each responsible for one concern:
16+
17+
- `bootstrap_profile_actions_repo.go` — repository variable, secret, and Copilot-auth action handlers
18+
- `bootstrap_profile_github_app.go` — GitHub App manifest registration flow, installation polling, and credential handling
19+
- `bootstrap_profile_git.go` — commit-and-push bootstrap action and git command helpers
20+
- `bootstrap_profile_helpers.go` — parsing, prompt/env resolution, naming, HTML/browser/network utilities, and Copilot permission detection
21+
22+
The top-level runner retains orchestration only (`executeBootstrapProfile`, `applyBootstrapAction`, `bootstrapProfileState`, `bootstrapActionNeedsMutation`). Shared types and injection points remain with the runner to preserve the external CLI surface unchanged. Focused test files are added alongside each new module.
23+
24+
### Alternatives Considered
25+
26+
#### Alternative 1: Keep the Monolithic File, Improve Organization with Comments
27+
28+
Retain `bootstrap_profile_runner.go` as a single file and introduce section comments or `//region` markers to delineate concerns internally. This avoids any file-restructuring risk and requires no changes to test organization. However, it does not improve testability (extracted helpers cannot be tested independently), does not reduce merge-conflict surface on the single file, and does not enforce separation of concerns — the file continues to grow unboundedly as new action types are added.
29+
30+
#### Alternative 2: Extract a Separate Package (`pkg/bootstrap/`)
31+
32+
Move bootstrap logic out of the `cli` package entirely into a dedicated `pkg/bootstrap/` package with an exported API. This would create a hard package boundary that Go's toolchain enforces, preventing inadvertent recoupling. The cost is a substantially more invasive change: exported type names, cross-package visibility decisions, and import rewiring across the `cli` package are all required. This was not chosen because the scope exceeded the stated goal of improving file organization without changing behavior, and the intra-package approach achieves most of the testability benefit with far less churn.
33+
34+
### Consequences
35+
36+
#### Positive
37+
- Each concern is independently navigable and reviewable — a contributor looking at the GitHub App flow reads one ~500-line file rather than scanning a much larger mixed file
38+
- Focused test files can exercise extracted helpers directly, improving test coverage granularity and making test failures easier to attribute to specific subsystems
39+
- Merge-conflict surface per logical concern is reduced: changes to, e.g., the git helper no longer touch the same file as changes to GitHub App registration
40+
41+
#### Negative
42+
- The bootstrap domain now spans five files rather than one; contributors unfamiliar with the decomposition must discover the module map before making changes
43+
- Intra-package boundaries are not enforced by the Go compiler — all symbols remain visible within `cli`, so discipline (code review, naming conventions) is the only guard against recoupling over time
44+
45+
#### Neutral
46+
- The external CLI surface and all observable behavior are preserved unchanged; no callers outside `pkg/cli` are affected
47+
- Shared injection points (function variables for side-effecting operations like `bootstrapUpsertVariable`, `bootstrapSetSecret`) are retained in the runner or helpers file and remain accessible to all test files in the package via the `package cli` test build tag
48+
49+
---
50+
51+
*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
package cli
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"os"
7+
8+
"github.com/cli/go-gh/v2/pkg/api"
9+
"github.com/github/gh-aw/pkg/console"
10+
"github.com/github/gh-aw/pkg/repoutil"
11+
"github.com/github/gh-aw/pkg/stringutil"
12+
)
13+
14+
func runBootstrapRepoVariableAction(ctx context.Context, repo string, action repositoryPackageBootstrapAction, state *bootstrapProfileExistingState) (bool, error) {
15+
if _, exists := state.variables[action.Name]; exists {
16+
return false, nil
17+
}
18+
value, ok, err := resolveBootstrapTextValue(bootstrapRepositoryVariableEnvName(action.Name), action.Prompt, action.Description, action.Default, action.Enum, action.Optional)
19+
if err != nil {
20+
return false, err
21+
}
22+
if !ok {
23+
return false, nil
24+
}
25+
if err := bootstrapUpsertVariable(ctx, repo, action.Name, value); err != nil {
26+
return false, err
27+
}
28+
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Set repository variable "+action.Name))
29+
return true, nil
30+
}
31+
32+
func runBootstrapRepoSecretAction(ctx context.Context, repo string, action repositoryPackageBootstrapAction, state *bootstrapProfileExistingState) (bool, error) {
33+
if _, exists := state.secrets[action.Name]; exists {
34+
return false, nil
35+
}
36+
value, ok, err := resolveBootstrapSecretValue(bootstrapRepositorySecretEnvName(action.Name), action.Prompt, action.Description, action.Optional)
37+
if err != nil {
38+
return false, err
39+
}
40+
if !ok {
41+
return false, nil
42+
}
43+
if err := bootstrapSetSecret(ctx, repo, action.Name, value); err != nil {
44+
return false, err
45+
}
46+
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Set repository secret "+action.Name))
47+
return true, nil
48+
}
49+
50+
func runBootstrapCopilotAuthAction(ctx context.Context, repo string, action repositoryPackageBootstrapAction, state *bootstrapProfileExistingState, usesActionsToken bool) (bool, error) {
51+
if usesActionsToken {
52+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Skipping Copilot PAT setup because selected workflows already support GitHub Actions token auth."))
53+
return false, nil
54+
}
55+
if _, exists := state.secrets[action.Secret]; exists {
56+
return false, nil
57+
}
58+
value, ok, err := resolveBootstrapSecretValue(action.Secret, "Copilot fine-grained PAT", "Enter a fine-grained personal access token starting with github_pat_.", false)
59+
if err != nil {
60+
return false, err
61+
}
62+
if !ok {
63+
return false, nil
64+
}
65+
if err := stringutil.ValidateCopilotPAT(value); err != nil {
66+
return false, err
67+
}
68+
if err := bootstrapSetSecret(ctx, repo, action.Secret, value); err != nil {
69+
return false, err
70+
}
71+
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Set repository secret "+action.Secret))
72+
return true, nil
73+
}
74+
75+
func listBootstrapRepoVariableNames(ctx context.Context, repo string) ([]string, error) {
76+
output, err := runBootstrapGHContext(ctx, "Checking repository variables...", "api", fmt.Sprintf("/repos/%s/actions/variables?per_page=100", repo), "--paginate", "--jq", ".variables[].name")
77+
if err != nil {
78+
return nil, fmt.Errorf("failed to list repository variables for %s: %w", repo, err)
79+
}
80+
return parseBootstrapNames(output), nil
81+
}
82+
83+
func listBootstrapRepoSecretNames(ctx context.Context, repo string) ([]string, error) {
84+
output, err := runBootstrapGHContext(ctx, "Checking repository secrets...", "api", fmt.Sprintf("/repos/%s/actions/secrets?per_page=100", repo), "--paginate", "--jq", ".secrets[].name")
85+
if err != nil {
86+
return nil, fmt.Errorf("failed to list repository secrets for %s: %w", repo, err)
87+
}
88+
return parseBootstrapNames(output), nil
89+
}
90+
91+
func upsertBootstrapRepoVariable(repo, name, value string) error {
92+
target := defaultsTarget{}
93+
owner, repoName, err := repoutil.SplitRepoSlug(repo)
94+
if err != nil {
95+
return err
96+
}
97+
target.scope = defaultsScopeRepo
98+
target.repoOwner = owner
99+
target.repoName = repoName
100+
return upsertDefaultsVariable(target, name, value)
101+
}
102+
103+
func setBootstrapRepoSecret(repo, name, value string) error {
104+
owner, repoName, err := repoutil.SplitRepoSlug(repo)
105+
if err != nil {
106+
return err
107+
}
108+
client, err := api.NewRESTClient(secretSetClientOptions(""))
109+
if err != nil {
110+
return err
111+
}
112+
return setRepoSecret(client, owner, repoName, name, value)
113+
}
Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
//go:build !integration
2+
3+
package cli
4+
5+
import (
6+
"context"
7+
"slices"
8+
"strings"
9+
"testing"
10+
)
11+
12+
func TestListBootstrapRepoNamesPaginate(t *testing.T) {
13+
originalRunGH := runBootstrapGHContext
14+
t.Cleanup(func() {
15+
runBootstrapGHContext = originalRunGH
16+
})
17+
18+
calls := []string{}
19+
runBootstrapGHContext = func(_ context.Context, _ string, args ...string) ([]byte, error) {
20+
calls = append(calls, strings.Join(args, " "))
21+
if strings.Contains(args[1], "/variables") {
22+
return []byte("ALPHA\nOMEGA\n"), nil
23+
}
24+
return []byte("FIRST\nSECOND\n"), nil
25+
}
26+
27+
variables, err := listBootstrapRepoVariableNames(context.Background(), "octo/platform-ops")
28+
if err != nil {
29+
t.Fatalf("listBootstrapRepoVariableNames returned error: %v", err)
30+
}
31+
if !slices.Equal(variables, []string{"ALPHA", "OMEGA"}) {
32+
t.Fatalf("unexpected variables: %#v", variables)
33+
}
34+
35+
secrets, err := listBootstrapRepoSecretNames(context.Background(), "octo/platform-ops")
36+
if err != nil {
37+
t.Fatalf("listBootstrapRepoSecretNames returned error: %v", err)
38+
}
39+
if !slices.Equal(secrets, []string{"FIRST", "SECOND"}) {
40+
t.Fatalf("unexpected secrets: %#v", secrets)
41+
}
42+
43+
if len(calls) != 2 {
44+
t.Fatalf("expected 2 gh api calls, got %d", len(calls))
45+
}
46+
for _, call := range calls {
47+
if !strings.Contains(call, "--paginate") {
48+
t.Fatalf("expected paginated gh api call, got %q", call)
49+
}
50+
}
51+
}
52+
53+
func TestRunBootstrapRepoVariableAction(t *testing.T) {
54+
originalUpsertVariable := bootstrapUpsertVariable
55+
t.Cleanup(func() {
56+
bootstrapUpsertVariable = originalUpsertVariable
57+
})
58+
59+
t.Setenv(bootstrapRepositoryVariableEnvName("MY_VAR"), "configured")
60+
var gotName, gotValue string
61+
bootstrapUpsertVariable = func(_ context.Context, _ string, name, value string) error {
62+
gotName = name
63+
gotValue = value
64+
return nil
65+
}
66+
67+
applied, err := runBootstrapRepoVariableAction(context.Background(), "octo/platform-ops", repositoryPackageBootstrapAction{
68+
Name: "MY_VAR",
69+
}, &bootstrapProfileExistingState{variables: map[string]struct{}{}, secrets: map[string]struct{}{}})
70+
if err != nil {
71+
t.Fatalf("runBootstrapRepoVariableAction returned error: %v", err)
72+
}
73+
if !applied {
74+
t.Fatal("expected variable action to apply")
75+
}
76+
if gotName != "MY_VAR" || gotValue != "configured" {
77+
t.Fatalf("unexpected variable write: %s=%s", gotName, gotValue)
78+
}
79+
}
80+
81+
func TestRunBootstrapRepoSecretAction(t *testing.T) {
82+
originalSetSecret := bootstrapSetSecret
83+
t.Cleanup(func() {
84+
bootstrapSetSecret = originalSetSecret
85+
})
86+
87+
t.Setenv(bootstrapRepositorySecretEnvName("MY_SECRET"), "top-secret")
88+
var gotName, gotValue string
89+
bootstrapSetSecret = func(_ context.Context, _ string, name, value string) error {
90+
gotName = name
91+
gotValue = value
92+
return nil
93+
}
94+
95+
applied, err := runBootstrapRepoSecretAction(context.Background(), "octo/platform-ops", repositoryPackageBootstrapAction{
96+
Name: "MY_SECRET",
97+
}, &bootstrapProfileExistingState{variables: map[string]struct{}{}, secrets: map[string]struct{}{}})
98+
if err != nil {
99+
t.Fatalf("runBootstrapRepoSecretAction returned error: %v", err)
100+
}
101+
if !applied {
102+
t.Fatal("expected secret action to apply")
103+
}
104+
if gotName != "MY_SECRET" || gotValue != "top-secret" {
105+
t.Fatalf("unexpected secret write: %s=%s", gotName, gotValue)
106+
}
107+
}
108+
109+
func TestRunBootstrapCopilotAuthAction(t *testing.T) {
110+
t.Run("skips actions token auth", func(t *testing.T) {
111+
applied, err := runBootstrapCopilotAuthAction(context.Background(), "octo/platform-ops", repositoryPackageBootstrapAction{
112+
Secret: "COPILOT_TOKEN",
113+
}, &bootstrapProfileExistingState{variables: map[string]struct{}{}, secrets: map[string]struct{}{}}, true)
114+
if err != nil {
115+
t.Fatalf("runBootstrapCopilotAuthAction returned error: %v", err)
116+
}
117+
if applied {
118+
t.Fatal("expected action to skip when Actions token auth is enabled")
119+
}
120+
})
121+
122+
t.Run("stores valid pat", func(t *testing.T) {
123+
originalSetSecret := bootstrapSetSecret
124+
t.Cleanup(func() {
125+
bootstrapSetSecret = originalSetSecret
126+
})
127+
128+
t.Setenv("COPILOT_TOKEN", "github_pat_abc123xyz")
129+
var wrote string
130+
bootstrapSetSecret = func(_ context.Context, _ string, name, value string) error {
131+
wrote = name + "=" + value
132+
return nil
133+
}
134+
135+
applied, err := runBootstrapCopilotAuthAction(context.Background(), "octo/platform-ops", repositoryPackageBootstrapAction{
136+
Secret: "COPILOT_TOKEN",
137+
}, &bootstrapProfileExistingState{variables: map[string]struct{}{}, secrets: map[string]struct{}{}}, false)
138+
if err != nil {
139+
t.Fatalf("runBootstrapCopilotAuthAction returned error: %v", err)
140+
}
141+
if !applied {
142+
t.Fatal("expected action to apply")
143+
}
144+
if wrote != "COPILOT_TOKEN=github_pat_abc123xyz" {
145+
t.Fatalf("unexpected secret write: %s", wrote)
146+
}
147+
})
148+
}
149+
150+
func TestBootstrapRepoMutationHelpers_RejectInvalidRepo(t *testing.T) {
151+
if err := upsertBootstrapRepoVariable("not-a-repo", "NAME", "value"); err == nil {
152+
t.Fatal("expected invalid repo slug error for variable upsert")
153+
}
154+
if err := setBootstrapRepoSecret("not-a-repo", "NAME", "value"); err == nil {
155+
t.Fatal("expected invalid repo slug error for secret set")
156+
}
157+
}

pkg/cli/bootstrap_profile_git.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package cli
2+
3+
import (
4+
"context"
5+
"errors"
6+
"fmt"
7+
"os"
8+
"os/exec"
9+
"strings"
10+
11+
"github.com/github/gh-aw/pkg/console"
12+
)
13+
14+
func runBootstrapCommitAndPushAction(ctx context.Context, repoDir string, action repositoryPackageBootstrapAction) error {
15+
if repoDir == "" {
16+
return errors.New("bootstrap commit-and-push requires a local checkout directory. Example: rerun from a git checkout and then rerun gh aw add from that checkout")
17+
}
18+
19+
pending, err := bootstrapRepoHasPendingChanges(ctx, repoDir)
20+
if err != nil {
21+
return err
22+
}
23+
if !pending {
24+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Skipping commit and push because the local checkout is already clean."))
25+
return nil
26+
}
27+
28+
if _, err := runBootstrapGitCommand(ctx, repoDir, "add", "-A"); err != nil {
29+
return err
30+
}
31+
if _, err := runBootstrapGitCommand(ctx, repoDir, "commit", "-m", action.Message); err != nil {
32+
return err
33+
}
34+
branch, err := getCurrentBranchIn(repoDir)
35+
if err != nil {
36+
return fmt.Errorf("failed to determine current branch for bootstrap commit-and-push: %w", err)
37+
}
38+
if _, err := runBootstrapGitCommand(ctx, repoDir, "push", "-u", "origin", branch); err != nil {
39+
return err
40+
}
41+
42+
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Committed and pushed bootstrap changes"))
43+
return nil
44+
}
45+
46+
func bootstrapRepoHasPendingChanges(ctx context.Context, repoDir string) (bool, error) {
47+
output, err := runBootstrapGitCommand(ctx, repoDir, "status", "--porcelain")
48+
if err != nil {
49+
return false, err
50+
}
51+
return strings.TrimSpace(string(output)) != "", nil
52+
}
53+
54+
func runBootstrapGitCommand(ctx context.Context, repoDir string, args ...string) ([]byte, error) {
55+
cmd := exec.CommandContext(ctx, "git", args...)
56+
cmd.Dir = repoDir
57+
output, err := cmd.CombinedOutput()
58+
if err != nil {
59+
return output, fmt.Errorf("failed to run git %s in %s: %w\n%s", strings.Join(args, " "), repoDir, err, strings.TrimSpace(string(output)))
60+
}
61+
return output, nil
62+
}

0 commit comments

Comments
 (0)