Skip to content

Commit 288fe9c

Browse files
claude[bot]claude
andauthored
fix(types): a link:/file: install's refusal states the LIMIT, not a false install remedy (#17030)
* wip: #15045 location-install diagnostic * fix(types): a location install's refusal states the LIMIT, not a false install remedy `createHostImporter`'s ESM-only fallback finder refuses a `link:` / `file:` install whose linked manifest names something other than the declaration key — correctly, and deliberately, since a location specifier carries no package name to expect and accepting the directory anyway would trade a wrong REMEDY for a wrong LOAD. It reported that refusal with the `declared-unresolvable` INSTALL wording, every remedy of which is measurably false for this shape: the finder had just read the manifest at `node_modules/<key>`, so the package is on disk, was not pruned, and its `import` target exists. The refusal is unchanged — same kind, same `MODULE_NOT_FOUND`, same exit path, same accept set. Only the words change: the message now states the directory it consulted, the name found there, the name expected, why a location specifier leaves it only the key, and the remedy that works (make the two names agree, from either end — both pinned as loading). The second verification axis the card also proposes — comparing `realpath(node_modules/<key>)` against the declared location, which would make these installs LOAD — is deliberately not built here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7e63370 commit 288fe9c

3 files changed

Lines changed: 422 additions & 12 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/types": patch
3+
---
4+
5+
`createHostImporter` stops prescribing an install repair for a `link:` / `file:` install that is already correct. The refusal is unchanged; only its wording is.
6+
7+
A host app declaring `{"foo": "link:../bar"}` links `node_modules/foo` to a directory whose manifest may be named anything. `link:`, `file:` and git or tarball URLs name a LOCATION or a remote artefact, never a package, so the specifier carries no name for the ESM-only fallback finder to expect and the KEY stays the expectation — kept deliberately, because widening it would accept any directory sitting at the key and trade a wrong REMEDY for a wrong LOAD. When the linked manifest names something else the finder therefore refuses, and it was reporting that refusal with the `declared-unresolvable` INSTALL wording: run `pnpm install`, check a production prune did not drop it, check the dist was built. Driven on a real symlinked install, all three are measurably false — the finder had just read the manifest at `node_modules/foo`, so the package is on disk, was not pruned, and its `import` target exists. The operator reinstalls, nothing changes, and they go looking for a build that is not broken.
8+
9+
That sub-case now states what was actually measured: the directory it consulted, the name the manifest there carries, the name it expected, and why a location specifier leaves it with only the key. It says outright that this is neither an install nor a declaration problem, and closes with the remedy that does work — make the two names agree, by declaring the linked package under its own name or by renaming the linked manifest to the key. Both ends are pinned as loading.
10+
11+
Unchanged: the refusal itself, its `declared-unresolvable` kind, its `MODULE_NOT_FOUND` code and every consumer branch that reads them; the finder's accept set, which is byte-for-byte what it was — a `link:` install whose manifest matches the key still loads silently, and a plain range or an `npm:` alias whose directory holds a different package still gets the INSTALL wording, because there the install really is the fault. The second verification axis that would make these installs LOAD (comparing `realpath(node_modules/<key>)` against the declared location) is deliberately not built here.

packages/types/src/node.test.ts

Lines changed: 222 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
*/
2828

2929
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
30-
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
30+
import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
3131
import * as NodeModule from 'node:module';
3232
import { tmpdir } from 'node:os';
3333
import { dirname, join } from 'node:path';
@@ -1472,13 +1472,18 @@ describe('an aliased install is verified against the name its DECLARATION names
14721472
// direction (refuse, never load the wrong thing) is kept rather than
14731473
// guessed at — widening it here would make the finder looser than the
14741474
// manifest-name check exists to be.
1475+
//
1476+
// ⚠️ This pin asserted the INSTALL wording until #15045. The REFUSAL is
1477+
// what #14278 declared and it is unchanged — same kind, same throw; only
1478+
// the words changed, because the install this message described was
1479+
// already correct. The wording itself is pinned in the #15045 suite below.
14751480
const root = app('link-mismatch', 'linked-other', 'link:../elsewhere');
14761481
installAs(root, 'linked-other', '@fixture/some-other-name', { exports: ESM_ONLY_EXPORTS }, {
14771482
'dist/index.js': "export const BUILD = 'other';\n",
14781483
});
14791484
const err = await createHostImporter(root)('linked-other').catch((e: unknown) => e);
14801485
expect(hostImportFailureKind(err)).toBe('declared-unresolvable');
1481-
expect((err as Error).message).toMatch(/INSTALL problem/);
1486+
expect((err as Error).message).not.toMatch(/INSTALL problem/);
14821487
});
14831488

14841489
it('TIGHTNESS: an alias naming one package does not license a directory holding another', async () => {
@@ -1862,3 +1867,218 @@ exports.BUILD = 'cjs';
18621867
expect((await importer(root)('linked-other')).BUILD).toBe('linked-cjs');
18631868
});
18641869
});
1870+
1871+
/**
1872+
* ── #15045: the location sub-case REFUSES correctly and EXPLAINED itself wrongly ─
1873+
*
1874+
* #14278 left one sub-case standing on the fallback leg, deliberately and with
1875+
* a pin: `link:` / `file:` (and a git or tarball URL) name a LOCATION, not a
1876+
* package, so the declaration carries no name for the finder to expect and the
1877+
* KEY stays the expectation. A linked package whose manifest says something
1878+
* else is therefore refused.
1879+
*
1880+
* The refusal is right — the alternative is loading a directory the host never
1881+
* named. What was wrong is what it SAID. Driven on a real symlinked `link:`
1882+
* install, the message read:
1883+
*
1884+
* This is an INSTALL problem, not a declaration problem ...
1885+
* • dependencies never installed ... -> run `pnpm install` in <app>
1886+
* • a production prune / filtered deploy dropped it
1887+
* • it IS installed but its "main"/"exports" points at a dist that was
1888+
* never built
1889+
*
1890+
* Every one of those is measurably FALSE for this shape: the finder had just
1891+
* READ the manifest at `node_modules/<key>`, so the package is on disk, was not
1892+
* pruned, and its `import` target exists. The operator runs `pnpm install`,
1893+
* nothing changes, and they go hunting for a build that is not broken.
1894+
*
1895+
* ⛔ What this suite does NOT pin, because it was NOT built: the second
1896+
* verification axis the card also proposes (comparing
1897+
* `realpath(node_modules/<key>)` against the declared location), which would
1898+
* make these installs LOAD. That relaxes the finder's accept set and is a
1899+
* contract decision; the card stays open for it. The tests below assert the
1900+
* opposite — that the refusal still fires on exactly the inputs it fired on
1901+
* before.
1902+
*/
1903+
describe('a location install whose manifest differs states the LIMIT, not a false remedy (#15045)', () => {
1904+
const bases: string[] = [];
1905+
1906+
afterAll(() => {
1907+
for (const dir of bases) rmSync(dir, { recursive: true, force: true });
1908+
});
1909+
1910+
/** The card's exact shape: `import` condition only, no `require`, no `main`. */
1911+
const ESM_ONLY_EXPORTS = { '.': { import: './dist/index.js' } };
1912+
1913+
/**
1914+
* A REAL location install: `node_modules/<key>` is a SYMLINK to a sibling
1915+
* directory, which is what `link:` and a directory `file:` actually produce.
1916+
*
1917+
* #14278's fixtures above install a plain directory instead — correct there,
1918+
* since the finder reads `node_modules/<key>` and realpaths only afterwards,
1919+
* so both shapes exercise one code path. This card is ABOUT the location
1920+
* shape, so it drives the on-disk shape an operator would really have.
1921+
*/
1922+
function linkedApp(
1923+
tag: string,
1924+
key: string,
1925+
specifier: string,
1926+
manifestName: string,
1927+
exportsField: unknown = ESM_ONLY_EXPORTS,
1928+
): string {
1929+
const base = mkdtempSync(join(tmpdir(), `os-loc-${tag}-`));
1930+
bases.push(base);
1931+
const root = join(base, 'app');
1932+
const linked = join(base, 'elsewhere');
1933+
mkdirSync(join(root, 'node_modules'), { recursive: true });
1934+
mkdirSync(join(linked, 'dist'), { recursive: true });
1935+
writeFileSync(
1936+
join(root, 'package.json'),
1937+
JSON.stringify({ name: 'location-host-fixture', type: 'module', dependencies: { [key]: specifier } }),
1938+
'utf8',
1939+
);
1940+
writeFileSync(
1941+
join(linked, 'package.json'),
1942+
JSON.stringify({ name: manifestName, version: '0.0.0-fixture', type: 'module', exports: exportsField }),
1943+
'utf8',
1944+
);
1945+
writeFileSync(join(linked, 'dist', 'index.js'), `export const BUILD = ${JSON.stringify(manifestName)};\n`, 'utf8');
1946+
const at = join(root, 'node_modules', ...key.split('/'));
1947+
mkdirSync(dirname(at), { recursive: true });
1948+
symlinkSync(linked, at, 'dir');
1949+
return root;
1950+
}
1951+
1952+
const refusalFor = async (root: string, spec: string): Promise<Error> =>
1953+
(await createHostImporter(root)(spec).catch((e: unknown) => e)) as Error;
1954+
1955+
it('PRECONDITION: the symlinked ESM-only install reaches the fallback at all', () => {
1956+
// Without this, everything below could be passing for the wrong reason:
1957+
// the CJS resolver must FIND the symlink and refuse on the CONDITION, so
1958+
// the fallback inside that catch is what decides.
1959+
const root = linkedApp('precondition', 'linked-other', 'link:../elsewhere', '@fixture/some-other-name');
1960+
let code: string | undefined;
1961+
try {
1962+
createHostRequire(root).resolve('linked-other');
1963+
} catch (e) {
1964+
code = (e as { code?: string }).code;
1965+
}
1966+
expect(code).toBe('ERR_PACKAGE_PATH_NOT_EXPORTED');
1967+
});
1968+
1969+
it('THE CARD: the refusal is UNCHANGED — same kind, same code, still not loaded', async () => {
1970+
// The half that must not move. A diff that turned this into a load would
1971+
// have left the card's scope whatever its message said.
1972+
const root = linkedApp('refusal', 'linked-other', 'link:../elsewhere', '@fixture/some-other-name');
1973+
const err = await refusalFor(root, 'linked-other');
1974+
expect(err).toBeInstanceOf(Error);
1975+
expect(hostImportFailureKind(err)).toBe('declared-unresolvable');
1976+
expect((err as unknown as { code?: string }).code).toBe('MODULE_NOT_FOUND');
1977+
});
1978+
1979+
it('THE CARD: the three false remedies are gone', async () => {
1980+
// Asserted as ABSENCES of the old message's load-bearing claims, not as
1981+
// punctuation. Each was measurably false for this shape.
1982+
const root = linkedApp('remedies-gone', 'linked-other', 'link:../elsewhere', '@fixture/some-other-name');
1983+
const { message } = await refusalFor(root, 'linked-other');
1984+
expect(message).not.toMatch(/INSTALL problem, not a declaration problem/);
1985+
expect(message).not.toMatch(/dependencies never installed/);
1986+
expect(message).not.toMatch(/production prune/);
1987+
expect(message).not.toMatch(/points at a dist that was never built/);
1988+
});
1989+
1990+
it('THE CARD: it states the MEASUREMENT — what is installed, and what was expected', async () => {
1991+
const root = linkedApp('measurement', 'linked-other', 'link:../elsewhere', '@fixture/some-other-name');
1992+
const { message } = await refusalFor(root, 'linked-other');
1993+
// The two names it compared, both present, so the operator can see the
1994+
// mismatch rather than infer it.
1995+
expect(message).toMatch(/its package\.json is named: "@fixture\/some-other-name"/);
1996+
expect(message).toMatch(/this finder expected: "linked-other"/);
1997+
// And the directory it read them from.
1998+
expect(message).toMatch(/installed at: .*node_modules\/linked-other/);
1999+
});
2000+
2001+
it('THE CARD: it names the LIMIT — a location specifier carries no name to expect', async () => {
2002+
const root = linkedApp('limit', 'linked-other', 'link:../elsewhere', '@fixture/some-other-name');
2003+
const { message } = await refusalFor(root, 'linked-other');
2004+
expect(message).toMatch(/NOT an install problem/);
2005+
expect(message).toMatch(/names a LOCATION, not a package/);
2006+
// The card asked for the git / tarball sentence: they carry no on-disk
2007+
// location either and land in exactly this sub-case.
2008+
expect(message).toMatch(/tarball URL/);
2009+
// ⚠️ `pnpm install` IS mentioned, inside the sentence that says it changes
2010+
// nothing. Pinned as a PRESENCE so nobody later "fixes" the mention by
2011+
// deleting the one line that stops the operator's reflex.
2012+
expect(message).toMatch(/re-running `pnpm install`.*change nothing/s);
2013+
});
2014+
2015+
it('THE CARD: the remedy it prints WORKS — renaming either end makes it load', async () => {
2016+
// The message tells the operator to make the two names agree. Both ends
2017+
// are pinned, because a printed remedy nobody measured is the class of
2018+
// defect this card is about.
2019+
const byKey = linkedApp('remedy-key', '@fixture/some-other-name', 'link:../elsewhere', '@fixture/some-other-name');
2020+
expect((await createHostImporter(byKey)('@fixture/some-other-name')).BUILD).toBe('@fixture/some-other-name');
2021+
2022+
const byManifest = linkedApp('remedy-manifest', 'linked-other', 'link:../elsewhere', 'linked-other');
2023+
expect((await createHostImporter(byManifest)('linked-other')).BUILD).toBe('linked-other');
2024+
});
2025+
2026+
it('NEGATIVE CONTROL: a `link:` install whose manifest MATCHES the key still loads, silently', async () => {
2027+
// The load path is untouched. If this ever reddens, the change stopped
2028+
// being a wording change.
2029+
const root = linkedApp('control', 'linked', 'link:../elsewhere', 'linked');
2030+
expect((await createHostImporter(root)('linked')).BUILD).toBe('linked');
2031+
});
2032+
2033+
it('the same wording covers `file:`, a tarball URL and the bare `owner/repo` shorthand', async () => {
2034+
// One fact, four spellings: none of them names a package. `file:` and
2035+
// `link:` are the two location protocols; a git or tarball URL names no
2036+
// on-disk location at all and installs under the key with whatever the
2037+
// published manifest carries.
2038+
for (const [tag, specifier] of [
2039+
['file', 'file:../elsewhere'],
2040+
['tarball', 'https://example.invalid/pkg.tgz'],
2041+
['github', 'github:acme/bar'],
2042+
['shorthand', 'acme/bar'],
2043+
] as const) {
2044+
const root = linkedApp(tag, 'located-other', specifier, '@fixture/some-other-name');
2045+
const { message } = await refusalFor(root, 'located-other');
2046+
expect(message, `${specifier} should get the location wording`).toMatch(/NOT an install problem/);
2047+
expect(message, `${specifier} should not keep the install remedy`).not.toMatch(/INSTALL problem/);
2048+
}
2049+
});
2050+
2051+
it('TIGHTNESS: a location declaration with NOTHING installed keeps the INSTALL wording', async () => {
2052+
// The split is real, not a blanket re-wording of `link:`. Here the install
2053+
// genuinely IS the problem and the old remedies are the right ones.
2054+
const base = mkdtempSync(join(tmpdir(), 'os-loc-absent-'));
2055+
bases.push(base);
2056+
const root = join(base, 'app');
2057+
mkdirSync(root, { recursive: true });
2058+
writeFileSync(
2059+
join(root, 'package.json'),
2060+
JSON.stringify({ name: 'h', type: 'module', dependencies: { 'linked-gone': 'link:../nowhere' } }),
2061+
'utf8',
2062+
);
2063+
const { message } = await refusalFor(root, 'linked-gone');
2064+
expect(message).toMatch(/INSTALL problem, not a declaration problem/);
2065+
});
2066+
2067+
it('TIGHTNESS: a plain RANGE with a mismatched directory keeps the INSTALL wording', async () => {
2068+
// The predicate is on the DECLARATION, not on the on-disk shape: the same
2069+
// symlinked mismatch under `^1.0.0` is a registry install that landed
2070+
// wrong, and `pnpm install` is exactly the remedy for it. A red here would
2071+
// mean the re-wording leaked out of the location sub-case.
2072+
const root = linkedApp('range-mismatch', '@fixture/plain-range', '^1.0.0', '@fixture/somebody-else');
2073+
const { message } = await refusalFor(root, '@fixture/plain-range');
2074+
expect(message).toMatch(/INSTALL problem, not a declaration problem/);
2075+
});
2076+
2077+
it('TIGHTNESS: an `npm:` alias naming one package still refuses another as an INSTALL fault', async () => {
2078+
// `npm:` DOES name a package, so the key is not the expectation and a
2079+
// directory holding a third name is a broken install, not a limit.
2080+
const root = linkedApp('alias-mismatch', 'aliased', 'npm:@fixture/declared@1', '@fixture/installed');
2081+
const { message } = await refusalFor(root, 'aliased');
2082+
expect(message).toMatch(/INSTALL problem, not a declaration problem/);
2083+
});
2084+
});

0 commit comments

Comments
 (0)