Skip to content

Commit 6e3462d

Browse files
os-justinclaude
andauthored
feat(types): verify a link:/file: install by the LOCATION its host declared (#17542)
* feat(types): verify a link:/file: install by the location the host declared WIP checkpoint: second verification axis + tests, before the closure build. Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt Co-authored-by: Claude <noreply@anthropic.com> * fix(cli): defer the multi-org declared-unresolvable remedy to the cause Also updates content/docs/deployment/tenancy-modes.mdx for the two shapes the declared-unresolvable kind now covers, and adds both changesets. Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2662a08 commit 6e3462d

7 files changed

Lines changed: 778 additions & 120 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/types': minor
3+
---
4+
5+
Host importer: a `link:` / `file:` install is now verified by the LOCATION the app declared, so a correctly linked package loads instead of being refused.
6+
7+
The ESM fallback finder (`createHostImporter`) verifies the one directory it consults — `<hostRoot>/node_modules/<key>` — against what the host's own `package.json` declares. Until now it could only do that by NAME, and a `link:` / `file:` value promises no name, so the KEY stood in for one: a package linked exactly as the app asked, whose own manifest happens to be named something else, was refused with `declared-unresolvable` / `MODULE_NOT_FOUND`. Nothing was broken, and the only way out was to stop using a supported linking mode.
8+
9+
Such a declaration does name something checkable — a directory — so the finder now checks that too: `realpath(node_modules/<key>)` against `realpath(resolve(hostRoot, <declared path>))`, both sides canonicalised, compared exactly (no basename matching, no case folding). If they are the same directory, the host declared it and it loads.
10+
11+
This is a second verification axis, not a looser first one. A directory the app declared neither by name nor by path is refused exactly as before, and the finder stays strictly tighter than the CommonJS resolution it backs up, which asks neither question. Unchanged: a plain version range licenses no path; an `npm:` alias is still checked by name; `github:` / tarball URLs and the bare `owner/repo` shorthand name no on-disk location, so they gain nothing; a package that publishes a `require` condition never reaches this fallback at all, so no load that succeeds today changes.
12+
13+
Measured on pnpm 10.33: `link:` symlinks the key at the declared directory and verifies; a `file:` directory install routes through pnpm's virtual store (a copy), so it does not, and keeps today's refusal. The refusal's text now states what the location check compared instead of asserting a limit the finder no longer has.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
`serve`: the multi-org runtime's stage-1 refusal no longer prints its own install remedy for a `declared-unresolvable` failure — it defers to the importer's message, which the same refusal already prints as its `cause:` line.
6+
7+
Driven on both shapes that kind covers, the minted bullet ("Repair the INSTALL … run `pnpm install`, check that a production prune did not drop it, and that its dist is actually built") was wrong twice over. For a genuinely broken install it repeated, word for word, the three remedies the cause line four lines below already carried. For a location install the finder cannot tie to the declaration, the cause says outright that re-running `pnpm install`, un-pruning a deploy and rebuilding a dist all change nothing — so one screen contradicted itself.
8+
9+
The arm now says only what it uniquely knows (the app DOES declare the package, so re-reading `package.json` will not help) and names the cause as the authority on the remedy — the same deferral the `declared-no-loadable-entry` arm has had since it landed.

content/docs/deployment/tenancy-modes.mdx

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -257,8 +257,26 @@ So, three ways out:
257257
- **set `OS_ALLOW_DEGRADED_TENANCY=1`** to boot anyway in an explicitly degraded
258258
single-org state.
259259

260-
If the package is already declared but the install is broken, the message says so
261-
and tells you to repair the install rather than re-read `package.json`.
260+
If the package is already declared, the remedy bullet says only that — the
261+
declaration is not the problem — and hands the remedy to the `cause:` line
262+
below it, which is the one thing that measured what actually went wrong. There
263+
are two shapes of that, and they want opposite actions:
264+
265+
- **the install is broken** — never installed, pruned away by a filtered deploy,
266+
or pointing at a dist that was never built. The cause names all three and
267+
tells you to repair the install rather than re-read `package.json`.
268+
- **the install is fine and the runtime still could not be verified** — the app
269+
declares the package under a `link:` / `file:` path, or a git or tarball URL,
270+
and the package sitting at that key carries a different `name` in its own
271+
`package.json`. This is neither an install nor a declaration fault, and the
272+
cause says so outright: re-running `pnpm install` changes nothing. A `link:`
273+
install pointing at exactly the directory the declaration names is verified by
274+
that path and loads normally; what reaches this message is the residue where
275+
neither the name nor the path ties the directory to the declaration — a
276+
`file:` directory install under pnpm (which lands in the virtual store rather
277+
than at the declared path), or a URL that names no on-disk location at all.
278+
The remedy the cause prints is to make the two names agree, from whichever end
279+
you own.
262280

263281
### A present-but-refusing runtime is a different failure
264282

packages/cli/src/commands/serve-organizations-message-spelling.test.ts

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,20 +157,49 @@ const DECLARED: HostDeclaration = {
157157
};
158158
const UNDECLARED: HostDeclaration = { packageName: PKG, hostRoot: '/srv/app', declared: false };
159159

160-
const remedyDeclared = () => formatOrganizationsInstallRemedy('declared-unresolvable', DECLARED, '/srv/app');
161-
const remedyUndeclared = () => formatOrganizationsInstallRemedy('undeclared', UNDECLARED, '/srv/app');
162-
const remedyNoLoadableEntry = () =>
163-
formatOrganizationsInstallRemedy('declared-no-loadable-entry', DECLARED, '/srv/app');
160+
const remedyDeclared = () => formatOrganizationsInstallRemedy('declared-unresolvable', DECLARED);
161+
const remedyUndeclared = () => formatOrganizationsInstallRemedy('undeclared', UNDECLARED);
162+
const remedyNoLoadableEntry = () => formatOrganizationsInstallRemedy('declared-no-loadable-entry', DECLARED);
164163

165164
describe('serve — the multi-org runtime name an operator READS comes from the declaration (#12151)', () => {
166-
it('site 1 — the "install is broken" remedy names it, with the spacing intact', () => {
167-
// The `declared-unresolvable` branch: the app's package.json IS correct and
168-
// the install is what broke (#4719). One of the two an operator ACTS on.
165+
it('site 1 — the "the declaration is not the problem" remedy names it, with the spacing intact', () => {
166+
// The `declared-unresolvable` branch: the app's package.json IS correct
167+
// (#4719). One of the two an operator ACTS on.
169168
expect(lines(remedyDeclared())[0]).toBe(
170169
` • this app DECLARES ${PKG} (dependencies: "^1.2.3") — the`,
171170
);
172171
});
173172

173+
it('site 1b — the `declared-unresolvable` remedy DEFERS too; it mints no install advice (#17046)', () => {
174+
// ── The arm that used to fire, quoted so the flip is legible ──────────
175+
//
176+
// Repair the INSTALL in /srv/app: run `pnpm install`, check that a
177+
// production prune did not drop it, and that its dist is actually built — or
178+
//
179+
// DRIVEN on both shapes this one kind covers, that text was wrong twice:
180+
// for a genuinely broken install it repeats, word for word, the three
181+
// remedies `unresolvableMessage` already prints in the `cause:` line four
182+
// lines below; and for #15045's location sub-case — narrowed by #17046 but
183+
// NOT removed, since pnpm's `file:` virtual-store copy and every git /
184+
// tarball declaration still reach it — the same `cause:` says outright
185+
// that re-running `pnpm install`, un-pruning and rebuilding change
186+
// nothing. One screen contradicting itself, the #14270 class.
187+
const rendered = plain(remedyDeclared());
188+
expect(rendered).not.toContain('Repair the INSTALL');
189+
expect(rendered).not.toContain('pnpm install');
190+
expect(rendered).not.toContain('production prune');
191+
expect(rendered).not.toContain('dist is actually built');
192+
// ⛔ Nor may it drift into the OTHER arm's instruction.
193+
expect(rendered).not.toContain("declare it in the app's package.json");
194+
// What it says instead: the declaration is fine, and the cause below owns
195+
// the remedy — the same DEFERRAL site 2b pins for the sibling kind.
196+
expect(rendered).toContain('declaration is NOT the problem');
197+
expect(rendered).toContain('the cause below names the');
198+
expect(rendered).toContain('authority on');
199+
// It still chains into the `Fix one of:` list the fatal assembles.
200+
expect(rendered.endsWith(' — or\n')).toBe(true);
201+
});
202+
174203
it('site 2 — the "add it to THIS APP" remedy names it, with the spacing intact', () => {
175204
// The other instruction an operator acts on: the app never declared it.
176205
expect(lines(remedyUndeclared())[0]).toBe(

packages/cli/src/commands/serve.ts

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3770,7 +3770,6 @@ export default class Serve extends Command {
37703770
const remedy = formatOrganizationsInstallRemedy(
37713771
hostImportFailureKind(orgErr),
37723772
declaration,
3773-
hostRoot,
37743773
);
37753774
console.error(
37763775
chalk.red(formatOrganizationsAbsentFatal(tenancyPosture, remedy, cause)),
@@ -5404,9 +5403,26 @@ export function formatI18nLoadDiagnostic(pkg: string, err: unknown): string {
54045403
*
54055404
* The question each arm answers is **"is the declaration the problem?"**:
54065405
*
5407-
* - `declared-unresolvable` — the app's `package.json` DOES name the package
5408-
* and the INSTALL is what is broken. Telling that operator to re-read a
5406+
* - `declared-unresolvable` — the app's `package.json` DOES name the package,
5407+
* so the DECLARATION is not the problem. Telling that operator to re-read a
54095408
* file that is already correct is the defect this branch exists to avoid.
5409+
* ⚠️ It used to mint an install remedy here too — "Repair the INSTALL: run
5410+
* `pnpm install`, check that a production prune did not drop it, and that
5411+
* its dist is actually built". DRIVEN, on both shapes this kind covers, it
5412+
* was wrong twice over (#17046):
5413+
* · a genuinely broken install already gets those three remedies, word
5414+
* for word, from `unresolvableMessage` in the `cause:` line printed
5415+
* four lines below — so the bullet was a second copy to drift;
5416+
* · the #15045 sub-case — a location install this finder cannot tie to
5417+
* the declaration — prints a `cause:` that says outright "This is NOT
5418+
* an install problem … re-running `pnpm install`, un-pruning a deploy
5419+
* and rebuilding a dist all change nothing here", so the two halves of
5420+
* one screen contradicted each other. #17046 narrowed that sub-case
5421+
* (a correctly linked package now LOADS) but did NOT remove it: pnpm's
5422+
* `file:` virtual-store copy and every git / tarball declaration still
5423+
* land here.
5424+
* So this arm now says only what it uniquely knows — the declaration — and
5425+
* defers the remedy, exactly as the next one does.
54105426
* - `declared-no-loadable-entry` (#14041) — the app declares it, the install
54115427
* DELIVERED it, and the package's own `exports` names no runtime entry
54125428
* Node can load. Neither absence applies: there is nothing to declare and
@@ -5434,15 +5450,15 @@ export function formatI18nLoadDiagnostic(pkg: string, err: unknown): string {
54345450
export function formatOrganizationsInstallRemedy(
54355451
kind: HostImportFailureKind | undefined,
54365452
declaration: HostDeclaration,
5437-
hostRoot: string,
54385453
): string {
54395454
const pkg = Serve.ORGANIZATIONS_RUNTIME_PKG;
54405455
if (kind === 'declared-unresolvable') {
54415456
return ` • this app DECLARES ${pkg} ` +
54425457
`(${declaration.field}: ${JSON.stringify(declaration.specifier)}) — the\n` +
54435458
' declaration is NOT the problem and re-reading package.json will not help.\n' +
5444-
` Repair the INSTALL in ${hostRoot}: run \`pnpm install\`, check that a\n` +
5445-
' production prune did not drop it, and that its dist is actually built — or\n';
5459+
' What IS wrong was measured by the importer: the cause below names the\n' +
5460+
' directory it consulted and what it found there, and is the authority on\n' +
5461+
' whether any install action can change it — or\n';
54465462
}
54475463
if (kind === 'declared-no-loadable-entry') {
54485464
return ` • this app DECLARES ${pkg} ` +

0 commit comments

Comments
 (0)