Skip to content

Commit 16e8dad

Browse files
os-zhuangclaude
andauthored
fix(objectql): don't stamp runtime-authored metadata into a loaded code package (#2252)
Studio sends the currently-selected package via `?package=` as a *viewing* context. When a user authored a brand-new object while browsing a code package (e.g. `com.example.showcase`), `saveMetaItem` persisted that id as the new row's `package_id`. The object then read back as "provided by a code package" and became read-only immediately after publish — the user could no longer edit what they had just created (the `sys_metadata` row was stamped `package_id = <code package>`). `saveMetaItem` now drops the package binding for a runtime-only (DB-only, non-artifact-backed) write whose target is a *loaded* code package, persisting it as a plain org overlay (`package_id = null`, editable). Left untouched: - `override-artifact` writes — an org overlay OF a packaged item keeps its binding; and - writes into a non-loaded "authoring workspace" package id, so ADR-0048 #1824 per-package scoping survives. `isLoadedPackage` distinguishes the two via the engine manifest map (every `registerApp` records it) plus a registered-objects/apps fallback. The binding is dropped by mutating `request.packageId` so every downstream consumer (repo write, parent-version lookup, the live registry mutation on publish, and the audit record) sees the coerced value — coercing only the repo write left the in-memory object stamped and still read-only. Found by dogfooding Studio as a business user: a freshly-created custom object locked itself read-only after publish. Verified end-to-end against a live showcase server (object now org-owned + editable) and with 3 new unit tests; full objectql suite (695) green. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 1b56e36 commit 16e8dad

2 files changed

Lines changed: 165 additions & 0 deletions

File tree

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

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,4 +262,97 @@ describe('saveMetaItem — repository write path (post PR-10d.6)', () => {
262262
const afterBody = (Array.from(rows.values())[0] as any).metadata;
263263
expect(afterBody).toBe(beforeBody);
264264
});
265+
266+
// ── runtime-only writes must not be stamped into a code package ──────
267+
// Regression: a custom object authored in Studio while a CODE package was
268+
// selected in the dropdown (`?package=`) was persisted with
269+
// `package_id = <code package>`, then read back as "code-provided" and
270+
// locked read-only after publish. saveMetaItem now coerces such a
271+
// runtime-only write to an org-owned overlay (`package_id = null`).
272+
//
273+
// We spy on the `sys_metadata` insert rather than reading back the stub
274+
// row map (whose key ignores the table, so lineage/history inserts can
275+
// collide with — and clobber — the parent row's `package_id`).
276+
function spyInserts(engine: any): Array<Record<string, unknown>> {
277+
const captured: Array<Record<string, unknown>> = [];
278+
const orig = engine.insert.bind(engine);
279+
engine.insert = async (t: string, data: Record<string, unknown>, opts?: unknown) => {
280+
if (t === 'sys_metadata') captured.push(data);
281+
return orig(t, data, opts);
282+
};
283+
return captured;
284+
}
285+
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.
292+
const { engine } = makeStubEngine();
293+
engine.manifests = new Map([['app.objectstack.hotcrm', { id: 'app.objectstack.hotcrm' }]]);
294+
const inserts = spyInserts(engine);
295+
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');
310+
});
311+
312+
it('runtime-only create into a NON-loaded package keeps its binding (ADR-0048 #1824 authoring scope)', async () => {
313+
// A package *authoring workspace* is a bare id with no booted manifest
314+
// and no registered metadata. Per-package authoring scope must survive,
315+
// so the guard must leave such a binding intact.
316+
const { engine } = makeStubEngine();
317+
// No manifests map / empty → isLoadedPackage('com.acme.beta') is false.
318+
const inserts = spyInserts(engine);
319+
const protocol = new ObjectStackProtocolImplementation(engine);
320+
await protocol.saveMetaItem({
321+
type: 'object',
322+
name: 'maint_ticket',
323+
organizationId: 'org_alpha',
324+
packageId: 'com.acme.beta',
325+
mode: 'draft',
326+
item: { name: 'maint_ticket', label: 'Ticket', fields: { name: { type: 'text', label: 'Name' } } },
327+
});
328+
const create = inserts.find((d) => d.type === 'object' && d.name === 'maint_ticket');
329+
expect(create).toBeTruthy();
330+
expect(create!.package_id).toBe('com.acme.beta');
331+
});
332+
333+
it('override-artifact write keeps its package binding (guard only touches runtime-only creates)', async () => {
334+
// An org overlay OF a packaged item (intent = 'override-artifact') must
335+
// stay bound to that package even though the package is loaded. Make the
336+
// packaged view artifact-backed so the write is an override, not a fresh
337+
// create. (Objects are not override-allowed, so the control uses a view.)
338+
const { engine } = makeStubEngine();
339+
engine.manifests = new Map([['app.objectstack.hotcrm', { id: 'app.objectstack.hotcrm' }]]);
340+
engine.registry.getArtifactItem = (type: string, name: string) =>
341+
type === 'view' && name === 'case_grid'
342+
? { _packageId: 'app.objectstack.hotcrm', name, type: 'grid' }
343+
: undefined;
344+
const inserts = spyInserts(engine);
345+
const protocol = new ObjectStackProtocolImplementation(engine);
346+
await protocol.saveMetaItem({
347+
type: 'view',
348+
name: 'case_grid',
349+
organizationId: 'org_alpha',
350+
packageId: 'app.objectstack.hotcrm',
351+
mode: 'draft',
352+
item: { name: 'case_grid', type: 'grid', label: 'Cases (org overlay)', columns: ['id', 'title'] },
353+
});
354+
const create = inserts.find((d) => d.type === 'view' && d.name === 'case_grid');
355+
expect(create).toBeTruthy();
356+
expect(create!.package_id).toBe('app.objectstack.hotcrm');
357+
});
265358
});

packages/objectql/src/protocol.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3147,6 +3147,44 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
31473147
return item;
31483148
}
31493149

3150+
/**
3151+
* True when `packageId` refers to a **loaded code package** — one that
3152+
* booted and registered a manifest (and typically objects/apps) into the
3153+
* engine. Such packages are read-only artifacts; runtime-authored items
3154+
* must not be bound to them (see {@link saveMetaItem}).
3155+
*
3156+
* A bare ADR-0048 *authoring-workspace* package id (no booted manifest,
3157+
* no registered metadata) returns `false`, so per-package authoring scope
3158+
* is preserved. Reads the engine's manifest map first (authoritative,
3159+
* O(1) — every `registerApp` call records the manifest there), then falls
3160+
* back to "owns ≥1 registered object" for defense in depth.
3161+
*/
3162+
private isLoadedPackage(packageId: string): boolean {
3163+
const engine = this.engine as any;
3164+
if (engine?.manifests?.has?.(packageId)) return true;
3165+
const registry = engine?.registry;
3166+
if (!registry) return false;
3167+
try {
3168+
// Objects contributed by the package (real data packages).
3169+
if (typeof registry.getAllObjects === 'function'
3170+
&& registry.getAllObjects(packageId).length > 0) {
3171+
return true;
3172+
}
3173+
// UI / logic metadata bound to the package id. ADR-0048 — a code
3174+
// package registers its app via `registerApp(app, packageId)`, so
3175+
// the app item's `_packageId` is the package id; UI-only packages
3176+
// (no objects) are still detected here.
3177+
if (typeof registry.listItems === 'function') {
3178+
for (const t of ['app', 'view', 'page', 'flow', 'report', 'dashboard', 'agent', 'skill', 'role', 'permission']) {
3179+
if (registry.listItems(t, packageId).length > 0) return true;
3180+
}
3181+
}
3182+
} catch {
3183+
// A partial registry (test mocks) → treat as "not loaded".
3184+
}
3185+
return false;
3186+
}
3187+
31503188
/**
31513189
* Resolve the effective `_lock` for an item by consulting the
31523190
* artifact registry first, then the persisted overlay row. Artifact
@@ -3672,6 +3710,40 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol {
36723710
const intent: 'override-artifact' | 'runtime-only' = artifactBacked
36733711
? 'override-artifact'
36743712
: 'runtime-only';
3713+
// GUARD — a brand-new, DB-only ("runtime-only") metadata item must
3714+
// not be bound to a *loaded code package*. Studio sends the
3715+
// currently-selected package via `?package=`; when a user authors a
3716+
// new object while browsing a code package (e.g. `app.objectstack.hotcrm`),
3717+
// persisting that id as the new row's `package_id` makes the org
3718+
// object read back as "provided by a code package" and become
3719+
// read-only after publish — the user can no longer edit what they
3720+
// just created. Drop the binding so it is a plain org overlay
3721+
// (`package_id = null`, editable).
3722+
//
3723+
// Two scopes are deliberately left bound:
3724+
// • `override-artifact` writes — an org overlay OF a packaged item
3725+
// must keep pointing at that package.
3726+
// • runtime writes into a package that is NOT loaded as code — an
3727+
// ADR-0048 #1824 package *authoring workspace* is a bare id with
3728+
// no registered manifest, and per-package scoping must survive.
3729+
// `isLoadedPackage` distinguishes the two: only a booted code
3730+
// package has a manifest / registered artifacts.
3731+
// Mutate `request.packageId` (not a local copy) so every downstream
3732+
// consumer — the repo write, the parent-version lookup, the live
3733+
// registry mutation on publish, and the audit record — sees the
3734+
// coerced value consistently. Coercing only the repo write left the
3735+
// in-memory object stamped with the code package, so it still read
3736+
// back as code-provided / read-only.
3737+
if (
3738+
intent === 'runtime-only' &&
3739+
request.packageId != null &&
3740+
this.isLoadedPackage(request.packageId)
3741+
) {
3742+
console.warn(
3743+
`[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+
);
3745+
request.packageId = null;
3746+
}
36753747
const orgId = request.organizationId ?? null;
36763748
const repo = this.getOverlayRepo(orgId);
36773749
const ref = {

0 commit comments

Comments
 (0)