Skip to content

Commit 8e284a9

Browse files
committed
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 <jasonbuildai@gmail.com>
1 parent a54c030 commit 8e284a9

2 files changed

Lines changed: 48 additions & 3 deletions

File tree

‎loopx/chat_runtime.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -992,8 +992,8 @@ def _ensure_adapter_locked(
992992
codex_home_override=codex_profile_home(self.codex_home, session.get("codex_chat_configuration") or {}),
993993
)
994994
try:
995-
observed_settings = self._observed_model_settings(adapter, session.get("requested_agent_reasoning_effort"),
996-
session.get("requested_agent_model_id"))
995+
# Recovery reads back the provider-restored configuration.
996+
observed_settings = self._observed_model_settings(adapter)
997997
if session.get("upstream_mode") == CODEX_GOAL_CHAT_MODE:
998998
if not isinstance(adapter, CodexAppServerAdapter):
999999
raise CodexChatAgentError("Native Goal continuation requires a Codex adapter.", error_code="native_goal_adapter_mismatch", gate=None)

‎tests/test_chat_reasoning_selection.py‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,12 @@ def options():
5050
request = json.loads(line)
5151
with capture.open("a") as out: out.write(json.dumps(request)+"\n")
5252
method = request.get("method")
53-
if method == "initialize": result = {"protocolVersion": 1, "agentCapabilities": {}}
53+
if method == "initialize": result = {"protocolVersion": 1, "agentCapabilities": {"loadSession": True}}
5454
elif method == "session/new": result = {"sessionId": "fixture-session", "configOptions": options()}
55+
elif method == "session/load":
56+
# Recovery restores the provider's own configuration, not the request.
57+
selected.update({"model": "first", "depth": "low"})
58+
result = {"configOptions": options()}
5559
elif method == "session/set_config_option":
5660
if behavior != "ignore" or request["params"]["configId"] != "depth":
5761
selected[request["params"]["configId"]] = request["params"]["value"]
@@ -181,6 +185,47 @@ def test_registered_acp_discovery_and_selection_share_native_configuration(tmp_p
181185
runtime.close()
182186

183187

188+
def test_acp_restart_records_provider_restored_settings_instead_of_failing(tmp_path):
189+
"""Recovery reports the provider's configuration; that is not a failed request."""
190+
script, capture = tmp_path / "agent.py", tmp_path / "capture.jsonl"
191+
script.write_text(ACP_CONFIG)
192+
store = ChatSessionStore(tmp_path / "runtime")
193+
endpoints = AgentEndpointRegistry(store.root)
194+
endpoints.upsert({"agent_id": "fixture-acp", "display_name": "Fixture ACP", "location": "local",
195+
"command": [sys.executable, str(script), str(capture), "apply"]})
196+
first = ChatRuntimeController(store=store, codex_bin="unavailable-fixture-codex",
197+
endpoint_registry=endpoints)
198+
try:
199+
session, was_resumed = first.open_session(
200+
goal_id="fixture-restart", agent_id="fixture-acp", work_dir=tmp_path,
201+
objective="Resume after a Chat restart.", mode="new",
202+
agent_model_id="second", agent_reasoning_effort="high")
203+
assert was_resumed is False
204+
assert session["agent_model_id"] == "second"
205+
assert session["requested_agent_model_id"] == "second"
206+
assert session["agent_reasoning_effort"] == "high"
207+
finally:
208+
first.close()
209+
210+
second = ChatRuntimeController(store=store, codex_bin="unavailable-fixture-codex",
211+
endpoint_registry=endpoints)
212+
try:
213+
restored, was_resumed = second.open_session(
214+
goal_id="fixture-restart", agent_id="fixture-acp", work_dir=tmp_path,
215+
objective="Resume after a Chat restart.", mode="resume_latest")
216+
assert was_resumed is True
217+
assert restored["session_id"] == session["session_id"]
218+
# The ACP provider restores its own values. Record that readback instead
219+
# of rejecting a valid recovery for not echoing the original request.
220+
assert restored["agent_model_id"] == "first"
221+
assert restored["agent_reasoning_effort"] == "low"
222+
assert restored["requested_agent_model_id"] == "second"
223+
assert any(json.loads(line)["method"] == "session/load"
224+
for line in capture.read_text().splitlines())
225+
finally:
226+
second.close()
227+
228+
184229
def test_codex_catalog_failure_is_retryable_without_inventing_current_model(tmp_path, monkeypatch):
185230
runtime = ChatRuntimeController(store=ChatSessionStore(tmp_path / "runtime"), codex_bin="fixture")
186231
monkeypatch.setattr(runtime, "capabilities", lambda: [{"agent_id": "codex", "available": True}])

0 commit comments

Comments
 (0)