fix(api): fail-closed disagreeing chat mode aliases #670
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,6 +183,47 @@ def _validate_mode(mode: Any) -> str: | |
| return mode | ||
|
|
||
|
|
||
| def _resolve_requested_chat_mode(body: dict[str, Any]) -> str: | ||
| """Resolve ``orchestration`` / ``orchestration_mode`` / ``mode`` without first-wins hide. | ||
|
|
||
| Each present alias is validated on its own. JSON ``null`` and ``""`` are | ||
| omit-equivalent so SDK optional defaults stay no-ops. Whitespace-only | ||
| values fail closed through ``_validate_mode``. Distinct non-omit aliases | ||
| fail closed so ``orchestration=route`` cannot hide ``mode=conduct`` and | ||
| bill a Fugu-style single-worker route for a Conductor workflow the buyer | ||
| asked for (Nielsen et al., 2025; Xu et al., 2025). When every alias is | ||
| omitted, the result is ``auto``. | ||
|
|
||
| Args: | ||
| body: Chat Completions JSON object after unknown-key rejection. | ||
|
|
||
| Returns: | ||
| One of ``auto``, ``route``, or ``conduct``. | ||
|
|
||
| Raises: | ||
| RequestError: invalid, whitespace-only, or disagreeing aliases. | ||
| """ | ||
| resolved_modes: list[str] = [] | ||
| for key in ("orchestration", "orchestration_mode", "mode"): | ||
| if key not in body: | ||
| continue | ||
| raw_mode = body.get(key) | ||
| if raw_mode is None or raw_mode == "": | ||
| continue | ||
| resolved_modes.append(_validate_mode(raw_mode)) | ||
| if not resolved_modes: | ||
| return "auto" | ||
| unique_modes = set(resolved_modes) | ||
| if len(unique_modes) > 1: | ||
| raise RequestError( | ||
| 400, | ||
| "invalid_mode", | ||
| "orchestration, orchestration_mode, and mode must agree; " | ||
| "omit unused aliases or send the same value on each", | ||
| ) | ||
| return resolved_modes[0] | ||
|
|
||
|
|
||
| def _validate_messages(messages: Any) -> list[dict[str, str]]: | ||
| if not isinstance(messages, list) or not messages: | ||
| raise RequestError(400, "invalid_message", "messages must be a non-empty array") | ||
|
|
@@ -732,7 +773,7 @@ def do_POST(self) -> None: # noqa: N802 | |
| self._send(proxied) | ||
| return | ||
| messages = _validate_messages(body.get("messages")) | ||
| mode = _resolve_requested_chat_mode(body) | ||
|
There was a problem hiding this comment. This is the no-tools path only. Next action: leave #647 / #640 as the passthrough landing. If README / |
||
| include_trace = bool(body.get("include_orchestration_trace", security.expose_trace_by_default)) | ||
| stream = body.get("stream", False) | ||
| if not isinstance(stream, bool): | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,191 @@ | ||
| """Live HTTP: mixed mode aliases must not hide a Conductor workflow. | ||
|
|
||
| A buyer who sends ``orchestration=route`` plus ``mode=conduct`` asked for a | ||
| Conductor workflow (Nielsen et al., 2025). The first-wins ``or`` chain billed a | ||
| Fugu-style single-worker route instead. Each alias is checked on its own. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import json | ||
| import threading | ||
| import urllib.error | ||
| import urllib.request | ||
| from pathlib import Path | ||
| import sys | ||
|
|
||
| sys.path.insert(0, str(Path(__file__).resolve().parents[1])) | ||
|
|
||
| from contextual_orchestrator import ModelAgent, TaskOrchestrator # noqa: E402 | ||
| from contextual_orchestrator.server import SecurityConfig, build_server # noqa: E402 | ||
|
|
||
| _TEST_AUTH_TOKEN = "chat_orchestration_mode_token" # noqa: S105 | ||
|
|
||
|
|
||
| def build() -> TaskOrchestrator: | ||
| return TaskOrchestrator( | ||
| [ | ||
| ModelAgent("planner_agent", "mock-planner", tags=("planning", "reasoning")), | ||
| ModelAgent("builder_agent", "mock-builder", tags=("coding", "writing")), | ||
| ModelAgent("reviewer_agent", "mock-reviewer", tags=("verification", "review")), | ||
| ] | ||
| ) | ||
|
|
||
|
|
||
| def _post(port: int, payload: dict) -> tuple[int, dict]: | ||
| request = urllib.request.Request( | ||
| f"http://127.0.0.1:{port}/v1/chat/completions", | ||
| data=json.dumps(payload).encode("utf-8"), | ||
| headers={ | ||
| "content-type": "application/json", | ||
| "authorization": f"Bearer {_TEST_AUTH_TOKEN}", | ||
| "connection": "close", | ||
| }, | ||
| method="POST", | ||
| ) | ||
| try: | ||
| with urllib.request.urlopen(request, timeout=15) as response: | ||
| return response.status, json.loads(response.read().decode("utf-8")) | ||
| except urllib.error.HTTPError as exc: | ||
| return exc.code, json.loads(exc.read().decode("utf-8")) | ||
|
|
||
|
|
||
| def _server() -> tuple[object, threading.Thread, int]: | ||
| server = build_server(build(), port=0, security=SecurityConfig(auth_token=_TEST_AUTH_TOKEN)) | ||
| thread = threading.Thread(target=server.serve_forever, daemon=True) | ||
| thread.start() | ||
| return server, thread, server.server_address[1] | ||
|
|
||
|
|
||
| def test_http_chat_rejects_mixed_route_and_conduct_aliases() -> None: | ||
|
There was a problem hiding this comment. This locks Next action: add one HTTP case with |
||
| """``orchestration=route`` must not hide ``mode=conduct`` on the chat path.""" | ||
| server, thread, port = _server() | ||
| try: | ||
| status, body = _post( | ||
| port, | ||
| { | ||
| "model": "mock-planner", | ||
| "messages": [{"role": "user", "content": "analyze, implement, and verify the invoice parser"}], | ||
| "orchestration": "route", | ||
| "mode": "conduct", | ||
| }, | ||
| ) | ||
| assert status == 400, body | ||
| assert body["error"]["code"] == "invalid_mode" | ||
| assert "agree" in body["error"]["message"] | ||
| assert "choices" not in body | ||
| finally: | ||
| server.shutdown() | ||
| thread.join(timeout=5) | ||
|
|
||
|
|
||
| def test_http_chat_rejects_mixed_route_and_whitespace_mode() -> None: | ||
| """``orchestration=route`` must not hide whitespace-only ``mode``.""" | ||
| server, thread, port = _server() | ||
| try: | ||
| status, body = _post( | ||
| port, | ||
| { | ||
| "model": "mock-planner", | ||
| "messages": [{"role": "user", "content": "say hi"}], | ||
| "orchestration": "route", | ||
| "mode": " ", | ||
| }, | ||
| ) | ||
| assert status == 400, body | ||
| assert body["error"]["code"] == "invalid_mode" | ||
| assert "choices" not in body | ||
| finally: | ||
| server.shutdown() | ||
| thread.join(timeout=5) | ||
|
|
||
|
|
||
| def test_http_chat_accepts_agreeing_route_aliases() -> None: | ||
| """The same value on two aliases is an honest no-op, not a conflict.""" | ||
| server, thread, port = _server() | ||
| try: | ||
| status, body = _post( | ||
| port, | ||
| { | ||
| "model": "mock-planner", | ||
| "messages": [{"role": "user", "content": "say hi"}], | ||
| "orchestration": "route", | ||
| "mode": "route", | ||
| }, | ||
| ) | ||
| assert status == 200, body | ||
| assert body["orchestration"]["mode"] == "route" | ||
| assert "choices" in body | ||
| finally: | ||
| server.shutdown() | ||
| thread.join(timeout=5) | ||
|
|
||
|
|
||
| def test_http_chat_accepts_empty_string_mode_as_omit() -> None: | ||
| """JSON empty-string mode stays omit-equivalent; spaces do not.""" | ||
| server, thread, port = _server() | ||
| try: | ||
| status, body = _post( | ||
| port, | ||
| { | ||
| "model": "mock-planner", | ||
| "messages": [{"role": "user", "content": "say hi"}], | ||
| "orchestration": "route", | ||
| "mode": "", | ||
| }, | ||
| ) | ||
| assert status == 200, body | ||
| assert body["orchestration"]["mode"] == "route" | ||
| assert "choices" in body | ||
| finally: | ||
| server.shutdown() | ||
| thread.join(timeout=5) | ||
|
|
||
|
|
||
| def test_http_chat_accepts_null_mode_as_omit() -> None: | ||
| """JSON null mode stays omit-equivalent.""" | ||
| server, thread, port = _server() | ||
| try: | ||
| status, body = _post( | ||
| port, | ||
| { | ||
| "model": "mock-planner", | ||
| "messages": [{"role": "user", "content": "say hi"}], | ||
| "mode": None, | ||
| }, | ||
| ) | ||
| assert status == 200, body | ||
| assert "choices" in body | ||
| finally: | ||
| server.shutdown() | ||
| thread.join(timeout=5) | ||
|
|
||
|
|
||
| def test_http_chat_accepts_mode_conduct() -> None: | ||
| """A single ``mode=conduct`` still runs the Conductor workflow.""" | ||
| server, thread, port = _server() | ||
| try: | ||
| status, body = _post( | ||
| port, | ||
| { | ||
| "model": "mock-planner", | ||
| "messages": [{"role": "user", "content": "analyze, implement, and verify the invoice parser"}], | ||
| "mode": "conduct", | ||
| }, | ||
| ) | ||
| assert status == 200, body | ||
| assert body["orchestration"]["mode"] == "conduct" | ||
| assert "choices" in body | ||
| finally: | ||
| server.shutdown() | ||
| thread.join(timeout=5) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| test_http_chat_rejects_mixed_route_and_conduct_aliases() | ||
| test_http_chat_rejects_mixed_route_and_whitespace_mode() | ||
| test_http_chat_accepts_agreeing_route_aliases() | ||
| test_http_chat_accepts_empty_string_mode_as_omit() | ||
| test_http_chat_accepts_null_mode_as_omit() | ||
| test_http_chat_accepts_mode_conduct() | ||
| print("ok") | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nulland""omit is right for SDK optional defaults. Whitespace-only correctly fails in_validate_mode(not inALLOWED_MODES). Keep that split; do notstrip()into omit, ororchestration=routeplusmode=" "becomes a silent route again.