Skip to content

feat(cli): os migrate duplicates — report the business identifiers already minted twice by the tenancy split - #9383

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-8928-duplicate-identifier-report
Aug 17, 2026
Merged

feat(cli): os migrate duplicates — report the business identifiers already minted twice by the tenancy split#9383
os-zhuang merged 2 commits into
mainfrom
claude/issue-8928-duplicate-identifier-report

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#8928

os migrate duplicates — a read-only, operator-facing inventory of the business identifiers the seed/API tenancy split already minted twice. Implemented to the 2026-08-16 maintainer ruling, which settled all five of the card's decision points; this PR is the implementation of that ruling, not a new reading of it.

What the five ruled points look like in code

RuledWhere it lives
1. CLI door only, beside os migratepackages/cli/src/commands/migrate/duplicates.tsos migrate duplicates, a sibling of the family's other read-only data command, os migrate value-shapes. No REST route, no boot hook.
2. The narrow definition of duplicateA value held by rows in more than one of the partitions COALESCE(organization_id, '__global__') separates (ADR-0120 D3). A value repeated inside one partition is refused by the partitioned unique index and is deliberately not reported — pinned by a fixture row that holds REF-1 twice in the same organization and does not appear in the report.
3. JSON to stdout, no persistenceOutput is always the JSON document; there is no --json flag and no human renderer, so there is one contract rather than two. No new schema, no rows written, nothing left behind.
4. The live condition tooliveConditions — every object/field still running a __global__ counter beside an organization-scoped one. An INNER JOIN, deliberately: "beside" is the whole claim (the backfill's own probe is a LEFT JOIN because it triggers a repair, which must also fire where no organization-scoped counter exists yet).
5. Data-side probeGROUP BY field HAVING COUNT(*) > 1 over the object's own table, narrowed to the cross-partition case. _objectstack_sequences appears in exactly one probe — the live condition of point 4, which is a fact about counters and lives nowhere else. A duplicate whose counter was since merged is still found.

Binding from the #8844 ruling and honoured throughout: report, do not rewrite. Nothing here renumbers, deduplicates or moves a row.

The timing note, measured rather than asserted

The ruling's perishability clause is why the card exists: organization_id = NULL is the marker saying "this row came from the untenanted side", and it is exactly what the #8686 repair overwrites. Two things were measured rather than reasoned about.

The command applies nothing — boot included. It boots read-only (deferSchemaDdl + readOnlyProbe, the same boot os migrate plan uses: no DDL, no seed, no database file brought into existence) and issues SELECTs only. duplicates.integration.test.ts boots the real standalone stack over a fixture carrying the damage and asserts the database is byte-identical afterwards, read through a connection of the test's own.

What the repair destroys, and what it does not.duplicates.pre-repair.test.ts runs the real backfillSeedTenancy between two report runs and measures the difference:

  • the live condition is gone — the __global__ counter was merged and deleted, so that line can never be produced again on that install;
  • the inventory survives — and not by luck: the backfill deliberately refuses to move a row whose identifier is already taken in the destination partition, which is the same ruling seen from the repair's side;
  • the rows that had no conflict did move, so their organization_id = NULL marker is gone — the perishability the ruling names.

That distinction is sharper than "run it first or lose everything", and it is what an operator actually needs to know.

Scope of the scan

Every registered object that is organization-scoped, and on it every field that is an identifier — type: 'autonumber', or carrying any unique spelling. Two deliberate choices:

  • sys_ / cloud_ / ai_ are not filtered out. That filter is correct for a repair (platform seeds stay global by design) and wrong for a report, which per the card must not silently omit a real duplicate.
  • Absence is loud. Anything that could not be probed lands in skipped with the driver's own message, so a target the command could not read never reads as a target with no findings; a driver with no raw-SQL seam (memory, mongodb) refuses with no_sql_seam and a non-zero exit rather than reporting zero duplicates. --object narrowing is recorded in the report, so an archived narrowed run cannot be mistaken for a full scan.

The output is a contract, and it is pinned as one

Point 8 of the dispatch: the JSON is consumed by auditors and tools, so its shape must not be an accident of the implementation. duplicates.contract.test.ts asserts the whole document against a real SQLite fixture — one row per duplicated value with holder ids, organizations, partitions and creation timestamps, plus scanned / skipped / liveConditions / summary and a reportVersion that is what changes when the shape must.

{
"report": "duplicate-identifiers",
"reportVersion": 1,
"duplicates": [
{ "object": "crm_case", "field": "case_number", "value": "CASE-00001", "holderCount": 2,
"partitions": ["__global__", "org_x"],
"holders": [
{ "id": "s1", "organization": null, "partition": "__global__", "createdAt": "2026-01-01T00:00:00.000Z" },
{ "id": "a1", "organization": "org_x", "partition": "org_x", "createdAt": "2026-02-01T00:00:00.000Z" }
] }
],
"liveConditions": [
{ "object": "crm_case", "field": "case_number", "globalLastValue": 38,
"organizationCounters": [{ "organization": "org_x", "lastValue": 4 }] }
]
}

An object with no created_at (system fields opted out) still produces holders, with a null timestamp — the holder probe retries without the column rather than failing the target.

Two things worth a reviewer's attention

Dialect-aware quoting. MySQL does not run with ANSI_QUOTES, so a probe quoted "like this" on every dialect compares a string literal with itself there. These probes read the live driver.config.client and quote with backticks for mysql/mysql2, double quotes otherwise. The convention in seed-tenancy-backfill.ts is the unconditional one; that is filed separately as #9381 rather than changed here (that file was out of surface, and the fix belongs with a live-MySQL run).

A gate two packages away decides whether a booted report is safe.assembleMetadataProtocol arms the #8686 backfill on kernel:ready behind if (environmentId === undefined), and the standalone stack stamps 'proj_local' — so today the migration never arms on a self-hosted boot at all. That is why booting is safe here, and it is a defect in its own right (the #8686 "existing install" half reaches no self-hosted install), filed as #9380 with the measurement. The integration test above is the assertion that will turn red the day that gate is fixed, which is the right place for that conversation to happen.

Verification

All gates re-derived from the actual changed paths (node scripts/pm/dispatch-gates.mjs) and run at 8e22df84c, the final commit:

GateResult
pnpm --filter @objectstack/cli test131 files / 1412 tests pass (includes the 4 new files, 18 tests)
pnpm --filter @objectstack/cli typecheckpass (tsconfig.json includes src with no test exclusion, so the new tests are really compiled)
pnpm check:cross-package-test-inputsOK — 12 packages read outside themselves, all declared
pnpm check:engine-double-contractOK — 317 pinned, no new double
pnpm check:where-matcherOK — 253 matchers, none new
pnpm check:query-options-erasureOK — baseline key set verified, no files added
pnpm check:type-check-coverage / check:type-check-debtOK — 33 ledger entries re-measured on the built closure, none above its recorded number
pnpm check:nul-bytesOK — 6103 files scanned
changeset family (adr-0087-registration, empty-changeset, changeset-no-major, check:objectui-changeset, check:changeset-gate-self-tests)OK — 1 non-breaking changeset seen
scripts/docs-audit/check-affected-docs.mjsOK (advisory; the one doc it names is reached through an unrelated route anchor)
pnpm exec eslint on the five new filesclean

Reverse verification. Dropping the partition test from the duplicate probe was predicted to make findings go up, not down — and did: duplicateValues 3 → 4, the in-partition repeat REF-1 appears, 4 tests red across two files. Restored from the commit and re-run green.

Follow-ups filed, not folded in

Renumbering is out of scope entirely, per both rulings. #8844 is not addressed here.


Generated by Claude Code

…zation partitions
`os migrate duplicates` — a read-only, operator-facing inventory of the values
the seed/API tenancy split already minted twice, per the #8928 maintainer
ruling (2026-08-16): CLI door only, the narrow cross-partition definition of
duplicate, JSON to stdout with no persistence, the live counter condition
reported alongside the damage, and a data-side probe rather than an
enumeration of the driver-private counter table.
Refs #8928
Co-Authored-By: Claude <noreply@anthropic.com>
…er inventory
Refs #8928
Co-Authored-By: Claude <noreply@anthropic.com>
@github-actionsgithub-actionsBot added size/xl documentation Improvements or additions to documentation tests tooling labels Aug 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 43 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/protocol/objectql/state-machine.mdx(via /:object/import/jobs (route))
What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: created_at (literal, 33 pages)
  • 22 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 22 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 origin/mainpackageMentionDocs.

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 origin/main → pass the list as args.docs.

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

2 participants

@os-zhuang@claude