diff --git a/.changeset/patch-refactor-safe-output-handlers.md b/.changeset/patch-refactor-safe-output-handlers.md new file mode 100644 index 0000000000..698cdbbe01 --- /dev/null +++ b/.changeset/patch-refactor-safe-output-handlers.md @@ -0,0 +1,15 @@ +--- +"gh-aw": patch +--- + +Refactor safe output handlers into a sequential processing system using a +handler registry and explicit temporary ID map propagation. + +This change extracts individual safe-output handlers (create_issue, +create_discussion, add_comment, update_issue, update_discussion, +close_issue, close_discussion) into their own files and consolidates the +processing loop into an ordered registry that tracks temporary ID map +availability via a shared context. This is an internal refactor that +improves maintainability and paves the way for migrating additional +handlers in the future. + diff --git a/.github/workflows/campaign-generator.lock.yml b/.github/workflows/campaign-generator.lock.yml index 9d982ecada..3b2d99985a 100644 --- a/.github/workflows/campaign-generator.lock.yml +++ b/.github/workflows/campaign-generator.lock.yml @@ -1357,30 +1357,30 @@ jobs: mkdir -p /tmp/gh-aw/safeoutputs/ find "/tmp/gh-aw/safeoutputs/" -type f -print echo "GH_AW_AGENT_OUTPUT=/tmp/gh-aw/safeoutputs/agent_output.json" >> "$GITHUB_ENV" - - name: Assign To Agent - id: assign_to_agent - if: ((!cancelled()) && (needs.agent.result != 'skipped')) && (contains(needs.agent.outputs.output_types, 'assign_to_agent')) + - name: Update Issue + id: update_issue + if: ((!cancelled()) && (needs.agent.result != 'skipped')) && (contains(needs.agent.outputs.output_types, 'update_issue')) uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 env: GH_AW_AGENT_OUTPUT: ${{ env.GH_AW_AGENT_OUTPUT }} with: - github-token: ${{ secrets.GH_AW_AGENT_TOKEN }} + github-token: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} script: | const { setupGlobals } = require('/tmp/gh-aw/actions/setup_globals.cjs'); setupGlobals(core, github, context, exec, io); - const { main } = require('/tmp/gh-aw/actions/assign_to_agent.cjs'); + const { main } = require('/tmp/gh-aw/actions/update_issue.cjs'); await main(); - - name: Update Issue - id: update_issue - if: ((!cancelled()) && (needs.agent.result != 'skipped')) && (contains(needs.agent.outputs.output_types, 'update_issue')) + - name: Assign To Agent + id: assign_to_agent + if: ((!cancelled()) && (needs.agent.result != 'skipped')) && (contains(needs.agent.outputs.output_types, 'assign_to_agent')) uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 env: GH_AW_AGENT_OUTPUT: ${{ env.GH_AW_AGENT_OUTPUT }} with: - github-token: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} + github-token: ${{ secrets.GH_AW_AGENT_TOKEN }} script: | const { setupGlobals } = require('/tmp/gh-aw/actions/setup_globals.cjs'); setupGlobals(core, github, context, exec, io); - const { main } = require('/tmp/gh-aw/actions/update_issue.cjs'); + const { main } = require('/tmp/gh-aw/actions/assign_to_agent.cjs'); await main(); diff --git a/pkg/workflow/compiler_safe_outputs_core.go b/pkg/workflow/compiler_safe_outputs_core.go index b11e62dac4..0003f3c848 100644 --- a/pkg/workflow/compiler_safe_outputs_core.go +++ b/pkg/workflow/compiler_safe_outputs_core.go @@ -51,9 +51,8 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa threatDetectionEnabled := data.SafeOutputs.ThreatDetection != nil // Track which outputs are created for dependency tracking + // These are tracked in the handler processing context now var createIssueEnabled bool - var createDiscussionEnabled bool - var createPullRequestEnabled bool // Add GitHub App token minting step if app is configured if data.SafeOutputs.App != nil { @@ -102,53 +101,33 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa // === Build individual safe output steps === - // 1. Create Issue step - if data.SafeOutputs.CreateIssues != nil { - createIssueEnabled = true - stepConfig := c.buildCreateIssueStepConfig(data, mainJobName, threatDetectionEnabled) - stepYAML := c.buildConsolidatedSafeOutputStep(data, stepConfig) - steps = append(steps, stepYAML...) - safeOutputStepNames = append(safeOutputStepNames, stepConfig.StepID) - - // Add outputs - outputs["create_issue_issue_number"] = "${{ steps.create_issue.outputs.issue_number }}" - outputs["create_issue_issue_url"] = "${{ steps.create_issue.outputs.issue_url }}" - outputs["create_issue_temporary_id_map"] = "${{ steps.create_issue.outputs.temporary_id_map }}" - - // Merge permissions - permissions.Merge(NewPermissionsContentsReadIssuesWrite()) - } - - // 2. Create Discussion step - if data.SafeOutputs.CreateDiscussions != nil { - createDiscussionEnabled = true - stepConfig := c.buildCreateDiscussionStepConfig(data, mainJobName, threatDetectionEnabled) - stepYAML := c.buildConsolidatedSafeOutputStep(data, stepConfig) - steps = append(steps, stepYAML...) - safeOutputStepNames = append(safeOutputStepNames, stepConfig.StepID) - - outputs["create_discussion_discussion_number"] = "${{ steps.create_discussion.outputs.discussion_number }}" - outputs["create_discussion_discussion_url"] = "${{ steps.create_discussion.outputs.discussion_url }}" - - permissions.Merge(NewPermissionsContentsReadDiscussionsWrite()) - } - - // 2a. Update Discussion step - if data.SafeOutputs.UpdateDiscussions != nil { - stepConfig := c.buildUpdateDiscussionStepConfig(data, mainJobName, threatDetectionEnabled) - stepYAML := c.buildConsolidatedSafeOutputStep(data, stepConfig) - steps = append(steps, stepYAML...) - safeOutputStepNames = append(safeOutputStepNames, stepConfig.StepID) - - outputs["update_discussion_discussion_number"] = "${{ steps.update_discussion.outputs.discussion_number }}" - outputs["update_discussion_discussion_url"] = "${{ steps.update_discussion.outputs.discussion_url }}" - - permissions.Merge(NewPermissionsContentsReadDiscussionsWrite()) + // Process migrated handlers through the new handler registry system + // This handles: create_issue, create_discussion, update_issue, update_discussion, add_comment + registry := getHandlerRegistry() + handlerSteps, handlerOutputs, handlerPermissions, handlerStepNames := c.processHandlersSequentially( + data, mainJobName, threatDetectionEnabled, registry) + + // Append handler steps and merge outputs/permissions + steps = append(steps, handlerSteps...) + for k, v := range handlerOutputs { + outputs[k] = v + } + permissions.Merge(handlerPermissions) + safeOutputStepNames = append(safeOutputStepNames, handlerStepNames...) + + // Track which handlers were processed for backward compatibility + for _, stepName := range handlerStepNames { + switch stepName { + case "create_issue": + createIssueEnabled = true + // Note: createDiscussionEnabled is no longer needed as dependencies + // are now managed through the SafeOutputContext + } } // 3. Create Pull Request step if data.SafeOutputs.CreatePullRequests != nil { - createPullRequestEnabled = true + // Note: createPullRequestEnabled tracking is now handled by SafeOutputContext stepConfig := c.buildCreatePullRequestStepConfig(data, mainJobName, threatDetectionEnabled) // Skip pre-steps if we've already added the shared checkout steps if !prCheckoutStepsAdded { @@ -165,39 +144,8 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa permissions.Merge(NewPermissionsContentsWriteIssuesWritePRWrite()) } - // 4. Add Comment step (needs to come after create_issue, create_discussion, create_pull_request) - if data.SafeOutputs.AddComments != nil { - stepConfig := c.buildAddCommentStepConfig(data, mainJobName, threatDetectionEnabled, - createIssueEnabled, createDiscussionEnabled, createPullRequestEnabled) - stepYAML := c.buildConsolidatedSafeOutputStep(data, stepConfig) - steps = append(steps, stepYAML...) - safeOutputStepNames = append(safeOutputStepNames, stepConfig.StepID) - - outputs["add_comment_comment_id"] = "${{ steps.add_comment.outputs.comment_id }}" - outputs["add_comment_comment_url"] = "${{ steps.add_comment.outputs.comment_url }}" - - permissions.Merge(NewPermissionsContentsReadIssuesWritePRWriteDiscussionsWrite()) - } - - // 5. Close Discussion step - if data.SafeOutputs.CloseDiscussions != nil { - stepConfig := c.buildCloseDiscussionStepConfig(data, mainJobName, threatDetectionEnabled) - stepYAML := c.buildConsolidatedSafeOutputStep(data, stepConfig) - steps = append(steps, stepYAML...) - safeOutputStepNames = append(safeOutputStepNames, stepConfig.StepID) - - permissions.Merge(NewPermissionsContentsReadDiscussionsWrite()) - } - - // 6. Close Issue step - if data.SafeOutputs.CloseIssues != nil { - stepConfig := c.buildCloseIssueStepConfig(data, mainJobName, threatDetectionEnabled) - stepYAML := c.buildConsolidatedSafeOutputStep(data, stepConfig) - steps = append(steps, stepYAML...) - safeOutputStepNames = append(safeOutputStepNames, stepConfig.StepID) - - permissions.Merge(NewPermissionsContentsReadIssuesWrite()) - } + // NOTE: Add Comment, Update Issue, Update Discussion, Close Issue, and Close Discussion + // are now handled by the handler registry above // 7. Close Pull Request step if data.SafeOutputs.ClosePullRequests != nil { @@ -290,16 +238,6 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa permissions.Merge(NewPermissionsContentsReadIssuesWritePRWrite()) } - // 15. Update Issue step - if data.SafeOutputs.UpdateIssues != nil { - stepConfig := c.buildUpdateIssueStepConfig(data, mainJobName, threatDetectionEnabled) - stepYAML := c.buildConsolidatedSafeOutputStep(data, stepConfig) - steps = append(steps, stepYAML...) - safeOutputStepNames = append(safeOutputStepNames, stepConfig.StepID) - - permissions.Merge(NewPermissionsContentsReadIssuesWrite()) - } - // 16. Update Pull Request step if data.SafeOutputs.UpdatePullRequests != nil { stepConfig := c.buildUpdatePullRequestStepConfig(data, mainJobName, threatDetectionEnabled) diff --git a/pkg/workflow/safe_output_handler.go b/pkg/workflow/safe_output_handler.go new file mode 100644 index 0000000000..77ac12b11b --- /dev/null +++ b/pkg/workflow/safe_output_handler.go @@ -0,0 +1,129 @@ +package workflow + +import ( + "github.com/githubnext/gh-aw/pkg/logger" +) + +var safeOutputHandlerLog = logger.New("workflow:safe_output_handler") + +// SafeOutputHandler defines the interface for safe output message handlers. +// Each handler is responsible for building the steps needed to process +// a specific type of safe output message (e.g., create_issue, add_comment). +type SafeOutputHandler interface { + // GetType returns the type identifier for this handler (e.g., "create_issue") + GetType() string + + // IsEnabled checks if this handler should be processed based on the workflow configuration + IsEnabled(data *WorkflowData) bool + + // BuildStepConfig builds the step configuration for this handler + // Returns nil if the handler is not enabled or cannot be built + BuildStepConfig(c *Compiler, data *WorkflowData, ctx *SafeOutputContext) *SafeOutputStepConfig + + // GetOutputs returns the outputs that this handler produces + // These are used to build the job outputs map + GetOutputs() map[string]string + + // RequiresTempIDMap returns true if this handler needs access to the temporary ID map + RequiresTempIDMap() bool +} + +// SafeOutputContext holds contextual information about previously processed handlers +// This allows handlers to reference outputs from earlier handlers in the sequence +type SafeOutputContext struct { + // ThreatDetectionEnabled indicates if threat detection is enabled + ThreatDetectionEnabled bool + + // MainJobName is the name of the main agent job + MainJobName string + + // ProcessedHandlers tracks which handlers have been processed so far + // Map key is the handler type (e.g., "create_issue") + ProcessedHandlers map[string]bool + + // HandlerOutputs tracks the outputs from each processed handler + // This allows later handlers to reference earlier outputs + HandlerOutputs map[string]map[string]string + + // TempIDMapAvailable indicates if a temporary ID map is available from a previous handler + TempIDMapAvailable bool + + // TempIDMapSource is the step ID that provides the temporary ID map + TempIDMapSource string +} + +// NewSafeOutputContext creates a new context for processing safe output handlers +func NewSafeOutputContext(mainJobName string, threatDetectionEnabled bool) *SafeOutputContext { + return &SafeOutputContext{ + ThreatDetectionEnabled: threatDetectionEnabled, + MainJobName: mainJobName, + ProcessedHandlers: make(map[string]bool), + HandlerOutputs: make(map[string]map[string]string), + TempIDMapAvailable: false, + TempIDMapSource: "", + } +} + +// MarkProcessed marks a handler as processed and records its outputs +func (ctx *SafeOutputContext) MarkProcessed(handlerType string, outputs map[string]string) { + ctx.ProcessedHandlers[handlerType] = true + if len(outputs) > 0 { + ctx.HandlerOutputs[handlerType] = outputs + } +} + +// IsProcessed checks if a handler has been processed +func (ctx *SafeOutputContext) IsProcessed(handlerType string) bool { + return ctx.ProcessedHandlers[handlerType] +} + +// GetHandlerOutput retrieves a specific output from a processed handler +func (ctx *SafeOutputContext) GetHandlerOutput(handlerType, outputKey string) string { + if outputs, exists := ctx.HandlerOutputs[handlerType]; exists { + return outputs[outputKey] + } + return "" +} + +// SetTempIDMapSource marks that a temporary ID map is available from a specific step +func (ctx *SafeOutputContext) SetTempIDMapSource(stepID string) { + ctx.TempIDMapAvailable = true + ctx.TempIDMapSource = stepID +} + +// SafeOutputHandlerRegistry manages the collection of safe output handlers +type SafeOutputHandlerRegistry struct { + handlers []SafeOutputHandler + logger *logger.Logger +} + +// NewSafeOutputHandlerRegistry creates a new handler registry +func NewSafeOutputHandlerRegistry() *SafeOutputHandlerRegistry { + return &SafeOutputHandlerRegistry{ + handlers: make([]SafeOutputHandler, 0), + logger: safeOutputHandlerLog, + } +} + +// Register adds a handler to the registry +func (r *SafeOutputHandlerRegistry) Register(handler SafeOutputHandler) { + r.handlers = append(r.handlers, handler) + r.logger.Printf("Registered safe output handler: %s", handler.GetType()) +} + +// GetHandlers returns all registered handlers in order +func (r *SafeOutputHandlerRegistry) GetHandlers() []SafeOutputHandler { + return r.handlers +} + +// GetEnabledHandlers returns only the handlers that are enabled for the given workflow +func (r *SafeOutputHandlerRegistry) GetEnabledHandlers(data *WorkflowData) []SafeOutputHandler { + enabled := make([]SafeOutputHandler, 0) + for _, handler := range r.handlers { + if handler.IsEnabled(data) { + enabled = append(enabled, handler) + r.logger.Printf("Handler enabled: %s", handler.GetType()) + } + } + return enabled +} diff --git a/pkg/workflow/safe_output_handler_add_comment.go b/pkg/workflow/safe_output_handler_add_comment.go new file mode 100644 index 0000000000..e827b28167 --- /dev/null +++ b/pkg/workflow/safe_output_handler_add_comment.go @@ -0,0 +1,133 @@ +package workflow + +import ( + "encoding/json" + + "github.com/githubnext/gh-aw/pkg/logger" +) + +var addCommentHandlerLog = logger.New("workflow:safe_output_handler_add_comment") + +// AddCommentHandler handles add_comment safe output messages +type AddCommentHandler struct{} + +// NewAddCommentHandler creates a new add_comment handler +func NewAddCommentHandler() *AddCommentHandler { + return &AddCommentHandler{} +} + +// GetType returns the type identifier for this handler +func (h *AddCommentHandler) GetType() string { + return "add_comment" +} + +// IsEnabled checks if add_comment is enabled in the workflow configuration +func (h *AddCommentHandler) IsEnabled(data *WorkflowData) bool { + return data.SafeOutputs != nil && data.SafeOutputs.AddComments != nil +} + +// BuildStepConfig builds the step configuration for add_comment +func (h *AddCommentHandler) BuildStepConfig(c *Compiler, data *WorkflowData, ctx *SafeOutputContext) *SafeOutputStepConfig { + if !h.IsEnabled(data) { + return nil + } + + addCommentHandlerLog.Printf("Building add_comment step: target=%s, discussion=%v", + data.SafeOutputs.AddComments.Target, + data.SafeOutputs.AddComments.Discussion != nil && *data.SafeOutputs.AddComments.Discussion) + + // Build custom environment variables specific to add-comment + var customEnvVars []string + + // Pass the comment target configuration + if data.SafeOutputs.AddComments.Target != "" { + customEnvVars = append(customEnvVars, " GH_AW_COMMENT_TARGET: \""+data.SafeOutputs.AddComments.Target+"\"\n") + } + + // Pass the discussion flag configuration + if data.SafeOutputs.AddComments.Discussion != nil && *data.SafeOutputs.AddComments.Discussion { + customEnvVars = append(customEnvVars, " GITHUB_AW_COMMENT_DISCUSSION: \"true\"\n") + } + + // Pass the hide-older-comments flag configuration + if data.SafeOutputs.AddComments.HideOlderComments { + customEnvVars = append(customEnvVars, " GH_AW_HIDE_OLDER_COMMENTS: \"true\"\n") + } + + // Pass the allowed-reasons list configuration + if len(data.SafeOutputs.AddComments.AllowedReasons) > 0 { + reasonsJSON, err := json.Marshal(data.SafeOutputs.AddComments.AllowedReasons) + if err == nil { + customEnvVars = append(customEnvVars, " GH_AW_ALLOWED_REASONS: \""+string(reasonsJSON)+"\"\n") + } + } + + // Add environment variables for outputs from previously processed handlers + if ctx.IsProcessed("create_issue") { + customEnvVars = append(customEnvVars, " GH_AW_CREATED_ISSUE_URL: ${{ steps.create_issue.outputs.issue_url }}\n") + customEnvVars = append(customEnvVars, " GH_AW_CREATED_ISSUE_NUMBER: ${{ steps.create_issue.outputs.issue_number }}\n") + } + + if ctx.IsProcessed("create_discussion") { + customEnvVars = append(customEnvVars, " GH_AW_CREATED_DISCUSSION_URL: ${{ steps.create_discussion.outputs.discussion_url }}\n") + customEnvVars = append(customEnvVars, " GH_AW_CREATED_DISCUSSION_NUMBER: ${{ steps.create_discussion.outputs.discussion_number }}\n") + } + + // Note: create_pull_request is not in the initial migration list, but keeping reference structure + // for when it's migrated later + + // Add temporary ID map if available + if ctx.TempIDMapAvailable { + customEnvVars = append(customEnvVars, " GH_AW_TEMPORARY_ID_MAP: ${{ steps."+ctx.TempIDMapSource+".outputs.temporary_id_map }}\n") + addCommentHandlerLog.Printf("Added temporary ID map reference from step: %s", ctx.TempIDMapSource) + } + + // Build job condition with event check if target is not specified + condition := BuildSafeOutputType("add_comment") + if data.SafeOutputs.AddComments != nil && data.SafeOutputs.AddComments.Target == "" { + eventCondition := BuildOr( + BuildOr( + BuildPropertyAccess("github.event.issue.number"), + BuildPropertyAccess("github.event.pull_request.number"), + ), + BuildPropertyAccess("github.event.discussion.number"), + ) + condition = BuildAnd(condition, eventCondition) + } + if ctx.ThreatDetectionEnabled { + condition = BuildAnd(condition, buildDetectionSuccessCondition()) + } + + // Create outputs map + outputs := map[string]string{ + "comment_id": "${{ steps.add_comment.outputs.comment_id }}", + "comment_url": "${{ steps.add_comment.outputs.comment_url }}", + } + + return &SafeOutputStepConfig{ + StepName: "Add Comment", + StepID: "add_comment", + ScriptName: "add_comment", + CustomEnvVars: customEnvVars, + Condition: condition, + Token: data.SafeOutputs.AddComments.GitHubToken, + UseCopilotToken: false, + UseAgentToken: false, + PreSteps: nil, + PostSteps: nil, + Outputs: outputs, + } +} + +// GetOutputs returns the outputs that add_comment produces +func (h *AddCommentHandler) GetOutputs() map[string]string { + return map[string]string{ + "add_comment_comment_id": "${{ steps.add_comment.outputs.comment_id }}", + "add_comment_comment_url": "${{ steps.add_comment.outputs.comment_url }}", + } +} + +// RequiresTempIDMap returns true if this handler needs access to the temporary ID map +func (h *AddCommentHandler) RequiresTempIDMap() bool { + return true // add_comment consumes the temporary ID map from create_issue +} diff --git a/pkg/workflow/safe_output_handler_close_discussion.go b/pkg/workflow/safe_output_handler_close_discussion.go new file mode 100644 index 0000000000..eedbfa316a --- /dev/null +++ b/pkg/workflow/safe_output_handler_close_discussion.go @@ -0,0 +1,57 @@ +package workflow + +import ( + "github.com/githubnext/gh-aw/pkg/logger" +) + +var closeDiscussionHandlerLog = logger.New("workflow:safe_output_handler_close_discussion") + +// CloseDiscussionHandler handles close_discussion safe output messages +type CloseDiscussionHandler struct{} + +// NewCloseDiscussionHandler creates a new close_discussion handler +func NewCloseDiscussionHandler() *CloseDiscussionHandler { + return &CloseDiscussionHandler{} +} + +// GetType returns the type identifier for this handler +func (h *CloseDiscussionHandler) GetType() string { + return "close_discussion" +} + +// IsEnabled checks if close_discussion is enabled in the workflow configuration +func (h *CloseDiscussionHandler) IsEnabled(data *WorkflowData) bool { + return data.SafeOutputs != nil && data.SafeOutputs.CloseDiscussions != nil +} + +// BuildStepConfig builds the step configuration for close_discussion +func (h *CloseDiscussionHandler) BuildStepConfig(c *Compiler, data *WorkflowData, ctx *SafeOutputContext) *SafeOutputStepConfig { + if !h.IsEnabled(data) { + return nil + } + + closeDiscussionHandlerLog.Print("Building close_discussion step config") + + // Build the step configuration using the existing builder + stepConfig := c.buildCloseDiscussionStepConfig(data, ctx.MainJobName, ctx.ThreatDetectionEnabled) + + // Add temporary ID map if available + if ctx.TempIDMapAvailable && h.RequiresTempIDMap() { + stepConfig.CustomEnvVars = append(stepConfig.CustomEnvVars, + " GH_AW_TEMPORARY_ID_MAP: ${{ steps."+ctx.TempIDMapSource+".outputs.temporary_id_map }}\n") + closeDiscussionHandlerLog.Printf("Added temporary ID map reference from step: %s", ctx.TempIDMapSource) + } + + return &stepConfig +} + +// GetOutputs returns the outputs that close_discussion produces +func (h *CloseDiscussionHandler) GetOutputs() map[string]string { + // close_discussion doesn't produce outputs in the current implementation + return map[string]string{} +} + +// RequiresTempIDMap returns true if this handler needs access to the temporary ID map +func (h *CloseDiscussionHandler) RequiresTempIDMap() bool { + return true // close_discussion may reference temporary IDs from create_issue +} diff --git a/pkg/workflow/safe_output_handler_close_issue.go b/pkg/workflow/safe_output_handler_close_issue.go new file mode 100644 index 0000000000..ec23565351 --- /dev/null +++ b/pkg/workflow/safe_output_handler_close_issue.go @@ -0,0 +1,57 @@ +package workflow + +import ( + "github.com/githubnext/gh-aw/pkg/logger" +) + +var closeIssueHandlerLog = logger.New("workflow:safe_output_handler_close_issue") + +// CloseIssueHandler handles close_issue safe output messages +type CloseIssueHandler struct{} + +// NewCloseIssueHandler creates a new close_issue handler +func NewCloseIssueHandler() *CloseIssueHandler { + return &CloseIssueHandler{} +} + +// GetType returns the type identifier for this handler +func (h *CloseIssueHandler) GetType() string { + return "close_issue" +} + +// IsEnabled checks if close_issue is enabled in the workflow configuration +func (h *CloseIssueHandler) IsEnabled(data *WorkflowData) bool { + return data.SafeOutputs != nil && data.SafeOutputs.CloseIssues != nil +} + +// BuildStepConfig builds the step configuration for close_issue +func (h *CloseIssueHandler) BuildStepConfig(c *Compiler, data *WorkflowData, ctx *SafeOutputContext) *SafeOutputStepConfig { + if !h.IsEnabled(data) { + return nil + } + + closeIssueHandlerLog.Print("Building close_issue step config") + + // Build the step configuration using the existing builder + stepConfig := c.buildCloseIssueStepConfig(data, ctx.MainJobName, ctx.ThreatDetectionEnabled) + + // Add temporary ID map if available + if ctx.TempIDMapAvailable && h.RequiresTempIDMap() { + stepConfig.CustomEnvVars = append(stepConfig.CustomEnvVars, + " GH_AW_TEMPORARY_ID_MAP: ${{ steps."+ctx.TempIDMapSource+".outputs.temporary_id_map }}\n") + closeIssueHandlerLog.Printf("Added temporary ID map reference from step: %s", ctx.TempIDMapSource) + } + + return &stepConfig +} + +// GetOutputs returns the outputs that close_issue produces +func (h *CloseIssueHandler) GetOutputs() map[string]string { + // close_issue doesn't produce outputs in the current implementation + return map[string]string{} +} + +// RequiresTempIDMap returns true if this handler needs access to the temporary ID map +func (h *CloseIssueHandler) RequiresTempIDMap() bool { + return true // close_issue may reference temporary IDs from create_issue +} diff --git a/pkg/workflow/safe_output_handler_create_discussion.go b/pkg/workflow/safe_output_handler_create_discussion.go new file mode 100644 index 0000000000..b155978802 --- /dev/null +++ b/pkg/workflow/safe_output_handler_create_discussion.go @@ -0,0 +1,99 @@ +package workflow + +import ( + "fmt" + + "github.com/githubnext/gh-aw/pkg/logger" +) + +var createDiscussionHandlerLog = logger.New("workflow:safe_output_handler_create_discussion") + +// CreateDiscussionHandler handles create_discussion safe output messages +type CreateDiscussionHandler struct{} + +// NewCreateDiscussionHandler creates a new create_discussion handler +func NewCreateDiscussionHandler() *CreateDiscussionHandler { + return &CreateDiscussionHandler{} +} + +// GetType returns the type identifier for this handler +func (h *CreateDiscussionHandler) GetType() string { + return "create_discussion" +} + +// IsEnabled checks if create_discussion is enabled in the workflow configuration +func (h *CreateDiscussionHandler) IsEnabled(data *WorkflowData) bool { + return data.SafeOutputs != nil && data.SafeOutputs.CreateDiscussions != nil +} + +// BuildStepConfig builds the step configuration for create_discussion +func (h *CreateDiscussionHandler) BuildStepConfig(c *Compiler, data *WorkflowData, ctx *SafeOutputContext) *SafeOutputStepConfig { + if !h.IsEnabled(data) { + return nil + } + + createDiscussionHandlerLog.Printf("Building create_discussion step config for workflow: %s", data.Name) + + // Build custom environment variables specific to create-discussion + var customEnvVars []string + customEnvVars = append(customEnvVars, buildTitlePrefixEnvVar("GH_AW_DISCUSSION_TITLE_PREFIX", data.SafeOutputs.CreateDiscussions.TitlePrefix)...) + customEnvVars = append(customEnvVars, buildCategoryEnvVar("GH_AW_DISCUSSION_CATEGORY", data.SafeOutputs.CreateDiscussions.Category)...) + customEnvVars = append(customEnvVars, buildLabelsEnvVar("GH_AW_DISCUSSION_LABELS", data.SafeOutputs.CreateDiscussions.Labels)...) + customEnvVars = append(customEnvVars, buildLabelsEnvVar("GH_AW_DISCUSSION_ALLOWED_LABELS", data.SafeOutputs.CreateDiscussions.AllowedLabels)...) + customEnvVars = append(customEnvVars, buildAllowedReposEnvVar("GH_AW_ALLOWED_REPOS", data.SafeOutputs.CreateDiscussions.AllowedRepos)...) + + // Add close-older-discussions flag if enabled + if data.SafeOutputs.CreateDiscussions.CloseOlderDiscussions { + customEnvVars = append(customEnvVars, " GH_AW_CLOSE_OLDER_DISCUSSIONS: \"true\"\n") + } + + // Add expires value if set + if data.SafeOutputs.CreateDiscussions.Expires > 0 { + customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_DISCUSSION_EXPIRES: \"%d\"\n", data.SafeOutputs.CreateDiscussions.Expires)) + } + + // Add temporary ID map from create_issue if available + if ctx.TempIDMapAvailable { + customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_TEMPORARY_ID_MAP: ${{ steps.%s.outputs.temporary_id_map }}\n", ctx.TempIDMapSource)) + createDiscussionHandlerLog.Printf("Added temporary ID map reference from step: %s", ctx.TempIDMapSource) + } + + // Build step condition + condition := BuildSafeOutputType("create_discussion") + if ctx.ThreatDetectionEnabled { + condition = BuildAnd(condition, buildDetectionSuccessCondition()) + } + + // Create outputs map + outputs := map[string]string{ + "discussion_number": "${{ steps.create_discussion.outputs.discussion_number }}", + "discussion_url": "${{ steps.create_discussion.outputs.discussion_url }}", + } + + return &SafeOutputStepConfig{ + StepName: "Create Discussion", + StepID: "create_discussion", + ScriptName: "create_discussion", + CustomEnvVars: customEnvVars, + Condition: condition, + Token: data.SafeOutputs.CreateDiscussions.GitHubToken, + UseCopilotToken: false, + UseAgentToken: false, + PreSteps: nil, + PostSteps: nil, + Outputs: outputs, + } +} + +// GetOutputs returns the outputs that create_discussion produces +func (h *CreateDiscussionHandler) GetOutputs() map[string]string { + return map[string]string{ + "create_discussion_discussion_number": "${{ steps.create_discussion.outputs.discussion_number }}", + "create_discussion_discussion_url": "${{ steps.create_discussion.outputs.discussion_url }}", + } +} + +// RequiresTempIDMap returns true if this handler needs access to the temporary ID map +func (h *CreateDiscussionHandler) RequiresTempIDMap() bool { + return true // create_discussion consumes the temporary ID map from create_issue +} diff --git a/pkg/workflow/safe_output_handler_create_issue.go b/pkg/workflow/safe_output_handler_create_issue.go new file mode 100644 index 0000000000..da993de54a --- /dev/null +++ b/pkg/workflow/safe_output_handler_create_issue.go @@ -0,0 +1,127 @@ +package workflow + +import ( + "fmt" + + "github.com/githubnext/gh-aw/pkg/logger" +) + +var createIssueHandlerLog = logger.New("workflow:safe_output_handler_create_issue") + +// CreateIssueHandler handles create_issue safe output messages +type CreateIssueHandler struct{} + +// NewCreateIssueHandler creates a new create_issue handler +func NewCreateIssueHandler() *CreateIssueHandler { + return &CreateIssueHandler{} +} + +// GetType returns the type identifier for this handler +func (h *CreateIssueHandler) GetType() string { + return "create_issue" +} + +// IsEnabled checks if create_issue is enabled in the workflow configuration +func (h *CreateIssueHandler) IsEnabled(data *WorkflowData) bool { + return data.SafeOutputs != nil && data.SafeOutputs.CreateIssues != nil +} + +// BuildStepConfig builds the step configuration for create_issue +func (h *CreateIssueHandler) BuildStepConfig(c *Compiler, data *WorkflowData, ctx *SafeOutputContext) *SafeOutputStepConfig { + if !h.IsEnabled(data) { + return nil + } + + createIssueHandlerLog.Printf("Building create_issue step config: workflow=%s, assignees=%d, labels=%d", + data.Name, len(data.SafeOutputs.CreateIssues.Assignees), len(data.SafeOutputs.CreateIssues.Labels)) + + // Build custom environment variables specific to create-issue + var customEnvVars []string + customEnvVars = append(customEnvVars, buildTitlePrefixEnvVar("GH_AW_ISSUE_TITLE_PREFIX", data.SafeOutputs.CreateIssues.TitlePrefix)...) + customEnvVars = append(customEnvVars, buildLabelsEnvVar("GH_AW_ISSUE_LABELS", data.SafeOutputs.CreateIssues.Labels)...) + customEnvVars = append(customEnvVars, buildLabelsEnvVar("GH_AW_ISSUE_ALLOWED_LABELS", data.SafeOutputs.CreateIssues.AllowedLabels)...) + customEnvVars = append(customEnvVars, buildAllowedReposEnvVar("GH_AW_ALLOWED_REPOS", data.SafeOutputs.CreateIssues.AllowedRepos)...) + + // Add expires value if set + if data.SafeOutputs.CreateIssues.Expires > 0 { + customEnvVars = append(customEnvVars, fmt.Sprintf(" GH_AW_ISSUE_EXPIRES: \"%d\"\n", data.SafeOutputs.CreateIssues.Expires)) + } + + // Check if copilot is in assignees - if so, we'll output issues for assign_to_agent job + assignCopilot := hasCopilotAssignee(data.SafeOutputs.CreateIssues.Assignees) + if assignCopilot { + customEnvVars = append(customEnvVars, " GH_AW_ASSIGN_COPILOT: \"true\"\n") + createIssueHandlerLog.Print("Copilot assignment requested - will output issues_to_assign_copilot") + } + + // Build post-steps for non-copilot assignees + var postSteps []string + nonCopilotAssignees := filterNonCopilotAssignees(data.SafeOutputs.CreateIssues.Assignees) + if len(nonCopilotAssignees) > 0 { + var safeOutputsToken string + if data.SafeOutputs != nil { + safeOutputsToken = data.SafeOutputs.GitHubToken + } + + postSteps = buildCopilotParticipantSteps(CopilotParticipantConfig{ + Participants: nonCopilotAssignees, + ParticipantType: "assignee", + CustomToken: data.SafeOutputs.CreateIssues.GitHubToken, + SafeOutputsToken: safeOutputsToken, + WorkflowToken: data.GitHubToken, + ConditionStepID: "create_issue", + ConditionOutputKey: "issue_number", + }) + } + + // Add post-step for copilot assignment using agent token + if assignCopilot { + postSteps = append(postSteps, buildCopilotAssignmentStep(data.SafeOutputs.CreateIssues.GitHubToken)...) + } + + // Build step condition + condition := BuildSafeOutputType("create_issue") + if ctx.ThreatDetectionEnabled { + condition = BuildAnd(condition, buildDetectionSuccessCondition()) + } + + // Create outputs map + outputs := map[string]string{ + "issue_number": "${{ steps.create_issue.outputs.issue_number }}", + "issue_url": "${{ steps.create_issue.outputs.issue_url }}", + "temporary_id_map": "${{ steps.create_issue.outputs.temporary_id_map }}", + } + + // Add issues_to_assign_copilot output if copilot assignment is requested + if assignCopilot { + outputs["issues_to_assign_copilot"] = "${{ steps.create_issue.outputs.issues_to_assign_copilot }}" + } + + return &SafeOutputStepConfig{ + StepName: "Create Issue", + StepID: "create_issue", + ScriptName: "create_issue", + CustomEnvVars: customEnvVars, + Condition: condition, + Token: data.SafeOutputs.CreateIssues.GitHubToken, + UseCopilotToken: false, + UseAgentToken: false, + PreSteps: nil, + PostSteps: postSteps, + Outputs: outputs, + } +} + +// GetOutputs returns the outputs that create_issue produces +func (h *CreateIssueHandler) GetOutputs() map[string]string { + return map[string]string{ + "create_issue_issue_number": "${{ steps.create_issue.outputs.issue_number }}", + "create_issue_issue_url": "${{ steps.create_issue.outputs.issue_url }}", + "create_issue_temporary_id_map": "${{ steps.create_issue.outputs.temporary_id_map }}", + } +} + +// RequiresTempIDMap returns true if this handler needs access to the temporary ID map +func (h *CreateIssueHandler) RequiresTempIDMap() bool { + return false // create_issue generates the map, doesn't consume it +} diff --git a/pkg/workflow/safe_output_handler_processor.go b/pkg/workflow/safe_output_handler_processor.go new file mode 100644 index 0000000000..ef45c40eda --- /dev/null +++ b/pkg/workflow/safe_output_handler_processor.go @@ -0,0 +1,106 @@ +package workflow + +import ( + "github.com/githubnext/gh-aw/pkg/logger" +) + +var handlerProcessorLog = logger.New("workflow:safe_output_handler_processor") + +// processHandlersSequentially processes safe output handlers in sequential order, +// maintaining a temporary ID map that is passed between handlers. +// Returns the generated steps, outputs, permissions, and step names. +func (c *Compiler) processHandlersSequentially( + data *WorkflowData, + mainJobName string, + threatDetectionEnabled bool, + registry *SafeOutputHandlerRegistry, +) ([]string, map[string]string, *Permissions, []string) { + var steps []string + outputs := make(map[string]string) + permissions := NewPermissions() + var stepNames []string + + // Create context for handler processing + ctx := NewSafeOutputContext(mainJobName, threatDetectionEnabled) + + // Process each handler in order + for _, handler := range registry.GetEnabledHandlers(data) { + handlerType := handler.GetType() + handlerProcessorLog.Printf("Processing handler: %s", handlerType) + + // Build step configuration for this handler + stepConfig := handler.BuildStepConfig(c, data, ctx) + if stepConfig == nil { + handlerProcessorLog.Printf("Handler %s returned nil config, skipping", handlerType) + continue + } + + // Build the YAML steps for this handler + stepYAML := c.buildConsolidatedSafeOutputStep(data, *stepConfig) + steps = append(steps, stepYAML...) + stepNames = append(stepNames, stepConfig.StepID) + + // Add handler outputs to job outputs + handlerOutputs := handler.GetOutputs() + for key, value := range handlerOutputs { + outputs[key] = value + } + + // Add handler-specific permissions + switch handlerType { + case "create_issue": + permissions.Merge(NewPermissionsContentsReadIssuesWrite()) + // Mark that temporary ID map is now available + ctx.SetTempIDMapSource("create_issue") + case "create_discussion": + permissions.Merge(NewPermissionsContentsReadDiscussionsWrite()) + case "add_comment": + permissions.Merge(NewPermissionsContentsReadIssuesWritePRWriteDiscussionsWrite()) + case "update_issue": + permissions.Merge(NewPermissionsContentsReadIssuesWrite()) + case "update_discussion": + permissions.Merge(NewPermissionsContentsReadDiscussionsWrite()) + case "close_issue": + permissions.Merge(NewPermissionsContentsReadIssuesWrite()) + case "close_discussion": + permissions.Merge(NewPermissionsContentsReadDiscussionsWrite()) + } + + // Mark handler as processed + ctx.MarkProcessed(handlerType, stepConfig.Outputs) + + handlerProcessorLog.Printf("Handler %s processed successfully", handlerType) + } + + handlerProcessorLog.Printf("Processed %d handlers, generated %d steps", len(stepNames), len(stepNames)) + + return steps, outputs, permissions, stepNames +} + +// getHandlerRegistry creates and initializes the safe output handler registry +// with all available handlers in the correct processing order. +func getHandlerRegistry() *SafeOutputHandlerRegistry { + registry := NewSafeOutputHandlerRegistry() + + // Register handlers in processing order: + // 1. create_issue - generates temporary ID map + // 2. create_discussion - consumes temporary ID map + // 3. update_issue - consumes temporary ID map + // 4. update_discussion - consumes temporary ID map + // 5. close_issue - consumes temporary ID map + // 6. close_discussion - consumes temporary ID map + // 7. add_comment - consumes temporary ID map and references other handler outputs + + registry.Register(NewCreateIssueHandler()) + registry.Register(NewCreateDiscussionHandler()) + registry.Register(NewUpdateIssueHandler()) + registry.Register(NewUpdateDiscussionHandler()) + registry.Register(NewCloseIssueHandler()) + registry.Register(NewCloseDiscussionHandler()) + registry.Register(NewAddCommentHandler()) + + // Note: Other handlers (create_pull_request, etc.) will be migrated later + // and added to the registry at that time + + return registry +} diff --git a/pkg/workflow/safe_output_handler_update_discussion.go b/pkg/workflow/safe_output_handler_update_discussion.go new file mode 100644 index 0000000000..b4de4edd69 --- /dev/null +++ b/pkg/workflow/safe_output_handler_update_discussion.go @@ -0,0 +1,59 @@ +package workflow + +import ( + "github.com/githubnext/gh-aw/pkg/logger" +) + +var updateDiscussionHandlerLog = logger.New("workflow:safe_output_handler_update_discussion") + +// UpdateDiscussionHandler handles update_discussion safe output messages +type UpdateDiscussionHandler struct{} + +// NewUpdateDiscussionHandler creates a new update_discussion handler +func NewUpdateDiscussionHandler() *UpdateDiscussionHandler { + return &UpdateDiscussionHandler{} +} + +// GetType returns the type identifier for this handler +func (h *UpdateDiscussionHandler) GetType() string { + return "update_discussion" +} + +// IsEnabled checks if update_discussion is enabled in the workflow configuration +func (h *UpdateDiscussionHandler) IsEnabled(data *WorkflowData) bool { + return data.SafeOutputs != nil && data.SafeOutputs.UpdateDiscussions != nil +} + +// BuildStepConfig builds the step configuration for update_discussion +func (h *UpdateDiscussionHandler) BuildStepConfig(c *Compiler, data *WorkflowData, ctx *SafeOutputContext) *SafeOutputStepConfig { + if !h.IsEnabled(data) { + return nil + } + + updateDiscussionHandlerLog.Print("Building update_discussion step config") + + // Build the step configuration using the existing builder + stepConfig := c.buildUpdateDiscussionStepConfig(data, ctx.MainJobName, ctx.ThreatDetectionEnabled) + + // Add temporary ID map if available + if ctx.TempIDMapAvailable && h.RequiresTempIDMap() { + stepConfig.CustomEnvVars = append(stepConfig.CustomEnvVars, + " GH_AW_TEMPORARY_ID_MAP: ${{ steps."+ctx.TempIDMapSource+".outputs.temporary_id_map }}\n") + updateDiscussionHandlerLog.Printf("Added temporary ID map reference from step: %s", ctx.TempIDMapSource) + } + + return &stepConfig +} + +// GetOutputs returns the outputs that update_discussion produces +func (h *UpdateDiscussionHandler) GetOutputs() map[string]string { + return map[string]string{ + "update_discussion_discussion_number": "${{ steps.update_discussion.outputs.discussion_number }}", + "update_discussion_discussion_url": "${{ steps.update_discussion.outputs.discussion_url }}", + } +} + +// RequiresTempIDMap returns true if this handler needs access to the temporary ID map +func (h *UpdateDiscussionHandler) RequiresTempIDMap() bool { + return true // update_discussion may reference temporary IDs from create_issue +} diff --git a/pkg/workflow/safe_output_handler_update_issue.go b/pkg/workflow/safe_output_handler_update_issue.go new file mode 100644 index 0000000000..66a2fc1c0a --- /dev/null +++ b/pkg/workflow/safe_output_handler_update_issue.go @@ -0,0 +1,57 @@ +package workflow + +import ( + "github.com/githubnext/gh-aw/pkg/logger" +) + +var updateIssueHandlerLog = logger.New("workflow:safe_output_handler_update_issue") + +// UpdateIssueHandler handles update_issue safe output messages +type UpdateIssueHandler struct{} + +// NewUpdateIssueHandler creates a new update_issue handler +func NewUpdateIssueHandler() *UpdateIssueHandler { + return &UpdateIssueHandler{} +} + +// GetType returns the type identifier for this handler +func (h *UpdateIssueHandler) GetType() string { + return "update_issue" +} + +// IsEnabled checks if update_issue is enabled in the workflow configuration +func (h *UpdateIssueHandler) IsEnabled(data *WorkflowData) bool { + return data.SafeOutputs != nil && data.SafeOutputs.UpdateIssues != nil +} + +// BuildStepConfig builds the step configuration for update_issue +func (h *UpdateIssueHandler) BuildStepConfig(c *Compiler, data *WorkflowData, ctx *SafeOutputContext) *SafeOutputStepConfig { + if !h.IsEnabled(data) { + return nil + } + + updateIssueHandlerLog.Print("Building update_issue step config") + + // Build the step configuration using the existing builder + stepConfig := c.buildUpdateIssueStepConfig(data, ctx.MainJobName, ctx.ThreatDetectionEnabled) + + // Add temporary ID map if available + if ctx.TempIDMapAvailable && h.RequiresTempIDMap() { + stepConfig.CustomEnvVars = append(stepConfig.CustomEnvVars, + " GH_AW_TEMPORARY_ID_MAP: ${{ steps."+ctx.TempIDMapSource+".outputs.temporary_id_map }}\n") + updateIssueHandlerLog.Printf("Added temporary ID map reference from step: %s", ctx.TempIDMapSource) + } + + return &stepConfig +} + +// GetOutputs returns the outputs that update_issue produces +func (h *UpdateIssueHandler) GetOutputs() map[string]string { + // update_issue doesn't produce outputs in the current implementation + return map[string]string{} +} + +// RequiresTempIDMap returns true if this handler needs access to the temporary ID map +func (h *UpdateIssueHandler) RequiresTempIDMap() bool { + return true // update_issue may reference temporary IDs from create_issue +}