[vqt123]

Why

comfy-deploy gives every deployment a CPU server that keeps it ready to open in the editor, and it stays on as long as the deployment does. A deployment that is only called through the API, often by an agent driving comfy-cli, never opens in the editor, so that server does nothing for it. comfy-deploy now takes an editor field to turn the server off (BE-23490), and comfy-cli is the only client that creates deployments: the Python and TypeScript SDKs cannot. Without this flag, a comfy-cli user has no way to use that field. Part of BE-23470.

What changes

Every deployment comfy-deploy makes also gets a small CPU server, so the deployment can be opened in the editor. Someone who only calls a deployment through the API, often an agent driving comfy-cli, has no use for it and had no way to say so.

Before: comfy deploy up always got that CPU server; comfy deploy ls did not say which deployments had one.

Now:

• comfy deploy up --no-editor makes the deployment without it, and --editor turns it back on. With neither flag the CLI sends nothing new: a new deployment gets comfy-deploy's default (on), an existing one keeps what it has, and existing commands send the same requests as before.
• On a deployment that already exists, the value goes with the move to the new release. With no move, it goes in a request of its own, which starts no new copy and touches no GPU settings. That request reads the deployment first, since the list answers no editor, so changed is true only when the value was different.
• comfy-deploy keeps a move it took even when the editor write fails. It also leaves editor out of an answer when it cannot look the value up. So up keeps the create or move it was answered, and judges the editor value on its own, from the latest answer that shows one.
• When no answer shows the value asked for, or the read to confirm it fails, up exits 1 with deploy_editor_unchanged. data still carries the created or moved deployment, and changed says what changed. A failed read puts its error in details.read_error. A later read in the watch that shows the value asked for clears the error.
• comfy deploy ls prints editor on or editor off per deployment, and --json carries editor (true, false or null).
• The comfy-deploy skill says when to pass --no-editor.

Chosen behaviour, different from the ticket:

• The ticket says --editor on an existing deployment goes "with the edit". comfy-deploy refuses editor in the same request as a GPU settings change ("editor goes alone or with a move"), so in that case up sends it as a separate request first, then the settings change.
• comfy-deploy answers editor when one deployment is read, renamed or switched, but not on its deployment list, which is what ls reads. So ls shows a blank (null in JSON) for every deployment until comfy-deploy's list answers it; the CLI side is ready for that and needs no release then. ls does not read each deployment one by one to fill the column.

Tests

• test_a_create_sends_editor_only_when_a_flag_says: a create sends no editor, false or true for no flag, --no-editor and --editor.
• test_a_move_carries_editor_only_when_a_flag_says: a move to a new release carries the value only when a flag is given.
• test_editor_on_a_bounds_edit_goes_in_a_request_of_its_own: --editor --min 1 --max 2 on an existing deployment sends editor alone, then the GPU settings change without it.
• test_editor_alone_on_a_deployment_with_nothing_else_to_change: --no-editor alone sends one request and reports changed: true.
• test_an_update_without_a_flag_sends_no_editor: a settings change without the flag sends no editor request.
• test_an_editor_value_comfy_deploy_did_not_keep_is_not_ok and test_a_move_that_lands_without_the_editor_value_asked_is_not_ok_and_keeps_the_move: an editor value comfy-deploy did not keep exits 1 with deploy_editor_unchanged and keeps the deployment in data.
• test_an_editor_edit_is_a_change_only_when_the_deployment_had_the_other_value: repeating a flag the deployment already has reports changed: false.
• test_an_editor_edit_on_a_stop_failed_deployment_is_a_change and test_an_editor_answer_that_leaves_the_value_out_is_read_again.
• test_a_move_whose_editor_read_fails_keeps_the_move and test_a_create_whose_editor_read_fails_keeps_the_create: a move or create taken, then a read answering 503, keeps the deployment in data and changed: true, and exits 1 with deploy_editor_unchanged and details.read_error. The move case runs twice: an answer without editor, and an editor write that failed.
• test_a_later_read_showing_the_editor_value_asked_confirms_it and test_an_editor_edit_a_watch_read_confirms_is_a_change: answers without editor, then a watch read showing the value asked for, exit 0; an editor-alone edit then reports changed: true.
• test_an_editor_read_that_confirms_the_value_outranks_the_edits_answer: an editor-alone edit whose answer shows the old value, then a read showing the value asked for, reports changed: true and no deploy_editor_unchanged, for --editor and --no-editor, on a ready and a stop_failed deployment.
• test_a_later_read_showing_the_other_editor_value_is_not_ok: a watch read showing the other value exits 1 and keeps the move.
• deploy_api_cases.py: four new wire cases check the exact bodies of the create, the move with true and with false, and the editor-alone request, and the error messages for the new request.
• test_rows_carry_and_print_the_editor_value_and_blank_where_none_was_sent: ls --json gives true, false and null and validates against deploy_ls.json; the table shows editor on, editor off and a blank for a deployment from a comfy-deploy that sends no editor.

The full tests/comfy_cli suite: 10697 passed, 7 failed, all 7 also failing on main (test_code_search, test_run_json, test_http). ruff check and ruff format --check pass.

Not run: a live comfy deploy up --no-editor against a running comfy-deploy; the tests use a recorded fake client.

Ref BE-23492

Summary by CodeRabbit

• New Features
  • comfy deploy up accepts --editor and --no-editor to enable or disable a deployment’s editor server. Without either flag, no editor setting is sent.
  • comfy deploy ls shows whether the editor server is on or off when that status is available.
• Bug Fixes
  • If the requested editor setting cannot be confirmed, the command reports an error while preserving the rest of the deployment result.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Comfy-Org/comfy-cli
• Branch: main
• File: .coderabbit.yaml
• Commit: 905c9e5
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: bd5b9d85-274c-4a24-b5b0-cfe47efcf29c

Commits

Reviewing files that changed from the base of the PR and between ec22374 and e6b7a1d.

Files selected for processing: 3

• comfy_cli/command/deploy_up.py
• tests/comfy_cli/command/deploy_up_support.py
• tests/comfy_cli/command/test_deploy_up.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

Walkthrough

comfy deploy up accepts optional editor settings for deployment creation, updates, and moves. It checks requested values against service responses or follow-up reads. comfy deploy ls displays editor status when available.

Changes: Deployment editor settings

Layer / File(s)	Summary
Editor request contract and API operations comfy_cli/command/deploy_types.py, comfy_cli/deploy_api.py, comfy_cli/deploy_api_errors.py, tests/comfy_cli/deploy_api_cases.py	Create and move requests accept optional editor values. A new API method sets the editor value. API status mappings and wire tests cover these operations.
Editor requests and confirmation in deploy up comfy_cli/command/deploy.py, comfy_cli/command/deploy_up.py, comfy_cli/error_codes.py, tests/comfy_cli/command/test_deploy_up.py, tests/comfy_cli/command/test_deploy_up_move.py, tests/comfy_cli/command/deploy_up_support.py, comfy_cli/skills/comfy-deploy/SKILL.md, CHANGELOG.md, tests/comfy_cli/output/test_error_code_registry.py	The CLI passes explicit editor flags through create, update, and move operations. It confirms the requested value from responses or follow-up reads. If confirmation fails, it reports deploy_editor_unchanged while retaining the operation result.
Editor status in deployment listings comfy_cli/command/deploy_ls.py, comfy_cli/schemas/deploy_ls.json, tests/comfy_cli/command/test_deploy_ls.py	Listing output includes boolean editor values. Pretty output shows on or off when the value is available; the schema permits null.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant up_cmd
  participant deploy_up
  participant DeployClient
  participant comfy_deploy
  up_cmd->>deploy_up: Pass UpRequest with optional editor value
  deploy_up->>DeployClient: Create, move, or set editor value
  DeployClient->>comfy_deploy: Send deployment request
  comfy_deploy-->>DeployClient: Return deployment response
  DeployClient-->>deploy_up: Return response data
  opt Response does not confirm requested value
    deploy_up->>DeployClient: Read deployment for confirmation
    DeployClient->>comfy_deploy: Request deployment state
    comfy_deploy-->>DeployClient: Return deployment state
    DeployClient-->>deploy_up: Return observed editor value or read error
  end

Loading

Suggested reviewers: james00012, guill

Merge Risk: Minimal · up to e6b7a

No concrete merge-blocking issue is established by the supplied evidence. The reported test failures and lack of a live deployment check do not, by themselves, change merge readiness.

Pre-merge checks: 4 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 36.90% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 84 functions across 13 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Architecture Decision Compliance		The PR creates a durable deployment compatibility contract without an ADR. comfy_cli/deploy_api.py:263-279 adds the editor API operation and move payload, while `comfy_cli/command/deploy_up.py:275…	Add and accept an ADR, or amend an applicable accepted ADR, that records comfy-deploy ownership of editor state, the separate editor endpoint versus move payload, confirmation-read rules, and partial-success behavior. Relate it to ADR-AUTOM…

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and specifically describes the primary change: allowing deployments created through the API to skip the editor server.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Architecture Decision Compliance

Explanation

The PR creates a durable deployment compatibility contract without an ADR. comfy_cli/deploy_api.py:263-279 adds the editor API operation and move payload, while comfy_cli/command/deploy_up.py:275-299 and :721-758 define confirmation, ownership, and partial-success invariants. comfy_cli/schemas/deploy_ls.json:27-30 and comfy_cli/error_codes.py:1663-1671 publish the contract to machine consumers. No docs/adr file changes in the PR, and the description does not explain why this boundary is local and reversible. ADR-AUTOMATION-0003 is followed for the renderer, append-only error code, and tolerant fields, but it does not record the deployment editor boundary.

Resolution

Add and accept an ADR, or amend an applicable accepted ADR, that records comfy-deploy ownership of editor state, the separate editor endpoint versus move payload, confirmation-read rules, and partial-success behavior. Relate it to ADR-AUTOMATION-0003 and document testable invariants and revisit triggers. Alternatively, explain in the PR or architecture documentation why this contract is local and reversible.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[james00012]

Approve: the create, move and editor-alone requests carry editor only when a flag says, and the deploy tests pass on the head.

I read this as: up could not say whether a deployment keeps its editor CPU server, and now --editor/--no-editor sets it and ls shows it. Reviewed at dc5b4d0.

Should fix, not blocking: editor write is lost from the report if the next request fails

Checked: the 1026 deploy and error-code tests pass on the head.

Not checked: comfy-deploy's server behavior for editor on a stopped or stop_failed deployment.

[vqt123]

Thanks @james00012. The one should-fix is done in ec22374 (reply in its thread). On what was not checked, a stopped or stop_failed deployment: up sends the editor edit on its own before the start, and on stop_failed it sends no start; what comfy-deploy does with editor there is its own handling, outside this PR.

[james00012]

Approve.

I read this as: a start or --min/--max edit that fails after an editor-only edit now says the editor server was already turned on or off, with its code, status and details unchanged. Reviewed dc5b4d0..ec22374.

Not checked: the new tests were read, not run.

[vqt123]

Thanks @james00012. The new tests run in this PR's test jobs on ec22374; nothing else changes from here.