Mail: read-state changes leave sidebar unread counts stale #771

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

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

Decided behavior

DESIGN §45 (read-state UI) and the owner UX completeness rule of 2026-10-02 require an edit to appear at once everywhere the item is shown.

Evidence

apps/web/src/lib/mail/MailView.svelte:234–247 updates only its own messages, threadMessages and detail after the read-state POST. apps/web/src/lib/mail/MailSidebar.svelte:33 derives Inbox unread from its own folders array. Its one load at lines 48–61 fetches folders. Its poll at lines 64–88 only calls loadMailSyncStatuses and updates syncStatuses, not folders. It has no read-state event or shared store subscription.

Observed behavior

Opening an unread message or choosing Mark as read changes the reader, but leaves the Inbox and provider-folder unread badges based on the original folder response while the sidebar stays mounted. Incoming mail has the same invalidation gap.

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

Expected

Use shared Mail state or the shared change feed to update folder counts and Inbox count after a committed read-state change and sync. Do not add a separate full-folder polling loop. Keep fast selection and focus unchanged.

Test idea

Keep the sidebar mounted, open an unread message, wait for automatic read marking, and check both counts without reload. Repeat Mark unread and a provider-side delivery. Verify that a failed write restores the prior counts.

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 and unread results. #396 covers the original reader; #613 covers sync status. Neither describes stale unread badges after a committed read-state change. #672 is the broad revision/delta architecture ticket.

Found during the source review for #427 on `origin/dev` at `c4a61e8cf090170f35b1bed3350d9de20c83ecd5`. ## Decided behavior DESIGN §45 (read-state UI) and the owner UX completeness rule of 2026-10-02 require an edit to appear at once everywhere the item is shown. ## Evidence `apps/web/src/lib/mail/MailView.svelte:234–247` updates only its own messages, threadMessages and detail after the read-state POST. `apps/web/src/lib/mail/MailSidebar.svelte:33` derives Inbox unread from its own folders array. Its one load at lines 48–61 fetches folders. Its poll at lines 64–88 only calls loadMailSyncStatuses and updates syncStatuses, not folders. It has no read-state event or shared store subscription. ## Observed behavior Opening an unread message or choosing Mark as read changes the reader, but leaves the Inbox and provider-folder unread badges based on the original folder response while the sidebar stays mounted. Incoming mail has the same invalidation gap. This is a source finding. No production build or live-server run was made in this review job. ## Expected Use shared Mail state or the shared change feed to update folder counts and Inbox count after a committed read-state change and sync. Do not add a separate full-folder polling loop. Keep fast selection and focus unchanged. ## Test idea Keep the sidebar mounted, open an unread message, wait for automatic read marking, and check both counts without reload. Repeat Mark unread and a provider-side delivery. Verify that a failed write restores the prior counts. 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 and unread results. #396 covers the original reader; #613 covers sync status. Neither describes stale unread badges after a committed read-state change. #672 is the broad revision/delta architecture ticket.
Author
Owner

Starting implementation on job/mailperf, based on job/merge-round-7a at 2f4482ded066d9c5d9c59130377907f7fd2916c9. The target areas are IDLE scheduling, bounded delta/expunge work, and unread-count invalidation. I will merge origin/dev and job/merge-round-7a before the final gates.

Starting implementation on `job/mailperf`, based on `job/merge-round-7a` at `2f4482ded066d9c5d9c59130377907f7fd2916c9`. The target areas are IDLE scheduling, bounded delta/expunge work, and unread-count invalidation. I will merge `origin/dev` and `job/merge-round-7a` before the final gates.
Author
Owner

Implemented in c6c4f7176: committed message windows, Seen changes and expunge pages publish folder invalidations after the Index commit. The User-scoped Mail SSE route returns only {"scope":"folders"}, includes an initial refresh and invalidates after event lag. The sidebar shares one EventSource and rereads folder counts. Route/read-state integration tests passed in the pre-merge Mail suite; the real-server adversarial run and Mac production screenshots remain outstanding at the four-hour cap.

Implemented in `c6c4f7176`: committed message windows, Seen changes and expunge pages publish folder invalidations after the Index commit. The User-scoped Mail SSE route returns only `{"scope":"folders"}`, includes an initial refresh and invalidates after event lag. The sidebar shares one EventSource and rereads folder counts. Route/read-state integration tests passed in the pre-merge Mail suite; the real-server adversarial run and Mac production screenshots remain outstanding at the four-hour cap.
Author
Owner

Independent read-only review complete.

Review branch: job/rev2-mailperf; base c4faf184df726a9375ae0c13bdfb6018ac2cf57e.
Review HEAD: 7bf33b3bb2528d357f92ad3073f0350cff9ea0f5.
Reviewed branch: job/mailperf, HEAD 2e724c0529ef38623fd86c239d638371be5d9702, compared with origin/dev at the base SHA above.

Built: review documents only. Files: audit-findings.md and review-mailperf.md. Product code was not changed. The documents are committed in three atomic commits. No push, deploy or merge was made.

Findings

Five P2 findings. No P1 or P3 finding was confirmed.

Finding Source evidence Existing owner
F1: quiet IDLE timeout cycles do not reconcile other folders or missed arrivals crates/plugins/mail/src/sync.rs:290, :1554, :1589 #753
F2: a manual sync can start a second listener for the same Connected Account crates/plugins/mail/src/sync.rs:95, :195, :402 #753
F3: out-of-order folder reads restore stale unread badges apps/web/src/lib/mail/MailSidebar.svelte:74, apps/web/src/lib/mail/live.ts:14 #771
F4: staged expunge count and cleanup hold the writer for the full set crates/plugins/mail/src/cache/store.rs:1048, :1078, :1245 #757
F5: arrivals reset all progress in a multi-run expunge scan crates/plugins/mail/src/cache/store.rs:1072, crates/plugins/mail/src/sync.rs:930 #757

All-state issue searches were made before recording defects. Evidence for F1/F2 was added to #753 and F4/F5 to #757. #763 is already the closed duplicate of #753. No new issue was created.

F3 detail for #771: every stream callback starts readFolders and every response assigns folders if the component is still active. The 100 ms event batch does not prevent overlapping requests. Read A can capture unread count 3, then read B captures 2 after the next commit. If B resolves first and A last, both badges return to 3. The completed-sync status poll does not read folders, so this stays stale if no more event occurs. This violates DESIGN §45 and the owner rule that edits appear everywhere. Use the shared revision/snapshot ordering primitive or one active read with a dirty flag and a guaranteed final refresh. Add a test with independent immutable snapshots and deferred promises that resolves the newer read first. The current mutable-getter test does not cover this ordering.

Source checks and limits

IDLE and sync now have distinct Worker kind limits. UID pages commit data with their cursors. Expunge staging, validation and deletion use durable state with generation/UIDVALIDITY checks. No separate crash-induced lost or duplicate message defect was confirmed from the page transactions. The SSE route filters the authenticated User ID and sends no message data; no cross-User disclosure was confirmed in that stream.

No build, test, server, browser or performance measurement was run. The LIGHT brief forbids them, so Rust/web gate output is absent. git diff --check returned exit 0 with no output. Final git status --short also had no output. These checks cover only the review documents, not product behavior.

For the merge round

Exact crate gates and focused web command are recorded in review-mailperf.md: cargo fmt --check; cargo clippy -p calternal-plugin-mail --all-targets -- -D warnings; cargo test -p calternal-plugin-mail; server clippy/test; bun run check; bunx vitest run src/lib/mail/MailSidebar.svelte.test.ts --maxWorkers=2.

Add and run F1–F5 regressions. Prove quiet timeout recovery, one listener per account, per-User progress, unread response ordering, bounded cleanup and convergence with arrivals. Exercise crash recovery after staged page, validation and partial prune commits. Run the focused Mail adversarial probe through the shared harness, production macOS screenshots at all required widths/themes, and the Mail profile on the locked perf VM. The existing profile starts isolated test processes and does not prove mixed-account fairness in one server.

Known gaps and decisions

All five defects need an implementation job and execution evidence. No product design decision was made. Existing issues own the findings. The specific read-only LIGHT brief takes precedence over general build and merge instructions. DESIGN §58 is absent from this review worktree; it was read in the target branch and covers agent discovery. Mail requirements come from §45 and §53.

UX gaps closed: none, because this job changes documents only. UX gaps left: stale unread badges, missed provider arrivals and stale expunged memberships described above.

Independent read-only review complete. Review branch: `job/rev2-mailperf`; base `c4faf184df726a9375ae0c13bdfb6018ac2cf57e`. Review HEAD: `7bf33b3bb2528d357f92ad3073f0350cff9ea0f5`. Reviewed branch: `job/mailperf`, HEAD `2e724c0529ef38623fd86c239d638371be5d9702`, compared with `origin/dev` at the base SHA above. Built: review documents only. Files: `audit-findings.md` and `review-mailperf.md`. Product code was not changed. The documents are committed in three atomic commits. No push, deploy or merge was made. ## Findings Five P2 findings. No P1 or P3 finding was confirmed. | Finding | Source evidence | Existing owner | | --- | --- | --- | | F1: quiet IDLE timeout cycles do not reconcile other folders or missed arrivals | `crates/plugins/mail/src/sync.rs:290`, `:1554`, `:1589` | #753 | | F2: a manual sync can start a second listener for the same Connected Account | `crates/plugins/mail/src/sync.rs:95`, `:195`, `:402` | #753 | | F3: out-of-order folder reads restore stale unread badges | `apps/web/src/lib/mail/MailSidebar.svelte:74`, `apps/web/src/lib/mail/live.ts:14` | #771 | | F4: staged expunge count and cleanup hold the writer for the full set | `crates/plugins/mail/src/cache/store.rs:1048`, `:1078`, `:1245` | #757 | | F5: arrivals reset all progress in a multi-run expunge scan | `crates/plugins/mail/src/cache/store.rs:1072`, `crates/plugins/mail/src/sync.rs:930` | #757 | All-state issue searches were made before recording defects. Evidence for F1/F2 was added to #753 and F4/F5 to #757. #763 is already the closed duplicate of #753. No new issue was created. **F3 detail for #771:** every stream callback starts readFolders and every response assigns folders if the component is still active. The 100 ms event batch does not prevent overlapping requests. Read A can capture unread count 3, then read B captures 2 after the next commit. If B resolves first and A last, both badges return to 3. The completed-sync status poll does not read folders, so this stays stale if no more event occurs. This violates DESIGN §45 and the owner rule that edits appear everywhere. Use the shared revision/snapshot ordering primitive or one active read with a dirty flag and a guaranteed final refresh. Add a test with independent immutable snapshots and deferred promises that resolves the newer read first. The current mutable-getter test does not cover this ordering. ## Source checks and limits IDLE and sync now have distinct Worker kind limits. UID pages commit data with their cursors. Expunge staging, validation and deletion use durable state with generation/UIDVALIDITY checks. No separate crash-induced lost or duplicate message defect was confirmed from the page transactions. The SSE route filters the authenticated User ID and sends no message data; no cross-User disclosure was confirmed in that stream. No build, test, server, browser or performance measurement was run. The LIGHT brief forbids them, so Rust/web gate output is absent. `git diff --check` returned exit 0 with no output. Final `git status --short` also had no output. These checks cover only the review documents, not product behavior. ## For the merge round Exact crate gates and focused web command are recorded in `review-mailperf.md`: `cargo fmt --check`; `cargo clippy -p calternal-plugin-mail --all-targets -- -D warnings`; `cargo test -p calternal-plugin-mail`; server clippy/test; `bun run check`; `bunx vitest run src/lib/mail/MailSidebar.svelte.test.ts --maxWorkers=2`. Add and run F1–F5 regressions. Prove quiet timeout recovery, one listener per account, per-User progress, unread response ordering, bounded cleanup and convergence with arrivals. Exercise crash recovery after staged page, validation and partial prune commits. Run the focused Mail adversarial probe through the shared harness, production macOS screenshots at all required widths/themes, and the Mail profile on the locked perf VM. The existing profile starts isolated test processes and does not prove mixed-account fairness in one server. ## Known gaps and decisions All five defects need an implementation job and execution evidence. No product design decision was made. Existing issues own the findings. The specific read-only LIGHT brief takes precedence over general build and merge instructions. DESIGN §58 is absent from this review worktree; it was read in the target branch and covers agent discovery. Mail requirements come from §45 and §53. UX gaps closed: none, because this job changes documents only. UX gaps left: stale unread badges, missed provider arrivals and stale expunged memberships described above.
Author
Owner

Review findings F1–F5 fixed (job/mailperf, head 7a22fbadecfc273f7b128867e3fbeb0265f7a700)

This fixes the five P2 findings from the independent review. Each fix has a regression test. I confirmed that each test fails when the old behavior is put back.

Finding Fix Regression test (fails on old code)
F1: quiet IDLE never reconciles (#753) A complete sync, and every listener cycle, make sure the account has one durable reconciliation poll: a delayed mail.sync job with run_at +5 min that does not depend on IDLE. Before it waits, the listener compares EXAMINE UIDVALIDITY/UIDNEXT with the committed delta cursor. Mail that arrived before IDLE started is then reported at once. A provider without IDLE gets no successor listener, so no slot sits in a polling sleep. quiet_idle_cycles_keep_one_durable_reconciliation_poll, quiet_listener_starts_the_poll_for_older_accounts, listener_reports_mail_that_arrived_before_idle
F2: manual sync starts a second listener (#753) ensure_listener reads both keys of the a/b chain (new JobQueue::active_by_dedup_keys) under the account lock. It adds a listener only when none is active. If a duplicate chain from an older build exists, the leased listener with the larger job ID yields. manual_sync_keeps_one_listener_per_account, active_jobs_are_read_across_a_dedup_key_pair (calternal-db)
F3: stale unread badges from reordered reads (#771) The sidebar runs one folder read at a time. An invalidation during a read sets a dirty flag, which causes one more read after it. An older response can no longer overwrite a newer one, and no polling was added. keeps the latest committed unread count when folder reads finish out of order (MailSidebar.svelte.test.ts)
F4: full-set writer transactions (#757) Each staged page commit now keeps expunge_staged_count, so validation is O(1). Finished, failed and reopened sweeps, and UIDVALIDITY changes, set expunge_cleanup and do not delete rows. cleanup_expunge_page then deletes at most 1,024 rows per transaction, and each page costs one window. No sweep can start or stage rows until cleanup is done. expunge_cleanup_is_bounded_resumable_and_blocks_new_sweeps (includes crash injection in a staged page commit and in the final cleanup commit)
F5: arrivals discard scan progress (#757) Validation runs one bounded UID SEARCH top+1:latest to count live UIDs above the fixed sweep top, then compares the staged count with EXISTS - arrivals. UIDs only grow within one UIDVALIDITY, so arrivals cannot enter the scanned range. A burst larger than 102,400 UIDs, or expunges during the scan, still reset the sweep and send its rows to bounded cleanup. Expunges during the scan no longer cause a Protocol error. expunge_sweep_converges_while_new_mail_keeps_arriving: one new message arrives at every EXAMINE, runs use budgets of 2–5 windows so they stop after every commit type, and the expunged UID goes away with no message lost or duplicated. Also expunge_validation_keeps_the_scan_when_only_new_uids_arrive.

Migration 0010 (new on this branch, not on dev) gets two more columns: expunge_staged_count and expunge_cleanup. I also fixed a clippy failure that was already on the branch (reconcile_expunged_uids had 9 arguments) and a svelte-check error that was already in MailSidebar.svelte.test.ts.

Gates (verbatim summary lines)

  • cargo fmt --check: exit 0
  • cargo clippy -p calternal-plugin-mail -p calternal-db --all-targets -- -D warnings: Finished \dev` profile [unoptimized + debuginfo] target(s)`
  • cargo test -p calternal-plugin-mail: test result: ok. 61 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out
  • cargo test -p calternal-db: test result: ok. 20 passed; 0 failed; 0 ignored / test result: ok. 17 passed; 0 failed; 1 ignored / test result: ok. 1 passed; 0 failed; 0 ignored
  • bun run check: COMPLETED 1994 FILES 0 ERRORS 0 WARNINGS 0 FILES_WITH_PROBLEMS
  • bun run test --maxWorkers=2: Test Files 154 passed (154), Tests 1075 passed (1075)

I did not touch calternal-server. The new calternal-db method is additive, so I did not run the server gates. No HTTP API changed. Still open for the merge round: the mixed-account bench/mail-sync.py profile on the perf VM, and screenshots of the badges after read/unread edits on a production build.

## Review findings F1–F5 fixed (job/mailperf, head `7a22fbadecfc273f7b128867e3fbeb0265f7a700`) This fixes the five P2 findings from the independent review. Each fix has a regression test. I confirmed that each test fails when the old behavior is put back. | Finding | Fix | Regression test (fails on old code) | | --- | --- | --- | | F1: quiet IDLE never reconciles (#753) | A complete sync, and every listener cycle, make sure the account has one durable reconciliation poll: a delayed `mail.sync` job with `run_at` +5 min that does not depend on IDLE. Before it waits, the listener compares EXAMINE UIDVALIDITY/UIDNEXT with the committed delta cursor. Mail that arrived before IDLE started is then reported at once. A provider without IDLE gets no successor listener, so no slot sits in a polling sleep. | `quiet_idle_cycles_keep_one_durable_reconciliation_poll`, `quiet_listener_starts_the_poll_for_older_accounts`, `listener_reports_mail_that_arrived_before_idle` | | F2: manual sync starts a second listener (#753) | `ensure_listener` reads both keys of the a/b chain (new `JobQueue::active_by_dedup_keys`) under the account lock. It adds a listener only when none is active. If a duplicate chain from an older build exists, the leased listener with the larger job ID yields. | `manual_sync_keeps_one_listener_per_account`, `active_jobs_are_read_across_a_dedup_key_pair` (calternal-db) | | F3: stale unread badges from reordered reads (#771) | The sidebar runs one folder read at a time. An invalidation during a read sets a dirty flag, which causes one more read after it. An older response can no longer overwrite a newer one, and no polling was added. | `keeps the latest committed unread count when folder reads finish out of order` (MailSidebar.svelte.test.ts) | | F4: full-set writer transactions (#757) | Each staged page commit now keeps `expunge_staged_count`, so validation is O(1). Finished, failed and reopened sweeps, and UIDVALIDITY changes, set `expunge_cleanup` and do not delete rows. `cleanup_expunge_page` then deletes at most 1,024 rows per transaction, and each page costs one window. No sweep can start or stage rows until cleanup is done. | `expunge_cleanup_is_bounded_resumable_and_blocks_new_sweeps` (includes crash injection in a staged page commit and in the final cleanup commit) | | F5: arrivals discard scan progress (#757) | Validation runs one bounded `UID SEARCH top+1:latest` to count live UIDs above the fixed sweep top, then compares the staged count with `EXISTS - arrivals`. UIDs only grow within one UIDVALIDITY, so arrivals cannot enter the scanned range. A burst larger than 102,400 UIDs, or expunges during the scan, still reset the sweep and send its rows to bounded cleanup. Expunges during the scan no longer cause a Protocol error. | `expunge_sweep_converges_while_new_mail_keeps_arriving`: one new message arrives at every EXAMINE, runs use budgets of 2–5 windows so they stop after every commit type, and the expunged UID goes away with no message lost or duplicated. Also `expunge_validation_keeps_the_scan_when_only_new_uids_arrive`. | Migration 0010 (new on this branch, not on dev) gets two more columns: `expunge_staged_count` and `expunge_cleanup`. I also fixed a clippy failure that was already on the branch (`reconcile_expunged_uids` had 9 arguments) and a svelte-check error that was already in MailSidebar.svelte.test.ts. ### Gates (verbatim summary lines) - `cargo fmt --check`: exit 0 - `cargo clippy -p calternal-plugin-mail -p calternal-db --all-targets -- -D warnings`: `Finished \`dev\` profile [unoptimized + debuginfo] target(s)` - `cargo test -p calternal-plugin-mail`: `test result: ok. 61 passed; 0 failed; 3 ignored; 0 measured; 0 filtered out` - `cargo test -p calternal-db`: `test result: ok. 20 passed; 0 failed; 0 ignored` / `test result: ok. 17 passed; 0 failed; 1 ignored` / `test result: ok. 1 passed; 0 failed; 0 ignored` - `bun run check`: `COMPLETED 1994 FILES 0 ERRORS 0 WARNINGS 0 FILES_WITH_PROBLEMS` - `bun run test --maxWorkers=2`: `Test Files 154 passed (154)`, `Tests 1075 passed (1075)` I did not touch calternal-server. The new calternal-db method is additive, so I did not run the server gates. No HTTP API changed. Still open for the merge round: the mixed-account `bench/mail-sync.py` profile on the perf VM, and screenshots of the badges after read/unread edits on a production build.
Author
Owner

Starting #771 follow-up on branch job/mailperf at 7a22fbadecfc273f7b128867e3fbeb0265f7a700. I will merge origin/dev first as requested, then add regression tests and fixes for the five independent review findings. The issue contract and relevant Mail design sections are being checked now.

Starting #771 follow-up on branch `job/mailperf` at `7a22fbadecfc273f7b128867e3fbeb0265f7a700`. I will merge `origin/dev` first as requested, then add regression tests and fixes for the five independent review findings. The issue contract and relevant Mail design sections are being checked now.
Author
Owner

The independent review found F3 at predecessor SHA 2e724c0529ef38623fd86c239d638371be5d9702. Current MailSidebar.svelte serializes folder reads and marks the state dirty when another invalidation arrives during a read, guaranteeing a trailing snapshot read. MailSidebar.svelte.test.ts uses immutable folder snapshots, delays two reads, and resolves them out of order; it asserts both Inbox and folder badges keep the latest count. Source/test review only so far; requested Mail gates are next.

The independent review found F3 at predecessor SHA `2e724c0529ef38623fd86c239d638371be5d9702`. Current `MailSidebar.svelte` serializes folder reads and marks the state dirty when another invalidation arrives during a read, guaranteeing a trailing snapshot read. `MailSidebar.svelte.test.ts` uses immutable folder snapshots, delays two reads, and resolves them out of order; it asserts both Inbox and folder badges keep the latest count. Source/test review only so far; requested Mail gates are next.
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#771
No description provided.