github/github-spec-kit
1
0
Fork 0
mirror of https://github.com/github/spec-kit.git synced 2026-07-03 20:36:23 +08:00
Code Issues Packages Projects Releases Wiki Activity
1,039 Commits 18 Branches 182 Tags
copilot/fix-code-comments
Commit Graph

7 Commits

Author SHA1 Message Date
Copilot
50da3a0f77 Extract agent context updates into bundled agent-context extension (#2546) 
* Initial plan

* Extract agent context updates into bundled agent-context extension

* Potential fix for pull request finding 'Unused import'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

* Potential fix for pull request finding 'Unused import'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

* fix: address review comments on agent-context extension

- bash: parse init-options.json with a single python3 invocation instead
  of three separate read_json_field calls, for parity with the PowerShell
  ConvertFrom-Json approach and to avoid divergent error semantics
- bash: use parameter expansion to strip PROJECT_ROOT prefix from plan
  path instead of sed interpolation, avoiding special-character fragility
- powershell: limit Get-ChildItem to -Depth 1 so plan.md discovery matches
  the bash glob specs/*/plan.md (one level deep) — fixes cross-platform
  inconsistency with nested plan.md files
- powershell: replace Substring+Length relative-path with
  [System.IO.Path]::GetRelativePath for robustness across case/PSDrive
  differences
- __init__.py: move agent-context extension install to after
  save_init_options so init-options.json is present when hooks run
- __init__.py: seed context_markers in init-options only when
  context_file is truthy; avoids noise for integrations without a context
  file
- integrations/base.py: narrow blanket except Exception in
  _resolve_context_markers to ImportError / (OSError, ValueError) so
  unexpected bugs surface instead of being silently swallowed

* fix: gate context_markers in _update_init_options_for_integration on context_file

Apply the same gating logic used during `specify init`: only write
context_markers to init-options.json when the integration actually has a
context_file set.  When switching to an integration without a context file
the stale markers are removed, keeping the two init paths consistent.

* fix: move context_file/context_markers from init-options.json to agent-context extension config

* Potential fix for pull request finding 'Unused global variable'

Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>

* fix: clarify local import comment in agents.py

* Fix remaining agent-context review findings

* Fix follow-up agent-context review issues

* Address review feedback: narrow except, improve PyYAML messaging, surface config-written note

* Fix double-space in PyYAML install hint message

* 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>

* 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 latest agent-context review feedback

* Harden bash config parse output handling

* Clarify ImportError-only fallback comment

* Apply review feedback: drop dead try/except, guard ext-config creation, explicit ConvertFrom-Yaml check

* Remove redundant $Options = $null in PS1 catch block

* Add constitution directives, deprecation warning, agent-context auto-install, and init flow fix

- Add constitution-loading directive to specify, clarify, tasks, checklist, taskstoissues commands
- Add deprecation warning (v0.12.0) in upsert_context_section()
- Auto-install agent-context extension during specify init
- Move context_file from init-options.json to agent-context extension config
- Add tests: deprecation warning, corrupt config, constitution directives
- Update file inventories across all integration tests

* Address review: fix init ordering, test coverage, and hermes inventory

- Move agent-context extension install after init-options.json is saved
  so skill registration can read ai_skills + integration key
- Write extension config after install (avoids template overwriting context_file)
- Fix test_defaults_when_markers_field_missing to truly test missing markers key
- Update hermes tests to allow extension-installed agent-context skill

* Address review: chmod ordering, preserve markers, PS1 Python check, YAML key order

- Move ensure_executable_scripts after agent-context extension install
  so extension scripts get execute bits set
- Use preserve_markers=True on reinit to keep user-customized markers
- Add Python 3 version check in PowerShell fallback (matching bash behavior)
- Add sort_keys=False to yaml.safe_dump for stable config output

* Address review: path traversal guards and docstring fix

- Reject absolute paths and '..' segments in context_file in both bash and
  PowerShell scripts to prevent writes outside the project root
- Fix docstring in _update_init_options_for_integration to accurately
  describe marker preservation behavior

* Address review: strict enabled check, docstring, segment-level path traversal

- Use 'is not False' for enabled check so only literal False disables
- Update upsert_context_section docstring to mention disabled-extension return
- Fix path traversal guards to check actual path segments, not substrings
  (allows filenames like 'notes..md' while rejecting '../' traversal)

* Address review: UnicodeError handling, missing extension warning

- Add UnicodeError to exception tuples in _load_agent_context_config and
  _resolve_context_markers so garbled UTF-8 config files fall back to defaults
- Emit error (with reinstall command) instead of silent skip when bundled
  agent-context extension is not found during init

* Address review: bash backslash traversal guard, wheel packaging

- Reject backslash separators and Windows drive-letter paths in bash
  context_file validation (prevents traversal on Git-Bash/Windows)
- Add extensions/agent-context to pyproject.toml force-include so the
  bundled extension is included in wheel builds

* Address review: write extension config before init-options.json

- Reorder writes in _update_init_options_for_integration so the
  agent-context extension config is updated first; if it fails,
  init-options.json remains consistent with the previous state

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
 2026-05-30 06:37:18 -05:00
Nimra Akram
259494a328 fix: honor template overrides for tasks-template (#2278) (#2292) 
* fix: honor template overrides for tasks-template (#2278)

- Add scripts/bash/setup-tasks.sh mirroring setup-plan.sh pattern
- Add scripts/powershell/setup-tasks.ps1 mirroring setup-plan.ps1 pattern
- Update tasks.md frontmatter to use dedicated setup-tasks scripts
- Resolve tasks template via override stack and emit path as TASKS_TEMPLATE in JSON output
- Reference resolved TASKS_TEMPLATE path in generate step instead of hardcoded path

* fix: remove stray EOF tokens from setup-tasks scripts

* fix: improve error messages for unresolved tasks-template

* test: update file inventory tests to include setup-tasks scripts

* fix: use Console::Error.WriteLine instead of Write-Error in setup-tasks.ps1

* fix: write prerequisite error messages to stderr in setup-tasks.ps1

* fix: validate tasks template is a file and normalize path in setup-tasks.ps1

* fix: improve tasks-template error message to mention full override stack

* test: add setup-tasks.sh to TestCopilotSkillsMode file inventory

* fix: skip feature-branch validation when feature.json pins FEATURE_DIR

* fix: correct override path in tasks-template error messages

* test: add integration tests for setup-tasks template resolution and branch validation

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix: correct fixture paths and add spec.md prerequisite checks

* fix: use correct .registry schema in preset priority test

* fix: remove stale aaa-preset block and duplicate comment in preset priority test

* fix: align preset directory names with registry IDs in priority test

---------

Co-authored-by: Nimraakram22 <nimra.akram123451@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-05-01 16:18:19 -05:00
Manfred Riem
52c0a5f88f fix: resolve command references per integration type (dot vs hyphen) (#2354) 
* fix: resolve command references per integration type (dot vs hyphen)

Replace hardcoded /speckit.<cmd> references in templates with
__SPECKIT_COMMAND_<NAME>__ placeholders that are resolved at
setup time based on the integration type:

- Markdown/TOML/YAML agents: separator='.' → /speckit.plan
- Skills agents: separator='-' → /speckit-plan

Changes:
- Add resolve_command_refs() static method to IntegrationBase
- Add invoke_separator class attribute (.  for base, - for skills)
- Wire into process_template() as step 8
- Update _install_shared_infra() to process page templates
- Replace /speckit.* in 5 command templates and 3 page templates
- Add unit tests for resolve_command_refs (positive + negative)
- Add integration tests verifying on-disk content for all agents
- Add end-to-end CLI tests for Claude (skills) and Copilot (markdown)

Fixes #2347

* review: use effective_invoke_separator() for Copilot skills mode

Address PR review feedback: instead of bleeding _skills_mode
knowledge into the CLI layer, add effective_invoke_separator()
method to IntegrationBase that accepts parsed_options.

CopilotIntegration overrides it to return "-" when skills
mode is requested. The CLI layer simply asks the integration
for its separator — no hasattr or _skills_mode coupling.

Also adds tests for the new method on both base and Copilot,
plus an end-to-end test for 'specify init --integration copilot
--integration-options --skills' verifying page templates get
hyphen refs.

* fix: build_command_invocation preserves full suffix for extension commands

Previously rsplit('.', 1)[-1] on 'speckit.git.commit' yielded
just 'commit', producing /speckit.commit instead of
/speckit.git.commit (or /speckit-git-commit for skills).

Fix: strip only the 'speckit.' prefix when present, then join
remaining segments with the appropriate separator.

Updated in IntegrationBase, SkillsIntegration, and
CopilotIntegration. Added tests for extension commands in
build_command_invocation across all three.

* fix: Copilot dispatch_command() preserves full extension command suffix

dispatch_command() had the same rsplit('.', 1)[-1] bug as
build_command_invocation() — speckit.git.commit would dispatch
as /speckit-commit instead of /speckit-git-commit in skills
mode, or --agent speckit.commit instead of speckit.git.commit
in default mode.
 2026-04-24 10:04:14 -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
b1832c9477 Stage 6: Complete migration — remove legacy scaffold path (#1924) (#2063) 
* Stage 6: Complete migration — remove legacy scaffold path (#1924)

Remove the legacy GitHub download and offline scaffold code paths.
All 26 agents now use the integration system exclusively.

Code removal (~1073 lines from __init__.py):
- download_template_from_github(), download_and_extract_template()
- scaffold_from_core_pack(), _locate_release_script()
- install_ai_skills(), _get_skills_dir (restored slim version for presets)
- _has_bundled_skills(), _migrate_legacy_kimi_dotted_skills()
- AGENT_SKILLS_MIGRATIONS, _handle_agent_skills_migration()
- _parse_rate_limit_headers(), _format_rate_limit_error()
- Three-way branch in init() collapsed to integration-only

Config derivation (single source of truth):
- AGENT_CONFIG derived from INTEGRATION_REGISTRY (replaced 180-line dict)
- CommandRegistrar.AGENT_CONFIGS derived from INTEGRATION_REGISTRY (replaced 160-line dict)
- Backward-compat constants kept for presets/extensions: SKILL_DESCRIPTIONS,
  NATIVE_SKILLS_AGENTS, DEFAULT_SKILLS_DIR

Release pipeline cleanup:
- Deleted create-release-packages.sh/.ps1 (948 lines of ZIP packaging)
- Deleted create-github-release.sh, generate-release-notes.sh
- Deleted simulate-release.sh, get-next-version.sh, update-version.sh
- Removed .github/workflows/scripts/ directory entirely
- release.yml is now self-contained: check, notes, release all inlined
- Install instructions use uv tool install with version tag

Test cleanup:
- Deleted test_ai_skills.py (tested removed functions)
- Deleted test_core_pack_scaffold.py (tested removed scaffold)
- Cleaned test_agent_config_consistency.py (removed 19 release-script tests)
- Fixed test_branch_numbering.py (removed dead monkeypatches)
- Updated auto-promote tests (verify files created, not tip messages)

1089 tests pass, 0 failures, ruff clean.

* fix: resolve merge conflicts with #2051 (claude as skills)

- Fix circular import: move CommandRegistrar import in claude
  integration to inside method bodies (was at module level)
- Lazy-populate AGENT_CONFIGS via _ensure_configs() to avoid
  circular import at class definition time
- Set claude registrar_config to .claude/commands (extension/preset
  target) since the integration handles .claude/skills in setup()
- Update tests from #2051 to match: registrar_config assertions,
  remove --integration tip assertions, remove install_ai_skills mocks

1086 tests pass.

* fix: properly preserve claude skills migration from #2051

Restore ClaudeIntegration.registrar_config to .claude/skills (not
.claude/commands) so extension/preset registrations write to the
correct skills directory.

Update tests that simulate claude setup to use .claude/skills and
check for SKILL.md layout. Some tests still need updating for the
full skills path — 10 remaining failures from the #2051 test
expectations around the extension/preset skill registration flow.

WIP: 1076/1086 pass.

* fix: properly handle SKILL.md paths in extension update rollback and tests

Fix extension update rollback using _compute_output_name() for SKILL.md
agents (converts dots to hyphens in skill directory names). Previously
the backup and cleanup code constructed paths with raw command names
(e.g. speckit.test-ext.hello/SKILL.md) instead of the correct computed
names (speckit-test-ext-hello/SKILL.md).

Test fixes for claude skills migration:
- Update claude tests to use .claude/skills paths and SKILL.md layout
- Use qwen (not claude) for skills-guard tests since claude's agent dir
  IS the skills dir — creating it triggers command registration
- Fix test_extension_command_registered_when_extension_present to check
  skills path format

1086 tests pass, 0 failures, ruff clean.

* fix: address PR review — lazy init, assertions, deprecated flags

- _ensure_configs(): catch ImportError (not Exception), don't set
  _configs_loaded on failure so retries work
- Move _ensure_configs() before unregister loop (not inside it)
- Module-level try/except catches ImportError specifically
- Remove tautology assertion (or True) in test_extensions.py
- Strengthen preset provenance assertion to check source: field
- Mark --offline, --skip-tls, --debug, --github-token as hidden
  deprecated no-ops in init()

1086 tests pass.

* fix: remove deleted release scripts from pyproject.toml force-include

Removes force-include entries for create-release-packages.sh/.ps1
which were deleted but still referenced in [tool.hatch.build].
 2026-04-02 12:34:34 -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