Skip to content

Nobody holds any position on a fresh install, so every position-based sharing rule and both approval flows resolve to an empty recipient set #640

Description

@os-zhuang

Found while fixing #638 (seeding billing addresses so the territory rules match records); filed unassigned per Prime Directive #10, and deliberately not fixed there — it needs demo-user/member design, not seed data.

What happens

#621 made the two territory rules install. #638 gave them accounts to match. The third layer is still empty: no user holds any position, so a matching account still materialises no share.

Measured on a fresh install (pnpm dev, empty .objectstack/data, 17.0.0-rc.1), reading the SQLite file directly:

sys_position          18 rows   (all 12 declared positions exist, incl. na_sales_team / eu_sales_team)
sys_sharing_rule       9 rows   (all declared rules seeded — seeded: 9, skipped: 0)
crm_account            9 rows   (6 match north_america_territory, 2 match europe_territory, 1 neither)
sys_user_position      0 rows   ← nobody holds anything
sys_member             1 row    (the dev admin, role "owner")
sys_record_share       0 rows   ← so nothing is granted to anyone

So the chain is: rules installed ✅ → criteria match real records ✅ → recipients staffed ❌ → grants 0.

Why it matters

This is the last layer of the same declared-vs-delivered gap as #621 / #638, and it is wider than territories:

  • Seven other positions are equally unstaffed (sales_manager, sales_director, executive, service_manager, service_director, marketing_manager, marketing_director), so every position-based sharing rule this app ships grants nobody anything on a fresh install.

  • pnpm lint already warns about the same hole on the approval side, at four nodes:

    approval-approvers-may-resolve-empty — flow "opportunity_approval" · node "manager_review": every approver on this node routes to a group (position/team/department) whose members are runtime data — if none is staffed, the request resolves to an empty slate and waits forever, and (lockRecord) the record stays locked with no in-product recovery.

    So an evaluator who submits a deal for approval on a fresh demo locks the record with nobody able to act on it.

Why it is not a one-liner in demo_bootstrap

The obvious patch — have the bootstrap sweep put the first user into na_sales_team and eu_sales_team — is a fix in appearance only:

  1. On a fresh install there is exactly one user, and demo_bootstrap has already claimed every seeded record for them (owner + platform owner_id, Seeded contracts are ownerless at the platform level (owner_id null) — nobody, admin included, can edit one #622). A share to that same user widens nothing: the OWD baseline already admits the record's owner, and an admin bypasses sharing anyway. sys_record_share would gain rows nobody can observe the effect of.
  2. Demonstrating territory sharing requires a user who does not own the accounts — i.e. at least one non-admin NA rep and one non-admin EU rep, which means creating demo users, deciding who holds which of the twelve positions, and reckoning with the fact that the same decision drives the other seven positions, the four approval nodes and lead_assignment's sales_rep pool (lead.hook.ts already documents its empty-pool fallback).

That is a design decision about the demo org's people, not a seed record, which is why #638 stops at the data.

What is needed

Roughly, and for a maintainer to confirm before anyone builds it:

  1. Whether the demo org should ship synthetic users at all (e.g. na.rep@objectos.ai, eu.rep@objectos.ai, a sales manager, a service agent), given that seeds cannot name a user — creation would have to happen in demo_bootstrap or an equivalent first-boot path, the same constraint that forced that flow to exist (see the note at the foot of src/data/index.ts).
  2. Which positions each demo user holds, so the sharing rules and the approval routes both become demonstrable.
  3. Whether staffing should be dev/demo-only (a real deployment staffs its own people, and shipping fake users into a customer org would be worse than shipping none).

Related: #621 (the rules), #638 (the data they match), #488 (a position no rule and no permission set names grants nothing), #622 (demo_bootstrap and the two ownership columns).

Activity

  1. added
    enhancementNew feature or request
    metadataDeclarative metadata — schema, security posture, UI surfaces
    bugSomething isn't working
    needs-user-decisionNeeds the maintainer's call before work proceeds
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    enhancementNew feature or request
    needs-user-decisionNeeds the maintainer's call before work proceeds
    on Aug 2, 2026
  2. self-assigned this
    on Aug 2, 2026
  3. os-zhuang commented on Aug 2, 2026

    @os-zhuang
    ContributorAuthor

    认领:PM 循环第 9 轮
    会话:session_019SS7C5SXpniKeCApxgARyf
    分支:claude/issue-640-demo-org-staffing
    Worktree:hotcrm-issue-640


    needs-user-decision 已摘掉,按否决窗口推进。 该问的两轮前问过、没收到反对,而且下面这个约束把最要紧的那一半风险直接消掉了,剩下的不再是产品语义分歧:

    决定(可否决):

    1. 发合成用户,但只发到 dev/demo 路径,真实部署一个都不装。 这正是 issue 第 3 条自己点出的顾虑——"把假用户塞进客户 org 比不塞更糟"——而它同时也是解法:只要装配只发生在 demo 引导路径上、真实部署走不到,这条顾虑就不成立,问题也就从产品决策降级成实现细节。这一点是硬约束,不是偏好:任何让合成用户可能出现在客户 org 的实现方式都要打回。
    2. 三个非管理员用户:一个 NA rep(持 na_sales_team)、一个 EU rep(持 eu_sales_team)、一个销售经理(持 sales_manager)。为什么恰好是这三个:前两个让领地共享第一次可被观察(必须是不拥有这些客户的人,否则 OWD 基线本来就放行、什么都证明不了——issue 里已经算过这笔账);第三个让 opportunity_approval 的 manager_review 节点第一次解析出非空审批人,pnpm lint 那条 approval-approvers-may-resolve-empty 才有东西可指。
    3. 另外七个职位这轮不配人。 配人的目的是让机制可被观察,不是把组织架构填满;sales_director / executive 等留空是如实反映"真实部署自己配人"这一事实。但要留下可观察的痕迹:装配逻辑必须是数据驱动的表,加一个人是加一行,不是再写一段代码。

    验收要能看见三层都通了,而不只是 sys_user_position 有了行:

    **文件边界(本轮三件并行,严格分工):**你拥有 src/flows/demo-bootstrap.flow.ts、职位/成员装配所需的新增 seed、src/permissions/。不要动 src/data/sales.seed.ts 的商机行(#593 本轮拥有它)、不要动 src/objects/account.object.ts / account.hook.ts / src/flows/lead-conversion.flow.ts(#626 本轮拥有)、不要动 i18n 语言包(本轮归 #593 独占)。若装配确实需要调整记录归属,在引导流程里做,不要改 seed。

    已知会踩的坑,先说:demo_bootstrap 现在把所有 seed 记录都认领给第一个用户(owner + owner_id,见 #622),它的清扫过滤器对每个对象跑两遍(缺 owner 一遍、缺 owner_id 一遍)。多用户之后"第一个用户"这个概念要重新定义清楚,但归属模型本身是 #548 的范围,不要在这里顺手改 ——本件只负责"人存在且持有职位",不负责重新分配所有权。


    Generated by Claude Code

  4. os-zhuang commented on Aug 2, 2026

    @os-zhuang
    ContributorAuthor

    验收通过 —— #652(9/9 绿,mergeable_state: clean,已入合并队列)。

    #621 → #638 → #640 这条链现在三层都通了,而且是端到端量出来的,不是断言出来的:

    装配前   sys_user_position 0    sys_record_share 0
    装配后   north_america_territory  matched=6  holders=1  granted=6
            europe_territory         matched=2  holders=1  granted=2
            account_team_sharing     matched=5  holders=1  granted=5
            → sys_record_share 13
    
    na.rep  看到 6 条 [CA, US]      eu.rep 看到 2 条 [DE, UK]
    探针 Apex Logistics (SG) 两人都看不到,记录未动
    $310K 商机 → manager_review pending_approvers 非空 = sales.manager
    

    硬约束按要求做成了结构性的,不是靠自觉:装配逻辑是仓库脚本而非元数据,发布产物里根本没有能创建用户的机制;test/demo-staffing.test.ts 会在任何 seed dataset 或 flow 节点写 sys_user / sys_member / sys_user_position 时失败;产物里 grep 三个邮箱和密码均为 0 命中。顺带证伪了"在 demo_bootstrap 里加个 create_record"这条省事路——身份表是 managedBy: 'better-auth'(ADR-0092),绕开那个 surface 插进去的行没有 credential,谁也登不进去,所以那条路不只是危险,是根本产不出可用的人。

    我追的那个机制问题,答案比我预期的更有价值——而且开发推翻了自己 PR 初版的说法。 初版写的是"补上 sales_rep 之后基线就不放行了",这是结论对、机制错。实测 security/explain 的分层是:

    1. member_default 开的是对象门不是行——补上 sales_rep 后这一层写的是 granted by [sales_rep, member_default],两个集合都在,基线没被谁覆盖掉;
    2. 能看到哪些行由下一层定:crm_account 是 private OWD,只放行自己拥有的行,sys_record_share 只能加宽。三个 demo 用户一行都不拥有,所以 NA rep 的可见集合恰好等于领地规则给他的 6 条;
    3. sales_rep 一行都没加宽(viewAllRecords: false, readScope: 'own',与基线同宽)。它加的是能做什么,把"碰巧能看到两条记录的普通成员"变成销售代表。

    所以这个 demo 不是碰巧成立的。但它压在一个能被一次改动悄悄关掉的前提上,而这正是最值得留下的收获:只要哪个绑到 rep 所持职位的 permission set 给了 crm_account 的 viewAllRecords,depth 就拓宽到全表,rep 读到全部 9 条,领地授权什么也证明不了——而 org 看上去仍然"配好人了"。这不是假想:sales_manager 就是这么配的,实测她读到全部 9 条(对经理完全正确,对领地 demo 是静默死亡)。现在有测试钉着这条不变量,负向对照确认会红。

    CodeQL 那条高危按要求在源头修掉:横幅只列邮箱并指向声明处,一次完整运行的 stdout 里密码出现 0 次;没有用抑制注释,也没有为了消告警把 password 从表里删掉——verify() 要靠它以每个 demo 用户身份真登录,那正是三层接通被断言而非假设的地方。

    如实记一条:pnpm lint 的 approval-approvers-may-resolve-empty 仍然会提示。那是纯静态规则,只要审批人全部路由到 group 就会提示。本 PR 修的是运行时那一半。

    衍生发现已归档:hotcrm#653(24 个流程的 kernel:ready 冷启动重绑定全部失败,日志把错误截断成一个孤零零的 [)、objectstack#4704(Seed.env 可声明、有默认值、类型检查通过,但从不被强制——env: ['dev'] 的数据集照样种进生产)。后面这条是本轮最重的发现,我在报告里单独说。


    Generated by Claude Code

  5. added
    priority:p1High: required for production / M2
    and removed on Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingmetadataDeclarative metadata — schema, security posture, UI surfacespm:dispatchedDispatched to a dev agent by /pm-dispatchpriority:p1High: required for production / M2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions