report unsubmitted agent patches to Sentry

services/hackbot-api/app/routers/runs.py

@@ -5,6 +5,7 @@
 from typing import Annotated
 
 from fastapi import APIRouter, Depends, Header, HTTPException, Query, status
+from hackbot_runtime.actions.phabricator import PATCH_ACTION_TYPES
 from pydantic import BeforeValidator
 from sqlalchemy import select
 from sqlalchemy.ext.asyncio import AsyncSession
@@ -19,6 +20,7 @@
 from app.jobs import ExecutionStatus
 from app.schemas import (
     AgentDescriptor,
+    ArtifactRef,
     RunActionDoc,
     RunDoc,
     RunRef,
@@ -30,6 +32,8 @@
 
 router = APIRouter(dependencies=[Depends(require_api_key)])
 
+_PATCH_ARTIFACT = "changes/changes.patch"
+
 
 def _normalize_identity(email: str | None) -> str | None:
     if not email:
@@ -269,9 +273,29 @@ async def finalize_run(db: AsyncSession, run: Run) -> None:
     run.finalized_at = datetime.now(timezone.utc)
 
     await db.commit()
+
+    if _has_unsubmitted_patch(summary, artifacts):
+        log.error(
+            "Agent run produced code changes without submitting a patch "
+            "(run_id=%s, agent=%s)",
+            run.run_id,
+            run.agent,
+        )
     await pubsub.publish_run_completed(str(run.run_id), run.agent, run.status)
 
 
+def _has_unsubmitted_patch(
+    summary: RunSummary | None,
+    artifacts: list[ArtifactRef],
+) -> bool:
+    """Whether a run produced source changes without a patch action."""
+    has_patch_artifact = any(artifact.name == _PATCH_ARTIFACT for artifact in artifacts)
+    has_patch_action = summary is not None and any(
+        action["type"] in PATCH_ACTION_TYPES for action in summary.actions
+    )
+    return has_patch_artifact and not has_patch_action
+
+
 def _terminal_status(
     exec_status: ExecutionStatus, summary: RunSummary | None
 ) -> tuple[RunStatus, str | None]:

services/hackbot-api/tests/test_finalize_run.py

@@ -12,6 +12,7 @@
 import pytest
 from app import gcs, jobs, pubsub
 from app.jobs import ExecutionStatus
+from app.routers import runs as runs_module
 from app.routers.runs import finalize_run
 from app.schemas import ArtifactRef, RunStatus, RunSummary
 
@@ -95,6 +96,35 @@ async def test_finalizes_succeeded_run(monkeypatch, _no_publish):
     assert _no_publish == [(str(run.run_id), run.agent, RunStatus.succeeded.value)]
 
 
+@pytest.mark.parametrize(
+    ("actions", "artifacts", "expected"),
+    [
+        ([], ["changes/changes.patch"], True),
+        (
+            ["phabricator.submit_patch"],
+            ["changes/changes.patch"],
+            False,
+        ),
+        (
+            ["phabricator.update_patch"],
+            ["changes/changes.patch"],
+            False,
+        ),
+        ([], [], False),
+        (None, ["changes/changes.patch"], True),
+    ],
+)
+def test_has_unsubmitted_patch(actions, artifacts, expected):
+    summary = (
+        None
+        if actions is None
+        else RunSummary(status="ok", actions=[{"type": action} for action in actions])
+    )
+    artifact_refs = [ArtifactRef(name=artifact, size=10) for artifact in artifacts]
+
+    assert runs_module._has_unsubmitted_patch(summary, artifact_refs) is expected
+
+
 async def test_finalizes_as_failed_when_summary_missing(monkeypatch):
     run = _FakeRun()
     db = _FakeDB()