Skip to content

Commit 5cf1c88

Browse files
os-trumpclaude
andauthored
fix(cli,verify,dogfood): branch the multi-org remedy on the ABSENCE, not on one failure kind (#15158)
* test(cli,verify,dogfood): pin the third failure kind's remedy at all three sites Red-first for #14270. #14041 added a third `HostImportFailureKind`, `declared-no-loadable-entry` — the app declares the package, the install delivered it, and the package's own `exports` names no runtime entry Node can load. Three consumers pick their remedy with a two-way branch written when only two kinds existed, so the third falls into the else leg and renders the DECLARE remedy at an operator who has already declared AND installed it. Each site gets a fixture carrying that kind and an assertion naming which arm fired. The verify and dogfood fixtures are real host roots on disk whose installed package publishes a `types`-only `exports` map; the CLI formatter takes the kind directly. Controls for `undeclared` and `declared-unresolvable` are untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza * fix(cli,verify,dogfood): branch the multi-org remedy on the ABSENCE, not on one kind `declared-no-loadable-entry` (#14041) means the app declares the package, the install delivered it, and the package's own `exports` names no runtime entry Node can load. All three consumers picked their remedy with a two-way branch written when only two kinds existed, so that kind fell into the else leg and printed "declare it in the app's package.json and install" — at an operator who has already done both, and directly contradicting the importer's own message printed beside it. Each branch now asks "is the declaration the problem?": undeclared keeps the declare remedy, `declared-unresolvable` keeps the install remedy, and the third kind prescribes nothing and defers to the importer's message, which every one of the three sites already interpolates. No fourth remedy sentence is minted — the importer words the package-shape case once, and a local re-wording would be a second copy to drift. `packages/types` is untouched: the strings deferred to are #14278's. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza * chore(changeset): the multi-org remedy stops re-prescribing a declaration the app already has Patch for @objectstack/cli and @objectstack/verify — the two published packages whose `files` field ships the changed code. @objectstack/dogfood is `private: true` with no `files`, so it publishes nothing and takes no changeset entry. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza * test(verify): keep the new cases off `@objectstack/verify`'s frozen TEST_DEBT `harness.host-resolution.test.ts` imports `./harness` without the `.js` extension, so under NodeNext the specifier does not resolve and every symbol it names is `any` — which makes a `.then` callback PARAMETER implicitly any. The two new cases used `.then(onFulfilled, onRejected)` and added 2 x TS7006, taking the package's TEST_DEBT entry from its recorded 3 to 5. That ledger is shrink-only, so the fix is the errors, not the ceiling. try/catch instead: a `const` initialised from an `any` call is not a diagnostic, only a parameter is. `check:type-check-debt --re-measure` now reports 17 entries re-measured, 217 raw errors, none above its recorded number. ⛔ Deliberately NOT the one-line `.js` fix that graduates the entry — that is a different card's, filed separately. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8422acd commit 5cf1c88

7 files changed

Lines changed: 369 additions & 44 deletions

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/cli": patch
3+
"@objectstack/verify": patch
4+
---
5+
6+
fix(cli,verify): the multi-org runtime's remedy stops telling an operator to declare a package the app already declares
7+
8+
The three consumers that turn a failed `@objectstack/organizations` import into
9+
operator-facing advice picked their remedy with a two-way branch, written when
10+
`HostImportFailureKind` had exactly two members. A third one — a package the app
11+
DECLARES, that the install DELIVERED, and whose own `exports` names no runtime
12+
entry Node can load (a `types`-only or `browser`-only publish, or an unexported
13+
subpath) — fell into the else leg and rendered the *declare* remedy:
14+
15+
- `objectstack serve`'s ADR-0093 D5 stage-1 fatal ("add the package to THIS APP
16+
— declare it in the app's package.json and install"),
17+
- `bootStack({ multiTenant: true })`'s refusal ("Install/link it in THIS APP —
18+
and DECLARE it in that app's package.json"),
19+
- the enterprise dogfood probe's skip reason and its `MULTI_ORG=1` throw.
20+
21+
Each of those three also prints the importer's own message, which words the case
22+
correctly — so the bullet an operator reads first contradicted the diagnosis
23+
printed underneath it, and prescribed two actions (declare it, install it) that
24+
were already done and could not have helped: no edit to the app and no install
25+
action changes what a package publishes.
26+
27+
Every branch now asks *is the declaration the problem?* rather than testing one
28+
kind. `undeclared` keeps the declare remedy and `declared-unresolvable` keeps
29+
the install remedy, both byte-for-byte unchanged. The third kind prescribes
30+
nothing: it names the two things that are NOT the problem and defers to the
31+
importer's message, which already states that the remedy lives in the package
32+
and what the package has to publish. No fourth remedy sentence is minted — one
33+
wording of the package-shape case, in the one place that measured it.

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@ const UNDECLARED: HostDeclaration = { packageName: PKG, hostRoot: '/srv/app', de
137137

138138
const remedyDeclared = () => formatOrganizationsInstallRemedy('declared-unresolvable', DECLARED, '/srv/app');
139139
const remedyUndeclared = () => formatOrganizationsInstallRemedy('undeclared', UNDECLARED, '/srv/app');
140+
const remedyNoLoadableEntry = () =>
141+
formatOrganizationsInstallRemedy('declared-no-loadable-entry', DECLARED, '/srv/app');
140142

141143
describe('serve — the multi-org runtime name an operator READS comes from the declaration (#12151)', () => {
142144
it('site 1 — the "install is broken" remedy names it, with the spacing intact', () => {
@@ -154,6 +156,40 @@ describe('serve — the multi-org runtime name an operator READS comes from the
154156
);
155157
});
156158

159+
it('site 2b — the `declared-no-loadable-entry` remedy DEFERS, it does not re-prescribe (#14270)', () => {
160+
// ── The arm that used to fire, quoted so the flip is legible ──────────
161+
// This formatter was a TWO-WAY branch written when only two kinds existed
162+
// (`declared-unresolvable` vs everything else). #14041 added a third —
163+
// the app DECLARES the package, the install DELIVERED it, and the
164+
// package's own `exports` names no runtime entry Node can load — and it
165+
// fell into the else leg, rendering the UNDECLARED remedy verbatim:
166+
//
167+
// • add @objectstack/organizations (the enterprise multi-org runtime) to THIS APP
168+
// — declare it in the app's package.json and install; the CLI resolves it from the
169+
//
170+
// i.e. "declare it and install it" to an operator who has already done
171+
// both. That superseded wording is what site 2 above still pins for the
172+
// kind it is actually right for; here it is the confidently-wrong verdict
173+
// #14041 exists to remove, one layer up.
174+
expect(lines(remedyNoLoadableEntry())[0]).toBe(
175+
` • this app DECLARES ${PKG} (dependencies: "^1.2.3") and it IS`,
176+
);
177+
178+
const rendered = plain(remedyNoLoadableEntry());
179+
// ⛔ NEITHER of the other two arms' instructions may appear. Both are
180+
// unfollowable for this kind, and each contradicts the importer's own
181+
// message, which is printed as the `cause:` line of the same refusal.
182+
expect(rendered).not.toContain("declare it in the app's package.json");
183+
expect(rendered).not.toContain('Repair the INSTALL');
184+
// What it says instead: where the remedy lives, and that the importer's
185+
// message below is the authority on it — a DEFERRAL, deliberately not a
186+
// fourth remedy sentence minted here (and in two other consumers).
187+
expect(rendered).toContain('publishes no entry Node can load');
188+
expect(rendered).toContain('the cause below is the authority');
189+
// It still chains into the `Fix one of:` list the fatal assembles.
190+
expect(rendered.endsWith(' — or\n')).toBe(true);
191+
});
192+
157193
it('site 3 — the ADR-0093 D5 fatal refusal names it', () => {
158194
const fatal = formatOrganizationsAbsentFatal('isolated', remedyUndeclared(), 'Cannot find package');
159195
// A leading blank line separates the refusal from whatever boot last printed.
@@ -188,6 +224,7 @@ describe('serve — the multi-org runtime name an operator READS comes from the
188224
for (const [label, rendered] of [
189225
['remedy (declared)', remedyDeclared()],
190226
['remedy (undeclared)', remedyUndeclared()],
227+
['remedy (no loadable entry)', remedyNoLoadableEntry()],
191228
['stage-1 fatal', formatOrganizationsAbsentFatal('group', remedyUndeclared(), 'ERR_MODULE_NOT_FOUND')],
192229
['degraded warning', formatDegradedTenancyWarning('group')],
193230
['stage-2 fatal', formatOrganizationsMountFatal('group', 'refused', undefined)],

packages/cli/src/commands/serve.ts

Lines changed: 53 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5225,34 +5225,69 @@ export function formatI18nLoadDiagnostic(pkg: string, err: unknown): string {
52255225
*/
52265226

52275227
/**
5228-
* The install remedy bullet for a multi-org runtime that would not load — two
5229-
* absences, two remedies (#4719).
5230-
*
5231-
* `declared-unresolvable` means the app's `package.json` DOES name the package
5232-
* and the install is what is broken; anything else means the app never declared
5233-
* it. Telling the first operator to re-read a file that is already correct is
5234-
* the defect this branch exists to avoid.
5228+
* The install remedy bullet for a multi-org runtime that would not load — one
5229+
* bullet per ABSENCE, and the absences are what the branch reads (#4719).
5230+
*
5231+
* ## What the branch is ON, and why it is not a kind list
5232+
*
5233+
* The question each arm answers is **"is the declaration the problem?"**:
5234+
*
5235+
* - `declared-unresolvable` — the app's `package.json` DOES name the package
5236+
* and the INSTALL is what is broken. Telling that operator to re-read a
5237+
* file that is already correct is the defect this branch exists to avoid.
5238+
* - `declared-no-loadable-entry` (#14041) — the app declares it, the install
5239+
* DELIVERED it, and the package's own `exports` names no runtime entry
5240+
* Node can load. Neither absence applies: there is nothing to declare and
5241+
* nothing to install, so this arm prescribes NOTHING and defers to the
5242+
* importer's own message, which `formatOrganizationsAbsentFatal` prints
5243+
* verbatim as the `cause:` line of the same refusal (#14270).
5244+
* - anything else — the app never declared it, so the declare-and-install
5245+
* remedy is the one that works.
5246+
*
5247+
* ⚠️ This was a TWO-WAY branch written when only two kinds existed, so the
5248+
* third fell into the else leg and told an operator whose app already declares
5249+
* AND installs the package to declare and install it — the
5250+
* confidently-wrong-verdict class #14041 removed one layer down, reintroduced
5251+
* by its consumer. ⛔ The third arm deliberately does NOT mint a fourth remedy
5252+
* sentence: the importer already words the package-shape case, and a local
5253+
* re-wording here would be a second copy to drift (the same reason
5254+
* {@link formatI18nLoadDiagnostic} interpolates only the kind TOKEN, and the
5255+
* same shape as {@link formatOrganizationsMountFatal}'s "its message is the
5256+
* authority on the remedy").
5257+
*
5258+
* An error carrying NO kind never came from the host importer at all and keeps
5259+
* the declare-and-install arm it has always had — narrowing that one is a
5260+
* different question, about a different fact, and is not this branch's.
52355261
*/
52365262
export function formatOrganizationsInstallRemedy(
52375263
kind: HostImportFailureKind | undefined,
52385264
declaration: HostDeclaration,
52395265
hostRoot: string,
52405266
): string {
52415267
const pkg = Serve.ORGANIZATIONS_RUNTIME_PKG;
5242-
return kind === 'declared-unresolvable'
5243-
? ` • this app DECLARES ${pkg} ` +
5268+
if (kind === 'declared-unresolvable') {
5269+
return ` • this app DECLARES ${pkg} ` +
52445270
`(${declaration.field}: ${JSON.stringify(declaration.specifier)}) — the\n` +
52455271
' declaration is NOT the problem and re-reading package.json will not help.\n' +
52465272
` Repair the INSTALL in ${hostRoot}: run \`pnpm install\`, check that a\n` +
5247-
' production prune did not drop it, and that its dist is actually built — or\n'
5248-
: ` • add ${pkg} (the enterprise multi-org runtime) to THIS APP\n` +
5249-
" — declare it in the app's package.json and install; the CLI resolves it from the\n" +
5250-
' app, not from the framework it is linked out of. Being merely reachable\n' +
5251-
' through NODE_PATH / a hoisted workspace store is deliberately not enough\n' +
5252-
' (#4719) — that made this wall depend on how the process was launched.\n' +
5253-
' NOTE: this runtime is closed-source and is NOT on the public npm registry —\n' +
5254-
' it is distributed with an enterprise / cloud subscription. Without one this\n' +
5255-
' bullet is not followable, and one of the two below is your path — or\n';
5273+
' production prune did not drop it, and that its dist is actually built — or\n';
5274+
}
5275+
if (kind === 'declared-no-loadable-entry') {
5276+
return ` • this app DECLARES ${pkg} ` +
5277+
`(${declaration.field}: ${JSON.stringify(declaration.specifier)}) and it IS\n` +
5278+
' INSTALLED — neither the declaration nor the install is the problem, and no change\n' +
5279+
' you make in this app can fix it. The package publishes no entry Node can load;\n' +
5280+
' the remedy is in the package, and the cause below is the authority on what it\n' +
5281+
' has to publish — or\n';
5282+
}
5283+
return ` • add ${pkg} (the enterprise multi-org runtime) to THIS APP\n` +
5284+
" — declare it in the app's package.json and install; the CLI resolves it from the\n" +
5285+
' app, not from the framework it is linked out of. Being merely reachable\n' +
5286+
' through NODE_PATH / a hoisted workspace store is deliberately not enough\n' +
5287+
' (#4719) — that made this wall depend on how the process was launched.\n' +
5288+
' NOTE: this runtime is closed-source and is NOT on the public npm registry —\n' +
5289+
' it is distributed with an enterprise / cloud subscription. Without one this\n' +
5290+
' bullet is not followable, and one of the two below is your path — or\n';
52565291
}
52575292

52585293
/**

packages/qa/dogfood/test/enterprise-organizations.test.ts

Lines changed: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,23 @@ let hostWithoutPkg: string;
2929
* resolver, and it used to read as AVAILABLE.
3030
*/
3131
let hostInstalledButUndeclared: string;
32+
/**
33+
* #14041/#14270 — declared AND installed, and the package's own `exports` names
34+
* no runtime entry Node can load (a `types`-only publish). The third
35+
* `HostImportFailureKind`; no install action can change what a package
36+
* publishes, so the "declare it and install it" remedy is unfollowable here.
37+
*/
38+
let hostDeclaredNoLoadableEntry: string;
39+
/**
40+
* The `declared-unresolvable` CONTROL: declared and NOT installed. #14270 left
41+
* this arm alone, so its wording must come out byte-identical.
42+
*/
43+
let hostDeclaredNotInstalled: string;
3244

3345
function writeHost(
3446
prefix: string,
3547
withPkg: boolean,
36-
opts: { declare?: boolean } = {},
48+
opts: { declare?: boolean; typesOnly?: boolean } = {},
3749
): string {
3850
const declare = opts.declare ?? withPkg;
3951
const dir = mkdtempSync(join(tmpdir(), prefix));
@@ -50,6 +62,27 @@ function writeHost(
5062
if (withPkg) {
5163
const pkgDir = join(dir, 'node_modules', ...ORGANIZATIONS_PKG.split('/'));
5264
mkdirSync(pkgDir, { recursive: true });
65+
if (opts.typesOnly) {
66+
// No `require` condition (the CJS resolver throws) and no `import`
67+
// condition (the #14041 fallback finder has nothing to load) — the
68+
// manifest names nothing runnable at all.
69+
writeFileSync(
70+
join(pkgDir, 'package.json'),
71+
JSON.stringify({
72+
name: ORGANIZATIONS_PKG,
73+
version: '0.0.0-fixture',
74+
type: 'module',
75+
exports: { '.': { types: './index.d.ts' } },
76+
}),
77+
'utf8',
78+
);
79+
writeFileSync(
80+
join(pkgDir, 'index.d.ts'),
81+
'export declare class OrganizationsPlugin {}\n',
82+
'utf8',
83+
);
84+
return dir;
85+
}
5386
writeFileSync(
5487
join(pkgDir, 'package.json'),
5588
JSON.stringify({
@@ -73,10 +106,18 @@ beforeAll(() => {
73106
hostWithPkg = writeHost('os-dogfood-org-ok-', true);
74107
hostWithoutPkg = writeHost('os-dogfood-org-missing-', false);
75108
hostInstalledButUndeclared = writeHost('os-dogfood-org-undeclared-', true, { declare: false });
109+
hostDeclaredNoLoadableEntry = writeHost('os-dogfood-org-no-entry-', true, { typesOnly: true });
110+
hostDeclaredNotInstalled = writeHost('os-dogfood-org-not-installed-', false, { declare: true });
76111
});
77112

78113
afterAll(() => {
79-
for (const dir of [hostWithPkg, hostWithoutPkg, hostInstalledButUndeclared]) {
114+
for (const dir of [
115+
hostWithPkg,
116+
hostWithoutPkg,
117+
hostInstalledButUndeclared,
118+
hostDeclaredNoLoadableEntry,
119+
hostDeclaredNotInstalled,
120+
]) {
80121
if (dir) rmSync(dir, { recursive: true, force: true });
81122
}
82123
});
@@ -132,4 +173,46 @@ describe('enterprise multi-org probe (#4700)', () => {
132173
new RegExp(`declare ${ORGANIZATIONS_PKG.replace('/', '\\/')} in .* package\\.json`),
133174
);
134175
});
176+
177+
it('CONTROL — the `declared-unresolvable` remedy is unchanged: declared, not installed', async () => {
178+
// The arm #14270 did NOT touch. Pinned here so the three-way rewrite is a
179+
// measurement: this text has to be byte-identical either side of it.
180+
const probe = await probeOrganizations(hostDeclaredNotInstalled, false);
181+
expect(probe.available).toBe(false);
182+
expect(probe.reason).toContain(
183+
`${hostDeclaredNotInstalled} DECLARES ${ORGANIZATIONS_PKG}, so repair its INSTALL there `
184+
+ '(`pnpm install`, un-prune, rebuild its dist)',
185+
);
186+
});
187+
188+
it('DEFERS to the importer when the package is declared, installed, and unloadable (#14270)', async () => {
189+
// #14041's third kind. This probe's remedy was a two-way branch written for
190+
// two, so `declared-no-loadable-entry` fell into the else leg and told an
191+
// operator whose app DECLARES the package and HAS it installed to declare
192+
// it and install it. No install action can change what a package publishes.
193+
const probe = await probeOrganizations(hostDeclaredNoLoadableEntry, false);
194+
expect(probe.available).toBe(false);
195+
// Which arm fired: the deferral names the two things that are NOT the
196+
// problem and hands the remedy to the importer's message, which this
197+
// reason interpolates at the end.
198+
expect(probe.reason).toContain('and it IS installed, so neither is the problem');
199+
expect(probe.reason).toContain('publishes no entry Node can load');
200+
// ⛔ Neither of the other two arms — both are unfollowable for this kind.
201+
expect(probe.reason).not.toContain(`declare ${ORGANIZATIONS_PKG} in`);
202+
expect(probe.reason).not.toContain('repair its INSTALL');
203+
// The message deferred TO has to actually arrive.
204+
expect(probe.reason).toContain('publishes no entry that Node can load');
205+
});
206+
207+
it('THROWS with that same deferral when the run declares the package (#14270)', async () => {
208+
// The loud half: MULTI_ORG=1 says the package is there, and it IS — it just
209+
// cannot be loaded. The refusal must still name the right remedy.
210+
const err = await probeOrganizations(hostDeclaredNoLoadableEntry, true).then(
211+
() => new Error('probeOrganizations resolved; MULTI_ORG=1 must make this a failure'),
212+
(e: unknown) => e as Error,
213+
);
214+
expect(err.message).toContain(MULTI_ORG_ENV);
215+
expect(err.message).toContain('and it IS installed, so neither is the problem');
216+
expect(err.message).not.toContain(`declare ${ORGANIZATIONS_PKG} in`);
217+
});
135218
});

packages/qa/dogfood/test/enterprise-organizations.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,23 @@ export async function probeOrganizations(
114114
const detail = (e as Error).message;
115115
// #4719 — name the remedy that actually applies. "Install it" is useless
116116
// advice for an app that installed it and never declared it, which is the
117-
// shape a workspace makes easy and a hoisted store used to hide.
117+
// shape a workspace makes easy and a hoisted store used to hide. #14270 —
118+
// the branch reads "is the declaration the problem?", so #14041's third
119+
// kind (declared, installed, and the package publishes nothing loadable)
120+
// gets neither instruction: no install action can change what a package
121+
// publishes, so this arm defers to the importer's own message, which
122+
// `detail` interpolates at the end of both strings below.
123+
const kind = hostImportFailureKind(e);
118124
const remedy =
119-
hostImportFailureKind(e) === 'declared-unresolvable'
125+
kind === 'declared-unresolvable'
120126
? `${root} DECLARES ${ORGANIZATIONS_PKG}, so repair its INSTALL there (\`pnpm install\`, ` +
121127
'un-prune, rebuild its dist)'
122-
: `declare ${ORGANIZATIONS_PKG} in ${root}'s own package.json and install it — being ` +
123-
'reachable as somebody else\'s transitive dependency is not enough (#4719)';
128+
: kind === 'declared-no-loadable-entry'
129+
? `${root} DECLARES ${ORGANIZATIONS_PKG} and it IS installed, so neither is the ` +
130+
'problem — the package publishes no entry Node can load, and the importer\'s ' +
131+
'message below is the authority on what it has to publish'
132+
: `declare ${ORGANIZATIONS_PKG} in ${root}'s own package.json and install it — being ` +
133+
'reachable as somebody else\'s transitive dependency is not enough (#4719)';
124134
if (declared) {
125135
throw new Error(
126136
`${MULTI_ORG_ENV}=1 declares that ${ORGANIZATIONS_PKG} (enterprise, ADR-0105 D12) is ` +

0 commit comments

Comments
 (0)