feat(orchestra): tell the user which options a command is missing

maestro-orchestra/src/main/java/maestro/orchestra/yaml/MaestroFlowParser.kt

@@ -41,6 +41,7 @@ import maestro.orchestra.error.InvalidFlowFile
 import maestro.orchestra.error.MediaFileNotFound
 import maestro.orchestra.util.Env.EnvVariableMissingValueError
 import maestro.orchestra.util.Env.withEnv
+import maestro.orchestra.yaml.schema.FlowCommandSchema
 import org.intellij.lang.annotations.Language
 import java.nio.file.Path
 import java.nio.file.Paths
@@ -56,9 +57,9 @@ import kotlin.reflect.jvm.javaType
 private val yamlFluentCommandConstructor = YamlFluentCommand::class.primaryConstructor!!
 private val yamlFluentCommandParameters = yamlFluentCommandConstructor.parameters
 private val yamlFluentCommandSourceInfoParameter = yamlFluentCommandParameters.first { it.name == "_sourceInfo" }
-private val objectCommands = yamlFluentCommandConstructor.parameters
-    .filter { it.name != "_sourceInfo" }
-    .map { it.name!! }
+// Lazy, not eager: FlowCommandSchema reads `stringCommands` further down this same file, so touching
+// it during this file's initialisation would read that map before it exists.
+private val objectCommands: List<String> by lazy { FlowCommandSchema.commands().map { it.name } }
 
 private const val PARSE_CONTEXT_ATTR = "maestroParseContext"
 
@@ -176,7 +177,7 @@ internal val stringCommands = mapOf<String, (YamlFluentCommand) -> YamlFluentCom
     "assertNoDefectsWithAI" to { it.copy(assertNoDefectsWithAI = YamlAssertNoDefectsWithAI()) },
 )
 
-private val allCommands = (stringCommands.keys + objectCommands).distinct()
+private val allCommands: List<String> by lazy { (stringCommands.keys + objectCommands).distinct() }
 
 private const val DOCS_FIRST_FLOW = "https://docs.maestro.dev/getting-started/writing-your-first-flow"
 private const val DOCS_COMMANDS = "https://docs.maestro.dev/api-reference/commands"
@@ -387,9 +388,9 @@ private object YamlCommandDeserializer : JsonDeserializer<YamlFluentCommand>() {
                 location = commandLocation,
                 title = "Missing Command Options",
                 errorMessage = """
-                    |The command `$commandText` requires additional options.
+                    |The command `$commandText` requires additional options.${optionsOf(commandText)}
                 """.trimMargin("|"),
-                // TODO: Add docs link
+                docs = DOCS_COMMANDS,
             )
         }
         throw ParseException(
@@ -478,6 +479,28 @@ private object YamlCommandDeserializer : JsonDeserializer<YamlFluentCommand>() {
         )
     }
 
+    /**
+     * What [commandName] is missing, read off [FlowCommandSchema] rather than written out here, so the
+     * message cannot name an option the parser does not accept.
+     */
+    private fun optionsOf(commandName: String): String {
+        val command = FlowCommandSchema.commands().firstOrNull { it.name == commandName } ?: return ""
+        if (command.selector) return " It takes an element selector, for example `$commandName: Login`."
+
+        val required = command.arguments.filter { it.required }.map { "`${it.name}`" }
+        val oneOf = command.requiredOneOf?.names.orEmpty().map { "`$it`" }
+        return buildString {
+            if (required.isNotEmpty()) append(" Requires ${required.joinToString(", ")}.")
+            when (oneOf.size) {
+                0 -> Unit
+                // A group of one is just "required" -- the parser enforces it in toCommands rather than
+                // through a non-null constructor parameter, but the user does not care why.
+                1 -> append(" Requires ${oneOf.single()}.")
+                else -> append(" Requires one of ${oneOf.joinToString(", ")}.")
+            }
+        }
+    }
+
     private fun suggestCommandMessage(invalidCommand: String): String {
         val prefixCommands = if (invalidCommand.length < 3) emptyList() else allCommands.filter { it.startsWith(invalidCommand) || invalidCommand.startsWith(it) }
         val substringCommands = if (invalidCommand.length < 3) emptyList() else allCommands.filter { it.contains(invalidCommand) || invalidCommand.contains(it) }

maestro-orchestra/src/test/java/maestro/orchestra/yaml/CommandSurfaceErrorsTest.kt

@@ -59,6 +59,46 @@ class CommandSurfaceErrorsTest {
         assertThat(error).doesNotContain("Did you mean")
     }
 
+    // ------------------------------------------ what the error says now that it reads the schema
+
+    @Test
+    fun `missing options names the required argument`() {
+        assertThat(errorFor("inputText")).contains("Requires `text`.")
+    }
+
+    @Test
+    fun `missing options names the alternatives when the command needs one of several`() {
+        assertThat(errorFor("runFlow")).contains("Requires one of `file`, `commands`.")
+        assertThat(errorFor("extendedWaitUntil")).contains("Requires one of `visible`, `notVisible`.")
+    }
+
+    @Test
+    fun `missing options names a single-member requirement without the one-of phrasing`() {
+        val error = errorFor("addMedia")
+
+        assertThat(error).contains("Requires `files`.")
+        assertThat(error).doesNotContain("one of")
+    }
+
+    @Test
+    fun `missing options says a selector command takes a selector`() {
+        assertThat(errorFor("tapOn")).contains("It takes an element selector")
+    }
+
+    /**
+     * The link is carried on the exception rather than inside the message, which is what the CLI and the
+     * workspace planner render from -- asserting on the message would pass for the wrong reason.
+     */
+    @Test
+    fun `missing options links to the command reference`() {
+        val thrown = runCatching {
+            MaestroFlowParser.parseFlow(Paths.get("test.yaml"), "appId: com.example.app\n---\n- inputText\n")
+        }.exceptionOrNull()
+
+        assertThat((thrown as FlowParseException).docs)
+            .isEqualTo("https://docs.maestro.dev/api-reference/commands")
+    }
+
     /** The parse error for a flow whose only command is [command] written bare, or null if it parses. */
     private fun errorFor(command: String): String? = runCatching {
         MaestroFlowParser.parseFlow(Paths.get("test.yaml"), "appId: com.example.app\n---\n- $command\n")

maestro-orchestra/src/test/resources/workspaces/e020_missing_command_options/error.txt

@@ -6,4 +6,5 @@ Missing Command Options at /tmp/WorkspaceExecutionPlannerErrorsTest_workspace/wo
              ^
        4 | 
 
-  The command `tapOn` requires additional options.
+  The command `tapOn` requires additional options. It takes an element selector, for example `tapOn: Login`.
+  See: https://docs.maestro.dev/api-reference/commands