mirror of
https://github.com/github/spec-kit.git
synced 2026-08-20 10:47:26 +00:00
4faecb8422de57fac3289a3e72ff9a652dec94e2
117
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Ngo Quoc Viet
|
d6e09a17c4 |
fix(workflows): validate non-string step types (#4111)
Return an actionable validation error when a workflow step type is a YAML list or mapping instead of raising during registry membership checks. Assisted-by: OpenAI Codex (model: GPT-5, autonomous) |
||
  Ali jawwadandClaude Opus 5
|
1b3695bfb3 |
fix(workflows): strip a resolved condition before the true/false check (#3883)
evaluate_condition() special-cases the strings "false"/"true" so that
`condition: "false"` behaves as a boolean, but it matches with
`result.lower()` and never strips.
The most common way a *string* reaches a condition is captured command
output, and the shell step stores stdout verbatim
(steps/shell/__init__.py:67 `"stdout": proc.stdout`). So `run: echo false`
resolves to "false\n", which matches neither branch and falls through to
`bool("false\n")` -> True:
'false' -> False
'false\n' -> True <-- bug
'false\r\n' -> True <-- bug
' false' -> True <-- bug
An `if` step therefore takes its `then` branch on a step that printed
"false", and `while`/`do-while` keep dispatching their body.
A workflow author cannot work around it: the registered filters are
default/join/map/contains/from_json — there is no `trim`.
`InitStep._resolve_bool` and both catalog readers already strip before
matching boolean text. `bool(result)` still sees the raw string, so no
non-boolean text changes truthiness.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
 Noor ul ainandClaude Opus 4.8
|
204d94fdb1 |
fix(workflows): handle an unreadable run state in workflow status (#3999)
`workflow status <run_id>` and `workflow resume <run_id>` both call `RunState.load()`, and a prior fix aligned them on the FileNotFoundError and ValueError boundaries. `resume` also handles OSError; `status` never gained that handler. So an unreadable `state.json` -- wrong permissions, an I/O error, or a directory sitting where the file belongs -- escapes as a raw traceback with no output at all, while `resume` on the same run prints a clean `Error:` line and exits 1. `state_path.exists()` is True for a directory, so the existing guard passes and `open()` raises. Add the missing `except OSError` next to its siblings, using the same `_escape_markup` + `typer.Exit(1)` shape, and routing through `err` so the message lands on stderr under `--json` and the stdout JSON stream stays parseable. Two regression tests: the end-to-end CLI path (a directory in place of state.json) and the `--json` stderr-routing path. Both fail without the source change. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
 Quratulain-bilal
|
1e85d4ff53 |
fix: skip corrupted run state files in list_runs (#3814)
* fix: skip corrupted run state files in list_runs * fix: address review comments - add UnicodeDecodeError, dict validation, and regression tests - Catch UnicodeDecodeError for invalid UTF-8 encoding - Validate loaded JSON is a dict with required 'run_id' key - Add 5 regression tests for corrupted state files Fixes #3814 |
||
  Marsel SafinandCopilot
|
cda7a921a4 |
fix(workflows): reject mismatched run state IDs (#3899)
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Ali jawwadandClaude Opus 5
|
84a2114338 |
fix(workflows): keep the init step's documented ignore_agent_tools default on an explicit null (#3889)
The step documents the default twice:
class docstring: "Because workflows run unattended, the step defaults to
``--ignore-agent-tools``"
field docs: "Skip checks for the coding agent CLI (defaults to ``true``)"
It implements that with `config.get("ignore_agent_tools", True)`, which
applies the default only when the key is ABSENT. A bare
`ignore_agent_tools:` in YAML parses to None, and `_resolve_bool(None)`
returns False:
key ABSENT -> True flag emitted: YES
bare ignore_agent_tools: -> False flag emitted: NO <-- bug
explicit true -> True flag emitted: YES
explicit false -> False flag emitted: NO
So the flag is dropped, `specify init` re-runs the agent-CLI presence
check, and an unattended run fails with "Agent Detection Error" for any
integration whose CLI is not installed on the runner.
Normalize an explicit null to the default, mirroring the while/do-while
`max_iterations` handling.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
 Manfred RiemandCopilot App
|
e9ffc9d8e7 |
feat: allow overriding default init integration via SPECKIT_INTEGRATION_DEFAULT (#3952)
* feat: allow overriding default init integration via SPECKIT_INTEGRATION_DEFAULT Resolve the non-interactive/init default integration from the SPECKIT_INTEGRATION_DEFAULT environment variable, fitting the existing SPECKIT_INTEGRATION_* namespace. Falls back to the hardcoded "copilot" default when unset, and warns to stderr (rather than silently falling back) when the value is not a registered integration key. Wires the resolver into specify init (interactive prompt default and non-interactive fallback), the init workflow step, and the bundle init default. Adds unit and CLI tests and documents the variable. Closes #3939 Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecda55f-fa13-42f7-99bf-bfb0bb8565a0 * test: cover env-var default wiring for picker, workflow step, and bundle Address PR review: add regression tests so each SPECKIT_INTEGRATION_DEFAULT wiring site cannot silently revert to the hardcoded constant. - init.py: interactive picker receives the resolved key as default_key. - workflow init step: no step/workflow default + env var drives output integration and argv. - bundle _resolve_init_integration: env-var default applies when unspecified, while explicit override and manifest-declared integration still win. Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecda55f-fa13-42f7-99bf-bfb0bb8565a0 --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecda55f-fa13-42f7-99bf-bfb0bb8565a0 |
||
|
Ali jawwadandClaude Opus 5
|
d1e86f6382 |
fix(workflows): fail a gate whose on_reject is not abort/skip/retry (#3888)
execute() reads `on_reject = config.get("on_reject", "abort")` and, in the
reject branch, handles only "abort" and "retry" before falling through to
its `# on_reject == "skip"` case. So any other value makes a REJECTED gate
report COMPLETED and the run walks straight past the review the gate
exists to enforce:
on_reject='abort' -> failed "Gate rejected by user at step 'g'"
on_reject='retry' -> paused
on_reject='skip' -> completed (by design)
on_reject='Abort' -> completed <-- rejection silently discarded
on_reject='fail' -> completed <-- same
on_reject='stop' -> completed <-- same
on_reject=None -> completed <-- same
on_reject=5 -> completed <-- same
Reachable by a capitalisation slip, a guessed verb, a non-string, or a
bare `on_reject:` — note `config.get(k, default)` does NOT substitute the
default for an explicit YAML null.
`validate` already rejects anything outside abort/skip/retry, but the
engine does not auto-validate before execute(). Fail loudly instead,
mirroring the `options` and `verdict_input` guards in the same method, and
placed before the non-TTY short-circuit so it surfaces in CI too.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwadandClaude Opus 5
|
521020bc3a |
fix(workflows): fail a fan-in step whose output is not a mapping (#3887)
execute() did:
output_config = config.get("output") or {}
if not isinstance(output_config, dict):
output_config = {}
so every non-mapping `output` was silently discarded and the step still
returned COMPLETED — every declared aggregation key vanished, and
downstream `{{ steps.<id>.output.<key> }}` resolved to None and
interpolated as an empty string:
output=[] -> completed, error=None
output=False -> completed, error=None
output=0 -> completed, error=None
output='' -> completed, error=None
output=['a'] -> completed, error=None
output='oops' -> completed, error=None
output=5 -> completed, error=None
`validate` already rejects this and its comment names the flaw exactly:
"execute() silently coerces a non-mapping output to {}, so the author's
declared aggregation keys would vanish with no error." The engine does not
auto-validate before execute(), so on an unvalidated run that is what
happened — and `x or {}` masked the falsy shapes before the isinstance
check even ran.
Fail loudly with validate()'s own message, mirroring the `wait_for` guard
in the same method. An explicit `output:` (YAML null) stays valid.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Marsel SafinandCopilot
|
14e82353cb |
fix(workflows): refetch non-UTF-8 catalog caches (#3901)
Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Noor ul ainandClaude Opus 5
|
cf71d00dfe |
fix(workflows): reject a retry gate whose verdict enum forbids the reset value (#3912)
A gate with `on_reject: retry` consumes a bound reject verdict before
pausing by resetting the named input to `""` (documented behaviour, so a
later resume prompts again). Every `resume()` re-resolves the persisted
inputs through `_coerce_input`.
Those two rules collide when the bound input declares an `enum` that does
not list `""`. The reset writes a value the input's own enum forbids, and
the run wedges:
inputs:
spec_verdict:
type: string
enum: [approve, reject]
steps:
- id: review
type: gate
options: [approve, reject]
on_reject: retry
verdict_input: spec_verdict
$ specify workflow run wf --input spec_verdict=reject
Status: paused
$ specify workflow resume <run_id> --input note=b
Error: Input 'spec_verdict' value '' not in allowed values:
['approve', 'reject'].
The workflow validates clean and the first run looks fine, so the failure
only appears at the second resume. It is also unrecoverable in practice:
`_resolve_inputs` re-coerces the whole persisted map, so *any* resume that
supplies an input dies on the stored `""`. Only a resume with no inputs at
all still works -- and that is precisely the call that cannot deliver a new
verdict, which is the one thing the retry cycle exists to allow.
Extend the existing `verdict_input` cross-check (which already confirms the
name is declared) to also require that a retry-bound input's `enum` admits
the reset sentinel, and report it with a fix hint. To do that, thread the
input *definitions* through `_validate_steps` instead of just their names.
Rejected the alternative of popping the key instead of writing `""`: that
lets the input's `default` flow back in on the next resume, so a gate the
user just rejected would silently auto-approve.
Docs: note the `enum` requirement next to the reset behaviour it follows
from.
Adds 4 validation tests for the new guard plus a characterization test that
drives the engine directly to pin the wedge it prevents.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assisted-by: Claude Opus 5 (1M context)
|
||
|
Manfred RiemandCopilot App
|
a3e183d069 |
feat: support tar archives for installs (#3874)
* feat: support tar archives for installs Add secure .tar.gz and .tgz parity with ZIP installation for extensions, presets, and workflows, including full workflow package preservation. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd * chore: clean rebased archive imports Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd * fix: preserve hardened archive install behavior Keep malformed ZIP diagnostics, filesystem-independent manifest selection, and reserved workflow overlays consistent after adding generic archive support. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd * fix: extract staged workflow archives by descriptor Avoid reopening a held staging path on Windows while retaining authoritative-inode archive validation. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd * fix: extract catalog archives from verified bytes Use the already bounded and SHA-verified response bytes directly so Windows file-sharing semantics cannot affect archive detection. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd * fix: address archive install review feedback Preserve forced preset reinstalls, sniff suffixless workflow archives without weakening YAML limits, and restore prior workflow packages before failed-install cleanup. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd --------- Copilot-Session: bd07c6b3-f1f9-484c-869a-94d8fef970dd |
||
|
Marsel SafinandCopilot
|
515d2810fb |
fix: reject non-object workflow caches (#3860)
* fix: reject non-object workflow caches Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: cover non-object stale workflow cache Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Marsel SafinandCopilot
|
0f3f2aa45e |
fix: escape workflow step metadata (#3863)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
  
|
675143591d |
feat: bind gate verdict to workflow input via verdict_input (#3725)
* feat: bind gate verdict to workflow input via verdict_input Add an optional `verdict_input` field to gate steps that lets an external system supply a verdict through a declared workflow input instead of an interactive TTY prompt. When the referenced input carries a non-empty string value that matches one of the gate's `options` (case-insensitive), the gate auto-decides, records the matched spelling in `output.choice`, and applies the existing `on_reject` / abort / skip / retry semantics. If the value is present but does not match an option, or is a non-string, the gate fails immediately with a clear error message. When the input is absent, null, or empty, the gate falls back to today's TTY-prompt-or-pause behaviour unchanged. The engine now persists `result.error` alongside each step's status and output so that failed-step error messages survive across runs. The CLI (`workflow run` and `workflow resume`) surfaces these persisted errors after a failed or aborted run. `validate_workflow` cross-references `verdict_input` against the workflow's declared inputs block and reports an error for undeclared names, consistent with the existing `wait_for` id cross-check. Closes discussion: https://github.com/github/spec-kit/discussions/3717 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fix workflow JSON error payloads Include persisted step errors in _workflow_run_payload so workflow run/resume/status --json all surface failure reasons consistently. Add JSON-path tests for failed and successful runs. Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot <223556219+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(workflows): reject verdict inputs in fan-out Fan-out items share workflow inputs and cannot safely consume a bound gate verdict. Reject verdict_input bindings during validation and at runtime while preserving unbound gates. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: update workflow command handling Assisted-by: GitHub Copilot (model: gpt-5.3-codex, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Markus <markus@example.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Noor ul ainandClaude Opus 5
|
6712665bba |
fix(workflows): guard the shell step's timeout check against OverflowError (#3865)
PR #3847 hardened the prompt step's `timeout` guard against a huge-int value, but its twin in the shell step — the step the prompt one was mirrored from — still has the hole. `math.isfinite(10**400)` raises `OverflowError: int too large to convert to float`. A 400-digit YAML scalar is an `int` and is not a `bool`, so it clears every clause before `isfinite()` and raises there, escaping `_timeout_error()` as exactly the uncaught crash that helper exists to prevent: steps: - id: qa type: shell run: echo hi timeout: 1000...0 # 400 digits $ specify workflow run wf.yml Traceback (most recent call last): ... File "src/specify_cli/workflows/engine.py", line 361, in _validate_steps step_errors = step_impl.validate(step_config) File "src/specify_cli/workflows/steps/shell/__init__.py", line 127 or not math.isfinite(timeout) OverflowError: int too large to convert to float `workflow_run` calls `engine.validate()` before executing any step, so the OverflowError propagates out of `validate_workflow` and kills the command with a bare traceback that names neither the step nor the field, instead of the "Workflow validation failed" report. `execute()` shares the same helper, so an unvalidated run raises there too — and the engine re-raises anything a step throws, aborting the whole workflow after earlier steps have already run their side effects. The value is genuinely invalid rather than merely unrepresentable in the check: `subprocess.run(timeout=10**400)` raises the same OverflowError. Unlike the prompt step, the shell step checks `isfinite()` *before* `timeout <= 0`, so a negative huge int (`-(10**400)`) crashes as well rather than being caught by the sign check. Wrapped the condition in `try/except OverflowError` and treated the value as invalid, mirroring the prompt step's guard so both steps reject the same values with the same message. Now: Workflow validation failed: - Shell step 'qa': 'timeout' must be a positive number of seconds, got 1000...0. Valid int/float timeouts, non-finite floats, bools, strings and non-positive values are unaffected — the existing clauses are unchanged. Regression tests in `TestShellStep`: `validate()` rejects both signs of the huge int, `validate_workflow()` reports it end to end (pinning the path the CLI actually takes, not just the helper), and `execute()` fails only that step with `subprocess.run` patched to assert it is never reached. Test-the-test: reverting the source change fails all three with `OverflowError` and leaves the rest of `TestShellStep` passing. Assisted-by: Claude Code (model: claude-opus-5, under direct human supervision) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
afbb2c7b65 |
fix(workflows): validate prompt step 'timeout' like the shell step (#3847)
* fix(workflows): validate prompt step 'timeout' like the shell step PR #3768 added a `timeout` to the prompt step and passed it straight into `subprocess.run(timeout=...)`. Neither `validate()` nor `execute()` checks it, so a bad value from a user-authored `workflow.yml` escapes as a raw exception: steps: - id: first type: shell run: echo side-effect - id: ask type: prompt prompt: do it timeout: abc $ specify workflow run wf.yml > [first] shell ... Workflow failed: unsupported operand type(s) for +: 'float' and 'str' The engine re-raises anything a step throws, so this takes down the whole run — after `first` has already run its side effect — with a message that names neither the step nor the field. `timeout: .nan` raises `ValueError: cannot convert float NaN to integer` the same way, and a non-positive `timeout` (`0`, `-5`) makes `subprocess.run` report an immediate TimeoutExpired for a command that never got the time to run. `timeout: true` silently becomes a 1-second limit, since bool is an int subclass. The sibling shell step already rejects exactly these values via a `_timeout_error()` helper shared by `execute()` and `validate()`, so the same workflow failed validation cleanly as a shell step and crashed as a prompt one. Mirrored that helper onto PromptStep: `validate()` reports the contract error, and `execute()` re-checks it so an unvalidated run fails just that step instead of aborting. Now: Workflow validation failed: - Prompt step 'ask': 'timeout' must be a positive number of seconds, got 'abc'. caught before the first step runs. Positive int/float timeouts and an absent `timeout` are unaffected. Regression tests in `TestPromptStep` mirror the shell step's: validate rejects "30"/True/inf/nan/0/-5/list/None, validate accepts 300/5/0.5 and an absent field, and execute fails cleanly with `subprocess.run` patched to assert it is never reached. With the source fix reverted, all 9 rejection tests fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Assisted-by: Claude Code (model: claude-opus-5, under direct human supervision) * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * test(workflows): cover the huge-int timeout OverflowError guard The autofix commit wrapped the prompt step's `_timeout_error()` check in `try/except OverflowError` but added no test, so nothing pins the behaviour it introduced. `math.isfinite(10**400)` raises `OverflowError: int too large to convert to float` — the value is an `int`, is `> 0`, and is not a `bool`, so it clears every other clause of the guard and reaches `isfinite()`. Without the `except`, validating ```yaml - id: ask type: prompt prompt: do it timeout: 1000...0 # 400 digits ``` raises that `OverflowError` out of `validate()`/`execute()` — exactly the uncaught-crash failure mode this guard was added to prevent. The same value raises `OverflowError` from `subprocess.run(timeout=...)`. Add `10**400` to both parametrized rejection lists (`validate()` and the `execute()` fails-cleanly loop). Test-the-test: reverting the `try/except` fails both new cases with `OverflowError` and leaves the rest passing. Assisted-by: Claude Opus 5 (model: claude-opus-5, autonomous) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Ali jawwadandClaude Opus 5
|
6033c6957b |
test(workflows): name the condition-rejection tests for the real boundary (#3808)
`test_validate_rejects_non_string_condition` contradicts its sibling `test_validate_accepts_string_or_bool_condition` in the same class: a bool *is* a non-string, so the two names disagree about the contract the validator actually implements. Rename to `test_validate_rejects_non_string_non_bool_condition` in all three step classes, matching the validator's own message: "'condition' must be a string or boolean, got <type>". Test names only — no behaviour change, and the parametrized values are untouched. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Ali jawwadandClaude Opus 5
|
d99170fb5e |
fix(workflows): dispatch prompt steps via the resolved executable (#3793)
PromptStep._try_dispatch runs `subprocess.run(exec_args, ...)` with an
UNRESOLVED argv[0] -- a bare name like `claude`. On Windows subprocess.run calls
CreateProcess, which does not consult PATHEXT, so an agent CLI installed as a
`.cmd`/`.bat` shim (the usual npm layout) raises FileNotFoundError [WinError 2].
That OSError is swallowed by the method's `except OSError: return None`, and
execute() then reports "CLI not found or not installed" -- even though the
step's own preflight `shutil.which(...)` two lines earlier just found it.
The sibling path does not have this bug: IntegrationBase.dispatch_command (used
by the `command` step) resolves argv[0] through shutil.which first, added in
|
||
|
Ali jawwadandClaude Opus 5
|
751eae727e |
fix(workflows): escape the step-progress line so step ids render (and / stops failing the run) (#3783)
`workflow run` and `workflow resume` both print the step-progress line as
`f" ▸ [{sid}] {label} …"`. Rich parses the bracketed step id as a style tag,
which produces three failures on main:
1. The id is SILENTLY SWALLOWED on every run -- the only identifying content on
the line. `id: greet` prints " ▸ shell …"; "[greet]" is absent.
2. An id that forms a closing tag FAILS THE WHOLE RUN. `validate_workflow`
places no charset restriction on step ids, so `id: "/"` is a valid workflow;
the callback then raises MarkupError, which propagates into execute()'s
handler -> run persisted as `failed` with empty `step_results`, the step
never executed, exit 1 with a Rich internals error.
3. An id that is a real style (`bold`, `red`) is applied as FORMATTING to the
rest of the line.
The unescaped `label` (from `step_config["command"]`) compounds it.
Escape the literal bracket with `\[` and escape both interpolated values, at
both sites. This mirrors the `\[<type>]` step-graph precedent already in this
file (workflow_info). Escaping only the values is NOT sufficient -- the
f-string's own brackets are what Rich consumes.
Verified through the real CLI: ids `greet`/`bold`/`a]b` now render verbatim, and
`id: "/"` goes from a failed run to `Status: completed`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwadandClaude Opus 5
|
52a6514e38 |
fix(workflows): reject falsy non-mapping workflow-catalogs.yml top level (#3707)
* fix(workflows): reject falsy non-mapping workflow-catalogs.yml top level
WorkflowCatalog._load_catalog_config parsed the config with
`yaml.safe_load(...) or {}`, then checked `isinstance(data, dict)`. The
`or {}` coerces a FALSY non-mapping top level (`[]`, `false`, `0`, `''`) to
`{}` *before* the guard runs, so those are silently swallowed as "empty
config" and fall back to the built-in defaults -- while a TRUTHY non-mapping
(`5`, a bare list) correctly raises. Same silent-swallow inconsistency the
bundler catalog reader fixed for its own config.
Drop the `or {}` and branch on `None` (empty document / explicit `null`)
explicitly: `None` stays a valid no-op, every non-mapping (falsy or truthy)
now raises the same actionable error. Correct configs are unaffected.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(workflows): describe the catalog-config fallthrough accurately
The comment said a None return means "no project catalogs, fall back to the
built-in defaults". Both halves were imprecise: _load_catalog_config serves the
project AND user configs, and get_active_catalogs falls through env -> project
-> user -> built-in, so a None from the project layer moves on to the USER
config; the built-in defaults apply only once every layer returned None.
Reword the loader comment and the mirror test docstring. Comments only -- no
behaviour change.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(workflows): close the same falsy-mask gap in 'catalogs' and StepCatalog
Self-review follow-up: the top-level fix left the identical asymmetry live five
lines below, and again in this file's twin loader.
1. WorkflowCatalog._load_catalog_config: the ``catalogs`` shape check sat behind
an emptiness check, so a FALSY non-list (``catalogs: {}``/``''``/``0``/
``false``) was silently swallowed as "no catalogs" while ``catalogs: 5``
raised. Verified before this commit: ``catalogs: {}`` -> None (no error).
Shape now checked first; absent/explicit-null and empty-list stay no-ops
(matching the bundler's reader).
2. StepCatalog._load_catalog_config -- the step-catalog twin, read the same way
-- still had ``yaml.safe_load(...) or {}``, so falsy non-mappings bypassed its
isinstance guard (``[]`` -> None while ``5`` raised). Same two guards applied,
keeping the two loaders in lockstep.
Eight new parametrized cases, all failing before this commit.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(workflows): move StepCatalog guards into TestStepCatalog and add the nested case
Address three review points:
1. The StepCatalog regression tests sat inside TestWorkflowCatalog, so a
targeted `pytest ...::TestStepCatalog` run skipped them entirely. Moved into
that class, where the duplicated twin loader belongs.
2. StepCatalog had no nested-value coverage (only top-level). Added the
parametrized falsy ``catalogs:`` case, plus the absent/null/empty no-op
cases. Verified against upstream/main's catalog.py: 8 fail there, pass here.
3. Dropped the inaccurate parity parenthetical. src/specify_cli/catalogs.py
RAISES for missing/empty ``catalogs`` and coerces a null document to {}, so
it is not the behavior this loader matches -- the comment now just states
what changed (only the misreported shapes) without claiming parity.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwadandClaude Opus 5
|
a9c9905400 |
fix(workflows): reject non-string/non-boolean 'condition' in if/while/do-while steps (#3706)
* fix(workflows): reject non-string 'condition' in if/while/do-while steps
`if_then`, `while_loop`, and `do_while` validate() confirm `condition` is
present but never that it is a string. execute() feeds it to
`evaluate_condition()`, which returns a non-string as-is and takes `bool()`
of it -- so `condition: [1, 2]` (a list authoring mistake) silently resolves
to `True`, branching wrongly / spinning the loop to `max_iterations`, with no
error reported.
Reject a present-but-non-string `condition` at validation, mirroring the
existing prompt/shell/command 'must be a string' guards. `"true"`/`"false"`
and expressions like `"{{ ... }}"` are strings, so they stay valid.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(workflows): describe the evaluate_expression/evaluate_condition split accurately
Address review feedback: the guard comments attributed the non-string
pass-through to evaluate_condition(), which always returns a bool. It is
evaluate_expression() (called by evaluate_condition) that returns a non-string
unchanged; evaluate_condition then applies bool() to that value.
Reword all four sites (if/while/do-while guards + the mirror test comment) to
name the two stages correctly. Comments only -- no behaviour change.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(workflows): keep a literal bool 'condition' valid
Self-review catch: the guard rejected EVERY non-string, which broke an input
that previously worked. An unquoted ``condition: false`` is idiomatic YAML and
resolves exactly today -- evaluate_expression passes the bool through and
evaluate_condition's bool() is a no-op (verified: evaluate_condition(False) is
False, (True) is True). The if/while steps even default ``condition`` to the
bool ``False`` themselves, so bool is the field's natural type, not an
authoring mistake.
Accept (str, bool) and reject only the genuinely silent-coercion types
(list/dict/int/float, e.g. condition: [1, 2] is always True). Message updated
to "must be a string or boolean"; the bad-value tests drop True and gain 1.5,
and each step gains a positive bool case.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Noor ul ainandClaude Opus 5
|
8bfe6e14d1 |
fix(workflows,extensions): tolerate non-list catalog tags in search/info display (#3770)
`workflow search`, `workflow info`, `extension search` and `extension info`
crashed with `TypeError: 'int' object is not iterable` when a catalog entry
carried a scalar `tags:` value (e.g. `tags: 5`). Catalog payloads are
user-editable YAML/JSON, so this shape reaches the display unvalidated.
Both backends already guard their tag *filter* with
`isinstance(raw_tags, list)` — `WorkflowCatalog.search` and
`ExtensionCatalog.search` skip a non-list `tags` cleanly. Only the display
paths were unguarded: they tested truthiness (`if info.get("tags"):`) and
then iterated. A scalar is truthy but not iterable, so `--tag` filtering
survived while plain `search`/`info` rendering blew up.
Note this is distinct from the non-string *element* handling added in
#3746/#3747: coercing elements with `str(t) for t in ...` does not help when
`tags` is not a sequence at all. The fix is the guard the sibling
integration commands already use — `integrations/_query_commands.py:332,402`
gate on `isinstance(tags, list) and tags`. This aligns workflows and
extensions with that reference pattern, leaving all four tag-join display
sites in these modules consistent.
Regression tests drive the full CLI via CliRunner and cover search + info in
one case per module; both fail before the fix with the exact TypeError.
Assisted-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
 Pascal THUET
|
118062eac4 |
harden: secure extension and preset archive downloads (#3141)
* harden: secure extension and preset archive downloads Adopt the shared download-security primitives from #3140 across extension and preset catalog, package, direct-URL, and ZIP-install flows: - bound catalog, package, and inline manifest reads; - verify catalog SHA-256 values when present; - replace path-only extraction with bounded traversal/symlink-safe extraction; - validate malformed hosts and ports before opening download URLs; - handle normalized trailing-backslash directory entries consistently. Redirect enforcement and checksum verification remain owned by the shared helpers already on main; this commit wires them into extension and preset behavior. Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * harden: close archive and catalog download edge cases Preflight ZIP central directories before ZipFile allocates them, bound both declared and actual payload sizes, and reject ambiguous or non-portable archive paths before extraction. Keep extension update manifest selection consistent with extraction, reject unsafe catalog-derived output filenames and malformed URL types, and escape untrusted values in download errors. Add regression coverage for parser differentials, collisions, platform-specific filenames, bounded call sites, and failure ordering. Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * harden: address download security review feedback Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * harden: close ZIP preflight review gaps Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * fix: harden extension update preflight and rollback Assisted-by: OpenAI Codex (model: GPT-5, autonomous) * fix: harden extension update rollback Assisted-by: OpenAI Codex (model: GPT-5, autonomous) |
||
|
Noor ul ainandClaude Opus 4.8
|
962f9f0765 |
fix(workflows): escape remaining untrusted fields in workflow info (#3731)
* fix(workflows): escape remaining untrusted fields in `workflow info` Follow-up to #3690, which escaped only the step-graph brackets. Every other metadata field `workflow info` prints is untrusted content (workflow.yml or catalog JSON), and console.print has Rich markup enabled, so an unescaped `[...]` in any of them is parsed as a style tag and silently swallowed: - definition path: name, version, author, description, integration, and each input's name/type - catalog path: name, version, description, tags, and the "not found" workflow id A description of `Does [stuff] nicely` rendered as `Does nicely`; an integration of `claude [code]` rendered as `claude `. Route every field through _escape_markup, matching the sibling `workflow list` / catalog `search` commands, so bracketed text renders literally. Add two regression tests covering the definition and catalog paths; both fail on the pre-fix source (fields with brackets come back truncated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: cover version + not-found-id escapes in workflow info Addresses Copilot review feedback on the workflow-info markup-escape tests: - The definition-path and catalog-path regression tests left `version` bracket-free and never asserted it, so the version escapes could be removed without failing. Use bracketed version values and assert they survive verbatim. - The newly escaped not-found identifier is a separate output path that no test reached. Add a case where local load raises FileNotFoundError and catalog lookup returns None, invoke `workflow info` with a bracketed ID, and assert the literal ID is preserved in the error. Verified each new assertion fails when its source escape is removed (test-the-test). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Noor ul ainandClaude Opus 4.8
|
99dc915ae3 |
fix: escape Rich markup in catalog list output (#3738)
The `catalog list` subcommands for workflows, workflow steps, presets, and integrations printed user-editable catalog fields (name/url/ description from the `*-catalogs.yml` files) through `console.print` with Rich markup enabled. Any bracketed content such as a description `Does [stuff] nicely` was parsed as a style tag and silently swallowed, and a malformed tag could raise while rendering. Route each untrusted field through the module's already-imported `escape` helper, matching the pattern already used by `extension catalog list`. Adds regression tests for all four commands that inject bracketed name/url/description and assert the brackets survive verbatim in the output. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Ali jawwadandClaude Opus 4.8
|
103ad73775 |
fix(workflows): guard non-mapping 'workflow:' block in WorkflowDefinition (#3694)
* fix(workflows): guard non-mapping 'workflow:' block in WorkflowDefinition
A present-but-non-mapping top-level `workflow:` block (bare `workflow:` ->
YAML null, or `workflow: <str>` / `workflow: [..]`) crashed
WorkflowDefinition.__init__ with AttributeError: the `{}` default of
`data.get("workflow", {})` only applies when the key is ABSENT, so a non-dict
value reached `workflow.get("id", ...)`. This fires inside from_yaml/
from_string — before validate_workflow can report the malformed shape — and
in the CLI escapes as a raw traceback (load_workflow is wrapped to catch only
FileNotFoundError/ValueError).
Normalize the local `workflow` to {} when it is not a mapping (self.data keeps
the raw value so validate_workflow still reports it), mirroring the adjacent
default_options guard.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(workflows): assert self.data preserves the raw non-mapping workflow value
Address review: the previous assertion only proved the key stayed present; it
would pass even if construction replaced the malformed value with {}. Assert
definition.data["workflow"] equals the original parsed value and is still a
non-mapping, proving the guard normalizes only the local variable.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwadandClaude Opus 4.8
|
ea6843c1fe |
fix(workflows): guard non-mapping 'inputs:' block in engine._resolve_inputs (#3696)
execute()/resume() run UNVALIDATED definitions (load_workflow does not
validate). WorkflowDefinition stores `inputs` raw, so a non-mapping
`inputs:` block (bare `inputs:` -> None, or `inputs: []`) crashed
_resolve_inputs at `for name, input_def in definition.inputs.items()` with
AttributeError, aborting the whole run.
Return {} when inputs is not a mapping, mirroring validate_workflow's own
`isinstance(definition.inputs, dict)` check. Protects both call sites
(execute and resume); normal dict resolution is unchanged.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwadandClaude Opus 4.8
|
0b6bf865c1 |
fix(workflows): escape step-graph brackets in workflow info so the type shows (#3690)
`workflow info` rendered each step as `→ <id> [<type>]`, but console.print has Rich markup enabled, so `[<type>]` was parsed as a style tag named after the step type (command/gate/prompt/…) and silently swallowed — every step printed as `→ <id> ` with the type gone. Escape the literal bracket with `\[` (and escape id/type via _escape_markup, as the sibling workflow_list does), so Rich renders `[<type>]` literally. Mirrors the in-file `\[disabled]` precedent. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Ali jawwadandClaude Opus 4.8
|
043c4ec572 |
fix(workflows): filter parser rejects trailing tokens (fullmatch, not match) (#3689)
_apply_filter parsed a name(arg) filter with an UNANCHORED regex (re.match(r"(\w+)\((.+)\)")), so any tokens after the closing paren were silently discarded. Because _evaluate_simple_expression splits the top-level pipe before comparison/boolean operators, `count | default(0) > 5` was split into value `count` and filter segment `default(0) > 5`; the segment matched as `default(0)` and `> 5` vanished — the filter's value was returned as the whole expression, giving a silently wrong result. Use re.fullmatch so a mis-wired segment falls through to the existing "unsupported form" ValueError, mirroring the from_json branch's strict trailing-token handling. The greedy `.+` still matches legitimate forms (literal `)` / `|` inside quoted args), so registered/chained/quoted-pipe filters are unaffected. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Ali jawwadandClaude Opus 4.8
|
37041087dd |
docs(workflows): gate step docstring lists the 'retry' on_reject behaviour (#3656)
The GateStep docstring said on_reject "controls abort / skip behaviour", omitting the third value. validate() accepts 'abort', 'skip', or 'retry', and execute() has a dedicated retry branch (returns PAUSED so the next resume re-runs the gate) distinct from abort (FAILED) and skip (COMPLETED). Add 'retry' to the docstring so it matches the same file's validate() and execute() authority. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Pascal THUET
|
3a7a8758f7 |
fix: harden bounded reads and redirect validation (#3671)
Assisted-by: OpenAI Codex (model: GPT-5, autonomous) |
||
|
Ali jawwadandClaude Opus 4.8
|
3356161d88 |
docs(workflows): init step docstring lists the 'py' script type (#3655)
The InitStep `script` field docstring claimed only 'sh' or 'ps', but the
step's own VALID_SCRIPT_TYPES = tuple(SCRIPT_TYPE_CHOICES.keys()) is
('sh', 'ps', 'py') and validate() accepts all three (its error message is
built from VALID_SCRIPT_TYPES). Update the docstring to list 'py' too, so
it no longer contradicts the same class's validate() authority.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Pascal THUET
|
5601830ba3 |
harden: bound HTTP reads and enforce strict redirects (#3140)
* harden: bound HTTP reads and enforce strict redirects Add a shared _download_security module (read_response_limited, is_https_or_localhost_http, size constants) and route the GitHub release and Azure DevOps token network reads through bounded reads so an oversized response can't exhaust memory. Add a strict_redirects mode to authentication.open_url: the redirect handler now rejects any redirect whose target isn't HTTPS (or HTTP to localhost), composing with the existing per-hop redirect_validator and auth-stripping. The Azure DevOps token POST is routed through that handler so a 307/308 cannot forward the client_secret body to a non-HTTPS host. Assisted-by: Codex (model: GPT-5, autonomous) * test: align HTTP fakes with bounded reads Assisted-by: Codex (model: GPT-5, autonomous) * fix: tolerate invalid token response encoding Assisted-by: Codex (model: GPT-5, autonomous) * test: align GHES fakes with bounded reads Assisted-by: Codex (model: GPT-5, autonomous) * test: reuse shared upgrade HTTP response helper Assisted-by: Codex (model: GPT-5, autonomous) * fix: include rejected redirect target in error Assisted-by: Codex (model: GPT-5, autonomous) * fix: enforce strict redirects by default Assisted-by: Codex (model: GPT-5, autonomous) * fix: close redirect credential and SSRF gaps Assisted-by: Codex (model: GPT-5, autonomous) |
||
|
Ali jawwadandClaude Opus 4.8
|
0add7131c9 |
fix(workflows): workflow add detects local YAML files case-insensitively (#3633)
`workflow add` gated the local-file branches on a case-SENSITIVE
`.suffix in (".yml", ".yaml")` (the `--dev` branch and the plain
local-path branch), while every other YAML-file detector in the CLI
normalizes case: `workflow run` uses `source_path.suffix.lower()` and
`WorkflowEngine.load_workflow` uses `path.suffix.lower()`.
The result was an add/run inconsistency: `specify workflow run Sample.YAML`
loads the file, but `specify workflow add Sample.YAML` does not recognize
it as a local workflow — the `--dev` branch rejects it with "--dev source
must be a workflow YAML file ..." and the plain path falls through to a
catalog lookup that fails with "not found in catalog".
Add `.lower()` to both suffix reads so `workflow add` matches its siblings.
The lowercase happy path is unchanged.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwadandClaude Opus 4.8
|
cef00a1cb3 |
fix(workflows): list-literal expression ignores trailing/empty commas (#3631)
A workflow list-literal expression with a trailing (or leading/double) comma —
'{{ [1, 2,] }}' — evaluated to [1, 2, None]: _split_top_level_commas returns a
trailing empty segment, which _evaluate_simple_expression resolves as an empty
dot-path to None. That silently widens membership tests and renders a stray
None in joins. Python and Jinja2 both tolerate trailing commas.
Drop whitespace-empty segments from the comprehension. An intentional
empty-string element ('') survives because its segment strips to "''" (truthy),
so ['', 'a'] is preserved. Completes the quoted-comma handling from #3134.
Test: [1, 2,] and [1,, 2] -> [1, 2]; ['', 'a'] -> ['', 'a'] (fails before:
trailing None).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwadandClaude Opus 4.8
|
03f9013a7b |
fix(workflows): StepRegistry.add tolerates a corrupted non-dict existing entry (#3630)
StepRegistry.add read existing = self.data['steps'].get(step_id, {}) then called
existing.get('installed_at', ...). A corrupted-but-parseable registry holding a
non-dict entry (e.g. {'steps': {'foo': 'corrupted'}}) — which _load() accepts,
since it validates only the top-level dict and that 'steps' is a dict — made
add() raise AttributeError. WorkflowRegistry.add was hardened for exactly this
(#3419); mirror its isinstance guard so a non-dict existing entry is treated as
absent.
Test copies the WorkflowRegistry sibling test for StepRegistry (fails before:
AttributeError on existing.get()).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwadandClaude Opus 4.8
|
aee9df00d4 |
fix(workflows): command/prompt steps fail cleanly on a non-string integration (#3626)
_try_dispatch guarded only 'if not integration_key', then called
get_integration(integration_key). A non-string integration (a list/dict, or an
expression like integration: "{{ steps.pick.output.agents }}" that resolves to a
list) reached the registry dict lookup and raised 'TypeError: unhashable type:
list', aborting the entire workflow run. Widen the guard to also require a str,
so a non-string integration is treated as not-dispatchable and execute() falls
through to its existing FAILED StepResult (unconfigured integration=None still
returns None as before). Applied to both command and prompt steps.
Tests: a list integration now yields a FAILED result (fail before: TypeError).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwadandClaude Opus 4.8
|
9ef477167d |
fix(workflows): gate prompt uses isdecimal() so a superscript digit doesn't crash (#3624)
The interactive gate prompt guarded numeric choices with raw.isdigit(), but
str.isdigit() returns True for characters int() rejects — superscripts/subscripts
like '²'. So typing '²' passed the guard and int('²') raised an uncaught
ValueError, crashing the prompt loop. Use raw.isdecimal(), which is exactly the
decimal-digit set int() accepts (Numeric_Type=Decimal), so such input is treated
as an invalid choice and re-prompted. No behavior change for valid input.
Test: input '²' then '1' returns the first option (fails before: ValueError).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Quratulain-bilal
|
30e99ec083 |
fix(workflows): validate every redirect hop when fetching workflow/step catalogs (#3637)
WorkflowCatalog._fetch_single_catalog and StepCatalog._fetch_single_catalog opened the catalog URL with open_url(entry.url, timeout=30) and validated only the final resp.geturl(). open_url follows redirects, so an https:// catalog entry that 30x-redirects through a non-HTTPS host mid-chain could let a network attacker rewrite the next hop and slip a payload past the terminal-URL-only check. The payload then drives step/workflow catalog data. Pass a redirect_validator that runs the existing HTTPS/hostname check before every redirect hop, keeping the final geturl() check as a defense-in-depth backstop. This brings both workflow catalog loaders to parity with the presets (#3523) and extensions (#3524) catalog fetchers. Tests: add per-hop redirect-validation tests for both WorkflowCatalog and StepCatalog (a non-HTTPS intermediate hop is rejected); both fail before the fix ("NoneType object is not callable" — no validator passed). Update the two existing malformed-redirect tests whose open_url stub lacked the redirect_validator kwarg. |
||
|
Noor ul ainandClaude Opus 4.8
|
48686521ff |
fix(workflows): reject a non-string 'integration'/'model' in command & prompt steps (#3597)
* fix(workflows): reject a non-string 'integration'/'model' in command & prompt steps
A non-string `integration` on a command or prompt step is passed to
`get_integration()`, which uses it as a dict key: an unhashable list/dict
raises a raw `TypeError` there — and because neither `validate()` nor
`validate_workflow` checked the type, this crashes even a *validated* run,
not just an unvalidated one. A non-string `model` likewise reaches
`build_exec_args()` and is fed into the CLI argv.
Guard both fields in `validate()` (reject a literal non-string, mirroring the
existing 'command'/'prompt'/'input'/'options' checks) and in `execute()`
(fail the step cleanly rather than take down the whole run, mirroring the
'input'/'options' guards). An explicit YAML-null (inherit the workflow
default) and a "{{ ... }}" expression both stay valid.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(workflows): route falsey non-string integration/model to the type guard
Address Copilot review: `config.get("integration") or context.default_integration`
(and the model equivalent) coerced a *falsey* non-string ([], {}, 0, False) into
the workflow default before the type guard ran. On an unvalidated execute() such a
step was silently accepted and — with a configured default — could dispatch using
the wrong integration/model instead of failing with the contract error.
Fall back to the workflow default only for genuinely-unset values (missing /
YAML-null / empty string) so every non-string reaches the guard. Add parametrized
falsey execute() cases ([], {}, 0, False) to both TestCommandStep and
TestPromptStep; with the fix stashed all 8 fail (swallowed into the default).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Noor ul ainandClaude Opus 4.8
|
d7699c39f2 |
fix(workflows): reject non-list input 'enum' instead of crashing (#3601)
A workflow input whose `enum` is a scalar or string (e.g. `enum: 5`,
`enum: "abc"`) previously slipped past `validate_workflow` and crashed
at run time. The `value not in enum_values` membership test in
`_coerce_input` raises a raw `TypeError` ("argument of type 'int' is
not iterable") for a scalar, and a bare string turns enum membership
into a silent substring test. The `TypeError` also escapes
`validate_workflow`'s `except ValueError`, breaking its documented
"return a list of errors, never raise" contract.
This is the same unvalidated-`execute()` crash class as the fan-in
`wait_for` (#3482) and fan-out step-template (#3537) fixes: `validate()`
should reject the value, but the value can still reach the engine via
`execute()`, which accepts unvalidated definitions.
Fix:
- `_coerce_input` requires a list `enum` (or `None`), raising a clean
ValueError for any other shape — so both `validate_workflow` and
runtime `_resolve_inputs` fail fast with a clear message.
- `validate_workflow` checks `enum` shape directly (not only via the
default-coercion path, which is reached only when a `default` exists),
and strips a malformed `enum` before coercing the default so the
wrong-typed-default error is not duplicated as an enum-shape error.
- The `integration: auto` sentinel only strips a *list* `enum`; a
non-list `enum` stays in the definition so it is rejected rather than
silently exempted by the `auto` membership skip.
Tests cover all three layers: `_coerce_input` directly, authoring-time
`validate_workflow` (with no default present), and runtime
`_resolve_inputs`, plus the `integration: auto` interaction.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Noor ul ainandClaude Opus 4.8
|
70c547cfab |
fix(workflows): reject a non-string 'command' in command-step (#3596)
`CommandStep.validate` only checked that a `command` field is *present*,
never its type. On an unvalidated run (the engine does not auto-validate
before `execute`) a non-string `command` — null, a list, an int — was
passed straight through `_try_dispatch` to the integration's
`build_command_invocation`, which does `command_name.startswith("speckit.")`
and crashes the whole workflow with a raw `AttributeError` once a
resolvable integration with an installed CLI is found.
Guard both paths, mirroring the sibling steps:
- `validate()` rejects a non-string `command` (like prompt-step `prompt`
#3582 and shell-step `run`).
- `execute()` fails the step cleanly with the same contract error before
dispatch (like the existing `input`/`options` guards in this file), so
an unvalidated run FAILs the step instead of crashing the run.
An expression like `{{ inputs.cmd }}` is still a string, so it stays valid.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Noor ul ainandClaude Opus 4.8
|
2f9e45514c |
fix(workflows): fail gate step loudly on a malformed 'options' (#3595)
`GateStep.validate` rejects a non-list (or empty) `options` and requires every option to be a string, but the engine does not auto-validate before `execute`. On an unvalidated run a scalar/dict/None `options` reached `_prompt` and crashed the whole workflow with a raw `TypeError` (`enumerate`/`len` on a non-iterable) or `KeyError` (indexing a dict); an empty list spun `_prompt`'s input loop forever; a non-string option crashed the reject check at `choice.lower()` with `AttributeError`. Guard `execute` to FAIL the step cleanly instead, before the non-TTY PAUSE short-circuit so the error surfaces in CI too rather than pausing and only crashing later on interactive resume. Mirrors the switch 'cases' and command 'input' unvalidated-execute guards. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
d6fa0460ed |
feat(workflows): WorkflowResolver standalone (PR 1) (#3557)
* feat(workflows): add standalone WorkflowResolver and overlay subsystem Implement PR 1 of the workflow-overlays plan: a concrete, standalone WorkflowResolver for downstream workflow extensibility without touching the Preset subsystem. - Add overlay manifest schema (Overlay, OverlayEdit, validate_overlay_yaml) - Add pure-function merge engine (find_step, apply_edit, merge_steps, validate_edits) with recursive anchor search and higher-wins semantics - Add StepListComposer and tiered layer sources (project, installed, base) - Add WorkflowResolver facade with inline HIGHER_WINS priority sorting - Add CLI verbs: workflow overlay add/set-priority/enable/disable/remove/list and workflow resolve <id> - Wire WorkflowEngine.load_workflow through WorkflowResolver - Extend workflow add to copy optional overlays/ subdirectory from local workflow directories - Add comprehensive unit, integration, and security tests Refs: discussion #3473 (https://github.com/github/spec-kit/discussions/3473) Assisted-by: Kimi (model: opencode-go/kimi-k2.7-code, autonomous) * fix(workflows): reject symlinked overlay directories in layer sources Address PR #3557 review comments r3594064534 and r3594064563: - ProjectOverlaySource.collect now rejects symlinked per-workflow overlay directories (.specify/workflows/overlays/<id>) before iterating - InstalledOverlaySource.collect now rejects symlinked installed overlay directories (.specify/workflows/<id>/overlays) before iterating - workflow_overlay_list catches ValueError from resolver and exits with code 1 instead of crashing on unhandled exceptions - Added .specify/workflows/overlays to _reject_unsafe_workflow_storage chokepoint for defense-in-depth These guards prevent symlinked overlay directories from redirecting auto-loaded overlay YAML to attacker-controlled content outside the project, which could inject executable shell steps into trusted workflows. Refs: PR #3557 review comments r3594064534, r3594064563 Assisted-by: opencode-go/qwen3.7-max (autonomous) * fix(workflows): address Copilot review findings in merge engine - Apply inserts before winning replace to prevent anchor-not-found errors when replace changes step ID (r3594064604) - Track attribution recursively for nested steps in composite inserts/replaces so workflow resolve attributes all child steps correctly (r3594064638) - Add regression tests for both fixes Refs: PR #3557 review discussion Assisted-by: GitHub Copilot (model: qwen3.7-plus, autonomous) * refactor(workflows): simplify overlay architecture to 2-tier Remove installed overlays tier to enforce clean separation of concerns: - workflow add installs workflows only (no overlay copying) - workflow overlay add installs overlays only (project-local) Changes: - Remove InstalledOverlaySource class and all references - Remove overlay-copying logic from _validate_and_install_local() - Update WorkflowResolver to 2-tier: project overlays + base workflow - Fix --priority override timing: apply before validation, not after - Remove tests for installed overlays (no longer applicable) Rationale: If upstream controls both base workflow and shipped overlays, and both get overwritten on bundle update, there's no reason to ship overlays separately. Overlays only make sense when someone other than the base author adds them. Resolves all three review findings from PR #3557: - r3594064677: workflow add no longer copies overlays from all call sites - r3594064705: --priority override now applied before validation - r3594064726: no stale installed overlays (tier removed entirely) Assisted-by: Claude (model: claude-opus-4-7, autonomous) * fix(workflows): harden overlay symlink handling Assisted-by: GitHub Copilot (model: GPT-5.4, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(workflows): remove stale installed-overlay references from workflows.md The 2-tier refactor ( |
||
|
Ali jawwadandClaude Opus 4.8
|
57cc518d63 |
fix(workflows): reject bool / .inf catalog priority in workflow & step catalog loaders (#3526)
* fix(workflows): reject bool/.inf catalog priority in workflow & step catalog loaders
The WorkflowRegistry and StepRegistry catalog-config loaders coerced priority
with int() inside except (TypeError, ValueError), missing two guards the base
CatalogStackBase loader already has:
- bool is an int subclass, so 'priority: true' was silently coerced to 1;
- int(float('inf')) raises OverflowError (not caught), so 'priority: .inf'
crashed with an uncaught traceback.
Add the explicit bool check and OverflowError to both loaders, and add
OverflowError to the two _coerce_priority helpers used by 'catalog add' (they
return 0 on an uncoercible existing priority instead of crashing).
Parametrized tests on both TestWorkflowCatalog and TestStepCatalog reject
priority true/false/.inf (fail before: bool coerced to 1 / inf OverflowError).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(workflows): cover add_catalog() OverflowError fallback for existing priority: .inf
The workflow/step catalog priority guards added OverflowError to _coerce_priority
(the 'catalog add' fallback), but the tests only exercised get_active_catalogs().
Add tests that prewrite an existing 'priority: .inf' entry and call add_catalog()
for both WorkflowCatalog and StepCatalog, asserting the command succeeds and the
new entry gets a valid priority (inf coerced to 0, +1). Fails before: int(inf)
OverflowError crashed add_catalog.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Noor ul ainandClaude Opus 4.8
|
3d2901eb75 |
fix(workflows): fail fan-in loudly on a non-string wait_for entry (#3579)
`FanInStep.execute` already guards a non-list `wait_for` (#3482), and the engine's load-time validation rejects non-string entries. But the engine does not auto-validate step config, so on an unvalidated run `execute` iterated the list's *elements* raw: - An unhashable entry (a list/dict from a YAML indentation slip like `wait_for: [[a, b]]`) crashed the whole run at `context.steps.get(entry, ...)` with a raw `TypeError: cannot use 'list' as a dict key`. - A hashable-but-non-string entry (`wait_for: [123]`) silently joined an empty `{}` and still reported COMPLETED — the exact "silent empty result + COMPLETED" wiring bug the whole-list guard and the engine's fan-in validation both exist to prevent. Extend the execute() guard to reject any non-string entry with the engine's "entries must be step-id strings" phrasing, mirroring the sibling non-list guard right above it. Adds regression coverage for unhashable and hashable-non-string entries. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Noor ul ainandClaude Opus 4.8
|
c1e5cfa0aa |
fix(workflows): fail fan-out loudly on a truthy non-mapping step template (#3537)
* fix(workflows): fail fan-out loudly on a truthy non-mapping step template
A fan-out step whose `step:` is a truthy scalar or list (an authoring mistake) passed execute and reached the engine, which calls template.get("id", ...) in _run_fan_out — raising AttributeError and taking down the whole run. validate already rejects a non-mapping step, but the engine does not auto-validate, so an unvalidated run crashed.
Guard execute to FAIL the step (with a clear error and normalized empty output) instead, mirroring the existing non-list items guard and the switch non-dict cases guard. Add the matching test_execute_non_dict_step_fails_loudly covering the execute-path guard (validate was already covered).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(workflows): reject explicit fan-out `step: null` in validate()
The runtime guard in execute() rejects a truthy non-mapping step, but
`config.get("step", {})` only substitutes the `{}` default for an *absent*
key — an explicit `step: null` reaches the guard as None and FAILS the step.
validate() previously exempted None (`step is not None and ...`), so such a
workflow passed validation and then failed during execution.
Align validate() with the runtime guard: a present-but-non-mapping `step`
(including `None`) is an authoring mistake and is now rejected up front.
Extend the validate and execute regression cases to cover None.
Addresses Copilot review feedback on #3537.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Noor ul ainandClaude Opus 4.8
|
b139bd0393 |
fix(workflows): reject a non-string prompt in prompt-step validate() (#3582)
`PromptStep.execute` str()-coerces `config['prompt']` and dispatches the
result to the integration CLI as the model's instructions. But its `validate`
only checked that `prompt` was *present*, not that it was a string — the exact
parity gap the sibling `ShellStep` closes for `run`.
So a YAML authoring slip like `prompt: [review, this]` or `prompt:` (null)
passed validation, then `execute` sent the Python repr (`"['review', 'this']"`,
`"None"`) to the LLM verbatim — silently wrong instructions with no error and a
COMPLETED status. The engine does not auto-validate step config
(`load_workflow` explicitly defers validation), so validation is the only place
this surfaces before dispatch.
Extend `validate` to reject any non-string `prompt` with the shell-step's
phrasing ("'prompt' must be a string, got <type>"), mirroring the shell `run`
and command `input`/`options` type checks. A `{{ ... }}` expression is still a
str, so it stays valid. Adds regression coverage for non-string prompts
(null/list/int/dict) and confirms an expression prompt still validates.
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Ali jawwadandClaude Opus 4.8
|
f75f5f836b |
fix(workflows): route 'workflow status --json' errors to stderr (#3520)
* fix(workflows): route 'workflow status --json' errors to stderr The workflow_status run_id error paths (FileNotFoundError -> 'Run not found', ValueError -> invalid run) used the stdout console and fired before the json_output branch, so 'specify workflow status <bad-id> --json' wrote a Rich-rendered error to stdout and corrupted the JSON stream a consumer would json.loads(). Route both through _error_console(json_output) so they go to stderr under --json, matching the sibling 'workflow run'/'workflow resume' commands (which use the identical RunState.load try/except) and the documented stdout-purity contract. Test asserts the not-found error appears on stderr and stdout stays empty under --json (fails before: the error was on stdout). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(workflows): cover the ValueError handler in workflow status --json purity The stderr-routing fix reroutes both the FileNotFoundError and ValueError run_id handlers, but the test only exercised FileNotFoundError — a regression of the ValueError path back to stdout would have gone uncaught. Add a ValueError case (RunState.load raising) asserting the same stderr-only / empty-stdout behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |