fix: clear the attention banner after a passing run and make source removal backend-aware - #310
Merged
Conversation
…emoval backend-aware Two independent bug fixes for v2.12.0: - Issue #271: the drill/scrub attention banner summed failures across the whole loaded history window, so one bad monthly drill (or weekly scrub) kept the red banner up for ~10 subsequent passing runs with no way for a later success to ever clear it. `needsAttention` (and the count it displays) is now keyed on the NEWEST loaded run only, per the review's recommended fix on the issue thread. Applied identically to both the drill and the scrub stores, per the issue's note that the scrub panel uses the same pattern. - Issue #227: removing a source with "also delete the backed-up files" unconditionally rejected the request on every destination (not just Drive) with a Drive-specific error message ("Remove it from Google Drive directly"). Every RemoteStore implementation (Google Drive, S3, the local folder, SFTP) already provides list_source_object_ids + trash, the same primitives the integrity scrub and the remote-existence audit use, so remote deletion on source removal is now implemented for every backend rather than gated behind a capability flag. Enumeration happens before any local row is touched and aborts with zero deletions on failure (mirrors the remote-existence audit's "abort with zero writes" rule); trash is idempotent so a retry after a partial failure is safe. The UI also now surfaces a failed removal inline (it previously failed silently with no visible error) and keeps the confirm panel open with the checkbox still ticked for an easy retry. 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). |
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
1 participant
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.
Summary
Two independent, unrelated bug fixes (PR6 of the v2.12.0 wave).
Issue #271 - drill/scrub attention banner stuck red
needsAttention(and the count it displays) summed failures across the whole loaded 10-run history window. At the monthly drill cadence, one bad run kept the red "N files could not be restored" banner up for ~10 subsequent passing drills, with no way for a later success to ever clear it (each drill draws a fresh time-seeded sample, so it can never "prove" an old failure is fine again by evidence). Applied the fix the linked review recommended: key the banner on the NEWEST loaded run only. The scrub panel used the identical sum-across-window pattern, so it got the same fix per the issue's explicit note that it applies there too.ui/src/stores/drill.ts,ui/src/stores/scrub.ts:failedTotal/unrecoverableTotal(and thereforeneedsAttention) now read offlatestonly, not a.reduce()over the whole window.ui/src/__tests__/drill-panel.test.tsandui/src/__tests__/scrub-panel.test.tscovering both the panel and the store layer, including the exact failed-then-succeeded regression sequence from the issue.Issue #227 - source removal + "delete backed-up files" broke on every non-Drive destination
remove_source(delete_remote: true)unconditionally rejected the request withdrive.unreachableand Drive-specific wording ("Remove it from Google Drive directly"), on EVERY destination, not just S3 as reported - the feature was simply never implemented. EveryRemoteStoreimplementation (Google Drive, S3, the local folder, SFTP) already provideslist_source_object_ids+trash- the same primitives the integrity scrub and the executor's remote-existence audit already use - so this is now implemented for every backend rather than gated behind a capability flag; there was nothing backend-specific left to gate.src-tauri/src/commands/sources.rs:remove_sourcenow deletes the source's live remote objects FIRST (before any local row is touched) whendelete_remoteis set. Enumeration failure aborts with zero deletions (mirrors the audit's "abort with zero writes" rule);trashis idempotent, so a retry after a partial failure only re-deletes what's still actually there. The core logic (delete_source_remote_objects_via) is decoupled fromAppStateso it's directly unit-testable against aRemoteStorefake.ui/src/components/SourceTable.vue: the remove-confirm flow previously failed silently on error (the promise rejection was simply unhandled). It now surfaces the stable error code inline and keeps the confirm panel open with the checkbox still ticked for a one-click retry, instead of leaving the user staring at an unremoved source with no explanation.settings.sources.deleteRemoteLabel, "Also delete the backed-up files from the destination") was already backend-neutral from an earlier pass; no copy change needed there.Test plan
cargo test -p driven-app --lib- 445 passed, including 3 new tests covering: skip-when-nothing-recorded, delete-every-live-object across all fourBackendKindvalues against aRemoteStorefake, and abort-with-zero-deletions on an enumeration failure.cargo clippy -p driven-app --all-targets --all-features -- -D warnings- clean.cargo fmt -p driven-app -- --check- clean.pnpm -C ui run test:unit- 794 passed (up from 793), including 3 new/extended tests for the banner fix and 1 new test for the removal-failure UI surface.pnpm -C ui exec vue-tsc --noEmit- clean.pnpm -C ui run lint- 0 errors (35 pre-existing unused-i18n-key warnings, unrelated to this change).pnpm -C ui run format:check- clean.README: checked, no changes needed (no user-visible feature list, destination table, or setting-location claim referenced either bug; the checkbox copy was already backend-neutral).
Closes #271, closes #227
Co-Authored-By: Claude Fable 5 noreply@anthropic.com
https://claude.ai/code/session_019xKUm9vH4ifb5LHR5szy1v