Skip to content

Commit 5e58193

Browse files
fix(plugin-approvals): every slot reader takes the caller's acting addresses — can_act, the decision slot test, the already-acted probe and the email-keyed slot (#21410)
Fixes #21379 Clause-②: no ## What changes PR #21378 moved the slot-address equivalence into `packages/plugins/plugin-approvals/src/approver-address.ts` and pointed the "My Pending" filter and the participant gate at it. Four readers still compared a slot with the bare user id. Each now takes the caller's **acting addresses** from that module. That set is the user id, the email the caller's own `sys_user` row carries, and both spellings of each position in the server-resolved `context.positions`. `approver-address.ts` gains three functions, and there is still one fold: - `actingAddresses(caller)` returns the default actor's addresses, in the order a slot is taken: user id, account email, then `position:P` and the deprecated prefix for each held position. - `actorAddresses(actorId, caller)` covers one resolved actor. The caller's own user id expands to `actingAddresses`. A named position address expands to both spellings of that position. Anything else, such as a named email or a machine actor, matches only itself. - `heldSlot(pending, actorId, caller)` is **the** slot test. It returns the first of those addresses that the slate holds. The readers: | reader | before | now | |---|---|---| | `attachViewers`' `viewer.can_act` | `pending.includes(uid)` | `heldSlot(pending, uid, caller)`, the same call the decision methods make with no `actorId` | | decision slot test: `decideNode`, `sendBack`, `reassign`, `requestInfo`, `comment` | `pending.includes(actorId)` | `takenSlot()`, which wraps `heldSlot` | | `visibleRequestIds`, already-acted probe | `actor_id: uid` | `actor_id` in `actingAddresses(caller)` | | `visibleRequestIds`, current approver | user id plus position addresses | `actingAddresses(caller)`, which adds the email half (item 4) | `resolveActor` is unchanged. It still admits exactly the identities it admitted before, and the #21350 oracle pin still holds. **Nobody new may decide.** The holder could always decide by naming `position:P`. Now the default actor and the console's spelling reach the same slot. A user who holds a different position is still refused: 403 at the route, pinned in the unit and dogfood suites. **What `sys_approval_action.actor_id` records.** A slot-gated action records the slot it took, in that slot's stored spelling: `position:P` on a `position:P` slot (whichever spelling the caller used), the deprecated-prefix literal on a 15.x slot, and the email on an email slot. Naming that slot has always recorded exactly this; the measured row on `main` is `approve:position:m21379_reviewer`. It must stay this way because the multi-approver tally in `decideNode` and the `decision_progress` enrichment count approvals by matching `actor_id` against the slate. A decision recorded under any other spelling would leave its slot pending after its holder approved, so a unanimous request would never finalize. The tally pin goes red if this changes. The cost is noted under Acceptance notes. `#21387` (retiring the deprecated prefix) is not addressed here. ## Measured on a booted app (`bootStack`, real `/api/v1/approvals` routes) The fixture opens a request on a position nobody holds, then staffs the holder. Readings were taken with a scratch measurement test, which was not committed. The committed dogfood pin below replaces it. | request | `main` at `ecb6ca025` | this branch | |---|---|---| | holder: `viewer.can_act` | `false` | `true` | | holder: approve, no `actorId` | 403 `FORBIDDEN` | 200 `approved` | | holder: approve, `actorId` with the deprecated prefix | 403 `FORBIDDEN` | 200 (dogfood pin, on its own request) | | holder: approve, `actorId: position:P` | 200 | 200 (unit pin) | | holder: `GET /requests/:id` after deciding | 404 | 200 | | bystander (holds another position): approve, no `actorId` | 403 | 403 | **Item 4, the email-keyed slot: it reproduced and is fixed here.** A `user` approver authored as an email stores the email as its slot. The table below is for the reviewer who owns that email and is not the submitter. | request | `main` at `ecb6ca025` | this branch | |---|---|---| | "My Pending" (`approverId` = id, email) | `[]` | the request; `can_act: true` | | `GET /requests/:id` | 404 | 200 | | approve, no `actorId` | 403 | 200 | | approve, `actorId` = the email | 200 | (decided above) | | `GET /requests/:id` after deciding | 404 | 200 | ## Pins - `approval-service.test.ts`, describe `every slot reader takes the acting addresses (#21379)`, has one pin per reader: - `can_act` as a table over holder, bystander, submitter and admin. It asserts that `can_act` equals "admitted as a slot holder" with no `actorId`, and that `can_override` covers the admin. - The decision slot test under the default actor, the deprecated prefix and `position:P`, each on its own request, with the recorded spelling. A 15.x slot keeps its own spelling. The bystander is refused under the default actor and under a named position. - The four sibling methods: holder admitted, bystander refused. - The unanimous tally consumes exactly the position slot. - The already-acted probe: the holder keeps sight of the request, the bystander does not, and a successor holder of the position does. - The email-keyed slot: listed, counted, `can_act`, decided by the default actor, and still in sight afterwards. Another account's email is a negative control. - The user-id slot is taken before a position slot. - `approver-address.test.ts` pins the three new pure functions, with negative controls. - `approver-address-readers.test.ts` is the **enumeration pin**: 1. `READERS` lists every reader of the equivalence: acting path, list filter, participant gate, `can_act`, `takenSlot`, and the five decision methods. Each listed method must read its `approver-address.ts` export, which the pin checks through the TypeScript AST. It also checks that both participant-gate probes read the one `acting` set. 2. The pin parses every non-test `.ts` file under `src/` except `approver-address.ts`. It collects every site shaped like a slot comparison: - a membership call (`includes`, `has`, `some`, and so on) on a receiver whose text names a pending slate; - a slot column (`approver`, `actor_id`, `pending_approvers`) inside a `where` or `filter` predicate, or assigned to `where.COLUMN`; - a declarative `field: 'pending_approvers'` row under a `filter`. Every site must be classified in `SLOT_SITES` by file, enclosing function and exact text. Today there are 3 `reader` sites, 6 `slot-vs-slot` sites, such as a hand-off target or a token's bound slot, and 1 `declarative` site (see Acceptance notes). A new site fails with its location. So does a changed or vanished site. A planted-source case shows that each of the four shapes is detected. What the pin does not see: a comparison written in none of those shapes, such as a hand-written loop using `===`. ## Ablations Each ablation is committed first and runs through `node scripts/ablation-replace.mjs` in WRAP mode. A belt trap restores from `git checkout HEAD` on an absolute path. Each mutation was checked on disk: anchor count went from 1 to 0, replacement count from 0 to 1, and the blob changed. Each restore was proven by blob == HEAD and an empty `git diff HEAD`. The unit pins import `src` directly, and dogfood's `isolated` project aliases `@objectstack/plugin-approvals` to `src/index.ts`, so no `dist` leg applies. Final run at `dabba36f3`: | ablation | unit (3 files, 338 tests) | dogfood (2 files) | |---|---|---| | A1: `can_act` back to `pending.includes(caller.userId)` | 4 red: table, email, both enumeration tests | red: holder `can_act` expected true | | A2: `takenSlot` back to the literal actor | 8 red | red: holder approve 403 | | A3: already-acted probe back to `actor_id: uid` | 3 red | red: holder detail 404 | | A4: current-approver probe back to the user id alone | 5 red, including both #21350 pins | both red | | A5: default actor widened to a non-held position (`heldSlot` takes any `position:` slot) | 8 red: bystander rows, the bystander refusals, and 4 existing #3424 override pins | red: submitter `can_act` became true | The first A5 attempt was refused by the tool: the replacement contained the anchor text, so the anchor count did not drop. No test ran on that attempt. It was redone with a replacement that does not contain the anchor. Every direction was red, as predicted. ## Gates and tests, at `dabba36f3` - `pnpm --filter @objectstack/plugin-approvals test`: 56 files, 855 tests, all passed. - `typecheck` for plugin-approvals and dogfood: exit 0. `check:test-typecheck` holds 8 files and 324 errors in the ledger, with no new debt. All three new or edited test files are in `tsconfig.test.json`'s program (checked with `--listFilesOnly`). - Dogfood (`--project isolated`): the new `position-address-readers` pin, the #21350 pin, `approval-override-composite-pin` and `approval-snapshot-masked-field` ran: 4 files, all passed. - `dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` derived 95 commands. All 95 exited 0 at `dabba36f3`, and `--ran` reconciled them as `95 derived, 95 run, 0 NOT-MEASURED, 0 UNRUN`. Three earlier readings were superseded: - `check-system-context-census` was red on the first pass, because a new `context.isSystem` read in `actingCaller` had no row. Commit `dabba36f3` removes that read, and the census holds at 114 sites. - `check:skill-examples` and `check:dual-build-cjs-loads` answered PREREQUISITE NOT MET until the eight packages without `dist/` were built. - `check:dual-build-cjs-loads` then went red once on `@objectstack/mcp`'s `index.d.cts`. That file is outside this diff, and its timestamp shows it was being written during the run. The rerun was green. - `eslint --no-inline-config --format json` on the 7 touched `.ts` files: 7 files, 0 errors, 0 warnings. `eslint.config.mjs` enables no type-aware linting (`--print-config` shows no `parserOptions.project` or `projectService`), so no untouched file's result can move. The repo-wide `pnpm lint` is left to CI. - `origin/main` was merged in as `8312775bd`. The two incoming commits do not touch these files. ## Docs `content/docs/automation/approvals.mdx`: corrected the sentences this change made false. - the inbox and participant counts now include the account email; - "already acted" is counted by the same identities; - the decision paragraph now describes the default actor's slot and the recorded spelling; - the `can_act` bullet; - the admin-override callout's "no concrete user can act", which no longer holds for a staffed `position:P` slot. "A position-addressed approver is never wrongly hidden" is now true and stays. The deprecated prefix is described in words, and `check:role-word` holds. ## Acceptance notes - **Admin who holds the routed position:** this admin now decides as a slot holder (`via_override: false`, one vote in a multi-approver tally), exactly as when they named the slot. An admin who holds no slot is unchanged. The changeset says this. - **Already acted belongs to the position:** a decision recorded under `position:P` stays visible to every current holder of P. A former holder who decided it stops seeing it once they leave P. Pinned (successor) and documented. - **Attribution:** `sys_approval_action.actor_id` is a `sys_user` lookup, but for a position or email slot it holds the slot literal. The person who decided is not on the row. This was already true when the slot was named; the default actor now reaches it too. Reported to the seat. - **Email casing:** the default actor matches the account's email exactly as stored. `resolveActor` admits a named email case-insensitively. So a slot authored in a casing that differs from the account's stored email can be decided only by naming that exact spelling, as before, and the participant gate does not count it. NOT MEASURED through a door. - **The `my_pending` list view** in `sys-approval-request.object.ts` filters `pending_approvers contains {current_user_id}`. It is the one declarative slot-against-caller comparison, and metadata cannot read `approver-address.ts`. It is served only by the generic data door. On this fixture that door answers 403 `PERMISSION_DENIED` to a member without read on `sys_approval_request`. The admin read returned the email-slot row. It is classified `declarative` in the enumeration pin, so a second such filter goes red. - **The spec contract's `can_act` TSDoc** (`packages/spec/src/contracts/approval-service.ts`) still says "their user id is in the request's resolved `pending_approvers`". Its main sentence, "mirrors the exact check the service uses to authorize a decision", is now true. The parenthetical is narrower than the behavior. `packages/spec` was not edited under this order. --- _Generated by [Claude Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 41a3c8d commit 5e58193

9 files changed

Lines changed: 1098 additions & 86 deletions

File tree

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
'@objectstack/plugin-approvals': patch
3+
---
4+
5+
A holder of a position whose approval slot reads `position:<name>` now decides it from the stock console, sees `can_act` on it, and keeps sight of it after deciding; a reviewer named by a `user` approver authored as an email does too
6+
7+
Clause-②: no
8+
9+
A request whose approver position nobody held when it opened keeps the literal `position:<name>` slot. After the position is staffed, its holder found the request in "My Pending", but `viewer.can_act` was `false`, an approve with no `actorId` (what the console's approve action sends) or with `role:<name>` (the deprecated pre-rename spelling the console uses) answered 403, only naming `position:<name>` decided it, and `GET /api/v1/approvals/requests/:id` then answered 404 to the holder who had just decided it. A `user` approver authored as an email had the same shape: its reviewer saw neither the request nor `can_act`, and only naming the email decided it.
10+
11+
Every place the approvals service compares a slot with the caller now reads the caller's acting addresses, the set its decision routes already admitted: the user id, the email the caller's own account carries, and both spellings of each position on the caller's server-resolved context.
12+
13+
- **Decisions** (approve, reject, send back, reassign, request info, comment): with no `actorId`, the caller takes the first pending slot keyed by one of those addresses, their user id first. A named `role:<name>` or `position:<name>` takes that position's slot under either spelling. Nobody new may decide: a user who holds another position is still refused with 403.
14+
- **What is recorded:** `sys_approval_action.actor_id` holds the slot the action took, in that slot's stored spelling. That is what naming the slot always recorded, and the multi-approver tally counts approvals by matching it against the slate.
15+
- **`viewer.can_act`** is computed by the same slot test the decision routes run with no `actorId`, so it is `true` exactly when such an approve would be admitted as a slot holder.
16+
- **Visibility:** the participant gate counts a current approver by the email half too. "Already acted" is counted by the same addresses, so a request decided under `position:<name>` stays visible to whoever holds that position.
17+
18+
An admin who holds the routed position now decides it as a slot holder (`via_override: false`, one vote in a multi-approver tally), exactly as when they named the slot; an admin who holds no slot is unchanged.

‎content/docs/automation/approvals.mdx‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,8 @@ and nothing can move it, so the run parks forever.
387387
The approver's surface is the **Approvals Inbox** — in the stock console, the
388388
**Approvals** entry in the account menu. It lists what is waiting on the current
389389
user (`status = pending`, `pending_approvers` containing them — their user id,
390-
or the `position:<name>` literal of a position they hold) and it is the only
390+
the email their own account carries, or the `position:<name>` literal of a
391+
position they hold) and it is the only
391392
surface that carries the **decision actions**: approve, reject, return, reassign,
392393
request info, delegate, remind, override.
393394

@@ -426,9 +427,13 @@ Other filters: `status`, `object`, `recordId`, `submitterId`, `q`, `limit`,
426427
<Callout type="warn">
427428
**`approverId` is a filter, not authorization.** What you may see is decided
428429
separately: a request is visible to its **participants** — the submitter, a
429-
current approver (counting anyone who holds the position a `position:<name>`
430-
slot names, since the decision routes admit them under it), and anyone who has
431-
already acted on it (a past approver whose slot has moved on, a commenter). So omitting `approverId` returns *your*
430+
current approver (counted by every identity the decision routes let the caller
431+
take a slot under: their user id, the email their own account carries, and
432+
either spelling of a position they hold), and anyone who has already acted on
433+
it (a past approver whose slot has moved on, a commenter). An action is recorded
434+
under the slot it took, so "already acted" is counted by those same identities:
435+
a decision taken on a `position:<name>` slot keeps the request visible to
436+
whoever holds that position. So omitting `approverId` returns *your*
432437
requests, not every request in the tenant. Admins with override authority
433438
(`admin_full_access`, or `organization_admin` within their org) see all of them
434439
— that is what the "all requests" view is for.
@@ -496,9 +501,17 @@ curl -b cookies.txt -X POST \
496501
# POST .../reject for the other direction; body: { actorId?, comment? }
497502
```
498503

499-
`actorId` defaults to the caller. The actor **must** be in `pending_approvers`
500-
or the call returns 403 (`FORBIDDEN: actor '…' is not a pending approver`); a
501-
request that isn't pending returns 409 (`INVALID_STATE`). Always go through
504+
`actorId` defaults to the caller, who takes the first slot in
505+
`pending_approvers` keyed by one of their identities — their user id, the email
506+
their own account carries, or either spelling of a position they hold (a
507+
`position:<name>` literal left by a position nobody held when the request
508+
opened, once someone is staffed into it). A named `actorId` must be one of those
509+
identities, and a position named under either spelling takes that position's
510+
slot. No such slot and no admin override returns 403 (`FORBIDDEN: actor '…' is
511+
not a pending approver`); a request that isn't pending returns 409
512+
(`INVALID_STATE`). The decision is recorded in `sys_approval_action.actor_id`
513+
under the slot it took, in that slot's stored spelling — the multi-approver
514+
tally counts approvals by matching that value against the slate. Always go through
502515
these endpoints — never resume the flow run directly, and since #3801 you
503516
**cannot**: `POST /api/v1/automation/{flow}/runs/{runId}/resume` answers 403 for
504517
a run parked on an `approval` node (including via a `subflow` pause) and changes
@@ -606,10 +619,12 @@ service attaches to every request it returns:
606619
"viewer": { "can_act": true, "is_submitter": false, "can_override": false }
607620
```
608621

609-
- `can_act` — the caller is a **current pending approver** (their id is in the
610-
resolved `pending_approvers` while the request is `pending`). This is the same
611-
check the decision routes authorize with, so it already reflects
612-
position/team/manager resolution.
622+
- `can_act` — the caller is a **current pending approver**: while the request is
623+
`pending`, the caller with no `actorId` named would take one of its slots —
624+
under their user id, the email their own account carries, or either spelling
625+
of a position they hold. It is computed by the same function the decision
626+
routes authorize with, so it reflects position/team/manager resolution and a
627+
`position:<name>` slot whose position was staffed after the request opened.
613628
- `is_submitter` — the caller submitted the request.
614629
- `can_override` — the caller is a **platform or tenant admin** who may act on a
615630
`pending` request despite holding no slot (see the admin-override callout
@@ -625,9 +640,11 @@ predicate only trims the UI.
625640

626641
<Callout type="info">
627642
**Admin override — recovering a stuck request.** An approval routed to a
628-
`position` / `team` / `department` with **no holders** resolves to only an
629-
unresolvable `position:<name>` literal in `pending_approvers`: no concrete user
630-
can act, and (with `lockRecord`) the record stays locked. A **platform admin**
643+
`position` / `team` / `department` with **no holders** resolves to only its
644+
`<type>:<name>` literal in `pending_approvers`. A `position:<name>` slot is
645+
decided by whoever holds that position once someone is staffed into it; until
646+
then — and for a `team` or `department` literal, always — no concrete user can
647+
act, and (with `lockRecord`) the record stays locked. A **platform admin**
631648
(`admin_full_access`) or **tenant admin** (`organization_admin`, org-scoped) may
632649
act on any `pending` request — **approve, reject, reassign** it to a real
633650
approver, or **recall** it — releasing the lock. An admin decision is

0 commit comments

Comments
 (0)