fix(app): never freeze on tray quit during a backup; quitting tray state; honest recovery status - #312
Merged
Merged
Conversation
Quit ran shutdown_orchestrators() inline via tauri::async_runtime::block_on straight from the RunEvent::ExitRequested callback - i.e. on the thread that owns the platform event loop. On Windows that stalls the Win32 message pump for the whole drain budget (up to RUN_LOOP_DRAIN_TIMEOUT = 20s while a backup cycle finishes), so the shell ghosts the window at ~5s, paints "Not Responding", and Windows Error Reporting kills the process at ~14s. The freeze did not even buy the graceful drain it was blocking for: the process is killed mid-drain, so the run loop is hard-killed anyway. - #299: hide the window + repaint the tray synchronously (single fast platform calls), then SPAWN the drain on the Tauri async runtime. The ExitRequested handler becomes a three-phase machine (IDLE -> DRAINING -> READY) so the event loop keeps pumping until the drain re-raises the exit itself. - #300: a distinct slate "quitting" tray icon with a stop glyph, the tooltip "Driven - quitting (finishing backup, right-click to force quit)", and a two-item menu: a disabled "Quitting - finishing current backup..." status line plus "Force quit now". apply_state and the macOS menu bar title engine both stand down while quitting so a still-draining run loop cannot repaint over it. - #301: reconcile ran one Drive round trip per pending op strictly serially (measured 65.2s for 18 ops on maxbook) while emitting no state transition at all, so the UI sat on a generic "Starting backup...". The adopt-or-requeue LOOKUPS now run with bounded concurrency (6) - they are pure reads plus the already single-flighted parent walk, and every state-DB commit still happens one at a time in the original order - and the pass reports honest per-op progress through OrchestratorState::Recovering (new ops_done/ops_total), surfaced as "Recovering - resuming 7 of 18 uploads". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019xKUm9vH4ifb5LHR5szy1v
Contributor
Coverage
Gate: passed - no coverage regression (epsilon 0.1 pp). |
The coverage gate flagged a 0.13pp Rust regression: the #301 prefetch added error branches with no test behind them. - ReconcileLookup drops the ParentFailed variant for a nested Result on Orphan (outer = the parent walk, inner = find_by_op_uuid). That removes the defensive "a Metadata result reached the create branch" arm entirely - the branch is selected by the absence of a recorded drive_file_id, so the not-prefetched fallback covers it. - reconcile_parent_walk_failure_aborts_the_pass_and_keeps_every_op: 8 nested create ops against a store whose ensure_folder always fails. Covers the parent-walk error propagation, the halt classifier's Orphan(Err) arm, and (with more ops than the concurrency bound) the prefetch's halt branch. - reconcile_adopts_an_update_whose_object_already_carries_the_op_uuid: the UPDATE path's successful metadata lookup, which had no test at all. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019xKUm9vH4ifb5LHR5szy1v
pmaxhogan
enabled auto-merge (squash)
August 17, 2026 19:53
pmaxhogan
added a commit
that referenced
this pull request
Aug 18, 2026
…quit drain Rebases wave-1-debug-diag onto origin/main now that #310-#316 merged (#312's quit-path restructuring in particular). Structural follow-up: - The debug-logging-mode watchdog (debug_mode.rs) previously used the detached memlog.rs pattern (no shutdown tracking). #312 replaced the old shutdown_orchestrators() with a proper ShutdownHandles/ drain_shutdown_handles structure that every other periodic background task (updater, telemetry, iostat, and now #311's bottleneck sampler) registers into for a no-orphan quit drain. Re-homed the watchdog into that same structure: a new DebugModeRuntime (task + shutdown watch, no shared hub - the watchdog only reads/writes settings directly) on AppState, set_debug_mode_task/shutdown_debug_mode_task mirroring set_bottleneck_task/shutdown_bottleneck_task exactly, a debug_mode field on ShutdownHandles, and spawn_watchdog now runs the same select!-on-shutdown-or-tick loop bottleneck_hub/iostat_hub use instead of a bare loop. - Added app_state::tests::debug_mode_runtime_task_and_shutdown_round_trip, mirroring bottleneck's round-trip test. - privacy.png (light+dark) and 9 shell.spec.ts baselines (light+dark) regenerated via `just visual-update` (Docker) - the shell baselines drifted independently of this PR's own diff (same delta across every scenario in both themes), consistent with normal headless-Chromium rendering drift between visual-update runs; all 106 visual specs pass against the regenerated set. No other conflicts: README.md, dtos.rs, settings.rs's redaction code, en-US.json, Activity.vue, and fixtures.ts all auto-merged cleanly with #311's bottleneck-tile additions coexisting alongside this PR's debug logging toggle and diagnostic-bundle changes. Verified after rebase: cargo test -p driven-app --lib (494 passed), cargo clippy --workspace --all-targets -- -D warnings (clean), cargo fmt --all --check (clean), pnpm lint (0 errors), pnpm format:check (clean), pnpm test:unit (861 passed, 64 files), pnpm build / vue-tsc (clean). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019xKUm9vH4ifb5LHR5szy1v
8 tasks
pmaxhogan
added a commit
that referenced
this pull request
Aug 18, 2026
…dles (#314) ## Summary Closes #309, closes #204. Part of the v2.12.0 wave (PR5). **#204 (diagnostic bundle PII/secret leaks)** - `redact_settings()` used to clone `GlobalSettings` verbatim and patch only `proxy_url`. It now builds a field-for-field `RedactedGlobalSettings` struct, so a future secret-bearing field added to `GlobalSettings` fails to *compile* here until someone decides how to redact it, instead of leaking silently. Fixes the three concrete leaks the issue named: - `pre_backup_hook` / `post_backup_hook` (command lines - a classic home for embedded secrets) are now redacted wholesale (`<hook-redacted: N chars>`), not shipped raw. - `custom_root_ca_path` is now hashed through the same `<path:hash>` scheme the rest of the bundle already uses. - `proxy_url` in PAC mode (a local file path, not a URL) is now hashed instead of only having userinfo-stripping applied (which never matched a bare path). - The issue's "also worth fixing" item (`ProxyError`'s `Display` embedding raw userinfo) was already fixed by #208 - verified via `git blame`, not touched again. **#309 (debug logging mode)** - a new Settings > Privacy & Data toggle ("Debug logging") with an always-visible amber warning panel (shown before the toggle is ever switched on, per the approved mockup), backed by: - A real runtime-reloadable tracing filter (`logging.rs`, `tracing_subscriber::reload`) - flipping the toggle now actually changes the live process's verbosity, no restart needed. This also closes a long-documented gap where `global.log_level` only ever exported `RUST_LOG` for the *next* launch; it now reloads the live filter too (deferred while debug mode is active, so it doesn't undo the debug-mode filter). - A persisted epoch-ms expiry + a boot-time reconcile and periodic watchdog (`debug_mode.rs`) that auto-turns the toggle off 24h after enabling - honoured across a restart, not just while the app keeps running. The watchdog is registered on `AppState` and joined by #312's no-orphan quit drain (`ShutdownHandles`/`drain_shutdown_handles`), the same pattern #311's bottleneck sampler uses. - A rolling log cap that widens from 25 MB to 250 MB while debug mode is on. - The diagnostic bundle gains `DEBUG_MODE.txt` and an unredacted `debug/engine_state.txt` while debug mode is on - the one deliberate exception to the #204 redaction rules, gated on the user's explicit opt-in (every other bundle file stays redacted regardless). - Every bundle now also ships `manifest.txt` (entry name + size), a small bundle-usefulness improvement. - Activity's "Export diagnostic bundle" button shows an amber "Debug data included" chip while debug mode is on. ## Also in this PR - **Rebased onto `main`** after #310-#316 merged. Re-homed the debug-mode watchdog from a detached `memlog.rs`-style task into #312's `ShutdownHandles`/`drain_shutdown_handles` no-orphan quit drain (new `DebugModeRuntime` on `AppState`, `set_debug_mode_task`/`shutdown_debug_mode_task` mirroring `set_bottleneck_task`/`shutdown_bottleneck_task`). - **CodeQL `rust/path-injection` fix** (not a dismissal): two test helpers (`settings.rs`'s pre-existing `seeded_repo()` and this PR's new `debug_mode.rs` one) hand-rolled a temp dir via `std::env::temp_dir().join(format!(...))` before feeding it to `SqliteStateRepo::open` - exactly the pattern this repo's CodeQL rule flags (see the `tempfile` dependency comment in `src-tauri/Cargo.toml`, and PR 151 precedent). Switched both to `tempfile::tempdir().keep()`, an opaque external call CodeQL's dataflow can't see into, so the taint chain never forms. - **Also carries the h2 advisory fix** (RUSTSEC-2026-0258, low severity, unbounded empty DATA frames) - `cargo update -p h2` (0.4.15 -> 0.4.16), lockfile-only, no `Cargo.toml` changes. This advisory is unrelated to this PR's own diff (`git diff` against the pre-PR base shows zero `Cargo.lock` changes before this commit) and would fail `cargo deny` repo-wide on `main` too; landing it here unblocks this PR's `cargo deny` check and delivers the fix to `main` in the same step. ## Test plan - [x] `cargo test -p driven-app --lib` - 494 passed (18 #204 redaction tests with leak-shaped fixtures, incl. one asserting the full serialized bundle JSON end-to-end; 5 debug-mode watchdog/expiry tests; 6 settings-persistence round-trip tests; 1 `AppState` debug-mode task/shutdown round-trip test) - [x] `cargo clippy --workspace --all-targets -- -D warnings` - clean - [x] `cargo fmt --all -- --check` - clean - [x] `cargo build --workspace --tests` - clean - [x] `cargo deny check` - clean (advisories ok, bans ok, licenses ok, sources ok) - [x] `pnpm lint` / `pnpm format:check` / `pnpm test:unit` (861 passed, 64 files) / `pnpm build` (vue-tsc + vite) - all clean, run in the CI job's exact order - [x] Linux visual baselines regenerated via `just visual-update` (Docker) - `privacy.png` (light+dark) plus 9 `shell.spec.ts` baselines (light+dark) that had drifted independently of this PR; all 106 visual specs pass - [x] README updated (Features list + comparison-table footnote ³⁴) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019xKUm9vH4ifb5LHR5szy1v --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
pmaxhogan
added a commit
that referenced
this pull request
Aug 18, 2026
🤖 I have created a release *beep* *boop* --- ## [2.12.0](v2.11.1...v2.12.0) (2026-08-18) ### Features * **app:** opt-in debug logging mode and safer, richer diagnostic bundles ([#314](#314)) ([33c281c](33c281c)) * **core:** allow nested backup sources when the parent excludes the child ([#294](#294)) ([0b62df9](0b62df9)) * **core:** live exclusion pickup and a visible pending-work queue ([#313](#313)) ([e6427c7](e6427c7)) * live bottleneck indicator on the Activity dashboard ([#311](#311)) ([2d9d763](2d9d763)) * **ui:** folder picker sort/filter/create/rename and exclusions size rollups ([#315](#315)) ([7e87341](7e87341)) ### Bug Fixes * **app:** never freeze on tray quit during a backup; quitting tray state; honest recovery status ([#312](#312)) ([f951cde](f951cde)) * clear the attention banner after a passing run and make source removal backend-aware ([#310](#310)) ([6d8e1ab](6d8e1ab)) * **s3:** honest per-source versioning across backends and multipart-upload leak sweep ([#316](#316)) ([d462592](d462592)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #299
Closes #300
Closes #301
PR1 of the v2.12.0 wave.
Diagnosis
#299 - tray quit freezes the app (and trips WER) during a backup
Root cause: the graceful drain ran on the platform event-loop thread.
tray.rsmenu_id::QUIT->app.exit(0)->RunEvent::ExitRequested->shutdown_orchestrators(), which drove the whole drain inline viatauri::async_runtime::block_on.block_onblocks the calling thread, andthe
RunEventcallback runs on the main thread that owns the Win32 messagepump. So the pump stalls for the entire drain budget:
RUN_LOOP_DRAIN_TIMEOUTis 20s and graceful orchestrator shutdown is only observed between cycles, so
quitting mid-backup reliably parks there. Windows ghosts a window that has not
pumped for ~5s, paints "Not Responding", and Windows Error Reporting kills the
process.
Evidence, from the owner's machine (maxbook, Windows 11 26200):
Application Hangprovider:8/16/2026 10:49:23 AM- "The program driven-app.exe version 2.11.2.0stopped interacting with Windows and was closed."
AppHangB1is theblocked-message-pump signature, not a crash.
driven-diagnostics-hung-on-starting-backup.zip,logs/driven.2026-08-16.log:quitting process never wrote
all per-account tasks shut down (no orphans)-it was killed mid-drain. Same shape on the second quit:
15:52:59.576Zquit-> new process at
15:53:12.858Z.So the freeze did not even buy the graceful semantics it was blocking for: the
drain is killed before it finishes and the run loop dies hard anyway.
#301 - ~1 minute of opaque "Starting backup"
Two independent defects, both visible in the same log:
reconcile_innerwalks pending ops strictly one ata time, and every op costs at least one Drive metadata round trip
(
metadataby id for the update path,find_by_op_uuidunder a re-derivedparent for the create path). 65.2s / 18 ops == ~3.6s per op, which is a
normal Drive
files.listlatency. An earlier run in the same log shows thesame 60.0s shape.
OrchestratorState::Recoveringwas emitted onlyfrom the byte-level resume branch, via the
on_recoversink. The 18plain adopt-or-requeue ops move no bytes, so they emitted nothing and the
orchestrator stayed on
PowerCheckfor the whole pass - which is thegeneric "Starting backup..." the user saw. (In the 15:49 run, where one op
did carry a resumable session,
Recoveringdid appear - for 2 of the 32seconds - which is why the bug looked intermittent.)
What changed
#299 quit never blocks the event loop
lib.rs:shutdown_orchestratorsis split intotake_shutdown_handles(synchronous, cheap - every call is a mutex-take or a watch-send, no await,
no I/O) and
async fn drain_shutdown_handles(the unchanged drain body).begin_graceful_quithides the window and repaints the tray synchronously(single fast platform calls - the window disappearing is the whole point of
clicking Quit), then spawns the drain on the Tauri async runtime and
returns. The event loop keeps pumping for the entire drain, so the app can
never be declared hung.
RunEvent::ExitRequestedbecomes a three-phase machine on a process-globalQUIT_PHASE:IDLE->prevent_exit(), start the drain, return at once;DRAINING-> a second explicit quit (repeat--quit, OS session end) isprevented and ignored; "Force quit now" is the deliberate escape hatch;
READY-> not prevented, so the process exits. The drain setsREADYandcalls
app.exit(0)itself when it is genuinely done.lib.rs:228-238andapp_state.rs:209-215are preserved verbatim: still no outertokio::time::timeoutaround the cancellation-UNSAFEAccountHandle::shutdownsweep, stilljoin_allfor concurrency, stilldrain_or_abort's self-bounding await-then-abort-and-await per handle. TheVSS/APFS broker shutdowns still run after the orchestrator drain, now at
the tail of the spawned task.
#300 quitting tray state
TrayIcon::Quitting: slate (#71717a) badge with a white stopsquare glyph. The approved mockup used amber; amber is already spent on
PausedandNetworkAttention, and "quitting" must not read as "paused,still running". Slate is the only desaturated badge in the set, and the stop
square is what carries the state on the macOS template path where hue is
discarded entirely.
tray.tooltip.quitting: "Driven - quitting (finishing backup,right-click to force quit)".
tray::enter_quittingswaps the menu for a two-item one: a disabledstatus line "Quitting - finishing current backup..." and a single enabled
"Force quit now". Every normal action is deliberately gone - they would all
queue work against orchestrators that are already winding down.
crate::force_quitmarks the phaseREADYand exits immediately. This issafe to do hard: the streaming resumable uploader persists
resumable.acked_offsetinto the pending op's payload after every ackedwire chunk (
executor.rspush_one), so the next launch's reconcile resumesfrom the last acked byte and at most one wire chunk is re-sent. Restore jobs
stage into a temp that is never promoted, so a kill cannot publish a partial
file.
apply_statereturns before touching the tray while quitting (the run loopis still finishing its cycle and keeps emitting transitions, which would
otherwise repaint the spinner over the quitting icon), and skips the OS
notification too - a "first sync complete" toast while the app is closing is
noise. The aggregate per-account map is still updated.
quitting, so a live "12.3 MB/s, 4 min left" cannot contradict the icon.
#301 faster recovery + honest status
RECONCILE_LOOKUP_CONCURRENCY = 6.prefetch_reconcile_lookupsruns theadopt-or-requeue lookups through
buffer_unorderedahead of the unchangedsequential decision pass.
remote.metadataandfind_by_op_uuidare pure reads; thecreate path's parent walk (
ensure_parents_once) is idempotent AND alreadysingle-flighted behind
parent_walkwith a per-sourceparent_dirscache,built precisely so concurrent uploads into one new directory cannot race
duplicate
ensure_foldercalls. Nothing in the prefetch touches thestate DB.
delete_pending_op/adopt_reconciled/clear_file_state_drive_file_idstill runs one at a time, in the originalop order, in the sequential pass - no whole-batch buffering (the 2026-08-14
OOM incident was reconcile buffering too much, so this deliberately buffers
only lookup results).
pacer.permit_request()andreports its
ResponseClass, so a rate limit or an open circuit breakerstill gates the pass. The accounting simply moved into the two lookup
helpers alongside the calls.
retryable lookup error, so prefetching could turn one failed request
during a Drive outage into N. The first future to see one sets a halt flag;
every not-yet-started future returns
None, and the pass makes that onelookup inline and hits the same abort.
resumablesession (theystream a whole file and own the byte ticks, so they stay strictly
sequential; their fall-through lookup when a session is stale is done
inline) and ops with no
client_op_uuid.OrchestratorState::Recoveringgainsops_done/ops_total, fed by a newOP flavour of
RecoverProgressemitted once before the first round trip andonce per recovered op.
reconcile_innerstamps the pass's live counters ontoevery tick, byte ticks included, so the deep resume call sites did not
have to thread them down. The orchestrator's existing ~1/s throttle now
treats "final" as both dimensions being complete.
progress.recoveringOps, a determinate percent from the op counters whenno bytes are moving, and the label "Recovering - resuming 7 of 18 uploads".
Test results
All run locally on the Mac unless noted.
cargo test -p driven-core --lib- 559 passed, 0 failedcargo test -p driven-app- 442 + 6 + 3 passed, 0 failedcargo clippy --workspace --all-targets -- -D warnings- cleancargo fmt --all --check- cleanpnpm -C ui run test:unit- 793 passed (59 files)pnpm -C ui run build(vue-tsc + vite) - cleanpnpm -C ui run lint- 0 errors (35 pre-existing unused-i18n-key warnings)pnpm -C ui run format:check- cleanx86_64-pc-windows-msvc, worktree atV:\driven-agents\wave1):cargo check -p driven-app --libandcargo clippy -p driven-app -p driven-core --all-targets -- -D warningsbothexit 0. (Cross-compiling from macOS is not possible -
ring/aws-lc-sysneed a Windows C toolchain - so this was checked on the machine itself.
Note: building this repo on maxbook now needs NASM for
aws-lc-sys0.43,pulled in by the dependabot bumps on main; I installed it via
winget install NASM.NASM.)New tests:
executor::reconcile_lookups_run_concurrently_and_report_op_progress- 12ops against a fake store with a fixed 120ms per-request delay. A serial pass
has a hard floor of 1440ms; the assertion is
< 720ms, and two concurrentrounds land near 240ms. This fails if the prefetch is removed or silently
serialised - it is not a sleep-and-hope timing test. Also asserts the first
tick announces
0 of 12before any lookup, every tick carries the total, thefinal tick reports
12 of 12, andops_doneis monotonic.executor::reconcile_with_no_pending_ops_emits_no_op_ticks- a clean bootstays free (no transition churn).
executor::reconcile_lookup_classifies_which_failures_halt_the_prefetch-transient metadata/orphan failures and a parent-walk failure halt; a
definitive not-found does not (it is per-op and the pass continues).
executor::resume_emits_recover_progress_ticksupdated for the new tickstream (byte ticks are now those with a non-zero total; op counters asserted
on every tick).
TrayIcon::Quittingadded to every exhaustive icon test - distinctcolour, distinct glyph, badge actually painted, static across frames, brand
dimensions, and the macOS template
STATESset.tray.quitting_status,tray.force_quit,tray.tooltip.quittingadded to both the exact-label and the no-raw-key sweeps.
the new progress-bar label.
README
Updated in this PR (repo rule):
concurrent startup recovery;
"Force quit now".
Remaining QA on maxbook (for the team lead, after a dev build exists)
The unit + Windows-toolchain checks above cannot exercise the message pump, so
these need the installed/dev app on maxbook:
existing source has multi-GB files), then Quit from the tray. Expect: the
window disappears immediately, the app stays responsive, the process lives
until the drain finishes, and the log ends with
all per-account tasks shut down (no orphans)->graceful quit drain complete; exiting. Then confirm no newApplication Hangevent:Get-WinEvent -FilterHashtable @{LogName='Application'; ProviderName='Application Hang'} -MaxEvents 5the slate stop badge, hover for the tooltip, and right-click: the menu must
be exactly the disabled "Quitting - finishing current backup..." plus
"Force quit now".
an immediate exit, and on the next launch the log shows
reconcile: resuming persisted resumable upload (streaming re-read)with aresume_fromat or near where it was killed (not 0).18-22), time from
reconcile: recovering pending opsto the firststate transition state="scanning". Baseline was 65.2s; expect roughly asixth of the round-trip time. During it, the app should read
"Recovering - resuming N of M uploads" with a moving determinate bar, never
a generic "Starting backup...".
Not merged - the team lead babysits the merge.
Follow-up commit: coverage
The first push failed the
coveragegate at Rust 84.43% vs main 84.56%(epsilon 0.1pp) - the new prefetch error branches had no tests behind them.
be55fe9fixes that:ReconcileLookupdrops itsParentFailedvariant in favour of a nestedOrphan(Result<Result<..>>)(outer = the parent walk, inner =find_by_op_uuid). That removes the one defensive-but-unreachable arm thefirst shape needed - the create branch is selected by the absence of a
recorded
drive_file_id, so the "not prefetched" fallback covers it.reconcile_parent_walk_failure_aborts_the_pass_and_keeps_every_op- 8 nestedcreate ops against a store whose
ensure_folderalways fails. Covers theparent-walk propagation, the halt classifier's
Orphan(Err)arm, and (withmore ops than the concurrency bound) the prefetch's halt branch, and asserts
every op is kept.
reconcile_adopts_an_update_whose_object_already_carries_the_op_uuid- theUPDATE path's successful metadata lookup, which had no test at all before.
All 17 checks green on
be55fe9;mergeStateStatus: CLEAN. Windowscargo test + clippypasses, so the concurrency timing assertion is not flakyon the slowest runner.