fix(spec)!: PackageInstallRequestSchema's wrapped branch refuses an unknown top-level key by name

.changeset/20249-install-request-wrapped-strict.md

@@ -0,0 +1,62 @@
+---
+'@objectstack/spec': minor
+---
+
+fix(spec): `PackageInstallRequestSchema`'s wrapped branch refuses an unknown top-level key by name (#20249)
+
+Clause-②: no (narrowing)
+
+**BREAKING for callers of the install door** — a WRAPPED install body
+(`{ manifest, … }`) that carries any top-level key the declaration does not
+name is now refused `400` / `VALIDATION_ERROR` at `POST /api/v1/packages`, and
+`PackageInstallRequestSchema` / `PackageInstallBodySchema` refuse it at parse.
+It used to parse green with the key silently DROPPED, and the door installed
+the package and answered `201`.
+
+This one narrows the declaration itself. The manifest and the bare form (a
+manifest as the whole body) already refused an unknown key by name; the wrapped
+top level was the one position of the install contract still declared strip
+mode. The sharp case is a misspelled option: `{ manifest, enabledOnInstall:
+false }` had `enabledOnInstall` dropped, so the package was installed
+**ENABLED** — the caller's explicit `false` inverted, with no word said. The
+wrapped branch is now a `strictObject`, like the other two positions: one rule
+for the whole install contract (decision batch #227 item 3, letter A; ruling
+record `5856869656`). No alias and no grace window.
+
+The install door needs no edit and gets none: since the door started parsing
+its whole body through `PackageInstallBodySchema`, it answers exactly what that
+declaration says, so the refusal reaches `POST /api/v1/packages` the moment the
+declaration moves. The same fact retires the sentence that had forbidden this
+close — «the declaration must not refuse a body the door answers `201` to» held
+only while the door did not parse its body.
+
+**What is not affected.** A wrapped body carrying only `manifest` and the
+declared install options — `settings`, `enableOnInstall`, `overwrite`,
+`platformVersion`, `artifactRef` — parses and installs exactly as before, and so
+does a bare manifest. Boot-time and in-process installs reach
+`SchemaRegistry.installPackage` / `ObjectQL.registerApp` directly and never pass
+through this declaration.
+
+**Reach, measured first-party.** The SDK's `client.packages.install` sends only
+`manifest`, `settings`, `enableOnInstall` and `overwrite`, and the objectui
+package dialog sends only `{ manifest }`; neither breaks.
+**Out-of-repo callers are NOT MEASURED** — there is no telemetry on them, so a
+caller that sends its own private top-level key (a trace id, a source tag) now
+gets a `400` naming that key. Check your own callers before upgrading rather
+than inheriting this result.
+
+**Migration — FROM → TO.** The refusal names the key and, for a near-miss,
+offers the declared one, so the prescription arrives with the `400`:
+
+- A misspelled option: FROM `{ "manifest": { … }, "enabledOnInstall": false }`
+  TO `{ "manifest": { … }, "enableOnInstall": false }` — respell it as the
+  declared option it meant.
+- Any other undeclared top-level key: FROM `{ "manifest": { … }, "_source":
+  "studio" }` TO `{ "manifest": { … } }` — remove it. There is no place on this
+  request to carry it.
+
+It is registered as an ADR-0087 structured TODO rather than a conversion: an
+unknown key has no mapping target, and deleting it automatically would repeat
+the silent drop this change closes.
+
+<!-- adr-0087: registered package-install-request-unknown-keys-refused -->

packages/runtime/src/domains/packages-install-body-contract.test.ts

@@ -27,14 +27,17 @@
  * enforced too: `name`, the `namespace` grammar, the closed value sets, the
  * retired-key tombstones and the nested blocks it closes. §8 pins that.
  *
- * ## What is deliberately NOT refused — the declaration decides
+ * ## The wrapped TOP LEVEL — refused too, because the declaration closed it
  *
- * An unknown key at the TOP LEVEL of the wrapped form. The wrapped branch is a
- * plain `z.object` (strip mode) and its docblock forbids closing it here, so
- * `{ manifest, bogus }` parses green with `bogus` dropped. ⛔ No case below
- * pins that as intended behaviour in either direction: §5 asserts the door
- * AGREES WITH THE DECLARATION on it, which stays green whichever way the
- * declaration is later decided.
+ * An unknown key at the TOP LEVEL of the wrapped form was the one position
+ * this door still admitted when the whole-body parse landed: the wrapped
+ * branch was strip mode, so `{ manifest, enabledOnInstall: false }` — a
+ * misspelled `enableOnInstall` — parsed green with the key dropped and
+ * installed the package ENABLED. Decision batch #227 item 3, letter A (ruling
+ * record `5856869656`) closed that branch in `@objectstack/spec`, and the door
+ * followed with no edit of its own, because it asks the declaration. §5 pins
+ * the refusal at this door: the status, the named key, and that nothing was
+ * installed.
  *
  * ## Envelope
  *
@@ -170,6 +173,8 @@ describe('§0 the declaration is what the door is held to', () => {
             { ...m, enableOnInstall: false },
             { ...m, overwrite: true },
             { ...m, settings: { a: 1 } },
+            { manifest: m, enabledOnInstall: false },
+            { manifest: m, _source: 'studio' },
         ];
         for (const body of refused) {
             expect(PackageInstallBodySchema.safeParse(body).success, JSON.stringify(body)).toBe(false);
@@ -315,23 +320,58 @@ describe('§4 row 4 — install options on the BARE form are refused, naming the
 });
 
 // ═══════════════════════════════════════════════════════════════════════
-// §5 — the door follows the declaration, including where it strips
+// §5 — the wrapped TOP LEVEL: refused, because the declaration closed it
 // ═══════════════════════════════════════════════════════════════════════
 
-describe('§5 the door answers what the declaration answers', () => {
-    it('an unknown TOP-LEVEL key on the wrapped form: door and declaration agree, whichever way that is', async () => {
-        // ⛔ Deliberately a PARITY assertion, not a status pin. The wrapped
-        // branch strips today, so both accept; if the declaration is ever
-        // closed, both must refuse — and the door follows with no edit,
-        // because it asks the declaration.
+describe('§5 an unknown TOP-LEVEL key on the wrapped form is refused by name, and nothing installs', () => {
+    // A STATUS pin, ⛔ not the parity assertion this section used to hold
+    // («door and declaration agree, whichever way that is»): parity stays green
+    // when the declaration is re-opened, because both would then accept, so it
+    // could not tell a closed branch from a strip-mode one. The ruling decided
+    // the direction (decision batch #227 item 3, letter A), so the direction is
+    // what is pinned. Each refusal case reads the declaration's verdict on the
+    // same body too, AFTER the door's answer, so a regression reddens the door
+    // assertion first and the trailing read names whether the declaration moved.
+
+    it('a MISSPELLED option — `enabledOnInstall: false` — is refused naming the key, ⛔ never installed ENABLED', async () => {
         const { registry, install } = door();
-        const m = manifest('toplevel.unknown');
-        const body = { manifest: m, bogus: 1 };
-        const declared = PackageInstallBodySchema.safeParse(body).success;
+        const m = manifest('toplevel.misspelled');
+        const body = { manifest: m, enabledOnInstall: false };
         const r = await install(body);
 
-        expect(r.response?.status).toBe(declared ? 201 : 400);
-        expect(registry.getPackage(m.id) !== undefined).toBe(declared);
+        expectRefused(r);
+        // The inversion this ruling is about: before it, this id was installed
+        // and ENABLED with the caller's `false` dropped. Refused, it is neither.
+        expect(registry.getPackage(m.id)).toBeUndefined();
+        expect(persistedDisabled().has(m.id)).toBe(false);
+        // Named subjects, ⛔ not prose: the key the caller wrote, and the
+        // declared option the refusal offers in its place.
+        expect(messageOf(r)).toContain('`enabledOnInstall`');
+        expect(messageOf(r)).toContain('`enableOnInstall`');
+        // Read LAST, so the door's own answer is what a regression turns red first.
+        expect(PackageInstallBodySchema.safeParse(body).success, 'the declaration refuses it').toBe(false);
+    });
+
+    it('a PRIVATE key — `_source` — is refused naming the key, and nothing installs', async () => {
+        const { registry, install } = door();
+        const m = manifest('toplevel.private');
+        const body = { manifest: m, _source: 'studio' };
+        const r = await install(body);
+
+        expectRefused(r);
+        expect(registry.getPackage(m.id)).toBeUndefined();
+        expect(messageOf(r)).toContain('`_source`');
+        expect(PackageInstallBodySchema.safeParse(body).success, 'the declaration refuses it').toBe(false);
+    });
+
+    it('control — the same body with the option spelled as declared installs, DISABLED as asked', async () => {
+        const { registry, install } = door();
+        const m = manifest('toplevel.control');
+        const r = await install({ manifest: m, enableOnInstall: false });
+
+        expect(r.response?.status).toBe(201);
+        expect(registry.getPackage(m.id)?.enabled).toBe(false);
+        expect(persistedDisabled().has(m.id)).toBe(true);
     });
 });
 

packages/runtime/src/domains/packages.ts

@@ -1000,14 +1000,17 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
             // declaration's own docblock forbids it («a caller that needs an
             // option sends the wrapped form»).
             //
-            // ⚠️ What the union does NOT refuse, deliberately: an unknown key at
-            // the TOP LEVEL of the wrapped form. `PackageInstallRequestSchema`
-            // is a plain `z.object` (strip mode) and its docblock forbids
-            // closing it, so `{ manifest, bogus }` parses green with `bogus`
-            // dropped — and this door, reading only parsed keys, now does
-            // exactly what the declaration says. Refusing it here without the
-            // declaration would make the published contract admit a body the
-            // door refuses; that is the declaration's decision to make.
+            // ⭐ The TOP LEVEL of the wrapped form is closed too — by the
+            // declaration, not by this door. `PackageInstallRequestSchema` is a
+            // `strictObject` (ruling record `5856869656`, decision batch #227
+            // item 3, letter A), so an unknown top-level key such as
+            // `{ manifest, enabledOnInstall: false }` fails the union by name,
+            // and this door answers `400` / `VALIDATION_ERROR` and installs
+            // nothing — with no line of its own for it: reading only parsed
+            // keys, it does exactly what the declaration says. ⛔ Never refuse or
+            // admit a key here that the declaration decides the other way: the
+            // published contract and this door would then answer differently,
+            // and which keys the body may carry is the declaration's decision.
             //
             // The verdict is answered after the `id` and `version` legs below,
             // which keep their own published sentences; every later read (the

packages/spec/src/api/package-api.test.ts

@@ -900,7 +900,7 @@ describe('#18058 — install contract bound to the live door', () => {
     });
   });
 
-  describe('the two declared body forms — disjoint, and only ONE of them closed', () => {
+  describe('the two declared body forms — disjoint, and BOTH closed', () => {
     it('parses the WRAPPED form the client SDK sends', () => {
       const body = { manifest: SDK_MANIFEST, settings: undefined, enableOnInstall: true };
       expect(PackageInstallBodySchema.safeParse(body).success).toBe(true);
@@ -933,23 +933,94 @@ describe('#18058 — install contract bound to the live door', () => {
     });
 
     /**
-     * ⭐ The WRAPPED branch is `z.object`, i.e. STRIP mode — it is NOT closed.
+     * ⭐ The WRAPPED branch is CLOSED — an unknown top-level key is refused by
+     * name, as the manifest's and the bare form's unknown keys are (decision
+     * batch #227 item 3, letter A; ruling record `5856869656`).
      *
-     * An earlier revision of the docblock claimed both branches were closed.
-     * They are not, and the asymmetry is the DOOR's behaviour: the handler
-     * reads `manifest`, `settings`, `enableOnInstall` and `overwrite` and
-     * ignores every other key, so dropping an unknown one is exactly what it
-     * does with it. ⛔ Closing this branch with `.strict()` would refuse bodies
-     * the door answers `201` to — the direction ruling A forbids — so what is
-     * pinned here is the drop, not a refusal.
+     * What this pinned before was the opposite: the wrapped branch was strip
+     * mode, so `{ manifest, enabledOnInstall: false }` parsed green with the
+     * misspelled key DROPPED, and the door installed the package ENABLED — the
+     * caller's explicit `false` inverted. Each refusal is read at BOTH doors of
+     * this declaration: the union the install route is bound to, and the
+     * wrapped branch alone, whose issue is the one the install door locates
+     * and surfaces.
      */
-    it('the WRAPPED branch DROPS an unknown key rather than refusing it', () => {
-      const verdict = PackageInstallBodySchema.safeParse({ manifest: SDK_MANIFEST, bogus: 1 });
-      expect(verdict.success).toBe(true);
-      expect(verdict.data && 'bogus' in verdict.data).toBe(false);
+    describe('the WRAPPED branch refuses an unknown top-level key by name', () => {
+      /** The wrapped branch's own verdict: one `unrecognized_keys` issue, at the top level, naming the key. */
+      function expectRefusedByName(body: Record<string, unknown>, key: string) {
+        expect(PackageInstallBodySchema.safeParse(body).success, 'the bound union refuses it').toBe(false);
+        const verdict = PackageInstallRequestSchema.safeParse(body);
+        expect(verdict.success).toBe(false);
+        const issues = verdict.error?.issues ?? [];
+        expect(issues).toHaveLength(1);
+        expect(issues[0]?.code).toBe('unrecognized_keys');
+        expect(issues[0]?.path).toEqual([]);
+        expect((issues[0] as { keys?: string[] }).keys).toEqual([key]);
+        expect(issues[0]?.message).toContain(`\`${key}\``);
+        return issues[0]!.message;
+      }
+
+      it('a MISSPELLED option — `enabledOnInstall` — is refused, and the declared key is offered', () => {
+        const message = expectRefusedByName({ manifest: SDK_MANIFEST, enabledOnInstall: false }, 'enabledOnInstall');
+        // The remedy's named subject: the option the caller meant.
+        expect(message).toContain('`enableOnInstall`');
+      });
+
+      it('a PRIVATE key — `_source` — is refused by name, not carried and not dropped', () => {
+        expectRefusedByName({ manifest: SDK_MANIFEST, _source: 'studio' }, '_source');
+      });
+
+      it('an unknown key BESIDE declared options is refused too — the options do not buy it a pass', () => {
+        expectRefusedByName(
+          { manifest: SDK_MANIFEST, enableOnInstall: false, overwrite: true, bogus: 1 },
+          'bogus',
+        );
+      });
+    });
+
+    describe('controls — every declared wrapped body still round-trips', () => {
+      /** A value for EVERY declared install option, each one the schema accepts. */
+      const DECLARED_OPTIONS = {
+        settings: { apiKey: 'abc123' },
+        enableOnInstall: false,
+        overwrite: true,
+        platformVersion: '3.2.0',
+        artifactRef: {
+          url: 'https://marketplace.objectstack.io/artifacts/com.acme.crm/1.0.0.tgz',
+          sha256: 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2',
+          size: 1024000,
+          // Spelled out: the artifact reference DEFAULTS it, and a round-trip
+          // fixture must be one the parse hands back unchanged.
+          format: 'tgz',
+          uploadedAt: '2026-02-01T10:00:00Z',
+        },
+      };
+
+      it('the fixture names every declared option — read off the shape, never listed by hand', () => {
+        const declared = Object.keys(PackageInstallRequestSchema.shape).filter((k) => k !== 'manifest');
+        expect(Object.keys(DECLARED_OPTIONS).sort()).toEqual(declared.sort());
+      });
+
+      it('`manifest` alone parses green at both doors', () => {
+        expect(PackageInstallRequestSchema.safeParse({ manifest: SDK_MANIFEST }).success).toBe(true);
+        expect(PackageInstallBodySchema.safeParse({ manifest: SDK_MANIFEST }).success).toBe(true);
+      });
+
+      it('`manifest` plus every declared option parses green, each option value carried through', () => {
+        const body = { manifest: SDK_MANIFEST, ...DECLARED_OPTIONS };
+        const request = PackageInstallRequestSchema.safeParse(body);
+        expect(request.error?.issues ?? []).toEqual([]);
+        const union = PackageInstallBodySchema.safeParse(body);
+        expect(union.error?.issues ?? []).toEqual([]);
+        for (const parsed of [request.data, union.data] as Array<Record<string, unknown> | undefined>) {
+          for (const [key, value] of Object.entries(DECLARED_OPTIONS)) {
+            expect(parsed?.[key], key).toEqual(value);
+          }
+        }
+      });
     });
 
-    it('lit control: the BARE branch IS closed — the same unknown key is refused there', () => {
+    it('the BARE branch is closed too — the same unknown key is refused there', () => {
       // `ManifestSchema` is a `strictObject`, so this is a refusal, not a drop.
       expect(PackageInstallBodySchema.safeParse({ ...SDK_MANIFEST, bogus: 1 }).success).toBe(false);
     });