diff --git a/.github/workflows/smoke-copilot-auto.lock.yml b/.github/workflows/smoke-copilot-auto.lock.yml index 80f7930c2f1..5c316f6e8b8 100644 --- a/.github/workflows/smoke-copilot-auto.lock.yml +++ b/.github/workflows/smoke-copilot-auto.lock.yml @@ -149,7 +149,7 @@ jobs: GH_AW_INFO_FIREWALL_TYPE: "squid" GH_AW_INFO_FRONTMATTER_EMOJI: "🌸" GH_AW_COMPILED_STRICT: "true" - GH_AW_INFO_MODEL_COSTS: '{"providers":{"github-copilot":{"models":{"auto":{"cost":{"input":"8.5e-07","output":"1.55e-06"}}}}}}' + GH_AW_INFO_MODEL_COSTS: '{"providers":{"github-copilot":{"models":{"auto":{"cost":{"input":"0","output":"0"}}}}}}' GH_AW_INFO_FEATURES: '{"gh-aw-detection":false}' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 with: diff --git a/docs/adr/48553-extract-private-helpers-to-reduce-function-length.md b/docs/adr/48553-extract-private-helpers-to-reduce-function-length.md new file mode 100644 index 00000000000..30d79293035 --- /dev/null +++ b/docs/adr/48553-extract-private-helpers-to-reduce-function-length.md @@ -0,0 +1,46 @@ +# ADR-48553: Extract Private Helpers to Reduce Function Length + +**Date**: 2026-07-28 +**Status**: Draft +**Deciders**: pelikhan, copilot-swe-agent + +--- + +### Context + +The `pkg/workflow` and `pkg/cli` packages contain numerous functions that exceed the project's function-length lint threshold. As of this PR the backlog stands at 675 findings, with individual functions reaching 377 lines (`BuildAWFCommand`), 259 lines (`generateUnifiedPromptCreationStep`), 226 lines (`buildSafeOutputsSections`), 220 lines (`buildSafeOutputsHandlerOutputsAndActionSteps`), 204 lines (`GetExecutionSteps`), and 199 lines (`checkoutConfigFromMap`). Long functions make it harder to read, reason about, and test individual behaviors in isolation. The lint enforcement creates a growing backlog that compounds with each new feature. + +### Decision + +We will incrementally reduce the function-length backlog by extracting focused private helper functions from the longest offenders. Each extraction preserves exact logic and introduces no behavioral changes. Helpers are kept private (unexported) in the same file or package to avoid widening the package API. Correctness is verified by recompiling lock files and confirming zero drift. + +### Alternatives Considered + +#### Alternative 1: Suppress Lint Warnings with `//nolint:funlen` Directives + +Add per-function nolint comments to acknowledge exceptions without changing code. This eliminates the backlog instantly with minimal churn. However, it normalizes the long-function pattern, provides no readability or testability benefit, and leaves future maintainers with the same cognitive burden. It was rejected because the lint rule exists precisely to prevent functions from growing unbounded. + +#### Alternative 2: Restructure with Design Patterns (Strategy, Visitor, Table-Driven Dispatch) + +Replace long imperative functions with data-driven dispatch tables, strategy interfaces, or visitor structs. This approach would eliminate the underlying cause rather than just the symptom, and would provide stronger extensibility. However, it requires non-trivial logic changes and increases refactoring scope and risk significantly β€” any behavioral divergence is hard to detect in a large diff. It was deferred in favor of the safer, mechanical extraction approach, which can be landed incrementally with high confidence. + +### Consequences + +#### Positive +- Function-length lint findings reduced (675 β†’ 664 in this PR), demonstrating a repeatable, incremental approach to clearing the backlog. +- Individual private helpers are independently understandable and can be targeted by focused unit tests without executing the entire parent function. +- Smaller call-site bodies reduce the cognitive load for reviewers reading the top-level orchestration logic. + +#### Negative +- Call stacks deepen as logic is indirected through private helpers, which can make debugging and profiling slightly harder. +- Granularity decisions are made per-PR rather than against a consistent design document, risking divergent helper naming and abstraction levels across the codebase. +- The backlog is reduced incrementally rather than eliminated; without a systematic plan, the remaining findings may accumulate faster than they are cleared. + +#### Neutral +- All extracted helpers remain unexported, so the public API surface of `pkg/workflow` and `pkg/cli` is unchanged. +- Lock file recompilation (`make recompile`) is used as the correctness oracle β€” zero drift confirms behavioral equivalence. +- The PR specifically calls out one ordering regression (`push_to_pull_request_branch` moved in the safe-output-tools prompt) that was caught and corrected, illustrating that purely mechanical extraction still requires careful review. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index bb4ae9a995e..6254c8f762f 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -22,17 +22,46 @@ func isAlreadyMergedGHError(err error) bool { return strings.Contains(err.Error(), "already merged") || strings.Contains(err.Error(), "MERGED") } +type mergeAction string + +const ( + mergeActionAttempt mergeAction = "attempt" + mergeActionEditTitle mergeAction = "editTitle" + mergeActionReview mergeAction = "review" + mergeActionConfirmed mergeAction = "confirmed" + mergeActionExit mergeAction = "exit" +) + +type prMergeState struct { + done bool + failed bool + userReviewing bool +} + // createWorkflowPRAndConfigureSecret creates the PR, merges it, and adds the secret func (c *AddInteractiveConfig) createWorkflowPRAndConfigureSecret(ctx context.Context, workflowFiles, initFiles []string, secretName, secretValue string) error { addInteractiveLog.Print("Applying changes") fmt.Fprintln(os.Stderr, "") + result, err := c.addResolvedWorkflowsWithPR(ctx) + if err != nil { + return fmt.Errorf("failed to add workflow: %w", err) + } + c.addResult = result + + if err := c.ensureWorkflowPRMerged(result); err != nil { + return err + } + + return c.configureRepositorySecret(secretName, secretValue) +} + +func (c *AddInteractiveConfig) addResolvedWorkflowsWithPR(ctx context.Context) (*AddWorkflowsResult, error) { // Add the workflow using existing implementation with --create-pull-request // Pass the resolved workflows to avoid re-fetching them // Pass Quiet=true to suppress detailed output (already shown earlier in interactive mode) - // This returns the result including PR number and HasWorkflowDispatch - opts := AddOptions{ + return AddResolvedWorkflows(ctx, c.WorkflowSpecs, c.resolvedWorkflows, AddOptions{ Verbose: c.Verbose, Quiet: true, EngineOverride: c.EngineOverride, @@ -46,154 +75,161 @@ func (c *AddInteractiveConfig) createWorkflowPRAndConfigureSecret(ctx context.Co StopAfter: c.StopAfter, DisableSecurityScanner: c.DisableSecurityScanner, AddCopilotRequestsPermission: c.UseCopilotRequests, - } - result, err := AddResolvedWorkflows(ctx, c.WorkflowSpecs, c.resolvedWorkflows, opts) - if err != nil { - return fmt.Errorf("failed to add workflow: %w", err) - } - c.addResult = result + }) +} - // Step 8b: Optionally merge the PR – loop until merged, confirmed-merged, or user exits +func (c *AddInteractiveConfig) ensureWorkflowPRMerged(result *AddWorkflowsResult) error { if result.PRNumber == 0 { - if result.PRURL == "" { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Requested workflow files already exist locally; no pull request was created.")) - return nil + return c.handleWorkflowPRWithoutNumber(result) + } + + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Pull request created: "+result.PRURL)) + fmt.Fprintln(os.Stderr, "") + + state := prMergeState{} + for !state.done { + action, err := c.promptPRMergeAction(result.PRURL, state) + if err != nil { + return err } - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Could not determine PR number")) - fmt.Fprintln(os.Stderr, "Please merge the PR manually from the GitHub web interface.") - } else { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Pull request created: "+result.PRURL)) + if err := c.handlePRMergeAction(result, action, &state); err != nil { + return err + } + } + + return nil +} + +func (c *AddInteractiveConfig) handleWorkflowPRWithoutNumber(result *AddWorkflowsResult) error { + if result.PRURL == "" { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Requested workflow files already exist locally; no pull request was created.")) + return nil + } + fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Could not determine PR number")) + fmt.Fprintln(os.Stderr, "Please merge the PR manually from the GitHub web interface.") + return nil +} + +func (c *AddInteractiveConfig) promptPRMergeAction(prURL string, state prMergeState) (mergeAction, error) { + var chosen mergeAction + selectForm := console.NewSelectForm( + huh.NewSelect[mergeAction](). + Title("What would you like to do with pull request " + prURL + "?"). + Options(c.buildPRMergeOptions(state)...). + Value(&chosen), + ) + if err := selectForm.Run(); err != nil { + return "", fmt.Errorf("failed to get user input: %w", err) + } + return chosen, nil +} + +func (c *AddInteractiveConfig) buildPRMergeOptions(state prMergeState) []huh.Option[mergeAction] { + options := []huh.Option[mergeAction]{ + huh.NewOption("Attempt to merge", mergeActionAttempt), + } + if state.failed { + options = append(options, huh.NewOption("Edit PR title and retry", mergeActionEditTitle)) + } + if state.userReviewing { + options = append(options, huh.NewOption("PR has been manually merged", mergeActionConfirmed)) + options = append(options, huh.NewOption("Exit, I'm done here", mergeActionExit)) + return options + } + options = append(options, huh.NewOption("I'll review/merge myself", mergeActionReview)) + options = append(options, huh.NewOption("Exit", mergeActionExit)) + return options +} + +func (c *AddInteractiveConfig) handlePRMergeAction(result *AddWorkflowsResult, action mergeAction, state *prMergeState) error { + switch action { + case mergeActionAttempt: + c.attemptWorkflowPRMerge(result, state) + case mergeActionEditTitle: + return c.promptAndUpdatePRTitle(result, state) + case mergeActionReview: + state.userReviewing = true + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Please review and merge the pull request: "+result.PRURL)) fmt.Fprintln(os.Stderr, "") + case mergeActionConfirmed: + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Great – continuing with the merged pull request")) + state.done = true + case mergeActionExit: + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Exiting. You can merge the pull request later: "+result.PRURL)) + return errors.New("user exited before PR was merged") + } + return nil +} - // mergeAction values used in the select loop - type mergeAction string - const ( - mergeActionAttempt mergeAction = "attempt" - mergeActionEditTitle mergeAction = "editTitle" - mergeActionReview mergeAction = "review" - mergeActionConfirmed mergeAction = "confirmed" - mergeActionExit mergeAction = "exit" - ) - - mergeDone := false // true when the PR is merged (or confirmed merged) - mergeFailed := false // true after an unsuccessful merge attempt - userReviewing := false // true after the user chose "I'll review myself" - - for !mergeDone { - // Build option list based on current state - var options []huh.Option[mergeAction] - - options = append(options, huh.NewOption("Attempt to merge", mergeActionAttempt)) - - if mergeFailed { - options = append(options, huh.NewOption("Edit PR title and retry", mergeActionEditTitle)) - } - - if userReviewing { - options = append(options, huh.NewOption("PR has been manually merged", mergeActionConfirmed)) - } else { - options = append(options, huh.NewOption("I'll review/merge myself", mergeActionReview)) - } - - if userReviewing { - options = append(options, huh.NewOption("Exit, I'm done here", mergeActionExit)) - } else { - options = append(options, huh.NewOption("Exit", mergeActionExit)) - } - - var chosen mergeAction - selectForm := console.NewSelectForm( - huh.NewSelect[mergeAction](). - Title("What would you like to do with pull request " + result.PRURL + "?"). - Options(options...). - Value(&chosen), - ) - - if selectErr := selectForm.Run(); selectErr != nil { - return fmt.Errorf("failed to get user input: %w", selectErr) - } - - switch chosen { - case mergeActionAttempt: - if mergeErr := c.mergePullRequest(result.PRNumber); mergeErr != nil { - if isAlreadyMergedGHError(mergeErr) { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Merged pull request "+result.PRURL)) - mergeDone = true - } else { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to merge PR: %v", mergeErr))) - if mergeFailed { - fmt.Fprintln(os.Stderr, "Please merge the PR manually: "+result.PRURL) - } - mergeFailed = true - } - } else { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Merged pull request "+result.PRURL)) - mergeDone = true - } - - case mergeActionEditTitle: - var newTitle string - titleForm := console.NewInputForm( - huh.NewInput(). - Title("Enter new PR title"). - Description("Add a prefix if required, for example: feat: or fix:"). - Value(&newTitle), - ) - if titleErr := titleForm.Run(); titleErr != nil { - return fmt.Errorf("failed to get user input: %w", titleErr) - } - newTitle = strings.TrimSpace(newTitle) - if newTitle == "" { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("PR title cannot be empty, keeping current title")) - } else if editErr := editPRTitle(result.PRNumber, newTitle, c.RepoOverride); editErr != nil { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to update PR title: %v", editErr))) - } else { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("PR title updated to: "+newTitle)) - mergeFailed = false - } - - case mergeActionReview: - userReviewing = true - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Please review and merge the pull request: "+result.PRURL)) - fmt.Fprintln(os.Stderr, "") - - case mergeActionConfirmed: - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Great – continuing with the merged pull request")) - mergeDone = true - - case mergeActionExit: - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Exiting. You can merge the pull request later: "+result.PRURL)) - return errors.New("user exited before PR was merged") - } +func (c *AddInteractiveConfig) attemptWorkflowPRMerge(result *AddWorkflowsResult, state *prMergeState) { + if err := c.mergePullRequest(result.PRNumber); err != nil { + if isAlreadyMergedGHError(err) { + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Merged pull request "+result.PRURL)) + state.done = true + return } + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to merge PR: %v", err))) + if state.failed { + fmt.Fprintln(os.Stderr, "Please merge the PR manually: "+result.PRURL) + } + state.failed = true + return } + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Merged pull request "+result.PRURL)) + state.done = true +} - // Step 8c: Add the secret (skip if no secret configured or already exists in repository) +func (c *AddInteractiveConfig) promptAndUpdatePRTitle(result *AddWorkflowsResult, state *prMergeState) error { + var newTitle string + titleForm := console.NewInputForm( + huh.NewInput(). + Title("Enter new PR title"). + Description("Add a prefix if required, for example: feat: or fix:"). + Value(&newTitle), + ) + if err := titleForm.Run(); err != nil { + return fmt.Errorf("failed to get user input: %w", err) + } + newTitle = strings.TrimSpace(newTitle) + if newTitle == "" { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage("PR title cannot be empty, keeping current title")) + return nil + } + if err := editPRTitle(result.PRNumber, newTitle, c.RepoOverride); err != nil { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to update PR title: %v", err))) + return nil + } + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("PR title updated to: "+newTitle)) + state.failed = false + return nil +} + +func (c *AddInteractiveConfig) configureRepositorySecret(secretName, secretValue string) error { if secretName == "" { // No secret to configure (e.g., user doesn't have write access to the repository) - } else if secretValue == "" { - // Secret already exists in repo, nothing to do + return nil + } + if secretValue == "" { if c.Verbose { fmt.Fprintln(os.Stderr, "") fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Secret '%s' already configured", secretName))) } - } else { - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatProgressMessage(fmt.Sprintf("Adding secret '%s' to repository...", secretName))) - - if err := c.addRepositorySecret(secretName, secretValue); err != nil { - fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("Failed to add secret: %v", err))) - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, "Please add the secret manually:") - fmt.Fprintln(os.Stderr, " 1. Go to your repository Settings β†’ Secrets and variables β†’ Actions") - fmt.Fprintf(os.Stderr, " 2. Click 'New repository secret' and add '%s'\n", secretName) - return fmt.Errorf("failed to add secret: %w", err) - } + return nil + } - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Secret '%s' added", secretName))) + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatProgressMessage(fmt.Sprintf("Adding secret '%s' to repository...", secretName))) + if err := c.addRepositorySecret(secretName, secretValue); err != nil { + fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("Failed to add secret: %v", err))) + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, "Please add the secret manually:") + fmt.Fprintln(os.Stderr, " 1. Go to your repository Settings β†’ Secrets and variables β†’ Actions") + fmt.Fprintf(os.Stderr, " 2. Click 'New repository secret' and add '%s'\n", secretName) + return fmt.Errorf("failed to add secret: %w", err) } + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Secret '%s' added", secretName))) return nil } diff --git a/pkg/cli/add_interactive_workflow.go b/pkg/cli/add_interactive_workflow.go index cb011f1dc2a..9ecb656623a 100644 --- a/pkg/cli/add_interactive_workflow.go +++ b/pkg/cli/add_interactive_workflow.go @@ -20,85 +20,129 @@ func (c *AddInteractiveConfig) checkStatusAndOfferRun(ctx context.Context) error // Wait a moment for GitHub to process the merge fmt.Fprintln(os.Stderr, "") - // Use spinner only in non-verbose mode (spinner can't be restarted after stop) - var spinner *console.SpinnerWrapper - if !c.Verbose { - spinner = console.NewSpinner("Waiting for workflow to be available...") - spinner.Start() + workflowFound, err := c.waitForWorkflowAvailability(ctx) + if err != nil { + return err + } + if !workflowFound { + return c.showWorkflowStatusUnavailableMessage() } - // Try a few times to see the workflow in status - var workflowFound bool - for i := range 5 { - // Wait 2 seconds before each check (including the first) - timer := time.NewTimer(2 * time.Second) - select { - case <-ctx.Done(): - timer.Stop() - if spinner != nil { - spinner.Stop() - } - return ctx.Err() - case <-timer.C: - // Continue with check - } + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Workflow is ready")) - workflowName := c.primaryWorkflowName() - if workflowName != "" { - if c.Verbose { - fmt.Fprintf(os.Stderr, "Checking workflow status (attempt %d/5) for: %s\n", i+1, workflowName) - } - // Check if workflow is in status - statuses, err := findWorkflowsByFilenamePattern(workflowName, c.RepoOverride, c.Verbose) - if err != nil { - if c.Verbose { - fmt.Fprintf(os.Stderr, "Status check error: %v\n", err) - } - } else if len(statuses) > 0 { - if c.Verbose { - fmt.Fprintf(os.Stderr, "Found %d workflow(s) matching pattern\n", len(statuses)) - } - workflowFound = true - break - } else if c.Verbose { - fmt.Fprintln(os.Stderr, "No workflows found matching pattern yet") - } - } + if !c.canOfferWorkflowRun() { + addInteractiveLog.Print("Workflow does not have workflow_dispatch trigger, skipping run offer") + c.showFinalInstructions() + return nil } - if spinner != nil { - spinner.Stop() + if isRunningInCodespace() { + return c.showCodespaceRunInstructions() } - if !workflowFound { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Could not verify workflow status.")) - fmt.Fprintf(os.Stderr, "You can check status with: %s status\n", string(constants.CLIExtensionPrefix)) + runNow, err := c.promptToRunWorkflowNow(ctx) + if err != nil { + return nil + } + if !runNow { c.showFinalInstructions() return nil } - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Workflow is ready")) + return c.runWorkflowAfterStatusCheck(ctx) +} - // Only offer to run if workflow has workflow_dispatch trigger - if c.addResult == nil || !c.addResult.HasWorkflowDispatch { - addInteractiveLog.Print("Workflow does not have workflow_dispatch trigger, skipping run offer") - c.showFinalInstructions() +func (c *AddInteractiveConfig) waitForWorkflowAvailability(ctx context.Context) (bool, error) { + spinner := c.startWorkflowStatusSpinner() + if spinner != nil { + defer spinner.Stop() + } + for i := range 5 { + found, err := c.checkWorkflowAvailabilityAttempt(ctx, i) + if err != nil || found { + return found, err + } + } + return false, nil +} + +func (c *AddInteractiveConfig) startWorkflowStatusSpinner() *console.SpinnerWrapper { + if c.Verbose { return nil } + spinner := console.NewSpinner("Waiting for workflow to be available...") + spinner.Start() + return spinner +} - // In Codespaces, don't offer to trigger - provide link to Actions page instead - if isRunningInCodespace() { - addInteractiveLog.Print("Running in Codespaces, skipping run offer and showing Actions link") - fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Running in GitHub Codespaces - please trigger the workflow manually from the Actions page")) - fmt.Fprintf(os.Stderr, "πŸ”— https://github.com/%s/actions\n", c.RepoOverride) - c.showFinalInstructions() +func (c *AddInteractiveConfig) checkWorkflowAvailabilityAttempt(ctx context.Context, attempt int) (bool, error) { + if err := waitForWorkflowStatusRetry(ctx); err != nil { + return false, err + } + workflowName := c.primaryWorkflowName() + if workflowName == "" { + return false, nil + } + if c.Verbose { + fmt.Fprintf(os.Stderr, "Checking workflow status (attempt %d/5) for: %s\n", attempt+1, workflowName) + } + statuses, err := findWorkflowsByFilenamePattern(workflowName, c.RepoOverride, c.Verbose) + return interpretWorkflowStatusCheck(statuses, err, c.Verbose) +} + +func waitForWorkflowStatusRetry(ctx context.Context) error { + timer := time.NewTimer(2 * time.Second) + defer timer.Stop() + select { + case <-ctx.Done(): + return ctx.Err() + case <-timer.C: return nil } +} + +func interpretWorkflowStatusCheck(statuses []WorkflowStatus, err error, verbose bool) (bool, error) { + if err != nil { + if verbose { + fmt.Fprintf(os.Stderr, "Status check error: %v\n", err) + } + return false, nil + } + if len(statuses) > 0 { + if verbose { + fmt.Fprintf(os.Stderr, "Found %d workflow(s) matching pattern\n", len(statuses)) + } + return true, nil + } + if verbose { + fmt.Fprintln(os.Stderr, "No workflows found matching pattern yet") + } + return false, nil +} + +func (c *AddInteractiveConfig) showWorkflowStatusUnavailableMessage() error { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Could not verify workflow status.")) + fmt.Fprintf(os.Stderr, "You can check status with: %s status\n", string(constants.CLIExtensionPrefix)) + c.showFinalInstructions() + return nil +} + +func (c *AddInteractiveConfig) canOfferWorkflowRun() bool { + return c.addResult != nil && c.addResult.HasWorkflowDispatch +} + +func (c *AddInteractiveConfig) showCodespaceRunInstructions() error { + addInteractiveLog.Print("Running in Codespaces, skipping run offer and showing Actions link") + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Running in GitHub Codespaces - please trigger the workflow manually from the Actions page")) + fmt.Fprintf(os.Stderr, "πŸ”— https://github.com/%s/actions\n", c.RepoOverride) + c.showFinalInstructions() + return nil +} - // Ask if user wants to run the workflow +func (c *AddInteractiveConfig) promptToRunWorkflowNow(ctx context.Context) (bool, error) { fmt.Fprintln(os.Stderr, "") - runNow := true // Default to yes + runNow := true form := console.NewConfirmForm( huh.NewConfirm(). Title("Would you like to run the workflow once now?"). @@ -107,62 +151,64 @@ func (c *AddInteractiveConfig) checkStatusAndOfferRun(ctx context.Context) error Negative("No, I'll run later"). Value(&runNow), ) - if err := form.RunWithContext(ctx); err != nil { - return nil // Not critical, just skip + return false, err } + return runNow, nil +} - if !runNow { +func (c *AddInteractiveConfig) runWorkflowAfterStatusCheck(ctx context.Context) error { + workflowName := c.primaryWorkflowName() + if workflowName == "" { c.showFinalInstructions() return nil } - // Run the workflow interactively (collects inputs if the workflow has them) - workflowName := c.primaryWorkflowName() - if workflowName != "" { - fmt.Fprintln(os.Stderr, "") - - // Pull the merged workflow files now that we know GitHub has processed the - // 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. - if !c.Verbose { - fmt.Fprintln(os.Stderr, "Updating local branch (this may take a few seconds)...") - } - if err := c.updateLocalBranch(); err != nil { - 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.") - } - if !c.Verbose { - fmt.Fprintln(os.Stderr, "Finished updating local branch.") - } - - if err := RunSpecificWorkflowInteractively(ctx, RunWorkflowOptions{ - WorkflowName: workflowName, - Verbose: c.Verbose, - EngineOverride: c.EngineOverride, - RepoOverride: c.RepoOverride, - }); err != nil { - fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("Failed to run workflow: %v", err))) - c.showFinalInstructions() - return nil - } - - // 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) - } + fmt.Fprintln(os.Stderr, "") + c.updateLocalBranchForRunOffer() + if err := RunSpecificWorkflowInteractively(ctx, RunWorkflowOptions{ + WorkflowName: workflowName, + Verbose: c.Verbose, + EngineOverride: c.EngineOverride, + RepoOverride: c.RepoOverride, + }); err != nil { + fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("Failed to run workflow: %v", err))) + c.showFinalInstructions() + return nil } + c.printLatestWorkflowRunURL(workflowName) c.showFinalInstructions() return nil } +func (c *AddInteractiveConfig) updateLocalBranchForRunOffer() { + // Pull the merged workflow files after GitHub has processed the merge to avoid a + // race where git fetch completes before the new workflow is available locally. + if !c.Verbose { + fmt.Fprintln(os.Stderr, "Updating local branch (this may take a few seconds)...") + } + if err := c.updateLocalBranch(); err != nil { + 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.") + } + if !c.Verbose { + fmt.Fprintln(os.Stderr, "Finished updating local branch.") + } +} + +func (c *AddInteractiveConfig) printLatestWorkflowRunURL(workflowName string) { + runInfo, err := getLatestWorkflowRunWithRetry(workflowName+".lock.yml", c.RepoOverride, c.Verbose) + if err != nil || runInfo.URL == "" { + return + } + 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) +} + // findWorkflowsByFilenamePattern is a helper to find workflows registered in GitHub by filename pattern. // The pattern is matched against the workflow filename (basename without extension) func findWorkflowsByFilenamePattern(pattern, repoOverride string, verbose bool) ([]WorkflowStatus, error) { diff --git a/pkg/cli/audit_diff.go b/pkg/cli/audit_diff.go index f002dee2cda..b1270533d4d 100644 --- a/pkg/cli/audit_diff.go +++ b/pkg/cli/audit_diff.go @@ -66,28 +66,39 @@ type FirewallDiffSummary struct { // Either analysis may be nil, indicating no firewall data for that run. func computeFirewallDiff(run1ID, run2ID int64, run1, run2 *FirewallAnalysis) *FirewallDiff { auditDiffLog.Printf("Computing firewall diff: run1=%d, run2=%d", run1ID, run2ID) - diff := &FirewallDiff{ - Run1ID: run1ID, - Run2ID: run2ID, + diff, run1Stats, run2Stats := newFirewallDiffCollector(run1ID, run2ID, run1, run2) + if len(run1Stats) == 0 && len(run2Stats) == 0 { + return diff.diff } + for _, domain := range collectFirewallDomains(run1Stats, run2Stats) { + diff.addDomain(domain, run1Stats, run2Stats) + } + diff.finalize() + auditDiffLog.Printf("Firewall diff complete: new=%d, removed=%d, status_changes=%d, volume_changes=%d, anomalies=%d", + len(diff.diff.NewDomains), len(diff.diff.RemovedDomains), len(diff.diff.StatusChanges), len(diff.diff.VolumeChanges), diff.anomalyCount) + return diff.diff +} - // Handle nil cases +type firewallDiffCollector struct { + diff *FirewallDiff + anomalyCount int +} + +func newFirewallDiffCollector(run1ID, run2ID int64, run1, run2 *FirewallAnalysis) (*firewallDiffCollector, map[string]DomainRequestStats, map[string]DomainRequestStats) { run1Stats := make(map[string]DomainRequestStats) run2Stats := make(map[string]DomainRequestStats) - if run1 != nil { run1Stats = run1.RequestsByDomain } if run2 != nil { run2Stats = run2.RequestsByDomain } + return &firewallDiffCollector{ + diff: &FirewallDiff{Run1ID: run1ID, Run2ID: run2ID}, + }, run1Stats, run2Stats +} - // If both are nil/empty, return empty diff - if len(run1Stats) == 0 && len(run2Stats) == 0 { - return diff - } - - // Collect all domains +func collectFirewallDomains(run1Stats, run2Stats map[string]DomainRequestStats) []string { allDomains := make(map[string]struct{}) for domain := range run1Stats { allDomains[domain] = struct{}{} @@ -95,120 +106,118 @@ func computeFirewallDiff(run1ID, run2ID int64, run1, run2 *FirewallAnalysis) *Fi for domain := range run2Stats { allDomains[domain] = struct{}{} } + return sliceutil.SortedKeys(allDomains) +} - // Sorted domain list for deterministic output - sortedDomains := sliceutil.SortedKeys(allDomains) +func (c *firewallDiffCollector) addDomain(domain string, run1Stats, run2Stats map[string]DomainRequestStats) { + stats1, inRun1 := run1Stats[domain] + stats2, inRun2 := run2Stats[domain] + switch { + case !inRun1 && inRun2: + c.addNewDomain(domain, stats2) + case inRun1 && !inRun2: + c.addRemovedDomain(domain, stats1) + default: + c.addExistingDomain(domain, stats1, stats2) + } +} - anomalyCount := 0 +func (c *firewallDiffCollector) addNewDomain(domain string, stats DomainRequestStats) { + entry := DomainDiffEntry{ + Domain: domain, + DiffEntryBase: DiffEntryBase{Status: "new"}, + Run2Allowed: stats.Allowed, + Run2Blocked: stats.Blocked, + Run2Status: classifyFirewallDomainStatus(stats), + } + if stats.Blocked > 0 { + c.markAnomaly(&entry, "new denied domain") + } + c.diff.NewDomains = append(c.diff.NewDomains, entry) +} - for _, domain := range sortedDomains { - stats1, inRun1 := run1Stats[domain] - stats2, inRun2 := run2Stats[domain] +func (c *firewallDiffCollector) addRemovedDomain(domain string, stats DomainRequestStats) { + entry := DomainDiffEntry{ + Domain: domain, + DiffEntryBase: DiffEntryBase{Status: "removed"}, + Run1Allowed: stats.Allowed, + Run1Blocked: stats.Blocked, + Run1Status: classifyFirewallDomainStatus(stats), + } + if stats.Blocked > 0 { + c.markAnomaly(&entry, "denied in base run β€” absent from comparison run") + } + c.diff.RemovedDomains = append(c.diff.RemovedDomains, entry) +} - if !inRun1 && inRun2 { - // New domain in run 2 - entry := DomainDiffEntry{ - Domain: domain, - DiffEntryBase: DiffEntryBase{Status: "new"}, - Run2Allowed: stats2.Allowed, - Run2Blocked: stats2.Blocked, - Run2Status: classifyFirewallDomainStatus(stats2), - } - // Anomaly: new denied domain - if stats2.Blocked > 0 { - entry.IsAnomaly = true - entry.AnomalyNote = "new denied domain" - anomalyCount++ - } - diff.NewDomains = append(diff.NewDomains, entry) - } else if inRun1 && !inRun2 { - // Removed domain - entry := DomainDiffEntry{ - Domain: domain, - DiffEntryBase: DiffEntryBase{Status: "removed"}, - Run1Allowed: stats1.Allowed, - Run1Blocked: stats1.Blocked, - Run1Status: classifyFirewallDomainStatus(stats1), - } - // Anomaly: the removed domain was denied in the base run. This indicates a - // transient firewall block that prevented the agent from reaching an MCP server - // (e.g. awmg-mcpg:8080) β€” even though the domain is absent from the comparison - // run (and therefore looks "normal"), its prior denial is worth surfacing so - // post-completion relaunch failures are detectable in audit diffs. - if stats1.Blocked > 0 { - entry.IsAnomaly = true - entry.AnomalyNote = "denied in base run β€” absent from comparison run" - anomalyCount++ - } - diff.RemovedDomains = append(diff.RemovedDomains, entry) - } else { - // Domain exists in both runs - check for changes - status1 := classifyFirewallDomainStatus(stats1) - status2 := classifyFirewallDomainStatus(stats2) - - if status1 != status2 { - // Status changed - entry := DomainDiffEntry{ - Domain: domain, - DiffEntryBase: DiffEntryBase{Status: "status_changed"}, - Run1Allowed: stats1.Allowed, - Run1Blocked: stats1.Blocked, - Run2Allowed: stats2.Allowed, - Run2Blocked: stats2.Blocked, - Run1Status: status1, - Run2Status: status2, - } - // Anomaly: previously denied, now allowed - if status1 == "denied" && status2 == "allowed" { - entry.IsAnomaly = true - entry.AnomalyNote = "previously denied, now allowed" - anomalyCount++ - } - // Anomaly: previously allowed, now denied - if status1 == "allowed" && status2 == "denied" { - entry.IsAnomaly = true - entry.AnomalyNote = "previously allowed, now denied" - anomalyCount++ - } - diff.StatusChanges = append(diff.StatusChanges, entry) - } else { - // Check for significant volume changes (>100% threshold) - total1 := stats1.Allowed + stats1.Blocked - total2 := stats2.Allowed + stats2.Blocked - - if total1 > 0 { - pctChange := (float64(total2-total1) / float64(total1)) * 100 - if math.Abs(pctChange) > volumeChangeThresholdPercent { - entry := DomainDiffEntry{ - Domain: domain, - DiffEntryBase: DiffEntryBase{Status: "volume_changed"}, - Run1Allowed: stats1.Allowed, - Run1Blocked: stats1.Blocked, - Run2Allowed: stats2.Allowed, - Run2Blocked: stats2.Blocked, - Run1Status: status1, - Run2Status: status2, - VolumeChange: formatVolumeChange(total1, total2), - } - diff.VolumeChanges = append(diff.VolumeChanges, entry) - } - } - } - } +func (c *firewallDiffCollector) addExistingDomain(domain string, stats1, stats2 DomainRequestStats) { + status1 := classifyFirewallDomainStatus(stats1) + status2 := classifyFirewallDomainStatus(stats2) + if status1 != status2 { + c.addStatusChange(domain, stats1, stats2, status1, status2) + return } + c.addVolumeChange(domain, stats1, stats2, status1, status2) +} - diff.Summary = FirewallDiffSummary{ - NewDomainCount: len(diff.NewDomains), - RemovedDomainCount: len(diff.RemovedDomains), - StatusChangeCount: len(diff.StatusChanges), - VolumeChangeCount: len(diff.VolumeChanges), - HasAnomalies: anomalyCount > 0, - AnomalyCount: anomalyCount, +func (c *firewallDiffCollector) addStatusChange(domain string, stats1, stats2 DomainRequestStats, status1, status2 string) { + entry := DomainDiffEntry{ + Domain: domain, + DiffEntryBase: DiffEntryBase{Status: "status_changed"}, + Run1Allowed: stats1.Allowed, + Run1Blocked: stats1.Blocked, + Run2Allowed: stats2.Allowed, + Run2Blocked: stats2.Blocked, + Run1Status: status1, + Run2Status: status2, + } + if status1 == "denied" && status2 == "allowed" { + c.markAnomaly(&entry, "previously denied, now allowed") + } + if status1 == "allowed" && status2 == "denied" { + c.markAnomaly(&entry, "previously allowed, now denied") } + c.diff.StatusChanges = append(c.diff.StatusChanges, entry) +} - auditDiffLog.Printf("Firewall diff complete: new=%d, removed=%d, status_changes=%d, volume_changes=%d, anomalies=%d", - len(diff.NewDomains), len(diff.RemovedDomains), len(diff.StatusChanges), len(diff.VolumeChanges), anomalyCount) - return diff +func (c *firewallDiffCollector) addVolumeChange(domain string, stats1, stats2 DomainRequestStats, status1, status2 string) { + total1 := stats1.Allowed + stats1.Blocked + total2 := stats2.Allowed + stats2.Blocked + if total1 == 0 { + return + } + pctChange := (float64(total2-total1) / float64(total1)) * 100 + if math.Abs(pctChange) <= volumeChangeThresholdPercent { + return + } + c.diff.VolumeChanges = append(c.diff.VolumeChanges, DomainDiffEntry{ + Domain: domain, + DiffEntryBase: DiffEntryBase{Status: "volume_changed"}, + Run1Allowed: stats1.Allowed, + Run1Blocked: stats1.Blocked, + Run2Allowed: stats2.Allowed, + Run2Blocked: stats2.Blocked, + Run1Status: status1, + Run2Status: status2, + VolumeChange: formatVolumeChange(total1, total2), + }) +} + +func (c *firewallDiffCollector) markAnomaly(entry *DomainDiffEntry, note string) { + entry.IsAnomaly = true + entry.AnomalyNote = note + c.anomalyCount++ +} + +func (c *firewallDiffCollector) finalize() { + c.diff.Summary = FirewallDiffSummary{ + NewDomainCount: len(c.diff.NewDomains), + RemovedDomainCount: len(c.diff.RemovedDomains), + StatusChangeCount: len(c.diff.StatusChanges), + VolumeChangeCount: len(c.diff.VolumeChanges), + HasAnomalies: c.anomalyCount > 0, + AnomalyCount: c.anomalyCount, + } } // classifyFirewallDomainStatus returns "allowed", "denied", or "mixed" based on request stats diff --git a/pkg/workflow/antigravity_engine.go b/pkg/workflow/antigravity_engine.go index a5aaa489fd7..43e418b2478 100644 --- a/pkg/workflow/antigravity_engine.go +++ b/pkg/workflow/antigravity_engine.go @@ -155,206 +155,161 @@ func (e *AntigravityEngine) GetPreBundleSteps(workflowData *WorkflowData) []GitH func (e *AntigravityEngine) GetExecutionSteps(workflowData *WorkflowData, logFile string) []GitHubActionStep { antigravityLog.Printf("Generating execution steps for Antigravity engine: workflow=%s, firewall=%v", workflowData.Name, isFirewallEnabled(workflowData)) - var steps []GitHubActionStep - - // Write .antigravity/settings.json with context.includeDirectories and tools.core. - // This step runs after the MCP gateway setup (which may have written mcpServers config) - // and merges the context/tools settings into any existing settings.json. - settingsStep := e.generateAntigravitySettingsStep(workflowData) - steps = append(steps, settingsStep) - - // Build agy CLI arguments based on configuration - var agyArgs []string - - // Model is passed via the native ANTIGRAVITY_MODEL environment variable only when explicitly - // configured. When not configured, the Antigravity CLI uses its built-in default model. - // This avoids embedding the value directly in the shell command (which fails template injection - // validation for GitHub Actions expressions like ${{ inputs.model }}). - modelConfigured := workflowData.Model != "" - - // Antigravity CLI reads MCP config from .antigravity/settings.json (project-level) - // The conversion script (convert_gateway_config_antigravity.sh) writes settings.json - // during the MCP setup step, so no --mcp-config flag is needed here. - - // Auto-approve all tool executions so non-interactive CI runs don't block on permission prompts. - // agy does not support the Gemini-style --yolo/--skip-trust flags. - // This flag grants broad tool permission inside the workflow sandbox, so it is only used in AWF-managed runs. - agyArgs = append(agyArgs, "--dangerously-skip-permissions") - - // Note: the --prompt argument is appended raw after shellJoinArgs below because it contains - // a shell command substitution ("$(cat ...)") that must NOT go through shellEscapeArg β€” - // single-quoting it would prevent shell expansion at runtime. - - // Build the command - commandName := "agy" - if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" { - commandName = workflowData.EngineConfig.Command + command, firewallEnabled, modelConfigured := e.buildAntigravityExecutionCommand(workflowData, logFile) + env := e.buildAntigravityExecutionEnv(workflowData, firewallEnabled, modelConfigured) + step := e.buildAntigravityExecutionStep(workflowData, command, env) + return []GitHubActionStep{ + e.generateAntigravitySettingsStep(workflowData), + step, } +} - // Append the prompt arg raw (not through shellJoinArgs) to preserve shell expansion - agyCommand := fmt.Sprintf(`%s %s --prompt "$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`, commandName, shellJoinArgs(agyArgs)) - agyCommand = getWorkspaceCommandPrefixFor(workflowData.EngineConfig) + agyCommand - - // Build the full command with AWF wrapping if enabled - var command string +func (e *AntigravityEngine) buildAntigravityExecutionCommand(workflowData *WorkflowData, logFile string) (string, bool, bool) { + agyCommand, modelConfigured := e.buildAntigravityCLICommand(workflowData) firewallEnabled := isFirewallEnabled(workflowData) if firewallEnabled { - // Get allowed domains: prefer the pre-warmed cache on WorkflowData to avoid - // re-running the expensive map+sort operation. - var allowedDomains string - if workflowData.CachedAllowedDomainsComputed { - allowedDomains = workflowData.CachedAllowedDomainsStr - } else { - allowedDomains = GetAllowedDomainsForEngine(constants.AntigravityEngine, - workflowData.NetworkPermissions, - workflowData.Tools, - workflowData.Runtimes, - ) - } - // Add GHES/custom API target domains to the firewall allow-list when engine.api-target is set - if workflowData.EngineConfig != nil && workflowData.EngineConfig.APITarget != "" { - allowedDomains = mergeAPITargetDomains(allowedDomains, workflowData.EngineConfig.APITarget) - } - - npmPathSetup := GetNpmBinPathSetup() - agyCommandWithPath := fmt.Sprintf("%s && %s", npmPathSetup, agyCommand) - // Add MCP CLI bin directory to PATH when cli-proxy is enabled - if mcpCLIPath := GetMCPCLIPathSetup(workflowData); mcpCLIPath != "" { - agyCommandWithPath = fmt.Sprintf("%s && %s", mcpCLIPath, agyCommandWithPath) - } - - command = BuildAWFCommand(AWFCommandConfig{ - EngineName: "antigravity", - EngineCommand: agyCommandWithPath, - LogFile: logFile, - WorkflowData: workflowData, - UsesTTY: false, - AllowedDomains: allowedDomains, - // Create the agent step summary file before AWF starts so it is accessible - // inside the sandbox. The agent writes its step summary content here, and the - // file is appended to $GITHUB_STEP_SUMMARY after secret redaction. - PathSetup: "touch " + AgentStepSummaryPath, - // Exclude every env var whose step-env value is a secret so the agent - // cannot read raw token values via bash tools (env / printenv). - ExcludeEnvVarNames: ComputeAWFExcludeEnvVarNames(workflowData, []string{"ANTIGRAVITY_API_KEY", "GEMINI_API_KEY"}), - }) - } else { - command = fmt.Sprintf(`set -o pipefail + return e.buildAntigravityAWFCommand(workflowData, logFile, agyCommand), true, modelConfigured + } + command := fmt.Sprintf(`set -o pipefail printf '%%s' "$(date +%%s%%3N)" > %s touch %s (umask 177 && touch %s) %s 2>&1 | tee -a %s`, AgentCLIStartMsPath, AgentStepSummaryPath, logFile, agyCommand, logFile) + return command, false, modelConfigured +} + +func (e *AntigravityEngine) buildAntigravityCLICommand(workflowData *WorkflowData) (string, bool) { + commandName := "agy" + if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" { + commandName = workflowData.EngineConfig.Command } + modelConfigured := workflowData.Model != "" + agyCommand := fmt.Sprintf(`%s %s --prompt "$(cat /tmp/gh-aw/aw-prompts/prompt.txt)"`, + commandName, + shellJoinArgs([]string{"--dangerously-skip-permissions"}), + ) + return getWorkspaceCommandPrefixFor(workflowData.EngineConfig) + agyCommand, modelConfigured +} - // Build environment variables - env := map[string]string{ - "ANTIGRAVITY_API_KEY": "${{ secrets.ANTIGRAVITY_API_KEY }}", - "GH_AW_PROMPT": constants.AwPromptsFile, - // Tag the step as a GitHub AW agentic execution for discoverability by agents - "GITHUB_AW": "true", - "GITHUB_WORKSPACE": "${{ github.workspace }}", - "RUNNER_TEMP": "${{ runner.temp }}", - // Override GITHUB_STEP_SUMMARY with a path that exists inside the sandbox. - // The runner's original path is unreachable within the AWF isolated filesystem; - // we create this file before the agent starts and append it to the real - // $GITHUB_STEP_SUMMARY after secret redaction. - "GITHUB_STEP_SUMMARY": AgentStepSummaryPath, - // Enable verbose debug logging from Antigravity CLI for better diagnostics. - // Antigravity CLI uses the npm 'debug' package, and 'antigravity-cli:*' enables all - // internal Antigravity CLI debug channels (see: https://antigravity.google/docs/cli-overview). - // Non-JSON debug lines are gracefully skipped by ParseLogMetrics. - "DEBUG": "antigravity-cli:*", - // Trust the workspace to prevent Antigravity CLI v1.x from overriding --yolo to default - // approval mode when the workspace is untrusted, which causes exit code 55. - "ANTIGRAVITY_CLI_TRUST_WORKSPACE": "true", +func (e *AntigravityEngine) buildAntigravityAWFCommand(workflowData *WorkflowData, logFile, agyCommand string) string { + agyCommandWithPath := fmt.Sprintf("%s && %s", GetNpmBinPathSetup(), agyCommand) + if mcpCLIPath := GetMCPCLIPathSetup(workflowData); mcpCLIPath != "" { + agyCommandWithPath = fmt.Sprintf("%s && %s", mcpCLIPath, agyCommandWithPath) + } + return BuildAWFCommand(AWFCommandConfig{ + EngineName: "antigravity", + EngineCommand: agyCommandWithPath, + LogFile: logFile, + WorkflowData: workflowData, + UsesTTY: false, + AllowedDomains: e.resolveAntigravityAllowedDomains(workflowData), + PathSetup: "touch " + AgentStepSummaryPath, + ExcludeEnvVarNames: ComputeAWFExcludeEnvVarNames(workflowData, []string{"ANTIGRAVITY_API_KEY", "GEMINI_API_KEY"}), + }) +} + +func (e *AntigravityEngine) resolveAntigravityAllowedDomains(workflowData *WorkflowData) string { + apiTarget := "" + if workflowData.EngineConfig != nil { + apiTarget = workflowData.EngineConfig.APITarget + } + if workflowData.CachedAllowedDomainsComputed { + return mergeAPITargetDomains(workflowData.CachedAllowedDomainsStr, apiTarget) + } + allowedDomains := GetAllowedDomainsForEngine(constants.AntigravityEngine, + workflowData.NetworkPermissions, + workflowData.Tools, + workflowData.Runtimes, + ) + if apiTarget != "" { + allowedDomains = mergeAPITargetDomains(allowedDomains, apiTarget) } + return allowedDomains +} + +func (e *AntigravityEngine) buildAntigravityExecutionEnv(workflowData *WorkflowData, firewallEnabled, modelConfigured bool) map[string]string { + env := buildAntigravityBaseEnv() injectWorkflowCallNetworkAllowedEnv(env, workflowData) - // Indicate the phase: "agent" for the main run, "detection" for threat detection, - // and "evals" for the eval harness execution. - // Include the compiler version so agents can identify which gh-aw version generated the workflow + applyAntigravityPhaseEnv(env, workflowData) + applyAntigravityMCPAndFirewallEnv(env, workflowData, firewallEnabled) + applySafeOutputEnvToMap(env, workflowData) + applyTraceContextEnvToMap(env) + applyAntigravityMaxTurnsEnv(env, workflowData) + applyAntigravityCustomEnv(env, workflowData, modelConfigured) + ensureAntigravityGeminiAPIKey(env) + return env +} + +func buildAntigravityBaseEnv() map[string]string { + return map[string]string{ + "ANTIGRAVITY_API_KEY": "${{ secrets.ANTIGRAVITY_API_KEY }}", + "GH_AW_PROMPT": constants.AwPromptsFile, + "GITHUB_AW": "true", + "GITHUB_WORKSPACE": "${{ github.workspace }}", + "RUNNER_TEMP": "${{ runner.temp }}", + "GITHUB_STEP_SUMMARY": AgentStepSummaryPath, + "DEBUG": "antigravity-cli:*", + "ANTIGRAVITY_CLI_TRUST_WORKSPACE": "true", + } +} + +func applyAntigravityPhaseEnv(env map[string]string, workflowData *WorkflowData) { env["GH_AW_PHASE"] = workflowRunPhase(workflowData) if IsRelease() { env["GH_AW_VERSION"] = GetVersion() - } else { - env["GH_AW_VERSION"] = "dev" + return } + env["GH_AW_VERSION"] = "dev" +} - // Add MCP config env var if needed (points to .antigravity/settings.json for Antigravity) +func applyAntigravityMCPAndFirewallEnv(env map[string]string, workflowData *WorkflowData, firewallEnabled bool) { if HasMCPServers(workflowData) { env["GH_AW_MCP_CONFIG"] = "${{ github.workspace }}/.antigravity/settings.json" } - - // When the firewall (AWF) is enabled with --enable-api-proxy, point Antigravity CLI at the - // LLM gateway sidecar instead of the real googleapis.com endpoint. - if firewallEnabled { - env["ANTIGRAVITY_API_BASE_URL"] = fmt.Sprintf("http://host.docker.internal:%d", constants.AntigravityLLMGatewayPort) - - // Set git identity environment variables so the first git commit succeeds inside the - // container. AWF's --env-all forwards these to the container, ensuring git does not - // rely on the host-side ~/.gitconfig which is not visible in the sandbox. - maps.Copy(env, getGitIdentityEnvVars()) + if !firewallEnabled { + return } + env["ANTIGRAVITY_API_BASE_URL"] = fmt.Sprintf("http://host.docker.internal:%d", constants.AntigravityLLMGatewayPort) + maps.Copy(env, getGitIdentityEnvVars()) +} - // Add safe outputs env - applySafeOutputEnvToMap(env, workflowData) - - // Propagate W3C trace context so engine spans nest under the gh-aw.agent.setup span. - applyTraceContextEnvToMap(env) - +func applyAntigravityMaxTurnsEnv(env map[string]string, workflowData *WorkflowData) { if workflowData.EngineConfig != nil && workflowData.EngineConfig.MaxTurns != "" { env["GH_AW_MAX_TURNS"] = workflowData.EngineConfig.MaxTurns - } else { - env["GH_AW_MAX_TURNS"] = compilerenv.BuildDefaultMaxTurnsExpression() + return } + env["GH_AW_MAX_TURNS"] = compilerenv.BuildDefaultMaxTurnsExpression() +} - // Set the model environment variable only when explicitly configured. - // When model is configured, use the native ANTIGRAVITY_MODEL env var - the Antigravity CLI reads it - // directly, avoiding the need to embed the value in the shell command (which would fail - // template injection validation for GitHub Actions expressions like ${{ inputs.model }}). - // When model is not configured, let the Antigravity CLI use its built-in default model. +func applyAntigravityCustomEnv(env map[string]string, workflowData *WorkflowData, modelConfigured bool) { if modelConfigured { antigravityLog.Printf("Setting %s env var for model: %s", constants.AntigravityCLIModelEnvVar, workflowData.Model) env[constants.AntigravityCLIModelEnvVar] = workflowData.Model } - - // Add custom environment variables from engine config. - // This allows users to override the default engine token expression (e.g. - // ANTIGRAVITY_API_KEY: ${{ secrets.MY_ORG_ANTIGRAVITY_KEY }}) via engine.env. applyEngineCwdEnv(env, workflowData) if workflowData.EngineConfig != nil && len(workflowData.EngineConfig.Env) > 0 { maps.Copy(env, workflowData.EngineConfig.Env) } - - // Add custom environment variables from agent config - agentConfig := getAgentConfig(workflowData) - if agentConfig != nil && len(agentConfig.Env) > 0 { + if agentConfig := getAgentConfig(workflowData); agentConfig != nil && len(agentConfig.Env) > 0 { maps.Copy(env, agentConfig.Env) antigravityLog.Printf("Added %d custom env vars from agent config", len(agentConfig.Env)) } - // The Antigravity CLI and AWF's Gemini API proxy both rely on a Gemini provider key. - // Keep GEMINI_API_KEY aligned with the effective ANTIGRAVITY_API_KEY by default so the - // workflow can authenticate non-interactively without requiring users to duplicate secrets. - if _, hasGeminiKey := env["GEMINI_API_KEY"]; !hasGeminiKey { - env["GEMINI_API_KEY"] = env["ANTIGRAVITY_API_KEY"] +} + +func ensureAntigravityGeminiAPIKey(env map[string]string) { + if _, hasGeminiKey := env["GEMINI_API_KEY"]; hasGeminiKey { + return } + env["GEMINI_API_KEY"] = env["ANTIGRAVITY_API_KEY"] +} - // Generate the execution step +func (e *AntigravityEngine) buildAntigravityExecutionStep(workflowData *WorkflowData, command string, env map[string]string) GitHubActionStep { stepLines := []string{ " - name: Execute Antigravity CLI", " id: agentic_execution", } - - // Filter environment variables for security allowedSecrets := append([]string{"GEMINI_API_KEY"}, e.GetRequiredSecretNames(workflowData)...) filteredEnv := FilterEnvForSecrets(env, allowedSecrets) - - // Inject GH_TOKEN for CLI proxy (added after filtering since it uses a special - // fallback expression that is always allowed when cli-proxy is enabled) addCliProxyGHTokenToEnv(filteredEnv, workflowData) - - // Format step with command and env - stepLines = FormatStepWithCommandAndEnv(stepLines, command, filteredEnv) - - steps = append(steps, GitHubActionStep(stepLines)) - return steps + return GitHubActionStep(FormatStepWithCommandAndEnv(stepLines, command, filteredEnv)) } diff --git a/pkg/workflow/awf_helpers.go b/pkg/workflow/awf_helpers.go index 22e6a7ee1c8..2e0b6a04536 100644 --- a/pkg/workflow/awf_helpers.go +++ b/pkg/workflow/awf_helpers.go @@ -217,36 +217,9 @@ func buildWorkflowCallNetworkAllowedUpdateScript() (string, error) { shellEscapeArg(string(ecosystemJSON))), nil } -// BuildAWFCommand builds a complete AWF command with all arguments. -// This consolidates the AWF command building logic that was duplicated across -// Copilot, Claude, and Codex engines. -// -// Parameters: -// - config: AWF command configuration -// -// Returns: -// - string: Complete AWF command with arguments and wrapped engine command -func BuildAWFCommand(config AWFCommandConfig) string { - awfHelpersLog.Printf("Building AWF command for engine: %s", config.EngineName) - isArcDind := isArcDindTopology(config.WorkflowData) - - // Get AWF command prefix (custom or standard) - awfCommand := GetAWFCommandPrefix(config.WorkflowData) - - // Build AWF arguments. The returned list contains only args that are safe to pass - // through shellJoinArgs. Expandable-var args (--container-workdir "${GITHUB_WORKSPACE}" - // and --mount "${RUNNER_TEMP}/...") are appended raw below so that shell variable - // expansion is not suppressed by single-quoting. - awfArgs := BuildAWFArgs(config) - firewallConfig := getFirewallConfig(config.WorkflowData) - - // Auto-detect ARC/DinD split daemon topology at runtime: probe DOCKER_HOST for a - // tcp:// scheme and pass it through to AWF via --docker-host. - // All behaviors avoid requiring workflow-authored sandbox.agent.args for standard ARC DinD setups. - // When AWF also supports chroot config (v0.27.1+), the Python patch body is embedded inside - // the same if-block so the script only contains one DOCKER_HOST condition check. - arcDindPrefixProbe := "" - arcDindDockerHostProbe := fmt.Sprintf(`%s="" +func buildArcDindProbe(config AWFCommandConfig, firewallConfig *FirewallConfig) (string, string, string) { + prefixProbe := "" + dockerHostProbe := fmt.Sprintf(`%s="" if [[ "${DOCKER_HOST:-}" =~ %s ]]; then %s="${DOCKER_HOST}" fi`, @@ -254,30 +227,41 @@ fi`, awfArcDindDockerHostRegex, awfDockerHostVarName, ) - arcDindDockerHostRef := fmt.Sprintf("${%s:+--docker-host \"$%s\"}", awfDockerHostVarName, awfDockerHostVarName) - if awfSupportsDockerHostPathPrefix(firewallConfig) { - chrootPatchBody := "" - if awfSupportsChrootConfig(firewallConfig) { - if config.WorkflowData != nil && config.WorkflowData.IsDetectionRun { - chrootPatchBody = "\n" + buildArcDindChrootConfigPatchBodyBash() - } else { - chrootPatchBody = "\n" + buildArcDindChrootConfigPatchBody() - } + dockerHostRef := fmt.Sprintf("${%s:+--docker-host \"$%s\"}", awfDockerHostVarName, awfDockerHostVarName) + if !awfSupportsDockerHostPathPrefix(firewallConfig) { + return prefixProbe, dockerHostProbe, dockerHostRef + } + + chrootPatchBody := "" + if awfSupportsChrootConfig(firewallConfig) { + if config.WorkflowData != nil && config.WorkflowData.IsDetectionRun { + chrootPatchBody = "\n" + buildArcDindChrootConfigPatchBodyBash() + } else { + chrootPatchBody = "\n" + buildArcDindChrootConfigPatchBody() } - // NOTE: --docker-host-path-prefix is intentionally NOT passed. With sysroot-stage - // active, all bind-mount source paths are on the shared work volume and visible to - // the Docker daemon without translation. The prefix caused AWF to translate - // GITHUB_WORKSPACE to a non-existent path, resulting in an empty workspace (gh-aw#34896). - // The probe block is preserved for the chroot config patch which still requires the - // DOCKER_HOST guard. - if chrootPatchBody != "" { - arcDindPrefixProbe = fmt.Sprintf(`if [[ "${DOCKER_HOST:-}" =~ %s ]]; then%s + } + if chrootPatchBody != "" { + prefixProbe = fmt.Sprintf(`if [[ "${DOCKER_HOST:-}" =~ %s ]]; then%s fi`, - awfArcDindDockerHostRegex, - chrootPatchBody) - } + awfArcDindDockerHostRegex, + chrootPatchBody, + ) + } + if isArcDindTopology(config.WorkflowData) { + dockerHostProbe += fmt.Sprintf("\nmkdir -p \"%s\" \"%s\"", + awfArcDindHomePathExpr, + awfArcDindRootPathExpr+"/sandbox/agent", + ) + dockerHostProbe += fmt.Sprintf("\nif [ -d /tmp/gh-aw/aw-prompts ]; then cp -a /tmp/gh-aw/aw-prompts \"%s/aw-prompts\"; fi", + awfArcDindRootPathExpr, + ) } - toolCacheMountProbe := fmt.Sprintf(`%s="" + + return prefixProbe, dockerHostProbe, dockerHostRef +} + +func buildToolCacheMountProbe() (string, string) { + mountProbe := fmt.Sprintf(`%s="" GH_AW_TOOL_CACHE="${RUNNER_TOOL_CACHE:?RUNNER_TOOL_CACHE must be set}" if [ -d "$GH_AW_TOOL_CACHE" ]; then if [[ "$GH_AW_TOOL_CACHE" != /opt/* ]]; then @@ -287,11 +271,11 @@ fi`, awfToolCacheMountVarName, awfToolCacheMountVarName, ) - toolCacheMountRef := fmt.Sprintf("${%s:+--mount \"$%s\"}", awfToolCacheMountVarName, awfToolCacheMountVarName) + mountRef := fmt.Sprintf("${%s:+--mount \"$%s\"}", awfToolCacheMountVarName, awfToolCacheMountVarName) + return mountProbe, mountRef +} - // Build the expandable args string for args that need shell variable expansion. - // These MUST be appended as raw (unescaped) strings because single-quoting would - // prevent the runner's shell from expanding ${GITHUB_WORKSPACE} and ${RUNNER_TEMP}. +func buildExpandableAWFArgs(isArcDind bool, workflowData *WorkflowData) string { ghAwDir := constants.GhAwRootDirShell expandableArgs := fmt.Sprintf( `--container-workdir "${GITHUB_WORKSPACE}" --mount "%s:%s:ro" --mount "%s:/host%s:ro"`, @@ -303,497 +287,335 @@ fi`, awfArcDindHomePathExpr, awfArcDindHomePathExpr, awfArcDindRootPathExpr+"/sandbox/agent", awfArcDindRootPathExpr+"/sandbox/agent", ) - // Explicitly mount the workspace so AWF can see it without path-prefix translation. - // GITHUB_WORKSPACE is on the shared work volume, so the Docker daemon can access it. expandableArgs += ` --mount "${GITHUB_WORKSPACE}:${GITHUB_WORKSPACE}:rw"` - // Pre-create the rw mount source directories. AWF validates that mount source - // paths exist before starting containers, so these must be created on the host - // before the AWF invocation. The parent ${RUNNER_TEMP}/gh-aw/ already exists - // (created by actions/setup), but the subdirectories may not. - arcDindDockerHostProbe += fmt.Sprintf("\nmkdir -p \"%s\" \"%s\"", - awfArcDindHomePathExpr, - awfArcDindRootPathExpr+"/sandbox/agent", - ) - // Copy prompt files to daemon-visible path. On ARC/DinD, /tmp/gh-aw/ is NOT - // accessible to the Docker daemon. The activation job writes prompts to - // /tmp/gh-aw/aw-prompts/, so we copy them to ${RUNNER_TEMP}/gh-aw/aw-prompts/. - arcDindDockerHostProbe += fmt.Sprintf("\nif [ -d /tmp/gh-aw/aw-prompts ]; then cp -a /tmp/gh-aw/aw-prompts \"%s/aw-prompts\"; fi", - awfArcDindRootPathExpr, - ) } - - // Generate a JSON config file and reference it via --config "${RUNNER_TEMP}/gh-aw/awf-config.json". - // This replaces several verbose CLI flags (--allow-domains, --enable-api-proxy, --image-tag, - // API targets) with a structured JSON file that is easier to audit and extend. - // - // The config file is written at runtime (inside the run: step) immediately before the AWF - // invocation, using printf to a fixed path inside the pre-existing ${RUNNER_TEMP}/gh-aw/ - // directory that is already set up by actions/setup. - var configFileSetup string - awfConfigJSON, err := BuildAWFConfigJSON(config) - if err != nil { - awfHelpersLog.Printf("Warning: failed to build AWF config JSON: %v", err) - } else { - // When max-ai-credits is not set by frontmatter/imports, export a local shell - // variable (GH_AW_MAX_AI_CREDITS) holding a GitHub Actions runtime expression, - // then inject a reference to that variable (${GH_AW_MAX_AI_CREDITS}) into the - // "maxAiCredits" field of the apiProxy JSON object. GitHub Actions evaluates - // the ${{ }} expression before the shell runs, so the variable is set to the - // resolved integer by the time printf writes the config file. - // - // Standard agent runs use vars.GH_AW_DEFAULT_MAX_AI_CREDITS with built-in - // fallback 1000. Threat-detection runs use - // vars.GH_AW_DEFAULT_DETECTION_MAX_AI_CREDITS with built-in fallback 400. - // Evals runs use vars.GH_AW_DEFAULT_EVALS_MAX_AI_CREDITS with built-in - // fallback 400 to align with detection budgets. - // EngineConfig.MaxAICredits is 0 when no compile-time value was set - // (neither frontmatter nor detection-engine config provided one). - // In that case, emit a runtime expression that lets the org variable - // or the built-in default resolve the budget at action run time. - // For detection runs, use the detection-specific variable/fallback; - // for standard agent runs, use the main-agent variable/fallback. - var maxAICreditsExportLine string - if config.WorkflowData == nil || config.WorkflowData.EngineConfig == nil || config.WorkflowData.EngineConfig.MaxAICredits == 0 { - defaultMaxAICredits := strconv.FormatInt(constants.DefaultMaxAICredits, 10) - if config.WorkflowData != nil { - switch { - case config.WorkflowData.IsEvalsRun: - defaultMaxAICredits = strconv.FormatInt(constants.DefaultDetectionMaxAICredits, 10) - case config.WorkflowData.IsDetectionRun: - defaultMaxAICredits = strconv.FormatInt(constants.DefaultDetectionMaxAICredits, 10) - } - } - awfConfigJSON = injectMaxAICreditsExpression(awfConfigJSON, fmt.Sprintf("${%s}", awfMaxAICreditsVarName)) - if config.ResolveMaxAICreditsFromEnv { - maxAICreditsExportLine = fmt.Sprintf(`%s="${%s:-%s}"`, awfMaxAICreditsVarName, awfMaxAICreditsVarName, defaultMaxAICredits) - } else { - expr := compilerenv.BuildDefaultMaxAICreditsExpression(defaultMaxAICredits) - if config.WorkflowData != nil { - switch { - case config.WorkflowData.IsEvalsRun: - expr = compilerenv.BuildDefaultEvalsMaxAICreditsExpression(defaultMaxAICredits) - case config.WorkflowData.IsDetectionRun: - expr = compilerenv.BuildDefaultDetectionMaxAICreditsExpression(defaultMaxAICredits) - } - } - maxAICreditsExportLine = fmt.Sprintf(`%s="%s"`, awfMaxAICreditsVarName, expr) - } - awfHelpersLog.Printf("Injected maxAiCredits local var reference into AWF config JSON") - } - // Write the config JSON to ${RUNNER_TEMP}/gh-aw/awf-config.json before AWF runs. - // When the generated JSON contains compiler-owned runtime variables such as - // ${GH_AW_MAX_AI_CREDITS} or ${RUNNER_TEMP}, use shellEscapeArgWithVarsPreserved - // which always uses double-quote wrapping: it escapes bare $ signs (e.g. - // "$schema" β†’ "\$schema") while preserving both ${{ }} GitHub Actions expressions - // (e.g. in AllowedDomains) and approved shell variable references so bash expands - // them to runtime-resolved values. When no such variables are injected, - // shellEscapeArg handles escaping normally. - // Also copy it to /tmp/gh-aw/awf-config.json for the unified agent artifact upload. - var printfArg string - preservedVars := make([]string, 0, 2) - if maxAICreditsExportLine != "" { - preservedVars = append(preservedVars, awfMaxAICreditsVarName) - } - if strings.Contains(awfConfigJSON, awfArcDindRootPathExpr) { - preservedVars = append(preservedVars, "RUNNER_TEMP") - } - if len(preservedVars) > 0 { - printfArg = shellEscapeArgWithVarsPreserved(awfConfigJSON, preservedVars...) - } else { - printfArg = shellEscapeArg(awfConfigJSON) - } - // SC2016 ("Expressions don't expand in single quotes") is only triggered when - // printfArg is single-quoted (no runtime variables injected). Double-quoted args - // already escape bare $ signs as \$schema, so shellcheck does not warn there. - var printfLine string - if strings.HasPrefix(printfArg, "'") { - printfLine = "# shellcheck disable=SC2016\nprintf '%%s\\n' %s > %q" + if workflowData != nil && workflowData.SafeOutputs != nil && workflowData.SafeOutputs.UploadArtifact != nil { + expandableArgs += fmt.Sprintf(` --mount "%s:%s:rw"`, SafeOutputsUploadArtifactsDir, SafeOutputsUploadArtifactsDir) + awfHelpersLog.Print("Added read-write mount for upload_artifact staging directory") + } + if workflowData != nil && workflowData.ServicePortExpressions != "" { + agentCfg := getAgentConfig(workflowData) + if agentCfg != nil && agentCfg.LegacySecurity { + expandableArgs += fmt.Sprintf(` --allow-host-service-ports "%s"`, workflowData.ServicePortExpressions) + awfHelpersLog.Printf("Added --allow-host-service-ports with %s", workflowData.ServicePortExpressions) } else { - printfLine = "printf '%%s\\n' %s > %q" + awfHelpersLog.Print("Skipping --allow-host-service-ports: requires legacy-security mode") } - configFileSetup = fmt.Sprintf(printfLine, printfArg, awfConfigRuntimePathExpr) - if maxAICreditsExportLine != "" { - configFileSetup = maxAICreditsExportLine + "\n" + configFileSetup - } - if shouldUseWorkflowCallNetworkAllowedInput(config.WorkflowData) { - updateScript, updateErr := buildWorkflowCallNetworkAllowedUpdateScript() - if updateErr != nil { - awfHelpersLog.Printf("Warning: failed to build workflow_call network_allowed updater: %v", updateErr) - } else { - configFileSetup += "\n" + updateScript - } + } + return expandableArgs +} + +func defaultMaxAICredits(config AWFCommandConfig) string { + defaultValue := strconv.FormatInt(constants.DefaultMaxAICredits, 10) + if config.WorkflowData == nil { + return defaultValue + } + if config.WorkflowData.IsEvalsRun || config.WorkflowData.IsDetectionRun { + return strconv.FormatInt(constants.DefaultDetectionMaxAICredits, 10) + } + return defaultValue +} + +func buildMaxAICreditsExportLine(config AWFCommandConfig, awfConfigJSON string) (string, string) { + if config.WorkflowData != nil && config.WorkflowData.EngineConfig != nil && config.WorkflowData.EngineConfig.MaxAICredits != 0 { + return awfConfigJSON, "" + } + + defaultValue := defaultMaxAICredits(config) + awfConfigJSON = injectMaxAICreditsExpression(awfConfigJSON, fmt.Sprintf("${%s}", awfMaxAICreditsVarName)) + if config.ResolveMaxAICreditsFromEnv { + return awfConfigJSON, fmt.Sprintf(`%s="${%s:-%s}"`, awfMaxAICreditsVarName, awfMaxAICreditsVarName, defaultValue) + } + + expr := compilerenv.BuildDefaultMaxAICreditsExpression(defaultValue) + if config.WorkflowData != nil { + switch { + case config.WorkflowData.IsEvalsRun: + expr = compilerenv.BuildDefaultEvalsMaxAICreditsExpression(defaultValue) + case config.WorkflowData.IsDetectionRun: + expr = compilerenv.BuildDefaultDetectionMaxAICreditsExpression(defaultValue) } - configFileSetup += fmt.Sprintf("\ncp %q %s", awfConfigRuntimePathExpr, constants.AWFConfigFilePath) - // Add --config as the first expandable arg so it appears before --container-workdir. - expandableArgs = fmt.Sprintf("--config %q ", awfConfigRuntimePathExpr) + expandableArgs - awfHelpersLog.Print("Using AWF config file (--config flag)") - } - modelsJSONPathExport := buildModelsJSONPathExportScript(isArcDind) - - // When upload_artifact is configured, add a read-write mount for the staging directory - // so the model can copy files there from inside the container. The parent ${RUNNER_TEMP}/gh-aw - // is mounted :ro above; this child mount overrides access for the staging subdirectory only. - // The staging directory must already exist on the host (created in Generate Safe Outputs Config step). - if config.WorkflowData != nil && config.WorkflowData.SafeOutputs != nil && config.WorkflowData.SafeOutputs.UploadArtifact != nil { - stagingDir := SafeOutputsUploadArtifactsDir - expandableArgs += fmt.Sprintf(` --mount "%s:%s:rw"`, stagingDir, stagingDir) - awfHelpersLog.Print("Added read-write mount for upload_artifact staging directory") } + return awfConfigJSON, fmt.Sprintf(`%s="%s"`, awfMaxAICreditsVarName, expr) +} - // Add --allow-host-service-ports for services with port mappings. - // This flag requires --legacy-security since it grants host network access. - // This is appended as a raw (expandable) arg because the value contains - // ${{ job.services..ports[''] }} expressions that include single quotes. - // These expressions are resolved by the GitHub Actions runner before shell execution, - // so they must not be shell-escaped. - agentCfg := getAgentConfig(config.WorkflowData) - isLegacyMode := agentCfg != nil && agentCfg.LegacySecurity - if config.WorkflowData != nil && config.WorkflowData.ServicePortExpressions != "" && isLegacyMode { - expandableArgs += fmt.Sprintf(` --allow-host-service-ports "%s"`, config.WorkflowData.ServicePortExpressions) - awfHelpersLog.Printf("Added --allow-host-service-ports with %s", config.WorkflowData.ServicePortExpressions) - } else if config.WorkflowData != nil && config.WorkflowData.ServicePortExpressions != "" { - awfHelpersLog.Print("Skipping --allow-host-service-ports: requires legacy-security mode") +func buildConfigPrintfArg(awfConfigJSON, maxAICreditsExportLine string) string { + preservedVars := make([]string, 0, 2) + if maxAICreditsExportLine != "" { + preservedVars = append(preservedVars, awfMaxAICreditsVarName) + } + if strings.Contains(awfConfigJSON, awfArcDindRootPathExpr) { + preservedVars = append(preservedVars, "RUNNER_TEMP") } + if len(preservedVars) > 0 { + return shellEscapeArgWithVarsPreserved(awfConfigJSON, preservedVars...) + } + return shellEscapeArg(awfConfigJSON) +} - engineCommand := config.EngineCommand - if isArcDind { - engineCommand = rewriteArcDindEngineCommand(engineCommand) +func buildConfigFileSetup(config AWFCommandConfig, awfConfigJSON, expandableArgs string) (string, string, error) { + awfConfigJSON, maxAICreditsExportLine := buildMaxAICreditsExportLine(config, awfConfigJSON) + if maxAICreditsExportLine != "" { + awfHelpersLog.Printf("Injected maxAiCredits local var reference into AWF config JSON") } - // Wrap engine command in shell (command already includes any internal setup like npm PATH) - shellWrappedCommand := WrapCommandInShell(engineCommand) + printfArg := buildConfigPrintfArg(awfConfigJSON, maxAICreditsExportLine) + printfLine := "printf '%%s\\n' %s > %q" + if strings.HasPrefix(printfArg, "'") { + printfLine = "# shellcheck disable=SC2016\n" + printfLine + } + configFileSetup := fmt.Sprintf(printfLine, printfArg, awfConfigRuntimePathExpr) + if maxAICreditsExportLine != "" { + configFileSetup = maxAICreditsExportLine + "\n" + configFileSetup + } + if shouldUseWorkflowCallNetworkAllowedInput(config.WorkflowData) { + updateScript, err := buildWorkflowCallNetworkAllowedUpdateScript() + if err != nil { + awfHelpersLog.Printf("Warning: failed to build workflow_call network_allowed updater: %v", err) + } else { + configFileSetup += "\n" + updateScript + } + } + configFileSetup += fmt.Sprintf("\ncp %q %s", awfConfigRuntimePathExpr, constants.AWFConfigFilePath) + updatedArgs := fmt.Sprintf("--config %q ", awfConfigRuntimePathExpr) + expandableArgs + awfHelpersLog.Print("Using AWF config file (--config flag)") + return configFileSetup, updatedArgs, nil +} - // Pre-create the agent stdio log file with restrictive permissions (0600) before - // starting the AWF container. tee would otherwise create it with the default - // umask (0644), leaving secrets (e.g. MCP gateway tokens) world-readable on the - // runner host until the secret-redaction step runs. - preCreateLog := fmt.Sprintf("(umask 177 && touch %s)", shellEscapeArg(config.LogFile)) +type awfCommandScriptParts struct { + pathSetup string + configFileSetup string + modelsJSONPath string + dockerHostProbe string + prefixProbe string + toolCacheMount string + awfCommand string + expandableArgs string + toolCacheMountRef string + dockerHostRef string + awfArgs []string +} - // Capture the epoch-millisecond timestamp at the very start of the Execute Agent CLI - // step on the host, before the AWF container launches. sendJobConclusionSpan reads - // this file to set the dedicated gh-aw..agent span start time, which excludes - // pre-agent overhead such as workspace audit and CLI proxy startup. +func buildAWFCommandScript(config AWFCommandConfig, parts awfCommandScriptParts) string { + preCreateLog := fmt.Sprintf("(umask 177 && touch %s)", shellEscapeArg(config.LogFile)) writeAgentCLIStartMs := "printf '%s' \"$(date +%s%3N)\" > " + shellEscapeArg(AgentCLIStartMsPath) - - // Build the complete command with proper formatting. - // configFileSetup (if non-empty) writes the AWF config JSON immediately before the - // AWF invocation so the file is present when AWF parses --config. - // - // shellcheck directive rationale: - // - SC1003 is expected because this generated block intentionally contains GitHub - // expression literals (for example ${{ job.services..ports[''] }}) - // that include single quotes and must survive into runtime unchanged. - // - SC2086 is expected because a subset of AWF arguments are intentionally emitted - // as expandable shell fragments (for example ${GH_AW_TOOL_CACHE_MOUNT:+...} and - // ${GH_AW_DOCKER_HOST:+...}). These fragments are produced by trusted - // compiler-owned probes above and are not user-provided free-form shell input. - // - // We keep normal quoting for all user-controlled values via shellEscapeArg/shellJoinArgs - // and scope this suppression to the generated AWF invocation line only. - var command string - if config.PathSetup != "" && configFileSetup != "" { - command = fmt.Sprintf(`set -o pipefail -%s -%s -%s -%s -%s -%s -%s -%s -%s -%s %s %s %s %s \ - -- %s 2>&1 | tee -a %s`, - writeAgentCLIStartMs, - config.PathSetup, - preCreateLog, - configFileSetup, - modelsJSONPathExport, - arcDindDockerHostProbe, - arcDindPrefixProbe, - toolCacheMountProbe, - awfShellcheckDirective, - awfCommand, - expandableArgs, - toolCacheMountRef, - arcDindDockerHostRef, - shellJoinArgs(awfArgs), - shellWrappedCommand, - shellEscapeArg(config.LogFile)) - } else if config.PathSetup != "" { - // Include path setup before AWF command (runs on host before AWF) - command = fmt.Sprintf(`set -o pipefail -%s -%s -%s -%s -%s -%s -%s -%s -%s %s %s %s %s \ - -- %s 2>&1 | tee -a %s`, - writeAgentCLIStartMs, - config.PathSetup, - preCreateLog, - modelsJSONPathExport, - arcDindDockerHostProbe, - arcDindPrefixProbe, - toolCacheMountProbe, - awfShellcheckDirective, - awfCommand, - expandableArgs, - toolCacheMountRef, - arcDindDockerHostRef, - shellJoinArgs(awfArgs), - shellWrappedCommand, - shellEscapeArg(config.LogFile)) - } else if configFileSetup != "" { - command = fmt.Sprintf(`set -o pipefail -%s -%s -%s -%s -%s -%s -%s -%s -%s %s %s %s %s \ - -- %s 2>&1 | tee -a %s`, - writeAgentCLIStartMs, - preCreateLog, - configFileSetup, - modelsJSONPathExport, - arcDindDockerHostProbe, - arcDindPrefixProbe, - toolCacheMountProbe, - awfShellcheckDirective, - awfCommand, - expandableArgs, - toolCacheMountRef, - arcDindDockerHostRef, - shellJoinArgs(awfArgs), - shellWrappedCommand, - shellEscapeArg(config.LogFile)) - } else { - command = fmt.Sprintf(`set -o pipefail -%s -%s -%s -%s -%s -%s -%s -%s %s %s %s %s \ - -- %s 2>&1 | tee -a %s`, - writeAgentCLIStartMs, - preCreateLog, - modelsJSONPathExport, - arcDindDockerHostProbe, - arcDindPrefixProbe, - toolCacheMountProbe, - awfShellcheckDirective, - awfCommand, - expandableArgs, - toolCacheMountRef, - arcDindDockerHostRef, - shellJoinArgs(awfArgs), - shellWrappedCommand, - shellEscapeArg(config.LogFile)) + engineCommand := config.EngineCommand + if isArcDindTopology(config.WorkflowData) { + engineCommand = rewriteArcDindEngineCommand(engineCommand) } - awfHelpersLog.Print("Successfully built AWF command") - return command + lines := []string{"set -o pipefail", writeAgentCLIStartMs} + if parts.pathSetup != "" { + lines = append(lines, parts.pathSetup) + } + lines = append(lines, preCreateLog) + if parts.configFileSetup != "" { + lines = append(lines, parts.configFileSetup) + } + lines = append(lines, parts.modelsJSONPath, parts.dockerHostProbe, parts.prefixProbe, parts.toolCacheMount, awfShellcheckDirective) + lines = append(lines, fmt.Sprintf(`%s %s %s %s %s \ + -- %s 2>&1 | tee -a %s`, + parts.awfCommand, + parts.expandableArgs, + parts.toolCacheMountRef, + parts.dockerHostRef, + shellJoinArgs(parts.awfArgs), + WrapCommandInShell(engineCommand), + shellEscapeArg(config.LogFile), + )) + return strings.Join(lines, "\n") } -// BuildAWFArgs constructs common AWF arguments from configuration. -// This extracts the shared AWF argument building logic from engine implementations. -// -// The following flags are expressed in the generated JSON config file written by -// BuildAWFCommand and are therefore not emitted here: -// - --allow-domains / --block-domains β†’ network.allowDomains / network.blockDomains -// - --image-tag β†’ container.imageTag -// - --openai-api-target β†’ apiProxy.targets.openai.host -// - --anthropic-api-target β†’ apiProxy.targets.anthropic.host -// - --copilot-api-target β†’ apiProxy.targets.copilot.host -// - --gemini-api-target β†’ apiProxy.targets.gemini.host -// -// Note: --enable-api-proxy is deprecated in AWF v0.27.32+ (API proxy is always on). -// The apiProxy.enabled field is still emitted in the config file for backward compat. +// BuildAWFCommand builds a complete AWF command with all arguments. +// This consolidates the AWF command building logic that was duplicated across +// Copilot, Claude, and Codex engines. // // Parameters: // - config: AWF command configuration // // Returns: -// - []string: List of AWF arguments (safe args only; expandable-var args like -// --container-workdir and --mount are handled by BuildAWFCommand) -func BuildAWFArgs(config AWFCommandConfig) []string { - awfHelpersLog.Printf("Building AWF args for engine: %s", config.EngineName) - +// - string: Complete AWF command with arguments and wrapped engine command +func BuildAWFCommand(config AWFCommandConfig) string { + awfHelpersLog.Printf("Building AWF command for engine: %s", config.EngineName) + isArcDind := isArcDindTopology(config.WorkflowData) + awfCommand := GetAWFCommandPrefix(config.WorkflowData) + awfArgs := BuildAWFArgs(config) firewallConfig := getFirewallConfig(config.WorkflowData) - agentConfig := getAgentConfig(config.WorkflowData) - - var awfArgs []string + arcDindPrefixProbe, arcDindDockerHostProbe, arcDindDockerHostRef := buildArcDindProbe(config, firewallConfig) + toolCacheMountProbe, toolCacheMountRef := buildToolCacheMountProbe() + expandableArgs := buildExpandableAWFArgs(isArcDind, config.WorkflowData) + configFileSetup := "" + if awfConfigJSON, err := BuildAWFConfigJSON(config); err != nil { + awfHelpersLog.Printf("Warning: failed to build AWF config JSON: %v", err) + } else if setup, updatedArgs, err := buildConfigFileSetup(config, awfConfigJSON, expandableArgs); err != nil { + awfHelpersLog.Printf("Warning: failed to build AWF config setup: %v", err) + } else { + configFileSetup = setup + expandableArgs = updatedArgs + } + command := buildAWFCommandScript(config, awfCommandScriptParts{ + pathSetup: config.PathSetup, + configFileSetup: configFileSetup, + modelsJSONPath: buildModelsJSONPathExportScript(isArcDind), + dockerHostProbe: arcDindDockerHostProbe, + prefixProbe: arcDindPrefixProbe, + toolCacheMount: toolCacheMountProbe, + awfCommand: awfCommand, + expandableArgs: expandableArgs, + toolCacheMountRef: toolCacheMountRef, + dockerHostRef: arcDindDockerHostRef, + awfArgs: awfArgs, + }) + awfHelpersLog.Print("Successfully built AWF command") + return command +} - // Add TTY flag if needed (Claude requires this), except for docker-sbx where - // sbx exec --tty can terminate long-running Claude sessions prematurely. +func appendAWFContainerRuntimeArgs(awfArgs []string, config AWFCommandConfig, firewallConfig *FirewallConfig) []string { if config.UsesTTY && !isDockerSbxRuntime(config.WorkflowData) { awfArgs = append(awfArgs, "--tty") } - - // docker-sbx: tell AWF to launch the agent inside a Docker sbx microVM instead - // of as a standard Docker Compose service. Guard on the effective AWF version so - // older binaries do not receive an unknown flag. if isDockerSbxRuntime(config.WorkflowData) && awfSupportsContainerRuntime(firewallConfig) { awfArgs = append(awfArgs, "--container-runtime", "sbx") awfHelpersLog.Print("Added --container-runtime sbx for docker-sbx microVM runtime") } else if isDockerSbxRuntime(config.WorkflowData) { awfHelpersLog.Printf("Skipping --container-runtime sbx: AWF version %q is older than required minimum %s", getAWFImageTag(firewallConfig), constants.AWFContainerRuntimeMinVersion) } + return awfArgs +} - // Pass all environment variables to the container, but exclude every variable whose - // step-env value comes from a GitHub Actions secret. AWF's API proxy (--enable-api-proxy) - // handles authentication for these tokens transparently, so the container does not need - // the raw values. Excluding them via --exclude-env prevents a prompt-injected agent from - // exfiltrating tokens through bash tools such as `env` or `printenv`. - // The caller computes ExcludeEnvVarNames from ComputeAWFExcludeEnvVarNames() so that every - // secret-bearing variable is covered β€” not just a hardcoded subset. - // --exclude-env requires AWF v0.25.3+; skip the flags for workflows that pin an older version. +func appendAWFExcludeEnvArgs(awfArgs []string, excludeEnvVarNames []string, firewallConfig *FirewallConfig) []string { awfArgs = append(awfArgs, "--env-all") - if awfSupportsExcludeEnv(firewallConfig) { - // Sort for deterministic output in compiled lock files. - sortedExclude := make([]string, len(config.ExcludeEnvVarNames)) - copy(sortedExclude, config.ExcludeEnvVarNames) - sort.Strings(sortedExclude) - for _, excludedVar := range sortedExclude { - awfArgs = append(awfArgs, "--exclude-env", excludedVar) - } - } else { + if !awfSupportsExcludeEnv(firewallConfig) { awfHelpersLog.Printf("Skipping --exclude-env: AWF version %q is older than minimum %s", getAWFImageTag(firewallConfig), constants.AWFExcludeEnvMinVersion) + return awfArgs } + sortedExclude := make([]string, len(excludeEnvVarNames)) + copy(sortedExclude, excludeEnvVarNames) + sort.Strings(sortedExclude) + for _, excludedVar := range sortedExclude { + awfArgs = append(awfArgs, "--exclude-env", excludedVar) + } + return awfArgs +} - // Note: --container-workdir "${GITHUB_WORKSPACE}" and --mount "${RUNNER_TEMP}/gh-aw:..." - // are intentionally NOT added here. They contain shell variable references that require - // double-quote expansion. These args are appended raw in BuildAWFCommand to ensure - // ${GITHUB_WORKSPACE} and ${RUNNER_TEMP} are expanded by the runner's shell. - - // Add custom mounts from agent config if specified +func appendAWFMountAndLogArgs(awfArgs []string, workflowData *WorkflowData, agentConfig *AgentSandboxConfig, firewallConfig *FirewallConfig) []string { if agentConfig != nil && len(agentConfig.Mounts) > 0 { - // Sort mounts for consistent output sortedMounts := make([]string, len(agentConfig.Mounts)) copy(sortedMounts, agentConfig.Mounts) sort.Strings(sortedMounts) - for _, mount := range sortedMounts { awfArgs = append(awfArgs, "--mount", mount) } awfHelpersLog.Printf("Added %d custom mounts from agent config", len(sortedMounts)) } - - // Set log level awfLogLevel := string(constants.AWFDefaultLogLevel) if firewallConfig != nil && firewallConfig.LogLevel != "" { awfLogLevel = firewallConfig.LogLevel } awfArgs = append(awfArgs, "--log-level", awfLogLevel) - if isFeatureEnabled(constants.AwfDiagnosticLogsFeatureFlag, config.WorkflowData) { + if isFeatureEnabled(constants.AwfDiagnosticLogsFeatureFlag, workflowData) { awfArgs = append(awfArgs, "--diagnostic-logs") awfHelpersLog.Print("Added --diagnostic-logs because awf-diagnostic-logs feature flag is enabled") } + return awfArgs +} - // Legacy security mode: emit --legacy-security, --enable-host-access, and --allow-host-ports - isLegacy := agentConfig != nil && agentConfig.LegacySecurity - if isLegacy { - if awfSupportsLegacySecurity(firewallConfig) { - awfArgs = append(awfArgs, "--legacy-security") - awfHelpersLog.Print("Added --legacy-security (legacy-security: enable in frontmatter)") - } else { - // AWF versions older than v0.27.32 don't support --legacy-security; - // they run in legacy mode by default so the flag is unnecessary. - awfHelpersLog.Printf("Skipping --legacy-security: AWF version %q is older than minimum %s (legacy mode is the default for older versions)", getAWFImageTag(firewallConfig), constants.AWFLegacySecurityMinVersion) - } - - awfArgs = append(awfArgs, "--enable-host-access") - awfHelpersLog.Print("Added --enable-host-access for legacy security mode") - - if awfSupportsAllowHostPorts(firewallConfig) { - mcpGatewayPort := int(DefaultMCPGatewayPort) - if config.WorkflowData != nil && config.WorkflowData.SandboxConfig != nil && - config.WorkflowData.SandboxConfig.MCP != nil && config.WorkflowData.SandboxConfig.MCP.Port > 0 { - mcpGatewayPort = config.WorkflowData.SandboxConfig.MCP.Port - } - hostPorts := fmt.Sprintf("80,443,%d", mcpGatewayPort) - awfArgs = append(awfArgs, "--allow-host-ports", hostPorts) - awfHelpersLog.Printf("Added --allow-host-ports %s for legacy security mode", hostPorts) - } - } else { +func appendAWFLegacySecurityArgs(awfArgs []string, config AWFCommandConfig, agentConfig *AgentSandboxConfig, firewallConfig *FirewallConfig) []string { + if agentConfig == nil || !agentConfig.LegacySecurity { awfHelpersLog.Print("Strict security: skipping host-access flags (default)") + return awfArgs } + if awfSupportsLegacySecurity(firewallConfig) { + awfArgs = append(awfArgs, "--legacy-security") + awfHelpersLog.Print("Added --legacy-security (legacy-security: enable in frontmatter)") + } else { + awfHelpersLog.Printf("Skipping --legacy-security: AWF version %q is older than minimum %s (legacy mode is the default for older versions)", getAWFImageTag(firewallConfig), constants.AWFLegacySecurityMinVersion) + } + awfArgs = append(awfArgs, "--enable-host-access") + awfHelpersLog.Print("Added --enable-host-access for legacy security mode") + if awfSupportsAllowHostPorts(firewallConfig) { + mcpGatewayPort := int(DefaultMCPGatewayPort) + if config.WorkflowData != nil && config.WorkflowData.SandboxConfig != nil && config.WorkflowData.SandboxConfig.MCP != nil && config.WorkflowData.SandboxConfig.MCP.Port > 0 { + mcpGatewayPort = config.WorkflowData.SandboxConfig.MCP.Port + } + hostPorts := fmt.Sprintf("80,443,%d", mcpGatewayPort) + awfArgs = append(awfArgs, "--allow-host-ports", hostPorts) + awfHelpersLog.Printf("Added --allow-host-ports %s for legacy security mode", hostPorts) + } + return awfArgs +} - // Skip pulling images since they are pre-downloaded - awfArgs = append(awfArgs, "--skip-pull") - awfHelpersLog.Print("Using --skip-pull since images are pre-downloaded") - - // Enable CLI proxy sidecar when GitHub mode is gh-proxy. - // Start the difc-proxy on the host and tell AWF where to connect - // (firewall v0.25.17+). - if isGitHubCLIModeEnabled(config.WorkflowData) { +func appendAWFCliProxyAndBasePathArgs(awfArgs []string, workflowData *WorkflowData, firewallConfig *FirewallConfig) []string { + if isGitHubCLIModeEnabled(workflowData) { if awfSupportsCliProxy(firewallConfig) { difcProxyHost := "host.docker.internal:18443" - if isAWFNetworkIsolationEnabled(config.WorkflowData) { + if isAWFNetworkIsolationEnabled(workflowData) { difcProxyHost = "awmg-cli-proxy:18443" } - awfArgs = append(awfArgs, "--difc-proxy-host", difcProxyHost) - awfArgs = append(awfArgs, "--difc-proxy-ca-cert", constants.TmpDIFCProxyTLSCACert) + awfArgs = append(awfArgs, "--difc-proxy-host", difcProxyHost, "--difc-proxy-ca-cert", constants.TmpDIFCProxyTLSCACert) awfHelpersLog.Print("Added --difc-proxy-host and --difc-proxy-ca-cert for CLI proxy sidecar") } else { awfHelpersLog.Printf("Skipping CLI proxy flags: AWF version %q is older than minimum %s", getAWFImageTag(firewallConfig), constants.AWFCliProxyMinVersion) } } - - // Pass base path if URL contains a path component - // This is required for endpoints with path prefixes (e.g., Databricks /serving-endpoints, - // Azure OpenAI /openai/deployments/, corporate LLM routers with path-based routing) - // Base paths remain as CLI flags β€” they are not yet represented in the config file schema. - openaiBasePath := extractAPIBasePath(config.WorkflowData, "OPENAI_BASE_URL") - if openaiBasePath != "" { - awfArgs = append(awfArgs, "--openai-api-base-path", openaiBasePath) - awfHelpersLog.Printf("Added --openai-api-base-path=%s", openaiBasePath) - } - - anthropicBasePath := extractAPIBasePath(config.WorkflowData, "ANTHROPIC_BASE_URL") - if anthropicBasePath != "" { - awfArgs = append(awfArgs, "--anthropic-api-base-path", anthropicBasePath) - awfHelpersLog.Printf("Added --anthropic-api-base-path=%s", anthropicBasePath) - } - - geminiBasePath := extractAPIBasePath(config.WorkflowData, "GEMINI_API_BASE_URL") - if geminiBasePath != "" { - awfArgs = append(awfArgs, "--gemini-api-base-path", geminiBasePath) - awfHelpersLog.Printf("Added --gemini-api-base-path=%s", geminiBasePath) + for _, spec := range []struct{ envVar, flag string }{{"OPENAI_BASE_URL", "--openai-api-base-path"}, {"ANTHROPIC_BASE_URL", "--anthropic-api-base-path"}, {"GEMINI_API_BASE_URL", "--gemini-api-base-path"}} { + if basePath := extractAPIBasePath(workflowData, spec.envVar); basePath != "" { + awfArgs = append(awfArgs, spec.flag, basePath) + awfHelpersLog.Printf("Added %s=%s", spec.flag, basePath) + } } + return awfArgs +} - // Add SSL Bump support for HTTPS content inspection (v0.9.0+) - sslBumpArgs := getSSLBumpArgs(firewallConfig) - awfArgs = append(awfArgs, sslBumpArgs...) - - // Add custom args if specified in firewall config +func appendAWFCustomArgs(awfArgs []string, agentConfig *AgentSandboxConfig, firewallConfig *FirewallConfig) []string { + awfArgs = append(awfArgs, getSSLBumpArgs(firewallConfig)...) if firewallConfig != nil && len(firewallConfig.Args) > 0 { awfArgs = append(awfArgs, firewallConfig.Args...) } - - // Add custom args from agent config if specified if agentConfig != nil && len(agentConfig.Args) > 0 { awfArgs = append(awfArgs, agentConfig.Args...) awfHelpersLog.Printf("Added %d custom args from agent config", len(agentConfig.Args)) } - - // Pass memory limit to AWF container if specified in agent config if agentConfig != nil && agentConfig.Memory != "" { awfArgs = append(awfArgs, "--memory-limit", agentConfig.Memory) awfHelpersLog.Printf("Set AWF memory limit to %s", agentConfig.Memory) } + return awfArgs +} +// BuildAWFArgs constructs common AWF arguments from configuration. +// This extracts the shared AWF argument building logic from engine implementations. +// +// The following flags are expressed in the generated JSON config file written by +// BuildAWFCommand and are therefore not emitted here: +// - --allow-domains / --block-domains β†’ network.allowDomains / network.blockDomains +// - --image-tag β†’ container.imageTag +// - --openai-api-target β†’ apiProxy.targets.openai.host +// - --anthropic-api-target β†’ apiProxy.targets.anthropic.host +// - --copilot-api-target β†’ apiProxy.targets.copilot.host +// - --gemini-api-target β†’ apiProxy.targets.gemini.host +// +// Note: --enable-api-proxy is deprecated in AWF v0.27.32+ (API proxy is always on). +// The apiProxy.enabled field is still emitted in the config file for backward compat. +// +// Parameters: +// - config: AWF command configuration +// +// Returns: +// - []string: List of AWF arguments (safe args only; expandable-var args like +// --container-workdir and --mount are handled by BuildAWFCommand) +func BuildAWFArgs(config AWFCommandConfig) []string { + awfHelpersLog.Printf("Building AWF args for engine: %s", config.EngineName) + firewallConfig := getFirewallConfig(config.WorkflowData) + agentConfig := getAgentConfig(config.WorkflowData) + awfArgs := appendAWFContainerRuntimeArgs(nil, config, firewallConfig) + awfArgs = appendAWFExcludeEnvArgs(awfArgs, config.ExcludeEnvVarNames, firewallConfig) + awfArgs = appendAWFMountAndLogArgs(awfArgs, config.WorkflowData, agentConfig, firewallConfig) + awfArgs = appendAWFLegacySecurityArgs(awfArgs, config, agentConfig, firewallConfig) + awfArgs = append(awfArgs, "--skip-pull") + awfHelpersLog.Print("Using --skip-pull since images are pre-downloaded") + awfArgs = appendAWFCliProxyAndBasePathArgs(awfArgs, config.WorkflowData, firewallConfig) + awfArgs = appendAWFCustomArgs(awfArgs, agentConfig, firewallConfig) awfHelpersLog.Printf("Built %d AWF arguments", len(awfArgs)) return awfArgs } diff --git a/pkg/workflow/checkout_config_parser.go b/pkg/workflow/checkout_config_parser.go index f03d40866b3..e2ee479e385 100644 --- a/pkg/workflow/checkout_config_parser.go +++ b/pkg/workflow/checkout_config_parser.go @@ -76,135 +76,152 @@ func ParseCheckoutConfigs(raw any) ([]*CheckoutConfig, error) { return configs, nil } -// checkoutConfigFromMap converts a raw map to a CheckoutConfig. -func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) { - cfg := &CheckoutConfig{} - - if v, ok := m["repository"]; ok { - s, ok := v.(string) - if !ok { - return nil, errors.New("checkout.repository must be a string") - } - cfg.Repository = s +func checkoutStringValue(m map[string]any, key string) (string, bool, error) { + v, ok := m[key] + if !ok { + return "", false, nil } - - if v, ok := m["ref"]; ok { - s, ok := v.(string) - if !ok { - return nil, errors.New("checkout.ref must be a string") - } - cfg.Ref = s + s, ok := v.(string) + if !ok { + return "", false, fmt.Errorf("checkout.%s must be a string", key) } + return s, true, nil +} - if v, ok := m["path"]; ok { - s, ok := v.(string) - if !ok { - return nil, errors.New("checkout.path must be a string") - } - cfg.PathExplicit = true - // Normalize "." to empty string: both mean the workspace root and - // are treated identically by the checkout step generator. - if s == "." { - s = "" - } - cfg.Path = s +func parseCheckoutAppConfig(fieldName string, value any) (*GitHubAppConfig, error) { + appMap, ok := value.(map[string]any) + if !ok { + return nil, fmt.Errorf("checkout.%s must be an object", fieldName) } - - // Support both "github-token" (preferred) and "token" (backward compat) - if v, ok := m["github-token"]; ok { - s, ok := v.(string) - if !ok { - return nil, errors.New("checkout.github-token must be a string") - } - cfg.GitHubToken = s - } else if v, ok := m["token"]; ok { - // Backward compatibility: "token" is accepted but "github-token" is preferred - s, ok := v.(string) - if !ok { - return nil, errors.New("checkout.token must be a string") - } - cfg.GitHubToken = s + appConfig := parseAppConfig(appMap) + if appConfig.AppID == "" || appConfig.PrivateKey == "" { + return nil, fmt.Errorf("checkout.%s requires both client-id (or app-id) and private-key", fieldName) } + return appConfig, nil +} - // Parse app configuration for GitHub App-based authentication - if v, ok := m["github-app"]; ok { - appMap, ok := v.(map[string]any) - if !ok { - return nil, errors.New("checkout.github-app must be an object") +func parseCheckoutCoreFields(m map[string]any, cfg *CheckoutConfig) error { + for _, field := range []struct { + key string + set func(string) + }{{"repository", func(value string) { cfg.Repository = value }}, {"ref", func(value string) { cfg.Ref = value }}} { + if value, ok, err := checkoutStringValue(m, field.key); err != nil { + return err + } else if ok { + field.set(value) } - cfg.GitHubApp = parseAppConfig(appMap) - if cfg.GitHubApp.AppID == "" || cfg.GitHubApp.PrivateKey == "" { - return nil, errors.New("checkout.github-app requires both client-id (or app-id) and private-key") + } + if pathValue, ok, err := checkoutStringValue(m, "path"); err != nil { + return err + } else if ok { + cfg.PathExplicit = true + if pathValue == "." { + pathValue = "" } + cfg.Path = pathValue + } + if tokenValue, ok, err := checkoutStringValue(m, "github-token"); err != nil { + return err + } else if ok { + cfg.GitHubToken = tokenValue + } else if tokenValue, ok, err := checkoutStringValue(m, "token"); err != nil { + return err + } else if ok { + cfg.GitHubToken = tokenValue } + return nil +} - // Parse app configuration for safe_outputs-only GitHub App authentication. - parseSafeOutputAppConfig := func(fieldName string, value any) (*GitHubAppConfig, error) { - appMap, ok := value.(map[string]any) - if !ok { - return nil, fmt.Errorf("checkout.%s must be an object", fieldName) - } - appConfig := parseAppConfig(appMap) - if appConfig.AppID == "" || appConfig.PrivateKey == "" { - return nil, fmt.Errorf("checkout.%s requires both client-id (or app-id) and private-key", fieldName) +func parseCheckoutAuthFields(m map[string]any, cfg *CheckoutConfig) error { + if value, ok := m["github-app"]; ok { + appConfig, err := parseCheckoutAppConfig("github-app", value) + if err != nil { + return err } - return appConfig, nil + cfg.GitHubApp = appConfig } - if v, ok := m["safe-outputs-github-app"]; ok { - appConfig, err := parseSafeOutputAppConfig("safe-outputs-github-app", v) + if value, ok := m["safe-outputs-github-app"]; ok { + appConfig, err := parseCheckoutAppConfig("safe-outputs-github-app", value) if err != nil { - return nil, err + return err } cfg.SafeOutputGitHubApp = appConfig } if _, ok := m["safe-output-github-app"]; ok { - return nil, errors.New("checkout.safe-output-github-app is not supported; use checkout.safe-outputs-github-app") + return errors.New("checkout.safe-output-github-app is not supported; use checkout.safe-outputs-github-app") } - - // Validate mutual exclusivity of github-token and github-app if cfg.GitHubToken != "" && cfg.GitHubApp != nil { checkoutManagerLog.Print("Rejecting checkout config: github-token and github-app are mutually exclusive") - return nil, errors.New("checkout: github-token and github-app are mutually exclusive; use one or the other") + return errors.New("checkout: github-token and github-app are mutually exclusive; use one or the other") } + return nil +} - checkoutManagerLog.Printf("Parsed checkout config: repo=%q, ref=%q, path=%q, current=%v, hasToken=%v, hasApp=%v", - cfg.Repository, cfg.Ref, cfg.Path, cfg.Current, cfg.GitHubToken != "", cfg.GitHubApp != nil) - - if v, ok := m["fetch-depth"]; ok { - switch n := v.(type) { - case int: - depth := n - cfg.FetchDepth = &depth - case int64: - depth := int(n) - cfg.FetchDepth = &depth - case uint64: - depth := int(n) - cfg.FetchDepth = &depth - case float64: - if n != float64(int64(n)) { - return nil, errors.New("checkout.fetch-depth must be an integer") - } - depth := int(n) - cfg.FetchDepth = &depth - default: +func parseCheckoutFetchDepth(value any) (*int, error) { + switch n := value.(type) { + case int: + depth := n + return &depth, nil + case int64: + depth := int(n) + return &depth, nil + case uint64: + depth := int(n) + return &depth, nil + case float64: + if n != float64(int64(n)) { return nil, errors.New("checkout.fetch-depth must be an integer") } - if cfg.FetchDepth != nil && *cfg.FetchDepth < 0 { - return nil, errors.New("checkout.fetch-depth must be >= 0") - } + depth := int(n) + return &depth, nil + default: + return nil, errors.New("checkout.fetch-depth must be an integer") } +} - if v, ok := m["sparse-checkout"]; ok { - s, ok := v.(string) - if !ok { - return nil, errors.New("checkout.sparse-checkout must be a string") +func parseCheckoutFetch(value any) ([]string, error) { + switch fv := value.(type) { + case string: + if strings.TrimSpace(fv) == "" { + return nil, errors.New("checkout.fetch string value must not be empty") + } + return []string{fv}, nil + case []any: + refs := make([]string, 0, len(fv)) + for i, item := range fv { + s, ok := item.(string) + if !ok { + return nil, fmt.Errorf("checkout.fetch[%d] must be a string, got %T", i, item) + } + if strings.TrimSpace(s) == "" { + return nil, fmt.Errorf("checkout.fetch[%d] must not be empty", i) + } + refs = append(refs, s) } - cfg.SparseCheckout = s + return refs, nil + default: + return nil, errors.New("checkout.fetch must be a string or an array of strings") } +} - if v, ok := m["submodules"]; ok { - switch sv := v.(type) { +func parseCheckoutBehaviorFields(m map[string]any, cfg *CheckoutConfig) error { + if value, ok := m["fetch-depth"]; ok { + depth, err := parseCheckoutFetchDepth(value) + if err != nil { + return err + } + if depth != nil && *depth < 0 { + return errors.New("checkout.fetch-depth must be >= 0") + } + cfg.FetchDepth = depth + } + if value, ok, err := checkoutStringValue(m, "sparse-checkout"); err != nil { + return err + } else if ok { + cfg.SparseCheckout = value + } + if value, ok := m["submodules"]; ok { + switch sv := value.(type) { case string: cfg.Submodules = sv case bool: @@ -214,68 +231,45 @@ func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) { cfg.Submodules = "false" } default: - return nil, errors.New("checkout.submodules must be a string or boolean") + return errors.New("checkout.submodules must be a string or boolean") } } - - if v, ok := m["lfs"]; ok { - b, ok := v.(bool) - if !ok { - return nil, errors.New("checkout.lfs must be a boolean") + for _, field := range []struct { + key string + set func(bool) + err string + }{{"lfs", func(v bool) { cfg.LFS = v }, "checkout.lfs must be a boolean"}, {"current", func(v bool) { cfg.Current = v }, "checkout.current must be a boolean"}, {"wiki", func(v bool) { cfg.Wiki = v }, "checkout.wiki must be a boolean"}, {"force-clean-git-credentials", func(v bool) { cfg.CleanGitCredentials = v }, "checkout.force-clean-git-credentials must be a boolean"}} { + if value, ok := m[field.key]; ok { + b, ok := value.(bool) + if !ok { + return errors.New(field.err) + } + field.set(b) } - cfg.LFS = b } - - if v, ok := m["current"]; ok { - b, ok := v.(bool) - if !ok { - return nil, errors.New("checkout.current must be a boolean") + if value, ok := m["fetch"]; ok { + fetchRefs, err := parseCheckoutFetch(value) + if err != nil { + return err } - cfg.Current = b + cfg.Fetch = fetchRefs } + return nil +} - if v, ok := m["fetch"]; ok { - switch fv := v.(type) { - case string: - // Single string shorthand: treat as a one-element list - if strings.TrimSpace(fv) == "" { - return nil, errors.New("checkout.fetch string value must not be empty") - } - cfg.Fetch = []string{fv} - case []any: - refs := make([]string, 0, len(fv)) - for i, item := range fv { - s, ok := item.(string) - if !ok { - return nil, fmt.Errorf("checkout.fetch[%d] must be a string, got %T", i, item) - } - if strings.TrimSpace(s) == "" { - return nil, fmt.Errorf("checkout.fetch[%d] must not be empty", i) - } - refs = append(refs, s) - } - cfg.Fetch = refs - default: - return nil, errors.New("checkout.fetch must be a string or an array of strings") - } +// checkoutConfigFromMap converts a raw map to a CheckoutConfig. +func checkoutConfigFromMap(m map[string]any) (*CheckoutConfig, error) { + cfg := &CheckoutConfig{} + if err := parseCheckoutCoreFields(m, cfg); err != nil { + return nil, err } - - if v, ok := m["wiki"]; ok { - b, ok := v.(bool) - if !ok { - return nil, errors.New("checkout.wiki must be a boolean") - } - cfg.Wiki = b + if err := parseCheckoutAuthFields(m, cfg); err != nil { + return nil, err } - - if v, ok := m["force-clean-git-credentials"]; ok { - b, ok := v.(bool) - if !ok { - return nil, errors.New("checkout.force-clean-git-credentials must be a boolean") - } - cfg.CleanGitCredentials = b + checkoutManagerLog.Printf("Parsed checkout config: repo=%q, ref=%q, path=%q, current=%v, hasToken=%v, hasApp=%v", cfg.Repository, cfg.Ref, cfg.Path, cfg.Current, cfg.GitHubToken != "", cfg.GitHubApp != nil) + if err := parseCheckoutBehaviorFields(m, cfg); err != nil { + return nil, err } - return cfg, nil } diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index 8f27c1646a2..c749489374c 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -267,27 +267,30 @@ func (c *Compiler) buildSafeOutputsSetupAndDownloadSteps(data *WorkflowData, age // and the named convenience outputs for first-created items. // It returns the collected steps, outputs map, and the list of safe-output step names registered. func (c *Compiler) buildSafeOutputsHandlerOutputsAndActionSteps(data *WorkflowData, agentArtifactPrefix, markdownPath string) ([]string, map[string]string, []string, error) { - var steps []string - outputs := make(map[string]string) - var safeOutputStepNames []string - - // Note: Unlock step has been moved to dedicated unlock job - // The safe_outputs job now depends on the unlock job, so the issue - // will already be unlocked when this job runs - - // === Build safe output steps === - // - // IMPORTANT: Step order matters for safe outputs that depend on each other. - // The execution order ensures dependencies are satisfied: - // 1. Handler Manager - processes create_issue, update_issue, add_comment, etc. - // 2. Assign To Agent - assigns issue to agent (after handler managers complete) - // 3. Create Agent Session - creates agent session (after assignment) - // - // Note: All project-related operations (create_project, update_project, create_project_status_update) - // are now handled by the unified handler in the handler manager step. - - // Check if any handler-manager-supported types are enabled - hasHandlerManagerTypes := data.SafeOutputs.CreateIssues != nil || + state := &safeOutputsHandlerBuildState{ + outputs: make(map[string]string), + } + if err := c.addCustomScriptSetupSteps(state, data); err != nil { + return nil, nil, nil, err + } + c.addUploadArtifactStagingDownloadStep(state, data, agentArtifactPrefix) + if err := c.addHandlerManagerOutputsAndSteps(state, data); err != nil { + return nil, nil, nil, err + } + c.addSarifOutputsAndSteps(state, data, agentArtifactPrefix) + c.addCustomSafeOutputActionSteps(state, data, markdownPath) + addConvenienceSafeOutputOutputs(state.outputs, data) + return state.steps, state.outputs, state.safeOutputStepNames, nil +} + +type safeOutputsHandlerBuildState struct { + steps []string + outputs map[string]string + safeOutputStepNames []string +} + +func hasHandlerManagerSafeOutputTypes(data *WorkflowData) bool { + return data.SafeOutputs.CreateIssues != nil || data.SafeOutputs.AddComments != nil || data.SafeOutputs.CreateDiscussions != nil || data.SafeOutputs.CloseIssues != nil || @@ -317,176 +320,144 @@ func (c *Compiler) buildSafeOutputsHandlerOutputsAndActionSteps(data *WorkflowDa data.SafeOutputs.CreateCheckRun != nil || data.SafeOutputs.MissingTool != nil || data.SafeOutputs.MissingData != nil || - data.SafeOutputs.AssignToAgent != nil || // assign_to_agent is now handled by the handler manager - data.SafeOutputs.CreateAgentSessions != nil || // create_agent_session is now handled by the handler manager - data.SafeOutputs.UploadArtifact != nil || // upload_artifact is handled inline in the handler loop - len(data.SafeOutputs.Scripts) > 0 || // Custom scripts run in the handler loop - len(data.SafeOutputs.Actions) > 0 // Custom actions need handler to export their payloads - - // Note: All project-related operations are now handled by the unified handler. - // The project handler manager has been removed. - - // Add custom script files step (writes inline scripts to the actions folder) - // This must run before the handler manager step so the files are available for require() - if len(data.SafeOutputs.Scripts) > 0 { - consolidatedSafeOutputsJobLog.Printf("Adding setup step for %d custom safe-output script(s)", len(data.SafeOutputs.Scripts)) - scriptSetupSteps, err := buildCustomScriptFilesStep(data.SafeOutputs.Scripts) - if err != nil { - return nil, nil, nil, fmt.Errorf("failed to build custom script files step: %w", err) - } - steps = append(steps, scriptSetupSteps...) + data.SafeOutputs.AssignToAgent != nil || + data.SafeOutputs.CreateAgentSessions != nil || + data.SafeOutputs.UploadArtifact != nil || + len(data.SafeOutputs.Scripts) > 0 || + len(data.SafeOutputs.Actions) > 0 +} + +func (c *Compiler) addCustomScriptSetupSteps(state *safeOutputsHandlerBuildState, data *WorkflowData) error { + if len(data.SafeOutputs.Scripts) == 0 { + return nil + } + consolidatedSafeOutputsJobLog.Printf("Adding setup step for %d custom safe-output script(s)", len(data.SafeOutputs.Scripts)) + scriptSetupSteps, err := buildCustomScriptFilesStep(data.SafeOutputs.Scripts) + if err != nil { + return fmt.Errorf("failed to build custom script files step: %w", err) } + state.steps = append(state.steps, scriptSetupSteps...) + return nil +} - // Download the upload-artifact staging artifact before the handler manager runs so that - // the upload_artifact handler (which runs inline in the handler loop) can access the files. - if data.SafeOutputs.UploadArtifact != nil { - consolidatedSafeOutputsJobLog.Print("Adding upload-artifact staging download step") - stagingArtifactName := agentArtifactPrefix + SafeOutputsUploadArtifactStagingArtifactName - steps = append(steps, - " - name: Download upload-artifact staging\n", - " continue-on-error: true\n", - fmt.Sprintf(" uses: %s\n", c.getActionPin("actions/download-artifact")), - " with:\n", - fmt.Sprintf(" name: %s\n", stagingArtifactName), - fmt.Sprintf(" path: %s\n", artifactStagingDirExpr), - ) +func (c *Compiler) addUploadArtifactStagingDownloadStep(state *safeOutputsHandlerBuildState, data *WorkflowData, agentArtifactPrefix string) { + if data.SafeOutputs.UploadArtifact == nil { + return } + consolidatedSafeOutputsJobLog.Print("Adding upload-artifact staging download step") + stagingArtifactName := agentArtifactPrefix + SafeOutputsUploadArtifactStagingArtifactName + state.steps = append(state.steps, + " - name: Download upload-artifact staging\n", + " continue-on-error: true\n", + fmt.Sprintf(" uses: %s\n", c.getActionPin("actions/download-artifact")), + " with:\n", + fmt.Sprintf(" name: %s\n", stagingArtifactName), + fmt.Sprintf(" path: %s\n", artifactStagingDirExpr), + ) +} - // 1. Handler Manager step (processes create_issue, update_issue, add_comment, assign_to_agent, - // upload_artifact, etc.) - // This processes all safe output types that are handled by the unified handler - // Critical for workflows that create projects and then add issues/PRs to those projects - if hasHandlerManagerTypes { - consolidatedSafeOutputsJobLog.Print("Using handler manager for safe outputs") - handlerManagerSteps, err := c.buildHandlerManagerStep(data) - if err != nil { - return nil, nil, nil, err - } - steps = append(steps, handlerManagerSteps...) - safeOutputStepNames = append(safeOutputStepNames, "process_safe_outputs") - - // Add outputs from handler manager - outputs["process_safe_outputs_temporary_id_map"] = "${{ steps.process_safe_outputs.outputs.temporary_id_map }}" - outputs["process_safe_outputs_processed_count"] = "${{ steps.process_safe_outputs.outputs.processed_count }}" - outputs["create_discussion_errors"] = "${{ steps.process_safe_outputs.outputs.create_discussion_errors }}" - outputs["create_discussion_error_count"] = "${{ steps.process_safe_outputs.outputs.create_discussion_error_count }}" - outputs["code_push_failure_errors"] = "${{ steps.process_safe_outputs.outputs.code_push_failure_errors }}" - outputs["code_push_failure_count"] = "${{ steps.process_safe_outputs.outputs.code_push_failure_count }}" - - // Note: Permissions are now computed centrally by ComputePermissionsForSafeOutputs() - // at the start of this function to ensure consistent permission calculation - - // Export assign_to_agent outputs from the handler manager step - if data.SafeOutputs.AssignToAgent != nil { - consolidatedSafeOutputsJobLog.Print("Exposing assign_to_agent outputs from handler manager") - outputs["assign_to_agent_assigned"] = "${{ steps.process_safe_outputs.outputs.assign_to_agent_assigned }}" - outputs["assign_to_agent_assignment_errors"] = "${{ steps.process_safe_outputs.outputs.assign_to_agent_assignment_errors }}" - outputs["assign_to_agent_assignment_error_count"] = "${{ steps.process_safe_outputs.outputs.assign_to_agent_assignment_error_count }}" - } +func (c *Compiler) addHandlerManagerOutputsAndSteps(state *safeOutputsHandlerBuildState, data *WorkflowData) error { + if !hasHandlerManagerSafeOutputTypes(data) { + return nil + } + consolidatedSafeOutputsJobLog.Print("Using handler manager for safe outputs") + handlerManagerSteps, err := c.buildHandlerManagerStep(data) + if err != nil { + return err + } + state.steps = append(state.steps, handlerManagerSteps...) + state.safeOutputStepNames = append(state.safeOutputStepNames, "process_safe_outputs") + addBaseHandlerManagerOutputs(state.outputs) + addOptionalHandlerManagerOutputs(state.outputs, data) + return nil +} - // Export create_agent_session outputs from the handler manager step - if data.SafeOutputs.CreateAgentSessions != nil { - consolidatedSafeOutputsJobLog.Print("Exposing create_agent_session outputs from handler manager") - outputs["create_agent_session_session_number"] = "${{ steps.process_safe_outputs.outputs.session_number }}" - outputs["create_agent_session_session_url"] = "${{ steps.process_safe_outputs.outputs.session_url }}" - } +func addBaseHandlerManagerOutputs(outputs map[string]string) { + outputs["process_safe_outputs_temporary_id_map"] = "${{ steps.process_safe_outputs.outputs.temporary_id_map }}" + outputs["process_safe_outputs_processed_count"] = "${{ steps.process_safe_outputs.outputs.processed_count }}" + outputs["create_discussion_errors"] = "${{ steps.process_safe_outputs.outputs.create_discussion_errors }}" + outputs["create_discussion_error_count"] = "${{ steps.process_safe_outputs.outputs.create_discussion_error_count }}" + outputs["code_push_failure_errors"] = "${{ steps.process_safe_outputs.outputs.code_push_failure_errors }}" + outputs["code_push_failure_count"] = "${{ steps.process_safe_outputs.outputs.code_push_failure_count }}" +} - // Export upload_artifact outputs. - // The handler sets slot_N_* outputs on the process_safe_outputs step; we expose - // them as upload_artifact_slot_N_* job outputs for external consumers. - // The actual artifact uploads are performed directly by the JS handler via - // @actions/artifact REST API β€” no additional YAML steps are required. - if data.SafeOutputs.UploadArtifact != nil { - consolidatedSafeOutputsJobLog.Print("Exposing upload_artifact outputs from handler manager") - cfg := data.SafeOutputs.UploadArtifact - outputs["upload_artifact_count"] = "${{ steps.process_safe_outputs.outputs.upload_artifact_count }}" - for i := range cfg.MaxUploads { - outputs[fmt.Sprintf("upload_artifact_slot_%d_tmp_id", i)] = fmt.Sprintf("${{ steps.process_safe_outputs.outputs.slot_%d_tmp_id }}", i) - } - } +func addOptionalHandlerManagerOutputs(outputs map[string]string, data *WorkflowData) { + if data.SafeOutputs.AssignToAgent != nil { + consolidatedSafeOutputsJobLog.Print("Exposing assign_to_agent outputs from handler manager") + outputs["assign_to_agent_assigned"] = "${{ steps.process_safe_outputs.outputs.assign_to_agent_assigned }}" + outputs["assign_to_agent_assignment_errors"] = "${{ steps.process_safe_outputs.outputs.assign_to_agent_assignment_errors }}" + outputs["assign_to_agent_assignment_error_count"] = "${{ steps.process_safe_outputs.outputs.assign_to_agent_assignment_error_count }}" + } + if data.SafeOutputs.CreateAgentSessions != nil { + consolidatedSafeOutputsJobLog.Print("Exposing create_agent_session outputs from handler manager") + outputs["create_agent_session_session_number"] = "${{ steps.process_safe_outputs.outputs.session_number }}" + outputs["create_agent_session_session_url"] = "${{ steps.process_safe_outputs.outputs.session_url }}" + } + if data.SafeOutputs.UploadArtifact != nil { + addUploadArtifactOutputs(outputs, data.SafeOutputs.UploadArtifact) + } +} +func addUploadArtifactOutputs(outputs map[string]string, cfg *UploadArtifactConfig) { + consolidatedSafeOutputsJobLog.Print("Exposing upload_artifact outputs from handler manager") + outputs["upload_artifact_count"] = "${{ steps.process_safe_outputs.outputs.upload_artifact_count }}" + for i := range cfg.MaxUploads { + outputs[fmt.Sprintf("upload_artifact_slot_%d_tmp_id", i)] = fmt.Sprintf("${{ steps.process_safe_outputs.outputs.slot_%d_tmp_id }}", i) } +} - // 2. SARIF output β€” expose sarif_file from the handler so the dedicated - // upload_code_scanning_sarif job (built in buildCodeScanningUploadJob) can access it - // via needs.safe_outputs.outputs.sarif_file and decide whether to run. - // Additionally, upload the SARIF file as a GitHub Actions artifact so the upload job - // can retrieve the actual file (job outputs only carry the path string; the file itself - // only exists in the safe_outputs job workspace). - // NOTE: We do NOT export checkout_token as a job output. GitHub Actions masks output - // values that contain secret references, so the downstream job would receive an empty - // string. The upload job computes the token directly from static secret references. - if data.SafeOutputs.CreateCodeScanningAlerts != nil && !isHandlerStaged(c.trialMode || templatableBoolIsTrue(data.SafeOutputs.Staged), data.SafeOutputs.CreateCodeScanningAlerts.Staged) { - consolidatedSafeOutputsJobLog.Print("Exposing sarif_file output for upload_code_scanning_sarif job") - outputs["sarif_file"] = "${{ steps.process_safe_outputs.outputs.sarif_file }}" - - // Upload the SARIF file as an artifact so the upload_code_scanning_sarif job - // (which runs in a separate, fresh workspace) can download and process it. - steps = append(steps, buildSarifArtifactUploadStep(agentArtifactPrefix, c.getActionPin)...) - } - - // 3. Custom action steps β€” compiler-generated steps for each configured safe-output action. - // These steps run after the handler manager, which processes the agent payload and exports - // a JSON payload output for each action tool call. Each step is guarded by an `if:` condition - // that checks whether the handler manager exported a payload for this action. - if len(data.SafeOutputs.Actions) > 0 { - // resolveAllActions was already called early in buildJobs (before generateToolsMetaJSON) - // so action configs already have Inputs/ActionDescription populated. We only call it - // again here as a safety net in case compileSafeOutputsJob is called independently. - c.resolveAllActions(data, markdownPath) - - actionStepYAML := c.buildActionSteps(data) - steps = append(steps, actionStepYAML...) - - // Register each action as having a handler manager output - for actionName := range data.SafeOutputs.Actions { - normalizedName := stringutil.NormalizeSafeOutputIdentifier(actionName) - safeOutputStepNames = append(safeOutputStepNames, "action_"+normalizedName) - } +func (c *Compiler) addSarifOutputsAndSteps(state *safeOutputsHandlerBuildState, data *WorkflowData, agentArtifactPrefix string) { + if data.SafeOutputs.CreateCodeScanningAlerts == nil || + isHandlerStaged(c.trialMode || templatableBoolIsTrue(data.SafeOutputs.Staged), data.SafeOutputs.CreateCodeScanningAlerts.Staged) { + return } + consolidatedSafeOutputsJobLog.Print("Exposing sarif_file output for upload_code_scanning_sarif job") + state.outputs["sarif_file"] = "${{ steps.process_safe_outputs.outputs.sarif_file }}" + state.steps = append(state.steps, buildSarifArtifactUploadStep(agentArtifactPrefix, c.getActionPin)...) +} - // The outputs and permissions are configured in the handler manager section above +func (c *Compiler) addCustomSafeOutputActionSteps(state *safeOutputsHandlerBuildState, data *WorkflowData, markdownPath string) { + if len(data.SafeOutputs.Actions) == 0 { + return + } + c.resolveAllActions(data, markdownPath) + state.steps = append(state.steps, c.buildActionSteps(data)...) + for actionName := range data.SafeOutputs.Actions { + normalizedName := stringutil.NormalizeSafeOutputIdentifier(actionName) + state.safeOutputStepNames = append(state.safeOutputStepNames, "action_"+normalizedName) + } +} + +func addConvenienceSafeOutputOutputs(outputs map[string]string, data *WorkflowData) { if data.SafeOutputs.AddReviewer != nil { outputs["add_reviewer_reviewers_added"] = "${{ steps.process_safe_outputs.outputs.reviewers_added }}" } - - // The outputs and permissions are configured in the handler manager section above if data.SafeOutputs.AssignMilestone != nil { outputs["assign_milestone_milestone_assigned"] = "${{ steps.process_safe_outputs.outputs.milestone_assigned }}" } - - // The outputs and permissions are configured in the handler manager section above if data.SafeOutputs.AssignToUser != nil { outputs["assign_to_user_assigned"] = "${{ steps.process_safe_outputs.outputs.assigned }}" } - - // Individual named outputs for first-created items (enables workflow_call consumers to access results) if data.SafeOutputs.CreateIssues != nil { outputs["created_issue_number"] = "${{ steps.process_safe_outputs.outputs.created_issue_number }}" outputs["created_issue_url"] = "${{ steps.process_safe_outputs.outputs.created_issue_url }}" } - if data.SafeOutputs.CreatePullRequests != nil { outputs["created_pr_number"] = "${{ steps.process_safe_outputs.outputs.created_pr_number }}" outputs["created_pr_url"] = "${{ steps.process_safe_outputs.outputs.created_pr_url }}" } - if data.SafeOutputs.AddComments != nil { outputs["comment_id"] = "${{ steps.process_safe_outputs.outputs.comment_id }}" outputs["comment_url"] = "${{ steps.process_safe_outputs.outputs.comment_url }}" } - if data.SafeOutputs.PushToPullRequestBranch != nil { outputs["push_commit_sha"] = "${{ steps.process_safe_outputs.outputs.push_commit_sha }}" outputs["push_commit_url"] = "${{ steps.process_safe_outputs.outputs.push_commit_url }}" } - if data.SafeOutputs.CallWorkflow != nil { outputs["call_workflow_name"] = "${{ steps.process_safe_outputs.outputs.call_workflow_name }}" outputs["call_workflow_payload"] = "${{ steps.process_safe_outputs.outputs.call_workflow_payload }}" } - - return steps, outputs, safeOutputStepNames, nil } // buildSafeOutputsJobFromParts finalizes the step list (app-token insertion, token invalidation, diff --git a/pkg/workflow/unified_prompt_step.go b/pkg/workflow/unified_prompt_step.go index d33d128d10b..a88c08ffa91 100644 --- a/pkg/workflow/unified_prompt_step.go +++ b/pkg/workflow/unified_prompt_step.go @@ -54,161 +54,101 @@ func removeConsecutiveEmptyLines(content string) string { return strings.Join(result, "\n") } -// collectPromptSections collects all prompt sections in the order they should be appended -func (c *Compiler) collectPromptSections(data *WorkflowData) []PromptSection { +func (c *Compiler) collectCorePromptSections(data *WorkflowData) []PromptSection { var sections []PromptSection - - // 0. XPia instructions (unless disabled by feature flag) if !isFeatureEnabled(constants.DisableXPIAPromptFeatureFlag, data) { unifiedPromptLog.Print("Adding XPIA section") - sections = append(sections, PromptSection{ - Content: xpiaPromptFile, - IsFile: true, - }) + sections = append(sections, PromptSection{Content: xpiaPromptFile, IsFile: true}) } else { unifiedPromptLog.Print("XPIA section disabled by feature flag") } - // 1. Temporary folder instructions (always included) unifiedPromptLog.Print("Adding temp folder section") - sections = append(sections, PromptSection{ - Content: tempFolderPromptFile, - IsFile: true, - }) - - // 2. Markdown generation instructions (always included) + sections = append(sections, PromptSection{Content: tempFolderPromptFile, IsFile: true}) unifiedPromptLog.Print("Adding markdown section") - sections = append(sections, PromptSection{ - Content: markdownPromptFile, - IsFile: true, - }) - - // 3. Playwright instructions (if playwright tool is enabled) + sections = append(sections, PromptSection{Content: markdownPromptFile, IsFile: true}) if hasPlaywrightTool(data.ParsedTools) { unifiedPromptLog.Print("Adding playwright section") - sections = append(sections, PromptSection{ - Content: playwrightPromptFile, - IsFile: true, - }) + sections = append(sections, PromptSection{Content: playwrightPromptFile, IsFile: true}) } - - // 4. Trial mode note (if in trial mode) if c.trialMode { unifiedPromptLog.Print("Adding trial mode section") - trialContent := fmt.Sprintf("## Note\nThis workflow is running in directory $GITHUB_WORKSPACE, but that directory actually contains the contents of the repository '%s'.", c.trialLogicalRepoSlug) sections = append(sections, PromptSection{ - Content: trialContent, - IsFile: false, + Content: fmt.Sprintf("## Note\nThis workflow is running in directory $GITHUB_WORKSPACE, but that directory actually contains the contents of the repository '%s'.", c.trialLogicalRepoSlug), }) } - - // 6. Cache memory instructions (if enabled) if data.CacheMemoryConfig != nil && len(data.CacheMemoryConfig.Caches) > 0 { unifiedPromptLog.Printf("Adding cache memory section: caches=%d", len(data.CacheMemoryConfig.Caches)) - section := buildCacheMemoryPromptSection(data.CacheMemoryConfig) - if section != nil { + if section := buildCacheMemoryPromptSection(data.CacheMemoryConfig); section != nil { sections = append(sections, *section) } } - - // 7. Repo memory instructions (if enabled) if data.RepoMemoryConfig != nil && len(data.RepoMemoryConfig.Memories) > 0 { unifiedPromptLog.Printf("Adding repo memory section: memories=%d", len(data.RepoMemoryConfig.Memories)) - section := buildRepoMemoryPromptSection(data.RepoMemoryConfig) - if section != nil { + if section := buildRepoMemoryPromptSection(data.RepoMemoryConfig); section != nil { sections = append(sections, *section) } } - - // 8. Safe outputs instructions (if enabled) if HasSafeOutputsEnabled(data.SafeOutputs) { unifiedPromptLog.Print("Adding safe outputs section") - // Static intro from file (gh CLI warning, temporary ID rules, noop note) - sections = append(sections, PromptSection{ - Content: safeOutputsPromptFile, - IsFile: true, - }) - // Per-tool sections: opening tag + tools list (inline), tool instruction files, closing tag + sections = append(sections, PromptSection{Content: safeOutputsPromptFile, IsFile: true}) sections = append(sections, buildSafeOutputsSections(data.SafeOutputs)...) } - - // 8a. MCP CLI tools instructions (if any MCP servers are mounted as CLIs) if section := buildMCPCLIPromptSection(data); section != nil { unifiedPromptLog.Printf("Adding MCP CLI tools section: servers=%v", getMCPCLIServerNames(data)) sections = append(sections, *section) } + return sections +} - // 9. GitHub context (if GitHub tool is enabled) - if hasGitHubTool(data.ParsedTools) { - unifiedPromptLog.Print("Adding GitHub context section") - - // Build the combined prompt text: base github context + optional checkout list. - // The checkout list may contain ${{ github.repository }} which must go through - // the expression extractor so the placeholder substitution step can resolve it. - combinedPromptText := githubContextPromptText - if checkoutsContent := buildCheckoutsPromptContent(data.CheckoutConfigs); checkoutsContent != "" { - unifiedPromptLog.Printf("Injecting checkout list into GitHub context (%d checkouts)", len(data.CheckoutConfigs)) - const closeTag = "" - if idx := strings.LastIndex(combinedPromptText, closeTag); idx >= 0 { - combinedPromptText = combinedPromptText[:idx] + checkoutsContent + combinedPromptText[idx:] - } else { - combinedPromptText += "\n" + checkoutsContent - } - } - - // Extract expressions from the combined content (includes any new expressions - // introduced by the checkout list, e.g. ${{ github.repository }}). - extractor := NewExpressionExtractor() - expressionMappings, err := extractor.ExtractExpressions(combinedPromptText) - if err == nil && len(expressionMappings) > 0 { - modifiedPromptText := extractor.ReplaceExpressionsWithEnvVars(combinedPromptText) - - // Build environment variables map - envVars := make(map[string]string) - for _, mapping := range expressionMappings { - envVars[mapping.EnvVar] = fmt.Sprintf("${{ %s }}", mapping.Content) - } - - sections = append(sections, PromptSection{ - Content: modifiedPromptText, - IsFile: false, - EnvVars: envVars, - }) +func buildGitHubContextPromptSection(data *WorkflowData) *PromptSection { + if !hasGitHubTool(data.ParsedTools) { + return nil + } + unifiedPromptLog.Print("Adding GitHub context section") + combinedPromptText := githubContextPromptText + if checkoutsContent := buildCheckoutsPromptContent(data.CheckoutConfigs); checkoutsContent != "" { + unifiedPromptLog.Printf("Injecting checkout list into GitHub context (%d checkouts)", len(data.CheckoutConfigs)) + const closeTag = "" + if idx := strings.LastIndex(combinedPromptText, closeTag); idx >= 0 { + combinedPromptText = combinedPromptText[:idx] + checkoutsContent + combinedPromptText[idx:] + } else { + combinedPromptText += "\n" + checkoutsContent } } + extractor := NewExpressionExtractor() + expressionMappings, err := extractor.ExtractExpressions(combinedPromptText) + if err != nil || len(expressionMappings) == 0 { + return nil + } + envVars := make(map[string]string, len(expressionMappings)) + for _, mapping := range expressionMappings { + envVars[mapping.EnvVar] = fmt.Sprintf("${{ %s }}", mapping.Content) + } + return &PromptSection{Content: extractor.ReplaceExpressionsWithEnvVars(combinedPromptText), EnvVars: envVars} +} - // 10. GitHub tool-use guidance: directs the model to the correct mechanism for - // GitHub reads (and writes when safe-outputs is also enabled). - // When GitHub mode is gh-proxy, the agent uses the pre-authenticated gh CLI for reads - // instead of a GitHub MCP server (which is not registered). Otherwise, the GitHub - // MCP server is used for reads. +func buildGitHubToolUsePromptSection(data *WorkflowData) *PromptSection { if isGitHubCLIModeEnabled(data) { unifiedPromptLog.Print("Adding cli-proxy tool-use guidance (gh CLI for reads, no GitHub MCP server)") cliProxyFile := cliProxyPromptFile if HasSafeOutputsEnabled(data.SafeOutputs) { cliProxyFile = cliProxyWithSafeOutputsPromptFile } - sections = append(sections, PromptSection{ - Content: cliProxyFile, - IsFile: true, - }) - } else if hasGitHubTool(data.ParsedTools) { - // GitHub MCP tool-use guidance: clarifies that the MCP server is read-only and - // directs the model to use it for GitHub reads. When safe-outputs is also enabled, - // the guidance explicitly separates reads (GitHub MCP) from writes (safeoutputs) so - // the model is never steered away from the available read tools. + return &PromptSection{Content: cliProxyFile, IsFile: true} + } + if hasGitHubTool(data.ParsedTools) { unifiedPromptLog.Print("Adding GitHub MCP tool-use guidance") githubMCPFile := githubMCPToolsPromptFile if HasSafeOutputsEnabled(data.SafeOutputs) { githubMCPFile = githubMCPToolsWithSafeOutputsPromptFile } - sections = append(sections, PromptSection{ - Content: githubMCPFile, - IsFile: true, - }) + return &PromptSection{Content: githubMCPFile, IsFile: true} } + return nil +} - // 11. PR context (if comment-related triggers and checkout is needed) +func (c *Compiler) buildPRContextPromptSections(data *WorkflowData) []PromptSection { hasCommentTriggers := c.hasCommentRelatedTriggers(data) needsCheckout := c.shouldAddCheckoutStep(data) var hasContentsRead bool @@ -217,58 +157,35 @@ func (c *Compiler) collectPromptSections(data *WorkflowData) []PromptSection { } else { hasContentsRead = NewPermissionsParser(data.Permissions).HasContentsReadAccess() } - - if hasCommentTriggers && needsCheckout && hasContentsRead { - unifiedPromptLog.Print("Adding PR context section with condition") - // Use shell condition for PR comment detection - // This checks for issue_comment, pull_request_review_comment, or pull_request_review events - // For issue_comment, we also need to check if it's on a PR (github.event.issue.pull_request != null) - // However, for simplicity in the unified step, we'll add an environment variable to check this - shellCondition := `[ "$GITHUB_EVENT_NAME" = "issue_comment" ] && [ -n "$GH_AW_IS_PR_COMMENT" ] || [ "$GITHUB_EVENT_NAME" = "pull_request_review_comment" ] || [ "$GITHUB_EVENT_NAME" = "pull_request_review" ]` - - // Add environment variable to check if issue_comment is on a PR - envVars := map[string]string{ - "GH_AW_IS_PR_COMMENT": "${{ github.event.issue.pull_request && 'true' || '' }}", - } - - sections = append(sections, PromptSection{ - Content: prContextPromptFile, - IsFile: true, - ShellCondition: shellCondition, - EnvVars: envVars, - }) - - // When push_to_pull_request_branch is configured, add guidance to prefer it over - // create_pull_request when the workflow was triggered by a PR comment. - if data.SafeOutputs != nil && data.SafeOutputs.PushToPullRequestBranch != nil { - unifiedPromptLog.Print("Adding push-to-PR-branch tool preference guidance for PR comment context") - sections = append(sections, PromptSection{ - Content: prContextPushToPRBranchGuidanceFile, - IsFile: true, - ShellCondition: shellCondition, - EnvVars: envVars, - }) - } + if !hasCommentTriggers || !needsCheckout || !hasContentsRead { + return nil } + unifiedPromptLog.Print("Adding PR context section with condition") + shellCondition := `[ "$GITHUB_EVENT_NAME" = "issue_comment" ] && [ -n "$GH_AW_IS_PR_COMMENT" ] || [ "$GITHUB_EVENT_NAME" = "pull_request_review_comment" ] || [ "$GITHUB_EVENT_NAME" = "pull_request_review" ]` + envVars := map[string]string{"GH_AW_IS_PR_COMMENT": "${{ github.event.issue.pull_request && 'true' || '' }}"} + sections := []PromptSection{{Content: prContextPromptFile, IsFile: true, ShellCondition: shellCondition, EnvVars: envVars}} + if data.SafeOutputs != nil && data.SafeOutputs.PushToPullRequestBranch != nil { + unifiedPromptLog.Print("Adding push-to-PR-branch tool preference guidance for PR comment context") + sections = append(sections, PromptSection{Content: prContextPushToPRBranchGuidanceFile, IsFile: true, ShellCondition: shellCondition, EnvVars: envVars}) + } return sections } -// generateUnifiedPromptCreationStep generates a single workflow step (or multiple if needed) that creates -// the complete prompt file with built-in context instructions prepended to the user prompt content. -// -// This consolidates the prompt creation process: -// 1. Built-in context instructions (temp folder, playwright, safe outputs, etc.) - PREPENDED -// 2. User prompt content from markdown - APPENDED -// -// The function handles chunking for large content and ensures proper environment variable handling. -// Returns the combined expression mappings for use in the placeholder substitution step. -func (c *Compiler) generateUnifiedPromptCreationStep(yaml *strings.Builder, builtinSections []PromptSection, userPromptChunks []string, expressionMappings []*ExpressionMapping, data *WorkflowData) []*ExpressionMapping { - unifiedPromptLog.Print("Generating unified prompt creation step") - unifiedPromptLog.Printf("Built-in sections: %d, User prompt chunks: %d", len(builtinSections), len(userPromptChunks)) +// collectPromptSections collects all prompt sections in the order they should be appended +func (c *Compiler) collectPromptSections(data *WorkflowData) []PromptSection { + sections := c.collectCorePromptSections(data) + if section := buildGitHubContextPromptSection(data); section != nil { + sections = append(sections, *section) + } + if section := buildGitHubToolUsePromptSection(data); section != nil { + sections = append(sections, *section) + } + sections = append(sections, c.buildPRContextPromptSections(data)...) + return sections +} - // Derive the heredoc delimiter from the combined prompt content so it is identical - // across builds for the same workflow and changes only when the prompt text changes. +func buildPromptDelimiter(builtinSections []PromptSection, userPromptChunks []string) string { var promptContentForHash strings.Builder for _, section := range builtinSections { promptContentForHash.WriteString(section.Content) @@ -276,252 +193,208 @@ func (c *Compiler) generateUnifiedPromptCreationStep(yaml *strings.Builder, buil for _, chunk := range userPromptChunks { promptContentForHash.WriteString(chunk) } - delimiter := GenerateHeredocDelimiterFromContent("PROMPT", promptContentForHash.String()) + return GenerateHeredocDelimiterFromContent("PROMPT", promptContentForHash.String()) +} - // Collect all environment variables from built-in sections and user prompt expressions +func collectEnvVarsAndMappings(builtinSections []PromptSection, expressionMappings []*ExpressionMapping) (map[string]string, []*ExpressionMapping) { allEnvVars := make(map[string]string) - - // Also collect all expression mappings for the substitution step (using a map to avoid duplicates) expressionMappingsMap := make(map[string]*ExpressionMapping) - - // Add environment variables and expression mappings from built-in sections for _, section := range builtinSections { for key, value := range section.EnvVars { - // Extract the GitHub expression from the value (e.g., "${{ github.repository }}" -> "github.repository") - // This is needed for the substitution step if strings.HasPrefix(value, "${{ ") && strings.HasSuffix(value, " }}") { content := strings.TrimSpace(value[4 : len(value)-3]) - // Add to both allEnvVars (for prompt creation step) and expressionMappingsMap (for substitution step) allEnvVars[key] = value - // Only add if not already present (user prompt expressions take precedence) if _, exists := expressionMappingsMap[key]; !exists { - expressionMappingsMap[key] = &ExpressionMapping{ - EnvVar: key, - Content: content, - } - } - } else { - // For static values (not GitHub Actions expressions), only add to expressionMappingsMap - // This ensures they're only available in the substitution step, not the prompt creation step - if _, exists := expressionMappingsMap[key]; !exists { - expressionMappingsMap[key] = &ExpressionMapping{ - EnvVar: key, - Content: fmt.Sprintf("'%s'", value), // Wrap in quotes for substitution - } + expressionMappingsMap[key] = &ExpressionMapping{EnvVar: key, Content: content} } + continue + } + if _, exists := expressionMappingsMap[key]; !exists { + expressionMappingsMap[key] = &ExpressionMapping{EnvVar: key, Content: fmt.Sprintf("'%s'", value)} } } } - - // Add environment variables from user prompt expressions (these override built-in ones) for _, mapping := range expressionMappings { allEnvVars[mapping.EnvVar] = fmt.Sprintf("${{ %s }}", mapping.Content) expressionMappingsMap[mapping.EnvVar] = mapping } + allMappings := make([]*ExpressionMapping, 0, len(expressionMappingsMap)) + for _, key := range sliceutil.SortedKeys(expressionMappingsMap) { + allMappings = append(allMappings, expressionMappingsMap[key]) + } + return allEnvVars, allMappings +} - // Convert map back to slice for the substitution step - allExpressionMappings := make([]*ExpressionMapping, 0, len(expressionMappingsMap)) +type promptYAMLWriter struct { + yaml *strings.Builder + delimiter string + inHeredoc bool + systemTagPending bool + userBlankRun int +} - // Sort the keys to ensure stable output - sortedKeys := sliceutil.SortedKeys(expressionMappingsMap) +func newPromptYAMLWriter(yaml *strings.Builder, delimiter string, hasBuiltinSections bool) *promptYAMLWriter { + return &promptYAMLWriter{yaml: yaml, delimiter: delimiter, systemTagPending: hasBuiltinSections} +} - // Add mappings in sorted order - for _, key := range sortedKeys { - allExpressionMappings = append(allExpressionMappings, expressionMappingsMap[key]) +func (w *promptYAMLWriter) openHeredoc(indent string) { + if w.inHeredoc { + return } + w.yaml.WriteString(indent + "cat << '" + w.delimiter + "'\n") + w.inHeredoc = true +} - // Generate the step with all environment variables - yaml.WriteString(" - name: Create prompt with built-in context\n") - yaml.WriteString(" env:\n") - yaml.WriteString(" GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt\n") - - if data.SafeOutputs != nil { - yaml.WriteString(" GH_AW_SAFE_OUTPUTS: ${{ runner.temp }}/gh-aw/safeoutputs/outputs.jsonl\n") +func (w *promptYAMLWriter) closeHeredoc(indent string) { + if !w.inHeredoc { + return } + w.yaml.WriteString(indent + w.delimiter + "\n") + w.inHeredoc = false +} - // Add all environment variables in sorted order for consistency - envKeys := sliceutil.SortedKeys(allEnvVars) - for _, key := range envKeys { - fmt.Fprintf(yaml, " %s: %s\n", key, allEnvVars[key]) +func cleanedPromptLines(content string) []string { + normalizedContent := stringutil.NormalizeLeadingWhitespace(content) + cleanedContent := removeConsecutiveEmptyLines(normalizedContent) + lines := make([]string, 0) + for line := range strings.SplitSeq(cleanedContent, "\n") { + lines = append(lines, line) } + return lines +} - yaml.WriteString(" # poutine:ignore untrusted_checkout_exec\n") - yaml.WriteString(" run: |\n") - yaml.WriteString(" bash \"${RUNNER_TEMP}/gh-aw/actions/create_prompt_first.sh\"\n") - yaml.WriteString(" {\n") - - // Track if we're inside a heredoc - inHeredoc := false +func (w *promptYAMLWriter) writeConditionalBuiltinSection(section PromptSection) { + w.closeHeredoc(" ") + if w.systemTagPending { + w.yaml.WriteString(" cat << '" + w.delimiter + "'\n") + w.yaml.WriteString(" \n") + w.yaml.WriteString(" " + w.delimiter + "\n") + w.systemTagPending = false + } + fmt.Fprintf(w.yaml, " if %s; then\n", section.ShellCondition) + if section.IsFile { + fmt.Fprintf(w.yaml, " cat \"%s/%s\"\n", promptsDir, section.Content) + } else { + w.yaml.WriteString(" cat << '" + w.delimiter + "'\n") + for _, line := range cleanedPromptLines(section.Content) { + w.yaml.WriteString(" " + line + "\n") + } + w.yaml.WriteString(" " + w.delimiter + "\n") + } + w.yaml.WriteString(" fi\n") +} - // 1. Write built-in sections first (prepended), wrapped in tags. - // The opening tag is deferred: it is written either as the first line - // of the first inline section's heredoc, or in its own block just before the - // first file or conditional section. This allows the opening tag to share a - // heredoc block with adjacent inline content, reducing the total number of blocks. - systemTagPending := len(builtinSections) > 0 +func (w *promptYAMLWriter) writeUnconditionalBuiltinSection(section PromptSection) { + if section.IsFile { + w.closeHeredoc(" ") + if w.systemTagPending { + w.yaml.WriteString(" cat << '" + w.delimiter + "'\n") + w.yaml.WriteString(" \n") + w.yaml.WriteString(" " + w.delimiter + "\n") + w.systemTagPending = false + } + fmt.Fprintf(w.yaml, " cat \"%s/%s\"\n", promptsDir, section.Content) + return + } + w.openHeredoc(" ") + if w.systemTagPending { + w.yaml.WriteString(" \n") + w.systemTagPending = false + } + for _, line := range cleanedPromptLines(section.Content) { + w.yaml.WriteString(" " + line + "\n") + } +} +func (w *promptYAMLWriter) writeBuiltinSections(builtinSections []PromptSection) { for i, section := range builtinSections { - unifiedPromptLog.Printf("Writing built-in section %d/%d: hasCondition=%v, isFile=%v", - i+1, len(builtinSections), section.ShellCondition != "", section.IsFile) - + unifiedPromptLog.Printf("Writing built-in section %d/%d: hasCondition=%v, isFile=%v", i+1, len(builtinSections), section.ShellCondition != "", section.IsFile) if section.ShellCondition != "" { - // Close heredoc if open, add conditional - if inHeredoc { - yaml.WriteString(" " + delimiter + "\n") - inHeredoc = false - } - // Write before conditional if still pending - if systemTagPending { - yaml.WriteString(" cat << '" + delimiter + "'\n") - yaml.WriteString(" \n") - yaml.WriteString(" " + delimiter + "\n") - systemTagPending = false - } - fmt.Fprintf(yaml, " if %s; then\n", section.ShellCondition) - - if section.IsFile { - // File reference inside conditional - promptPath := fmt.Sprintf("%s/%s", promptsDir, section.Content) - yaml.WriteString(" " + fmt.Sprintf("cat \"%s\"\n", promptPath)) - } else { - // Inline content inside conditional - open heredoc, write content, close - yaml.WriteString(" cat << '" + delimiter + "'\n") - normalizedContent := stringutil.NormalizeLeadingWhitespace(section.Content) - cleanedContent := removeConsecutiveEmptyLines(normalizedContent) - contentLines := strings.SplitSeq(cleanedContent, "\n") - for line := range contentLines { - yaml.WriteString(" " + line + "\n") - } - yaml.WriteString(" " + delimiter + "\n") - } - - yaml.WriteString(" fi\n") - } else { - // Unconditional section - if section.IsFile { - // Close heredoc if open - if inHeredoc { - yaml.WriteString(" " + delimiter + "\n") - inHeredoc = false - } - // Write before file if still pending - if systemTagPending { - yaml.WriteString(" cat << '" + delimiter + "'\n") - yaml.WriteString(" \n") - yaml.WriteString(" " + delimiter + "\n") - systemTagPending = false - } - // Cat the file - promptPath := fmt.Sprintf("%s/%s", promptsDir, section.Content) - yaml.WriteString(" " + fmt.Sprintf("cat \"%s\"\n", promptPath)) - } else { - // Inline content - open heredoc if not already open - if !inHeredoc { - yaml.WriteString(" cat << '" + delimiter + "'\n") - inHeredoc = true - // Write as first line when opening the heredoc - if systemTagPending { - yaml.WriteString(" \n") - systemTagPending = false - } - } - // Write content directly to open heredoc - normalizedContent := stringutil.NormalizeLeadingWhitespace(section.Content) - cleanedContent := removeConsecutiveEmptyLines(normalizedContent) - contentLines := strings.SplitSeq(cleanedContent, "\n") - for line := range contentLines { - yaml.WriteString(" " + line + "\n") - } - } + w.writeConditionalBuiltinSection(section) + continue } + w.writeUnconditionalBuiltinSection(section) } - - // Close tag after all built-in sections. - // Merge with the open heredoc (if any) to minimise the total number of cat/heredoc - // blocks, which reduces the number of lines that change in the diff when the user - // prompt changes (each block boundary contributes two delimiter lines). - if len(builtinSections) > 0 { - if inHeredoc { - // Append to the still-open heredoc and keep it open for - // the user content that follows. - yaml.WriteString(" \n") - } else { - // No heredoc is open: start a new one for and keep it - // open so the subsequent user content lands in the same block. - yaml.WriteString(" cat << '" + delimiter + "'\n") - yaml.WriteString(" \n") - inHeredoc = true - } + if len(builtinSections) == 0 { + return } + if w.inHeredoc { + w.yaml.WriteString(" \n") + return + } + w.yaml.WriteString(" cat << '" + w.delimiter + "'\n") + w.yaml.WriteString(" \n") + w.inHeredoc = true +} - // 2. Write user prompt chunks (appended after built-in sections). - // All chunks are written into the same heredoc block (opened above or here) - // to minimise the number of delimiter lines in the compiled lock file. - // - // The heredoc payload is a YAML block scalar, so normalizeBlankLines preserves - // it verbatim (it must, since arbitrary block scalars can carry semantically - // significant trailing whitespace and blank runs). Prompt content is markdown - // text the compiler owns, where trailing whitespace is never meaningful and - // long blank runs are noise, so it is cleaned here instead: trailing whitespace - // is trimmed from every line and consecutive blank lines are capped at - // maxConsecutiveBlankLines. userBlankRun is tracked across chunks so a run that - // straddles a chunk boundary is still collapsed. - userBlankRun := 0 +func (w *promptYAMLWriter) writeUserPromptChunks(userPromptChunks []string) { for chunkIdx, chunk := range userPromptChunks { unifiedPromptLog.Printf("Writing user prompt chunk %d/%d", chunkIdx+1, len(userPromptChunks)) - - // Check if this chunk is a runtime-import macro if strings.HasPrefix(chunk, "{{#runtime-import ") && strings.HasSuffix(chunk, "}}") { - // Runtime-import macros are plain text lines processed by the - // interpolate-prompt step; they can live in the same heredoc block - // as surrounding content. unifiedPromptLog.Print("Detected runtime-import macro, writing inline in heredoc") - - if !inHeredoc { - yaml.WriteString(" cat << '" + delimiter + "'\n") - inHeredoc = true - } - yaml.WriteString(" " + chunk + "\n") - userBlankRun = 0 + w.openHeredoc(" ") + w.yaml.WriteString(" " + chunk + "\n") + w.userBlankRun = 0 continue } - - // Regular chunk: write to the current heredoc (or open one). - if !inHeredoc { - yaml.WriteString(" cat << '" + delimiter + "'\n") - inHeredoc = true - } - - lines := strings.SplitSeq(chunk, "\n") - for line := range lines { + w.openHeredoc(" ") + for line := range strings.SplitSeq(chunk, "\n") { trimmed := strings.TrimRight(line, " \t") if trimmed == "" { - // Collapse over-long blank runs; emit truly empty lines (no - // indentation) so they carry no trailing whitespace. - if userBlankRun >= maxConsecutiveBlankLines { + if w.userBlankRun >= maxConsecutiveBlankLines { continue } - userBlankRun++ - yaml.WriteByte('\n') + w.userBlankRun++ + w.yaml.WriteByte('\n') continue } - userBlankRun = 0 - yaml.WriteString(" ") - yaml.WriteString(trimmed) - yaml.WriteByte('\n') + w.userBlankRun = 0 + w.yaml.WriteString(" " + trimmed + "\n") } } +} - // Close heredoc if still open - if inHeredoc { - yaml.WriteString(" " + delimiter + "\n") +func writePromptStepHeader(yaml *strings.Builder, data *WorkflowData, allEnvVars map[string]string) { + yaml.WriteString(" - name: Create prompt with built-in context\n") + yaml.WriteString(" env:\n") + yaml.WriteString(" GH_AW_PROMPT: /tmp/gh-aw/aw-prompts/prompt.txt\n") + if data.SafeOutputs != nil { + yaml.WriteString(" GH_AW_SAFE_OUTPUTS: ${{ runner.temp }}/gh-aw/safeoutputs/outputs.jsonl\n") } + for _, key := range sliceutil.SortedKeys(allEnvVars) { + fmt.Fprintf(yaml, " %s: %s\n", key, allEnvVars[key]) + } + yaml.WriteString(" # poutine:ignore untrusted_checkout_exec\n") + yaml.WriteString(" run: |\n") + yaml.WriteString(" bash \"${RUNNER_TEMP}/gh-aw/actions/create_prompt_first.sh\"\n") + yaml.WriteString(" {\n") +} + +func writePromptYAMLStep(yaml *strings.Builder, data *WorkflowData, allEnvVars map[string]string, builtinSections []PromptSection, userPromptChunks []string, delimiter string) { + writePromptStepHeader(yaml, data, allEnvVars) + writer := newPromptYAMLWriter(yaml, delimiter, len(builtinSections) > 0) + writer.writeBuiltinSections(builtinSections) + writer.writeUserPromptChunks(userPromptChunks) + writer.closeHeredoc(" ") yaml.WriteString(" } > \"$GH_AW_PROMPT\"\n") +} +// generateUnifiedPromptCreationStep generates a single workflow step (or multiple if needed) that creates +// the complete prompt file with built-in context instructions prepended to the user prompt content. +// +// This consolidates the prompt creation process: +// 1. Built-in context instructions (temp folder, playwright, safe outputs, etc.) - PREPENDED +// 2. User prompt content from markdown - APPENDED +// +// The function handles chunking for large content and ensures proper environment variable handling. +// Returns the combined expression mappings for use in the placeholder substitution step. +func (c *Compiler) generateUnifiedPromptCreationStep(yaml *strings.Builder, builtinSections []PromptSection, userPromptChunks []string, expressionMappings []*ExpressionMapping, data *WorkflowData) []*ExpressionMapping { + unifiedPromptLog.Print("Generating unified prompt creation step") + unifiedPromptLog.Printf("Built-in sections: %d, User prompt chunks: %d", len(builtinSections), len(userPromptChunks)) + delimiter := buildPromptDelimiter(builtinSections, userPromptChunks) + allEnvVars, allExpressionMappings := collectEnvVarsAndMappings(builtinSections, expressionMappings) + writePromptYAMLStep(yaml, data, allEnvVars, builtinSections, userPromptChunks, delimiter) unifiedPromptLog.Print("Unified prompt creation step generated successfully") - - // Return all expression mappings for use in the placeholder substitution step - // This allows the substitution to happen AFTER runtime-import processing return allExpressionMappings } @@ -540,27 +413,7 @@ func toolWithMaxBudget(name string, max *string) string { return fmt.Sprintf("%s(max:%s)", name, *max) } -// buildSafeOutputsSections returns the PromptSections that form the block. -// The block contains: -// 1. An inline opening tag with a compact Tools list (dynamic, depends on which tools are enabled). -// Any ${{ }} expressions in max: values are extracted to GH_AW_* env vars and replaced -// with __GH_AW_*__ placeholders so they do not appear in the run: heredoc, avoiding the -// GitHub Actions 21KB expression-size limit. -// 2. File references for tools that require multi-step instructions (create_pull_request, -// push_to_pull_request_branch, auto-injected create_issue notice). -// 3. An inline closing tag. -// -// The static intro (gh CLI warning, temporary ID rules, noop note) lives in -// actions/setup/md/safe_outputs_prompt.md and is included by the caller before these sections. -func buildSafeOutputsSections(safeOutputs *SafeOutputsConfig) []PromptSection { - if safeOutputs == nil { - return nil - } - - safeOutputsPromptLog.Print("Building safe outputs sections") - - // Build compact list of enabled tool names, annotated with max budget when > 1. - var tools []string +func appendIssueAndDiscussionSafeOutputTools(tools []string, safeOutputs *SafeOutputsConfig) []string { if safeOutputs.AddComments != nil { tools = append(tools, toolWithMaxBudget("add_comment", safeOutputs.AddComments.Max)) } @@ -585,6 +438,10 @@ func buildSafeOutputsSections(safeOutputs *SafeOutputsConfig) []PromptSection { if safeOutputs.CreateAgentSessions != nil { tools = append(tools, toolWithMaxBudget("create_agent_session", safeOutputs.CreateAgentSessions.Max)) } + return tools +} + +func appendPullRequestSafeOutputTools(tools []string, safeOutputs *SafeOutputsConfig) []string { if safeOutputs.CreatePullRequests != nil { tools = append(tools, toolWithMaxBudget("create_pull_request", safeOutputs.CreatePullRequests.Max)) } @@ -612,6 +469,10 @@ func buildSafeOutputsSections(safeOutputs *SafeOutputsConfig) []PromptSection { if safeOutputs.ResolvePullRequestReviewThread != nil { tools = append(tools, toolWithMaxBudget("resolve_pull_request_review_thread", safeOutputs.ResolvePullRequestReviewThread.Max)) } + return tools +} + +func appendAdministrativeSafeOutputTools(tools []string, safeOutputs *SafeOutputsConfig) []string { if safeOutputs.AddLabels != nil { tools = append(tools, toolWithMaxBudget("add_labels", safeOutputs.AddLabels.Max)) } @@ -654,6 +515,10 @@ func buildSafeOutputsSections(safeOutputs *SafeOutputsConfig) []PromptSection { if safeOutputs.UpdateRelease != nil { tools = append(tools, toolWithMaxBudget("update_release", safeOutputs.UpdateRelease.Max)) } + return tools +} + +func appendWorkflowAndProjectSafeOutputTools(tools []string, safeOutputs *SafeOutputsConfig) []string { if safeOutputs.UpdateProjects != nil { tools = append(tools, toolWithMaxBudget("update_project", safeOutputs.UpdateProjects.Max)) } @@ -679,7 +544,6 @@ func buildSafeOutputsSections(safeOutputs *SafeOutputsConfig) []PromptSection { tools = append(tools, toolWithMaxBudget("dispatch_workflow", safeOutputs.DispatchWorkflow.Max)) } if safeOutputs.DispatchRepository != nil { - // dispatch_repository uses per-tool max values (map-of-tools pattern); no top-level max. tools = append(tools, "dispatch_repository") } if safeOutputs.CallWorkflow != nil { @@ -691,67 +555,41 @@ func buildSafeOutputsSections(safeOutputs *SafeOutputsConfig) []PromptSection { if safeOutputs.MissingData != nil { tools = append(tools, toolWithMaxBudget("missing_data", safeOutputs.MissingData.Max)) } - // noop is always included: it is auto-injected by extractSafeOutputsConfig and - // must always appear in the tools list so agents can signal no-op completion. if safeOutputs.NoOp != nil { tools = append(tools, toolWithMaxBudget("noop", safeOutputs.NoOp.Max)) } + return tools +} - // Add custom job tools from SafeOutputs.Jobs (sorted for deterministic output). - if len(safeOutputs.Jobs) > 0 { - jobNames := sliceutil.SortedKeys(safeOutputs.Jobs) - for _, jobName := range jobNames { - tools = append(tools, stringutil.NormalizeSafeOutputIdentifier(jobName)) - } - } - - // Add custom script tools from SafeOutputs.Scripts (sorted for deterministic output). - if len(safeOutputs.Scripts) > 0 { - scriptNames := sliceutil.SortedKeys(safeOutputs.Scripts) - for _, scriptName := range scriptNames { - tools = append(tools, stringutil.NormalizeSafeOutputIdentifier(scriptName)) - } +func appendCustomSafeOutputToolNames(tools []string, safeOutputs *SafeOutputsConfig) []string { + for _, jobName := range sliceutil.SortedKeys(safeOutputs.Jobs) { + tools = append(tools, stringutil.NormalizeSafeOutputIdentifier(jobName)) } - - // Add custom action tools from SafeOutputs.Actions (sorted for deterministic output). - if len(safeOutputs.Actions) > 0 { - actionNames := sliceutil.SortedKeys(safeOutputs.Actions) - for _, actionName := range actionNames { - tools = append(tools, stringutil.NormalizeSafeOutputIdentifier(actionName)) - } + for _, scriptName := range sliceutil.SortedKeys(safeOutputs.Scripts) { + tools = append(tools, stringutil.NormalizeSafeOutputIdentifier(scriptName)) } - - if len(tools) == 0 { - return nil + for _, actionName := range sliceutil.SortedKeys(safeOutputs.Actions) { + tools = append(tools, stringutil.NormalizeSafeOutputIdentifier(actionName)) } + return tools +} - var sections []PromptSection - - // Build the inline opening: XML tag + compact tools list. - // Extract any ${{ }} expressions from max: values so they do not appear in the - // run: heredoc (which is subject to GitHub Actions' 21KB expression-size limit). - // Expressions are replaced with __GH_AW_...__ placeholders and added to EnvVars - // so the placeholder substitution step can resolve them at runtime. +func buildSafeOutputsOpeningSection(tools []string) PromptSection { toolsContent := "\nTools: " + strings.Join(tools, ", ") envVars := make(map[string]string) extractor := NewExpressionExtractor() - exprMappings, err := extractor.ExtractExpressions(toolsContent) - if err == nil && len(exprMappings) > 0 { + if exprMappings, err := extractor.ExtractExpressions(toolsContent); err == nil && len(exprMappings) > 0 { safeOutputsPromptLog.Printf("Extracted %d expression(s) from safe-output-tools block", len(exprMappings)) toolsContent = extractor.ReplaceExpressionsWithEnvVars(toolsContent) for _, mapping := range exprMappings { envVars[mapping.EnvVar] = fmt.Sprintf("${{ %s }}", mapping.Content) } } + return PromptSection{Content: toolsContent, EnvVars: envVars} +} - // Inline opening: XML tag + compact tools list (with placeholders for any expressions) - sections = append(sections, PromptSection{ - Content: toolsContent, - IsFile: false, - EnvVars: envVars, - }) - - // File sections for tools with multi-step instructions +func buildSafeOutputsInstructionSections(safeOutputs *SafeOutputsConfig) []PromptSection { + sections := make([]PromptSection, 0, 5) if safeOutputs.CreatePullRequests != nil { sections = append(sections, PromptSection{Content: safeOutputsCreatePRFile, IsFile: true}) } @@ -762,21 +600,42 @@ func buildSafeOutputsSections(safeOutputs *SafeOutputsConfig) []PromptSection { sections = append(sections, PromptSection{Content: safeOutputsCommentMemoryFile, IsFile: true}) } if safeOutputs.UploadAssets != nil { - sections = append(sections, PromptSection{ - Content: "\nupload_asset: provide a file path; returns a URL; assets are published after the workflow completes (" + constants.SafeOutputsMCPServerID.String() + ").", - IsFile: false, - }) + sections = append(sections, PromptSection{Content: "\nupload_asset: provide a file path; returns a URL; assets are published after the workflow completes (" + constants.SafeOutputsMCPServerID.String() + ")."}) } - // Auto-injected create_issue special notice if safeOutputs.CreateIssues != nil && safeOutputs.AutoInjectedCreateIssue { sections = append(sections, PromptSection{Content: safeOutputsAutoCreateIssueFile, IsFile: true}) } + return sections +} - // Inline closing tag - sections = append(sections, PromptSection{ - Content: "", - IsFile: false, - }) +// buildSafeOutputsSections returns the PromptSections that form the block. +// The block contains: +// 1. An inline opening tag with a compact Tools list (dynamic, depends on which tools are enabled). +// Any ${{ }} expressions in max: values are extracted to GH_AW_* env vars and replaced +// with __GH_AW_*__ placeholders so they do not appear in the run: heredoc, avoiding the +// GitHub Actions 21KB expression-size limit. +// 2. File references for tools that require multi-step instructions (create_pull_request, +// push_to_pull_request_branch, auto-injected create_issue notice). +// 3. An inline closing tag. +// +// The static intro (gh CLI warning, temporary ID rules, noop note) lives in +// actions/setup/md/safe_outputs_prompt.md and is included by the caller before these sections. +func buildSafeOutputsSections(safeOutputs *SafeOutputsConfig) []PromptSection { + if safeOutputs == nil { + return nil + } + safeOutputsPromptLog.Print("Building safe outputs sections") + tools := appendIssueAndDiscussionSafeOutputTools(nil, safeOutputs) + tools = appendPullRequestSafeOutputTools(tools, safeOutputs) + tools = appendAdministrativeSafeOutputTools(tools, safeOutputs) + tools = appendWorkflowAndProjectSafeOutputTools(tools, safeOutputs) + tools = appendCustomSafeOutputToolNames(tools, safeOutputs) + if len(tools) == 0 { + return nil + } + sections := []PromptSection{buildSafeOutputsOpeningSection(tools)} + sections = append(sections, buildSafeOutputsInstructionSections(safeOutputs)...) + sections = append(sections, PromptSection{Content: ""}) return sections }