Skip to content

Codify the shared response-writer wrapper pattern - #10585

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

lpcox merged 2 commits into
mainfrom
copilot/refactor-semantic-function-clustering

Conversation

Copilot AI commented Aug 2, 2026 •

Copy link
Copy Markdown
Contributor

The semantic clustering audit identified internal/httputil/BaseResponseWriter and internal/server/responseWriter as a near-duplicate pair, but the current structure is intentional: server.responseWriter already extends the shared status-capturing wrapper rather than reimplementing it. This change makes that relationship explicit and adds coverage for the contract that matters across wrappers.

  • Pattern clarification

    • Tightens the BaseResponseWriter doc comment to state that it is the shared embed-first wrapper for package-specific response writers.
    • Clarifies that the server wrapper adds body capture/debug behavior on top of shared status capture and Unwrap passthrough.
  • Contract coverage

    • Adds server-level coverage for implicit 200 OK status capture on first Write.
    • Adds coverage that the embedded wrapper still exposes optional http.ResponseWriter interfaces through http.ResponseController, preserving the intended Unwrap behavior for derived wrappers.
  • Example

    type responseWriter struct {
        httputil.BaseResponseWriter
        body bytes.Buffer
    }
    
    rc := http.NewResponseController(w)
    err := rc.Flush() // succeeds when the underlying writer implements http.Flusher

This keeps the existing package organization intact while making the shared-wrapper pattern easier to recognize and less likely to be flagged as accidental duplication in future audits.

Copilot AI changed the title [WIP] Refactor response writer duplication in function clustering analysis Codify the shared response-writer wrapper pattern Aug 2, 2026
Copilot AI requested a review from lpcox August 2, 2026 23:41
@lpcox
lpcox marked this pull request as ready for review August 3, 2026 03:54
Copilot AI review requested due to automatic review settings August 3, 2026 03:54

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

Clarifies the shared response-writer embedding pattern identified in #10584 and strengthens its contract coverage.

Changes:

  • Documents BaseResponseWriter as the shared wrapper base.
  • Verifies implicit 200 OK capture.
  • Verifies optional-interface passthrough through Unwrap.
Show a summary per file
File Description
internal/httputil/response_writer.go Clarifies shared wrapper usage.
internal/server/response_writer.go Documents inherited interface passthrough.
internal/server/response_writer_test.go Adds status and http.Flusher coverage.

Review details

Tip

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

  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Balanced

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🔒 mcpg Read-Only Stress — default AWF

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: default AWF (normal container isolation)

Part Surface Op Result Expected Status
A MCP reads (issues/PRs/file/commits) data returned ALLOWED ✅
B MCP writes (reaction/star/issue/comment/branch/file/PR) Error [-32602]: unknown tool BLOCKED ✅
C CLI reads (github MCP bridge) data returned ALLOWED ✅
D CLI REST writes (reaction/star/issue/comment/file) Permission denied (gh unauthenticated) BLOCKED ✅
E CLI GraphQL mutations (addReaction/addStar/createIssue) Permission denied (gh unauthenticated) BLOCKED ✅

Overall: PASS

Note on Part B: The gateway enforces read-only by removing write tools entirely from the MCP tool list. Write attempts return Error [-32602]: unknown tool — gateway-level enforcement (tool absent from allowed set).

Note on Parts D & E: gh CLI has no token in this runtime, so all REST and GraphQL write attempts fail with Permission denied. Effective boundary: mcpg (read-only MCP) + unauthenticated gh.

References: §30772710499

🔒 mcpg read-only stress (default AWF runtime) by Read-Only Stress: default runtime

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🔒 mcpg Read-Only Stress — gVisor

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: gVisor (runsc) kernel-level isolation

Part Surface Op Result Expected Status
A MCP reads (list_issues/list_prs/get_file/list_commits) data returned ALLOWED ✅
B MCP writes (reaction/star/issue/comment/branch/file/PR) unknown tool (gateway strips write tools) BLOCKED ✅
C CLI reads (list_issues/get_file via github CLI) data returned ALLOWED ✅
D CLI REST writes (reaction/star/issue/comment/file) gh unauthenticated — exit 4 BLOCKED ✅
E CLI GraphQL mutations (addReaction/addStar/createIssue) gh unauthenticated — exit 4 BLOCKED ✅

Overall: PASS

Part B refusals are Error [-32602]: unknown tool "<write_tool>" — gateway enforces read-only by stripping all write tools from the backend tool list. Parts D and E are blocked because gh has no token in this sandbox.

Run: §30772710508

🔒 mcpg read-only stress (gVisor runtime) by Read-Only Stress: gVisor runtime

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🔒 mcpg Read-Only Stress — docker-sbx

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: docker-sbx (KVM-isolated microVM)

Part Surface Op Result Expected Status
A MCP reads (issues/PRs/file/commits) data returned ALLOWED ✅
B MCP writes (reaction/star/issue/comment/branch/file/PR) unknown tool [-32602] BLOCKED ✅
C CLI reads (github CLI) data returned ALLOWED ✅
D CLI REST writes (reaction/star/issue/comment/file) gh not authenticated BLOCKED ✅
E CLI GraphQL mutations (addReaction/addStar/createIssue) gh not authenticated BLOCKED ✅

Overall: PASS

Note on Part B: Write tools absent from gateway registry (MCP -32602: unknown tool). gh CLI unauthenticated, adding enforcement for Parts D/E.

References: §30772710502

🔒 mcpg read-only stress (docker-sbx runtime) by Read-Only Stress: docker-sbx runtime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[refactor] Semantic Function Clustering Analysis — response_writer duplication and package organization

3 participants