diff --git a/src/specify_cli/workflows/steps/init/__init__.py b/src/specify_cli/workflows/steps/init/__init__.py index 270badc4fc..5dc04484a9 100644 --- a/src/specify_cli/workflows/steps/init/__init__.py +++ b/src/specify_cli/workflows/steps/init/__init__.py @@ -295,9 +295,34 @@ def _run_init( # steps..output.stderr for error details. stderr = stdout if result.exit_code != 0 else "" - if result.exit_code != 0 and result.exception is not None: + # Record the exception only for an UNEXPECTED crash. ``typer.Exit(n)`` + # -- how ``specify init`` reports every ordinary failure -- surfaces + # through ``CliRunner`` as ``result.exception = SystemExit(n)``, so this + # branch used to fire on routine errors too. ``init`` prints its + # diagnostics through Rich to stdout, leaving ``result.stderr`` empty, + # so the synthesized "SystemExit: 1" became the whole of ``stderr`` and + # preempted ``execute``'s ``stderr.strip() or stdout.strip()`` fallback. + # Every failing init step then reported ``error: 'SystemExit: 1'`` while + # init's real message -- e.g. the list of valid integrations for a + # typo'd ``integration:`` -- sat unread in stdout. + if ( + result.exit_code != 0 + and result.exception is not None + and not isinstance(result.exception, SystemExit) + ): detail = f"{type(result.exception).__name__}: {result.exception}" stderr = f"{stderr}\n{detail}".strip() if stderr else detail + elif result.exit_code != 0 and not stderr.strip(): + # Ordinary failure with no real stderr: under click >= 8.2 the + # streams are separate and ``init`` prints its diagnostics through + # Rich to stdout, so ``result.stderr`` is genuinely empty. Merely + # dropping the synthesized "SystemExit: 1" would leave + # ``steps..output.stderr`` empty -- still no diagnostic for a + # downstream step to read, and still contrary to the contract above + # ("treat stdout as stderr so workflows can consistently read + # steps..output.stderr for error details"). Carry the captured + # output across, matching what the older-Click branch already does. + stderr = stdout.strip() return (result.exit_code, stdout, stderr) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 2c7141e954..29ba2e2536 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -2594,6 +2594,80 @@ def test_invalid_integration_fails(self, tmp_path): assert result.output["exit_code"] != 0 assert result.error is not None + def test_failed_init_surfaces_inits_own_message(self, tmp_path): + """A failing init step must report init's diagnostics, not 'SystemExit: 1'. + + `typer.Exit(n)` — how `specify init` reports every ordinary failure — + surfaces through `CliRunner` as `result.exception = SystemExit(n)`, so + the unexpected-crash branch fired on routine errors too. `init` prints + through Rich to stdout, leaving `result.stderr` empty, so the + synthesized "SystemExit: 1" became the whole of stderr and preempted + the `stderr.strip() or stdout.strip()` fallback — stranding the real + message (here, the list of valid integrations) in stdout. + """ + from specify_cli.workflows.steps.init import InitStep + from specify_cli.workflows.base import StepContext, StepStatus + + result = InitStep().execute( + { + "id": "bootstrap", + "here": True, + "integration": "no-such-agent", + "script": "sh", + }, + StepContext(project_root=str(tmp_path)), + ) + + assert result.status == StepStatus.FAILED + assert result.error is not None + assert result.error.strip() != "SystemExit: 1" + + # The real diagnostic reaches the caller... + collapsed = " ".join(result.error.split()) + assert "no-such-agent" in collapsed or "Unknown" in collapsed, collapsed + + # ...and reaches a downstream step reading steps..output.stderr. + # Asserting only "not the old sentinel" was too weak: an EMPTY stderr + # satisfies that while still carrying no diagnostic at all, which is + # exactly what dropping the synthesized detail left behind under + # click >= 8.2 (separate streams, init prints through Rich to stdout). + stderr = " ".join(result.output["stderr"].split()) + assert stderr, "output.stderr must not be empty for a failed init" + assert stderr != "SystemExit: 1" + assert "no-such-agent" in stderr or "Unknown" in stderr, stderr + + def test_unexpected_crash_still_reports_its_exception(self, monkeypatch, tmp_path): + """The branch's original purpose is preserved for a genuine crash. + + Only `SystemExit` is now excluded; any other exception escaping the + runner must still be surfaced, since nothing else would describe it. + """ + from specify_cli.workflows.steps.init import InitStep + import typer.testing + + class _Result: + exit_code = 1 + output = "" + stderr = "" + exception = RuntimeError("boom inside init") + + class _Runner: + def __init__(self, *a, **k): + pass + + def invoke(self, *a, **k): + return _Result() + + monkeypatch.setattr(typer.testing, "CliRunner", _Runner) + + from specify_cli.workflows.base import StepContext + + _code, _stdout, stderr = InitStep()._run_init( + ["init", "demo"], StepContext(project_root=str(tmp_path)) + ) + + assert "RuntimeError: boom inside init" in stderr + def test_non_empty_current_dir_without_force_fails_fast(self, tmp_path): from specify_cli.workflows.steps.init import InitStep from specify_cli.workflows.base import StepContext, StepStatus