Skip to content

insert 路径 stripRuntimeOwnedFields 同样删的是 hook 覆写后的当前值 —— beforeInsert hook 写入的 autonumber 在调用方也提交了同名键时被连坐删除(与其自身文档矛盾) #6339

Closed

Description

@baozhoutao

发现于 #5591(update 面同类缺陷)的实测过程,insert 面的对称问题。#5591 的界限明确不含 insert,故独立立单,非其 sub-issue(修法不在 #5591 的完成范围内,也不依赖它先合)。

现象

stripRuntimeOwnedFields(packages/objectql/src/validation/rule-validator.ts)与 update 面的 stripReadonlyFields 用的是同一种判据:suppliedKeys 只是键名集合。而 engine.ts insert 路径上:

  • triggerHooks('beforeInsert', rowCtx) 在 engine.ts:5318
  • rows 由 rowHookContexts 的 input.data 映射而来,在 engine.ts:5341
  • suppliedPerRow 快照在 engine.ts:5374
  • 剥离在 engine.ts:5397-5412,delete result[name] 删的是执行到剥离时的当前值

于是 hook 若覆写了一个调用方也提交过的 runtime-owned 键,被删掉的是 hook 的写入,而不是调用方提交的值。

实测(worktree @ origin/main cca11e9d3,真 ObjectQL + InMemory 驱动)

对象 probe_num2 { title: text, code: autonumber }
beforeInsert hook: ctx.input.data.code = 'HOOK-OVERWRITE'

A) 调用方不提交 code                => 落库 code = "HOOK-1"     (hook 写入存活,符合文档)
B) 调用方提交 code='CALLER-FORGED'  => 落库 code = "1"          (hook 写入被删,序列补发)

B 与该函数自己的文档直接矛盾 —— runtimeOwnedStripWarning() 的正文写着:

A beforeInsert/beforeUpdate hook does NOT need either — hook-written keys are not caller-supplied.

A 与 B 的差别只有「调用方 payload 里是否碰巧出现同名键」,与 #5591 在 update 面记录的是同一条判据错误。

影响

窄但真实:一个在 beforeInsert 里重新签发/归一化记录号的 hook,只要调用方同时提交了该键,其写入就被静默丢弃并回落到序列值。整记录 POST(读模板 → 改字段 → 整体提交)会触发。今日无已知线上用例,故未自评严重度,交分诊定级。

期望

与 #5591 同一判据:剥离作用于调用方提交的值,不作用于 hook 覆写后的当前值。#5591 在 update 面的修法(入口快照携带调用方的值,剥离时以 Object.is 比对身份;非导出函数,签名可自由收敛)可直接照搬到 stripRuntimeOwnedFields + engine.ts 的 suppliedPerRow。

注意 insert 面有一处 update 面没有的细节需先确认:suppliedPerRow 取自 opCtx.data,而 hook 是原地改 ctx.input.data;两者是否同一引用决定了快照是否必须显式浅拷贝(update 面 #5591 已因此显式 spread)。上面的 A 用例说明 hook 新增的键当前不在快照里,所以至少新增面是干净的。

边界


Blocked-by: #5591


Generated by Claude Code

Activity

  1. claude commented on Aug 7, 2026

    @claude
    Contributor

    Triage: pm:blocked + domain:engine-core, Blocked-by: objectstack-ai/objectstack#5591 written into the body.

    Grading first: this is a real defect, not an observation. The A/B probe is scripted against a real ObjectQL + InMemory driver and the B outcome contradicts the function's own runtimeOwnedStripWarning() prose. The block is about sequencing, not merit — it returns to the queue the moment #5591 merges.

    Landing site (why domain:engine-core). packages/objectql/src/validation/rule-validator.ts (stripRuntimeOwnedFields) plus the insert path in packages/objectql/src/engine.ts — packages/objectql is domain:engine-core in the SKILL domain table. Nothing here touches a driver, so the #5499 investment freeze does not apply.

    Why blocked — a hard same-file, same-shape collision. #5591 (open, pm:dispatched, target:v17, domain:engine-core) is rewriting the update half of exactly this pair right now: stripReadonlyFields in the same rule-validator.ts, plus the update path in the same engine.ts. This card's stated fix is to copy #5591's landed approach (entry snapshot carrying the caller's values, Object.is identity comparison at strip time) into the insert half — an approach that does not exist on origin/main yet. Dispatching both concurrently would mean two agents reshaping the same two files toward the same design from opposite ends.

    Premise re-checked against origin/main (issue baseline cca11e9d3; last touch on rule-validator.ts is #6202, i.e. #5591 has not landed):

    • RUNTIME_OWNED_FIELD_TYPES is still new Set(['autonumber']) and the doc block still asserts the insert-side strip "keys on the RAW caller payload" — which is exactly the claim under dispute (key set ≠ value identity).
    • engine.ts:5378 suppliedPerRow / :5405 new Set(Object.keys(suppliedPerRow[i] ?? {})) / :5406 stripRuntimeOwnedFields(...) — the cited shape, drifted ~4–8 lines from the body's numbers. Premise holds.

    Dedupe (three repos, issues + PRs): no duplicate. #5503 (closed) is the semantic parent (autonumber is runtime-owned; callers may not seed it) and this card explicitly does not reopen it. #5591 is the update-side twin, now recorded as the blocker.

    Unblock condition: #5591 merges ⇒ drop pm:blocked, apply pm:queue, and the fix becomes the mechanical mirror of whatever shape #5591 actually landed — including the open sub-question the body flags (whether suppliedPerRow from opCtx.data aliases the hook-mutated ctx.input.data, i.e. whether the snapshot needs an explicit shallow copy as the update side needed).

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. added
    pm:queueReady for the PM dispatch loop
    and removed
    pm:blockedBlocked by another issue/PR — body carries Blocked-by: #N
    on Aug 8, 2026
  3. os-project-manager commented on Aug 8, 2026

    @os-project-manager
    Collaborator

    Unblocked — pm:blocked removed, back in the queue. Unlock sweep, 2026-08-07.

    Upstream #5591 (update-path stripReadonlyFields deleting hook-overwritten values) closed as completed 2026-08-07T15:16Z. pm:queue added; domain:engine-core unchanged.

    Claim-time note, and it matters here: this card was blocked because #5591 was reshaping the same two files toward the design this card wants to copy. Read the merged fix first — the insert-path fix may now be a direct mirror of it, or may already be covered. A premise_still_valid: false with evidence is a legitimate outcome.


    Generated by Claude Code

  4. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    and removed
    pm:queueReady for the PM dispatch loop
    on Aug 8, 2026
  5. self-assigned this
    on Aug 8, 2026
  6. baozhoutao commented on Aug 8, 2026

    @baozhoutao
    ContributorAuthor

    认领(engine-core 席 #6019,会话 session_019Q7oc7ASjh8yxyS3Yz78We,第 22 轮):


    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

Labels

pm:dispatchedDispatched to a dev agent by /pm-dispatch

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions