[BUG] runScript: output.<array>.push() mutations don't persist; only whole-value reassignment works

[Sina-KH]

Hi. Thanks for your great runner! This is the issue I faced today, and decided to submit. It's documented by claude but is valid and was really hard to debug!

---

Description

runScript in maestro-runner mutates a snapshot of output, not the persistent bag. Object/array property mutations like output.list.push(x) are silently discarded across runScript calls — and even within the same script, reading back the value returns the unmutated snapshot. Scalar reassignments (output.foo = "x") DO persist.

This breaks any flow that builds up state across multiple runScript invocations using the documented output.<field> API. The same flows work in upstream maestro because Maestro's JS bridge exposes a live output reference.

Steps to Reproduce

Note on the IIFE wrapping below: maestro-runner reuses the same JS context across runScript calls — top-level const word = … in a script that runs more than once throws SyntaxError: Identifier 'word' has already been declared on the second invocation (vanilla maestro gives each call a fresh scope, so this works there). I wrap each script in an IIFE (function () { … })() to keep const locally scoped and avoid the collision while reproducing the actual output bug. The wrapping is unrelated to the persistence issue and is needed regardless.

1. Create three scripts:

   // init.js
   (function () {
       output.list = []
       output.list.push("a")
       console.log("[init] output.list =", JSON.stringify(output.list))
   })()

   // push.js
   (function () {
       output.list.push("b")
       console.log("[push] output.list =", JSON.stringify(output.list))
   })()

   // check.js
   (function () {
       console.log("[check] output =", JSON.stringify(output))
   })()

2. Flow:

   appId: <any>
   ---
   - runScript: init.js
   - runScript: push.js
   - runScript: check.js

3. Run with maestro-runner -p ios --device <udid> --app-file <app> test probe.yaml (also reproduces on Android with appropriate args).

Expected Behavior

[init] output.list = ["a"]
[push] output.list = ["a","b"]
[check] output = {"list":["a","b"]}

(Matches upstream maestro behavior.)

Actual Behavior

[init] output.list = []
[push] output.list = []
[check] output = {"list":[]}

The .push() mutation is invisible even one line later within the same script. The only way to persist a non-scalar is to reassign the whole value:

(function () {
    const list = output.list
    list.push("a")
    output.list = list   // reassign — only this persists
})()

Environment

• OS: macOS 26.4 (Darwin 25.4.0, arm64)
• maestro-runner version: 1.1.13 (commit 7addd21)
• Executor: Native (UIAutomator2 / WDA)
• Device/Simulator: iPhone 17e (iOS 26.4 Simulator) — also reproduced on Pixel 9 API 35 emulator

Flow File

appId: org.example.app
---
- runScript: init.js
- runScript: push.js
- runScript: check.js

Error Output

No error — silent data loss. Downstream tapOn: ${output.list[0]} resolves to empty string, which Maestro treats as a tap with empty selector and the test "passes" while doing nothing meaningful.

Additional Context

• Real-world impact: a wallet test reads 24 mnemonic words via copyTextFrom + runScript push, then runScript answer.js computes a verification answer from output.words. Under maestro-runner, output.words ends empty, so output.answerWords is empty, and the three tapOn: ${output.answerWords[N]} taps fire with empty selectors. Test reports passing taps but the app screen never advances.
• Two distinct semantics issues are entangled here: (a) shared JS context across runScript calls (causes const collisions, fixable with IIFE), (b) output reads return frozen/cloned snapshots so in-place mutations are invisible (cannot be worked around inside the script — must reassign the whole value).
• Workaround in scripts: wrap in IIFE to dodge (a), and always reassign the whole value (output.list = list after mutating) to dodge (b).

[omnarayan]

@Sina-KH Thanks for the really thorough write-up — the IIFE note especially helped, because it pointed straight at the fact that there were actually two bugs entangled here, not one. You called that out yourself in the "Additional Context" section, and you were exactly right. Both are real and both are now fixed.

Splitting them out for the changelog:

Bug 1 — shared JS context across runScript calls

Maestro CLI gives every runScript call its own fresh scope. We weren't doing that — every script was running against the same Goja runtime's global scope, so the second time any flow declared a top-level binding, you'd get SyntaxError: Identifier 'word' has already been declared. The IIFE wrapping you used was the right idea, you just shouldn't have had to write it.

Fix: ScriptEngine.RunScript now wraps the user script in an IIFE before evaluating it, so top-level const/let/var/function end up function-scoped to that invocation. The documented globals (output, console, json, http, maestro, env var stubs) still work fine because they live on the runtime's globalThis, not in any lexical parent scope.

Bug 2 — output.list.push() silently dropped

This is the nasty one, and the one that actually broke your wallet flow.

We were exposing the output JS global by passing the Go-side map[string]interface{} straight to Goja's runtime.Set. Goja was proxying that as a read-through-export object — so every read of a nested value (like output.list) was building a fresh JS array wrapper around an exported copy of the Go slice. .push() would mutate the wrapper, the wrapper got GC'd at end of statement, and the Go map never saw any of it. Scalar property writes worked fine because those hit the proxy's Set hook directly. That asymmetry is exactly what made this so hard to spot — and exactly what your three-script repro pinned down.

The downstream impact is the worst part of this bug, and you nailed it in your write-up: when output.words ends up [], then tapOn: ${output.words[N]} evaluates to a tap with an empty selector, and the runner happily reports the step as passing while the app screen never actually advances. Silent data loss showing up as a green test.

Fix: output is now a JS-native object (runtime.NewObject()) instead of a proxy over the Go map. All mutations happen JS-side with normal semantics. GetOutput() already deep-exports the JS tree back to map[string]interface{} at script exit, so everything downstream of output keeps working without changes.

What's shipped

Commit b5b0811 on main. Added five regression tests covering:

• array push
• nested object mutation
• whole-value reassignment (your existing workaround — still works)
• repeated const
• repeated let

Verified end-to-end with your exact three-script repro on a Pixel 4a / Android 33:

[init]  output.list = ["a"]
[push]  output.list = ["a","b"]
[check] output      = {"list":["a","b"]}

Matches your "Expected Behavior" section exactly.

Thanks again — the clean repro plus the fact that you'd already isolated the two semantics issues in your "Additional Context" made this much faster to fix than it had any right to be.