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..f0fb4deeadf --- /dev/null +++ b/.changeset/patch-use-existing-copilot-token-add-wizard.md @@ -0,0 +1,5 @@ +--- +"gh-aw": minor +--- + +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/docs/src/content/docs/setup/cli.md b/docs/src/content/docs/setup/cli.md index de10f1dac98..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 @@ -198,9 +200,9 @@ 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. +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` @@ -217,7 +219,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..f78d3bd3877 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -78,12 +78,40 @@ 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 // 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 + 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 @@ -131,6 +159,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 +184,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 +255,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") @@ -264,8 +300,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.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) + } } } @@ -436,9 +474,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 { @@ -451,6 +491,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 { @@ -538,23 +587,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) } } @@ -825,7 +874,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_command_test.go b/pkg/cli/add_command_test.go index 79ac4645442..432a66d40cc 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) { @@ -524,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 + originalConfirmAuthoringSupport := addConfirmAuthoringSupport + t.Cleanup(func() { + addFindGitRoot = originalFindGitRoot + addInitRepository = originalInitRepository + addMissingInitMarkers = originalMissingInitMarkers + addConfirmAuthoringSupport = originalConfirmAuthoringSupport + }) + + repoDir := t.TempDir() + addFindGitRoot = func() (string, error) { return repoDir, nil } + + t.Run("already initialized skips confirmation", func(t *testing.T) { + 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 + } + 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) { + addMissingInitMarkers = func(string, string) ([]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" + 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, []addInitializedFile{{ + path: filepath.Join(repoDir, filepath.FromSlash(marker)), displayPath: marker, + }}, files) + }) + + 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) + } + + plan, err := confirmAddRepositoryInitialization(context.Background(), "copilot", false) + require.NoError(t, err) + files, err := applyAddRepositoryInitialization(plan, "copilot", false, false) + require.NoError(t, err) + require.Equal(t, []addInitializedFile{{ + path: markerPath, displayPath: marker, wasExisting: true, originalContent: []byte("original"), + }}, files) + }) +} + func TestAddResolvedWorkflows_IgnoresBootstrapRequireOwnerTypeDuringInstall(t *testing.T) { originalCheckOwnerType := bootstrapCheckOwnerType t.Cleanup(func() { 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_init.go b/pkg/cli/add_init.go index 95f2d54d649..537c7cdaf91 100644 --- a/pkg/cli/add_init.go +++ b/pkg/cli/add_init.go @@ -1,16 +1,127 @@ package cli import ( + "context" "errors" "fmt" + "os" "path/filepath" + "slices" + "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 addConfirmAuthoringSupport = func(ctx context.Context) (bool, error) { + addAuthoringSupport := true + form := console.NewConfirmForm( + huh.NewConfirm(). + 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 prompts and skills confirmation failed: %w", err) + } + return addAuthoringSupport, nil +} + +type addRepositoryInitializationPlan struct { + enabled bool + 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 { + if errors.Is(err, gitutil.ErrNotGitRepository) { + return addRepositoryInitializationPlan{}, nil + } + return addRepositoryInitializationPlan{}, 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 = addMissingInitMarkers(".", engineOverride) + if inspectErr != nil { + return inspectErr + } + if noGitattributes { + missingMarkers = slices.DeleteFunc(missingMarkers, func(path string) bool { return path == ".gitattributes" }) + } + return nil + }); err != nil { + return addRepositoryInitializationPlan{}, fmt.Errorf("failed to inspect repository initialization state: %w", err) + } + if len(missingMarkers) == 0 { + return addRepositoryInitializationPlan{}, nil + } + + confirmed, err := addConfirmAuthoringSupport(ctx) + if err != nil || !confirmed { + if err == nil { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Coding agent prompts and skills: skipped")) + } + return addRepositoryInitializationPlan{}, err + } + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Coding agent prompts and skills: enabled")) + return addRepositoryInitializationPlan{enabled: true, files: missingMarkers}, nil +} + +func applyAddRepositoryInitialization(plan addRepositoryInitializationPlan, engineOverride string, verbose bool, noGitattributes bool) ([]addInitializedFile, error) { + if !plan.enabled { + return nil, nil + } + return ensureAddRepositoryInitializedFromPlan(plan.files, engineOverride, verbose, noGitattributes) +} + +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 + } + 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) @@ -33,42 +144,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, - 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 @@ -76,3 +153,39 @@ func ensureAddRepositoryInitializedWithDetails(engineOverride string, verbose bo return initializedFiles, nil } + +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 6083d844b32..405dc7179b6 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( @@ -68,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 96ee85a911a..d9e81c53b51 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?"). @@ -262,11 +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)) - } - - fmt.Fprintln(os.Stderr, "") // Build select options. // When billing is confirmed enabled, copilot-requests is listed first (pre-selected). @@ -289,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("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(copilotAuthMethodDescription(probe, c.secretSources[constants.CopilotGitHubToken])). Options(options...). Value(&authMethod) @@ -312,6 +306,18 @@ func (c *AddInteractiveConfig) selectCopilotAuthMethod() error { return nil } +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.)" + } + patDescription := "• PAT: Create or use 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 // 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. @@ -322,6 +328,6 @@ 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.FormatInfoMessage("A fine-grained PAT with Copilot Requests permission will be required.")) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Selected authentication: COPILOT_GITHUB_TOKEN")) } } diff --git a/pkg/cli/add_interactive_engine_test.go b/pkg/cli/add_interactive_engine_test.go index 37192aa7291..0df869c518d 100644 --- a/pkg/cli/add_interactive_engine_test.go +++ b/pkg/cli/add_interactive_engine_test.go @@ -56,6 +56,25 @@ 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: 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("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}, 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) + }) +} + func TestPrioritizeEngineOption(t *testing.T) { t.Parallel() options := []huh.Option[string]{ diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index dc0034b2054..37242b57280 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -6,6 +6,8 @@ import ( "fmt" "os" "os/exec" + "path/filepath" + "slices" "strconv" "strings" @@ -33,11 +35,9 @@ 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") - 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) @@ -47,7 +47,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, @@ -55,9 +55,18 @@ func (c *AddInteractiveConfig) createWorkflowChangesAndConfigureSecret(ctx conte NoStopAfter: c.NoStopAfter, StopAfter: c.StopAfter, DisableSecurityScanner: c.DisableSecurityScanner, + RepoSlug: c.RepoOverride, AddCopilotRequestsPermission: c.UseCopilotRequests, - initializedFiles: initFiles, + GhAwRef: c.GhAwRef, + 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) @@ -65,7 +74,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 } @@ -89,7 +97,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) } @@ -119,8 +126,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 @@ -136,12 +141,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) } @@ -303,31 +310,176 @@ 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") + 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 } - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Working directory is clean")) - return nil + for { + if c.Ctx != nil { + select { + case <-c.Ctx.Done(): + return c.Ctx.Err() + default: + } + } + blockers, inspectErr := inspectAddWorkingTreeAtRoot(gitRoot, 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) plannedAddPathsAtRoot(gitRoot string, workflowFiles, initFiles []string) ([]string, error) { + 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)) + } + planned = append(planned, initFiles...) + 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) + } + 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() + 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 { + if slices.Contains(values, 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), + ) + 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 21efa90c44c..7f388fc4644 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, @@ -49,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. @@ -60,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 @@ -68,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 @@ -100,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() @@ -178,11 +166,12 @@ 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() + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(c.sourceWorkflowMessage())) if err := c.checkGHAuthStatus(); err != nil { return err } @@ -195,7 +184,24 @@ func (c *AddInteractiveConfig) runInitialAddInteractiveChecks() error { return c.checkUserPermissions() } -func (c *AddInteractiveConfig) prepareAndConfirmAddInteractive() (workflowFiles, initFiles []string, secretName, secretValue string, createPR bool, err error) { +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) sourceWorkflowMessage() string { + return "Source workflow: " + strings.Join(c.WorkflowSpecs, ", ") +} + +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 @@ -204,32 +210,40 @@ 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 { + initializationPlan, err := confirmAddRepositoryInitialization(c.Ctx, c.EngineOverride, c.NoGitattributes) + if err != nil { return nil, nil, "", "", false, err } - createPR, err = c.confirmChanges(workflowFiles, initFiles) + createPR, err = c.confirmChanges(workflowFiles, initializationPlan.files) if err != nil { return nil, nil, "", "", false, err } + if createPR { + 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, initFiles, "", "", false, nil + return workflowFiles, nil, "", "", false, nil } - if err := c.checkCleanWorkingDirectoryForPR(); err != nil { + initFiles, err = applyAddRepositoryInitialization(initializationPlan, c.EngineOverride, c.Verbose, c.NoGitattributes) + if err != nil { return nil, nil, "", "", false, err } - // 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 { @@ -320,8 +334,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) } @@ -372,13 +385,12 @@ 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) } - fmt.Fprintln(os.Stderr, "") } createPR := true // Default to yes @@ -394,6 +406,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 } @@ -428,26 +445,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 ae35bf082b8..b2ed5cd18fb 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" ) @@ -134,6 +136,37 @@ 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()) + 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 { @@ -311,8 +344,26 @@ 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 + 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. t.Setenv("ACCESSIBLE", "1") r, w, err := os.Pipe() require.NoError(t, err) @@ -345,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_schedule.go b/pkg/cli/add_interactive_schedule.go index 5e3961d6625..7ea399d19b5 100644 --- a/pkg/cli/add_interactive_schedule.go +++ b/pkg/cli/add_interactive_schedule.go @@ -215,13 +215,10 @@ 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), @@ -236,6 +233,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 +270,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_secrets.go b/pkg/cli/add_interactive_secrets.go index 35caabf0ffe..0963ef8c8eb 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,33 +12,65 @@ import ( "github.com/github/gh-aw/pkg/workflow" ) +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 := 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 } 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 := 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), "--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 { - for _, name := range parseSecretNames(orgOutput) { - c.existingSecrets[name] = struct{}{} - addInteractiveLog.Printf("Found existing org secret: %s", name) + responses, err := parseOrganizationSecretsResponses(orgOutput) + if err != nil { + addInteractiveLog.Printf("Could not parse organization secrets: %v", err) + } else { + 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) + } + } + } } } } @@ -49,6 +82,59 @@ 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 (c *AddInteractiveConfig) organizationSecretAvailable(org string, secret organizationSecret) bool { + switch secret.Visibility { + case "all": + return true + case "private": + 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", + ) + 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 + } +} + +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 72710dd0ca4..3cb013c5323 100644 --- a/pkg/cli/add_interactive_secrets_test.go +++ b/pkg/cli/add_interactive_secrets_test.go @@ -295,15 +295,49 @@ 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": + 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"} + ]},{"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) + 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", repositoryVisibility: "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, secretSourceRepository, config.secretSources["REPOSITORY_SECRET"]) + assert.Equal(t, secretSourceOrganizationSelected, config.secretSources["SELECTED_SECRET"]) - assert.NotNil(t, config.existingSecrets, "existingSecrets map should be initialized") + config.repositoryVisibility = "internal" + assert.False(t, config.organizationSecretAvailable("test-owner", organizationSecret{ + Name: "PRIVATE_SECRET", + Visibility: "private", + })) } diff --git a/pkg/cli/add_interactive_workflow.go b/pkg/cli/add_interactive_workflow.go index 555c69f978e..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 @@ -57,9 +55,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))) @@ -159,7 +159,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(). @@ -184,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 } @@ -205,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/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_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..910db268368 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,16 @@ 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 { +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 +113,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 +143,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 +154,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 +173,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 +218,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 +235,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/add_workflow_pr.go b/pkg/cli/add_workflow_pr.go index d5a32bbc616..e35f842390f 100644 --- a/pkg/cli/add_workflow_pr.go +++ b/pkg/cli/add_workflow_pr.go @@ -4,17 +4,25 @@ 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" ) 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_-]+`) @@ -76,8 +84,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 @@ -95,6 +110,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 { @@ -115,7 +136,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 @@ -123,8 +143,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. @@ -148,6 +168,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,8 +186,9 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts } // Create PR + prepareSpinner.Stop() 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 { @@ -183,3 +207,172 @@ 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.addWizard != nil { + 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`](%s) from [GitHub Agentic Workflows](%s), version `%s`.\n", ghAwDocumentationURL, ghAwRepositoryURL, 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", markdownBlock(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") + 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.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) + } + 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.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 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") + 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.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") + 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 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 { + 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..68d570cb2f7 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,98 @@ 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", + 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) + + 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` 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`") +} + +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", addWizard: &addWizardOptions{}} + + 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`") +} + +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: | + 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") +} diff --git a/pkg/cli/copilot_billing_check.go b/pkg/cli/copilot_billing_check.go index 96156ef3500..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 — check with your org admin." +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 diff --git a/pkg/cli/engine_secrets.go b/pkg/cli/engine_secrets.go index 1ed19cf9bb9..f04e7e7e843 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 @@ -338,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/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) { 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/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/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 } 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/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..0c65ca0366a 100644 --- a/pkg/console/prompt_form.go +++ b/pkg/console/prompt_form.go @@ -3,32 +3,90 @@ package console import ( + "context" "errors" + "fmt" + "io" + "strings" "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" + promptReservedRows = 12 +) + +// 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 +} + // 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() + clearOnRun := tty.IsStderrTerminal() && !accessible + form := huh.NewForm(groups...).WithTheme(styles.HuhTheme).WithAccessible(accessible) + if clearOnRun { + form = form.WithHeight(promptReservedRows) + } + return &PromptForm{ + Form: form, + out: stderrWriter(), + clearOnRun: clearOnRun, + } } // 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)) } +// 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 { + fmt.Fprintln(f.out) + return runForm() + } + // 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 19edc3ddf31..21534d8cb42 100644 --- a/pkg/console/prompt_form_test.go +++ b/pkg/console/prompt_form_test.go @@ -3,8 +3,10 @@ package console import ( + "bytes" "errors" "fmt" + "strings" "testing" "charm.land/huh/v2" @@ -13,7 +15,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](). @@ -24,6 +27,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} + + err := form.run(func() error { return nil }) + + require.NoError(t, err) + require.Equal(t, strings.Repeat("\n", promptReservedRows)+cursorUp(promptReservedRows)+ansiSaveCursor+"\n"+ansiRestoreCursor+ansiClearScreenBelow, output.String()) +} + +func TestPromptFormDoesNotClearAccessibleOrNonTTYQuestion(t *testing.T) { + var output bytes.Buffer + form := &PromptForm{out: &output, clearOnRun: false} + + 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)) 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