Skip to content

Commit ea9f383

Browse files
committed
fix(runner): close the adversarial-review findings on the approvals train
1 parent 4d6ac3c commit ea9f383

25 files changed

Lines changed: 678 additions & 130 deletions

File tree

api/oss/src/apis/fastapi/sessions/models.py

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
from datetime import datetime
2-
from typing import Any, Dict, List, Optional
2+
from typing import Any, Dict, List, Literal, Optional
33
from uuid import UUID
44

5-
from pydantic import BaseModel, Field
5+
from pydantic import BaseModel, ConfigDict, Field, model_validator
66

77
from oss.src.core.sessions.streams.dtos import (
88
SessionStream,
@@ -101,17 +101,34 @@ class SessionInteractionCreateRequest(BaseModel):
101101
meta: Optional[Dict[str, Any]] = None
102102

103103

104+
class SessionInteractionResolution(BaseModel):
105+
model_config = ConfigDict(extra="forbid")
106+
107+
verdict: Literal["approved", "denied"]
108+
tool_call_id: str
109+
110+
104111
class SessionInteractionTransitionRequest(BaseModel):
105112
# No project_id: scope comes from the caller's credential (request.state).
106113
session_id: str
107114
token: str
108115
status: SessionInteractionStatus
109-
resolution: Optional[Dict[str, Any]] = None
116+
resolution: Optional[SessionInteractionResolution] = None
117+
118+
@model_validator(mode="after")
119+
def validate_resolution_status(self) -> "SessionInteractionTransitionRequest":
120+
# Resolution is answer data, so it is valid only on the resolved lifecycle edge.
121+
if (
122+
self.resolution is not None
123+
and self.status != SessionInteractionStatus.resolved
124+
):
125+
raise ValueError("resolution is only valid when status is resolved")
126+
return self
110127

111128

112129
class SessionInteractionCancelStaleRequest(BaseModel):
113-
# Cancels prior turns' pending gates, sparing this turn's own (`turn_id`) and any gates
114-
# the current turn answers in-band (`tokens` — a resume must resolve them, not cancel).
130+
# Cancels prior turns' pending gates, sparing this turn's own (`turn_id`) and every prior
131+
# gate still owned by a live partial resume (`tokens`, including carried gates).
115132
session_id: str
116133
turn_id: str
117134
tokens: Optional[List[str]] = None

api/oss/src/apis/fastapi/sessions/router.py

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@
5454
from oss.src.core.sessions.records.streaming import publish_record
5555
from oss.src.core.sessions.interactions.dtos import (
5656
SessionInteractionCreate,
57+
SessionInteractionKind,
58+
SessionInteractionQuery,
5759
SessionInteractionStatus,
5860
SessionInteractionTransition,
5961
)
@@ -638,14 +640,41 @@ async def transition_interaction(
638640
):
639641
raise FORBIDDEN_EXCEPTION
640642

643+
resolution = (
644+
body.resolution.model_dump() if body.resolution is not None else None
645+
)
646+
if resolution is not None:
647+
interactions = await self.interactions_service.query_interactions(
648+
project_id=project_id,
649+
query=SessionInteractionQuery(session_id=body.session_id),
650+
)
651+
source = next(
652+
(
653+
interaction
654+
for interaction in interactions
655+
if interaction.token == body.token
656+
),
657+
None,
658+
)
659+
if source is None:
660+
raise HTTPException(
661+
status_code=status.HTTP_404_NOT_FOUND,
662+
detail="Interaction not found or already terminal",
663+
)
664+
if source.kind != SessionInteractionKind.user_approval:
665+
raise HTTPException(
666+
status_code=status.HTTP_409_CONFLICT,
667+
detail="Resolution is only valid for user approval interactions",
668+
)
669+
641670
try:
642671
interaction = await self.interactions_service.transition_interaction(
643672
transition=SessionInteractionTransition(
644673
project_id=project_id,
645674
session_id=body.session_id,
646675
token=body.token,
647676
status=body.status,
648-
resolution=body.resolution,
677+
resolution=resolution,
649678
),
650679
)
651680
except InteractionNotFound:

api/oss/tests/pytest/unit/sessions/test_transition_interaction_resolution.py

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
from unittest.mock import AsyncMock, patch
22
from uuid import uuid4
33

4-
from fastapi import FastAPI, Request
4+
from fastapi import FastAPI, HTTPException, Request
5+
from fastapi.testclient import TestClient
6+
import pytest
57

68
from oss.src.apis.fastapi.sessions.models import SessionInteractionTransitionRequest
79
from oss.src.apis.fastapi.sessions.router import InteractionsRouter
@@ -34,6 +36,17 @@ async def test_transition_route_passes_resolution_to_the_domain_transition():
3436
captured = []
3537

3638
class _InteractionsService:
39+
async def query_interactions(self, *, project_id, query):
40+
return [
41+
SessionInteraction(
42+
project_id=project_id,
43+
session_id=query.session_id,
44+
token="approval-token",
45+
kind=SessionInteractionKind.user_approval,
46+
status=SessionInteractionStatus.pending,
47+
)
48+
]
49+
3750
async def transition_interaction(self, *, transition):
3851
captured.append(transition)
3952
return SessionInteraction(
@@ -75,3 +88,80 @@ async def transition_interaction(self, *, transition):
7588
assert response.interaction is not None
7689
assert response.interaction.data is not None
7790
assert response.interaction.data.resolution == captured[0].resolution
91+
92+
93+
@pytest.mark.parametrize(
94+
"payload",
95+
[
96+
{
97+
"session_id": "session-1",
98+
"token": "approval-token",
99+
"status": "pending",
100+
"resolution": {"verdict": "approved", "tool_call_id": "tool-1"},
101+
},
102+
{
103+
"session_id": "session-1",
104+
"token": "approval-token",
105+
"status": "resolved",
106+
"resolution": {"verdict": "maybe", "tool_call_id": "tool-1"},
107+
},
108+
{
109+
"session_id": "session-1",
110+
"token": "approval-token",
111+
"status": "resolved",
112+
"resolution": {"verdict": "approved"},
113+
},
114+
],
115+
)
116+
def test_transition_route_rejects_invalid_approval_resolution_with_422(payload):
117+
router = InteractionsRouter(
118+
interactions_service=AsyncMock(),
119+
workflows_service=AsyncMock(),
120+
respond_task=AsyncMock(),
121+
)
122+
app = FastAPI()
123+
app.include_router(router.router)
124+
125+
response = TestClient(app).post("/transition", json=payload)
126+
127+
assert response.status_code == 422
128+
129+
130+
async def test_transition_route_rejects_resolution_for_client_tool_with_409():
131+
project_id = uuid4()
132+
user_id = uuid4()
133+
interactions_service = AsyncMock()
134+
interactions_service.query_interactions.return_value = [
135+
SessionInteraction(
136+
project_id=project_id,
137+
session_id="session-1",
138+
token="client-tool-token",
139+
kind=SessionInteractionKind.client_tool,
140+
status=SessionInteractionStatus.pending,
141+
)
142+
]
143+
router = InteractionsRouter(
144+
interactions_service=interactions_service,
145+
workflows_service=AsyncMock(),
146+
respond_task=AsyncMock(),
147+
)
148+
body = SessionInteractionTransitionRequest(
149+
session_id="session-1",
150+
token="client-tool-token",
151+
status=SessionInteractionStatus.resolved,
152+
resolution={"verdict": "approved", "tool_call_id": "tool-1"},
153+
)
154+
155+
with patch(
156+
"oss.src.apis.fastapi.sessions.router.check_action_access",
157+
new_callable=AsyncMock,
158+
return_value=True,
159+
):
160+
with pytest.raises(HTTPException) as caught:
161+
await router.transition_interaction(
162+
request=_make_authed_request(FastAPI(), project_id, user_id),
163+
body=body,
164+
)
165+
166+
assert caught.value.status_code == 409
167+
interactions_service.transition_interaction.assert_not_awaited()

sdks/python/agenta/sdk/agents/adapters/vercel/messages.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,12 +196,19 @@ def _tool_part_blocks(part: Dict[str, Any], ptype: str) -> List[ContentBlock]:
196196
if isinstance(_input, dict)
197197
else type(_input).__name__,
198198
)
199+
approval_output: Dict[str, Any] = {"approved": approved}
200+
approval = part.get("approval")
201+
interaction_token = (
202+
approval.get("id") if isinstance(approval, dict) else None
203+
)
204+
if isinstance(interaction_token, str) and interaction_token:
205+
approval_output["interactionToken"] = interaction_token
199206
blocks.append(
200207
ContentBlock(
201208
type="tool_result",
202209
tool_call_id=tool_call_id,
203210
tool_name=tool_name,
204-
output={"approved": approved},
211+
output=approval_output,
205212
)
206213
)
207214
return blocks
@@ -229,7 +236,13 @@ def _approval_response_blocks(part: Dict[str, Any]) -> List[ContentBlock]:
229236
output = part.get("output")
230237
if output is None:
231238
approved = part.get("approved")
232-
output = {"approved": approved} if approved is not None else part.get("reason")
239+
if approved is not None:
240+
output = {"approved": approved}
241+
interaction_token = part.get("approvalId")
242+
if isinstance(interaction_token, str) and interaction_token:
243+
output["interactionToken"] = interaction_token
244+
else:
245+
output = part.get("reason")
233246
return [ContentBlock(type="tool_result", tool_call_id=tool_call_id, output=output)]
234247

235248

sdks/python/oss/tests/pytest/unit/agents/test_ui_messages.py

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,10 @@ def test_inline_approval_responded_deny_emits_approved_envelope(self):
212212
"type": "tool_result",
213213
"toolCallId": "call_1",
214214
"toolName": "deleteFile",
215-
"output": {"approved": False},
215+
"output": {
216+
"approved": False,
217+
"interactionToken": "perm_1",
218+
},
216219
},
217220
]
218221

@@ -238,7 +241,10 @@ def test_inline_approval_responded_approve_emits_approved_envelope(self):
238241
result = msgs[0].content[1]
239242
assert result.type == "tool_result"
240243
assert result.tool_call_id == "call_2"
241-
assert result.output == {"approved": True}
244+
assert result.output == {
245+
"approved": True,
246+
"interactionToken": "perm_2",
247+
}
242248

243249
def test_concurrent_inline_approvals_each_emit_their_own_result(self):
244250
# Concurrent approvals: the browser round-trips TWO tool parts in one turn, each with its
@@ -274,11 +280,11 @@ def test_concurrent_inline_approvals_each_emit_their_own_result(self):
274280
# Each decision lands on its own toolCallId, in order, approve and deny preserved.
275281
assert (results[0].tool_call_id, results[0].output) == (
276282
"call_1",
277-
{"approved": True},
283+
{"approved": True, "interactionToken": "perm_1"},
278284
)
279285
assert (results[1].tool_call_id, results[1].output) == (
280286
"call_2",
281-
{"approved": False},
287+
{"approved": False, "interactionToken": "perm_2"},
282288
)
283289

284290
def test_inline_output_denied_state_emits_denied_envelope(self):
@@ -304,7 +310,10 @@ def test_inline_output_denied_state_emits_denied_envelope(self):
304310
result = msgs[0].content[1]
305311
assert result.type == "tool_result"
306312
assert result.tool_call_id == "call_3"
307-
assert result.output == {"approved": False}
313+
assert result.output == {
314+
"approved": False,
315+
"interactionToken": "perm_3",
316+
}
308317

309318
def test_pending_approval_request_only_part_emits_no_decision(self):
310319
# A tool part still awaiting a decision (`approval-requested`, no `approval.approved`)

sdks/python/oss/tests/pytest/unit/test_workflow_control_running.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ def row(self, sid: str) -> dict:
117117

118118
@pytest.mark.asyncio
119119
async def test_send_on_idle_session_marks_alive_and_increments_token():
120-
121120
store = _FakeSessionStore()
122121
started = asyncio.Event()
123122

@@ -142,7 +141,6 @@ async def slow_run():
142141

143142
@pytest.mark.asyncio
144143
async def test_send_collision_when_alive_without_control_is_409():
145-
146144
store = _FakeSessionStore()
147145
store.row("s1")["alive"] = True
148146

@@ -154,7 +152,6 @@ async def test_send_collision_when_alive_without_control_is_409():
154152

155153
@pytest.mark.asyncio
156154
async def test_steer_cancels_alive_run_and_restarts():
157-
158155
store = _FakeSessionStore()
159156
row = store.row("s1")
160157
row["alive"] = True
@@ -176,7 +173,6 @@ async def new_run():
176173

177174
@pytest.mark.asyncio
178175
async def test_cancel_marks_dead():
179-
180176
store = _FakeSessionStore()
181177
row = store.row("s1")
182178
row["alive"] = True
@@ -194,7 +190,6 @@ async def test_cancel_marks_dead():
194190

195191
@pytest.mark.asyncio
196192
async def test_attach_to_alive_detached_run():
197-
198193
store = _FakeSessionStore()
199194
row = store.row("s1")
200195
row["alive"] = True
@@ -211,7 +206,6 @@ async def test_attach_to_alive_detached_run():
211206

212207
@pytest.mark.asyncio
213208
async def test_detach_on_client_disconnect_keeps_run_alive():
214-
215209
store = _FakeSessionStore()
216210
row = store.row("s1")
217211
row["alive"] = True

0 commit comments

Comments
 (0)