fix(gateway): correct approval URL and thread session history into runs API (#4479)
This commit is contained in:
1 parent
ccae754c84
commit
c6392902cd
5 files changed
+135
-6
No files matched your search
+11
-1
@@ -260,7 +260,7 @@ def _run_gateway_runs_api_streaming(
|
||||
session_id, msg_text, model, workspace, stream_id,
|
||||
base_url, api_key, prefill_messages, body_extras,
|
||||
*, put_gateway_event, cancel_event,
|
||||
attachments=None, cfg=None,
|
||||
attachments=None, cfg=None, session=None,
|
||||
):
|
||||
"""Submit via POST /v1/runs and relay SSE events including approval."""
|
||||
url_runs = f"{base_url.rstrip('/')}/v1/runs"
|
||||
@@ -283,6 +283,15 @@ def _run_gateway_runs_api_streaming(
|
||||
message_content = str(msg_text or "")
|
||||
instructions_parts = []
|
||||
conversation_history = []
|
||||
for entry in getattr(session, "context_messages", None) or []:
|
||||
if not isinstance(entry, dict):
|
||||
continue
|
||||
role = str(entry.get("role") or "").strip().lower()
|
||||
if role not in {"user", "assistant"}:
|
||||
continue
|
||||
content = entry.get("content")
|
||||
if content is not None:
|
||||
conversation_history.append({"role": role, "content": content})
|
||||
for entry in prefill_messages or []:
|
||||
if not isinstance(entry, dict):
|
||||
continue
|
||||
@@ -572,6 +581,7 @@ def _run_gateway_chat_streaming(
|
||||
cancel_event=cancel_event,
|
||||
attachments=attachments,
|
||||
cfg=cfg,
|
||||
session=s,
|
||||
)
|
||||
except Exception as exc:
|
||||
put_gateway_event("apperror", {
|
||||
|
||||
@@ -81,8 +81,8 @@ class HttpRunnerClient:
|
||||
|
||||
def respond_approval(self, run_id: str, approval_id: str, choice: str) -> dict[str, Any]:
|
||||
return self._post(
|
||||
f"/v1/runs/{urllib.parse.quote(str(run_id), safe='')}/approvals/{urllib.parse.quote(str(approval_id), safe='')}/respond",
|
||||
{"choice": choice},
|
||||
f"/v1/runs/{urllib.parse.quote(str(run_id), safe='')}/approval",
|
||||
{"choice": choice, "approval_id": approval_id},
|
||||
)
|
||||
|
||||
def respond_clarify(self, run_id: str, clarify_id: str, response: str) -> dict[str, Any]:
|
||||
|
||||
@@ -470,8 +470,8 @@ def test_gateway_approval_response_relay():
|
||||
from api.routes import _handle_approval_respond
|
||||
_handle_approval_respond(handler, body)
|
||||
|
||||
assert captured.get("url", "") == "http://gw:8642/v1/runs/run%20abc%2F1/approvals/appr%20x%2Fy/respond"
|
||||
assert captured["body"] == {"choice": "once"}
|
||||
assert captured.get("url", "") == "http://gw:8642/v1/runs/run%20abc%2F1/approval"
|
||||
assert captured["body"] == {"choice": "once", "approval_id": "appr x/y"}
|
||||
handler.send_response.assert_called_with(200)
|
||||
|
||||
# Cleanup.
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
"""Tests for #4479: gateway approval URL contract and conversation history threading."""
|
||||
from __future__ import annotations
|
||||
|
||||
import types
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Bug 1: respond_approval URL and payload shape
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_respond_approval_url_matches_gateway_contract():
|
||||
from api.runner_client import HttpRunnerClient
|
||||
|
||||
client = HttpRunnerClient(base_url="http://gw:8642")
|
||||
captured = {}
|
||||
|
||||
def fake_post(url, body):
|
||||
captured["url"] = url
|
||||
captured["body"] = body
|
||||
return {}
|
||||
|
||||
client._post = fake_post
|
||||
client.respond_approval("run-abc", "appr-xyz", "approve")
|
||||
|
||||
assert captured["url"] == "/v1/runs/run-abc/approval"
|
||||
assert "/approvals/" not in captured["url"]
|
||||
assert "/respond" not in captured["url"]
|
||||
|
||||
|
||||
def test_respond_approval_body_includes_approval_id():
|
||||
from api.runner_client import HttpRunnerClient
|
||||
|
||||
client = HttpRunnerClient(base_url="http://gw:8642")
|
||||
captured = {}
|
||||
|
||||
def fake_post(url, body):
|
||||
captured["url"] = url
|
||||
captured["body"] = body
|
||||
return {}
|
||||
|
||||
client._post = fake_post
|
||||
client.respond_approval("run-1", "appr-2", "deny")
|
||||
|
||||
assert captured["body"] == {"choice": "deny", "approval_id": "appr-2"}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Bug 2: conversation history threading via session parameter
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _build_conversation_history(context_messages, prefill_messages):
|
||||
"""Exercise the same logic as _run_gateway_runs_api_streaming's conversation builder."""
|
||||
session = types.SimpleNamespace(context_messages=context_messages)
|
||||
instructions_parts = []
|
||||
conversation_history = []
|
||||
for entry in getattr(session, "context_messages", None) or []:
|
||||
if not isinstance(entry, dict):
|
||||
continue
|
||||
role = str(entry.get("role") or "").strip().lower()
|
||||
if role not in {"user", "assistant"}:
|
||||
continue
|
||||
content = entry.get("content")
|
||||
if content is not None:
|
||||
conversation_history.append({"role": role, "content": content})
|
||||
for entry in prefill_messages or []:
|
||||
if not isinstance(entry, dict):
|
||||
continue
|
||||
role = str(entry.get("role") or "").strip().lower()
|
||||
content = entry.get("content")
|
||||
if role == "system":
|
||||
if isinstance(content, str) and content.strip():
|
||||
instructions_parts.append(content)
|
||||
elif content is not None:
|
||||
instructions_parts.append(str(content))
|
||||
continue
|
||||
if role not in {"user", "assistant"}:
|
||||
continue
|
||||
conversation_history.append({"role": role, "content": content})
|
||||
return conversation_history, instructions_parts
|
||||
|
||||
|
||||
def test_runs_api_threads_session_context_messages():
|
||||
context = [
|
||||
{"role": "user", "content": "hello"},
|
||||
{"role": "assistant", "content": "hi there"},
|
||||
]
|
||||
prefill = [{"role": "system", "content": "You are helpful."}]
|
||||
|
||||
history, instructions = _build_conversation_history(context, prefill)
|
||||
|
||||
assert len(history) == 2
|
||||
assert history[0] == {"role": "user", "content": "hello"}
|
||||
assert history[1] == {"role": "assistant", "content": "hi there"}
|
||||
assert instructions == ["You are helpful."]
|
||||
|
||||
|
||||
def test_runs_api_filters_system_from_context_messages():
|
||||
context = [
|
||||
{"role": "user", "content": "hello"},
|
||||
{"role": "system", "content": "injected system"},
|
||||
{"role": "assistant", "content": "hi"},
|
||||
{"role": "tool", "content": "tool output"},
|
||||
]
|
||||
prefill = []
|
||||
|
||||
history, _ = _build_conversation_history(context, prefill)
|
||||
|
||||
roles = [m["role"] for m in history]
|
||||
assert "system" not in roles
|
||||
assert "tool" not in roles
|
||||
assert roles == ["user", "assistant"]
|
||||
|
||||
|
||||
def test_runs_api_empty_context_messages():
|
||||
history, _ = _build_conversation_history(None, [{"role": "system", "content": "sys"}])
|
||||
assert history == []
|
||||
|
||||
history2, _ = _build_conversation_history([], [{"role": "user", "content": "q"}])
|
||||
assert history2 == [{"role": "user", "content": "q"}]
|
||||
@@ -111,7 +111,7 @@ def test_runner_client_maps_observe_status_and_controls(monkeypatch):
|
||||
("GET", "http://runner.local/v1/runs/run%2F1/events?cursor=event%3A2", None),
|
||||
("GET", "http://runner.local/v1/runs/run%2F1", None),
|
||||
("POST", "http://runner.local/v1/runs/run%2F1/cancel", {}),
|
||||
("POST", "http://runner.local/v1/runs/run%2F1/approvals/approval%2F1/respond", {"choice": "once"}),
|
||||
("POST", "http://runner.local/v1/runs/run%2F1/approval", {"choice": "once", "approval_id": "approval/1"}),
|
||||
("POST", "http://runner.local/v1/runs/run%2F1/clarifications/clarify%2F1/respond", {"response": "answer"}),
|
||||
("POST", "http://runner.local/v1/runs/run%2F1/messages", {"message": "next", "mode": "interrupt"}),
|
||||
("POST", "http://runner.local/v1/sessions/session%2F1/goal", {"action": "set", "text": "finish"}),
|
||||
|
||||
Reference in new issue
Block a user