From 27d3427de0563bc29c3c66b1458f815f84eaee94 Mon Sep 17 00:00:00 2001 From: Juan Diaz Date: Mon, 13 Jul 2026 15:01:18 +0000 Subject: [PATCH] Add run/shell `--force-start` flag for error VMs --- src/sbx/cli.py | 57 ++++++++++++++++++++++++++++++++-------- tests/test_cli.py | 4 +-- tests/test_cli_extra.py | 58 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 105 insertions(+), 14 deletions(-) diff --git a/src/sbx/cli.py b/src/sbx/cli.py index 53af926..68ab3a6 100644 --- a/src/sbx/cli.py +++ b/src/sbx/cli.py @@ -630,16 +630,32 @@ def _maybe_print_boot_timeout_running_hint(vm_id: str | None, boot_timeout: floa return True -def _start_existing_vm_if_needed(vm_id: str, status: str, boot_timeout: float) -> int: +def _mark_error_vm_stopped_for_restart(vm_id: str) -> None: + db_path = SMOLVM_DB_PATH.expanduser() + if not db_path.exists(): + return + + with sqlite3.connect(db_path) as conn: + conn.execute( + "UPDATE vms SET status = 'stopped', pid = NULL, socket_path = NULL WHERE id = ?", + (vm_id,), + ) + + +def _start_existing_vm_if_needed( + vm_id: str, status: str, boot_timeout: float, *, force_start: bool = False +) -> int: if status == "running": return 0 if status == "error": - print( - f"sbx: VM '{vm_id}' is in error state; run `sbx recreate {vm_id} --force` " - "to delete it and create a fresh VM.", - file=sys.stderr, - ) - return 1 + if not force_start: + print( + f"sbx: VM '{vm_id}' is in error state; retry with `--force-start` or run " + f"`sbx recreate {vm_id} --force` to delete it and create a fresh VM.", + file=sys.stderr, + ) + return 1 + _mark_error_vm_stopped_for_restart(vm_id) rc = _run_smolvm(["sandbox", "start", vm_id, "--boot-timeout", f"{boot_timeout:g}"]) if rc != 0: _maybe_print_boot_timeout_running_hint(vm_id, boot_timeout) @@ -1645,7 +1661,10 @@ def cmd_start(args: argparse.Namespace) -> int: print(f"sbx: {exc}", file=sys.stderr) return 2 start_rc = _start_existing_vm_if_needed( - str(requested_name), existing_status, boot_timeout + str(requested_name), + existing_status, + boot_timeout, + force_start=bool(getattr(args, "force_start", False)), ) if start_rc != 0: return start_rc @@ -1871,7 +1890,12 @@ def cmd_passthrough(args: argparse.Namespace) -> int: print(f"sbx: {_missing_vm_message(name)}", file=sys.stderr) return 1 if existing_status is not None: - start_rc = _start_existing_vm_if_needed(name, existing_status, DEFAULT_BOOT_TIMEOUT) + start_rc = _start_existing_vm_if_needed( + name, + existing_status, + DEFAULT_BOOT_TIMEOUT, + force_start=bool(getattr(args, "force_start", False)), + ) if start_rc != 0: return start_rc if not _sync_forwarded_env_or_error(name, forward_env): @@ -2159,7 +2183,16 @@ def build_parser() -> argparse.ArgumentParser: parser.add_argument("--version", action="version", version=f"sbx {__version__}") sub = parser.add_subparsers(dest="action", required=True) - run = sub.add_parser("run", help="Run an agent session in a sandbox.") + force_start_parent = argparse.ArgumentParser(add_help=False) + force_start_parent.add_argument( + "--force-start", + action="store_true", + help="Retry starting an existing VM even when its status is error.", + ) + + run = sub.add_parser( + "run", help="Run an agent session in a sandbox.", parents=[force_start_parent] + ) _add_start_options(run) run.set_defaults(func=cmd_start) @@ -2183,7 +2216,9 @@ def build_parser() -> argparse.ArgumentParser: stop.add_argument("name", nargs="?", help="Sandbox name. Defaults to [sbx].name.") stop.set_defaults(func=cmd_passthrough) - shell = sub.add_parser("shell", help="Open a shell in a sandbox.") + shell = sub.add_parser( + "shell", help="Open a shell in a sandbox.", parents=[force_start_parent] + ) shell.add_argument( "--keep-running", action="store_true", diff --git a/tests/test_cli.py b/tests/test_cli.py index 904aee7..c4cf537 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -350,7 +350,7 @@ def test_shell_starts_stopped_vm_before_env_sync( monkeypatch.setattr(cli, "_get_existing_vm_status", lambda name: "stopped") monkeypatch.setattr( - cli, "_start_existing_vm_if_needed", lambda *args: calls.append("start") or 0 + cli, "_start_existing_vm_if_needed", lambda *args, **kwargs: calls.append("start") or 0 ) monkeypatch.setattr(cli, "_sync_forwarded_env", lambda *args: calls.append("sync")) monkeypatch.setattr(cli, "_run_smolvm", lambda *args, **kwargs: calls.append("attach") or 0) @@ -1715,7 +1715,7 @@ def test_existing_vm_start_does_not_reset_hostname(monkeypatch: pytest.MonkeyPat hostnames: list[str] = [] monkeypatch.setattr(cli, "_set_vm_hostname", hostnames.append) monkeypatch.setattr(cli, "_get_existing_vm_status", lambda name: "stopped") - monkeypatch.setattr(cli, "_start_existing_vm_if_needed", lambda name, status, timeout: 0) + monkeypatch.setattr(cli, "_start_existing_vm_if_needed", lambda *args, **kwargs: 0) monkeypatch.setattr(cli, "_post_start_actions", lambda **kwargs: 0) assert cli.main(["run", "vm1", "--no-attach", "--no-auth-port"]) == 0 diff --git a/tests/test_cli_extra.py b/tests/test_cli_extra.py index 8925cfe..0f8bc99 100644 --- a/tests/test_cli_extra.py +++ b/tests/test_cli_extra.py @@ -164,7 +164,7 @@ def test_cmd_start_syncs_env_before_existing_vm_attach( monkeypatch.setattr(cli, "_get_existing_vm_status", lambda vm_id: "running") monkeypatch.setattr( - cli, "_start_existing_vm_if_needed", lambda *args: calls.append("start") or 0 + cli, "_start_existing_vm_if_needed", lambda *args, **kwargs: calls.append("start") or 0 ) monkeypatch.setattr(cli, "_host_git_config", lambda: None) monkeypatch.setattr(cli, "_sync_forwarded_env", lambda *args: calls.append("sync")) @@ -1373,12 +1373,68 @@ def test_recreate_success_deletes_then_starts(monkeypatch: pytest.MonkeyPatch) - def test_start_existing_vm_if_needed_variants(monkeypatch: pytest.MonkeyPatch) -> None: calls: list[list[str]] = [] + marked: list[str] = [] monkeypatch.setattr(cli, "_run_smolvm", lambda argv: calls.append(list(argv)) or 0) + monkeypatch.setattr( + cli, "_mark_error_vm_stopped_for_restart", lambda vm_id: marked.append(vm_id) + ) + assert cli._start_existing_vm_if_needed("vm1", "running", 60) == 0 assert calls == [] assert cli._start_existing_vm_if_needed("vm1", "stopped", 60) == 0 assert calls == [["sandbox", "start", "vm1", "--boot-timeout", "60"]] assert cli._start_existing_vm_if_needed("vm1", "error", 60) == 1 + assert marked == [] + assert cli._start_existing_vm_if_needed("vm1", "error", 60, force_start=True) == 0 + assert marked == ["vm1"] + assert calls[-1] == ["sandbox", "start", "vm1", "--boot-timeout", "60"] + + +def test_mark_error_vm_stopped_for_restart_clears_stale_runtime_fields() -> None: + with sqlite3.connect(cli.SMOLVM_DB_PATH) as conn: + conn.execute( + "CREATE TABLE vms (id TEXT PRIMARY KEY, status TEXT, pid INTEGER, socket_path TEXT)" + ) + conn.execute( + "INSERT INTO vms (id, status, pid, socket_path) VALUES (?, ?, ?, ?)", + ("vm1", "error", 123, "/tmp/stale.sock"), + ) + + cli._mark_error_vm_stopped_for_restart("vm1") + + with sqlite3.connect(cli.SMOLVM_DB_PATH) as conn: + row = conn.execute( + "SELECT status, pid, socket_path FROM vms WHERE id = ?", ("vm1",) + ).fetchone() + assert row == ("stopped", None, None) + + +@pytest.mark.parametrize( + "argv", + [ + ["run", "vm1", "--force-start", "--no-attach"], + ["shell", "--force-start", "--keep-running", "--no-git-config", "vm1"], + ], +) +def test_force_start_is_passed_to_existing_vm_start( + monkeypatch: pytest.MonkeyPatch, argv: list[str] +) -> None: + captured: dict[str, object] = {} + monkeypatch.setattr(cli, "_get_existing_vm_status", lambda vm_id: "error") + monkeypatch.setattr( + cli, + "_start_existing_vm_if_needed", + lambda vm_id, status, timeout, *, force_start=False: captured.update( + {"vm_id": vm_id, "status": status, "force_start": force_start} + ) + or 0, + ) + monkeypatch.setattr(cli, "_post_start_actions", lambda **kwargs: 0) + monkeypatch.setattr(cli, "_sync_forwarded_env_or_error", lambda vm_id, names: True) + monkeypatch.setattr(cli, "_run_smolvm", lambda argv: 0) + + assert cli.main(argv) == 0 + assert captured == {"vm_id": "vm1", "status": "error", "force_start": True} def test_start_existing_vm_timeout_hint_when_vm_is_running(