Skip to content

Commit 0f07b2c

Browse files
claude[bot]os-salesclaude
authored
fix(cli): pin tsx to the CLI's own tsconfig, and stop prescribing a build that was never consulted (#16643)
* fix(cli): pin tsx to the CLI's own tsconfig, and stop prescribing a build that was never consulted `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. tsx reads the CWD's tsconfig, not the entry's, and applies its `paths` to every specifier it resolves -- the CLI's own included. The shim now asks its own loader whether any of this package's workspace dependencies is being resolved to TypeScript source, and re-execs once with `TSX_TSCONFIG_PATH` pinned to `packages/cli/tsconfig.json` when one is. The diagnostic keeps its value for every other cause of the same masking: it now asks WHERE the failing specifier resolved before prescribing a rebuild, and names the cwd tsconfig redirect when the build output was never consulted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ * test(cli): make the #16547 redirect fixture hermetic The tsconfig `paths` target is now a stub this suite writes into its own temp cwd, not a path into another package: `check:cross-package-test-inputs` refuses inputs wider than the package (they are invisible to the affected-subset filter and to turbo's cache), and a stub redirects exactly as well as real source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ * fix(cli): keep the redirect probe from ever becoming the report `new URL()` was parsed outside the guard, so a resolution that is not a URL would have replaced the whole run with an error about the probe -- the rule the two reporters in this file are already written to. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ --------- Co-authored-by: os-sales <sales@objectstack.ai> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 233222e commit 0f07b2c

6 files changed

Lines changed: 655 additions & 9 deletions

‎packages/cli/bin/run-dev.js‎

Lines changed: 174 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@
1111
// to land on stderr before it. `NODE_ENV` and `settings.debug` are what
1212
// `development: true` sets — they are set here so this shim keeps behaving
1313
// exactly as it did.
14+
import { spawnSync } from 'node:child_process';
15+
import { readFileSync } from 'node:fs';
16+
import { fileURLToPath } from 'node:url';
17+
1418
import { flush, handle, run, settings } from '@oclif/core';
1519

1620
import { keepStderrNonBlocking } from '../src/utils/stderr-nonblocking.ts';
@@ -151,6 +155,28 @@ async function announceInvocationFailure(error) {
151155
*/
152156
const moduleLoadFailures = [];
153157

158+
/**
159+
* Where THIS process's loader sends a specifier — the probe
160+
* `unbuiltWorkspaceLines` uses to tell a stale build output apart from a build
161+
* output that was never consulted (#16547).
162+
*
163+
* It has to be the shim's own `import.meta.resolve` rather than one the
164+
* diagnostic builds for itself: this is the resolver that produced the failure
165+
* being reported, tsconfig `paths` and all, so it is the only one that can
166+
* answer for it. A resolver constructed anywhere else answers about a different
167+
* loader and could contradict the run it is describing.
168+
*
169+
* `undefined` on any failure, which the diagnostic reads as "no evidence of a
170+
* redirect" and which leaves the build remedy exactly as it was.
171+
*/
172+
function resolveThroughThisLoader(specifier) {
173+
try {
174+
return import.meta.resolve(specifier);
175+
} catch {
176+
return undefined;
177+
}
178+
}
179+
154180
/**
155181
* The other reading of "command … not found": the command is there and its
156182
* MODULE would not load, because a workspace package this repo builds has no
@@ -169,7 +195,7 @@ async function announceUnbuiltWorkspace(error) {
169195
]);
170196
// One write, so the drain that matters happens once, immediately before
171197
// `handle()` gets its turn at the same pipe.
172-
const lines = unbuiltWorkspaceLines(error, moduleLoadFailures, INVOCATION_PREFIX) ?? [];
198+
const lines = unbuiltWorkspaceLines(error, moduleLoadFailures, INVOCATION_PREFIX, resolveThroughThisLoader) ?? [];
173199
if (lines.length) await writeStderr(`${lines.join('\n')}\n`);
174200
} catch {
175201
// Stay quiet rather than replacing oclif's report with an error about the
@@ -195,6 +221,153 @@ settings.debug = true;
195221
// why the re-assert has to sit on the write path rather than run once here.
196222
keepStderrNonBlocking();
197223

224+
/**
225+
* This package's OWN tsconfig — the one its `src/` is written against, and the
226+
* one the guard below pins tsx to.
227+
*/
228+
const CLI_TSCONFIG = fileURLToPath(new URL('../tsconfig.json', import.meta.url));
229+
230+
/** This package's manifest, read for the dependency list the probe sweeps. */
231+
const CLI_PACKAGE_JSON = fileURLToPath(new URL('../package.json', import.meta.url));
232+
233+
/**
234+
* The first of this package's OWN workspace dependencies that tsx is resolving
235+
* to TypeScript SOURCE instead of to build output — or '' when none is, which
236+
* is every run from a cwd that carries no redirecting tsconfig.
237+
*
238+
* ## What it detects, measured rather than reasoned (#16547)
239+
*
240+
* tsx reads the CWD's tsconfig, not the entry file's, and applies its
241+
* `compilerOptions.paths` to EVERY specifier it resolves — including this
242+
* CLI's own. Ten in-tree directories carry such a rule, written for tsc so a
243+
* `typecheck` grades against a producer's source rather than its last build
244+
* (`check:type-source-resolution` requires them), and #11094 named the runtime
245+
* half "a latent runtime redirect for any tsx-honouring tool". Run this shim
246+
* from one of them and the CLI's imports are re-routed:
247+
*
248+
* cd examples/app-multi-package
249+
* ../../node_modules/.bin/tsx ../../packages/cli/bin/run-dev.js lint objectstack.config.ts
250+
* → exit 2, "command lint:objectstack.config.ts not found"
251+
*
252+
* ⚠️ NOT because the source is missing an export — the correction #16547's
253+
* repro earned over the reading it was filed with. `@objectstack/spec/data`'s
254+
* source subpath exports the very name the failure blames (470 names, measured
255+
* through `await import()`, `DATABASE_DRIVER_SELECTION_IDS` among them). What
256+
* breaks is the STATIC LINK, and the reason is module FORMAT: `packages/spec`
257+
* and `packages/types` declare no `"type": "module"`, so tsx loads their `.ts`
258+
* sources as CommonJS, and a static ESM named import can then bind only the
259+
* names `cjs-module-lexer` detects — which does not follow the two-hop
260+
* `export *` chain (`data/index.ts` → `./driver/index` →
261+
* `./config-registry.zod`) that publishes this one. Measured with a two-leg
262+
* fixture whose only difference was the `"type"` field: CJS leg SyntaxError,
263+
* ESM leg links. `packages/cli` IS `"type": "module"`, so every one of its
264+
* command modules is on the failing side of that seam.
265+
*
266+
* ## Why the probe is a RESOLUTION and not a tsconfig read
267+
*
268+
* The question "would this cwd redirect us" is decided by tsx's own resolver,
269+
* so it is asked of the resolver. Reading the cwd's tsconfig would mean
270+
* reimplementing get-tsconfig's lookup, its JSONC parse and its `extends`
271+
* walk — three chances to disagree with the thing whose behaviour is the whole
272+
* subject, for an answer this call gets exactly right.
273+
*
274+
* The criterion is that a resolution lands on a TypeScript SOURCE file. No
275+
* workspace package's `exports` map points at one — every one targets `dist/`
276+
* — so a `.ts` answer cannot be produced by node resolution alone. Measured on
277+
* this manifest: 0 of 49 workspace dependencies answer `.ts` from the repo
278+
* root, exactly 1 does from `examples/app-multi-package` (`@objectstack/spec`)
279+
* and exactly 1 from `packages/plugins/plugin-security` (`@objectstack/types`)
280+
* — the two directories #16547 reproduced from, each naming its own package.
281+
*
282+
* Cost, measured on the box this landed on: ~72 ms for the full 49-specifier
283+
* sweep (~1.35 ms per `import.meta.resolve`), against a ~11.3 s end-to-end
284+
* `lint` run through this shim — 0.6%, and it is paid once per process. The
285+
* alternative that needs no probe at all, re-execing unconditionally, costs a
286+
* whole second tsx bootstrap (~550 ms measured) on every run instead.
287+
*
288+
* ⚠️ The error direction is the safe one and is worth stating: a dependency
289+
* that legitimately published a `.ts` entry point would cost one unnecessary
290+
* re-exec, never a wrong answer — pinning this CLI to its own tsconfig is
291+
* always correct for this CLI's own code.
292+
*/
293+
function firstSourceRedirectedDependency() {
294+
let manifest;
295+
try {
296+
manifest = JSON.parse(readFileSync(CLI_PACKAGE_JSON, 'utf8'));
297+
} catch {
298+
// No manifest, no probe. Degrade to the behaviour this shim had before the
299+
// pin existed rather than fail on the way to running the CLI.
300+
return '';
301+
}
302+
// The scope comes from this package's OWN name rather than a constant, so
303+
// "a package this repo builds" cannot drift away from what this repo calls
304+
// itself: `@objectstack/cli` → `@objectstack/`.
305+
const scope = String(manifest?.name ?? '').split('/')[0];
306+
if (!scope.startsWith('@')) return '';
307+
for (const dep of Object.keys(manifest?.dependencies ?? {})) {
308+
if (!dep.startsWith(`${scope}/`)) continue;
309+
let pathname;
310+
try {
311+
// The URL is parsed INSIDE the guard on purpose. This runs before the CLI
312+
// does anything, so a throw here would replace the whole run with an error
313+
// about the probe — the same rule the two reporters below are written to.
314+
pathname = new URL(import.meta.resolve(dep)).pathname;
315+
} catch {
316+
// Not installed, no such subpath, or an answer that is not a URL. Not this
317+
// probe's business, and never this probe's report.
318+
continue;
319+
}
320+
if (/\.[cm]?tsx?$/.test(pathname)) return dep;
321+
}
322+
return '';
323+
}
324+
325+
// ⛔ The pin can only be applied by RE-EXEC, and that is a measured constraint
326+
// rather than a preference. tsx parses its tsconfig in the loader's
327+
// `initialize` / `globalPreload`, both of which have already run by the time
328+
// this file gets control: setting `process.env.TSX_TSCONFIG_PATH` here and
329+
// re-resolving answers the SOURCE path exactly as before (measured). So the
330+
// choice is a second process or no pin at all.
331+
//
332+
// The env var doubles as the loop guard, and as the caller's override: a run
333+
// that already carries one is either the child this block spawned or someone
334+
// who pinned deliberately, and neither wants a second opinion.
335+
if (!process.env.TSX_TSCONFIG_PATH) {
336+
const redirected = firstSourceRedirectedDependency();
337+
if (redirected) {
338+
// ⚠️ `process.stderr.write` followed by an exit is the #6531 defect this
339+
// file exists to avoid, and this is deliberately NOT that shape: what
340+
// follows the write is `spawnSync`, which blocks this process for the
341+
// whole lifetime of the child (seconds), so the write has the entire run
342+
// to drain instead of racing a tear-down. `keepStderrNonBlocking()` above
343+
// has already run, so the write cannot park the thread either.
344+
process.stderr.write(
345+
`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`,
346+
);
347+
const child = spawnSync(process.execPath, [...process.execArgv, ...process.argv.slice(1)], {
348+
// Inherited, so the child holds the very fds this process was handed and
349+
// every byte-level property the suites below pin is the CHILD's, not a
350+
// forwarding copy. libuv clears `O_NONBLOCK` on fd 2's shared
351+
// description in the pre-exec — the hazard `keepStderrNonBlocking()`
352+
// exists for — and the child re-asserts it on its own write path, which
353+
// is why that guard had to live on the write rather than run once.
354+
stdio: 'inherit',
355+
env: { ...process.env, TSX_TSCONFIG_PATH: CLI_TSCONFIG },
356+
});
357+
if (!child.error) {
358+
// A signalled child is reported as a signal, never as an exit code: #14715
359+
// pinned that this CLI answers 2 for a failed run, and laundering a
360+
// SIGKILL into some number would make a killed child indistinguishable
361+
// from one that decided.
362+
if (child.signal) process.kill(process.pid, child.signal);
363+
process.exit(child.status ?? 1);
364+
}
365+
// Spawn itself failed. Degrade to the behaviour this shim had before the
366+
// pin existed — which is the failure #16547 describes, and still better
367+
// than replacing the CLI's report with one about the re-exec.
368+
}
369+
}
370+
198371
/**
199372
* Make a FAILED stderr write non-fatal, so a caller whose read end is gone
200373
* still gets this CLI's own exit status instead of a crash. #14858.

0 commit comments

Comments
 (0)