diff --git a/agent-edge.mjs b/agent-edge.mjs index 0d64de2..0f98d6b 100644 --- a/agent-edge.mjs +++ b/agent-edge.mjs @@ -3,9 +3,11 @@ * Spec: foundry/ops/docs/agent-indexing-standard.md * * Usage in worker.mjs (before openNext.fetch): - * import { handleAgentEdge } from './agent-edge.mjs' + * import { handleAgentEdge, withApiJsonNotFound } from './agent-edge.mjs' * const agent = handleAgentEdge(request) * if (agent) return agent + * // ...and on the way back out, so unknown /api/* paths answer in JSON: + * return withApiJsonNotFound(request, await openNext.fetch(request, env, ctx)) */ /** @type {{ name: string, url: string, llmsTxt: string, llmsFullTxt?: string, indexMd: string, catalog: object }} */ @@ -88,6 +90,18 @@ export const AGENT_SURFACE = { }; /** + * Paths under `/api/` that the edge itself serves. Everything else under + * `/api/` belongs to Next.js and MUST fall through — see `withApiJsonNotFound`. + */ +const EDGE_OWNED_API_PATHS = new Set(['/api/ai']); + +/** + * Pre-handler: answers only for surfaces the edge itself owns. + * + * Returns `null` for everything else so the request reaches OpenNext/Next.js. + * This function must never decide that a path does *not* exist — only Next.js + * knows the route table. + * * @param {Request} request * @returns {Response | null} */ @@ -105,7 +119,7 @@ export function handleAgentEdge(request) { if (path === '/index.md') { return text(AGENT_SURFACE.indexMd, 'text/markdown; charset=utf-8'); } - if (path === '/api/ai') { + if (EDGE_OWNED_API_PATHS.has(path)) { // Re-bind origin so preview/custom domains stay correct const catalog = { ...AGENT_SURFACE.catalog, @@ -129,11 +143,6 @@ export function handleAgentEdge(request) { return json(openapiSpecForOrigin(url.origin)); } - // JSON error for unknown /api/* paths - if (path.startsWith('/api/')) { - return jsonError(404, 'not_found', `Unknown API path: ${path}`, path); - } - // Homepage markdown negotiation if ((path === '/' || path === '') && wantsMarkdown(request)) { return text(AGENT_SURFACE.indexMd, 'text/markdown; charset=utf-8', { @@ -142,14 +151,43 @@ export function handleAgentEdge(request) { }); } - // Agent-friendly 404: markdown body for Accept: text/markdown - if (wantsMarkdown(request) && !path.includes('.')) { + // Agent-friendly 404: markdown body for Accept: text/markdown. + // `/api/*` is excluded: those paths are Next.js route handlers, and an + // Accept header must never stop a real API request from reaching them. + // Unknown API paths are shaped into JSON by `withApiJsonNotFound` instead. + if (!isApiPath(path) && wantsMarkdown(request) && !path.includes('.')) { return markdown404(path, url.origin); } return null; } +/** + * Post-handler: shape Next.js's own 404 for `/api/*` into a JSON error body. + * + * The edge deliberately does not know which API routes exist — Next.js does. + * We call it, and only if *it* reports 404 do we swap the HTML error page for + * the machine-readable JSON envelope. That is why this cannot rot the way the + * previous allow-list did: adding a route handler under `src/app/api/` makes it + * reachable with no edge change, because the edge never asserts non-existence. + * + * @param {Request} request + * @param {Response} response Response from the downstream Next.js handler. + * @returns {Response} + */ +export function withApiJsonNotFound(request, response) { + if (request.method !== 'GET' && request.method !== 'HEAD') return response; + if (response.status !== 404) return response; + const path = new URL(request.url).pathname; + if (!isApiPath(path)) return response; + if ((response.headers.get('content-type') || '').includes('application/json')) return response; + return jsonError(404, 'not_found', `Unknown API path: ${path}`, path); +} + +function isApiPath(pathname) { + return pathname.startsWith('/api/'); +} + function wantsMarkdown(request) { const accept = (request.headers.get('accept') || '').toLowerCase(); if (!accept.includes('text/markdown')) return false; diff --git a/docs/development/conventions.md b/docs/development/conventions.md index c5dc1a9..c819c25 100644 --- a/docs/development/conventions.md +++ b/docs/development/conventions.md @@ -64,6 +64,14 @@ Code style and repo conventions. The executable source of truth is - `worker.mjs` — OpenNext-generated Worker entry. - `cloudflare-env.d.ts` — generated by `wrangler types`. +**Carry-forward when regenerating `agent-edge.mjs`:** the upstream template's +`/api/*` catch-all 404 shadowed every real Next.js route handler in production +(only `/api/ai` was allow-listed above it). The local file replaces that +pre-emptive catch-all with `withApiJsonNotFound`, a post-handler that shapes +*Next.js's own* 404 into JSON. Regenerating from an unfixed template +reintroduces the outage — `src/__tests__/agent-edge-api-routing.test.ts` fails +loudly if it comes back. + ## Agent skills / plugins — do not modify Do not modify, move, rename, or delete agent skills, plugins, or agent-profile diff --git a/e2e/public-app.spec.ts b/e2e/public-app.spec.ts index 66d80dd..e858904 100644 --- a/e2e/public-app.spec.ts +++ b/e2e/public-app.spec.ts @@ -322,3 +322,28 @@ test('uncataloged public preview asks for sign-in without presenting a rate-limi await expect(page.getByRole('link', { name: 'Sign in to preview' })).toBeVisible(); await expect(page.getByText(/rate limit/i)).toHaveCount(0); }); + +test('public API routes are reachable through the Worker edge on GET', async ({ request }) => { + // Regression guard for the `agent-edge.mjs` `/api/*` catch-all that 404'd + // every real route handler before OpenNext ever saw the request. Statuses + // vary with the e2e database contents, so assert only that these are NOT the + // edge's "Unknown API path" 404. + for (const path of ['/api/health', '/api/discover', '/api/tools', '/api/auth/session']) { + const response = await request.get(path, { maxRedirects: 0 }); + expect( + response.status(), + `${path} was 404'd — the edge is shadowing the Next.js route again` + ).not.toBe(404); + } + + // The edge's own surface still works... + const catalog = await request.get('/api/ai'); + expect(catalog.status()).toBe(200); + expect((await catalog.json()).name).toBe('Starboard'); + + // ...and a genuinely unknown API path still answers in JSON, not HTML. + const unknown = await request.get('/api/definitely-not-a-real-path'); + expect(unknown.status()).toBe(404); + expect(unknown.headers()['content-type']).toContain('application/json'); + expect((await unknown.json()).error.code).toBe('not_found'); +}); diff --git a/src/__tests__/agent-edge-api-routing.test.ts b/src/__tests__/agent-edge-api-routing.test.ts new file mode 100644 index 0000000..017a15b --- /dev/null +++ b/src/__tests__/agent-edge-api-routing.test.ts @@ -0,0 +1,195 @@ +/** + * Regression guard for the edge shadowing real API routes. + * + * Commit 4c67733 gave `agent-edge.mjs` a `path.startsWith('/api/')` catch-all + * that answered 404 *before* OpenNext ever saw the request, with only + * `/api/ai` allow-listed above it. Every other `GET /api/*` — Discover, health, + * tools, the whole NextAuth surface — was 404'd at the edge in production for + * six days. + * + * These tests drive the real `worker.mjs` entrypoint with a stubbed OpenNext + * handler (see `fixtures/open-next-worker-stub.mjs`), so they cover the actual + * wiring, not a reimplementation of it. + * + * The route list is read off the filesystem rather than hard-coded: a route + * handler added under `src/app/api/` is covered the moment it lands, which is + * exactly the rot that the original hand-maintained allow-list suffered. + */ +import { readdirSync } from 'node:fs'; +import { join, resolve } from 'node:path'; +import { beforeEach, describe, expect, it } from 'vitest'; + +import { openNextStub } from './fixtures/open-next-worker-stub.mjs'; +import worker from '../../worker.mjs'; + +const API_DIR = resolve(__dirname, '../app/api'); + +/** Turn `src/app/api/repos/[repoId]/route.ts` into a concrete `/api/repos/sample`. */ +function collectApiRoutePaths(dir: string, prefix = '/api'): string[] { + const paths: string[] = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (entry.isDirectory()) { + const segment = entry.name.startsWith('[') + ? // Dynamic and catch-all segments get a concrete stand-in value. + entry.name.startsWith('[...') || entry.name.startsWith('[[...') + ? 'sample/segment' + : 'sample' + : entry.name; + paths.push(...collectApiRoutePaths(join(dir, entry.name), `${prefix}/${segment}`)); + } else if (entry.name === 'route.ts' || entry.name === 'route.tsx') { + paths.push(prefix); + } + } + return paths; +} + +const API_ROUTE_PATHS = collectApiRoutePaths(API_DIR); + +const ctx = { waitUntil: () => undefined, passThroughOnException: () => undefined }; +const env = {} as Record; + +function get(path: string, headers: Record = {}) { + return worker.fetch( + new Request(`https://starboard.codevetter.com${path}`, { headers }), + env, + ctx + ); +} + +beforeEach(() => { + openNextStub.reset(); +}); + +describe('edge does not shadow Next.js API routes', () => { + it('found the route handlers to guard', () => { + // Sanity check: if this ever hits zero the suite below is vacuous. + expect(API_ROUTE_PATHS.length).toBeGreaterThan(20); + expect(API_ROUTE_PATHS).toContain('/api/discover'); + expect(API_ROUTE_PATHS).toContain('/api/health'); + }); + + it('reaches a real API route through the edge on GET', async () => { + openNextStub.handler = () => + new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + + const response = await get('/api/discover'); + + expect(openNextStub.calls).toEqual(['/api/discover']); + expect(response.status).toBe(200); + expect(await response.json()).toEqual({ ok: true }); + }); + + it.each(API_ROUTE_PATHS)('lets GET %s reach Next.js', async (path) => { + openNextStub.handler = () => new Response('routed', { status: 200 }); + + const response = await get(path); + + expect(openNextStub.calls).toEqual([path]); + expect(response.status).toBe(200); + }); + + it('does not let an Accept header divert an API route to the markdown 404', async () => { + openNextStub.handler = () => new Response('routed', { status: 200 }); + + const response = await get('/api/discover', { accept: 'text/markdown' }); + + expect(openNextStub.calls).toEqual(['/api/discover']); + expect(response.status).toBe(200); + }); +}); + +describe('unknown API paths still answer with the JSON 404 envelope', () => { + it('replaces the Next.js HTML 404 with JSON', async () => { + openNextStub.handler = () => + new Response('404', { + status: 404, + headers: { 'content-type': 'text/html; charset=utf-8' }, + }); + + const response = await get('/api/definitely-not-a-real-path'); + + expect(openNextStub.calls).toEqual(['/api/definitely-not-a-real-path']); + expect(response.status).toBe(404); + expect(response.headers.get('content-type')).toContain('application/json'); + expect(await response.json()).toEqual({ + error: { + code: 'not_found', + message: 'Unknown API path: /api/definitely-not-a-real-path', + path: '/api/definitely-not-a-real-path', + }, + }); + }); + + it("leaves a route handler's own 404 JSON body untouched", async () => { + openNextStub.handler = () => + new Response(JSON.stringify({ error: 'repo not found' }), { + status: 404, + headers: { 'content-type': 'application/json' }, + }); + + const response = await get('/api/repos/999999'); + + expect(await response.json()).toEqual({ error: 'repo not found' }); + }); + + it('leaves non-API 404s alone so the HTML error page still renders', async () => { + openNextStub.handler = () => + new Response('404', { + status: 404, + headers: { 'content-type': 'text/html; charset=utf-8' }, + }); + + // Deliberately not a cacheable document path — the edge HTML cache branch + // needs `caches.default`, which only exists in workerd. + const response = await get('/no-such-page'); + + expect(openNextStub.calls).toEqual(['/no-such-page']); + expect(response.status).toBe(404); + expect(response.headers.get('content-type')).toContain('text/html'); + }); +}); + +describe('edge-owned surfaces still work', () => { + it('serves the agent catalog at /api/ai without touching Next.js', async () => { + const response = await get('/api/ai'); + + expect(openNextStub.calls).toEqual([]); + expect(response.status).toBe(200); + const catalog = (await response.json()) as { name: string; openapi: string }; + expect(catalog.name).toBe('Starboard'); + expect(catalog.openapi).toBe('https://starboard.codevetter.com/openapi.json'); + }); + + it('serves the OpenAPI spec', async () => { + const response = await get('/openapi.json'); + + expect(openNextStub.calls).toEqual([]); + expect(response.status).toBe(200); + const spec = (await response.json()) as { openapi: string }; + expect(spec.openapi).toBe('3.1.0'); + }); + + it('serves llms.txt and index.md', async () => { + expect((await get('/llms.txt')).status).toBe(200); + expect((await get('/index.md')).status).toBe(200); + expect(openNextStub.calls).toEqual([]); + }); + + it('still negotiates markdown on the homepage', async () => { + const response = await get('/', { accept: 'text/markdown' }); + + expect(openNextStub.calls).toEqual([]); + expect(response.headers.get('content-type')).toContain('text/markdown'); + }); + + it('still serves the markdown 404 for unknown non-API pages', async () => { + const response = await get('/no-such-page', { accept: 'text/markdown' }); + + expect(openNextStub.calls).toEqual([]); + expect(response.status).toBe(404); + expect(response.headers.get('content-type')).toContain('text/markdown'); + }); +}); diff --git a/src/__tests__/fixtures/open-next-worker-stub.mjs b/src/__tests__/fixtures/open-next-worker-stub.mjs new file mode 100644 index 0000000..13544a9 --- /dev/null +++ b/src/__tests__/fixtures/open-next-worker-stub.mjs @@ -0,0 +1,34 @@ +/** + * Stand-in for `.open-next/worker.js` (a build artifact, gitignored, so it is + * absent in CI). `vitest.config.ts` aliases the OpenNext worker import to this + * module so `worker.mjs` — the real Cloudflare entrypoint — can be imported and + * exercised in a plain Node test. + * + * Tests set `openNextStub.handler` to decide what "Next.js" answers, and read + * `openNextStub.calls` to assert that a request actually reached it. + */ + +export const openNextStub = { + /** @type {(request: Request) => Response | Promise} */ + handler: () => new Response('stub handler not configured', { status: 500 }), + /** @type {string[]} */ + calls: [], + reset() { + openNextStub.calls = []; + openNextStub.handler = () => new Response('stub handler not configured', { status: 500 }); + }, +}; + +const worker = { + async fetch(request) { + openNextStub.calls.push(new URL(request.url).pathname); + return await openNextStub.handler(request); + }, +}; + +export default worker; + +// `worker.mjs` re-exports these Durable Object classes from the OpenNext entry. +export class DOQueueHandler {} +export class DOShardedTagCache {} +export class BucketCachePurge {} diff --git a/vitest.config.ts b/vitest.config.ts index ff18d8b..6f632d0 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -34,8 +34,15 @@ export default defineConfig({ }, }, resolve: { - alias: { - '@': resolve(__dirname, 'src'), - }, + alias: [ + // `.open-next/worker.js` is a gitignored build artifact, so it does not + // exist in CI. Alias it to a stub so `worker.mjs` — the real Cloudflare + // entrypoint — can be imported and exercised by unit tests. + { + find: /^\.\/\.open-next\/worker\.js$/, + replacement: resolve(__dirname, 'src/__tests__/fixtures/open-next-worker-stub.mjs'), + }, + { find: '@', replacement: resolve(__dirname, 'src') }, + ], }, }); diff --git a/worker.mjs b/worker.mjs index 7a3af6a..88aa982 100644 --- a/worker.mjs +++ b/worker.mjs @@ -13,7 +13,7 @@ import openNext from './.open-next/worker.js'; import { withTiming } from './timing.mjs'; -import { handleAgentEdge } from './agent-edge.mjs'; +import { handleAgentEdge, withApiJsonNotFound } from './agent-edge.mjs'; // Durable Objects must be re-exported from the entry that wrangler.toml // points at, otherwise the bindings can't resolve them at deploy time. @@ -68,18 +68,22 @@ function cacheKeyFor(request, versionId) { const worker = { fetch: withTiming(async function fetch(request, env, ctx) { - // Agent / LLM indexing surfaces (fleet GEO standard) + // Agent / LLM indexing surfaces (fleet GEO standard). This only answers + // for paths the edge itself owns; everything else falls through below. { const agent = handleAgentEdge(request); if (agent) return agent; } try { + // `withApiJsonNotFound` is applied on the way back out: Next.js owns the + // route table, so only its 404 (not an edge guess) turns into a JSON + // error body for `/api/*`. if (request.method !== 'GET') { - return openNext.fetch(request, env, ctx); + return withApiJsonNotFound(request, await openNext.fetch(request, env, ctx)); } const url = new URL(request.url); if (!isCacheableDocumentPath(url.pathname)) { - return openNext.fetch(request, env, ctx); + return withApiJsonNotFound(request, await openNext.fetch(request, env, ctx)); } // Auth-bearing requests pass straight through; the user is likely // going to be redirected by middleware to /library or /dashboard.