Repository navigation
Commit 9bdfe37
fix(plugins)!: team and position recipients reach every holder past 10,000 rows, and a failed holder read refuses instead of answering nobody (#29602)
<!-- claude-projects-attribution: {"github_login":"os-steve"} -->
_Requested by **Steve** · [project
thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi91ow1LxYabwyJZre5CQfiY4)_
Fixes #29503
domain:server#3 · os-steve · session_01ADHkmT6XzrVS2iPpNyXNYL
Clause-②: yes (narrowing)
Metadata-protocol: none
Skills-docs: updated here — `content/docs/automation/approvals.mdx` now
names `team` and the `sys_user_position` read beside the `sys_member`
one for a failed holder read; no skill teaches a bound on team or
position holders, or what a failed holder read answers.
## 维护者速读
-
改了什么:共享规则与审批里「团队成员」和「岗位任职」两类收件人/审批人现在分页读完全部人员;读失败不再当作「没人」,而是明确报错;按单个用户的成员判定(团队/角色/岗位)读失败也报错,不再答「不是成员」。
- 为什么改:这两处只读一次、上限 1
万行,读失败还会变空:共享规则会漏授权,按用户重算时一次读故障就会撤销这个人的授权;审批会派给缩水的名单(岗位只剩 sys_member
来源)却看起来完整。
- 风险与回滚:唯一「从有答案变成报错」的情形是这些表的读本身失败(原来答空或 false),属收窄,标题带 `!`;1
万人以下仍是一次读、结果不变;队列(ownership queue)按文件头声明的「读失败该成员规格贡献 0 人」作答:团队规格不变;岗位规格在
`sys_user_position` 读失败时,main 会贡献 `sys_member.role` 过渡源持有人,本 PR 起贡献 0
人,这是声明姿态第一次真正生效,是否改为拒绝留待裁决。回滚即 revert 本 PR。
-
先看:`packages/plugins/plugin-sharing/src/team-graph.ts`(`readMembershipToCompletion`
/
`probeMembership`)、`packages/plugins/plugin-sharing/src/ownership-queue-service.ts`(`holderCheck`
保持队列姿态)、`packages/plugins/plugin-approvals/src/approval-service.ts`(`readHoldersToCompletion`)。
- 两分钟验证:`pnpm --filter @objectstack/plugin-sharing exec vitest run
src/recipient-reads-complete.test.ts`,预期 16/16 通过(含 10,001
人全部在集合内、读失败抛错、缺表仍答无人)。
- 需要你做的:无,席位落地。
## What changes
The team and position recipient reads still read once at a 10,000-row
bound and turned a failed read into nobody. This completes the family
#29362 (PR #29497) started with the two `sys_member` reads, through the
same shared `keysetWalk` (`@objectstack/types`). No new spelling of the
walk: each plugin now has exactly one walk call for all its holder
reads.
**Site 1, `sys_team_member`.**
- Sharing `TeamGraphService.expandUsers` (every `team` sharing
recipient, and queue team members) reads to completion through
`readMembershipToCompletion`, the generalised #29362 helper in
`team-graph.ts`. A failed or incomplete read throws, and nothing is
cached for it.
- Approvals `expandTeamUsers` (the `team` approver, and `resolveAs:
'team'` expressions) reads through `readHoldersToCompletion`, the
generalised #29362 tier read in `approval-service.ts`. This is the same
table and defect in the second plugin the card title names. It is folded
in under os-dev's bounded in-place exemption (same class, mechanical,
same file as the card's site 2, same gate family) and declared in the
report.
**Site 2, `sys_user_position`.** The assignment limb of sharing
`PositionGraphService.expandPositionUsers` and of approvals
`expandPositionUsers` reads to completion. A failed read throws instead
of answering the `sys_member.role` limb alone. The ADR-0091 D2 validity
filter in sharing and the raw read in approvals (commit 04d03c3
ruling) are unchanged.
**Site 3, the user-scoped holder checks.**
- `teamHasMember` and `holdsRole` (`limit: 1`) keep their one-row probe,
which no bound truncates, through `probeMembership`. Only the catch
changed: absence answers `false`, and anything else throws.
- `PositionGraphService.holdsPosition` keeps its 1,000-row page, now as
a walk, so an active row behind a long run of lapsed rows still counts.
It refuses a failed assignment read instead of falling through to the
role check alone.
- Measured caller: `recipient-membership-recompute.ts` →
`SharingRuleService.reconcileRuleForUser` → `ruleReachesUser`. A thrown
probe now rejects before any revoke. The recompute's per-(rule, user)
catch logs `recipient membership re-evaluation failed — this user's
grants of the rule may not match …`, and the grants stay. On
`origin/main` a failed probe answered `false`, and
`reconcileRuleForUser` revoked every grant the user held from that rule.
Pinned in `recipient-membership-recompute.test.ts`: a user still on the
team, or still assigned, through a second row lost every grant on main.
**Site 4, queue `expandSpec`: left as declared.** ADR-0142 does not
declare "contributes nobody" on a failed read. D2 says "An empty
expansion is a fact, not a fault", and D5 says "A read that fails during
evaluation leaves the row as it was before evaluation … and logs at
`warn`" (for assignment rules). The posture is declared in
`ownership-queue-service.ts`'s header: "A failed read of a member table
contributes nobody for that specification (fail closed for every
admission that reads it)". This PR does not narrow it.
- Because sites 1–3 now throw, the queue's own holder check
(`specsReachUser`) would have started throwing to the access path and
the claim admission. A new `holderCheck` wrapper answers it exactly as
`expandSpec` answers the expansion: that specification contributes
nobody, logged once. A team specification's answer is therefore
unchanged. A position specification's is not: on a `sys_user_position`
outage it now contributes nobody, where main contributed that position's
`sys_member.role` transition-source holders. That is the posture the
queue's header declares for a failed member-table read, reached for the
first time. Either failure is now logged where before it was silent.
Pinned in `ownership-queue.test.ts`.
- The posture question (should a queue sharing recipient refuse too?) is
in the report's `open_questions`.
**Site 5, the absence check is homed.** `isObjectAbsentError(error,
readObject)` is new in `@objectstack/types`, beside
`isMissingTableError`. It is the shared missing-table predicate for
`readObject`, or the engine's `OBJECT_NOT_FOUND` naming `readObject`.
This card adds third and fourth callers (`sys_team_member` and
`sys_user_position`, in both plugins), so the two
`membershipObjectAbsent` copies are deleted. `readObject` is required.
No new export from `@objectstack/core`.
**What each end caller now sees.**
| Caller | Before (failed read) | Now |
|---|---|---|
| Sharing whole-rule pass (`reconcile`,
`revokeRuleGrantsForRetiredRecipients`, per-record pass), `team` /
`position` recipient | team: `[]`; position: role holders only → grants
revoked | expansion rejects → existing per-rule catch logs, grants stay
(**refuses where it answered**) |
| Sharing per-user recompute (`sys_team_member` / `sys_user_position`
write) | probe `false` → user's grants revoked | probe rejects →
per-pair catch logs, grants stay (**refuses where it answered**) |
| Approvals `team` / `position` approver | team: "expanded to nobody" +
literal; position: tier holders only | literal slot + `could not be
expanded — a directory or read-filter lookup failed` with `error`
(**refuses where it answered**) |
| Approvals escalation (`escalateTo` position), `resolveAs` expressions
| assignment-limb failure swallowed | their existing catches take the
throw (literal target / no users), as they already do for a `sys_member`
failure since #29497 |
| Approvals reassign door (`position:NAME` target) | assignment-limb
failure swallowed | the throw reaches the caller, as a `sys_member`
failure already does since #29497 |
| Ownership queue (access, claim, assignment pool, approver slate, queue
recipient) | team spec contributes nobody; position spec contributes the
`sys_member.role` transition-source holders only | both contribute
nobody, logged once. Team: unchanged. Position, on a `sys_user_position`
outage: nobody where main gave the transition-source holders (the
header's declared posture, reached for the first time) |
| Absent object (unregistered, or table never created) | no holders | no
holders (unchanged) |
**Forced test edits (two files, no assertion changed).**
`position-graph.test.ts`'s assignment fixtures and
`ownership-queue.test.ts`'s `sys_user_position` / `sys_team_member`
fixtures now carry an `id` on each row, as every stored row does.
`keysetWalk` reports a row without its key as `truncated`, and the read
refuses such a walk. That is the same edit #29497 made to the
`sys_member` fixtures.
## Tests
- `packages/plugins/plugin-sharing/src/recipient-reads-complete.test.ts`
(new). Real engine: ObjectQL + SqlDriver over in-memory SQLite, shipped
`SysTeamMember` and `SysMember`, scrambled row ids. `sys_user_position`
is declared with the shipped columns, because plugin-security is not a
dependency.
- Site 1: 10,001 members all in the set, the other team out, 2 reads.
Control: 9,999, 1 read. A failed read rejects. A failed read is not
cached. Unregistered and dropped table answer `[]`.
- Site 2: 10,001 active holders, other-org and lapsed rows out, 2 reads.
Control: 9,999 matching rows, 1 read. A failed assignment read rejects
even with a role holder present. Unregistered and dropped table answer
the role limb.
- Site 3: a positive and negative control for all three checks.
`teamHasMember`, `holdsRole` and `holdsPosition` each reject on a failed
read. Absence of all three objects answers `false`.
- `recipient-membership-recompute.test.ts` (3 added). The team and
position cases: deleting one of a user's two rows while the holder read
fails keeps the user's grants and logs the re-evaluation failure. A
healthy-read control.
- `ownership-queue.test.ts` (1 added). The queue posture: a failed team
or position read leaves `expandMembers` and `isMember` answering nobody
for that spec, and logs. Its fixture seeds no `sys_member` transition
row for the position, so main answered nobody there too; the pin that
seeds one (where main answered the transition-source holders) is
follow-up #29604.
-
`packages/plugins/plugin-approvals/src/team-position-holders-complete.integration.test.ts`
(new, same rig):
- Team: 10,001 members past the bound, and a team approver routes to all
of them. Control: 1 read. A failed read rejects. What the caller sees:
the literal plus the failure warning with `error`. Unregistered: the
literal, with no failure warning.
- Position: 10,001 past the bound. Control: 1 read. A failed assignment
read rejects. What the caller sees: the literal, never the tier holder
alone. Unregistered and dropped table: the tier limb.
- `packages/types/src/object-absence.test.ts` (new). Both absence
spellings. An absence attributed to another object, an unattributed
`OBJECT_NOT_FOUND`, and outages are not absence.
<details><summary>Reds on <code>origin/main</code> code (tests committed
first, at <code>5ee0c466</code>; the four source files restored from
<code>dbca3ef0</code>)</summary>
- Sharing: 11 failed / 68 passed across the three files. The shapes:
- `expected [ 'usr_000000', …(9998) ] to deeply equal [ …(9999) ]` (team
and position past the bound);
- `promise resolved "[]" instead of rejecting` (team);
- `promise resolved "[ 'usr_role' ]" instead of rejecting` (position,
role limb alone);
- `promise resolved "false" instead of rejecting` (all three checks);
- `expected [] to deeply equal [ 'acct_eu_1:read', 'acct_eu_2:read' ]`
(recompute team) and the same for `:edit` (recompute position): the
grants were revoked;
- the queue pin: `expected false to be true` on the warn only. Its
answers (`[]`, `false`) already held on main for this fixture, which has
no `sys_member` transition row (see the queue row above).
- Approvals: 7 failed / 5 passed. The past-bound pair for each table,
`promise resolved "[]" instead of rejecting` (team), `promise resolved
"[ 'usr_tier' ]" instead of rejecting` (position), and `expected [
'usr_tier' ] to deeply equal [ 'position:cfo_office' ]` (caller sees).
- Types: `Cannot find module './object-absence.js'`.
</details>
<details><summary>Ablations (each leg through
<code>scripts/ablation-replace.mjs</code>: anchor 1→0, blob moved, then
restored to blob == HEAD with an empty <code>git diff HEAD</code>;
plugin suites import source, so no dist leg)</summary>
- S1, sharing walk `break` after page 1: 2 failed (team and position
past the bound).
- S2, sharing walk catch → `return null`: 5 failed (team refuses,
not-cached, position refuses, `holdsPosition` refuses, recompute
position).
- S3, sharing probe catch → `return []`: 3 failed (`teamHasMember`,
`holdsRole`, recompute team).
- Q2, queue `holderCheck` rethrows: 1 failed (queue posture pin:
`whether user 'u_d' is a member of team 't_night' could not be read …`).
- A1, approvals walk `break` after page 1: 3 failed (team ×2, position
past the bound).
- A2, approvals catch → `return []`: 4 failed (team and position refuse,
both caller-sees pins).
- T2, `isObjectAbsentError` drops the object attribution: 2 failed
(another object's absence, the unattributed `OBJECT_NOT_FOUND`).
- First attempts at Q and T were refused by the tool before any run: the
anchor and replacement overlapped, so the counts could not move. They
were re-anchored as Q2 and T2. No result was read from the refused
attempts.
</details>
## Acceptance notes
- **Clause-② line differs from the claim.** The claim says `no
(narrowing)`. This diff adds a public export to `@objectstack/types`
(`isObjectAbsentError`), which widens that package's surface, so the PR
and changeset say `yes (narrowing)`, as the card delta's PM assumption
foresaw. Flagged to the seat.
- **Review face:** `node scripts/pm/record-recognisers.mjs --template
--paths …` names no path face and defers to the changeset's `Clause-②:`
line, whose `(narrowing)` arm makes it `--face narrowing`
(`CONTRACT_REVIEW_TIER`).
- **Same-family reads left as they are** (outside this card's five
sites, listed in the report for the seat):
- the business-unit member reads: sharing `business-unit-graph.ts` and
`primary-bu-projection.ts` (`limit: 10000`), and approvals
`expandBusinessUnitUsers` (`limit: 10000`, catch → `[]`);
- the queue service's own `sys_queue_member` reads (`QUEUE_READ_LIMIT`);
- approvals `organizationAdministrators` (`APPROVER_INDEX_CAP`, warns
when the cap is hit).
- `protocol.ts` (frozen) and `resolve-authz-context.ts` (#29398) are
untouched.
## Verification
Head: `38c62472` (merges `origin/main` `66ba23e4`; the incoming commits
touch none of these packages). Every reading below was taken on that
head. The one commit after it, `85872264`, changes only the changeset's
prose (the queue position-spec sentence), so no reading was re-taken.
The Dogfood Regression Gate legs skip on `pull_request`, so the dogfood
reading below is the only one on this code.
<details><summary>Package suites and typecheck</summary>
- `@objectstack/types`: Test Files 35/35, Tests 842/842; typecheck exit
0.
- `@objectstack/plugin-sharing`: Test Files 62/62, Tests 1345/1345;
typecheck exit 0.
- `@objectstack/plugin-approvals`: Test Files 109/109, Tests 1400/1400;
typecheck exit 0.
- `pnpm --filter @objectstack/spec check:generated` exit 0 (spec moved
on `main`'s side of the merge).
</details>
<details><summary>Full dogfood battery (the title carries <code>!</code>
and the declaration a <code>(narrowing)</code> arm)</summary>
- Closure build `turbo run build --filter='@objectstack/dogfood^...'`
under the lock: `VERDICT command-exit 0`, 68/68 tasks.
- `pnpm --filter @objectstack/dogfood test` under the lock: exit 0, Test
Files 397 passed and 3 skipped (400), Tests 3277 passed and 28 skipped
(3305), 0 failed.
</details>
<details><summary>Derived gates</summary>
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`: 93 families derived on `38c62472`. All 93
were run, each with its exit code recorded.
- Three first answered exit 3 (PREREQUISITE NOT MET), so they were
re-run after their prerequisite: `check-plugin-teardown-shape
--self-test` after fetching its pinned fixture commit;
`check:skill-examples` and `check:dual-build-cjs-loads` after `turbo run
build --filter='./packages/**'` (all cache hits). All three then exited
0.
- `--ran` reconciliation: `93 derived, 93 run, 0 NOT-MEASURED, 0 UNRUN`
(a DERIVED zero).
- `pnpm lint` (repo-wide eslint, `--no-inline-config`): exit 0.
- CI families are not re-run locally; the PR check-runs are the record.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01ADHkmT6XzrVS2iPpNyXNYL)_
<!-- signed: domain:server#3 · os-steve ·
session_01ADHkmT6XzrVS2iPpNyXNYL -->
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 0bfb225 commit 9bdfe37
14 files changed
Lines changed: 1029 additions & 203 deletions
File tree
- .changeset
- content/docs/automation
- packages
- plugins
- plugin-approvals/src
- plugin-sharing/src
- types/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 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
455 | 455 | | |
456 | 456 | | |
457 | 457 | | |
458 | | - | |
459 | | - | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
460 | 461 | | |
461 | 462 | | |
462 | 463 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
24 | | - | |
| 24 | + | |
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| |||
1119 | 1119 | | |
1120 | 1120 | | |
1121 | 1121 | | |
1122 | | - | |
1123 | | - | |
1124 | | - | |
1125 | | - | |
| 1122 | + | |
| 1123 | + | |
| 1124 | + | |
| 1125 | + | |
| 1126 | + | |
1126 | 1127 | | |
1127 | | - | |
1128 | | - | |
1129 | | - | |
1130 | | - | |
1131 | | - | |
1132 | | - | |
1133 | | - | |
1134 | | - | |
1135 | | - | |
1136 | | - | |
1137 | | - | |
1138 | | - | |
1139 | | - | |
1140 | | - | |
1141 | | - | |
1142 | | - | |
1143 | | - | |
1144 | | - | |
| 1128 | + | |
1145 | 1129 | | |
1146 | 1130 | | |
1147 | 1131 | | |
| |||
2935 | 2919 | | |
2936 | 2920 | | |
2937 | 2921 | | |
2938 | | - | |
2939 | | - | |
2940 | | - | |
2941 | | - | |
2942 | | - | |
2943 | | - | |
2944 | | - | |
2945 | | - | |
2946 | | - | |
2947 | | - | |
| 2922 | + | |
| 2923 | + | |
| 2924 | + | |
| 2925 | + | |
| 2926 | + | |
| 2927 | + | |
2948 | 2928 | | |
2949 | 2929 | | |
2950 | 2930 | | |
| |||
3341 | 3321 | | |
3342 | 3322 | | |
3343 | 3323 | | |
3344 | | - | |
3345 | | - | |
| 3324 | + | |
3346 | 3325 | | |
3347 | | - | |
3348 | | - | |
3349 | | - | |
3350 | | - | |
3351 | | - | |
3352 | | - | |
3353 | | - | |
3354 | | - | |
3355 | | - | |
| 3326 | + | |
| 3327 | + | |
| 3328 | + | |
| 3329 | + | |
| 3330 | + | |
| 3331 | + | |
| 3332 | + | |
| 3333 | + | |
3356 | 3334 | | |
3357 | 3335 | | |
3358 | 3336 | | |
| |||
3440 | 3418 | | |
3441 | 3419 | | |
3442 | 3420 | | |
3443 | | - | |
| 3421 | + | |
3444 | 3422 | | |
3445 | 3423 | | |
3446 | 3424 | | |
| |||
3452 | 3430 | | |
3453 | 3431 | | |
3454 | 3432 | | |
3455 | | - | |
| 3433 | + | |
3456 | 3434 | | |
3457 | 3435 | | |
3458 | 3436 | | |
3459 | 3437 | | |
3460 | 3438 | | |
3461 | | - | |
| 3439 | + | |
| 3440 | + | |
| 3441 | + | |
| 3442 | + | |
| 3443 | + | |
| 3444 | + | |
| 3445 | + | |
| 3446 | + | |
| 3447 | + | |
| 3448 | + | |
| 3449 | + | |
| 3450 | + | |
| 3451 | + | |
| 3452 | + | |
| 3453 | + | |
| 3454 | + | |
| 3455 | + | |
| 3456 | + | |
| 3457 | + | |
| 3458 | + | |
| 3459 | + | |
| 3460 | + | |
| 3461 | + | |
| 3462 | + | |
3462 | 3463 | | |
3463 | 3464 | | |
3464 | | - | |
3465 | | - | |
| 3465 | + | |
| 3466 | + | |
3466 | 3467 | | |
3467 | 3468 | | |
3468 | 3469 | | |
| |||
3472 | 3473 | | |
3473 | 3474 | | |
3474 | 3475 | | |
3475 | | - | |
| 3476 | + | |
3476 | 3477 | | |
3477 | 3478 | | |
3478 | 3479 | | |
| |||
0 commit comments