Skip to content

fix(plugin-auth): os migrate apply's account-issuer pre-flight names each colliding row's stored issuer - #25056

Merged
objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/issue-25030-preflight-issuer-physical-read
Oct 21, 2026
Merged

objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/issue-25030-preflight-issuer-physical-read

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #25030

Clause-②: yes (widening)

What was wrong

When the sys_account.issuer retirement pre-flight finds a (provider_id, account_id) key held by more than one row, os migrate apply --allow-destructive refuses to drop sys_account.issuer and prints the pre-flight's report. That report labelled every colliding row's issuer (none). os migrate account-issuer printed the stored values for the same database. The issuers are what an operator needs to decide which row survives.

Cause: probeAccountIdentityCollisions (packages/plugins/plugin-auth/src/account-identity-preflight.ts) read through whatever find it was handed. apply's gate handed it the objectql engine, whose read verbs answer declared columns only, and the shipped sys_account no longer declares issuer. account-issuer.ts handed it a reader of its own over the driver (getDriverForObject). So the two doors ran one probe over two readers, and the readers disagreed.

Reproduced first, at d68aab6fde, on the built CLI

The fixture is a SQLite copy. The driver built sys_account from today's object. Then the legacy issuer column was added and the (provider_id, account_id) unique was dropped. Two rows, acc_dup and acc_two, share credential / usr_25030, with stored issuers https://idp-a.example and https://idp-b.example. The app config declares requires: ['auth'], so apply composes the auth family.

Door (exit) Report at d68aab6fde At this branch
os migrate apply --allow-destructive --yes (1) credential / usr_25030 — 2 rows [acc_dup, acc_two], issuers [(none)], users [usr_25030] … issuers [https://idp-a.example, https://idp-b.example], users [usr_25030]
os migrate apply … --json (1) preflight.collisions[0].issuers: ["(none)"] ["https://idp-a.example","https://idp-b.example"]
os migrate account-issuer (1) … issuers [https://idp-a.example, https://idp-b.example] … unchanged

The fix: one reader, owned by plugin-auth

  • The probe reads storage itself. When its source offers the storage door (getDriverForObject, the objectql engine both CLI doors pass), probeAccountIdentityCollisions reads sys_account through the driver the engine routes that name to, and only through it. A missing driver refuses; the probe never falls back to the engine's find. The query is the one account-issuer.ts used to send: same columns, same keyset walk, no driver options. apply's gate (utils/apply-drop-gates.ts) is untouched and now reports the stored issuers.
  • os migrate account-issuer adopts it. The command passes its engine to the probe and deletes its own reader. Its missing-table reading (a database with no sys_account table reads as no rows) moves into the probe as an opt-in, onTableAbsent. A caller that passes it gets no rows and is told once. A caller that does not pass it is refused, as for any other failed read. No other packages/cli file is touched.
  • A source that offers find alone is still read as handed, as before. No production caller does this any more. It is kept because packages/cli/test/migrate-drop-gate-remedies.pin.test.ts drives the real probe through a find-only fake engine, and that file is outside this card's fence. The open question in the dev report covers retiring that path.
  • Nothing else moves. sys_account does not re-declare issuer, and nothing under packages/spec or packages/objectql changes.

Unchanged, measured

Public door, before and after. The CLI was built at d68aab6fde and again at this branch (turbo run build --filter='@objectstack/cli...'). account-issuer and apply --allow-destructive --yes, human and --json, ran on five databases: issuer column with a collision, no issuer column with a collision, issuer column and clean, no sys_account table, and no database file. That is 20 runs per build. Their stdout and stderr were compared after normalising timestamps and timings.

  • Exit codes are identical in all 20.
  • 38 of the 40 streams are byte-identical. The two that differ are apply's report on the issuer-column collision database, human and --json, and only on the issuers line shown above.
  • With preflight.collisions[].issuers removed, the --json document equals the old one: the same message: refused_account_issuer_collisions, ok, keys, row ids, users and counts.
  • account-issuer is byte-identical on all five databases, human and --json, including the no-table line and the no-file run.

These comparisons were measured twice: at 37077ac369, an earlier strict-contract cut before the merge of origin/main, and at 010afefb70, the merged head that this PR carries. Both builds compare against the same d68aab6fde baseline, with the same 20 exit codes and the same 38 identical streams. The card's own database was re-run at 010afefb70 with the result in the table above.

Pins, in a new file: packages/plugins/plugin-auth/src/account-identity-preflight-stored-issuer.test.ts. They run on a real SqlDriver over a SQLite file that is built independently of the code under test. Each database is read three ways:

  • through apply's composition, with sys_account registered as it ships;
  • through account-issuer's composition, with sys_account unregistered;
  • through a reference: the engine-find read the probe made before this change.

What they pin:

  • Stored issuers are named. The colliding rows' stored issuers appear in the report and in its text. A premise check shows the engine read of the same database gives (none).
  • One reader. Both compositions give the same report and the same text.
  • The verdict is unchanged. Every field except issuers equals the reference, and the refusal (RESOURCE_CONFLICT / 409) carries the same message. This also holds over a 1,200-row walk with the colliding rows on its first and last page. A clean table stays clean.
  • No issuer column. A database without the column gives exactly the reference report (every row (none)) on both compositions.
  • Missing table. With onTableAbsent, the probe reads no rows and tells the caller once. Without it, the probe refuses. When the table is present, the caller is never told it is absent.
  • No driver. If the storage door yields no driver, the probe refuses and never calls the source's own find.

Ablation, one leg per pinned behaviour, on the committed fix through scripts/ablation-replace.mjs. In every leg the anchor went from 1 hit to 0, the blob changed, and the run went red at the assertion of interest. Every restore was git checkout HEAD -- PATH, proven by the blob equalling HEAD's and an empty git diff HEAD. The final git status was clean. The subject resolves through a relative src/ import, so there is no dist/ leg.

Every leg ran at 010afefb70.

Leg Mutation Red, and where
L1 the engine's find wins again whenever the source has one (the defect) 7/10. The stored-issuer pin fails at its toEqual (issuers (none)). The one-reader pin, the multi-page pin, the no-column pin and two missing-table pins fail because account-issuer's composition is refused OBJECT_NOT_FOUND. The no-driver pin fails because the source's find is read. The verdict and clean-table pins stay green, as they should.
L2 a reader that refuses rows without a stored issuer 5/10. The no-column pin fails at its own probe call. The other four fail at their engine-read reference, which carries no issuer by construction.
L3 collisions grouped by issuer too 4/10. The verdict pin fails at after.ok (expected false, got true). The stored-issuer, one-reader and multi-page pins fail too. Before this fix the mutation was invisible, because issuer always read as undefined.
L4 the onTableAbsent call removed 1/10: "a caller that asks … is told once" (told 0)
L5 a missing table softened without the opt-in 1/10: "a caller that does not ask is refused"
L6 no driver falls back to the source's find 1/10: the no-driver pin

Verification

Every run went through scripts/pm/os-verify-lock.sh, with each exit captured before any pipe. Merged head 010afefb70; the only later commit is changeset text, 652462c694.

  • Build closures: pnpm --filter '@objectstack/plugin-auth^...' build exit 0. turbo run build --filter='@objectstack/cli...' exit 0 (61/61 tasks).
  • New pins: pnpm --filter @objectstack/plugin-auth exec vitest run --maxWorkers=2 src/account-identity-preflight-stored-issuer.test.ts, 10 passed.
  • pnpm --filter @objectstack/plugin-auth test: Test Files 176 passed (176), Tests 3348 passed | 10 skipped (3358).
  • pnpm --filter @objectstack/plugin-auth typecheck: exit 0, check:test-typecheck: OK. Debt is unchanged at 10 files / 88 errors. tsc -p tsconfig.test.json --listFiles lists the new test file once.
  • pnpm --filter @objectstack/cli typecheck: exit 0, check:test-typecheck: OK.
  • CLI unit tier: pnpm --filter @objectstack/cli exec vitest run --project unit --maxWorkers=2, Test Files 319 passed (319), Tests 4730 passed (4730).
    • An earlier strict-contract cut of this branch reddened four cases in test/migrate-drop-gate-remedies.pin.test.ts. That file's fake engine offers find alone. Those reds are why the probe still reads a handed find source.
  • CLI integration: src/commands/migrate/apply.account-issuer-retirement.integration.test.ts was run locally because account-issuer's read moved: Tests 4 passed (4). The rest of the integration tier is declared to CI.
  • Gates: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack at 652462c694 derived 72 commands.
    • 70 exited 0.
    • check:dual-build-cjs-loads and check:i18n-coverage exited 3 (PREREQUISITE NOT MET: untouched packages have no dist/). After a full turbo run build (76/76 tasks, exit 0), both exited 0 on the same head: check:dual-build-cjs-loads — 112 published require entry point(s) across 69 package(s) load, and check-i18n-coverage: OK (14 config(s), 621 baselined untranslated string(s), none new).
    • --ran over the first record: 72 derived famil(ies) accounted for — 70 run, 2 NOT-MEASURED. Over the record with the two re-runs: ✓ dispatch-gates --ran: 72 derived famil(ies) accounted for — 72 run, 0 NOT-MEASURED (a DERIVED zero — all 72 recorded an exit code and none of them is 3).
    • Every family CI runs is read from this PR's check-runs, not re-run here.

Acceptance notes

  • Published surface. @objectstack/plugin-auth gains:

    • two exported types, AccountTableStorage (getDriverForObject) and AccountTableDriver;
    • one option, AccountIdentityPreflightOptions.onTableAbsent.

    probeAccountIdentityCollisions now takes AccountTableStorage | AccountIdentityReadEngine, so every earlier caller still type-checks. The dispatch's Clause-②: no is kept as given. The dev report asks whether these additions are "public surface" under the clause.

  • Unreachable wording delta in account-issuer. If the booted stack had no objectql service at all, the refusal used to read no driver serves sys_account on this stack. It now reads no readable ObjectQL engine. Both are refusals with exit 1. bootSchemaStack always composes objectql, so this path is not reachable today. Where an engine is present but has no driver for the table, the old wording is unchanged.

  • Stale measurement row. find-envelope-limb-removal.test.ts row B9 still measures an engine find of sys_account as "the exact read" of this site. The site now reads the driver. SqlDriver.find declares an array of rows as its answer, and the probe still refuses a non-array. The row stays green; it is outside the file surface, so it is left as is.

  • Merged origin/main before opening. The merge (eddffc3390) was clean. The incoming commits touch packages/spec translations, packages/cli/src/utils/i18n-extract.ts and its tests, core, lint and rest, and none of them touch this diff's files. Everything under Verification was re-run on the merged head.


Seat's append (domain:services seat 1, from the dev's patch-round report on #25030; the dev writes its PR body once). The seat moved the Clause-②: line above from no to yes (widening), per the claim amendment 6308624635.

  • Patch round 1 (dffec4ba41), changeset only: @objectstack/plugin-auth moves from patch to minor under Clause-②: yes (widening). One bullet names the widened surface: AccountTableStorage, AccountTableDriver, AccountIdentityPreflightOptions.onTableAbsent, and the probe's AccountTableStorage | AccountIdentityReadEngine source. @objectstack/cli stays patch.
  • Re-run at dffec4ba41: check-changeset-no-major exit 0 and check-empty-changeset exit 0. git diff --stat 652462c694 HEAD touches only that changeset (+3/-2).

Generated by Claude Code

…ough the driver, so apply's report names the stored issuers

probeAccountIdentityCollisions read through the engine's find, which answers
declared columns only. The shipped sys_account no longer declares issuer, so
os migrate apply's drop-gate report labelled every colliding row's issuer
(none), while os migrate account-issuer, reading through the driver, printed
the stored values. The probe now takes the storage door (getDriverForObject)
and reads the driver the engine routes sys_account to, for both doors.
account-issuer adopts it; its missing-table reading is the probe's opt-in
onTableAbsent.

Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK
Co-authored-by: Claude <noreply@anthropic.com>
…nswer before its reference read

Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK
Co-authored-by: Claude <noreply@anthropic.com>
…eads a handed find source

A source that offers getDriverForObject (the objectql engine both CLI doors
pass) is read through that driver alone; a missing driver refuses and never
falls back to the source's find. A source offering find alone is read as
handed, as before, so callers that hand the probe their own reader (the CLI's
drop-gate pin fake, the probe's own refusal fakes) keep their answers.

Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 21, 2026
@github-actions

github-actions Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/cli, @objectstack/plugin-auth, touching 14 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 63085c3806fdeece890713614d455d6a36790ec6.

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

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 38 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 63085c3806fdeece890713614d455d6a36790ec6 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 63085c3806fdeece890713614d455d6a36790ec6

⚠️ 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 63085c3806fdeece890713614d455d6a36790ec6 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… widened surface named

Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:services seat 1 · 2026-10-21T05:46Z

os-justin · session_011TdZ8GjixNZFLz1xJD9zzK (seat post #6021). Reviewed against GitHub and origin/main 72518d20f5, not against the reports (os-dev reports 6308594858 and 6308654956 on #25030).

Head under review:

dffec4ba41aca533fb2fd17763e4c85d7f7e87bf
  1. Shape.
    • Draft, base main, first line Fixes #25030.
    • Line-start Clause-②: yes (widening) in the body and the changeset. The seat moved it from no by claim amendment 6308624635.
    • 4 files, none on a governed surface (governedPathsIn answers []). The trial merge onto origin/main is clean.
    • The review face is declaration at TIER_DEFAULT, so this ACCEPT is the review of record of the widening.
  2. The widening, read sentence by sentence against the code.
    • plugin-auth's package entry re-exports account-identity-preflight.ts (export *).
    • It now publishes AccountTableStorage (getDriverForObject), AccountTableDriver (find) and the option AccountIdentityPreflightOptions.onTableAbsent.
    • probeAccountIdentityCollisions accepts AccountTableStorage | AccountIdentityReadEngine.
    • Every earlier caller still type-checks: the union keeps the old parameter type. Nothing is removed or narrowed. That makes it a minor, with cli at patch.
  3. The fix, read in the code.
  4. Measured on the built CLI: 20 runs over five database shapes.
    • Exit codes are identical in all 20.
    • 38 of 40 normalised streams are byte-identical. The two that differ are apply's issuers line on the collision database, which is the fix.
    • account-issuer is byte-identical on every shape.
  5. Pins and ablation.
    • account-identity-preflight-stored-issuer.test.ts: 10 cases on a real SqlDriver over a SQLite file, read through both doors' compositions and the engine-find reference.
    • Six ablation legs, each red at its assertion. Every restore was proven by blob equality.
  6. Gates.
    • At ACCEPT, every check-run on this head is success or an expected skip. Auto Label and Check PR Size are skipped after the seat's body edit, which is on the expected-skips roster.
    • Locally, 72 of 72 derived families exited 0, and --ran reads 0 NOT MEASURED.
    • plugin-auth: 3348 tests passed. The CLI unit tier passed 4730 tests. apply.account-issuer-retirement.integration.test.ts: 4 passed.
    • dogfood_battery: n/a. There is no ! in the title and no (narrowing) arm.
  7. The dev's open questions.
  8. Accepted deviations.
    • The first cut was reworked so that a cli-lane pin file stays untouched.
    • The unreachable no-engine refusal is reworded.
    • One merge of main.
    • One lock queue-timeout, retried.
    • Model-free trailers.
  9. Out-of-scope disposition. find-envelope-limb-removal.test.ts row B9 is a stale measurement row: it stays green and is not a defect. It is noted in the PR's Acceptance notes with no carrier.
  10. Landing clause. Tier S: not governed, under the human-merge threshold. The next step is ready + auto-merge through the relay, once every check on the current head is green or an expected skip. A push resets this reading.

Generated by Claude Code

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/l tests tooling

Projects

None yet

2 participants