From 66c7f17fb418d85d19fe6d645891b9dae181398b Mon Sep 17 00:00:00 2001 From: Mason Daugherty <61371264+mdrxy@users.noreply.github.com> Date: Wed, 15 Jul 2026 14:43:12 +0000 Subject: [PATCH 1/2] fix(code): clean offloaded history when deleting a thread Deleting a thread via the /threads switcher (or the threads delete CLI) only removed its SQLite checkpoints, leaving the per-thread offloaded conversation-history archive orphaned under ~/.deepagents. Add delete_offloaded_history to remove that archive and call it from delete_thread. Co-authored-by: open-swe[bot] --- libs/code/deepagents_code/offload.py | 50 +++++++++++++++++++++ libs/code/deepagents_code/sessions.py | 41 ++++++++++------- libs/code/tests/unit_tests/test_offload.py | 50 +++++++++++++++++++++ libs/code/tests/unit_tests/test_sessions.py | 20 ++++++++- 4 files changed, 143 insertions(+), 18 deletions(-) diff --git a/libs/code/deepagents_code/offload.py b/libs/code/deepagents_code/offload.py index a3124ad46f..5d3d99817b 100644 --- a/libs/code/deepagents_code/offload.py +++ b/libs/code/deepagents_code/offload.py @@ -223,3 +223,53 @@ def _prepare_temp_dir(path: Path) -> Path: ) unique = Path(tempfile.mkdtemp(prefix=f"deepagents-{suffix}-", dir=temp_root)) return _prepare_temp_dir(unique) + + +def delete_offloaded_history(thread_id: str) -> bool: + """Remove a thread's offloaded conversation-history archive. + + Deletes the per-thread markdown file written by the local-mode + `conversation_history` backend (`{root}/conversation_history/{thread_id}.md`), + resolving `root` with `_offload_fallback_root` so the persistent + `~/.deepagents` location and any temporary fallback are both covered. This + is a no-op when the thread has no archive. + + In server/sandbox mode the archive lives on the sandbox backend rather than + the local `~/.deepagents` directory, so there is nothing local to remove and + this simply finds no file. + + Args: + thread_id: Thread whose offloaded history should be removed. + + Returns: + `True` if an archive file was removed, `False` when none existed or the + offload root could not be resolved. + """ + if not thread_id: + return False + try: + archive_dir = _offload_fallback_root() / "conversation_history" + except (OSError, RuntimeError): + logger.warning( + "Could not resolve offload root to clean history for thread %s", + thread_id, + exc_info=True, + ) + return False + archive_path = archive_dir / f"{thread_id}.md" + # Guard against a crafted thread id escaping the archive directory. + if archive_path.parent != archive_dir: + return False + try: + archive_path.unlink() + except FileNotFoundError: + return False + except OSError: + logger.warning( + "Failed to delete offloaded conversation history for thread %s", + thread_id, + exc_info=True, + ) + return False + logger.debug("Deleted offloaded conversation history for thread %s", thread_id) + return True diff --git a/libs/code/deepagents_code/sessions.py b/libs/code/deepagents_code/sessions.py index cfbe04be01..2f84704763 100644 --- a/libs/code/deepagents_code/sessions.py +++ b/libs/code/deepagents_code/sessions.py @@ -1406,28 +1406,37 @@ async def find_similar_threads(thread_id: str, limit: int = 3) -> list[str]: async def delete_thread(thread_id: str) -> bool: - """Delete thread checkpoints. + """Delete thread checkpoints and any offloaded conversation history. + + Removes the thread's checkpoint/write rows and cleans up the per-thread + offloaded conversation-history archive under `~/.deepagents` (local mode), + so deleting a thread does not leave orphaned history behind. Returns: True if thread was deleted, False if not found. """ + deleted = False async with _connect() as conn: - if not await _table_exists(conn, "checkpoints"): - return False + if await _table_exists(conn, "checkpoints"): + cursor = await conn.execute( + "DELETE FROM checkpoints WHERE thread_id = ?", (thread_id,) + ) + deleted = cursor.rowcount > 0 + if await _table_exists(conn, "writes"): + await conn.execute( + "DELETE FROM writes WHERE thread_id = ?", (thread_id,) + ) + await conn.commit() + if deleted: + _message_count_cache.pop(thread_id, None) + for key, rows in list(_recent_threads_cache.items()): + filtered = [row for row in rows if row["thread_id"] != thread_id] + _recent_threads_cache[key] = filtered - cursor = await conn.execute( - "DELETE FROM checkpoints WHERE thread_id = ?", (thread_id,) - ) - deleted = cursor.rowcount > 0 - if await _table_exists(conn, "writes"): - await conn.execute("DELETE FROM writes WHERE thread_id = ?", (thread_id,)) - await conn.commit() - if deleted: - _message_count_cache.pop(thread_id, None) - for key, rows in list(_recent_threads_cache.items()): - filtered = [row for row in rows if row["thread_id"] != thread_id] - _recent_threads_cache[key] = filtered - return deleted + from deepagents_code.offload import delete_offloaded_history + + delete_offloaded_history(thread_id) + return deleted @asynccontextmanager diff --git a/libs/code/tests/unit_tests/test_offload.py b/libs/code/tests/unit_tests/test_offload.py index b7688b2a60..8f58bd7ee5 100644 --- a/libs/code/tests/unit_tests/test_offload.py +++ b/libs/code/tests/unit_tests/test_offload.py @@ -20,6 +20,7 @@ _artifacts_root, _filesystem_tool_path, _offload_fallback_root, + delete_offloaded_history, ) from deepagents_code.tui.widgets.messages import AppMessage, ErrorMessage @@ -1189,6 +1190,55 @@ def test_fallback_root_tightens_preexisting_loose_archive_subdir( assert offload_storage_is_ephemeral() is False +class TestDeleteOffloadedHistory: + """Cover cleanup of a thread's offloaded conversation-history archive.""" + + def test_removes_persistent_archive( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """The per-thread archive under `~/.deepagents` is removed.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + archive_dir = tmp_path / ".deepagents" / "conversation_history" + archive_dir.mkdir(parents=True) + archive = archive_dir / "thread-1.md" + archive.write_text("history") + keep = archive_dir / "thread-2.md" + keep.write_text("other") + + assert delete_offloaded_history("thread-1") is True + assert not archive.exists() + # Unrelated threads' archives are left untouched. + assert keep.exists() + + def test_missing_archive_is_noop( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Deleting a thread with no archive reports nothing removed.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + + assert delete_offloaded_history("thread-1") is False + + def test_empty_thread_id_is_noop( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """An empty thread id never touches the filesystem.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + + assert delete_offloaded_history("") is False + + def test_rejects_thread_id_path_traversal( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A crafted thread id cannot escape the archive directory.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + (tmp_path / ".deepagents" / "conversation_history").mkdir(parents=True) + secret = tmp_path / ".deepagents" / "config.toml" + secret.write_text("secret") + + assert delete_offloaded_history("../config") is False + assert secret.exists() + + class TestArtifactsRoot: """Cover the real-filesystem artifacts root for offloaded tool results.""" diff --git a/libs/code/tests/unit_tests/test_sessions.py b/libs/code/tests/unit_tests/test_sessions.py index 38a8846ace..cb8d95a291 100644 --- a/libs/code/tests/unit_tests/test_sessions.py +++ b/libs/code/tests/unit_tests/test_sessions.py @@ -351,19 +351,35 @@ def test_get_thread_cwd_ignores_empty_string(self, temp_db): cwd = asyncio.run(sessions.get_thread_cwd("thread-empty")) assert cwd is None - def test_delete_thread(self, temp_db): + def test_delete_thread(self, temp_db, monkeypatch, tmp_path): """Delete thread removes thread.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) with patch.object(sessions, "get_db_path", return_value=temp_db): result = asyncio.run(sessions.delete_thread("thread1")) assert result is True assert asyncio.run(sessions.thread_exists("thread1")) is False - def test_delete_thread_not_found(self, temp_db): + def test_delete_thread_not_found(self, temp_db, monkeypatch, tmp_path): """Delete thread returns False for non-existing thread.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) with patch.object(sessions, "get_db_path", return_value=temp_db): result = asyncio.run(sessions.delete_thread("nonexistent")) assert result is False + def test_delete_thread_cleans_offloaded_history( + self, temp_db, monkeypatch, tmp_path + ): + """Deleting a thread removes its offloaded conversation history.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + archive_dir = tmp_path / ".deepagents" / "conversation_history" + archive_dir.mkdir(parents=True) + archive = archive_dir / "thread1.md" + archive.write_text("history") + with patch.object(sessions, "get_db_path", return_value=temp_db): + result = asyncio.run(sessions.delete_thread("thread1")) + assert result is True + assert not archive.exists() + class TestGetCheckpointer: """Tests for get_checkpointer async context manager.""" From b0f12f329eafbfb8d0f343e1645993c8cee725d2 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 15 Jul 2026 14:25:56 -0400 Subject: [PATCH 2/2] cr --- libs/code/deepagents_code/offload.py | 41 ++++++++--- libs/code/deepagents_code/sessions.py | 10 +-- libs/code/tests/unit_tests/test_offload.py | 80 +++++++++++++++++++-- libs/code/tests/unit_tests/test_sessions.py | 34 ++++++++- 4 files changed, 147 insertions(+), 18 deletions(-) diff --git a/libs/code/deepagents_code/offload.py b/libs/code/deepagents_code/offload.py index 5d3d99817b..194900214e 100644 --- a/libs/code/deepagents_code/offload.py +++ b/libs/code/deepagents_code/offload.py @@ -45,6 +45,9 @@ def _filesystem_tool_path(path: PurePath) -> str: _EPHEMERAL_OFFLOAD_STORAGE = False """Whether the most recent `_offload_fallback_root` fell back to temp storage.""" +_UNIQUE_OFFLOAD_FALLBACK_ROOT: Path | None = None +"""Private random fallback root that cannot be reconstructed on a later call.""" + def offload_storage_is_ephemeral() -> bool: """Return whether offload history is routed to non-persistent storage. @@ -194,7 +197,13 @@ def _prepare_temp_dir(path: Path) -> Path: _probe_writable(path) return path - global _EPHEMERAL_OFFLOAD_STORAGE # noqa: PLW0603 + global _EPHEMERAL_OFFLOAD_STORAGE, _UNIQUE_OFFLOAD_FALLBACK_ROOT # noqa: PLW0603 + if _UNIQUE_OFFLOAD_FALLBACK_ROOT is not None: + # Unlike the persistent and predictable temp paths, a directory created + # by `mkdtemp` cannot be derived again. Keep returning the root already + # used by the archive backend so cleanup reaches the same files. + _EPHEMERAL_OFFLOAD_STORAGE = True + return _UNIQUE_OFFLOAD_FALLBACK_ROOT try: root = _prepare_user_dir() except (RuntimeError, OSError): @@ -222,7 +231,8 @@ def _prepare_temp_dir(path: Path) -> Path: exc_info=True, ) unique = Path(tempfile.mkdtemp(prefix=f"deepagents-{suffix}-", dir=temp_root)) - return _prepare_temp_dir(unique) + _UNIQUE_OFFLOAD_FALLBACK_ROOT = _prepare_temp_dir(unique) + return _UNIQUE_OFFLOAD_FALLBACK_ROOT def delete_offloaded_history(thread_id: str) -> bool: @@ -231,19 +241,25 @@ def delete_offloaded_history(thread_id: str) -> bool: Deletes the per-thread markdown file written by the local-mode `conversation_history` backend (`{root}/conversation_history/{thread_id}.md`), resolving `root` with `_offload_fallback_root` so the persistent - `~/.deepagents` location and any temporary fallback are both covered. This - is a no-op when the thread has no archive. + `~/.deepagents` location and any temporary fallback are both covered. + + Best-effort: filesystem failures are logged and swallowed rather than + raised, so a failed cleanup never blocks thread deletion. Resolving the + offload root is not side-effect-free -- it creates (and hardens) the + `conversation_history` directory and writes a short-lived probe file -- so a + call for a thread that has no archive still touches the filesystem before + returning `False`. In server/sandbox mode the archive lives on the sandbox backend rather than - the local `~/.deepagents` directory, so there is nothing local to remove and - this simply finds no file. + the local `~/.deepagents` directory, so there is no local archive to remove. Args: thread_id: Thread whose offloaded history should be removed. Returns: - `True` if an archive file was removed, `False` when none existed or the - offload root could not be resolved. + `True` only if an archive file was removed. `False` in every other case: + an empty or rejected `thread_id`, an unresolvable offload root, a missing + archive, or an `unlink` failure. """ if not thread_id: return False @@ -257,8 +273,15 @@ def delete_offloaded_history(thread_id: str) -> bool: ) return False archive_path = archive_dir / f"{thread_id}.md" - # Guard against a crafted thread id escaping the archive directory. + # Guard against a crafted thread id escaping the archive directory. Thread + # ids are system-generated UUID7 strings, so a rejection here means either a + # crafted input or a bug emitting malformed ids -- both worth a trace, and + # both distinct from the benign "no archive exists" path below. if archive_path.parent != archive_dir: + logger.warning( + "Refusing to delete offloaded history for suspicious thread id %r", + thread_id, + ) return False try: archive_path.unlink() diff --git a/libs/code/deepagents_code/sessions.py b/libs/code/deepagents_code/sessions.py index 2f84704763..09ea568836 100644 --- a/libs/code/deepagents_code/sessions.py +++ b/libs/code/deepagents_code/sessions.py @@ -1408,12 +1408,14 @@ async def find_similar_threads(thread_id: str, limit: int = 3) -> list[str]: async def delete_thread(thread_id: str) -> bool: """Delete thread checkpoints and any offloaded conversation history. - Removes the thread's checkpoint/write rows and cleans up the per-thread - offloaded conversation-history archive under `~/.deepagents` (local mode), - so deleting a thread does not leave orphaned history behind. + Removes the thread's checkpoint/write rows, then makes a best-effort attempt + to remove the per-thread offloaded conversation-history archive under + `~/.deepagents` (local mode) so deletion does not leave orphaned history + behind. History cleanup failures are logged, not raised, and do not affect + the return value, which reflects only whether checkpoint rows were removed. Returns: - True if thread was deleted, False if not found. + True if thread checkpoints were deleted, False if not found. """ deleted = False async with _connect() as conn: diff --git a/libs/code/tests/unit_tests/test_offload.py b/libs/code/tests/unit_tests/test_offload.py index 8f58bd7ee5..98f2bb4f97 100644 --- a/libs/code/tests/unit_tests/test_offload.py +++ b/libs/code/tests/unit_tests/test_offload.py @@ -13,6 +13,7 @@ import pytest from deepagents.backends.utils import validate_path +from deepagents_code import offload from deepagents_code._session_stats import format_token_count from deepagents_code.app import DeepAgentsApp from deepagents_code.command_registry import get_slash_commands @@ -1078,6 +1079,7 @@ def test_fallback_root_avoids_file_at_predictable_per_user_path( monkeypatch.setattr(Path, "home", lambda: tmp_path / "home") monkeypatch.setattr(tempfile, "gettempdir", lambda: str(temp_dir)) monkeypatch.setattr(tempfile, "NamedTemporaryFile", probe) + monkeypatch.setattr(offload, "_UNIQUE_OFFLOAD_FALLBACK_ROOT", None) root = _offload_fallback_root() @@ -1124,6 +1126,7 @@ def fake_lstat(self: Path) -> Any: # noqa: ANN401 monkeypatch.setattr(tempfile, "gettempdir", lambda: str(temp_dir)) monkeypatch.setattr(tempfile, "NamedTemporaryFile", probe) monkeypatch.setattr(Path, "lstat", fake_lstat) + monkeypatch.setattr(offload, "_UNIQUE_OFFLOAD_FALLBACK_ROOT", None) root = _offload_fallback_root() @@ -1210,7 +1213,36 @@ def test_removes_persistent_archive( # Unrelated threads' archives are left untouched. assert keep.exists() - def test_missing_archive_is_noop( + def test_removes_archive_from_reused_unique_fallback( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Cleanup reuses the random root selected when the archive was written.""" + home_root = tmp_path / "home" / ".deepagents" + home_root.mkdir(parents=True) + temp_dir = tmp_path / "tmp" + temp_dir.mkdir() + getuid = getattr(os, "getuid", None) + uid = getuid() if getuid is not None else os.getpid() + (temp_dir / f"deepagents-{uid}").write_text("not a directory") + probe = MagicMock( + side_effect=[PermissionError("read-only home"), nullcontext()] + ) + + monkeypatch.setattr(Path, "home", lambda: tmp_path / "home") + monkeypatch.setattr(tempfile, "gettempdir", lambda: str(temp_dir)) + monkeypatch.setattr(tempfile, "NamedTemporaryFile", probe) + monkeypatch.setattr(offload, "_UNIQUE_OFFLOAD_FALLBACK_ROOT", None) + + root = _offload_fallback_root() + archive = root / "conversation_history" / "thread-1.md" + archive.parent.mkdir(parents=True) + archive.write_text("history") + + assert delete_offloaded_history("thread-1") is True + assert not archive.exists() + assert probe.call_count == 2 + + def test_missing_archive_reports_nothing_removed( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """Deleting a thread with no archive reports nothing removed.""" @@ -1226,17 +1258,57 @@ def test_empty_thread_id_is_noop( assert delete_offloaded_history("") is False + def test_unlink_failure_is_swallowed( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A failing `unlink` is logged and reported as nothing removed.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setattr(offload, "_UNIQUE_OFFLOAD_FALLBACK_ROOT", None) + archive_dir = tmp_path / ".deepagents" / "conversation_history" + archive_dir.mkdir(parents=True) + archive = archive_dir / "thread-1.md" + archive.write_text("history") + monkeypatch.setattr( + Path, "unlink", MagicMock(side_effect=PermissionError("read-only mount")) + ) + + assert delete_offloaded_history("thread-1") is False + # The archive survives the failed deletion rather than being lost. + assert archive.exists() + + def test_unresolvable_root_returns_false( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """An unresolvable offload root is swallowed, not raised.""" + monkeypatch.setattr( + offload, + "_offload_fallback_root", + MagicMock(side_effect=OSError("no writable location")), + ) + + assert delete_offloaded_history("thread-1") is False + def test_rejects_thread_id_path_traversal( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: """A crafted thread id cannot escape the archive directory.""" monkeypatch.setattr(Path, "home", lambda: tmp_path) (tmp_path / ".deepagents" / "conversation_history").mkdir(parents=True) - secret = tmp_path / ".deepagents" / "config.toml" - secret.write_text("secret") + # A relative escape resolves to `.deepagents/config.md`, so a decoy there + # is load-bearing: were the guard removed, `unlink` would delete it. + relative_decoy = tmp_path / ".deepagents" / "config.md" + relative_decoy.write_text("secret") + # An absolute thread id resets the join, escaping the archive tree + # entirely; place its decoy where that reset lands. + outside = tmp_path / "outside.md" + outside.write_text("secret") assert delete_offloaded_history("../config") is False - assert secret.exists() + assert delete_offloaded_history(str(tmp_path / "outside")) is False + # An embedded separator lands in a subdirectory, not `archive_dir`. + assert delete_offloaded_history("sub/thread") is False + assert relative_decoy.exists() + assert outside.exists() class TestArtifactsRoot: diff --git a/libs/code/tests/unit_tests/test_sessions.py b/libs/code/tests/unit_tests/test_sessions.py index cb8d95a291..787cb57052 100644 --- a/libs/code/tests/unit_tests/test_sessions.py +++ b/libs/code/tests/unit_tests/test_sessions.py @@ -7,7 +7,7 @@ from datetime import UTC, datetime, timedelta from pathlib import Path from typing import TYPE_CHECKING, ClassVar, cast -from unittest.mock import AsyncMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest from langgraph.checkpoint.serde.jsonplus import JsonPlusSerializer @@ -380,6 +380,38 @@ def test_delete_thread_cleans_offloaded_history( assert result is True assert not archive.exists() + def test_delete_thread_succeeds_when_history_cleanup_fails( + self, temp_db, monkeypatch, tmp_path + ): + """Checkpoint deletion drives the result; history cleanup is best-effort.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + # `delete_thread` imports the helper from `offload` at call time, so + # patching it there simulates a failed (but swallowed) cleanup. + from deepagents_code import offload + + monkeypatch.setattr( + offload, "delete_offloaded_history", MagicMock(return_value=False) + ) + with patch.object(sessions, "get_db_path", return_value=temp_db): + result = asyncio.run(sessions.delete_thread("thread1")) + assert result is True + + def test_delete_thread_removes_orphan_archive_without_checkpoints( + self, temp_db, monkeypatch, tmp_path + ): + """A thread with no checkpoints still has its orphaned archive cleaned.""" + monkeypatch.setattr(Path, "home", lambda: tmp_path) + archive_dir = tmp_path / ".deepagents" / "conversation_history" + archive_dir.mkdir(parents=True) + archive = archive_dir / "orphan-thread.md" + archive.write_text("history") + with patch.object(sessions, "get_db_path", return_value=temp_db): + result = asyncio.run(sessions.delete_thread("orphan-thread")) + # No checkpoint rows existed, so the thread is reported "not found"... + assert result is False + # ...but its stranded archive is removed regardless. + assert not archive.exists() + class TestGetCheckpointer: """Tests for get_checkpointer async context manager."""