From 0a60b164f56e632544600bf368e374c0ef4e8986 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:00:18 -0700 Subject: [PATCH] fix(desktop): single-owner console capture + HUD lifecycle coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reconcile the salvaged #81533 lifecycle helper with the renderer-log console pipeline that landed in #83535 (the two PRs raced): - window-renderer-lifecycle.ts no longer handles console-message — renderer-log.ts is the single owner (per-window labels, boundary reports). One owner means no double-logged errors on windows wearing both, and OAuth/portal windows (lifecycle-wired for process events) cannot spill third-party page console output into desktop.log. - wake indicator window gets attachRendererConsoleCapture, keeping the console coverage it previously got from the helper. - HUD window (added after the PR branched) gets log-only lifecycle coverage — it was the one renderer window the PR couldn't have known about. - Tests updated: lifecycle helper asserts it attaches NO console-message listener; parser tests live in renderer-log.test.ts. --- apps/desktop/electron/main.ts | 3 + .../desktop/electron/wake-indicator-window.ts | 4 ++ .../window-renderer-lifecycle.test.ts | 24 ++----- .../electron/window-renderer-lifecycle.ts | 72 ++----------------- 4 files changed, 19 insertions(+), 84 deletions(-) diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 3f51a01685627..0a2c4a968584e 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -9538,6 +9538,9 @@ function spawnHudWindow(sessionId, profile) { }) attachRendererConsoleCapture(win, 'hud', rememberLog) + // Log-only lifecycle (#81290): the HUD is a compact auxiliary surface the + // user can re-toggle; a dead renderer should be diagnosable, not resurrected. + installWindowRendererLifecycle(win, { kind: 'hud', callbacks: { log: rememberLog } }) loadWindowUrl(win, hudUrl(sessionId, profile), 'HUD') return win diff --git a/apps/desktop/electron/wake-indicator-window.ts b/apps/desktop/electron/wake-indicator-window.ts index 5ad12260ca125..60b884bf380c8 100644 --- a/apps/desktop/electron/wake-indicator-window.ts +++ b/apps/desktop/electron/wake-indicator-window.ts @@ -2,6 +2,7 @@ import { pathToFileURL } from 'node:url' import { BrowserWindow, screen } from 'electron' +import { attachRendererConsoleCapture } from './renderer-log' import { normalizeWakeIndicatorState, selectWakeIndicatorDisplay, @@ -106,6 +107,9 @@ export function createWakeIndicatorWindowController({ // Log-only renderer lifecycle (#81290): the wake cue is ambient and // macOS-only; its loss belongs in desktop.log, never resurrected. installWindowRendererLifecycle(next, { kind: 'wake', callbacks: { log } }) + // Console errors go through the shared capture (renderer-log.ts owns + // console-message; the lifecycle helper deliberately does not). + attachRendererConsoleCapture(next, 'wake', log) next.webContents.on('did-finish-load', sendState) next.once('ready-to-show', () => { diff --git a/apps/desktop/electron/window-renderer-lifecycle.test.ts b/apps/desktop/electron/window-renderer-lifecycle.test.ts index 1b5620b69f034..57f7c07cd10ff 100644 --- a/apps/desktop/electron/window-renderer-lifecycle.test.ts +++ b/apps/desktop/electron/window-renderer-lifecycle.test.ts @@ -3,7 +3,6 @@ import assert from 'node:assert/strict' import { test } from 'vitest' import { - consoleMessageLog, describeRendererLifecycleEvent, installWindowRendererLifecycle, pruneReloadTimes, @@ -296,21 +295,18 @@ test('did-fail-load on the main frame is logged, not reloaded', () => { assert.equal(logs.length, 1) }) -test('console-message at error level is logged, lower levels ignored', () => { +test('console-message events are NOT handled here (renderer-log.ts is the single owner)', () => { const win = makeFakeWindow() const { logs, options } = makeOptions(win, 'secondary') installWindowRendererLifecycle(win, options) - // Modern shape: (event, messageDetails). + // OAuth/portal windows install this helper for process events; their pages + // must not be able to spill console output (tokens/PII) into desktop.log. win.webContents.emit('console-message', {}, { level: 3, message: 'boom', sourceUrl: 'file:///app.js', lineNumber: 42 }) - // Deprecated positional shape: (event, level, message, line, sourceId). - win.webContents.emit('console-message', {}, 3, 'legacy boom', 7, 'file:///legacy.js') - win.webContents.emit('console-message', {}, { level: 1, message: 'info', sourceUrl: 'file:///app.js', lineNumber: 1 }) - assert.equal(logs.length, 2) - assert.match(logs[0], /\[renderer:secondary console\] boom \(file:\/\/\/app\.js:42\)/) - assert.match(logs[1], /\[renderer:secondary console\] legacy boom \(file:\/\/\/legacy\.js:7\)/) + assert.equal(win.webContents.listenerCount('console-message'), 0) + assert.equal(logs.length, 0) }) test('onCrashLoopSuppressed fires when the budget trips (main sandbox-relaunch hook)', async () => { @@ -371,16 +367,6 @@ test('dispose removes every listener (no stacking on window recreation)', () => assert.equal(win.webContents.listenerCount('render-process-gone'), before - 1) }) -test('consoleMessageLog parses both Electron argument shapes', () => { - const modern = consoleMessageLog([{}, { level: 3, message: 'm', sourceUrl: 's', lineNumber: 1 }]) - - assert.deepEqual(modern, { level: 3, message: 'm', sourceUrl: 's', lineNumber: 1 }) - - const legacy = consoleMessageLog([{}, 3, 'm', 7, 's']) - - assert.deepEqual(legacy, { level: 3, message: 'm', sourceUrl: 's', lineNumber: 7 }) -}) - test('describeRendererLifecycleEvent sanitizes unknown fields', () => { assert.equal(describeRendererLifecycleEvent({ kind: 'secondary', event: 'render-process-gone' }), '[renderer:secondary] render-process-gone reason=? exitCode=?') assert.equal(describeRendererLifecycleEvent({ kind: 'secondary', event: 'render-process-gone', reason: 'crashed', exitCode: undefined }), '[renderer:secondary] render-process-gone reason=crashed exitCode=?') diff --git a/apps/desktop/electron/window-renderer-lifecycle.ts b/apps/desktop/electron/window-renderer-lifecycle.ts index b0066377082c2..06767f3ffc4e9 100644 --- a/apps/desktop/electron/window-renderer-lifecycle.ts +++ b/apps/desktop/electron/window-renderer-lifecycle.ts @@ -22,8 +22,12 @@ // - `did-fail-load` on the MAIN frame → log only (no blind reload: a repeatable // startup failure would boot-loop; the backend startup path already surfaces // the actionable error). -// - `console-message` at error level → log renderer console errors, same as the -// primary window did. +// +// Console-message capture is deliberately NOT here: renderer-log.ts owns it +// (per-window labels, boundary-report formatting). Keeping one owner avoids +// double-logging on windows that have both, and keeps third-party pages +// (OAuth/portal windows, which install this helper for process events) from +// spilling their console output — potentially tokens/PII — into desktop.log. export interface RendererLifecycleDetails { reason?: string @@ -33,7 +37,7 @@ export interface RendererLifecycleDetails { export interface RendererLifecycleEvent { kind: string - event: 'render-process-gone' | 'unresponsive' | 'did-fail-load' | 'console-message' + event: 'render-process-gone' | 'unresponsive' | 'did-fail-load' reason?: string exitCode?: number | string | undefined isDestroyed?: boolean @@ -43,12 +47,6 @@ export interface RendererLifecycleEvent { errorCode?: number | string | undefined /** did-fail-load: the URL that failed. */ url?: string - /** console-message: renderer error text. */ - message?: string - /** console-message: source URL of the console message. */ - sourceUrl?: string - /** console-message: line number of the console message. */ - lineNumber?: number | string | undefined } export interface ReloadPolicyDecision { @@ -160,13 +158,6 @@ export function shouldReloadAfterRendererGone(details: { export function describeRendererLifecycleEvent(event: RendererLifecycleEvent): string { const kind = String(event.kind || '?') - if (event.event === 'console-message') { - const src = String(event.sourceUrl || '?') - const line = event.lineNumber === undefined ? '?' : String(event.lineNumber) - - return `[renderer:${kind} console] ${String(event.message || '(empty)')} (${src}:${line})` - } - if (event.event === 'unresponsive') { return `[renderer:${kind}] webContents became unresponsive` } @@ -185,35 +176,6 @@ export function describeRendererLifecycleEvent(event: RendererLifecycleEvent): s return `[renderer:${kind}] render-process-gone reason=${reason} exitCode=${exitCode}${teardown}` } -// Electron ≥36 passes (event, messageDetails); older/deprecated shape is -// (event, level, message, line, sourceId). Handle both, matching the primary -// window's previous console-message handling. -export function consoleMessageLog(args: readonly unknown[]): { - level: number - message: string - sourceUrl: string - lineNumber: number | string | undefined -} { - const details = args[1] && typeof args[1] === 'object' ? (args[1] as Record) : null - const level = details ? Number(details.level ?? 0) : Number(args[1] ?? 0) - const modernLine = details?.lineNumber - - return { - level, - message: details ? String(details.message ?? '') : String(args[2] ?? ''), - sourceUrl: details ? String(details.sourceUrl ?? '') : String(args[4] ?? ''), - lineNumber: details - ? typeof modernLine === 'number' - ? modernLine - : modernLine === undefined - ? undefined - : String(modernLine) - : typeof args[3] === 'number' - ? args[3] - : String(args[3] ?? '') - } -} - /** * Attach renderer lifecycle listeners to a window. Returns a dispose() that * removes every listener (window recreation must not stack handlers). @@ -302,28 +264,9 @@ export function installWindowRendererLifecycle( } } - const onConsoleMessage = (...args: unknown[]) => { - const parsed = consoleMessageLog(args) - - if (parsed.level !== 3) { - return - } - - log( - describeRendererLifecycleEvent({ - kind, - event: 'console-message', - message: parsed.message, - sourceUrl: parsed.sourceUrl, - lineNumber: parsed.lineNumber - }) - ) - } - contents.on('render-process-gone', onRendererGone) contents.on('unresponsive', onUnresponsive) contents.on('did-fail-load', onDidFailLoad) - contents.on('console-message', onConsoleMessage) let disposed = false @@ -338,7 +281,6 @@ export function installWindowRendererLifecycle( contents.removeListener('render-process-gone', onRendererGone) contents.removeListener('unresponsive', onUnresponsive) contents.removeListener('did-fail-load', onDidFailLoad) - contents.removeListener('console-message', onConsoleMessage) } } }