From 8e284a906619f643f82f56d22d21822a502d5641 Mon Sep 17 00:00:00 2001 From: JasonBuildAI Date: Sun, 18 Oct 2026 15:52:56 +0800 Subject: [PATCH] fix(chat): keep ACP session recovery from failing on restored settings Recovery observes the configuration a provider restores for an existing session; it does not apply a new request. ACP skips the reapply entirely and some providers omit configOptions from session/load, so the restored model or reasoning effort can legitimately differ from the request that originally opened the session. The resume path nevertheless passed the stored request to _observed_model_settings as a required confirmation. Any ACP conversation opened with a non-default model or effort then failed to reopen after a Chat restart: the controller marked the session resume_failed and rendered "The previous Agent conversation could not be restored." This also turned the daily full-public loopx-chat-acp-adapter-smoke red. Record the restored identity on recovery instead, while the creation path keeps the confirmation check for the value it just applied. The original request stays recorded as requested_agent_model_id and requested_agent_reasoning_effort provenance. Signed-off-by: JasonBuildAI --- loopx/chat_runtime.py | 4 +-- tests/test_chat_reasoning_selection.py | 47 +++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/loopx/chat_runtime.py b/loopx/chat_runtime.py index 354fe7fa98..0443973048 100644 --- a/loopx/chat_runtime.py +++ b/loopx/chat_runtime.py @@ -992,8 +992,8 @@ def _ensure_adapter_locked( codex_home_override=codex_profile_home(self.codex_home, session.get("codex_chat_configuration") or {}), ) try: - observed_settings = self._observed_model_settings(adapter, session.get("requested_agent_reasoning_effort"), - session.get("requested_agent_model_id")) + # Recovery reads back the provider-restored configuration. + observed_settings = self._observed_model_settings(adapter) if session.get("upstream_mode") == CODEX_GOAL_CHAT_MODE: if not isinstance(adapter, CodexAppServerAdapter): raise CodexChatAgentError("Native Goal continuation requires a Codex adapter.", error_code="native_goal_adapter_mismatch", gate=None) diff --git a/tests/test_chat_reasoning_selection.py b/tests/test_chat_reasoning_selection.py index ceb7e71337..760f62fbd7 100644 --- a/tests/test_chat_reasoning_selection.py +++ b/tests/test_chat_reasoning_selection.py @@ -50,8 +50,12 @@ def options(): request = json.loads(line) with capture.open("a") as out: out.write(json.dumps(request)+"\n") method = request.get("method") - if method == "initialize": result = {"protocolVersion": 1, "agentCapabilities": {}} + if method == "initialize": result = {"protocolVersion": 1, "agentCapabilities": {"loadSession": True}} elif method == "session/new": result = {"sessionId": "fixture-session", "configOptions": options()} + elif method == "session/load": + # Recovery restores the provider's own configuration, not the request. + selected.update({"model": "first", "depth": "low"}) + result = {"configOptions": options()} elif method == "session/set_config_option": if behavior != "ignore" or request["params"]["configId"] != "depth": selected[request["params"]["configId"]] = request["params"]["value"] @@ -181,6 +185,47 @@ def test_registered_acp_discovery_and_selection_share_native_configuration(tmp_p runtime.close() +def test_acp_restart_records_provider_restored_settings_instead_of_failing(tmp_path): + """Recovery reports the provider's configuration; that is not a failed request.""" + script, capture = tmp_path / "agent.py", tmp_path / "capture.jsonl" + script.write_text(ACP_CONFIG) + store = ChatSessionStore(tmp_path / "runtime") + endpoints = AgentEndpointRegistry(store.root) + endpoints.upsert({"agent_id": "fixture-acp", "display_name": "Fixture ACP", "location": "local", + "command": [sys.executable, str(script), str(capture), "apply"]}) + first = ChatRuntimeController(store=store, codex_bin="unavailable-fixture-codex", + endpoint_registry=endpoints) + try: + session, was_resumed = first.open_session( + goal_id="fixture-restart", agent_id="fixture-acp", work_dir=tmp_path, + objective="Resume after a Chat restart.", mode="new", + agent_model_id="second", agent_reasoning_effort="high") + assert was_resumed is False + assert session["agent_model_id"] == "second" + assert session["requested_agent_model_id"] == "second" + assert session["agent_reasoning_effort"] == "high" + finally: + first.close() + + second = ChatRuntimeController(store=store, codex_bin="unavailable-fixture-codex", + endpoint_registry=endpoints) + try: + restored, was_resumed = second.open_session( + goal_id="fixture-restart", agent_id="fixture-acp", work_dir=tmp_path, + objective="Resume after a Chat restart.", mode="resume_latest") + assert was_resumed is True + assert restored["session_id"] == session["session_id"] + # The ACP provider restores its own values. Record that readback instead + # of rejecting a valid recovery for not echoing the original request. + assert restored["agent_model_id"] == "first" + assert restored["agent_reasoning_effort"] == "low" + assert restored["requested_agent_model_id"] == "second" + assert any(json.loads(line)["method"] == "session/load" + for line in capture.read_text().splitlines()) + finally: + second.close() + + def test_codex_catalog_failure_is_retryable_without_inventing_current_model(tmp_path, monkeypatch): runtime = ChatRuntimeController(store=ChatSessionStore(tmp_path / "runtime"), codex_bin="fixture") monkeypatch.setattr(runtime, "capabilities", lambda: [{"agent_id": "codex", "available": True}])