Skip to content

Backfill the org-less platform rows the #10101 writers no longer produce — dry run first, idempotent - #11439

Merged
os-sam merged 3 commits into
mainfrom
claude/issue-11308-backfill-orgless-platform-rows
Aug 23, 2026
Merged

Backfill the org-less platform rows the #10101 writers no longer produce — dry run first, idempotent#11439
os-sam merged 3 commits into
mainfrom
claude/issue-11308-backfill-orgless-platform-rows

Conversation

@claude

@claudeclaudeBot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes#11308

The one-off, idempotent backfill for the platform rows the pre-#10101 writers stranded with no organization — on the maintainer's 2026-08-23 ruling (#11308 (comment)5386673394), direction 3: derive organization_id from the SUBJECT record, only for rows whose subject has an organization, dry-run output first, row counts checkable in the same window as cloud#1395's acceptance leg.

⚠️ Historical rows only. The write path already landed in #11311 and is not touched here.

The carrier

packages/plugins/plugin-approvals/src/backfill-platform-row-organizations.ts — a module, its test, and a changeset. No CLI command, no public export, no durable API surface: it is not exported from the package index, tsup builds src/index.ts so it is not in dist, files does not publish src, and nothing runs at boot or on a schedule. The only other file in the diff is the line scripts/check-engine-double-contract.mjs --write added to its own pinned ledger. Clause-② stays no.

Dry run first — and it exists first

planPlatformRowOrganizationBackfill(engine) reads only and returns a per-object report; applyPlatformRowOrganizationBackfill takes that plan and writes it. runPlatformRowOrganizationBackfill defaults to dryRun: true, so writing is opt-in.

The two halves also landed as two commits in that order (bb61306f5c plan-only, then 29a71d5205 write), because for a script that touches existing data the dry run has to be what defines the plan rather than a view retrofitted onto a write path.

Per object the report gives: the schema-resolved organization column, scanned (rows with that column unset), what it would write with each row's id, subject and provenance, a per-status breakdown, and each skip reason counted separately.

What it derives, and from where

  • Subject precedence: live record first, write-time snapshot second — the same order openNodeRequest resolves with. A repair exists to put the row behind the wall its subject is behind now; the snapshot (payload_json; context_json's record for a paused run) is the fallback for a subject that has since been deleted. Terminal sys_automation_run rows carry no context blob, so they resolve from the live record or not at all. Every planned row records which candidate answered.
  • ⛔ Rows whose subject is equally org-less are counted and named, never written. The writers' acting-context fallback is not available to a repair — the acting context is gone — and inventing one is the option that stayed vetoed. Their ids are in the report so the population is checkable.
  • sys_api_key's divergence is preserved, not flattened. Both the column read on a subject and the column written on a platform row come from the shared createRecordOrganizationResolver (@objectstack/metadata-core) — never hard-coded to organization_id. A platform row about an API key is repaired from that object's stamp-only active_organization_id (limb 0, spec: audit stamping needs a read-neutral organization declaration — tenancy.tenantField cannot serve sys_api_key without walling the credential table (#8707 remainder) #8778), and the credential table is never written to.
  • Children move with their request.sys_approval_action and sys_approval_approver take their parent row's organization, never a second resolution from the subject. Sweeping them by their own null also makes an interrupted run self-completing.

The automation half — decided explicitly, as the card asked

sys_automation_run is swept in every status. The retention facts are real (maxAge: '30d', onlyWhen: status in [completed, failed], so terminal history ages out and paused rows never do), but the ruling's criterion is the subject's organization, not the row's status, and a status carve-out would narrow it. Retention is also a configurable sweep — an install with it disabled keeps those rows forever — and 30 days is precisely the window an operator investigating this defect reads. The retention fact stays visible instead: the report breaks the plan out by status, so a reader sees exactly how many of the writes are to rows that will age out anyway.

Idempotency — tested, not asserted

Every scan is WHERE <organization column> IS NULL and every write fills that column, so a repaired row cannot match again. The suite runs the sweep twice against the same engine and pins the second run at planned === 0 and written === 0, with the deliberately-skipped rows re-reported at the same count.

Rollback posture

Not "there are backups". The specific undo:

  1. Keep the applied report. It names every { object, id } that was written, per object. That output is the undo list — capture it before anything else.
  2. Write null back to exactly those ids, one field per row:
    for(constplanofreport.objects)for(constrowofplan.rows.slice(0,plan.written))awaitengine.update(plan.object,{id: row.id,[row.organizationField]: null},{context: SYSTEM_CTX});
  3. Why that is complete, not approximate. The sweep only ever selects rows whose organization column was already unset, so null is the exact prior value of every row it wrote — there is no earlier organization to remember. Each write carries only { id, <organization column> }, so no other column on the row moved, and a row that already carried an organization was never scanned. Subject records are read and never written; sys_api_key is never written at all.
  4. Undoing does not re-open a growing population.Promote resolveRecordOrganizationField to the shared platform-row resolver: approvals + automation runs stamp the SUBJECT record's organization (cloud#1395 Option A) #11311 already fixed the writers, so the stranded set is closed and historical: a rollback restores exactly the pre-sweep state of a set that is no longer growing.
  5. A row whose write throws is recorded on its object's plan (failures) and the sweep continues — a partial run is safe because the next run picks up exactly what is still unstamped, and the undo list is still just what written names.

Verifying row counts in cloud#1395's acceptance window

  • Before: each object's scanned is the organization_id IS NULL count the acceptance leg reads.
  • After: re-run the dry run. Per object, planned must be 0, and scanned must equal subjectHasNoOrganization + subjectUnaddressable + subjectNotFound — the rows the ruling deliberately leaves alone. The drop in scanned between the two runs equals written.

⚠️The live population was NOT re-measured from this seat, and the card's 9/9 + 12/12 must not be copied forward. No cloud#1395 database, checkout or credential is reachable from this container (no DATABASE_* env, no sibling cloud worktree), so this PR states no row count of its own. The dry run is the re-measurement instrument, and it must be run against the affected install before the write. This PR deliberately makes no claim that any particular row class is empty.

Verification

Union re-derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no hand-supplied paths) on the final change set, at 43f1ffe9bc. Exit captured before any pipe; each verdict quoted from the gate's own line.

checkverdict
pnpm --filter @objectstack/plugin-approvals testTest Files 31 passed (31) · Tests 585 passed (585)
pnpm --filter @objectstack/plugin-approvals typecheckVERDICT command-exit 0
pnpm lint (repo-wide, eslint . --no-inline-config)VERDICT command-exit 0
check:engine-double-contractOK — 389 pinned, 133 in the DEBT ledger, 2 exempt
check:where-matcher287 matcher(s): 286 conforming … 0 silently wrong, 0 unjudged
check:type-check-debt (--re-measure, closure built)OK — 33 ledger entr(ies) re-measured … none above its recorded number
check:type-check-coverageOK — 65/78 workspace packages type-checked
check:i18n (after turbo run build --filter=@objectstack/cli)OK (9 package(s) — all bundles in sync)
check:published-files, check:slot-lookup, check:test-source-alias, check:type-source-resolution, check:cross-package-test-inputs, check:query-options-erasure, check:changeset-gate-self-tests, check:objectui-changeset, check:entry-guard, check:parse-guard, check:pnpm-filter-targets, check:nul-bytesexit 0
check-adr-0087-registration, check-changeset-no-major, check-ci-filter-parity, check-empty-changeset, check-plugin-teardown-shape, docs-audit/check-affected-docsexit 0

Two gates found real defects in the test double and were repaired rather than baselined: its update() was looser than ObjectQL.update (now opens with assertEngineUpdateDispatch), and its WHERE matcher read a $-combinator as a field name inside a closure the conformance gate could not lift (now refuses combinators, at module scope).

Ablations

Both mutations were confirmed on disk by anchored grep -F in both directions, restored byte-identically (git hash-object equal to the baseline both times), under trap restore EXIT INT TERM. No rebuild was needed and the reason is stated rather than assumed: the suite imports the mutated module by relative path, so vitest resolves it from src; only @objectstack/metadata-core comes from dist, and ablation A removes the call into it.

A — hard-code the organization column (the "unify everything onto one organization field" intuition). Markers 2 → 0, deleted text 0 → 2. Predicted 3 failures; observed 4, and the composition differed — reporting what happened, not the prediction. I expected the out-of-ruling count to rise 1 → 2 and the formatter's …=1 line to fail. Instead the count stayed 1 and the identity inverted: outOfRulingScopeIds became ['areq_4'] instead of ['areq_3']. The hard-coded read flattens two limbs at once, not one — it loses sys_api_key's active_organization_id (limb 0, so areq_4 drops out of scope) and it fabricates org_C for an ADR-0066 tenancy.enabled: false subject (limb 1, so areq_3 is wrongly swept in). The knock-on failures were moves the action / approver children … (a child inherits the fabricated org) and ⛔ leaves the out-of-ruling rows … untouched (expected 'org_C' to be null). This is why the test asserts the ids and not just the count: a count-only assertion would have sat green through a total inversion of which rows are correct.

B — drop the IS NULL scan guard. Markers 1 → 0, deleted text 0 → 1. Predicted 2 failures, observed exactly 2: is idempotent: the SECOND run writes zero rows and never scans a row that already carries an organization.

Reverse control for the one zero-hit this PR relies on ("the new test file adds no type-layer errors"): the same run and the same anchor family reported 347 errors across the package's other test files, so the pattern is demonstrably able to match. check:type-check-debt --re-measure then confirmed the ledger did not move.

Generated by Claude Code


Generated by Claude Code

os-samand others added 3 commits August 23, 2026 18:21
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
…n the dry run printed
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
…n write contracts
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals, touching 66 documentable anchor(s).

26 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json d0e3a885b890e89d6d47ea5c70ae15b0a4a5d139.

4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 15 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json d0e3a885b890e89d6d47ea5c70ae15b0a4a5d139packageMentionDocs.

Which tree this was computed on

This run read content/docs from 9f843ec35a6dbbe4ebc7ca946906a121d78f6e21 — the merge of head 43f1ffe9bc30d5ee528289a184b98d925ffc1203 into base d0e3a885b890e89d6d47ea5c70ae15b0a4a5d139, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9f843ec35a6dbbe4ebc7ca946906a121d78f6e21 && git checkout 9f843ec35a6dbbe4ebc7ca946906a121d78f6e21
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d0e3a885b890e89d6d47ea5c70ae15b0a4a5d139 43f1ffe9bc30d5ee528289a184b98d925ffc1203 && git checkout -B drift-repro d0e3a885b890e89d6d47ea5c70ae15b0a4a5d139 && git merge --no-ff 43f1ffe9bc30d5ee528289a184b98d925ffc1203
node scripts/docs-audit/affected-docs.mjs --json d0e3a885b890e89d6d47ea5c70ae15b0a4a5d139

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs d0e3a885b890e89d6d47ea5c70ae15b0a4a5d139 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 23, 2026
@os-sam
os-sam marked this pull request as ready for review August 23, 2026 19:22
@os-sam
os-sam added this pull request to the merge queueAug 23, 2026
Merged via the queue into main with commit 5a916c4Aug 23, 2026
32 checks passed
@os-sam
os-sam deleted the claude/issue-11308-backfill-orgless-platform-rows branch August 23, 2026 19:31
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/xlteststooling

Projects

None yet

1 participant

@os-sam