From cb6ee721320faff8ab3ef9e8b27f03d41831cae0 Mon Sep 17 00:00:00 2001 From: Aling33 <152494711+Aling33@users.noreply.github.com> Date: Sun, 25 Oct 2026 11:02:49 +0800 Subject: [PATCH] fix(config): delete read-only thread directories on Windows (#7446) `Paths.delete_thread_dir()` called `shutil.rmtree()` directly. Windows refuses to unlink a file that carries the read-only attribute (`WinError 5`) where POSIX only needs a writable parent, so one read-only file under `user-data/` made the whole thread undeletable: `DELETE /api/threads/{id}` answered 500 `Failed to delete local thread data`, `rmtree` had already removed everything it walked before that entry, and every later retry failed on the same file. Retry at the call site: clear the read-only attribute tree-wide and call `rmtree()` once more. Only `PermissionError` is retried, the retry still raises when it cannot help, so a file another process holds open stays a real error instead of becoming a silent skip. A `onexc` handler cannot do this portably: POSIX `_rmtree_safe_fd()` catches the handler's own `PermissionError`, re-dispatches it as a `scandir` error and leaves the deletion to fail later as `ENOTEMPTY`. Co-authored-by: Aling33 --- .../packages/harness/deerflow/config/paths.py | 35 +++++++++++++++- backend/tests/test_paths_user_isolation.py | 42 +++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/backend/packages/harness/deerflow/config/paths.py b/backend/packages/harness/deerflow/config/paths.py index e8508c6808..9a2c184a83 100644 --- a/backend/packages/harness/deerflow/config/paths.py +++ b/backend/packages/harness/deerflow/config/paths.py @@ -3,6 +3,7 @@ import logging import os import re import shutil +import stat from pathlib import Path, PureWindowsPath from deerflow.config.runtime_paths import runtime_home @@ -30,6 +31,25 @@ def _validate_thread_id(thread_id: str) -> str: return validate_thread_id(thread_id) +def _clear_read_only_tree(root: Path) -> None: + """Clear the read-only attribute under *root* before repeating a failed deletion. + + Windows refuses ``unlink``/``rmdir`` on a read-only file or directory, where + POSIX needs only a writable parent, so a restored archive or a mode-preserving + sync leaves a thread undeletable (``WinError 5``). Best effort: entries that + vanished or cannot be stat-ed are skipped, and the retry reports what is left. + Symlinks are skipped because ``chmod`` would follow them to their target. + """ + for entry in (root, *root.rglob("*")): + try: + mode = entry.lstat().st_mode + if stat.S_ISLNK(mode) or mode & stat.S_IWRITE: + continue + entry.chmod(mode | stat.S_IWRITE) + except OSError: + continue + + def _validate_user_id(user_id: str) -> str: """Validate a user ID before using it in filesystem paths.""" if not _SAFE_USER_ID_RE.fullmatch(user_id): @@ -488,11 +508,22 @@ class Paths: def delete_thread_dir(self, thread_id: str, *, user_id: str | None = None) -> None: """Delete all persisted data for a thread. - The operation is idempotent: missing thread directories are ignored. + Idempotent: missing thread directories are ignored. A file the host marks + read-only is retried once after clearing that bit, because Windows refuses + to unlink it where POSIX only needs a writable parent; a file that stays + undeletable still raises. """ thread_dir = self.thread_dir(thread_id, user_id=user_id) - if thread_dir.exists(): + try: shutil.rmtree(thread_dir) + except FileNotFoundError: + return + except PermissionError: + _clear_read_only_tree(thread_dir) + try: + shutil.rmtree(thread_dir) + except FileNotFoundError: + return def resolve_virtual_path(self, thread_id: str, virtual_path: str, *, user_id: str | None = None) -> Path: """Resolve a sandbox virtual path to the actual host filesystem path. diff --git a/backend/tests/test_paths_user_isolation.py b/backend/tests/test_paths_user_isolation.py index 9c7a8b1181..c3015c1dd7 100644 --- a/backend/tests/test_paths_user_isolation.py +++ b/backend/tests/test_paths_user_isolation.py @@ -1,5 +1,7 @@ """Tests for user-scoped path resolution in Paths.""" +import os +import stat from pathlib import Path import pytest @@ -265,6 +267,46 @@ class TestEnsureAndDeleteWithUserId: def test_delete_thread_dir_idempotent(self, paths: Paths): paths.delete_thread_dir("nonexistent", user_id="u1") # should not raise + def test_delete_thread_dir_retries_read_only_files(self, paths: Paths, monkeypatch): + """Windows refuses a read-only file even though the parent is writable.""" + paths.ensure_thread_dirs("t1", user_id="u1") + artifact = paths.sandbox_outputs_dir("t1", user_id="u1") / "report.pdf" + artifact.write_text("quarterly") + artifact.chmod(0o444) + real_unlink = os.unlink + + def windows_unlink(path, *args, **kwargs): + # POSIX deletes through ``os.unlink(name, dir_fd=...)``, so the refusal + # has to key on that call shape to fire anywhere but Windows. + dir_fd = kwargs.get("dir_fd") + try: + if dir_fd is not None: + mode = os.stat(path, dir_fd=dir_fd, follow_symlinks=False).st_mode + else: + mode = os.stat(path).st_mode + except FileNotFoundError: + return real_unlink(path, *args, **kwargs) + if stat.S_ISREG(mode) and not mode & stat.S_IWRITE: + raise PermissionError(13, "Access is denied", str(path)) + return real_unlink(path, *args, **kwargs) + + monkeypatch.setattr(os, "unlink", windows_unlink) + paths.delete_thread_dir("t1", user_id="u1") + assert not paths.thread_dir("t1", user_id="u1").exists() + + def test_delete_thread_dir_keeps_raising_when_the_file_stays_locked(self, paths: Paths, monkeypatch): + """A file still held open is a real failure; the retry must not swallow it.""" + paths.ensure_thread_dirs("t1", user_id="u1") + (paths.sandbox_uploads_dir("t1", user_id="u1") / "notes.txt").write_text("x") + + def locked_unlink(path, *args, **kwargs): + raise PermissionError(13, "The process cannot access the file", str(path)) + + monkeypatch.setattr(os, "unlink", locked_unlink) + with pytest.raises(PermissionError): + paths.delete_thread_dir("t1", user_id="u1") + assert paths.thread_dir("t1", user_id="u1").exists() + def test_ensure_thread_dirs_legacy_still_works(self, paths: Paths): paths.ensure_thread_dirs("t1") assert paths.sandbox_work_dir("t1").is_dir()