fix(actions): one precedence for target/execute, and stop mislabeling server-side body - #2895
Merged
Merged
Conversation
`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 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
…er-side `body`
`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 <noreply@anthropic.com>
os-zhuang
changed the title
target over deprecated execute aliastarget/execute, and stop mislabeling server-side body
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two fixes in
ActionRunner.executeScript, both about the same question — what source does a script action actually run? Completes the cross-repo half of objectstack#3713 / objectstack#3742, and closes #2896.1.
targetvsexecuteprecedenceexecuteScriptpicked the deprecated alias first:ActionPreviewalready picked the canonical key first:Two readers in this repo, two precedences — the drift shape objectstack#3713 fixed one field over.
@objectstack/specActionSchemadeclarestargetcanonical for every action type (includingscript) andexecutedeprecated.This is hardening, not a live bug fix, and that's worth knowing before you review it: spec >= 16.1 (objectstack#3742) folds
executeintotargetand drops the alias at parse, so parsed metadata never carries both keys, and the old||fell through to the right value anyway. The divergence only bites on raw, unparsed metadata — reachable, since this repo'sActionDefis hand-written rather than imported from the spec, but I could not produce it on a live path. What was wrong is that nothing made the two readers agree.Now
target || execute. Alias-only authoring is unaffected — withtargetabsent the fallback still picks upexecute, which is how this repo's own script-action tests are written. Behavior differs only when both keys are set.2. A
bodyis a script — say soActionSchema.body(HookBodySchema) is the spec's preferred binding for script actions:executeScriptreads neither key of it, so a spec-valid action:{ "type": "script", "body": { "language": "expression", "source": "input.amount > 1000" } }failed with "No script provided for script action" — false, and it sends the author hunting for a field they had written.
Scope check: this is not broken in the console.
useConsoleActionRuntimeregistersscript: serverActionHandler, registered handlers beat built-ins (ActionRunner.ts:493), and the server runs the body through its sandbox (runtime/src/sandbox/*→engine.executeAction). Only consumers registering noscripthandler — standalone core, SDUI, embedded renderers — hit the built-in.The fix is deliberately not a client-side body evaluator:
js) must never run in the browser. The contract is an isolated VM enforcing declared capabilities (api.read/api.write/api.transaction/…),timeoutMsandmemoryMb. There is no isolate in a page, so "checking" capabilities client-side is decoration — andapi.writefrom an unsandboxed page is strictly worse than no support.expression) is a different dialect here. The spec evaluates L1 with the formula engine (CEL). This package'sExpressionEvaluatoris a${…}template evaluator. They agree on simple comparisons and diverge on the rest — silently wrong beats loudly unsupported only in the wrong direction.So
bodyis declared opaque onActionDef(documented as server-executed) and the error now names the cause and the remedy: register ascripthandler POSTing to/api/v1/actions/{object}/{action}. Actions carrying no source at all keep the original message.Verification
jsbody refused, not approximated; a client-sidetargetstill wins when a body is also present.pnpm type-check --filter @object-ui/core— clean.unitproject: 270 files / 3703 tests, all passing.Out of scope
Wiring a default server dispatcher into
@object-ui/core, so standalone consumers get body support without registering a handler. Core has no opinion about auth, base URL, or object resolution today — all of whichserverActionHandlerneeds. Noted in #2896.Also unchanged: the four renderers (
action-button,action-group,action-icon,action-menu) forward bothtargetandexecutefaithfully — pure plumbing, no precedence decision. Andexecutestays accepted on input; removing it is a separate breaking change to coordinate with the spec's own eventual removal.Refs objectstack#3713, objectstack#3742
Closes #2896