Repository navigation
Commit 6d487d2
fix(plugin-approvals): My Pending lists a position-routed request under either address spelling (#21378)
Fixes #21350
Clause-②: no
## What changes
The approvals inbox's "My Pending" (`GET
/api/v1/approvals/requests?approverId=...`) now finds a request routed
to a position for every user who holds that position, whichever spelling
of the position address the client sends.
The acting path's position-address equivalence now lives in one module,
`packages/plugins/plugin-approvals/src/approver-address.ts`, and three
readers use it:
- `resolveActor` (the acting path) admits exactly the identities it
admitted before. A pin compares it against the old predicate.
- `approverRequestIds` (the "My Pending" filter): a `role:P` or
`position:P` value matches the stored slot under both spellings. No
other spelling folds.
- `visibleRequestIds` (the participant gate behind every approvals
read): a "current approver" now also counts the slot addresses of every
position on the caller's server-resolved `context.positions`.
No second fold sits beside any of them, and nothing is client-specific.
## Why the participant gate is in this PR
Triage's ruling named the list filter. I measured on a real boot at
`origin/main` 5fd4855, before the fix. The scene was a `bootStack` app
whose flow routes to a position nobody holds at open time, with users
staffed into it after submission:
| caller | approverId with `role:P` | approverId with `position:P` | GET
the request | approve as `position:P` |
|---|---|---|---|---|
| staffed submitter (the card's HotCRM shape) | 0 rows | 1 row | not
read | not read |
| staffed reviewer, not the submitter, not an admin | 0 rows | 0 rows |
404 | 200 |
| bystander who holds a different position | 0 rows | 0 rows | not read
| 403 `FORBIDDEN` |
The first row is the card. The second row shows that folding the filter
alone would not reach a staffed reviewer who neither submitted the
request nor has admin standing. The participant gate counted a "current
approver" by bare user id, so the request was hidden under every
spelling and the detail read answered 404, even though that same user
could approve it. The ruling's pin ("a user staffed into the position
sees the request in My Pending under either spelling") needs both
halves.
The gate reads the same `positionAddresses` the acting path reads. So it
adds a request only for a caller whom `resolveActor` lets act on that
slot.
After the fix, in the same scene: the reviewer and the submitter each
get 1 row under both spellings, the reviewer's detail read answers 200,
the bystander gets 0 rows, and the acting path behaves as before.
## The spellings `resolveActor` accepts
For a non-system caller, `resolveActor` accepts three identities:
- the bare user id;
- `position:P` or `role:P`, for each P in the server-resolved
`context.positions`;
- an email that the caller's own `sys_user` row carries (a
case-insensitive read).
Only the second is a spelling equivalence, and it is the only one
folded. User ids and emails already match literally, and the console
sends both. `team:P`, `org_membership_level:P` and a bare name fold onto
nothing (pinned).
## Pins
- `plugin-approvals/src/approver-address.test.ts` pins the equivalence
itself:
- both prefixes, canonical first;
- the split is at the first prefix only;
- eight non-position addresses are equal only to themselves.
- `plugin-approvals/src/approval-service.test.ts`, describe "My Pending
position addresses (#21350)":
- a holder lists, counts and reads the request under both spellings;
- a 15.x `role:P` slot is found under `position:P`;
- a non-holder sees nothing;
- a spelling the acting path does not admit folds onto nothing;
- acting-path control: the holder decides, the bystander is refused;
- `resolveActor` is compared with the old predicate, verbatim, over a 15
x 7 matrix that includes non-string positions and position names that
contain a colon.
- `qa/dogfood/test/my-pending-position-address.dogfood.test.ts` (plus
its fixture) drives the real route on a booted app:
- reviewer and submitter, under both spellings;
- reviewer detail read answers 200;
- bystander gets an empty list and a 404;
- `team:P` and `org_membership_level:P` fold onto nothing (read as
admin);
- the bystander's approve answers 403 `FORBIDDEN`;
- the reviewer's approve answers 200, and the request is `approved`.
## Ablations
The fix was committed first (4007e6c). Each ablation went through
`scripts/ablation-replace.mjs` in WRAP mode. For every ablation the
anchor hit exactly once and the blob changed, and the restore was
proven: blob equals HEAD, and `git diff HEAD` is empty.
| ablation | unit pins | dogfood pin |
|---|---|---|
| A1: list filter matches literally | 3 red: both spellings, 15.x slot,
positive half of the non-admitted-spelling pin | red: reviewer under
`role:P` |
| A2: participant gate keyed on the user id only | 2 red: both
spellings, 15.x slot | red: reviewer under `role:P` |
| A3: normalizer widened with `team:` | 6 red, including the
`resolveActor` oracle | red: `team:P` must not fold |
All three went red, as predicted. The unit pins import the service
source relatively, and dogfood's `isolated` project aliases
`@objectstack/plugin-approvals` to `src/`, so no `dist/` leg applies.
## Tests (head d788026)
- `pnpm --filter @objectstack/plugin-approvals test`: 55 files, 842
tests passed.
- `pnpm --filter @objectstack/plugin-approvals typecheck`: exit 0.
`check:test-typecheck` OK, with no new debt; the new test files are in
the `tsconfig.test.json` program (`--listFilesOnly`).
- Dogfood `--project isolated`: the new pin plus the two sibling
approval pins (`approval-override-composite-pin`,
`approval-snapshot-masked-field`) all passed.
- `pnpm --filter @objectstack/dogfood typecheck`: exit 0. Both new files
are in the program.
- Gates: `node scripts/pm/dispatch-gates.mjs --repo
objectstack-ai/objectstack --commands` derived 94 commands at d788026,
and all 94 exited 0. Each exit code was captured before any pipe.
`--ran` reconciled 94 derived, 94 run, 0 NOT-MEASURED, 0 UNRUN.
- In the first pass, `check:skill-examples` and
`check:dual-build-cjs-loads` answered `PREREQUISITE NOT MET` (exit 3). I
built `@objectstack/client-react` and the seven packages without a
`dist/`, then re-ran the whole union at the final head.
## Acceptance notes
- **Not changed here, measured on the fixed branch (reported to the
seat, not filed by this PR):**
- The staffed reviewer still cannot decide from the console. The served
`viewer.can_act` is `false`, because `attachViewers` keys it on the user
id. The server-declared approve action sends no `actorId`, which
defaults to the user id and answers 403 "not a pending approver".
Sending `role:P` also answers 403, because the slot check is literal.
Only `actorId: position:P` succeeds.
- After deciding as `position:P`, the decider's detail read answers 404.
`sys_approval_action.actor_id` records `position:P`, and the gate's
"already acted" probe keys on the user id.
- **Docs:** `content/docs/automation/approvals.mdx` now says that the
inbox counts a held position's literal slot, that a position literal
matches under both spellings, and that the participant gate counts
holders. The deprecated prefix is described in words, because
`check:role-word` holds that file's count of the word.
- **ADR-0090 D3 note:** the `role:` arm is the deprecated spelling,
which the ADR retires without an alias window. This PR applies triage's
ruling and does not reopen it. With the equivalence in one module,
retiring `role:` once the console sends `position:` is a one-line edit.
- **objectui:** the docblock of `approverIdentities()` in
`packages/app-shell/src/hooks/sharedUserFeeds.ts`, at the pinned
`.objectui-sha` 31971ff1e, says: "The `role:` prefix is the SERVER's
addressing scheme for `pending_approvers` and stays as it is". The
server stores `position:P`. Per the claim, the coordination child is the
seat's to file; this PR does not touch objectui.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent d956910 commit 6d487d2
8 files changed
Lines changed: 561 additions & 19 deletions
File tree
- .changeset
- content/docs/automation
- packages
- plugins/plugin-approvals/src
- qa/dogfood/test
- fixtures
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
386 | 386 | | |
387 | 387 | | |
388 | 388 | | |
389 | | - | |
| 389 | + | |
| 390 | + | |
390 | 391 | | |
391 | 392 | | |
392 | 393 | | |
| |||
415 | 416 | | |
416 | 417 | | |
417 | 418 | | |
418 | | - | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
419 | 423 | | |
420 | 424 | | |
421 | 425 | | |
422 | 426 | | |
423 | 427 | | |
424 | 428 | | |
425 | | - | |
426 | | - | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
427 | 432 | | |
428 | 433 | | |
429 | 434 | | |
| |||
Lines changed: 136 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3587 | 3587 | | |
3588 | 3588 | | |
3589 | 3589 | | |
| 3590 | + | |
| 3591 | + | |
| 3592 | + | |
| 3593 | + | |
| 3594 | + | |
| 3595 | + | |
| 3596 | + | |
| 3597 | + | |
| 3598 | + | |
| 3599 | + | |
| 3600 | + | |
| 3601 | + | |
| 3602 | + | |
| 3603 | + | |
| 3604 | + | |
| 3605 | + | |
| 3606 | + | |
| 3607 | + | |
| 3608 | + | |
| 3609 | + | |
| 3610 | + | |
| 3611 | + | |
| 3612 | + | |
| 3613 | + | |
| 3614 | + | |
| 3615 | + | |
| 3616 | + | |
| 3617 | + | |
| 3618 | + | |
| 3619 | + | |
| 3620 | + | |
| 3621 | + | |
| 3622 | + | |
| 3623 | + | |
| 3624 | + | |
| 3625 | + | |
| 3626 | + | |
| 3627 | + | |
| 3628 | + | |
| 3629 | + | |
| 3630 | + | |
| 3631 | + | |
| 3632 | + | |
| 3633 | + | |
| 3634 | + | |
| 3635 | + | |
| 3636 | + | |
| 3637 | + | |
| 3638 | + | |
| 3639 | + | |
| 3640 | + | |
| 3641 | + | |
| 3642 | + | |
| 3643 | + | |
| 3644 | + | |
| 3645 | + | |
| 3646 | + | |
| 3647 | + | |
| 3648 | + | |
| 3649 | + | |
| 3650 | + | |
| 3651 | + | |
| 3652 | + | |
| 3653 | + | |
| 3654 | + | |
| 3655 | + | |
| 3656 | + | |
| 3657 | + | |
| 3658 | + | |
| 3659 | + | |
| 3660 | + | |
| 3661 | + | |
| 3662 | + | |
| 3663 | + | |
| 3664 | + | |
| 3665 | + | |
| 3666 | + | |
| 3667 | + | |
| 3668 | + | |
| 3669 | + | |
| 3670 | + | |
| 3671 | + | |
| 3672 | + | |
| 3673 | + | |
| 3674 | + | |
| 3675 | + | |
| 3676 | + | |
| 3677 | + | |
| 3678 | + | |
| 3679 | + | |
| 3680 | + | |
| 3681 | + | |
| 3682 | + | |
| 3683 | + | |
| 3684 | + | |
| 3685 | + | |
| 3686 | + | |
| 3687 | + | |
| 3688 | + | |
| 3689 | + | |
| 3690 | + | |
| 3691 | + | |
| 3692 | + | |
| 3693 | + | |
| 3694 | + | |
| 3695 | + | |
| 3696 | + | |
| 3697 | + | |
| 3698 | + | |
| 3699 | + | |
| 3700 | + | |
| 3701 | + | |
| 3702 | + | |
| 3703 | + | |
| 3704 | + | |
| 3705 | + | |
| 3706 | + | |
| 3707 | + | |
| 3708 | + | |
| 3709 | + | |
| 3710 | + | |
| 3711 | + | |
| 3712 | + | |
| 3713 | + | |
| 3714 | + | |
| 3715 | + | |
| 3716 | + | |
| 3717 | + | |
| 3718 | + | |
| 3719 | + | |
| 3720 | + | |
| 3721 | + | |
| 3722 | + | |
| 3723 | + | |
| 3724 | + | |
| 3725 | + | |
3590 | 3726 | | |
3591 | 3727 | | |
3592 | 3728 | | |
| |||
Lines changed: 38 additions & 15 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
70 | 70 | | |
71 | 71 | | |
72 | 72 | | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
73 | 76 | | |
74 | 77 | | |
75 | 78 | | |
| |||
1529 | 1532 | | |
1530 | 1533 | | |
1531 | 1534 | | |
1532 | | - | |
| 1535 | + | |
| 1536 | + | |
| 1537 | + | |
1533 | 1538 | | |
1534 | 1539 | | |
1535 | | - | |
| 1540 | + | |
1536 | 1541 | | |
1537 | 1542 | | |
1538 | 1543 | | |
| |||
6228 | 6233 | | |
6229 | 6234 | | |
6230 | 6235 | | |
6231 | | - | |
6232 | | - | |
| 6236 | + | |
| 6237 | + | |
| 6238 | + | |
| 6239 | + | |
| 6240 | + | |
| 6241 | + | |
| 6242 | + | |
| 6243 | + | |
| 6244 | + | |
| 6245 | + | |
6233 | 6246 | | |
6234 | 6247 | | |
6235 | 6248 | | |
6236 | 6249 | | |
6237 | 6250 | | |
6238 | 6251 | | |
6239 | | - | |
6240 | | - | |
6241 | | - | |
| 6252 | + | |
| 6253 | + | |
| 6254 | + | |
| 6255 | + | |
6242 | 6256 | | |
6243 | 6257 | | |
6244 | 6258 | | |
| |||
6269 | 6283 | | |
6270 | 6284 | | |
6271 | 6285 | | |
6272 | | - | |
6273 | | - | |
6274 | | - | |
6275 | | - | |
6276 | | - | |
6277 | | - | |
| 6286 | + | |
| 6287 | + | |
| 6288 | + | |
| 6289 | + | |
| 6290 | + | |
| 6291 | + | |
| 6292 | + | |
| 6293 | + | |
| 6294 | + | |
| 6295 | + | |
| 6296 | + | |
| 6297 | + | |
| 6298 | + | |
6278 | 6299 | | |
6279 | 6300 | | |
6280 | 6301 | | |
| |||
6301 | 6322 | | |
6302 | 6323 | | |
6303 | 6324 | | |
6304 | | - | |
6305 | | - | |
| 6325 | + | |
| 6326 | + | |
| 6327 | + | |
| 6328 | + | |
6306 | 6329 | | |
6307 | 6330 | | |
6308 | 6331 | | |
| |||
Lines changed: 47 additions & 0 deletions
| 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 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
0 commit comments