Skip to content

Commit 78caf51

Browse files
os-zhuangclaude
andauthored
fix(lint): write-set diagnostics describe the real driver split, not 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>
1 parent d92c72d commit 78caf51

9 files changed

Lines changed: 300 additions & 38 deletions

.changeset/action-body-write-set-lint.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@ that carries one. An action body is the same artefact: the same
1010
`HookBodySchema` union, parsed by the same `HookBodySchema.safeParse` in
1111
`actionBodyRunnerFactory`, run in the same QuickJS sandbox. So it fails the
1212
same way — `ctx.api.object('crm_deal').update({ stag: 'won' })` inside an
13-
action succeeds, returns success to the caller, and the unknown column simply
14-
never lands. Half the surface was still blind.
13+
action reaches the driver unfiltered, and the outcome splits by driver: on SQL
14+
the stray column fails the whole call with a driver-level error far from the
15+
authoring site, and on a schemaless driver the stray key is persisted. Half
16+
the surface was still blind.
1517

1618
**New rule — `action-body-write-unknown-field` (advisory).** Wired into
1719
`REFERENCE_INTEGRITY_RULES`, so `os validate`, `os lint` and `os compile` all
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
"@objectstack/lint": patch
3+
"@objectstack/spec": patch
4+
---
5+
6+
fix(lint): the write-set diagnostics describe what the runtime actually does (#4271)
7+
8+
`hook-body-write-unknown-field` and `action-body-write-unknown-field` told
9+
authors the undeclared column "silently never lands in the stored record".
10+
Measured on `main`, that is wrong in **both** directions. Nothing between the
11+
body and the driver filters the key — `applyMutationsToInput` is a plain
12+
`Object.assign`, and `validateRecord` walks declared fields on insert and
13+
`continue`s past a key with no field def on update — so the driver decides:
14+
15+
- **SQL** — the stray column enters the statement and the **whole write
16+
fails** with a driver-level error (`table deal has no column named stagee`).
17+
Nothing is stored, so the correctly-spelled fields of that row are lost too,
18+
and the error names a column far from the body that wrote it.
19+
- **Schemaless** (memory, MongoDB — both spread the payload without consulting
20+
the declared field set) — the stray key **is** persisted, as an undeclared
21+
column nothing downstream reads.
22+
23+
A lint that misdescribes the failure it is warning about teaches the wrong
24+
debugging instinct: an author told the value silently vanishes will not connect
25+
the driver error they actually see to the typo that caused it, and on a
26+
schemaless driver will not go looking for the stray key that is really there.
27+
All three messages now state the split, matching the "What still happens at
28+
runtime" description #4355 gave `content/docs/automation/hook-bodies.mdx`.
29+
30+
Both outcomes are pinned by a new integration test —
31+
`runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts`.
32+
Its insert cases run the full chain (real QuickJS sandbox, real hook body, real
33+
engine, real driver against a real SQLite table), so "reaches the driver
34+
unfiltered" is proved rather than asserted: if anything on that path ever
35+
learns to filter, the SQL half stops throwing and the test goes red. The rule
36+
headers, the `ScriptBodySchema` / `ActionSchema.body` notes and the two
37+
still-unreleased #4271 changesets are corrected to match. #4355 fixed the
38+
prose docs; this is the same correction on the surfaces that ship in the
39+
packages — the diagnostic an author actually reads, and a test that pins it.
40+
41+
`@objectstack/spec`: doc comments only — no schema or generated-artifact change.

.changeset/hook-body-write-set-lint.md

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,23 @@ feat(lint): L2 hook-body writes to undeclared fields warn at author time (#4271)
77

88
An L2 (`language:'js'`) hook body that writes a field the target object never
99
declares — `ctx.input.amout = 0`, `ctx.api.object('deal').update({ stag: … })`
10-
— runs clean in the QuickJS sandbox, reports success, and the unknown column
11-
simply never lands in the stored record. No diagnostic anywhere: the #4001
12-
"silent no-op manufactures false completion" failure mode at the
13-
runtime-expression layer. The read side (`hook.condition`) and the capability
14-
surface were already statically checked; the write side was the one blind face,
15-
and `hook-body.zod.ts` carried it as an **accepted gap**.
10+
— runs clean in the QuickJS sandbox and reaches the driver **unfiltered**:
11+
`applyMutationsToInput` is a plain `Object.assign`, and the write-path
12+
validator walks declared fields on insert and skips a key it has no field def
13+
for on update. What happens next depends on the driver, and neither half is
14+
acceptable:
15+
16+
- **SQL** — the stray column enters the statement and the **whole write fails**
17+
with a driver-level error (`table deal has no column named stagee`). The
18+
write is lost, and the error surfaces far from the mistake that caused it.
19+
- **Schemaless** (memory, MongoDB) — the driver spreads the payload, so the
20+
stray key **is** persisted: an undeclared column nothing downstream reads.
21+
22+
No diagnostic anywhere, and nothing at the authoring site either way — the
23+
#4001 "the mistake is invisible where it is made" family. The read side
24+
(`hook.condition`) and the capability surface were already statically checked;
25+
the write side was the one blind face, and `hook-body.zod.ts` carried it as an
26+
**accepted gap**.
1627

1728
**New rule — `hook-body-write-unknown-field` (advisory).** `@objectstack/lint`
1829
now parses each L2 body (TypeScript parser; parsed, never executed, never

packages/lint/src/reference-integrity-suite.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,9 @@ describe('reference-integrity suite — every member actually runs', () => {
6363
// validateObjectReferences: a param pointing at an object nothing declares.
6464
{ name: 'assign', label: 'Assign', params: [{ name: 'owner', reference: 'user' }] },
6565
// validateActionBodyWrites, both of its rule ids from one body:
66-
// - the L2 body persists a field crm_lead does not declare — the action
67-
// returns success and the column never lands (#4271);
66+
// - the L2 body writes a field crm_lead does not declare — on SQL that
67+
// fails the whole call at the driver, on a schemaless driver it
68+
// persists a stray key (#4271);
6869
// - and it assigns ctx.record, a snapshot the runtime never writes
6970
// back, without ever passing it anywhere (#4345).
7071
{
@@ -143,7 +144,8 @@ describe('reference-integrity suite — every member actually runs', () => {
143144
skills: [{ name: 'metadata_authoring', surface: 'build', tools: ['forecast_revenue'] }],
144145
hooks: [
145146
// validateHookBodyWrites: the L2 body writes a field crm_lead does not
146-
// declare — runs clean in the sandbox, never lands in the record (#4271).
147+
// declare — runs clean in the sandbox, then fails the whole write at a
148+
// SQL driver / persists a stray key on a schemaless one (#4271).
147149
{
148150
name: 'score_lead',
149151
object: 'crm_lead',

packages/lint/src/validate-action-body-writes.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,14 @@
66
// An action body is the same artefact as a hook body: the same
77
// `HookBodySchema` union, parsed by the same `HookBodySchema.safeParse` in
88
// `actionBodyRunnerFactory` (packages/runtime/src/sandbox/body-runner.ts), run
9-
// in the same QuickJS sandbox. So it fails the same way — an action that
10-
// persists a field the target object never declares runs clean, returns
11-
// success to the caller, and the unknown column simply never lands. Same
12-
// silent no-op, same #4001 family; the hook rule alone left half the surface
13-
// uncovered.
9+
// in the same QuickJS sandbox. So it fails the same way — an action body that
10+
// writes a field the target object never declares reaches the driver
11+
// unfiltered, and the outcome is DRIVER-DEPENDENT: on SQL the stray column
12+
// fails the whole call with a driver-level error far from the authoring
13+
// mistake, on a schemaless driver the stray key is persisted. Same #4271
14+
// split as the hook side (see that file's header for the measured chain, and
15+
// `undeclared-field-write-driver-split.integration.test.ts` for the pin); the
16+
// hook rule alone left half the surface uncovered.
1417
//
1518
// ─── What does NOT carry over ───────────────────────────────────────────────
1619
//
@@ -331,8 +334,9 @@ export function validateActionBodyWrites(stack: AnyRec): ActionBodyWriteFinding[
331334
path: site.path,
332335
message:
333336
`body calls ctx.api.object('${w.object}').${w.method ?? 'update'}(…) writing '${w.field}', but ` +
334-
`object '${w.object}' declares no such field. The action returns success while the unknown column ` +
335-
`silently never lands (#4271).`,
337+
`object '${w.object}' declares no such field. The write-path validator skips the unknown key — ` +
338+
`on a SQL driver the whole action then fails with a driver-level error far from here; on a ` +
339+
`schemaless driver (memory, MongoDB) the stray key is persisted (#4271).`,
336340
hint: fixHint(w.field, [...known]),
337341
});
338342
}

packages/lint/src/validate-hook-body-writes.ts

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,28 @@
44
//
55
// An L2 body that writes a field the target object never declares —
66
// `ctx.input.amout = 0`, `ctx.api.object('deal').update({ stag: 'won' })` —
7-
// runs clean in the QuickJS sandbox, reports success, and the unknown column
8-
// simply never lands in the stored record. No diagnostic anywhere: the exact
9-
// "silent no-op manufactures false completion" failure mode of #4001, at the
10-
// runtime-expression layer. The read side (`hook.condition`, ADR-0032) and the
11-
// capability surface are statically checked; until this rule, the write side
12-
// was the one blind face (the gap `hook-body.zod.ts` used to carry as
13-
// "accepted").
7+
// runs clean in the QuickJS sandbox and reaches the driver UNFILTERED:
8+
// `applyMutationsToInput` (runtime/src/sandbox/body-runner.ts) is a plain
9+
// `Object.assign`, and `validateRecord` walks declared fields on insert and
10+
// `continue`s past a key with no field def on update. What happens after that
11+
// is DRIVER-DEPENDENT, and neither half is acceptable:
12+
//
13+
// • SQL — the stray column enters the knex statement and the WHOLE write
14+
// fails with a driver-level error (`table deal has no column named
15+
// stagee`). The write is lost, and the error surfaces far from the
16+
// authoring mistake that caused it.
17+
// • Schemaless (memory, MongoDB) — the driver spreads the payload, so the
18+
// stray key IS persisted: an undeclared column nothing downstream reads.
19+
//
20+
// Either way the mistake is invisible where it is MADE — the #4001 family, if
21+
// not literally its silent-no-op shape. Both runtime outcomes are pinned by
22+
// `runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts`
23+
// so this rule's wording cannot drift from what the runtime does; the same
24+
// split is documented in `content/docs/automation/hook-bodies.mdx`.
25+
//
26+
// The read side (`hook.condition`, ADR-0032) and the capability surface are
27+
// statically checked; until this rule, the write side was the one blind face
28+
// (the gap `hook-body.zod.ts` used to carry as "accepted").
1429
//
1530
// Scope — the literal write patterns in {@link HOOK_BODY_WRITE_PATTERNS}, and
1631
// nothing else. The body is PARSED (TypeScript parser, never executed, never
@@ -586,8 +601,9 @@ export function validateHookBodyWrites(stack: AnyRec): HookBodyWriteFinding[] {
586601
path,
587602
message:
588603
`body writes '${w.field}' to its input, but ${objDesc} ${declares}. The sandboxed script runs ` +
589-
`clean and the value is copied back onto the record payload — then the unknown column silently ` +
590-
`never lands in the stored record (#4271).`,
604+
`clean and the value is copied back onto the record payload unfiltered — on a SQL driver the ` +
605+
`stray column then fails the WHOLE write with a driver-level error far from here; on a ` +
606+
`schemaless driver (memory, MongoDB) it is persisted as an undeclared key (#4271).`,
591607
hint: fixHint(w.field, unionCandidates(targetSets)),
592608
});
593609
} else {
@@ -604,8 +620,9 @@ export function validateHookBodyWrites(stack: AnyRec): HookBodyWriteFinding[] {
604620
path,
605621
message:
606622
`body calls ctx.api.object('${w.object}').${w.method ?? 'update'}(…) writing '${w.field}', but ` +
607-
`object '${w.object}' declares no such field. The call succeeds while the unknown column silently ` +
608-
`never lands (#4271).`,
623+
`object '${w.object}' declares no such field. The write-path validator skips the unknown key — ` +
624+
`on a SQL driver the whole call then fails with a driver-level error far from here; on a ` +
625+
`schemaless driver (memory, MongoDB) the stray key is persisted (#4271).`,
609626
hint: fixHint(w.field, [...known]),
610627
});
611628
}

0 commit comments

Comments
 (0)