From 427a73eb3082baeacb1165fc8d68758efa09ba4c Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Wed, 24 Jun 2026 21:11:32 +0800 Subject: [PATCH] =?UTF-8?q?feat(objectql):=20reject=20runtime=20creates=20?= =?UTF-8?q?into=20read-only=20packages=20=E2=80=94=20writable=5Fpackage=5F?= =?UTF-8?q?required=20(ADR-0070=20D1/D2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the #2252 coerce-to-null stopgap with an actionable rejection, and generalizes isLoadedPackage into isWritablePackage (drops the owns-an-object heuristic that was the read-only-after-publish trap). Updates the repo-path tests to assert rejection. Null stays a legacy overlay destination (D5 retires). Co-Authored-By: Claude Opus 4.8 --- .../adr-0070-kernel-writable-package.md | 21 ++++ .../src/protocol-save-meta-repo-path.test.ts | 59 ++++++---- packages/objectql/src/protocol.ts | 110 +++++++++--------- 3 files changed, 112 insertions(+), 78 deletions(-) create mode 100644 .changeset/adr-0070-kernel-writable-package.md diff --git a/.changeset/adr-0070-kernel-writable-package.md b/.changeset/adr-0070-kernel-writable-package.md new file mode 100644 index 0000000000..c41b02317c --- /dev/null +++ b/.changeset/adr-0070-kernel-writable-package.md @@ -0,0 +1,21 @@ +--- +"@objectstack/objectql": patch +--- + +feat(objectql): enforce package-first authoring at the kernel (ADR-0070 D1/D2) + +A runtime-only metadata **create** that targets a read-only code/installed +package now throws `writable_package_required` (status 422) instead of silently +coercing `package_id` to `null`. The old coercion (#2252 stopgap) unblocked +editing but scattered orphans into a package-less bucket with no container to +delete (#1946); the rejection instead directs the authoring surface (Studio / +AI) to pick or create a writable base first. + +`isLoadedPackage` is generalized into `isWritablePackage` (D2): a package is +writable unless it is a booted code package (registered in the engine manifest +map) or a `system`/`cloud`-scoped installed package. The old "owns ≥1 registered +object" heuristic is dropped — it was the read-only-after-publish trap (#2252), +since a writable base accrues registered objects once its drafts publish. + +`null` is still accepted as the legacy org-overlay destination; ADR-0070 D5 +retires it after the orphan migration. diff --git a/packages/objectql/src/protocol-save-meta-repo-path.test.ts b/packages/objectql/src/protocol-save-meta-repo-path.test.ts index 62d17712c8..195c426958 100644 --- a/packages/objectql/src/protocol-save-meta-repo-path.test.ts +++ b/packages/objectql/src/protocol-save-meta-repo-path.test.ts @@ -283,30 +283,49 @@ describe('saveMetaItem — repository write path (post PR-10d.6)', () => { return captured; } - it('runtime-only create while a LOADED code package is selected drops the binding (package_id = null)', async () => { - // The bug: a brand-new object authored while a loaded code package was - // selected in Studio's dropdown got `package_id = `. The - // object is NOT artifact-backed (intent = 'runtime-only') and the - // package IS loaded (its manifest is registered), so the binding must - // be dropped — otherwise it reads back as code-provided / read-only. + it('runtime-only create into a LOADED code package is rejected (writable_package_required)', async () => { + // ADR-0070 D1: a brand-new object authored while a loaded code package + // is selected must NOT be silently coerced to a package-less orphan + // (the old #2252 behaviour) — it is rejected so the authoring surface + // redirects the user to pick or create a writable base first. const { engine } = makeStubEngine(); engine.manifests = new Map([['app.objectstack.hotcrm', { id: 'app.objectstack.hotcrm' }]]); const inserts = spyInserts(engine); const protocol = new ObjectStackProtocolImplementation(engine); - const result = await protocol.saveMetaItem({ - type: 'object', - name: 'maint_asset', - organizationId: 'org_alpha', - packageId: 'app.objectstack.hotcrm', // Studio had a code package selected - mode: 'draft', - item: { name: 'maint_asset', label: 'Asset', fields: { name: { type: 'text', label: 'Name' } } }, - }); - expect(result.success).toBe(true); - const create = inserts.find((d) => d.type === 'object' && d.name === 'maint_asset'); - expect(create).toBeTruthy(); - // brand-new org object must NOT carry the package binding → stays editable. - expect(create!.package_id ?? null).toBeNull(); - expect(create!.package_id).not.toBe('app.objectstack.hotcrm'); + await expect( + protocol.saveMetaItem({ + type: 'object', + name: 'maint_asset', + organizationId: 'org_alpha', + packageId: 'app.objectstack.hotcrm', // Studio had a code package selected + mode: 'draft', + item: { name: 'maint_asset', label: 'Asset', fields: { name: { type: 'text', label: 'Name' } } }, + }), + ).rejects.toMatchObject({ code: 'writable_package_required', status: 422 }); + // The orphan is never created — nothing was persisted. + expect(inserts.find((d) => d.type === 'object' && d.name === 'maint_asset')).toBeUndefined(); + }); + + it('runtime-only create into a system-scoped installed package is rejected', async () => { + // ADR-0070 D2: read-only is also detected via manifest scope — an + // installed/platform package (scope system|cloud) that is NOT in the + // engine manifest map is still not a writable base. + const { engine } = makeStubEngine(); + engine.registry.getPackage = (id: string) => + id === 'platform.core' ? { manifest: { id, scope: 'system' } } : undefined; + const inserts = spyInserts(engine); + const protocol = new ObjectStackProtocolImplementation(engine); + await expect( + protocol.saveMetaItem({ + type: 'object', + name: 'maint_asset', + organizationId: 'org_alpha', + packageId: 'platform.core', + mode: 'draft', + item: { name: 'maint_asset', label: 'Asset', fields: { name: { type: 'text', label: 'Name' } } }, + }), + ).rejects.toMatchObject({ code: 'writable_package_required', status: 422 }); + expect(inserts.find((d) => d.type === 'object' && d.name === 'maint_asset')).toBeUndefined(); }); it('runtime-only create into a NON-loaded package keeps its binding (ADR-0048 #1824 authoring scope)', async () => { diff --git a/packages/objectql/src/protocol.ts b/packages/objectql/src/protocol.ts index 2b454a8fe7..5a0ea0f810 100644 --- a/packages/objectql/src/protocol.ts +++ b/packages/objectql/src/protocol.ts @@ -3162,41 +3162,36 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { } /** - * True when `packageId` refers to a **loaded code package** — one that - * booted and registered a manifest (and typically objects/apps) into the - * engine. Such packages are read-only artifacts; runtime-authored items - * must not be bound to them (see {@link saveMetaItem}). + * True when `packageId` is a **writable base** — a DB-backed package an + * org or the AI may author *new* metadata into (ADR-0070 D2). The two + * read-only kinds return `false`: * - * A bare ADR-0048 *authoring-workspace* package id (no booted manifest, - * no registered metadata) returns `false`, so per-package authoring scope - * is preserved. Reads the engine's manifest map first (authoritative, - * O(1) — every `registerApp` call records the manifest there), then falls - * back to "owns ≥1 registered object" for defense in depth. + * • **Booted code packages** — they register a manifest into the engine + * at startup (`registerApp` → `engine.manifests`); their items are + * code-shipped artifacts. Only `allowOrgOverride` overlays are allowed + * (ADR-0005), never fresh authored items. + * • **Installed / platform packages** — manifest `scope` is `system` or + * `cloud` (marketplace / platform-delivered). + * + * A project-scoped DB package, or a bare ADR-0048 *authoring-workspace* id + * with no registered manifest, is writable. + * + * NOTE: the code-package signal is the engine manifest map ONLY — we + * deliberately do NOT fall back to "owns ≥1 registered object" (the old + * `isLoadedPackage` heuristic). A writable base accrues registered objects + * once its drafts publish, and that must never flip the base to read-only + * — that is the exact #2252 read-only-after-publish trap this ADR removes. */ - private isLoadedPackage(packageId: string): boolean { + private isWritablePackage(packageId: string | null | undefined): boolean { + if (!packageId) return false; const engine = this.engine as any; - if (engine?.manifests?.has?.(packageId)) return true; - const registry = engine?.registry; - if (!registry) return false; - try { - // Objects contributed by the package (real data packages). - if (typeof registry.getAllObjects === 'function' - && registry.getAllObjects(packageId).length > 0) { - return true; - } - // UI / logic metadata bound to the package id. ADR-0048 — a code - // package registers its app via `registerApp(app, packageId)`, so - // the app item's `_packageId` is the package id; UI-only packages - // (no objects) are still detected here. - if (typeof registry.listItems === 'function') { - for (const t of ['app', 'view', 'page', 'flow', 'report', 'dashboard', 'agent', 'skill', 'role', 'permission']) { - if (registry.listItems(t, packageId).length > 0) return true; - } - } - } catch { - // A partial registry (test mocks) → treat as "not loaded". - } - return false; + // Booted code package → read-only artifact source. + if (engine?.manifests?.has?.(packageId)) return false; + // Installed / platform package → read-only by manifest scope. + const scope = engine?.registry?.getPackage?.(packageId)?.manifest?.scope; + if (scope === 'system' || scope === 'cloud') return false; + // Project-scoped base, or unregistered authoring-workspace id → writable. + return true; } /** @@ -3724,39 +3719,38 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { const intent: 'override-artifact' | 'runtime-only' = artifactBacked ? 'override-artifact' : 'runtime-only'; - // GUARD — a brand-new, DB-only ("runtime-only") metadata item must - // not be bound to a *loaded code package*. Studio sends the - // currently-selected package via `?package=`; when a user authors a - // new object while browsing a code package (e.g. `app.objectstack.hotcrm`), - // persisting that id as the new row's `package_id` makes the org - // object read back as "provided by a code package" and become - // read-only after publish — the user can no longer edit what they - // just created. Drop the binding so it is a plain org overlay - // (`package_id = null`, editable). + // D1 (ADR-0070) — a brand-new, DB-only ("runtime-only") metadata + // item MUST resolve to a WRITABLE base. Binding it to a read-only + // code/installed package makes it read back as "code-provided" and + // lock read-only after publish (the #2252 bug). We used to silently + // coerce such a binding to `null`, but that scattered orphans into a + // package-less bucket with no container to delete; ADR-0070 replaces + // the coercion with an actionable rejection so the authoring surface + // (Studio / AI) redirects the user to pick or create a base first. // - // Two scopes are deliberately left bound: + // Left untouched (the binding survives): // • `override-artifact` writes — an org overlay OF a packaged item - // must keep pointing at that package. - // • runtime writes into a package that is NOT loaded as code — an - // ADR-0048 #1824 package *authoring workspace* is a bare id with - // no registered manifest, and per-package scoping must survive. - // `isLoadedPackage` distinguishes the two: only a booted code - // package has a manifest / registered artifacts. - // Mutate `request.packageId` (not a local copy) so every downstream - // consumer — the repo write, the parent-version lookup, the live - // registry mutation on publish, and the audit record — sees the - // coerced value consistently. Coercing only the repo write left the - // in-memory object stamped with the code package, so it still read - // back as code-provided / read-only. + // must keep pointing at the package it customizes (ADR-0005). + // • a project-scoped base, or a bare ADR-0048 authoring-workspace + // id — both are writable; `isWritablePackage` returns true. + // A `null` packageId is still accepted here (legacy org-overlay + // destination); ADR-0070 D5 retires it once the surfaces always + // resolve a base and the orphan migration has run. if ( intent === 'runtime-only' && request.packageId != null && - this.isLoadedPackage(request.packageId) + !this.isWritablePackage(request.packageId) ) { - console.warn( - `[Protocol] dropping package binding '${request.packageId}' from runtime-authored ${singularTypeForRepo}/${request.name} (it belongs to a loaded code package); persisting as an org overlay (package_id = null) so it stays editable.`, + const err = new Error( + `[writable_package_required] Cannot author ${singularTypeForRepo}/${request.name} into ` + + `'${request.packageId}': it is a read-only code/installed package, not a writable base. ` + + `Create or select a writable base (package) first, then retry. ` + + `See docs/adr/0070-package-first-authoring.md.`, ); - request.packageId = null; + (err as any).code = 'writable_package_required'; + (err as any).status = 422; + (err as any).packageId = request.packageId; + throw err; } const orgId = request.organizationId ?? null; const repo = this.getOverlayRepo(orgId);