From dca9aaeecfa543bd31dd6b12ec77d505cd517a2b Mon Sep 17 00:00:00 2001 From: os-litant Date: Wed, 26 Aug 2026 08:02:36 +0000 Subject: [PATCH 1/2] test(cli): the serve host-resolution sweep reports what it cannot resolve An unresolvable specifier carried no package name, so it fell OUT of the judged population rather than into it: the sweep did not report the load as unknowable, it reported nothing and kept passing over the remainder. That is how the @objectstack/organizations load left this sweep during #11614, and teaching the resolver one more spelling only moves the boundary rather than making a crossing audible. An unresolved specifier is now a failure naming the file, the line, the callee and the specifier text, for every callee, unless the site is declared out of the sweep with its reason. Declarations are kept per callee so a bare import() can never inherit a host-anchored load's excuse, and a declaration matching no live site fails too. Three scanner repairs the loud path needed to be true: - `function importFromHost(` was read as a load site, adding a phantom, permanently unresolvable member to the population. - The whole argument list was read as the specifier, so the two-argument `importFromHost(pluginSpecifier, root)` was unresolvable for a reason unrelated to its specifier. - `resolveIdentifier` searched the whole file for the FIRST `const =`, so the parameter `pkg` in `loadOptionalServicePlugin` resolved against a binding 1160 lines below it. The search is now confined to the source above the call and takes the nearest preceding binding. A blank specifier now counts as no specifier, and the sweep's own RED path is exercised over synthetic sources so it is observed on every run. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd --- .../serve-cluster-host-resolution.test.ts | 488 ++++++++++++++++-- 1 file changed, 431 insertions(+), 57 deletions(-) diff --git a/packages/cli/src/commands/serve-cluster-host-resolution.test.ts b/packages/cli/src/commands/serve-cluster-host-resolution.test.ts index 8309ed424c..425385a75a 100644 --- a/packages/cli/src/commands/serve-cluster-host-resolution.test.ts +++ b/packages/cli/src/commands/serve-cluster-host-resolution.test.ts @@ -59,9 +59,21 @@ * pair was only the instance that happened to ship. A package is treated as * app-declarable exactly when `packages/cli`'s own manifest does not declare * it — mechanically, so a newly added optional package is covered without - * anyone remembering this file. Bare `import()` of such a package fails, and - * a bare `import()` whose specifier the scan cannot resolve must be - * enumerated with its reason. + * anyone remembering this file. Bare `import()` of such a package fails. + * + * 4. The DIRECTION of the scan's own ignorance (#12162). A specifier the scan + * cannot resolve carries no package name, so it falls OUT of the judged + * population rather than into it: the sweep does not report the load as + * unknowable, it reports nothing at all and keeps passing over the + * remainder. That is how the `@objectstack/organizations` load left this + * sweep during #11614, and teaching the resolver one more spelling each time + * only moves the boundary — it never makes crossing it audible. + * + * So an unresolved specifier is a FAILURE naming the file, the line, the + * callee and the specifier text, for every callee, unless the site is + * DECLARED out of the sweep with its reason — and a declaration that matches + * no live site fails too. The declarations are kept per callee: a bare + * `import()` may never inherit the reason written for a host-anchored load. * * The source scan reads `serve.ts` and `package.json` from THIS package, so no * cross-package test input is declared or needed. @@ -181,14 +193,45 @@ type LoadSite = { packageName?: string; }; -/** Read the balanced argument text of the call whose `(` is at `open`. */ -function argumentAt(code: string, open: number): string { +/** + * Read the FIRST argument of the call whose `(` is at `open` — the specifier. + * + * Reading the WHOLE argument list is a way to lose a load: the two-argument + * `importFromHost(pluginSpecifier, root)` came out as the argument text + * `pluginSpecifier, root`, which matches no specifier shape, so the site + * classified as unresolvable for a reason that has nothing to do with its + * specifier. Brackets and string/template literals are tracked so a comma inside + * either does not end the argument. + */ +function firstArgumentAt(code: string, open: number): string { let depth = 0; let out = ''; for (let i = open; i < code.length; i++) { const c = code[i]; - if (c === '(') { depth++; if (depth === 1) continue; } - if (c === ')') { depth--; if (depth === 0) break; } + if (c === '(' || c === '[' || c === '{') { + depth++; + if (depth === 1 && c === '(') continue; + out += c; + continue; + } + if (c === ')' || c === ']' || c === '}') { + depth--; + if (depth === 0) break; + out += c; + continue; + } + if (depth === 1 && c === ',') break; // end of the first argument + if (c === '"' || c === "'" || c === '`') { // copy a literal whole + const quote = c; + out += c; + i++; + while (i < code.length && code[i] !== quote) { + if (code[i] === '\\') { out += code[i]; i++; } + if (i < code.length) { out += code[i]; i++; } + } + if (i < code.length) out += code[i]; + continue; + } out += c; } return out.replace(/\s+/g, ' ').trim(); @@ -208,46 +251,81 @@ function packageNameOf(specifier: string): string | undefined { * `tsc` from statically resolving an optional package * (`const i18nPkg = '@objectstack/service-i18n'`) — and one further hop, * `const X = Serve.MEMBER`, where `MEMBER` is a `static readonly` string on the - * command class in this same file. Without this the sweep sees only an - * identifier and classifies the load as unknowable. + * command class in this same file. * * The second hop is not a convenience. #11614 single-sourced the * `@objectstack/organizations` spelling onto `Serve.ORGANIZATIONS_RUNTIME_PKG` - * so the spec-owned provenance roster could pin it, and a resolver that stops - * one hop short turns that load from "app-declarable, host-anchored, checked" - * into "unknowable" — SILENTLY, because an unresolved specifier drops OUT of - * `APP_DECLARABLE_LOADS` rather than into it. The named half of the vacuity - * guard below is what caught that, and is why it names packages instead of only - * counting them. Resolving one hop further strictly WIDENS what the sweep - * judges; it can never excuse a load. + * so the spec-owned provenance roster could pin it, and a resolver that stopped + * one hop short turned that load from "app-declarable, host-anchored, checked" + * into "unknowable". + * + * ── `before`: the resolver may only look BACKWARDS ─────────────────────────── + * + * The search is confined to the source ABOVE the call and takes the NEAREST + * preceding binding, because a `const` only exists below itself — the same + * temporal-dead-zone fact #10769 pinned for `importFromHost`. Unconfined, the + * match is a whole-file FIRST-HIT search for `const =`, and in a + * 5000-line file that is a coin toss: `importFromHost(pkg)` inside + * `loadOptionalServicePlugin`, whose `pkg` is a PARAMETER, resolved against a + * `const pkg = Serve.ORGANIZATIONS_RUNTIME_PKG` in an unrelated string helper + * 1160 lines FURTHER DOWN. The sweep then reported an organizations load that + * does not exist at that line — worse than reporting nothing, because the named + * vacuity list below is satisfied by the phantom. + * + * Resolving may only ever WIDEN what the sweep judges. When it cannot resolve, + * it says so and the site is reported; it never excuses a load. */ -function resolveIdentifier(code: string, name: string): string | undefined { - const direct = code.match( - new RegExp(`\\bconst\\s+${name}\\s*(?::\\s*string\\s*)?=\\s*(['"\`])([^'"\`]*)\\1`), - ); - if (direct) return direct[2]; +function resolveIdentifier(code: string, name: string, before: number): string | undefined { + const region = code.slice(0, before); + const candidates: Array<{ index: number; literal?: string; member?: string }> = []; + + for (const m of region.matchAll( + new RegExp(`\\bconst\\s+${name}\\s*(?::\\s*string\\s*)?=\\s*(['"\`])([^'"\`]*)\\1`, 'g'), + )) { + candidates.push({ index: m.index ?? 0, literal: m[2] }); + } // `const organizationsPkg = Serve.ORGANIZATIONS_RUNTIME_PKG;` (#11614) - const viaStatic = code.match( - new RegExp(`\\bconst\\s+${name}\\s*(?::\\s*string\\s*)?=\\s*Serve\\.([A-Za-z_$][\\w$]*)\\s*;`), - ); - if (!viaStatic) return undefined; + for (const m of region.matchAll( + new RegExp(`\\bconst\\s+${name}\\s*(?::\\s*string\\s*)?=\\s*Serve\\.([A-Za-z_$][\\w$]*)\\s*;`, 'g'), + )) { + candidates.push({ index: m.index ?? 0, member: m[1] }); + } + + candidates.sort((a, b) => a.index - b.index); + const nearest = candidates.at(-1); + if (!nearest) return undefined; + if (nearest.literal !== undefined) return nearest.literal; + // A `static readonly` is a property of the class object rather than a binding, + // so its position relative to the call carries no meaning: this hop reads the + // whole file, and the anchor that keeps it honest is the `Serve.` prefix. const member = code.match( - new RegExp(`\\bstatic\\s+readonly\\s+${viaStatic[1]}\\s*(?::\\s*string\\s*)?=\\s*(['"\`])([^'"\`]*)\\1`), + new RegExp(`\\bstatic\\s+readonly\\s+${nearest.member}\\s*(?::\\s*string\\s*)?=\\s*(['"\`])([^'"\`]*)\\1`), ); return member?.[2]; } -/** Every dynamic load in `serve.ts`, bare or host-anchored. */ +/** + * Every dynamic load in `serve.ts`, bare or host-anchored. + * + * `function importFromHost(` is deliberately NOT a load site: the regex below + * matches the helper's own DECLARATION as readily as a call to it, and that + * phantom site can never have a specifier — so it sat in the population forever + * as a permanently unresolvable entry, inflating the vacuity floors by one and + * guaranteeing at least one member of any "cannot resolve" report is noise. + */ function collectLoadSites(code: string): LoadSite[] { const sites: LoadSite[] = []; const re = /\b(?:await\s+)?(importFromHost|import)\s*\(/g; let m: RegExpExecArray | null; while ((m = re.exec(code))) { + // `function importFromHost(...)` — the definition, not a load. + if (/\bfunction\s+$/.test(code.slice(Math.max(0, m.index - 16), m.index))) continue; + const callee = m[1] as LoadSite['callee']; const open = m.index + m[0].length - 1; - const argument = argumentAt(code, open); + const argument = firstArgumentAt(code, open); const line = code.slice(0, m.index).split('\n').length; let specifier: string | undefined; @@ -258,7 +336,14 @@ function collectLoadSites(code: string): LoadSite[] { if (literal) specifier = literal[2]; else if (plainTemplate) specifier = plainTemplate[1]; else if (prefixTemplate) specifier = prefixTemplate[1]; // `@objectstack/service-cluster-${driver}` - else if (identifier) specifier = resolveIdentifier(code, identifier[1]); + else if (identifier) specifier = resolveIdentifier(code, identifier[1], m.index); + + // A BLANK specifier is not a specifier. `import(`${base}/plugin`)` yields the + // empty prefix, which is falsy everywhere downstream: `packageNameOf` is + // never called, `packageName` is `undefined`, and the site leaves the + // population WITHOUT ever being counted as unresolvable. That is the same + // silent drop this file exists to stop, so it is folded into the loud path. + if (specifier !== undefined && specifier.trim() === '') specifier = undefined; sites.push({ line, @@ -271,6 +356,14 @@ function collectLoadSites(code: string): LoadSite[] { return sites; } +/** How a site is named when the sweep reports it. */ +function formatSite(file: string, site: LoadSite): string { + return `${file}:${site.line} ${site.callee}(${site.argument})`; +} + +/** The swept file, as it is named in a failure. */ +const SERVE_PATH = 'packages/cli/src/commands/serve.ts'; + const LOAD_SITES = collectLoadSites(SERVE_CODE); /** @@ -284,11 +377,59 @@ const APP_DECLARABLE_LOADS = LOAD_SITES.filter( ); /** - * Bare `import()` calls whose specifier no source scan can resolve — a member - * expression or a loop/parameter variable. Each is allowlisted BY ITS ARGUMENT - * TEXT (stable across line moves) with the reason it is not the class above. - * A new one fails the test, which is the point: an unknowable specifier is - * exactly where a bare `import()` hides. + * ── The DECLARED boundary of the sweep ─────────────────────────────────────── + * + * A load whose specifier no source scan can resolve — a parameter, a loop + * variable, a member expression, a computed path. The dangerous thing about + * this set is its DIRECTION: an unresolved specifier has no `packageName`, so it + * falls out of `APP_DECLARABLE_LOADS` rather than into it, and every assertion + * below then passes over a smaller population without a word. Silence is this + * guard's own failure mode, and silence is the mode that does not announce + * itself. + * + * So membership here is DECLARED, never inferred. Each site is listed by its + * argument text (stable across line moves) with the reason it cannot name an + * app-declarable package, and anything unresolved and NOT listed fails the sweep + * naming its file, line, callee and specifier text. A stale entry fails too — an + * exclusion that matches no live site is silence with a reason attached. + * + * ⛔ These are not allowlists in the sense of "make the sweep pass". They are + * the two halves of one question — *what does this specifier name?* — and the + * halves are kept apart ON PURPOSE, keyed per callee. A bare `import()` may + * never borrow a host load's excuse: the excuses are different in kind, and one + * shared table would let `import(pkg)` inherit the reason written for + * `importFromHost(pkg)`, which is exactly the load this file must catch. + * + * ── The census, measured rather than counted ──────────────────────────────── + * + * The premise "nothing in the swept region is legitimately unresolvable" was + * measured against `origin/main` at `3dafd8c9c` and is FALSE. Eight of the 43 + * load sites are unresolvable, and every one of them legitimately so — which is + * why the expression of "fail loudly" is a DECLARED boundary rather than an + * absent one. A bare hard failure would have been eight false positives. + * + * Lines drift; the argument text is the key. This is the census the next reader + * should inherit instead of the number 8: + * + * bare `import()` — the strong excuse, "can only ever name a CLI-declared + * package or a non-package": + * :431 fallbackSpecifier the host importer's own caller base (#11157) + * :754 pluginSpecifier the app's config-plugin, non-package branch (#10908) + * :1653 absolutePath ? … a path to the served artifact, never a package + * :2778 appPkg loops @objectstack/setup + /account, both CLI-declared + * :3346 spec.pkg Serve.CAPABILITY_PROVIDERS, all CLI-declared + * :3411 ex.pkg the same table's `extras`, all CLI-declared + * + * `importFromHost()` — the structural excuse, "host-anchored by construction, + * which is the property this sweep asks for": + * :756 pluginSpecifier the same app config-plugin, taking the host path + * :3217 pkg loadOptionalServicePlugin's PARAMETER. ⚠️ Its + * callers pass '@objectstack/service-ai' and + * '@objectstack/service-ai-studio' as literals one + * frame above, and both are app-declarable — so two + * app-declarable loads sit outside the swept + * population. Declared here on purpose; a scan that + * reads the call cannot read the caller. */ const UNRESOLVABLE_BARE_IMPORTS: Record = { // A filesystem path to the app's own compiled config/artifact, never a package. @@ -328,6 +469,47 @@ const UNRESOLVABLE_BARE_IMPORTS: Record = { "importFromHost's caller base — the CLI's own resolver, handed to createHostImporter (#11157)", }; +/** + * The same declaration, for `importFromHost(...)` sites. + * + * This half did not exist, and its absence is the gap #11614 fell through. An + * unresolvable specifier handed to the HOST importer left the population in + * total silence: the load is host-anchored, so no assertion about bare imports + * had anything to say, and the count floor below absorbed the loss. What was + * lost was the sweep's knowledge that the load exists at all — and a load the + * sweep cannot see is a load nobody notices being rewritten. + * + * Listing these makes the next one RED on the day it appears. Replaying #11614 + * on today's tree — `const organizationsPkg` written as `let organizationsPkg` — + * is reported here by file, line and specifier text instead of vanishing. + */ +const UNRESOLVABLE_HOST_LOADS: Record = { + // `Serve.importConfigPlugin`'s package branch (#10908): the same app-config + // specifier as the bare entry above, taking the host-anchored path. It is a + // runtime value from the served app's own `plugins: [...]`, so no scan can + // know it — and it needs no scan, because reaching it through the host + // importer is precisely what the sweep demands of an app-declarable load. + pluginSpecifier: "the served app's own plugin specifier, host-anchored by construction (#10908)", + // `loadOptionalServicePlugin(pkg, …)`'s parameter. Its callers pass + // '@objectstack/service-ai' and '@objectstack/service-ai-studio' as literals + // AT THE CALL SITE, so the specifier is app-declarable but lives one frame + // above this load — out of reach of a scan that reads the call, not the + // caller. Host-anchored by construction, which is what this sweep asks; the + // packages themselves are therefore outside the swept population, and that is + // an EXCLUSION DECLARED HERE rather than an accident of the resolver. + pkg: 'loadOptionalServicePlugin(pkg) — a generic helper parameter; host-anchored by construction', +}; + +/** Which declaration covers which callee. Deliberately not one shared table. */ +const DECLARED_UNRESOLVABLE: Record> = { + import: UNRESOLVABLE_BARE_IMPORTS, + importFromHost: UNRESOLVABLE_HOST_LOADS, +}; + +/** Every load the scan could not pin to a specifier, whatever the callee. */ +const UNRESOLVED_LOADS = LOAD_SITES.filter((site) => site.specifier === undefined); + + /** * A host app that DECLARES an optional package and carries it in its own * `node_modules` — the shape of every EE app that declares @@ -464,11 +646,42 @@ describe('os serve → every app-declarable optional load is host-anchored', () // read as a clean bill of health. expect(LOAD_SITES.length, 'no dynamic loads found in serve.ts at all').toBeGreaterThan(25); + // ── Does a COUNTING floor still earn its place? (#12162) ──────────────── + // + // Yes — but NOT for the job it used to be given, and it must never again be + // read as the guard of last resort. + // + // The card that asked this is right that a count cannot detect one member + // leaving a population: the >20 floor absorbed the #11614 organizations loss + // without a word. It is no longer asked to. A specifier that stops resolving + // is now REPORTED by name, per site, by the failing check further down. + // + // What is left is resolution to a WRONG but PRESENT value — nothing is + // unresolved, the values simply are not the file's — which per-site + // reporting cannot see. Three ablations measured who catches what, and the + // two halves turn out to be split between two different guards: + // + // • specifier becomes unresolvable (`const x` written as `let x`) + // → the loud check below, naming serve.ts:2841 importFromHost(...). + // The floor here does not fire, and never could. + // • the IDENTIFIER path resolves wrongly (`const =` capture group) + // → the named list below fires; THIS FLOOR STAYS GREEN (31 → 23, + // still over 20). Measured, not assumed. + // • the LITERAL path resolves wrongly (`import('…')` capture group) + // → THIS FLOOR IS THE ONLY ONE THAT FIRES (31 → 9). The named list + // stays green, because all four packages it names reach the sweep + // through the identifier and template paths, not the literal one. + // + // So it covers the literal half of "wrong but present" and the named list + // covers the identifier half. Keep both. ⛔ Read neither as "no load went + // missing" — that is the check below, and only the check below. const resolvedPackages = LOAD_SITES.filter((s) => s.packageName?.startsWith('@objectstack/')); expect( resolvedPackages.length, - 'the specifier resolver stopped resolving — every load now looks unknowable, ' - + 'which would empty the sweep below without failing it', + "the specifier resolver is no longer returning serve.ts's own package strings — " + + 'most likely the LITERAL path, which is the half the named list below cannot ' + + 'see. This floor does NOT guard against a load going missing: an unresolvable ' + + 'specifier is reported by name further down.', ).toBeGreaterThan(20); expect( @@ -477,12 +690,19 @@ describe('os serve → every app-declarable optional load is host-anchored', () ).toBeGreaterThan(20); // Named, not just counted: this proves the resolver still handles every - // spelling serve.ts uses — a `const` binding, a template prefix, a `const` - // bound to a class static, and the manifest cross-check that decides - // app-declarable at all. Naming them is what caught #11614: the - // organizations spelling moved onto a static, the resolver stopped one hop - // short, and that load dropped OUT of the swept population — which the - // count-only floor of >20 absorbed without a word. + // spelling serve.ts uses — a `const` binding, a template prefix, and a + // `const` bound to a class static — plus the manifest cross-check that + // decides app-declarable at all. + // + // ⚠️ Naming them caught #11614, and this list must not be trusted to do it + // again. It is satisfied by ANY site resolving to the package, including one + // that resolves there wrongly: while `resolveIdentifier` searched the whole + // file for `const =`, the parameter `pkg` in `loadOptionalServicePlugin` + // matched a `const pkg = Serve.ORGANIZATIONS_RUNTIME_PKG` 1160 lines below it, + // and that phantom kept '@objectstack/organizations' in this set on a tree + // where the REAL organizations load had already dropped out. A hand-kept list + // of four packages is a smoke alarm, not the fire door; the fire door is the + // failing check below. const found = new Set(APP_DECLARABLE_LOADS.map((s) => s.packageName)); for (const pkg of [ '@objectstack/service-cluster', // const binding (#10645) @@ -526,26 +746,71 @@ describe('os serve → every app-declarable optional load is host-anchored', () ).toEqual([]); }); - it('enumerates every bare import() whose specifier a scan cannot resolve', () => { - const unresolvable = LOAD_SITES.filter( - (site) => site.callee === 'import' && site.specifier === undefined, - ); + it('FAILS on any load whose specifier it cannot resolve — it never drops one', () => { + // ── The direction, which is the whole defect ───────────────────────────── + // + // A specifier this scan cannot resolve carries no `packageName`, so it falls + // OUT of APP_DECLARABLE_LOADS rather than into it: the sweep does not report + // the load as unknowable, it reports nothing at all and keeps asserting over + // the remainder. Widening the resolver one spelling at a time cannot close + // that — it moves the boundary, it does not make crossing it audible. + // + // So: unresolved and undeclared is a FAILURE, for EVERY callee, naming the + // file, the line, the callee and the specifier text. // Non-vacuity: these sites exist, so an empty list means the scan broke. - expect(unresolvable.length).toBeGreaterThan(0); + expect( + UNRESOLVED_LOADS.length, + 'no unresolvable load sites at all — serve.ts has several, so the scan broke', + ).toBeGreaterThan(0); - const unjustified = unresolvable - .filter((site) => !(site.argument in UNRESOLVABLE_BARE_IMPORTS)) - .map((site) => `serve.ts:${site.line} import(${site.argument})`); + const undeclared = UNRESOLVED_LOADS + .filter((site) => !(site.argument in DECLARED_UNRESOLVABLE[site.callee])) + .map((site) => formatSite(SERVE_PATH, site)); expect( - unjustified, - 'A new bare `import()` whose specifier this scan cannot resolve. An unknowable ' - + 'specifier is exactly where an app-declared package hides from the sweep above, ' - + 'so it cannot pass silently. Either load it through `importFromHost(...)` — the ' - + 'right answer whenever the specifier can come from the served app — or add it to ' - + 'UNRESOLVABLE_BARE_IMPORTS with the reason it can only ever name a CLI-declared ' - + 'package or a filesystem path.', + undeclared, + 'A load whose specifier this scan cannot resolve, and which is not declared ' + + 'as out of the sweep. It CANNOT pass silently: an unresolved specifier drops ' + + 'the load out of the judged population, so the sweep would go on reporting ' + + 'green over a set that no longer contains it. That is how the ' + + '@objectstack/organizations load left this sweep during #11614.\n' + + ' • A bare `import()`: load it through `importFromHost(...)` — the right ' + + 'answer whenever the specifier can come from the served app — or add it to ' + + 'UNRESOLVABLE_BARE_IMPORTS with the reason it can only ever name a ' + + 'CLI-declared package or a filesystem path.\n' + + ' • An `importFromHost(...)`: the load is host-anchored, which is what this ' + + 'sweep asks — but the sweep must still KNOW it is here. Give the specifier a ' + + '`const` binding above the call, or add it to UNRESOLVABLE_HOST_LOADS with ' + + 'the reason no scan can name it.\n' + + '⛔ Neither table is a way to make the sweep pass. An entry states, in the ' + + 'open, that a site is outside the swept population; a site that is inside it ' + + 'and merely spelled unusually belongs in the sweep, not in a table.', + ).toEqual([]); + }); + + it('keeps every declared exclusion LIVE (a stale one is silence with a reason)', () => { + // The tables above are the only place this sweep is allowed to be quiet, so + // they are the only place a lie can survive: an entry whose site was deleted, + // renamed, or has since become resolvable goes on excusing an argument text + // that no longer means what the reason says. The next site to be spelled that + // way inherits an excuse written for something else — the silence back, with + // an audit trail pointing the wrong way. + const live = new Set(UNRESOLVED_LOADS.map((site) => `${site.callee} :: ${site.argument}`)); + + const stale: string[] = []; + for (const [callee, table] of Object.entries(DECLARED_UNRESOLVABLE)) { + for (const argument of Object.keys(table)) { + if (!live.has(`${callee} :: ${argument}`)) stale.push(`${callee}(${argument})`); + } + } + + expect( + stale, + 'A declared exclusion matching no live load site in serve.ts. Either the site ' + + 'is gone (delete the entry) or its specifier now resolves (delete the entry — ' + + 'the sweep judges it properly). Keeping it leaves an excuse lying around for ' + + 'whatever is spelled that way next.', ).toEqual([]); }); @@ -566,3 +831,112 @@ describe('os serve → every app-declarable optional load is host-anchored', () } }); }); + +/** + * The sweep's OWN failure path, run rather than assumed. + * + * This file's subject is a guard whose failure mode is silence. A guard whose + * RED path has never been observed is the same silence one layer up, so the + * classifier is exercised here over small synthetic sources: every spelling the + * card listed as "vanishes quietly" is asserted to come back as UNRESOLVED, by + * line and by argument text, on every CI run and not only on the day this was + * written. + */ +describe('os serve → the sweep reports what it cannot resolve (failure path)', () => { + const scan = (src: string) => collectLoadSites(stripComments(src)); + const shape = (src: string) => + scan(src).map((s) => `${s.line} ${s.callee}(${s.argument}) → ${s.specifier ?? 'UNRESOLVED'}`); + + it('reports an unknown spelling instead of dropping the load', () => { + // A `let`, and a member of something that is not `Serve.` — the two + // spellings the resolver does not know. The known load beside it proves the + // scan is working, so UNRESOLVED here means "said so", not "saw nothing". + expect( + shape([ + "const known = '@objectstack/service-i18n';", + 'await importFromHost(known);', + 'let unknown = Other.SOME_MEMBER;', + 'await importFromHost(unknown);', + ].join('\n')), + ).toEqual([ + '2 importFromHost(known) → @objectstack/service-i18n', + '4 importFromHost(unknown) → UNRESOLVED', + ]); + }); + + it('names the file, the line and the specifier text when it reports', () => { + expect(formatSite(SERVE_PATH, scan('await import(mystery);')[0])).toBe( + 'packages/cli/src/commands/serve.ts:1 import(mystery)', + ); + }); + + it('refuses a binding BELOW the call — a const only exists below itself', () => { + expect( + shape([ + 'await importFromHost(later);', + "const later = '@objectstack/organizations';", + ].join('\n')), + ).toEqual(['1 importFromHost(later) → UNRESOLVED']); + }); + + it('takes the NEAREST preceding binding, never a same-named one elsewhere', () => { + // The `pkg` hazard in miniature: two scopes, one name. Reading the file for + // the FIRST `const pkg =` anywhere is how a parameter got resolved against a + // binding 1160 lines away. + expect( + shape([ + '{', + " const dup = '@objectstack/service-cluster';", + '}', + '{', + " const dup = '@objectstack/service-i18n';", + ' await importFromHost(dup);', + '}', + ].join('\n')), + ).toEqual(['6 importFromHost(dup) → @objectstack/service-i18n']); + }); + + it('treats a blank specifier as no specifier', () => { + // `import(`${base}/plugin`)` yields the empty prefix — falsy, so it would + // leave the population without ever counting as unresolvable. + expect(shape('await import(`${base}/plugin`);')).toEqual([ + '1 import(`${base}/plugin`) → UNRESOLVED', + ]); + }); + + it('does not read `function importFromHost(...)` as a load site', () => { + expect( + shape([ + 'function importFromHost(specifier: string, hostRoot: string = servedAppRootOrCwd()) {}', + "const pkg = '@objectstack/organizations';", + 'await importFromHost(pkg);', + ].join('\n')), + ).toEqual(['3 importFromHost(pkg) → @objectstack/organizations']); + }); + + it('classifies a two-argument host load by its FIRST argument', () => { + expect( + shape([ + "const i18nPkg = '@objectstack/service-i18n';", + 'await importFromHost(i18nPkg, root);', + ].join('\n')), + ).toEqual(['2 importFromHost(i18nPkg) → @objectstack/service-i18n']); + }); + + it('does not end the first argument at a comma inside a literal', () => { + expect(shape("await import('@objectstack/a,b');")).toEqual([ + "1 import('@objectstack/a,b') → @objectstack/a,b", + ]); + }); + + it('keeps the declarations per callee: a bare import cannot borrow a host excuse', () => { + // `importFromHost(pkg)` is declared out of the sweep because the callee makes + // it host-anchored. Rewritten as `import(pkg)` that reason evaporates — and + // with one shared table it would inherit the excuse anyway. Two tables is + // what makes the rewrite go red. + expect(Object.keys(UNRESOLVABLE_HOST_LOADS)).toContain('pkg'); + expect('pkg' in UNRESOLVABLE_BARE_IMPORTS).toBe(false); + expect('pkg' in DECLARED_UNRESOLVABLE.import).toBe(false); + expect('pkg' in DECLARED_UNRESOLVABLE.importFromHost).toBe(true); + }); +}); From dbb6f334384392db8f8edc124e429c13457ca817 Mon Sep 17 00:00:00 2001 From: os-litant Date: Wed, 26 Aug 2026 08:55:47 +0000 Subject: [PATCH 2/2] style(cli): collapse a stray blank-line run in the host-resolution sweep Whitespace only; no assertion, table entry or message text changes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd --- packages/cli/src/commands/serve-cluster-host-resolution.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli/src/commands/serve-cluster-host-resolution.test.ts b/packages/cli/src/commands/serve-cluster-host-resolution.test.ts index 425385a75a..41f53aa3e6 100644 --- a/packages/cli/src/commands/serve-cluster-host-resolution.test.ts +++ b/packages/cli/src/commands/serve-cluster-host-resolution.test.ts @@ -509,7 +509,6 @@ const DECLARED_UNRESOLVABLE: Record> /** Every load the scan could not pin to a specifier, whatever the callee. */ const UNRESOLVED_LOADS = LOAD_SITES.filter((site) => site.specifier === undefined); - /** * A host app that DECLARES an optional package and carries it in its own * `node_modules` — the shape of every EE app that declares