diff --git a/pkg/cli/bootstrap_profile_github_app.go b/pkg/cli/bootstrap_profile_github_app.go index aa56ea81410..ccebda8c42d 100644 --- a/pkg/cli/bootstrap_profile_github_app.go +++ b/pkg/cli/bootstrap_profile_github_app.go @@ -39,8 +39,8 @@ func runBootstrapGitHubAppAction(ctx context.Context, repo string, action reposi return nil, err } - clientID := strings.TrimSpace(os.Getenv(bootstrapGitHubAppClientIDEnv)) - privateKey := strings.TrimRight(os.Getenv(bootstrapGitHubAppPrivateKeyEnv), "\r\n") + clientID := strings.TrimSpace(lookupEnv(bootstrapGitHubAppClientIDEnv)) + privateKey := strings.TrimRight(lookupEnv(bootstrapGitHubAppPrivateKeyEnv), "\r\n") handled, err := handleBootstrapGitHubAppExistingFlow(ctx, repo, action, overrides, clientID, privateKey) if err != nil { return nil, err @@ -324,14 +324,14 @@ func buildBootstrapGitHubAppMux(ctx context.Context, csrfState, owner, ownerType func loadBootstrapGitHubAppOverrides() (bootstrapGitHubAppOverrides, error) { overrides := bootstrapGitHubAppOverrides{ Mode: "", - Owner: strings.TrimSpace(os.Getenv(bootstrapGitHubAppOwnerEnv)), - Name: strings.TrimSpace(os.Getenv(bootstrapGitHubAppNameEnv)), - HomepageURL: strings.TrimSpace(os.Getenv(bootstrapGitHubAppURLEnv)), - Description: strings.TrimSpace(os.Getenv(bootstrapGitHubAppDescriptionEnv)), + Owner: strings.TrimSpace(lookupEnv(bootstrapGitHubAppOwnerEnv)), + Name: strings.TrimSpace(lookupEnv(bootstrapGitHubAppNameEnv)), + HomepageURL: strings.TrimSpace(lookupEnv(bootstrapGitHubAppURLEnv)), + Description: strings.TrimSpace(lookupEnv(bootstrapGitHubAppDescriptionEnv)), OpenBrowser: true, } - switch mode := strings.ToLower(strings.TrimSpace(os.Getenv(bootstrapGitHubAppModeEnv))); mode { + switch mode := strings.ToLower(strings.TrimSpace(lookupEnv(bootstrapGitHubAppModeEnv))); mode { case "", "auto": case "create", "existing": overrides.Mode = mode @@ -339,7 +339,7 @@ func loadBootstrapGitHubAppOverrides() (bootstrapGitHubAppOverrides, error) { return bootstrapGitHubAppOverrides{}, fmt.Errorf("%s must be one of: auto, create, existing. Example: export %s=create", bootstrapGitHubAppModeEnv, bootstrapGitHubAppModeEnv) } - if raw := strings.TrimSpace(os.Getenv(bootstrapNoOpenBrowserEnv)); raw != "" { + if raw := strings.TrimSpace(lookupEnv(bootstrapNoOpenBrowserEnv)); raw != "" { disabled, err := parseBootstrapBool(raw) if err != nil { return bootstrapGitHubAppOverrides{}, fmt.Errorf("%s: %w", bootstrapNoOpenBrowserEnv, err) diff --git a/pkg/cli/bootstrap_profile_helpers.go b/pkg/cli/bootstrap_profile_helpers.go index 5b4c9c9183f..1af2099c04b 100644 --- a/pkg/cli/bootstrap_profile_helpers.go +++ b/pkg/cli/bootstrap_profile_helpers.go @@ -74,7 +74,7 @@ func parseBootstrapNames(output []byte) []string { func resolveBootstrapTextValue(envName, title, description, defaultValue string, allowed []string, optional bool) (string, bool, error) { bootstrapProfileHelpersLog.Printf("Resolving text value: env=%s, optional=%v, hasDefault=%v", envName, optional, defaultValue != "") - if envValue := strings.TrimSpace(os.Getenv(envName)); envValue != "" { + if envValue := strings.TrimSpace(lookupEnv(envName)); envValue != "" { bootstrapProfileHelpersLog.Printf("Resolved %s from environment variable", envName) if err := validateBootstrapEnumValue(envValue, allowed, optional); err != nil { return "", false, err @@ -127,7 +127,7 @@ func resolveBootstrapTextValue(envName, title, description, defaultValue string, } func resolveBootstrapSecretValue(envName, title, description string, optional bool) (string, bool, error) { - if envValue := strings.TrimRight(os.Getenv(envName), "\r\n"); envValue != "" { + if envValue := strings.TrimRight(lookupEnv(envName), "\r\n"); envValue != "" { return envValue, true, nil } if !tty.IsStderrTerminal() { diff --git a/pkg/cli/bootstrap_profile_manifest.go b/pkg/cli/bootstrap_profile_manifest.go index 747ea5c62df..5c34ae97656 100644 --- a/pkg/cli/bootstrap_profile_manifest.go +++ b/pkg/cli/bootstrap_profile_manifest.go @@ -263,9 +263,9 @@ func stringMapValue(value any) (map[string]string, error) { func manifestBootstrapFieldError(manifestPath string, index int, field string, err error) error { if example, ok := manifestBootstrapFieldExample(field); ok { - return fmt.Errorf("invalid Agentic Workflow manifest %q: config[%d].%s %s. Example: config[%d].%s: %s", manifestPath, index, field, err.Error(), index, field, example) + return fmt.Errorf("invalid Agentic Workflow manifest %q: config[%d].%s %w. Example: config[%d].%s: %s", manifestPath, index, field, err, index, field, example) } - return fmt.Errorf("invalid Agentic Workflow manifest %q: config[%d].%s %s", manifestPath, index, field, err.Error()) + return fmt.Errorf("invalid Agentic Workflow manifest %q: config[%d].%s %w", manifestPath, index, field, err) } func manifestBootstrapFieldExample(field string) (string, bool) { diff --git a/pkg/cli/copilot_agents.go b/pkg/cli/copilot_agents.go index 9cc3bdd9227..9ee5023de9a 100644 --- a/pkg/cli/copilot_agents.go +++ b/pkg/cli/copilot_agents.go @@ -24,6 +24,8 @@ var copilotAgentsLog = logger.New("cli:copilot_agents") const agenticWorkflowsSkillFileListPlaceholder = "{{AW_FILE_LIST}}" const ghAWMarkdownFilesAPIURL = "https://api.github.com/repos/github/gh-aw/contents/.github/aw?ref=main" +const agenticWorkflowsSkillDirDescription = ".github/skills/agentic-workflows directory" +const agenticWorkflowsAgentDirDescription = ".github/agents directory" //go:embed data/agentic_workflows_agent.md var agenticWorkflowsAgentTemplate string @@ -84,7 +86,7 @@ func ensureAgenticWorkflowsDispatcher(verbose bool, skipInstructions bool, write return nil } - if err := writeGeneratedRepositoryInstructionFile(targetPath, []byte(skillContent), write, ".github/skills/agentic-workflows directory", "dispatcher skill"); err != nil { + if err := writeGeneratedRepositoryInstructionFile(targetPath, []byte(skillContent), write, agenticWorkflowsSkillDirDescription, "dispatcher skill"); err != nil { copilotAgentsLog.Printf("Failed to write dispatcher skill: %s, error: %v", targetPath, err) return fmt.Errorf("failed to write dispatcher skill: %w", err) } @@ -144,7 +146,7 @@ func ensureAgenticWorkflowsAgent(verbose bool, write bool) error { return nil } - if err := writeGeneratedRepositoryInstructionFile(targetPath, []byte(agenticWorkflowsAgentContent), write, ".github/agents directory", "Agentic Workflows custom agent"); err != nil { + if err := writeGeneratedRepositoryInstructionFile(targetPath, []byte(agenticWorkflowsAgentContent), write, agenticWorkflowsAgentDirDescription, "Agentic Workflows custom agent"); err != nil { return fmt.Errorf("failed to write Agentic Workflows custom agent: %w", err) } diff --git a/pkg/cli/mcp_inspect_inspector.go b/pkg/cli/mcp_inspect_inspector.go index f30c47eea6f..33324d654f7 100644 --- a/pkg/cli/mcp_inspect_inspector.go +++ b/pkg/cli/mcp_inspect_inspector.go @@ -46,6 +46,35 @@ func spawnMCPInspector(ctx context.Context, workflowFile string, serverFilter st g, gctx := errgroup.WithContext(ctx) + // Set up cleanup function for stdio servers. Registered here, before any + // server is started, so that early returns (e.g. context cancellation during + // the startup wait) still kill started processes and drain monitor goroutines. + defer func() { + if len(serverProcesses) > 0 { + mcpInspectorLog.Printf("Cleaning up %d MCP server processes", len(serverProcesses)) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Cleaning up MCP servers...")) + for i, cmd := range serverProcesses { + if cmd.Process != nil { + if err := cmd.Process.Kill(); err != nil && verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to kill server process %d: %v", cmd.Process.Pid, err))) + } + } + // Give each process a chance to clean up + if i < len(serverProcesses)-1 { + timer := time.NewTimer(mcpProcessCleanupDelay) + defer timer.Stop() + select { + case <-timer.C: + case <-gctx.Done(): + } + } + } + if err := g.Wait(); err != nil { + mcpInspectorLog.Printf("Error from MCP server monitor goroutine: %v", err) + } + } + }() + // If workflow file is specified, extract MCP configurations and start servers if workflowFile != "" { // Resolve the workflow file path (supports shared workflows) @@ -180,7 +209,11 @@ func spawnMCPInspector(ctx context.Context, workflowFile string, serverFilter st } // Give servers a moment to start up - time.Sleep(mcpStdioServerStartupDelay) + select { + case <-time.After(mcpStdioServerStartupDelay): + case <-gctx.Done(): + return gctx.Err() + } fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("All stdio servers started successfully")) } @@ -211,28 +244,6 @@ func spawnMCPInspector(ctx context.Context, workflowFile string, serverFilter st } } - // Set up cleanup function for stdio servers - defer func() { - if len(serverProcesses) > 0 { - mcpInspectorLog.Printf("Cleaning up %d MCP server processes", len(serverProcesses)) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Cleaning up MCP servers...")) - for i, cmd := range serverProcesses { - if cmd.Process != nil { - if err := cmd.Process.Kill(); err != nil && verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to kill server process %d: %v", cmd.Process.Pid, err))) - } - } - // Give each process a chance to clean up - if i < len(serverProcesses)-1 { - time.Sleep(mcpProcessCleanupDelay) - } - } - if err := g.Wait(); err != nil { - mcpInspectorLog.Printf("Error from MCP server monitor goroutine: %v", err) - } - } - }() - mcpInspectorLog.Print("Launching @modelcontextprotocol/inspector") fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Launching @modelcontextprotocol/inspector...")) fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Visit http://localhost:5173 after the inspector starts")) diff --git a/pkg/workflow/github_cli.go b/pkg/workflow/github_cli.go index 50b0aac36a4..d3b08563a08 100644 --- a/pkg/workflow/github_cli.go +++ b/pkg/workflow/github_cli.go @@ -103,8 +103,7 @@ func filteredGHCLIEnv(ghToken, githubToken, ghHost string) []string { // cmd := ExecGH("api", "/user") // output, err := cmd.Output() func ExecGH(args ...string) *exec.Cmd { - //nolint:staticcheck // Passing nil context to use exec.Command instead of exec.CommandContext - return setupGHCommand(nil, args...) + return setupGHCommand(context.Background(), args...) } // ExecGHContext wraps gh CLI calls with context support and ensures proper token configuration. diff --git a/pkg/workflow/llm_provider.go b/pkg/workflow/llm_provider.go index dd134d688bb..5cb92705276 100644 --- a/pkg/workflow/llm_provider.go +++ b/pkg/workflow/llm_provider.go @@ -31,10 +31,11 @@ type llmProviderProfile struct { } func normalizeLLMProvider(provider string) string { - normalized := strings.ToLower(strings.TrimSpace(provider)) - if len(normalized) == 0 { + normalized := strings.TrimSpace(provider) + if normalized == "" { return LLMProviderAnthropic } + normalized = strings.ToLower(normalized) if alias, ok := llmProviderAliases[normalized]; ok { return alias } diff --git a/pkg/workflow/safe_outputs_tools_generation.go b/pkg/workflow/safe_outputs_tools_generation.go index 99927df9573..dc0a7a84462 100644 --- a/pkg/workflow/safe_outputs_tools_generation.go +++ b/pkg/workflow/safe_outputs_tools_generation.go @@ -311,13 +311,13 @@ func computePropertyInjections(safeOutputs *SafeOutputsConfig) map[string]map[st // Validate each configured value against the supported API values so that // invalid strings (e.g. "done", "wontfix") are caught at compile time rather // than producing a GitHub API 422 at runtime. - supported := make(map[string]bool, len(closeIssueStateReasonValues)) + supported := make(map[string]struct{}, len(closeIssueStateReasonValues)) for _, v := range closeIssueStateReasonValues { - supported[v] = true + supported[v] = struct{}{} } valid := make([]string, 0, len(enumValues)) for _, v := range enumValues { - if supported[v] { + if _, ok := supported[v]; ok { valid = append(valid, v) } else { safeOutputsConfigLog.Printf("Warning: allowed-state-reason value %q is not a supported GitHub API value; valid values: %v", v, closeIssueStateReasonValues) diff --git a/pkg/workflow/workflow_builder.go b/pkg/workflow/workflow_builder.go index 280efeb03f4..65d02235efd 100644 --- a/pkg/workflow/workflow_builder.go +++ b/pkg/workflow/workflow_builder.go @@ -502,17 +502,17 @@ func mergeExcludedEnvVarNames(fromImports, fromMain []string) []string { // Use max() for capacity hints: overflow-safe (no addition) and a tighter // lower-bound than either length alone. hint := max(len(fromImports), len(fromMain)) - seen := make(map[string]bool, hint) + seen := make(map[string]struct{}, hint) merged := make([]string, 0, hint) for _, name := range fromImports { - if !seen[name] { - seen[name] = true + if _, ok := seen[name]; !ok { + seen[name] = struct{}{} merged = append(merged, name) } } for _, name := range fromMain { - if !seen[name] { - seen[name] = true + if _, ok := seen[name]; !ok { + seen[name] = struct{}{} merged = append(merged, name) } }