Commit Graph

3 Commits

Author SHA1 Message Date
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
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
Manfred Riem
3899dcc0d4 Stage 2: Copilot integration — proof of concept with shared template primitives (#2035)
* feat: Stage 2a — CopilotIntegration with shared template primitives

- base.py: added granular primitives (shared_commands_dir,
  shared_templates_dir, list_command_templates, command_filename,
  commands_dest, copy_command_to_directory, record_file_in_manifest,
  write_file_and_record, process_template)
- CopilotIntegration: uses primitives to produce .agent.md commands,
  companion .prompt.md files, and .vscode/settings.json
- Verified byte-for-byte parity with old release script output
- Copilot auto-registered in INTEGRATION_REGISTRY
- 70 tests (22 new: base primitives + copilot integration)

Part of #1924

* feat: Stage 2b — --integration flag, routing, agent.json, shared infra

- Added --integration flag to init() (mutually exclusive with --ai)
- --ai copilot auto-promotes to integration path with migration nudge
- Integration setup writes .specify/agent.json with integration key
- _install_shared_infra() copies scripts and templates to .specify/
- init-options.json records 'integration' key when used
- 4 new CLI tests: mutual exclusivity, unknown rejection, copilot
  end-to-end, auto-promote (74 total integration tests)

Part of #1924

* feat: Stage 2 completion — integration scripts, integration.json, shared manifest

- Added copilot/scripts/update-context.sh and .ps1 (thin wrappers
  that delegate to the shared update-agent-context script)
- CopilotIntegration.setup() installs integration scripts to
  .specify/integrations/copilot/scripts/
- Renamed agent.json → integration.json with script paths
- _install_shared_infra() now tracks files in
  integration-shared.manifest.json
- Updated tests: scripts installed, integration.json has script paths,
  shared manifest recorded (74 tests)

Part of #1924

* refactor: rename shared manifest to speckit.manifest.json

Cleaner naming — the shared infrastructure (scripts, templates)
belongs to spec-kit itself, not to any specific integration.

* fix: copilot update-context scripts reflect target architecture

Scripts now source shared functions (via SPECKIT_SOURCE_ONLY=1) and
call update_agent_file directly with .github/copilot-instructions.md,
rather than delegating back to the shared case statement.

* fix: simplify copilot scripts — dispatcher sources common functions

Integration scripts now contain only copilot-specific logic (target
path + agent name). The dispatcher is responsible for sourcing shared
functions before calling the integration script.

* fix: copilot update-context scripts are self-contained implementations

These scripts ARE the implementation — the dispatcher calls them.
They source common.sh + update-agent-context functions, gather
feature/plan data, then call update_agent_file with the copilot
target path (.github/copilot-instructions.md).

* docs: add Stage 7 activation note to copilot update-context scripts

* test: add complete file inventory test for copilot integration

Validates every single file (37 total) produced by
specify init --integration copilot --script sh --no-git.

* test: add PowerShell file inventory test for copilot integration

Validates all 37 files produced by --script ps variant, including
.specify/scripts/powershell/ instead of bash.

* refactor: split test_integrations.py into tests/integrations/ directory

- test_base.py: IntegrationOption, IntegrationBase, MarkdownIntegration, primitives
- test_manifest.py: IntegrationManifest, path traversal, persistence, validation
- test_registry.py: INTEGRATION_REGISTRY
- test_copilot.py: CopilotIntegration unit tests
- test_cli.py: --integration flag, auto-promote, file inventories (sh + ps)
- conftest.py: shared StubIntegration helper

76 integration tests + 48 consistency tests = 124 total, all passing.

* refactor: move file inventory tests from test_cli to test_copilot

File inventories are copilot-specific. test_cli.py now only tests
CLI flag mechanics (mutual exclusivity, unknown rejection, auto-promote).

* fix: skip JSONC merge to preserve user settings, fix docstring

- _merge_vscode_settings() now returns early (skips merge) when
  existing settings.json can't be parsed (e.g. JSONC with comments),
  instead of overwriting with empty settings
- Updated _install_shared_infra() docstring to match implementation
  (scripts + templates, speckit.manifest.json)

* fix: warn user when JSONC settings merge is skipped

* fix: show template content when JSONC merge is skipped

User now sees the exact settings they should add manually.

* fix: document process_template requirement, merge scripts without rmtree

- base.py setup() docstring now explicitly states raw copy behavior
  and directs to CopilotIntegration for process_template example
- _install_shared_infra() uses merge/overwrite instead of rmtree to
  preserve user-added files under .specify/scripts/

* fix: don't overwrite pre-existing shared scripts or templates

Only write files that don't already exist — preserves any user
modifications to shared scripts (common.sh etc.) and templates.

* fix: warn user about skipped pre-existing shared files

Lists all shared scripts and templates that were not copied because
they already existed in the project.

* test: add test for shared infra skip behavior on pre-existing files

Verifies that _install_shared_infra() preserves user-modified scripts
and templates while still installing missing ones.

* fix: address review — containment check, deterministic prompts, manifest accuracy

- CopilotIntegration.setup() adds dest containment check (relative_to)
- Companion prompts generated from templates list, not directory glob
- _install_shared_infra() only records files actually copied (not pre-existing)
- VS Code settings tests made unconditional (assert template exists)
- Inventory tests use .as_posix() for cross-platform paths

* fix: correct PS1 function names, document SPECKIT_SOURCE_ONLY prerequisite

- Fixed Get-FeaturePaths → Get-FeaturePathsEnv, Read-PlanData → Parse-PlanData
- Documented that shared scripts must guard Main with SPECKIT_SOURCE_ONLY
  before these integration scripts can be activated (Stage 7)

* fix: add dict type check for settings merge, simplify PS1 to subprocess

- _merge_vscode_settings() skips merge with warning if parsed JSON
  is not a dict (array, null, etc.)
- PS1 update-context.ps1 uses & invocation instead of dot-sourcing
  since the shared script runs Main unconditionally

* fix: skip-write on no-op merge, bash subprocess, dynamic integration list

- _merge_vscode_settings() only writes when keys were actually added
- update-context.sh uses exec subprocess like PS1 version
- Unknown integration error lists available integrations dynamically

* fix: align path rewriting with release script, add .specify/.specify/ fix

Path rewrite regex matches the release script's rewrite_paths()
exactly (verified byte-identical output). Added .specify/.specify/
double-prefix fix for additional safety.
2026-03-31 17:40:32 -05:00