Skip to content

Commit ee385c7

Browse files
claude[bot]claude
andauthored
fix(devx): print the importer's real repo-relative path in the prerequisite advisory (#14217)
The advisory interpolated the gate BASENAME into a hard-coded `scripts/` directory. Correct for the 42 importers under `scripts/**`, wrong for the three under `packages/lint/scripts/**`: it printed `node scripts/check-doc-formula-expressions.mjs`, which names no file. Copy- pasting it answered `Cannot find module` at exit 1 — a banner whose purpose is to stop a reader misreading an exit code handed them a third failure wearing a finding's code. Derive the path from `importerUrl` instead. The headline gate name stays a basename (it is correct as one), and so does the `/tmp/NAME.log` sink — a repo-relative path there would name directories that do not exist. Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5234b7c commit ee385c7

1 file changed

Lines changed: 167 additions & 3 deletions

File tree

scripts/import-prerequisite.mjs

Lines changed: 167 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@
7979
*/
8080
import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs';
8181
import { tmpdir } from 'node:os';
82-
import { dirname, join, resolve } from 'node:path';
83-
import { fileURLToPath } from 'node:url';
82+
import { dirname, join, relative, resolve, sep } from 'node:path';
83+
import { fileURLToPath, pathToFileURL } from 'node:url';
8484
import { WORKSPACE_SCOPE, workspaceBuildFix } from './cli-build-prerequisite.mjs';
8585
import { isEntrypoint } from './invoked-as.mjs';
8686

@@ -253,6 +253,70 @@ function isWholePackage(pkg, fromDir) {
253253
return Boolean(dir) && entryPointOnDisk(dir).present === true;
254254
}
255255

256+
/**
257+
* The repo root at or above a directory: the nearest ancestor holding
258+
* `pnpm-workspace.yaml`. '' when none does.
259+
*
260+
* ⚠️ The marker is the workspace manifest and NOT `.git`, which looks like the
261+
* more obvious choice and is wrong for the exact checkout shape `CLAUDE.md`
262+
* mandates. In a linked worktree — `git worktree add ../objectstack-<task>` —
263+
* `.git` is a FILE containing a `gitdir:` pointer, not a directory (measured
264+
* here: 70 bytes, ASCII). A walk testing `statSync('.git').isDirectory()`
265+
* therefore steps straight PAST the worktree root, finds nothing above it, and
266+
* reports no root at all — in the one tree every agent actually works in.
267+
* `existsSync` on `.git` would survive that, but the workspace manifest is the
268+
* marker this repo already publishes for "repo root" (AGENTS.md names
269+
* `findUp(existsSync(join(dir, 'pnpm-workspace.yaml')))` as the spelling), so
270+
* it is the one used here rather than a second convention.
271+
*
272+
* NEAREST wins, as node and pnpm resolve. The tree holds a second
273+
* `pnpm-workspace.yaml`, under `packages/create-objectstack/src/templates/blank/`
274+
* — a scaffold template, containing no gate and importing nothing, so no
275+
* importer resolves through it. A "highest wins" walk would be the riskier rule:
276+
* it can climb OUT of the repo when the checkout sits inside another workspace.
277+
*/
278+
export function repoRootFrom(dir) {
279+
let d = resolve(dir);
280+
for (;;) {
281+
if (existsSync(join(d, 'pnpm-workspace.yaml'))) return d;
282+
const parent = dirname(d);
283+
if (parent === d) return '';
284+
d = parent;
285+
}
286+
}
287+
288+
/**
289+
* The importer spelled the way the reader can actually RUN it.
290+
*
291+
* The gate NAME is a basename and is correct as one (it is what the headline
292+
* says, and what the gate is called). The COMMAND is not a name, it is a path,
293+
* and interpolating the basename into a hard-coded `scripts/` directory was
294+
* right for the 42 importers under `scripts/**` and wrong for the three under
295+
* `packages/lint/scripts/**`: it printed `node scripts/check-doc-formula-
296+
* expressions.mjs`, which does not exist. Copy-pasting it answered
297+
* `Cannot find module` at exit 1 — so a banner whose whole purpose is to stop a
298+
* reader misreading an exit code handed them a THIRD failure, wearing a
299+
* finding's exit code, one level down from the defect this module removes.
300+
*
301+
* Falls back to the ABSOLUTE path when no root is found, never to the old
302+
* basename guess. An absolute path always runs; a relative path invented
303+
* against a root that was never located is the confident wrong answer this
304+
* module exists to refuse.
305+
*
306+
* Emitted with POSIX separators: the result is a shell command, not a path for
307+
* this process to open.
308+
*/
309+
export function importerCommandPath(importerUrl) {
310+
const file = fileURLToPath(importerUrl);
311+
const root = repoRootFrom(dirname(file));
312+
if (!root) return file;
313+
const rel = relative(root, file);
314+
// A path that climbs out of the root is not repo-relative in any useful
315+
// sense; print the absolute one rather than a `../..` chain.
316+
if (!rel || rel.startsWith('..')) return file;
317+
return rel.split(sep).join('/');
318+
}
319+
256320
/**
257321
* The package node says it could not find, read out of its own error text.
258322
*
@@ -493,6 +557,13 @@ export function reportPrerequisiteNotMet(importerUrl, verdict, measures) {
493557
*/
494558
function prerequisiteNotMetText(importerUrl, verdict, measures) {
495559
const gate = fileURLToPath(importerUrl).split('/').pop().replace(/\.mjs$/, '');
560+
// The path to RUN and the name to CALL IT BY are two different strings, and
561+
// only the first moves. ⛔ The `/tmp/${gate}.log` sink below keeps the
562+
// BASENAME on purpose: a repo-relative path there would spell
563+
// `/tmp/packages/lint/scripts/….log`, whose parent directories do not exist,
564+
// so the redirect fails and the reader is handed a broken command again —
565+
// the same defect, relocated one token to the right.
566+
const command = importerCommandPath(importerUrl);
496567
const subject = measures ? `whether ${measures}` : `what it gates`;
497568
return (
498569
`\n${gate}: PREREQUISITE NOT MET — ${verdict.headline}\n\n` +
@@ -502,7 +573,7 @@ function prerequisiteNotMetText(importerUrl, verdict, measures) {
502573
` result says NOTHING about ${subject}. It is NOT a finding, and it is not\n` +
503574
` evidence that anything in the tree is wrong.\n` +
504575
` (Exit code ${EXIT_PREREQUISITE_NOT_MET}, distinct from a finding's ${EXIT_FINDINGS} — capture it BEFORE any pipe:\n` +
505-
` \`node scripts/${gate}.mjs > /tmp/${gate}.log 2>&1; echo "EXIT=$?"\`.\n` +
576+
` \`node ${command} > /tmp/${gate}.log 2>&1; echo "EXIT=$?"\`.\n` +
506577
` Piped, \`$?\` is the LAST command's status, and \`head\`/\`tail\` essentially never fail — that\n` +
507578
` is the false green, and no pipe shape repairs it. \`\${PIPESTATUS[0]}\`/\`pipefail\` do recover\n` +
508579
` this gate's own code: \`| tail\` reads to EOF and forwards it, while \`| head -N\` closes the\n` +
@@ -682,6 +753,87 @@ export function selfTest() {
682753
findPackageDir('whole-fixture', deep) === join(nm, 'whole-fixture'));
683754
t('a package that is nowhere on the path is not found',
684755
findPackageDir('totally-absent-fixture', deep) === '');
756+
757+
// ── the printed COMMAND: a path the reader can run, not a name ──────────
758+
//
759+
// A REAL tree again, for this module's standing reason: the derivation
760+
// turns on files being on disk, and a model would agree with an
761+
// implementation that never looked.
762+
//
763+
// The fixture is shaped like the checkout every agent actually works in —
764+
// a LINKED WORKTREE, whose `.git` is a FILE holding a `gitdir:` pointer.
765+
// That shape is the whole reason the marker is the workspace manifest, and
766+
// the negative control below is what turns that from a preference into a
767+
// measurement.
768+
const wt = join(dir, 'objectstack-issue-fixture');
769+
mkdirSync(join(wt, 'scripts'), { recursive: true });
770+
mkdirSync(join(wt, 'packages', 'lint', 'scripts'), { recursive: true });
771+
writeFileSync(join(wt, 'pnpm-workspace.yaml'), "packages:\n - 'packages/*'\n");
772+
writeFileSync(join(wt, '.git'), `gitdir: ${join(dir, 'common', 'worktrees', 'wt')}\n`);
773+
774+
const advisoryFor = (abs) =>
775+
prerequisiteNotMetText(pathToFileURL(abs).href, { headline: 'h', detail: ['d'], fix: 'f' }, undefined);
776+
const lintGate = join(wt, 'packages', 'lint', 'scripts', 'check-doc-formula-expressions.mjs');
777+
const rootGate = join(wt, 'scripts', 'check-ci-filter-parity.mjs');
778+
779+
// (a) THE DEFECT, in both directions. The negative is the load-bearing
780+
// one: `node scripts/check-doc-formula-expressions.mjs` is the exact string
781+
// this card exists to stop printing, and it names a file that has never
782+
// existed.
783+
t('a packages/lint/scripts importer prints its REAL repo-relative path',
784+
advisoryFor(lintGate).includes('`node packages/lint/scripts/check-doc-formula-expressions.mjs > '),
785+
advisoryFor(lintGate));
786+
t('⛔ and NEVER the `scripts/` basename guess, which names no file on disk',
787+
!advisoryFor(lintGate).includes('node scripts/check-doc-formula-expressions.mjs'));
788+
789+
// (b) The 42 importers under `scripts/**` inherit this line verbatim, so
790+
// the spelling is pinned WHOLE — command, log sink and the `echo` that
791+
// captures the code before any pipe.
792+
t('a scripts/** importer still prints `scripts/NAME.mjs`, spelling unchanged',
793+
advisoryFor(rootGate).includes(
794+
'`node scripts/check-ci-filter-parity.mjs > /tmp/check-ci-filter-parity.log 2>&1; echo "EXIT=$?"`'),
795+
advisoryFor(rootGate));
796+
797+
// (c) The marker choice, as a paired measurement rather than an assertion.
798+
const gitIsDirectoryWalk = (from) => {
799+
let d = resolve(from);
800+
for (;;) {
801+
try {
802+
if (statSync(join(d, '.git')).isDirectory()) return d;
803+
} catch { /* absent here; keep walking */ }
804+
const parent = dirname(d);
805+
if (parent === d) return '';
806+
d = parent;
807+
}
808+
};
809+
t('the workspace-manifest walk finds the worktree root',
810+
repoRootFrom(join(wt, 'packages', 'lint', 'scripts')) === wt);
811+
t('NEGATIVE CONTROL: a `.git`-isDirectory walk finds NO root in a worktree shape',
812+
gitIsDirectoryWalk(join(wt, 'packages', 'lint', 'scripts')) === '' && statSync(join(wt, '.git')).isFile());
813+
814+
// (d) No locatable root: absolute, which always runs. ⛔ Never a relative
815+
// path invented against a root that was never found.
816+
const orphanGate = join(dir, 'no-marker', 'scripts', 'check-orphan-fixture.mjs');
817+
mkdirSync(dirname(orphanGate), { recursive: true });
818+
t('an importer with no locatable root prints an ABSOLUTE path, always runnable',
819+
importerCommandPath(pathToFileURL(orphanGate).href) === orphanGate,
820+
importerCommandPath(pathToFileURL(orphanGate).href));
821+
t('⛔ and never a relative path invented against a root that was not found',
822+
!advisoryFor(orphanGate).includes('`node scripts/check-orphan-fixture.mjs'));
823+
824+
// (e) Triage's explicit boundary on this card: the HEADLINE gate name is a
825+
// basename and is CORRECT as one. Only the command was wrong.
826+
t('the headline still names the gate by BASENAME, never by path',
827+
advisoryFor(lintGate).includes('\ncheck-doc-formula-expressions: PREREQUISITE NOT MET')
828+
&& !advisoryFor(lintGate).includes('packages/lint/scripts/check-doc-formula-expressions: PREREQUISITE'),
829+
advisoryFor(lintGate));
830+
831+
// (f) The log sink keeps the basename too — `/tmp/packages/lint/scripts/
832+
// ….log` names directories that do not exist, so a blanket substitution
833+
// would hand back a broken command one token to the right.
834+
t('the /tmp log sink keeps the BASENAME — a repo-relative one names absent directories',
835+
advisoryFor(lintGate).includes('> /tmp/check-doc-formula-expressions.log 2>&1')
836+
&& !advisoryFor(lintGate).includes('/tmp/packages/lint'));
685837
} finally {
686838
rmSync(dir, { recursive: true, force: true });
687839
}
@@ -738,6 +890,18 @@ export function selfTest() {
738890
t('NEGATIVE CONTROL: the literal-exit pin can still fail', hardcodesExitCall(controlHardcodedExit));
739891
t('NEGATIVE CONTROL: the literal-advisory pin can still fail', spellsALiteralCode(controlLiteralAdvisory));
740892

893+
// The same shape for the directory. The regression that costs something here
894+
// is not a mistyped path: it is an author interpolating the gate NAME back
895+
// into a hard-coded `scripts/` because the headline beside it does exactly
896+
// that. It would read correct, stay green for the 42 gates under `scripts/`,
897+
// and be wrong only for the three that are the whole point of this pin.
898+
const hardcodesScriptsDir = (fn) => /`node scripts\//.test(fn.toString());
899+
const controlHardcodedScriptsDir = () => ` \`node scripts/${'gate'}.mjs > /tmp/x.log\``;
900+
t('the advisory INTERPOLATES the importer path rather than hard-coding `scripts/`',
901+
!hardcodesScriptsDir(prerequisiteNotMetText));
902+
t('NEGATIVE CONTROL: the hard-coded-directory pin can still fail',
903+
hardcodesScriptsDir(controlHardcodedScriptsDir));
904+
741905
t('the refusal class is 3 — the code four sibling gates answer these words with',
742906
EXIT_PREREQUISITE_NOT_MET === 3, String(EXIT_PREREQUISITE_NOT_MET));
743907
t('the refusal class is distinct from a finding and from a pass',

0 commit comments

Comments
 (0)