Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions .changeset/approval-vocabularies-derived.md
Original file line number Diff line number Diff line change
@@ -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`.
6 changes: 5 additions & 1 deletion content/docs/automation/approvals.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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.

</Steps>

Expand Down
Original file line number Diff line number Diff line change
@@ -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'),
);
});
});
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions packages/spec/api-surface.json
Original file line number Diff line number Diff line change
Expand Up @@ -3554,6 +3554,8 @@
"AIToolCall (type)",
"AIToolDefinition (interface)",
"AIToolResult (type)",
"APPROVAL_ACTION_KINDS (const)",
"APPROVAL_STATUSES (const)",
"AdapterContext (interface)",
"AdapterSearchOptions (interface)",
"AnalyticsQuery (interface)",
Expand Down
89 changes: 64 additions & 25 deletions packages/spec/src/contracts/approval-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
Loading