Skip to content

Commit af021d2

Browse files
jawwad-aliclaude
andauthored
fix(workflows): init step must not replace init's own error with "SystemExit: 1" (#4530)
`InitStep._run_init` appended the runner's exception to stderr: if result.exit_code != 0 and result.exception is not None: detail = f"{type(result.exception).__name__}: {result.exception}" stderr = f"{stderr}\n{detail}".strip() if stderr else detail That branch is written for an unexpected crash, but `typer.Exit(n)` -- how `specify init` reports every ordinary failure -- surfaces through `CliRunner` as `result.exception = SystemExit(n)`, so it fired on routine errors too. `init` prints its diagnostics through Rich to stdout, so `result.stderr` is empty and the synthesized detail became the ENTIRE stderr, preempting `execute`'s fallback: error=(stderr.strip() or stdout.strip() or f"specify init exited ...") `stdout.strip()` -- which holds the real message -- was never reached. Reproduced on main with an ordinary typo in `integration:` (which `InitStep.validate` does not value-check): validate : [] status : failed | exit_code: 1 result.error : 'SystemExit: 1' output.stderr: 'SystemExit: 1' stdout holds : "... lingma, muse, omp, opencode, pi, qodercli, qwen, rovodev, shai, tabnine, trae, vibe, zcode, zed" Now excludes only `SystemExit`, so an ordinary non-zero exit falls through to init's own output while a genuine crash still reports its exception. Rebased onto current main (files moved in the workflow/bundler restructure). Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 2dda047 commit af021d2

2 files changed

Lines changed: 100 additions & 1 deletion

File tree

‎src/specify_cli/workflows/step/init/__init__.py‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,9 +295,34 @@ def _run_init(
295295
# steps.<id>.output.stderr for error details.
296296
stderr = stdout if result.exit_code != 0 else ""
297297

298-
if result.exit_code != 0 and result.exception is not None:
298+
# Record the exception only for an UNEXPECTED crash. ``typer.Exit(n)``
299+
# -- how ``specify init`` reports every ordinary failure -- surfaces
300+
# through ``CliRunner`` as ``result.exception = SystemExit(n)``, so this
301+
# branch used to fire on routine errors too. ``init`` prints its
302+
# diagnostics through Rich to stdout, leaving ``result.stderr`` empty,
303+
# so the synthesized "SystemExit: 1" became the whole of ``stderr`` and
304+
# preempted ``execute``'s ``stderr.strip() or stdout.strip()`` fallback.
305+
# Every failing init step then reported ``error: 'SystemExit: 1'`` while
306+
# init's real message -- e.g. the list of valid integrations for a
307+
# typo'd ``integration:`` -- sat unread in stdout.
308+
if (
309+
result.exit_code != 0
310+
and result.exception is not None
311+
and not isinstance(result.exception, SystemExit)
312+
):
299313
detail = f"{type(result.exception).__name__}: {result.exception}"
300314
stderr = f"{stderr}\n{detail}".strip() if stderr else detail
315+
elif result.exit_code != 0 and not stderr.strip():
316+
# Ordinary failure with no real stderr: under click >= 8.2 the
317+
# streams are separate and ``init`` prints its diagnostics through
318+
# Rich to stdout, so ``result.stderr`` is genuinely empty. Merely
319+
# dropping the synthesized "SystemExit: 1" would leave
320+
# ``steps.<id>.output.stderr`` empty -- still no diagnostic for a
321+
# downstream step to read, and still contrary to the contract above
322+
# ("treat stdout as stderr so workflows can consistently read
323+
# steps.<id>.output.stderr for error details"). Carry the captured
324+
# output across, matching what the older-Click branch already does.
325+
stderr = stdout.strip()
301326

302327
return (result.exit_code, stdout, stderr)
303328

‎tests/test_workflows.py‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3150,6 +3150,80 @@ def test_invalid_integration_fails(self, tmp_path):
31503150
assert result.output["exit_code"] != 0
31513151
assert result.error is not None
31523152

3153+
def test_failed_init_surfaces_inits_own_message(self, tmp_path):
3154+
"""A failing init step must report init's diagnostics, not 'SystemExit: 1'.
3155+
3156+
`typer.Exit(n)` — how `specify init` reports every ordinary failure —
3157+
surfaces through `CliRunner` as `result.exception = SystemExit(n)`, so
3158+
the unexpected-crash branch fired on routine errors too. `init` prints
3159+
through Rich to stdout, leaving `result.stderr` empty, so the
3160+
synthesized "SystemExit: 1" became the whole of stderr and preempted
3161+
the `stderr.strip() or stdout.strip()` fallback — stranding the real
3162+
message (here, the list of valid integrations) in stdout.
3163+
"""
3164+
from specify_cli.workflows.step.init import InitStep
3165+
from specify_cli.workflows.base import StepContext, StepStatus
3166+
3167+
result = InitStep().execute(
3168+
{
3169+
"id": "bootstrap",
3170+
"here": True,
3171+
"integration": "no-such-agent",
3172+
"script": "sh",
3173+
},
3174+
StepContext(project_root=str(tmp_path)),
3175+
)
3176+
3177+
assert result.status == StepStatus.FAILED
3178+
assert result.error is not None
3179+
assert result.error.strip() != "SystemExit: 1"
3180+
3181+
# The real diagnostic reaches the caller...
3182+
collapsed = " ".join(result.error.split())
3183+
assert "no-such-agent" in collapsed or "Unknown" in collapsed, collapsed
3184+
3185+
# ...and reaches a downstream step reading steps.<id>.output.stderr.
3186+
# Asserting only "not the old sentinel" was too weak: an EMPTY stderr
3187+
# satisfies that while still carrying no diagnostic at all, which is
3188+
# exactly what dropping the synthesized detail left behind under
3189+
# click >= 8.2 (separate streams, init prints through Rich to stdout).
3190+
stderr = " ".join(result.output["stderr"].split())
3191+
assert stderr, "output.stderr must not be empty for a failed init"
3192+
assert stderr != "SystemExit: 1"
3193+
assert "no-such-agent" in stderr or "Unknown" in stderr, stderr
3194+
3195+
def test_unexpected_crash_still_reports_its_exception(self, monkeypatch, tmp_path):
3196+
"""The branch's original purpose is preserved for a genuine crash.
3197+
3198+
Only `SystemExit` is now excluded; any other exception escaping the
3199+
runner must still be surfaced, since nothing else would describe it.
3200+
"""
3201+
from specify_cli.workflows.step.init import InitStep
3202+
import typer.testing
3203+
3204+
class _Result:
3205+
exit_code = 1
3206+
output = ""
3207+
stderr = ""
3208+
exception = RuntimeError("boom inside init")
3209+
3210+
class _Runner:
3211+
def __init__(self, *a, **k):
3212+
pass
3213+
3214+
def invoke(self, *a, **k):
3215+
return _Result()
3216+
3217+
monkeypatch.setattr(typer.testing, "CliRunner", _Runner)
3218+
3219+
from specify_cli.workflows.base import StepContext
3220+
3221+
_code, _stdout, stderr = InitStep()._run_init(
3222+
["init", "demo"], StepContext(project_root=str(tmp_path))
3223+
)
3224+
3225+
assert "RuntimeError: boom inside init" in stderr
3226+
31533227
def test_non_empty_current_dir_without_force_fails_fast(self, tmp_path):
31543228
from specify_cli.workflows.step.init import InitStep
31553229
from specify_cli.workflows.base import StepContext, StepStatus

0 commit comments

Comments
 (0)