diff --git a/.changeset/approval-vocabularies-derived.md b/.changeset/approval-vocabularies-derived.md new file mode 100644 index 0000000000..d5a9cd5ba9 --- /dev/null +++ b/.changeset/approval-vocabularies-derived.md @@ -0,0 +1,46 @@ +--- +"@objectstack/spec": minor +"@objectstack/plugin-approvals": patch +--- + +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; the sweep that found them (#3786) verified that verbatim before changing +anything. + +Agreeing is not the same as 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; +- 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. So the contract now publishes the +lists as values — `APPROVAL_STATUSES` and `APPROVAL_ACTION_KINDS` — with +`ApprovalStatus` / `ApprovalActionKind` derived from them via +`(typeof X)[number]`, and the two 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 +— verified against the built artifact before and after. Existing imports of the +two types are unaffected; the types resolve to the same unions. + +`approval-vocabularies.test.ts` pins the qualifier that derivation alone cannot: +the columns agree with the contract *while the spread is there*, and the test +fails if either is re-inlined as a literal that has drifted. It also guards the +guard (an unresolvable import would compare two empty lists and pass) and asserts +the two vocabularies stay distinct, since a copy-paste pointing one column at the +other constant would satisfy "derived from the contract" while being the wrong +vocabulary entirely. + +Verified by mutation 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`. diff --git a/content/docs/automation/approvals.mdx b/content/docs/automation/approvals.mdx index 177ac92a8a..d348da984d 100644 --- a/content/docs/automation/approvals.mdx +++ b/content/docs/automation/approvals.mdx @@ -395,7 +395,11 @@ On finalization the request row gets `status: 'approved'` (or `'rejected'`), mirrored to the same value, and the record unlocks. The parked run resumes down the `approve` or `reject` branch. -Statuses in full: `pending`, `approved`, `rejected`, `recalled`, `returned`. +Statuses in full: `pending`, `approved`, `rejected`, `recalled`, `returned` — +the `APPROVAL_STATUSES` constant in `@objectstack/spec/contracts`, which is also +what `sys_approval_request.status` offers and what `ApprovalStatus` is derived +from. If you add a status there, this line is a fourth copy that nothing updates +for you. diff --git a/packages/plugins/plugin-approvals/src/approval-vocabularies.test.ts b/packages/plugins/plugin-approvals/src/approval-vocabularies.test.ts new file mode 100644 index 0000000000..5535522774 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/approval-vocabularies.test.ts @@ -0,0 +1,60 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The two approval vocabularies are DERIVED from the contract, not re-typed + * (#3786). + * + * `sys_approval_request.status` and `sys_approval_action.action` used to spell + * their option lists out — five values and twelve — each under a "Keep in sync + * with `ApprovalStatus` / `ApprovalActionKind` (spec/contracts)" comment. They + * happened to agree, but nothing made them, 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, while one 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. + * + * Both columns now spread `APPROVAL_STATUSES` / `APPROVAL_ACTION_KINDS`, so + * disagreement is unrepresentable **while the spread is there**. What this file + * pins is exactly that qualifier: it fails if someone re-inlines a literal that + * has drifted. It cannot catch an identical re-inline — that is correct today + * and fragile tomorrow — which is why the derivation, not this test, is the + * mechanism. + */ + +import { describe, it, expect } from 'vitest'; +import { APPROVAL_STATUSES, APPROVAL_ACTION_KINDS } from '@objectstack/spec/contracts'; + +import { SysApprovalRequest } from './sys-approval-request.object.js'; +import { SysApprovalAction } from './sys-approval-action.object.js'; + +/** `Field.select` normalizes options to either bare strings or `{ value, label }`. */ +const optionValues = (obj: unknown, field: string): string[] => { + const raw = (obj as any)?.fields?.[field]?.options ?? []; + return raw.map((o: unknown) => (typeof o === 'string' ? o : (o as any)?.value)); +}; + +describe('approval vocabularies are derived from @objectstack/spec/contracts (#3786)', () => { + it('the contract vocabularies are non-empty and reachable', () => { + // Guard the guard: an unresolvable import would make both assertions below + // compare an empty list against an empty list and pass. + expect(APPROVAL_STATUSES.length).toBeGreaterThan(3); + expect(APPROVAL_ACTION_KINDS.length).toBeGreaterThan(8); + }); + + it('sys_approval_request.status offers exactly the contract statuses, in order', () => { + expect(optionValues(SysApprovalRequest, 'status')).toEqual([...APPROVAL_STATUSES]); + }); + + it('sys_approval_action.action offers exactly the contract action kinds, in order', () => { + expect(optionValues(SysApprovalAction, 'action')).toEqual([...APPROVAL_ACTION_KINDS]); + }); + + it('the two vocabularies stay distinct', () => { + // A copy-paste that pointed one column at the other constant would satisfy + // "derived from the contract" while being the wrong vocabulary entirely. + expect([...APPROVAL_STATUSES]).not.toEqual([...APPROVAL_ACTION_KINDS]); + expect(optionValues(SysApprovalRequest, 'status')).not.toEqual( + optionValues(SysApprovalAction, 'action'), + ); + }); +}); diff --git a/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts index b45a30d73a..df4152cabd 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-action.object.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { APPROVAL_ACTION_KINDS } from '@objectstack/spec/contracts'; /** * sys_approval_action — Audit trail row per approval action. @@ -98,12 +99,11 @@ export const SysApprovalAction = ObjectSchema.create({ }), action: Field.select( - // Keep in sync with `ApprovalActionKind` (spec/contracts). reassign / - // remind / request_info / comment are thread interactions — they never - // move the flow. revise / resubmit (ADR-0044) DO move it: send back for - // revision and the later resubmission. ooo_substitute (#1322 M1) is a - // system-recorded reroute of an out-of-office approver — no flow movement. - ['submit', 'approve', 'reject', 'recall', 'escalate', 'reassign', 'remind', 'request_info', 'comment', 'revise', 'resubmit', 'ooo_substitute'], + // Spread from the contract, not re-typed (#3786). `APPROVAL_ACTION_KINDS` + // is where the list and the per-kind notes live (which kinds move the flow + // and which are thread-only); `ApprovalActionKind` is derived from it, so + // this column and the contract cannot disagree. + [...APPROVAL_ACTION_KINDS], { label: 'Action', required: true, diff --git a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts index 4f810813fe..60f674bec2 100644 --- a/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts +++ b/packages/plugins/plugin-approvals/src/sys-approval-request.object.ts @@ -1,6 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { APPROVAL_STATUSES } from '@objectstack/spec/contracts'; /** * sys_approval_request — Live approval instance. @@ -128,9 +129,10 @@ export const SysApprovalRequest = ObjectSchema.create({ }), status: Field.select( - // Keep in sync with `ApprovalStatus` (spec/contracts). `returned` = - // sent back for revision (ADR-0044) — terminal for this round. - ['pending', 'approved', 'rejected', 'recalled', 'returned'], + // Spread from the contract, not re-typed (#3786). `APPROVAL_STATUSES` is + // where the list and the reason for each entry live; `ApprovalStatus` is + // derived from it, so this column and the contract cannot disagree. + [...APPROVAL_STATUSES], { label: 'Status', required: true, diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index 03d2d0cc73..b5b74b3f84 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -3554,6 +3554,8 @@ "AIToolCall (type)", "AIToolDefinition (interface)", "AIToolResult (type)", + "APPROVAL_ACTION_KINDS (const)", + "APPROVAL_STATUSES (const)", "AdapterContext (interface)", "AdapterSearchOptions (interface)", "AnalyticsQuery (interface)", diff --git a/packages/spec/src/contracts/approval-service.ts b/packages/spec/src/contracts/approval-service.ts index 6de7aba5e7..dc23e030f6 100644 --- a/packages/spec/src/contracts/approval-service.ts +++ b/packages/spec/src/contracts/approval-service.ts @@ -19,16 +19,33 @@ import type { SharingExecutionContext } from './sharing-service.js'; /** - * Lifecycle state of an approval request. + * Lifecycle states of an approval request, in the order the + * `sys_approval_request.status` select presents them. + * + * A VALUE, not only a type, so `plugin-approvals` can spread it into that select + * rather than re-typing the list (#3786). It used to be a bare union under a + * "keep in sync with the `sys_approval_request` status select" comment, with the + * plugin holding the second copy. The two agreed — but nothing made them, and + * both directions of drift fail quietly: a status the column accepts and the + * contract omits is invisible to every consumer typed against the contract, + * while one the contract declares and the column rejects surfaces only at write + * time, on whichever tenant first reaches that transition. * * `returned` (ADR-0044): the approver sent the request back for revision — * terminal for THIS request/round; the flow walks the `revise` edge to a wait * point, and a later resubmit opens a fresh `pending` request (next round). * Distinct from `recalled` (submitter-initiated withdrawal). - * - * Dual-source: keep in sync with the `sys_approval_request` status select. */ -export type ApprovalStatus = 'pending' | 'approved' | 'rejected' | 'recalled' | 'returned'; +export const APPROVAL_STATUSES = [ + 'pending', + 'approved', + 'rejected', + 'recalled', + 'returned', +] as const; + +/** Lifecycle state of an approval request — derived from {@link APPROVAL_STATUSES}. */ +export type ApprovalStatus = (typeof APPROVAL_STATUSES)[number]; /** Live request row. */ export interface ApprovalRequestRow { @@ -211,27 +228,49 @@ export interface ApprovalRequestRow { }; } -/** Kinds of entries on a request's audit trail. */ -export type ApprovalActionKind = - | 'submit' - | 'approve' - | 'reject' - | 'recall' - | 'escalate' - /** A pending approver handed their slot to someone else. */ - | 'reassign' - /** The submitter nudged the pending approvers. */ - | 'remind' - /** An approver asked the submitter for more information (request stays pending). */ - | 'request_info' - /** A free-form reply on the thread (submitter or approver). */ - | 'comment' - /** ADR-0044: an approver sent the request back for revision (request finalizes `returned`). */ - | 'revise' - /** ADR-0044: the submitter resubmitted after rework (the next round's request opens with its own `submit`). */ - | 'resubmit' - /** #1322 M1: an out-of-office approver's slot was auto-rerouted to their delegate at resolution time. */ - | 'ooo_substitute'; +/** + * Kinds of entries on a request's audit trail, in the order the + * `sys_approval_action.action` select presents them. + * + * A VALUE for the same reason as {@link APPROVAL_STATUSES}: `plugin-approvals` + * spreads it into that select instead of holding a twelfth-entry copy of it + * (#3786). Twelve hand-matched strings across a package boundary is the widest + * of the sweep's remaining copies, and an audit vocabulary is the worst place + * for a silent gap — a kind the column accepts but the contract omits produces + * rows no typed consumer can narrow, and the audit trail is exactly what gets + * read back when someone asks what happened. + * + * Only some entries need explaining; the first five are self-describing: + * reassign a pending approver handed their slot to someone else + * remind the submitter nudged the pending approvers + * request_info an approver asked for more information (request stays pending) + * comment a free-form reply on the thread (submitter or approver) + * revise ADR-0044: sent back for revision (request finalizes `returned`) + * resubmit ADR-0044: resubmitted after rework (the next round opens with + * its own `submit`) + * ooo_substitute #1322 M1: an out-of-office approver's slot was auto-rerouted + * to their delegate at resolution time + * + * `reassign` / `remind` / `request_info` / `comment` / `ooo_substitute` are + * thread interactions and never move the flow; `revise` / `resubmit` do. + */ +export const APPROVAL_ACTION_KINDS = [ + 'submit', + 'approve', + 'reject', + 'recall', + 'escalate', + 'reassign', + 'remind', + 'request_info', + 'comment', + 'revise', + 'resubmit', + 'ooo_substitute', +] as const; + +/** Kinds of entries on a request's audit trail — derived from {@link APPROVAL_ACTION_KINDS}. */ +export type ApprovalActionKind = (typeof APPROVAL_ACTION_KINDS)[number]; /** * A file attached to a decision action (#3266) — the READ shape of one