fix(plugin-auth): let ImportProtocolLike type the admin import protocol's three members - #17489
Conversation
…bers The `const protocol: ImportProtocolLike` literal in `admin-import-users.ts` annotated all three required members `args: any`. An explicit parameter annotation wins over the contextual type, so the one hand-written in-repo implementor of the protocol was the one implementor NOT checked against the request dialect the runner sends — the same file whose frozen `$filter` read once degraded the duplicate probe into match-everything. Deleting the three annotations lets `ImportProtocolLike` type the parameters. `FindDataRequest` declares `query` optional, so `findData` now states its refusal explicitly (already-ledgered `INVALID_REQUEST`) instead of relying on an incidental TypeError from a property read on `undefined`. No `??` fallback, no optional chaining: both spell match-everything. Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW Co-authored-by: Claude <noreply@anthropic.com>
…d changeset
The behavioural upsert tests already discriminate the CONSEQUENCE of the
historical read — ablated to `args.query?.$filter ?? {}`, two of the 26 go red
— but nothing in the repo can see the opt-out itself: re-annotating the
parameter `any` leaves every test and every gate green, which is exactly how
the hole this card closes stayed open. The new source pin is that guard.
Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW
Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 13 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 9adf5942d60a146be5654754a7f15ad3d86db4f7 && git checkout 9adf5942d60a146be5654754a7f15ad3d86db4f7
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fa23d69875d1d75b55afc485790fb3df3c9fb712 fb659886748439fe7283763aa956d659864686ac && git checkout -B drift-repro fa23d69875d1d75b55afc485790fb3df3c9fb712 && git merge --no-ff fb659886748439fe7283763aa956d659864686ac
node scripts/docs-audit/affected-docs.mjs --json fa23d69875d1d75b55afc485790fb3df3c9fb712
|
Docs Drift Check — answered per page, on the tree the bot readRe-derived on The anchor is real, not incidental. This diff genuinely adds But the emission is unreachable, and it is not an endpoint-level refusal. The Per-page verdicts, with the falsifying shape named
The half the bot says it can never do — emitter-blind hand sweepIts own note: a page stating a rule by its inputs shares no identifier with the emitter, so an emitter-only diff cannot list it. So I hand-checked the pages that would describe this endpoint's failure behaviour without naming
One observation, explicitly not a falsification by this PR
No page is falsified, so there is nothing to file and nothing under Generated by Claude Code |
Fixes #17422
Clause-②: no
_(declaration line written by the
domain:servicesreview seat, not by the implementer.Check Changesetwent RED on headfb659886for the ABSENCE of this line, not for a wrong one: with no declaration and noneeds:contract-reviewcarrier, the gate resolvesnot-measured-material— a moved package gradedpatchis exactly what ayeswould have refused. Declaringnoresolves it asnot-declared. Measured on THIS diff, ⛔ not predicted from the dispatch — the dispatch predictednoon the reasoning that deleting a parameter annotation only narrows, and that reasoning did NOT cover the new refusal this diff adds, so both limbs were re-measured:• limb A (accept set): unchanged. A
findDatacall arriving withoutquerywas already rejected — by an incidentalTypeErroron a property read ofundefined. It is now rejected by a named throw. Nothing is newly accepted and nothing is newly refused; the same call fails, legibly. ⛔ Not a widening.• limb B (public surface): unchanged. 0 added lines containing
exportinpackages/**outside tests, against a control of 23 added source lines. The 1 addedcode:entry isINVALID_REQUEST, which is already registered inpackages/spec/src/api/error-code-ledger.zod.ts— and@objectstack/plugin-authis already listed as an owner of it (:550, inside the owner block opening at:536), socheck:error-code-provenanceneeds neither a new owner-key row nor aPROVENANCE_WAIVERSentry. ⛔ Nopackages/specedit is owed, and none is made.⇒
no+ apatchchangeset is the correct and consistent pair here. ⛔ Do not delete this line; the gate reads the body, so removing it re-reds the PR.)_What this closes
packages/plugins/plugin-auth/src/admin-import-users.tsis the only hand-written in-repo implementor of the runner'sImportProtocolLike, and it annotated all three required membersargs: any. An explicit parameter annotation wins over the contextual type, so the contract #16952 declared held every implementor except the one with the demonstrated history — the file whose frozen$filterread once degraded the import's duplicate probe into match-everything and updated the wrong users.Three annotations removed, all members of the
const protocol: ImportProtocolLike = {literal (positions re-derived by text, not from the card's line numbers):findDataasync findData(args: any)async findData(args)createDataasync createData(args: any)async createData(args)updateDataasync updateData(args: any)async updateData(args)The three OPTIONAL members of the interface —
createManyData,insertManyData,validateData— are not implemented by this literal (grep -cfor each: 0 implementations;createManyData's absence is deliberate and commented in place). So the class the card names is exactly three here, not "three plus".Repo-wide sweep for other implementors: every other
ImportProtocolLikeliteral is inpackages/rest's own test doubles, and #16952 already annotated those FROM the exported declaration (Parameters[ImportProtocolLike['findData']]and siblings). The two productionrunImportcall sites inrest-server.tshand it the realDataProtocolservice, not a hand-written literal.Triage's fork, answered: MERELY UNTYPED — not a dialect divergence
Triage asked to stop and report if the errors revealed a genuinely different dialect. They do not. Every key this file reads is one the protocol declares:
findDatareadsargs.object,args.query.where,args.query.limit—FindDataRequestdeclaresobjectandquery;QuerySchemadeclareswhereandlimit.createDatareadsargs.data— declared, required.updateDatareadsargs.dataandargs.id— both declared, both required.The compiler reported two errors and both are about optionality, not about an undeclared key:
FindDataRequestSchemadeclaresqueryoptional (packages/spec/src/api/protocol.zod.ts:1865), while all three of the runner'sfindDatadispatch sites always supply it (import-runner.ts:446,:489,:612, all throughfindArgsBase). So the file was reading the right key untypedly, and the contract's only complaint is that the file never wrote down what it does when the caller omitsquery.Resolution — one change, no cast.
findDatanow states the refusal instead of leaning on an incidentalTypeErrorfrom a property read onundefined:No
as any, no@ts-expect-error, no!, no widened local alias, and deliberately noargs.query ?? {}and noargs.query?.where— both of those spell match-everything, which is the exact regression the file's own comment block is about.INVALID_REQUESTis already registered for@objectstack/plugin-authinpackages/spec/src/api/error-code-ledger.zod.ts:550, so no spec file is touched by this PR (⛔packages/specwas the lane's red line) andpnpm check:error-code-casingstays green.packages/rest/src/import-runner.tswas read only. Nothing in it needed to move.Reverse verification — the green is a reading, not a dead check
Two legs plus a control, then restored by state.
pnpm --filter @objectstack/plugin-auth exec tsc --noEmitargs.query?.$filtersrc/admin-import-users.ts(384,43): error TS2339: Property '$filter' does not exist on type 'QueryInput'.args: anyrestoredgit checkout HEAD -- patha371c6d7c7938833b7130040b98335c0cfc6656dequals the HEAD blob;git diff HEADempty;git status --porcelainemptyLeg 2 is the point: the probe does not decide the colour — the annotation does. (The card cited
TS2353; that came from an object-literal probe. This probe is a property READ, so the same live type answersTS2339. Direction as predicted; the code differs because the probe shape differs.)Does a regression test for the
$filterto match-everything history exist?Behaviourally, yes — and it discriminates. Ablated
findDataback to the pre-#16950 read ((args.query as any)?.$filter ?? {}) and ran the sibling suite: 2 of 26 failed —upsert > matches by email …—AssertionError: expected 2 to be 1(withwhereempty the second row matches the existing user instead of creating one: the wrong-user update, reproduced)upsert > matches by phone_number when enabled— theexpect(find).toHaveBeenCalledWith(…, objectContaining({ where: … }))pin fails.Restored by state (blob hash equal to HEAD,
git diff HEADempty).For the opt-out itself, no — nothing could see it. Re-annotating all three members
anyand re-running:tsc --noEmitexit 0 and all 26 behavioural tests green. That is precisely how this hole stayed open under a green #16952. So this PR adds the missing guard as a source pin in the package's own test sibling (the same techniquerest-server-canonical-query-ast.test.ts§1b uses for the declaration half). Ablated: with the three annotations restored, 3 of 30 fail — exactly the three new pins — and the other 27 stay green.Verification
Working tree clean at
fb6598867; every exit code captured before any pipe.pnpm --filter @objectstack/plugin-auth build— exit 0,check-dts-emitted: 2/2 declared declaration file(s) present.pnpm --filter @objectstack/plugin-auth typecheck— exit 0 (tsc --noEmit+tsconfig.examples.json+check:test-typecheck; the test layer compiles undertsconfig.test.jsonwith its shrink-only debt ledger unmoved: 10 files / 94 errors / 23 pinned signatures).pnpm --filter @objectstack/plugin-auth test— exit 0, 106 files / 2260 tests passed.pnpm lint(eslint . --no-inline-config, the whole repo, not a narrowed set) — exit 0 in 1m53s at this same head.pnpm --filter '@objectstack/plugin-auth^...' build— exit 0, sotscread a freshly builtpackages/rest/dist/index.d.tscarrying the narrowed interface (verified by grepping the three member signatures out of the emitted.d.ts), not a stale one.scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 61 families; all 61 run, 59 exit 0, 2 exit 3 = PREREQUISITE NOT MET = NOT MEASURED (check:dual-build-cjs-loads,check:type-check-debt— both refuse without a whole-workspace build, which is CI'sBuild Core/TypeScript Type Check, not a package-scoped local tier).--ranreconciliation with exit codes recorded:61 derived, 59 run, 2 NOT-MEASURED, 0 UNRUN.check-changeset-fixed,check:auth-mount-ledger,check:authz-resolver,check:error-code-casing,check:filter-alias-parity— all exit 0.scripts/pm/os-verify-lock.sh; every verdict read from itsVERDICT command-exitline.Acceptance notes
Observations from this file, deliberately not changed here and not filed (none meets the file-an-issue bar — no repro, no violated declared contract, no metadata-authoring trap):
IdentityImportEngineatsrc/admin-import-users.ts:89–:91(find(objectName: string, query?: any),update(…, data: any, options?: any),insert(…)) is a different, local, engine-shaped interface and not part ofImportProtocolLike. Left exactly as found, per the dispatch. Typing it against ObjectQL's real engine surface would be its own card; no PR or seat is in flight over it today.createData/updateDatastill spellargs?.data ?? {}. Both request types declaredatarequired, so the fallback is now provably unreachable rather than merely unused — belt-and-braces of the family Prime Directive Add comprehensive test suite for Zod schema validation #12 names, but not a defect and not compiler-forced. The next PR touching this literal is the natural carrier.engine.find(args.object, { where, limit, context: SYSTEM_CTX } as any)keeps its pre-existingas anyon the OPTIONS bag (a consequence of the local interface above, not of the protocol).pnpm check:query-options-erasureis green over it.Generated by Claude Code