Skip to content

Restructure tests + real-KV/MSW e2e (drop self-mocks) - #155

Merged
mattzcarey merged 14 commits into
mainfrom
add-e2e-tool-call-test
Jun 9, 2026
Merged

Restructure tests + real-KV/MSW e2e (drop self-mocks)#155
mattzcarey merged 14 commits into
mainfrom
add-e2e-tool-call-test

Conversation

@mattzcarey

@mattzcarey mattzcarey commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What

Three things, building on each other:

  1. Moves the test suite src/tests/ → top-level tests/ (mirroring src/), via git mv.
  2. Restructures tests around a clear seam model + shared helpers, and adds a worker-level test.
  3. Removes self-mocking in the auth tests — uses the real OAUTH_KV binding and mocks only true external boundaries (the Cloudflare API) with MSW.

Structure (no e2e/ tier)

Every test already runs in the real workerd pool, so there's no "less production-like" tier — the old e2e/ folder was capturing a seam difference, not a runtime one. New layout:

tests/
├── setup/msw.ts              # global MSW server + listen/reset/close (setupFile)
├── helpers/
│   ├── mcp.ts                # callTool(), mcpToolCallRequest(), parseMcpResult()
│   ├── cloudflare-api.ts     # mockIdentityProbe(), cfSuccess/cfError envelopes
│   └── kv.ts                 # clearKv() — per-test isolation for shared bindings
├── auth/*.test.ts
├── worker.test.ts            # worker-seam test (was e2e/tool-call.test.ts)
└── *.test.ts                 # module-seam tests, mirroring src/

helpers/ and setup/ aren't matched by the *.test.ts glob, so they're importable infra, not suites.

The worker-seam test (tests/worker.test.ts)

Drives the real worker via exports.default.fetch() (Cloudflare recipe). A JSON-RPC tools/call for execute runs real code in a Worker Loader isolate, forwarded through the real GlobalOutbound. Only outbound fetch() is mocked (MSW). Verified with a negative control (mutating the MSW response fails the assertion).

Stop mocking our own systems → real KV + MSW

This was the main ask. Before, the auth tests mocked our own functions and faked KV; now they exercise the real code paths and only mock the Cloudflare API boundary:

  • api-token-mode.test.ts: dropped vi.mock(getUserAndAccounts) and the env.OAUTH_KV.get/put spies. The real getUserAndAccounts now runs against MSW /user+/accounts, and assertions read the real OAUTH_KV to prove cache write/hit.
  • oauth-handler.test.ts: deleted the mockKV() Map-fake — all ~10 guardRefreshTokenExchange tests use the real env.OAUTH_KV (spying only where a test needs fault injection or call counts). Dropped vi.mock(refreshAuthToken): the handleTokenExchangeCallback tests run the real function against an MSW-mocked /oauth2/token, including a real in-flight-marker collision (429) and a malformed-body ZodError case.
  • cloudflare-auth.test.ts: vi.stubGlobal('fetch') → MSW.

The only self-mock left is fetch-retry's maxRetries: 0 — timing, not behavior (keeps identity-probe retry tests fast).

Isolation note

vitest-pool-workers isolates storage per test file, not per test. Tests sharing real bindings (OAUTH_KV) clear state in afterEach via clearKv().

Stack upgrade (required for MSW)

msw/node only loads under workerd via the cloudflareTest() Vite plugin → vitest 4 + pool 0.16. So vitest 3.2→4.1, @cloudflare/vitest-pool-workers 0.12→0.16, config migrated to the plugin form, msw added.

Testing

npm run check fully green — format, lint, typecheck, and 204 tests across 13 files pass.
mattzcarey added 2 commits June 9, 2026 12:06
@mattzcarey
Move tests to top-level tests/ and add e2e tool-call test
c0f5b19 
Relocate the suite from src/tests/ to a top-level tests/ directory
(mirroring src/) via git mv to preserve history, and update relative
imports, the test tsconfig, the cloudflare-test.d.ts reference path,
vitest include glob, and the root tsconfig exclude.

Add tests/e2e/tool-call.test.ts: a true end-to-end test following the
current Cloudflare vitest recipes — invokes the real worker through
exports.default.fetch() (from cloudflare:workers) with a full JSON-RPC
tools/call for the execute tool. The code runs in a real Worker Loader
isolate and its cloudflare.request() is forwarded through the real
GlobalOutbound proxy. The ONLY mock is outbound fetch() via
vi.spyOn(globalThis,'fetch') (the test isolate shares the worker isolate,
so the spy catches both the auth-guard /user+/accounts probes and the
GlobalOutbound-forwarded API call). Auth, MCP transport, tool dispatch
and Worker Loader are all the real code path.

Verified with a negative control (mutating the mock fails the assertion)
so it is not a false pass.
@mattzcarey
Use MSW for e2e outbound mocking; upgrade to vitest 4 + pool 0.16
ba793f0 
Switch the e2e outbound-fetch mock from vi.spyOn(globalThis,'fetch') to
MSW (declarative recipe). msw/node only loads under workerd with the new
cloudflareTest() Vite plugin, which requires vitest 4 and
@cloudflare/vitest-pool-workers 0.16 — so this also migrates the vitest
config to the plugin form and bumps both deps.

- add msw devDependency; tests/e2e/msw-server.ts + msw-setup.ts
  (setupServer + listen/reset/close, onUnhandledRequest:'error')
- tool-call.test.ts now registers handlers via server.use(http.get(...))
- vitest.config.ts: defineWorkersConfig -> defineConfig + cloudflareTest()
  plugin; setupFiles wires MSW lifecycle
- vitest 3.2 -> 4.1, pool 0.12 -> 0.16

Fix one isolation regression surfaced by the upgrade: pool 0.16 isolates
storage per test FILE (0.12 was per test), so refresh-guard tests reusing
the same token hit a cached failure in the real OAUTH_KV. Clear OAUTH_KV
in afterEach to restore per-test isolation.

All 204 tests pass; negative control (mutating the MSW response) fails
the e2e assertion, confirming it's not a false pass.
@mattzcarey mattzcarey changed the title Move tests to top-level tests/ + add e2e tool-call test Jun 9, 2026
@mattzcarey
Restructure tests; replace self-mocks with real KV + MSW boundaries
939667c 
Structure (no e2e tier — every test is production-like in the workerd pool):
- tests/setup/msw.ts: global MSW server + lifecycle (was tests/e2e/msw-*)
- tests/helpers/{mcp,cloudflare-api,kv}.ts: callTool()/parseMcpResult(),
  MSW identity-probe + CF envelope helpers, clearKv()
- tests/e2e/tool-call.test.ts -> tests/worker.test.ts (worker-seam test,
  mirrors src/index.ts), rewritten on the helpers

Stop mocking our own systems; use real bindings + mock only true boundaries:
- api-token-mode.test.ts: drop vi.mock(getUserAndAccounts) and KV get/put
  spies. Real getUserAndAccounts runs against MSW /user+/accounts; assert on
  real OAUTH_KV contents (cache hit/miss) instead of spy calls.
- oauth-handler.test.ts: delete the mockKV() Map-fake; all
  guardRefreshTokenExchange tests use the real env.OAUTH_KV (spying only for
  fault injection / call counts). Drop vi.mock(refreshAuthToken): the
  handleTokenExchangeCallback tests now run the real function against an
  MSW-mocked /oauth2/token (incl. a real in-flight-marker collision and a
  malformed-body ZodError case).
- cloudflare-auth.test.ts: vi.stubGlobal('fetch') -> MSW.

Only remaining self-mock is fetch-retry's maxRetries:0 (timing, not behavior).
All 204 tests pass; full check green.
@mattzcarey mattzcarey changed the title Move tests to top-level tests/ + add e2e tool-call test (MSW, vitest 4) Jun 9, 2026
mattzcarey added 10 commits June 9, 2026 12:41
@mattzcarey
Remove stale src/tests/ duplicates left by the move
3ac8d92 
The earlier git mv added tests/ but the src/tests/ deletions were never
staged (git add was scoped to tests/), so HEAD carried both copies. CI
typechecks src/**/* and the stale src/tests/* (old mockKV/fetch-retry
code) failed. Stage the deletions so only the moved tests/ remain.
@mattzcarey
test: cover OAuth protocol primitives in cloudflare-auth
c50d5d8 
Add unit tests for the previously-untested OAuth2 wire helpers, all
running the REAL functions against MSW-mocked upstream responses:

- generatePKCECodes: verifier is base64url, challenge == base64url(
  SHA-256(verifier)) recomputed, and each call is fresh
- getAuthorizationURL: /oauth2/auth URL with S256, encoded state, joined
  scopes
- getAuthToken: the authorization_code exchange (happy path asserts
  grant_type/code_verifier/Basic auth), plus 400->invalid_grant,
  401->invalid_client, 5xx->502, and a malformed-body non-OAuth throw

cloudflare-auth.test.ts: 2 -> 10 tests.
@mattzcarey
test: cover the OAuth route layer end-to-end
bab3406 
Add tests/auth/oauth-routes.test.ts driving createAuthHandlers' routes
through exports.default.fetch() (the real OAuthProvider-wrapped worker),
covering paths that previously had zero coverage:

- GET /authorize: renders the consent dialog (form + CSRF + session
  cookie) for a registered client; unknown client -> 500 + auth_user error
- GET /oauth/callback: missing code -> 400 invalid_request; missing state
  -> auth_user error; unknown/expired state token -> 400 + auth_user error

Real auth, real OAUTH_KV and OAuthProvider (client registered via the RFC
7591 /register endpoint); only MCP_METRICS is spied, to assert the
auth_user datapoints the error paths emit. The callback happy path is
left out: completeAuthorization needs provider-internal grant state from a
real /authorize round-trip, which isn't worth faking.

Total: 204 -> 217 tests.
@mattzcarey
test: add OAuth callback happy-path (full login flow)
74eee51 
Drive the complete login through the real worker: register a client,
GET /authorize for the consent dialog, POST the consent form (CSRF token
+ cookie) to get a provider-tracked grant, then GET /oauth/callback with
that state. Earlier the happy path 404'd because oauthReqInfo was
hand-rolled; going through the real /authorize round-trip gives
completeAuthorization the grant state it needs.

Asserts the callback 302s back to the client redirect URI with an auth
code, and that a successful auth_user datapoint is written (userId in
blob3, no error message in blob4). Upstream token + /user + /accounts are
mocked with MSW.

oauth-routes.test.ts: 5 -> 6 tests; suite 217 -> 218.
@mattzcarey
Unify tsconfig over src+tests; fix CI flake and deprecated test APIs
7f75b54 
- tsconfig: drop the tests/ exclude and the separate tests/tsconfig.json;
  one config now includes src + tests with both worker-configuration and
  @cloudflare/vitest-pool-workers/types. Fixes the IDE 'Cannot find module
  cloudflare:test / cloudflare:workers' errors and makes typecheck cover
  tests (which surfaced + fixed real latent type errors).
- vitest.config: provide dummy MCP_COOKIE_ENCRYPTION_KEY / CLOUDFLARE_*
  via miniflare.bindings so tests don't depend on a local .env. Fixes the
  CI-only callback failure (500 'Cookie secret is not defined' vs 302).
- Migrate deprecated cloudflare:test env imports to cloudflare:workers in
  oauth-handler/oauth-routes tests.
- Fix latent type errors now that tests are checked: OAuthHelpers mock
  cast, computeRetryDelay Omit<...,'caller'>, datapoint param typing.
- Remove obsolete tests/cloudflare-test.d.ts (env now comes typed from
  cloudflare:workers).

npm run check green; 218 tests pass.
@mattzcarey
test(index): integration-test scheduled() with real R2 + MSW
c684b71 
Replace the globalThis.fetch swap and fake SPEC_BUCKET with MSW (GitHub
boundary) and the real env.SPEC_BUCKET binding, driven by real
createScheduledController/createExecutionContext. Assertions now read the
processed spec.json/products.json back out of R2 instead of inspecting
put() spy args.

Adds tests/helpers/r2.ts clearR2() (parallel to clearKv) and two
bug-documenting cases on the real path:
- scheduled() does NOT retry a transient GitHub 5xx (bare fetch, no
  fetchWithRetry) -> spec update silently skipped for the day
- a 200 with a non-JSON body throws a raw SyntaxError instead of the
  friendly 'Failed to fetch OpenAPI spec' message

Suite 218 -> 220.
@mattzcarey
test(metrics): spy the real MCP_METRICS binding + cover tool_call emi…
09ae7be 
…ssion

- Replace the two hand-rolled { writeDataPoint } as unknown as
  AnalyticsEngineDataset fakes with vi.spyOn on the real env.MCP_METRICS
  binding (write + throw-on-write cases).
- Add worker-seam tests driving a real tools/call for execute through
  exports.default.fetch, asserting attachMetrics emits a tool_call
  datapoint (blob4='execute') on success and double1=-1 on an isError
  result. This covers the production emission path (userId-from-props,
  logResult, errorCodeOf) that had zero coverage.

Keep the pure toDataPoint/mapBlobs/guard unit tests and the
missing-binding no-op test as-is. Suite 220 -> 222.
@mattzcarey
fix(executor): guard REST error branch; behaviour-test via real isolate
656d441 
BUG FIX: the REST branch did data.errors.map(...) with no Array.isArray
guard (unlike the GraphQL branch), so a {success:false} response with a
missing/null errors array (e.g. a gateway/proxy envelope) threw
'Cannot read properties of undefined (reading map)' instead of a clean
'Cloudflare API error'. Guard it and fall back to the status code.

This was invisible before because executor.test.ts only grepped the
GENERATED worker source as strings and never compiled or ran it.

Rewrite executor.test.ts as behaviour tests through the real worker:
tools/call for execute loads code into a real Worker Loader isolate
(forwarded by the real GlobalOutbound), and search runs against a real
seeded SPEC_BUCKET; only the Cloudflare API boundary is MSW-mocked. Covers
REST success/error/no-errors-array/non-JSON, GraphQL
success/partial/complete-failure, and search spec-seeded/missing-spec/
no-network — none of which had runtime coverage before.

Suite 222 -> 216 (16 string-greps -> 10 behaviour tests).
@mattzcarey
fix(server): account_id optional for auto-resolvable non-codemode tools
db0fb7c 
BUG FIX: registerNonCodemodeTools marked account_id as a REQUIRED schema
field for every {account_id} path. The MCP SDK validates tool arguments
against the input schema BEFORE the handler runs, so account-token and
single-account user-token sessions could never call an account-scoped
non-codemode tool without passing account_id manually — they got
'MCP error -32602: Input validation error', and the handler's
auto-resolution (server.ts:359) was unreachable dead code in production.

Make account_id optional in the schema when it is auto-resolvable
(account token, or single-account user token); keep it required only for
multi-account user tokens where it genuinely can't be resolved.

This was invisible because the old tests called tool.handler({}) directly,
bypassing MCP validation. Add tests/non-codemode-worker.test.ts driving
/mcp?codemode=false through the real worker + MSW: proves an account
token can call WITHOUT account_id (verified red without the fix), explicit
account_id still forwarded, bearer/method asserted on the intercepted
request, and CF failures surface as isError. Remove the two misleading
direct-handler auto-resolve tests.

Suite 216 -> 218.
@mattzcarey
fix(oauth): preserve status+Retry-After on cached refresh replays; MS…
ac6cf1d 
…W-ify probes

BUG FIX: cacheRefreshFailure stored only { code, description }, and the
cached-replay path threw a hardcoded 400 with no headers. So a terminal
failure that was a 401 (invalid_client) or 403 (unauthorized_client), or
any failure carrying a Retry-After header, was replayed as a flat 400
with the header dropped — clients lost the correct status and backoff
hint on every retry within the cache TTL. Now store statusCode + headers
and restore them on replay. Two tests added (verified red without the
fix): 401 preserved across replay, Retry-After preserved across replay.

Also convert the 15 getUserAndAccounts identity-probe tests from
vi.stubGlobal('fetch') call-order stubs to MSW handlers matched by path
(/user, /accounts). The old stubs ignored URL/method entirely and relied
on Promise.all call ordering; MSW matches the real request, so a wrong
URL/method/dropped-bearer now fails instead of silently passing. Network
failure now uses HttpResponse.error() instead of a rejected fetch.
Removes the now-dead jsonResponse helper and vi.unstubAllGlobals.

Suite 218 -> 220.
@mattzcarey
mattzcarey force-pushed the add-e2e-tool-call-test branch from b865389 to ac6cf1d Compare June 9, 2026 13:06
@mattzcarey
fix(oauth): match userId when revoking grants (defense-in-depth)
1cf3c72 
revokeGrantsForClient filtered only on clientId and trusted
listUserGrants to scope by userId. Add an explicit grant.userId === userId
check so a provider bug returning a cross-user grant for the same client
can never cause us to revoke another user's grant. Add a test seeding a
same-client grant for a different user and asserting only the calling
user's grant is revoked (verified red without the guard).

Suite 220 -> 221.
@mattzcarey
mattzcarey merged commit d588476 into main Jun 9, 2026
5 checks passed
@mattzcarey
mattzcarey deleted the add-e2e-tool-call-test branch June 9, 2026 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant

@mattzcarey