mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
f50839a928
* feat(integrations): support SPECIFY_<KEY>_EXTRA_ARGS env var for agent subprocess flags Read a per-integration env var (SPECIFY_<KEY>_EXTRA_ARGS) inside `SkillsIntegration.build_exec_args`, `MarkdownIntegration.build_exec_args`, and `TomlIntegration.build_exec_args` and append the parsed flags to the spawned agent's argv, gated per integration key. Operators can now opt into extra CLI flags (e.g. `SPECIFY_CLAUDE_EXTRA_ARGS=--dangerously-skip-permissions`) without modifying any SKILL or workflow YAML. Useful in CI / non-interactive contexts where the spawned `<agent> -p ...` would otherwise hang on an internal permission or input prompt invisible to the parent `specify workflow run` process. Key normalization: `kiro-cli` → `SPECIFY_KIRO_CLI_EXTRA_ARGS` (hyphen replaced with underscore, then uppercased). Default (env var unset or whitespace-only) is byte-identical to previous behaviour. Extra args are inserted between `-p prompt` and the model / output-format flags so they cannot clobber canonical Spec Kit args. Implementation: a single helper `IntegrationBase._apply_extra_args_env_var` encapsulates the env-var read + shlex parsing; each of the three concrete `build_exec_args` implementations calls it. Closes #2595 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(integrations): wire SPECIFY_<KEY>_EXTRA_ARGS into Codex/Devin/Opencode/Copilot Four integrations override `build_exec_args` and were silently ignoring the env-var hook introduced in the previous commit: - CodexIntegration (`codex exec ...`) - DevinIntegration (`devin -p ...`) - OpencodeIntegration (`opencode run ...`) - CopilotIntegration (`copilot -p ...`) Each now calls `self._apply_extra_args_env_var(args)` between the base argv and the canonical Spec Kit flags (matching the placement in `MarkdownIntegration`, `TomlIntegration`, and `SkillsIntegration`), so operator-injected flags cannot clobber `--model` / `--output-format` / `--json`. Adds 4 parameterized override-integration tests locking the wiring per agent. Also cleans up an inline `__import__("os").environ` in the fixture to a top-of-file `import os`. Drive-by typing fix: guard `self.registrar_config.get(...)` in `CopilotIntegration.add_commands` against the `None` case, matching the pattern already used in `base.py` for the same access. Addresses Copilot review on #2596. * fix(integrations): apply Opencode extra-args before prompt-derived --command When the Opencode prompt starts with `/`, `build_exec_args` injects `--command <X>` derived from the prompt. The previous placement of `self._apply_extra_args_env_var(args)` appended operator-injected args AFTER that `--command`, so a user setting `SPECIFY_OPENCODE_EXTRA_ARGS="--command override"` could redirect the command under typical last-wins repeated-flag CLI semantics. Move the hook to immediately after `args = [self.key, "run"]`, before the prompt-parsing block. The operator's `--command override` (if any) now precedes the Spec Kit-derived `--command speckit`, so the canonical choice wins. Adds `test_opencode_extra_args_cannot_clobber_prompt_derived_command` locking the ordering. Also corrects the module docstring to describe the hook as living in `IntegrationBase` (not `SkillsIntegration`) and to acknowledge that this file covers Codex/Devin/Opencode/Copilot in addition to SkillsIntegration stubs. Addresses Copilot review on #2596. * fix(integrations): honour SPECIFY_COPILOT_EXTRA_ARGS in dispatch_command `CopilotIntegration` is the only integration that overrides `dispatch_command` — it builds `cli_args` inline rather than going through `build_exec_args`. The previous commit wired `_apply_extra_args_env_var` into `build_exec_args` but workflow execution calls `dispatch_command`, so `SPECIFY_COPILOT_EXTRA_ARGS` was silently ignored at runtime. Add the hook in `dispatch_command` immediately after `cli_args = ["copilot", "-p", prompt]`, mirroring the placement in `build_exec_args` (between `-p prompt` and the canonical `--agent` / `--yolo` / `--model` / `--output-format` flags). `IntegrationBase.dispatch_command` already delegates to `build_exec_args`, so Codex, Devin, and Opencode continue to honour their respective env vars through inheritance — no further changes needed for them. Adds two end-to-end tests that monkeypatch `subprocess.run` and assert the env-var args reach the executed argv: - `test_copilot_dispatch_command_includes_extra_args` locks the bypass fix on the overridden path. - `test_codex_dispatch_command_includes_extra_args` locks the inherited-base-dispatch path for the other three integrations. Addresses Copilot review on #2596. * refactor(integrations): rename env var to SPECIFY_INTEGRATION_<KEY>_EXTRA_ARGS Per maintainer request: scope the operator-injected env var to the integration subsystem by prepending `INTEGRATION_` to the key segment, so it does not collide with other Spec Kit env-var namespaces. Renames everywhere it appears: - Helper `IntegrationBase._apply_extra_args_env_var` env_name format and docstring (`base.py`). - Inline comment in `CopilotIntegration.dispatch_command`. - All `monkeypatch.setenv(...)` calls, docstrings, and the autouse fixture's scope filter in `tests/integrations/test_extra_args.py`. No behaviour change beyond the variable name. Default (env var unset) still byte-identical to before this PR. Addresses review on #2596. * fix(integrations): raise actionable error on malformed EXTRA_ARGS quoting Wrap `shlex.split` in `_apply_extra_args_env_var` so an unmatched quote in `SPECIFY_INTEGRATION_<KEY>_EXTRA_ARGS` surfaces a clear `ValueError` naming the offending env var and showing the invalid value, instead of crashing workflow dispatch with a bare shlex traceback. Adds a new test locking the actionable error path. Addresses Copilot review feedback on #2596. * test(integrations): use `_copilot_executable()` in Copilot extra-args test `test_copilot_integration_honours_extra_args` hardcoded `"copilot"` in the expected argv, but `CopilotIntegration.build_exec_args` calls `_copilot_executable()` which returns `"copilot.cmd"` on Windows (`os.name == "nt"`). The test passed on Linux/macOS and failed on all three Windows-latest matrix entries. Resolve by importing `_copilot_executable` alongside `CopilotIntegration` and using it as the first expected argv token. The companion `test_copilot_dispatch_command_includes_extra_args` already uses `index()` lookups rather than full-argv equality so it was unaffected. * fix(integrations): couple Codex executable to self.key + cover base classes Two Copilot findings on the latest pass: 1. `CodexIntegration.build_exec_args` hardcoded the executable name as the literal `"codex"` while the env-var lookup derives from `self.key`. The two should stay coupled (matching Devin/Opencode, which both use `self.key` already). Replace the literal with `self.key` so the argv and env-var scoping cannot drift. 2. `tests/integrations/test_extra_args.py` covered the `SkillsIntegration` mechanism (via stubs near the top) and the four `build_exec_args` overrides (Codex/Devin/Opencode/Copilot) end-to-end, but did not exercise the `MarkdownIntegration` or `TomlIntegration` base implementations directly. Add bare `_MarkdownAgentStub` and `_TomlAgentStub` test stubs and a test each — the most common integration pattern (Amp, Auggie, Generic, Gemini, Tabnine, …) inherits without overriding, so the base wiring is now locked. Full suite: 3011 passed (was 3009), 40 skipped, no regressions. Addresses Copilot review feedback on #2596. * fix(integrations): rename env var to SPECKIT_INTEGRATION_<KEY>_EXTRA_ARGS Renames the env-var hook prefix from `SPECIFY_INTEGRATION_*` to `SPECKIT_INTEGRATION_*` to match the established codebase convention for integration-subsystem env vars (`SPECKIT_INTEGRATION_CATALOG_URL` in `integrations/catalog.py`, `SPECKIT_COPILOT_ALLOW_ALL_TOOLS` in `integrations/copilot/__init__.py`). The `SPECIFY_*` prefix is reserved for user-facing feature-resolution variables (`SPECIFY_FEATURE`, `SPECIFY_FEATURE_DIRECTORY`); reusing it for integration-subsystem scoping would introduce a second integration namespace under a different prefix, confusing operators who already set `SPECKIT_INTEGRATION_CATALOG_URL`. Also reverts the unrelated defensive `arg_placeholder` / `registrar_config is None` guard in `CopilotIntegration.setup_skills_mode` — it was a drive-by pyright cleanup mixed into this PR. Every concrete integration sets `registrar_config` so the guard never fires in practice; the typing issue belongs in a focused follow-up rather than this env-var-hook PR. Updates everywhere the old prefix appeared: - `IntegrationBase._apply_extra_args_env_var` helper + docstring - `CopilotIntegration.dispatch_command` inline comment - All `monkeypatch.setenv(...)` calls in `tests/integrations/test_extra_args.py` - The autouse fixture scope filter - Test module docstring Full suite: 3011 passed, 40 skipped, no regressions. Addresses Copilot review feedback on #2596. --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>