Repository navigation
fix(plugin-sharing)!: a failed business-unit head or standalone manager read refuses instead of answering nobody - #29787
Conversation
…er read refuses instead of answering nobody BusinessUnitGraphService.headOf read sys_business_unit, caught any error to null and cached that null for the pass. The standalone managerOf fallback (no TeamGraphService wired) caught a failed sys_user read to null. Both now read through readMembershipToCompletion, the walk seedIsUsable already uses: a failed read throws, absence of the object answers null, and nothing is cached after a failure. The wired TeamGraphService limb is unchanged. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JQwxbXRtwbd8e9gz1XdRgn
📓 Docs Drift CheckThis PR changes 1 package(s): 11 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 5 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 11 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 5e9a0461ad39c9e3d82aacd1d13c46e2f0622af5 && git checkout 5e9a0461ad39c9e3d82aacd1d13c46e2f0622af5
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 946a173fcbe44a7d6961656a47156322a6ae4b1e f357cc8f8fe25e9f7fc24e622331692b34c005c7 && git checkout -B drift-repro 946a173fcbe44a7d6961656a47156322a6ae4b1e && git merge --no-ff f357cc8f8fe25e9f7fc24e622331692b34c005c7
node scripts/docs-audit/affected-docs.mjs --json 946a173fcbe44a7d6961656a47156322a6ae4b1e
|
|
domain:server#1 · os-warren · session_01JQwxbXsK9j25sd7eVwW96G Contract-review-claim: session_01JQwxbXsK9j25sd7eVwW96G · head f357cc8 · 2026-11-03T04:47Z |
|
domain:server#1 · os-warren · session_01JQwxbXsK9j25sd7eVwW96G Contract reviewServed-tier: Face: ① Derived judgments
② Semver level
Check-runs on ③ Boundary flags
④ Design judgmentThe narrowing read against the published spec on Tried to break it:
Findings (none is a must-fix):
Implemented-by: VERDICT: PASS |
|
domain:server#1 · os-warren · session_01JQwxbXsK9j25sd7eVwW96G 席位核对ACCEPT ·
|
Requested by Warren · project thread
Fixes #29713
domain:server#1 · os-warren · session_01JQwxbXRtwbd8e9gz1XdRgn
Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: not needed. No skill and no
content/docspage namesheadOfor the standalonemanagerOffallback, or says what either answers on a failed read.维护者速读
BusinessUnitGraphService里还剩的两处查询 ——headOf(单元负责人)和未接TeamGraphService时的独立managerOf(用户经理)—— 读表失败时不再答「没有负责人/没有经理」,而是明确报错;headOf失败的结果也不再缓存一整轮。表或对象不存在仍答null。null,headOf还把这个null缓存到本轮结束,读恢复后负责人仍是空的。今天还没有调用方,但第一个按「单元负责人/用户经理」授权或派审批的消费者会在故障期间悄悄漏掉这个人。null变成报错」的是导出类的外部调用方在读失败时,属收窄,标题带!。接了TeamGraphService的managerOf原样转发,未改。回滚即 revert 本 PR。packages/plugins/plugin-sharing/src/business-unit-graph.ts(headOf、managerOf的独立分支)。pnpm --filter @objectstack/plugin-sharing exec vitest run src/business-unit-head-manager-reads.test.ts,预期 11/11 通过(两个方法各自:读失败报错、恢复后答真值、对象缺失答 null;接线分支对照不变)。What changes
BusinessUnitGraphService(packages/plugins/plugin-sharing/src/business-unit-graph.ts): the two lookups #29655 left now read throughreadMembershipToCompletion, the walkseedIsUsableandreadChildUnitsalready use. No new helper.headOf(businessUnitId). A failedsys_business_unitread throws[sharing] the head of business unit 'ID' could not be read: …. It used to answernulland cache thatnullfor the pass. A failed read now throws before anyset, so the next call reads again. A unit that does not exist, sits in another organization, or names no head still answersnulland is cached as before. A missingsys_business_unit(isObjectAbsentError) answersnull.managerOf(userId, organizationId), standalone fallback (noteamGraph). A failedsys_userread throws[sharing] the manager of user 'ID' could not be read: …. It used to answernull. This fallback caches nothing, before or after. A missingsys_useranswersnull. The organization screen (managerIsProvablyOutsideOrg) keeps its ruled fail-open posture (approvals: themanagerapprover resolvessys_user.manager_idwith no organization screen, while every sibling approver expansion is org-scoped #10153) and is unchanged.teamGraph,managerOfstill returnsTeamGraphService.managerOf's answer as it stands.SYSTEM_CTXconstant had no reader left and is removed. The walk reads under team-graph's own system context, as the other reads of this file already did.packages/specchange, noprotocol.ts, no new export.Measured first (real engine, before and after)
The same scratch probe ran on
main(87956dcc) and on the fix. On each, one read failed, then the same instance was called again.mainheadOfon a failed readnullthe head of business unit 'bu_sales' could not be read: SQLITE_IOERR…headOf, same instance, after the read recoversnull(the failed answer was cached)usr_headheadOf,sys_business_unitnot registerednullnullmanagerOfon a failed readnullthe manager of user 'usr_alice' could not be read: SQLITE_IOERR…managerOf, same instance, after recoveryusr_bob(nothing cached)usr_bobmanagerOf,sys_usernot registerednullnullThe card said both lookups cache the failed answer. Only
headOfdoes. The standalonemanagerOffallback has no cache, so its fix is the refusal alone.Callers on
main(traced)SharingRuleService.expandRecipient)expandUnitMembers/expandUsersonlyownership-queue-service.ts)expandUnitMembers/expandUsersonlyApprovalService.resolveApproverSpec)lookupManagerandexpandBusinessUnitUsers; it does not constructBusinessUnitGraphServiceexpandPrincipalmanagerarm (team-graph.ts)TeamGraphService.managerOf, not this classBusinessUnitGraphService.managerOfwired limbTeamGraphService.managerOfheadOfandBusinessUnitGraphService.managerOfhave no production caller in this repository, hotcrm or docs.objectos.ai. The only reach is the exported class (@objectstack/plugin-sharingindex) and theIBusinessUnitGraphServicecontract. No caller fails a whole pass where it used to drop one person.Tests
New file
src/business-unit-head-manager-reads.test.tsruns on a real engine: ObjectQL + SqlDriver over in-memory SQLite, with the shippedSysOrganization,SysBusinessUnit,SysUserandSysMember. 11 tests:headOfmanagerOfnullnullusr_headusr_bobnullnullnullnullCONTROL: with a
TeamGraphServicewired,managerOfanswers exactly whatTeamGraphService.managerOfanswers, on a healthy and on a failed read.pnpm --filter @objectstack/plugin-sharing testat39b2f260: 65 files / 1380 tests passed.pnpm --filter @objectstack/plugin-sharing typecheck: exit 0.headOf/managerOfpins (business-unit-graph.test.ts,sharing-rule.test.ts,manager-org-screen.test.ts) pass unchanged.Ablations: each negative pin's behaviour put back (red), restored by ablation-replace (blob == HEAD, git diff HEAD empty)
The test imports the subject from
src/by relative path, so no build sits between the mutation and the run.A1,
headOfrefusal:.catch(() => null)put back on the readA2,
headOfcache: anullcached for the unit before the readA3, standalone
managerOfrefusal:.catch(() => null)put back on the readThe standalone
managerOfhas no cache, so this diff has no cache line to put back. Its not-cached pin guards against a cache being added, and A3 turns it red at its first leg.Acceptance notes
TeamGraphService.managerOf(team-graph.ts) also catches a failedsys_userread tonulland caches it for the pass. It belongs to the same family, but the order keeps the wired limb unchanged, and its one in-repo consumer,expandPrincipal'smanagerarm, is exported with no in-repo caller. Noted, not filed.readMembershipToCompletion's sentence family. Its closing clause speaks of a holder set, which reads loosely for a single head or manager. Reusing the helper was the order. A wording change belongs to that helper, not here.Verification
All local readings at
f357cc8f(the fix39b2f260plus a merge ofmainatc76c0e04).node scripts/pm/dispatch-gates.mjs --commands: 60 derived families, all run.--ranreconciliation exit 0: "60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN". Two first exited 3 on prerequisites, not on the diff: thecheck-plugin-teardown-shapeself-test on a shallow clone, andcheck:dual-build-cjs-loadsbefore seven unrelated packages had adist/. Both were re-run and passed aftergit fetch --unshallow origin mainand a build of those packages.pnpm lint: exit 0.(narrowing)arm):f357cc8f·os-verify-lock: VERDICT command-exit 0 · held the lock 2046s· Test Files 402 passed, 3 skipped (405) · Tests 3317 passed, 28 skipped (3345) · no failures.Generated by Claude Code