Audit follow-ups: keyboard, wording, performance, transfer, sync, retry and relay fixes #22

2026-10-02T12:47:33Z

jaap-jan commented

2026-10-02 12:47:33 +00:00

The rest of the 2026-10-02 full sweep. The critical and high items went straight to main in a4b81da. This branch covers the UI, performance and lower-severity findings: 21 commits, one per fix.

Each fix has a regression test, with the three noted below as exceptions. Where marked (fails without), the test was also run against the original code and failed there.

Server

A retried write transaction committed nothing (fails without). EF's retrying strategy re-runs the delegate on the same context. If the commit was lost after SaveChanges succeeded, every entity was already marked unchanged, so the retry saved nothing and returned success. A team hand-over answered 204 with no roles changed. FreshAttempts.ExecuteFreshAsync now clears the tracker on a retry and reloads the entities the attempt carries in. It covers all five write transactions (push, enrolment, team archive, hand-over, member removal). The test uses an interceptor that fails one commit.
Relay sessions survived losing access (fails without). RelayAccessWatch re-checks a user's open relay sessions when their access changes and closes lost ones with the new code 4403 / AccessRevoked. Before this, sessions ran on for up to 12 hours.
A team could be archived while a vault was created in it. Both operations now take a per-team advisory lock and re-check inside their transaction. No race test.

Sync

Deleting a create that never landed (fails without). This used to leave a permanent "Rejected" conflict. When the server answers that it has no such item, the delete is now treated as done. A create that did land behaves as before (the delete gives way, and the user is told).
A pulled page was applied one change at a time. It is now mirrored in one SQLite write, and the outbox is read once per page.

Desktop and SSH

Keyboard:
- The closed-vault cards take focus on launch, and Enter signs in and enrols (fails without).
- The host-key, delete and sign-out questions focus their harmless answer, and Escape says no (fails without).
- Arrow-key selection on the Hosts board reaches the vault, and Enter connects (fails without).
Wording: device unlock is named for the platform (Windows Hello, Touch ID, device PIN).
Layout:
- Session tab labels are trimmed.
- VAULT columns that are always empty are gone.
- Snippet buttons appear only with a snippet chosen.
Session timer: the status-bar timer stops when the shell ends (fails without).
Sync passes that changed nothing no longer decrypt and rebuild the whole keychain (fails without).
TPM probe: it runs once, off the UI thread.
OpenShellAsync: it no longer makes three blocking round trips on the UI thread, and it honours cancellation (fails without).
Transfer queue:
- The lost wake-up is fixed (fails without, 2 of 2 runs).
- Cancel as a transfer finishes no longer throws.
- A refused drop says so.
Lock during unlock: a lock pressed while an unlock is loading now wins (fails without).
Crashes: a vault-visibility failure no longer closes the app. Uncaught exceptions are written to crash.log in the profile, and the process still exits.
Title bar: a double-click is decided on the press, before BeginMoveDrag takes it. Only a headless test covers this; it is not verified on a real Windows window.

Added after the PR was opened

Whole-list replacement (fails without). The host lists are swapped in whole and announced once. Before, every row was announced separately, and the transfers picker and the shell each redid all their work per row: about 90,000 notifications for 300 hosts. Each transfers picker now refreshes only from its own list.
Virtualised lists (fails without: 2,000 of 2,000 rows were built). The transfer queue and both import lists are virtualised. A custom template's ItemsPresenter needs ItemsPanel="{TemplateBinding ItemsPanel}", or it quietly falls back to a plain StackPanel.
Find-box debounce. The hosts board rebuilds once the typing pauses, not on every letter. The view model still filters synchronously. The box binds one way and the code-behind hands the text back after a 150 ms pause. That pause is injectable, because dispatcher timers and thread-pool posts don't run in the headless test session.

Not in this PR

Virtualising the Hosts board itself: Avalonia has no virtualising wrap panel, so this would mean writing one.
P4 (tombstone GC and change-log compaction): needs a protocol decision first. A client offline past the GC horizon would restart with a full pull, and nothing in a full pull removes items the client still holds. An item deleted elsewhere would survive on that machine, and its next edit would recreate it on the server.
The production default server URL: a product decision.

Tests

The flaky ShellFlowTests failures listed in the audit predate this branch: on untouched main, 3 of 8 runs each had one failure. They still show up about one run in three, usually TheCountsAPhoneShows_FollowEveryForwardsState.

The rest of the 2026-10-02 full sweep. The critical and high items went straight to `main` in `a4b81da`. This branch covers the UI, performance and lower-severity findings: 21 commits, one per fix. Each fix has a regression test, with the three noted below as exceptions. Where marked **(fails without)**, the test was also run against the original code and failed there. ## Server - **A retried write transaction committed nothing (fails without).** EF's retrying strategy re-runs the delegate on the same context. If the commit was lost after `SaveChanges` succeeded, every entity was already marked unchanged, so the retry saved nothing and returned success. A team hand-over answered 204 with no roles changed. `FreshAttempts.ExecuteFreshAsync` now clears the tracker on a retry and reloads the entities the attempt carries in. It covers all five write transactions (push, enrolment, team archive, hand-over, member removal). The test uses an interceptor that fails one commit. - **Relay sessions survived losing access (fails without).** `RelayAccessWatch` re-checks a user's open relay sessions when their access changes and closes lost ones with the new code `4403` / `AccessRevoked`. Before this, sessions ran on for up to 12 hours. - **A team could be archived while a vault was created in it.** Both operations now take a per-team advisory lock and re-check inside their transaction. No race test. ## Sync - **Deleting a create that never landed (fails without).** This used to leave a permanent "Rejected" conflict. When the server answers that it has no such item, the delete is now treated as done. A create that did land behaves as before (the delete gives way, and the user is told). - **A pulled page was applied one change at a time.** It is now mirrored in one SQLite write, and the outbox is read once per page. ## Desktop and SSH - **Keyboard:** - The closed-vault cards take focus on launch, and Enter signs in and enrols **(fails without)**. - The host-key, delete and sign-out questions focus their harmless answer, and Escape says no **(fails without)**. - Arrow-key selection on the Hosts board reaches the vault, and Enter connects **(fails without)**. - **Wording:** device unlock is named for the platform (Windows Hello, Touch ID, device PIN). - **Layout:** - Session tab labels are trimmed. - VAULT columns that are always empty are gone. - Snippet buttons appear only with a snippet chosen. - **Session timer:** the status-bar timer stops when the shell ends **(fails without)**. - **Sync passes that changed nothing** no longer decrypt and rebuild the whole keychain **(fails without)**. - **TPM probe:** it runs once, off the UI thread. - **`OpenShellAsync`:** it no longer makes three blocking round trips on the UI thread, and it honours cancellation **(fails without)**. - **Transfer queue:** - The lost wake-up is fixed **(fails without, 2 of 2 runs)**. - Cancel as a transfer finishes no longer throws. - A refused drop says so. - **Lock during unlock:** a lock pressed while an unlock is loading now wins **(fails without)**. - **Crashes:** a vault-visibility failure no longer closes the app. Uncaught exceptions are written to `crash.log` in the profile, and the process still exits. - **Title bar:** a double-click is decided on the press, before `BeginMoveDrag` takes it. Only a headless test covers this; it is not verified on a real Windows window. ## Added after the PR was opened - **Whole-list replacement (fails without).** The host lists are swapped in whole and announced once. Before, every row was announced separately, and the transfers picker and the shell each redid all their work per row: about 90,000 notifications for 300 hosts. Each transfers picker now refreshes only from its own list. - **Virtualised lists (fails without: 2,000 of 2,000 rows were built).** The transfer queue and both import lists are virtualised. A custom template's `ItemsPresenter` needs `ItemsPanel="{TemplateBinding ItemsPanel}"`, or it quietly falls back to a plain `StackPanel`. - **Find-box debounce.** The hosts board rebuilds once the typing pauses, not on every letter. The view model still filters synchronously. The box binds one way and the code-behind hands the text back after a 150 ms pause. That pause is injectable, because dispatcher timers and thread-pool posts don't run in the headless test session. ## Not in this PR - **Virtualising the Hosts board itself:** Avalonia has no virtualising wrap panel, so this would mean writing one. - **P4 (tombstone GC and change-log compaction):** needs a protocol decision first. A client offline past the GC horizon would restart with a full pull, and nothing in a full pull removes items the client still holds. An item deleted elsewhere would survive on that machine, and its next edit would recreate it on the server. - **The production default server URL:** a product decision. ## Tests The flaky `ShellFlowTests` failures listed in the audit predate this branch: on untouched `main`, 3 of 8 runs each had one failure. They still show up about one run in three, usually `TheCountsAPhoneShows_FollowEveryForwardsState`.

jaap-jan added 21 commits 2026-10-02 12:47:34 +00:00

Desktop: put the keyboard in the closed vault's card on launch, and let Enter sign in and enrol 372a7cbdc7

Desktop: give the host-key, delete and sign-out questions the keyboard on their harmless answer, and let Escape say no 6c09bdecb6

Desktop: let the arrows choose the host the drawer acts on, and Enter connect to it 079914bfb4

Desktop: call device unlock what this platform calls it, not Windows Hello everywhere a6d3924e0d

Desktop: trim a long session tab label instead of letting it push the + off the strip 619f49a35a

Desktop: stop the status bar's session timer when the shell ends 22b4d82ba8

Desktop: drop the VAULT columns that had nothing in them, and the snippet buttons with nothing to act on 3ccafdd53b

Desktop: probe for the TPM once, off the UI thread, instead of on every unlock 365954c541

SSH: open a shell off the caller's thread, and stop waiting when the caller gives up f997e87c5f

Sync: mirror a pulled page in one write and read the outbox once per page, not twice per change 873405e591

Desktop: stop decrypting the whole keychain on a sync pass that changed nothing 441709f1e9