chore(workflow): make logger namespaces statically greppable by Copilot · Pull Request #45721 · github/gh-aw

Copilot · 2026-07-15T11:15:59Z

Two patterns in the workflow package made the complete set of logger namespaces impossible to discover statically via grep -rn 'var.*= logger.New'.

Changes

close_entity_helpers.go: Extract three logger.New(...) calls embedded in the closeEntityRegistry slice literal into named package-level vars (logCloseIssue, logClosePullRequest, logCloseDiscussion).
48 *_validation.go files: Replace every newValidationLogger("domain") call — which built the namespace via runtime string concatenation — with a direct logger.New("workflow:domain_validation") string literal.
validation_helpers.go: Remove the now-unused newValidationLogger helper and its doc entry.
push_to_pull_request_branch_validation.go: Fix a latent bug exposed by inlining — the original call newValidationLogger("push_to_pull_request_branch_validation") silently produced "workflow:push_to_pull_request_branch_validation_validation" (doubled suffix). Corrected to "workflow:push_to_pull_request_branch_validation".

Before / After

// Before — namespace only knowable at runtime
var agentValidationLog = newValidationLogger("agent")

// After — fully static and greppable
var agentValidationLog = logger.New("workflow:agent_validation")

No behaviour change beyond the one bug fix above.

…kage - Lift 3 inline logger.New calls in closeEntityRegistry to package-level vars (logCloseIssue, logClosePullRequest, logCloseDiscussion) - Inline all newValidationLogger(domain) call sites (~48 files) with direct logger.New("workflow:domain_validation") string literals - Fix doubled _validation suffix bug in push_to_pull_request_branch_validation.go - Remove newValidationLogger helper function (no remaining callers) Fixes #45509 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot

Pull request overview

Makes workflow logger namespaces statically discoverable while preserving existing behavior.

Changes:

Replaces validation logger helper calls with literal logger.New namespaces.
Extracts close-entity loggers into package variables.
Removes the obsolete helper and fixes the duplicated push-to-PR namespace suffix.

Show a summary per file

File	Description
`pkg/workflow/validation_helpers.go`	Removes the obsolete logger helper.
`pkg/workflow/tools_validation.go`	Uses a static tools namespace.
`pkg/workflow/template_validation.go`	Uses a static template namespace.
`pkg/workflow/template_injection_validation.go`	Uses a static template-injection namespace.
`pkg/workflow/strict_mode_validation.go`	Uses a static strict-mode namespace.
`pkg/workflow/strict_mode_update_check_validation.go`	Uses a static update-check namespace.
`pkg/workflow/step_shell_validator.go`	Uses a static shell-validator namespace.
`pkg/workflow/step_order_validation.go`	Uses a static step-order namespace.
`pkg/workflow/secrets_validation.go`	Uses a static secrets namespace.
`pkg/workflow/schema_validation.go`	Uses a static schema namespace.
`pkg/workflow/sandbox_validation.go`	Uses a static sandbox namespace.
`pkg/workflow/samples_validation.go`	Uses a static samples namespace.
`pkg/workflow/safe_outputs_validation.go`	Inlines safe-output logger namespaces.
`pkg/workflow/safe_outputs_steps_shell_expansion_validation.go`	Uses a static shell-expansion namespace.
`pkg/workflow/safe_outputs_max_validation.go`	Uses a static maximum-value namespace.
`pkg/workflow/safe_outputs_allowed_labels_validation.go`	Uses a static allowed-labels namespace.
`pkg/workflow/runtime_validation.go`	Uses a static runtime namespace.
`pkg/workflow/runs_on_validation.go`	Uses a static runner namespace.
`pkg/workflow/run_install_scripts_validation.go`	Uses a static install-scripts namespace.
`pkg/workflow/repository_features_validation.go`	Uses a static repository-features namespace.
`pkg/workflow/repo_memory_validation.go`	Uses a static repository-memory namespace.
`pkg/workflow/push_to_pull_request_branch_validation.go`	Corrects and inlines the logger namespace.
`pkg/workflow/pull_request_target_validation.go`	Uses a static PR-target namespace.
`pkg/workflow/pip_validation.go`	Uses a static pip namespace.
`pkg/workflow/permissions_toolset_data.go`	Uses a static permissions namespace.
`pkg/workflow/npm_validation.go`	Uses a static npm namespace.
`pkg/workflow/network_firewall_validation.go`	Uses a static network-firewall namespace.
`pkg/workflow/model_alias_validation.go`	Uses a static model-alias namespace.
`pkg/workflow/mcp_scripts_dependencies_validation.go`	Uses a static MCP-dependencies namespace.
`pkg/workflow/mcp_mount_validation.go`	Uses a static MCP-mount namespace.
`pkg/workflow/mcp_config_validation.go`	Uses a static MCP-config namespace.
`pkg/workflow/labels_validation.go`	Uses a static labels namespace.
`pkg/workflow/imported_steps_validation.go`	Uses a static imported-steps namespace.
`pkg/workflow/github_app_permissions_validation.go`	Uses a static app-permissions namespace.
`pkg/workflow/firewall_validation.go`	Uses a static firewall namespace.
`pkg/workflow/features_validation.go`	Uses a static features namespace.
`pkg/workflow/expression_secrets_serialization_validation.go`	Uses a static secrets-serialization namespace.
`pkg/workflow/expression_safety_validation.go`	Uses a static expression namespace.
`pkg/workflow/event_validation.go`	Uses a static event namespace.
`pkg/workflow/engine_validation.go`	Uses a static engine namespace.
`pkg/workflow/docker_validation.go`	Uses a static Docker namespace.
`pkg/workflow/dispatch_workflow_validation.go`	Uses a static workflow-dispatch namespace.
`pkg/workflow/dispatch_repository_validation.go`	Uses a static repository-dispatch namespace.
`pkg/workflow/dangerous_permissions_validation.go`	Uses a static dangerous-permissions namespace.
`pkg/workflow/concurrency_validation.go`	Uses a static concurrency namespace.
`pkg/workflow/compiler_filters_validation.go`	Uses a static filter namespace.
`pkg/workflow/close_entity_helpers.go`	Extracts three close-entity logger variables.
`pkg/workflow/checkout_path_validation.go`	Uses a static checkout-path namespace.
`pkg/workflow/call_workflow_validation.go`	Uses a static called-workflow namespace.
`pkg/workflow/agent_validation.go`	Uses a static agent namespace.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Files reviewed: 50/50 changed files
Comments generated: 0
Review effort level: Medium

github-actions · 2026-07-15T12:31:14Z

✅ PR Code Quality Reviewer completed the code quality review.

github-actions · 2026-07-15T12:31:17Z

✅ Design Decision Gate 🏗️ completed the design decision gate check.

github-actions · 2026-07-15T12:31:18Z

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

github-actions · 2026-07-15T12:31:34Z

✅ Test Quality Sentinel completed test quality analysis.

No test files were added or modified in this PR. Test Quality Sentinel skipped.

github-actions

Skills-Based Review 🧠

Applied /codebase-design — no blocking issues found. ✅

📋 Summary

This is a clean, mechanical refactor that improves static analysability of the codebase. All changes are consistent:

All 48 *_validation.go files correctly inline logger.New("workflow:<domain>_validation") replacing the runtime-concatenating helper.
The doubled-suffix bug in push_to_pull_request_branch_validation.go (_validation_validation) is correctly fixed.
close_entity_helpers.go extracts the three embedded logger.New calls into named package-level vars, making them greppable.
validation_helpers.go cleanly removes the now-unused newValidationLogger function and its doc entry.

Positive Highlights

✅ Namespace strings are now fully static — grep -rn 'logger.New' gives a complete inventory.
✅ The latent doubled-suffix bug is fixed as a well-documented side effect.
✅ No behaviour change beyond the one bug fix, clearly stated in the PR description.
✅ Consistent naming convention maintained across all files.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 21.3 AIC · ⌖ 4.37 AIC · ⊞ 6.7K
_{Comment /matt to run again}

github-actions

The changes are clean and achieve the stated goal of making logger namespaces statically greppable via grep -rn 'var.*= logger.New'.

All 48 validation files follow a consistent naming scheme (workflow:<domain>_validation), and the three close-entity loggers are correctly extracted from the slice literal into named package-level vars.

One minor note: stepShellValidatorLog gets the namespace workflow:step_shell_validator_validation — the _validator_validation suffix is slightly redundant, but this is non-blocking and consistent with the variable name.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 18.3 AIC · ⌖ 5.38 AIC · ⊞ 5K

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-actions · 2026-07-15T12:38:40Z

🏗️ Design Decision Gate — ADR Required

This PR makes significant changes to core business logic (123 new lines in pkg/ — exceeds the 100-line threshold) but does not have a linked Architecture Decision Record (ADR).

📄 Draft ADR committed: docs/adr/45721-make-logger-namespaces-statically-greppable.md — review and complete it before merging.

🔒 This PR cannot merge until an ADR is linked in the PR body.

📋 What to do next

Review the draft ADR committed to your branch — it was generated from the PR diff
Complete the missing sections — confirm the alternatives are the ones you actually weighed, and add any context the AI could not infer
Commit the finalized ADR to docs/adr/ on your branch (or accept the draft as-is)
Reference the ADR in this PR body by adding a line such as:

ADR: ADR-45721: Make Workflow Logger Namespaces Statically Greppable

Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision.

❓ Why ADRs Matter

"AI made me procrastinate on key design decisions. Because refactoring was cheap, I could always say 'I'll deal with this later.' Deferring decisions corroded my ability to think clearly."

ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you.

📋 Michael Nygard ADR Format Reference

An ADR must contain these four sections to be considered complete:

Context — What is the problem? What forces are at play?
Decision — What did you decide? Why?
Alternatives Considered — What else could have been done?
Consequences — What are the trade-offs (positive and negative)?

All ADRs are stored in docs/adr/ as Markdown files numbered by PR number (e.g., 45721-make-logger-namespaces-statically-greppable.md for PR #45721).

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · 73.1 AIC · ⌖ 13.2 AIC · ⊞ 8.5K · ◷
_{Comment /review to run again}

github-actions

REQUEST_CHANGES — The inlining approach is correct, and the push_to_pull_request_branch doubled-suffix fix is a good catch. However, the same doubled-suffix bug and several namespace-to-filename mismatches were introduced or carried forward unchanged.

### Findings summary

File	Issue
`step_shell_validator.go`	Doubled `_validation` suffix — same class of bug as the one explicitly fixed, missed here
`expression_safety_validation.go`	`_safety` dropped from namespace
`compiler_filters_validation.go`	`compiler_` prefix and pluralization dropped
`strict_mode_update_check_validation.go`	`strict_mode_` prefix dropped
`mcp_scripts_dependencies_validation.go`	`scripts` → `script` (singular/plural)

All five introduce exactly the namespace-to-filename drift this PR is meant to eliminate. The step_shell_validator.go bug is the most egregious because it was explicitly called out in the PR description as a fixed pattern.

🔎 Code quality review by PR Code Quality Reviewer · 71 AIC · ⌖ 4.78 AIC · ⊞ 5.6K
_{Comment /review to run again}

github-actions · 2026-07-15T12:39:25Z

 )

-var stepShellValidatorLog = newValidationLogger("step_shell_validator")
+var stepShellValidatorLog = logger.New("workflow:step_shell_validator_validation")


Doubled _validation suffix bug: namespace "workflow:step_shell_validator_validation" has a spurious _validation suffix — the same doubled-suffix class of bug the PR explicitly fixes for push_to_pull_request_branch_validation.go, but left unaddressed here.

💡 Suggested fix

step_shell_validator.go is not a _validation.go file; the old newValidationLogger helper was passed "step_shell_validator" as its domain, then blindly appended "_validation", producing the doubled form. The corrected namespace should be:

var stepShellValidatorLog = logger.New("workflow:step_shell_validator")

This makes the namespace match the file stem exactly, consistent with how close_entity_helpers.go loggers use their own domain names without the _validation suffix.

github-actions · 2026-07-15T12:39:25Z

 )

-var expressionValidationLog = newValidationLogger("expression")
+var expressionValidationLog = logger.New("workflow:expression_validation")


Namespace drops _safety component: "workflow:expression_validation" silently discards _safety, making it indistinguishable from a hypothetical expression_validation.go and defeating the static-grep-discoverability goal of this PR.

💡 Suggested fix

The old newValidationLogger("expression") convention predates safety/non-safety split, so the short domain was acceptable. Now that namespaces are inlined and meant to be grep-stable, the namespace should reflect the actual file:

var expressionValidationLog = logger.New("workflow:expression_safety_validation")

Note: you may also want to rename the variable from expressionValidationLog to expressionSafetyValidationLog to stay consistent.

github-actions · 2026-07-15T12:39:25Z

 )

-var filterValidationLog = newValidationLogger("filter")
+var filterValidationLog = logger.New("workflow:filter_validation")


Namespace loses compiler_ prefix and pluralization: "workflow:filter_validation" drops both compiler_ and the s in filters, making the static string not grep-discoverable from the filename.

💡 Suggested fix

To satisfy the PR's stated goal — namespace derivable from filename by grep — the namespace should mirror the file stem:

var filterValidationLog = logger.New("workflow:compiler_filters_validation")

Also consider renaming the variable filterValidationLog → compilerFiltersValidationLog for full consistency.

github-actions · 2026-07-15T12:39:25Z

 )

-var updateCheckValidationLog = newValidationLogger("update_check")
+var updateCheckValidationLog = logger.New("workflow:update_check_validation")


Namespace drops strict_mode_ prefix: "workflow:update_check_validation" omits the strict_mode_ context present in the filename, weakening the grep-discoverability guarantee this PR is meant to provide.

💡 Suggested fix

var updateCheckValidationLog = logger.New("workflow:strict_mode_update_check_validation")

The strict_mode_ prefix is important — it indicates this check only fires in strict mode. Losing it in the namespace makes log output harder to triage.

github-actions · 2026-07-15T12:39:25Z

 )

-var mcpScriptDepsValidationLog = newValidationLogger("mcp_script_dependencies")
+var mcpScriptDepsValidationLog = logger.New("workflow:mcp_script_dependencies_validation")


Singular/plural mismatch: namespace uses mcp_script_dependencies_validation (singular script) while the filename is mcp_scripts_dependencies_validation.go (plural scripts) — one character difference introduced during inlining.

💡 Suggested fix

var mcpScriptDepsValidationLog = logger.New("workflow:mcp_scripts_dependencies_validation")

This is the exact kind of silent drift the PR aims to eliminate.

github-actions · 2026-07-15T12:47:39Z

🤖 PR Triage

Field	Value
Category	`refactor`
Risk	🟢 Low
Priority Score	45/100
Recommended Action	📋 `batch_review`
Batch	`refactor-go-pkg` (with #45700)

Score breakdown: Impact 20 + Urgency 10 + Quality 15

Rationale: Extracts logger namespace vars to make them statically greppable. Low risk refactor with ADR. CI status unknown (CHANGES_REQUESTED). Can be batch-reviewed alongside #45700.

Generated by 🔧 PR Triage Agent · 35 AIC · ⌖ 7.73 AIC · ⊞ 5.6K · ◷

gh-aw-bot · 2026-07-15T13:52:48Z

pr-sous-chef
@copilot please run the pr-finisher skill, address unresolved review comments, and rerun checks once finished.

Run: https://github.com/github/gh-aw/actions/runs/29420425558

Generated by 👨‍🍳 PR Sous Chef · 10.2 AIC · ⌖ 4.88 AIC · ⊞ 5.1K · ◷
_{Comment /souschef to run again}

- step_shell_validator.go: remove spurious _validation suffix (step_shell_validator_validation → step_shell_validator) - expression_safety_validation.go: restore dropped _safety component (expression_validation → expression_safety_validation) - compiler_filters_validation.go: restore compiler_ prefix and plural (filter_validation → compiler_filters_validation), rename var - strict_mode_update_check_validation.go: restore strict_mode_ prefix (update_check_validation → strict_mode_update_check_validation) - mcp_scripts_dependencies_validation.go: fix singular/plural mismatch (mcp_script_dependencies_validation → mcp_scripts_dependencies_validation), rename var Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot · 2026-07-15T14:05:36Z

pr-sous-chef
@copilot please run the pr-finisher skill, address unresolved review comments, and rerun checks once finished.
...

All 5 namespace mismatches from the PR Code Quality Reviewer have been fixed in 4ec772f:

step_shell_validator.go: step_shell_validator_validation → step_shell_validator (spurious _validation suffix removed; file is not a _validation.go file)
expression_safety_validation.go: expression_validation → expression_safety_validation (dropped _safety restored)
compiler_filters_validation.go: filter_validation → compiler_filters_validation (dropped compiler_ prefix and plural s restored; var renamed to compilerFiltersValidationLog)
strict_mode_update_check_validation.go: update_check_validation → strict_mode_update_check_validation (dropped strict_mode_ prefix restored)
mcp_scripts_dependencies_validation.go: mcp_script_dependencies_validation → mcp_scripts_dependencies_validation (singular→plural fixed; var renamed to mcpScriptsDepsValidationLog)

make fmt, make lint, and pkg/workflow tests all pass locally. CI needs a human re-trigger to confirm.

Done

github-actions · 2026-07-16T06:42:09Z

🎉 This pull request is included in a new release.

Release: v0.82.10

Initial plan

bed98d8

Copilot AI assigned Copilot and pelikhan Jul 15, 2026

Copilot AI linked an issue Jul 15, 2026 that may be closed by this pull request

[deep-report] Make inline/dynamic logger.New calls greppable in workflow package #45509

Closed

Copilot started work on behalf of pelikhan July 15, 2026 11:16 View session

Copilot AI changed the title ~~[WIP] Lift logger.New calls for greppable logger namespaces~~ chore(workflow): make logger namespaces statically greppable Jul 15, 2026

Copilot finished work on behalf of pelikhan July 15, 2026 11:33

Copilot AI requested a review from pelikhan July 15, 2026 11:33

pelikhan marked this pull request as ready for review July 15, 2026 12:23

Copilot AI review requested due to automatic review settings July 15, 2026 12:23

Copilot started reviewing on behalf of pelikhan July 15, 2026 12:23 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

github-actions Bot approved these changes Jul 15, 2026

View reviewed changes

docs(adr): draft ADR-45721 for static logger namespace refactor

bd8faae

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

github-actions Bot previously requested changes Jul 15, 2026

View reviewed changes

github-actions Bot added pr-action:batch_review pr-agent:copilot-swe-agent pr-priority:medium pr-risk:low pr-type:refactor pr-batch:refactor-go-pkg labels Jul 15, 2026

github-actions Bot mentioned this pull request Jul 15, 2026

Share nolint and generated-file indexes across custom Go analyzers #45700

Merged

This was referenced Jul 15, 2026

[PR Triage Report] PR Triage Report — 2026-07-15 [Run 29416064927] #45730

Closed

[Contribution Check Report] Contribution Check — 2026-07-15 #45590

Closed

github-actions Bot mentioned this pull request Jul 15, 2026

[pr-sous-chef] Run report — 0 nudged, 0 skipped #45581

Closed

Copilot started work on behalf of gh-aw-bot July 15, 2026 13:53 View session

Copilot finished work on behalf of gh-aw-bot July 15, 2026 14:06

Copilot AI requested a review from gh-aw-bot July 15, 2026 14:06

pelikhan merged commit 594328e into main Jul 15, 2026
20 checks passed

pelikhan deleted the copilot/deep-report-fix-inline-logger-calls branch July 15, 2026 14:10

Uh oh!

Conversation

Copilot AI commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Before / After

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

github-actions Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Positive Highlights

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 15, 2026

🏗️ Design Decision Gate — ADR Required

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 15, 2026

🤖 PR Triage

Uh oh!

gh-aw-bot commented Jul 15, 2026

Uh oh!

Copilot AI commented Jul 15, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Jul 15, 2026 •

edited

Loading

github-actions Bot commented Jul 15, 2026 •

edited

Loading

github-actions Bot commented Jul 15, 2026 •

edited

Loading

github-actions Bot commented Jul 15, 2026 •

edited

Loading

github-actions Bot commented Jul 15, 2026 •

edited

Loading