Skip to content

Both territory sharing rules are silently dropped at seed — in [...] conditions are untranslatable, so na_sales_team / eu_sales_team get no access at all #621

Closed
#637
Closed
Both territory sharing rules are silently dropped at seed — in [...] conditions are untranslatable, so na_sales_team / eu_sales_team get no access at all#621
#637
Assignees
os-zhuang
Labels
bugSomething isn't workingmetadataDeclarative metadata — schema, security posture, UI surfacespm:dispatchedDispatched to a dev agent by /pm-dispatchpm:queueReady for the PM dispatch looppriority:p0Critical: blocker, must ship before MVP

Description

@os-zhuang
os-zhuang
opened on Aug 2, 2026

Found while runtime-verifying #616 (booting a fresh install to count hook throws); filed unassigned per Prime Directive #10.

What happens

On every boot, two of the nine declared sharing rules are skipped:

WARN [sharing-rule] skipped (missing or untranslatable CEL condition — never seeded as match-all) [experimental]
  {"rule":"north_america_territory",
   "condition":{"dialect":"cel","source":"record.billing_address.country in [\"US\", \"CA\", \"MX\"]"}}
WARN [sharing-rule] skipped (missing or untranslatable CEL condition — never seeded as match-all) [experimental]
  {"rule":"europe_territory",
   "condition":{"dialect":"cel","source":"record.billing_address.country in [\"UK\", \"DE\", \"FR\", \"IT\", \"ES\"]"}}
INFO [sharing-rule] declared rules seeded into sys_sharing_rule {"seeded":7,"skipped":2,"total":9}

Both come from src/sharing/account.sharing.ts (TerritorySharingRules). The platform is doing the safe thing — it refuses to seed a rule whose condition it cannot translate rather than degrading it to match-all — but the app-side consequence is that na_sales_team and eu_sales_team receive no criteria-based account access whatsoever, while the metadata, the docs and the positions file all say they do.

The sibling rule in the same file seeds fine:

condition: P`record.type == "customer" && record.is_active == true`   // ✅ seeded
condition: P`record.billing_address.country in ["US", "CA", "MX"]`   // ❌ skipped

so the blocker is the in [...] membership operator and/or the nested billing_address.country path on an address-typed field, not CEL conditions in general.

Why it matters

This is a declared-≠-enforced gap of the kind Prime Directive #10 covers: territory sharing is presented as a working capability (it has positions, labels, and documentation) and delivers nothing. It also fails quietly — a WARN in the boot log, seven of nine seeded, no error anywhere a user or an admin would look. The Setup UI will list the two positions with no explanation of why their members see nothing.

Repro

rm -rf .objectstack/data
pnpm dev -- --fresh -p 38617 --seed-admin
grep 'sharing-rule' <log>

Suggested next step

Establish which operators the sharing-rule translator actually supports, then pick one of:

  • A. Rewrite both conditions into the supported subset (e.g. a disjunction of == comparisons) if that translates — cheapest, keeps the capability.
  • B. If nested paths into an address field are the blocker, denormalise a billing_country text field on crm_account and key the rules off that.
  • C. If neither works, remove the two rules and the two positions rather than shipping inert metadata, and file the missing operator support upstream.

Whichever lands, this class deserves a test: a check that every rule declared in src/sharing/ is actually seeded (seeded + 0 skipped) would have caught it on the first boot instead of on a log read months later. Compare test/sharing-coverage.test.ts, which today asserts the declared shape rather than the seeded outcome.

Possibly related, different layer: #549 (sharing coverage gap on related lists).

Generated by Claude Code

Activity

  1. os-zhuang
    added
    bugSomething isn't working
    metadataDeclarative metadata — schema, security posture, UI surfaces
    prio:p0
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    pm:queueReady for the PM dispatch loop
    on Aug 2, 2026
  2. os-zhuang
    self-assigned this
    on Aug 2, 2026

  3. os-zhuang commented on Aug 2, 2026

    @os-zhuang
    os-zhuang
    on Aug 2, 2026 – with Claude
    ContributorAuthor

    认领:PM 循环第 6 轮
    会话:session_019SS7C5SXpniKeCApxgARyf
    分支:claude/issue-621-territory-sharing-conditions
    Worktree:hotcrm-issue-621

    方案 C(删掉两条规则与两个 position)是不可自行决定的:那是移除一项已发货的能力,属于难以撤销的动作。若实测发现 A 与 B 都走不通、只剩 C,停下来返回 needs_decision,把"翻译器到底支持哪些算子"的实测结果一并交回,由维护者决定是删能力还是改用别的表达。不要自己删。

    先做 issue 里 suggested next step 的第一句:摸清 sharing-rule 翻译器实际支持的算子集合,再据此选 A 或 B。目前已知的两个线索——同文件里 record.type == "customer" && record.is_active == true 能翻译、record.billing_address.country in [...] 不能——只能说明"in [...] 或 address 字段的嵌套路径"二者之一(或都)是障碍,这两者要分别验证:如果障碍只是 in,方案 A(改写成 == 的析取)就够了,不需要新增字段;如果障碍是嵌套路径,A 无效,必须走 B。

    src/data/index.ts 本轮归 #613,不要编辑。若你选了 B 且发现它需要改种子(例如给既有客户回填 billing_country),先确认 hook 在种子写入时是否会执行(#617 记录了"种子不跑钩子"这个前提与启动日志矛盾),如果确实需要动种子文件,不要动,在报告里说明,我把它串到下一轮。

    issue 最后那条要求(加一条"src/sharing/ 里声明的每条规则都必须真的被 seed"的测试,即 seeded + 0 skipped)是硬要求。现有的 test/sharing-coverage.test.ts 断言的是声明的形状,而不是 seed 的结果——这正是它漏掉这个缺陷的原因。

    相关但不在本轮范围:#633(9 条 sharing rule 与 9 条 flow 起始条件都没有 has() 守卫)。它和本件落在同一批文件,但要回答的是完全不同的问题(求值失败时 fail-open 还是 fail-close),已排第 7 轮。本轮请勿顺手改守卫——那会让两件事的证据混在一个 PR 里。

    Generated by Claude Code

  4. os-zhuang
    closed this as completedin #637on Aug 2, 2026

  5. os-zhuang commented on Aug 2, 2026

    @os-zhuang
    os-zhuang
    on Aug 2, 2026 – with Claude
    ContributorAuthor

    PM 复核:通过,已入队合并 — #637

    核验依据(以 GitHub 为准):修复 lockfile 后 9 项检查全绿;18 个文件在范围内;未碰 src/data/index.ts(本轮归 #613)、未加 has() 守卫(#633 排第 7 轮)、方案 C 一行未删——三条认领约束全部守住。

    我要求"两个嫌疑分开验证"是这一轮最值钱的一次坚持:issue 的猜测是错的。 实测矩阵显示 in [...] 编译得好好的({billing_country:{$in:[...]}}),真正的障碍是 address 复合值上的嵌套路径不可下推。所以方案 A(改写成 == 的析取)从一开始就不可行——同一条嵌套路径上 == 一样失败。如果按 issue 的猜测直接做 A,会得到一个"改完了还是不生效"的 PR。

    两处设计判断我认为很好:

    • 只读 country 槽位、故意不读 countryCode:后者是 ISO alpha-2,英国是 GB,而 Europe 规则写的是 UK——优先用 ISO 会让英国客户悄无声息掉出区域。本 PR 因此只改变"国家从哪一列读",不改变"谁属于哪个区域"。这是把变更面收窄到最小的正确做法。
    • 投影内联而非抽 helper:钩子体必须能降级成 metadata-only,test/action-sandbox.test.ts 直接把第一版抽 helper 的写法判红了。并且补了三条在真实 QuickJS 沙箱里跑降级后钩子体的测试——直接调 handler 证明不了降级形态还能工作。

    新增的 test/sharing-seeding.test.ts 断言的是播种结果而不是声明形状,并复刻了 seeder 的每一条跳过分支。现有的 sharing-coverage.test.ts 正是因为只断言形状,才会在 9 条规则里 2 条什么都不做时依然全绿。

    lockfile 那条 open question:采纳方案 A(保持生成结果),并且我复核后比报告更放心。 我自己 diff 了一遍:新增/删除的包条目数是 0 ——@objectstack/formula 本来就作为传递依赖在锁里,这次只是根 devDependencies 多了一个引用,没有任何新包进入依赖树,34/33 行全是既有条目的版本串漂移,且都在 package.json 已允许的范围内。手工裁剪会让这个文件无法由它自己的文档化命令重现,那正是这个 gate 要防的漂移。

    第二条 open question(billing_country 是自由文本,填 "United States" 就不属于任何区域)是真问题,我已另开 issue 跟踪,不在本 PR 处理——它需要定义"territory"这个业务概念的取值域,属于产品决定。

    派生的 #638(种子里没有任何客户带 billing_address,所以两条规则虽然装对了、条件也能求值,在演示数据上仍匹配 0 条)已标 P1。

    Generated by Claude Code

  6. yinlianghui
    added 3 commits that reference this issue on Aug 10, 2026

    fix(sharing): key the territory rules off a flat billing_country colu…

    112f3da

    fix(data): give the demo accounts billing addresses so the territory …

    adeadd2

    fix(flows): make record-change conditions total so three automations …

    3dcc301
  7. claude
    added
    priority:p0Critical: blocker, must ship before MVP
    and removed
    prio:p0
    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-dispatchpm:queueReady for the PM dispatch looppriority:p0Critical: blocker, must ship before MVP

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions