Skip to content

Commit 53d37f1

Browse files
os-zhuangclaude
andauthored
docs: accept the hook-body write-set static-analysis gap (ADR-0107) (#3716)
Implements option 1 of #3700 (assessment §5 D4a): the hook-body write set is an accepted static-analysis gap — recorded in ADR-0107, documented in hook-bodies.mdx / hooks.mdx, stated in ScriptBodySchema's TSDoc, and marked decided in the reference-integrity assessment. The structured `writes` declaration stays open in #3700 as a deferred, evidence-gated proposal. Refs #3700, #3583 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EsRh53G7SMwFQL57BpFDsx
1 parent 6920c1c commit 53d37f1

6 files changed

Lines changed: 112 additions & 1 deletion

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-only: ADR-0107 accepts the hook-body write-set static-analysis gap (#3700). Records the decision, documents the coverage boundary in `content/docs/automation/hook-bodies.mdx` / `hooks.mdx`, and states the blind spot in `ScriptBodySchema`'s TSDoc. No runtime or API change; releases nothing.

content/docs/automation/hook-bodies.mdx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,22 @@ 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
120+
121+
Static validation around a hook is asymmetric, and it is worth knowing exactly where the line is:
122+
123+
- **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.
124+
- **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** (ADR-0107); the structured `writes` declaration that would close it is tracked in [#3700](https://github.com/objectstack-ai/objectstack/issues/3700).
126+
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.
128+
129+
Until a structured write declaration exists:
130+
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.
133+
- **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.
134+
119135
### Signature conventions
120136

121137
| Surface | TS authoring | Sandbox invocation |

content/docs/automation/hooks.mdx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ ctx = {
101101
- Use after hooks for related-record side effects
102102
- Use `ctx.api.object('x')` for cross-object access so writes stay in scope
103103
- Handle errors gracefully
104+
- Verify every field your hook writes exists on the target object(s) — the
105+
write set of a hook body is **not statically validated**, unlike `condition`
106+
(see [Hook & Action Bodies](/docs/automation/hook-bodies#not-statically-checked-the-write-set))
104107

105108
**DON'T:**
106109
- Query in loops
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
# ADR-0107: The Hook-Body Write Set Is an Accepted Static-Analysis Gap
2+
3+
**Status**: Proposed (2026-07-27)
4+
**Deciders**: ObjectStack Protocol Architects
5+
**Builds on**: [ADR-0049](./0049-no-unenforced-security-properties.md) (enforce-or-remove trichotomy — an uncoverable surface is *marked*, never silently assumed covered), [ADR-0072](./0072-reference-scope-and-resolvability.md) (D1 linter trust contract), [ADR-0078](./0078-no-silently-inert-metadata.md) (§4 registration-time diagnostics — deferred, evidence-gated)
6+
**Tracking**: [#3700](https://github.com/objectstack-ai/objectstack/issues/3700) (this decision + the deferred `writes` proposal), [#3583](https://github.com/objectstack-ai/objectstack/issues/3583) (HotCRM reference-integrity audit, category 5b), `docs/audits/2026-07-app-metadata-reference-integrity-assessment.md` §5 D4
7+
**Consumers**: `@objectstack/spec` (`ScriptBodySchema` TSDoc), `content/docs/automation/hook-bodies.mdx`, `@objectstack/lint` (explicitly **no new rule**)
8+
9+
---
10+
11+
## TL;DR
12+
13+
Of the nine HotCRM bug categories (#3583), eight have a static path — lint rules shipped, phased behind the assessment's ratchet, or blocked only on a spec decision (§5 D1/D2 there). The ninth does not and cannot: **a hook body that writes fields its target object doesn't have** (`contact_email` / `contact_phone` in the audit). The write set lives inside `body.source`, an opaque JS string (`packages/spec/src/data/hook-body.zod.ts`); no metadata anywhere declares which fields a hook writes, so there is nothing for a lint to check.
14+
15+
**Decision:** accept the gap and mark it loudly at every surface an author reads (D1); do **not** approximate coverage by guessing at JS source (D2); treat the structured `writes` declaration that *would* close it as a deferred, evidence-gated capability proposal — #3700 — not a lint chore (D3).
16+
17+
## Context
18+
19+
### Why the write side is dark when everything around it is lit
20+
21+
| Surface | Structured? | Static check |
22+
|---|---|---|
23+
| Hook read side — `hook.condition` (CEL) | yes (`ExpressionInputSchema`) | `validate-expressions.ts:255-288` checks fields against the target object(s), incl. array-valued `hook.object` |
24+
| Hook capability side — `body.capabilities` | yes (enum tokens) | declared in metadata; the sandbox enforces at call time |
25+
| Flow write side — `update_record` node `fields` | yes (per-field config) | `validate-readonly-flow-writes`, `validate-flow-template-paths` |
26+
| **Hook write side — assignments inside `body.source`** | **no — opaque `z.string()`** | **none possible** |
27+
28+
The flow rules work *because* flow nodes declare their writes structurally: the rules read node config, not JS. A hook body's writes (`ctx.input.x = …`, `ctx.api.object('y').update(id, { x: … })`) exist only as source text.
29+
30+
### What happens today when a body writes an unknown field
31+
32+
Nothing author-visible, until far too late:
33+
34+
1. The write-path validator skips unknown keys entirely — `validateRecord` walks *declared* fields on insert and `continue`s past unknown keys on update (`packages/objectql/src/validation/record-validator.ts:482-503`). That skip is deliberate PATCH tolerance; it is blind to this class.
35+
2. Unknown-field dropping exists only on the **read** path (the `find`/`findOne` projection filter in `packages/objectql/src/engine.ts`). The write path forwards the hook-mutated payload to the driver untouched.
36+
3. The driver decides the failure mode:
37+
- **SQL** (`driver-sql`): `formatInput` / `applyWriteColumnMap` pass unknown keys through, the database rejects the statement (`no such column`), and the whole operation fails at runtime — in production, far from the authoring mistake.
38+
- **Schemaless** (memory, MongoDB): the stray key is stored as-is — the worst outcome, indistinguishable from success (the ADR-0078 asymmetry: an AI author gets a success envelope and reports *done*).
39+
40+
## Decision
41+
42+
### D1 — Accept and mark (the ADR-0049 discipline)
43+
44+
Per the ADR-0049 trichotomy, a surface we cannot enforce is **marked**, never left for authors to assume covered. The gap is documented where authors actually meet it:
45+
46+
- `content/docs/automation/hook-bodies.mdx` — "Not statically checked: the write set" (exactly what is checked, what is not, what happens at runtime, what to do instead);
47+
- `ScriptBodySchema` TSDoc in `packages/spec/src/data/hook-body.zod.ts` — the schema is the contract, and the contract now states its own blind spot;
48+
- the reference-integrity assessment records its open decision D4 as decided.
49+
50+
### D2 — No heuristic source analysis (permanent posture, not a deferral)
51+
52+
We will not regex- or AST-guess the write set out of a Turing-complete body. `ctx.input[computeKey()] = v`, destructuring, aliasing (`const i = ctx.input`), helper indirection — ordinary code defeats the analysis. A partial rule is worse than none: it manufactures the belief that writes are checked, and the ADR-0072 D1 trust contract cuts both ways — false *confidence* is as corrosive as false positives. The assessment already took this posture (§7: "surfaces without structure are marked, not guessed at"); this ADR makes it permanent for hook bodies.
53+
54+
### D3 — The structured `writes` declaration is a capability proposal: deferred, evidence-gated
55+
56+
The only honest closure is contract-first (Prime Directive #12): the author declares the write set, lint verifies the declared fields exist, and the runtime rejects undeclared writes — the exact shape `body.capabilities` already has (declare → lint → sandbox-enforce). That is a genuine spec + runtime capability, not a lint rule:
57+
58+
- new spec surface — `writes` on the body (or hook), with semantics to design for multi-object / `"*"` targets and cross-object write sets;
59+
- runtime enforcement — a write-guard proxy over `ctx.input` plus per-object interception of `ctx.api` writes. Without enforcement the declaration itself would violate ADR-0049 (declared ≠ enforced);
60+
- migration for every existing body.
61+
62+
Deferred until the bug class recurs outside the HotCRM audit — the same evidence gate ADR-0078 §4 applies to registration-time diagnostics, which stay deferred there and are *not* re-decided here. #3700 remains open as the proposal's tracking issue. Per the assessment: "do not let this category stall the other eight" — the proposal must not be smuggled into a lint task.
63+
64+
### D4 — Author guidance (the mitigation that exists today)
65+
66+
- Every field a body assigns must exist on **each** target object (array and `"*"` hooks included). This is an author responsibility; no tool has your back here.
67+
- When the write set is fixed, prefer a **flow `update_record` node** — structural `fields` get the static checking hook bodies can't.
68+
- Exercise the hook against a real object before shipping: SQL drivers surface the mistake on first write; schemaless drivers won't.
69+
70+
## Consequences
71+
72+
- **Positive.** All nine HotCRM categories now carry an explicit disposition — eight checkable, one marked. An author reading the hook-body docs or the schema learns the exact boundary of static coverage instead of assuming lint sees everything. The `writes` proposal keeps a clean runway: capability-shaped, evidence-gated, undiluted by a rushed approximation.
73+
- **Negative.** The gap stays real: a typo'd write still ships silently on schemaless drivers and fails late on SQL. Accepting it means HotCRM-class write bugs remain possible until (and unless) #3700's proposal earns its evidence.
74+
75+
## Non-goals
76+
77+
- **Any lint rule over `body.source` write patterns** — including a "best effort" one (D2).
78+
- **Deciding the `writes` proposal.** This ADR only routes it: #3700, deferred, evidence-gated.
79+
- **Registration-time dev diagnostics** — already deferred by ADR-0078 §4; unchanged here.
80+
- **Field-level write authorization.** FLS / readonly enforcement (`stripReadonlyFields`, permission `editable` bits) governs *permitted* fields — a different axis from *existing* fields; untouched.

docs/audits/2026-07-app-metadata-reference-integrity-assessment.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ Field-level checks inside a cross-package object stay S3 (skip) — we cannot ju
115115
- **D1 — knowledge indexes/sources need a definition site or an experimental marker.** `agent.knowledge.indexes` references a namespace with no stack slot (`KnowledgeSourceSchema` is orphaned from `stack.zod.ts`). Per the ADR-0049/0078 trichotomy the current state — parsed, unmarked, unresolvable — is the prohibited fourth state. Either add `knowledgeSources` to the stack (spec change, ADR-worthy) or mark the fields `[EXPERIMENTAL — not enforced]`. R7 is blocked on this for the knowledge half only.
116116
- **D2 — runtime-registered skills/tools vs. static lint.** `ask`/`build` kernel agents register skills/tools via service at boot (same invisibility as plugin objects). Extend the Phase-1 registry with official skill/tool names, or run R7 at S2 advisory severity. Small decision; take it when R7 starts.
117117
- **D3 — where does nav-access belong?** R5-as-lint-warning (proposed) vs. folding into `compile`'s access-matrix snapshot gate. Lint wins on path symmetry (ADR-0078); the snapshot gate stays the drift detector.
118-
- **D4 — hook body writes are not statically checkable, say so.** The write set lives in opaque JS (`hook-body.zod.ts`). Options: (a) accept the gap and document it (hook writes are the one HotCRM category with no static answer); (b) a structured `writes: [field]` declaration on `HookSchema` that the runtime enforces (contract-first, but new spec surface + runtime work); (c) registration-time dev diagnostics (ADR-0078 §4, deferred/evidence-gated there). Recommendation: (a) now, file (b) as its own issue — do not let this category stall the other eight.
118+
- **D4 — hook body writes are not statically checkable, say so.** The write set lives in opaque JS (`hook-body.zod.ts`). Options: (a) accept the gap and document it (hook writes are the one HotCRM category with no static answer); (b) a structured `writes: [field]` declaration on `HookSchema` that the runtime enforces (contract-first, but new spec surface + runtime work); (c) registration-time dev diagnostics (ADR-0078 §4, deferred/evidence-gated there). Recommendation: (a) now, file (b) as its own issue — do not let this category stall the other eight. **Decided 2026-07-27 (#3700): (a).** The accepted gap is recorded in [ADR-0107](../adr/0107-hook-body-write-set-accepted-static-gap.md) and documented at the authoring surfaces (`content/docs/automation/hook-bodies.mdx` "Not statically checked: the write set"; `ScriptBodySchema` TSDoc). (b) stays open as #3700's deferred, evidence-gated proposal; (c) remains deferred under ADR-0078 §4, unchanged.
119119
- **D5 — rule-suite wiring drift.** `validate`/`lint`/`compile`/`doctor` each hand-pick rule subsets. A shared "run the reference-integrity suite" entry point in `@objectstack/lint` would make the next rule's wiring a one-liner and end the drift. Worth doing with Phase 2, not before.
120120

121121
---

packages/spec/src/data/hook-body.zod.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,14 @@ export type ExpressionBody = z.infer<typeof ExpressionBodySchema>;
6262
* - `process`, `globalThis`, `eval`, `new Function`
6363
* - any identifier resolved from a value-only top-level import
6464
*
65+
* **Write-set opacity — accepted static-analysis gap (ADR-0107, #3700).**
66+
* `source` is opaque to static analysis: no lint verifies that the fields the
67+
* body writes (`ctx.input.x = …`, `ctx.api.object('y').update(id, { x })`)
68+
* exist on the target object(s). Only the read side (`hook.condition`) and
69+
* the capability surface are statically checked. Write-target field existence
70+
* is the author's responsibility; when the write set is fixed, prefer a flow
71+
* `update_record` node, whose structural `fields` config IS lint-checked.
72+
*
6573
* @example
6674
* ```json
6775
* {

0 commit comments

Comments
 (0)