diff --git a/scripts/check-skills-token-ratchet.mjs b/scripts/check-skills-token-ratchet.mjs index 4ff52d219eb..c3fea520d79 100644 --- a/scripts/check-skills-token-ratchet.mjs +++ b/scripts/check-skills-token-ratchet.mjs @@ -381,7 +381,17 @@ export const CEILINGS = new Map([ // row this PR itself lowered by 7,006, and the entry stays 448 lines). // 5785 -> 5780: A2′ — the release-stamp line left the count (see // STAMP_LINE_REPIN); the file's count fell by the same 5, headroom unchanged. - ['skills/objectstack-automation/SKILL.md', 5780], + // 5780 -> 6004 (card #26298, audit #26184 rows A1/A3/A4): three teachings an + // author had no row for — the fourth `notify` recipient form `{ emailField }` + // (an envelope over an address no user holds now fails the node), flow-level + // `requiredPermissions` (who may START a flow), and the screen field typed + // `file` (the run resumes with the `sys_file` id). Paid down first: the CRM + // Blueprint's approval row restated the approvals reference's config table + // and branching section and was cut to a pointer (~17 tokens). Landed 5771 -> + // 6004: +233 net, 9 absorbed by headroom, ceiling +224. Maintainer ruling + // 2026-10-27 on the skills-lane decision card 「skills 修复卡的教学和 eval 都撞 + // token 天花板,怎么付?」, verbatim and untranslated: 「A:先削重复,余额按实测提高」. + ['skills/objectstack-automation/SKILL.md', 6004], // 13783 -> 13892 (2026-08-31 app-repo-principles raise, see the block above). // The invariant-vs-transition-gate choice, placed in the field-conditional-rule // bullet list where the tool is actually picked. Both wrong picks are silent: @@ -676,7 +686,13 @@ export const CEILINGS = new Map([ // Independently reproducible without running this script: // ceil($(wc -c < skills/objectstack-api/evals/endpoints-auth-routes.json) / 4) == ceil(4936 / 4) ['skills/objectstack-api/evals/endpoints-auth-routes.json', 1234], - ['skills/objectstack-automation/evals/flows-triggers-approvals.json', 1255], + // 1255 -> 1823 (card #26298): three eval prompts, one per new teaching — the + // applicant email over `{ emailField }`, `requiredPermissions` + `excludeSubmitter` + // on one approval, the screen `file` field written by a later `create_record`. + // +568 tokens, nothing in the file to delete, zero headroom, ceiling +568. + // Same maintainer ruling as the SKILL.md row above (2026-10-27, verbatim and + // untranslated): 「A:先削重复,余额按实测提高」. + ['skills/objectstack-automation/evals/flows-triggers-approvals.json', 1823], // 894 -> 1095 (#26179): one regression case for the field permission `when` // (the contract prompt: lock the legal terms for finance from approval on, // legal keeps the edit) — expected: `when` on the finance set's entries with @@ -706,7 +722,14 @@ export const CEILINGS = new Map([ // because a split that arrives with budget is a raise wearing a new path. // The package deltas are the pointer lines and the four new file titles only. ['skills/objectstack-automation/references/examples-flows.md', 1436], - ['skills/objectstack-automation/references/state-machines-and-approvals.md', 5569], + // 5569 -> 5587 (card #26298, audit #26184 row A2): the approval config table + // gains `excludeSubmitter` (refused beside `onEmptyApprovers: 'auto_approve'`), + // +105 tokens. Paid down first: Best Practices 1 and 4 restated the Branching + // section's "Conditional step" and "Roll back on reject" / "Hard reject" + // bullets and were deleted (~73 tokens). Landed 5555 -> 5587: +32 net, 14 + // absorbed by headroom, ceiling +18. Same maintainer ruling as the SKILL.md + // row above (2026-10-27, verbatim and untranslated): 「A:先削重复,余额按实测提高」. + ['skills/objectstack-automation/references/state-machines-and-approvals.md', 5587], ['skills/objectstack-data/references/examples-objects.md', 1779], ['skills/objectstack-data/references/lint-rules.md', 970], ['skills/objectstack-data/references/seeds.md', 1299], diff --git a/skills/objectstack-automation/SKILL.md b/skills/objectstack-automation/SKILL.md index be3f4fa3e60..4e8d5bcc7b8 100644 --- a/skills/objectstack-automation/SKILL.md +++ b/skills/objectstack-automation/SKILL.md @@ -88,7 +88,7 @@ Flows are built from **20 built-in node types** (plugins register more via | `notify` | Send a notification through the messaging service (inbox channel by default) | | `connector_action` | Invoke a pre-built integration connector | | `script` | Call a **registered** function named by `config.function` (see *Valid-but-silently-wrong* #3) | -| `screen` | Display a UI form to the user (screen flows only) | +| `screen` | Display a UI form to the user (screen flows only). `config.fields[]`: `{ name, label, type, required, options, defaultValue }`, `type` an open widget hint; `'file'` renders the upload widget and resumes with the `sys_file` id bound to `name` — a later `create_record` / `update_record` writes it into a `file` field, which claims the file: collect first, write after the last screen | #### Human Decision @@ -104,7 +104,9 @@ undeclared key is a named parse error. **RAW** keys are read as written: a ```ts { id: 'tell_owner', type: 'notify', label: 'Notify Owner', config: { - recipients: { dialect: 'cel', source: 'record.assignee' }, // REQUIRED — id, list, or envelope; a CSV is ONE id + recipients: { dialect: 'cel', source: 'record.assignee' }, // REQUIRED — id, list, or envelope; a CSV is ONE id. + // 4th form `{ emailField: 'applicant_email' }`: an `email` field of the trigger record, for a non-user — + // then `channels: 'email'` alone. An envelope must yield USER ids: an address no user holds fails the node title: 'Done: {{ record.title }}', // inline; XOR `template` (RAW, localizable) message: 'Closed {{ record.closed_at | date:long }}', // body; only with inline `title` topic: 'task', // RAW; default 'notify' @@ -182,6 +184,13 @@ variables: [ > organization predicate reads and writes across every tenant. The tenant column > is platform-injected — filter on it, never re-declare it per object. +> **Who may START a flow: `requiredPermissions: ['…']` on the flow** (ADR-0066 +> D4) — every door that starts it by name (trigger route, action of type `flow`, +> declared endpoint) refuses a non-system caller missing any named capability +> with `403 PERMISSION_DENIED`; an action starting it inherits the list, a +> `subflow` node is not a door. Gate an elevated `screen` / `api` flow here, never +> on `current_user.positions` inside it. + > **A hook elevates itself with `runAs`, never with `sudo`.** An object hook > (objectstack-data) declares its own `runAs: 'system' | 'user' | 'inherit'` — > default `'inherit'`, the context of the write that fired it — scoping that @@ -399,7 +408,7 @@ in this CRM-style structure: | Automation Type | Typical Location | Pattern | |:--|:--|:--| | Screen flow | `src/flows/*.flow.ts` | Use explicit `variables`, node graph (`nodes` + `edges`), and decision branches | -| Approval flow | `src/flows/*.flow.ts` | A flow with `approval` node(s); set `approvers` / `behavior` / `lockRecord` / `approvalStatusField` in node `config`, branch on `approve` / `reject` edges | +| Approval flow | `src/flows/*.flow.ts` | A flow with `approval` node(s) — config and branching in the approvals reference | | Flow registry | `src/flows/index.ts` | Export `allFlows: Flow[]` and register centrally in `defineStack({ flows })` | --- diff --git a/skills/objectstack-automation/evals/flows-triggers-approvals.json b/skills/objectstack-automation/evals/flows-triggers-approvals.json index d83155aadad..8eda3494f14 100644 --- a/skills/objectstack-automation/evals/flows-triggers-approvals.json +++ b/skills/objectstack-automation/evals/flows-triggers-approvals.json @@ -50,6 +50,36 @@ "must_contain": ["function:", "outputVariable", "type: 'fault'", "timeoutMs", "update_record", "dialect: 'cel'"], "must_not_contain": ["label: 'error'", "query_record", "{recalc"] } + }, + { + "id": 6, + "prompt": "When a `job_application` is approved, email the applicant. Applicants are not users; the address is in the record's `applicant_email` field.", + "expected_output": "`notify` with `recipients: { emailField: 'applicant_email' }` (the fourth recipient form: an `email`-typed field of the trigger record) and `channels: 'email'` alone — never a CEL envelope over `record.applicant_email`, which must yield user ids and fails the node for an address no user holds.", + "files": [], + "assertions": { + "must_contain": ["notify", "emailField", "applicant_email", "'email'"], + "must_not_contain": ["source: 'record.applicant_email'", "channels: ['inbox']"] + } + }, + { + "id": 7, + "prompt": "An expense approval only holders of the `expense.approve_request` capability may start; route to the `finance_manager` position, and the submitter must never approve their own expense.", + "expected_output": "Flow-level `requiredPermissions: ['expense.approve_request']` (checked at every door that starts the flow, not a `current_user.positions` check inside it); the `approval` node sets `excludeSubmitter: true`, and its `onEmptyApprovers` is anything but `auto_approve`, which is refused beside it.", + "files": [], + "assertions": { + "must_contain": ["requiredPermissions", "excludeSubmitter: true", "type: 'position'"], + "must_not_contain": ["onEmptyApprovers: 'auto_approve'", "current_user.positions"] + } + }, + { + "id": 8, + "prompt": "A screen flow: the user uploads the signed contract PDF on the last step, then the flow creates the `contract` record with the file in its `signed_copy` field.", + "expected_output": "A `screen` field `{ name: 'signed_copy', type: 'file', required: true }`; the screen resumes with the uploaded `sys_file` id bound to `signed_copy`, and a `create_record` after the last screen writes `fields: { signed_copy: { dialect: 'cel', source: 'signed_copy' } }`, which claims the file for the record.", + "files": [], + "assertions": { + "must_contain": ["type: 'screen'", "type: 'file'", "create_record", "signed_copy"], + "must_not_contain": ["type: 'upload'", "type: 'attachment'"] + } } ] } diff --git a/skills/objectstack-automation/references/state-machines-and-approvals.md b/skills/objectstack-automation/references/state-machines-and-approvals.md index f592cf93766..d9e6b207818 100644 --- a/skills/objectstack-automation/references/state-machines-and-approvals.md +++ b/skills/objectstack-automation/references/state-machines-and-approvals.md @@ -347,6 +347,7 @@ Object-hook `ctx` is a different vocabulary — see **objectstack-data** | `lockRecord` | Lock the triggering record from edits while pending. Default `true` | | `approvalStatusField` | Business-object field to mirror `pending`/`approved`/`rejected`/`recalled` onto (should be readonly) | | `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) | +| `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` | | `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 `.` variables; undeclared keys reject the decision; `decision`/`requestId` reserved | | `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) | | `maxRevisions` | ADR-0044 — max **send-backs-for-revision** per run before auto-reject. Default `3`; `0` disables send-back. Only meaningful when the node has a `revise` out-edge | @@ -369,15 +370,11 @@ These are wired on the **graph**, not in node config: ### Approval Best Practices -1. **Gate entry on the edge** (`condition` into the Approval node) so the flow - only pauses for records that actually need sign-off. -2. **Set `approvalStatusField`** to mirror status onto the row — views and +1. **Set `approvalStatusField`** to mirror status onto the row — views and formulas can then filter on it without joining `sys_approval_request`. -3. **Keep `lockRecord: true`** unless you have a strong reason to allow +2. **Keep `lockRecord: true`** unless you have a strong reason to allow edits while pending — otherwise approvers chase a moving target. -4. **Model rejection as a visible branch** — a back-edge to revise, or an `end` - node to terminate. The path is on the diagram, not hidden in config. -5. **Notify the submitter from downstream nodes** wired to the `approve` / +3. **Notify the submitter from downstream nodes** wired to the `approve` / `reject` edges. Opening already tells each resolved approver (`approval.requested`); ⛔ no `notify` node for the opening: it tells them twice.