diff --git a/.github/scripts/run_offline_qualified.py b/.github/scripts/run_offline_qualified.py index 8a9548cd..796cd962 100644 --- a/.github/scripts/run_offline_qualified.py +++ b/.github/scripts/run_offline_qualified.py @@ -28,6 +28,7 @@ PYTHON_FILES = ( "AppImage/scripts/tests/test_health_persistence_recurrence.py", ) NODE_FILES = ( + "tests/test_oci_update_cancellation.cjs", "tests/lxc_updates/test_local_app_icons_ui.cjs", "tests/lxc_updates/test_docker_app_discovery_ui.cjs", "tests/lxc_updates/test_native_app_discovery_ui.cjs", diff --git a/.github/scripts/tests/test_oci_recovery_inventory_wording.py b/.github/scripts/tests/test_oci_recovery_inventory_wording.py index 338698ae..097fc5c6 100644 --- a/.github/scripts/tests/test_oci_recovery_inventory_wording.py +++ b/.github/scripts/tests/test_oci_recovery_inventory_wording.py @@ -116,7 +116,7 @@ class RecoveryMessages(unittest.TestCase): instances.ROOT = Path('/inert') instances.read = lambda root, vmid: self.primary self.modules = {'oci_instances': instances, 'oci_stack_replay': ModuleType('oci_stack_replay')} - self.scope = {'sys': SimpleNamespace(path=[], executable='python3'), 'Path': Path, + self.scope = {'sys': SimpleNamespace(path=[], executable='python3'), 'Path': Path, 'CANCELLED': None, 'translate': lambda text: text, 'images': SimpleNamespace(offer_removal=lambda *args: None), '_remove': lambda *args: self.fail('remove must not run'), '_run_lifecycle': lambda command, title: self.calls.append((command, title)) or False} diff --git a/.github/scripts/tests/test_oci_update_cancellation.py b/.github/scripts/tests/test_oci_update_cancellation.py new file mode 100644 index 00000000..d06f5255 --- /dev/null +++ b/.github/scripts/tests/test_oci_update_cancellation.py @@ -0,0 +1,166 @@ +"""Real native OCI menu/CLI paths with all external effects replaced.""" +from contextlib import redirect_stdout +import copy +import io +import json +from pathlib import Path +import sys +import unittest +from unittest.mock import Mock, patch + +ROOT = Path(__file__).resolve().parents[3] +sys.path[:0] = [str(ROOT / 'oci/src'), str(ROOT / 'oci/remote')] +from proxmenux_oci import cli, i18n, management as menu, ui as native_ui, updates +import oci_instances as instances +import oci_instance_reconcile as reconcile + + +class UpdateCancellationTests(unittest.TestCase): + def setUp(self): + self.record = {'status': 'installed', 'vmid': 110, 'deployment': {}, 'template': {}} + self.original = copy.deepcopy(self.record) + self.row = dict(vmid=110, reason='matched', stack=False, pending=False, status='installed', + title='Fixture application', hostname='fixture', image='fixture:latest') + self.ui = Mock() + self.ui.unattended = False + self.ui.review.return_value = False + for module, name, options in [ + (i18n, '_language', dict(new='en')), (i18n, '_cache', dict(new={})), + (menu.os, 'geteuid', dict(return_value=0)), (menu.shutil, 'which', dict(return_value='/fixture/pct')), + (native_ui, 'interactive_ui', dict(return_value=self.ui)), + (menu, 'recover_automatically', dict(return_value=None)), + (menu, 'carry_records', dict(return_value={})), + (menu, 'check_selected', dict(side_effect=lambda project, row: row)), + (menu, 'saved_inventory', dict(return_value=[self.row])), + (menu, '_selected_configuration', dict(return_value=b'fixture configuration')), + (instances, 'read', dict(return_value=self.record)), + (reconcile, 'propose', dict(return_value=None)), + ]: + item = patch.object(module, name, **options) + item.start() + self.addCleanup(item.stop) + self.lifecycle = self.guard(menu, '_run_lifecycle') + self.backup = self.guard(menu, '_work_backup_ready') + self.write = self.guard(instances, 'write') + self.command = self.guard(instances, 'command') + self.commit = self.guard(reconcile, 'commit') + + def guard(self, module, name): + item = patch.object(module, name, side_effect=AssertionError('Forbidden effect: ' + name)) + mock = item.start() + self.addCleanup(item.stop) + return mock + + def no_effects(self): + for mock in [self.lifecycle, self.backup, self.write, self.command, self.commit]: + mock.assert_not_called() + self.assertEqual(self.record, self.original) + + def test_no_is_a_distinct_falsy_result_before_backup_or_lifecycle(self): + result = menu.manage_instance(ROOT / 'oci', self.ui, self.row, action='update') + self.assertIs(result, menu.CANCELLED) + self.assertFalse(result) + self.no_effects() + + def test_real_cli_no_returns_cancel_code_instead_of_failure_or_success(self): + with patch.object(cli, 'Catalog'), redirect_stdout(io.StringIO()) as output: + result = cli.main(['manage', '110', '--action', 'update']) + self.assertEqual(result, 6) + self.assertEqual(output.getvalue(), '') # The terminal renders the localized outcome once. + self.no_effects() + + def test_no_in_stack_confirmation_keeps_every_member_unchanged(self): + self.row['stack'] = True + self.record['stack'] = {'members': [{'vmid': 110}, {'vmid': 111}]} + self.original = copy.deepcopy(self.record) + with redirect_stdout(io.StringIO()): + result = menu.direct_management(ROOT / 'oci', 110, 'update') + self.assertEqual(result, 6) + self.no_effects() + + def test_native_menu_returns_to_its_list_after_no(self): + self.ui.choose.side_effect = ['110', 'update', None] + with patch.object(menu, '_clean_orphans'), patch.object(menu, 'offer_recovery'), \ + patch.object(menu, '_records_unread', return_value=[]): + menu.interactive_management(ROOT / 'oci', self.ui) + self.assertEqual(self.ui.choose.call_count, 3) + self.no_effects() + + def test_batch_no_does_not_start_the_next_application(self): + available = [{'vmid': 110, 'title': 'First'}, {'vmid': 111, 'title': 'Second'}] + rows = [self.row, {**self.row, 'vmid': 111}] + self.ui.checklist.return_value = ['all'] + self.ui.review.side_effect = [True, False] # Batch review, first application's confirmation. + with patch.object(updates, 'UpdateCheck') as checker, patch.object(menu, 'saved_inventory', return_value=rows), \ + patch.object(menu, 'check_selected', wraps=lambda project, row: row) as selected: + checker.return_value.status.return_value = {'available': available} + self.assertFalse(updates.select_updates(ROOT / 'oci', self.ui, available)) + self.assertEqual([call.args[1]['vmid'] for call in selected.call_args_list], [110]) + self.no_effects() + + def test_accepted_update_failure_keeps_failure_code_one(self): + self.ui.review.return_value = True + self.backup.side_effect = None + self.backup.return_value = False + self.assertEqual(menu.direct_management(ROOT / 'oci', 110, 'update'), 1) + self.lifecycle.assert_not_called() + self.write.assert_not_called() + + def test_accepted_update_preserves_success_and_existing_lifecycle_command(self): + self.ui.review.return_value = True + self.backup.side_effect = None + self.backup.return_value = True + self.lifecycle.side_effect = None + self.lifecycle.return_value = True + self.assertEqual(menu.direct_management(ROOT / 'oci', 110, 'update'), 0) + self.lifecycle.assert_called_once() + command = self.lifecycle.call_args.args[0] + self.assertEqual(command[:3], [sys.executable, str(ROOT / 'oci/remote/oci_update_current.py'), '110']) + self.assertIn('--acknowledge-external-data', command) + self.write.assert_not_called() + + def test_actual_lifecycle_failure_remains_failure_after_positive_confirmation(self): + self.ui.review.return_value = True + self.backup.side_effect = None + self.backup.return_value = True + self.lifecycle.side_effect = None + self.lifecycle.return_value = False + self.assertEqual(menu.direct_management(ROOT / 'oci', 110, 'update'), 1) + self.lifecycle.assert_called_once() + self.write.assert_not_called() + + def test_unattended_positive_default_still_reaches_existing_update(self): + self.backup.side_effect = None + self.backup.return_value = True + self.lifecycle.side_effect = None + self.lifecycle.return_value = True + self.assertEqual(menu.direct_management(ROOT / 'oci', 110, 'update', unattended=True), 0) + self.lifecycle.assert_called_once() + self.write.assert_not_called() + + def test_existing_unattended_review_age_busy_and_missing_codes_are_preserved(self): + with patch.object(reconcile, 'propose', return_value={'details': ['external change']}), redirect_stdout(io.StringIO()): + self.assertEqual(menu.direct_management(ROOT / 'oci', 110, 'update', unattended=True), 4) + with patch.object(menu, '_image_too_recent', return_value=True), redirect_stdout(io.StringIO()): + self.assertEqual(menu.direct_management(ROOT / 'oci', 110, 'update', unattended=True, min_image_age_days=3), 5) + with patch.object(menu, 'saved_inventory', return_value=[]): + self.assertEqual(menu.direct_management(ROOT / 'oci', 110, 'update'), 2) + with patch.object(menu, 'recover_automatically', side_effect=BlockingIOError()): + self.assertEqual(menu.direct_management(ROOT / 'oci', 110, 'update'), 3) + self.no_effects() + + def test_slovak_dialog_and_recovery_choice_have_the_approved_meaning(self): + cache = json.loads((ROOT / 'lang/sk.json').read_text()) + with patch.object(i18n, '_language', 'sk'), patch.object(i18n, '_cache', cache): + menu.manage_instance(ROOT / 'oci', self.ui, self.row, action='update') + self.assertEqual(i18n.translate('Recover the previous installation'), 'Obnoviť predchádzajúcu inštaláciu') + text = self.ui.review.call_args.args[0] + for part in ['novší obraz aplikácie', 'overí sa jej integrita', 'pokúsi obnoviť', 'Pripojené adresáre hostiteľa']: + self.assertIn(part, text) + self.assertNotIn('kanál obrazov', text) + self.assertEqual(self.ui.review.call_args.kwargs['question'], 'Spustiť aktualizáciu?') + self.no_effects() + + +if __name__ == '__main__': + unittest.main() diff --git a/.github/scripts/tests/test_oci_update_preservation_wording.py b/.github/scripts/tests/test_oci_update_preservation_wording.py index e3fdae92..448b4832 100644 --- a/.github/scripts/tests/test_oci_update_preservation_wording.py +++ b/.github/scripts/tests/test_oci_update_preservation_wording.py @@ -24,9 +24,12 @@ OLD_GUARD = 'The current template changes the image or identity; an explicit mig GUARD = 'The current template changes the template identity or image repository; an explicit migration is required' OLD_PREVIEW = ('The saved image channel is checked for a newer image. If replacement is needed, the CT is stopped and ' 'a native backup is verified before its root is replaced. Host directories are outside that backup.') -PREVIEW = ('If the image channel has a new version, the CT is stopped, backed up and verified, and replaced with ' - 'the new image to update the container. If the update fails after the container was changed, the backup ' - 'is restored. An update that stops halfway is recovered from the OCI management menu.') +PREVIEW = ('If a newer application image is available, the container is stopped and backed up, and the backup ' + 'integrity is verified. The container is then updated using the new image.\n\n' + 'If the update fails after the container was changed, ProxMenux attempts to restore the previous ' + 'installation from the backup. If the operation remains incomplete, select the application in OCI ' + 'management and choose "Recover the previous installation".\n\n' + 'Host directories mounted in the container are not restored by the container backup.') def extracted(path, name, scope): @@ -130,7 +133,7 @@ class UpdateWording(unittest.TestCase): reconcile = ModuleType('oci_instance_reconcile') reconcile.propose = lambda *args: None scope = {'sys': SimpleNamespace(path=[], executable='python3'), - 'os': os, 'tempfile': tempfile, 'Path': Path, + 'os': os, 'tempfile': tempfile, 'Path': Path, 'CANCELLED': None, 'translate': lambda text: text, 'check_selected': lambda project, row: row, '_selected_configuration': lambda vmid: instances.command('pct', 'config', str(vmid)), 'images': SimpleNamespace(offer_removal=lambda *args: None), diff --git a/.github/scripts/tests/test_outcome_promises_wording.py b/.github/scripts/tests/test_outcome_promises_wording.py index 2b793640..29e70f9e 100644 --- a/.github/scripts/tests/test_outcome_promises_wording.py +++ b/.github/scripts/tests/test_outcome_promises_wording.py @@ -20,7 +20,11 @@ class OutcomePromises(TestCase): texts = oci_menu_texts() self.assertEqual([text for text in texts if 'If anything fails' in text], []) previews = [text for text in texts if 'stops halfway is recovered from the OCI management menu' in text] - self.assertEqual(len(previews), 3) + self.assertEqual(len(previews), 2) # Coordinated stack update/recreation previews. + update = next(text for text in texts if text.startswith('If a newer application image is available')) + self.assertIn('attempts to restore the previous installation', update) + self.assertIn('If the operation remains incomplete', update) + self.assertIn('choose "Recover the previous installation"', update) def test_the_recovery_preview_says_when_each_step_happens(self): preview = next(text for text in oci_menu_texts() if 'previous native backup' in text) diff --git a/AppImage/components/script-terminal-modal.tsx b/AppImage/components/script-terminal-modal.tsx index c993281d..76f90669 100644 --- a/AppImage/components/script-terminal-modal.tsx +++ b/AppImage/components/script-terminal-modal.tsx @@ -31,6 +31,7 @@ import { import "xterm/css/xterm.css" import { API_PORT } from "@/lib/api-config" import { getTicketedWsUrl } from "@/lib/terminal-ws" +import { scriptCompletion } from "@/lib/script-completion" import { useT } from "../lib/i18n/provider" interface WebInteraction { @@ -52,6 +53,7 @@ interface ScriptTerminalModalProps { params?: Record completedSuccessfullyMessage?: string completedWithErrorMessage?: (exitCode: number) => string + cancelledExitCode?: number // Optional callback fired when the script's WebSocket closes // (script_runner sends an exit code and then closes). Lets the // parent auto-dismiss the modal — used by host-backup's Restore @@ -69,6 +71,7 @@ export function ScriptTerminalModal({ params = { EXECUTION_MODE: "web" }, completedSuccessfullyMessage, completedWithErrorMessage, + cancelledExitCode, onComplete, }: ScriptTerminalModalProps) { const t = useT() @@ -103,10 +106,12 @@ export function ScriptTerminalModal({ const getCompletionMessage = useCallback( (exitCode: number) => - exitCode === 0 + scriptCompletion(exitCode, cancelledExitCode).kind === "cancelled" + ? t("scriptTerminal.cancelled") + : exitCode === 0 ? (completedSuccessfullyMessage ?? t("scriptTerminal.completedSuccessfully")) : (completedWithErrorMessage?.(exitCode) ?? t("scriptTerminal.completedWithError", { code: exitCode })), - [completedSuccessfullyMessage, completedWithErrorMessage, t], + [completedSuccessfullyMessage, completedWithErrorMessage, cancelledExitCode, t], ) const terminalContainerRef = useRef(null) @@ -139,6 +144,11 @@ export function ScriptTerminalModal({ reconnectTimeoutRef.current = setTimeout(async () => { if (wsRef.current?.readyState !== WebSocket.OPEN && termRef.current) { if (wsRef.current) { + // Replacing a stale socket is not a script result. Its close event + // must not complete the operation before the new socket reports it. + wsRef.current.onclose = null + wsRef.current.onerror = null + wsRef.current.onmessage = null wsRef.current.close() } @@ -176,6 +186,7 @@ const initMessage = { } ws.onmessage = (event) => { + if (completionReceivedRef.current) return // Filter out pong responses from heartbeat if (event.data === '{"type": "pong"}' || event.data === '{"type":"pong"}') { return @@ -189,10 +200,10 @@ const initMessage = { : null if (exitMatch) { const exitCode = Number(exitMatch[1]) - termRef.current?.write(event.data) + if (scriptCompletion(exitCode, cancelledExitCode).kind !== "cancelled") termRef.current?.write(event.data) completionReceivedRef.current = true setIsComplete(true) - termRef.current?.writeln(`\x1b[${exitCode === 0 ? "32" : "31"}m${getCompletionMessage(exitCode)}\x1b[0m`) + termRef.current?.writeln(`\x1b[${scriptCompletion(exitCode, cancelledExitCode).color}m${getCompletionMessage(exitCode)}\x1b[0m`) onCompleteRef.current?.(exitCode) if (ws.readyState === WebSocket.OPEN) ws.close(1000, "script complete") return @@ -223,7 +234,7 @@ const initMessage = { const exitCode = Number(msg.exit_code ?? 1) completionReceivedRef.current = true setIsComplete(true) - termRef.current?.writeln(`\x1b[${exitCode === 0 ? "32" : "31"}m${getCompletionMessage(exitCode)}\x1b[0m`) + termRef.current?.writeln(`\x1b[${scriptCompletion(exitCode, cancelledExitCode).color}m${getCompletionMessage(exitCode)}\x1b[0m`) onCompleteRef.current?.(exitCode) if (ws.readyState === WebSocket.OPEN) ws.close(1000, "script complete") return @@ -261,7 +272,7 @@ const initMessage = { } } }, 1000) - }, [isOpen, isComplete, scriptPath, getCompletionMessage, t]) + }, [isOpen, isComplete, scriptPath, getCompletionMessage, cancelledExitCode, t]) const sendKey = useCallback((key: string) => { if (!termRef.current) return @@ -392,6 +403,7 @@ const initMessage = { } ws.onmessage = (event) => { + if (completionReceivedRef.current) return // Filter out pong responses from heartbeat - don't display in terminal if (event.data === '{"type": "pong"}' || event.data === '{"type":"pong"}') { return @@ -405,10 +417,10 @@ const initMessage = { : null if (exitMatch) { const exitCode = Number(exitMatch[1]) - term.write(event.data) + if (scriptCompletion(exitCode, cancelledExitCode).kind !== "cancelled") term.write(event.data) completionReceivedRef.current = true setIsComplete(true) - term.writeln(`\x1b[${exitCode === 0 ? "32" : "31"}m${getCompletionMessage(exitCode)}\x1b[0m`) + term.writeln(`\x1b[${scriptCompletion(exitCode, cancelledExitCode).color}m${getCompletionMessage(exitCode)}\x1b[0m`) onCompleteRef.current?.(exitCode) if (ws.readyState === WebSocket.OPEN) ws.close(1000, "script complete") return @@ -441,7 +453,7 @@ const initMessage = { const exitCode = Number(msg.exit_code ?? 1) completionReceivedRef.current = true setIsComplete(true) - term.writeln(`\x1b[${exitCode === 0 ? "32" : "31"}m${getCompletionMessage(exitCode)}\x1b[0m`) + term.writeln(`\x1b[${scriptCompletion(exitCode, cancelledExitCode).color}m${getCompletionMessage(exitCode)}\x1b[0m`) onCompleteRef.current?.(exitCode) if (ws.readyState === WebSocket.OPEN) ws.close(1000, "script complete") return @@ -814,7 +826,7 @@ const initMessage = { )} {(isMobile || isTablet) && ( -
+