Skip to content

Commit 9f05b7d

Browse files
Jack Qclaude
andauthored
feat(spec): declare organization_id on ApprovalRequestRow and ApprovalActionRow (#10465)
* feat(spec): declare organization_id on ApprovalRequestRow and ApprovalActionRow The approval service stamps the tenancy placement on every row it inserts (and returns it on request-row reads), but the published contract types omitted the field, forcing consumers to cast past the contract. Type-only widening per the maintainer ruling on #10331 (option A). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016gcKVsiywU9CcS96S5t9qD * test(spec): use expectTypeOf idiom for the organization_id pins The Expect/Equals helper aliases trip noUnusedLocals (TS6196) under the test-typecheck ratchet; expectTypeOf is the live idiom in spec tests (api/protocol.test.ts). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016gcKVsiywU9CcS96S5t9qD --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0c24898 commit 9f05b7d

4 files changed

Lines changed: 99 additions & 0 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
Declare `organization_id?: string | null` on `ApprovalRequestRow` and
6+
`ApprovalActionRow` (#10331). The approval service has always stamped the
7+
tenancy placement on the rows it inserts — and returns it on request-row
8+
reads — but the published contract types omitted the field, so consumers had
9+
to cast past the contract to reach it. Type-only widening: one declared
10+
optional field per row, no runtime change.

packages/plugins/plugin-approvals/src/approver-cross-org.integration.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ describe('ADR-0105 D9 — cross-org approver targeting through ApprovalService',
129129
// This is the whole point of the split: targeting must not relocate the
130130
// request into the approver's organization, or the plant would lose its own
131131
// audit trail to the group.
132+
// (#10331 has since declared `organization_id` on the contract row types,
133+
// so returned rows carry it typed; these persisted-row reads stay because
134+
// storage placement across all three tables is exactly what's pinned here.)
132135
expect(engine._tables['sys_approval_request'][0].organization_id).toBe('o_plant');
133136
expect(engine._tables['sys_approval_approver'][0].organization_id).toBe('o_plant');
134137
expect(engine._tables['sys_approval_action'][0].organization_id).toBe('o_plant');
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// [#10331] `organization_id` existed on the wire (plugin-approvals stamps it
4+
// on insert and `rowFromRequest` returns it) but not on the published row
5+
// types, so every consumer cast past the contract to reach it. These pins keep
6+
// the declaration honest: the field must stay readable off the DECLARED types
7+
// without a cast, at the exact optional-nullable shape the write expression
8+
// produces (`context.organizationId ?? context.tenantId ?? input.organizationId
9+
// ?? null` — a resolved org id, or `null` when none resolved, or absent on
10+
// rows written before stamping existed).
11+
12+
import { describe, it, expect, expectTypeOf } from 'vitest';
13+
import type { ApprovalActionRow, ApprovalRequestRow } from './approval-service';
14+
15+
describe('approval row organization_id declaration (#10331)', () => {
16+
it('is readable off ApprovalRequestRow without a cast, at the stamped shape', () => {
17+
// Reading the property off the declared type — no `as`, no indexing
18+
// through `any`. This line failing to compile is the regression.
19+
const read = (row: ApprovalRequestRow): string | null | undefined => row.organization_id;
20+
21+
// Exactly `string | null | undefined`: `null` is the write path's
22+
// "no org resolved" value and must not be silently narrowed away.
23+
expectTypeOf<ApprovalRequestRow['organization_id']>().toEqualTypeOf<string | null | undefined>();
24+
25+
// Optional: a row written before the stamp existed still satisfies the
26+
// type (this object literal fails to compile if the field is required).
27+
const preStamp: ApprovalRequestRow = {
28+
id: 'req_1',
29+
process_name: 'flow:review',
30+
object_name: 'showcase_project',
31+
record_id: 'rec_1',
32+
status: 'pending',
33+
};
34+
expect(read(preStamp)).toBeUndefined();
35+
expect(read({ ...preStamp, organization_id: null })).toBeNull();
36+
expect(read({ ...preStamp, organization_id: 'o_plant' })).toBe('o_plant');
37+
});
38+
39+
it('is readable off ApprovalActionRow without a cast, at the stamped shape', () => {
40+
// Every `sys_approval_action` insert site stamps the owning request's
41+
// org on the persisted row; see the field's docblock for the read-path
42+
// caveat (the service's `rowFromAction` mapping does not surface it).
43+
const read = (row: ApprovalActionRow): string | null | undefined => row.organization_id;
44+
45+
expectTypeOf<ApprovalActionRow['organization_id']>().toEqualTypeOf<string | null | undefined>();
46+
47+
const minimal: ApprovalActionRow = {
48+
id: 'aact_1',
49+
request_id: 'req_1',
50+
action: 'submit',
51+
};
52+
expect(read(minimal)).toBeUndefined();
53+
expect(read({ ...minimal, organization_id: null })).toBeNull();
54+
expect(read({ ...minimal, organization_id: 'o_plant' })).toBe('o_plant');
55+
});
56+
});

packages/spec/src/contracts/approval-service.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,22 @@ export const APPROVAL_STATUS_LABELS = {
8383
/** Live request row. */
8484
export interface ApprovalRequestRow {
8585
id: string;
86+
/**
87+
* Tenancy placement stamp (#10331): the organization the request belongs
88+
* to. `plugin-approvals` writes it on the inserted `sys_approval_request`
89+
* row at submit time — resolved as
90+
* `context.organizationId ?? context.tenantId ?? input.organizationId ?? null`
91+
* — and its read mapping (`rowFromRequest`) returns it on every service
92+
* read. The field was always on the wire; this declaration types it so
93+
* consumers (and tests pinning org placement) read it without casting past
94+
* the contract.
95+
*
96+
* Optional-nullable: rows written before the stamp existed carry no value,
97+
* and a submit whose context resolves no org stamps `null` — "owned by no
98+
* organization", which org-scoped reads treat as globally visible rather
99+
* than tenant-owned.
100+
*/
101+
organization_id?: string | null;
86102
/** Origin of the request — `flow:<flowName|nodeId>` for node-driven approvals. */
87103
process_name: string;
88104
object_name: string;
@@ -364,6 +380,20 @@ export interface ApprovalActionAttachment {
364380
export interface ApprovalActionRow {
365381
id: string;
366382
request_id: string;
383+
/**
384+
* Tenancy placement stamp (#10331), same semantics as
385+
* {@link ApprovalRequestRow.organization_id}: every `sys_approval_action`
386+
* insert site in `plugin-approvals` stamps the owning request's org (or
387+
* `null` when none resolved), so the persisted row always carries it.
388+
*
389+
* Read-path caveat, measured at declaration time: the service's
390+
* `rowFromAction` mapping does not surface it, so rows returned by
391+
* `listActions` currently omit the field — it is populated when a persisted
392+
* row is read directly off the engine (the pattern org-placement tests
393+
* use). Declared optional-nullable for that gap as well as for pre-stamp
394+
* rows.
395+
*/
396+
organization_id?: string | null;
367397
step_name?: string;
368398
step_index?: number;
369399
action: ApprovalActionKind;

0 commit comments

Comments
 (0)