Skip to content

refactor: extract shared httputil, remove trivial wrappers in mcp and cmd#2794

Merged
lpcox merged 2 commits intomainfrom
copilot/refactor-semantic-function-clustering
Mar 29, 2026
Merged

refactor: extract shared httputil, remove trivial wrappers in mcp and cmd#2794
lpcox merged 2 commits intomainfrom
copilot/refactor-semantic-function-clustering

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 29, 2026

Three categories of dead indirection identified across internal/: one exact cross-package duplicate, one private wrapper delegating to the public function three lines below it, and one exported wrapper used only within its own package.

Changes

internal/httputil (new package)

  • Extracts WriteJSONResponse — previously copy-pasted byte-for-byte in both server/http_helpers.go and proxy/handler.go
  • server keeps a package-local shim for internal callers; proxy call-sites updated directly
// internal/httputil/httputil.go
func WriteJSONResponse(w http.ResponseWriter, statusCode int, body interface{}) {
    w.Header().Set("Content-Type", "application/json")
    w.WriteHeader(statusCode)
    json.NewEncoder(w).Encode(body)
}

internal/mcp/connection.go

  • Removes getAgentTagsSnapshotFromContext — a one-liner wrapping GetAgentTagsSnapshotFromContext with no added value
  • Single call-site on line 374 updated to call the exported function directly

internal/cmd/flags_difc.go

  • Removes exported ValidateDIFCMode — never called outside cmd, was a one-liner over difc.ParseEnforcementMode
  • Both call-sites (root.go, proxy.go) inlined to difc.ParseEnforcementMode; proxy.go gains the difc import
  • Tests updated to call difc.ParseEnforcementMode directly

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • example.com
    • Triggering command: /tmp/go-build502407326/b336/launcher.test /tmp/go-build502407326/b336/launcher.test -test.testlogfile=/tmp/go-build502407326/b336/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build502407326/b252/vet.cfg 64/src/runtime/cgo main x_amd64/vet (dns block)
  • invalid-host-that-does-not-exist-12345.com
    • Triggering command: /tmp/go-build502407326/b320/config.test /tmp/go-build502407326/b320/config.test -test.testlogfile=/tmp/go-build502407326/b320/testlog.txt -test.paniconexit0 -test.timeout=10m0s 64/s�� go edprintable/reader.go x_amd64/compile (dns block)
  • nonexistent.local
    • Triggering command: /tmp/go-build502407326/b336/launcher.test /tmp/go-build502407326/b336/launcher.test -test.testlogfile=/tmp/go-build502407326/b336/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build502407326/b252/vet.cfg 64/src/runtime/cgo main x_amd64/vet (dns block)
  • slow.example.com
    • Triggering command: /tmp/go-build502407326/b336/launcher.test /tmp/go-build502407326/b336/launcher.test -test.testlogfile=/tmp/go-build502407326/b336/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build502407326/b252/vet.cfg 64/src/runtime/cgo main x_amd64/vet (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build502407326/b345/mcp.test /tmp/go-build502407326/b345/mcp.test -test.testlogfile=/tmp/go-build502407326/b345/testlog.txt -test.paniconexit0 -test.timeout=10m0s --no�� _.a (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

- Create internal/httputil package with shared WriteJSONResponse function
- Update server/http_helpers.go to delegate to httputil.WriteJSONResponse
- Update proxy/handler.go to use httputil.WriteJSONResponse, remove local copy
- Remove getAgentTagsSnapshotFromContext private wrapper from mcp/connection.go
- Update its call-site to call GetAgentTagsSnapshotFromContext directly
- Inline ValidateDIFCMode in cmd/root.go and cmd/proxy.go via difc.ParseEnforcementMode
- Remove ValidateDIFCMode wrapper from cmd/flags_difc.go
- Update tests in flags_difc_test.go to use difc.ParseEnforcementMode directly

Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/7a2a0dee-be9a-4cff-84c6-793489ae1996

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor duplicate function writeJSONResponse in Go source files refactor: extract shared httputil, remove trivial wrappers in mcp and cmd Mar 29, 2026
Copilot AI requested a review from lpcox March 29, 2026 18:08
@lpcox lpcox marked this pull request as ready for review March 29, 2026 18:11
Copilot AI review requested due to automatic review settings March 29, 2026 18:11
@lpcox lpcox merged commit 068a6ef into main Mar 29, 2026
27 checks passed
@lpcox lpcox deleted the copilot/refactor-semantic-function-clustering branch March 29, 2026 18:11
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 internal packages to remove dead indirection and consolidate duplicated HTTP/JSON response logic, reducing maintenance overhead across server/proxy/cmd/mcp.

Changes:

  • Introduces internal/httputil.WriteJSONResponse and updates server/proxy to use it (server keeps a local shim).
  • Removes trivial wrapper getAgentTagsSnapshotFromContext in internal/mcp.
  • Removes unused exported ValidateDIFCMode wrapper in internal/cmd and updates call sites/tests to use difc.ParseEnforcementMode.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/server/http_helpers.go Replaces duplicated JSON response writer implementation with a shim calling internal/httputil.
internal/proxy/handler.go Switches JSON responses to httputil.WriteJSONResponse and removes local helper.
internal/mcp/connection.go Drops redundant private wrapper and calls exported context helper directly.
internal/httputil/httputil.go Adds shared JSON response writer helper for HTTP-facing packages.
internal/cmd/root.go Inlines DIFC mode validation via difc.ParseEnforcementMode.
internal/cmd/proxy.go Inlines DIFC mode validation and adds difc import.
internal/cmd/flags_difc_test.go Updates tests to validate modes via difc.ParseEnforcementMode.
internal/cmd/flags_difc.go Removes unused exported ValidateDIFCMode wrapper.

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

Comment on lines 61 to 66
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := ValidateDIFCMode(tt.mode)
_, err := difc.ParseEnforcementMode(tt.mode)
if tt.wantErr {
assert.Error(t, err, "expected error for mode %q", tt.mode)
} else {
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

This test now validates difc.ParseEnforcementMode directly, but the test function name still references the removed ValidateDIFCMode helper. Renaming the test (and any related descriptions) will keep intent clear and avoid confusion when searching for ValidateDIFCMode usages.

Copilot uses AI. Check for mistakes.
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 — Outliers & Duplicates

3 participants