Skip to content

refactor: Move guard-policy parsing, AllowOnly factory, and DIFC sink state to proper packages#1987

Merged
lpcox merged 4 commits intomainfrom
copilot/refactor-semantic-function-clustering-again
Mar 16, 2026
Merged

refactor: Move guard-policy parsing, AllowOnly factory, and DIFC sink state to proper packages#1987
lpcox merged 4 commits intomainfrom
copilot/refactor-semantic-function-clustering-again

Conversation

Copy link
Contributor

Copilot AI commented Mar 15, 2026

Summary

Addresses the three most impactful semantic function clustering refactoring opportunities identified in the issue.

Changes

1. Guard-policy parsing moved to config package

  • parseServerGuardPolicy and parsePolicyMap moved from internal/server/unified.gointernal/config/guard_policy.go, exported as ParseServerGuardPolicy / ParsePolicyMap
  • These are domain parsing functions that understand the GuardPolicy structure and should live alongside the type definitions, not in the transport/server layer
  • internal/server/unified.go now delegates to config.ParseServerGuardPolicy

2. BuildAllowOnlyPolicy factory moved to config package

  • buildAllowOnlyPolicy moved from internal/cmd/flags_difc.gointernal/config/guard_policy.go, exported as BuildAllowOnlyPolicy
  • Policy construction is domain logic and belongs next to the policy types, not in the CLI layer
  • internal/cmd/root.go now delegates to config.BuildAllowOnlyPolicy

3. DIFC sink server ID state moved to difc package

  • difcSinkServerIDs global state, SetDIFCSinkServerIDs, and isDIFCSinkServerID removed from internal/mcp/connection.go
  • New file internal/difc/sink_server_ids.go exports SetSinkServerIDs and IsSinkServerID
  • The transport layer (mcp) should not own security policy state — it now imports difc and delegates to difc.IsSinkServerID
  • internal/cmd/root.go now calls difc.SetSinkServerIDs directly

Testing

All existing tests pass (make agent-finished). Call sites updated throughout.

Closes #[issue-number]

Copilot AI and others added 2 commits March 15, 2026 23:30
… state to proper packages

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI linked an issue Mar 15, 2026 that may be closed by this pull request
5 tasks
Copilot AI requested a review from lpcox March 15, 2026 23:43
@lpcox lpcox marked this pull request as ready for review March 16, 2026 00:26
Copilot AI review requested due to automatic review settings March 16, 2026 00:26
Copy link
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 DIFC/guard-policy domain logic out of transport/CLI layers into the config and difc packages, reducing semantic clustering in server and mcp while keeping behavior/call-sites consistent.

Changes:

  • Moved server guard-policy parsing helpers into internal/config (ParseServerGuardPolicy, ParsePolicyMap) and updated UnifiedServer + tests to call them.
  • Moved AllowOnly policy factory into internal/config (BuildAllowOnlyPolicy) and updated CLI override resolution + tests accordingly.
  • Moved DIFC sink server ID global state into internal/difc (SetSinkServerIDs, IsSinkServerID), updated mcp logging enrichment and CLI wiring.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/server/unified.go Delegates server guard-policy parsing to config.ParseServerGuardPolicy; removes local parsing helpers.
internal/server/guard_policy_parsing_test.go Updates tests to call config.ParseServerGuardPolicy.
internal/config/guard_policy.go Adds exported parsing helpers and BuildAllowOnlyPolicy factory.
internal/cmd/root.go Wires sink server IDs via difc.SetSinkServerIDs; builds AllowOnly policy via config.BuildAllowOnlyPolicy.
internal/cmd/flags_difc.go Removes CLI-local buildAllowOnlyPolicy helper.
internal/cmd/flags_difc_test.go Updates tests to call config.BuildAllowOnlyPolicy.
internal/mcp/connection.go Delegates sink-server check to difc.IsSinkServerID for tag snapshot enrichment.
internal/mcp/connection_test.go Updates sink-server ID tests to use difc API.
internal/mcp/sdk_method_dispatch_test.go Updates tests to configure sink-server IDs via difc.SetSinkServerIDs.
internal/difc/sink_server_ids.go Introduces concurrency-safe global sink server ID set and lookup helpers.

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

You can also share your feedback on Copilot code review. Take the survey.

lpcox and others added 2 commits March 15, 2026 17:29
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox lpcox merged commit 6af2b5b into main Mar 16, 2026
13 checks passed
@lpcox lpcox deleted the copilot/refactor-semantic-function-clustering-again branch March 16, 2026 00:34
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: Outliers and Refactoring Opportunities

3 participants