From eb42ce01e14620ee880a1aa772631212b9984db8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 04:06:45 +0000 Subject: [PATCH 1/7] test(rest): pin one head row per /meta write address for a package-less draft and ?package=all (#22128) Red at the base: the second unpinned package-less draft save of a package-owned item answers 409 METADATA_CONFLICT, and ?state=draft serves version null while the draft exists. Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848 Co-authored-by: Claude --- ...eta-draft-head-package-inheritance.test.ts | 269 ++++++++++++++++++ 1 file changed, 269 insertions(+) create mode 100644 packages/rest/src/meta-draft-head-package-inheritance.test.ts diff --git a/packages/rest/src/meta-draft-head-package-inheritance.test.ts b/packages/rest/src/meta-draft-head-package-inheritance.test.ts new file mode 100644 index 00000000000..e1feea20d67 --- /dev/null +++ b/packages/rest/src/meta-draft-head-package-inheritance.test.ts @@ -0,0 +1,269 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#22128] One head row per `/meta` write address: the save's expected-parent + * read, the repository's optimistic lock and the item read's served `version` + * resolve the same row. + * + * ADR-0008 makes the lock on `PUT /meta` opt-in: a save with no `If-Match` is + * last-writer-wins. A package-less draft save of a package-owned item is stored + * in the package (#11087: the repository inherits the active row's binding, so + * the draft is not orphaned). The save door's head read used to look for the + * draft at the package-UNBOUND row instead, found none, and handed the + * repository a parent of `null`, which the repository's lock judged against + * the inherited row. So the second unpinned draft save was refused 409 + * `METADATA_CONFLICT`, and the `?state=draft` read served `version: null` + * while the draft existed. + * + * `?package=all` is the second member of that class. The save door folds it + * to the env-local overlay; the read door forwarded the literal `all`, so its + * `version` was resolved at a package address no save writes. + * + * Driven at the HTTP door on the real stack: a better-sqlite3 `:memory:` + * engine, the real `sys_metadata*` objects, a real + * `ObjectStackProtocolImplementation` and the real routes. The stub is the + * auth boundary (`resolveExecCtx`). + * + * ⚠️ `@objectstack/metadata-protocol` resolves through `exports` to its + * **`dist/`** (`check-test-source-alias.mjs`, `KNOWN_UNALIASED_TEST_IMPORTS`): + * rebuild it before reading a result here after touching `protocol.ts` or + * `sys-metadata-repository.ts`. + * + * Every refusal pin asserts the ADR-0112 `code` and the HTTP status, and reads + * the store to show the refused write did not land. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { SysMetadata, SysMetadataHistoryObject, SysMetadataAuditObject } from '@objectstack/platform-objects/metadata'; +import { RestServer } from './rest-server.js'; + +/** `registry.registerObject` requires a package id (see the sibling real-stack tests). */ +const TEST_PACKAGE_ID = 'objectstack-test'; +const META = '/api/v1/meta'; +/** The keyed token shape the doors serve with no crypto provider registered (the process key). */ +const KEYED = /^hmac-sha256:[0-9a-f]{64}$/; +/** A writable authoring package: no installed package carries this id. */ +const PKG = 'com.probe.pkg'; + +const TASK = { + name: 'task', + label: 'Task', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true, label: 'ID' }, + name: { name: 'name', type: 'text' as const, label: 'Name' }, + }, +}; + +const view = (label: string) => ({ + name: 'case_grid', + label, + object: 'task', + viewKind: 'list', + columns: [{ field: 'name', label: 'Name' }], +}); + +const liveEngines: ObjectQL[] = []; +afterEach(async () => { + while (liveEngines.length) { + try { await liveEngines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { statusCode: 200, body: undefined, headers: {} as Record }; + res.status = (code: number) => { res.statusCode = code; return res; }; + res.json = (body: any) => { res.body = body; return res; }; + res.send = () => res; + res.end = () => res; + res.header = (k: string, v: string) => { res.headers[k] = v; return res; }; + res.setHeader = () => {}; + res.write = () => true; + return res; +} + +/** + * Boot the real stack with the cache off: the default cached arm of the plain + * read publishes no OCC carriers, so the active-row pins read the uncached arm. + */ +async function boot() { + const engine = new ObjectQL(); + liveEngines.push(engine); + engine.registerDriver(new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }), true); + await engine.init(); + engine.registry.registerObject(TASK as any, TEST_PACKAGE_ID); + engine.registry.registerObject(SysMetadata as any, TEST_PACKAGE_ID); + engine.registry.registerObject(SysMetadataHistoryObject as any, TEST_PACKAGE_ID); + engine.registry.registerObject(SysMetadataAuditObject as any, TEST_PACKAGE_ID); + await engine.syncSchemas(); + + const protocol = new ObjectStackProtocolImplementation(engine as any); + const config = { api: { requireAuth: false }, metadata: { enableCache: false } }; + const rest: any = new RestServer(createMockServer() as any, protocol as any, config as any); + // An author: `manage_metadata` saves and reads drafts. + rest.resolveExecCtx = async () => ({ userId: 'u_author', systemPermissions: ['manage_metadata'] }); + rest.registerRoutes(); + + const route = (method: string, path: string) => { + const found = rest.getRoutes().find((r: any) => r.method === method && r.path === path); + if (!found) throw new Error(`${method} ${path} is not registered`); + return found; + }; + const call = async (method: string, routePath: string, req: Record) => { + const res = makeRes(); + await route(method, routePath).handler({ method, headers: {}, query: {}, params: {}, ...req }, res); + return res; + }; + const item = { type: 'view', name: 'case_grid' }; + const path = `${META}/view/case_grid`; + + /** `GET /meta/view/case_grid`. */ + const read = (query: Record = {}) => + call('GET', `${META}/:type/:name`, { path, params: item, query }); + /** `PUT /meta/view/case_grid`. */ + const save = (body: unknown, query: Record = {}, headers: Record = {}) => + call('PUT', `${META}/:type/:name`, { path, params: item, query, headers, body }); + + /** The stored rows of the item at one lifecycle, with their binding — the store, not the door's word for it. */ + const stored = async (state: 'active' | 'draft') => { + const rows = await engine.find('sys_metadata', { where: { type: 'view', name: 'case_grid', state } }); + return (rows ?? []).map((r: any) => ({ + label: JSON.parse(String(r.metadata)).label as string, + packageId: (r.package_id ?? null) as string | null, + })); + }; + + return { read, save, stored }; +} + +const ok = (res: any) => { + expect(res.statusCode, JSON.stringify(res.body)).toBe(200); + return res.body; +}; + +/** A refusal: the ADR-0112 `code` and the status, both asserted. */ +const refusedConflict = (res: any) => { + expect(res.statusCode, JSON.stringify(res.body)).toBe(409); + expect(res.body.code).toBe('METADATA_CONFLICT'); + return res.body; +}; + +describe('[#22128] a package-less draft of a package-owned item: one head row for the save, the lock and the read', () => { + it('the measured sequence: active save in a package, a package-less draft, the same draft again with no `If-Match` → 200, 200, 200', async () => { + const h = await boot(); + ok(await h.save(view('live'), { package: PKG })); + ok(await h.save(view('draft 1'), { mode: 'draft' })); + // #11087's inheritance, kept: the package-less draft is stored in the package. + expect(await h.stored('draft')).toEqual([{ label: 'draft 1', packageId: PKG }]); + + // The defect: this unpinned save was refused 409 METADATA_CONFLICT + // ("Expected parent null but current is hmac-sha256:…"). + ok(await h.save(view('draft 2'), { mode: 'draft' })); + // Last-writer-wins on the ONE draft row — updated in place, never forked. + expect(await h.stored('draft')).toEqual([{ label: 'draft 2', packageId: PKG }]); + expect(await h.stored('active')).toEqual([{ label: 'live', packageId: PKG }]); + }, 60_000); + + it('`?state=draft` after the package-less draft serves that draft\'s `version` (the save receipt\'s), not `null`', async () => { + const h = await boot(); + ok(await h.save(view('live'), { package: PKG })); + const receipt = ok(await h.save(view('draft 1'), { mode: 'draft' })); + expect(receipt.version).toMatch(KEYED); + + const body = ok(await h.read({ state: 'draft' })); + expect(body.version).toBe(receipt.version); + + // And that token pins the next save at the same address. + const next = ok(await h.save(view('draft 2'), { mode: 'draft' }, { 'if-match': body.version })); + expect(ok(await h.read({ state: 'draft' })).version).toBe(next.version); + expect(await h.stored('draft')).toEqual([{ label: 'draft 2', packageId: PKG }]); + }, 60_000); + + it('control: a stale `If-Match` is still refused 409, with the inherited draft\'s token as `currentVersion`', async () => { + const h = await boot(); + ok(await h.save(view('live'), { package: PKG })); + const first = ok(await h.save(view('draft 1'), { mode: 'draft' })); + const second = ok(await h.save(view('draft 2'), { mode: 'draft' }, { 'if-match': first.version })); + + const stale = refusedConflict(await h.save(view('stale edit'), { mode: 'draft' }, { 'if-match': first.version })); + expect(stale.currentVersion).toBe(second.version); + expect(ok(await h.read({ state: 'draft' })).version).toBe(stale.currentVersion); + // The refused write did not land. + expect(await h.stored('draft')).toEqual([{ label: 'draft 2', packageId: PKG }]); + + // `If-None-Match: *` asserts no draft: refused while the inherited one exists. + refusedConflict(await h.save(view('as if first'), { mode: 'draft' }, { 'if-none-match': '*' })); + expect(await h.stored('draft')).toEqual([{ label: 'draft 2', packageId: PKG }]); + }, 60_000); + + it('control: an env-local item with no package is unchanged — unbound rows, unpinned saves accepted, the read serves the receipt\'s token', async () => { + const h = await boot(); + ok(await h.save(view('live'))); + const first = ok(await h.save(view('draft 1'), { mode: 'draft' })); + ok(await h.save(view('draft 2'), { mode: 'draft' })); + expect(await h.stored('draft')).toEqual([{ label: 'draft 2', packageId: null }]); + expect(await h.stored('active')).toEqual([{ label: 'live', packageId: null }]); + + const held = ok(await h.read({ state: 'draft' })).version; + expect(held).toMatch(KEYED); + expect(held).not.toBe(first.version); + refusedConflict(await h.save(view('stale'), { mode: 'draft' }, { 'if-match': first.version })); + ok(await h.save(view('draft 3'), { mode: 'draft' }, { 'if-match': held })); + expect(await h.stored('draft')).toEqual([{ label: 'draft 3', packageId: null }]); + }, 60_000); +}); + +describe('[#22128] `?package=all`: the read resolves `all` the way the save does', () => { + it('active: the read\'s `version` is the token the save compares against; `If-None-Match: *` is refused only because a row is there', async () => { + const h = await boot(); + const created = ok(await h.save(view('live'), { package: 'all' })); + // The save folds `all` to the env-local overlay. + expect(await h.stored('active')).toEqual([{ label: 'live', packageId: null }]); + + const token = ok(await h.read({ package: 'all' })).version; + expect(token).toBe(created.version); + + const refused = refusedConflict(await h.save(view('as if first'), { package: 'all' }, { 'if-none-match': '*' })); + expect(refused.currentVersion).toBe(token); + ok(await h.save(view('edited'), { package: 'all' }, { 'if-match': token })); + expect(await h.stored('active')).toEqual([{ label: 'edited', packageId: null }]); + }, 60_000); + + it('active, no row at the env-local address: the read serves `null`, and `If-None-Match: *` creates it', async () => { + const h = await boot(); + ok(await h.save(view('live'), { package: PKG })); + + expect(ok(await h.read({ package: 'all' })).version).toBeNull(); + const created = ok(await h.save(view('env-local'), { package: 'all' }, { 'if-none-match': '*' })); + expect(ok(await h.read({ package: 'all' })).version).toBe(created.version); + expect(await h.stored('active')).toEqual(expect.arrayContaining([ + { label: 'live', packageId: PKG }, + { label: 'env-local', packageId: null }, + ])); + }, 60_000); + + it('draft of a package-owned item: `?state=draft&package=all` serves the inherited draft and its token, and the save accepts it', async () => { + const h = await boot(); + ok(await h.save(view('live'), { package: PKG })); + const draft = ok(await h.save(view('draft 1'), { mode: 'draft', package: 'all' })); + expect(await h.stored('draft')).toEqual([{ label: 'draft 1', packageId: PKG }]); + + const body = ok(await h.read({ state: 'draft', package: 'all' })); + expect(body.item.label).toBe('draft 1'); + expect(body.version).toBe(draft.version); + + ok(await h.save(view('draft 2'), { mode: 'draft', package: 'all' }, { 'if-match': body.version })); + expect(await h.stored('draft')).toEqual([{ label: 'draft 2', packageId: PKG }]); + }, 60_000); +}); From ca16932c7f1e59f18fb3fac5683f81aa97a40fa6 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 04:09:05 +0000 Subject: [PATCH 2/7] fix(metadata-protocol): one head resolution per /meta write address for the save's parent, the repository's lock and the read's version (#22128) SysMetadataRepository.headAt answers the row a put at the same address upserts, through the same resolution put's lock runs (resolveWriteHead), the #11087 draft package inheritance included. storedHeadAt asks it, so the save door's expected parent and the item read's served version are the row the lock judges. The /meta item read folds ?package=all through the save door's reading (metaItemPackageBinding, shared by GET, PUT and publish). SaveMetaItemRequestSchema.packageId states the inheritance; the publish route's description spells the wire code METADATA_CONFLICT. Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848 Co-authored-by: Claude --- packages/metadata-protocol/src/protocol.ts | 41 ++-- .../src/sys-metadata-repository.ts | 188 +++++++++++------- packages/rest/src/rest-server.ts | 40 +++- packages/spec/src/api/plugin-rest-api.zod.ts | 2 +- packages/spec/src/api/protocol.zod.ts | 20 +- 5 files changed, 190 insertions(+), 101 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index fe4ae2aa24a..ece667b9505 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -18039,24 +18039,33 @@ export class ObjectStackProtocolImplementation implements /** * [#22114] The stored head a `/meta` save at one address compares a * caller's version token against: the `state` row at `ref`'s organization - * partition, bound to `packageId` (`null` = the package-unbound row), as - * the repository reads and serves it ({@link SysMetadataRepository.get}). - * `null` when no such row exists. + * partition that a save naming `packageId` (`null` = no package) upserts, + * as the repository serves it. `null` when that save would create. * * The ONE head read both halves of the ADR-0008 chain make. The save door - * judges an inbound token against it ({@link storedParentForToken}), and - * the item read serves its keyed form as `version` - * ({@link readVersionToken}). So the token a read hands out is the token a - * save at the same address accepts, and the one a receipt serves for that - * row — ⛔ never a second derivation of either. + * takes its expected parent from it — the inbound token judged against it + * ({@link storedParentForToken}), or, unpinned, the head itself — and the + * item read serves its keyed form as `version` ({@link readVersionToken}). + * So the token a read hands out is the token a save at the same address + * accepts, and the one a receipt serves for that row — ⛔ never a second + * derivation of either. + * + * [#22128] Resolved by the repository's own write-address resolution + * ({@link SysMetadataRepository.headAt}), the one its lock judges with — + * ⛔ never {@link SysMetadataRepository.get} at the named key. A draft + * save that names no package is stored in the package of the item's + * active row (#11087), so the row at the package-unbound key is not the + * row that save overwrites: read there, the parent of an existing draft + * was `null`, the repository's lock refused the unpinned save 409, and the + * `?state=draft` read served `version: null` while the draft existed. */ private async storedHeadAt( repo: SysMetadataRepository, - ref: Parameters[0], + ref: Parameters[0], state: 'active' | 'draft', packageId: string | null, ): Promise { - return (await repo.get(ref, { state, packageId }))?.hash ?? null; + return (await repo.headAt(ref, { state, packageId }))?.hash ?? null; } /** @@ -18075,7 +18084,11 @@ export class ObjectStackProtocolImplementation implements * and from a package's own row to the package-less one (ADR-0048); a save * does not, it writes its own partition. A token of a row the save would * not overwrite would be refused by that save every time, so such a read - * serves `null` rather than the served row's token. + * serves `null` rather than the served row's token. [#22128] And the + * other way round: a `?state=draft` read that names no package serves the + * token of the draft a package-less draft save overwrites, which is the + * draft bound to the package of the item's active row when there is one + * (#11087) — the address the save resolves, not the key it was handed. */ private async readVersionToken( request: { type: string; name: string; packageId?: string }, @@ -18088,7 +18101,7 @@ export class ObjectStackProtocolImplementation implements type: PLURAL_TO_SINGULAR[request.type] ?? request.type, name: request.name, org: organizationId ?? 'env', - } as Parameters[0]; + } as Parameters[0]; const stored = await this.storedHeadAt(repo, ref, state, request.packageId ?? null); return stored === null ? null : this.receiptVersion(stored); } @@ -20932,7 +20945,9 @@ export class ObjectStackProtocolImplementation implements // package the upsert targets so a collision's other-package // row is never read as this item's parent. // [#22114] The head the item read's `version` is the keyed form of - // — one read for both halves ({@link storedHeadAt}). + // — one read for both halves ({@link storedHeadAt}). [#22128] The + // row the repository's `put` resolves for this address and locks + // against, the inherited draft binding included. const currentStored = await this.storedHeadAt( repo, ref, mode === 'draft' ? 'draft' : 'active', request.packageId ?? null, ); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index eaef13a2846..0f3514e0baa 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -554,6 +554,119 @@ export class SysMetadataRepository implements MetadataRepository { return this.rowToItem(ref, row); } + /** + * [#22128] The stored HEAD of one write address: the row a {@link put} with + * the same `state` and `packageId` upserts — and so the row its optimistic + * lock judges a parent against — served as {@link get} serves a row (its + * version is the one {@link lockAccepts} accepts). `null` when that put + * would create. + * + * Ask this, never {@link get}, for the parent of a write. `get` reads a row + * at the key it is handed; a write's key is not always the key its caller + * named. A `draft` put that names no package inherits the package of the + * item's active row (#11087), so `get(ref, { state: 'draft', packageId: + * null })` reads the package-UNBOUND row while the put upserts, and locks + * against, the inherited one. A parent taken from that read is `null` for a + * draft that exists, and an unpinned (ADR-0008 last-writer-wins) save was + * refused 409 by its own lock. + * + * Both answers come from ONE resolution, {@link resolveWriteHead}. + */ + async headAt( + ref: MetaRef, + opts: { state?: OverlayState; packageId?: string | null }, + ): Promise { + this.assertOpen(); + const { row } = await this.resolveWriteHead(ref, opts.state ?? 'active', opts.packageId); + if (!row) return null; + return this.rowToItem(ref, row); + } + + /** + * [#22128] THE write-address resolution: for a write of `state` that names + * `packageId`, the row it upserts (`row`, `null` for a create) and the + * binding it stamps on a create (`targetPackageId`). {@link put} runs it + * inside its transaction for its lock and its stamp; {@link headAt} runs it + * for a caller that needs the parent the lock will judge. + * + * ADR-0048 — a write is not a search: it upserts exactly one + * `(org, type, name, package_id)` row, so its scope is always a concrete + * package or the unbound row — never {@link get}'s "any package" match. + * #6215 was a caller (`restoreVersion`) whose silence about the binding was + * resolved to `null` here, so the lock looked up a row that does not exist + * for every package-bound overlay. Callers state their scope; nothing here + * decides it but the documented `PutOptions.packageId` default + * (omitted/undefined = the env-local, unbound row) and the draft + * inheritance below. A row for package B is never found for a save naming + * package A. + * + * [#11087] Draft-save package inheritance. A `state='draft'` save is a + * pending change OVER the published row, and every package-scoped consumer + * — `listDrafts({ packageId })`, the console's pending-changes surfaces, + * `publishPackageDrafts` — keys drafts by `package_id`. A caller that names + * no base (the console's plain `?mode=draft` save) used to stamp NULL even + * when the row it overlays is package-bound, producing an "orphan draft" + * that no package view counts and no per-package publish can ever promote + * (measured live: `_drafts` lists it, `_drafts?packageId=` does not). + * Inherit the overlaid ACTIVE row's binding instead. An explicit + * `packageId` is untouched (a caller that states its base keeps it, + * ADR-0048), and with no active row — a brand-new item drafted first — + * there is nothing to inherit and the package-less semantics stand. + * + * @param ctx - The transaction handle when the caller holds one. + */ + private async resolveWriteHead( + ref: Pick, + state: OverlayState, + packageId: string | null | undefined, + ctx?: Record, + ): Promise<{ row: any | null; targetPackageId: string | null }> { + const context = { ...ctx, isSystem: true }; + let targetPackageId: string | null = packageId ?? null; + if (state === 'draft' && packageId == null) { + // The overlaid base is resolved with the SAME two-scope reach the draft + // list applies (`listDrafts`' $or contract): an org-scoped caller's + // active row usually lives ENV-WIDE (`organization_id IS NULL`) — the + // ADR-0005 overlay order — so a same-org-only lookup here found nothing + // for exactly the console-session saves this inheritance exists for + // (measured live: an org-scoped view draft stayed `package_id NULL` + // over an env-wide active row bound to `app.k9qk`). + const activeWhere: Record = { + type: ref.type, + name: ref.name, + state: 'active', + }; + if (this.organizationId != null) { + activeWhere.$or = [ + { organization_id: this.organizationId }, + { organization_id: null }, + ]; + } else { + activeWhere.organization_id = null; + } + const activeRow = await this.engine.findOne('sys_metadata', { where: activeWhere, context }); + const activePkg = (activeRow as { package_id?: string | null } | null)?.package_id ?? null; + if (activePkg) targetPackageId = activePkg; + } + let row = await this.engine.findOne('sys_metadata', { + where: this.whereFor(ref, state, targetPackageId), + context, + }); + // [#11087] Orphan-draft adoption: when the package binding above was + // INHERITED (caller named none), a pre-fix draft for the same + // (org, type, name) sits at `package_id NULL` and the scoped lookup + // misses it — creating a second draft row would fork the pending change. + // That package-less row is the one the write updates; `put`'s stamp + // (`existingPkg ?? targetPackageId`) then adopts it into the package. + if (!row && state === 'draft' && packageId == null && targetPackageId !== null) { + row = await this.engine.findOne('sys_metadata', { + where: this.whereFor(ref, state, null), + context, + }); + } + return { row: row ?? null, targetPackageId }; + } + /** * Resolve a historical version by content hash (ADR-0009). * @@ -620,79 +733,14 @@ export class SysMetadataRepository implements MetadataRepository { // object's `fields`) keeps its key order, so a pure reorder is a change. const hash = hashSpec(body, ref.type); - // ADR-0048 — the ONE row this write targets. A write is not a search: it - // upserts exactly one `(org, type, name, package_id)` row, so its scope is - // always a concrete package or the unbound row — never `get`'s "any - // package" match. That asymmetry with the sibling read at {@link get} is - // deliberate, and it is why this value is named here rather than inlined: - // `put` reads it TWICE (the optimistic-lock lookup below and the - // `package_id` stamp), and #6215 was a caller — `restoreVersion` — whose - // silence about the binding was resolved to `null` by this expression, so - // the lock looked up a row that does not exist for every package-bound - // overlay. Callers state their scope; this line no longer decides it - // anywhere but for the documented `PutOptions.packageId` default - // (omitted/undefined = the env-local, unbound row). - let targetPackageId: string | null = opts.packageId ?? null; - // [#11087] Draft-save package inheritance. A `state='draft'` save is a - // pending change OVER the published row, and every package-scoped consumer - // — `listDrafts({ packageId })`, the console's pending-changes surfaces, - // `publishPackageDrafts` — keys drafts by `package_id`. A caller that names - // no base (the console's plain `?mode=draft` save) used to stamp NULL even - // when the row it overlays is package-bound, producing an "orphan draft" - // that no package view counts and no per-package publish can ever promote - // (measured live: `_drafts` lists it, `_drafts?packageId=` does not). - // Inherit the overlaid ACTIVE row's binding instead. Explicit - // `opts.packageId` is untouched (a caller that states its base keeps it, - // ADR-0048), and with no active row — a brand-new item drafted first — - // there is nothing to inherit and the package-less semantics stand. - if (state === 'draft' && opts.packageId == null) { - // The overlaid base is resolved with the SAME two-scope reach the draft - // list applies (`listDrafts`' $or contract): an org-scoped caller's - // active row usually lives ENV-WIDE (`organization_id IS NULL`) — the - // ADR-0005 overlay order — so a same-org-only lookup here found nothing - // for exactly the console-session saves this inheritance exists for - // (measured live: an org-scoped view draft stayed `package_id NULL` - // over an env-wide active row bound to `app.k9qk`). - const activeWhere: Record = { - type: ref.type, - name: ref.name, - state: 'active', - }; - if (this.organizationId != null) { - activeWhere.$or = [ - { organization_id: this.organizationId }, - { organization_id: null }, - ]; - } else { - activeWhere.organization_id = null; - } - const activeRow = await this.engine.findOne('sys_metadata', { where: activeWhere, context: { isSystem: true } }); - const activePkg = (activeRow as { package_id?: string | null } | null)?.package_id ?? null; - if (activePkg) targetPackageId = activePkg; - } - // Run all reads + writes inside one transaction so the optimistic // lock, the parent-row mutation, and the history append are atomic. const result = await this.withTxn(async (ctx) => { - // ADR-0048 — scope the existing-row lookup to the requested package so a - // save for package B does not find (and overwrite) package A's same-name - // overlay. A package-less save (packageId null) targets the global row. - let existing = await this.engine.findOne('sys_metadata', { - where: this.whereFor(ref, state, targetPackageId), - context: { ...ctx, isSystem: true }, - }); - // [#11087] Orphan-draft adoption: when the package binding above was - // INHERITED (caller named none), a pre-fix draft for the same - // (org, type, name) sits at `package_id NULL` and the scoped lookup - // misses it — creating a second draft row would fork the pending change. - // Re-read the package-less row and update THAT one; the stamp below - // (`existingPkg ?? targetPackageId`) then adopts it into the package. - if (!existing && state === 'draft' && opts.packageId == null && targetPackageId !== null) { - existing = await this.engine.findOne('sys_metadata', { - where: this.whereFor(ref, state, null), - context: { ...ctx, isSystem: true }, - }); - } + // [#22128] The row this write upserts and the binding it stamps on a + // create, from THE write-address resolution ({@link resolveWriteHead}): + // the one {@link headAt} answers too, so the head a caller was served + // for this address is the head this lock judges. + const { row: existing, targetPackageId } = await this.resolveWriteHead(ref, state, opts.packageId, ctx); // The row's stored stamp, as written: the parent link this write records // (`previous_checksum`, the event's `parentHash`) and the no-op check's // input. `null` for a row stored without one. diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 30dc3f1168c..3bdbd288ea0 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -1768,6 +1768,27 @@ function metaSavePreconditionPin(headers: Record | undefined): return { ok: true }; } +/** + * [#22128] The ONE reading of `?package=` on the `/meta/:type/:name` item doors + * — the read (`GET`), the save (`PUT`) and the publish + * (`POST …/publish`): the package the request names, or `undefined` when it + * names none. `all` is the metadata list's "show everything" scope and the + * empty value states nothing, so both name no package: the save writes the + * env-local overlay (its draft inheriting the package of the item's active + * row, #11087) and the publish keeps its historical match-any-package draft + * resolution. A non-string value is not forwarded either. + * + * One function because the read's `version` is the token the save at the same + * address compares against: a read that forwarded the literal `all` resolved + * that token at a package no save writes, so it served `null` beside a row the + * save then judged, and a client that pinned `If-None-Match: *` on that `null` + * was refused. ⛔ Never a third inline copy at a door: the save and publish + * doors each carried one, and the read door had none. + */ +function metaItemPackageBinding(raw: unknown): string | undefined { + return typeof raw === 'string' && raw !== '' && raw !== 'all' ? raw : undefined; +} + /** * [#20378 · #20441] THE AUTHORING-DOOR REFUSAL — ruling 5865708652 (letter B), * carried to `/audit` by triage's grade 5871509797. Sends it and answers `true` @@ -6867,7 +6888,11 @@ export class RestServer { )); } else { // Non-cached version - const packageId = req.query?.package || undefined; + // [#22128] The save door's reading of `?package=` + // ({@link metaItemPackageBinding}): the served + // `version` is resolved at the address that save + // writes, so `all` names no package here either. + const packageId = metaItemPackageBinding(req.query?.package); // [commit 2a29caa53] Typed against the spec request shape — // the `as any` this literal used to carry is // retired now that the spec declares `state` and @@ -7165,11 +7190,9 @@ export class RestServer { // Software-package binding (Studio package authoring). // `?package=` binds the saved row to that package - // (sys_metadata.package_id). 'all'/empty = env-local overlay. - const packageRaw = req.query?.package; - const packageId = typeof packageRaw === 'string' && packageRaw && packageRaw !== 'all' - ? packageRaw - : undefined; + // (sys_metadata.package_id). 'all'/empty = env-local overlay + // ({@link metaItemPackageBinding}, the item read's too). + const packageId = metaItemPackageBinding(req.query?.package); // [#8805] THE WRITE-SIDE ORGANIZATION. Until this landed the // door passed none, so `recordMetadataAudit` stamped @@ -7952,10 +7975,7 @@ export class RestServer { // does. Widening `MetadataItem` is a `packages/spec` contract // change and stays filed rather than taken here. if (refuseRepeatedQueryParams(req, res, ['package'])) return; - const packageRaw = req.query?.package; - const packageId = typeof packageRaw === 'string' && packageRaw && packageRaw !== 'all' - ? packageRaw - : undefined; + const packageId = metaItemPackageBinding(req.query?.package); // [#8805] The publish half of the same organization, and it // is REQUIRED for the `PUT` fix to be usable rather than a diff --git a/packages/spec/src/api/plugin-rest-api.zod.ts b/packages/spec/src/api/plugin-rest-api.zod.ts index 9e737451795..b161ec3044d 100644 --- a/packages/spec/src/api/plugin-rest-api.zod.ts +++ b/packages/spec/src/api/plugin-rest-api.zod.ts @@ -895,7 +895,7 @@ export const DEFAULT_METADATA_ROUTES: RestApiRouteRegistration = { 'Promotes the item\'s pending DRAFT overlay to the live `active` row and records an ' + '`op=\'publish\'` history event. The sibling write door of `PUT /:type/:name` — the ' + 'ADR-0033 two-step spelling, where `?mode=draft` stages a body and this makes it live. ' - + '404 `NO_DRAFT` when there is nothing to publish; 409 `metadata_conflict` when the ' + + '404 `NO_DRAFT` when there is nothing to publish; 409 `METADATA_CONFLICT` when the ' + 'published row advanced while the draft was held. The route was served for a long time with no ' + 'declaration behind it — this entry is what makes its response contract nameable, the same ' + 'declared-equals-returned rule the save door follows.', diff --git a/packages/spec/src/api/protocol.zod.ts b/packages/spec/src/api/protocol.zod.ts index 17970d0c430..60907a9a4c2 100644 --- a/packages/spec/src/api/protocol.zod.ts +++ b/packages/spec/src/api/protocol.zod.ts @@ -717,13 +717,19 @@ export const SaveMetaItemRequestSchema = lazySchema(() => z.object({ ), packageId: z.string().nullable().optional().describe( 'ADR-0048 — the software package to bind the saved row to ' - + '(`sys_metadata.package_id`; `?package=` on the REST door, sent ' - + 'only when it names a real package). Set when authoring inside a ' - + 'Studio package workspace; a named read-only base package is refused. ' - + 'On create the row is stamped with this id; on update an existing ' - + 'binding is preserved, never silently re-bound. Absent = env-local ' - + 'overlay (no package stamp); it also scopes which row the unpinned ' - + 'parent-version resolution reads.', + + '(`sys_metadata.package_id`; `?package=` on the REST door, where ' + + '`all` and an empty value name no package). Set when authoring inside ' + + 'a Studio package workspace; a named read-only base package is ' + + 'refused. On create the row is stamped with this id; on update an ' + + 'existing binding is preserved, never silently re-bound. Absent or ' + + '`null` names no package: an `active` save writes the env-local ' + + 'overlay (no package stamp), and a `draft` save writes the draft of ' + + 'the package the item\'s active row is bound to when it is ' + + 'package-bound (a pending change stays in the package it edits), else ' + + 'the env-local draft. That row is also the one the parent-version ' + + 'resolution reads: an unpinned save takes its version as the parent ' + + '(last-write-wins), a pinned one is judged against it, and the item ' + + 'read at the same scope serves it as `version`.', ), writeFace: z.enum(['package-duplicate', 'meta-envelope', 'meta-dispatch', 'external-import']).optional().describe( 'Which write door a refusal is being rendered FOR — stated by the ' From bee4b0e8cc3bfc5658fc489d4a89d274b0944f9e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 04:13:09 +0000 Subject: [PATCH 3/7] test(metadata-protocol): pin headAt as the row put locks against (#22128) Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848 Co-authored-by: Claude --- ...a-repository.draft-package-inherit.test.ts | 72 ++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts b/packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts index e1aec72b961..8efcd06660a 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts @@ -29,7 +29,7 @@ import { describe, it, expect } from 'vitest'; // The engine-double contract gate: a fake looser than ObjectQL's own verb // dispatch is how #4434 shipped a dead route with its suite green. -import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate, ConflictError } from '@objectstack/metadata-core'; import { SysMetadataRepository } from './sys-metadata-repository.js'; interface Row { @@ -176,3 +176,73 @@ describe('SysMetadataRepository draft-save package inheritance (#11087)', () => expect(drafts[0].package_id).toBe('app.k9qk'); // adopted into the package }); }); + +/** + * [#22128] `headAt` is the parent a write at one address must name: the row + * `put` upserts there and locks against, from the same resolution. `get` at + * the key the caller named is not — a package-less draft is stored in the + * package of the active row, so `get` at the unbound key reads no draft while + * `put` judges the inherited one. + */ +describe('SysMetadataRepository.headAt — the row put locks against (#22128)', () => { + const seededActive = (pkg: string | null) => ({ + type: REF.type, name: REF.name, organization_id: null, state: 'active', + package_id: pkg, metadata: '{"label":"Member"}', checksum: 'sha-active', version: 1, + }); + + it('a package-less draft address resolves the INHERITED draft, and put accepts its version as the parent', async () => { + const engine = makeFakeEngine([seededActive('app.k9qk')]); + const repo = makeRepo(engine); + const first = await repo.put(REF, { label: 'draft 1' }, { parentVersion: null, actor: 't', state: 'draft' as const }); + + const head = await repo.headAt(REF, { state: 'draft', packageId: null }); + expect(head?.hash).toBe(first.version); + // The key the caller named holds no draft: that read is not the parent. + expect(await repo.get(REF, { state: 'draft', packageId: null })).toBeNull(); + + await expect(repo.put(REF, { label: 'from null' }, { parentVersion: null, actor: 't', state: 'draft' as const })) + .rejects.toBeInstanceOf(ConflictError); + const second = await repo.put(REF, { label: 'draft 2' }, { parentVersion: head!.hash, actor: 't', state: 'draft' as const }); + expect((await repo.headAt(REF, { state: 'draft' }))?.hash).toBe(second.version); + expect(engine.rows.filter((r) => r.state === 'draft').map((r) => r.package_id)).toEqual(['app.k9qk']); + }); + + it('resolves a pre-fix orphan draft (NULL package) as the head the adopting put judges', async () => { + const engine = makeFakeEngine([ + seededActive('app.k9qk'), + { + type: REF.type, name: REF.name, organization_id: null, state: 'draft', + package_id: null, metadata: '{"label":"orphan"}', checksum: 'sha-orphan', version: 2, + }, + ]); + const repo = makeRepo(engine); + expect((await repo.headAt(REF, { state: 'draft', packageId: null }))?.hash).toBe('sha-orphan'); + }); + + it('an explicit package is its own address: never another package\'s row', async () => { + const engine = makeFakeEngine([seededActive('app.k9qk')]); + const repo = makeRepo(engine); + await repo.put(REF, { label: 'draft' }, { parentVersion: null, actor: 't', state: 'draft' as const }); + + expect(await repo.headAt(REF, { state: 'draft', packageId: 'app.other' })).toBeNull(); + expect((await repo.headAt(REF, { state: 'draft', packageId: 'app.k9qk' }))?.hash).toBeDefined(); + }); + + it('an active address inherits nothing: the unbound key is the head, as put writes it', async () => { + const engine = makeFakeEngine([seededActive('app.k9qk')]); + const repo = makeRepo(engine); + expect(await repo.headAt(REF, { state: 'active' })).toBeNull(); + expect((await repo.headAt(REF, { state: 'active', packageId: 'app.k9qk' }))?.hash).toBe('sha-active'); + }); + + it('an org-scoped draft address resolves across the ADR-0005 reach, as the inheriting put does', async () => { + const engine = makeFakeEngine([seededActive('app.k9qk')]); + const repo = new SysMetadataRepository({ + engine: engine as never, + organizationId: 'org_1', + orgLabel: 'org_1', + } as never); + const first = await repo.put(REF, { label: 'org draft' }, { parentVersion: null, actor: 't', state: 'draft' as const }); + expect((await repo.headAt(REF, { state: 'draft' }))?.hash).toBe(first.version); + }); +}); From 4b12ae9192aba9587e8b45c363ca2ab1d6b1b068 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 04:17:53 +0000 Subject: [PATCH 4/7] docs(spec): regenerate the protocol reference for the packageId describe; add the #22128 changeset Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848 Co-authored-by: Claude --- .changeset/22128-draft-head-resolution.md | 15 +++++++++++++++ content/docs/references/api/protocol.mdx | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) create mode 100644 .changeset/22128-draft-head-resolution.md diff --git a/.changeset/22128-draft-head-resolution.md b/.changeset/22128-draft-head-resolution.md new file mode 100644 index 00000000000..ae234a839a7 --- /dev/null +++ b/.changeset/22128-draft-head-resolution.md @@ -0,0 +1,15 @@ +--- +"@objectstack/metadata-protocol": patch +"@objectstack/rest": patch +"@objectstack/spec": patch +--- + +fix(meta): a second unpinned draft save of a package-owned item is no longer refused 409, and the item read serves that draft's `version` + +Clause-②: no + +- **The second draft save is accepted.** Save an item into a package (`PUT /api/v1/meta/:type/:name?package=`), then save a draft of it without `?package=` twice. The repository stores that draft in the item's package (the draft inheritance that keeps a pending change in the package it edits). The save door took its expected parent from the package-unbound draft row instead. It found none, so the second save, which sent no `If-Match`, was refused `409 METADATA_CONFLICT` ("Expected parent null but current is hmac-sha256:…"). ADR-0008 makes an unpinned save last-writer-wins, and it is accepted again. +- **One head row per write address.** `SysMetadataRepository.headAt(ref, { state, packageId })` answers the row a `put` at that address upserts, from the same resolution `put`'s optimistic lock runs. That resolution includes the draft inheritance and the adoption of a draft stored without a package. The save door's expected parent and the item read's `version` both come from it, so the token `GET /meta/:type/:name?state=draft` serves is the token a draft save at the same address accepts. Before this, that read served `version: null` while the inherited draft existed. +- **`?package=all` on the item read.** `GET /meta/:type/:name?package=all` now reads `all` as the save and publish doors read it: as naming no package. Before, the read forwarded the literal `all`. Its `version` was resolved at a package no save writes, so it served `null` beside a row the save then judged. Its `?state=draft` read answered `404 NO_DRAFT` for a draft stored in the item's package. +- **Descriptions only, in `@objectstack/spec`.** `SaveMetaItemRequestSchema.packageId` now states the draft inheritance, where it said "absent = env-local overlay". The `POST /meta/:type/:name/publish` route description spells the conflict code `METADATA_CONFLICT`, as the wire sends it. +- A stale `If-Match` is still refused `409`, and `If-None-Match: *` is still refused while a row exists. An item with no package binding is unchanged. No spec key, request field or response field is added or removed; `headAt` is the one new method, on the exported `SysMetadataRepository`. diff --git a/content/docs/references/api/protocol.mdx b/content/docs/references/api/protocol.mdx index a90904c3f07..6fafebc98e1 100644 --- a/content/docs/references/api/protocol.mdx +++ b/content/docs/references/api/protocol.mdx @@ -2601,7 +2601,7 @@ Installed package with runtime lifecycle state | **actor** | `string` | optional | Identity recorded on the write's history event (`recorded_by`, a lookup into `sys_user`) and audit row. On the REST door this is the request's authenticated identity (one producer) — never a caller-supplied header. Absent, the event is recorded actor-less (null), deliberately not attributed to "system". | | **force** | `boolean` | optional | Destructive-change acknowledgement (`?force=true` on the REST door): skips the safety diff that refuses an `object` save whose body drops fields or narrows types the stored item still carries (409 with the findings otherwise). Only `object` saves reach that diff, so the flag is inert for every other type. Absent = the guard runs. | | **mode** | `Enum<'draft' \| 'publish'>` | optional | Per-item lifecycle (ADR-0005 drafts): `draft` stages the body as a pending draft overlay (`?mode=draft` on the REST door; the publish door promotes it later); `publish` or ABSENT writes straight to the live `active` row — the legacy default, kept so callers that predate the draft/publish split keep working. Any value other than `draft` is read as `publish`. | -| **packageId** | `string \| null` | optional | ADR-0048 — the software package to bind the saved row to (`sys_metadata.package_id`; `?package=` on the REST door, sent only when it names a real package). Set when authoring inside a Studio package workspace; a named read-only base package is refused. On create the row is stamped with this id; on update an existing binding is preserved, never silently re-bound. Absent = env-local overlay (no package stamp); it also scopes which row the unpinned parent-version resolution reads. | +| **packageId** | `string \| null` | optional | ADR-0048 — the software package to bind the saved row to (`sys_metadata.package_id`; `?package=` on the REST door, where `all` and an empty value name no package). Set when authoring inside a Studio package workspace; a named read-only base package is refused. On create the row is stamped with this id; on update an existing binding is preserved, never silently re-bound. Absent or `null` names no package: an `active` save writes the env-local overlay (no package stamp), and a `draft` save writes the draft of the package the item's active row is bound to when it is package-bound (a pending change stays in the package it edits), else the env-local draft. That row is also the one the parent-version resolution reads: an unpinned save takes its version as the parent (last-write-wins), a pinned one is judged against it, and the item read at the same scope serves it as `version`. | | **writeFace** | `Enum<'package-duplicate' \| 'meta-envelope' \| 'meta-dispatch' \| 'external-import'>` | optional | Which write door a refusal is being rendered FOR — stated by the SERVER, never by a remote caller: every door builds this request field by field and never spreads a request body into it, so there is no path for a client to smuggle a face in, and a face arriving in a wire body is simply never read. Two refusals branch on it, for different questions: the 409 destructive-change remedy names the acknowledgement mechanism that actually exists on the refusing door (`?force=true` on the REST doors; the dispatcher, the duplicate-package door and the external-table import have none), and the 422 invalid-metadata message adapts to whether a structured `issues[]` channel reaches the consumer beside it. Absent = the conservative default wording. | From 6c5740fb70d8498b839296a1d1833df3f6eef3cd Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 04:18:04 +0000 Subject: [PATCH 5/7] docs(changeset): state what the ?package=all item read now serves (#22128) Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848 Co-authored-by: Claude --- .changeset/22128-draft-head-resolution.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/22128-draft-head-resolution.md b/.changeset/22128-draft-head-resolution.md index ae234a839a7..765ed5d87a8 100644 --- a/.changeset/22128-draft-head-resolution.md +++ b/.changeset/22128-draft-head-resolution.md @@ -10,6 +10,6 @@ Clause-②: no - **The second draft save is accepted.** Save an item into a package (`PUT /api/v1/meta/:type/:name?package=`), then save a draft of it without `?package=` twice. The repository stores that draft in the item's package (the draft inheritance that keeps a pending change in the package it edits). The save door took its expected parent from the package-unbound draft row instead. It found none, so the second save, which sent no `If-Match`, was refused `409 METADATA_CONFLICT` ("Expected parent null but current is hmac-sha256:…"). ADR-0008 makes an unpinned save last-writer-wins, and it is accepted again. - **One head row per write address.** `SysMetadataRepository.headAt(ref, { state, packageId })` answers the row a `put` at that address upserts, from the same resolution `put`'s optimistic lock runs. That resolution includes the draft inheritance and the adoption of a draft stored without a package. The save door's expected parent and the item read's `version` both come from it, so the token `GET /meta/:type/:name?state=draft` serves is the token a draft save at the same address accepts. Before this, that read served `version: null` while the inherited draft existed. -- **`?package=all` on the item read.** `GET /meta/:type/:name?package=all` now reads `all` as the save and publish doors read it: as naming no package. Before, the read forwarded the literal `all`. Its `version` was resolved at a package no save writes, so it served `null` beside a row the save then judged. Its `?state=draft` read answered `404 NO_DRAFT` for a draft stored in the item's package. +- **`?package=all` on the item read.** `GET /meta/:type/:name?package=all` now reads `all` as the save and publish doors read it: as naming no package. Before, the read forwarded the literal `all`. Its `version` was resolved at a package no save writes, so it served `null` beside a row the save then judged. Its `?state=draft` read answered `404 NO_DRAFT` for a draft stored in the item's package. The read now serves the item as a read with no `?package=` serves it, and `?package=all` still takes the uncached arm, so it keeps serving `version`. - **Descriptions only, in `@objectstack/spec`.** `SaveMetaItemRequestSchema.packageId` now states the draft inheritance, where it said "absent = env-local overlay". The `POST /meta/:type/:name/publish` route description spells the conflict code `METADATA_CONFLICT`, as the wire sends it. - A stale `If-Match` is still refused `409`, and `If-None-Match: *` is still refused while a row exists. An item with no package binding is unchanged. No spec key, request field or response field is added or removed; `headAt` is the one new method, on the exported `SysMetadataRepository`. From 4e63f11220a58c20c66e7449ba6c90b028a4ef07 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 05:38:07 +0000 Subject: [PATCH 6/7] fix(metadata-protocol): the credential carry-forward reads the row the draft save overwrites (#22128) storedBodyForCarryForward read the stored body at the key the caller named. For a package-less draft save of a package-owned datasource that is neither the inherited draft nor the package-bound active row, so the comparison fell to the code layer and the stored credential was persisted away (measured on the real route). Both of its reads now go through SysMetadataRepository.headAt, which also answers the binding the write targets, so the active fallback reads the row the draft overlays. Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848 Co-authored-by: Claude --- packages/metadata-protocol/src/protocol.ts | 31 ++++-- ...a-repository.draft-package-inherit.test.ts | 26 +++-- .../src/sys-metadata-repository.ts | 36 ++++--- ...eta-draft-head-package-inheritance.test.ts | 101 ++++++++++++++++-- 4 files changed, 151 insertions(+), 43 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index ece667b9505..64b88259848 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -8586,12 +8586,20 @@ export class ObjectStackProtocolImplementation implements * back. Here the stakes are higher than a phantom customization — the * subtracted material is a credential nobody can retype from the wire. * - * Reads the row at rest through the overlay repository, whose `get()` is - * documented VERBATIM (no ADR-0087 conversion): the comparison must be + * Reads the row at rest through the overlay repository, whose rows are + * served VERBATIM (no ADR-0087 conversion): the comparison must be * against the bytes that were written, because those are the bytes the read * exit redacted. {@link carryForwardRedactedValues} then decides per * redacted path — see its docblock for the three outcomes. * + * [#22128] The row is the one THIS save overwrites, from the repository's + * write-address resolution ({@link SysMetadataRepository.headAt}) — ⛔ never + * a read at the key the caller named. A draft save that names no package is + * stored in the package of the item's active row (#11087); read at the + * package-unbound key, neither that draft nor that active row was found, + * the comparison fell to the code layer, and the first package-less draft + * save of a package-owned datasource persisted its stored credential away. + * * ⚠️ THE DRAFT FALLBACK IS LOAD-BEARING, not defensive. A `?mode=draft` * save of an item with no draft row yet has nothing at its own state to * compare against, while the body the author edited came from the ACTIVE @@ -8621,8 +8629,9 @@ export class ObjectStackProtocolImplementation implements } /** - * The body a carry-forward compares against: the overlay row at the write's - * own state, the ACTIVE row for a draft save with no draft row yet (see + * The body a carry-forward compares against: the row the write overwrites + * at its own state ({@link SysMetadataRepository.headAt}), the ACTIVE row + * the draft overlays for a draft save with no draft row yet (see * {@link carryForwardRedactedCredentials} for why that fallback is * load-bearing), else the CODE layer. RAW in every case — the bytes the * read exits redacted — and read with no `try`/`catch`, for the reason the @@ -8635,15 +8644,19 @@ export class ObjectStackProtocolImplementation implements state: 'draft' | 'active'; packageId: string | null; }): Promise { - let stored = await args.repo.get(args.ref, { + // [#22128] The row this save overwrites, and with no draft row yet the + // active row the draft overlays: the one in the package the draft is + // stamped into (`packageId`, the inherited binding included). + const target = await args.repo.headAt(args.ref, { state: args.state, packageId: args.packageId, }); + let stored = target.head; if (!stored && args.state === 'draft') { - stored = await args.repo.get(args.ref, { + stored = (await args.repo.headAt(args.ref, { state: 'active', - packageId: args.packageId, - }); + packageId: target.packageId, + })).head; } // [#20552] NO overlay row at either state: the body the read served is // the CODE layer, so that is what the incoming body is compared with. @@ -18065,7 +18078,7 @@ export class ObjectStackProtocolImplementation implements state: 'active' | 'draft', packageId: string | null, ): Promise { - return (await repo.headAt(ref, { state, packageId }))?.hash ?? null; + return (await repo.headAt(ref, { state, packageId })).head?.hash ?? null; } /** diff --git a/packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts b/packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts index 8efcd06660a..cb31f2fc9eb 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts @@ -195,15 +195,17 @@ describe('SysMetadataRepository.headAt — the row put locks against (#22128)', const repo = makeRepo(engine); const first = await repo.put(REF, { label: 'draft 1' }, { parentVersion: null, actor: 't', state: 'draft' as const }); - const head = await repo.headAt(REF, { state: 'draft', packageId: null }); + const { head, packageId } = await repo.headAt(REF, { state: 'draft', packageId: null }); expect(head?.hash).toBe(first.version); + // The binding the put writes under: the inherited package. + expect(packageId).toBe('app.k9qk'); // The key the caller named holds no draft: that read is not the parent. expect(await repo.get(REF, { state: 'draft', packageId: null })).toBeNull(); await expect(repo.put(REF, { label: 'from null' }, { parentVersion: null, actor: 't', state: 'draft' as const })) .rejects.toBeInstanceOf(ConflictError); const second = await repo.put(REF, { label: 'draft 2' }, { parentVersion: head!.hash, actor: 't', state: 'draft' as const }); - expect((await repo.headAt(REF, { state: 'draft' }))?.hash).toBe(second.version); + expect((await repo.headAt(REF, { state: 'draft' })).head?.hash).toBe(second.version); expect(engine.rows.filter((r) => r.state === 'draft').map((r) => r.package_id)).toEqual(['app.k9qk']); }); @@ -216,7 +218,10 @@ describe('SysMetadataRepository.headAt — the row put locks against (#22128)', }, ]); const repo = makeRepo(engine); - expect((await repo.headAt(REF, { state: 'draft', packageId: null }))?.hash).toBe('sha-orphan'); + const adopted = await repo.headAt(REF, { state: 'draft', packageId: null }); + expect(adopted.head?.hash).toBe('sha-orphan'); + // The orphan is updated INTO the package: the put's binding is the inherited one. + expect(adopted.packageId).toBe('app.k9qk'); }); it('an explicit package is its own address: never another package\'s row', async () => { @@ -224,15 +229,15 @@ describe('SysMetadataRepository.headAt — the row put locks against (#22128)', const repo = makeRepo(engine); await repo.put(REF, { label: 'draft' }, { parentVersion: null, actor: 't', state: 'draft' as const }); - expect(await repo.headAt(REF, { state: 'draft', packageId: 'app.other' })).toBeNull(); - expect((await repo.headAt(REF, { state: 'draft', packageId: 'app.k9qk' }))?.hash).toBeDefined(); + expect(await repo.headAt(REF, { state: 'draft', packageId: 'app.other' })).toEqual({ head: null, packageId: 'app.other' }); + expect((await repo.headAt(REF, { state: 'draft', packageId: 'app.k9qk' })).head?.hash).toBeDefined(); }); it('an active address inherits nothing: the unbound key is the head, as put writes it', async () => { const engine = makeFakeEngine([seededActive('app.k9qk')]); const repo = makeRepo(engine); - expect(await repo.headAt(REF, { state: 'active' })).toBeNull(); - expect((await repo.headAt(REF, { state: 'active', packageId: 'app.k9qk' }))?.hash).toBe('sha-active'); + expect(await repo.headAt(REF, { state: 'active' })).toEqual({ head: null, packageId: null }); + expect((await repo.headAt(REF, { state: 'active', packageId: 'app.k9qk' })).head?.hash).toBe('sha-active'); }); it('an org-scoped draft address resolves across the ADR-0005 reach, as the inheriting put does', async () => { @@ -243,6 +248,11 @@ describe('SysMetadataRepository.headAt — the row put locks against (#22128)', orgLabel: 'org_1', } as never); const first = await repo.put(REF, { label: 'org draft' }, { parentVersion: null, actor: 't', state: 'draft' as const }); - expect((await repo.headAt(REF, { state: 'draft' }))?.hash).toBe(first.version); + expect((await repo.headAt(REF, { state: 'draft' })).head?.hash).toBe(first.version); + }); + + it('a brand-new item drafted first: no head, and the package-less binding (nothing to inherit)', async () => { + const repo = makeRepo(makeFakeEngine()); + expect(await repo.headAt(REF, { state: 'draft' })).toEqual({ head: null, packageId: null }); }); }); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index 0f3514e0baa..cfe84ec1ede 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -558,28 +558,30 @@ export class SysMetadataRepository implements MetadataRepository { * [#22128] The stored HEAD of one write address: the row a {@link put} with * the same `state` and `packageId` upserts — and so the row its optimistic * lock judges a parent against — served as {@link get} serves a row (its - * version is the one {@link lockAccepts} accepts). `null` when that put - * would create. - * - * Ask this, never {@link get}, for the parent of a write. `get` reads a row - * at the key it is handed; a write's key is not always the key its caller - * named. A `draft` put that names no package inherits the package of the - * item's active row (#11087), so `get(ref, { state: 'draft', packageId: - * null })` reads the package-UNBOUND row while the put upserts, and locks - * against, the inherited one. A parent taken from that read is `null` for a - * draft that exists, and an unpinned (ADR-0008 last-writer-wins) save was - * refused 409 by its own lock. - * - * Both answers come from ONE resolution, {@link resolveWriteHead}. + * version is the one {@link lockAccepts} accepts), or `null` when that put + * would create; and the package binding that put writes under (`packageId` + * here: the named package, the one a package-less draft inherits, or `null` + * for the package-unbound row). + * + * Ask this, never {@link get}, for anything a write at an address compares + * against — its parent, or the stored body it carries a withheld credential + * forward from. `get` reads a row at the key it is handed; a write's key is + * not always the key its caller named. A `draft` put that names no package + * inherits the package of the item's active row (#11087), so + * `get(ref, { state: 'draft', packageId: null })` reads the package-UNBOUND + * row while the put upserts, and locks against, the inherited one. A parent + * taken from that read is `null` for a draft that exists, and an unpinned + * (ADR-0008 last-writer-wins) save was refused 409 by its own lock. + * + * Every answer comes from ONE resolution, {@link resolveWriteHead}. */ async headAt( ref: MetaRef, opts: { state?: OverlayState; packageId?: string | null }, - ): Promise { + ): Promise<{ head: MetadataItem | null; packageId: string | null }> { this.assertOpen(); - const { row } = await this.resolveWriteHead(ref, opts.state ?? 'active', opts.packageId); - if (!row) return null; - return this.rowToItem(ref, row); + const { row, targetPackageId } = await this.resolveWriteHead(ref, opts.state ?? 'active', opts.packageId); + return { head: row ? this.rowToItem(ref, row) : null, packageId: targetPackageId }; } /** diff --git a/packages/rest/src/meta-draft-head-package-inheritance.test.ts b/packages/rest/src/meta-draft-head-package-inheritance.test.ts index e1feea20d67..46512cd8029 100644 --- a/packages/rest/src/meta-draft-head-package-inheritance.test.ts +++ b/packages/rest/src/meta-draft-head-package-inheritance.test.ts @@ -17,7 +17,9 @@ * * `?package=all` is the second member of that class. The save door folds it * to the env-local overlay; the read door forwarded the literal `all`, so its - * `version` was resolved at a package address no save writes. + * `version` was resolved at a package address no save writes. The credential + * carry-forward (#8154) is the third: it compared the served body against the + * row at the named key, not the row the draft save overwrites. * * Driven at the HTTP door on the real stack: a better-sqlite3 `:memory:` * engine, the real `sys_metadata*` objects, a real @@ -38,6 +40,7 @@ import { ObjectQL } from '@objectstack/objectql'; import { SqlDriver } from '@objectstack/driver-sql'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; import { SysMetadata, SysMetadataHistoryObject, SysMetadataAuditObject } from '@objectstack/platform-objects/metadata'; +import { hashSpec } from '@objectstack/metadata-core'; import { RestServer } from './rest-server.js'; /** `registry.registerObject` requires a package id (see the sibling real-stack tests). */ @@ -92,8 +95,11 @@ function makeRes() { /** * Boot the real stack with the cache off: the default cached arm of the plain * read publishes no OCC carriers, so the active-row pins read the uncached arm. + * `item` names the one item the helpers address; `permissions` the caller's + * system capabilities (a `datasource` is read and written under + * `manage_platform_settings`). */ -async function boot() { +async function boot(opts: { item?: { type: string; name: string }; permissions?: string[] } = {}) { const engine = new ObjectQL(); liveEngines.push(engine); engine.registerDriver(new SqlDriver({ @@ -112,7 +118,8 @@ async function boot() { const config = { api: { requireAuth: false }, metadata: { enableCache: false } }; const rest: any = new RestServer(createMockServer() as any, protocol as any, config as any); // An author: `manage_metadata` saves and reads drafts. - rest.resolveExecCtx = async () => ({ userId: 'u_author', systemPermissions: ['manage_metadata'] }); + const systemPermissions = opts.permissions ?? ['manage_metadata']; + rest.resolveExecCtx = async () => ({ userId: 'u_author', systemPermissions }); rest.registerRoutes(); const route = (method: string, path: string) => { @@ -125,8 +132,8 @@ async function boot() { await route(method, routePath).handler({ method, headers: {}, query: {}, params: {}, ...req }, res); return res; }; - const item = { type: 'view', name: 'case_grid' }; - const path = `${META}/view/case_grid`; + const item = opts.item ?? { type: 'view', name: 'case_grid' }; + const path = `${META}/${item.type}/${item.name}`; /** `GET /meta/view/case_grid`. */ const read = (query: Record = {}) => @@ -136,15 +143,31 @@ async function boot() { call('PUT', `${META}/:type/:name`, { path, params: item, query, headers, body }); /** The stored rows of the item at one lifecycle, with their binding — the store, not the door's word for it. */ - const stored = async (state: 'active' | 'draft') => { - const rows = await engine.find('sys_metadata', { where: { type: 'view', name: 'case_grid', state } }); + const storedBodies = async (state: 'active' | 'draft') => { + const rows = await engine.find('sys_metadata', { where: { type: item.type, name: item.name, state } }); return (rows ?? []).map((r: any) => ({ - label: JSON.parse(String(r.metadata)).label as string, + body: JSON.parse(String(r.metadata)) as Record, packageId: (r.package_id ?? null) as string | null, })); }; + const stored = async (state: 'active' | 'draft') => + (await storedBodies(state)).map(({ body, packageId }) => ({ label: body.label as string, packageId })); + + /** + * A row as it exists at rest from before the write gates — the only way a + * stored credential is still at rest today, since the save door refuses + * one — written straight to the store, stamped as `put` stamps a row. + */ + const seed = async (state: 'active' | 'draft', packageId: string | null, body: Record) => { + const now = new Date().toISOString(); + await engine.insert('sys_metadata', { + id: `seed_${state}_${packageId ?? 'unbound'}`, type: item.type, name: item.name, organization_id: null, + package_id: packageId, state, metadata: JSON.stringify(body), checksum: hashSpec(body, item.type), + version: 1, created_at: now, updated_at: now, + }, { context: { isSystem: true } } as any); + }; - return { read, save, stored }; + return { read, save, stored, storedBodies, seed }; } const ok = (res: any) => { @@ -267,3 +290,63 @@ describe('[#22128] `?package=all`: the read resolves `all` the way the save does expect(await h.stored('draft')).toEqual([{ label: 'draft 2', packageId: PKG }]); }, 60_000); }); + +/** + * [#22128] The credential carry-forward (#8154) compares the served body + * against the row the save OVERWRITES. It read at the key the caller named, so + * a package-less draft save of a package-owned item found neither the + * inherited draft nor the package-bound active row, compared against the code + * layer, and persisted the stored credential away. Measured on the real route + * before the fix: the stored draft's `config.url` no longer held it. + */ +describe('[#22128] the credential carry-forward reads the row the draft save overwrites', () => { + /** A fixture value standing in for the userinfo password a legacy row holds. */ + const URL_CREDENTIAL = 'fixture-not-a-credential'; + const DATASOURCE = { type: 'datasource', name: 'warehouse' }; + const legacyDatasource = (label: string) => ({ + name: 'warehouse', + label, + driver: 'postgres', + config: { + host: 'db.internal', + port: 5432, + database: 'warehouse', + username: 'reporting', + url: `postgresql://reporting:${URL_CREDENTIAL}@db.internal:5432/warehouse`, + }, + }); + const holdsCredential = (body: Record) => String(body?.config?.url ?? '').includes(URL_CREDENTIAL); + const bootDatasource = () => boot({ item: DATASOURCE, permissions: ['manage_metadata', 'manage_platform_settings'] }); + + it('package-owned: the redacted read saved back as a package-less draft, twice, keeps the stored credential', async () => { + const h = await bootDatasource(); + await h.seed('active', PKG, legacyDatasource('live')); + + const served = ok(await h.read()).item; + expect(holdsCredential(served)).toBe(false); + ok(await h.save({ ...served, label: 'draft 1' }, { mode: 'draft' })); + const first = await h.storedBodies('draft'); + expect(first.map((r) => r.packageId)).toEqual([PKG]); + expect(first.map((r) => holdsCredential(r.body))).toEqual([true]); + + const servedDraft = ok(await h.read({ state: 'draft' })).item; + expect(holdsCredential(servedDraft)).toBe(false); + ok(await h.save({ ...servedDraft, label: 'draft 2' }, { mode: 'draft' })); + const second = await h.storedBodies('draft'); + expect(second.map((r) => [r.body.label, r.packageId, holdsCredential(r.body)])).toEqual([['draft 2', PKG, true]]); + // The active row is untouched. + expect((await h.storedBodies('active')).map((r) => holdsCredential(r.body))).toEqual([true]); + }, 60_000); + + it('control: env-local, the same round trip keeps the stored credential, as before', async () => { + const h = await bootDatasource(); + await h.seed('active', null, legacyDatasource('live')); + + const served = ok(await h.read()).item; + ok(await h.save({ ...served, label: 'draft 1' }, { mode: 'draft' })); + const servedDraft = ok(await h.read({ state: 'draft' })).item; + ok(await h.save({ ...servedDraft, label: 'draft 2' }, { mode: 'draft' })); + const drafts = await h.storedBodies('draft'); + expect(drafts.map((r) => [r.body.label, r.packageId, holdsCredential(r.body)])).toEqual([['draft 2', null, true]]); + }, 60_000); +}); From ea3c748f8c42f6e5a0266b405a33430bc9a70477 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 8 Oct 2026 05:40:50 +0000 Subject: [PATCH 7/7] =?UTF-8?q?docs(changeset):=20@objectstack/metadata-pr?= =?UTF-8?q?otocol=20minor,=20Clause-=E2=91=A1:=20yes=20(widening)=20for=20?= =?UTF-8?q?the=20added=20headAt;=20the=20carry-forward=20bullet=20(#22128)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude-Session: https://claude.ai/code/session_01RPo7FUd6bSnAfkWMAKi848 Co-authored-by: Claude --- .changeset/22128-draft-head-resolution.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/.changeset/22128-draft-head-resolution.md b/.changeset/22128-draft-head-resolution.md index 765ed5d87a8..50f79e68959 100644 --- a/.changeset/22128-draft-head-resolution.md +++ b/.changeset/22128-draft-head-resolution.md @@ -1,15 +1,17 @@ --- -"@objectstack/metadata-protocol": patch +"@objectstack/metadata-protocol": minor "@objectstack/rest": patch "@objectstack/spec": patch --- -fix(meta): a second unpinned draft save of a package-owned item is no longer refused 409, and the item read serves that draft's `version` +fix(meta): a second unpinned draft save of a package-owned item is no longer refused 409, the item read serves that draft's `version`, and a package-less draft save keeps a package-owned datasource's stored credential -Clause-②: no +Clause-②: yes (widening) - **The second draft save is accepted.** Save an item into a package (`PUT /api/v1/meta/:type/:name?package=`), then save a draft of it without `?package=` twice. The repository stores that draft in the item's package (the draft inheritance that keeps a pending change in the package it edits). The save door took its expected parent from the package-unbound draft row instead. It found none, so the second save, which sent no `If-Match`, was refused `409 METADATA_CONFLICT` ("Expected parent null but current is hmac-sha256:…"). ADR-0008 makes an unpinned save last-writer-wins, and it is accepted again. -- **One head row per write address.** `SysMetadataRepository.headAt(ref, { state, packageId })` answers the row a `put` at that address upserts, from the same resolution `put`'s optimistic lock runs. That resolution includes the draft inheritance and the adoption of a draft stored without a package. The save door's expected parent and the item read's `version` both come from it, so the token `GET /meta/:type/:name?state=draft` serves is the token a draft save at the same address accepts. Before this, that read served `version: null` while the inherited draft existed. +- **One head row per write address.** `SysMetadataRepository.headAt(ref, { state, packageId })` answers `{ head, packageId }`: the row a `put` at that address upserts (`null` for a create), and the package binding that put writes under. Both come from the same resolution `put`'s optimistic lock runs. That resolution includes the draft inheritance and the adoption of a draft stored without a package. The save door's expected parent and the item read's `version` both come from it, so the token `GET /meta/:type/:name?state=draft` serves is the token a draft save at the same address accepts. Before this, that read served `version: null` while the inherited draft existed. - **`?package=all` on the item read.** `GET /meta/:type/:name?package=all` now reads `all` as the save and publish doors read it: as naming no package. Before, the read forwarded the literal `all`. Its `version` was resolved at a package no save writes, so it served `null` beside a row the save then judged. Its `?state=draft` read answered `404 NO_DRAFT` for a draft stored in the item's package. The read now serves the item as a read with no `?package=` serves it, and `?package=all` still takes the uncached arm, so it keeps serving `version`. +- **The credential carry-forward reads the same row.** A save of a type whose read withholds a credential (`datasource`; `flow` with the automation plugin) carries the stored credential back into a body that returns as it was served. It compared that body against the row at the key the caller named. A package-less draft save of a package-owned item therefore found neither the inherited draft nor the package's active row, and compared against the code layer. The stored credential was persisted away on the first such save, measured on the real route with a datasource whose `config.url` holds one. Both reads now go through `headAt`: the draft the save overwrites, or with no draft yet the active row in the package the draft is stamped into. An item with no package binding is unchanged. - **Descriptions only, in `@objectstack/spec`.** `SaveMetaItemRequestSchema.packageId` now states the draft inheritance, where it said "absent = env-local overlay". The `POST /meta/:type/:name/publish` route description spells the conflict code `METADATA_CONFLICT`, as the wire sends it. -- A stale `If-Match` is still refused `409`, and `If-None-Match: *` is still refused while a row exists. An item with no package binding is unchanged. No spec key, request field or response field is added or removed; `headAt` is the one new method, on the exported `SysMetadataRepository`. +- A stale `If-Match` is still refused `409`, and `If-None-Match: *` is still refused while a row exists. An item with no package binding is unchanged. No spec key, request field or response field is added or removed. +- The only change to the public surface is one added method on the exported `SysMetadataRepository`: `headAt`. No existing signature, schema or accept set changes.