Skip to content

fix(plugins)!: a failed business-unit hierarchy read refuses instead of answering nobody, and a unit keeps every child unit past 1,000 - #29703

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-pakgma
Nov 3, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/project-thread-pakgma

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Warren · project thread

Fixes #29655

domain:server#1 · os-warren · session_01JQwxbXcxCoeCAw1FyTWwwf

Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: updated here. content/docs/automation/approvals.mdx now says a department entry also keeps its literal and logs the failed-read line when the sys_business_unit read of the unit or of its child units fails. No skill teaches what a failed business-unit hierarchy read answers.

维护者速读

  • 改了什么:共享规则与审批的「业务单元」展开里,读 sys_business_unit 本身(种子单元是否存在且启用、逐层读下级单元)失败时不再当作「没人」,而是明确报错,且失败结果不进本轮缓存;下级单元的读改为按 id 分页读完(每页 1,000),超过 1,000 个下级单元不再被截断。
  • 为什么改:上一单(成员表)修完后,单元表这条路仍会把读失败吞成「没人」并缓存一整轮:业务单元结构一变,重算就把 business_unit / unit_and_subordinates 规则的授权全部收回;审批则记成「expanded to nobody」或悄悄派给缩水的名单。
  • 风险与回滚:唯一「从有答案变成报错」的情形是 sys_business_unit 的读本身失败(原来答空),属收窄,标题带 !;表或对象不存在仍答无人;1,000 个下级以内仍是一次读、结果不变。队列文件未改动,其失败姿态(记一次日志、贡献 0 人)照旧。回滚即 revert 本 PR。
  • 先看:packages/plugins/plugin-sharing/src/business-unit-graph.ts(seedIsUsable、readChildUnits)、packages/plugins/plugin-approvals/src/approval-service.ts(expandBusinessUnitUsers、readRowsToCompletion)。
  • 两分钟验证:pnpm --filter @objectstack/plugin-sharing exec vitest run src/business-unit-hierarchy-reads-complete.test.ts,预期 9/9 通过(种子读失败报错且不缓存、下级读失败报错且不缓存、1,001 个下级单元全部在内、对象缺失仍答无人)。
  • 需要你做的:无,席位落地。

What changes

The sys_business_unit reads behind the business-unit expansions now answer every unit or refuse, the shape the member reads (sys_business_unit_member) got in #29647, through the same walk.

  • plugin-sharing, BusinessUnitGraphService
    • seedIsUsable reads the seed row through readMembershipToCompletion (the one keysetWalk the member reads share). A failed read throws; it used to answer false, which both widths cached as [] for the pass. A missing sys_business_unit (isObjectAbsentError) still answers false.
    • descendants reads each parent's child units through the new private readChildUnits, the same walk in pages of 1,000 (CHILD_UNIT_PAGE_ROWS, the bound the read always carried). A failed read throws before anything is cached; it used to answer [] for that parent. A parent with more than 1,000 active children keeps every child.
    • The per-pass cache: an absent or inactive seed is still cached as []. A failed read throws before any set, so the next call on the same pass reads again.
  • plugin-approvals, ApprovalService.expandBusinessUnitUsers
    • The seed check (businessUnitIsLive, unchanged, already throws) is no longer wrapped in a catch-all: absence answers [], any other failure refuses with [approvals] business unit 'ID' in organization 'ORG' could not be read: ….
    • The child-unit descent reads through the new private readRowsToCompletion, factored out of readHoldersToCompletion (which now calls it), in pages of 1,000. The refusal sentence is the existing one, now in one helper holderReadRefusal.
    • The caller is unchanged: resolveApproverSpec's directory catch turns the refusal into the literal slot plus the could not be expanded — a directory or read-filter lookup failed warning, as for a failed member read.
  • No new export from any package index, no packages/spec change, ownership-queue-service.ts untouched (its expandSpec catch already turns a refused expansion into nobody plus one warning).

Tests

Two new real-engine files (ObjectQL + SqlDriver over in-memory SQLite, shipped SysBusinessUnit / SysBusinessUnitMember), the pins the card asks for per site:

Pin sharing (business-unit-hierarchy-reads-complete.test.ts) approvals (business-unit-hierarchy-complete.integration.test.ts)
failed seed read refuses on expandUnitMembers, expandUsers, descendants; not cached refuses; the slot is the literal with the failed-read warning
failed descent read refuses on descendants and expandUsers; not cached; the single-unit width is untouched refuses; literal plus warning, not a shrunk slate
1,001 child units all 1,002 units and every member; 2 child reads (control: 999 children, 1 read) every member on the slate; 2 child reads (control: 1 read)
absence unregistered object and dropped table both answer [] unregistered object answers [] with no failure warning; dropped table answers []
  • pnpm --filter @objectstack/plugin-sharing --filter @objectstack/plugin-approvals test at 483976d8: plugin-sharing 64 files / 1369 tests passed, plugin-approvals 111 files / 1442 tests passed.
  • pnpm --filter @objectstack/plugin-sharing --filter @objectstack/plugin-approvals typecheck: exit 0.
Ablations: each negative pin with its catch put back (red), restored by ablation-replace (blob == HEAD, git diff HEAD empty)

The tests import the subject from src/ by relative path, so no build sits between the mutation and the run.

A1, sharing seed: seedIsUsable wrapped back in try … catch { return false; }

ablation-replace: anchor   x1 -> x0
ablation-replace: blob     a27bf6e27fab2fd11e1799c51ff2a94a7e17a9d2 -> 7cb3ff7b49566876bfe98e5c3fc4241a69bf4cd2
     × ⛔ both widths and the descent refuse — none answers an invented empty set 170ms
     × ⛔ a failed seed read is not cached: the next read on the same pass answers the real set 25ms
AssertionError: promise resolved "[]" instead of rejecting
AssertionError: promise resolved "[]" instead of rejecting
      Tests  2 failed | 7 passed (9)
ablation-replace: ok restored: blob == HEAD (a27bf6e27fab) and `git diff HEAD` is empty

A2, sharing descent: readChildUnits wrapped back in try … catch { return []; }

ablation-replace: anchor   x1 -> x0
ablation-replace: blob     a27bf6e27fab2fd11e1799c51ff2a94a7e17a9d2 -> a6e5df6cd8631cd961094f92c51ed1647811455e
     × ⛔ the subtree width and the descent refuse — the subtree never shrinks to the seed 20ms
     × ⛔ a failed descent read is not cached: the next read on the same pass answers the whole subtree 18ms
AssertionError: promise resolved "[ 'bu_root' ]" instead of rejecting
AssertionError: promise resolved "[ 'bu_root' ]" instead of rejecting
      Tests  2 failed | 7 passed (9)
ablation-replace: ok restored: blob == HEAD (a27bf6e27fab) and `git diff HEAD` is empty

A3, approvals seed: the refusal replaced by return [];

ablation-replace: anchor   x1 -> x0
ablation-replace: blob     4f51e3768113dbde75ea2e4e63a8c57aa28e6517 -> c3abb7bdd78284efca1f23c01ffb68705f92735e
     × ⛔ the expansion refuses — it never answers an invented empty slate 208ms
     × ⛔ what the caller sees: the department slot falls back to its literal, and the warning names the failed read 30ms
AssertionError: promise resolved "[]" instead of rejecting
AssertionError: expected [] to deeply equal [ 'department' ]
      Tests  2 failed | 6 passed (8)
ablation-replace: ok restored: blob == HEAD (4f51e3768113) and `git diff HEAD` is empty

A4, approvals descent: .catch(() => []) put back on the child-unit read

ablation-replace: anchor   x1 -> x0
ablation-replace: blob     4f51e3768113dbde75ea2e4e63a8c57aa28e6517 -> a3210bc8c8e5dbe0a645f1aac84b45ed25c90491
     × ⛔ the expansion refuses — the slate never shrinks to the seed unit 44ms
     × ⛔ what the caller sees: the literal and the failed-read warning, not a shrunk slate 23ms
AssertionError: promise resolved "[ 'usr_of_bu_root' ]" instead of rejecting
AssertionError: expected [ 'usr_of_bu_root' ] to deeply equal [ 'department:bu_root' ]
      Tests  2 failed | 6 passed (8)
ablation-replace: ok restored: blob == HEAD (4f51e3768113) and `git diff HEAD` is empty

Acceptance notes

Verification

  • node scripts/pm/dispatch-gates.mjs --commands at 483976d8: 92 derived families, all run; --ran reconciliation exit 0, "92 run, 0 NOT-MEASURED". Four first exited 3 on prerequisites (teardown-shape self-test on a shallow clone; skill-examples, dual-build-cjs-loads and i18n before a package build) and were re-run green after git fetch --unshallow and turbo run build --filter='./packages/**'.
  • pnpm lint: exit 0.
  • Dogfood battery: 483976d8 · os-verify-lock: VERDICT command-exit 0 · held the lock 1498s · Test Files 399 passed, 3 skipped (402) · Tests 3297 passed, 28 skipped (3325) · no failures.

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic. (No spec change.)
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes.
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: not touched.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English.

Generated by Claude Code

@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-approvals, @objectstack/plugin-sharing, touching 11 documentable anchor(s).

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 2ea604223a4136eb50af81b7b70669f09fb89f93.

⛔ 6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 101 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 13 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 2ea604223a4136eb50af81b7b70669f09fb89f93 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 3fe28222a1e1655f7f1591be07e1563da30917a4 — the merge of head 483976d87fcab886af74a7c8d7d5966ead76d63f into base 2ea604223a4136eb50af81b7b70669f09fb89f93, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 3fe28222a1e1655f7f1591be07e1563da30917a4 && git checkout 3fe28222a1e1655f7f1591be07e1563da30917a4
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2ea604223a4136eb50af81b7b70669f09fb89f93 483976d87fcab886af74a7c8d7d5966ead76d63f && git checkout -B drift-repro 2ea604223a4136eb50af81b7b70669f09fb89f93 && git merge --no-ff 483976d87fcab886af74a7c8d7d5966ead76d63f

node scripts/docs-audit/affected-docs.mjs --json 2ea604223a4136eb50af81b7b70669f09fb89f93

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 2ea604223a4136eb50af81b7b70669f09fb89f93 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#1 · os-warren · session_01JQwxbXsK9j25sd7eVwW96G

Contract-review-claim: session_01JQwxbXsK9j25sd7eVwW96G · head 483976d · 2026-11-03T02:14Z

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#1 · os-warren · session_01JQwxbXsK9j25sd7eVwW96G

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 483976d87fcab886af74a7c8d7d5966ead76d63f
Local-runs: none
Scope: packages/plugins/plugin-approvals/src/approval-service.ts, packages/plugins/plugin-approvals/src/business-unit-hierarchy-complete.integration.test.ts, packages/plugins/plugin-sharing/src/business-unit-graph.ts, packages/plugins/plugin-sharing/src/business-unit-hierarchy-reads-complete.test.ts + packages/plugins/plugin-sharing/src/team-graph.ts (readMembershipToCompletion, holderReadRefusal), packages/types/src/keyset-walk.ts (keysetWalk, withCursor), packages/types/src/object-absence.ts (isObjectAbsentError), packages/plugins/plugin-sharing/src/sharing-rule-service.ts (expandRecipient, revokeRuleGrantsForRetiredRecipients, evaluateAllForRecord, evaluateAllRulesForObject, ruleReachesUser), packages/plugins/plugin-sharing/src/bu-tree-recompute.ts (onGraphWrite, queueRegrant), packages/plugins/plugin-sharing/src/recipient-membership-recompute.ts, packages/plugins/plugin-sharing/src/ownership-queue-service.ts (expandSpec), packages/plugins/plugin-sharing/src/business-unit-graph.test.ts (the hand-rolled engine the unit reads now walk through), the serial predecessor PR #29647 at e0187d0f (same two source files, its changeset), and the .changeset/29655-business-unit-hierarchy-reads-refuse.md declaration lines read under ②.

Face: Clause-②: no (narrowing) on the PR body and the changeset; --template --face narrowing. Reviewed read-only from the diff against main at 2ea60422, the card (body plus comments 6592460295, 6592611581, 6593642509), the head's check-runs, and the cited files at the head (git show 483976d8:PATH, REST contents?ref=). Nothing built, run or re-run.

① Derived judgments

Check-runs on the head at review time (three polls, 02:15Z to 02:22Z): 49 runs at the last poll, 32 success, 12 skipped, 5 in_progress (Test Core shards 2, 3, 4, 6, 8), 0 failures. Green: Lint · ESLint, Type Check · source gates, Type Check · consumer gates, Type Check · workspace, Check Changeset, Governed Surface Queue Guard, PR-body guards, Check PR Size, Spec property liveness, Flag docs affected by code changes, Dogfood Regression Gate, Lint · PM dispatch-gates, Build Core, Build Docs, Test Core shards 1, 5 and 7. The Test Core shards had not concluded, so the test-file pins below are judged from the code and the rig, not from a gate. The seat should read them green before landing.

Accept-set and public-surface changes the diff implies, each judged:

  1. Sharing BusinessUnitGraphService.seedIsUsable: a failed sys_business_unit read throws instead of answering false. Right. The read now goes through readMembershipToCompletion (team-graph.ts:183), whose catch returns null only for isObjectAbsentError(err, 'sys_business_unit') and otherwise throws holderReadRefusal ([sharing] business unit 'ID' could not be read: REASON. A shortened or empty holder set would withhold or revoke …). seedRows?.[0] on null is undefined, so absence still answers false. Both callers (descendants:98, expandUnitMembers:198) await it before their cache.set, so a thrown read sets nothing. The one-row-by-id lookup through a 10,000-row keyset page is correct: withCursor is never reached (the first page has no cursor), one row is shorter than want, the walk returns after one find.
  2. Sharing descendants: child units are read through the new private readChildUnits in pages of 1,000 (CHILD_UNIT_PAGE_ROWS) to completion; a failed read throws before cache.descendants.set. Right. readMembershipToCompletion with fields: [] projects ['id']; keysetWalk has no max, so a parent with more than 1,000 active children is read in further pages, each AND-ed with { id: { $gt: cursor } } through $and (keyset-walk.ts withCursor), which composes with the caller's parent_business_unit_id / active: { $ne: false } / organization_id predicate rather than overwriting any of them. A non-advancing cursor sets truncated and readMembershipToCompletion throws WALK_INCOMPLETE, so the subtree never silently shortens. Absence returns null → [] for that parent. The loop is the same breadth-first seen / queue, so the descent's shape and dedupe are unchanged.
  3. Sharing: nothing is cached after a failed read. Right, for all three maps touched by this diff: descendants sets only after the loop completes (:116) or after a non-failed unusable seed (:100); expandUnitMembers sets only after seedIsUsable returned and readMembers returned (:212); expandUsers sets only after descendants and readMembers returned (:236). The absent-or-inactive seed is still cached as [], which is the card's own stipulation (absence is an answer, not a failure). The cache.head map (headOf) is untouched by the diff, see ③.
  4. Approvals expandBusinessUnitUsers: the seed catch-all becomes absence-only. Right. businessUnitIsLive (:3293) is unchanged and already throws; the new catch returns [] on isObjectAbsentError(err, 'sys_business_unit') and otherwise throws holderReadRefusal('business unit ID in organization ORG', err), the driver error on cause. An inactive or missing unit still answers [] (if (!live) return []).
  5. Approvals: the descent reads through the new private readRowsToCompletion in pages of 1,000. Right. readRowsToCompletion is readHoldersToCompletion's walk with the projection and page size lifted to parameters and the row collection returned instead of the user_id set; readHoldersToCompletion now calls it with ['user_id'] and HOLDER_PAGE_ROWS and maps null to [], so the member read of 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 answers exactly as before. The descent passes fields: [] → ['id'], CHILD_UNIT_PAGE_ROWS, and the existing businessUnitOrgScope predicate as the walk's where. where (not filter) is the key businessUnitIsLive and the 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 member read already pass to the same engine.find, so the switch from the old filter: spelling is safe. kids ?? [] keeps absence as "no children".
  6. One refusal sentence, holderReadRefusal(scope, err). Right: the string is the one readHoldersToCompletion carried since plugins: sharing expandRole and approvals role-tier recipients read sys_member at a 10,000-row bound and turn a failed read into an empty list, so a large or unreadable membership silently shrinks the set #29362, moved to a module function and reused by the seed catch and the walk; no wording changes, cause preserved.
  7. Public surface. No export is added to or removed from any package index; packages/spec is untouched; readChildUnits, readRowsToCompletion and holderReadRefusal (approvals) are private / module-local; CHILD_UNIT_PAGE_ROWS is a module constant in each plugin. The only observable change of kind is on the failure arm: a failed sys_business_unit read, which answered [] / false, now rejects. That is a narrowing of an answer the spec never declared, not a declared capability removed at a consumer.

The brief's four questions: a failed unit-table read (seed check or child-unit descent) can no longer answer "nobody" at either site; nothing is cached after a failed read at either site; absence (isObjectAbsentError on sys_business_unit) still answers [] / false at both sites, both for an unregistered object and for a dropped table (the test rigs pin both forms); descents read to completion past 1,000 rows at both sites through the shared keysetWalk with no max, and the two test files count 2 driver reads for 1,001 children against 1 for 999.

Tests, judged from the rig. Both new files run a real ObjectQL + SqlDriver over in-memory SQLite with the shipped SysBusinessUnit / SysBusinessUnitMember and scramble unit ids against creation order, so the 1,001-child pin exercises the seek past the last id ($gt on the cursor), not insertion order. The fault is injected at the driver's find for sys_business_unit and keyed on whether the query carries parent_business_unit_id, so the seed and descent faults are distinct and the member reads run real. The sharing rig's failUnitReads(which, times) takes the driver's real find once at boot, so the not-cached pins (fail once, then read again on the same instance) wrap the real method rather than the previous spy. The read counter matches select … from sys_business_unit … parent_business_unit_id with ROOT among the bindings, so it counts only the descent's reads of ROOT's children. The approvals rig asserts the caller's view through resolveApproverSpec (the literal department:bu_root plus one could not be expanded — a directory or read-filter lookup failed warning whose error names SQLITE_IOERR), and that absence of the object leaves no failure warning. The four ablations in the PR body each put one catch back and turn exactly the two pins for that site red; the record does not re-run them.

② Semver level

.changeset/29655-business-unit-hierarchy-reads-refuse.md declares @objectstack/plugin-sharing: patch and @objectstack/plugin-approvals: patch, carries Clause-②: no (narrowing) and Metadata-protocol: none, an adr-0087: not-required (no-migration-prescription) marker, and a **BREAKING** only where a sys_business_unit read fails paragraph under a fix(plugins)!: subject. The PR body carries the same two declaration lines. The diff publishes no new or removed export, no schema key, no metadata change; the only consumer-visible change is the failure arm's kind. patch with the ! marker and the (narrowing) arm is exactly the level and the lines the serial predecessor #29647 (e0187d0f, .changeset/29606-business-unit-member-reads-complete.md) landed for the same two packages on the same shape, and Check Changeset is green on the head. The changeset's four claims about callers are each true in the cited code: sharing's rule pass fails and its grants stay (④, bullets 1 to 3); approvals' slot falls back to its literal with the failed-read warning (resolveApproverSpec catch at approval-service.ts:2631, resolveExpressionApprovers catch at :2898 via warnExpansionFailed); a queue BU spec contributes nobody and logs once (OwnershipQueueService.expandSpec .catch → warnOnce → [], ownership-queue-service.ts:309); a missing object is still nobody. The semver level and the Clause-②: line match the diff.

③ Boundary flags

From the os-dev-report (comment 6593642509), open_questions: [], security_findings: none, and two out_of_scope_findings:

  1. BusinessUnitGraphService.headOf still turns a failed sys_business_unit read into null and caches it in cache.head for the pass (business-unit-graph.ts:264-282, untouched by the diff). Judged: a follow-up card, not this PR's scope, and not a FAIL. The card's "Wanted" names the seed check and the child-unit descent, the two reads that feed a recipient set; headOf names one manager_user_id. On this tree it has no production consumer: grep -rn "\.headOf(" across packages/ (test files excluded) finds only the IBusinessUnitGraphService contract in packages/spec/src/contracts/sharing-service.ts and the dogfood authz matrix. No grant is computed or revoked from it. It is still the same family (a swallowed read cached as an answer for the pass), so it should be filed as its own p3 card against business-unit-graph.ts (headOf → refuse on failure, absence → null, nothing cached after a failed read), with managerOf's standalone-fallback catch (:314, sys_user read → null) noted alongside since it sits in the same file. The seat files it; the dev's "noted, not filed" is answered here by escalation, not by leaving it in the PR body alone.
  2. Deviations: content/docs/automation/approvals.mdx touched outside the claim's file surface (one sentence beside 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's member-read sentence) — a prose face, the seat's; its content agrees with the approvals change under ① bullet 4 (a department entry keeps its literal and logs the failed-read line when the unit or child-unit read fails). Commit messages without a card trailer and WIP subjects squashed by the PR title — the PR body carries Fixes #29655 and the seat line, so the merge commit will. No action.

No other flag. ownership-queue-service.ts is untouched as the claim required (#29604's). The check-runs still in progress at review time are named under ①; this record judges the code and leaves the seat to read the shards green before landing.

④ Design judgment

Each in-scope file against the code it cites and the scenario the card accepts it against (a sys_business_unit read fails during a business-unit graph write; 1,001 child units; absence), with the attempts to break it:

packages/plugins/plugin-sharing/src/business-unit-graph.ts — holds.

packages/plugins/plugin-approvals/src/approval-service.ts — holds.

  • Callers that relied on the old swallow. resolveApproverSpec (:2613) and resolveExpressionApprovers (:2884) both wrap the expansion in the directory catch (:2631 → directoryFailure, :2898 → warnExpansionFailed), so an approval step degrades to the literal slot plus the warning, never to a crash and never to a shrunk slate. decisionOutputValueRefusal (:4751) calls businessUnitIsLive directly under its own catch → undefined, unchanged. No other caller of expandBusinessUnitUsers exists.
  • Error propagation. The seed refusal wraps the driver error with cause; the descent refusal comes from readRowsToCompletion's catch with the walk's scope sentence (the child units of business unit 'P' in organization 'O'), so the warning names which unit's read failed. A mid-walk failure on a second page throws with nothing returned (the rows array is local). walk.truncated throws the existing incomplete-walk sentence.
  • Factoring. readHoldersToCompletion is now a thin projection over readRowsToCompletion; null → [] keeps the 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 absence answer for every holder read (sys_team_member, sys_user_position, sys_member, sys_business_unit_member), and the user_id dedupe is unchanged. holderReadRefusal moves the sentence to one place without changing it.

packages/plugins/plugin-sharing/src/business-unit-hierarchy-reads-complete.test.ts — holds against the card's four pins per site: failed seed read refuses on all three entry points and is not cached (fail once, next call answers the real set, for both widths); failed descent read refuses on descendants and expandUsers, is not cached, and leaves expandUnitMembers untouched (a control that proves the fault is keyed to the descent); 1,001 children → every unit and every member, 2 reads, against 999 → 1 read; absence by unregistered object and by dropped table answers [] on all three. Real engine, real seek.

packages/plugins/plugin-approvals/src/business-unit-hierarchy-complete.integration.test.ts — holds against the same four pins plus the caller's view: refusal text per site, literal slot plus one department failure warning carrying SQLITE_IOERR, 1,001 children → every member on the slate with 2 reads (control 1), absence → [] and the literal with no failure warning.

Must-fix: none.
Should-fix: none in this tree. The headOf / managerOf swallow is escalated under ③ as a follow-up card, not a change to this PR.

Implemented-by: claude/project-thread-pakgma
Reviewed-by: subagent of session_01JQwxbXsK9j25sd7eVwW96G

VERDICT: PASS

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#1 · os-warren · session_01JQwxbXsK9j25sd7eVwW96G

席位核对

ACCEPT · domain:server#1 · head 483976d87fcab886af74a7c8d7d5966ead76d63f · 2026-11-03T02:33Z · contract review PASS 6593863224 at CONTRACT_REVIEW_TIER (narrowing face, fable subagent, independent of the dev); the prose remainder (changeset, content/docs/automation/approvals.mdx) read by the seat at TIER_DEFAULT

  1. Shape: draft, base main, Fixes #29655. 6 files, +597/-59: plugin-sharing/src/business-unit-graph.ts (seedIsUsable and the new readChildUnits read through readMembershipToCompletion), plugin-approvals/src/approval-service.ts (expandBusinessUnitUsers: the seed catch now covers absence only, the descent runs through the new private readRowsToCompletion, factored from readHoldersToCompletion), two new pin files, and one changeset (patch ×2, Clause-②: no (narrowing), Metadata-protocol: none, the same lines as the predecessor 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). Not governed, under 3000 lines.
  2. Premise measured: a unit-table read failure answered "nobody" and was cached for the pass. The review read bu-tree-recompute.ts: the revoke half awaits expandRecipient before any delete, so after this PR an outage grants nothing and revokes nothing. Before, it deleted every grant. Four ablations went red (report 6593642509). The dogfood battery gave VERDICT 0 with 399 files passing.
  3. Gates on this head: check-runs pinned to 483976d8: 50 total, 38 success, 12 skipped, 0 failed. Dev local: dispatch-gates --ran 92/92, pnpm lint 0, typecheck 0.
  4. Accepted deviations: one sentence in content/docs/automation/approvals.mdx, outside the claim's surface. A narrowing that changes what an approval step sees on an outage owes its docs page, so the seat keeps it (Skills-docs: updated here). Seat-read sentence: the existing "a failed member read keeps its literal and logs" sentence now also names the sys_business_unit read of the unit and its child units for a department. That matches the tree: the review's ④ traced resolveApproverSpec catching the refusal into the literal slot plus the warning.
  5. Out of scope: headOf and the standalone managerOf fallback still swallow a failed read to null (dev note, review ③): filed plugin-sharing: a failed business-unit head or standalone manager read still answers null and caches it for the pass (headOf, managerOf fallback) #29713. Review nits (the 10,000-row page size on a one-row seed lookup, and the sharing refusal subject not naming the organization): Acceptance notes, no card.
  6. Landing: not governed and under 3000 lines ⇒ the seat lands it (pr_ready + automerge_enable). At MERGED: landing record on plugins: a failed sys_business_unit hierarchy read still answers nobody (and sharing caches it for the pass), so a business-unit graph write can revoke a rule's grants on an outage #29655 (closed by Fixes), pm:dispatched removed; plugin-sharing: a failed business-unit head or standalone manager read still answers null and caches it for the pass (headOf, managerOf fallback) #29713 becomes dispatchable after this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

1 participant