From 10e18a2dfe4e0748f0e1d2266e7d4b94a2912ae0 Mon Sep 17 00:00:00 2001 From: James Chang Date: Wed, 20 May 2026 02:36:48 -0700 Subject: [PATCH 1/6] Use shutdown-first detached reboot wrappers --- src/timecapsulesmb/cli/flows.py | 2 +- src/timecapsulesmb/cli/fsck.py | 8 +++++++- src/timecapsulesmb/deploy/executor.py | 7 ++++++- tests/test_cli.py | 4 ++++ tests/test_cli_flows.py | 2 +- tests/test_deploy_modules.py | 5 +++++ 6 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/timecapsulesmb/cli/flows.py b/src/timecapsulesmb/cli/flows.py index 40a5d45..9f2fe90 100644 --- a/src/timecapsulesmb/cli/flows.py +++ b/src/timecapsulesmb/cli/flows.py @@ -178,7 +178,7 @@ def _request_reboot_via_ssh( *, log: LogCallback = None, request_reboot: Callable[[SshConnection], None] | None = None, - progress_message: str = "SSH: /sbin/reboot", + progress_message: str = "SSH: /bin/sync; /sbin/shutdown -r now (fallback /sbin/reboot)", ) -> None: command_context.add_debug_fields(ssh_reboot_attempted=True) emit_progress(log, progress_message) diff --git a/src/timecapsulesmb/cli/fsck.py b/src/timecapsulesmb/cli/fsck.py index c3aa3e0..57f0961 100644 --- a/src/timecapsulesmb/cli/fsck.py +++ b/src/timecapsulesmb/cli/fsck.py @@ -18,6 +18,12 @@ FSCK_REBOOT_NO_DOWN_MESSAGE = "fsck requested reboot from the device, but SSH did not go down." FSCK_REMOTE_COMMAND_TIMEOUT_SECONDS = 3 * 60 * 60 +DETACHED_FSCK_REBOOT_COMMAND = ( + "/bin/sh -c 'exec /dev/null 2>&1; " + "(/bin/sync; /bin/sleep 1; " + "if [ -x /sbin/shutdown ]; then /sbin/shutdown -r now || /sbin/reboot; else /sbin/reboot; fi" + ") & exit 0'" +) NO_MOUNTED_HFS_VOLUMES_MESSAGE = "no mounted HFS volumes found" MULTIPLE_MOUNTED_HFS_VOLUMES_MESSAGE = "multiple mounted HFS volumes found; specify --volume to select one" @@ -89,7 +95,7 @@ def build_remote_fsck_script(device: str, mountpoint: str, *, reboot: bool) -> s lines.extend( [ "echo '--- reboot ---'", - "/sbin/reboot >/dev/null 2>&1 || true", + DETACHED_FSCK_REBOOT_COMMAND, ] ) return "\n".join(lines) diff --git a/src/timecapsulesmb/deploy/executor.py b/src/timecapsulesmb/deploy/executor.py index 646eaa3..b86cc64 100644 --- a/src/timecapsulesmb/deploy/executor.py +++ b/src/timecapsulesmb/deploy/executor.py @@ -10,7 +10,12 @@ from timecapsulesmb.transport.ssh import SshConnection, run_scp, run_ssh -DETACHED_REBOOT_COMMAND = "/bin/sh -c 'exec /dev/null 2>&1; (/bin/sleep 1; /sbin/reboot) & exit 0'" +DETACHED_REBOOT_COMMAND = ( + "/bin/sh -c 'exec /dev/null 2>&1; " + "(/bin/sync; /bin/sleep 1; " + "if [ -x /sbin/shutdown ]; then /sbin/shutdown -r now || /sbin/reboot; else /sbin/reboot; fi" + ") & exit 0'" +) DETACHED_SHUTDOWN_REBOOT_COMMAND = ( "/bin/sh -c 'exec /dev/null 2>&1; " "(/bin/sleep 1; " diff --git a/tests/test_cli.py b/tests/test_cli.py index 57d7e64..5b644fe 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -7324,7 +7324,11 @@ def test_fsck_yes_reboots_and_waits_by_default(self) -> None: self.assertIn("^wcifsfs$", remote_cmd) self.assertIn("umount -f /Volumes/dk2", remote_cmd) self.assertIn("fsck_hfs -fy /dev/dk2", remote_cmd) + self.assertIn("exec /dev/null 2>&1", remote_cmd) + self.assertIn("/bin/sync; /bin/sleep 1;", remote_cmd) + self.assertIn("/sbin/shutdown -r now", remote_cmd) self.assertIn("/sbin/reboot", remote_cmd) + self.assertIn(") & exit 0", remote_cmd) self.assertEqual(wait_mock.call_args_list[0].kwargs, {"expected_up": False, "timeout_seconds": 90}) self.assertEqual(wait_mock.call_args_list[1].kwargs, {"expected_up": True, "timeout_seconds": 420}) text = output.getvalue() diff --git a/tests/test_cli_flows.py b/tests/test_cli_flows.py index 5028d1c..6174208 100644 --- a/tests/test_cli_flows.py +++ b/tests/test_cli_flows.py @@ -268,7 +268,7 @@ def test_request_ssh_reboot_uses_ssh_only_strategy_and_progress_log(self) -> Non self.assertEqual(command_context.debug_fields["reboot_request_strategy"], "ssh") self.assertEqual(command_context.debug_fields["ssh_reboot_attempted"], True) self.assertEqual(command_context.debug_fields["ssh_reboot_succeeded"], True) - self.assertEqual(messages, ["SSH: /sbin/reboot"]) + self.assertEqual(messages, ["SSH: /bin/sync; /sbin/shutdown -r now (fallback /sbin/reboot)"]) self.assertIn("SSH reboot requested.", output.getvalue()) def test_request_ssh_reboot_records_timeout_without_raising(self) -> None: diff --git a/tests/test_deploy_modules.py b/tests/test_deploy_modules.py index cf63eef..837ecd9 100644 --- a/tests/test_deploy_modules.py +++ b/tests/test_deploy_modules.py @@ -303,6 +303,11 @@ def test_remote_request_reboot_uses_explicit_reboot_timeout(self) -> None: check=False, timeout=REBOOT_REQUEST_TIMEOUT_SECONDS, ) + self.assertIn("exec /dev/null 2>&1", DETACHED_REBOOT_COMMAND) + self.assertIn("/bin/sync; /bin/sleep 1;", DETACHED_REBOOT_COMMAND) + self.assertIn("/sbin/shutdown -r now", DETACHED_REBOOT_COMMAND) + self.assertIn("|| /sbin/reboot", DETACHED_REBOOT_COMMAND) + self.assertIn(") & exit 0", DETACHED_REBOOT_COMMAND) def test_remote_request_shutdown_reboot_uses_shutdown_with_reboot_fallback(self) -> None: connection = SshConnection("root@10.0.0.2", "pw", "-o foo") From 3049b86f0ab8197aac4e0d6acbea28ed61880673 Mon Sep 17 00:00:00 2001 From: James Chang Date: Wed, 20 May 2026 02:46:55 -0700 Subject: [PATCH 2/6] Share detached reboot wrapper --- src/timecapsulesmb/cli/fsck.py | 9 ++------- src/timecapsulesmb/deploy/executor.py | 9 ++------- tests/test_deploy_modules.py | 4 ++++ 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/timecapsulesmb/cli/fsck.py b/src/timecapsulesmb/cli/fsck.py index 57f0961..0211c86 100644 --- a/src/timecapsulesmb/cli/fsck.py +++ b/src/timecapsulesmb/cli/fsck.py @@ -8,6 +8,7 @@ from timecapsulesmb.cli.context import CommandContext from timecapsulesmb.cli.flows import observe_reboot_cycle from timecapsulesmb.cli.runtime import add_config_argument, load_env_config +from timecapsulesmb.deploy.executor import DETACHED_REBOOT_COMMAND from timecapsulesmb.deploy.planner import DEFAULT_APPLE_MOUNT_WAIT_SECONDS from timecapsulesmb.device.processes import render_direct_pkill9_by_ucomm, render_direct_pkill9_watchdog from timecapsulesmb.identity import ensure_install_id @@ -18,12 +19,6 @@ FSCK_REBOOT_NO_DOWN_MESSAGE = "fsck requested reboot from the device, but SSH did not go down." FSCK_REMOTE_COMMAND_TIMEOUT_SECONDS = 3 * 60 * 60 -DETACHED_FSCK_REBOOT_COMMAND = ( - "/bin/sh -c 'exec /dev/null 2>&1; " - "(/bin/sync; /bin/sleep 1; " - "if [ -x /sbin/shutdown ]; then /sbin/shutdown -r now || /sbin/reboot; else /sbin/reboot; fi" - ") & exit 0'" -) NO_MOUNTED_HFS_VOLUMES_MESSAGE = "no mounted HFS volumes found" MULTIPLE_MOUNTED_HFS_VOLUMES_MESSAGE = "multiple mounted HFS volumes found; specify --volume to select one" @@ -95,7 +90,7 @@ def build_remote_fsck_script(device: str, mountpoint: str, *, reboot: bool) -> s lines.extend( [ "echo '--- reboot ---'", - DETACHED_FSCK_REBOOT_COMMAND, + DETACHED_REBOOT_COMMAND, ] ) return "\n".join(lines) diff --git a/src/timecapsulesmb/deploy/executor.py b/src/timecapsulesmb/deploy/executor.py index b86cc64..20f1524 100644 --- a/src/timecapsulesmb/deploy/executor.py +++ b/src/timecapsulesmb/deploy/executor.py @@ -10,18 +10,13 @@ from timecapsulesmb.transport.ssh import SshConnection, run_scp, run_ssh -DETACHED_REBOOT_COMMAND = ( - "/bin/sh -c 'exec /dev/null 2>&1; " - "(/bin/sync; /bin/sleep 1; " - "if [ -x /sbin/shutdown ]; then /sbin/shutdown -r now || /sbin/reboot; else /sbin/reboot; fi" - ") & exit 0'" -) DETACHED_SHUTDOWN_REBOOT_COMMAND = ( "/bin/sh -c 'exec /dev/null 2>&1; " - "(/bin/sleep 1; " + "(/bin/sync; /bin/sleep 1; " "if [ -x /sbin/shutdown ]; then /sbin/shutdown -r now || /sbin/reboot; else /sbin/reboot; fi" ") & exit 0'" ) +DETACHED_REBOOT_COMMAND = DETACHED_SHUTDOWN_REBOOT_COMMAND REBOOT_REQUEST_TIMEOUT_SECONDS = 30 PAYLOAD_FLUSH_SETTLE_SECONDS = 5 FLUSH_REMOTE_FILESYSTEMS_COMMAND = ( diff --git a/tests/test_deploy_modules.py b/tests/test_deploy_modules.py index 837ecd9..7776dc3 100644 --- a/tests/test_deploy_modules.py +++ b/tests/test_deploy_modules.py @@ -319,8 +319,12 @@ def test_remote_request_shutdown_reboot_uses_shutdown_with_reboot_fallback(self) check=False, timeout=REBOOT_REQUEST_TIMEOUT_SECONDS, ) + self.assertEqual(DETACHED_SHUTDOWN_REBOOT_COMMAND, DETACHED_REBOOT_COMMAND) + self.assertIn("exec /dev/null 2>&1", DETACHED_SHUTDOWN_REBOOT_COMMAND) + self.assertIn("/bin/sync; /bin/sleep 1;", DETACHED_SHUTDOWN_REBOOT_COMMAND) self.assertIn("/sbin/shutdown -r now", DETACHED_SHUTDOWN_REBOOT_COMMAND) self.assertIn("|| /sbin/reboot", DETACHED_SHUTDOWN_REBOOT_COMMAND) + self.assertIn(") & exit 0", DETACHED_SHUTDOWN_REBOOT_COMMAND) def test_flush_remote_filesystem_writes_syncs_and_waits(self) -> None: connection = SshConnection("root@10.0.0.2", "pw", "-o foo") From 8d306acdcdbbaf4c2bfcc2e10a80759c14eb28be Mon Sep 17 00:00:00 2001 From: James Chang Date: Wed, 20 May 2026 02:49:24 -0700 Subject: [PATCH 3/6] Simplify shutdown reboot wrapper --- src/timecapsulesmb/cli/fsck.py | 4 ++-- src/timecapsulesmb/deploy/executor.py | 5 ++--- tests/test_deploy_modules.py | 16 ++++++++-------- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/timecapsulesmb/cli/fsck.py b/src/timecapsulesmb/cli/fsck.py index 0211c86..78cffad 100644 --- a/src/timecapsulesmb/cli/fsck.py +++ b/src/timecapsulesmb/cli/fsck.py @@ -8,7 +8,7 @@ from timecapsulesmb.cli.context import CommandContext from timecapsulesmb.cli.flows import observe_reboot_cycle from timecapsulesmb.cli.runtime import add_config_argument, load_env_config -from timecapsulesmb.deploy.executor import DETACHED_REBOOT_COMMAND +from timecapsulesmb.deploy.executor import DETACHED_SHUTDOWN_REBOOT_COMMAND from timecapsulesmb.deploy.planner import DEFAULT_APPLE_MOUNT_WAIT_SECONDS from timecapsulesmb.device.processes import render_direct_pkill9_by_ucomm, render_direct_pkill9_watchdog from timecapsulesmb.identity import ensure_install_id @@ -90,7 +90,7 @@ def build_remote_fsck_script(device: str, mountpoint: str, *, reboot: bool) -> s lines.extend( [ "echo '--- reboot ---'", - DETACHED_REBOOT_COMMAND, + DETACHED_SHUTDOWN_REBOOT_COMMAND, ] ) return "\n".join(lines) diff --git a/src/timecapsulesmb/deploy/executor.py b/src/timecapsulesmb/deploy/executor.py index 20f1524..6b6fd3b 100644 --- a/src/timecapsulesmb/deploy/executor.py +++ b/src/timecapsulesmb/deploy/executor.py @@ -13,10 +13,9 @@ DETACHED_SHUTDOWN_REBOOT_COMMAND = ( "/bin/sh -c 'exec /dev/null 2>&1; " "(/bin/sync; /bin/sleep 1; " - "if [ -x /sbin/shutdown ]; then /sbin/shutdown -r now || /sbin/reboot; else /sbin/reboot; fi" + "/sbin/shutdown -r now || /sbin/reboot" ") & exit 0'" ) -DETACHED_REBOOT_COMMAND = DETACHED_SHUTDOWN_REBOOT_COMMAND REBOOT_REQUEST_TIMEOUT_SECONDS = 30 PAYLOAD_FLUSH_SETTLE_SECONDS = 5 FLUSH_REMOTE_FILESYSTEMS_COMMAND = ( @@ -120,7 +119,7 @@ def run_remote_actions(connection: SshConnection, actions: Iterable[RemoteAction def remote_request_reboot(connection: SshConnection) -> None: - run_ssh(connection, DETACHED_REBOOT_COMMAND, check=False, timeout=REBOOT_REQUEST_TIMEOUT_SECONDS) + run_ssh(connection, DETACHED_SHUTDOWN_REBOOT_COMMAND, check=False, timeout=REBOOT_REQUEST_TIMEOUT_SECONDS) def remote_request_shutdown_reboot(connection: SshConnection) -> None: diff --git a/tests/test_deploy_modules.py b/tests/test_deploy_modules.py index 7776dc3..05058a3 100644 --- a/tests/test_deploy_modules.py +++ b/tests/test_deploy_modules.py @@ -40,7 +40,6 @@ ) from timecapsulesmb.deploy.dry_run import format_deployment_plan from timecapsulesmb.deploy.executor import ( - DETACHED_REBOOT_COMMAND, DETACHED_SHUTDOWN_REBOOT_COMMAND, FLUSH_REMOTE_FILESYSTEMS_COMMAND, FLUSH_REMOTE_FILESYSTEMS_TIMEOUT_SECONDS, @@ -299,15 +298,16 @@ def test_remote_request_reboot_uses_explicit_reboot_timeout(self) -> None: remote_request_reboot(connection) run_ssh_mock.assert_called_once_with( connection, - DETACHED_REBOOT_COMMAND, + DETACHED_SHUTDOWN_REBOOT_COMMAND, check=False, timeout=REBOOT_REQUEST_TIMEOUT_SECONDS, ) - self.assertIn("exec /dev/null 2>&1", DETACHED_REBOOT_COMMAND) - self.assertIn("/bin/sync; /bin/sleep 1;", DETACHED_REBOOT_COMMAND) - self.assertIn("/sbin/shutdown -r now", DETACHED_REBOOT_COMMAND) - self.assertIn("|| /sbin/reboot", DETACHED_REBOOT_COMMAND) - self.assertIn(") & exit 0", DETACHED_REBOOT_COMMAND) + self.assertIn("exec /dev/null 2>&1", DETACHED_SHUTDOWN_REBOOT_COMMAND) + self.assertIn("/bin/sync; /bin/sleep 1;", DETACHED_SHUTDOWN_REBOOT_COMMAND) + self.assertIn("/sbin/shutdown -r now", DETACHED_SHUTDOWN_REBOOT_COMMAND) + self.assertIn("|| /sbin/reboot", DETACHED_SHUTDOWN_REBOOT_COMMAND) + self.assertNotIn("[ -x /sbin/shutdown ]", DETACHED_SHUTDOWN_REBOOT_COMMAND) + self.assertIn(") & exit 0", DETACHED_SHUTDOWN_REBOOT_COMMAND) def test_remote_request_shutdown_reboot_uses_shutdown_with_reboot_fallback(self) -> None: connection = SshConnection("root@10.0.0.2", "pw", "-o foo") @@ -319,11 +319,11 @@ def test_remote_request_shutdown_reboot_uses_shutdown_with_reboot_fallback(self) check=False, timeout=REBOOT_REQUEST_TIMEOUT_SECONDS, ) - self.assertEqual(DETACHED_SHUTDOWN_REBOOT_COMMAND, DETACHED_REBOOT_COMMAND) self.assertIn("exec /dev/null 2>&1", DETACHED_SHUTDOWN_REBOOT_COMMAND) self.assertIn("/bin/sync; /bin/sleep 1;", DETACHED_SHUTDOWN_REBOOT_COMMAND) self.assertIn("/sbin/shutdown -r now", DETACHED_SHUTDOWN_REBOOT_COMMAND) self.assertIn("|| /sbin/reboot", DETACHED_SHUTDOWN_REBOOT_COMMAND) + self.assertNotIn("[ -x /sbin/shutdown ]", DETACHED_SHUTDOWN_REBOOT_COMMAND) self.assertIn(") & exit 0", DETACHED_SHUTDOWN_REBOOT_COMMAND) def test_flush_remote_filesystem_writes_syncs_and_waits(self) -> None: From 6ce0c1f85b1093478f1a0f048eca1aff94356f93 Mon Sep 17 00:00:00 2001 From: James Chang Date: Wed, 20 May 2026 03:13:11 -0700 Subject: [PATCH 4/6] Address reboot wrapper review comments --- src/timecapsulesmb/cli/flows.py | 2 +- src/timecapsulesmb/deploy/executor.py | 2 +- tests/test_deploy_modules.py | 9 ++------- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/timecapsulesmb/cli/flows.py b/src/timecapsulesmb/cli/flows.py index 9f2fe90..7dd870e 100644 --- a/src/timecapsulesmb/cli/flows.py +++ b/src/timecapsulesmb/cli/flows.py @@ -168,7 +168,7 @@ def _request_reboot_via_ssh_shutdown( command_context, log=log, request_reboot=remote_request_shutdown_reboot, - progress_message="SSH: /sbin/shutdown -r now (fallback /sbin/reboot)", + progress_message="SSH: /bin/sync; /sbin/shutdown -r now (fallback /sbin/reboot)", ) diff --git a/src/timecapsulesmb/deploy/executor.py b/src/timecapsulesmb/deploy/executor.py index 6b6fd3b..7fa4953 100644 --- a/src/timecapsulesmb/deploy/executor.py +++ b/src/timecapsulesmb/deploy/executor.py @@ -123,7 +123,7 @@ def remote_request_reboot(connection: SshConnection) -> None: def remote_request_shutdown_reboot(connection: SshConnection) -> None: - run_ssh(connection, DETACHED_SHUTDOWN_REBOOT_COMMAND, check=False, timeout=REBOOT_REQUEST_TIMEOUT_SECONDS) + remote_request_reboot(connection) def flush_remote_filesystem_writes(connection: SshConnection) -> None: diff --git a/tests/test_deploy_modules.py b/tests/test_deploy_modules.py index 05058a3..923b513 100644 --- a/tests/test_deploy_modules.py +++ b/tests/test_deploy_modules.py @@ -311,14 +311,9 @@ def test_remote_request_reboot_uses_explicit_reboot_timeout(self) -> None: def test_remote_request_shutdown_reboot_uses_shutdown_with_reboot_fallback(self) -> None: connection = SshConnection("root@10.0.0.2", "pw", "-o foo") - with mock.patch("timecapsulesmb.deploy.executor.run_ssh") as run_ssh_mock: + with mock.patch("timecapsulesmb.deploy.executor.remote_request_reboot") as reboot_mock: remote_request_shutdown_reboot(connection) - run_ssh_mock.assert_called_once_with( - connection, - DETACHED_SHUTDOWN_REBOOT_COMMAND, - check=False, - timeout=REBOOT_REQUEST_TIMEOUT_SECONDS, - ) + reboot_mock.assert_called_once_with(connection) self.assertIn("exec /dev/null 2>&1", DETACHED_SHUTDOWN_REBOOT_COMMAND) self.assertIn("/bin/sync; /bin/sleep 1;", DETACHED_SHUTDOWN_REBOOT_COMMAND) self.assertIn("/sbin/shutdown -r now", DETACHED_SHUTDOWN_REBOOT_COMMAND) From 4bbdd8f22ec82cc55d4152981257f8ef8a967a45 Mon Sep 17 00:00:00 2001 From: James Chang Date: Wed, 20 May 2026 03:27:18 -0700 Subject: [PATCH 5/6] Share reboot progress message --- src/timecapsulesmb/cli/flows.py | 5 +++-- tests/test_cli_flows.py | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/timecapsulesmb/cli/flows.py b/src/timecapsulesmb/cli/flows.py index 7dd870e..20cf157 100644 --- a/src/timecapsulesmb/cli/flows.py +++ b/src/timecapsulesmb/cli/flows.py @@ -26,6 +26,7 @@ REBOOT_UP_TIMEOUT_MESSAGE = "Timed out waiting for SSH after reboot." ACP_REBOOT_REQUEST_TIMEOUT_SECONDS = 10 +SSH_SHUTDOWN_REBOOT_PROGRESS_MESSAGE = "SSH: /bin/sync; /sbin/shutdown -r now (fallback /sbin/reboot)" def wait_for_tcp_port_state( @@ -168,7 +169,7 @@ def _request_reboot_via_ssh_shutdown( command_context, log=log, request_reboot=remote_request_shutdown_reboot, - progress_message="SSH: /bin/sync; /sbin/shutdown -r now (fallback /sbin/reboot)", + progress_message=SSH_SHUTDOWN_REBOOT_PROGRESS_MESSAGE, ) @@ -178,7 +179,7 @@ def _request_reboot_via_ssh( *, log: LogCallback = None, request_reboot: Callable[[SshConnection], None] | None = None, - progress_message: str = "SSH: /bin/sync; /sbin/shutdown -r now (fallback /sbin/reboot)", + progress_message: str = SSH_SHUTDOWN_REBOOT_PROGRESS_MESSAGE, ) -> None: command_context.add_debug_fields(ssh_reboot_attempted=True) emit_progress(log, progress_message) diff --git a/tests/test_cli_flows.py b/tests/test_cli_flows.py index 6174208..46ab1c8 100644 --- a/tests/test_cli_flows.py +++ b/tests/test_cli_flows.py @@ -16,6 +16,7 @@ from timecapsulesmb.cli.flows import ( ACP_REBOOT_REQUEST_TIMEOUT_SECONDS, REBOOT_UP_TIMEOUT_MESSAGE, + SSH_SHUTDOWN_REBOOT_PROGRESS_MESSAGE, observe_reboot_cycle, request_deploy_reboot_and_wait, request_reboot_and_wait, @@ -268,7 +269,7 @@ def test_request_ssh_reboot_uses_ssh_only_strategy_and_progress_log(self) -> Non self.assertEqual(command_context.debug_fields["reboot_request_strategy"], "ssh") self.assertEqual(command_context.debug_fields["ssh_reboot_attempted"], True) self.assertEqual(command_context.debug_fields["ssh_reboot_succeeded"], True) - self.assertEqual(messages, ["SSH: /bin/sync; /sbin/shutdown -r now (fallback /sbin/reboot)"]) + self.assertEqual(messages, [SSH_SHUTDOWN_REBOOT_PROGRESS_MESSAGE]) self.assertIn("SSH reboot requested.", output.getvalue()) def test_request_ssh_reboot_records_timeout_without_raising(self) -> None: From dfeb231f9fbc812e88914eaa116b69a813afbb50 Mon Sep 17 00:00:00 2001 From: James Chang Date: Wed, 20 May 2026 04:00:14 -0700 Subject: [PATCH 6/6] Remove redundant shutdown reboot wrapper --- src/timecapsulesmb/cli/flows.py | 4 ++-- src/timecapsulesmb/deploy/executor.py | 4 ---- tests/test_cli.py | 16 ++-------------- tests/test_cli_flows.py | 6 +++--- tests/test_deploy_modules.py | 13 ------------- 5 files changed, 7 insertions(+), 36 deletions(-) diff --git a/src/timecapsulesmb/cli/flows.py b/src/timecapsulesmb/cli/flows.py index 20cf157..12dce9b 100644 --- a/src/timecapsulesmb/cli/flows.py +++ b/src/timecapsulesmb/cli/flows.py @@ -7,7 +7,7 @@ from timecapsulesmb.cli.runtime import LogCallback, emit_progress from timecapsulesmb.core.net import extract_host from timecapsulesmb.core.errors import system_exit_message -from timecapsulesmb.deploy.executor import remote_request_reboot, remote_request_shutdown_reboot +from timecapsulesmb.deploy.executor import remote_request_reboot from timecapsulesmb.deploy.verify import ( managed_runtime_ready, render_managed_runtime_verification, @@ -168,7 +168,7 @@ def _request_reboot_via_ssh_shutdown( connection, command_context, log=log, - request_reboot=remote_request_shutdown_reboot, + request_reboot=remote_request_reboot, progress_message=SSH_SHUTDOWN_REBOOT_PROGRESS_MESSAGE, ) diff --git a/src/timecapsulesmb/deploy/executor.py b/src/timecapsulesmb/deploy/executor.py index 7fa4953..db97d0e 100644 --- a/src/timecapsulesmb/deploy/executor.py +++ b/src/timecapsulesmb/deploy/executor.py @@ -122,10 +122,6 @@ def remote_request_reboot(connection: SshConnection) -> None: run_ssh(connection, DETACHED_SHUTDOWN_REBOOT_COMMAND, check=False, timeout=REBOOT_REQUEST_TIMEOUT_SECONDS) -def remote_request_shutdown_reboot(connection: SshConnection) -> None: - remote_request_reboot(connection) - - def flush_remote_filesystem_writes(connection: SshConnection) -> None: run_ssh(connection, FLUSH_REMOTE_FILESYSTEMS_COMMAND, timeout=FLUSH_REMOTE_FILESYSTEMS_TIMEOUT_SECONDS) diff --git a/tests/test_cli.py b/tests/test_cli.py index 5b644fe..65e3a6f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -902,14 +902,8 @@ def run_deploy_cli( mocks.verify_managed_runtime = stack.enter_context( mock.patch("timecapsulesmb.cli.flows.verify_managed_runtime", return_value=verify_runtime) ) - mocks.remote_request_shutdown_reboot = stack.enter_context( - mock.patch("timecapsulesmb.cli.flows.remote_request_shutdown_reboot", side_effect=reboot_side_effect) - ) mocks.remote_request_reboot = stack.enter_context( - mock.patch( - "timecapsulesmb.cli.flows.remote_request_reboot", - side_effect=AssertionError("deploy should not request legacy SSH reboot directly"), - ) + mock.patch("timecapsulesmb.cli.flows.remote_request_reboot", side_effect=reboot_side_effect) ) mocks.acp_reboot = stack.enter_context( mock.patch( @@ -4529,7 +4523,6 @@ def test_deploy_no_reboot_stops_after_upload_phase(self) -> None: ) self.assertEqual(result.rc, 0) - result.mocks.remote_request_shutdown_reboot.assert_not_called() result.mocks.remote_request_reboot.assert_not_called() self.assertEqual(result.mocks.verify_payload_home_conn.call_count, 2) result.mocks.flush_remote_filesystem_writes.assert_called_once() @@ -4546,7 +4539,6 @@ def test_deploy_payload_verification_failure_aborts_before_reboot(self) -> None: ) self.assertEqual(str(result.exception), "managed payload verification failed at /Volumes/dk2/.samba4: missing smbd") - result.mocks.remote_request_shutdown_reboot.assert_not_called() result.mocks.remote_request_reboot.assert_not_called() result.mocks.verify_payload_home_conn.assert_called_once() result.mocks.flush_remote_filesystem_writes.assert_not_called() @@ -4572,7 +4564,6 @@ def test_deploy_post_sync_payload_verification_failure_aborts_before_reboot(self "managed payload verification failed at /Volumes/dk2/.samba4: missing payload directory", ) result.mocks.flush_remote_filesystem_writes.assert_called_once() - result.mocks.remote_request_shutdown_reboot.assert_not_called() result.mocks.remote_request_reboot.assert_not_called() self.assertEqual(result.mocks.verify_payload_home_conn.call_count, 2) telemetry_error = self.telemetry_payload("deploy_finished")["error"] @@ -4591,7 +4582,6 @@ def test_deploy_declined_reboot_returns_without_rebooting(self) -> None: self.assertEqual(result.rc, 0) self.assertIn("Deployment complete without reboot.", result.text) - result.mocks.remote_request_shutdown_reboot.assert_not_called() result.mocks.remote_request_reboot.assert_not_called() self.assertEqual(result.mocks.verify_payload_home_conn.call_count, 2) result.mocks.flush_remote_filesystem_writes.assert_called_once() @@ -4610,8 +4600,7 @@ def test_deploy_reboot_timeout_returns_failure(self) -> None: self.assertEqual(result.rc, 1) self.assertIn("SSH reboot request timed out; checking whether the device is rebooting...", result.text) self.assertIn(deploy.REBOOT_NO_DOWN_MESSAGE, result.text) - result.mocks.remote_request_shutdown_reboot.assert_called_once() - result.mocks.remote_request_reboot.assert_not_called() + result.mocks.remote_request_reboot.assert_called_once() result.mocks.acp_reboot.assert_not_called() result.mocks.verify_managed_runtime.assert_not_called() @@ -4673,7 +4662,6 @@ def test_deploy_netbsd4_yes_runs_activation_and_skips_reboot(self) -> None: self.assertEqual(result.mocks.run_remote_actions.call_count, 3) self.assertEqual(result.mocks.verify_payload_home_conn.call_count, 2) result.mocks.flush_remote_filesystem_writes.assert_called_once() - result.mocks.remote_request_shutdown_reboot.assert_not_called() result.mocks.remote_request_reboot.assert_not_called() self.assertIn("Activating NetBSD4 payload without reboot.", result.text) self.assertIn("NetBSD4 activation complete.", result.text) diff --git a/tests/test_cli_flows.py b/tests/test_cli_flows.py index 46ab1c8..f86eb63 100644 --- a/tests/test_cli_flows.py +++ b/tests/test_cli_flows.py @@ -153,10 +153,10 @@ def test_request_reboot_and_wait_succeeds_after_acp_reboot_request(self) -> None self.assertIn("ACP reboot requested.", output.getvalue()) self.assertIn("Waiting for the device to go down...", output.getvalue()) - def test_request_deploy_reboot_and_wait_uses_ssh_shutdown_request(self) -> None: + def test_request_deploy_reboot_and_wait_uses_ssh_reboot_request(self) -> None: command_context = FakeCommandContext() output = io.StringIO() - with mock.patch("timecapsulesmb.cli.flows.remote_request_shutdown_reboot") as shutdown_reboot_mock: + with mock.patch("timecapsulesmb.cli.flows.remote_request_reboot") as reboot_mock: with mock.patch("timecapsulesmb.cli.flows.acp_reboot", side_effect=AssertionError("deploy should not use ACP reboot")): with mock.patch("timecapsulesmb.cli.flows.wait_for_ssh_state_conn", side_effect=[True, True]) as wait_mock: with redirect_stdout(output): @@ -167,7 +167,7 @@ def test_request_deploy_reboot_and_wait_uses_ssh_shutdown_request(self) -> None: ) self.assertTrue(ok) - shutdown_reboot_mock.assert_called_once() + reboot_mock.assert_called_once() self.assertEqual(wait_mock.call_args_list[0].kwargs, {"expected_up": False, "timeout_seconds": 60}) self.assertEqual(wait_mock.call_args_list[1].kwargs, {"expected_up": True, "timeout_seconds": 240}) self.assertEqual(command_context.finish_fields["reboot_was_attempted"], True) diff --git a/tests/test_deploy_modules.py b/tests/test_deploy_modules.py index 923b513..d1a716d 100644 --- a/tests/test_deploy_modules.py +++ b/tests/test_deploy_modules.py @@ -46,7 +46,6 @@ REBOOT_REQUEST_TIMEOUT_SECONDS, flush_remote_filesystem_writes, remote_request_reboot, - remote_request_shutdown_reboot, remote_uninstall_payload, upload_deployment_payload, upload_flash_file, @@ -309,18 +308,6 @@ def test_remote_request_reboot_uses_explicit_reboot_timeout(self) -> None: self.assertNotIn("[ -x /sbin/shutdown ]", DETACHED_SHUTDOWN_REBOOT_COMMAND) self.assertIn(") & exit 0", DETACHED_SHUTDOWN_REBOOT_COMMAND) - def test_remote_request_shutdown_reboot_uses_shutdown_with_reboot_fallback(self) -> None: - connection = SshConnection("root@10.0.0.2", "pw", "-o foo") - with mock.patch("timecapsulesmb.deploy.executor.remote_request_reboot") as reboot_mock: - remote_request_shutdown_reboot(connection) - reboot_mock.assert_called_once_with(connection) - self.assertIn("exec /dev/null 2>&1", DETACHED_SHUTDOWN_REBOOT_COMMAND) - self.assertIn("/bin/sync; /bin/sleep 1;", DETACHED_SHUTDOWN_REBOOT_COMMAND) - self.assertIn("/sbin/shutdown -r now", DETACHED_SHUTDOWN_REBOOT_COMMAND) - self.assertIn("|| /sbin/reboot", DETACHED_SHUTDOWN_REBOOT_COMMAND) - self.assertNotIn("[ -x /sbin/shutdown ]", DETACHED_SHUTDOWN_REBOOT_COMMAND) - self.assertIn(") & exit 0", DETACHED_SHUTDOWN_REBOOT_COMMAND) - def test_flush_remote_filesystem_writes_syncs_and_waits(self) -> None: connection = SshConnection("root@10.0.0.2", "pw", "-o foo") with mock.patch("timecapsulesmb.deploy.executor.run_ssh") as run_ssh_mock: