You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit a57643d
Browse filesBrowse the repository at this point in the historyBrowse files
fix(plugin-approvals)!: refuse a reassign target that resolves to no one (#25592)
<!-- claude-projects-attribution: {"github_login":"os-sales"} -->
_Requested by **Sales** · [project
thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuMdu6odAshaFZ1Ujr7q9LcW)_Fixes#25485
Clause-②: no (narrowing)
Metadata-protocol: none
## What changed
`ApprovalService.reassign` now resolves `to` before it writes anything.
A target that resolves to no one is refused with `VALIDATION_FAILED`,
which the route already answers as `400`, and the message names the
address. The slate, the approver index, the audit trail and the
notifications are left as they were.
| `to` | Before | After |
|---|---|---|
| a user id with a `sys_user` row, member here or with no membership
rows | moves the slot | moves the slot |
| a user id with no `sys_user` row (`usr_does_not_exist`,
`nobody-at-all`) | **200, request stranded** | `400`: names no user in
this request's organization |
| a user whose memberships are all in other organizations | **200** |
`400`, same words as a missing user |
| an email an account carries (as stored) | moves the slot | moves the
slot |
| an email no account carries | **200** | `400` |
| `position:<p>` with a holder in the request's organization | moves the
slot | moves the slot |
| `position:<p>` that no one holds here | **200** | `400`: no one holds
position `p` here |
| `team:` / `department:` / `org_membership_level:` / a stored `role:` |
**200** | `400`: a slot literal no approver acts under |
- **Where the check sits.** It runs after the authorization branches, so
a caller who may not move the slot still gets `403 FORBIDDEN` and no
directory read is made for the target. A pin asserts that no read names
the target. The ordering ablation (check moved above the authorization)
turns that pin red.
- **The organization screen** is the posture the manager and team
expansions already use on `sys_member` (`managerIsProvablyOutsideOrg`,
`dropMembersProvablyOutsideOrg`). Membership rows that name only other
organizations mean refuse. No membership rows, or an unreadable
`sys_member`, means there is no tenancy fact, and the target stands. A
request with no organization skips the screen.
- **No probe of other tenants.** A missing user and a user of another
organization get one refusal text.
- **A failed `sys_user` read propagates**, so the caller is told and
nothing moves.
- **Positions** resolve through `expandPositionUsers`, the routing read
a position approver opens on. `approver-address.ts` gains
`addressedPosition(address)`, the inverse of `positionAddresses`, so the
`position:` prefix stays in the one module that owns it. The module is
not exported from the package entry.
- **The admin rescue** (`isOverride` with no slot held) is held to the
same rule. The override admits the caller; it does not exempt the new
slot.
- **No new error code.** `VALIDATION_FAILED` is already this door's
refusal (an empty `to` and an already-pending `to` use it).
`packages/rest` is untouched.
The changeset (`patch`, `**BREAKING**` banner, `(narrowing)` arm,
ADR-0087 `not-required (no-migration-prescription)`) gives the FROM → TO
and what a caller does.
## Tests
- `approval-service.test.ts` gains `reassign target resolution
(#25485)`, 10 cases. Each refusal asserts the named message, the slate
and the approver index unchanged, no `reassign` action row, and no
`approval.reassigned` notice:
- the card's two measurements: holder → `nobody-at-all`, platform admin
→ `usr_does_not_exist`;
- the admin rescue of a `position:ghost` slate onto an unknown id;
- an email no account carries;
- a user whose only membership is another organization, with the same
text as a missing user;
- a position held only in another organization;
- four `type:value` literals;
- the FORBIDDEN-first ordering (no read names the target).
- Preservation: a member here and a user with no membership rows; an
email in its stored spelling; a held `position:finance`, whose holder
then decides the request with the default actor.
- New
`packages/qa/dogfood/test/approval-reassign-target-resolves.dogfood.test.ts`
drives the real REST door on a booted stack, reusing the #22824
fixture's staffed object. The holder (unknown id, bare word, `team:`
literal) and the platform admin each get `400 VALIDATION_FAILED` naming
the address. The slate is unchanged and no `reassign` row exists. Then a
reassign to a member of the organization answers 200, and the member's
"My Pending" lists the request.
Fixtures that reassigned to an id with no account now give it one:
- `approval-service.test.ts`: a `staffDirectory(engine, …ids)` helper,
called in the 11 tests that hand a slot to `u7`, `u_next` or `u_holder`.
- `admin-exemption-retired.test.ts` and
`approval-override-audit.test.ts`: their `beforeEach` seeds `sys_user`
`u7`.
- `actor-lookups-hold-ids.integration.test.ts`: registers a minimal
`sys_user_position` and gives `position:finance` a holder (`u_boss`), so
the ruling's "email handed to a position" hand-off still runs.
## Verification
- `pnpm --filter @objectstack/plugin-approvals exec vitest run`: 95
files, 1244 tests, all passed. `pnpm --filter
@objectstack/plugin-approvals typecheck` passed, `check:test-typecheck`
included (the ledger held at 324). `pnpm --filter @objectstack/dogfood
typecheck` passed. eslint on the 7 changed source and test files: clean.
- Ablation, with `approval-service.ts` restored from `origin/main`:
- The 6 refusal pins in the new block went red. The 4 preservation pins
stayed green, as expected: the old code already moved a slot to a valid
target.
- The dogfood pin went red on its first refusal (`expected 200 to be
400`), after a rebuild of the plugin's `dist`.
- Ordering ablation, with the check moved above the authorization
branches: the FORBIDDEN-first pin went red.
- After each restore the file matched HEAD and `git status --porcelain`
was empty.
- Local gates (`dispatch-gates --commands`, 76 families): 74 exited 0 on
the first pass. `check-plugin-teardown-shape --self-test` and
`check:dual-build-cjs-loads` exited 3 on missing prerequisites (the
shallow clone's pinned fixture commit, 7 dists). Both exited 0 once the
fixture commit was fetched and the dists were built. `--ran` reconcile:
76 derived, 76 run, 0 NOT-MEASURED.
- `main` merged at `00d5635ad9`; nothing incoming touches
`plugin-approvals`, `rest` or the dogfood package. The full dogfood
battery (owed by the `(narrowing)` arm) runs on this head next. Its
counts go in the `os-dev-report` on #25485.
## Acceptance notes
- The branch is the harness-assigned `claude/project-thread-h5z02d`, not
`claude/issue-25485-…`. Claim 6378753403 on #25485 is the identity
record.
- The claim named "a new pin file". The unit pins went into the existing
`approval-service.test.ts` instead, beside the reassign tests it already
holds, so no new engine double was needed. The new file is the dogfood
pin.
- Out of scope, same family: the SLA escalation `action: 'reassign'`
(`escalateRequest`) falls back to the literal `escalateTo` when its
position expansion is empty. A config naming a user id that does not
exist, or an unstaffed position, still writes an unresolvable slate on
timeout. That target is authored in flow metadata, not sent to a door,
so the right fix is at authoring or publish time, not here. Handed to
the lane seat to file or cite.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01HopxxRtFE6z3z91wUD54tp)_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01HopxxRtFE6z3z91wUD54tp
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(plugin-approvals): reassigning an approval to an address that resolves to no one is refused, instead of stranding the request on a slate nobody can decide
6
+
7
+
Clause-②: no (narrowing)
8
+
Metadata-protocol: none
9
+
10
+
<!-- adr-0087: not-required (no-migration-prescription) No key of any metadata schema is removed, renamed or re-shaped, so there is nothing for `objectstack migrate meta` to rewrite and no tombstone. What changes is which `to` the runtime reassign door accepts in its request body. -->
11
+
12
+
**BREAKING** for a caller of `POST /api/v1/approvals/requests/:id/reassign` (or `ApprovalService.reassign`) that names a `to` no one can take.
13
+
14
+
-**What is refused.** FROM: any non-empty `to` that was not already pending was written onto the slate and answered 200, so `{"to":"usr_does_not_exist"}` left the request waiting on nobody until an override actor moved it again. TO: `to` must resolve in the request's organization, or the call answers `400 VALIDATION_FAILED` naming the address, and the slate, the audit trail and the notifications are unchanged:
15
+
- a user id or an email: a user account carries it (the email as the account stores it), and that user is not a member of other organizations only;
16
+
-`position:<name>`: someone holds that position in the request's organization;
17
+
- any other `type:value` address (`team:`, `department:`, `org_membership_level:`, a stored `role:`): never, because no approver acts under it.
18
+
-**Who is affected.** Every caller of the door, the admin rescue of a request whose slate names no one included. A missing user and a user of another organization get the same refusal text, so the answer does not reveal who exists elsewhere.
19
+
-**Unchanged.** A caller who may not move the slot is still refused `403 FORBIDDEN` first, before any directory read. An empty `to`, and a `to` that is already pending, are refused as before. The SLA escalation `reassign` action is not this door and is unchanged.
20
+
21
+
**What a caller does.** Reassign to the user id or email of a person in the request's organization, or to `position:<name>` for a position someone in it holds.
it('moves the slot to a user id of a member here, and to one with no membership rows at all (no tenancy fact, as the sibling screens read it)',async()=>{
0 commit comments