refactor(plugin-grid): adopt the spec's ExplainRequest on the explain request body

.changeset/6332-explain-request-adoption.md

@@ -0,0 +1,25 @@
+---
+---
+
+Type-only: `useRecordCrudVerdicts` now builds its `POST /api/v1/security/explain`
+body against `@objectstack/spec/security`'s `ExplainRequest` instead of an
+untyped object literal, and `RecordCrudOperation` is now a *declared* subset of
+the spec's eight-verb `ExplainOperation` rather than a coincidental one.
+
+No release: `satisfies` and the subset wrapper are erased at compile time, the
+emitted JavaScript is unchanged, and `RecordCrudOperation` is not part of
+`@object-ui/plugin-grid`'s public `.d.ts` (verified — it does not appear in
+`dist/index.d.ts`).
+
+What it buys is a class of compile errors the untyped literal accepted:
+a renamed or mis-cased request key (`recordIDs`, `objectName`) and a verb the
+explain API does not accept are now `tsc` failures at the call site instead of
+a `400 VALIDATION_FAILED` — or, for a mis-cased `recordIds`, a request the
+server reads as "no ids at all". Pinned at compile time in
+`useRecordCrudVerdicts.explainRequest.test.ts`.
+
+The narrowing at the heart of the hook is deliberately preserved: the two kebab
+verbs stay written out locally, so an upstream release that adds a ninth verb
+cannot widen what this list asks about. Only the request side is adopted — the
+response stays `unknown` on purpose, because asserting the spec's entry type
+would make the hook's fail-open runtime guards look like dead code.

packages/plugin-grid/src/hooks/useRecordCrudVerdicts.explainRequest.test.ts

@@ -0,0 +1,177 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * [objectui#6332] The explain REQUEST this hook sends is the spec's
+ * `ExplainRequest`, and `RecordCrudOperation` is a DECLARED subset of the
+ * spec's eight-verb enum rather than an unrelated union that happens to spell
+ * two of them.
+ *
+ * ## Why every assertion here is a compile-time one
+ *
+ * State the limit first, because it decides the whole shape of this file: the
+ * adoption is **type-only and erased at runtime**. The bytes on the wire are
+ * identical before and after — `{ object, operation, recordIds }` either way.
+ * So there is no runtime behaviour to pin, and a vitest case that captured the
+ * outgoing body and parsed it with `ExplainRequestSchema` would pass against
+ * the PRE-fix hook too. That is exactly the "ghost assertion" shape
+ * `./useRecordCrudVerdicts.batchCap.test.tsx` documents next door: an
+ * assertion that reads like a contract test and cannot fail for the drift it
+ * claims to cover. This file does not write one.
+ *
+ * What the change genuinely buys is a class of COMPILE errors that the old
+ * code accepted, and the honest place to pin that is `tsc`. Two independent
+ * mechanisms are used, both of which fail loudly:
+ *
+ * - `Assert<...>` aliases (the house idiom — see
+ * `../__tests__/spec-symbol-batch7.test.ts`), which raise TS2344 when the
+ * relation they state stops holding.
+ * - `@ts-expect-error`, which is SELF-PROVING in the other direction: if the
+ * error it expects stops happening, the directive is unused and tsc raises
+ * TS2578. A pin that silently stopped checking is therefore not a way this
+ * file can fail — the narrowing pins below all take this form.
+ *
+ * ⚠️ These assertions are erased before anything runs, so vitest proves nothing
+ * about them. They are checked by `tsc -p tsconfig.test.json`, the second half
+ * of this package's `type-check` script — verified with `--listFiles`, which
+ * names this file as a program input. The single runtime case at the bottom is
+ * a vacuity control, not a contract test.
+ */
+
+import { describe, it, expect } from 'vitest';
+import {
+ ExplainRequestSchema,
+ type ExplainOperation,
+ type ExplainRequest,
+} from '@objectstack/spec/security';
+
+import type { RecordCrudOperation } from './useRecordCrudVerdicts';
+
+type Assert<T extends true> = T;
+type Equal<A, B> = (<T>() => T extends A ? 1 : 2) extends <T>() => T extends B ? 1 : 2 ? true : false;
+type Extends<A, B> = [A] extends [B] ? true : false;
+type IsAny<T> = 0 extends 1 & T ? true : false;
+
+/* -------------------------------------------------------------------------- */
+/* The spec symbols are real — without this the pins below are vacuous */
+/* -------------------------------------------------------------------------- */
+
+// If `@objectstack/spec/security` ever stopped exporting these, an untyped
+// import would degrade to `any` and EVERY assertion in this file would pass
+// while checking nothing. Both are pinned as not-any first, for the same reason
+// `spec-symbol-batch7.test.ts` opens with `_SpecIsReal`.
+type _ExplainOperationIsReal = Assert<Equal<IsAny<ExplainOperation>, false>>;
+type _ExplainRequestIsReal = Assert<Equal<IsAny<ExplainRequest>, false>>;
+
+/* -------------------------------------------------------------------------- */
+/* The narrowing survives the adoption (the card's load-bearing constraint) */
+/* -------------------------------------------------------------------------- */
+
+// Exactly the two kebab verbs, still — adopting the spec's request type must
+// not have widened this to the spec's vocabulary.
+type _StillExactlyTwoVerbs = Assert<Equal<RecordCrudOperation, 'update' | 'delete'>>;
+
+// ...and they ARE spec verbs. This is the half `SpecVerbSubset` enforces at the
+// declaration; restating it here means the relation is pinned from outside the
+// module too, so deleting the wrapper does not delete the check.
+type _NarrowingIsASpecSubset = Assert<Extends<RecordCrudOperation, ExplainOperation>>;
+
+// ...a PROPER subset. Equality here would mean the narrowing was replaced by
+// the spec enum wholesale — the specific regression this card must not cause.
+type _NarrowingIsNotTheWholeEnum = Assert<Equal<Equal<RecordCrudOperation, ExplainOperation>, false>>;
+
+describe('[#6332] the kebab verbs stay a declared subset of the spec enum', () => {
+ it('rejects every spec verb that is not a row-kebab verb', () => {
+ // Each directive is live only while the verb beside it is NOT assignable.
+ // Widen `RecordCrudOperation` to `ExplainOperation` and these do not fail
+ // to catch anything — they become UNUSED, which is TS2578 and red. That is
+ // why the narrowing is pinned this way rather than with a bare `Assert`.
+ // @ts-expect-error 'read' is a spec verb, but a row kebab offers no read affordance
+ const _read: RecordCrudOperation = 'read';
+ // @ts-expect-error 'create' is a spec verb; creation is not a per-ROW verdict at all
+ const _create: RecordCrudOperation = 'create';
+ // @ts-expect-error 'restore' is a spec verb this list has no affordance for
+ const _restore: RecordCrudOperation = 'restore';
+ // @ts-expect-error 'purge' is a spec verb this list has no affordance for
+ const _purge: RecordCrudOperation = 'purge';
+ // @ts-expect-error 'export' is a spec verb, and it is an OBJECT-level affordance here
+ const _export: RecordCrudOperation = 'export';
+ // @ts-expect-error 'transfer' is a spec verb this list has no affordance for
+ const _transfer: RecordCrudOperation = 'transfer';
+ expect(true).toBe(true);
+ });
+
+ it('rejects a verb the explain API does not accept at all', () => {
+ // The other direction, and the one `SpecVerbSubset` on the declaration
+ // catches at its source: an invented verb is neither a kebab verb nor a
+ // spec verb. Before the adoption a widening to `'archive'` compiled
+ // cleanly here and failed as `400 VALIDATION_FAILED` in a browser.
+ // @ts-expect-error 'archive' is not a verb the explain API accepts
+ const _archive: RecordCrudOperation = 'archive';
+ // @ts-expect-error ...and it is not one the spec's own enum accepts either
+ const _archiveSpec: ExplainOperation = 'archive';
+ expect(true).toBe(true);
+ });
+});
+
+/* -------------------------------------------------------------------------- */
+/* The request body answers to the spec's request contract */
+/* -------------------------------------------------------------------------- */
+
+describe('[#6332] the explain request body is the spec\'s ExplainRequest', () => {
+ it('accepts the exact literal the hook builds', () => {
+ // The shape at the call site, spelled out. If the spec renames a key or
+ // makes one required that this hook does not send, this stops compiling —
+ // which is the whole point of annotating the body over there.
+ const _body = {
+ object: 'showcase_project',
+ operation: 'update' as RecordCrudOperation,
+ recordIds: ['r_0', 'r_1'],
+ } satisfies ExplainRequest;
+ expect(Object.keys(_body).sort()).toEqual(['object', 'operation', 'recordIds']);
+ });
+
+ it('rejects the key drift the untyped literal used to accept', () => {
+ // These four are what the change actually catches. Every one of them
+ // compiled cleanly when the body was an inline object literal passed
+ // straight to `JSON.stringify` — the reverse-verification leg on this card
+ // confirmed it against the pre-fix file.
+ // @ts-expect-error `recordIDs` is not the spec's casing; the server would see no ids
+ const _misCased = { object: 'o', operation: 'update', recordIDs: ['r'] } satisfies ExplainRequest;
+ // @ts-expect-error the key is `object`, not `objectName` (the hook's local variable name)
+ const _localName = { objectName: 'o', operation: 'update' } satisfies ExplainRequest;
+ // @ts-expect-error `recordIds` is `string[]`; ids are not coerced from numbers
+ const _numericIds = { object: 'o', operation: 'update', recordIds: [1, 2] } satisfies ExplainRequest;
+ // @ts-expect-error `object` is required — the spec has no "current object" default
+ const _noObject = { operation: 'update', recordIds: ['r'] } satisfies ExplainRequest;
+ expect(true).toBe(true);
+ });
+});
+
+/* -------------------------------------------------------------------------- */
+/* Vacuity control — the ONE thing here that runs */
+/* -------------------------------------------------------------------------- */
+
+describe('[#6332] control: the spec module really resolved', () => {
+ it('is a live zod schema, so the types above came from the spec and not from `any`', () => {
+ // The type-level pins cannot tell a resolved module from a stubbed one.
+ // This can: it touches the runtime export that sits beside them. A build
+ // where `@objectstack/spec/security` failed to resolve fails HERE, loudly,
+ // instead of leaving a file of assertions that quietly check nothing.
+ expect(ExplainRequestSchema).toBeDefined();
+ const parsed = ExplainRequestSchema.safeParse({
+ object: 'showcase_project',
+ operation: 'update',
+ recordIds: ['r_0'],
+ });
+ expect(parsed.success).toBe(true);
+ // ...and it is genuinely the eight-verb enum the narrowing is a subset OF,
+ // so `_NarrowingIsNotTheWholeEnum` above is asserting against a real enum.
+ expect(ExplainRequestSchema.safeParse({ object: 'o', operation: 'archive' }).success).toBe(false);
+ });
+});

packages/plugin-grid/src/hooks/useRecordCrudVerdicts.ts

@@ -66,17 +66,64 @@ import * as React from 'react';
  * copy passes every value comparison on the day it is written and drifts
  * silently on the day the server moves the cap.
  */
-import { EXPLAIN_BATCH_MAX_RECORD_IDS } from '@objectstack/spec/security';
+import {
+ EXPLAIN_BATCH_MAX_RECORD_IDS,
+ type ExplainOperation,
+ type ExplainRequest,
+} from '@objectstack/spec/security';
 import { SchemaRendererContext } from '@object-ui/react';
 
-/** The two write verbs a list row's kebab can offer. */
-export type RecordCrudOperation = 'update' | 'delete';
+/**
+ * Compile-time proof that `Verbs` is a SUBSET of the spec's `ExplainOperation`,
+ * resolving to `Verbs` unchanged. Widening past the spec's vocabulary is TS2344
+ * on the declaration below rather than a `400 VALIDATION_FAILED` in a browser.
+ */
+type SpecVerbSubset<Verbs extends ExplainOperation> = Verbs;
+
+/**
+ * The two write verbs a list row's kebab can offer.
+ *
+ * DELIBERATELY NARROWER than the spec (objectui#6332). `ExplainOperation` has
+ * eight verbs — `read`, `create`, `restore`, `purge`, `export`, `transfer` as
+ * well as these two — and this hook is contractually limited to the two a row
+ * kebab can act on. Every other verb has a different affordance, a different
+ * caller and a different fail-open story; answering one here would put a
+ * verdict on screen that nothing on this row can use.
+ *
+ * The `SpecVerbSubset` wrapper is what makes that a DECLARED subset rather than
+ * a coincidence: the two members stay written out here (nothing is inherited
+ * from the spec, so the narrowing cannot be widened by an upstream release),
+ * while the constraint fails compilation the moment they stop being verbs the
+ * explain API accepts. It is erased at runtime — the emitted type is exactly
+ * `'update' | 'delete'`.
+ *
+ * ⛔ Do not "simplify" this to `ExplainOperation`, and do not derive it with
+ * `Extract<ExplainOperation, ...>` — `Extract` answers `never` for a member the
+ * spec renames, which is silent narrowing, the failure this wrapper exists to
+ * make loud.
+ */
+export type RecordCrudOperation = SpecVerbSubset<'update' | 'delete'>;
 
 /**
  * One entry of the batch response's `records` array, narrowed to what this hook
  * reads. Wire payloads are `unknown` until proven otherwise — a malformed entry
  * must land on "no verdict for this row" (fail open), never on a coerced
  * boolean.
+ *
+ * ⛔ This is NOT an oversight to tidy up into the spec's response entry type,
+ * and objectui#6332 rejected doing so deliberately. The spec's entry types
+ * `visible` as `boolean`, so asserting it here would make the runtime guards
+ * below (`typeof entry.visible !== 'boolean'`) unreachable in the compiler's
+ * eyes — dead code a future reader or lint rule then deletes, taking the
+ * fail-open path with it. A change that makes runtime safety code look
+ * redundant is not a tightening; it is a silent removal of the safety.
+ *
+ * The response-side contract-first move is not a type at all — it is
+ * `ExplainDecisionSchema.safeParse(...)`, which is a BEHAVIOUR change (the
+ * schema requires `allowed`/`object`/`operation`/`principal`, so a
+ * reduced-but-usable response this hook answers today would start failing
+ * open). That needs its own card and its own fail-open regression coverage.
+ * The request side above is adoptable precisely because it is type-only.
  */
 interface WireRecordVerdict {
  recordId?: unknown;
@@ -168,12 +215,28 @@ export function useRecordCrudVerdicts(opts: {
  const missing = ids.filter((id) => !verdictCache.has(cacheKey(objectName, id, operation)));
  for (let i = 0; i < missing.length; i += EXPLAIN_BATCH_MAX_RECORD_IDS) {
  const chunk = missing.slice(i, i + EXPLAIN_BATCH_MAX_RECORD_IDS);
+ // The body IS the spec's request contract, not a lookalike shaped to
+ // match it (objectui#6332). `satisfies` keeps the literal's own type
+ // while making every key and the verb answer to `ExplainRequestSchema`
+ // — a renamed or mis-cased key (`recordIDs`, `objectName`) is a
+ // compile error here instead of a request the server rejects, or
+ // worse, silently reads as "no ids".
+ //
+ // Type-only: erased at runtime, so nothing about the request that
+ // goes out over the wire changes. This deliberately stops at the
+ // REQUEST — the response below stays `unknown` on purpose (see the
+ // fail-open note in the file header and `WireRecordVerdict`).
+ const body = {
+ object: objectName,
+ operation,
+ recordIds: chunk,
+ } satisfies ExplainRequest;
  try {
  const res = await doFetch('/api/v1/security/explain', {
  method: 'POST',
  headers: { 'Content-Type': 'application/json' },
  credentials: 'include',
- body: JSON.stringify({ object: objectName, operation, recordIds: chunk }),
+ body: JSON.stringify(body),
  });
  if (!res.ok) continue; // 401 / 403 / 501 → fail open
  const decision: unknown = await res.json();