Repository navigation
test(mcp): two loopback checks no longer fail behind an HTTP proxy - #6971
Conversation
configured_local_same_origin_redirect_and_connection_reuse_work and
event_stream_shares_the_client_and_other_bodies_keep_the_read_deadline
set NO_PROXY=* so an ambient HTTP(S)_PROXY cannot take their 127.0.0.1
fixture. The proxy matcher configured_mcp_proxy uses (hyper-util's, the
one reqwest uses too) checks `*` against host names only: for an IP
literal it looks at the address entries alone. With HTTP_PROXY exported
both requests are proxied, and since the guard replaces the developer's
own NO_PROXY, listing 127.0.0.1 there does not help.
Both tests assert what only the direct path gives: the redirect arrives
on the same socket, and the event stream and its POST share one cached
client (a proxied client is never cached). So they fail on any machine
that exports HTTP_PROXY, with
assertion failed: String::from_utf8_lossy(&buf[..size]).starts_with("GET /mcp/v2 ")
and
assertion `left == right` failed
left: 0
right: 1
The guard now names the address the fixtures bind.
Signed-off-by: Lstarsky0 <59827030+Lstarsky0@users.noreply.github.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
github-actions
Bot
added
the
contribution-gate
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Thank you @Lstarsky0 for isolating the exact loopback/proxy interaction and reporting the full 15,244 passed / 42 failed result against main’s 15,242 passed / 44 failed, rather than calling that a clean suite. I checked the two changed fixtures and the matcher they use: an IP-literal request follows the IP matcher, so naming The current diff is the two existing fixture values only; it leaves production proxy behavior unchanged. Current CI is still running, and I have not yet run this head locally. I will keep the review and eventual landing on your PR. |
|
Thank you, @Lstarsky0, for tracing the two loopback MCP fixtures to the proxy matcher and fixing their NO_PROXY values to the literal 127.0.0.1 host. Your PR is merged as your own work in 39f0733, for 0.10.2. Both affected tests passed in hosted CI on macOS, Ubuntu and Windows; the complete jobs reported 19,033 passed / 0 failed / 33 skipped, 19,030 / 0 / 34, and 18,297 / 0 / 27 respectively. This fixes the fixture routing without changing production proxy behavior. The release candidate is still undergoing final CI and package QA; 0.10.2 has not shipped yet. |
Document the merged contributor sub-agent state fix with its actual existing-target junction scope. Credit SparkofSpike for #6929 and #6949, and Lstarsky0 for #6971 in the 0.10.2 contributor list. Regenerate the embedded changelog from the same source. Verification on macOS, source410d8310ca plus these two changelog paths: - npm test: Node540 passed;0 failed;7 skipped; Vitest772 passed. Combined1312 passed;0 failed;7 skipped. - npm run check:web: PASS. - sync-changelog.sh --check and git diff --check: PASS. #6949 final PR head hosted CI: Windows18301 passed/27 skipped, macOS19035 passed/33 skipped, Ubuntu19032 passed/34 skipped. Both Windows junction cases and owner_crash_reentry passed. Final candidate CI, dangling-target, native/package and oldest-Windows acceptance are separate remaining gates. No tests were added in this documentation slice. Refs #6949 Refs #6971
#6971 fixed two tests whose NO_PROXY=* guard did not cover their 127.0.0.1 fixture: the proxy matcher applies `*` to host names only and checks an IP literal against the address entries alone. The same guard is in other tests. Behind a local proxy that forwards to loopback they still pass, which is why only two showed up there. Behind one that does not (HTTP_PROXY and HTTPS_PROXY pointing at a closed port, NO_PROXY=localhost,127.0.0.1,::1 in the environment) twenty tests fail on main that pass without the proxy variables. Each replaces a NO_PROXY that already exempted its fixture with one that does not. Fifteen of them now set `*,127.0.0.1`: the wildcard stays for host names and the fixture's address is added. The other five are in mcp/oauth.rs, which this change leaves alone. Two guards in mcp/http_client.rs also stay as they are; their tests pass behind the same proxy. Signed-off-by: Lstarsky0 <59827030+Lstarsky0@users.noreply.github.com>
2 participants
What and why
Two MCP HTTP client tests fail on any machine that exports
HTTP_PROXY/HTTPS_PROXY, which is the normal setup behind a local proxy:mcp::http_client::tests::configured_local_same_origin_redirect_and_connection_reuse_workmcp::http_client::tests::event_stream_shares_the_client_and_other_bodies_keep_the_read_deadlineBoth set
NO_PROXY=*to keep an ambient proxy away from their127.0.0.1fixture. The matcherconfigured_mcp_proxyuses (hyper-util's, the one reqwest uses too) applies*to host names only; for an IP literal it looks at the address entries and nothing else. So the fixture request is proxied anyway. The guard also replaces the developer's ownNO_PROXY, so having127.0.0.1in it doesn't help.Both tests assert something only the direct path gives: the redirect arrives on the same socket, and the event stream and its POST share one cached client (a proxied client is never cached). They fail with
and
In these two tests the guard now names the address the fixtures bind.
The same
NO_PROXY=*guard is in 22 other tests. I left those alone. They pass on my machine because the local proxy forwards to loopback; with the proxy variables pointing at a closed port, six more in this module fail on main. These two are the only ones that fail in an ordinary proxy setup.Issue
No-Issue: fails locally behind a proxy, no issue filed for it
How I tested it
HTTP_PROXYandHTTPS_PROXYset to a running local proxy,NO_PROXY=localhost,127.0.0.1,::1,.local): both fail as above. With the proxy variables unset: both pass.NO_PROXY, the proxy variables pointing at a closed port, only the lowercase variables set, and no proxy variables at all.cargo fmt --all -- --check,cargo clippy -p codewhale-tui --all-targets --all-featureswith CI's flags, andcargo check -p codewhale-tui --all-targets: clean. Clippy also on 1.99, pluscargo +1.89.0 check -p codewhale-tui.cargo nextest run -p codewhale-tui --lib: 15244 passed, 42 failed. Main (008d21d) on the same machine: 15242 passed, 44 failed. The two that moved are these two; the other 42 are the same on both, and most need an extension-host Node version I don't have.No new tests; this changes one value in two existing ones.
Checklist
maincargo fmt --allpasses, and tests cover new or changed behaviordocs/features.toml(cargo test -p codewhale-tui --test feature_registrychecks it) — n/a