fix(doctor): make the two backup Repair buttons actually do something

Plan 72 item A.1. The backup_drill_stale and backup_unverified checks both
emitted repairable: True, but DoctorService.repair() only knew drift, service,
dns and extension. Every Repair button those checks rendered came back
"Unknown repair kind: backup_drill" -- a button that failed 100% of the time,
on the check family whose entire job is telling you whether your backups can
be trusted.

The dispatcher now handles both, and they are deliberately asymmetric because
the underlying services are. request_drill enqueues a backup.drill.run job, so
that answer carries the job id for the console to link to. verify_run does the
size and checksum comparison inline and returns a verdict, so there is no job
id -- claiming one would send the console looking for a job that does not
exist. The plan assumed both enqueued; only the drill does.

Verification finding a BAD copy is reported as a failure, not a fix. It is a
worse outcome than "unverified" and calling it a successful repair would be a
lie told by a green checkmark. Both branches turn their services' ordinary
refusals -- a drill already in flight, no successful backup to drill, backup
not found -- into plain error text rather than letting them escape, and a
policy_id echoed back for a since-deleted policy reads as gone instead of
raising.

The second half is the same defect from the other side. verify_run compares
against the REMOTE copy and raises when there is none, so a local-only backup
was rendering a Repair button that could never succeed. That check now only
offers the button when there is something to verify against, and otherwise
says what would actually fix it: send the policy offsite, or run a restore
drill.

Drills started this way are attributed to 'doctor' rather than 'manual', so
the drill history says where they came from -- the field is free-form and only
displayed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

backend/app/services/doctor_service.py

@@ -590,15 +590,27 @@ class DoctorService:
         key = f'backup_unverified.{policy.id}'
         title = f'Backup verification: {cls._policy_label(policy)}'
         level = run.effective_verify_level()
-        if level == 'none':
+        if level != 'none':
+            return _check(key, title, 'ok', f'Latest backup verified ({level}).')
+
+        # The repair verifies the REMOTE copy (size + checksum against the
+        # provider). With no remote copy there is nothing for it to compare, so
+        # offering the button would be offering a guaranteed failure — the
+        # exact thing this check family got wrong. Say what would fix it
+        # instead: an offsite copy, or a restore drill.
+        if not getattr(run, 'remote_key', None):
             return _check(
                 key, title, 'warn',
-                'The latest backup has not been verified (no integrity check or '
+                'The latest backup has not been verified, and it has no offsite '
-                'restore drill has run against it yet).',
+                'copy to verify against. Send this policy to remote storage, or '
-                repairable=True,
+                'run a restore drill to prove the backup restores.')
-                repair_ref={'kind': 'backup_verify', 'policy_id': policy.id,
+        return _check(
-                            'run_id': run.id})
+            key, title, 'warn',
-        return _check(key, title, 'ok', f'Latest backup verified ({level}).')
+            'The latest backup has not been verified (no integrity check or '
+            'restore drill has run against it yet).',
+            repairable=True,
+            repair_ref={'kind': 'backup_verify', 'policy_id': policy.id,
+                        'run_id': run.id})
 
     @classmethod
     def _setup_checks(cls):
@@ -700,11 +712,88 @@ class DoctorService:
                 from app.services import doctor_check_registry
                 results.append({'item': item, **doctor_check_registry.repair(
                     item.get('namespace'), item.get('ref'))})
+            elif kind == 'backup_drill':
+                results.append({'item': item,
+                                **cls._repair_backup_drill(item.get('policy_id'))})
+            elif kind == 'backup_verify':
+                results.append({'item': item, **cls._repair_backup_verify(
+                    item.get('policy_id'), item.get('run_id'))})
             else:
                 results.append({'item': item, 'success': False,
                                 'error': f'Unknown repair kind: {kind}'})
         return results
 
+    @staticmethod
+    def _load_policy(policy_id):
+        """Resolve a backup policy for a repair, or ``None``.
+
+        The id arrives from the client echoing back a repair_ref, so it is
+        looked up rather than trusted — an id for a policy that has since been
+        deleted must read as "gone", not raise.
+        """
+        try:
+            from app.models.backup_policy import BackupPolicy
+            return BackupPolicy.query.filter_by(id=policy_id).first()
+        except Exception:  # noqa: BLE001
+            return None
+
+    @classmethod
+    def _repair_backup_drill(cls, policy_id):
+        """Run a fresh restore drill for a policy.
+
+        Asynchronous: request_drill enqueues a ``backup.drill.run`` job and
+        returns it, so the answer carries the job id for the console to link
+        to rather than pretending the drill is already done.
+        """
+        policy = cls._load_policy(policy_id)
+        if policy is None:
+            return {'success': False,
+                    'error': f'Backup policy {policy_id} no longer exists.'}
+        from app.services.backup_drill_service import (
+            BackupDrillError, BackupDrillService,
+        )
+        try:
+            job = BackupDrillService.request_drill(policy, trigger='doctor')
+        except BackupDrillError as e:
+            # "already drilling" / "no successful backup to drill" are ordinary
+            # refusals, not faults — surface the reason as-is.
+            return {'success': False, 'error': str(e)}
+        except Exception as e:  # noqa: BLE001
+            return {'success': False, 'error': str(e)}
+        return {'success': True, 'job_id': getattr(job, 'id', None),
+                'detail': 'Restore drill queued.'}
+
+    @classmethod
+    def _repair_backup_verify(cls, policy_id, run_id):
+        """Verify a backup run's remote copy.
+
+        Unlike the drill this is synchronous — verify_run does the size and
+        checksum comparison inline and returns the verdict, so there is no job
+        id to hand back.
+        """
+        policy = cls._load_policy(policy_id)
+        if policy is None:
+            return {'success': False,
+                    'error': f'Backup policy {policy_id} no longer exists.'}
+        from app.services.backup_policy_service import (
+            BackupPolicyError, BackupPolicyService,
+        )
+        try:
+            result = BackupPolicyService.verify_run(policy, run_id)
+        except BackupPolicyError as e:
+            return {'success': False, 'error': str(e)}
+        except Exception as e:  # noqa: BLE001
+            return {'success': False, 'error': str(e)}
+        if not result.get('verified'):
+            # The check ran and the copy did NOT match — a real answer, and a
+            # worse one than "unverified". Do not report it as a fix.
+            return {'success': False,
+                    'error': 'Verification ran but the remote copy did not '
+                             'match — this backup may not restore.',
+                    'detail': result.get('detail')}
+        return {'success': True, 'verified': True,
+                'detail': 'Remote copy verified.'}
+
     @classmethod
     def _restart_service(cls, name):
         if name not in CORE_SERVICES:

backend/tests/test_doctor_backup_repairs.py

@@ -0,0 +1,215 @@
+"""Plan 72 A.1 — the two doctor repair kinds that could never work.
+
+``backup_drill_stale.*`` and ``backup_unverified.*`` both emitted
+``repairable: True``, but ``DoctorService.repair()`` only knew drift, service,
+dns and extension. Every Repair button those checks rendered came back
+"Unknown repair kind: backup_drill" — a button that failed 100% of the time.
+
+The dispatcher now handles both. They are deliberately asymmetric: a drill is
+enqueued as a job (so the answer carries a job id), while verification runs
+inline and returns a verdict.
+"""
+from unittest.mock import patch
+
+import pytest
+
+from app.services.doctor_service import DoctorService
+
+
+class _Job:
+    id = 4242
+
+
+@pytest.fixture
+def policy(app):
+    """A backup policy the repair refs can point at."""
+    from app import db
+    from app.models.backup_policy import BackupPolicy
+
+    row = BackupPolicy(target_type='app', target_id=1)
+    db.session.add(row)
+    db.session.commit()
+    return row
+
+
+# --------------------------------------------------------------------------- #
+# The regression itself
+# --------------------------------------------------------------------------- #
+def test_both_kinds_are_known_to_the_dispatcher(app, policy):
+    """The whole bug: these two came back 'Unknown repair kind'."""
+    with patch('app.services.backup_drill_service.BackupDrillService.request_drill',
+               return_value=_Job()), \
+            patch('app.services.backup_policy_service.BackupPolicyService.verify_run',
+                  return_value={'success': True, 'verified': True}):
+        results = DoctorService.repair([
+            {'kind': 'backup_drill', 'policy_id': policy.id},
+            {'kind': 'backup_verify', 'policy_id': policy.id, 'run_id': 7},
+        ])
+
+    assert [r['success'] for r in results] == [True, True]
+    for r in results:
+        assert 'Unknown repair kind' not in str(r.get('error') or '')
+
+
+def test_unknown_kind_still_errors_cleanly(app):
+    results = DoctorService.repair([{'kind': 'nonsense'}])
+
+    assert results[0]['success'] is False
+    assert 'Unknown repair kind: nonsense' in results[0]['error']
+
+
+def test_each_result_echoes_its_input_item(app, policy):
+    """DoctorPanel pairs answers back to checks by the echoed item."""
+    with patch('app.services.backup_drill_service.BackupDrillService.request_drill',
+               return_value=_Job()):
+        item = {'kind': 'backup_drill', 'policy_id': policy.id}
+        results = DoctorService.repair([item])
+
+    assert results[0]['item'] == item
+
+
+# --------------------------------------------------------------------------- #
+# backup_drill — asynchronous, surfaces a job id
+# --------------------------------------------------------------------------- #
+class TestBackupDrillRepair:
+
+    def test_enqueues_a_drill_and_returns_the_job_id(self, app, policy):
+        with patch('app.services.backup_drill_service.BackupDrillService.request_drill',
+                   return_value=_Job()) as request:
+            result = DoctorService.repair(
+                [{'kind': 'backup_drill', 'policy_id': policy.id}])[0]
+
+        assert result['success'] is True
+        assert result['job_id'] == 4242
+        assert request.call_args.args[0].id == policy.id
+
+    def test_the_drill_is_attributed_to_the_doctor(self, app, policy):
+        """The drill history shows where a drill came from; a doctor-initiated
+        one should not masquerade as someone pressing the manual button."""
+        with patch('app.services.backup_drill_service.BackupDrillService.request_drill',
+                   return_value=_Job()) as request:
+            DoctorService.repair([{'kind': 'backup_drill', 'policy_id': policy.id}])
+
+        assert request.call_args.kwargs['trigger'] == 'doctor'
+
+    def test_a_refusal_is_reported_not_raised(self, app, policy):
+        """'already drilling' / 'no successful backup' are ordinary answers."""
+        from app.services.backup_drill_service import BackupDrillError
+
+        with patch('app.services.backup_drill_service.BackupDrillService.request_drill',
+                   side_effect=BackupDrillError('A restore drill is already in progress.')):
+            result = DoctorService.repair(
+                [{'kind': 'backup_drill', 'policy_id': policy.id}])[0]
+
+        assert result['success'] is False
+        assert 'already in progress' in result['error']
+
+    def test_an_unexpected_failure_does_not_escape(self, app, policy):
+        with patch('app.services.backup_drill_service.BackupDrillService.request_drill',
+                   side_effect=RuntimeError('queue exploded')):
+            result = DoctorService.repair(
+                [{'kind': 'backup_drill', 'policy_id': policy.id}])[0]
+
+        assert result['success'] is False
+        assert 'queue exploded' in result['error']
+
+    def test_a_deleted_policy_reads_as_gone(self, app):
+        """policy_id comes back from the client echoing a repair_ref; a stale
+        one must not raise."""
+        result = DoctorService.repair(
+            [{'kind': 'backup_drill', 'policy_id': 999999}])[0]
+
+        assert result['success'] is False
+        assert 'no longer exists' in result['error']
+
+
+# --------------------------------------------------------------------------- #
+# backup_verify — synchronous, returns a verdict
+# --------------------------------------------------------------------------- #
+class TestBackupVerifyRepair:
+
+    def _verify(self, policy, **patch_kwargs):
+        with patch('app.services.backup_policy_service.BackupPolicyService.verify_run',
+                   **patch_kwargs) as verify:
+            result = DoctorService.repair([{'kind': 'backup_verify',
+                                            'policy_id': policy.id,
+                                            'run_id': 7}])[0]
+        return result, verify
+
+    def test_a_verified_copy_succeeds(self, app, policy):
+        result, verify = self._verify(
+            policy, return_value={'success': True, 'verified': True})
+
+        assert result['success'] is True
+        assert result['verified'] is True
+        assert verify.call_args.args[1] == 7
+
+    def test_no_job_id_because_it_is_synchronous(self, app, policy):
+        """Unlike the drill, nothing is queued — claiming a job id would send
+        the console looking for a job that does not exist."""
+        result, _ = self._verify(
+            policy, return_value={'success': True, 'verified': True})
+
+        assert 'job_id' not in result
+
+    def test_a_mismatching_copy_is_a_failure_not_a_fix(self, app, policy):
+        """Verification running and finding a BAD copy is a worse outcome than
+        'unverified' — reporting it as a successful repair would be a lie."""
+        result, _ = self._verify(
+            policy, return_value={'success': True, 'verified': False,
+                                  'detail': {'size': 'mismatch'}})
+
+        assert result['success'] is False
+        assert 'did not match' in result['error']
+
+    def test_a_policy_error_is_reported_not_raised(self, app, policy):
+        from app.services.backup_policy_service import BackupPolicyError
+
+        result, _ = self._verify(
+            policy, side_effect=BackupPolicyError('Backup not found'))
+
+        assert result['success'] is False
+        assert 'Backup not found' in result['error']
+
+    def test_a_deleted_policy_reads_as_gone(self, app):
+        result = DoctorService.repair(
+            [{'kind': 'backup_verify', 'policy_id': 999999, 'run_id': 7}])[0]
+
+        assert result['success'] is False
+        assert 'no longer exists' in result['error']
+
+
+# --------------------------------------------------------------------------- #
+# The other half: don't render a button that cannot work
+# --------------------------------------------------------------------------- #
+class TestVerifyButtonIsOnlyOfferedWhenItCanWork:
+    """verify_run compares against the REMOTE copy and raises when there is
+    none. A local-only backup would therefore render a Repair button that
+    fails every time — the same defect A.1 exists to remove."""
+
+    def _check_for(self, app, policy, remote_key):
+        from app import db
+        from app.models.backup_run import BackupRun
+
+        run = BackupRun(policy_id=policy.id, kind='full', status='success')
+        run.remote_key = remote_key
+        db.session.add(run)
+        db.session.commit()
+        return DoctorService._backup_unverified_check(policy)
+
+    def test_local_only_backup_is_not_repairable(self, app, policy):
+        check = self._check_for(app, policy, None)
+
+        assert check['status'] == 'warn'
+        assert check['repairable'] is False
+        assert check['repair_ref'] is None
+        # and it says what WOULD fix it
+        assert 'offsite' in check['detail'] or 'restore drill' in check['detail']
+
+    def test_backup_with_a_remote_copy_is_repairable(self, app, policy):
+        check = self._check_for(app, policy, 's3://bucket/key')
+
+        assert check['status'] == 'warn'
+        assert check['repairable'] is True
+        assert check['repair_ref']['kind'] == 'backup_verify'
+        assert check['repair_ref']['policy_id'] == policy.id