From 153d771e53337ece0859140c9e398fe00c2a4d14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 27 Aug 2026 20:21:42 +0200 Subject: [PATCH 1/3] fix: dispose late durable capture authorities --- ...durable-capture-recovery-authority.test.ts | 166 ++++++++++++++++++ .../durable-capture-recovery-authority.ts | 59 ++++++- 2 files changed, 223 insertions(+), 2 deletions(-) diff --git a/src/daemon/__tests__/durable-capture-recovery-authority.test.ts b/src/daemon/__tests__/durable-capture-recovery-authority.test.ts index 8e6b017e40..6b8a9edb0c 100644 --- a/src/daemon/__tests__/durable-capture-recovery-authority.test.ts +++ b/src/daemon/__tests__/durable-capture-recovery-authority.test.ts @@ -18,6 +18,31 @@ const envelope = createDurableResourceEnvelope({ descriptor: { version: 1, body: {} }, }); +test('acquisition winner leaves authority disposal to the caller', async () => { + const disposeControl = vi.fn(async () => {}); + const authority = await acquireDurableCaptureRecoveryAuthorityBeforeDeadline({ + displayName: 'app-log', + envelope, + scope: { + signal: new AbortController().signal, + diagnostics: { emit: () => {} }, + progress: { report: () => {} }, + }, + deadlineMs: 10_000, + acquireControl: async () => ({ + reattach: async () => ({ status: 'missing' as const }), + cleanup: async () => ({ status: 'already-missing' as const }), + [Symbol.asyncDispose]: disposeControl, + }), + onLateCleanupFailure: () => {}, + }); + + expect(authority.reattached).toEqual({ status: 'missing' }); + expect(disposeControl).not.toHaveBeenCalled(); + await authority.control[Symbol.asyncDispose](); + expect(disposeControl).toHaveBeenCalledOnce(); +}); + test('deadline abort disposes authority that becomes active after the caller has timed out', async () => { vi.useFakeTimers(); try { @@ -73,6 +98,147 @@ test('deadline abort disposes authority that becomes active after the caller has } }); +test.each(['active', 'missing'] as const)( + 'deadline disposes a late resolved %s authority after the race has timed out', + async (lateStatus) => { + vi.useFakeTimers(); + const combinedSignal = vi + .spyOn(AbortSignal, 'any') + .mockReturnValue(new AbortController().signal); + try { + let resolveReattach!: ( + outcome: { status: 'active'; handle: AppLogLiveHandle } | { status: 'missing' }, + ) => void; + const disposalOrder: string[] = []; + const forceCleanup = vi.fn(async () => { + disposalOrder.push('handle'); + return { status: 'cleaned' } as const; + }); + const disposeControl = vi.fn(async () => { + disposalOrder.push('control'); + }); + const acquisition = acquireDurableCaptureRecoveryAuthorityBeforeDeadline({ + displayName: 'app-log', + envelope, + scope: { + signal: new AbortController().signal, + diagnostics: { emit: () => {} }, + progress: { report: () => {} }, + }, + deadlineMs: 25, + acquireControl: async () => ({ + reattach: async () => + await new Promise< + { status: 'active'; handle: AppLogLiveHandle } | { status: 'missing' } + >((resolve) => { + resolveReattach = resolve; + }), + cleanup: async () => ({ status: 'already-missing' as const }), + [Symbol.asyncDispose]: disposeControl, + }), + onLateCleanupFailure: () => {}, + }); + + const timedOut = expect(acquisition).rejects.toBeInstanceOf( + DurableCaptureRecoveryDeadlineError, + ); + await vi.advanceTimersByTimeAsync(25); + await timedOut; + + resolveReattach( + lateStatus === 'active' + ? { + status: 'active', + handle: createTestAppLogLiveHandle({ + inspect: () => ({ backend: 'android', state: 'active', startedAt: 1 }), + finish: async () => ({ + status: 'completed', + alreadyCompleted: true, + result: { backend: 'android', outputPath: '/tmp/app.log', completedAt: 1 }, + }), + forceCleanup, + }), + } + : { status: 'missing' }, + ); + await vi.advanceTimersByTimeAsync(0); + + expect(forceCleanup).toHaveBeenCalledTimes(lateStatus === 'active' ? 1 : 0); + expect(disposeControl).toHaveBeenCalledOnce(); + expect(disposalOrder).toEqual(lateStatus === 'active' ? ['handle', 'control'] : ['control']); + } finally { + combinedSignal.mockRestore(); + vi.useRealTimers(); + } + }, +); + +test.each(['deadline', 'cancellation'] as const)( + 'late control cleanup failure remains secondary to the %s error', + async (winner) => { + vi.useFakeTimers(); + const combinedSignal = vi + .spyOn(AbortSignal, 'any') + .mockReturnValue(new AbortController().signal); + try { + const controller = new AbortController(); + const cancellation = new Error('request canceled'); + const cleanupError = new Error('late cleanup failed'); + const cleanupFailures = vi.fn(() => { + throw new Error('cleanup diagnostic reporter failed'); + }); + let resolveReattach!: (outcome: { status: 'missing' }) => void; + const acquisition = acquireDurableCaptureRecoveryAuthorityBeforeDeadline({ + displayName: 'app-log', + envelope, + scope: { + signal: controller.signal, + diagnostics: { emit: () => {} }, + progress: { report: () => {} }, + }, + deadlineMs: 25, + acquireControl: async () => ({ + reattach: async () => + await new Promise<{ status: 'missing' }>((resolve) => { + resolveReattach = resolve; + }), + cleanup: async () => ({ status: 'already-missing' as const }), + [Symbol.asyncDispose]: async () => { + throw cleanupError; + }, + }), + onLateCleanupFailure: cleanupFailures, + }); + const primaryErrorPromise = acquisition.catch((error: unknown) => error); + await Promise.resolve(); + + if (winner === 'deadline') { + await vi.advanceTimersByTimeAsync(25); + } else { + controller.abort(cancellation); + } + const primaryError = await primaryErrorPromise; + + resolveReattach({ status: 'missing' }); + await vi.advanceTimersByTimeAsync(0); + + if (winner === 'deadline') { + expect(primaryError).toBeInstanceOf(DurableCaptureRecoveryDeadlineError); + } else { + expect(primaryError).toBe(cancellation); + } + expect(cleanupFailures).toHaveBeenCalledWith( + 'late_control_cleanup_failed', + cleanupError, + primaryError, + ); + } finally { + combinedSignal.mockRestore(); + vi.useRealTimers(); + } + }, +); + test('request cancellation wins before the deadline and disposes late exact-owner control', async () => { const controller = new AbortController(); const cancellation = new Error('request canceled'); diff --git a/src/daemon/durable-capture-recovery-authority.ts b/src/daemon/durable-capture-recovery-authority.ts index e77d3083ca..4591da3c4f 100644 --- a/src/daemon/durable-capture-recovery-authority.ts +++ b/src/daemon/durable-capture-recovery-authority.ts @@ -83,13 +83,68 @@ export async function acquireDurableCaptureRecoveryAuthorityBeforeDeadline< }, params.deadlineMs); timer.unref?.(); const acquisition = acquireRecoveryAuthority(params, scope); + type RaceWinner = + | Readonly<{ kind: 'acquisition' }> + | Readonly<{ kind: 'acquisition-error' }> + | Readonly<{ kind: 'deadline'; error: DurableCaptureRecoveryDeadlineError }> + | Readonly<{ kind: 'cancellation'; error: unknown }>; + let raceWinner: RaceWinner | undefined; + const acquisitionForRace = acquisition.then( + (authority) => { + raceWinner ??= { kind: 'acquisition' }; + return authority; + }, + (error: unknown) => { + raceWinner ??= { kind: 'acquisition-error' }; + throw error; + }, + ); + const deadlineForRace = deadline.catch((error: DurableCaptureRecoveryDeadlineError) => { + raceWinner ??= { kind: 'deadline', error }; + throw error; + }); + const cancellationForRace = cancellation.catch((error: unknown) => { + raceWinner ??= { kind: 'cancellation', error }; + throw error; + }); try { - return await Promise.race([acquisition, deadline, cancellation]); + return await Promise.race([acquisitionForRace, deadlineForRace, cancellationForRace]); } finally { clearTimeout(timer); stopListeningForCancellation(); - void acquisition.catch(() => {}); + if (raceWinner?.kind === 'deadline' || raceWinner?.kind === 'cancellation') { + const primaryError = raceWinner.error; + void acquisition + .then( + (authority) => disposeLateRecoveryAuthority(params, authority, primaryError), + () => {}, + ) + .catch(() => {}); + } else { + void acquisition.catch(() => {}); + } + } +} + +async function disposeLateRecoveryAuthority, C>( + params: DurableCaptureRecoveryAuthorityParams, + authority: DurableCaptureRecoveryAuthority, + primaryError: unknown, +): Promise { + if (authority.reattached.status === 'active') { + await disposeLateAuthority( + params, + authority.reattached.handle, + 'late_handle_cleanup_failed', + primaryError, + ); } + await disposeLateAuthority( + params, + authority.control, + 'late_control_cleanup_failed', + primaryError, + ); } async function acquireRecoveryAuthority, C>( From 71c51b0644a3170f11373567a362bb9bf17a8e95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 27 Aug 2026 20:47:57 +0200 Subject: [PATCH 2/3] fix: preserve late authority control cleanup --- ...durable-capture-recovery-authority.test.ts | 72 +++++++++++++++++++ .../durable-capture-recovery-authority.ts | 21 +++--- 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/src/daemon/__tests__/durable-capture-recovery-authority.test.ts b/src/daemon/__tests__/durable-capture-recovery-authority.test.ts index 6b8a9edb0c..c5725df953 100644 --- a/src/daemon/__tests__/durable-capture-recovery-authority.test.ts +++ b/src/daemon/__tests__/durable-capture-recovery-authority.test.ts @@ -173,6 +173,78 @@ test.each(['active', 'missing'] as const)( }, ); +test('late handle cleanup failure does not skip control disposal when diagnostics throw', async () => { + vi.useFakeTimers(); + const combinedSignal = vi.spyOn(AbortSignal, 'any').mockReturnValue(new AbortController().signal); + try { + const cleanupError = new Error('late handle cleanup failed'); + const reporterError = new Error('late cleanup diagnostic reporter failed'); + const cleanupFailures = vi.fn(() => { + throw reporterError; + }); + const disposeControl = vi.fn(async () => {}); + let resolveReattach!: (outcome: { status: 'active'; handle: AppLogLiveHandle }) => void; + let signalReattachStarted!: () => void; + const reattachStarted = new Promise((resolve) => { + signalReattachStarted = resolve; + }); + const acquisition = acquireDurableCaptureRecoveryAuthorityBeforeDeadline({ + displayName: 'app-log', + envelope, + scope: { + signal: new AbortController().signal, + diagnostics: { emit: () => {} }, + progress: { report: () => {} }, + }, + deadlineMs: 25, + acquireControl: async () => ({ + reattach: async () => { + signalReattachStarted(); + return await new Promise<{ status: 'active'; handle: AppLogLiveHandle }>((resolve) => { + resolveReattach = resolve; + }); + }, + cleanup: async () => ({ status: 'already-missing' as const }), + [Symbol.asyncDispose]: disposeControl, + }), + onLateCleanupFailure: cleanupFailures, + }); + const primaryErrorPromise = acquisition.catch((error: unknown) => error); + await reattachStarted; + await vi.advanceTimersByTimeAsync(25); + const primaryError = await primaryErrorPromise; + + const forceCleanup = vi.fn(async () => { + throw cleanupError; + }); + resolveReattach({ + status: 'active', + handle: createTestAppLogLiveHandle({ + inspect: () => ({ backend: 'android', state: 'active', startedAt: 1 }), + finish: async () => ({ + status: 'completed', + alreadyCompleted: true, + result: { backend: 'android', outputPath: '/tmp/app.log', completedAt: 1 }, + }), + forceCleanup, + }), + }); + await vi.advanceTimersByTimeAsync(0); + + expect(primaryError).toBeInstanceOf(DurableCaptureRecoveryDeadlineError); + expect(forceCleanup).toHaveBeenCalledOnce(); + expect(cleanupFailures).toHaveBeenCalledWith( + 'late_handle_cleanup_failed', + cleanupError, + primaryError, + ); + expect(disposeControl).toHaveBeenCalledOnce(); + } finally { + combinedSignal.mockRestore(); + vi.useRealTimers(); + } +}); + test.each(['deadline', 'cancellation'] as const)( 'late control cleanup failure remains secondary to the %s error', async (winner) => { diff --git a/src/daemon/durable-capture-recovery-authority.ts b/src/daemon/durable-capture-recovery-authority.ts index 4591da3c4f..f505fd6d32 100644 --- a/src/daemon/durable-capture-recovery-authority.ts +++ b/src/daemon/durable-capture-recovery-authority.ts @@ -131,20 +131,23 @@ async function disposeLateRecoveryAuthority, primaryError: unknown, ): Promise { - if (authority.reattached.status === 'active') { + try { + if (authority.reattached.status === 'active') { + await disposeLateAuthority( + params, + authority.reattached.handle, + 'late_handle_cleanup_failed', + primaryError, + ); + } + } finally { await disposeLateAuthority( params, - authority.reattached.handle, - 'late_handle_cleanup_failed', + authority.control, + 'late_control_cleanup_failed', primaryError, ); } - await disposeLateAuthority( - params, - authority.control, - 'late_control_cleanup_failed', - primaryError, - ); } async function acquireRecoveryAuthority, C>( From 10c6e966aae375682dcc1a742bf1b46dd25cb3cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Pierzcha=C5=82a?= Date: Thu, 27 Aug 2026 21:31:09 +0200 Subject: [PATCH 3/3] fix: preserve aborted recovery cleanup --- ...durable-capture-recovery-authority.test.ts | 76 +++++++++++++++++++ .../durable-capture-recovery-authority.ts | 13 ++-- 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/daemon/__tests__/durable-capture-recovery-authority.test.ts b/src/daemon/__tests__/durable-capture-recovery-authority.test.ts index c5725df953..cd9c970f4d 100644 --- a/src/daemon/__tests__/durable-capture-recovery-authority.test.ts +++ b/src/daemon/__tests__/durable-capture-recovery-authority.test.ts @@ -245,6 +245,82 @@ test('late handle cleanup failure does not skip control disposal when diagnostic } }); +test('aborted recovery does not skip control disposal when late handle diagnostics throw', async () => { + vi.useFakeTimers(); + try { + const controller = new AbortController(); + const cancellation = new Error('request canceled'); + const cleanupError = new Error('late handle cleanup failed'); + const reporterError = new Error('late cleanup diagnostic reporter failed'); + const cleanupFailures = vi.fn(() => { + throw reporterError; + }); + const disposeControl = vi.fn(async () => {}); + let observedSignal: AbortSignal | undefined; + let resolveReattach!: (outcome: { status: 'active'; handle: AppLogLiveHandle }) => void; + let signalReattachStarted!: () => void; + const reattachStarted = new Promise((resolve) => { + signalReattachStarted = resolve; + }); + const acquisition = acquireDurableCaptureRecoveryAuthorityBeforeDeadline({ + displayName: 'app-log', + envelope, + scope: { + signal: controller.signal, + diagnostics: { emit: () => {} }, + progress: { report: () => {} }, + }, + deadlineMs: 10_000, + acquireControl: async (_candidate, scope) => { + observedSignal = scope.signal; + return { + reattach: async () => { + signalReattachStarted(); + return await new Promise<{ status: 'active'; handle: AppLogLiveHandle }>((resolve) => { + resolveReattach = resolve; + }); + }, + cleanup: async () => ({ status: 'already-missing' as const }), + [Symbol.asyncDispose]: disposeControl, + }; + }, + onLateCleanupFailure: cleanupFailures, + }); + const primaryErrorPromise = acquisition.catch((error: unknown) => error); + await reattachStarted; + controller.abort(cancellation); + const primaryError = await primaryErrorPromise; + + expect(observedSignal?.aborted).toBe(true); + expect(observedSignal?.reason).toBe(cancellation); + resolveReattach({ + status: 'active', + handle: createTestAppLogLiveHandle({ + inspect: () => ({ backend: 'android', state: 'active', startedAt: 1 }), + finish: async () => ({ + status: 'completed', + alreadyCompleted: true, + result: { backend: 'android', outputPath: '/tmp/app.log', completedAt: 1 }, + }), + forceCleanup: vi.fn(async () => { + throw cleanupError; + }), + }), + }); + await vi.advanceTimersByTimeAsync(0); + + expect(primaryError).toBe(cancellation); + expect(cleanupFailures).toHaveBeenCalledWith( + 'late_handle_cleanup_failed', + cleanupError, + cancellation, + ); + expect(disposeControl).toHaveBeenCalledOnce(); + } finally { + vi.useRealTimers(); + } +}); + test.each(['deadline', 'cancellation'] as const)( 'late control cleanup failure remains secondary to the %s error', async (winner) => { diff --git a/src/daemon/durable-capture-recovery-authority.ts b/src/daemon/durable-capture-recovery-authority.ts index f505fd6d32..5e5f2bfea2 100644 --- a/src/daemon/durable-capture-recovery-authority.ts +++ b/src/daemon/durable-capture-recovery-authority.ts @@ -163,11 +163,14 @@ async function acquireRecoveryAuthority