From b485cd8c1f27b75f02bef0337da5cb8a856c9c8a Mon Sep 17 00:00:00 2001 From: Ali jawwad <33836051+jawwad-ali@users.noreply.github.com> Date: Fri, 14 Aug 2026 01:02:48 +0500 Subject: [PATCH] fix(integrations): dispatch goose commands via `goose run` (#2416) (#3781) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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. ` invocation maps exactly onto `--recipe --params args=`. 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) * 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) * 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) --------- Co-authored-by: Claude Opus 5 (1M context) --- .../integrations/goose/__init__.py | 83 ++++++++++++++++ tests/integrations/test_extra_args.py | 43 ++++++++ tests/integrations/test_integration_goose.py | 98 +++++++++++++++++++ 3 files changed, 224 insertions(+) diff --git a/src/specify_cli/integrations/goose/__init__.py b/src/specify_cli/integrations/goose/__init__.py index 0af56907..caed191b 100644 --- a/src/specify_cli/integrations/goose/__init__.py +++ b/src/specify_cli/integrations/goose/__init__.py @@ -1,5 +1,7 @@ """Goose integration — open source AI agent (Agentic AI Foundation).""" +from __future__ import annotations + from ..base import YamlIntegration @@ -18,3 +20,84 @@ class GooseIntegration(YamlIntegration): "args": "{{args}}", "extension": ".yaml", } + + def build_exec_args( + self, + prompt: str, + *, + model: str | None = None, + output_json: bool = True, + ) -> list[str] | None: + """Build CLI arguments for non-interactive ``goose`` execution. + + ``YamlIntegration`` never overrode ``build_exec_args()``, so Goose + inherited the ``IntegrationBase`` no-op returning ``None``. Callers read + ``None`` as "this CLI is 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`` has no ``-p`` flag; its 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`` string + parameter, so a ``/speckit. `` invocation maps onto + ``--recipe --params args=``. Only that namespace is + mapped: ``--recipe`` is a *path* Spec Kit synthesizes, unlike + opencode's ``--command`` or hermes' ``-s``, which hand a bare name to + the agent's own resolver. Any other prompt -- including Goose's own + session commands such as ``/help`` or ``/plan`` -- goes to ``-t``. + """ + args = [self._resolve_executable(), "run"] + # Extra args are applied first, matching the opencode / codex / + # cursor-agent ordering. Positional parity only, NOT precedence: + # ``goose run`` is clap-derive based, and --recipe / --model / + # --output-format are single-value args with no ``args_override_self``, + # so re-passing any of them through + # SPECKIT_INTEGRATION_GOOSE_EXTRA_ARGS makes goose exit with + # "cannot be used multiple times" whichever side comes first. The same + # is true of goose's boolean flags. Only its ``Vec``-typed args (which + # clap infers as ArgAction::Append) may legitimately repeat. + self._apply_extra_args_env_var(args) + + if model: + args.extend(["--model", model]) + if output_json: + args.extend(["--output-format", "json"]) + + # Only the ``speckit.`` namespace maps to a recipe: this branch + # synthesizes a *path*, and ``command_filename()`` can only ever spell + # ``speckit..yaml``. ``PromptStep`` passes arbitrary ``prompt:`` + # strings here, so other slash text -- including Goose's own session + # commands ``/help`` and ``/plan`` -- must reach ``-t`` unchanged. + if prompt.startswith("/speckit."): + command, _, remainder = prompt[1:].partition(" ") + # ``command_filename`` re-adds the ``speckit.`` prefix and the + # ``.yaml`` extension, so strip it here; a dotted extension command + # (``speckit.git.commit``) round-trips too. A bare ``/speckit.`` + # leaves no stem and falls through to ``-t``. + stem = command[len("speckit."):] + if stem: + # Derive the recipe path from the same two sources ``setup()`` + # uses -- ``config["folder"]`` + ``config["commands_subdir"]`` + # (exactly what ``commands_dest()`` does) and + # ``command_filename()`` -- so the dispatch target cannot drift + # from the file that was actually installed. + folder = (self.config.get("folder") or "").strip("/") + subdir = (self.config.get("commands_subdir") or "").strip("/") + # Relative, forward-slash path: dispatch runs with + # ``cwd=project_root``, and goose accepts a POSIX separator on + # every platform (``commands_dest()`` yields backslashes on + # win32). + parts = [ + p for p in (folder, subdir, self.command_filename(stem)) if p + ] + args.extend(["--recipe", "/".join(parts)]) + if remainder.strip(): + args.extend(["--params", f"args={remainder}"]) + return args + + args.extend(["-t", prompt]) + return args diff --git a/tests/integrations/test_extra_args.py b/tests/integrations/test_extra_args.py index 84f48a5f..0ab68cb4 100644 --- a/tests/integrations/test_extra_args.py +++ b/tests/integrations/test_extra_args.py @@ -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 diff --git a/tests/integrations/test_integration_goose.py b/tests/integrations/test_integration_goose.py index 300b056c..a9780998 100644 --- a/tests/integrations/test_integration_goose.py +++ b/tests/integrations/test_integration_goose.py @@ -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..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()"