diff --git a/.changeset/lifecycle-governance-tenant-scan-read-failure.md b/.changeset/lifecycle-governance-tenant-scan-read-failure.md new file mode 100644 index 0000000000..d5c4571737 --- /dev/null +++ b/.changeset/lifecycle-governance-tenant-scan-read-failure.md @@ -0,0 +1,42 @@ +--- +"@objectstack/objectql": patch +--- + +fix(objectql): a failed `sys_organization` tenant scan no longer decides a retention window (#12853) + +`LifecycleService.loadGovernance()` filled `snapshot.tenantOverrides` — the +ADR-0057 §3.2 per-tenant retention/expiry window set — behind a bare `catch {}` +whose comment named ONE benign cause ("No sys_organization (single-tenant +kernel)") while the `catch` swallowed every cause. On a connection drop, a +timeout, a permission refusal or a driver fault the map came back EMPTY, and an +empty map is not a neutral value: `reap()` and `archiveObject()` read it as +"this deployment has tuned no tenant" and fall every tenant back to the global +window. That window is wrong in both directions, and the expensive direction is +a tenant configured to retain LONGER having its rows expired early. Nothing +reported it: `GovernanceSnapshot` carries no field saying the tenant pass did not +complete, and the catch logged nothing — so the platform deleted on knowingly +incomplete evidence, without knowing the evidence was incomplete. + +The scan now discriminates by error TYPE through the shared +`isMissingTableError` predicate. An unprovisioned `sys_organization` really does +mean "no tenant overrides", so a single-tenant kernel is unchanged. Every other +cause aborts the sweep **before any policy is applied** — for a deletion action, +"do not act on incomplete evidence" is the correct failure direction, and a log +cannot bring back a reaped row. The rows a deferred sweep leaves are still there +for the next one. + +Operational posture change, deliberate and worth stating: a transient +`sys_organization` outage now costs a sweep. The abort is REPORTED, not thrown — +one `report.errors` entry per declared object plus a `warn` — because `sweep()`'s +declared contract is that it never throws and the scheduler enters it as +`void this.sweep()`, where a rejection would be unhandled. That is the same +objection #8906 recorded when it declined to rethrow from `checkGovernance` one +method below. + +Bump argued, not defaulted: `patch`. No exported signature, type, option or +report field moves — the failure surfaces through `LifecycleSweepReport.errors`, +which already exists for exactly this. The tension is honest and does not change +the answer: what a deployment observes on a failure path DOES change (a sweep +that used to complete silently now aborts and says so), but that is the +correction of a defect, not a new capability, and the sibling repairs in this +family (#8896, #8906, #9817) all shipped as `patch`. diff --git a/.changeset/seed-loader-sole-organization-read-failure.md b/.changeset/seed-loader-sole-organization-read-failure.md new file mode 100644 index 0000000000..e6a62932b6 --- /dev/null +++ b/.changeset/seed-loader-sole-organization-read-failure.md @@ -0,0 +1,30 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): a failed `sys_organization` probe is no longer answered as "no sole organization" (#12852) + +`SeedLoaderService.resolveSoleOrganizationId()` sat behind a bare `catch {}` whose +comment named ONE benign cause — "sys_organization may not exist (single-tenant +runtime)" — while the `catch` swallowed every cause. A dropped connection, a +timeout, a permission refusal or a driver fault all arrived at the caller as +`undefined`, which is not a neutral value here: it is the verdict the method's +own JSDoc calls "genuinely ambiguous", so `load()` stamped no `organization_id` +and every BUSINESS seed row of that run landed org-less — invisible afterwards +under strict org-scoping. Nothing reported it either: `SeedLoadResult` carries an +`errors` field and this path never touched it, so the operator saw a clean, +successful seed. + +The repair is the one already landed on the sibling probe across the engine +boundary (`ObjectQL.probeInstallOrganizations`, #9817), copied: bind the +parameter and ask the declared predicate. Only an unprovisioned TABLE is +truthful emptiness — the exact cause the swallowed comment already named — so +the JSDoc's "or when `sys_organization` is absent" stays true, while every other +cause now propagates with its envelope intact. + +Bump argued, not defaulted: `patch`. No exported signature, type or option +moves, and the declared answer for every case the JSDoc describes is unchanged. +What changes is a failure path — a seed run that used to complete while writing +invisible rows now fails loudly — which is the correction of a defect rather +than a new capability. The three landed repairs in this family (#8896, #8906, +#9817) all shipped as `patch`, and this is the site that pass missed. diff --git a/packages/metadata-protocol/src/seed-loader-sole-organization-read-failure.test.ts b/packages/metadata-protocol/src/seed-loader-sole-organization-read-failure.test.ts new file mode 100644 index 0000000000..cc61f5e291 --- /dev/null +++ b/packages/metadata-protocol/src/seed-loader-sole-organization-read-failure.test.ts @@ -0,0 +1,378 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#12852] `resolveSoleOrganizationId` must not answer a FAILED + * `sys_organization` read with `undefined`. + * + * `undefined` is not a neutral value at this seam. It is the verdict the + * method's own JSDoc calls "genuinely ambiguous" (zero or several orgs), so + * `load()` stamps no `organization_id` and every BUSINESS seed row of the run + * lands org-less — invisible afterwards under strict org-scoping. And nothing + * says so: `SeedLoadResult` has an `errors` field, and this path never touches + * it, so a transient outage mid-seed reads to the operator as a clean, + * successful seed. ADR-0110 D3's exact shape — "the read found no sole + * organization" and "the read could not run" are different facts. + * + * The sibling probe on the objectql side, `ObjectQL.probeInstallOrganizations`, + * had the SAME shape and was repaired by PR #9817 to bind the parameter and ask + * the declared predicate. This site was missed by that pass; the repair here is + * that repair, copied. + * + * Only an unprovisioned TABLE is truthful emptiness — precisely the cause the + * swallowed comment already named — so the JSDoc's "or when `sys_organization` + * is absent" stays true while every other cause stops being answered as an + * emptiness. + * + * Every expectation below is written against LITERALS — the exact injected + * error object, its literal message and code, literal row counts and literal + * stamped ids — never a value re-derived from the code under test. Each failure + * assertion is paired with a positive control in this same file (the probe + * SUCCEEDING with one org, with none, and — for the benign branch — proof the + * injected throw actually fired), so a harness that had stopped exercising the + * seam could not pass vacuously. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { IDataEngine, IMetadataService } from '@objectstack/spec/contracts'; +// [#5619] The producer's OWN write-verb dispatch decisions (#4550 delete / +// #5480 update), so the fake engine below cannot accept a call ObjectQL +// refuses. From `@objectstack/metadata-core`, not `@objectstack/objectql` — +// objectql depends on THIS package, so that import would close a dependency +// cycle turbo rejects outright. +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core'; +// `.js` extension deliberately: `moduleResolution: nodenext` requires it, and +// an extensionless specifier is exactly the TS2835 that makes up part of this +// package's frozen TEST_DEBT (#5278). That ledger is shrink-only, so a new file +// may not add to it. +import { SeedLoaderService } from './seed-loader.js'; + +interface StoreRow extends Record { + id: string; +} + +/** + * The fixture's WHERE matcher, flat equality only. + * + * A combinator is REFUSED rather than read as a field name: a matcher that + * treats `$or` as a column answers a combinator query with an empty result set + * and nothing erroring, which is the silently-wrong shape + * `pnpm check:where-matcher` exists to keep out. This fixture only ever + * receives flat equality (`organization_id`, `sku`). + */ +function matchesWhere(row: Record, where: Record): boolean { + return Object.entries(where).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`fake driver: unsupported operator ${k}`); + return row[k] === v; + }); +} + +function createLogger() { + return { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }; +} + +/** + * A faithful in-memory engine plus a PER-OBJECT read-failure injector. + * + * Per-object, deliberately: the defect is specific to the `sys_organization` + * probe, so the widget reads must keep succeeding. A blanket read failure would + * also fail the loader's existing-records pre-load (#8896's already-repaired + * seam), and every assertion below would then be satisfied by that repair + * instead of this one. + * + * `findCalls` records that a read really ran, which is what turns "the seed + * proceeded" into "the seed proceeded AND the injected throw fired". + */ +function createEngine() { + const store: Record = {}; + const findCalls: string[] = []; + const failFor: Record = {}; + let idCounter = 0; + + const engine = { + find: vi.fn(async (objectName: string, query?: { where?: Record; limit?: number }) => { + findCalls.push(objectName); + if (objectName in failFor) throw failFor[objectName]; + let records = store[objectName] ?? []; + if (query?.where) records = records.filter((r) => matchesWhere(r, query.where!)); + if (typeof query?.limit === 'number') records = records.slice(0, query.limit); + return records; + }), + findOne: vi.fn(async (objectName: string, query?: Record) => { + assertEngineFindOnePredicate(objectName, query); + const rows = await (engine.find as unknown as (o: string, q: unknown) => Promise)( + objectName, { ...query, limit: 1 }, + ); + return rows[0] ?? null; + }), + insert: vi.fn(async (objectName: string, data: Record | Record[]) => { + store[objectName] ??= []; + if (Array.isArray(data)) { + const records = data.map((d) => ({ ...d, id: `gen-${++idCounter}` }) as StoreRow); + store[objectName].push(...records); + return records; + } + const record = { ...data, id: `gen-${++idCounter}` } as StoreRow; + store[objectName].push(record); + return record; + }), + update: vi.fn(async (objectName: string, data: Record) => { + // The seed loader dispatches an update by the id carried IN `data` + // (no `where`), so the producer's own decision is asked in exactly + // that form. + assertEngineUpdateDispatch(data, undefined); + const records = store[objectName] ?? []; + const idx = records.findIndex((r) => r.id === data.id); + if (idx >= 0) { + records[idx] = { ...records[idx], ...data } as StoreRow; + return records[idx]; + } + return data; + }), + delete: vi.fn(async (_objectName: string, options?: { where?: Record }) => { + assertEngineDeleteDispatch(options); + return { deleted: 1 }; + }), + count: vi.fn(async (objectName: string) => (store[objectName] ?? []).length), + aggregate: vi.fn(async () => []), + } as unknown as IDataEngine; + + return { + engine, + store, + findCalls, + failReadsOf: (objectName: string, error: unknown) => { failFor[objectName] = error; }, + }; +} + +const WIDGET = { + name: 'my_app_widget', + fields: { + name: { type: 'text' }, + sku: { type: 'text' }, + }, +}; + +function createMetadata(): IMetadataService { + return { + getObject: vi.fn(async () => WIDGET), + listObjects: vi.fn(async () => [WIDGET]), + register: vi.fn(async () => {}), + get: vi.fn(async () => WIDGET), + list: vi.fn(async () => []), + unregister: vi.fn(async () => {}), + exists: vi.fn(async () => false), + listNames: vi.fn(async () => []), + } as unknown as IMetadataService; +} + +const CONFIG = { + dryRun: false, + haltOnError: false, + multiPass: true, + defaultMode: 'insert', + batchSize: 1000, + transaction: false, +} as never; + +const seedOf = (records: Array>) => [{ + object: 'my_app_widget', + externalId: 'sku', + mode: 'insert', + env: ['prod', 'dev', 'test'], + records, +}] as never; + +/** The real driver phrasings, verbatim. */ +const connectionDropped = () => + Object.assign(new Error('connection terminated unexpectedly'), { code: 'ECONNRESET' }); +const permissionDenied = () => + Object.assign(new Error('permission denied for table sys_organization'), { code: '42501' }); +const tableNotProvisioned = () => + Object.assign(new Error('SQLITE_ERROR: no such table: sys_organization'), { code: 'SQLITE_ERROR' }); + +/** Capture a rejection without letting a resolve pass silently. */ +async function rejection(run: () => Promise): Promise<{ code?: string; message?: string } & Record> { + let caught: unknown; + let resolved: unknown; + let didResolve = false; + try { + resolved = await run(); + didResolve = true; + } catch (e) { + caught = e; + } + expect( + didResolve, + `expected a rejection, but the load resolved with ${JSON.stringify(resolved)}`, + ).toBe(false); + return caught as { code?: string; message?: string } & Record; +} + +const widgets = (store: Record) => store.my_app_widget ?? []; + +describe('[#12852] seed loader — a sole-organization read that FAILED is not "no sole organization"', () => { + // ── POSITIVE CONTROLS. Without these, every assertion below could pass on + // a harness that no longer consults `resolveSoleOrganizationId` at all. + + it('control: a probe that RUNS and finds exactly one org stamps business rows with it', async () => { + const { engine, store, findCalls } = createEngine(); + store.sys_organization = [{ id: 'org_solo' }]; + + const result = await new SeedLoaderService(engine, createMetadata(), createLogger()).load({ + seeds: seedOf([{ name: 'Fresh', sku: 'W-A' }]), + config: CONFIG, + }); + + expect(findCalls).toContain('sys_organization'); + expect(result.summary.totalInserted).toBe(1); + expect(widgets(store)).toHaveLength(1); + expect(widgets(store)[0].organization_id).toBe('org_solo'); + }); + + it('control: a probe that RUNS and finds NO org leaves the row org-less — the declared ambiguity', async () => { + const { engine, store, findCalls } = createEngine(); + + const result = await new SeedLoaderService(engine, createMetadata(), createLogger()).load({ + seeds: seedOf([{ name: 'Fresh', sku: 'W-A' }]), + config: CONFIG, + }); + + expect(findCalls).toContain('sys_organization'); + expect(result.summary.totalInserted).toBe(1); + expect(widgets(store)[0].organization_id).toBeUndefined(); + }); + + it('control: a probe that RUNS and finds SEVERAL orgs leaves the row org-less — the declared ambiguity', async () => { + const { engine, store } = createEngine(); + store.sys_organization = [{ id: 'org_a' }, { id: 'org_b' }]; + + await new SeedLoaderService(engine, createMetadata(), createLogger()).load({ + seeds: seedOf([{ name: 'Fresh', sku: 'W-A' }]), + config: CONFIG, + }); + + expect(widgets(store)[0].organization_id).toBeUndefined(); + }); + + // ── THE FIX — a probe that could not run must surface, not invent "none". + + it('a dropped connection surfaces that error and writes NO org-less rows', async () => { + const { engine, store, findCalls, failReadsOf } = createEngine(); + // An org that the probe MUST see. Pre-fix the failed read hid it and + // the seed wrote rows that no member of `org_solo` could ever see. + store.sys_organization = [{ id: 'org_solo' }]; + const injected = connectionDropped(); + failReadsOf('sys_organization', injected); + + const caught = await rejection(() => new SeedLoaderService(engine, createMetadata(), createLogger()).load({ + seeds: seedOf([{ name: 'Fresh', sku: 'W-A' }]), + config: CONFIG, + })); + + // The caller receives the READ's own failure, envelope intact — this + // fix mints no new code and no new result field. + expect(caught).toBe(injected); + expect(caught.message).toBe('connection terminated unexpectedly'); + expect(caught.code).toBe('ECONNRESET'); + // Proof the probe really ran and really threw. + expect(findCalls).toContain('sys_organization'); + // …and emphatically NOT the pre-fix outcome: a batch of org-less rows + // reported as a clean seed. + expect(widgets(store)).toHaveLength(0); + }); + + it('a permission refusal surfaces the same way', async () => { + const { engine, store, failReadsOf } = createEngine(); + const injected = permissionDenied(); + failReadsOf('sys_organization', injected); + + const caught = await rejection(() => new SeedLoaderService(engine, createMetadata(), createLogger()).load({ + seeds: seedOf([{ name: 'Fresh', sku: 'W-A' }]), + config: CONFIG, + })); + + expect(caught).toBe(injected); + expect(caught.code).toBe('42501'); + expect(widgets(store)).toHaveLength(0); + }); + + // ── THE ONE BENIGN CASE — an unprovisioned table can hold no org, so + // "no sole organization" IS the truth and the historical + // global/cross-tenant NULL is the right answer. + + it('an UNPROVISIONED sys_organization is truthful emptiness: the seed writes its rows org-less', async () => { + const { engine, store, findCalls, failReadsOf } = createEngine(); + failReadsOf('sys_organization', tableNotProvisioned()); + + const result = await new SeedLoaderService(engine, createMetadata(), createLogger()).load({ + seeds: seedOf([{ name: 'Fresh', sku: 'W-A' }]), + config: CONFIG, + }); + + expect(result.summary.totalInserted).toBe(1); + expect(result.summary.totalErrored).toBe(0); + // Proof the benign branch was actually EXERCISED — the probe ran and + // threw. Without this, the passing insert above would be consistent + // with a harness that never probes at all. + expect(findCalls).toContain('sys_organization'); + expect(widgets(store)).toHaveLength(1); + expect(widgets(store)[0].organization_id).toBeUndefined(); + }); + + it('an UNPROVISIONED sys_organization in the postgres phrasing (42P01) is benign too', async () => { + const { engine, store, findCalls, failReadsOf } = createEngine(); + failReadsOf('sys_organization', Object.assign( + new Error('relation "sys_organization" does not exist'), + { code: '42P01' }, + )); + + const result = await new SeedLoaderService(engine, createMetadata(), createLogger()).load({ + seeds: seedOf([{ name: 'Fresh', sku: 'W-A' }]), + config: CONFIG, + }); + + expect(result.summary.totalInserted).toBe(1); + expect(findCalls).toContain('sys_organization'); + expect(widgets(store)[0].organization_id).toBeUndefined(); + }); + + it('a missing COLUMN on an existing sys_organization stays loud (the superstring case)', async () => { + const { engine, store, failReadsOf } = createEngine(); + // Postgres phrases this failure as `column "x" of relation "y" does not + // exist` — which CONTAINS a complete, legal missing-table phrase. + // `isMissingTableError`'s front-exclusion is what keeps it loud, and + // this pin is what stops a future hand-rolled message test reading it + // as benign and silently re-arming the org-less write. + const injected = Object.assign( + new Error('column "id" of relation "sys_organization" does not exist'), + { code: '42703' }, + ); + failReadsOf('sys_organization', injected); + + const caught = await rejection(() => new SeedLoaderService(engine, createMetadata(), createLogger()).load({ + seeds: seedOf([{ name: 'Fresh', sku: 'W-A' }]), + config: CONFIG, + })); + + expect(caught).toBe(injected); + expect(caught.message).toBe('column "id" of relation "sys_organization" does not exist'); + expect(widgets(store)).toHaveLength(0); + }); + + // ── NON-EFFECT — a caller that pinned its own org never asks the probe, so + // an unreadable `sys_organization` cannot fail a scoped seed. + + it('a pinned config.organizationId never consults the probe at all', async () => { + const { engine, store, findCalls, failReadsOf } = createEngine(); + failReadsOf('sys_organization', connectionDropped()); + + const result = await new SeedLoaderService(engine, createMetadata(), createLogger()).load({ + seeds: seedOf([{ name: 'Fresh', sku: 'W-A' }]), + config: { ...(CONFIG as object), organizationId: 'org_pinned' } as never, + }); + + expect(result.summary.totalInserted).toBe(1); + expect(findCalls).not.toContain('sys_organization'); + expect(widgets(store)[0].organization_id).toBe('org_pinned'); + }); +}); diff --git a/packages/metadata-protocol/src/seed-loader.ts b/packages/metadata-protocol/src/seed-loader.ts index d36dea3892..1790b7c3bf 100644 --- a/packages/metadata-protocol/src/seed-loader.ts +++ b/packages/metadata-protocol/src/seed-loader.ts @@ -1382,6 +1382,9 @@ export class SeedLoaderService implements ISeedLoaderService { * org-less (→ invisible under strict org-scoping). Returns undefined when * there are zero or several orgs (genuinely ambiguous — keep the historical * global/cross-tenant NULL) or when `sys_organization` is absent. + * + * [#12852] A read that FAILED for any other reason PROPAGATES — it is not + * an emptiness. See the catch below. */ private async resolveSoleOrganizationId(): Promise { try { @@ -1394,8 +1397,38 @@ export class SeedLoaderService implements ISeedLoaderService { const id = (rows[0] as { id?: unknown; _id?: unknown })?.id ?? (rows[0] as { _id?: unknown })?._id; return id ? String(id) : undefined; } - } catch { - // sys_organization may not exist (single-tenant runtime) — ignore. + } catch (error) { + // [#12852] Discriminate by error TYPE, the same repair PR #9817 made to + // `ObjectQL.probeInstallOrganizations` — the sibling probe with this + // exact shape, on the other side of the engine boundary. This site was + // missed by that pass. + // + // The bare `catch {}` this replaces answered EVERY cause with `undefined`, + // and `undefined` is not a neutral value here: it is the verdict this + // method's own JSDoc describes as "genuinely ambiguous", so the caller + // stamps no `organization_id` and every BUSINESS seed row of the run lands + // org-less — invisible afterwards under strict org-scoping. Nothing says + // so: `load()` reports through `SeedLoadResult.errors`, and this path + // never touches it, so a transient outage mid-seed reads to the operator + // as a clean, successful seed. ADR-0110 D3's exact shape: "the read found + // no sole organization" and "the read could not run" are different facts. + // + // Benign, and the only one — it is precisely the cause the swallowed + // comment already named: `sys_organization` is absent because its TABLE + // was never provisioned (schema sync not run yet, a single-tenant + // runtime). It can hold no row, so "no sole organization" IS the truth and + // the historical global/cross-tenant NULL is the right answer. Asked + // through the shared `isMissingTableError` predicate + // (`@objectstack/metadata/errors`) — the same call `loadExistingRecords` + // makes below — never a hand-rolled message test, so one vocabulary of + // "benign driver error" serves every seam that needs one. + // + // Everything else (connection loss, a timeout, a permission denial, a + // driver fault) means organizations may well exist and simply were not + // seen. It propagates, envelope intact: the seed run fails loudly instead + // of writing a batch of rows nobody will be able to see. No new error code + // and no new result field — the caller receives the read's own failure. + if (!isMissingTableError(error)) throw error; } return undefined; } diff --git a/packages/objectql/src/lifecycle/lifecycle-service.test.ts b/packages/objectql/src/lifecycle/lifecycle-service.test.ts index e7626cade0..f633cdc358 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.test.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.test.ts @@ -3048,3 +3048,240 @@ describe('LifecycleService teardown (#4747)', () => { svc.stop(); }); }); + +// [#12853] `loadGovernance`'s tenant scan used to fail into a bare +// `catch { /* No sys_organization (single-tenant kernel) */ }`, which made "this +// deployment has tuned no tenant" and "the read that would have found the +// tenants never happened" the same observable value: an empty +// `snapshot.tenantOverrides`. +// +// That value is not inert. `reap()` and `archiveObject()` read it to decide +// which cutoff each tenant's rows are DELETED on, so an empty map falls every +// tenant back to the global window — wrong in both directions, and the +// expensive direction is a tenant configured to retain LONGER having its rows +// expired early. Nothing reported it: `GovernanceSnapshot` has no field saying +// the tenant pass did not complete, and the catch logged nothing. +// +// The repair discriminates by error TYPE through the declared +// `isMissingTableError` predicate — an unprovisioned `sys_organization` really +// does mean "no tenant overrides" — and every other cause aborts the sweep +// BEFORE any policy is applied, reported through `report.errors` and a `warn`. +// Aborting, not logging-and-continuing: a log cannot bring back a reaped row, +// and the rows a deferred sweep leaves are still there for the next one. +// +// The pins below are about the CONSEQUENCE, not about a throw: what must be +// observable is that no candidate row is read and no row is deleted on a window +// nobody managed to verify. Every expectation is a LITERAL, and each refusal is +// paired with a positive control on the same harness, so a fixture that had +// stopped reaping at all could not satisfy them vacuously. +describe('LifecycleService.sweep — governance tenant scan failure (#12853)', () => { + const TELEMETRY_OBJ: LifecycleObjectLike = { + name: 'sys_job_run', + lifecycle: { class: 'telemetry', retention: { maxAge: '30d' } } as any, + }; + + /** Settings service backed by a value map, with per-tenant values. */ + function fakeSettings(values: Record, tenantValues: Record> = {}) { + return { + async get(_ns: string, key: string, ctx?: Record) { + const tenantId = ctx?.tenantId as string | undefined; + if (tenantId && tenantValues[tenantId] && key in tenantValues[tenantId]) { + return { value: tenantValues[tenantId][key], source: 'tenant' }; + } + if (key in values) return { value: values[key], source: 'global' }; + return { value: undefined, source: 'default' }; + }, + }; + } + + /** + * An engine that answers both reads this sweep issues: the governance tenant + * scan on `sys_organization`, and `batchedReap`'s candidate page for the + * declared object. The candidate page always yields exactly one row, so a + * reap that RUNS always issues a delete — which is what makes "no delete" + * a refusal rather than an empty fixture. + */ + function orgScanEngine(orgs: Array<{ id: string }>) { + let orgReadThrows: unknown; + let candidateRow = 0; + const { engine, deletes, finds } = captureEngine([TELEMETRY_OBJ], { + findImpl: (object: string) => { + if (object === 'sys_organization') { + if (orgReadThrows !== undefined) throw orgReadThrows; + return orgs; + } + return [{ id: `row-${++candidateRow}` }]; + }, + }); + return { + engine, + deletes, + /** The `where` of every candidate read the reaper issued. */ + reapReads: () => finds.filter((f) => f.object === 'sys_job_run').map((f) => f.where), + /** Did the tenant scan actually run? */ + orgReads: () => finds.filter((f) => f.object === 'sys_organization').length, + failOrgReadWith: (error: unknown) => { orgReadThrows = error; }, + healOrgRead: () => { orgReadThrows = undefined; }, + }; + } + + /** The real driver phrasings, verbatim. */ + const connectionDropped = () => Object.assign(new Error('connection reset by peer'), { code: 'ECONNRESET' }); + const tableNotProvisioned = () => + Object.assign(new Error('SQLITE_ERROR: no such table: sys_organization'), { code: 'SQLITE_ERROR' }); + + const abortedError = (message: string) => + `governance snapshot could not be loaded (${message}) — sweep aborted before any policy ` + + 'was applied, so no rows were reaped for this object'; + const abortedWarn = (message: string) => + `[lifecycle] governance snapshot could not be loaded (${message}); sweep aborted before any ` + + 'policy was applied — no rows were reaped, and the next scheduled sweep retries'; + + // ── POSITIVE CONTROLS ───────────────────────────────────────────────────── + + it('control: a tenant scan that RUNS gives the regulated tenant its own longer window', async () => { + const box = orgScanEngine([{ id: 'org_reg' }]); + const settings = fakeSettings( + {}, + { org_reg: { retention_overrides: { sys_job_run: { maxAge: '90d' } } } }, + ); + + const report = await service(box.engine, { getSettings: () => settings }).sweep(); + + // The tenant pass on the tenant's own rows, then the global pass for + // everyone else — this is the behaviour a failed scan silently destroys. + expect(box.reapReads()).toEqual([ + { created_at: { $lt: isoCutoff('90d') }, organization_id: 'org_reg' }, + { + created_at: { $lt: isoCutoff('30d') }, + $or: [{ organization_id: { $nin: ['org_reg'] } }, { organization_id: null }], + }, + ]); + expect(box.deletes).toHaveLength(2); + expect(report.errors).toEqual([]); + expect(report.swept).toHaveLength(1); + }); + + it('control: a tenant scan that RUNS and finds no organization reaps on one global pass', async () => { + const box = orgScanEngine([]); + + const report = await service(box.engine, { getSettings: () => fakeSettings({}) }).sweep(); + + expect(box.orgReads()).toBe(1); + expect(box.reapReads()).toEqual([{ created_at: { $lt: isoCutoff('30d') } }]); + expect(box.deletes).toHaveLength(1); + expect(report.errors).toEqual([]); + }); + + // ── THE ONE BENIGN CAUSE ────────────────────────────────────────────────── + + it('an UNPROVISIONED sys_organization is truthful emptiness: the sweep reaps on the global window', async () => { + const box = orgScanEngine([{ id: 'org_reg' }]); + box.failOrgReadWith(tableNotProvisioned()); + const settings = fakeSettings( + {}, + { org_reg: { retention_overrides: { sys_job_run: { maxAge: '90d' } } } }, + ); + + const report = await service(box.engine, { getSettings: () => settings }).sweep(); + + // Proof the benign branch was actually EXERCISED — the scan ran and threw. + // Without it, the reap below is equally consistent with a harness that + // never scans. + expect(box.orgReads()).toBe(1); + expect(box.reapReads()).toEqual([{ created_at: { $lt: isoCutoff('30d') } }]); + expect(box.deletes).toHaveLength(1); + expect(report.errors).toEqual([]); + expect(report.swept).toHaveLength(1); + }); + + // ── THE FIX — a scan that could not run must not decide a deletion window ── + + it('a dropped connection aborts the sweep: nothing is read as a candidate and nothing is deleted', async () => { + const warn = vi.fn(); + const box = orgScanEngine([{ id: 'org_reg' }]); + box.failOrgReadWith(connectionDropped()); + // The tenant that would have been found keeps rows THREE TIMES longer than + // the global window. Pre-fix this sweep reaped its rows at the 30d cutoff. + const settings = fakeSettings( + {}, + { org_reg: { retention_overrides: { sys_job_run: { maxAge: '90d' } } } }, + ); + + const report = await service(box.engine, { + getSettings: () => settings, + logger: { ...silentLogger(), warn }, + }).sweep(); + + // Proof the scan really ran and really threw. + expect(box.orgReads()).toBe(1); + // THE CONSEQUENCE, and the reason this card exists: no candidate row was + // even READ against a window nobody verified, so none was deleted. + expect(box.reapReads()).toEqual([]); + expect(box.deletes).toEqual([]); + expect(report.swept).toEqual([]); + // Surfaced through the channels that already exist — no new report field. + expect(report.errors).toEqual([ + { object: 'sys_job_run', error: abortedError('connection reset by peer') }, + ]); + expect(warn).toHaveBeenCalledWith(abortedWarn('connection reset by peer')); + }); + + it('a permission refusal aborts it the same way', async () => { + const box = orgScanEngine([{ id: 'org_reg' }]); + box.failOrgReadWith(Object.assign(new Error('permission denied for table sys_organization'), { code: '42501' })); + + const report = await service(box.engine, { getSettings: () => fakeSettings({}) }).sweep(); + + expect(box.deletes).toEqual([]); + expect(report.errors).toEqual([ + { object: 'sys_job_run', error: abortedError('permission denied for table sys_organization') }, + ]); + }); + + it('a missing COLUMN on an existing sys_organization stays loud (the superstring case)', async () => { + // Postgres phrases this as `column "x" of relation "y" does not exist`, + // which CONTAINS a complete, legal missing-table phrase. + // `isMissingTableError`'s front-exclusion is what keeps it loud, and this + // pin stops a future hand-rolled message test reading it as benign and + // silently re-arming the wrong-window reap. + const box = orgScanEngine([{ id: 'org_reg' }]); + box.failOrgReadWith(Object.assign( + new Error('column "id" of relation "sys_organization" does not exist'), + { code: '42703' }, + )); + + const report = await service(box.engine, { getSettings: () => fakeSettings({}) }).sweep(); + + expect(box.deletes).toEqual([]); + expect(report.errors).toEqual([ + { + object: 'sys_job_run', + error: abortedError('column "id" of relation "sys_organization" does not exist'), + }, + ]); + }); + + // ── THE CONTRACT THE ABORT KEEPS ────────────────────────────────────────── + + it('the abort is REPORTED, never thrown — `sweep()` still resolves, and the next sweep runs', async () => { + const box = orgScanEngine([{ id: 'org_reg' }]); + box.failOrgReadWith(connectionDropped()); + const svc = service(box.engine, { getSettings: () => fakeSettings({}) }); + + // Resolves. The scheduler enters this as `void this.sweep()`, where a + // rejection would be unhandled — the objection #8906 recorded when it + // declined to rethrow from `checkGovernance`. + const first = await svc.sweep(); + expect(first.errors).toHaveLength(1); + expect(box.deletes).toEqual([]); + + // …and the `sweeping` latch was released, so a transient outage costs one + // sweep, not the schedule. + box.healOrgRead(); + const second = await svc.sweep(); + expect(second.errors).toEqual([]); + expect(box.reapReads()).toEqual([{ created_at: { $lt: isoCutoff('30d') } }]); + expect(box.deletes).toHaveLength(1); + }); +}); diff --git a/packages/objectql/src/lifecycle/lifecycle-service.ts b/packages/objectql/src/lifecycle/lifecycle-service.ts index d1f5da2b9d..e7fa653556 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.ts @@ -48,7 +48,12 @@ import type { * it produces no audit rows either way, and `sys_file`, the one that is * audited, already reaped per id because it carries reap guards. * - A sweep failure is logged and isolated; it never throws into the - * scheduler and never blocks other objects' policies. + * scheduler and never blocks other objects' policies. [#12853] One + * deliberate exception to the second half: a governance snapshot that + * could not be READ aborts the whole sweep before any policy is applied, + * because every object's window would otherwise be decided from evidence + * nobody gathered. It is still not THROWN — `sweep()` reports it through + * `report.errors` and logs it, like every other failure here. */ /** Cross-tenant operator context — lifecycle is a system policy, not a user @@ -600,7 +605,41 @@ export class LifecycleService { .filter((o) => o?.lifecycle && o.lifecycle.class !== 'record'); // Governance snapshot (P4): settings-driven overrides / quotas. - this.governance = await this.loadGovernance(engine, declared); + // + // [#12853] A snapshot that could not be READ aborts the sweep here, + // before a single policy is applied. `loadGovernance` throws only when + // the `sys_organization` scan failed for a non-benign reason, and every + // window this sweep would use is downstream of that scan — so carrying on + // means reaping every tenant against the global window while believing it + // was the configured one. Deferring costs a sweep; the rows are still + // there for the next one. + // + // Reported and logged rather than rethrown, deliberately. `sweep()`'s + // declared contract is that it never throws (`LifecycleSweepReport.errors` + // is its failure channel) and the scheduler enters it as + // `void this.sweep()`, where a rejection is unhandled — the same + // objection #8906 recorded when it declined to rethrow from + // `checkGovernance` one method below. One `errors` entry per declared + // object, because that field means "a lifecycle policy did not get + // applied" and here none of them did. + try { + this.governance = await this.loadGovernance(engine, declared); + } catch (error) { + const msg = (error as Error)?.message ?? String(error); + for (const obj of declared) { + report.errors.push({ + object: obj.name, + error: + `governance snapshot could not be loaded (${msg}) — sweep aborted before any policy ` + + `was applied, so no rows were reaped for this object`, + }); + } + this.opts.logger.warn( + `[lifecycle] governance snapshot could not be loaded (${msg}); sweep aborted before any ` + + 'policy was applied — no rows were reaped, and the next scheduled sweep retries', + ); + return report; + } if (!this.governance.enabled) { this.opts.logger.debug?.('[lifecycle] disabled via settings; sweep skipped'); return report; @@ -731,8 +770,13 @@ export class LifecycleService { } /** Resolve the `lifecycle` settings namespace into a per-sweep snapshot. - * Every read is best-effort: no settings service / unregistered namespace - * ⇒ declared policies apply unmodified. */ + * Every SETTINGS read is best-effort: no settings service / unregistered + * namespace ⇒ declared policies apply unmodified. + * + * [#12853] The tenant scan is not. A `sys_organization` read that FAILED + * throws out of here, because an empty `tenantOverrides` is the same value + * as "this deployment has no tenant overrides" and the caller acts on the + * difference by DELETING rows. See the catch below. */ private async loadGovernance( engine: LifecycleEngineLike, declared: LifecycleObjectLike[], @@ -783,8 +827,37 @@ export class LifecycleService { snapshot.tenantOverrides.set(objectName, list); } } - } catch { - // No sys_organization (single-tenant kernel) — tenant overrides n/a. + } catch (error) { + // [#12853] Discriminate by error TYPE. `snapshot.tenantOverrides` is + // the ADR-0057 §3.2 per-tenant retention/expiry window set, and an + // EMPTY map is not a neutral value: `reap()` and `archiveObject()` read + // it as "this deployment has tuned no tenant", fall every tenant back + // to the global window, and DELETE on it. That window is wrong in both + // directions — a tenant configured to retain LONGER has its rows + // expired early, and one configured to retain shorter keeps them. + // + // The bare `catch {}` this replaces answered every cause that way, and + // nothing reported it: `GovernanceSnapshot` carries no field saying the + // tenant pass did not complete, and the catch logged nothing. So a + // connection drop, a timeout, a permission refusal or a driver fault + // produced a deletion executed on knowingly incomplete evidence, by an + // executor that did not know it was incomplete. ADR-0110 D3's shape, + // with a delete on the wrong side of it. + // + // Benign, and the only one — precisely the cause the swallowed comment + // already named: no `sys_organization` at all (a single-tenant kernel), + // i.e. its TABLE was never provisioned. It can hold no organization, so + // "no tenant overrides" IS the truth and the global window is the right + // window for everyone. Asked through the shared `isMissingTableError` + // predicate (`@objectstack/metadata/errors`) — the same call + // `checkGovernance` makes below — never a hand-rolled message test. + // + // Everything else propagates to `sweep()`, which aborts the sweep + // before applying any policy. For a DELETION action, "do not act on + // incomplete evidence" is the correct failure direction: a log cannot + // bring back a reaped row, and the rows this defers are still there for + // the next sweep to reap once the read succeeds. + if (!isMissingTableError(error)) throw error; } } diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 882c83a48d..cd59591e14 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1356,6 +1356,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/seed-loader-sole-organization-read-failure.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/seed-loader-sole-organization-read-failure.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/seed-loader-sole-organization-read-failure.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/seed-loader-state-machine-exempt.test.ts", "verb": "delete",