Skip to content

Commit 4b8cbe2

Browse files
fix(core): the fellow-org peer list keeps every member the first membership read returned (#29545)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8UqrAZsJzxmjyAzE8Z3mZR1)_ Refs #29398 (item 2) Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:组织成员数达到 1,000 时,解析器会先读一次 1,000 行、再按 id 分页读完整成员;现在两次读的结果取并集,而不是用第二次的结果替换第一次。 - 为什么改:若补全读看到的成员比第一次少(另一连接上的旧快照),同组织成员列表 `org_user_ids` 会比第一次读本身还短,这些成员在普通成员的 `sys_user` 读取里悄悄消失。 - 风险与回滚:两次读都按当前组织过滤,并集不会加入组织外的人;不增加也不改变任何读;回滚即 revert 本 PR。 - 先看:`packages/core/src/security/resolve-authz-context.ts` 第 5 步(`orgMembers` 的并集一行),再看同目录 `resolve-authz-context.org-members-past-first-read.test.ts` 的新用例。 - 两分钟验证:`pnpm --filter @objectstack/core exec vitest run src/security/resolve-authz-context.org-members-past-first-read.test.ts`,预期 9/9 通过。 - 需要你做的:无,席位落地。卡的第 1 项(按 id 顺序的逐页校验)不在本 PR,卡保持打开,见 Acceptance notes。 ## What changes `resolveUserAuthzGrants` step 5 builds `org_user_ids` from Leg 1's `sys_member` read (`orgMembersLeg`, bounded at 1,000 rows). When that read comes back at its bound, `readOrganizationMembersToCompletion` reads the whole membership in id order. Its rows used to replace `orgMembersLeg`. They are now unioned with it, so a completion that sees fewer members than Leg 1 already held cannot shorten the list. Both reads filter on the active organization, so the union adds nobody outside it. No read is added, removed or changed, and the batch-equivalence goldens do not move. One `patch` changeset for `@objectstack/core`. ## Why This is item 2 of the card: a completion that answers from an older snapshot on another connection returned a shorter list than the first read alone. Item 1 (the per-page order check) is held back. The Acceptance notes say why. ## Tests - New case in `resolve-authz-context.org-members-past-first-read.test.ts`: 1,005 members, and the completion's ordered reads cannot see five members the unordered first read returned. Every one of the five, and the whole membership, is still in `org_user_ids`, and nobody from the other organization is. - The fake driver gains `staleForOrderedReads`, a set of user ids that its ordered `sys_member` reads (the completion's) do not return. <details><summary>Ablation (head 3c6ddc2)</summary> The union was reverted on disk with `scripts/ablation-replace.mjs`: anchor `[...orgMembersLeg, ...await ` changed to `[...await ` (anchor x1 to x0, replacement x0 to x1, blob c3eddd2 to 93bebcf0). The test imports the resolver from `src/`, so no build is involved. - Mutated: `Tests 1 failed | 8 passed (9)`. The new case is the only red: `expected [ 'u000000', 'u000884', …(998) ] to include 'u000001'`. - Restored: blob back to c3eddd2, equal to HEAD, and `git diff HEAD` is empty. </details> <details><summary>Suites (head 3c6ddc2)</summary> - `pnpm --filter @objectstack/core test`: Test Files 131 passed (131), Tests 2927 passed (2927). - `pnpm --filter @objectstack/core typecheck`: exit 0. - The past-first-read suite plus the batch-equivalence suite: Test Files 2 passed (2), Tests 45 passed (45). </details> ## Verification <details><summary>Gates (head 3c6ddc2)</summary> - `node scripts/pm/dispatch-gates.mjs --commands` derived 61 commands. All were run locally, exit codes were captured before any pipe, and `--ran` reconciled them: 61 derived, 60 run, 1 NOT-MEASURED, 0 UNRUN. - `pnpm check:dual-build-cjs-loads` exited 3 (PREREQUISITE NOT MET, it needs every package built). It is NOT MEASURED locally and left to CI. - `check-plugin-teardown-shape --self-test` passed after `git fetch --unshallow` (its fixture is pinned to an old commit). `check:lean-entry-closure` passed after building `@objectstack/objectql`. - `pnpm lint` exited 0. </details> ## Acceptance notes **Item 1 of the card is held back, as the seat ruled.** The card's suggested per-page check ("each id strictly greater than the row before it") would compare ids in JavaScript code-point order. `driver-sql` emits a plain `ORDER BY id` and `id > ?`, so the database collation decides the order. The keyset itself is sound under any collation, because the cursor and the order use the same comparison. A JavaScript check would not be. Measured on a local PostgreSQL 16, sorting seven nanoid-shaped values: - `C.UTF-8` database: `AC Ab9 B-2 a-c aB3 ab b_1`, which matches JavaScript. - ICU `en-US` database (`CREATE DATABASE ... LOCALE_PROVIDER icu ICU_LOCALE 'en-US'`): `a-c ab aB3 Ab9 AC b_1 B-2`. Member ids are mixed-case (nanoid in the drivers, better-auth's generator through plugin-auth). On such a database, every correctly ordered completion page would fail the check, and every request in an organization past the first read's bound would get a 503. MySQL's default `utf8mb4_0900_ai_ci` collation is case-insensitive and differs from code-point order in the same way (not measured here). Collation-safe designs are left on the card for a decision. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (No spec change.) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; `pnpm lint` passes. - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset. - [x] 6. `packages/spec` touched: n/a. - [x] 7. No hand edits to `content/docs/references/`, `packages/*/CHANGELOG.md` or `content/docs/releases/`. - [x] 8. Title, body and comments are in English. --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbZDZLmVprAaL7DLh1K)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 85f77e9 commit 4b8cbe2

3 files changed

Lines changed: 43 additions & 3 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@objectstack/core': patch
3+
---
4+
5+
fix(core): the fellow-org peer list keeps every member the first membership read returned, even when the completion read sees fewer
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
In an organization of 1,000 members or more, the resolver reads the membership twice: a first read bounded at 1,000 rows, then a completion read in id order. The completion's rows used to replace the first read's, so a completion that saw fewer members than the first read (an older snapshot on another connection) left `org_user_ids` shorter than the first read alone would have, and those members vanished from ordinary members' `sys_user` reads with no signal. The two reads are now unioned. Both are filtered to the active organization, so the union cannot add anyone outside it. No read is added or changed.

‎packages/core/src/security/resolve-authz-context.org-members-past-first-read.test.ts‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,10 @@ const PAGE_ROWS = 10000;
3535

3636
interface Read { object: string; where: Record<string, unknown>; limit?: number; orderBy?: unknown }
3737

38-
function makeQl(tables: Record<string, any[]>, opts: { ignoreCursor?: boolean; failAfter?: number } = {}) {
38+
function makeQl(
39+
tables: Record<string, any[]>,
40+
opts: { ignoreCursor?: boolean; failAfter?: number; staleForOrderedReads?: ReadonlySet<string> } = {},
41+
) {
3942
const reads: Read[] = [];
4043
const holds = (row: any, where: Record<string, unknown>) =>
4144
Object.entries(where).every(([k, v]) => {
@@ -53,8 +56,11 @@ function makeQl(tables: Record<string, any[]>, opts: { ignoreCursor?: boolean; f
5356
if (opts.failAfter !== undefined && object === 'sys_member' && reads.filter((r) => r.object === 'sys_member').length > opts.failAfter) {
5457
throw new Error('connection reset');
5558
}
56-
let rows = (tables[object] ?? []).filter((r) => holds(r, q?.where ?? {}));
5759
const order = Array.isArray(q?.orderBy) ? q.orderBy[0] : undefined;
60+
// An ordered read (the completion's) answering from an older snapshot: the
61+
// members named here had not joined yet as far as it can see.
62+
const stale = order && object === 'sys_member' ? opts.staleForOrderedReads : undefined;
63+
let rows = (tables[object] ?? []).filter((r) => holds(r, q?.where ?? {}) && !stale?.has(r.user_id));
5864
if (order) {
5965
rows = [...rows].sort((a, b) => (a[order.field] < b[order.field] ? -1 : a[order.field] > b[order.field] ? 1 : 0));
6066
if (order.order === 'desc') rows.reverse();
@@ -163,6 +169,23 @@ describe('[#29281] org_user_ids holds the whole active organization', () => {
163169
expect(lines[0]).toContain('org_user_ids');
164170
});
165171

172+
it('a completion that sees fewer members than the first read still yields every member the first read returned', async () => {
173+
vi.spyOn(console, 'warn').mockImplementation(() => undefined);
174+
const tables = tablesOf(FIRST_READ_BOUND + 5);
175+
// Five members the unordered first read returns (it answers in insertion
176+
// order) that the completion's older snapshot does not hold.
177+
const unseen = new Set(['u000001', 'u000002', 'u000003', 'u000004', 'u000005']);
178+
const ql = makeQl(tables, { staleForOrderedReads: unseen });
179+
const grants = await resolveUserAuthzGrants(ql, ME, { tenantId: ORG });
180+
181+
// The completion did run, and did come back short of them.
182+
expect(orgReads(ql.reads)).toHaveLength(2);
183+
for (const u of unseen) expect(grants.org_user_ids).toContain(u);
184+
expect([...grants.org_user_ids].sort()).toEqual(membersOf(tables));
185+
expect(grants.org_user_ids).not.toContain('u_elsewhere_1');
186+
expect(grants.org_user_ids).not.toContain('u_elsewhere_2');
187+
});
188+
166189
it('a page that fails to read fails the resolution loudly — never a shorter list', async () => {
167190
vi.spyOn(console, 'warn').mockImplementation(() => undefined);
168191
const tables = tablesOf(FIRST_READ_BOUND + 5);

‎packages/core/src/security/resolve-authz-context.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1922,9 +1922,16 @@ export async function resolveUserAuthzGrants(
19221922
// membership only when it came back under its bound; at the bound it is a
19231923
// first page and the membership is read to completion here. Under the
19241924
// bound nothing more is read.
1925+
//
1926+
// The completion's rows are UNIONED with Leg 1's, never put in their
1927+
// place: a completion that comes back with fewer members than Leg 1
1928+
// already held (a snapshot older than Leg 1's on another connection) would
1929+
// otherwise shorten the list below what the first read alone answered.
1930+
// The union cannot add a non-member — both reads are filtered to the
1931+
// active organization — so the list is never shorter than Leg 1's.
19251932
if (tenantId) {
19261933
const orgMembers = orgMembersLeg.length >= ORG_MEMBERS_FIRST_READ_ROWS
1927-
? await readOrganizationMembersToCompletion(ql, tenantId)
1934+
? [...orgMembersLeg, ...await readOrganizationMembersToCompletion(ql, tenantId)]
19281935
: orgMembersLeg; // (read in Leg 1 above)
19291936
const ids = new Set<string>(
19301937
orgMembers

0 commit comments

Comments
 (0)