From ea3c3d4dce99c3f0e52d3b61dbbca64025c11c81 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Thu, 29 Oct 2026 14:45:25 -0700 Subject: [PATCH] fix(images): say plainly why Windows offers no Recycle Bin (#500) Maintainer ruling (a): Windows never uses the Recycle Bin, because it deletes files larger than the bin's capacity permanently and IFileOperation's PreDeleteItem guard cannot detect it (native probe runs 40951037575, 40953347904). Each file in the Delete review now reads "Moving files to the Recycle Bin isn't available on Windows: files larger than the bin's capacity would be deleted permanently." (code windows-recycle) instead of "This drive has no trash.", and the summary line drops its "yet": "Moving to the trash is not available for these files; each one lists why." --- TESTING.md | 8 +++++-- py/api/image_delete.py | 4 ++++ tests/js/workspace-images-delete.test.js | 2 ++ tests/test_image_delete_routes.py | 10 ++++++++- tests/test_trash_freedesktop.py | 14 ++++++++++++ utils/trash.py | 27 ++++++++++++++++++------ web/js/workspace/images-delete.mjs | 2 +- 7 files changed, 56 insertions(+), 11 deletions(-) diff --git a/TESTING.md b/TESTING.md index f81916ab..4dec7769 100644 --- a/TESTING.md +++ b/TESTING.md @@ -2465,8 +2465,12 @@ files to the system trash. **No code path unlinks or overwrites a media file.** `.Trash/$uid` or `.Trash-$uid`), reserve the name with `O_EXCL` and move with `renameat2(RENAME_NOREPLACE)`; without it, across devices, on a read-only mount or with a trash inside an output root the move is refused (`no-trash`, `trash-in-root`). Windows - and macOS report `no-trash` until their own backends land; library-only removal works - everywhere. Tests use `tests/fake_trash.py`; the browser test server installs it, so no + never uses the Recycle Bin (#500, maintainer ruling (a)): files larger than the bin's + capacity are deleted permanently and IFileOperation's `PreDeleteItem` guard cannot detect + it (native probe runs 40951037575, 40953347904), so every file answers `windows-recycle` + ("Moving files to the Recycle Bin isn't available on Windows: files larger than the bin's + capacity would be deleted permanently."). macOS reports `no-trash` until #499 lands; + library-only removal works everywhere. Tests use `tests/fake_trash.py`; the browser test server installs it, so no test touches a real trash. ### Prompt library cards and recorded deviations (#249 PR E) diff --git a/py/api/image_delete.py b/py/api/image_delete.py index fe011149..b83c767f 100644 --- a/py/api/image_delete.py +++ b/py/api/image_delete.py @@ -80,6 +80,10 @@ REASONS = { trash.TRASH_IN_ROOT: "This drive's trash is inside an output folder.", trash.LOCKED: "The file is open in another program. Close it and try again.", trash.FAILED: "The file could not be moved to the trash.", + trash.WINDOWS_RECYCLE: ( + "Moving files to the Recycle Bin isn't available on Windows: files larger " + "than the bin's capacity would be deleted permanently." + ), "not-attempted": "Not attempted: the job stopped after an error.", } _PROBLEM_CODES = { diff --git a/tests/js/workspace-images-delete.test.js b/tests/js/workspace-images-delete.test.js index 90782294..d03e8c81 100644 --- a/tests/js/workspace-images-delete.test.js +++ b/tests/js/workspace-images-delete.test.js @@ -65,6 +65,8 @@ test('sentences follow the mode and the backup policy', () => { assert.deepEqual(deleteSentences(plan(), 'trash'), [SENTENCES.trash, USE_LIBRARY_ONLY]); const nowhere = plan({ trash: { count: 0 }, trash_refused: { count: 3, rows: [] } }); assert.deepEqual(deleteSentences(nowhere, 'trash'), [SENTENCES.trash, NO_TRASH_HERE, USE_LIBRARY_ONLY]); + // #500: no "yet" (Windows is unavailable by design); each file's reason says why. + assert.equal(NO_TRASH_HERE, 'Moving to the trash is not available for these files; each one lists why.'); assert.deepEqual(deleteSentences(plan({ trash_refused: { count: 0 } }), 'trash'), [SENTENCES.trash]); }); diff --git a/tests/test_image_delete_routes.py b/tests/test_image_delete_routes.py index 1fb364df..53f34471 100644 --- a/tests/test_image_delete_routes.py +++ b/tests/test_image_delete_routes.py @@ -306,6 +306,13 @@ class TestFileCheck(ImageDeleteTestCase): file_check(str(self.output_dir / "ok.png"), root, "ok.png")[0] ) + def test_windows_reason_says_why_plainly(self): + self.assertEqual( + image_delete.REASONS["windows-recycle"], + "Moving files to the Recycle Bin isn't available on Windows: files " + "larger than the bin's capacity would be deleted permanently.", + ) + def test_reasons_cover_every_code(self): from utils import trash @@ -324,7 +331,8 @@ class TestNativeBackend(ImageDeleteTestCase): native_trash = sys.platform not in ("win32", "darwin") if not native_trash: self.assertEqual(plan["trash"], {"count": 0}) - self.assertEqual(plan["trash_refused"]["counts"], {"no-trash": 1}) + code = "windows-recycle" if sys.platform == "win32" else "no-trash" + self.assertEqual(plan["trash_refused"]["counts"], {code: 1}) body = await self.post(APPLY, {"token": plan["token"], "mode": "library"}, 202) job = self.api._jobs.get(body["job"]["id"]) await asyncio.wait_for(asyncio.shield(job.task), DISK_BOUND_SECONDS) diff --git a/tests/test_trash_freedesktop.py b/tests/test_trash_freedesktop.py index a0f3f3ec..dd8ab6d5 100644 --- a/tests/test_trash_freedesktop.py +++ b/tests/test_trash_freedesktop.py @@ -285,6 +285,20 @@ class UnavailableBackendTests(unittest.TestCase): ) self.assertIsInstance(trash.default_backend("linux"), trash.FreedesktopBackend) + def test_windows_says_why_the_recycle_bin_is_not_used(self): + # #500 ruling (a): the Recycle Bin can delete permanently, so it is never used. + backend = trash.default_backend("win32") + self.assertEqual( + backend.available("C:/x/a.png", ["C:/x"]), trash.WINDOWS_RECYCLE + ) + with self.assertRaises(trash.TrashRefused) as caught: + backend.move("C:/x/a.png", ["C:/x"]) + self.assertEqual(caught.exception.code, trash.WINDOWS_RECYCLE) + self.assertIn(trash.WINDOWS_RECYCLE, trash.CODES) + self.assertEqual( + trash.default_backend("darwin").available("/x", ["/"]), trash.NO_TRASH + ) + def test_module_functions_use_the_replaceable_backend(self): fake = mock.Mock() fake.available.return_value = "network" diff --git a/utils/trash.py b/utils/trash.py index 242236b5..729f6d66 100644 --- a/utils/trash.py +++ b/utils/trash.py @@ -14,8 +14,9 @@ same mount, else the file's mount's ``.Trash/$uid`` (only under a real, sticky ``.Trash``) or ``.Trash-$uid``. Names are reserved by creating the ``.trashinfo`` with ``O_EXCL`` and the file is moved with ``renameat2(RENAME_NOREPLACE)``; without that call the move is refused. -Windows and macOS have no backend yet (:class:`UnavailableBackend`): their -own PRs add one with a native proof. +Windows never uses the Recycle Bin (#500: files over its capacity are deleted +permanently, undetectably), so it answers ``windows-recycle``; macOS has no +backend yet (#499). Both use :class:`UnavailableBackend`. ``BACKEND`` is the one replaceable seam; tests install a fake. """ @@ -36,7 +37,11 @@ TRASH_IN_ROOT = "trash-in-root" PATH_TOO_LONG = "path-too-long" LOCKED = "locked" FAILED = "failed" -CODES = frozenset({NO_TRASH, NETWORK, TRASH_IN_ROOT, PATH_TOO_LONG, LOCKED, FAILED}) +# Windows never uses the Recycle Bin (#500): it can delete permanently. +WINDOWS_RECYCLE = "windows-recycle" +CODES = frozenset( + {NO_TRASH, NETWORK, TRASH_IN_ROOT, PATH_TOO_LONG, LOCKED, FAILED, WINDOWS_RECYCLE} +) HOME_TRASH_LABEL = "home trash" RENAME_NOREPLACE = 1 @@ -177,13 +182,16 @@ def _owned_folder(path: str, uid: int, *, create: bool) -> bool: class UnavailableBackend: - """No trash on this system: every move is refused, nothing is touched.""" + """No trash on this system: every move is refused with *code*, untouched.""" + + def __init__(self, code: str = NO_TRASH): + self.code = code def available(self, path: str, roots) -> str | None: - return NO_TRASH + return self.code def move(self, path: str, roots) -> str: - raise TrashRefused(NO_TRASH) + raise TrashRefused(self.code) class FreedesktopBackend: @@ -346,7 +354,12 @@ class FreedesktopBackend: def default_backend(platform: str = sys.platform): """The backend for *platform*; Windows and macOS have none yet (#480).""" - if platform in ("win32", "darwin") or platform.startswith("cygwin"): + if platform == "win32" or platform.startswith("cygwin"): + # #500, ruling (a): Windows deletes files larger than the Recycle Bin's + # capacity permanently, and IFileOperation's PreDeleteItem guard cannot + # tell (native probe runs 40951037575, 40953347904). + return UnavailableBackend(WINDOWS_RECYCLE) + if platform == "darwin": return UnavailableBackend() return FreedesktopBackend() diff --git a/web/js/workspace/images-delete.mjs b/web/js/workspace/images-delete.mjs index 7f53015e..28b92219 100644 --- a/web/js/workspace/images-delete.mjs +++ b/web/js/workspace/images-delete.mjs @@ -32,7 +32,7 @@ export const REVIEW_FAILED = 'The review failed. Nothing was changed.'; export const SOURCE_CHANGED = 'The sources changed. Refresh the images and try again.'; export const STORAGE_CHANGED = 'Prompt storage was restored. Reopen Delete.'; export const USE_LIBRARY_ONLY = 'To remove these files from the library anyway, choose “Remove from the library only”.'; -export const NO_TRASH_HERE = 'Moving to the trash is not available on this system yet.'; +export const NO_TRASH_HERE = 'Moving to the trash is not available for these files; each one lists why.'; const SENTENCES = Object.freeze({ libraryBackup: 'Removed images stay hidden unless the file is modified or replaced. Their tags, category, collections ' + 'and favorite can only be recovered by restoring the safety backup, which also undoes every later change.',