From 8fb1b96538dd602ea7cc000c9fa3478f8d89868b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 15:52:44 +0000 Subject: [PATCH 1/2] docs: accept the hook-body write-set static-analysis gap (ADR-0107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes out option 1 of #3700 (from the #3583 reference-integrity assessment, §5 D4): the set of fields a hook body writes lives in an opaque JS string, so unknown-field writes cannot be statically checked — and per the assessment's own posture, must be marked rather than guessed at. - ADR-0107 records the decision: accept + mark the gap (D1), never heuristically analyze body source (D2), and route the structured `writes` declaration as a deferred, evidence-gated capability proposal tracked in #3700 (D3), with author guidance (D4). - content/docs/automation/hook-bodies.mdx gains "Not statically checked: the write set" — the exact coverage boundary (condition and capabilities are checked; writes are not), the runtime failure modes (late SQL driver error vs. silent persistence on schemaless drivers), and what to do instead. - hooks.mdx best practices point at that section. - ScriptBodySchema TSDoc states the blind spot at the contract itself. - The assessment's open decision D4 is recorded as decided. No behavior change; documentation and TSDoc only. Refs #3700 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01EsRh53G7SMwFQL57BpFDsx --- content/docs/automation/hook-bodies.mdx | 16 ++++ content/docs/automation/hooks.mdx | 3 + ...hook-body-write-set-accepted-static-gap.md | 80 +++++++++++++++++++ ...metadata-reference-integrity-assessment.md | 2 +- packages/spec/src/data/hook-body.zod.ts | 8 ++ 5 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 docs/adr/0107-hook-body-write-set-accepted-static-gap.md diff --git a/content/docs/automation/hook-bodies.mdx b/content/docs/automation/hook-bodies.mdx index 02b0bc7fe8..44b4bbde86 100644 --- a/content/docs/automation/hook-bodies.mdx +++ b/content/docs/automation/hook-bodies.mdx @@ -116,6 +116,22 @@ 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 + +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** (ADR-0107); the structured `writes` declaration that would close it is tracked in [#3700](https://github.com/objectstack-ai/objectstack/issues/3700). + +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. + +Until a structured write declaration exists: + +- **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. +- **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 | Surface | TS authoring | Sandbox invocation | diff --git a/content/docs/automation/hooks.mdx b/content/docs/automation/hooks.mdx index a59f813e0b..29687f5fb9 100644 --- a/content/docs/automation/hooks.mdx +++ b/content/docs/automation/hooks.mdx @@ -101,6 +101,9 @@ ctx = { - Use after hooks for related-record side effects - 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)) ❌ **DON'T:** - Query in loops diff --git a/docs/adr/0107-hook-body-write-set-accepted-static-gap.md b/docs/adr/0107-hook-body-write-set-accepted-static-gap.md new file mode 100644 index 0000000000..2a09c950b1 --- /dev/null +++ b/docs/adr/0107-hook-body-write-set-accepted-static-gap.md @@ -0,0 +1,80 @@ +# ADR-0107: The Hook-Body Write Set Is an Accepted Static-Analysis Gap + +**Status**: Proposed (2026-07-27) +**Deciders**: ObjectStack Protocol Architects +**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) +**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 +**Consumers**: `@objectstack/spec` (`ScriptBodySchema` TSDoc), `content/docs/automation/hook-bodies.mdx`, `@objectstack/lint` (explicitly **no new rule**) + +--- + +## TL;DR + +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. + +**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). + +## Context + +### Why the write side is dark when everything around it is lit + +| Surface | Structured? | Static check | +|---|---|---| +| 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` | +| Hook capability side — `body.capabilities` | yes (enum tokens) | declared in metadata; the sandbox enforces at call time | +| Flow write side — `update_record` node `fields` | yes (per-field config) | `validate-readonly-flow-writes`, `validate-flow-template-paths` | +| **Hook write side — assignments inside `body.source`** | **no — opaque `z.string()`** | **none possible** | + +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. + +### What happens today when a body writes an unknown field + +Nothing author-visible, until far too late: + +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. +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. +3. The driver decides the failure mode: + - **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. + - **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*). + +## Decision + +### D1 — Accept and mark (the ADR-0049 discipline) + +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: + +- `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); +- `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; +- the reference-integrity assessment records its open decision D4 as decided. + +### D2 — No heuristic source analysis (permanent posture, not a deferral) + +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. + +### D3 — The structured `writes` declaration is a capability proposal: deferred, evidence-gated + +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: + +- new spec surface — `writes` on the body (or hook), with semantics to design for multi-object / `"*"` targets and cross-object write sets; +- 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); +- migration for every existing body. + +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. + +### D4 — Author guidance (the mitigation that exists today) + +- 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. +- When the write set is fixed, prefer a **flow `update_record` node** — structural `fields` get the static checking hook bodies can't. +- Exercise the hook against a real object before shipping: SQL drivers surface the mistake on first write; schemaless drivers won't. + +## Consequences + +- **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. +- **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. + +## Non-goals + +- **Any lint rule over `body.source` write patterns** — including a "best effort" one (D2). +- **Deciding the `writes` proposal.** This ADR only routes it: #3700, deferred, evidence-gated. +- **Registration-time dev diagnostics** — already deferred by ADR-0078 §4; unchanged here. +- **Field-level write authorization.** FLS / readonly enforcement (`stripReadonlyFields`, permission `editable` bits) governs *permitted* fields — a different axis from *existing* fields; untouched. diff --git a/docs/audits/2026-07-app-metadata-reference-integrity-assessment.md b/docs/audits/2026-07-app-metadata-reference-integrity-assessment.md index 36a23f6c1f..1e1532d419 100644 --- a/docs/audits/2026-07-app-metadata-reference-integrity-assessment.md +++ b/docs/audits/2026-07-app-metadata-reference-integrity-assessment.md @@ -115,7 +115,7 @@ Field-level checks inside a cross-package object stay S3 (skip) — we cannot ju - **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. - **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. - **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. -- **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. +- **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. - **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. --- diff --git a/packages/spec/src/data/hook-body.zod.ts b/packages/spec/src/data/hook-body.zod.ts index b06458ed4f..b61c1bd415 100644 --- a/packages/spec/src/data/hook-body.zod.ts +++ b/packages/spec/src/data/hook-body.zod.ts @@ -62,6 +62,14 @@ export type ExpressionBody = z.infer; * - `process`, `globalThis`, `eval`, `new Function` * - any identifier resolved from a value-only top-level import * + * **Write-set opacity — accepted static-analysis gap (ADR-0107, #3700).** + * `source` is opaque to static analysis: no lint verifies that the fields the + * body writes (`ctx.input.x = …`, `ctx.api.object('y').update(id, { x })`) + * exist on the target object(s). Only the read side (`hook.condition`) and + * the capability surface are statically checked. Write-target field existence + * is the author's responsibility; when the write set is fixed, prefer a flow + * `update_record` node, whose structural `fields` config IS lint-checked. + * * @example * ```json * { From 8ba5b464eb902ca10746769ba35bd4e2d79bccdc Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 15:56:04 +0000 Subject: [PATCH 2/2] =?UTF-8?q?chore:=20add=20empty=20changeset=20?= =?UTF-8?q?=E2=80=94=20docs-only=20PR,=20releases=20nothing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Check Changeset gate requires every PR to add a changeset; an empty-frontmatter changeset is its sanctioned "this PR releases nothing" declaration. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01EsRh53G7SMwFQL57BpFDsx --- .changeset/adr-0107-hook-body-write-set-accepted-gap.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .changeset/adr-0107-hook-body-write-set-accepted-gap.md diff --git a/.changeset/adr-0107-hook-body-write-set-accepted-gap.md b/.changeset/adr-0107-hook-body-write-set-accepted-gap.md new file mode 100644 index 0000000000..276a6aa39f --- /dev/null +++ b/.changeset/adr-0107-hook-body-write-set-accepted-gap.md @@ -0,0 +1,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.