From bb61306f5c6968292c48c8e39342fd3eedfdfc8c Mon Sep 17 00:00:00 2001 From: os-sam Date: Sun, 23 Aug 2026 18:21:02 +0000 Subject: [PATCH 1/3] =?UTF-8?q?feat(approvals):=20plan=20the=20org-less=20?= =?UTF-8?q?platform-row=20backfill=20=E2=80=94=20dry=20run=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DRY-RUN half of the #11308 one-off repair sweep, landed on its own so the write path cannot be what defines the plan: for a script that touches existing data the dry run has to exist first, and be a deliverable in its own right. `planPlatformRowOrganizationBackfill` reads only. It scans each stranded platform table for rows whose organization column is unset, re-reads each row's SUBJECT record, and reports — broken out per object — what it would write, plus the rows it deliberately would not: a subject with no organization of its own is counted and NAMED (out of the ruling), never given an invented one. Both the column read on the subject and the column written on the platform row come from the shared `createRecordOrganizationResolver`, so `sys_api_key`'s stamp-only `active_organization_id` fork is honoured rather than flattened. Refs #11308 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- ...ackfill-platform-row-organizations.test.ts | 280 ++++++++ .../backfill-platform-row-organizations.ts | 639 ++++++++++++++++++ 2 files changed, 919 insertions(+) create mode 100644 packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts create mode 100644 packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts diff --git a/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts b/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts new file mode 100644 index 0000000000..bfc6fbd011 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts @@ -0,0 +1,280 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The one-off platform-row organization backfill (#11308) — dry run and write. + * + * The three properties the 2026-08-23 maintainer ruling names are asserted + * here rather than described anywhere: + * + * 1. **Dry run writes nothing** — the fake engine fails the test if `update` + * is reached at all during a plan. + * 2. **Only rows whose subject HAS an organization are written** — a stranded + * row whose subject is equally org-less is COUNTED and reported, never + * given an invented organization. + * 3. **Idempotent** — the sweep runs twice against the same engine and the + * second run's write count is asserted to be 0. + * + * Plus the one thing this card must not do: a platform row about a + * `sys_api_key` is repaired from `active_organization_id` (limb 0, + * stamp-only, #8778), and the credential table is never written to. A sweep + * that "unified everything onto one organization field" would flatten that + * fork, so it is pinned rather than trusted. + */ + +import { describe, it, expect, beforeEach } from 'vitest'; +import { + planPlatformRowOrganizationBackfill, + formatBackfillReport, + BACKFILL_TARGETS, + type BackfillEngine, +} from './backfill-platform-row-organizations.js'; + +type Row = Record; + +/** + * Registered schemas for the objects under test. `fields` is what the shared + * resolver's presence probe reads; `tenancy` is what its limbs read. + */ +const SCHEMAS: Record = { + sys_approval_request: { name: 'sys_approval_request', fields: { id: {}, organization_id: {}, object_name: {}, record_id: {}, payload_json: {}, status: {} } }, + sys_approval_action: { name: 'sys_approval_action', fields: { id: {}, organization_id: {}, request_id: {} } }, + sys_approval_approver: { name: 'sys_approval_approver', fields: { id: {}, organization_id: {}, request_id: {} } }, + sys_automation_run: { name: 'sys_automation_run', fields: { id: {}, organization_id: {}, trigger_object: {}, trigger_record_id: {}, context_json: {}, status: {} } }, + crm_deal: { name: 'crm_deal', fields: { id: {}, organization_id: {}, name: {} } }, + // ADR-0066 platform-global: an optional org FK while explicitly NOT + // tenant-scoped. Limb 1 resolves it to `null` — a subject with no + // organization of its own. + sys_sso_provider: { name: 'sys_sso_provider', tenancy: { enabled: false }, fields: { id: {}, organization_id: {} } }, + // ⛔ The deliberately preserved fork: stamp-only `organizationField` (limb 0) + // on an unwalled credential table. + sys_api_key: { + name: 'sys_api_key', + tenancy: { enabled: false, organizationField: 'active_organization_id' }, + fields: { id: {}, active_organization_id: {}, organization_id: {} }, + }, +}; + +interface FakeEngine extends BackfillEngine { + tables: Record; + updates: Array<{ object: string; data: Row }>; + failUpdates: boolean; +} + +function makeEngine(tables: Record, opts: { withSchema?: boolean } = {}): FakeEngine { + const withSchema = opts.withSchema !== false; + const matches = (row: Row, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [key, expected] of Object.entries(where)) { + const actual = row[key] ?? null; + if (expected && typeof expected === 'object' && '$in' in (expected as any)) { + if (!(expected as any).$in.includes(actual)) return false; + continue; + } + if (expected === null) { + if (actual !== null && actual !== undefined) return false; + continue; + } + if (actual !== expected) return false; + } + return true; + }; + const engine: FakeEngine = { + tables, + updates: [], + failUpdates: false, + async find(object: string, options?: any) { + const table = tables[object]; + // An unregistered object throws, exactly as an engine does for a plugin + // that is not mounted. + if (!table) throw new Error(`fake engine: unknown object '${object}'`); + let rows = table.filter(r => matches(r, options?.where)); + const order = options?.orderBy?.[0]; + if (order) { + rows = [...rows].sort((a, b) => String(a[order.field] ?? '').localeCompare(String(b[order.field] ?? ''))); + } + const offset = typeof options?.offset === 'number' ? options.offset : 0; + const limit = typeof options?.limit === 'number' ? options.limit : rows.length; + return rows.slice(offset, offset + limit).map(r => ({ ...r })); + }, + async update(object: string, data: any) { + if (engine.failUpdates) throw new Error(`fake engine: update('${object}') must not be reached in a dry run`); + engine.updates.push({ object, data: { ...data } }); + const table = tables[object] ?? []; + const i = table.findIndex(r => r.id === data.id); + if (i >= 0) table[i] = { ...table[i], ...data }; + return table[i]; + }, + getSchema(object: string) { + return withSchema ? SCHEMAS[object] : undefined; + }, + }; + if (!withSchema) delete (engine as any).getSchema; + return engine; +} + +/** The population the card measures: stranded rows across all four tables. */ +function strandedFixture(): Record { + return { + sys_approval_request: [ + // (a) pending, subject alive and org-owned — the harmful half. + { id: 'areq_1', organization_id: null, object_name: 'crm_deal', record_id: 'deal_1', status: 'pending', payload_json: JSON.stringify({ id: 'deal_1', organization_id: 'org_A' }) }, + // (b) terminal, subject DELETED, snapshot survives it. + { id: 'areq_2', organization_id: null, object_name: 'crm_deal', record_id: 'deal_gone', status: 'approved', payload_json: JSON.stringify({ id: 'deal_gone', organization_id: 'org_B' }) }, + // (c) ⛔ subject is org-less too (ADR-0066 platform-global) — OUT OF RULING. + { id: 'areq_3', organization_id: null, object_name: 'sys_sso_provider', record_id: 'sso_1', status: 'pending', payload_json: JSON.stringify({ id: 'sso_1', organization_id: 'org_C' }) }, + // (d) ⛔ subject is a sys_api_key — the preserved fork. + { id: 'areq_4', organization_id: null, object_name: 'sys_api_key', record_id: 'key_1', status: 'pending', payload_json: null }, + // (e) already stamped — must never be re-read or re-written. + { id: 'areq_5', organization_id: 'org_A', object_name: 'crm_deal', record_id: 'deal_1', status: 'pending', payload_json: null }, + ], + sys_approval_action: [ + { id: 'aact_1', organization_id: null, request_id: 'areq_1' }, + { id: 'aact_2', organization_id: null, request_id: 'areq_3' }, + // A child left behind by an interrupted run: its parent already carries + // an organization. + { id: 'aact_3', organization_id: null, request_id: 'areq_5' }, + ], + sys_approval_approver: [ + { id: 'aapr_1', organization_id: null, request_id: 'areq_1' }, + ], + sys_automation_run: [ + // paused: live resumable state, subject alive. + { id: 'run_p1', organization_id: null, trigger_object: 'crm_deal', trigger_record_id: 'deal_1', status: 'paused', context_json: JSON.stringify({ object: 'crm_deal', record: { id: 'deal_1', organization_id: 'org_A' } }) }, + // terminal history: no context_json at all (recordTerminal writes none). + { id: 'run_run_t1', organization_id: null, trigger_object: 'crm_deal', trigger_record_id: 'deal_2', status: 'completed', context_json: null }, + // a plain scheduled sweep: no ONE subject, by construction. + { id: 'run_p2', organization_id: null, trigger_object: null, trigger_record_id: null, status: 'paused', context_json: JSON.stringify({ event: 'schedule' }) }, + ], + crm_deal: [ + { id: 'deal_1', organization_id: 'org_A', name: 'Alpha' }, + { id: 'deal_2', organization_id: 'org_B', name: 'Beta' }, + ], + sys_sso_provider: [ + { id: 'sso_1', organization_id: 'org_C' }, + ], + sys_api_key: [ + { id: 'key_1', active_organization_id: 'org_K', organization_id: null }, + ], + }; +} + +function planFor(report: Awaited>, object: string) { + const plan = report.objects.find(o => o.object === object); + if (!plan) throw new Error(`no plan for ${object}`); + return plan; +} + +describe('platform-row organization backfill — dry run', () => { + let engine: FakeEngine; + beforeEach(() => { + engine = makeEngine(strandedFixture()); + }); + + it('writes nothing: update() is unreachable while planning', async () => { + engine.failUpdates = true; + const report = await planPlatformRowOrganizationBackfill(engine); + expect(report.dryRun).toBe(true); + expect(engine.updates).toHaveLength(0); + expect(report.totals.written).toBe(0); + expect(report.totals.planned).toBeGreaterThan(0); + }); + + it('breaks the report out per object, one entry per table it touches', async () => { + const report = await planPlatformRowOrganizationBackfill(engine); + expect(report.objects.map(o => o.object)).toEqual([ + 'sys_approval_request', + 'sys_approval_action', + 'sys_approval_approver', + 'sys_automation_run', + ]); + }); + + it('plans a stranded request from its LIVE subject record', async () => { + const report = await planPlatformRowOrganizationBackfill(engine); + const row = planFor(report, 'sys_approval_request').rows.find(r => r.id === 'areq_1'); + expect(row).toMatchObject({ + organizationField: 'organization_id', + organization: 'org_A', + resolvedFrom: 'live-record', + subjectObject: 'crm_deal', + subjectId: 'deal_1', + status: 'pending', + }); + }); + + it('falls back to the write-time snapshot when the subject is gone', async () => { + const report = await planPlatformRowOrganizationBackfill(engine); + const row = planFor(report, 'sys_approval_request').rows.find(r => r.id === 'areq_2'); + expect(row).toMatchObject({ organization: 'org_B', resolvedFrom: 'snapshot' }); + }); + + it('⛔ counts and names rows whose subject has no organization, and invents none', async () => { + const report = await planPlatformRowOrganizationBackfill(engine); + const plan = planFor(report, 'sys_approval_request'); + expect(plan.skipped.subjectHasNoOrganization).toBe(1); + expect(plan.outOfRulingScopeIds).toEqual(['areq_3']); + expect(plan.rows.map(r => r.id)).not.toContain('areq_3'); + expect(report.totals.outOfRulingScope).toBe(1); + }); + + it('⛔ repairs a sys_api_key subject from active_organization_id — the fork is not flattened', async () => { + const report = await planPlatformRowOrganizationBackfill(engine); + const row = planFor(report, 'sys_approval_request').rows.find(r => r.id === 'areq_4'); + // `sys_api_key.organization_id` is NULL on the fixture; only the stamp-only + // `active_organization_id` carries 'org_K'. Reading the canonical column + // would have produced no plan at all. + expect(row).toMatchObject({ organization: 'org_K', subjectObject: 'sys_api_key' }); + }); + + it('never scans a row that already carries an organization', async () => { + const report = await planPlatformRowOrganizationBackfill(engine); + const plan = planFor(report, 'sys_approval_request'); + expect(plan.scanned).toBe(4); + expect(plan.rows.map(r => r.id)).not.toContain('areq_5'); + }); + + it('moves the action / approver children with their request, from the PARENT row', async () => { + const report = await planPlatformRowOrganizationBackfill(engine); + const actions = planFor(report, 'sys_approval_action'); + expect(actions.role).toBe('parent-derived'); + expect(actions.rows.map(r => [r.id, r.organization])).toEqual([ + ['aact_1', 'org_A'], + // aact_3's parent (areq_5) was already stamped — an interrupted run's + // leftover child is still repaired. + ['aact_3', 'org_A'], + ]); + // aact_2 hangs off the out-of-ruling request and stays put. + expect(actions.skipped.parentHasNoOrganization).toBe(1); + expect(planFor(report, 'sys_approval_approver').rows.map(r => r.id)).toEqual(['aapr_1']); + }); + + it('sweeps automation runs in every status and breaks the plan out by status', async () => { + const report = await planPlatformRowOrganizationBackfill(engine); + const plan = planFor(report, 'sys_automation_run'); + expect(plan.plannedByStatus).toEqual({ paused: 1, completed: 1 }); + expect(plan.rows.find(r => r.id === 'run_p1')).toMatchObject({ organization: 'org_A', resolvedFrom: 'live-record' }); + expect(plan.rows.find(r => r.id === 'run_run_t1')).toMatchObject({ organization: 'org_B', resolvedFrom: 'live-record' }); + // The record-less scheduled run names no subject and gets none. + expect(plan.skipped.subjectUnaddressable).toBe(1); + }); + + it('says so loudly when the engine exposes no organization column at all', async () => { + const blind = makeEngine(strandedFixture(), { withSchema: false }); + const report = await planPlatformRowOrganizationBackfill(blind); + expect(report.totals.scanned).toBe(0); + expect(report.totals.planned).toBe(0); + for (const plan of report.objects) { + expect(plan.organizationField).toBeNull(); + expect(plan.notes.join(' ')).toContain('no organization column resolved'); + } + }); + + it('renders a per-object operator report naming the out-of-ruling ids', async () => { + const report = await planPlatformRowOrganizationBackfill(engine); + const text = formatBackfillReport(report); + expect(text).toContain('DRY RUN (nothing written)'); + for (const target of BACKFILL_TARGETS) expect(text).toContain(target.object); + expect(text).toContain('areq_3'); + expect(text).toContain('out-of-ruling(subject has no organization)=1'); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts b/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts new file mode 100644 index 0000000000..1d7d72dfac --- /dev/null +++ b/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts @@ -0,0 +1,639 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * backfill-platform-row-organizations — the ONE-OFF repair sweep for the + * platform rows the pre-#10101 writers stranded with no organization. + * + * ## What this repairs, and what it deliberately does not + * + * #10101 (landed as PR #11311) fixed the WRITERS: a `sys_approval_request` and + * a `sys_automation_run` are now stamped from the SUBJECT record's own + * organization, with the acting context as the ruled fallback. It wrote + * nothing to existing rows, so the population produced before it persists — + * measured on cloud#1395 as pending approval requests that LOCK the record + * they are about while being invisible in every organization-scoped inbox, + * their own owner's included, plus unattributed automation-run history. + * + * This module is that backfill, on the maintainer's 2026-08-23 ruling + * (direction 3): a one-off, idempotent sweep deriving each stranded row's + * organization from its SUBJECT record, **only for rows whose subject HAS an + * organization**, dry-run first. + * + * ⛔ Rows whose subject ALSO has no organization are OUT OF THE RULING. They + * are counted and reported (`skipped.subjectHasNoOrganization`) and never + * written: the acting-context fallback the WRITERS apply is not available to a + * repair — the acting context is gone — and inventing one is exactly the + * "fabricate an organization" option (Option C) that stayed vetoed on + * cloud#1395. A reported count is the deliverable for those rows. + * + * ⛔ It does NOT touch the write path, and it does NOT unify anybody's + * organization column. Both the column it READS on a subject and the column it + * WRITES on a platform row are resolved from the registered schema through the + * ONE shared resolver (`createRecordOrganizationResolver`, + * `@objectstack/metadata-core`) — never hard-coded to `organization_id`. That + * is what keeps `sys_api_key`'s deliberate divergence intact: its + * `tenancy.organizationField: 'active_organization_id'` (stamp-only, #8778) + * wins limb 0 of the resolver, so a platform row ABOUT an API key is repaired + * from that column, and the credential table itself is never written to. A + * sweep written on the intuition "unify everything onto one organization + * field" would flatten that fork; asking the schema cannot. + * + * ## Subject precedence — live record first, write-time snapshot second + * + * The same order `openNodeRequest` resolves with + * (`organizationOf(object, liveRecord, triggerSnapshot)`), and for the same + * reason one level up: a repair exists to put the row behind the wall its + * subject is behind NOW. The snapshot is the fallback for a subject that has + * since been deleted (`payload_json` for an approval request, `context_json`'s + * `record` for a paused automation run — terminal run rows carry no context + * blob, so they resolve from the live record or not at all). Every planned row + * records which of the two answered (`resolvedFrom`), so a dry-run reader can + * see the snapshot-derived rows without re-deriving them. + * + * ⚠️ This is the one place the sweep reads differently from the automation + * WRITER, which resolves from the trigger snapshot alone — it is serializing + * in-memory state and has no live read available at that moment. A repair + * does. The issue names this ("subject-record re-read at repair time") as + * blast radius to be deliberate about rather than as a thing to avoid. + * + * ## Child rows move with their parent + * + * `sys_approval_action` and `sys_approval_approver` are stamped from the same + * `requestOrg` as their request ("all three move together"), so they are swept + * from the PARENT ROW's organization rather than re-resolved from the subject + * — one resolution per request, never two answers for one request. Sweeping + * them by their own null also makes an interrupted run self-completing: a + * child left behind by a partial write is repaired on the next run, and the + * run after that writes nothing. + * + * ## Idempotency + * + * Every scan is `WHERE IS NULL`, and every write fills + * that column, so a repaired row cannot match again. Rows deliberately skipped + * (subject without an organization, unaddressable, missing) keep matching and + * keep being skipped — they are re-reported, never re-written. `planned` and + * `written` are both 0 on a second run over an unchanged database, and + * `backfill-platform-row-organizations.test.ts` asserts exactly that rather + * than asserting the property in prose. + * + * ## Usage + * + * Not exported from the package index and not shipped in `dist` — this is a + * one-off operational module, not platform surface. Run it server-side from a + * boot context that already holds an engine: + * + * ```ts + * const report = await planPlatformRowOrganizationBackfill(engine); + * console.log(formatBackfillReport(report)); // dry run: writes nothing + * ``` + * + * Rollback posture is stated on the PR: the dry-run report names every row id + * it would touch, so the undo is to write the previous value (NULL) back to + * exactly those ids. + */ + +import { createRecordOrganizationResolver } from '@objectstack/metadata-core'; + +/** + * The engine surface the sweep needs — a structural subset of `ApprovalEngine` + * / the ObjectQL engine, declared here so the module can be driven by a test + * double without pulling the service in. + * + * `getSchema` is what the shared resolver probes for. An engine without it + * resolves every organization column to `null`, which would make the sweep a + * silent no-op — so the report says so out loud instead (see `notes`). + */ +export interface BackfillEngine { + find(object: string, options?: unknown): Promise; + update(object: string, data: unknown, options?: unknown): Promise; + getSchema?(object: string): unknown; +} + +/** A child table stamped from its parent platform row's organization. */ +export interface BackfillChild { + /** The child object name. */ + object: string; + /** The child column naming its parent platform row. */ + parentField: string; +} + +/** One platform table the sweep repairs, plus how to find its subject. */ +export interface BackfillTarget { + /** The platform object carrying stranded rows. */ + object: string; + /** Column naming the SUBJECT's object. */ + subjectObjectField: string; + /** Column naming the SUBJECT's record id. */ + subjectIdField: string; + /** JSON column carrying the write-time subject snapshot, if the row has one. */ + snapshotField?: string; + /** Path to the record inside the parsed snapshot (`[]` = the snapshot IS the record). */ + snapshotPath?: readonly string[]; + /** Status column, reported as a per-status breakdown so retention posture stays visible. */ + statusField?: string; + /** Tables stamped from THIS row's organization. */ + children?: readonly BackfillChild[]; +} + +/** + * The tables the ruling's population lives in. + * + * `sys_automation_run` is swept in EVERY status, deliberately. Its terminal + * (`completed` / `failed`) history is subject to the object's declared + * retention (`maxAge: '30d'`, `onlyWhen: status in [completed, failed]`), so + * those rows age out on their own — but retention is a configurable sweep, an + * install that has it disabled keeps them forever, and the 30-day window is + * precisely the window an operator investigating this defect reads. The + * ruling's criterion is the subject's organization, not the row's status, so + * no status carve-out is encoded here; the per-status breakdown in the report + * is what keeps the retention fact visible to whoever reads the dry run. + */ +export const BACKFILL_TARGETS: readonly BackfillTarget[] = [ + { + object: 'sys_approval_request', + subjectObjectField: 'object_name', + subjectIdField: 'record_id', + // `payload_json` is `JSON.stringify(input.record)` — the subject record + // itself, snapshotted at submission time. + snapshotField: 'payload_json', + snapshotPath: [], + statusField: 'status', + children: [ + { object: 'sys_approval_action', parentField: 'request_id' }, + { object: 'sys_approval_approver', parentField: 'request_id' }, + ], + }, + { + object: 'sys_automation_run', + subjectObjectField: 'trigger_object', + subjectIdField: 'trigger_record_id', + // `context_json` is the serialized AutomationContext; the trigger record + // sits at `.record`. Written on paused rows only — `recordTerminal` does + // not persist it, so terminal rows resolve from the live subject or not + // at all. + snapshotField: 'context_json', + snapshotPath: ['record'], + statusField: 'status', + }, +]; + +/** Which candidate answered for a planned row. */ +export type SubjectProvenance = 'live-record' | 'snapshot' | 'parent-row'; + +/** One row the sweep would write, named in full so the dry run is auditable. */ +export interface PlannedRow { + object: string; + id: string; + /** The column on THIS row that carries its organization (schema-resolved). */ + organizationField: string; + /** The value that would be written. */ + organization: string; + subjectObject: string | null; + subjectId: string | null; + resolvedFrom: SubjectProvenance; + status?: string | null; +} + +/** Why a scanned row was left alone. */ +export interface BackfillSkips { + /** ⛔ Out of the ruling: the subject exists and has no organization either. */ + subjectHasNoOrganization: number; + /** The row names no subject object / record id at all. */ + subjectUnaddressable: number; + /** The subject row is gone and no write-time snapshot survives it. */ + subjectNotFound: number; + /** A child row whose parent has (and would get) no organization. */ + parentHasNoOrganization: number; +} + +/** Per-object plan and outcome — the unit the dry-run report is broken out by. */ +export interface ObjectPlan { + object: string; + /** `subject-derived` for a platform row, `parent-derived` for its children. */ + role: 'subject-derived' | 'parent-derived'; + /** The schema-resolved organization column, or `null` when it has none here. */ + organizationField: string | null; + scanned: number; + planned: number; + written: number; + skipped: BackfillSkips; + /** Planned rows per status value, when the object declares a status column. */ + plannedByStatus: Record; + /** Ids skipped as out-of-ruling — reported so the count is checkable, never written. */ + outOfRulingScopeIds: string[]; + rows: PlannedRow[]; + /** Conditions a reader must see, e.g. "this engine exposes no such column". */ + notes: string[]; +} + +/** The whole sweep's plan / outcome. */ +export interface BackfillReport { + /** `true` when nothing was written. */ + dryRun: boolean; + objects: ObjectPlan[]; + totals: { + scanned: number; + planned: number; + written: number; + outOfRulingScope: number; + }; +} + +/** Options both halves of the sweep accept. */ +export interface BackfillOptions { + /** + * Execution context for every read/write. Defaults to a system context — + * the sweep has to see rows across every organization, exactly as the + * writers' `SYSTEM_CTX` does. + */ + context?: unknown; + /** Rows per page while scanning. */ + pageSize?: number; + /** Hard ceiling per object, so a pathological table cannot spin forever. */ + maxRowsPerObject?: number; +} + +const SYSTEM_CONTEXT = { isSystem: true, positions: [], permissions: [] }; +const DEFAULT_PAGE_SIZE = 200; +const DEFAULT_MAX_ROWS = 100_000; +/** Id batches for subject / parent lookups. */ +const LOOKUP_CHUNK = 100; + +function emptySkips(): BackfillSkips { + return { + subjectHasNoOrganization: 0, + subjectUnaddressable: 0, + subjectNotFound: 0, + parentHasNoOrganization: 0, + }; +} + +function newObjectPlan(object: string, role: ObjectPlan['role'], organizationField: string | null): ObjectPlan { + return { + object, + role, + organizationField, + scanned: 0, + planned: 0, + written: 0, + skipped: emptySkips(), + plannedByStatus: {}, + outOfRulingScopeIds: [], + rows: [], + notes: [], + }; +} + +/** `''` and `null` and a non-string all mean "no organization here". */ +function nonEmpty(value: unknown): string | null { + return typeof value === 'string' && value.length > 0 ? value : null; +} + +function rowId(row: unknown): string | null { + const raw = (row as Record | null)?.id; + if (typeof raw === 'string' && raw.length > 0) return raw; + if (typeof raw === 'number') return String(raw); + return null; +} + +function parseSnapshot(raw: unknown, path: readonly string[]): Record | null { + if (typeof raw !== 'string' || raw.length === 0) return null; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + // A snapshot that will not parse is not a subject — the row falls through + // to the live record, or is reported as unresolved. Never a throw: one + // corrupt blob must not abort a sweep over thousands of healthy rows. + return null; + } + let cursor: unknown = parsed; + for (const key of path) { + if (!cursor || typeof cursor !== 'object') return null; + cursor = (cursor as Record)[key]; + } + return cursor && typeof cursor === 'object' && !Array.isArray(cursor) + ? (cursor as Record) + : null; +} + +function chunk(items: readonly T[], size: number): T[][] { + const out: T[][] = []; + for (let i = 0; i < items.length; i += size) out.push(items.slice(i, i + size)); + return out; +} + +/** + * Page through every row of `object` whose organization column is unset. + * + * Ordered by `id` so the pages partition the population instead of + * overlapping, and read in full BEFORE anything is written — a plan built + * while writing would move rows out from under its own offset. + */ +async function scanUnstampedRows( + engine: BackfillEngine, + object: string, + organizationField: string, + options: Required> & { context: unknown }, + plan: ObjectPlan, +): Promise[]> { + const out: Record[] = []; + for (let offset = 0; offset < options.maxRowsPerObject; offset += options.pageSize) { + let page: unknown[]; + try { + page = await engine.find(object, { + where: { [organizationField]: null }, + limit: options.pageSize, + offset, + orderBy: [{ field: 'id', order: 'asc' }], + context: options.context, + }); + } catch (err) { + // An install that does not mount the owning plugin has no such table. + // Named, not thrown: one absent table must not cost the sweep the other + // three, and a reader has to be able to tell "no stranded rows" from + // "never looked". + plan.notes.push(`scan of '${object}' failed — ${String((err as Error)?.message ?? err)}`); + break; + } + const rows = Array.isArray(page) ? page : []; + for (const row of rows) { + if (row && typeof row === 'object') out.push(row as Record); + } + if (rows.length < options.pageSize) break; + } + return out; +} + +/** Read a set of records by id, keyed by id. */ +async function readById( + engine: BackfillEngine, + object: string, + ids: readonly string[], + context: unknown, +): Promise>> { + const found = new Map>(); + for (const batch of chunk(ids, LOOKUP_CHUNK)) { + let rows: unknown[] = []; + try { + rows = await engine.find(object, { + where: { id: { $in: batch } }, + limit: batch.length, + context, + }); + } catch { + // An object that is not registered on this install (a plugin that is not + // mounted) answers with a throw. That is "subject not found", not a + // reason to abort the sweep. + rows = []; + } + for (const row of Array.isArray(rows) ? rows : []) { + const id = rowId(row); + if (id) found.set(id, row as Record); + } + } + return found; +} + +/** + * Build the sweep's plan — the DRY RUN. Reads only; `written` is 0 on every + * object it returns. + * + * This is the deliverable in its own right: it is the only thing that shows + * what will move before a single row is written. + */ +export async function planPlatformRowOrganizationBackfill( + engine: BackfillEngine, + options: BackfillOptions = {}, +): Promise { + const context = options.context ?? SYSTEM_CONTEXT; + const pageSize = options.pageSize ?? DEFAULT_PAGE_SIZE; + const maxRowsPerObject = options.maxRowsPerObject ?? DEFAULT_MAX_ROWS; + const scanOptions = { context, pageSize, maxRowsPerObject }; + // ⛔ ONE resolver for the whole sweep, and the only source of "which column + // carries this object's organization" on either side of the copy. + const resolver = createRecordOrganizationResolver(engine); + const objects: ObjectPlan[] = []; + + for (const target of BACKFILL_TARGETS) { + const organizationField = resolver.organizationFieldFor(target.object); + const plan = newObjectPlan(target.object, 'subject-derived', organizationField); + objects.push(plan); + if (!organizationField) { + // Not an error: a single-tenant install genuinely has no such column, and + // so does an engine double with no `getSchema`. Both must be LOUD — a + // backfill that silently sweeps nothing is the worst possible outcome, + // because it reads exactly like a clean database. + plan.notes.push( + `no organization column resolved for '${target.object}' — nothing scanned. ` + + 'On a multi-tenant install this means the engine exposed no schema for the object; ' + + 'on a single-tenant install it is expected.', + ); + continue; + } + + const rows = await scanUnstampedRows(engine, target.object, organizationField, scanOptions, plan); + plan.scanned = rows.length; + + // Group the addressable rows by subject object so the live re-read is one + // query per object rather than one per row. + const bySubjectObject = new Map>(); + for (const row of rows) { + const subjectObject = nonEmpty(row[target.subjectObjectField]); + const subjectId = nonEmpty(row[target.subjectIdField]); + if (!subjectObject || !subjectId) continue; + let ids = bySubjectObject.get(subjectObject); + if (!ids) bySubjectObject.set(subjectObject, (ids = new Set())); + ids.add(subjectId); + } + const liveByObject = new Map>>(); + for (const [subjectObject, ids] of bySubjectObject) { + liveByObject.set(subjectObject, await readById(engine, subjectObject, [...ids], context)); + } + + // The organization each planned parent row would carry, for the child pass. + const parentOrganizations = new Map(); + + for (const row of rows) { + const id = rowId(row); + if (!id) { + plan.skipped.subjectUnaddressable += 1; + continue; + } + const status = target.statusField ? (row[target.statusField] ?? null) : undefined; + const subjectObject = nonEmpty(row[target.subjectObjectField]); + const subjectId = nonEmpty(row[target.subjectIdField]); + if (!subjectObject || !subjectId) { + // A scheduled sweep has no ONE subject, by construction. Nothing to + // derive from, and nothing to invent. + plan.skipped.subjectUnaddressable += 1; + continue; + } + const live = liveByObject.get(subjectObject)?.get(subjectId) ?? null; + const snapshot = target.snapshotField + ? parseSnapshot(row[target.snapshotField], target.snapshotPath ?? []) + : null; + if (!live && !snapshot) { + plan.skipped.subjectNotFound += 1; + continue; + } + // Equivalent to `organizationOf(subjectObject, live, snapshot)` — the + // resolver returns the first non-empty value across its candidates, in + // order — split in two calls only so the row can record WHICH candidate + // answered. Same resolver, same precedence, one resolution. + const fromLive = live ? resolver.organizationOf(subjectObject, live) : null; + const organization = fromLive ?? (snapshot ? resolver.organizationOf(subjectObject, snapshot) : null); + if (!organization) { + // ⛔ Out of the ruling — the subject has no organization either. + plan.skipped.subjectHasNoOrganization += 1; + plan.outOfRulingScopeIds.push(id); + continue; + } + plan.planned += 1; + const statusKey = status == null ? 'unknown' : String(status); + if (target.statusField) plan.plannedByStatus[statusKey] = (plan.plannedByStatus[statusKey] ?? 0) + 1; + parentOrganizations.set(id, organization); + plan.rows.push({ + object: target.object, + id, + organizationField, + organization, + subjectObject, + subjectId, + resolvedFrom: fromLive ? 'live-record' : 'snapshot', + status: status === undefined ? undefined : (status as string | null), + }); + } + + for (const child of target.children ?? []) { + objects.push(await planChild(engine, resolver, child, parentOrganizations, target.object, scanOptions)); + } + } + + return { dryRun: true, objects, totals: totalsOf(objects) }; +} + +/** + * Plan one child table. A child's organization is its PARENT ROW's — never a + * second resolution from the subject, which is what "all three move together" + * means in code. + */ +async function planChild( + engine: BackfillEngine, + resolver: ReturnType, + child: BackfillChild, + plannedParents: ReadonlyMap, + parentObject: string, + scanOptions: { context: unknown; pageSize: number; maxRowsPerObject: number }, +): Promise { + const organizationField = resolver.organizationFieldFor(child.object); + const plan = newObjectPlan(child.object, 'parent-derived', organizationField); + if (!organizationField) { + plan.notes.push(`no organization column resolved for '${child.object}' — nothing scanned.`); + return plan; + } + const rows = await scanUnstampedRows(engine, child.object, organizationField, scanOptions, plan); + plan.scanned = rows.length; + + // Parents this run is NOT already planning have to be read: a child left + // behind by an interrupted run hangs off a parent that already carries its + // organization. + const unknownParents = new Set(); + for (const row of rows) { + const parentId = nonEmpty(row[child.parentField]); + if (parentId && !plannedParents.has(parentId)) unknownParents.add(parentId); + } + const parentRows = unknownParents.size + ? await readById(engine, parentObject, [...unknownParents], scanOptions.context) + : new Map>(); + const parentOrganizationField = resolver.organizationFieldFor(parentObject); + + for (const row of rows) { + const id = rowId(row); + const parentId = nonEmpty(row[child.parentField]); + if (!id || !parentId) { + plan.skipped.subjectUnaddressable += 1; + continue; + } + const organization = plannedParents.get(parentId) + ?? (parentOrganizationField ? nonEmpty(parentRows.get(parentId)?.[parentOrganizationField]) : null); + if (!organization) { + plan.skipped.parentHasNoOrganization += 1; + continue; + } + plan.planned += 1; + plan.rows.push({ + object: child.object, + id, + organizationField, + organization, + subjectObject: parentObject, + subjectId: parentId, + resolvedFrom: 'parent-row', + }); + } + return plan; +} + +function totalsOf(objects: readonly ObjectPlan[]): BackfillReport['totals'] { + return objects.reduce( + (acc, o) => ({ + scanned: acc.scanned + o.scanned, + planned: acc.planned + o.planned, + written: acc.written + o.written, + outOfRulingScope: acc.outOfRulingScope + o.skipped.subjectHasNoOrganization, + }), + { scanned: 0, planned: 0, written: 0, outOfRulingScope: 0 }, + ); +} + +/** + * Render a report as the operator-facing text — broken out per object, which + * is what the ruling asks the dry run to be readable as. + */ +export function formatBackfillReport(report: BackfillReport): string { + const lines: string[] = []; + lines.push( + report.dryRun + ? 'Platform-row organization backfill — DRY RUN (nothing written)' + : 'Platform-row organization backfill — APPLIED', + ); + lines.push('='.repeat(62)); + for (const plan of report.objects) { + lines.push(''); + lines.push(`${plan.object} [${plan.role}]`); + lines.push(` organization column : ${plan.organizationField ?? '(none resolved)'}`); + lines.push(` scanned (unstamped) : ${plan.scanned}`); + lines.push(` ${report.dryRun ? 'would write' : 'written '} : ${report.dryRun ? plan.planned : plan.written}`); + if (Object.keys(plan.plannedByStatus).length) { + const byStatus = Object.entries(plan.plannedByStatus) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([status, n]) => `${status}=${n}`) + .join(' '); + lines.push(` by status : ${byStatus}`); + } + lines.push(` skipped — subject has no organization (OUT OF RULING) : ${plan.skipped.subjectHasNoOrganization}`); + if (plan.outOfRulingScopeIds.length) { + lines.push(` ids : ${plan.outOfRulingScopeIds.join(', ')}`); + } + lines.push(` skipped — no subject named on the row : ${plan.skipped.subjectUnaddressable}`); + lines.push(` skipped — subject gone, no surviving snapshot : ${plan.skipped.subjectNotFound}`); + lines.push(` skipped — parent row has no organization : ${plan.skipped.parentHasNoOrganization}`); + for (const row of plan.rows) { + lines.push( + ` ${row.id} -> ${row.organizationField}=${row.organization}` + + ` (from ${row.resolvedFrom}: ${row.subjectObject}/${row.subjectId}` + + `${row.status ? `, status=${row.status}` : ''})`, + ); + } + for (const note of plan.notes) lines.push(` ⚠️ ${note}`); + } + lines.push(''); + lines.push('-'.repeat(62)); + lines.push( + `TOTAL scanned=${report.totals.scanned} ` + + `${report.dryRun ? 'would-write' : 'written'}=${report.dryRun ? report.totals.planned : report.totals.written} ` + + `out-of-ruling(subject has no organization)=${report.totals.outOfRulingScope}`, + ); + return lines.join('\n'); +} From 29a71d5205434fc6b656bf7401a5e0603e25faf7 Mon Sep 17 00:00:00 2001 From: os-sam Date: Sun, 23 Aug 2026 18:23:36 +0000 Subject: [PATCH 2/3] feat(approvals): write the org-less platform-row backfill, on the plan the dry run printed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The write half, on top of the dry run rather than beside it: `applyPlatformRowOrganizationBackfill` takes the PLAN a human already read and issues one update per planned row carrying its id and its resolved organization column — nothing else on the row, which is what makes the undo expressible as "write NULL back to these ids". `runPlatformRowOrganizationBackfill` defaults to `dryRun: true`; writing is opt-in. A row whose update throws is recorded on its object's plan and the sweep continues — a driver rejecting one row must not cost the other N-1 their repair, and a half-done sweep is safe because the next run picks up exactly what is still unstamped. Idempotency is asserted, not claimed: the suite runs the sweep twice against the same engine and pins the second run at zero planned and zero written, with the deliberately-skipped rows re-REPORTED at the same count. Refs #11308 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- .changeset/backfill-orgless-platform-rows.md | 19 ++++++ ...ackfill-platform-row-organizations.test.ts | 59 ++++++++++++++++ .../backfill-platform-row-organizations.ts | 68 +++++++++++++++++++ 3 files changed, 146 insertions(+) create mode 100644 .changeset/backfill-orgless-platform-rows.md diff --git a/.changeset/backfill-orgless-platform-rows.md b/.changeset/backfill-orgless-platform-rows.md new file mode 100644 index 0000000000..649624d726 --- /dev/null +++ b/.changeset/backfill-orgless-platform-rows.md @@ -0,0 +1,19 @@ +--- +"@objectstack/plugin-approvals": patch +--- + +**Ops:** a one-off, idempotent backfill for the platform rows the pre-#10101 writers stranded with no organization — dry run first (#11308). + +#10101 fixed the WRITERS: a `sys_approval_request` and a `sys_automation_run` are now stamped from the SUBJECT record's organization, with the acting context as the ruled fallback. It wrote nothing to existing rows, so the population produced before it persists — a **pending** org-less approval request LOCKS the record it is about while being invisible in every organization-scoped inbox, its own owner's included, and automation-run history stays unattributed. This is the repair for those rows, on the maintainer's 2026-08-23 ruling (direction 3). + +`packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts` sweeps `sys_approval_request` (with its `sys_approval_action` / `sys_approval_approver` children, which move with their request) and `sys_automation_run`. It scans only rows whose organization column is unset, re-reads each row's subject at repair time — live record first, the write-time snapshot (`payload_json` / `context_json`'s `record`) second for a subject that has since been deleted — and stamps the platform row with the subject's own organization. + +**Dry run first, and by default.** `planPlatformRowOrganizationBackfill(engine)` reads only and returns a per-object report naming every row it would touch; `runPlatformRowOrganizationBackfill(engine, { dryRun: false })` writes. Nothing runs at boot and nothing is scheduled: this is an operator-invoked module, run once against an affected install. + +**Rows whose subject is equally org-less are counted and named, never written.** The acting-context fallback the writers apply is not available to a repair — the acting context is gone — and inventing one stays vetoed. Those ids are reported so the population is checkable and stays visible. + +**`sys_api_key`'s divergence is preserved, not flattened.** Both the column read on a subject and the column written on a platform row are resolved from the registered schema through the shared `createRecordOrganizationResolver` (`@objectstack/metadata-core`), so a platform row ABOUT an API key is repaired from that object's stamp-only `active_organization_id` (limb 0, #8778) and the credential table itself is never written to. + +**Idempotent, and asserted rather than claimed.** Every scan is `WHERE IS NULL` and every write fills that column, so a repaired row cannot match again; the test suite runs the sweep twice and pins the second run at zero writes. + +Publishes no runtime code: the module is not exported from the package index and not bundled into `dist` (`tsup` builds `src/index.ts`). It is graded rather than skipped because the release notes are where an operator of an affected install learns the repair exists, what it will and will not touch, and that the dry run comes first. diff --git a/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts b/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts index bfc6fbd011..f392a2975e 100644 --- a/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts +++ b/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts @@ -24,6 +24,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { planPlatformRowOrganizationBackfill, + runPlatformRowOrganizationBackfill, formatBackfillReport, BACKFILL_TARGETS, type BackfillEngine, @@ -278,3 +279,61 @@ describe('platform-row organization backfill — dry run', () => { expect(text).toContain('out-of-ruling(subject has no organization)=1'); }); }); + +describe('platform-row organization backfill — write', () => { + let engine: FakeEngine; + beforeEach(() => { + engine = makeEngine(strandedFixture()); + }); + + it('stamps exactly the planned rows, and only the resolved column', async () => { + const dry = await planPlatformRowOrganizationBackfill(engine); + const applied = await runPlatformRowOrganizationBackfill(engine, { dryRun: false }); + expect(applied.dryRun).toBe(false); + expect(applied.totals.written).toBe(dry.totals.planned); + expect(engine.updates).toHaveLength(dry.totals.planned); + for (const update of engine.updates) { + expect(Object.keys(update.data).sort()).toEqual(['id', 'organization_id']); + } + expect(engine.tables.sys_approval_request.find(r => r.id === 'areq_1')?.organization_id).toBe('org_A'); + expect(engine.tables.sys_approval_action.find(r => r.id === 'aact_1')?.organization_id).toBe('org_A'); + expect(engine.tables.sys_approval_approver.find(r => r.id === 'aapr_1')?.organization_id).toBe('org_A'); + expect(engine.tables.sys_automation_run.find(r => r.id === 'run_p1')?.organization_id).toBe('org_A'); + }); + + it('⛔ leaves the out-of-ruling rows and the credential table untouched', async () => { + await runPlatformRowOrganizationBackfill(engine, { dryRun: false }); + expect(engine.tables.sys_approval_request.find(r => r.id === 'areq_3')?.organization_id).toBeNull(); + expect(engine.tables.sys_approval_action.find(r => r.id === 'aact_2')?.organization_id).toBeNull(); + expect(engine.updates.some(u => u.object === 'sys_api_key')).toBe(false); + expect(engine.tables.sys_api_key[0].organization_id).toBeNull(); + expect(engine.tables.sys_api_key[0].active_organization_id).toBe('org_K'); + }); + + it('is idempotent: the SECOND run writes zero rows', async () => { + const first = await runPlatformRowOrganizationBackfill(engine, { dryRun: false }); + expect(first.totals.written).toBeGreaterThan(0); + const writesAfterFirst = engine.updates.length; + + const second = await runPlatformRowOrganizationBackfill(engine, { dryRun: false }); + expect(second.totals.planned).toBe(0); + expect(second.totals.written).toBe(0); + expect(engine.updates.length).toBe(writesAfterFirst); + // The rows it deliberately skipped are re-REPORTED, never re-written. + expect(second.totals.outOfRulingScope).toBe(first.totals.outOfRulingScope); + }); + + it('reports a row that failed to write instead of aborting the sweep', async () => { + const dry = await planPlatformRowOrganizationBackfill(engine); + const realUpdate = engine.update.bind(engine); + engine.update = async (object: string, data: any) => { + if ((data as any).id === 'areq_1') throw new Error('driver: constraint violation'); + return realUpdate(object, data); + }; + const applied = await runPlatformRowOrganizationBackfill(engine, { dryRun: false }); + expect(applied.totals.written).toBe(dry.totals.planned - 1); + const plan = planFor(applied, 'sys_approval_request'); + expect(plan.failures.map(f => f.id)).toEqual(['areq_1']); + expect(formatBackfillReport(applied)).toContain('constraint violation'); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts b/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts index 1d7d72dfac..b4d0c269c7 100644 --- a/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts +++ b/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts @@ -221,6 +221,8 @@ export interface ObjectPlan { plannedByStatus: Record; /** Ids skipped as out-of-ruling — reported so the count is checkable, never written. */ outOfRulingScopeIds: string[]; + /** Planned rows whose write threw. Reported, never retried, never fatal. */ + failures: Array<{ id: string; error: string }>; rows: PlannedRow[]; /** Conditions a reader must see, e.g. "this engine exposes no such column". */ notes: string[]; @@ -251,6 +253,12 @@ export interface BackfillOptions { pageSize?: number; /** Hard ceiling per object, so a pathological table cannot spin forever. */ maxRowsPerObject?: number; + /** + * `false` writes. Defaults to `true`: a sweep over existing data that + * defaults to writing is one typo away from an unplanned migration, and the + * ruling puts the dry run first anyway. + */ + dryRun?: boolean; } const SYSTEM_CONTEXT = { isSystem: true, positions: [], permissions: [] }; @@ -279,6 +287,7 @@ function newObjectPlan(object: string, role: ObjectPlan['role'], organizationFie skipped: emptySkips(), plannedByStatus: {}, outOfRulingScopeIds: [], + failures: [], rows: [], notes: [], }; @@ -626,6 +635,9 @@ export function formatBackfillReport(report: BackfillReport): string { + `${row.status ? `, status=${row.status}` : ''})`, ); } + for (const failure of plan.failures) { + lines.push(` ✗ ${failure.id} NOT written — ${failure.error}`); + } for (const note of plan.notes) lines.push(` ⚠️ ${note}`); } lines.push(''); @@ -637,3 +649,59 @@ export function formatBackfillReport(report: BackfillReport): string { ); return lines.join('\n'); } + +/** + * Write the plan. Each planned row gets ONE update carrying its id and its + * resolved organization column — nothing else on the row is touched, which is + * what makes the undo expressible as "write NULL back to these ids". + * + * A row whose write throws is RECORDED and the sweep continues: a driver + * rejecting one row must not cost the other N-1 their repair, and a half-done + * sweep is safe here precisely because the next run picks up exactly what is + * still unstamped. + * + * ⛔ Takes a plan rather than building one, so the rows written are the rows a + * human read in the dry run — not a fresh scan that may have moved. + */ +export async function applyPlatformRowOrganizationBackfill( + engine: BackfillEngine, + plan: BackfillReport, + options: BackfillOptions = {}, +): Promise { + const context = options.context ?? SYSTEM_CONTEXT; + for (const objectPlan of plan.objects) { + objectPlan.written = 0; + objectPlan.failures = []; + for (const row of objectPlan.rows) { + try { + await engine.update( + objectPlan.object, + { id: row.id, [row.organizationField]: row.organization }, + { context }, + ); + objectPlan.written += 1; + } catch (err) { + objectPlan.failures.push({ id: row.id, error: String((err as Error)?.message ?? err) }); + } + } + } + return { dryRun: false, objects: plan.objects, totals: totalsOf(plan.objects) }; +} + +/** + * Plan, then (unless `dryRun`) write — the whole sweep in one call. + * + * Idempotent by construction rather than by a guard: the plan is built from + * `WHERE IS NULL`, and every write fills that column, so + * a second call over an unchanged database plans nothing and writes nothing. + * `backfill-platform-row-organizations.test.ts` asserts that second run rather + * than describing it. + */ +export async function runPlatformRowOrganizationBackfill( + engine: BackfillEngine, + options: BackfillOptions = {}, +): Promise { + const plan = await planPlatformRowOrganizationBackfill(engine, options); + if (options.dryRun !== false) return plan; + return applyPlatformRowOrganizationBackfill(engine, plan, options); +} From 43f1ffe9bc30d5ee528289a184b98d925ffc1203 Mon Sep 17 00:00:00 2001 From: os-sam Date: Sun, 23 Aug 2026 18:40:48 +0000 Subject: [PATCH 3/3] test(approvals): hold the backfill's fake engine to the platform's own write contracts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gate findings, both real and both about the DOUBLE rather than the sweep: - its `update()` was looser than `ObjectQL.update` — now opened with `assertEngineUpdateDispatch(data, options)` from `@objectstack/metadata-core`, and the pinned ledger learns the file; - its WHERE matcher read a `$`-combinator as a field name, and sat inside a closure the conformance gate could not lift. It now refuses the combinators it does not implement, at module scope where the gate can judge it. Refs #11308 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- ...ackfill-platform-row-organizations.test.ts | 53 ++++++++++++------- scripts/engine-double-contract.pinned.json | 5 ++ 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts b/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts index f392a2975e..d62c8d110c 100644 --- a/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts +++ b/packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts @@ -22,6 +22,7 @@ */ import { describe, it, expect, beforeEach } from 'vitest'; +import { assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { planPlatformRowOrganizationBackfill, runPlatformRowOrganizationBackfill, @@ -61,24 +62,34 @@ interface FakeEngine extends BackfillEngine { failUpdates: boolean; } +/** + * The double's WHERE matcher, at module scope and closing over nothing — it is + * judged on its own by `scripts/check-where-matcher-conformance.mjs`, and a + * matcher it cannot lift is a matcher nobody has checked. + */ +function matches(row: Row, where: any): boolean { + if (!where || typeof where !== 'object') return true; + for (const [key, expected] of Object.entries(where)) { + // REFUSE the combinators this double does not implement, rather than + // reading `$or` as a field name and silently matching nothing — the sweep + // must never pass a filter shape its fixture answers wrongly. + if (key.startsWith('$')) throw new Error(`fake engine: unsupported WHERE combinator '${key}'`); + const actual = row[key] ?? null; + if (expected && typeof expected === 'object' && '$in' in (expected as any)) { + if (!(expected as any).$in.includes(actual)) return false; + continue; + } + if (expected === null) { + if (actual !== null && actual !== undefined) return false; + continue; + } + if (actual !== expected) return false; + } + return true; +} + function makeEngine(tables: Record, opts: { withSchema?: boolean } = {}): FakeEngine { const withSchema = opts.withSchema !== false; - const matches = (row: Row, where: any): boolean => { - if (!where || typeof where !== 'object') return true; - for (const [key, expected] of Object.entries(where)) { - const actual = row[key] ?? null; - if (expected && typeof expected === 'object' && '$in' in (expected as any)) { - if (!(expected as any).$in.includes(actual)) return false; - continue; - } - if (expected === null) { - if (actual !== null && actual !== undefined) return false; - continue; - } - if (actual !== expected) return false; - } - return true; - }; const engine: FakeEngine = { tables, updates: [], @@ -97,7 +108,11 @@ function makeEngine(tables: Record, opts: { withSchema?: boolean const limit = typeof options?.limit === 'number' ? options.limit : rows.length; return rows.slice(offset, offset + limit).map(r => ({ ...r })); }, - async update(object: string, data: any) { + async update(object: string, data: any, options?: any) { + // Hold the double to ObjectQL.update's own dispatch contract — a fake + // looser than the engine is how a write path ships dead with its suite + // green (scripts/check-engine-double-contract.mjs). + assertEngineUpdateDispatch(data, options); if (engine.failUpdates) throw new Error(`fake engine: update('${object}') must not be reached in a dry run`); engine.updates.push({ object, data: { ...data } }); const table = tables[object] ?? []; @@ -326,9 +341,9 @@ describe('platform-row organization backfill — write', () => { it('reports a row that failed to write instead of aborting the sweep', async () => { const dry = await planPlatformRowOrganizationBackfill(engine); const realUpdate = engine.update.bind(engine); - engine.update = async (object: string, data: any) => { + engine.update = async (object: string, data: any, options?: any) => { if ((data as any).id === 'areq_1') throw new Error('driver: constraint violation'); - return realUpdate(object, data); + return realUpdate(object, data, options); }; const applied = await runPlatformRowOrganizationBackfill(engine, { dryRun: false }); expect(applied.totals.written).toBe(dry.totals.planned - 1); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index e92c29611a..a4a28999e9 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1196,6 +1196,11 @@ "verb": "delete", "pinned": 1 }, + { + "file": "packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/plugins/plugin-approvals/src/manager-approver-org-screen.test.ts", "verb": "delete",