* feat(extensions): add bundled bug triage workflow extension (#2870)
Add a bundled 'bug' extension providing a three-stage bug triage workflow:
- speckit.bug.assess: triage a bug report (pasted text or URL), locate
suspected code paths, and propose a remediation
- speckit.bug.fix: apply the proposed remediation and record what changed
- speckit.bug.test: validate the fix and record the verification result
Each bug gets its own directory under .specify/bugs/<slug>/ with one
Markdown report per stage (assessment.md, fix.md, test.md). The slug is
the only handle the three commands share; existing bug directories are
never overwritten.
Mirrors the layout of the existing bundled extensions (git, agent-context):
- extensions/bug/extension.yml, README.md, commands/
- extensions/catalog.json: register 'bug' (alphabetical, between
agent-context and git)
- pyproject.toml: add wheel mapping to specify_cli/core_pack/extensions/bug
Closes#2870
* address Copilot review on #2871
- speckit.bug.assess.md: drop POSIX-specific 'mkdir -p' example;
reword the prerequisite to describe the requirement (ensure BUG_DIR
exists) without assuming a specific shell.
- speckit.bug.fix.md: fix the slug-resolution fallback wording. It
listed '.specify/bugs/*/assessment.md' but then keyed off whether
'exactly one bug directory' existed; now it correctly keys off whether
exactly one matching 'assessment.md' was found and uses the slug from
its parent directory.
- tests/extensions/bug/test_bug_extension.py: add a smoke test analogous
to the agent-context extension's coverage. Validates the bundled
layout, catalog registration, '_locate_bundled_extension("bug")'
resolution, and that 'ExtensionManager.install_from_directory' installs
the three commands.
All 333 tests in tests/extensions/, tests/test_extensions.py, and
tests/test_extension_registration.py pass.
* address Copilot review on #2871 (round 2)
- Import _locate_bundled_extension from the public 'specify_cli'
package (it is re-exported in __init__.py) instead of the private
'specify_cli._assets' module, so the test does not depend on internal
module layout.
- Clarify module docstring: install_from_directory is called with
register_commands=False, so commands are copied and recorded in the
installed manifest but not registered with AI agents. Wording updated
to avoid implying otherwise.
* address Copilot review on #2871 (round 3)
- tests/extensions/bug/test_bug_extension.py: read extension.yml as
UTF-8 explicitly to avoid platform-dependent default encoding (notably
on Windows). Matches how the README is read in the same module.
- extensions/bug/commands/speckit.bug.assess.md: add a 'Safety When
Fetching URLs' section. Instructs the agent to treat fetched page
content as untrusted input (no obeying embedded prompt-injection
directives), forbids supplying credentials/secrets that a page asks
for, scopes the fetch to the URL the user provided (no following
redirects to other resources), and requires suspicious content to be
quoted verbatim under an 'Unverified' heading rather than acted on.
- extensions/catalog.json: bump 'updated_at' to today (2026-06-05) so
consumers that cache by this field invalidate when 'bug' is added.
- extensions/bug/README.md: minor grammar fix ('a reproduction that was
not actually performed').
All 251 tests in tests/extensions/bug/, tests/test_extensions.py, and
tests/test_extension_registration.py pass.
* speckit.bug.assess: add URL Trust Policy for fetched bug-report URLs
Builds on the 'Safety When Fetching URLs' section by adding a tiered
classification rule the agent applies before any fetch:
1. Refuse outright (no fetch, no prompt) for non-http(s) schemes,
loopback, link-local, RFC1918 private space, and known cloud
instance-metadata endpoints (169.254.169.254, metadata.google.internal,
100.100.100.200, metadata.azure.com). This closes the SSRF /
internal-recon vector opened by 'paste any URL'.
2. Fetch silently for an explicit allowlist of widely-used public
bug-report sources (github, gitlab, bitbucket, atlassian.net, linear,
stackoverflow/stackexchange, sentry). This preserves the paste-a-URL
ergonomics the workflow is built for.
3. Otherwise prompt once in interactive mode (default 'no', naming the
resolved host explicitly); in automated mode skip the fetch and
record '[UNVERIFIED - fetch skipped: host not on safe list: <host>]'
in assessment.md so a human can decide later.
In every case, assessment.md records the verbatim URL, the resolved host,
and which branch of the policy was taken (allowlisted /
confirmed-by-user / auto-refused: <reason>) so the per-bug directory's
audit trail is complete. Preflight HEAD probes are explicitly forbidden
since the probe itself is the request the policy gates.
Execution step 1 now defers to the policy before fetching.
* speckit.bug.assess: remove 'post-redirect-resolution' inconsistency
The URL Trust Policy explicitly forbids following redirects, but the
audit-trail bullet asked the agent to record the host
'post-redirect-resolution', which contradicted that rule and could lead
agents to follow redirects unintentionally to determine what to log.
Reword both call sites to refer to the host parsed from the URL the user
supplied (no resolution implied):
- Tier-3 interactive prompt: '...naming the host parsed from the URL
explicitly...'
- Recorded fields: 'The host parsed from that URL (no redirect following
- see the rule above).'
No behavior change; clarification only.
* fix: resolve GitHub release asset API URL for private repo preset and workflow downloads
- Add shared `resolve_github_release_asset_api_url` utility to `_github_http.py` for
reuse across preset and workflow download paths
- Apply the same private-repo fix from PR #2792 (extensions) to:
- `PresetCatalog.download_pack` — ZIP downloads via catalog `download_url`
- `preset add --from <url>` — ZIP downloads from a direct URL
- `workflow add <url>` — workflow YAML downloads from a direct URL
- `workflow add <id>` (catalog) — workflow YAML downloads via catalog `url`
- For browser release URLs (`github.com/…/releases/download/…`), the asset is
resolved via the GitHub REST API and downloaded with `Accept: application/octet-stream`
- Direct REST API asset URLs (`api.github.com/…/releases/assets/<id>`) are
downloaded directly with `Accept: application/octet-stream`
- Auth is preserved end-to-end through the existing `open_url` infrastructure
- Update `test_download_pack_sends_auth_header` and add
`test_download_pack_accepts_direct_github_rest_asset_url` to cover both paths
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: URL-encode tag in release API URL to handle special characters
Encode the tag as a path segment (using quote with safe='') when
building the releases/tags/<tag> API URL. This prevents malformed
URLs when tags contain reserved characters like '/' or '#'.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* test: add CLI-level tests for preset add --from GitHub release URL resolution
Adds regression tests covering:
- resolve_github_release_asset_api_url unit tests (passthrough, resolution,
network error, URL encoding of special chars in tags)
- CLI-level 'preset add --from <github-release-url>' end-to-end flow
- CLI-level 'preset add --from <api-asset-url>' direct passthrough
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor: deduplicate release URL resolution; fix test issues
- ExtensionCatalog._resolve_github_release_asset_api_url now delegates
to the shared helper in _github_http.py (also gains URL-encoding fix)
- Remove unused 'io' import from test_github_http.py
- Remove duplicate 'provides' dict keys accidentally added to test_presets.py
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: align resolver timeout with download timeout; add workflow CLI tests
- Pass timeout=30 to resolve_github_release_asset_api_url in both
workflow add paths so worst-case latency matches the download timeout
- Add CLI-level regression tests for 'workflow add <url>' covering
browser URL resolution and direct API asset URL passthrough
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: remove unused urllib.request import; add catalog workflow test
- Remove unused 'import urllib.request' in preset add --from path
- Add CLI test for catalog-based 'workflow add <id>' with GitHub
release URL resolution
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* style: remove unused MagicMock imports from tests
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Manfred Riem <mnriem@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(workflows): render gate show_file contents in the interactive prompt
The gate step read and recorded `show_file` but never displayed its
contents at the interactive prompt, so the operator approved/rejected
without seeing the referenced file. Render the file inside the prompt
when stdin is a TTY, with a graceful notice for missing/unreadable
files. Non-interactive PAUSED behaviour, exit codes, resume semantics,
and no-`show_file` output are unchanged.
Closes#2809.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(workflows): keep gate _prompt signature stable and harden show_file reads
The gate prompt rendered show_file by passing it as a third positional
argument to _prompt. A test that stubs _prompt with a two-argument lambda
(test_gate_abort_still_halts_with_continue_on_error) then failed once the
branch caught up to main, because the call site passed three arguments to
the two-argument stub.
Compose the show_file material into the displayed message in execute() and
keep _prompt to its (message, options) contract. Display data no longer
widens the interactive seam, so stubbing _prompt stays stable and future
review material can be added without breaking callers. _prompt now renders
a multi-line message inside the gate box.
Also catch ValueError in _read_show_file so a path the OS rejects outright
(e.g. an embedded NUL byte) degrades to a notice instead of crashing the
prompt, matching the helper's stated contract.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(workflows): coerce gate prompt message to str before rendering
The multi-line render loop split the message on newlines, which assumes a
str. A non-string message (e.g. a YAML numeric literal) previously rendered
fine through the old f-string and would now raise on .split. Coerce with
str() to preserve that tolerance, and add a regression test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(workflows): make gate stdin handling robust; tidy compose_prompt typing
Address review feedback on the gate tests and helper:
- Swap the gate module's sys.stdin for a fixed-isatty stub (shared
_StubStdin / _force_gate_stdin helpers) instead of setattr on
sys.stdin.isatty, which is not assignable under some pytest capture
modes. This also forces the non-interactive tests to a non-TTY so they
cannot block on input() when run in a real terminal.
- The non-interactive show_file test now hard-fails if _read_show_file is
called, proving the file is not read on the PAUSED path.
- _compose_prompt accepts a non-string message (e.g. a YAML numeric
literal) and always returns str via str(message), keeping its annotation
and docstring accurate; the redundant coercion in _prompt is removed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(workflows): strip control chars from gate show_file; default tests non-TTY
Address review feedback:
- _read_show_file strips C0 control characters (except tab) from each line,
so a show_file containing ANSI escape sequences (e.g. \x1b[2J) cannot
clear the screen or spoof the prompt/options when rendered to a terminal.
- Add an autouse fixture on TestGateStep that defaults every gate test to a
non-TTY stdin, so no test can drop into the interactive prompt and block
on input() when the suite runs under a real TTY. Interactive tests opt
back in via _force_gate_stdin(tty=True); the now-redundant explicit
non-TTY calls were removed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(workflows): localize gate stdin patch to the gate module's sys
_force_gate_stdin rebinds the gate module's `sys` name to a stand-in whose
stdin has a fixed isatty() and which delegates every other attribute to the
real sys, instead of mutating the process-wide sys.stdin. This keeps the
patch local to the gate module and leaves real stdin untouched. The gate
abort test, which used the same process-wide swap, now shares the helper, so
the pattern exists in exactly one place.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(workflows): sanitize the displayed gate show_file path, not just content
Control characters were stripped from show_file *contents* but the path was
still printed verbatim as the header (`f"{show_file}:"`) and echoed in the
read-error notice, so a show_file path containing ANSI escapes could still
inject terminal sequences. Centralize stripping in `_sanitize_for_display`
and apply it to every show_file-derived string that reaches the terminal —
the displayed path, each file line, and the error notice — while still
opening the file with the original path. Add a test for path sanitization.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(workflows): inline control-char stripping, drop the helper
Reuse the existing _CONTROL_CHARS regex directly at the three display sites
instead of wrapping it in a one-line helper.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(workflows): also strip LF and C1 controls from gate show_file display
The control-char class skipped LF (so an embedded newline in a show_file
path could break the boxed layout) and the C1 range (so \x9b CSI and other
8-bit controls survived). Widen the class to [\x00-\x08\x0a-\x1f\x7f-\x9f]
(still keeping tab).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat: add support for rovodev
* fixup! feat: add support for rovodev
* fixup! feat: add support for rovodev
* fixup! feat: add support for rovodev
* fixup! feat: add support for rovodev
* fixup! feat: add support for rovodev
* fixup! feat: add support for rovodev
* fixup! feat: add support for rovodev
* fixup! feat: add support for rovodev
* fixup! feat: add support for rovodev
* fixup! feat: add support for rovodev
* fixup! feat: add support for rovodev
* chore: bump version to 0.9.4
* chore: begin 0.9.5.dev0 development
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* feat(workflows): add --json output to workflow run, resume, and status
Adds an opt-in `--json` flag to `workflow run`, `workflow resume`, and
`workflow status` that emits a single machine-readable object (run_id,
workflow_id, status, current step; status also reports per-step states
and a runs list) for automation and external orchestrators.
JSON is written via a small `_emit_workflow_json` helper using plain
stdout, so Rich markup, highlighting, and line-wrapping can never alter
the emitted object. Default human-readable output and exit codes are
unchanged when `--json` is omitted. Reference docs updated.
Closes#2811.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(workflows): keep --json stdout clean while steps write output
Suppressing the banner and the step-start callback was not enough to
guarantee a single parseable JSON object on stdout: individual steps still
write there while the engine runs. The gate step prints its prompt, and the
prompt step runs a CLI subprocess that inherits the process's stdout file
descriptor — either can corrupt the JSON stream for interactive runs or
integration-backed workflows.
Wrap engine.execute()/engine.resume() in a file-descriptor-level redirect
(dup2) when --json is set, so both Python-level writes and inherited-fd
subprocess output go to stderr while stdout carries only the emitted JSON.
Step progress stays visible on stderr. status does not run the engine, so
it is unaffected.
Tests cover both pollution channels (a Python print and a real subprocess)
via fd-level capture, and the inactive no-op path. Docs note the
stdout/stderr split.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(workflows): fix stray escape sequence in --json redirect comments
The redirect helper's docstring and its test comment wrote ``print``\s,
which renders as "print\s" rather than "prints". Replace with plain
"prints".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extension command registration now resolves the active skills directory before writing command artifacts. This lets initialized skills-backed agents recover a missing active skills directory while preserving the existing preset registration behavior.
Add regression coverage for missing active skills directories, shared skills directories, and symlinked parent guards.
Fixes#2769.
Co-authored-by: OpenAI Codex <codex@openai.com>
* fix(cursor-agent): enable CLI dispatch via ``-p --trust`` headless mode
Restores the ability for ``specify workflow run`` to dispatch the
cursor-agent CLI, complementing the existing in-IDE skill flow.
Without this fix, ``specify workflow run speckit --input
integration=cursor-agent ...`` fails with a misleading
``CLI not found or not installed`` error even when the CLI is
installed (since cursor-agent had ``requires_cli=False`` and an
unset ``build_exec_args``).
The cursor-agent CLI (>= 2026.05.16) supports headless execution
via ``-p`` (print mode with full tool access including write/shell)
and ``--trust`` (bypass Workspace Trust prompt). Without ``--trust``
the CLI exits non-zero in non-TTY contexts (verified locally).
Changes to ``src/specify_cli/integrations/cursor_agent/__init__.py``:
* ``config.requires_cli``: ``False`` -> ``True``
* ``config.install_url``: ``None`` -> Cursor CLI docs URL
* Override ``build_exec_args()`` to emit
``[cursor-agent, -p, --trust, <prompt>, ...]``
with optional ``--model`` and ``--output-format json`` flags,
mirroring the shape used by ``claude``/``codex``/``gemini``.
Tests:
* 34 existing cursor-agent tests still pass.
* 6 new tests in ``TestCursorAgentCliDispatch`` pin
``requires_cli``, ``install_url``, and the exact argv shape
(default, text-output, with-model, and the hyphenated skill
invocation form ``/speckit-<name>``).
* Full repo: 1085 / 1085 passed, no regressions.
Fixes#2629
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(integrations): resolve ``.cmd``/``.bat`` shims before subprocess.run
On Windows, ``shutil.which`` honors ``PATHEXT`` and locates wrappers
like ``cursor-agent.cmd`` and ``codex.cmd``, but Python's
``subprocess.run`` calls ``CreateProcess`` which does **not** consult
``PATHEXT`` and therefore fails with ``WinError 2`` on a bare argv
like ``[cursor-agent, ...]``.
Resolve ``exec_args[0]`` via ``shutil.which`` in
``IntegrationBase.dispatch_command`` so ``.cmd``/``.bat`` shims work
transparently. On POSIX this is a no-op for absolute paths and a
harmless lookup otherwise.
Verified locally on Windows 10 + cursor-agent 2026.05.16:
without this fix, ``specify workflow run speckit --input
integration=cursor-agent`` fails with ``FileNotFoundError`` even
after the cursor-agent integration starts producing valid exec
args (per the prior commit on this branch).
Tests:
* New: 2 cursor-agent tests pin the shim-resolution + passthrough
behavior (``test_dispatch_command_resolves_cmd_shim_for_subprocess``
and ``test_dispatch_command_passthrough_when_shutil_which_finds_nothing``).
* Updated: ``tests/test_workflows.py::TestCommandStep::test_dispatch_with_mock_cli``
was mocking ``shutil.which`` only at the ``command`` step level
and not at the ``base`` level, which made it environment-sensitive
(fails locally when the real ``claude`` CLI is on PATH). Added the
matching base-level patch and updated the argv-assertion to reflect
the resolved path. ``test_dispatch_failure_returns_failed_status``
gets the same patch for consistency.
* Full repo: 2867 passed, 0 regression from this PR. The 12 remaining
pre-existing failures are unrelated Windows ``symlink`` privilege
failures (``WinError 1314``) on a non-admin Windows runner.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(cursor-agent): inject --approve-mcps --force for headless MCP/tool access
The previous commit (1c55988) wired up ``-p --trust`` so the CLI launches
in headless mode without the Workspace Trust prompt, but that alone is
not enough to let ``specify workflow run`` drive a real speckit feature
end-to-end with cursor-agent on Windows. Two more flags are required:
* ``--approve-mcps``: without it, every MCP server configured in
``.cursor/mcp.json`` stays ``not loaded (needs approval)``, and any
tool call against them is silently dropped. We hit this immediately
trying to read a DingTalk PRD from a remote MCP server during the
``/speckit-specify`` step.
* ``--force``: without it, the agent halts on the first tool-call
approval prompt (the tool call gets rejected and the workflow exits
non-zero with a misleading message). With ``--force`` cursor-agent
matches the implicit "trusted environment" semantics that ``claude -p``
and ``codex --exec`` already have by default -- which is the right
semantics for an unattended ``specify workflow run`` invocation.
Verified end-to-end on Windows 10 + cursor-agent 2026.05.16-0338208:
* ``cursor-agent -p --trust --approve-mcps --force --output-format text``
+ a ``/speckit-specify`` prompt that included a DingTalk URL produced
a full spec.md (31.5 KB) plus checklists/requirements.md in ~10.7 min,
reading the source PRD through the ``dingtalk-doc`` remote MCP server,
deciding the ``specs/`` subpath itself, and updating
``.specify/feature.json`` and ``specs/menu-dictionary.md`` along the
way -- no human-in-the-loop, no source PRD ever touched the filesystem.
* Without ``--approve-mcps`` the same prompt errors with the tool call
rejected message; without ``--force`` the agent stops at the first
non-MCP tool call.
Tests:
* ``test_build_exec_args_*`` updated to pin the new four-flag prefix.
* New ``test_build_exec_args_contains_mandatory_headless_flags`` asserts
the four flags are always present together.
* ``test_dispatch_command_resolves_cmd_shim_for_subprocess`` updated to
match the new argv layout.
* All 43 cursor-agent tests pass; no other tests touched.
Co-authored-by: Cursor <cursoragent@cursor.com>
* refactor(cursor-agent): express dispatch support via build_exec_args() instead of requires_cli
Co-authored-by: Cursor <cursoragent@cursor.com>
* test(cursor-agent): use urlparse hostname check and cover dispatch without requires_cli
Co-authored-by: Cursor <cursoragent@cursor.com>
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: 刘一 <liuyi@oureman.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Update Superpowers Implementation Bridge extension to v1.0.2
Update speckit-superpowers-bridge extension submitted by @lihan3238:
- extensions/catalog.community.json (version, download_url, updated_at)
The download URL now uses the stable latest-release alias
(speckit-superpowers-bridge.zip) per the maintainer's distribution policy.
Closes#2848
* Pin speckit-superpowers-bridge download_url to v1.0.2
Use the version-pinned release asset URL instead of the
releases/latest/download alias so the catalog entry tracks the
specific version declared in the entry rather than silently
following future releases. Matches the pinning convention used
by other entries in the catalog.
* docs(agents): add PR review response guidance to prevent comment flooding
Adds a 'Responding to PR Review Comments' section to AGENTS.md so agents
acting on PRs stop posting one reply per review comment. Directs them to
post one summary comment per review round, disclose their identity and
the human they're acting for, never click 'Resolve conversation', and
re-request review once per round rather than after every push.
Closes#2849
* 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>
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Initial plan
* feat: add --workflow option to init command for post-init workflow execution
* chore: remove unused import in test file
* refactor: allow workflow run without project when given a YAML file path
Instead of adding --workflow to init, make `specify workflow run ./file.yml`
work without requiring a .specify/ project directory. When the source is a
YAML file that exists on disk, cwd is used as the project root. When it's a
workflow ID, the .specify/ project requirement is preserved.
* Handle standalone workflow path edge cases
* Fix USERPROFILE env var portability and docs notation
* Fix workflow YAML path detection to require regular files
* Harden workflow run against unsafe .specify paths
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
* feat(extensions): add --force flag to extension add for overwrite reinstall
Add --force support to `specify extension add` that allows overwriting
an already-installed extension without manually removing it first.
- install_from_directory() and install_from_zip() accept force=True,
automatically calling remove() before installation
- The --force CLI flag works with all install modes (--dev, --from URL,
bundled, and catalog)
- Config files (*-config.yml) are preserved across force reinstall
- Error message suggests --force when extension is already installed
- 6 new tests covering unit and CLI force reinstall flows
* fix: address PR review feedback on --force implementation
- Remove unused `backup_config_dir` variable assignment (Ruff F841)
- Defer `remove()` until after `_validate_install_conflicts()` to prevent
data loss if validation fails mid-reinstall
- Use `TemporaryDirectory` instead of `NamedTemporaryFile` in ZIP test
to avoid Windows file-locking failures
* fix: only restore config backup when --force actually triggers a remove
When --force is used but the extension is not already installed, the
backup restore/cleanup should not run. Previously it could resurrect
stale config files from a previous removal and delete the backup
directory unnecessarily.
* fix: address Copilot review feedback on --force implementation
- Clear stale backup dir before remove() so only fresh backups are restored
- Restore only config files (*-config.yml, *-config.local.yml) from backup
- Remove trailing \n from --force console message (console.print adds newline)
* fix: handle non-directory paths in backup cleanup/restore
- Use is_dir() before rmtree/iterdir on backup path to avoid crashes
when .backup/<id> exists as a file or symlink
- Remove unused manifest1 variable in test_install_force_reinstall
* fix: handle symlinks in backup cleanup/restore and correct CLI message
- Check is_symlink() before is_dir() in backup cleanup and restore:
Path.is_dir() follows symlinks (returns True for symlink-to-dir) but
shutil.rmtree() raises OSError on symlinks. Handle symlinks by
unlinking them instead.
- Skip symlink entries during config file restore.
- Change --force dev-install message from "Reinstalling" to
"Installing [...] (will overwrite if already installed)" because
--force also works for first-time installs.
* chore: bump version to 0.9.3
* chore: begin 0.9.4.dev0 development
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Clear pre-existing lint debt flagged by repo-wide `ruff check` (the lint
config only scopes src/, so tests/ had drifted). No behavior change.
- F401/F541: drop unused imports and redundant f-string prefixes (autofix)
- E741: rename ambiguous `l` to `ln` in comprehensions
- E702: split semicolon-joined statements onto separate lines
- F841: drop unused bindings while keeping the side-effecting calls
(_minimal_feature, install_from_directory)
Full suite: 3344 passed, 40 skipped. ruff check (repo-wide): clean.
* fix(workflows): validate run_id in RunState.load before touching the filesystem
``RunState.load(run_id, project_root)`` interpolates ``run_id`` directly
into ``project_root / ".specify" / "workflows" / "runs" / run_id`` and
then calls ``state_path.exists()`` and ``json.load`` on the result. The
run_id is reachable from user input via ``specify workflow resume
<run_id>`` (CLI argument) and via ``SPECKIT_WORKFLOW_RUN_ID`` (env var
override on the engine's run path), so a value like ``../escape``
turns ``runs_dir`` into ``.specify/workflows/escape/`` and:
* ``state_path.exists()`` becomes a file-existence oracle for any
path the process can read.
* if a ``state.json`` exists at the traversed location (planted by
a malicious dependency, a misconfigured shared workspace, or an
older spec-kit version that happened to write there),
``json.load`` parses it and the workflow resumes under the
attacker-chosen ``workflow_id`` / step state.
* a subsequent ``state.save()`` then writes back to the traversed
location, persisting the corruption.
``RunState.__init__`` already validates ``run_id`` against
``r'^[a-zA-Z0-9][a-zA-Z0-9_-]*$'`` — but that check runs on
``state_data["run_id"]`` *after* ``load`` has already done the file
lookup, which is too late to prevent the disclosure.
This change extracts the pattern into a class-level constant
``_RUN_ID_PATTERN`` and a single ``_validate_run_id`` classmethod so
``__init__`` and ``load`` cannot drift, then calls the validator at the
top of ``load`` before any path is built. Mirrors the precedent in
``src/specify_cli/agents.py::_ensure_within_directory`` (used at line
437 of that file) which guards extension-install paths against the
same threat model.
Regression tests parametrize 9 traversal vectors (``../escape``,
``..``, ``../../etc/passwd``, ``foo/bar``, ``foo\bar``, ``.hidden``,
``-flag``, ``foo\x00bar``, empty) and plant a malicious ``state.json``
outside ``runs/`` so a missing guard would surface as a successful
load rather than the ambiguous ``FileNotFoundError``. A second test
asserts ``__init__`` and ``load`` reject the same representative
malformed ID, so future changes to one path can't silently drift from
the other.
* test(workflows): exercise RunState.load in shared-validation test, fix __init__ empty-string asymmetry
Copilot's review on this PR pointed out that
test_init_and_load_share_validation claimed to verify both entry
points share the same validation rules but never actually called
RunState.load — only __init__ and the shared
_validate_run_id helper. A regression in load (e.g. someone
deleting the cls._validate_run_id(run_id) call before the path is
built) would slip through even though __init__ and the helper
stayed aligned, defeating the whole point of the test.
Tightening the test surfaced a real asymmetry the previous version was
silently masking:
self.run_id = run_id or str(uuid.uuid4())[:8]
The truthiness fallback meant RunState(run_id="") silently
substituted a UUID and skipped validation, while
RunState.load("", project_root) correctly rejected the empty
string. The two entry points diverged on the empty-string vector.
That is exactly the drift the test name claimed to defend against —
and the original test missed it.
Changes
-------
* engine.py: __init__ now distinguishes run_id is None
(caller omitted it → auto-generate UUID) from an empty string
(caller provided it → must validate like any other value). Both
paths still flow through _validate_run_id, but only the
explicit-None case auto-generates.
* test_workflows.py: test_init_and_load_share_validation is
now parametrized over one representative vector per category from
test_load_rejects_path_traversal (parent traversal, embedded
separator, leading non-alphanumeric, empty string) and asserts that
*all three* entry points — __init__, _validate_run_id, and
load — reject the same input. Adding load to the assertion
is the substantive fix Copilot asked for; keeping __init__ and
the helper alongside it makes any future drift between the three
immediately observable instead of having to read three separate
tests.
Verification
------------
pytest tests/test_workflows.py — 168 passed (was 165 before the
parametrize expansion; __init__ empty-string vector would have
failed the new test against the old engine code, confirming the
asymmetry was real).
* feat(cli): implement specify self upgrade
* fix(cli): normalize self-upgrade prerelease tags
* fix(cli): tighten self-upgrade diagnostics
* fix(cli): harden self-upgrade verification parsing
* fix(cli): sanitize self-check fallback tags
* fix(cli): harden self-check release display
* fix(cli): validate resolved upgrade tags
* fix(cli): tolerate invalid install metadata
* test(cli): align upgrade network mocks
* fix(cli): respect relative installer paths
* fix(cli): tighten upgrade failure handling
* fix(cli): align installer path diagnostics
* fix(cli): validate release and version output
* fix(cli): clarify source checkout guidance
* fix(cli): harden upgrade detection helpers
* fix(cli): avoid echoing invalid release tags
* fix(cli): tolerate argv path resolve failures
* chore: remove self-upgrade formatting-only diffs
* fix: address self-upgrade review feedback
* fix: address self-upgrade review followups
* fix: address self-upgrade review edge cases
* fix: address self-upgrade review docs
* fix: refine self-upgrade review followups
* fix: address self-upgrade review cleanup
* fix: handle self-upgrade review edge cases
* fix: address self-upgrade review nits
* fix: address follow-up self-upgrade review
* fix: resolve self-upgrade review and Windows CI failures
- README: promote "Optional Commands" to ### so it is a sibling of
"Core Commands" under "Available Slash Commands" (consistent heading
levels; avoids the h2->h4 jump a revert would create).
- _version: allow --tag prerelease/dev and build-metadata suffixes to
compose (e.g. v1.0.0-rc1+build.42), matching PEP 440 / semver; the
Version() check still enforces canonical validity.
- tests: compare resolved argv0 as Path objects instead of POSIX strings
so the assertion holds on Windows; skip the relative-installer-path
executable-bit tests on Windows via a new requires_posix marker (they
rely on chmod/X_OK semantics and chdir-into-tmp teardown that do not
hold there). Add a combined prerelease+build-metadata tag test.
* fix: address second self-upgrade review round
- self_check: clarify that the "up to date" branch is reached only for
parseable latest tags (the unparseable case returns earlier), so the
InvalidVersion fallback assumption is not reintroduced.
- self_upgrade: compare target/current as Version instances directly
instead of re-parsing the canonical strings through _is_newer; the
empty-current case stays explicit via the not-None guard.
- tests: document the intentional broad GH_/GITHUB_ env scrub with a test
asserting non-credential context vars (GH_HOST, GITHUB_REPOSITORY, …) are
stripped from the installer subprocess env — a deliberate fail-safe that
also catches credential-adjacent names without a recognized suffix.
* fix: address third self-upgrade review round
- self_upgrade: unify the no-op short-circuits on packaging Version
equality instead of canonical-string equality. Version("1.0") equals
Version("1.0.0") but their str() forms differ, so the old check could
misreport an equal install as "already on latest release or newer".
Both the unpinned and pinned branches now use Version comparison.
- self_upgrade: compare the verified version as a parsed Version against
the target so a non-version verifier result is a mismatch (exit 2)
rather than a coincidental canonical-string match.
- resolver: map HTTP 429 (Too Many Requests / secondary rate limit) to
the rate-limited category so users get the same actionable token hint
as 403.
- _is_github_credential_env_key: document the precise (intentionally
broad) scrub matching contract in the docstring.
- tests: add a trailing-zero Version-equality regression test and a
parametrized HTTP-status categorization test (429 -> rate limited;
404/502 -> verbatim).
* fix: address fourth self-upgrade review round
- self_upgrade: label a pinned target older than the installed version as
"Downgrading" rather than "Upgrading" so `--tag <older>` is not mistaken
for a forward upgrade.
- resolver: drop the unused `typing.Optional` import and annotate the
`--tag` option as `str | None`, consistent with the rest of the module
(verified Typer resolves it on the supported Python versions).
- _is_github_credential_env_key: add `_PASSWORD` and `_CREDENTIALS` to the
recognized credential suffixes and document that only these shapes are
scrubbed (not blanket coverage).
- tests: assert the precise exit code (1) for the re-raised transient
OSError path; skip the InvalidMetadataError test on Pythons where the
real exception is absent instead of fabricating it; update the pinned
downgrade test to expect the "Downgrading" label.
* fix: accept uppercase V prefix in --tag
Fold a leading uppercase `V` (a common paste) to the canonical lowercase
`v` before validating `--tag`. The remainder of the tag stays
case-sensitive on purpose: the validated value is used verbatim as a git
ref, which is case-sensitive on GitHub, so rewriting label/build-metadata
casing could point at a tag that does not exist. Adds a normalization test.
`workflow resume` now accepts `--input key=value` (the same flag and
parsing as `workflow run`, via a shared `_parse_input_values` helper).
Supplied values are merged over the run's persisted inputs and
re-resolved through the existing typed-validation path
(`_resolve_inputs`), so a resumed/re-run step sees the updated inputs
and ill-typed values fail fast. Keys not supplied keep their persisted
values; resuming without `--input` is unchanged. Reference docs updated.
Distinct from #2405 (file-reference inputs at run time): this is about
supplying inputs at resume time, reusing the existing input model.
Closes#2812.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
On Windows, when stdout/stderr are not a UTF-8 TTY (output piped, redirected
to a file, or running under a legacy code page such as cp1252), Rich cannot
encode the banner and box-drawing glyphs, so the CLI aborts with a
UnicodeEncodeError traceback instead of printing. This breaks basic commands
like `specify --help` and `specify version` whenever their output is captured
rather than written to an interactive terminal.
Reconfigure sys.stdout/sys.stderr to UTF-8 with errors="replace" at the
main() entry point on win32 so output degrades gracefully instead of crashing.
The change is a no-op on POSIX, is guarded by try/except so it can never make
stream setup worse, and lives at the CLI entry point only -- importing
specify_cli as a library does not touch global streams.
Verified on Windows 11 (cp1252): `specify --help` piped and `specify version`
redirected to a file both render correctly and exit 0 without setting
PYTHONUTF8 / PYTHONIOENCODING.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* chore: bump version to 0.9.2
* chore: begin 0.9.3.dev0 development
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* fix: resolve GitHub release asset API URL for private repo downloads
For private or SSO-protected GitHub repos, browser release download URLs
redirect to HTML/SSO instead of the ZIP asset. This commit resolves the
asset via the GitHub REST API and downloads with Accept: application/octet-stream,
falling back to the original URL if the API call fails.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: support direct GitHub REST release asset URLs in extension downloads
When a catalog download_url is already a GitHub REST release asset URL
(https://api.github.com/repos/<owner>/<repo>/releases/assets/<id>),
skip the release metadata lookup and download directly with
Accept: application/octet-stream. This complements the browser URL
resolution from the previous commit, covering catalogs that reference
the REST API directly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
VS Code Copilot Agent Skills do not support the `mode:` frontmatter field.
The generated SKILL.md files included `mode: speckit.<stem>` injected by
CopilotIntegration.post_process_skill_content(), which had no effect in
VS Code and could cause confusion. Simplify post_process_skill_content to
delegate directly to _CopilotSkillsHelper without injecting mode:.
Update tests to assert mode: is absent from generated skill frontmatter.
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(integrations): co-locate integration commands in integrations/ domain dir
- Remove commands/ stubs (handlers will live in domain dirs)
- Move all integration CLI handlers out of __init__.py into integrations/
- Split into focused modules under integrations/:
_helpers.py (340 lines) — domain helpers
_install_commands.py (306 lines) — install / uninstall
_migrate_commands.py (487 lines) — switch / upgrade
_query_commands.py (442 lines) — list / use / search / info / catalog
_commands.py (34 lines) — app objects + register()
- __init__.py reduced by ~1400 lines; integration block replaced with register() call
- Fix patch paths in tests to new module locations
* fix(integrations): restore original integration list output in refactor
Preserve the CLI Required column, post-table default/installed summary,
and no-installed guidance that were dropped during the no-behavior-change
refactor of integration list into _query_commands.py.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix(integrations): restore _clear/_update_init_options public imports
The refactor that split integration commands moved
_clear_init_options_for_integration and _update_init_options_for_integration
into integrations/_helpers.py, but tests still import them from the top-level
specify_cli package, causing ImportError. Re-export them with explicit aliases
at the end of __init__.py to preserve the public import surface.
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* feat(workflows): add continue_on_error step field
Adds an optional `continue_on_error: bool` field on every step.
When set to `true` and the step fails, the engine records the
result (`exit_code`, `stderr` on `steps.<id>.output` plus `status`
as a sibling key on `steps.<id>`) and continues to the next sibling
step instead of halting the run. Downstream `if`, `switch`, or
`gate` steps can then branch on
`{{ steps.<id>.output.exit_code }}` to route the recovery path.
Engine details
--------------
`WorkflowEngine._execute_steps` now consults the step config when a
step returns `StepStatus.FAILED`:
- Gate aborts (`output.aborted`) always halt the run — operator
decisions take precedence over the flag.
- Otherwise, if `continue_on_error` is the literal `True`, log a
`step_continue_on_error` event and proceed to the next sibling.
The runtime check uses identity comparison (`is True`) rather
than truthiness, so truthy non-bool values like the string
`"true"` cannot silently change run semantics even if a caller
bypasses `validate_workflow()`.
- Otherwise, behave as before: log `step_failed`, set
`RunStatus.FAILED`, and return.
Validation
----------
`_validate_steps` rejects non-bool values for `continue_on_error`.
Coerced strings like `"true"` are not accepted so authoring
mistakes surface at validation time rather than silently changing
run semantics.
Tests
-----
`TestContinueOnError` in `tests/test_workflows.py` (8 tests):
- `test_undeclared_failure_halts_run` — default halt behaviour.
- `test_declared_and_fired_continues_run` — flag + fail → continue.
- `test_declared_but_step_succeeded_is_noop` — flag + success → no-op.
- `test_if_branch_routes_around_failure` — end-to-end recovery.
- `test_gate_abort_still_halts_with_continue_on_error` — abort
always halts.
- `test_validation_rejects_non_bool_continue_on_error` — `"true"`
rejected at validation.
- `test_validation_accepts_bool_continue_on_error` — `true`/`false`
pass cleanly.
- `test_engine_ignores_truthy_non_bool_continue_on_error` —
defense-in-depth: engine ignores string `"true"` even when
validation is bypassed.
Rebased onto current upstream/main (post #2664 merge); the new
`TestContinueOnError` class sits immediately after upstream's
`TestContextRunId` so the two feature suites coexist cleanly.
Closes#2591.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(workflows): restore runtime context section, clarify gate prompt
Two Copilot findings on d0b9e00:
1. The `### Runtime Context` documentation for `{{ context.* }}` was
lost during the rebase onto current main (the squash dropped the
anchor where #2664 had added it). Restored under `## Expressions`
so users can find `context.run_id` semantics and examples.
2. The continue_on_error example gate had message "Retry or skip?"
but used the default `options: [approve, reject]` with `on_reject:
skip`, which implied an automatic retry path that gates do not
provide. Reworded the message to match the actual approve/reject
semantics and added an explicit note that retry requires either
custom gate options + downstream branching or a wrapper loop.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(workflows): clarify continue_on_error scope — returned FAILED only
Copilot finding on d0b9e00:
The README's "Error Handling" intro implied `continue_on_error` covers
"any other runtime error raised during step execution", but the engine
only consults the flag when a step returns `StepResult(status=FAILED, ...)`.
Exceptions raised out of `step_impl.execute()` propagate to
`WorkflowEngine.execute()`, where the catch-all logs `workflow_failed`
and re-raises — the step result is never recorded, and the flag is
never consulted.
Audited the whole PR diff for the same overclaim:
1. workflows/README.md — main fix. Reworded the Error Handling intro to
"any step that returns StepResult(status=FAILED, ...)" and promoted
the parenthetical structural-validation note into the Notes block.
Added a new "Scope: returned failures only" note that names the
exception path explicitly and tells step authors how to bring the
flag into scope for exceptional code (catch internally and return
FAILED with the failure encoded in `output`).
2. tests/test_workflows.py — section comment used "when an executable
step fails", same ambiguity. Tightened to "when a step returns
StepResult(status=FAILED, ...)" and added a sentence calling out
that unhandled exceptions are out of scope.
3. src/specify_cli/workflows/engine.py — already correct ("any step
that returns FAILED" in the validator comment; "lets the pipeline
route around the failure" in the execute path). No change.
Engine semantics and test bodies are unchanged. Docs-only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(workflows): clarify on_reject:skip semantics — engine returns COMPLETED, not auto-skip
Copilot finding on b8982a7:
The README example's gate message said "reject to skip the rest of this
branch", and the explanatory paragraph claimed [approve, reject] map
to "continue" vs "skip the rest of this branch". The engine does not
implement automatic branch-skipping. `on_reject: skip` returns
`StepStatus.COMPLETED` (gate/__init__.py:65-66); the next sibling step
runs unconditionally unless the author wires a downstream `if` reading
`{{ steps.<gate-id>.output.choice }}`.
Two fixes:
1. Restructured the YAML example so it actually demonstrates the
manual-branching pattern: added a `recover` if-step after the gate
that conditions on `steps.review.output.choice == 'approve'`. Now
the example shows the real workflow author's responsibility instead
of implying the engine does it.
2. Replaced the trailing paragraph with three precise notes:
- both gate options return COMPLETED; `on_reject: skip` controls
abort behaviour only, not sibling-skipping
- all three `on_reject` values enumerated with their actual engine
semantics (FAILED+aborted / COMPLETED / PAUSED)
- the original retry-loop guidance retained as the third bullet
Updated the gate message in the example to match — "reject to leave the
failure recorded and move on" instead of "reject to skip the rest of
this branch".
Audited the whole PR diff for the same overclaim: no other instance.
Engine semantics, validation, and test bodies are unchanged. Docs-only.
161/161 tests/test_workflows.py pass locally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(workflows): clarify gate's role — surfaces, doesn't programmatically branch
Audit follow-up to 393ac6b — three sites repeated the same minor
overclaim about gates being one of the "branch on it" step types
alongside `if` and `switch`:
1. workflows/README.md (the "downstream `if`, `switch`, or `gate`
steps can branch on it" sentence introducing the example)
2. engine.py:236 (validator inline comment)
3. engine.py:657 (execute-path inline comment)
A `gate` step does not have a `condition` or `expression` field — it
only evaluates expressions for `message` and `show_file` (gate/__init__.py:29,36).
Programmatic branching happens in `if`/`switch`; a gate surfaces the
value to a human operator via message interpolation, and the operator's
choice is recorded in `output.choice` for a *subsequent* `if`/`switch`
to route on.
Reworded all three sites consistently: "a downstream `if` or `switch`
can branch on it (or a `gate` can surface it to the operator via
message interpolation)". The README example already demonstrates this
distinction — the gate carries `{{ }}` template variables in its
message and the `recover` if-step downstream is what actually branches
on the choice.
Engine semantics, validation, and test bodies are unchanged. Docs-only
on the README; comment-only on engine.py.
161/161 tests/test_workflows.py pass locally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* docs(workflows): use qualified StepStatus.* instead of bare FAILED/COMPLETED/PAUSED
Three Copilot inline comments on workflows/README.md lines 226, 282, 288
flagged that ``StepResult(status=FAILED, ...)`` is not valid Python —
``StepResult.status`` is a ``StepStatus`` enum value, so the
documented form should be ``StepStatus.FAILED``.
Audited the whole PR diff for the same shorthand. The bare unqualified
form appears in three files added/modified by this PR:
1. workflows/README.md (6 sites) — three ``StepResult(status=FAILED, ...)``
parentheticals, plus the on_reject Notes bullet listing the three
step statuses (``FAILED``, ``COMPLETED``, ``PAUSED``).
2. tests/test_workflows.py (4 sites) — section header for
TestContinueOnError, two test-method docstrings, one inline comment
about a gate's TTY-fallback behaviour.
3. src/specify_cli/workflows/engine.py (1 site) — the validator inline
comment added in d0b9e00 said "returns FAILED" where the engine
code itself uses ``StepStatus.FAILED``.
All 11 sites normalised to the qualified ``StepStatus.<name>`` form so
the docs / test docstrings / inline comments match what readers will
actually find in the engine code and the tests. Engine semantics,
validation, and test bodies are unchanged.
161/161 tests/test_workflows.py pass locally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(shared-infra): record skipped files in speckit.manifest.json
`install_shared_infra` skipped files that already existed on disk
when `force=False`, but the skip branches in both the scripts loop
and the templates loop only appended to `skipped_files` without
calling `manifest.record_existing`. So when the function ran with a
fresh manifest against an already-populated `.specify/` tree (e.g.
after the manifest was deleted, corrupted, or extracted out of band),
every file went down the skip path, `planned_copies` /
`planned_templates` stayed empty, and `manifest.save()` wrote an
empty `files` field — leaving the integration believing nothing was
installed.
Record every skipped file in the manifest, but only when it is not
already tracked. This preserves the original hash for files that
were previously recorded so `check_modified()` (used by
`integration use` to decide whether a user has customized a
template) keeps working correctly.
Add `TestSpeckitManifestRecordsSkippedFiles` in
`tests/integrations/test_integration_claude.py` covering both the
fresh-skip path and the recover-after-lost-manifest path.
Fixes#2107
* fix(shared-infra): guard manifest.record_existing against non-file dst
Address Copilot review feedback on PR #2483. The previous fix called
``manifest.record_existing(rel_skip)`` from the skip branch of both
loops in ``install_shared_infra``, which would crash with
``IsADirectoryError`` (or another ``OSError``) if a directory or other
non-regular-file happened to exist at the expected destination path —
since ``record_existing`` opens the file to compute its SHA-256.
Three coordinated fixes:
1. ``IntegrationManifest.record_existing`` now validates its
precondition: it raises ``ValueError`` if the path is a symlink or
is not a regular file. The docstring already promised "an
already-existing file"; this enforces it. The symlink check runs on
the un-resolved path because ``_validate_rel_path`` calls
``resolve()``, which would silently follow the symlink. Mirrors the
existing ``_ensure_safe_manifest_destination`` precedent in the
same module.
2. In ``install_shared_infra``'s scripts and templates skip branches,
guard the ``record_existing`` call with ``dst.is_file()`` and wrap
it in ``try/except (OSError, ValueError)``. A directory collision,
permission error, or TOCTOU race no longer aborts the whole
install — the user gets a per-path warning, the path still
surfaces in ``skipped_files``, and the rest of the install
continues.
3. ``_read_manifest_files`` in the regression test no longer falls
back to ``data.get("_files")`` (Copilot's low-confidence finding):
the silent fallback could mask a schema regression where the
public ``files`` key is renamed. It now asserts ``"files" in data``
and that the value is a dict.
Add two regression tests in ``TestSpeckitManifestRecordsSkippedFiles``
covering the directory-at-destination edge case for both the scripts
loop and the templates loop. Both verify (a) install does not crash,
(b) the non-file path is not recorded in the manifest, and (c) the
path still surfaces in the user-visible warning.
The "shared infrastructure file(s)" warning text is changed to
"path(s)" so it remains accurate when non-file entries appear in the
list.
Refs #2107
* fix(manifest): lexical pre-check for record_existing + add error-case tests
Address Copilot review (2026-05-11, review id 4266902103):
1. `record_existing` was calling `(self.project_root / rel).is_symlink()`
BEFORE validating containment. For absolute paths or paths containing
`..`, this performed a filesystem stat outside the project root before
`_validate_rel_path()` raised. Add a cheap lexical pre-check that
delegates to `_validate_rel_path()` for the canonical error messages,
so the symlink stat only ever runs on paths that are already lexically
inside the project root.
2. Add focused unit tests in `tests/integrations/test_manifest.py` for
the symlink and non-regular-file error paths, including:
- symlink target rejection
- dangling symlink rejection (caught by the symlink guard before
the is_file check)
- directory path rejection (is_file == False)
- missing-path rejection (is_file == False)
- absolute-path lexical pre-check
The Copilot reviewer noted these guards had no focused coverage in
`test_manifest.py`, only via the `test_integration_claude.py`
regression test.
3. The third Copilot finding (repeated `dict(self._files)` copies via
`manifest.files` in the skip branches) is already resolved on this
branch by using `prior_hashes` — the function-scope snapshot taken at
the top of `install_shared_infra` — for the membership check, instead
of `manifest.files`.
AI disclosure: drafted with assistance from Claude (Opus 4.7).
* fix(manifest): track recovered files separately + symlink-ancestor + canonical-path guards
Address Copilot review id 4309888722 (2026-05-18) on PR #2483:
1. Recovery semantics (shared_infra.py:371, 412) — install_shared_infra
now passes ``recovered=True`` when re-recording a skipped existing
file. This flag funnels into a new ``recovered_files`` array in the
manifest JSON, so a future ``refresh_managed`` run can distinguish
"hash I produced" from "hash I observed on a file that may be a user
customization" and avoid silent overwrite without ``--refresh-shared-infra``.
Schema is purely additive: ``files: dict[str, str]`` is unchanged; the
new ``recovered_files: list[str]`` is omitted when empty.
2. Symlinked ancestor (manifest.py:172) — ``record_existing`` now walks
every component of the rel path and rejects any symlinked ancestor,
not just a symlinked leaf. Catches ``linked_dir/file.txt`` where
``linked_dir`` is a symlink, which previously slipped past the leaf-only
``is_symlink()`` check and was resolved through by ``_validate_rel_path``.
Mirrors the component-walk pattern in ``_ensure_safe_manifest_directory``.
3. Misleading "escapes project root" message (manifest.py:168) — paths
like ``dir/../file.txt`` normalize inside the project, so the old
message lied about what was wrong. New message: "Manifest paths must
be canonical; '..' segments are not allowed". Still rejects (canonical
keys are required so ``check_modified``/``uninstall`` cannot key the
same file under two paths).
Tests: 7 new test methods across TestManifestRecoveredFiles and
TestRecordExistingNewGuards covering all 4 Copilot findings. Full suite
passes locally.
🤖 AI disclosure: drafted with assistance from Claude (Opus 4.7).
* fix(manifest): normalize is_recovered input through _validate_rel_path
Address Copilot review comment id 4309888722 round-5 (2026-05-21) on PR #2483:
``is_recovered()`` previously checked ``self._recovered_files`` membership
with bare ``Path(rel).as_posix()``, while ``record_existing()`` stores keys
via ``_validate_rel_path(rel, root).relative_to(root).as_posix()``. The two
normalizations disagreed on absolute paths and paths that escape the
project root — ``is_recovered`` would silently return False for inputs that
``record_existing`` would have refused entirely.
The fix routes ``is_recovered`` through the same ``_validate_rel_path``
pipeline; ``ValueError`` from the validator is caught and converted to
False so query semantics stay exception-free (Python ``__contains__``
convention).
Tests: 2 new methods in ``TestManifestRecoveredFiles``:
- ``test_is_recovered_absolute_path_returns_false``
- ``test_is_recovered_escaping_path_returns_false``
🤖 AI disclosure: drafted with assistance from Claude (Opus 4.7).
* fix(manifest): clear recovered marker on managed re-record + reject '..' in is_recovered
Address Copilot Round-7 review comments on PR #2483:
1. record_existing(recovered=False) and record_file now BOTH discard the
path from _recovered_files. The marker is meant to flag "we observed
this file but cannot vouch it's a managed baseline" — once the same
path is re-recorded as managed (either explicitly or by writing fresh
bytes), the marker is stale and must clear so refresh_managed and
future is_recovered queries return the truthful answer.
2. is_recovered now applies the same canonical-key guard as record_existing
(rejects absolute paths and '..' segments lexically before delegating
to _validate_rel_path). Such paths can never be stored keys, so the
query correctly returns False without depending on _validate_rel_path
semantics that diverged from record_existing's stricter contract.
record_file docstring updated to mention the side-effect on recovered
markers.
Tests: 3 new methods in TestManifestRecoveredFiles covering
record_existing(false) clearing, record_file clearing, and is_recovered
dotdot rejection.
* test(manifest): update is_recovered comments to reflect Round-7 lexical guard
Round 8 — addresses Copilot review comment on tests/integrations/test_manifest.py:362.
After Round-7 (1dbf0c2), is_recovered() rejects absolute paths and '..' segments
up front via a lexical guard, returning False without calling _validate_rel_path
at all. The test comments still described the prior "_validate_rel_path raises;
we catch" code path, which is misleading for readers.
Updated comments in both:
- test_is_recovered_absolute_path_returns_false (Copilot's exact target)
- test_is_recovered_escaping_path_returns_false (same comment-class issue;
fixed preemptively to avoid a Round-9 finding on the same drift)
Pure documentation change. Test assertions and behavior unchanged; all manifest
tests still green.
* fix(manifest): document OS errors on record_existing + filter orphan recovered_files on load
Round 9 — addresses Copilot review on PR #2483:
1. record_existing's docstring now documents OSError/PermissionError as
possible raises (in addition to ValueError) — the implementation has
always been able to raise them from is_symlink, is_file, or the
file-read used to hash, but the contract did not reflect that.
Callers should be prepared for both surfaces.
2. load() now filters recovered_files entries that don't correspond to
keys in files. An externally-edited or partially-corrupted manifest
can deserialize with orphan recovered paths; rather than reject the
whole manifest (too strict on the upgrade path), we drop the orphans
and let the inconsistency self-correct on the next save(). is_recovered
then returns the truthful False for the orphan.
Tests: new test_load_filters_recovered_files_not_in_files asserting an
orphan recovered entry is dropped on load.
* chore: bump version to 0.9.1
* chore: begin 0.9.2.dev0 development
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* fix(cli): pin UTF-8 encoding on init-options and .extensionignore I/O
``Path.read_text`` / ``Path.write_text`` default to the system locale
codec, which is cp1252 / gb2312 / cp932 on Windows. Two user-facing
file paths in spec-kit were calling them without an explicit
``encoding=`` argument:
- ``src/specify_cli/__init__.py:400,412`` —
``save_init_options`` / ``load_init_options`` for
``.specify/init-options.json``. A peer machine with a different
default locale (or a UTF-8 Unix CI runner reading a file written on
a cp1252 Windows host) cannot decode the file, raising
``UnicodeDecodeError``. ``UnicodeDecodeError`` is a subclass of
``ValueError`` — not ``OSError`` / ``json.JSONDecodeError`` — so
the existing fall-back ``except`` tuple in ``load_init_options``
also misses it and the error propagates raw to the CLI.
- ``src/specify_cli/extensions.py:764`` — ``.extensionignore``
pattern reader. The very next line already normalises
backslashes "so Windows-authored files work", proving the codebase
expects Windows authors to write this file. Multibyte UTF-8
patterns (Chinese filenames, accented directory names) silently
mojibake when the host locale is not UTF-8, so the patterns fail
to match and unintended files are shipped with the extension.
The sibling integration-catalog reader at
``src/specify_cli/integrations/catalog.py:150,156,193,202,374``
already pins ``encoding="utf-8"`` everywhere. PR #2280 fixed the
symmetric PowerShell-template BOM bug. This change brings the two
remaining drifted paths in line with that precedent.
Regression tests:
- ``tests/test_presets.py::TestInitOptions`` — parametrized non-ASCII
round-trip (CJK, Latin-1, Greek, emoji) plus a corrupted-file case
that asserts the existing "fall back to {}" contract still holds
when a peer file contains bytes invalid as UTF-8.
- ``tests/test_extensions.py::TestExtensionIgnore`` — Japanese
(``ドキュメント/``) and Latin-1 (``café/``) ignore patterns
correctly exclude their directories during install.
* fix(cli): wrap .extensionignore decode error and tighten UTF-8 contract
Addresses Copilot review feedback on this PR.
Three issues, three fixes:
1. ``save_init_options`` now writes JSON with ``ensure_ascii=False``.
Without that flag, ``json.dumps`` emits ASCII-only ``\uXXXX``
escapes, which means the ``encoding="utf-8"`` pin on the
surrounding ``Path.write_text`` makes no observable difference for
any value we currently write. Flipping ``ensure_ascii`` makes the
non-ASCII bytes hit the file directly, so the encoding pin becomes
the thing that decides between cp1252 garbage and clean UTF-8 on
Windows. The comment above the call now describes the real reason
instead of the previously-misleading rationale Copilot flagged.
2. ``test_save_load_round_trip_preserves_non_ascii`` was a no-op under
the old ``ensure_ascii=True`` writer (Copilot's second comment).
Added ``test_save_writes_real_utf8_bytes`` that asserts the on-disk
bytes contain the UTF-8 encoding of ``café`` (``0xC3 0xA9``), not
its JSON escape form ``é``. Removing either
``ensure_ascii=False`` or ``encoding="utf-8"`` from the writer now
breaks this test — the contract is pinned.
3. ``.extensionignore`` reader wraps ``UnicodeDecodeError`` as
``ValidationError`` with a pointer to the offending byte
(Copilot's third comment). Mirrors
``ExtensionManifest._load_yaml``'s existing handler for
``extension.yml``. Adds
``test_extensionignore_invalid_utf8_raises_validation_error``
asserting installation aborts with the wrapped error instead of a
raw Python traceback.
The Hermes Agent integration ships in the CLI (src/specify_cli/integrations/hermes/)
and is registered in the catalog, but the supported-agents table in the
integrations reference omitted it. Add the row so the docs match the shipped
integration.
TestClineIntegration._expected_files() overrides the base-class version but
was not updated when the bundled agent-context extension files were added to
test_integration_base_markdown.py, causing test_complete_file_inventory_sh
and test_complete_file_inventory_ps to fail.
Fixes#2796
* Add spec-kit-linear extension to community catalog
Add linear extension submitted by @ashbrener to:\n- extensions/catalog.community.json\n- docs/community/extensions.md\n\nCloses #2778
* Address PR review feedback for spec-kit-linear entry
- Use Unicode arrow (→) in catalog/docs description\n- Move docs row to alphabetical Spec section
* Address follow-up review naming/order feedback
- Use human-friendly display name: Linear Integration\n- Move docs row to alphabetical L section
* chore: bump version to 0.9.0
* chore: begin 0.9.1.dev0 development
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* docs: add May 2026 newsletter
Publish the May 2026 newsletter documenting project milestones including:
- Crossing 100K GitHub stars and top-100 GitHub project status
- 100+ community extensions in catalog
- Fourteen releases (v0.8.4–v0.8.17)
- Multi-agent install support and constitution governance features
- Open Source Friday livestream and media coverage across 25+ languages
- Industry analyst recognition
* 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>
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix: move URL install confirmation prompt before spinner (#2783)
The typer.confirm() prompt inside console.status() was overwritten by
Rich's spinner animation, making extension add --from <url> appear hung.
Move URL validation and the default-deny confirmation prompt before the
spinner block so the user can see and respond to the [y/N] prompt.
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* fix: guard prompt with not dev, escape from_url in Rich markup
Address PR review feedback:
- Gate URL confirmation prompt on 'not dev' so --dev + --from does not
show a confusing prompt for a URL path that will be ignored.
- Escape from_url with rich.markup.escape() in both the warning panel
and the download message to prevent markup injection via crafted URLs.
* fix: remove unused import, reuse safe_url, add regression tests
Address second round of PR review:
- Remove unused urllib.request import from URL install path
- Remove redundant re-import of rich.markup.escape; reuse safe_url
computed before the spinner for download and error messages
- Add test_add_from_url_prompts_before_spinner: asserts typer.confirm
fires before console.status spinner to prevent #2783 regression
- Add test_add_from_url_cancel_exits_cleanly: asserts declining the
prompt exits with code 0 and prints Cancelled
---------
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
