Repository navigation
fix(plugin-auth): os migrate apply's account-issuer pre-flight names each colliding row's stored issuer - #25056
Conversation
…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>
…ed-issuer read 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>
…eflight-issuer-physical-read
… find source Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 26 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 38 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # 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
|
… widened surface named Claude-Session: https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK Co-authored-by: Claude <noreply@anthropic.com>
席位核对 · ACCEPT ·
|
Fixes #25030
Clause-②: yes (widening)
What was wrong
When the
sys_account.issuerretirement pre-flight finds a(provider_id, account_id)key held by more than one row,os migrate apply --allow-destructiverefuses to dropsys_account.issuerand prints the pre-flight's report. That report labelled every colliding row's issuer(none).os migrate account-issuerprinted 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 whateverfindit was handed.apply's gate handed it theobjectqlengine, whose read verbs answer declared columns only, and the shippedsys_accountno longer declaresissuer.account-issuer.tshanded 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 CLIThe fixture is a SQLite copy. The driver built
sys_accountfrom today's object. Then the legacyissuercolumn was added and the(provider_id, account_id)unique was dropped. Two rows,acc_dupandacc_two, sharecredential / usr_25030, with stored issuershttps://idp-a.exampleandhttps://idp-b.example. The app config declaresrequires: ['auth'], soapplycomposes the auth family.d68aab6fdeos 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] …The fix: one reader, owned by plugin-auth
getDriverForObject, theobjectqlengine both CLI doors pass),probeAccountIdentityCollisionsreadssys_accountthrough the driver the engine routes that name to, and only through it. A missing driver refuses; the probe never falls back to the engine'sfind. The query is the oneaccount-issuer.tsused 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-issueradopts it. The command passes its engine to the probe and deletes its own reader. Its missing-table reading (a database with nosys_accounttable 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 otherpackages/clifile is touched.findalone is still read as handed, as before. No production caller does this any more. It is kept becausepackages/cli/test/migrate-drop-gate-remedies.pin.test.tsdrives the real probe through afind-only fake engine, and that file is outside this card's fence. The open question in the dev report covers retiring that path.sys_accountdoes not re-declareissuer, and nothing underpackages/specorpackages/objectqlchanges.Unchanged, measured
Public door, before and after. The CLI was built at
d68aab6fdeand again at this branch (turbo run build --filter='@objectstack/cli...').account-issuerandapply --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, nosys_accounttable, and no database file. That is 20 runs per build. Their stdout and stderr were compared after normalising timestamps and timings.apply's report on the issuer-column collision database, human and--json, and only on the issuers line shown above.preflight.collisions[].issuersremoved, the--jsondocument equals the old one: the samemessage: refused_account_issuer_collisions,ok, keys, row ids, users and counts.account-issueris 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 oforigin/main, and at010afefb70, the merged head that this PR carries. Both builds compare against the samed68aab6fdebaseline, with the same 20 exit codes and the same 38 identical streams. The card's own database was re-run at010afefb70with 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 realSqlDriverover a SQLite file that is built independently of the code under test. Each database is read three ways:apply's composition, withsys_accountregistered as it ships;account-issuer's composition, withsys_accountunregistered;findread the probe made before this change.What they pin:
(none).issuersequals 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.issuercolumn. A database without the column gives exactly the reference report (every row(none)) on both compositions.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.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 wasgit checkout HEAD -- PATH, proven by the blob equalling HEAD's and an emptygit diff HEAD. The finalgit statuswas clean. The subject resolves through a relativesrc/import, so there is nodist/leg.Every leg ran at
010afefb70.findwins again whenever the source has one (the defect)toEqual(issuers(none)). The one-reader pin, the multi-page pin, the no-column pin and two missing-table pins fail becauseaccount-issuer's composition is refusedOBJECT_NOT_FOUND. The no-driver pin fails because the source'sfindis read. The verdict and clean-table pins stay green, as they should.issuerissuerby construction.issuertooafter.ok(expected false, got true). The stored-issuer, one-reader and multi-page pins fail too. Before this fix the mutation was invisible, becauseissueralways read as undefined.onTableAbsentcall removedtold0)findVerification
Every run went through
scripts/pm/os-verify-lock.sh, with each exit captured before any pipe. Merged head010afefb70; the only later commit is changeset text,652462c694.pnpm --filter '@objectstack/plugin-auth^...' buildexit 0.turbo run build --filter='@objectstack/cli...'exit 0 (61/61 tasks).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 --listFileslists the new test file once.pnpm --filter @objectstack/cli typecheck: exit 0,check:test-typecheck: OK.pnpm --filter @objectstack/cli exec vitest run --project unit --maxWorkers=2,Test Files 319 passed (319),Tests 4730 passed (4730).test/migrate-drop-gate-remedies.pin.test.ts. That file's fake engine offersfindalone. Those reds are why the probe still reads a handedfindsource.src/commands/migrate/apply.account-issuer-retirement.integration.test.tswas run locally becauseaccount-issuer's read moved:Tests 4 passed (4). The rest of the integration tier is declared to CI.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackat652462c694derived 72 commands.check:dual-build-cjs-loadsandcheck:i18n-coverageexited 3 (PREREQUISITE NOT MET: untouched packages have nodist/). After a fullturbo 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, andcheck-i18n-coverage: OK (14 config(s), 621 baselined untranslated string(s), none new).--ranover 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).Acceptance notes
Published surface.
@objectstack/plugin-authgains:AccountTableStorage(getDriverForObject) andAccountTableDriver;AccountIdentityPreflightOptions.onTableAbsent.probeAccountIdentityCollisionsnow takesAccountTableStorage | AccountIdentityReadEngine, so every earlier caller still type-checks. The dispatch'sClause-②: nois 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 noobjectqlservice at all, the refusal used to readno driver serves sys_account on this stack. It now readsno readable ObjectQL engine. Both are refusals with exit 1.bootSchemaStackalways composesobjectql, 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.tsrow B9 still measures an enginefindofsys_accountas "the exact read" of this site. The site now reads the driver.SqlDriver.finddeclares 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/mainbefore opening. The merge (eddffc3390) was clean. The incoming commits touchpackages/spectranslations,packages/cli/src/utils/i18n-extract.tsand its tests,core,lintandrest, and none of them touch this diff's files. Everything under Verification was re-run on the merged head.Seat's append (
domain:servicesseat 1, from the dev's patch-round report on #25030; the dev writes its PR body once). The seat moved theClause-②:line above fromnotoyes (widening), per the claim amendment6308624635.dffec4ba41), changeset only:@objectstack/plugin-authmoves from patch to minor underClause-②: yes (widening). One bullet names the widened surface:AccountTableStorage,AccountTableDriver,AccountIdentityPreflightOptions.onTableAbsent, and the probe'sAccountTableStorage | AccountIdentityReadEnginesource.@objectstack/clistays patch.dffec4ba41:check-changeset-no-majorexit 0 andcheck-empty-changesetexit 0.git diff --stat 652462c694 HEADtouches only that changeset (+3/-2).Generated by Claude Code