Skip to content

Follow-up to #514: remaining behaviour fixes, conventions, and the action-sandbox test gap #575

Closed
Closed
Follow-up to #514: remaining behaviour fixes, conventions, and the action-sandbox test gap#575
Labels
backendServer-side behaviour — hooks, flows, actionsbugSomething isn't workingpriority:p2Medium: important, M3

Description

@yinlianghui
yinlianghui
opened on Jul 31, 2026

延续 #514（已关闭，17 条里约 2/3 已完成）。本 issue 只收剩下的部分，每条都已对照 main（3ed27b26）逐一核实，不再需要去 #514 里考古哪些做过了。

分两类：A 组可以直接开工，B 组需要先定方案。

A 组 — 可直接开工（4 条，彼此文件面不重叠，可并行）

A1. 真实 QuickJS action 执行器（原 #514 项 17，剩余部分中价值最高）

test/helpers/ 已有 flow-harness.ts 和 hook-harness.ts，但 action 侧仍是 test/global-actions.test.ts 里的简易执行器——不是真实 QuickJS 沙箱，所以「body-only、不得读取模块作用域」这类约束测不出来。

具体要守住的一条现存约束（目前只活在注释里）：src/objects/_line-item-price-fill.ts 的共享工厂只在 handler body 从不读取工厂参数时才安全——一旦读取，会在编写期解析、进 QuickJS 后变成 undefined（#570 的作者自己在 PR 里 flag 了这点）。

这条的价值不止于测试覆盖：2026-07-31 当天出现过三次「PR 各自绿、合并后 main 红」，只有一次被测试自动抓住，另外两次（#547 悬空授权、#439 锁文件）靠人工发现。

A2. disqualification_reason 承诺了必填却零强制（原项 4）

src/objects/lead.object.ts:246 的字段 description 写着 "Required when status is Unqualified"，但仓库里没有任何校验或 hook 实现它，也没有写入方。种子数据里有 3 条 unqualified 线索。

仓库内已有现成的实现范式：case.object.ts 用 P 校验实现了同样的「状态为 X 时必填」。

A3. fieldGroups 缺失（原项 13）

crm_campaign 和 crm_task 都是有完整详情页的对象，但 fieldGroups 计数为 0（其余业务对象都有）。两个 line-item 对象没有可以接受。

A4. priority_rank 默认值分歧（原项 14）

crm_case.priority_rank 默认 1，crm_task.priority_rank 默认 2，且 rank 映射表在两个 hook 里各写了一份——未知优先级在两个对象上排序结果不同。注意 test/metadata-references.test.ts 已经钉住了这一带，改前先看。

B 组 — 需要先定方案（4 条，请勿直接派给 agent）

B1. 转换后线索保护：两套实现，保哪套？

  • lead.object.ts:311 的 validation：只锁 4 个身份字段，可恢复错误
  • lead.hook.ts:164 的 beforeUpdate 守卫：白名单之外全锁，抛错

注意：#514 原文说 hook「没有 ctx.user?.id 守卫」——这一条已经不成立，守卫已补上，注释里也写明了两者的分工意图（validation 管身份字段的友好报错，hook 管其余字段的硬拦截）。所以现在是「要不要保留这个有意的双层设计」，而不是「修一个缺陷」。选项：(a) 保留双层并把分工写进文档；(b) 合并成一套。

B2. 两个无写入方的 readonly 字段

  • crm_opportunity.created_date —— 平台已自动注入 created_at，建议直接删；代价是 opportunity.view.ts:149 的 startDateField 要改指 created_at，并清掉 zh-CN 两处翻译
  • crm_case.first_response_date —— 属于 SLA 叙事的一部分。这条是业务决策：服务团队真要这个指标 → 去掉 readonly 并在 case.hook 里接写入方（首次对外活动时打戳）；没人看 → 删

B3. 40 个 changeset 堆积，无人消费

.changeset/ 下有 40 个待发布条目，没有任何 workflow 跑 changeset version/publish。要么定发布流程（打 tag 时跑），要么手工折进 CHANGELOG 并移除机制。改错会影响版本号，建议先跟发布节奏对齐。

B4. state machine 覆盖不全（feature-shaped）

15 个业务对象里只有 3 个（lead / opportunity / case）声明了 type: 'state_machine' 转换表；quote / contract / campaign / task 都有状态字段但没有转换约束。补不补是产品决策，不是缺陷修复。

给接手者的约定（2026-07-31 踩出来的）

  1. 动手前先核实问题仍存在 —— 本 issue 已核实到 3ed27b26，但 main 移动很快
  2. 新守卫写进新建的测试文件，不要追加到 test/metadata-references.test.ts（当天的高频冲突文件）
  3. 提交信息用 Refs，不要用 Closes/Fixes —— 除非该 PR 完整解决本 issue。[CRM] 9 个孤儿 __search 列待清理（17.0 收紧搜索伴生列供给条件后遗留） #528 当天就是被提交信息里的 Closes 误关的，改 PR 描述拦不住
  4. 必须附 changeset，否则 CI 的 Check Changeset 会红
  5. 合并后在 main 上复跑一次 pnpm verify —— 当天三次「合并时绿、落 main 才红」

Activity

  1. yinlianghui
    added
    bugSomething isn't working
    backendServer-side behaviour — hooks, flows, actions
    on Jul 31, 2026

  2. yinlianghui commented on Jul 31, 2026

    @yinlianghui
    yinlianghui
    on Jul 31, 2026
    ContributorAuthor

    B 组方案已定 —— 三条可直接开工，两处待业务确认

    方案基于实测，不是偏好。逐条给结论和证据。

    B1 — 删掉 validation，保留 hook ✅ 可开工

    实测（16.1.0，3ed27b26）：造一条已转换线索，PATCH company →

    400 {"error":"Cannot edit a converted lead (attempted: company). Make changes on the converted records instead."}

    回来的是 hook 的文案。lead.object.ts:311 那条 cannot_edit_converted validation 永远轮不到发言。

    这与 revenue_positive 的实测结果一模一样（当时也是 hook 先抛、validation 是死配置，已在 #571 删除）。代码注释里写的「双层分工」是意图，不是运行时事实——hook 的 beforeUpdate 先抛错，validation 没有机会产生它承诺的「可恢复的友好报错」。

    留着它就是下一个 revenue_positive：两份实现慢慢漂移，其中一份还是死的。

    做法：删除 lead.object.ts 的 cannot_edit_converted validation；hook 的文案已经写得不错（点出了具体字段），保持。补一条守卫测试断言「已转换线索的身份字段不可改，且错误来自 hook」。

    B2 — 拆开处理：一个删，一个补写入方

    crm_opportunity.created_date → 删 ✅ 可开工

    平台已自动注入 created_at，这是重复字段。连带要改：opportunity.view.ts:149 的 startDateField 改指 created_at，清掉 zh-CN.ts 两处翻译。

    crm_case.first_response_date → 保留并补写入方 ⏸ 语义待确认

    不是遗留垃圾。查了 SLA 字段家族的写入情况：

    字段 写入方
    sla_due_date ✅ hook，2 处
    resolution_time_hours ✅ hook，1 处
    is_sla_violated ✅ case_sla_monitor flow
    first_response_date ❌ 无

    四个字段里三个都有写入方，只有它没有——它是一套成体系指标里缺的那一块，而「首次响应时间」是客服领域最标准的 SLA 指标之一。删掉等于砍掉一个真实指标。

    建议实现：先用简单代理——状态首次离开 new 时打戳。比接活动/评论时间线便宜得多，语义也站得住；将来真要精确到「首次对外沟通」再换实现。

    待确认：这个代理语义服务团队是否接受。如果他们要的是「客服第一次回复客户的时刻」，那状态变更不等价（有人可能先改状态再回复）。

    B3 — 保留 changesets，补上消费端 ⏸ 发版节奏待确认

    40 个堆积说明机制只装了一半：CI 已经强制每个 PR 附 changeset（#503），输入一直在收集，却没有任何 workflow 消费它们。

    不建议废除机制——那会把已建立的每 PR 记录纪律一起扔掉，40 条历史记录也会烂在那儿。

    建议：补一个发布 workflow，打 tag 时跑 changeset version（生成 CHANGELOG + 版本号），不跑 publish——这个仓库走 marketplace 分发，root 包是 private: true，不发 npm。

    一次性动作：先跑一次把 40 条折进 CHANGELOG，之后每次发版自动消化。

    待确认：发版节奏和版本号策略。改错会影响版本号，建议先跟领导对齐。

    B4 — 只给 quote 和 contract 加 ✅ 可开工

    判断依据是非法转换有没有业务后果：

    加 — crm_quote（draft → in_review → presented → accepted/rejected）和 crm_contract。报价直接从草稿跳到「已接受」、合同未签署就激活，在 CPQ 和法务上是真实风险。而且查过：这两个对象的 hook 里目前连状态守卫都没有，完全不设防。

    不加 — crm_campaign 和 crm_task。它们的 status 是描述性的（活动在跑/暂停、任务待办/进行中），不是受控生命周期，强加转换表只会给用户添堵。

    所以 #514 项 11 说的「只有 3 个对象声明了 state machine」不是缺陷——缺的只是 quote 和 contract 这两个，其余是合理的。

    派发状态

    • 可直接开工：B1、B2 的 created_date 部分、B4
    • 待业务确认后再动：B2 的 first_response_date 写入语义、B3 的发版节奏

  3. yinlianghui commented on Jul 31, 2026

    @yinlianghui
    yinlianghui
    on Jul 31, 2026
    ContributorAuthor

    更正 B2 的实现方案，并给 B3 定性

    B2 —— 撤回「状态离开 new 时打戳」，改为「首条活动时打戳」

    上一条评论里我建议用「状态首次离开 new」作为 first_response_date 的代理。这个方案是错的，撤回。

    为什么错：客服可以把工单改成「处理中」然后去排查，客户那边一个字都没收到。用状态打戳会让 SLA 显示「已响应」而客户仍在等待——等于把指标造假写进 schema。这正是本仓库近期一直在清理的那类「看着像生效、实际测不到真东西」的缺陷。

    行业标准定义：Salesforce 的 FirstResponseDateTime、Zendesk 的 first reply time，衡量的都是客户第一次收到人工回复的时刻，而非内部状态变更。

    HotCRM 里的落地方式：工单上唯一的对客沟通记录是 log_call / log_meeting，它们写 sys_activity 行（object_name + record_id + metadata.kind），工单详情页的 record:activity 时间线也在读它们。

    因此：在该工单第一条 sys_activity 产生时打戳。实现位置很干净——#569 已把这两个 action 收敛成单一构造器 logActivityAction(spec)（src/actions/global.actions.ts），所以只需在一处添加：若目标工单的 first_response_date 为空，则写入当前时间。

    要在代码注释里写明的约定：将来若为工单增加「回复邮件」等对客沟通路径，该路径也必须打戳，否则指标会漏记。

    这个方案不再需要业务方确认语义——它就是标准定义本身。仍建议向服务团队知会一次口径（「首次响应 = 首次记录的通话/会议」），以便他们理解报表数字的来源。

    B3 —— 不是缺陷，可以无限期推迟

    澄清定性：这不是 bug，没有任何运行时影响，用户无法感知，不会报错。它只关乎版本号与 CHANGELOG 生成。

    不处理的后果仅两条：CHANGELOG 不再反映实际发布内容；40 个文件继续堆积。

    唯一值得记住的成本：CI 强制每个 PR 附 changeset（#503），而没有任何流程消费它们——等于每个贡献者都在做一份被丢弃的功课。所以长期只有两个合理终局：接上消费端（发版时跑 changeset version，不跑 publish——本仓库走 marketplace、root 包 private: true），或取消该强制。维持现状是在持续支付一份没有回报的税。

    建议：下次真正发版前再定，现在不必占用排期。

  4. yinlianghui
    added 2 commits that reference this issue on Jul 31, 2026

    Merge pull request #576 from objectstack-ai/claude/issue-575-a1-quick…

    a4a5ac0

    Merge pull request #578 from objectstack-ai/claude/issue-575-group-b-…

    26d7c8d
  5. os-zhuang
    added a commit that references this issue on Aug 1, 2026

    fix(task): drop the shadowed assignment field group (#582)

    7709114
  6. os-zhuang
    added
    prio:p2
    on Aug 2, 2026

  7. yinlianghui commented on Aug 5, 2026

    @yinlianghui
    yinlianghui
    on Aug 5, 2026 – with Claude
    ContributorAuthor

    关闭核验(PM 派发循环,会话 session_0194QUW67v7vyLToEd9NSeRM,基线 main@5a78f88):

    本 issue 的可开工项已全部落地,逐条对照:

    项 状态 证据
    A1 真实 QuickJS action 执行器 ✅ PR #576(merge a4a5ac0)
    A2 disqualification_reason 强制 / A3 fieldGroups / A4 priority_rank 统一 ✅ PR #577(merge 28e87c9)+ 修正 #582(7709114)
    B1 删 validation 保 hook ✅ PR #578;cannot_edit_converted 已从 lead.object.ts 移除(现仅存说明注释,lead.object.ts:218)
    B2 created_date 删除 ✅ opportunity.object.ts:128 注释确认平台 created_at 替代;view 已改指
    B2 first_response_date 写入方 ✅ 按更正后的「首条活动打戳」方案落地,global.actions.ts:293-323
    B4 quote/contract state machine ✅ 两对象均已声明 state_machine(PR #578)

    B3(40 个 changeset 堆积):按 2026-07-31 评论定性为非缺陷、零运行时影响,「下次真正发版前再定」。不再占用本单;届时随发版流程另立专项。

    按 completed 关闭;如有异议可重开(否决窗口)。

    Generated by Claude Code

  8. yinlianghui
    closed this as completedon Aug 5, 2026
  9. yinlianghui
    added 4 commits that reference this issue on Aug 10, 2026

    test: run action and hook bodies through the real QuickJS sandbox

    252723a

    Enforce lead disqualification reason, group campaign/task fields, uni…

    fa43181

    Retire dead lead/opportunity rules, stamp first response, guard CPQ l…

    f01027a

    docs(administration): list all five objects that have a state machine (…

    6014b2c
  10. claude
    added
    priority:p2Medium: important, M3
    and removed
    prio:p2
    on Sep 9, 2026
  11. os-zhuang
    added a commit that references this issue on Oct 8, 2026

    fix(service): retire crm_case.created_date; every reader moves to the…

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendServer-side behaviour — hooks, flows, actionsbugSomething isn't workingpriority:p2Medium: important, M3

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions