Skip to content

Deduplicate canonical guard-policy missing-key error message#7046

Merged
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-fix-error-string
Jun 5, 2026
Merged

Deduplicate canonical guard-policy missing-key error message#7046
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-fix-error-string

Conversation

Copilot AI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

The guard-policy validation path duplicated the same missing-key error string in three places (UnmarshalJSON, ValidateGuardPolicy, NormalizeGuardPolicy). This centralizes that message into one canonical constant to prevent drift across validation and parsing paths.

  • Error message canonicalization

    • Added errMsgPolicyMissingKey in internal/config/guard_policy_validation.go as the single source of truth for:
      • "policy must include allow-only or write-sink"
  • Callsite updates

    • Replaced duplicated literals at all three sites:
      • internal/config/guard_policy.go (GuardPolicy.UnmarshalJSON)
      • internal/config/guard_policy_validation.go (ValidateGuardPolicy, NormalizeGuardPolicy)
    • Switched these returns to errors.New(errMsgPolicyMissingKey) for direct constant-backed errors.
  • Test alignment

    • Updated targeted assertions in internal/config/guard_policy_test.go to reference the canonical constant for the missing-key path, reducing brittle string duplication in tests.
const errMsgPolicyMissingKey = "policy must include allow-only or write-sink"

if policy == nil || (policy.AllowOnly == nil && policy.WriteSink == nil) {
    return nil, errors.New(errMsgPolicyMissingKey)
}

Copilot AI changed the title [WIP] Refactor hardcoded error string into a constant Deduplicate canonical guard-policy missing-key error message Jun 5, 2026
Copilot finished work on behalf of lpcox June 5, 2026 14:07
Copilot AI requested a review from lpcox June 5, 2026 14:07
@lpcox lpcox marked this pull request as ready for review June 5, 2026 14:39
Copilot AI review requested due to automatic review settings June 5, 2026 14:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR centralizes the guard-policy “missing key” error message into a single constant so the parsing (UnmarshalJSON) and validation/normalization paths can’t drift on wording.

Changes:

  • Added errMsgPolicyMissingKey constant in internal/config/guard_policy_validation.go.
  • Replaced duplicated missing-key error literals in GuardPolicy.UnmarshalJSON, ValidateGuardPolicy, and NormalizeGuardPolicy with errors.New(errMsgPolicyMissingKey).
  • Updated select tests to reference the canonical constant instead of duplicating the string.
Show a summary per file
File Description
internal/config/guard_policy.go Switches the missing-key JSON unmarshal error to use the canonical constant.
internal/config/guard_policy_validation.go Introduces the canonical error message constant and updates validation/normalization callsites to use it.
internal/config/guard_policy_test.go Aligns some test assertions with the canonical missing-key constant.

Copilot's findings

Tip

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

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

Comment thread internal/config/guard_policy_test.go Outdated
@@ -626,7 +626,7 @@ func TestGuardPolicyUnmarshalJSON(t *testing.T) {
{
name: "missing allow-only field",
"strings"
)

const errMsgPolicyMissingKey = "policy must include allow-only or write-sink"
@lpcox

lpcox commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

@copilot address review feedback

Copilot AI commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

@copilot address review feedback

Addressed in a786d1a.

  • Updated remaining TestNormalizeGuardPolicy missing-key assertions to use errMsgPolicyMissingKey.
  • Renamed the {} unmarshal test case to missing allow-only and write-sink fields for clarity.
  • Re-ran targeted guard-policy tests and full make agent-finished validation.

Copilot finished work on behalf of lpcox June 5, 2026 15:29
@lpcox lpcox merged commit ab0afc7 into main Jun 5, 2026
16 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-fix-error-string branch June 5, 2026 15:37
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.

[duplicate-code] Duplicate Code Pattern: Hardcoded error string 'policy must include allow-only or write-sink' repeated 3 times

3 participants