Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion docs/agents/end-to-end-testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ that are unlikely to recur.

- Keep tests flat: top-level `test(...)` with no `describe` nesting.
- Inline setup per test; avoid shared `beforeEach` unless required.
- Prefer one clear assertion per step and a small number of final assertions.
- Prefer fewer, longer tests when one user journey covers the behavior.
- Treat each E2E test like a manual tester's script: one setup, then the
actions and assertions needed to validate the whole flow.
- Do not split a single journey into multiple tiny tests just to isolate each
assertion.
- Use Playwright’s `expect` and locator APIs (role/label/placeholder).

## Locators
Expand Down Expand Up @@ -84,6 +88,8 @@ handled by the static asset fetcher in `packages/worker/src/index.ts`.

- Assert user-facing results (success message, redirect, visible element).
- For async actions, wait on the UI result, not arbitrary timeouts.
- Assert important intermediate states as part of the same journey that causes
them instead of creating isolated loading-state or transition-state tests.
- For client-router regressions, you may set a `window` marker before clicking a
link and assert it survives navigation to prove there was no full document
reload.
Expand Down
18 changes: 16 additions & 2 deletions docs/agents/testing-principles.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
# Testing principles

This codebase favors small, readable tests with explicit setup and minimal
magic.
This codebase favors small, readable test suites with explicit setup and
minimal magic. Individual tests should follow a meaningful workflow end-to-end,
even when that makes a single test longer and more assertion-heavy.

## Principles

- Prefer the "fewer, longer tests" style from Kent C. Dodds when assertions
belong to one workflow.
- Treat each test like a manual tester's script: one setup, then as many
actions and assertions as needed to validate the whole journey.
- Do not split a single flow into many tiny tests just to satisfy "one
assertion per test." Multiple related assertions in one test are a feature,
not a smell.
- Prefer flat test files: use top-level `test(...)` and avoid `describe`
nesting.
- Avoid shared setup like `beforeEach`/`afterEach`; inline setup per test.
- Avoid shared mutable test state across cases. If the next assertion depends on
the same rendered object, request, or response, it likely belongs in the same
test.
- Don't write tests for what the type system already guarantees.
- Use disposable objects only when there is real cleanup. If no cleanup, skip
`using` and `Symbol.dispose`.
Expand All @@ -20,6 +31,9 @@ magic.
tests.
- Prefer fast unit tests for server logic; keep e2e tests focused on a very
small number of important happy-path journeys.
- Prefer asserting intermediate states inside the broader workflow that causes
them rather than adding isolated tests that only check an incidental loading
or transition state.
- Do not add regression tests for bugs that are unlikely to happen again unless
the flow is important enough to justify the maintenance cost.
- Avoid tests that only assert a string blob contains a description or other
Expand Down
99 changes: 54 additions & 45 deletions packages/worker/src/mcp-auth.workers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,24 @@ import {
} from './mcp-auth.ts'
import { oauthScopes } from './oauth-handlers.ts'

function expectAuthenticateHeader(
header: string,
origin: string,
options: {
expectScope?: boolean
} = {},
) {
expect(header).toContain(
`resource_metadata="${origin}${protectedResourceMetadataPath}"`,
)

if (options.expectScope ?? true) {
if (oauthScopes.length > 0) {
expect(header).toContain(`scope="${oauthScopes.join(' ')}"`)
}
}
}

function createHelpers(overrides: Partial<OAuthHelpers> = {}): OAuthHelpers {
return {
async parseAuthRequest() {
Expand Down Expand Up @@ -44,66 +62,57 @@ function createContext() {
} as unknown as ExecutionContext
}

test('protected resource metadata describes MCP server', async () => {
const request = new Request(
`https://example.com${protectedResourceMetadataPath}`,
)
const response = handleProtectedResourceMetadata(request)
test('protected resource metadata and auth challenge resolve origin consistently', async () => {
const requestOrigin = 'https://example.com'
const appBaseUrl = 'https://heykody.dev'

expect(response.status).toBe(200)
const payload = await response.json()
expect(payload).toEqual(buildProtectedResourceMetadata('https://example.com'))
})

test('protected resource metadata prefers APP_BASE_URL over request origin', async () => {
const request = new Request(
`https://kody-production.kentcdodds.workers.dev${protectedResourceMetadataPath}`,
const requestOriginMetadataResponse = handleProtectedResourceMetadata(
new Request(`${requestOrigin}${protectedResourceMetadataPath}`),
)
expect(requestOriginMetadataResponse.status).toBe(200)
expect(await requestOriginMetadataResponse.json()).toEqual(
buildProtectedResourceMetadata(requestOrigin),
)
const response = handleProtectedResourceMetadata(request, {
APP_BASE_URL: 'https://heykody.dev',
} as Env)

expect(response.status).toBe(200)
const payload = await response.json()
expect(payload).toEqual(buildProtectedResourceMetadata('https://heykody.dev'))
})
const appBaseUrlMetadataResponse = handleProtectedResourceMetadata(
new Request(
`https://kody-production.kentcdodds.workers.dev${protectedResourceMetadataPath}`,
),
{
APP_BASE_URL: appBaseUrl,
} as Env,
)
expect(appBaseUrlMetadataResponse.status).toBe(200)
expect(await appBaseUrlMetadataResponse.json()).toEqual(
buildProtectedResourceMetadata(appBaseUrl),
)

test('mcp request without token returns 401 with resource metadata', async () => {
const request = new Request(`https://example.com${mcpResourcePath}`)
const response = await handleMcpRequest({
request,
const requestOriginUnauthorizedResponse = await handleMcpRequest({
request: new Request(`${requestOrigin}${mcpResourcePath}`),
env: createEnv(createHelpers()),
ctx: createContext(),
fetchMcp: () => new Response('ok'),
})

expect(response.status).toBe(401)
const header = response.headers.get('WWW-Authenticate') ?? ''
expect(header).toContain(
`resource_metadata="https://example.com${protectedResourceMetadataPath}"`,
expect(requestOriginUnauthorizedResponse.status).toBe(401)
expectAuthenticateHeader(
requestOriginUnauthorizedResponse.headers.get('WWW-Authenticate') ?? '',
requestOrigin,
)
if (oauthScopes.length > 0) {
expect(header).toContain(`scope="${oauthScopes.join(' ')}"`)
}
})

test('mcp request without token prefers APP_BASE_URL in auth metadata', async () => {
const request = new Request(
`https://kody-production.kentcdodds.workers.dev${mcpResourcePath}`,
)
const response = await handleMcpRequest({
request,
const appBaseUrlUnauthorizedResponse = await handleMcpRequest({
request: new Request(
`https://kody-production.kentcdodds.workers.dev${mcpResourcePath}`,
),
env: createEnv(createHelpers(), {
APP_BASE_URL: 'https://heykody.dev',
APP_BASE_URL: appBaseUrl,
}),
ctx: createContext(),
fetchMcp: () => new Response('ok'),
})

expect(response.status).toBe(401)
const header = response.headers.get('WWW-Authenticate') ?? ''
expect(header).toContain(
`resource_metadata="https://heykody.dev${protectedResourceMetadataPath}"`,
expect(appBaseUrlUnauthorizedResponse.status).toBe(401)
expectAuthenticateHeader(
appBaseUrlUnauthorizedResponse.headers.get('WWW-Authenticate') ?? '',
appBaseUrl,
)
})

Expand Down
Loading
Loading