Skip to content

Refactor DIFC/guard helper ownership to restore package boundaries#6749

Merged
lpcox merged 3 commits into
mainfrom
copilot/refactor-semantic-function-clustering
May 30, 2026
Merged

Refactor DIFC/guard helper ownership to restore package boundaries#6749
lpcox merged 3 commits into
mainfrom
copilot/refactor-semantic-function-clustering

Conversation

Copilot AI commented May 30, 2026

Copy link
Copy Markdown
Contributor

Static analysis flagged domain logic living in internal/cmd and a guard-policy utility living in internal/guard, creating avoidable layering leaks. This PR rehomes those functions into their domain packages and removes thin CLI indirection.

  • DIFC defaults and parsing moved out of CLI

    • Added difc.DefaultEnforcementMode() and difc.ParseSinkServerIDs(...).
    • Updated CLI wiring to call these directly.
    • Removed cmd-local helpers (getDefaultDIFCMode, parseDIFCSinkServerIDs, and wrapper-only mode validator).
  • Guard policy map marshaling moved to config

    • Moved policy struct/map conversion into config.GuardPolicyToMap(...).
    • Updated guard WASM payload code to use config-owned conversion.
    • Removed the old guard.PolicyToMap helper.
  • Logger utility placement corrected

    • Moved ExtractErrorMessage out of rpc_helpers.go into log_cleanup.go to align file semantics with function behavior.
    • Kept API/behavior unchanged.
  • Dependency direction cleanup

    • Introduced config-local integrity-level list for guard-policy validation/error text to avoid pulling internal/guard into internal/config.
cmd.Flags().StringVar(
    &difcMode,
    "guards-mode",
    difc.DefaultEnforcementMode(),
    "Guards enforcement mode: strict, filter, or propagate",
)

resolvedSinkServerIDs, err := difc.ParseSinkServerIDs(difcSinkServerIDs)

Copilot AI changed the title [WIP] Refactor CLI layer to improve domain logic separation Refactor DIFC/guard helper ownership to restore package boundaries May 30, 2026
Copilot finished work on behalf of lpcox May 30, 2026 13:52
Copilot AI requested a review from lpcox May 30, 2026 13:52
@lpcox lpcox marked this pull request as ready for review May 30, 2026 14:00
Copilot AI review requested due to automatic review settings May 30, 2026 14:00

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

Refactors layering by relocating helpers to their proper domain packages: DIFC default-mode and sink-server-ID parsing move from internal/cmd into internal/difc; the policy→map conversion moves from internal/guard into internal/config (along with a config-local integrity-levels list); and ExtractErrorMessage (plus its log-cleanup regexes) moves out of rpc_helpers.go into a new log_cleanup.go within internal/logger. Tests are migrated alongside the code, and CLI sites are updated to call the new domain APIs.

Changes:

  • Move DIFC defaults/parsing into internal/difc and update CLI wiring; remove cmd-local wrappers.
  • Move policy marshaling into internal/config as GuardPolicyToMap and add a config-local allowedGuardPolicyIntegrityLevels to break the config→guard dependency.
  • Move ExtractErrorMessage and its precompiled regexes into internal/logger/log_cleanup.go.
Show a summary per file
File Description
internal/logger/rpc_helpers.go Removes ExtractErrorMessage, regexes, and unused imports.
internal/logger/log_cleanup.go New home for ExtractErrorMessage and log-cleanup regexes.
internal/guard/wasm_payload.go Switches to config.GuardPolicyToMap.
internal/difc/sink_server_ids.go Adds ParseSinkServerIDs with validation/dedup.
internal/difc/sink_server_ids_test.go Tests for ParseSinkServerIDs.
internal/difc/evaluator.go Adds DefaultEnforcementMode reading MCP_GATEWAY_GUARDS_MODE.
internal/difc/evaluator_test.go Tests for DefaultEnforcementMode.
internal/config/guard_policy_validation.go Uses local integrity-levels list instead of guard.AllowedIntegrityLevels.
internal/config/guard_policy_parse.go Same: drops internal/guard import, uses local list.
internal/config/guard_policy_parse_test.go Updates expectation to local list.
internal/config/guard_policy_marshal.go Moves PolicyToMap here as GuardPolicyToMap.
internal/config/guard_policy_marshal_test.go Test moved into config package.
internal/config/guard_policy_integrity_levels.go New local integrity-levels list.
internal/cmd/root.go Calls difc.ParseEnforcementMode/ParseSinkServerIDs directly.
internal/cmd/proxy.go Same direct call to difc.ParseEnforcementMode.
internal/cmd/flags.go Updates exception comment to reference difc.DefaultEnforcementMode().
internal/cmd/flags_difc.go Removes cmd-local helpers; uses difc.DefaultEnforcementMode().
internal/cmd/flags_difc_test.go Drops removed-helper tests; redirects sink-id test to difc.ParseSinkServerIDs.

Copilot's findings

Tip

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

  • Files reviewed: 18/18 changed files
  • Comments generated: 0

@lpcox lpcox merged commit 4f83afe into main May 30, 2026
29 checks passed
@lpcox lpcox deleted the copilot/refactor-semantic-function-clustering branch May 30, 2026 14:03
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: Domain Logic Leaked into CLI Layer + Misplaced Utility

3 participants