diff --git a/.changeset/19417-install-door-parses-manifest-id.md b/.changeset/19417-install-door-parses-manifest-id.md new file mode 100644 index 00000000000..c4b1c6c4390 --- /dev/null +++ b/.changeset/19417-install-door-parses-manifest-id.md @@ -0,0 +1,76 @@ +--- +'@objectstack/runtime': minor +--- + +fix(runtime): `POST /api/v1/packages` parses the manifest's `id` leg instead of reading it positionally (#19417) + +Clause-②: no (narrowing) + +**BREAKING for callers of the install door** — a manifest whose `id` is not +reverse-domain notation is now refused `400` / `VALIDATION_ERROR`. It used to +install and answer `201`. + +The accept set only shrinks back to what the published declaration has always +said. `MANIFEST_ID_PATTERN` is declared once in +`packages/spec/src/kernel/manifest.zod.ts` and referenced by both faces of one +identity — `ManifestSchema.id`, what an author writes, and +`PackageSchema.manifestId`, what the registry stores and publishes by. The door +read `manifest.id` POSITIONALLY (`typeof manifest?.id === 'string' ? +manifest.id.trim() : ''`) and parsed nothing, so `id: 'pkg-a'` installed and +answered `201` while `defineStack()`, `os build`, `os validate` and the publish +face all refused the same id. The author was handed a package that could never +be rebuilt or published. That is «declared ≠ enforced» on a published API +contract — and nothing in `packages/spec` moves for it: the declaration was +already right. + +The gate asks the declaration **by reference** — `ManifestSchema.shape.id` — +rather than keeping a copy of the grammar, exactly as the `version` leg beside +it does, so a future move of the reverse-domain rule reaches this door with no +further edit. The sentence the caller reads is the declaration's own +(`manifestIdRefusal`), **surfaced rather than reworded**: it names the key, +echoes the value, lists the two examples, and carries a suggestion arm that +verifies its candidate against the pattern before offering it. Posting +`id: 'pkg-a'` now answers, in the response envelope's `error.message`: + +```text +Invalid package id 'pkg-a' on `manifest.id`. Expected reverse-domain notation +('com.steedos.crm', 'org.apache.superset') — lowercase dot-separated segments; +hyphens allowed inside a segment, underscores are not. Did you mean +'com.example.pkg-a'? +``` + +**What is not affected.** Boot-time and in-process installs reach +`SchemaRegistry.installPackage` / `ObjectQL.registerApp` directly and never pass +through this branch, so nothing about how a package is loaded from disk or +registered by a plugin changes. A conforming manifest installs exactly as +before, on both body forms (wrapped and bare) and on both install limbs (the +protocol primitive and the bare-registry fallback). + +**The `Package id is required` sentence does NOT move.** `''` fails +`MANIFEST_ID_PATTERN` too, so where this gate sits decides whether a published +message changes or only the accept set does. It is ordered AFTER the existing +`!pkgId` check: an absent, empty, whitespace-only or non-string `id` still +answers `400 Package id is required`, never the schema's sentence — which on +that input is the one case where the suggestion arm has nothing to offer. +Measured both directions. It is ordered BEFORE the `version` gate for the +mirror-image reason: that refusal's sentence names the id it prescribes for, and +prescribing a `version` repair for an id that can never be legal sends the +author round twice. + +**Scope — the `id` leg alone.** The declaration's residual docblock records the +classes this door answers `201` to while `PackageInstallBodySchema` refuses +them. This change closes one: `id`. A missing `type`, unknown keys on either +body form, a string-typed `enableOnInstall` / `overwrite`, and install options +spelled on the bare form are each left exactly as they were — measured after the +change, all seven spellings of those four classes still answer `201`, against a +`pkg-a` control that answers `400` and a conforming control that answers `201`. +Each is its own reading and its own card. Closing them is the one call this +handler still pointedly does not make, `PackageInstallBodySchema.safeParse(body)`. + +**If you are refused.** Give the manifest an id in reverse-domain notation — +lowercase dot-separated segments, hyphens allowed inside a segment, underscores +not. The refusal names the key, echoes what you wrote and, where a mechanical +repair exists, offers one it has already checked against the rule, so the +prescription arrives with the `400` rather than in a changelog. + + diff --git a/packages/runtime/src/domain-handler-registry.test.ts b/packages/runtime/src/domain-handler-registry.test.ts index 3b1bfb93ab4..fe7814bb378 100644 --- a/packages/runtime/src/domain-handler-registry.test.ts +++ b/packages/runtime/src/domain-handler-registry.test.ts @@ -588,13 +588,41 @@ describe('HttpDispatcher extracted domains (PR-5: packages)', () => { // ⚠️ The repair is owed whatever order the new gate sits in: the // `forced` limb asserts `201`, which an under-specified manifest must // never reach — so no placement of that gate leaves this fixture valid. - const manifest = { id: 'pkg-a', name: 'A', version: '1.0.0' }; + // + // [#19417] Same repair, one key over — and the LAST one this fixture + // owed. `ManifestSchema.id` carries `MANIFEST_ID_PATTERN` since #18319, + // `pkg-a` is not reverse-domain notation, and the install door parses + // that leg now: the `forced` limb's `201` was unreachable for this id. + // ⛔ The `pkg-a` reading is REVERSED, never deleted — the case below + // keeps it, asserting the refusal this fixture used to assert the + // acceptance of. `com.example.pkg-a` is the same name made publishable, + // which is the repair `manifestIdRefusal` itself prescribes for it. + const manifest = { id: 'com.example.pkg-a', name: 'A', version: '1.0.0' }; const dup = await dispatcher.dispatch('POST', '/packages', manifest, {}, {} as any); expect(dup.response?.status).toBe(409); const forced = await dispatcher.dispatch('POST', '/packages', manifest, { overwrite: 'true' }, {} as any); expect(forced.response?.status).toBe(201); }); + it('[#19417] POST /packages refuses `pkg-a` — the id the pattern refuses (the REVERSED pin)', async () => { + // ⭐ This case is the other half of the repair above, and it is why that + // repair is not a deletion. `pkg-a` was the one standing pin on the gap + // this card closes: the door answered it `201` while `defineStack()`, + // `os build`, `os validate` and the publish face all refused it, so an + // author could install a package that could never be rebuilt or + // published. The reading is kept, pointing the other way. + // + // ⛔ Deliberately thin here — status only. The full pins (the + // declaration's own sentence, both install writers silent, and the + // gate's placement against `Package id is required`, the `version` leg + // and the `409`) live in `domains/packages-install-manifest-id.test.ts` + // beside the door. This file's subject is ROUTING. + const objectql = qlWithRegistry(); + const result = await withPkgCaller(makeDispatcher({ objectql })) + .dispatch('POST', '/packages', { id: 'pkg-a', name: 'A', version: '1.0.0' }, {}, {} as any); + expect(result.response?.status).toBe(400); + }); + it('POST /packages without an id is rejected with 400', async () => { const objectql = qlWithRegistry(); const result = await withPkgCaller(makeDispatcher({ objectql })).dispatch('POST', '/packages', { name: 'no-id' }, {}, {} as any); diff --git a/packages/runtime/src/domains/packages-install-manifest-id.test.ts b/packages/runtime/src/domains/packages-install-manifest-id.test.ts new file mode 100644 index 00000000000..422f9fda11f --- /dev/null +++ b/packages/runtime/src/domains/packages-install-manifest-id.test.ts @@ -0,0 +1,451 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#19417] `POST /api/v1/packages` parses the manifest's `id` leg. + * + * ## The defect this file pins shut + * + * `MANIFEST_ID_PATTERN` (`packages/spec/src/kernel/manifest.zod.ts`) is the + * reverse-domain rule declared ONCE and referenced by BOTH faces of one + * identity — `ManifestSchema.id`, what an author writes, and + * `PackageSchema.manifestId`, what the registry stores and publishes by. The + * install door read `manifest.id` POSITIONALLY (`typeof manifest?.id === + * 'string' ? manifest.id.trim() : ''`) and parsed nothing, so a complete + * manifest carrying `id: 'pkg-a'` installed and the door answered `201` while + * `defineStack()`, `os build`, `os validate` and the publish face all refused + * the same id. The author got a package that could never be rebuilt or + * published — «declared ≠ enforced» on a PUBLISHED API contract, which Prime + * Directive #10 refuses outright, and the exact failure 北极星 clause 4 names: + * 「错的必须被**响亮拒绝**并给处方,**永不静默落库**」. + * + * The authorising ruling is 基本裁决原则 — + * 「spec 声明 > 实现 > 文档面;**声明而未兑现是实现缺口,补实现或退役**, + * ⛔ 不在消费端收窄」 — and by the mechanical boundary test, making a door parse + * what its schema ALREADY declares is 拉回已声明契约, ⛔ not 扩大接受集. + * Nothing in `packages/spec` moves for it. + * + * ## ⛔ The scope fence — the `id` leg ALONE + * + * The declaration's residual docblock records the classes this door answers + * `201` to. This card was graded on the `id` leg, which is the leg that was + * measured. A missing `type`, unknown keys on either body form, a string-typed + * `enableOnInstall`/`overwrite` and install options spelled on the bare form + * are each their own reading and are deliberately LEFT STANDING by both the fix + * and this file. + * + * ⛔ **No case here asserts anything about those four**, in either direction — + * the discipline `packages-install-manifest-version.test.ts` recorded for the + * same fence and the reason it gave: pinning them as `201` would freeze four + * known residuals as intended behaviour and turn the card that closes one of + * them red for doing its job; pinning them as refused would be this file quietly + * widening a graded scope. The separability evidence lives where a one-shot + * measurement belongs — the PR body and the changeset — not in a permanent + * expectation. + * + * ## Why §0 exists + * + * Every refusal below is a claim that the DOOR agrees with the DECLARATION, + * never that the door matches a grammar this file has an opinion about. So each + * fixture is first asserted off-spec *by the declaration itself*. If + * `MANIFEST_ID_PATTERN` ever moves, §0 goes red first and names what happened, + * instead of §1 failing for a reason a reader would have to reconstruct. + * + * ## Why §3 is the longest section + * + * `''` fails `MANIFEST_ID_PATTERN` too, so WHERE this gate sits decides whether + * a PUBLISHED message moves or only the accept set does. The gate is ordered + * AFTER the `!pkgId` check, and §3 pins that in both directions: an absent or + * empty id still prints `Package id is required` and ⛔ never the schema's + * sentence. It is ordered BEFORE the `version` gate and before the duplicate + * `409`, each with a lit control that makes the case a statement about ORDER + * rather than about the other gate having been removed. + * + * ## The harness + * + * Spies on BOTH install writers — the protocol primitive and the bare registry + * fallback — because a refusal pin owes two halves: the status and code the + * caller is told, AND that the write never happened. A door that answered `400` + * and installed anyway would satisfy the first half alone. + * + * `OS_HOME` is redirected for the whole file: the positive control installs for + * real, so a test that wrote into the developer's own home would touch packages + * in their running system. + */ + +import { describe, it, expect, vi, beforeAll, afterAll } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { ManifestSchema, manifestIdRefusal } from '@objectstack/spec/kernel'; +import { HttpDispatcher } from '../http-dispatcher.js'; + +let home: string; +let priorHome: string | undefined; + +beforeAll(() => { + priorHome = process.env.OS_HOME; + home = mkdtempSync(join(tmpdir(), 'os-install-id-')); + process.env.OS_HOME = home; +}); + +afterAll(() => { + if (priorHome === undefined) delete process.env.OS_HOME; + else process.env.OS_HOME = priorHome; + rmSync(home, { recursive: true, force: true }); +}); + +const PKG_ADMIN = () => ({ + request: {}, + environmentId: 'pkg-install-id-test', + executionContext: { + userId: 'u_pkg_admin', + systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], + }, +}) as any; + +/** A complete AUTHORING-stage manifest — the stage this door is reached at. */ +const WELL_FORMED = { id: 'com.acme.crm', name: 'Acme CRM', namespace: 'acme', version: '1.0.0', type: 'app' }; + +/** + * The card's own named case: a bare word, the shape the scaffolder and `os init` + * used to produce. Asserted off-spec in §0, never assumed. + */ +const CARD_CASE = 'pkg-a'; + +/** + * The ids the door must start refusing, each a different way of failing ONE + * rule — a bare word with no dot, an underscore inside a segment, an uppercase + * segment, and a value whose only defect is the whitespace around it. + * + * ⚠️ The last one is why the gate reads the RAW value rather than the trimmed + * `pkgId`: the trim keys the package, and if it also laundered the id past its + * own rule the door would still store a manifest whose `id` the declaration + * refuses. + */ +const OFF_SPEC_IDS = [CARD_CASE, 'com.example.my_erp', 'Com.Acme.Crm', ' com.acme.crm '] as const; + +/** + * A manifest carrying every key the well-formed one does EXCEPT the named one. + * + * ⛔ Spelled as a delete rather than as a shorter literal on purpose: two + * fixtures must differ in exactly one key, or a refusal proves nothing about + * which key caused it. + */ +function without(obj: Record, key: K) { + const copy = { ...obj }; + delete copy[key]; + return copy; +} + +interface Door { + dispatcher: HttpDispatcher; + protocolInstall: ReturnType; + registryInstall: ReturnType; +} + +/** + * @param existingId an id the registry should report as already installed — + * how §3 reaches the `409` branch this gate is ordered ahead of. + */ +function makeDoor(options: { existingId?: string } = {}): Door { + const registryInstall = vi.fn().mockImplementation((manifest: any) => ({ + manifest, + status: 'installed', + enabled: true, + })); + const protocolInstall = vi.fn().mockImplementation(({ manifest }: any) => ({ + package: { manifest, status: 'installed', enabled: true }, + })); + const registry = { + installPackage: registryInstall, + getPackage: vi.fn().mockImplementation((id: string) => + (options.existingId && id === options.existingId) + ? { manifest: { id, version: '0.9.0' }, status: 'installed', enabled: true } + : undefined), + getAllPackages: vi.fn().mockReturnValue([]), + disablePackage: vi.fn(), + }; + const kernel: any = { + getService: (name: string) => { + if (name === 'protocol') return Promise.resolve({ installPackage: protocolInstall }); + if (name === 'objectql') return Promise.resolve({ registry }); + return null; + }, + context: { getService: () => null }, + }; + return { dispatcher: new HttpDispatcher(kernel), protocolInstall, registryInstall }; +} + +/** `POST /api/v1/packages` exactly as the route reads it. */ +const install = (door: Door, body: unknown, query: Record = {}) => + door.dispatcher.handlePackages('', 'POST', body, query, PKG_ADMIN()); + +const messageOf = (r: Awaited>) => + String((r.response as any)?.body?.error?.message ?? ''); + +// ═══════════════════════════════════════════════════════════════════════ +// §0 — the premise every case below rests on +// ═══════════════════════════════════════════════════════════════════════ + +describe('§0 the DECLARATION is what the door is being held to', () => { + it('refuses every §1 fixture BY THE DECLARATION, not by this file', () => { + // ⛔ If `MANIFEST_ID_PATTERN` moves, THIS is the assertion that goes red + // and says what happened. The fix under test asks + // `ManifestSchema.shape.id` by reference, so the DOOR follows the rule + // with no edit — only these fixtures would need re-choosing. + for (const id of OFF_SPEC_IDS) { + expect(ManifestSchema.shape.id.safeParse(id).success).toBe(false); + } + }); + + it('accepts the positive control, so §2 can actually fail', () => { + expect(ManifestSchema.shape.id.safeParse(WELL_FORMED.id).success).toBe(true); + }); + + it('⭐ refuses `\'\'` too — which is what makes §3 a real decision, not a formality', () => { + // The whole of §3 exists because this is `false`. Were `''` admitted by + // the declaration, the gate's placement relative to `Package id is + // required` could not move a published message and there would be + // nothing to rule on. + expect(ManifestSchema.shape.id.safeParse('').success).toBe(false); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════ +// §1 — the refusal: code + status, and the write never happened +// ═══════════════════════════════════════════════════════════════════════ + +describe('§1 an id the declaration refuses is refused loudly, and nothing installs', () => { + const cases: ReadonlyArray = [ + ...OFF_SPEC_IDS.map((id) => [ + `WRAPPED form, id '${id}'`, + { manifest: { ...WELL_FORMED, id } }, + ] as const), + [ + `BARE form, id '${CARD_CASE}' — \`body.manifest || body\` reaches the same gate`, + { ...WELL_FORMED, id: CARD_CASE }, + ], + ]; + + for (const [label, body] of cases) { + it(`${label} → 400 VALIDATION_ERROR`, async () => { + const door = makeDoor(); + const r = await install(door, body); + + expect(r.handled).toBe(true); + // The minimum assertion an ADR-0112 refusal owes: code + status. + // `deps.error(msg, 400)` derives `VALIDATION_ERROR` from + // `standardErrorCodeForHttpStatus(400)` — ⛔ no new code was minted + // for this card. + expect(r.response?.status).toBe(400); + expect((r.response as any)?.body?.error?.code).toBe('VALIDATION_ERROR'); + expect((r.response as any)?.body?.success).toBe(false); + expect((r.response as any)?.body?.data).toBeUndefined(); + }); + + it(`${label} → NEITHER install writer was called`, async () => { + const door = makeDoor(); + await install(door, body); + // The other half of a refusal pin. A door that answered 400 and + // installed anyway satisfies the status assertion above, and this + // card is precisely about a write that should never have landed. + expect(door.protocolInstall).not.toHaveBeenCalled(); + expect(door.registryInstall).not.toHaveBeenCalled(); + }); + } + + it('⭐ the sentence is the DECLARATION\'S, surfaced rather than reworded', async () => { + const door = makeDoor(); + const r = await install(door, { manifest: { ...WELL_FORMED, id: CARD_CASE } }); + + // ⛔ Not a prose pin on wording this file chose. The door is held to + // `manifestIdRefusal`'s OWN output, by reference: if spec rewords the + // refusal, the door follows it and this stays green; if the door starts + // inventing a fourth sentence for one rule, this goes red. + expect(messageOf(r)).toBe(manifestIdRefusal('manifest.id', CARD_CASE)); + }); + + it('that sentence carries the prescription — key, value, and a VERIFIED repair', async () => { + const door = makeDoor(); + const r = await install(door, { manifest: { ...WELL_FORMED, id: CARD_CASE } }); + const message = messageOf(r); + + // 北极星 clause 4 requires the refusal to give a prescription. These are + // the three things an author who is already stuck needs: which key, what + // they actually wrote, and something to write instead. + expect(message).toContain('manifest.id'); + expect(message).toContain(`'${CARD_CASE}'`); + expect(message).toContain('com.example.pkg-a'); + // And the suggested repair is itself legal — the arm verifies its + // candidate against the pattern before offering it, so a refusal can + // never teach an id the same door would refuse. + expect(ManifestSchema.shape.id.safeParse('com.example.pkg-a').success).toBe(true); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════ +// §2 — the positive control +// ═══════════════════════════════════════════════════════════════════════ + +describe('§2 a conforming id still installs', () => { + it('⭐ answers 201 and routes the manifest through the protocol primitive, unchanged', async () => { + const door = makeDoor(); + const r = await install(door, { manifest: WELL_FORMED, settings: { a: 1 } }); + + // Without this, §1 is satisfied by a door that refuses everything. + expect(r.response?.status).toBe(201); + // The preservation half: the gate reads `manifest.id` and hands the + // manifest on untouched — it is a gate, ⛔ not a normaliser. + expect(door.protocolInstall).toHaveBeenCalledWith({ manifest: WELL_FORMED, settings: { a: 1 } }); + expect((r.response as any)?.body?.data?.manifest).toEqual(WELL_FORMED); + }); + + it('the BARE form installs too — the gate did not close a body form', async () => { + const door = makeDoor(); + const r = await install(door, WELL_FORMED); + + expect(r.response?.status).toBe(201); + expect(door.protocolInstall).toHaveBeenCalledWith({ manifest: WELL_FORMED, settings: undefined }); + }); + + it('falls through to the registry writer when the protocol lacks the method', async () => { + const registryInstall = vi.fn().mockReturnValue({ manifest: WELL_FORMED, status: 'installed', enabled: true }); + const kernel: any = { + getService: (name: string) => { + if (name === 'protocol') return Promise.resolve({}); + if (name === 'objectql') { + return Promise.resolve({ + registry: { + installPackage: registryInstall, + getPackage: vi.fn().mockReturnValue(undefined), + getAllPackages: vi.fn().mockReturnValue([]), + disablePackage: vi.fn(), + }, + }); + } + return null; + }, + context: { getService: () => null }, + }; + const r = await new HttpDispatcher(kernel) + .handlePackages('', 'POST', { manifest: WELL_FORMED }, {}, PKG_ADMIN()); + + // The gate sits ahead of BOTH limbs, so the fallback limb is reached by + // a conforming manifest exactly as before this card. + expect(r.response?.status).toBe(201); + expect(registryInstall).toHaveBeenCalledWith(WELL_FORMED, undefined); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════ +// §3 — ordering, and the published message that does NOT move +// ═══════════════════════════════════════════════════════════════════════ + +describe('§3a the `Package id is required` sentence is UNCHANGED — both directions', () => { + const cases: ReadonlyArray = [ + ['no `id` key at all', { manifest: without(WELL_FORMED, 'id') }], + ['an empty-string `id`', { manifest: { ...WELL_FORMED, id: '' } }], + ['a whitespace-only `id` — the trim still decides this one', { manifest: { ...WELL_FORMED, id: ' ' } }], + ['a non-string `id`', { manifest: { ...WELL_FORMED, id: 123 } }], + ['BARE form, no `id`', without(WELL_FORMED, 'id')], + ]; + + for (const [label, body] of cases) { + it(`${label} → still 400 \`Package id is required\``, async () => { + const door = makeDoor(); + const r = await install(door, body); + + expect(r.response?.status).toBe(400); + expect(messageOf(r)).toContain('Package id is required'); + // ⭐ The other direction, and the reason this section exists. `''` + // fails `MANIFEST_ID_PATTERN` as surely as `pkg-a` does, so a gate + // placed one line EARLIER would answer these with the schema's + // sentence instead — replacing a published message for bodies this + // door already refused, and doing it on the one input where the + // refusal's suggestion arm has nothing to offer. The gate is + // ordered after the `!pkgId` check precisely so this cannot happen, + // and the accept set is all that narrows. + expect(messageOf(r)).not.toContain('Invalid package id'); + }); + + it(`${label} → NEITHER install writer was called`, async () => { + const door = makeDoor(); + await install(door, body); + expect(door.protocolInstall).not.toHaveBeenCalled(); + expect(door.registryInstall).not.toHaveBeenCalled(); + }); + } +}); + +describe('§3b the id gate is ordered AHEAD of the `version` gate', () => { + it('an off-spec id with no `version` is answered on the ID, ⛔ not the version', async () => { + const door = makeDoor(); + const r = await install(door, { manifest: without({ ...WELL_FORMED, id: CARD_CASE }, 'version') }); + + // The version refusal's sentence NAMES the id it is prescribing for + // («add it to the manifest for ''»). Prescribing a `version` repair + // for an id that can never be legal sends the author round twice, so + // the id is judged first. + expect(r.response?.status).toBe(400); + expect(messageOf(r)).toBe(manifestIdRefusal('manifest.id', CARD_CASE)); + expect(messageOf(r)).not.toContain('manifest.version is required'); + }); + + it('the version gate is untouched for a CONFORMING id with no `version`', async () => { + const door = makeDoor(); + const r = await install(door, { manifest: without(WELL_FORMED, 'version') }); + + // The lit control that makes the case above a statement about ORDER + // rather than about the version gate having been removed. + expect(r.response?.status).toBe(400); + expect(messageOf(r)).toContain('manifest.version is required'); + expect(messageOf(r)).not.toContain('Invalid package id'); + }); +}); + +describe('§3c the id gate is ordered AHEAD of the duplicate-id 409', () => { + it('an already-installed OFF-SPEC id answers 400, ⛔ not 409', async () => { + const door = makeDoor({ existingId: CARD_CASE }); + const r = await install(door, { manifest: { ...WELL_FORMED, id: CARD_CASE } }); + + // A request-shape refusal must not depend on server state: placed after + // the duplicate check, one and the same unpublishable id would answer + // 400 or 409 according to whether it happened to be installed already — + // two different answers to one authoring mistake, and the 409 tells the + // author nothing about the real problem. + expect(r.response?.status).toBe(400); + expect((r.response as any)?.body?.error?.code).toBe('VALIDATION_ERROR'); + }); + + it('the 409 is untouched for a conforming duplicate', async () => { + const door = makeDoor({ existingId: WELL_FORMED.id }); + const r = await install(door, { manifest: WELL_FORMED }); + + // The control that makes the case above a statement about ORDER rather + // than about the 409 having been removed. + expect(r.response?.status).toBe(409); + expect(door.protocolInstall).not.toHaveBeenCalled(); + }); + + it('`?overwrite=true` still reaches 201 for a conforming duplicate', async () => { + const door = makeDoor({ existingId: WELL_FORMED.id }); + const r = await install(door, { manifest: WELL_FORMED }, { overwrite: 'true' }); + + // The opt-in past the 409 is a separate branch and this gate sits ahead + // of it too — so the control needs its own limb, or «ahead of the 409» + // would be pinned on the refusing path alone. + expect(r.response?.status).toBe(201); + }); + + it('⛔ `?overwrite=true` does NOT buy past the id gate', async () => { + const door = makeDoor({ existingId: CARD_CASE }); + const r = await install(door, { manifest: { ...WELL_FORMED, id: CARD_CASE } }, { overwrite: 'true' }); + + // `overwrite` opts back into replacing an existing package. It has never + // been an opt-out of the request's own shape, and an id the publish face + // refuses is a shape defect whichever way the duplicate branch goes. + expect(r.response?.status).toBe(400); + expect(door.protocolInstall).not.toHaveBeenCalled(); + expect(door.registryInstall).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index 6db66d8936e..0df0cab46a2 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -71,15 +71,23 @@ import { organizationIdForMetaWrite } from '@objectstack/metadata-core'; // that already call it — the dataset query in `rest-server.ts`, the cold-boot // flow bind in `service-automation`, and `saveMetaItem`'s verbatim persist. import { stripReadDecorations } from '@objectstack/spec/kernel'; -// [#19120] The DECLARED grammar of one manifest key, asked BY REFERENCE at the -// install door below. `ManifestSchema.shape.version` is the very field schema +// [#19120 / #19417] The DECLARED grammar of TWO manifest keys, asked BY +// REFERENCE at the install door below. `ManifestSchema.shape.version` and +// `ManifestSchema.shape.id` are the very field schemas // `PackageInstallRequestSchema` binds through `manifest: ManifestSchema` — not -// a copy of it. ⛔ A hand-written semver regex here would be the THIRD judgment -// of this one key on this one surface (the `PATCH /packages/:id` door further -// down already keeps its own copy), and the version-grammar canon is an open -// question on its own card: asking the declaration means whatever that canon -// decides reaches this door with no edit to this file. -import { ManifestSchema } from '@objectstack/spec/kernel'; +// copies of them. ⛔ A hand-written semver regex here would be the THIRD +// judgment of that one key on this one surface (the `PATCH /packages/:id` door +// further down already keeps its own copy), and the version-grammar canon is an +// open question on its own card: asking the declaration means whatever that +// canon decides reaches this door with no edit to this file. The same holds for +// the id — `MANIFEST_ID_PATTERN` is declared ONCE in `kernel/manifest.zod.ts` +// and shared with `PackageSchema.manifestId`, so ⛔ no reverse-domain regex is +// spelled here either. +// +// `manifestIdRefusal` is imported for exactly one limb: the fallback when a +// failed parse somehow carries no issue. Even that limb then prints the +// DECLARATION's own sentence rather than a second one invented here. +import { ManifestSchema, manifestIdRefusal } from '@objectstack/spec/kernel'; // [#17672] The repo's ONE message for a single-valued query parameter supplied // more than once, from the module whose header is the authority on the rule // (`packages/rest/src/query-multiplicity.ts`). Imported, never restated: this @@ -881,6 +889,86 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin if (!pkgId) { return { handled: true, response: deps.error('Package id is required', 400) }; } + // [#19417] ⭐ THE DOOR PARSES THE `id` LEG — the declaration, by + // reference, exactly as the `version` leg below is parsed. + // + // `MANIFEST_ID_PATTERN` (`packages/spec/src/kernel/manifest.zod.ts`) + // is the reverse-domain rule declared ONCE and referenced by BOTH + // faces of this identity — `ManifestSchema.id`, what an author + // writes, and `PackageSchema.manifestId`, what the registry stores + // and publishes by. This door read `manifest.id` POSITIONALLY and + // parsed nothing, so `id: 'pkg-a'` installed and answered `201` + // while `defineStack()`, `os build`, `os validate` and the publish + // face all refused the same id. The author got a package that could + // never be rebuilt or published — «declared ≠ enforced» on a + // PUBLISHED API contract, the shape Prime Directive #10 refuses + // outright, and the failure 北极星 clause 4 names in as many words: + // 「错的必须被**响亮拒绝**并给处方,**永不静默落库**」. + // + // The authorising ruling is 基本裁决原则 —「声明而未兑现是实现缺口, + // 补实现或退役,⛔ 不在消费端收窄」— and by the mechanical boundary + // test, making a door parse what its schema ALREADY declares is + // 拉回已声明契约, ⛔ not 扩大接受集. Nothing in `packages/spec` moves + // for this; the declaration was already right. + // + // ⭐ THE SENTENCE IS THE DECLARATION'S, NOT THIS FILE'S. The issue + // message is SURFACED rather than reworded: `manifestIdRefusal` + // names the key, echoes the value the author wrote, lists the + // examples, and carries a suggestion arm that VERIFIES its candidate + // against the pattern before offering it. Rewording it here would + // have produced a fourth sentence for one rule and dropped the + // repair. + // + // ⛔ SCOPE — THE `id` LEG ALONE. The declaration's residual docblock + // records the classes this door still answers `201` to; a missing + // `type`, unknown keys on either body form, a string-typed + // `enableOnInstall`/`overwrite` and install options spelled on the + // bare form are each their own narrowing of a published wire + // contract and are deliberately LEFT STANDING. Closing them is the + // ONE call this code still pointedly does not make, + // `PackageInstallBodySchema.safeParse(body)`. + // + // ⭐ ORDERED AFTER THE `!pkgId` GATE, DELIBERATELY — and that is a + // decision, because `''` fails the pattern too. Left of this gate, + // an absent id would stop printing `Package id is required` and + // start printing `Invalid package id ''`, which is the ONE input + // where the refusal's suggestion arm has nothing to offer: a + // PUBLISHED message replaced by a weaker one, for a body this door + // already refused. The gate below therefore narrows the ACCEPT SET + // only. `packages-install-manifest-version.test.ts` pins the same + // precedence for the version leg («no id means no sentence this gate + // could print»), and the artifact path's DOOR-1 precedent is not + // this door: there the schema parse is the FIRST door, with no + // published `required` sentence ahead of it to displace. + // + // ⭐ ORDERED BEFORE THE `version` GATE, for that same reason read + // one key over: the version refusal's sentence NAMES the id + // («add it to the manifest for ''»). Prescribing a repair for a + // package id that can never be legal sends the author round twice. + // + // ⛔ THE RAW VALUE IS PARSED, not `pkgId`. The trim above keys the + // package; it must not also launder the id past its own rule, or + // `' com.acme.crm '` would keep installing a manifest whose stored + // `id` the declaration refuses. Downstream this makes the trim a + // no-op by construction — the pattern admits no whitespace — so + // every accepted path now has `pkgId === manifest.id`. + // + // ⛔ HTTP-DOOR-ONLY BY CONSTRUCTION, as the version leg is: + // boot-time and in-process installs reach + // `SchemaRegistry.installPackage` / `registerApp` directly and never + // pass through this branch. + const rawId = (manifest as any)?.id; + const declaredId = ManifestSchema.shape.id.safeParse(rawId); + if (!declaredId.success) { + const [issue] = declaredId.error.issues; + return { + handled: true, + response: deps.error( + issue?.message || manifestIdRefusal('manifest.id', rawId), + 400, + ), + }; + } // [#19120] ⭐ THE DOOR PARSES THE `version` LEG — the declaration, // by reference. //