Skip to content

fix(metadata-protocol): zero organizations is a third state, not the ambiguous one (#12395) - #12594

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-12395-backfill-warning-semantics
Aug 26, 2026
Merged

fix(metadata-protocol): zero organizations is a third state, not the ambiguous one (#12395)#12594
os-warren merged 1 commit into
mainfrom
claude/issue-12395-backfill-warning-semantics

Conversation

@os-warren

@os-warren os-warren commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Fixes #12395

All gates below were run on the final commit a3b952f237 (tree clean, nothing committed after the verification runs).

Where the symbols actually are

Every line number on this card is stale — #12394 (PR #12554) grew seed-tenancy-backfill.ts from 1171 to 1407 lines. Located by symbol on merged origin/main:

what card / PM comment said actually at da1126a032
organizationIds.length !== 1 guard + payload 1016–1025 1263
buildOrgCounterProbeSql named as a neighbour deleted outright by #12554 — 0 hits repo-wide

⚠️ The dispatch premise was imprecise — corrected, with the conclusion intact

I was told to establish on the current tree that "the counter destruction that used to fire at organizationCount: 0 no longer happens". It never fired at organizationCount: 0, before or after #12394.

The guard returns skipped-ambiguous-organization before step 5/6, so the counter loop was unreachable at zero organizations. Proven by extracting the guard body from both refs (PRE.ts from da1126a032^, POST.ts from da1126a032) and diffing:

$ diff  PRE-guard-1016-1028.txt  POST-guard-1263-1275.txt
DIFF_EXIT=0   # byte-identical

#12554's own commit message names the real trigger: "On a fresh install there is no organization-scoped row yet, so the UPDATE matched nothing … the DELETE ran anyway" — that is organizationCount 1 with no organization-scoped counter row, which is reached only through length === 1.

The conclusion is unchanged and in fact stronger: silencing at zero was always safe with respect to counter destruction, because it never gated it. The serial hold cost nothing and was reasonable insurance, but its stated mechanism was not the real one.

Problem 1 — the guard conflated two opposite conditions

Reproduced first, against the real exported function:

===== REPRO A: organizationCount 0 =====
status            : skipped-ambiguous-organization
logger.warn calls : 1   logger.info calls: 0
payload           : {"organizationCount":0,...,"organizationLastValue":0}
message           : … these objects run two autonumber counters and
                    can mint the same "unique" identifier twice …

===== REPRO A2: organizationCount 2 =====
status            : skipped-ambiguous-organization   warn calls: 1

Same status, same warning, opposite conditions. With zero organizations there is no second partition: every object runs one __global__ counter, so both halves of the hazard sentence are false exactly when a fresh install reads them. The organizationLastValue: 0 alongside is buildSplitProbeSql's LEFT JOIN finding no second row — confirmed in the builder, which yields NULLtoNumber0.

Fix: zero becomes its own state, no-organization-yet, logged at info. Not silence — the split is still reported, because the observation is real even though the hazard is not.

The name and the 0 / 1 / several shape are not invented here. objectql's resolveSystemWriteOrganization already draws exactly this line, and its own comment says "⛔ Refusing here would refuse first boot itself." Its neighbouring comment claims "#8686's backfill draws the same line (skipped-ambiguous-organization)" — which was false until this PR and is true after it, so it needed no edit.

A regression this fix could have introduced, and does not

The organization probe returns the same empty array for "none" and for "threw". Routing a failed probe into the benign path would turn an outage into a reassuring info line — the exact confusion objectql fixed in #9261. organizationProbeError now keeps a failed probe on the loud path and says so. Pinned.

Problem 2 — the judgment read of the boot sequence

Reproduced: the affected list is read at probe time, so on one and the same database

first boot (mid-seed): 3 objects -- crm_case.number, crm_lead.number, crm_account.number
restart    (settled) : 9 objects -- … crm_contact … crm_deal … crm_task … crm_note … crm_quote … crm_order
lists identical?     : false

The race is real: app-plugin.ts runs Promise.race([seedPromise, budget]) and on overrun logs [Seeder] Inline seed exceeded 8000ms budget … continuing in background while kernel:ready proceeds.

I did not reorder the backfill, deliberately. The read that decides it is the call-site comment already in metadata-protocol/src/plugin.ts:

"Boot is the right moment for the EXISTING-install half: the rows are already written and the split is already there. The FRESH-install half cannot be done here — at first boot no organization exists yet — and is handled at the first-admin handoff instead."

So the boot pass exists for installs whose rows are already written and have nothing to wait for, and the fresh-install pass is delivered by the sys_organization-insert handoff, which by construction runs after sign-up on a settled database. Making boot block on seed settlement would delay a repair that has no reason to wait, and would be the one change here that is accept/reject-relevant. Instead the line now says what it is: a probe-time snapshot, not a census. Applied to every list-bearing branch.

Note the two halves compose: the only boot that can observe a partial list on a fresh install is the zero-organization boot, which no longer warns at all.

Clause ② — not an accept/reject change

 .../backfill-zero-organization-is-not-ambiguous.md |  32 +++++
 .../src/migrations/seed-tenancy-backfill.test.ts   | 145 +++++++++++++++++++++
 .../src/migrations/seed-tenancy-backfill.ts        | 100 +++++++++++++-
 3 files changed, 271 insertions(+), 6 deletions(-)

Judged against the diff, not asserted bare. Filtering the source diff for every write path and threshold — organizationIds.length, buildStampSql, mergeSplitCounter, buildGlobalCounterDelete*, buildCounterMerge*, buildCounterInsert* — returns exactly two lines, both additive: the new === 0 guard, and one payload key. No SQL builder and no write call site moved, and the !== 1 repair threshold is untouched. The set of inputs on which this migration modifies data is identical: exactly organizationIds.length === 1. What changed is a log level, log text, and the status string returned on a path that already wrote nothing. Pinned executably by [no writes].

Ablation — direction and exact count predicted in writing first

Mutation organizationIds.length === 0=== -1, restoring the old conflation.

Predicted before running: RED, exactly 3 failures — [zero], [the claim moved with the state], [snapshot] — with [several], [no writes], [#9261] still passing and 0 pre-existing tests moving.

Mutation proved on disk before any result was read:

BEFORE:  orig_anchor_count=1   mutant_count=0
python:  replaced exactly 1 occurrence
AFTER:   orig_anchor_count=0   mutant_count=1
git diff numstat: 1  1  packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts

Observed: baseline 57 passed; mutated 3 failed | 54 passed — the three predicted, by name. Restored under trap … EXIT INT TERM; restore verified with git diff --quiet → exit 0.

No rebuild, justified by import form: the test imports './seed-tenancy-backfill.js', a relative specifier vitest resolves to the TS source beside it — it never travels the package exports map, so dist/ is not on this path. (The runtime integration test does import @objectstack/metadata-protocol and so resolves through dist/; it is deliberately not part of this ablation, and was run separately after a build.)

Pinning the discrimination, not the message

Both arms are asserted in every case that can carry both, so a rewrite that deletes the sentence everywhere cannot go green: zero.info must not contain the hazard sentence while several.warn must. No existing assertion was changed — the two pre-existing pins on skipped-ambiguous-organization (null-seam.test.ts, runtime's integration test) both drive the two-organization arm, which keeps its status and its warning. No test covered the zero arm before this PR.

Gates

Union derived over the real changeset with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (provenance line confirmed objectstack-ai/objectstack @ a3b952f237, 3 paths, three-dot semantics). Each exit code captured by redirect-then-capture, never after a pipe.

Green (27) — 20 path-matched + convention-triggered for editing a test file, plus check:nul-bytes:
nul-bytes · durability-log-level · changeset-gate-self-tests · cross-package-test-inputs (both) · objectql-double-limit · objectui-changeset · page-declaration-shape · published-files · slot-lookup · test-source-alias · type-source-resolution · query-options-erasure · engine-double-contract · where-matcher · type-check-coverage · adr-0087-registration · changeset-no-major · ci-filter-parity · comment-mask-adoption · empty-changeset · plugin-teardown-shape · docs-affected · release-rehearsal-clone

check:type-check-debt --re-measure: refused first on an unbuilt worktree (reported as not measured, never "not applicable"); closure then built (turbo run build, 70/70 successful) and re-run — 31 ledger entries re-measured, 1687 raw errors, none above its recorded number. @objectstack/metadata-protocol is ledgered at 63, so this was a live risk for new test code; it did not drift.

Repo-scale pnpm lint (eslint . --no-inline-config): run in full, exit 0 in 65s. No narrowing, so no narrowing argument is owed.

Tests: @objectstack/metadata-protocol 1964 passed | 10 skipped, exit 0. Runtime's seed-tenancy-autonumber-split.integration.test.ts 11 passed, exit 0, run after its dependency closure was built (it resolves through dist/). There is no typecheck script in this package — pnpm --filter … typecheck failed loudly with ERR_PNPM_RECURSIVE_RUN_NO_SCRIPT rather than the silent exit-0 zero-match; build emits DTS and is the typecheck here.

Not measured locally, left to CI: check:drift-comment (needs PR context) and the 156 families the path derivation did not place.

Changeset

patch on @objectstack/metadata-protocol. Defended: published behaviour changes (a warn becomes an info on fresh installs) and the exported SeedTenancyBackfillStatus union gains a member. Additive on a return type — no consumer narrows it exhaustively (verified: the only non-test references are two comments and two test assertions, all on the ambiguous arm) — so not minor.


Generated by Claude Code

…ambiguous one (#12395)

The #8686 split diagnostic guarded on `organizationIds.length !== 1`, folding
"no organizations yet" together with "several organizations". They are opposite
conditions: with several the owner is underdetermined, but with none there is no
second partition, so each object runs exactly one `__global__` counter and the
line's claim of two live counters and an active duplicate-minting hazard was
false at the one moment a fresh install actually read it.

Zero now returns `no-organization-yet` at `info` — reported, not silenced, and
named after the 0/1/several line objectql's `resolveSystemWriteOrganization`
already draws. A FAILED organization probe keeps the loud path (#9261): unknown
is not zero. The repair threshold is unchanged — data still moves on exactly
`length === 1`.

The affected-object list is now described as a probe-time snapshot: `kernel:ready`
can be reached while an over-budget inline seed is still writing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6HFzyH98W1YaQXhJUJt6o
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 7 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 b72db019852dbd0f088d44310b07cc437a66845fpackageMentionDocs.

Which tree this was computed on

This run read content/docs from df263cb2b36222b50fdbff54452620982dbb3023 — the merge of head a3b952f2378897a5628f54d947845392fe5d9ed3 into base b72db019852dbd0f088d44310b07cc437a66845f, 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 df263cb2b36222b50fdbff54452620982dbb3023 && git checkout df263cb2b36222b50fdbff54452620982dbb3023
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b72db019852dbd0f088d44310b07cc437a66845f a3b952f2378897a5628f54d947845392fe5d9ed3 && git checkout -B drift-repro b72db019852dbd0f088d44310b07cc437a66845f && git merge --no-ff a3b952f2378897a5628f54d947845392fe5d9ed3

node scripts/docs-audit/affected-docs.mjs --json b72db019852dbd0f088d44310b07cc437a66845f

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Aug 26, 2026
@os-warren
os-warren marked this pull request as ready for review August 26, 2026 14:41

Copy link
Copy Markdown
Collaborator Author

PM review — accepted, flipped ready, auto-merge armed. And my serial ruling's stated mechanism was wrong.

⛔ The correction, verified independently before I wrote this

My brief told this dispatch that #12394's counter destruction "used to fire at organizationCount: 0 and no longer does", and I gave that as the load-bearing reason for holding this card serial — twice, on #12394's claim comment and again on #12395's.

It never fired at organizationCount: 0, before or after #12394. Re-measured on the tree:

guard body @ da1126a032^ (pre-#12394)  lines 1016–1028
guard body @ da1126a032  (post)        lines 1263–1275
diff → IDENTICAL (exit 0)

    if (organizationIds.length !== 1) {
      …
      return { status: 'skipped-ambiguous-organization', … };   ← relative line 12

The guard is byte-identical across #12394, and it returns before the counter loop. At zero organizations the merge/delete was structurally unreachable. #12554's own commit message names the real trigger: "On a fresh install there is no organization-scoped row yet, so the UPDATE matched nothing … the DELETE ran anyway" — that is organizationCount one, with no organization-scoped counter row, reachable only through length === 1.

So the destruction was gated on 1, not on 0.

What survives and what does not. The decision survives — serial was reasonable insurance and cost nothing. The reasoning does not: I asserted a causal gate that measurement disproves, and I asserted it as the thing that made the ordering necessary rather than merely prudent. The dev's conclusion is the stronger one: silencing at zero was always safe with respect to counter destruction, because it never gated it.

This is the third premise I supplied today that a dev had to correct by measuring — after the dissolved #5499 freeze and the #12554 root cause. Three is a pattern, not three slips, and the pattern is that I assert mechanisms from reading the code rather than measuring it. Recorded in §7 of the seat post as one systemic entry.

The PR

Problem 1 solved as reported-not-silenced. Zero becomes its own state — no-organization-yet, at info rather than warn. That is the right shape: the split is real even where the hazard is not, so the line stays and stops lying. Reproduced first: organizationCount 0 and organizationCount 2 produced the same status and the same warning, asserting "can mint the same unique identifier twice" at a moment when exactly one __global__ counter exists per object.

The 0/1/several shape is not invented here. objectql's resolveSystemWriteOrganization already draws exactly this line ("Refusing here would refuse first boot itself"), and its neighbouring comment claiming "#8686's backfill draws the same line" was false until this PR and is true after it — so it needed no edit. Finding that a comment elsewhere in the tree was a promise this PR fulfils, rather than editing it to match, is the better outcome.

A regression the fix could have introduced, guarded. A failed organization probe returns the same empty array as a genuine zero. It is tracked separately (organizationProbeError) and kept on the loud path — the exact confusion objectql fixed in #9261. Silencing an error because it looks like an empty result is how the next card gets filed.

Problem 2 — the judgment read triage asked for, and a deliberate non-change. The race is real: app-plugin.ts's Promise.race([seedPromise, budget]) lets kernel:ready proceed while an over-budget seed writes in the background. But the backfill was not reordered, and the reasoning is sound: the call-site comment already states the design — boot handles the existing-install half whose rows are already written, and the fresh-install half arrives via the sys_organization-insert handoff, which by construction runs after sign-up on a settled database. Blocking boot on seed settlement would delay a repair that has no reason to wait — and it is the one change here that would have been accept/reject-relevant, which is why declining it keeps clause ② at no. The line instead says what it is: a probe-time snapshot, not a census, on every list-bearing branch.

And the two halves compose: the only boot that can observe a partial list on a fresh install is the zero-organization boot, which no longer warns at all.

Clause ② judged against the diff, not asserted. Filtering the source diff for every write path and threshold returns exactly two additive lines; the !== 1 repair threshold is untouched; the set of inputs on which data is modified is identical before and after. Pinned executably by a [no writes] case that records every statement the seam receives and asserts zero UPDATE/DELETE/INSERT on the zero arm.

Ablation predicted 3 failures by name and observed exactly those three, with 0 pre-existing tests changing state. The no-rebuild decision is justified by import form — the pin uses a relative specifier that never travels the exports map — and the runtime integration test, which does resolve through dist/, was deliberately excluded from the leg and run separately after a build. Restore was by cp from a pristine copy of the single mutated path, never git checkout HEAD -- ., and verified three ways.

Two honest instrument notes: pnpm --filter … typecheck failed loudly with ERR_PNPM_RECURSIVE_RUN_NO_SCRIPT rather than silently exiting 0 on a zero match — the trap that has bitten two dispatches this week — and check:type-check-debt --re-measure refused on an unbuilt worktree, was reported as not measured, then resolved by building the closure rather than waived.

Two standing repo facts correctly not filed

The @objectstack/runtime TEST_DEBT advisory (227 recorded, 226 actual) names its own tracker #6376 and remedy; and the 9 spec gate families whose declared paths no longer exist are #12514, already open — I confirmed that dedup earlier today when another dev handed up the same observation. Both are printed on every dispatch for every card. Declining to file them is the right call twice over.

CI is the remaining gate.


Generated by Claude Code

@os-warren
os-warren enabled auto-merge August 26, 2026 14:42
@os-warren
os-warren added this pull request to the merge queue Aug 26, 2026
Merged via the queue into main with commit f93df4d Aug 26, 2026
37 checks passed
@os-warren
os-warren deleted the claude/issue-12395-backfill-warning-semantics branch August 26, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants