diff --git a/CHANGELOG.md b/CHANGELOG.md index 87a843ce..cb31d019 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,16 @@ history. ## [Unreleased] +### Added + +- `comfy deploy up --startup-arg=` sets the ComfyUI startup flags on the + deployment it creates, and `comfy deploy scale --startup-arg=` changes + them on a stopped deployment (`--highvram`, `--reserve-vram 2`, and the rest + of the service's allowlist); `scale --clear-startup-args` removes them. Flags + apply when the deployment starts, so stop a running deployment first + (`comfy deploy stop`, then `scale`, then `start`); `deploy status`, `deploy up` + and `deploy scale` report the stored flags as `computeConfig.startupArgs`. + ### Fixed - `comfy install --fast-deps --nvidia` no longer installs a torch that its diff --git a/comfy_cli/command/deploy.py b/comfy_cli/command/deploy.py index e0ecb932..6bd91064 100644 --- a/comfy_cli/command/deploy.py +++ b/comfy_cli/command/deploy.py @@ -56,6 +56,38 @@ typer.Argument(help="ComfyUI install directory or build spec path. Default: the current directory."), ] DeploymentOption = Annotated[str | None, typer.Option("--deployment", help="Select this deployment id.")] +StartupArgOption = Annotated[ + list[str] | None, + typer.Option( + "--startup-arg", + help=( + "ComfyUI startup flag, one token per use, written as --startup-arg=--highvram " + "(a value is its own token: --startup-arg=--reserve-vram --startup-arg=2). " + "Only the service's allowlist of VRAM, precision, attention, cache and performance " + "flags is accepted. `up` sets the flags on the deployment it creates; `scale` replaces " + "the whole stored set on a stopped deployment. Flags apply when the deployment starts." + ), + ), +] + + +def _startup_args(values: list[str] | None) -> tuple[str, ...] | None: + """``None`` when the flag was never given, so the request omits the field.""" + return tuple(values) if values else None + + +def _refuse_startup_arg_conflict(renderer, startup_arg: list[str] | None, clear: bool) -> None: + """``--startup-arg`` sets the flags and ``--clear-startup-args`` removes them; + both at once has no meaning the service could honor, so it is refused + before any round trip.""" + if not startup_arg or not clear: + return + renderer.error( + code="deploy_conflicting_input", + message="--startup-arg and --clear-startup-args cannot be combined", + details={"conflicting": ["--startup-arg", "--clear-startup-args"]}, + ) + raise typer.Exit(code=1) def _require_paired_bounds(renderer, minimum: int | None, maximum: int | None) -> None: @@ -127,10 +159,21 @@ def scale_cmd( ] = None, gpu: Annotated[str | None, typer.Option("--gpu", help="GPU class; deployment must be stopped.")] = None, region: Annotated[str | None, typer.Option("--region", help="Region; deployment must be stopped.")] = None, + startup_arg: StartupArgOption = None, + clear_startup_args: Annotated[ + bool, + typer.Option("--clear-startup-args", help="Remove every ComfyUI startup flag; deployment must be stopped."), + ] = False, ) -> None: - _require_paired_bounds(get_renderer(), minimum, maximum) + renderer = get_renderer() + _require_paired_bounds(renderer, minimum, maximum) + _refuse_startup_arg_conflict(renderer, startup_arg, clear_startup_args) target = _deploy_read.ReadRequest(path, deployment_id) - _deploy_lifecycle.run_scale(_deploy_lifecycle.ScaleRequest(target, minimum, maximum, gpu, region)) + _deploy_lifecycle.run_scale( + _deploy_lifecycle.ScaleRequest( + target, minimum, maximum, gpu, region, _startup_args(startup_arg), clear_startup_args + ) + ) @app.command("stop", help="Pause a deployment while retaining its endpoint and staged models.") @@ -234,6 +277,7 @@ def up_cmd( ] = None, release: Annotated[str | None, typer.Option("--release", help="Deploy this release id.")] = None, deployment_id: DeploymentOption = None, + startup_arg: StartupArgOption = None, watch: Annotated[bool, typer.Option("--watch", help="Poll until the deployment reaches a terminal state.")] = False, ) -> None: renderer = get_renderer() @@ -247,6 +291,7 @@ def up_cmd( minimum=minimum, maximum=maximum, deployment_id=deployment_id, + startup_args=_startup_args(startup_arg), ) try: result = reconcile_up(builder, client, request) diff --git a/comfy_cli/command/deploy_lifecycle.py b/comfy_cli/command/deploy_lifecycle.py index c2aacf5d..f469d396 100644 --- a/comfy_cli/command/deploy_lifecycle.py +++ b/comfy_cli/command/deploy_lifecycle.py @@ -31,6 +31,11 @@ class ScaleRequest: maximum: int | None gpu: str | None region: str | None + # ``None`` is "flag omitted", and the field is then left out of the request + # so the service keeps the stored flags; an explicit clear sends an empty + # list, which is how the service reads "remove them". + startup_args: tuple[str, ...] | None = None + clear_startup_args: bool = False @runtime_checkable @@ -87,6 +92,10 @@ def scale(renderer: Renderer, client: DeploymentLifecycleClient, deployment_id: value = requested if requested is not None else current.get(bound) if value is not None: merged[bound] = value + if request.clear_startup_args: + merged["startupArgs"] = [] + elif request.startup_args is not None: + merged["startupArgs"] = list(request.startup_args) try: result = client.update_deployment(deployment_id, merged) except DeployAPIError as error: @@ -100,11 +109,17 @@ def scale(renderer: Renderer, client: DeploymentLifecycleClient, deployment_id: details=details, ) from error if renderer.is_pretty(): - renderer.success( - f"Scaled deployment {deployment_id} to " - f"min={merged.get('min', 'unset')}, max={merged.get('max', 'unset')}" - ) - renderer.emit(result, command="deploy scale", changed=merged != current) + settings = [f"{bound}={merged[bound]}" for bound in ("min", "max") if bound in merged] + if "startupArgs" in merged: + settings.append(f"startupArgs={' '.join(merged['startupArgs']) or 'none'}") + elif not settings: + settings = ["min=unset", "max=unset"] + renderer.success(f"Scaled deployment {deployment_id} to {', '.join(settings)}") + # Overlaid on the stored config: a field the request leaves out is kept + # by the service, so it is not a change either. The service stores no + # empty flag set, so a clear against none stored is not a change. + before = {**current, "startupArgs": current.get("startupArgs", [])} + renderer.emit(result, command="deploy scale", changed={**before, **merged} != before) _run_lifecycle(request.target, scale) diff --git a/comfy_cli/command/deploy_types.py b/comfy_cli/command/deploy_types.py index ded69808..1059134c 100644 --- a/comfy_cli/command/deploy_types.py +++ b/comfy_cli/command/deploy_types.py @@ -38,6 +38,10 @@ class UpRequest: # The deployment `--deployment` named, when the Build has more than one the # ranking cannot separate. deployment_id: str | None = None + # ComfyUI startup flags, one token each. ``None`` is "flag omitted": the + # service keeps whatever the deployment stores, so only an explicit list is + # ever sent, and only a create applies it (see ``reconcile_up``). + startup_args: tuple[str, ...] | None = None @dataclass(frozen=True, slots=True) @@ -49,10 +53,10 @@ class UpResult: created: bool changed: bool # Flags the caller supplied that this reconcile could not apply. Restarting - # a stopped deployment is a start, not an edit, so bounds passed alongside - # it are dropped — silently discarding explicit input is the same defect as - # silently resetting it, so the renderer says so. - dropped_bounds: tuple[str, ...] = () + # a stopped deployment is a start, not an edit, so bounds and startup flags + # passed alongside it are dropped; silently discarding explicit input is the + # same defect as silently resetting it, so the renderer says so. + dropped_flags: tuple[str, ...] = () def payload(self) -> JsonObject: supersedes: list[JsonValue] = [*self.supersedes] @@ -91,6 +95,13 @@ def required_int(value: JsonObject, key: str) -> int: return field +def required_string_list(value: JsonObject, key: str) -> list[str]: + field = value.get(key) + if not isinstance(field, list) or any(not isinstance(item, str) or not item.strip() for item in field): + raise server_shape_error(f"the deploy service returned an invalid {key}", field=key) + return list(field) + + def compute_config(deployment: JsonObject) -> JsonObject: """The deployment's compute configuration, as the service models it. @@ -109,6 +120,10 @@ def compute_config(deployment: JsonObject) -> JsonObject: for bound in ("min", "max"): if bound in raw: config[bound] = required_int(raw, bound) + # Stored only when the deployment has flags: the service drops the key on a + # clear, so absence here is "none" and an omitted flag on `scale` keeps it. + if "startupArgs" in raw: + config["startupArgs"] = required_string_list(raw, "startupArgs") return config diff --git a/comfy_cli/command/deploy_up.py b/comfy_cli/command/deploy_up.py index bfdd26f5..37b65681 100644 --- a/comfy_cli/command/deploy_up.py +++ b/comfy_cli/command/deploy_up.py @@ -119,15 +119,39 @@ def _create_live_deployment(client: DeployUpClient, request: UpRequest, compute: raise AssertionError("bounded create loop exhausted without returning or raising") -def _dropped_bounds(request: UpRequest, compute: JsonObject) -> tuple[str, ...]: - """Name the bound flags that were supplied but would not change the live value. +def _dropped_flags(request: UpRequest, compute: JsonObject) -> tuple[str, ...]: + """Name the flags the caller supplied that a restart will not apply. Only the restart branches consult this: they hand back ``compute`` untouched, - so a bound the caller actually typed is discarded. A bound equal to what is - already live is not reported — nothing was lost. + so a bound that differs from the live value is discarded, and so is a startup + flag set that differs from the stored one (flags are set while a deployment + is stopped, and the restart just started it). A value equal to what is + already live is not reported: nothing was lost. """ supplied = (("--min", request.minimum, compute.get("min")), ("--max", request.maximum, compute.get("max"))) - return tuple(flag for flag, value, live in supplied if value is not None and value != live) + dropped = [flag for flag, value, live in supplied if value is not None and value != live] + if _startup_args_changed(request, compute): + dropped.append("--startup-arg") + return tuple(dropped) + + +def _startup_args_changed(request: UpRequest, compute: JsonObject) -> bool: + """Whether the caller named startup flags that differ from the live set. + + An omitted flag is never a change, and the same list as live is not one + either: re-running `up` with the flags it was created with must be a no-op. + """ + return request.startup_args is not None and list(request.startup_args) != compute.get("startupArgs", []) + + +def _startup_args_remedy(deployment_id: str, also: str = "") -> str: + """Startup flags are set only while a deployment is stopped, so the remedy + is always the same three steps; ``also`` rides on the `scale`.""" + return ( + f"run `comfy deploy stop --deployment {deployment_id}`, then " + f"`comfy deploy scale --deployment {deployment_id} --startup-arg={also}`, then " + f"`comfy deploy start --deployment {deployment_id}`" + ) def reconcile_up(builder: BuilderReleaseClient, client: DeployUpClient, request: UpRequest) -> UpResult: @@ -152,6 +176,8 @@ def reconcile_up(builder: BuilderReleaseClient, client: DeployUpClient, request: "min": minimum, "max": maximum, } + if request.startup_args is not None: + compute["startupArgs"] = list(request.startup_args) snapshot = _create_live_deployment(client, request, compute) return UpResult(snapshot, _release_summary(request.release), compute, supersedes, True, True) @@ -166,12 +192,23 @@ def reconcile_up(builder: BuilderReleaseClient, client: DeployUpClient, request: ) deployment_id = _required_string(existing, "id") status = _required_string(existing, "status") - dropped = _dropped_bounds(request, compute) + dropped = _dropped_flags(request, compute) if status in {"stopped", "failed"}: started = client.start_deployment(deployment_id) return UpResult(started, _release_summary(request.release), compute, supersedes, False, True, dropped) if status == "stop_failed": return UpResult(existing, _release_summary(request.release), compute, supersedes, False, False, dropped) + # Startup flags are set when compute is provisioned and the service refuses + # to change them on a running deployment, the same as gpuClass and region. + # Refused here for the same reason those are: the remedy is stop, then + # `scale --startup-arg`, and a 409 relayed from the service says less. + if _startup_args_changed(request, compute): + raise DeployAPIError( + "deploy_immutable_compute", + "a running deployment cannot change startupArgs in place", + details={"deploymentId": deployment_id, "computeConfig": compute}, + hint=_startup_args_remedy(deployment_id), + ) # An omitted bound keeps the live value, exactly as `comfy deploy scale` # merges: re-running `up` after a release must not silently unscale. desired = {**compute} @@ -179,8 +216,12 @@ def reconcile_up(builder: BuilderReleaseClient, client: DeployUpClient, request: if requested is not None: desired[bound] = requested if desired != compute: - updated = client.update_deployment(deployment_id, desired) - return UpResult(updated, _release_summary(request.release), desired, supersedes, False, True) + # The stored flags are left out of the request, as `scale` leaves them + # out: the service keeps them when the field is absent, and a copy read + # before this edit could be stale. The response is what is stored now. + body = {key: value for key, value in desired.items() if key != "startupArgs"} + updated = client.update_deployment(deployment_id, body) + return UpResult(updated, _release_summary(request.release), _compute_config(updated), supersedes, False, True) return UpResult(existing, _release_summary(request.release), compute, supersedes, False, False) @@ -196,17 +237,22 @@ def _render_result(renderer, result: UpResult, *, watch: bool) -> None: ) elif watch and status in {"failed", "stopped"}: renderer.warn(f"Deployment {deployment_id} reached terminal status {status}.") - if result.dropped_bounds: - joined = " and ".join(result.dropped_bounds) - renderer.warn( - f"{joined} had no effect; deployment {deployment_id} kept its existing worker bounds.", + if result.dropped_flags: + joined = " and ".join(result.dropped_flags) + bounds = " --min --max " if any(flag != "--startup-arg" for flag in result.dropped_flags) else "" + if status == "stop_failed": # `scale` is only actionable once the deployment settles: the API # rejects an edit unless it is ready or stopped (`run_scale` re-wraps # that as `deploy_conflict`), so a `stop_failed` deployment is sent # to the stop remedy warned about just above instead. - hint=None - if status == "stop_failed" - else f"run `comfy deploy scale --deployment {deployment_id} --min --max ` to change them", + hint = None + elif "--startup-arg" in result.dropped_flags: + hint = _startup_args_remedy(deployment_id, bounds) + else: + hint = f"run `comfy deploy scale --deployment {deployment_id}{bounds}` to change them" + renderer.warn( + f"{joined} had no effect; deployment {deployment_id} kept its existing compute settings.", + hint=hint, ) terminal = status in {"failed", "stopped", "stop_failed"} renderer.emit( diff --git a/comfy_cli/deploy_api.py b/comfy_cli/deploy_api.py index a65d460c..6ede9249 100644 --- a/comfy_cli/deploy_api.py +++ b/comfy_cli/deploy_api.py @@ -72,6 +72,14 @@ def _validate_compute_config(compute_config: dict) -> None: # refused against the placeholder max of 1 above — a ceiling nobody set. if "min" in compute_config and "max" in compute_config and minimum > maximum: raise bad_request("computeConfig.min must not exceed computeConfig.max") + # Shape only; which flags are allowed is the service's call and its 400 + # names the offending token. + startup_args = compute_config.get("startupArgs") + if startup_args is not None and ( + not isinstance(startup_args, list) + or any(not isinstance(token, str) or not token.strip() for token in startup_args) + ): + raise bad_request("computeConfig.startupArgs must be a list of ComfyUI flag tokens") class DeployClient: diff --git a/comfy_cli/deploy_api_errors.py b/comfy_cli/deploy_api_errors.py index ce4e7168..1baa9ecd 100644 --- a/comfy_cli/deploy_api_errors.py +++ b/comfy_cli/deploy_api_errors.py @@ -140,7 +140,14 @@ def _compute_or_bad_request(message: str) -> dict[str, str]: def _immutable_or_conflict(message: str) -> dict[str, str]: - return _IMMUTABLE_COMPUTE if "changing gpuclass or region" in message.lower() else _CONFLICT + # The service names the field it refused in its own 409 text; both are + # "stop the deployment first" refusals rather than a state conflict. + lowered = message.lower() + return ( + _IMMUTABLE_COMPUTE + if "changing gpuclass or region" in lowered or "changing startupargs" in lowered + else _CONFLICT + ) def _deleted_or_conflict(message: str) -> dict[str, str]: diff --git a/comfy_cli/error_codes.py b/comfy_cli/error_codes.py index 4f71cf42..520d82f9 100644 --- a/comfy_cli/error_codes.py +++ b/comfy_cli/error_codes.py @@ -1282,6 +1282,13 @@ class ErrorCode: "choose. `details.missing` lists every required option.", "pass every option named in `details.missing`, then retry", ), + ErrorCode( + "deploy_conflicting_input", + "`comfy deploy scale` was given options that cancel each other: `--startup-arg` sets the ComfyUI startup " + "flags and `--clear-startup-args` removes them, so both at once has no meaning the service could honor. " + "`details.conflicting` names the pair.", + "pass either `--startup-arg` or `--clear-startup-args`, not both, then retry", + ), ErrorCode( "deploy_bad_request", "The deploy control plane rejected structurally invalid input. The message names the invalid field or query parameter.", @@ -1329,8 +1336,10 @@ class ErrorCode: ), ErrorCode( "deploy_immutable_compute", - "A ready deployment cannot change its GPU class or region in place.", - "run `comfy deploy stop`, then `comfy deploy scale --gpu --region `, then `comfy deploy start`", + "A ready deployment cannot change its GPU class, region or ComfyUI startup flags in place. The message " + "names the setting.", + "run `comfy deploy stop`, then `comfy deploy scale` with the setting to change (`--gpu --region " + "` or `--startup-arg=`), then `comfy deploy start`", ), ErrorCode( "deploy_deleted", diff --git a/comfy_cli/schemas/deploy_scale.json b/comfy_cli/schemas/deploy_scale.json index 9603eff6..ec19b1ac 100644 --- a/comfy_cli/schemas/deploy_scale.json +++ b/comfy_cli/schemas/deploy_scale.json @@ -15,7 +15,8 @@ "gpuClass": {"type": "string", "minLength": 1}, "region": {"type": "string", "minLength": 1}, "min": {"type": "integer", "minimum": 0, "maximum": 20}, - "max": {"type": "integer", "minimum": 1, "maximum": 20} + "max": {"type": "integer", "minimum": 1, "maximum": 20}, + "startupArgs": {"type": "array", "items": {"type": "string", "pattern": "\\S"}} } }, "id": {"type": "string", "minLength": 1}, diff --git a/comfy_cli/schemas/deploy_status.json b/comfy_cli/schemas/deploy_status.json index e0c7873a..c7375543 100644 --- a/comfy_cli/schemas/deploy_status.json +++ b/comfy_cli/schemas/deploy_status.json @@ -33,7 +33,8 @@ "gpuClass": {"type": "string", "minLength": 1}, "region": {"type": "string", "minLength": 1}, "min": {"type": "integer", "minimum": 0}, - "max": {"type": "integer", "minimum": 1} + "max": {"type": "integer", "minimum": 1}, + "startupArgs": {"type": "array", "items": {"type": "string", "pattern": "\\S"}} } }, "stopReason": {"enum": [null, "user", "credits", "policy"]}, diff --git a/comfy_cli/schemas/deploy_up.json b/comfy_cli/schemas/deploy_up.json index 82b838fa..2f0c1be9 100644 --- a/comfy_cli/schemas/deploy_up.json +++ b/comfy_cli/schemas/deploy_up.json @@ -35,7 +35,8 @@ "gpuClass": {"type": "string", "minLength": 1}, "region": {"type": "string", "minLength": 1}, "min": {"type": "integer", "minimum": 0}, - "max": {"type": "integer", "minimum": 1} + "max": {"type": "integer", "minimum": 1}, + "startupArgs": {"type": "array", "items": {"type": "string", "pattern": "\\S"}} } }, "supersedes": { diff --git a/comfy_cli/skills/comfy-deploy-failures/SKILL.md b/comfy_cli/skills/comfy-deploy-failures/SKILL.md index 7cc759d6..d83c2d1e 100644 --- a/comfy_cli/skills/comfy-deploy-failures/SKILL.md +++ b/comfy_cli/skills/comfy-deploy-failures/SKILL.md @@ -32,11 +32,13 @@ argument you pass. | `deploy_build_not_pushed` | The local spec has no Build id | `comfy build push` | | `deploy_no_deployable_release` | No release with a ready `linux/nvidia` artifact | `comfy build release create --target linux/nvidia` | | `deploy_not_ready` | The deployment is not in `ready` | Wait if transitional; read `events` if terminal | -| `deploy_immutable_compute` | Tried to change GPU/region in place | `stop` → `scale` → `start` | +| `deploy_immutable_compute` | Tried to change GPU/region or startup flags in place | `stop` → `scale` → `start` | | `deploy_deleted` | Tried to start a deleted deployment | `comfy deploy up` makes a new one | | `deploy_ambiguous_deployment` | Several deployments tie for selection | Pass `--deployment ` | | `deploy_unrelated_deployment` | `--deployment` names one outside this scope | Pick from `details.candidateIds` | | `deploy_missing_input` | A required option was omitted non-interactively | Pass everything in `details.missing` | +| `deploy_conflicting_input` | `--startup-arg` and `--clear-startup-args` given together | Pass one of the pair | +| `deploy_bad_request` | The service refused the input; the message names the field or token (a startup flag outside the allowlist, for one) | Fix what the message names, then retry | | `deploy_compute_unavailable` | That GPU/region cannot provision now | Choose another pair from `comfy deploy refs compute` | | `deploy_quota_exceeded` | Workspace deployment or worker limit | Stop or scale down another deployment | | `deploy_payment_required` | No active subscription or credit | Billing problem; a retry will not fix it | diff --git a/comfy_cli/skills/comfy-deploy/SKILL.md b/comfy_cli/skills/comfy-deploy/SKILL.md index 8cab261b..8132bb1c 100644 --- a/comfy_cli/skills/comfy-deploy/SKILL.md +++ b/comfy_cli/skills/comfy-deploy/SKILL.md @@ -35,7 +35,7 @@ Every other verb reads, or gives compute back. up Create or reconcile a deployment for the selected Build release. SPENDS run Submit an API-format workflow to a ready deployment. SPENDS status Deployment health, release freshness, and serving activity. -scale Edit worker bounds, or GPU/region on a stopped deployment. +scale Edit worker bounds, or GPU/region/startup flags on a stopped deployment. stop Pause a deployment, retaining its endpoint and staged models. start Resume a stopped or failed deployment. SPENDS delete Enqueue teardown and soft-delete the record. @@ -117,7 +117,7 @@ fixes — say so rather than restarting into the same wall. ```shell comfy deploy up [PATH] --gpu --region [--min N --max N] - [--release ] [--deployment ] [--watch] + [--startup-arg=]... [--release ] [--deployment ] [--watch] ``` - **It selects the newest deployable release of the Build** unless `--release` @@ -144,6 +144,17 @@ comfy deploy up [PATH] --gpu --region [--min N --max N] reported back as dropped. - **It restarts a `stopped` or `failed` deployment** for that release instead of creating another. +- **`--startup-arg=` sets the ComfyUI flags the deployment boots with**, one + token per use (`--startup-arg=--highvram`; a value is its own token: + `--startup-arg=--reserve-vram --startup-arg=2`). Write it with `=` so the + token is unambiguously the value and never swallows the option after it. The + service accepts only its allowlist of VRAM, precision, attention, cache and + performance flags and names a refused token in a 400. Flags apply when the + deployment starts: on a live deployment a different set is + `deploy_immutable_compute` (`stop` → `scale --startup-arg` → `start`), and on + a restart it is reported as dropped rather than applied, so set the flags with + `scale --startup-arg` while the deployment is still stopped, then `up`. The + stored set comes back as `computeConfig.startupArgs`. ## `comfy deploy run` @@ -225,7 +236,10 @@ The rest are narrower: - **`start`** resumes a `stopped` or `failed` deployment. It spends again from that moment. - **`scale --min N --max N`** changes the bounds on a live or stopped deployment. - `scale --gpu / --region` requires the deployment to be **stopped**. + `scale --gpu / --region` requires the deployment to be **stopped**, and so do + `scale --startup-arg=` (repeatable; replaces the whole set) and + `scale --clear-startup-args` (removes them). A `scale` that names neither + leaves the stored flags alone. - **`delete`** enqueues teardown and soft-deletes the record. **It is not reversible**: a deleted deployment cannot be started, and serving again means `up` creating a new one with a new URL. The record stays visible under @@ -282,6 +296,8 @@ ever hit one, because the wrong reflex costs money or trust: for comes back as a refusal envelope and exits 1: `deploy_delete_needs_confirm` for `delete`, `deploy_missing_input` for an omitted `--gpu`, `--region` or `--workflow`. Pass `--yes` or the named option once the user has actually agreed. +`deploy_conflicting_input` is `--startup-arg` together with `--clear-startup-args`; +pass one or the other. ## Going back to the build diff --git a/tests/comfy_cli/command/deploy_up_support.py b/tests/comfy_cli/command/deploy_up_support.py index 196ee0c1..8b414404 100644 --- a/tests/comfy_cli/command/deploy_up_support.py +++ b/tests/comfy_cli/command/deploy_up_support.py @@ -36,12 +36,16 @@ def deployment( minimum: int = 0, maximum: int = 1, deleted_at: str | None = None, + startup_args: list[str] | None = None, ) -> JsonObject: + compute: JsonObject = {"gpuClass": gpu, "region": region, "min": minimum, "max": maximum} + if startup_args is not None: + compute["startupArgs"] = list(startup_args) return { "id": deployment_id, "releaseId": release_id, "status": status, - "computeConfig": {"gpuClass": gpu, "region": region, "min": minimum, "max": maximum}, + "computeConfig": compute, "createdAt": f"2026-08-23T12:00:{deployment_id[-1].zfill(2) if deployment_id[-1].isdigit() else '00'}Z", "deletedAt": deleted_at, } @@ -81,6 +85,7 @@ def __init__( self.create_keys: list[str] = [] self.generation_deleted_counts: list[int] = [] self.update_calls: list[str] = [] + self.update_bodies: list[JsonObject] = [] self.start_calls: list[str] = [] self.catalog_calls = 0 self._keys: dict[str, str] = {} @@ -135,7 +140,16 @@ def get_deployment(self, deployment_id: str) -> JsonObject: def update_deployment(self, deployment_id: str, compute_config: JsonObject) -> JsonObject: with self._lock: self.update_calls.append(deployment_id) - self.rows[deployment_id]["computeConfig"] = copy.deepcopy(compute_config) + self.update_bodies.append(copy.deepcopy(compute_config)) + stored = self.rows[deployment_id]["computeConfig"] + merged = copy.deepcopy(compute_config) + # As the service reads the field: absent keeps the stored flags, an + # empty list clears them, and no empty set is ever stored. + if "startupArgs" not in merged and "startupArgs" in stored: + merged["startupArgs"] = copy.deepcopy(stored["startupArgs"]) + if merged.get("startupArgs") == []: + del merged["startupArgs"] + self.rows[deployment_id]["computeConfig"] = merged return copy.deepcopy(self.rows[deployment_id]) def start_deployment(self, deployment_id: str) -> JsonObject: diff --git a/tests/comfy_cli/command/test_deploy_lifecycle.py b/tests/comfy_cli/command/test_deploy_lifecycle.py index c199578d..4b84aad2 100644 --- a/tests/comfy_cli/command/test_deploy_lifecycle.py +++ b/tests/comfy_cli/command/test_deploy_lifecycle.py @@ -425,3 +425,214 @@ def test_scale_of_a_bounds_free_deployment_validates_against_its_schema(monkeypa # Then assert result.exit_code == 0, result.stderr jsonschema.Draft202012Validator(_schema("deploy_scale.json")).validate(_envelope(result)["data"]) + + +def test_scale_registers_the_startup_arg_options() -> None: + assert {"--startup-arg", "--clear-startup-args"} <= option_names("scale") + + +def test_scale_sends_startup_args_only_when_named(monkeypatch) -> None: + # Given + responder = APIResponder(deployment("dep-1", minimum=2, maximum=4)) + monkeypatch.setattr("comfy_cli.deploy_api.request_json", responder) + _install_client(monkeypatch, DeployClient("https://deploy.test", "token")) + + # When + result = _invoke( + "scale", + "--deployment", + "dep-1", + "--startup-arg=--highvram", + "--startup-arg=--reserve-vram", + "--startup-arg=2", + ) + + # Then + assert result.exit_code == 0, result.stderr + assert responder.bodies == [ + { + "computeConfig": { + "gpuClass": "l4", + "region": "US-MO-2", + "min": 2, + "max": 4, + "startupArgs": ["--highvram", "--reserve-vram", "2"], + } + } + ] + assert _envelope(result)["changed"] is True + + +def test_scale_leaves_stored_startup_args_out_of_a_bounds_edit(monkeypatch) -> None: + """The service keeps the stored flags when the field is absent, so a bounds + edit must not resend them (a stale copy would overwrite a newer set) and must + not send an empty list (which the service reads as a clear).""" + # Given + responder = APIResponder(deployment("dep-1", minimum=2, maximum=4, startup_args=["--highvram"])) + monkeypatch.setattr("comfy_cli.deploy_api.request_json", responder) + _install_client(monkeypatch, DeployClient("https://deploy.test", "token")) + + # When + result = _invoke("scale", "--deployment", "dep-1", "--min", "1", "--max", "4") + + # Then + assert result.exit_code == 0, result.stderr + assert responder.bodies == [{"computeConfig": {"gpuClass": "l4", "region": "US-MO-2", "min": 1, "max": 4}}] + assert _envelope(result)["changed"] is True + + +def test_scale_that_names_nothing_new_is_not_a_change(monkeypatch) -> None: + # Given + responder = APIResponder(deployment("dep-1", minimum=2, maximum=4, startup_args=["--highvram"])) + monkeypatch.setattr("comfy_cli.deploy_api.request_json", responder) + _install_client(monkeypatch, DeployClient("https://deploy.test", "token")) + + # When + result = _invoke("scale", "--deployment", "dep-1", "--min", "2", "--max", "4") + + # Then + assert result.exit_code == 0, result.stderr + assert _envelope(result)["changed"] is False + + +def test_scale_clears_startup_args_with_an_explicit_empty_list(monkeypatch) -> None: + # Given + responder = APIResponder(deployment("dep-1", minimum=2, maximum=4, startup_args=["--highvram"])) + monkeypatch.setattr("comfy_cli.deploy_api.request_json", responder) + _install_client(monkeypatch, DeployClient("https://deploy.test", "token")) + + # When + result = _invoke("scale", "--deployment", "dep-1", "--clear-startup-args") + + # Then + assert result.exit_code == 0, result.stderr + assert responder.bodies == [ + {"computeConfig": {"gpuClass": "l4", "region": "US-MO-2", "min": 2, "max": 4, "startupArgs": []}} + ] + assert _envelope(result)["changed"] is True + + +def test_scale_refuses_startup_arg_together_with_clear(monkeypatch) -> None: + # Given + responder = APIResponder(deployment("dep-1")) + monkeypatch.setattr("comfy_cli.deploy_api.request_json", responder) + _install_client(monkeypatch, DeployClient("https://deploy.test", "token")) + + # When + result = _invoke("scale", "--deployment", "dep-1", "--startup-arg=--highvram", "--clear-startup-args") + + # Then + error = _object(_envelope(result), "error") + assert result.exit_code == 1 + assert error["code"] == "deploy_conflicting_input" + assert _object(error, "details")["conflicting"] == ["--startup-arg", "--clear-startup-args"] + assert responder.bodies == [] + + +def test_scale_clear_against_no_stored_startup_args_is_not_a_change(monkeypatch) -> None: + """The service stores no empty flag set, so clearing a deployment that has + none changes nothing it stores; `changed` must say so.""" + # Given + responder = APIResponder(deployment("dep-1", minimum=2, maximum=4)) + monkeypatch.setattr("comfy_cli.deploy_api.request_json", responder) + _install_client(monkeypatch, DeployClient("https://deploy.test", "token")) + + # When + result = _invoke("scale", "--deployment", "dep-1", "--clear-startup-args") + + # Then + assert result.exit_code == 0, result.stderr + assert responder.bodies == [ + {"computeConfig": {"gpuClass": "l4", "region": "US-MO-2", "min": 2, "max": 4, "startupArgs": []}} + ] + assert _envelope(result)["changed"] is False + + +@pytest.mark.parametrize( + ("row", "args", "line"), + [ + ( + deployment("dep-1", minimum=2, maximum=4), + ["--startup-arg=--highvram", "--startup-arg=--reserve-vram", "--startup-arg=2"], + "Scaled deployment dep-1 to min=2, max=4, startupArgs=--highvram --reserve-vram 2", + ), + ( + deployment("dep-1", minimum=2, maximum=4, startup_args=["--highvram"]), + ["--clear-startup-args"], + "Scaled deployment dep-1 to min=2, max=4, startupArgs=none", + ), + ( + _bounds_free("dep-2"), + ["--startup-arg=--highvram"], + "Scaled deployment dep-2 to startupArgs=--highvram", + ), + ( + deployment("dep-1", minimum=2, maximum=4), + ["--min", "1", "--max", "4"], + "Scaled deployment dep-1 to min=1, max=4", + ), + ], + ids=["set-with-bounds", "clear", "set-without-bounds", "bounds-only"], +) +def test_scale_pretty_line_reports_what_the_request_changed(monkeypatch, row, args, line) -> None: + # Given + responder = APIResponder(row) + monkeypatch.setattr("comfy_cli.deploy_api.request_json", responder) + _install_client(monkeypatch, DeployClient("https://deploy.test", "token")) + + # When + result = _invoke("scale", "--deployment", row["id"], *args, pretty=True) + + # Then + assert result.exit_code == 0, result.stderr + assert line in result.stdout + assert "unset" not in result.stdout + + +def test_scale_pretty_line_keeps_the_unset_form_when_neither_bounds_nor_flags_are_named(monkeypatch) -> None: + # Given + responder = APIResponder(_bounds_free("dep-2")) + monkeypatch.setattr("comfy_cli.deploy_api.request_json", responder) + _install_client(monkeypatch, DeployClient("https://deploy.test", "token")) + + # When + result = _invoke("scale", "--deployment", "dep-2", "--gpu", "a100", pretty=True) + + # Then + assert result.exit_code == 0, result.stderr + assert "Scaled deployment dep-2 to min=unset, max=unset" in result.stdout + + +def test_ready_startup_args_change_maps_to_immutable_compute(monkeypatch) -> None: + # Given + responder = APIResponder( + deployment("dep-1"), + status=409, + message="stop the deployment before changing startupArgs; they apply when it starts", + ) + monkeypatch.setattr("comfy_cli.deploy_api.request_json", responder) + _install_client(monkeypatch, DeployClient("https://deploy.test", "token")) + + # When + result = _invoke("scale", "--deployment", "dep-1", "--startup-arg=--lowvram") + + # Then + error = _object(_envelope(result), "error") + assert result.exit_code == 1 + assert error["code"] == "deploy_immutable_compute" + + +def test_scale_with_startup_args_validates_against_its_schema(monkeypatch) -> None: + # Given + responder = APIResponder(deployment("dep-1", minimum=0, maximum=3)) + monkeypatch.setattr("comfy_cli.deploy_api.request_json", responder) + _install_client(monkeypatch, DeployClient("https://deploy.test", "token")) + + # When + result = _invoke("scale", "--deployment", "dep-1", "--startup-arg=--highvram") + + # Then + assert result.exit_code == 0, result.stderr + data = _envelope(result)["data"] + assert data["computeConfig"]["startupArgs"] == ["--highvram"] + jsonschema.Draft202012Validator(_schema("deploy_scale.json")).validate(data) diff --git a/tests/comfy_cli/command/test_deploy_status.py b/tests/comfy_cli/command/test_deploy_status.py index c6715701..e67a07aa 100644 --- a/tests/comfy_cli/command/test_deploy_status.py +++ b/tests/comfy_cli/command/test_deploy_status.py @@ -305,3 +305,19 @@ def test_watch_exits_promptly_on_stop_failed_with_retry_stop_hint(tmp_path, monk assert client.get_calls == ["dep-status"] assert client.get_statuses == ["ready"] assert sleeps == [] + + +def test_a_deployment_with_startup_args_validates_against_the_published_status_schema(tmp_path, monkeypatch) -> None: + # Given + row = _status_deployment() + row["computeConfig"] = {"gpuClass": "l4", "region": "US-MO-2", "startupArgs": ["--highvram", "--reserve-vram", "2"]} + _install_clients(monkeypatch, FakeBuilder([_release(5)]), RecordingDeploy([row]), []) + + # When + result = _invoke_json(write_spec(tmp_path)) + + # Then + assert result.exit_code == 0, result.stderr + data = _json_envelope(result)["data"] + assert data["deployment"]["computeConfig"]["startupArgs"] == ["--highvram", "--reserve-vram", "2"] + jsonschema.Draft202012Validator(_schema("deploy_status.json")).validate(data) diff --git a/tests/comfy_cli/command/test_deploy_up.py b/tests/comfy_cli/command/test_deploy_up.py index 9b543771..491a7609 100644 --- a/tests/comfy_cli/command/test_deploy_up.py +++ b/tests/comfy_cli/command/test_deploy_up.py @@ -407,7 +407,7 @@ def test_bounds_supplied_to_a_restart_are_reported_rather_than_dropped(status: s result = module.reconcile_up(FakeBuilder(), client, _request(module, minimum=3, maximum=5)) # Then - assert result.dropped_bounds == ("--min", "--max") + assert result.dropped_flags == ("--min", "--max") assert result.compute_config == {"gpuClass": "l4", "region": "US-MO-2", "min": 2, "max": 8} assert client.update_calls == [] @@ -421,7 +421,7 @@ def test_a_restart_reports_only_the_bound_that_would_have_changed() -> None: result = module.reconcile_up(FakeBuilder(), client, _request(module, minimum=2, maximum=5)) # Then - assert result.dropped_bounds == ("--max",) + assert result.dropped_flags == ("--max",) def test_a_restart_without_bounds_reports_nothing() -> None: @@ -433,7 +433,7 @@ def test_a_restart_without_bounds_reports_nothing() -> None: result = module.reconcile_up(FakeBuilder(), client, _request(module, minimum=None, maximum=None)) # Then - assert result.dropped_bounds == () + assert result.dropped_flags == () def test_the_dropped_bound_warning_reaches_a_json_caller_on_stderr(tmp_path, monkeypatch) -> None: @@ -443,11 +443,16 @@ def test_the_dropped_bound_warning_reaches_a_json_caller_on_stderr(tmp_path, mon monkeypatch.setattr(module, "_command_clients", lambda: (FakeBuilder(), client)) # When - result = CliRunner().invoke(app, ["--json", "deploy", "up", str(write_spec(tmp_path)), "--min", "3", "--max", "8"]) + result = CliRunner().invoke( + app, + ["--json", "deploy", "up", str(write_spec(tmp_path)), "--min", "3", "--max", "8"], + env={"COLUMNS": "400"}, + ) # Then assert "--min had no effect" in result.stderr - assert "comfy deploy scale --deployment" in result.stderr + assert "comfy deploy scale --deployment dep-live --min --max " in result.stderr + assert "comfy deploy stop" not in result.stderr assert _json_envelope(result)["data"]["computeConfig"]["min"] == 2 @@ -597,3 +602,197 @@ def test_watch_continues_through_unhealthy_until_ready(tmp_path, monkeypatch) -> assert result.exit_code == 0 assert _json_envelope(result)["data"]["deployment"]["status"] == "ready" assert sleeps == [DEPLOY_POLL_SECONDS] + + +def test_up_registers_the_startup_arg_option() -> None: + assert "--startup-arg" in option_names("up") + + +def test_a_created_deployment_carries_its_startup_args() -> None: + # Given + module = _deploy() + client = FakeDeploy([]) + + # When + result = module.reconcile_up( + FakeBuilder(), client, _request(module, startup_args=("--highvram", "--reserve-vram", "2")) + ) + + # Then + assert result.created is True + assert result.compute_config["startupArgs"] == ["--highvram", "--reserve-vram", "2"] + assert client.rows["dep-1"]["computeConfig"]["startupArgs"] == ["--highvram", "--reserve-vram", "2"] + _validate_compute_config(result.compute_config) + + +def test_a_created_deployment_without_startup_args_sends_no_key() -> None: + """Absence and an empty list mean different things to the service (an empty + list on an edit is a clear), so a create that names no flags sends nothing.""" + # Given + module = _deploy() + client = FakeDeploy([]) + + # When + result = module.reconcile_up(FakeBuilder(), client, _request(module)) + + # Then + assert "startupArgs" not in result.compute_config + assert "startupArgs" not in client.rows["dep-1"]["computeConfig"] + + +def test_reconcile_refuses_a_startup_arg_change_on_a_running_deployment() -> None: + """The service refuses it with a 409 for the same reason it refuses a gpu + change; refusing here names the remedy in the user's vocabulary.""" + # Given + module = _deploy() + client = FakeDeploy([deployment("dep-live")]) + + # When / Then + with pytest.raises(DeployAPIError) as raised: + module.reconcile_up(FakeBuilder(), client, _request(module, startup_args=("--highvram",))) + assert raised.value.code == "deploy_immutable_compute" + assert raised.value.hint == ( + "run `comfy deploy stop --deployment dep-live`, then " + "`comfy deploy scale --deployment dep-live --startup-arg=`, then " + "`comfy deploy start --deployment dep-live`" + ) + assert client.update_calls == [] + assert client.create_keys == [] + + +def test_reconcile_with_the_live_startup_args_is_a_noop() -> None: + # Given + module = _deploy() + client = FakeDeploy([deployment("dep-live", minimum=2, maximum=8, startup_args=["--highvram"])]) + + # When + result = module.reconcile_up( + FakeBuilder(), client, _request(module, minimum=None, maximum=None, startup_args=("--highvram",)) + ) + + # Then + assert result.changed is False + assert client.update_calls == [] + assert result.compute_config["startupArgs"] == ["--highvram"] + + +def test_a_bounds_edit_on_a_running_deployment_keeps_its_startup_args() -> None: + """An omitted `--startup-arg` is never a change. The request leaves the field + out, as `scale` does, so the service keeps the stored flags and a copy read + before the edit cannot overwrite a newer set; the result still reports them.""" + # Given + module = _deploy() + client = FakeDeploy([deployment("dep-live", minimum=2, maximum=8, startup_args=["--highvram"])]) + + # When + result = module.reconcile_up(FakeBuilder(), client, _request(module, minimum=0, maximum=4)) + + # Then + assert client.update_bodies == [{"gpuClass": "l4", "region": "US-MO-2", "min": 0, "max": 4}] + assert client.rows["dep-live"]["computeConfig"]["startupArgs"] == ["--highvram"] + assert result.compute_config == { + "gpuClass": "l4", + "region": "US-MO-2", + "min": 0, + "max": 4, + "startupArgs": ["--highvram"], + } + + +@pytest.mark.parametrize("status", ["stopped", "failed", "stop_failed"]) +def test_startup_args_supplied_to_a_restart_are_reported_rather_than_dropped(status: str) -> None: + # Given + module = _deploy() + client = FakeDeploy([deployment("dep-live", status=status)]) + + # When + result = module.reconcile_up( + FakeBuilder(), client, _request(module, minimum=None, maximum=None, startup_args=("--highvram",)) + ) + + # Then + assert result.dropped_flags == ("--startup-arg",) + assert client.start_calls == ([] if status == "stop_failed" else ["dep-live"]) + assert client.update_calls == [] + assert "startupArgs" not in result.compute_config + + +def test_a_restart_that_dropped_startup_args_is_pointed_at_stop_scale_start(tmp_path, monkeypatch) -> None: + """The restart just left the only state in which flags can be set, so a bare + `scale --startup-arg` would bounce; the remedy names all three steps.""" + # Given + module = _deploy() + client = FakeDeploy([deployment("dep-live", status="stopped", minimum=2, maximum=8)]) + monkeypatch.setattr(module, "_command_clients", lambda: (FakeBuilder(), client)) + + # When + result = CliRunner().invoke( + app, + ["--json", "deploy", "up", str(write_spec(tmp_path)), "--startup-arg=--highvram", "--min", "3", "--max", "8"], + env={"COLUMNS": "400"}, + ) + + # Then + assert "--min and --startup-arg had no effect" in result.stderr + assert "run `comfy deploy stop --deployment dep-live`, then" in result.stderr + assert "`comfy deploy scale --deployment dep-live --startup-arg= --min --max `, then" in result.stderr + assert "`comfy deploy start --deployment dep-live`" in result.stderr + assert client.start_calls == ["dep-live"] + + +def test_up_with_startup_args_validates_against_the_published_up_schema(tmp_path, monkeypatch) -> None: + # Given + module = _deploy() + client = FakeDeploy([]) + monkeypatch.setattr(module, "_command_clients", lambda: (FakeBuilder(), client)) + + # When + result = CliRunner().invoke( + app, + [ + "--json", + "deploy", + "up", + str(write_spec(tmp_path)), + "--gpu", + "l4", + "--region", + "US-MO-2", + "--startup-arg=--highvram", + "--startup-arg=--reserve-vram", + "--startup-arg=2", + ], + ) + + # Then + assert result.exit_code == 0, result.stderr + data = _json_envelope(result)["data"] + assert data["computeConfig"]["startupArgs"] == ["--highvram", "--reserve-vram", "2"] + jsonschema.Draft202012Validator(_schema("deploy_up.json")).validate(data) + + +def test_compute_config_carries_startup_args_and_refuses_a_malformed_set() -> None: + from comfy_cli.command.deploy_types import compute_config + + row = deployment("dep-1") + row["computeConfig"]["startupArgs"] = ["--highvram", "--reserve-vram", "2"] + assert compute_config(row)["startupArgs"] == ["--highvram", "--reserve-vram", "2"] + + row["computeConfig"]["startupArgs"] = "--highvram" + with pytest.raises(DeployAPIError) as raised: + compute_config(row) + assert raised.value.code == "deploy_server_error" + + for blank in ("", " "): + row["computeConfig"]["startupArgs"] = ["--highvram", blank] + with pytest.raises(DeployAPIError): + compute_config(row) + + +def test_client_validation_refuses_a_startup_args_shape_the_service_would_reject() -> None: + _validate_compute_config({"gpuClass": "l4", "region": "US-MO-2", "startupArgs": ["--highvram"]}) + _validate_compute_config({"gpuClass": "l4", "region": "US-MO-2", "startupArgs": []}) + for bad in ("--highvram", ["--highvram", " "], [1]): + with pytest.raises(DeployAPIError) as raised: + _validate_compute_config({"gpuClass": "l4", "region": "US-MO-2", "startupArgs": bad}) + assert raised.value.code == "deploy_bad_request" diff --git a/tests/comfy_cli/output/test_error_code_registry.py b/tests/comfy_cli/output/test_error_code_registry.py index 105a30d7..6ee6e3c7 100644 --- a/tests/comfy_cli/output/test_error_code_registry.py +++ b/tests/comfy_cli/output/test_error_code_registry.py @@ -277,14 +277,14 @@ def test_deploy_error_codes_are_the_exact_final_set() -> None: deploy_server_error deploy_idempotency_reuse deploy_workflow_format_ui deploy_workflow_asset_outside_root deploy_workflow_asset_marker_reserved deploy_insecure_url deploy_unrelated_deployment deploy_workflow_empty - deploy_workflow_not_api_format deploy_status_terminal""".split() + deploy_workflow_not_api_format deploy_status_terminal deploy_conflicting_input""".split() ) # When actual = {code for code in error_codes.all_codes() if code.startswith("deploy_")} # Then - assert len(actual) == 34 + assert len(actual) == 35 assert actual == expected