test(mcp): add OAuth smoke test and dummy-server e2e coverage - #447
Merged
Conversation
The existing HTTP MCP proxy OAuth client (mcp-http-stdio-proxy.ts) had no coverage beyond one browser-launcher unit test. Add: - scripts/smoke-mcp-oauth.ts: an interactive smoke test that connects to a real OAuth-protected remote MCP server via `allagents mcp proxy`, lists its tools, and asks a question -- for manually confirming the OAuth flow against a live private server. - tests/helpers/dummy-mcp-oauth-server.ts: a local, CI-safe stand-in OAuth IdP + MCP server (PKCE, dynamic client registration, auto-approving authorization) so the OAuth flow can be exercised without a human or network access to a real server. - tests/e2e/mcp-proxy-oauth.test.ts: covers first-connection OAuth, cached-token reuse on a second connection, and automatic token refresh on an expired access token -- all with zero browser/human interaction required. - tests/helpers/mcp-proxy-client.ts: shared helper connecting an SDK Client to the proxy over stdio, used by both the smoke script and the e2e tests so both exercise the same production code path. mcp-http-stdio-proxy.ts: export hashServerUrl and AUTH_URL_LOG_PREFIX for test reuse, and add an ALLAGENTS_MCP_OAUTH_NO_BROWSER escape hatch so e2e tests never spawn a real OS browser.
cmd-ts's compiled CJS output does require("chalk") internally, but
chalk v5 is pure ESM. Under Bun this intermittently throws
"TypeError: require() async module ... is unsupported" -- observed on
~50% of dev-path CLI invocations (bun run src/cli/index.ts), including
in already-merged e2e tests, though not in the built dist binary real
users run.
Force chalk to v4.1.2 everywhere via a package.json `overrides` entry,
since cmd-ts declares its own "chalk": "^5.4.1" dependency independent
of this project's own version pin -- changing only this project's
direct dependency would leave a nested v5 copy for cmd-ts to crash on.
Confirmed 0 crashes across 10 repeated CLI invocations after the fix
(previously ~5/10), and the full test suite (including e2e) passes
consistently across repeated runs.
The script defaulted to a private company MCP endpoint. The whole point of this tooling is to not depend on any internal/company URL — that's what the dummy server + e2e suite are for. Require --url-style positional input instead, with a clear usage message.
Deploying allagents with
|
| Latest commit: |
7b801ab
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://647e6408.allagents.pages.dev |
| Branch Preview URL: | https://feat-mcp-remote.allagents.pages.dev |
Requiring an explicit URL was correct in spirit (don't hardcode a private endpoint) but missed the actual point: the smoke test should work entirely on localhost by default, with zero external dependency and zero setup. - scripts/smoke-mcp-oauth.ts: with no URL argument, spins up tests/helpers/dummy-mcp-oauth-server.ts itself and runs the full connect -> OAuth -> list-tools -> call-tool flow against it. OAuth completes via a plain fetch() to the dummy IdP's auto-approving /authorize endpoint -- the same "simulate the browser with curl" trick the e2e tests already use, no real login screen involved. Passing an explicit URL still runs the original real-OAuth path (opens a real browser) unchanged, for validating against an actual private server when you have one. - scripts/dev-mcp-server.ts: new standalone script that starts the same dummy server and keeps it running (Ctrl+C to stop), so it can be used for general local development against `allagents mcp` -- not just this smoke test.
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.
Summary
The existing HTTP MCP proxy OAuth client (
src/core/mcp-http-stdio-proxy.ts, exposed viaallagents mcp proxy <url>) had no test coverage beyond one Windows-browser-launcher unit test. This PR adds:allagents mcp, reusing the same dummy server.User-facing impact
None for the published CLI.
scripts/is dev-only tooling (not in package.json'sfiles, never shipped in the npm package), and the only change to shipped code (src/core/mcp-http-stdio-proxy.ts) is:ALLAGENTS_MCP_OAUTH_NO_BROWSERenv var (test-only escape hatch — skips the real OS browser-open; the authorization URL is still always printed either way, so nothing is hidden from a real user even if this were somehow set).hashServerUrlandAUTH_URL_LOG_PREFIXare now exported (for test reuse only, no behavior change).Default
allagents mcp proxybehavior for real users is unchanged.What was delivered
scripts/smoke-mcp-oauth.ts— with no arguments, spins up the local dummy MCP+OAuth server itself and runs the fullconnect -> OAuth -> list tools -> call toolflow against it end-to-end, printing the response. OAuth completes via a plainfetch()to the dummy IdP's auto-approving/authorizeendpoint (the same "simulate the browser with curl" trick the e2e tests use) — no login screen, no external network, no setup required. Pass a URL explicitly (bun run smoke:mcp-oauth <url> [question]) to instead run the original real-OAuth path against any OAuth-protected server you have access to (opens a real browser for you to complete login).scripts/dev-mcp-server.ts— standalone script that starts the same dummy MCP+OAuth server and keeps it running (Ctrl+C to stop), so it can be used for local development againstallagents mcpgenerally, not just this smoke test.tests/helpers/dummy-mcp-oauth-server.ts— a local stand-in OAuth IdP (PKCE, dynamic client registration, auto-approving/authorize) plus a protected MCP endpoint (RFC 9728 metadata, bearer-token enforcement, multi-session support) — entirely on127.0.0.1, no external dependency.tests/e2e/mcp-proxy-oauth.test.ts— 3 scenarios against the dummy server:/authorizecalls./authorizecalls.tests/helpers/mcp-proxy-client.ts— shared stdio-client helper used by the smoke script, dev server example, and the e2e tests, so all of them exercise the identical production code path.chalkv5→v4 (+package.jsonoverrides) to resolve a pre-existing, intermittentTypeError: require() async module ... unsupportedcrash — a Bun/cmd-tsCJS↔ESM interop issue (cmd-tsinternallyrequire()s chalk, but chalk v5 is ESM-only). This was hitting ~50% of dev-path CLI invocations (bun run src/cli/index.ts, the same path both existing and new e2e tests spawn), including the already-mergedmcp-add-proxy.test.ts. Confirmed 0/10 crashes after the fix vs ~5/10 before; does not affect the builtdist/index.jsbinary real users run (was already unaffected).Manual test steps (reproduction)
Expected: all pass, no browser windows open during steps 1/2/3/4, no
require() async modulecrashes anywhere.Review
Went through a 9-persona automated code review (correctness, security, reliability, adversarial, testing, maintainability, project-standards, agent-native, learnings) on the initial implementation. No P0/P1 findings. 6 fixes were applied directly (truncated-URL parsing on stderr chunk boundaries, shared log-prefix constant instead of duplication, explicit logging when the no-browser test escape hatch is active, child-process cleanup on connect failure, clean error response instead of an unhandled rejection in the dummy server, error handling on a fire-and-forget test fetch). Remaining open items (all P2/P3, none blocking):
tests/helpers/dummy-mcp-oauth-server.ts:stop()doesn't close sessions left mid-handshake (test-only cleanup edge case).scripts/smoke-mcp-oauth.ts's argument-building/tool-selection heuristics.invalid_request/ PKCE-failure error branches aren't exercised by any test.connect()fails (would help debug a hung/crashed proxy in test failures).Follow-up recommendation (not in this PR)
Compared this implementation against
geelen/mcp-remote's approach to reducing repeated OAuth flows. We already avoid its two biggest footguns (version-scoped credential directory that wipes tokens on every upgrade;expires_in-only token bookkeeping with no proactive refresh). The one gap that concretely matters forallagentsspecifically: no cross-process coordination. Sinceallagentssyncs one MCP server entry into Claude Code, Copilot/VS Code, and Cursor, a cold start aftersync/installspawns separatemcp proxyprocesses per client — each would trigger its own browser popup for the same server on first use.mcp-remote's lockfile + long-poll rendezvous pattern is the direct fix, but is a distinct, separately-scoped change from this PR.