fix(nodes,jobs): render a where_invalid envelope for a bad --where instead of a traceback or a silent misroute

[mattmillerai]

ELI-5

comfy nodes show --where bogus used to crash with a raw Python stack trace and print nothing on stdout. comfy jobs ls --where bogus was worse — it printed a successful local listing and exited 0. Both now print the same one-line JSON error envelope (error.code == "where_invalid") that every other routed command already prints, so a script or agent reading the output gets an error code instead of a traceback or a wrong answer.

What was wrong

nodes._resolved_where (comfy_cli/command/nodes.py) is written to survive a corrupt persisted where_default: it catches the ValueError and retries with the config dropped. But when the flag / COMFY_WHERE / project defaults.where value is the bad one, that retry re-parses the exact same value, where._parse raises again, and nothing catches it. Reproduced on origin/main (3fddc3e1):

$ comfy --json nodes show KSampler --where bogus --input tests/comfy_cli/fixtures/nodes_path_object_info.json
<rich traceback: ValueError: invalid --where value 'bogus': expected one of ['local', 'cloud']>
exit 1, stdout empty

jobs._is_cloud had the same hole with a different symptom: it swallowed the ValueError and returned False, on the commented assumption that cmdline.py's top-level --where had already rejected the value. That only covers comfy --where X jobs ls; a per-command flag or an exported COMFY_WHERE reaches no such validator:

$ comfy --json jobs ls --where bogus
{"ok": true, "where": "local", "data": {..., "jobs": []}, "error": null}
exit 0

Every other routed command reaches where.resolve_default_or_exit, which renders a where_invalid envelope. The nodes and jobs verbs were deliberately kept on their own recovery fallbacks, but those fallbacks only ever recovered the config case.

What changed

• comfy_cli/where.py: split the emit-and-exit tail of resolve_default_or_exit into a reusable where_invalid_exit(exc) -> typer.Exit, and lifted its hint text into a WHERE_INVALID_HINT module constant, so the call sites that render this envelope cannot drift apart. The helper returns the typer.Exit for the caller to raise ... from exc rather than being annotated NoReturn — nothing in CI enforces a NoReturn contract, so the raise is made structural at each call site instead.
• comfy_cli/command/nodes.py: the config-recovery retry is now wrapped in its own except ValueError, which raises where_invalid_exit. The config-recovery branch itself is untouched — a corrupt where_default still drops to the next precedence source rather than failing the command.
• comfy_cli/command/jobs.py: _is_cloud now mirrors nodes exactly — recover from a corrupt persisted where_default, emit the envelope and exit 1 when the flag/env/project value is the bad one.
• comfy_cli/cmdline.py: run, upload and download fail the same multi-source way (they all call resolve(flag=..., config_value=...)), so they now use WHERE_INVALID_HINT instead of three copies of a shorter string. The sites that validate a single explicit value keep the shorter hint on purpose — the top-level --where, set-default --where and setup call _parse on exactly the string the user just typed, and logs is local-only, so pointing any of them at COMFY_WHERE/comfy.yaml would send the user hunting in a file that had nothing to do with the failure.
• tests/comfy_cli/command/test_nodes_where_invalid.py (17 tests) and tests/comfy_cli/command/test_jobs_where_invalid.py (13 tests).

Verification

Reproduced both bugs first, then verified the fixes by running the real CLI (not only the test harness):

case	before	after
nodes show KSampler --where bogus --input FX	traceback, stdout empty, exit 1	where_invalid envelope, exit 1
COMFY_WHERE=bogus comfy nodes show KSampler --input FX	traceback, stdout empty	where_invalid envelope, exit 1
defaults.where: bogus in a schema: project/1 comfy.yaml	traceback	where_invalid envelope, exit 1
jobs ls --where bogus	ok: true, where: "local", exit 0	where_invalid envelope, exit 1
COMFY_WHERE=bogus comfy jobs ls	ok: true, where: "local", exit 0	where_invalid envelope, exit 1
corrupt where_default + --where local	worked	still works (unchanged)
jobs ls --where cloud	routed cloud	still routes cloud (cloud_not_configured, where: "cloud")

All 9 graph-loading nodes verbs were swept live with --where bogus (ls, show, search, upstream, downstream, path, types, categories, widget-catalog) — every one now returns the envelope with exit 1. nodes refresh is the tenth verb; it accepts --where as a documented legacy no-op and never resolves routing, so it never had the defect. All 5 routed jobs verbs (ls, status, wait, cancel, watch) are covered too — each calls _stamp_where as its first statement, so a bad value exits before any host/port resolution or network call.

Only the genuinely invalid value is rejected: --where local and --where cloud both still resolve and route on every touched verb, confirmed live.

Red→green proof: with the source files stashed, 14 of the 17 nodes tests fail. The 3 that pass without the fix are exactly the regression controls (the two corrupt-config recovery cases and the valid-flag unit test) — i.e. they test pre-existing behavior rather than asserting my own premise. The jobs file's TestBadFlag/TestBadEnvAndProject cases all asserted exit 1 against a path that returned exit 0 before the change.

Judgment calls

Exit code is 1, not 2. The plan this PR implements asked for "exit 2, exactly as resolve_default_or_exit does for the other verbs" — those two clauses contradict each other. resolve_default_or_exit exits 1, as do workflow.py's and launch.py's where_invalid paths and the top-level comfy --where bogus flag (verified live: rc=1). I honored the "exactly as the other verbs" half, because an agent runner keying on where_invalid across commands would otherwise see this one verb family disagree with all the rest. Say the word if 2 was actually intended and I'll flip it — but then it should be flipped everywhere at once.

jobs is now in scope. It was originally left out because the plan's precondition ("apply the identical fix if it re-raises the same way") is false — jobs swallowed rather than re-raised. Review pointed out that this made where.py's own docstring untrue, and that silently answering local when the user named an unparseable target is a worse failure than the traceback. Folded in rather than deferred: it is the same five-line shape as nodes, on a resolver this PR already had to document.

Residual

Not fixed here; each is actionable on its own.

1. assets library still emits a raw traceback for the same input, via a different resolver. Verified live on this branch: comfy --json assets library ls --where bogus → rich traceback, exit 1. It routes through target.resolve_target / cloud_target_or_local_error (comfy_cli/target.py ~L86), which calls where.resolve() with no ValueError handling at all — a separate code path from the resolve_default family this PR touches. Sizing the half not fixed: 22 call sites across 18 modules go through resolve_target / cloud_target_or_local_error; each would need its own check to know whether a bad --where there tracebacks, silently misroutes, or is unreachable. I did not sweep all 22 — only the surfaces I could drive offline. Fixing that family centrally (have resolve_target render the shared envelope) is the natural follow-up and would subsume this.

2. Unexercised artifacts. The plan this PR implements was written from an upstream investigation whose write-up lives on a tracker I have no access to; I did not read it, so this PR rests entirely on the reproductions I ran myself rather than on that upstream evidence. The GitHub half of the duplicate check I could re-run: gh pr list --repo Comfy-Org/comfy-cli --state open --search '_resolved_where OR where_invalid' returns nothing overlapping.

3. One pre-existing test failure, unrelated to this change. tests/comfy_cli/test_http.py::test_an_unloadable_supplement_falls_through_to_the_platform_roots fails on this machine (a platform trust-store root count). Confirmed it fails identically on the unmodified branch head with these changes stashed, and it is green in CI, so it is environment-dependent and not introduced here. It is left untouched.

Provenance

• Authored by: agent-work loop
• Verified: uv run --extra dev pytest -q: 7410 passed, 38 skipped, with the pre-existing test_http.py trust-store failure deselected (see Residual [New Feature] Web interface? #3); ruff check .: all checks passed; ruff format --diff .: 447 files already formatted; the two new files alone: 30 passed; live CLI repro + fix confirmed for both nodes and jobs across the flag/env/project routing sources, plus a live check that --where local and --where cloud still route correctly
• Deviations: exit code 1 rather than the plan's "exit 2" (the plan's own "exactly as resolve_default_or_exit does" clause, and every existing where_invalid site in the repo, use 1 — see Judgment calls); the plan's step 2 jobs fix, initially skipped because its stated precondition was false, is now applied after review — jobs silently misrouted rather than re-raising, which is a worse shape than the one the precondition described

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 28098833-3971-4862-92fd-1e4064d9b64d

📥 Commits

Reviewing files that changed from the base of the PR and between e3dc9e8 and 1779b24.

📒 Files selected for processing (5)

• comfy_cli/cmdline.py
• comfy_cli/command/jobs.py
• comfy_cli/command/nodes.py
• comfy_cli/where.py
• tests/comfy_cli/command/test_jobs_where_invalid.py

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The change standardizes invalid where routing errors for comfy nodes and comfy jobs. Invalid flag, environment, and project values now return one JSON error envelope without a traceback. Corrupt persisted defaults still recover through valid overrides or local resolution.

Changes

Invalid where routing

Layer / File(s)	Summary
Shared invalid-routing error contract comfy_cli/where.py, comfy_cli/cmdline.py	Adds WHERE_INVALID_HINT and where_invalid_exit. Routing errors now use a standardized where_invalid envelope and exit status 1.
Routing command fallback comfy_cli/command/nodes.py, comfy_cli/command/jobs.py	comfy nodes and comfy jobs discard invalid persisted defaults. Invalid remaining routing values use the shared structured error path instead of a traceback or silent local fallback.
Invalid routing validation tests/comfy_cli/command/test_nodes_where_invalid.py, tests/comfy_cli/command/test_jobs_where_invalid.py	Tests cover invalid routing sources, JSON output, traceback suppression, valid overrides, persisted-default recovery, and direct resolver behavior.

Merge Risk: ⚪ Minimal · up to 1779b

Invalid routing values for nodes and jobs now return a structured error and exit code instead of tracebacks or unintended local fallback. Coverage includes invalid routing sources and persisted-default recovery, with no current merge-readiness risk identified.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/be-10653-nodes-where-invalid-envelope

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/be-10653-nodes-where-invalid-envelope

---

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

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 3 finding(s).

Severity	Count
🟢 Low	1
⚪ Nit	2

Panel: 6/6 reviewers contributed findings.