Saved searches: Trash and Unpin have no Undo action #758

Open
opened 2026-10-02 13:09:36 +00:00 by kayg · 6 comments
Owner

Found during the source review for #427 on origin/dev at c4a61e8cf090170f35b1bed3350d9de20c83ecd5.

Decided behavior

DESIGN §32 S10 (saved searches), §34 (8 s Undo toast), and the owner UX completeness rule of 2026-10-02 require recoverable state changes.

Evidence

apps/web/src/lib/search/SavedSearchList.svelte:67–74 awaits Unpin or Trash, then posts a plain toast. Neither toast has an action. apps/web/src/lib/search/saved.svelte.ts:139–141 calls DELETE and removes the item. crates/calternal-search/src/saved.rs:491–507 moves the collection file to Trash but discards the Trash receipt and returns 204.

Observed behavior

The User can lose a pinned search or remove it with one action, but cannot undo that action from the result toast.

This is a source finding. No production build or live-server run was made in this review job.

Expected

Offer Undo for Unpin and Trash. Restore the same saved-search ID, name, query and pin state. Use the shared toast and shared write routes. Do not recreate the search with a new ID.

Test idea

Create and pin a search. Unpin, undo, then Trash and undo. Check the sidebar, cold saved-search link and collection file, including a failed Undo and a concurrent query edit.

For UI evidence, use a production build at 390, 820 and 1440 px in light and dark, with macOS platform hints. Check pointer, keyboard and touch, plus reduced motion.

Duplicate check

Searched all issue titles plus saved-search search results. Read #64 and #167: they cover saved-search implementation and the general deep-link audit, not this missing recovery action. #722 covers existing delete Undo actions that fail to repaint; these actions have no Undo at all.

Found during the source review for #427 on `origin/dev` at `c4a61e8cf090170f35b1bed3350d9de20c83ecd5`. ## Decided behavior DESIGN §32 S10 (saved searches), §34 (8 s Undo toast), and the owner UX completeness rule of 2026-10-02 require recoverable state changes. ## Evidence `apps/web/src/lib/search/SavedSearchList.svelte:67–74` awaits Unpin or Trash, then posts a plain toast. Neither toast has an action. `apps/web/src/lib/search/saved.svelte.ts:139–141` calls DELETE and removes the item. `crates/calternal-search/src/saved.rs:491–507` moves the collection file to Trash but discards the Trash receipt and returns 204. ## Observed behavior The User can lose a pinned search or remove it with one action, but cannot undo that action from the result toast. This is a source finding. No production build or live-server run was made in this review job. ## Expected Offer Undo for Unpin and Trash. Restore the same saved-search ID, name, query and pin state. Use the shared toast and shared write routes. Do not recreate the search with a new ID. ## Test idea Create and pin a search. Unpin, undo, then Trash and undo. Check the sidebar, cold saved-search link and collection file, including a failed Undo and a concurrent query edit. For UI evidence, use a production build at 390, 820 and 1440 px in light and dark, with macOS platform hints. Check pointer, keyboard and touch, plus reduced motion. ## Duplicate check Searched all issue titles plus saved-search search results. Read #64 and #167: they cover saved-search implementation and the general deep-link audit, not this missing recovery action. #722 covers existing delete Undo actions that fail to repaint; these actions have no Undo at all.
Author
Owner

Started implementation on job/undo-a11y, based at 2f4482ded066d9c5d9c59130377907f7fd2916c9 (job/merge-round-7a). I am reading the full owned issue set and audit findings, then I will merge the shared mutation helper before implementing the Undo paths.

Started implementation on `job/undo-a11y`, based at `2f4482ded066d9c5d9c59130377907f7fd2916c9` (`job/merge-round-7a`). I am reading the full owned issue set and audit findings, then I will merge the shared mutation helper before implementing the Undo paths.
Author
Owner

Additional source check: the global Search palette duplicates Unpin and Trash in search-dialog.svelte, so both surfaces need the same Undo path. The saved-search DELETE drops Root::trash's unique Trash name and returns 204. The #667 receipt guide says a SQLite receipt cannot make a filesystem rename atomic; I will use the Trash recovery identity and reject Undo if the saved search changed after removal, so Undo cannot overwrite a newer query edit.

Additional source check: the global Search palette duplicates Unpin and Trash in `search-dialog.svelte`, so both surfaces need the same Undo path. The saved-search DELETE drops `Root::trash`'s unique Trash name and returns 204. The #667 receipt guide says a SQLite receipt cannot make a filesystem rename atomic; I will use the Trash recovery identity and reject Undo if the saved search changed after removal, so Undo cannot overwrite a newer query edit.
Author
Owner

Saved Search Undo implementation is in progress on job/undo-a11y. It now adds checked Unpin and Trash restore routes, store methods, and shared 8-second Undo toasts in the sidebar and search dialog.

The new store regression first failed because the client sent the entire display receipt (including saved_search) where the Undo API accepts only the server-issued hash plus Version or Trash identity. The client now sends only those two authority fields. Focused Vitest result: Test Files 1 passed (1); Tests 4 passed (4).

Decision: keep the file operations in calternal-fs and use its Version and Trash journal with content-hash checks. The DB mutation receipt transaction cannot safely include filesystem writes; routing this through the generic DB receipt would split the file write and receipt commit. The checked Files journal preserves atomic writes and refuses Undo after a later edit.

Saved Search Undo implementation is in progress on `job/undo-a11y`. It now adds checked Unpin and Trash restore routes, store methods, and shared 8-second Undo toasts in the sidebar and search dialog. The new store regression first failed because the client sent the entire display receipt (including `saved_search`) where the Undo API accepts only the server-issued hash plus Version or Trash identity. The client now sends only those two authority fields. Focused Vitest result: `Test Files 1 passed (1); Tests 4 passed (4)`. Decision: keep the file operations in `calternal-fs` and use its Version and Trash journal with content-hash checks. The DB mutation receipt transaction cannot safely include filesystem writes; routing this through the generic DB receipt would split the file write and receipt commit. The checked Files journal preserves atomic writes and refuses Undo after a later edit.
Author
Owner

Regression review found that the Unpin Undo route accepted any valid older pinned version for the same saved-search ID. A stale or edited receipt could therefore restore an older query. The route now also checks the previous version's name, query and creation identity against the unchanged current file. unpin_undo_rejects_a_different_historical_search_version covers the rejected substitution. The Files Version and Trash journals remain the source of the restore bytes; the shared database mutation transaction explicitly excludes filesystem writes, so it cannot safely commit with these file mutations.

Regression review found that the Unpin Undo route accepted any valid older pinned version for the same saved-search ID. A stale or edited receipt could therefore restore an older query. The route now also checks the previous version's name, query and creation identity against the unchanged current file. `unpin_undo_rejects_a_different_historical_search_version` covers the rejected substitution. The Files Version and Trash journals remain the source of the restore bytes; the shared database mutation transaction explicitly excludes filesystem writes, so it cannot safely commit with these file mutations.
Author
Owner

Stopped at the four-hour limit. This is a progress report, not a completion report.

Branch: job/undo-a11y. Current HEAD: 85c139b8764050932151e3dc004b667d761285f4.

Built on this branch

  • #742 Search scope actions are keyboard and touch accessible (539cad033).
  • #773 Search deep links restore the selected search (d1460f667).
  • #772 calendar format settings preserve the confirmed selection (85c139b87).
  • #758 saved-search Unpin and Trash Undo routes, checked Files Version/Trash receipts, web store/toast updates, regression and adversarial probes, and a mutation benchmark profile are present but uncommitted. The Rust test command was interrupted at the deadline, so this slice is not verified or complete.

Gate output

cargo fmt --all --check exited 0 with no output.

cargo clippy -p calternal-search --all-targets -- -D warnings:

Finished `dev` profile [unoptimized + debuginfo] target(s) in 3m 14s

The first clippy run reported private-interface visibility, a redundant closure and a redundant cast. Those were fixed before the passing run.

Focused saved-search web test:

Test Files  1 passed (1)
     Tests  4 passed (4)
   Start at 19:19:58
   Duration 84.58s (transform 51%, environment 38%, setup 8%, import 3%)

node --check apps/web/e2e/search.mjs, node --check bench/mutation-receipts.mjs, python3 -m py_compile tests/adversarial/attack.py, and git diff --check exited 0 with no output.

cargo test -p calternal-search was stopped at the time limit while compiling dependencies; it exited 130 and produced no test results. The OpenAPI/client generation, server and other crate gates, bun run check, full bun run test, local-server adversarial round, production E2E/screenshots, performance run, final origin/dev and job/merge-round-7a merges, and cleanup remain undone.

Decisions

Saved-search Undo uses calternal-fs Versions and Trash receipts with content-hash checks. The shared database mutation transaction explicitly excludes filesystem work, so it cannot safely commit atomically with these file mutations. Unpin Undo also checks that the selected version retains the current search name, query and creation identity.

UX gaps

Saved-search UI work offers keyboard-accessible Undo, retains the same ID/name/query/pin state, and restores a missing item into the shared store. The required production screenshots at 390/820/1440, light/dark, with macOS platform hints were not captured. The saved-search UI/code remains uncommitted and needs the generated API client before type-checking.

Other owned slices are in separate worktrees and were not integrated: #740/#744/#745/#819/#820 commits are available on undo-a11y-a11y; #768/#776 commits are available on undo-a11y-reminders; #770 commits are available on undo-a11y-filesphotos. #741 and #775 remain uncommitted and incomplete in their worktrees. Their agents' final status messages record exact commits and gaps.

Stopped at the four-hour limit. This is a progress report, not a completion report. Branch: `job/undo-a11y`. Current HEAD: `85c139b8764050932151e3dc004b667d761285f4`. ## Built on this branch - #742 Search scope actions are keyboard and touch accessible (`539cad033`). - #773 Search deep links restore the selected search (`d1460f667`). - #772 calendar format settings preserve the confirmed selection (`85c139b87`). - #758 saved-search Unpin and Trash Undo routes, checked Files Version/Trash receipts, web store/toast updates, regression and adversarial probes, and a mutation benchmark profile are present but uncommitted. The Rust test command was interrupted at the deadline, so this slice is not verified or complete. ## Gate output `cargo fmt --all --check` exited 0 with no output. `cargo clippy -p calternal-search --all-targets -- -D warnings`: ```text Finished `dev` profile [unoptimized + debuginfo] target(s) in 3m 14s ``` The first clippy run reported private-interface visibility, a redundant closure and a redundant cast. Those were fixed before the passing run. Focused saved-search web test: ```text Test Files 1 passed (1) Tests 4 passed (4) Start at 19:19:58 Duration 84.58s (transform 51%, environment 38%, setup 8%, import 3%) ``` `node --check apps/web/e2e/search.mjs`, `node --check bench/mutation-receipts.mjs`, `python3 -m py_compile tests/adversarial/attack.py`, and `git diff --check` exited 0 with no output. `cargo test -p calternal-search` was stopped at the time limit while compiling dependencies; it exited 130 and produced no test results. The OpenAPI/client generation, server and other crate gates, `bun run check`, full `bun run test`, local-server adversarial round, production E2E/screenshots, performance run, final `origin/dev` and `job/merge-round-7a` merges, and cleanup remain undone. ## Decisions Saved-search Undo uses `calternal-fs` Versions and Trash receipts with content-hash checks. The shared database mutation transaction explicitly excludes filesystem work, so it cannot safely commit atomically with these file mutations. Unpin Undo also checks that the selected version retains the current search name, query and creation identity. ## UX gaps Saved-search UI work offers keyboard-accessible Undo, retains the same ID/name/query/pin state, and restores a missing item into the shared store. The required production screenshots at 390/820/1440, light/dark, with macOS platform hints were not captured. The saved-search UI/code remains uncommitted and needs the generated API client before type-checking. Other owned slices are in separate worktrees and were not integrated: #740/#744/#745/#819/#820 commits are available on `undo-a11y-a11y`; #768/#776 commits are available on `undo-a11y-reminders`; #770 commits are available on `undo-a11y-filesphotos`. #741 and #775 remain uncommitted and incomplete in their worktrees. Their agents' final status messages record exact commits and gaps.
Author
Owner

Progress report for job undo-a11y. Branch job/undo-a11y, HEAD ce638ca2d499465ea1f04f1db9eac90c0db0c14c. No push, no deploy.

#758 saved-search Undo: done and verified

  • The WIP checkpoint is replaced by 7dbf97c3c. Unpin Undo restores the exact pre-unpin Files Version only while the file hash still matches and the Version has the same name, query and creation time. Trash Undo restores the journaled Trash entry only while its bytes match and the ID is free. A later edit or a new occupant returns 409.
  • Bug found and fixed: Trash Undo returned 500. It built a .Trash/files/... RelPath, which calternal-fs rejects as reserved. calternal-fs now has Root::read_trash(home, name) (directory-handle-relative, name checked by trash_original). The saved-search route reads through it with a byte bound.
  • OpenAPI contract and API client regenerated. Sidebar and Search dialog use one shared Undo toast.
  • Also fixed on this branch: the #667 merge did not compile (mail_read_marking was re-exported while gated to cfg(test)), and /search/+page.svelte dereferenced a possibly null to (#773).

Other slices merged into this branch

The sibling slice branches were merged and then verified here: reminders/inbox (#768, #776), Files Version Undo (#770), and a11y (#740, #744, #745, #819, #820). The uncommitted #775 (burst key photo) and #741 (PDF text layer) work was ported, fixed and committed. Fixes made while verifying:

  • notes restore route: borrow error (did not compile).
  • notifications: removed dead duplicate Done and Snooze store functions. The Undo test assumed the inbox row order of two rows created in the same millisecond, so it now finds rows by ID.
  • files test: called std::fs::read on a File.
  • photos: 10 compile errors against the quick-xml 0.42 &str API.
  • PdfView: the dynamic .pdf-link styles were scoped away, so links had no position. They are now :global under the layer. Line height now uses a token.
  • TooltipLayer: fixed a null narrowing error.

#743 is not in this job. It is owned by job/scopefix.

Gates

  • cargo fmt --all --check: exit 0
  • cargo clippy -p calternal-fs -p calternal-search -p calternal-plugin-photos -p calternal-server --all-targets -- -D warnings: Finished dev profile [unoptimized + debuginfo] target(s) in 12m 15s
  • cargo clippy for the notes, notifications, files, photos and mail plugins: Finished dev profile [unoptimized + debuginfo] target(s) in 4m 16s
  • cargo test -p calternal-search: test result: ok. 44 passed; 0 failed; 1 ignored
  • files test result: ok. 157 passed; mail ok. 48 passed; notifications ok. 28 passed (3 runs); photos ok. 51 passed; notes ok. 184 passed with --test-threads=1
  • bun run check: COMPLETED 2008 FILES 0 ERRORS 0 WARNINGS 0 FILES_WITH_PROBLEMS
  • bun run test --maxWorkers=2: Tests 1 failed | 1107 passed (1108)

Known failures that come from the #667 merge, not this job's slices

  • MailSection.svelte.test.ts > saves the Mail read-marking preference through the server fails, also when run alone. The #667 preference write no longer matches the old apiFetch('/api/v1/mail/preferences','patch',{body}) assertion. The test needs an update by the #667 owner. The assertion was not weakened here.
  • notes daily_and_composer_preserve_unrelated_bytes fails only in the parallel suite (GET /journal returns 404). It passes alone and with one thread. This job changed notes only in the reminder restore route.

Not done

No real-server adversarial round, production screenshots (390/820/1440, light/dark) or bench run.

Progress report for job undo-a11y. Branch `job/undo-a11y`, HEAD `ce638ca2d499465ea1f04f1db9eac90c0db0c14c`. No push, no deploy. ## #758 saved-search Undo: done and verified - The WIP checkpoint is replaced by `7dbf97c3c`. Unpin Undo restores the exact pre-unpin Files Version only while the file hash still matches and the Version has the same name, query and creation time. Trash Undo restores the journaled Trash entry only while its bytes match and the ID is free. A later edit or a new occupant returns 409. - Bug found and fixed: Trash Undo returned 500. It built a `.Trash/files/...` RelPath, which calternal-fs rejects as reserved. calternal-fs now has `Root::read_trash(home, name)` (directory-handle-relative, name checked by `trash_original`). The saved-search route reads through it with a byte bound. - OpenAPI contract and API client regenerated. Sidebar and Search dialog use one shared Undo toast. - Also fixed on this branch: the #667 merge did not compile (`mail_read_marking` was re-exported while gated to `cfg(test)`), and `/search/+page.svelte` dereferenced a possibly null `to` (#773). ## Other slices merged into this branch The sibling slice branches were merged and then verified here: reminders/inbox (#768, #776), Files Version Undo (#770), and a11y (#740, #744, #745, #819, #820). The uncommitted #775 (burst key photo) and #741 (PDF text layer) work was ported, fixed and committed. Fixes made while verifying: - notes restore route: borrow error (did not compile). - notifications: removed dead duplicate Done and Snooze store functions. The Undo test assumed the inbox row order of two rows created in the same millisecond, so it now finds rows by ID. - files test: called `std::fs::read` on a File. - photos: 10 compile errors against the quick-xml 0.42 `&str` API. - PdfView: the dynamic `.pdf-link` styles were scoped away, so links had no position. They are now `:global` under the layer. Line height now uses a token. - TooltipLayer: fixed a null narrowing error. #743 is not in this job. It is owned by `job/scopefix`. ## Gates - `cargo fmt --all --check`: exit 0 - `cargo clippy -p calternal-fs -p calternal-search -p calternal-plugin-photos -p calternal-server --all-targets -- -D warnings`: `Finished `dev` profile [unoptimized + debuginfo] target(s) in 12m 15s` - `cargo clippy` for the notes, notifications, files, photos and mail plugins: `Finished `dev` profile [unoptimized + debuginfo] target(s) in 4m 16s` - `cargo test -p calternal-search`: `test result: ok. 44 passed; 0 failed; 1 ignored` - files `test result: ok. 157 passed`; mail `ok. 48 passed`; notifications `ok. 28 passed` (3 runs); photos `ok. 51 passed`; notes `ok. 184 passed` with `--test-threads=1` - `bun run check`: `COMPLETED 2008 FILES 0 ERRORS 0 WARNINGS 0 FILES_WITH_PROBLEMS` - `bun run test --maxWorkers=2`: `Tests 1 failed | 1107 passed (1108)` ## Known failures that come from the #667 merge, not this job's slices - `MailSection.svelte.test.ts > saves the Mail read-marking preference through the server` fails, also when run alone. The #667 preference write no longer matches the old `apiFetch('/api/v1/mail/preferences','patch',{body})` assertion. The test needs an update by the #667 owner. The assertion was not weakened here. - notes `daily_and_composer_preserve_unrelated_bytes` fails only in the parallel suite (GET /journal returns 404). It passes alone and with one thread. This job changed notes only in the reminder restore route. ## Not done No real-server adversarial round, production screenshots (390/820/1440, light/dark) or bench run.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal#758
No description provided.