Skip to content

docs(hook-bodies): 写集一节改为准确的覆盖面描述 —— #4305/#4344 之后"没有静态检查"已不成立 (#4351) - #4355

Merged
os-zhuang merged 1 commit into
mainfrom
claude/hook-bodies-docs-update-085504
Jul 31, 2026
Merged

docs(hook-bodies): 写集一节改为准确的覆盖面描述 —— #4305/#4344 之后"没有静态检查"已不成立 (#4351)#4355
os-zhuang merged 1 commit into
mainfrom
claude/hook-bodies-docs-update-085504

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Closes #4351.

问题

content/docs/automation/hook-bodies.mdx 的 "Not statically checked: the write set" 一节声称写面完全没有静态检查,并称其为 "accepted gap with no planned closure"。自 #4305(validateHookBodyWrites)与 #4344(action 侧姊妹规则)起,这不成立 —— hook-body-write-unknown-field / action-body-write-unknown-field 会解析 L2 body,把字面写集拿去和目标对象声明的字段比对,未知字段带 did-you-mean 告警。

"declared ≠ enforced" 的镜像:enforced ≠ declared —— 能力已交付,文档还在说没有。作者按文档放弃一个真实存在的护栏,和按文档相信一个不存在的护栏,代价是对称的。

改动

hook-bodies.mdx —— ### Not statically checked: the write set### Write-set checking

  • 覆盖面写准:三种字面写法按 hook / action 分列成表(action 的 ctx.input 是 params bag,只有 api-crud-literal 过继),advisory warning 不 gate,跑在 os validate / os lint / os compile
  • 单列 "A missing warning is not a clean bill of health":computed key、spread、动态对象名、wildcard ctx.input、多目标部分命中、跨包对象、aliased input —— 静态不可知的一律静默跳过,所以告警的缺席不等于正确
  • Hook body writes are not statically checkable — accept the gap or give HookSchema a structured writes declaration #3700(结构化 writes,closed as not planned)保留为准确历史,但撤回 "accepted gap" 的结论:缺口是从另一头(解析写集)关的,不是没关。

hooks.mdx —— 三处交叉链接指向被改名的锚点,且都在复述已撤回的说法,一并更新。wildcard 那条反而更锐利:wildcard hook 的 ctx.input 写恰恰是 lint 必须跳过的那一种。

两条前提按实测修正,而非照抄

原 issue 的清单里有两条建立在错误前提上。我实测后按事实写,并各开了独立跟进项。

1. flow update_record 根本没有未知字段检查。

原文档建议"优先用 flow update_record,它有 hook body 没有的静态检查"—— 在这条轴上现在正好反了。同一 stack 三个面写同一个不存在字段 stagee,validateReferenceIntegrity 的实际输出:

写面 结果
hook body ctx.input.stagee = … hook-body-write-unknown-field (warning)
action body ctx.api.object('deal').update({stagee}) action-body-write-unknown-field (warning)
flow update_record fields: { stagee } 什么都不报

validate-readonly-flow-writes.ts 是唯一走 update_record.fields 的规则,而它显式跳过未知字段(if (!meta) continue; // unknown field — a form/field-layout lint concern, not this rule's)。建议予以保留(结构化 diff、gating 的 flow-update-readonly-field 仍然成立),但把 gap 点明。

2. "silently never lands" 两个方向都不对。

lint 自己的诊断文案说未知列静默消失。实测:没有任何一层剥离它 —— applyMutationsToInput 是裸 Object.assign,validateRecord 只遍历已声明字段 —— 所以它一路到驱动:

  • SQL 驱动:未知列进入 knex 语句,整条写失败(better-sqlite3 实测 table deal has no column named stagee,零行落库);
  • schemaless 驱动(memory / MongoDB):杂散键确实被持久化。

文档原来那段运行时描述是对的,予以保留并按驱动族拆开写细。lint 的文案问题属代码缺陷,单独跟进。

边界

ctx.record 那一半不属于本 PR —— 那是 #4345,已在同名 PR 里连同 sandbox-surface 表和 signature-conventions 一起处理。

验证

  • 两个 MDX 经 @mdx-js/mdx + remark-gfm 编译通过;fumadocs-mdx 生成通过。
  • 新表经 remark 解析确认每行 3 列,\| 正确渲染为字面 |
  • 全仓已无 not-statically-checked-the-write-set 锚点残留,亦无 "accepted gap" / "not statically validated" 等旧说法。
  • content/docs/automation/ 为手写目录(生成的只有 content/docs/references/),不受 check:docs 影响。

纯文档,无行为变更,空 changeset。

🤖 Generated with Claude Code

…the section #4305/#4344 made false (#4351)

`content/docs/automation/hook-bodies.mdx` claimed the write side had **no**
static checking at all, and called it an "accepted gap with no planned
closure". Since #4305 (`validateHookBodyWrites`) and #4344 (the action-side
sibling) that is false: `hook-body-write-unknown-field` and
`action-body-write-unknown-field` parse an L2 body, resolve its literal writes
against the target object's declared fields, and warn with a did-you-mean.

The mirror image of "declared ≠ enforced": here the capability shipped and the
docs still said it had not. An author who drops a real guardrail because the
docs said it does not exist pays the same price as one who trusts a guardrail
that does not.

- "Not statically checked: the write set" → "Write-set checking". Coverage
  described accurately: the three literal patterns (tabulated per surface,
  since an action's `ctx.input` is its params bag and only `api-crud-literal`
  carries over), advisory `warning` that never gates, and the shapes the parser
  must skip — computed keys, spreads, dynamic object names, wildcard
  `ctx.input`, partial multi-target hits, cross-package targets, aliased input
  — so the ABSENCE of a warning is not read as proof of correctness.
- #3700 (structured `writes`, closed as not planned) stays as accurate history,
  but the "accepted gap" conclusion is retracted: the gap was closed from the
  other end, by parsing the write set out of the source.
- Three now-stale cross-links in `hooks.mdx` follow the renamed anchor. The
  wildcard bullet gets sharper rather than deleted — a wildcard hook's
  `ctx.input` writes are exactly the shape the lint must skip.

Two claims were corrected against measured behaviour rather than restated:

- **Flow `update_record` has no unknown-field check.** The old advice
  ("prefer a flow node, they get the static checking hook bodies can't") is
  now backwards on this axis. Verified on main: for the same undeclared field,
  the hook body and action body each warn and the `update_record` node reports
  nothing — `validateReadonlyFlowWrites` walks its `fields` but skips unknown
  ones by design (`if (!meta) continue`). The recommendation is kept for what
  is still true (structured diffing, the gating `flow-update-readonly-field`)
  with the gap named.
- **"Silently never lands" is wrong in both directions.** The lint's own
  message says the unknown column quietly vanishes. Measured: nothing strips
  it — `applyMutationsToInput` is a plain `Object.assign`, `validateRecord`
  walks declared fields only — so it reaches the driver. On SQL the whole write
  FAILS (`table deal has no column named stagee`, zero rows stored); on memory/
  MongoDB the stray key IS persisted. The doc's runtime paragraph was right and
  is kept, now split by driver family. The lint's wording is tracked separately.

Docs only — no behaviour change, empty changeset.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Jul 31, 2026 8:38am

Request Review

@github-actions github-actions Bot added size/s documentation Improvements or additions to documentation tooling labels Jul 31, 2026
@os-zhuang
os-zhuang merged commit 0449c63 into main Jul 31, 2026
16 checks passed
@os-zhuang
os-zhuang deleted the claude/hook-bodies-docs-update-085504 branch July 31, 2026 08:42
xuyushun441-sys pushed a commit that referenced this pull request Jul 31, 2026
…just rewrote

#4351/#4355 rewrote content/docs/automation/hook-bodies.mdx to stop claiming the
write side was unchecked. That section lands squarely on this branch's subject,
so it is updated here rather than left false for a second time: the write-shape
table gains the ctx.record row (and is no longer 'three shapes'), the silent-bail
list gains the escape rule, and Signature conventions now says outright that an
action's ctx.record is read-only in effect and ctx.api.object(...) is the one way
a body persists anything.
xuyushun441-sys pushed a commit that referenced this pull request Jul 31, 2026
Both landed on the same surface while this branch was open, so the merge is a
reconciliation rather than a textual resolution:

- #4344 built the same `actions` + `objects[].actions` walk, with the same
  value-dedupe of defineStack's merged copies (it found the double-report the
  same way). Two walks over one surface would drift silently, so its
  `collectActionBodies` is now exported (module-level, not via the barrel —
  same posture as BODY_WRITE_SYSTEM_FIELDS), carries the bound object, and this
  rule consumes it. The duplicate walk and its helpers are deleted.

- The two rules split one write surface and answer different questions, so the
  suite, the barrel and the spec JSDoc now say which is which: `ctx.api` writes
  → does this field exist (did-you-mean); `ctx.record` writes → none of them
  land, declared or not (no did-you-mean, because the name is not the bug).

- #4351/#4355 rewrote the hook-bodies write-set section this branch also
  edited. Its write-shape table listed three shapes and said nothing about
  `ctx.record`, which this branch now checks — added the row plus the note that
  it is a different defect, and that the record rule's statically-opaque cases
  are covered by the runtime recorder instead.

Verified after the merge: lint 642, spec 7160, runtime 989, cli 628, spec
check:generated 8/8, typecheck + eslint clean, showcase 0 findings clean and
exactly 1 with the repro planted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
os-zhuang added a commit that referenced this pull request Jul 31, 2026
* feat(lint): action body 里落不了地的 ctx.record 写在作者时告警 (#4345)

#4344 deliberately left `ctx.record` alone and said why: an action's
`ctx.record` is a plain snapshot (`unwrapProxyToPlain(actionCtx?.record)`) that
`boundActionHandler` never writes back — the hook path's
`applyMutationsToInput` has no action-side counterpart — so `ctx.record.x = …`
is discarded for DECLARED and undeclared fields alike. Reporting that through
the unknown-field rule would have been actively wrong: flagging only the
undeclared half implies the declared half persists. It needed its own finding.

New rule `action-record-write-discarded`, advisory.

It is NOT "flag every ctx.record assignment" — mutating the snapshot to build a
payload is legitimate:

    ctx.record.stage = 'won';
    await ctx.api.object('crm_deal').update(ctx.record);   // LIVE

So the finding requires the write to be provably dead: `ctx.record` never
escapes the body as a value. Property reads do not rescue a write; handing the
object to anything — argument, assignment RHS, spread, return — does. Aliasing
reads as an escape, the safe direction.

Truthiness and type tests are NOT escapes, and that is what makes the rule fire
on real code. Running it against the showcase surfaced it: `mark_done` opens
with `ctx.recordId || (ctx.record && ctx.record.id)` — the idiom action bodies
are actually written with — and counting that guard as an escape silenced the
finding on the one body in the repo that had a record write. Only the LEFT
operand of &&/||/?? is a test; `x || ctx.record` still escapes.

One suite member, two rule ids: both findings fall out of one parse of one
source on one surface. A second REFERENCE_INTEGRITY_RULES member would parse
every action body twice to say two things about one walk, and hand-wiring it
into the three CLI commands is the drift that suite exists to end —
validateReadonlyFlowWrites is the standing proof, wired into validate and
compile but never into lint. Trade-off written down at both ends.

The ledger ratchet fired as designed. `record-property-assign` joins the shared
HOOK_BODY_WRITE_PATTERNS — the extractor's shape inventory, not any one rule's
— and both consumers had to classify it first. Not cosmetic on the hook side: a
record write carries no `object`, and validateHookBodyWrites branched on
exactly that to mean "a ctx.input write", so the new shape would have been
reported as "the hook writes 'stage' to its input". The hook rule now declares
its own consumed subset and its exclusion with a reason — a hook sandbox
context has no ctx.record at all, so the expression throws at run time rather
than silently no-op'ing, and a loud failure is not an advisory rule's business.

extractHookBodyWriteSet is the new one-parse entry point (writes +
ctxRecordEscapes); extractHookBodyWrites stays a thin projection.

Boot path: the action prefilter widens from `api` to `api`-or-`record`, so a
body reaching neither still never loads the ~9 MB TypeScript compiler.
lazy-deps.test.ts pins it — and its header plus two case names, which still
claimed every lazy dep waited on "a react page", now name the trigger each one
pins (typescript has also been loaded by the hook-body gate since #4271).

spec/runtime: ScriptBodySchema, ActionSchema.body and ScriptContext.record now
state that ctx.api.object(...) is the only path that persists anything and that
ctx.record is read-only in effect. Doc comments only — all 8 generated
artifacts verified unchanged. Whether the runtime should instead refuse or
honour a record write stays open on #4345.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Merge origin/main; document the ctx.record write in the section #4351 just rewrote

#4351/#4355 rewrote content/docs/automation/hook-bodies.mdx to stop claiming the
write side was unchecked. That section lands squarely on this branch's subject,
so it is updated here rather than left false for a second time: the write-shape
table gains the ctx.record row (and is no longer 'three shapes'), the silent-bail
list gains the escape rule, and Signature conventions now says outright that an
action's ctx.record is read-only in effect and ctx.api.object(...) is the one way
a body persists anything.

---------

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
os-zhuang added a commit that referenced this pull request Jul 31, 2026
…a silent no-op (#4271) (#4368)

`hook-body-write-unknown-field` and `action-body-write-unknown-field` told
authors the undeclared column "silently never lands in the stored record".
Measured on main, that is wrong in both directions.

Nothing between the body and the driver filters the key: `applyMutationsToInput`
is a plain `Object.assign`, and `validateRecord` walks declared fields on insert
and `continue`s past a key with no field def on update. So the driver decides —
SQL puts the stray column in the statement and the WHOLE write fails with a
driver-level error (nothing stored, error far from the authoring mistake), while
a schemaless driver spreads the payload and persists the stray key.

A lint that misdescribes the failure it warns about teaches the wrong debugging
instinct: an author told the value silently vanishes will not connect the driver
error they actually see to the typo that caused it, and on a schemaless driver
will not go looking for the stray key that is really there.

All three messages now state the split. The rule headers, the `ScriptBodySchema`
/ `ActionSchema.body` notes and the two still-unreleased #4271 changesets are
corrected to match. #4355 fixed the prose docs; this is the same correction on
the surfaces that ship in the packages.

Both outcomes are pinned by a new integration test,
`runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts`.
Its insert cases run the full chain — real QuickJS sandbox, real hook body, real
engine, real SQLite table — so "reaches the driver unfiltered" is proved rather
than asserted: if anything on that path learns to filter, the SQL half stops
throwing and the test goes red.

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/s tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs(hook-bodies): 写集一节还在说"没有静态检查",#4305 之后已不成立(enforced ≠ declared)

1 participant