Skip to content

feat(maestro): support evalScript inline JavaScript expressions - #2158

Open
Rohit3523 wants to merge 1 commit into
callstack:mainfrom
Rohit3523:feat/maestro-evalscript
Open

feat(maestro): support evalScript inline JavaScript expressions#2158
Rohit3523 wants to merge 1 commit into
callstack:mainfrom
Rohit3523:feat/maestro-evalscript

Conversation

@Rohit3523

Copy link
Copy Markdown

Summary

Adds evalScript to the supported Maestro YAML subset. The command evaluates a
single-line JavaScript expression with flow env values and prior output
leaves in scope, then folds the assigned output object back into the flat
string-key variable model so later steps resolve ${output.x} and
${output.list.length}.

env:
  MY_NAME: John
---
- evalScript: ${output.uppercaseName = MY_NAME.toUpperCase()}
- inputText: ${output.uppercaseName}

Evaluation is host-side in the compute engine (node:vm, matching runScript
trust semantics); the command is never dispatched to the device port. Without a
device round trip, the engine merges output leaves directly into flow scope and
marks the step executed. repeat.times: ${output.list.length} works because
assigned arrays fold to index and length leaves. --from resume refuses to
skip evalScript steps, since they produce output.

Validation

  • pnpm test:maestro-compat: 335 unit tests pass (new parser, engine, and
    engine-eval-script eval tests).
  • pnpm maestro:conformance: 57 pass. upstream/053_repeat_times reclassifies
    from we-reject (evalScript unsupported) to identical; the declared
    divergence is removed.
  • Live Android evidence: installed the test-app release build on
    emulator-5554, ran an eval-script flow; output.sum = 1 + 2 then
    inputText: ${output.sum} landed the computed 3 in the focused field
    (field value Ada Lovelace3), proving arbitrary-expression evaluation and
    ${output.x} consumption end-to-end on a device.

Tradeoffs and follow-ups

  • evalScript output is string-typed across steps (runScript parity): JS
    object/array identity is not preserved, so ${output.list.length} and
    indexed leaves resolve but in-flow mutation like output.list.push() does
    not.
  • evalScript is the only command whose payload runs as JavaScript; all other
    fields stay literal/${VAR}-lookup-only (assertTrue scoped to lookups,
    repeat.while unsupported).
  • No new issue to close; ADR 0015 and the help maestro support matrix updated
    to document the evalScript boundary.

13 files touched, +298/−15, within the Maestro compatibility engine and its
conformance/docs. No scope growth outside that family.

Add evalScript as a supported Maestro command: a single-line JavaScript
expression evaluated against flow env and prior output leaves, with the
assigned output object folded back into the flat string-key variable
model so ${output.x} and ${output.list.length} resolve in later steps.
The command is handled host-side by the compute engine (node:vm) and
never dispatched to the device port.

Update the support matrix, ADR 0015, the conformance expected-divergence
(upstream/053_repeat_times is now identical), and add parser, engine, and
eval-module unit tests.
@thymikee

Copy link
Copy Markdown
Member

Reviewed exact head 1b4e08204425c6a16175426469e3734942424e54. This is blocked by a security boundary, plus one vacuous regression.

P0 — remote caller-supplied YAML gains daemon-host code execution. Maestro source bundles are sent to the daemon; engine-eval-script.ts passes the inline expression to vm.runInNewContext. The supplied { ...values, output } context is escapable through the global constructor chain (reproduced by reading process.versions.node), which enables filesystem/environment access, child processes, and private-network requests. node:vm is explicitly not a security mechanism, and without microtaskMode: 'afterEvaluate' promise work can also escape the timeout. The existing daemon-local runScript trust contract does not transfer to inline remote payloads. The smallest safe contract is to reject evalScript for remotely supplied flows; remote support requires an OS/process-isolated evaluator with strict CPU, memory, filesystem, and network boundaries.

P2 — resume safety is untested. replay-plan-resume.ts adds evalScript to the no-skip condition, but the resume fixture only contains runScript; deleting the new condition still leaves the suite green. Add an explicit --from/plan-digest regression with a preceding evalScript.

After redesigning the trust boundary, add planted-red remote adversarial tests for process/filesystem/child-process/private-network access and execution-budget escape, then run exact-head CI/Size and repeat device evidence against the shipped path. Reference: https://nodejs.org/api/vm.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants