fix(action): one precedence for target vs deprecated execute — lower the alias, then drop it (#3713)

.changeset/action-execute-target-precedence.md

@@ -0,0 +1,49 @@
+---
+"@objectstack/spec": minor
+"@objectstack/cli": patch
+---
+
+fix(action): one precedence for `target` vs the deprecated `execute` — lower the alias, then drop it (#3713)
+
+`execute` is the deprecated alias of `target`, and three readers resolved "the
+author declared both" in **two opposite directions**:
+
+| Reader | Preferred |
+|---|---|
+| `ActionSchema` transform (spec) | `target` |
+| objectui `ActionRunner.executeScript` | `execute` |
+| CLI compile step (`lowerCallables`) | `execute` |
+
+So `defineAction({ type: 'script', target: 'preferredHandler', execute: 'legacyHandler' })`
+ran `preferredHandler` server-side and `legacyHandler` client-side — two
+different scripts for one button, silently, with no error anywhere. Low
+frequency (it needs an author to set both, which happens mid-migration or by
+copy-paste), but the failure mode is "the wrong code ran".
+
+**`target` now wins everywhere, and the alias is removed from the parsed
+output** — the same "canonical wins, alias disappears" shape as
+`agent.knowledge.topics` → `sources`. The conflict is now *unrepresentable*
+rather than merely agreed-upon: no renderer can see a second slot to disagree
+about. Worth noting the server runtime never read `execute` at all
+(`isHeadlessInvokableAction` gates on `target || body`; dispatch probes
+`target`/`name`), so authoring `execute` worked *solely* because it was lowered
+at parse time — dropping it costs the server nothing.
+
+The CLI's inline-handler lowering had the same bug in compile-time form: with a
+function in both slots it bundled the `execute` one and then overwrote
+`action.target` with that ref, silently discarding the function the author
+declared on `target`. It now probes `target` first and drops the alias.
+
+**Authoring is unchanged** — `execute` is still accepted on input (`ActionInput`),
+still lowered to `target`, and still listed in the reference docs. Nothing to
+migrate in your app metadata.
+
+**Consumers of the parsed metadata**, however, must read the canonical slot:
+
+- FROM: `parsedAction.execute` → TO: `parsedAction.target`
+- One-line fix: delete the alias fallback, e.g. `action.execute || action.target`
+  becomes `action.target`.
+
+`z.infer<typeof ActionSchema>` no longer carries `execute`, so any such reader
+fails to compile rather than silently reading `undefined`. The objectui
+`ActionRunner` counterpart ships separately.

content/docs/protocol/objectui/actions.mdx

@@ -51,7 +51,7 @@ The `type` field selects how an action is dispatched. The complete enum is **`sc
 | `api` | Call an API endpoint (`method` defaults to `POST`). | Yes |
 | `form` | Open a FormView by name, routed to `/console/forms/:name`. | Yes |
 
-`target` is the canonical binding for every non-`script` type. The deprecated `execute` field is auto-migrated to `target` during parsing.
+`target` is the canonical binding for every non-`script` type. The deprecated `execute` field is lowered into `target` during parsing and then **removed from the parsed metadata**, so every consumer — server dispatch and renderer alike — reads exactly one slot. If an action declares both, `target` wins and `execute` is discarded.
 
 ### Script Actions
 

content/docs/references/ui/action.mdx

@@ -87,7 +87,7 @@ const result = Action.parse(data);
 | **target** | `string` | optional | URL, Script Name, Flow ID, or API Endpoint. Supports $`{param.X}` and $`{ctx.X}` interpolation. |
 | **openIn** | `Enum<'self' \| 'new-tab'>` | optional | For type:'url' — where to open `target`. 'new-tab' opens a new browser tab; 'self' navigates in place. When omitted, external/absolute URLs open in a new tab and relative URLs navigate in place. Static execution option — keep it OUT of `params` (which is user-input-collection only). |
 | **body** | `{ language: 'expression'; source: string } \| { language: 'js'; source: string; capabilities?: Enum<'api.read' \| 'api.write' \| 'api.transaction' \| 'crypto.uuid' \| 'crypto.hash' \| 'log'>[]; timeoutMs?: integer; … }` | optional | Action body — expression (L1) or sandboxed JS (L2). Only used when type is `script`. |
-| **execute** | `string` | optional | @deprecated — Use target instead. Auto-migrated to target during parsing. |
+| **execute** | `string` | optional | @deprecated — Use target instead. Lowered into target during parsing and dropped from the parsed output; when both are set, target wins. |
 | **params** | `{ name?: string; field?: string; objectOverride?: string; label?: string; … }[]` | optional | Input parameters required from user |
 | **variant** | `Enum<'primary' \| 'secondary' \| 'danger' \| 'ghost' \| 'link'>` | optional | Button visual variant for styling (primary = highlighted, danger = destructive, ghost = transparent) |
 | **order** | `number` | optional | Sort order within a location group (lower = higher). Promotes/demotes an action toward the record_header primary button; stable, so actions without `order` keep their registration order. |

packages/cli/src/utils/lower-callables.test.ts

@@ -0,0 +1,115 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect } from 'vitest';
+import { lowerCallables } from './lower-callables.js';
+
+// ── #3713: `target` vs the deprecated `execute` alias — one precedence, one slot ──
+//
+// `execute` is the deprecated alias of `target`. Before this was aligned, three
+// readers resolved "the author declared both" in two directions: the spec
+// transform kept `target`, objectui's ActionRunner did `execute || target`, and
+// THIS compile step preferred `execute`. The compile-step half was the nastiest:
+// it bundled the `execute` function and then overwrote `action.target` with the
+// resulting ref, so the function the author actually declared on `target` was
+// silently dropped from the build.
+describe('lowerCallables — action handler slot precedence (#3713)', () => {
+  const actionsOf = (result: { lowered: Record<string, unknown> }) =>
+    (result.lowered as { actions: Array<Record<string, unknown>> }).actions;
+
+  it('binds the canonical `target` function when both slots carry a callable', () => {
+    const result = lowerCallables({
+      actions: [{
+        name: 'convert',
+        label: 'Convert',
+        type: 'script',
+        target: function preferredHandler() { return 'preferred'; },
+        execute: function legacyHandler() { return 'legacy'; },
+      }],
+    });
+
+    const [action] = actionsOf(result);
+    const ref = action.target as string;
+    expect(typeof ref).toBe('string');
+    // The bundled function must be the one from `target`, not from `execute`.
+    expect(result.functions[ref]()).toBe('preferred');
+    expect(Object.values(result.functions).map((fn) => fn())).not.toContain('legacy');
+  });
+
+  it('drops the `execute` alias once a ref is bound to `target`', () => {
+    const result = lowerCallables({
+      actions: [{
+        name: 'convert',
+        label: 'Convert',
+        type: 'script',
+        target: function preferredHandler() { return 'preferred'; },
+        execute: function legacyHandler() { return 'legacy'; },
+      }],
+    });
+
+    const [action] = actionsOf(result);
+    // A leftover alias is stale by construction — and a function-valued one
+    // would fail `ActionSchema` (it expects a string) further down the pipeline.
+    expect('execute' in action).toBe(false);
+    // The lowered stack must be JSON-safe: no function values survive.
+    expect(() => JSON.stringify(result.lowered)).not.toThrow();
+    expect(JSON.stringify(result.lowered)).not.toContain('legacy');
+  });
+
+  it('still lowers an `execute`-only callable (back-compat) onto `target`', () => {
+    const result = lowerCallables({
+      actions: [{
+        name: 'legacy_only',
+        label: 'Legacy',
+        type: 'script',
+        execute: function legacyHandler() { return 'legacy'; },
+      }],
+    });
+
+    const [action] = actionsOf(result);
+    const ref = action.target as string;
+    expect(typeof ref).toBe('string');
+    expect(result.functions[ref]()).toBe('legacy');
+    expect('execute' in action).toBe(false);
+  });
+
+  it('leaves a string-valued handler pair untouched (the spec transform owns it)', () => {
+    // No callable to lower here, so the compile step must not editorialise: the
+    // spec's `ActionSchema` transform is the single place that resolves a
+    // string/string pair (keeping `target`, dropping `execute`).
+    const result = lowerCallables({
+      actions: [{
+        name: 'strings',
+        label: 'Strings',
+        type: 'script',
+        target: 'preferredHandler',
+        execute: 'legacyHandler',
+      }],
+    });
+
+    const [action] = actionsOf(result);
+    expect(action.target).toBe('preferredHandler');
+    expect(action.execute).toBe('legacyHandler');
+    expect(result.count).toBe(0);
+  });
+
+  it('applies the same precedence to actions nested under an object', () => {
+    const result = lowerCallables({
+      objects: [{
+        name: 'crm_deal',
+        actions: [{
+          name: 'convert',
+          label: 'Convert',
+          type: 'script',
+          target: function preferredHandler() { return 'preferred'; },
+          execute: function legacyHandler() { return 'legacy'; },
+        }],
+      }],
+    });
+
+    const objects = (result.lowered as { objects: Array<{ actions: Array<Record<string, unknown>> }> }).objects;
+    const [action] = objects[0].actions;
+    const ref = action.target as string;
+    expect(result.functions[ref]()).toBe('preferred');
+    expect('execute' in action).toBe(false);
+  });
+});

packages/cli/src/utils/lower-callables.ts

@@ -105,7 +105,8 @@ export function lowerCallables(input: Record<string, unknown>): LoweringResult {
   }
 
   // 1b. Lower inline action handlers found inside `objects[*].actions[*]`
-  //     and `actions[*]`. We accept either `execute: fn` or `target: fn`.
+  //     and `actions[*]`. We accept either `target: fn` or the deprecated
+  //     `execute: fn`; `target` wins when both are present (#3713).
   if (Array.isArray(lowered.objects)) {
     lowered.objects = (lowered.objects as unknown[]).map((rawObj) => {
       if (!isPlainObject(rawObj)) return rawObj;
@@ -173,9 +174,10 @@ export function lowerCallables(input: Record<string, unknown>): LoweringResult {
 }
 
 /**
- * Lower a single action definition: detect callable on `execute` or
- * `target`, register it, optionally extract a metadata body. Mutates a
- * shallow clone, never the input.
+ * Lower a single action definition: detect a callable on `target` or on the
+ * deprecated `execute` alias (canonical `target` first — #3713), register it,
+ * optionally extract a metadata body, and drop the alias. Mutates a shallow
+ * clone, never the input.
  */
 function lowerActionCallable(
   raw: unknown,
@@ -189,11 +191,17 @@ function lowerActionCallable(
   const baseName = typeof action.name === 'string' && action.name.length > 0
     ? `${ownerLabel}_${action.name}`
     : `${ownerLabel}_anon_action`;
+  // #3713: `target` is the canonical slot and `execute` its deprecated alias, so
+  // probe `target` FIRST. This used to prefer `execute`, which made the compile
+  // step the third reader of this pair — and it disagreed with the spec transform
+  // (which keeps `target` when both are set). An author who inlined a function in
+  // both slots got the `execute` one bundled while `action.target = ref` below
+  // silently overwrote the `target` function they had actually declared.
   const handlerSlot: 'execute' | 'target' | null =
-    typeof action.execute === 'function'
-      ? 'execute'
-      : typeof action.target === 'function'
-        ? 'target'
+    typeof action.target === 'function'
+      ? 'target'
+      : typeof action.execute === 'function'
+        ? 'execute'
         : null;
   if (!handlerSlot) return action;
   const fn = action[handlerSlot] as AnyFn;
@@ -206,6 +214,10 @@ function lowerActionCallable(
   }
   // Keep a string-named target so the legacy executor can still resolve it.
   action.target = ref;
-  if (handlerSlot === 'execute') delete action.execute;
+  // Drop the alias unconditionally, matching the spec transform's "canonical wins,
+  // alias disappears" rule (#3713). Once `target` carries the bundled ref, any
+  // leftover `execute` is stale by construction — and a function-valued one would
+  // fail `ActionSchema` (it expects a string) further down the pipeline.
+  if ('execute' in action) delete action.execute;
   return action;
 }

packages/spec/liveness/README.md

@@ -66,7 +66,7 @@ as `live`, 10 were wrong** — a 77% error rate for the preview-renderer standar
 
 | Verdict | Properties |
 |---|---|
-| `live`, evidence corrected to the real reader | `action.execute` (ActionRunner + the spec transform), `action.disabled` (six render surfaces), `flow.status` (engine gates binding + execution since `497bda853`) |
+| `live`, evidence corrected to the real reader | `action.execute` (the spec transform's parse-time lowering — the second reader, objectui's ActionRunner, resolved the `target`/`execute` pair in the *opposite* direction; aligned and the alias dropped from the parsed output in #3713), `action.disabled` (six render surfaces), `flow.status` (engine gates binding + execution since `497bda853`) |
 | corrected to `dead` + `authorWarn` | `action.shortcut`, `action.bulkEnabled`, `flow.active`, `skill.triggerPhrases`, `tool.category`, `tool.requiresConfirmation`, `tool.active`, `tool.builtIn`, `skill.permissions`*, `agent.knowledge` |
 
 \* `skill.permissions` was subsequently pruned outright — it was never enforced.

packages/spec/liveness/action.json

@@ -44,8 +44,8 @@
     },
     "execute": {
       "status": "live",
-      "evidence": "objectui packages/core/src/actions/ActionRunner.ts:704 (executeScript reads action.execute) + framework packages/spec/src/ui/action.zod.ts:577-580 (.transform lowers execute -> target)",
-      "note": "RE-VERIFIED 2026-07 (#3686 preview-claim sweep): the prior `live` verdict cited only a metadata-admin PREVIEW panel, which echoes what the author typed. Verdict stands but for a different reason: TWO independent runtime readers exist. NOTE a live divergence — the spec transform keeps `target` when both are set (action.test.ts:1032), while ActionRunner does `execute || target`, so an action declaring both runs different code client- vs server-side."
+      "evidence": "packages/spec/src/ui/action.zod.ts:581 — the .transform lowers execute -> target and DROPS the alias, so authoring it changes what the runtime dispatches; `target` is the one slot every consumer reads",
+      "note": "RE-VERIFIED 2026-07 (#3686 preview-claim sweep): the prior `live` verdict cited only a metadata-admin PREVIEW panel, which echoes what the author typed. Verdict stands, but the real reader is the parse-time lowering, not a second field reader. DIVERGENCE RESOLVED in #3713: the alias is now consumed at parse time and removed from the output, so the 'both declared' conflict is unrepresentable rather than merely agreed-upon (mirrors agent.knowledge.topics -> sources, #1891). Before that fix three readers disagreed in two directions — this transform kept `target`, objectui ActionRunner did `execute || target`, and the CLI compile step (packages/cli/src/utils/lower-callables.ts) preferred a function on `execute`; all now prefer `target`. Note the server runtime never read `execute` at all (runtime/src/action-execution.ts gates on `target || body` and dispatches on `target`/`name`), which is why lowering is the whole of this property's liveness."
     },
     "params": {
       "status": "live",

packages/spec/src/ui/action.test.ts

@@ -1027,6 +1027,8 @@ describe('ActionSchema - execute → target migration', () => {
       execute: 'legacyHandler',
     });
     expect(result.target).toBe('legacyHandler');
+    // #3713: the alias is consumed, not carried alongside the canonical slot.
+    expect('execute' in result).toBe(false);
   });
 
   it('should preserve target over execute when both are set', () => {
@@ -1040,6 +1042,35 @@ describe('ActionSchema - execute → target migration', () => {
     expect(result.target).toBe('preferredHandler');
   });
 
+  it('should DROP execute from the parsed output so no consumer can disagree (#3713)', () => {
+    // The divergence this pins: the spec kept `target` when both were set, while
+    // objectui's ActionRunner did `execute || target` — so one button ran
+    // `preferredHandler` server-side and `legacyHandler` client-side, silently.
+    // Lowering the alias and removing it makes the conflict *unrepresentable*
+    // rather than merely agreed-upon — same shape as `agent.knowledge.topics`
+    // → `sources` (#1891), which asserts `'topics' in parsed === false`.
+    const both = ActionSchema.parse({
+      name: 'both_fields',
+      label: 'Both',
+      type: 'script',
+      target: 'preferredHandler',
+      execute: 'legacyHandler',
+    });
+    expect('execute' in both).toBe(false);
+    expect(Object.keys(both)).not.toContain('execute');
+    expect(JSON.parse(JSON.stringify(both)).execute).toBeUndefined();
+
+    // Authors may still WRITE `execute` — only the parsed output is canonical.
+    const aliasOnly = ActionSchema.parse({
+      name: 'alias_only',
+      label: 'Alias',
+      type: 'url',
+      execute: 'https://example.com/report',
+    });
+    expect(aliasOnly.target).toBe('https://example.com/report');
+    expect('execute' in aliasOnly).toBe(false);
+  });
+
   it('should reject a script with neither target/execute nor body', () => {
     // #2169: a script action with no handler binding registers nothing.
     expect(() => ActionSchema.parse({