Skip to content

refactor: eliminate integrity-level duplication, relocate truncateCacheKeyForLog, extract jsonutil package#6911

Merged
lpcox merged 3 commits into
mainfrom
copilot/refactor-internal-file-organization
Jun 2, 2026
Merged

refactor: eliminate integrity-level duplication, relocate truncateCacheKeyForLog, extract jsonutil package#6911
lpcox merged 3 commits into
mainfrom
copilot/refactor-internal-file-organization

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 2, 2026

Three static-analysis findings where code lived in the wrong place or was independently redefined, creating silent drift risk and semantic confusion.

Finding 1 — Single source of truth for integrity levels

guard/wasm_validate.go hardcoded ["none","unapproved","approved","merged"] independently of the identical constants already in config. Adding a new level required three edits.

  • Added config.AllIntegrityLevels() as the canonical exported accessor
  • Kept the backing integrity-level list unexported in config and return a defensive copy to prevent mutation drift
  • guard.AllowedIntegrityLevels now derives from config.AllIntegrityLevels()
  • allowedGuardPolicyIntegrityLevels in config/guard_policy_validation.go now derives from config.AllIntegrityLevels()
  • Added a test to confirm callers cannot mutate the canonical integrity-level list
// before
var AllowedIntegrityLevels = []string{"none", "unapproved", "approved", "merged"}

// after
var AllowedIntegrityLevels = config.AllIntegrityLevels()

Finding 2 — Move truncateCacheKeyForLog to its only call site

The function lived in http_helpers.go but was exclusively called from routed.go (3 call sites, all in cache eviction logging). Moved it there; removed the now-unused fmt and strings imports from http_helpers.go.

Finding 3 — Extract DeepCloneJSON from strutil

strutil is a string-formatting package; DeepCloneJSON operates on arbitrary interface{} values and doesn't belong there. Created internal/jsonutil package, moved the function and its tests, updated the one production caller (proxy/response_transform.go) and three proxy test files.

…extract jsonutil package

- Finding 1: Add config.AllIntegrityLevels exported slice; update
  guard/wasm_validate.go to reference config constants instead of
  hardcoded strings; update guard_policy_validation.go to alias the
  exported slice.

- Finding 2: Move truncateCacheKeyForLog from http_helpers.go to
  routed.go (its only call site), removing now-unused fmt/strings
  imports from http_helpers.go.

- Finding 3: Create internal/jsonutil package with DeepCloneJSON;
  remove it from strutil/util.go; update proxy/response_transform.go
  and all proxy test files to use jsonutil.DeepCloneJSON.
Copilot AI changed the title [WIP] Refactor integrity-level duplication and minor file organization refactor: eliminate integrity-level duplication, relocate truncateCacheKeyForLog, extract jsonutil package Jun 2, 2026
Copilot finished work on behalf of lpcox June 2, 2026 23:15
Copilot AI requested a review from lpcox June 2, 2026 23:15
@lpcox lpcox marked this pull request as ready for review June 2, 2026 23:21
Copilot AI review requested due to automatic review settings June 2, 2026 23:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors shared validation/utilities to reduce duplication and drift risk by centralizing integrity-level definitions, relocating a cache-key logging helper to its sole call site, and extracting JSON-cloning logic into a dedicated internal/jsonutil package.

Changes:

  • Centralized integrity-level lists/constants via config.AllIntegrityLevels and updated guard/validation call sites.
  • Moved truncateCacheKeyForLog from http_helpers.go to routed.go and cleaned up unused imports.
  • Extracted DeepCloneJSON into internal/jsonutil, migrated its unit tests, and updated proxy code/tests to use the new package.
Show a summary per file
File Description
internal/strutil/util.go Removes DeepCloneJSON from strutil to keep the package focused on string helpers.
internal/strutil/util_test.go Drops DeepCloneJSON unit tests now that cloning moved to jsonutil.
internal/server/routed.go Adds truncateCacheKeyForLog at its only usage site (routed-mode cache logging).
internal/server/http_helpers.go Removes now-unused truncateCacheKeyForLog and associated imports.
internal/proxy/response_transform.go Switches deep-cloning to jsonutil.DeepCloneJSON.
internal/proxy/response_transform_test.go Updates tests to reference jsonutil.DeepCloneJSON.
internal/proxy/response_transform_coverage_test.go Updates coverage tests to reference jsonutil.DeepCloneJSON.
internal/proxy/proxy_test.go Updates proxy tests to reference jsonutil.DeepCloneJSON.
internal/jsonutil/clone.go Introduces jsonutil.DeepCloneJSON in a semantically appropriate package.
internal/jsonutil/clone_test.go Adds unit tests for jsonutil.DeepCloneJSON.
internal/guard/wasm_validate.go Replaces hardcoded integrity strings with config-derived values.
internal/config/guard_policy.go Adds AllIntegrityLevels as a central integrity-level list.
internal/config/guard_policy_validation.go Aliases allowed integrity-levels to AllIntegrityLevels for validation.

Copilot's findings

Tip

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

  • Files reviewed: 13/13 changed files
  • Comments generated: 2

Comment thread internal/guard/wasm_validate.go Outdated
Comment on lines +10 to +16
// AllowedIntegrityLevels is derived from the canonical integrity constants in config.
var AllowedIntegrityLevels = []string{
config.IntegrityNone,
config.IntegrityUnapproved,
config.IntegrityApproved,
config.IntegrityMerged,
}
Comment thread internal/config/guard_policy.go Outdated
Comment on lines +20 to +21
// AllIntegrityLevels is the canonical ordered list of all valid integrity-level values.
var AllIntegrityLevels = []string{IntegrityNone, IntegrityUnapproved, IntegrityApproved, IntegrityMerged}
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Jun 2, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 2, 2026

@copilot address review feedback

Addressed in commit ff68c5b.

  • Replaced exported mutable integrity-level slice with config.AllIntegrityLevels() accessor that returns a defensive copy.
  • Updated guard/config call sites to consume the accessor, removing local redefinition drift risk.
  • Added a test to verify callers cannot mutate the canonical integrity-level list.

@lpcox lpcox merged commit 6723f0d into main Jun 2, 2026
16 checks passed
@lpcox lpcox deleted the copilot/refactor-internal-file-organization branch June 2, 2026 23:31
Copilot stopped work on behalf of lpcox due to an error June 2, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[refactor] Semantic Function Clustering Analysis: Integrity-Level Duplication and Minor File Organization Findings

3 participants