Skip to content

批量写(multi: true)路径上 ctx.previous 恒为空 —— 15 个读 previous 的 hook 在该路径全部空转,知识文章可被批量写成「已发布但从未复核」 #779

Description

@yinlianghui

发现于 #769(PR #776)的实测过程。#769 是 view label 与 filter 一致性问题,这一条是跨对象的 hook 语义缺陷,越界,单独记录。

事实(pinned 17.0.0-rc.2,真 object schema + 真 hook + sys_fetch_previous_update 内建的忠实复刻)

引擎内建 sys_fetch_previous_update(priority: 5,beforeUpdate,object: '*',由 kernel 服务的 registerAuditHooks() 装载)是 ctx.previous 在 beforeUpdate 上的唯一来源 —— 引擎主体只在 afterUpdate 分支里赋 hookContext.previous。而这个内建的第一行是:

if (hookCtx.input?.id && !hookCtx.previous) { … }

multi: true 的批量更新没有 input.id(目标由 where 圈定),所以该内建取不到任何东西,ctx.previous 保持 undefined。用一个 priority 299 的探针 hook 观测(它排在 knowledge_article_publish_timestamps 的 300 之前):

=== single-row update (category only) ===
  [prev-fetch] input.id="crm_knowledge_article-…-1" previous=undefined
  [knowledge-hook sees] previous.status="published" input={"id":"…","category":"api"}
  => last_reviewed_at refreshed: true

=== MULTI update (category only, every published row) ===
  [prev-fetch] input.id=undefined previous=undefined
  [knowledge-hook sees] previous.status=undefined input={"category":"troubleshooting"}
  => B.category=troubleshooting  last_reviewed_at refreshed: false

=== MULTI update writing last_reviewed_at: null ===
  [prev-fetch] input.id=undefined previous=undefined
  [knowledge-hook sees] previous.status=undefined input={"last_reviewed_at":null}
  => A: last_reviewed_at=null
  => B: last_reviewed_at=null

单行路径一切正常;批量路径上 hook 拿到的 previous 是空的。

影响面

src/objects/*.hook.ts 里有 15 个文件读 ctx.previous。它们几乎都用 previous?.x 的可选链写法,所以在批量路径上不会抛错 —— 而是静默地什么都不做,或者走进「当作插入/当作首次转换」的分支。已确认的两种具体后果(均在 crm_knowledge_article 上实测):

  1. 批量编辑不会把文章标记为已复核。 knowledge_article_publish_timestamps 的注释写着「On any subsequent edit while published, refreshes last_reviewed_at so admin "stale article" reports work」—— 在批量路径上这句不成立。
  2. 批量写能把已发布文章的 last_reviewed_at 写成 null,readonly: true 不拦(单行路径上 hook 会补戳盖掉,批量路径上不会)。于是数据里出现「已发布 + 从未复核」的行 —— 而批量导入正是一家公司把既有知识库搬进 CRM 的常规方式,这批行恰恰是最需要复核的。

第 2 点是 #769 最终没有给 stale_articles 加 180 天窗口的直接依据(见 PR #776 的实测第 2 条):$lt 不命中空值,而 view filter 语法写不出「早于窗口或为空」的析取,加窗会把这批导入行一条不剩地藏掉。

其余 14 个 hook 尚未逐个核对,但凡是靠 previous 判断状态迁移的(campaign.hook 的 previous?.status === 'completed'、contract.hook 的 previous?.status === 'activated' 守卫、case.hook 的 becameClosed、opportunity 的赢单/丢单捕获等)都在同一条船上:批量路径上迁移判定退化为「没有迁移」或「首次迁移」。contract.hook 那条尤其值得先看 —— 它是守卫语义(if (event === 'beforeUpdate' && previous?.status === 'activated')),守卫在批量路径上失效的方向是放行。

归属

引擎侧的行为(input.id 缺失时不取 previous)大概率是平台的设计取舍而非 bug —— 批量路径要为 N 行取 N 份 previous,代价与语义都需要平台定夺;引擎自己在 needsPriorRecord(updateSchema) 为真时确实会取 priorRows(用于校验规则),只是没有把它喂给 hook。所以这一条可能需要 upstream 配合,本仓能先做的是:

  • 逐个核对 15 个 hook 在 previous === undefined 下的行为,把「静默不做」改成显式、可预期的语义;
  • 或者在本仓约定「批量写不经过业务 hook 语义」,并把依赖 previous 的不变量挪到别处(校验规则 / flow)。

两条路都是行为决策,需要先定夺再动手。

复现

在 worktree 里跑(需要 @objectstack/objectql + driver-memory):绑定一个 sys_fetch_previous_update 的复刻(priority 5 / beforeUpdate / object *)与真实 knowledge_article.hook,插入一条 status: 'published' 的文章,然后
api.object('crm_knowledge_article').update({ last_reviewed_at: null }, { where: { status: 'published' }, multi: true }),读回即为 null。PR #776 的探针脚本可照抄(未入库,按越界规则删除)。

Activity

  1. added
    bugSomething isn't working
    upstream:objectstackBlocked on / caused by the ObjectStack platform — tracked upstream
    on Aug 5, 2026
  2. yinlianghui commented on Aug 5, 2026

    @yinlianghui
    CollaboratorAuthor

    PM 分诊(修复线):平台半已镜像,方向待上游答复 → pm:blocked

    Blocked-by: objectstack-ai/objectstack#5574

    归属分析采纳原报告:引擎在 needsPriorRecord 为真时已为校验规则取 priorRows,却不喂给 hook —— 「批量路径不供给 previous」是设计取舍还是遗漏,只有平台能答。已开镜像 objectstack#5574(带探针实录),向平台线提两问:(1) 若是取舍,请把「multi:true 上 ctx.previous 不可用」写进 hook 契约文档;(2) 若愿供给,per-row previous 的语义与代价请平台定夺。

    本仓两条修复路线(hook 逐个显式化 vs 不变量搬到校验规则/flow)取决于上游答案,故整单 pm:blocked,不派。 上游答复到达即解锁重分诊。

    两点先行记录:

    • contract.hook 的守卫失效方向是放行(previous?.status === 'activated'),在 15 个 hook 里风险最高——解锁后优先处理;
    • 17.0 发布口径:批量写路径的 hook 语义缺口属已知问题,console 侧主要暴露面(bulkActions)已摘除(Un-wire the known-broken mass_update_stage bulk button until #508 lands #588),暴露面集中在 API/批量导入;是否需要在发布材料里提示由维护者定(不在本仓 releases/ 动手,见 CLAUDE.md 铁律)。

    关联:#780(同批发现,独立缺陷,已入队)、#781(出路 C 依赖本单)。


    Generated by Claude Code

  3. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    Collaborator

    Re-measured on @objectstack/* 17.0.0-rc.6 during the rc.5 → rc.6 upgrade (#1059, PR #1066). Fixed upstream.

    Both halves of the diagnosis in this card have moved:

    1. sys_fetch_previous_update is retired. @objectstack/objectql marks it ⛔ RETIRED — #5846 (a), delivered with #5574's engine half. Do not reintroduce it. The reason given is exactly the mechanism this card identified: its if (input.id && !ctx.previous) guard "is now PERMANENTLY FALSE" because update() reads the prior row and binds hookContext.previous before dispatching beforeUpdate.

    2. The multi: true path now dispatches per row, with previous bound. Under ADR-0058 Addendum II (D1–D4), a predicate write fires one before* dispatch per matched row on the single-record shape:

    input: carriesPayload ? { id: rowId, data: batchCtx.input.data, options } : { id: rowId, options },
    previous: coerceBooleanFields(schema, row),
    result: void 0                    // D2: no post-state in the before phase

    input.id names the row and previous is that row's pre-image — so the 15 hooks reading ctx.previous are no longer blind on bulk writes, and the two measured consequences (bulk edits not refreshing last_reviewed_at; a bulk write able to null it on a published article) should no longer hold.

    Two things worth knowing before closing:

    • There is now a ceiling on the matched-row set a predicate write fires per-row hooks over (#5038), and exceeding it rejects the write before the first dispatch. Bulk imports — the exact scenario in this card's impact section — are what will meet it.
    • Fan-out is real: a hook that ran once per batch now runs once per row.

    Scope note: static read of the shipped engine, not a re-run of this card's probe script. The last_reviewed_at-nulling repro is worth re-running before closing, since it also decides whether #769's stale_articles window can be revisited.


    Generated by Claude Code

  4. hotlong commented on Aug 14, 2026

    @hotlong
    Contributor

    Not closed — the recorded reading asks, in its own words, to be re-run before closing. Needs a GA probe.

    This card was on the direct-close list for the GA close-out bookkeeping sweep (#1151, group 3), which authorised closing it on its recorded rc.6 retest reading. That reading explicitly withholds the close, so the card stays open. Recording why.

    What the reading does establish

    On @objectstack/* 17.0.0-rc.6, a static read of the shipped engine shows both halves of this card's diagnosis have moved:

    1. sys_fetch_previous_update is retired. @objectstack/objectql marks it ⛔ RETIRED — #5846 (a), delivered with #5574's engine half. Do not reintroduce it. The stated reason is exactly the mechanism this card identified: its if (input.id && !ctx.previous) guard "is now PERMANENTLY FALSE" because update() reads the prior row and binds hookContext.previous before dispatching beforeUpdate.
    2. The multi: true path now dispatches per row with previous bound. Under ADR-0058 Addendum II (D1–D4), a predicate write fires one before-phase dispatch per matched row on the single-record shape, with input.id naming the row and previous carrying that row's pre-image.

    Source: #779 (comment)

    Why that is not enough to close this card

    1. The reading is a static read and says its conclusion is untested. Its scope note:

    static read of the shipped engine, not a re-run of this card's probe script. The last_reviewed_at-nulling repro is worth re-running before closing, since it also decides whether #769's stale_articles window can be revisited.

    Its conclusion is stated conditionally too — the two measured consequences "should no longer hold". This card was filed on a live probe with observed output for three write shapes; nothing of that kind has been re-run.

    2. The same reading surfaces a new behaviour that lands on this card's headline scenario. Per-row hook dispatch on a predicate write now has a ceiling on the matched-row set (objectstack#5038), and exceeding it rejects the write before the first dispatch — and the reading names bulk imports as what will meet it. Bulk import is precisely this card's impact section ("批量导入正是一家公司把既有知识库搬进 CRM 的常规方式"). So the scenario has not simply been repaired; its failure mode may have been exchanged for a different one that nobody has measured. Fan-out is real too: a hook that ran once per batch now runs once per row.

    3. The card's stated repo-side work was never resolved, only deferred. This card asked for a decision between two routes — make all 15 previous-reading hooks explicit under previous === undefined, or move the invariants to validation rules / flows — and the PM triage parked it pm:blocked pending the upstream answer. The upstream answer arrived as a behaviour change, which makes the decision moot in a different way than either route anticipated, and the specific risk flagged as highest (contract.hook's guard, whose failure direction is permissive: previous?.status === 'activated') has not been re-checked under per-row dispatch.

    What would close it

    A GA probe re-running this card's own reproduction: bind the real knowledge_article.hook, insert a status: 'published' article, then

    api.object('crm_knowledge_article').update({ last_reviewed_at: null }, { where: { status: 'published' }, multi: true })
    

    and read back — confirming last_reviewed_at is now stamped rather than nulled, and that a bulk edit refreshes it. Worth measuring the row-count ceiling in the same run, since it decides whether the bulk-import scenario is actually safe. That probe also unblocks #769/#781's stale_articles window question.

    ⚠️ Two routing notes for the PM:

    Nothing on this card was changed: still open, labels untouched.


    Generated by Claude Code


    Generated by Claude Code

  5. hotlong commented on Aug 14, 2026

    @hotlong
    Contributor

    GA reading (17.0.0) — NOT REPRODUCED. Closing, and the new ceiling neither masks it nor replaces it.

    From #1153 (GA close-out B2), session session_01XAK3brMLjd4ykF4QAhFnuo. This is the live re-run that #1151's refusal comment asked for: the recorded rc.6 reading was a static source read that stated its conclusion conditionally and said the repro was "worth re-running before closing". It has now been re-run.

    Environment: @objectstack/objectql 17.0.0 GA + @objectstack/driver-memory, real crm_knowledge_article schema, the real shipped knowledge_article.hook bound through the shipped bindHooksToEngine (not a probe-local reimplementation), plus a priority-299 spy hook ahead of the 300 the card names.

    Guards against a probe that cannot fail

    • The bind result is asserted before anything else runs ({"registered":2,"skipped":0,"errors":[]}) — an unbound hook would have made every number below vacuous, so it aborts instead.
    • The single-row path runs in the same process as a positive control. Had it also shown previous=undefined, the harness would be broken rather than the engine, and the run says so.
    • The closing assertions are on stored data read back, not on the absence of a throw.

    The card's three write shapes, re-run

    === single-row update (category only) ===          <- positive control
      [spy] input.id="crm_knowledge_article-…-1" previous={"status":"published","last_reviewed_at":"…40.960Z"}
      => last_reviewed_at refreshed: true
    
    === MULTI update (category only, every published row) ===
      [spy] input.id="crm_knowledge_article-…-1" previous={"status":"published","last_reviewed_at":"…40.989Z"}
      [spy] input.id="crm_knowledge_article-…-2" previous={"status":"published","last_reviewed_at":"…40.993Z"}
      update() returned: 2
      => B.category=troubleshooting  last_reviewed_at refreshed: true
    
    === MULTI update writing last_reviewed_at: null ===
      [spy] input.id="crm_knowledge_article-…-1" previous={"status":"published","last_reviewed_at":"…41.003Z"}
      [spy] input.id="crm_knowledge_article-…-2" previous={"status":"published","last_reviewed_at":"…41.003Z"}
      => #1153 A: status=published last_reviewed_at="2026-08-14T13:18:41.007Z"
      => #1153 B: status=published last_reviewed_at="2026-08-14T13:18:41.007Z"
      published rows left with an EMPTY last_reviewed_at: 0 of 2
    

    Set against this card's original rc.2 output, every line has inverted:

    shape rc.2 (this card) 17.0.0 GA
    MULTI, input.id seen by the hook undefined the row's own id, one dispatch per row
    MULTI, ctx.previous seen by the hook undefined that row's pre-image
    MULTI edit refreshes last_reviewed_at false true
    MULTI write of last_reviewed_at: null lands as null on published rows overwritten by the hook stamp; 0 of 2 left empty

    Both measured consequences are gone, and with them the "已发布但从未复核" row this card is titled on. The 15 previous-reading hooks are no longer blind on the bulk path — including contract.hook's guard, whose failure direction was permissive.

    The new per-row dispatch ceiling: masks nothing, replaces nothing

    This was #1151's specific worry, and it is the one that decides whether a close is honest. The ceiling is MAX_BULK_PER_ROW_HOOK_ROWS = 10000, re-exported by the engine from the spec contract. Measured by crossing the boundary on the real engine:

      seeding 10000 rows … write OK (modified=10000);
          rows carrying the new body = 10000/10000;  rows with a review stamp = 10000/10000
      seeding 10001 rows … REFUSED code=ERR_BULK_PER_ROW_HOOK_LIMIT matched=10001 limit=10000
          rows carrying the new body = 0/10001   (0 means nothing was written)
    

    So:

    • It cannot mask the fix. At the ceiling the write succeeds and all 10000 rows carry the hook's stamp — the per-row dispatch demonstrably runs at the largest batch it accepts. A masking ceiling would have had to reject the batches this card's scenario uses; the whole earlier sweep (50 / 200 / 1000 / 5000 rows) also passed with hooks firing.
    • It is not the same defect wearing new clothes. This card's failure mode is silence: the hook no-ops and nothing says so. The ceiling's failure mode is a refusal with a named code, a message that states the limit and the remedy, and zero rows written. Loud-and-atomic is the opposite of the shape this card exists to record.

    The honest residual — worth knowing, not worth blocking a close: a knowledge-base import of more than 10000 articles in one predicate write is now rejected and must be paginated, and a hook that used to run once per batch now runs once per row. Both are documented, both are loud.

    Disposition

    Closed as completed. Version: @objectstack/* 17.0.0 GA. Probe: this card's own reproduction on the real engine with the real hook, plus the ceiling boundary. Result: not reproduced in any of the three write shapes.

    Upstream: objectstack#5574 → PR objectstack#6697 (merged 2026-08-08), which delivered per-row before* dispatch under ADR-0058 Addendum II and retired sys_fetch_previous_update.

    Label note: pm:blocked dropped in the same write — its blocker objectstack#5574 is closed as completed. #1151 deliberately left it because its mandate only authorised a label write alongside a close; that close is now happening.

    This also unblocks the stale_articles window question parked on #769 / #781: a bulk write can no longer leave a published article with an empty last_reviewed_at, so the disjunction that made a 180-day window unsafe no longer has to be written.


    Generated by Claude Code


    Generated by Claude Code

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

    bugSomething isn't workingupstream:objectstackBlocked on / caused by the ObjectStack platform — tracked upstream

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions