Skip to content

Commit 0449c63

Browse files
os-zhuangclaude
andauthored
docs(hook-bodies): the write set is statically checked now — rewrite 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>
1 parent a1b61e0 commit 0449c63

3 files changed

Lines changed: 53 additions & 15 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
---
3+
4+
docs(hook-bodies): the write-set section said the write side had no static checking, which #4305/#4344 made false (#4351) — rewritten as an accurate coverage description (three literal patterns, advisory `hook-body-write-unknown-field` / `action-body-write-unknown-field`, and the shapes the parser must skip, so a missing warning is not read as proof of correctness), the "accepted gap with no planned closure" conclusion retracted (#3700 stays accurate history; the gap was closed from the other end, by parsing the write set), the runtime paragraph corrected against measured behaviour, and the three now-stale cross-links in `hooks.mdx` updated to the renamed anchor. Releases nothing.

content/docs/automation/hook-bodies.mdx

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,20 +116,49 @@ The CLI builder **rejects** any source that uses:
116116

117117
Need outbound HTTP? Define a **Connector recipe** as metadata and call it via `ctx.connector(...)`. (Connector spec is tracked separately and ships after L1+L2 stabilises.)
118118

119-
### Not statically checked: the write set
119+
### Write-set checking
120120

121121
Static validation around a hook is asymmetric, and it is worth knowing exactly where the line is:
122122

123123
- **Checked — read side.** `hook.condition` is validated at build time against the target object's fields by the expression validator (`@objectstack/lint`), including array-valued `hook.object` targets. A condition referencing a nonexistent field fails the lint.
124124
- **Checked — capability side.** `body.capabilities` gates which `ctx` APIs the body may call at all; the sandbox throws on an undeclared call.
125-
- **Not checked — write side.** Nothing validates *which fields* your body writes. `body.source` is an opaque JS string: the fields it assigns (`ctx.input.foo = …`) or writes cross-object (`ctx.api.object('x').update(id, { foo })`) are not represented anywhere in metadata, so no lint can verify they exist. This is an **accepted gap** with no planned closure — a structured `writes` declaration was considered and dropped ([#3700](https://github.com/objectstack-ai/objectstack/issues/3700), closed as not planned).
125+
- **Checked — write side, advisory and literal-only.** Since [#4271](https://github.com/objectstack-ai/objectstack/issues/4271), `body.source` is **parsed** (never executed, never type-checked) and the field names it writes are resolved against the target object's declarations. An unknown field raises `hook-body-write-unknown-field` — a **warning** carrying a did-you-mean suggestion, which never blocks a build. Action bodies get the same check on their `ctx.api` writes (`action-body-write-unknown-field`). Both run under `os validate`, `os lint` and `os compile`.
126126

127-
Writing a field the target object doesn't have is not caught at runtime either — the write-path validator skips unknown keys. On a SQL driver the stray column reaches the database and the whole operation fails with a driver-level error, far from the authoring mistake; a schemaless driver (memory, MongoDB) silently persists the stray key.
127+
Three literal write shapes are recognized, and only these:
128128

129-
Because the write set carries no static checking:
129+
| Write shape | Hook body | Action body |
130+
|---|---|---|
131+
| `ctx.input.<field> = …` / `ctx.input['<field>'] ⟨op⟩= …` (including `+=`, `??=`, …) | checked | not checked — an action's `ctx.input` is its **params bag**, not a record |
132+
| `Object.assign(ctx.input, { <field>: … })` | checked | not checked — same surface |
133+
| `ctx.api.object('<literal>').insert\|create\|update({ <field>: … })`, `.updateById(id, { <field>: … })` | checked | checked |
134+
135+
**A missing warning is not a clean bill of health.** The rule bails *silently* on everything it cannot resolve statically, deliberately preferring a missed finding to a false one — a false positive kills an advisory lint, while a miss just leaves the gap open a little longer:
136+
137+
- computed keys (`ctx.input[k] = …`), spreads, and non-literal payloads;
138+
- dynamic object names (`ctx.api.object(name)`);
139+
- `ctx.input` writes in a wildcard (`object: '*'`) hook — there is no single target to resolve against;
140+
- multi-target hooks where the field exists on *some* target: a body may legitimately branch per object, so only a field missing on **every** named target is flagged;
141+
- objects declared by another package;
142+
- aliased input (`const doc = ctx.input; doc.x = 1`) — v1 does no data-flow analysis.
143+
144+
System/audit columns and the flat-input envelope keys (`id`, `options`, `ast`, `data`) are never flagged.
145+
146+
A structured `writes` declaration was considered and dropped ([#3700](https://github.com/objectstack-ai/objectstack/issues/3700), closed as not planned) — but the gap it left was closed from the other end, by parsing the write set out of the source. The practical consequence of that route: coverage is bounded by what a parser can see, not by what an author remembered to declare.
147+
148+
#### What still happens at runtime
149+
150+
An unknown field is **not** caught at runtime, and it does not fail quietly either. The write-path validator walks the object's *declared* fields, so an undeclared key is neither rejected nor stripped, and the sandbox's mutations are copied back onto the payload verbatim. What happens next is the driver's call:
151+
152+
- **SQL drivers** put the stray column into the statement, so the **whole write fails** with a driver-level error (`table deal has no column named stagee`) — nothing is stored, and the error surfaces far from the authoring mistake.
153+
- **Schemaless drivers** (memory, MongoDB) silently persist the stray key alongside the real ones.
154+
155+
Neither outcome is the one you wanted, and the advisory warning is the earliest signal you get.
156+
157+
Because the checking is advisory and literal-only:
130158

131-
- **Check your write targets by hand.** Every field the body assigns must exist on the target object(s) — for an array or `"*"` hook, on every target.
132-
- **Prefer a flow `update_record` node when the write set is fixed.** Flow nodes declare their writes structurally in `fields`, so they get the static checking hook bodies can't.
159+
- **Treat `hook-body-write-unknown-field` as a build failure by convention.** It does not gate, but the rule is tuned for near-zero false positives — in practice a warning is a real typo.
160+
- **Check by hand what the parser cannot see.** Computed keys, spreads, aliased input and dynamic object names are invisible to the rule; for an array or `"*"` hook, every field must exist on every target.
161+
- **Prefer a flow `update_record` node when the write set is fixed — but not for *this* check.** A flow node's writes are structured config: they diff field-by-field, render in the Console designer, and a write to a `readonly:true` field is a **gating error** (`flow-update-readonly-field`) that hooks have no counterpart for. What an `update_record` node does *not* get today is a field-existence check — writing a field the object never declares is currently reported by nothing at all. On that one axis an L2 body is now the better-checked surface.
133162
- **Exercise the hook against a real object before shipping** — on SQL drivers the mistake surfaces on the first write; schemaless drivers won't tell you.
134163

135164
### Signature conventions

content/docs/automation/hooks.mdx

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,15 @@ cannot express.
3333

3434
Two structural reasons to prefer the flow when either could work:
3535

36-
- **A flow's writes are validatable metadata; a hook body is opaque code.** An
36+
- **A flow's writes are structured metadata; a hook body is code.** An
3737
`update_record` node's `fields` is structural config that `os validate`
3838
checks — readonly targets, template dialects, declared expression slots. A
39-
hook body's write set is **not** statically checked (see
40-
[Hook & Action Bodies](/docs/automation/hook-bodies#not-statically-checked-the-write-set)):
41-
a misspelled field name runs green and writes nothing.
39+
hook body's write set is checked too, but only for the literal patterns a
40+
parser can recognise and only as an advisory warning (see
41+
[Hook & Action Bodies](/docs/automation/hook-bodies#write-set-checking)).
42+
Note the one axis where this reverses: writing a field the target object never
43+
declares warns in a hook body, and is reported nowhere in an `update_record`
44+
node.
4245
- **A flow reviews as data.** The node graph diffs field-by-field and renders
4346
in the Console designer with per-node run history; a hook body reviews as
4447
code only.
@@ -84,9 +87,10 @@ a hook can be, so review it as such:
8487
belongs behind a `condition` (evaluated before the handler) rather than an
8588
early `return` inside the body.
8689
- **Reads and writes both widen.** A wildcard hook holding an L2 body with
87-
`api.write` can write anywhere. Whether the fields it writes exist is
88-
[not statically checked](/docs/automation/hook-bodies#not-statically-checked-the-write-set),
89-
so a wildcard write set is the least verifiable shape in the system.
90+
`api.write` can write anywhere. Its `ctx.input` writes are also the one shape
91+
the [write-set lint](/docs/automation/hook-bodies#write-set-checking) has to
92+
skip — with no single target object there is nothing to resolve field names
93+
against — so a wildcard write set is the least verifiable shape in the system.
9094

9195
Where a wildcard is the honest answer, say so in `description` — it is the one
9296
place a reviewer can find out why the broad target was chosen.
@@ -179,8 +183,9 @@ ctx = {
179183
- Use `ctx.api.object('x')` for cross-object access so writes stay in scope
180184
- Handle errors gracefully
181185
- Verify every field your hook writes exists on the target object(s) — the
182-
write set of a hook body is **not statically validated**, unlike `condition`
183-
(see [Hook & Action Bodies](/docs/automation/hook-bodies#not-statically-checked-the-write-set))
186+
write-set lint catches the literal cases, but it is advisory, and computed
187+
keys, spreads and aliased input are invisible to it (see
188+
[Hook & Action Bodies](/docs/automation/hook-bodies#write-set-checking))
184189

185190
**DON'T:**
186191
- Query in loops

0 commit comments

Comments
 (0)