approvals: the `manager` approver resolves `sys_user.manager_id` with no organization screen, while every sibling approver expansion is org-scoped

[baozhoutao]

Observed while implementing the read-only crossing-manager_id audit for cloud#1369 (not fixed there — that card is explicitly read-only and files no writes). Recording it here because a fix would land in plugin-approvals, not in cloud.

What was measured

In packages/plugins/plugin-approvals/src/approval-service.ts, expandApprovers hands the directory organization to every graph-shaped approver expansion:

• team → expandTeamUsers(value, directoryOrg)
• department → expandBusinessUnitUsers(value, directoryOrg)
• position → expandPositionUsers(value, directoryOrg)
• org_membership_level → expandMembershipTierUsers(value, directoryOrg)

The manager branch does not:

} else if (type === 'manager' && record) {
 const subject = (record as any)[a.value] ?? (record as any).owner_id;
 if (subject) {
 const mgr = await this.lookupManager(String(subject));
 if (mgr) return this.applyOooDelegation(mgr, now, organizationId, substitutions);
 }
}

and lookupManager (same file) reads the column directly under a system context, taking no organization argument at all:

private async lookupManager(userId: string): Promise<string | null> {
 const rows = await this.engine.find('sys_user', {
 where: { id: userId }, fields: ['id', 'manager_id'], limit: 1, context: SYSTEM_CTX,
 } as any);
 ...
}

Why it matters

sys_user is a global identity table with no organization_id column, so nothing else in this path supplies the tenancy fact. If a sys_user.manager_id crosses an organization boundary, an approval step with approverType: 'manager' routes the submission to an approver in another organization — an out-of-tenant person granted approval authority over the record.

The hierarchy consumer of the same column has been screened since cloud#1195 (HierarchyScopeResolver.screenToTenant drops users sys_member proves are outside the caller's org). Approvals is a different consumer of the same column and was not covered by that work, so the same row behaves in opposite directions: it silently narrows an own_and_reports owner set and widens approval routing.

Not asserting the fix shape

Two directions exist and they are not equivalent, so this is filed for triage rather than argued:

1. screen lookupManager the way the sibling expansions are screened (refuse / fall through when provablyDisjointOrgs holds), which makes the approval slot resolve to nobody and needs a story for that;
2. treat it as an authoring-time invariant only, on the grounds that cloud#1195's write gate now refuses new crossing links — which leaves rows that predate the gate, the exact residual cloud#1369 exists to report.

Whether approver routing should imply tenancy at all overlaps with the open design question in #7497.

Related

• objectstack-ai/cloud#1369 — the read-only audit that surfaces crossing rows, and names this consumer per row as NOT screened
• objectstack-ai/cloud#1195 — the write gate and the hierarchy-side read screen
• Design: does approver routing imply record read visibility? (#7345 model half) #7497 — Design: does approver routing imply record read visibility?

Verified against 907c11d2cee5fcdd420e2eedf79dc6081d08f5c3 (the SHA cloud pins today).

[os-zhuang]

Triage: lands in packages/plugins/plugin-approvals/src/approval-service.ts ⇒ domain:services + security, type Bug, pm:queue.

Grading rationale: of the two directions the filer declines to argue, direction 1 (screen lookupManager the way every sibling expansion is screened) is the default under the standing meta-rule that when one column has two consumers behaving inconsistently, the governed side wins — the org screen has been the ruled treatment of sys_user.manager_id since cloud#1195, and approvals is an unscreened straggler of that same work, not a deliberately different contract. The "resolves to nobody" story already exists (any approver expansion can come back empty; the existing unresolved-approver path handles it). Direction 2 (authoring-time invariant only) is refuted by the card itself: pre-gate rows are the exact residual cloud#1369 reports.

Scope fence for the dispatch: this restores the invariant for the manager branch only — it does NOT decide #7497 (whether approver routing implies read visibility), which stays an open design card; if implementation shows the screen requires answering #7497 first, stop and report the fork.

---

Generated by Claude Code

[os-warren]

Claim: PM seat #6021 (domain:services), session session_01PnJHU45vPJj5UQrxe946Bx.

• Branch: claude/issue-10153-manager-approver-org-screen
• Worktree: /home/user/objectstack-issue-10153（⛔ 不在共享主检出上编辑；⛔ 禁 git stash）
• File surface: packages/plugins/plugin-approvals/src/approval-service.ts + 其测试 + changeset
• Container & model: claude-opus-5
• Clause-②: no —— 判据写明以便被推翻：本卡收紧的是内部 approver 展开的范围，不是端点的接受/拒绝契约；分诊亦指出"解析不到人"的既有路径本就存在（任何 approver 展开都可能返回空，未解析 approver 的处理路径已在）。
• Serial constraints: 与同批的 Attachment lifecycle bookkeeping ignores updates — a file_id re-point orphans the old sys_file with no tombstone (retention leak) #10171（service-storage）、SettingsService accepts a write before its engine is bound and answers "resolved" while nothing reaches sys_setting — every kernel:ready hook registered from init() is inside that window #10159（SettingsService）面不相交。

⚠️ 重定级绊线（写进派单）：若实现过程中发现该改动会让某个调用者从"被接受"变成"被拒绝"（例如审批创建在无人可解析时由成功转为失败），⇒ 那就是 Clause-② yes，本车道当前没有 claude-fable-5 ⇒ ⛔ 停下回报，不要继续实现。我按内容判为 no，但判据在上面，可被证伪。

⚠️ 范围围栏（分诊原文，我照搬）：本卡只恢复 manager 分支的不变式，不决定 #7497（approver routing 是否蕴含读可见性），那仍是一张开放设计卡。⛔ 若实现显示必须先回答 #7497 才能加这道 org 屏，停下报分叉。

---

Generated by Claude Code