diff --git a/comfy_cli/command/custom_nodes/command.py b/comfy_cli/command/custom_nodes/command.py index b995ad1e..723f3e0e 100644 --- a/comfy_cli/command/custom_nodes/command.py +++ b/comfy_cli/command/custom_nodes/command.py @@ -601,7 +601,7 @@ def install( ), ): if "all" in nodes: - typer.echo(f"Invalid command: {mode}. `install all` is not allowed", err=True) + typer.echo("`install all` is not allowed", err=True) raise typer.Exit(code=1) exclusive_flags = [ @@ -670,7 +670,7 @@ def reinstall( ), ): if "all" in nodes: - typer.echo(f"Invalid command: {mode}. `reinstall all` is not allowed", err=True) + typer.echo("`reinstall all` is not allowed", err=True) raise typer.Exit(code=1) exclusive_flags = [name for name, val in [("--fast-deps", fast_deps), ("--uv-compile", uv_compile)] if val] @@ -715,7 +715,7 @@ def uninstall( ), ): if "all" in nodes: - typer.echo(f"Invalid command: {mode}. `uninstall all` is not allowed", err=True) + typer.echo("`uninstall all` is not allowed", err=True) raise typer.Exit(code=1) validate_mode(mode) @@ -1155,7 +1155,7 @@ def registry_install( node_specific_path = custom_nodes_path / node_id # Subdirectory for the node if node_specific_path.exists(): print( - f"[bold red] The node {node_id} already exists in the workspace. This migit delete any model files in the node.[/bold red]" + f"[bold red] The node {node_id} already exists in the workspace. This might delete any model files in the node.[/bold red]" ) confirm = ui.prompt_confirm_action( diff --git a/comfy_cli/command/install.py b/comfy_cli/command/install.py index 24ce1a59..efaf21ca 100755 --- a/comfy_cli/command/install.py +++ b/comfy_cli/command/install.py @@ -430,7 +430,7 @@ def handle_github_rate_limit(response): remaining = int(response.headers.get("x-ratelimit-remaining", 0)) if remaining == 0: reset_time = int(response.headers.get("x-ratelimit-reset", 0)) - message = f"Primary rate limit from Github exceeded! Please retry after: {reset_time})" + message = f"Primary rate limit from Github exceeded! Please retry after: {reset_time}" raise GitHubRateLimitError(message) if "retry-after" in response.headers: diff --git a/comfy_cli/command/launch.py b/comfy_cli/command/launch.py index 61d0e8c3..07b1c928 100644 --- a/comfy_cli/command/launch.py +++ b/comfy_cli/command/launch.py @@ -87,7 +87,7 @@ def launch_comfyui(extra, frontend_pr=None, python=sys.executable): if reboot_path is None: print("[bold red]ComfyUI is not installed.[/bold red]\n") - exit(res) + exit(res.returncode) if not os.path.exists(reboot_path): exit(res.returncode) @@ -135,10 +135,10 @@ def redirector_stdout(): if reboot_path is None: print("[bold red]ComfyUI is not installed.[/bold red]\n") - os._exit(process.pid) + os._exit(1) if not os.path.exists(reboot_path): - os._exit(process.pid) + os._exit(process.returncode) os.remove(reboot_path) except KeyboardInterrupt: diff --git a/tests/comfy_cli/command/github/test_pr.py b/tests/comfy_cli/command/github/test_pr.py index 47f747b4..5922e47e 100644 --- a/tests/comfy_cli/command/github/test_pr.py +++ b/tests/comfy_cli/command/github/test_pr.py @@ -6,7 +6,15 @@ from typer.testing import CliRunner from comfy_cli.cmdline import app, g_exclusivity, g_gpu_exclusivity -from comfy_cli.command.install import PRInfo, fetch_pr_info, find_pr_by_branch, handle_pr_checkout, parse_pr_reference +from comfy_cli.command.install import ( + GitHubRateLimitError, + PRInfo, + fetch_pr_info, + find_pr_by_branch, + handle_github_rate_limit, + handle_pr_checkout, + parse_pr_reference, +) from comfy_cli.git_utils import checkout_pr @@ -437,5 +445,32 @@ def test_checkout_pr_remote_already_exists(self, mock_getcwd, mock_chdir, mock_s assert mock_subprocess.call_count == 3 +class TestHandleGithubRateLimit: + def test_primary_rate_limit_message_format(self): + """Verify the error message does not contain stray characters.""" + mock_response = Mock() + mock_response.headers = {"x-ratelimit-remaining": "0", "x-ratelimit-reset": "1700000000"} + + with pytest.raises(GitHubRateLimitError) as exc_info: + handle_github_rate_limit(mock_response) + + msg = str(exc_info.value) + assert "1700000000" in msg + assert msg.endswith("1700000000") # no stray trailing characters + + def test_retry_after_header(self): + mock_response = Mock() + mock_response.headers = {"x-ratelimit-remaining": "5", "retry-after": "30"} + + with pytest.raises(GitHubRateLimitError, match="30 seconds"): + handle_github_rate_limit(mock_response) + + def test_no_rate_limit_does_not_raise(self): + mock_response = Mock() + mock_response.headers = {"x-ratelimit-remaining": "100"} + + handle_github_rate_limit(mock_response) # should not raise + + if __name__ == "__main__": pytest.main([__file__]) diff --git a/tests/comfy_cli/command/nodes/test_node_install.py b/tests/comfy_cli/command/nodes/test_node_install.py index 5c9fc7d0..2239879c 100644 --- a/tests/comfy_cli/command/nodes/test_node_install.py +++ b/tests/comfy_cli/command/nodes/test_node_install.py @@ -215,11 +215,15 @@ def test_fix_with_uv_compile(): def test_uninstall_rejects_all(): result = runner.invoke(app, ["uninstall", "all"]) assert result.exit_code != 0 + assert "`uninstall all` is not allowed" in result.output + assert "Invalid command" not in result.output def test_reinstall_rejects_all(): result = runner.invoke(app, ["reinstall", "all"]) assert result.exit_code != 0 + assert "`reinstall all` is not allowed" in result.output + assert "Invalid command" not in result.output def test_validate_mode_rejects_invalid(): @@ -310,7 +314,8 @@ def test_install_deps_with_workflow(tmp_path): def test_install_rejects_all(): result = runner.invoke(app, ["install", "all"]) assert result.exit_code != 0 - assert "not allowed" in result.output + assert "`install all` is not allowed" in result.output + assert "Invalid command" not in result.output def test_simple_show_installed(): diff --git a/tests/comfy_cli/test_launch_python_resolution.py b/tests/comfy_cli/test_launch_python_resolution.py index 7f2163b0..418cafb6 100644 --- a/tests/comfy_cli/test_launch_python_resolution.py +++ b/tests/comfy_cli/test_launch_python_resolution.py @@ -23,6 +23,20 @@ def test_uses_python_param(self): assert cmd[0] == "/resolved/python" assert cmd[0] != sys.executable + @pytest.mark.parametrize("returncode", [0, 1, 42]) + def test_foreground_exit_code_matches_subprocess(self, returncode): + """exit() should receive the subprocess returncode, not the CompletedProcess object.""" + mock_result = subprocess.CompletedProcess(args=[], returncode=returncode) + + with ( + patch("comfy_cli.command.launch.ConfigManager"), + patch("comfy_cli.command.launch.subprocess.run", return_value=mock_result), + ): + with pytest.raises(SystemExit) as exc_info: + launch.launch_comfyui(extra=[], python="/resolved/python") + + assert exc_info.value.code == returncode + class TestLaunchResolvesWorkspacePython: def test_resolves_and_passes_python(self):