feat: emit envelope/1 for comfy launch / comfy stop under --json - #588
feat: emit envelope/1 for comfy launch / comfy stop under --json#588mattmillerai wants to merge 2 commits into
comfy launch / comfy stop under --json#588Conversation
comfy launch and comfy stop previously ignored the global --json flag, emitting no envelope/1 object on stdout unlike every other subcommand. Programmatic callers (e.g. comfy-local-mcp's launch_comfyui / stop_comfyui tools, which parse envelope/1) had to special-case these two commands. Add envelope emission on every reachable success and error path for both commands, guarded by renderer.is_json() so human (pretty) output and exit codes are unchanged. Register launch/stop schemas in discovery and the new lifecycle error codes in the registry.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 4 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
mattmillerai
added
agent-coded
dosubot
Bot
added
size:L
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 7 finding(s).
| Severity | Count |
|---|---|
| 🟠 High | 1 |
| 🟡 Medium | 2 |
| 🟢 Low | 3 |
| ⚪ Nit | 1 |
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
- stop: keep the background record when kill_all fails (was unconditionally removed, orphaning a live server that a retried `comfy stop` could no longer target); raise typer.Exit(1) on the failure path so pretty mode exits non-zero too, not just --json. - launch: parse the `--port=`/`--listen=` single-token form (was ignored, leaving port at the 8188 default that disagreed with the bound port). - launch: range-check --port to 1-65535 for a precise port_invalid verdict. - launch: cap the launch_failed `details.log` payload via the existing LOGS_MAX_LINES/LOGS_MAX_BYTES bounds (was unbounded, could leak secrets). - launch: bracket IPv6 hosts in emitted/printed URLs via shared _bracket_host. - launch: redirect the foreground child's stdout to stderr under --json so the single-envelope-on-stdout contract holds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
ELI-5
Every
comfycommand run with--jsonprints one machine-readableenvelope/1object on stdout so tools can parse the result. Two commands —comfy launchandcomfy stop— forgot to do that, so a program callingthem got nothing back and had to guess whether it worked. This teaches those
two commands to print the same envelope everyone else does. Humans running
the commands normally see exactly the same output as before.
Closes #509.
What & why
comfy launchandcomfy stopignored the global--jsonflag: unlikeevery other subcommand they emitted no
envelope/1on stdout. Programmaticcallers (e.g. comfy-local-mcp's
launch_comfyui/stop_comfyui, which parsethe versioned envelope) got nothing machine-readable and had to special-case
these two commands. This completes the existing
--jsoncontract for them —no new user-visible UI.
What changed
comfy stopemits an envelope on every path: success(
data.stopped/host/port/pid), no background server recorded(
error.code = no_background_server), and kill failure(
error.code = stop_failed).comfy launchemits an envelope on every reachable path:data.background=true,url,pid) — emitted fromthe monitor right before it
os._exit(0)s, via a small extracted_emit_launch_successhelper so it is unit-testable;server_already_running,port_invalid,port_in_use, andlaunch_failed(log-open failure or no success line);not_in_workspace;launch_failedon anon-zero return code.
launch/stopoutput schemas incomfy discoverand the newlifecycle error codes in the
error_codesregistry (both are enforced byexisting contract tests).
Human output is unchanged
Every emission is guarded by
if renderer.is_json():, so pretty mode printsonly the existing Rich lines and keeps the same process exit codes. A test
asserts stdout carries no JSON in pretty mode.
Tests
tests/comfy_cli/command/test_launch_stop_envelope.pyasserts the envelope(schema,
ok,command,data/error) on the launch and stop success anderror paths, plus the pretty-mode purity check. Full suite green
(2678 passed, 13 skipped);
ruff check+ruff formatclean on changedfiles.
Notes / judgment calls
comfy launchis a blocking, interactive server — its"success" envelope is only emitted when the server process exits. Streaming
progress (
--json-stream) is explicitly out of scope; an agent uses--background, which is the fully-covered path._emit_launch_successwas extracted specifically because the success pathos._exit(0)s immediately, which is otherwise untestable without killingthe test process.
machine output. No capability is denied — the error envelopes mirror
pre-existing human error messages that already raised/exited.