From e24308c0172304a5f56ecf8e35282c76009d2f8d Mon Sep 17 00:00:00 2001 From: martino <32328813+f3rs3n@users.noreply.github.com> Date: Tue, 29 Sep 2026 23:52:07 +0200 Subject: [PATCH] Correct backup outcome diagnostics and email presentation chains --- .github/scripts/tests/notification_fixture.py | 51 ++++ .../tests/test_notification_corrections.py | 251 ++++++++++++++++++ .../test_notification_outcome_wording.py | 8 +- AppImage/scripts/notification_channels.py | 25 +- AppImage/scripts/notification_events.py | 21 +- AppImage/scripts/notification_manager.py | 5 + AppImage/scripts/notification_templates.py | 80 +++++- .../tests/test_notification_runtime_i18n.py | 13 +- .../scripts/tests/test_vzdump_ai_integrity.py | 8 +- 9 files changed, 431 insertions(+), 31 deletions(-) create mode 100644 .github/scripts/tests/notification_fixture.py create mode 100644 .github/scripts/tests/test_notification_corrections.py diff --git a/.github/scripts/tests/notification_fixture.py b/.github/scripts/tests/notification_fixture.py new file mode 100644 index 00000000..90ba64e4 --- /dev/null +++ b/.github/scripts/tests/notification_fixture.py @@ -0,0 +1,51 @@ +"""Assertion-free inert actual consumers; no operational host imports.""" +import ast +import re +import sys +from pathlib import Path +ROOT = Path(__file__).resolve().parents[3] +SCRIPTS = ROOT / 'AppImage/scripts' +if str(SCRIPTS) not in sys.path: + sys.path.insert(0, str(SCRIPTS)) +import notification_templates as templates +# Display-name resolution is an infrastructure boundary, never load manager. +templates._get_hostname = lambda: 'node-a' +from notification_channels import EmailChannel +LANGUAGES = ('en', 'de', 'es', 'fr', 'it', 'pt', 'sk', 'sv') + +def extract(path, name, owner, ns): + tree = ast.parse(path.read_text()) + nodes = tree.body if owner is None else next(n.body for n in tree.body if isinstance(n, ast.ClassDef) and n.name == owner) + node = next(n for n in nodes if isinstance(n, ast.FunctionDef) and n.name == name) + node.decorator_list = [] + exec(compile(ast.Module(body=[node], type_ignores=[]), str(path), 'exec'), ns) + return ns[name] + +def receive(message, severity='info', title='Backup', kind='vzdump'): + ns = {'re': re, 'capture_journal_context': lambda **kw: ''} + class Event: + def __init__(self, **kw): self.__dict__.update(kw); self.event_id = 'inert' + ns['NotificationEvent'] = Event + methods = {name: extract(SCRIPTS / 'notification_events.py', name, 'ProxmoxHookWatcher', ns) + for name in ('_classify_pve', '_map_severity', '_backup_outcome', 'process_webhook')} + class Queue: + def __init__(self): self.items = [] + def put(self, event): self.items.append(event) + class Receiver: + _hostname = 'node-a' + _classify_pve = methods['_classify_pve'] + _map_severity = staticmethod(methods['_map_severity']) + _backup_outcome = staticmethod(methods['_backup_outcome']) + process_webhook = methods['process_webhook'] + def __init__(self): self._queue = Queue() + target = Receiver() + result = target.process_webhook({'fields': {'type': kind}, 'severity': severity, 'title': title, 'message': message}) + assert result['accepted'] and len(target._queue.items) == 1 + return target._queue.items[0] + +def email(event_type, data, severity='INFO', language='en'): + result = templates.render_template(event_type, data, language) + channel = object.__new__(EmailChannel) + channel.subject_prefix = '[ProxMenux]' + context = {**data, 'severity': severity, '_event_type': event_type, '_group': result['group'], '_notification_language': language} + return result, channel._format_html(result['title'], result['body'], severity, context) diff --git a/.github/scripts/tests/test_notification_corrections.py b/.github/scripts/tests/test_notification_corrections.py new file mode 100644 index 00000000..94ae1469 --- /dev/null +++ b/.github/scripts/tests/test_notification_corrections.py @@ -0,0 +1,251 @@ +"""Whole-PR outcome corrections, inert producer/actual email consumers.""" +import html +import unittest +from notification_fixture import templates, receive, email, LANGUAGES + +REPORT = """Details +======= +VMID Name Status Time Size Filename +100 web ok 1m 1s 1 GiB vm/100/2026-09-29T17:00:00Z + +Total running time: 1m 1s +Total size: 1 GiB +""" + +class CorrectionTests(unittest.TestCase): + def test_reversed_finish_is_not_completion_evidence(self): + event = receive('INFO: Finished Backup of VM 100 (00:01:01)\nINFO: Starting Backup of VM 100 (qemu)') + self.assertEqual(event.data['backup_outcome'], 'unconfirmed') + + + def test_interleaved_complete_logs_keep_both_finished_guests(self): + message = ('INFO: Starting Backup of VM 100 (qemu)\nINFO: VM Name: web\n' + 'INFO: Starting Backup of VM 101 (lxc)\nINFO: CT Name: db\n' + 'INFO: Finished Backup of VM 100 (00:01:01)\nINFO: Finished Backup of VM 101 (00:01:02)') + event = receive(message) + self.assertEqual(event.data['backup_outcome'], 'confirmed') + result, markup = email(event.event_type, event.data, event.severity) + self.assertIn('✅ VM web (100)', result['body']) + self.assertIn('✅ CT db (101)', result['body']) + self.assertNotIn('❔', result['body']) + self.assertIn('00:01:01', result['body']) + + + def test_backup_identity_has_event_scoped_mail_compatible_wrapping(self): + result, markup = email('backup_complete', {'hostname': 'n' * 64, + 'backup_outcome': 'confirmed', 'pve_message': 'INFO: Starting Backup of VM 100 (qemu)\nINFO: VM Name: customerproductionpostgresqlreplicaeuropewestdatacenter01\nINFO: Finished Backup of VM 100 (00:01:01)'}) + self.assertIn('table-layout:fixed;', markup) + title_tag = markup.split('

RESOLVED', markup) + self.assertNotIn('color:#16a34a', markup) + self.assertEqual(data['_event_type'], 'node_reconnect') # caller not mutated + + + def test_disappearance_body_keeps_observation_age_not_green_severity(self): + data = {'hostname': 'node-a', 'category': 'temperature', 'reason': 'old observation', + 'duration': '3d 2h', 'original_severity': 'WARNING', 'severity': 'OK'} + for language in LANGUAGES: + result, markup = email('error_resolved', data, 'OK', language) + for line in result['body'].splitlines(): + if line.strip(): self.assertIn(line.strip(), html.unescape(markup)) + self.assertNotIn('>OK', markup) + self.assertNotIn('color:#16a34a', markup) + self.assertNotIn('>RESOLVED', markup) + + + def test_actual_restore_endpoint_warnings_and_counts_reach_email(self): + from notification_fixture import extract, SCRIPTS + from types import SimpleNamespace + for warnings in ('', 'missing module zfs'): + events = [] + ns = {'request': SimpleNamespace(remote_addr='127.0.0.1', get_json=lambda **kw: { + 'hostname': 'node-a', 'guests': '3', 'stubs': '1', 'stale_nodes': '0', + 'components': '2', 'duration': '2m', 'warnings': warnings}), + 'notification_manager': SimpleNamespace(emit_event=lambda **kw: events.append(kw)), + 'jsonify': lambda value: value} + handler = extract(SCRIPTS / 'flask_notification_routes.py', 'internal_restore_event', None, ns) + response, status = handler() + self.assertEqual(status, 200) + event = events[0] + for language in LANGUAGES: + result, markup = email(event['event_type'], event['data'], event['severity'], language) + self.assertIn('2m', markup) + for line in result['body'].splitlines(): + if line.strip(): self.assertIn(line.strip(), html.unescape(markup)) + if warnings: self.assertIn(warnings, markup) + + + def test_raw_display_hostname_is_substituted_exactly_once(self): + for event_type, outcome in (('backup_complete', 'confirmed'), ('backup_complete', 'failed'), ('backup_fail', 'failed')): + for hostname in ('Sala {rack} – Zürich', 'node-{vmid}', 'Sala {rack.location}'): + for language in LANGUAGES: + result, markup = email(event_type, {'hostname': hostname, + 'backup_outcome': outcome, 'pve_message': REPORT}, language=language) + self.assertIn(hostname, result['title']) + self.assertIn(hostname, html.unescape(markup)) + + + def test_malformed_numeric_report_is_queued_uncertain_and_renderable(self): + for size in ('1..5 GiB', '..5 GiB'): + message = REPORT.replace('1 GiB ', size.ljust(9)).split('Total size:')[0] + event = receive(message) + self.assertEqual(event.data['backup_outcome'], 'unconfirmed') + result, markup = email(event.event_type, event.data, event.severity) + self.assertIn(size, result['body']) + + + def test_unknown_backup_type_keeps_explicit_err_but_cannot_certify_ok(self): + event = receive(REPORT.replace('ok ', 'err '), kind='') + self.assertEqual(event.event_type, 'backup_complete') + self.assertEqual(event.severity, 'INFO') + self.assertEqual(event.data['backup_outcome'], 'failed') + result, markup = email(event.event_type, event.data, event.severity) + self.assertIn('FAILED', markup) + self.assertEqual(receive(REPORT, kind='').data['backup_outcome'], 'unconfirmed') + + + def test_confirmed_metadata_only_context_is_retained(self): + data = {'hostname': 'node-a', 'backup_outcome': 'confirmed', + 'vmid': '100', 'vmname': 'web {literal}', 'storage': 'PBS', 'size': '1 GiB'} + for language in LANGUAGES: + result, markup = email('backup_complete', data, language=language) + self.assertIn('web {literal} (100)', result['title']) + self.assertIn('1 GiB', result['body']) + self.assertIn('1 GiB', markup) + + + def test_explicit_guest_failure_overrides_only_the_linked_ok_row(self): + other = '101 db ok 1m 1s 1 GiB ct/101/2026-09-29T17:00:00Z' + message = REPORT.replace('\n\nTotal', '\n' + other + '\n\nTotal') + diagnostic = '100: 2026-09-29 17:00:00 ERROR: Backup of VM 100 failed - archive write failed' + for severity in ('info', 'error'): + event = receive(message + '\n' + diagnostic, severity) + self.assertEqual(event.data.get('vmid'), '100') + for language in LANGUAGES: + result, markup = email(event.event_type, event.data, event.severity, language) + self.assertIn('❌ VM web (100)', result['body']) + self.assertNotIn('✅ VM web (100)', markup) + self.assertIn('✅ CT db (101)', result['body']) + self.assertIn('web (100)', result['title']) + + + def test_official_week_month_year_durations_stay_confirmed(self): + for duration in ('1w', '1w 1m 1s', '1M', '1y'): + # Fixed-column widths are unchanged for these bounded values. + message = REPORT.replace('1m 1s ', duration.ljust(9)) + event = receive(message) + self.assertEqual(event.data['backup_outcome'], 'confirmed', duration) + result, markup = email(event.event_type, event.data, event.severity) + self.assertIn(duration, result['body']) + + + def test_prefixed_error_diagnostics_survive_both_source_severities(self): + diagnostic = '100: 2026-09-29 17:00:00 ERROR: archive write failed: permission denied' + for severity in ('info', 'error'): + event = receive(REPORT + '\n' + diagnostic, severity) + self.assertEqual(event.data['backup_outcome'], 'failed') + for language in LANGUAGES: + result, markup = email(event.event_type, event.data, event.severity, language) + self.assertIn(diagnostic, result['body']) + self.assertIn(diagnostic, html.unescape(markup)) + # A job-level error must not invent a failed guest. + self.assertIn('✅ VM web (100)', result['body']) + + + def test_official_prefixed_warning_is_uncertain_and_retained(self): + warning = '100: 2026-09-29 17:00:00 WARN: unable to add notes - permission denied' + event = receive(REPORT + '\n' + warning) + self.assertEqual(event.data['backup_outcome'], 'unconfirmed') + for language in LANGUAGES: + result, markup = email(event.event_type, event.data, event.severity, language) + self.assertIn(warning, result['body']) + self.assertIn(warning, html.unescape(markup)) + + def test_batch_and_abort_failure_titles_have_no_empty_guest_slot(self): + failed = REPORT.replace('ok ', 'err ') + failed = failed.replace('\n\nTotal', '\n101 db err 1m 1s 0 B null\n\nTotal') + for message in (failed, REPORT.replace('ok ', 'todo '), + REPORT.split('100 web')[0] + '\nTotal running time: 0s'): + event = receive(message + '\nINFO: vzdump --storage PBS', 'error') + for language in LANGUAGES: + result, markup = email(event.event_type, event.data, event.severity, language) + self.assertNotIn('()', result['title']) + self.assertIn('PBS', result['title']) + self.assertNotIn('web (100)', result['title']) + + def test_subject_only_setup_failure_survives_actual_email(self): + reason = 'unable to activate storage PBS' + title = 'vzdump backup status (node-a): backup failed: ' + reason + message = REPORT.split('100 web')[0] + '\nTotal running time: 0s\nTotal size: 0 B' + event = receive(message, 'error', title) + for language in LANGUAGES: + result, markup = email(event.event_type, event.data, event.severity, language) + self.assertIn(reason, result['body']) + self.assertIn(reason, html.unescape(markup)) + +if __name__ == '__main__': unittest.main() diff --git a/.github/scripts/tests/test_notification_outcome_wording.py b/.github/scripts/tests/test_notification_outcome_wording.py index 5b2f8820..5be667c3 100644 --- a/.github/scripts/tests/test_notification_outcome_wording.py +++ b/.github/scripts/tests/test_notification_outcome_wording.py @@ -10,6 +10,7 @@ import unittest from pathlib import Path from typing import Any from unittest.mock import patch +from notification_fixture import templates as actual_templates ROOT = Path(__file__).resolve().parents[3] SCRIPTS = ROOT / 'AppImage/scripts' @@ -292,10 +293,9 @@ class OutcomeWording(unittest.TestCase): build = extract(path, '_build_detail_rows', 'EmailChannel', ns) fmt = extract(path, '_format_html', 'EmailChannel', ns) class Email: - _SEV_STYLE = {'OK': {'color':'#16a34a','bg':'#f0fdf4','border':'#bbf7d0'}, - 'CRITICAL': {'color':'#dc2626','bg':'#fef2f2','border':'#fecaca'}, - 'INFO': {'color':'blue','bg':'white','border':'gray'}} - _SEV_DEFAULT = {'color':'#6b7280','bg':'#f9fafb','border':'#e5e7eb'} + from notification_channels import EmailChannel + _SEV_STYLE = EmailChannel._SEV_STYLE + _SEV_DEFAULT = EmailChannel._SEV_DEFAULT subject_prefix = 'ProxMenux' _build_detail_rows = staticmethod(build) badge = catalog['channels']['email']['severity'].get('observation') or english['channels']['email']['severity']['observation'] diff --git a/AppImage/scripts/notification_channels.py b/AppImage/scripts/notification_channels.py index 6580826f..eddc2bde 100644 --- a/AppImage/scripts/notification_channels.py +++ b/AppImage/scripts/notification_channels.py @@ -1053,11 +1053,13 @@ class EmailChannel(NotificationChannel): status = 'unconfirmed' sev['label'] = _runtime_text(f'email.status.{status}', data) group = data.get('_group', 'other') - # Keep unbroken recorded text inside the temperature email's table. - # Both properties are inline for mail clients; other events retain - # their original markup and layout. - temp_cell_wrap = 'word-wrap:break-word;overflow-wrap:break-word;word-break:break-word;' if event_type == 'temp_high' else '' - temp_table_layout = 'table-layout:fixed;' if event_type == 'temp_high' else '' + # Scoped inline mail-compatible wrapping: temperature measurements + # and backup identities/raw diagnostics. Other events retain layout. + backup_email = event_type in {'backup_complete', 'backup_fail'} + temp_cell_wrap = 'word-wrap:break-word;overflow-wrap:break-word;word-break:break-word;' if event_type == 'temp_high' or backup_email else '' + temp_table_layout = 'table-layout:fixed;' if event_type == 'temp_high' or backup_email else '' + backup_title_wrap = temp_cell_wrap if backup_email else '' + backup_metadata_layout = 'table-layout:fixed;' if backup_email else '' section_label = _runtime_text(f'email.groups.{group}', data) report_label = _runtime_text('email.report', data, group=section_label) host_label = _runtime_text('email.host', data) @@ -1089,6 +1091,13 @@ class EmailChannel(NotificationChannel): for line in body.split('\n') if line.strip() ) + if event_type in {'system_restore_completed', 'error_resolved'}: + # Observation age/disappearance must not become a green OK row. + # The endpoint's warnings_block and task counts live in the + # localized body, not the generic services Event row. + detail_rows = [('', html_mod.escape(line.strip())) + for line in body.split('\n') if line.strip()] + # ── Fallback: if no structured rows, render body text lines ── if not detail_rows: for line in body.split('\n'): @@ -1155,15 +1164,15 @@ class EmailChannel(NotificationChannel):
-

{html_mod.escape(display_title)}

+

{html_mod.escape(display_title)}

- +
-
+ {html_mod.escape(host_label)}: {html_mod.escape(data.get('hostname', ''))} diff --git a/AppImage/scripts/notification_events.py b/AppImage/scripts/notification_events.py index 6d9ac520..4aa91067 100644 --- a/AppImage/scripts/notification_events.py +++ b/AppImage/scripts/notification_events.py @@ -4294,7 +4294,7 @@ class ProxmoxHookWatcher: """Distinguish explicit failure, complete guest logs and unknown results.""" text = str(message or '') if severity in ('error', 'err', 'critical') or re.search( - r'(?im)^\s*(?:ERROR:|TASK ERROR:|.*\bStatus\s+ERROR\b)', text): + r'(?im)^\s*(?:\d+:\s*)?(?:\d{4}-\d{2}-\d{2}\s+\S+\s+)?(?:ERROR:|TASK ERROR:|.*\bStatus\s+ERROR\b)', text): return 'failed' starts = re.findall(r'(?im)\bStarting Backup of VM (\d+)\s*\(', text) finished = re.findall(r'(?im)\bFinished Backup of VM (\d+)\s*\(', text) @@ -4303,7 +4303,7 @@ class ProxmoxHookWatcher: if table is not None and any(guest['status'].lower() == 'error' for guest in table['vms']): return 'failed' if severity not in ('info', 'ok', 'success') or re.search( - r'(?im)(?:^\s*WARNING:|\bWARNINGS\s*:\s*\d+)', text): + r'(?im)(?:^\s*(?:\d+:\s*)?(?:\d{4}-\d{2}-\d{2}\s+\S+\s+)?WARN(?:ING)?:|\bWARNINGS\s*:\s*\d+)', text): return 'unconfirmed' # A present table is authoritative: do not certify an incomplete table # from a finished guest log, or reject a complete OK table merely @@ -4313,7 +4313,16 @@ class ProxmoxHookWatcher: all(guest['status'].lower() == 'ok' for guest in table['vms']) else 'unconfirmed') if starts: - return 'confirmed' if sorted(starts) == sorted(finished) else 'unconfirmed' + pending = {} + for match in re.finditer(r'(?im)\b(Starting|Finished) Backup of VM (\d+)\s*\(', text): + action, vmid = match.groups() + if action.lower() == 'starting': + pending[vmid] = pending.get(vmid, 0) + 1 + elif not pending.get(vmid): + return 'unconfirmed' # A finish before its start is not evidence. + else: + pending[vmid] -= 1 + return 'confirmed' if not any(pending.values()) else 'unconfirmed' if re.search( r'(?im)^\s*(?:INFO:\s*)?TASK OK\s*$', text): return 'confirmed' @@ -4379,10 +4388,10 @@ class ProxmoxHookWatcher: } if event_type in ('backup_complete', 'backup_fail'): # This is presentation metadata, not a new event/toggle/delivery path. + outcome = self._backup_outcome(severity_raw, message) data['backup_outcome'] = ( - 'failed' if event_type == 'backup_fail' else - self._backup_outcome(severity_raw, message) if pve_type == 'vzdump' - else 'unconfirmed' + 'failed' if event_type == 'backup_fail' or outcome == 'failed' else + outcome if pve_type == 'vzdump' else 'unconfirmed' ) if pve_type == 'replication': diff --git a/AppImage/scripts/notification_manager.py b/AppImage/scripts/notification_manager.py index 2b68d298..332072a9 100644 --- a/AppImage/scripts/notification_manager.py +++ b/AppImage/scripts/notification_manager.py @@ -2467,6 +2467,11 @@ class NotificationManager: runtime_data.get('hostname'), self._config, ) runtime_data.setdefault('_notification_language', self._notification_language()) + # Match queued dispatch's presentation context for these outcome + # notices; this does not alter event/severity or direct-send policy. + if event_type in ('backup_complete', 'backup_fail', 'error_resolved', 'system_restore_completed'): + runtime_data['_event_type'] = event_type + runtime_data['_group'] = TEMPLATES[event_type].get('group', 'other') # Render template if available if event_type in TEMPLATES and not message: diff --git a/AppImage/scripts/notification_templates.py b/AppImage/scripts/notification_templates.py index d0a20dd5..69d6bdbd 100644 --- a/AppImage/scripts/notification_templates.py +++ b/AppImage/scripts/notification_templates.py @@ -238,7 +238,7 @@ def _parse_vzdump_table(message: str) -> Optional[Dict[str, Any]]: valid = False break valid = bool(valid and all(values) - and re.fullmatch(r'(?:\d+:\d{2}:\d{2}|(?:\d+[dhms]\s*)+)', duration) + and re.fullmatch(r'(?:\d+:\d{2}:\d{2}|(?:\d+[yMwdhms]\s*)+)', duration) and re.fullmatch(r'\d+(?:\.\d+)?\s*(?:[KMGTPE]i?B?|B)', size, re.IGNORECASE)) if status.lower() in ('err', 'error'): status = 'error' @@ -309,6 +309,18 @@ def _parse_vzdump_message(message: str) -> Optional[Dict[str, Any]]: } continue + # A finish can belong to a guest already stored when another + # start arrived. Preserve that guest's actual completion too. + prior_finish = re.match(r'Finished Backup of VM (\d+)\s+\(([^)]+)\)', clean) + if prior_finish: + prior = next((vm for vm in reversed(vms) + if vm['vmid'] == prior_finish.group(1)), None) + if prior is not None: + prior['time'] = prior_finish.group(2) + if prior['status'] != 'error': + prior['status'] = 'ok' + continue + if current_vm: # Guest name m_name = re.match(r'(?:CT|VM) Name:\s*(.+)', clean) @@ -357,6 +369,21 @@ def _parse_vzdump_message(message: str) -> Optional[Dict[str, Any]]: if current_vm: vms.append(current_vm) + # Explicit guest-linked failures outrank a contradictory summary OK row. + # Job-level/prune errors do not invalidate unrelated successfully saved guests. + for line in lines: + error = re.match(r'^\s*(?:\d+:\s*)?(?:\d{4}-\d{2}-\d{2}\s+\S+\s+)?(?:ERROR:|TASK ERROR:)\s*(.*)', + line, re.IGNORECASE) + if not error: + continue + failed_guest = re.search(r'\bBackup of (?:VM|CT) (\d+) failed\b|\bbackup failed for (?:VM|CT) (\d+)\b', + error.group(1), re.IGNORECASE) + if failed_guest: + vmid = failed_guest.group(1) or failed_guest.group(2) + for vm in vms: + if vm['vmid'] == vmid: + vm['status'] = 'error' + # ── Extract totals ── for line in lines: m_time = re.search(r'Total running time:\s*(.+)', line) @@ -372,7 +399,7 @@ def _parse_vzdump_message(message: str) -> Optional[Dict[str, Any]]: sizes_gib = 0.0 for vm in vms: s = vm.get('size', '') - m = re.match(r'([\d.]+)\s+(.*)', s) + m = re.fullmatch(r'(\d+(?:\.\d+)?)\s+([KMGTPE]i?B|B)', s, re.IGNORECASE) if m: val = float(m.group(1)) unit = m.group(2).strip().upper() @@ -1869,11 +1896,19 @@ def render_template(event_type: str, data: Dict[str, Any], template['title'] = runtime_message('backup.errorTitle', language, hostname=data.get('hostname') or _get_hostname()) template['body'] = runtime_message('backup.errorBody', language) - if event_type == 'backup_complete' and data.get('backup_outcome') in ('confirmed', 'failed'): + if event_type == 'backup_fail': + template['title'] = runtime_message('backup.errorTitle', language, + hostname=data.get('hostname') or _get_hostname()) + if event_type == 'backup_fail' or (event_type == 'backup_complete' and data.get('backup_outcome') in ('confirmed', 'failed')): parsed_backup = _parse_vzdump_message(str(data.get('pve_message') or '')) storage = str((parsed_backup or {}).get('storage_name') or data.get('storage') or '').strip() guests = (parsed_backup or {}).get('vms') or [] - if data.get('backup_outcome') == 'failed': + # Explicit confirmed manual metadata is useful context, not evidence + # about an unparsed batch. Only use it when there is no raw report. + if not data.get('pve_message') and data.get('backup_outcome') == 'confirmed' and data.get('vmid'): + guests = [{'vmid': str(data['vmid']), 'name': str(data.get('vmname') or ''), + 'type': str(data.get('vm_type') or ''), 'status': 'ok'}] + if event_type == 'backup_fail' or data.get('backup_outcome') == 'failed': guests = [guest for guest in guests if guest.get('status', '').lower() == 'error'] target = [] if storage: @@ -1915,6 +1950,11 @@ def render_template(event_type: str, data: Dict[str, Any], 'log_file': '', } variables.update(data) + if event_type == 'backup_fail' or (event_type == 'backup_complete' and data.get('backup_outcome') in ('confirmed', 'failed')): + # The provider has already substituted raw Display Names. Insert the + # resolved title as a value, never reinterpret its literal braces. + variables['_backup_title'] = template['title'] + template['title'] = '{_backup_title}' # Old persisted errors and manual events may lack a complete reading. # Accept plain numeric strings, but never interpret booleans or objects as @@ -2047,11 +2087,11 @@ def render_template(event_type: str, data: Dict[str, Any], if parsed: is_success = (event_type == 'backup_complete') body_text = _format_vzdump_body(parsed, is_success, language=language) - if event_type == 'backup_complete' and data.get('backup_outcome') == 'failed': - error_lines = [line.strip() for line in pve_message.splitlines() - if re.match(r'^\s*(?:ERROR:|TASK ERROR)', line, re.IGNORECASE)] - if error_lines: - body_text += '\n' + '\n'.join(error_lines) + diagnostic_lines = [line.strip() for line in pve_message.splitlines() + if re.match(r'^\s*(?:\d+:\s*)?(?:\d{4}-\d{2}-\d{2}\s+\S+\s+)?(?:WARN(?:ING)?:|ERROR:|TASK ERROR)', + line, re.IGNORECASE)] + if diagnostic_lines: + body_text += '\n' + '\n'.join(dict.fromkeys(diagnostic_lines)) else: # Couldn't parse -- use PVE raw message as body body_text = pve_message.strip() @@ -2068,6 +2108,28 @@ def render_template(event_type: str, data: Dict[str, Any], except (ValueError, IndexError): body_text = template['body'] + if event_type == 'backup_complete' and data.get('backup_outcome') == 'confirmed' and not pve_message: + context = [] + if data.get('vmid'): + name = str(data.get('vmname') or '') + context.append(f"{name} ({data['vmid']})" if name else str(data['vmid'])) + if data.get('storage'): + context.append(str(data['storage'])) + if data.get('size'): + context.append(runtime_message('vzdump.size', language, value=data['size'])) + if data.get('duration'): + context.append(runtime_message('vzdump.duration', language, value=data['duration'])) + if context: + body_text += '\n' + '\n'.join(context) + + # PVE can move a one-line setup/abort reason exclusively into its subject. + # Preserve that raw failure context, without using it as a localized title. + if event_type in ('backup_complete', 'backup_fail') and ( + event_type == 'backup_fail' or data.get('backup_outcome') == 'failed'): + source_subject = str(data.get('pve_title') or '').strip() + if source_subject and source_subject not in body_text: + body_text += '\n' + source_subject + # Clean up: collapse runs of 3+ blank lines into 1, remove trailing whitespace import re as _re body_text = _re.sub(r'\n{3,}', '\n\n', body_text.strip()) diff --git a/AppImage/scripts/tests/test_notification_runtime_i18n.py b/AppImage/scripts/tests/test_notification_runtime_i18n.py index abd1b127..6242ffae 100644 --- a/AppImage/scripts/tests/test_notification_runtime_i18n.py +++ b/AppImage/scripts/tests/test_notification_runtime_i18n.py @@ -77,9 +77,16 @@ class RuntimeCatalogTests(unittest.TestCase): "channels.email.severity.observation", "channels.email.status.unconfirmed"} for language, catalog in self.catalogs.items(): translated = flatten(catalog) - expected = set(en) - pending_slovak if language == 'sk' else set(en) - self.assertEqual(set(translated), expected, language) - for key in expected: + if language == 'sk': + # Missing maintainer-owned leaves may be generated later. + # Accept only this bounded gap, and validate every present leaf. + self.assertTrue(set(en) - pending_slovak <= set(translated), language) + self.assertTrue(set(translated) <= set(en), language) + else: + self.assertEqual(set(translated), set(en), language) + for key in translated: + self.assertIsInstance(translated[key], str, f"{language}:{key}") + self.assertTrue(translated[key].strip(), f"{language}:{key}") if language == 'sk' and key in ('templates.backup_complete.title', 'templates.backup_complete.body'): continue # exact upstream SK, superseded only at render time diff --git a/AppImage/scripts/tests/test_vzdump_ai_integrity.py b/AppImage/scripts/tests/test_vzdump_ai_integrity.py index 458606bf..2286842c 100644 --- a/AppImage/scripts/tests/test_vzdump_ai_integrity.py +++ b/AppImage/scripts/tests/test_vzdump_ai_integrity.py @@ -275,8 +275,14 @@ class VzdumpAIIntegrityTests(unittest.TestCase): rendered["body"], "CRITICAL", data, ) + # The failure title now identifies the unique failed guest; inventory + # remains exactly once in the detail table, not suppressed from body. + inventory = html.split('', 1)[1].split('
', 1)[0] for vmid in range(100, 149): - self.assertEqual(html.count(f"guest-{vmid} ({vmid})"), 1, vmid) + self.assertEqual(inventory.count(f"guest-{vmid} ({vmid})"), 1, vmid) + self.assertIn('guest-148 (148)', rendered['title']) + self.assertNotIn('guest-100 (100)', rendered['title']) + self.assertEqual(html.count('guest-148 (148)'), 2) # title + inventory self.assertEqual(html.count("49 backups"), 1) self.assertEqual(html.count("1 failed"), 1) self.assertEqual(html.count(">Zlyhalo<"), 1)