fix(integrations): dispatch goose commands via goose run (#2416) (#3781)

* fix(integrations): dispatch goose commands via `goose run` (#2416) `YamlIntegration` never overrode `build_exec_args()`, so `GooseIntegration` inherited the `IntegrationBase` no-op that returns `None`. Callers read `None` as "this CLI is unavailable", so every workflow command/prompt step targeting Goose reported `CLI not found or not installed` even with `goose` on PATH. Reproduced with the agent CLI present on PATH (shutil.which stubbed to a real path, subprocess.run stubbed): amp -> completed argv=['amp', '-p', '/speckit.specify'] opencode -> completed argv=['opencode', 'run', '--command', 'speckit.specify'] goose -> FAILED "integration 'goose' CLI not found or not installed" Implement `build_exec_args()` for Goose. Per the goose CLI docs there is no `-p` flag; the non-interactive entry point is `goose run`, which takes `-t/--text` for free-form text, `--recipe` for a stored recipe, `--params KEY=VALUE` for recipe parameters, plus `--model` and `--output-format`. Spec Kit installs its commands as Goose *recipes* under `.goose/recipes/`, each declaring an optional `args` parameter (already enforced by test_setup_declares_args_parameter_for_args_prompt), so a `/speckit.<name> <rest>` invocation maps exactly onto `--recipe <path> --params args=<rest>`. This mirrors `OpencodeIntegration`, which maps the same leading slash-command onto opencode's `--command`. The recipe path is derived from the same two sources `setup()` uses -- `config["folder"]` + `config["commands_subdir"]` and `command_filename()` -- so the dispatch target cannot drift from the installed file; a test asserts the resolved `--recipe` path exists after `setup()`. Dotted extension commands (`speckit.git.commit`) round-trip. Extra args are applied before the canonical flags so Spec Kit's selection stays authoritative, matching opencode. No behaviour change for other integrations, and `requires_cli` is untouched. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(goose): only map the speckit. namespace onto --recipe build_exec_args() treated every prompt starting with "/" as a Spec Kit recipe. Because command_filename() unconditionally re-adds the "speckit." prefix, a free-form slash prompt was silently promoted into a recipe run against a file that was never installed: /help -> --recipe .goose/recipes/speckit.help.yaml /plan the sprint -> --recipe .goose/recipes/speckit.plan.yaml /speckit. -> --recipe .goose/recipes/speckit..yaml PromptStep passes arbitrary prompt: strings to build_exec_args, and both /help and /plan are Goose's own session commands, so this is reachable. Unlike opencode's --command or hermes' -s, which hand a bare name to the agent's own resolver, --recipe is a path Spec Kit synthesizes -- so only the namespace it can actually spell may take that branch. Gate the branch on "/speckit." and fall through to -t otherwise. A bare "/speckit." leaves no stem and also falls through. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(goose): stop asserting an argv that goose would reject test_goose_extra_args_cannot_clobber_prompt_derived_recipe asserted that a duplicated --recipe is merely reordered, on a "last value wins" premise. That premise is wrong for goose: `goose run` is clap-derive based and --recipe/--model/--output-format are single-value args without args_override_self, so a duplicate makes goose exit with "cannot be used multiple times" whichever side comes first. The test passed in pytest while pinning a command line that cannot run. Replace it with an ordering-parity test that asserts only what Spec Kit actually controls: extra args precede the canonical flags (matching opencode/codex/cursor-agent), and Spec Kit never emits a duplicate single-value flag itself. Verified non-vacuous -- it fails if the extra-args hook is moved after the canonical flags. The ordering comment claimed precedence it cannot deliver; corrected to state positional parity only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-15 14:02:04 +02:00 · 2026-08-13 15:02:48 -05:00
parent 2b36f0ce94
commit b485cd8c1f
3 changed files with 224 additions and 0 deletions
@@ -565,6 +565,49 @@ def test_executable_env_var_devin_integration(monkeypatch):
    assert args[0] == "/opt/devin"


+def test_goose_integration_honours_extra_args(monkeypatch):
+    """Goose gained ``build_exec_args()`` (the Goose item in #2416), so it must
+    honour the shared extra-args hook like every other dispatching integration."""
+    from specify_cli.integrations.goose import GooseIntegration
+
+    monkeypatch.setenv("SPECKIT_INTEGRATION_GOOSE_EXTRA_ARGS", "--debug")
+    args = GooseIntegration().build_exec_args("hi", output_json=False)
+    assert args == ["goose", "run", "--debug", "-t", "hi"]
+
+
+def test_goose_extra_args_precede_canonical_flags(monkeypatch):
+    """Extra args are applied before Spec Kit's canonical flags, matching the
+    opencode / codex / cursor-agent ordering.
+
+    Ordering parity only. This deliberately does not assert that a duplicated
+    canonical flag gets overridden: ``goose run`` is clap-derive based, and its
+    ``--recipe`` / ``--model`` / ``--output-format`` are single-value args with
+    no ``args_override_self``, so duplicating one makes goose exit with "cannot
+    be used multiple times" regardless of which side wins the ordering.
+    """
+    from specify_cli.integrations.goose import GooseIntegration
+
+    monkeypatch.setenv("SPECKIT_INTEGRATION_GOOSE_EXTRA_ARGS", "--debug")
+    args = GooseIntegration().build_exec_args("/speckit.specify", model="gpt-4o")
+    assert args[:3] == ["goose", "run", "--debug"]
+    assert args.index("--debug") < args.index("--model")
+    assert args.index("--debug") < args.index("--output-format")
+    assert args.index("--debug") < args.index("--recipe")
+    # Spec Kit itself must never emit a duplicate single-value flag.
+    for flag in ("--recipe", "--model", "--output-format"):
+        assert args.count(flag) == 1
+
+
+def test_executable_env_var_goose_integration(monkeypatch):
+    """GooseIntegration honours the executable env var."""
+    from specify_cli.integrations.goose import GooseIntegration
+
+    monkeypatch.setenv("SPECKIT_INTEGRATION_GOOSE_EXECUTABLE", "/opt/goose")
+    args = GooseIntegration().build_exec_args("hi")
+    assert args[0] == "/opt/goose"
+    assert args[1] == "run"
+
+
 def test_executable_env_var_opencode_integration(monkeypatch):
    """OpencodeIntegration honours the executable env var."""
    from specify_cli.integrations.opencode import OpencodeIntegration
@@ -83,3 +83,101 @@ class TestGooseCommandPlaceholderResolution:
        assert "{SCRIPT}" not in prompt
        assert "__AGENT__" not in prompt
        assert "$ARGUMENTS" not in prompt
+
+
+class TestGooseCliDispatch:
+    """`goose` must produce argv for non-interactive dispatch.
+
+    `YamlIntegration` never overrode `build_exec_args()`, so Goose inherited the
+    `IntegrationBase` no-op returning `None`. Callers read `None` as "CLI
+    unavailable", so a workflow command/prompt step targeting Goose reported
+    "CLI not found or not installed" even with `goose` on PATH — the Goose item
+    in issue #2416. `goose run` supports `-t/--text`, `--recipe`,
+    `--params KEY=VALUE`, `--model` and `--output-format`.
+    """
+
+    def test_build_exec_args_is_not_none(self):
+        integration = get_integration("goose")
+        assert integration.build_exec_args("/speckit.specify") is not None
+
+    def test_slash_command_maps_to_recipe(self):
+        integration = get_integration("goose")
+        args = integration.build_exec_args("/speckit.specify", output_json=False)
+        assert args[1] == "run"
+        assert "--recipe" in args
+        assert args[args.index("--recipe") + 1] == ".goose/recipes/speckit.specify.yaml"
+        # No trailing args -> no --params
+        assert "--params" not in args
+
+    def test_slash_command_arguments_map_to_params(self):
+        integration = get_integration("goose")
+        args = integration.build_exec_args("/speckit.specify add auth", output_json=False)
+        assert args[args.index("--params") + 1] == "args=add auth"
+
+    def test_dotted_extension_command_maps_to_recipe(self):
+        integration = get_integration("goose")
+        args = integration.build_exec_args("/speckit.git.commit msg", output_json=False)
+        assert args[args.index("--recipe") + 1] == (
+            ".goose/recipes/speckit.git.commit.yaml"
+        )
+
+    def test_free_form_prompt_uses_text_flag(self):
+        """goose has no `-p`; free-form text goes to `-t/--text`."""
+        integration = get_integration("goose")
+        args = integration.build_exec_args("just do it", output_json=False)
+        assert args[-2:] == ["-t", "just do it"]
+        assert "--recipe" not in args
+
+    def test_non_speckit_slash_prompt_is_not_treated_as_a_recipe(self):
+        """`/help` is a goose session command, not a Spec Kit recipe.
+
+        `PromptStep` passes arbitrary `prompt:` strings to `build_exec_args`,
+        and the recipe branch synthesizes a *file path*, so slash text outside
+        the `speckit.` namespace must not become
+        `--recipe .goose/recipes/speckit.help.yaml` — `setup()` only ever
+        writes `command_filename(stem)` = `speckit.<name>.yaml`.
+        """
+        integration = get_integration("goose")
+        args = integration.build_exec_args("/help", output_json=False)
+        assert "--recipe" not in args
+        assert "--params" not in args
+        assert args[-2:] == ["-t", "/help"]
+
+    def test_non_speckit_slash_prompt_is_not_promoted_to_a_recipe(self):
+        """`/plan` is goose's own command and must not run speckit.plan.
+
+        `command_filename()` re-adds the `speckit.` prefix, so the old
+        unconditional call silently promoted the free-form goose command
+        `/plan` into a real Spec Kit recipe run. Dispatch always spells
+        commands `/speckit.plan` (`IntegrationBase.build_command_invocation`),
+        so no reachable recipe is lost.
+        """
+        integration = get_integration("goose")
+        args = integration.build_exec_args("/plan the sprint", output_json=False)
+        assert "--recipe" not in args
+        assert args[-2:] == ["-t", "/plan the sprint"]
+
+    def test_bare_speckit_prefix_falls_through_to_text(self):
+        """`/speckit.` alone has no stem and must not yield `speckit..yaml`."""
+        integration = get_integration("goose")
+        args = integration.build_exec_args("/speckit.", output_json=False)
+        assert "--recipe" not in args
+        assert args[-2:] == ["-t", "/speckit."]
+
+    def test_model_and_output_format_flags(self):
+        integration = get_integration("goose")
+        args = integration.build_exec_args("hi", model="gpt-4o", output_json=True)
+        assert args[args.index("--model") + 1] == "gpt-4o"
+        assert args[args.index("--output-format") + 1] == "json"
+
+    def test_recipe_target_matches_what_setup_writes(self, tmp_path):
+        """Anti-drift: the dispatched `--recipe` path must be the file `setup()`
+        actually installed, so the two cannot diverge."""
+        integration = get_integration("goose")
+        manifest = IntegrationManifest("goose", tmp_path)
+        created = integration.setup(tmp_path, manifest, script_type="sh")
+        assert created
+
+        args = integration.build_exec_args("/speckit.specify hello")
+        recipe = args[args.index("--recipe") + 1]
+        assert (tmp_path / recipe).is_file(), f"{recipe} was not installed by setup()"