Skip to content

Commit 72217cd

Browse files
feat(spec): ApprovalActionRow declares acted_as, the slot an approval action was taken as (#21479)
Fixes #21458 Clause-②: yes ## What this does `ApprovalActionRow` in `@objectstack/spec/contracts` gains one optional member, `acted_as?: string`, declared next to `via_override`. This is the row type of an approval request's action log: `IApprovalService.listActions` returns it, `GET /api/v1/approvals/requests/:id/actions` passes the rows through as they are, and the client SDK types them. The docblock takes `via_override`'s form: - **What it records.** The pending-approver slot the action was admitted under, in the slot's stored spelling as it stood in the request's `pending_approvers`: a `position:NAME` address (or `role:NAME`, the deprecated pre-rename spelling), an email, or a user id. - **That it is never a person.** The person who acted is `actor_id`, which under ADR-0118 D1 holds a `sys_user` id or nothing. A slot addressed by a user id carries that id in `acted_as` as the slot's address. That makes no claim about who acted. - **When it is absent.** Either the action was not admitted through a slot (a submitter's own action, a system action, or an admin override, which `via_override` marks), or the row was written before the slot was recorded. So absent alone never proves that no slot was involved. There is no authorable surface, no Zod schema, no request-side change, and no producer. Triage ordered this member to land first (#21411 ruling B, `5954022700`; the `pm:retriage` answer Q1, `5960329631`). The approvals card's PR then writes it from `rowFromAction`. ## The member's name The name is `acted_as`. It is read from the #21411 dev report `5959282267`: - `plan_on_yes.column`: "sys-approval-action.object.ts gets acted_as: Field.text, maxLength 255"; - `open_questions[0]`: "add one optional member, acted_as?: string, to ApprovalActionRow". The seat's claim `5958740584` names no spelling ("a plugin-object field recording the slot"). Its branch `claude/issue-21411-approval-actor-person` holds no commit past `53fd35e3e`, the empty claim marker. The card's spelling and the approvals side's spelling agree, so there is nothing to reconcile. ## Files - `packages/spec/src/contracts/approval-service.ts`: the member and its docblock. The docblock names no tracker number. - `packages/spec/src/contracts/approval-service.test.ts`: the pins. - `.changeset/21458-spec-approval-action-acted-as.md`: `@objectstack/spec` `minor`, `Clause-②: yes`. Its level and header follow the precedent's spec changeset (`.changeset/21333-spec-boolean-comparand-contract.md` in `9f13c949b0`). ## Pins (the card's three) 1. **The member is optional and string-typed.** Two exported type aliases, which `check:test-typecheck` compiles: - `ActedAsIsOptional` asks whether an empty object type extends the `Pick` of `ApprovalActionRow` on `acted_as`. An `expectTypeOf` equality against `string | undefined` cannot tell an optional member from a REQUIRED `acted_as: string | undefined`, and this alias can. - `ActedAsIsAString` asserts the indexed type `ApprovalActionRow['acted_as']` equals `string | undefined` exactly. 2. **A row without it still type-checks.** A row literal with no `acted_as` is assigned to `ApprovalActionRow`. No Zod schema exists for this row type (it is a TypeScript interface), so "parses" means type-checks. The existing producer (`rowFromAction`) and the client SDK reference the type only as `ApprovalActionRow[]`. No `keyof ApprovalActionRow` or `Required` of it exists anywhere in the repo. 3. **The `api-surface` row is present.** `api-surface/contracts.json` carries `ApprovalActionRow (interface)`, and `export-origins/contracts.json` carries its origin. Regenerating both after a real build changed **no** byte, because these artifacts record export names, not members. `check:api-surface` reports "public API surface + factory signatures unchanged". This gate cannot see a member being added, so the pins in 1 and 2 are what hold the member. ## Ablation (one-shot, from the committed state) Each mutation went through `scripts/ablation-replace.mjs` (WRAP mode, trap restore), with `pnpm check:test-typecheck` as the measurement: - `acted_as?: string` → `acted_as: string` (required): anchor 1 → 0, blob `6ed13aee9a82` → `04260649fc42`. **RED**: "src/contracts/approval-service.test.ts: 5 type error(s)". It was restored with blob == HEAD `6ed13aee9a82` and `git diff HEAD` empty. - `acted_as?: string` → `acted_as?: string | null`: blob `6ed13aee9a82` → `675062e3d238`. **RED**: 3 type errors. It was restored the same way. The test file imports `./approval-service` from `src`, so neither ablation goes through `dist/`. **Reverse check against the rebuilt `.d.ts`.** This proves that a consumer reads the rebuilt type. A scratch module loaded `@objectstack/spec/contracts` from `packages/spec/dist/contracts/index.d.mts` (`--traceResolution`) and ran two legs: - `acted_as: 'position:finance'` is green. - `acted_as: 42` is red with `TS2322: Type 'number' is not assignable to type 'string'`. A stale `.d.ts` would have rejected both legs with an excess-property error instead. ## Verification at `cb86024a83` `cb86024a83` is this branch with `origin/main` `49524f6906` merged in, which touched other spec contracts and no file of this diff. Every build and test ran under `os-verify-lock`, and each exit code was written to disk before any pipe. | command | exit | verdict line | |---|---|---| | `pnpm --filter @objectstack/spec build` (with the objectql and lint closures) | 0 | `VERDICT command-exit 0` | | `pnpm --filter @objectstack/spec check:generated` | 0 | `All 15 generated artifacts are up to date` | | `pnpm --filter @objectstack/spec check:api-surface` | 0 | `public API surface + factory signatures unchanged` | | `pnpm --filter @objectstack/spec test` | 0 | `Test Files 602 passed (602)`, `Tests 17738 passed, 1 todo` | | `pnpm --filter @objectstack/spec typecheck` | 0 | `check:test-typecheck: OK`, 52 files / 246 errors / 135 pinned signatures held | | `vitest run src/contracts/approval-service.test.ts` (at `828d3f2db3`; the full suite above includes it) | 0 | 7 passed (6 before this diff) | | `node scripts/check-changeset-no-major.mjs --base origin/main` | 0 | `This diff introduces no major bump` | | the same, with `--event` carrying this PR body | 0 | `LEVEL AXIS: this PR declares clause-② yes, and no package whose packages/**/src/** it moves is graded patch` | | `node scripts/check-adr-0087-registration.mjs --base origin/main` | 0 | `this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen)` | | `node scripts/check-empty-changeset.mjs --base origin/main` | 0 | `No empty-frontmatter changeset introduced by this diff (1 declaring changeset(s) added)` | | `pnpm check:doc-authoring` | 0 | `17285 customer-facing string(s) across 1236 spec sources clean` | | `pnpm check:nul-bytes` | 0 | `scanned 9810 text file(s) ... no raw ASCII control bytes` | | `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --ran FILE` | 0 | `85 derived famil(ies) accounted for — 85 run, 0 NOT-MEASURED` | The derivation (`--commands`, no paths) gave the same 85 commands at `828d3f2db3`, `d576d5b4c7` and `cb86024a83`, and all 85 exited 0 at `cb86024a83`. In the first pass (`d576d5b4c7`), two gates exited 3 (PREREQUISITE NOT MET): - `check:lean-entry-closure` passed after an objectql-closure build. - `check:dual-build-cjs-loads` passed in the final pass. The dists that the final pass of `check:dual-build-cjs-loads` read were partly built by an earlier gate from the tree before the merge. This diff emits no JavaScript (an interface member), and CI builds fresh. `origin/main` has since moved one more commit (`aa4632235b`, a page-block spec change). It touches no file of this diff and was not merged in. CI runs on the merge ref. ## Acceptance notes - **The window before the producer lands.** Until the approvals card's PR merges, no producer writes `acted_as`, and every row omits it. That is the member's declared absent case. On slot-gated rows, `actor_id` still holds the slot literal. That is the ADR-0118 D1 violation the approvals card exists to remove, and it is not touched here (the card forbids a producer change). The docblock describes the ruled contract. The changeset says that the approvals service's own changeset states when `listActions` starts returning the member. - **No REST or client edit.** REST passes `listActions` rows through as `{ data: rows }`, and `packages/client` re-exports the spec type. No gate asked for an edit. - **The DTO's `actor_id` has no docblock.** The ADR-0118 D1 description the approvals card plans is for the object field (`sys-approval-action.object.ts`), not for this row type. Noted, not filed. Carrier: none. - A contract-tier review is owed before the queue, as the claim says (a published response contract widens). --- _Generated by [Claude Code](https://claude.ai/code/session_01YDt3PzwfrkuFzUBF89WPmM)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7097859 commit 72217cd

3 files changed

Lines changed: 85 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": minor
3+
---
4+
5+
feat(spec): `ApprovalActionRow` declares `acted_as`, the pending-approver slot an approval action was taken as, beside the person in `actor_id`
6+
7+
Clause-②: yes
8+
9+
**What it declares.** One optional string member on `ApprovalActionRow` in `@objectstack/spec/contracts`, the row type of an approval request's action log (`IApprovalService.listActions`, served at `GET /api/v1/approvals/requests/:id/actions` and typed by the client SDK):
10+
11+
- `acted_as?: string` is the slot the action was admitted under, in the slot's stored spelling as it stood in the request's `pending_approvers`: a `position:<name>` address (or `role:<name>`, the deprecated pre-rename spelling), an email, or a user id.
12+
- It is never a person. The person who acted is `actor_id`, which under ADR-0118 D1 holds a `sys_user` id or nothing. A slot addressed by a user id carries that id in `acted_as` as the slot's address, which makes no claim about who acted.
13+
- Absent means the action was not admitted through a slot (a submitter's own action, a system action, or an admin override, which `via_override` marks), or the row was written before the slot was recorded. So absent alone never proves that no slot was involved.
14+
15+
**What moves for consumers.** Nothing in this package writes the member, and every existing export and member is unchanged: a row without `acted_as` conforms exactly as before. The approvals service is its producer, and that package's own changeset states when `listActions` starts returning it. Until then every row omits it, which is the member's declared absent case. A client that renders the action log can show `acted_as` beside the actor's name as the capacity the actor acted in.

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

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,53 @@ describe('approval row organization_id declaration (#10331)', () => {
6363
});
6464
});
6565

66+
// ---------------------------------------------------------------------------
67+
// [#21458] `ApprovalActionRow.acted_as` — the slot an action was taken as.
68+
//
69+
// Triage ruled the person into `actor_id` and the slot into a field of its own
70+
// (#21411 ruling B); the action log shows the slot, so the published row type
71+
// declares it. The pins hold what the card decided: the member is OPTIONAL and
72+
// a STRING, and a row without it (every producer that does not write it, and
73+
// every row written before it existed) still conforms.
74+
//
75+
// The optionality pin is a type alias, not `expectTypeOf`: `toEqualTypeOf<string
76+
// | undefined>` cannot tell `acted_as?: string` from a REQUIRED `acted_as: string
77+
// | undefined`, while `{} extends Pick<…>` can. Exported for the same TS6196
78+
// reason the aliases below are; `check:test-typecheck` compiles this file.
79+
// ---------------------------------------------------------------------------
80+
81+
type Eq<A, B> = (<T>() => T extends A ? 1 : 2) extends (<T>() => T extends B ? 1 : 2) ? true : false;
82+
type Assert<T extends true> = T;
83+
84+
/** Optional: a row may omit it. A required member turns this alias red. */
85+
export type ActedAsIsOptional = Assert<{} extends Pick<ApprovalActionRow, 'acted_as'> ? true : false>;
86+
87+
/** A string slot address — not a boolean flag, not an object, not nullable. */
88+
export type ActedAsIsAString = Assert<Eq<ApprovalActionRow['acted_as'], string | undefined>>;
89+
90+
describe('[#21458] ApprovalActionRow.acted_as — the slot an approval action was taken as', () => {
91+
it('is optional and string-typed: a row without it still conforms, and one with it reads back', () => {
92+
const read = (row: ApprovalActionRow): string | undefined => row.acted_as;
93+
94+
expectTypeOf<ApprovalActionRow['acted_as']>().toEqualTypeOf<string | undefined>();
95+
96+
// The shape every existing producer returns: no slot member at all.
97+
// This literal failing to compile is the regression.
98+
const withoutSlot: ApprovalActionRow = {
99+
id: 'aact_1',
100+
request_id: 'req_1',
101+
action: 'approve',
102+
actor_id: 'usr_holder',
103+
};
104+
expect(read(withoutSlot)).toBeUndefined();
105+
106+
// The person and the slot side by side, each in its own member.
107+
const withSlot: ApprovalActionRow = { ...withoutSlot, acted_as: 'position:finance' };
108+
expect(read(withSlot)).toBe('position:finance');
109+
expect(withSlot.actor_id).toBe('usr_holder');
110+
});
111+
});
112+
66113
// ---------------------------------------------------------------------------
67114
// [#15389] `continueRestoredRun` — the approvals half of the operator repair
68115
// pair, declared on the contract.
@@ -82,8 +129,6 @@ describe('approval row organization_id declaration (#10331)', () => {
82129
// file.
83130
// ---------------------------------------------------------------------------
84131

85-
type Eq<A, B> = (<T>() => T extends A ? 1 : 2) extends (<T>() => T extends B ? 1 : 2) ? true : false;
86-
type Assert<T extends true> = T;
87132
type ContinueRestoredRun = NonNullable<IApprovalService['continueRestoredRun']>;
88133

89134
/**

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,29 @@ export interface ApprovalActionRow {
513513
* predates the column — "not recorded", which is not the same claim.
514514
*/
515515
via_override?: boolean;
516+
/**
517+
* The pending-approver slot this action was taken as: the slot's address in
518+
* its stored spelling, exactly as it stood in the request's
519+
* `pending_approvers` when the action was admitted — a `position:<name>`
520+
* address (or `role:<name>`, the deprecated pre-rename spelling), an email,
521+
* or a user id.
522+
*
523+
* "Who acted" and "as which slot" are two facts. One holder of a position
524+
* can act for it, and one person can hold several slots, so the slot is
525+
* recorded beside the person rather than in place of them. It is what a
526+
* timeline shows as the capacity an approver acted in.
527+
*
528+
* It is never a person. The person who acted is `actor_id` (a `sys_user` id
529+
* or nothing, ADR-0118 D1). A slot addressed by a user id carries that id
530+
* here as the slot's address, which makes no claim about who acted.
531+
*
532+
* Absent means one of two things, and this member alone does not tell them
533+
* apart: the action was not admitted through a slot (a submitter's own
534+
* action, a system action, or an admin override — see `via_override`), or
535+
* the row was written before the slot was recorded. So absent alone never
536+
* proves that no slot was involved: "not recorded" is not the same claim.
537+
*/
538+
acted_as?: string;
516539
}
517540

518541
/** Input for a decision on an approval request. */

0 commit comments

Comments
 (0)