[repository-quality] Repository Quality Improvement Report — Logger Name Collision & Instantiation Hygiene

[github-actions[bot]]

🎯 Repository Quality Improvement Report — Logger Name Collision & Instantiation Hygiene

Analysis Date: 2026-07-14
Focus Area: Logger Name Collision & Instantiation Hygiene
Strategy Type: Custom
Custom Area: Yes — the pkg/logger subsystem uses namespace strings as its filtering identity; collisions silently corrupt DEBUG= filter behaviour at runtime

Executive Summary

The project uses pkg/logger with namespace strings as the primary observability mechanism (DEBUG=cli:update_check filters log output). Three namespace strings are declared more than once across independent source files, meaning DEBUG=cli:update_check enables two unrelated loggers that share the same label — confusing operators and obscuring which subsystem emitted a line. Six packages (pkg/linters/* and pkg/modelsdev) declare var log = logger.New(...), shadowing the stdlib log identifier and masking accidental raw-log calls from the rawloginlib linter. Three logger.New calls live inside a static slice literal (struct fields) rather than as package-level variables, creating logger objects with no greppable variable identity. One helper (newValidationLogger) constructs logger names by runtime string concatenation, making the full set of names statically unknowable.

None of these issues are crashes, but collectively they erode the namespace-based filtering contract that operators rely on when debugging agentic runs. All four tasks are small, mechanical, and independently deliverable.

Full Analysis Report

Focus Area: Logger Name Collision & Instantiation Hygiene

Current State Assessment

The codebase contains 685 logger.New(...) calls across 683 source files producing 682 unique namespace strings.

Metrics Collected:

Metric	Value	Status
Total logger.New calls	685	✅
Unique namespace strings	682	⚠️
Duplicate namespace strings	3	❌
var log shadowing variables	6	⚠️
Inline (struct-literal) logger.New calls	3	⚠️
Dynamic (runtime-concat) logger names	1	⚠️
Callers of newValidationLogger	10	i️

Findings

Strengths

• Consistent pkg:component naming convention used across all 682+ namespaces — zero deviations from the colon-separator scheme
• rawloginlib linter already prevents raw log.Print* calls in library packages
• Logger namespaces are well-granular (file-level) enabling precise filtering

Areas for Improvement

• [HIGH] cli:update_check declared in both pkg/cli/compile_update_check.go (line 21) and pkg/cli/update_check.go (line 18) — two unrelated update-check subsystems share one filter label
• [HIGH] var log = logger.New(...) in 6 files shadows the stdlib log identifier: pkg/linters/largefunc, deferinloop, httprespbodyclose, httpstatuscode, excessivefuncparams, and pkg/modelsdev/catalog.go — rawloginlib linter cannot fire because log resolves to pkg/logger not stdlib log
• [MEDIUM] Three logger.New calls inside closeEntityRegistry slice literal (pkg/workflow/close_entity_helpers.go lines 172, 185, 198) are not reachable by grep -rn 'var.*Log.*= logger.New' audits
• [MEDIUM] newValidationLogger in pkg/workflow/validation_helpers.go:48 builds names via runtime concatenation — all 10 call sites pass string literals and could be inlined

Detailed Analysis

Duplicate 1 — cli:update_check

compile_update_check.go handles compile-time version checking (unauthenticated HTTP probe), while update_check.go handles authenticated gh-release version checking. Running DEBUG=cli:update_check produces interleaved output from both, defeating namespace isolation.

var log shadowing

rawloginlib detects log.Print*(...) calls only when log resolves to the stdlib log package via astutil.IsPkgSelector. When a file declares var log = logger.New(...), the log symbol resolves to pkg/logger.Logger and the linter is blind to it. Six production files are affected.

Inline logger instantiation in closeEntityRegistry

The three Logger: logger.New("workflow:close_*") struct field initializers diverge from the convention (var xxxLog = logger.New(...) at package scope) and are invisible to standard auditing greps.

Dynamic name construction

newValidationLogger("domain") → "workflow:" + domain + "_validation" makes the 10 generated names unreachable by static tooling. All 10 callers already pass string literals.

---

🤖 Tasks for Copilot Agent

NOTE TO PLANNER AGENT: Split the following tasks into individual work items.

Improvement Tasks

Task 1: Rename colliding cli:update_check logger in compile_update_check.go

Priority: High
Estimated Effort: Small
Focus Area: Logger Name Collision & Instantiation Hygiene

Description: Two independent source files both call logger.New("cli:update_check"). Rename the logger in pkg/cli/compile_update_check.go to "cli:compile_update_check" so the two subsystems have distinct, filterable namespace strings.

Acceptance Criteria:

• pkg/cli/compile_update_check.go line 21: logger.New("cli:update_check") → logger.New("cli:compile_update_check")
• grep -rn 'logger.New("cli:update_check")' pkg/ returns exactly one result (in update_check.go)
• make fmt && make lint passes

Code Region: pkg/cli/compile_update_check.go:21

In `pkg/cli/compile_update_check.go`, change the package-level variable declaration:

  var compileUpdateCheckLog = logger.New("cli:update_check")

to:

  var compileUpdateCheckLog = logger.New("cli:compile_update_check")

After the change verify:
1. `grep -rn 'logger.New("cli:update_check")' pkg/` returns exactly one match (update_check.go only).
2. `make fmt && make lint` passes.

---

Task 2: Rename var log to descriptive names in linter and modelsdev packages

Priority: High
Estimated Effort: Small
Focus Area: Logger Name Collision & Instantiation Hygiene

Description: Six source files declare var log = logger.New(...), shadowing the stdlib log identifier. Rename each to a descriptive name following the var xxxLog = logger.New(...) convention.

Acceptance Criteria:

• pkg/linters/largefunc/largefunc.go: var log → var largefuncLog; all log.Printf(...) call sites updated
• pkg/linters/deferinloop/deferinloop.go: var log → var deferinloopLog; call sites updated
• pkg/linters/httprespbodyclose/httprespbodyclose.go: var log → var httprespbodycloseLog; call sites updated
• pkg/linters/httpstatuscode/httpstatuscode.go: var log → var httpstatuscodeLog; call sites updated
• pkg/linters/excessivefuncparams/excessivefuncparams.go: var log → var excessivefuncparamsLog; call sites updated
• pkg/modelsdev/catalog.go: var log → var catalogLog; call sites updated
• grep -rn '^var log = logger.New' pkg/ returns zero results
• make fmt && make lint passes

Code Region: pkg/linters/largefunc/largefunc.go, pkg/linters/deferinloop/deferinloop.go, pkg/linters/httprespbodyclose/httprespbodyclose.go, pkg/linters/httpstatuscode/httpstatuscode.go, pkg/linters/excessivefuncparams/excessivefuncparams.go, pkg/modelsdev/catalog.go

In each of the six files below, rename `var log = logger.New(...)` to a descriptive name and update all call sites in the same file.

Rename targets:
- `pkg/linters/largefunc/largefunc.go`: `var log` → `var largefuncLog`
- `pkg/linters/deferinloop/deferinloop.go`: `var log` → `var deferinloopLog`
- `pkg/linters/httprespbodyclose/httprespbodyclose.go`: `var log` → `var httprespbodycloseLog`
- `pkg/linters/httpstatuscode/httpstatuscode.go`: `var log` → `var httpstatuscodeLog`
- `pkg/linters/excessivefuncparams/excessivefuncparams.go`: `var log` → `var excessivefuncparamsLog`
- `pkg/modelsdev/catalog.go`: `var log` → `var catalogLog`

After all changes: `grep -rn '^var log = logger.New' pkg/` must return zero results. `make fmt && make lint` must pass.

---

Task 3: Hoist inline logger.New calls in closeEntityRegistry to package-level variables

Priority: Medium
Estimated Effort: Small
Focus Area: Logger Name Collision & Instantiation Hygiene

Description: Three entries in the closeEntityRegistry slice in pkg/workflow/close_entity_helpers.go (lines 172, 185, 198) instantiate loggers inline as struct field values, diverging from the package-level var xxxLog = logger.New(...) convention.

Acceptance Criteria:

• Three new package-level vars added: var closeIssueLog, var closePRLog, var closeDiscussionLog
• closeEntityRegistry entries reference these variables instead of inline logger.New(...) calls
• Logger namespace strings unchanged ("workflow:close_issue", "workflow:close_pull_request", "workflow:close_discussion")
• grep -n 'logger.New(' pkg/workflow/close_entity_helpers.go returns zero results
• make fmt && make lint passes

Code Region: pkg/workflow/close_entity_helpers.go:172,185,198

In `pkg/workflow/close_entity_helpers.go`, extract the three inline logger instantiations from the `closeEntityRegistry` slice literal to package-level variables:

  var closeIssueLog      = logger.New("workflow:close_issue")
  var closePRLog         = logger.New("workflow:close_pull_request")
  var closeDiscussionLog = logger.New("workflow:close_discussion")

Replace the struct field values `Logger: logger.New("workflow:close_issue")` etc. with `Logger: closeIssueLog` etc.

After the change: `grep -n 'logger.New(' pkg/workflow/close_entity_helpers.go` must return zero results. `make fmt && make lint` must pass.

---

Task 4: Replace newValidationLogger dynamic construction with explicit package-level vars

Priority: Medium
Estimated Effort: Small
Focus Area: Logger Name Collision & Instantiation Hygiene

Description: newValidationLogger(domain string) in pkg/workflow/validation_helpers.go:48 builds logger names at runtime. All 10 callers pass string literals and can be inlined, making the full name set statically enumerable.

Acceptance Criteria:

• newValidationLogger function removed from pkg/workflow/validation_helpers.go
• Each of the 10 callers replaced with logger.New("workflow:<domain>_validation") directly
• grep -rn 'newValidationLogger' pkg/ returns zero results
• make fmt && make lint && make test passes

Code Region: pkg/workflow/validation_helpers.go:48, pkg/workflow/features_validation.go, pkg/workflow/network_firewall_validation.go, pkg/workflow/pull_request_target_validation.go, pkg/workflow/runtime_validation.go, pkg/workflow/tools_validation.go, pkg/workflow/compiler_filters_validation.go, pkg/workflow/event_validation.go, pkg/workflow/agent_validation.go, pkg/workflow/runs_on_validation.go, pkg/workflow/dangerous_permissions_validation.go

In `pkg/workflow/validation_helpers.go`, delete the `newValidationLogger` helper function:

  func newValidationLogger(domain string) *logger.Logger {
      return logger.New("workflow:" + domain + "_validation")
  }

In each of the 10 calling files, replace `newValidationLogger("<domain>")` with the explicit string:
  `logger.New("workflow:<domain>_validation")`

Callers and their inline replacements:
- `features_validation.go`: `newValidationLogger("features")` → `logger.New("workflow:features_validation")`
- `network_firewall_validation.go`: → `logger.New("workflow:network_firewall_validation")`
- `pull_request_target_validation.go`: → `logger.New("workflow:pull_request_target_validation")`
- `runtime_validation.go`: → `logger.New("workflow:runtime_validation")`
- `tools_validation.go`: → `logger.New("workflow:tools_validation")`
- `compiler_filters_validation.go`: → `logger.New("workflow:filter_validation")`
- `event_validation.go`: → `logger.New("workflow:event_validation")`
- `agent_validation.go`: → `logger.New("workflow:agent_validation")`
- `runs_on_validation.go`: → `logger.New("workflow:runs_on_validation")`
- `dangerous_permissions_validation.go`: → `logger.New("workflow:dangerous_permissions_validation")`

After: `grep -rn 'newValidationLogger' pkg/` must return zero results. `make fmt && make lint && make test` must pass.

---

📊 Historical Context

Previous Focus Areas

Date	Focus Area	Type	Custom	Key Outcomes
2026-07-08	Deprecated API Alias Proliferation & Zombie Callsite Cleanup	Custom	Y	3 deprecated callsites, zombie no-op, always-true guards
2026-07-07	Environment Variable Access Fragmentation	Custom	Y	45 nolint suppressions, 6 duplicated env vars
2026-07-06	Context-Blind Sleep Gap	Custom	Y	4 functions with 7 time.Sleep calls uncancellable
2026-07-03	ADR-42150 Implementation Gap	Custom	Y	5 sites still using underscore key form
2026-07-02	Remote Fetch API Context Gap	Custom	Y	8 public API wrappers stripped caller context

---

🎯 Recommendations

Immediate Actions (This Week)

1. Rename cli:update_check collision in pkg/cli/compile_update_check.go — Priority: High
2. Rename var log shadow variables in 6 linter/modelsdev files — Priority: High

Short-term Actions (This Month)

1. Hoist inline closeEntityRegistry loggers to package-level vars — Priority: Medium
2. Inline newValidationLogger call sites and delete the helper — Priority: Medium

Long-term Actions (This Quarter)

1. Add a duplicateloggernames linter rule to detect duplicate logger.New("...") strings at compile time — Priority: Low

---

📈 Success Metrics

• Duplicate logger names: 3 → 0
• var log shadow declarations: 6 → 0
• Inline logger instantiations: 3 → 0
• Dynamically-constructed logger names: 1 → 0

---

Next Steps

1. Review and prioritise the 4 tasks above
2. Assign tasks to Copilot coding agent via planner agent
3. Re-evaluate this focus area in 30 days

---

References:

• §29335766196

Generated by Repository Quality Improvement Agent

Generated by ⚡ Repository Quality Improvement Agent · 79 AIC · ⌖ 6.2 AIC · ⊞ 6.6K · ◷

• expires on Jul 15, 2026, 5:27 AM UTC-08:00