fix(crash-reporting): let a late sibling amend withdraw the attribution it disproves

A renderer crash is labelled `crashAttribution: concurrent-process-deaths` from the
sibling set known at record time. A child that dies afterwards is folded in by
`attachDetails`, which merges — so when the wider set turns out to be a crash-looping
child, `siblingProcessDeathDetails` merely OMITS the key and the stale label survives.
Report 11a9d459 shipped `concurrent-process-deaths` next to
`siblingProcessDeathRepeats: 2`, a pair `isOneIncident()` never emits.

`attachDetails` now treats a null value as a withdrawal, and the late attribution emits
`crashAttribution: null` once the report has been given a label the new evidence
disproves. The flag is sticky, so a dropped fire-and-forget write is retried by the
next late sibling.

src/main/crash-reporting/crash-report-store.ts

@@ -9,6 +9,7 @@ import {
   sanitizeCrashReportBreadcrumbs,
   sanitizeCrashReportDetails,
   type CrashReportCreateInput,
+  type CrashReportDetailValue,
   type CrashReportRecord,
   type CrashReportStatus
 } from '../../shared/crash-reporting'
@@ -39,6 +40,26 @@ function isRelatedCrashEvent(anchor: CrashReportRecord, candidate: CrashReportRe
   )
 }
 
+/**
+ * Applies an amend. A `null` value WITHDRAWS the key rather than storing it — see
+ * `attachDetails`. Nothing emits a null detail as data, and `record` never routes
+ * through here, so the initial write can still store one if a producer ever needs it.
+ */
+function mergeCrashReportDetails(
+  details: Record<string, CrashReportDetailValue>,
+  extraDetails: Record<string, unknown>
+): Record<string, CrashReportDetailValue> {
+  const merged = { ...details }
+  for (const [key, value] of Object.entries(sanitizeCrashReportDetails(extraDetails))) {
+    if (value === null) {
+      delete merged[key]
+    } else {
+      merged[key] = value
+    }
+  }
+  return merged
+}
+
 function isRetryableWindowsFileOperationError(error: unknown): boolean {
   const code = (error as NodeJS.ErrnoException).code
   return code === 'EPERM' || code === 'EACCES' || code === 'EBUSY'
@@ -111,6 +132,11 @@ export class CrashReportStore {
    * Merges late-arriving details into an existing report. Crashpad finishes
    * writing the minidump after the process-gone event that created the record,
    * so the signature can only be folded in afterwards.
+   *
+   * A `null` value withdraws the key instead of storing it: an amend can also disprove a
+   * label the first write made on partial evidence, and a merge alone could never take one
+   * back (report 11a9d459 kept a one-incident sibling attribution beside the repeat count
+   * that contradicted it).
    */
   async attachDetails(
     id: string,
@@ -122,10 +148,7 @@ export class CrashReportStore {
         if (report.id !== id) {
           return report
         }
-        result = {
-          ...report,
-          details: { ...report.details, ...sanitizeCrashReportDetails(extraDetails) }
-        }
+        result = { ...report, details: mergeCrashReportDetails(report.details, extraDetails) }
         return result
       })
       return { reports: nextReports, result }

src/main/crash-reporting/process-gone-sibling-attribution-withdrawal.test.ts

@@ -0,0 +1,229 @@
+import fs from 'node:fs/promises'
+import os from 'node:os'
+import path from 'node:path'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+vi.mock('electron', () => ({
+  app: {
+    getVersion: () => '1.4.199-test',
+    getAppMetrics: () => [],
+    getPath: () => os.tmpdir()
+  }
+}))
+
+import { CrashReportStore } from './crash-report-store'
+import { clearCrashBreadcrumbsForTest } from './crash-breadcrumb-store'
+import { ProcessGoneDedupe } from './process-gone-dedupe'
+import { recordProcessGoneCrash, type ProcessGoneCrashEvent } from './process-gone-recorder'
+import {
+  collectLateSiblingAttributions,
+  resetProcessGoneSiblingCorrelationForTest,
+  trackRendererCrashReport,
+  type ChildProcessDeath
+} from './process-gone-sibling-correlation'
+import { _resetTracerForTests } from '../observability/tracer'
+
+// Field shape: crash report 11a9d459 (v1.4.199, win32 10.0.19045), renderer
+// crashed/-2147483645 (0x80000003) with a GPU process crash-looping at -17ms / +14ms / +61ms.
+const BREAKPOINT_EXIT = -2_147_483_645
+const GONE_AT = 1_789_021_889_062
+
+const noMinidump = async () => null
+const originalPlatform = process.platform
+
+function gpuChildCrash(): ProcessGoneCrashEvent {
+  return {
+    source: 'child',
+    processType: 'GPU',
+    reason: 'crashed',
+    exitCode: BREAKPOINT_EXIT,
+    expectedTeardown: 'none',
+    details: { serviceName: 'GPU', type: 'GPU' }
+  }
+}
+
+function networkServiceCrash(): ProcessGoneCrashEvent {
+  return {
+    source: 'child',
+    processType: 'Utility',
+    reason: 'crashed',
+    exitCode: BREAKPOINT_EXIT,
+    expectedTeardown: 'none',
+    details: { serviceName: 'network.mojom.NetworkService', type: 'Utility' }
+  }
+}
+
+function rendererCrash(): ProcessGoneCrashEvent {
+  return {
+    source: 'renderer',
+    processType: 'renderer',
+    reason: 'crashed',
+    exitCode: BREAKPOINT_EXIT,
+    expectedTeardown: 'none',
+    details: { processType: 'renderer' },
+    webContentsId: 1
+  }
+}
+
+/** The amend is fire-and-forget behind two real file writes, so poll the stored document. */
+async function rendererDetails(
+  store: CrashReportStore,
+  expectedSiblingCount: number
+): Promise<Record<string, unknown>> {
+  return vi.waitFor(async () => {
+    const [report] = await store.listRecent()
+    expect(report?.details.siblingProcessDeathCount).toBe(expectedSiblingCount)
+    return report.details
+  })
+}
+
+describe('sibling attribution withdrawal', () => {
+  let directory: string
+  let store: CrashReportStore
+
+  beforeEach(async () => {
+    Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
+    directory = await fs.mkdtemp(path.join(os.tmpdir(), 'orca-sibling-attr-'))
+    store = new CrashReportStore(path.join(directory, 'crash-reports.json'))
+    resetProcessGoneSiblingCorrelationForTest()
+    clearCrashBreadcrumbsForTest()
+    _resetTracerForTests()
+  })
+
+  afterEach(async () => {
+    Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
+    vi.restoreAllMocks()
+    resetProcessGoneSiblingCorrelationForTest()
+    clearCrashBreadcrumbsForTest()
+    _resetTracerForTests()
+    await fs.rm(directory, { recursive: true, force: true })
+  })
+
+  it('drops crashAttribution once a repeating sibling identity disqualifies it', async () => {
+    const dedupe = new ProcessGoneDedupe()
+    const now = vi.spyOn(Date, 'now')
+
+    now.mockReturnValue(GONE_AT - 17)
+    recordProcessGoneCrash(store, gpuChildCrash(), dedupe, noMinidump)
+
+    now.mockReturnValue(GONE_AT)
+    recordProcessGoneCrash(store, rendererCrash(), dedupe, noMinidump)
+
+    // Same GPU identity dies twice more: a crash-looping child, which
+    // isOneIncident() explicitly refuses to call concurrent-process-deaths.
+    now.mockReturnValue(GONE_AT + 14)
+    recordProcessGoneCrash(store, gpuChildCrash(), dedupe, noMinidump)
+    now.mockReturnValue(GONE_AT + 61)
+    recordProcessGoneCrash(store, gpuChildCrash(), dedupe, noMinidump)
+
+    const details = await rendererDetails(store, 3)
+    expect(details.siblingProcessDeathRepeats).toBe(2)
+    expect(details.siblingProcessDeaths).toBe('GPU +14ms, GPU -17ms, GPU +61ms')
+    expect(details).not.toHaveProperty('crashAttribution')
+  })
+
+  it('keeps the label when a second distinct sibling leaves the incident intact', async () => {
+    const dedupe = new ProcessGoneDedupe()
+    const now = vi.spyOn(Date, 'now')
+
+    now.mockReturnValue(GONE_AT - 17)
+    recordProcessGoneCrash(store, gpuChildCrash(), dedupe, noMinidump)
+    now.mockReturnValue(GONE_AT)
+    recordProcessGoneCrash(store, rendererCrash(), dedupe, noMinidump)
+    now.mockReturnValue(GONE_AT + 14)
+    recordProcessGoneCrash(store, networkServiceCrash(), dedupe, noMinidump)
+
+    const details = await rendererDetails(store, 2)
+    expect(details.siblingProcessDeathRepeats).toBeUndefined()
+    expect(details.crashAttribution).toBe('concurrent-process-deaths')
+  })
+
+  it('withdraws only the disproved key and leaves the rest of the report intact', async () => {
+    const recorded = await store.record({
+      source: 'renderer',
+      processType: 'renderer',
+      reason: 'crashed',
+      exitCode: BREAKPOINT_EXIT,
+      appVersion: '1.4.199-test',
+      platform: 'win32',
+      osRelease: '10.0.19045',
+      arch: 'x64',
+      electronVersion: '38.0.0',
+      chromeVersion: '140.0.0.0',
+      details: { crashAttribution: 'concurrent-process-deaths', minidumpStatus: 'captured' }
+    })
+
+    const amended = await store.attachDetails(recorded.id, {
+      crashAttribution: null,
+      siblingProcessDeathRepeats: 2
+    })
+
+    expect(amended?.details).toEqual({
+      minidumpStatus: 'captured',
+      siblingProcessDeathRepeats: 2
+    })
+    // Withdrawal must survive the round trip, not just the in-memory result.
+    const [persisted] = await store.listRecent()
+    expect(persisted.details).not.toHaveProperty('crashAttribution')
+  })
+})
+
+/**
+ * The amend is fire-and-forget: `attachAttribution` logs a failed write and moves on. A
+ * withdrawal that never reached the file has to stay owed, or the Windows retry ladder
+ * exhausting once (EPERM/EBUSY) ships the stale label the amend existed to take back.
+ */
+describe('a withdrawal amend whose write never lands', () => {
+  const gpuDeath = (at: number): ChildProcessDeath => ({
+    at,
+    processType: 'GPU',
+    serviceName: 'GPU',
+    reason: 'crashed',
+    exitCode: BREAKPOINT_EXIT
+  })
+
+  beforeEach(() => {
+    Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' })
+    resetProcessGoneSiblingCorrelationForTest()
+  })
+
+  afterEach(() => {
+    Object.defineProperty(process, 'platform', { configurable: true, value: originalPlatform })
+    resetProcessGoneSiblingCorrelationForTest()
+  })
+
+  it('re-emits it on the next late sibling instead of reporting it withdrawn', () => {
+    trackRendererCrashReport(
+      {
+        at: GONE_AT,
+        reason: 'crashed',
+        exitCode: BREAKPOINT_EXIT,
+        attachAttribution: () => {
+          throw new Error('the amend is fire-and-forget; this write is dropped')
+        }
+      },
+      [gpuDeath(GONE_AT - 17)]
+    )
+
+    const [dropped] = collectLateSiblingAttributions(gpuDeath(GONE_AT + 14))
+    expect(dropped.attribution.crashAttribution).toBeNull()
+    const [retried] = collectLateSiblingAttributions(gpuDeath(GONE_AT + 61))
+    expect(retried.attribution.crashAttribution).toBeNull()
+  })
+
+  it('never withdraws a label the report was never given', () => {
+    trackRendererCrashReport(
+      {
+        at: GONE_AT,
+        reason: 'crashed',
+        exitCode: BREAKPOINT_EXIT,
+        attachAttribution: () => {}
+      },
+      // -900ms is in-window but too loose to attribute, so nothing was ever written.
+      [gpuDeath(GONE_AT - 900)]
+    )
+
+    const [attribution] = collectLateSiblingAttributions(gpuDeath(GONE_AT + 14))
+    expect(attribution.attribution).not.toHaveProperty('crashAttribution')
+  })
+})

src/main/crash-reporting/process-gone-sibling-correlation.test.ts

@@ -215,7 +215,8 @@ describe('sibling process-death attribution', () => {
     await vi.waitFor(() => expect(siblingAttaches(store)).toHaveLength(2))
     expect(siblingAttaches(store)[0]).toMatchObject({ crashAttribution: CONCURRENT })
     expect(siblingAttaches(store)[1]).toMatchObject({ siblingProcessDeathRepeats: 1 })
-    expect(siblingAttaches(store)[1]).not.toHaveProperty('crashAttribution')
+    // Null withdraws the label the first attach wrote: a merge alone can never take it back.
+    expect(siblingAttaches(store)[1]).toMatchObject({ crashAttribution: null })
   })
 
   it('does not erase recorded sibling evidence during unrelated child churn', async () => {

src/main/crash-reporting/process-gone-sibling-correlation.ts

@@ -69,6 +69,12 @@ export type PendingRendererCrashReport = {
 type TrackedRendererCrashReport = PendingRendererCrashReport & {
   siblingDeaths: ChildProcessDeath[]
   lateAttaches: number
+  /**
+   * Whether the one-incident label may still be on the persisted report. Sticky once set:
+   * the amend that withdraws it is fire-and-forget, so a failed write must leave the next
+   * amend still trying rather than treating the label as already gone.
+   */
+  attributedOneIncident: boolean
 }
 
 export type LateSiblingAttribution = {
@@ -135,7 +141,12 @@ export function trackRendererCrashReport(
     .slice(-(MAX_PENDING_RENDERER_REPORTS - 1))
   pendingRendererReports = [
     ...liveReports,
-    { ...pending, siblingDeaths: [...siblingDeaths], lateAttaches: 0 }
+    {
+      ...pending,
+      siblingDeaths: [...siblingDeaths],
+      lateAttaches: 0,
+      attributedOneIncident: isOneIncident(siblingDeaths, pending.at)
+    }
   ]
 }
 
@@ -181,13 +192,22 @@ function isOneIncident(siblings: ChildProcessDeath[], rendererAt: number): boole
   )
 }
 
+/**
+ * `withdrawStaleAttribution` is for an amend onto a report that already carries the label:
+ * attachDetails merges, so evidence that disproves one incident (report 11a9d459 shipped
+ * `crashAttribution=concurrent-process-deaths` beside `siblingProcessDeathRepeats=2`) has
+ * to say so explicitly. A null detail value withdraws the key.
+ */
 export function siblingProcessDeathDetails(
   siblings: ChildProcessDeath[],
-  rendererAt: number
+  rendererAt: number,
+  { withdrawStaleAttribution = false }: { withdrawStaleAttribution?: boolean } = {}
 ): Record<string, CrashReportDetailValue> {
   const repeats = repeatedIdentityCount(siblings)
+  const oneIncident = isOneIncident(siblings, rendererAt)
   return {
-    ...(isOneIncident(siblings, rendererAt) ? { crashAttribution: CONCURRENT_PROCESS_DEATHS } : {}),
+    ...(oneIncident ? { crashAttribution: CONCURRENT_PROCESS_DEATHS } : {}),
+    ...(!oneIncident && withdrawStaleAttribution ? { crashAttribution: null } : {}),
     siblingProcessDeathCount: siblings.length,
     ...(repeats > 0 ? { siblingProcessDeathRepeats: repeats } : {}),
     siblingProcessDeaths: describeSiblingDeaths(siblings, rendererAt)
@@ -212,10 +232,11 @@ export function collectLateSiblingAttributions(death: ChildProcessDeath): LateSi
     }
     pending.siblingDeaths.push(death)
     pending.lateAttaches += 1
-    attributions.push({
-      pending,
-      attribution: siblingProcessDeathDetails(pending.siblingDeaths, pending.at)
+    const attribution = siblingProcessDeathDetails(pending.siblingDeaths, pending.at, {
+      withdrawStaleAttribution: pending.attributedOneIncident
     })
+    pending.attributedOneIncident ||= attribution.crashAttribution === CONCURRENT_PROCESS_DEATHS
+    attributions.push({ pending, attribution })
   }
   return attributions
 }