Skip to content

Commit ca734db

Browse files
committed
fix: catch BaseException in materialize cleanup so CancelledError removes temp dir
asyncio.CancelledError is BaseException (not Exception) since 3.8. The post-mkdtemp cleanup block used 'except Exception:', so a task cancelled while awaiting _materialize_subkeys() would skip shutil.rmtree and leak the temp dir — including .credentials.json with the user's accessToken. Callers can't compensate because the assignment raises before completing. Adds test_cancelled_after_mkdtemp_cleans_temp_dir.
1 parent 46c4cf1 commit ca734db

2 files changed

Lines changed: 48 additions & 2 deletions

File tree

src/claude_agent_sdk/_internal/session_resume.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,13 @@ async def materialize_resume_session(
112112
await _materialize_subkeys(
113113
store, tmp_base, project_dir, project_key, session_id, timeout_s
114114
)
115-
except Exception:
115+
except BaseException:
116116
# Any failure after mkdtemp leaves tmp_base (which may already
117117
# contain a .credentials.json copy) on disk with no path for the
118-
# caller to clean it up. Remove it before rethrowing.
118+
# caller to clean it up. Remove it before rethrowing. BaseException
119+
# so asyncio.CancelledError (BaseException since 3.8) also triggers
120+
# cleanup — callers can't compensate because the assignment raises
121+
# before completing.
119122
shutil.rmtree(tmp_base, ignore_errors=True)
120123
raise
121124

tests/test_session_resume.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,49 @@ def spy(*a, **kw):
528528
assert created, "load() succeeded so mkdtemp should have run"
529529
assert not Path(created[0]).exists()
530530

531+
@pytest.mark.asyncio
532+
async def test_cancelled_after_mkdtemp_cleans_temp_dir(
533+
self, cwd: Path, project_key: str, isolated_home: Path
534+
) -> None:
535+
"""``asyncio.CancelledError`` is ``BaseException`` (not ``Exception``)
536+
since 3.8 — the cleanup-on-failure block must catch it so a temp dir
537+
already containing ``.credentials.json`` is not leaked when the outer
538+
task is cancelled mid-subkey-load."""
539+
540+
class HungSubkeysStore(InMemorySessionStore):
541+
async def list_subkeys(self, key): # type: ignore[override]
542+
await asyncio.sleep(3600)
543+
return []
544+
545+
store = HungSubkeysStore()
546+
await store.append(
547+
{"project_key": project_key, "session_id": SESSION_ID},
548+
[{"type": "user", "uuid": "u1"}],
549+
)
550+
opts = ClaudeAgentOptions(cwd=cwd, session_store=store, resume=SESSION_ID)
551+
552+
real_mkdtemp = tempfile.mkdtemp
553+
created: list[str] = []
554+
555+
def spy(*a, **kw):
556+
d = real_mkdtemp(*a, **kw)
557+
created.append(d)
558+
return d
559+
560+
with patch("tempfile.mkdtemp", side_effect=spy):
561+
task = asyncio.create_task(materialize_resume_session(opts))
562+
# Let it run past mkdtemp and into list_subkeys.
563+
for _ in range(50):
564+
await asyncio.sleep(0)
565+
if created:
566+
break
567+
assert created, "mkdtemp should have run before cancellation"
568+
task.cancel()
569+
with pytest.raises(asyncio.CancelledError):
570+
await task
571+
572+
assert not Path(created[0]).exists()
573+
531574
@pytest.mark.asyncio
532575
async def test_non_json_serializable_entry_surfaces_clear_error(
533576
self, cwd: Path, project_key: str, isolated_home: Path

0 commit comments

Comments
 (0)