From a5ed24f7eeb50abd35375c6b1864bb424fe699b3 Mon Sep 17 00:00:00 2001 From: Dotta Date: Sun, 11 Oct 2026 01:45:01 -0500 Subject: [PATCH] Avoid duplicate diagnostics for immediate native session close failure --- .../native-session-executor.test.ts | 20 +++++++++++++++++++ .../native-runtime/native-session-executor.ts | 6 +++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index 1c3affd002..cc80747010 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -14371,6 +14371,7 @@ describe("warm native retirement diagnostics", () => { closeState: ["superseded", "retirement_failed", "not_started"].includes(mode) ? "not_started" : mode === "collection_failed" ? "completed" : mode, failureCode: mode === "superseded" ? "computer_owner_retirement_superseded" : ["not_started", "pending"].includes(mode) ? "native_session_cleanup_quarantined" : "native_session_interrupted", }); + expect(diagnostic).toHaveBeenCalledTimes(1); if (mode !== "collection_failed") expect(collect).not.toHaveBeenCalled(); if (["not_started", "superseded", "retirement_failed"].includes(mode)) expect(close).not.toHaveBeenCalled(); if (mode === "pending") { @@ -14379,6 +14380,7 @@ describe("warm native retirement diagnostics", () => { rejectClose!(failure); await expect(lateClose).rejects.toBe(failure); expect(successorObserver).not.toHaveBeenCalled(); + expect(diagnostic).toHaveBeenCalledTimes(2); expect(diagnostic).toHaveBeenLastCalledWith({ phase: "session_close", closeState: "failed", failureCode: "native_session_interrupted" }); } expect(JSON.stringify(diagnostic.mock.calls)).not.toContain(secret); @@ -14391,11 +14393,29 @@ describe("warm native retirement diagnostics", () => { const entry = { session: { close: async () => { throw failure; } }, onRetirementDiagnostic: diagnostic } as unknown as Parameters[0]; await expect(closeWarmNativeSession(entry, "test")).rejects.toBe(failure); + expect(diagnostic).toHaveBeenCalledTimes(1); expect(diagnostic).toHaveBeenLastCalledWith({ phase: "session_close", closeState: "failed", failureCode: "native_session_close_unrecoverable" }); expect(JSON.stringify(diagnostic.mock.calls)).not.toContain("CANARY"); expect(JSON.stringify(diagnostic.mock.calls)).not.toContain("privatePath"); }); + it.each(["controller_detach", "instruction_receipt", "access_cleanup"] as const)("preserves a distinct failure after close rejection (%s)", async phase => { + const failure = new Error("PRIVATE-CANARY"); + const diagnostic = vi.fn(); + const entry = { session: { close: async () => { throw failure; }, + detachControllerForRestart: async () => { if (phase === "controller_detach") throw failure; } }, + computerTarget: { retire: async ({ beforeStop }: { beforeStop: () => Promise }) => { await beforeStop().catch(() => undefined); return true; } }, + instructionWorkingCopy: { retirementFailed: async () => { if (phase === "instruction_receipt") throw failure; } }, + githubAccess: { stop: async () => { if (phase === "access_cleanup") throw failure; } }, + onRetirementDiagnostic: diagnostic } as unknown as Parameters[0]; + if (phase === "instruction_receipt") await expect(closeWarmNativeSession(entry, "test")).rejects.toBeInstanceOf(AggregateError); + else await expect(closeWarmNativeSession(entry, "test")).rejects.toBe(failure); + expect(diagnostic).toHaveBeenCalledTimes(2); + expect(diagnostic.mock.calls[0]?.[0]).toMatchObject({ phase: "session_close", closeState: "failed" }); + expect(diagnostic.mock.calls[1]?.[0]).toMatchObject({ phase: phase === "instruction_receipt" ? "session_close" : phase, closeState: "failed" }); + expect(JSON.stringify(diagnostic.mock.calls)).not.toContain("PRIVATE-CANARY"); + }); + it("does not let an observer failure change a failed close", async () => { const failure = new Error("original close failure"); const entry = { session: { close: async () => { throw failure; } }, diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index 79ac8f8a3a..a7ba645a48 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -519,6 +519,8 @@ export async function closeWarmNativeSession(entry: WarmNativeSession, reason: s ? "native_session_close_unrecoverable" : nativeSessionFailureSourceCode(error) }); } catch { /* Diagnostics never own retirement or alter its result. */ } }; + const isReportedCloseFailure = (error: unknown) => + phase === "session_close" && closeState === "failed" && error === closeFailure; const close = async () => { closeState = "pending"; try { await entry.session.close({ reason }); closeCompleted = true; closeState = "completed"; } @@ -562,7 +564,9 @@ export async function closeWarmNativeSession(entry: WarmNativeSession, reason: s catch (error) { phase = "access_cleanup"; throw error; } } } catch (error) { - report(error); + // The close callback already reports its own rejection, including one + // arriving after the retirement deadline. Preserve distinct cleanup failures. + if (!isReportedCloseFailure(error)) report(error); throw error; } }