8 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
Puneet Dixit	5a678c552e	Share skills hook note post-processing (#2679) ... * fix(integrations): share skills hook note post-processing * fix(integrations): tighten skill post-processing Apply skill content post-processing before the initial write, use an exact hook-note sentinel for idempotence, and route Copilot skill post-processing through the shared helper before adding mode frontmatter. * Make hook note injection per instruction * Deduplicate Codex hook note processing --------- Co-authored-by: Puneet Dixit <236133619+puneetdixit200@users.noreply.github.com> Co-authored-by: Puneet Dixit <puneetdixit200@users.noreply.github.com>	2026-05-28 09:08:48 -05:00
Chao Z	9735145289	fix(codex): inject dot-to-hyphen hook command note in Codex skills (#2503) ... * fix(codex): inject dot-to-hyphen hook command note in Codex skills Hook commands in `.specify/extensions.yml` use dotted ids like `speckit.git.commit`, but Codex skills are named with hyphens (`speckit-git-commit`). The Claude integration handles this via an explicit instruction injected into each generated SKILL.md by `ClaudeIntegration.post_process_skill_content`, but the Codex integration had no such override, so Codex would emit `/speckit.git.commit` (which does not resolve) instead of `/speckit-git-commit`. This adds the same `_inject_hook_command_note` helper and a `post_process_skill_content` override to `CodexIntegration`, plus a small `setup()` override that applies the post-process to each generated SKILL.md (mirroring the pattern in `ClaudeIntegration`). Also widens the existing `test_non_claude_post_process_is_identity` test to use `agy` (another `SkillsIntegration` with no override), since asserting identity behavior on Codex would now incorrectly fail. Tests: - New `TestCodexHookCommandNote` class mirrors `TestClaudeHookCommandNote`: setup-level injection, no-op when no hook block is present, idempotency, and indentation preservation. - `pytest tests/` → 2866 passed, 34 skipped. Signed-off-by: Chao Zhang <1175468+picklebento@users.noreply.github.com> * fix(codex): handle empty eol when instruction is final line without newline The hook-note injection regex allowed end-of-string matches via ``$``, which left the captured ``eol`` empty. When the matched indent was also empty, the substitution concatenated the note onto the same line as the instruction. Default ``eol`` to ``\n`` when the capture is empty. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Signed-off-by: Chao Zhang <1175468+picklebento@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>	2026-05-20 09:04:47 -05:00
Pascal THUET	38fd1f6cc2	Support controlled multi-install for safe AI agent integrations (#2389) ... * support controlled multi-install integrations * fix: harden multi-install integration state * refactor: isolate integration runtime helpers * fix: address copilot review feedback * fix: address follow-up copilot feedback * fix: tighten integration switch semantics * fix: address final copilot review feedback * fix: harden integration manifest read errors * fix: refuse symlinked shared infra paths * test: filter expected self-test preset warning * test: address copilot review nits * refactor: centralize safe shared infra writes * fix: use no-follow writes for shared infra * fix: keep default integration atomic on template refresh * fix: harden shared infra error paths * fix: preflight shared infra and future state schemas * fix: support nested shared scripts during preflight * test: tolerate wrapped schema error output * fix: use safe default mode for shared text writes * fix: use posix paths in shared skip output * fix: share project guard for integration use * fix: centralize spec-kit project guards * fix: use posix project paths in cli output * fix: harden shared manifest and upgrade refresh	2026-05-01 11:54:41 -05:00
Manfred Riem	fc3d1244c0	fix: replace shell-based context updates with marker-based upsert (#2259) ... * Replace shell-based context updates with marker-based upsert Replace ~3500 lines of bash/PowerShell agent context update scripts with a Python-based approach using <!-- SPECKIT START/END --> markers. IntegrationBase now manages the agent context file directly: - upsert_context_section(): creates or updates the marked section at init/install/switch time with a directive to read the current plan - remove_context_section(): removes the section at uninstall, deleting the file only if it becomes empty - __CONTEXT_FILE__ placeholder in command templates is resolved per integration so the plan command references the correct agent file - context_file is persisted in init-options.json for extension access The plan command template instructs the LLM to update the plan reference between the markers in the agent context file. Removed: - scripts/bash/update-agent-context.sh (857 lines) - scripts/powershell/update-agent-context.ps1 (515 lines) - 56 integration wrapper scripts (update-context.sh/.ps1) - templates/agent-file-template.md - agent_scripts frontmatter key and {AGENT_SCRIPT} replacement logic - update-context reference from integration.json - tests/test_cursor_frontmatter.py (tested deleted scripts) Added: - upsert/remove context section methods on IntegrationBase - __CONTEXT_FILE__ placeholder support in process_template() - context_file field in init-options.json (init/switch/uninstall) - Per-integration tests: context file correctness, plan reference, init-options persistence (78 new context_file tests) - End-to-end CLI validation across all 28 integrations * fix: search for end marker after start marker in context section methods Address Copilot review: content.find(CONTEXT_MARKER_END) searched from the start of the file rather than after the located start marker. If the file contained a stray end marker before the start marker, the wrong slice could be replaced. Now both upsert_context_section() and remove_context_section() pass start_idx as the second argument to find() and validate end_idx > start_idx before performing the replacement. * fix: address Copilot review feedback on context section handling 1. Fix grammar in _build_context_section() directive text — add commas for a complete sentence. 2. Resolve __CONTEXT_FILE__ in resolve_skill_placeholders() — skills generated via extensions/presets for codex/kimi now replace the placeholder using the context_file value from init-options.json. 3. Handle Cursor .mdc frontmatter — when creating a new .mdc context file, prepend alwaysApply: true YAML frontmatter so Cursor auto-loads the rules. 4. Fix empty-file leading newline — when the context file exists but is empty, write the section directly instead of prepending a blank line. * fix: address second round of Copilot review feedback 1. Ensure .mdc frontmatter on existing files — upsert_context_section() now checks for missing YAML frontmatter on .mdc files during updates (not just creation), so pre-existing Cursor files get alwaysApply. 2. Guard against context_file=None — use 'or ""' instead of a default arg so explicit null values in init-options.json don't cause a TypeError in str.replace(). 3. Clean up .mdc files on removal — remove_context_section() treats files containing only the Speckit-generated frontmatter block as empty, deleting them rather than leaving orphaned frontmatter. * fix: address third round of Copilot review feedback 1. CRLF-safe .mdc frontmatter check — use lstrip().startswith('---') instead of startswith('---\n') so CRLF files don't get duplicate frontmatter. 2. CRLF-safe .mdc removal check — normalize line endings before comparing against the sentinel frontmatter string. 3. Call remove_context_section() during integration_uninstall() — the manifest-only uninstall was leaving the managed SPECKIT markers behind in the agent context file. 4. Fix stale docstring — remove 'agent_scripts' mention from test_lean_commands_have_no_scripts(). * fix: address fourth round of Copilot review feedback 1. Remove unused script_type parameter from _write_integration_json() and all 3 call sites — the parameter was no longer referenced after the update-context script removal. 2. Fix _build_context_section() docstring — correct example path from '.specify/plans/plan.md' to 'specs/<feature>/plan.md'. 3. Improve .mdc frontmatter-only detection in remove_context_section() — use regex to match any YAML frontmatter block (not just the exact Speckit-generated one), so .mdc files with additional frontmatter keys are also cleaned up when no body content remains. * fix: handle corrupted markers and parse .mdc frontmatter robustly 1. Handle partial/corrupted markers in upsert_context_section() — if only the START marker exists (no END), replace from START through EOF. If only the END marker exists, replace from BOF through END. This keeps upsert idempotent even when a user accidentally deletes one marker. 2. Parse .mdc YAML frontmatter properly — new _ensure_mdc_frontmatter() helper parses existing frontmatter and ensures alwaysApply: true is set, rather than just checking for the --- delimiter. Handles missing frontmatter, existing frontmatter without alwaysApply, and already-correct frontmatter. * fix: preserve .mdc frontmatter, add tests, clean up on switch 1. Rewrite _ensure_mdc_frontmatter() with regex — preserves comments, formatting, and custom keys in existing frontmatter instead of destructively re-serializing via yaml.safe_dump(). Inserts or fixes alwaysApply: true in place. 2. Add 6 focused .mdc frontmatter tests to cursor-agent test file: new file creation, missing frontmatter, preserved custom keys, wrong alwaysApply value, idempotent upserts, removal cleanup. 3. Call remove_context_section() during integration switch Phase 1 — prevents stale SPECKIT markers from being left in the old integration's context file. Also clear context_file from init-options during the metadata reset. * fix: remove unused MDC_FRONTMATTER, preserve inline comments, normalize bare CR 1. Remove unused MDC_FRONTMATTER class variable — dead code after _ensure_mdc_frontmatter() was rewritten with regex. 2. Preserve inline comments when fixing alwaysApply — the regex substitution now captures trailing '# comment' text and keeps it. 3. Normalize bare CR in upsert_context_section() — match the behavior of remove_context_section() which already normalizes both CRLF and bare CR. 4. Clarify .mdc removal comment — 'treat frontmatter-only as empty' instead of misleading 'strip frontmatter'. * fix: handle corrupted markers in remove, CRLF-safe end-marker consumption 1. Handle corrupted markers in remove_context_section() — mirror upsert's behavior: start-only removes start→EOF, end-only removes BOF→end. Previously bailed out leaving partial markers behind. 2. CRLF-safe end-marker consumption — both upsert and remove now handle \r\n after the end marker, not just \n. Prevents extra blank lines at replacement boundaries in CRLF files. 3. Clarify path rule in plan template — distinguish filesystem operations (absolute paths) from documentation/agent context references (project-relative paths). * fix: only remove context section when both markers are well-ordered remove_context_section() previously treated mismatched markers as corruption and aggressively removed from BOF→end-marker or start-marker→EOF, which could delete user-authored content if only one marker remained. Now it only removes when both START and END markers exist and are properly ordered, returning False otherwise.	2026-04-17 13:57:51 -05:00
Copilot	a00e679918	Add workflow engine with catalog system (#2158) ... * Initial plan * Add workflow engine with step registry, expression engine, catalog system, and CLI commands Agent-Logs-Url: https://github.com/github/spec-kit/sessions/72a7bb5d-071f-4d67-a507-7e1abae2384d Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * Add comprehensive tests for workflow engine (94 tests) Agent-Logs-Url: https://github.com/github/spec-kit/sessions/72a7bb5d-071f-4d67-a507-7e1abae2384d Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * Address review feedback: do-while condition preservation and URL scheme validation Agent-Logs-Url: https://github.com/github/spec-kit/sessions/72a7bb5d-071f-4d67-a507-7e1abae2384d Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> * Address review feedback, add CLI dispatch, interactive gates, and docs Review comments (7/7): - Add explanatory comment to empty except block - Implement workflow catalog download with cleanup on failure - Add input type coercion for number/boolean/enum - Fix example workflow to remove non-existent output references - Fix while_loop and if_then condition defaults (string 'false' → bool False) - Fix resume step index tracking with step_offset parameter CLI dispatch: - Add build_exec_args() and dispatch_command() to IntegrationBase - Override for Claude (skills: /speckit-specify), Gemini (-m flag), Codex (codex exec), Copilot (--agent speckit.specify) - CommandStep invokes installed commands by name via integration CLI - Add PromptStep for arbitrary inline prompts (10th step type) - Stream CLI output live to terminal (no silent blocking) - Remove timeout when streaming (user can Ctrl+C) - Ctrl+C saves state as PAUSED for clean resume Interactive gates: - Gate steps prompt [1] approve [2] reject in TTY - Fall back to PAUSED in non-interactive environments - Resume re-executes the gate for interactive prompting Documentation: - workflows/README.md — user guide - workflows/ARCHITECTURE.md — internals with Mermaid diagrams - workflows/PUBLISHING.md — catalog submission guide Tests: 94 → 122 workflow tests, 1362 total (all passing) * Fix ruff lint errors: unused imports, f-string placeholders, undefined name * Address second review: registry-backed validation, shell failures, loop/fan-out execution, URL validation - VALID_STEP_TYPES now queries STEP_REGISTRY dynamically - Shell step returns FAILED on non-zero exit code - Persist workflow YAML in run directory for reliable resume - Resume loads from run copy, falls back to installed workflow - Engine iterates while/do-while loops up to max_iterations - Engine expands fan-out per item with context.item - HTTPS URL validation for catalog workflow installs (HTTP allowed for localhost) - Fix catalog merge priority docstring (lower number wins) - Fix dispatch_command docstring (no build_exec_args_for_command) - Gate on_reject=retry pauses for re-prompt on resume - Update docs to 10 step types, add prompt step to tables and README * Potential fix for pull request finding 'Empty except' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Address third review: fan-out IDs, catalog guards, shell coercion, docs - Fan-out generates unique per-item step IDs and collects results - Catalog merge skips non-dict workflow entries (malformed data guard) - Shell step coerces run_cmd to str after expression evaluation - urlopen timeout=30 for catalog workflow installs - yaml.dump with sort_keys=False, allow_unicode=True for catalog configs - Document streaming timeout as intentionally unbounded (user Ctrl+C) - Document --allow-all-tools as required for non-interactive + future enhancement - Update test docstring and PUBLISHING.md to 10 step types with prompt * Validate final URL after redirects in catalog fetch urlopen follows redirects, so validate the response URL against the same HTTPS/localhost rules to prevent redirect-based downgrade attacks. * Address fourth review: filter arg eval, tags normalization, install redirect check - Filter arguments now evaluated via _evaluate_simple_expression() so default(42) returns int not string - Tags normalized: non-list/non-string values handled gracefully - Install URL redirect validation (same as catalog fetch) - Remove unused 'skipped' variable in catalog config parsing - Author 'github' → 'GitHub' in example workflow - Document nested step resume limitation (re-runs parent step) * Add explanatory comment to empty except ValueError block * Address fifth review: expression parsing, fan-out output, URL install, gate options - Move string literal parsing before operator detection in expressions so quoted strings with operators (e.g. 'a in b') are not mis-parsed - Fan-out: remove max_concurrency from persisted output, fix docstring to reflect sequential execution - workflow add: support URL sources with HTTPS/redirect validation, validate workflow ID is non-empty before writing files - Deduplicate local install logic via _validate_and_install_local() - Remove 'edit' gate option from speckit workflow (not implemented) * Add comments to empty except ValueError blocks in URL install * Address sixth review: operator precedence, fan_in cleanup, registry resilience, docs - Fix or/and operator precedence (or parsed first = lower precedence) - Restore context.fan_in after fan-in step completes - Catch JSONDecodeError in registry load for corrupted files - Replace print() with on_step_start callback (library-safe) - Gate validation warns when on_reject set but no reject option - Shell step: document shell=True security tradeoff - README: sdd-pipeline → speckit, parallel → sequential for fan-out - ARCHITECTURE.md: parallel → fan-out/fan-in in diagram * Address seventh review: string literal before pipe, type annotations, validate on install - Move string literal check above pipe filter parsing so 'a | b' works - Fix type annotations: input_values list[str] | None, run_id str | None - Run validate_workflow() before installing from local path/URL - Remove duplicate string literal check from expression parser * Address eighth review: fan-out namespaced IDs, early return, catalog validation - Fan-out per-item step IDs use _fanout_{step_id}_{base}_{idx} namespace to avoid collisions with user-defined step IDs - Early return after fan-out loop when state is paused/failed/aborted - Catalog installs parse + validate downloaded YAML before registering, using definition metadata instead of catalog entry for registry * Address ninth review: populate catalog, fix indentation, priority, README - Add speckit workflow entry to catalog.json so it's discoverable - Fix shell step output dict indentation - Catalog add_catalog priority derived from max existing + 1 - README Quick Start clarified with install + local file examples * Address tenth review: max_iterations validation, catalog config guard, version alignment - Validate max_iterations is int >= 1 in while and do-while steps - Guard add_catalog against corrupted config (non-dict/non-list) - Align speckit_version requirement to >=0.6.1 (current package version) - Fan-out template validation uses separate seen_ids set to avoid false duplication errors with user-defined step IDs * Address eleventh review: command step fails without CLI, ID mismatch warning, state persistence - Command step returns FAILED when CLI not installed (was silent COMPLETED) - Catalog install warns on workflow ID vs catalog key mismatch - Engine persists state.save() before returning on unknown step type - Update tests to expect FAILED for command steps without CLI - Integration tests use shell steps for CLI-independent execution * Address twelfth review: type annotations, version examples, streaming docs, requires - Fix workflow_search type annotations (str | None) - PUBLISHING.md: speckit_version >=0.15.0 → >=0.6.1 - Document that exit_code is captured and referenceable by later steps - Mark requires as declared-but-not-enforced (planned enhancement) - Note full stdout/stderr capture as planned enhancement * Enforce catalog key matches workflow ID (fail instead of warn) * Bundle speckit workflow: auto-install during specify init - Add workflows/speckit to pyproject.toml force-include for wheel builds - Add _locate_bundled_workflow() helper (mirrors _locate_bundled_extension) - Auto-install speckit workflow during specify init (after git extension) - Update all integration file inventory tests to expect workflow files * Address fourteenth review: prompt fails without CLI, resolved step data, fan-out normalization - PromptStep returns FAILED when CLI not installed (was silent COMPLETED) - Engine step_data prefers resolved values from step output - Fan-out normalizes output.results=[] for empty item lists - subprocess.run inherits stdout/stderr (no explicit sys.stdout) - Registry tests use issubset for extensibility * Address fifteenth review: fan_in docstring, gate defaults, validation guards, reserved prefix - FanInStep docstring: aggregate-only, no blocking semantics - FanInStep: guard output_config as dict, handle None - Gate validate: use same default options as execute - Validate inputs is dict and steps is list before iterating - Reserve _fanout_ prefix in step ID validation - PUBLISHING.md: remove unenforced checklist items, add _fanout_ note * Address sixteenth review: docs regex, fan_in try/finally, hyphenated dot-path keys - PUBLISHING.md: update ID regex docs to match implementation (single-char OK) - FanInStep: wrap expression evaluation in try/finally for context.fan_in - Expression dot-path: allow hyphens in keys before list index (e.g. run-tests[0]) * Make speckit workflow integration-agnostic, document Copilot CLI requirement - Workflow integration selectable via input (default: claude) - Each command step uses {{ inputs.integration }} instead of hardcoded copilot - Copilot docstring documents CLI requirement for workflow dispatch - Added install_url for Copilot CLI docs * Address seventeenth review: project checks, catalog robustness - Add .specify/ project check to workflow run/resume/status/search/info - remove_catalog validates config shape (dict + list) before indexing - _fetch_single_catalog validates response is a dict - _get_merged_workflows raises when all catalogs fail to fetch - add_catalog guards against non-dict catalog entries in config * Address eighteenth review: condition coercion, gate abort result, while default, cache guard, resume log - evaluate_condition treats plain 'false'/'true' strings as booleans - Gate abort returns StepResult(FAILED) instead of raising exception so step output is persisted in state for inspection - while_loop max_iterations optional (default 10), validation aligned - Catalog cache fallback catches invalid JSON gracefully - resume() appends workflow_finished log entry like execute() * Address nineteenth review: allow-all-tools opt-in, empty catalogs, abort dead code, while docstring - --allow-all-tools controlled by SPECKIT_ALLOW_ALL_TOOLS env var (default: 1) Set to 0 to disable automatic tool approval for Copilot CLI - Empty catalogs list falls back to built-in defaults (not an error) - Remove unreachable WorkflowAbortError catches from execute/resume (gate abort now returns StepResult(FAILED) instead of raising) - while_loop docstring updated: max_iterations is optional (default 10) * Address twentieth review: gate abort maps to ABORTED status, do-while max_iterations optional - Engine detects output.aborted from gate step and sets RunStatus.ABORTED (was unreachable — gate abort returned FAILED but status was always FAILED) - do-while max_iterations now optional (default 10), aligned with while_loop - do-while docstring and validation updated accordingly * Coerce default_options to dict, align bundled workflow ID regex with validator * Gate validates string options, prompt uses resolved integration, loop normalizes max_iterations * Use parentId:childId convention for nested step IDs - Fan-out per-item IDs use parentId:templateId:index (e.g. parallel:impl:0) - Reserve ':' in user step IDs (validation rejects) - Replaces _fanout_ prefix with cleaner namespacing - Expressions like {{ steps.parallel:impl:0.output.file }} work naturally * Validate workflow version is semantic versioning (X.Y.Z) * Schema version validation, strict semver, load_workflow docstring, preserve max_concurrency - Validate schema_version is '1.0' (reject unknown future schemas) - Strict semver regex: ^\d+\.\d+\.\d+$ (rejects 1.0.0beta etc.) - load_workflow docstring: 'parsed' not 'validated' - Keep max_concurrency in fan-out output (was dropped) - do_while docstring: engine re-evaluates step_config condition - ARCHITECTURE.md: document nested resume limitation * Path traversal prevention, loop step ID namespacing - RunState validates run_id is alphanumeric+hyphens (no path separators) - workflow_add validates catalog source doesn't escape workflows_dir - Loop iterations namespace nested step IDs as parentId:childId:iteration so multiple iterations don't overwrite each other in context/state --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mnriem <15701806+mnriem@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>	2026-04-14 10:11:56 -05:00
Manfred Riem	4f9d966beb	Stage 5: Skills, Generic & Option-Driven Integrations (#1924) (#2052) ... * Stage 5: Skills, Generic & Option-Driven Integrations (#1924) Add SkillsIntegration base class and migrate codex, kimi, agy, and generic to the integration system. Integrations: - SkillsIntegration(IntegrationBase) in base.py — creates speckit-<name>/SKILL.md layout matching release ZIP output byte-for-byte - CodexIntegration — .agents/skills/, --skills default=True - KimiIntegration — .kimi/skills/, --skills + --migrate-legacy options, dotted→hyphenated skill directory migration - AgyIntegration — .agent/skills/, skills-only (commands deprecated v1.20.5) - GenericIntegration — user-specified --commands-dir, MarkdownIntegration - All four have update-context.sh/.ps1 scripts - All four registered in INTEGRATION_REGISTRY CLI changes: - --ai <agent> auto-promotes to integration path for all registered agents - Interactive agent selection also auto-promotes (bug fix) - --ai-skills and --ai-commands-dir show deprecation notices on integration path - Next-steps display shows correct skill invocation syntax for skills integrations - agy added to CommandRegistrar.AGENT_CONFIGS Tests: - test_integration_base_skills.py — reusable mixin with setup, frontmatter, directory structure, scripts, CLI auto-promote, and complete file inventory (sh+ps) tests - Per-agent test files: test_integration_{codex,kimi,agy,generic}.py - Kimi legacy migration tests, generic --commands-dir validation - Registry updated with Stage 5 keys - Removed 9 dead-mock tests, moved 4 integration tests to proper locations - Fixed all bare project-name tests to use tmp_path - Fixed 6 pre-existing ANSI escape code test failures in test_extensions.py and test_presets.py 1524 tests pass, 0 failures. * fix: remove unused variable flagged by ruff (F841) * fix: address PR review — integration-type-aware deprecation messages and early generic validation - --ai-skills deprecation message now distinguishes SkillsIntegration ("skills are the default") from command-based integrations ("has no effect") - --ai-commands-dir validation for generic runs even when auto-promoted, giving clear CLI error instead of late ValueError from setup() - Resolves review comments from #2052 * fix: address PR review round 2 - Remove unused SKILL_DESCRIPTIONS dict from base.py (dead code after switching to template descriptions for ZIP parity) - Narrow YAML parse catch from Exception to yaml.YAMLError - Remove unused shutil import from test_integration_kimi.py - Remove unused _REGISTRAR_EXEMPT class attr from test_registry.py - Reword --ai-commands-dir deprecation to be actionable - Update generic validation error to mention both --ai and --integration * fix: address PR review round 3 - Clarify parsed_options forwarding is intentional (all options passed, integrations decide what to use) - Extract _strip_ansi() helper in test_extensions.py and test_presets.py - Remove unused pytest import (test_cli.py), unused locals (test_integration_base_skills.py) - Reword --ai-commands-dir deprecation to be actionable without referencing the not-yet-implemented --integration-options * fix: address PR review round 4 - Reorder kimi migration: run super().setup() first so hyphenated targets exist, then migrate dotted dirs (prevents user content loss) - Move _strip_ansi() to shared tests/conftest.py, import from there in test_extensions.py, test_presets.py, test_ai_skills.py - Remove now-unused re imports from all three test files * fix: address PR review round 5 - Use write_bytes() for LF-only newlines (no CRLF on Windows) - Add --integration-options CLI parameter — raw string passed through to the integration via opts['raw_options']; the integration owns parsing of its own options - GenericIntegration.setup() reads --commands-dir from raw_options when not in parsed_options (supports --integration-options="...") - Skip early --ai-commands-dir validation when --integration-options is provided (integration validates in its own setup()) - Remove parse_integration_options from core — integrations parse their own options * fix: address PR review round 6 - GenericIntegration is now stateless: removed self._commands_dir instance state, overrides setup() directly to compute destination from parsed_options/raw_options on the stack - commands_dest() raises by design (stateless singleton) - _quote() in SkillsIntegration now escapes backslashes and double quotes to produce valid YAML even with special characters * fix: address PR review round 7 - Support --commands-dir=value form in raw_options parsing (not just --commands-dir value with space separator) - Normalize CRLF to LF in write_file_and_record() before encoding - Persist ai_skills=True in init-options.json when using a SkillsIntegration, so extensions/presets emit SKILL.md overrides correctly even without explicit --ai-skills flag	2026-04-02 08:00:12 -05:00