Split safe outputs handler registry by domain

docs/adr/55482-split-safe-outputs-handler-registry-by-domain.md

@@ -0,0 +1,45 @@
+# ADR-55482: Split Safe Outputs Handler Registry by Domain
+
+**Date**: 2026-08-24
+**Status**: Draft
+**Deciders**: pelikhan, copilot-swe-agent
+
+---
+
+### Context
+
+`pkg/workflow/safe_outputs_handler_registry.go` accumulated all safe-output handler builders in a single monolithic `map[string]handlerBuilder` literal that grew to 1 000+ lines. Reviewing, editing, or adding handlers required navigating a single large file with no logical grouping, making domain-specific changes error-prone and PR reviews difficult. The handlers naturally cluster by GitHub entity: issues, discussions, pull requests, workflow-level actions, projects, assignments, comments, releases, and diagnostics.
+
+### Decision
+
+We will decompose the single `handlerRegistry` map into focused sub-registries (one per domain), each in its own file, and compose them at package init time via a new `mergeHandlerMaps(...)` helper in the existing file. The resulting `handlerRegistry` variable and all call sites remain unchanged; only the file layout and initialization sequence change.
+
+### Alternatives Considered
+
+#### Alternative 1: Keep the monolith, improve navigation with comments/regions
+
+Add prominent section dividers and a table-of-contents comment. No structural change; IDE jump-to-definition still works. Rejected because the file continues to grow with each new handler and the readability problem recurs with every future domain.
+
+#### Alternative 2: Plugin/interface-based extensible registry
+
+Define a `HandlerProvider` interface; each domain registers via `init()`. Fully decoupled, but requires a registry-registration protocol, initialization-order care, and significantly more boilerplate for a problem that does not require runtime extensibility. Over-engineered for a compile-time-only registry.
+
+### Consequences
+
+#### Positive
+- Domain handlers can be reviewed, tested, and changed in isolation without touching an unrelated 1 000-line file.
+- `mergeHandlerMaps` explicitly detects duplicate handler keys and logs a warning rather than silently overwriting, reducing accidental collision risk.
+- Smaller per-file diffs improve PR review clarity.
+- New registry test coverage verifies domain membership, full composition, builder enable/disable behavior, duplicate-key handling, and token-helper behavior.
+
+#### Negative
+- `mergeHandlerMaps` applies a first-wins policy on key collisions (logged, not panicked). A future developer who inadvertently registers the same key in two domain files will only see a log line rather than a compile-time or startup error.
+- Registry initialization order (the order of arguments to `mergeHandlerMaps`) is now implicit; the order matters for collision resolution but is not enforced by type system or test.
+
+#### Neutral
+- All existing call sites (`handlerRegistry[key]`, `handlerSupportsPerHandlerGitHubAppToken`, etc.) are unchanged; this is a pure internal reorganization.
+- The `add_comment` handler lives in `commentHandlerRegistry` because it applies to both issues and discussions; this placement is a documentation convention, not enforced by code.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/workflow/safe_outputs_handler_registry_assignments.go

@@ -0,0 +1,79 @@
+package workflow
+
+// assignmentHandlerRegistry contains assignment and agent session handler builders.
+var assignmentHandlerRegistry = map[string]handlerBuilder{
+	"assign_to_agent": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.AssignToAgent == nil {
+			return nil
+		}
+		c := cfg.AssignToAgent
+		return newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddIfNotEmpty("name", c.DefaultAgent).
+			AddIfNotEmpty("model", c.DefaultModel).
+			AddIfNotEmpty("custom-agent", c.DefaultCustomAgent).
+			AddIfNotEmpty("custom-instructions", c.DefaultCustomInstructions).
+			AddStringSlice("allowed", c.Allowed).
+			AddBoolPtr("issue_intent", c.IssueIntent).
+			AddIfTrue("ignore-if-error", c.IgnoreIfError).
+			AddIfNotEmpty("target", c.Target).
+			AddIfNotEmpty("target-repo", c.TargetRepoSlug).
+			AddStringSlice("allowed-repos", c.AllowedRepos).
+			AddIfNotEmpty("pull-request-repo", c.PullRequestRepoSlug).
+			AddStringSlice("allowed-pull-request-repos", c.AllowedPullRequestRepos).
+			AddIfNotEmpty("base-branch", c.BaseBranch).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "assign-to-agent", c.GitHubToken)).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+	"assign_to_user": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.AssignToUser == nil {
+			return nil
+		}
+		c := cfg.AssignToUser
+		return newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddStringSlice("allowed", c.Allowed).
+			AddStringSlice("blocked", c.Blocked).
+			AddIfNotEmpty("target", c.Target).AddStringSlice("required_labels", c.RequiredLabels).
+			AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix).AddIfNotEmpty("target-repo", c.TargetRepoSlug).
+			AddStringSlice("allowed_repos", c.AllowedRepos).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "assign-to-user", c.GitHubToken)).
+			AddTemplatableBool("unassign_first", c.UnassignFirst).
+			AddBoolPtr("issue_intent", c.IssueIntent).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+	"unassign_from_user": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.UnassignFromUser == nil {
+			return nil
+		}
+		c := cfg.UnassignFromUser
+		return newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddStringSlice("allowed", c.Allowed).
+			AddStringSlice("blocked", c.Blocked).
+			AddIfNotEmpty("target", c.Target).
+			AddStringSlice("required_labels", c.RequiredLabels).
+			AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix).
+			AddIfNotEmpty("target-repo", c.TargetRepoSlug).
+			AddStringSlice("allowed_repos", c.AllowedRepos).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "unassign-from-user", c.GitHubToken)).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+	"create_agent_session": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.CreateAgentSessions == nil {
+			return nil
+		}
+		c := cfg.CreateAgentSessions
+		return newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddIfNotEmpty("base", c.Base).
+			AddIfNotEmpty("target-repo", c.TargetRepoSlug).
+			AddStringSlice("allowed_repos", c.AllowedRepos).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "create-agent-session", c.GitHubToken)).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+}

pkg/workflow/safe_outputs_handler_registry_comments.go

@@ -0,0 +1,42 @@
+package workflow
+
+// commentHandlerRegistry contains comment handler builders.
+var commentHandlerRegistry = map[string]handlerBuilder{
+	"add_comment": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.AddComments == nil {
+			return nil
+		}
+		c := cfg.AddComments
+		return newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddIfNotEmpty("target", c.Target).
+			AddTemplatableBool("hide_older_comments", c.HideOlderComments).
+			AddStringSlice("hide_older_comments_match", c.HideOlderCommentsMatch).
+			AddBoolPtr("discussions", c.Discussions).
+			AddIfNotEmpty("target-repo", c.TargetRepoSlug).
+			AddTemplatableStringSlice("allowed_repos", c.AllowedRepos).
+			AddTemplatableStringSlice("allows_comment_ids", c.AllowedCommentIDs).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "add-comment", c.GitHubToken)).
+			AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)).
+			AddBoolPtr("normalize_closing_keywords", c.NormalizeClosingKeywords).
+			AddStringSlice("required_labels", c.RequiredLabels).
+			AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+	"hide_comment": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.HideComment == nil {
+			return nil
+		}
+		c := cfg.HideComment
+		return newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddStringSlice("allowed_reasons", c.AllowedReasons).AddIfNotEmpty("target", c.Target).
+			AddStringSlice("required_labels", c.RequiredLabels).
+			AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix).AddIfNotEmpty("target-repo", c.TargetRepoSlug).
+			AddStringSlice("allowed_repos", c.AllowedRepos).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "hide-comment", c.GitHubToken)).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+}

pkg/workflow/safe_outputs_handler_registry_diagnostics.go

@@ -0,0 +1,74 @@
+package workflow
+
+// diagnosticHandlerRegistry contains diagnostic, reporting, and no-op handler builders.
+var diagnosticHandlerRegistry = map[string]handlerBuilder{
+	"missing_tool": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.MissingTool == nil {
+			return nil
+		}
+		c := cfg.MissingTool
+		return newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "missing-tool", c.GitHubToken)).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+	"missing_data": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.MissingData == nil {
+			return nil
+		}
+		c := cfg.MissingData
+		return newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "missing-data", c.GitHubToken)).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+	"noop": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.NoOp == nil {
+			return nil
+		}
+		c := cfg.NoOp
+		return newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddStringPtr("report-as-issue", c.ReportAsIssue).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+	"report_incomplete": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.ReportIncomplete == nil {
+			return nil
+		}
+		c := cfg.ReportIncomplete
+		return newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "report-incomplete", c.GitHubToken)).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+	"create_report_incomplete_issue": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.ReportIncomplete == nil {
+			return nil
+		}
+		c := cfg.ReportIncomplete
+		// If create-issue is explicitly false, skip generating the issue handler.
+		// For nil (default) or "true", always include; for expressions, include
+		// the handler and embed the expression so it is evaluated at runtime.
+		if c.CreateIssue != nil && *c.CreateIssue == "false" {
+			return nil
+		}
+		builder := newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddIfNotEmpty("title-prefix", c.TitlePrefix).
+			AddStringSlice("labels", c.Labels).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "report-incomplete", c.GitHubToken)).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged))
+		// When create-issue is a GitHub Actions expression, embed it in the handler config.
+		// GitHub Actions evaluates the expression before the handler runs; the JavaScript
+		// handler then parses the resolved value via parseBoolTemplatable at runtime.
+		if c.CreateIssue != nil && isExpression(*c.CreateIssue) {
+			builder = builder.AddTemplatableBool("create-issue", c.CreateIssue)
+		}
+		return builder.Build()
+	},
+}

pkg/workflow/safe_outputs_handler_registry_discussions.go

@@ -0,0 +1,74 @@
+package workflow
+
+// discussionHandlerRegistry contains discussion lifecycle handler builders.
+var discussionHandlerRegistry = map[string]handlerBuilder{
+	"create_discussion": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.CreateDiscussions == nil {
+			return nil
+		}
+		c := cfg.CreateDiscussions
+		return newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddIfNotEmpty("category", c.Category).
+			AddIfNotEmpty("title_prefix", c.TitlePrefix).
+			AddIfPositive("min_body_length", c.MinBodyLength).
+			AddStringSlice("labels", c.Labels).
+			AddStringSlice("allowed_labels", c.AllowedLabels).
+			AddStringSlice("allowed_repos", c.AllowedRepos).
+			// entity-specific env key name per shared CloseOlderConfig field (see the create_issue handler in safe_outputs_handler_registry_issues.go)
+			AddTemplatableBool("close_older_discussions", c.Enabled).
+			AddIfNotEmpty("close_older_key", c.Key).
+			AddIfNotEmpty("required_category", c.RequiredCategory).
+			AddIfPositive("expires", c.Expires).
+			AddBoolPtr("fallback_to_issue", c.FallbackToIssue).
+			AddIfNotEmpty("target-repo", c.TargetRepoSlug).
+			AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "create-discussion", c.GitHubToken)).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+	"close_discussion": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.CloseDiscussions == nil {
+			return nil
+		}
+		c := cfg.CloseDiscussions
+		return newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddIfNotEmpty("target", c.Target).
+			AddStringSlice("required_labels", c.RequiredLabels).
+			AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix).
+			AddIfNotEmpty("target-repo", c.TargetRepoSlug).
+			AddStringSlice("allowed_repos", c.AllowedRepos).
+			AddBoolPtr("allow_body", c.AllowBody).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "close-discussion", c.GitHubToken)).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+	"update_discussion": func(cfg *SafeOutputsConfig) map[string]any {
+		if cfg.UpdateDiscussions == nil {
+			return nil
+		}
+		c := cfg.UpdateDiscussions
+		builder := newHandlerConfigBuilder().
+			AddTemplatableInt("max", c.Max).
+			AddIfNotEmpty("target", c.Target)
+		// Boolean pointer fields indicate which fields can be updated
+		if c.Title != nil {
+			builder.AddDefault("allow_title", true)
+		}
+		if c.Body != nil {
+			builder.AddDefault("allow_body", true)
+		}
+		if c.Labels != nil {
+			builder.AddDefault("allow_labels", true)
+		}
+		return builder.
+			AddStringSlice("allowed_labels", c.AllowedLabels).
+			AddIfNotEmpty("target-repo", c.TargetRepoSlug).
+			AddStringSlice("allowed_repos", c.AllowedRepos).
+			AddIfNotEmpty("github-token", resolveHandlerGitHubToken(c.GitHubApp, "update-discussion", c.GitHubToken)).
+			AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)).
+			AddTemplatableBool("staged", templatableBoolPtrToStringPtr(c.Staged)).
+			Build()
+	},
+}