fix(desktop): expand export diagnostics for startup failures#600
fix(desktop): expand export diagnostics for startup failures#600
Conversation
📝 WalkthroughWalkthroughThis PR introduces a comprehensive startup probe diagnostic system that records initialization events across the main process, preload, and renderer layers. Probes capture source, stage, status, and optional details with timestamps. The system collects these into bounded entries and includes machine info and app signing summaries during diagnostics export. Changes
Sequence DiagramsequenceDiagram
participant Main as Main Process
participant Preload as Preload
participant Renderer as Renderer
participant DiagReporter as Diag Reporter
participant IPC as IPC Channel
rect rgba(100, 200, 150, 0.5)
Note over Main: Startup Phase
Main->>DiagReporter: recordStartupProbe(app-when-ready)
Main->>Preload: Create webContents
Main->>IPC: Register host:startup-probe handler
end
rect rgba(150, 150, 200, 0.5)
Note over Preload: Preload Initialization
Preload->>Preload: Emit startup-phase probe
Preload->>IPC: reportStartupProbe via IPC
IPC->>DiagReporter: Forward payload
Preload->>Preload: Register error handlers
end
rect rgba(200, 150, 100, 0.5)
Note over Renderer: Renderer Startup
Renderer->>Renderer: Send renderer:module-start
Renderer->>IPC: reportStartupProbe (module-start)
IPC->>DiagReporter: Record probe
Renderer->>Renderer: Register window error/rejection listeners
Renderer->>Renderer: Initialize React (emit start probe)
Renderer->>Renderer: Mount RendererTelemetryBootstrap
Renderer->>IPC: reportStartupProbe (sentry/amplitude init)
IPC->>DiagReporter: Record probe
Renderer->>Renderer: Mount RendererStartupSentinel
Renderer->>IPC: reportStartupProbe (react-render:committed)
IPC->>DiagReporter: Record probe
end
rect rgba(100, 150, 200, 0.5)
Note over Main: Lifecycle Events
Main->>IPC: did-finish-load event
IPC->>DiagReporter: recordStartupProbe
Main->>IPC: ready-to-show event
IPC->>DiagReporter: recordStartupProbe
end
Note over DiagReporter: Entries trimmed to max 200<br/>Flags set for preload/renderer seen
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14ec48bd99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/main/diagnostics-export.ts (1)
407-458: Consider reusing the file buffer to avoid double read.The desktop diagnostics file is read twice: once via
addFile()for inclusion in the ZIP (line 407-413), and again viatryReadFile()for parsing (line 416-417). While not a blocking issue, you could reuse the buffer from the first read to improve efficiency.♻️ Optional refactor to reuse file buffer
- const desktopDiagnosticsMetadata = await addFile( - "diagnostics/desktop-diagnostics.json", - getDesktopDiagnosticsFilePath(), - { - redact: true, - }, - ); - - if (desktopDiagnosticsMetadata) { - const desktopDiagnosticsFile = await tryReadFile( - getDesktopDiagnosticsFilePath(), - ); - const parsedDiagnostics = desktopDiagnosticsFile - ? parseJsonBuffer<...>(desktopDiagnosticsFile.data) + const desktopDiagnosticsFile = await tryReadFile(getDesktopDiagnosticsFilePath()); + + if (desktopDiagnosticsFile) { + // Add redacted version to ZIP + const redactedData = redactJsonBuffer(desktopDiagnosticsFile.data); + entries.push({ + name: `${archiveRoot}/diagnostics/desktop-diagnostics.json`, + data: redactedData, + modTime: desktopDiagnosticsFile.mtime, + }); + included.push("diagnostics/desktop-diagnostics.json"); + + // Parse original (unredacted) for summary extraction + const parsedDiagnostics = parseJsonBuffer<...>(desktopDiagnosticsFile.data);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/main/diagnostics-export.ts` around lines 407 - 458, The code reads the same diagnostics file twice (once via addFile(...) and again via tryReadFile(...)); instead, reuse the file buffer produced during adding to the archive: adjust addFile or its call so you capture the file buffer (or make addFile return { metadata, data } or expose the read Buffer) and then pass that Buffer to parseJsonBuffer(...) instead of calling tryReadFile(getDesktopDiagnosticsFilePath()); update references around desktopDiagnosticsMetadata, desktopDiagnosticsFile and parsedDiagnostics to use the captured buffer when available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/main/diagnostics-export.ts`:
- Around line 240-271: The runCommand function crashes when spawnSync returns
undefined stdout/stderr; change the assignments for stdout and stderr to safely
handle undefined by defaulting to empty string before trimming (e.g., use
result.stdout ?? "" and result.stderr ?? ""), then preserve the existing logic
to map empty -> null; update any type expectations if needed and keep the rest
of runCommand (ok, status, signal, error) unchanged so all call sites continue
to receive the same shape.
---
Nitpick comments:
In `@apps/desktop/main/diagnostics-export.ts`:
- Around line 407-458: The code reads the same diagnostics file twice (once via
addFile(...) and again via tryReadFile(...)); instead, reuse the file buffer
produced during adding to the archive: adjust addFile or its call so you capture
the file buffer (or make addFile return { metadata, data } or expose the read
Buffer) and then pass that Buffer to parseJsonBuffer(...) instead of calling
tryReadFile(getDesktopDiagnosticsFilePath()); update references around
desktopDiagnosticsMetadata, desktopDiagnosticsFile and parsedDiagnostics to use
the captured buffer when available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 77c6c035-1375-43c9-93a9-747ed196807e
📒 Files selected for processing (11)
apps/desktop/main/desktop-diagnostics.tsapps/desktop/main/diagnostics-export.tsapps/desktop/main/index.tsapps/desktop/main/ipc.tsapps/desktop/preload/index.tsapps/desktop/preload/webview-preload.tsapps/desktop/shared/host.tsapps/desktop/src/lib/host-api.tsapps/desktop/src/main.tsxspecs/current/diagnostics/export-diagnostics.mdspecs/current/diagnostics/trigger-export-diagnostics.md
What
Expand desktop
Export Diagnosticsto capture more startup evidence, especially for Intel macOS launch failures.Why
Some desktop users hit early startup failures where the existing diagnostics bundle was not enough to identify whether the crash happened in preload, renderer bootstrap, telemetry initialization, or native macOS packaging/signing layers.
How
desktop-diagnostics.jsonspecs/current/diagnostics/summary/manifest.jsonso warnings reflect missing files and the manifest includes itself in the included file listAffected areas
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpassespnpm generate-typesrun (if API routes/schemas changed)anytypes introduced (useunknownwith narrowing)Notes for reviewers
.tmp/and confirmed the new bundle containsmachine-info.json,app-signing.json, andstartup-probe-summary.json.pnpm lintstill reports one pre-existing test warning about an unused variable intests/desktop/data-directory-runtime.test.ts.Summary by CodeRabbit
Release Notes
New Features
Documentation