diff --git a/acp_adapter/session.py b/acp_adapter/session.py index d6dace66b4e..61d06432a71 100644 --- a/acp_adapter/session.py +++ b/acp_adapter/session.py @@ -466,17 +466,10 @@ def _persist(self, state: SessionState) -> None: except Exception: logger.debug("Failed to update ACP session metadata", exc_info=True) - # Replace stored messages with current history. - db.clear_messages(state.session_id) - for msg in state.history: - db.append_message( - session_id=state.session_id, - role=msg.get("role", "user"), - content=msg.get("content"), - tool_name=msg.get("tool_name") or msg.get("name"), - tool_calls=msg.get("tool_calls"), - tool_call_id=msg.get("tool_call_id"), - ) + # Replace stored messages with current history atomically so a + # mid-rewrite failure rolls back and the previously persisted + # conversation is preserved (salvaged from #13675). + db.replace_messages(state.session_id, state.history) except Exception: logger.warning("Failed to persist ACP session %s", state.session_id, exc_info=True) diff --git a/scripts/release.py b/scripts/release.py index 1b795c27d31..231b5d7090e 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -56,6 +56,7 @@ "657290301@qq.com": "IMHaoyan", "revar@users.noreply.github.com": "revaraver", "dengtaoyuan@dengtaoyuandeMac-mini.local": "dengtaoyuan450-a11y", + "ysfalweshcan@gmail.com": "Junass1", "beardthelion@users.noreply.github.com": "beardthelion", "tangyuanjc@JCdeAIfenshendeMac-mini.local": "tangyuanjc", "leon@agentlinker.ai": "agentlinker", diff --git a/tests/acp/test_session.py b/tests/acp/test_session.py index 03d5f3f658c..60a006b2a8c 100644 --- a/tests/acp/test_session.py +++ b/tests/acp/test_session.py @@ -188,6 +188,31 @@ def test_list_sessions_hides_empty_threads(self, manager): manager.create_session(cwd="/empty") assert manager.list_sessions() == [] + def test_save_session_preserves_existing_messages_on_encode_failure(self, manager): + """Regression for #13675: a bad message in state.history must not + clobber the previously-persisted transcript. replace_messages() + wraps DELETE + INSERT in a single rolled-back-on-exception txn. + """ + state = manager.create_session() + state.history.append({"role": "user", "content": "original"}) + manager.save_session(state.session_id) + + # Now swap history with a message whose tool_calls is non-JSON-serializable. + # _execute_write rolls back; the previously persisted "original" stays. + state.history = [ + {"role": "user", "content": "replacement"}, + { + "role": "assistant", + "content": None, + "tool_calls": [{"bad": object()}], + }, + ] + manager.save_session(state.session_id) + + db = manager._get_db() + messages = db.get_messages_as_conversation(state.session_id) + assert messages == [{"role": "user", "content": "original"}] + def test_cleanup_clears_all(self, manager): s1 = manager.create_session() s2 = manager.create_session()