fix: release conversation subject in cancel terminal update

openhands/automation/dispatcher.py

@@ -497,7 +497,18 @@ async def _fail(
     if result.success:
         await update_run_current_phase(session_factory, run.id, "Starting automation")
         if ctx.sandbox_id:
-            await update_sandbox_id(session_factory, run.id, ctx.sandbox_id)
+            recorded = await update_sandbox_id(session_factory, run.id, ctx.sandbox_id)
+            if recorded is False:
+                # The run left RUNNING while provisioning (cancelled or
+                # failed concurrently): drop the sandbox instead of
+                # attaching it to a terminal row nobody will clean up.
+                logger.info(
+                    "Run %s left RUNNING during provisioning; releasing sandbox",
+                    run_id,
+                    extra=_log_ctx(sandbox_id=ctx.sandbox_id),
+                )
+                await backend.release_context(client, ctx)
+                return
         if result.bash_command_id:
             # Persist the BashCommand id so the verifier can filter
             # BashOutput events by exactly this command (avoids

openhands/automation/router.py

@@ -18,7 +18,7 @@
     status,
 )
 from fastapi.responses import RedirectResponse
-from sqlalchemy import func, select, update
+from sqlalchemy import case, func, select, update
 from sqlalchemy.engine import CursorResult
 from sqlalchemy.ext.asyncio import AsyncSession
 from sqlalchemy.orm import selectinload
@@ -1003,6 +1003,16 @@ async def cancel_run(
             completed_at=now,
             error_detail="Cancelled by user",
             status_detail=None,
+            # A cancelled run is terminal, so it can never open or continue
+            # its conversation afterwards: release its subject atomically in
+            # this same update, whether or not it ever recorded a sandbox.
+            # Runs without a subject keep NULL either way. A sandbox that
+            # finishes provisioning afterwards is ignored (status-guarded)
+            # and released by the dispatcher, so no orphan can attach here.
+            subject_released_at=case(
+                (AutomationRun.subject_key.is_not(None), now),
+                else_=AutomationRun.subject_released_at,
+            ),
         )
     )
     db_result: CursorResult = await session.execute(stmt)  # type: ignore[assignment]
@@ -1025,15 +1035,11 @@ async def cancel_run(
     )
 
     # Clean up sandbox for runs that were RUNNING. Cancelling is explicit, so
-    # unlike `complete_run` the sandbox goes even when the run owns a subject
-    # -- but the subject is released with it, or the next event would pick this
-    # run and pay a lookup for a sandbox we just deleted. The key stays on the
-    # row as the record of what this run was about.
+    # unlike `complete_run` the sandbox goes even when the run owned a subject
+    # (already released by the terminal update above, so the next event will
+    # not pick this run). The key stays on the row as the record of what this
+    # run was about.
     if run.sandbox_id:
-        if run.subject_key and run.subject_released_at is None:
-            run.subject_released_at = utcnow()
-            await session.commit()
-
         from openhands.automation.config import get_settings
 
         settings = get_settings()

openhands/automation/utils/run.py

@@ -293,24 +293,35 @@ async def update_sandbox_id(
     session_factory: async_sessionmaker[AsyncSession],
     run_id: uuid.UUID,
     sandbox_id: str,
-) -> None:
+) -> bool | None:
     """Store the sandbox ID on the automation run for later verification.
 
+    Only applies while the run is still RUNNING. A run cancelled (or
+    otherwise finished) mid-provisioning must not gain a sandbox
+    afterwards: nobody would clean it up, and it would sit on a terminal
+    row. Returns True when recorded, False when the run is no longer
+    RUNNING, and None on a write error so execution can continue.
+
     Args:
         session_factory: Async session factory
         run_id: The run ID to update
         sandbox_id: The sandbox ID to store
     """
     try:
         async with session_factory() as session:
-            await session.execute(
+            result: CursorResult = await session.execute(  # type: ignore[assignment]
                 update(AutomationRun)
-                .where(AutomationRun.id == run_id)
+                .where(
+                    AutomationRun.id == run_id,
+                    AutomationRun.status == AutomationRunStatus.RUNNING,
+                )
                 .values(sandbox_id=sandbox_id)
             )
             await session.commit()
+            return (result.rowcount or 0) > 0
     except Exception:
         logger.exception("Failed to update sandbox_id for run %s", run_id)
+        return None
 
 
 async def update_bash_command_id(

tests/test_cancel_run.py

@@ -131,6 +131,114 @@ async def test_cancel_other_orgs_run_returns_403(async_client, async_session):
     assert resp.status_code == 403
 
 
+async def test_cancel_subject_run_without_sandbox_releases_subject(
+    async_client, async_session
+):
+    """Cancelling a subject run with no sandbox still releases the subject."""
+    _, run = await _create_automation_with_run(
+        async_session, status=AutomationRunStatus.RUNNING
+    )
+    run.subject_key = "team/C123/1755000000.000100"
+    await async_session.commit()
+
+    resp = await async_client.post(f"/api/automation/v1/runs/{run.id}/cancel")
+    assert resp.status_code == 200
+
+    await async_session.refresh(run)
+    assert run.status == AutomationRunStatus.CANCELLED
+    assert run.subject_released_at is not None
+    assert run.subject_key == "team/C123/1755000000.000100"
+
+
+async def test_cancel_ordinary_run_touches_no_subject(async_client, async_session):
+    """Cancelling a run without a subject leaves subject columns alone."""
+    _, run = await _create_automation_with_run(
+        async_session, status=AutomationRunStatus.RUNNING
+    )
+
+    resp = await async_client.post(f"/api/automation/v1/runs/{run.id}/cancel")
+    assert resp.status_code == 200
+
+    await async_session.refresh(run)
+    assert run.status == AutomationRunStatus.CANCELLED
+    assert run.subject_key is None
+    assert run.subject_released_at is None
+
+
+async def test_cancelled_subject_no_longer_blocks_resubmission(
+    async_client, async_session
+):
+    """After cancel, a resubmitted event routes to a new run instead of being
+    folded into the cancelled run that still holds the subject."""
+    from openhands.automation.conversations import continue_conversation
+
+    automation, run = await _create_automation_with_run(
+        async_session, status=AutomationRunStatus.PENDING
+    )
+    run.subject_key = "team/C123/1755000000.000100"
+    await async_session.commit()
+
+    resp = await async_client.post(f"/api/automation/v1/runs/{run.id}/cancel")
+    assert resp.status_code == 200
+
+    result = await continue_conversation(
+        async_session,
+        org_id=TEST_ORG_ID,
+        source="slack",
+        subject_key="team/C123/1755000000.000100",
+        automation_id=automation.id,
+        event_key="Ev2",
+        event_payload={},
+    )
+    assert result.needs_run is True
+
+
+async def test_late_sandbox_record_after_cancel_is_ignored(
+    async_client, async_session, async_session_factory
+):
+    """A sandbox recorded after cancel must not attach to the cancelled run.
+
+    Covers the race where the dispatcher is still provisioning while the
+    user cancels: without the status guard the late record would orphan a
+    sandbox on a terminal row and fork the released subject.
+    """
+    from openhands.automation.conversations import continue_conversation
+    from openhands.automation.utils.run import update_sandbox_id
+
+    automation, run = await _create_automation_with_run(
+        async_session, status=AutomationRunStatus.RUNNING
+    )
+    run.subject_key = "team/C123/1755000000.000100"
+    await async_session.commit()
+
+    resp = await async_client.post(f"/api/automation/v1/runs/{run.id}/cancel")
+    assert resp.status_code == 200
+
+    # The endpoint's session stays open in tests (the app commits it on
+    # teardown in production): close its transaction so the dispatcher's
+    # own session below sees a committed row instead of blocking on it.
+    await async_session.commit()
+
+    recorded = await update_sandbox_id(async_session_factory, run.id, "sandbox-A")
+    assert recorded is False
+
+    await async_session.refresh(run)
+    assert run.status == AutomationRunStatus.CANCELLED
+    assert run.sandbox_id is None
+    assert run.subject_released_at is not None
+
+    result = await continue_conversation(
+        async_session,
+        org_id=TEST_ORG_ID,
+        source="slack",
+        subject_key="team/C123/1755000000.000100",
+        automation_id=automation.id,
+        event_key="Ev2",
+        event_payload={},
+    )
+    assert result.needs_run is True
+
+
 async def test_cancel_same_org_other_users_run(async_client, async_session):
     """Cancelling a run owned by another member of the same org should succeed."""
     automation = Automation(