Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .changeset/hook-bodies-write-set-docs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,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.
41 changes: 35 additions & 6 deletions content/docs/automation/hook-bodies.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -116,20 +116,49 @@ The CLI builder **rejects** any source that uses:

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.)

### Not statically checked: the write set
### Write-set checking

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

- **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.
- **Checked — capability side.** `body.capabilities` gates which `ctx` APIs the body may call at all; the sandbox throws on an undeclared call.
- **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).
- **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`.

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.
Three literal write shapes are recognized, and only these:

Because the write set carries no static checking:
| Write shape | Hook body | Action body |
|---|---|---|
| `ctx.input.<field> = …` / `ctx.input['<field>'] ⟨op⟩= …` (including `+=`, `??=`, …) | checked | not checked — an action's `ctx.input` is its **params bag**, not a record |
| `Object.assign(ctx.input, { <field>: … })` | checked | not checked — same surface |
| `ctx.api.object('<literal>').insert\|create\|update({ <field>: … })`, `.updateById(id, { <field>: … })` | checked | checked |

**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:

- computed keys (`ctx.input[k] = …`), spreads, and non-literal payloads;
- dynamic object names (`ctx.api.object(name)`);
- `ctx.input` writes in a wildcard (`object: '*'`) hook — there is no single target to resolve against;
- 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;
- objects declared by another package;
- aliased input (`const doc = ctx.input; doc.x = 1`) — v1 does no data-flow analysis.

System/audit columns and the flat-input envelope keys (`id`, `options`, `ast`, `data`) are never flagged.

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.

#### What still happens at runtime

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:

- **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.
- **Schemaless drivers** (memory, MongoDB) silently persist the stray key alongside the real ones.

Neither outcome is the one you wanted, and the advisory warning is the earliest signal you get.

Because the checking is advisory and literal-only:

- **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.
- **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.
- **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.
- **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.
- **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.
- **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.

### Signature conventions
Expand Down
23 changes: 14 additions & 9 deletions content/docs/automation/hooks.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,15 @@ cannot express.

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

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

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

❌ **DON'T:**
- Query in loops
Expand Down
Loading