Skip to content

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

Merged
objectstack-fleet[bot] merged 10 commits into
mainfrom
claude/issue-22725-admin-rescue-queue-arm
Oct 11, 2026
Merged

objectstack-fleet[bot] merged 10 commits into
mainfrom
claude/issue-22725-admin-rescue-queue-arm

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

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

…nbox read returns an admin_rescue request to a caller who may override it (#22725)

IApprovalService.listRequests' filter declares `unstaffed?: boolean`, ORed
with `approverId`: pending requests whose current slate names no person,
returned only where isOverrideActor admits the caller. listRequests and
countRequests read it through one helper; the REST list door declares
`?unstaffed=true|false` and refuses any other value with 400 invalid_boolean.
The open path and the arm share one shape test, slateNamesAPerson.

Claude-Session: https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK
Co-authored-by: Claude <noreply@anthropic.com>
…ps an org-less tenant admin to org-less rescues

Claude-Session: https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK
Co-authored-by: Claude <noreply@anthropic.com>
…taffed-slate scan

Claude-Session: https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK
Co-authored-by: Claude <noreply@anthropic.com>
…oor on a real boot

Claude-Session: https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK
Co-authored-by: Claude <noreply@anthropic.com>
…ory — a later-staffed position slot still matches

Claude-Session: https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 11, 2026
@github-actions

github-actions Bot commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/plugin-approvals, @objectstack/rest, @objectstack/spec, touching 17 documentable anchor(s).

29 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json e84aeb36ce14169a633670f14ce8280fc998e2b9.

⛔ 5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: organization_id (literal, 33 pages)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 141 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json e84aeb36ce14169a633670f14ce8280fc998e2b9 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from c2ccf0bb6eee48bdc4961802e9951ec6d2283eb1 — the merge of head 640aed576dcfc7cd43e18b43c424296299124c51 into base e84aeb36ce14169a633670f14ce8280fc998e2b9, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin c2ccf0bb6eee48bdc4961802e9951ec6d2283eb1 && git checkout c2ccf0bb6eee48bdc4961802e9951ec6d2283eb1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e84aeb36ce14169a633670f14ce8280fc998e2b9 640aed576dcfc7cd43e18b43c424296299124c51 && git checkout -B drift-repro e84aeb36ce14169a633670f14ce8280fc998e2b9 && git merge --no-ff 640aed576dcfc7cd43e18b43c424296299124c51

node scripts/docs-audit/affected-docs.mjs --json e84aeb36ce14169a633670f14ce8280fc998e2b9

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs e84aeb36ce14169a633670f14ce8280fc998e2b9 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…w the head serves — the who-acts-next arms resolve to a bounded id set first

Claude-Session: https://claude.ai/code/session_016njDy8ozy9B9Ns5Y8kAWEK
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 640aed576dcfc7cd43e18b43c424296299124c51
Local-runs: none

Delta review over the held bb112188a9 record (the seat order 6106047492, the dev report 6106372839, the PR body as edited). The head is bb112188a9 plus two commits: e40ef1f1dc, a merge of origin/main e84aeb36ce, and 640aed576d, the patch-round-1 docblock rewrite. Net diff against origin/main (merge-base e84aeb36ce): the same 10 files, +788/-25. Every bb112188a9 judgment is carried forward where the delta does not touch it; the delta is re-judged below.

① Derived judgments

Check-runs on the head: 42, every one concluded — 38 success, 4 skipped, 0 failed, 0 cancelled. The skips: Console Pin Gate (path-scheduled; this diff removes nothing), Packed-tarball smoke (opt-in), and one Auto Label plus one Check PR Size row from a second run of the PR-meta suite on the same head, each green in the other run; none of the four is a required context. All seven required contexts on main are green as named rows: TypeScript Type Check, Test Core (and its six shards), Dogfood Regression Gate (and its three shards), Build Core, Temporal Conformance (live PG + MySQL), Lint & Repo Gates, Governed Surface Queue Guard.

Carried forward from bb112188a9, unchanged by the delta (per-file interdiff of the PR's change at the two heads, hunk headers ignored: identical for all nine non-spec files; the spec contract differs by the docblock hunk alone):

  • unstaffed?: boolean on the listRequests filter, countRequests on the same parameter type — right.
  • OR with approverId, confined to the two "who acts next" arms; AND with every other member; the alone case and false equals absent stated and right — right.
  • isOverrideActor the only admin read, asked first with the caller's org (as visibleRequestIds) and per row with the row's org (as attachViewers); every added row can_override: true; the tenant wall holds; no path hands a member or an org-A tenant admin an org-B row — right.
  • slateNamesAPerson behaviour-identical to the three inline copies; the in/out population true at the head; team:NAME literals do occur — right.
  • Window APPROVER_INDEX_CAP with the same truncation warn as the approver index; a failed read throws; list and count share awaitingRequestIds — right.
  • REST door: closed set, multiplicity set, readDeclaredQueryBoolean refusing everything but true/false as 400 VALIDATION_FAILED with fields[0] { field: 'unstaffed', code: 'invalid_boolean' } through handleRouteError, the limit/offset envelope; invalid_boolean in the ADR-0114 catalog — right.

Re-judged, the delta:

  • The merge e40ef1f1dc — right, a conflict-free union. The head's merge-base with main is e84aeb36ce (the branch contains main whole). Between the two merge-bases, main moved packages/plugins/plugin-approvals/src/approval-service.ts only through fix(plugin-approvals, plugin-audit)!: a lookup title is served only for a target whose declared exposure serves get #22766 (an import line and three hunks near the payload display-target exposure read, about lines 56, 6391, 6419 and 6477); none of those hunks names or neighbours awaitingRequestIds, unstaffedOverridableRequestIds, approverRequestIds, slateNamesAPerson, listRequests, countRequests, buildRequestWhere or isOverrideActor. The PR's change in that file is byte-identical at both heads; the arm's code is as reviewed, with APPROVER_INDEX_CAP = 10_000 now at line 6811. feat(spec,objectql,metadata-protocol,client): a write answer carries its advisory validation-rule hits as warnings #22749, feat(spec): declare IWebhookService.handleRedeliver, the optional transport-neutral webhook redeliver member #22767 and feat(runtime): activation door for positions and permission sets on sys_metadata_activation (stage 2c) #22736 touch no PR path. So the net diff at 640aed576d equals the bb112188a9 net diff plus the docblock delta, and nothing else.
  • The new limit docblock sentence — true at the head, and complete for what it claims. Each clause checked against the code: "applied by the engine for every filter" — listRequests puts limit/offset on the engine findOpts after the id constraint and the visibility intersection, with no in-memory slice on the path; "a status array pushes down as $in" — buildRequestWhere; "the two arms first resolve to a bounded set of request ids: the approver index for approverId, the pending-request scan for unstaffed" — awaitingRequestIds over approverRequestIds (sys_approval_approver) and unstaffedOverridableRequestIds (sys_approval_request, status: 'pending'); "each within the implementation's id window (APPROVER_INDEX_CAP, 10,000 …, which warns when a probe reaches it)" — both probes read limit: APPROVER_INDEX_CAP and warn on >= cap; "the window then applies engine-side to the rows in that set" — where.id is the id or $in the set, then the engine window. The old sentence survives nowhere at the head (no generated artifact carried it; check:generated and the reference docs are green in CI). Pre-existing and outside the order, not a defect of this delta: the contract still does not say the implementation clamps limit to 1–200 (default 50) and serves unpaginated callers a 500-row window.
  • Naming APPROVER_INDEX_CAP and its value in a spec contract docblock — a wording note, not blocking. The sentence attributes the bound to the implementation ("the implementation's id window … in @objectstack/plugin-approvals"), so it is not stated as a contract guarantee, and the file already names that package as the implementation (line 7); the seat's order asked for the constant by name, and the name and value are right at the head. The residual is drift: check:spec-docblock-symbol-anchors holds path:NNN citations, not bare identifiers, so a rename or re-tuning of the constant leaves this sentence stale with no gate reddening. A spelling that survives that — "an implementation-defined bound; the bundled @objectstack/plugin-approvals holds it at 10,000 at this release" — is the better one. Not owed by this round; the next change to the constant pays it.
  • The PR body — right. One Acceptance-note edit, the line on the limit docblock, now saying it is corrected in this PR by 640aed576d; no other line of the body changed (unified diff of the two bodies: that single line). The body's Suites and Gates sections still cite the bb112188a9 measurements; the dev's report 6106372839 carries the 640aed576d readings (spec suite, typecheck, check:generated, the plugin-approvals suite and the three dogfood approvals pins as the joint check for the merge). The check-runs on this head are the gate verdicts; the body's stale citation is not a false claim about the diff.

② Semver level

  • Unchanged from bb112188a9: @objectstack/spec, @objectstack/plugin-approvals, @objectstack/rest at minor in .changeset/22725-approvals-unstaffed-arm.md; @objectstack/dogfood is private; content/docs is no package; @objectstack/client's src does not move. Nothing moved is missing.
  • The docblock rewrite is text inside a file already in the changeset's @objectstack/spec entry; it moves no accept set, so minor still fits and no changeset edit was owed (the order's own condition).
  • Clause-②: yes (widening) in the PR body and the changeset — unchanged, matched; not breaking, so no ADR-0087 marker is owed.
  • No changeset line becomes false. The "How it composes" line ("paged server-side, and total counts both") now agrees with the contract's limit docblock instead of contradicting it; the "What it selects", "Who it returns them to", "What is refused" and "Before this" lines are judged true at the head as at bb112188a9.

③ Boundary flags

  • (c) the limit docblock, escalated at bb112188a9: resolved in-PR by 640aed576d, as ordered — a text-only rewrite, no other contract text, no code, no changeset change. Answered.
  • (a) @objectstack/client not widened: the seat's disposition stands — accepted as declared, no card. Answered; nothing at this head changes the reading (packages/client/src/index.ts hand-writes a filter without q, limit, offset or unstaffed, and no SDK consumer of the member is measured).
  • (b) the rest boolean reader mirroring runtime's parseBooleanParam: the seat accepts it as written. Answered.
  • The merge before the edit (os-regen-merge.sh, clean), the joint plugin-approvals and dogfood check beyond the order's gate list, the worktree recreated and removed: answered — procedural, and the per-file interdiff above is the review's own proof that the merge altered no PR path.
  • The body's Suites/Gates sections citing bb112188a9: answered above — the order allowed one body edit, the current-head readings are in the report, and the head's check-runs are the verdicts.
  • Writes this round as reported: git push, one PR-body patch, one report comment — within the order's budget. No governed surface in the file list (Governed Surface Queue Guard green); same-repo head.

Implemented-by: claude/issue-22725-admin-rescue-queue-arm
Reviewed-by: session_016njDy8ozy9B9Ns5Y8kAWEK

VERDICT: PASS

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 11, 2026 06:52
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 11, 2026 06:52
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 11, 2026
Merged via the queue into main with commit c1b7f2b Oct 11, 2026
44 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-22725-admin-rescue-queue-arm branch October 11, 2026 07:21
github-merge-queue Bot pushed a commit that referenced this pull request Oct 11, 2026
…nization's administrators; retire the unstaffed arm (#22850)

Fixes #22824

Clause-②: yes

An approval node whose approvers resolve to no concrete person now
opens, under the default `onEmptyApprovers: 'admin_rescue'`, on the
request organization's administrators, as ordinary approvers in the
ordinary inbox. The `unstaffed` queue arm that #22775 added (never
published) retires in the same stroke, since the named slate makes it
obsolete. Route P1, as ruled (maintainer 「同意 P1」, recorded as
`6108375797` on objectstack-ai/hotcrm#2057); triage `6108769161`; claim
`6108865702`.

## FROM → TO: who receives an `admin_rescue` request

- **FROM:** the request opened on a slate that named no one (the literal
`position:NAME`). Measured on this branch's base `efcbac73c`, on a real
boot (`bootStack`, the seeded admin owning the default organization):
the stored slate was `["position:unstaffed_arm_nobody"]`, the admin's My
Pending (`status=pending&approverId=ADMIN_ID,EMAIL`) answered `data: [],
total: 0`, their `GET /notifications?type=approval.requested` answered
`notifications: [], unreadCount: 0`, and no `sys_inbox_message` row was
written. Only the #3424 override could decide it.
- **TO:** the request opens on the holders of the `TENANT_ADMIN` rung of
the request's organization: the same authority the tenant arm of
`isOverrideActor` trusts (ADR-0095 D3, `ORGANIZATION_ADMIN_GRANTS`),
resolved to people. There is no second notion of "admin".

## What changed

**`@objectstack/plugin-approvals`** (`approval-service.ts`)
- `organizationAdministrators(org)`: the candidates are the
organization's members (`sys_member.organization_id`, this file's
existing answer to "who is in this organization"). Each is asked the
rung question over the grants `resolveUserAuthzGrants`
(`@objectstack/core`) resolves for them with that organization as the
active tenant: the resolver every authorization envelope comes from, so
grant windows, deactivated sets, other organizations' scoped grants and
position-bound sets answer exactly as when the person acts. No
`plugin-security` edit; no better-auth role is read as authority.
- `holdsTenantAdminRung(permissions)` is now the one spelling of the
grant read, shared by the tenant arm of `isOverrideActor` (a mechanical
swap of the identical expression; its authority does not change) and by
the slate.
- Platform administrators are not added for being platform
administrators. One who also holds the organization's rung (its owner,
say) is on the slate for that rung.
- At node entry, an empty slate under `admin_rescue` (and a `fallback`
that found no one either, which already degraded to `admin_rescue`) goes
through the SAME `expandApprovers` as `approvers`, as `user` entries in
one synthetic group (`#0`). So OOO delegation, the `approverId` index,
the `approval.requested` fan-out and `decide` apply unchanged, and a
decision by one of them is a slot decision recorded with `via_override:
false`.
- The request says why. The abandoned slate and the reason are kept in
`__resolvedFrom` as `admin_rescue:abandoned` / `admin_rescue:reason`
(the `fallback` branch's flat, policy-prefixed shape), and the timeline
carries one system note (`action: 'comment'`, `actor_id: null`, the
shape the OOO hop row already uses), for example "Routed to this
organization's administrators because the approver slate named no one:
position 'sales_manager' has no holder."
- Break-glass stays break-glass. An organization with no administrator,
a request with no organization, or an administrator read that fails
opens on the slate that names no one, logged as a warning that names
which of the three and the fix. A read failure answers a failure, never
an invented "nobody".
- The `unstaffed` arm retires: `awaitingRequestIds` and
`unstaffedOverridableRequestIds` are removed, and `listRequests` /
`countRequests` are byte-identical to their pre-#22775 bodies (compared
function by function). `slateNamesAPerson` stays (the open path reads
it); its docblock no longer names the arm.

**`@objectstack/spec`**: `IApprovalService.listRequests`' `unstaffed?:
boolean` member retires with its docblock. The `limit` docblock keeps
#22775's correction minus the arm. The `onEmptyApprovers` `.describe()`
text and TSDoc say that `admin_rescue` opens on the organization's
administrators and that `fallback` names who decides instead.
`content/docs/references/automation/approval.mdx` is regenerated
(`check:generated --fix`, `check:docs` only).

**`@objectstack/rest`**: `unstaffed` leaves
`APPROVAL_REQUEST_LIST_PARAMS` and the multiplicity set, and
`readDeclaredQueryBoolean` (its only reader) goes. The file equals the
exact reverse-apply of #22775's hunks onto `efcbac73c` (`git apply -R`,
then `cmp`). `?unstaffed=` is now refused as an unknown name, `400` with
nested `error.code: VALIDATION_ERROR`, before the service runs.

**`@objectstack/lint`**: `approval-approvers-may-resolve-empty` no
longer says "waits forever". Both arms say the request opens on the
organization's administrators. The hints name `onEmptyApprovers:
'fallback'` as the way to name who decides, and the `os explain`
paragraph says the same. Severity, firing conditions and rule ids are
unchanged.

**Docs** (`approvals.mdx`): a new "When a slate names no one" section.
The manager callout, the `onEmptyApprovers` list, the fallback text, the
notification callout and the admin-override callout now describe the
administrators' slate. The `unstaffed` filter paragraph and curl are
gone.

## The census, first

No shipped consumer of the `unstaffed` member or `?unstaffed` exists
outside #22775's own files:
- this repository at `efcbac73c`: every code hit of `unstaffed:`,
`?unstaffed`, `.unstaffed` and `'unstaffed'` is in #22775's file list.
The only non-test callers of `listRequests` / `countRequests` are the
REST handler's two calls;
- objectstack-ai/objectui `origin/main` `1d1cfe0` (read only):
`approvalsApi.ts` builds the list query from `status / object / recordId
/ approverId / submitterId / q / limit / offset` and never sends
`unstaffed`. Every other `unstaffed` hit there is the console's own seat
label (`approverIdentity.ts`'s `unstaffed: boolean` chip state), not the
filter.
- Not published: `npm pack
@objectstack/{spec,rest,plugin-approvals}@17.7.0` (npm `latest` is
`17.7.0`) holds 0 files matching `unstaffed?: boolean |
unstaffedOverridableRequestIds | readDeclaredQueryBoolean`; control:
each tarball holds 3–5 files naming `listRequests`.

## Mechanism assumptions, measured

1. **Holds.** See FROM above: the real boot at `efcbac73c` stored
literals only, listed and counted nothing for the admin, and notified no
one.
2. **Partly.** `ORGANIZATION_ADMIN_GRANTS` is declared in
`packages/spec/src/identity/eval-user.zod.ts`. NO reader resolves "the
users holding that rung in organization X": every reader is per user
(`resolveUserAuthzGrants`, `derivePosture`, the explain engine). So the
slate composes the per-user resolver over the organization's members.
Nothing in `plugin-security` was edited.
3. **Partly false.** The `fallback` branch keeps the primary slate's
resolution INPUTS (`field:NAME`, `expression#N`) and namespaces its own
as `fallback:KEY`. It keeps neither the abandoned slate nor a reason,
and a position-only slate writes nothing there (measured:
`__resolvedFrom` absent on the boot above). The new keys mirror that
flat, prefixed shape (`admin_rescue:abandoned`, `admin_rescue:reason`);
there is no second snapshot shape.
4. **Holds.** The member never shipped (the tarball reading above).
`check:adr-0087-registration --base origin/main`: exit 0. The new
changeset declares no breaking arm, so no disposition marker is owed,
and the deleted 22725 row is not a judged status. See Changesets for the
one gate that stays red.

## Pins

`src/admin-rescue-named-slate.integration.test.ts`: the real
`ApprovalService` on a real `ObjectQL` over `SqlDriver`
(better-sqlite3), the real `resolveUserAuthzGrants` over a bound
security catalog, and the real `listRequests` / `countRequests` /
`getRequest` / `decide` / messaging fan-out:
- a position nobody holds opens on the two administrators
(`organization_admin`, `organization_admin_no_bypass`), each one's My
Pending lists it with `total: 1` and `viewer.can_act: true`, and each is
told once;
- the scene anchored on that slate: a member sees nothing and is told
nothing, and a platform admin (unscoped `admin_full_access`) and another
organization's administrator are not on it;
- a tenant administrator's decision: `finalized: true`, `approved`,
`acted_as` their id, `via_override` false;
- the timeline note and `__resolvedFrom` (exact keys and reason);
- a `fallback` that resolves to no one degrades to the administrators
and says so in the reason;
- an administrator out of office is covered by their delegate
(`approval.ooo_substituted`), and is not told twice;
- CONTROLS: a staffed slate is unchanged (no administrator, no note);
`fallback` opens on its own list; `auto_approve` opens nothing and
`fail` refuses (`NO_APPROVERS`); an organization with no administrator
gets the slate that names no one, logs it, tells no one, and the
platform admin's decision there is an override (`via_override` true).

`packages/qa/dogfood/test/approval-admin-rescue-slate.dogfood.test.ts`
(the #22775 dogfood pin, rewritten to the named slate) on a real boot,
through the REST door: the seeded admin owns the default organization,
and a second member is promoted by a `sys_member` role write, which
provisions their capability. Both administrators find the request in My
Pending (`total: 1`, `can_act: true`) and hold one `approval.requested`
notification. A member sees and holds nothing. The staffed control is
unchanged. An administrator's `POST …/approve` is an approval with
`via_override` false and the routing note on the timeline.
`?unstaffed=true` answers `400 VALIDATION_ERROR`.

**Retired with the code they pinned**, ⛔ none skipped or disabled: the
9-case `ApprovalService — the unstaffed arm (#22725)` block in
`approval-service.test.ts` (the file equals the exact reverse-apply of
#22775's hunk), the REST §4 `unstaffed` block (replaced by one refusal
pin: `?unstaffed=true|false` answers the full ADR-0112 unknown-name
refusal and neither service method runs), and the dogfood arm pin
(rewritten above).

## Ablation (one-time, from committed `f3ff683a1`)

`node scripts/ablation-replace.mjs` replaced `if (administrators.length)
{` with `if (false && administrators.length) {` (anchor 1 → 0, blob
`259e3e834b00` → `551f2027d03b`), wrapped in a `trap` restoring `git
checkout HEAD --` on an absolute path. The integration file then had **6
failed / 4 passed**: every named-slate case red, and the four controls
(staffed, fallback, auto_approve / fail, and the leaderless organization
with its override) green. Restore proven: blob `259e3e834b00` equals
HEAD's, and `git diff HEAD` is 0 bytes. The integration test imports the
service from `src`, so no `dist` hop applies. The dogfood pin was not
ablated (it reads `plugin-approvals` through `dist`).

## Suites (final runs at `06f9948c8` unless noted)

- `@objectstack/plugin-approvals`: `test` 73 files / 1035 passed;
`typecheck` exit 0 (`check:test-typecheck` OK, the test layer included).
- dogfood, 12 approval-reading files (the new pin, override-composite,
my-pending, position-address-readers, flow-node-config,
notification-body, activity-text-predicate, both snapshot pins,
lookup-title-exposure, seed-ownership-claim-dispatch,
audit-log-admin-search): 12 files / 43 passed. `dogfood` `typecheck`
exit 0, and its `tsc --listFiles` lists the new pin and fixtures.
- `@objectstack/rest` (at `23a63a18a`; `rest` is unchanged since):
`test` 272 files / 5239 passed, 326 skipped; `typecheck` exit 0.
- `@objectstack/lint` (at `23a63a18a`; unchanged since): `test` 137
files / 6416 passed; `typecheck` exit 0.
- `@objectstack/spec` (at `f3ff683a1`; unchanged since): `typecheck`
exit 0; `test` 648 files / 19342 passed; `check:generated` all artifacts
up to date after the one `gen:docs` regeneration.
- The Dogfood Regression Gate's full population is left to CI.

## Gates (at `06f9948c8`, the final commit)

- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived **116** families (32 paths against
merge base `efcbac73c`). Each was run with its exit code recorded to
disk before any pipe: **115 exit 0, 1 exit 1**. The one red is
`check-empty-changeset.mjs --base origin/main`, the deliberate
correction below. `--ran` reconciliation: "116 derived, 116 run, 0
NOT-MEASURED, 0 UNRUN".
- Two families first answered `PREREQUISITE NOT MET` (exit 3) on unbuilt
packages: `check:skill-examples` (client-react) and
`check:dual-build-cjs-loads`. After those packages were built, both exit
0 in the final run. In an earlier run, `check:query-options-erasure`
caught two `as any` option erasures this branch had added, and
`check-tenant-audit-census` caught the new write call site. Both were
fixed (commits `06f9948c8`, `8af84ef80`) and both exit 0 in the final
run.
- Lint, narrowed and proven: eslint's population is
`**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}` (`eslint.config.mjs`). `eslint
--no-inline-config --format json` over the 24 changed TypeScript files
reports 24 files, 0 errors, 0 warnings. The config sets no
`parserOptions.project` (no type-aware linting), so this diff cannot
move any untouched file's verdict. The Markdown and MDX files are
outside eslint's population.
- The 49 artifact-roster families, the CI-only jobs and the type-check
lanes are left to CI.

## Changesets, and one deliberately red check

- `.changeset/22824-admin-rescue-named-slate.md`:
`@objectstack/plugin-approvals` minor, `@objectstack/spec` patch
(description text), `@objectstack/lint` patch (verdict text). `Clause-②:
yes`, stating FROM → TO. `@objectstack/rest` is not listed: against the
published 17.7.0, its shipped content does not move.
- **`.changeset/22725-approvals-unstaffed-arm.md` is withdrawn
(deleted).** It announces an `unstaffed` arm that this PR retires before
it ever ships, so it would publish a false release note. This is the
DELIBERATE CORRECTION class `check-empty-changeset.mjs` names.
`check:empty-changeset --base origin/main` stays **red by design**:
Check Changeset is not a required context, and per ruling D the
`skip-changeset` label is not applied. **Confirmation requested here:**
the 22725 note is withdrawn because the behaviour it describes does not
ship.

## File surface beyond the claim's list (declared)

My change turned these red, so each is updated to keep what it pins:
- `packages/lint/src/rule-explanations.ts` (the rule's `os explain`
paragraph) and the rule's hints in `validate-approval-approvers.ts`. The
dispatch ruled "the message text only"; the explanation and hints said
"waits forever" / "recoverable only by override", which this change
makes false, and the os-dev standard clause makes a text this change
falsifies a must-fix. The rule's logic is untouched. The conflict is
named in the report, not silently resolved.
- `approval-requested-notification.integration.test.ts`: one test name
and the header now say "with no organization administrator".
- Dogfood pins whose scene was a slate the administrators' slate now
replaces:
- `approval-override-composite-pin` + fixture route to a named approver
who never acts (the override's remaining break-glass case);
- `my-pending-position-address` and `position-address-readers` +
fixtures add a named co-approver, so the `position:NAME` literal stays
on the slate for the late holder;
- `flow-node-config-values-at-registration` asserts the administrators'
slate;
- `activity-text-predicate` and both approval-snapshot pins stamp the
holder's organization on the `sys_user_position` rows they seed. The
routing read never resolved an unstamped (unknown-tenancy) row, and the
literal fallback had masked that.
- `content/docs/permissions/tenant-audit-census.mdx` and
`docs/audits/2026-08-tenant-audit-write-call-sites.counts.md`: the
routing note's insert is one more write call site (238 → 239, decidable
159 → 160, elevated 126 → 127). Regenerated with `node
scripts/tenant-audit-census.mjs --write`, and the census-held prose
figures were updated by hand.

## Acceptance notes

- **The abandoned slate is replaced, not kept beside the
administrators**, as the ruling's "abandoned primary slate" and the
`fallback` branch do. A holder staffed into a position after an
all-literal request opened no longer receives it (an administrator can
reassign it). A `position:NAME` literal beside a named approver is
untouched, and so is the late holder's path to it.
- **Unstamped position assignments.** `expandPositionUsers` matches
`sys_user_position.organization_id` by strict equality (the platform's
stated fail-closed reading of unknown tenancy, shared with
`plugin-sharing`), so an organization-less assignment row never put its
holder on a slate. Before, the literal stayed and that holder could
still act through the `position:NAME` address. Now an all-literal slate
opens on the administrators. Rows written through a door are stamped,
and the showcase seed stamps them. Noted, not filed; carrier: none.
- **Cost.** One grant resolution per member of the request's
organization, eight at a time, on the empty-slate path only.
- **The submitter is not excluded** from the administrators' slate (a
one-admin organization decides its own request), the same as any slate
that names its submitter.
- `docs/qa/platform-checklist/areas/approvals.json` (the empty-slate
fork item) still says `admin_rescue` is "decidable only via the #3424
privileged override". It is internal QA text, not a published surface;
carrier: none.
- The `ApproverType` describe text ("a manager step resolves to nobody
and the request waits") is still literally true (it waits on the
administrators). It was left alone under the describe-only scope.


## Seat append: patch round 1 (the dev's note, appended by the
`domain:services` seat 1; the dev never edits this body)

- `origin/main` `d48931871` was merged in by merge commit `9152dfdf5`,
made by `scripts/pm/os-regen-merge.sh` (no rebase, no amend, no
force-push).
- The census conflict took `main`'s bytes in the merge. `9e24d7812` then
regenerates both census files with `scripts/tenant-audit-census.mjs
--write` and re-holds the page's prose figures (233 → 234: this branch's
one new write call site).
- **Gates at `9e24d7812`:** 116 derived, 116 run. The one expected red
is `check-empty-changeset`, the deletion of the `22725` note, which the
maintainer confirmed in `6109850105`.
- **Tests:** `plugin-approvals` 1035/1035, lint 6455/6455, approval
dogfood 43/43.


## Seat append: patch round 2 (the dev's note, appended by the
`domain:services` seat 1)

- The merge queue dequeued this PR at 2026-10-11T15:18Z
(`MERGE_CONFLICT`, the same two census files).
- `origin/main` `6364375b5` was merged in by merge commit `5d0a4a152`
(`os-regen-merge.sh`), and `e25e6c27f` regenerates both census files
with `scripts/tenant-audit-census.mjs --write` (231 → 232: this branch's
one write call site).
- **Gates at `e25e6c27f`:** 116 derived, 116 run. The one expected red
is `check-empty-changeset`, the `22725` deletion confirmed in
`6109850105`.
- **Tests:** `plugin-approvals` 1035/1035, lint 6461/6461, approval
dogfood 43/43, on dists rebuilt from the merged tree (which carries
#22854 and #22819).
- **The earlier contract review PASS (`6110255223`, on `9e24d7812e`)
does NOT carry to this head.** The hop moved paths this PR owns that are
not `merge=os-regen` regenerations: the two census files, plus
`rest-server.ts` and `rule-explanations.ts` from `main`'s side
(`unexplainedPathsBetween`, machine-read on the committed trees). So a
fresh at-tier review runs on `e25e6c27f`.

---
_Generated by [Claude
Code](https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants