* fix(workflows): harden catalog.py against mis-shaped registry & non-string fields
Two robustness gaps where WorkflowRegistry/WorkflowCatalog diverged from
their StepRegistry/StepCatalog siblings, which already guard both:
- WorkflowRegistry._load returned json.load() verbatim, so a JSON-valid
but mis-shaped registry (a list root, or a dict lacking a 'workflows'
mapping) made is_installed/get/list/remove/add crash with
TypeError/KeyError. Mirror StepRegistry._load: validate the shape and
reset to default, and widen the except tuple to OSError/UnicodeError.
- WorkflowCatalog.search joined name/description/id without coercion, so a
null or non-string field raised TypeError. Coerce with str(... or '')
exactly as StepCatalog.search does.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(workflows): tighten mis-shaped-registry assertions
Per review: WorkflowRegistry.list() always returns a dict, so assert
'== {}' directly (the previous '== {} or == []' called list() twice and
admitted a shape it never returns), and reference
WorkflowRegistry.SCHEMA_VERSION instead of hard-coding '1.0'.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(bundle): resolve file:// download_url via the file-URL helper
_download_manifest built the local path from raw parsed.path, which
keeps the leading slash of file:///C:/x (yielding a \C:\x path that
never exists on Windows) and skips percent-decoding (my%20bundles stays
encoded on every OS) — so a catalog entry whose download_url is the
canonical URI Python itself produces via Path.as_uri() always fails
with 'Bundle manifest not found'. Route the file scheme through the
existing bundler.services.adapters._file_url_to_path helper, which
already handles drive letters, UNC hosts, and percent-decoding for
catalog file:// URLs (make_catalog_fetcher). The bare-path branch is
unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(bundle): reject file:// / local download_url; catalog URLs are HTTPS-only
Per maintainer review (route B): file:// in a catalog download_url was
never intended — catalog URLs are HTTPS-only (http for localhost) across
the extensions/presets/workflows catalog systems, and disk installs go
through the positional path (specify bundle install <path>), handled by
_local_manifest_source before catalog resolution. Remove the
file:///bare-path branch from _download_manifest and route everything
through _download_remote_manifest (HTTPS-only via _require_https), with an
actionable error pointing at the positional install. Invert the file://
tests to assert rejection (+ a positional-path resolution test), and
migrate the three bundle-info contract tests off local download_urls onto
an HTTPS-only entry with a mocked manifest fetch.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(bundle): validate HTTPS before the offline gate in _download_manifest
Per review: for a non-local download_url the offline check ran before any
URL validation, so an invalid/non-HTTPS scheme surfaced a misleading
'Network access disabled' error under --offline when the real problem is
the URL would be rejected even online. Call _require_https before the
offline gate so the correct error is reported in every mode.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(bundle): reword non-HTTPS download_url error to not mislabel scheme-less URLs
A scheme-less download_url (urlparse scheme == "") can be a bare
filesystem path OR a missing-scheme value like 'example.com/foo.zip',
not necessarily file://. Reword the reject error to state the real
HTTPS-only constraint and enumerate what is rejected (file://, local
path, scheme-less), instead of labeling every case 'local/file://'.
Behavior unchanged; the 'bundle install' actionable hint is preserved,
so the existing reject-path tests still pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(extensions): handle prefix-colliding env vars in _get_env_config
_get_env_config built the nested dict with 'if part not in current:
current[part] = {}' and an unconditional leaf assignment. Two env vars
that collide on a prefix — e.g. SPECKIT_X_CONNECTION and
SPECKIT_X_CONNECTION_URL — then either crash (scalar processed first:
the walk indexes into a str -> TypeError 'str object does not support
item assignment') or silently clobber the nested dict (scalar processed
last). Via should_execute_hook's blanket except, the crash silently
disables every config-based hook for the extension. Guard the walk and
the leaf assignment with isinstance checks so a colliding scalar yields
to the nested dict; result is order-independent
({'connection': {'url': ...}} either way), matching _merge_configs'
dict-preserving semantics.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(extensions): assert via public should_execute_hook, not private helper
Per review: the colliding-env hook test described should_execute_hook
swallowing the TypeError, but asserted on the private _evaluate_condition.
Assert on the public should_execute_hook instead — it returns False
(silently disabled) before the fix and True after, matching the
real-world failure mode and not coupling to a private helper.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(extensions): ignore malformed env var names in _get_env_config
Per review: a name like SPECKIT_<EXT>_ (no key) or with consecutive
underscores produced empty path components, creating surprising entries
under an empty key (env_config[''] = ...). Filter out empty parts and
skip the variable entirely when nothing remains.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(agent-context): discover nested plan.md in scoped layouts (#3024)
The agent-context updater only looked for plan.md one level deep
(specs/*/plan.md), so scoped layouts created via SPECIFY_FEATURE_DIRECTORY
(specs/<scope>/<feature>/plan.md) were never picked up and no plan
reference was written into the context file.
Recurse into specs/ in both the bash (rglob) and PowerShell (-Recurse)
scripts. In the PowerShell script, also replace
[System.IO.Path]::GetRelativePath, which is .NET Core 2.1+ only and throws
under Windows PowerShell 5.1 (.NET Framework); the exception was swallowed
by the surrounding try/catch, leaving the plan path empty on 5.1 even when
a plan was found. Compute the project-relative path by stripping the root
prefix instead.
Add regression tests for both scripts covering nested discovery.
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(agent-context): guard mtime plan discovery against symlink escape
Address Copilot review feedback on #3301:
- bash updater: the mtime fallback filtered candidates lexically via
relative_to() on the *unresolved* path, so a plan reached through a
specs/ symlink pointing outside the project could be selected and emit
an in-project-looking path. Resolve each candidate and keep only those
whose resolved path stays under root before picking the newest.
- test: the nested-plan PowerShell regression targets a Windows
PowerShell 5.1 (.NET Framework) failure mode, but ran whatever
POWERSHELL resolved to (prefers pwsh). Prefer powershell.exe on Windows
so the 5.1-only compat fix is actually exercised.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(agent-context): note recursive plan.md discovery in update command
Auto-detection now recurses (`specs/**/plan.md`) to support nested scoped layouts created via SPECIFY_FEATURE_DIRECTORY (#3024). The update command doc still described the old one-level `specs/*/plan.md` glob, which could mislead users troubleshooting plan detection. Addresses Copilot review feedback on PR #3301.
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>
find_entries_for_url did (urlparse(url).hostname or "").lower() unguarded. a
malformed authority (e.g. an unterminated ipv6 bracket "https://[::1") makes
urlparse/hostname raise ValueError, so instead of the empty list the function
already returns for a host-less url, a raw ValueError leaked out of the shared
http client (build_request / open_url call this before any url validation).
no auth entry can match such a url, so treat it like the host-less case and
return no matches. added a regression test over an unterminated bracket and a
bracketed non-ip host; confirmed it fails on the pre-fix code.
CatalogStackBase._validate_catalog_url read parsed.hostname, which raises
ValueError on a malformed authority (e.g. an unclosed ipv6 bracket
"https://[::1"). every other reject path raises the class catalog error, so
the raw ValueError leaked to the caller. wrap the parse + hostname access and
convert ValueError to the normal error via cls._error.
twin of the bundler fix in adapters._validate_remote_url.
adapters._validate_remote_url reads parsed.hostname, which raises ValueError
on a malformed authority (e.g. an unclosed ipv6 bracket https://[::1). the
function's contract is to raise BundlerError for any bad url - every other
reject path does - so the raw ValueError leaked to the caller and crashed the
fetch instead of failing cleanly. wrap the parse and convert to BundlerError.
bundler sibling of #3369, which fixed the cli extension/preset/workflow add
paths but not this validator. added a regression test that fails pre-fix.
* fix(workflows): validate scalar types before string operations in workflow validation
YAML parses unquoted scalars like version: 1.0 and id: 123 as
float/int, which crashed validate_workflow and workflow add with raw
tracebacks. Type-check id, name, version and step ids before regex
and string operations so these surface as validation errors. Accept
an unquoted schema_version: 1.0 instead of printing a self-identical
rejection message.
Fixes#3420
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(workflows): treat falsey non-strings as type errors, not missing fields
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(workflows): only accept schema_version 1.0 so the error message is accurate
The check also accepted "1" while the error said Expected '1.0'.
Unquoted YAML 1.0 still works via str(); plain 1 is now rejected with
the message that matches.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`-Number` defaults to 0, so the previous `-eq 0` / `-ne 0` checks could
not distinguish an unset flag from an explicit `-Number 0`: a user
requesting branch `000-...` was silently routed into auto-detection.
Switch both checks to `$PSBoundParameters.ContainsKey('Number')`, which
tests whether the flag was actually supplied — mirroring the bash twin's
empty-string sentinel (`[ -z "$BRANCH_NUMBER" ]` / `[ -n ... ]`).
Add a parity regression test to both TestCreateFeatureBash and
TestCreateFeaturePowerShell asserting `--number 0` / `-Number 0` yields
`000-zero`.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_template_renders_python_invocation monkeypatches shutil.which to
return /usr/bin/python3, but on Windows resolve_python_interpreter guards
the which() result with a real _interpreter_runs subprocess probe (#3304).
The mocked /usr/bin/python3 path does not exist on a Windows runner, so the
probe fails, the resolver falls back to sys.executable (a ...python.exe
path), and the python3-anchored regex assertion fails.
Patch _interpreter_runs to return True in the _pin_interpreter fixture so
the resolved interpreter token stays python3 across all platforms, keeping
the #3304 production guard intact while making the assertion deterministic.
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* feat(workflows): make shell step timeout configurable
The shell step hardcoded a 300s subprocess timeout, killing any
legitimate long-running QA command. Read an optional timeout field
(seconds, positive integer, default 300) and validate it.
Fixes#3327
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* style: multi-line timeout validation, assert status in default test
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Guard against unvalidated timeout in ShellStep.execute()
The engine does not auto-validate step config, so a string or null
timeout would reach subprocess.run() and crash the run with a
TypeError. Fall back to the 300s default for malformed values,
mirroring how the engine treats unvalidated continue_on_error.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: find plans in nested spec directories
The agent-context mtime fallback used a one-level specs/*/plan.md
glob, so scoped layouts (specs/<scope>/<feature>/plan.md via
SPECIFY_FEATURE_DIRECTORY) never matched and the SPECKIT block was
written without a plan path. Recurse in both script variants and
update the command doc wording.
Fixes#3024
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor: pick newest plan with max(), align doc wording
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat(templates): add py: lines to command templates' scripts frontmatter
Every templates/commands/*.md with a scripts: block now declares a py:
variant so --script py renders a Python invocation via the existing
interpreter-prefixing in process_template.
Fixes#3283
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: install scripts/python for the py script type
install_shared_infra mapped every non-sh script type to powershell, so
--script py rendered invocations pointing at files that were never
installed. Map py to the python variant dir and skip __pycache__
artifacts during the copy.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* test: read templates with explicit utf-8 encoding
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: keep py: lines standalone, enforce script existence in tests
Drop the plan/tasks py: lines that referenced scripts shipping in the
core port (#3280); they move to that PR. Tests now assert every py:
line points at a script the repo ships, so a dangling reference can
never merge green.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(integrations): skip Windows Store python3 alias stub in resolve_python_interpreter
On stock Windows, python3 on PATH is the Microsoft Store App Execution
Alias stub: it exists but only prints an installer hint and exits
non-zero, so generated {SCRIPT} invocations for the py script type were
broken. Verify the found interpreter actually runs before accepting it,
on Windows only, mirroring the parse-success-not-availability approach
of #3312/#3320 for the sh scripts. POSIX keeps the plain existence
check. sys.executable remains the fallback and is always live.
Fixes#3383
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(integrations): probe interpreter isolated and without site, discard I/O
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* test: pin POSIX platform in PATH-resolution tests
The tests fake shutil.which with POSIX paths; on Windows CI the real
sys.platform made the stub probe run against those fake paths and
fall through to sys.executable.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
yaml.safe_dump with default_style='"' replaces the hand-rolled quote
helpers in base.py and hermes, so newlines and control characters in
template descriptions round-trip instead of producing unparseable YAML.
Fixes#3391
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(workflows): apply chained expression filters left-to-right
The pipe-filter parser in `_evaluate_simple_expression` split the
expression only at the *first* top-level `|` and treated the whole
remainder as a single filter. So a filter chain like
`{{ inputs.rows | map('name') | join(', ') }}` handed
`map('name') | join(', ')` to one filter, where the `(\w+)\((.+)\)`
regex mangled it and raised `ValueError`.
This broke the canonical use of `map`: it returns a list, and `join`
is the only filter that renders a list to a string, so the two are
meant to be chained. Chaining was impossible for every registered
filter.
Split the pipe segments at the top level (quote/bracket aware, so a
literal `|` inside a quoted argument like `join(' | ')` is preserved)
and fold each filter over the running value. The single-filter logic
is extracted verbatim into `_apply_filter`, so all existing strict
handling (`from_json` arity, unsupported-form vs unknown-filter
messages) is unchanged and now applies to every link in the chain.
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>
* fix(scripts): resolve invoke_separator by parse success, add awk fallback (#3304)
`get_invoke_separator` in common.sh selected its JSON parser by tool
*availability* (`command -v python3`) rather than parse *success*, and had
no text fallback after the python3 branch. On stock Windows + Git Bash —
no jq, and `python3` resolving to the Microsoft Store App Execution Alias
stub that passes `command -v` but exits 49 at runtime — it silently fell
back to "." even for `-`-separator integrations (e.g. forge, cline). The
observable result was wrong command hints in error messages, such as
`/speckit.plan` instead of `/speckit-plan`, in check-prerequisites.sh and
setup-tasks.sh.
This is the same existence-vs-runtime pattern fixed for the feature.json
parser in #3304's primary report; the reporter explicitly asked that other
python3 call sites be checked. The two remaining sites (resolve_template,
resolve_template_content) already fall through on failure and are unaffected.
- Restructure the jq -> python3 chain to fall through on parse failure,
gated on a `parsed` success flag rather than exclusive elif branches.
- Make the python3 branch signal failure (sys.exit(1)) instead of printing
"." so a stub failure falls through instead of being accepted.
- Add an awk text fallback (portable, no gawk-only whole-file slurp) that
reads the active integration key and its invoke_separator, handling both
pretty-printed (the written form) and compact JSON. Malformed input
safely defaults to ".".
Regression test simulates the broken-stub environment (jq + python3 stubs
that exit 49 on PATH) and asserts the `-` separator is still recovered.
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(scripts): close awk END block so invoke_separator fallback works
The awk text fallback in get_invoke_separator was missing the closing brace for its END block, so awk aborted with a syntax error on every invocation and the separator silently stayed at the default '.'. On environments with neither jq nor a working python3 (stock Windows + Git Bash, the exact case this fallback exists for) a '-'-separator integration like forge produced a wrong command hint. The bug escaped CI because the test that exercises it is gated on working bash and skips on the Windows dev shell.
Also harden the fallback per review: use the portable '[-.]' character class (a leading '-' inside '[]' can be read as an ill-defined range on some awk builds), and correct the test docstring, which said 'with no jq' though the helper installs a present-but-failing jq stub. Addresses Copilot review feedback on PR #3320.
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>
* fix(shared-infra): refresh_shared_templates preserves recovered user files
refresh_shared_templates skipped a shared template only when it was
untracked or modified, ignoring the manifest's is_recovered marker that
install_shared_infra already honors. So a pre-existing user template
(adopted via record_existing(recovered=True), hence tracked and
hash-unmodified) was silently overwritten with bundled content on
refresh — the exact data-loss class that #2918 fixed for
install_shared_infra. Add the is_recovered check to the skip predicate.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(shared-infra): include recovered files in refresh skip warning
The skip predicate now also skips recovered (pre-existing user) files,
so the warning saying only 'modified or untracked' could mislead a user
into thinking they edited a file that was simply recorded as recovered.
Reword to 'modified, untracked, or preserved (recovered)'.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(agents): resolve skill placeholders in Goose (yaml) command output
CommandRegistrar.register_commands resolves {SCRIPT}/__AGENT__ and the
$ARGUMENTS placeholder in the markdown and toml branches, but the yaml
branch (Goose recipes) called render_yaml_command directly, skipping
both. So extension/preset command bodies installed for Goose kept literal
{SCRIPT}, __AGENT__, and repo-relative script paths in the generated
.goose/recipes/*.yaml prompt. Mirror the markdown/toml branches: run
resolve_skill_placeholders + _convert_argument_placeholder on the body
before render_yaml_command.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(goose): assert positive placeholder replacements in recipe prompt
Per review: parse the generated recipe with yaml.safe_load and assert the
prompt contains the resolved values (.specify/scripts/, 'agent goose',
{{args}}), not merely that the literal tokens are absent — a wrong output
that happens to omit the exact strings would otherwise pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(bundler): enforce version pin on bundled preset/extension installs
The bundled install branch of _PresetKindManager/_ExtensionKindManager
called install_from_directory and returned before _assert_pinned_version,
so a bundle manifest pinning e.g. 2.0.0 would silently install the
bundled asset's own version (1.0.0) — the pin was only enforced on the
catalog path. _WorkflowKindManager already enforces it unconditionally.
Read the bundled asset's declared version from its manifest (best-effort;
None => cannot enforce, matching the catalog 'advertises no version'
escape hatch) and call _assert_pinned_version before install, in both
bundled branches.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(bundler): address review on bundled version-pin check
- Make _assert_pinned_version's error source-agnostic ('resolved version'
/ 'the source') so bundled preset.yml/extension.yml mismatches read
correctly, not just catalog ones.
- Type-guard _bundled_manifest_version: only a non-empty string version is
usable; missing/non-string/whitespace -> None ('cannot enforce').
- Add bundled-preset success-path test (matching pin + version=None both
proceed to install_from_directory), mirroring the extension test.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* test: isolate integration test home
Assisted-by: Codex (model: GPT-5, autonomous)
* test: assert integration home isolation
Assisted-by: Codex (model: GPT-5, autonomous)
* test: extend integration home isolation to module-scoped setup
Address Copilot review on #3144.
Add a session-scoped autouse fixture so HOME/USERPROFILE/XDG are redirected
for setup that runs outside a test function (e.g. the module-scoped status_*
fixtures in test_integration_subcommand.py that run `specify init` before any
per-test isolation applies). The function-scoped fixture still overrides HOME
per test.
Also assert Path.home() resolves to the isolated home, since most integrations
(Hermes, catalog) read the home via that API rather than the env vars directly.
* feat(extensions): port update-agent-context to Python
Ports the agent-context extension updater to a single Python script,
per #3281 and the check-prerequisites PoC pattern from #3302. The bash
version already ran its core logic through embedded Python heredocs, so
the port lifts that logic into a standalone script. Parity tests run
bash and Python side by side and compare output and resulting
context-file bytes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(extensions): match bash case-insensitivity on MSYS, test unparseable config gate
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(scripts): fall through to grep/sed when python3 is a broken stub in feature.json parser
read_feature_json_feature_directory picked its json parser by availability
(if jq / elif python3 / else grep-sed). on windows `python3` usually resolves
to the microsoft store app execution alias stub: it satisfies `command -v` but
fails at runtime (exit 49). the elif selected it, the runtime failure was
swallowed to _fd='', and the grep/sed last resort was never reached - so a
valid .specify/feature.json read as empty and every setup-plan / setup-tasks /
check-prerequisites call errored with "Feature directory not found" right after
a successful `specify init --script sh`.
change selection from availability to parse success: try jq, then python3 only
if still empty, then grep/sed only if still empty. a parser that exists but
produces nothing now falls through instead of terminating the chain.
the write path (_persist_feature_json) already uses jq-or-printf with no
python3, so it was unaffected; only the read path needed this.
add a regression test that puts a python3 stub (exit 49, like the store alias)
first on PATH and asserts setup-plan.sh still resolves the feature via the
grep/sed fallback.
fixes#3304
* test: shadow jq so the broken-python3 fallback test actually exercises it
the test claimed it dropped jq so the parser chain would reach python3 and
then grep/sed, but it only prepended the python3 stub dir to PATH. on a
runner with jq installed, read_feature_json_feature_directory parses via jq
and never reaches the fallback the test is meant to cover.
add a failing jq stub alongside the python3 stub so the chain is forced
through jq -> python3 -> grep/sed regardless of what the runner has installed.
* fix(toml): escape control characters so generated command files parse
both toml renderers (TomlIntegration._render_toml_string for gemini/tabnine
and CommandRegistrar.render_toml_command for extension/preset commands) wrote
control characters raw into a multiline or basic string. toml forbids literal
control chars (U+0000-U+001F except tab/newline, and U+007F) in every string
form, and a bare CR that is not part of a CRLF pair, so a description or body
containing one produced a .toml file that fails to parse.
route any value with such a character to a fully-escaped basic string that
emits the leftover control chars as \uXXXX. added regression tests that
round-trip through tomllib.
* refactor(toml): centralize control-char escaping in one shared helper
the control-char detection and basic-string escaping added for both toml
renderers were copy-pasted into agents.py and integrations/base.py. move the
two functions into specify_cli/_toml_string.py and have both renderers
delegate to it, so the escaping rules can't drift apart later.
no behavior change; both renderers now reference the same implementation.
* fix(cli): exit cleanly on malformed IPv6 URLs in extension/preset/workflow add
extension add --from, preset add --from, and workflow add <url> parsed
the user-supplied URL with a bare urlparse before their HTTPS/host
validation, so an unclosed IPv6 bracket escaped as a raw ValueError
traceback. Wrap each parse and emit the surrounding validation's clean
error style + typer.Exit(1) instead.
Fixes#3368
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(cli): convert malformed redirect URLs to URLError in shared redirect handler
Parse the redirect target once in _StripAuthOnRedirect.redirect_request
before the validator and stdlib handler run, converting ValueError into
URLError which every download path already catches. Also escape from_url
in the preset install message so IPv6 brackets don't break Rich markup.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
resolve_github_release_asset_api_url's is_ghes branch built the authority
with 'parsed.port', which raises ValueError on a malformed port (e.g.
host:notaport). The function's contract is to resolve or return None,
never raise — every other unresolvable case returns None. An allowlisted
GHES host with a bad port therefore crashed the caller. Read parsed.port
defensively and return None on ValueError.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
manifest.py::_sha256 does an unguarded open(). check_modified() and
uninstall() both call it on a readable-but-unopenable regular file
(e.g. permission denied) without catching OSError, so
'specify integration upgrade/uninstall/switch' surface a raw
PermissionError traceback. Guard both call sites: in check_modified()
treat an unreadable file as modified (consistent with the adjacent
symlink / non-regular-file handling); in uninstall() treat it as skipped
and preserve it (mirroring the existing path.unlink() OSError guard just
below). The force short-circuit is unchanged.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
On refresh (bundle update), install_bundle iterated only the new plan's
components, so a component the previous version owned but the new one no
longer ships was left installed on disk while being dropped from the
rewritten record (contributed only holds plan.components). With no
record referencing it, remove_bundle could never clean it up —
permanently orphaned, violating the provenance invariant (FR-022). After
the component loop, when refresh and a prior record exists, uninstall
each previously-owned component absent from the new plan — unless another
bundle still needs it (components_still_needed refcount, mirroring
remove_bundle), in which case it stays installed and is simply
de-attributed. Runs inside the existing try so a failed removal takes the
same no-record-written rollback path.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
workflow run/resume --json is contracted to emit a single JSON object on
stdout, but every error path (workflow-not-found, invalid workflow,
validation failure, execute/resume failure, and the shared
_parse_input_values invalid-input error) used console.print, landing the
human error text on stdout and corrupting the machine-readable stream.
Route those messages through err_console when --json is set (a no-op for
normal text mode), mirroring the stderr-only error routing already used
by 'specify bundle' (_fail) and err_console elsewhere in this module.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
FanInStep.validate() only checked wait_for, so a non-mapping 'output'
(a list or scalar) validated clean; execute() then silently coerces it
to {}, so the author's declared aggregation keys vanish with COMPLETED
status and no diagnostic. Reject a non-mapping output at validation,
mirroring the command-step (#3262) non-mapping fix. execute()'s
defensive coercion is left in place for unvalidated callers.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ShellStep.validate() only checked that 'run' was present, so run: (null)
or a GitHub-Actions-style list validated clean; execute() then
str()-coerces the value and invokes it under shell=True, literally
running 'None' or "['echo', 'hi']" as a command. Add a type check after
the presence check, mirroring the command-step (#3262) and gate options
validation. Expression strings ('{{ ... }}') are strings, so they stay
valid.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
AgyIntegration.build_exec_args returned [exe, '--print', prompt] without
calling _apply_extra_args_env_var(), so the documented per-integration
extra-args env hook was silently dropped for agy — same class as the
cursor-agent fix#3265. Append the hook after the positional prompt,
matching the devin integration's shape. agy still ignores model/output
as before.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* feat(integrations): add post_process_command_content() hook for all format types
Add post_process_command_content(self, content: str) -> str to IntegrationBase
with a no-op default. Wire it into register_commands() for non-skills format
types (Markdown, TOML, YAML) after format rendering, before writing to disk.
Also applies to aliases rendered via the inject_name path (cline, forge).
Skills-format agents are excluded to preserve the existing
post_process_skill_content() path and avoid double-processing.
This gives extension authors a clean per-agent content transformation seam
for all 21 non-skills integrations that previously had no post-processing hook.
Ref: #3303
Assisted-By: 🤖 Claude Code
* fix: initialize _integration before conditional branch
Prevents potential UnboundLocalError if the non-skills guard is
refactored without updating the alias path reference.
Assisted-By: 🤖 Claude Code
* fix(bundler): validate catalog URLs in `catalog add` (HTTPS-only, require host)
add_source persisted remote catalog URLs without the HTTPS/host checks
that specify_cli.catalogs (#3210) and the bundler adapters (#3333)
enforce, and an unclosed IPv6 bracket escaped as a raw ValueError.
Mirror the catalogs.py validation for http(s) schemes and wrap urlparse
so malformed input raises BundlerError.
Fixes#3366
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs: correct config filename and validation reference in comment
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
HermesIntegration.build_exec_args routed argv[0] through
_resolve_executable() but never called _apply_extra_args_env_var(), so
the documented per-integration extra-args env hook was silently dropped
for hermes — the same class of bug fixed for cursor-agent in #3265.
Insert the hook after the base 'chat -Q' command and before Spec Kit's
canonical -m/--json/-s/-q flags (mirrors opencode), so operator args
can't displace or clobber the canonical flags.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
ConfigManager._load_yaml_config returned yaml.safe_load(...) or {}, which
only guards falsy roots — a truthy non-mapping root (a YAML list or
scalar) flows straight into _merge_configs, whose .items() raises
AttributeError. get_config()/has_value()/get_value() then crash, and via
should_execute_hook's blanket 'except Exception: return False' every
config-based hook condition for that extension is silently disabled.
Coerce a non-dict root to {}, mirroring the existing non-dict-root guard
in get_project_config(). Hardens all three call sites in one place.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
the goose recipe renderer emitted the prompt body under a bare '|' block
scalar. yaml infers a plain block scalar's indentation from its first
non-empty line, so a command body whose first line is itself indented (a
markdown code block, a nested list item) made the parser expect that deeper
indent for the whole block and reject the later, shallower lines - the
generated .goose recipe then failed to parse.
use an explicit '|2' indentation indicator so the block is always read at
2 spaces regardless of the body. added a regression test that round-trips
an indented-first-line body through the yaml parser.
`validate` accepts a reject option case-insensitively
(`o.lower() in {"reject", "abort"}`), so a gate authored as
`options: [Approve, Reject]` passes validation. But `execute`
compared the echoed choice case-sensitively, so picking `Reject`
fell through to the approval path and silently ran downstream
steps instead of aborting.
Lower-case `choice` before the reject comparison so the runtime
agrees with the validation that let the option through.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_validate_remote_url in bundler/services/adapters.py guarded on parsed.netloc,
which is truthy for host-less URLs like "https://:8080" or "https://user@" even
though they carry no host. so those passed the "must be a valid URL with a host"
check. its docstring says it mirrors specify_cli.catalogs validation, but that
site was already fixed to use hostname in #3210/#3227 and this twin was missed.
switch to parsed.hostname (None for host-less URLs), matching catalogs.py. this
guard runs before any network call, so it is a pre-flight safety check.
add parametrized regression tests for the host-less forms plus a valid
host+port sanity case.
CatalogStack.search() claimed a bundle id in `seen` only when the entry matched
the query. so when the highest-precedence entry for an id did NOT match, a
lower-precedence entry with the same id could match and be returned instead --
even though resolve()/install always use the highest-precedence entry. search
advertised a bundle (name, version, source) the user could never actually get,
contradicting the method's own docstring ("resolved at its highest-precedence
source").
resolve every id to its highest-precedence entry first, then filter the
resolved set by the query. search now agrees with resolve(): a query that only
a shadowed lower-precedence copy matches returns nothing.
add a regression test covering the shadowed-entry case.
_safe_compare coerced both operands to int/float unconditionally for <, >, <=,
>=. any non-numeric string (an iso date, a version tag, a name) failed that
coercion and the whole comparison silently returned False -- so
`{{ inputs.d < '2026-02-01' }}` was False even when the date was earlier.
only coerce when both operands look numeric; otherwise compare the original
values, so two strings order lexicographically the way python does and two
numeric strings still compare as numbers ("10" > "9"). a number vs a
non-numeric string stays incomparable and yields False.
add a regression test covering dates, plain strings, numeric strings, and the
number-vs-string case.
* fix(workflows): quote-aware interpolation so a literal }} in a filter arg doesn't break multi-expression templates
#3208/#3228 hardened the single-expression fast path (_is_single_expression)
so a literal {{ or }} inside a string argument like `| default('}}')` stays on
the typed path. the multi-expression interpolation path was left on the old
_EXPR_PATTERN regex, whose non-greedy `(.+?)}}` body stops at the first }}
regardless of quoting. so a multi-expression template with a literal }} in any
block captured a truncated body, hit the filter parser malformed, and raised
ValueError.
e.g. `{{ inputs.name }}: {{ inputs.missing | default('}}') }}` raised instead
of interpolating.
replace _EXPR_PATTERN.sub with _interpolate_expressions, which scans each block
for a }} outside string literals - the same quote handling _is_single_expression
already uses. plain-value passthrough (a literal }} in a resolved value, not an
expression) is unchanged.
add regression tests for a literal }} in the second block and in the first
block, plus a literal {{ guard.
* fix(workflows): surface malformed templates in interpolation instead of emitting verbatim
address copilot review on #3307: when the quote-aware scan finds no block-closing
`}}` (e.g. an unbalanced quote in a filter arg swallowed the delimiter), fall back
to the first raw `}}` in the tail and evaluate it, so the parser raises ValueError
just as the old _EXPR_PATTERN.sub path did. only when there is no `}}` at all is
the tail left verbatim (a genuinely unterminated `{{`, which the regex also could
not match). keeps a typo failing loudly rather than being silently hidden.
add a regression test for an unbalanced quote in a multi-expression template.
* fix(integrations): cursor-agent ignores executable/extra-args env overrides
cursor-agent's build_exec_args() hardcoded self.key as argv[0] and never
called _apply_extra_args_env_var(), so the documented
SPECKIT_INTEGRATION_CURSOR_AGENT_EXECUTABLE (issue #2596) and
SPECKIT_INTEGRATION_CURSOR_AGENT_EXTRA_ARGS (issue #2595) hooks were
silently dropped — unlike every other CLI-dispatch integration (codex,
devin). Route argv[0] through _resolve_executable() and apply the
extra-args hook after the mandatory headless flags, mirroring the twins.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(integrations): pin extra-args insertion order for cursor-agent
Per Copilot feedback: the extra-args override test only asserted the
injected tokens were present, not that they land before Spec Kit's
canonical --model / --output-format flags. Exercise build_exec_args with
both a model and JSON output and assert the extra args are inserted
before --model / --output-format (and the canonical flags stay intact and
paired). Verified this fails if the _apply_extra_args_env_var call is
moved after the flag extends.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* feat(cli): add `py` script type & Python interpreter resolution (#3278)
Introduce a third script variant alongside `sh`/`ps` as the foundation
for unifying workflow scripts under a single Python implementation.
- Add `"py": "Python"` to `SCRIPT_TYPE_CHOICES`; `VALID_SCRIPT_TYPES`
consumers (init workflow step, init command, _helpers) pick it up
automatically since they derive from that mapping.
- Add `IntegrationBase.resolve_python_interpreter()` (project venv →
`python3` → `python`, falling back to `python3`).
- Prefix the resolved interpreter when `process_template()` expands
`{SCRIPT}` for the `py` script type so `.py` scripts run portably
(notably on Windows); thread `project_root` through callers so venv
preference works.
- Make `install_scripts()` mark copied `.py` files executable too.
Includes positive and negative unit tests for interpreter resolution,
`py` template processing, the new choice, and script installation.
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(cli): return repo-relative venv interpreter & correct docstring
Address PR review feedback on #3285:
- `resolve_python_interpreter()` now returns the venv interpreter as a
path relative to the project root (`.venv/bin/python` /
`.venv/Scripts/python.exe`) instead of an absolute/joined path, so the
generated `{SCRIPT}` invocation stays portable and runnable from the
repo root regardless of where the project lives.
- Update `install_scripts()` docstring to note `.py` scripts are now
made executable alongside `.sh`.
- Update tests to assert the repo-relative interpreter path.
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(cli): fall back to sys.executable for interpreter resolution
When neither python3 nor python is discoverable on PATH (and no project
venv is found), resolve_python_interpreter() now returns the running
interpreter (sys.executable) so the generated {SCRIPT} invocation works
in the current environment, falling back to "python3" only if that is
also unavailable. Update unit tests accordingly.
* fix(cli): quote py interpreter path when it contains whitespace
For the `py` script type, the resolved interpreter may be an absolute
path containing spaces (notably `sys.executable` under Windows
`Program Files`). Quote it when it contains whitespace so the `{SCRIPT}`
invocation isn't split into multiple arguments. Add positive/negative
tests for the quoting behavior.
* test: guard executable-bit assertions from Windows chmod semantics
The Windows CI job failed because `os.chmod` does not set POSIX
executable bits on Windows, so `install_scripts()` cannot make `.py`/
`.sh` files executable there (nor is it needed — the interpreter is
invoked explicitly). Split the install_scripts test so file-copy
behavior is still verified cross-platform, and skip the executable-bit
assertions on win32 (matching the repo's existing pattern).
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: resolve GitHub release asset API URL for private repo bundle downloads
For private/SSO-protected GitHub repos, browser release download URLs
(https://github.com/<owner>/<repo>/releases/download/<tag>/<asset>)
redirect to an HTML/SSO page instead of delivering the asset, causing
bundle manifest downloads to fail.
Extends the pattern from #2855 (presets/workflows) to cover the bundle
manifest download path in _download_remote_manifest:
- Resolves browser release URLs to GitHub REST API asset URLs via
resolve_github_release_asset_api_url before downloading
- Direct REST API asset URLs (api.github.com/repos/.../releases/assets/<id>)
are passed through directly
- Both cases use Accept: application/octet-stream so the API returns the
binary payload rather than JSON metadata
- The original catalog URL is used to determine artifact format (.zip vs
YAML) since the resolved API URL does not carry the file extension
Adds two CLI-level contract tests:
- bundle info resolves browser release URL via GitHub tags API
- bundle info passes direct API asset URL through with octet-stream
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: detect ZIP payload by magic bytes; add zip and API-asset tests
Address Copilot review feedback on PR #3136:
1. Detect ZIP payloads by magic bytes (PK\x03\x04) in addition to the
'.zip' URL suffix so that direct GitHub REST asset URLs — which carry
no file extension — are correctly routed through the ZIP extraction
path when the asset is a ZIP bundle artifact.
2. Add two new contract tests:
- test_bundle_info_resolves_github_browser_release_url_zip: exercises
the '.zip' browser release URL path end-to-end, verifying the tags
API lookup fires, octet-stream header is used, and bundle.yml is
successfully extracted from the ZIP payload.
- test_bundle_info_api_asset_url_zip_detected_by_magic_bytes: verifies
that a direct REST asset URL returning ZIP bytes is detected by magic
and parsed correctly without a tags API call.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: improve error message, broaden ZIP magic, drop unused tmp_path
Address second-round Copilot review feedback on PR #3136:
- Error message: when the download fails, report the original catalog
download_url so the user knows which entry to fix; include the resolved
REST API URL when it differs for easier debugging.
- ZIP detection: broaden the magic-bytes check from PK\x03\x04 to raw[:2]
== b"PK", covering all valid ZIP variants (local-file header PK\x03\x04,
empty-archive PK\x05\x06, spanned/split PK\x07\x08).
- Tests: remove the unused tmp_path parameter from
test_bundle_info_resolves_github_browser_release_url_zip.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: use full 4-byte ZIP signatures instead of 2-byte PK prefix
Address Copilot feedback: raw[:2] == b"PK" is too broad and could
misclassify any payload starting with ASCII "PK" as a ZIP, producing
a confusing "not a valid bundle" error.
Use the three specific 4-byte ZIP magic signatures instead:
PK\x03\x04 — local file header (standard ZIP)
PK\x05\x06 — end-of-central-directory (empty archive)
PK\x07\x08 — data descriptor / spanning marker
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: harden _download_remote_manifest parsing and tighten tests
- Promote _ZIP_SIGNATURES to module-level constant (was redefined per call)
- Use PurePosixPath for URL path suffix extraction so query strings and
fragments are ignored and URL paths are treated as POSIX on all OSes
- Move yaml/BundleManifest imports to function top to flatten the
previously nested try/except into a single handler with explicit
except _yaml.YAMLError and except Exception clauses
- Re-add None guard on _local_manifest_source return: the function is
typed Optional[BundleManifest] and without the guard a None return
propagates silently to callers that degrade gracefully rather than
raising an actionable error; comment explains it is defensive not dead
- Assert exact resolved asset URL in browser-URL download tests, not
just the Accept header, so a regression where download uses the
original URL instead of the resolved one would be caught
- Add resolution-failure test: when tags API finds no matching asset the
code falls back to the original URL and exits non-zero with Error:
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(bundle): pass github_provider_hosts() for GHES private release downloads
Extends the GHES support pattern from extensions and presets (#2855, #3157)
to the bundle manifest download path: resolve_github_release_asset_api_url
now receives github_hosts=github_provider_hosts() so browser release URLs
from GitHub Enterprise Server instances are resolved via /api/v3 rather
than falling back to the unauthenticated download path.
Also adds a contract test covering the GHES resolution path for
_download_remote_manifest (analogous to the existing github.com tests).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* test(bundle): remove unused ghes_entry variable from GHES contract test
The dict was defined but never consumed — the test drives GHES host
recognition entirely through the github_provider_hosts() patch.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(bundle): include source URL in remote manifest parse errors
Thread the catalog URL (and resolved API URL when it differs) into the
YAML parse, generic parse, and ZIP-extraction error paths of
_download_remote_manifest so failures point at the offending source
instead of an opaque temp path. Addresses PR review feedback.
Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous)
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 <15701806+mnriem@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
