fix(regression): handle sandbox expiry during resource sampling

tools/regression_traffic.py

@@ -28,23 +28,54 @@ def percentile(values, fraction):
     return sorted(values)[max(0, math.ceil(len(values) * fraction) - 1)] if values else None
 
 
+def missing_containers(error, candidates):
+    messages = {
+        prefix + container: container
+        for container in candidates
+        for prefix in ('Error: No such object: ', 'Error response from daemon: No such container: ')
+    }
+    lines = [line.strip() for line in error.splitlines() if line.strip()]
+    return {messages[line] for line in lines} if lines and all(line in messages for line in lines) else set()
+
+
 def owned_box_containers(data_root, checked=None):
     """Discover sandboxes mounted below one run's data root, including newly created groups."""
     if checked is None:
 
         def checked(command):
-            return subprocess.check_output(command, text=True, timeout=10)
+            return subprocess.check_output(command, text=True, stderr=subprocess.PIPE, timeout=10)
 
     candidates = checked(['docker', 'ps', '-a', '--filter', 'label=langbot.box=true', '--format', '{{.ID}}']).split()
     if not candidates:
         return []
     root = Path(data_root).resolve()
-    descriptions = checked(['docker', 'inspect', '--format', '{{json .}}', *candidates])
     owned = []
-    for line in descriptions.splitlines():
-        if not line:
+    for container in candidates:
+        try:
+            description = checked(['docker', 'inspect', '--format', '{{json .}}', container])
+        except subprocess.CalledProcessError as exc:
+            if (
+                exc.returncode != 1
+                or (exc.output or '').strip()
+                or not missing_containers(exc.stderr or '', {container})
+            ):
+                raise
+            continue  # A sandbox may expire between discovery and inspection.
+        except RuntimeError as exc:
+            if not missing_containers(str(exc), {container}):
+                raise
             continue
-        entry = json.loads(line)
+        entry = json.loads(description)
+        if (
+            not isinstance(entry, dict)
+            or not isinstance(entry.get('Id'), str)
+            or not entry['Id'].startswith(container)
+            or not isinstance(entry.get('Mounts'), list)
+            or any(
+                not isinstance(mount, dict) or not isinstance(mount.get('Source', ''), str) for mount in entry['Mounts']
+            )
+        ):
+            raise ValueError(f'Malformed container inspection: {container}')
         if any(
             Path(mount['Source']).resolve().is_relative_to(root) for mount in entry['Mounts'] if mount.get('Source')
         ):
@@ -70,9 +101,10 @@ def sample_containers(configured, data_root, run=subprocess.run, discover=owned_
 
     result = stats(containers)
     observation = {}
-    if result.returncode and data_root:
+    missing = missing_containers(result.stderr, containers) if result.returncode == 1 else set()
+    if missing and data_root:
         current = selected()
-        if containers - current:
+        if missing <= containers - current:
             observation['container_churn'] = {
                 'removed': sorted(containers - current),
                 'added': sorted(current - containers),

tools/test_regression_containers.py

@@ -0,0 +1,104 @@
+"""Container expiry must not hide Docker failures or expand cleanup ownership."""
+
+import json
+import subprocess
+from types import SimpleNamespace
+import unittest
+from unittest.mock import Mock
+
+from regression_traffic import owned_box_containers, sample_containers
+
+
+def description(container, mount):
+    return json.dumps({'Id': container, 'Mounts': [{'Source': mount}]})
+
+
+class ContainerSamplingTests(unittest.TestCase):
+    def test_expiry_between_list_and_inspect_preserves_owned_survivors(self):
+        for error in (
+            subprocess.CalledProcessError(1, ['docker', 'inspect'], '\n', 'Error: No such object: expired'),
+            RuntimeError('Error: No such object: expired\n'),
+        ):
+            with self.subTest(error=type(error).__name__):
+                checked = Mock(
+                    side_effect=[
+                        'expired live foreign',
+                        error,
+                        description('live', '/owned/box/session'),
+                        description('foreign', '/owned/box-other/session'),
+                    ]
+                )
+                self.assertEqual(owned_box_containers('/owned/box', checked), ['live'])
+
+    def test_inspect_errors_are_not_reported_as_expiry(self):
+        for error in (
+            subprocess.CalledProcessError(1, [], '', 'permission denied'),
+            subprocess.CalledProcessError(1, [], '', 'Cannot connect to the Docker daemon'),
+            subprocess.CalledProcessError(1, [], '', 'Error: No such object: different-id'),
+            subprocess.CalledProcessError(1, [], '', 'Error: No such object: live\npermission denied'),
+            subprocess.CalledProcessError(2, [], '', 'Error: No such object: live'),
+            subprocess.CalledProcessError(1, [], '{"partial":true}', 'Error: No such object: live'),
+            subprocess.TimeoutExpired(['docker', 'inspect'], 10),
+            RuntimeError('Cannot connect to the Docker daemon'),
+        ):
+            with self.subTest(error=repr(error)):
+                with self.assertRaises(type(error)) as caught:
+                    owned_box_containers('/owned', Mock(side_effect=['live', error]))
+                self.assertIs(caught.exception, error)
+
+    def test_malformed_inspection_fails_instead_of_dropping_a_sample(self):
+        for output in (
+            '',
+            '{truncated',
+            '{}',
+            '{"Mounts":null}',
+            '[]',
+            '{"Id":"live","Mounts":[null]}',
+            '{"Id":"other","Mounts":[]}',
+            '{"Id":"live","Mounts":[{"Source":42}]}',
+        ):
+            with self.subTest(output=output):
+                with self.assertRaises((ValueError, KeyError, TypeError)):
+                    owned_box_containers('/owned', Mock(side_effect=['live', output]))
+
+    def test_discovery_failure_is_not_an_empty_inventory(self):
+        error = subprocess.CalledProcessError(1, [], '', 'permission denied')
+        with self.assertRaises(subprocess.CalledProcessError) as caught:
+            owned_box_containers('/owned', Mock(side_effect=error))
+        self.assertIs(caught.exception, error)
+
+    def test_unrelated_stats_failure_is_not_hidden_by_concurrent_removal(self):
+        for error in (
+            'permission denied',
+            'Cannot connect to the Docker daemon',
+            'Error response from daemon: No such container: old\npermission denied',
+        ):
+            with self.subTest(error=error):
+                run = Mock(return_value=SimpleNamespace(returncode=1, stdout='', stderr=error))
+                discover = Mock(side_effect=[['old', 'live'], ['live']])
+                result = sample_containers([], '/owned', run, discover)
+                self.assertEqual(result['container_error'], error)
+                self.assertEqual(run.call_count, 1)
+                self.assertEqual(discover.call_count, 1)
+
+    def test_configured_container_loss_remains_a_failure(self):
+        error = 'Error response from daemon: No such container: configured'
+        run = Mock(return_value=SimpleNamespace(returncode=1, stdout='', stderr=error))
+        result = sample_containers(['configured'], '/owned', run, Mock(return_value=[]))
+        self.assertEqual(result['container_error'], error)
+        self.assertEqual(run.call_count, 1)
+
+    def test_all_dynamic_containers_removed_returns_observed_churn(self):
+        run = Mock(
+            return_value=SimpleNamespace(
+                returncode=1, stdout='', stderr='Error response from daemon: No such container: old'
+            )
+        )
+        result = sample_containers([], '/owned', run, Mock(side_effect=[['old'], []]))
+        self.assertEqual(result['containers'], [])
+        self.assertEqual(result['container_churn']['removed'], ['old'])
+        self.assertNotIn('container_error', result)
+
+
+if __name__ == '__main__':
+    unittest.main()

tools/test_regression_load_ledger.py

@@ -42,7 +42,7 @@ class LoadLedgerTests(unittest.TestCase):
         self.assertEqual(result['not_completed'], ['late'])
 
     def test_container_churn_recovers_once_but_other_collection_errors_fail(self):
-        failed = SimpleNamespace(returncode=1, stdout='', stderr='No such container: old')
+        failed = SimpleNamespace(returncode=1, stdout='', stderr='Error response from daemon: No such container: old')
         success = SimpleNamespace(returncode=0, stdout='{"MemUsage":"1MiB / 1GiB","PIDs":"1"}\n', stderr='')
         run = Mock(side_effect=[failed, success])
         result = sample_containers([], '/owned', run, Mock(side_effect=[['old', 'live'], ['live']]))