fix(registry): exit non-zero on non-RegistryAPIError registry-install failures

[mattmillerai]

ELI-5

If your wifi dropped in the middle of comfy node registry-install, the command printed a friendly "Failed to download..." message and then told your shell everything was fine (exit code 0). A CI job or an agent watching the exit code would happily carry on as if the node had installed. This makes the command admit it failed.

What this changes

registry_install had two silent-success paths:

1. The broad except Exception — every non-RegistryAPIError failure (connection error, DNS failure, timeout, JSON decode error) logged, printed a message, and returned → exit 0, with no error output at all in JSON mode.
2. The if not node_version.download_url: branch — same shape, same silent exit 0.

Both now emit the node_install_failed structured envelope and raise typer.Exit(code=1), mirroring the existing RegistryAPIError branch. node_install_failed was already registered, so no new error code.

This is a deliberate, behavioral change: connection errors and other non-registry failures now exit 1 where they previously exited 0. That is the point of the change.

Stacked PR

Based on matt/be-3271-registry-api-typed-errors (#528), not main — the RegistryAPIError branch, get_renderer() in this module, and the node_install_failed error code all come from that PR and are not on main yet. GitHub will retarget this to main once #528 merges. Review only the net diff (one commit).

The one non-obvious line

The download_url check moves below the try block. typer.Exit subclasses Exception, so raising it inside the try would be caught by the broad except Exception — logging a nonsense error: 1 and emitting a second envelope. Moving the check out is safe because install_node either returns a NodeVersion or raises, and both except branches now raise, so the check is only reached on the success path with a non-None node_version. (Response-mapping errors like a missing id/version still raise inside install_node, i.e. still inside the try.)

Testing

New TestRegistryInstallNonApiFailure covers the two cases the ticket asked for — a requests.ConnectionError from install_node, and a response with no download_url — each asserting exit 1 plus the node_install_failed envelope with details={"node_id": ...}.

Beyond the mocked tests, I drove the real CLI with the real renderer in JSON mode (the unit tests mock get_renderer, so they can't prove the envelope is well-formed). Both paths produce exit 1 and exactly one valid envelope, confirming there's no double-emit:

{"schema": "envelope/1", "ok": false, "command": "node registry-install",
 "error": {"code": "node_install_failed", "message": "Failed to download the custom node test-node.",
           "details": {"node_id": "test-node"}}}

Full suite green: 2584 passed, 37 skipped. ruff format --check clean; ruff check clean on both touched files.

Judgment calls

• Audited the pre-existing assert result.exit_code == 0 cases in test_node_install.py, per the ticket's guardrail. All of them exercise other commands (install, uv-sync, show, …) against a mocked execute_cm_cli success — none were asserting the silent-success bug fixed here, so none needed changing.
• ruff check . reports 14 pre-existing UP038 errors in files this PR doesn't touch. They reproduce with my changes stashed (a local ruff newer than the pinned one), so I left them alone rather than take a drive-by refactor into this diff.
• The registered hint for node_install_failed ends with "check details.body", but the two new non-API paths have no body in details. The hint comes from fix: type registry API failures and surface machine-readable error codes (BE-3271) #528's registry and is shared with the API path, so retuning it is out of scope here — flagging it as a cosmetic wart rather than silently widening this PR.
• Not a capability denial. This diff adds raise/exit-1 paths, but nothing that previously worked now fails: it only changes how already-failed installs report themselves. There's no working alternate path a user should be redirected to instead.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f0d1bddd-552e-4622-b355-e7ff5b888dc5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-3294-registry-install-exit-code

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-3294-registry-install-exit-code

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

✅ No high-signal findings.

Panel: 6/8 reviewers contributed findings.

Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)