chore(workflow): make logger namespaces statically greppable

docs/adr/45721-make-logger-namespaces-statically-greppable.md

@@ -0,0 +1,48 @@
+# ADR-45721: Make Workflow Logger Namespaces Statically Greppable
+
+**Date**: 2026-07-15
+**Status**: Draft
+**Deciders**: pelikhan, copilot-swe-agent
+
+---
+
+### Context
+
+The `pkg/workflow` package contains ~48 validation files, each declaring a package-level logger variable. These loggers were created via a shared helper `newValidationLogger(domain string)` in `validation_helpers.go`, which constructed the namespace string at runtime via concatenation: `"workflow:" + domain + "_validation"`. This made it impossible to statically discover the complete set of logger namespaces using grep (e.g., `grep -rn 'logger.New'`) because the actual string values only existed at runtime. Additionally, `close_entity_helpers.go` embedded three `logger.New(...)` calls directly inside a composite slice literal, which also evaded static grep discovery. One call in `push_to_pull_request_branch_validation.go` carried a latent bug where `newValidationLogger("push_to_pull_request_branch_validation")` silently produced the doubly-suffixed namespace `"workflow:push_to_pull_request_branch_validation_validation"`.
+
+### Decision
+
+We will remove the `newValidationLogger()` helper and replace every call site with a direct `logger.New("workflow:<domain>_validation")` string literal. In `close_entity_helpers.go`, the three inline `logger.New(...)` calls inside the registry slice literal will be extracted into named package-level vars. The doubled-suffix bug will be corrected to `"workflow:push_to_pull_request_branch_validation"` as part of the same pass. No behaviour changes beyond that one bug fix are introduced.
+
+### Alternatives Considered
+
+#### Alternative 1: Keep `newValidationLogger()` and add a static analysis lint rule
+
+The helper is retained but a custom lint rule (or `go generate` pass) enumerates its call sites and records the resolved namespace strings in a generated file. This preserves the DRY convention enforced by the helper while still enabling static discovery.
+
+Rejected because it adds tooling complexity (a custom linter or generator that must be maintained and run in CI) without eliminating the runtime indirection. The direct string literal approach is simpler and equally correct, and Go's import cycle prevents the namespace string from drifting silently.
+
+#### Alternative 2: Use a code generator to produce a central namespace registry
+
+A generator scans all `*_validation.go` files, extracts the domain names, and emits a single `logger_namespaces_gen.go` file listing all `logger.New(...)` calls. Callers import the generated constants.
+
+Rejected as over-engineering. The benefit (one authoritative file) does not outweigh the cost (a generator to maintain, a generated file to keep in sync, and a new build step). Direct `logger.New()` literals with a well-known naming convention are greppable and self-documenting without any tooling.
+
+### Consequences
+
+#### Positive
+- All logger namespace strings are now fully static: `grep -rn 'logger.New'` in `pkg/workflow` yields the complete, authoritative list.
+- The latent doubled-suffix bug (`push_to_pull_request_branch_validation_validation`) is fixed.
+- The `newValidationLogger` helper is removed, shrinking the public API surface of `validation_helpers.go`.
+
+#### Negative
+- Each of the ~48 `*_validation.go` files now carries a direct `import "github.com/github/gh-aw/pkg/logger"`, increasing per-file coupling to the logger package (previously mediated by the shared helper in one file).
+- Future validation files must manually follow the `"workflow:<domain>_validation"` naming convention; the helper previously enforced this implicitly. A new file author could deviate without a compile error.
+
+#### Neutral
+- The change is a mechanical find-and-replace across 50 files; the diff is large by line count but trivially reviewable because every hunk is structurally identical.
+- `close_entity_helpers.go` acquires three extra package-level var declarations moved out of the slice literal initialiser.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/workflow/agent_validation.go

@@ -54,10 +54,11 @@ import (
 
 	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/logger"
 	"github.com/goccy/go-yaml"
 )
 
-var agentValidationLog = newValidationLogger("agent")
+var agentValidationLog = logger.New("workflow:agent_validation")
 
 // validateAgentFile validates that the custom agent file specified in imports exists
 func (c *Compiler) validateAgentFile(workflowData *WorkflowData, markdownPath string) error {

pkg/workflow/call_workflow_validation.go

@@ -7,11 +7,12 @@ import (
 	"path/filepath"
 
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/parser"
 	"github.com/goccy/go-yaml"
 )
 
-var callWorkflowValidationLog = newValidationLogger("call_workflow")
+var callWorkflowValidationLog = logger.New("workflow:call_workflow_validation")
 
 // validateCallWorkflow validates that the call-workflow configuration is correct.
 // It checks that each workflow exists, declares a workflow_call trigger, and is not

pkg/workflow/checkout_path_validation.go

@@ -4,9 +4,11 @@ import (
 	"fmt"
 	"os"
 	"strings"
+
+	"github.com/github/gh-aw/pkg/logger"
 )
 
-var checkoutPathValidationLog = newValidationLogger("checkout_path")
+var checkoutPathValidationLog = logger.New("workflow:checkout_path_validation")
 
 // deriveAndWarnCrossRepoCheckoutPaths emits warnings for cross-repository
 // checkout entries that have no explicit path: field, and auto-derives a path from

pkg/workflow/close_entity_helpers.go

@@ -156,6 +156,10 @@ type closeEntityDefinition struct {
 	Logger           *logger.Logger
 }
 
+var logCloseIssue = logger.New("workflow:close_issue")
+var logClosePullRequest = logger.New("workflow:close_pull_request")
+var logCloseDiscussion = logger.New("workflow:close_discussion")
+
 // closeEntityRegistry holds all close entity definitions
 var closeEntityRegistry = []closeEntityDefinition{
 	{
@@ -169,7 +173,7 @@ var closeEntityRegistry = []closeEntityDefinition{
 		EventNumberPath1: "github.event.issue.number",
 		EventNumberPath2: "github.event.comment.issue.number",
 		PermissionsFunc:  NewPermissionsContentsReadIssuesWrite,
-		Logger:           logger.New("workflow:close_issue"),
+		Logger:           logCloseIssue,
 	},
 	{
 		EntityType:       CloseEntityPullRequest,
@@ -182,7 +186,7 @@ var closeEntityRegistry = []closeEntityDefinition{
 		EventNumberPath1: "github.event.pull_request.number",
 		EventNumberPath2: "github.event.comment.pull_request.number",
 		PermissionsFunc:  NewPermissionsContentsReadPRWrite,
-		Logger:           logger.New("workflow:close_pull_request"),
+		Logger:           logClosePullRequest,
 	},
 	{
 		EntityType:       CloseEntityDiscussion,
@@ -195,7 +199,7 @@ var closeEntityRegistry = []closeEntityDefinition{
 		EventNumberPath1: "github.event.discussion.number",
 		EventNumberPath2: "github.event.comment.discussion.number",
 		PermissionsFunc:  NewPermissionsContentsReadDiscussionsWrite,
-		Logger:           logger.New("workflow:close_discussion"),
+		Logger:           logCloseDiscussion,
 	},
 }
 

pkg/workflow/compiler_filters_validation.go

@@ -48,43 +48,45 @@ package workflow
 import (
 	"fmt"
 	"strings"
+
+	"github.com/github/gh-aw/pkg/logger"
 )
 
-var filterValidationLog = newValidationLogger("filter")
+var compilerFiltersValidationLog = logger.New("workflow:compiler_filters_validation")
 
 // ValidateEventFilters checks for GitHub Actions filter mutual exclusivity rules
 func ValidateEventFilters(frontmatter map[string]any) error {
-	filterValidationLog.Print("Validating event filter mutual exclusivity")
+	compilerFiltersValidationLog.Print("Validating event filter mutual exclusivity")
 
 	on, exists := frontmatter["on"]
 	if !exists {
-		filterValidationLog.Print("No 'on' section found, skipping filter validation")
+		compilerFiltersValidationLog.Print("No 'on' section found, skipping filter validation")
 		return nil
 	}
 
 	onMap, ok := on.(map[string]any)
 	if !ok {
-		filterValidationLog.Print("'on' section is not a map, skipping filter validation")
+		compilerFiltersValidationLog.Print("'on' section is not a map, skipping filter validation")
 		return nil
 	}
 
 	// Check push event
 	if pushVal, exists := onMap["push"]; exists {
-		filterValidationLog.Print("Validating push event filters")
+		compilerFiltersValidationLog.Print("Validating push event filters")
 		if err := validateFilterExclusivity(pushVal, "push"); err != nil {
 			return err
 		}
 	}
 
 	// Check pull_request event
 	if prVal, exists := onMap["pull_request"]; exists {
-		filterValidationLog.Print("Validating pull_request event filters")
+		compilerFiltersValidationLog.Print("Validating pull_request event filters")
 		if err := validateFilterExclusivity(prVal, "pull_request"); err != nil {
 			return err
 		}
 	}
 
-	filterValidationLog.Print("Event filter validation completed successfully")
+	compilerFiltersValidationLog.Print("Event filter validation completed successfully")
 	return nil
 }
 
@@ -94,7 +96,7 @@ func ValidateEventFilters(frontmatter map[string]any) error {
 // activate immediately after new lock files are first pushed to the branch, producing
 // zero-turn failures for every agentic workflow in the repository).
 func ValidatePushBranchScope(frontmatter map[string]any) error {
-	filterValidationLog.Print("Validating push event branch/tag scope")
+	compilerFiltersValidationLog.Print("Validating push event branch/tag scope")
 
 	on, exists := frontmatter["on"]
 	if !exists {
@@ -113,7 +115,7 @@ func ValidatePushBranchScope(frontmatter map[string]any) error {
 
 	// A nil push value (bare `push:` key with no sub-keys) is unscoped.
 	if pushVal == nil {
-		filterValidationLog.Print("ERROR: push event has no branch/tag scope (nil push value)")
+		compilerFiltersValidationLog.Print("ERROR: push event has no branch/tag scope (nil push value)")
 		return newUnScopedPushError()
 	}
 
@@ -129,11 +131,11 @@ func ValidatePushBranchScope(frontmatter map[string]any) error {
 	_, hasTagsIgnore := pushMap["tags-ignore"]
 
 	if !hasBranches && !hasBranchesIgnore && !hasTags && !hasTagsIgnore {
-		filterValidationLog.Print("ERROR: push event has no branches or tags scope")
+		compilerFiltersValidationLog.Print("ERROR: push event has no branches or tags scope")
 		return newUnScopedPushError()
 	}
 
-	filterValidationLog.Print("Push event branches or tags scope is valid")
+	compilerFiltersValidationLog.Print("Push event branches or tags scope is valid")
 	return nil
 }
 
@@ -150,7 +152,7 @@ func newUnScopedPushError() *WorkflowValidationError {
 func validateFilterExclusivity(eventVal any, eventName string) error {
 	eventMap, ok := eventVal.(map[string]any)
 	if !ok {
-		filterValidationLog.Printf("Event '%s' is not a map, skipping filter validation", eventName)
+		compilerFiltersValidationLog.Printf("Event '%s' is not a map, skipping filter validation", eventName)
 		return nil
 	}
 
@@ -159,7 +161,7 @@ func validateFilterExclusivity(eventVal any, eventName string) error {
 	_, hasBranchesIgnore := eventMap["branches-ignore"]
 
 	if hasBranches && hasBranchesIgnore {
-		filterValidationLog.Printf("ERROR: Event '%s' has both 'branches' and 'branches-ignore' filters", eventName)
+		compilerFiltersValidationLog.Printf("ERROR: Event '%s' has both 'branches' and 'branches-ignore' filters", eventName)
 		return NewValidationError(
 			"on."+eventName,
 			"branches + branches-ignore",
@@ -173,7 +175,7 @@ func validateFilterExclusivity(eventVal any, eventName string) error {
 	_, hasPathsIgnore := eventMap["paths-ignore"]
 
 	if hasPaths && hasPathsIgnore {
-		filterValidationLog.Printf("ERROR: Event '%s' has both 'paths' and 'paths-ignore' filters", eventName)
+		compilerFiltersValidationLog.Printf("ERROR: Event '%s' has both 'paths' and 'paths-ignore' filters", eventName)
 		return NewValidationError(
 			"on."+eventName,
 			"paths + paths-ignore",
@@ -182,7 +184,7 @@ func validateFilterExclusivity(eventVal any, eventName string) error {
 		)
 	}
 
-	filterValidationLog.Printf("Event '%s' filters are valid", eventName)
+	compilerFiltersValidationLog.Printf("Event '%s' filters are valid", eventName)
 	return nil
 }
 
@@ -198,7 +200,7 @@ var globValidationEvents = []string{"push", "pull_request", "pull_request_target
 // ValidateGlobPatterns validates branch, tag, and path glob patterns in the 'on' section
 // of a workflow's frontmatter. It returns the first validation error encountered, if any.
 func ValidateGlobPatterns(frontmatter map[string]any) error {
-	filterValidationLog.Print("Validating glob patterns in event filters")
+	compilerFiltersValidationLog.Print("Validating glob patterns in event filters")
 
 	on, exists := frontmatter["on"]
 	if !exists {
@@ -235,7 +237,7 @@ func ValidateGlobPatterns(frontmatter map[string]any) error {
 		}
 	}
 
-	filterValidationLog.Print("Glob pattern validation completed successfully")
+	compilerFiltersValidationLog.Print("Glob pattern validation completed successfully")
 	return nil
 }
 
@@ -261,7 +263,7 @@ func validateGlobList(eventMap map[string]any, eventName, filterKey string, isPa
 	}
 
 	return validateGlobPatternList(patterns, validate, func(_ int, pat string, msgs []string) error {
-		filterValidationLog.Printf("ERROR: invalid glob pattern %q in %s.%s: %s", pat, eventName, filterKey, strings.Join(msgs, "; "))
+		compilerFiltersValidationLog.Printf("ERROR: invalid glob pattern %q in %s.%s: %s", pat, eventName, filterKey, strings.Join(msgs, "; "))
 		return NewValidationError(
 			fmt.Sprintf("on.%s.%s", eventName, filterKey),
 			pat,

pkg/workflow/concurrency_validation.go

@@ -30,9 +30,11 @@ package workflow
 import (
 	"regexp"
 	"strings"
+
+	"github.com/github/gh-aw/pkg/logger"
 )
 
-var concurrencyValidationLog = newValidationLogger("concurrency")
+var concurrencyValidationLog = logger.New("workflow:concurrency_validation")
 
 var (
 	concurrencyGroupPattern                = regexp.MustCompile(`(?m)^\s*group:\s*["']?([^"'\n]+?)["']?\s*$`)

pkg/workflow/dangerous_permissions_validation.go

@@ -4,9 +4,11 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+
+	"github.com/github/gh-aw/pkg/logger"
 )
 
-var dangerousPermissionsLog = newValidationLogger("dangerous_permissions")
+var dangerousPermissionsLog = logger.New("workflow:dangerous_permissions_validation")
 
 // validateDangerousPermissions validates that write permissions are not used.
 //

pkg/workflow/dispatch_repository_validation.go

@@ -5,9 +5,11 @@ import (
 	"fmt"
 	"regexp"
 	"strings"
+
+	"github.com/github/gh-aw/pkg/logger"
 )
 
-var dispatchRepositoryValidationLog = newValidationLogger("dispatch_repository")
+var dispatchRepositoryValidationLog = logger.New("workflow:dispatch_repository_validation")
 
 // repoSlugPattern matches a valid owner/repo GitHub repository slug.
 // Owner names: alphanumerics and hyphens (no dots - GitHub usernames/org names cannot have dots).

pkg/workflow/dispatch_workflow_validation.go

@@ -8,11 +8,12 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/repoutil"
 	"github.com/goccy/go-yaml"
 )
 
-var dispatchWorkflowValidationLog = newValidationLogger("dispatch_workflow")
+var dispatchWorkflowValidationLog = logger.New("workflow:dispatch_workflow_validation")
 
 // validateDispatchWorkflow validates that the dispatch-workflow configuration is correct
 func (c *Compiler) validateDispatchWorkflow(data *WorkflowData, workflowPath string) error {

pkg/workflow/docker_validation.go

@@ -49,10 +49,11 @@ import (
 	"time"
 
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/syncutil"
 )
 
-var dockerValidationLog = newValidationLogger("docker")
+var dockerValidationLog = logger.New("workflow:docker_validation")
 
 // dockerDaemonCheckTimeout is how long to wait for `docker info` to respond.
 // If the daemon isn't running, this prevents long hangs on every docker command.

pkg/workflow/engine_validation.go

@@ -45,10 +45,11 @@ import (
 
 	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/parser"
 )
 
-var engineValidationLog = newValidationLogger("engine")
+var engineValidationLog = logger.New("workflow:engine_validation")
 var safeHarnessScriptPattern = regexp.MustCompile(`^[A-Za-z0-9_][A-Za-z0-9._-]*$`)
 
 // safeSDKDriverSegmentPattern allows path segments that may start with a dot followed by an

pkg/workflow/event_validation.go

@@ -31,10 +31,11 @@ import (
 	"slices"
 	"strings"
 
+	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/stringutil"
 )
 
-var eventValidationLog = newValidationLogger("event")
+var eventValidationLog = logger.New("workflow:event_validation")
 
 // validGitHubEventTypes is the list of all supported GitHub Actions event types.
 // Source: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows

pkg/workflow/expression_safety_validation.go

@@ -12,10 +12,11 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/constants"
+	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/parser"
 )
 
-var expressionValidationLog = newValidationLogger("expression")
+var expressionValidationLog = logger.New("workflow:expression_safety_validation")
 
 // maxFuzzyMatchSuggestions is the maximum number of similar expressions to suggest
 // when an unauthorized expression is found

pkg/workflow/expression_secrets_serialization_validation.go

@@ -25,10 +25,11 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/console"
+	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/sliceutil"
 )
 
-var expressionSecretsSerializationLog = newValidationLogger("expression_secrets_serialization")
+var expressionSecretsSerializationLog = logger.New("workflow:expression_secrets_serialization_validation")
 
 // secretsSerializationPattern matches function calls that pass the entire secrets
 // context as an argument, e.g. toJSON(secrets).