From e018e7aee60511c4f56729a37f01a8590feb924c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 21:09:27 +0000 Subject: [PATCH 1/2] fix(metadata-protocol): thread packageId into both resolveDraftOrgScopeForPublish probes (#11003) Maintainer ruling 2026-08-22, option A: the scope probes ask the promote's question. A package-stating per-item publish now resolves the org scope of the draft it named; the package-agnostic probe could match another package's row in the caller's org and answer 404 [no_draft] over a publishable env-wide draft. Accepted cost, on the record: a caller stating a package no longer discovers a no-package draft of the same (type, name). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --- .../publish-scope-probe-package-dimension.md | 11 + ...tocol-publish-drafts-package-scope.test.ts | 275 +++++++++++++++++- packages/metadata-protocol/src/protocol.ts | 70 +++-- 3 files changed, 337 insertions(+), 19 deletions(-) create mode 100644 .changeset/publish-scope-probe-package-dimension.md diff --git a/.changeset/publish-scope-probe-package-dimension.md b/.changeset/publish-scope-probe-package-dimension.md new file mode 100644 index 0000000000..0d59bff225 --- /dev/null +++ b/.changeset/publish-scope-probe-package-dimension.md @@ -0,0 +1,11 @@ +--- +'@objectstack/metadata-protocol': minor +--- + +**BREAKING** (behavioral narrowing, maintainer-adjudicated): a per-item publish that states a package (`POST /api/v1/meta/:type/:name/publish?package=PKG_ID`) now resolves its draft's org scope package-exactly. Both probes in `resolveDraftOrgScopeForPublish` carry the stated `package_id`, so the scope probe asks the same question the promote does. + +What this fixes: with two packages holding drafts for one `(type, name)` in different org scopes (the ADR-0048 coexistence), the package-agnostic scope probe could match another package's row in the caller's org, name that scope, and the package-exact promote then answered `404 [no_draft]` over a publishable draft sitting env-wide — the exact row the caller named. + +What you may newly see: a publish that states `?package=` no longer discovers a draft of the same `(type, name)` authored with no package binding — it answers `404 [no_draft]`. That narrowing is the ruling, not a side effect: a mistyped package must fail loudly rather than silently publish some other package's draft. Remedy: if the draft you mean is the package-less one, retry the publish without the `?package=` query parameter; an unstated package keeps the historical match-any resolution. + + diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts index b80b0a6e1f..a699e6c737 100644 --- a/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts +++ b/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts @@ -1,6 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import { describe, expect, it } from 'vitest'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; // [#5619] The producer's OWN write-verb dispatch decisions (#4550 delete / // #5480 update), so the fake engine below cannot accept a call ObjectQL // refuses. Imported from `@objectstack/metadata-core` and not from @@ -393,3 +393,276 @@ describe('publishMetaItem — the per-item door names a package too (#10350)', ( expect(labelOf(active[0])).toBe('FROM_OTHER'); }); }); + +/** + * [#11003] The ORG-SCOPE probes' half of the same ADR-0048 key — maintainer + * ruling 2026-08-22, option A (recorded on the issue): the scope probes ask + * the promote's question, i.e. `resolveDraftOrgScopeForPublish` threads the + * stated `packageId` into BOTH of its `sys_metadata` probes. + * + * ## The defect these cases reproduce + * + * With two packages holding drafts for ONE `(type, name)` in DIFFERENT org + * scopes, a package-stating publish resolved the wrong scope: probe 1 was + * package-agnostic, matched the OTHER package's row in the caller's org, + * named that org as the scope — and the promote (whose `whereFor` IS + * package-exact since #8907/#10350) then found nothing there and answered + * `404 [no_draft]` over a draft sitting env-wide, publishable, and named by + * the caller. + * + * Unlike the #8907 cases above, NO insertion-order rigging is needed for the + * wrong row to win: the two drafts live in different org partitions, so probe + * 1's `organization_id` filter alone selects the foreign package's row — the + * pre-fix failure is deterministic, not a driver-order coin toss. + * + * ## Why type `object` and the `OS_METADATA_WRITABLE` hatch + * + * The card's scenario is `(object, shared_ticket)`. `object` is + * `allowOrgOverride: false` in the static registry, so an org-scoped object + * draft exists only where the operator hatch (`OS_METADATA_WRITABLE=object` + * — the Studio-side editing escape, #6190 R7) is open; with the hatch closed + * the promote's own #6190 gate would answer `403 [not_overridable]` before + * the probes' answer mattered, and the card's measured `404 [no_draft]` + * could not be reproduced as filed. The hatch is scoped to this describe + * (`beforeAll`/`afterAll` + cache reset), the same pattern + * `protocol.org-scoped-write-refused.test.ts` R7 uses. + * + * ## Accepted cost, pinned on purpose + * + * The ruling's own words: a caller stating a package no longer discovers a + * no-package draft of the same `(type, name)` — it 404s and the caller + * retries without `?package=`; that narrowing is the ruling, not a side + * effect. The last case pins BOTH halves of that sentence. The package-less + * draft row is seeded by DIRECT `engine.insert`, not through + * `saveMetaItem(mode:'draft')` — PR #11139 is changing how a package-less + * draft save resolves its binding (inheriting the overlaid active row's + * `package_id`), so a fixture seeded through that save path would stop + * meaning "a package-less draft exists" the day it lands. + */ +describe('publishMetaItem — the scope probes ask the promote\'s question (#11003)', () => { + beforeAll(() => { + process.env.OS_METADATA_WRITABLE = 'object'; + ObjectStackProtocolImplementation.resetEnvWritableCache(); + }); + afterAll(() => { + delete process.env.OS_METADATA_WRITABLE; + ObjectStackProtocolImplementation.resetEnvWritableCache(); + }); + + /** + * The card's coexistence arrangement: `app.other` holds the caller's-org + * (`org1`) draft, `app.demo` holds the env-wide one. Distinct ADR-0048 + * rows — different `(org, package)` pairings, one `(type, name)`. + */ + async function seedCrossScopeDrafts(protocol: ObjectStackProtocolImplementation) { + await protocol.saveMetaItem({ + type: 'object', + name: 'shared_ticket', + item: objectBody('shared_ticket', 'FROM_OTHER_ORG1'), + packageId: 'app.other', + organizationId: 'org1', + mode: 'draft', + }); + await protocol.saveMetaItem({ + type: 'object', + name: 'shared_ticket', + item: objectBody('shared_ticket', 'FROM_DEMO_ENV'), + packageId: 'app.demo', + mode: 'draft', + }); + } + + it('finds the draft the caller NAMED: publishing app.demo succeeds over app.other\'s same-org row', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + await seedCrossScopeDrafts(protocol); + + // The ADR-0048 coexistence precondition, asserted so a future change + // to the seeding cannot silently drain these cases' discriminating + // power: two draft rows, the foreign package's in the CALLER'S org, + // the named package's env-wide. + const drafts = draftRowsOf(rows); + expect(drafts.map((r) => [r.package_id, r.organization_id])).toEqual([ + ['app.other', 'org1'], + ['app.demo', null], + ]); + + // Pre-fix this REJECTED with `404 [no_draft]`: probe 1, package- + // agnostic, matched app.other's org1 row and answered `org1`; the + // package-exact promote then looked in org1 WITH + // `package_id = 'app.demo'` and found nothing — while app.demo's + // draft sat env-wide, publishable, and was the row the caller named. + const res = await protocol.publishMetaItem({ + type: 'object', + name: 'shared_ticket', + packageId: 'app.demo', + organizationId: 'org1', + }); + + expect(res).toMatchObject({ success: true }); + const active = activeRowsOf(rows); + expect(active).toHaveLength(1); + expect(labelOf(active[0])).toBe('FROM_DEMO_ENV'); + }); + + it('lands env-wide: the caller\'s org row belongs to another package, and the promotion never touches that partition', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + await seedCrossScopeDrafts(protocol); + await protocol.publishMetaItem({ + type: 'object', + name: 'shared_ticket', + packageId: 'app.demo', + organizationId: 'org1', + }); + + // The resolution's landing, pinned row-by-row: the probe fell through + // to env-wide BECAUSE the caller's own org row belongs to another + // package, so the active row is ENV-WIDE under the named package — + // not an org1 row minted from a partition holding nothing of + // app.demo's. + const active = activeRowsOf(rows); + expect(active).toHaveLength(1); + expect(active[0].organization_id).toBeNull(); + expect(active[0].package_id).toBe('app.demo'); + // …and app.other's org1 draft is untouched — pending, undrained, in + // its own partition. Pre-fix there was nothing to assert here: the + // door had already refused. + const drafts = draftRowsOf(rows); + expect(drafts).toHaveLength(1); + expect(drafts[0].package_id).toBe('app.other'); + expect(drafts[0].organization_id).toBe('org1'); + expect(labelOf(drafts[0])).toBe('FROM_OTHER_ORG1'); + }); + + it('still resolves the caller\'s own org when THAT is where the named package\'s draft lives (no overshoot)', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + // Mirrored arrangement: app.other env-wide (seeded FIRST, so any + // regression back toward package-agnostic env probing has a wrong row + // to find), app.demo in the caller's org. + await protocol.saveMetaItem({ + type: 'object', + name: 'shared_ticket', + item: objectBody('shared_ticket', 'FROM_OTHER_ENV'), + packageId: 'app.other', + mode: 'draft', + }); + await protocol.saveMetaItem({ + type: 'object', + name: 'shared_ticket', + item: objectBody('shared_ticket', 'FROM_DEMO_ORG1'), + packageId: 'app.demo', + organizationId: 'org1', + mode: 'draft', + }); + + const res = await protocol.publishMetaItem({ + type: 'object', + name: 'shared_ticket', + packageId: 'app.demo', + organizationId: 'org1', + }); + + // GREEN BEFORE THE FIX TOO, and stated so nobody reads a repro into + // it: pre-fix probe 1 happened to answer `org1` because the only org1 + // row WAS app.demo's. What this case bounds is the fix itself — the + // ADR-0005 precedence (own org shadows env-wide) must survive the + // package narrowing, so a "package-exact means env-first/env-only" + // mis-fix fails here loudly. + expect(res).toMatchObject({ success: true }); + const active = activeRowsOf(rows); + expect(active).toHaveLength(1); + expect(active[0].organization_id).toBe('org1'); + expect(active[0].package_id).toBe('app.demo'); + expect(labelOf(active[0])).toBe('FROM_DEMO_ORG1'); + // app.other's env-wide draft: pending, undrained. + const drafts = draftRowsOf(rows); + expect(drafts).toHaveLength(1); + expect(drafts[0].package_id).toBe('app.other'); + expect(drafts[0].organization_id).toBeNull(); + }); + + it('keeps the historical match-any probes when the caller states NO package (cross-scope fixture)', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + await seedCrossScopeDrafts(protocol); + + // No `packageId` key at all: the probes stay package-agnostic, the + // promote matches any package, and the ADR-0005 precedence picks the + // caller's own org row — app.other's, whatever package it belongs to. + // This is the same absent-key contract the #10350 case above pins + // env-wide, exercised HERE because these probes only run for an + // org-scoped caller (`requestOrgId === null` returns early). + const res = await protocol.publishMetaItem({ + type: 'object', + name: 'shared_ticket', + organizationId: 'org1', + }); + + expect(res).toMatchObject({ success: true }); + const active = activeRowsOf(rows); + expect(active).toHaveLength(1); + expect(active[0].organization_id).toBe('org1'); + expect(active[0].package_id).toBe('app.other'); + expect(labelOf(active[0])).toBe('FROM_OTHER_ORG1'); + }); + + it('accepted cost (the ruling, not a side effect): a package-stating caller 404s over a package-less draft, and retrying without ?package= publishes it', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + // Seeded by DIRECT insert — see the describe header for why this row + // must not come from `saveMetaItem(mode:'draft')` while PR #11139 is + // changing that path's binding resolution. The shape mirrors what the + // repository's `put` writes for a package-less org draft. + await engine.insert('sys_metadata', { + type: 'object', + name: 'shared_ticket', + organization_id: 'org1', + package_id: null, + state: 'draft', + metadata: JSON.stringify(objectBody('shared_ticket', 'NO_PACKAGE')), + version: 1, + created_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + }); + + // Half 1 — the narrowing: the caller stated `app.demo`, so neither + // probe nor promote may discover the unbound row. ADR-0112 envelope, + // not a bare `toThrow`. (No pre-fix red here, stated plainly: the + // package-exact PROMOTE already answered `no_draft` for this + // arrangement; what this pins is that the ruling's cost sentence + // holds end-to-end and stays held.) + await expect( + protocol.publishMetaItem({ + type: 'object', + name: 'shared_ticket', + packageId: 'app.demo', + organizationId: 'org1', + }), + ).rejects.toMatchObject({ code: 'NO_DRAFT', status: 404 }); + // …and the refusal touched nothing: the package-less draft is intact. + expect(draftRowsOf(rows)).toHaveLength(1); + + // Half 2 — the documented remedy: retry WITHOUT `?package=`. The + // absent key restores the match-any resolution and the unbound draft + // publishes. + const res = await protocol.publishMetaItem({ + type: 'object', + name: 'shared_ticket', + organizationId: 'org1', + }); + expect(res).toMatchObject({ success: true }); + const active = activeRowsOf(rows); + expect(active).toHaveLength(1); + expect(active[0].package_id).toBeNull(); + expect(active[0].organization_id).toBe('org1'); + expect(labelOf(active[0])).toBe('NO_PACKAGE'); + expect(draftRowsOf(rows)).toHaveLength(0); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 9b8f03c258..62585dd3ab 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -4603,6 +4603,28 @@ export class ObjectStackProtocolImplementation implements * scope is returned unchanged, so a genuinely absent draft still raises the * same `NO_DRAFT` refusal, from the scope the caller asked about. * + * [#11003] `packageId` — the ADR-0048 package dimension, threaded into BOTH + * probes exactly as {@link promoteDraftForPublish} threads it into + * `repo.promoteDraft`: stated (string, or `null` pinning the unbound row), + * each probe adds `package_id` to its `where`, so the scope probes ask the + * promote's question and the ADR-0005 precedence above is applied WITHIN + * the stated package's rows. `undefined` (caller stated no package) keeps + * the historical package-agnostic probes — the promote is then + * package-agnostic too, so the two questions still agree. + * + * Maintainer ruling 2026-08-22 (#11003, option A — recorded on the issue): + * a package-stating publish resolves the scope of the draft it NAMED. + * Without the dimension, probe 1 could match ANOTHER package's row in the + * caller's org, name a scope the package-exact promote then finds empty, + * and answer `404 [no_draft]` over a publishable draft sitting env-wide. + * Accepted cost, on the record: a caller stating a package no longer + * discovers a no-package draft of the same `(type, name)` — it 404s and the + * caller retries without `?package=`; that narrowing is the ruling, not a + * side effect. A package-first probe FALLING BACK to package-agnostic was + * rejected by name (it reintroduces the two-question resolution #8907 + * removed, and a mistyped package would silently publish another package's + * draft instead of failing loudly). + * * ⛔ This is discovery, not a tolerant fallback: it names the one row the * promote will then address, and it reads DRAFT rows in `sys_metadata` (the * thing being promoted) rather than the history lineage @@ -4617,31 +4639,32 @@ export class ObjectStackProtocolImplementation implements singularType: string, name: string, requestOrgId: string | null, + packageId?: string | null, ): Promise { if (requestOrgId === null) return null; - // The package dimension is absent from both probes, and the rule - // behind that is the LIVE one: these reads must ask the same question - // `promoteDraft` will (see `SysMetadataRepository.whereFor`), because - // their whole job is to name the scope the promote then addresses. A - // probe NARROWER than the promote hides a draft the promote can see; a - // probe WIDER names a scope it cannot. + // These reads must ask the same question `promoteDraft` will (see + // `SysMetadataRepository.whereFor`), because their whole job is to name + // the scope the promote then addresses. A probe NARROWER than the + // promote hides a draft the promote can see; a probe WIDER names a + // scope it cannot. // - // [#10350] ⚠️ The justification this comment used to carry — "the - // per-item door names no package" — is STALE. Since #10063 - // `publishMetaItem` accepts a `packageId` and forwards it, so when the - // caller states one the promote IS package-scoped while these probes - // stay package-agnostic, and the two can then ask different questions. - // Behaviour is deliberately UNCHANGED here: closing that asymmetry is - // its own fix with its own fixture and its own precedence ruling - // (ADR-0005 overlay order vs the ADR-0048 package key), filed as #11003 - // rather than ridden in on a comment repair. What is corrected is the claim, so the next - // reader does not conclude the per-item door still cannot name one. + // [#11003] That rule is what threads the package dimension in: since + // #10063 the per-item door names a package whenever its HTTP caller + // does (`?package=PKG_ID`), and the promote's `whereFor` then + // constrains `package_id` — so a package-agnostic probe here was the + // WIDER shape, naming a scope off another package's row (ADR-0048 keys + // overlay rows by `(org, type, name, package_id)`, so two packages' + // same-name drafts coexist in different scopes). `undefined` spreads + // NOTHING — the caller stated no package, the promote matches any + // package, and these probes keep asking that same question. See the + // docblock above for the #11003 ruling and its accepted narrowing. + const packageDim = packageId !== undefined ? { package_id: packageId } : {}; const inOrg = await this.engine.findOne('sys_metadata', { - where: { organization_id: requestOrgId, type: singularType, name, state: 'draft' }, + where: { organization_id: requestOrgId, type: singularType, name, state: 'draft', ...packageDim }, }); if (inOrg) return requestOrgId; const inEnv = await this.engine.findOne('sys_metadata', { - where: { organization_id: null, type: singularType, name, state: 'draft' }, + where: { organization_id: null, type: singularType, name, state: 'draft', ...packageDim }, }); return inEnv ? null : requestOrgId; } @@ -14649,10 +14672,21 @@ export class ObjectStackProtocolImplementation implements // #6190 org-scoped-write refusal and the promote all judge ONE scope — // the one the row is actually in. Resolving it later would gate against // a partition the promotion never touches. + // + // [#11003] The package dimension rides along under the SAME + // present/absent contract `promoteDraftForPublish` spells as + // `...('packageId' in request ? { packageId: request.packageId ?? null } + // : {})`: an ABSENT key keeps the historical package-agnostic probes, + // a stated one (string, or `null` for the unbound row) makes both + // probes ask the promote's package-exact question. Passing + // `request.packageId` bare would collapse "absent" and + // "present-and-undefined" into one spelling — the coercion trap the + // request type's own TSDoc warns against. { const singular = PLURAL_TO_SINGULAR[request.type] ?? request.type; const resolvedOrgId = await this.resolveDraftOrgScopeForPublish( singular, request.name, request.organizationId ?? null, + 'packageId' in request ? (request.packageId ?? null) : undefined, ); if (resolvedOrgId !== (request.organizationId ?? null)) { const { organizationId: _requested, ...rest } = request; From 0e7499df5240d6217589060265b5c4466023f997 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 21:32:12 +0000 Subject: [PATCH 2/2] test(metadata-protocol): stamp checksum on the direct-seeded package-less draft row The post-promotion drain is an optimistic-lock delete keyed on the row's checksum; a checksum-less hand-built row makes the drain classify the mismatch as the benign newer-draft race and survive, failing the remedy half of the accepted-cost pin. Stamp hashSpec(body) exactly as the repository's put does. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --- .../protocol-publish-drafts-package-scope.test.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts index a699e6c737..f00660d061 100644 --- a/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts +++ b/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts @@ -6,7 +6,7 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; // refuses. Imported from `@objectstack/metadata-core` and not from // `@objectstack/objectql`: objectql DEPENDS ON this package, so that import // would close a dependency cycle turbo rejects outright. -import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, hashSpec } from '@objectstack/metadata-core'; import { ObjectStackProtocolImplementation } from './protocol.js'; /** @@ -619,14 +619,20 @@ describe('publishMetaItem — the scope probes ask the promote\'s question (#110 // Seeded by DIRECT insert — see the describe header for why this row // must not come from `saveMetaItem(mode:'draft')` while PR #11139 is // changing that path's binding resolution. The shape mirrors what the - // repository's `put` writes for a package-less org draft. + // repository's `put` writes for a package-less org draft — `checksum` + // included: the post-promotion drain is an optimistic-lock delete + // keyed on it, and a checksum-less row makes the drain read as the + // benign "newer draft saved" race and survive (measured on this + // fixture's first run). + const noPackageBody = objectBody('shared_ticket', 'NO_PACKAGE'); await engine.insert('sys_metadata', { type: 'object', name: 'shared_ticket', organization_id: 'org1', package_id: null, state: 'draft', - metadata: JSON.stringify(objectBody('shared_ticket', 'NO_PACKAGE')), + metadata: JSON.stringify(noPackageBody), + checksum: hashSpec(noPackageBody), version: 1, created_at: new Date().toISOString(), updated_at: new Date().toISOString(),