Skip to content

fix(sessions): make hard deletes complete and retryable - #1174

Open
GautamSharma99 wants to merge 1 commit into
anthropics:mainfrom
GautamSharma99:fix/1164-retryable-hard-delete
Open

fix(sessions): make hard deletes complete and retryable#1174
GautamSharma99 wants to merge 1 commit into
anthropics:mainfrom
GautamSharma99:fix/1164-retryable-hard-delete

Conversation

@GautamSharma99

Copy link
Copy Markdown

Summary

delete_session() documented a permanent hard delete of both the primary JSONL transcript and its sibling subagent directory, but it removed the primary file first and then called:

shutil.rmtree(subagent_dir, ignore_errors=True)

Any permission, read-only filesystem, traversal, or transient I/O failure was silently discarded. The call returned success with sensitive subagent transcripts still present, and the deleted primary file made the same public operation impossible to retry.

This PR makes hard-delete success truthful and failures recoverable.

Fixes #1164.

New deletion order

1. Locate and validate the primary session transcript.
2. Remove the sibling subagent tree.
3. Only after that succeeds, unlink the primary transcript.

Subagent cleanup now happens first because it contains the data whose silent retention created the bug. If that cleanup fails, the primary transcript remains discoverable and the caller can retry delete_session() after resolving the filesystem problem.

If the final primary unlink fails, the subagent tree is already gone and the retained primary transcript still permits another retry.

Error semantics

Removed broad suppression

ignore_errors=True is gone. Permission errors, read-only filesystem errors, busy files, and other OSError failures now propagate to the caller.

The public docstring now documents OSError.

Ignore only an actually absent sibling tree

A missing subagent directory is normal and remains a no-op.

However, shutil.rmtree() can also surface FileNotFoundError for a nested entry that disappears during traversal on supported Python versions/filesystems. Treating every ENOENT as "the tree is absent" would recreate partial-delete behavior.

After rmtree() raises FileNotFoundError, the implementation uses lstat() on the top-level sibling path:

  • top-level path is gone: cleanup goal is satisfied, continue;
  • top-level path still exists: re-raise the traversal failure and preserve the primary transcript.

This also handles a concurrent process removing the entire tree between lookup and deletion without turning an already-satisfied cleanup into an error.

Resulting guarantees

On normal return:

  • the primary {session_id}.jsonl is absent;
  • the sibling {session_id}/ subagent tree is absent.

On subagent cleanup failure:

  • the error is visible;
  • the primary transcript remains;
  • the same delete_session() call is retryable.

On primary unlink failure:

  • the error is visible;
  • the subagent tree is already absent;
  • the primary transcript remains retryable.

As with any unlocked filesystem API, another process can recreate a path after deletion; this PR guarantees the state established by this operation rather than attempting cross-process locking.

Regression tests

Added tests that verify:

  • a simulated PermissionError from subagent cleanup propagates;
  • both the primary transcript and subagent directory remain after that failure;
  • retrying the same public call succeeds after the transient failure is removed;
  • nested FileNotFoundError propagates while the top-level tree still exists;
  • a final primary unlink() failure occurs only after the subagent tree is removed;
  • the existing no-subagent-directory case still succeeds;
  • the existing normal cascading delete still removes both locations.

Validation

uv run --extra dev pytest -q
1294 passed, 5 skipped

uv run --extra dev pytest -q   tests/test_session_mutations.py   tests/test_sessions.py   tests/test_session_helpers_store.py
237 passed

uv run --extra dev ruff check src tests
All checks passed!

uv run --extra dev ruff format --check src tests
59 files already formatted

uv run --extra dev mypy src
Success: no issues found in 24 source files

Scope

This changes only the filesystem-backed delete_session() operation and its tests/documentation. SessionStore-backed deletion, other mutation APIs, and public signatures are unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

delete_session can report success while leaving subagent transcripts behind

1 participant