diff --git a/loopx/chat_agent.py b/loopx/chat_agent.py index e66ed6924d..83d9c3dc45 100644 --- a/loopx/chat_agent.py +++ b/loopx/chat_agent.py @@ -985,6 +985,7 @@ def send( if on_event: on_event("turn.started", {"upstream_turn_id": turn_id}) parts: list[str] = [] + completed_structured_response: str | None = None display_filter = VisibleResponseStreamFilter(protected_paths=[self.work_dir]) visible_delta_count = 0 started_at = time.monotonic() @@ -1069,6 +1070,18 @@ def send( on_event("answer.delta", {"text": visible}) elif method == "item/completed": item_text = _agent_item_text(message) + if output_schema is not None and isinstance(params, dict): + item = params.get("item") + if isinstance(item, dict) and item.get("type") == "agentMessage": + # Completed items are authoritative. A structured Turn + # may stream commentary before its final JSON; joining + # all deltas would turn that valid answer into invalid + # JSON (or promote commentary JSON as the result). + phase = item.get("phase") + if phase is None or phase == "final_answer": + completed_structured_response = item_text + elif phase != "commentary" or completed_structured_response is None: + completed_structured_response = "" if item_text and not parts: parts.append(item_text) visible = display_filter.feed(item_text) @@ -1113,6 +1126,8 @@ def send( on_event("answer.delta", {"text": visible_tail}) raw_response = "".join(parts) if output_schema is not None: + if completed_structured_response is not None: + raw_response = completed_structured_response try: result = json.loads(raw_response) except (ValueError, TypeError) as exc: diff --git a/loopx/control_plane/work_items/operation_agent_handoff.ts b/loopx/control_plane/work_items/operation_agent_handoff.ts index 5814501af5..0845ca29ec 100644 --- a/loopx/control_plane/work_items/operation_agent_handoff.ts +++ b/loopx/control_plane/work_items/operation_agent_handoff.ts @@ -28,12 +28,15 @@ function timestamp(value: unknown): number { } /** Readback of an operator-selected transport. This is not a session binding, - * a runtime qualification or an execution permit. Python supplies argv facts. */ + * a runtime qualification or an execution permit. Python supplies argv facts. + * Accept the CLI's provider-neutral effort vocabulary; actual model support + * remains a host qualification, not something this preflight can establish. */ export function projectManagedOperationTransport(input: JsonObject): JsonObject { const reason = input.host !== "codex-cli" ? "operation_transport_host_unsupported" : !["read-only", "workspace-write"].includes(String(input.sandbox)) ? "operation_transport_sandbox_unsupported" : typeof input.model !== "string" || !ID.test(input.model) - || !["minimal", "low", "medium", "high", "xhigh"].includes(String(input.reasoning_effort)) + || typeof input.reasoning_effort !== "string" + || !["none", "minimal", "low", "medium", "high", "xhigh", "max", "ultra"].includes(input.reasoning_effort) ? "operation_transport_profile_required" : null; return {reason, transport: {schema_version: "loopx_operation_transport_v0", kind: "owned_app_server", revision: "app-server-operation-tools-v0", diff --git a/tests/control_plane_ts/operation_agent_handoff.test.ts b/tests/control_plane_ts/operation_agent_handoff.test.ts index f904f4f180..bd65cce56c 100644 --- a/tests/control_plane_ts/operation_agent_handoff.test.ts +++ b/tests/control_plane_ts/operation_agent_handoff.test.ts @@ -36,6 +36,28 @@ test("operator transport preflight projects pinned configuration without claimin } }); +test("operation transport accepts the explicit CLI effort vocabulary without qualifying a model", () => { + for (const reasoning_effort of ["none", "minimal", "low", "medium", "high", "xhigh", "max", "ultra"]) { + for (const sandbox of ["read-only", "workspace-write"]) { + const projected = projectManagedOperationTransport({host: "codex-cli", sandbox, + model: "test-model", reasoning_effort}); + assert.equal(projected.reason, null, reasoning_effort); + const transport = projected.transport as JsonObject; + assert.equal(transport.configuration_valid, true); + assert.equal(transport.runtime_qualified, false); + assert.equal(transport.human_confirmation_required, true); + assert.equal(transport.first_consumption_required, true); + assert.equal(transport.source_conversation_is_executor, false); + } + } + for (const reasoning_effort of [null, "", "unknown", "MAX", 1, ["max"]]) { + const projected = projectManagedOperationTransport({host: "codex-cli", sandbox: "read-only", + model: "test-model", reasoning_effort}); + assert.equal(projected.reason, "operation_transport_profile_required"); + assert.equal((projected.transport as JsonObject).configuration_valid, false); + } +}); + function managedInput(): JsonObject { const value = input(); const parameters = (value.proposal as JsonObject).normalized_parameters as JsonObject; diff --git a/tests/test_chat_agent.py b/tests/test_chat_agent.py index 31cbecdc2a..db4989261c 100644 --- a/tests/test_chat_agent.py +++ b/tests/test_chat_agent.py @@ -227,6 +227,50 @@ def popen(command, **kwargs): session.close() +@pytest.mark.parametrize("commentary", ["I will read the context.", '{"answer":"not-final"}']) +@pytest.mark.parametrize("final_phase", [None, "final_answer"]) +def test_structured_turn_uses_completed_answer_not_commentary_or_partial_deltas( + monkeypatch, tmp_path, commentary, final_phase, +): + session = chat_agent.CodexChatAgentSession( + process=_FakeAppServerProcess(), messages=queue.Queue(), thread_id="thread-fixture", + work_dir=tmp_path, execution_mode=True, + ) + params = {"threadId": "thread-fixture", "turnId": "turn-fixture"} + final = {"type": "agentMessage", "text": '{"answer":"actual"}'} + if final_phase is not None: + final["phase"] = final_phase + upstream = iter([ + {"method": "item/agentMessage/delta", "params": {**params, "delta": commentary}}, + {"method": "item/completed", "params": {**params, "item": { + "type": "agentMessage", "phase": "commentary", "text": commentary}}}, + {"method": "item/agentMessage/delta", "params": {**params, "delta": '{"answer":'}}, + {"method": "item/completed", "params": {**params, "item": final}}, + {"method": "turn/completed", "params": {"turn": {"status": "completed"}}}, + ]) + monkeypatch.setattr(session, "_request", lambda *a, **kw: {"turn": {"id": "turn-fixture"}}) + monkeypatch.setattr(session, "_next_event", lambda **kw: next(upstream)) + assert session.send("Return the structured result.", output_schema={"type": "object"}) == {"answer": "actual"} + + +@pytest.mark.parametrize("phase", ["commentary", "futurePhase", []]) +def test_structured_turn_cannot_promote_nonfinal_json_to_a_final_result(monkeypatch, tmp_path, phase): + session = chat_agent.CodexChatAgentSession( + process=_FakeAppServerProcess(), messages=queue.Queue(), thread_id="thread-fixture", + work_dir=tmp_path, execution_mode=True, + ) + upstream = iter([ + {"method": "item/agentMessage/delta", "params": {"delta": '{"answer":"not-final"}'}}, + {"method": "item/completed", "params": {"item": { + "type": "agentMessage", "phase": phase, "text": '{"answer":"not-final"}'}}}, + {"method": "turn/completed", "params": {"turn": {"status": "completed"}}}, + ]) + monkeypatch.setattr(session, "_request", lambda *a, **kw: {"turn": {"id": "turn-fixture"}}) + monkeypatch.setattr(session, "_next_event", lambda **kw: next(upstream)) + with pytest.raises(chat_agent.CodexChatAgentError, match="structured output"): + session.send("Return the structured result.", output_schema={"type": "object"}) + + def test_trusted_manager_profile_reaches_app_server_and_turn_prompt( monkeypatch, tmp_path, diff --git a/tests/test_codex_operation_host.py b/tests/test_codex_operation_host.py index 0903ca0a2c..7256e3710f 100644 --- a/tests/test_codex_operation_host.py +++ b/tests/test_codex_operation_host.py @@ -217,8 +217,11 @@ def test_owned_operation_host_reaps_descendants_on_success_and_timeout( not os.environ.get("LOOPX_QUALIFY_CODEX_OPERATION_HOST"), reason="explicit live-host release qualification only", ) +@pytest.mark.parametrize( + ("model", "effort"), [("gpt-6-sol", "xhigh"), ("gpt-6-luna", "max")] +) def test_live_owned_app_server_native_tool_metadata( - tmp_path: Path, monkeypatch: pytest.MonkeyPatch + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, model: str, effort: str ) -> None: from loopx.control_plane.turn_driver import codex_operation_host @@ -247,8 +250,8 @@ def record(tool, arguments, native): runtime_root=tmp_path / "runtime", registry_path=tmp_path / "registry.json", project=tmp_path, - model="gpt-6-sol", - reasoning_effort="xhigh", + model=model, + reasoning_effort=effort, timeout_seconds=120, ) result = run_codex_operation_host(request, **options) diff --git a/tests/test_turn_managed_executor_binding.py b/tests/test_turn_managed_executor_binding.py index b735c17d27..836de2fe7f 100644 --- a/tests/test_turn_managed_executor_binding.py +++ b/tests/test_turn_managed_executor_binding.py @@ -6,6 +6,8 @@ import pytest +from loopx.reasoning_effort import REASONING_EFFORTS + from loopx.control_plane.turn_driver.host_binding import ( DEFAULT_DSH_OUTPUT_TOKEN_LIMIT, DSH_OUTPUT_TOKEN_BUDGET_SCHEMA_VERSION, @@ -108,17 +110,19 @@ def test_trusted_codex_binding_projects_its_independent_agent_profile(): assert "unrelated-managed-credential" not in json.dumps(binding) -def test_owned_operation_transport_is_opt_in_and_read_back_by_the_existing_host_owner(): +@pytest.mark.parametrize("effort", REASONING_EFFORTS) +def test_owned_operation_transport_is_opt_in_and_read_back_by_the_existing_host_owner(effort): options = ["--host", "codex-cli", "--codex-operation-tools"] blocked = managed_executor_binding_from_host_args(options, environ={}) assert blocked["available"] is False assert blocked["unavailable_reason"] == "operation_transport_profile_required" pinned = managed_executor_binding_from_host_args( - [*options, "--codex-model", "test-model", "--codex-reasoning-effort", "xhigh"], + [*options, "--codex-model", "test-model", "--codex-reasoning-effort", effort], environ={}, ) assert pinned["available"] is None # argv cannot qualify a real host - assert pinned["execution_profile"] == "test-model@xhigh" + assert pinned["execution_profile"] == f"test-model@{effort}" + assert pinned["operation_transport"]["configuration_valid"] is True assert pinned["operation_transport"]["identity_source"] == "native_thread_turn_metadata" assert pinned["operation_transport"]["runtime_qualified"] is False assert "operation_transport" not in managed_executor_binding("codex-cli")