fix(server): bypass agent on /v1/chat/completions when caller passes tools (#454)

Closes #414.

Root cause: routes.py:151 unconditionally routed non-streaming /v1/chat/completions through _handle_agent when an agent was registered. _handle_agent calls agent.run(input_text) which IGNORES request_body.tools entirely, runs the agent's own internal tool loop with its own (different) tool spec, and returns only result.content — never result.tool_calls. The "Understood. If you have another request..." filler is not hardcoded anywhere in OpenJarvis (the cloud_router.py:126 "Understood." is a different Gemini-only injection). It's the model's actual generic response when the agent re-prompts it without the user's intended tools.

Fix: one conditional. Skip _handle_agent when request_body.tools is present — the client is asking for raw OpenAI-compat function-calling, so route to _handle_direct which preserves tool_calls. Plus a forward-looking comment documenting this as an intentional trade-off so a future maintainer doesn't naively remove the guard.

Streaming path left intact (its asymmetry — "use agent_stream WHEN tools present" — is intentional per the existing comment at lines 143-145; reporter's repro is non-streaming).

Two regression tests:
- test_with_tools_bypasses_agent: mocks engine+agent, asserts tool_calls survives, agent.run is NOT called.
- test_without_tools_still_uses_agent: pins existing behavior for the no-tools path.

Reported by @gilbert-barajas — the side-by-side curl repro made the triage tractable.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

src/openjarvis/server/routes.py

@@ -147,8 +147,23 @@ async def chat_completions(request_body: ChatCompletionRequest, request: Request
             return await _handle_agent_stream(agent, bus, model, request_body)
         return await _handle_stream(engine, model, request_body, complexity_info)
 
-    # Non-streaming: use agent if available, otherwise direct engine call
-    if agent is not None:
+    # Non-streaming: use agent if available, otherwise direct engine call.
+    #
+    # EXCEPTION: when the client explicitly passed `tools`, they're asking
+    # for raw OpenAI-compat function-calling — return the model's
+    # tool_call decision verbatim. Routing through `_handle_agent` would
+    # call `agent.run(input_text)`, which IGNORES `request_body.tools`,
+    # runs the agent's own internal tool loop with its own (different)
+    # tool spec, and returns only `result.content` — so the model's
+    # tool_calls vanish and the user sees a generic acknowledgement
+    # (e.g. "Understood. If you have another request...") that the
+    # agent's re-prompted LLM produced. See #414.
+    #
+    # If a future caller needs agent orchestration WITH client-supplied
+    # tools (e.g. injecting MCP tools through this endpoint and wanting
+    # the agent to execute them), add an explicit opt-in header rather
+    # than removing this guard — silent re-routing is what produced #414.
+    if agent is not None and not request_body.tools:
         return _handle_agent(agent, model, request_body, complexity_info)
 
     bus = getattr(request.app.state, "bus", None)

tests/server/test_routes.py

@@ -171,6 +171,85 @@ class TestChatCompletions:
         data = resp.json()
         assert data["choices"][0]["message"]["content"] == "Hello from agent"
 
+    def test_with_tools_bypasses_agent(self):
+        """Regression for #414.
+
+        When the client passes explicit `tools` AND an agent is
+        registered, the request must go to `_handle_direct` (which
+        preserves tool_calls from the engine) rather than `_handle_agent`
+        (which calls `agent.run()` ignoring `request_body.tools` and
+        returns only `result.content`, dropping tool_calls and
+        substituting whatever generic content the agent's re-prompted
+        LLM produced).
+        """
+        engine = _make_engine()
+        engine.generate.return_value = {
+            "content": "",
+            "tool_calls": [
+                {"id": "c1", "name": "list_files", "arguments": '{"directory":"/tmp"}'},
+            ],
+            "usage": {"prompt_tokens": 5, "completion_tokens": 3, "total_tokens": 8},
+            "model": "test-model",
+            "finish_reason": "tool_calls",
+        }
+        agent = _make_agent(content="GENERIC AGENT FILLER")
+        app = create_app(engine, "test-model", agent=agent)
+        client = TestClient(app)
+
+        resp = client.post(
+            "/v1/chat/completions",
+            json={
+                "model": "test-model",
+                "messages": [{"role": "user", "content": "Use list_files on /tmp."}],
+                "tools": [
+                    {
+                        "type": "function",
+                        "function": {
+                            "name": "list_files",
+                            "parameters": {
+                                "type": "object",
+                                "properties": {"directory": {"type": "string"}},
+                                "required": ["directory"],
+                            },
+                        },
+                    },
+                ],
+            },
+        )
+        assert resp.status_code == 200
+        msg = resp.json()["choices"][0]["message"]
+        # The engine's tool_calls must survive — proves we bypassed
+        # _handle_agent and reached _handle_direct.
+        assert msg["tool_calls"] is not None
+        assert msg["tool_calls"][0]["function"]["name"] == "list_files"
+        # Content must be the engine's empty string, NOT the agent's
+        # filler. If this assertion fails, the agent ran and produced
+        # filler content while dropping the real tool_calls — exactly
+        # the bug #414 reported.
+        assert msg["content"] == ""
+        assert "GENERIC AGENT FILLER" not in (msg["content"] or "")
+        # And the engine was actually called (proves we hit _handle_direct
+        # rather than short-circuiting somewhere else).
+        assert engine.generate.called
+        # And the agent was NOT called (proves the bypass worked).
+        assert not agent.run.called
+
+    def test_without_tools_still_uses_agent(self, client_with_agent):
+        """Counterpart to test_with_tools_bypasses_agent: when no tools
+        are requested, the agent path is still used (preserves existing
+        behavior for plain chat through an agent)."""
+        resp = client_with_agent.post(
+            "/v1/chat/completions",
+            json={
+                "model": "test-model",
+                "messages": [{"role": "user", "content": "Hello"}],
+            },
+        )
+        assert resp.status_code == 200
+        data = resp.json()
+        # No tools → agent path → agent's content surfaces.
+        assert data["choices"][0]["message"]["content"] == "Hello from agent"
+
     def test_agent_with_conversation(self, client_with_agent):
         resp = client_with_agent.post(
             "/v1/chat/completions",