Repository navigation
Commit 888d686
feat(plugin-approvals): excludeSubmitter keeps the submitter from approving their own request (#25722)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"}
-->
_Requested by **Leehom** · [project
thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X4Svj29jUyus5ER1MGdNio8)_
Fixes #25573
Clause-②: no
Metadata-protocol: none
## What this does
An approval node's `excludeSubmitter: true` now keeps the request's
submitter from approving their own request. The key was declared by
#25609 (PR #25632), and its pairing with `onEmptyApprovers:
'auto_approve'` is refused at the parse (#25635 ruled A, PR #25700).
Until this PR the approval engine parsed the key and read nothing.
Rulings carried, not re-decided:
- 6377888641 (batch #9 item 1, letter B): "step option
`excludeSubmitter`, default off; empty set escalates per ADR-0042 or
reroutes to the fallback".
- Triage 6390007010: "remove the submitter from the resolved slate. An
emptied slate takes the node's `onEmptyApprovers` fallback, else
ADR-0042 escalation, never a silent pass. The approval record notes when
the exclusion changed the route." No runtime override of `auto_approve`
(ruling A on #25635).
## Where the exclusion lives (measured)
- **One seam covers every approver type.** `user`, `field`, `manager`,
`position`, `team`, `department`, `org_membership_level` and
`expression` all resolve through `ApprovalService.expandApprovers`,
called from `openNodeRequest`. The exclusion runs on its RESOLVED
output, so it needs no per-type code and also catches an out-of-office
hop that lands on the submitter. The removed slots are the addresses the
submitter acts under without naming one (`actingAddresses` from
`approver-address.ts`: the user id and the account's email). A
`position:P` literal is never removed, because other holders take it.
- **Four writers put people on a slate, so four places read the
option.** The open (`openNodeRequest`), the reassign door (`reassign`),
the SLA sweep (`escalateRequest`, action `reassign`), and the decision
(`decideNode`), which is the one check every approval passes through.
The mid-tally slate update in `decideNode` only shrinks the open-time
slate, so it needs nothing.
## Choices made inside the ruling's words
The order left two questions to this card. Both are answered by "never
approved by the submitter" and "never a silent pass":
1. **The `fallbackApprovers` slate and the `admin_rescue` administrators
are screened too.** Measured before this change: both slates go through
the same `expandApprovers` with no submitter screen, so a submitter on
the fallback list, or a submitter who is an organization administrator,
would get their own request back one route later. Screening them is the
only reading under which "never approved by the submitter" holds. A
fallback slate that names only the submitter degrades to `admin_rescue`,
exactly as a fallback that resolves to no one already does. An
organization whose only administrator is the submitter opens on the
slate that names no one (the existing break-glass outcome), never on the
submitter.
2. **Delegation, reassign and escalation hold the exclusion too.**
Out-of-office delegation is applied inside `expandApprovers`, so the
open-time screen covers it. After the open: `reassign` refuses a target
that is the submitter (id or email) or a position only the submitter
holds, with `VALIDATION_FAILED`, asked after the authorization branches
like the #25485 target check. An SLA `reassign` escalation leaves the
submitter off the new slate; when the submitter is its only target, it
is not applied and the pending approvers are told the SLA lapsed, the
shape #25598 gives an unresolvable target. `decideNode` refuses an
approval decided by the submitter with `FORBIDDEN`, the privileged
override included. A rejection by the submitter stands, because it
approves nothing.
## The route note
When the exclusion took the submitter off a slate, the request writes
one system note on its timeline (`action: 'comment'`, no actor), stored
as data in the node-config snapshot under `__submitterExclusionNote`
with the route the request took: `remaining_approvers`, `fallback`,
`administrators` or `no_one`. It reuses the #25576 shape: the row's
`comment` is the English, and the approvals door renders the reader's
language from new `approvalSubmitterExcluded*` catalog sentences in en,
zh-CN, ja-JP and es-ES. When the exclusion emptied the slate and the
request went to the administrators, the administrators' routing note now
says "the only approver it resolved to was the request's submitter, whom
excludeSubmitter removes" instead of "no approver on the node resolved
to a person", which would have been false. A node with the option on
whose slate never named the submitter writes no note.
## Declared surface widening: `packages/spec` (seat-approved)
The claim's file surface was `packages/plugins/plugin-approvals/src/**`.
#25609 handed this card an edit in `packages/spec`: the
`excludeSubmitter` describe opened with `[PLANNED — not yet enforced]`
and ended "The approval engine does not read this key yet, so today the
submitter is NOT removed", and both the TSDoc and `liveness/flow.json`'s
`_note` said "When #25573 lands, that marker and that last sentence
leave and this sentence is rewritten." Leaving them would make shipped
text false. The server seat approved the edit within this fence: no
schema shape, default or refusal change. Spec files touched:
- `packages/spec/src/automation/approval.zod.ts`: marker and last
sentence dropped from the describe; the TSDoc's PLANNED paragraph
rewritten to name the consumer.
- `packages/spec/liveness/flow.json`: the `_note` PLANNED RECORD
sentence rewritten as a LIVE RECORD naming the consumer seams.
-
`packages/spec/src/automation/approval-exclude-submitter-25609.test.ts`:
the §5 status pin flipped from "opens with the planned marker" to
"carries no planned marker".
- `content/docs/references/automation/approval.mdx`: regenerated by
`check:generated --fix` (the one artifact it proved stale).
## Tests
- New
`packages/plugins/plugin-approvals/src/exclude-submitter.integration.test.ts`:
14 cases on a real `ObjectQL` over `SqlDriver` (better-sqlite3), with
real administrator resolution. Covers a `field` step and a `position`
step resolving to the submitter (administrators, fallback, `fail`), a
shared slate minus the submitter (per_group tags), the email slot and an
out-of-office hop onto the submitter, a fallback naming only the
submitter, a submitter who is the organization's only administrator (and
whose override is refused), the zh-CN render, the decision guard, the
reassign door, the SLA escalation, and three controls with the option
off or never triggered.
- New `src/submitter-exclusion-note.test.ts`: 3 cases (every route in
every shipped locale, the strict shape read, the administrators' note
clause).
- `src/approver-address-readers.test.ts`: the `PERSON_SITES` ledger key
follows the rename `localizeAdminRescueNote` → `localizeRoutingNotes`
(same two `a.actor_id` presence tests).
Ablations (WRAP mode of `scripts/ablation-replace.mjs`, each restored
and proven `blob == HEAD`, `git diff HEAD` empty; tests import `src/`,
so no dist rebuild is involved):
- A1, open-time filter removed: 8 failed / 6 passed.
- A2, decision guard disabled: 2 failed / 12 passed. The first attempt
was a no-op (the replacement contained the anchor, the tool refused it)
and was re-run with a non-overlapping replacement.
- A3, reassign refusal deleted: 1 failed / 13 passed.
- A4, escalation filter removed: 1 failed / 13 passed.
- A5, route note disabled: 6 failed / 8 passed.
## Gates (local, on head `0aed0b90`)
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`: 119 commands derived; all 119 run, each
exit code captured before any pipe, all 0 on the final head. `--ran`
reconciliation: "119 derived, 119 run, 0 NOT-MEASURED, 0 UNRUN".
- Reds found and fixed on the way: `check-tenant-audit-census` (and its
`--self-test`, which reads the tree) counted the route note's new
`sys_approval_action` insert, 238 to 239 write call sites. Regenerated
with `node scripts/tenant-audit-census.mjs --write` and the hand-written
prose counts on `content/docs/permissions/tenant-audit-census.mdx`
updated to match; both exit 0.
- Prerequisite refusals cleared, not skipped:
`check-plugin-teardown-shape --self-test` (exit 3 on this shallow clone
until its pinned fixture commit was fetched), `check:skill-examples`
(needed `@objectstack/client-react` built), `check:i18n` (needed its
extract closure built), `check:dual-build-cjs-loads` (needed built
output). Each re-run green.
- `@objectstack/plugin-approvals`: `pnpm test` 98 files / 1276 tests
passed; `pnpm typecheck` exit 0 (includes `check:test-typecheck`).
`@objectstack/spec`: `approval-exclude-submitter-25609.test.ts` 28/28;
`check:generated` green after `--fix` regenerated the one stale
artifact.
- Scoped lint: `eslint --no-inline-config` over
`packages/plugins/plugin-approvals/src` and the two touched spec files,
exit 0.
- CI owns the full farm (`pnpm lint`, `Test Core`, `Build Core`, the
type-check lanes).
## Acceptance notes
- `onEmptyApprovers: 'auto_approve'` beside `excludeSubmitter: true` is
not overridden at runtime (ruling A on #25635). The approval node's
`ApprovalNodeConfigSchema.safeParse` refuses the pair before
`openNodeRequest`, so only a caller that bypasses the parse could reach
it.
- An SLA escalation with `action: 'auto_approve'` still approves an
overdue request under the system context. That is the author's ADR-0042
timeout policy, not the submitter deciding, so it is left as it is.
- A `position:P` literal slot (a position nobody held at open time) is
not removed at open. If the submitter is later staffed into it, the
decision guard refuses their approval.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01LXX9EBHog6ZMo6apV2jvXX)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent e5cf9f0 commit 888d686
14 files changed
Lines changed: 836 additions & 80 deletions
File tree
- .changeset
- content/docs
- permissions
- references/automation
- docs/audits
- packages
- plugins/plugin-approvals/src
- translations
- spec
- liveness
- src/automation
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
132 | 132 | | |
133 | 133 | | |
134 | 134 | | |
135 | | - | |
| 135 | + | |
136 | 136 | | |
137 | 137 | | |
138 | 138 | | |
| |||
197 | 197 | | |
198 | 198 | | |
199 | 199 | | |
200 | | - | |
| 200 | + | |
201 | 201 | | |
202 | | - | |
203 | | - | |
| 202 | + | |
| 203 | + | |
204 | 204 | | |
205 | 205 | | |
206 | 206 | | |
| |||
217 | 217 | | |
218 | 218 | | |
219 | 219 | | |
220 | | - | |
| 220 | + | |
221 | 221 | | |
222 | 222 | | |
223 | 223 | | |
224 | | - | |
| 224 | + | |
225 | 225 | | |
226 | 226 | | |
227 | 227 | | |
| |||
233 | 233 | | |
234 | 234 | | |
235 | 235 | | |
236 | | - | |
237 | | - | |
| 236 | + | |
| 237 | + | |
238 | 238 | | |
239 | | - | |
| 239 | + | |
240 | 240 | | |
241 | | - | |
| 241 | + | |
242 | 242 | | |
243 | 243 | | |
244 | 244 | | |
245 | 245 | | |
246 | | - | |
| 246 | + | |
247 | 247 | | |
248 | 248 | | |
249 | 249 | | |
250 | 250 | | |
251 | 251 | | |
252 | | - | |
| 252 | + | |
253 | 253 | | |
254 | 254 | | |
255 | 255 | | |
256 | 256 | | |
257 | | - | |
| 257 | + | |
258 | 258 | | |
259 | 259 | | |
260 | 260 | | |
| |||
307 | 307 | | |
308 | 308 | | |
309 | 309 | | |
310 | | - | |
| 310 | + | |
311 | 311 | | |
312 | 312 | | |
313 | 313 | | |
314 | | - | |
| 314 | + | |
315 | 315 | | |
316 | 316 | | |
317 | | - | |
| 317 | + | |
318 | 318 | | |
319 | 319 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
69 | 69 | | |
70 | 70 | | |
71 | 71 | | |
72 | | - | |
| 72 | + | |
73 | 73 | | |
74 | 74 | | |
75 | 75 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
33 | 33 | | |
34 | 34 | | |
35 | 35 | | |
36 | | - | |
37 | | - | |
| 36 | + | |
| 37 | + | |
38 | 38 | | |
39 | | - | |
| 39 | + | |
40 | 40 | | |
41 | | - | |
| 41 | + | |
42 | 42 | | |
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
46 | | - | |
| 46 | + | |
47 | 47 | | |
48 | 48 | | |
49 | 49 | | |
| |||
90 | 90 | | |
91 | 91 | | |
92 | 92 | | |
93 | | - | |
| 93 | + | |
94 | 94 | | |
95 | 95 | | |
96 | 96 | | |
97 | | - | |
| 97 | + | |
98 | 98 | | |
99 | 99 | | |
100 | | - | |
| 100 | + | |
101 | 101 | | |
102 | 102 | | |
103 | 103 | | |
| |||
106 | 106 | | |
107 | 107 | | |
108 | 108 | | |
109 | | - | |
| 109 | + | |
110 | 110 | | |
111 | 111 | | |
112 | 112 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
59 | 59 | | |
60 | 60 | | |
61 | 61 | | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
62 | 68 | | |
63 | 69 | | |
64 | 70 | | |
| |||
73 | 79 | | |
74 | 80 | | |
75 | 81 | | |
76 | | - | |
77 | | - | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
78 | 91 | | |
79 | 92 | | |
80 | 93 | | |
| |||
85 | 98 | | |
86 | 99 | | |
87 | 100 | | |
| 101 | + | |
88 | 102 | | |
89 | 103 | | |
90 | 104 | | |
| |||
97 | 111 | | |
98 | 112 | | |
99 | 113 | | |
100 | | - | |
101 | | - | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
102 | 121 | | |
103 | 122 | | |
104 | 123 | | |
| |||
115 | 134 | | |
116 | 135 | | |
117 | 136 | | |
118 | | - | |
| 137 | + | |
119 | 138 | | |
120 | 139 | | |
121 | 140 | | |
| |||
136 | 155 | | |
137 | 156 | | |
138 | 157 | | |
139 | | - | |
| 158 | + | |
140 | 159 | | |
141 | 160 | | |
142 | 161 | | |
143 | | - | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
144 | 166 | | |
0 commit comments