From e34d3eeaed39676bb6ae91457b22c8b296b25244 Mon Sep 17 00:00:00 2001 From: mvschwarz Date: Fri, 7 Aug 2026 20:50:00 -0700 Subject: [PATCH] The table IS the router: routes cannot exist without being enumerated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sdk-qa returned 93b5612 with three findings of one shape — each earlier fix handled the instance, not the class. All three closed here, each with a control seen failing on the broken version before the fix was written. 1) Capture targets: performability, not the final component. The isDirectory check covered "the target is a file" and missed "the deepest existing ancestor is a file" — declare file-parent/child and the 200 hands the consumer a guaranteed ENOTDIR. resolveInRoot now requires every existing component of the resolved path to be a directory: an existing target must be a dir, a missing target's deepest existing ancestor must be a dir. Unperformable bindings lose to performable ones exactly as existence decides between bindings generally, and blocked-everywhere refuses naming the blocking file. Tests for both, plus the multi-binding composition. 2) Route discovery: PM's ruling, opt-in IS the hole. serves() was a registration wrapper a raw handler could skip — sdk-qa planted three spellings that served 200, appeared nowhere in runtime.routes, and passed the suite, reproducing the original product failure. The wrapper is DELETED, not patched: API_ROUTES is now a declarative table that request dispatch walks AND that runtime.routes/classification enumerate — one structure, two readers, so a route cannot serve without a row. The boot-time enumeration walk is gone with it (the set is complete at load by construction), which also removes the top-level await that caused the TDZ boot failure two commits ago. What a table cannot prevent is a raw arm typed above the dispatch — so the routing surface is fenced by markers and routingSurfaceViolations() (verbs.mjs) fails the suite on any /api mention outside the fence, in every spelling, replacing discoverApiRoutes (a per-spelling regex, the approximation PM ruled against). The bypass control plants sdk-qa's exact three raw forms, boots each, and asserts orch's dichotomy: served implies reported-or-named, else 404. Run against 93b5612 it fails with the finding's own words: serves 200, not in runtime.routes, tripwire blind. 3) The TDZ guard now covers INITIALIZERS. `const probe = handleRequest;` executes at module load exactly like a statement; the previous scan excluded declarations wholesale, so the runtime crashed while the guard passed 2/2. Initializers are scanned for references to later bindings — except pure function expressions, whose bodies run at call time, with embedded function bodies and string contents stripped so handler tables do not false-positive. Controls: the planted initializer fires (statement N names handleRequest), the arrow initializer does not, and sdk-qa's exact plant against the real file reports the probe line naming handleRequest. contract-meta.md's runtime.routes bullet now states the by-construction claim and records why the registration wording was false. 344 -> 348 tests, 348/348 green. --- app/serve-studio.mjs | 198 +++++++++++++++++---------------- app/verbs.mjs | 73 +++++++++--- contract/contract-meta.md | 17 ++- test/capture-target.test.mjs | 51 +++++++++ test/reserved-verbs.test.mjs | 120 ++++++++++++++------ test/runtime-redeploy.test.mjs | 103 ++++++++++++++--- 6 files changed, 398 insertions(+), 164 deletions(-) diff --git a/app/serve-studio.mjs b/app/serve-studio.mjs index b970058..8d4c07e 100644 --- a/app/serve-studio.mjs +++ b/app/serve-studio.mjs @@ -147,34 +147,12 @@ const TYPES = { ".html": "text/html; charset=utf-8", ".json": "application/json" ".js": "text/javascript", ".mjs": "text/javascript", ".svg": "image/svg+xml", ".png": "image/png", ".md": "text/plain; charset=utf-8", ".txt": "text/plain; charset=utf-8" }; -// THE ROUTING TABLE IS THE CLASSIFICATION SOURCE. -// -// Every `/api` arm goes through `serves()`, which MATCHES using the verb grammar -// (a trailing `/` is a prefix) and RECORDS the verb it matched on. So the set of -// routes this runtime serves is OBSERVED FROM THE ROUTER ITSELF rather than -// approximated by reading the file. -// -// The previous guard scanned source with a regex that recognised only -// `u.pathname === "..."`. A live `startsWith("/api/qa-added-prefix/")` route was -// INVISIBLE to it: the route answered 200 while the classification suite passed, -// because a route the scanner cannot see is a route the classifier is never asked -// about. A regex over source is an approximation of the router; this is the router. -// -// SERVED_ROUTES is exposed at `/api/contract` under `runtime.routes` so a test can -// compare what is SERVED against what is CLASSIFIED without parsing anything. -const SERVED_ROUTES = new Set(); -const serves = (pathname, verb) => { - SERVED_ROUTES.add(verb); - return verbMatches(verb, pathname); -}; - -// ⚠️ THE CHAIN IS ORDERED, SO A MATCH STOPS IT. A request that matches an early -// arm never reaches the later ones, and they would go unregistered — an observed -// set that is silently PARTIAL is worse than a declared one, because it looks -// derived. So the chain is walked ONCE at boot with a path that matches nothing, -// which reaches every arm and registers all of them before anyone can read it. -const ENUMERATE_PATH = "/api/\u0000enumerate-routes"; - +// ROUTING LIVES IN ONE PLACE: the API ROUTING SURFACE in the HTTP section below. +// The dispatch table there is simultaneously the matcher, the enumeration behind +// `runtime.routes`, and the classification source — one structure, two readers. +// Its predecessor here was a registration wrapper (`serves()`) every arm had to +// remember to call; opt-in registration is a convention, not a mechanism, and a +// raw handler that skipped it served live while the classifier stayed blind. const sendJson = (res, code, obj) => { res.writeHead(code, { "content-type": "application/json" }); res.end(JSON.stringify(obj)); }; @@ -927,26 +905,36 @@ function resolveInRoot(kind, rel) { const rest = path.relative(probe, target); const resolved = rest ? path.join(real, rest) : real; if (resolved !== baseReal && !resolved.startsWith(baseReal + path.sep)) continue; - // EXISTS IS NOT ENOUGH — IT MUST BE A DIRECTORY. A capture target is a place - // to write files into, and an existing regular file at that path was accepted - // and stored: the declaration returned 200 and the real consumer then failed - // with EEXIST on mkdir. Accepted-but-unperformable, and this contract puts the - // refusal AT DECLARATION precisely so a consumer cannot inherit one. + // THE PROPERTY IS PERFORMABILITY, NOT THE FINAL COMPONENT. A capture target + // is a place a consumer will `mkdir recursive` into, so EVERY existing + // component of the resolved path must be a directory: + // - the target itself, if it exists (a regular file there was accepted and + // the consumer failed EEXIST), and + // - the deepest existing ANCESTOR when the target does not exist yet (a + // regular file there was ALSO accepted — same class, one level up — and + // the consumer failed ENOTDIR; the path can never become a directory). + // Both are refusals AT DECLARATION so a consumer cannot inherit either. let stat = null; try { stat = fs.statSync(resolved); } catch {} - candidates.push({ base: baseReal, path: resolved, - exists: !!stat, isDir: !!stat && stat.isDirectory() }); + const exists = !!stat, isDir = exists && stat.isDirectory(); + let ancIsDir = false; + try { ancIsDir = fs.statSync(real).isDirectory(); } catch {} + const performable = exists ? isDir : ancIsDir; + candidates.push({ base: baseReal, path: resolved, exists, isDir, performable, + blockedBy: performable ? null : (exists ? resolved : real) }); } if (!candidates.length) { return { ok: false, error: `'${rel}' resolves outside every location bound to '${kind}'` }; } - // A path that exists as a NON-directory is a refusal, not a candidate to skip: - // skipping it would silently resolve to a different binding and land captures - // somewhere the caller did not name. - const blocked = candidates.filter((c) => c.exists && !c.isDir); - if (blocked.length && !candidates.some((c) => c.isDir)) { - return { ok: false, error: `'${rel}' exists under '${kind}' but is not a directory ` + - `(${blocked[0].path}) — a capture target is a directory to write into` }; + // An UNPERFORMABLE path is a refusal, not a candidate to skip — unless another + // binding is genuinely performable, in which case existence/performability + // decides exactly as it does between bindings generally. Skipping silently with + // nothing better would land captures somewhere the caller did not name; + // accepting the unperformable one hands the consumer a guaranteed failure. + const blocked = candidates.filter((c) => !c.performable); + if (blocked.length && !candidates.some((c) => c.performable)) { + return { ok: false, error: `'${rel}' cannot be a capture target under '${kind}': ` + + `'${blocked[0].blockedBy}' is not a directory — a capture target is a directory to write into` }; } // DEDUPE BY RESOLVED PATH before counting ambiguity. Two bindings that resolve to // the SAME real location are one location: `[first, first]`, or two paths through @@ -967,8 +955,9 @@ function resolveInRoot(kind, rel) { `(${existing.map((c) => c.base).join(", ")}) — declare a kind that binds one, or remove the ambiguity` }; } // Exactly one existing wins. None existing is the create-it-later case, and the - // FIRST binding is the deliberate choice there — stated rather than incidental. - const chosen = existing[0] ?? candidates[0]; + // first PERFORMABLE binding is the deliberate choice there — stated rather than + // incidental, and never a binding that cannot honour the declaration. + const chosen = existing[0] ?? candidates.find((c) => c.performable); return { ok: true, root: kind, base: chosen.base, path: chosen.path, existed: chosen.exists }; } @@ -1019,10 +1008,22 @@ function filesSearch(q) { } // ---- HTTP ------------------------------------------------------------------ -const handleRequest = async (req, res) => { - const u = new URL(req.url, `http://127.0.0.1:${PORT}`); - try { - if (serves(u.pathname, "/api/focus")) { +// >>> API ROUTING SURFACE — every /api mention in this file lives between these +// two markers, enforced by routingSurfaceViolations() in app/verbs.mjs and the +// reserved-verbs tests. A mention outside the fence fails the suite. +// +// ONE STRUCTURE, TWO READERS — PM's ruling, third round on this class. This +// table is what dispatch consults at request time AND what `runtime.routes` and +// classification enumerate, so a route cannot serve without a row and +// enumeration cannot be opt-in. The registration wrapper it replaces +// (`serves()`) was a convention: a raw handler that never called it served live +// while the classifier stayed blind — sdk-qa proved it in three spellings. +// +// ADD A ROUTE BY ADDING A ROW, then classify the verb in app/verbs.mjs. +// A verb ending in `/` is a PREFIX, per the shared grammar (verbMatches). +const CONTRACT_VERB = "/api/contract"; +const API_ROUTES = [ + { verb: "/api/focus", handler: async (u, req, res) => { if (req.method === "POST") { const raw = await readBody(req); let patch; @@ -1039,8 +1040,8 @@ const handleRequest = async (req, res) => { const since = u.searchParams.get("since"); const marker = focusMarker(); return sendJson(res, 200, { ok: true, changed: since === null || since !== marker, marker, focus: focusRecord }); - } - if (serves(u.pathname, "/api/drive")) { + } }, + { verb: "/api/drive", handler: async (u, req, res) => { if (req.method === "POST") { const raw = await readBody(req); let op; @@ -1056,8 +1057,8 @@ const handleRequest = async (req, res) => { const since = u.searchParams.get("since"); const marker = driveMarker(); return sendJson(res, 200, { ok: true, changed: since === null || since !== marker, marker, op: driveOp }); - } - if (serves(u.pathname, "/api/capture-target")) { + } }, + { verb: "/api/capture-target", handler: async (u, req, res) => { if (req.method === "POST") { const raw = await readBody(req); let body; @@ -1077,8 +1078,8 @@ const handleRequest = async (req, res) => { return sendJson(res, 200, { ok: true, target: captureTarget }); } return sendJson(res, 200, { ok: true, target: captureTarget }); - } - if (serves(u.pathname, "/api/annotations")) { + } }, + { verb: "/api/annotations", handler: async (u, req, res) => { if (req.method === "POST") { const raw = await readBody(req); let body; @@ -1098,8 +1099,8 @@ const handleRequest = async (req, res) => { // receive another surface's marks and render them over this one. const scope = u.searchParams.get("scope"); return sendJson(res, 200, { ok: true, scope: scope || null, records: scope ? (annotations.get(scope) || []) : [] }); - } - if (serves(u.pathname, "/api/contract")) { + } }, + { verb: CONTRACT_VERB, handler: (u, req, res) => { return sendJson(res, 200, { contractVersion: CONTRACT_VERSION, // VERSION IS READ, NEVER TYPED. A runtime that states its own version from a @@ -1109,11 +1110,13 @@ const handleRequest = async (req, res) => { // from the package this file is part of; null if that cannot be read, because // "I do not know" is an answer and a guess is not. runtime: { name: "openrig-studio", version: SDK_VERSION, flavor: "reference-fixture", boot: BOOT_ID, - // OBSERVED from the router, not declared: every verb an arm has matched - // on. A test compares this against the classification, so a route that - // exists but is unclassified is caught by the runtime's own account of - // itself rather than by reading the source. - routes: [...SERVED_ROUTES].sort() }, + // THE SERVING TABLE ITSELF, enumerated. Not observed, not registered, + // not approximated: the same rows dispatch walks on every request. A + // test compares this against the classification, so a route that + // exists but is unclassified is caught by the runtime's own account — + // and a route cannot exist without a row, because dispatch only + // consults rows. + routes: API_ROUTES.map((r) => r.verb).sort() }, capabilities: CAPABILITIES, // Inspectable without posting: an agent can ask whether anything is // listening before it drives, rather than learning from a no-op. @@ -1139,21 +1142,21 @@ const handleRequest = async (req, res) => { scopes: annotations.size, writes: annotationWrites }, manifest: manifestReport(), }); - } - if (serves(u.pathname, "/api/factory/state")) return sendJson(res, 200, factoryState()); - if (serves(u.pathname, "/api/events")) { + } }, + { verb: "/api/factory/state", handler: (u, req, res) => sendJson(res, 200, factoryState()) }, + { verb: "/api/events", handler: (u, req, res) => { res.writeHead(200, { "content-type": "text/event-stream", "cache-control": "no-store", connection: "keep-alive" }); res.write(`: connected\n\n`); sseClients.add(res); req.on("close", () => sseClients.delete(res)); return; - } - if (serves(u.pathname, "/api/files/tree")) { + } }, + { verb: "/api/files/tree", handler: (u, req, res) => { const t = filesTree(u.searchParams.get("dir")); return t ? sendJson(res, 200, { ok: true, ...t }) : sendJson(res, 400, { ok: false, error: "path outside the files root — files verbs are pinned to the runtime's files-root" }); - } - if (serves(u.pathname, "/api/files/read")) { + } }, + { verb: "/api/files/read", handler: (u, req, res) => { const r = insideRoot(u.searchParams.get("path") || ""); if (!r || !fs.existsSync(r) || fs.statSync(r).isDirectory()) return sendJson(res, 404, { ok: false, error: "not found or outside the files root" }); @@ -1162,22 +1165,38 @@ const handleRequest = async (req, res) => { return sendJson(res, 200, { ok: true, kind: kind === "other" ? "text" : kind, content: fs.readFileSync(r, "utf8"), mtime: fs.statSync(r).mtimeMs }); return sendJson(res, 200, { ok: true, kind, raw: "/api/files/raw?path=" + encodeURIComponent(r), mtime: fs.statSync(r).mtimeMs }); - } - if (serves(u.pathname, "/api/files/raw")) { + } }, + { verb: "/api/files/raw", handler: (u, req, res) => { const r = insideRoot(u.searchParams.get("path") || ""); if (!r || !fs.existsSync(r)) { res.writeHead(404); return res.end(); } res.writeHead(200, { "content-type": TYPES[path.extname(r).toLowerCase()] || "application/octet-stream", "content-length": fs.statSync(r).size, "cache-control": "no-store" }); return fs.createReadStream(r).pipe(res); - } - if (serves(u.pathname, "/api/files/search")) { + } }, + { verb: "/api/files/search", handler: (u, req, res) => { const q = (u.searchParams.get("q") || "").trim(); return sendJson(res, 200, { ok: true, hits: q.length < 2 ? [] : filesSearch(q) }); - } - if (u.pathname.startsWith("/api/")) { - return sendJson(res, 404, { ok: false, - error: `no such contract route: ${u.pathname} — see GET /api/contract for capabilities and contract/runtime-api.md for the verb set` }); - } + } }, +]; + +// The one door. Claims the WHOLE /api namespace: a row matches or the request +// 404s here, so nothing after this dispatcher ever sees an /api path and a +// handler cannot exist outside the table it is enumerated from. +const dispatchApi = async (u, req, res) => { + if (!u.pathname.startsWith("/api/")) return false; + for (const r of API_ROUTES) { + if (verbMatches(r.verb, u.pathname)) { await r.handler(u, req, res); return true; } + } + sendJson(res, 404, { ok: false, + error: `no such contract route: ${u.pathname} — see GET ${CONTRACT_VERB} for capabilities and contract/runtime-api.md for the verb set` }); + return true; +}; +// <<< API ROUTING SURFACE + +const handleRequest = async (req, res) => { + const u = new URL(req.url, `http://127.0.0.1:${PORT}`); + try { + if (await dispatchApi(u, req, res)) return; } catch (e) { return sendJson(res, 500, { ok: false, error: String(e.message || e) }); } // the manifest is served as its VALIDATED projection, not the raw file — @@ -1276,7 +1295,7 @@ const handleRequest = async (req, res) => { `no declared surface claims this path — the runtime serves only the SDK's own declared ` + `surfaces plus your overlay-declared ones\n` + `(if this file is in the installed package, it is undeclared content; see manifest.warnings ` + - `at /api/contract)` + `at ${CONTRACT_VERB})` ); } @@ -1285,7 +1304,7 @@ const handleRequest = async (req, res) => { const file = within(HERE, rel); if (!file) { res.writeHead(404, { "content-type": "text/plain" }); - return res.end(`not found: ${u.pathname}\n(surfaces live under /surfaces/, contract meta at /api/contract)`); + return res.end(`not found: ${u.pathname}\n(surfaces live under /surfaces/, contract meta at ${CONTRACT_VERB})`); } res.writeHead(200, { "content-type": TYPES[path.extname(file)] || "application/octet-stream", "cache-control": "no-store" }); res.end(fs.readFileSync(file)); @@ -1296,10 +1315,10 @@ const handleRequest = async (req, res) => { // sits after EVERY module binding — verified by a test, not by reading — so // startup cannot land in a temporal dead zone no matter what these statements // grow to touch later. Being last is NOT the same as being after everything you -// touch: the walk below calls `handleRequest`, so it must follow that binding -// specifically, and it once sat in an init block that was "at the bottom" of the -// declarations it happened to know about while `handleRequest` was declared -// beneath it. The test asserts both properties — position AND reachability. +// touch: a boot-time statement once sat in an init block that was "at the +// bottom" of the declarations it happened to know about while `handleRequest` +// was declared beneath it, and died in that binding's dead zone. The test +// asserts both properties — position AND reachability. // // The weaker arrangement — init partway down, "safe" because the current call // graph happens not to reach the later bindings — is a temporal dead zone @@ -1318,20 +1337,13 @@ setInterval(() => { for (const res of sseClients) { try { res.write(`: heartbeat // this line exists. loadAnnotations(); -// WALK THE CHAIN ONCE, so `runtime.routes` is COMPLETE rather than "whatever has -// been hit so far". The sentinel matches no arm, so every `serves()` runs and -// registers. A partial observed set would look derived and be a lie of exactly the -// kind this mechanism exists to remove. It runs after `handleRequest` — which it -// calls — and before `.listen()`, so the first real request sees the complete set. -await handleRequest( - { url: ENUMERATE_PATH, method: "GET", headers: {}, on() {}, once() {}, removeListener() {} }, - { writeHead() {}, end() {}, write() {}, setHeader() {}, on() {}, once() {} }, -).catch(() => {}); +// No boot-time walk registers routes any more: `runtime.routes` enumerates the +// dispatch table directly, so the set is complete the moment the module loads. http.createServer(handleRequest).listen(PORT, "127.0.0.1", () => { console.log(`openrig studio runtime: http://127.0.0.1:${PORT}/ (contract v${CONTRACT_VERSION})`); console.log(`fixtures: ${FIXTURES}`); const report = manifestReport(); if (OVERLAY_DIR) console.log(`consumer surfaces: ${OVERLAY_DIR} (repo-owned; nothing is written into the package)`); - if (report.errors.length) console.error(`manifest has ${report.errors.length} error(s) — serving ${report.surfaces} valid surface(s); see /api/contract`); + if (report.errors.length) console.error(`manifest has ${report.errors.length} error(s) — serving ${report.surfaces} valid surface(s); see ${CONTRACT_VERB}`); }); diff --git a/app/verbs.mjs b/app/verbs.mjs index a67658d..cb80a4a 100644 --- a/app/verbs.mjs +++ b/app/verbs.mjs @@ -77,22 +77,63 @@ export const verbMatches = (declared, pathname) => export const isRuntimeOwned = (pathname) => RUNTIME_OWNED_VERBS.some((v) => verbMatches(v, pathname)); +// ---- the routing surface tripwire ------------------------------------------ +// +// PM's ruling, third round on this class: OPT-IN DISCOVERY IS THE HOLE. A +// registration wrapper (`serves()`) is a convention — a raw handler that never +// calls it serves live while `runtime.routes` and the classifier stay blind. +// The mechanism is now a TABLE the dispatcher consults at request time, and the +// same table is what `runtime.routes` and classification enumerate: one +// structure, two readers, so a route cannot serve without a row. +// +// What a table cannot prevent is a raw arm typed ABOVE the dispatch — JavaScript +// is open text. That is what this tripwire is for: every textual `/api` mention +// in the runtime source must live between the two markers that fence the table +// and its dispatcher. A raw arm in ANY spelling — exact, prefix, regex — carries +// the `/api` literal it routes on, so it trips this in all forms, and the fix it +// forces is "add a table row", which is the mechanism. Over-blocking is safe; +// silence is not. +// +// (Its predecessor here was `discoverApiRoutes`, a regex over source recognising +// specific comparison spellings — an approximation of the router, exactly what +// PM ruled against, and each new spelling was invisible until added. A negative +// tripwire has no per-spelling blind spot: it needs no grammar of arms, only the +// literal every arm must carry.) + +export const ROUTING_SURFACE_BEGIN = ">>> API ROUTING SURFACE"; +export const ROUTING_SURFACE_END = "<<< API ROUTING SURFACE"; + +// Cut a trailing `//` comment, respecting string quotes so `"http://…"` and +// route literals inside strings survive to be inspected. +const stripLineComment = (line) => { + let q = null; + for (let i = 0; i < line.length; i++) { + const ch = line[i]; + if (q) { if (ch === "\\") i++; else if (ch === q) q = null; continue; } + if (ch === '"' || ch === "'" || ch === "`") { q = ch; continue; } + if (ch === "/" && line[i + 1] === "/") return line.slice(0, i); + } + return line; +}; + /** - * Every `/api/` route a runtime source actually serves, in EVERY form it can be - * written. - * - * **DISCOVERY IS THE STAGE THAT MATTERS AND IT WAS THE STAGE NOT UNDER TEST.** The - * scanner recognised only `u.pathname === "/api/x"`. A live route written - * `u.pathname.startsWith("/api/x/")` was INVISIBLE: it answered 200 while the - * classification suite passed, because a route the scanner cannot see is a route - * the classifier is never asked about. - * - * Shared with the test rather than written there, so the thing under test and the - * thing that runs are the same function. + * Violations of "every `/api` mention lives inside the routing surface". + * Returns `[]` for a conforming source. Missing markers are themselves a + * violation — an absent fence must fail loud, not pass vacuous. */ -export function discoverApiRoutes(src) { - const exact = [...src.matchAll(/u\.pathname === "(\/api\/[^"]+)"/g)].map((m) => m[1]); - const prefix = [...src.matchAll(/u\.pathname\.startsWith\("(\/api\/[^"]+)"\)/g)].map((m) => m[1]); - // `/api/` itself is the catch-all 404 arm, not a served verb. - return [...new Set([...exact, ...prefix])].filter((r) => r !== "/api/"); +export function routingSurfaceViolations(src) { + const lines = src.split("\n"); + const begin = lines.findIndex((l) => l.includes(ROUTING_SURFACE_BEGIN)); + const end = lines.findIndex((l) => l.includes(ROUTING_SURFACE_END)); + if (begin === -1 || end === -1 || end < begin) { + return [{ line: 0, text: null, reason: "routing-surface-markers-missing" }]; + } + const out = []; + lines.forEach((raw, i) => { + if (i >= begin && i <= end) return; + if (stripLineComment(raw).includes("/api")) { + out.push({ line: i + 1, text: raw.trim(), reason: "api-reference-outside-routing-surface" }); + } + }); + return out; } diff --git a/contract/contract-meta.md b/contract/contract-meta.md index 4c9fd5d..cb4d611 100644 --- a/contract/contract-meta.md +++ b/contract/contract-meta.md @@ -106,12 +106,17 @@ Two schemas sit beside them: `surface-row.schema.json` for a manifest row, and eventually state the wrong one, and a wrong version is worse than an absent one precisely because it is stated to be acted on. -- `runtime.routes` — the `/api` verbs this runtime **actually serves**, observed - from its own router rather than declared beside it. Informational, and it exists - so a consumer (or a test) can ask what is served instead of inferring it: a list - maintained separately from the routing table drifts from it, and the drift is - silent in the direction that matters — a verb that exists and is unclassified is - routed away from the runtime that implements it. +- `runtime.routes` — the `/api` verbs this runtime **actually serves**: the + dispatch table itself, enumerated. The same rows request dispatch consults are + what this field reports, so a route cannot serve without appearing here — the + claim holds by construction, not by registration. (An earlier design had arms + *register* themselves on match; a handler that skipped registration served live + while this field stayed silent, which made this exact sentence false.) + Informational, and it exists so a consumer (or a test) can ask what is served + instead of inferring it: a list maintained separately from the routing table + drifts from it, and the drift is silent in the direction that matters — a verb + that exists and is unclassified is routed away from the runtime that + implements it. - `runtime.boot` — an opaque id, **stable for the life of this process and different after a restart**. It is how a surface knows its own CODE changed: a studio copies diff --git a/test/capture-target.test.mjs b/test/capture-target.test.mjs index 60109ad..cd81acd 100644 --- a/test/capture-target.test.mjs +++ b/test/capture-target.test.mjs @@ -266,6 +266,57 @@ test("two bindings resolving to the SAME location are ONE location, not ambiguit assert.equal(fs.realpathSync(r.body.target.path), fs.realpathSync(path.join(one, "proof"))); }); +test("a MISSING target below an existing regular FILE is refused — the ancestor decides", async (t) => { + // QA finding, one level below the last one, and the same class: the fix checked + // whether the TARGET was a directory and never whether the path could BECOME + // one. A missing target whose deepest existing ancestor is a regular file was + // accepted at declaration; the consumer's `mkdir recursive` then failed ENOTDIR. + // The property is performability, not the final component: EVERY existing + // component of the resolved path must be a directory. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "ancfile-")); + fs.mkdirSync(path.join(dir, "work"), { recursive: true }); + fs.writeFileSync(path.join(dir, "work", "file-parent"), "a regular file"); + const s = await start({ roots: { work: path.join(dir, "work") } }); + t.after(() => s.stop()); + + for (const p of ["file-parent/child", "file-parent/a/b/c"]) { + const r = await s.declare({ root: "work", path: p }); + assert.equal(r.status, 400, `'${p}' was accepted — mkdir recursive under a regular file is ENOTDIR`); + assert.match(r.body.error, /not a directory/, `'${p}' refused without naming the blocking file`); + } + assert.equal((await s.read()).target, null, "a refused target was stored anyway"); + + // positive control: the same DEPTH under a real directory still resolves, + // so this is the ancestor check refusing, not depth. + fs.mkdirSync(path.join(dir, "work", "dir-parent"), { recursive: true }); + assert.equal((await s.declare({ root: "work", path: "dir-parent/a/b/c" })).status, 200, + "the ancestor check refuses paths under real directories too"); +}); + +test("multi-location: a file-blocked binding loses to a performable one — and blocked everywhere refuses", async (t) => { + // The class fix must compose with existence-decides: a binding whose ancestor is + // a regular file is unperformable and must not win the create-it-later fallback + // just by being FIRST — that is accepted-but-unperformable through the side door. + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "ancmulti-")); + const first = path.join(dir, "first"), second = path.join(dir, "second"); + fs.mkdirSync(first, { recursive: true }); fs.mkdirSync(second, { recursive: true }); + fs.writeFileSync(path.join(first, "proof"), "a file where a directory is needed"); + + const s = await start({ roots: { feedback: [first, second] } }); + t.after(() => s.stop()); + const r = await s.declare({ root: "feedback", path: "proof/shot" }); + assert.equal(r.status, 200, `the performable second binding lost to a file-blocked first: ${r.body.error}`); + assert.ok(r.body.target.path.startsWith(fs.realpathSync(second) + path.sep), + "resolved under the file-blocked binding"); + + // blocked under EVERY binding: refuse, never accept the unperformable. + fs.writeFileSync(path.join(second, "blocked"), "also a file"); + fs.writeFileSync(path.join(first, "blocked"), "also a file"); + const r2 = await s.declare({ root: "feedback", path: "blocked/shot" }); + assert.equal(r2.status, 400, "blocked under every binding was still accepted"); + assert.match(r2.body.error, /not a directory/); +}); + test("a SYMLINKED duplicate is also one location", async (t) => { // The same property through a different spelling: two paths that realpath to one // place are one place. Deduping by the RESOLVED target rather than the declared diff --git a/test/reserved-verbs.test.mjs b/test/reserved-verbs.test.mjs index fea6eb2..540568c 100644 --- a/test/reserved-verbs.test.mjs +++ b/test/reserved-verbs.test.mjs @@ -29,7 +29,7 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { spawn } from "node:child_process"; import { RUNTIME_OWNED_VERBS, SUBSTITUTABLE_VERBS, openVocabMap, lookup, - discoverApiRoutes, verbMatches, isRuntimeOwned } from "../app/verbs.mjs"; + routingSurfaceViolations, verbMatches, isRuntimeOwned } from "../app/verbs.mjs"; const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const read = (...p) => fs.readFileSync(path.join(REPO, ...p), "utf8"); @@ -132,31 +132,13 @@ test("the safe primitive actually resists prototype keys", () => { } }); -test("CONTROL: a LIVE unclassified route of EACH served form is caught", async () => { - // ⛔ THE CONTROL THAT USED TO BE HERE FIRED ON THE WRONG STAGE OF THE PIPELINE. - // It appended a string to the SCANNER'S RESULT, proving the set subtraction could - // fail and proving nothing about whether discovery sees the forms the runtime - // serves. sdk-qa planted a live `startsWith("/api/qa-added-prefix/")` route: it - // answered 200 while the suite passed 10/10, because a route the scanner cannot - // see is a route the classifier is never asked about. - // - // So this plants BOTH FORMS into a real runtime, BOOTS it, and asserts the - // runtime's own reported route set contains them and the classifier rejects them. - // Nothing here is scanned; the routes are live and answering. - const src = read("app", "serve-studio.mjs"); - const marker = 'if (serves(u.pathname, "/api/contract")) {'; - assert.ok(src.includes(marker), "the contract arm moved — re-anchor this control before trusting it"); - const planted = src.replace(marker, - 'if (serves(u.pathname, "/api/qa-exact-probe")) return sendJson(res, 200, { ok: true });\n' + - ' if (serves(u.pathname, "/api/qa-prefix-probe/")) return sendJson(res, 200, { ok: true });\n' + - ' ' + marker); - assert.notEqual(planted, src, "the plant did not apply — every result below would be meaningless"); - +// Boot a mutated copy of the runtime and probe it. Shared by the two controls +// below, which differ only in WHAT they plant. +async function bootMutated(plantedSrc, probes) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "routes-ctl-")); fs.cpSync(path.join(REPO, "app"), path.join(dir, "app"), { recursive: true }); fs.cpSync(path.join(REPO, "fixtures"), path.join(dir, "fixtures"), { recursive: true }); - fs.writeFileSync(path.join(dir, "app", "serve-studio.mjs"), planted); - + fs.writeFileSync(path.join(dir, "app", "serve-studio.mjs"), plantedSrc); const port = 9960 + Math.floor(Math.random() * 30); const proc = spawn(process.execPath, [path.join(dir, "app", "serve-studio.mjs"), "--port", String(port)], { cwd: dir, stdio: ["ignore", "pipe", "pipe"] }); @@ -165,19 +147,89 @@ test("CONTROL: a LIVE unclassified route of EACH served form is caught", async ( try { await fetch(`http://127.0.0.1:${port}/api/contract`, { cache: "no-store" }); break; } catch { await new Promise((r) => setTimeout(r, 50)); } } - // VERIFY THE PLANT IS LIVE, not merely present in the file. A planted route - // that does not answer would make the rest of this control vacuous. - const exact = await fetch(`http://127.0.0.1:${port}/api/qa-exact-probe`, { cache: "no-store" }); - const prefix = await fetch(`http://127.0.0.1:${port}/api/qa-prefix-probe/control`, { cache: "no-store" }); - assert.equal(exact.status, 200, "the planted EXACT route does not answer — the control proves nothing"); - assert.equal(prefix.status, 200, "the planted PREFIX route does not answer — this is sdk-qa's exact case"); - + const statuses = {}; + for (const p of probes) statuses[p] = (await fetch(`http://127.0.0.1:${port}${p}`, { cache: "no-store" })).status; const c = await (await fetch(`http://127.0.0.1:${port}/api/contract`, { cache: "no-store" })).json(); - const classified = new Set([...RUNTIME_OWNED_VERBS, ...SUBSTITUTABLE_VERBS]); - const unclassified = c.runtime.routes.filter((v) => !classified.has(v)); - assert.deepEqual(unclassified.sort(), ["/api/qa-exact-probe", "/api/qa-prefix-probe/"], - "a LIVE unclassified route was not reported by the runtime — the guard cannot see the form it most needs to catch"); + return { statuses, routes: c.runtime.routes }; } finally { proc.kill(); fs.rmSync(dir, { recursive: true, force: true }); } +} + +test("CONTROL: a LIVE unclassified TABLE ROW of each form is served, reported, and caught", async () => { + // The positive half of the mechanism: a route added the way routes are now + // added — a table row — must be live, must appear in the runtime's own account, + // and must fail classification if unclassified. Planted into a real runtime and + // BOOTED; nothing here is scanned. + const src = read("app", "serve-studio.mjs"); + const marker = "const API_ROUTES = ["; + assert.ok(src.includes(marker), "the route table moved — re-anchor this control before trusting it"); + const planted = src.replace(marker, marker + + '\n { verb: "/api/qa-exact-probe", handler: (u, req, res) => sendJson(res, 200, { ok: true }) },' + + '\n { verb: "/api/qa-prefix-probe/", handler: (u, req, res) => sendJson(res, 200, { ok: true }) },'); + assert.notEqual(planted, src, "the plant did not apply — every result below would be meaningless"); + + const { statuses, routes } = await bootMutated(planted, + ["/api/qa-exact-probe", "/api/qa-prefix-probe/control"]); + assert.equal(statuses["/api/qa-exact-probe"], 200, "the planted EXACT row does not answer — the control proves nothing"); + assert.equal(statuses["/api/qa-prefix-probe/control"], 200, "the planted PREFIX row does not answer"); + + const classified = new Set([...RUNTIME_OWNED_VERBS, ...SUBSTITUTABLE_VERBS]); + const unclassified = routes.filter((v) => !classified.has(v)); + assert.deepEqual(unclassified.sort(), ["/api/qa-exact-probe", "/api/qa-prefix-probe/"], + "a LIVE unclassified table row was not reported by the runtime — enumeration is not reading the serving table"); +}); + +test("BYPASS CONTROL: a raw handler that does not go through the table cannot land silently", async () => { + // ⛔ THE DISCRIMINATING CONTROL, and the one that has now failed twice from the + // other side: sdk-qa planted live raw handlers — no registration call, no table + // row — in exact, prefix, and parameterized form. All three served 200, all + // three were absent from runtime.routes, and the suite passed. An opt-in + // mechanism is a convention, not a mechanism. + // + // Under the table design a raw arm can only serve from ABOVE the dispatch, and + // there are exactly two honest outcomes, asserted here per orch's ruling: + // - it cannot serve (dispatch only walks the table): the probe must 404; or + // - it can still serve: then it must be REPORTED, or the structural tripwire + // must NAME the planted line — otherwise the table is not the only door and + // this test fails, which is the correct verdict on that design. + const src = read("app", "serve-studio.mjs"); + const anchor = /const u = new URL\(req\.url[^\n]*\n/; + assert.match(src, anchor, "handleRequest's entry moved — re-anchor this control before trusting it"); + + const forms = [ + ["exact", ' if (u.pathname === "/api/qa-raw-exact") return sendJson(res, 200, { ok: true });\n', + "/api/qa-raw-exact", "/api/qa-raw-exact"], + ["prefix", ' if (u.pathname.startsWith("/api/qa-raw-prefix/")) return sendJson(res, 200, { ok: true });\n', + "/api/qa-raw-prefix/x", "/api/qa-raw-prefix"], + ["parameterized", ' { const qm = u.pathname.match(/^\\/api\\/qa-raw-param\\/([^/]+)$/); if (qm) return sendJson(res, 200, { ok: true, id: qm[1] }); }\n', + "/api/qa-raw-param/42", "qa-raw-param"], + ]; + for (const [form, arm, probe, needle] of forms) { + const planted = src.replace(anchor, (m) => m + arm); + assert.notEqual(planted, src, `${form}: the plant did not apply`); + const { statuses, routes } = await bootMutated(planted, [probe]); + + if (statuses[probe] === 200) { + const reported = routes.some((v) => v === probe || (v.endsWith("/") && probe.startsWith(v))); + const named = routingSurfaceViolations(planted).some((v) => (v.text || "").includes(needle)); + assert.ok(reported || named, + `${form}: a raw handler SERVES (200), is NOT in runtime.routes, and the routing-surface ` + + `tripwire does not name it — a live route invisible to classification, the original product failure`); + } else { + assert.equal(statuses[probe], 404, + `${form}: a raw handler neither served nor 404'd — some residual path half-handled it`); + } + } +}); + +test("the runtime source keeps every /api mention inside the routing surface", () => { + // The standing half of the bypass control: the tripwire that makes a raw arm + // unable to LAND in the repo. The control above proves this fires on a live + // bypass; this proves the real source is clean, so a violation is always a + // regression and never ambient noise. + const violations = routingSurfaceViolations(read("app", "serve-studio.mjs")); + assert.deepEqual(violations, [], + "an /api reference lives outside the fenced routing surface — routes are added as TABLE ROWS, " + + "never as raw arms; a raw arm serves without being enumerated or classified"); }); test("a PREFIX verb is matched as a prefix, and an exact verb is not", () => { diff --git a/test/runtime-redeploy.test.mjs b/test/runtime-redeploy.test.mjs index 20f9529..c9f148a 100644 --- a/test/runtime-redeploy.test.mjs +++ b/test/runtime-redeploy.test.mjs @@ -502,6 +502,36 @@ test("the SSE change-signal still fires on a manifest change", async (t) => { // top-level awaits, method chains — where the old guard matched two whitelisted // call names and never saw the statement that carried the defect. A checker that // cannot see the statement is a checker nothing runs, for that statement. +// Blank out string contents so braces and identifier-lookalikes inside them +// neither break depth tracking nor register as references. +function stripStrings(text) { + return text + .replace(/`(?:[^`\\]|\\[\s\S])*`/g, "``") + .replace(/"(?:[^"\\]|\\.)*"/g, '""') + .replace(/'(?:[^'\\]|\\.)*'/g, "''"); +} + +// Remove `=> { … }` and `function … { … }` bodies: code inside a function does +// NOT evaluate at module load, so its references belong to call time, not to +// the initializer being scanned. Expects string-stripped input so brace +// counting cannot be derailed by a brace in a message. +function stripFunctionBodies(text) { + let out = "", i = 0; + for (;;) { + const m = /(?:=>\s*\{|function\b[^{]*\{)/.exec(text.slice(i)); + if (!m) { out += text.slice(i); break; } + out += text.slice(i, i + m.index) + "{}"; + let depth = 1, j = i + m.index + m[0].length; + while (j < text.length && depth > 0) { + if (text[j] === "{") depth++; + else if (text[j] === "}") depth--; + j++; + } + i = j; + } + return out; +} + function startupOrderingViolations(src) { const lines = src.split("\n"); @@ -515,12 +545,45 @@ function startupOrderingViolations(src) { for (const name of names) bindings.push({ name, line: i + 1 }); }); - // Module-level executable statements (column 0): anything that RUNS at load - // time rather than declaring. A statement spans lines until one closes at - // column 0 with `;`. - const executables = []; // { line, text } + // Consume a multi-line statement starting at line i: cumulative ()[]{} depth + // over string-stripped lines, ending on the line where depth returns to zero + // and the line closes with `;`. + const statementEnd = (i) => { + let depth = 0; + for (let j = i; j < lines.length; j++) { + const s = stripStrings(lines[j]); + for (const ch of s) { + if ("([{".includes(ch)) depth++; + else if (")]}".includes(ch)) depth--; + } + if (depth <= 0 && /;\s*$/.test(s)) return j; + } + return lines.length - 1; + }; + + // What EXECUTES at module load, in both spellings: + // - executable statements (calls, awaits, chains), and + // - const/let INITIALIZERS — an initializer runs at load exactly like a + // statement, unless the whole initializer is a function expression, whose + // body waits for call time. Missing the second spelling is how + // `const probe = handleRequest;` crashed the runtime while this guard + // passed: it enforced reachability over the wrong SET of statements. + const executables = []; // { line, text, kind: "statement" } + const initializers = []; // { line, text, kind: "initializer" } for (let i = 0; i < lines.length; i++) { const line = lines[i]; + const decl = line.match(/^(?:const|let)\s+[^=]*=\s*(.*)$/); + if (decl) { + const initStart = decl[1]; + // A pure function expression: nothing but the closure evaluates at load. + if (/^(?:async\s+)?(?:\([^)]*\)|[A-Za-z_$][\w$]*)\s*=>/.test(initStart) || + /^(?:async\s+)?function\b/.test(initStart)) continue; + const end = statementEnd(i); + const text = lines.slice(i, end + 1).join("\n"); + initializers.push({ line: i + 1, text: stripFunctionBodies(stripStrings(text)) }); + i = end; + continue; + } if (!/^(?:await\s+|[A-Za-z_$][\w$]*\s*[.(])/.test(line)) continue; if (/^(?:import|export|const|let|var|function|class|async\s+function)\b/.test(line)) continue; let text = line; @@ -531,17 +594,19 @@ function startupOrderingViolations(src) { } const violations = []; - for (const ex of executables) { + for (const ex of [...executables, ...initializers]) { + const what = ex === undefined ? "" : (executables.includes(ex) ? "statement" : "initializer"); const referenced = new Set(ex.text.match(/[A-Za-z_$][\w$]*/g) || []); for (const b of bindings) { if (b.line > ex.line && referenced.has(b.name)) { violations.push({ kind: "reachability", statement: ex.line, binding: b.name, declared: b.line, message: - `the statement at line ${ex.line} references '${b.name}', declared at line ${b.line} — ` + - `that is a ReferenceError in the temporal dead zone the moment the module loads. ` + - `Being last is not the same as being after everything you TOUCH: startup must follow ` + - `the bindings it references, so move the STATEMENT below line ${b.line}, not the binding above.`, + `the ${what} at line ${ex.line} references '${b.name}', declared at line ${b.line} — ` + + `an ${what === "initializer" ? "initializer executes at module load exactly like a statement, so this" : "executable statement"} ` + + `is a ReferenceError in the temporal dead zone the moment the module loads. ` + + `Being last is not the same as being after everything you TOUCH: load-time code must follow ` + + `the bindings it references, so move it below line ${b.line}, not the binding above.`, }); } } @@ -581,23 +646,31 @@ test("startup follows every binding it touches AND every binding, so the TDZ cla "startup ordering violations in app/serve-studio.mjs"); }); -test("positive control: the ordering checker fires on a statement in a later binding's dead zone", () => { - // The defect this guard exists for, planted verbatim: startup work sitting - // "at the bottom" of an init block while referencing a const declared beneath - // it. A checker that has never been seen to fire is not a control. +test("positive control: the ordering checker fires on BOTH load-time spellings of the dead zone", () => { + // The two defects this guard exists for, planted verbatim: the historical + // top-level statement referencing a const declared beneath it, and sdk-qa's + // `const probe = handleRequest;` — an INITIALIZER, which executes at module + // load exactly like a statement and which the previous scan excluded + // wholesale. A checker that has never been seen to fire is not a control. const planted = [ "const early = 1;", "doStartup();", "await handleThing(", " { url: x },", ").catch(() => {});", + "const qaInitializerProbe = handleThing;", + "const notALoad = () => handleThing;", "const handleThing = async () => {};", "listenSomething();", ].join("\n"); const { violations } = startupOrderingViolations(planted); const reach = violations.filter((v) => v.kind === "reachability"); - assert.equal(reach.length, 1, `expected exactly the planted TDZ, got: ${JSON.stringify(violations)}`); - assert.equal(reach[0].binding, "handleThing"); + assert.deepEqual(reach.map((v) => [v.statement, v.binding]).sort((a, b) => a[0] - b[0]), + [[3, "handleThing"], [6, "handleThing"]], + `expected the STATEMENT plant and the INITIALIZER plant, exactly: ${JSON.stringify(reach)}`); + assert.ok(!reach.some((v) => v.statement === 7), + "a function-expression initializer was flagged — its body runs at call time, not load time, " + + "and over-firing there would force every helper above the binding it eventually calls"); assert.ok(violations.some((v) => v.kind === "position"), "the position property must also fire on statements before the last binding"); });