* fix(workflows): validate command step input/options are mappings
CommandStep.validate() only checked for 'command'; execute() then does input.items() and options.update(step_options). A non-mapping input:/options: (e.g. a YAML list or scalar) raised AttributeError at run time, bypassing the per-step FAILED/continue-on-error contract -- unlike the sibling steps (switch 'cases', fan-out 'step') which type-check their config fields in validate(). Add the same checks, plus a defense-in-depth coercion in execute() since the engine does not auto-validate before running a step.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs: fix code-comment typo in CommandStep.validate
The explanatory comment said options.update(options) but execute() does
options.update(step_options). Comment-only change; no behavior change.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(workflows): command step FAILS on malformed input/options instead of coercing
execute() previously coerced a non-mapping 'input' to {} and silently ignored a
non-mapping 'options', then dispatched the command anyway. For a workflow that
skipped validation (the engine does not auto-validate before execute()), that
let an explicitly malformed step run with empty args and report COMPLETED —
masking the config error and defeating the per-step FAILED / continue_on_error
semantics this change is meant to provide.
Both now return a FAILED StepResult with the same contract error validate()
reports (never crashing on .items()/.update()). Valid mapping configs are
unaffected. Strengthened the execute() test to assert FAILED + the exact
'must be a mapping' error for input and options (fails before: the result
carried the downstream dispatch error, not the shape error).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(presets): resolve() honors manifest-declared file: for installed presets
PresetResolver.resolve()'s tier-2 (installed presets) loop was
convention-only: it looked for templates/<name>.md and <name>.md,
ignoring a preset manifest that declares the template with an explicit,
non-convention file: path. So resolve() returned the core template (and
resolve_with_source() misattributed source='core') while
collect_all_layers()/resolve_content() correctly used the preset's
declared file — a divergence inside the same class. It could also return
a stray convention-path file the manifest deliberately points away from.
Mirror collect_all_layers()'s manifest-first logic: use the declared
file: when present (skip convention fallback if it's missing, to avoid
masking typos), and fall back to the convention walk only when the
manifest is absent or doesn't list the template.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(presets): clarify the empty/falsey manifest-file branch comment
Per review: 'file' is a required key for every template entry
(PresetManifest._validate()), so the manifest-found branch is reached
for an empty/falsey/non-usable 'file' value, not a truly absent one.
Reword the comment to say so. Comment-only.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(presets): resolve() returns only real files; test missing-file skip
Per review:
- Use is_file() (not exists()) when honoring a manifest-declared file: so a
manifest pointing at a directory is treated as missing rather than
returned to a caller that will read_text() it. Applied in both resolve()
and collect_all_layers() so the two stay consistent.
- Add a regression test for the skip-convention-fallback-when-declared-file-
missing behavior: manifest declares a missing custom/spec.md while the pack
has a convention templates/spec-template.md; resolve() must skip the pack
and fall through to core, not pick up the stray convention file.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(presets): resolve()/collect_all_layers() require a regular file for manifest file:
A manifest-declared file: path is honored via exists(), which also accepts
a directory. If a preset points file: at a directory, resolve() returned it
and downstream read_text() crashes. Use is_file() in both resolve() and
collect_all_layers() so a non-file (directory) is treated as missing and the
convention fallback is skipped (pack yields to core), matching the existing
missing-file behavior.
Adds a directory-at-file: test (fails on exists(), passes on is_file()) that
also asserts collect_all_layers() never returns the directory as a layer.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor(presets): extract shared _manifest_declared_template for resolve()/collect_all_layers()
Both methods reimplemented the manifest-entry lookup + authoritative-fallback
rules independently — the exact duplication that let them diverge and caused the
bug this PR fixes. Extract a single _manifest_declared_template(pack_dir, name,
type) -> (entry, candidate) helper (candidate is the declared file only when it
is_file(); a declared-but-unusable file returns (entry, None) so callers skip the
convention fallback). resolve() and collect_all_layers() now both call it, so
their manifest-first resolution cannot silently diverge again.
Pure refactor, behavior-preserving: full test_presets.py (331) still passes,
including the directory-at-file:, missing-file, and manifest-file-wins cases.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix(init): don't block on confirmation for 'init --here' without a TTY
When 'specify init --here' targets a non-empty directory without --force, it called typer.confirm() unconditionally. In a non-interactive session (no TTY -- CI, piped, agent) there is no input, so the prompt reads EOF and aborts unhelpfully (or blocks), with no actionable message. The named-project path already fails fast and points to --force; --here was the inconsistent outlier.
Guard the confirmation with the existing _stdin_is_interactive() helper: when non-interactive, print a clear 'directory not empty; re-run with --force' error and exit 1 instead of prompting.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(init): honor piped confirmation for 'init --here'; only fail-fast on empty stdin
The first version of this fix short-circuited on '_stdin_is_interactive()' (isatty) before typer.confirm, which broke 'init --here' when confirmation is piped (e.g. 'echo y | specify init --here' / CliRunner input='y\n') -- a non-TTY pipe with valid input was wrongly rejected, regressing test_init_here_without_force_preserves_shared_infra. Instead, call typer.confirm normally (piped 'y'/'n' is honored) and catch the Abort/EOFError it raises only when stdin is empty, converting that to the actionable '--force' guidance. This keeps the UX win for the no-input case without rejecting piped input.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(init): distinguish interactive cancel from no-input; defer merge warning
Address Copilot review on the --here non-empty path: (1) treat typer.Abort during an interactive confirm (e.g. Ctrl+C) as a normal cancellation (exit 0), and only emit the '--force' guidance + exit 1 when there is no TTY (empty stdin / EOF) -- no longer conflating the two; (2) move the 'will be merged / may overwrite' warning so it only shows when actually proceeding (force) or folded into the confirmation prompt, not on the fail-fast path where nothing is merged.
Piped confirmation (e.g. 'echo y | specify init --here') is still honored, which is why the prompt is attempted rather than refused outright when non-interactive -- the existing test_init_here_without_force_preserves_shared_infra pipes 'y' and must succeed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(init): fail fast on non-interactive --here instead of prompting
Per Copilot review: do not call typer.confirm when stdin is not a TTY -- an open-but-idle non-TTY stdin (CI/agent) could block on the prompt. When the directory is non-empty and --force is not given, fail fast with '--force' guidance unless an interactive terminal is present. Interactive confirm still offers the merge-but-preserve path (distinct from --force, which overwrites); a Ctrl+C there is treated as a normal cancellation (exit 0). The merge/overwrite warning is only printed when actually proceeding, not on the fail-fast path.
Updated the preserve-merge E2E test to simulate an interactive terminal so it exercises the confirm path (non-interactive sessions now require --force).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(init): honor piped y/n for 'init --here', error only on no-input
Per maintainer review: restore the second-revision shape. Calling
typer.confirm normally keeps 'echo y | specify init --here' reaching the
non-destructive preserve-merge path (and piped 'n' cancels with exit 0).
Only when no confirmation input is available at all (closed/empty stdin
-> typer.Abort/EOFError) is it converted into the actionable error that
points at --force. This drops the _stdin_is_interactive fail-fast that
broke the common piped-confirm idiom and made preserve-merge
interactive-only. The preserve test no longer needs to monkeypatch
_stdin_is_interactive - it passes on the real contract.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(init): preserve interactive-cancel semantics; fold merge risk into the prompt
Two review-driven refinements to the 'init --here' non-empty confirm, keeping
the maintainer-endorsed control flow (piped y/n honored; non-interactive EOF →
actionable --force error):
1. typer.confirm raises typer.Abort for BOTH an interactive Ctrl+C and an EOF on
closed/empty stdin. Catching it unconditionally reported 'no confirmation
input available, use --force' and exited 1 even when the user cancelled at a
real TTY. Branch on _stdin_is_interactive(): a TTY cancel is a normal exit 0
('Operation cancelled'); only non-interactive EOF becomes the --force error.
2. Fold the merge-risk warning into the confirmation question instead of printing
it unconditionally beforehand, so the EOF/no-input path (which exits without
changing anything) no longer prints a misleading 'will be merged' line first.
Adds test_init_here_interactive_cancel_exits_zero (fails before: exit 1 with
--force; passes after: exit 0, 'cancelled', pre-existing file untouched). The
non-interactive EOF and piped-y preserve-merge tests are unchanged and still pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(integrations): exit cleanly on unbalanced quote in --integration-options (#3457)
`_parse_integration_options` called `shlex.split(raw_options)` unguarded, so an
unbalanced quote in the flag value (e.g. `--integration-options='--commands-dir
"foo'`) made shlex raise `ValueError: No closing quotation` and a raw traceback
escaped — unlike every other bad-input path in this function (unknown option,
missing value, unexpected value), which print a message and exit 1.
Reachable from `specify init --integration-options=...` and every `specify
integration install/switch/upgrade/migrate --integration-options=...`.
Wrap the split in a try/except ValueError that prints a one-line error and
raises `typer.Exit(1)`, matching the existing loud-fail UX. Add a test asserting
the unbalanced-quote input raises `typer.Exit` with exit code 1.
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>
kiro-cli confines all of its managed files to an isolated agent root
(`.kiro/`, with commands in `.kiro/prompts`) that no other integration
writes to, so it meets every documented criterion for multi-install
safety — but `KiroCliIntegration` never set `multi_install_safe = True`.
As a result, co-installing kiro-cli alongside any other integration left
`specify integration status` permanently in ERROR:
error unsafe-multi-install: Installed integrations are not all
declared multi-install safe: kiro-cli
`--force` bypasses the install-time gate but does not clear the status
error, and there is no flag or config to acknowledge it, so the error is
permanent while both integrations remain installed.
Set `multi_install_safe = True`. The registry's parametrized
multi-install-safe contract tests (static isolated root, distinct agent
roots / command dirs, disjoint manifests) now cover kiro-cli
automatically, and a focused regression test pins the declaration so a
future edit cannot silently drop it and reintroduce the error.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`FanInStep.validate()` and the engine's fan-in checks both reject a
non-list `wait_for`, but the engine's `execute()` path does not
auto-validate (see `WorkflowEngine.load_workflow`, whose docstring notes
the definition is "not yet validated"). On an unvalidated run, `execute`
iterated the raw value with `for step_id in wait_for`, with two bad
outcomes:
* a scalar (`wait_for: 5`, `wait_for: null`) raised `TypeError` and
took down the whole run — the engine invokes `step_impl.execute()`
with no surrounding try/except; and
* a string (`wait_for: stepA`) silently iterated its characters and
returned a join of empty results with a COMPLETED status — the exact
"silent empty result + COMPLETED" wiring bug the engine's own fan-in
validation comment warns against.
Guard `execute` to return a FAILED StepResult naming the type error
instead, mirroring the fan-out step's non-list `items` handling. A
missing `wait_for` key still defaults to an empty list (COMPLETED),
unchanged; the guard fires only on an explicit non-list value.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`SwitchStep.validate()` already rejects a non-mapping `cases`, but the
engine's `execute()` path does not auto-validate (see
`WorkflowEngine.load_workflow`, whose docstring notes the definition is
"not yet validated"). On an unvalidated run, `execute` called
`cases.items()` on the raw value, so a list or scalar `cases` authoring
mistake raised `AttributeError` and took down the whole run — the engine
invokes `step_impl.execute()` with no surrounding try/except.
Guard `execute` to return a FAILED StepResult naming the type error
instead, mirroring the fan-out step's non-list `items` handling. The
expression is still evaluated first, so its value is surfaced in the
step output for downstream context.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix: rewrite extension-relative subdir paths in generated command bodies
Extension command bodies reference bundled files relative to the
extension root (agents/, knowledge-base/, templates/, ...). Generated
SKILL.md and command files emitted those paths verbatim, so agents
resolved them against the workspace root where they do not exist.
Add CommandRegistrar.rewrite_extension_paths, which rewrites references
to subdirectories that actually exist in the installed extension to
.specify/extensions/<id>/..., and call it once in register_commands so
every output format and alias gets the fix. commands/, specs/ and
dot-directories are never rewritten.
Fixes#2101
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: only rewrite relative extension path references
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: use callable re.sub replacement for extension subdir rewrite
subdir and extension_id come from filesystem directory names and were
interpolated into a re.sub string replacement template. A directory name
containing a backslash (e.g. assets\q) would raise re.error: bad escape,
aborting command registration even when the body didn't reference it.
Use a callable replacement so these values are treated literally.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: make subdir rewrite regression test cross-platform
Renamed the test's subdir fixture from "assets\\q" to "assets[q]":
on Windows, backslash is a path separator, so mkdir would create
nested "assets/q" dirs instead of one literally-named directory,
and iterdir() would only discover "assets", never exercising the
rewrite. extension_id keeps a real backslash/"\\1" since it isn't
used to create a directory, still verifying the callable replacement
handles it literally. Added a sanity assertion for this assumption.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: apply extension subdir path rewrite in skills-mode renderer
register_commands() rewrote extension-relative subdir references
(agents/, knowledge-base/, etc.) via rewrite_extension_paths(), but
_register_extension_skills() - the separate renderer used for active
non-native skills agents (e.g. Claude with ai_skills: true) - never
called it. Generated SKILL.md files left agents/... and
knowledge-base/... unresolved, and mapped the extension's own
templates/ through the generic project-level rewrite instead of its
installed .specify/extensions/<id>/templates/ location.
Reuse the existing rewrite_extension_paths() helper in
_register_extension_skills() at the same point register_commands()
applies it (before resolve_skill_placeholders' generic rewrite), and
add a skills-mode regression test.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: apply extension subdir path rewrite on preset restore/reconcile paths
_unregister_skills() restored extension-backed SKILL.md content via
resolve_skill_placeholders() without first calling
rewrite_extension_paths(), so removing a preset override that shadowed
an extension command restored the bare, unresolvable agents/... and
knowledge-base/... references. Carried extension_id/extension_dir
through _build_extension_skill_restore_index() and applied the same
rewrite used at initial registration before restoring.
Found the identical gap in _reconcile_composed_commands()'s non-skill
agent path: when a removed preset's command reverts to an extension
winner, register_commands_for_non_skill_agents() was called without
extension_id, so the rewrite never ran for plain command-file agents
either. Passed extension_id through there too.
Added regression tests for both restore paths (skills-mode and
non-skill-agent command files) in tests/test_presets.py.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: apply extension subdir path rewrite when composing over extension base
PresetResolver.resolve_content() read the effective base layer's raw
content directly via path.read_text() before composing append/prepend/
wrap overlays on top of it, and its outright-replace shortcut did the
same. When that base layer was extension-provided, neither read path
applied rewrite_extension_paths(), so composing a preset over an
extension command (or an extension winning outright through
resolve_content) left bare, unresolvable agents/... and
knowledge-base/... references in the composed output.
All three call sites (PresetManager._register_commands()'s composed
path, _reconcile_composed_commands()'s composed path, and skills-mode
reading the .composed file written by either) consume resolve_content's
return value, so fixing the read at its source covers command output,
skill output, and both initial-install and reconcile flows without
threading extension identity through each caller.
Tagged extension layers in collect_all_layers() with extension_id/
extension_dir, and added a _read_layer_content() helper in
resolve_content() that applies rewrite_extension_paths() whenever a
layer carries that extension identity — used at both raw-read sites
(outright-replace shortcut and composition base). Composing
(append/prepend/wrap) layers are never extension-provided (extensions
are always inserted with strategy "replace"), so no other read site
needs the rewrite.
Added regression tests: a parametrized resolve_content() test covering
append/prepend/wrap composing over an extension base, a skills-mode
test asserting the composed SKILL.md resolves the extension's subdir
references, and a non-skill-agent (Gemini) install-time test matching
the reported live repro.
Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat(workflows): make shell step timeout configurable (#3327)
The `shell` step hardcoded a 300s subprocess timeout, so any command
that legitimately runs longer than five minutes (a full build, a linter
aggregator, an integration-test target) was killed with TimeoutExpired
and failed the whole run, with no YAML knob to raise the limit.
Add an optional `timeout` field (seconds) that defaults to 300 for
backward compatibility and is threaded through to `subprocess.run`. The
timeout failure message now reports the configured value instead of a
hardcoded 300. `validate` rejects a `timeout` that is not a positive
number (bool is rejected explicitly, since it is an int subclass but a
config error rather than a duration).
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>
* test(workflows): cover non-finite timeout rejection in shell step
The isfinite guard added in 955d46a rejects YAML .inf/.nan timeouts, but no test asserted it. inf and nan are floats that pass a plain > 0 check (nan <= 0 is False), so without an explicit case a regression could silently reaccept them and crash subprocess.run(timeout=...) at runtime. Addresses the remaining Copilot review comment on PR #3328.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(workflows): document configurable shell step timeout
Address Copilot review feedback on #3328: the per-step `timeout`
option was not reflected in the public workflow docs. The Shell Steps
section only showed `run:`, so readers couldn't discover `timeout:`,
its unit (seconds), or its default (300).
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>
* refactor(workflows): consolidate shell-step timeout validation into one path
Address Copilot review feedback on #3328:
- Remove the dead "fall back to default" timeout block in execute(): it
re-read `timeout` from config immediately after, so the fallback was
discarded and its comment contradicted the new fail-on-invalid behavior.
- Extract a single `_timeout_error()` helper shared by execute() and
validate() so both reject the same values with the same message, instead
of two drifting copies of the check.
- Hoist the duplicated inline `import math` to module scope.
- Add test_execute_fails_cleanly_on_invalid_timeout: asserts execute()
fails the step (rather than raising) on an unvalidated string/bool/inf/0
timeout, covering the engine-skips-validate path Copilot flagged.
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(workflows): don't crash on membership test against a non-iterable
the `in` / `not in` operators in _evaluate_simple_expression only guarded
`right is not None`, so `left in right` still raised a raw TypeError when the
right operand was any other non-iterable (int, bool, float). a condition like
`{{ inputs.tag in inputs.count }}` where count is a number crashed the whole
workflow run instead of evaluating.
nothing is contained in a non-iterable, so treat membership as False (`not in`
as True) via a new _safe_membership helper that swallows TypeError. this
generalizes the old None guard and mirrors _safe_compare, which already
catches TypeError for the ordering operators.
added a regression test; confirmed it fails on the pre-fix code (raw
TypeError) and that genuine list/substring membership still works.
* address review: float membership case + broaden _safe_membership docstring
- add a float right-operand assertion so the test matches its comment (was
claiming float coverage while only exercising int/bool/None).
- reword the _safe_membership docstring to describe TypeError generally
(non-iterable right is the common case, but also e.g. an unhashable left
against a set) rather than implying only the right operand matters.
* fix(workflows): if-step validate accepts falsy non-list else
IfThenStep.validate() guarded the 'else' branch with
'if else_branch and not isinstance(else_branch, list)'. The leading
truthiness check short-circuits for falsy non-list values (False, 0,
'', {}), so a malformed else-branch passes validation and is then
silently skipped at runtime. The sibling 'then' branch is validated
strictly; 'else' now matches by switching to an 'is not None' guard.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(workflows): cover explicit else:None and missing-else separately
Per Copilot feedback: the parametrized valid-else test omitted the
'else' key when the value was None, so it covered only the missing-else
case, not an explicit 'else: None'. Set 'else' explicitly (including
None) in the parametrized test and add a dedicated missing-else test, so
both accepted shapes are pinned.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The set-priority skip guard 'isinstance(raw_priority, int) and
raw_priority == priority' treats a stored boolean as a match because
isinstance(True, int) is True and True == 1 (False == 0). So a corrupted
boolean priority short-circuits to 'already has priority N' and is never
rewritten to a real int — contradicting the adjacent comment that
promises corrupted values get repaired. Exclude bools explicitly,
mirroring normalize_priority's own bool guard.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Same bool-is-int trap as the extension set-priority command: the skip
guard 'isinstance(raw_priority, int) and raw_priority == priority' treats
a stored boolean as a match (isinstance(True, int) is True, True == 1),
so a corrupted boolean priority reports 'already has priority N' and is
never rewritten to a real int — contradicting the adjacent comment.
Exclude bools explicitly, mirroring normalize_priority's bool guard.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The while/do-while loop cap guard 'not isinstance(max_iters, int) or
max_iters < 1' does not fall back to the default for a boolean
max_iterations: isinstance(True, int) is True and True < 1 is False. The
loop then runs range(max_iters - 1) == range(True - 1) == range(0),
capping at a single iteration instead of the default 10. Exclude bools,
mirroring the merged while/do-while validators (#3237) and this
function's own continue_on_error bool handling. execute() does not
auto-validate, so this engine guard is the only defence.
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* 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>
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>
* 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>
* 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(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>
* 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.
