mirror of
https://github.com/mvschwarz/openrig-studio.git
synced 2026-11-07 05:56:21 -10:00
Boot again, and make the ordering guard name the property that actually failed
The runtime died at load: the boot-time route-chain walk sat in the init block
above `const handleRequest` and called it in its temporal dead zone — the fifth
TDZ from this file, created while satisfying the position guard that exists to
prevent it. The guard asserted POSITION ("startup after every module binding")
and matched only two whitelisted call names, so it never saw the statement that
carried the defect, and being "at the bottom" of the known bindings read as
compliance while being before the one binding the statement touches.
The arrangement now: ALL module-level executable statements — init calls, the
two intervals that sat mid-file, the walk, .listen — live in one init block
after the last declaration, and the walk runs after `handleRequest` (which it
calls) and before .listen so the first request sees the complete route set.
The guard now checks two properties and can be seen to fire:
- REACHABILITY: no module-level statement references a binding declared later.
This is the property whose violation IS the boot ReferenceError; its failure
message names the binding and says that being last is not the same as being
after everything you TOUCH. Run against the broken tree it reports exactly
statement 1051 → handleRequest@1057.
- POSITION: every executable statement follows every binding — kept because it
also covers what startup's callees may GROW to touch, which no reference scan
of the call site can see.
- It recognises every top-level executable statement (bare calls, awaits,
chains), not a name whitelist, and the analysis is a function a positive
control runs against a planted violation.
Also: contract-response-parity's namespace-advertisement check keyed on a regex
over source (`u.pathname === "..."`) that found 0 routes once the arms went
through serves(). It now reads `runtime.routes` from /api/contract — the
router itself, the same one source PM ruled classification must use.
343 -> 344 tests, 344/344 green.
This commit is contained in:
1 parent
4d231186e6
commit
93b561204b
3 files changed
+162
-62
No files matched your search
+37
-35
@@ -723,10 +723,6 @@ function integrityCheck() {
|
||||
}
|
||||
}
|
||||
|
||||
setInterval(integrityCheck, 1000).unref?.();
|
||||
|
||||
setInterval(() => { for (const res of sseClients) { try { res.write(`: heartbeat\n\n`); } catch {} } }, 25000).unref();
|
||||
|
||||
// ---- focus: what the user is looking at ------------------------------------
|
||||
// Held in memory, not on disk, and that is the point rather than a shortcut: the
|
||||
// measured defect is a write verb with no matching read, which forces every
|
||||
@@ -1023,37 +1019,6 @@ function filesSearch(q) {
|
||||
}
|
||||
|
||||
// ---- HTTP ------------------------------------------------------------------
|
||||
// ---- init: everything above is declarations -------------------------------
|
||||
// Ordering here is a correctness property, not a style preference. This block
|
||||
// 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 functions
|
||||
// grow to touch later.
|
||||
//
|
||||
// The weaker arrangement — init partway down, "safe" because the current call
|
||||
// graph happens not to reach the later bindings — is a temporal dead zone
|
||||
// waiting for the next edit. Hoisting the one binding that trips only moves the
|
||||
// tripwire; the position is what makes it safe. Add new startup work HERE, not
|
||||
// next to the function it calls.
|
||||
watchForChanges();
|
||||
loadManifest();
|
||||
// Marks written by a previous run of this file-backed runtime. Omitting this call
|
||||
// is not a visible failure: writing works, the board looks healthy, and the loss
|
||||
// only appears on the next restart. A test restarts the runtime rather than
|
||||
// reading back within one process, because a write and a read in the same process
|
||||
// are both served by the in-memory map and agree with each other whether or not
|
||||
// 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.
|
||||
await handleRequest(
|
||||
{ url: ENUMERATE_PATH, method: "GET", headers: {}, on() {}, once() {}, removeListener() {} },
|
||||
{ writeHead() {}, end() {}, write() {}, setHeader() {}, on() {}, once() {} },
|
||||
).catch(() => {});
|
||||
|
||||
|
||||
const handleRequest = async (req, res) => {
|
||||
const u = new URL(req.url, `http://127.0.0.1:${PORT}`);
|
||||
try {
|
||||
@@ -1326,6 +1291,43 @@ const handleRequest = async (req, res) => {
|
||||
res.end(fs.readFileSync(file));
|
||||
};
|
||||
|
||||
// ---- init: everything above is declarations -------------------------------
|
||||
// Ordering here is a correctness property, not a style preference. This block
|
||||
// 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.
|
||||
//
|
||||
// The weaker arrangement — init partway down, "safe" because the current call
|
||||
// graph happens not to reach the later bindings — is a temporal dead zone
|
||||
// waiting for the next edit. Hoisting the one binding that trips only moves the
|
||||
// tripwire; the position is what makes it safe. Add new startup work HERE, not
|
||||
// next to the function it calls.
|
||||
watchForChanges();
|
||||
loadManifest();
|
||||
setInterval(integrityCheck, 1000).unref?.();
|
||||
setInterval(() => { for (const res of sseClients) { try { res.write(`: heartbeat\n\n`); } catch {} } }, 25000).unref();
|
||||
// Marks written by a previous run of this file-backed runtime. Omitting this call
|
||||
// is not a visible failure: writing works, the board looks healthy, and the loss
|
||||
// only appears on the next restart. A test restarts the runtime rather than
|
||||
// reading back within one process, because a write and a read in the same process
|
||||
// are both served by the in-memory map and agree with each other whether or not
|
||||
// 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(() => {});
|
||||
|
||||
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}`);
|
||||
|
||||
@@ -175,15 +175,17 @@ test("every namespace with a served verb is advertised", async (t) => {
|
||||
// doc and the runtime in step with each other; both could still agree while
|
||||
// omitting a verb the runtime answers.
|
||||
//
|
||||
// Keyed on the ROUTES THE RUNTIME ACTUALLY HANDLES, read out of its source, so a
|
||||
// verb added without a capability fails here rather than being found by whoever
|
||||
// tries to use it.
|
||||
// Keyed on the ROUTES THE RUNTIME ACTUALLY HANDLES, read from its own routing
|
||||
// table (`runtime.routes` at /api/contract), so a verb added without a
|
||||
// capability fails here rather than being found by whoever tries to use it.
|
||||
// This used to be a regex over source recognising only `u.pathname === "..."`;
|
||||
// it broke the moment the arms were rewritten to `serves()` — a regex is an
|
||||
// approximation of the router, and this is the router.
|
||||
const s = await start(false);
|
||||
t.after(() => s.stop());
|
||||
|
||||
const src = fs.readFileSync(path.join(REPO, "app", "serve-studio.mjs"), "utf8");
|
||||
const routes = [...src.matchAll(/u\.pathname === "(\/api\/[a-z/-]+)"/g)].map((m) => m[1]);
|
||||
assert.ok(routes.length >= 4, `positive control: parsed ${routes.length} routes from the runtime`);
|
||||
const routes = s.response.runtime.routes.map((r) => r.replace(/\/$/, ""));
|
||||
assert.ok(routes.length >= 4, `positive control: the runtime reports ${routes.length} routes`);
|
||||
|
||||
// namespace -> the capability prefix that advertises it
|
||||
const NAMESPACE = { "/api/contract": "contract", "/api/factory/state": "observe",
|
||||
|
||||
+117
-21
@@ -480,30 +480,126 @@ test("the SSE change-signal still fires on a manifest change", async (t) => {
|
||||
// fourth, the comparison became structural and descends everywhere. Its controls
|
||||
// plant a field at all four levels and assert the path each is reported at.
|
||||
|
||||
test("startup runs after EVERY module binding, so the TDZ class cannot return", () => {
|
||||
// The startup-ordering analysis, extracted so a control can run it against a
|
||||
// PLANTED violation. The previous guard asserted POSITION only — "startup after
|
||||
// every module binding" — and it invited exactly the fix that reproduces the
|
||||
// defect it exists to prevent: startup work was moved "to the bottom" of the
|
||||
// init block, below every binding the mover knew about, and directly into the
|
||||
// temporal dead zone of a `const` declared beneath it. Being last is not the
|
||||
// same as being after everything you TOUCH. So this checks two properties:
|
||||
//
|
||||
// REACHABILITY — no module-level executable statement references a `let`/
|
||||
// `const` binding declared later. This is the property whose violation IS the
|
||||
// boot-time ReferenceError, checked against what each statement actually
|
||||
// touches rather than against line order.
|
||||
//
|
||||
// POSITION — every executable statement still follows every binding. This is
|
||||
// deliberately broader than reachability: it also covers what the functions
|
||||
// startup CALLS may grow to touch later, which no textual reference scan of
|
||||
// the call site can see.
|
||||
//
|
||||
// It also recognises EVERY module-level executable statement — bare calls,
|
||||
// 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.
|
||||
function startupOrderingViolations(src) {
|
||||
const lines = src.split("\n");
|
||||
|
||||
// Module-level bindings (column 0), including simple destructuring.
|
||||
const bindings = []; // { name, line }
|
||||
lines.forEach((line, i) => {
|
||||
const m = line.match(/^(?:const|let)\s+(?:\{([^}]*)\}|\[([^\]]*)\]|([A-Za-z_$][\w$]*))/);
|
||||
if (!m) return;
|
||||
const names = m[3] ? [m[3]]
|
||||
: (m[1] || m[2]).split(",").map((s) => s.split(/[:=]/)[0].trim()).filter(Boolean);
|
||||
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 }
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
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;
|
||||
let j = i;
|
||||
while (j < lines.length - 1 && !/;\s*$/.test(lines[j]) ) { j++; text += "\n" + lines[j]; }
|
||||
executables.push({ line: i + 1, text });
|
||||
i = j;
|
||||
}
|
||||
|
||||
const violations = [];
|
||||
for (const ex of executables) {
|
||||
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.`,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const lastBinding = Math.max(0, ...bindings.map((b) => b.line));
|
||||
for (const ex of executables) {
|
||||
if (ex.line < lastBinding) {
|
||||
violations.push({
|
||||
kind: "position", statement: ex.line, declared: lastBinding,
|
||||
message:
|
||||
`the statement at line ${ex.line} runs before the last module binding (line ${lastBinding}). ` +
|
||||
`Even if it touches nothing declared later TODAY, the functions it calls can grow a ` +
|
||||
`dependency on a later binding and land in its dead zone — position is the property that ` +
|
||||
`covers what your callees may come to touch. Put startup work in the init block at the ` +
|
||||
`bottom, after every declaration AND after everything it touches.`,
|
||||
});
|
||||
}
|
||||
}
|
||||
return { bindings, executables, violations };
|
||||
}
|
||||
|
||||
test("startup follows every binding it touches AND every binding, so the TDZ class cannot return", () => {
|
||||
// This asserts the STRUCTURE the source comment claims, rather than trusting
|
||||
// the comment. The earlier arrangement put init partway down the module and
|
||||
// was "safe" only because the current call graph happened not to reach the
|
||||
// later bindings — a property that silently expires the moment loadManifest
|
||||
// or watchForChanges grows a new dependency. Hoisting whichever binding
|
||||
// trips only moves the tripwire, so the position is asserted rather than
|
||||
// maintained by care.
|
||||
const src = fs.readFileSync(path.join(REPO, "app", "serve-studio.mjs"), "utf8").split("\n");
|
||||
// the comment. Reachability is checked as well as position because the two
|
||||
// properties fail differently: position says "not at the bottom", reachability
|
||||
// names the exact binding whose dead zone the statement would die in — which
|
||||
// is the message that would have prevented the fifth TDZ from this file.
|
||||
const src = fs.readFileSync(path.join(REPO, "app", "serve-studio.mjs"), "utf8");
|
||||
const { bindings, executables, violations } = startupOrderingViolations(src);
|
||||
|
||||
const declarations = src
|
||||
.map((line, i) => (/^(let|const)\s/.test(line) ? i + 1 : null))
|
||||
.filter(Boolean);
|
||||
const startup = src
|
||||
.map((line, i) => (/^(watchForChanges|loadManifest)\(\);\s*$/.test(line) ? i + 1 : null))
|
||||
.filter(Boolean);
|
||||
assert.ok(bindings.length >= 10, `positive control: found only ${bindings.length} module bindings`);
|
||||
assert.ok(executables.length >= 4,
|
||||
`positive control: found only ${executables.length} module-level executable statements — ` +
|
||||
`expected at least watchForChanges, loadManifest, loadAnnotations, the route walk, and .listen`);
|
||||
assert.deepEqual(violations.map((v) => v.message), [],
|
||||
"startup ordering violations in app/serve-studio.mjs");
|
||||
});
|
||||
|
||||
assert.ok(startup.length >= 2, "expected the startup calls at module level");
|
||||
assert.ok(
|
||||
Math.min(...startup) > Math.max(...declarations),
|
||||
`startup runs at line ${Math.min(...startup)} but a module binding is declared at line ` +
|
||||
`${Math.max(...declarations)} — startup must follow EVERY declaration, not merely the ones ` +
|
||||
`it currently happens to touch`
|
||||
);
|
||||
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.
|
||||
const planted = [
|
||||
"const early = 1;",
|
||||
"doStartup();",
|
||||
"await handleThing(",
|
||||
" { url: x },",
|
||||
").catch(() => {});",
|
||||
"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.ok(violations.some((v) => v.kind === "position"),
|
||||
"the position property must also fire on statements before the last binding");
|
||||
});
|
||||
|
||||
test("positive control: this suite is capable of failing", () => {
|
||||
|
||||
Reference in new issue
Block a user