Skip to content

fix(workflows): report a run failed by a raised step error under --json - #5015

Merged
mnriem merged 2 commits into
github:mainfrom
yxshee:fix/workflow-json-step-error
Oct 19, 2026
Merged

mnriem merged 2 commits into
github:mainfrom
yxshee:fix/workflow-json-step-error

Conversation

@yxshee

@yxshee yxshee commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Description

docs/reference/workflows.md says specify workflow run --json prints a single object that "always parses". For failed runs, that object includes an error field. When a step raises instead of returning a failed result, run --json and resume --json print nothing on stdout. Examples: from_json on invalid JSON in a condition, or an exception from a custom step.

The engine has already saved the run as failed with that error, so workflow status --json reports it. But a caller parsing stdout gets an empty string and never learns the run ID it needs to resume.

# wjson.yml
schema_version: "1.0"
workflow: {id: "wjson", name: "JSON repro", version: "1.0.0"}
inputs:
  payload: {type: string, default: "not-json"}
steps:
  - id: count
    type: if
    condition: "{{ inputs.payload | from_json | length }}"
    then:
      - {id: noop, type: shell, run: "echo yes"}
# main
$ specify workflow run wjson.yml --json
exit=1
stdout: (empty)
stderr: Error: from_json: invalid JSON: Expecting value: line 1 column 1 (char 0)
$ specify workflow status 5095af24 --json   # run ID found by listing .specify/workflows/runs
{"run_id": "5095af24", "status": "failed", "error": "from_json: invalid JSON: ...", ...}

# this branch
$ specify workflow run wjson.yml --json
exit=1
stdout:
{
  "run_id": "ea2926a8",
  "workflow_id": "wjson",
  "status": "failed",
  "current_step_id": "count",
  "current_step_index": 0,
  "error": "from_json: invalid JSON: Expecting value: line 1 column 1 (char 0)"
}
stderr: (empty)

Root cause:

  • WorkflowEngine.execute() / resume() persist an exception raised while steps run as the run's failed status and error, then re-raise it.
  • The except ValueError and catch-all handlers in command_run.py and command_resume.py only print the message, which goes to stderr under --json.
  • An OSError at the same point already goes through _fail_workflow_execution(), which prints the failed run's payload.

The fix:

  • A new _fail_recorded_run() in workflows/_commands.py, called from both handlers in run and resume.
  • When the execution state is failed and its error equals the exception's message, it reports through _fail_workflow_execution().
  • Errors raised before the run is recorded keep their current output. Examples: a missing required input on run, or an invalid --input on resume of an already-failed run.
  • One visible change in text mode: a recorded failure now reads "Workflow failed:" / "Resume failed:", as I/O failures already do, instead of "Error:".

Testing

  • Tested locally with uv run specify --help
  • Ran existing tests with uv sync && uv run pytest
  • Tested with a sample project (if applicable): the reproduction above, on main and on this branch

New tests in tests/specify_cli/workflows/test_command_run.py:

  • test_run_json_step_exception_reports_failed_run
  • test_run_json_non_value_error_reports_failed_run (a step raising RuntimeError)
  • test_run_step_exception_reports_failed_run_in_text
  • negative: test_run_json_missing_required_input_is_not_a_run_outcome

New tests in tests/specify_cli/workflows/test_command_resume.py:

  • test_resume_json_step_exception_reports_failed_run
  • negative: test_resume_json_refused_before_running_is_not_a_run_outcome

On main, the 4 positive tests fail: the JSON ones with json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0), the text one because "Workflow failed:" is missing. The 2 negative tests pass. With the change both files give 91 passed.

  • Workflows suites plus the external-adapter tests: 2644 passed, 27 skipped.
  • Full suite (macOS, Python 3.13): 12336 passed, 376 skipped. 11 failures in the tests/test_*_python_parity.py files occur identically on unmodified main on this machine under full-suite load; those files pass when run on their own (257 passed, 112 skipped).
  • uvx ruff@0.15.0 check src tests: clean

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (fill in the disclosure below)

AI disclosure: Claude Code (model: Claude Opus 5.5, max reasoning effort, autonomous agent mode) opened this PR on behalf of @yxshee. A Claude Code subagent (same model, autonomous) found the bug and wrote the fix and tests. The lead agent independently re-ran the new tests against main and the branch, reproduced the transcripts above with the real CLI, reviewed the diff, and wrote this description. No human reviewed the change line by line before it was opened.

🤖 Generated with Claude Code

`workflow run --json` and `workflow resume --json` printed nothing on
stdout when a step raised instead of returning a failed result, for
example `from_json` on invalid JSON in a condition, or an exception from
a custom step. The engine had already saved the run as `failed` with
that error before re-raising it, and `workflow status --json` reported it
that way, but the CLI's ValueError and catch-all handlers only printed
the message (to stderr under --json). A caller parsing stdout got an
empty string and never learned the run ID it needs to resume. An I/O
failure at the same point already prints the failed run's payload
through `_fail_workflow_execution`.

Send an exception through that reporter when the engine recorded it as
the run's failure: the state is `failed` and its error is this
exception's message. Errors raised before the run is recorded, such as a
missing required input on `run` or an invalid `--input` on `resume`,
keep their current output. In text mode the recorded failure is now
labelled "Workflow failed:" or "Resume failed:", as I/O failures are,
instead of "Error:".

Assisted-by: Claude Code (model: Claude Opus 5.5, autonomous)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@yxshee
yxshee requested a review from mnriem as a code owner October 17, 2026 19:21
@mnriem mnriem added the triage-must-have Verdict: high-value, important work for Spec Kit — do first label Oct 18, 2026
@mnriem
mnriem requested a max review from Copilot October 19, 2026 13:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Matching error text can misreport a rejected resume as a newly recorded execution failure.

1 open issue
What changed in this PR

Updates workflow CLI failure reporting so raised step errors produce saved run details under --json.

Changes:

  • Shares recorded-failure reporting between run and resume.
  • Adds regression tests for raised errors and pre-execution rejections.

Validation used an isolated helper check; project tests and baseline comparisons were not rerun.

AI disclosure: GitHub Copilot (model unknown, autonomous) generated this review for the requester.

File Description
tests/​specify_cli/​workflows/​test_command_run.py Tests raised errors, text output, and missing inputs.
tests/​specify_cli/​workflows/​test_command_resume.py Tests resumed errors and rejected input updates.
src/​specify_cli/​workflows/​command_run.py Routes recorded exceptions through shared reporting.
src/​specify_cli/​workflows/​command_resume.py Applies shared reporting to resumed failures.
src/​specify_cli/​workflows/​_commands.py Adds the recorded-failure reporting helper.

Review effort: Max

Comment thread src/specify_cli/workflows/_commands.py Outdated
…essage

resume() exposes the loaded failed state before validating updated
inputs, so a refused resume whose error text matched the previous
failure (for example the same "Input 'count' expected a number" message)
was reported as the run's outcome: the old run's JSON on stdout and no
diagnostic on stderr.

Have execute() and resume() mark the exception they record as the run's
failure on the in-memory state (never persisted), and report a run
outcome only when that marker is the exception being handled.

Assisted-by: Claude Code (model: Claude Opus 5.5, autonomous)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@yxshee

yxshee commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Review round 1, posted on behalf of @yxshee.

Addressed the Copilot finding that a refused resume with a matching error message was reported as the run's outcome (commit 109d147):

  • The engine marks the exception it records as the run's failure on the in-memory state (not persisted); the CLI reports a run outcome only for that exact exception.
  • New negative test test_resume_json_refused_with_matching_old_error_is_not_a_run_outcome (fails on the previous head, passes now).
  • tests/specify_cli/workflows, tests/test_workflows.py: 1468 passed, 27 skipped. uvx ruff@0.15.0 check src tests: clean.

AI disclosure: written and pushed by Claude Code (model: Claude Opus 5.5, max reasoning effort, autonomous agent mode); the code, test and this comment are AI-generated.

🤖 Addressed by Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused exception-identity guard preserves validation diagnostics, and the remaining feedback is non-blocking.

No open issues 🎉

1 resolved since last review

Review effort: Max

@mnriem
mnriem merged commit 103e7c3 into github:main Oct 19, 2026
16 checks passed
@mnriem

mnriem commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-must-have Verdict: high-value, important work for Spec Kit — do first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants