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()