feat(lint,spec): L2 action body 写不存在字段从盲区变为作者时 lint 告警 (#4271) - #4344
Merged
Conversation
The write-set lint #4305 gave L2 hook bodies now covers the other surface that carries one. An action body is the same artefact — same `HookBodySchema`, same `HookBodySchema.safeParse` in `actionBodyRunnerFactory`, same QuickJS sandbox — so it fails the same way: `ctx.api.object('crm_deal').update({ stag: 'won' })` succeeds, returns success to the caller, and the unknown column never lands. New rule `action-body-write-unknown-field`, advisory, wired into REFERENCE_INTEGRITY_RULES so `os validate` / `os lint` / `os compile` all report it. Both places the runtime reads actions from are walked (top-level `actions` and `objects[].actions`); a defineStack-merged action, which lives in both, is reported once at its authored path. Only the `ctx.api` write family carries over. An action's `ctx.input` is its PARAMS bag, not a record, so resolving those names against object fields would flag every correctly-named parameter. `ctx.record` is not a write surface either: the runner hands the body a plain snapshot and never writes it back, so `ctx.record.x = …` is discarded for declared and undeclared fields alike — a different defect, and flagging only its undeclared half would imply the declared half persists. So the rule ships a declared PARTITION of the shared HOOK_BODY_WRITE_PATTERNS — ACTION_BODY_WRITE_PATTERN_IDS plus ACTION_BODY_WRITE_EXCLUSIONS, each exclusion carrying its reason — tested to cover the shared ledger exactly, so a fourth pattern landing on the hook side fails this rule's test until someone classifies it. Every applicable pattern is proved end-to-end through the full validator; every exclusion is proved extractable-but-inapplicable. One extractor, one field index, one system-column set, shared rather than copied. The dedupe is by value (bound object + name + body source), not by object identity the way collectBundleActions can afford: the suite runs on the parsed stack and parsing rebuilds every node, so a merged action's two copies arrive distinct-but-equal. An identity check passed the unit fixture and reported the showcase app's one warning twice — caught by running `os validate` on it. Lands one notch tighter than the hook side on the boot path: the only applicable pattern is rooted at `ctx.api`, so an action body that never mentions it does not parse at all, let alone load the ~9 MB TypeScript compiler. Guarded by lazy-deps.test.ts. spec: ScriptBodySchema and ActionSchema.body point at the action-side rule and spell out that ctx.input (params) and ctx.record (a discarded snapshot) are not record-write surfaces. Doc comments only — all 8 generated artifacts verified unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Conflict in validate-hook-body-writes.ts, resolved in main's favour and then some. #4339 replaced this rule's hand-copied `SYSTEM_FIELDS` list with `IMPLICIT_FIELDS` — derived from the spec via the package-shared `system-fields.ts` (#4330) — which supersedes the `BODY_WRITE_SYSTEM_FIELDS` rename this branch had introduced for the same reason. So the branch drops its rename and exports `IMPLICIT_FIELDS` instead: the action rule shares the derived set rather than the hand-copied one, which is strictly the outcome both changes were after.
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 107 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
github-actions
Bot
added
documentation
This was referenced Jul 31, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 31, 2026
…the section #4305/#4344 made false (#4351) (#4355) `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: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
…es the build (#4271) (#4369) The write-set family #4305 (hooks) and #4344 (actions) opened had a third surface, and it was the one the docs had spent the longest recommending as the safe alternative to the other two. A flow `update_record` node whose `config.fields` names a field the target object never declares was caught by NOTHING: `validate-readonly-flow-writes.ts` walks that exact map and explicitly stepped over the unknown key (`if (!meta) continue; // a form/field-layout lint concern` — a referral to a rule that does not check writes), and `validate-flow-template-paths.ts` checks the `{record.<path>}` READ tokens interpolated into node config, never the write-side key. New rule `flow-node-write-unknown-field`, wired into REFERENCE_INTEGRITY_RULES so `os validate`, `os lint` and `os compile` report it at once — one more path than the hand-wired readonly rule next door reaches. It GATES where its two siblings advise. The hook and action rules are advisory because they PARSE JavaScript: the finding is only as good as the extractor. Nothing here is parsed — `config.fields` is a literal map next to a literal `objectName`, the same certainty `flow-update-readonly-field` already gates on one config key over. A rule that errors on a write the engine strips while only warning on a write that names no column at all would be incoherent in the same map. And the runtime consequence is not the benign "consumer skips the unknown name and renders the rest" that keeps page-field-unknown / form-field-unknown advisory. Both halves were measured: through the engine an undeclared key reaches `driver.update` verbatim, and on SQLite/knex it becomes `no such column: stagee` — the statement is rejected WHOLE, so the correctly named fields in the same payload never land either. On a schemaless datasource the stray key is persisted into a column no schema-driven read returns. One field index and one implicit-field set across all three surfaces: `indexObjectFields` and `IMPLICIT_FIELDS` are imported from the hook rule rather than copied. Every skip is silent and exists so the gate only fires on a certainty — templated `objectName`, non-literal `fields`, cross-package objects, objects declaring no fields at all (external / introspected schemas), dotted keys. `runAs` is deliberately not consulted, unlike the readonly rule: an elevated identity bypasses the readonly strip, but no run identity conjures a column. Scope is declared as data: FLOW_WRITE_NODE_TYPES (today `update_record`) and FLOW_WRITE_NODE_TYPES_DEFERRED (`create_record`, with its reason) are partition-tested against the CRUD node types that carry a `fields` write map, derived behaviourally from the spec's executor-written config schemas. Docs: automation/hook-bodies.mdx and automation/hooks.mdx carried the pre-#4271 framing (write side "not checked", an "accepted gap with no planned closure") long after both body rules landed. Both now state the real asymmetry, with an explicit callout that a clean run is not proof. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景
#4305(commit c1d44f7)让 L2 hook body 的写集在作者时可查。但 action body 是同一个artefact:同一个
HookBodySchema、actionBodyRunnerFactory里同一个HookBodySchema.safeParse、同一个 QuickJS 沙箱。所以它以同样的方式失败 —— action 里写跑通、返回成功给调用方,未知列静默不落库。写面还有一半是盲的。
这个 PR
新规则
action-body-write-unknown-field(advisory,永不 gate),挂进REFERENCE_INTEGRITY_RULES,所以os validate/os lint/os compile一起报。runtime 从两处读 action(
collectBundleActions:顶层actions与objects[].actions),两处都走;defineStack合并过的 action 同时存在于两处,只报一次,报在作者写的位置。只移植
ctx.api一族 —— 这是重点,不是省事ctx.input不适用。 action 的ctx.input是它的 params 包(input: unwrapProxyToPlain(actionCtx?.params)),不是记录。拿这些名字去对象字段里找,会把每一个拼写正确的参数都告警一遍 —— 纯误报机器,而一条误报就足以让一个 advisory lint 被无视。ctx.record也不是写面。 runner 交给 body 的是一份普通快照(record: unwrapProxyToPlain(actionCtx?.record)),boundActionHandler只return result.value,没有applyMutationsToInput(那是 hook 路径独有的)。所以ctx.record.x = …对已声明字段和未声明字段一样被丢弃。这跟"未知列静默消失"是两种缺陷;只报未声明的那一半,等于暗示已声明的那一半会落库 —— 恰好是这个规则家族要消灭的假完成。单独开了 issue 跟踪,不在本 PR 混做。台账做成 partition,不是第二份清单
ACTION_BODY_WRITE_PATTERN_IDS(今天只有api-crud-literal)ACTION_BODY_WRITE_EXCLUSIONS(两条 input 族,各带 reason)测试断言两半恰好覆盖
HOOK_BODY_WRITE_PATTERNS。以后 hook 侧加第四条模式,这个规则的测试直接红,逼人做分类决定 —— 沉默不算决定。用 include-list 而非 exclude-list 是有意的:未分类的新模式在这里是哑的(漏报),不会带着没推敲过的语境上线(误报)。跟 extractor 的静默 bail 是同一个不对称。
延续 #4305 的 reconciliation 姿态,并且更严一档:
extractor、字段索引、系统列集合都从 hook 模块复用(
SYSTEM_FIELDS提升为BODY_WRITE_SYSTEM_FIELDS并导出,不进 barrel)。两份 system 列清单迟早会给出两个不同答案。一个跑出来的真 bug
拿 showcase 打反例(
progress→progres)跑pnpm validate,同一条告警报了两次。defineStack的mergeObjectActions把带objectName的 action 追加进对象、同时保留顶层条目;而 suite 跑在 Zod parse 之后的 stack 上,parse 会重建每个节点 —— 两份副本"结构相等、引用不同",身份去重(runtime 用的那种)在这里等于没去重。单测用共享引用的 fixture 是过得去的假测试。改为按值去重(绑定对象 + name + body source),单测改成用两个克隆对象。复跑:恰好 1 条,带
Did you mean 'progress'?。还原后 showcase 本身零告警(真实 app 上无误报)。boot path
比 hook 侧紧一档:唯一适用的模式根在
ctx.api,所以 body 不含api标识符就连 parse 都不做,更别说加载 ~9 MB 的 TypeScript。lazy-deps.test.ts的 in-process 与两个 dist 探针都加了这条契约。spec
ScriptBodySchema与ActionSchema.body指向 action 侧规则,并写明ctx.input(params)与ctx.record(会被丢弃的快照)都不是记录写面。只动 JSDoc,没碰.describe()——check:generated8/8 全部 up to date。验证
@objectstack/linttest@objectstack/spectest@objectstack/clitesttypecheck@objectstack/spec check:generatedos validate🤖 Generated with Claude Code