Repository navigation
Commit fe835b4
fix(plugin-sharing)!: the ownership-queue service refuses a failed member read when the caller asks, and the approver slate and the sharing-recipient grant recompute ask (#30386)
<!-- claude-projects-attribution: {"github_login":"os-bill"} -->
_Requested by **Bill** · [project
thread](https://claude.ai/code/project/chan_016iMXWSZDzqWRWvWzdNo8wg?thread=cmsg_016iMXWSZDzqWRWvWzdNo8wgTHYrCvGEffG5116eJa3f7Z)_
domain:server#4 · os-bill · session_01Utp6CZMkHvuDeVYgTD73qF
## 维护者速读
- **改了什么:** 队列服务的 `expandMembers` / `isMember` 在调用方传 `onFailedRead:
'refuse'`
时,队列背后任何一次读取失败(`sys_queue`、`sys_queue_member`、team、position、`sys_member`、BU
表)都抛错并点名那次读取,不再答「无人」。审批节点的 queue 审批人(D6)和共享规则的 queue 收件人重算(D7)都传
`'refuse'`;准入类(D3 访问、D4 认领、D5 分配池)不传,仍答「无人」。
- **为什么改:** ADR-0142 D2 修订:空展开是事实,读取失败是故障。以前成员表一故障,D7 就把这些成员的授权全撤了,D6
节点照样开给剩下的人、fallback、管理员或自动通过。第 1 部分(#30288)留下的 "Not yet" 在这里兑现。
- **风险与回滚:** 只有读取失败时多一次拒绝:D6 节点报可重试的 `APPROVER_DIRECTORY_UNREADABLE`;D7
各重算保留规则已有授权并告警,读取恢复后下一次重算收敛(有 pin)。默认 `'nobody'` 路径不变,激活台账读失败仍按「开启」。回滚即撤
PR。
- **先看:** `ownership-queue-service.ts` 的
`queueReadRefusal`、`specsReachUser`、`holderCheck` 与 `expandSpec` 的
`pass.refuse` 分支;`sharing-rule-service.ts`、`approval-service.ts` 各一处传参。
- **两分钟验证:** `pnpm --filter @objectstack/plugin-sharing exec vitest run
src/ownership-queue.test.ts src/queue-recipient.test.ts -t "onFailedRead
'refuse'|keeps the queue-recipient|29604\] control"`,预期 13 条全过;`pnpm
--filter @objectstack/plugin-approvals exec vitest run
src/approval-service.test.ts -t "asks the queue service to refuse"`,预期 4
条全过。
- **需要你做的:** 无,席位合同审查后落地。spec 文档措辞有 3 处 follow-up owed(见 Acceptance
notes),本 PR 不改 spec。
Fixes #29604
Clause-②: yes (narrowing) — `expandMembers` and `isMember` of the
registered ownership-queue service, called with `onFailedRead:
'refuse'`, now reject when a read behind the queue fails, where they
answered nobody. The approver slate asks for that answer, so an approval
node whose `queue` approver cannot be read refuses to open under every
`onEmptyApprovers` policy, where it opened on the rest of the slate, on
the fallback slate or on the administrators, or auto-approved. The
sharing-rule recompute asks for it too, so a whole-rule pass
(`evaluateRule`, the REST evaluate endpoint, the boot backfill), the
recipient-axis revoke and the per-user pass on a `queue`-recipient rule
reject on such a failure, where they completed by revoking the grants of
the members who could not be read. Every call without the option, and
every read that succeeds, answers as before.
Metadata-protocol: none
Skills-docs: `content/docs/automation/approvals.mdx` corrected: it still
said a `team` or `position` entry whose holder read failed keeps its
literal slot, which part 1 (#30288) already made false; it now says a
failed read behind a `team`, `position` or `queue` entry refuses the
node, and keeps the literal statement for `department` and
`org_membership_level`. No page in `skills/**` states what a queue
answers on a failed read.
## What changes
Part 2 of #29604, under claim 6648819700 and seat ruling 6648946388, now
that #30251 (PR #30318) put `onFailedRead` on the contract. Part 1 (PR
#30288, `13c0c5baa`) is merged.
**1. `OwnershipQueueService` honours `onFailedRead`
(`plugin-sharing/src/ownership-queue-service.ts`).**
- `expandMembers(name, org, { purpose?, onFailedRead? })` and
`isMember(user, name, org, { onFailedRead? })` take the contract's
option. The per-call `ExpansionPass` carries `refuse`, an axis of its
own beside `purpose`.
- Under `'refuse'`, every read behind the queue passes its failure on
instead of contributing nobody: the `sys_queue` row lookup
(`authoredQueueRow`, which now words its own failure as `the sys_queue
row of queue '<name>' in organization '<org>' could not be read: …`),
the `sys_queue_member` walk (`authoredMemberSpecs`), each
specification's expansion (`expandSpec`) and each team or position
holder check (`holderCheck`). The paged reads already refuse on a failed
page or an incomplete walk (part 1 / #29503), so a refusal is never an
answer on the pages already read.
- The public call wraps the failure once, in `queueReadRefusal`:
`[queue] queue '<name>' in organization '<org>' does not answer its
members, because a read behind it failed: <the read, in its own words>.
The caller asked for a refusal over a partial answer (onFailedRead
'refuse'): …`. The inner read loses its `[sharing]` tag and its own
closing consequence ("…, so the expansion refuses instead."), and rides
along whole as `cause`.
- `isMember` under `'refuse'` walks every specification: a later one
that reaches the user still answers `true`, since a read that succeeded
and found the user is a fact a failure beside it cannot remove. Only
when none reaches does the first failure become the answer. So `false`
is answered only when every read succeeded, as the contract says. The
queue's own `sys_queue` / `sys_queue_member` reads come first and reject
before any specification is read, because without them the call does not
know what to ask.
- Under `'nobody'`, the default, nothing moves: each failed read
contributes nobody, logged once. `resolveQueue`, `queuesOf`,
`queuesNaming` and `hasAnyQueue` take no option and keep that answer.
- The activation-ledger read (`switchedOff`) keeps reading "switched on"
when it fails, under both answers. The contract left that to this card;
see Acceptance notes.
- The module header and the `holderCheck` / `readFailureReason` comments
no longer say the refusal waits on the contract.
**2. D7, the sharing-recipient grant recompute
(`plugin-sharing/src/sharing-rule-service.ts`).** The `queue` branch of
`expandRecipient` and of `ruleReachesUser` pass `{ onFailedRead:
'refuse' }`. `expandRecipient` feeds the whole-rule pass
(`desiredGrantsForRule`), the recipient-axis revoke
(`revokeRuleGrantsForRetiredRecipients`, which expands before it diffs)
and the record pass (`expandRecipientForRecord`); `ruleReachesUser`
feeds the per-user pass (`reconcileRuleForUser`). In each, the refusal
throws before any grant is diffed or revoked, so the rule's grants stay
as they were. No new catch was needed: each production caller already
isolates a refusing rule and warns, as it has for `team` and `position`
recipients since #29503 / #29362 (`evaluateAllForRecord`,
`evaluateAllRulesForObject`, the queue-hooks and BU-tree recomputes, the
membership-row recompute, the boot backfill).
**3. D6, the approver slate
(`plugin-approvals/src/approval-service.ts`).** `expandQueueUsers`
passes `{ purpose: 'addressing', onFailedRead: 'refuse' }`. Part 1's
catch in `resolveApproverSpec` already turns a `queue` expansion's
rejection into `ApproverDirectoryUnreadableError` before the "expanded
to nobody" return, so a queue the registered service cannot read now
refuses the node under every `onEmptyApprovers` policy, the fallback
slate included, with the queue's failed read as `failedRead`.
**4. Admission consumers untouched.** The sitting-record access path
(`SharingService` → `isMember` / `queuesOf`), the claim gate
(`security-plugin-authz-middleware.ts` → `isMember`), `explain-access`,
and the assignment pool (`objectql/src/assignment-rules.ts` →
`expandMembers`) call without the option and answer nobody on a failed
read, as before.
**5. Changeset.** The new `29604-queue-service-refuses-failed-read.md`
(BREAKING, `!`, Clause-② narrowing, ADR-0087 `not-required
(no-migration-prescription)`) carries this part. Its last bullet honours
part 1's "Not yet": it names the two sentences of part 1's pending entry
(its `Clause-②:` sentence that the service "does not reject yet", and
its **Not yet** bullet) and says they no longer hold and are to be read
as this bullet. Part 1's file itself is left as it stands on `main`; see
Acceptance notes.
## Tests
All new pins sit in the existing shared harnesses as top-level
`describe`s. Each `⛔` case was run against `origin/main`'s three source
files with this branch's tests: every one is red there.
- `ownership-queue.test.ts`, "onFailedRead 'refuse' rejects a failed
read, naming it (#29604)", 7 cases on the shared fake engine (one
table's reads made to throw):
- a failed `sys_queue` read rejects `expandMembers` and `isMember`,
naming the row read, with no warning; the default still answers `[]` /
`false`, warned once;
- a failed `sys_queue_member` read rejects, never the declared positions
alone (default: the positions alone);
- a failed `sys_team_member`, `sys_user_position` or `sys_member` read
behind a specification rejects with the exact read sentence, without
"refuses instead", and the driver error as `cause`;
- `isMember` under refuse: `true` when a read that succeeded reaches the
user, with the failing specification before or after it; a rejection for
a user only the failed read could reach, and for a non-member; the
default answers `false` for the same user;
- facts stay facts: an empty queue `[]`, an unresolved name `[]` /
`false`, healthy reads equal to the default;
- a refusal is not remembered: the next call, once the table reads,
answers the members;
- a failed activation-ledger read is not a failed read under refuse
either: "switched on", logged.
- `ownership-queue.test.ts`, D3 `describe`, 1 control: with
`sys_team_member` failing, the access path gives a team-only member no
access and throws nothing, and a position member keeps access.
- `queue-recipient.test.ts`, "a failed queue-member read keeps the
queue-recipient rule's grants (#29604)", 5 cases through the real
`OwnershipQueueService` and `SharingRuleService` over one engine (queue
`billing` = user `u_c` + team `t_night` → `u_d`): the whole-rule pass
rejects and revokes nothing, for a failed `sys_queue_member` and for a
failed `sys_team_member`; the recipient-axis revoke rejects and retires
nobody; the per-user pass rejects for `u_d` and still answers `u_c`; the
record pass keeps the grants and warns per rule with the queue's
refusal; once the read succeeds, the next pass converges (a member who
left loses the grant). The existing case "grants each member the
expander returns" now pins the third argument `{ onFailedRead: 'refuse'
}`. The fake engine gained a `failing` set.
- `approval-service.test.ts`, "a queue approver asks the queue service
to refuse a failed read, and the node refuses with it (ADR-0142 D6)", 4
cases, one per `onEmptyApprovers` policy: a queue double that answers as
the registered service does (rejects under `'refuse'`, `[]` under
`'nobody'`) is asked `{ purpose: 'addressing', onFailedRead: 'refuse'
}`, and the node refuses with `ApproverDirectoryUnreadableError` whose
`failedRead` is the queue's refusal, logs no "expanded to nobody" and
writes no request, approver or action row. The existing "asked for
ADDRESSING" case pins the new option too.
### Ablations, one per new refusal (`scripts/ablation-replace.mjs`, wrap
mode)
Each ran on the committed head, each restore proven (`blob == HEAD`,
`git diff HEAD` empty). Subjects are imported from source.
| # | mutation | red | which |
|---|---|---|---|
| A1 | the `sys_queue` row read never refuses | 1 of 64 | the
`sys_queue` case |
| A2 | the `sys_queue_member` read never refuses | 2 of 64 | the
`sys_queue_member` case, the whole-rule pass |
| A3 | a specification's expansion never refuses | 6 of 64 | the
member-table case, the not-remembered case, the whole-rule pass, the
revoke, the record pass, the convergence case |
| A4 | the holder check never refuses | 2 of 64 | the `isMember` case,
the per-user pass |
| A5 | `isMember`'s first failure ends the walk | 1 of 64 | the
`isMember` case (`true` past a failed position read) |
| A6 | D7 `isMember` asks for nobody | 1 of 64 | the per-user pass |
| A7 | D7 `expandMembers` asks for nobody | 5 of 64 | the
asked-arguments case, the whole-rule pass, the revoke, the record pass,
the convergence case |
| A8 | D6 asks for nobody | 5 of 463 | the asked-arguments case and the
4 policy cases |
| A9 | the refusal keeps the read's own "refuses instead" | 3 of 64 |
the three cases that pin the read sentence |
### Gates, local
Hashes are the pushed ones: the seven unpublished commits were reworded
once before the first push (the pre-push trailer check), with every tree
unchanged, so each run below stands on the commit named. `origin/main`
merged once (`b66d9c676`), then the whole workspace built (`pnpm build`,
119 of 119 tasks). On head `33a64b2be`, `node
scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 92 families on the changed paths,
and each exit code was written to a file before any pipe. 91 exited 0.
`check-empty-changeset --base origin/main` exited 1 on the in-place edit
of part 1's pending changeset (Acceptance notes); `b24881883` restores
that file from the merge base, and on it the derivation is unchanged
(same 92) and the four changeset gates exit 0 (`check-empty-changeset`,
`check-adr-0087-registration`, `check-changeset-no-major`,
`check:changeset-fixed`). Reconciled on that record:
```
Run reconciliation — 92 derived, 92 run, 0 NOT-MEASURED, 0 UNRUN.
✓ dispatch-gates --ran: 92 derived famil(ies) accounted for — 92 run, 0 NOT-MEASURED (a DERIVED zero — all 92 recorded an exit code and none of them is 3).
```
Also exit 0: the four roster gates whose roster sits under a changed
directory (`check:authz-resolver`, `check:changeset-fixed`,
`check:error-code-casing`, `check:filter-alias-parity`), `pnpm lint`,
the engine-double gates (`check:engine-double-contract`,
`check:objectql-double-limit`, both in the derived list), and
`typecheck` of both packages with the test-layer ledger
(plugin-sharing's new test file errors fixed, no ledger entry added).
Full suites: plugin-sharing 1405 passed (65 files), plugin-approvals
1469 passed (111 files).
### Dogfood battery, once, on the pushed head's tree (`b24881883`)
The whole workspace built, then `pnpm --filter @objectstack/dogfood
test` under `os-verify-lock.sh`:
```
Test Files 419 passed | 3 skipped (422)
Tests 3492 passed | 36 skipped (3528)
os-verify-lock: VERDICT command-exit 0 · held the lock 1656s (27m36s)
```
The `permission-set-clone-door` failure part 1 reported is gone on this
base (main reverted its cause in #30262).
## Acceptance notes
- **Activation ledger, decided: no move.** The contract says whether its
failure posture moves is this card's to decide. It stays "switched on"
under both answers: it is the posture the sharing-rule service takes for
the same ledger on its own `position` recipients
(`positionConfersAccess`), so a queue's position and a rule's position
cannot disagree during the outage, and the D6 slate reads positions on
the addressing purpose, which skips the position ledger. The cost,
stated: during such an outage a D7 pass can grant the holders of a
position that is switched off. That predates this card and is unchanged
here.
- **`isMember` under refuse answers `true` when a successful read
reaches the user.** The contract fixes when `false` may be answered
("only when every read behind it succeeded"); it does not say a failure
beside a positive read refuses. Refusing there would make the answer
depend on specification order, and on D7 it would hold back a grant the
user is owed.
- **The queue's own rows refuse first.** A failed `sys_queue` or
`sys_queue_member` read rejects `isMember` even for a declared queue
whose positions might reach the user, since the call cannot know the
queue's full membership without them.
- **Follow-up owed, spec wording (no spec file changed here, as
dispatched):** in
`packages/spec/src/contracts/ownership-queue-service.ts`,
`OwnershipQueueFailedReadAnswer`: (1) "the registered one … honours
`'refuse'` from #29604's code half on, so until then a caller passing
`'refuse'` still gets `'nobody'`" can drop its "until then"; (2)
"whether that posture moves is #29604's code half to decide" can state
the decision (it does not move); (3) `isMember`'s doc could say that
`true` is answered when a read that succeeded reaches the user, even
beside a failed one.
- **Part 1's pending changeset is not edited here; a person decides
whether it is.** Two of its sentences (the `Clause-②:` line's "The
registered ownership-queue service does not reject yet: …" and the **Not
yet** bullet) become false when this merges, while both entries are
still unreleased. I first corrected them in place, and
`check-empty-changeset` refused it as designed: rewriting a pending note
on the merge base is a release decision that needs a person's
confirmation, and the gate stays red until then. So this PR leaves the
file as on `main` and supersedes the two sentences from its own entry
instead. If the maintainer prefers the note corrected in place, it is a
three-line change
(`.changeset/29604-approver-directory-refusal-queue-paging.md`) that I
can push once confirmed, with that gate red by design.
- **Docs correction rides along.**
`content/docs/automation/approvals.mdx` had kept part 1's stale sentence
(a `team` / `position` entry whose holder read failed keeps its
literal). Part 1's PR body said no page stated it; that was wrong, and
this PR fixes the page.
- **Assignee and reviewers:** none set, as dispatched.
## Verification
```
pnpm --filter @objectstack/plugin-sharing exec vitest run src/ownership-queue.test.ts src/queue-recipient.test.ts -t "onFailedRead 'refuse'|keeps the queue-recipient|29604\] control"
pnpm --filter @objectstack/plugin-approvals exec vitest run src/approval-service.test.ts -t "asks the queue service to refuse"
```
Expect 13 and 4 passed.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01Utp6CXqXpuVf14Mqo1pznh
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 0e4dfbe commit fe835b4
8 files changed
Lines changed: 514 additions & 74 deletions
File tree
- .changeset
- content/docs/automation
- packages/plugins
- plugin-approvals/src
- plugin-sharing/src
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
455 | 455 | | |
456 | 456 | | |
457 | 457 | | |
458 | | - | |
459 | | - | |
460 | | - | |
461 | | - | |
462 | | - | |
463 | | - | |
464 | | - | |
465 | | - | |
466 | | - | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
467 | 481 | | |
468 | 482 | | |
469 | 483 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4176 | 4176 | | |
4177 | 4177 | | |
4178 | 4178 | | |
4179 | | - | |
| 4179 | + | |
4180 | 4180 | | |
4181 | 4181 | | |
4182 | 4182 | | |
| |||
4349 | 4349 | | |
4350 | 4350 | | |
4351 | 4351 | | |
| 4352 | + | |
| 4353 | + | |
| 4354 | + | |
| 4355 | + | |
| 4356 | + | |
| 4357 | + | |
| 4358 | + | |
| 4359 | + | |
| 4360 | + | |
| 4361 | + | |
| 4362 | + | |
| 4363 | + | |
| 4364 | + | |
| 4365 | + | |
| 4366 | + | |
| 4367 | + | |
| 4368 | + | |
| 4369 | + | |
| 4370 | + | |
| 4371 | + | |
| 4372 | + | |
| 4373 | + | |
| 4374 | + | |
| 4375 | + | |
| 4376 | + | |
| 4377 | + | |
| 4378 | + | |
| 4379 | + | |
| 4380 | + | |
| 4381 | + | |
| 4382 | + | |
| 4383 | + | |
| 4384 | + | |
| 4385 | + | |
| 4386 | + | |
| 4387 | + | |
| 4388 | + | |
| 4389 | + | |
| 4390 | + | |
| 4391 | + | |
| 4392 | + | |
| 4393 | + | |
| 4394 | + | |
| 4395 | + | |
| 4396 | + | |
| 4397 | + | |
| 4398 | + | |
| 4399 | + | |
| 4400 | + | |
| 4401 | + | |
| 4402 | + | |
| 4403 | + | |
| 4404 | + | |
| 4405 | + | |
| 4406 | + | |
| 4407 | + | |
4352 | 4408 | | |
4353 | 4409 | | |
4354 | 4410 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3482 | 3482 | | |
3483 | 3483 | | |
3484 | 3484 | | |
| 3485 | + | |
| 3486 | + | |
| 3487 | + | |
| 3488 | + | |
| 3489 | + | |
| 3490 | + | |
3485 | 3491 | | |
3486 | 3492 | | |
3487 | 3493 | | |
3488 | 3494 | | |
3489 | 3495 | | |
3490 | | - | |
| 3496 | + | |
3491 | 3497 | | |
3492 | 3498 | | |
3493 | 3499 | | |
| |||
0 commit comments