Skip to content

Commit 08b5a3d

Browse files
authored
fix(action): one precedence for target vs deprecated execute — lower the alias, then drop it (#3713) (#3742)
`execute` is the deprecated alias of `target`, and three readers resolved "the author declared both" in TWO OPPOSITE directions: the `ActionSchema` transform kept `target`, objectui's `ActionRunner` did `execute || target`, and the CLI compile step preferred a function on `execute`. An action declaring both ran one script server-side and a different one client-side, silently. `target` now wins everywhere, and the transform DROPS the alias from its output, making the conflict unrepresentable rather than merely agreed-upon (same shape as `agent.knowledge.topics` → `sources`, #1891). The server runtime never read `execute` at all — `isHeadlessInvokableAction` gates on `target || body` and dispatch probes `target`/`name` — so authoring it worked solely because it was lowered at parse time, and dropping it costs the server nothing. It also fixes unpatched renderers by construction: an `execute || target` reader falls through to `target` once the alias is gone. The CLI had the same bug in compile-time form (not recorded in #3713): with a function inlined 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`. Authoring is unchanged — `execute` is still accepted on input, still lowered, still documented. Consumers of the parsed metadata must read `target`; `z.infer` no longer carries `execute`, so a stale reader fails to compile instead of silently reading `undefined`. Closes #3713. Follow-up on the author-facing warning: #3743.
1 parent db48ad5 commit 08b5a3d

9 files changed

Lines changed: 251 additions & 22 deletions

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/cli": patch
4+
---
5+
6+
fix(action): one precedence for `target` vs the deprecated `execute` — lower the alias, then drop it (#3713)
7+
8+
`execute` is the deprecated alias of `target`, and three readers resolved "the
9+
author declared both" in **two opposite directions**:
10+
11+
| Reader | Preferred |
12+
|---|---|
13+
| `ActionSchema` transform (spec) | `target` |
14+
| objectui `ActionRunner.executeScript` | `execute` |
15+
| CLI compile step (`lowerCallables`) | `execute` |
16+
17+
So `defineAction({ type: 'script', target: 'preferredHandler', execute: 'legacyHandler' })`
18+
ran `preferredHandler` server-side and `legacyHandler` client-side — two
19+
different scripts for one button, silently, with no error anywhere. Low
20+
frequency (it needs an author to set both, which happens mid-migration or by
21+
copy-paste), but the failure mode is "the wrong code ran".
22+
23+
**`target` now wins everywhere, and the alias is removed from the parsed
24+
output** — the same "canonical wins, alias disappears" shape as
25+
`agent.knowledge.topics``sources`. The conflict is now *unrepresentable*
26+
rather than merely agreed-upon: no renderer can see a second slot to disagree
27+
about. Worth noting the server runtime never read `execute` at all
28+
(`isHeadlessInvokableAction` gates on `target || body`; dispatch probes
29+
`target`/`name`), so authoring `execute` worked *solely* because it was lowered
30+
at parse time — dropping it costs the server nothing.
31+
32+
The CLI's inline-handler lowering had the same bug in compile-time form: with a
33+
function in both slots it bundled the `execute` one and then overwrote
34+
`action.target` with that ref, silently discarding the function the author
35+
declared on `target`. It now probes `target` first and drops the alias.
36+
37+
**Authoring is unchanged**`execute` is still accepted on input (`ActionInput`),
38+
still lowered to `target`, and still listed in the reference docs. Nothing to
39+
migrate in your app metadata.
40+
41+
**Consumers of the parsed metadata**, however, must read the canonical slot:
42+
43+
- FROM: `parsedAction.execute` → TO: `parsedAction.target`
44+
- One-line fix: delete the alias fallback, e.g. `action.execute || action.target`
45+
becomes `action.target`.
46+
47+
`z.infer<typeof ActionSchema>` no longer carries `execute`, so any such reader
48+
fails to compile rather than silently reading `undefined`. The objectui
49+
`ActionRunner` counterpart ships separately.

content/docs/protocol/objectui/actions.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ The `type` field selects how an action is dispatched. The complete enum is **`sc
5151
| `api` | Call an API endpoint (`method` defaults to `POST`). | Yes |
5252
| `form` | Open a FormView by name, routed to `/console/forms/:name`. | Yes |
5353

54-
`target` is the canonical binding for every non-`script` type. The deprecated `execute` field is auto-migrated to `target` during parsing.
54+
`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.
5555

5656
### Script Actions
5757

content/docs/references/ui/action.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ const result = Action.parse(data);
8787
| **target** | `string` | optional | URL, Script Name, Flow ID, or API Endpoint. Supports $`{param.X}` and $`{ctx.X}` interpolation. |
8888
| **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). |
8989
| **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`. |
90-
| **execute** | `string` | optional | @deprecated — Use target instead. Auto-migrated to target during parsing. |
90+
| **execute** | `string` | optional | @deprecated — Use target instead. Lowered into target during parsing and dropped from the parsed output; when both are set, target wins. |
9191
| **params** | `{ name?: string; field?: string; objectOverride?: string; label?: string; … }[]` | optional | Input parameters required from user |
9292
| **variant** | `Enum<'primary' \| 'secondary' \| 'danger' \| 'ghost' \| 'link'>` | optional | Button visual variant for styling (primary = highlighted, danger = destructive, ghost = transparent) |
9393
| **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. |
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { lowerCallables } from './lower-callables.js';
5+
6+
// ── #3713: `target` vs the deprecated `execute` alias — one precedence, one slot ──
7+
//
8+
// `execute` is the deprecated alias of `target`. Before this was aligned, three
9+
// readers resolved "the author declared both" in two directions: the spec
10+
// transform kept `target`, objectui's ActionRunner did `execute || target`, and
11+
// THIS compile step preferred `execute`. The compile-step half was the nastiest:
12+
// it bundled the `execute` function and then overwrote `action.target` with the
13+
// resulting ref, so the function the author actually declared on `target` was
14+
// silently dropped from the build.
15+
describe('lowerCallables — action handler slot precedence (#3713)', () => {
16+
const actionsOf = (result: { lowered: Record<string, unknown> }) =>
17+
(result.lowered as { actions: Array<Record<string, unknown>> }).actions;
18+
19+
it('binds the canonical `target` function when both slots carry a callable', () => {
20+
const result = lowerCallables({
21+
actions: [{
22+
name: 'convert',
23+
label: 'Convert',
24+
type: 'script',
25+
target: function preferredHandler() { return 'preferred'; },
26+
execute: function legacyHandler() { return 'legacy'; },
27+
}],
28+
});
29+
30+
const [action] = actionsOf(result);
31+
const ref = action.target as string;
32+
expect(typeof ref).toBe('string');
33+
// The bundled function must be the one from `target`, not from `execute`.
34+
expect(result.functions[ref]()).toBe('preferred');
35+
expect(Object.values(result.functions).map((fn) => fn())).not.toContain('legacy');
36+
});
37+
38+
it('drops the `execute` alias once a ref is bound to `target`', () => {
39+
const result = lowerCallables({
40+
actions: [{
41+
name: 'convert',
42+
label: 'Convert',
43+
type: 'script',
44+
target: function preferredHandler() { return 'preferred'; },
45+
execute: function legacyHandler() { return 'legacy'; },
46+
}],
47+
});
48+
49+
const [action] = actionsOf(result);
50+
// A leftover alias is stale by construction — and a function-valued one
51+
// would fail `ActionSchema` (it expects a string) further down the pipeline.
52+
expect('execute' in action).toBe(false);
53+
// The lowered stack must be JSON-safe: no function values survive.
54+
expect(() => JSON.stringify(result.lowered)).not.toThrow();
55+
expect(JSON.stringify(result.lowered)).not.toContain('legacy');
56+
});
57+
58+
it('still lowers an `execute`-only callable (back-compat) onto `target`', () => {
59+
const result = lowerCallables({
60+
actions: [{
61+
name: 'legacy_only',
62+
label: 'Legacy',
63+
type: 'script',
64+
execute: function legacyHandler() { return 'legacy'; },
65+
}],
66+
});
67+
68+
const [action] = actionsOf(result);
69+
const ref = action.target as string;
70+
expect(typeof ref).toBe('string');
71+
expect(result.functions[ref]()).toBe('legacy');
72+
expect('execute' in action).toBe(false);
73+
});
74+
75+
it('leaves a string-valued handler pair untouched (the spec transform owns it)', () => {
76+
// No callable to lower here, so the compile step must not editorialise: the
77+
// spec's `ActionSchema` transform is the single place that resolves a
78+
// string/string pair (keeping `target`, dropping `execute`).
79+
const result = lowerCallables({
80+
actions: [{
81+
name: 'strings',
82+
label: 'Strings',
83+
type: 'script',
84+
target: 'preferredHandler',
85+
execute: 'legacyHandler',
86+
}],
87+
});
88+
89+
const [action] = actionsOf(result);
90+
expect(action.target).toBe('preferredHandler');
91+
expect(action.execute).toBe('legacyHandler');
92+
expect(result.count).toBe(0);
93+
});
94+
95+
it('applies the same precedence to actions nested under an object', () => {
96+
const result = lowerCallables({
97+
objects: [{
98+
name: 'crm_deal',
99+
actions: [{
100+
name: 'convert',
101+
label: 'Convert',
102+
type: 'script',
103+
target: function preferredHandler() { return 'preferred'; },
104+
execute: function legacyHandler() { return 'legacy'; },
105+
}],
106+
}],
107+
});
108+
109+
const objects = (result.lowered as { objects: Array<{ actions: Array<Record<string, unknown>> }> }).objects;
110+
const [action] = objects[0].actions;
111+
const ref = action.target as string;
112+
expect(result.functions[ref]()).toBe('preferred');
113+
expect('execute' in action).toBe(false);
114+
});
115+
});

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

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ export function lowerCallables(input: Record<string, unknown>): LoweringResult {
105105
}
106106

107107
// 1b. Lower inline action handlers found inside `objects[*].actions[*]`
108-
// and `actions[*]`. We accept either `execute: fn` or `target: fn`.
108+
// and `actions[*]`. We accept either `target: fn` or the deprecated
109+
// `execute: fn`; `target` wins when both are present (#3713).
109110
if (Array.isArray(lowered.objects)) {
110111
lowered.objects = (lowered.objects as unknown[]).map((rawObj) => {
111112
if (!isPlainObject(rawObj)) return rawObj;
@@ -173,9 +174,10 @@ export function lowerCallables(input: Record<string, unknown>): LoweringResult {
173174
}
174175

175176
/**
176-
* Lower a single action definition: detect callable on `execute` or
177-
* `target`, register it, optionally extract a metadata body. Mutates a
178-
* shallow clone, never the input.
177+
* Lower a single action definition: detect a callable on `target` or on the
178+
* deprecated `execute` alias (canonical `target` first — #3713), register it,
179+
* optionally extract a metadata body, and drop the alias. Mutates a shallow
180+
* clone, never the input.
179181
*/
180182
function lowerActionCallable(
181183
raw: unknown,
@@ -189,11 +191,17 @@ function lowerActionCallable(
189191
const baseName = typeof action.name === 'string' && action.name.length > 0
190192
? `${ownerLabel}_${action.name}`
191193
: `${ownerLabel}_anon_action`;
194+
// #3713: `target` is the canonical slot and `execute` its deprecated alias, so
195+
// probe `target` FIRST. This used to prefer `execute`, which made the compile
196+
// step the third reader of this pair — and it disagreed with the spec transform
197+
// (which keeps `target` when both are set). An author who inlined a function in
198+
// both slots got the `execute` one bundled while `action.target = ref` below
199+
// silently overwrote the `target` function they had actually declared.
192200
const handlerSlot: 'execute' | 'target' | null =
193-
typeof action.execute === 'function'
194-
? 'execute'
195-
: typeof action.target === 'function'
196-
? 'target'
201+
typeof action.target === 'function'
202+
? 'target'
203+
: typeof action.execute === 'function'
204+
? 'execute'
197205
: null;
198206
if (!handlerSlot) return action;
199207
const fn = action[handlerSlot] as AnyFn;
@@ -206,6 +214,10 @@ function lowerActionCallable(
206214
}
207215
// Keep a string-named target so the legacy executor can still resolve it.
208216
action.target = ref;
209-
if (handlerSlot === 'execute') delete action.execute;
217+
// Drop the alias unconditionally, matching the spec transform's "canonical wins,
218+
// alias disappears" rule (#3713). Once `target` carries the bundled ref, any
219+
// leftover `execute` is stale by construction — and a function-valued one would
220+
// fail `ActionSchema` (it expects a string) further down the pipeline.
221+
if ('execute' in action) delete action.execute;
210222
return action;
211223
}

packages/spec/liveness/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ as `live`, 10 were wrong** — a 77% error rate for the preview-renderer standar
6666

6767
| Verdict | Properties |
6868
|---|---|
69-
| `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`) |
69+
| `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`) |
7070
| corrected to `dead` + `authorWarn` | `action.shortcut`, `action.bulkEnabled`, `flow.active`, `skill.triggerPhrases`, `tool.category`, `tool.requiresConfirmation`, `tool.active`, `tool.builtIn`, `skill.permissions`*, `agent.knowledge` |
7171

7272
\* `skill.permissions` was subsequently pruned outright — it was never enforced.

packages/spec/liveness/action.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@
4444
},
4545
"execute": {
4646
"status": "live",
47-
"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)",
48-
"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."
47+
"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",
48+
"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."
4949
},
5050
"params": {
5151
"status": "live",

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,6 +1094,8 @@ describe('ActionSchema - execute → target migration', () => {
10941094
execute: 'legacyHandler',
10951095
});
10961096
expect(result.target).toBe('legacyHandler');
1097+
// #3713: the alias is consumed, not carried alongside the canonical slot.
1098+
expect('execute' in result).toBe(false);
10971099
});
10981100

10991101
it('should preserve target over execute when both are set', () => {
@@ -1107,6 +1109,35 @@ describe('ActionSchema - execute → target migration', () => {
11071109
expect(result.target).toBe('preferredHandler');
11081110
});
11091111

1112+
it('should DROP execute from the parsed output so no consumer can disagree (#3713)', () => {
1113+
// The divergence this pins: the spec kept `target` when both were set, while
1114+
// objectui's ActionRunner did `execute || target` — so one button ran
1115+
// `preferredHandler` server-side and `legacyHandler` client-side, silently.
1116+
// Lowering the alias and removing it makes the conflict *unrepresentable*
1117+
// rather than merely agreed-upon — same shape as `agent.knowledge.topics`
1118+
// → `sources` (#1891), which asserts `'topics' in parsed === false`.
1119+
const both = ActionSchema.parse({
1120+
name: 'both_fields',
1121+
label: 'Both',
1122+
type: 'script',
1123+
target: 'preferredHandler',
1124+
execute: 'legacyHandler',
1125+
});
1126+
expect('execute' in both).toBe(false);
1127+
expect(Object.keys(both)).not.toContain('execute');
1128+
expect(JSON.parse(JSON.stringify(both)).execute).toBeUndefined();
1129+
1130+
// Authors may still WRITE `execute` — only the parsed output is canonical.
1131+
const aliasOnly = ActionSchema.parse({
1132+
name: 'alias_only',
1133+
label: 'Alias',
1134+
type: 'url',
1135+
execute: 'https://example.com/report',
1136+
});
1137+
expect(aliasOnly.target).toBe('https://example.com/report');
1138+
expect('execute' in aliasOnly).toBe(false);
1139+
});
1140+
11101141
it('should reject a script with neither target/execute nor body', () => {
11111142
// #2169: a script action with no handler binding registers nothing.
11121143
expect(() => ActionSchema.parse({

0 commit comments

Comments
 (0)