Repository navigation
Commit c1b7f2b
feat(spec,plugin-approvals,rest): an unstaffed arm on the approvals inbox read returns an admin_rescue request to a caller who may override it (#22775)
Fixes #22725
Clause-②: yes (widening)
An `onEmptyApprovers: 'admin_rescue'` request opens on a slate that
names no person (the literal `position:NAME`, or nothing). A privileged
admin may decide it, but nothing put it in that admin's queue.
`approverId` matches a slot against the addresses the caller names, and
an admin's own addresses never equal an unstaffed `position:NAME`. So no
"My Pending" listed it, the bell read 0, and a `lockRecord: true` record
stayed locked until someone looked under **All**. This PR adds the one
server-side arm that returns such a request to a caller who may override
it. It is one vertical PR, as triage's answer A on the card directs.
## What changed
- **Contract** (`packages/spec/src/contracts/approval-service.ts`):
`IApprovalService.listRequests`' filter gains ONE optional member,
`unstaffed?: boolean`, with its docblock. `countRequests` takes the same
filter type as `listRequests` (its first parameter), so it answers the
member too. Nothing else on the filter moves: `approverId` matches
exactly what it matched before.
- **Service** (`packages/plugins/plugin-approvals`):
- `awaitingRequestIds` is the one helper `listRequests` and
`countRequests` both call. It returns `approverRequestIds(approverId)`
ORed with `unstaffedOverridableRequestIds` when `unstaffed === true`, so
the list and its paged `total` cannot disagree.
- `unstaffedOverridableRequestIds` scans the pending requests inside the
caller's organization wall (`SYSTEM_CTX`, the approver-index window). It
keeps a row when the slate names no person AND `isOverrideActor(context,
row.organization_id)` holds.
- `isOverrideActor` is the only "admin" read, and there is no second
notion. It is asked first as `visibleRequestIds` asks it, so a caller
who may override nothing pays no scan. Then it is asked per row, as
`attachViewers` asks it for `viewer.can_override`, so every added row
reads `can_override: true`.
- `slateNamesAPerson` (in `approver-address.ts`) is the shape test, now
shared. The open path's three copies of the inline test that decides
"decidable only by a privileged admin" read it too, so the slate the
open path warns about and the slate the arm returns cannot drift apart.
These are mechanical replacements and change no behaviour.
- **REST door** (`packages/rest/src/rest-server.ts`): `unstaffed` joins
`APPROVAL_REQUEST_LIST_PARAMS` and the single-valued multiplicity set.
`readDeclaredQueryBoolean` reads it as `true` or `false` only. Anything
else (`1`, `TRUE`, `yes`, an empty `?unstaffed=`) is refused `400
VALIDATION_FAILED` with `fields[0]` `{ field: 'unstaffed', code:
'invalid_boolean' }` through the door's `handleRouteError`. That is the
envelope its `limit` / `offset` refusals answer, and the accepted set
and code are runtime's `parseBooleanParam` (`?read=` on the
notifications inbox). A repeated `?unstaffed=` gets the multiplicity
`400`.
- **Docs**: the approvals page's list-filter paragraph names the
parameter and its OR semantics.
- **Changeset**: `@objectstack/spec`, `@objectstack/plugin-approvals`
and `@objectstack/rest`, all `minor`.
## The member's shape: the spec seat's contract call, on the four axes
A NEW explicit member, ORed with `approverId`, never ANDed. I judge this
a contract-shape call, not a product split. The card's done-when and the
filed console follow-up (objectui#12102: "My Pending and the bell
include the requests the server's rescue arm returns") already place the
rescue requests in My Pending and the bell.
- **Actual business need, measured.** The console asks My Pending as ONE
server-side scope, and the bell as one polled request:
`status=pending&approverId=IDS` in objectui
`approvalRequestsDataSource.ts` (the `awaiting_me` scope, paged
server-side through `limit` / `offset` / `total`) and
`sharedUserFeeds.ts` (`useSharedPendingApprovalsCount`). Under an AND
member, the console would need two requests merged on the client. A
`ListView` over one data source cannot page that, and the bell would sum
two counts. Under OR, the follow-up is one added parameter on each
existing request. The badge objectui#12102 asks for is already on the
wire: a rescue row reads `viewer.can_act: false, can_override: true`.
- **Long-term soundness / contract-first.** `approverId` is NOT re-meant
for override actors. That would silently change a published member for
every caller, including callers that never asked. The new member is
opt-in and declared, and the REST door declares it in its closed set.
- **Keeping AI from writing it wrong.** The docblock states the OR, the
"alone it selects rescues only", and the `false` equals absent rule, so
`unstaffed: false` cannot be read as "exclude". The REST door refuses
every spelling but `true` / `false`, so a guessed value never answers a
`200` without the arm, a reply shaped exactly like "nothing to rescue".
The member's name says what it selects, not which policy produced it.
- **Startup scope.** One member, one arm, zero new gates and zero new
row fields. The client SDK is not widened (see Acceptance notes).
## Which population the arm returns, and why
The arm returns **pending requests whose current `pending_approvers`
holds no user id and no email**: only `type:value` literals, or nothing
at all. This is the test the open path makes before it warns "decidable
only by a privileged admin", read off the stored slate as it stands,
never off a flag stamped at open time. So the arm covers more than
`admin_rescue`'s opening slot:
- **in:** an `admin_rescue` open on a literal (`position:NAME`,
`team:NAME`) or on an empty slate (an expression or `manager` rung that
resolved to nobody). It also covers a `fallback` that degraded to
`admin_rescue`, a stored pre-rename literal that only the override can
decide, and a slate whose last named approver acted and left only
literals (pinned: `unanimous` over `[u9, position:sales_manager]` joins
the arm once u9 approves).
- **in, deliberately:** a `position:NAME` slot whose position gained a
holder AFTER the request opened. That holder also finds it under
`approverId`, and the admin can still decide it, so the arm never shows
a request its caller cannot act on. Dropping it would need a live
directory read per literal, a second staffing notion beside the open
path's.
- **out:** a slate that still names a person, even one who has since
left. A position that "becomes unstaffed later" is this case: a position
approver resolves to its holders' user ids at open time, so the slate
keeps naming them. Proving they left needs a per-slot directory read
that neither this arm nor the open path makes. Such a request stays
reachable under **All** and through `reassign`.
## Pins and evidence (measured on this branch)
Service, through `listRequests` AND `countRequests`
(`approval-service.test.ts`, `ApprovalService — the unstaffed arm
(#22725)`, 9 cases):
- an `admin_rescue` request (literal slot, and empty slate) appears for
a tenant admin and a platform admin, with every row reading
`can_override: true`;
- it does not appear for a member. The flag adds nothing and refuses
nothing, and the member pays no slate scan;
- CONTROL: a staffed request's queue is unchanged. Its approver reads
the same rows and total with and without the flag, the arm never claims
it, and the admin's flagless My Pending is unchanged;
- OR with `approverId`, across a paged window; `unstaffed: false` equals
absent; a tenant admin's arm stops at their organization; per row, an
org-less tenant admin reaches org-less rescues only; the slate as it
stands; a finalized rescue leaves the arm.
REST door, unit (`rest-server-approvals-unknown-filter.test.ts` §4):
`true` / `false` reach both `listRequests` and `countRequests` beside
`approverId`. Absent hands no arm. `1` / `TRUE` / `yes` / empty are
refused `400 invalid_boolean`, and neither service method runs. Repeated
is refused by the multiplicity gate. The closed-set preservation loop
sends `unstaffed` one of its own values.
Through the REST door on a real boot
(`packages/qa/dogfood/test/approval-unstaffed-arm.dogfood.test.ts`, a
new two-object fixture): the card's three pins on the paged list and its
`total` (the bell's number), plus the door's `400`.
Ablations, each run from a committed state with the restore proven by
blob equal to HEAD and an empty `git diff HEAD`:
- arm disabled (`if (filter?.unstaffed !== true || true) return
byApprover;`): 6 of the 8 service cases red. The 2 that stay green
(`false` equals absent, the tenant wall) do not depend on the arm being
on.
- per-row override test deleted: the org-less tenant-admin case red, the
other 8 green.
- REST reader replaced by the bare `q.unstaffed === 'true'`: 5 door
cases red (the four refusals, and "absent hands no arm").
- dogfood, through `dist/`: the arm mutation rebuilt into
`plugin-approvals`. `ablation-dist-preflight` confirmed the marker in
`dist/index.js` and `dist/index.mjs`, and the pin went red on the
admin's `approverId`-plus-`unstaffed` read. After the restore, the
rebuilt `dist/` had the marker absent and the tree clean.
Suites (full, not filtered), measured at the PR head `bb112188a9` (after
merging `origin/main` at `996aa86e0a`):
- `pnpm --filter @objectstack/plugin-approvals test`: 71 files, 1032
tests passed;
- `pnpm --filter @objectstack/rest test`: 272 files, 5247 passed, 326
skipped;
- dogfood: `approval-unstaffed-arm`, plus the two neighbouring approvals
pins (`approval-override-composite-pin`, `my-pending-position-address`):
3 of 3 passed;
- `typecheck` exit 0 for `plugin-approvals` and `rest` (both run
`check:test-typecheck`, which covers their test files), for `spec`, and
for `dogfood`, whose `tsc` program lists both new files;
- `pnpm --filter @objectstack/spec check:generated`: all 14 generated
artifacts up to date. `api-surface` records the interface by name, so
the member moves no snapshot.
Gates at `bb112188a9`:
- `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
--ran` reports 114 derived families, 114 run, 0 NOT-MEASURED, 0 UNRUN.
Every one carries a recorded exit code, and all are 0.
- The six artifact-roster families whose roster sits beside my paths are
also 0: `check-changeset-fixed`, `check:meta-url-spelling`,
`check:error-code-casing`, `check:filter-alias-parity`,
`check:engine-double-contract`, `check:route-ledger-census`.
- The CI-only jobs and type-check lanes that derivation names are left
to CI.
- Lint, narrowed and proven. eslint's own population is
`**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` (`eslint.config.mjs`), so the two
Markdown files are outside it. All 8 changed TypeScript files were
linted with `--no-inline-config --format json`: 8 files, 0 errors, 0
warnings. The config never enables type-aware linting (no
`parserOptions.project`), so this diff cannot move the verdict on any
untouched file.
## Acceptance notes
- **Client SDK not widened.** `@objectstack/client`'s
`approvals.listRequests` hand-writes its filter type, and it was already
a subset of the contract's (no `q`, `limit` or `offset`). The console
reads the door through its own `approvalsApi`, so there is no measured
consumer of the SDK method for this member. It is noted here, not filed.
Carrier: none.
- **The boolean reader is a second spelling of runtime's rule.**
`@objectstack/rest` cannot import `@objectstack/runtime`'s
`parseBooleanParam` (runtime depends on rest). The reader's docblock
names the rule it mirrors, the accepted set, and the code. Carrier:
none.
- **`listRequests`' `limit` docblock in the contract**: corrected in
this PR (patch round 1, `640aed576d`). It said an `approverId` /
status-array filter "post-filters in memory", with the window applied
after filtering, which was false at the head. It now says what the head
serves: the window is engine-level for every filter (a status array
pushes down as `$in`), and the two "who acts next" arms (`approverId`,
`unstaffed`) first resolve to a bounded id set (the approver index and
the pending scan, each within `APPROVER_INDEX_CAP`), with the window
applied to that set engine-side.
- **File surface beyond the claim's list:** the docs paragraph
(`content/docs/automation/approvals.mdx`) and the dogfood pin plus its
fixture, which carry the claim's "each pin runs through the service and
through the REST door".
---
_Generated by [Claude
Code](https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent a8f24b0 commit c1b7f2b
10 files changed
Lines changed: 788 additions & 25 deletions
File tree
- .changeset
- content/docs/automation
- packages
- plugins/plugin-approvals/src
- qa/dogfood/test
- fixtures
- rest/src
- spec/src/contracts
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
453 | 453 | | |
454 | 454 | | |
455 | 455 | | |
456 | | - | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
457 | 478 | | |
458 | 479 | | |
459 | 480 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2066 | 2066 | | |
2067 | 2067 | | |
2068 | 2068 | | |
| 2069 | + | |
| 2070 | + | |
| 2071 | + | |
| 2072 | + | |
| 2073 | + | |
| 2074 | + | |
| 2075 | + | |
| 2076 | + | |
| 2077 | + | |
| 2078 | + | |
| 2079 | + | |
| 2080 | + | |
| 2081 | + | |
| 2082 | + | |
| 2083 | + | |
| 2084 | + | |
| 2085 | + | |
| 2086 | + | |
| 2087 | + | |
| 2088 | + | |
| 2089 | + | |
| 2090 | + | |
| 2091 | + | |
| 2092 | + | |
| 2093 | + | |
| 2094 | + | |
| 2095 | + | |
| 2096 | + | |
| 2097 | + | |
| 2098 | + | |
| 2099 | + | |
| 2100 | + | |
| 2101 | + | |
| 2102 | + | |
| 2103 | + | |
| 2104 | + | |
| 2105 | + | |
| 2106 | + | |
| 2107 | + | |
| 2108 | + | |
| 2109 | + | |
| 2110 | + | |
| 2111 | + | |
| 2112 | + | |
| 2113 | + | |
| 2114 | + | |
| 2115 | + | |
| 2116 | + | |
| 2117 | + | |
| 2118 | + | |
| 2119 | + | |
| 2120 | + | |
| 2121 | + | |
| 2122 | + | |
| 2123 | + | |
| 2124 | + | |
| 2125 | + | |
| 2126 | + | |
| 2127 | + | |
| 2128 | + | |
| 2129 | + | |
| 2130 | + | |
| 2131 | + | |
| 2132 | + | |
| 2133 | + | |
| 2134 | + | |
| 2135 | + | |
| 2136 | + | |
| 2137 | + | |
| 2138 | + | |
| 2139 | + | |
| 2140 | + | |
| 2141 | + | |
| 2142 | + | |
| 2143 | + | |
| 2144 | + | |
| 2145 | + | |
| 2146 | + | |
| 2147 | + | |
| 2148 | + | |
| 2149 | + | |
| 2150 | + | |
| 2151 | + | |
| 2152 | + | |
| 2153 | + | |
| 2154 | + | |
| 2155 | + | |
| 2156 | + | |
| 2157 | + | |
| 2158 | + | |
| 2159 | + | |
| 2160 | + | |
| 2161 | + | |
| 2162 | + | |
| 2163 | + | |
| 2164 | + | |
| 2165 | + | |
| 2166 | + | |
| 2167 | + | |
| 2168 | + | |
| 2169 | + | |
| 2170 | + | |
| 2171 | + | |
| 2172 | + | |
| 2173 | + | |
| 2174 | + | |
| 2175 | + | |
| 2176 | + | |
| 2177 | + | |
| 2178 | + | |
| 2179 | + | |
| 2180 | + | |
| 2181 | + | |
| 2182 | + | |
| 2183 | + | |
| 2184 | + | |
| 2185 | + | |
| 2186 | + | |
| 2187 | + | |
| 2188 | + | |
| 2189 | + | |
| 2190 | + | |
| 2191 | + | |
| 2192 | + | |
| 2193 | + | |
| 2194 | + | |
| 2195 | + | |
| 2196 | + | |
| 2197 | + | |
| 2198 | + | |
| 2199 | + | |
| 2200 | + | |
| 2201 | + | |
| 2202 | + | |
| 2203 | + | |
| 2204 | + | |
| 2205 | + | |
| 2206 | + | |
| 2207 | + | |
| 2208 | + | |
| 2209 | + | |
| 2210 | + | |
| 2211 | + | |
| 2212 | + | |
| 2213 | + | |
| 2214 | + | |
| 2215 | + | |
| 2216 | + | |
| 2217 | + | |
| 2218 | + | |
| 2219 | + | |
| 2220 | + | |
| 2221 | + | |
| 2222 | + | |
| 2223 | + | |
| 2224 | + | |
| 2225 | + | |
| 2226 | + | |
| 2227 | + | |
| 2228 | + | |
| 2229 | + | |
| 2230 | + | |
| 2231 | + | |
| 2232 | + | |
| 2233 | + | |
| 2234 | + | |
| 2235 | + | |
| 2236 | + | |
| 2237 | + | |
| 2238 | + | |
| 2239 | + | |
| 2240 | + | |
| 2241 | + | |
| 2242 | + | |
| 2243 | + | |
| 2244 | + | |
| 2245 | + | |
| 2246 | + | |
| 2247 | + | |
| 2248 | + | |
| 2249 | + | |
| 2250 | + | |
| 2251 | + | |
| 2252 | + | |
| 2253 | + | |
| 2254 | + | |
| 2255 | + | |
| 2256 | + | |
| 2257 | + | |
| 2258 | + | |
| 2259 | + | |
| 2260 | + | |
| 2261 | + | |
| 2262 | + | |
2069 | 2263 | | |
2070 | 2264 | | |
2071 | 2265 | | |
| |||
0 commit comments