Skip to content

Commit 820eff9

Browse files
os-zhuangClaudeclaude
authored
fix(spec,plugin-approvals): the two approval vocabularies are derived, not hand-matched (#3786) (#4295)
* fix(spec,plugin-approvals): the two approval vocabularies are derived, not hand-matched (#3786) `sys_approval_request.status` and `sys_approval_action.action` spelled their option lists out — five values and twelve — each under a "Keep in sync with `ApprovalStatus` / `ApprovalActionKind` (spec/contracts)" comment, while the contract held the same sets as bare type unions. Seventeen strings matched by hand across a package boundary with nothing checking them. They did all still agree — verified verbatim before changing anything. But agreeing is not being held, and both directions of drift are quiet: a value the COLUMN accepts and the contract omits is invisible to every consumer typed against the contract (the row exists and nothing can narrow it), while a value the CONTRACT declares and the column rejects surfaces only at write time, on whichever tenant first reaches that transition. An audit vocabulary is a bad place for either. The contract now publishes the lists as values — APPROVAL_STATUSES and APPROVAL_ACTION_KINDS — with ApprovalStatus / ApprovalActionKind derived via `(typeof X)[number]`, and both columns spread the constants. The per-entry rationale (which action kinds move the flow, which are thread-only, why `returned` differs from `recalled`) moved onto the constants, where the values live. New exports, no behaviour change: the emitted option lists are byte-identical, checked against the built artifact before and after. Existing imports of the two types are unaffected — they resolve to the same unions. approval-vocabularies.test.ts pins the qualifier derivation alone cannot: the columns agree with the contract WHILE THE SPREAD IS THERE, so the test fails if either is re-inlined as a literal that drifted. It guards the guard (an unresolvable import would compare two empty lists and pass) and asserts the two vocabularies stay distinct — a copy-paste pointing one column at the other constant would satisfy "derived from the contract" while being the wrong vocabulary entirely. Mutation-tested in both directions: adding a value to APPROVAL_STATUSES propagates into the BUILT sys_approval_request.status options (the derivation is live, not a stale build), and re-inlining a drifted literal fails `sys_approval_request.status offers exactly the contract statuses, in order`. Verified: plugin-approvals 330 tests green, spec 7169 green, api-surface regenerated (0 breaking, 2 added), check:generated / exported-any / liveness / docs all PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb * docs(automation): name the source of the approval status list (#3786) The docs-drift check turned up a FOURTH copy of the status set, in the highest- visibility place: `automation/approvals.mdx`'s "Statuses in full" line. Unlike the CORS case in #4282 this one is correct today — all five values, same order — so there is nothing to fix, only something unguarded. Points at APPROVAL_STATUSES and says plainly that this line is a copy nothing updates automatically. That is weaker than derivation and I am not pretending otherwise: prose docs legitimately restate values, and a path-coupled test asserting this exact sentence would be brittle. Recorded as a follow-up instead — `check:variant-docs` already solves this shape for schema variants and is the right place to extend if the set starts moving. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UXGj3Z5TmwSV6RK2oGc3cb --------- Co-authored-by: Claude <support@objectstack.ai> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 5b47ab5 commit 820eff9

7 files changed

Lines changed: 188 additions & 35 deletions

File tree

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/plugin-approvals": patch
4+
---
5+
6+
fix(spec,plugin-approvals): the two approval vocabularies are derived, not hand-matched (#3786)
7+
8+
`sys_approval_request.status` and `sys_approval_action.action` spelled their
9+
option lists out — five values and twelve — each under a "Keep in sync with
10+
`ApprovalStatus` / `ApprovalActionKind` (spec/contracts)" comment, while the
11+
contract held the same sets as bare type unions. Seventeen strings matched by
12+
hand across a package boundary, with nothing checking them. They did all still
13+
agree; the sweep that found them (#3786) verified that verbatim before changing
14+
anything.
15+
16+
Agreeing is not the same as being held, and both directions of drift are quiet:
17+
18+
- a value the **column** accepts and the contract omits is invisible to every
19+
consumer typed against the contract — the row exists and nothing can narrow it;
20+
- a value the **contract** declares and the column rejects surfaces only at write
21+
time, on whichever tenant first reaches that transition.
22+
23+
An audit vocabulary is a bad place for either. So the contract now publishes the
24+
lists as values — `APPROVAL_STATUSES` and `APPROVAL_ACTION_KINDS` — with
25+
`ApprovalStatus` / `ApprovalActionKind` derived from them via
26+
`(typeof X)[number]`, and the two columns spread the constants. The per-entry
27+
rationale (which action kinds move the flow, which are thread-only, why
28+
`returned` differs from `recalled`) moved onto the constants, where the values
29+
live.
30+
31+
**New exports, no behaviour change.** The emitted option lists are byte-identical
32+
— verified against the built artifact before and after. Existing imports of the
33+
two types are unaffected; the types resolve to the same unions.
34+
35+
`approval-vocabularies.test.ts` pins the qualifier that derivation alone cannot:
36+
the columns agree with the contract *while the spread is there*, and the test
37+
fails if either is re-inlined as a literal that has drifted. It also guards the
38+
guard (an unresolvable import would compare two empty lists and pass) and asserts
39+
the two vocabularies stay distinct, since a copy-paste pointing one column at the
40+
other constant would satisfy "derived from the contract" while being the wrong
41+
vocabulary entirely.
42+
43+
Verified by mutation in both directions: adding a value to `APPROVAL_STATUSES`
44+
propagates into the built `sys_approval_request.status` options (the derivation
45+
is live, not a stale build), and re-inlining a drifted literal fails
46+
`sys_approval_request.status offers exactly the contract statuses, in order`.

content/docs/automation/approvals.mdx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,11 @@ On finalization the request row gets `status: 'approved'` (or `'rejected'`),
395395
mirrored to the same value, and the record unlocks. The parked run resumes down
396396
the `approve` or `reject` branch.
397397

398-
Statuses in full: `pending`, `approved`, `rejected`, `recalled`, `returned`.
398+
Statuses in full: `pending`, `approved`, `rejected`, `recalled`, `returned`
399+
the `APPROVAL_STATUSES` constant in `@objectstack/spec/contracts`, which is also
400+
what `sys_approval_request.status` offers and what `ApprovalStatus` is derived
401+
from. If you add a status there, this line is a fourth copy that nothing updates
402+
for you.
399403

400404
</Steps>
401405

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* The two approval vocabularies are DERIVED from the contract, not re-typed
5+
* (#3786).
6+
*
7+
* `sys_approval_request.status` and `sys_approval_action.action` used to spell
8+
* their option lists out — five values and twelve — each under a "Keep in sync
9+
* with `ApprovalStatus` / `ApprovalActionKind` (spec/contracts)" comment. They
10+
* happened to agree, but nothing made them, and both directions of drift are
11+
* quiet: a value the column accepts and the contract omits is invisible to every
12+
* consumer typed against the contract, while one the contract declares and the
13+
* column rejects surfaces only at write time, on whichever tenant first reaches
14+
* that transition. An audit vocabulary is a bad place for either.
15+
*
16+
* Both columns now spread `APPROVAL_STATUSES` / `APPROVAL_ACTION_KINDS`, so
17+
* disagreement is unrepresentable **while the spread is there**. What this file
18+
* pins is exactly that qualifier: it fails if someone re-inlines a literal that
19+
* has drifted. It cannot catch an identical re-inline — that is correct today
20+
* and fragile tomorrow — which is why the derivation, not this test, is the
21+
* mechanism.
22+
*/
23+
24+
import { describe, it, expect } from 'vitest';
25+
import { APPROVAL_STATUSES, APPROVAL_ACTION_KINDS } from '@objectstack/spec/contracts';
26+
27+
import { SysApprovalRequest } from './sys-approval-request.object.js';
28+
import { SysApprovalAction } from './sys-approval-action.object.js';
29+
30+
/** `Field.select` normalizes options to either bare strings or `{ value, label }`. */
31+
const optionValues = (obj: unknown, field: string): string[] => {
32+
const raw = (obj as any)?.fields?.[field]?.options ?? [];
33+
return raw.map((o: unknown) => (typeof o === 'string' ? o : (o as any)?.value));
34+
};
35+
36+
describe('approval vocabularies are derived from @objectstack/spec/contracts (#3786)', () => {
37+
it('the contract vocabularies are non-empty and reachable', () => {
38+
// Guard the guard: an unresolvable import would make both assertions below
39+
// compare an empty list against an empty list and pass.
40+
expect(APPROVAL_STATUSES.length).toBeGreaterThan(3);
41+
expect(APPROVAL_ACTION_KINDS.length).toBeGreaterThan(8);
42+
});
43+
44+
it('sys_approval_request.status offers exactly the contract statuses, in order', () => {
45+
expect(optionValues(SysApprovalRequest, 'status')).toEqual([...APPROVAL_STATUSES]);
46+
});
47+
48+
it('sys_approval_action.action offers exactly the contract action kinds, in order', () => {
49+
expect(optionValues(SysApprovalAction, 'action')).toEqual([...APPROVAL_ACTION_KINDS]);
50+
});
51+
52+
it('the two vocabularies stay distinct', () => {
53+
// A copy-paste that pointed one column at the other constant would satisfy
54+
// "derived from the contract" while being the wrong vocabulary entirely.
55+
expect([...APPROVAL_STATUSES]).not.toEqual([...APPROVAL_ACTION_KINDS]);
56+
expect(optionValues(SysApprovalRequest, 'status')).not.toEqual(
57+
optionValues(SysApprovalAction, 'action'),
58+
);
59+
});
60+
});

packages/plugins/plugin-approvals/src/sys-approval-action.object.ts

Lines changed: 6 additions & 6 deletions
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

33
import { ObjectSchema, Field } from '@objectstack/spec/data';
4+
import { APPROVAL_ACTION_KINDS } from '@objectstack/spec/contracts';
45

56
/**
67
* sys_approval_action — Audit trail row per approval action.
@@ -98,12 +99,11 @@ export const SysApprovalAction = ObjectSchema.create({
9899
}),
99100

100101
action: Field.select(
101-
// Keep in sync with `ApprovalActionKind` (spec/contracts). reassign /
102-
// remind / request_info / comment are thread interactions — they never
103-
// move the flow. revise / resubmit (ADR-0044) DO move it: send back for
104-
// revision and the later resubmission. ooo_substitute (#1322 M1) is a
105-
// system-recorded reroute of an out-of-office approver — no flow movement.
106-
['submit', 'approve', 'reject', 'recall', 'escalate', 'reassign', 'remind', 'request_info', 'comment', 'revise', 'resubmit', 'ooo_substitute'],
102+
// Spread from the contract, not re-typed (#3786). `APPROVAL_ACTION_KINDS`
103+
// is where the list and the per-kind notes live (which kinds move the flow
104+
// and which are thread-only); `ApprovalActionKind` is derived from it, so
105+
// this column and the contract cannot disagree.
106+
[...APPROVAL_ACTION_KINDS],
107107
{
108108
label: 'Action',
109109
required: true,

packages/plugins/plugin-approvals/src/sys-approval-request.object.ts

Lines changed: 5 additions & 3 deletions
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

33
import { ObjectSchema, Field } from '@objectstack/spec/data';
4+
import { APPROVAL_STATUSES } from '@objectstack/spec/contracts';
45

56
/**
67
* sys_approval_request — Live approval instance.
@@ -128,9 +129,10 @@ export const SysApprovalRequest = ObjectSchema.create({
128129
}),
129130

130131
status: Field.select(
131-
// Keep in sync with `ApprovalStatus` (spec/contracts). `returned` =
132-
// sent back for revision (ADR-0044) — terminal for this round.
133-
['pending', 'approved', 'rejected', 'recalled', 'returned'],
132+
// Spread from the contract, not re-typed (#3786). `APPROVAL_STATUSES` is
133+
// where the list and the reason for each entry live; `ApprovalStatus` is
134+
// derived from it, so this column and the contract cannot disagree.
135+
[...APPROVAL_STATUSES],
134136
{
135137
label: 'Status',
136138
required: true,

packages/spec/api-surface.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3544,6 +3544,8 @@
35443544
"AIToolCall (type)",
35453545
"AIToolDefinition (interface)",
35463546
"AIToolResult (type)",
3547+
"APPROVAL_ACTION_KINDS (const)",
3548+
"APPROVAL_STATUSES (const)",
35473549
"AdapterContext (interface)",
35483550
"AdapterSearchOptions (interface)",
35493551
"AnalyticsQuery (interface)",

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

Lines changed: 64 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,33 @@
1919
import type { SharingExecutionContext } from './sharing-service.js';
2020

2121
/**
22-
* Lifecycle state of an approval request.
22+
* Lifecycle states of an approval request, in the order the
23+
* `sys_approval_request.status` select presents them.
24+
*
25+
* A VALUE, not only a type, so `plugin-approvals` can spread it into that select
26+
* rather than re-typing the list (#3786). It used to be a bare union under a
27+
* "keep in sync with the `sys_approval_request` status select" comment, with the
28+
* plugin holding the second copy. The two agreed — but nothing made them, and
29+
* both directions of drift fail quietly: a status the column accepts and the
30+
* contract omits is invisible to every consumer typed against the contract,
31+
* while one the contract declares and the column rejects surfaces only at write
32+
* time, on whichever tenant first reaches that transition.
2333
*
2434
* `returned` (ADR-0044): the approver sent the request back for revision —
2535
* terminal for THIS request/round; the flow walks the `revise` edge to a wait
2636
* point, and a later resubmit opens a fresh `pending` request (next round).
2737
* Distinct from `recalled` (submitter-initiated withdrawal).
28-
*
29-
* Dual-source: keep in sync with the `sys_approval_request` status select.
3038
*/
31-
export type ApprovalStatus = 'pending' | 'approved' | 'rejected' | 'recalled' | 'returned';
39+
export const APPROVAL_STATUSES = [
40+
'pending',
41+
'approved',
42+
'rejected',
43+
'recalled',
44+
'returned',
45+
] as const;
46+
47+
/** Lifecycle state of an approval request — derived from {@link APPROVAL_STATUSES}. */
48+
export type ApprovalStatus = (typeof APPROVAL_STATUSES)[number];
3249

3350
/** Live request row. */
3451
export interface ApprovalRequestRow {
@@ -211,27 +228,49 @@ export interface ApprovalRequestRow {
211228
};
212229
}
213230

214-
/** Kinds of entries on a request's audit trail. */
215-
export type ApprovalActionKind =
216-
| 'submit'
217-
| 'approve'
218-
| 'reject'
219-
| 'recall'
220-
| 'escalate'
221-
/** A pending approver handed their slot to someone else. */
222-
| 'reassign'
223-
/** The submitter nudged the pending approvers. */
224-
| 'remind'
225-
/** An approver asked the submitter for more information (request stays pending). */
226-
| 'request_info'
227-
/** A free-form reply on the thread (submitter or approver). */
228-
| 'comment'
229-
/** ADR-0044: an approver sent the request back for revision (request finalizes `returned`). */
230-
| 'revise'
231-
/** ADR-0044: the submitter resubmitted after rework (the next round's request opens with its own `submit`). */
232-
| 'resubmit'
233-
/** #1322 M1: an out-of-office approver's slot was auto-rerouted to their delegate at resolution time. */
234-
| 'ooo_substitute';
231+
/**
232+
* Kinds of entries on a request's audit trail, in the order the
233+
* `sys_approval_action.action` select presents them.
234+
*
235+
* A VALUE for the same reason as {@link APPROVAL_STATUSES}: `plugin-approvals`
236+
* spreads it into that select instead of holding a twelfth-entry copy of it
237+
* (#3786). Twelve hand-matched strings across a package boundary is the widest
238+
* of the sweep's remaining copies, and an audit vocabulary is the worst place
239+
* for a silent gap — a kind the column accepts but the contract omits produces
240+
* rows no typed consumer can narrow, and the audit trail is exactly what gets
241+
* read back when someone asks what happened.
242+
*
243+
* Only some entries need explaining; the first five are self-describing:
244+
* reassign a pending approver handed their slot to someone else
245+
* remind the submitter nudged the pending approvers
246+
* request_info an approver asked for more information (request stays pending)
247+
* comment a free-form reply on the thread (submitter or approver)
248+
* revise ADR-0044: sent back for revision (request finalizes `returned`)
249+
* resubmit ADR-0044: resubmitted after rework (the next round opens with
250+
* its own `submit`)
251+
* ooo_substitute #1322 M1: an out-of-office approver's slot was auto-rerouted
252+
* to their delegate at resolution time
253+
*
254+
* `reassign` / `remind` / `request_info` / `comment` / `ooo_substitute` are
255+
* thread interactions and never move the flow; `revise` / `resubmit` do.
256+
*/
257+
export const APPROVAL_ACTION_KINDS = [
258+
'submit',
259+
'approve',
260+
'reject',
261+
'recall',
262+
'escalate',
263+
'reassign',
264+
'remind',
265+
'request_info',
266+
'comment',
267+
'revise',
268+
'resubmit',
269+
'ooo_substitute',
270+
] as const;
271+
272+
/** Kinds of entries on a request's audit trail — derived from {@link APPROVAL_ACTION_KINDS}. */
273+
export type ApprovalActionKind = (typeof APPROVAL_ACTION_KINDS)[number];
235274

236275
/**
237276
* A file attached to a decision action (#3266) — the READ shape of one

0 commit comments

Comments
 (0)