diff --git a/routes/compare/compare_routes.py b/routes/compare/compare_routes.py index ad42f1a89..93bd9a422 100644 --- a/routes/compare/compare_routes.py +++ b/routes/compare/compare_routes.py @@ -235,12 +235,27 @@ def setup_compare_routes(session_manager: SessionManager): } @router.post("/{comp_id}/vote") - def vote_comparison( + async def vote_comparison( request: Request, comp_id: str, - winner: str = Form(...), # "left", "right", or "tie" ): """Record the user's vote and reveal model names if blind.""" + content_type = request.headers.get("content-type", "").split(";", 1)[0].lower() + if content_type == "application/json": + try: + vote_payload = await request.json() + except Exception as exc: + raise HTTPException(400, "Invalid vote JSON") from exc + if not isinstance(vote_payload, dict): + raise HTTPException(422, "Vote request must be an object") + winner = vote_payload.get("winner") + else: + form = await request.form() + winner = form.get("winner") + if not isinstance(winner, str) or not winner.strip(): + raise HTTPException(422, "winner is required") + winner = winner.strip() + user = get_current_user(request) db = SessionLocal() try: diff --git a/src/agent_loop.py b/src/agent_loop.py index cca93fe56..075e940e3 100644 --- a/src/agent_loop.py +++ b/src/agent_loop.py @@ -757,7 +757,7 @@ GENERIC LOOPBACK to allowed Odysseus internal endpoints. Use this whenever the u - Themes: `/api/prefs/themes`, `/api/prefs/custom-themes` - Settings: `/api/settings`, `/api/prefs/{key}` - Research: `/api/research/start`, `/api/research/tasks` (note: `/api/research/report/{id}` renders HTML — to READ a report's text use the `manage_research` tool with `action:read`, not this endpoint) -- Compare: `/api/compare/sessions`, `/api/compare/start` +- Compare: history via `GET /api/compare/history`, record a completed comparison via `POST /api/compare/record`, vote via `POST /api/compare/{comp_id}/vote` with body `{"winner":"left|right|tie"}`, and delete via `DELETE /api/compare/{comp_id}`. `/api/compare/start` is a browser multipart/session-creation flow; use the Compare UI rather than calling it through this JSON bridge. - Email: use named email tools (`list_email_accounts`, `list_emails`, `read_email`, `scan_email_unsubscribes`, `unsubscribe_email`, `send_email`, `reply_to_email`). Do NOT use `/api/email/accounts`; it is owner-filtered in tool context and may falsely return empty. - Endpoints (model providers): `/api/endpoints`, `/api/endpoints/{id}` - Shell: do NOT use `app_api` for `/api/shell/*`; use named command tooling instead. diff --git a/tests/test_agent_app_api_compare_routes.py b/tests/test_agent_app_api_compare_routes.py new file mode 100644 index 000000000..abaaedad9 --- /dev/null +++ b/tests/test_agent_app_api_compare_routes.py @@ -0,0 +1,116 @@ +"""The agent's app_api guide must describe routes its JSON bridge can call.""" + +import json +from types import SimpleNamespace + +from fastapi import FastAPI +from fastapi.testclient import TestClient + + +def test_app_api_compare_guide_matches_live_router(): + from routes.compare.compare_routes import setup_compare_routes + from src.agent_loop import TOOL_SECTIONS + + class _Sessions: + pass + + live = { + (method, route.path) + for route in setup_compare_routes(_Sessions()).routes + for method in getattr(route, "methods", set()) + } + guide = TOOL_SECTIONS["app_api"] + + assert "/api/compare/sessions" not in guide + assert ("GET", "/api/compare/history") in live + assert ("POST", "/api/compare/record") in live + assert ("POST", "/api/compare/{comp_id}/vote") in live + assert ("DELETE", "/api/compare/{comp_id}") in live + for path in ( + "/api/compare/history", + "/api/compare/record", + "/api/compare/{comp_id}/vote", + "/api/compare/{comp_id}", + ): + assert path in guide + + +def test_app_api_guide_does_not_claim_json_bridge_can_start_compare(): + from src.agent_loop import TOOL_SECTIONS + + guide = TOOL_SECTIONS["app_api"] + + assert "`/api/compare/start` is a browser multipart/session-creation flow" in guide + assert "use the Compare UI" in guide + + +def test_app_api_shaped_json_vote_reaches_compare_handler(monkeypatch): + import routes.compare.compare_routes as compare_routes + + comparison = SimpleNamespace( + id="cmp-1", + owner=None, + winner=None, + blind_mapping=json.dumps({"left": "b", "right": "a"}), + model_a="model-a", + model_b="model-b", + voted_at=None, + ) + + class _Query: + def filter(self, *_args): + return self + + def first(self): + return comparison + + class _DB: + committed = False + closed = False + + def query(self, _model): + return _Query() + + def commit(self): + self.committed = True + + def close(self): + self.closed = True + + db = _DB() + monkeypatch.setattr(compare_routes, "SessionLocal", lambda: db) + monkeypatch.setattr(compare_routes, "get_current_user", lambda _request: None) + + app = FastAPI() + app.include_router(compare_routes.setup_compare_routes(SimpleNamespace())) + bridge_call = { + "action": "call", + "method": "POST", + "path": "/api/compare/cmp-1/vote", + "body": {"winner": "left"}, + } + response = TestClient(app).request( + bridge_call["method"], bridge_call["path"], json=bridge_call["body"] + ) + + assert response.status_code == 200 + assert response.json() == { + "winner": "b", + "model_a": "model-a", + "model_b": "model-b", + "revealed": {"left": "model-b", "right": "model-a"}, + } + assert comparison.winner == "b" + assert db.committed is True + assert db.closed is True + + comparison.winner = None + db.committed = False + db.closed = False + form_response = TestClient(app).post( + "/api/compare/cmp-1/vote", data={"winner": "right"} + ) + assert form_response.status_code == 200 + assert form_response.json()["winner"] == "a" + assert db.committed is True + assert db.closed is True