Skip to content

Commit f9977c1

Browse files
fix(runtime): POST /api/v1/packages parses the manifest's id leg (#19473)
Part of #19417 Clause-②: no (narrowing) ## What landed `POST /api/v1/packages` parses the manifest's `id` leg through `ManifestSchema.shape.id` — the declaration, **by reference** — exactly as the `version` leg beside it has since #19120. An id `MANIFEST_ID_PATTERN` refuses is answered `400` / `VALIDATION_ERROR` carrying the declaration's OWN sentence, and neither install writer runs. Before: `packages/runtime/src/domains/packages.ts` read the id positionally (`typeof manifest?.id === 'string' ? manifest.id.trim() : ''`) and parsed nothing, so a complete manifest with `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. Nothing in `packages/spec` moves. `ManifestSchema.shape.id` is already a plain `ZodString` carrying the regex and its custom refusal, so the escape this order named — *"the id leg cannot be gated by reference without a `packages/spec` change"* — is **false**, measured: ```text typeof leg: object ZodString "pkg-a" -> false Invalid package id 'pkg-a' on `manifest.id`. … Did you mean 'com.example.pkg-a'? "com.acme.crm" -> true "com.example.my_erp"-> false … Did you mean 'com.example.my-erp'? ``` ## The two design calls, and what decided them ### 1. Scope — the `id` leg alone Taken as the seat preferred, and for a reason the measurement supplies rather than a preference: each of the remaining residual classes is a separate narrowing of a published wire contract, and this card was graded on the `manifest.id` class. Measured through `HttpDispatcher.handlePackages` **after** this change: ```text 201 1. manifest missing `type` (wrapped) 201 1. manifest missing `type` (bare) 201 2. unknown key, WRAPPED form (dropped) 201 2. unknown key, BARE form (schema refuses by name) 201 3. string-typed `enableOnInstall` 201 3. string-typed body `overwrite` 201 4. install options on the BARE form 400 CONTROL the card class — id `pkg-a` 201 CONTROL a conforming install ``` All seven spellings of the four left-standing classes still answer `201`; the card's class is the only one that moved, and the conforming control still installs. Closing the rest remains the one call this handler pointedly does not make, `PackageInstallBodySchema.safeParse(body)`. ⚠️ **One deliberate divergence from the order's pin 5.** The order asks for those four to be **pinned** as `201`, saying #19120 did that. #19120 did not: its changeset carries the measurement, and its test file forbids the pin in as many words — *"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 … The separability evidence lives where a one-shot measurement belongs — the PR body — not in a permanent expectation."* That reasoning is sound and landed, so the substance of pin 5 is delivered here (a measured, checkable scope claim, in this body and in the changeset) and its form is not. The probe that produced the table was a one-shot file, run and deleted in the same round. ### 2. Ordering — the gate sits AFTER the empty-id check, and BEFORE the version gate `''` fails `MANIFEST_ID_PATTERN` too, so placement decides whether a **published message** moves or only the accept set does. Three readings decided it: - **The weaker sentence.** On `''` the refusal's suggestion arm has nothing to offer — it verifies its candidate against the pattern and `com.example.` plus an empty string does not match — so a schema-first gate would replace `Package id is required` with `Invalid package id ''` and no remedy, for a body this door **already refused**. The defect on this card is `201`s, not `400`s. - **A landed pin says so.** `packages-install-manifest-version.test.ts` already pins *"the id gate still wins — no id means no sentence this gate could print"*. Measured: disabling the `!pkgId` check turns that landed case red along with five new ones. A schema-first gate reverses a pin #19120 wrote deliberately. - **The artifact-path precedent does not reach here.** `DOOR 1 (schema)` in `artifact-granted-permissions.test.ts:237` is the FIRST door on that path, with no published `required` sentence ahead of it to displace. This door has one. Ordered **before** the `version` gate for the mirror-image reason: the version refusal's sentence names the id it is prescribing for, and prescribing a `version` repair for an id that can never be legal sends the author round twice. ⛔ The **raw** value is parsed, not the trimmed `pkgId`: the trim keys the package and must not also launder an 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 on every accepted path. ## The `pkg-a` pin is REVERSED, not deleted `domain-handler-registry.test.ts` drove the duplicate-id guard with `{ id: 'pkg-a', … }`, whose `forced` limb asserts `201` — unreachable for this id once the door parses it, whatever order the gate sits in. The fixture is repaired to `com.example.pkg-a` (the repair `manifestIdRefusal` itself prescribes for `pkg-a`) so that case keeps testing the duplicate guard, and a new case beside it asserts the `pkg-a` refusal. The reading survives, pointing the other way — the same repair #19120 owed this fixture one key over. ## Verification **Tests** — `pnpm --filter @objectstack/runtime test`: **272 files, 3796 passed, 1 skipped**, at `1736a93b3a`. `pnpm --filter @objectstack/runtime typecheck`: green (test layer included, via `tsconfig.test.json`). **Reverse verification** — two ablations, both through `scripts/ablation-replace.mjs`, each proving the mutation landed on disk (anchor count and `git hash-object` blob) and each proving its own restore (blob equals HEAD, `git diff HEAD` empty). Both ran from the committed fix, and the direction is the one predicted in advance: red. | ablation | what it removes | result | | --- | --- | --- | | `if (!declaredId.success) {` to `… && false) {` | the new gate | blob `0df0cab46a2a` to `4653ea1e6f4c`; **16 failed / 72 passed** — every new pin plus the reversed `pkg-a` one; §0, §2 and §3a stayed green | | `if (!pkgId) {` to `… && false) {` | the empty-id gate ahead of it | blob `0df0cab46a2a` to `6b2a24fbfc8a`; **6 failed / 48 passed** — all five §3a message pins **and** #19120's landed *"the id gate still wins"* | The second ablation is what makes §3a a lit control rather than an assertion that cannot fail, and it is the direct measurement behind design call 2. **Gates** — `node scripts/pm/dispatch-gates.mjs --commands` derived **60** families from the real change set; reconciled with `--ran` carrying every exit code: **59 run green, 1 NOT MEASURED, 0 unrun.** The one not measured is `pnpm check:dual-build-cjs-loads`, which exits **3** with `PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/` (38 packages); it needs a whole-repo `pnpm build`, which is CI's Build Core job. ⛔ Recorded as not measured, not as a pass. `pnpm check:type-check-debt` also exited 3 for the same class on the first pass and was re-run green after building `@objectstack/runtime`. **Repo-wide lint** — `pnpm lint` (`eslint . --no-inline-config`, the repo's only style authority) run **in full** over the whole tree at `1736a93b3a`, exit **0**. No narrowing was needed, so no narrowing is claimed. ## Acceptance notes ⛔ Nothing below is fixed here. - **The residual docblock on `PackageInstallBodySchema` (`packages/spec/src/api/package-api.zod.ts`) has drifted.** It enumerates five classes; class 1 lost its `version` half to #19120, and class 5 — *"it answers `400` in the OPPOSITE direction, to a whitespace-only `id` this declaration admits"* — stopped being true when #18319 gave `ManifestSchema.id` the pattern, as the spec's own test at `package-api.test.ts` already records from the other side. Stale prose in a comment, so noted rather than filed. Successor: the next card that closes one of the remaining residual classes — that docblock is the register those cards are graded against, so it is read before it is edited. `packages/spec` is fenced out of this PR, which is the other reason it is not touched here. - **The trim at the door is now provably inert on every accepted path** — the pattern admits no whitespace, so after the gate `pkgId` equals `manifest.id` byte for byte. Left in place (it still decides the `Package id is required` answer for a whitespace-only id) and documented at the site. Dead-ish code, noted only. - **The second seam recorded on this card is NOT closed here** — see below. ## ⭐ Why `Part of` and not a closing keyword — the one thing to read before merging The card carries a **second seam**, added to its thread by the filing seat: `packages/metadata-protocol/src/protocol.ts` builds `dupManifest` with `id: request.targetPackageId` and writes it through `installPackage` with no `ManifestSchema` parse anywhere in that file, while a few lines up it already *assumes* the reverse-domain shape by defaulting the namespace to `targetPackageId.split('.').pop()`. That is the duplicate door, not this HTTP install door, and it is untouched by this PR. The dispatch order asked for a closing keyword. Its re-derived premise, its file surface and all five of its pins are about the install door alone, and it does not mention the second seam anywhere — so the order appears not to have weighed it. A closing keyword here would close the card with a live, explicitly recorded seam still open, and GitHub's parser ignores any prose written to prevent that. `Part of` is the reversible half of that choice: the card stays open, and the seat decides — either file the duplicate-door seam as its own card and then retire #19417 by hand, or edit this body's first line. ⛔ This is flagged, not decided, and it is the one place this PR departs from its order without the order having left the question open. --- _Generated by [Claude Code](https://claude.ai/code/session_01QCdUBjM47SxioST9z5Zwdf)_ --- ### Seat edit — 2026-09-21T01:50Z Two words rewritten by the `domain:cli` PM seat #6024, and nothing else: *"…as its own card **and then retire** #19417 by hand…"*. The previous wording bound a closing keyword to `#19417` **across a newline**, which GitHub's parser joins — so this body declared `Part of #19417` and a closing binding on the same number, the shape `check:partof-closing-keyword` RULE 1 refuses, and with `squash_merge_commit_message = PR_BODY` it would have retired the card on merge. ⭐ **`Part of` is kept deliberately.** Verified before editing: the card carries a **second seam of the same class** that this PR does not touch — `packages/metadata-protocol`'s duplicate door, recorded on #19417 by the filing seat at comment `5751874616`. So #19417 must survive this merge and be retired by hand once that seam has a card of its own. A closing keyword would have been the irreversible half of that choice. **Receipt for the docs drift check (`5754348208`)** — read in full and swept by hand, ⛔ not deferred: The check named **14** hand-written pages, but most matched on `/packages/:id` *"a path literal in a **comment** on a changed line"* — a comment mentioning a route is not an assertion this diff can falsify. Scoped to the one anchor that can be — `manifest.id`, since this change makes ids the door used to accept start being refused: - **0** pages carry a package-id example that `MANIFEST_ID_PATTERN` refuses. - ⭐ **Lit control**, 3 real package-id examples, all passing: `com.acme.crm` (`api/declarative-endpoints.mdx:50`, `protocol/kernel/http-protocol.mdx:1182`) and `com.example.my-app` (`deployment/cli.mdx:2036`). ⇒ clean negative; nothing filed. The 3 release-owned pages matched via the same comment-literal anchor and are read-only regardless. ⚠️ Three impostor hits were caught and discarded on the way, recorded so the next sweep does not trust a count: a first probe matched every `id:` in the corpus (56 "refusals" that were record ids like `acc_123` and nav ids like `nav_accounts`); a tightened context heuristic still surfaced `ui/apps.mdx:538` `id: 'active_package'`, which is a contextSelector's own id and matched only because `valueKey: 'manifest.id'` sits five lines below it. Opening each hit is the only detector. --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5e7d83c commit f9977c1

4 files changed

Lines changed: 652 additions & 9 deletions

File tree

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
---
2+
'@objectstack/runtime': minor
3+
---
4+
5+
fix(runtime): `POST /api/v1/packages` parses the manifest's `id` leg instead of reading it positionally (#19417)
6+
7+
Clause-②: no (narrowing)
8+
9+
**BREAKING for callers of the install door** — a manifest whose `id` is not
10+
reverse-domain notation is now refused `400` / `VALIDATION_ERROR`. It used to
11+
install and answer `201`.
12+
13+
The accept set only shrinks back to what the published declaration has always
14+
said. `MANIFEST_ID_PATTERN` is declared once in
15+
`packages/spec/src/kernel/manifest.zod.ts` and referenced by both faces of one
16+
identity — `ManifestSchema.id`, what an author writes, and
17+
`PackageSchema.manifestId`, what the registry stores and publishes by. The door
18+
read `manifest.id` POSITIONALLY (`typeof manifest?.id === 'string' ?
19+
manifest.id.trim() : ''`) and parsed nothing, so `id: 'pkg-a'` installed and
20+
answered `201` while `defineStack()`, `os build`, `os validate` and the publish
21+
face all refused the same id. The author was handed a package that could never
22+
be rebuilt or published. That is «declared ≠ enforced» on a published API
23+
contract — and nothing in `packages/spec` moves for it: the declaration was
24+
already right.
25+
26+
The gate asks the declaration **by reference** — `ManifestSchema.shape.id` —
27+
rather than keeping a copy of the grammar, exactly as the `version` leg beside
28+
it does, so a future move of the reverse-domain rule reaches this door with no
29+
further edit. The sentence the caller reads is the declaration's own
30+
(`manifestIdRefusal`), **surfaced rather than reworded**: it names the key,
31+
echoes the value, lists the two examples, and carries a suggestion arm that
32+
verifies its candidate against the pattern before offering it. Posting
33+
`id: 'pkg-a'` now answers, in the response envelope's `error.message`:
34+
35+
```text
36+
Invalid package id 'pkg-a' on `manifest.id`. Expected reverse-domain notation
37+
('com.steedos.crm', 'org.apache.superset') — lowercase dot-separated segments;
38+
hyphens allowed inside a segment, underscores are not. Did you mean
39+
'com.example.pkg-a'?
40+
```
41+
42+
**What is not affected.** Boot-time and in-process installs reach
43+
`SchemaRegistry.installPackage` / `ObjectQL.registerApp` directly and never pass
44+
through this branch, so nothing about how a package is loaded from disk or
45+
registered by a plugin changes. A conforming manifest installs exactly as
46+
before, on both body forms (wrapped and bare) and on both install limbs (the
47+
protocol primitive and the bare-registry fallback).
48+
49+
**The `Package id is required` sentence does NOT move.** `''` fails
50+
`MANIFEST_ID_PATTERN` too, so where this gate sits decides whether a published
51+
message changes or only the accept set does. It is ordered AFTER the existing
52+
`!pkgId` check: an absent, empty, whitespace-only or non-string `id` still
53+
answers `400 Package id is required`, never the schema's sentence — which on
54+
that input is the one case where the suggestion arm has nothing to offer.
55+
Measured both directions. It is ordered BEFORE the `version` gate for the
56+
mirror-image reason: that refusal's sentence names the id it prescribes for, and
57+
prescribing a `version` repair for an id that can never be legal sends the
58+
author round twice.
59+
60+
**Scope — the `id` leg alone.** The declaration's residual docblock records the
61+
classes this door answers `201` to while `PackageInstallBodySchema` refuses
62+
them. This change closes one: `id`. A missing `type`, unknown keys on either
63+
body form, a string-typed `enableOnInstall` / `overwrite`, and install options
64+
spelled on the bare form are each left exactly as they were — measured after the
65+
change, all seven spellings of those four classes still answer `201`, against a
66+
`pkg-a` control that answers `400` and a conforming control that answers `201`.
67+
Each is its own reading and its own card. Closing them is the one call this
68+
handler still pointedly does not make, `PackageInstallBodySchema.safeParse(body)`.
69+
70+
**If you are refused.** Give the manifest an id in reverse-domain notation —
71+
lowercase dot-separated segments, hyphens allowed inside a segment, underscores
72+
not. The refusal names the key, echoes what you wrote and, where a mechanical
73+
repair exists, offers one it has already checked against the rule, so the
74+
prescription arrives with the `400` rather than in a changelog.
75+
76+
<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable is removed, renamed or reshaped: no spec key, no export, no stored row. `objectstack migrate meta` has nothing to reach, because there is no old spelling that maps to a new one — a caller supplies an id the declaration already required, and an id's repair changes the package's identity, so no mechanical mapping could be prescribed even in principle. The refusal itself carries the remedy. -->

‎packages/runtime/src/domain-handler-registry.test.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -588,13 +588,41 @@ describe('HttpDispatcher extracted domains (PR-5: packages)', () => {
588588
// ⚠️ The repair is owed whatever order the new gate sits in: the
589589
// `forced` limb asserts `201`, which an under-specified manifest must
590590
// never reach — so no placement of that gate leaves this fixture valid.
591-
const manifest = { id: 'pkg-a', name: 'A', version: '1.0.0' };
591+
//
592+
// [#19417] Same repair, one key over — and the LAST one this fixture
593+
// owed. `ManifestSchema.id` carries `MANIFEST_ID_PATTERN` since #18319,
594+
// `pkg-a` is not reverse-domain notation, and the install door parses
595+
// that leg now: the `forced` limb's `201` was unreachable for this id.
596+
// ⛔ The `pkg-a` reading is REVERSED, never deleted — the case below
597+
// keeps it, asserting the refusal this fixture used to assert the
598+
// acceptance of. `com.example.pkg-a` is the same name made publishable,
599+
// which is the repair `manifestIdRefusal` itself prescribes for it.
600+
const manifest = { id: 'com.example.pkg-a', name: 'A', version: '1.0.0' };
592601
const dup = await dispatcher.dispatch('POST', '/packages', manifest, {}, {} as any);
593602
expect(dup.response?.status).toBe(409);
594603
const forced = await dispatcher.dispatch('POST', '/packages', manifest, { overwrite: 'true' }, {} as any);
595604
expect(forced.response?.status).toBe(201);
596605
});
597606

607+
it('[#19417] POST /packages refuses `pkg-a` — the id the pattern refuses (the REVERSED pin)', async () => {
608+
// ⭐ This case is the other half of the repair above, and it is why that
609+
// repair is not a deletion. `pkg-a` was the one standing pin on the gap
610+
// this card closes: the door answered it `201` while `defineStack()`,
611+
// `os build`, `os validate` and the publish face all refused it, so an
612+
// author could install a package that could never be rebuilt or
613+
// published. The reading is kept, pointing the other way.
614+
//
615+
// ⛔ Deliberately thin here — status only. The full pins (the
616+
// declaration's own sentence, both install writers silent, and the
617+
// gate's placement against `Package id is required`, the `version` leg
618+
// and the `409`) live in `domains/packages-install-manifest-id.test.ts`
619+
// beside the door. This file's subject is ROUTING.
620+
const objectql = qlWithRegistry();
621+
const result = await withPkgCaller(makeDispatcher({ objectql }))
622+
.dispatch('POST', '/packages', { id: 'pkg-a', name: 'A', version: '1.0.0' }, {}, {} as any);
623+
expect(result.response?.status).toBe(400);
624+
});
625+
598626
it('POST /packages without an id is rejected with 400', async () => {
599627
const objectql = qlWithRegistry();
600628
const result = await withPkgCaller(makeDispatcher({ objectql })).dispatch('POST', '/packages', { name: 'no-id' }, {}, {} as any);

0 commit comments

Comments
 (0)