From a7845acd81e59126ba5eb63261449616f03d3639 Mon Sep 17 00:00:00 2001 From: David Vornholt <157214705+davidvornholt@users.noreply.github.com> Date: Sun, 10 May 2026 22:35:33 +0200 Subject: [PATCH] fix(electron): retry Wayland idle helper startup (#7527) * fix(electron): retry wayland idle helper startup * build(electron): enforce wayland helper in CI * fix(electron): avoid blocking wayland idle promotion * test(e2e): make reminder option selector exact * test(e2e): resolve reminder option selector conflict * fix(electron): gate wayland helper redetection * fix(electron): avoid stale Wayland helper packaging --------- Co-authored-by: Johannes Millan --- docs/long-term-plans/secure-storage.md | 40 +++++----- e2e/store-video/CLAUDE.md | 20 ++--- ...minders-default-task-remind-option.spec.ts | 1 - electron/idle-time-handler.ts | 77 ++++++++++++++++++- tools/afterPack.js | 14 +++- tools/build-wayland-idle-helper.js | 37 ++++++++- 6 files changed, 151 insertions(+), 38 deletions(-) diff --git a/docs/long-term-plans/secure-storage.md b/docs/long-term-plans/secure-storage.md index b2dd01764f..e3c59f4dc7 100644 --- a/docs/long-term-plans/secure-storage.md +++ b/docs/long-term-plans/secure-storage.md @@ -234,11 +234,11 @@ V1 explicitly defers: V1 storage capability matrix: -| Platform | V1 persistent local secret store | Notes | -| --- | --- | --- | -| Electron desktop | Yes, `indexedDbProfile` | Local-isolation tier only; not OS-backed at-rest protection | -| Browser/PWA | No | Use session-only or unavailable mode in V1 | -| Android/iOS Capacitor | No | Use session-only or unavailable mode until native storage/backup rules are implemented | +| Platform | V1 persistent local secret store | Notes | +| --------------------- | -------------------------------- | -------------------------------------------------------------------------------------- | +| Electron desktop | Yes, `indexedDbProfile` | Local-isolation tier only; not OS-backed at-rest protection | +| Browser/PWA | No | Use session-only or unavailable mode in V1 | +| Android/iOS Capacitor | No | Use session-only or unavailable mode until native storage/backup rules are implemented | V1 does not improve local at-rest protection for persisted Electron profile data. Its main win is removing raw integration secrets from synced state, op-log @@ -397,7 +397,11 @@ export interface LocalSecretStoreCapabilities { export interface LocalSecretStore { capabilities(): Promise; - set(input: SecretRefInput, value: string, context: SecretAccessContext): Promise; + set( + input: SecretRefInput, + value: string, + context: SecretAccessContext, + ): Promise; useSecret( ref: SecretRef, context: SecretAccessContext, @@ -884,18 +888,18 @@ Only after device-local storage and compatibility gates are stable: ## Error Handling -| Scenario | Handling | -| --- | --- | -| V1 device-local entry missing | Show "credential missing on this device" and offer reauth/re-enter | -| V1 local profile store unavailable | Mark affected integration missing/read-only on this device; do not write raw fallback state | -| V1 corrupted local entry | Do not delete the `SecretRef` automatically; offer reconnect/replace and diagnostic export without the secret | -| Migration fails mid-way | Keep the legacy value only in its original legacy source for retry; do not re-persist it through NgRx, op-log, backup, or synced state | -| Secret lookup fails during sync | Stop sync and ask for credential; do not overwrite or disable config | -| Older client still syncing | Keep affected providers read-only or unmigrated until compatibility gate is satisfied | -| Post-V1 portable vault locked | Show "credential locked" and offer the existing sync E2EE unlock, vault passphrase, recovery key, or device-pairing flow | -| Post-V1 OS-backed storage unavailable | Use session-only storage or explicit passphrase-protected/degraded mode | -| Post-V1 Linux `basic_text` backend | Do not silently persist secrets; require explicit plaintext-equivalent consent or passphrase/session-only alternative | -| Post-V1 sync E2EE key rotation cannot rewrap vault | Keep old wrapper during grace period; if unavailable, require old unlock material or integration reauth | +| Scenario | Handling | +| -------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------- | +| V1 device-local entry missing | Show "credential missing on this device" and offer reauth/re-enter | +| V1 local profile store unavailable | Mark affected integration missing/read-only on this device; do not write raw fallback state | +| V1 corrupted local entry | Do not delete the `SecretRef` automatically; offer reconnect/replace and diagnostic export without the secret | +| Migration fails mid-way | Keep the legacy value only in its original legacy source for retry; do not re-persist it through NgRx, op-log, backup, or synced state | +| Secret lookup fails during sync | Stop sync and ask for credential; do not overwrite or disable config | +| Older client still syncing | Keep affected providers read-only or unmigrated until compatibility gate is satisfied | +| Post-V1 portable vault locked | Show "credential locked" and offer the existing sync E2EE unlock, vault passphrase, recovery key, or device-pairing flow | +| Post-V1 OS-backed storage unavailable | Use session-only storage or explicit passphrase-protected/degraded mode | +| Post-V1 Linux `basic_text` backend | Do not silently persist secrets; require explicit plaintext-equivalent consent or passphrase/session-only alternative | +| Post-V1 sync E2EE key rotation cannot rewrap vault | Keep old wrapper during grace period; if unavailable, require old unlock material or integration reauth | ## Backup and Restore Rules diff --git a/e2e/store-video/CLAUDE.md b/e2e/store-video/CLAUDE.md index ae971f8877..4d2af98224 100644 --- a/e2e/store-video/CLAUDE.md +++ b/e2e/store-video/CLAUDE.md @@ -20,14 +20,14 @@ npm run video:open # opens an autoplay browser preview, skips in CI ## Files -| File | Responsibility | -| ----------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `playwright.store-video.config.ts` | Single chromium project, `video: 'off'` at project level (the fixture handles `recordVideo` itself because `browser.newContext()` doesn't inherit `use.video`). | -| `store-video/fixture.ts` | Custom context with `recordVideo` enabled at 1024×1024 / DPR 2. Reuses the screenshot pipeline's seed builder. Init scripts handle: cursor highlight ring, dialog/snack/tooltip/mention suppression, app zoom. | -| `store-video/overlays.ts` | DOM-injected overlay primitives: `showOverlay`, `showCaption`, `showIntegrationsCard`, `showEndCard`, `cutToScene`, `fadeTransition`, `loopBoundary`, `attachDragGhost`, `smoothMouseMove`. Plus inline brand SVGs in the `LOGOS` constant. | -| `store-video/scenarios/reel.spec.ts` | Six-beat choreography. `REEL_VARIANT=full` triggers the optional "No account. No tracking." beat and relaxes hold timings. | -| `store-video/build-video.ts` | Picks the most recent `.webm` under `.tmp/video/recordings/`, applies the trim sidecar (cuts the seed-import lead-in), produces mp4/webm/gif via ffmpeg, optionally `gifsicle`-optimizes. | -| `store-video/open-video.ts` | Opens an autoplay browser preview after `npm run video`. Prefers mp4, respects `REEL_VARIANT`, seeks slightly past the black first frame for preview only, and skips auto-open in CI. | +| File | Responsibility | +| ------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `playwright.store-video.config.ts` | Single chromium project, `video: 'off'` at project level (the fixture handles `recordVideo` itself because `browser.newContext()` doesn't inherit `use.video`). | +| `store-video/fixture.ts` | Custom context with `recordVideo` enabled at 1024×1024 / DPR 2. Reuses the screenshot pipeline's seed builder. Init scripts handle: cursor highlight ring, dialog/snack/tooltip/mention suppression, app zoom. | +| `store-video/overlays.ts` | DOM-injected overlay primitives: `showOverlay`, `showCaption`, `showIntegrationsCard`, `showEndCard`, `cutToScene`, `fadeTransition`, `loopBoundary`, `attachDragGhost`, `smoothMouseMove`. Plus inline brand SVGs in the `LOGOS` constant. | +| `store-video/scenarios/reel.spec.ts` | Six-beat choreography. `REEL_VARIANT=full` triggers the optional "No account. No tracking." beat and relaxes hold timings. | +| `store-video/build-video.ts` | Picks the most recent `.webm` under `.tmp/video/recordings/`, applies the trim sidecar (cuts the seed-import lead-in), produces mp4/webm/gif via ffmpeg, optionally `gifsicle`-optimizes. | +| `store-video/open-video.ts` | Opens an autoplay browser preview after `npm run video`. Prefers mp4, respects `REEL_VARIANT`, seeks slightly past the black first frame for preview only, and skips auto-open in CI. | ## Beat structure (current) @@ -77,11 +77,11 @@ The main reel uses two transition styles: **Material snack bars and dialogs.** Hidden via CSS in the fixture init script (`.cdk-overlay-pane:has(.mat-mdc-dialog-container)`, `.mat-mdc-snack-bar-container`, plus `mention-list`, `.mention-menu`, `.add-task-bar-panel`). With the clock installed, snack auto-dismiss timers don't fire on their own — hiding them is cleaner than waiting. focus-mode-overlay isn't a mat-dialog so it's unaffected. -**App zoom.** `app-root { zoom: 1.4 }` in fixture init "zooms in" on the SP UI without shrinking the recording canvas. At 1.4 the inner viewport for the layout is 1024/1.4 ≈ 731px — enough for the work-view + collapsed sidenav + 240-wide right panel without clipping. Earlier 1.5 was cropping the right edge of the work view. Earlier 1.4 iterations *also* stacked `transform: scale(1.45)` on the add-task-bar, but that compounded badly with the zoom and clipped past the viewport — letting `app-root zoom` do the work alone is simpler. Overlays are siblings of `app-root` in the DOM tree (appended to `body`), so they're unaffected by this zoom — design overlay sizes against the un-zoomed viewport. +**App zoom.** `app-root { zoom: 1.4 }` in fixture init "zooms in" on the SP UI without shrinking the recording canvas. At 1.4 the inner viewport for the layout is 1024/1.4 ≈ 731px — enough for the work-view + collapsed sidenav + 240-wide right panel without clipping. Earlier 1.5 was cropping the right edge of the work view. Earlier 1.4 iterations _also_ stacked `transform: scale(1.45)` on the add-task-bar, but that compounded badly with the zoom and clipped past the viewport — letting `app-root zoom` do the work alone is simpler. Overlays are siblings of `app-root` in the DOM tree (appended to `body`), so they're unaffected by this zoom — design overlay sizes against the un-zoomed viewport. **Schedule day-panel width.** Pre-seeded in `ONBOARDING_INIT` via `localStorage.setItem('SUP_RIGHT_PANEL_WIDTH', '250')` — that's `RIGHT_PANEL_CONFIG.MIN_WIDTH`, the smallest the panel allows before its 200px CLOSE_THRESHOLD kicks in. Pre-seeding through the panel's own persistence path means the inner schedule grid computes its column widths against 250 and the event blocks don't overflow. Earlier iterations forced `width !important` on `.side`, which sized the chrome but didn't propagate to the grid — events then spilled past the panel's right edge and required ugly `overflow-x: hidden` belt-and-braces clips. Don't go that route. -**Add-task-bar overlays.** The fixture only hides the *overlay surfaces* that pop on top of the bar while typing (mat-autocomplete suggestions, mention-list, loading spinner) — those would otherwise read as glitchy white boxes mid-gif. The bar itself is not styled by the fixture; it uses its real `:host` rules. Beat 1 → 2 closes it by clicking the real `.backdrop`, matching normal UI behavior instead of dispatching layout state directly. +**Add-task-bar overlays.** The fixture only hides the _overlay surfaces_ that pop on top of the bar while typing (mat-autocomplete suggestions, mention-list, loading spinner) — those would otherwise read as glitchy white boxes mid-gif. The bar itself is not styled by the fixture; it uses its real `:host` rules. Beat 1 → 2 closes it by clicking the real `.backdrop`, matching normal UI behavior instead of dispatching layout state directly. **Cursor highlight.** Soft white radial-gradient ring at `z-index: 2147483640`, follows mousemove. Toggle visibility per-beat via `body.__sp-hide-cursor-highlight` — used during the capture beat where the focused input would otherwise show the ring as a stray dot. diff --git a/e2e/tests/reminders/reminders-default-task-remind-option.spec.ts b/e2e/tests/reminders/reminders-default-task-remind-option.spec.ts index 5552d48568..c4dc49100b 100644 --- a/e2e/tests/reminders/reminders-default-task-remind-option.spec.ts +++ b/e2e/tests/reminders/reminders-default-task-remind-option.spec.ts @@ -13,7 +13,6 @@ test.describe('Default task reminder option', () => { // text precisely and not collide with substrings like the "...never run code..." // tooltip on the theme upload button in the General tab. const changedOptionText = 'Never'; - const expandConfigSection = async (section: Locator): Promise => { const collapsible = section.locator('collapsible').first(); await collapsible.waitFor({ state: 'visible', timeout: 10000 }); diff --git a/electron/idle-time-handler.ts b/electron/idle-time-handler.ts index 4d2856ddaf..f4501b21da 100644 --- a/electron/idle-time-handler.ts +++ b/electron/idle-time-handler.ts @@ -12,6 +12,8 @@ const log = electronLog.scope('IdleTimeHandler'); const WAYLAND_IDLE_HELPER_READY_TIMEOUT_MS = 3000; const WAYLAND_IDLE_HELPER_KILL_TIMEOUT_MS = 500; const WAYLAND_IDLE_HELPER_FAILURE_THRESHOLD = 3; +const WAYLAND_IDLE_NOTIFY_RETRY_INTERVAL_MS = 60_000; +const WAYLAND_IDLE_NOTIFY_DEMOTION_COOLDOWN_MS = 5 * 60_000; type IdleDetectionMethod = | 'powerMonitor' @@ -45,6 +47,9 @@ export class IdleTimeHandler { private _waylandIdleHelperReadyPromise: Promise | null = null; private _waylandIdleSinceMs: number | null = null; private _waylandIdleHelperFailureCount: number = 0; + private _waylandIdleNotifyPromotionPromise: Promise | null = null; + private _lastWaylandIdleNotifyRetryMs: number = 0; + private _lastWaylandIdleNotifyDemotionMs: number = 0; private readonly _waylandHelperTimeoutMs = CONFIG.MIN_IDLE_TIME; private readonly _resetWaylandIdleAfterResume = (): void => { this._waylandIdleSinceMs = null; @@ -101,6 +106,7 @@ export class IdleTimeHandler { async getIdleTime(): Promise { const methodUsed = await this._ensureWorkingMethod(); + this._maybePromoteToWaylandIdleNotify(methodUsed); switch (methodUsed) { case 'powerMonitor': @@ -182,6 +188,64 @@ export class IdleTimeHandler { return this._methodDetectionPromise; } + private _maybePromoteToWaylandIdleNotify(currentMethod: IdleDetectionMethod): void { + if ( + !this._environment.isWayland || + currentMethod === 'gnomeDBus' || + currentMethod === 'waylandIdleNotify' || + this._waylandIdleNotifyPromotionPromise + ) { + return; + } + + const now = Date.now(); + if (this._isWaylandIdleNotifyDemotionCoolingDown(now)) { + return; + } + + if ( + this._lastWaylandIdleNotifyRetryMs && + now - this._lastWaylandIdleNotifyRetryMs < WAYLAND_IDLE_NOTIFY_RETRY_INTERVAL_MS + ) { + return; + } + + // Stamp before the async helper-ready attempt so repeated idle polls do not + // start duplicate helper processes while the up-to-3s readiness check runs. + this._lastWaylandIdleNotifyRetryMs = now; + void this._tryPromoteToWaylandIdleNotify(); + } + + private _isWaylandIdleNotifyDemotionCoolingDown(now = Date.now()): boolean { + return ( + !!this._lastWaylandIdleNotifyDemotionMs && + now - this._lastWaylandIdleNotifyDemotionMs < + WAYLAND_IDLE_NOTIFY_DEMOTION_COOLDOWN_MS + ); + } + + private async _tryPromoteToWaylandIdleNotify(): Promise { + this._waylandIdleNotifyPromotionPromise = this._ensureWaylandIdleHelperStarted() + .then((works) => { + if (!works) { + return; + } + + log.info('Promoted idle detection method to waylandIdleNotify'); + this._workingMethod = 'waylandIdleNotify'; + this._methodDetectionPromise = Promise.resolve('waylandIdleNotify'); + this._lastWaylandIdleNotifyDemotionMs = 0; + }) + .catch((error: unknown) => { + log.debug('Wayland idle-notify promotion failed', error); + }) + .finally(() => { + this._waylandIdleNotifyPromotionPromise = null; + }); + + await this._waylandIdleNotifyPromotionPromise; + } + private async _determineWorkingMethod(): Promise { log.debug('Determining idle detection method...'); @@ -230,10 +294,14 @@ export class IdleTimeHandler { }); } - candidates.push({ - name: 'waylandIdleNotify', - test: async () => this._ensureWaylandIdleHelperStarted(), - }); + if (this._isWaylandIdleNotifyDemotionCoolingDown()) { + log.debug('Skipping waylandIdleNotify during demotion cooldown'); + } else { + candidates.push({ + name: 'waylandIdleNotify', + test: async () => this._ensureWaylandIdleHelperStarted(), + }); + } candidates.push({ name: 'xprintidle', @@ -465,6 +533,7 @@ export class IdleTimeHandler { log.warn('Wayland idle helper failed repeatedly; re-detecting idle method'); this._waylandIdleHelperFailureCount = 0; + this._lastWaylandIdleNotifyDemotionMs = Date.now(); this._workingMethod = 'none'; this._methodDetectionPromise = null; } diff --git a/tools/afterPack.js b/tools/afterPack.js index d25f92d0b8..883f685a37 100644 --- a/tools/afterPack.js +++ b/tools/afterPack.js @@ -31,6 +31,8 @@ const WAYLAND_IDLE_HELPER_SRC = join( ); const WAYLAND_IDLE_HELPER_DEST = 'wayland-idle-helper'; +const isTruthyEnv = (value) => value === '1' || value?.toLowerCase() === 'true'; + async function afterPack(context) { if (context.electronPlatformName !== 'linux') return; @@ -40,14 +42,20 @@ async function afterPack(context) { const helperDestPath = join(appOutDir, WAYLAND_IDLE_HELPER_DEST); const installWaylandIdleHelper = async () => { - const helperStat = await fs.stat(WAYLAND_IDLE_HELPER_SRC).catch(() => null); - if (!helperStat) { + if (isTruthyEnv(process.env.SP_SKIP_WAYLAND_IDLE_HELPER_BUILD)) { console.warn( - `[afterPack] ${WAYLAND_IDLE_HELPER_SRC} not found; skipping Wayland idle helper copy`, + '[afterPack] Skipping Wayland idle helper copy because SP_SKIP_WAYLAND_IDLE_HELPER_BUILD is set', ); return; } + const helperStat = await fs.stat(WAYLAND_IDLE_HELPER_SRC).catch(() => null); + if (!helperStat) { + throw new Error( + `[afterPack] ${WAYLAND_IDLE_HELPER_SRC} not found. Linux packages must include the Wayland idle helper; install Rust/Cargo before packaging or set SP_SKIP_WAYLAND_IDLE_HELPER_BUILD=1 to intentionally omit ext-idle-notify support.`, + ); + } + await fs.copyFile(WAYLAND_IDLE_HELPER_SRC, helperDestPath); await fs.chmod(helperDestPath, 0o755); }; diff --git a/tools/build-wayland-idle-helper.js b/tools/build-wayland-idle-helper.js index 9ba10dc005..4c05f02906 100644 --- a/tools/build-wayland-idle-helper.js +++ b/tools/build-wayland-idle-helper.js @@ -25,14 +25,47 @@ const hasCargo = () => { return result.status === 0; }; +const isTruthyEnv = (value) => value === '1' || value?.toLowerCase() === 'true'; + +const isExplicitlySkipped = () => + isTruthyEnv(process.env.SP_SKIP_WAYLAND_IDLE_HELPER_BUILD); + +const isBuildRequired = () => + isTruthyEnv(process.env.CI) || + isTruthyEnv(process.env.SP_REQUIRE_WAYLAND_IDLE_HELPER_BUILD); + +const removeBuiltHelper = () => { + fs.rmSync(OUTPUT_PATH, { force: true }); + fs.rmSync(`${OUTPUT_PATH}.tmp`, { force: true }); +}; + const buildHelper = () => { - if (!hasCargo()) { + if (isExplicitlySkipped()) { + removeBuiltHelper(); console.warn( - '[build-wayland-idle-helper] Rust toolchain not found -- skipping Wayland idle helper. Install via https://rustup.rs if you need it.', + '[build-wayland-idle-helper] Skipping Wayland idle helper because SP_SKIP_WAYLAND_IDLE_HELPER_BUILD is set.', ); return; } + if (!hasCargo()) { + const message = + '[build-wayland-idle-helper] Rust/Cargo is required to build the Wayland idle helper.'; + + if (isBuildRequired()) { + console.error( + `${message} Install rustup/cargo, or set SP_SKIP_WAYLAND_IDLE_HELPER_BUILD=1 for builds that intentionally omit ext-idle-notify support.`, + ); + process.exit(1); + } + + console.warn( + `${message} Skipping for this local build. Set SP_REQUIRE_WAYLAND_IDLE_HELPER_BUILD=1 to enforce helper builds outside CI.`, + ); + removeBuiltHelper(); + return; + } + run('cargo', [ 'build', '--release',