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

.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" />; }' }],