diff --git a/packages/cli/bin/run-dev.js b/packages/cli/bin/run-dev.js index 4f8d2ed04b..79570d698f 100644 --- a/packages/cli/bin/run-dev.js +++ b/packages/cli/bin/run-dev.js @@ -11,6 +11,10 @@ // to land on stderr before it. `NODE_ENV` and `settings.debug` are what // `development: true` sets — they are set here so this shim keeps behaving // exactly as it did. +import { spawnSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + import { flush, handle, run, settings } from '@oclif/core'; import { keepStderrNonBlocking } from '../src/utils/stderr-nonblocking.ts'; @@ -151,6 +155,28 @@ async function announceInvocationFailure(error) { */ const moduleLoadFailures = []; +/** + * Where THIS process's loader sends a specifier — the probe + * `unbuiltWorkspaceLines` uses to tell a stale build output apart from a build + * output that was never consulted (#16547). + * + * It has to be the shim's own `import.meta.resolve` rather than one the + * diagnostic builds for itself: this is the resolver that produced the failure + * being reported, tsconfig `paths` and all, so it is the only one that can + * answer for it. A resolver constructed anywhere else answers about a different + * loader and could contradict the run it is describing. + * + * `undefined` on any failure, which the diagnostic reads as "no evidence of a + * redirect" and which leaves the build remedy exactly as it was. + */ +function resolveThroughThisLoader(specifier) { + try { + return import.meta.resolve(specifier); + } catch { + return undefined; + } +} + /** * The other reading of "command … not found": the command is there and its * MODULE would not load, because a workspace package this repo builds has no @@ -169,7 +195,7 @@ async function announceUnbuiltWorkspace(error) { ]); // One write, so the drain that matters happens once, immediately before // `handle()` gets its turn at the same pipe. - const lines = unbuiltWorkspaceLines(error, moduleLoadFailures, INVOCATION_PREFIX) ?? []; + const lines = unbuiltWorkspaceLines(error, moduleLoadFailures, INVOCATION_PREFIX, resolveThroughThisLoader) ?? []; if (lines.length) await writeStderr(`${lines.join('\n')}\n`); } catch { // Stay quiet rather than replacing oclif's report with an error about the @@ -195,6 +221,153 @@ settings.debug = true; // why the re-assert has to sit on the write path rather than run once here. keepStderrNonBlocking(); +/** + * This package's OWN tsconfig — the one its `src/` is written against, and the + * one the guard below pins tsx to. + */ +const CLI_TSCONFIG = fileURLToPath(new URL('../tsconfig.json', import.meta.url)); + +/** This package's manifest, read for the dependency list the probe sweeps. */ +const CLI_PACKAGE_JSON = fileURLToPath(new URL('../package.json', import.meta.url)); + +/** + * The first of this package's OWN workspace dependencies that tsx is resolving + * to TypeScript SOURCE instead of to build output — or '' when none is, which + * is every run from a cwd that carries no redirecting tsconfig. + * + * ## What it detects, measured rather than reasoned (#16547) + * + * tsx reads the CWD's tsconfig, not the entry file's, and applies its + * `compilerOptions.paths` to EVERY specifier it resolves — including this + * CLI's own. Ten in-tree directories carry such a rule, written for tsc so a + * `typecheck` grades against a producer's source rather than its last build + * (`check:type-source-resolution` requires them), and #11094 named the runtime + * half "a latent runtime redirect for any tsx-honouring tool". Run this shim + * from one of them and the CLI's imports are re-routed: + * + * cd examples/app-multi-package + * ../../node_modules/.bin/tsx ../../packages/cli/bin/run-dev.js lint objectstack.config.ts + * → exit 2, "command lint:objectstack.config.ts not found" + * + * ⚠️ NOT because the source is missing an export — the correction #16547's + * repro earned over the reading it was filed with. `@objectstack/spec/data`'s + * source subpath exports the very name the failure blames (470 names, measured + * through `await import()`, `DATABASE_DRIVER_SELECTION_IDS` among them). What + * breaks is the STATIC LINK, and the reason is module FORMAT: `packages/spec` + * and `packages/types` declare no `"type": "module"`, so tsx loads their `.ts` + * sources as CommonJS, and a static ESM named import can then bind only the + * names `cjs-module-lexer` detects — which does not follow the two-hop + * `export *` chain (`data/index.ts` → `./driver/index` → + * `./config-registry.zod`) that publishes this one. Measured with a two-leg + * fixture whose only difference was the `"type"` field: CJS leg SyntaxError, + * ESM leg links. `packages/cli` IS `"type": "module"`, so every one of its + * command modules is on the failing side of that seam. + * + * ## Why the probe is a RESOLUTION and not a tsconfig read + * + * The question "would this cwd redirect us" is decided by tsx's own resolver, + * so it is asked of the resolver. Reading the cwd's tsconfig would mean + * reimplementing get-tsconfig's lookup, its JSONC parse and its `extends` + * walk — three chances to disagree with the thing whose behaviour is the whole + * subject, for an answer this call gets exactly right. + * + * The criterion is that a resolution lands on a TypeScript SOURCE file. No + * workspace package's `exports` map points at one — every one targets `dist/` + * — so a `.ts` answer cannot be produced by node resolution alone. Measured on + * this manifest: 0 of 49 workspace dependencies answer `.ts` from the repo + * root, exactly 1 does from `examples/app-multi-package` (`@objectstack/spec`) + * and exactly 1 from `packages/plugins/plugin-security` (`@objectstack/types`) + * — the two directories #16547 reproduced from, each naming its own package. + * + * Cost, measured on the box this landed on: ~72 ms for the full 49-specifier + * sweep (~1.35 ms per `import.meta.resolve`), against a ~11.3 s end-to-end + * `lint` run through this shim — 0.6%, and it is paid once per process. The + * alternative that needs no probe at all, re-execing unconditionally, costs a + * whole second tsx bootstrap (~550 ms measured) on every run instead. + * + * ⚠️ The error direction is the safe one and is worth stating: a dependency + * that legitimately published a `.ts` entry point would cost one unnecessary + * re-exec, never a wrong answer — pinning this CLI to its own tsconfig is + * always correct for this CLI's own code. + */ +function firstSourceRedirectedDependency() { + let manifest; + try { + manifest = JSON.parse(readFileSync(CLI_PACKAGE_JSON, 'utf8')); + } catch { + // No manifest, no probe. Degrade to the behaviour this shim had before the + // pin existed rather than fail on the way to running the CLI. + return ''; + } + // The scope comes from this package's OWN name rather than a constant, so + // "a package this repo builds" cannot drift away from what this repo calls + // itself: `@objectstack/cli` → `@objectstack/`. + const scope = String(manifest?.name ?? '').split('/')[0]; + if (!scope.startsWith('@')) return ''; + for (const dep of Object.keys(manifest?.dependencies ?? {})) { + if (!dep.startsWith(`${scope}/`)) continue; + let pathname; + try { + // The URL is parsed INSIDE the guard on purpose. This runs before the CLI + // does anything, so a throw here would replace the whole run with an error + // about the probe — the same rule the two reporters below are written to. + pathname = new URL(import.meta.resolve(dep)).pathname; + } catch { + // Not installed, no such subpath, or an answer that is not a URL. Not this + // probe's business, and never this probe's report. + continue; + } + if (/\.[cm]?tsx?$/.test(pathname)) return dep; + } + return ''; +} + +// ⛔ The pin can only be applied by RE-EXEC, and that is a measured constraint +// rather than a preference. tsx parses its tsconfig in the loader's +// `initialize` / `globalPreload`, both of which have already run by the time +// this file gets control: setting `process.env.TSX_TSCONFIG_PATH` here and +// re-resolving answers the SOURCE path exactly as before (measured). So the +// choice is a second process or no pin at all. +// +// The env var doubles as the loop guard, and as the caller's override: a run +// that already carries one is either the child this block spawned or someone +// who pinned deliberately, and neither wants a second opinion. +if (!process.env.TSX_TSCONFIG_PATH) { + const redirected = firstSourceRedirectedDependency(); + if (redirected) { + // ⚠️ `process.stderr.write` followed by an exit is the #6531 defect this + // file exists to avoid, and this is deliberately NOT that shape: what + // follows the write is `spawnSync`, which blocks this process for the + // whole lifetime of the child (seconds), so the write has the entire run + // to drain instead of racing a tear-down. `keepStderrNonBlocking()` above + // has already run, so the write cannot park the thread either. + process.stderr.write( + `objectstack: the current directory's tsconfig redirects '${redirected}' to TypeScript source, and tsx honours the CWD's tsconfig — re-running with tsx pinned to ${CLI_TSCONFIG}\n`, + ); + const child = spawnSync(process.execPath, [...process.execArgv, ...process.argv.slice(1)], { + // Inherited, so the child holds the very fds this process was handed and + // every byte-level property the suites below pin is the CHILD's, not a + // forwarding copy. libuv clears `O_NONBLOCK` on fd 2's shared + // description in the pre-exec — the hazard `keepStderrNonBlocking()` + // exists for — and the child re-asserts it on its own write path, which + // is why that guard had to live on the write rather than run once. + stdio: 'inherit', + env: { ...process.env, TSX_TSCONFIG_PATH: CLI_TSCONFIG }, + }); + if (!child.error) { + // A signalled child is reported as a signal, never as an exit code: #14715 + // pinned that this CLI answers 2 for a failed run, and laundering a + // SIGKILL into some number would make a killed child indistinguishable + // from one that decided. + if (child.signal) process.kill(process.pid, child.signal); + process.exit(child.status ?? 1); + } + // Spawn itself failed. Degrade to the behaviour this shim had before the + // pin existed — which is the failure #16547 describes, and still better + // than replacing the CLI's report with one about the re-exec. + } +} + /** * Make a FAILED stderr write non-fatal, so a caller whose read end is gone * still gets this CLI's own exit status instead of a crash. #14858. diff --git a/packages/cli/test/run-dev-cwd-tsconfig-redirect.e2e.test.ts b/packages/cli/test/run-dev-cwd-tsconfig-redirect.e2e.test.ts new file mode 100644 index 0000000000..08b265755a --- /dev/null +++ b/packages/cli/test/run-dev-cwd-tsconfig-redirect.e2e.test.ts @@ -0,0 +1,237 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #16547 — `bin/run-dev.js` run from a cwd whose tsconfig maps a workspace + * package to its source loaded no command set at all, and blamed a build that + * was present and fresh. + * + * ``` + * $ cd examples/app-multi-package + * $ ../../node_modules/.bin/tsx ../../packages/cli/bin/run-dev.js lint objectstack.config.ts --json + * … + * message: The requested module '@objectstack/spec/data' does not provide an export named 'DATABASE_DRIVER_SELECTION_IDS' + * objectstack: NOT A MISSING COMMAND — … + * objectstack: Fix: pnpm exec turbo run build --filter=@objectstack/spec + * Error: command lint:objectstack.config.ts not found + * $ echo $? + * 2 + * ``` + * + * ## The mechanism, as MEASURED — and the half of the card's reading it corrected + * + * CONFIRMED. tsx reads the CWD's tsconfig, not the entry file's, and applies + * its `compilerOptions.paths` to every specifier it resolves — the CLI's own + * included. From `examples/app-multi-package`, `import.meta.resolve` answers + * the spec package's `data` SOURCE index for `@objectstack/spec/data`; from + * the repo root it answers that package's dist target. Ten in-tree + * directories carry such a rule, written so `tsc --noEmit` grades against a + * producer's SOURCE rather than its last build + * (`check:type-source-resolution` requires them), and #11094 named the + * runtime half "a latent runtime redirect for any tsx-honouring tool". + * + * ⛔ CORRECTED. The card read the failure as the source subpath's export set + * DIFFERING from `dist`. It does not. The spec package's `data` source index + * exports the very name the failure blames — 470 names through + * `await import()`, `DATABASE_DRIVER_SELECTION_IDS` among them. What breaks is + * the STATIC LINK, and the reason is module FORMAT: the spec and types + * packages declare no `"type": "module"`, so tsx loads their `.ts` sources as + * CommonJS; a static ESM named import can then bind only what + * `cjs-module-lexer` detects statically, and the lexer does not follow the + * two-hop `export *` chain (the data index → its driver index → the driver + * config registry) that publishes this name. Measured with a two-leg fixture + * whose ONLY difference was that field — CJS leg `SyntaxError: … does not + * provide an export named 'DEEP_NAME'`, ESM leg links and prints 42. This CLI + * IS `"type": "module"`, which puts every one of its command modules on the + * failing side of that seam. + * + * That correction is why this file asserts on the RESOLUTION and never on an + * export set: the export set is a red herring, and a suite written against it + * would pass for the wrong reason. + * + * ## Why the redirecting cwd is MANUFACTURED + * + * The two directories the card reproduced from are real and still redirect — + * but a suite anchored to `examples/app-multi-package` measures that example's + * tsconfig, not this shim's behaviour, and goes quietly green the day someone + * removes a `paths` rule for reasons of their own. So the cwd is built here: + * a temp directory whose whole content is a tsconfig with one `paths` rule + * aimed at a real workspace source file. It reproduces the card's exit 2 and + * its exact stderr against the pre-#16547 shim (verified by ablation, not + * assumed), and it cannot be disarmed from outside this file. + * + * ## The cases are a control set, not one assertion + * + * 1. redirecting cwd, pin ACTIVE → the command table loads; the run fails + * for its OWN reason (no config file), + * and the shim says what it re-ran. + * 2. redirecting cwd, pin DEFEATED → the diagnostic refuses the rebuild and + * (caller pinned it themselves) names the redirect. This is the branch + * that keeps its value after (1) lands. + * 3. repo root, nothing redirecting → no re-exec, no advisory, and the same + * own-reason failure as (1). + * + * (1) without (3) would pass in a tree where every run is re-exec'd — the shape + * that would cost every invocation a second tsx bootstrap without anyone + * noticing. (3) without (1) is a zero reading. (2) is the only case that can + * see the misdirection this card is graded on. + */ + +import { describe, it, expect, beforeAll } from 'vitest'; +import { execFile } from 'node:child_process'; +import { mkdtempSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { childEnv } from './helpers/serve-process.js'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +/** The SOURCE entry point — this suite is about it, and it needs no `dist/`. */ +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); +const REPO_ROOT = resolve(HERE, '../../..'); +/** The tsconfig the shim pins tsx to, spelled here so a move reds this file. */ +const CLI_TSCONFIG = resolve(HERE, '../tsconfig.json'); + +/** + * A real command whose ARGUMENT names nothing on disk. + * + * Both halves matter. Real, so "command … not found" is a lie rather than the + * truth — a typo would make oclif emit no module-load warning at all and the + * branch under test would never be entered. Naming nothing, so a run whose + * command table DID load fails immediately for its own reason instead of doing + * ~11 s of linting: the point of cases 1 and 3 is WHICH failure, never the work. + */ +const REAL_COMMAND = ['lint', 'nope.ts']; + +/** oclif + tsx cold start, twice over in case 1, on a shared runner. */ +const RUN_TIMEOUT_MS = 180_000; + +interface Run { + code: number; + stdout: string; + stderr: string; +} + +function runCli(cwd: string, env: Record): Promise { + return new Promise((resolvePromise) => { + execFile( + TSX, + [CLI, ...REAL_COMMAND], + { cwd, timeout: RUN_TIMEOUT_MS, maxBuffer: 32 * 1024 * 1024, env: childEnv({ NO_COLOR: '1', ...env }) }, + (err, stdout, stderr) => { + resolvePromise({ + // `err.code` is the real exit status; a non-number means the child was + // signalled — a failure of a different kind, never reported as 0. + code: err ? (typeof (err as { code?: unknown }).code === 'number' ? (err as unknown as { code: number }).code : 1) : 0, + stdout: String(stdout), + stderr: String(stderr), + }); + }, + ); + }); +} + +/** + * The specifier the fixture redirects. Any workspace package this CLI declares + * as a dependency would do; this one is named because it is the one the card + * reproduced on. + */ +const REDIRECTED_SPECIFIER = '@objectstack/spec'; + +/** What the redirect points AT — a stub this suite writes, never real source. */ +const STUB_BASENAME = 'spec-stub.ts'; + +/** + * A cwd that redirects one workspace specifier this CLI imports to a TypeScript + * file that is not that package. + * + * ⚠️ Two properties, and BOTH are load-bearing. + * + * The target must EXIST. A `paths` target that resolves to nothing on disk is + * not a redirect at all — get-tsconfig falls back to node resolution, the run + * succeeds, and every case here would pass against an unfixed shim. + * + * The target must be THIS SUITE'S OWN FILE, not a path into another package. + * Pointing at real workspace source would make this suite's inputs wider than + * its package — invisible to the affected-subset filter and to turbo's cache, + * the defect `check:cross-package-test-inputs` exists to refuse — and it would + * buy nothing: the assertions below are about what the SHIM does with a + * redirect, and a stub redirects exactly as well as the real thing. It is also + * the stronger fixture: a stub that exports nothing the CLI imports cannot + * quietly start satisfying those imports the way a real package could. + */ +let redirectingCwd: string; + +beforeAll(() => { + redirectingCwd = mkdtempSync(join(tmpdir(), 'os-16547-')); + writeFileSync(join(redirectingCwd, STUB_BASENAME), 'export const NOT_THE_REAL_PACKAGE = true;\n'); + writeFileSync( + join(redirectingCwd, 'tsconfig.json'), + `${JSON.stringify({ compilerOptions: { baseUrl: '.', paths: { [REDIRECTED_SPECIFIER]: [`./${STUB_BASENAME}`] } } }, null, 2)}\n`, + ); +}); + +describe('bin/run-dev.js under a cwd tsconfig that redirects a workspace package (#16547)', () => { + it( + 'loads its command set anyway, and says what it re-ran', + async () => { + const run = await runCli(redirectingCwd, { TSX_TSCONFIG_PATH: undefined }); + + // ⛔ The card's failure, gone: the command table loaded, so the run is + // allowed to fail for the reason its ARGUMENT deserves. + expect(run.stderr).not.toContain('NOT A MISSING COMMAND'); + // oclif's own sentence, spelled with its `Error: command` prefix rather + // than as a bare `not found`: the run this case WANTS ends on "Config + // file not found", which contains that substring and made the first + // version of this assertion fail against a working fix. + expect(run.stderr).not.toMatch(/Error:\s*command\b/); + expect(run.stderr).toContain('Config file not found'); + // Not silent. A second process appearing with no explanation is its own + // kind of misdirection, so the shim names the specifier and the pin. + expect(run.stderr).toContain(`redirects '${REDIRECTED_SPECIFIER}' to TypeScript source`); + expect(run.stderr).toContain(CLI_TSCONFIG); + }, + RUN_TIMEOUT_MS, + ); + + it( + 'still explains itself when the pin is defeated, and refuses the rebuild remedy', + async () => { + // A caller who pinned tsx themselves is either this shim's own child or + // someone who meant it; the shim stands down either way. That leaves the + // redirect in force and is the reachable route to the DIAGNOSTIC half of + // this card — the half that keeps its value for every other cause of the + // same masking. + const run = await runCli(redirectingCwd, { TSX_TSCONFIG_PATH: join(redirectingCwd, 'tsconfig.json') }); + + expect(run.code).toBe(2); + expect(run.stderr).toContain('NOT A MISSING COMMAND'); + expect(run.stderr).toContain(`The unmet precondition is NOT ${REDIRECTED_SPECIFIER}'s build output`); + // The evidence, carried rather than summarised. + expect(run.stderr).toContain(STUB_BASENAME); + // ⛔ THE assertion this card is graded on. `packages/spec/dist` is present + // and fresh in this tree — the whole suite depends on a built workspace — + // so a prescription to rebuild it is an action that succeeds and changes + // nothing, which is worse for an agent than a bare failure. + expect(run.stderr).not.toContain(`turbo run build --filter=${REDIRECTED_SPECIFIER}`); + expect(run.stderr).toContain("tsx reads the CWD's tsconfig"); + }, + RUN_TIMEOUT_MS, + ); + + it( + 'CONTROL — from the repo root nothing is redirected, so nothing is re-exec\'d', + async () => { + const run = await runCli(REPO_ROOT, { TSX_TSCONFIG_PATH: undefined }); + + expect(run.stderr).toContain('Config file not found'); + expect(run.stderr).not.toContain('NOT A MISSING COMMAND'); + // The half that keeps the pin from becoming an unconditional second + // process: measured, the probe answers "no redirect" for all 49 workspace + // dependencies from here, and a re-exec would cost a whole tsx bootstrap + // (~550 ms) on every run in the tree. + expect(run.stderr).not.toContain('re-running with tsx pinned'); + }, + RUN_TIMEOUT_MS, + ); +}); diff --git a/packages/cli/test/unbuilt-workspace-lead.test.ts b/packages/cli/test/unbuilt-workspace-lead.test.ts index 44005cc38a..94617c264d 100644 --- a/packages/cli/test/unbuilt-workspace-lead.test.ts +++ b/packages/cli/test/unbuilt-workspace-lead.test.ts @@ -134,3 +134,96 @@ describe('unbuiltWorkspaceLines', () => { expect(unbuiltWorkspaceLines(notFound(), [thirdParty, MEASURED_DETAIL], INVOCATION_PREFIX)?.[0]).toContain('@objectstack/spec'); }); }); + +/** + * ## #16547 — the SAME classified failure, with the build output never consulted + * + * Transcript, not invention. Measured in a worktree at `de0bcdd44d` with + * `packages/types/dist` present and fresh, running the dev entry from + * `packages/plugins/plugin-security`, whose tsconfig maps `@objectstack/types` + * to `../../types/src/index.ts` so its own typecheck grades against source: + * + * $ TSX_TSCONFIG_PATH="$PWD/tsconfig.json" \ + * ../../../node_modules/.bin/tsx ../../../packages/cli/bin/run-dev.js lint objectstack.config.ts + * … + * objectstack: Fix: pnpm exec turbo run build --filter=@objectstack/types + * Error: command lint:objectstack.config.ts not found + * $ echo $? + * 2 + * + * The classifier is right that this is an export mismatch on a package this + * repo builds. It is the REMEDY that was wrong: tsx honours the CWD's tsconfig, + * so the import never reached `packages/types/dist` at all, and the build it + * prescribed succeeds and changes nothing. + */ +const REDIRECTED_DETAIL = [ + 'module: @oclif/core@4.13.3', + 'task: findCommand (start)', + 'plugin: @objectstack/cli', + 'root: /home/user/objectstack-issue-16547/packages/cli', + "message: The requested module '@objectstack/types' does not provide an export named 'PLATFORM_OWNER_EMAIL_ENV'", + 'See more details with DEBUG=*', +].join('\n'); + +/** What the shim's own `import.meta.resolve` answered on that run, verbatim. */ +const REDIRECTED_TO = 'file:///home/user/objectstack-issue-16547/packages/types/src/index.ts'; + +describe('unbuiltWorkspaceLines — build output that was never consulted (#16547)', () => { + it('refuses the rebuild remedy and names the redirect instead', () => { + const lines = unbuiltWorkspaceLines(notFound(), [REDIRECTED_DETAIL], INVOCATION_PREFIX, () => REDIRECTED_TO); + + expect(lines).toHaveLength(2); + // Still contradicts "not found" — that half of #12964 is unchanged. + expect(lines?.[0]).toContain('NOT A MISSING COMMAND'); + // …but the attribution is inverted, and says so in words a reader cannot + // misread as the old line: the precondition is NOT the build output. + expect(lines?.[0]).toContain("The unmet precondition is NOT @objectstack/types's build output"); + // The evidence is carried, not summarised — a reader can check it. + expect(lines?.[0]).toContain(REDIRECTED_TO); + // ⛔ The one assertion the whole card is about: the misdirection is GONE. + // A `toContain` on the new text would pass while the old text sat beside it. + expect(lines?.[1]).not.toContain('turbo run build'); + expect(lines?.[1]).toContain('tsx reads the CWD'); + expect(lines?.[1]).toContain('TSX_TSCONFIG_PATH=packages/cli/tsconfig.json'); + }); + + it('CONTROL — the same failure whose specifier DID reach build output keeps the rebuild', () => { + // The positive control for the case above. Without it, a probe that + // answered "redirect" for everything would read green there and would have + // silently retired #7681's remedy for the cause it was written for. + const lines = unbuiltWorkspaceLines(notFound(), [REDIRECTED_DETAIL], INVOCATION_PREFIX, () => 'file:///repo/packages/types/dist/index.mjs'); + expect(lines?.[0]).toContain("The unmet precondition is @objectstack/types's build output"); + expect(lines?.[1]).toBe('objectstack: Fix: pnpm exec turbo run build --filter=@objectstack/types'); + }); + + it('CONTROL — a caller that asks no question gets the pre-#16547 answer', () => { + // The parameter is optional, and omitting it must not change a verdict. + // `check:declaration-mirrors` holds the declaration to the same optionality. + const lines = unbuiltWorkspaceLines(notFound(), [REDIRECTED_DETAIL], INVOCATION_PREFIX); + expect(lines?.[1]).toBe('objectstack: Fix: pnpm exec turbo run build --filter=@objectstack/types'); + }); + + it('a probe that throws is no evidence, and never becomes the report', () => { + const lines = unbuiltWorkspaceLines(notFound(), [REDIRECTED_DETAIL], INVOCATION_PREFIX, () => { + throw new Error('resolver exploded'); + }); + expect(lines?.[1]).toBe('objectstack: Fix: pnpm exec turbo run build --filter=@objectstack/types'); + }); + + it('leaves the MISSING-OUTPUT shape alone even when the probe would answer source', () => { + // The narrowing stated in `sourceRedirectOf`. A `missing-output` failure + // names a PATH node could not find, so re-resolving it asks a different + // question — and a `paths` target that does not exist on disk falls back to + // node resolution, so a redirect cannot produce this shape in the first + // place. Pinned so a future widening has to argue with this case. + const lines = unbuiltWorkspaceLines(notFound(), [MEASURED_DETAIL], INVOCATION_PREFIX, () => REDIRECTED_TO); + expect(lines?.[1]).toBe('objectstack: Fix: pnpm exec turbo run build --filter=@objectstack/spec'); + }); + + it('reads a bare path as well as a file:// URL', () => { + // `import.meta.resolve` answers a URL; a caller with a plain path must get + // the same verdict, so the extension test runs on the PATH either way. + const lines = unbuiltWorkspaceLines(notFound(), [REDIRECTED_DETAIL], INVOCATION_PREFIX, () => '/repo/packages/types/src/index.ts'); + expect(lines?.[1]).not.toContain('turbo run build'); + }); +}); diff --git a/scripts/cli-build-prerequisite.mjs b/scripts/cli-build-prerequisite.mjs index 98a6a5093b..a11aa4cd28 100644 --- a/scripts/cli-build-prerequisite.mjs +++ b/scripts/cli-build-prerequisite.mjs @@ -326,21 +326,28 @@ const WORKSPACE_PACKAGE_IN_SPECIFIER = new RegExp(`(?:^|node_modules/)(${WORKSPA * and CommonJS interop's differently-worded `Named export 'x' not found`, which is * an authoring problem and not staleness. * + * `specifier` is the module specifier the failure names, carried out alongside + * the package so a caller can ASK WHERE IT WENT rather than assume it reached + * the package's build output (#16547). For `export-mismatch` it is the bare + * specifier as written (`@objectstack/spec/data`); for `missing-output` it is + * whatever path node reported it could not find. Reported, never interpreted + * here: this module still decides nothing it cannot decide from the text. + * * @param {string} text combined stdout/stderr - * @returns {{ kind: 'export-mismatch' | 'missing-output', pkg: string, missingExport: string, sentence: string } | null} + * @returns {{ kind: 'export-mismatch' | 'missing-output', pkg: string, specifier: string, missingExport: string, sentence: string } | null} */ export function looksLikeStaleWorkspaceDist(text) { const flat = flattenCliOutput(text); const mismatch = flat.match(/The requested module '([^']+)' does not provide an export named '([^']+)'/); if (mismatch) { const pkg = mismatch[1].match(WORKSPACE_PACKAGE_IN_SPECIFIER)?.[1] ?? ''; - return pkg ? { kind: 'export-mismatch', pkg, missingExport: mismatch[2], sentence: mismatch[0] } : null; + return pkg ? { kind: 'export-mismatch', pkg, specifier: mismatch[1], missingExport: mismatch[2], sentence: mismatch[0] } : null; } const missing = flat.match(/Cannot find (?:module|package) '([^']+)'/); if (missing) { const pkg = missing[1].match(WORKSPACE_PACKAGE_IN_SPECIFIER)?.[1] ?? ''; if (pkg && /(?:^|\/)dist\//.test(missing[1])) { - return { kind: 'missing-output', pkg, missingExport: '', sentence: missing[0] }; + return { kind: 'missing-output', pkg, specifier: missing[1], missingExport: '', sentence: missing[0] }; } } return null; diff --git a/scripts/cli-unbuilt-workspace-lead.d.mts b/scripts/cli-unbuilt-workspace-lead.d.mts index dfb8fbfea7..b2cb6423d8 100644 --- a/scripts/cli-unbuilt-workspace-lead.d.mts +++ b/scripts/cli-unbuilt-workspace-lead.d.mts @@ -16,17 +16,42 @@ // COMPLETE rather than partial, unlike `invoked-as.d.mts`: the module exports // exactly one thing. Keep this file in step with the module by hand; the mirror // gate checks name, kind and required arity, never types. +// +// ⚠️ `resolveSpecifier` is OPTIONAL, and that is load-bearing for this file +// rather than a style choice: the mirror gate compares REQUIRED arity against +// the module's `fn.length`, and a parameter declared required here while the +// module leaves it optional is exactly the drift that gate exists to catch. +// Optional on both sides is also the honest signature — a caller that cannot +// answer where a specifier resolved gets the pre-#16547 answer. + +/** + * Where a specifier resolved, as the CALLER's own loader answers it. + * + * The diagnostic uses it to tell "this package's build output is stale" apart + * from "this package's build output was never consulted", which are different + * facts with opposite remedies (#16547). `undefined` means the caller cannot + * answer, and is read as no evidence of a redirect. + */ +export type ResolveSpecifier = (specifier: string) => string | undefined; /** * The two lines to print when oclif's "command … not found" was really a - * workspace package with no usable build output — or `undefined` when the - * failure is not that one, which is every ordinary invocation error and every - * command that genuinely does not exist. + * workspace package whose build output could not serve the import — or + * `undefined` when the failure is not that one, which is every ordinary + * invocation error and every command that genuinely does not exist. * * @param error the error `run()` rejected with; only its string form is read. * @param moduleLoadFailures `detail` of each warning oclif emitted while * loading its command table, in emission order. * @param prefix the CLI's own name, which every line it prints starts with * (`INVOCATION_PREFIX` in `packages/cli/src/utils/invocation.ts`). + * @param resolveSpecifier the caller's loader, asked whether the failing + * specifier reached build output at all. Omitted, the answer is the one this + * module gave before #16547. */ -export function unbuiltWorkspaceLines(error: unknown, moduleLoadFailures: readonly string[], prefix: string): [string, string] | undefined; +export function unbuiltWorkspaceLines( + error: unknown, + moduleLoadFailures: readonly string[], + prefix: string, + resolveSpecifier?: ResolveSpecifier, +): [string, string] | undefined; diff --git a/scripts/cli-unbuilt-workspace-lead.mjs b/scripts/cli-unbuilt-workspace-lead.mjs index 1fcfce0e43..0dd942b012 100644 --- a/scripts/cli-unbuilt-workspace-lead.mjs +++ b/scripts/cli-unbuilt-workspace-lead.mjs @@ -79,6 +79,105 @@ import { looksLikeMissingCliCommand, looksLikeStaleWorkspaceDist, workspaceBuildFix } from './cli-build-prerequisite.mjs'; +/** + * Where a specifier actually resolved, when the caller can answer that. + * + * ## Why this module asks at all (#16547) + * + * The remedy below used to be unconditional: classify the failure, name the + * package, prescribe its build. That is right whenever the failing import + * REACHED the package's build output — and #16547 measured a whole class where + * it did not, and where the prescription is therefore worse than silence. + * + * Run this CLI's dev entry from a directory whose tsconfig maps a workspace + * package to its `src/`, and tsx — which reads the CWD's tsconfig, not the + * entry's — re-routes the CLI's OWN imports to that source. The package's + * `dist/` is present, fresh, and never consulted. The failure still arrives as + * an export mismatch on a `@objectstack/*` specifier, so the classifier next + * door still says "stale build output", and the operator is handed a build that + * succeeds and changes nothing: + * + * objectstack: Fix: pnpm exec turbo run build --filter=@objectstack/spec + * $ pnpm exec turbo run build --filter=@objectstack/spec # succeeds + * $ # fails identically + * + * The triage on #16547 graded that loop as the reason the card is p2 rather than + * p3: a named, plausible, executable action that cannot converge leaves nothing + * in the output to break the loop, which for an agent is worse than a bare + * failure. So the remedy is now conditional on the one fact that separates the + * two causes. + * + * ## Why RESOLUTION and not "does the dist exist, and is it fresh" + * + * Existence and freshness are proxies for the question that actually decides + * the remedy, and both answer it WRONG here: `packages/spec/dist` is present AND + * fresh in exactly the runs this exists to catch — #16547's repro verified both + * of the blamed exports present in `dist` before the card was filed. Asking + * where the specifier RESOLVED answers the question directly — was the build + * output consulted at all — and needs no build-input hash, no stamp read, and no + * second definition of "fresh" to keep in step with `check-dev-prereqs.mjs`, + * which owns the only one this repo has. + * + * The criterion is that the answer is a TypeScript SOURCE file. No workspace + * package's `exports` map points at one — every one targets `dist/` — so a `.ts` + * answer cannot come out of node resolution alone, and is positive evidence of a + * redirect rather than a guess about one. + * + * ⛔ The resolution is INJECTED rather than performed here, and that keeps this + * module what its header promises: a decision over strings, with no filesystem + * and no loader of its own. Only the shim knows which loader was in play, and + * only the shim's `import.meta.resolve` answers for the very resolver that + * produced the failure — one built here would answer for a DIFFERENT resolver + * and could contradict the run it is describing. + * + * @callback ResolveSpecifier + * @param {string} specifier the specifier the failure named + * @returns {string | undefined} the resolved URL or path, or `undefined` when the + * caller cannot answer, which is read as "no evidence of a redirect" and leaves + * the build remedy exactly as it was + */ + +/** Extensions no workspace `exports` map targets, so seeing one means a redirect. */ +const TYPESCRIPT_SOURCE = /\.[cm]?tsx?$/; + +/** + * The source file a failing specifier was redirected to, or '' for no redirect. + * + * Narrowed to `export-mismatch` on purpose. A `missing-output` failure names a + * PATH node could not find rather than a bare specifier, so re-resolving it asks + * a different question — and a redirect cannot produce that shape anyway: a + * `paths` target that does not exist on disk falls back to node resolution, so + * the redirect either lands on a real source file or never happened. + * + * @param {{ kind: string, pkg: string, specifier: string }} cause + * @param {ResolveSpecifier} [resolveSpecifier] + * @returns {string} + */ +function sourceRedirectOf(cause, resolveSpecifier) { + if (cause.kind !== 'export-mismatch') return ''; + if (typeof resolveSpecifier !== 'function') return ''; + let resolved; + try { + resolved = resolveSpecifier(cause.specifier); + } catch { + // A probe that throws must never become the report. No evidence is not + // evidence of no redirect, so the caller keeps the remedy it already had. + return ''; + } + if (typeof resolved !== 'string' || !resolved) return ''; + // Compared on the PATH, so a `file://` URL and a bare path answer alike and a + // query string cannot hide the extension. + let pathname = resolved; + if (resolved.includes('://')) { + try { + pathname = new URL(resolved).pathname; + } catch { + // Not a URL after all; the raw string is the path. + } + } + return TYPESCRIPT_SOURCE.test(pathname) ? resolved : ''; +} + /** * The two lines, or `undefined` when this failure is not that one. * @@ -94,9 +193,12 @@ import { looksLikeMissingCliCommand, looksLikeStaleWorkspaceDist, workspaceBuild * its `ModuleLoadError` warnings there * @param {string} prefix the CLI's own name, as every line it prints starts * with (`INVOCATION_PREFIX` in `packages/cli/src/utils/invocation.ts`) + * @param {ResolveSpecifier} [resolveSpecifier] the caller's own resolver, asked + * whether the failing specifier reached build output at all (#16547). Omitted, + * this module answers exactly as it did before that card. * @returns {[string, string] | undefined} `[lead, fix]`, or `undefined` */ -export function unbuiltWorkspaceLines(error, moduleLoadFailures, prefix) { +export function unbuiltWorkspaceLines(error, moduleLoadFailures, prefix, resolveSpecifier = undefined) { if (!looksLikeMissingCliCommand(String(error))) return undefined; for (const detail of moduleLoadFailures) { @@ -105,6 +207,15 @@ export function unbuiltWorkspaceLines(error, moduleLoadFailures, prefix) { // list of them would be the "9 bundle problems" shape #5217 removed. const cause = looksLikeStaleWorkspaceDist(String(detail)); if (!cause) continue; + // The build output was never consulted, so naming a build would send the + // reader round a loop that cannot converge (#16547). + const redirect = sourceRedirectOf(cause, resolveSpecifier); + if (redirect) { + return [ + `${prefix}: NOT A MISSING COMMAND — @oclif/core reports a command module that failed to LOAD as "not found", and one did: ${cause.sentence}. The unmet precondition is NOT ${cause.pkg}'s build output: '${cause.specifier}' resolved to ${redirect}, a TypeScript SOURCE file, so ${cause.pkg}'s build output was never consulted and rebuilding it changes nothing.`, + `${prefix}: Fix: a tsconfig \`paths\` rule found from the current directory redirects '${cause.specifier}' to source, and tsx reads the CWD's tsconfig rather than the entry's. Run from the repository root, or pin tsx to this CLI's own: TSX_TSCONFIG_PATH=packages/cli/tsconfig.json`, + ]; + } return [ `${prefix}: NOT A MISSING COMMAND — @oclif/core reports a command module that failed to LOAD as "not found", and one did: ${cause.sentence}. The unmet precondition is ${cause.pkg}'s build output, not the invocation.`, `${prefix}: Fix: ${workspaceBuildFix(cause.pkg)}`,