Skip to content

Commit 5d46f43

Browse files
feat(spec): ExplainAccessRequest declares recordIds, the batch selector the explain wire schema already carries (#29470)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi99U9EnFooYs4VpVTYdtMEsX)_ Fixes #29297 Clause-②: yes (widening) Metadata-protocol: none Skills-docs: none — no page under `skills/` or `content/docs/` spells the members of the `ExplainAccessRequest` contract type; `content/docs/permissions/explain.mdx` shows the kernel call with `object`/`operation`/`userId` only, and the batch form is already a wire-schema fact of `ExplainRequestSchema` (grep `explain` and `recordIds` in both trees, on 29000bb). ## 维护者速读 - 改了什么:`security` 服务契约的 explain 请求类型 `ExplainAccessRequest` 补上可选的 `recordIds`(批量记录 id),类型直接取自线上 schema `ExplainRequestSchema` 的同名成员。 - 为什么改:线上 schema、REST 解释路由和参考安全服务早已支持批量形态,唯独它们之间的 TypeScript 契约没声明,调用方只能强转才能传批量请求。 - 风险与回滚:仅类型放宽,无运行时改动;此前能编译的调用照常编译。回滚即撤销本 PR。 - 先看:`packages/spec/src/contracts/security-service.ts`(新成员与文档注释)、`packages/spec/src/contracts/security-service.test.ts`(类型钉)。 - 两分钟验证:`pnpm --filter @objectstack/spec check:test-typecheck`,预期 OK;把新成员改成 `readonly string[]` 再跑,预期该测试文件报 3 个类型错误。 - 需要你做的:无,席位落地。 ## What changes - `ExplainAccessRequest` (`packages/spec/src/contracts/security-service.ts`) gains `recordIds?: ExplainRequest['recordIds']`, with a doc comment: mutually exclusive with `recordId`; at least one and at most `EXPLAIN_BATCH_MAX_RECORD_IDS` ids (linked, not spelled); answered by `ExplainDecision.records` in the same order; an implementation may ignore it and answer only the object-level decision, and the caller then asks each id singularly, as the REST explain route does. - The member's type is the wire schema's own (`ExplainRequest` = `z.input<typeof ExplainRequestSchema>`), not a hand-written `string[]`. Only this member is derived: tying the whole interface to the schema would change `operation` (the contract's `ExplainOperation | string` against the schema's closed enum) and replace the contract's member docs. - The last doc sentence is measured against both readers on 29000bb: the REST door hands the service the batch and falls back per id when the answer carries no `records` of the right length (`packages/rest/src/security-routes.ts` L256–299, the call at L279, the check at L280, the per-id loop at L289–296); the reference service reads the member and refuses it beside `recordId` (`packages/plugins/plugin-security/src/security-plugin-explain-access.ts` L128, L142–145, L526–527). - No runtime change. `check:api-surface` agrees with no regeneration (it records export names; no export was added or removed). - Changeset: `.changeset/29297-explain-access-request-record-ids.md`, `@objectstack/spec` patch. ## Tests - New in `packages/spec/src/contracts/security-service.test.ts` (no existing test was edited): - `ExplainRecordIdsIsOptional`: a type alias that reds if the member becomes required. - `ExplainRecordIdsMatchesWire`: a type alias holding `ExplainAccessRequest['recordIds']` equal to `z.input<typeof ExplainRequestSchema>['recordIds']`. - `explain accepts a batch request…`: the same equality through `expectTypeOf`, a batch literal reaching the service, and a `@ts-expect-error` that a bare string is not a batch. - The file is compiled by `check:test-typecheck` (`tsconfig.test.json`) and is not in `test-typecheck-debt.json` (0 hits), so the aliases are live checks. <details><summary>Ablation (type pin)</summary> `node scripts/ablation-replace.mjs --file packages/spec/src/contracts/security-service.ts --anchor "recordIds?: ExplainRequest['recordIds'];" --replacement 'recordIds?: readonly string[];' -- pnpm --filter @objectstack/spec check:test-typecheck` - mutation landed: anchor 1 → 0, blob `ea8eb5b53fb0` → `f7a8cee931f6` - `src/contracts/security-service.test.ts: 3 type error(s) in a file the ledger does not cover` · command exited 1 - restored: blob == HEAD (`ea8eb5b53fb0`), `git diff HEAD` empty </details> <details><summary>Package suites on 29000bb (os-verify-lock)</summary> - closure build `--filter '@objectstack/plugin-security^...' --filter '@objectstack/rest^...'`: VERDICT command-exit 0 - `@objectstack/spec` test: Test Files 791 passed (791) · Tests 23470 passed, 1 todo · exit 0 - `@objectstack/spec` typecheck (tsc + scripts + test layer): exit 0 · `check:test-typecheck: OK — 52 file(s) / 240 error(s) / 131 pinned signature(s) held` - `@objectstack/plugin-security` test: Test Files 240 passed (240) · Tests 5175 passed, 45 skipped · exit 0 - `@objectstack/plugin-security` typecheck: exit 0 - `@objectstack/rest` test: Test Files 371 passed (371) · Tests 7032 passed, 332 skipped · exit 0 - `@objectstack/rest` typecheck: exit 0 - eslint `--no-inline-config` on both changed files: exit 0 </details> ## Acceptance notes - `@objectstack/plugin-sharing`, `@objectstack/service-storage` and `@objectstack/plugin-audit` also import `ExplainAccessRequest`; the change only adds an optional member, so their code and test doubles stay assignable. Not run locally; CI's Test Core and TypeScript Type Check cover them. ## Verification - Gates on 29000bb: `dispatch-gates.mjs --commands` derived 79; `--ran` reconciles 79 derived, 78 run, 1 NOT-MEASURED, 0 UNRUN. 78 exit 0. - `check-plugin-teardown-shape.mjs --self-test` first exited 3 on this shallow clone (its positive-control fixture is pinned to a commit the clone lacked); after fetching that one commit it exits 0 (`48 cases pass`). - `pnpm check:dual-build-cjs-loads` exits 3, `PREREQUISITE NOT MET` (48 packages have no `dist/`; it needs a whole-repo build). NOT MEASURED locally; this diff changes one declared type member and no emitted JavaScript. CI's Lint & Repo Gates runs it. - `check-changeset-no-major.mjs --base origin/main --event` against this body, offline: exit 0, `LEVEL AXIS … clause-② yes (widening) and metadata-protocol none … a non-protocol change at patch`. --- _Generated by [Claude Code](https://claude.ai/code/session_01ADHkmQS1wrCba8FkVowBLy)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 128681f commit 5d46f43

3 files changed

Lines changed: 89 additions & 2 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/spec': patch
3+
---
4+
5+
feat(spec): `ExplainAccessRequest` declares `recordIds`, the batch selector the explain wire schema already carries
6+
7+
Clause-②: yes (widening)
8+
Metadata-protocol: none
9+
10+
The `security` service contract's `explain(request)` selector, `ExplainAccessRequest`, now declares the optional `recordIds` member. The wire schema `ExplainRequestSchema` already declared it, and both the REST explain route and the reference security service already honour it. A TypeScript caller handing `explain` a batch no longer needs a cast to get past the contract type.
11+
12+
- **The type is the wire schema's own.** `recordIds` is typed as `ExplainRequest['recordIds']` (a list of ids), so the contract and the schema cannot drift; a type-level test holds the two equal.
13+
- **What it means.** The same record-grained question as `recordId`, for several records of one `(object, operation)` pair: mutually exclusive with `recordId`, at least one and at most `EXPLAIN_BATCH_MAX_RECORD_IDS` ids, answered by `ExplainDecision.records` in the same order.
14+
- **Nothing is required of an implementation.** A security service may ignore `recordIds` and answer only the object-level decision, with no `records`; the REST explain route then asks each id singularly, as it already does.
15+
- **No runtime change.** Only the declared type moves. Every request that compiled before still compiles.

‎packages/spec/src/contracts/security-service.test.ts‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
22

3-
import { describe, it, expect } from 'vitest';
3+
import { describe, it, expect, expectTypeOf } from 'vitest';
4+
import type { z } from 'zod';
45
import type {
56
ISecurityService,
67
AuthoredRowWriteVerdict,
@@ -9,7 +10,36 @@ import type {
910
ControlledByParentWriteOutcome,
1011
ControlledByParentWriteDenialLeg,
1112
ControlledByParentWriteUnresolvedReason,
13+
ExplainAccessRequest,
1214
} from './security-service';
15+
import type { ExplainRequestSchema } from '../security/explain.zod';
16+
17+
// ---------------------------------------------------------------------------
18+
// `ExplainAccessRequest.recordIds` — the service contract's batch selector.
19+
//
20+
// The wire schema (`ExplainRequestSchema`) declared `recordIds` and both the
21+
// REST explain route and the reference security service honour it, while the
22+
// service contract the route calls did not declare it. These pins hold the two
23+
// declarations to one type: the member is OPTIONAL (every singular and
24+
// object-level caller omits it) and EQUAL to the wire schema's input member, so
25+
// a retype on either side turns this file red under `check:test-typecheck`.
26+
//
27+
// The optionality pin is a type alias, not `expectTypeOf`: `toEqualTypeOf`
28+
// cannot tell `recordIds?: string[]` from a REQUIRED `recordIds: string[] |
29+
// undefined`, while `{} extends Pick<…>` can. Exported so TS6196 does not
30+
// flag an alias nothing reads.
31+
// ---------------------------------------------------------------------------
32+
33+
type Eq<A, B> = (<T>() => T extends A ? 1 : 2) extends (<T>() => T extends B ? 1 : 2) ? true : false;
34+
type Assert<T extends true> = T;
35+
36+
/** Optional: a request may omit it. A required member turns this alias red. */
37+
export type ExplainRecordIdsIsOptional = Assert<{} extends Pick<ExplainAccessRequest, 'recordIds'> ? true : false>;
38+
39+
/** The wire schema's own input type for the member — neither side may drift. */
40+
export type ExplainRecordIdsMatchesWire = Assert<
41+
Eq<ExplainAccessRequest['recordIds'], z.input<typeof ExplainRequestSchema>['recordIds']>
42+
>;
1343

1444
/**
1545
* These tests pin the two things a consumer of the `security` service reasons
@@ -563,6 +593,29 @@ describe('Security Service Contract', () => {
563593
expect(seen[0]).toEqual({ object: 'deal', operation: 'update', userId: 'u2', recordId: 'r1' });
564594
});
565595

596+
it('explain accepts a batch request: recordIds is the wire schema\'s member, typed as a list of ids', async () => {
597+
expectTypeOf<ExplainAccessRequest['recordIds']>()
598+
.toEqualTypeOf<z.input<typeof ExplainRequestSchema>['recordIds']>();
599+
expectTypeOf<ExplainAccessRequest['recordIds']>().toEqualTypeOf<string[] | undefined>();
600+
601+
const seen: ExplainAccessRequest[] = [];
602+
const service = makeService({
603+
explain: async (request) => { seen.push(request); return {} as any; },
604+
});
605+
// The literal compiling is the point: before the contract declared the
606+
// member, this call was an excess-property error.
607+
await service.explain(
608+
{ object: 'deal', operation: 'read', recordIds: ['r1', 'r2', 'r1'] },
609+
{ userId: 'admin' },
610+
);
611+
expect(seen[0]?.recordIds).toEqual(['r1', 'r2', 'r1']);
612+
613+
// A bare id is the singular selector's job, not a one-element batch.
614+
// @ts-expect-error — `recordIds` is a list, never a lone string.
615+
const bare: ExplainAccessRequest = { object: 'deal', operation: 'read', recordIds: 'r1' };
616+
expect(bare.object).toBe('deal');
617+
});
618+
566619
it('contributeOwnershipFloorAlternates is OPTIONAL — absence is typed, and an alternate names exactly one floor limb', () => {
567620
// A security service without the seam still satisfies the contract.
568621
// Absence leaves the floor in force, which is the fail-closed direction:

‎packages/spec/src/contracts/security-service.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
import type { GetRecordFieldAccessResponse } from '../api/protocol.zod.js';
6565
import type { FilterCondition } from '../data/filter.zod.js';
6666
import type { ExecutionContext } from '../kernel/execution-context.zod.js';
67-
import type { ExplainDecision, ExplainOperation } from '../security/explain.zod.js';
67+
import type { EXPLAIN_BATCH_MAX_RECORD_IDS, ExplainDecision, ExplainOperation, ExplainRequest } from '../security/explain.zod.js';
6868
import type { EffectiveObjectPermission, ObjectAccessScope, PermissionSet } from '../security/permission.zod.js';
6969

7070
/**
@@ -166,6 +166,25 @@ export interface ExplainAccessRequest {
166166
userId?: string;
167167
/** Narrow the explanation to a single record (adds record-level layers). */
168168
recordId?: string;
169+
/**
170+
* Batch form of `recordId`: the same record-grained question for several
171+
* records of one `(object, operation)` pair, in one call. The type is the
172+
* wire schema's own member ({@link ExplainRequest}), so the two cannot drift.
173+
*
174+
* - Mutually exclusive with `recordId`. The wire schema refuses a request
175+
* carrying both at the REST explain route, and the reference security
176+
* service refuses it too; an implementation that ignores `recordIds` does
177+
* not. A direct caller of `explain` sends one spelling, never both.
178+
* - At least one and at most {@link EXPLAIN_BATCH_MAX_RECORD_IDS} ids; the
179+
* wire schema refuses an empty or a longer array.
180+
* - Answered by `ExplainDecision.records` in the same order: `records[i]`
181+
* answers `recordIds[i]`, same length, duplicates answered per position.
182+
* - An implementation may ignore it and answer only the object-level
183+
* decision, with no `records`. A caller that needs the per-record verdicts
184+
* then asks each id singularly through `recordId`, as the REST explain
185+
* route does.
186+
*/
187+
recordIds?: ExplainRequest['recordIds'];
169188
}
170189

171190
/** Filter accepted by the audience-binding suggestion list. */

0 commit comments

Comments
 (0)