fix(orchestrator): interrupt-proof teardown so a timeout can't drop task.json

src/coder_eval/orchestrator.py

@@ -557,10 +557,28 @@ def _kill_agent_subprocess_sync() -> None:
                 logger.error(f"Evaluation failed: {e}", exc_info=True)
 
             finally:
-                # BEFORE post-run/cleanup: needs the live sandbox to resolve
-                # the agent-aligned `uip`, and post-task tool state on disk.
-                self._refresh_runtime_tool_versions()
-                await self._run_post_run_commands()
+                # Teardown must be interrupt-proof: the task-timeout watchdog can
+                # fire while post-run commands are awaiting and deliver its
+                # CancelledError right here in the finally block, which used to
+                # abort it wholesale — skipping _cleanup() (tempdir leaked) AND
+                # _finalize_result() (task.json lost, so the task silently drops
+                # out of the run). Catch the interrupt, finish the full teardown,
+                # then re-raise it at the end so callers observe the same exception
+                # as before. The watchdog cancels exactly once, so the teardown
+                # awaits below run normally after the CancelledError is caught.
+                teardown_interrupt: BaseException | None = None
+                try:
+                    # BEFORE post-run/cleanup: needs the live sandbox to resolve
+                    # the agent-aligned `uip`, and post-task tool state on disk.
+                    self._refresh_runtime_tool_versions()
+                    await self._run_post_run_commands()
+                except (Exception, asyncio.CancelledError) as e:
+                    teardown_interrupt = e
+                    logger.warning(
+                        "Teardown interrupted during post-run (%s: %s); completing cleanup before re-raising",
+                        type(e).__name__,
+                        e,
+                    )
                 await self._cleanup()
                 # Capture the sanitised log tail AFTER teardown so any errors
                 # logged during post-run / cleanup also land in the report,
@@ -576,6 +594,8 @@ def _kill_agent_subprocess_sync() -> None:
                 }:
                     self.result.error_log_tail = log_tail.get_text() or None
                 self._finalize_result(start_time)
+                if teardown_interrupt is not None:
+                    raise teardown_interrupt
 
         return self.result
 

tests/test_teardown_interrupt.py

@@ -0,0 +1,90 @@
+"""``run()``'s teardown must be interrupt-proof.
+
+The task-timeout watchdog cancels the asyncio task via
+``loop.call_soon_threadsafe(task.cancel)``, delivering a ``CancelledError`` at
+the next await. When that await is inside ``_run_post_run_commands`` (post-run
+commands do awaited I/O), the cancellation used to land in ``run()``'s
+``finally`` block and abort it wholesale — skipping ``_cleanup()`` (tempdir
+leaked; workspace never preserved) AND ``_finalize_result()`` (task.json lost,
+so the task silently vanished from the run). These tests pin that a post-run
+interrupt — cancellation or a plain exception — still runs the full teardown
+(cleanup + finalize) and then re-raises the original exception unchanged.
+"""
+
+import asyncio
+from pathlib import Path
+from unittest.mock import AsyncMock, patch
+
+import pytest
+
+from coder_eval.models import (
+    AgentKind,
+    FileExistsCriterion,
+    SandboxConfig,
+    TaskDefinition,
+    parse_agent_config,
+)
+from coder_eval.orchestrator import Orchestrator
+
+
+def _build_orchestrator(tmp_path: Path) -> Orchestrator:
+    task = TaskDefinition(
+        task_id="teardown_interrupt_task",
+        description="d",
+        initial_prompt="p",
+        agent=parse_agent_config(type=AgentKind.CLAUDE_CODE),
+        sandbox=SandboxConfig(driver="tempdir"),
+        success_criteria=[FileExistsCriterion(description="x", path="x.py")],
+    )
+    run_dir = tmp_path / "run" / "teardown_interrupt_task"
+    return Orchestrator(task=task, run_dir=run_dir, variant_id="v1")
+
+
+def _patch_finalize_persistence():
+    """Skip the on-disk persistence side-effects of _finalize_result."""
+    return patch("coder_eval.reports_html.write_task_html", return_value=None)
+
+
+@pytest.mark.asyncio
+async def test_post_run_cancellation_still_runs_cleanup_and_finalize(tmp_path):
+    """The task-timeout watchdog's CancelledError lands during post-run: teardown
+    must still complete (cleanup + finalize) and the cancellation still propagate."""
+    orch = _build_orchestrator(tmp_path)
+    cleanup = AsyncMock()
+
+    async def boom_setup() -> None:
+        raise RuntimeError("synthetic setup failure")
+
+    with (
+        _patch_finalize_persistence(),
+        patch.object(orch, "_setup", side_effect=boom_setup),
+        patch.object(orch, "_run_post_run_commands", AsyncMock(side_effect=asyncio.CancelledError())),
+        patch.object(orch, "_cleanup", cleanup),
+        pytest.raises(asyncio.CancelledError),
+    ):
+        await orch.run()
+
+    cleanup.assert_awaited_once()
+    # _finalize_result ran: the result was completed and scored despite the interrupt.
+    assert orch.result is not None
+    assert orch.result.completed_at is not None
+
+
+@pytest.mark.asyncio
+async def test_post_run_plain_exception_still_runs_cleanup_and_reraises(tmp_path):
+    orch = _build_orchestrator(tmp_path)
+    cleanup = AsyncMock()
+
+    with (
+        _patch_finalize_persistence(),
+        patch.object(orch, "_setup", AsyncMock()),
+        patch.object(orch, "_evaluation_loop", AsyncMock(return_value=True)),
+        patch.object(orch, "_run_post_run_commands", AsyncMock(side_effect=OSError("post-run blew up"))),
+        patch.object(orch, "_cleanup", cleanup),
+        pytest.raises(OSError, match="post-run blew up"),
+    ):
+        await orch.run()
+
+    cleanup.assert_awaited_once()
+    assert orch.result is not None
+    assert orch.result.completed_at is not None