diff --git a/adapters/adapter-core/package.json b/adapters/adapter-core/package.json index bdceb7f..b7f4d55 100644 --- a/adapters/adapter-core/package.json +++ b/adapters/adapter-core/package.json @@ -27,6 +27,6 @@ "scripts": { "build": "tsc -p tsconfig.json", "typecheck": "tsc -p tsconfig.json --noEmit", - "test": "tsc -p tsconfig.json && node test/static.test.mjs && node test/host.test.mjs && node test/whoami.test.mjs && node test/agent-text.test.mjs && node test/delete-guard.test.mjs" + "test": "tsc -p tsconfig.json && node test/static.test.mjs && node test/host.test.mjs && node test/whoami.test.mjs && node test/agent-text.test.mjs && node test/delete-guard.test.mjs && node test/unknown-route.test.mjs" } } diff --git a/adapters/adapter-core/src/server.ts b/adapters/adapter-core/src/server.ts index 195bb47..35e8466 100644 --- a/adapters/adapter-core/src/server.ts +++ b/adapters/adapter-core/src/server.ts @@ -61,10 +61,21 @@ const REFERENCE_SCAN_MAX_PAGES = 200; const TASK_TIMEOUT_MS = 60_000; const TASK_MAX_DELIVERIES = 5; const DESCRIBE_PREFIX = "/api/_a2app/describe/"; +/** A miss is usually a guessed route, so it names the routes that answer. */ +const UNKNOWN_ROUTE_MESSAGE = + "No such route. Operations are invoked with POST /api/ops/; GET /api/_a2app/describe lists them."; +/** Path prefixes owned exclusively by the adapter. Keep in step with the router + * in `handle`. */ +const ADAPTER_NAMESPACES = ["/api/_a2app", "/api/ops", "/api/collections"]; + +function isAdapterNamespace(path: string): boolean { + return ADAPTER_NAMESPACES.some((ns) => path === ns || path.startsWith(`${ns}/`)); +} export interface A2App { - /** Route a request. Resolves to a reply, or null if the path is not an A2App - * path (the host app then handles it with its own routes). */ + /** Route a request. Resolves to a reply, or null if the path is outside the + * adapter's namespaces (the host app then handles it with its own routes). A + * miss inside them is a 404 reply, never null. */ handle(req: A2AppRequest): Promise; /** The identity document. */ identity(): Record; @@ -1141,6 +1152,11 @@ export function createA2App(binding: Binding, config: A2AppConfig): A2App { return handleOperation(req, decodeURIComponent(op[1]!)); } + // A miss in the adapter's own namespaces is the adapter's to answer. The rest + // of /api/ stays the host's: this is middleware, and a host may mount routes + // of its own there. + if (isAdapterNamespace(path)) return err(404, "not_found", UNKNOWN_ROUTE_MESSAGE); + return null; // not an A2App path — the host app handles it } diff --git a/adapters/adapter-core/src/static.ts b/adapters/adapter-core/src/static.ts index 2a2c21c..662fe96 100644 --- a/adapters/adapter-core/src/static.ts +++ b/adapters/adapter-core/src/static.ts @@ -196,6 +196,14 @@ export function createStaticView(dir: string, options: StaticViewOptions = {}): } function handler(req: IncomingMessage, res: ServerResponse): void { + const url = new URL(req.url ?? "/", "http://localhost"); + const file = resolveFile(url.pathname); + // Path before method: 405 is only for a file that exists. + if (file === null) { + notFound(res); + return; + } + const method = (req.method ?? "GET").toUpperCase(); if (method !== "GET" && method !== "HEAD") { res.writeHead(405, { "content-type": "application/json", allow: "GET, HEAD" }); @@ -205,13 +213,6 @@ export function createStaticView(dir: string, options: StaticViewOptions = {}): return; } - const url = new URL(req.url ?? "/", "http://localhost"); - const file = resolveFile(url.pathname); - if (file === null) { - notFound(res); - return; - } - const body = readFileSync(file); const etag = `"${createHash("sha256").update(body).digest("hex").slice(0, 32)}"`; const lastModified = statSync(file).mtime; diff --git a/adapters/adapter-core/test/static.test.mjs b/adapters/adapter-core/test/static.test.mjs index 5559903..7c6a475 100644 --- a/adapters/adapter-core/test/static.test.mjs +++ b/adapters/adapter-core/test/static.test.mjs @@ -136,6 +136,15 @@ await check("static assets are GET-only", async () => { assert.equal(res.headers.get("allow"), "GET, HEAD"); }); +await check("a POST to a path that names no file is a 404, not a 405", async () => { + // 405 means "right resource, wrong verb" — only true of a file that exists. + for (const path of ["/missing.html", "/sub"]) { + const res = await get(path, { method: "POST" }); + assert.equal(res.status, 404, `${path} answered ${res.status}`); + assert.equal((await res.json()).code, "not_found"); + } +}); + /* -------------------------------------------------------------- aliases */ await check("an alias serves a file from outside the View directory", async () => { diff --git a/adapters/adapter-core/test/unknown-route.test.mjs b/adapters/adapter-core/test/unknown-route.test.mjs new file mode 100644 index 0000000..b68454b --- /dev/null +++ b/adapters/adapter-core/test/unknown-route.test.mjs @@ -0,0 +1,109 @@ +/** + * A miss inside the adapter's namespaces is the adapter's to answer; the rest of + * /api/ stays the host's. + * + * Wired the way a blueprint wires it — the adapter first, the static View as the + * fallthrough — because the contract lives in the seam between the two. + * + * Run: node test/unknown-route.test.mjs + */ +import assert from "node:assert/strict"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { createServer } from "node:http"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createA2App, MemoryBinding, createStaticView } from "../dist/index.js"; +import { a2appMiddleware, createA2AppServer } from "../dist/http.js"; + +const dir = mkdtempSync(join(tmpdir(), "a2app-unknown-route-")); +writeFileSync(join(dir, "index.html"), "

view

"); + +const app = createA2App( + new MemoryBinding({ + appId: "unknown_route_test", + appName: "Unknown Route Test", + entities: { notes: { module: "desk", fields: [{ name: "title", type: "string" }], seed: [] } }, + }), + { modules: [{ name: "desk", summary: "notes" }] }, +); + +const server = createA2AppServer(app, createStaticView(dir).handler); +await new Promise((r) => server.listen(0, "127.0.0.1", r)); +const base = `http://127.0.0.1:${server.address().port}`; +const send = (method, path) => + fetch(`${base}${path}`, { + method, + headers: { "content-type": "application/json" }, + ...(method === "GET" ? {} : { body: "{}" }), + }); + +let failures = 0; +async function check(name, fn) { + try { + await fn(); + console.log(` ok ${name}`); + } catch (err) { + failures += 1; + console.error(` FAIL ${name}\n ${err.message}`); + } +} + +await check("POST to an unknown /api/_a2app route is a 404 that names the real routes", async () => { + const res = await send("POST", "/api/_a2app/operate"); + assert.equal(res.status, 404); + const body = await res.json(); + assert.equal(body.code, "not_found"); + assert.equal(body.ok, false); + assert.match(body.message, /POST \/api\/ops\//); + assert.match(body.message, /\/api\/_a2app\/describe/); +}); + +await check("the adapter, not the View, answers every miss under its namespaces", async () => { + const paths = ["/api/_a2app/does-not-exist", "/api/ops", "/api/ops/a/b", "/api/collections/notes", "/api/collections"]; + for (const path of paths) { + for (const method of ["GET", "POST", "PATCH", "DELETE"]) { + const res = await send(method, path); + assert.equal(res.status, 404, `${method} ${path} answered ${res.status}`); + // The View's 404 has the same code; only the adapter's names the routes. + assert.match((await res.json()).message, /\/api\/_a2app\/describe/, `${method} ${path}`); + } + } +}); + +await check("the routes that exist still answer", async () => { + assert.equal((await send("GET", "/api/_a2app/describe")).status, 200); + assert.equal((await (await send("POST", "/api/ops/nope")).json()).code, "unknown_operation"); +}); + +await check("a POST to a real static file is still a 405", async () => { + const res = await send("POST", "/index.html"); + assert.equal(res.status, 405); + assert.equal(res.headers.get("allow"), "GET, HEAD"); +}); + +await check("the rest of /api/ is still the host's", async () => { + const mw = a2appMiddleware(app); + const host = createServer((req, res) => + mw(req, res, () => { + res.writeHead(200, { "content-type": "application/json" }); + res.end(JSON.stringify({ host: true })); + }), + ); + await new Promise((r) => host.listen(0, "127.0.0.1", r)); + try { + const res = await fetch(`http://127.0.0.1:${host.address().port}/api/custom`, { method: "POST", body: "{}" }); + assert.equal(res.status, 200); + assert.equal((await res.json()).host, true); + } finally { + host.close(); + } +}); + +server.close(); +rmSync(dir, { recursive: true, force: true }); + +if (failures > 0) { + console.error(`\nunknown route: ${failures} check(s) failed`); + process.exit(1); +} +console.log("\nunknown route: all checks passed"); diff --git a/toolkits/blueprint-go-react/template/a2app_adapter.go b/toolkits/blueprint-go-react/template/a2app_adapter.go index c0e1296..98f5508 100644 --- a/toolkits/blueprint-go-react/template/a2app_adapter.go +++ b/toolkits/blueprint-go-react/template/a2app_adapter.go @@ -2454,7 +2454,7 @@ func (a *Adapter) dispatch(method, path string, headers map[string]string, body return a.handleOperation(headers, m[1], body) } - return errEnv(404, "not_found", "No such route.", nil) + return errEnv(404, "not_found", "No such route. Operations are invoked with POST /api/ops/; GET /api/_a2app/describe lists them.", nil) } /* -- records ------------------------------------------------------------------ */ diff --git a/toolkits/blueprint-go-react/template/main.go b/toolkits/blueprint-go-react/template/main.go index 4ba9a60..0aabcf6 100644 --- a/toolkits/blueprint-go-react/template/main.go +++ b/toolkits/blueprint-go-react/template/main.go @@ -379,6 +379,13 @@ func matchesEtag(header, etag string) bool { } func (v *staticView) serveHTTP(w http.ResponseWriter, r *http.Request) { + // Path before method: 405 is only for a file that exists. + file := v.resolveFile(r.URL.Path) + if file == "" { + writeJSON(w, 404, M{"a2app": true, "ok": false, "code": "not_found", "message": "No such route."}) + return + } + method := strings.ToUpper(r.Method) if method != "GET" && method != "HEAD" { w.Header().Set("Allow", "GET, HEAD") @@ -386,12 +393,6 @@ func (v *staticView) serveHTTP(w http.ResponseWriter, r *http.Request) { return } - file := v.resolveFile(r.URL.Path) - if file == "" { - writeJSON(w, 404, M{"a2app": true, "ok": false, "code": "not_found", "message": "No such route."}) - return - } - body, err := os.ReadFile(file) if err != nil { writeJSON(w, 404, M{"a2app": true, "ok": false, "code": "not_found", "message": "No such route."}) diff --git a/toolkits/blueprint-python-fastapi/template/a2app_adapter.py b/toolkits/blueprint-python-fastapi/template/a2app_adapter.py index 6961600..9425c40 100644 --- a/toolkits/blueprint-python-fastapi/template/a2app_adapter.py +++ b/toolkits/blueprint-python-fastapi/template/a2app_adapter.py @@ -1401,7 +1401,7 @@ def dispatch(self, method: str, path: str, headers: dict, body: Optional[dict], return self._err(405, "usage", "Operations are POST-only.") return self._handle_operation(headers, m.group(1), body or {}) - return self._err(404, "not_found", "No such route.") + return self._err(404, "not_found", "No such route. Operations are invoked with POST /api/ops/; GET /api/_a2app/describe lists them.") # -- records ------------------------------------------------------------ def _references_to(self, entity: str, rec_id: str) -> list[dict]: diff --git a/toolkits/blueprint-python-fastapi/template/main.py b/toolkits/blueprint-python-fastapi/template/main.py index 32e3e54..2bb5967 100644 --- a/toolkits/blueprint-python-fastapi/template/main.py +++ b/toolkits/blueprint-python-fastapi/template/main.py @@ -110,10 +110,13 @@ async def _read_body(request: Request): return {"__unparsed__": raw.decode("utf8", "replace")}, None -@app.api_route("/api/{path:path}", methods=["GET", "POST", "PATCH", "DELETE"]) +# Every standard method, so the adapter decides: a method left off this list would get +# Starlette's own 405, outside the a2app envelope, even on a route that does not +# exist. +@app.api_route("/api/{path:path}", methods=["GET", "HEAD", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"]) async def a2app_route(path: str, request: Request): body = None - if request.method in ("POST", "PATCH"): + if request.method in ("POST", "PUT", "PATCH"): body, too_large = await _read_body(request) if too_large is not None: return JSONResponse(status_code=too_large[0], content=too_large[1]) diff --git a/toolkits/blueprint-rails-vue/template/lib/a2app_adapter.rb b/toolkits/blueprint-rails-vue/template/lib/a2app_adapter.rb index 7f0a497..763b8ec 100644 --- a/toolkits/blueprint-rails-vue/template/lib/a2app_adapter.rb +++ b/toolkits/blueprint-rails-vue/template/lib/a2app_adapter.rb @@ -1431,7 +1431,7 @@ def dispatch(method, path, headers, body, query = nil) return handle_operation(headers, m[1], body || {}) end - err(404, "not_found", "No such route.") + err(404, "not_found", "No such route. Operations are invoked with POST /api/ops/; GET /api/_a2app/describe lists them.") end # -- records ------------------------------------------------------------ diff --git a/toolkits/blueprint-rust-react/template/src/a2app_adapter.rs b/toolkits/blueprint-rust-react/template/src/a2app_adapter.rs index cd1f9e4..ad88ea7 100644 --- a/toolkits/blueprint-rust-react/template/src/a2app_adapter.rs +++ b/toolkits/blueprint-rust-react/template/src/a2app_adapter.rs @@ -2540,7 +2540,7 @@ impl Adapter { } } - Self::err(404, "not_found", "No such route.") + Self::err(404, "not_found", "No such route. Operations are invoked with POST /api/ops/; GET /api/_a2app/describe lists them.") } // -- records ------------------------------------------------------------ diff --git a/toolkits/blueprint-rust-react/template/src/main.rs b/toolkits/blueprint-rust-react/template/src/main.rs index c0c7309..0af8534 100644 --- a/toolkits/blueprint-rust-react/template/src/main.rs +++ b/toolkits/blueprint-rust-react/template/src/main.rs @@ -600,21 +600,6 @@ fn handle_request( } // ---------------------------------------------------------- static View - if method != "GET" && method != "HEAD" { - let response = tiny_http::Response::from_string( - json!({ - "a2app": true, "ok": false, "code": "method_not_allowed", - "message": "Static assets are GET-only.", - }) - .to_string(), - ) - .with_status_code(405) - .with_header(header("Content-Type", "application/json")) - .with_header(header("Allow", "GET, HEAD")); - let _ = request.respond(response); - return 405; - } - // Alias before path resolution: the watcher is served from the project // root, outside the built tree, so it survives any View rewrite. let file = if path == "/_a2app/update.js" { @@ -626,11 +611,27 @@ fn handle_request( } else { resolve_static(served_dir, path) }; + // Path before method: 405 is only for a file that exists. let file = match file { Some(f) => f, None => return respond_json(request, 404, ¬_found_envelope()), }; + if method != "GET" && method != "HEAD" { + let response = tiny_http::Response::from_string( + json!({ + "a2app": true, "ok": false, "code": "method_not_allowed", + "message": "Static assets are GET-only.", + }) + .to_string(), + ) + .with_status_code(405) + .with_header(header("Content-Type", "application/json")) + .with_header(header("Allow", "GET, HEAD")); + let _ = request.respond(response); + return 405; + } + let body = match fs::read(&file) { Ok(b) => b, Err(_) => return respond_json(request, 404, ¬_found_envelope()),