refactor: semantic function clustering — dedup linter helpers, drop stub files, rename outliers#33434
Merged
Merged
Conversation
9 tasks
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/129bbae6-dd17-410b-bb2e-8e77a0bc97fa Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
…ename outliers
- Extract pkg/linters/internal/nolint with HasDirective, BuildLineIndex,
ImplementsError; update errstringmatch and errormessage linters to use it
- Delete 6 comment-only stub files: pkg/cli/{list_command,verify_command,
compile_batch_operations,gateway_logs,run_command}.go and
pkg/workflow/scripts.go
- Rename update_check_validation.go → strict_mode_update_check_validation.go
(and its test file) to align with the strict_mode_* cluster
- Refresh file-list comments in validation.go (remove nonexistent bundler_*/
gateway_validation files, fix expression_safety.go → expression_safety_validation.go)
and strict_mode_validation.go (add steps, sandbox, update_check variants)
- Inline gitutil.IsValidFullSHA at all call sites; delete the two trivial
wrapper functions in features_validation.go and actionpins.go
- Rename resolveContainerDigest → fetchContainerDigest in update_container_pins.go
and resolveContainerDigest → lookupContainerDigest in awf_helpers.go
- Move boolPtr from pkg/cli/util.go into pkg/cli/mcp_helpers.go; delete util.go
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/129bbae6-dd17-410b-bb2e-8e77a0bc97fa
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/129bbae6-dd17-410b-bb2e-8e77a0bc97fa Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Refactor linter helpers and clean up unused files
refactor: semantic function clustering — dedup linter helpers, drop stub files, rename outliers
May 20, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors and consolidates scattered helpers and stub files, adds strict-mode validation for check-for-updates, and resolves a few naming/collision inconsistencies uncovered by semantic clustering.
Changes:
- Extract shared
nolintdirective +error-interface helpers intopkg/linters/internal/nolintand update linters to use them. - Add strict-mode policy validation (error in strict, warning otherwise) for
check-for-updates: false, with tests and activation-job step coverage. - Cleanup/housekeeping: delete comment-only stubs, rename digest helpers to avoid collisions, and inline trivial SHA-validation wrappers.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/validation.go | Updates the validator file-list comment to match current files. |
| pkg/workflow/strict_mode_validation.go | Updates strict-mode validator file-list comment to include new/omitted validators. |
| pkg/workflow/strict_mode_update_check_validation.go | Adds compiler validation enforcing strict-mode policy for check-for-updates: false. |
| pkg/workflow/strict_mode_update_check_validation_test.go | Adds unit + compile-output tests for the update-check policy and activation step inclusion. |
| pkg/workflow/scripts.go | Deletes a comment-only stub file. |
| pkg/workflow/safe_outputs_actions.go | Switches SHA detection to gitutil.IsValidFullSHA directly. |
| pkg/workflow/features_validation.go | Removes local SHA wrapper usage in favor of gitutil.IsValidFullSHA. |
| pkg/workflow/features_validation_test.go | Updates SHA-validation test to call gitutil.IsValidFullSHA directly. |
| pkg/workflow/awf_helpers.go | Renames digest lookup helper to avoid cross-package name collision. |
| pkg/linters/internal/nolint/nolint.go | Adds shared nolint directive indexing and error-interface helpers for linters. |
| pkg/linters/errstringmatch/errstringmatch.go | Uses shared nolint helpers; removes duplicated local implementations. |
| pkg/linters/errormessage/errormessage.go | Uses shared nolint helpers; removes duplicated local implementations. |
| pkg/cli/verify_command.go | Deletes a comment-only stub file. |
| pkg/cli/util.go | Deletes single-function util file after moving helper. |
| pkg/cli/update_container_pins.go | Renames digest resolver function to fetchContainerDigest for clarity/uniqueness. |
| pkg/cli/run_command.go | Deletes a comment-only stub file. |
| pkg/cli/mcp_helpers.go | Moves boolPtr helper here from deleted util file. |
| pkg/cli/list_command.go | Deletes a stub file containing only a package declaration. |
| pkg/cli/gateway_logs.go | Deletes a comment-only stub file. |
| pkg/cli/compile_batch_operations.go | Deletes a comment-only stub file. |
| pkg/actionpins/actionpins.go | Inlines SHA validation via gitutil.IsValidFullSHA (removes trivial wrapper). |
| .github/workflows/mcp-inspector.lock.yml | Updates Datadog header env-var mapping in the workflow lock file. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 20/22 changed files
- Comments generated: 2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Semantic analysis of
pkg/surfaced seven actionable cleanup categories: near-identical duplicated helpers across two linter packages, comment-only stub files, a misnamed validation file, stale doc-comment file lists, trivial wrapper duplicates, a same-name collision across packages, and a single-functionutil.go.Shared linter helpers
Extracted
pkg/linters/internal/nolintwith three functions used identically by botherrstringmatchanderrormessage:Deleted 6 comment-only stub files
All contained zero Go declarations — only stale redirect comments:
pkg/cli/{list_command,verify_command,compile_batch_operations,gateway_logs,run_command}.gopkg/workflow/scripts.goRenamed
update_check_validation.go→strict_mode_update_check_validation.goDespite the
update_*prefix, the file validates thecheck-for-updates: falsestrict-mode flag, not the safe-output update-entity cluster. Aligns withstrict_mode_{permissions,network,env,steps,sandbox}_validation.go.Fixed stale file-list comments
validation.go: removed nonexistentbundler_*andgateway_validation.goentries; fixedexpression_safety.go→expression_safety_validation.gostrict_mode_validation.go: added missingsteps,sandbox, andupdate_checkvariants to the sibling-file listInlined
gitutil.IsValidFullSHAwrappersRemoved the two trivial 1-line wrappers in
features_validation.goandactionpins.go; updatedsafe_outputs_actions.goto callgitutil.IsValidFullSHAdirectly.Resolved
resolveContainerDigestname collisionThe same name existed in two packages with completely different semantics:
pkg/cli/update_container_pins.go→fetchContainerDigest(queries Docker buildx / crane / docker pull)pkg/workflow/awf_helpers.go→lookupContainerDigest(pure pin-cache lookup, returns""on miss)Moved
boolPtrintomcp_helpers.go; deletedutil.gopkg/cli/util.gowas 4 lines with a single helper used only by MCP tool files. Moved tomcp_helpers.go— its natural home — and deleted the file.