From 835bb4aa748cbc517ab12249410997288aac0566 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 17:52:15 +0200 Subject: [PATCH 01/20] Fix add-wizard existing Copilot token flow --- ...h-use-existing-copilot-token-add-wizard.md | 5 +++ pkg/cli/engine_secrets.go | 36 +++++++++++++++- pkg/cli/engine_secrets_test.go | 41 ++++++++++++++++--- 3 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 .changeset/patch-use-existing-copilot-token-add-wizard.md diff --git a/.changeset/patch-use-existing-copilot-token-add-wizard.md b/.changeset/patch-use-existing-copilot-token-add-wizard.md new file mode 100644 index 00000000000..70fa3531365 --- /dev/null +++ b/.changeset/patch-use-existing-copilot-token-add-wizard.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Default add-wizard to using an existing `COPILOT_GITHUB_TOKEN` secret while retaining an explicit option to replace and validate it. \ No newline at end of file diff --git a/pkg/cli/engine_secrets.go b/pkg/cli/engine_secrets.go index 1ed19cf9bb9..d21776746de 100644 --- a/pkg/cli/engine_secrets.go +++ b/pkg/cli/engine_secrets.go @@ -29,6 +29,24 @@ var ( engineSecretsPromptFn = func(req SecretRequirement, config EngineSecretConfig) error { return promptForSecret(req, config) } + engineSecretsConfirmExistingFn = func(secretName string, config EngineSecretConfig) (bool, error) { + useExisting := true + form := console.NewConfirmForm( + huh.NewConfirm(). + Title(fmt.Sprintf("Use the existing %s repository secret?", secretName)). + Description("GitHub does not expose stored secret values. Choosing this asserts that the existing secret is a valid fine-grained PAT with Copilot Requests permission."). + Affirmative("Use existing secret"). + Negative("Replace secret"). + Value(&useExisting), + ) + if err := form.RunWithContext(config.ctx()); err != nil { + if console.IsCancelled(err) { + return false, promptCancelled() + } + return false, fmt.Errorf("failed to confirm existing %s secret: %w", secretName, err) + } + return useExisting, nil + } engineSecretsUploadFn = func(ctx context.Context, secretName, secretValue, repoSlug string, verbose bool, overwriteExisting bool) error { return uploadSecretToRepo(ctx, secretName, secretValue, repoSlug, verbose, overwriteExisting) } @@ -241,7 +259,14 @@ func ensureSecretAvailable(req SecretRequirement, config EngineSecretConfig) err // Check if secret already exists in the repository if setutil.Contains(config.ExistingSecrets, req.Name) { if mustValidateExistingSecretValue(req) { - console.PrintWarningMessage(req.Name + " already exists, but GitHub does not expose stored secret values for validation.") + useExisting, err := engineSecretsConfirmExistingFn(req.Name, config) + if err != nil { + return err + } + if useExisting { + console.PrintSuccessMessage(fmt.Sprintf("Using existing %s secret in repository", req.Name)) + return nil + } console.PrintInfoMessage("Paste the current or replacement fine-grained PAT so gh aw can validate it and update the repository secret.") revalidateConfig := config revalidateConfig.OverwriteExistingSecret = true @@ -255,7 +280,14 @@ func ensureSecretAvailable(req SecretRequirement, config EngineSecretConfig) err for _, alt := range req.AlternativeEnvVars { if setutil.Contains(config.ExistingSecrets, alt) { if mustValidateExistingSecretValue(req) { - console.PrintWarningMessage(alt + " already exists in the repository, but GitHub does not expose stored secret values for validation.") + useExisting, err := engineSecretsConfirmExistingFn(alt, config) + if err != nil { + return err + } + if useExisting { + console.PrintSuccessMessage(fmt.Sprintf("Using existing %s secret in repository (alternative for %s)", alt, req.Name)) + return nil + } console.PrintInfoMessage(fmt.Sprintf("Paste the current or replacement fine-grained PAT so gh aw can validate it and store it as %s.", req.Name)) revalidateConfig := config revalidateConfig.OverwriteExistingSecret = true diff --git a/pkg/cli/engine_secrets_test.go b/pkg/cli/engine_secrets_test.go index 3fdf7c531d7..82007d9bfc5 100644 --- a/pkg/cli/engine_secrets_test.go +++ b/pkg/cli/engine_secrets_test.go @@ -241,7 +241,7 @@ func TestBuildCopilotPATCreationURL(t *testing.T) { }) } -func TestEnsureSecretAvailable_CopilotRepromptsWithOverwrite(t *testing.T) { +func TestEnsureSecretAvailable_ExistingCopilotSecret(t *testing.T) { copilotReq := SecretRequirement{ Name: constants.CopilotGitHubToken, IsEngineSecret: true, @@ -253,10 +253,41 @@ func TestEnsureSecretAvailable_CopilotRepromptsWithOverwrite(t *testing.T) { EngineName: string(constants.ClaudeEngine), } - t.Run("existing Copilot secret triggers re-prompt with OverwriteExistingSecret=true", func(t *testing.T) { + t.Run("uses existing Copilot secret when confirmed", func(t *testing.T) { + promptCalled := false + origConfirm := engineSecretsConfirmExistingFn + origPrompt := engineSecretsPromptFn + t.Cleanup(func() { + engineSecretsConfirmExistingFn = origConfirm + engineSecretsPromptFn = origPrompt + }) + engineSecretsConfirmExistingFn = func(secretName string, _ EngineSecretConfig) (bool, error) { + assert.Equal(t, constants.CopilotGitHubToken, secretName) + return true, nil + } + engineSecretsPromptFn = func(_ SecretRequirement, _ EngineSecretConfig) error { + promptCalled = true + return nil + } + + cfg := EngineSecretConfig{ + ExistingSecrets: map[string]struct{}{constants.CopilotGitHubToken: {}}, + } + require.NoError(t, ensureSecretAvailable(copilotReq, cfg)) + assert.False(t, promptCalled, "confirmed existing secret must not trigger a token prompt") + }) + + t.Run("replaces existing Copilot secret when requested", func(t *testing.T) { var capturedConfig EngineSecretConfig - orig := engineSecretsPromptFn - t.Cleanup(func() { engineSecretsPromptFn = orig }) + origConfirm := engineSecretsConfirmExistingFn + origPrompt := engineSecretsPromptFn + t.Cleanup(func() { + engineSecretsConfirmExistingFn = origConfirm + engineSecretsPromptFn = origPrompt + }) + engineSecretsConfirmExistingFn = func(_ string, _ EngineSecretConfig) (bool, error) { + return false, nil + } engineSecretsPromptFn = func(req SecretRequirement, config EngineSecretConfig) error { capturedConfig = config return nil @@ -266,7 +297,7 @@ func TestEnsureSecretAvailable_CopilotRepromptsWithOverwrite(t *testing.T) { ExistingSecrets: map[string]struct{}{constants.CopilotGitHubToken: {}}, } require.NoError(t, ensureSecretAvailable(copilotReq, cfg)) - assert.True(t, capturedConfig.OverwriteExistingSecret, "prompt must be called with OverwriteExistingSecret=true") + assert.True(t, capturedConfig.OverwriteExistingSecret, "replacement prompt must overwrite the existing secret") }) t.Run("existing non-Copilot secret skips prompt", func(t *testing.T) { From 1355eb0ad9c3e3156bb0f78d3f6e47c27eb8e30c Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 18:17:21 +0200 Subject: [PATCH 02/20] Improve add workflow setup flow --- ...h-use-existing-copilot-token-add-wizard.md | 2 +- docs/src/content/docs/setup/cli.md | 4 +- pkg/cli/add_command.go | 18 +++++-- pkg/cli/add_command_test.go | 32 +++++++++++++ pkg/cli/add_gh_aw_ref.go | 19 ++++++++ pkg/cli/add_interactive_engine.go | 3 +- pkg/cli/add_interactive_git.go | 1 + pkg/cli/add_interactive_orchestrator.go | 16 ++++++- pkg/cli/add_interactive_orchestrator_test.go | 12 +++++ pkg/cli/add_wizard_command.go | 7 +++ pkg/cli/add_wizard_command_test.go | 2 +- pkg/cli/add_workflow_compilation.go | 47 ++++++++++++++++--- pkg/cli/copilot_billing_check.go | 2 +- 13 files changed, 147 insertions(+), 18 deletions(-) create mode 100644 pkg/cli/add_gh_aw_ref.go diff --git a/.changeset/patch-use-existing-copilot-token-add-wizard.md b/.changeset/patch-use-existing-copilot-token-add-wizard.md index 70fa3531365..5facfbb9faf 100644 --- a/.changeset/patch-use-existing-copilot-token-add-wizard.md +++ b/.changeset/patch-use-existing-copilot-token-add-wizard.md @@ -2,4 +2,4 @@ "gh-aw": patch --- -Default add-wizard to using an existing `COPILOT_GITHUB_TOKEN` secret while retaining an explicit option to replace and validate it. \ No newline at end of file +Default add-wizard to using an existing `COPILOT_GITHUB_TOKEN` secret, improve workflow and Copilot authentication prompt formatting, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file diff --git a/docs/src/content/docs/setup/cli.md b/docs/src/content/docs/setup/cli.md index de10f1dac98..3211eecf161 100644 --- a/docs/src/content/docs/setup/cli.md +++ b/docs/src/content/docs/setup/cli.md @@ -198,7 +198,7 @@ gh aw add-wizard https://example.com/workflows/my-workflow.json # Arbitrary UR gh aw add-wizard githubnext/agentics/ci-doctor --no-secret # Skip secret prompt ``` -**Options:** `--no-secret`, `--dir/-d`, `--engine/-e`, `--no-gitattributes`, `--no-stop-after`, `--stop-after`, `--append`, `--no-security-scanner`, `--no-config` +**Options:** `--no-secret`, `--dir/-d`, `--engine/-e`, `--gh-aw-ref`, `--no-gitattributes`, `--no-stop-after`, `--stop-after`, `--append`, `--no-security-scanner`, `--no-config` When the Copilot engine is selected, the wizard prompts the user to choose an authentication method: organization billing via [`permissions.copilot-requests: write`](/gh-aw/reference/auth/#copilot-requests-write-permission) (no PAT required), or a [`COPILOT_GITHUB_TOKEN`](/gh-aw/reference/auth/#copilot_github_token) personal access token (a separate token from the default `GITHUB_TOKEN`, because the agent needs elevated Copilot API access that the ephemeral workflow token does not carry). On the PAT path, the wizard auto-opens a preconfigured fine-grained PAT creation page (prefilled token name, expiration, and Copilot Requests permission). The GitHub page still must be completed manually in the browser. Users may paste either an existing suitable fine-grained PAT or a newly created one into the masked CLI prompt, but reuse should be based on the token's properties: personal-account resource owner, repository access set to Public repositories, and Copilot Requests permission available. If `COPILOT_GITHUB_TOKEN` already exists, the wizard still asks for the token again because GitHub does not expose stored secret values for validation. The flow does not rely on the PAT display name in GitHub's token list. The pasted token is then validated and stored as a repository secret. @@ -217,7 +217,7 @@ gh aw add https://example.com/workflows/my-workflow.md # Arbitrary gh aw add https://example.com/workflows/my-workflow.json # Arbitrary HTTPS URL (JSON workflow definition) ``` -**Options:** `--dir/-d`, `--create-pull-request`, `--no-gitattributes`, `--append`, `--no-security-scanner`, `--engine/-e`, `--force/-f`, `--name/-n`, `--no-stop-after`, `--stop-after` +**Options:** `--dir/-d`, `--create-pull-request`, `--no-gitattributes`, `--append`, `--no-security-scanner`, `--engine/-e`, `--force/-f`, `--gh-aw-ref`, `--name/-n`, `--no-stop-after`, `--stop-after` Repository-level packages can declare an [`aw.yml` manifest](/gh-aw/reference/aw-yml-package-manifest/) at the repository root or in a nested package folder to define installable files, package `README.md`, schema compatibility, and minimum supported CLI versions. diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index b2329a56fad..28c9f083734 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -78,6 +78,8 @@ type AddOptions struct { NoStopAfter bool StopAfter string DisableSecurityScanner bool + // GhAwRef is the resolved github/gh-aw commit SHA used by compiled action references. + GhAwRef string // AddCopilotRequestsPermission injects permissions.copilot-requests: write into // the workflow frontmatter, enabling GitHub Actions token auth for Copilot. // Set by the add-wizard when the user selects org-billing auth instead of a PAT. @@ -131,6 +133,11 @@ func runAddCommand(cmd *cobra.Command, args []string, validateEngine func(string noStopAfter, _ := cmd.Flags().GetBool("no-stop-after") stopAfter, _ := cmd.Flags().GetString("stop-after") disableSecurityScanner := resolveDeprecatedBoolFlag(cmd, "no-security-scanner", "disable-security-scanner") + ghAwRef, _ := cmd.Flags().GetString("gh-aw-ref") + resolvedGhAwRef, err := resolveAddGhAwRef(cmd.Context(), ghAwRef) + if err != nil { + return err + } if nameFlag != "" && len(args) > 1 { return errors.New("--name was set while multiple workflows were provided. Expected --name only with a single workflow source. Example: gh aw add githubnext/agentics/daily-repo-status --name daily-repo-status") @@ -151,6 +158,7 @@ func runAddCommand(cmd *cobra.Command, args []string, validateEngine func(string NoStopAfter: noStopAfter, StopAfter: stopAfter, DisableSecurityScanner: disableSecurityScanner, + GhAwRef: resolvedGhAwRef, } resolved, err := ResolveWorkflows(cmd.Context(), args, verbose) if err != nil { @@ -221,6 +229,8 @@ func registerAddCommandFlags(cmd *cobra.Command) { // Add no-security-scanner flag to add command (--disable-security-scanner is kept as a deprecated alias) addSecurityScannerFlag(cmd) + cmd.Flags().String("gh-aw-ref", "", "Pin compiled workflows to a branch, tag, or commit SHA of github/gh-aw; branch and tag names are resolved to an immutable full SHA") + // Register completions for add command RegisterEngineFlagCompletion(cmd) RegisterDirFlagCompletion(cmd, "dir") @@ -538,23 +548,23 @@ func compileAddedWorkflow(ctx context.Context, destFile string, workflowSpec *Wo // .lock.yml. The dispatch-workflow validator requires every .md dispatch target to be // compiled before the main workflow can be validated. With --force, always recompile // to pick up freshly overwritten worker files. - compileDispatchWorkflowDependencies(ctx, destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, opts.Force, tracker) + compileDispatchWorkflowDependenciesWithActionRef(ctx, destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, opts.GhAwRef, opts.Force, tracker) // Compile any call-workflow .md worker dependencies that were just fetched and lack a // .lock.yml. Errors are propagated: a missing worker .lock.yml would leave the // orchestrator referencing a non-existent file. With --force, always recompile to // pick up freshly overwritten worker files. - if err := compileCallWorkflowDependencies(ctx, destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, opts.Force, tracker); err != nil { + if err := compileCallWorkflowDependenciesWithActionRef(ctx, destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, opts.GhAwRef, opts.Force, tracker); err != nil { printCompilationError(err, opts.Quiet) return } // Compile the workflow if tracker != nil { - if err := compileWorkflowWithTracking(ctx, destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, tracker); err != nil { + if err := compileWorkflowWithTrackingAndActionRef(ctx, destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, opts.GhAwRef, tracker); err != nil { printCompilationError(err, opts.Quiet) } return } - if err := compileWorkflow(ctx, destFile, opts.Verbose, opts.Quiet, opts.EngineOverride); err != nil { + if err := compileWorkflowWithActionRef(ctx, destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, opts.GhAwRef); err != nil { printCompilationError(err, opts.Quiet) } } diff --git a/pkg/cli/add_command_test.go b/pkg/cli/add_command_test.go index 79ac4645442..41901b0951a 100644 --- a/pkg/cli/add_command_test.go +++ b/pkg/cli/add_command_test.go @@ -82,6 +82,38 @@ func TestNewAddCommand(t *testing.T) { // Check stop-after flag stopAfterFlag := flags.Lookup("stop-after") assert.NotNil(t, stopAfterFlag, "Should have 'stop-after' flag") + + ghAwRefFlag := flags.Lookup("gh-aw-ref") + assert.NotNil(t, ghAwRefFlag, "Should have 'gh-aw-ref' flag") +} + +func TestResolveAddGhAwRef_FullSHA(t *testing.T) { + t.Parallel() + const sha = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + resolved, err := resolveAddGhAwRef(context.Background(), sha) + require.NoError(t, err) + assert.Equal(t, sha, resolved) +} + +func TestCompileWorkflowWithActionRef(t *testing.T) { + const sha = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + tmpDir := t.TempDir() + require.NoError(t, initTestGitRepo(tmpDir)) + workflowFile := filepath.Join(tmpDir, ".github", "workflows", "pinned.md") + require.NoError(t, os.MkdirAll(filepath.Dir(workflowFile), 0o755)) + require.NoError(t, os.WriteFile(workflowFile, []byte(`--- +on: workflow_dispatch +permissions: + contents: read +--- + +# Pinned workflow +`), 0o644)) + + require.NoError(t, compileWorkflowWithActionRef(context.Background(), workflowFile, false, true, "", sha)) + lockContent, err := os.ReadFile(filepath.Join(tmpDir, ".github", "workflows", "pinned.lock.yml")) + require.NoError(t, err) + assert.Contains(t, string(lockContent), "github/gh-aw/actions/setup@"+sha) } func TestNewAddCommand_MentionsEnterpriseSourceResolution(t *testing.T) { diff --git a/pkg/cli/add_gh_aw_ref.go b/pkg/cli/add_gh_aw_ref.go new file mode 100644 index 00000000000..b2a24084937 --- /dev/null +++ b/pkg/cli/add_gh_aw_ref.go @@ -0,0 +1,19 @@ +package cli + +import ( + "context" + "fmt" + + "github.com/github/gh-aw/pkg/workflow" +) + +func resolveAddGhAwRef(ctx context.Context, ref string) (string, error) { + if ref == "" { + return "", nil + } + resolvedRef, err := workflow.ResolveGhAwRef(ctx, ref) + if err != nil { + return "", fmt.Errorf("--gh-aw-ref: %w", err) + } + return resolvedRef, nil +} diff --git a/pkg/cli/add_interactive_engine.go b/pkg/cli/add_interactive_engine.go index 96ee85a911a..bfd09e03ed6 100644 --- a/pkg/cli/add_interactive_engine.go +++ b/pkg/cli/add_interactive_engine.go @@ -46,7 +46,6 @@ func (c *AddInteractiveConfig) selectAIEngineAndKey() error { prioritizeEngineOption(engineOptions, defaultEngine) - fmt.Fprintln(os.Stderr, "") form := console.NewSelectForm( huh.NewSelect[string](). Title("Which coding agent would you like to use?"). @@ -289,7 +288,7 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { var authMethod string selectField := huh.NewSelect[string](). Title("How would you like Copilot workflows to authenticate?"). - Description("copilot-requests uses the org's Copilot billing seat — no PAT required.\nPAT uses a fine-grained personal access token stored as COPILOT_GITHUB_TOKEN (requires repo write access to configure)."). + Description("PAT uses the existing COPILOT_GITHUB_TOKEN repository secret.\ncopilot-requests uses the org's Copilot billing seat and requires no PAT."). Options(options...). Value(&authMethod) diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index dc0034b2054..dbd51bfcce2 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -56,6 +56,7 @@ func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx conte StopAfter: c.StopAfter, DisableSecurityScanner: c.DisableSecurityScanner, AddCopilotRequestsPermission: c.UseCopilotRequests, + GhAwRef: c.GhAwRef, initializedFiles: initFiles, } result, err := AddResolvedWorkflows(ctx, c.WorkflowSpecs, c.resolvedWorkflows, opts) diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index 21efa90c44c..54328c9221a 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -32,6 +32,7 @@ type AddInteractiveConfig struct { RepoOverride string // owner/repo format, if user provides it AppendText string // Extra content to append to the workflow on installation DisableSecurityScanner bool // Disable security scanning of workflow markdown content + GhAwRef string // Resolved github/gh-aw commit SHA used by compiled action references // DisableGitHubAppPermissionInference disables inferring GitHub App // permissions/events from the package's resolved workflows during bootstrap, @@ -178,10 +179,10 @@ func (c *AddInteractiveConfig) applyBootstrapConfigIfNeeded(ctx context.Context, } func (c *AddInteractiveConfig) runInitialAddInteractiveChecks() error { - console.ShowWelcomeBanner("This tool will walk you through adding an automated workflow to your repository.") if err := c.resolveWorkflows(); err != nil { return err } + console.ShowWelcomeBanner(c.welcomeMessage()) c.showWorkflowDescriptions() if err := c.checkGHAuthStatus(); err != nil { return err @@ -195,6 +196,19 @@ func (c *AddInteractiveConfig) runInitialAddInteractiveChecks() error { return c.checkUserPermissions() } +func (c *AddInteractiveConfig) welcomeMessage() string { + workflowNames, err := c.workflowNamesForInteractiveAdd() + if err != nil || len(workflowNames) == 0 { + return "This tool will walk you through adding automated workflows to your repository." + } + + source := strings.Join(c.WorkflowSpecs, ", ") + if len(workflowNames) == 1 { + return fmt.Sprintf("This tool will walk you through adding the automated workflow %q from %q.", workflowNames[0], source) + } + return fmt.Sprintf("This tool will walk you through adding %d automated workflows from %q.", len(workflowNames), source) +} + func (c *AddInteractiveConfig) prepareAndConfirmAddInteractive() (workflowFiles, initFiles []string, secretName, secretValue string, createPR bool, err error) { // selectAIEngineAndKey only selects the engine and, for Copilot, the auth method // (org billing vs. PAT). It does not prompt for or upload any secret value, since diff --git a/pkg/cli/add_interactive_orchestrator_test.go b/pkg/cli/add_interactive_orchestrator_test.go index ae35bf082b8..e1c222f2555 100644 --- a/pkg/cli/add_interactive_orchestrator_test.go +++ b/pkg/cli/add_interactive_orchestrator_test.go @@ -134,6 +134,18 @@ func TestAddInteractiveConfig_primaryWorkflowName(t *testing.T) { }) } +func TestAddInteractiveConfig_welcomeMessage(t *testing.T) { + t.Parallel() + config := &AddInteractiveConfig{ + WorkflowSpecs: []string{"githubnext/agentics/repo-assist"}, + resolvedWorkflows: &ResolvedWorkflows{Workflows: []*ResolvedWorkflow{ + {Spec: &WorkflowSpec{WorkflowName: "repo-assist"}}, + }}, + } + + assert.Equal(t, `This tool will walk you through adding the automated workflow "repo-assist" from "githubnext/agentics/repo-assist".`, config.welcomeMessage()) +} + func TestAddInteractiveConfig_showWorkflowDescriptions(t *testing.T) { t.Parallel() tests := []struct { diff --git a/pkg/cli/add_wizard_command.go b/pkg/cli/add_wizard_command.go index 9c1d8a1a810..4821f227474 100644 --- a/pkg/cli/add_wizard_command.go +++ b/pkg/cli/add_wizard_command.go @@ -76,6 +76,11 @@ Note: To create a new workflow from scratch, use the 'new' command instead.`, appendText, _ := cmd.Flags().GetString("append") disableSecurityScanner := resolveDeprecatedBoolFlag(cmd, "no-security-scanner", "disable-security-scanner") noGitHubAppInference, _ := cmd.Flags().GetBool("no-config") + ghAwRef, _ := cmd.Flags().GetString("gh-aw-ref") + resolvedGhAwRef, err := resolveAddGhAwRef(cmd.Context(), ghAwRef) + if err != nil { + return err + } addWizardLog.Printf("Starting add-wizard: workflows=%v, engine=%s, verbose=%v", workflows, engineOverride, verbose) @@ -103,6 +108,7 @@ Note: To create a new workflow from scratch, use the 'new' command instead.`, AppendText: appendText, DisableSecurityScanner: disableSecurityScanner, DisableGitHubAppPermissionInference: noGitHubAppInference, + GhAwRef: resolvedGhAwRef, }) }, } @@ -137,6 +143,7 @@ Note: To create a new workflow from scratch, use the 'new' command instead.`, // Add no-config flag to allow disabling automatic inference of GitHub App // permissions/events from resolved package workflows. cmd.Flags().Bool("no-config", false, "Disable inferring GitHub App permissions/events from the package's workflows; use only permissions/events declared in aw.yml") + cmd.Flags().String("gh-aw-ref", "", "Pin compiled workflows to a branch, tag, or commit SHA of github/gh-aw; branch and tag names are resolved to an immutable full SHA") // Register completions RegisterEngineFlagCompletion(cmd) diff --git a/pkg/cli/add_wizard_command_test.go b/pkg/cli/add_wizard_command_test.go index a504d8131a7..0e32e4a0c56 100644 --- a/pkg/cli/add_wizard_command_test.go +++ b/pkg/cli/add_wizard_command_test.go @@ -32,7 +32,7 @@ func TestAddWizardCommand_FlagUsageMatchesAddCommand(t *testing.T) { addCmd := NewAddCommand(validateEngineStub) wizardCmd := NewAddWizardCommand(validateEngineStub) - for _, flagName := range []string{"append", "no-security-scanner"} { + for _, flagName := range []string{"append", "no-security-scanner", "gh-aw-ref"} { addFlag := addCmd.Flags().Lookup(flagName) wizardFlag := wizardCmd.Flags().Lookup(flagName) diff --git a/pkg/cli/add_workflow_compilation.go b/pkg/cli/add_workflow_compilation.go index 08f7c5c7c3c..cfab4812fb7 100644 --- a/pkg/cli/add_workflow_compilation.go +++ b/pkg/cli/add_workflow_compilation.go @@ -22,12 +22,20 @@ var addWorkflowCompilationLog = logger.New("cli:add_workflow_compilation") // compileWorkflow compiles a workflow file without refreshing stop time. // This is a convenience wrapper around compileWorkflowWithRefresh. func compileWorkflow(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride string) error { - return compileWorkflowWithRefresh(ctx, filePath, verbose, quiet, engineOverride, false, false) + return compileWorkflowWithActionRef(ctx, filePath, verbose, quiet, engineOverride, "") +} + +func compileWorkflowWithActionRef(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride, actionRef string) error { + return compileWorkflowWithRefreshAndActionRef(ctx, filePath, verbose, quiet, engineOverride, actionRef, false, false) } // compileWorkflowWithRefresh compiles a workflow file with optional stop time refresh. // This function handles the compilation process and ensures .gitattributes is updated. func compileWorkflowWithRefresh(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride string, refreshStopTime bool, approve bool) error { + return compileWorkflowWithRefreshAndActionRef(ctx, filePath, verbose, quiet, engineOverride, "", refreshStopTime, approve) +} + +func compileWorkflowWithRefreshAndActionRef(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride, actionRef string, refreshStopTime bool, approve bool) error { addWorkflowCompilationLog.Printf("Compiling workflow: file=%s, refresh_stop_time=%v, engine=%s, approve=%v", filePath, refreshStopTime, engineOverride, approve) // Create compiler with auto-detected version and action mode @@ -35,6 +43,7 @@ func compileWorkflowWithRefresh(ctx context.Context, filePath string, verbose bo workflow.WithVerbose(verbose), workflow.WithEngineOverride(engineOverride), ) + applyAddActionRef(compiler, actionRef) compiler.SetRefreshStopTime(refreshStopTime) compiler.SetApprove(approve) @@ -62,12 +71,20 @@ func compileWorkflowWithRefresh(ctx context.Context, filePath string, verbose bo // compileWorkflowWithTracking compiles a workflow and tracks generated files. // This is a convenience wrapper around compileWorkflowWithTrackingAndRefresh. func compileWorkflowWithTracking(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride string, tracker *FileTracker) error { - return compileWorkflowWithTrackingAndRefresh(ctx, filePath, verbose, quiet, engineOverride, tracker, false) + return compileWorkflowWithTrackingAndActionRef(ctx, filePath, verbose, quiet, engineOverride, "", tracker) +} + +func compileWorkflowWithTrackingAndActionRef(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride, actionRef string, tracker *FileTracker) error { + return compileWorkflowWithTrackingAndRefreshAndActionRef(ctx, filePath, verbose, quiet, engineOverride, actionRef, tracker, false) } // compileWorkflowWithTrackingAndRefresh compiles a workflow, tracks generated files, and optionally refreshes stop time. // This function ensures that the file tracker records all files created or modified during compilation. func compileWorkflowWithTrackingAndRefresh(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride string, tracker *FileTracker, refreshStopTime bool) error { + return compileWorkflowWithTrackingAndRefreshAndActionRef(ctx, filePath, verbose, quiet, engineOverride, "", tracker, refreshStopTime) +} + +func compileWorkflowWithTrackingAndRefreshAndActionRef(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride, actionRef string, tracker *FileTracker, refreshStopTime bool) error { addWorkflowCompilationLog.Printf("Compiling workflow with tracking: file=%s, refresh_stop_time=%v", filePath, refreshStopTime) // Generate the expected lock file path @@ -100,6 +117,7 @@ func compileWorkflowWithTrackingAndRefresh(ctx context.Context, filePath string, workflow.WithVerbose(verbose), workflow.WithEngineOverride(engineOverride), ) + applyAddActionRef(compiler, actionRef) compiler.SetFileTracker(tracker) compiler.SetRefreshStopTime(refreshStopTime) compiler.SetQuiet(quiet) @@ -129,6 +147,7 @@ func compileWorkflowWithTrackingAndRefresh(ctx context.Context, filePath string, type compileDepsOptions struct { verbose, quiet bool engineOverride string + actionRef string force bool propagateErrors bool tracker *FileTracker @@ -139,8 +158,12 @@ type compileDepsOptions struct { // called before compiling the main workflow, because the dispatch-workflow validator // requires every referenced .md workflow to have an up-to-date .lock.yml. func compileDispatchWorkflowDependencies(ctx context.Context, workflowFile string, verbose, quiet bool, engineOverride string, force bool, tracker *FileTracker) { + compileDispatchWorkflowDependenciesWithActionRef(ctx, workflowFile, verbose, quiet, engineOverride, "", force, tracker) +} + +func compileDispatchWorkflowDependenciesWithActionRef(ctx context.Context, workflowFile string, verbose, quiet bool, engineOverride, actionRef string, force bool, tracker *FileTracker) { compileSafeOutputsWorkflowDependencies(ctx, workflowFile, "dispatch-workflow dependency", dispatchWorkflowNamesForCompilation, compileDepsOptions{ - verbose: verbose, quiet: quiet, engineOverride: engineOverride, force: force, propagateErrors: false, tracker: tracker, + verbose: verbose, quiet: quiet, engineOverride: engineOverride, actionRef: actionRef, force: force, propagateErrors: false, tracker: tracker, }) } @@ -154,8 +177,12 @@ func compileDispatchWorkflowDependencies(ctx context.Context, workflowFile strin // worker whose lock cannot be produced would leave the orchestrator referencing a file that // does not exist. func compileCallWorkflowDependencies(ctx context.Context, workflowFile string, verbose, quiet bool, engineOverride string, force bool, tracker *FileTracker) error { + return compileCallWorkflowDependenciesWithActionRef(ctx, workflowFile, verbose, quiet, engineOverride, "", force, tracker) +} + +func compileCallWorkflowDependenciesWithActionRef(ctx context.Context, workflowFile string, verbose, quiet bool, engineOverride, actionRef string, force bool, tracker *FileTracker) error { return compileSafeOutputsWorkflowDependencies(ctx, workflowFile, "call-workflow worker", callWorkflowNamesForCompilation, compileDepsOptions{ - verbose: verbose, quiet: quiet, engineOverride: engineOverride, force: force, propagateErrors: true, tracker: tracker, + verbose: verbose, quiet: quiet, engineOverride: engineOverride, actionRef: actionRef, force: force, propagateErrors: true, tracker: tracker, }) } @@ -195,9 +222,9 @@ func compileSafeOutputsWorkflowDependencies(ctx context.Context, workflowFile, l var compileErr error if opts.tracker != nil { - compileErr = compileWorkflowWithTracking(ctx, mdPath, opts.verbose, opts.quiet, opts.engineOverride, opts.tracker) + compileErr = compileWorkflowWithTrackingAndActionRef(ctx, mdPath, opts.verbose, opts.quiet, opts.engineOverride, opts.actionRef, opts.tracker) } else { - compileErr = compileWorkflow(ctx, mdPath, opts.verbose, opts.quiet, opts.engineOverride) + compileErr = compileWorkflowWithActionRef(ctx, mdPath, opts.verbose, opts.quiet, opts.engineOverride, opts.actionRef) } if compileErr != nil { if opts.propagateErrors { @@ -212,6 +239,14 @@ func compileSafeOutputsWorkflowDependencies(ctx context.Context, workflowFile, l return nil } +func applyAddActionRef(compiler *workflow.Compiler, actionRef string) { + if actionRef == "" { + return + } + compiler.SetActionMode(workflow.ActionModeRelease) + compiler.SetActionTag(actionRef) +} + func callWorkflowNamesForCompilation(workflowFile string) []string { return safeOutputsWorkflowNamesForCompilation(workflowFile, "call-workflow", func(data *workflow.WorkflowData) []string { if data.SafeOutputs.CallWorkflow == nil { diff --git a/pkg/cli/copilot_billing_check.go b/pkg/cli/copilot_billing_check.go index 96156ef3500..f9fa632f020 100644 --- a/pkg/cli/copilot_billing_check.go +++ b/pkg/cli/copilot_billing_check.go @@ -17,7 +17,7 @@ const copilotBillingTimeout = 3 * time.Second // copilotBillingInconclusiveNote is the user-facing message printed when the // org's Copilot CLI billing status cannot be confirmed (non-200 response, // network error, missing field, or no org login available). -const copilotBillingInconclusiveNote = "Could not confirm org Copilot CLI billing — check with your org admin." +const copilotBillingInconclusiveNote = "Could not confirm org Copilot CLI billing — consider checking with your org admin if this is an option." // detectOrgCopilotCLIBillingWithClient calls GET /orgs/{org}/copilot/billing with // a 3 s timeout and returns the raw "cli" field. Any non-200 response or error From 3f7675b31e29360c7f38a979e3ef3a5772a9b1c0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:21:47 +0000 Subject: [PATCH 03/20] Plan review-feedback follow-up Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- .github/workflows/smoke-claude.lock.yml | 6 +++++- .github/workflows/step-name-alignment.lock.yml | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/smoke-claude.lock.yml b/.github/workflows/smoke-claude.lock.yml index 5df0a0ba4ad..bd609de57f0 100644 --- a/.github/workflows/smoke-claude.lock.yml +++ b/.github/workflows/smoke-claude.lock.yml @@ -1549,6 +1549,7 @@ jobs: # - Bash # - BashOutput # - Edit + # - Edit(/tmp/*) # - Edit(/tmp/gh-aw/agent/*) # - Edit(/tmp/gh-aw/cache-memory/*) # - ExitPlanMode @@ -1557,16 +1558,19 @@ jobs: # - KillBash # - LS # - MultiEdit + # - MultiEdit(/tmp/*) # - MultiEdit(/tmp/gh-aw/agent/*) # - MultiEdit(/tmp/gh-aw/cache-memory/*) # - NotebookEdit # - NotebookRead # - Read + # - Read(/tmp/*) # - Read(/tmp/gh-aw/agent/*) # - Read(/tmp/gh-aw/cache-memory/*) # - Task # - TodoWrite # - Write + # - Write(/tmp/*) # - Write(/tmp/gh-aw/agent/*) # - Write(/tmp/gh-aw/cache-memory/*) # - mcp__agenticworkflows @@ -1667,7 +1671,7 @@ jobs: fi # shellcheck disable=SC1003,SC2016,SC2086 awf --config "${RUNNER_TEMP}/gh-aw/awf-config.json" --container-workdir "${GITHUB_WORKSPACE}" --mount "${RUNNER_TEMP}/gh-aw:${RUNNER_TEMP}/gh-aw:ro" --mount "${RUNNER_TEMP}/gh-aw:/host${RUNNER_TEMP}/gh-aw:ro" ${GH_AW_TOOL_CACHE_MOUNT:+--mount "$GH_AW_TOOL_CACHE_MOUNT"} ${GH_AW_DOCKER_HOST:+--docker-host "$GH_AW_DOCKER_HOST"} --tty --env-all --exclude-env ACTIONS_ID_TOKEN_REQUEST_TOKEN --exclude-env ACTIONS_ID_TOKEN_REQUEST_URL --exclude-env ANTHROPIC_API_KEY --exclude-env GH_TOKEN --exclude-env GITHUB_MCP_SERVER_TOKEN --exclude-env MCP_GATEWAY_API_KEY --exclude-env TAVILY_API_KEY --mount /tmp/gh-aw:/tmp/gh-aw:rw --log-level info --skip-pull --difc-proxy-host awmg-cli-proxy:18443 --difc-proxy-ca-cert /tmp/gh-aw/difc-proxy-tls/ca.crt \ - -- /bin/bash -c 'set +o histexpand; export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH" && : "${RUNNER_TOOL_CACHE:?RUNNER_TOOL_CACHE must be set}"; GH_AW_TOOL_CACHE="$RUNNER_TOOL_CACHE"; export PATH="$(find "$GH_AW_TOOL_CACHE" -maxdepth 5 -type d -name bin 2>/dev/null | tr '\''\n'\'' '\'':'\'')$PATH"; [ -n "$GOROOT" ] && export PATH="$GOROOT/bin:$PATH" || true; [ -n "$ERLANG_HOME" ] && export PATH="$ERLANG_HOME/bin:$PATH" || true && GH_AW_NODE_EXEC="${GH_AW_NODE_BIN:-}"; if [ -z "$GH_AW_NODE_EXEC" ] || [ ! -x "$GH_AW_NODE_EXEC" ]; then GH_AW_NODE_EXEC="$(command -v node 2>/dev/null || true)"; fi; if [ -z "$GH_AW_NODE_EXEC" ]; then echo "node runtime missing on this runner — check runtimes.node in workflow YAML" >&2; exit 127; fi; GH_AW_NPM_GLOBAL_ROOT="$(npm root -g 2>/dev/null || true)"; if [ -n "$GH_AW_NPM_GLOBAL_ROOT" ]; then export NODE_PATH="${GH_AW_NPM_GLOBAL_ROOT}${NODE_PATH:+:${NODE_PATH}}"; fi; "$GH_AW_NODE_EXEC" ${RUNNER_TEMP}/gh-aw/actions/claude_harness.cjs claude --print --no-chrome --max-turns 100 --allowed-tools '\''Bash,BashOutput,Edit,Edit(/tmp/gh-aw/agent/*),Edit(/tmp/gh-aw/cache-memory/*),ExitPlanMode,Glob,Grep,KillBash,LS,MultiEdit,MultiEdit(/tmp/gh-aw/agent/*),MultiEdit(/tmp/gh-aw/cache-memory/*),NotebookEdit,NotebookRead,Read,Read(/tmp/gh-aw/agent/*),Read(/tmp/gh-aw/cache-memory/*),Task,TodoWrite,Write,Write(/tmp/gh-aw/agent/*),Write(/tmp/gh-aw/cache-memory/*),mcp__agenticworkflows,mcp__github__actions_get,mcp__github__actions_list,mcp__github__get_code_scanning_alert,mcp__github__get_commit,mcp__github__get_dependabot_alert,mcp__github__get_discussion,mcp__github__get_discussion_comments,mcp__github__get_file_contents,mcp__github__get_job_logs,mcp__github__get_label,mcp__github__get_latest_release,mcp__github__get_me,mcp__github__get_notification_details,mcp__github__get_pull_request,mcp__github__get_pull_request_comments,mcp__github__get_pull_request_diff,mcp__github__get_pull_request_files,mcp__github__get_pull_request_review_comments,mcp__github__get_pull_request_reviews,mcp__github__get_pull_request_status,mcp__github__get_release_by_tag,mcp__github__get_secret_scanning_alert,mcp__github__get_tag,mcp__github__issue_read,mcp__github__list_branches,mcp__github__list_code_scanning_alerts,mcp__github__list_commits,mcp__github__list_dependabot_alerts,mcp__github__list_discussion_categories,mcp__github__list_discussions,mcp__github__list_issue_types,mcp__github__list_issues,mcp__github__list_label,mcp__github__list_notifications,mcp__github__list_pull_requests,mcp__github__list_releases,mcp__github__list_secret_scanning_alerts,mcp__github__list_starred_repositories,mcp__github__list_tags,mcp__github__pull_request_read,mcp__github__search_code,mcp__github__search_issues,mcp__github__search_orgs,mcp__github__search_pull_requests,mcp__github__search_repositories,mcp__github__search_users,mcp__mcpscripts,mcp__playwright__browser_click,mcp__playwright__browser_close,mcp__playwright__browser_console_messages,mcp__playwright__browser_drag,mcp__playwright__browser_evaluate,mcp__playwright__browser_file_upload,mcp__playwright__browser_fill_form,mcp__playwright__browser_handle_dialog,mcp__playwright__browser_hover,mcp__playwright__browser_install,mcp__playwright__browser_navigate,mcp__playwright__browser_navigate_back,mcp__playwright__browser_network_requests,mcp__playwright__browser_press_key,mcp__playwright__browser_resize,mcp__playwright__browser_select_option,mcp__playwright__browser_snapshot,mcp__playwright__browser_tabs,mcp__playwright__browser_take_screenshot,mcp__playwright__browser_type,mcp__playwright__browser_wait_for,mcp__safeoutputs,mcp__tavily'\'' --debug-file /tmp/gh-aw/agent/claude-debug.log --verbose --permission-mode acceptEdits --output-format stream-json --bare --mcp-config "${RUNNER_TEMP}/gh-aw/mcp-config/mcp-servers.json" --prompt-file /tmp/gh-aw/aw-prompts/prompt.txt' 2>&1 | tee -a /tmp/gh-aw/agent-stdio.log + -- /bin/bash -c 'set +o histexpand; export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH" && : "${RUNNER_TOOL_CACHE:?RUNNER_TOOL_CACHE must be set}"; GH_AW_TOOL_CACHE="$RUNNER_TOOL_CACHE"; export PATH="$(find "$GH_AW_TOOL_CACHE" -maxdepth 5 -type d -name bin 2>/dev/null | tr '\''\n'\'' '\'':'\'')$PATH"; [ -n "$GOROOT" ] && export PATH="$GOROOT/bin:$PATH" || true; [ -n "$ERLANG_HOME" ] && export PATH="$ERLANG_HOME/bin:$PATH" || true && GH_AW_NODE_EXEC="${GH_AW_NODE_BIN:-}"; if [ -z "$GH_AW_NODE_EXEC" ] || [ ! -x "$GH_AW_NODE_EXEC" ]; then GH_AW_NODE_EXEC="$(command -v node 2>/dev/null || true)"; fi; if [ -z "$GH_AW_NODE_EXEC" ]; then echo "node runtime missing on this runner — check runtimes.node in workflow YAML" >&2; exit 127; fi; GH_AW_NPM_GLOBAL_ROOT="$(npm root -g 2>/dev/null || true)"; if [ -n "$GH_AW_NPM_GLOBAL_ROOT" ]; then export NODE_PATH="${GH_AW_NPM_GLOBAL_ROOT}${NODE_PATH:+:${NODE_PATH}}"; fi; "$GH_AW_NODE_EXEC" ${RUNNER_TEMP}/gh-aw/actions/claude_harness.cjs claude --print --no-chrome --max-turns 100 --allowed-tools '\''Bash,BashOutput,Edit,Edit(/tmp/*),Edit(/tmp/gh-aw/agent/*),Edit(/tmp/gh-aw/cache-memory/*),ExitPlanMode,Glob,Grep,KillBash,LS,MultiEdit,MultiEdit(/tmp/*),MultiEdit(/tmp/gh-aw/agent/*),MultiEdit(/tmp/gh-aw/cache-memory/*),NotebookEdit,NotebookRead,Read,Read(/tmp/*),Read(/tmp/gh-aw/agent/*),Read(/tmp/gh-aw/cache-memory/*),Task,TodoWrite,Write,Write(/tmp/*),Write(/tmp/gh-aw/agent/*),Write(/tmp/gh-aw/cache-memory/*),mcp__agenticworkflows,mcp__github__actions_get,mcp__github__actions_list,mcp__github__get_code_scanning_alert,mcp__github__get_commit,mcp__github__get_dependabot_alert,mcp__github__get_discussion,mcp__github__get_discussion_comments,mcp__github__get_file_contents,mcp__github__get_job_logs,mcp__github__get_label,mcp__github__get_latest_release,mcp__github__get_me,mcp__github__get_notification_details,mcp__github__get_pull_request,mcp__github__get_pull_request_comments,mcp__github__get_pull_request_diff,mcp__github__get_pull_request_files,mcp__github__get_pull_request_review_comments,mcp__github__get_pull_request_reviews,mcp__github__get_pull_request_status,mcp__github__get_release_by_tag,mcp__github__get_secret_scanning_alert,mcp__github__get_tag,mcp__github__issue_read,mcp__github__list_branches,mcp__github__list_code_scanning_alerts,mcp__github__list_commits,mcp__github__list_dependabot_alerts,mcp__github__list_discussion_categories,mcp__github__list_discussions,mcp__github__list_issue_types,mcp__github__list_issues,mcp__github__list_label,mcp__github__list_notifications,mcp__github__list_pull_requests,mcp__github__list_releases,mcp__github__list_secret_scanning_alerts,mcp__github__list_starred_repositories,mcp__github__list_tags,mcp__github__pull_request_read,mcp__github__search_code,mcp__github__search_issues,mcp__github__search_orgs,mcp__github__search_pull_requests,mcp__github__search_repositories,mcp__github__search_users,mcp__mcpscripts,mcp__playwright__browser_click,mcp__playwright__browser_close,mcp__playwright__browser_console_messages,mcp__playwright__browser_drag,mcp__playwright__browser_evaluate,mcp__playwright__browser_file_upload,mcp__playwright__browser_fill_form,mcp__playwright__browser_handle_dialog,mcp__playwright__browser_hover,mcp__playwright__browser_install,mcp__playwright__browser_navigate,mcp__playwright__browser_navigate_back,mcp__playwright__browser_network_requests,mcp__playwright__browser_press_key,mcp__playwright__browser_resize,mcp__playwright__browser_select_option,mcp__playwright__browser_snapshot,mcp__playwright__browser_tabs,mcp__playwright__browser_take_screenshot,mcp__playwright__browser_type,mcp__playwright__browser_wait_for,mcp__safeoutputs,mcp__tavily'\'' --debug-file /tmp/gh-aw/agent/claude-debug.log --verbose --permission-mode acceptEdits --output-format stream-json --bare --mcp-config "${RUNNER_TEMP}/gh-aw/mcp-config/mcp-servers.json" --prompt-file /tmp/gh-aw/aw-prompts/prompt.txt' 2>&1 | tee -a /tmp/gh-aw/agent-stdio.log env: ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} ANTHROPIC_MAX_RETRIES: 0 diff --git a/.github/workflows/step-name-alignment.lock.yml b/.github/workflows/step-name-alignment.lock.yml index 79ea464dff2..621b8c694a6 100644 --- a/.github/workflows/step-name-alignment.lock.yml +++ b/.github/workflows/step-name-alignment.lock.yml @@ -871,6 +871,7 @@ jobs: # - Bash(yq*) # - BashOutput # - Edit + # - Edit(/tmp/*) # - Edit(/tmp/gh-aw/agent/*) # - Edit(/tmp/gh-aw/cache-memory/*) # - ExitPlanMode @@ -879,16 +880,19 @@ jobs: # - KillBash # - LS # - MultiEdit + # - MultiEdit(/tmp/*) # - MultiEdit(/tmp/gh-aw/agent/*) # - MultiEdit(/tmp/gh-aw/cache-memory/*) # - NotebookEdit # - NotebookRead # - Read + # - Read(/tmp/*) # - Read(/tmp/gh-aw/agent/*) # - Read(/tmp/gh-aw/cache-memory/*) # - Task # - TodoWrite # - Write + # - Write(/tmp/*) # - Write(/tmp/gh-aw/agent/*) # - Write(/tmp/gh-aw/cache-memory/*) # - mcp__github__actions_get @@ -965,7 +969,7 @@ jobs: fi # shellcheck disable=SC1003,SC2016,SC2086 awf --config "${RUNNER_TEMP}/gh-aw/awf-config.json" --container-workdir "${GITHUB_WORKSPACE}" --mount "${RUNNER_TEMP}/gh-aw:${RUNNER_TEMP}/gh-aw:ro" --mount "${RUNNER_TEMP}/gh-aw:/host${RUNNER_TEMP}/gh-aw:ro" ${GH_AW_TOOL_CACHE_MOUNT:+--mount "$GH_AW_TOOL_CACHE_MOUNT"} ${GH_AW_DOCKER_HOST:+--docker-host "$GH_AW_DOCKER_HOST"} --tty --env-all --exclude-env ACTIONS_ID_TOKEN_REQUEST_TOKEN --exclude-env ACTIONS_ID_TOKEN_REQUEST_URL --exclude-env ANTHROPIC_API_KEY --exclude-env GH_TOKEN --exclude-env GITHUB_MCP_SERVER_TOKEN --exclude-env MCP_GATEWAY_API_KEY --mount /tmp/gh-aw:/tmp/gh-aw:rw --log-level info --skip-pull --difc-proxy-host awmg-cli-proxy:18443 --difc-proxy-ca-cert /tmp/gh-aw/difc-proxy-tls/ca.crt \ - -- /bin/bash -c 'set +o histexpand; export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH" && : "${RUNNER_TOOL_CACHE:?RUNNER_TOOL_CACHE must be set}"; GH_AW_TOOL_CACHE="$RUNNER_TOOL_CACHE"; export PATH="$(find "$GH_AW_TOOL_CACHE" -maxdepth 5 -type d -name bin 2>/dev/null | tr '\''\n'\'' '\'':'\'')$PATH"; [ -n "$GOROOT" ] && export PATH="$GOROOT/bin:$PATH" || true; [ -n "$ERLANG_HOME" ] && export PATH="$ERLANG_HOME/bin:$PATH" || true && GH_AW_NODE_EXEC="${GH_AW_NODE_BIN:-}"; if [ -z "$GH_AW_NODE_EXEC" ] || [ ! -x "$GH_AW_NODE_EXEC" ]; then GH_AW_NODE_EXEC="$(command -v node 2>/dev/null || true)"; fi; if [ -z "$GH_AW_NODE_EXEC" ]; then echo "node runtime missing on this runner — check runtimes.node in workflow YAML" >&2; exit 127; fi; GH_AW_NPM_GLOBAL_ROOT="$(npm root -g 2>/dev/null || true)"; if [ -n "$GH_AW_NPM_GLOBAL_ROOT" ]; then export NODE_PATH="${GH_AW_NPM_GLOBAL_ROOT}${NODE_PATH:+:${NODE_PATH}}"; fi; "$GH_AW_NODE_EXEC" ${RUNNER_TEMP}/gh-aw/actions/claude_harness.cjs claude --print --no-chrome --max-turns 50 --allowed-tools '\''Bash(cat /tmp/gh-aw/agent/step-alignment-input.json),Bash(cat /tmp/gh-aw/cache-memory/),Bash(cat > /tmp/gh-aw/cache-memory/),Bash(cat docs/src/content/docs/reference/glossary.md),Bash(cat),Bash(date),Bash(echo),Bash(find .github/workflows -name "*.lock.yml" -type f),Bash(gh:*),Bash(git log --since="24 hours ago" --oneline --name-only -- ".github/workflows/*.lock.yml"),Bash(grep),Bash(head),Bash(jq* /tmp/gh-aw/agent/step-alignment-input.json),Bash(ls),Bash(mkdir -p /tmp/gh-aw/cache-memory/),Bash(mv /tmp/gh-aw/cache-memory/),Bash(printf),Bash(pwd),Bash(safeoutputs:*),Bash(sort),Bash(tail),Bash(uniq),Bash(wc),Bash(yq),Bash(yq*),BashOutput,Edit,Edit(/tmp/gh-aw/agent/*),Edit(/tmp/gh-aw/cache-memory/*),ExitPlanMode,Glob,Grep,KillBash,LS,MultiEdit,MultiEdit(/tmp/gh-aw/agent/*),MultiEdit(/tmp/gh-aw/cache-memory/*),NotebookEdit,NotebookRead,Read,Read(/tmp/gh-aw/agent/*),Read(/tmp/gh-aw/cache-memory/*),Task,TodoWrite,Write,Write(/tmp/gh-aw/agent/*),Write(/tmp/gh-aw/cache-memory/*),mcp__github__actions_get,mcp__github__actions_list,mcp__github__get_code_scanning_alert,mcp__github__get_commit,mcp__github__get_dependabot_alert,mcp__github__get_discussion,mcp__github__get_discussion_comments,mcp__github__get_file_contents,mcp__github__get_job_logs,mcp__github__get_label,mcp__github__get_latest_release,mcp__github__get_me,mcp__github__get_notification_details,mcp__github__get_pull_request,mcp__github__get_pull_request_comments,mcp__github__get_pull_request_diff,mcp__github__get_pull_request_files,mcp__github__get_pull_request_review_comments,mcp__github__get_pull_request_reviews,mcp__github__get_pull_request_status,mcp__github__get_release_by_tag,mcp__github__get_secret_scanning_alert,mcp__github__get_tag,mcp__github__issue_read,mcp__github__list_branches,mcp__github__list_code_scanning_alerts,mcp__github__list_commits,mcp__github__list_dependabot_alerts,mcp__github__list_discussion_categories,mcp__github__list_discussions,mcp__github__list_issue_types,mcp__github__list_issues,mcp__github__list_label,mcp__github__list_notifications,mcp__github__list_pull_requests,mcp__github__list_releases,mcp__github__list_secret_scanning_alerts,mcp__github__list_starred_repositories,mcp__github__list_tags,mcp__github__pull_request_read,mcp__github__search_code,mcp__github__search_issues,mcp__github__search_orgs,mcp__github__search_pull_requests,mcp__github__search_repositories,mcp__github__search_users,mcp__safeoutputs'\'' --debug-file /tmp/gh-aw/agent/claude-debug.log --verbose --permission-mode acceptEdits --output-format stream-json --mcp-config "${RUNNER_TEMP}/gh-aw/mcp-config/mcp-servers.json" --prompt-file /tmp/gh-aw/aw-prompts/prompt.txt${GH_AW_MODEL_AGENT_CLAUDE:+ --model "$GH_AW_MODEL_AGENT_CLAUDE"}' 2>&1 | tee -a /tmp/gh-aw/agent-stdio.log + -- /bin/bash -c 'set +o histexpand; export PATH="${RUNNER_TEMP}/gh-aw/mcp-cli/bin:$PATH" && : "${RUNNER_TOOL_CACHE:?RUNNER_TOOL_CACHE must be set}"; GH_AW_TOOL_CACHE="$RUNNER_TOOL_CACHE"; export PATH="$(find "$GH_AW_TOOL_CACHE" -maxdepth 5 -type d -name bin 2>/dev/null | tr '\''\n'\'' '\'':'\'')$PATH"; [ -n "$GOROOT" ] && export PATH="$GOROOT/bin:$PATH" || true; [ -n "$ERLANG_HOME" ] && export PATH="$ERLANG_HOME/bin:$PATH" || true && GH_AW_NODE_EXEC="${GH_AW_NODE_BIN:-}"; if [ -z "$GH_AW_NODE_EXEC" ] || [ ! -x "$GH_AW_NODE_EXEC" ]; then GH_AW_NODE_EXEC="$(command -v node 2>/dev/null || true)"; fi; if [ -z "$GH_AW_NODE_EXEC" ]; then echo "node runtime missing on this runner — check runtimes.node in workflow YAML" >&2; exit 127; fi; GH_AW_NPM_GLOBAL_ROOT="$(npm root -g 2>/dev/null || true)"; if [ -n "$GH_AW_NPM_GLOBAL_ROOT" ]; then export NODE_PATH="${GH_AW_NPM_GLOBAL_ROOT}${NODE_PATH:+:${NODE_PATH}}"; fi; "$GH_AW_NODE_EXEC" ${RUNNER_TEMP}/gh-aw/actions/claude_harness.cjs claude --print --no-chrome --max-turns 50 --allowed-tools '\''Bash(cat /tmp/gh-aw/agent/step-alignment-input.json),Bash(cat /tmp/gh-aw/cache-memory/),Bash(cat > /tmp/gh-aw/cache-memory/),Bash(cat docs/src/content/docs/reference/glossary.md),Bash(cat),Bash(date),Bash(echo),Bash(find .github/workflows -name "*.lock.yml" -type f),Bash(gh:*),Bash(git log --since="24 hours ago" --oneline --name-only -- ".github/workflows/*.lock.yml"),Bash(grep),Bash(head),Bash(jq* /tmp/gh-aw/agent/step-alignment-input.json),Bash(ls),Bash(mkdir -p /tmp/gh-aw/cache-memory/),Bash(mv /tmp/gh-aw/cache-memory/),Bash(printf),Bash(pwd),Bash(safeoutputs:*),Bash(sort),Bash(tail),Bash(uniq),Bash(wc),Bash(yq),Bash(yq*),BashOutput,Edit,Edit(/tmp/*),Edit(/tmp/gh-aw/agent/*),Edit(/tmp/gh-aw/cache-memory/*),ExitPlanMode,Glob,Grep,KillBash,LS,MultiEdit,MultiEdit(/tmp/*),MultiEdit(/tmp/gh-aw/agent/*),MultiEdit(/tmp/gh-aw/cache-memory/*),NotebookEdit,NotebookRead,Read,Read(/tmp/*),Read(/tmp/gh-aw/agent/*),Read(/tmp/gh-aw/cache-memory/*),Task,TodoWrite,Write,Write(/tmp/*),Write(/tmp/gh-aw/agent/*),Write(/tmp/gh-aw/cache-memory/*),mcp__github__actions_get,mcp__github__actions_list,mcp__github__get_code_scanning_alert,mcp__github__get_commit,mcp__github__get_dependabot_alert,mcp__github__get_discussion,mcp__github__get_discussion_comments,mcp__github__get_file_contents,mcp__github__get_job_logs,mcp__github__get_label,mcp__github__get_latest_release,mcp__github__get_me,mcp__github__get_notification_details,mcp__github__get_pull_request,mcp__github__get_pull_request_comments,mcp__github__get_pull_request_diff,mcp__github__get_pull_request_files,mcp__github__get_pull_request_review_comments,mcp__github__get_pull_request_reviews,mcp__github__get_pull_request_status,mcp__github__get_release_by_tag,mcp__github__get_secret_scanning_alert,mcp__github__get_tag,mcp__github__issue_read,mcp__github__list_branches,mcp__github__list_code_scanning_alerts,mcp__github__list_commits,mcp__github__list_dependabot_alerts,mcp__github__list_discussion_categories,mcp__github__list_discussions,mcp__github__list_issue_types,mcp__github__list_issues,mcp__github__list_label,mcp__github__list_notifications,mcp__github__list_pull_requests,mcp__github__list_releases,mcp__github__list_secret_scanning_alerts,mcp__github__list_starred_repositories,mcp__github__list_tags,mcp__github__pull_request_read,mcp__github__search_code,mcp__github__search_issues,mcp__github__search_orgs,mcp__github__search_pull_requests,mcp__github__search_repositories,mcp__github__search_users,mcp__safeoutputs'\'' --debug-file /tmp/gh-aw/agent/claude-debug.log --verbose --permission-mode acceptEdits --output-format stream-json --mcp-config "${RUNNER_TEMP}/gh-aw/mcp-config/mcp-servers.json" --prompt-file /tmp/gh-aw/aw-prompts/prompt.txt${GH_AW_MODEL_AGENT_CLAUDE:+ --model "$GH_AW_MODEL_AGENT_CLAUDE"}' 2>&1 | tee -a /tmp/gh-aw/agent-stdio.log env: ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} ANTHROPIC_MAX_RETRIES: 0 From 37c58e4e37da4301948a312a48b172ade0812c2c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 16:25:20 +0000 Subject: [PATCH 04/20] Respect organization secret visibility Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- ...h-use-existing-copilot-token-add-wizard.md | 2 +- pkg/cli/add_interactive_secrets.go | 56 +++++++++++++++++-- pkg/cli/add_interactive_secrets_test.go | 43 +++++++++++--- 3 files changed, 87 insertions(+), 14 deletions(-) diff --git a/.changeset/patch-use-existing-copilot-token-add-wizard.md b/.changeset/patch-use-existing-copilot-token-add-wizard.md index 5facfbb9faf..2e9714103cd 100644 --- a/.changeset/patch-use-existing-copilot-token-add-wizard.md +++ b/.changeset/patch-use-existing-copilot-token-add-wizard.md @@ -2,4 +2,4 @@ "gh-aw": patch --- -Default add-wizard to using an existing `COPILOT_GITHUB_TOKEN` secret, improve workflow and Copilot authentication prompt formatting, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file +Default add-wizard to use an existing `COPILOT_GITHUB_TOKEN` secret, improve workflow and Copilot authentication prompt formatting, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file diff --git a/pkg/cli/add_interactive_secrets.go b/pkg/cli/add_interactive_secrets.go index 35caabf0ffe..784e4bab67a 100644 --- a/pkg/cli/add_interactive_secrets.go +++ b/pkg/cli/add_interactive_secrets.go @@ -2,6 +2,7 @@ package cli import ( "bytes" + "encoding/json" "fmt" "os" "strings" @@ -11,6 +12,17 @@ import ( "github.com/github/gh-aw/pkg/workflow" ) +var addInteractiveRunGH = workflow.RunGH + +type organizationSecret struct { + Name string `json:"name"` + Visibility string `json:"visibility"` +} + +type organizationSecretsResponse struct { + Secrets []organizationSecret `json:"secrets"` +} + // checkExistingSecrets fetches which secrets already exist in the repository or its organization func (c *AddInteractiveConfig) checkExistingSecrets() error { addInteractiveLog.Print("Checking existing repository secrets") @@ -18,7 +30,7 @@ func (c *AddInteractiveConfig) checkExistingSecrets() error { c.existingSecrets = make(map[string]struct{}) // Use gh api to list repository secrets - output, err := workflow.RunGH("Checking repository secrets...", "api", fmt.Sprintf("/repos/%s/actions/secrets", c.RepoOverride), "--jq", ".secrets[].name") + output, err := addInteractiveRunGH("Checking repository secrets...", "api", fmt.Sprintf("/repos/%s/actions/secrets", c.RepoOverride), "--jq", ".secrets[].name") if err != nil { addInteractiveLog.Printf("Could not fetch existing secrets: %v", err) // Continue without error - we'll just assume no secrets exist @@ -31,13 +43,20 @@ func (c *AddInteractiveConfig) checkExistingSecrets() error { // Also check org-level secrets if the repo belongs to an organization if org, _, found := strings.Cut(c.RepoOverride, "/"); found && org != "" { - orgOutput, orgErr := workflow.RunGH("Checking organization secrets...", "api", fmt.Sprintf("/orgs/%s/actions/secrets", org), "--jq", ".secrets[].name") + orgOutput, orgErr := addInteractiveRunGH("Checking organization secrets...", "api", fmt.Sprintf("/orgs/%s/actions/secrets", org)) if orgErr != nil { addInteractiveLog.Printf("Could not fetch org secrets (this is expected for personal repos or if org access is restricted): %v", orgErr) } else { - for _, name := range parseSecretNames(orgOutput) { - c.existingSecrets[name] = struct{}{} - addInteractiveLog.Printf("Found existing org secret: %s", name) + var response organizationSecretsResponse + if err := json.Unmarshal(orgOutput, &response); err != nil { + addInteractiveLog.Printf("Could not parse organization secrets: %v", err) + } else { + for _, secret := range response.Secrets { + if c.organizationSecretAvailable(org, secret) { + c.existingSecrets[secret.Name] = struct{}{} + addInteractiveLog.Printf("Found available organization secret: %s", secret.Name) + } + } } } } @@ -49,6 +68,33 @@ func (c *AddInteractiveConfig) checkExistingSecrets() error { return nil } +func (c *AddInteractiveConfig) organizationSecretAvailable(org string, secret organizationSecret) bool { + switch secret.Visibility { + case "all": + return true + case "private": + return !c.isPublicRepo + case "selected": + output, err := addInteractiveRunGH( + "Checking organization secret repository access...", + "api", + fmt.Sprintf("/orgs/%s/actions/secrets/%s/repositories", org, secret.Name), + "--jq", + ".repositories[].full_name", + ) + if err != nil { + addInteractiveLog.Printf("Could not check repository access for organization secret %s: %v", secret.Name, err) + return false + } + return sliceutil.Any(parseSecretNames(output), func(repo string) bool { + return repo == c.RepoOverride + }) + default: + addInteractiveLog.Printf("Organization secret %s has unsupported visibility %q", secret.Name, secret.Visibility) + return false + } +} + // addRepositorySecret adds a secret to the repository func (c *AddInteractiveConfig) addRepositorySecret(name, value string) error { output, err := workflow.RunGHInputContext(c.Ctx, "Adding repository secret...", bytes.NewBufferString(value), "secret", "set", name, "--repo", c.RepoOverride) diff --git a/pkg/cli/add_interactive_secrets_test.go b/pkg/cli/add_interactive_secrets_test.go index 72710dd0ca4..7d47b479cae 100644 --- a/pkg/cli/add_interactive_secrets_test.go +++ b/pkg/cli/add_interactive_secrets_test.go @@ -295,15 +295,42 @@ func TestAddInteractiveConfig_addRepositorySecret_UsesStdinForSecretValue(t *tes } func TestAddInteractiveConfig_checkExistingSecrets(t *testing.T) { - config := &AddInteractiveConfig{ - RepoOverride: "test-owner/test-repo", + originalRunGH := addInteractiveRunGH + t.Cleanup(func() { addInteractiveRunGH = originalRunGH }) + + addInteractiveRunGH = func(_ string, args ...string) ([]byte, error) { + switch args[1] { + case "/repos/test-owner/test-repo/actions/secrets": + return []byte("REPOSITORY_SECRET\n"), nil + case "/orgs/test-owner/actions/secrets": + return []byte(`{"secrets":[ + {"name":"ALL_SECRET","visibility":"all"}, + {"name":"PRIVATE_SECRET","visibility":"private"}, + {"name":"SELECTED_SECRET","visibility":"selected"}, + {"name":"INACCESSIBLE_SECRET","visibility":"selected"} + ]}`), nil + case "/orgs/test-owner/actions/secrets/SELECTED_SECRET/repositories": + return []byte("test-owner/test-repo\n"), nil + case "/orgs/test-owner/actions/secrets/INACCESSIBLE_SECRET/repositories": + return []byte("test-owner/another-repo\n"), nil + default: + t.Fatalf("unexpected gh api arguments: %v", args) + return nil, nil + } } - // This test requires GitHub CLI access, so we just verify it doesn't panic - // and initializes the existingSecrets map - require.NotPanics(t, func() { - _ = config.checkExistingSecrets() - }, "checkExistingSecrets should not panic") + config := &AddInteractiveConfig{RepoOverride: "test-owner/test-repo"} + require.NoError(t, config.checkExistingSecrets()) + + assert.Contains(t, config.existingSecrets, "REPOSITORY_SECRET") + assert.Contains(t, config.existingSecrets, "ALL_SECRET") + assert.Contains(t, config.existingSecrets, "PRIVATE_SECRET") + assert.Contains(t, config.existingSecrets, "SELECTED_SECRET") + assert.NotContains(t, config.existingSecrets, "INACCESSIBLE_SECRET") - assert.NotNil(t, config.existingSecrets, "existingSecrets map should be initialized") + config.isPublicRepo = true + assert.False(t, config.organizationSecretAvailable("test-owner", organizationSecret{ + Name: "PRIVATE_SECRET", + Visibility: "private", + })) } From 9d306b5a6b7f55cfbce6aa9a6dcb815644da1112 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 18:34:23 +0200 Subject: [PATCH 05/20] Add animated add-wizard welcome --- ...h-use-existing-copilot-token-add-wizard.md | 2 +- docs/src/content/docs/setup/cli.md | 4 +- pkg/cli/add_command_test.go | 85 +++++++++++++ pkg/cli/add_init.go | 66 ++++++++++ pkg/cli/add_interactive_orchestrator.go | 10 +- pkg/cli/init.go | 32 ++--- pkg/console/README.md | 2 + pkg/console/brand_intro.go | 117 ++++++++++++++++++ pkg/console/brand_intro_test.go | 31 +++++ pkg/console/terminal.go | 10 ++ 10 files changed, 337 insertions(+), 22 deletions(-) create mode 100644 pkg/console/brand_intro.go create mode 100644 pkg/console/brand_intro_test.go diff --git a/.changeset/patch-use-existing-copilot-token-add-wizard.md b/.changeset/patch-use-existing-copilot-token-add-wizard.md index 2e9714103cd..ff52420fd7e 100644 --- a/.changeset/patch-use-existing-copilot-token-add-wizard.md +++ b/.changeset/patch-use-existing-copilot-token-add-wizard.md @@ -2,4 +2,4 @@ "gh-aw": patch --- -Default add-wizard to use an existing `COPILOT_GITHUB_TOKEN` secret, improve workflow and Copilot authentication prompt formatting, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file +Default add-wizard to using an existing `COPILOT_GITHUB_TOKEN` secret, make repository authoring support files optional, add a reusable animated welcome logo, improve workflow and Copilot authentication prompt formatting, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file diff --git a/docs/src/content/docs/setup/cli.md b/docs/src/content/docs/setup/cli.md index 3211eecf161..11458ed6a0e 100644 --- a/docs/src/content/docs/setup/cli.md +++ b/docs/src/content/docs/setup/cli.md @@ -191,6 +191,8 @@ The engine chosen at `init` time does not restrict workflows: every workflow sel Add a workflow with interactive guided setup. Checks requirements, adds the markdown file, and generates the compiled YAML. Prompts for missing API keys and secrets. For remote workflows, this command follows frontmatter [`redirect`](/gh-aw/reference/frontmatter/#redirect-redirect) declarations before installation. +Before the final pull request confirmation, the wizard optionally offers to add repository support files for using coding agents to author, debug, update, and audit agentic workflows. The prompt is skipped when those support files are already configured. Declining adds only the selected workflow files. + ```bash wrap gh aw add-wizard githubnext/agentics/ci-doctor # Interactive setup gh aw add-wizard https://github.com/org/repo/blob/main/workflows/my-workflow.md @@ -200,7 +202,7 @@ gh aw add-wizard githubnext/agentics/ci-doctor --no-secret # Skip secret prompt **Options:** `--no-secret`, `--dir/-d`, `--engine/-e`, `--gh-aw-ref`, `--no-gitattributes`, `--no-stop-after`, `--stop-after`, `--append`, `--no-security-scanner`, `--no-config` -When the Copilot engine is selected, the wizard prompts the user to choose an authentication method: organization billing via [`permissions.copilot-requests: write`](/gh-aw/reference/auth/#copilot-requests-write-permission) (no PAT required), or a [`COPILOT_GITHUB_TOKEN`](/gh-aw/reference/auth/#copilot_github_token) personal access token (a separate token from the default `GITHUB_TOKEN`, because the agent needs elevated Copilot API access that the ephemeral workflow token does not carry). On the PAT path, the wizard auto-opens a preconfigured fine-grained PAT creation page (prefilled token name, expiration, and Copilot Requests permission). The GitHub page still must be completed manually in the browser. Users may paste either an existing suitable fine-grained PAT or a newly created one into the masked CLI prompt, but reuse should be based on the token's properties: personal-account resource owner, repository access set to Public repositories, and Copilot Requests permission available. If `COPILOT_GITHUB_TOKEN` already exists, the wizard still asks for the token again because GitHub does not expose stored secret values for validation. The flow does not rely on the PAT display name in GitHub's token list. The pasted token is then validated and stored as a repository secret. +When the Copilot engine is selected, the wizard prompts the user to choose an authentication method: organization billing via [`permissions.copilot-requests: write`](/gh-aw/reference/auth/#copilot-requests-write-permission) (no PAT required), or a [`COPILOT_GITHUB_TOKEN`](/gh-aw/reference/auth/#copilot_github_token) personal access token (a separate token from the default `GITHUB_TOKEN`, because the agent needs elevated Copilot API access that the ephemeral workflow token does not carry). When `COPILOT_GITHUB_TOKEN` already exists, using it is the default; because GitHub does not expose stored secret values, the user asserts that it is a suitable fine-grained PAT with Copilot Requests permission. The alternative replacement path opens a preconfigured fine-grained PAT creation page, validates the pasted token, and updates the repository secret. #### `add` diff --git a/pkg/cli/add_command_test.go b/pkg/cli/add_command_test.go index 41901b0951a..c583f297158 100644 --- a/pkg/cli/add_command_test.go +++ b/pkg/cli/add_command_test.go @@ -556,6 +556,91 @@ func TestEnsureAddRepositoryInitializedWithDetails_AbsolutePaths(t *testing.T) { require.Equal(t, filepath.Join(repoDir, filepath.FromSlash(writtenMarker)), files[0]) } +func TestConfirmAndInitializeAddRepository(t *testing.T) { + originalFindGitRoot := addFindGitRoot + originalInitRepository := addInitRepository + originalMissingInitMarkers := addMissingInitMarkers + originalMissingAuthoringSupportFiles := addMissingAuthoringSupportFiles + originalConfirmAuthoringSupport := addConfirmAuthoringSupport + t.Cleanup(func() { + addFindGitRoot = originalFindGitRoot + addInitRepository = originalInitRepository + addMissingInitMarkers = originalMissingInitMarkers + addMissingAuthoringSupportFiles = originalMissingAuthoringSupportFiles + addConfirmAuthoringSupport = originalConfirmAuthoringSupport + }) + + repoDir := t.TempDir() + addFindGitRoot = func() (string, error) { return repoDir, nil } + + t.Run("already initialized skips confirmation", func(t *testing.T) { + addMissingAuthoringSupportFiles = func(string, string, bool) ([]string, error) { return nil, nil } + addConfirmAuthoringSupport = func(context.Context) (bool, error) { + t.Fatal("confirmation should not be shown when all support files exist") + return false, nil + } + addInitRepository = func(InitOptions) error { + t.Fatal("initialization should not run when all support files exist") + return nil + } + + files, err := confirmAndInitializeAddRepository(context.Background(), "copilot", false, false) + require.NoError(t, err) + assert.Empty(t, files) + }) + + t.Run("declining creates no support files", func(t *testing.T) { + addMissingAuthoringSupportFiles = func(string, string, bool) ([]string, error) { + return []string{bootstrapAgenticSkillPath}, nil + } + addConfirmAuthoringSupport = func(context.Context) (bool, error) { return false, nil } + addInitRepository = func(InitOptions) error { + t.Fatal("initialization should not run after confirmation is declined") + return nil + } + + files, err := confirmAndInitializeAddRepository(context.Background(), "copilot", false, false) + require.NoError(t, err) + assert.Empty(t, files) + }) + + t.Run("accepting quietly initializes support files", func(t *testing.T) { + marker := ".vscode/settings.json" + addMissingAuthoringSupportFiles = func(string, string, bool) ([]string, error) { return []string{marker}, nil } + addMissingInitMarkers = func(string, string) ([]string, error) { return []string{marker}, nil } + addConfirmAuthoringSupport = func(context.Context) (bool, error) { return true, nil } + addInitRepository = func(opts InitOptions) error { + assert.True(t, opts.Quiet) + assert.Equal(t, "copilot", opts.Engine) + path := filepath.Join(repoDir, filepath.FromSlash(marker)) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0755)) + return os.WriteFile(path, []byte(`{}`), 0644) + } + + files, err := confirmAndInitializeAddRepository(context.Background(), "copilot", false, false) + require.NoError(t, err) + require.Equal(t, []string{filepath.Join(repoDir, filepath.FromSlash(marker))}, files) + }) +} + +func TestMissingAddAuthoringSupportFiles(t *testing.T) { + repoDir := t.TempDir() + for _, path := range expectedBootstrapInitMarkers("copilot") { + fullPath := filepath.Join(repoDir, filepath.FromSlash(path)) + require.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0755)) + require.NoError(t, os.WriteFile(fullPath, nil, 0644)) + } + + missing, err := missingAddAuthoringSupportFiles(repoDir, "copilot", false) + require.NoError(t, err) + assert.Empty(t, missing, "existing support files should suppress the optional setup prompt") + + require.NoError(t, os.Remove(filepath.Join(repoDir, ".gitattributes"))) + missing, err = missingAddAuthoringSupportFiles(repoDir, "copilot", true) + require.NoError(t, err) + assert.Empty(t, missing, "--no-gitattributes should not require .gitattributes") +} + func TestAddResolvedWorkflows_IgnoresBootstrapRequireOwnerTypeDuringInstall(t *testing.T) { originalCheckOwnerType := bootstrapCheckOwnerType t.Cleanup(func() { diff --git a/pkg/cli/add_init.go b/pkg/cli/add_init.go index 95f2d54d649..428be6187ae 100644 --- a/pkg/cli/add_init.go +++ b/pkg/cli/add_init.go @@ -1,16 +1,81 @@ package cli import ( + "context" "errors" "fmt" + "os" "path/filepath" + "charm.land/huh/v2" + "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/gitutil" ) var addFindGitRoot = gitutil.FindGitRoot var addInitRepository = InitRepository var addMissingInitMarkers = missingBootstrapInitMarkers +var addMissingAuthoringSupportFiles = missingAddAuthoringSupportFiles +var addConfirmAuthoringSupport = func(ctx context.Context) (bool, error) { + addAuthoringSupport := true + form := console.NewConfirmForm( + huh.NewConfirm(). + Title("Would you also like to add support to use coding agents in this repository to author, debug, update and audit agentic workflows?"). + Affirmative("Yes, add coding agent support"). + Negative("No, add only the workflow"). + Value(&addAuthoringSupport), + ) + if err := form.RunWithContext(ctx); err != nil { + return false, fmt.Errorf("coding agent support confirmation failed: %w", err) + } + return addAuthoringSupport, nil +} + +func missingAddAuthoringSupportFiles(baseDir string, engineOverride string, noGitattributes bool) ([]string, error) { + var missing []string + for _, path := range expectedBootstrapInitMarkers(engineOverride) { + if noGitattributes && path == ".gitattributes" { + continue + } + info, err := os.Stat(filepath.Join(baseDir, filepath.FromSlash(path))) + if err == nil && info.Mode().IsRegular() { + continue + } + if err != nil && !errors.Is(err, os.ErrNotExist) { + return nil, fmt.Errorf("failed to inspect %s: %w", path, err) + } + missing = append(missing, path) + } + return missing, nil +} + +func confirmAndInitializeAddRepository(ctx context.Context, engineOverride string, verbose bool, noGitattributes bool) ([]string, error) { + gitRoot, err := addFindGitRoot() + if err != nil { + if errors.Is(err, gitutil.ErrNotGitRepository) { + return nil, nil + } + return nil, fmt.Errorf("failed to determine repository root for automatic initialization: %w", err) + } + + var missingMarkers []string + if err := withWorkingDir(gitRoot, func() error { + var inspectErr error + missingMarkers, inspectErr = addMissingAuthoringSupportFiles(".", engineOverride, noGitattributes) + return inspectErr + }); err != nil { + return nil, fmt.Errorf("failed to inspect repository initialization state: %w", err) + } + if len(missingMarkers) == 0 { + return nil, nil + } + + confirmed, err := addConfirmAuthoringSupport(ctx) + if err != nil || !confirmed { + return nil, err + } + return ensureAddRepositoryInitializedWithDetails(engineOverride, verbose, noGitattributes) +} func ensureAddRepositoryInitialized(engineOverride string, verbose bool, noGitattributes bool) error { _, err := ensureAddRepositoryInitializedWithDetails(engineOverride, verbose, noGitattributes) @@ -40,6 +105,7 @@ func ensureAddRepositoryInitializedWithDetails(engineOverride string, verbose bo addLog.Printf("Repository missing init markers; running init: %v", missingMarkers) if err := addInitRepository(InitOptions{ Verbose: verbose, + Quiet: true, Engine: engineOverride, NoGitattributes: noGitattributes, Skill: true, diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index 54328c9221a..e92dc428616 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -182,7 +182,7 @@ func (c *AddInteractiveConfig) runInitialAddInteractiveChecks() error { if err := c.resolveWorkflows(); err != nil { return err } - console.ShowWelcomeBanner(c.welcomeMessage()) + console.ShowAnimatedWelcomeBanner(c.welcomeMessage()) c.showWorkflowDescriptions() if err := c.checkGHAuthStatus(); err != nil { return err @@ -218,17 +218,17 @@ func (c *AddInteractiveConfig) prepareAndConfirmAddInteractive() (workflowFiles, return nil, nil, "", "", false, err } - initFiles, err = ensureAddRepositoryInitializedWithDetails(c.EngineOverride, c.Verbose, c.NoGitattributes) + workflowFiles, _, err = c.determineFilesToAdd() if err != nil { return nil, nil, "", "", false, err } - workflowFiles, _, err = c.determineFilesToAdd() - if err != nil { + if err := c.selectScheduleFrequency(); err != nil { return nil, nil, "", "", false, err } - if err := c.selectScheduleFrequency(); err != nil { + initFiles, err = confirmAndInitializeAddRepository(c.Ctx, c.EngineOverride, c.Verbose, c.NoGitattributes) + if err != nil { return nil, nil, "", "", false, err } diff --git a/pkg/cli/init.go b/pkg/cli/init.go index 90fea41ed9a..074e0525d33 100644 --- a/pkg/cli/init.go +++ b/pkg/cli/init.go @@ -25,6 +25,7 @@ var initLog = logger.New("cli:init") type InitOptions struct { Ctx context.Context Verbose bool + Quiet bool Engine string NoGitattributes bool Skill bool @@ -44,11 +45,11 @@ func InitRepository(opts InitOptions) error { ctx := ctxutil.OrBackground(opts.Ctx) copilotArtifactsEnabled := opts.Engine == "copilot" - // Show welcome banner for interactive mode - console.ShowWelcomeBanner("This tool will initialize your repository for GitHub Agentic Workflows.") - - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Setting up repository...")) - fmt.Fprintln(os.Stderr, "") + if !opts.Quiet { + console.ShowWelcomeBanner("This tool will initialize your repository for GitHub Agentic Workflows.") + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Setting up repository...")) + fmt.Fprintln(os.Stderr, "") + } // If --create-pull-request is enabled, run pre-flight checks before doing any work if opts.CreatePR { @@ -219,18 +220,19 @@ func InitRepository(opts InitOptions) error { } } - // Display success message with next steps - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Repository initialized for agentic workflows!")) - fmt.Fprintln(os.Stderr, "") - if len(opts.CodespaceRepos) > 0 { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("GitHub Codespaces devcontainer configured")) + if !opts.Quiet { + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Repository initialized for agentic workflows!")) + fmt.Fprintln(os.Stderr, "") + if len(opts.CodespaceRepos) > 0 { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("GitHub Codespaces devcontainer configured")) + fmt.Fprintln(os.Stderr, "") + } + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("To create a workflow, see https://github.github.com/gh-aw/setup/creating-workflows")) + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Or add an example workflow, see https://github.com/githubnext/agentics")) fmt.Fprintln(os.Stderr, "") } - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("To create a workflow, see https://github.github.com/gh-aw/setup/creating-workflows")) - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Or add an example workflow, see https://github.com/githubnext/agentics")) - fmt.Fprintln(os.Stderr, "") return nil } diff --git a/pkg/console/README.md b/pkg/console/README.md index 32c93215d8b..adb4fdac3fe 100644 --- a/pkg/console/README.md +++ b/pkg/console/README.md @@ -101,6 +101,8 @@ The package is designed to adapt to the execution environment. Native builds det | `SetTimeLocation` | `func SetTimeLocation(location *time.Location)` | Sets the location used when rendering `time.Time` values. | | `ShowInteractiveList` | `func ShowInteractiveList(title string, items []ListItem) (string, error)` | Shows a single-selection interactive list; native builds use `huh` and non-TTY mode falls back to numbered text input. | | `ShowWelcomeBanner` | `func ShowWelcomeBanner(description string)` | Clears the screen and prints the interactive welcome banner and description to stderr. | +| `ShowAnimatedWelcomeBanner` | `func ShowAnimatedWelcomeBanner(description string)` | Displays the standard welcome text beside the compact animated GH-AW logo. | +| `ShowAnimatedBrandIntro` | `func ShowAnimatedBrandIntro(title, description string)` | Displays the reusable compact GH-AW logo animation beside wizard intro text, with a static accessible fallback. | | `ToRelativePath` | `func ToRelativePath(path string) string` | Converts an absolute path to a cwd-relative display path when possible. | ### Constants diff --git a/pkg/console/brand_intro.go b/pkg/console/brand_intro.go new file mode 100644 index 00000000000..2f02e2411cd --- /dev/null +++ b/pkg/console/brand_intro.go @@ -0,0 +1,117 @@ +package console + +import ( + "fmt" + "io" + "strings" + "time" + + lipgloss "charm.land/lipgloss/v2" + "github.com/github/gh-aw/pkg/styles" + "github.com/github/gh-aw/pkg/tty" +) + +const brandIntroFrameDelay = 65 * time.Millisecond + +var brandLogoStyle = lipgloss.NewStyle(). + Bold(true). + Foreground(styles.ColorPurple) + +var brandLogoFrames = [][]string{ + { + "+-----+", + "| |", + "+--+--+", + " |", + " +----+", + " +-----+", + " | |", + " +-----+", + }, + { + "+-----+ .", + "| |", + "+--+--+", + " |", + " +----+", + " +-----+", + " | |", + " +-----+", + }, + { + "+-----+ *", + "| | *+*", + "+--+--+ *", + " |", + " +----+", + " +-----+", + " | |", + " +-----+", + }, + { + "+-----+ +", + "| | +++", + "+--+--+ +", + " |", + " +----+", + " +-----+", + " | |", + " +-----+", + }, +} + +// ShowAnimatedBrandIntro clears the screen and displays a compact animated +// GitHub Agentic Workflows mark beside the supplied title and description. +// Animation is disabled outside a TTY and in accessible mode. +func ShowAnimatedBrandIntro(title, description string) { + ClearScreen() + out := stderrWriter() + animate := tty.IsStderrTerminal() && !IsAccessibleMode() + lastFrame := len(brandLogoFrames) - 1 + + if animate { + for frame := range lastFrame { + printBrandIntroFrame(out, frame, "", "", true) + time.Sleep(brandIntroFrameDelay) + moveCursorToBrandIntroStart(out) + } + } + + printBrandIntroFrame(out, lastFrame, title, description, animate) + fmt.Fprintln(out) +} + +func printBrandIntroFrame(out io.Writer, frame int, title, description string, clearLines bool) { + lines := formatBrandIntroFrame(frame, title, description) + for _, line := range lines { + if clearLines { + fmt.Fprint(out, ansiClearLine) + } + fmt.Fprintln(out, line) + } +} + +func moveCursorToBrandIntroStart(out io.Writer) { + fmt.Fprintf(out, "\033[%dA\r", len(brandLogoFrames[0])) +} + +func formatBrandIntroFrame(frame int, title, description string) []string { + logo := brandLogoFrames[frame] + lines := make([]string, len(logo)) + for index, logoLine := range logo { + visibleWidth := len([]rune(logoLine)) + if tty.IsStderrTerminal() { + logoLine = brandLogoStyle.Render(logoLine) + } + lines[index] = padBrandLogoLine(logoLine, visibleWidth, 21) + } + + lines[1] += title + lines[3] += description + return lines +} + +func padBrandLogoLine(value string, visibleWidth, width int) string { + padding := max(1, width-visibleWidth) + return value + strings.Repeat(" ", padding) +} diff --git a/pkg/console/brand_intro_test.go b/pkg/console/brand_intro_test.go new file mode 100644 index 00000000000..571663de0bd --- /dev/null +++ b/pkg/console/brand_intro_test.go @@ -0,0 +1,31 @@ +//go:build !integration + +package console + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFormatBrandIntroFrame(t *testing.T) { + t.Setenv("TERM", "dumb") + + lines := formatBrandIntroFrame(len(brandLogoFrames)-1, "Welcome", "Add a workflow") + + require.Len(t, lines, 8) + assert.Contains(t, lines[0], "+-----+") + assert.Contains(t, lines[1], "+++", "final frame should show the logo sparkle") + assert.Contains(t, lines[1], "Welcome") + assert.Contains(t, lines[3], "Add a workflow") + assert.Contains(t, strings.Join(lines, "\n"), "+----+", "logo should show linked workflow nodes") +} + +func TestBrandLogoAnimationFramesKeepStableHeight(t *testing.T) { + for _, frame := range brandLogoFrames { + assert.Len(t, frame, len(brandLogoFrames[0])) + } + assert.NotEqual(t, brandLogoFrames[0], brandLogoFrames[len(brandLogoFrames)-1]) +} diff --git a/pkg/console/terminal.go b/pkg/console/terminal.go index f9eb3767c89..3eb50e09b8e 100644 --- a/pkg/console/terminal.go +++ b/pkg/console/terminal.go @@ -54,3 +54,13 @@ func ShowWelcomeBanner(description string) { fmt.Fprintln(out, description) fmt.Fprintln(out, "") } + +// ShowAnimatedWelcomeBanner displays the welcome text beside the reusable +// animated GitHub Agentic Workflows mark. +func ShowAnimatedWelcomeBanner(description string) { + header := "→ Welcome to GitHub Agentic Workflows!" + if tty.IsStderrTerminal() { + header = styles.Header.Render(header) + } + ShowAnimatedBrandIntro(header, description) +} From 18cf80b37edcd4b51851cdfb40996b3781f3271c Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 18:42:03 +0200 Subject: [PATCH 06/20] Remove add-wizard ASCII logo --- ...h-use-existing-copilot-token-add-wizard.md | 2 +- pkg/cli/add_interactive_orchestrator.go | 2 +- pkg/console/README.md | 2 - pkg/console/brand_intro.go | 117 ------------------ pkg/console/brand_intro_test.go | 31 ----- pkg/console/terminal.go | 10 -- 6 files changed, 2 insertions(+), 162 deletions(-) delete mode 100644 pkg/console/brand_intro.go delete mode 100644 pkg/console/brand_intro_test.go diff --git a/.changeset/patch-use-existing-copilot-token-add-wizard.md b/.changeset/patch-use-existing-copilot-token-add-wizard.md index ff52420fd7e..3ca2ffc1754 100644 --- a/.changeset/patch-use-existing-copilot-token-add-wizard.md +++ b/.changeset/patch-use-existing-copilot-token-add-wizard.md @@ -2,4 +2,4 @@ "gh-aw": patch --- -Default add-wizard to using an existing `COPILOT_GITHUB_TOKEN` secret, make repository authoring support files optional, add a reusable animated welcome logo, improve workflow and Copilot authentication prompt formatting, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file +Default add-wizard to using an existing `COPILOT_GITHUB_TOKEN` secret, make repository authoring support files optional, improve workflow and Copilot authentication prompt formatting, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index e92dc428616..0f7e6a4f210 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -182,7 +182,7 @@ func (c *AddInteractiveConfig) runInitialAddInteractiveChecks() error { if err := c.resolveWorkflows(); err != nil { return err } - console.ShowAnimatedWelcomeBanner(c.welcomeMessage()) + console.ShowWelcomeBanner(c.welcomeMessage()) c.showWorkflowDescriptions() if err := c.checkGHAuthStatus(); err != nil { return err diff --git a/pkg/console/README.md b/pkg/console/README.md index adb4fdac3fe..32c93215d8b 100644 --- a/pkg/console/README.md +++ b/pkg/console/README.md @@ -101,8 +101,6 @@ The package is designed to adapt to the execution environment. Native builds det | `SetTimeLocation` | `func SetTimeLocation(location *time.Location)` | Sets the location used when rendering `time.Time` values. | | `ShowInteractiveList` | `func ShowInteractiveList(title string, items []ListItem) (string, error)` | Shows a single-selection interactive list; native builds use `huh` and non-TTY mode falls back to numbered text input. | | `ShowWelcomeBanner` | `func ShowWelcomeBanner(description string)` | Clears the screen and prints the interactive welcome banner and description to stderr. | -| `ShowAnimatedWelcomeBanner` | `func ShowAnimatedWelcomeBanner(description string)` | Displays the standard welcome text beside the compact animated GH-AW logo. | -| `ShowAnimatedBrandIntro` | `func ShowAnimatedBrandIntro(title, description string)` | Displays the reusable compact GH-AW logo animation beside wizard intro text, with a static accessible fallback. | | `ToRelativePath` | `func ToRelativePath(path string) string` | Converts an absolute path to a cwd-relative display path when possible. | ### Constants diff --git a/pkg/console/brand_intro.go b/pkg/console/brand_intro.go deleted file mode 100644 index 2f02e2411cd..00000000000 --- a/pkg/console/brand_intro.go +++ /dev/null @@ -1,117 +0,0 @@ -package console - -import ( - "fmt" - "io" - "strings" - "time" - - lipgloss "charm.land/lipgloss/v2" - "github.com/github/gh-aw/pkg/styles" - "github.com/github/gh-aw/pkg/tty" -) - -const brandIntroFrameDelay = 65 * time.Millisecond - -var brandLogoStyle = lipgloss.NewStyle(). - Bold(true). - Foreground(styles.ColorPurple) - -var brandLogoFrames = [][]string{ - { - "+-----+", - "| |", - "+--+--+", - " |", - " +----+", - " +-----+", - " | |", - " +-----+", - }, - { - "+-----+ .", - "| |", - "+--+--+", - " |", - " +----+", - " +-----+", - " | |", - " +-----+", - }, - { - "+-----+ *", - "| | *+*", - "+--+--+ *", - " |", - " +----+", - " +-----+", - " | |", - " +-----+", - }, - { - "+-----+ +", - "| | +++", - "+--+--+ +", - " |", - " +----+", - " +-----+", - " | |", - " +-----+", - }, -} - -// ShowAnimatedBrandIntro clears the screen and displays a compact animated -// GitHub Agentic Workflows mark beside the supplied title and description. -// Animation is disabled outside a TTY and in accessible mode. -func ShowAnimatedBrandIntro(title, description string) { - ClearScreen() - out := stderrWriter() - animate := tty.IsStderrTerminal() && !IsAccessibleMode() - lastFrame := len(brandLogoFrames) - 1 - - if animate { - for frame := range lastFrame { - printBrandIntroFrame(out, frame, "", "", true) - time.Sleep(brandIntroFrameDelay) - moveCursorToBrandIntroStart(out) - } - } - - printBrandIntroFrame(out, lastFrame, title, description, animate) - fmt.Fprintln(out) -} - -func printBrandIntroFrame(out io.Writer, frame int, title, description string, clearLines bool) { - lines := formatBrandIntroFrame(frame, title, description) - for _, line := range lines { - if clearLines { - fmt.Fprint(out, ansiClearLine) - } - fmt.Fprintln(out, line) - } -} - -func moveCursorToBrandIntroStart(out io.Writer) { - fmt.Fprintf(out, "\033[%dA\r", len(brandLogoFrames[0])) -} - -func formatBrandIntroFrame(frame int, title, description string) []string { - logo := brandLogoFrames[frame] - lines := make([]string, len(logo)) - for index, logoLine := range logo { - visibleWidth := len([]rune(logoLine)) - if tty.IsStderrTerminal() { - logoLine = brandLogoStyle.Render(logoLine) - } - lines[index] = padBrandLogoLine(logoLine, visibleWidth, 21) - } - - lines[1] += title - lines[3] += description - return lines -} - -func padBrandLogoLine(value string, visibleWidth, width int) string { - padding := max(1, width-visibleWidth) - return value + strings.Repeat(" ", padding) -} diff --git a/pkg/console/brand_intro_test.go b/pkg/console/brand_intro_test.go deleted file mode 100644 index 571663de0bd..00000000000 --- a/pkg/console/brand_intro_test.go +++ /dev/null @@ -1,31 +0,0 @@ -//go:build !integration - -package console - -import ( - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestFormatBrandIntroFrame(t *testing.T) { - t.Setenv("TERM", "dumb") - - lines := formatBrandIntroFrame(len(brandLogoFrames)-1, "Welcome", "Add a workflow") - - require.Len(t, lines, 8) - assert.Contains(t, lines[0], "+-----+") - assert.Contains(t, lines[1], "+++", "final frame should show the logo sparkle") - assert.Contains(t, lines[1], "Welcome") - assert.Contains(t, lines[3], "Add a workflow") - assert.Contains(t, strings.Join(lines, "\n"), "+----+", "logo should show linked workflow nodes") -} - -func TestBrandLogoAnimationFramesKeepStableHeight(t *testing.T) { - for _, frame := range brandLogoFrames { - assert.Len(t, frame, len(brandLogoFrames[0])) - } - assert.NotEqual(t, brandLogoFrames[0], brandLogoFrames[len(brandLogoFrames)-1]) -} diff --git a/pkg/console/terminal.go b/pkg/console/terminal.go index 3eb50e09b8e..f9eb3767c89 100644 --- a/pkg/console/terminal.go +++ b/pkg/console/terminal.go @@ -54,13 +54,3 @@ func ShowWelcomeBanner(description string) { fmt.Fprintln(out, description) fmt.Fprintln(out, "") } - -// ShowAnimatedWelcomeBanner displays the welcome text beside the reusable -// animated GitHub Agentic Workflows mark. -func ShowAnimatedWelcomeBanner(description string) { - header := "→ Welcome to GitHub Agentic Workflows!" - if tty.IsStderrTerminal() { - header = styles.Header.Render(header) - } - ShowAnimatedBrandIntro(header, description) -} From f5e674e2d588440385584acfcae0ae71b7ad60a0 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 19:05:55 +0200 Subject: [PATCH 07/20] Polish add-wizard decision display --- ...h-use-existing-copilot-token-add-wizard.md | 2 +- pkg/cli/add_command.go | 3 + pkg/cli/add_init.go | 4 ++ pkg/cli/add_interactive_engine.go | 3 +- pkg/cli/add_interactive_git.go | 1 + pkg/cli/add_interactive_orchestrator.go | 10 +++ pkg/cli/add_interactive_orchestrator_test.go | 1 + pkg/cli/add_interactive_schedule.go | 3 +- pkg/cli/add_interactive_workflow.go | 2 + pkg/cli/add_package_manifest_remote.go | 4 +- pkg/cli/add_workflow_pr.go | 2 +- pkg/cli/pr_command.go | 46 ++++++++----- pkg/cli/pr_command_test.go | 30 +++++++++ pkg/console/README.md | 8 +-- pkg/console/prompt_form.go | 67 +++++++++++++++++-- pkg/console/prompt_form_test.go | 21 ++++++ 16 files changed, 174 insertions(+), 33 deletions(-) diff --git a/.changeset/patch-use-existing-copilot-token-add-wizard.md b/.changeset/patch-use-existing-copilot-token-add-wizard.md index 3ca2ffc1754..b611b231885 100644 --- a/.changeset/patch-use-existing-copilot-token-add-wizard.md +++ b/.changeset/patch-use-existing-copilot-token-add-wizard.md @@ -2,4 +2,4 @@ "gh-aw": patch --- -Default add-wizard to using an existing `COPILOT_GITHUB_TOKEN` secret, make repository authoring support files optional, improve workflow and Copilot authentication prompt formatting, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file +Default add-wizard to using an existing `COPILOT_GITHUB_TOKEN` secret, reuse fetched repository metadata, make repository authoring support files optional, improve workflow and Copilot authentication prompt formatting, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index 28c9f083734..ee286216542 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -78,6 +78,9 @@ type AddOptions struct { NoStopAfter bool StopAfter string DisableSecurityScanner bool + // RepoSlug is the already-resolved target repository in owner/repo format. + // When set, PR creation avoids fetching the same repository metadata again. + RepoSlug string // GhAwRef is the resolved github/gh-aw commit SHA used by compiled action references. GhAwRef string // AddCopilotRequestsPermission injects permissions.copilot-requests: write into diff --git a/pkg/cli/add_init.go b/pkg/cli/add_init.go index 428be6187ae..2576f4353d0 100644 --- a/pkg/cli/add_init.go +++ b/pkg/cli/add_init.go @@ -72,8 +72,12 @@ func confirmAndInitializeAddRepository(ctx context.Context, engineOverride strin confirmed, err := addConfirmAuthoringSupport(ctx) if err != nil || !confirmed { + if err == nil { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Coding agent authoring support: skipped")) + } return nil, err } + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Coding agent authoring support: enabled")) return ensureAddRepositoryInitializedWithDetails(engineOverride, verbose, noGitattributes) } diff --git a/pkg/cli/add_interactive_engine.go b/pkg/cli/add_interactive_engine.go index bfd09e03ed6..e0cd15be847 100644 --- a/pkg/cli/add_interactive_engine.go +++ b/pkg/cli/add_interactive_engine.go @@ -52,7 +52,7 @@ func (c *AddInteractiveConfig) selectAIEngineAndKey() error { Description("This determines which coding agent processes your workflows"). Options(engineOptions...). Value(&selectedEngine), - ) + ).WithLeadingBlankLine() if err := form.RunWithContext(c.Ctx); err != nil { return fmt.Errorf("failed to select coding agent: %w", err) @@ -321,6 +321,7 @@ func (c *AddInteractiveConfig) applyCopilotAuthMethodChoice(authMethod string) { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No COPILOT_GITHUB_TOKEN secret is required — Copilot usage is billed to your org's Copilot seat.")) } else { c.UseCopilotRequests = false + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Selected authentication: COPILOT_GITHUB_TOKEN")) fmt.Fprintln(os.Stderr, console.FormatInfoMessage("A fine-grained PAT with Copilot Requests permission will be required.")) } } diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index dbd51bfcce2..22c51428aa4 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -55,6 +55,7 @@ func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx conte NoStopAfter: c.NoStopAfter, StopAfter: c.StopAfter, DisableSecurityScanner: c.DisableSecurityScanner, + RepoSlug: c.RepoOverride, AddCopilotRequestsPermission: c.UseCopilotRequests, GhAwRef: c.GhAwRef, initializedFiles: initFiles, diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index 0f7e6a4f210..efbaa916a82 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -184,6 +184,7 @@ func (c *AddInteractiveConfig) runInitialAddInteractiveChecks() error { } console.ShowWelcomeBanner(c.welcomeMessage()) c.showWorkflowDescriptions() + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(c.sourceWorkflowMessage())) if err := c.checkGHAuthStatus(); err != nil { return err } @@ -209,6 +210,10 @@ func (c *AddInteractiveConfig) welcomeMessage() string { return fmt.Sprintf("This tool will walk you through adding %d automated workflows from %q.", len(workflowNames), source) } +func (c *AddInteractiveConfig) sourceWorkflowMessage() string { + return "Source workflow: " + strings.Join(c.WorkflowSpecs, ", ") +} + func (c *AddInteractiveConfig) prepareAndConfirmAddInteractive() (workflowFiles, initFiles []string, secretName, secretValue string, createPR bool, err error) { // selectAIEngineAndKey only selects the engine and, for Copilot, the auth method // (org billing vs. PAT). It does not prompt for or upload any secret value, since @@ -408,6 +413,11 @@ func (c *AddInteractiveConfig) confirmChanges(workflowFiles, initFiles []string) if err := form.RunWithContext(c.Ctx); err != nil { return false, fmt.Errorf("confirmation failed: %w", err) } + if createPR { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Selected delivery: create a pull request")) + } else { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Selected delivery: write files locally")) + } return createPR, nil } diff --git a/pkg/cli/add_interactive_orchestrator_test.go b/pkg/cli/add_interactive_orchestrator_test.go index e1c222f2555..4ac74f10c6e 100644 --- a/pkg/cli/add_interactive_orchestrator_test.go +++ b/pkg/cli/add_interactive_orchestrator_test.go @@ -144,6 +144,7 @@ func TestAddInteractiveConfig_welcomeMessage(t *testing.T) { } assert.Equal(t, `This tool will walk you through adding the automated workflow "repo-assist" from "githubnext/agentics/repo-assist".`, config.welcomeMessage()) + assert.Equal(t, "Source workflow: githubnext/agentics/repo-assist", config.sourceWorkflowMessage()) } func TestAddInteractiveConfig_showWorkflowDescriptions(t *testing.T) { diff --git a/pkg/cli/add_interactive_schedule.go b/pkg/cli/add_interactive_schedule.go index 5e3961d6625..34f9e82c170 100644 --- a/pkg/cli/add_interactive_schedule.go +++ b/pkg/cli/add_interactive_schedule.go @@ -236,6 +236,7 @@ func (c *AddInteractiveConfig) selectScheduleFrequency() error { // "custom" or same frequency means keep as-is if selected == "custom" || selected == currentFreq { scheduleWizardLog.Printf("Schedule unchanged: keeping %q", rawExpr) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Selected schedule: "+rawExpr)) continue } @@ -272,7 +273,7 @@ func (c *AddInteractiveConfig) selectScheduleFrequency() error { if wf.SourceInfo != nil { wf.SourceInfo.Content = []byte(updatedContent) } - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Schedule updated to: "+selected)) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Selected schedule: "+selected)) } return nil diff --git a/pkg/cli/add_interactive_workflow.go b/pkg/cli/add_interactive_workflow.go index 555c69f978e..19070f2969c 100644 --- a/pkg/cli/add_interactive_workflow.go +++ b/pkg/cli/add_interactive_workflow.go @@ -57,9 +57,11 @@ func (c *AddInteractiveConfig) checkStatusAndOfferRun(ctx context.Context) error } if !runNow { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Selected workflow run: later")) c.showFinalInstructions() return nil } + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Selected workflow run: now")) if err := c.runAddedWorkflowOnce(ctx); err != nil { fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("Failed to run workflow: %v", err))) diff --git a/pkg/cli/add_package_manifest_remote.go b/pkg/cli/add_package_manifest_remote.go index 1bb1a055371..46ca8822fdf 100644 --- a/pkg/cli/add_package_manifest_remote.go +++ b/pkg/cli/add_package_manifest_remote.go @@ -116,12 +116,12 @@ func resolveRepositoryPackageDefaultBranch(ctx context.Context, repoSlug, host s var output []byte var err error if host != "" { - output, err = workflow.RunGHContextWithHost(ctx, "Fetching repo info...", host, args...) + output, err = workflow.RunGHContextWithHost(ctx, "Resolving source repository default branch...", host, args...) if err != nil { return "", err } } else { - output, err = workflow.RunGH("Fetching repo info...", args...) + output, err = workflow.RunGH("Resolving source repository default branch...", args...) if err != nil { return "", err } diff --git a/pkg/cli/add_workflow_pr.go b/pkg/cli/add_workflow_pr.go index d5a32bbc616..a287b949116 100644 --- a/pkg/cli/add_workflow_pr.go +++ b/pkg/cli/add_workflow_pr.go @@ -164,7 +164,7 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts // Create PR addWorkflowPRLog.Printf("Creating pull request: %s", prTitle) - prNumber, prURL, err := createPR(ctx, branchName, prTitle, prBody, opts.Verbose) + prNumber, prURL, err := createPRForRepo(ctx, branchName, prTitle, prBody, opts.RepoSlug, opts.Verbose) if err != nil { addWorkflowPRLog.Printf("Failed to create PR: %v", err) if rollbackErr := tracker.RollbackAllFiles(opts.Verbose); rollbackErr != nil && opts.Verbose { diff --git a/pkg/cli/pr_command.go b/pkg/cli/pr_command.go index f49bfe9ffe3..b9e026665d8 100644 --- a/pkg/cli/pr_command.go +++ b/pkg/cli/pr_command.go @@ -770,8 +770,16 @@ func transferPR(prURL, targetRepo string, verbose bool) error { return nil } -// createPR creates a pull request using GitHub CLI and returns the PR number +var createPRRunGHContextWithHost = workflow.RunGHContextWithHost + +// createPR creates a pull request using GitHub CLI and returns the PR number. func createPR(ctx context.Context, branchName, title, body string, verbose bool) (int, string, error) { + return createPRForRepo(ctx, branchName, title, body, "", verbose) +} + +// createPRForRepo creates a pull request in repoSlug. When repoSlug is empty, +// it resolves the current repository for compatibility with other PR callers. +func createPRForRepo(ctx context.Context, branchName, title, body, repoSlug string, verbose bool) (int, string, error) { if verbose { fmt.Fprintln(os.Stderr, console.FormatProgressMessage("Creating PR: "+title)) } @@ -780,31 +788,33 @@ func createPR(ctx context.Context, branchName, title, body string, verbose bool) // repositories are targeted correctly instead of defaulting to github.com. remoteHost := getHostFromOriginRemote() - // Get the current repository info to ensure PR is created in the correct repo. - // Use GH_HOST env var instead of --hostname (which is only valid for gh api, not gh repo view). - repoOutput, err := workflow.RunGHContextWithHost(ctx, "Fetching repository info...", remoteHost, "repo", "view", "--json", "owner,name") - if err != nil { - return 0, "", fmt.Errorf("could not get current repository info; ensure required prerequisites are configured, then retry: %w", err) - } + repoSpec := repoSlug + if repoSpec == "" { + // Use GH_HOST env var instead of --hostname (which is only valid for gh api, not gh repo view). + repoOutput, err := createPRRunGHContextWithHost(ctx, "Fetching repository info...", remoteHost, "repo", "view", "--json", "owner,name") + if err != nil { + return 0, "", fmt.Errorf("could not get current repository info; ensure required prerequisites are configured, then retry: %w", err) + } - var repoInfo struct { - Owner struct { - Login string `json:"login"` - } `json:"owner"` - Name string `json:"name"` - } + var repoInfo struct { + Owner struct { + Login string `json:"login"` + } `json:"owner"` + Name string `json:"name"` + } - if err := json.Unmarshal(repoOutput, &repoInfo); err != nil { - return 0, "", fmt.Errorf("could not parse repository info; the GitHub API response may be malformed or unexpected: %w", err) - } + if err := json.Unmarshal(repoOutput, &repoInfo); err != nil { + return 0, "", fmt.Errorf("could not parse repository info; the GitHub API response may be malformed or unexpected: %w", err) + } - repoSpec := fmt.Sprintf("%s/%s", repoInfo.Owner.Login, repoInfo.Name) + repoSpec = fmt.Sprintf("%s/%s", repoInfo.Owner.Login, repoInfo.Name) + } // Build gh pr create args. Explicitly specifying --repo ensures the PR is created in the // current repo (not an upstream fork). Use GH_HOST env var instead of --hostname // (which is only valid for gh api, not gh pr create). prCreateArgs := []string{"pr", "create", "--repo", repoSpec, "--title", title, "--body", body, "--head", branchName} - output, err := workflow.RunGHContextWithHost(ctx, "Creating pull request...", remoteHost, prCreateArgs...) + output, err := createPRRunGHContextWithHost(ctx, "Creating pull request...", remoteHost, prCreateArgs...) if err != nil { // Try to get stderr for better error reporting var exitError *exec.ExitError diff --git a/pkg/cli/pr_command_test.go b/pkg/cli/pr_command_test.go index 222efaff69d..12741693220 100644 --- a/pkg/cli/pr_command_test.go +++ b/pkg/cli/pr_command_test.go @@ -3,6 +3,7 @@ package cli import ( + "context" "encoding/json" "strings" "testing" @@ -11,6 +12,35 @@ import ( "github.com/github/gh-aw/pkg/parser" ) +func TestCreatePRForRepoSkipsRepositoryLookup(t *testing.T) { + originalRunGH := createPRRunGHContextWithHost + t.Cleanup(func() { createPRRunGHContextWithHost = originalRunGH }) + + var calls [][]string + createPRRunGHContextWithHost = func(_ context.Context, _ string, _ string, args ...string) ([]byte, error) { + calls = append(calls, args) + if len(args) >= 2 && args[0] == "repo" && args[1] == "view" { + t.Fatal("known repository slug must skip gh repo view") + } + return []byte("https://github.com/owner/repo/pull/42\n"), nil + } + + prNumber, prURL, err := createPRForRepo(context.Background(), "feature", "Title", "Body", "owner/repo", false) + if err != nil { + t.Fatalf("createPRForRepo() error = %v", err) + } + if prNumber != 42 || prURL != "https://github.com/owner/repo/pull/42" { + t.Fatalf("createPRForRepo() = (%d, %q), want (42, PR URL)", prNumber, prURL) + } + if len(calls) != 1 { + t.Fatalf("gh call count = %d, want 1", len(calls)) + } + args := strings.Join(calls[0], " ") + if !strings.Contains(args, "pr create --repo owner/repo") { + t.Fatalf("gh args = %q, want explicit repository", args) + } +} + func TestParsePRURL(t *testing.T) { t.Parallel() tests := []struct { diff --git a/pkg/console/README.md b/pkg/console/README.md index 32c93215d8b..04ca84fbf2b 100644 --- a/pkg/console/README.md +++ b/pkg/console/README.md @@ -69,13 +69,13 @@ The package is designed to adapt to the execution environment. Native builds det | `LayoutJoinVertical` | `func LayoutJoinVertical(sections ...string) string` | Joins multiple sections vertically in WASM builds. | | `LayoutTitleBox` | `func LayoutTitleBox(title string, width int) string` | Returns a simple title-box layout in WASM builds. | | `LogVerbose` | `func LogVerbose(verbose bool, message string)` | Prints a verbose message to stderr only when verbose mode is enabled. | -| `NewConfirmForm` | `func NewConfirmForm(confirm *huh.Confirm) *huh.Form` | Wraps a confirm field in a themed, accessibility-aware `huh` form. | -| `NewForm` | `func NewForm(groups ...*huh.Group) *huh.Form` | Creates a themed, accessibility-aware `huh` form. | +| `NewConfirmForm` | `func NewConfirmForm(confirm *huh.Confirm) *PromptForm` | Wraps a confirm field in a themed, accessibility-aware form that clears after completion. | +| `NewForm` | `func NewForm(groups ...*huh.Group) *PromptForm` | Creates a themed, accessibility-aware form that clears after completion. | | `NewIndeterminateProgressBar` | `func NewIndeterminateProgressBar() *ProgressBar` | Creates an indeterminate progress bar; available in WASM builds. | -| `NewInputForm` | `func NewInputForm(input *huh.Input) *huh.Form` | Wraps an input field in a themed, accessibility-aware `huh` form. | +| `NewInputForm` | `func NewInputForm(input *huh.Input) *PromptForm` | Wraps an input field in a themed, accessibility-aware form that clears after completion. | | `NewListItem` | `func NewListItem(title, description, value string) ListItem` | Constructs a `ListItem` for interactive list APIs. | | `NewProgressBar` | `func NewProgressBar(total int64) *ProgressBar` | Creates a progress bar for a known total amount of work. | -| `NewSelectForm` | `func NewSelectForm[T comparable](selectField *huh.Select[T]) *huh.Form` | Wraps a select field in a themed, accessibility-aware `huh` form. | +| `NewSelectForm` | `func NewSelectForm[T comparable](selectField *huh.Select[T]) *PromptForm` | Wraps a select field in a themed, accessibility-aware form that clears after completion. | | `NewSpinner` | `func NewSpinner(message string) *SpinnerWrapper` | Creates a spinner configured for stderr TTY and accessibility conditions. | | `PrintBanner` | `func PrintBanner()` | Prints the banner to stderr in native builds; no-op in WASM. | | `PrintCommandMessage` | `func PrintCommandMessage(command string)` | Prints a formatted command message to stderr. | diff --git a/pkg/console/prompt_form.go b/pkg/console/prompt_form.go index ef8bffec310..99f5619e9b1 100644 --- a/pkg/console/prompt_form.go +++ b/pkg/console/prompt_form.go @@ -3,32 +3,89 @@ package console import ( + "context" "errors" + "fmt" + "io" "charm.land/huh/v2" "github.com/github/gh-aw/pkg/styles" + "github.com/github/gh-aw/pkg/tty" ) +const ( + ansiSaveCursor = "\0337" + ansiRestoreCursor = "\0338" + ansiClearScreenBelow = "\033[J" +) + +// PromptForm wraps a huh form so completed questions are removed before the +// caller prints the decision result. +type PromptForm struct { + *huh.Form + out io.Writer + clearOnRun bool + leadingBlankLine bool +} + // NewForm creates a huh form with gh-aw's default theme and accessibility mode. -func NewForm(groups ...*huh.Group) *huh.Form { - return huh.NewForm(groups...).WithTheme(styles.HuhTheme).WithAccessible(IsAccessibleMode()) +func NewForm(groups ...*huh.Group) *PromptForm { + accessible := IsAccessibleMode() + return &PromptForm{ + Form: huh.NewForm(groups...).WithTheme(styles.HuhTheme).WithAccessible(accessible), + out: stderrWriter(), + clearOnRun: tty.IsStderrTerminal() && !accessible, + } } // NewInputForm creates a themed, accessibility-aware single-input form. -func NewInputForm(input *huh.Input) *huh.Form { +func NewInputForm(input *huh.Input) *PromptForm { return NewForm(huh.NewGroup(input)) } // NewSelectForm creates a themed, accessibility-aware single-select form. -func NewSelectForm[T comparable](selectField *huh.Select[T]) *huh.Form { +func NewSelectForm[T comparable](selectField *huh.Select[T]) *PromptForm { return NewForm(huh.NewGroup(selectField)) } // NewConfirmForm creates a themed, accessibility-aware single-confirm form. -func NewConfirmForm(confirm *huh.Confirm) *huh.Form { +func NewConfirmForm(confirm *huh.Confirm) *PromptForm { return NewForm(huh.NewGroup(confirm)) } +// WithLeadingBlankLine separates the form from preceding status output. When +// completed forms are cleared, the separator is cleared with the question so +// the caller's resolution message remains adjacent to prior status messages. +func (f *PromptForm) WithLeadingBlankLine() *PromptForm { + f.leadingBlankLine = true + return f +} + +// Run runs the form and removes its rendered question when it exits. +func (f *PromptForm) Run() error { + return f.run(func() error { return f.Form.Run() }) +} + +// RunWithContext runs the form with a context and removes its rendered question when it exits. +func (f *PromptForm) RunWithContext(ctx context.Context) error { + return f.run(func() error { return f.Form.RunWithContext(ctx) }) +} + +func (f *PromptForm) run(runForm func() error) error { + if !f.clearOnRun { + if f.leadingBlankLine { + fmt.Fprintln(f.out) + } + return runForm() + } + fmt.Fprint(f.out, ansiSaveCursor) + if f.leadingBlankLine { + fmt.Fprintln(f.out) + } + defer fmt.Fprint(f.out, ansiRestoreCursor, ansiClearScreenBelow) + return runForm() +} + // IsCancelled reports whether err represents a deliberate user cancellation // (Ctrl-C / Esc before form submission, i.e. huh.ErrUserAborted). // Use this to distinguish graceful cancellation from genuine failures. diff --git a/pkg/console/prompt_form_test.go b/pkg/console/prompt_form_test.go index 19edc3ddf31..9ed064bff7c 100644 --- a/pkg/console/prompt_form_test.go +++ b/pkg/console/prompt_form_test.go @@ -3,6 +3,7 @@ package console import ( + "bytes" "errors" "fmt" "testing" @@ -24,6 +25,26 @@ func TestPromptWrappersReturnNonNilForms(t *testing.T) { require.NotNil(t, NewConfirmForm(huh.NewConfirm().Value(&confirmValue))) } +func TestPromptFormClearsCompletedQuestion(t *testing.T) { + var output bytes.Buffer + form := (&PromptForm{out: &output, clearOnRun: true}).WithLeadingBlankLine() + + err := form.run(func() error { return nil }) + + require.NoError(t, err) + require.Equal(t, ansiSaveCursor+"\n"+ansiRestoreCursor+ansiClearScreenBelow, output.String()) +} + +func TestPromptFormDoesNotClearAccessibleOrNonTTYQuestion(t *testing.T) { + var output bytes.Buffer + form := (&PromptForm{out: &output, clearOnRun: false}).WithLeadingBlankLine() + + err := form.run(func() error { return nil }) + + require.NoError(t, err) + require.Equal(t, "\n", output.String()) +} + func TestIsCancelled(t *testing.T) { t.Run("returns true for huh.ErrUserAborted", func(t *testing.T) { require.True(t, IsCancelled(huh.ErrUserAborted)) From 72886b09908519810f1632fa3925bfa00944f475 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 19:13:42 +0200 Subject: [PATCH 08/20] improve add-wizard --- pkg/cli/add_interactive_engine.go | 4 +--- pkg/cli/add_interactive_git.go | 1 - pkg/cli/add_interactive_orchestrator.go | 19 ++++++++---------- pkg/cli/add_interactive_orchestrator_test.go | 20 +++++++++++++++++++ pkg/cli/add_interactive_schedule.go | 7 ++----- pkg/workflow/compiler_validators_test.go | 20 +++++++++++++++++++ .../permissions_compiler_validator.go | 2 +- 7 files changed, 52 insertions(+), 21 deletions(-) diff --git a/pkg/cli/add_interactive_engine.go b/pkg/cli/add_interactive_engine.go index e0cd15be847..c879f48a1f7 100644 --- a/pkg/cli/add_interactive_engine.go +++ b/pkg/cli/add_interactive_engine.go @@ -265,8 +265,6 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { fmt.Fprintln(os.Stderr, console.FormatInfoMessage(probe.InfoNote)) } - fmt.Fprintln(os.Stderr, "") - // Build select options. // When billing is confirmed enabled, copilot-requests is listed first (pre-selected). // When billing is disabled or inconclusive, PAT is listed first (default selection). @@ -301,7 +299,7 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { }) } - form := console.NewSelectForm(selectField) + form := console.NewSelectForm(selectField).WithLeadingBlankLine() if err := form.RunWithContext(c.Ctx); err != nil { return fmt.Errorf("failed to select Copilot authentication method: %w", err) diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index 22c51428aa4..f9d10be80d7 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -67,7 +67,6 @@ func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx conte c.addResult = result if !createPR { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Workflow files written locally. No pull request was created.")) return nil } diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index efbaa916a82..4b8cb254035 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -339,8 +339,7 @@ func (c *AddInteractiveConfig) determineFilesToAdd() (workflowFiles []string, in } } - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, "The following workflow files will be added:") + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Workflow files ready to add:")) for _, f := range workflowFiles { fmt.Fprintf(os.Stderr, " • .github/workflows/%s\n", f) } @@ -452,26 +451,24 @@ func (c *AddInteractiveConfig) showFinalInstructions() { // not claim the workflow is already running or recommend remote status/run commands, // since the files only exist in the local checkout and have not been pushed. func (c *AddInteractiveConfig) showLocalWriteInstructions() { - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━") - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("🎉 Files written locally!")) - fmt.Fprintln(os.Stderr, "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━") - fmt.Fprintln(os.Stderr, "") - // Show summary with workflow name(s) if c.resolvedWorkflows != nil && len(c.resolvedWorkflows.Workflows) > 0 { wf := c.resolvedWorkflows.Workflows[0] - fmt.Fprintf(os.Stderr, "The workflow '%s' has been written to your local checkout. No pull request was created.\n", wf.Spec.WorkflowName) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Workflow '%s' written locally; no pull request was created.", wf.Spec.WorkflowName))) c.showWorkflowDescriptions() } + workflowName := c.primaryWorkflowName() + if workflowName == "" { + workflowName = "agentic workflow" + } fmt.Fprintln(os.Stderr, "Commit and push the new files before the workflow can run on GitHub:") - fmt.Fprintln(os.Stderr, console.FormatCommandMessage(" git add -A && git commit -m 'Add agentic workflow'")) + fmt.Fprintln(os.Stderr, console.FormatCommandMessage(fmt.Sprintf(" git add -A && git commit -m 'Add %s'", workflowName))) fmt.Fprintln(os.Stderr, console.FormatCommandMessage(" git push")) fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, "Once pushed, these commands will work against the remote repository:") fmt.Fprintln(os.Stderr, console.FormatCommandMessage(fmt.Sprintf(" %s status # Check workflow status", string(constants.CLIExtensionPrefix)))) - fmt.Fprintln(os.Stderr, console.FormatCommandMessage(fmt.Sprintf(" %s run # Trigger a workflow", string(constants.CLIExtensionPrefix)))) + fmt.Fprintln(os.Stderr, console.FormatCommandMessage(fmt.Sprintf(" %s run %s # Trigger the workflow", string(constants.CLIExtensionPrefix), workflowName))) fmt.Fprintln(os.Stderr, console.FormatCommandMessage(fmt.Sprintf(" %s logs # View workflow logs", string(constants.CLIExtensionPrefix)))) fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, "Learn more at: https://github.github.com/gh-aw/") diff --git a/pkg/cli/add_interactive_orchestrator_test.go b/pkg/cli/add_interactive_orchestrator_test.go index 4ac74f10c6e..879cebffccd 100644 --- a/pkg/cli/add_interactive_orchestrator_test.go +++ b/pkg/cli/add_interactive_orchestrator_test.go @@ -7,8 +7,10 @@ import ( "os" "os/exec" "path/filepath" + "strings" "testing" + "github.com/github/gh-aw/pkg/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -147,6 +149,24 @@ func TestAddInteractiveConfig_welcomeMessage(t *testing.T) { assert.Equal(t, "Source workflow: githubnext/agentics/repo-assist", config.sourceWorkflowMessage()) } +func TestAddInteractiveConfig_showLocalWriteInstructionsUsesWorkflowName(t *testing.T) { + config := &AddInteractiveConfig{ + WorkflowSpecs: []string{"githubnext/agentics/repo-assist"}, + resolvedWorkflows: &ResolvedWorkflows{Workflows: []*ResolvedWorkflow{ + {Spec: &WorkflowSpec{WorkflowName: "repo-assist"}}, + }}, + } + + output := testutil.CaptureStderr(t, config.showLocalWriteInstructions) + + assert.Equal(t, 1, strings.Count(output, "written locally"), "local completion should be reported once") + assert.Contains(t, output, "Workflow 'repo-assist' written locally; no pull request was created.") + assert.Contains(t, output, "git add -A && git commit -m 'Add repo-assist'") + assert.Contains(t, output, "gh aw run repo-assist # Trigger the workflow") + assert.NotContains(t, output, "Files written locally!") + assert.NotContains(t, output, "gh aw run ") +} + func TestAddInteractiveConfig_showWorkflowDescriptions(t *testing.T) { t.Parallel() tests := []struct { diff --git a/pkg/cli/add_interactive_schedule.go b/pkg/cli/add_interactive_schedule.go index 34f9e82c170..70e9f0f5285 100644 --- a/pkg/cli/add_interactive_schedule.go +++ b/pkg/cli/add_interactive_schedule.go @@ -215,17 +215,14 @@ func (c *AddInteractiveConfig) selectScheduleFrequency() error { // Build the ordered option list options := buildScheduleOptions(rawExpr, currentFreq) - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("This workflow runs on a schedule.")) - var selected string form := console.NewSelectForm( huh.NewSelect[string](). - Title("How often should this workflow run?"). + Title("This workflow runs on a schedule. How often should it run?"). Description("Current schedule: " + rawExpr). Options(options...). Value(&selected), - ) + ).WithLeadingBlankLine() if err := form.RunWithContext(c.Ctx); err != nil { return fmt.Errorf("failed to select schedule frequency: %w", err) diff --git a/pkg/workflow/compiler_validators_test.go b/pkg/workflow/compiler_validators_test.go index c2698c786ae..00cb9d4bb19 100644 --- a/pkg/workflow/compiler_validators_test.go +++ b/pkg/workflow/compiler_validators_test.go @@ -670,6 +670,26 @@ func TestValidatePermissions_EmitsCopilotRequestsTipOncePerMarkdownPath(t *testi assert.Equal(t, 1, strings.Count(stderr, tipText), "copilot-requests tip should be emitted only once per markdown path") } +func TestValidatePermissions_QuietSuppressesCopilotRequestsTip(t *testing.T) { + workflowData := &WorkflowData{ + Name: "Test", + AI: "copilot", + Permissions: "permissions:\n contents: read\n", + EngineConfig: &EngineConfig{ + ID: "copilot", + }, + } + compiler := NewCompiler() + compiler.SetQuiet(true) + + stderr := testutil.CaptureStderr(t, func() { + _, err := compiler.validatePermissions(workflowData, "test.md") + require.NoError(t, err) + }) + + assert.NotContains(t, stderr, "Tip: set permissions.copilot-requests: write") +} + func TestShouldEmitCopilotRequestsEnableTip(t *testing.T) { tests := []struct { name string diff --git a/pkg/workflow/permissions_compiler_validator.go b/pkg/workflow/permissions_compiler_validator.go index d6983136306..733c8a20ea2 100644 --- a/pkg/workflow/permissions_compiler_validator.go +++ b/pkg/workflow/permissions_compiler_validator.go @@ -194,7 +194,7 @@ Ensure proper audience validation and trust policies are configured.` fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", warningMsg)) c.IncrementWarningCount() } - if shouldEmitCopilotRequestsEnableTip(workflowData, workflowPermissions) && !c.repositoryOwnerIsIndividualUser() { + if !c.quiet && shouldEmitCopilotRequestsEnableTip(workflowData, workflowPermissions) && !c.repositoryOwnerIsIndividualUser() { if !c.copilotRequestsTipShown[markdownPath] { if c.batchMode { c.copilotTipNeeded = true From b88f1e8036039125c7b3546d5ee743c52fd9bb9b Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 19:34:24 +0200 Subject: [PATCH 09/20] improve add-wizard --- pkg/cli/add_command.go | 9 +- pkg/cli/add_init.go | 32 +++- pkg/cli/add_interactive_engine.go | 1 - pkg/cli/add_interactive_git.go | 188 ++++++++++++++++--- pkg/cli/add_interactive_git_test.go | 81 ++++++++ pkg/cli/add_interactive_orchestrator.go | 38 ++-- pkg/cli/add_interactive_orchestrator_test.go | 8 +- 7 files changed, 300 insertions(+), 57 deletions(-) diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index ee286216542..0caaf715153 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -89,6 +89,9 @@ type AddOptions struct { AddCopilotRequestsPermission bool // initializedFiles contains files created by add-wizard after its clean-tree check. initializedFiles []string + // workingTreePrevalidated indicates add-wizard already verified that staged + // changes and changes overlapping planned files are absent. + workingTreePrevalidated bool } // AddWorkflowsResult contains the result of adding workflows @@ -277,8 +280,10 @@ func AddResolvedWorkflows(ctx context.Context, workflowStrings []string, resolve } // Check no other changes are present - if err := checkCleanWorkingDirectoryIgnoring(opts.Verbose, opts.initializedFiles); err != nil { - return nil, fmt.Errorf("working directory is not clean: %w", err) + if !opts.workingTreePrevalidated { + if err := checkCleanWorkingDirectoryIgnoring(opts.Verbose, opts.initializedFiles); err != nil { + return nil, fmt.Errorf("working directory is not clean: %w", err) + } } } diff --git a/pkg/cli/add_init.go b/pkg/cli/add_init.go index 2576f4353d0..667c89b155c 100644 --- a/pkg/cli/add_init.go +++ b/pkg/cli/add_init.go @@ -49,13 +49,18 @@ func missingAddAuthoringSupportFiles(baseDir string, engineOverride string, noGi return missing, nil } -func confirmAndInitializeAddRepository(ctx context.Context, engineOverride string, verbose bool, noGitattributes bool) ([]string, error) { +type addRepositoryInitializationPlan struct { + enabled bool + files []string +} + +func confirmAddRepositoryInitialization(ctx context.Context, engineOverride string, noGitattributes bool) (addRepositoryInitializationPlan, error) { gitRoot, err := addFindGitRoot() if err != nil { if errors.Is(err, gitutil.ErrNotGitRepository) { - return nil, nil + return addRepositoryInitializationPlan{}, nil } - return nil, fmt.Errorf("failed to determine repository root for automatic initialization: %w", err) + return addRepositoryInitializationPlan{}, fmt.Errorf("failed to determine repository root for automatic initialization: %w", err) } var missingMarkers []string @@ -64,10 +69,10 @@ func confirmAndInitializeAddRepository(ctx context.Context, engineOverride strin missingMarkers, inspectErr = addMissingAuthoringSupportFiles(".", engineOverride, noGitattributes) return inspectErr }); err != nil { - return nil, fmt.Errorf("failed to inspect repository initialization state: %w", err) + return addRepositoryInitializationPlan{}, fmt.Errorf("failed to inspect repository initialization state: %w", err) } if len(missingMarkers) == 0 { - return nil, nil + return addRepositoryInitializationPlan{}, nil } confirmed, err := addConfirmAuthoringSupport(ctx) @@ -75,12 +80,27 @@ func confirmAndInitializeAddRepository(ctx context.Context, engineOverride strin if err == nil { fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Coding agent authoring support: skipped")) } - return nil, err + return addRepositoryInitializationPlan{}, err } fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Coding agent authoring support: enabled")) + return addRepositoryInitializationPlan{enabled: true, files: missingMarkers}, nil +} + +func applyAddRepositoryInitialization(plan addRepositoryInitializationPlan, engineOverride string, verbose bool, noGitattributes bool) ([]string, error) { + if !plan.enabled { + return nil, nil + } return ensureAddRepositoryInitializedWithDetails(engineOverride, verbose, noGitattributes) } +func confirmAndInitializeAddRepository(ctx context.Context, engineOverride string, verbose bool, noGitattributes bool) ([]string, error) { + plan, err := confirmAddRepositoryInitialization(ctx, engineOverride, noGitattributes) + if err != nil { + return nil, err + } + return applyAddRepositoryInitialization(plan, engineOverride, verbose, noGitattributes) +} + func ensureAddRepositoryInitialized(engineOverride string, verbose bool, noGitattributes bool) error { _, err := ensureAddRepositoryInitializedWithDetails(engineOverride, verbose, noGitattributes) return err diff --git a/pkg/cli/add_interactive_engine.go b/pkg/cli/add_interactive_engine.go index c879f48a1f7..267981d2f38 100644 --- a/pkg/cli/add_interactive_engine.go +++ b/pkg/cli/add_interactive_engine.go @@ -320,6 +320,5 @@ func (c *AddInteractiveConfig) applyCopilotAuthMethodChoice(authMethod string) { } else { c.UseCopilotRequests = false fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Selected authentication: COPILOT_GITHUB_TOKEN")) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("A fine-grained PAT with Copilot Requests permission will be required.")) } } diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index f9d10be80d7..32030a5adae 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "os/exec" + "path/filepath" "strconv" "strings" @@ -47,7 +48,7 @@ func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx conte Quiet: true, EngineOverride: c.EngineOverride, Name: "", - Force: false, + Force: c.forceOverwrite, AppendText: c.AppendText, CreatePR: createPR, NoGitattributes: c.NoGitattributes, @@ -59,6 +60,7 @@ func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx conte AddCopilotRequestsPermission: c.UseCopilotRequests, GhAwRef: c.GhAwRef, initializedFiles: initFiles, + workingTreePrevalidated: createPR, } result, err := AddResolvedWorkflows(ctx, c.WorkflowSpecs, c.resolvedWorkflows, opts) if err != nil { @@ -304,31 +306,169 @@ func (c *AddInteractiveConfig) updateLocalBranch() error { return nil } -// checkCleanWorkingDirectoryForPR verifies the working directory had no user changes -// before the wizard began repository initialization. It relies on the cleanliness -// snapshot captured in workingDirDirtyBeforeInit (taken before -// ensureAddRepositoryInitializedWithDetails ran) rather than re-checking git status and -// excluding the wizard's init files. Excluding whole init file paths post-hoc would -// wrongly ignore pre-existing, non-conforming files (e.g. a dirty .gitattributes -// missing a required entry) that ensureAddRepositoryInitializedWithDetails rewrites in -// place, letting the PR path silently overwrite or commit pre-existing user edits. -func (c *AddInteractiveConfig) checkCleanWorkingDirectoryForPR() error { - addInteractiveLog.Print("Checking working directory is clean before PR creation") - - if c.workingDirDirtyBeforeInit { - fmt.Fprintln(os.Stderr, console.FormatErrorMessage("Working directory is not clean.")) - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, "Creating a pull request requires a clean working directory.") - fmt.Fprintln(os.Stderr, "Please commit or stash your changes first, or choose the local write option:") - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatCommandMessage(" git stash # Temporarily stash changes")) - fmt.Fprintln(os.Stderr, console.FormatCommandMessage(" git add -A && git commit -m 'wip' # Commit changes")) - fmt.Fprintln(os.Stderr, "") - return errors.New("working directory is not clean") +type addWorkingTreeBlockers struct { + staged []string + overlapping []string +} + +func (b addWorkingTreeBlockers) empty() bool { + return len(b.staged) == 0 && len(b.overlapping) == 0 +} + +type workingTreeResolution string + +const ( + workingTreeOverwrite workingTreeResolution = "overwrite" + workingTreeCleaned workingTreeResolution = "cleaned" + workingTreeExit workingTreeResolution = "exit" +) + +// checkCleanWorkingDirectoryForPR allows unrelated unstaged and untracked files, +// but requires staged changes and edits to files the wizard will write to be cleaned. +func (c *AddInteractiveConfig) checkCleanWorkingDirectoryForPR(workflowFiles, initFiles []string) error { + addInteractiveLog.Print("Checking working tree changes before PR creation") + plannedPaths, err := c.plannedAddPaths(workflowFiles, initFiles) + if err != nil { + return err } - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Working directory is clean")) - return nil + for { + blockers, inspectErr := inspectAddWorkingTree(plannedPaths) + if inspectErr != nil { + return inspectErr + } + if blockers.empty() { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Working tree is ready for pull request creation")) + return nil + } + + allowOverwrite := len(blockers.staged) == 0 && len(blockers.overlapping) > 0 + resolution, promptErr := promptWorkingTreeResolution(c.Ctx, blockers, allowOverwrite) + if promptErr != nil { + return promptErr + } + switch resolution { + case workingTreeOverwrite: + c.forceOverwrite = true + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Overlapping workflow files will be overwritten")) + return nil + case workingTreeExit: + return errors.New("user exited before cleaning the working tree") + } + } +} + +func (c *AddInteractiveConfig) plannedAddPaths(workflowFiles, initFiles []string) ([]string, error) { + gitRoot, err := addFindGitRoot() + if err != nil { + return nil, fmt.Errorf("failed to determine repository root for PR preflight: %w", err) + } + workflowDir := c.WorkflowDir + if workflowDir == "" { + workflowDir = getWorkflowsDir() + } + planned := make([]string, 0, len(workflowFiles)+len(initFiles)) + for _, path := range workflowFiles { + planned = append(planned, filepath.Join(workflowDir, path)) + } + for _, path := range initFiles { + planned = append(planned, path) + } + for index, path := range planned { + if filepath.IsAbs(path) { + rel, relErr := filepath.Rel(gitRoot, path) + if relErr != nil { + return nil, fmt.Errorf("failed to resolve planned path %s: %w", path, relErr) + } + path = rel + } + planned[index] = filepath.ToSlash(filepath.Clean(path)) + } + return planned, nil +} + +func inspectAddWorkingTree(plannedPaths []string) (addWorkingTreeBlockers, error) { + gitRoot, err := addFindGitRoot() + if err != nil { + return addWorkingTreeBlockers{}, fmt.Errorf("failed to determine repository root for PR preflight: %w", err) + } + cmd := exec.Command("git", "status", "--porcelain=v1", "-z", "--untracked-files=all") + cmd.Dir = gitRoot + output, err := cmd.Output() + if err != nil { + return addWorkingTreeBlockers{}, fmt.Errorf("failed to inspect working tree: %w", err) + } + + planned := make(map[string]struct{}, len(plannedPaths)) + for _, path := range plannedPaths { + planned[filepath.ToSlash(filepath.Clean(path))] = struct{}{} + } + var blockers addWorkingTreeBlockers + entries := strings.Split(string(output), "\x00") + for index := 0; index < len(entries); index++ { + entry := entries[index] + if len(entry) < 4 { + continue + } + status := entry[:2] + path := filepath.ToSlash(filepath.Clean(entry[3:])) + if status[0] != ' ' && status[0] != '?' { + blockers.staged = appendUniqueString(blockers.staged, path) + } + if _, overlaps := planned[path]; overlaps { + blockers.overlapping = appendUniqueString(blockers.overlapping, path) + } + if status[0] == 'R' || status[0] == 'C' { + index++ + } + } + return blockers, nil +} + +func appendUniqueString(values []string, value string) []string { + for _, existing := range values { + if existing == value { + return values + } + } + return append(values, value) +} + +func promptWorkingTreeResolution(ctx context.Context, blockers addWorkingTreeBlockers, allowOverwrite bool) (workingTreeResolution, error) { + var resolution workingTreeResolution + form := console.NewSelectForm( + huh.NewSelect[workingTreeResolution](). + Title("Some working tree changes must be resolved before creating the pull request."). + Description(formatWorkingTreeBlockers(blockers)). + Options(buildWorkingTreeResolutionOptions(allowOverwrite)...). + Value(&resolution), + ).WithLeadingBlankLine() + if err := form.RunWithContext(ctx); err != nil { + return "", fmt.Errorf("working tree confirmation failed: %w", err) + } + return resolution, nil +} + +func formatWorkingTreeBlockers(blockers addWorkingTreeBlockers) string { + sections := make([]string, 0, 2) + if len(blockers.staged) > 0 { + sections = append(sections, "Staged changes:\n • "+strings.Join(blockers.staged, "\n • ")) + } + if len(blockers.overlapping) > 0 { + sections = append(sections, "Changes overlapping files the wizard will add:\n • "+strings.Join(blockers.overlapping, "\n • ")) + } + return strings.Join(sections, "\n") +} + +func buildWorkingTreeResolutionOptions(allowOverwrite bool) []huh.Option[workingTreeResolution] { + options := make([]huh.Option[workingTreeResolution], 0, 3) + if allowOverwrite { + options = append(options, huh.NewOption("Overwrite", workingTreeOverwrite).Selected(true)) + } + return append(options, + huh.NewOption("I've cleaned the working tree", workingTreeCleaned), + huh.NewOption("Exit, I'm done here", workingTreeExit), + ) } // squashMergeNotAllowedErr is the lowercase substring of the GitHub GraphQL API error diff --git a/pkg/cli/add_interactive_git_test.go b/pkg/cli/add_interactive_git_test.go index 8403c446fa1..1b7c72de59f 100644 --- a/pkg/cli/add_interactive_git_test.go +++ b/pkg/cli/add_interactive_git_test.go @@ -186,6 +186,87 @@ func runGitIn(t *testing.T, dir string, args ...string) { require.NoError(t, err, "git %s failed: %s", strings.Join(args, " "), string(out)) } +func TestInspectAddWorkingTree(t *testing.T) { + tests := []struct { + name string + prepare func(t *testing.T, repoDir string) + plannedPaths []string + wantStaged []string + wantOverlapping []string + }{ + { + name: "allows unrelated unstaged and untracked files", + prepare: func(t *testing.T, repoDir string) { + require.NoError(t, os.WriteFile(filepath.Join(repoDir, "README.md"), []byte("changed\n"), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(repoDir, "notes.txt"), []byte("notes\n"), 0644)) + }, + plannedPaths: []string{".github/workflows/new-workflow.md"}, + }, + { + name: "blocks staged changes", + prepare: func(t *testing.T, repoDir string) { + require.NoError(t, os.WriteFile(filepath.Join(repoDir, "notes.txt"), []byte("notes\n"), 0644)) + runGit(t, repoDir, "add", "notes.txt") + }, + plannedPaths: []string{".github/workflows/new-workflow.md"}, + wantStaged: []string{"notes.txt"}, + }, + { + name: "blocks unstaged changes to a planned path", + prepare: func(t *testing.T, repoDir string) { + workflowDir := filepath.Join(repoDir, ".github", "workflows") + require.NoError(t, os.MkdirAll(workflowDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(workflowDir, "new-workflow.md"), []byte("local draft\n"), 0644)) + }, + plannedPaths: []string{".github/workflows/new-workflow.md"}, + wantOverlapping: []string{".github/workflows/new-workflow.md"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repoDir := t.TempDir() + runGit(t, repoDir, "init", "--initial-branch=main") + require.NoError(t, os.WriteFile(filepath.Join(repoDir, "README.md"), []byte("initial\n"), 0644)) + runGit(t, repoDir, "add", "README.md") + runGit(t, repoDir, "commit", "-m", "initial") + t.Chdir(repoDir) + tt.prepare(t, repoDir) + + blockers, err := inspectAddWorkingTree(tt.plannedPaths) + require.NoError(t, err) + assert.Equal(t, tt.wantStaged, blockers.staged) + assert.Equal(t, tt.wantOverlapping, blockers.overlapping) + }) + } +} + +func TestBuildWorkingTreeResolutionOptions(t *testing.T) { + t.Run("offers overwrite first for overlapping unstaged files", func(t *testing.T) { + options := buildWorkingTreeResolutionOptions(true) + require.Len(t, options, 3) + assert.Equal(t, workingTreeOverwrite, options[0].Value) + assert.Equal(t, workingTreeCleaned, options[1].Value) + assert.Equal(t, workingTreeExit, options[2].Value) + }) + + t.Run("does not offer overwrite for staged changes", func(t *testing.T) { + options := buildWorkingTreeResolutionOptions(false) + require.Len(t, options, 2) + assert.Equal(t, workingTreeCleaned, options[0].Value) + assert.Equal(t, workingTreeExit, options[1].Value) + }) +} + +func TestFormatWorkingTreeBlockers(t *testing.T) { + description := formatWorkingTreeBlockers(addWorkingTreeBlockers{ + staged: []string{"notes.txt"}, + overlapping: []string{".github/workflows/repo-assist.md", ".github/workflows/repo-assist.lock.yml"}, + }) + + assert.Equal(t, "Staged changes:\n • notes.txt\nChanges overlapping files the wizard will add:\n • .github/workflows/repo-assist.md\n • .github/workflows/repo-assist.lock.yml", description) +} + func TestIsAlreadyMergedGHError(t *testing.T) { t.Parallel() tests := []struct { diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index 4b8cb254035..f66f0cc20fc 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -69,12 +69,9 @@ type AddInteractiveConfig struct { // This is populated early in the flow by resolveWorkflows() resolvedWorkflows *ResolvedWorkflows - // workingDirDirtyBeforeInit records whether the working directory already had - // uncommitted changes before any wizard-driven repository initialization ran. - // It is captured once, early in RunAddInteractive, and used by - // checkCleanWorkingDirectoryForPR so that wizard-modified init markers (which may - // rewrite pre-existing, non-conforming files) are never mistaken for a clean tree. - workingDirDirtyBeforeInit bool + // forceOverwrite records that the user chose to replace unstaged or untracked + // files overlapping the wizard's planned output. Staged changes never enable it. + forceOverwrite bool } // RunAddInteractive runs the interactive add workflow @@ -101,16 +98,6 @@ func RunAddInteractive(ctx context.Context, config *AddInteractiveConfig) error return err } - // Snapshot working directory cleanliness before any wizard-driven repository - // initialization runs. This is used later, only for the PR path, to detect - // pre-existing user changes without mistaking wizard-modified init markers for - // pre-existing dirty state. - pendingChanges, err := hasPendingChanges() - if err != nil { - return err - } - config.workingDirDirtyBeforeInit = pendingChanges - remainingBootstrapProfile := config.getRemainingBootstrapProfile() filesToAdd, initFiles, secretName, secretValue, createPR, err := config.prepareAndConfirmAddInteractive() @@ -232,22 +219,29 @@ func (c *AddInteractiveConfig) prepareAndConfirmAddInteractive() (workflowFiles, return nil, nil, "", "", false, err } - initFiles, err = confirmAndInitializeAddRepository(c.Ctx, c.EngineOverride, c.Verbose, c.NoGitattributes) + initializationPlan, err := confirmAddRepositoryInitialization(c.Ctx, c.EngineOverride, c.NoGitattributes) if err != nil { return nil, nil, "", "", false, err } + initFiles = initializationPlan.files createPR, err = c.confirmChanges(workflowFiles, initFiles) if err != nil { return nil, nil, "", "", false, err } - if !createPR { - return workflowFiles, initFiles, "", "", false, nil + if createPR { + if err := c.checkCleanWorkingDirectoryForPR(workflowFiles, initFiles); err != nil { + return nil, nil, "", "", false, err + } } - if err := c.checkCleanWorkingDirectoryForPR(); err != nil { + initFiles, err = applyAddRepositoryInitialization(initializationPlan, c.EngineOverride, c.Verbose, c.NoGitattributes) + if err != nil { return nil, nil, "", "", false, err } + if !createPR { + return workflowFiles, initFiles, "", "", false, nil + } // Secret collection and upload only happen once the user has committed to the // PR path and the clean-tree check has succeeded. @@ -390,8 +384,8 @@ func (c *AddInteractiveConfig) primaryWorkflowName() string { func (c *AddInteractiveConfig) confirmChanges(workflowFiles, initFiles []string) (bool, error) { addInteractiveLog.Print("Confirming changes with user") - fmt.Fprintln(os.Stderr, "") if len(initFiles) > 0 { + fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, "The repository will also be initialized with:") for _, f := range initFiles { fmt.Fprintf(os.Stderr, " • %s\n", f) @@ -407,7 +401,7 @@ func (c *AddInteractiveConfig) confirmChanges(workflowFiles, initFiles []string) Affirmative("Yes, create pull request"). Negative("No, write files locally"). Value(&createPR), - ) + ).WithLeadingBlankLine() if err := form.RunWithContext(c.Ctx); err != nil { return false, fmt.Errorf("confirmation failed: %w", err) diff --git a/pkg/cli/add_interactive_orchestrator_test.go b/pkg/cli/add_interactive_orchestrator_test.go index 879cebffccd..b31cd51a854 100644 --- a/pkg/cli/add_interactive_orchestrator_test.go +++ b/pkg/cli/add_interactive_orchestrator_test.go @@ -344,8 +344,12 @@ func TestAddInteractiveConfig_prepareAndConfirmAddInteractive_localWriteSkipsSec require.NoError(t, os.WriteFile(fakeGH, []byte(script), 0o755)) t.Setenv("PATH", tmpDir+string(os.PathListSeparator)+os.Getenv("PATH")) - // Drive the huh confirm form via accessible (line-based) mode, answering "no" to - // "Do you want to create a pull request with these changes?". + originalConfirmAuthoringSupport := addConfirmAuthoringSupport + addConfirmAuthoringSupport = func(context.Context) (bool, error) { return false, nil } + t.Cleanup(func() { addConfirmAuthoringSupport = originalConfirmAuthoringSupport }) + + // Drive the delivery confirm form via accessible (line-based) mode, answering + // "no" to pull request creation. t.Setenv("ACCESSIBLE", "1") r, w, err := os.Pipe() require.NoError(t, err) From a268f48d8c9f06e3a6a6cf225c95e8fbd6625f65 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 19:36:28 +0200 Subject: [PATCH 10/20] Clarify Copilot authentication prompt --- pkg/cli/add_interactive_engine.go | 13 +++++++++---- pkg/cli/add_interactive_engine_test.go | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/pkg/cli/add_interactive_engine.go b/pkg/cli/add_interactive_engine.go index 267981d2f38..1f9a0cf2ef4 100644 --- a/pkg/cli/add_interactive_engine.go +++ b/pkg/cli/add_interactive_engine.go @@ -261,9 +261,6 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { } c.copilotCLIBillingStatus = probe.BillingStatus copilotRequestsLabel += probe.LabelSuffix - if probe.InfoNote != "" { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(probe.InfoNote)) - } // Build select options. // When billing is confirmed enabled, copilot-requests is listed first (pre-selected). @@ -286,7 +283,7 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { var authMethod string selectField := huh.NewSelect[string](). Title("How would you like Copilot workflows to authenticate?"). - Description("PAT uses the existing COPILOT_GITHUB_TOKEN repository secret.\ncopilot-requests uses the org's Copilot billing seat and requires no PAT."). + Description(copilotAuthMethodDescription(probe)). Options(options...). Value(&authMethod) @@ -309,6 +306,14 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { return nil } +func copilotAuthMethodDescription(probe orgCopilotBillingProbeResult) string { + copilotRequestsDescription := "• copilot-requests: Use the org's Copilot billing seat; no PAT required." + if probe.InfoNote != "" { + copilotRequestsDescription += " " + probe.InfoNote + } + return "• PAT: Use the existing COPILOT_GITHUB_TOKEN repository secret.\n" + copilotRequestsDescription +} + // applyCopilotAuthMethodChoice records the user's Copilot auth method selection and prints // the corresponding status message. It is pure (no I/O beyond stderr) and intentionally // separated from the huh form so the assignment logic is unit-testable without mocking the TUI. diff --git a/pkg/cli/add_interactive_engine_test.go b/pkg/cli/add_interactive_engine_test.go index 37192aa7291..42c793fc051 100644 --- a/pkg/cli/add_interactive_engine_test.go +++ b/pkg/cli/add_interactive_engine_test.go @@ -3,6 +3,7 @@ package cli import ( + "strings" "testing" "charm.land/huh/v2" @@ -56,6 +57,21 @@ func TestApplyCopilotAuthMethodChoice_ReEntryClearsOldValue(t *testing.T) { assert.False(t, cfg.UseCopilotRequests) } +func TestCopilotAuthMethodDescription(t *testing.T) { + t.Parallel() + + t.Run("bullets both authentication methods", func(t *testing.T) { + description := copilotAuthMethodDescription(orgCopilotBillingProbeResult{}) + assert.Equal(t, "• PAT: Use the existing COPILOT_GITHUB_TOKEN repository secret.\n• copilot-requests: Use the org's Copilot billing seat; no PAT required.", description) + }) + + t.Run("includes inconclusive billing note in copilot-requests bullet", func(t *testing.T) { + description := copilotAuthMethodDescription(orgCopilotBillingProbeResult{InfoNote: copilotBillingInconclusiveNote}) + assert.NotContains(t, strings.Split(description, "\n")[0], copilotBillingInconclusiveNote) + assert.Contains(t, strings.Split(description, "\n")[1], copilotBillingInconclusiveNote) + }) +} + func TestPrioritizeEngineOption(t *testing.T) { t.Parallel() options := []huh.Option[string]{ From f3274d450a2e77e957724a97a19481160509b70f Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 19:45:42 +0200 Subject: [PATCH 11/20] improve add-wizard --- pkg/cli/add_interactive_engine.go | 2 +- pkg/cli/add_interactive_engine_test.go | 4 +--- pkg/cli/copilot_billing_check.go | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/cli/add_interactive_engine.go b/pkg/cli/add_interactive_engine.go index 1f9a0cf2ef4..1a747c97b5e 100644 --- a/pkg/cli/add_interactive_engine.go +++ b/pkg/cli/add_interactive_engine.go @@ -309,7 +309,7 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { func copilotAuthMethodDescription(probe orgCopilotBillingProbeResult) string { copilotRequestsDescription := "• copilot-requests: Use the org's Copilot billing seat; no PAT required." if probe.InfoNote != "" { - copilotRequestsDescription += " " + probe.InfoNote + copilotRequestsDescription += "\n (NOTE: " + probe.InfoNote + "\n Check with your org admin if you want to use this option.)" } return "• PAT: Use the existing COPILOT_GITHUB_TOKEN repository secret.\n" + copilotRequestsDescription } diff --git a/pkg/cli/add_interactive_engine_test.go b/pkg/cli/add_interactive_engine_test.go index 42c793fc051..550e396b881 100644 --- a/pkg/cli/add_interactive_engine_test.go +++ b/pkg/cli/add_interactive_engine_test.go @@ -3,7 +3,6 @@ package cli import ( - "strings" "testing" "charm.land/huh/v2" @@ -67,8 +66,7 @@ func TestCopilotAuthMethodDescription(t *testing.T) { t.Run("includes inconclusive billing note in copilot-requests bullet", func(t *testing.T) { description := copilotAuthMethodDescription(orgCopilotBillingProbeResult{InfoNote: copilotBillingInconclusiveNote}) - assert.NotContains(t, strings.Split(description, "\n")[0], copilotBillingInconclusiveNote) - assert.Contains(t, strings.Split(description, "\n")[1], copilotBillingInconclusiveNote) + assert.Equal(t, "• PAT: Use the existing COPILOT_GITHUB_TOKEN repository secret.\n• copilot-requests: Use the org's Copilot billing seat; no PAT required.\n (NOTE: Could not confirm org Copilot CLI billing.\n Check with your org admin if you want to use this option.)", description) }) } diff --git a/pkg/cli/copilot_billing_check.go b/pkg/cli/copilot_billing_check.go index f9fa632f020..14f7993ef2c 100644 --- a/pkg/cli/copilot_billing_check.go +++ b/pkg/cli/copilot_billing_check.go @@ -17,7 +17,7 @@ const copilotBillingTimeout = 3 * time.Second // copilotBillingInconclusiveNote is the user-facing message printed when the // org's Copilot CLI billing status cannot be confirmed (non-200 response, // network error, missing field, or no org login available). -const copilotBillingInconclusiveNote = "Could not confirm org Copilot CLI billing — consider checking with your org admin if this is an option." +const copilotBillingInconclusiveNote = "Could not confirm org Copilot CLI billing." // detectOrgCopilotCLIBillingWithClient calls GET /orgs/{org}/copilot/billing with // a 3 s timeout and returns the raw "cli" field. Any non-200 response or error From 843af0cf3045155905be8979f3f047bcd572db6e Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 19:52:45 +0200 Subject: [PATCH 12/20] Show add-wizard pull request progress --- pkg/cli/add_command.go | 20 ++++++++++++++++++-- pkg/cli/add_interactive_git.go | 1 + pkg/cli/add_workflow_pr.go | 10 ++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index 0caaf715153..50e4e0ca697 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -92,6 +92,9 @@ type AddOptions struct { // workingTreePrevalidated indicates add-wizard already verified that staged // changes and changes overlapping planned files are absent. workingTreePrevalidated bool + // showInteractiveProgress enables high-level progress indicators for the + // otherwise quiet add-wizard write, compile, commit, and push phases. + showInteractiveProgress bool } // AddWorkflowsResult contains the result of adding workflows @@ -454,9 +457,11 @@ func addWorkflowWithTracking(ctx context.Context, resolved *ResolvedWorkflow, tr destFile := filepath.Join(githubWorkflowsDir, workflowName+".md") fileExists := fileutil.FileExists(destFile) - if fileExists { + if fileExists && !opts.showInteractiveProgress { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Overwriting existing file: "+destFile)) } + stopProgress := startAddInteractiveProgress(opts, "Preparing workflow files...") + defer stopProgress() workflowSpec = resolvedWorkflowSpec(workflowSpec, sourceInfo) content, err := processWorkflowContentModifications(string(sourceContent), workflowSpec, sourceInfo, githubWorkflowsDir, opts) if err != nil { @@ -469,6 +474,15 @@ func addWorkflowWithTracking(ctx context.Context, resolved *ResolvedWorkflow, tr return nil } +func startAddInteractiveProgress(opts AddOptions, message string) func() { + if !opts.showInteractiveProgress { + return func() {} + } + spinner := console.NewSpinner(message) + spinner.Start() + return spinner.Stop +} + func reportAddWorkflowStart(workflowSpec *WorkflowSpec, sourceContent []byte, opts AddOptions) { addLog.Printf("Adding workflow: name=%s, content_size=%d bytes", workflowSpec.WorkflowName, len(sourceContent)) if !opts.Verbose { @@ -843,7 +857,9 @@ func addActionWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTrac } return fmt.Errorf("action workflow '%s' already exists in %s. Use --force to overwrite", workflowName+".yml", githubWorkflowsDir) } - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Overwriting existing file: "+destFile)) + if !opts.showInteractiveProgress { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Overwriting existing file: "+destFile)) + } } if tracker != nil { diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index 32030a5adae..463aedc12a3 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -61,6 +61,7 @@ func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx conte GhAwRef: c.GhAwRef, initializedFiles: initFiles, workingTreePrevalidated: createPR, + showInteractiveProgress: true, } result, err := AddResolvedWorkflows(ctx, c.WorkflowSpecs, c.resolvedWorkflows, opts) if err != nil { diff --git a/pkg/cli/add_workflow_pr.go b/pkg/cli/add_workflow_pr.go index a287b949116..94bc2135847 100644 --- a/pkg/cli/add_workflow_pr.go +++ b/pkg/cli/add_workflow_pr.go @@ -95,6 +95,12 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts return 0, "", fmt.Errorf("failed to add workflows: %w", err) } + prepareSpinner := console.NewSpinner("Preparing pull request...") + if opts.showInteractiveProgress { + prepareSpinner.Start() + } + defer prepareSpinner.Stop() + // Stage all files before creating PR addWorkflowPRLog.Print("Staging workflow files") if err := tracker.StageAllFiles(opts.Verbose); err != nil { @@ -148,6 +154,9 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts // Push branch addWorkflowPRLog.Printf("Pushing branch %s to remote", branchName) + if opts.showInteractiveProgress { + prepareSpinner.UpdateMessage("Pushing pull request branch...") + } if err := pushBranch(branchName, opts.Verbose); err != nil { addWorkflowPRLog.Printf("Failed to push branch: %v", err) // Treat push failure as a warning: keep the files and commit intact so the @@ -163,6 +172,7 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts } // Create PR + prepareSpinner.Stop() addWorkflowPRLog.Printf("Creating pull request: %s", prTitle) prNumber, prURL, err := createPRForRepo(ctx, branchName, prTitle, prBody, opts.RepoSlug, opts.Verbose) if err != nil { From 2fc09ec1c4a6c447f6acc44804256a8abe8d536c Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 20:03:29 +0200 Subject: [PATCH 13/20] Improve add workflow pull request descriptions --- ...h-use-existing-copilot-token-add-wizard.md | 2 +- pkg/cli/add_command.go | 5 + pkg/cli/add_interactive_git.go | 42 +++-- pkg/cli/add_workflow_pr.go | 165 +++++++++++++++++- pkg/cli/add_workflow_pr_test.go | 56 ++++++ 5 files changed, 248 insertions(+), 22 deletions(-) diff --git a/.changeset/patch-use-existing-copilot-token-add-wizard.md b/.changeset/patch-use-existing-copilot-token-add-wizard.md index b611b231885..1b0b1d7f154 100644 --- a/.changeset/patch-use-existing-copilot-token-add-wizard.md +++ b/.changeset/patch-use-existing-copilot-token-add-wizard.md @@ -2,4 +2,4 @@ "gh-aw": patch --- -Default add-wizard to using an existing `COPILOT_GITHUB_TOKEN` secret, reuse fetched repository metadata, make repository authoring support files optional, improve workflow and Copilot authentication prompt formatting, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file +Default add-wizard to using an existing `COPILOT_GITHUB_TOKEN` secret, reuse fetched repository metadata, make repository authoring support files optional, improve workflow and Copilot authentication prompt formatting, generate detailed pull request descriptions, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index 50e4e0ca697..698d60e096e 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -95,6 +95,11 @@ type AddOptions struct { // showInteractiveProgress enables high-level progress indicators for the // otherwise quiet add-wizard write, compile, commit, and push phases. showInteractiveProgress bool + // createdByAddWizard records that the interactive wizard selected these options. + createdByAddWizard bool + addWizardSkipSecret bool + addWizardSecretExists bool + addWizardDisableGitHubAppInference bool } // AddWorkflowsResult contains the result of adding workflows diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index 463aedc12a3..2b84f1301c8 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -44,25 +44,29 @@ func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx conte // Pass Quiet=true to suppress detailed output (already shown earlier in interactive mode) // This returns the result including PR number and HasWorkflowDispatch opts := AddOptions{ - Verbose: c.Verbose, - Quiet: true, - EngineOverride: c.EngineOverride, - Name: "", - Force: c.forceOverwrite, - AppendText: c.AppendText, - CreatePR: createPR, - NoGitattributes: c.NoGitattributes, - WorkflowDir: c.WorkflowDir, - NoStopAfter: c.NoStopAfter, - StopAfter: c.StopAfter, - DisableSecurityScanner: c.DisableSecurityScanner, - RepoSlug: c.RepoOverride, - AddCopilotRequestsPermission: c.UseCopilotRequests, - GhAwRef: c.GhAwRef, - initializedFiles: initFiles, - workingTreePrevalidated: createPR, - showInteractiveProgress: true, - } + Verbose: c.Verbose, + Quiet: true, + EngineOverride: c.EngineOverride, + Name: "", + Force: c.forceOverwrite, + AppendText: c.AppendText, + CreatePR: createPR, + NoGitattributes: c.NoGitattributes, + WorkflowDir: c.WorkflowDir, + NoStopAfter: c.NoStopAfter, + StopAfter: c.StopAfter, + DisableSecurityScanner: c.DisableSecurityScanner, + RepoSlug: c.RepoOverride, + AddCopilotRequestsPermission: c.UseCopilotRequests, + GhAwRef: c.GhAwRef, + initializedFiles: initFiles, + workingTreePrevalidated: createPR, + showInteractiveProgress: true, + createdByAddWizard: true, + addWizardSkipSecret: c.SkipSecret, + addWizardDisableGitHubAppInference: c.DisableGitHubAppPermissionInference, + } + _, opts.addWizardSecretExists = c.existingSecrets["COPILOT_GITHUB_TOKEN"] result, err := AddResolvedWorkflows(ctx, c.WorkflowSpecs, c.resolvedWorkflows, opts) if err != nil { return fmt.Errorf("failed to add workflow: %w", err) diff --git a/pkg/cli/add_workflow_pr.go b/pkg/cli/add_workflow_pr.go index 94bc2135847..2e2f1db588d 100644 --- a/pkg/cli/add_workflow_pr.go +++ b/pkg/cli/add_workflow_pr.go @@ -4,12 +4,15 @@ import ( "context" "fmt" "math/rand" + "net/url" "os" "regexp" + "slices" "strings" "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/parser" "github.com/github/gh-aw/pkg/sliceutil" ) @@ -121,7 +124,6 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts joinedNames = workflows[0].Spec.WorkflowName commitMessage = "Add agentic workflow " + joinedNames prTitle = "Add agentic workflow " + joinedNames - prBody = "Add agentic workflow " + joinedNames } else { workflowNames := sliceutil.Map(workflows, func(wf *ResolvedWorkflow) string { return wf.Spec.WorkflowName @@ -129,8 +131,8 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts joinedNames = strings.Join(workflowNames, ", ") commitMessage = "Add agentic workflows: " + joinedNames prTitle = "Add agentic workflows: " + joinedNames - prBody = "Add agentic workflows: " + joinedNames } + prBody = buildAddWorkflowPRBody(workflows, opts) if err := commitChanges(commitMessage, opts.Verbose); err != nil { // Don't rollback - leave the workflow files on disk for manual recovery. @@ -193,3 +195,162 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Created pull request "+prURL)) return prNumber, prURL, nil } + +func buildAddWorkflowPRBody(workflows []*ResolvedWorkflow, opts AddOptions) string { + var body strings.Builder + if opts.createdByAddWizard { + fmt.Fprintf(&body, "This pull request was created with [`gh aw add-wizard`](https://github.github.com/gh-aw/) from [GitHub Agentic Workflows](https://github.com/github/gh-aw), version `%s`.\n", markdownText(GetVersion())) + } else { + fmt.Fprintf(&body, "This pull request was created with [`gh aw add`](https://github.github.com/gh-aw/) from [GitHub Agentic Workflows](https://github.com/github/gh-aw), version `%s`.\n", markdownText(GetVersion())) + } + + body.WriteString("\n## Workflows\n") + for _, resolved := range workflows { + fmt.Fprintf(&body, "\n### `%s`\n\n", markdownText(resolved.Spec.WorkflowName)) + if resolved.Description != "" { + fmt.Fprintf(&body, "%s\n\n", markdownText(resolved.Description)) + } + fmt.Fprintf(&body, "- **Source:** %s\n", workflowSourceMarkdown(resolved)) + fmt.Fprintf(&body, "- **Triggers:** %s\n", workflowTriggerSummary(resolved.Content)) + } + + body.WriteString("\n## Options selected\n\n") + body.WriteString("- **Delivery:** pull request\n") + fmt.Fprintf(&body, "- **Engine:** `%s`\n", markdownText(opts.EngineOverride)) + if opts.EngineOverride == "copilot" { + auth := "`COPILOT_GITHUB_TOKEN` repository secret" + if opts.AddCopilotRequestsPermission { + auth = "organization billing via `permissions.copilot-requests: write`" + } else if opts.addWizardSecretExists { + auth = "existing `COPILOT_GITHUB_TOKEN` repository or organization secret" + } else if opts.addWizardSkipSecret { + auth = "`COPILOT_GITHUB_TOKEN` setup skipped" + } + fmt.Fprintf(&body, "- **Authentication:** %s\n", auth) + } + fmt.Fprintf(&body, "- **Security scanner:** %s\n", enabledText(!opts.DisableSecurityScanner)) + fmt.Fprintf(&body, "- **Stop-after guard:** %s\n", stopAfterSummary(opts)) + fmt.Fprintf(&body, "- **Git attributes:** %s\n", enabledText(!opts.NoGitattributes)) + if opts.WorkflowDir != "" { + fmt.Fprintf(&body, "- **Workflow directory:** `%s`\n", markdownText(opts.WorkflowDir)) + } + if opts.GhAwRef != "" { + fmt.Fprintf(&body, "- **GitHub Agentic Workflows action reference:** `%s`\n", markdownText(opts.GhAwRef)) + } + if opts.Force { + body.WriteString("- **Existing workflow files:** overwrite confirmed\n") + } + if opts.createdByAddWizard { + fmt.Fprintf(&body, "- **GitHub App permission and event inference:** %s\n", enabledText(!opts.addWizardDisableGitHubAppInference)) + } + if opts.AppendText != "" { + body.WriteString("- **Custom appended instructions:** included\n") + } + if len(opts.initializedFiles) > 0 { + fmt.Fprintf(&body, "- **Repository initialization:** %s\n", joinCodeValues(opts.initializedFiles)) + } + + body.WriteString("\n## Review criteria\n\n") + body.WriteString("- Confirm each workflow's source, description, and triggers match the intended automation.\n") + body.WriteString("- Review the workflow permissions, network access, tools, and safe outputs before enabling it.\n") + body.WriteString("- Verify the generated `.lock.yml` changes contain only the expected compiled workflow behavior.\n") + if opts.EngineOverride == "copilot" && !opts.AddCopilotRequestsPermission { + body.WriteString("- Confirm `COPILOT_GITHUB_TOKEN` is available to the repository; its value is not included in this pull request.\n") + } + + body.WriteString("\n## Forward progress\n\n") + body.WriteString("1. Review the changes against the criteria above; request or make changes in the Markdown workflow source, then recompile it with `gh aw compile`.\n") + body.WriteString("2. Merge this pull request to install the workflow") + if opts.createdByAddWizard && opts.EngineOverride == "copilot" && !opts.AddCopilotRequestsPermission && !opts.addWizardSecretExists && !opts.addWizardSkipSecret { + body.WriteString(". After merge, the add wizard will configure `COPILOT_GITHUB_TOKEN` when needed") + } + body.WriteString(".\n") + body.WriteString("3. Monitor the first scheduled or manually dispatched run, then adjust the Markdown source and recompile if the workflow needs refinement.\n") + + return body.String() +} + +func workflowSourceMarkdown(resolved *ResolvedWorkflow) string { + label := markdownText(resolved.Spec.String()) + if resolved.Spec.RawURL != "" { + return fmt.Sprintf("[%s](%s)", label, resolved.Spec.RawURL) + } + if resolved.SourceInfo == nil || resolved.SourceInfo.IsLocal || resolved.Spec.RepoSlug == "" { + return "`" + label + "` (local source)" + } + host := resolved.Spec.Host + if host == "" { + host = "github.com" + } + ref := resolved.SourceInfo.CommitSHA + if ref == "" { + ref = resolved.Spec.Version + } + if ref == "" { + ref = "HEAD" + } + sourceURL := url.URL{Scheme: "https", Host: host, Path: "/" + resolved.Spec.RepoSlug + "/blob/" + ref + "/" + resolved.Spec.WorkflowPath} + return fmt.Sprintf("[%s](%s)", label, sourceURL.String()) +} + +func workflowTriggerSummary(content []byte) string { + frontmatter, err := parser.ExtractFrontmatterFromContent(string(content)) + if err != nil { + return "not detected" + } + on, found := frontmatter.Frontmatter["on"] + if !found { + return "not declared" + } + if trigger, ok := on.(string); ok { + return "`" + markdownText(trigger) + "`" + } + onMap, ok := on.(map[string]any) + if !ok { + return "declared in workflow frontmatter" + } + triggers := make([]string, 0, len(onMap)) + for trigger, config := range onMap { + summary := "`" + markdownText(trigger) + "`" + if trigger == "schedule" { + if schedule := detectWorkflowScheduleInfo(string(content)).RawExpr; schedule != "" { + summary += " (`" + markdownText(schedule) + "`)" + } + } else if configString, ok := config.(string); ok && configString != "" { + summary += " (`" + markdownText(configString) + "`)" + } + triggers = append(triggers, summary) + } + slices.Sort(triggers) + return strings.Join(triggers, ", ") +} + +func stopAfterSummary(opts AddOptions) string { + if opts.NoStopAfter { + return "disabled" + } + if opts.StopAfter != "" { + return "`" + markdownText(opts.StopAfter) + "`" + } + return "default" +} + +func enabledText(enabled bool) string { + if enabled { + return "enabled" + } + return "disabled" +} + +func markdownText(value string) string { + value = strings.Join(strings.Fields(value), " ") + return strings.NewReplacer("\\", "\\\\", "`", "\\`", "[", "\\[", "]", "\\]", "<", "<", ">", ">").Replace(value) +} + +func joinCodeValues(values []string) string { + formatted := make([]string, 0, len(values)) + for _, value := range values { + formatted = append(formatted, "`"+markdownText(value)+"`") + } + return strings.Join(formatted, ", ") +} diff --git a/pkg/cli/add_workflow_pr_test.go b/pkg/cli/add_workflow_pr_test.go index 5758ec5f458..55c087e7942 100644 --- a/pkg/cli/add_workflow_pr_test.go +++ b/pkg/cli/add_workflow_pr_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSanitizeBranchName(t *testing.T) { @@ -200,3 +201,58 @@ func TestSanitizeBranchName(t *testing.T) { }) } } + +func TestBuildAddWorkflowPRBody(t *testing.T) { + originalVersion := GetVersion() + SetVersionInfo("v1.2.3") + t.Cleanup(func() { SetVersionInfo(originalVersion) }) + + workflow := &ResolvedWorkflow{ + Spec: &WorkflowSpec{ + RepoSpec: RepoSpec{RepoSlug: "githubnext/agentics", Version: "main"}, + WorkflowPath: "workflows/repo-assist.md", + WorkflowName: "repo-assist", + }, + Content: []byte("---\ndescription: Helps maintain the repository\non:\n schedule: weekly\n workflow_dispatch:\n---\n"), + SourceInfo: &FetchedWorkflow{CommitSHA: "abc123", SourcePath: "workflows/repo-assist.md"}, + Description: "Helps maintain the repository", + } + opts := AddOptions{ + EngineOverride: "copilot", + createdByAddWizard: true, + addWizardSecretExists: true, + initializedFiles: []string{".gitattributes", ".github/aw/actions-lock.json"}, + addWizardDisableGitHubAppInference: true, + } + + body := buildAddWorkflowPRBody([]*ResolvedWorkflow{workflow}, opts) + + require.Contains(t, body, "[`gh aw add-wizard`](https://github.github.com/gh-aw/)") + assert.Contains(t, body, "[GitHub Agentic Workflows](https://github.com/github/gh-aw), version `v1.2.3`") + assert.Contains(t, body, "[githubnext/agentics/workflows/repo-assist.md@main](https://github.com/githubnext/agentics/blob/abc123/workflows/repo-assist.md)") + assert.Contains(t, body, "Helps maintain the repository") + assert.Contains(t, body, "`schedule` (`weekly`), `workflow_dispatch`") + assert.Contains(t, body, "**Delivery:** pull request") + assert.Contains(t, body, "existing `COPILOT_GITHUB_TOKEN` repository or organization secret") + assert.Contains(t, body, "**GitHub App permission and event inference:** disabled") + assert.Contains(t, body, "`.gitattributes`, `.github/aw/actions-lock.json`") + assert.Contains(t, body, "## Review criteria") + assert.Contains(t, body, "## Forward progress") + assert.NotContains(t, body, "will configure `COPILOT_GITHUB_TOKEN`") +} + +func TestBuildAddWorkflowPRBodyUsesLocalSourceAndSecretNextStep(t *testing.T) { + workflow := &ResolvedWorkflow{ + Spec: &WorkflowSpec{WorkflowPath: "./review.md", WorkflowName: "review"}, + Content: []byte("---\non: issues\n---\n"), + SourceInfo: &FetchedWorkflow{IsLocal: true, SourcePath: "./review.md"}, + } + opts := AddOptions{EngineOverride: "copilot", createdByAddWizard: true} + + body := buildAddWorkflowPRBody([]*ResolvedWorkflow{workflow}, opts) + + assert.Contains(t, body, "`./review.md` (local source)") + assert.Contains(t, body, "**Triggers:** `issues`") + assert.Contains(t, body, "After merge, the add wizard will configure `COPILOT_GITHUB_TOKEN` when needed") + assert.Contains(t, body, "recompile it with `gh aw compile`") +} From 3c1ed134f968fa5fd8ecfb7a1246bc41e79ea6d4 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 20:31:11 +0200 Subject: [PATCH 14/20] Centralize interactive prompt spacing --- pkg/cli/add_init.go | 11 ++++++----- pkg/cli/add_interactive_auth.go | 1 - pkg/cli/add_interactive_engine.go | 4 ++-- pkg/cli/add_interactive_git.go | 5 +---- pkg/cli/add_interactive_orchestrator.go | 3 +-- pkg/cli/add_interactive_schedule.go | 2 +- pkg/cli/add_interactive_workflow.go | 1 - pkg/cli/engine_secrets.go | 1 - pkg/console/prompt_form.go | 21 ++++----------------- pkg/console/prompt_form_test.go | 7 ++++--- 10 files changed, 19 insertions(+), 37 deletions(-) diff --git a/pkg/cli/add_init.go b/pkg/cli/add_init.go index 667c89b155c..02bdf0836e5 100644 --- a/pkg/cli/add_init.go +++ b/pkg/cli/add_init.go @@ -20,13 +20,14 @@ var addConfirmAuthoringSupport = func(ctx context.Context) (bool, error) { addAuthoringSupport := true form := console.NewConfirmForm( huh.NewConfirm(). - Title("Would you also like to add support to use coding agents in this repository to author, debug, update and audit agentic workflows?"). - Affirmative("Yes, add coding agent support"). + Title("Add prompts and skills for coding agents?"). + Description("These help coding agents author, debug, update, and audit agentic workflows in this repository."). + Affirmative("Yes, add prompts and skills"). Negative("No, add only the workflow"). Value(&addAuthoringSupport), ) if err := form.RunWithContext(ctx); err != nil { - return false, fmt.Errorf("coding agent support confirmation failed: %w", err) + return false, fmt.Errorf("coding agent prompts and skills confirmation failed: %w", err) } return addAuthoringSupport, nil } @@ -78,11 +79,11 @@ func confirmAddRepositoryInitialization(ctx context.Context, engineOverride stri confirmed, err := addConfirmAuthoringSupport(ctx) if err != nil || !confirmed { if err == nil { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Coding agent authoring support: skipped")) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Coding agent prompts and skills: skipped")) } return addRepositoryInitializationPlan{}, err } - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Coding agent authoring support: enabled")) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Coding agent prompts and skills: enabled")) return addRepositoryInitializationPlan{enabled: true, files: missingMarkers}, nil } diff --git a/pkg/cli/add_interactive_auth.go b/pkg/cli/add_interactive_auth.go index 6083d844b32..015eeeb8691 100644 --- a/pkg/cli/add_interactive_auth.go +++ b/pkg/cli/add_interactive_auth.go @@ -38,7 +38,6 @@ func (c *AddInteractiveConfig) checkGitRepository() error { // Ask the user for the repository (interactive-only feature) fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Could not determine the repository automatically.")) - fmt.Fprintln(os.Stderr, "") var userRepo string form := console.NewInputForm( diff --git a/pkg/cli/add_interactive_engine.go b/pkg/cli/add_interactive_engine.go index 1a747c97b5e..98205e90901 100644 --- a/pkg/cli/add_interactive_engine.go +++ b/pkg/cli/add_interactive_engine.go @@ -52,7 +52,7 @@ func (c *AddInteractiveConfig) selectAIEngineAndKey() error { Description("This determines which coding agent processes your workflows"). Options(engineOptions...). Value(&selectedEngine), - ).WithLeadingBlankLine() + ) if err := form.RunWithContext(c.Ctx); err != nil { return fmt.Errorf("failed to select coding agent: %w", err) @@ -296,7 +296,7 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { }) } - form := console.NewSelectForm(selectField).WithLeadingBlankLine() + form := console.NewSelectForm(selectField) if err := form.RunWithContext(c.Ctx); err != nil { return fmt.Errorf("failed to select Copilot authentication method: %w", err) diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index 2b84f1301c8..cbd47af3b0f 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -37,8 +37,6 @@ const ( func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx context.Context, workflowFiles, initFiles []string, secretName, secretValue string, createPR bool) error { addInteractiveLog.Print("Applying changes") - fmt.Fprintln(os.Stderr, "") - // Add the workflow using the existing implementation. // Pass the resolved workflows to avoid re-fetching them // Pass Quiet=true to suppress detailed output (already shown earlier in interactive mode) @@ -97,7 +95,6 @@ func (c *AddInteractiveConfig) ensurePullRequestMerged(prNumber int, prURL strin } fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Pull request created: "+prURL)) - fmt.Fprintln(os.Stderr, "") return c.runPRMergeLoop(prNumber, prURL) } @@ -447,7 +444,7 @@ func promptWorkingTreeResolution(ctx context.Context, blockers addWorkingTreeBlo Description(formatWorkingTreeBlockers(blockers)). Options(buildWorkingTreeResolutionOptions(allowOverwrite)...). Value(&resolution), - ).WithLeadingBlankLine() + ) if err := form.RunWithContext(ctx); err != nil { return "", fmt.Errorf("working tree confirmation failed: %w", err) } diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index f66f0cc20fc..d772e8e0eff 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -390,7 +390,6 @@ func (c *AddInteractiveConfig) confirmChanges(workflowFiles, initFiles []string) for _, f := range initFiles { fmt.Fprintf(os.Stderr, " • %s\n", f) } - fmt.Fprintln(os.Stderr, "") } createPR := true // Default to yes @@ -401,7 +400,7 @@ func (c *AddInteractiveConfig) confirmChanges(workflowFiles, initFiles []string) Affirmative("Yes, create pull request"). Negative("No, write files locally"). Value(&createPR), - ).WithLeadingBlankLine() + ) if err := form.RunWithContext(c.Ctx); err != nil { return false, fmt.Errorf("confirmation failed: %w", err) diff --git a/pkg/cli/add_interactive_schedule.go b/pkg/cli/add_interactive_schedule.go index 70e9f0f5285..7ea399d19b5 100644 --- a/pkg/cli/add_interactive_schedule.go +++ b/pkg/cli/add_interactive_schedule.go @@ -222,7 +222,7 @@ func (c *AddInteractiveConfig) selectScheduleFrequency() error { Description("Current schedule: " + rawExpr). Options(options...). Value(&selected), - ).WithLeadingBlankLine() + ) if err := form.RunWithContext(c.Ctx); err != nil { return fmt.Errorf("failed to select schedule frequency: %w", err) diff --git a/pkg/cli/add_interactive_workflow.go b/pkg/cli/add_interactive_workflow.go index 19070f2969c..6c52d79efcd 100644 --- a/pkg/cli/add_interactive_workflow.go +++ b/pkg/cli/add_interactive_workflow.go @@ -161,7 +161,6 @@ func (c *AddInteractiveConfig) showCodespaceRunInstructions() { func confirmRunAddedWorkflow(ctx context.Context) (bool, error) { // Ask if user wants to run the workflow - fmt.Fprintln(os.Stderr, "") runNow := true // Default to yes form := console.NewConfirmForm( huh.NewConfirm(). diff --git a/pkg/cli/engine_secrets.go b/pkg/cli/engine_secrets.go index d21776746de..f04e7e7e843 100644 --- a/pkg/cli/engine_secrets.go +++ b/pkg/cli/engine_secrets.go @@ -370,7 +370,6 @@ func promptForCopilotPATUnified(req SecretRequirement, config EngineSecretConfig fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, "Preconfigured token creation page:") console.PrintCommandMessage(" " + preconfiguredPATURL) - fmt.Fprintln(os.Stderr, "") openBrowser := true confirmForm := console.NewConfirmForm( diff --git a/pkg/console/prompt_form.go b/pkg/console/prompt_form.go index 99f5619e9b1..880c2be062d 100644 --- a/pkg/console/prompt_form.go +++ b/pkg/console/prompt_form.go @@ -23,9 +23,8 @@ const ( // caller prints the decision result. type PromptForm struct { *huh.Form - out io.Writer - clearOnRun bool - leadingBlankLine bool + out io.Writer + clearOnRun bool } // NewForm creates a huh form with gh-aw's default theme and accessibility mode. @@ -53,14 +52,6 @@ func NewConfirmForm(confirm *huh.Confirm) *PromptForm { return NewForm(huh.NewGroup(confirm)) } -// WithLeadingBlankLine separates the form from preceding status output. When -// completed forms are cleared, the separator is cleared with the question so -// the caller's resolution message remains adjacent to prior status messages. -func (f *PromptForm) WithLeadingBlankLine() *PromptForm { - f.leadingBlankLine = true - return f -} - // Run runs the form and removes its rendered question when it exits. func (f *PromptForm) Run() error { return f.run(func() error { return f.Form.Run() }) @@ -73,15 +64,11 @@ func (f *PromptForm) RunWithContext(ctx context.Context) error { func (f *PromptForm) run(runForm func() error) error { if !f.clearOnRun { - if f.leadingBlankLine { - fmt.Fprintln(f.out) - } + fmt.Fprintln(f.out) return runForm() } fmt.Fprint(f.out, ansiSaveCursor) - if f.leadingBlankLine { - fmt.Fprintln(f.out) - } + fmt.Fprintln(f.out) defer fmt.Fprint(f.out, ansiRestoreCursor, ansiClearScreenBelow) return runForm() } diff --git a/pkg/console/prompt_form_test.go b/pkg/console/prompt_form_test.go index 9ed064bff7c..993533b7e66 100644 --- a/pkg/console/prompt_form_test.go +++ b/pkg/console/prompt_form_test.go @@ -14,7 +14,8 @@ import ( func TestPromptWrappersReturnNonNilForms(t *testing.T) { var inputValue string - require.NotNil(t, NewInputForm(huh.NewInput().Value(&inputValue))) + inputForm := NewInputForm(huh.NewInput().Value(&inputValue)) + require.NotNil(t, inputForm) var selectValue string require.NotNil(t, NewSelectForm(huh.NewSelect[string](). @@ -27,7 +28,7 @@ func TestPromptWrappersReturnNonNilForms(t *testing.T) { func TestPromptFormClearsCompletedQuestion(t *testing.T) { var output bytes.Buffer - form := (&PromptForm{out: &output, clearOnRun: true}).WithLeadingBlankLine() + form := &PromptForm{out: &output, clearOnRun: true} err := form.run(func() error { return nil }) @@ -37,7 +38,7 @@ func TestPromptFormClearsCompletedQuestion(t *testing.T) { func TestPromptFormDoesNotClearAccessibleOrNonTTYQuestion(t *testing.T) { var output bytes.Buffer - form := (&PromptForm{out: &output, clearOnRun: false}).WithLeadingBlankLine() + form := &PromptForm{out: &output, clearOnRun: false} err := form.run(func() error { return nil }) From 2443eed560a7d9ea3e9b5a982202121ed0469c21 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 20:44:28 +0200 Subject: [PATCH 15/20] improve add-wizard --- pkg/cli/add_workflow_pr.go | 6 +++++- pkg/cli/add_workflow_pr_test.go | 23 +++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/pkg/cli/add_workflow_pr.go b/pkg/cli/add_workflow_pr.go index 2e2f1db588d..a9e3dff9e7e 100644 --- a/pkg/cli/add_workflow_pr.go +++ b/pkg/cli/add_workflow_pr.go @@ -208,7 +208,7 @@ func buildAddWorkflowPRBody(workflows []*ResolvedWorkflow, opts AddOptions) stri for _, resolved := range workflows { fmt.Fprintf(&body, "\n### `%s`\n\n", markdownText(resolved.Spec.WorkflowName)) if resolved.Description != "" { - fmt.Fprintf(&body, "%s\n\n", markdownText(resolved.Description)) + fmt.Fprintf(&body, "%s\n\n", markdownBlock(resolved.Description)) } fmt.Fprintf(&body, "- **Source:** %s\n", workflowSourceMarkdown(resolved)) fmt.Fprintf(&body, "- **Triggers:** %s\n", workflowTriggerSummary(resolved.Content)) @@ -347,6 +347,10 @@ func markdownText(value string) string { return strings.NewReplacer("\\", "\\\\", "`", "\\`", "[", "\\[", "]", "\\]", "<", "<", ">", ">").Replace(value) } +func markdownBlock(value string) string { + return strings.TrimSpace(strings.ReplaceAll(value, "\r\n", "\n")) +} + func joinCodeValues(values []string) string { formatted := make([]string, 0, len(values)) for _, value := range values { diff --git a/pkg/cli/add_workflow_pr_test.go b/pkg/cli/add_workflow_pr_test.go index 55c087e7942..aba12594f0d 100644 --- a/pkg/cli/add_workflow_pr_test.go +++ b/pkg/cli/add_workflow_pr_test.go @@ -256,3 +256,26 @@ func TestBuildAddWorkflowPRBodyUsesLocalSourceAndSecretNextStep(t *testing.T) { assert.Contains(t, body, "After merge, the add wizard will configure `COPILOT_GITHUB_TOKEN` when needed") assert.Contains(t, body, "recompile it with `gh aw compile`") } + +func TestBuildAddWorkflowPRBodyPreservesDescriptionMarkdown(t *testing.T) { + content := `--- +description: | + A friendly repository assistant. + + - Labels and triages open issues + - Creates draft pull requests with fixes +on: workflow_dispatch +--- +` + workflow := &ResolvedWorkflow{ + Spec: &WorkflowSpec{WorkflowPath: "./repo-assist.md", WorkflowName: "repo-assist"}, + Content: []byte(content), + SourceInfo: &FetchedWorkflow{IsLocal: true, SourcePath: "./repo-assist.md"}, + Description: ExtractWorkflowDescription(content), + } + + body := buildAddWorkflowPRBody([]*ResolvedWorkflow{workflow}, AddOptions{EngineOverride: "copilot"}) + + assert.Contains(t, body, "A friendly repository assistant.\n\n- Labels and triages open issues\n- Creates draft pull requests with fixes") + assert.NotContains(t, body, "assistant. - Labels") +} From 5f95160376ec594112f748450a739e9b61cb9995 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 21:02:40 +0200 Subject: [PATCH 16/20] Polish add-wizard workflow handoff --- pkg/cli/add_interactive_git.go | 16 ++++----- pkg/cli/add_interactive_workflow.go | 36 ++++++++----------- pkg/cli/run_input_validation_test.go | 22 ++++++++++++ pkg/cli/run_interactive.go | 52 +++++++++++++++------------- pkg/cli/run_workflow_execution.go | 8 ++--- pkg/console/prompt_form.go | 20 +++++++++-- pkg/console/prompt_form_test.go | 3 +- 7 files changed, 94 insertions(+), 63 deletions(-) diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index cbd47af3b0f..cc6b2d492a0 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -124,8 +124,6 @@ func (c *AddInteractiveConfig) runPRMergeLoop(prNumber int, prURL string) error } case mergeActionReview: userReviewing = true - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Please review and merge the pull request: "+prURL)) - fmt.Fprintln(os.Stderr, "") case mergeActionConfirmed: fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Great – continuing with the merged pull request")) mergeDone = true @@ -141,12 +139,14 @@ func (c *AddInteractiveConfig) runPRMergeLoop(prNumber int, prURL string) error func promptMergeAction(prURL string, mergeFailed, userReviewing bool) (mergeAction, error) { var chosen mergeAction - selectForm := console.NewSelectForm( - huh.NewSelect[mergeAction](). - Title("What would you like to do with pull request " + prURL + "?"). - Options(buildMergeOptions(mergeFailed, userReviewing)...). - Value(&chosen), - ) + selectField := huh.NewSelect[mergeAction](). + Title("What would you like to do with pull request " + prURL + "?"). + Options(buildMergeOptions(mergeFailed, userReviewing)...). + Value(&chosen) + if userReviewing { + selectField = selectField.Description("Please review and merge the pull request before continuing: " + prURL) + } + selectForm := console.NewSelectForm(selectField) if err := selectForm.Run(); err != nil { return "", fmt.Errorf("failed to get user input: %w", err) } diff --git a/pkg/cli/add_interactive_workflow.go b/pkg/cli/add_interactive_workflow.go index 6c52d79efcd..3d6c995c616 100644 --- a/pkg/cli/add_interactive_workflow.go +++ b/pkg/cli/add_interactive_workflow.go @@ -18,8 +18,6 @@ func (c *AddInteractiveConfig) checkStatusAndOfferRun(ctx context.Context) error addInteractiveLog.Print("Checking workflow status and offering to run") // Wait a moment for GitHub to process the merge - fmt.Fprintln(os.Stderr, "") - workflowFound, err := c.waitForWorkflowStatus(ctx) if err != nil { return err @@ -185,19 +183,18 @@ func (c *AddInteractiveConfig) runAddedWorkflowOnce(ctx context.Context) error { return nil } - fmt.Fprintln(os.Stderr, "") c.updateLocalBranchBeforeWorkflowRun() if err := RunSpecificWorkflowInteractively(ctx, RunWorkflowOptions{ - WorkflowName: workflowName, - Verbose: c.Verbose, - EngineOverride: c.EngineOverride, - RepoOverride: c.RepoOverride, + WorkflowName: workflowName, + Verbose: c.Verbose, + EngineOverride: c.EngineOverride, + RepoOverride: c.RepoOverride, + requiredInputsOnly: true, }); err != nil { return err } - c.showWorkflowRunURL(workflowName) return nil } @@ -206,27 +203,22 @@ func (c *AddInteractiveConfig) updateLocalBranchBeforeWorkflowRun() { // merge (workflowFound is true). Doing this here—rather than immediately // after the PR merge—avoids a race where git fetch runs before GitHub's git // objects have been updated, which caused "workflow file not found" errors. + var spinner *console.SpinnerWrapper if !c.Verbose { - fmt.Fprintln(os.Stderr, "Updating local branch (this may take a few seconds)...") + spinner = console.NewSpinner("Updating local branch...") + spinner.Start() } if err := c.updateLocalBranch(); err != nil { + if spinner != nil { + spinner.Stop() + } addInteractiveLog.Printf("Failed to update local branch: %v", err) fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not update local branch: %v", err))) fmt.Fprintln(os.Stderr, "You may need to switch to your repository's default branch (for example 'main') and run 'git pull' manually before running the workflow.") + return } - if !c.Verbose { - fmt.Fprintln(os.Stderr, "Finished updating local branch.") - } -} - -func (c *AddInteractiveConfig) showWorkflowRunURL(workflowName string) { - // Get the run URL for step 10 - runInfo, err := getLatestWorkflowRunWithRetry(workflowName+".lock.yml", c.RepoOverride, c.Verbose) - if err == nil && runInfo.URL != "" { - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Workflow triggered successfully!")) - fmt.Fprintln(os.Stderr, "") - fmt.Fprintf(os.Stderr, "🔗 View workflow run: %s\n", runInfo.URL) + if spinner != nil { + spinner.StopWithMessage(console.FormatSuccessMessage("Updated local branch")) } } diff --git a/pkg/cli/run_input_validation_test.go b/pkg/cli/run_input_validation_test.go index 484b0d13c07..31f10809029 100644 --- a/pkg/cli/run_input_validation_test.go +++ b/pkg/cli/run_input_validation_test.go @@ -7,6 +7,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/github/gh-aw/pkg/workflow" ) func TestGetWorkflowInputs(t *testing.T) { @@ -116,6 +118,26 @@ jobs: } } +func TestRequiredWorkflowInputs(t *testing.T) { + inputs := map[string]*workflow.InputDefinition{ + "required": {Required: true}, + "optional": {Required: false}, + "nil": nil, + } + + filtered := requiredWorkflowInputs(inputs) + + if len(filtered) != 1 { + t.Fatalf("expected one required input, got %d", len(filtered)) + } + if _, found := filtered["required"]; !found { + t.Fatal("expected required input to be retained") + } + if _, found := filtered["optional"]; found { + t.Fatal("expected optional input to be omitted") + } +} + func TestValidateWorkflowInputs(t *testing.T) { t.Parallel() tests := []struct { diff --git a/pkg/cli/run_interactive.go b/pkg/cli/run_interactive.go index f4e7006bb68..03c7de0a2c2 100644 --- a/pkg/cli/run_interactive.go +++ b/pkg/cli/run_interactive.go @@ -364,15 +364,16 @@ func confirmExecution(ctx context.Context, wf *WorkflowOption, inputs []string) // RunWorkflowOptions holds parameters for RunSpecificWorkflowInteractively. type RunWorkflowOptions struct { - WorkflowName string - Verbose bool - EngineOverride string - RepoOverride string - RefOverride string - AutoMergePRs bool - Push bool - DryRun bool - Approve bool + WorkflowName string + Verbose bool + EngineOverride string + RepoOverride string + RefOverride string + AutoMergePRs bool + Push bool + DryRun bool + Approve bool + requiredInputsOnly bool } // RunSpecificWorkflowInteractively runs a specific workflow in interactive mode @@ -397,18 +398,15 @@ func RunSpecificWorkflowInteractively(ctx context.Context, opts RunWorkflowOptio // Continue without inputs - they might not be required inputs = nil } - - // Create workflow option for display - wf := &WorkflowOption{ - Name: opts.WorkflowName, - Description: buildWorkflowDescription(inputs), - FilePath: mdFile, - Inputs: inputs, + if opts.requiredInputsOnly { + inputs = requiredWorkflowInputs(inputs) } - // Show workflow info if there are inputs - if len(inputs) > 0 { - showWorkflowInfo(wf) + // Create the workflow option used to collect required inputs. + wf := &WorkflowOption{ + Name: opts.WorkflowName, + FilePath: mdFile, + Inputs: inputs, } // Collect workflow inputs if needed @@ -423,12 +421,6 @@ func RunSpecificWorkflowInteractively(ctx context.Context, opts RunWorkflowOptio return nil } - // Build command string for display - cmdStr := buildCommandString(opts.WorkflowName, inputValues, opts.RepoOverride, opts.RefOverride, opts.AutoMergePRs, opts.Push, opts.EngineOverride, opts.Approve) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("\nRunning workflow...")) - fmt.Fprintln(os.Stderr, console.FormatCommandMessage("Equivalent command: "+cmdStr)) - fmt.Fprintln(os.Stderr, "") - // Execute the workflow err = RunWorkflowOnGitHub(ctx, opts.WorkflowName, RunOptions{ Enable: false, @@ -450,6 +442,16 @@ func RunSpecificWorkflowInteractively(ctx context.Context, opts RunWorkflowOptio return nil } +func requiredWorkflowInputs(inputs map[string]*workflow.InputDefinition) map[string]*workflow.InputDefinition { + requiredInputs := make(map[string]*workflow.InputDefinition) + for name, definition := range inputs { + if definition != nil && definition.Required { + requiredInputs[name] = definition + } + } + return requiredInputs +} + // buildCommandString builds the equivalent command string for display func buildCommandString(workflowName string, inputs []string, repoOverride, refOverride string, autoMergePRs, push bool, engineOverride string, approve bool) string { parts := []string{string(constants.CLIExtensionPrefix), "run", workflowName} diff --git a/pkg/cli/run_workflow_execution.go b/pkg/cli/run_workflow_execution.go index 2ac6280129a..44d0c449b29 100644 --- a/pkg/cli/run_workflow_execution.go +++ b/pkg/cli/run_workflow_execution.go @@ -407,7 +407,7 @@ func executeWorkflowRun(ctx context.Context, lockFileName string, args []string, if output != "" { fmt.Fprintln(os.Stderr, console.FormatInfoMessage(output)) } - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Successfully triggered workflow: "+lockFileName)) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Triggered workflow: "+lockFileName)) executionLog.Printf("Workflow triggered successfully: %s", lockFileName) runInfo, runErr := resolveWorkflowRunInfo(lockFileName, output, opts) return &workflowRunExecutionResult{ @@ -441,9 +441,9 @@ func resolveWorkflowRunInfo(lockFileName, output string, opts RunOptions) (*Work func handleWorkflowRunInfo(runInfo *WorkflowRunInfo, runErr error, opts RunOptions) { if runErr == nil && runInfo != nil && runInfo.URL != "" { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("🔗 View workflow run: "+runInfo.URL)) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("View workflow run: "+runInfo.URL)) executionLog.Printf("Workflow run URL: %s (ID: %d)", runInfo.URL, runInfo.DatabaseID) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("💡 To analyze this run, use: %s audit %d", string(constants.CLIExtensionPrefix), runInfo.DatabaseID))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Analyze this run: %s audit %d", string(constants.CLIExtensionPrefix), runInfo.DatabaseID))) return } if opts.Verbose && runErr != nil { @@ -503,7 +503,7 @@ func resolveWorkflowTargetRepo(opts RunOptions) string { } func printWorkflowWaitMessage(autoMerge bool) { - message := "Waiting for workflow completion..." + message := "Waiting for workflow to complete..." if autoMerge { message = "Auto-merge PRs enabled - waiting for workflow completion..." } diff --git a/pkg/console/prompt_form.go b/pkg/console/prompt_form.go index 880c2be062d..0c65ca0366a 100644 --- a/pkg/console/prompt_form.go +++ b/pkg/console/prompt_form.go @@ -7,6 +7,7 @@ import ( "errors" "fmt" "io" + "strings" "charm.land/huh/v2" "github.com/github/gh-aw/pkg/styles" @@ -17,6 +18,7 @@ const ( ansiSaveCursor = "\0337" ansiRestoreCursor = "\0338" ansiClearScreenBelow = "\033[J" + promptReservedRows = 12 ) // PromptForm wraps a huh form so completed questions are removed before the @@ -30,10 +32,15 @@ type PromptForm struct { // NewForm creates a huh form with gh-aw's default theme and accessibility mode. func NewForm(groups ...*huh.Group) *PromptForm { accessible := IsAccessibleMode() + clearOnRun := tty.IsStderrTerminal() && !accessible + form := huh.NewForm(groups...).WithTheme(styles.HuhTheme).WithAccessible(accessible) + if clearOnRun { + form = form.WithHeight(promptReservedRows) + } return &PromptForm{ - Form: huh.NewForm(groups...).WithTheme(styles.HuhTheme).WithAccessible(accessible), + Form: form, out: stderrWriter(), - clearOnRun: tty.IsStderrTerminal() && !accessible, + clearOnRun: clearOnRun, } } @@ -67,12 +74,19 @@ func (f *PromptForm) run(runForm func() error) error { fmt.Fprintln(f.out) return runForm() } - fmt.Fprint(f.out, ansiSaveCursor) + // Reserve enough inline space before saving the cursor. Without this, a form + // rendered near the bottom of the terminal can scroll its saved origin upward, + // leaving completed question lines behind when the cursor is restored. + fmt.Fprint(f.out, strings.Repeat("\n", promptReservedRows), cursorUp(promptReservedRows), ansiSaveCursor) fmt.Fprintln(f.out) defer fmt.Fprint(f.out, ansiRestoreCursor, ansiClearScreenBelow) return runForm() } +func cursorUp(rows int) string { + return fmt.Sprintf("\033[%dA", rows) +} + // IsCancelled reports whether err represents a deliberate user cancellation // (Ctrl-C / Esc before form submission, i.e. huh.ErrUserAborted). // Use this to distinguish graceful cancellation from genuine failures. diff --git a/pkg/console/prompt_form_test.go b/pkg/console/prompt_form_test.go index 993533b7e66..21534d8cb42 100644 --- a/pkg/console/prompt_form_test.go +++ b/pkg/console/prompt_form_test.go @@ -6,6 +6,7 @@ import ( "bytes" "errors" "fmt" + "strings" "testing" "charm.land/huh/v2" @@ -33,7 +34,7 @@ func TestPromptFormClearsCompletedQuestion(t *testing.T) { err := form.run(func() error { return nil }) require.NoError(t, err) - require.Equal(t, ansiSaveCursor+"\n"+ansiRestoreCursor+ansiClearScreenBelow, output.String()) + require.Equal(t, strings.Repeat("\n", promptReservedRows)+cursorUp(promptReservedRows)+ansiSaveCursor+"\n"+ansiRestoreCursor+ansiClearScreenBelow, output.String()) } func TestPromptFormDoesNotClearAccessibleOrNonTTYQuestion(t *testing.T) { From 0687eb4e423b9c92ed9bd9d12e629a9249bb9d56 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 21:17:35 +0200 Subject: [PATCH 17/20] Fix add-wizard Go lint findings --- pkg/cli/add_interactive_git.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index cc6b2d492a0..99bbe34c1c1 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strconv" "strings" @@ -373,9 +374,7 @@ func (c *AddInteractiveConfig) plannedAddPaths(workflowFiles, initFiles []string for _, path := range workflowFiles { planned = append(planned, filepath.Join(workflowDir, path)) } - for _, path := range initFiles { - planned = append(planned, path) - } + planned = append(planned, initFiles...) for index, path := range planned { if filepath.IsAbs(path) { rel, relErr := filepath.Rel(gitRoot, path) @@ -428,10 +427,8 @@ func inspectAddWorkingTree(plannedPaths []string) (addWorkingTreeBlockers, error } func appendUniqueString(values []string, value string) []string { - for _, existing := range values { - if existing == value { - return values - } + if slices.Contains(values, value) { + return values } return append(values, value) } From a5ccc64a0e6b0e0c0678d9b6999baab5e83b194e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 20:05:17 +0000 Subject: [PATCH 18/20] Address add-wizard review feedback Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- ...h-use-existing-copilot-token-add-wizard.md | 4 +- pkg/cli/add_command.go | 4 + pkg/cli/add_command_test.go | 39 ++--- pkg/cli/add_init.go | 158 +++++++++++------- pkg/cli/add_interactive_auth.go | 3 +- pkg/cli/add_interactive_engine.go | 10 +- pkg/cli/add_interactive_engine_test.go | 8 +- pkg/cli/add_interactive_git.go | 23 ++- pkg/cli/add_interactive_orchestrator.go | 19 ++- pkg/cli/add_interactive_orchestrator_test.go | 22 ++- pkg/cli/add_interactive_secrets.go | 40 ++++- pkg/cli/add_interactive_secrets_test.go | 15 +- pkg/cli/add_workflow_compilation.go | 4 - pkg/cli/add_workflow_pr.go | 45 ++++- pkg/cli/add_workflow_pr_test.go | 24 ++- pkg/cli/preconditions.go | 10 +- 16 files changed, 285 insertions(+), 143 deletions(-) diff --git a/.changeset/patch-use-existing-copilot-token-add-wizard.md b/.changeset/patch-use-existing-copilot-token-add-wizard.md index 1b0b1d7f154..f0fb4deeadf 100644 --- a/.changeset/patch-use-existing-copilot-token-add-wizard.md +++ b/.changeset/patch-use-existing-copilot-token-add-wizard.md @@ -1,5 +1,5 @@ --- -"gh-aw": patch +"gh-aw": minor --- -Default add-wizard to using an existing `COPILOT_GITHUB_TOKEN` secret, reuse fetched repository metadata, make repository authoring support files optional, improve workflow and Copilot authentication prompt formatting, generate detailed pull request descriptions, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file +Default add-wizard to use an existing `COPILOT_GITHUB_TOKEN` secret, reuse fetched repository metadata, make repository authoring support files optional, improve workflow and Copilot authentication prompt formatting, generate detailed pull request descriptions, and support `--gh-aw-ref` in both `add` and `add-wizard`. \ No newline at end of file diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index 698d60e096e..8f2d9f4e1a4 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -89,6 +89,9 @@ type AddOptions struct { AddCopilotRequestsPermission bool // initializedFiles contains files created by add-wizard after its clean-tree check. initializedFiles []string + // initializedOriginalContents contains the pre-initialization contents of files + // updated by add-wizard, keyed by absolute path. + initializedOriginalContents map[string][]byte // workingTreePrevalidated indicates add-wizard already verified that staged // changes and changes overlapping planned files are absent. workingTreePrevalidated bool @@ -99,6 +102,7 @@ type AddOptions struct { createdByAddWizard bool addWizardSkipSecret bool addWizardSecretExists bool + addWizardSecretSource string addWizardDisableGitHubAppInference bool } diff --git a/pkg/cli/add_command_test.go b/pkg/cli/add_command_test.go index c583f297158..4780141ffa3 100644 --- a/pkg/cli/add_command_test.go +++ b/pkg/cli/add_command_test.go @@ -560,13 +560,11 @@ func TestConfirmAndInitializeAddRepository(t *testing.T) { originalFindGitRoot := addFindGitRoot originalInitRepository := addInitRepository originalMissingInitMarkers := addMissingInitMarkers - originalMissingAuthoringSupportFiles := addMissingAuthoringSupportFiles originalConfirmAuthoringSupport := addConfirmAuthoringSupport t.Cleanup(func() { addFindGitRoot = originalFindGitRoot addInitRepository = originalInitRepository addMissingInitMarkers = originalMissingInitMarkers - addMissingAuthoringSupportFiles = originalMissingAuthoringSupportFiles addConfirmAuthoringSupport = originalConfirmAuthoringSupport }) @@ -574,7 +572,7 @@ func TestConfirmAndInitializeAddRepository(t *testing.T) { addFindGitRoot = func() (string, error) { return repoDir, nil } t.Run("already initialized skips confirmation", func(t *testing.T) { - addMissingAuthoringSupportFiles = func(string, string, bool) ([]string, error) { return nil, nil } + addMissingInitMarkers = func(string, string) ([]string, error) { return nil, nil } addConfirmAuthoringSupport = func(context.Context) (bool, error) { t.Fatal("confirmation should not be shown when all support files exist") return false, nil @@ -590,7 +588,7 @@ func TestConfirmAndInitializeAddRepository(t *testing.T) { }) t.Run("declining creates no support files", func(t *testing.T) { - addMissingAuthoringSupportFiles = func(string, string, bool) ([]string, error) { + addMissingInitMarkers = func(string, string) ([]string, error) { return []string{bootstrapAgenticSkillPath}, nil } addConfirmAuthoringSupport = func(context.Context) (bool, error) { return false, nil } @@ -606,7 +604,6 @@ func TestConfirmAndInitializeAddRepository(t *testing.T) { t.Run("accepting quietly initializes support files", func(t *testing.T) { marker := ".vscode/settings.json" - addMissingAuthoringSupportFiles = func(string, string, bool) ([]string, error) { return []string{marker}, nil } addMissingInitMarkers = func(string, string) ([]string, error) { return []string{marker}, nil } addConfirmAuthoringSupport = func(context.Context) (bool, error) { return true, nil } addInitRepository = func(opts InitOptions) error { @@ -621,24 +618,24 @@ func TestConfirmAndInitializeAddRepository(t *testing.T) { require.NoError(t, err) require.Equal(t, []string{filepath.Join(repoDir, filepath.FromSlash(marker))}, files) }) -} - -func TestMissingAddAuthoringSupportFiles(t *testing.T) { - repoDir := t.TempDir() - for _, path := range expectedBootstrapInitMarkers("copilot") { - fullPath := filepath.Join(repoDir, filepath.FromSlash(path)) - require.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0755)) - require.NoError(t, os.WriteFile(fullPath, nil, 0644)) - } - missing, err := missingAddAuthoringSupportFiles(repoDir, "copilot", false) - require.NoError(t, err) - assert.Empty(t, missing, "existing support files should suppress the optional setup prompt") + t.Run("preserves original contents for stale support files", func(t *testing.T) { + marker := ".vscode/settings.json" + markerPath := filepath.Join(repoDir, filepath.FromSlash(marker)) + require.NoError(t, os.WriteFile(markerPath, []byte("original"), 0644)) + addMissingInitMarkers = func(string, string) ([]string, error) { return []string{marker}, nil } + addConfirmAuthoringSupport = func(context.Context) (bool, error) { return true, nil } + addInitRepository = func(InitOptions) error { + return os.WriteFile(markerPath, []byte("updated"), 0644) + } - require.NoError(t, os.Remove(filepath.Join(repoDir, ".gitattributes"))) - missing, err = missingAddAuthoringSupportFiles(repoDir, "copilot", true) - require.NoError(t, err) - assert.Empty(t, missing, "--no-gitattributes should not require .gitattributes") + plan, err := confirmAddRepositoryInitialization(context.Background(), "copilot", false) + require.NoError(t, err) + files, originalContents, err := applyAddRepositoryInitialization(plan, "copilot", false, false) + require.NoError(t, err) + require.Equal(t, []string{markerPath}, files) + assert.Equal(t, []byte("original"), originalContents[markerPath]) + }) } func TestAddResolvedWorkflows_IgnoresBootstrapRequireOwnerTypeDuringInstall(t *testing.T) { diff --git a/pkg/cli/add_init.go b/pkg/cli/add_init.go index 02bdf0836e5..a44fea2f772 100644 --- a/pkg/cli/add_init.go +++ b/pkg/cli/add_init.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "charm.land/huh/v2" "github.com/github/gh-aw/pkg/console" @@ -15,7 +16,6 @@ import ( var addFindGitRoot = gitutil.FindGitRoot var addInitRepository = InitRepository var addMissingInitMarkers = missingBootstrapInitMarkers -var addMissingAuthoringSupportFiles = missingAddAuthoringSupportFiles var addConfirmAuthoringSupport = func(ctx context.Context) (bool, error) { addAuthoringSupport := true form := console.NewConfirmForm( @@ -32,27 +32,10 @@ var addConfirmAuthoringSupport = func(ctx context.Context) (bool, error) { return addAuthoringSupport, nil } -func missingAddAuthoringSupportFiles(baseDir string, engineOverride string, noGitattributes bool) ([]string, error) { - var missing []string - for _, path := range expectedBootstrapInitMarkers(engineOverride) { - if noGitattributes && path == ".gitattributes" { - continue - } - info, err := os.Stat(filepath.Join(baseDir, filepath.FromSlash(path))) - if err == nil && info.Mode().IsRegular() { - continue - } - if err != nil && !errors.Is(err, os.ErrNotExist) { - return nil, fmt.Errorf("failed to inspect %s: %w", path, err) - } - missing = append(missing, path) - } - return missing, nil -} - type addRepositoryInitializationPlan struct { - enabled bool - files []string + enabled bool + files []string + originalContents map[string][]byte } func confirmAddRepositoryInitialization(ctx context.Context, engineOverride string, noGitattributes bool) (addRepositoryInitializationPlan, error) { @@ -65,10 +48,27 @@ func confirmAddRepositoryInitialization(ctx context.Context, engineOverride stri } var missingMarkers []string + originalContents := make(map[string][]byte) if err := withWorkingDir(gitRoot, func() error { var inspectErr error - missingMarkers, inspectErr = addMissingAuthoringSupportFiles(".", engineOverride, noGitattributes) - return inspectErr + missingMarkers, inspectErr = addMissingInitMarkers(".", engineOverride) + if inspectErr != nil { + return inspectErr + } + if noGitattributes { + missingMarkers = slices.DeleteFunc(missingMarkers, func(path string) bool { + return path == ".gitattributes" + }) + } + for _, marker := range missingMarkers { + content, readErr := os.ReadFile(filepath.FromSlash(marker)) + if readErr == nil { + originalContents[marker] = content + } else if !errors.Is(readErr, os.ErrNotExist) { + return fmt.Errorf("failed to read %s: %w", marker, readErr) + } + } + return nil }); err != nil { return addRepositoryInitializationPlan{}, fmt.Errorf("failed to inspect repository initialization state: %w", err) } @@ -84,14 +84,26 @@ func confirmAddRepositoryInitialization(ctx context.Context, engineOverride stri return addRepositoryInitializationPlan{}, err } fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Coding agent prompts and skills: enabled")) - return addRepositoryInitializationPlan{enabled: true, files: missingMarkers}, nil + return addRepositoryInitializationPlan{enabled: true, files: missingMarkers, originalContents: originalContents}, nil } -func applyAddRepositoryInitialization(plan addRepositoryInitializationPlan, engineOverride string, verbose bool, noGitattributes bool) ([]string, error) { +func applyAddRepositoryInitialization(plan addRepositoryInitializationPlan, engineOverride string, verbose bool, noGitattributes bool) ([]string, map[string][]byte, error) { if !plan.enabled { - return nil, nil + return nil, nil, nil + } + files, err := ensureAddRepositoryInitializedFromPlan(plan.files, engineOverride, verbose, noGitattributes) + if err != nil { + return nil, nil, err + } + originalContents := make(map[string][]byte, len(plan.originalContents)) + gitRoot, err := addFindGitRoot() + if err != nil { + return nil, nil, fmt.Errorf("failed to determine repository root for initialized files: %w", err) } - return ensureAddRepositoryInitializedWithDetails(engineOverride, verbose, noGitattributes) + for path, content := range plan.originalContents { + originalContents[filepath.Join(gitRoot, filepath.FromSlash(path))] = content + } + return files, originalContents, nil } func confirmAndInitializeAddRepository(ctx context.Context, engineOverride string, verbose bool, noGitattributes bool) ([]string, error) { @@ -99,7 +111,8 @@ func confirmAndInitializeAddRepository(ctx context.Context, engineOverride strin if err != nil { return nil, err } - return applyAddRepositoryInitialization(plan, engineOverride, verbose, noGitattributes) + files, _, err := applyAddRepositoryInitialization(plan, engineOverride, verbose, noGitattributes) + return files, err } func ensureAddRepositoryInitialized(engineOverride string, verbose bool, noGitattributes bool) error { @@ -123,43 +136,8 @@ func ensureAddRepositoryInitializedWithDetails(engineOverride string, verbose bo if err != nil { return fmt.Errorf("failed to inspect repository initialization state: %w", err) } - if len(missingMarkers) == 0 { - return nil - } - - addLog.Printf("Repository missing init markers; running init: %v", missingMarkers) - if err := addInitRepository(InitOptions{ - Verbose: verbose, - Quiet: true, - Engine: engineOverride, - NoGitattributes: noGitattributes, - Skill: true, - Agent: true, - MCP: true, - CodespaceRepos: []string{}, - CodespaceEnabled: false, - Completions: false, - CreatePR: false, - }); err != nil { - return fmt.Errorf("failed to initialize repository for agentic workflows: %w", err) - } - - // Record only the files that were actually written by init (some markers, - // e.g. .gitattributes with --no-gitattributes, may intentionally be skipped). - // Use absolute paths so callers don't need to resolve against gitRoot. - for _, marker := range missingMarkers { - ok, statErr := isBootstrapInitMarkerSatisfied(".", marker) - if statErr != nil || !ok { - continue - } - absPath, pathErr := filepath.Abs(marker) - if pathErr != nil { - return fmt.Errorf("failed to resolve path for initialized file %s: %w", marker, pathErr) - } - initializedFiles = append(initializedFiles, absPath) - } - - return nil + initializedFiles, err = initializeAddRepositoryFiles(missingMarkers, engineOverride, verbose, noGitattributes) + return err }) if err != nil { return nil, err @@ -167,3 +145,53 @@ func ensureAddRepositoryInitializedWithDetails(engineOverride string, verbose bo return initializedFiles, nil } + +func ensureAddRepositoryInitializedFromPlan(markers []string, engineOverride string, verbose bool, noGitattributes bool) ([]string, error) { + gitRoot, err := addFindGitRoot() + if err != nil { + return nil, fmt.Errorf("failed to determine repository root for automatic initialization: %w", err) + } + var initializedFiles []string + err = withWorkingDir(gitRoot, func() error { + var initErr error + initializedFiles, initErr = initializeAddRepositoryFiles(markers, engineOverride, verbose, noGitattributes) + return initErr + }) + return initializedFiles, err +} + +func initializeAddRepositoryFiles(markers []string, engineOverride string, verbose bool, noGitattributes bool) ([]string, error) { + if len(markers) == 0 { + return nil, nil + } + addLog.Printf("Repository missing init markers; running init: %v", markers) + if err := addInitRepository(InitOptions{ + Verbose: verbose, + Quiet: true, + Engine: engineOverride, + NoGitattributes: noGitattributes, + Skill: true, + Agent: true, + MCP: true, + CodespaceRepos: []string{}, + CodespaceEnabled: false, + Completions: false, + CreatePR: false, + }); err != nil { + return nil, fmt.Errorf("failed to initialize repository for agentic workflows: %w", err) + } + + initializedFiles := make([]string, 0, len(markers)) + for _, marker := range markers { + ok, statErr := isBootstrapInitMarkerSatisfied(".", marker) + if statErr != nil || !ok { + continue + } + absPath, pathErr := filepath.Abs(marker) + if pathErr != nil { + return nil, fmt.Errorf("failed to resolve path for initialized file %s: %w", marker, pathErr) + } + initializedFiles = append(initializedFiles, absPath) + } + return initializedFiles, nil +} diff --git a/pkg/cli/add_interactive_auth.go b/pkg/cli/add_interactive_auth.go index 015eeeb8691..81b5cf68a59 100644 --- a/pkg/cli/add_interactive_auth.go +++ b/pkg/cli/add_interactive_auth.go @@ -67,8 +67,7 @@ func (c *AddInteractiveConfig) checkGitRepository() error { fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Target repository: "+repoSlug)) addInteractiveLog.Printf("Target repository: %s", repoSlug) - // Check if repository is public or private - c.isPublicRepo = checkRepoVisibilityShared(c.RepoOverride) + c.repoVisibility = getRepoVisibilityShared(c.RepoOverride) return nil } diff --git a/pkg/cli/add_interactive_engine.go b/pkg/cli/add_interactive_engine.go index 98205e90901..849c831493d 100644 --- a/pkg/cli/add_interactive_engine.go +++ b/pkg/cli/add_interactive_engine.go @@ -283,7 +283,7 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { var authMethod string selectField := huh.NewSelect[string](). Title("How would you like Copilot workflows to authenticate?"). - Description(copilotAuthMethodDescription(probe)). + Description(copilotAuthMethodDescription(probe, c.secretSources["COPILOT_GITHUB_TOKEN"])). Options(options...). Value(&authMethod) @@ -306,12 +306,16 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { return nil } -func copilotAuthMethodDescription(probe orgCopilotBillingProbeResult) string { +func copilotAuthMethodDescription(probe orgCopilotBillingProbeResult, secretSource string) string { copilotRequestsDescription := "• copilot-requests: Use the org's Copilot billing seat; no PAT required." if probe.InfoNote != "" { copilotRequestsDescription += "\n (NOTE: " + probe.InfoNote + "\n Check with your org admin if you want to use this option.)" } - return "• PAT: Use the existing COPILOT_GITHUB_TOKEN repository secret.\n" + copilotRequestsDescription + patDescription := "• PAT: Create or use a COPILOT_GITHUB_TOKEN repository secret." + if secretSource != "" { + patDescription = "• PAT: Reuse the existing COPILOT_GITHUB_TOKEN " + secretSource + "." + } + return patDescription + "\n" + copilotRequestsDescription } // applyCopilotAuthMethodChoice records the user's Copilot auth method selection and prints diff --git a/pkg/cli/add_interactive_engine_test.go b/pkg/cli/add_interactive_engine_test.go index 550e396b881..7927a365dcf 100644 --- a/pkg/cli/add_interactive_engine_test.go +++ b/pkg/cli/add_interactive_engine_test.go @@ -60,13 +60,13 @@ func TestCopilotAuthMethodDescription(t *testing.T) { t.Parallel() t.Run("bullets both authentication methods", func(t *testing.T) { - description := copilotAuthMethodDescription(orgCopilotBillingProbeResult{}) - assert.Equal(t, "• PAT: Use the existing COPILOT_GITHUB_TOKEN repository secret.\n• copilot-requests: Use the org's Copilot billing seat; no PAT required.", description) + description := copilotAuthMethodDescription(orgCopilotBillingProbeResult{}, "") + assert.Equal(t, "• PAT: Create or use a COPILOT_GITHUB_TOKEN repository secret.\n• copilot-requests: Use the org's Copilot billing seat; no PAT required.", description) }) t.Run("includes inconclusive billing note in copilot-requests bullet", func(t *testing.T) { - description := copilotAuthMethodDescription(orgCopilotBillingProbeResult{InfoNote: copilotBillingInconclusiveNote}) - assert.Equal(t, "• PAT: Use the existing COPILOT_GITHUB_TOKEN repository secret.\n• copilot-requests: Use the org's Copilot billing seat; no PAT required.\n (NOTE: Could not confirm org Copilot CLI billing.\n Check with your org admin if you want to use this option.)", description) + description := copilotAuthMethodDescription(orgCopilotBillingProbeResult{InfoNote: copilotBillingInconclusiveNote}, repositorySecretSource) + assert.Equal(t, "• PAT: Reuse the existing COPILOT_GITHUB_TOKEN repository secret.\n• copilot-requests: Use the org's Copilot billing seat; no PAT required.\n (NOTE: Could not confirm org Copilot CLI billing.\n Check with your org admin if you want to use this option.)", description) }) } diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index 99bbe34c1c1..598f077d935 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -59,6 +59,7 @@ func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx conte AddCopilotRequestsPermission: c.UseCopilotRequests, GhAwRef: c.GhAwRef, initializedFiles: initFiles, + initializedOriginalContents: c.initializedOriginalContents, workingTreePrevalidated: createPR, showInteractiveProgress: true, createdByAddWizard: true, @@ -66,6 +67,7 @@ func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx conte addWizardDisableGitHubAppInference: c.DisableGitHubAppPermissionInference, } _, opts.addWizardSecretExists = c.existingSecrets["COPILOT_GITHUB_TOKEN"] + opts.addWizardSecretSource = c.secretSources["COPILOT_GITHUB_TOKEN"] result, err := AddResolvedWorkflows(ctx, c.WorkflowSpecs, c.resolvedWorkflows, opts) if err != nil { return fmt.Errorf("failed to add workflow: %w", err) @@ -330,13 +332,20 @@ const ( // but requires staged changes and edits to files the wizard will write to be cleaned. func (c *AddInteractiveConfig) checkCleanWorkingDirectoryForPR(workflowFiles, initFiles []string) error { addInteractiveLog.Print("Checking working tree changes before PR creation") - plannedPaths, err := c.plannedAddPaths(workflowFiles, initFiles) + gitRoot, err := addFindGitRoot() + if err != nil { + return fmt.Errorf("failed to determine repository root for PR preflight: %w", err) + } + plannedPaths, err := c.plannedAddPathsAtRoot(gitRoot, workflowFiles, initFiles) if err != nil { return err } for { - blockers, inspectErr := inspectAddWorkingTree(plannedPaths) + if err := c.Ctx.Err(); err != nil { + return err + } + blockers, inspectErr := inspectAddWorkingTreeAtRoot(gitRoot, plannedPaths) if inspectErr != nil { return inspectErr } @@ -361,11 +370,7 @@ func (c *AddInteractiveConfig) checkCleanWorkingDirectoryForPR(workflowFiles, in } } -func (c *AddInteractiveConfig) plannedAddPaths(workflowFiles, initFiles []string) ([]string, error) { - gitRoot, err := addFindGitRoot() - if err != nil { - return nil, fmt.Errorf("failed to determine repository root for PR preflight: %w", err) - } +func (c *AddInteractiveConfig) plannedAddPathsAtRoot(gitRoot string, workflowFiles, initFiles []string) ([]string, error) { workflowDir := c.WorkflowDir if workflowDir == "" { workflowDir = getWorkflowsDir() @@ -393,6 +398,10 @@ func inspectAddWorkingTree(plannedPaths []string) (addWorkingTreeBlockers, error if err != nil { return addWorkingTreeBlockers{}, fmt.Errorf("failed to determine repository root for PR preflight: %w", err) } + return inspectAddWorkingTreeAtRoot(gitRoot, plannedPaths) +} + +func inspectAddWorkingTreeAtRoot(gitRoot string, plannedPaths []string) (addWorkingTreeBlockers, error) { cmd := exec.Command("git", "status", "--porcelain=v1", "-z", "--untracked-files=all") cmd.Dir = gitRoot output, err := cmd.Output() diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index d772e8e0eff..abc649d713b 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -50,9 +50,8 @@ type AddInteractiveConfig struct { // Populated by selectCopilotAuthMethod() via probeCopilotBillingForOrg(). copilotCLIBillingStatus string - // isPublicRepo tracks whether the target repository is public - // This is populated by checkGitRepository() when determining the repo - isPublicRepo bool + // repoVisibility is the target repository's public, private, or internal visibility. + repoVisibility string // hasWriteAccess tracks whether the user has write access to the target repository. // When false, secrets configuration is skipped since users cannot configure repository secrets. @@ -61,6 +60,7 @@ type AddInteractiveConfig struct { // existingSecrets tracks which secrets already exist in the repository // This is populated by checkExistingSecrets() before engine selection existingSecrets map[string]struct{} + secretSources map[string]string // addResult holds the result from AddWorkflows, including HasWorkflowDispatch addResult *AddWorkflowsResult @@ -72,6 +72,10 @@ type AddInteractiveConfig struct { // forceOverwrite records that the user chose to replace unstaged or untracked // files overlapping the wizard's planned output. Staged changes never enable it. forceOverwrite bool + + // initializedOriginalContents preserves files updated by repository initialization + // so pull request rollback can restore their pre-wizard contents. + initializedOriginalContents map[string][]byte } // RunAddInteractive runs the interactive add workflow @@ -235,13 +239,14 @@ func (c *AddInteractiveConfig) prepareAndConfirmAddInteractive() (workflowFiles, } } - initFiles, err = applyAddRepositoryInitialization(initializationPlan, c.EngineOverride, c.Verbose, c.NoGitattributes) + if !createPR { + return workflowFiles, nil, "", "", false, nil + } + + initFiles, c.initializedOriginalContents, err = applyAddRepositoryInitialization(initializationPlan, c.EngineOverride, c.Verbose, c.NoGitattributes) if err != nil { return nil, nil, "", "", false, err } - if !createPR { - return workflowFiles, initFiles, "", "", false, nil - } // Secret collection and upload only happen once the user has committed to the // PR path and the clean-tree check has succeeded. diff --git a/pkg/cli/add_interactive_orchestrator_test.go b/pkg/cli/add_interactive_orchestrator_test.go index b31cd51a854..b2ed5cd18fb 100644 --- a/pkg/cli/add_interactive_orchestrator_test.go +++ b/pkg/cli/add_interactive_orchestrator_test.go @@ -345,8 +345,22 @@ func TestAddInteractiveConfig_prepareAndConfirmAddInteractive_localWriteSkipsSec t.Setenv("PATH", tmpDir+string(os.PathListSeparator)+os.Getenv("PATH")) originalConfirmAuthoringSupport := addConfirmAuthoringSupport - addConfirmAuthoringSupport = func(context.Context) (bool, error) { return false, nil } - t.Cleanup(func() { addConfirmAuthoringSupport = originalConfirmAuthoringSupport }) + originalMissingInitMarkers := addMissingInitMarkers + originalInitRepository := addInitRepository + initializationRan := false + addConfirmAuthoringSupport = func(context.Context) (bool, error) { return true, nil } + addMissingInitMarkers = func(string, string) ([]string, error) { + return []string{bootstrapAgenticSkillPath}, nil + } + addInitRepository = func(InitOptions) error { + initializationRan = true + return nil + } + t.Cleanup(func() { + addConfirmAuthoringSupport = originalConfirmAuthoringSupport + addMissingInitMarkers = originalMissingInitMarkers + addInitRepository = originalInitRepository + }) // Drive the delivery confirm form via accessible (line-based) mode, answering // "no" to pull request creation. @@ -382,13 +396,15 @@ func TestAddInteractiveConfig_prepareAndConfirmAddInteractive_localWriteSkipsSec }, } - workflowFiles, _, secretName, secretValue, createPR, err := config.prepareAndConfirmAddInteractive() + workflowFiles, initFiles, secretName, secretValue, createPR, err := config.prepareAndConfirmAddInteractive() require.NoError(t, err) assert.False(t, createPR, "choosing local writes should report createPR=false") assert.Empty(t, secretName, "local writes must not resolve a secret to configure") assert.Empty(t, secretValue, "local writes must not resolve a secret value") assert.NotEmpty(t, workflowFiles, "workflow files should still be determined for local writes") + assert.Empty(t, initFiles, "local writes must not include repository initialization files") + assert.False(t, initializationRan, "local writes must not initialize repository support files") if _, statErr := os.Stat(ghLog); statErr == nil { logContent, readErr := os.ReadFile(ghLog) diff --git a/pkg/cli/add_interactive_secrets.go b/pkg/cli/add_interactive_secrets.go index 784e4bab67a..faa35d6813c 100644 --- a/pkg/cli/add_interactive_secrets.go +++ b/pkg/cli/add_interactive_secrets.go @@ -23,11 +23,14 @@ type organizationSecretsResponse struct { Secrets []organizationSecret `json:"secrets"` } +const repositorySecretSource = "repository secret" + // checkExistingSecrets fetches which secrets already exist in the repository or its organization func (c *AddInteractiveConfig) checkExistingSecrets() error { addInteractiveLog.Print("Checking existing repository secrets") c.existingSecrets = make(map[string]struct{}) + c.secretSources = make(map[string]string) // Use gh api to list repository secrets output, err := addInteractiveRunGH("Checking repository secrets...", "api", fmt.Sprintf("/repos/%s/actions/secrets", c.RepoOverride), "--jq", ".secrets[].name") @@ -37,24 +40,28 @@ func (c *AddInteractiveConfig) checkExistingSecrets() error { } else { for _, name := range parseSecretNames(output) { c.existingSecrets[name] = struct{}{} + c.secretSources[name] = repositorySecretSource addInteractiveLog.Printf("Found existing repository secret: %s", name) } } // Also check org-level secrets if the repo belongs to an organization if org, _, found := strings.Cut(c.RepoOverride, "/"); found && org != "" { - orgOutput, orgErr := addInteractiveRunGH("Checking organization secrets...", "api", fmt.Sprintf("/orgs/%s/actions/secrets", org)) + orgOutput, orgErr := addInteractiveRunGH("Checking organization secrets...", "api", fmt.Sprintf("/orgs/%s/actions/secrets", org), "--paginate", "--slurp") if orgErr != nil { addInteractiveLog.Printf("Could not fetch org secrets (this is expected for personal repos or if org access is restricted): %v", orgErr) } else { - var response organizationSecretsResponse - if err := json.Unmarshal(orgOutput, &response); err != nil { + responses, err := parseOrganizationSecretsResponses(orgOutput) + if err != nil { addInteractiveLog.Printf("Could not parse organization secrets: %v", err) } else { - for _, secret := range response.Secrets { - if c.organizationSecretAvailable(org, secret) { - c.existingSecrets[secret.Name] = struct{}{} - addInteractiveLog.Printf("Found available organization secret: %s", secret.Name) + for _, response := range responses { + for _, secret := range response.Secrets { + if c.organizationSecretAvailable(org, secret) { + c.existingSecrets[secret.Name] = struct{}{} + c.secretSources[secret.Name] = organizationSecretSource(secret.Visibility) + addInteractiveLog.Printf("Found available organization secret: %s", secret.Name) + } } } } @@ -68,17 +75,34 @@ func (c *AddInteractiveConfig) checkExistingSecrets() error { return nil } +func parseOrganizationSecretsResponses(output []byte) ([]organizationSecretsResponse, error) { + var responses []organizationSecretsResponse + if err := json.Unmarshal(output, &responses); err == nil { + return responses, nil + } + var response organizationSecretsResponse + if err := json.Unmarshal(output, &response); err != nil { + return nil, err + } + return []organizationSecretsResponse{response}, nil +} + +func organizationSecretSource(visibility string) string { + return "organization secret (" + visibility + " visibility)" +} + func (c *AddInteractiveConfig) organizationSecretAvailable(org string, secret organizationSecret) bool { switch secret.Visibility { case "all": return true case "private": - return !c.isPublicRepo + return c.repoVisibility == "private" case "selected": output, err := addInteractiveRunGH( "Checking organization secret repository access...", "api", fmt.Sprintf("/orgs/%s/actions/secrets/%s/repositories", org, secret.Name), + "--paginate", "--jq", ".repositories[].full_name", ) diff --git a/pkg/cli/add_interactive_secrets_test.go b/pkg/cli/add_interactive_secrets_test.go index 7d47b479cae..eec0b70ca15 100644 --- a/pkg/cli/add_interactive_secrets_test.go +++ b/pkg/cli/add_interactive_secrets_test.go @@ -303,15 +303,19 @@ func TestAddInteractiveConfig_checkExistingSecrets(t *testing.T) { case "/repos/test-owner/test-repo/actions/secrets": return []byte("REPOSITORY_SECRET\n"), nil case "/orgs/test-owner/actions/secrets": - return []byte(`{"secrets":[ + assert.Contains(t, args, "--paginate") + assert.Contains(t, args, "--slurp") + return []byte(`[{"secrets":[ {"name":"ALL_SECRET","visibility":"all"}, {"name":"PRIVATE_SECRET","visibility":"private"}, {"name":"SELECTED_SECRET","visibility":"selected"}, {"name":"INACCESSIBLE_SECRET","visibility":"selected"} - ]}`), nil + ]},{"secrets":[{"name":"PAGINATED_SECRET","visibility":"all"}]}]`), nil case "/orgs/test-owner/actions/secrets/SELECTED_SECRET/repositories": + assert.Contains(t, args, "--paginate") return []byte("test-owner/test-repo\n"), nil case "/orgs/test-owner/actions/secrets/INACCESSIBLE_SECRET/repositories": + assert.Contains(t, args, "--paginate") return []byte("test-owner/another-repo\n"), nil default: t.Fatalf("unexpected gh api arguments: %v", args) @@ -319,16 +323,19 @@ func TestAddInteractiveConfig_checkExistingSecrets(t *testing.T) { } } - config := &AddInteractiveConfig{RepoOverride: "test-owner/test-repo"} + config := &AddInteractiveConfig{RepoOverride: "test-owner/test-repo", repoVisibility: "private"} require.NoError(t, config.checkExistingSecrets()) assert.Contains(t, config.existingSecrets, "REPOSITORY_SECRET") assert.Contains(t, config.existingSecrets, "ALL_SECRET") assert.Contains(t, config.existingSecrets, "PRIVATE_SECRET") assert.Contains(t, config.existingSecrets, "SELECTED_SECRET") + assert.Contains(t, config.existingSecrets, "PAGINATED_SECRET") assert.NotContains(t, config.existingSecrets, "INACCESSIBLE_SECRET") + assert.Equal(t, repositorySecretSource, config.secretSources["REPOSITORY_SECRET"]) + assert.Equal(t, organizationSecretSource("selected"), config.secretSources["SELECTED_SECRET"]) - config.isPublicRepo = true + config.repoVisibility = "internal" assert.False(t, config.organizationSecretAvailable("test-owner", organizationSecret{ Name: "PRIVATE_SECRET", Visibility: "private", diff --git a/pkg/cli/add_workflow_compilation.go b/pkg/cli/add_workflow_compilation.go index cfab4812fb7..910db268368 100644 --- a/pkg/cli/add_workflow_compilation.go +++ b/pkg/cli/add_workflow_compilation.go @@ -80,10 +80,6 @@ func compileWorkflowWithTrackingAndActionRef(ctx context.Context, filePath strin // compileWorkflowWithTrackingAndRefresh compiles a workflow, tracks generated files, and optionally refreshes stop time. // This function ensures that the file tracker records all files created or modified during compilation. -func compileWorkflowWithTrackingAndRefresh(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride string, tracker *FileTracker, refreshStopTime bool) error { - return compileWorkflowWithTrackingAndRefreshAndActionRef(ctx, filePath, verbose, quiet, engineOverride, "", tracker, refreshStopTime) -} - func compileWorkflowWithTrackingAndRefreshAndActionRef(ctx context.Context, filePath string, verbose bool, quiet bool, engineOverride, actionRef string, tracker *FileTracker, refreshStopTime bool) error { addWorkflowCompilationLog.Printf("Compiling workflow with tracking: file=%s, refresh_stop_time=%v", filePath, refreshStopTime) diff --git a/pkg/cli/add_workflow_pr.go b/pkg/cli/add_workflow_pr.go index a9e3dff9e7e..540e1275fe1 100644 --- a/pkg/cli/add_workflow_pr.go +++ b/pkg/cli/add_workflow_pr.go @@ -6,6 +6,7 @@ import ( "math/rand" "net/url" "os" + "path/filepath" "regexp" "slices" "strings" @@ -18,6 +19,11 @@ import ( var addWorkflowPRLog = logger.New("cli:add_workflow_pr") +const ( + ghAwDocumentationURL = "https://github.github.com/gh-aw/" + ghAwRepositoryURL = "https://github.com/github/gh-aw" +) + // invalidBranchCharsPattern matches characters not allowed in git branch names var invalidBranchCharsPattern = regexp.MustCompile(`[^a-zA-Z0-9_-]+`) @@ -80,7 +86,12 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts // Create file tracker for rollback capability tracker := NewFileTracker() for _, initializedFile := range opts.initializedFiles { - tracker.TrackCreated(initializedFile) + if originalContent, modified := opts.initializedOriginalContents[initializedFile]; modified { + tracker.TrackModified(initializedFile) + tracker.OriginalContent[initializedFile] = originalContent + } else { + tracker.TrackCreated(initializedFile) + } } // Ensure we switch back to original branch on exit @@ -199,9 +210,9 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts func buildAddWorkflowPRBody(workflows []*ResolvedWorkflow, opts AddOptions) string { var body strings.Builder if opts.createdByAddWizard { - fmt.Fprintf(&body, "This pull request was created with [`gh aw add-wizard`](https://github.github.com/gh-aw/) from [GitHub Agentic Workflows](https://github.com/github/gh-aw), version `%s`.\n", markdownText(GetVersion())) + fmt.Fprintf(&body, "This pull request was created with [`gh aw add-wizard`](%s) from [GitHub Agentic Workflows](%s), version `%s`.\n", ghAwDocumentationURL, ghAwRepositoryURL, markdownText(GetVersion())) } else { - fmt.Fprintf(&body, "This pull request was created with [`gh aw add`](https://github.github.com/gh-aw/) from [GitHub Agentic Workflows](https://github.com/github/gh-aw), version `%s`.\n", markdownText(GetVersion())) + fmt.Fprintf(&body, "This pull request was created with [`gh aw add`](%s) from [GitHub Agentic Workflows](%s), version `%s`.\n", ghAwDocumentationURL, ghAwRepositoryURL, markdownText(GetVersion())) } body.WriteString("\n## Workflows\n") @@ -216,13 +227,19 @@ func buildAddWorkflowPRBody(workflows []*ResolvedWorkflow, opts AddOptions) stri body.WriteString("\n## Options selected\n\n") body.WriteString("- **Delivery:** pull request\n") - fmt.Fprintf(&body, "- **Engine:** `%s`\n", markdownText(opts.EngineOverride)) + if opts.EngineOverride != "" { + fmt.Fprintf(&body, "- **Engine:** `%s`\n", markdownText(opts.EngineOverride)) + } if opts.EngineOverride == "copilot" { auth := "`COPILOT_GITHUB_TOKEN` repository secret" if opts.AddCopilotRequestsPermission { auth = "organization billing via `permissions.copilot-requests: write`" } else if opts.addWizardSecretExists { - auth = "existing `COPILOT_GITHUB_TOKEN` repository or organization secret" + source := opts.addWizardSecretSource + if source == "" { + source = repositorySecretSource + } + auth = "existing `COPILOT_GITHUB_TOKEN` " + source } else if opts.addWizardSkipSecret { auth = "`COPILOT_GITHUB_TOKEN` setup skipped" } @@ -247,7 +264,7 @@ func buildAddWorkflowPRBody(workflows []*ResolvedWorkflow, opts AddOptions) stri body.WriteString("- **Custom appended instructions:** included\n") } if len(opts.initializedFiles) > 0 { - fmt.Fprintf(&body, "- **Repository initialization:** %s\n", joinCodeValues(opts.initializedFiles)) + fmt.Fprintf(&body, "- **Repository initialization:** %s\n", joinCodeValues(repositoryRelativePaths(opts.initializedFiles))) } body.WriteString("\n## Review criteria\n\n") @@ -270,6 +287,22 @@ func buildAddWorkflowPRBody(workflows []*ResolvedWorkflow, opts AddOptions) stri return body.String() } +func repositoryRelativePaths(paths []string) []string { + gitRoot, err := addFindGitRoot() + displayPaths := make([]string, 0, len(paths)) + for _, path := range paths { + if filepath.IsAbs(path) { + if relative, relErr := filepath.Rel(gitRoot, path); err == nil && relErr == nil && relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + path = relative + } else { + path = filepath.Base(path) + } + } + displayPaths = append(displayPaths, filepath.ToSlash(path)) + } + return displayPaths +} + func workflowSourceMarkdown(resolved *ResolvedWorkflow) string { label := markdownText(resolved.Spec.String()) if resolved.Spec.RawURL != "" { diff --git a/pkg/cli/add_workflow_pr_test.go b/pkg/cli/add_workflow_pr_test.go index aba12594f0d..c48d8a97ea7 100644 --- a/pkg/cli/add_workflow_pr_test.go +++ b/pkg/cli/add_workflow_pr_test.go @@ -3,6 +3,7 @@ package cli import ( + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -221,6 +222,7 @@ func TestBuildAddWorkflowPRBody(t *testing.T) { EngineOverride: "copilot", createdByAddWizard: true, addWizardSecretExists: true, + addWizardSecretSource: organizationSecretSource("selected"), initializedFiles: []string{".gitattributes", ".github/aw/actions-lock.json"}, addWizardDisableGitHubAppInference: true, } @@ -233,7 +235,7 @@ func TestBuildAddWorkflowPRBody(t *testing.T) { assert.Contains(t, body, "Helps maintain the repository") assert.Contains(t, body, "`schedule` (`weekly`), `workflow_dispatch`") assert.Contains(t, body, "**Delivery:** pull request") - assert.Contains(t, body, "existing `COPILOT_GITHUB_TOKEN` repository or organization secret") + assert.Contains(t, body, "existing `COPILOT_GITHUB_TOKEN` organization secret (selected visibility)") assert.Contains(t, body, "**GitHub App permission and event inference:** disabled") assert.Contains(t, body, "`.gitattributes`, `.github/aw/actions-lock.json`") assert.Contains(t, body, "## Review criteria") @@ -257,6 +259,26 @@ func TestBuildAddWorkflowPRBodyUsesLocalSourceAndSecretNextStep(t *testing.T) { assert.Contains(t, body, "recompile it with `gh aw compile`") } +func TestBuildAddWorkflowPRBodyOmitsEmptyEngineAndLocalPaths(t *testing.T) { + originalFindGitRoot := addFindGitRoot + repoDir := t.TempDir() + addFindGitRoot = func() (string, error) { return repoDir, nil } + t.Cleanup(func() { addFindGitRoot = originalFindGitRoot }) + + workflow := &ResolvedWorkflow{ + Spec: &WorkflowSpec{WorkflowPath: "./review.md", WorkflowName: "review"}, + SourceInfo: &FetchedWorkflow{IsLocal: true}, + } + localPath := filepath.Join(repoDir, ".github", "skills", "agentic-workflows", "SKILL.md") + body := buildAddWorkflowPRBody([]*ResolvedWorkflow{workflow}, AddOptions{ + initializedFiles: []string{localPath}, + }) + + assert.NotContains(t, body, "**Engine:**") + assert.NotContains(t, body, repoDir) + assert.Contains(t, body, "`.github/skills/agentic-workflows/SKILL.md`") +} + func TestBuildAddWorkflowPRBodyPreservesDescriptionMarkdown(t *testing.T) { content := `--- description: | diff --git a/pkg/cli/preconditions.go b/pkg/cli/preconditions.go index bd07dde9dfe..2760d319bd5 100644 --- a/pkg/cli/preconditions.go +++ b/pkg/cli/preconditions.go @@ -192,8 +192,7 @@ func checkUserPermissionsShared(repoSlug string, verbose bool) (bool, error) { return hasAccess, nil } -// checkRepoVisibilityShared checks if the repository is public or private -func checkRepoVisibilityShared(repoSlug string) bool { +func getRepoVisibilityShared(repoSlug string) string { preconditionsLog.Print("Checking repository visibility") // Use gh api to check repository visibility @@ -201,11 +200,10 @@ func checkRepoVisibilityShared(repoSlug string) bool { if err != nil { preconditionsLog.Printf("Could not check repository visibility: %v", err) // Default to public if we can't determine - return true + return "public" } visibility := strings.TrimSpace(string(output)) - isPublic := visibility == "public" - preconditionsLog.Printf("Repository visibility: %s (isPublic=%v)", visibility, isPublic) - return isPublic + preconditionsLog.Printf("Repository visibility: %s", visibility) + return visibility } From f84f90a1e9707b187d7efaacc3ec1dbda75e9cd3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Aug 2026 20:07:20 +0000 Subject: [PATCH 19/20] Clarify PR path fallback logging Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/add_workflow_pr.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/cli/add_workflow_pr.go b/pkg/cli/add_workflow_pr.go index 540e1275fe1..bad4f7d8fdc 100644 --- a/pkg/cli/add_workflow_pr.go +++ b/pkg/cli/add_workflow_pr.go @@ -289,10 +289,14 @@ func buildAddWorkflowPRBody(workflows []*ResolvedWorkflow, opts AddOptions) stri func repositoryRelativePaths(paths []string) []string { gitRoot, err := addFindGitRoot() + if err != nil { + addWorkflowPRLog.Printf("Could not determine repository root for PR path rendering: %v", err) + } + canRelativize := err == nil displayPaths := make([]string, 0, len(paths)) for _, path := range paths { if filepath.IsAbs(path) { - if relative, relErr := filepath.Rel(gitRoot, path); err == nil && relErr == nil && relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + if relative, relErr := filepath.Rel(gitRoot, path); canRelativize && relErr == nil && relative != ".." && !strings.HasPrefix(relative, ".."+string(filepath.Separator)) { path = relative } else { path = filepath.Base(path) From def00ceeb2ee429d8863c583610dabae548b7951 Mon Sep 17 00:00:00 2001 From: Don Syme Date: Mon, 24 Aug 2026 22:17:30 +0200 Subject: [PATCH 20/20] code review --- pkg/cli/add_command.go | 48 ++++++++++++------- pkg/cli/add_command_test.go | 9 ++-- pkg/cli/add_init.go | 45 ++++++++++++++++-- pkg/cli/add_interactive_auth.go | 3 +- pkg/cli/add_interactive_engine.go | 10 ++-- pkg/cli/add_interactive_engine_test.go | 13 +++-- pkg/cli/add_interactive_git.go | 42 ++++++++++++----- pkg/cli/add_interactive_orchestrator.go | 23 ++++----- pkg/cli/add_interactive_secrets.go | 63 ++++++++++++++++++------- pkg/cli/add_interactive_secrets_test.go | 16 +++---- pkg/cli/add_workflow_pr.go | 41 ++++++++++------ pkg/cli/add_workflow_pr_test.go | 31 +++++++++--- pkg/cli/preconditions.go | 12 +++-- 13 files changed, 249 insertions(+), 107 deletions(-) diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index 698d60e096e..661ed4ea163 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -87,19 +87,31 @@ type AddOptions struct { // the workflow frontmatter, enabling GitHub Actions token auth for Copilot. // Set by the add-wizard when the user selects org-billing auth instead of a PAT. AddCopilotRequestsPermission bool - // initializedFiles contains files created by add-wizard after its clean-tree check. - initializedFiles []string - // workingTreePrevalidated indicates add-wizard already verified that staged - // changes and changes overlapping planned files are absent. - workingTreePrevalidated bool - // showInteractiveProgress enables high-level progress indicators for the - // otherwise quiet add-wizard write, compile, commit, and push phases. - showInteractiveProgress bool - // createdByAddWizard records that the interactive wizard selected these options. - createdByAddWizard bool - addWizardSkipSecret bool - addWizardSecretExists bool - addWizardDisableGitHubAppInference bool + addWizard *addWizardOptions +} + +type addWizardOptions struct { + initializedFiles []addInitializedFile + workingTreePrevalidated bool + showInteractiveProgress bool + secretSource secretSource + skipSecret bool + disableGitHubAppPermissionInference bool +} + +func (opts AddOptions) wizardInitializedPaths() []string { + if opts.addWizard == nil { + return nil + } + paths := make([]string, 0, len(opts.addWizard.initializedFiles)) + for _, file := range opts.addWizard.initializedFiles { + paths = append(paths, file.path) + } + return paths +} + +func (opts AddOptions) showInteractiveProgress() bool { + return opts.addWizard != nil && opts.addWizard.showInteractiveProgress } // AddWorkflowsResult contains the result of adding workflows @@ -288,8 +300,8 @@ func AddResolvedWorkflows(ctx context.Context, workflowStrings []string, resolve } // Check no other changes are present - if !opts.workingTreePrevalidated { - if err := checkCleanWorkingDirectoryIgnoring(opts.Verbose, opts.initializedFiles); err != nil { + if opts.addWizard == nil || !opts.addWizard.workingTreePrevalidated { + if err := checkCleanWorkingDirectoryIgnoring(opts.Verbose, opts.wizardInitializedPaths()); err != nil { return nil, fmt.Errorf("working directory is not clean: %w", err) } } @@ -462,7 +474,7 @@ func addWorkflowWithTracking(ctx context.Context, resolved *ResolvedWorkflow, tr destFile := filepath.Join(githubWorkflowsDir, workflowName+".md") fileExists := fileutil.FileExists(destFile) - if fileExists && !opts.showInteractiveProgress { + if fileExists && !opts.showInteractiveProgress() { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Overwriting existing file: "+destFile)) } stopProgress := startAddInteractiveProgress(opts, "Preparing workflow files...") @@ -480,7 +492,7 @@ func addWorkflowWithTracking(ctx context.Context, resolved *ResolvedWorkflow, tr } func startAddInteractiveProgress(opts AddOptions, message string) func() { - if !opts.showInteractiveProgress { + if !opts.showInteractiveProgress() { return func() {} } spinner := console.NewSpinner(message) @@ -862,7 +874,7 @@ func addActionWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTrac } return fmt.Errorf("action workflow '%s' already exists in %s. Use --force to overwrite", workflowName+".yml", githubWorkflowsDir) } - if !opts.showInteractiveProgress { + if !opts.showInteractiveProgress() { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Overwriting existing file: "+destFile)) } } diff --git a/pkg/cli/add_command_test.go b/pkg/cli/add_command_test.go index c583f297158..8190db8a3da 100644 --- a/pkg/cli/add_command_test.go +++ b/pkg/cli/add_command_test.go @@ -574,7 +574,7 @@ func TestConfirmAndInitializeAddRepository(t *testing.T) { addFindGitRoot = func() (string, error) { return repoDir, nil } t.Run("already initialized skips confirmation", func(t *testing.T) { - addMissingAuthoringSupportFiles = func(string, string, bool) ([]string, error) { return nil, nil } + addMissingInitMarkers = func(string, string) ([]string, error) { return nil, nil } addConfirmAuthoringSupport = func(context.Context) (bool, error) { t.Fatal("confirmation should not be shown when all support files exist") return false, nil @@ -590,7 +590,7 @@ func TestConfirmAndInitializeAddRepository(t *testing.T) { }) t.Run("declining creates no support files", func(t *testing.T) { - addMissingAuthoringSupportFiles = func(string, string, bool) ([]string, error) { + addMissingInitMarkers = func(string, string) ([]string, error) { return []string{bootstrapAgenticSkillPath}, nil } addConfirmAuthoringSupport = func(context.Context) (bool, error) { return false, nil } @@ -606,7 +606,6 @@ func TestConfirmAndInitializeAddRepository(t *testing.T) { t.Run("accepting quietly initializes support files", func(t *testing.T) { marker := ".vscode/settings.json" - addMissingAuthoringSupportFiles = func(string, string, bool) ([]string, error) { return []string{marker}, nil } addMissingInitMarkers = func(string, string) ([]string, error) { return []string{marker}, nil } addConfirmAuthoringSupport = func(context.Context) (bool, error) { return true, nil } addInitRepository = func(opts InitOptions) error { @@ -619,7 +618,9 @@ func TestConfirmAndInitializeAddRepository(t *testing.T) { files, err := confirmAndInitializeAddRepository(context.Background(), "copilot", false, false) require.NoError(t, err) - require.Equal(t, []string{filepath.Join(repoDir, filepath.FromSlash(marker))}, files) + require.Equal(t, []addInitializedFile{{ + path: filepath.Join(repoDir, filepath.FromSlash(marker)), displayPath: marker, + }}, files) }) } diff --git a/pkg/cli/add_init.go b/pkg/cli/add_init.go index 02bdf0836e5..48677cd2dc3 100644 --- a/pkg/cli/add_init.go +++ b/pkg/cli/add_init.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "charm.land/huh/v2" "github.com/github/gh-aw/pkg/console" @@ -55,6 +56,13 @@ type addRepositoryInitializationPlan struct { files []string } +type addInitializedFile struct { + path string + displayPath string + wasExisting bool + originalContent []byte +} + func confirmAddRepositoryInitialization(ctx context.Context, engineOverride string, noGitattributes bool) (addRepositoryInitializationPlan, error) { gitRoot, err := addFindGitRoot() if err != nil { @@ -67,7 +75,10 @@ func confirmAddRepositoryInitialization(ctx context.Context, engineOverride stri var missingMarkers []string if err := withWorkingDir(gitRoot, func() error { var inspectErr error - missingMarkers, inspectErr = addMissingAuthoringSupportFiles(".", engineOverride, noGitattributes) + missingMarkers, inspectErr = addMissingInitMarkers(".", engineOverride) + if noGitattributes { + missingMarkers = slices.DeleteFunc(missingMarkers, func(path string) bool { return path == ".gitattributes" }) + } return inspectErr }); err != nil { return addRepositoryInitializationPlan{}, fmt.Errorf("failed to inspect repository initialization state: %w", err) @@ -87,14 +98,14 @@ func confirmAddRepositoryInitialization(ctx context.Context, engineOverride stri return addRepositoryInitializationPlan{enabled: true, files: missingMarkers}, nil } -func applyAddRepositoryInitialization(plan addRepositoryInitializationPlan, engineOverride string, verbose bool, noGitattributes bool) ([]string, error) { +func applyAddRepositoryInitialization(plan addRepositoryInitializationPlan, engineOverride string, verbose bool, noGitattributes bool) ([]addInitializedFile, error) { if !plan.enabled { return nil, nil } - return ensureAddRepositoryInitializedWithDetails(engineOverride, verbose, noGitattributes) + return ensureAddRepositoryInitializedFromPlan(plan.files, engineOverride, verbose, noGitattributes) } -func confirmAndInitializeAddRepository(ctx context.Context, engineOverride string, verbose bool, noGitattributes bool) ([]string, error) { +func confirmAndInitializeAddRepository(ctx context.Context, engineOverride string, verbose bool, noGitattributes bool) ([]addInitializedFile, error) { plan, err := confirmAddRepositoryInitialization(ctx, engineOverride, noGitattributes) if err != nil { return nil, err @@ -102,6 +113,32 @@ func confirmAndInitializeAddRepository(ctx context.Context, engineOverride strin return applyAddRepositoryInitialization(plan, engineOverride, verbose, noGitattributes) } +func ensureAddRepositoryInitializedFromPlan(markers []string, engineOverride string, verbose bool, noGitattributes bool) ([]addInitializedFile, error) { + gitRoot, err := addFindGitRoot() + if err != nil { + return nil, fmt.Errorf("failed to determine repository root for automatic initialization: %w", err) + } + + files := make([]addInitializedFile, 0, len(markers)) + err = withWorkingDir(gitRoot, func() error { + for _, marker := range markers { + originalContent, readErr := os.ReadFile(marker) + files = append(files, addInitializedFile{ + path: filepath.Join(gitRoot, filepath.FromSlash(marker)), displayPath: filepath.ToSlash(marker), + wasExisting: readErr == nil, originalContent: originalContent, + }) + } + if err := addInitRepository(InitOptions{ + Verbose: verbose, Quiet: true, Engine: engineOverride, NoGitattributes: noGitattributes, + Skill: true, Agent: true, MCP: true, + }); err != nil { + return fmt.Errorf("failed to initialize repository for agentic workflows: %w", err) + } + return nil + }) + return files, err +} + func ensureAddRepositoryInitialized(engineOverride string, verbose bool, noGitattributes bool) error { _, err := ensureAddRepositoryInitializedWithDetails(engineOverride, verbose, noGitattributes) return err diff --git a/pkg/cli/add_interactive_auth.go b/pkg/cli/add_interactive_auth.go index 015eeeb8691..405dc7179b6 100644 --- a/pkg/cli/add_interactive_auth.go +++ b/pkg/cli/add_interactive_auth.go @@ -67,8 +67,7 @@ func (c *AddInteractiveConfig) checkGitRepository() error { fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Target repository: "+repoSlug)) addInteractiveLog.Printf("Target repository: %s", repoSlug) - // Check if repository is public or private - c.isPublicRepo = checkRepoVisibilityShared(c.RepoOverride) + c.repositoryVisibility = getRepoVisibilityShared(c.RepoOverride) return nil } diff --git a/pkg/cli/add_interactive_engine.go b/pkg/cli/add_interactive_engine.go index 98205e90901..43b13941c7c 100644 --- a/pkg/cli/add_interactive_engine.go +++ b/pkg/cli/add_interactive_engine.go @@ -283,7 +283,7 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { var authMethod string selectField := huh.NewSelect[string](). Title("How would you like Copilot workflows to authenticate?"). - Description(copilotAuthMethodDescription(probe)). + Description(copilotAuthMethodDescription(probe, c.secretSources[constants.CopilotGitHubToken])). Options(options...). Value(&authMethod) @@ -306,12 +306,16 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { return nil } -func copilotAuthMethodDescription(probe orgCopilotBillingProbeResult) string { +func copilotAuthMethodDescription(probe orgCopilotBillingProbeResult, source secretSource) string { copilotRequestsDescription := "• copilot-requests: Use the org's Copilot billing seat; no PAT required." if probe.InfoNote != "" { copilotRequestsDescription += "\n (NOTE: " + probe.InfoNote + "\n Check with your org admin if you want to use this option.)" } - return "• PAT: Use the existing COPILOT_GITHUB_TOKEN repository secret.\n" + copilotRequestsDescription + patDescription := "• PAT: Create a COPILOT_GITHUB_TOKEN repository secret." + if source != "" { + patDescription = "• PAT: Reuse the existing COPILOT_GITHUB_TOKEN " + string(source) + " secret." + } + return patDescription + "\n" + copilotRequestsDescription } // applyCopilotAuthMethodChoice records the user's Copilot auth method selection and prints diff --git a/pkg/cli/add_interactive_engine_test.go b/pkg/cli/add_interactive_engine_test.go index 550e396b881..b46f25506a3 100644 --- a/pkg/cli/add_interactive_engine_test.go +++ b/pkg/cli/add_interactive_engine_test.go @@ -60,13 +60,18 @@ func TestCopilotAuthMethodDescription(t *testing.T) { t.Parallel() t.Run("bullets both authentication methods", func(t *testing.T) { - description := copilotAuthMethodDescription(orgCopilotBillingProbeResult{}) - assert.Equal(t, "• PAT: Use the existing COPILOT_GITHUB_TOKEN repository secret.\n• copilot-requests: Use the org's Copilot billing seat; no PAT required.", description) + description := copilotAuthMethodDescription(orgCopilotBillingProbeResult{}, "") + assert.Equal(t, "• PAT: Create a COPILOT_GITHUB_TOKEN repository secret.\n• copilot-requests: Use the org's Copilot billing seat; no PAT required.", description) + }) + + t.Run("describes an existing organization secret precisely", func(t *testing.T) { + description := copilotAuthMethodDescription(orgCopilotBillingProbeResult{}, secretSourceOrganizationSelected) + assert.Contains(t, description, "Reuse the existing COPILOT_GITHUB_TOKEN organization (selected repository) secret.") }) t.Run("includes inconclusive billing note in copilot-requests bullet", func(t *testing.T) { - description := copilotAuthMethodDescription(orgCopilotBillingProbeResult{InfoNote: copilotBillingInconclusiveNote}) - assert.Equal(t, "• PAT: Use the existing COPILOT_GITHUB_TOKEN repository secret.\n• copilot-requests: Use the org's Copilot billing seat; no PAT required.\n (NOTE: Could not confirm org Copilot CLI billing.\n Check with your org admin if you want to use this option.)", description) + description := copilotAuthMethodDescription(orgCopilotBillingProbeResult{InfoNote: copilotBillingInconclusiveNote}, secretSourceRepository) + assert.Equal(t, "• PAT: Reuse the existing COPILOT_GITHUB_TOKEN repository secret.\n• copilot-requests: Use the org's Copilot billing seat; no PAT required.\n (NOTE: Could not confirm org Copilot CLI billing.\n Check with your org admin if you want to use this option.)", description) }) } diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index 99bbe34c1c1..7a22a6631ea 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -35,7 +35,7 @@ const ( ) // createWorkflowChangesAndConfigureSecret writes the workflows, optionally creates and merges a PR, and adds the secret. -func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx context.Context, workflowFiles, initFiles []string, secretName, secretValue string, createPR bool) error { +func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx context.Context, workflowFiles []string, initFiles []addInitializedFile, secretName, secretValue string, createPR bool) error { addInteractiveLog.Print("Applying changes") // Add the workflow using the existing implementation. @@ -58,14 +58,15 @@ func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx conte RepoSlug: c.RepoOverride, AddCopilotRequestsPermission: c.UseCopilotRequests, GhAwRef: c.GhAwRef, - initializedFiles: initFiles, - workingTreePrevalidated: createPR, - showInteractiveProgress: true, - createdByAddWizard: true, - addWizardSkipSecret: c.SkipSecret, - addWizardDisableGitHubAppInference: c.DisableGitHubAppPermissionInference, - } - _, opts.addWizardSecretExists = c.existingSecrets["COPILOT_GITHUB_TOKEN"] + addWizard: &addWizardOptions{ + initializedFiles: initFiles, + workingTreePrevalidated: createPR, + showInteractiveProgress: true, + skipSecret: c.SkipSecret, + disableGitHubAppPermissionInference: c.DisableGitHubAppPermissionInference, + }, + } + opts.addWizard.secretSource = c.secretSources["COPILOT_GITHUB_TOKEN"] result, err := AddResolvedWorkflows(ctx, c.WorkflowSpecs, c.resolvedWorkflows, opts) if err != nil { return fmt.Errorf("failed to add workflow: %w", err) @@ -330,13 +331,24 @@ const ( // but requires staged changes and edits to files the wizard will write to be cleaned. func (c *AddInteractiveConfig) checkCleanWorkingDirectoryForPR(workflowFiles, initFiles []string) error { addInteractiveLog.Print("Checking working tree changes before PR creation") - plannedPaths, err := c.plannedAddPaths(workflowFiles, initFiles) + gitRoot, err := addFindGitRoot() + if err != nil { + return fmt.Errorf("failed to determine repository root for PR preflight: %w", err) + } + plannedPaths, err := c.plannedAddPathsAtRoot(gitRoot, workflowFiles, initFiles) if err != nil { return err } for { - blockers, inspectErr := inspectAddWorkingTree(plannedPaths) + if c.Ctx != nil { + select { + case <-c.Ctx.Done(): + return c.Ctx.Err() + default: + } + } + blockers, inspectErr := inspectAddWorkingTreeAtRoot(gitRoot, plannedPaths) if inspectErr != nil { return inspectErr } @@ -366,6 +378,10 @@ func (c *AddInteractiveConfig) plannedAddPaths(workflowFiles, initFiles []string if err != nil { return nil, fmt.Errorf("failed to determine repository root for PR preflight: %w", err) } + return c.plannedAddPathsAtRoot(gitRoot, workflowFiles, initFiles) +} + +func (c *AddInteractiveConfig) plannedAddPathsAtRoot(gitRoot string, workflowFiles, initFiles []string) ([]string, error) { workflowDir := c.WorkflowDir if workflowDir == "" { workflowDir = getWorkflowsDir() @@ -393,6 +409,10 @@ func inspectAddWorkingTree(plannedPaths []string) (addWorkingTreeBlockers, error if err != nil { return addWorkingTreeBlockers{}, fmt.Errorf("failed to determine repository root for PR preflight: %w", err) } + return inspectAddWorkingTreeAtRoot(gitRoot, plannedPaths) +} + +func inspectAddWorkingTreeAtRoot(gitRoot string, plannedPaths []string) (addWorkingTreeBlockers, error) { cmd := exec.Command("git", "status", "--porcelain=v1", "-z", "--untracked-files=all") cmd.Dir = gitRoot output, err := cmd.Output() diff --git a/pkg/cli/add_interactive_orchestrator.go b/pkg/cli/add_interactive_orchestrator.go index d772e8e0eff..7f388fc4644 100644 --- a/pkg/cli/add_interactive_orchestrator.go +++ b/pkg/cli/add_interactive_orchestrator.go @@ -50,9 +50,8 @@ type AddInteractiveConfig struct { // Populated by selectCopilotAuthMethod() via probeCopilotBillingForOrg(). copilotCLIBillingStatus string - // isPublicRepo tracks whether the target repository is public - // This is populated by checkGitRepository() when determining the repo - isPublicRepo bool + // repositoryVisibility is populated before organization secrets are inspected. + repositoryVisibility string // hasWriteAccess tracks whether the user has write access to the target repository. // When false, secrets configuration is skipped since users cannot configure repository secrets. @@ -61,6 +60,7 @@ type AddInteractiveConfig struct { // existingSecrets tracks which secrets already exist in the repository // This is populated by checkExistingSecrets() before engine selection existingSecrets map[string]struct{} + secretSources map[string]secretSource // addResult holds the result from AddWorkflows, including HasWorkflowDispatch addResult *AddWorkflowsResult @@ -201,7 +201,7 @@ func (c *AddInteractiveConfig) sourceWorkflowMessage() string { return "Source workflow: " + strings.Join(c.WorkflowSpecs, ", ") } -func (c *AddInteractiveConfig) prepareAndConfirmAddInteractive() (workflowFiles, initFiles []string, secretName, secretValue string, createPR bool, err error) { +func (c *AddInteractiveConfig) prepareAndConfirmAddInteractive() (workflowFiles []string, initFiles []addInitializedFile, secretName, secretValue string, createPR bool, err error) { // selectAIEngineAndKey only selects the engine and, for Copilot, the auth method // (org billing vs. PAT). It does not prompt for or upload any secret value, since // that has remote repository side effects and must wait until the user has @@ -223,26 +223,27 @@ func (c *AddInteractiveConfig) prepareAndConfirmAddInteractive() (workflowFiles, if err != nil { return nil, nil, "", "", false, err } - initFiles = initializationPlan.files - createPR, err = c.confirmChanges(workflowFiles, initFiles) + createPR, err = c.confirmChanges(workflowFiles, initializationPlan.files) if err != nil { return nil, nil, "", "", false, err } if createPR { - if err := c.checkCleanWorkingDirectoryForPR(workflowFiles, initFiles); err != nil { + plannedInitFiles := make([]string, 0, len(initializationPlan.files)) + plannedInitFiles = append(plannedInitFiles, initializationPlan.files...) + if err := c.checkCleanWorkingDirectoryForPR(workflowFiles, plannedInitFiles); err != nil { return nil, nil, "", "", false, err } } + if !createPR { + return workflowFiles, nil, "", "", false, nil + } + initFiles, err = applyAddRepositoryInitialization(initializationPlan, c.EngineOverride, c.Verbose, c.NoGitattributes) if err != nil { return nil, nil, "", "", false, err } - if !createPR { - return workflowFiles, initFiles, "", "", false, nil - } - // Secret collection and upload only happen once the user has committed to the // PR path and the clean-tree check has succeeded. if err := c.configureEngineAPISecret(c.EngineOverride); err != nil { diff --git a/pkg/cli/add_interactive_secrets.go b/pkg/cli/add_interactive_secrets.go index 784e4bab67a..56e4faca718 100644 --- a/pkg/cli/add_interactive_secrets.go +++ b/pkg/cli/add_interactive_secrets.go @@ -2,7 +2,6 @@ package cli import ( "bytes" - "encoding/json" "fmt" "os" "strings" @@ -14,20 +13,26 @@ import ( var addInteractiveRunGH = workflow.RunGH +type secretSource string + +const ( + secretSourceRepository secretSource = "repository" + secretSourceOrganizationAll secretSource = "organization (all repositories)" + secretSourceOrganizationPrivate secretSource = "organization (private repositories)" + secretSourceOrganizationSelected secretSource = "organization (selected repository)" +) + type organizationSecret struct { Name string `json:"name"` Visibility string `json:"visibility"` } -type organizationSecretsResponse struct { - Secrets []organizationSecret `json:"secrets"` -} - // checkExistingSecrets fetches which secrets already exist in the repository or its organization func (c *AddInteractiveConfig) checkExistingSecrets() error { addInteractiveLog.Print("Checking existing repository secrets") c.existingSecrets = make(map[string]struct{}) + c.secretSources = make(map[string]secretSource) // Use gh api to list repository secrets output, err := addInteractiveRunGH("Checking repository secrets...", "api", fmt.Sprintf("/repos/%s/actions/secrets", c.RepoOverride), "--jq", ".secrets[].name") @@ -37,25 +42,26 @@ func (c *AddInteractiveConfig) checkExistingSecrets() error { } else { for _, name := range parseSecretNames(output) { c.existingSecrets[name] = struct{}{} + c.secretSources[name] = secretSourceRepository addInteractiveLog.Printf("Found existing repository secret: %s", name) } } // Also check org-level secrets if the repo belongs to an organization if org, _, found := strings.Cut(c.RepoOverride, "/"); found && org != "" { - orgOutput, orgErr := addInteractiveRunGH("Checking organization secrets...", "api", fmt.Sprintf("/orgs/%s/actions/secrets", org)) + orgOutput, orgErr := addInteractiveRunGH( + "Checking organization secrets...", + "api", fmt.Sprintf("/orgs/%s/actions/secrets", org), + "--paginate", "--jq", ".secrets[] | [.name, .visibility] | @tsv", + ) if orgErr != nil { addInteractiveLog.Printf("Could not fetch org secrets (this is expected for personal repos or if org access is restricted): %v", orgErr) } else { - var response organizationSecretsResponse - if err := json.Unmarshal(orgOutput, &response); err != nil { - addInteractiveLog.Printf("Could not parse organization secrets: %v", err) - } else { - for _, secret := range response.Secrets { - if c.organizationSecretAvailable(org, secret) { - c.existingSecrets[secret.Name] = struct{}{} - addInteractiveLog.Printf("Found available organization secret: %s", secret.Name) - } + for _, secret := range parseOrganizationSecrets(orgOutput) { + if c.organizationSecretAvailable(org, secret) { + c.existingSecrets[secret.Name] = struct{}{} + c.secretSources[secret.Name] = organizationSecretSource(secret.Visibility) + addInteractiveLog.Printf("Found available organization secret: %s", secret.Name) } } } @@ -73,12 +79,13 @@ func (c *AddInteractiveConfig) organizationSecretAvailable(org string, secret or case "all": return true case "private": - return !c.isPublicRepo + return c.repositoryVisibility == "private" case "selected": output, err := addInteractiveRunGH( "Checking organization secret repository access...", "api", fmt.Sprintf("/orgs/%s/actions/secrets/%s/repositories", org, secret.Name), + "--paginate", "--jq", ".repositories[].full_name", ) @@ -95,6 +102,30 @@ func (c *AddInteractiveConfig) organizationSecretAvailable(org string, secret or } } +func parseOrganizationSecrets(output []byte) []organizationSecret { + var secrets []organizationSecret + for _, line := range parseSecretNames(output) { + name, visibility, found := strings.Cut(line, "\t") + if found && name != "" && visibility != "" { + secrets = append(secrets, organizationSecret{Name: name, Visibility: visibility}) + } + } + return secrets +} + +func organizationSecretSource(visibility string) secretSource { + switch visibility { + case "all": + return secretSourceOrganizationAll + case "private": + return secretSourceOrganizationPrivate + case "selected": + return secretSourceOrganizationSelected + default: + return "" + } +} + // addRepositorySecret adds a secret to the repository func (c *AddInteractiveConfig) addRepositorySecret(name, value string) error { output, err := workflow.RunGHInputContext(c.Ctx, "Adding repository secret...", bytes.NewBufferString(value), "secret", "set", name, "--repo", c.RepoOverride) diff --git a/pkg/cli/add_interactive_secrets_test.go b/pkg/cli/add_interactive_secrets_test.go index 7d47b479cae..a0dc95fdab3 100644 --- a/pkg/cli/add_interactive_secrets_test.go +++ b/pkg/cli/add_interactive_secrets_test.go @@ -303,15 +303,13 @@ func TestAddInteractiveConfig_checkExistingSecrets(t *testing.T) { case "/repos/test-owner/test-repo/actions/secrets": return []byte("REPOSITORY_SECRET\n"), nil case "/orgs/test-owner/actions/secrets": - return []byte(`{"secrets":[ - {"name":"ALL_SECRET","visibility":"all"}, - {"name":"PRIVATE_SECRET","visibility":"private"}, - {"name":"SELECTED_SECRET","visibility":"selected"}, - {"name":"INACCESSIBLE_SECRET","visibility":"selected"} - ]}`), nil + assert.Contains(t, args, "--paginate") + return []byte("ALL_SECRET\tall\nPRIVATE_SECRET\tprivate\nSELECTED_SECRET\tselected\nINACCESSIBLE_SECRET\tselected\n"), nil case "/orgs/test-owner/actions/secrets/SELECTED_SECRET/repositories": + assert.Contains(t, args, "--paginate") return []byte("test-owner/test-repo\n"), nil case "/orgs/test-owner/actions/secrets/INACCESSIBLE_SECRET/repositories": + assert.Contains(t, args, "--paginate") return []byte("test-owner/another-repo\n"), nil default: t.Fatalf("unexpected gh api arguments: %v", args) @@ -319,7 +317,7 @@ func TestAddInteractiveConfig_checkExistingSecrets(t *testing.T) { } } - config := &AddInteractiveConfig{RepoOverride: "test-owner/test-repo"} + config := &AddInteractiveConfig{RepoOverride: "test-owner/test-repo", repositoryVisibility: "private"} require.NoError(t, config.checkExistingSecrets()) assert.Contains(t, config.existingSecrets, "REPOSITORY_SECRET") @@ -327,8 +325,10 @@ func TestAddInteractiveConfig_checkExistingSecrets(t *testing.T) { assert.Contains(t, config.existingSecrets, "PRIVATE_SECRET") assert.Contains(t, config.existingSecrets, "SELECTED_SECRET") assert.NotContains(t, config.existingSecrets, "INACCESSIBLE_SECRET") + assert.Equal(t, secretSourceRepository, config.secretSources["REPOSITORY_SECRET"]) + assert.Equal(t, secretSourceOrganizationSelected, config.secretSources["SELECTED_SECRET"]) - config.isPublicRepo = true + config.repositoryVisibility = "internal" assert.False(t, config.organizationSecretAvailable("test-owner", organizationSecret{ Name: "PRIVATE_SECRET", Visibility: "private", diff --git a/pkg/cli/add_workflow_pr.go b/pkg/cli/add_workflow_pr.go index a9e3dff9e7e..ae3a5986ea2 100644 --- a/pkg/cli/add_workflow_pr.go +++ b/pkg/cli/add_workflow_pr.go @@ -79,8 +79,15 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts // Create file tracker for rollback capability tracker := NewFileTracker() - for _, initializedFile := range opts.initializedFiles { - tracker.TrackCreated(initializedFile) + if opts.addWizard != nil { + for _, initializedFile := range opts.addWizard.initializedFiles { + if initializedFile.wasExisting { + tracker.OriginalContent[initializedFile.path] = initializedFile.originalContent + tracker.TrackModified(initializedFile.path) + } else { + tracker.TrackCreated(initializedFile.path) + } + } } // Ensure we switch back to original branch on exit @@ -99,7 +106,7 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts } prepareSpinner := console.NewSpinner("Preparing pull request...") - if opts.showInteractiveProgress { + if opts.showInteractiveProgress() { prepareSpinner.Start() } defer prepareSpinner.Stop() @@ -156,7 +163,7 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts // Push branch addWorkflowPRLog.Printf("Pushing branch %s to remote", branchName) - if opts.showInteractiveProgress { + if opts.showInteractiveProgress() { prepareSpinner.UpdateMessage("Pushing pull request branch...") } if err := pushBranch(branchName, opts.Verbose); err != nil { @@ -198,7 +205,7 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts func buildAddWorkflowPRBody(workflows []*ResolvedWorkflow, opts AddOptions) string { var body strings.Builder - if opts.createdByAddWizard { + if opts.addWizard != nil { fmt.Fprintf(&body, "This pull request was created with [`gh aw add-wizard`](https://github.github.com/gh-aw/) from [GitHub Agentic Workflows](https://github.com/github/gh-aw), version `%s`.\n", markdownText(GetVersion())) } else { fmt.Fprintf(&body, "This pull request was created with [`gh aw add`](https://github.github.com/gh-aw/) from [GitHub Agentic Workflows](https://github.com/github/gh-aw), version `%s`.\n", markdownText(GetVersion())) @@ -216,14 +223,16 @@ func buildAddWorkflowPRBody(workflows []*ResolvedWorkflow, opts AddOptions) stri body.WriteString("\n## Options selected\n\n") body.WriteString("- **Delivery:** pull request\n") - fmt.Fprintf(&body, "- **Engine:** `%s`\n", markdownText(opts.EngineOverride)) + if opts.EngineOverride != "" { + fmt.Fprintf(&body, "- **Engine:** `%s`\n", markdownText(opts.EngineOverride)) + } if opts.EngineOverride == "copilot" { auth := "`COPILOT_GITHUB_TOKEN` repository secret" if opts.AddCopilotRequestsPermission { auth = "organization billing via `permissions.copilot-requests: write`" - } else if opts.addWizardSecretExists { - auth = "existing `COPILOT_GITHUB_TOKEN` repository or organization secret" - } else if opts.addWizardSkipSecret { + } else if opts.addWizard != nil && opts.addWizard.secretSource != "" { + auth = fmt.Sprintf("existing `COPILOT_GITHUB_TOKEN` %s secret", opts.addWizard.secretSource) + } else if opts.addWizard != nil && opts.addWizard.skipSecret { auth = "`COPILOT_GITHUB_TOKEN` setup skipped" } fmt.Fprintf(&body, "- **Authentication:** %s\n", auth) @@ -240,14 +249,18 @@ func buildAddWorkflowPRBody(workflows []*ResolvedWorkflow, opts AddOptions) stri if opts.Force { body.WriteString("- **Existing workflow files:** overwrite confirmed\n") } - if opts.createdByAddWizard { - fmt.Fprintf(&body, "- **GitHub App permission and event inference:** %s\n", enabledText(!opts.addWizardDisableGitHubAppInference)) + if opts.addWizard != nil { + fmt.Fprintf(&body, "- **GitHub App permission and event inference:** %s\n", enabledText(!opts.addWizard.disableGitHubAppPermissionInference)) } if opts.AppendText != "" { body.WriteString("- **Custom appended instructions:** included\n") } - if len(opts.initializedFiles) > 0 { - fmt.Fprintf(&body, "- **Repository initialization:** %s\n", joinCodeValues(opts.initializedFiles)) + if opts.addWizard != nil && len(opts.addWizard.initializedFiles) > 0 { + paths := make([]string, 0, len(opts.addWizard.initializedFiles)) + for _, file := range opts.addWizard.initializedFiles { + paths = append(paths, file.displayPath) + } + fmt.Fprintf(&body, "- **Repository initialization:** %s\n", joinCodeValues(paths)) } body.WriteString("\n## Review criteria\n\n") @@ -261,7 +274,7 @@ func buildAddWorkflowPRBody(workflows []*ResolvedWorkflow, opts AddOptions) stri body.WriteString("\n## Forward progress\n\n") body.WriteString("1. Review the changes against the criteria above; request or make changes in the Markdown workflow source, then recompile it with `gh aw compile`.\n") body.WriteString("2. Merge this pull request to install the workflow") - if opts.createdByAddWizard && opts.EngineOverride == "copilot" && !opts.AddCopilotRequestsPermission && !opts.addWizardSecretExists && !opts.addWizardSkipSecret { + if opts.addWizard != nil && opts.EngineOverride == "copilot" && !opts.AddCopilotRequestsPermission && opts.addWizard.secretSource == "" && !opts.addWizard.skipSecret { body.WriteString(". After merge, the add wizard will configure `COPILOT_GITHUB_TOKEN` when needed") } body.WriteString(".\n") diff --git a/pkg/cli/add_workflow_pr_test.go b/pkg/cli/add_workflow_pr_test.go index aba12594f0d..68d570cb2f7 100644 --- a/pkg/cli/add_workflow_pr_test.go +++ b/pkg/cli/add_workflow_pr_test.go @@ -218,11 +218,15 @@ func TestBuildAddWorkflowPRBody(t *testing.T) { Description: "Helps maintain the repository", } opts := AddOptions{ - EngineOverride: "copilot", - createdByAddWizard: true, - addWizardSecretExists: true, - initializedFiles: []string{".gitattributes", ".github/aw/actions-lock.json"}, - addWizardDisableGitHubAppInference: true, + EngineOverride: "copilot", + addWizard: &addWizardOptions{ + secretSource: secretSourceOrganizationSelected, + initializedFiles: []addInitializedFile{ + {path: "/home/user/repo/.gitattributes", displayPath: ".gitattributes"}, + {path: "/home/user/repo/.github/aw/actions-lock.json", displayPath: ".github/aw/actions-lock.json"}, + }, + disableGitHubAppPermissionInference: true, + }, } body := buildAddWorkflowPRBody([]*ResolvedWorkflow{workflow}, opts) @@ -233,9 +237,10 @@ func TestBuildAddWorkflowPRBody(t *testing.T) { assert.Contains(t, body, "Helps maintain the repository") assert.Contains(t, body, "`schedule` (`weekly`), `workflow_dispatch`") assert.Contains(t, body, "**Delivery:** pull request") - assert.Contains(t, body, "existing `COPILOT_GITHUB_TOKEN` repository or organization secret") + assert.Contains(t, body, "existing `COPILOT_GITHUB_TOKEN` organization (selected repository) secret") assert.Contains(t, body, "**GitHub App permission and event inference:** disabled") assert.Contains(t, body, "`.gitattributes`, `.github/aw/actions-lock.json`") + assert.NotContains(t, body, "/home/user/repo") assert.Contains(t, body, "## Review criteria") assert.Contains(t, body, "## Forward progress") assert.NotContains(t, body, "will configure `COPILOT_GITHUB_TOKEN`") @@ -247,7 +252,7 @@ func TestBuildAddWorkflowPRBodyUsesLocalSourceAndSecretNextStep(t *testing.T) { Content: []byte("---\non: issues\n---\n"), SourceInfo: &FetchedWorkflow{IsLocal: true, SourcePath: "./review.md"}, } - opts := AddOptions{EngineOverride: "copilot", createdByAddWizard: true} + opts := AddOptions{EngineOverride: "copilot", addWizard: &addWizardOptions{}} body := buildAddWorkflowPRBody([]*ResolvedWorkflow{workflow}, opts) @@ -257,6 +262,18 @@ func TestBuildAddWorkflowPRBodyUsesLocalSourceAndSecretNextStep(t *testing.T) { assert.Contains(t, body, "recompile it with `gh aw compile`") } +func TestBuildAddWorkflowPRBodyOmitsEmptyEngine(t *testing.T) { + workflow := &ResolvedWorkflow{ + Spec: &WorkflowSpec{WorkflowPath: "./review.md", WorkflowName: "review"}, + Content: []byte("---\non: issues\n---\n"), + SourceInfo: &FetchedWorkflow{IsLocal: true, SourcePath: "./review.md"}, + } + + body := buildAddWorkflowPRBody([]*ResolvedWorkflow{workflow}, AddOptions{}) + + assert.NotContains(t, body, "**Engine:**") +} + func TestBuildAddWorkflowPRBodyPreservesDescriptionMarkdown(t *testing.T) { content := `--- description: | diff --git a/pkg/cli/preconditions.go b/pkg/cli/preconditions.go index bd07dde9dfe..b3fbd47f881 100644 --- a/pkg/cli/preconditions.go +++ b/pkg/cli/preconditions.go @@ -194,18 +194,20 @@ func checkUserPermissionsShared(repoSlug string, verbose bool) (bool, error) { // checkRepoVisibilityShared checks if the repository is public or private func checkRepoVisibilityShared(repoSlug string) bool { + return getRepoVisibilityShared(repoSlug) == "public" +} + +func getRepoVisibilityShared(repoSlug string) string { preconditionsLog.Print("Checking repository visibility") // Use gh api to check repository visibility output, err := workflow.RunGH("Checking repository visibility...", "api", "/repos/"+repoSlug, "--jq", ".visibility") if err != nil { preconditionsLog.Printf("Could not check repository visibility: %v", err) - // Default to public if we can't determine - return true + return "unknown" } visibility := strings.TrimSpace(string(output)) - isPublic := visibility == "public" - preconditionsLog.Printf("Repository visibility: %s (isPublic=%v)", visibility, isPublic) - return isPublic + preconditionsLog.Printf("Repository visibility: %s", visibility) + return visibility }