From eb1f477f45fcec916b9a46f436bdcf35eb21d737 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 03:44:11 +0000 Subject: [PATCH 1/2] fix(actions): prefer canonical `target` over deprecated `execute` alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ActionRunner.executeScript` read `action.execute || action.target`, so the deprecated alias won whenever both keys were set. `ActionPreview` already read `d.target ?? d.execute` — two readers in this repo, two precedences, which is the drift shape objectstack#3713 fixed one field over. `@objectstack/spec` ActionSchema declares `target` canonical for every action type (including `script`) and `execute` deprecated; spec >=16.1 folds `execute` into `target` and drops it at parse. Parsed metadata therefore never carries both keys, so this only bit on raw, unparsed metadata — but nothing made the two readers agree. Now they do. Alias-only authoring is unaffected: with `target` absent the fallback still picks up `execute`, which is how this repo's own script-action tests are written. Behavior changes only when both keys are set, where canonical now wins — matching the spec's fold. Refs objectstack#3713, objectstack#3742 Co-Authored-By: Claude Opus 5 --- packages/core/src/actions/ActionRunner.ts | 8 +++++++- .../src/actions/__tests__/ActionRunner.test.ts | 15 ++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/packages/core/src/actions/ActionRunner.ts b/packages/core/src/actions/ActionRunner.ts index b7b4cb71e6..2ced556a78 100644 --- a/packages/core/src/actions/ActionRunner.ts +++ b/packages/core/src/actions/ActionRunner.ts @@ -701,7 +701,13 @@ export class ActionRunner { * Supports ${} template expressions referencing data, record, user context. */ private async executeScript(action: ActionDef): Promise { - const script = action.execute || action.target; + // `target` is the canonical binding; `execute` is its deprecated alias + // (@objectstack/spec ActionSchema). Canonical wins when both are present, + // matching the spec's own fold and ActionPreview's `target ?? execute`. + // Spec >=16.1 folds `execute` into `target` and drops it at parse, so this + // only bites on raw, unparsed metadata — where the two readers used to + // disagree. Alias-only authoring still works via the fallback. + const script = action.target || action.execute; if (!script) { return { success: false, error: 'No script provided for script action' }; } diff --git a/packages/core/src/actions/__tests__/ActionRunner.test.ts b/packages/core/src/actions/__tests__/ActionRunner.test.ts index e92ac1f69c..c089d0afb2 100644 --- a/packages/core/src/actions/__tests__/ActionRunner.test.ts +++ b/packages/core/src/actions/__tests__/ActionRunner.test.ts @@ -200,7 +200,7 @@ describe('ActionRunner', () => { expect(result.data).toBe(101); }); - it('should evaluate script with string target fallback', async () => { + it('should evaluate script from the canonical target field', async () => { const result = await runner.execute({ type: 'script', target: 'data.name', @@ -209,6 +209,19 @@ describe('ActionRunner', () => { expect(result.data).toBe('Test'); }); + it('should prefer canonical target over the deprecated execute alias', async () => { + // Spec >=16.1 folds `execute` into `target` at parse, so the two keys only + // coexist on raw metadata. When they do, canonical wins — the same + // precedence ActionPreview and the spec's own fold already use. + const result = await runner.execute({ + type: 'script', + target: 'data.name', + execute: 'record.id + 100', + }); + expect(result.success).toBe(true); + expect(result.data).toBe('Test'); + }); + it('should fail when no script provided', async () => { const result = await runner.execute({ type: 'script' }); expect(result.success).toBe(false); From e293a349d6e8eb52ec00d079adfce078f27bd293 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 03:51:33 +0000 Subject: [PATCH 2/2] fix(actions): name the real cause when a script action carries a server-side `body` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ActionSchema.body` (HookBodySchema — L1 expression / L2 sandboxed JS) is the spec's PREFERRED binding for script actions, but the built-in `executeScript` reads only `target`/`execute`. A spec-valid `{ type: 'script', body: {...} }` failed with "No script provided for script action" — false, and it sends the author hunting for a field they had written. Bodies are not broken in the console: `useConsoleActionRuntime` registers `script: serverActionHandler`, registered handlers beat built-ins, and the server runs the body through its sandbox. This only affects consumers that register no `script` handler (standalone core, SDUI, embedded renderers). The fix is deliberately NOT a client-side body evaluator: - L2 (`js`) is defined as a function body inside an isolated VM enforcing declared capabilities, `timeoutMs` and `memoryMb`. A browser has no isolate, so checking capabilities there would be decoration — and `api.write` from an unsandboxed page is worse than no support. - L1 (`expression`) is formula-engine (CEL) source; this package's evaluator is a `${…}` template evaluator. They agree on simple comparisons and diverge on the rest, i.e. silently wrong rather than loudly unsupported. So `body` is declared opaque on `ActionDef` and the error now names the cause and the remedy (register a `script` handler POSTing to /api/v1/actions/{object}/{action}). Actions carrying no source at all keep the original message. Closes #2896 Co-Authored-By: Claude Opus 5 --- packages/core/src/actions/ActionRunner.ts | 30 +++++++++++++++++ .../actions/__tests__/ActionRunner.test.ts | 33 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/packages/core/src/actions/ActionRunner.ts b/packages/core/src/actions/ActionRunner.ts index 2ced556a78..205ed35a77 100644 --- a/packages/core/src/actions/ActionRunner.ts +++ b/packages/core/src/actions/ActionRunner.ts @@ -132,6 +132,23 @@ export interface ActionDef { execute?: string; /** Target URL or identifier (for type: 'url', 'modal', 'flow') */ target?: string; + /** + * Action body (spec `ActionSchema.body` — `HookBodySchema`). Opaque here: + * bodies are a SERVER-side execution surface and this runner never + * interprets one. + * + * L2 (`language: 'js'`) is defined as a function body run inside an isolated + * VM enforcing declared capabilities, `timeoutMs` and `memoryMb`. A browser + * has no such isolate, so "enforcing" those client-side would be decoration. + * L1 (`language: 'expression'`) is formula-engine (CEL) source, a different + * dialect from this package's `${…}` ExpressionEvaluator — running it here + * would diverge silently rather than fail. + * + * Consumers dispatch bodies by registering a `script` handler that POSTs to + * `/api/v1/actions/{object}/{action}` (see app-shell's + * `useConsoleActionRuntime`); the server runs the body through its sandbox. + */ + body?: unknown; /** For type: 'url' — where to open `target`. `'new-tab'` forces a new * browser tab/window, `'self'` forces same-page navigation. When omitted, * external URLs open in a new tab and relative URLs navigate in place. @@ -709,6 +726,19 @@ export class ActionRunner { // disagree. Alias-only authoring still works via the fallback. const script = action.target || action.execute; if (!script) { + // A spec `body` IS a script — this runner just cannot run one (see the + // `body` field docs). Saying "no script provided" would send the author + // hunting for a missing field they actually wrote, so name the real + // cause and the remedy instead. + if (action.body != null) { + return { + success: false, + error: + 'Action body must be executed server-side — this client runner does not interpret ' + + '`body` (sandboxed JS needs an isolated VM; expression bodies use the formula engine). ' + + 'Register a `script` handler that POSTs to /api/v1/actions/{object}/{action}.', + }; + } return { success: false, error: 'No script provided for script action' }; } diff --git a/packages/core/src/actions/__tests__/ActionRunner.test.ts b/packages/core/src/actions/__tests__/ActionRunner.test.ts index c089d0afb2..b16ec661f1 100644 --- a/packages/core/src/actions/__tests__/ActionRunner.test.ts +++ b/packages/core/src/actions/__tests__/ActionRunner.test.ts @@ -228,6 +228,39 @@ describe('ActionRunner', () => { expect(result.error).toContain('No script provided'); }); + it('should report a server-side body rather than claiming no script was provided', async () => { + // Spec-valid action: `body` IS the script, but bodies run server-side. + // The old message sent authors hunting for a field they had written. + const result = await runner.execute({ + type: 'script', + body: { language: 'expression', source: 'input.amount > 1000' }, + }); + expect(result.success).toBe(false); + expect(result.error).toContain('server-side'); + expect(result.error).not.toContain('No script provided'); + }); + + it('should not interpret a js body client-side', async () => { + // L2 needs an isolated VM enforcing capabilities/timeout/memory — the + // browser has none, so this must refuse rather than approximate. + const result = await runner.execute({ + type: 'script', + body: { language: 'js', source: 'return 1 + 1;', capabilities: [] }, + }); + expect(result.success).toBe(false); + expect(result.error).toContain('server-side'); + }); + + it('should still evaluate a client-side target when a body is also present', async () => { + const result = await runner.execute({ + type: 'script', + target: 'data.name', + body: { language: 'expression', source: 'input.amount > 1000' }, + }); + expect(result.success).toBe(true); + expect(result.data).toBe('Test'); + }); + it('should return data as undefined for expressions referencing missing vars', async () => { const result = await runner.execute({ type: 'script',