From 01e08ea43d450251a95896c8b18463ecff9eedd2 Mon Sep 17 00:00:00 2001
From: Neil <4138956+nwparker@users.noreply.github.com>
Date: Sat, 1 Aug 2026 06:53:17 -0700
Subject: [PATCH] fix(crash-reporting): stop the sibling sweep re-admitting
 excluded crashes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`markRendererCrashesAutoRecovered` rules on each pending renderer-process
report against `notBeforeMs`, then sweeps burst siblings. The sweep did not
honour the first pass's decision: `isRelatedCrashEvent` ignores processType
and reaches 5s either side of any anchor, and the guard only excluded
react-render. A renderer crash the cutoff had just excluded was therefore
dismissed anyway as a "burst sibling" of a crash from the same process,
pushing effective reach to cutoff + 5s and dropping the
`rendererAutoRecovered` flag that records why it was resolved.

Collapse the guard to `status !== 'pending' || source === 'renderer'`. Every
renderer-sourced report is either an anchor the first pass already ruled on,
or a react-render boundary report the reload did not cause; neither is ever a
burst sibling, which is what the sweep exists to catch. This subsumes the
previous react-render exclusion rather than replacing it — deleting the whole
guard reds three tests, the two pre-existing boundary ones plus the new
cutoff one, so both protections still ride on it.

Scope, so a reader does not overrate this: the newest anchor normally sits
250ms before the arm (the recovery timer delay), so the naturally exploitable
band is about 250ms wide. It only widens toward the full 5s when the arm
lands late relative to its crash, or through a chain of anchors. The fix is
worth making on structural grounds regardless — the sweep should not be able
to override the cutoff the anchor pass just applied.
---
 .../crash-report-store.test.ts                | 24 +++++++++++++++++++
 .../crash-reporting/crash-report-store.ts     | 13 +++++-----
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/src/main/crash-reporting/crash-report-store.test.ts b/src/main/crash-reporting/crash-report-store.test.ts
index ed08a903f79..22ccaa73076 100644
--- a/src/main/crash-reporting/crash-report-store.test.ts
+++ b/src/main/crash-reporting/crash-report-store.test.ts
@@ -48,6 +48,7 @@ function input(reason = 'crashed'): CrashReportCreateInput {
 }
 
 afterEach(async () => {
+  vi.useRealTimers()
   vi.restoreAllMocks()
   grantDirAclAsyncMock.mockReset()
   grantDirAclAsyncMock.mockResolvedValue(undefined)
@@ -125,6 +126,29 @@ describe('CrashReportStore', () => {
     })
   })
 
+  it('never sweeps a renderer crash the auto-recovery cutoff excluded', async () => {
+    const { store } = await createStore()
+    // Why fake Date: the sweep matches within 5s of an anchor, so the two
+    // records must sit on either side of the cutoff by construction.
+    vi.useFakeTimers({ toFake: ['Date'] })
+    const startedAtMs = Date.parse('2026-08-01T00:00:00.000Z')
+    vi.setSystemTime(startedAtMs)
+    const stale = await store.record(input())
+    vi.setSystemTime(startedAtMs + 4_000)
+    const anchor = await store.record(input())
+
+    // The reload was armed 6s after the stale crash, so only the anchor is in scope.
+    await store.markRendererCrashesAutoRecovered(startedAtMs + 1_000)
+
+    const reports = await store.listRecent()
+    expect(reports).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({ id: anchor.id, status: 'dismissed' }),
+        expect.objectContaining({ id: stale.id, status: 'pending' })
+      ])
+    )
+  })
+
   it('dismisses sibling pending records after one crash report is sent', async () => {
     const { store } = await createStore()
     await store.record(input())
diff --git a/src/main/crash-reporting/crash-report-store.ts b/src/main/crash-reporting/crash-report-store.ts
index 6ca3ff63f17..ff1b89755a3 100644
--- a/src/main/crash-reporting/crash-report-store.ts
+++ b/src/main/crash-reporting/crash-report-store.ts
@@ -166,13 +166,12 @@ export class CrashReportStore {
       // healed crash still reaches the user, just wearing the sibling's report.
       const anchors = [...resolved]
       const sweptReports = nextReports.map((report) => {
-        // Why: relation matching ignores processType, so re-state the anchor
-        // pass's exclusion — a renderer-sourced report that is not the renderer
-        // process is the error boundary reporting itself, not a burst sibling.
-        if (
-          report.status !== 'pending' ||
-          (report.source === 'renderer' && report.processType !== 'renderer')
-        ) {
+        // Why: no renderer-sourced report is a burst sibling. The anchor pass
+        // already ruled on the renderer process, and relation matching (which
+        // ignores processType and reaches 5s either side of an anchor) would
+        // otherwise re-admit the older crashes `notBeforeMs` just excluded, or
+        // an error-boundary report that no reload caused.
+        if (report.status !== 'pending' || report.source === 'renderer') {
           return report
         }
         if (!anchors.some((anchor) => isRelatedCrashEvent(anchor, report))) {