Conversation
…and first_matching_scope - Change extract_mcp_response to return Cow<'_, Value> so the common fallthrough path borrows instead of deep-cloning the entire JSON payload - Change first_matching_scope to return Option<&'a PolicyScopeEntry> eliminating three String clones per call to policy_private_scope_label - Update response_items.rs call sites to use .as_ref() where needed Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/db4efc4f-4113-417d-a7a0-8146b8abe209 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Eliminate redundant Value clone in extract_mcp_response
perf(rust-guard): eliminate redundant clones in May 4, 2026
extract_mcp_response and first_matching_scope
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes the Rust guard’s response-labeling hot path by removing unnecessary JSON and scope-entry cloning, reducing allocation overhead without changing the labeling model used across the guard.
Changes:
- Changed
extract_mcp_responseto returnCow<'_, Value>so already-unwrapped responses are borrowed instead of deep-cloned. - Updated
response_items.rscall sites to work with borrowed-or-owned responses when a concreteValueor&Valueis needed. - Changed
first_matching_scopeto return&PolicyScopeEntryinstead of cloning scope entries on each lookup.
Show a summary per file
| File | Description |
|---|---|
guards/github-guard/rust-guard/src/labels/response_items.rs |
Adjusts legacy item-labeling code to consume Cow<Value> safely at the two sites that require explicit cloning/references. |
guards/github-guard/rust-guard/src/labels/mod.rs |
Converts MCP response extraction to borrow on the common path and allocate only when unwrapping MCP text JSON. |
guards/github-guard/rust-guard/src/labels/helpers.rs |
Removes per-call cloning from scope matching by returning a borrowed scope entry. |
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: 0
This was referenced May 4, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two hot-path allocations in the WASM guard's response labeling pipeline:
extract_mcp_responseunconditionally deep-clones the full JSON payload on the common (already-unwrapped) path, andfirst_matching_scopeclones aPolicyScopeEntry(3×Option<String>) on every call topolicy_private_scope_label.extract_mcp_response→Cow<'_, Value>(labels/mod.rs)Common path now borrows; only the rare MCP-wrapped case allocates:
Call site updates in
response_items.rswhereValue(notCow) is required:actual_response.clone()→actual_response.as_ref().clone()and&actual_response→actual_response.as_ref(). All other call sites auto-deref throughDeref<Target = Value>unchanged.first_matching_scope→Option<&'a PolicyScopeEntry>(labels/helpers.rs)Removes
.cloned()to eliminate 3Stringallocations per lookup.policy_private_scope_label(the only call site) is unaffected —ScopeKindisCopyand field access works identically through a reference.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/tmp/go-build3904760875/b513/launcher.test /tmp/go-build3904760875/b513/launcher.test -test.testlogfile=/tmp/go-build3904760875/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s go_.�� ache/go/1.25.9/x-errorsas etut/NpEHhtvPW_A-ifaceassert x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet(dns block)/tmp/go-build100690393/b509/launcher.test /tmp/go-build100690393/b509/launcher.test -test.testlogfile=/tmp/go-build100690393/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -bool 72e203ec55626255a1c2f6351011f81be538b3bb6463e1a9d5c ker/cli-plugins/docker-compose 72e203ec55626255bash(dns block)invalid-host-that-does-not-exist-12345.com/tmp/go-build3904760875/b495/config.test /tmp/go-build3904760875/b495/config.test -test.testlogfile=/tmp/go-build3904760875/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build3904760875/b396/vet.cfg 1154246/b196/_pkg_.a -trimpath x_amd64/vet -p go.opentelemetry-atomic -lang=go1.25 x_amd64/vet go_.�� .cfg knP4/DXqMOEsmzZ1-ifaceassert x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet(dns block)/tmp/go-build100690393/b491/config.test /tmp/go-build100690393/b491/config.test -test.testlogfile=/tmp/go-build100690393/b491/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 4760875/b504/_pkg_.a stmain.go docker-compose by/32b5d288a6552/usr/lib/open-iscsi/net-interface-handler -ifaceassert -nilfunc docker-compose n-me�� b6b9402ad1484ea3 -buildtags "CURL_CA_BUNDLE=/-id by/181260d18b4dc/usr/bin/networkctl -ifaceassert -nilfunc iginal(dns block)nonexistent.local/tmp/go-build3904760875/b513/launcher.test /tmp/go-build3904760875/b513/launcher.test -test.testlogfile=/tmp/go-build3904760875/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s go_.�� ache/go/1.25.9/x-errorsas etut/NpEHhtvPW_A-ifaceassert x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet(dns block)/tmp/go-build100690393/b509/launcher.test /tmp/go-build100690393/b509/launcher.test -test.testlogfile=/tmp/go-build100690393/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -bool 72e203ec55626255a1c2f6351011f81be538b3bb6463e1a9d5c ker/cli-plugins/docker-compose 72e203ec55626255bash(dns block)slow.example.com/tmp/go-build3904760875/b513/launcher.test /tmp/go-build3904760875/b513/launcher.test -test.testlogfile=/tmp/go-build3904760875/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s go_.�� ache/go/1.25.9/x-errorsas etut/NpEHhtvPW_A-ifaceassert x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet(dns block)/tmp/go-build100690393/b509/launcher.test /tmp/go-build100690393/b509/launcher.test -test.testlogfile=/tmp/go-build100690393/b509/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -bool 72e203ec55626255a1c2f6351011f81be538b3bb6463e1a9d5c ker/cli-plugins/docker-compose 72e203ec55626255bash(dns block)this-host-does-not-exist-12345.com/tmp/go-build3904760875/b522/mcp.test /tmp/go-build3904760875/b522/mcp.test -test.testlogfile=/tmp/go-build3904760875/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o 64/src/net -trimpath x_amd64/vet -p net/http/httptes--version -lang=go1.25 x_amd64/vet .cfg�� /opt/hostedtoolcache/go/1.25.9/xgo1.25.9 1154246/b166/ x_amd64/vet --gdwarf-5 --64(dns block)/tmp/go-build100690393/b518/mcp.test /tmp/go-build100690393/b518/mcp.test -test.testlogfile=/tmp/go-build100690393/b518/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -bool y /usr/local/bin/bash ntime.v2.task/mobash -ifaceassert 2d7/log.json bash /usr�� 6788dd938a942ab2a742ecfee664e61b/run/containerd/io.containerd.runtime.v2.task/moby/1166b6fa98e14docker -tests /usr/sbin/bash 2d7 /tmp/go-build410/usr/bin/runc 2d7/init.pid bash(dns block)If you need me to access, download, or install something from one of these locations, you can either: