fix(runtime,spec,lint): bind action.body only for type 'script' (#4352)

`ActionSchema.body` has always said "Only used when type is `script`", but
the runtime read `body` alone: `actionBodyRunnerFactory` bound a handler the
moment the body parsed, so a `type: 'url'` action carrying a leftover body
was registered and executed. Declared != enforced, in its nastiest shape —
an author flips `type` away from `script`, reasonably concludes the body is
dead, and it keeps running.

- runtime: `actionBodyRunnerFactory` refuses to bind unless the type is
  `script` (omitted `type` = the spec's own `ActionType.default('script')`),
  and logs the refusal with the schema's prescription rather than dropping
  it silently. The gate lives at the single bind point, not the collector —
  `collectBundleActions` stays type-blind so governance surfaces still see
  every declared action, and the second binder
  (`engine.setDefaultActionRunner`) never walks the collector at all.
- spec: pins that the publish gate RESOLVES to the rejecting schema —
  `getMetadataTypeSchema('action')` and `ObjectSchema.actions` — so a
  re-point of either registration cannot silently reopen the hole.
- lint: `validate-action-body-writes` filters by `type` again (#4344's
  provisional type-blindness is over) and its stale rationale is rewritten.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012C2cd7tL8QDoZ2QKN3djJ5

Author: claude

packages/lint/src/validate-action-body-writes.test.ts

@@ -390,10 +390,32 @@ describe('validateActionBodyWrites — where action bodies live', () => {
     expect(findings[0].where).toBe('action "close_deal" › body');
   });
 
-  it('checks a body on a non-script action — the runtime binds one regardless of `type`', () => {
+  // [#4352] The inversion of #4344's original assertion. That test pinned
+  // "checks a body on a non-script action — the runtime binds one regardless of
+  // `type`", which was true of the runtime at the time and was recorded as
+  // provisional. The ruling closed the gap at the producer instead: the body
+  // no longer binds (`actionBodyRunnerFactory`) and the pair no longer
+  // publishes (`ActionSchema`). Nothing runs, so there is no write set to
+  // advise about — and a finding here would point at the write when the defect
+  // is the `type`.
+  it('skips a body on a non-script action — nothing binds it, so there is no write to check', () => {
     const findings = validateActionBodyWrites(
       stackWith("await ctx.api.object('crm_deal').update({ stag: 'won' });", { type: 'url', target: '/x' }),
     );
+    expect(findings).toEqual([]);
+  });
+
+  it('still checks a body on an action that omits `type` — the spec default is `script`', () => {
+    const findings = validateActionBodyWrites(
+      stackWith("await ctx.api.object('crm_deal').update({ stag: 'won' });"),
+    );
+    expect(findings).toHaveLength(1);
+  });
+
+  it('still checks a body on an explicit `type: "script"` action', () => {
+    const findings = validateActionBodyWrites(
+      stackWith("await ctx.api.object('crm_deal').update({ stag: 'won' });", { type: 'script' }),
+    );
     expect(findings).toHaveLength(1);
   });
 });

packages/lint/src/validate-action-body-writes.ts

@@ -218,17 +218,32 @@ function actionObjectBinding(action: AnyRec, parentObject?: string): string | un
  * The top-level entry is walked first, so a merged action reports at
  * `actions[i]` — the authored location, not the derived copy.
  *
- * `type` is deliberately not consulted: the runtime binds a handler from
- * `action.body` alone (`actionBodyRunnerFactory` never reads `type`), so a body
- * on a non-`script` action still runs and still fails silently. Checking what
- * executes beats checking what the schema says should.
+ * Only `type: 'script'` bodies are walked (`type` omitted counts, since
+ * `ActionType.default('script')` makes that the same declaration).
+ *
+ * This rule USED to be deliberately type-blind, on the grounds that the
+ * runtime bound a handler from `action.body` alone and so a body on a
+ * non-`script` action still ran and still failed silently — checking what
+ * executes beat checking what the schema said should. That comment predicted
+ * its own revision ("定了之后 lint 那边要跟着调"), and #4352 is the ruling:
+ * `actionBodyRunnerFactory` now refuses to bind a handler unless the type is
+ * `script`, and `ActionSchema` rejects the contradictory pair at publish. So
+ * what executes and what the schema says are the same set again, and walking
+ * a non-`script` body here would produce advice about writes that provably
+ * never happen — noise pointing at metadata whose real defect is the `type`,
+ * which the publish gate already names with its own prescription.
  */
 function collectActionBodies(stack: AnyRec): ActionBodySite[] {
   const sites: ActionBodySite[] = [];
   const seen = new Set<string>();
 
   const collect = (actions: unknown, pathPrefix: string, parentObject?: string): void => {
     asArray(actions).forEach((action, index) => {
+      // Same default the spec declares, and the same one the runtime gate
+      // applies — a stack may reach lint unparsed, so an omitted `type` is
+      // `'script'`, not "unknown".
+      const type = typeof action.type === 'string' ? action.type : 'script';
+      if (type !== 'script') return;
       const body = action.body;
       if (!isRec(body) || body.language !== 'js') return;
       const source = body.source;

packages/runtime/src/action-body-type-gate.test.ts

@@ -0,0 +1,122 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#4352] `action.body` binds a handler ONLY for `type: 'script'`.
+ *
+ * `ActionSchema.body` has always said "Only used when type is `script`", and
+ * its JSDoc is more explicit still ("Only meaningful when `type === 'script'`").
+ * The runtime read neither: `collectBundleActions` collected any named action
+ * and `actionBodyRunnerFactory` bound a handler the moment `body` parsed. So a
+ * `type: 'url'` action carrying a leftover body was registered in the action
+ * registry and executed in the sandbox — the declared ≠ enforced shape of
+ * Prime Directive #10, in its nastiest form: an author flips `type` away from
+ * `script`, reasonably concludes the body is now dead, and it is not.
+ *
+ * The sibling tests in `sandbox/body-runner.test.ts` pin the factory in
+ * isolation. THIS file pins the composition AppPlugin actually performs —
+ * `collectBundleActions(bundle)` → `actionBodyRunnerFactory(...)` → skip when
+ * no handler → `ql.registerAction(...)` — because that loop is where the
+ * registration decision is really made, and a factory that returns `undefined`
+ * only matters if the loop honours it (it does: `if (!handler) continue`).
+ *
+ * The bind loop is replicated rather than driven through a booted AppPlugin on
+ * purpose: booting one needs a kernel, an ObjectQL engine and a QuickJS
+ * sandbox, none of which participate in the decision under test. The
+ * replication is kept honest by asserting the collector's own output too, so a
+ * change to how actions are collected still surfaces here.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { collectBundleActions } from './app-plugin.js';
+import { actionBodyRunnerFactory } from './sandbox/body-runner.js';
+import { QuickJSScriptRunner } from './sandbox/quickjs-runner.js';
+
+const jsBody = { language: 'js', source: 'return { ran: true };', capabilities: [] } as const;
+
+/** The exact registration loop from `AppPlugin.bindDeclarativeActions`. */
+function bindActions(bundle: unknown, logger?: { warn: (msg: string) => void }) {
+  const registered: Array<{ object: string; name: string }> = [];
+  const actions = collectBundleActions(bundle);
+  const runner = actionBodyRunnerFactory(new QuickJSScriptRunner(), {
+    ql: {},
+    appId: 'crm',
+    logger,
+  });
+  for (const action of actions) {
+    const handler = runner(action);
+    if (!handler) continue;
+    registered.push({ object: action.object ?? 'global', name: action.name });
+  }
+  return { collected: actions, registered };
+}
+
+describe('#4352 — a non-script action with a body binds no handler', () => {
+  it("registers the script action and skips the `type: 'url'` one", () => {
+    const warnings: string[] = [];
+    const { collected, registered } = bindActions(
+      {
+        actions: [
+          // The regression population: an explicit non-script type + a body.
+          { name: 'open_docs', label: 'Docs', type: 'url', target: 'https://x', body: jsBody },
+          // The overwhelmingly common case — unchanged.
+          { name: 'close_deal', label: 'Close', type: 'script', object: 'crm_deal', body: jsBody },
+        ],
+      },
+      { warn: (msg: string) => warnings.push(msg) },
+    );
+
+    // The collector stays type-blind by design — it feeds governance surfaces
+    // that must see every declared action, bound or not.
+    expect(collected.map((a) => a.name)).toEqual(['open_docs', 'close_deal']);
+
+    // ...but only the script action becomes an executable handler.
+    expect(registered).toEqual([{ object: 'crm_deal', name: 'close_deal' }]);
+
+    // And the refusal is audible: silence here would just move the invisibility.
+    expect(warnings).toHaveLength(1);
+    expect(warnings[0]).toContain('open_docs');
+    expect(warnings[0]).toContain("type: 'url'");
+  });
+
+  it('skips a non-script body declared under an object', () => {
+    const { registered } = bindActions({
+      objects: [
+        {
+          name: 'crm_lead',
+          actions: [
+            { name: 'open_portal', label: 'Portal', type: 'url', target: '/p', body: jsBody },
+            { name: 'score_lead', label: 'Score', type: 'script', body: jsBody },
+          ],
+        },
+      ],
+    });
+    expect(registered).toEqual([{ object: 'crm_lead', name: 'score_lead' }]);
+  });
+
+  it('binds an action that omits `type` — `ActionType.default(\'script\')`', () => {
+    // Bundles reach the collector RAW. A `strict: false` `defineStack` and a
+    // legacy `manifest.actions[]` never pass through `ActionSchema`, so the
+    // schema's default has to be applied here or the common shape breaks.
+    const { registered } = bindActions({
+      manifest: { actions: [{ name: 'legacy_untyped', label: 'Legacy', body: jsBody }] },
+    });
+    expect(registered).toEqual([{ object: 'global', name: 'legacy_untyped' }]);
+  });
+
+  it('leaves bodyless non-script actions exactly as they were', () => {
+    const warnings: string[] = [];
+    const { collected, registered } = bindActions(
+      {
+        actions: [
+          { name: 'open_docs', label: 'Docs', type: 'url', target: 'https://x' },
+          { name: 'convert', label: 'Convert', type: 'flow', target: 'crm_convert' },
+        ],
+      },
+      { warn: (msg: string) => warnings.push(msg) },
+    );
+    expect(collected).toHaveLength(2);
+    // They never bound a handler before this change either — nothing to warn about.
+    expect(registered).toEqual([]);
+    expect(warnings).toEqual([]);
+  });
+});

packages/runtime/src/app-plugin.ts

@@ -1441,6 +1441,15 @@ export function collectBundleHooks(bundle: any): any[] {
  *
  * Each returned record is a shallow copy with `object` set when the action
  * originated under an object (and not already present on the action itself).
+ *
+ * Deliberately type-BLIND, and it must stay that way: this collects every
+ * declared action, most of which (`url`, `modal`, `flow`, `api`, `form`)
+ * legitimately have no body and bind nothing. The `type: 'script'` gate that
+ * decides whether a `body` becomes an executable handler lives at the single
+ * bind point — `actionBodyRunnerFactory` (#4352) — because the other binder
+ * (`engine.setDefaultActionRunner`, for Studio-authored actions) never walks
+ * this collector at all. Re-filtering here would duplicate half the rule and
+ * leave the other binder ungated.
  */
 export function collectBundleActions(
     bundle: any,

packages/runtime/src/sandbox/body-runner.test.ts

@@ -166,6 +166,64 @@ describe('actionBodyRunnerFactory', () => {
     expect(factory({ name: 'noop' })).toBeUndefined();
   });
 
+  // ─── [#4352] the `type` gate ──────────────────────────────────────────────
+  // `ActionSchema.body` always said "Only used when type is `script`"; the
+  // runtime never read `type`, so a `type: 'url'` action carrying a leftover
+  // body still bound a handler and still executed. These pin the enforcement.
+  describe('binds a body only for `type: "script"` (#4352)', () => {
+    const body = { language: 'js', source: 'return { ran: true };', capabilities: [] } as const;
+
+    for (const type of ['url', 'modal', 'flow', 'api', 'form'] as const) {
+      it(`binds no handler for type: '${type}' and says why`, () => {
+        const warnings: string[] = [];
+        const factory = actionBodyRunnerFactory(runner, {
+          ql: {},
+          appId: 'crm',
+          logger: { warn: (msg: string) => warnings.push(msg) },
+        });
+        expect(factory({ name: 'leftover', object: 'lead', type, body })).toBeUndefined();
+        // Refusing silently would only relocate the invisibility the issue is about.
+        expect(warnings).toHaveLength(1);
+        expect(warnings[0]).toContain("type: '" + type + "'");
+        expect(warnings[0]).toContain('#4352');
+      });
+    }
+
+    it("binds for an explicit type: 'script'", async () => {
+      const factory = actionBodyRunnerFactory(runner, { ql: {}, appId: 'crm' });
+      const fn = factory({ name: 'ok', object: 'lead', type: 'script', body });
+      expect(typeof fn).toBe('function');
+      await expect(fn!({ params: {} })).resolves.toEqual({ ran: true });
+    });
+
+    it('binds when `type` is omitted — the spec default is `script`', async () => {
+      // The collectors walk RAW bundle objects; a `strict: false` defineStack or
+      // a legacy `manifest.actions[]` never passed through `ActionType.default`,
+      // so an omitted type must still mean `script` here.
+      const warnings: string[] = [];
+      const factory = actionBodyRunnerFactory(runner, {
+        ql: {},
+        appId: 'crm',
+        logger: { warn: (msg: string) => warnings.push(msg) },
+      });
+      const fn = factory({ name: 'ok', object: 'lead', body });
+      expect(typeof fn).toBe('function');
+      await expect(fn!({ params: {} })).resolves.toEqual({ ran: true });
+      expect(warnings).toEqual([]);
+    });
+
+    it('stays silent for a non-script action with no body — nothing is contradictory', () => {
+      const warnings: string[] = [];
+      const factory = actionBodyRunnerFactory(runner, {
+        ql: {},
+        appId: 'crm',
+        logger: { warn: (msg: string) => warnings.push(msg) },
+      });
+      expect(factory({ name: 'open_docs', type: 'url' })).toBeUndefined();
+      expect(warnings).toEqual([]);
+    });
+  });
+
   it('runs an L2 action body and returns its value', async () => {
     const factory = actionBodyRunnerFactory(runner, { ql: {}, appId: 'crm' });
     const fn = factory({

packages/runtime/src/sandbox/body-runner.ts

@@ -102,17 +102,61 @@ export function hookBodyRunnerFactory(
  * Returns a handler with the shape ObjectQL's `executeAction` expects:
  * `(actionCtx) => Promise<unknown>`. The action's return value bubbles up
  * to the HTTP dispatcher which JSON-serialises it back to the caller.
+ *
+ * This is the ONE choke point where an `action.body` becomes an executable
+ * handler — both bind paths go through it (`AppPlugin`'s bundle walk over
+ * `collectBundleActions`, and `engine.setDefaultActionRunner` for the
+ * Studio-authored `action` metadata ObjectQLPlugin re-syncs). So the
+ * `type` gate below is enforced here rather than at either call site: a
+ * second copy at the collector would be a rule that can drift from this one.
  */
 export function actionBodyRunnerFactory(
   runner: ScriptRunner,
   opts: FactoryOptions,
-): (action: { name: string; body?: unknown; object?: string; timeoutMs?: number }) =>
+): (action: { name: string; body?: unknown; object?: string; type?: string; timeoutMs?: number }) =>
   | ((actionCtx: any) => Promise<unknown>)
   | undefined {
   return (action) => {
     const raw = action.body;
     if (!raw) return undefined;
 
+    // [#4352] `body` binds a handler ONLY for `type: 'script'` — the rule the
+    // spec always stated (`ActionSchema.body`: "Only used when type is
+    // `script`") and the runtime never enforced. Every other type dispatches
+    // on `target` (the URL, page, flow or endpoint), so a body alongside one
+    // is self-contradictory metadata: two implementations, only one of which
+    // the author can see running.
+    //
+    // Binding it anyway produced the worst-shaped bug this repo has a name
+    // for — an author flips `type` from `script` to `url`, reasonably reads
+    // that as "the body no longer runs", and it keeps running, reachable
+    // through `ql.object(o).execute(name)` (the ObjectQL proxy calls
+    // `executeAction` with no type branching of its own) and counted by the
+    // ADR-0110 D5 governance inventory as a live handler.
+    //
+    // `?? 'script'` is the schema's own default (`ActionType.default('script')`),
+    // not a tolerant fallback: the collectors walk RAW bundle objects, which
+    // for a `strict: false` `defineStack` or a legacy `manifest.actions[]`
+    // never went through `ActionSchema`, so an omitted `type` still has to
+    // mean what the spec says it means. An action that EXPLICITLY declares
+    // another type is the only one whose behavior changes.
+    //
+    // The publish gate rejects this shape at authoring time (`ActionSchema`'s
+    // non-script-body refinement, #4438), so anything arriving here is either
+    // data at rest published before that gate existed or a bundle that never
+    // parsed. Refusing silently would just relocate the invisibility, so the
+    // refusal is logged with the same prescription the schema gives.
+    const type = action.type ?? 'script';
+    if (type !== 'script') {
+      opts.logger?.warn?.(
+        `[BodyRunner] action '${action.name}' declares \`type: '${type}'\` and carries a \`body\` — ` +
+          `no handler was bound. \`body\` only runs for \`type: 'script'\`; a '${type}' action dispatches ` +
+          `on \`target\`. Set \`type: 'script'\` to run the body, or drop the \`body\`. See #4352.`,
+        { appId: opts.appId, action: action.name, object: action.object, type },
+      );
+      return undefined;
+    }
+
     const parsed = HookBodySchema.safeParse(raw);
     if (!parsed.success) {
       opts.logger?.warn?.('[BodyRunner] invalid action.body shape', {