diff --git a/.changeset/seed-read-drops-dead-org-rung.md b/.changeset/seed-read-drops-dead-org-rung.md new file mode 100644 index 0000000000..7c249b06fe --- /dev/null +++ b/.changeset/seed-read-drops-dead-org-rung.md @@ -0,0 +1,13 @@ +--- +"@objectstack/runtime": patch +--- + +The package-publish seed read-back no longer runs a two-attempt org-then-env ladder whose rungs resolve the same row. + +`applyPublishedSeeds` — the route-level seed apply behind `POST /packages/:id/publish-drafts`, which runs for protocols that do not self-apply seeds inside `publishPackageDrafts` — read each just-published `seed` body twice when the session had an active organization: once naming the organization, then once env-wide. The comment above it said the first attempt tried the active org and the second fell back, "and resolving the wrong scope here is what silently produced `0 rows loaded`". + +That was true when it was written and is not true now. `seed` declares `allowOrgOverride: false`, and `getMetaItem` resolves `organizationIdForMetaRead(request.type, request.organizationId)` once at its top and spends that binding — never the raw argument — on every read beneath it. The predicate answers `undefined` for every non-overridable type, so both rungs asked the engine the same predicates and served the same answer. Measured rather than reasoned: against the shipping protocol over one store, the two requests produce byte-identical engine reads and byte-identical answers on both the hit and the miss branch, and neutering the second rung reddens nothing on a pinned publish-then-read path (a `view` control confirms the same comparison does separate the two rungs for an org-overridable type). + +The read is now a single call naming no organization, and the comment states that the scope is decided by the registry flag and the gate inside `getMetaItem` rather than by this call site — matching the sentence the `app` flip in the same file already carries. + +One observable changes, and only on the failure branch: `getMetaItem` answers a wrapper rather than a falsy value for a name it cannot resolve, so the second rung was in practice reached only when the read *threw* — where it repeated the identical failing read and appended the same sentence to the client-facing `seedApplied.errors[]` twice. A failed read-back is now reported once. Nothing about which row a publish resolves, or whether its rows load, moves. diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 6e66fe99b5..9814a1e775 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -7550,7 +7550,7 @@ export class ObjectStackProtocolImplementation implements // hydration walks past. // // ⇒ What is left to move is the runtime callers that hand this method a - // RAW active organization. FOUR, across two files — the population is + // RAW active organization. THREE, all in one file — the population is // stated with the method that establishes it, because the first // enumeration of it named only the first file and was wrong: grep every // `getMetaItem(` / `getMetaItemCached(` invocation in the repo, then @@ -7560,12 +7560,14 @@ export class ObjectStackProtocolImplementation implements // be right about scope, by construction. // • `runtime/src/domains/meta.ts:768` — `singularType` off the URL, so // it moves only for the non-overridable half of what it serves. - // • `runtime/src/domains/packages.ts:1239` (`applyPublishedSeeds`, - // organization from `deps.resolveActiveOrganizationId`) — `type: - // 'seed'`, also non-overridable, so its org-first attempt now reads - // the env-wide partition directly. It hand-rolls the same fallback - // as a second attempt, so what it used to reach on the second try it - // now gets on the first. + // A FOURTH used to sit here: `applyPublishedSeeds` in + // `runtime/src/domains/packages.ts`, `type: 'seed'`, equally + // non-overridable. It hand-rolled an org-then-env ladder that this gate + // had turned into a byte-identical repeat — both rungs asking the + // engine the same predicates and serving the same answer. #15068 + // measured that (ablation: neutering the second rung reddened nothing + // on a pinned publish-then-read path) and collapsed it to a single read + // naming no organization at all, so it now belongs to the bucket below. // Every other invocation either names no organization at all or is a // REST door that already computed `organizationIdForMetaRead` — the // idempotence legs above are what make those two cases no-ops. diff --git a/packages/runtime/src/domains/packages-seed-apply-org-scope.test.ts b/packages/runtime/src/domains/packages-seed-apply-org-scope.test.ts new file mode 100644 index 0000000000..d50ef72dc3 --- /dev/null +++ b/packages/runtime/src/domains/packages-seed-apply-org-scope.test.ts @@ -0,0 +1,538 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15068 — the publish-then-read path under `applyPublishedSeeds`, and the + * proof that its org-then-env ladder cannot choose between its two rungs. + * + * ## What the ladder was, and why deleting it needed a measurement + * + * `POST /packages/:id/publish-drafts` reads each just-published `seed` body + * back before handing it to `SeedLoaderService`. That read ran twice: + * + * ``` + * const attempts = organizationId + * ? [{ type: 'seed', name, organizationId }, { type: 'seed', name }] + * : [{ type: 'seed', name }]; + * ``` + * + * The comment above it said "try the active org first, then fall back to an + * env-wide read … resolving the wrong scope here is what silently produced + * `0 rows loaded`". So the ladder was written for a real outage, and ⛔ a + * registry reading alone was not licence to delete it. The deletion is + * licensed by §1 below instead: the two rungs issue the SAME query. + * + * ## The mechanism, in one line + * + * `getMetaItem` opens with `organizationIdForMetaRead(request.type, + * request.organizationId)` (#14908, the singular twin of #14683's plural + * gate) and spends that binding — never `request.organizationId` — on every + * read below it. `seed` declares `allowOrgOverride: false` + * (`metadata-plugin.zod.ts`), so the predicate answers `undefined` whatever + * organization arrives. ⇒ `{ type:'seed', name, organizationId }` and + * `{ type:'seed', name }` differ in a field the callee provably drops. + * + * ⛔ The repair is NOT to restore org-awareness to this read. An org-scoped + * `seed` row is the unhydratable phantom `reportUnhydratableOrgScopedRows` + * exists to warn about — the same argument the `app` flip one function up + * carries since #15063. + * + * ## How this file is composed, and which half is doubled + * + * The PUBLISH is real: §-fixture stages a `state:'draft'` seed row and + * promotes it with the shipping `publishPackageDrafts`, over the shipping + * `ObjectStackProtocolImplementation`. The READ-BACK is real: the same + * protocol instance serves `getMetaItem`, over the same engine, reading the + * row that publish just wrote. The loader is the real `SeedLoaderService`. + * + * ONE thing is doubled, and only to reach the code under test at all: the + * route-level apply runs *only* for protocols that do not self-apply seeds + * inside `publishPackageDrafts` ("never run both, or an externalId-less seed + * would double-insert"). So the second call presents a `publishPackageDrafts` + * that reports the published seed without a `seedApplied` field — the exact + * population this fallback documents itself as existing for. It also records + * the request it received, which is §0's positive control that the session's + * organization really reached this request. + * + * ## Sections, and which are evidence vs. which are the bound + * + * §0 · positive control — the ladder is REACHED and the read resolves the row + * the publish just wrote. Without it "nothing reddened" is + * indistinguishable from "nothing ran". + * §1 · the identity — both rungs, run against one store, ask the engine the + * same predicates and serve the same answer, with a control proving the + * comparison CAN separate them (`view`). GREEN before and after the + * collapse: this section is the ablation's instrument, not its result. + * §2 · the collapse — the read verb is no longer handed an organization the + * gate drops, and one failing read is one read. RED before the fix. + * §3 · the payload — a failed read-back is reported ONCE, not twice. RED + * before the fix, and the one observable the deletion changes. + * + * ⭐ Measured here and NOT claimed by the card: `getMetaItem` answers a + * wrapper with no `item` rather than a falsy value for a name it cannot + * resolve, so `if (item) break` fires on the first attempt even on a MISS + * (§1 [MECHANISM]). The second rung therefore only ever executed on the THROW + * branch — where it repeated the identical failing read and appended the same + * sentence to a client-facing payload twice (§3). + */ + +import { describe, expect, it, vi } from 'vitest'; +import { + assertEngineDeleteDispatch, + assertEngineFindOnePredicate, + assertEngineUpdateDispatch, + organizationIdForMetaRead, +} from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { DEFAULT_METADATA_TYPE_REGISTRY } from '@objectstack/spec/kernel'; +import { HttpDispatcher } from '../http-dispatcher.js'; + +const ORG = 'org_acme'; +const PKG = 'com.workspace'; +const SEED = 'project_seed'; + +/** The seed body a publish stores and the read-back must return. */ +const SEED_BODY = { + object: 'project', + externalId: 'name', + mode: 'upsert', + records: [{ name: 'Apollo', status: 'active' }, { name: 'Gemini', status: 'planned' }], +}; + +// --------------------------------------------------------------------------- +// Engine double — a row store that RECORDS every `sys_metadata` predicate. +// --------------------------------------------------------------------------- + +interface Row { id: string; [k: string]: unknown } + +function matches(row: Row, where: Record | undefined): boolean { + if (!where) return true; + for (const [key, cond] of Object.entries(where)) { + if (cond === undefined) continue; + if (key === '$or') { + const branches = cond as Array>; + if (!branches.some((b) => matches(row, b))) return false; + continue; + } + const value = row[key]; + if (cond !== null && typeof cond === 'object') { + const op = cond as Record; + if ('$null' in op) { + if ((value === null || value === undefined) !== (op.$null === true)) return false; + continue; + } + if ('$in' in op) { + if (!(op.$in as unknown[]).includes(value)) return false; + continue; + } + continue; + } + if (cond === null) { + if (value !== null && value !== undefined) return false; + continue; + } + if (value !== cond) return false; + } + return true; +} + +/** + * ⛔ Every write verb opens with the PRODUCER's own dispatch predicate + * (`check:engine-double-contract`) so this double cannot accept a call the + * real ObjectQL engine would refuse — imported from `@objectstack/metadata-core`, + * never from `@objectstack/objectql` (that reverse edge is a cycle turbo refuses). + */ +function makeEngine() { + const tables = new Map(); + /** Every `sys_metadata` WHERE the read path issued — the observation channel. */ + const metaReads: Array> = []; + let nextId = 0; + const tableOf = (name: string): Row[] => { + let t = tables.get(name); + if (!t) { t = []; tables.set(name, t); } + return t; + }; + const engine: any = { + registry: { + listItems: () => [], + getItem: () => undefined, + getObject: () => undefined, + getPackage: () => undefined, + getArtifactItem: () => undefined, + getAllPackages: () => [], + isPackageDisabled: () => false, + applyNavContributions: (app: unknown) => app, + registerItem: () => undefined, + registerObject: () => undefined, + }, + async find(table: string, opts?: { where?: Record, limit?: number }) { + if (table === 'sys_metadata') metaReads.push({ ...(opts?.where ?? {}) }); + const rows = tableOf(table).filter((r) => matches(r, opts?.where)); + // The caller's bound, applied AFTER the filter and by PRESENCE + // (`check:objectql-double-limit`): a double that silently ignores + // `limit` answers more rows than the real engine would, and a pin + // written against a paged read would pass on rows it never sees. + return typeof opts?.limit === 'number' ? rows.slice(0, opts.limit) : rows; + }, + async findOne(table: string, opts?: { where?: Record }) { + assertEngineFindOnePredicate(table, opts); + if (table === 'sys_metadata') metaReads.push({ ...(opts?.where ?? {}) }); + return tableOf(table).find((r) => matches(r, opts?.where)) ?? null; + }, + async insert(table: string, data: any) { + const one = (d: Record): Row => { + nextId += 1; + const row: Row = { id: (d.id as string) ?? `r_${nextId}`, ...d }; + tableOf(table).push(row); + return row; + }; + return Array.isArray(data) ? data.map(one) : one(data); + }, + async update(table: string, data: Record, opts?: { where?: Record }) { + const dispatch = assertEngineUpdateDispatch(data as any, opts as any); + const rows = tableOf(table); + const target = dispatch.kind === 'by-id' + ? rows.find((r) => r.id === dispatch.id) + : rows.find((r) => matches(r, opts?.where)); + if (target) Object.assign(target, data); + return target ?? null; + }, + async delete(table: string, opts?: { where?: Record }) { + const dispatch = assertEngineDeleteDispatch(opts as any); + const rows = tableOf(table); + const keep = dispatch.kind === 'by-id' + ? rows.filter((r) => r.id !== dispatch.id) + : rows.filter((r) => !matches(r, opts?.where)); + const deleted = rows.length - keep.length; + tables.set(table, keep); + return { deleted }; + }, + async count(table: string, opts?: { where?: Record }) { + return tableOf(table).filter((r) => matches(r, opts?.where)).length; + }, + async aggregate() { return []; }, + async execute() { return undefined; }, + rowsOf: tableOf, + metaReads, + }; + return engine; +} + +/** An authenticated package admin — the route's anonymous-deny + capability floor. */ +const PKG_ADMIN = (): any => ({ + request: { headers: {} }, + environmentId: 'env_1', + executionContext: { + userId: 'u_pkg_admin', + systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], + }, +}); + +interface DriveOptions { + /** Session's active organization; `undefined` drives the one-rung branch. */ + activeOrganizationId?: string; + /** Injection: the read-back throws this instead of answering. */ + readBackError?: () => Error; +} + +/** + * Stage a draft seed, promote it with the REAL `publishPackageDrafts`, then + * drive the route whose fallback reads it back. + */ +async function publishThenRead(opts: DriveOptions = {}) { + const engine = makeEngine(); + engine.rowsOf('sys_metadata').push({ + id: 'row_seed_draft', + type: 'seed', + name: SEED, + organization_id: null, + package_id: PKG, + state: 'draft', + metadata: JSON.stringify(SEED_BODY), + }); + + const real = new ObjectStackProtocolImplementation(engine, () => new Map()) as any; + + // ── The real publish. Draft → active, through the shipping primitive. ── + const published = await real.publishPackageDrafts({ packageId: PKG }); + + // Anything the publish itself loaded is not what this file measures; the + // read-back below must stand on its own. + engine.rowsOf('project').length = 0; + engine.metaReads.length = 0; + + /** Every request the read-back verb received, in order. */ + const readBackArgs: Array> = []; + const getMetaItem = vi.fn(async (request: any) => { + readBackArgs.push({ ...request }); + if (opts.readBackError) throw opts.readBackError(); + return await real.getMetaItem(request); + }); + + /** What `publishPackageDrafts` was asked for — §0's organization control. */ + let publishRequest: any; + const facade = { + // ⛔ Deliberately reports NO `seedApplied`: the route-level apply runs + // only for protocols that do not self-apply, and that branch is the + // code under test. + publishPackageDrafts: async (request: any) => { + publishRequest = request; + return { + success: true, + outcome: 'published', + publishedCount: 1, + failedCount: 0, + published: [{ type: 'seed', name: SEED, version: 'h' }], + failed: [], + }; + }, + getMetaItem, + }; + + const services: Record = { + protocol: facade, + objectql: engine, + metadata: { + getObject: async () => ({ + name: 'project', + fields: { name: { type: 'text' }, status: { type: 'select' } }, + }), + }, + auth: { + api: { + getSession: async () => (opts.activeOrganizationId + ? { session: { activeOrganizationId: opts.activeOrganizationId } } + : { session: {} }), + }, + }, + }; + const kernel: any = { + getServiceAsync: async (name: string) => services[name] ?? null, + getService: (name: string) => services[name] ?? null, + context: { getService: (name: string) => services[name] ?? null }, + }; + + const result = await new HttpDispatcher(kernel).handlePackages( + `/${PKG}/publish-drafts`, 'POST', {}, {}, PKG_ADMIN(), + ); + expect(result.response?.status).toBe(200); + const body: any = (result.response as any)?.body; + return { + engine, + published, + publishRequest: () => publishRequest, + readBackArgs, + getMetaItem, + body, + seedApplied: body?.data?.seedApplied, + }; +} + +/** + * A self-correcting refusal of the shape `SysMetadataRepository` raises. It + * DECLARED itself 4xx (ADR-0112), which is what makes its sentence quotable to + * the author at all — the bound `packages-seed-apply-disclosure.test.ts` owns. + */ +const DECLARED_REFUSAL = () => { + const e: any = new Error(`[item_locked] seed "${SEED}" is locked by another publish`); + e.code = 'ITEM_LOCKED'; + e.status = 403; + return e; +}; + +/** A store holding the published seed row, env-wide, exactly as publish leaves it. */ +function seededStore() { + const engine = makeEngine(); + engine.rowsOf('sys_metadata').push({ + id: 'row_seed_active', + type: 'seed', + name: SEED, + organization_id: null, + package_id: PKG, + state: 'active', + metadata: JSON.stringify(SEED_BODY), + }); + return engine; +} + +/** The `sys_metadata` predicates issued for `type:'seed'`, in order. */ +const seedReads = (engine: any): Array> => + engine.metaReads.filter((w: any) => w.type === 'seed'); + +// ═══════════════════════════════════════════════════════════════════════════ +// §0 — the positive control: the ladder is REACHED, on a path that works +// ═══════════════════════════════════════════════════════════════════════════ + +describe('#15068 · 0 · the publish-then-read path really runs', () => { + it('promotes the draft and reads the just-published body back out of the store', async () => { + const { published, engine, getMetaItem } = await publishThenRead({ + activeOrganizationId: ORG, + }); + + // The publish is the shipping one, and it really promoted the draft. + expect(published?.publishedCount).toBe(1); + expect(engine.rowsOf('sys_metadata').some( + (r: any) => r.type === 'seed' && r.name === SEED && r.state === 'active', + )).toBe(true); + + // The read-back reached `sys_metadata` — not a registry cache, not a + // double. Without this, §1's "every partition was env-wide" could be + // satisfied by zero partitions. + expect(getMetaItem).toHaveBeenCalled(); + expect(seedReads(engine).length).toBeGreaterThan(0); + + // And it resolved the row the publish just wrote. `0 rows loaded` is + // the outage the ladder was written for, and THIS is the layer that + // outage lives at: the body coming back, from the right partition. + const served: any = await getMetaItem.mock.results[0]?.value; + expect(served?.item?.object).toBe('project'); + expect(served?.item?.records).toHaveLength(2); + }); + + it('the session organization really reaches this request', async () => { + const { publishRequest } = await publishThenRead({ activeOrganizationId: ORG }); + + // `applyPublishedSeeds` receives the SAME binding this route handed + // `publishPackageDrafts` — one `resolveActiveOrganizationId` call + // serves both. So an org here is what put the ladder on its two-rung + // branch: without this control every measurement below could be of the + // one-rung branch and would prove nothing. + expect(publishRequest()?.organizationId).toBe(ORG); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// §1 — the identity. GREEN before AND after the collapse: this section is the +// ablation's instrument, not its result. +// ═══════════════════════════════════════════════════════════════════════════ + +describe('#15068 · 1 · the two rungs resolve to one read', () => { + it('`seed` is non-overridable, so the read gate drops the organization', () => { + // The registry fact the card rests on, read from the registry rather + // than restated. + expect(DEFAULT_METADATA_TYPE_REGISTRY.find((e) => e.type === 'seed')?.allowOrgOverride) + .toBe(false); + expect(organizationIdForMetaRead('seed', ORG)).toBeUndefined(); + // ⇒ and the control that makes that reading mean something: the same + // predicate DOES carry an organization for an overridable type. + expect(organizationIdForMetaRead('view', ORG)).toBe(ORG); + }); + + it('the two rungs issue the same predicates and serve the same answer', async () => { + // The measurement the deletion rests on, taken at the verb itself: + // run BOTH rungs against one store and compare what the engine was + // asked and what came back. Done for the HIT and the MISS, the only + // two branches the loop distinguishes. + for (const name of [SEED, 'no_such_seed']) { + const engine = seededStore(); + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any; + + const orgFirst = await protocol.getMetaItem({ type: 'seed', name, organizationId: ORG }); + const orgFirstReads = engine.metaReads.splice(0); + const envWide = await protocol.getMetaItem({ type: 'seed', name }); + const envWideReads = engine.metaReads.splice(0); + + expect(orgFirstReads.length, name).toBeGreaterThan(0); + expect(JSON.stringify(orgFirstReads), name).toBe(JSON.stringify(envWideReads)); + expect(JSON.stringify(orgFirst), name).toBe(JSON.stringify(envWide)); + } + }); + + it('[CONTROL] the same comparison DOES separate the two rungs for an overridable type', async () => { + // Anti-vacuity, and the reason the assertion above is a reading rather + // than a tautology: on `view` — `allowOrgOverride: true` — the org-first + // rung reads a partition the env-wide rung never touches. + const engine = seededStore(); + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any; + + await protocol.getMetaItem({ type: 'view', name: 'anything', organizationId: ORG }); + const orgFirstReads = engine.metaReads.splice(0); + await protocol.getMetaItem({ type: 'view', name: 'anything' }); + const envWideReads = engine.metaReads.splice(0); + + expect(JSON.stringify(orgFirstReads)).not.toBe(JSON.stringify(envWideReads)); + expect(orgFirstReads.map((w: any) => w.organization_id)).toContain(ORG); + }); + + it('every partition the publish path touches for a seed is env-wide', async () => { + const { engine } = await publishThenRead({ activeOrganizationId: ORG }); + + expect([...new Set(seedReads(engine).map((w) => w.organization_id ?? null))]) + .toEqual([null]); + }); + + it('[MECHANISM] a read that resolves nothing still answers a wrapper, so rung 2 is not even reached', async () => { + // Measured, and it is why the ladder is deader than the card claims: + // `getMetaItem` answers an envelope (`{ type, name, lock, editable, … }`) + // with no `item` rather than a falsy value, so `if (item) break` fires + // on the FIRST attempt even for a name nothing resolves. The only + // branch on which the second attempt ever executed is the THROW branch + // — where it repeats the identical failing read (§3). + // + // GREEN before and after the collapse. It is a bound on what the + // deletion can possibly have changed, not evidence that it changed it. + const engine = seededStore(); + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any; + const miss = await protocol.getMetaItem({ type: 'seed', name: 'no_such_seed' }); + + expect(miss).toBeTruthy(); + expect(miss.item).toBeUndefined(); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// §2 — the collapse. RED before the fix, GREEN after. +// ═══════════════════════════════════════════════════════════════════════════ + +describe('#15068 · 2 · the publish path stops spending an organization the gate drops', () => { + it('hands the read verb exactly the request the gate will act on', async () => { + const { readBackArgs } = await publishThenRead({ activeOrganizationId: ORG }); + + // Not cosmetic: an `organizationId` on a non-overridable read is the + // shape #14908 and #15063 exist to stop anyone reading as meaningful. + expect(readBackArgs).toEqual([{ type: 'seed', name: SEED }]); + }); + + it('issues one read-back per seed even when the read fails', async () => { + const withOrg = await publishThenRead({ + activeOrganizationId: ORG, readBackError: DECLARED_REFUSAL, + }); + const withoutOrg = await publishThenRead({ readBackError: DECLARED_REFUSAL }); + + // The throw branch is the one place the second rung ever ran. One + // failing read, reported once — and the same count with or without an + // active organization, which is the whole content of "the rung was + // dead". + expect(withOrg.readBackArgs).toHaveLength(1); + expect(withOrg.readBackArgs.length).toBe(withoutOrg.readBackArgs.length); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// §3 — the payload. The one observable the deletion changes. +// ═══════════════════════════════════════════════════════════════════════════ + +describe('#15068 · 3 · a failed read-back is reported once, not twice', () => { + it('quotes a declared 4xx refusal exactly once with an org active', async () => { + const refusal = DECLARED_REFUSAL(); + // ADR-0112 — the declaration that makes the sentence quotable to the + // author at all, asserted on `code` AND `status`. ⛔ Never a bare + // `toThrow()`: this door does not throw, it REPORTS, and the whole + // assertion is about what the report says. + expect(refusal.code).toBe('ITEM_LOCKED'); + expect(refusal.status).toBe(403); + + const { seedApplied } = await publishThenRead({ + activeOrganizationId: ORG, readBackError: DECLARED_REFUSAL, + }); + + // `seedApplied.errors[]` rides on a 200 as DATA. Running the same + // failed read twice put the same sentence on it twice, and an author + // reading two identical lines has no way to tell that from two + // distinct failures. + expect(seedApplied?.errors?.filter( + (e: unknown) => String(e) === `read ${SEED}: ${refusal.message}`, + )).toHaveLength(1); + expect(seedApplied?.success).toBe(false); + expect(seedApplied?.error).toBe('seed apply: no readable seed bodies'); + }); +}); diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index 1c0349a734..39672a7e89 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -1345,36 +1345,49 @@ _context: HttpProtocolContext, const datasets: any[] = []; const readErrors: string[] = []; for (const name of names) { - // Read the just-published seed body. Try the active org first, then - // fall back to an env-wide read — a workspace seed is often stored - // org-wide (organization_id IS NULL), and resolving the wrong scope - // here is what silently produced "0 rows loaded". - const attempts = organizationId - ? [{ type: 'seed', name, organizationId }, { type: 'seed', name }] - : [{ type: 'seed', name }]; + // Read the just-published seed body. THE REGISTRY DECIDES THE SCOPE, + // not this call site: `seed` declares `allowOrgOverride: false`, and + // since #14908 `getMetaItem` opens by resolving + // `organizationIdForMetaRead(request.type, request.organizationId)` + // and spends THAT binding — never the raw argument — on every read + // beneath it. The predicate answers `undefined` for every type the + // registry declares non-overridable, so this read is env-wide by + // construction: `organization_id IS NULL`, the partition a workspace + // seed is stored in and the only one cold boot hydrates. + // + // [#15068] This used to be a two-attempt org-then-env ladder, written + // when resolving the wrong scope here is what silently produced "0 + // rows loaded". The gate is that fix now, and it made the org-first + // rung a byte-identical repeat: both attempts resolved the same + // partition and served the same answer, so the only thing the second + // one could still do was report an identical failed read twice on a + // client-facing `seedApplied.errors[]`. + // + // ⛔ Do NOT restore an org-first attempt. An org-scoped `seed` row is + // the unhydratable phantom `reportUnhydratableOrgScopedRows` exists to + // warn about — reading it back would serve a body that vanishes at the + // next restart. Dropping the organization is the REPAIR, exactly as on + // the `app` flip above. let item: any; - for (const args of attempts) { - try { - item = await protocol.getMetaItem(args); - if (item) break; - } catch (e) { - // [#8443] The SAME rule as the catch at the door, applied to - // the sibling key of the same field: `readErrors` becomes - // `seedApplied.errors[]` on that 200 response, so it is a - // client-facing payload too. Measured before the change: with - // `sys_metadata` unreachable this read fails FIRST — before the - // loader is ever constructed — and answered `"errors": ["read - // project_seed: SQLITE_ERROR: no such table: sys_metadata"]`, - // so fixing only the door's catch would have left the commonest - // outage shape disclosing exactly as before. A DECLARED 4xx - // refusal (`[item_locked]`, `[writable_package_required]`, …) - // still reaches the author verbatim — that is the point of the - // positive list. - (deps.logger ?? console).warn( - `[applyPublishedSeeds] seed body read failed for "${name}": ${(e as Error)?.message ?? String(e)}`, - ); - readErrors.push(`read ${name}: ${clientFacingFailureText(e, 'the reason is in the server log')}`); - } + try { + item = await protocol.getMetaItem({ type: 'seed', name }); + } catch (e) { + // [#8443] The SAME rule as the catch at the door, applied to the + // sibling key of the same field: `readErrors` becomes + // `seedApplied.errors[]` on that 200 response, so it is a + // client-facing payload too. Measured before the change: with + // `sys_metadata` unreachable this read fails FIRST — before the + // loader is ever constructed — and answered `"errors": ["read + // project_seed: SQLITE_ERROR: no such table: sys_metadata"]`, so + // fixing only the door's catch would have left the commonest + // outage shape disclosing exactly as before. A DECLARED 4xx + // refusal (`[item_locked]`, `[writable_package_required]`, …) + // still reaches the author verbatim — that is the point of the + // positive list. + (deps.logger ?? console).warn( + `[applyPublishedSeeds] seed body read failed for "${name}": ${(e as Error)?.message ?? String(e)}`, + ); + readErrors.push(`read ${name}: ${clientFacingFailureText(e, 'the reason is in the server log')}`); } // protocol.getMetaItem returns a WRAPPER: `{ type, name, item, lock, // editable, … }` — the seed body (object/records) lives under diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 996640222d..0314dcd082 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -3141,6 +3141,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/runtime/src/domains/packages-seed-apply-org-scope.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/runtime/src/domains/packages-seed-apply-org-scope.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/runtime/src/domains/packages-seed-apply-org-scope.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/runtime/src/domains/share-links-enforcement-context.test.ts", "verb": "delete",