mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
5b682b2cb3a75dc693e9bdb1e33a169365b340f6
261 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Noor ul ain
|
5b682b2cb3 |
fix: interpolate multi-expression templates instead of returning None (#3208) (#3228)
* fix: interpolate multi-expression templates instead of returning None (#3208) `evaluate_expression` returned None for templates containing two or more `{{ }}` blocks with no surrounding literal text, e.g. `"{{ context.run_id }} {{ inputs.issue }}"`. The single-expression fast path used `_EXPR_PATTERN.fullmatch()`, but `fullmatch` defeats the pattern's non-greedy `(.+?)` body: for two adjacent expressions it still matches, capturing everything between the first `{{` and the last `}}` (`"context.run_id }} {{ inputs.issue"`) as the body. That garbage failed dot-path resolution and returned None directly, bypassing the `sub()` interpolation path that would have resolved each expression. Downstream this surfaced as the literal string "None" reaching commands. Guard the fast path on `stripped.count("{{") == 1` so only genuine single-expression templates take the typed return; multi-expression templates fall through to `sub()` and interpolate correctly. Add regression tests for two expressions separated by a space and for adjacent expressions with no separator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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(expressions): use match-span guard so single expressions with literal {{ keep their type The previous `stripped.count("{{") == 1` guard misclassified a genuine single expression whose string argument contains a literal `{{` (e.g. `{{ inputs.text | contains('{{') }}`) as multi-expression, routing it through `sub()` interpolation and coercing the typed (bool/int/list) return value to a string -- breaking the type-preservation the docstring promises (Copilot review on #3228). Anchor a single match at the start and require it to consume the whole stripped string instead. The non-greedy body stops at the first `}}`, so a two-block template fails the span check (falls through to interpolation, fixing #3208) while a lone expression -- including one with a `{{` inside a string literal -- matches to the end and keeps its typed value. Add a regression test for the literal-brace single-expression case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix(expressions): detect single expression with quote-aware scan The match-span guard using the non-greedy _EXPR_PATTERN stopped at the first `}}`, so a lone expression whose string argument contains a literal `}}` (e.g. `{{ inputs.text | contains('}}') }}`) was misclassified as multi-expression and mis-parsed by the interpolation path, raising ValueError and turning CI red (Copilot review on #3228). Replace the span check with `_is_single_expression`, which scans the `{{ ... }}` body for a block-closing `}}` outside string literals (mirrors the quote handling already in `_split_top_level_commas`). A genuine two-block template closes early and falls through to interpolation (fixing #3208); a lone expression with a literal `{{` or `}}` inside a string argument keeps its typed return value. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Pascal THUET
|
490566847c |
feat(cli): honor SPECIFY_INIT_DIR in the specify CLI project resolver (#3186)
* feat(cli): honor SPECIFY_INIT_DIR in the specify CLI project resolver The shell resolver honors SPECIFY_INIT_DIR (#2892), but the Python CLI did not: it resolved the project as Path.cwd() + a .specify/ check and never read the override. So setup-plan.sh respected it while `specify integration install` ignored it, and you still had to cd into the member project. Route project resolution through a shared _resolve_init_dir_override() that applies the shell resolver's validation rules (relative to cwd, must exist and contain .specify/, hard error, no fallback, same error strings). It's wired into _require_specify_project() — the chokepoint for every project-scoped subcommand (integration/extension/workflow/preset/...) — and the `workflow run <file>` standalone path, which re-applies its symlinked-.specify guard on the override branch too. init is unchanged: it creates .specify/, so the must-pre-exist rule doesn't apply. The resolver canonicalizes symlinks via Path.resolve() while the shell keeps the logical path; they agree for non-symlinked paths (documented in the resolver). Tests in tests/test_init_dir_cli.py mirror the strict cases from test_init_dir.py through the CLI; conftest now strips SPECIFY_* for the whole suite so a stray export can't perturb the now-env-reading resolver. Docs note the CLI applies the same rules. Discussion: github/spec-kit#2834 (Disclosure: I used an AI coding agent to audit the call sites and resolver, draft the change, and run an adversarial code review; reviewed by me.) * fix(cli): honor SPECIFY_INIT_DIR for bundle commands Assisted-by: Codex (model: GPT-5, autonomous) * fix(bundler): refuse symlinked .specify on the SPECIFY_INIT_DIR override path find_project_root refuses a symlinked .specify (following it could read/write outside the tree, and a test pins that), but the SPECIFY_INIT_DIR override added for bundle commands returned early and skipped that guard: _resolve_init_dir_override validates .specify with is_dir(), which follows symlinks. So `specify bundle` accepted via the override a layout the cwd path rejects. Re-check the override result with the same guard, plus a regression test. (Disclosure: found via an AI code review and fixed with an AI coding agent; reviewed by me.) * fix(cli): keep SPECIFY_INIT_DIR strict for bundles Treat an explicit symlinked SPECIFY_INIT_DIR project as a hard bundle error instead of returning no project, which could initialize the current directory. Align the docs with the actual unset resolver behavior. Assisted-by: Codex (model: GPT-5, autonomous) * docs(core): note symlinked .specify handling differs across CLI surfaces A symlinked .specify is followed by integration/extension/workflow (matching the shell resolver) but refused by bundle and workflow run <file> (write confinement). Document the asymmetry so it reads as intentional. (Disclosure: AI-assisted; reviewed by me.) * docs(core): reframe symlinked .specify note around the override invariant Per maintainer feedback on #3186: SPECIFY_INIT_DIR relocates where the project is, not how a surface treats symlinks. Each surface keeps its cwd-path stance (write surfaces refuse a symlinked .specify, read/config surfaces follow it), so the split is one policy relocated, not an inconsistency. * docs: address Copilot review on resolver docstrings - _project.py: the error messages "mirror" the shell wording rather than "match" it (the CLI renders a Rich `Error:` line, the shell a plain `ERROR:`). - find_project_root: document that honoring SPECIFY_INIT_DIR when start is None can raise typer.Exit / BundlerError, so the Path | None signature isn't surprising to direct callers. * docs(bundler): note require_project_root inherits the override raise behavior find_project_root can raise typer.Exit / BundlerError under the SPECIFY_INIT_DIR override (start=None); require_project_root inherits that, so document it alongside its own BundlerError-on-missing-project. * docs: clarify symlinked project root behavior Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * Address SPECIFY_INIT_DIR review feedback Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * Route workflow JSON errors to stderr Assisted-by: OpenAI Codex (model: GPT-5, autonomous) |
||
|
Noor ul ain
|
f59fd81608 |
fix(extensions): resolve core-command dirs via _assets helpers (#3274) (#3287)
`_load_core_command_names()` computed its candidate command dirs with bespoke `Path(__file__)` arithmetic. The #3014 move of this module from `specify_cli/extensions.py` to `specify_cli/extensions/__init__.py` pushed the file one directory deeper but left the `.parent` counts unchanged, so both candidates resolved to non-existent paths: wheel -> specify_cli/extensions/core_pack/commands (real: specify_cli/core_pack/commands) source -> src/templates/commands (real: repo-root templates/commands) Neither exists, so every call silently fell through to `_FALLBACK_CORE_COMMAND_NAMES`. Discovery is latent-dead: the fallback happens to equal the real stems today, but the shadowing guard (#1994) that depends on it now relies on someone hand-editing the fallback on every core-command add/remove (as already happened for `converge`, #3001). Delegate path resolution to the canonical `_locate_core_pack` / `_repo_root` resolvers in `_assets` — the same ones the presets and bundle loaders use. They are anchored to the package root, so discovery survives future module moves. Add regression tests that point the resolvers at a temp tree with *different* command names, proving discovery reads from disk rather than returning the fallback (they fail on the pre-fix code). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Noor ul ain
|
1849543611 |
fix: fall back to feature dir basename for empty CURRENT_BRANCH (#3026) (#3229)
* fix: fall back to feature dir basename for empty CURRENT_BRANCH (#3026) When a feature is resolved via SPECIFY_FEATURE_DIRECTORY or .specify/feature.json without SPECIFY_FEATURE set, get_current_branch() returns empty, so get_feature_paths / Get-FeaturePathsEnv emitted CURRENT_BRANCH= (empty) even though the feature directory was resolvable. Downstream scripts and agents that expect a non-empty identifier got misleading output. Fall back to the basename of the resolved feature directory when the branch is empty, in both the bash (`${feature_dir##*/}`) and PowerShell (`Split-Path -Leaf`) resolvers. An explicit SPECIFY_FEATURE still takes precedence, so this only fills the previously-empty case. Add bash + PowerShell regression tests: the basename fallback fires when SPECIFY_FEATURE is unset, and an explicit SPECIFY_FEATURE still overrides it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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> * 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: address Copilot feedback — PS 5.1 compat + parametrize bash test - common.ps1: replace [System.IO.Path]::TrimEndingDirectorySeparator (a .NET Core-only method that throws MethodNotFound on Windows PowerShell 5.1 / .NET Framework) with a portable String.TrimEnd, so the trailing-slash trim actually works on 5.1. - tests: parametrize the bash fallback test to cover feature.json, SPECIFY_FEATURE_DIRECTORY, and the explicit SPECIFY_FEATURE override (mirrors the PowerShell test), folding in the old explicit-override test; add the missing blank line before the next test (PEP 8). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
WOLIKIMCHENG
|
d982c2f67f |
feat(copilot): warn before skills default rollout (#3256)
* feat(copilot): default to skills mode * feat(copilot): warn before skills default rollout * Make Copilot skills warning test less brittle --------- Co-authored-by: root <kinsonnee@gmail.com> |
||
|
Ali jawwad
|
b4a0f8b564 |
fix(integrations): add zed to discovery catalog.json (#3266)
zed is registered, registrar-aligned and registry-tested, but it was the only one of the 34 integrations absent from integrations/catalog.json, making it undiscoverable through the discovery manifest. Add the missing 'zed' entry (mirroring the sibling skills entries) and a registry<->catalog parity regression test so a future integration can't silently drift. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Ali jawwad
|
2d56dfd73d |
fix(integrations): cline hook note collapses onto instruction at EOF (#3263)
The hook-note injection regex matches the line terminator via (\r\n|\n|$), so the captured eol group is empty when the instruction is the final line of a file with no trailing newline. The cline integration emitted the note with that empty eol, mashing the note text and the instruction onto a single line. Default eol to '\n', matching the agy integration twin which already guards this case. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
darion-yaphet
|
810d6fcfe1 |
refactor: move workflow command handlers to workflows/_commands.py (PR-8/8) (#3159)
* refactor: move workflow command handlers to workflows/_commands.py (PR-8/8) Final PR of the __init__.py split. Moves the workflow command group out of __init__.py into the existing workflows/ package, completing the domain-dir layout established in PR-5 (integrations), PR-6 (presets) and PR-7 (extensions). - New workflows/_commands.py holds the four Typer apps (workflow / catalog / step / step-catalog), all 25 command handlers, the six workflow-only helpers (_parse_input_values, _workflow_run_payload, _emit_workflow_json, _stdout_to_stderr_when, _validate_step_id_or_exit, _resolve_steps_base_dir_or_exit), and a register(app) entry point. - workflows is already a package, so no rename is needed; intra-package imports change from `.workflows.x` to `.x`. The only root-helper dep (_require_specify_project) is reached through a call-time shim so test monkeypatching of specify_cli._require_specify_project keeps working. - __init__.py drops ~1445 lines (2066 -> 621); the workflow group is re-attached via register(app). Dead `contextlib` import removed. - tests/test_workflows.py: import the now-relocated _stdout_to_stderr_when helper from its new home (workflows._commands) instead of the package root. No behavior change. Full suite green (3847 passed), ruff clean. * Prevent workflow state writes through symlinked storage Workflow commands persist run state under .specify/workflows/runs, so the command-local project shim now rejects symlinked workflow storage before any workflow command proceeds. The standalone YAML path uses the same guard because it intentionally bypasses the normal project requirement while still creating workflow state under the current directory. Constraint: Local YAML workflow runs do not require an existing .specify project directory but still create .specify/workflows/runs state Rejected: Guard only .specify in the file-source path | .specify/workflows and runs can independently redirect writes Confidence: high Scope-risk: narrow Directive: Keep workflow storage symlink checks centralized before constructing WorkflowEngine Tested: .venv/bin/python -m pytest tests/test_workflow_run_without_project.py tests/test_workflows.py::TestWorkflowAddSymlinkGuard -v Tested: .venv/bin/python -m py_compile src/specify_cli/workflows/_commands.py tests/test_workflow_run_without_project.py tests/test_workflows.py Not-tested: Ruff lint; ruff is not installed in the repo virtualenv Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * fix(workflows): pass github_hosts allowlist to GHES release asset resolver workflow add resolved GitHub release download URLs without forwarding the github_provider_hosts() allowlist, so resolve_github_release_asset_api_url never treated any host as GHES. This regressed GitHub Enterprise Server release asset resolution and diverged from presets/extensions, which already pass github_hosts. Forward github_provider_hosts() at both the direct-URL and catalog install call sites. The allowlist remains the anti-SSRF gate. * fix(workflows): reject symlinked/traversal <id> dir on workflow install Local/URL and catalog installs wrote to .specify/workflows/<id>/workflow.yml without guarding the <id> segment. A pre-planted symlink at <id> or <id>/workflow.yml let mkdir+copy/download follow it and write outside the project root; a non-directory <id> made mkdir raise unhandled. Add _safe_workflow_id_dir() to reject path traversal, symlinked or non-directory <id>, and a symlinked workflow.yml leaf before any write. Fold the catalog branch's existing traversal check into the helper. * fix(workflows): harden _safe_workflow_id_dir output and leaf checks - Reorder symlink/non-directory check before resolve() so a symlinked <id> reports as symlinked instead of misleading "Invalid workflow ID" - Reject a pre-existing <id>/workflow.yml that is not a file, avoiding an unhandled IsADirectoryError on later write/copy2 - Escape workflow_id in Rich output to prevent markup injection; escape the repr (not the raw id) so repr-added backslashes cannot re-expose brackets, matching extensions/_commands.py hardening - Add tests for workflow.yml-as-directory and markup-escaped invalid id * Avoid stale lint failures from config helper imports Move PyYAML loading into the helpers that read and write agent-context configuration, and replace the broad Any annotation with object. The runtime behavior stays the same while the module no longer exposes top-level imports that can be flagged as unused when CI analyzes a narrower code shape. * Prevent workflow commands from targeting reserved storage Workflow install and removal paths are derived from workflow IDs before any catalog download, local copy, or directory deletion. Validate that IDs are single workflow-id path segments and reject names reserved for workflow runtime storage so commands cannot target .specify/workflows/runs or .specify/workflows/steps. |
||
|
Ben Buttigieg
|
36501d459f |
chore: retire Roo Code integration — extension shut down (#3167) (#3212)
* chore: retire roo integration — extension shut down (#3167) Remove the Roo Code integration after the extension was shut down: subpackage, registry entry, catalog entry, docs, tests, and issue-template options. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: remove stale Roo Code mention in upgrade guide Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: remove leftover Roo Code references after merge Drop roo from presets/ARCHITECTURE.md example and the agent-context defaults map; these came in from main and were flagged by review. Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Ali jawwad
|
c5ac90b245 |
fix(bundle): allow 'catalog remove' by the same relative path used to add (#3242)
* fix(bundle): allow 'catalog remove' by the same relative path used to add
add_source canonicalizes a local catalog path to an absolute url before persisting it, but remove_source compared only the raw input against the stored id/url. So 'bundle catalog remove ./cat.json' could not undo 'bundle catalog add ./cat.json' -- the stored url was absolute, the removal target relative, and they never matched ('No project-scoped catalog source found'). Match the canonicalized form too (a no-op for ids and remote urls), so a local source is removable by the same path it was added with.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(bundle): match catalog removal target exactly first, canonical only as fallback
Address Copilot review: canonicalizing the removal target unconditionally could let 'remove <id>' also delete a different source whose url equals that id's canonicalized path (ids are treated as local paths by _canonicalize_url, empty scheme). Try an exact id/url match first; only fall back to a canonicalized-url match when no exact match is found, so relative-path removal still works without collateral deletion.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwad
|
3571ba72d8 |
fix(workflows): reject bool max_iterations in while/do-while validation (#3237)
* fix(workflows): reject bool max_iterations in while/do-while validation while/do-while validate() checked 'not isinstance(max_iter, int) or max_iter < 1'. Since bool is a subclass of int, isinstance(True, int) is True and True < 1 is False, so 'max_iterations: true' passed validation and then ran as a single iteration (range(True) == range(1)) instead of being reported as a type error. Reject bools explicitly, matching the fail-fast-on-bool handling already used for number inputs and gate options. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: assert empty error list for the valid do-while max_iterations case Address Copilot review: the accepted-config assertion only checked that no error mentioned 'max_iterations', which could let an unrelated validation error pass unnoticed. For a known-good config, assert the entire error list is empty (consistent with the other validate tests in this file). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Dyan Galih
|
6fb7e77b3e |
fix: allow prerelease spec-kit versions in compatibility checks (#2695)
* docs: generate integrations reference from catalog * refactor: integrate table rendering into specify integration search --markdown - Remove standalone scripts/generate_integrations_reference.py - Strip doc injection machinery from catalog_docs.py; keep only table rendering - Wire render_integrations_table() into existing --markdown flag of integration search - Remove old simple markdown table block from integration_search (was Name|ID|Version|Description|Author) - Simplify tests: drop subprocess/doc-path tests, keep table rendering and metadata tests - Clean up docs/reference/integrations.md: remove generated markers, update note * fix: address Copilot review feedback on catalog_docs and integration_search - Warn when --markdown is combined with filters (query/--tag/--author) which are silently ignored; catch ValueError/FileNotFoundError and surface clean error via console instead of raw traceback (r3244821516) - Add coverage enforcement in list_integrations_for_docs(): raises ValueError with actionable message if any registry key is missing from INTEGRATION_DOC_URLS, preventing silently incomplete doc tables (r3244821589) - Rename test to accurately reflect sources: label derives from registry config, URL comes from INTEGRATION_DOC_URLS doc map — not solely from registry (r3244821607) - Simplify test dict construction to idiomatic dict comprehension (r3244821619) * fix: add sync test, INTEGRATIONS_REFERENCE_PATH constant, and fix naming * revert: restore docs/reference/integrations.md to upstream/main; remove sync test (GH Actions job will handle) * fix: remove dead INTEGRATIONS_REFERENCE_PATH, drop URL-length padding, fix docstring, drop FileNotFoundError * fix: send --markdown warnings/errors to stderr, rename test for clarity * fix: detect stale doc-map keys, test _render_cell escaping, strengthen header assertion * refactor: promote _render_cell to public render_cell function * test: mock registry and doc maps to avoid brittle live registry coupling * refactor: flatten patches, remove unused imports, fix trailing whitespace, optimize missing calculation * refactor: make validation non-fatal, fix context manager syntax, add CLI tests * fix: improve docstring clarity, test robustness, and exception handling * fix: improve test assertions, disable warnings by default, enhance exception handling * fix: make CLI tests deterministic and improve config access resilience * fix: remove extra blank line, add stale keys validation, add regression test for docs sync * Fix 5 remaining feedback items: - Rename _get_mocked_cli_runner() to _get_catalog_docs_patches() for clarity - Use ExitStack context manager for guaranteed patch cleanup - Add explicit UTF-8 encoding to file reads - Skip doc sync test gracefully when docs aren't present - Remove exception chaining from typer.Exit to avoid noisy tracebacks * address all outstanding copilot review feedback on PR 2563 * Address Copilot feedback: escape URLs in markdown links, deduplicate cell rendering, fix table parser for escaped pipes * Address 3 new Copilot feedback: add URL escaping test, fix parse_first_markdown_table for escaped pipes, guard community tests with skip * Address 3 new Copilot feedback: escape id field, remove unused alias, escape integration URLs * Address 3 new Copilot feedback: fix comment name, include all integrations in list * Fix architectural issue: escape raw fields before composing Markdown to prevent double-escaping * Deduplicate _escape_url_for_markdown_link and add URL escaping test * Address 4 new Copilot feedback: add trailing newline, fix test helper ExitStack, update warning message * Address 4 new Copilot feedback: make escape function public, fix error message, validate test rows, prevent double newline * Update error message in test_missing_catalog_file for clarity * Remove obsolete integrations sync test * keep integrations docs in sync * fix: allow prerelease spec-kit versions in compatibility checks Allow prerelease/dev builds to satisfy extension and preset compatibility checks when their version number falls within the required specifier range. Also harden the integrations docs rendering helpers and add regression coverage for the markdown table parsing and version gating paths. Tests: pytest -q; python3 -m compileall -q .; black/flake8 unavailable Reference: branch 002-generate-integrations-docs; source patch /tmp/spec-kit-changes.patch * fix: isolate prerelease compatibility gate changes Keep the prerelease/version compatibility fix on its own branch and remove the unrelated integrations docs updates that belong with PR 2563. Tests: full suite passed on the prerelease branch before splitting; docs branch covered by targeted docs tests Reference: upstream/main; source patch /tmp/spec-kit-changes.patch * Address PR 2695 feedback: Centralize prerelease policy and add boundary test * Address remaining Copilot PR feedback: revert docs and add preset prerelease tests * Remove unreachable raise CompatibilityError * Fix PEP8 E302 and E303 formatting issues |
||
|
Ben Buttigieg
|
c47dd2b812 |
chore: retire Windsurf integration — absorbed into Cognition Devin (#3168) (#3213)
* chore: retire windsurf integration — absorbed into Cognition Devin (#3168) windsurf.com now permanently redirects to devin.ai/desktop following acquisition. Remove subpackage, registry/catalog entries, docs, and tests; re-point sample-agent test fixtures to Kilo Code. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: remove stale Windsurf support references Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: fix Kilo Code command path in upgrade guide Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * chore: align integration lists after rebase Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * docs: align kilocode example with runtime behavior Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Huy Do
|
20f430686c |
feat(workflows): honor max_concurrency in fan-out via a bounded thread pool (#3224)
* feat(workflows): honor max_concurrency in fan-out via a bounded thread pool * feat(workflows): address review — sliding-window fan-out, locked output, faithful halt Address the reviewer feedback on the bounded fan-out concurrency: - Sliding submission window: keep at most `workers` items in flight and stop launching new items once the run is halting, instead of submitting all items up front (which let the pool keep starting queued work after a halt). - Faithful halt prefix: attribute a halt to the specific item whose own recorded result halted the run (replaying the sequential break condition, honoring continue_on_error/aborted), not the shared run status a later concurrent item may have flipped. The returned prefix now includes the actual halting item, matching the sequential path. An item that fails before recording a result (e.g. an unknown step type) is attributed too, since every item runs the same template. - Lock the parent fan-out output mutation: route the post-fan-out step_results[...]['output'] update through a new RunState.set_step_output() under the run lock, so it cannot race a concurrent save(). - Docstring: describe int() coercion accurately (numeric strings / floats are honored; only non-coercible or <= 1 runs sequentially). Tests: add concurrent halt-includes-halting-item, continue_on_error-does-not- truncate, and unknown-template-type-matches-sequential coverage; make the timing test use a monotonic clock with a looser threshold to avoid CI flakiness. * feat(workflows): address second review pass — concurrency hardening - append_log: serialize the log_entries append + log.jsonl write under a dedicated RunState._log_lock so concurrent fan-out workers can't interleave or corrupt log lines (kept separate from the state lock; never nested). - _run_fan_out.run_item: read the item output back through the item_ctx it executed against rather than the outer context closure — clearer and robust if StepContext ever stops sharing the steps dict by reference. - StepBase: document the thread-safety contract — STEP_REGISTRY holds one shared instance per type, so concurrent fan-out invokes execute() on the same object; implementations must be stateless/thread-safe (the built-ins already are). - test_concurrency_is_real: prove parallelism deterministically with a threading.Barrier (sequential execution can't clear it) instead of a wall-clock timing assertion. * feat(workflows): address review — stamp updated_at under lock, clarify cancel semantics - RunState.save(): move the updated_at timestamp assignment inside the run lock so the timestamp matches the snapshot the thread serializes and concurrent savers don't race on it. - _run_fan_out docstring: clarify that on a halt only not-yet-started items are cancelled; items already running finish but their outputs are ignored (Future.cancel() can't stop running work, and the pool joins on exit). * feat(workflows): serialize on_step_start callback under a lock The concurrent fan-out path invokes _execute_steps from worker threads, which calls the engine's on_step_start callback (the CLI sets it to a console.print lambda). Concurrent invocation could interleave/garble progress output. Guard the call with a WorkflowEngine._callback_lock so callbacks are serialized; the lock is uncontended for sequential runs. * feat(workflows): re-raise worker exceptions in-place to preserve traceback In _run_fan_out's concurrent path, a worker exception was stashed in first_exc and re-raised after the loop. Re-raise it from within the except block with a bare `raise` (after cancelling outstanding futures) so the original traceback is preserved, and drop the now-unneeded first_exc variable. The ThreadPoolExecutor __exit__ still joins any already-running workers before the exception escapes. * feat(workflows): lock final fan-out status, drop redundant output write, bound workers Address third review pass: - Remove the unlocked `context.steps[step_id]["output"] = …` writes in the fan-out parent update. context.steps[step_id] is the same dict object that set_step_output() updates under the run lock, so the direct (unsynchronized) mutation was redundant. - Preserve sequential halt semantics under concurrency: a later in-flight item could overwrite state.status after the halting item was identified. _run_fan_out now derives the halting item's run status (item_halt_status, replacing the bool item_halted) and restores it after the pool joins, so the final status is the first halting item's outcome. - Bound the pool: workers = min(max_concurrency, len(items)) and early-return for empty items, so a user-controlled max_concurrency can't over-allocate threads. Add coverage that an earlier PAUSED item's status wins over a later concurrent FAILED item. * feat(workflows): avoid unlocked context.steps writes when it aliases step_results On a resume run, StepContext is built with steps=state.step_results, so the two direct `context.steps[...] = ...` writes mutated the shared dict outside the run lock and could race save(). Route both through a new _record_result helper that mirrors into context.steps only when it is a distinct object (a fresh run) and otherwise relies solely on record_step_result's locked write. |
||
|
Ben Buttigieg
|
28a38af6c1 |
chore: retire iflow integration — product discontinued (#3166) (#3211)
Remove the iFlow CLI integration whose product was shut down: subpackage, registry entry, catalog entry, docs, tests, and issue-template options. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Noor ul ain
|
cb7c36c95b |
fix: reject host-less catalog URLs in base and preset validators (#3209) (#3227)
`CatalogStackBase._validate_catalog_url` (inherited by `IntegrationCatalog`) and `PresetCatalog._validate_catalog_url` checked `parsed.netloc`, which is truthy for host-less URLs like `https://:8080` (port only) or `https://user@` (userinfo only). Such URLs slipped past validation despite the error message promising "a valid URL with a host", then failed later with a confusing fetch error. Switch both validators to `parsed.hostname` (None for those inputs), matching the workflow, step, and bundler catalog validators that already do this. Add regression tests covering port-only and userinfo-only URLs for both validators. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Noor ul ain
|
ea1827769a |
fix: stop check-prerequisites --paths-only from writing feature.json (#3025) (#3190)
* fix: stop check-prerequisites --paths-only from writing feature.json (#3025) check-prerequisites --paths-only / -PathsOnly is documented as pure, read-only path resolution, but when SPECIFY_FEATURE_DIRECTORY was set it called the persist routine and rewrote .specify/feature.json. That dirtied the working tree and overwrote a pinned feature directory during what should be a no-op. Add an explicit opt-out at the resolver boundary instead of a global env back-channel: - bash: get_feature_paths accepts a leading --no-persist flag that skips _persist_feature_json; check-prerequisites.sh passes it in --paths-only mode. - PowerShell: Get-FeaturePathsEnv gains a -NoPersist switch that skips Save-FeatureJson; check-prerequisites.ps1 passes it in -PathsOnly mode. Normal (non-paths-only) invocations are unchanged and still persist the override, so future sessions without the env var keep working. Add regression tests asserting --paths-only/-PathsOnly leaves a pinned feature.json untouched even when the env override differs, plus a guard that normal mode still persists. * fix: use ASCII hyphen in common.ps1 comment for PS 5.1 compatibility The em-dash in the persist comment introduced non-ASCII bytes, failing test_ps1_file_is_ascii_only which enforces ASCII-only PowerShell sources for Windows PowerShell 5.1 compatibility. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: add PowerShell normal-mode persistence guard (#3025) Addresses Copilot review feedback on #3190: the bash side had a `test_normal_mode_still_persists_feature_json` guard, but there was no symmetric PowerShell test asserting that running check-prerequisites.ps1 *without* -PathsOnly still persists the SPECIFY_FEATURE_DIRECTORY override into .specify/feature.json. Add test_ps_normal_mode_still_persists_feature_json, which guards against accidentally passing -NoPersist unconditionally (or flipping the default) in a future refactor. Verified it fails when -NoPersist is passed in the non -PathsOnly branch and passes with the current conditional. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Ali jawwad
|
4badf3b5b1 |
fix(scripts): use ASCII [OK] marker in initialize-repo.sh (parity with PowerShell twin) (#3231)
* fix(scripts): use ASCII [OK] marker in initialize-repo.sh (parity with PowerShell twin)
initialize-repo.sh printed its success line with a Unicode checkmark ('✓ Git repository initialized'), while the PowerShell twin initialize-repo.ps1 and both auto-commit scripts use the ASCII marker '[OK]'. That is an output-text divergence across the bash/PowerShell twins and an inconsistency among sibling extension scripts. Use '[OK]' to match.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: assert full [OK] init line and surface stderr on failure
Address Copilot review: assert the full success line '[OK] Git repository initialized' (not just the '[OK]' substring, which could pass if unrelated [OK] output is added later) and include result.stderr in the assertion message so a failure is debuggable.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwad
|
804e7329b8 |
fix(scripts): route 'Plan template not found' per --json in setup-plan.ps1 (parity with bash) (#3241)
The 'template not found' fallback used Write-Warning, which emits 'WARNING: Plan template not found' on the warning stream -- diverging from the bash twin (echo 'Warning: Plan template not found' to stderr in --json, stdout in text mode) in both wording and routing, and inconsistent with the sibling 'Copied plan template' message (#3198) in the same block. Route it the same way so the two scripts share one status-output contract. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Ali jawwad
|
c5fb3dc86f |
fix(bundle): send command errors to stderr so --json stdout stays parseable (#3235)
The bundle command group's _fail() helper is documented as printing 'to stderr', and the module contract is 'human logs go to stderr/console' while --json 'emits machine-readable data on stdout'. But it called console.print(), and the shared console writes to STDOUT, so every bundle error (every command routes through _fail) landed on stdout -- corrupting the JSON stream that --json consumers parse. Add a stderr-bound err_console to _console.py (its documented role as the single Console source) and use it in _fail. stdout now carries only the JSON payload. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Manfred Riem
|
53d9543355 |
feat: make agent-context extension a full opt-in (#3097)
* docs: add Spec Kit spec for agent-context full opt-in Use Spec Kit's own specify workflow to author the spec that makes the agent-context extension a full opt-in, removing all agent-context configuration/support from the Python codebase and removing the deprecation message. Force-added despite specs/ being gitignored; the generated artifact will be purged prior to merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: add Spec Kit plan artifacts for agent-context full opt-in Phase 0/1 of the SDD plan workflow: plan.md, research.md, data-model.md, quickstart.md, and contracts/cli-behavior.md. Constitution Check is a documented no-op (repo has no ratified constitution). Force-added despite specs/ being gitignored; generated artifacts will be purged prior to merge. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: correct Constitution Check against ratified v1.0.0 Earlier draft wrongly treated the gate as a no-op; the fork's main is 16 commits behind upstream/main, which carries .specify/memory/constitution.md. Re-evaluate the feature against Principles I-V (all PASS) and note that Principle I mandates keeping context_file as a declared class attribute, validating the R1 metadata decision. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: refresh plan artifacts against synced upstream/main After syncing fork main to upstream and rebasing, re-scan the current agent-context surface. Upstream generalized the single context_file into a plural context_files concept with new resolver helpers (_resolve_context_files, _resolve_context_file_values, _format_context_file_values) and upsert/remove now loop over multiple files. Update research.md, data-model.md, contracts, quickstart grep guards, and the plan summary to cover the expanded removal scope. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: add Spec Kit tasks for agent-context full opt-in Phase 2 of SDD: dependency-ordered tasks.md (30 tasks) organized by the three user stories, with mandatory test tasks (Constitution Principle II) and a foundational phase decoupling __CONTEXT_FILE__ resolution from the extension config. Includes the extension self-seeding task (T015) and a static guard test (T002) enforcing zero agent-context references in the CLI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat!: remove agent-context lifecycle from the Specify CLI Make the agent-context extension a full opt-in. The CLI no longer installs the extension during init, writes agent-context-config.yml, or creates/updates/removes the managed Spec Kit section in agent context files. Context-section upsert/remove, marker resolution, extension-enabled gating, the config helpers, and the obsolete inline deprecation warning are all removed. Integration context_file stays as inert metadata; __CONTEXT_FILE__ now resolves from registry metadata. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(agent-context): self-seed context file from the active integration When agent-context-config.yml has no context_file/context_files, the bundled bash and PowerShell update scripts now resolve the context file from the active integration in .specify/init-options.json via the integration registry, so the extension no longer depends on the CLI writing its config. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test+docs: update suite and docs for agent-context opt-in Update integration/extension tests to expect no agent-context install, config, or context-section writes during init. Add a static guard test (test_agent_context_cli_free.py) asserting the CLI source is free of agent-context lifecycle symbols, plus backward-compatibility tests for legacy projects. Refresh AGENTS.md, the extension README, and add a CHANGELOG entry describing the opt-in behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(agent-context): warn on self-seed failure, correct docs, speed up guard test Address PR review feedback: - Self-seed scripts (bash + PowerShell) now emit an actionable warning when an active integration is configured but specify_cli cannot be imported by the chosen Python (e.g. pipx installs), or when the integration declares no context file, instead of silently falling through to 'nothing to do'. - Correct the extension README disable note: command rendering never reads the extension config; __CONTEXT_FILE__ is always substituted from integration metadata, so a stale context_files value cannot affect rendering. - Cache CLI source reads in the static guard test via a module-scoped fixture so the directory walk happens once instead of once per forbidden symbol. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(agent-context): ship self-owned per-agent context-file defaults The extension now bundles agent-context-defaults.json (key→context_file map) and self-seeds from it, dropping any dependency on the Specify CLI registry. Both the bash and PowerShell update scripts read the bundled JSON map keyed by the active integration from init-options.json. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat!: remove all agent-context state from the Specify CLI Strip every context_file reference from the CLI: the field on all 35 integration classes, the IntegrationBase plumbing (process_template param/step, _context_file_display, docstrings), the __CONTEXT_FILE__ resolution in agents.py, the legacy context_file/context_markers popping in _helpers.py, and the context_file template in integration_scaffold.py. Also drop the Agent context update step and __CONTEXT_FILE__ placeholder from templates/commands/plan.md. The agent-context extension now solely owns all context-file knowledge, including the per-agent default mapping. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: drop context_file coverage and guard against CLI reintroduction Remove CONTEXT_FILE attrs and context_file assertions across the base mixins, all 35 per-integration test files, shared integration tests, and conftest stubs. Rewrite the base-mixin context tests to assert no managed section is written and no __CONTEXT_FILE__ placeholder survives. Extend the CLI-free static guard to forbid context_file, __CONTEXT_FILE__, and _context_file_display in src/specify_cli, and have the extension tests copy the bundled defaults JSON so self-seed runs without the CLI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: reflect full removal of agent-context state from the CLI Update AGENTS.md (integration examples, required-fields table, context behavior section, pitfalls), CHANGELOG, and the SDD spec artifacts (FR-007, SC-002, data-model) to state that the CLI carries no context_file and the extension fully owns the per-agent default mapping via agent-context-defaults.json. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: align SDD artifacts with full context_file removal Update research.md (R1, R2, R4, summary table), contracts/cli-behavior.md (C3, C5), tasks.md (Phase 2, T026, notes), plan.md (Principle I, source map), and checklists/requirements.md so the spec artifacts reflect the implemented decision: the CLI carries no context_file attribute or __CONTEXT_FILE__ resolution, and the per-agent defaults map lives in the extension. Resolves PR review #4548130110. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: scrub stale context-file mentions from CLI docstrings Update the multi_install_safe docstring (drop the removed "context file" invariant), the RovoDev setup docstring (no longer upserts a context section), the Copilot module docstring (drop the context-file line), and tighten the _update_init_options_for_integration note. Pure docstring changes — no behavioral impact. Resolves PR review #4548237085. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test+docs: harden agent-context test helper and fix stale docs - base.py: document multi_install_safe as an optional subclass attribute in the IntegrationBase docstring. - test_cli.py: clarify the init-options assertion is guarding against leftover legacy agent-context keys, not relocation. - test_extension_agent_context.py: _install_agent_context_config now asserts the bundled agent-context-defaults.json exists and always copies it, so self-seeding tests fail loudly instead of silently skipping when the map is missing. - test_integration_cursor_agent.py: drop Path/IntegrationManifest imports left unused after removing the context-section frontmatter tests. Resolves PR review #4548293116. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: remove gitignored SDD artifacts from specs/ The specs/001-agent-context-full-optin/ artifacts were force-added for dogfooding visibility, but specs/ is gitignored and these were always intended to be purged before merge. Remove them so merging does not add an intentionally-untracked directory to repo history. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore: keep CHANGELOG.md identical to upstream CHANGELOG.md is auto-generated at release time, so the branch should not carry a manual entry. Restore it to match upstream/main exactly. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: preserve Cursor .mdc frontmatter in agent-context updater scripts The bundled agent-context updater scripts wrote the managed section as plain text. For Cursor-style `.mdc` targets this dropped the required `---\nalwaysApply: true\n---` frontmatter, reintroducing the rule-loading bug originally fixed in #1699. Port the `_ensure_mdc_frontmatter` logic into both the bash and PowerShell updaters: prepend frontmatter when missing, repair `alwaysApply` when set to the wrong value, and leave non-`.mdc` targets untouched. Add regression tests covering both shells. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: scope CLI-free guard to agent-context-specific symbols Drop the bare "context_file" substring from FORBIDDEN_SYMBOLS so the guard no longer fails on unrelated future CLI fields named context_file. The list still covers agent-context-specific identifiers (__CONTEXT_FILE__, _context_file_display, _resolve_context_files, _resolve_context_file_values). Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: harden agent-context bash self-seed against malformed init JSON Two robustness fixes in the embedded Python self-seed logic: - Coerce the integration value from init-options.json to a string only when it is actually a string; otherwise treat it as unset so a corrupted dict/list value degrades to the existing nothing-to-do behavior instead of breaking the agents-map lookup. - Normalize agent-context-defaults.json: only use 'agents' when both the JSON root and the 'agents' value are dicts, so a wrong-shaped (but valid) JSON falls back to the warning path instead of raising on .get. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: correct PowerShell hyphenated key lookup and regex replace count - Self-seed now reads the defaults mapping via $defaults.agents.PSObject.Properties[$integrationKey].Value instead of member access ($defaults.agents.$integrationKey), which parsed hyphenated keys like 'cursor-agent'/'kiro-cli' as subtraction and failed to resolve. - Replace the static [regex]::Replace(..., 1) call, whose trailing 1 was interpreted as RegexOptions.IgnoreCase rather than a replacement count, with an instance Regex whose Replace(input, replacement, 1) limits to the first match as intended. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: make bash .mdc frontmatter guard case-insensitive The bash updater only injected Cursor .mdc frontmatter when ctx_path ended in lowercase '.mdc', so a mixed/upper-case extension (e.g. specify-rules.MDC) was skipped and Cursor would not auto-load the rule file. Compare against the casefolded path. The PowerShell variant already uses -match, which is case-insensitive by default, so no change is needed there. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: document separator-agnostic agent-context update invocation The README hard-coded the dot-notation slash command (/speckit.agent-context.update), which hyphen-separator agents like Forge and Cline do not recognize. Document the canonical command ID plus both slash invocations so users copy the form their agent accepts. Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Ali jawwad
|
876dca8659 |
fix(workflows): gate validate() must not crash on non-string options (#3233)
GateStep.validate() reports non-string options as an error, but then -- when on_reject is 'abort'/'retry' -- still runs the reject-choice check 'any(o.lower() in ... for o in options)'. For a non-string option (e.g. options: [123]) o.lower() raised AttributeError, which escaped validate() and broke validate_workflow's documented 'return a list of errors, never raise' contract. Guard the check so it only runs when every option is a string (the non-string case is already reported above). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Ali jawwad
|
9ece347a77 |
fix(workflows): make pipe-filter detection quote-aware in expressions (#3232)
_evaluate_simple_expression used 'if "|" in expr' / expr.split("|", 1) to detect a filter pipe, so a literal '|' inside a quoted operand (e.g. inputs.x == 'a|b') was mistaken for a filter separator and raised a spurious ValueError ('unknown filter') instead of comparing the string. Use the existing quote/bracket-aware _find_top_level helper (added for the operator-splitting fix) so only a top-level pipe is treated as a filter separator.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Huy Do
|
3036fe6954 |
fix(workflows): reject a fan-in wait_for that names an unknown step at validation (#3225)
* fix(workflows): reject a fan-in wait_for that names an unknown step at validation * fix(workflows): reject fan-in wait_for self-reference and non-string entries Address review feedback on the fan-in wait_for validator: - A fan-in's own id is added to seen_ids before the wait_for check, so `wait_for: [<self>]` passed validation while producing a silent empty join at runtime. Reject self-references explicitly. - Non-string entries (e.g. YAML `wait_for: [123]`) were skipped by the isinstance(str) guard and validated even though they can never match a real step id. Flag them as wiring errors. Add coverage for both cases. |
||
|
Ali jawwad
|
a473955e3e |
fix(scripts): warn when spec template is missing in create-new-feature.ps1 (parity with bash) (#3230)
* fix(scripts): warn when spec template is missing in create-new-feature.ps1 (parity with bash) create-new-feature.sh prints 'Warning: Spec template not found; created empty spec file' to stderr when no spec template resolves, then touches an empty spec. The PowerShell twin created the empty file silently with no warning, so on Windows a missing/broken template tree gave no signal. Emit the same warning on stderr (keeps stdout/JSON pure), matching the bash wording and stream. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: assert create-new-feature.ps1 warns on missing spec template Regression test for the bash/PowerShell parity fix: with no resolvable spec template, the PowerShell script must emit 'Spec template not found' on stderr (matching bash) while keeping stdout parseable JSON and still creating the empty spec file. Gated on pwsh; decodes stdout/stderr as UTF-8. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Ali jawwad
|
a4972da717 |
fix(scripts): count subdirectory-only dirs as non-empty in PowerShell (parity with bash) (#3137)
* fix(scripts): count subdirectory-only dirs as non-empty in PowerShell
Test-DirHasFiles (the documented PowerShell twin of bash check_dir) tested
non-emptiness with `Get-ChildItem | Where-Object { -not $_.PSIsContainer }`,
counting only top-level FILES and ignoring subdirectories. Bash check_dir
(`-n $(ls -A ...)`) and the PowerShell JSON-path contracts checks
(check-prerequisites.ps1 / setup-tasks.ps1, no PSIsContainer filter) both
count ANY entry. So a contracts/ directory whose only contents are
subdirectories (e.g. contracts/v1/openapi.yaml) was reported present by
bash, by bash JSON, and by PowerShell JSON, but [FAIL]/absent by PowerShell
text mode — the lone outlier.
Drop the PSIsContainer filter so Test-DirHasFiles counts any entry, matching
the other three code paths.
Add bash + PowerShell parity tests asserting a subdir-only contracts/ dir is
reported non-empty in both shells.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* review: accurate non-empty comment + drop doubled test prefix
Address review feedback on Test-DirHasFiles parity fix:
- Reword the common.ps1 comment so it no longer claims exact `ls -A` parity (Get-ChildItem omits hidden entries without -Force); it now points at the in-repo PowerShell JSON contracts checks as the matching reference and keeps the subdir-only-is-non-empty rationale.
- Rename test_test_dir_has_files_ps_... -> test_dir_has_files_ps_... to drop the doubled 'test_' prefix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: assert dir-non-emptiness via stdout marker, not exit code
Address Copilot review: check_dir always exits 0 (it echoes the marker rather than setting an exit code) and Test-DirHasFiles returns a boolean (pwsh still exits 0 when it returns $false), so 'result.returncode == 0' validated nothing. Drop the misleading assertion and rely on the [OK]/checkmark marker in stdout, which is the actual behavioral signal; document why inline.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: keep common.ps1 ASCII-only (PowerShell 5.1 compatibility)
My reworded Test-DirHasFiles comment introduced an em dash (U+2014), which tripped tests/test_ps1_encoding.py::test_ps1_file_is_ascii_only -- .ps1 files must stay ASCII for Windows PowerShell 5.1. Replace it with '--', matching the existing comment style in this file (e.g. the Resolve-SpecifyInitDir docstring).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: decode dir-parity subprocess output as UTF-8 explicitly
Address Copilot review: check_dir echoes the non-ASCII markers ✓/✗, and subprocess.run with text=True but no encoding decodes via the platform locale (cp1252 on Windows), which can raise UnicodeDecodeError or mangle stdout. Pin encoding='utf-8' on both the bash and PowerShell dir-parity helpers so decoding is deterministic across CI runners.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwad
|
7b687d8bbd |
fix(scripts): drop HAS_GIT from PowerShell git-extension output (parity with bash) (#3195)
* fix(scripts): drop HAS_GIT from PowerShell git-extension output (parity with bash)
create-new-feature-branch.ps1 emitted a HAS_GIT key in its JSON output and a 'HAS_GIT:' line in text output that the bash twin never emits. The bash output contract is {BRANCH_NAME, FEATURE_NUM} (+ DRY_RUN) only, so a tool parsing the machine-readable output got a different shape on Windows/PowerShell vs macOS/Linux -- a cross-platform contract divergence.
$hasGit is still computed and used internally for branch-creation logic; only its two output emissions are removed, restoring parity. Added regression tests asserting neither the PS nor the bash output contains HAS_GIT (JSON and text). Noted as a follow-up in #3129.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: note DRY_RUN in the HAS_GIT-omission comment (parity)
Address Copilot review: the comment described the output contract as {BRANCH_NAME, FEATURE_NUM} without mentioning that DRY_RUN is still conditionally added in JSON mode on dry runs. Clarify so the contract description is complete for future maintainers. Comment-only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Manfred Riem
|
bbc5f176e3 |
fix(extensions): apply GHES auth and resolve release assets for extension add --from (#3217)
* fix(extensions): apply GHES auth and resolve release assets for --from The 'specify extension add --from <url>' path fetched ZIPs via a bare open_url with no GitHub release-asset resolution and no Accept header, diverging from the catalog download path. Against GHES it received an HTML login page and failed obscurely with zipfile.BadZipFile. Route --from through ExtensionCatalog so configured GHES credentials apply and release-download URLs resolve via /api/v3, and reject non-ZIP content with a clear error pointing at auth.json. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(extensions): use zipfile.is_zipfile for --from content guard Replace the weak zip_data.startswith(b"PK") prefix check with zipfile.is_zipfile() on a BytesIO so any non-ZIP payload (not just those lacking the PK magic) is rejected with the friendly error before install_from_zip can raise BadZipFile. Addresses PR review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Quratulain-bilal
|
5bdcb4ad14 |
fix(catalogs): reject host-less catalog URLs in base and preset validators (#3210)
the shared CatalogStackBase validator and PresetCatalog validator checked parsed.netloc to enforce 'a valid URL with a host'. but netloc is truthy for host-less URLs like https://:8080 or https://user@, so those slipped through even though they have no host - contradicting the error message. the workflow, step, and bundler validators already check parsed.hostname (which is None in those cases); this aligns the two stragglers with that. add regression tests covering port-only and userinfo-only URLs. |
||
|
WOLIKIMCHENG
|
9a40ed0b6e |
fix: update CodeBuddy install docs URL (#3187)
* fix: update CodeBuddy install docs URL * test: assert codebuddy integration is registered before checking install_url --------- Co-authored-by: root <kinsonnee@gmail.com> |
||
|
Ali jawwad
|
d378485696 |
fix(workflows): reject infinite number-input default instead of raising OverflowError (#3199)
WorkflowEngine._coerce_input normalizes a whole-valued number to int via int(value). For an infinite float (e.g. a 'type: number' input with YAML 'default: .inf') int(inf) raises OverflowError, which is not in the except (ValueError, TypeError) tuple. validate_workflow eager-coerces declared defaults and is documented to RETURN a list of errors, but it only catches ValueError -- so the OverflowError escaped and validate_workflow raised instead of reporting, breaking its contract. (NaN already surfaced cleanly because int(nan) raises ValueError.) Add OverflowError to the except tuple so an infinite default surfaces as the same clean 'expected a number' ValueError as NaN, consistent with the function's existing fail-fast-on-authoring-mistakes design. Finite values (5.0 -> 5, 3.5 -> 3.5) are unaffected. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Ali jawwad
|
96f73d192c |
fix(scripts): emit 'Copied plan template' status in setup-plan.ps1 (parity with bash) (#3198)
setup-plan.sh prints 'Copied plan template to $IMPL_PLAN' after copying the template (to stderr in --json mode, stdout otherwise), but the PowerShell twin emitted nothing on the successful-copy path -- only the 'Plan already exists' skip message and the 'Plan template not found' warning existed. So the two scripts had a divergent status-output contract on a fresh run. Emit the same message after WriteAllText, routed like the sibling skip message ([Console]::Error.WriteLine in -Json so stdout stays pure JSON, Write-Output in text mode). Mirrors the bash wording and stream routing exactly. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Ali jawwad
|
2a9db1d350 |
fix(workflows): make expression operator/literal parsing quote-aware (#3197)
_evaluate_simple_expression split on operator keywords using naive str.find/split, so a keyword INSIDE a quoted operand was treated as an operator: `inputs.mode == 'read and write'` split on the inner ' and ' and evaluated as `(inputs.mode == 'read) and (write')`. The literal short-circuit was also too greedy -- `'a' == 'b'` matched startswith("'")/endswith("'") and was stripped to the garbage truthy string `a' == 'b`, so `'done' == 'failed'` evaluated truthy and gated the wrong branch.
Add a quote/bracket-aware _find_top_level helper (mirroring the existing _split_top_level_commas) and use it for the and/or/comparison/in/not-in splits; tighten the literal short-circuit to fire only when the opening quote's match is the final char. The docstring already lists comparisons + and/or/not + in/not-in + string literals as supported, so this restores the documented contract.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwad
|
fd185c1fd8 |
fix(scripts): honor explicit -Number 0 in PowerShell create-new-feature (parity with bash) (#3196)
Get-BranchName used `[long]$Number = 0` as both the default and the 'auto-detect' sentinel (`if ($Number -eq 0)`), so an explicitly-passed `-Number 0` was indistinguishable from 'not supplied' and silently auto-incremented. The bash twin keys off whether the value is non-empty (`[ -z "$BRANCH_NUMBER" ]`), so `--number 0` is honored and yields FEATURE_NUM 000 -- a cross-platform divergence for identical input.
Use $PSBoundParameters.ContainsKey('Number') instead, so an explicit value (including 0) is honored and only a missing -Number auto-detects -- mirroring bash. This also aligns the -Timestamp+-Number warning, which bash emits for `--number 0 --timestamp` (non-empty check) but PowerShell previously skipped.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Manfred Riem
|
c49966da4d |
fix(claude): stop forking /speckit-analyze to prevent long-session freezes (#3188)
PR #2511 added `context: fork` + `agent: general-purpose` to the generated speckit-analyze SKILL.md on the assumption that its heavy reads collapse to a short summary. In practice /speckit-analyze returns a 300-500 line report that is injected back into the main conversation. In long sessions each subsequent fork inherits that growing context, compounding overhead until the chat freezes (#3185). Empty FORK_CONTEXT_COMMANDS so no command opts into context: fork, restoring direct in-session execution for analyze. The injection mechanism is retained so a command can be re-enabled once it genuinely returns a compact result. Fixes #3185 Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Amirreza Alibeigi
|
49cc05384a |
fix: derive plan path from feature.json in update-agent-context (#3069)
* fix: derive plan path from feature.json in update-agent-context When `plan_path` is omitted, prefer `.specify/feature.json` (written by /speckit-specify) over the mtime heuristic. The old approach picked the most recently modified `specs/*/plan.md`, which could inject an unrelated plan into CLAUDE.md if another spec's plan was touched after the active feature directory was created but before its own plan.md existed. Bash: handle both relative and absolute feature_directory values, normalizing absolute paths back to project-relative for the context file. Fall back to mtime only when feature.json is absent or the derived plan.md does not yet exist. PowerShell: same logic, PS 5.1-compatible (nested Join-Path, IsPathRooted guard to avoid Unix Join-Path mis-joining absolute ChildPaths, manual prefix-strip instead of GetRelativePath). Fixes #3067 * fix: address Copilot review feedback on update-agent-context - bash: add explicit encoding="utf-8" to feature.json open() call - powershell: replace GetRelativePath (.NET 5+ only) with manual prefix-strip in mtime fallback for PS 5.1 compatibility - tests: add coverage for absolute feature_directory values (under and outside PROJECT_ROOT) * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * test: replace time.sleep with os.utime and strengthen PS normalization assertion * Apply suggestions from code review 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: normalize trailing slash and guard non-string feature_directory in PS script * Fix: use .resolve().as_posix(). Valid. The PS tests run on Windows where str(tmp_path) uses backslashes, but the PS script normalizes output to forward slashes. Assertions like assert str(tmp_path) not in ctx become false negatives on Windows CI. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: use context manager for feature.json open() in bash heredoc * test: add PS coverage for absolute feature_directory outside project root * fix: guard null feature_directory, re-check empty after trailing-slash strip, fix blank line * test: add stale plan to absolute-path tests so feature.json preference is actually exercised * test: convert absolute paths to MSYS2 style for Git-for-Windows bash compatibility * fix: revert PS test to native path, fix bash outside-root assertion for Git bash * fix: use _to_bash_path in not-in assertion for Git bash Windows compat * fix: add ConvertFrom-Json fallback in PS script, write test config as JSON * fix: use OS-appropriate StringComparison in PS prefix-strip (matches common.ps1) * fix: emit project-relative POSIX path from mtime fallback; use upstream test helpers * fix: write config as JSON directly, drop _install_agent_context_config * fix: normalize backslashes to forward slashes in feature_directory before path ops * fix: treat drive-qualified paths (C:/...) as absolute after backslash normalization * fix: resolve symlinks when computing relative plan path; use UTF8 encoding in PS ConvertFrom-Yaml path * fix: use bash-side path for outside-root case to avoid WindowsPath backslashes * fix: use .as_posix() instead of PurePosixPath() to avoid backslashes on native Windows Python * fix: resolve ./.. segments in PS feature_directory via GetFullPath before relativizing * fix: replace $IsWindows guard with OSVersion.Platform check for PS 5.1 StrictMode compat * fix: guard empty relDir to avoid leading slash in PlanPath when feature_directory is project root * fix: remove unused PurePosixPath import; fix stale PS comment after ConvertFrom-Json fallback was added * fix: use cand.as_posix() for outside-root path instead of raw bash-side argv --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Si Zengyu
|
1add20341d |
fix(extensions,presets,workflows): resolve private GHES release assets via /api/v3 (#3157)
* feat(auth): add github_provider_hosts() to enumerate GHES hosts from auth.json Assisted-by: Claude Code (model: claude-sonnet-4-6, autonomous) * fix(extensions): resolve GHES release assets via /api/v3 Generalizes resolve_github_release_asset_api_url to GitHub Enterprise Server hosts (gated by auth.json github hosts), fixing private GHES extension/preset downloads. github/spec-kit#3147 Assisted-by: Claude Code (model: claude-sonnet-4-6, autonomous) * fix(extensions,presets): pass auth.json github hosts into release resolver Assisted-by: Claude Code (model: claude-sonnet-4-6, autonomous) * docs(auth): document GHES private catalog + release-asset auth Assisted-by: Claude Code (model: claude-sonnet-4-6, autonomous) * fix(presets,workflows): pass auth.json github hosts into remaining release resolvers Wires preset add --from and workflow add through github_provider_hosts() so private GHES release assets resolve via /api/v3 there too. github/spec-kit#3147 Assisted-by: Claude Code (model: claude-sonnet-4-6, autonomous) * test(presets): use module-level io.BytesIO in GHES preset test Addresses Copilot review on PR #3157: drop unnecessary __import__("io") in test_preset_add_from_ghes_release_url_resolves_via_api_v3 since io is already imported at module level. * fix(github-http): pass through GHES asset API URLs by path shape Addresses Copilot review on PR #3157. A direct GHES /api/v3 release asset URL was only returned as already-resolved when its host was in the allowlist; otherwise the resolver returned None and the caller downloaded the same URL without 'Accept: application/octet-stream', fetching JSON metadata instead of the binary. Gate the passthrough on path shape alone, mirroring the github.com case. This is safe: passthrough returns the input URL unchanged and the caller fetches it either way, so no new request to an arbitrary host is induced; the token stays independently gated by auth.json in open_url. The allowlist remains the anti-SSRF gate on the tag-lookup resolving path. Add test_passthrough_for_unlisted_ghes_api_asset_url. |
||
|
Ali jawwad
|
9fe1c4cc5c |
fix(scripts): keep PowerShell branch-name acronym match case-sensitive (parity with bash) (#3129)
* fix(scripts): keep PowerShell branch-name acronym match case-sensitive
Get-BranchName keeps a sub-3-character word only when it appears as an
UPPERCASE acronym in the description. The bash twin checks this
case-sensitively (grep "\b${word^^}\b" / grep -qw -- "${word^^}"), but the
PowerShell twin used -match, which is case-INSENSITIVE, so it kept EVERY
short word regardless of case -- contradicting its own comment and diverging
from bash. The same description then produced different spec-directory and
branch names on Windows/PowerShell vs macOS/Linux (e.g. "Add go support" ->
001-go-support instead of 001-support), desyncing specs/, feature.json, and
git branches across a mixed-OS team.
Use the case-sensitive -cmatch so a short word is kept only for a genuine
uppercase acronym, matching bash. Applied to both the core
scripts/powershell/create-new-feature.ps1 and the git extension's
create-new-feature-branch.ps1.
Add bash + PowerShell regression tests (core and git-extension) asserting a
lowercase short word is dropped while an uppercase acronym is kept.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: fix article grammar in branch-name docstrings
Address review: 'an UPPERCASE acronym' -> 'an acronym in UPPERCASE' across the four branch-name case-sensitivity test docstrings (the indefinite article reads cleanly before 'acronym'). Docstring-only; no behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
meymchen
|
dc840f07d0 |
feat(integration): update Kimi integration for Kimi Code CLI (#2979)
* feat(integration): update Kimi integration for Kimi Code CLI Update the Kimi integration to target the new Kimi Code CLI (MoonshotAI/kimi-code) layout: - Change skills directory from .kimi/skills/ to .kimi-code/skills/ - Change context file from KIMI.md to AGENTS.md - Extend --migrate-legacy to move old .kimi/skills/ installs and migrate KIMI.md user content to AGENTS.md - Clean up leftover legacy .kimi/skills/ directories on teardown - Update devcontainer installer to @moonshot-ai/kimi-code - Update docs and tests Relates to #1532 * fix(integration): align Kimi dispatch and harden legacy migration - Override build_command_invocation to emit /skill:speckit-<stem> so dispatched commands match Kimi Code CLI's native slash syntax. - Skip symlinked .kimi/skills directories during legacy migration and teardown to avoid operating on files outside the project. - Remove kimi from the multi-install-safe integrations table. - Add tests for command invocation and symlink safety. * fix(integration): resolve custom context markers in Kimi legacy migration Use IntegrationBase._resolve_context_markers() when migrating legacy KIMI.md content so that projects with customized context_markers in .specify/extensions/agent-context/agent-context-config.yml have the managed section stripped with the correct markers instead of the hard-coded defaults. Adds a test verifying custom markers are respected during --migrate-legacy. * fix(integration): harden Kimi legacy migration against symlinked paths * fix(kimi): guard symlinked SKILL.md during migration and teardown * docs(kimi): mention KIMI.md→AGENTS.md migration in --migrate-legacy help The --migrate-legacy help text listed only the skills directory move and dotted→hyphenated renaming, but the flag also migrates KIMI.md user content into AGENTS.md. Align the help with the actual behavior, docs, and tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(kimi): validate legacy migration destination; clarify docstrings Address Copilot review feedback on PR #2979: - setup(): gate skills migration on _is_safe_legacy_dir(new_skills_dir) as well as the source. base setup() already rejects a destination that escapes the project root, but an in-tree symlinked .kimi-code/skills (e.g. -> .) could still misdirect the move; this gives the destination the same symlink-component protection as the source. - _migrate_legacy_kimi_dotted_skills: rewrite docstring as a compatibility shim describing same-path delegation to _migrate_legacy_kimi_skills_dir. - test_presets: clarify that the dotted-skill test exercises legacy naming under the current .kimi-code/ base, not the legacy .kimi/ location. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(kimi): harden legacy KIMI.md→AGENTS.md context migration - Skip context-file migration when the agent-context extension is disabled, matching upsert/remove_context_section opt-out behavior so an opted-out project's KIMI.md/AGENTS.md are left untouched. - Safely skip (instead of raising) on filesystem edge cases: unreadable or non-UTF-8 KIMI.md, and AGENTS.md existing as a non-file/unwritable. - Refuse to migrate a corrupted managed section (single marker, or end before start) so a partial managed block is never copied into AGENTS.md; KIMI.md is preserved for manual repair. Add regression tests for all three cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Approve fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * chore(kimi): revert CHANGELOG.md edit (auto-generated) The CHANGELOG is generated from merged PR titles, so a hand-written entry is redundant; it was also placed under the already-released 0.10.2 section, which would make those release notes historically inaccurate. Revert to match main per maintainer feedback. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(kimi): skip symlink-safety tests when symlinks are unavailable The Kimi legacy-migration safety tests create symlinks to assert that migration/teardown never follow them out of the project. Symlink creation fails on Windows without the create-symlink privilege and in some restricted CI sandboxes, so these tests errored during setup instead of skipping. Wrap every symlink_to() call in a shared _symlink_or_skip() helper that pytest.skip()s on OSError/NotImplementedError, matching the guard pattern already used by one of these tests. Verified on Windows: the 6 symlink tests now skip cleanly (51 passed, 6 skipped) instead of erroring. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(kimi): reject symlinked skills destination before install Add a destination symlink pre-check in KimiIntegration.setup() before super().setup() writes any SKILL.md. The base class only rejects a destination that escapes project_root after resolve(), so an in-tree symlinked .kimi-code/.kimi-code/skills (e.g. `-> .`) would still misdirect writes into an unintended in-tree location (./skills/). Extract the symlink-component walk into a shared _has_symlinked_component() helper and reuse it from _is_safe_legacy_dir(). Add a regression test. Also clarify that --migrate-legacy only migrates KIMI.md -> AGENTS.md when the agent-context extension is enabled, in the CLI help text and the integration docs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Refactor formatting and simplify logic in Kimi integration * fix(kimi): reject symlinked target dir during legacy skills migration When the migration destination already exists, guard against a symlinked (or non-directory) target_dir before comparing SKILL.md bytes, so the comparison never follows a link outside the project root. Also skip a missing/non-file target SKILL.md explicitly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Ali jawwad
|
fdaaf18371 |
fix(workflows): preserve commas inside quoted list-literal elements (#3134)
* fix(workflows): preserve commas inside quoted list-literal elements
The simple-expression evaluator parsed a list literal with a naive
`inner.split(",")`, which splits on commas inside quoted strings (and
nested brackets). So `{{ ["a, b", "c"] }}` evaluated to three items
(`["a", "b", "c"]`) instead of two, silently corrupting `fan-out` `items:`
and any list expression that contains a comma inside a quoted element.
Split list-literal elements on top-level commas only, ignoring commas
inside quotes or nested brackets, via a small `_split_top_level_commas`
helper. Plain and empty lists are unchanged.
Add tests covering quoted commas, nested lists, and the existing
plain/empty cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(workflows): cover single-quoted and nested list literals
Address review: extend the list-literal regression test to assert single-quoted elements with commas and nested lists parse correctly, alongside the existing double-quoted cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Pascal THUET
|
e5df517ddc |
ci: pin actions to commit SHAs and add shellcheck (#3126)
* ci: pin actions to commit SHAs and add shellcheck Pin actions/github-script in catalog-assign.yml to a full commit SHA; all other workflows were already pinned. Add a repo-wide regression test that every workflow `uses:` ref is pinned to a 40-char commit SHA. Add a shellcheck job to lint.yml (--severity=error over scripts/bash/*.sh) and document the local command in CONTRIBUTING.md. * ci: use repo-standard actions/checkout v7.0.0 in shellcheck job * ci: shellcheck all tracked shell scripts Assisted-by: Codex (model: GPT-5, autonomous) * ci: address workflow hygiene review feedback Assisted-by: Codex (model: GPT-5, autonomous) |
||
|
Zied Jlassi
|
b042d2a843 |
feat(extensions): verify catalog archive sha256 before install (#3080)
* feat(extensions): verify catalog archive sha256 before install Extension and preset archives were downloaded over HTTPS and unpacked (with Zip-Slip protection) but their bytes were never checked against a known digest. Trust rested entirely on TLS and the integrity of the release host, so a tampered or swapped archive from a compromised third-party release would be installed silently. Maintainers do not audit extension code, so consumer-side integrity is the only available defence. Catalog entries may now pin an optional `sha256` digest. When present, the downloaded archive is verified before it is written to disk and installed; a mismatch aborts with a clear error. Entries without `sha256` keep working unchanged (a DEBUG line records that the download was unverified), so the change is backwards compatible. The check runs on both download paths (extensions and presets) via a single shared helper so the two stay in parity. - Add `verify_archive_sha256` helper in shared_infra (digest match, `sha256:` prefix, case-insensitive; DEBUG log when no digest declared) - Enforce it in ExtensionCatalog.download_extension and PresetCatalog.download_pack, before the archive is written to disk - Document the optional `sha256` field in the publishing guides - Tests: helper unit tests + matching/mismatch/no-digest on both paths Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> Assisted-by: AI * fix(extensions): harden sha256 parsing and tidy download test mocks Follow-up to the review on #3080: - shared_infra.verify_archive_sha256: strip only a literal `sha256:` algorithm prefix (case-insensitive) instead of `split(':', 1)[-1]`, which silently dropped any prefix — so `md5:<64-hex>` was accepted as if it were a valid SHA-256. Validate that the declared value is exactly 64 hex characters and raise a clear error otherwise, and compare with `hmac.compare_digest` for a constant-time check. Add tests covering a malformed digest and a non-`sha256:` prefix (both previously accepted). - Download test helpers: configure the context-manager mock via `__enter__.return_value`/`__exit__.return_value` rather than assigning a `lambda s: s`, which is clearer and independent of the invocation arity. Assisted-by: AI Signed-off-by: Zied Jlassi (Architect AI) <6190550+zied-jlassi@users.noreply.github.com> * fix(extensions): reject a declared-but-empty sha256 instead of skipping verification verify_archive_sha256 skipped on any falsy expected value, so a present-but-empty digest (e.g. sha256: "" reached via ...get("sha256")) silently disabled the integrity check instead of surfacing the authoring error. Guard on expected is None so only an absent digest skips; blank/whitespace/bare-prefix values fall through to the 64-hex validation and are rejected. Adds a regression test. Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> * docs(shared_infra): clarify _SHA256_HEX_RE accepts and normalizes uppercase The comment described the regex as matching '64 lowercase' hex characters, but verify_archive_sha256 lowercases the declared value (raw.lower()) before matching, so an uppercase digest is accepted and normalized rather than rejected. Clarify the comment to avoid misleading future readers. Addresses Copilot review feedback on shared_infra.py. Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> * test(presets): cover the no-sha256 backwards-compatible path Address Copilot review: download_pack's optional sha256 verification was tested for match/mismatch but not the backwards-compatible path where a catalog entry has no sha256 (pack_info.get("sha256") is None). Add a no-sha256 test mirroring the extensions coverage so the helper never silently becomes mandatory for presets. Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> --------- Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> Signed-off-by: Zied Jlassi (Architect AI) <6190550+zied-jlassi@users.noreply.github.com> |
||
|
Zied Jlassi
|
f846d6526c |
fix(workflows): validate requires keys and reject phantom permissions gate (#3079)
* fix(workflows): validate requires keys and reject phantom permissions gate A workflow's `requires` block was parsed but its keys were never validated, so a typo or an unsupported key was silently ignored. Most importantly, authors could write `requires.permissions.shell: true` expecting a runtime capability gate — but no such gate exists: a `shell` step always runs with the user's privileges. The declaration gave a false sense of sandboxing. `validate_workflow` now accepts only the recognised keys (`speckit_version`, `integrations`, `tools`, `mcp`) and rejects anything else, with an explicit error for `requires.permissions` pointing authors to `gate` steps for approval. Docs and the model comment are updated to state that `requires` is advisory, not a security boundary. - Reject non-mapping `requires`, unknown keys, and `requires.permissions` - Clarify workflows reference + PUBLISHING.md shell-step guidance - Tests for valid keys, non-mapping, unknown key, and permissions Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> Assisted-by: AI * fix(workflows): address review feedback on requires validation Follow-up to the review on #3079: - Guard `requires` validation on `is not None` instead of truthiness so a falsy non-mapping value (e.g. `requires: []` or `requires: ''`) is reported as an error instead of being silently skipped; `requires:` (YAML null) is still treated as an omitted block. Add a regression test. - Reword the workflows security note so `requires.permissions` is shown as rejected/unsupported rather than as a valid example of `requires`. - Standardize on US spelling (`_RECOGNIZED_REQUIRES_KEYS`, "recognized") to match the surrounding code and ease searching. - Tighten the permissions-rejection test to assert on specific message markers (`requires.permissions` and the `gate` guidance) so it fails if the validation path or wording drifts. Assisted-by: AI Signed-off-by: Zied Jlassi (Architect AI) <6190550+zied-jlassi@users.noreply.github.com> * fix(workflows): scope requires validation to workflow keys (drop tools/mcp) tools and mcp belong to the bundle manifest requires schema (bundler/models/manifest.py, resolved in bundler/services/resolver.py), not the workflow requires validated here. Drop them from _RECOGNIZED_REQUIRES_KEYS and revert the PUBLISHING.md claim that this PR had introduced, so workflow requires only recognizes speckit_version and integrations. This keeps the existing docs accurate and resolves the inline doc-consistency review comments. Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> * refactor(workflows): type WorkflowDefinition.requires as Any pre-validation self.requires holds the raw parsed value, which before validate_workflow() runs may be a non-mapping (None for a bare 'requires:', a list for 'requires: []', etc.). Annotating it dict[str, Any] was misleading for editors/type-checkers; use Any and document that validate_workflow() enforces the mapping shape. Addresses Copilot review feedback on engine.py. Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> * fix(workflows): reject YAML-null requires: as a non-mapping Address Copilot review: validate requires the same way as inputs. A bare requires: parses as YAML null and was previously treated as an omitted block, which is inconsistent with inputs and lets a stray requires: line be silently ignored. Drop the is-not-None guard and check isinstance(..., dict) directly: an omitted block still defaults to {} (valid), but a present-but-non-mapping value -- YAML null, [] or '' -- is now an authoring error that surfaces. Tests: add YAML-null rejection + an omitted-is-still-valid guard test. Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> --------- Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> Signed-off-by: Zied Jlassi (Architect AI) <6190550+zied-jlassi@users.noreply.github.com> |
||
|
Quratulain-bilal
|
37e0e71b4e |
fix(scripts): use case-sensitive match for acronym retention in PS branch names (#3130)
The branch-name generator keeps a short (<3 char) word only when it appears in uppercase in the description, treating it as an acronym (the comment says as much). The bash script uses a case-sensitive grep for this, but the PowerShell script used -match, which is case-insensitive by default. As a result every short non-stop word was retained on PowerShell even when lowercase, so the same description produced different branch names across the two shells (e.g. 'go AI now' -> 001-go-ai-now on PS vs 001-ai-now on bash). Switch to -cmatch so the check is case-sensitive and the two shells agree. Adds parity tests covering a dropped lowercase short word and a kept uppercase acronym. |
||
|
Omar
|
44ef11aa18 |
feat(integrations): add omp support (#3107)
* feat(integrations): add omp support * Update updated_at timestamp * refactor(integrations): delegate omp build_exec_args to base, register in issue templates Inherit MarkdownIntegration.build_exec_args so omp picks up shared CLI contract changes (requires_cli gating, extra-args ordering, --model handling) automatically; only specialize the --mode json flag. Also add Oh My Pi / omp to the issue-template agent lists so test_issue_template_agent_lists_match_runtime_integrations passes. * fix(integrations): use --print + positional prompt for omp argv OMP's CLI parser treats `-p`/`--print` as a boolean (one-shot mode) and consumes the prompt as a positional message; the previous inherited `-p <prompt>` shape worked by accident only because `-p` ignores its next token. Build the argv explicitly with flags first and the prompt as a trailing positional, matching upstream args.ts. |
||
|
Ali jawwad
|
034fbfcbb4 |
fix: render valid TOML when a command body contains backslashes (#3135)
render_toml_command() emitted the body inside a multiline *basic* TOML
string ("""..."""), which processes backslash escape sequences. A command
body containing a backslash — e.g. a Windows path like C:\Users\... whose
\U reads as an invalid unicode escape — therefore produced unparseable TOML
("Invalid hex value"), so the generated Gemini/Tabnine command file failed
to load. A body ending in a backslash also silently ate the closing newline
via TOML line-continuation.
Route bodies containing a backslash to the multiline *literal* form
('''...'''), which does not process escapes, or to the escaped basic string
when both triple-quote styles are present. Mirrors the escaping already done
by base.py's TomlIntegration.
Add tests covering a Windows path, a trailing backslash, and the
backslash + both-triple-quote-styles fallback.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Pascal THUET
|
8e76ff3d5c |
harden: reject shell=True in run_command (#3132)
run_command() forwarded shell= straight to subprocess.run, so a caller passing shell=True would invoke a shell. Reject shell=True with ValueError (keeping the parameter for signature compatibility) and drop shell= from both subprocess.run calls. Enable ruff S602/S604/S605 to flag any future shell=True reintroduction, annotate the one intentional workflow shell sink with # noqa: S602, and document the shell-step execution risk in workflows/PUBLISHING.md. |
||
|
Quratulain-bilal
|
0ef53eb91f |
fix(scripts): send check-prerequisites.ps1 errors to stderr (#3123)
* fix(scripts): send check-prerequisites.ps1 errors to stderr The validation errors and run-hints in check-prerequisites.ps1 were written with Write-Output, so they went to stdout. This script is usually run with -Json and its stdout parsed by the agent, so an error (e.g. missing plan.md) leaves the parser with an error string instead of JSON. The bash counterpart already writes these to stderr (>&2), as do the sibling PowerShell scripts (setup-tasks.ps1, common.ps1's Get-FeaturePathsEnv). Switch the six error/hint lines to [Console]::Error.WriteLine so stdout stays clean and the two shells match. * test(scripts): assert check-prerequisites errors stay on stderr Per the #3122 bug assessment, tighten the failure-path tests so they verify stdout stays clean (empty / valid JSON) and the error text only appears on stderr, instead of checking the combined stdout+stderr string. Covers all three PowerShell validation paths (missing feature dir, missing plan.md, missing tasks.md with -RequireTasks) and the bash counterpart. The two new error-routing tests fail on the pre-fix script (errors on stdout) and pass after it. |
||
|
JinHyuk Sung
|
0c975bbef7 |
fix: write Codex dev skills as files (#2988)
* fix: write Codex dev skills as files * fix: route codex dev symlink policy through metadata * fix: replace codex dev symlinks on refresh * fix: migrate codex dev skill symlinks * fix: avoid inactive shared skill dev symlinks * fix: preserve unrelated dev skill symlinks |
||
|
github-actions[bot]
|
45423d6bc6 |
[extension] Update Spec Kit Preview extension to v1.1.0 and sync Firebender agent lists (#3116)
* Update Spec Kit Preview extension to v1.1.0 Update preview extension submitted by @bigsmartben to: - extensions/catalog.community.json (version, name, description, download_url, commands, tags, updated_at) - docs/community/extensions.md community extensions table (name, description, alphabetical order) Closes #3109 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Sync issue templates with firebender integration Assisted-by: GitHub Copilot (model: GPT-5, autonomous) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com> |