Skip to content

Commit bd5a443

Browse files
fix(metadata-protocol): a refused targetNamespace answers 400 INVALID_REQUEST with its reason (#31182)
<!-- claude-projects-attribution: {"github_login":"marchtian"} --> _Requested by **田騏** · [project thread](https://claude.ai/code/project/chan_01VmuUhXU9SSUzqJYyXSmYSi?thread=cmsg_01VmuUhXU9SSUzqJYyXSmYSiU1rhbHT2BKdngdowSzudqP)_ Part of #30794 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:unmanaged 安装时填错 `targetNamespace`(不合命名规则,或填了共享的 `base`/`system`/`sys`),现在回 `400 INVALID_REQUEST` 并说明原因;另加测试钉住 Q6 = B(改名副本旁边装 managed 版本放行,未改名副本旁边仍 409 拒绝)。 - 为什么改:之前这两处回 `VALIDATION_ERROR`,而 install-local 接口只回「原因在服务器日志里」,客户看不到填错了什么;文档写的一直是 `INVALID_REQUEST`。 - 风险与回滚:只改两个拒绝的错误码和附带的 `details`,拒绝时机不变、仍在任何写入之前;回滚即 revert 本 PR。`targetPackageId` 的拒绝没动。 - 先看:`packages/metadata-protocol/src/package-unmanaged-install.ts`(两处 `refusal('INVALID_REQUEST', …)`)、`packages/cloud-connection/src/marketplace-install-local-install-modes.test.ts`(末尾两个新 describe)。 - 两分钟验证:`pnpm --filter @objectstack/cloud-connection exec vitest run src/marketplace-install-local-install-modes.test.ts`,预期 34 个全过,其中 4 个是本 PR 新加的。 - 需要你做的:无,席位落地。 ## What changes - `planUnmanagedImport` (`packages/metadata-protocol/src/package-unmanaged-install.ts`): the two `targetNamespace` refusals (a value `ManifestSchema.shape.namespace` refuses; a platform-reserved namespace `base`, `system`, `sys`) now throw `refusal('INVALID_REQUEST', 400, …, { targetNamespace })` instead of a code-less `statusCode: 400`. That is the code the install-modes guide's refusal row and PR A's changeset already state, and the code the sibling rename refusal (`unmanagedInstallRenameRefusal`) answers. - The install-local door now passes the reason through. Before, `clientFacingFailureText` withheld the message of a `statusCode`-only 400, so the door answered `{"code":"VALIDATION_ERROR","message":"The unmanaged install of 'com.example.crm' failed; the reason is in the server log."}`. Now it answers `400 {"code":"INVALID_REQUEST","message":"Invalid package namespace 'Acme' on `targetNamespace`. …","details":{"targetNamespace":"Acme"}}`, and no log line is written because the caller has the reason. - Q6 = B pins (ruling record 6709037798): a managed install of `com.example.crm` beside a copy renamed to `acme` proceeds to its own steps; its twin, beside a copy that kept `crm`, is refused `409 RESOURCE_CONFLICT` before any write. Step 1f (`findEnvironmentBaseHoldingNamespace`) is untouched. - One changeset, `@objectstack/metadata-protocol` patch. `@objectstack/cloud-connection` changes only a test file, so it publishes nothing new. ## Tests - `packages/metadata-protocol/src/package-unmanaged-install.test.ts`: the two planner pins move from `[400, 'VALIDATION_ERROR']` to `[400, 'INVALID_REQUEST']` and assert `details` is `{ targetNamespace }`. - `packages/cloud-connection/src/marketplace-install-local-install-modes.test.ts`, two new top-level `describe`s in the existing harness: - the door with the REAL `ObjectStackProtocolImplementation.importUnmanagedPackage` (not a double): `Acme` and `sys` each answer 400 `INVALID_REQUEST` with the reason in the body and `details.targetNamespace`, nothing registered, no ledger entry; - the Q6 pair described above. ## Acceptance notes - **Left alone on purpose:** the `targetPackageId` refusals (id rule, same id as the source) keep their code-less 400 and still answer `VALIDATION_ERROR`. Changing them is a separate call, as the hand-over (6690317826) says. The non-string `targetNamespace` refused by the request's published declaration (`ImportUnmanagedPackageRequestSchema`) also stays `VALIDATION_ERROR` with `issues`; that is a schema refusal, not the plan's. - **Docs:** no sentence became false. `install-modes.mdx` already says `400 INVALID_REQUEST, naming targetNamespace`, and "a managed install of the package is refused while the copy holds its namespace" agrees with Q6 = B (a renamed copy does not hold it). PR B states Q6 in ADR-0131 D6. ## Verification <details><summary>Local tests (head 9aa77b4)</summary> - `pnpm --filter @objectstack/metadata-protocol exec vitest run --maxWorkers=2 src/package-unmanaged-install.test.ts`: Test Files 1 passed, Tests 82 passed. - `pnpm --filter @objectstack/cloud-connection exec vitest run --maxWorkers=2 src/marketplace-install-local-install-modes.test.ts`: Test Files 1 passed, Tests 34 passed. </details> <details><summary>Ablation (door pin)</summary> `node scripts/ablation-replace.mjs` turned the reserved-namespace refusal back into the code-less `invalidRequest(…)` (anchor 1 → 0, blob eb17407 → 23f7fd3), then rebuilt `@objectstack/metadata-protocol` and ran the door pin: `× a platform-reserved namespace: 400 INVALID_REQUEST …`, Expected `"INVALID_REQUEST"`, Received `"VALIDATION_ERROR"` (1 failed, 1 passed). Restored (blob == HEAD, `git diff HEAD` empty), rebuilt, and the file passed 34/34. </details> <details><summary>Derived gates (head 9aa77b4): 60 derived, 59 exit 0, 1 NOT MEASURED</summary> Derived with `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` (4 paths vs merge base 8ef4d30), run under `NODE_OPTIONS=--max-old-space-size=8192`, exit codes captured before any pipe, reconciled with `--ran`: "60 derived famil(ies) accounted for — 59 run, 1 NOT-MEASURED". ```text node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0 node scripts/check-adr-0087-registration.mjs --self-test :: exit 0 node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0 node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-adoption.mjs :: exit 0 node scripts/check-comment-mask-adoption.mjs --self-test :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-dts-emitted.mjs --self-test :: exit 0 node scripts/check-empty-changeset.mjs --base origin/main :: exit 0 node scripts/check-empty-changeset.mjs --self-test :: exit 0 node scripts/check-issue-citations.mjs :: exit 0 node scripts/check-plugin-teardown-shape.mjs :: exit 0 node scripts/check-plugin-teardown-shape.mjs --self-test :: exit 0 node scripts/ci/assert-turbo-remote-cache.mjs --self-test :: exit 0 node scripts/ci/plan-test-core-shards.mjs --self-test :: exit 0 node scripts/ci/prune-turbo-cache.mjs --self-test :: exit 0 node scripts/ci/snapshot-next.mjs --self-test :: exit 0 node scripts/docs-audit/check-affected-docs.mjs :: exit 0 node scripts/docs-audit/check-drift-comment.mjs :: exit 0 node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0 node scripts/release-pending-publish.mjs --self-test :: exit 0 pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0 pnpm check:changeset-gate-self-tests :: exit 0 pnpm check:ci-filter-parity :: exit 0 pnpm check:closed-ceremony-text :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:doc-authoring :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:dts-closure :: exit 0 pnpm check:dual-build-cjs-loads :: exit 3 pnpm check:durability-log-level :: exit 0 pnpm check:engine-double-contract :: exit 0 pnpm check:error-status-conformance :: exit 0 pnpm check:gitlink-declared :: exit 0 pnpm check:issue-citations :: exit 0 pnpm check:keyed-text-bounds :: exit 0 pnpm check:lean-entry-closure :: exit 0 pnpm check:logger-receiver-detach :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:objectql-double-limit :: exit 0 pnpm check:org-identifier :: exit 0 pnpm check:page-declaration-shape :: exit 0 pnpm check:platform-object-tenancy-census :: exit 0 pnpm check:published-files :: exit 0 pnpm check:query-options-erasure :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:registry-log-declared :: exit 0 pnpm check:rest-log-spy-declared :: exit 0 pnpm check:slot-lookup :: exit 0 pnpm check:sourcemap-no-sources-content :: exit 0 pnpm check:system-context-census :: exit 0 pnpm check:test-source-alias :: exit 0 pnpm check:tier-file-adoption :: exit 0 pnpm check:type-check-coverage :: exit 0 pnpm check:type-check-debt :: exit 0 pnpm check:undeclared-dep-imports :: exit 0 pnpm check:vouch-for-reference-callers :: exit 0 pnpm check:watch-hint-literal :: exit 0 pnpm check:where-matcher :: exit 0 ``` - `pnpm check:dual-build-cjs-loads` exit 3 = PREREQUISITE NOT MET (no whole-tree `pnpm build` in this container; its self-test passed 93 cases). NOT MEASURED locally; CI's `Lint & Repo Gates` runs it. - `pnpm lint` :: exit 0. - `pnpm --filter @objectstack/metadata-protocol --filter @objectstack/cloud-connection typecheck` :: exit 0. </details> 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- _Generated by [Claude Code](https://claude.ai/code/session_017ihL2ewD2rAszkPvzKBT8T)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8acbfab commit bd5a443

4 files changed

Lines changed: 89 additions & 6 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): a refused `targetNamespace` on an unmanaged install answers `400 INVALID_REQUEST` with the reason, as documented
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
An unmanaged install whose `targetNamespace` the namespace rule refuses (`Acme`, `my-ns`, too long), or that names a shared namespace (`base`, `system`, `sys`), was refused with a 400 that carried no code. The boundary answered it as `VALIDATION_ERROR`, and the install-local door (`POST /api/v1/marketplace/install-local`) withheld the reason, answering only that it was in the server log.
11+
12+
Both refusals now answer `400 INVALID_REQUEST`, the code the install-modes guide and the other rename refusals already give, with `details.targetNamespace`. Through the install-local door the answer now carries the reason, for example `Invalid package namespace 'Acme' on targetNamespace. ...`. Nothing else changes: the refusal still comes before anything is written, and a `targetPackageId` the id rule refuses still answers `VALIDATION_ERROR`.

‎packages/cloud-connection/src/marketplace-install-local-install-modes.test.ts‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import '@objectstack/runtime';
3636
import { MarketplaceInstallLocalPlugin } from './marketplace-install-local-plugin.js';
3737
import { declareInstallerPlatformOwner, installerAuthService, withInstallerGrants } from './install-local-principal.fixtures.js';
3838
import { LocalManifestSource } from './local-manifest-source.js';
39+
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';
3940

4041
const ROUTE = '/api/v1/marketplace/install-local';
4142
const INSTALLER = 'usr_installer';
@@ -567,3 +568,61 @@ describe('install-local — `targetNamespace` renames an unmanaged copy, and onl
567568
});
568569
}
569570
});
571+
572+
describe('install-local — the import\'s own `targetNamespace` refusal, as the door answers it (#30794)', () => {
573+
// The REAL copy-once import, not a double: the refusal is judged by its
574+
// plan before the engine is asked anything, so an empty engine suffices.
575+
const realImporter = () => {
576+
const protocol = new ObjectStackProtocolImplementation({ registry: {} } as any, () => new Map()) as any;
577+
return (request: any) => protocol.importUnmanagedPackage(request);
578+
};
579+
580+
for (const [label, targetNamespace, sentence] of [
581+
['a value the namespace declaration refuses', 'Acme', "Invalid package namespace 'Acme' on `targetNamespace`"],
582+
['a platform-reserved namespace', 'sys', "`targetNamespace` 'sys' is a platform-reserved namespace"],
583+
] as const) {
584+
it(`${label}: 400 INVALID_REQUEST, the reason in the body and \`details.targetNamespace\`; nothing written`, async () => {
585+
const h = await mount(dir, { importer: realImporter() });
586+
const res = await h.install({ manifest: crm({ installModes: BOTH }), installMode: 'unmanaged', targetNamespace });
587+
expect(res.status, JSON.stringify(res.payload)).toBe(400);
588+
expect(res.payload.error.code).toBe('INVALID_REQUEST');
589+
expect(res.payload.error.message).toContain(sentence);
590+
expect(res.payload.error.message).not.toContain('the reason is in the server log');
591+
expect(res.payload.error.details).toEqual({ targetNamespace });
592+
expect(h.register).not.toHaveBeenCalled();
593+
expect(h.ledgerHas('com.example.crm')).toBe(false);
594+
});
595+
}
596+
});
597+
598+
describe('install-local — Q6 after a RENAMED copy: the managed install is admitted; after an unrenamed one, refused (#30794)', () => {
599+
// The ruling (record 6709037798, Q6 = B): a copy renamed into a namespace
600+
// of its own does not hold the source namespace, so a later managed
601+
// install of the same source package is not refused. A copy that kept the
602+
// source namespace still holds it, and Q6 (step 1f) refuses as written.
603+
const renamedCopyHeld: RegistryState = {
604+
owners: { acme: ['com.example.crm-copy'] },
605+
packages: { 'com.example.crm-copy': { manifest: { id: 'com.example.crm-copy', namespace: 'acme' } } },
606+
};
607+
const unrenamedCopyHeld: RegistryState = {
608+
owners: { crm: ['com.example.crm-copy'] },
609+
packages: { 'com.example.crm-copy': { manifest: { id: 'com.example.crm-copy', namespace: 'crm' } } },
610+
};
611+
612+
it('beside a copy renamed to `acme`: the managed install of `com.example.crm` proceeds to its own steps', async () => {
613+
const h = await mount(dir, { registry: renamedCopyHeld });
614+
const res = await h.install({ manifest: crm({ installModes: BOTH }) });
615+
expect(res.payload?.error?.code, JSON.stringify(res.payload)).not.toBe('RESOURCE_CONFLICT');
616+
expect(h.register).toHaveBeenCalledTimes(1);
617+
});
618+
619+
it('TWIN: beside a copy that kept `crm`, the same install is refused 409 RESOURCE_CONFLICT, before any write', async () => {
620+
const h = await mount(dir, { registry: unrenamedCopyHeld });
621+
const res = await h.install({ manifest: crm({ installModes: BOTH }) });
622+
expect(res.status).toBe(409);
623+
expect(res.payload.error.code).toBe('RESOURCE_CONFLICT');
624+
expect(res.payload.error.details).toEqual({ namespace: 'crm', base: 'com.example.crm-copy' });
625+
expect(h.register).not.toHaveBeenCalled();
626+
expect(h.ledgerHas('com.example.crm')).toBe(false);
627+
});
628+
});

‎packages/metadata-protocol/src/package-unmanaged-install.test.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -954,20 +954,22 @@ describe('planUnmanagedImport — `targetNamespace`, the rename-on-install (#307
954954
}
955955
});
956956

957-
it('refuses a value the namespace declaration refuses — 400, naming `targetNamespace`, never sanitised', () => {
957+
it('refuses a value the namespace declaration refuses — INVALID_REQUEST / 400, naming `targetNamespace`, never sanitised', () => {
958958
for (const targetNamespace of ['Acme', 'a', 'my-ns', '1acme', 'acme_namespace_too_long']) {
959959
const err = (() => { try { plan(renamableBody(), { targetNamespace }); } catch (e) { return e as any; } })();
960960
const envelope = resolveThrownHttpError(err);
961-
expect([envelope.status, envelope.code]).toEqual([400, 'VALIDATION_ERROR']);
961+
expect([envelope.status, envelope.code]).toEqual([400, 'INVALID_REQUEST']);
962+
expect(err.details).toEqual({ targetNamespace });
962963
expect(err.message).toContain(`Invalid package namespace '${targetNamespace}' on \`targetNamespace\``);
963964
}
964965
});
965966

966-
it('refuses a platform-reserved namespace — `base`, `system`, `sys` — 400', () => {
967+
it('refuses a platform-reserved namespace — `base`, `system`, `sys` — INVALID_REQUEST / 400, naming `targetNamespace`', () => {
967968
for (const targetNamespace of ['base', 'system', 'sys']) {
968969
const err = (() => { try { plan(renamableBody(), { targetNamespace }); } catch (e) { return e as any; } })();
969970
const envelope = resolveThrownHttpError(err);
970-
expect([envelope.status, envelope.code]).toEqual([400, 'VALIDATION_ERROR']);
971+
expect([envelope.status, envelope.code]).toEqual([400, 'INVALID_REQUEST']);
972+
expect(err.details).toEqual({ targetNamespace });
971973
expect(err.message).toContain(`\`targetNamespace\` '${targetNamespace}' is a platform-reserved namespace`);
972974
}
973975
});

‎packages/metadata-protocol/src/package-unmanaged-install.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,16 +480,26 @@ export function planUnmanagedImport(
480480
const declared = ManifestSchema.shape.namespace.safeParse(targetNamespace);
481481
if (!declared.success) {
482482
const rule = declared.error.issues[0]?.message ?? 'See `manifest.namespace`';
483-
throw invalidRequest(
483+
// `INVALID_REQUEST` / 400 with `details.targetNamespace`, as the
484+
// documented refusal row and the rename refusal below answer: a
485+
// code-less 400 reaches the boundary as the status-derived
486+
// `VALIDATION_ERROR`, and the install-local door withholds its reason.
487+
throw refusal(
488+
'INVALID_REQUEST',
489+
400,
484490
`Invalid package namespace '${targetNamespace}' on \`targetNamespace\`. ${rule}. It becomes the `
485491
+ 'copy\'s `manifest.namespace` and the prefix of every copied object name.',
492+
{ targetNamespace },
486493
);
487494
}
488495
if (PLATFORM_RESERVED_NAMESPACES.has(targetNamespace)) {
489-
throw invalidRequest(
496+
throw refusal(
497+
'INVALID_REQUEST',
498+
400,
490499
`\`targetNamespace\` '${targetNamespace}' is a platform-reserved namespace `
491500
+ `(${[...PLATFORM_RESERVED_NAMESPACES].map((ns) => `'${ns}'`).join(', ')}), which many packages share, `
492501
+ 'so it cannot carry one copy\'s names. Name a namespace of the copy\'s own.',
502+
{ targetNamespace },
493503
);
494504
}
495505
}

0 commit comments

Comments
 (0)