From 9d61cbbbe8eaa9d421749da41ad999819f9c8221 Mon Sep 17 00:00:00 2001 From: Vaibhav Srivastava Date: Sat, 22 Aug 2026 10:40:06 +0530 Subject: [PATCH] fix: recover the original result on a duplicate approval decision The /approve, /reject and /decision endpoints all funnel through _decide, which mapped ApprovalAlreadyProcessed straight to a 409. A client retrying after a network timeout has no way to read that as "the decision you are re-sending already succeeded", so a successful approval surfaced as an error. Catch ApprovalAlreadyProcessed and recover via engine.get_processed_result, exactly as ConversationService already does for the same exception, and keep the 409 only when the result is genuinely unavailable. Closes #109 --- src/agent_engine/api/app.py | 20 +++++++++++++++++++- tests/api/test_approval_endpoints.py | 19 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/agent_engine/api/app.py b/src/agent_engine/api/app.py index ce8a22a7..2583c21e 100644 --- a/src/agent_engine/api/app.py +++ b/src/agent_engine/api/app.py @@ -26,6 +26,7 @@ ) from agent_engine.approvals.decision import ApprovalDecision, parse_decision from agent_engine.approvals.errors import ( + ApprovalAlreadyProcessed, ApprovalError, InvalidDecision, approval_http_status, @@ -311,15 +312,32 @@ async def _decide( ) -> InvokeResponse: engine = _hitl_engine() auth = _auth_context(authorization) + caller_session_id = _run_context(session_id, run_id=run_id).conversation_id try: result = await engine.resume( run_id, approval_id, decision, caller_user_id=user_id, - caller_session_id=_run_context(session_id, run_id=run_id).conversation_id, + caller_session_id=caller_session_id, access_token=auth.inbound_access_token if auth else None, ) + except ApprovalAlreadyProcessed as exc: + # A duplicate decision is usually a client retry after a network + # timeout, not an error: the decision it is re-sending already + # succeeded. Recover the original result so the retry sees the same + # answer it would have seen the first time, matching what + # ConversationService already does for the same exception. Only fall + # through to the 409 when the result is genuinely unavailable. + recovered = await engine.get_processed_result( + run_id, + approval_id, + caller_user_id=user_id, + caller_session_id=caller_session_id, + ) + if recovered is None: + raise _map_approval_error(exc) from exc + result = recovered except ApprovalError as exc: raise _map_approval_error(exc) from exc except Exception: diff --git a/tests/api/test_approval_endpoints.py b/tests/api/test_approval_endpoints.py index db8090af..92353959 100644 --- a/tests/api/test_approval_endpoints.py +++ b/tests/api/test_approval_endpoints.py @@ -230,6 +230,25 @@ def test_approve_with_no_body_at_all_succeeds(client: TestClient) -> None: assert response.json()["status"] == "completed" +def test_duplicate_approval_recovers_the_original_result(client: TestClient) -> None: + """A retried decision returns the first result, not a bare 409. + + A client that times out and retries has no way to read 409 as "your + decision already succeeded". ConversationService already recovers via + get_processed_result for this case; the HTTP layer must match. + """ + run_id, approval_id = _trigger_pending_approval(client) + + first = client.post(f"/runs/{run_id}/approvals/{approval_id}/approve") + assert first.status_code == 200 + + duplicate = client.post(f"/runs/{run_id}/approvals/{approval_id}/approve") + + assert duplicate.status_code == 200 + assert duplicate.json()["status"] == first.json()["status"] + assert duplicate.json()["answer"] == first.json()["answer"] + + def test_reject_with_no_body_at_all_succeeds(client: TestClient) -> None: run_id, approval_id = _trigger_pending_approval(client)