Skip to content

Commit cde9a80

Browse files
docs(skills): an approval's decision outputs are enforced at the decision in the automation skill (#29928)
domain:skills#1 · huangyiirene · session_01J8VVAnMUWA9rufXzFaEh4J <!-- claude-projects-attribution: {"github_login":"huangyiirene"} --> _Requested by **Irene** · [project thread](https://claude.ai/code/project/chan_01TJXtQ1tVD26ecLDZxFS67x?thread=cmsg_01TJXtQ1tVD26ecLDZxFS67xXaMfp5V5DwGdQNfBzjZJEJ)_ Fixes #29512 Clause-②: no Metadata-protocol: none Fable mandate waived by the maintainer; contract review by triage. ## 维护者速读 - 改了什么:objectstack-automation 技能的审批参考文档现在教「审批节点声明的 `decisionOutputs` 在作出决定时被强制校验」:带类型的条目仍渲染对应选择器,同时 `type` 与 `multiple` 在同意和驳回时、任何写入之前都会检查;picker 值按 `expression` 审批人的解析器解析(`user` 任一账号的记录 id,`department` 本组织的记录 id,`team` 不属于别的组织的记录 id,`position` 本组织有人担任的岗位机器名);`multiple: true` 只收不重复的列表;其他非空值一律 `400 VALIDATION_FAILED` 并点名 `outputs.KEY`;Console 对 `multiple: true` 的文本输出发送列表。附 `@objectstack/skills` patch changeset。 - 为什么改:PR #29372(卡 #28658,裁决 A 6566731522)已把校验合入引擎,其 `Skills-docs:` 行欠下本卡;技能原文只说「带类型的条目渲染对应的记录选择器」,AI 写元数据时会以为 `type`/`multiple` 只影响界面。 - 风险与回滚:纯技能文本,无代码、无契约改动。token 天花板就地付账(删掉同一文件里已有原文的七处复述),上限行未动,无 re-wrap。回滚即 revert。 - 先看:`skills/objectstack-automation/references/state-machines-and-approvals.md` 的 *Recording a decision* 节新增段落,以及 Node Config 表的 `decisionOutputs` 行。 - 两分钟验证:`node scripts/check-skills-token-ratchet.mjs`,预期该文件 5724/5730,全绿。 - 需要你做的:Tier H 受管面(`skills/**`),需要 `GOVERNED_APPROVERS` 的批准;契约复审由 triage 做;PR 保持 draft。 ## What changes Facts are taken from current `main`, not the card text alone: `assertDecisionOutputsMatchDeclarations` and `decisionOutputValueRefusal` in `packages/plugins/plugin-approvals/src/approval-service.ts` (called before any write, on approve and reject), `content/docs/automation/approvals.mdx` → Dynamic approvers (as updated by #29371), and `ui/packages/app-shell/src/utils/decisionOutputParams.ts` (a `multiple: true` free-value output rides a `tags` param), pinned by `ActionParamDialog.decisionOutputList-28658.test.tsx`. Two files: `skills/objectstack-automation/references/state-machines-and-approvals.md` and its changeset. 1. **Recording a decision.** One new paragraph after the existing decision-outputs paragraph, which is byte-identical: a typed entry renders the matching picker in the decision dialog, and `type` and `multiple` are also enforced at the decision, on approve and reject alike, before any write. A picker value must resolve through the `expression` approver's resolvers, each scoped as the code scopes it: `user` by any account's record id (`reassignTargetRefusal(value, null)`), `department` by record id in the request's organization (`businessUnitIsLive`), `team` by record id not owned by another organization (`readTeamRow` + `teamOrganizationOf`), `position` by the machine name of a position someone in that organization holds (`expandPositionUsers`). `multiple: true` takes a list of distinct values (every kind, bare keys included); without it, one value. Any other non-blank value is `400 VALIDATION_FAILED` naming `outputs.KEY` (a blank value is left to `required`). The Console sends a list for a `multiple: true` text output. The paragraph points at the existing `lead_review` node (`{ key: 'next_reviewers', label: 'Next Reviewers', type: 'user', multiple: true, required: true }`) in the dynamic-approval flow as its example, so no second copy of the example is added. 2. **Node Config table, `decisionOutputs` row.** "a typed entry renders the matching record picker in the decision dialog (`multiple` collects an id array)" is replaced by "enforced at the decision — see *Recording a decision*". The declaration shape stays in the row. 3. **The dynamic-approval flow's comments.** The `lead_review` comment drops "The TYPED declaration renders a multi-select sys_user picker in the decision dialog" (the new paragraph keeps the picker fact) and keeps the `POST …/approve { outputs: { next_reviewers: ['u2', 'u3'] } }` line. The code itself is unchanged. 4. **`.changeset/29512-approval-decision-outputs-skill.md`.** `'@objectstack/skills': patch` with both declaration lines and a Before/Now body: `skills/**` ships byte for byte in `@objectstack/skills`, so the change publishes. ### Token ratchet, paid in place (no ceiling row moved, no re-wrap) | file | bytes before / after | tokens before / after / ceiling | lines before / after | |:--|:--|:--|:--| | `references/state-machines-and-approvals.md` | 22918 / 22894 | 5730 / 5724 / 5730 (headroom 0 → 6) | 392 / 399 | The new text is paid by deleting restated text whose home stays in the same file. Each deletion removes whole lines, a span of one table row, or a trailing comment. No surrounding line is reflowed: - `decisionOutputs` row: "(author declares, approvers fill values)" and "Accepted outputs resume the run as `<nodeId>.<key>` variables; undeclared keys reject the decision; `decision`/`requestId` reserved". Home: *Recording a decision*, which states all four. - `lead_review` comment: "`required: true` is enforced by the runtime on APPROVE (never on reject) — node B below has nobody to route to without it." Home: *Recording a decision* ("A declaration marked `required: true` must carry a non-blank value to **approve** (never to reject) — enforced before any write …"). - `co_sign` comment: "Node B resolves them at entry from the lead's decision outputs." Home: example ② in *Dynamic approvers* ("The previous approval node's decision outputs pick this node's approvers") and the node's own `vars.lead_review.next_reviewers` approver. - `expression` row of Approver Types: "Only `current.*` / `trigger.*` / `vars.*` roots are available;". Home: the roots table directly below in *Dynamic approvers* ("Its CEL source sees exactly **three roots** — nothing else"). - The first example's three trailing comments, `// or 'unanimous' / 'quorum' / 'per_group'`, `// lock the record while pending` and `// mirror pending|approved|rejected|recalled onto the row`. Home: the Node Config table's `behavior`, `lockRecord` and `approvalStatusField` rows, which state each in full. The three keys and values stay. No repo test pins this file's text (`grep -rl state-machines-and-approvals packages ui/packages` finds nothing). ## Why PR #29372 (card #28658, ruling A 6566731522) made the approvals service judge each declared decision output at the decision, and its `Skills-docs:` line owed this card. The skill still described `type` and `multiple` as what the decision dialog renders, so an AI authoring an approval flow from it had no way to know that a mismatched value is refused with `400 VALIDATION_FAILED` before anything is written, that a `position` output takes a machine name and not a `sys_position` id, or that a `multiple: true` output refuses a repeated entry. ## Verification Head `a6110351` (four commits: d070954 the change, 1912f51 a one-phrase clarity fix, 597efdf the contract review's MF-1/SF-1/SF-2, a611035 the changeset renamed and given the Before/Now shape). No code changed, so no package test, typecheck, ablation or dogfood battery is owed (`Clause-②: no`, no `!` in the title). <details><summary>Local gates on a611035: 43 derived, all 43 exit 0 (<code>dispatch-gates --ran</code>: 0 NOT-MEASURED)</summary> - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 43 commands for the two changed files (the 33 of round 1 plus 10 changeset families: `check-adr-0087-registration.mjs` and its `--self-test`, `check-changeset-no-major.mjs`, `check-empty-changeset.mjs` and its `--self-test`, `release-rehearsal-clone.mjs --self-test`, `release-pending-publish.mjs --self-test`, `check:changeset-gate-self-tests`, `check:closed-ceremony-text`, `check:published-files`), the same list on 597efdf and a611035. - All 43 ran in a tree with `pnpm install --frozen-lockfile` (exit 0) and a full `pnpm build` (exit 0), each with a 900 s timeout, each exit code captured before any pipe. All 43 exited 0, among them `check-skills-token-ratchet.mjs` (5724 of 5730), `check-comment-mask-corpus.mjs`, `check:doc-authoring`, `check:skill-refs`, `check:skill-identifier-liveness`, `check:role-word`, `check:pm-governed-merges` and `check:dual-build-cjs-loads`. - `--ran` printed: `43 derived famil(ies) accounted for — 43 run, 0 NOT-MEASURED`. - `node scripts/check-commit-card-trailers.mjs --range origin/main..HEAD`: exit 0 (four commits, no card relation, model-free trailer pair). - Round 1 (1912f51): 33 derived, all 33 exit 0. CI on 1912f51: every check-run `success` or skipped by path filter or opt-in. CI on 597efdf was cancelled by the a611035 push (its two aggregate jobs read `failure` over cancelled jobs). - Writes: `git push` ×5 (the bare branch pointer, d070954, 1912f51, 597efdf, a611035); one relay `pr_create`; no GitHub MCP write. </details> ## Acceptance notes - Round 2 answers the contract review of 1912f51 (comment 6606420300). MF-1: the changeset is added and the `skip-changeset` label is gone (the seat removed it). SF-1: the organization scope applies per kind, as the code scopes it. SF-2: the paragraph keeps that a typed entry renders the matching picker. The review's blank-value gap is closed by "any other non-blank value". - `content/docs/automation/approvals.mdx` carries the same "in the request's organization" overstatement for a `user` output. It is outside this PR's file surface and is the docs lane's follow-up, per the review. - No eval case was added. No eval in `skills/objectstack-automation/evals/**` covers `decisionOutputs` today, and the card asked for prose only. - The edit is made on the branch the platform fixes for this thread (`claude/project-thread-bo65kq`) in this container's own checkout, not in a sibling worktree, as on #29781. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (No schema touched.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` is CI's repo-wide run. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. The change publishes `@objectstack/skills`, so it carries a `patch` changeset; both declaration lines are in the PR body and the changeset. - [x] 6. `packages/spec` not touched. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English (the 维护者速读 block is the repo's own Chinese reading aid). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01J8VVAkWdcYL2ftVcWZ2scY --- _Generated by [Claude Code](https://claude.ai/code/session_01J8VVAkWdcYL2ftVcWZ2scY)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 322b6bf commit cde9a80

2 files changed

Lines changed: 29 additions & 11 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
'@objectstack/skills': patch
3+
---
4+
5+
docs(skills): the automation skill teaches that an approval's decision outputs are enforced at the decision
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
- **Before.** `skills/objectstack-automation/references/state-machines-and-approvals.md` said only that a typed `decisionOutputs` entry renders the matching record picker in the decision dialog (`multiple` collects an id array). An agent writing an approval flow from it could read `type` and `multiple` as widget hints and not know that a value the next step cannot use is refused.
11+
- **Now.** The *Recording a decision* section says what the approvals service enforces, on approve and reject alike and before any write: a picker value must resolve through the `expression` approver's resolvers (`user` by any account's record id, `department` by record id in the request's organization, `team` by record id not owned by another organization, `position` by the machine name of a position someone in that organization holds); `multiple: true` takes a list of distinct values for every kind, bare keys included, and without it one value; any other non-blank value is refused with `400 VALIDATION_FAILED` naming `outputs.KEY`. The Console sends a list for a `multiple: true` text output. The existing `lead_review` node is the example. The file stays within its token ceiling (5724 of 5730).

‎skills/objectstack-automation/references/state-machines-and-approvals.md‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,9 @@ is one diagram a reviewer (or AI) can read end-to-end.
9696
label: 'Sales Manager Review',
9797
config: {
9898
approvers: [{ type: 'position', value: 'sales_manager' }],
99-
behavior: 'first_response', // or 'unanimous' / 'quorum' / 'per_group'
100-
lockRecord: true, // lock the record while pending
101-
approvalStatusField: 'approval_status', // mirror pending|approved|rejected|recalled onto the row
99+
behavior: 'first_response',
100+
lockRecord: true,
101+
approvalStatusField: 'approval_status',
102102
},
103103
},
104104
// Decision routing lives on the OUT-EDGES, not in node config: the engine
@@ -219,6 +219,17 @@ reject) — enforced before any write, with no elevation bypass, so the run
219219
cannot resume past the node with the key a later `expression` approver reads
220220
still missing.
221221

222+
A typed entry renders the matching picker in the decision dialog, and `type`
223+
and `multiple` are also enforced at the decision, on approve and reject alike,
224+
before any write. A picker value must resolve through the `expression`
225+
approver's resolvers: `user` by any account's record id, `department` by record
226+
id in the request's organization, `team` by record id (not another
227+
organization's), `position` by the machine name of a position someone in that
228+
organization holds. `multiple: true` takes a list of distinct values (every
229+
kind, bare keys included); without it, one value. Any other non-blank value is
230+
`400 VALIDATION_FAILED` naming `outputs.KEY`. The Console sends a list for a
231+
`multiple: true` text output. The `lead_review` node below is the example.
232+
222233
### Reading the lock
223234

224235
A page over a record under approval must not offer an edit the server then
@@ -245,7 +256,7 @@ an id that does not exist gets.
245256
| `manager` | The submitter's manager (`sys_user.manager_id`) |
246257
| `field` | User id read from a record field (`value` = field name). Resolved against the record's **live** state at node entry, so a field written mid-flow routes correctly; a multi-select user field fans out into one approver per user |
247258
| `queue` | Members of ownership queue `value` at node entry; an empty queue adds nobody |
248-
| `expression` | A **CEL expression** resolved at node entry (`value` = the expression) — see **Dynamic approvers** below. Only `current.*` / `trigger.*` / `vars.*` roots are available; the optional `resolveAs: 'user'(default) \| 'department' \| 'position' \| 'team'` re-expands each resolved id through the graph |
259+
| `expression` | A **CEL expression** resolved at node entry (`value` = the expression) — see **Dynamic approvers** below. The optional `resolveAs: 'user'(default) \| 'department' \| 'position' \| 'team'` re-expands each resolved id through the graph |
249260

250261
### Dynamic approvers (`type: 'expression'`)
251262

@@ -307,20 +318,16 @@ export const DynamicApprovalFlow = defineFlow({
307318
config: { objectName: 'expense', triggerType: 'record-after-update', condition: "status == 'submitted'" },
308319
},
309320
{
310-
// Node A declares what a decision may hand to the flow. The TYPED
311-
// declaration renders a multi-select sys_user picker in the decision
312-
// dialog; the lead approves with outputs:
321+
// Node A declares what a decision may hand to the flow; the lead
322+
// approves with outputs:
313323
// POST …/approve { outputs: { next_reviewers: ['u2', 'u3'] } }
314-
// `required: true` is enforced by the runtime on APPROVE (never on
315-
// reject) — node B below has nobody to route to without it.
316324
id: 'lead_review', type: 'approval', label: 'Lead Review',
317325
config: {
318326
approvers: [{ type: 'org_membership_level', value: 'owner' }],
319327
decisionOutputs: [{ key: 'next_reviewers', label: 'Next Reviewers', type: 'user', multiple: true, required: true }],
320328
},
321329
},
322330
{
323-
// Node B resolves them at entry from the lead's decision outputs.
324331
id: 'co_sign', type: 'approval', label: 'Co-sign',
325332
config: {
326333
approvers: [{ type: 'expression', value: 'vars.lead_review.next_reviewers' }],
@@ -362,7 +369,7 @@ Object-hook `ctx` is a different vocabulary — see **objectstack-data**
362369
| `approvalStatusField` | Business-object field to mirror `pending`/`approved`/`rejected`/`recalled` onto (should be readonly) |
363370
| `onEmptyApprovers` | What an EMPTY resolved slate does: `admin_rescue` (default — request opens, only a privileged admin can act via Reassign; never waves through, never kills the run), `fail` (node fails — treat an empty slate as a config bug), `auto_approve` (skip the request, continue down `approve` with `output.autoApproved = true` — opt-in because it silently waves the record through), `fallback` (request opens on the sibling `fallbackApprovers` instead — same shape as `approvers`, required by this policy and refused under any other; a fallback that itself resolves to nobody degrades to `admin_rescue`). Declare it explicitly on any node with an `expression` approver (linted) |
364371
| `excludeSubmitter` | `true` drops the run's submitter (`submitter_id`) from the resolved slate, whatever approver type produced them — segregation of duties; a slate it empties takes the `onEmptyApprovers` route, never a silent pass. `onEmptyApprovers: 'auto_approve'` is **refused** beside it (it would approve exactly the request this exists to stop): use `admin_rescue`, `fail` or `fallback`. Default `false` |
365-
| `decisionOutputs` | Decision outputs a decision may carry (author declares, approvers fill values). Entries are bare keys (free-text input) **or typed declarations** `{ key, label?, type: 'text'\|'user'\|'department'\|'position'\|'team', multiple?, required? }` — a typed entry renders the matching record picker in the decision dialog (`multiple` collects an id array). Accepted outputs resume the run as `<nodeId>.<key>` variables; undeclared keys reject the decision; `decision`/`requestId` reserved |
372+
| `decisionOutputs` | Decision outputs a decision may carry: bare keys (free text) **or typed declarations** `{ key, label?, type: 'text'\|'user'\|'department'\|'position'\|'team', multiple?, required? }`, enforced at the decision — see *Recording a decision* |
366373
| `escalation` | Optional per-node SLA — `{ enabled, timeoutHours, action: reassign\|auto_approve\|auto_reject\|notify, escalateTo?, notifySubmitter }`. `timeoutHours` is **calendar (wall-clock) hours** — nights, weekends and holidays count; the platform ships no business-hours calendar. `escalateTo` is a **position machine name** (expanded to its holders via `sys_user_position`, ADR-0090 D3) or a specific user id — never a membership tier. `reassign` without `escalateTo` degrades to notify (linted) |
367374
| `maxRevisions` | Send-backs per run before auto-reject (ADR-0044) — see *Send-back for revision* above |
368375

0 commit comments

Comments
 (0)