Skip to content

Commit 48a3e75

Browse files
Lukas Geigerclaude
andcommitted
fix(ops): leerer Ordner mit gesperrtem Eigen-Handle meldet nicht mehr falsch "completed" (Bug 4)
_delete_dir_skip_locked() wertete len(locked)==0 als Erfolg. Ein leerer Ordner, dessen eigenes Handle gesperrt ist (kein gesperrtes Kind, sondern der Ordner selbst -- z.B. von SearchIndexer.exe gehalten), hat keine gesperrte Innendatei; das verschluckte p.rmdir()-OSError wurde so faelschlich als geloescht gewertet. Der Worker markierte den Task "completed" und verwarf ihn statt zu retryen -- der Ordner blieb fuer immer liegen. Fix: Erfolg wird am echten FS-Zustand gemessen (not p.exists()); _delete_path() liefert fuer den Eigen-Handle-Lock eine eigene Retry-Meldung. Regressionstest: TestEmptyDirOwnHandleLocked (4 Tests). Suite 88 -> 92 gruen. Ursache empirisch bestaetigt (Windows Search Indexer, 2026-06-13). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9a1593e commit 48a3e75

6 files changed

Lines changed: 115 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,16 @@ Format basiert auf [Keep a Changelog](https://keepachangelog.com/de/1.1.0/).
1313
- `.github/workflows/source-platform-smoke.yml`: CI-Matrix für `ubuntu-latest` und `macos-latest`, die die Smoke-Tests bei jedem Push/PR auf `main` ausführt.
1414

1515
### Behoben / Fixed
16+
- **Bug 4 — falsches „completed" bei leerem Verzeichnis mit gesperrtem Eigen-Handle:**
17+
`_delete_dir_skip_locked()` meldete Erfolg anhand von `len(locked) == 0`. Ein
18+
LEERER Ordner, dessen eigenes Handle gesperrt ist (kein gesperrtes Kind, sondern
19+
der Ordner selbst — z. B. von `SearchIndexer.exe` gehalten), hat keine gesperrte
20+
Innendatei; das verschluckte `p.rmdir()`-`OSError` wurde so fälschlich als gelöscht
21+
gewertet. Folge: Der Worker markierte den Task „completed" und verwarf ihn, statt
22+
ihn erneut zu versuchen — der Ordner blieb für immer liegen. Fix: Erfolg wird jetzt
23+
am echten FS-Zustand gemessen (`not p.exists()`); `_delete_path()` liefert für den
24+
Eigen-Handle-Lock eine eigene Retry-Meldung. Tests: `TestEmptyDirOwnHandleLocked`
25+
(4 Tests). Ursache empirisch bestätigt (Windows Search Indexer, 2026-06-13).
1626
- Guard in `_refresh_status()` against race between queue reload and worker thread.
1727
- Thread-safe dict snapshot in `_watcher_tick` (tray).
1828
- `tick_all()` helper extracted; real snapshot test added.

README.de.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ stehen, aber temporär durch einen Cloud-Sync-Provider blockiert werden.
109109
- **P3 (fertig):** Präventiv-Wächter (Änderungsrate *konfigurierter* Ordner
110110
beobachten → Sync-Client automatisch pausieren/fortsetzen; bounded + stat-only,
111111
hydratisiert keine Online-only-Placeholder; opt-in).
112-
- **Tests:** `pytest`, **88 grün** (Core + P2/P3 + i18n + Multicloud-Regressionen).
112+
- **Tests:** `pytest`, **92 grün** (Core + P2/P3 + i18n + Multicloud + Leerordner-Eigen-Handle-Lock-Regressionen).
113113
- **Im Lifetest gehärtet (2026-05-29):** `is_running()/pause()` robust gegen
114114
nicht-UTF-8-`tasklist`-Ausgabe; `delete` entfernt read-only-Attribute statt an
115115
WinError 5 zu scheitern. Erster echter Einsatz: ein Ordner-Rename, den manuelle

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ cloud-sync provider.
110110
- **P3 (done):** Preventive watcher (observes the change rate of *configured*
111111
folders → pauses/resumes the sync client; bounded, stat-only, does not
112112
hydrate online-only placeholders; opt-in).
113-
- **Tests:** `pytest`, 88 passing (core + P2/P3 + i18n + multicloud regressions).
113+
- **Tests:** `pytest`, 92 passing (core + P2/P3 + i18n + multicloud + empty-dir own-handle-lock regressions).
114114
- **Open/future:** more provider adapters (Dropbox / Google Drive / iCloud);
115115
optional suppression of sync-client relaunch during long operations.
116116

TODO.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ Beim Port der copy+delete-Logik in den ellmos-filecommander-mcp-Server (TypeScri
4343
`_delete_dir_skip_locked()` in ops.py: Best-effort-Bereinigung, überspringt EBUSY-Dateien.
4444
`_delete_path()` nutzt es automatisch wenn `_rmtree()` mit Lock-Fehler scheitert.
4545
Tests: TestDeleteDirSkipLocked (5 Tests).
46+
- [x] **Bug 4: leerer Ordner mit gesperrtem Eigen-Handle → falsches „completed"** — DONE 2026-06-13
47+
`_delete_dir_skip_locked()` wertete `len(locked) == 0` als Erfolg; bei einem leeren,
48+
am eigenen Handle gesperrten Ordner (z. B. Windows Search Indexer) gibt es keine
49+
gesperrte Innendatei → das verschluckte `p.rmdir()`-OSError wurde als Erfolg gewertet,
50+
der Worker verwarf den Task statt zu retryen. Fix: Erfolg = `not p.exists()`; eigene
51+
Retry-Meldung in `_delete_path()`. Tests: TestEmptyDirOwnHandleLocked (4 Tests).
4652

4753
## Nächste Schritte (aus ROADMAP.md)
4854

src/cloudlockfixer/ops.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,14 @@ def _delete_path(p: Path) -> tuple[bool, str]:
100100
ok, locked = _delete_dir_skip_locked(p)
101101
if ok:
102102
return True, "geloescht (gesperrte Innendateien uebersprungen)"
103-
names = ", ".join(str(lp.name) for lp in locked[:3])
104-
return False, f"Innendatei(en) noch gesperrt (EBUSY): {names}"
103+
if locked:
104+
names = ", ".join(str(lp.name) for lp in locked[:3])
105+
return False, f"Innendatei(en) noch gesperrt (EBUSY): {names}"
106+
# Bug 4: leeres Verzeichnis, aber sein eigenes Handle ist gesperrt
107+
return False, (
108+
"Verzeichnis-Handle selbst gesperrt (offenes Handle, "
109+
"z.B. Windows Search-Indexer) -- Retry"
110+
)
105111
raise
106112
else:
107113
try:
@@ -150,7 +156,12 @@ def _delete_dir_skip_locked(p: Path) -> tuple[bool, list[Path]]:
150156
except OSError:
151157
pass
152158

153-
return len(locked) == 0, locked
159+
# Bug 4: Erfolg am echten FS-Zustand messen, nicht an `locked`. Ein leeres
160+
# Verzeichnis, dessen EIGENES Handle gesperrt ist (z.B. Windows Search-Indexer),
161+
# hat KEINE gesperrte Innendatei -> locked bliebe [] und das oben verschluckte
162+
# p.rmdir()-OSError wuerde faelschlich als Erfolg gewertet. Re-Check verhindert
163+
# das stille "completed", sodass der Worker den Task korrekt erneut versucht.
164+
return (not p.exists()), locked
154165

155166

156167
def _do_move(src: Path, dst: Path) -> tuple[bool, str, bool]:

tests/test_ebusy_and_lock_errors.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,3 +240,86 @@ def fake_skip(p):
240240

241241
assert ok, f"_delete_path soll OK zurückgeben wenn skip_locked erfolgreich: {msg}"
242242
assert skip_called, "_delete_dir_skip_locked muss aufgerufen worden sein"
243+
244+
245+
# ── Bug 4: leeres Verzeichnis mit gesperrtem EIGEN-Handle (z.B. Search-Indexer) ──
246+
247+
class TestEmptyDirOwnHandleLocked:
248+
"""Regression: ein LEERES Verzeichnis, dessen eigenes Handle gesperrt ist
249+
(kein gesperrtes Kind, sondern der Ordner selbst — z.B. von SearchIndexer.exe
250+
gehalten), darf NICHT fälschlich als gelöscht ('completed') gemeldet werden.
251+
Sonst verwirft der Worker den Task statt ihn erneut zu versuchen (false-completed).
252+
"""
253+
254+
def _lock_rmdir(self, monkeypatch, target: Path) -> None:
255+
"""Path.rmdir für genau `target` mit EBUSY scheitern lassen (Handle gehalten)."""
256+
original_rmdir = Path.rmdir
257+
258+
def fake_rmdir(self):
259+
if self == target:
260+
e = OSError(errno.EBUSY, "EBUSY: directory handle held by another process")
261+
e.errno = errno.EBUSY
262+
raise e
263+
return original_rmdir(self)
264+
265+
monkeypatch.setattr(Path, "rmdir", fake_rmdir)
266+
267+
def test_skip_locked_reports_failure_when_own_handle_locked(self, tmp_path, monkeypatch):
268+
d = tmp_path / "emptylocked"
269+
d.mkdir()
270+
self._lock_rmdir(monkeypatch, d)
271+
272+
ok, locked = _delete_dir_skip_locked(d)
273+
274+
assert not ok, "Leeres, am Eigen-Handle gesperrtes Verzeichnis darf nicht als Erfolg gelten"
275+
assert locked == [], "Keine gesperrte Innendatei -> locked muss leer bleiben"
276+
assert d.exists(), "Verzeichnis existiert noch (rmdir scheiterte) -> kein Erfolg"
277+
278+
def test_delete_path_reports_failure_and_retry_hint(self, tmp_path, monkeypatch):
279+
d = tmp_path / "busyempty"
280+
d.mkdir()
281+
282+
# rmtree muss mit Lock-Fehler scheitern, damit der skip_locked-Pfad genommen wird
283+
def fake_rmtree(p, **kwargs):
284+
e = OSError(errno.EBUSY, "EBUSY")
285+
e.errno = errno.EBUSY
286+
raise e
287+
288+
monkeypatch.setattr(ops, "_rmtree", fake_rmtree)
289+
self._lock_rmdir(monkeypatch, d)
290+
291+
ok, msg = ops._delete_path(d)
292+
293+
assert not ok, "_delete_path muss Fehlschlag melden (kein stilles completed)"
294+
assert d.exists()
295+
assert "handle" in msg.lower() or "retry" in msg.lower(), \
296+
f"Meldung soll Eigen-Handle-Lock/Retry benennen, war: '{msg}'"
297+
298+
def test_chain_stays_pending_not_completed(self, tmp_path, monkeypatch):
299+
"""execute_chain darf den Delete-Task NICHT als done markieren (-> Worker retryt)."""
300+
d = tmp_path / "queuedempty"
301+
d.mkdir()
302+
303+
def fake_rmtree(p, **kwargs):
304+
e = OSError(errno.EBUSY, "EBUSY")
305+
e.errno = errno.EBUSY
306+
raise e
307+
308+
monkeypatch.setattr(ops, "_rmtree", fake_rmtree)
309+
self._lock_rmdir(monkeypatch, d)
310+
311+
task = Task(chain=[Step(op="delete", src=str(d))])
312+
result = ops.execute_chain(task)
313+
314+
assert result is False, "Kette muss fehlschlagen solange das Handle gesperrt ist"
315+
assert task.status != "done", "Task darf nicht fälschlich 'done' sein"
316+
assert task.last_error, "last_error muss gesetzt sein (Grundlage für nächsten Retry)"
317+
assert d.exists()
318+
319+
def test_succeeds_once_handle_released(self, tmp_path):
320+
"""Gegenprobe: ohne Lock wird derselbe leere Ordner sauber gelöscht."""
321+
d = tmp_path / "freed"
322+
d.mkdir()
323+
ok, msg = ops._delete_path(d)
324+
assert ok, f"Ohne Lock muss der leere Ordner gelöscht werden: {msg}"
325+
assert not d.exists()

0 commit comments

Comments
 (0)