From a7b9f966dadef0c2251f3a5b5378ff685e82873e Mon Sep 17 00:00:00 2001 From: Suhaib Mujahid Date: Thu, 17 Sep 2026 16:30:29 -0400 Subject: [PATCH] Let a trigger tell a new run from a deduplicated one `dedupe_key` made `trigger_run` answer two different things with the same body: the run this request started, or the run that already held the key. The API distinguishes them by status code alone, `201` against `200`, and `RunRef.model_validate` throws that away, so a caller that needs to react to one and not the other has no way to. Which callers need to depends on what the key is keying. A retried webhook delivery does not care, and neither does a push that fans out. A Slack button does: someone is waiting on an answer, and "a run is already underway, here it is" is a different message from silence. `TriggeredRun` subclasses `RunRef` and adds `is_new`, so the three existing callers keep reading `run_id` off the return value unchanged. `is_new` belongs on this type rather than on `RunRef` because it is a property of the request, not of the run: the same run is `is_new` to the request that started it and not to every later request carrying its key. Named for the predicate it is, rather than `created`, which sits one underscore from the `created_at` on a run row. --- .../hackbot-client/hackbot_client/__init__.py | 3 +- libs/hackbot-client/hackbot_client/client.py | 12 ++++-- libs/hackbot-client/hackbot_client/models.py | 13 +++++++ libs/hackbot-client/tests/test_client.py | 37 +++++++++++++++++++ 4 files changed, 60 insertions(+), 5 deletions(-) diff --git a/libs/hackbot-client/hackbot_client/__init__.py b/libs/hackbot-client/hackbot_client/__init__.py index 1f3eecd1e9..12fa3438cd 100644 --- a/libs/hackbot-client/hackbot_client/__init__.py +++ b/libs/hackbot-client/hackbot_client/__init__.py @@ -1,8 +1,9 @@ from hackbot_client.client import HackbotClient -from hackbot_client.models import RunRef, RunStatus +from hackbot_client.models import RunRef, RunStatus, TriggeredRun __all__ = [ "HackbotClient", "RunRef", "RunStatus", + "TriggeredRun", ] diff --git a/libs/hackbot-client/hackbot_client/client.py b/libs/hackbot-client/hackbot_client/client.py index 4668b8ec6b..aa4306f402 100644 --- a/libs/hackbot-client/hackbot_client/client.py +++ b/libs/hackbot-client/hackbot_client/client.py @@ -7,7 +7,7 @@ import httpx -from hackbot_client.models import RunRef +from hackbot_client.models import TriggeredRun class HackbotClient: @@ -27,12 +27,12 @@ async def trigger_run( *, on_behalf_of: str | None = None, dedupe_key: str | None = None, - ) -> RunRef: + ) -> TriggeredRun: """Create an agent run and return the API's typed run reference. `dedupe_key` keys the work the run does, and a key belongs to one run for good: repeated triggers carrying it are no-ops, answered with the - same run reference. + same run reference and `is_new=False`. """ headers = {"X-API-Key": self._api_key} if on_behalf_of is not None: @@ -49,4 +49,8 @@ async def trigger_run( ) response.raise_for_status() - return RunRef.model_validate(response.json()) + # The API distinguishes the two outcomes only by status code: `201` for the + # run this request started, `200` for one a `dedupe_key` collapsed onto. + return TriggeredRun.model_validate( + {**response.json(), "is_new": response.status_code == 201} + ) diff --git a/libs/hackbot-client/hackbot_client/models.py b/libs/hackbot-client/hackbot_client/models.py index 8100013a59..afc4c89d87 100644 --- a/libs/hackbot-client/hackbot_client/models.py +++ b/libs/hackbot-client/hackbot_client/models.py @@ -20,3 +20,16 @@ class RunRef(BaseModel): run_id: UUID agent: str status: RunStatus + + +class TriggeredRun(RunRef): + """The run a trigger resolved to, and whether that request is what started it. + + `is_new` is a property of the request rather than of the run, and says + nothing about the run's `status`: the same run is `is_new` to the request + that created it and not to every later request carrying its `dedupe_key`. + That is why it is not on the API's own `RunRef`. Subclasses `RunRef` so a + caller that only wants the run keeps reading `run_id` off it unchanged. + """ + + is_new: bool diff --git a/libs/hackbot-client/tests/test_client.py b/libs/hackbot-client/tests/test_client.py index 58d1e132a2..5a3bba61b5 100644 --- a/libs/hackbot-client/tests/test_client.py +++ b/libs/hackbot-client/tests/test_client.py @@ -58,6 +58,8 @@ async def test_trigger_run_posts_inputs_and_returns_typed_reference(monkeypatch) assert run.run_id == UUID(RUN_ID) assert run.agent == "bug-fix" assert run.status is RunStatus.pending + # `201`: this request is what started it. + assert run.is_new is True assert captured == { "timeout": 30.0, "url": "https://hackbot.example/agents/bug-fix/runs", @@ -97,3 +99,38 @@ async def test_trigger_run_rejects_an_invalid_success_response(monkeypatch): with pytest.raises(ValidationError): await _client().trigger_run("bug-fix", {"bug_id": 1234}) + + +async def test_trigger_run_reports_a_deduplicated_run_as_not_new(monkeypatch): + # `200` rather than `201`: the key already belonged to this run, so the + # request that got this answer started nothing. + _capture_post( + monkeypatch, + httpx.Response( + 200, + json={"run_id": RUN_ID, "agent": "bug-fix", "status": "running"}, + ), + ) + + run = await _client().trigger_run( + "bug-fix", {"bug_id": 1234}, dedupe_key="push:autoland:abc123" + ) + + assert run.run_id == UUID(RUN_ID) + assert run.is_new is False + + +async def test_trigger_run_sends_the_dedupe_key_as_a_query_parameter(monkeypatch): + captured = _capture_post( + monkeypatch, + httpx.Response( + 201, + json={"run_id": RUN_ID, "agent": "bug-fix", "status": "pending"}, + ), + ) + + await _client().trigger_run( + "bug-fix", {"bug_id": 1234}, dedupe_key="push:autoland:abc123" + ) + + assert captured["params"] == {"dedupe_key": "push:autoland:abc123"}