From 5644d9f245dbba2fb2db553201d7c11bb3bac015 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 01:00:25 +0000 Subject: [PATCH] docs: withdraw ADR-0107, drop the structured writes proposal (#3700) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Owner decision: the structured writes declaration will not be built — it duplicates the write set in a second place authors must keep in sync, adding friction and a new error surface, while testing hooks against a SQL driver (in-memory SQLite) already surfaces the failure mode that schemaless memory:// green lights hide. - delete docs/adr/0107-hook-body-write-set-accepted-static-gap.md and its changeset; the author-facing documentation of the gap (hook-bodies.mdx, hooks.mdx best practice, ScriptBodySchema TSDoc) stays — it is the whole disposition - strip now-dangling ADR-0107/#3700 references from hook-bodies.mdx and the TSDoc; reword the mitigation intro that implied a declaration was coming - audit assessment §5 D4: record the revision — (b) dropped, #3700 closed as not planned; (c) unchanged under ADR-0078 §4 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01EsRh53G7SMwFQL57BpFDsx --- ...r-0107-hook-body-write-set-accepted-gap.md | 4 - .../withdraw-adr-0107-drop-writes-proposal.md | 4 + content/docs/automation/hook-bodies.mdx | 4 +- ...hook-body-write-set-accepted-static-gap.md | 80 ------------------- ...metadata-reference-integrity-assessment.md | 2 +- packages/spec/src/data/hook-body.zod.ts | 2 +- 6 files changed, 8 insertions(+), 88 deletions(-) delete mode 100644 .changeset/adr-0107-hook-body-write-set-accepted-gap.md create mode 100644 .changeset/withdraw-adr-0107-drop-writes-proposal.md delete mode 100644 docs/adr/0107-hook-body-write-set-accepted-static-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 deleted file mode 100644 index 276a6aa39f..0000000000 --- a/.changeset/adr-0107-hook-body-write-set-accepted-gap.md +++ /dev/null @@ -1,4 +0,0 @@ ---- ---- - -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. diff --git a/.changeset/withdraw-adr-0107-drop-writes-proposal.md b/.changeset/withdraw-adr-0107-drop-writes-proposal.md new file mode 100644 index 0000000000..12a5c0e816 --- /dev/null +++ b/.changeset/withdraw-adr-0107-drop-writes-proposal.md @@ -0,0 +1,4 @@ +--- +--- + +Docs-only: withdraw ADR-0107 and drop the structured `writes` proposal (#3700, closed as not planned). The hook-body write-set gap stays documented at the authoring surfaces (`hook-bodies.mdx`, `ScriptBodySchema` TSDoc) without a decision-record apparatus. No runtime or API change; releases nothing. diff --git a/content/docs/automation/hook-bodies.mdx b/content/docs/automation/hook-bodies.mdx index 44b4bbde86..d33b3307a8 100644 --- a/content/docs/automation/hook-bodies.mdx +++ b/content/docs/automation/hook-bodies.mdx @@ -122,11 +122,11 @@ Static validation around a hook is asymmetric, and it is worth knowing exactly w - **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). +- **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). 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: +Because the write set carries no static checking: - **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. 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 deleted file mode 100644 index 2a09c950b1..0000000000 --- a/docs/adr/0107-hook-body-write-set-accepted-static-gap.md +++ /dev/null @@ -1,80 +0,0 @@ -# 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 1e1532d419..a930d01d4c 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. **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. +- **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); revised 2026-07-28 — (b) dropped, #3700 closed as not planned.** The gap stays documented at the authoring surfaces (`content/docs/automation/hook-bodies.mdx` "Not statically checked: the write set"; `ScriptBodySchema` TSDoc); the documentation is the whole disposition, so ADR-0107, which only recorded it, was withdrawn. (b) will not be built — the declaration duplicates the write set in a second place the author must keep in sync, adding authoring friction and a new error surface (worst for AI authors); in practice, exercising hooks against a SQL driver (in-memory SQLite) covers the failure mode that schemaless `memory://` green lights hide. (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 b61c1bd415..9374851319 100644 --- a/packages/spec/src/data/hook-body.zod.ts +++ b/packages/spec/src/data/hook-body.zod.ts @@ -62,7 +62,7 @@ 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).** + * **Write-set opacity — accepted static-analysis gap.** * `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