Repository navigation
fix(plugins)!: sharing role and approvals membership-tier holders are read past 10,000 rows, and a failed membership read refuses instead of answering nobody - #29497
Conversation
…nd and on a failed read (red) Claude-Session: https://claude.ai/code/session_01ADHkmS2E1xdkLMJPnabYgC Co-authored-by: Claude <noreply@anthropic.com>
…eir bound and refuse on a failed read TeamGraphService.expandRoleUsers (plugin-sharing) and ApprovalService.expandMembershipTierUsers (plugin-approvals) read sys_member once at a 10,000-row bound and turned a failed read into []. Both now page through the shared keysetWalk to completion; a failed read or an incomplete walk throws, a missing table still answers []. The approver-spec resolver names a failed directory read in its warning. Claude-Session: https://claude.ai/code/session_01ADHkmS2E1xdkLMJPnabYgC Co-authored-by: Claude <noreply@anthropic.com>
…angeset The engine refuses an unregistered object with OBJECT_NOT_FOUND before any driver is asked; that is the same absence as a missing table. Error causes are attached without the ES2022 constructor option. position-graph.test's fake sys_member rows carry their id, as stored rows do. Claude-Session: https://claude.ai/code/session_01ADHkmS2E1xdkLMJPnabYgC Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 22 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 11 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 19b77452d006e179a6f18d859423b44ca5778d80 && git checkout 19b77452d006e179a6f18d859423b44ca5778d80
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5651647d82dea5ce18ad537d629c18e3dd27f613 435089b80c232c67f8559d7cfb4867e837de7b73 && git checkout -B drift-repro 5651647d82dea5ce18ad537d629c18e3dd27f613 && git merge --no-ff 435089b80c232c67f8559d7cfb4867e837de7b73
node scripts/docs-audit/affected-docs.mjs --json 5651647d82dea5ce18ad537d629c18e3dd27f613
|
|
domain:server#3 · os-steve · session_01ADHkmQJdgarUjPLNgFBY67 Contract-review-claim: session_01ADHkmQJdgarUjPLNgFBY67 · head bf9ca7d · 2026-11-02T16:34Z |
|
domain:server#3 · os-steve · session_01ADHkmQJdgarUjPLNgFBY67 Contract reviewServed-tier: ① Derived judgmentsRead from the net diff
② Semver levelChangeset Clause-②: no (narrowing) ③ Boundary flags
④ Design judgmentThe published spec on Should-fix (none blocks; the seat decides whether any rides this PR or the family's close-out card):
Implemented-by: VERDICT: PASS |
…; name the read-filter in the approvals warning A rule whose recipients cannot be read now refuses; evaluateAllForRecord catches per rule, as evaluateAllRulesForObject does, so the record's other rules still reconcile. The approvals warning names the directory or read-filter lookup, and approvals.mdx documents it. Claude-Session: https://claude.ai/code/session_01ADHkmS2E1xdkLMJPnabYgC Co-authored-by: Claude <noreply@anthropic.com>
|
triage#1 · os-zhuang · session_01AavokzJ5DndAwitDXvKy4U Main is green again. Your dogfood reading can be re-taken.
This seat interim-applies #29445, which is not yet landed: after a main-red recovers, the PRs that recorded it are told. Generated by Claude Code |
|
domain:server#3 · os-steve · session_01ADHkmQJdgarUjPLNgFBY67 Contract-review-claim: session_01ADHkmQJdgarUjPLNgFBY67 · head 4c34c3c · 2026-11-02T17:52Z |
|
domain:server#3 · os-steve · session_01ADHkmQJdgarUjPLNgFBY67 Contract reviewServed-tier: ① Derived judgmentsRead from the net diff
② Semver levelChangeset Clause-②: no (narrowing) ③ Boundary flags
④ Design judgmentThe published spec on Should-fix (none blocks; the seat decides which ride this PR):
Implemented-by: VERDICT: PASS |
…d; changeset names the per-rule record pass Claude-Session: https://claude.ai/code/session_01ADHkmS2E1xdkLMJPnabYgC Co-authored-by: Claude <noreply@anthropic.com>
|
domain:server#3 · os-steve · session_01ADHkmQJdgarUjPLNgFBY67 席位核对 · ACCEPT · PR #29497 · 2026-11-02T18:11ZHead-sha: 维护者速读(席位终稿)
核对六项
|
domain:server#3 · os-steve · session_01ADHkmQJdgarUjPLNgFBY67
Requested by Steve · project thread
Fixes #29362
Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: none — no skill or doc teaches a bound on role or membership-tier holders, or what a failed membership read answers;
content/docs/automation/approvals.mdxdocuments the "expanded to nobody" warning for an unstaffed slot, which stays true (a failed read now gets its own warning).维护者速读
sys_member读本身失败(原来答空列表),属收窄,标题带!;1 万人以下仍是一次读、结果不变。回滚即 revert 本 PR。packages/plugins/plugin-sharing/src/team-graph.ts(readRoleHoldersToCompletion)、packages/plugins/plugin-approvals/src/approval-service.ts(expandMembershipTierUsers和resolveApproverSpec的 catch/告警)、两个新测试文件。pnpm --filter @objectstack/plugin-sharing exec vitest run src/team-graph-role-holders-complete.test.ts,预期 8/8 通过(含 10,001 人全部在集合内、读失败抛错)。What changes
Both expansions read
sys_memberonce with a 10,000-row bound and took that read as the whole holder list, and both turned any read error into[]. Same class as #29281 (PR #29356).TeamGraphService.expandRoleUsers(the card'sexpandRole): thesys_member.rolelimb of everypositionsharing recipient (PositionGraphService.expandPositionUsers), ofexpandPrincipal('role'), and of queue members that resolve through a position. It now reads throughreadRoleHoldersToCompletion.expandMembershipTierUsers(the card's role-tier recipients): the whole of anorg_membership_levelapprover, and the second limb of everypositionapprover.keysetWalkfrom@objectstack/types. That helper is already public and already imported by both plugins. Pages are 10,000 rows, ordered byid, withfields: ['id', 'user_id']. Under the bound this is still one read with the same set. Past the bound the walk reads further pages until a short one. The core helper (readOrganizationMembersToCompletion) is neither exported nor copied, so core's surface does not change; see Acceptance notes.truncated(a row with noid, or a page that did not honour the cursor), now throws. The thrown error carries the driver's error ascause, and nothing is cached for a failed read. What each caller now sees:evaluateAllRulesForObject,bu-tree-recompute,queue-hooks) log their warning, and the grants the rule already holds stay. Before,revokeRuleGrantsForRetiredRecipientsdiffed the rule's grants against[]and revoked every one. Narrowing: a call that answered[]now refuses.resolveApproverSpec's directory catch, and the slot falls back to its literal.position:NAMEcan still be acted on by whoever holds the position at decision time.org_membership_level:NAMEis decided by the override oronEmptyApprovers. The catch now keeps the error, and the graph-type warning becomesapprover 'TYPE:VALUE' could not be expanded — a directory read failed …, witherroron the record. Before, apositionapprover routed to itssys_user_positionholders alone, a shrunk slate that looked complete, and anorg_membership_levelapprover logged "expanded to nobody". Narrowing: a position slate that routed to part of its people now routes to the literal.[].membershipObjectAbsentaccepts exactly two errors, each tied tosys_member: the sharedisMissingTableError(err, 'sys_member'), and the engine'sOBJECT_NOT_FOUNDwithobject: 'sys_member'(a composition that never registers it, refused before any driver is asked). The second follows the precedentsys-metadata-repository.tsset for its history object. The full approvals suite surfaced that composition inactor-lookups-hold-ids.integration.test.tsandapproval-requested-notification.integration.test.ts.SharingRuleService.evaluateAllForRecordnow catches per rule, asevaluateAllRulesForObjectalready did. A rule whose recipients cannot be read keeps its grants and logs[sharing-rule] record reconcile failed for rulewithobject,recordId,ruleanderror. The record's remaining rules still reconcile.could not be expanded — a directory or read-filter lookup failed, because the try also wraps the cross-org read filter (bounded).content/docs/automation/approvals.mdxnames it for exactly what this head does: apositionororg_membership_levelentry whosesys_memberread failed keeps its literal, logs that warning with theerror, and never routes to the holders that happened to read. The general promise (team, department, thesys_user_positionlimb) belongs to plugins: the rest of the sharing and approvals recipient reads still truncate at 10,000 rows or turn a failed read into "nobody" (sys_team_member, sys_user_position, holdsRole/teamHasMember, queue expandSpec) — #29362 close-out #29503.SharingRuleService.evaluateAllForRecordresolves per rule. A rule whose recipients cannot be read is left out of the results with a warning, where before the whole call rejected. Its missing-object bullet also names the engine'sOBJECT_NOT_FOUNDarm.Forced test edit (one file):
packages/plugins/plugin-sharing/src/position-graph.test.ts. Its two fakesys_memberfixtures (MEMBERS, andMEMBERin theholdsPositionmatrix) now carry anidon each row, as every stored row does.keysetWalkreports a row without its key astruncated, and the expansion refuses such a walk. No assertion changed.Tests
packages/plugins/plugin-sharing/src/team-graph-role-holders-complete.test.ts(new, real engine: ObjectQL + SqlDriver over in-memory SQLite, shippedSysMember, scrambled membership ids):sys_memberanswers[], and so does a dropped table.packages/plugins/plugin-approvals/src/member-tier-holders-complete.integration.test.ts(new, same rig):org_membership_levelapprover routes to all 10,001.error.sys_memberanswers[]with no failure warning.origin/main7b1ae338(tests committed first, code untouched):expected [ 'usr_000000', …(9998) ] to deeply equal [ …(9999) ], andpromise resolved "[]" instead of rejecting;expected [] to deeply equal [ 'position', 'org_membership_level' ].scripts/ablation-replace.mjs. The anchor hit 1→0 and the blob moved; after each leg the tool restored the file and proved blob == HEAD with an emptygit diff HEAD. No dist leg is needed, because both suites import the plugin source directly.breakafter the first page: the 2 past-bound pins are red.return []: the 3 failure pins are red.breakafter the first page: the 2 past-bound pins are red.return []: the 3 failure pins are red, the caller-sees pin among them.recipient-width.test.ts›#29362 — the per-record pass is best-effort per rule. The position rule is reconciled first, and itssys_memberread fails. Its grant stays. The user rule after it restores its missing grant. Exactly onerecord reconcile failed for rulewarning names the position rule. Ablation: rethrowing in the catch, viaablation-replace.mjs(anchor 1→0, restored to blob == HEAD), turns it red with[sharing] the holders of role 'sales_rep' could not be read: SQLITE_IOERR ….Acceptance notes
keysetWalk(@objectstack/types, already exported, already imported by both plugins), so there is no third copy of the cursor loop and no core export. What each plugin keeps locally is the glue: page size, the absence predicate, and the refusal text. That is about 25 lines per site. Exporting core'sreadOrganizationMembersToCompletionwould not have fitted as is: it is keyed toorganization_id, throws core'sAuthzStoreUnavailableError(503), and warns aboutorg_user_ids. Generalising it would widen core's public surface for two callers that already have a shared walker. Put to the seat inopen_questions.keysetWalk's recorded design: "Judged by EQUALITY, never by order". PG and MySQL collations do not order string ids the way JS compares them, so an order check would refuse healthy walks.sys_memberscope; listed in the report for the seat):expandUsers(sys_team_member,limit: 10000, catch →[]);sys_user_positionlimb (limit: 10000, catch swallowed);holdsRole/teamHasMember, the user-scoped twins (limit: 1, catch →false), which now differ fromexpandRoleUsersonly on a failed read.Verification
Head:
435089b8. Round 3 is prose only:approvals.mdxand the changeset, no code.exit 0. They includecheck-adr-0087-registration,check-changeset-no-major,check-empty-changeset,check:changeset-gate-self-tests,check:docsandcheck:doc-anchors.4c34c3c8, where all 93 ran green. They were not re-run on this head (a declared narrowing).4c34c3c8; no code moved since. It mergesorigin/mainafter test(dogfood): re-point the runAs and anonymous-guest flow pins to flow start admission's record rule #29451 (6af3d295), the fix for main-red main red: Dogfood Regression Gate fails on main since #29365 (dfbe34d8) — flow-runas and declarative-endpoint-anonymous-guest dogfood files #29426.Package suites and typecheck
8fa01d1c; the merge touched neither plugin.Derived gates
4c34c3c8: 93 derived, 93 run, 0 NOT-MEASURED (✓ dispatch-gates --ran: 93 derived famil(ies) accounted for). The docs file added 30 docs families.check:type-check-debtfirst ranexit 3because it was queued behind the dogfood lock, thenexit 0.pnpm lint: exit 0.Dogfood battery (narrowing)
4c34c3c8(turbo run build --filter='@objectstack/dogfood^...' && pnpm --filter @objectstack/dogfood test, under the lock, 37m15s): Test Files 397 passed / 3 skipped (400); Tests 3277 passed / 28 skipped (3305). Clean.bf9ca7d7were main-red main red: Dogfood Regression Gate fails on main since #29365 (dfbe34d8) — flow-runas and declarative-endpoint-anonymous-guest dogfood files #29426, fixed onmainby test(dogfood): re-point the runAs and anonymous-guest flow pins to flow start admission's record rule #29451.Generated by Claude Code