Commit Graph

5 Commits

Author SHA1 Message Date
Copilot
b4e5a1c3be feat: support SPECKIT_INTEGRATION_<KEY>_EXECUTABLE env var (#2743)
* Initial plan

* feat: support SPECKIT_INTEGRATION_<KEY>_EXECUTABLE env var override

Adds `IntegrationBase._resolve_executable()` which reads
`SPECKIT_INTEGRATION_<KEY>_EXECUTABLE` (hyphens→underscores, uppercased)
and falls back to `self.key` when unset or whitespace-only.

All concrete `build_exec_args()` implementations now call
`self._resolve_executable()` instead of hard-coding `self.key` or
`"agy"` as the first argv token:
- MarkdownIntegration, TomlIntegration, SkillsIntegration (base classes)
- CodexIntegration, DevinIntegration, OpencodeIntegration, HermesIntegration, AgyIntegration
- CopilotIntegration (overrides `_resolve_executable()` to fall back to
  the platform-specific `_copilot_executable()` default; also updates
  `dispatch_command()` to use `_resolve_executable()`)

Tests added to tests/integrations/test_extra_args.py covering:
- default (unset) falls back to key
- env var replaces first argv token
- whitespace-only env var is a no-op
- key hyphen→underscore normalisation
- cross-integration scoping (wrong key ignored)
- all override integrations (Codex, Devin, Opencode, Copilot)
- Copilot dispatch_command path
- EXECUTABLE and EXTRA_ARGS can be set simultaneously

See issue #2596."

* fix: complete docstring sentence in _resolve_executable

* test: generalize extra-args test comments for override coverage

* Fix stale Codex executable comment

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
2026-05-29 10:19:31 -05:00
Huy Do
f50839a928 feat(integrations): support SPECIFY_<KEY>_EXTRA_ARGS env var for agent subprocess flags (#2596)
* 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>
2026-05-28 14:00:15 -05:00
Marcus Burghardt
9732a4d092 fix(opencode): use commands/ directory (plural) to match OpenCode docs (#2453)
* fix(opencode): use commands/ directory (plural) to match OpenCode docs

OpenCode documentation (https://opencode.ai/docs/commands/) uses
.opencode/commands/ (plural) as the canonical command directory.
The OpenCode runtime supports both .opencode/command/ and
.opencode/commands/ via a {command,commands} glob, but the
singular form was the original convention and is now outdated.

Update the OpenCode integration to write to .opencode/commands/
instead of .opencode/command/, aligning with the documented
standard and the OpenSpec fix (Fission-AI/OpenSpec#748).

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
Assisted-by: OpenCode (claude-opus-4-6)

* feat(registrar): add legacy_dir fallback for backward-compatible directory migration

Add _resolve_agent_dir() to CommandRegistrar that checks a
legacy_dir fallback when the canonical directory does not exist.
When legacy_dir is found, a deprecation warning directs users to
run "specify integration upgrade" to migrate.

The OpenCode integration declares legacy_dir: ".opencode/command"
so that extension and preset registration, as well as command
cleanup, continue working for projects that have not yet migrated
to .opencode/commands/.

The legacy_dir mechanism is opt-in: integrations that do not
declare it get no fallback and no behavioral change.

Add end-to-end test verifying that "specify integration upgrade
opencode" migrates commands from legacy .opencode/command/ to
canonical .opencode/commands/ and removes stale files.

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
Assisted-by: OpenCode (claude-opus-4-6)

* fix(registrar): address PR review feedback on legacy_dir handling

- Fix deprecation warning formatting: quote paths and remove trailing
  '/.' that produced confusing '.opencode/commands/.' output
- Eliminate duplicate warnings: pass pre-resolved directory to
  register_commands() via _resolved_dir parameter so
  _resolve_agent_dir() is only called once per agent
- Fix unregister_commands() to clean both canonical and legacy dirs
  when both exist, preventing orphaned command files after upgrade
- Add test_unregister_cleans_legacy_when_both_dirs_exist regression
  test and tighten warning count assertion to exactly 1

Assisted-by: OpenCode (claude-opus-4-6)
Signed-off-by: Marcus Burghardt <maburgha@redhat.com>

---------

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
2026-05-13 09:55:56 -05:00
Andrii Furmanets
c079b2cc32 fix: dispatch opencode commands via run (#2410) 2026-04-29 09:39:45 -05:00
Manfred Riem
255371d367 Stage 3: Standard markdown integrations — 19 agents migrated to plugin architecture (#2038)
* Stage 3: Standard markdown integrations — 19 agents migrated to plugin architecture

Migrate all standard markdown integrations to self-contained subpackages
under integrations/. Each subclasses MarkdownIntegration with config-only
overrides (~10 lines per __init__.py).

Integrations migrated (19):
claude, qwen, opencode, junie, kilocode, auggie, roo, codebuddy,
qodercli, amp, shai, bob, trae, pi, iflow, kiro-cli, windsurf,
vibe, cursor-agent

Changes:
- Create integrations/<key>/ subpackage with __init__.py and scripts/
  (update-context.sh, update-context.ps1) for each integration
- Register all 19 in INTEGRATION_REGISTRY (20 total with copilot)
- MarkdownIntegration.setup() processes templates (replaces {SCRIPT},
  {ARGS}, __AGENT__; strips frontmatter blocks; rewrites paths)
- Extract install_scripts() to IntegrationBase; refactor copilot to use it
- Generalize --ai auto-promote from copilot-only to registry-driven:
  any integration registered in INTEGRATION_REGISTRY auto-promotes.
  Unregistered agents (gemini, tabnine, codex, kimi, agy, generic)
  continue through the legacy --ai path unchanged.
- Fix cursor/cursor-agent key mismatch in CommandRegistrar.AGENT_CONFIGS
- Add missing vibe entry to CommandRegistrar.AGENT_CONFIGS
- Update kiro alias test to reflect auto-promote behavior

Testing:
- Per-agent test files (test_integration_<agent>.py) with shared mixin
- 1316 tests passing, 0 failures
- Complete file inventory tests for both sh and ps variants
- Byte-for-byte validated against v0.4.3 release packages (684 files)

* Address PR review: fix repo root detection and no-op test

- Fix repo root fallback in all 20 update-context.sh scripts: walk up
  from script location to find .specify/ instead of falling back to pwd
- Fix repo root fallback in all 20 update-context.ps1 scripts: walk up
  from script location to find .specify/ instead of falling back to $PWD
- Add assertions to test_setup_writes_to_correct_directory: verify
  expected_dir exists and all command files reside under it

* Fix REPO_ROOT priority: prefer .specify walk-up over git root

In monorepos the git toplevel may differ from the project root that
contains .specify/. The previous fix still preferred git rev-parse
over the walk-up result.

Bash scripts (20): prefer the discovered _root when it contains
.specify/; only accept git root if it also contains .specify/.

PowerShell scripts (20): validate git root contains .specify/ before
using it; fall back to walking up from script directory otherwise.

* Guard git call with try/catch in PowerShell scripts

With $ErrorActionPreference = 'Stop', an unguarded git rev-parse
throws a terminating CommandNotFoundException when git is not
installed, preventing the .specify walk-up fallback from running.

Wrap the git call in try/catch across all 20 update-context.ps1
scripts so the fallback works reliably without git.

* Rename hyphenated package dirs to valid Python identifiers

Rename kiro-cli → kiro_cli and cursor-agent → cursor_agent so the
packages can be imported with normal Python syntax instead of
importlib.  The user-facing integration key (IntegrationBase.key)
stays hyphenated to match the actual CLI tool / binary name.

Also reorganize _register_builtins(): imports and registrations
are now grouped alphabetically with clear section comments.

* Reuse CommandRegistrar path rewriting in process_template()

Replace the duplicated regex-based path rewriting in
MarkdownIntegration.process_template() with a call to the shared
CommandRegistrar._rewrite_project_relative_paths() implementation.

This ensures extension-local paths are preserved and boundary rules
stay consistent across the codebase.

* Promote _rewrite_project_relative_paths to public API

Rename CommandRegistrar._rewrite_project_relative_paths() to
rewrite_project_relative_paths() (drop leading underscore) so
integrations can call it without reaching into a private method
across subsystem boundaries.

Addresses PR review feedback:
https://github.com/github/spec-kit/pull/2038#discussion_r3022105627

* Broaden TestRegistrarKeyAlignment to cover all integration keys

Parametrize across ALL_INTEGRATION_KEYS instead of only checking
cursor-agent and vibe.  Keeps a separate negative test for the
stale 'cursor' shorthand.

Addresses PR review feedback:
https://github.com/github/spec-kit/pull/2038#discussion_r3022269032
2026-04-01 09:17:21 -05:00