feat(lint): action body 里落不了地的 ctx.record 写在作者时告警 (#4345) (#4362)

* feat(lint): action body 里落不了地的 ctx.record 写在作者时告警 (#4345)

#4344 deliberately left `ctx.record` alone and said why: an action's
`ctx.record` is a plain snapshot (`unwrapProxyToPlain(actionCtx?.record)`) that
`boundActionHandler` never writes back — the hook path's
`applyMutationsToInput` has no action-side counterpart — so `ctx.record.x = …`
is discarded for DECLARED and undeclared fields alike. Reporting that through
the unknown-field rule would have been actively wrong: flagging only the
undeclared half implies the declared half persists. It needed its own finding.

New rule `action-record-write-discarded`, advisory.

It is NOT "flag every ctx.record assignment" — mutating the snapshot to build a
payload is legitimate:

    ctx.record.stage = 'won';
    await ctx.api.object('crm_deal').update(ctx.record);   // LIVE

So the finding requires the write to be provably dead: `ctx.record` never
escapes the body as a value. Property reads do not rescue a write; handing the
object to anything — argument, assignment RHS, spread, return — does. Aliasing
reads as an escape, the safe direction.

Truthiness and type tests are NOT escapes, and that is what makes the rule fire
on real code. Running it against the showcase surfaced it: `mark_done` opens
with `ctx.recordId || (ctx.record && ctx.record.id)` — the idiom action bodies
are actually written with — and counting that guard as an escape silenced the
finding on the one body in the repo that had a record write. Only the LEFT
operand of &&/||/?? is a test; `x || ctx.record` still escapes.

One suite member, two rule ids: both findings fall out of one parse of one
source on one surface. A second REFERENCE_INTEGRITY_RULES member would parse
every action body twice to say two things about one walk, and hand-wiring it
into the three CLI commands is the drift that suite exists to end —
validateReadonlyFlowWrites is the standing proof, wired into validate and
compile but never into lint. Trade-off written down at both ends.

The ledger ratchet fired as designed. `record-property-assign` joins the shared
HOOK_BODY_WRITE_PATTERNS — the extractor's shape inventory, not any one rule's
— and both consumers had to classify it first. Not cosmetic on the hook side: a
record write carries no `object`, and validateHookBodyWrites branched on
exactly that to mean "a ctx.input write", so the new shape would have been
reported as "the hook writes 'stage' to its input". The hook rule now declares
its own consumed subset and its exclusion with a reason — a hook sandbox
context has no ctx.record at all, so the expression throws at run time rather
than silently no-op'ing, and a loud failure is not an advisory rule's business.

extractHookBodyWriteSet is the new one-parse entry point (writes +
ctxRecordEscapes); extractHookBodyWrites stays a thin projection.

Boot path: the action prefilter widens from `api` to `api`-or-`record`, so a
body reaching neither still never loads the ~9 MB TypeScript compiler.
lazy-deps.test.ts pins it — and its header plus two case names, which still
claimed every lazy dep waited on "a react page", now name the trigger each one
pins (typescript has also been loaded by the hook-body gate since #4271).

spec/runtime: ScriptBodySchema, ActionSchema.body and ScriptContext.record now
state that ctx.api.object(...) is the only path that persists anything and that
ctx.record is read-only in effect. Doc comments only — all 8 generated
artifacts verified unchanged. Whether the runtime should instead refuse or
honour a record write stays open on #4345.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* Merge origin/main; document the ctx.record write in the section #4351 just rewrote

#4351/#4355 rewrote content/docs/automation/hook-bodies.mdx to stop claiming the
write side was unchecked. That section lands squarely on this branch's subject,
so it is updated here rather than left false for a second time: the write-shape
table gains the ctx.record row (and is no longer 'three shapes'), the silent-bail
list gains the escape rule, and Signature conventions now says outright that an
action's ctx.record is read-only in effect and ctx.api.object(...) is the one way
a body persists anything.

---------

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: os-zhuang

.changeset/action-record-write-discarded-lint.md

@@ -0,0 +1,86 @@
+---
+"@objectstack/lint": minor
+"@objectstack/spec": patch
+"@objectstack/runtime": patch
+---
+
+feat(lint): an action body's discarded `ctx.record` write warns at author time (#4345)
+
+`#4344` deliberately left `ctx.record` alone, and said why: an action's
+`ctx.record` is a plain snapshot (`unwrapProxyToPlain(actionCtx?.record)`) that
+`boundActionHandler` never writes back — the hook path's
+`applyMutationsToInput` has no action-side counterpart — so `ctx.record.x = …`
+is discarded for **declared and undeclared fields alike**. Reporting that
+through the unknown-field rule would have been actively wrong: flagging only
+the undeclared half implies the declared half persists, which is the false
+completion this rule family exists to stop manufacturing. It needed its own
+finding, and now has one.
+
+**New rule — `action-record-write-discarded` (advisory).**
+
+**It is not "flag every `ctx.record.<field>` assignment"** — that would be a
+false-positive machine, because mutating the snapshot to build a payload is a
+legitimate idiom:
+
+```js
+ctx.record.stage = 'won';
+await ctx.api.object('crm_deal').update(ctx.record);   // the write is LIVE
+```
+
+So the finding requires the write to be **provably dead**: reported only when
+`ctx.record` never escapes the body as a value. Property reads
+(`ctx.record.id`) do not rescue a write and do not suppress the finding;
+handing the object to anything — an argument, an assignment RHS, a spread, a
+return — does. Aliasing (`const r = ctx.record`) reads as an escape, which is
+the safe direction: it costs a missed finding, never a false one.
+
+Truthiness and type tests are **not** escapes, and that distinction is what
+makes the rule fire on real code rather than almost never. Running it against
+the showcase app is what surfaced it: `mark_done` opens with
+`ctx.recordId || (ctx.record && ctx.record.id)`, the defensive idiom action
+bodies are actually written with, and counting that guard as an escape silenced
+the finding on the one body in the repo that had a record write. A test reads
+the reference and yields a boolean — or, for `&&`/`||`/`??`, yields the left
+operand only when it is falsy, which is null or undefined and persists nothing.
+Only the LEFT operand is a test: `x || ctx.record` really does evaluate to the
+object, and still escapes.
+
+**One suite member, two rule ids.** Both findings fall out of one parse of one
+source on one surface, so `validateActionBodyWrites` reports both rather than
+`REFERENCE_INTEGRITY_RULES` growing a second member that would parse every
+action body again to say two things about the same walk. The alternative —
+hand-wiring it into the three CLI commands — is the drift that suite exists to
+end, and `validateReadonlyFlowWrites` is the standing proof: wired into
+`validate` and `compile`, never into `lint`. The trade-off is written down at
+both ends rather than left to be rediscovered.
+
+**The ledger ratchet fired, as designed.** `record-property-assign` joins the
+shared `HOOK_BODY_WRITE_PATTERNS` — the extractor's shape inventory, not any
+one rule's — and both existing consumers had to classify it before it could
+land. That was not cosmetic on the hook side: a `record-property-assign` write
+carries no `object`, and `validateHookBodyWrites` branched on exactly that to
+mean "a `ctx.input` write", so the new shape would have been reported as *"the
+hook writes 'stage' to its input"*. The hook rule now declares its own
+consumed subset (`HOOK_BODY_WRITE_PATTERN_IDS`) and its exclusion with a
+reason — a hook sandbox context has no `ctx.record` at all
+(`buildSandboxContext` never sets it), so the expression throws at run time
+rather than silently no-op'ing, and a loud failure is not an advisory rule's
+business.
+
+`extractHookBodyWriteSet` is the new one-parse entry point, returning the
+writes plus the `ctxRecordEscapes` signal; `extractHookBodyWrites` stays as a
+thin projection of it.
+
+**Boot path.** The action gate's prefilter widens from `api` to `api`-or-
+`record`, so a body reaching neither still never loads the ~9 MB TypeScript
+compiler. `lazy-deps.test.ts` pins it — and its header and two case names,
+which still claimed every lazy dep waited on "a react page", now say which
+trigger each one pins (typescript has also been loaded by the hook-body gate
+since #4271).
+
+`@objectstack/spec` / `@objectstack/runtime`: `ScriptBodySchema`,
+`ActionSchema.body` and `ScriptContext.record` now state that
+`ctx.api.object(...)` is the only path that persists anything, and that
+`ctx.record` is read-only in effect. Doc comments only — no schema or
+generated-artifact change. Whether the runtime should instead refuse or honour
+a record write stays open on #4345.

content/docs/automation/hook-bodies.mdx

@@ -123,14 +123,16 @@ 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.
 - **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`.
+- **Checked — writes that reach nothing at all.** Since [#4345](https://github.com/objectstack-ai/objectstack/issues/4345), an action body assigning to `ctx.record` raises `action-record-write-discarded`, also a warning. This one is **not** a field-resolution question: an action's `ctx.record` is a snapshot the runtime never writes back, so the assignment is discarded whether or not the field is declared — see [Signature conventions](#signature-conventions) below.
 
-Three literal write shapes are recognized, and only these:
+Four literal write shapes are recognized, and only these:
 
 | 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 |
+| `ctx.record.<field> = …` / `ctx.record['<field>'] ⟨op⟩= …` | n/a — a hook context has no `ctx.record` (the expression throws) | checked: warns as **discarded**, declared field or not |
 
 **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:
 
@@ -139,7 +141,8 @@ Three literal write shapes are recognized, and only these:
 - `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.
+- aliased input (`const doc = ctx.input; doc.x = 1`) — v1 does no data-flow analysis;
+- `ctx.record` writes in a body that hands `ctx.record` to anything — an argument, an assignment RHS, a spread, a return. Mutating the snapshot and then persisting it (`ctx.record.stage = 'won'; await ctx.api.object('crm_deal').update(ctx.record)`) is a **live** payload, so the whole body's record writes are skipped rather than guessed at. Truthiness and type guards (`ctx.record && ctx.record.id`, `if (!ctx.record) …`) are not escapes — they cannot persist anything.
 
 System/audit columns and the flat-input envelope keys (`id`, `options`, `ast`, `data`) are never flagged.
 
@@ -170,6 +173,14 @@ Because the checking is advisory and literal-only:
 
 Hooks mutate `ctx.input`/`ctx.result`; actions return their output value explicitly.
 
+An action's `ctx` is **not** a hook's. `ctx.input` is the action's **params bag** — validated against its declared `params`, not a record. `ctx.record` is the record the dispatcher pre-fetched, and it is **read-only in effect**: the sandbox receives a plain snapshot and the runtime never writes it back, so `ctx.record.<field> = …` is discarded even for a perfectly valid field name. There is exactly one way an action body persists anything:
+
+```js
+await ctx.api.object('crm_deal').updateById(ctx.recordId, { stage: 'won' });
+```
+
+Mutating the snapshot *as a payload* and then handing it to such a call is fine — that write is live, and the lint leaves it alone.
+
 ### Engine
 
 The sandbox engine is **`quickjs-emscripten`** — pure-WASM, runs on every JS host. We considered `isolated-vm` but its native dependency disqualifies edge targets. The choice is hidden behind the `ScriptRunner` interface in `packages/runtime/src/sandbox/`, so a node-only deployment can swap in a faster engine later without touching call sites.

packages/lint/src/index.ts

@@ -261,14 +261,19 @@ export type {
 export {
   validateHookBodyWrites,
   extractHookBodyWrites,
+  extractHookBodyWriteSet,
   HOOK_BODY_WRITE_PATTERNS,
+  HOOK_BODY_WRITE_PATTERN_IDS,
+  HOOK_BODY_WRITE_EXCLUSIONS,
   HOOK_BODY_WRITE_UNKNOWN_FIELD,
 } from './validate-hook-body-writes.js';
 export type {
   HookBodyWriteFinding,
   HookBodyWriteSeverity,
   HookBodyWritePattern,
+  BodyWritePatternExclusion,
   ExtractedHookBodyWrite,
+  ExtractedHookBodyWriteSet,
 } from './validate-hook-body-writes.js';
 
 // The same write-set check on action bodies — same schema, same sandbox, same
@@ -279,8 +284,11 @@ export {
   validateActionBodyWrites,
   ACTION_BODY_WRITE_PATTERNS,
   ACTION_BODY_WRITE_PATTERN_IDS,
+  ACTION_RECORD_WRITE_PATTERNS,
+  ACTION_RECORD_WRITE_PATTERN_IDS,
   ACTION_BODY_WRITE_EXCLUSIONS,
   ACTION_BODY_WRITE_UNKNOWN_FIELD,
+  ACTION_RECORD_WRITE_DISCARDED,
 } from './validate-action-body-writes.js';
 export type {
   ActionBodyWriteFinding,

packages/lint/src/lazy-deps.test.ts

@@ -1,21 +1,28 @@
 // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
 //
 // Boot-path contract: importing @objectstack/lint must NOT load its heavy,
-// gate-only dependencies. The package sits on the kernel boot path, while
-// each dep below serves a gate that only runs when a `kind:'react'` page is
-// actually validated — so each must load lazily, on first use:
+// gate-only dependencies. The package sits on the kernel boot path, while each
+// dep below serves gates that only run when a particular, uncommon piece of
+// metadata is actually validated — so each must load lazily, on first use:
 //   - `typescript` (~9 MB, and has been pruned from production images before
-//     — see validate-react-page-props.ts), loaded by the react-props gate;
+//     — see validate-react-page-props.ts), loaded by the react-props gate AND
+//     by the L2 body write-set gates (validate-hook-body-writes.ts since
+//     #4271, validate-action-body-writes.ts since #4345);
 //   - `sucrase` (~1.5 MB), loaded by the react syntax gate
 //     (validate-react-pages.ts).
 //
+// "A react page" was the whole story when this file was written; it is not any
+// more, and the cases below say which trigger they are pinning. Keep them
+// named that way — a test called "until a react page is validated" that also
+// covers hook and action bodies is a test nobody can read against its subject.
+//
 // Guarded at three levels because vitest inlines static imports through its
 // transform (they never hit the native require cache), so an in-worker
 // require.cache probe alone CANNOT catch a reintroduced eager import:
 //   1. structural — no src file may eagerly `import ... from` a lazy dep
 //      (only `import type`, which erases at build);
 //   2. built dist — child `node` processes import both dist formats and prove
-//      each dep is absent until a react page is validated (skipped when dist
+//      each dep is absent until the metadata that needs it is validated (skipped when dist
 //      has not been built);
 //   3. behavioral — each lazy path really loads its dep on demand and the
 //      gate still produces findings.
@@ -62,8 +69,8 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
   });
 
   // Reused by both dist probes: import the dist entry, prove no lazy dep came
-  // with it, then run each react-page gate and prove exactly its own dep loads
-  // — and that the gate still produces its finding.
+  // with it, then run each gate and prove exactly its own dep loads — and that
+  // the gate still produces its finding.
   const reactStack = (source: string) => `{ pages: [{ name: 'r', kind: 'react', source: ${JSON.stringify(source)} }] }`;
   const childBody = `
     const probe = require('node:module').createRequire(process.cwd() + '/probe.js');
@@ -79,7 +86,7 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
       if (loaded('typescript')) fail('the hook-body write gate on an L1-only stack must not load typescript');
       mod.validateActionBodyWrites(jsAction('input.x > 0', 'expression'));
       mod.validateActionBodyWrites(jsAction('ctx.input.amout = 1;'));
-      if (loaded('typescript')) fail('the action-body write gate must not load typescript for a body that never touches ctx.api');
+      if (loaded('typescript')) fail('the action-body write gate must not load typescript for a body that reaches neither ctx.api nor ctx.record');
       const syntax = mod.validateReactPages(${reactStack('function Page(){ return <div>oops; }')});
       if (!loaded('sucrase')) fail('sucrase was not loaded by a react-page syntax validation');
       if (loaded('typescript')) fail('the syntax gate must not load typescript');
@@ -89,14 +96,16 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
       if (!hookWrites.some((f) => f.rule === 'hook-body-write-unknown-field')) fail('hook-body write gate produced no finding');
       const actionWrites = mod.validateActionBodyWrites(jsAction("await ctx.api.object('a').update({ amout: 1 });"));
       if (!actionWrites.some((f) => f.rule === 'action-body-write-unknown-field')) fail('action-body write gate produced no finding');
+      const recordWrites = mod.validateActionBodyWrites(jsAction("ctx.record.amount = 1;"));
+      if (!recordWrites.some((f) => f.rule === 'action-record-write-discarded')) fail('action-body record-write gate produced no finding');
       const props = mod.validateReactPageProps(${reactStack('function Page(){ return <ObjectForm mode="edit" />; }')});
       if (!loaded('typescript')) fail('typescript was not loaded by a react-page props validation');
       if (!props.some((f) => f.rule === 'react-prop-missing-required')) fail('props gate produced no finding');
       console.log('OK');
     };
   `;
 
-  it.skipIf(!existsSync(join(distDir, 'index.cjs')))('built CJS dist does not load a lazy dep until a react page is validated', () => {
+  it.skipIf(!existsSync(join(distDir, 'index.cjs')))('built CJS dist loads no lazy dep at import time, and each only on its own trigger', () => {
     const out = execFileSync(
       process.execPath,
       ['-e', `${childBody}; check(require(${JSON.stringify(join(distDir, 'index.cjs'))}));`],
@@ -105,7 +114,7 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
     expect(out).toContain('OK');
   }, COLD_LOAD_TIMEOUT_MS);
 
-  it.skipIf(!existsSync(join(distDir, 'index.js')))('built ESM dist does not load a lazy dep until a react page is validated', () => {
+  it.skipIf(!existsSync(join(distDir, 'index.js')))('built ESM dist loads no lazy dep at import time, and each only on its own trigger', () => {
     const out = execFileSync(
       process.execPath,
       [
@@ -136,15 +145,16 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
     expect(validateHookBodyWrites({ hooks: [hook(undefined)] })).toEqual([]);
     expect(validateHookBodyWrites({ hooks: [hook({ language: 'expression', source: 'input.x > 0' })] })).toEqual([]);
     expect(validateHookBodyWrites({ hooks: [hook({ language: 'js', source: 'return 1;' })] })).toEqual([]);
-    // Action bodies narrow it further: the only pattern the action rule carries
-    // is rooted at `ctx.api`, so even an L2 body that writes params never parses.
+    // Action bodies narrow it further: both patterns the action rule carries
+    // are rooted at `ctx.api` or `ctx.record`, so an L2 body that only writes
+    // params never parses at all.
     const action = (body: unknown) => ({ name: 'act', label: 'Act', objectName: 'a', body });
     expect(validateActionBodyWrites({ actions: [action(undefined)] })).toEqual([]);
     expect(validateActionBodyWrites({ actions: [action({ language: 'expression', source: 'input.x > 0' })] })).toEqual([]);
     expect(
       validateActionBodyWrites({
         objects: [{ name: 'a', fields: { amount: {} } }],
-        actions: [action({ language: 'js', source: 'ctx.input.amout = 1; ctx.record.nope = 2;' })],
+        actions: [action({ language: 'js', source: 'ctx.input.amout = 1; return { ok: true };' })],
       }),
     ).toEqual([]);
     for (const dep of LAZY_DEPS) {
@@ -177,6 +187,13 @@ describe('lazy dependency loading (kernel boot-path contract)', () => {
     expect(actionWrites.some((f) => f.rule === 'action-body-write-unknown-field' && f.severity === 'warning')).toBe(
       true,
     );
+    const recordWrites = validateActionBodyWrites({
+      objects: [{ name: 'a', fields: { amount: {} } }],
+      actions: [action({ language: 'js', source: 'ctx.record.amount = 1;' })],
+    });
+    expect(recordWrites.some((f) => f.rule === 'action-record-write-discarded' && f.severity === 'warning')).toBe(
+      true,
+    );
 
     const props = validateReactPageProps({
       pages: [{ name: 'r', kind: 'react', source: 'function Page(){ return <ObjectForm mode="edit" />; }' }],