Skip to content

Commit 427a73e

Browse files
os-zhuangclaude
andcommitted
feat(objectql): reject runtime creates into read-only packages — writable_package_required (ADR-0070 D1/D2)
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 <noreply@anthropic.com>
1 parent 195c7e8 commit 427a73e

3 files changed

Lines changed: 112 additions & 78 deletions

File tree

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
feat(objectql): enforce package-first authoring at the kernel (ADR-0070 D1/D2)
6+
7+
A runtime-only metadata **create** that targets a read-only code/installed
8+
package now throws `writable_package_required` (status 422) instead of silently
9+
coercing `package_id` to `null`. The old coercion (#2252 stopgap) unblocked
10+
editing but scattered orphans into a package-less bucket with no container to
11+
delete (#1946); the rejection instead directs the authoring surface (Studio /
12+
AI) to pick or create a writable base first.
13+
14+
`isLoadedPackage` is generalized into `isWritablePackage` (D2): a package is
15+
writable unless it is a booted code package (registered in the engine manifest
16+
map) or a `system`/`cloud`-scoped installed package. The old "owns ≥1 registered
17+
object" heuristic is dropped — it was the read-only-after-publish trap (#2252),
18+
since a writable base accrues registered objects once its drafts publish.
19+
20+
`null` is still accepted as the legacy org-overlay destination; ADR-0070 D5
21+
retires it after the orphan migration.

packages/objectql/src/protocol-save-meta-repo-path.test.ts

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -283,30 +283,49 @@ describe('saveMetaItem — repository write path (post PR-10d.6)', () => {
283283
return captured;
284284
}
285285

286-
it('runtime-only create while a LOADED code package is selected drops the binding (package_id = null)', async () => {
287-
// The bug: a brand-new object authored while a loaded code package was
288-
// selected in Studio's dropdown got `package_id = <that package>`. The
289-
// object is NOT artifact-backed (intent = 'runtime-only') and the
290-
// package IS loaded (its manifest is registered), so the binding must
291-
// be dropped — otherwise it reads back as code-provided / read-only.
286+
it('runtime-only create into a LOADED code package is rejected (writable_package_required)', async () => {
287+
// ADR-0070 D1: a brand-new object authored while a loaded code package
288+
// is selected must NOT be silently coerced to a package-less orphan
289+
// (the old #2252 behaviour) — it is rejected so the authoring surface
290+
// redirects the user to pick or create a writable base first.
292291
const { engine } = makeStubEngine();
293292
engine.manifests = new Map([['app.objectstack.hotcrm', { id: 'app.objectstack.hotcrm' }]]);
294293
const inserts = spyInserts(engine);
295294
const protocol = new ObjectStackProtocolImplementation(engine);
296-
const result = await protocol.saveMetaItem({
297-
type: 'object',
298-
name: 'maint_asset',
299-
organizationId: 'org_alpha',
300-
packageId: 'app.objectstack.hotcrm', // Studio had a code package selected
301-
mode: 'draft',
302-
item: { name: 'maint_asset', label: 'Asset', fields: { name: { type: 'text', label: 'Name' } } },
303-
});
304-
expect(result.success).toBe(true);
305-
const create = inserts.find((d) => d.type === 'object' && d.name === 'maint_asset');
306-
expect(create).toBeTruthy();
307-
// brand-new org object must NOT carry the package binding → stays editable.
308-
expect(create!.package_id ?? null).toBeNull();
309-
expect(create!.package_id).not.toBe('app.objectstack.hotcrm');
295+
await expect(
296+
protocol.saveMetaItem({
297+
type: 'object',
298+
name: 'maint_asset',
299+
organizationId: 'org_alpha',
300+
packageId: 'app.objectstack.hotcrm', // Studio had a code package selected
301+
mode: 'draft',
302+
item: { name: 'maint_asset', label: 'Asset', fields: { name: { type: 'text', label: 'Name' } } },
303+
}),
304+
).rejects.toMatchObject({ code: 'writable_package_required', status: 422 });
305+
// The orphan is never created — nothing was persisted.
306+
expect(inserts.find((d) => d.type === 'object' && d.name === 'maint_asset')).toBeUndefined();
307+
});
308+
309+
it('runtime-only create into a system-scoped installed package is rejected', async () => {
310+
// ADR-0070 D2: read-only is also detected via manifest scope — an
311+
// installed/platform package (scope system|cloud) that is NOT in the
312+
// engine manifest map is still not a writable base.
313+
const { engine } = makeStubEngine();
314+
engine.registry.getPackage = (id: string) =>
315+
id === 'platform.core' ? { manifest: { id, scope: 'system' } } : undefined;
316+
const inserts = spyInserts(engine);
317+
const protocol = new ObjectStackProtocolImplementation(engine);
318+
await expect(
319+
protocol.saveMetaItem({
320+
type: 'object',
321+
name: 'maint_asset',
322+
organizationId: 'org_alpha',
323+
packageId: 'platform.core',
324+
mode: 'draft',
325+
item: { name: 'maint_asset', label: 'Asset', fields: { name: { type: 'text', label: 'Name' } } },
326+
}),
327+
).rejects.toMatchObject({ code: 'writable_package_required', status: 422 });
328+
expect(inserts.find((d) => d.type === 'object' && d.name === 'maint_asset')).toBeUndefined();
310329
});
311330

312331
it('runtime-only create into a NON-loaded package keeps its binding (ADR-0048 #1824 authoring scope)', async () => {

packages/objectql/src/protocol.ts

Lines changed: 52 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -3162,41 +3162,36 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
31623162
}
31633163

31643164
/**
3165-
* True when `packageId` refers to a **loaded code package** — one that
3166-
* booted and registered a manifest (and typically objects/apps) into the
3167-
* engine. Such packages are read-only artifacts; runtime-authored items
3168-
* must not be bound to them (see {@link saveMetaItem}).
3165+
* True when `packageId` is a **writable base** — a DB-backed package an
3166+
* org or the AI may author *new* metadata into (ADR-0070 D2). The two
3167+
* read-only kinds return `false`:
31693168
*
3170-
* A bare ADR-0048 *authoring-workspace* package id (no booted manifest,
3171-
* no registered metadata) returns `false`, so per-package authoring scope
3172-
* is preserved. Reads the engine's manifest map first (authoritative,
3173-
* O(1) — every `registerApp` call records the manifest there), then falls
3174-
* back to "owns ≥1 registered object" for defense in depth.
3169+
* • **Booted code packages** — they register a manifest into the engine
3170+
* at startup (`registerApp` → `engine.manifests`); their items are
3171+
* code-shipped artifacts. Only `allowOrgOverride` overlays are allowed
3172+
* (ADR-0005), never fresh authored items.
3173+
* • **Installed / platform packages** — manifest `scope` is `system` or
3174+
* `cloud` (marketplace / platform-delivered).
3175+
*
3176+
* A project-scoped DB package, or a bare ADR-0048 *authoring-workspace* id
3177+
* with no registered manifest, is writable.
3178+
*
3179+
* NOTE: the code-package signal is the engine manifest map ONLY — we
3180+
* deliberately do NOT fall back to "owns ≥1 registered object" (the old
3181+
* `isLoadedPackage` heuristic). A writable base accrues registered objects
3182+
* once its drafts publish, and that must never flip the base to read-only
3183+
* — that is the exact #2252 read-only-after-publish trap this ADR removes.
31753184
*/
3176-
private isLoadedPackage(packageId: string): boolean {
3185+
private isWritablePackage(packageId: string | null | undefined): boolean {
3186+
if (!packageId) return false;
31773187
const engine = this.engine as any;
3178-
if (engine?.manifests?.has?.(packageId)) return true;
3179-
const registry = engine?.registry;
3180-
if (!registry) return false;
3181-
try {
3182-
// Objects contributed by the package (real data packages).
3183-
if (typeof registry.getAllObjects === 'function'
3184-
&& registry.getAllObjects(packageId).length > 0) {
3185-
return true;
3186-
}
3187-
// UI / logic metadata bound to the package id. ADR-0048 — a code
3188-
// package registers its app via `registerApp(app, packageId)`, so
3189-
// the app item's `_packageId` is the package id; UI-only packages
3190-
// (no objects) are still detected here.
3191-
if (typeof registry.listItems === 'function') {
3192-
for (const t of ['app', 'view', 'page', 'flow', 'report', 'dashboard', 'agent', 'skill', 'role', 'permission']) {
3193-
if (registry.listItems(t, packageId).length > 0) return true;
3194-
}
3195-
}
3196-
} catch {
3197-
// A partial registry (test mocks) → treat as "not loaded".
3198-
}
3199-
return false;
3188+
// Booted code package → read-only artifact source.
3189+
if (engine?.manifests?.has?.(packageId)) return false;
3190+
// Installed / platform package → read-only by manifest scope.
3191+
const scope = engine?.registry?.getPackage?.(packageId)?.manifest?.scope;
3192+
if (scope === 'system' || scope === 'cloud') return false;
3193+
// Project-scoped base, or unregistered authoring-workspace id → writable.
3194+
return true;
32003195
}
32013196

32023197
/**
@@ -3724,39 +3719,38 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
37243719
const intent: 'override-artifact' | 'runtime-only' = artifactBacked
37253720
? 'override-artifact'
37263721
: 'runtime-only';
3727-
// GUARD — a brand-new, DB-only ("runtime-only") metadata item must
3728-
// not be bound to a *loaded code package*. Studio sends the
3729-
// currently-selected package via `?package=`; when a user authors a
3730-
// new object while browsing a code package (e.g. `app.objectstack.hotcrm`),
3731-
// persisting that id as the new row's `package_id` makes the org
3732-
// object read back as "provided by a code package" and become
3733-
// read-only after publish — the user can no longer edit what they
3734-
// just created. Drop the binding so it is a plain org overlay
3735-
// (`package_id = null`, editable).
3722+
// D1 (ADR-0070) — a brand-new, DB-only ("runtime-only") metadata
3723+
// item MUST resolve to a WRITABLE base. Binding it to a read-only
3724+
// code/installed package makes it read back as "code-provided" and
3725+
// lock read-only after publish (the #2252 bug). We used to silently
3726+
// coerce such a binding to `null`, but that scattered orphans into a
3727+
// package-less bucket with no container to delete; ADR-0070 replaces
3728+
// the coercion with an actionable rejection so the authoring surface
3729+
// (Studio / AI) redirects the user to pick or create a base first.
37363730
//
3737-
// Two scopes are deliberately left bound:
3731+
// Left untouched (the binding survives):
37383732
// • `override-artifact` writes — an org overlay OF a packaged item
3739-
// must keep pointing at that package.
3740-
// • runtime writes into a package that is NOT loaded as code — an
3741-
// ADR-0048 #1824 package *authoring workspace* is a bare id with
3742-
// no registered manifest, and per-package scoping must survive.
3743-
// `isLoadedPackage` distinguishes the two: only a booted code
3744-
// package has a manifest / registered artifacts.
3745-
// Mutate `request.packageId` (not a local copy) so every downstream
3746-
// consumer — the repo write, the parent-version lookup, the live
3747-
// registry mutation on publish, and the audit record — sees the
3748-
// coerced value consistently. Coercing only the repo write left the
3749-
// in-memory object stamped with the code package, so it still read
3750-
// back as code-provided / read-only.
3733+
// must keep pointing at the package it customizes (ADR-0005).
3734+
// • a project-scoped base, or a bare ADR-0048 authoring-workspace
3735+
// id — both are writable; `isWritablePackage` returns true.
3736+
// A `null` packageId is still accepted here (legacy org-overlay
3737+
// destination); ADR-0070 D5 retires it once the surfaces always
3738+
// resolve a base and the orphan migration has run.
37513739
if (
37523740
intent === 'runtime-only' &&
37533741
request.packageId != null &&
3754-
this.isLoadedPackage(request.packageId)
3742+
!this.isWritablePackage(request.packageId)
37553743
) {
3756-
console.warn(
3757-
`[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.`,
3744+
const err = new Error(
3745+
`[writable_package_required] Cannot author ${singularTypeForRepo}/${request.name} into `
3746+
+ `'${request.packageId}': it is a read-only code/installed package, not a writable base. `
3747+
+ `Create or select a writable base (package) first, then retry. `
3748+
+ `See docs/adr/0070-package-first-authoring.md.`,
37583749
);
3759-
request.packageId = null;
3750+
(err as any).code = 'writable_package_required';
3751+
(err as any).status = 422;
3752+
(err as any).packageId = request.packageId;
3753+
throw err;
37603754
}
37613755
const orgId = request.organizationId ?? null;
37623756
const repo = this.getOverlayRepo(orgId);

0 commit comments

Comments
 (0)