Uh oh!
There was an error while loading. Please reload this page.
refactor(plugin-grid): adopt the spec's ExplainRequest on the explain request body - #6550
Conversation
…in request body `useRecordCrudVerdicts` hand-shaped its `POST /api/v1/security/explain` body as an untyped object literal passed straight to `JSON.stringify`, so a renamed or mis-cased key was not a compile error — it was a `400 VALIDATION_FAILED`, or, for `recordIDs`, a request the server reads as "no ids at all". The body is now `satisfies ExplainRequest` from `@objectstack/spec/security`, the package that owns the contract. Type-only: erased at compile time, emitted JavaScript unchanged. `RecordCrudOperation` stays the two kebab verbs, but is now a DECLARED subset of the spec's eight-verb `ExplainOperation` rather than a coincidental one: the two members are still written out locally (so an upstream release adding a ninth verb cannot widen what this list asks about), wrapped in a constraint that fails compilation if they stop being verbs the explain API accepts. Only the request side is adopted. The response deliberately stays `unknown` — asserting the spec's entry type would type `visible` as `boolean` and make the hook's fail-open runtime guards look like dead code, which is a silent removal of the safety rather than a tightening. That reasoning is now recorded beside the guards it protects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-support-ai
commented
Aug 26, 2026
ACCEPT — objectui#6332 ( My order asked one question for a type-adoption card: what does your assertion catch that the old code let through? The answer is a pair of legs, and it is the right shape. ⭐⭐⭐ Leg 1a vs 1b is the answerIdentical mutation, applied twice:
Green before, red after, same edit. That is the cleanest available demonstration that the adoption changed what is checkable rather than merely what is written — and it beats any assertion the PR could have made about itself. ⭐ And the defect class it now catches is a silent one: a mis-cased Two further legs, both with their direction predicted first:
⭐⭐⭐ The honest limit, volunteered rather than extracted
That is exactly what my order asked for, and it goes one better: it names the test it deliberately did not write, so nobody adds it later believing it verifies something. A green ghost assertion is worse than no assertion, because it occupies the slot where a real one would go. The subset mechanism, and the alternative it rejects
⭐ Deliberately not (For the record: my first grep counted one Piece (2) stayed out — and its rejection is now load-bearing⛔ Triage rejected asserting the response entry type, because it would make the fail-open runtime guards look like type-level dead code while reading as a tightening. It is not in the diff. Better than that, the rejection is recorded in the source at That is the durable half. A rejected option that lives only in a card gets re-proposed; one written beside the code it would have damaged stops the next person at the point of temptation. Fences
⭐⭐ A gate that cannot see this change, checked by hand instead
So the floor was verified directly: Same care on the NOT-MEASURED trap: Lint was not narrowed: the full 3838-file population, count read from eslint's own output. CI: 29 checks, zero failed, 9 running, on the head reported. Landing on green. Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#6332
Piece (1) only, per the triage ruling. Piece (2) is not done — see below, where its rejection is now recorded in the source. Piece (3) is future work and needs its own card.
Verification union run at
546a97f32(final commit, tree clean).Premise check — done first, before any edit
The dispatch flagged that
@objectstack/spec's dist was not installed in the shared checkout, so nobody had confirmedExplainRequestexists. Measured in this worktree against the resolved@objectstack/spec@17.2.0:Exported from the
./securitysubpath alongsideEXPLAIN_BATCH_MAX_RECORD_IDS, which this hook already imports. The premise holds and the shape is exactly what the hook builds — nothing was hand-rolled.The batch cap was not touched. It remains the server's contract; adopting
ExplainRequestdid not conflict with it in any way (recordIdsisstring[]in the spec, unbounded there, with the cap carried by the separate constant — the two are orthogonal).Spec floor:
plugin-griddeclares@objectstack/spec: ^17.0.0.check:spec-floorsis green but cannot see this change — it reads symbol references out of published artifacts, and these imports are type-only, so they are erased fromdist/. Verified by hand instead:npm pack @objectstack/spec@17.0.0and grep —ExplainRequestandExplainOperationare both present at the declared floor. No floor bump needed.What changed
The request body is the spec's type.
satisfies ExplainRequeston the literal, rather than an untyped object passed straight toJSON.stringify.RecordCrudOperationbecomes a declared subset, not a coincidental one:The two members stay written out locally, so an upstream release that adds a ninth verb cannot widen what this list asks about — while the constraint fails compilation the moment they stop being verbs the explain API accepts. The emitted type is exactly
'update' | 'delete';RecordCrudOperationdoes not appear indist/index.d.tsat all, so there is no public API change.Deliberately not
Extract<ExplainOperation, 'update' | 'delete'>—Extractanswersneverfor a member the spec renames, which is silent narrowing, precisely the failure the wrapper exists to make loud. That reasoning is in a comment at the declaration.Piece (2) is not done, and the source now says why
The rejection is recorded on
WireRecordVerdict, beside the guards it protects, so the next reader does not "finish the job":Reverse verification — what the assertion catches that the old code let through
Stating the limit plainly first, because it shapes everything else: at runtime this catches nothing. The change is type-only and the bytes on the wire are identical before and after. A vitest case that captured the outgoing body and parsed it with
ExplainRequestSchemawould pass against the pre-fix hook too — the "ghost assertion" shape thatuseRecordCrudVerdicts.batchCap.test.tsxdocuments next door. No such test was written.What it buys is a class of compile errors. Each leg below: mutate → prove the mutation landed on disk (blob hash differs from the
HEADblob) → measure → restore → prove the restore landed (hash equalsHEADblob andgit diff HEADempty). Predictions were written before running. No rebuild sits between mutation and measurement: bothtscprograms read this hook from source (confirmed via--listFiles), not from adist/.exit=0, testexit=0recordIds→recordIDs, new treeexit=2—TS2561: … 'recordIDs' does not exist in type … Did you mean to write 'recordIds'?exit=0, 0 errors'archive'exit=2—TS2344at:105(the constraint) +TS2322at the body'read'exit=0, testexit=2—TS2578: Unused '@ts-expect-error' directiveLeg 1a vs 1b is the answer to the card's question. The identical mutation compiles clean against the old code and fails against the new one. A mis-cased
recordIdsis not a loud failure at runtime — the server sees a request with no ids at all.Leg 3 matters on its own: it shows the two mechanisms are independent. Widening to a genuine spec verb still satisfies the subset constraint (
exit=0), so the declaration-site wrapper alone would not have caught it — the pin does. And it fails viaTS2578, which is self-proving: a pin that silently stopped checking is not a way this file can fail.The pin
useRecordCrudVerdicts.explainRequest.test.ts— 13@ts-expect-errordirectives plusAssert<…>aliases in the house idiom of__tests__/spec-symbol-batch7.test.ts. Confirmed to be a real program input oftsc -p tsconfig.test.json(the second half of this package'stype-check) with--listFiles, rather than assumed:It also opens with not-
anyguards on both spec symbols, and closes with a runtime vacuity control that touchesExplainRequestSchema— so a build where the spec module failed to resolve fails loudly instead of leaving a file of assertions that quietly check nothing.Tests and gates
pnpm exec vitest run packages/plugin-grid/src/hooks/— 3 files, 13 tests passed at546a97f32.pnpm --filter @object-ui/plugin-grid type-check(tsc --noEmit && tsc -p tsconfig.test.json) — verdictcommand-exit 0.pnpm --filter '@object-ui/plugin-grid^...' buildfirst, so the dependency closure is real rather than stale.package.json+.github/workflows(scripts/pm/dispatch-gates.mjslives only in objectstack and answers only about that tree, so it was not used here):check:control-bytes✅ ·check:spec-symbols✅ ·check:phantom-deps✅ ·check:self-import✅ ·check:spec-floors✅ ·check:vi-mock-specifiers✅ ·check-changeset-presence✅.eslint . --format jsonover the full population of 3838 files (count read from eslint's own output, not estimated). The three touched files carry 0 errors. The single warning on the hook —react-hooks/set-state-in-effectat:195— is on untouched code outside both diff hunks, and iswarnseverity.dist/contains neither it nor the existingbatchCapsuite.Changeset
Empty frontmatter — declared as releasing nothing, the explicit exemption. Honest here because the change is erased at compile time, the emitted JavaScript is unchanged, and
RecordCrudOperationis not part of the package's public.d.ts.Generated by Claude Code