Repository navigation
Commit e0187d0
fix(plugins)!: business-unit recipients and approvers reach every member past 10,000 rows, and a failed member read refuses instead of answering nobody (#29647)
<!-- claude-projects-attribution: {"github_login":"os-warren"} -->
_Requested by **Warren** · [project
thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8XHx1fy3fEnpoGAbZ7vvncC)_
Fixes #29606
domain:server#4 · os-warren · session_01JQwxbWdszDuXCYxfoR2ynB
Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: updated here. `content/docs/automation/approvals.mdx` now
names `department` and the `sys_business_unit_member` read beside the
team, position and tier reads for a failed holder read, and says that an
`expression` value whose `resolveAs` read failed logs the same line. No
skill teaches a bound on business-unit members or what a failed member
read answers.
## 维护者速读
- 改了什么:共享规则与审批里「业务单元成员」(本单元 / 含下级单元) 现在分页读完全部成员;读失败不再当作「没人」,而是明确报错。开机回填
`sys_user.primary_business_unit_id` 也读完全部主成员关系,读失败或读不完整就不写并记录原因。审批表达式
`resolveAs` 展开失败时,现在和静态审批人一样打出「could not be expanded」告警并带上错误。
- 为什么改:这些读只读一次、上限 1
万行,读失败还会变空:共享规则会漏授权,业务单元结构一变,撤销流程会把整条规则的授权全部收回;审批会派给缩水的名单,或者把读失败记成「没人」;回填超过
1 万的用户永远不会被投影,而且什么都不说。
- 风险与回滚:唯一「从有答案变成报错」的情形是 `sys_business_unit_member` 的读本身失败(原来答空),属收窄,标题带
`!`;1 万人以下仍是一次读、结果不变;队列(ownership queue)的业务单元成员规格在读失败时仍贡献 0
人(文件头声明的姿态),只是现在会记一次日志,队列文件未改动。回滚即 revert 本 PR。
-
先看:`packages/plugins/plugin-sharing/src/business-unit-graph.ts`(`readMembers`)、`packages/plugins/plugin-approvals/src/approval-service.ts`(`expandBusinessUnitUsers`、`warnExpansionFailed`)、`packages/plugins/plugin-sharing/src/primary-bu-projection.ts`(`readPrimaryMemberships`)。
- 两分钟验证:`pnpm --filter @objectstack/plugin-sharing exec vitest run
src/business-unit-member-reads-complete.test.ts`,预期 14/14 通过(含 10,001
名成员全部在集合内、读失败抛错、缺表仍答无人、回填 10,001 行全部投影)。
- 需要你做的:无,席位落地。
## What changes
The `sys_business_unit_member` reads still read once at a 10,000-row
bound and turned a failed read into nobody. This extends the family
#29362 (PR #29497) and #29503 (PR #29602) closed for the team, role and
position reads to the last membership object, through the same shared
`keysetWalk` and `isObjectAbsentError` (`@objectstack/types`). No new
walk spelling for a holder read: plugin-sharing's member reads go
through `readMembershipToCompletion` (`team-graph.ts`),
plugin-approvals' through `readHoldersToCompletion`. No new export from
any package index.
**Item 1, site 1: sharing `BusinessUnitGraphService`.**
- `expandUnitMembers` (every `business_unit` recipient, and queue
`business_unit` specs) and `expandUsers` (every `unit_and_subordinates`
recipient, and queue `unit_and_subordinates` specs) read through one
private `readMembers`, which calls `readMembershipToCompletion`. A
failed or incomplete read throws, and nothing is cached for it.
- What a caller sees: a whole-rule pass rejects, so the rule's existing
per-rule warning fires and its grants stay. Before, `[]` made the
retired-recipient revoke that a business-unit graph write triggers
(`bu-tree-recompute.ts`) take the rule's grants from every member.
- Queues: `OwnershipQueueService.expandSpec` already catches an
expansion that throws, logs it once and contributes nobody (its declared
posture). That file is untouched (#29604 holds it). A queue BU spec
answers what it answered before on a failed read, and now logs it.
**Item 1, site 2: sharing primary-business-unit backfill
(`primary-bu-projection.ts`).**
- The boot scan of `is_primary` rows pages to completion through
`keysetWalk`. A scan that fails, or a walk that cannot vouch for
completion, writes nothing and warns with the consequence and the error.
Absence of the object answers nothing to project, silently.
- Refusing here means writing nothing and saying so, not throwing. The
scan's only caller is the plugin's boot step, whose catch would log
"projection not started" though the hooks were bound. The failed-scan
path already answered `{ updated: 0, refused: 0 }` with a warning, and
the existing pin ("a refused SCAN answers both counters, not a
half-built result") keeps that shape. It uses `keysetWalk` directly, not
`readMembershipToCompletion`: this is a projection scan rather than a
holder read, and the holder helper's refusal sentence speaks of grants.
- The two one-row reads in the same file (`recompute`, the
`beforeDelete` stash) are `limit: 1` existence probes that a bound
cannot cut, so they are unchanged.
**Item 1, site 3: approvals `expandBusinessUnitUsers`.**
- The member read goes through
`readHoldersToCompletion('sys_business_unit_member', …)`. A failed read
throws, the approver-spec resolver's directory catch takes it, and the
`department:ID` slot falls back to its literal with the `could not be
expanded` warning and its `error`. Before, a failed read logged
"expanded to nobody".
**Item 3: `resolveAs` expansion failure warns.**
- `resolveExpressionApprovers` caught a failed `department` / `position`
/ `team` expansion into `users = []` and pushed the `KIND:VALUE` literal
with no log line. It now logs the same warning `resolveApproverSpec`
logs, with the `error` and the `expression` source.
- The sentence lives in one private method, `warnExpansionFailed`, which
both resolvers call, so the two cannot drift.
**Item 2: measured, reported, not changed.** See the report on the card
and the Acceptance notes. ⛔ `ownership-queue-service.ts` is untouched.
## Tests
- New `plugin-sharing/src/business-unit-member-reads-complete.test.ts`
(14 cases, real ObjectQL + SqlDriver over in-memory SQLite, shipped
`SysBusinessUnit` / `SysBusinessUnitMember`, row ids scrambled against
user ids):
- `expandUnitMembers`: 10,001 members all in the set and nobody from
another unit or organization (2 reads); 9,999 members in one read; a
failed read throws naming the unit and the driver error; a failed read
is not cached; the object unregistered, or its table dropped, answers
`[]`.
- `expandUsers` (members split across the unit and its child): 10,001 (2
reads); 9,999 (1 read); a failed read throws; unregistered answers `[]`.
- Backfill: 10,001 primary rows all projected (2 reads, no warning);
9,999 in one read; a failed scan writes nothing and warns with the
error; unregistered projects nothing and says nothing.
- `primary-bu-projection.test.ts`: fixture rows now carry their `id`
(the scan seeks by it). The refused-scan case also asserts the warning
and its error, and a new case pins that a scan that cannot vouch for
completion writes nothing and warns.
- `business-unit-graph.test.ts` (fixture triage, no assertion changed):
its hand-rolled engine now stamps the `id` a stored row carries and
models `$and`, `$gt`, `orderBy` and `limit` on the member side, so its
25 cases keep pinning the two tenant screens and the two widths.
- New
`plugin-approvals/src/business-unit-holders-complete.integration.test.ts`
(11 cases, real engine):
- `expandBusinessUnitUsers`: 10,001 (2 reads, nobody from another unit
or organization); a `department` approver routes to all 10,001; 9,999 in
one read; a failed read throws; what the caller sees is
`department:bu_root` plus the `could not be expanded` warning with the
`SQLITE_IOERR` error; unregistered answers the literal with no failure
warning; dropped table answers `[]`.
- Item 3: a `resolveAs: 'department'` control expands to the members
with no failure warning; a failed `department` read keeps `{ id:
'department:bu_root', subGroup: 'bu_root' }` and logs one `could not be
expanded` warning carrying `type`, `value`, `organizationId`,
`expression` and the error; a failed `team` read through `resolveAs`
warns the same way; an empty unit keeps the literal with no failure
warning.
<details><summary>Ablation (source paths: these suites import the files
under test relatively, so no build is in the path)</summary>
- Committed the fix first (`2324d29f`), then a trapped script checked
out the three source files at the base (`4d8095ba`) and confirmed the
mutation landed (`readMembers` 0 hits, `keysetWalk` in the projection 0,
`warnExpansionFailed` 0, `limit: 10000` back in approvals 1).
- Sharing: `Tests 9 failed | 11 passed (20)`, the red ones being every
10,001, failed-read, not-cached, truncated-scan and quiet-absence case.
Approvals: `Tests 6 failed | 5 passed (11)`, the red ones being the
10,001 cases, both failed-read cases and both item-3 warning cases.
- Restored with `git checkout HEAD -- PATH`: `git diff HEAD` 0 bytes,
and each file's `git hash-object` equals its `HEAD` blob (`ddbfccfd…`,
`8ccd37de…`, `6d3a5423…`).
</details>
## Acceptance notes
- **Clause-② and Metadata-protocol** are the claim's lines, confirmed
from the diff: no export is added or removed from any package index, no
schema key moves, and the only answer that changes kind is a failed
member read (empty becomes a refusal), the `(narrowing)` arm. Review
face: `node scripts/pm/record-recognisers.mjs --template --paths …`
names no path face and defers to the changeset's `Clause-②:` line, so it
is `--face narrowing` (`CONTRACT_REVIEW_TIER`).
- **Item 2, measured on a real engine (scratch tests, not committed):**
- `QUEUE_READ_LIMIT` can silently drop a holder. One authored queue with
10,001 `user` member rows: `expandMembers` answered 10,000 and missed
`usr_010000`, `isMember` answered `false` for it, `queuesOf` answered
`[]` for it, and no warning was logged. 10,001 authored queues:
`queuesOf` missed a member of the 10,001st queue, no warning. 10,001
`team` member rows: `queuesNaming` missed the queue past the cap, so
rules naming it are not re-evaluated, no warning. ⛔ Not changed:
`ownership-queue-service.ts` belongs to #29604.
- `APPROVER_INDEX_CAP` in `organizationAdministrators` can drop an
administrator, loudly. 10,001 `sys_member` rows with the one
administrator on the last row: `{"ids":[]}`; on the first row:
`{"ids":["usr_010000"]}`. Both runs logged `organization-administrator
probe hit its window — results may be truncated`. The read has no
`orderBy`, so which members fall off is driver order. It is a declared
bound with a loud warning, so per the card it is reported and left; the
choice is on the card report.
- **Same-family reads left as they are** (outside this card's sites,
listed in the report for the seat): the `sys_business_unit` hierarchy
reads in both plugins. Sharing `descendants` (`limit: 1000`, catch →
`[]`) and `seedIsUsable` (catch → `false`, and the empty answer is
cached for the pass), approvals' seed check (catch → `[]`) and descent
(`limit: 1000`, catch → `[]`). A failed unit read still answers nobody
there. The new not-cached pin had to key its fault to the member object
for exactly this reason: a fault on the seed check is cached as `[]`.
- `protocol.ts` (frozen), `packages/spec` and
`ownership-queue-service.ts` are untouched.
## Verification
Head: `241a137f` (merges `origin/main` `fa07650b`; the incoming commit
touches none of these packages). Every reading below was taken on that
head, except the approvals suite, typechecks and the first derived-gate
run, which were taken on `0443a73e`. The only commit after it changes
`business-unit-graph.test.ts`, and the sharing suite was re-run on
`241a137f`.
<details><summary>Package suites and typecheck</summary>
- `@objectstack/plugin-sharing`: Test Files 63/63, Tests 1360/1360
(`241a137f`); typecheck exit 0.
- `@objectstack/plugin-approvals`: Test Files 110/110, Tests 1434/1434;
typecheck exit 0.
</details>
<details><summary>Full dogfood battery (the title carries <code>!</code>
and the declaration a <code>(narrowing)</code> arm)</summary>
- Closure: `turbo run build --concurrency=2 --filter='./packages/**'`
under the lock, `VERDICT command-exit 0`, 78/78 tasks (covers the
dogfood closure).
- `pnpm --filter @objectstack/dogfood test` under the lock on
`241a137f`: `VERDICT command-exit 0`, Test Files 398 passed and 3
skipped (401), Tests 3280 passed and 28 skipped (3308), 0 failed.
</details>
<details><summary>Derived gates</summary>
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`: 94 families derived (the same list on
`0443a73e` and `241a137f`). All 94 were run, each with its exit code
recorded.
- Four first answered exit 3 (PREREQUISITE NOT MET) and were re-run
after their prerequisite: `check-plugin-teardown-shape --self-test`
after `git fetch --unshallow origin main`; `check:skill-examples`,
`check:dual-build-cjs-loads` and `check:i18n` after `turbo run build
--filter='./packages/**'` (78/78 tasks). All four then exited 0.
- `--ran` reconciliation: `94 derived, 94 run, 0 NOT-MEASURED, 0 UNRUN`
(a DERIVED zero).
- `pnpm lint` (repo-wide eslint, `--no-inline-config`) on `241a137f`:
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_01JQwxbWdszDuXCYxfoR2ynB)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 7dae3b3 commit e0187d0
10 files changed
Lines changed: 753 additions & 80 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 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
456 | 456 | | |
457 | 457 | | |
458 | 458 | | |
459 | | - | |
460 | | - | |
461 | | - | |
462 | | - | |
463 | | - | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
464 | 466 | | |
465 | 467 | | |
466 | 468 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1121 | 1121 | | |
1122 | 1122 | | |
1123 | 1123 | | |
1124 | | - | |
| 1124 | + | |
1125 | 1125 | | |
1126 | 1126 | | |
1127 | 1127 | | |
| |||
2695 | 2695 | | |
2696 | 2696 | | |
2697 | 2697 | | |
2698 | | - | |
2699 | | - | |
2700 | | - | |
2701 | | - | |
2702 | | - | |
2703 | | - | |
| 2698 | + | |
2704 | 2699 | | |
2705 | 2700 | | |
2706 | 2701 | | |
| |||
2717 | 2712 | | |
2718 | 2713 | | |
2719 | 2714 | | |
| 2715 | + | |
| 2716 | + | |
| 2717 | + | |
| 2718 | + | |
| 2719 | + | |
| 2720 | + | |
| 2721 | + | |
| 2722 | + | |
| 2723 | + | |
| 2724 | + | |
| 2725 | + | |
| 2726 | + | |
| 2727 | + | |
| 2728 | + | |
| 2729 | + | |
| 2730 | + | |
| 2731 | + | |
| 2732 | + | |
| 2733 | + | |
| 2734 | + | |
| 2735 | + | |
| 2736 | + | |
| 2737 | + | |
| 2738 | + | |
| 2739 | + | |
| 2740 | + | |
2720 | 2741 | | |
2721 | 2742 | | |
2722 | 2743 | | |
| |||
2859 | 2880 | | |
2860 | 2881 | | |
2861 | 2882 | | |
| 2883 | + | |
| 2884 | + | |
| 2885 | + | |
| 2886 | + | |
| 2887 | + | |
2862 | 2888 | | |
2863 | 2889 | | |
2864 | 2890 | | |
| |||
3210 | 3236 | | |
3211 | 3237 | | |
3212 | 3238 | | |
3213 | | - | |
3214 | | - | |
3215 | | - | |
3216 | | - | |
3217 | | - | |
3218 | | - | |
3219 | | - | |
3220 | | - | |
3221 | | - | |
3222 | | - | |
3223 | | - | |
3224 | | - | |
3225 | | - | |
3226 | | - | |
| 3239 | + | |
| 3240 | + | |
| 3241 | + | |
| 3242 | + | |
| 3243 | + | |
| 3244 | + | |
| 3245 | + | |
| 3246 | + | |
| 3247 | + | |
| 3248 | + | |
| 3249 | + | |
| 3250 | + | |
| 3251 | + | |
| 3252 | + | |
| 3253 | + | |
| 3254 | + | |
3227 | 3255 | | |
3228 | 3256 | | |
3229 | 3257 | | |
| |||
3445 | 3473 | | |
3446 | 3474 | | |
3447 | 3475 | | |
3448 | | - | |
3449 | | - | |
| 3476 | + | |
| 3477 | + | |
3450 | 3478 | | |
3451 | 3479 | | |
3452 | 3480 | | |
| |||
0 commit comments