feat(orchestra): add custom commands with typed arguments

[sidferreira]

Proposed changes

Adds support for declaring custom commands directly in YAML flows. A flow file can declare itself as a reusable, first-class command via a command: + arguments: header in its config block; callers invoke it like a built-in instead of going through runFlow: + env:.

Example — declare:

# subflows/greet.yaml
appId: com.example
command: greet
arguments:
  - { name: who,   type: string,  required: true }
  - { name: count, type: number,  default: 1 }
---
- inputText: "Hello \${who}"

Example — call:

- greet:
    who: world

Highlights:

• Workspace mode auto-registers any flow with a command: header.
• Single-file mode (maestro test foo.yaml) auto-discovers commands from <flow parent>/subflows/.
• Arguments support string / number / boolean with required and default.
• Parse-time validation: unknown keys, missing required, type mismatch.
• Custom-command bodies cannot invoke other custom commands (no nesting); enforced explicitly during the workspace pre-pass.
• A custom-command call desugars at parse time into a RunFlowCommand with a prepended DefineVariablesCommand — zero changes to Orchestra's dispatcher.

A cross-cutting fix is bundled: TestCommand.makeChunkPlans was reconstructing ExecutionPlan and silently dropping the discovered registry — even single-file runs go through chunking, so the registry was being lost between plan() and the run. This had no prior visible effect (the registry was always empty before this change) but had to be fixed for the feature to function end-to-end.

Testing

• New unit tests in YamlCommandReaderTest (header parsing, call expansion, all four validation error types, suggestion-list inclusion).
• New planner tests in WorkspaceExecutionPlannerTest (workspace-mode discovery, single-file subflows/ discovery, command-definition files excluded from runnable flows, nested-custom-command rejection, malformed-command-file is skipped during discovery).
• New end-to-end test in maestro-test/IntegrationTest (Case 100) — drives Orchestra with a fake driver and asserts the executed sequence.
• All pre-existing tests continue to pass (./gradlew :maestro-orchestra-models:test :maestro-orchestra:test :maestro-test:test).
• Manual end-to-end against an Android emulator — full reproduction steps + sample fixtures: feat(orchestra): add custom commands with typed arguments #3339 (comment)
• Self code-review (Kotlin best practices) — findings + resolutions: feat(orchestra): add custom commands with typed arguments #3339 (comment)

Does this need e2e tests? — The feature is parser/orchestra-layer and is covered by integration tests under maestro-test. No demo-app change needed.

Issues fixed

N/A

[sidferreira]

Self code-review findings (Kotlin best practices + code quality)

Pre-merge findings I'd like reviewers to weigh in on. None are blocking compile/tests, but some are worth addressing before merge. Counts: 0 Critical / 4 Important / 6 Minor.

Update: All 10 findings have been addressed in follow-up commits (91c5fcf1..618192e0). Each fix is a separate, individually-verifiable commit so the history is bisectable.

Important

1. ✅ Files.walk not closed — WorkspaceExecutionPlanner.kt (in discoverCustomCommandsFromSubflowsDir and the workspace pre-pass .maestro/commands/ walk). The returned Stream is backed by an open directory handle; leaks an FD per planner invocation. Wrap with .use { ... }. (The same pattern appears in WorkspaceUtils.kt already, so this is a pre-existing convention worth correcting.)
   Fixed in 91c5fcf1.

2. ✅ catch (_: Throwable) is too broad — WorkspaceExecutionPlanner.discoverCustomCommands and the nesting-detection loop. Catches OutOfMemoryError, StackOverflowError, CancellationException. Also: a malformed command-definition file is silently dropped from the registry, so callers downstream get a confusing "Invalid Command: did you mean…" suggestion instead of the real syntax error. Narrow to Exception (or specifically IOException / JsonProcessingException / ValidationError) and at minimum log a DEBUG entry when swallowing.
   Fixed in c621af6d — narrowed to Exception, added DEBUG log, added regression test (022_malformed_command_file).

3. ✅ Nesting detection is fragile — WorkspaceExecutionPlanner compares RunFlowCommand.sourceDescription (a String) against CustomCommandDef.sourceFile.toString(). Works today because both go through .absolute().toString(), but any future normalization (.normalize(), symlink resolution) silently breaks nesting detection. Recommend adding a typed customCommandName: String? field to RunFlowCommand populated by buildCustomCommandRunFlow, then checking that field instead of string-matching.
   Fixed in 1c758a9a — added typed customCommandName to RunFlowCommand, nesting detection now uses direct map lookup by name.

4. ✅ appId = url ?: _appId ?: "" sentinel — YamlConfig.kt. Command-definition files (which legitimately have no app target) end up with an empty-string appId in MaestroConfig. Fine if all consumers use isNotBlank(); latent bug if anyone treats it as a literal bundle ID. Worth either making appId nullable or adding a comment documenting the sentinel.
   Fixed in 618192e0 — branch the init: runnable flows keep the original _appId!! assertion; command-definition files take the explicit "" fallback with a comment explaining why it's safe. Preserves the pre-feature invariant without changing the public type.

Minor

5. ✅ validateAndCoerceArgs — call.args.keys - knownNames allocates an extra Set; filterNot { it in knownNames } reads better.
   Fixed in 325f1451.

6. ✅ Number coercion duplicates the toDoubleOrNull()-for-side-effect pattern in both YamlFlowArgument.coerceDefault and YamlFluentCommand.coerceArg. Extract a requireNumeric(str): String helper.
   Fixed in 0747e9d3.

7. ✅ Boolean coercion already computes asString; small .also { require(...) } polish opportunity.
   Fixed in 46cf8ba9 — extracted requireBoolean helper to mirror requireNumeric.

8. ✅ TestCommand.kt:559, TestRunner.kt:54, TestSuiteInteractor.kt:161 — fully-qualified maestro.orchestra.CustomCommandDef in signatures. Should be imported at file top per Kotlin convention.
   Fixed in eda09fcb.

9. ✅ MaestroFlowParser.kt:563-566 — builtInCommandNames (Set) AND isBuiltInCommand(name) are both exposed. Only the latter is used externally — drop one.
   Fixed in 9dc12002 — dropped the unused builtInCommandNames set.

10. ✅ YamlConfig.toCustomCommandArguments() is public but only called from toCommand(...) in the same file — make it internal or private.
    Fixed in b8314570 — narrowed to internal (planner calls it cross-file).

What's well-done

• @JsonIgnore customCommand + INTERNAL_FLUENT_COMMAND_FIELDS exclusion cleanly extends the existing _sourceInfo pattern to keep the field out of YAML deserialization while still allowing callBy to set it.
• Token-consumption in parseCustomCommandCall correctly mirrors the built-in command path's END_OBJECT assertion.
• Argument coercion uses exhaustive when over ArgumentType — no fallthrough risk.

---

All findings resolved. Each fix is in its own commit (see git log a07efea3..HEAD). Full regression (:maestro-orchestra-models:test :maestro-orchestra:test :maestro-test:test) passes, plus Android-emulator E2E (maestro test samples/screenshot-command-android.yaml) verified after every commit.

[sidferreira]

Manual reproduction — takeScreenshotInFormat custom command

This is the end-to-end example I used while developing/verifying the feature.

Setup

1. Build & install the CLI from this branch:

   ./gradlew :maestro-cli:installDist -x installMcpViewerDeps -x buildMcpViewer
   rm -rf ~/.maestro/bin ~/.maestro/lib
   cp -r ./maestro-cli/build/install/maestro/bin ~/.maestro/bin
   cp -r ./maestro-cli/build/install/maestro/lib ~/.maestro/lib
   export PATH="$HOME/.maestro/bin:$PATH"
   maestro --version

2. Download the standard samples (provides the Wikipedia APK + the conventional subflows/ folder):

   maestro download-samples
   cd samples

3. Start an Android emulator and verify it's attached:

   adb devices
   # Expected: emulator-XXXX  device

4. Install the Wikipedia app (target appId org.wikipedia):

   adb install wikipedia.apk

Add the two files

subflows/takeScreenshotInFormat.yaml — the custom command definition:

command: takeScreenshotInFormat
arguments:
  - { name: platform, type: string, required: true }
---
- takeScreenshot: "screenshot-from-command--${platform}"

screenshot-command-android.yaml — the caller flow (alongside the existing samples):

appId: org.wikipedia
tags:
  - android
---
- launchApp
- takeScreenshotInFormat:
    platform: android

(iOS variant: appId: org.wikimedia.wikipedia + tags: [ios] — works against an iOS Simulator with the Wikipedia app installed.)

Run

rm -f screenshot-from-command--android.png
maestro test screenshot-command-android.yaml
ls -la screenshot-from-command--android.png      # ~20–100 KB

Expected flow output

 > Flow screenshot-command-android
Launch app "org.wikipedia"... COMPLETED
Run /path/to/samples/subflows/takeScreenshotInFormat.yaml...
  Take screenshot screenshot-from-command--${platform}... COMPLETED
Run /path/to/samples/subflows/takeScreenshotInFormat.yaml... COMPLETED

Note screenshot-from-command--${platform} is shown unevaluated in the step log (it's the raw path: value before interpolation), but the actual file on disk has platform substituted: screenshot-from-command--android.png.

Negative-path sanity check

Drop the platform: line from screenshot-command-android.yaml and re-run — parsing fails at the call site with:

Missing required argument 'platform' for command 'takeScreenshotInFormat'

Filename note

The screenshot writes as .png regardless of the path: value. Orchestra.takeScreenshotCommand unconditionally appends .png — that's existing Maestro behavior, intentionally not touched by this PR.