From aaa04507e021c2a2582fa52d822dfc19e45129d0 Mon Sep 17 00:00:00 2001 From: murashit Date: Mon, 20 Jul 2026 17:41:03 +0900 Subject: [PATCH] refactor(chat): make reconnect reset one scoped transition --- .../connection/connection-manager.ts | 4 -- .../connection/reconnect-actions.ts | 42 ++++++----- src/features/chat/host/session-runtime.ts | 15 ++-- .../connection/reconnect-actions.test.ts | 72 ++++++++++++++----- 4 files changed, 78 insertions(+), 55 deletions(-) diff --git a/src/app-server/connection/connection-manager.ts b/src/app-server/connection/connection-manager.ts index 2f0143ee..fc40b2e5 100644 --- a/src/app-server/connection/connection-manager.ts +++ b/src/app-server/connection/connection-manager.ts @@ -105,10 +105,6 @@ export class ConnectionManager { return promise; } - resetConnection(): void { - this.disconnect(); - } - disconnect(): void { const previous = this.state; this.state = { kind: "disconnected", generation: previous.generation + 1 }; diff --git a/src/features/chat/application/connection/reconnect-actions.ts b/src/features/chat/application/connection/reconnect-actions.ts index 3c49edc3..ff154667 100644 --- a/src/features/chat/application/connection/reconnect-actions.ts +++ b/src/features/chat/application/connection/reconnect-actions.ts @@ -6,10 +6,7 @@ const STATUS_RECONNECTING = "Reconnecting..."; export interface ChatReconnectActionsHost { stateStore: ChatStateStore; - invalidateConnectionWork: () => void; - invalidateThreadWork: () => void; - clearDeferredDiagnostics: () => void; - resetConnection: () => void; + resetConnectionScope: () => void; setStatus: (statusText: string, phase?: ChatConnectionPhase) => void; ensureConnected: () => Promise; isConnected: () => boolean; @@ -17,34 +14,35 @@ export interface ChatReconnectActionsHost { addSystemMessage: (text: string) => void; } -export async function reconnectPanel( - host: ChatReconnectActionsHost, - target: { resumeThreadId: string | null; isCurrent?: () => boolean } | null = null, -): Promise { +export function createReconnectPanelAction(host: ChatReconnectActionsHost): () => Promise { + let activeReconnect: Promise | null = null; + return async () => { + if (activeReconnect) return activeReconnect; + const operation = reconnectPanel(host); + activeReconnect = operation; + try { + return await operation; + } finally { + if (activeReconnect === operation) activeReconnect = null; + } + }; +} + +async function reconnectPanel(host: ChatReconnectActionsHost): Promise { const currentState = host.stateStore.getState(); const panelTarget = capturePanelTargetLease(currentState); - const threadId = target - ? target.resumeThreadId - : activeThreadState(currentState)?.lifetime?.kind === "ephemeral" - ? null - : panelThreadId(currentState); - const isCurrent = target?.isCurrent ?? (() => true); + const threadId = activeThreadState(currentState)?.lifetime?.kind === "ephemeral" ? null : panelThreadId(currentState); host.stateStore.dispatch({ type: "ui/panel-set", panel: null }); - host.invalidateConnectionWork(); - host.invalidateThreadWork(); - host.clearDeferredDiagnostics(); - host.resetConnection(); + host.resetConnectionScope(); host.stateStore.dispatch({ type: "connection/scoped-cleared" }); host.setStatus(STATUS_RECONNECTING, { kind: "connecting" }); await host.ensureConnected(); - if (!isCurrent() || !host.isConnected() || !panelTargetLeaseIsCurrent(host.stateStore.getState(), panelTarget)) return false; + if (!host.isConnected() || !panelTargetLeaseIsCurrent(host.stateStore.getState(), panelTarget)) return false; if (!threadId) return true; try { if (!(await host.resumeThread(threadId))) return false; - if (!isCurrent() || !panelTargetLeaseIsCurrent(host.stateStore.getState(), panelTarget)) { - return false; - } + if (!panelTargetLeaseIsCurrent(host.stateStore.getState(), panelTarget)) return false; return activeThreadId(host.stateStore.getState()) === threadId; } catch (error) { host.addSystemMessage(error instanceof Error ? error.message : String(error)); diff --git a/src/features/chat/host/session-runtime.ts b/src/features/chat/host/session-runtime.ts index 6544a45a..64c9c626 100644 --- a/src/features/chat/host/session-runtime.ts +++ b/src/features/chat/host/session-runtime.ts @@ -2,7 +2,7 @@ import { codexPanelAppServerInitializeParams } from "../../../app-server/connect import { ConnectionManager } from "../../../app-server/connection/connection-manager"; import { isStaleAppServerResourceContextError } from "../../../app-server/query/resource-store"; import { createChatAppServerGateway, createChatCurrentAppServerGateway } from "../app-server/session-gateway"; -import { reconnectPanel } from "../application/connection/reconnect-actions"; +import { createReconnectPanelAction } from "../application/connection/reconnect-actions"; import { createLocalIdSource, type LocalIdSource } from "../application/local-id-source"; import { runtimeSnapshotForChatState } from "../application/runtime/snapshot"; import type { ChatAction, ChatConnectionPhase } from "../application/state/root-reducer"; @@ -224,18 +224,12 @@ function composeChatPanelSessionRuntime(host: ChatPanelSessionRuntimeHost): Chat }); const reconnectHost = { stateStore, - invalidateConnectionWork: () => { + resetConnectionScope: () => { connectionActions.invalidate(); - }, - invalidateThreadWork: () => { invalidateThreadWork(); - }, - clearDeferredDiagnostics: () => { host.deferredTasks.clearDiagnostics(); - }, - resetConnection: () => { connectionBundle.invalidateConnectionScope(); - connection.resetConnection(); + connection.disconnect(); }, setStatus: (statusText: string, phase?: ChatConnectionPhase) => { status.set(statusText, phase); @@ -247,8 +241,9 @@ function composeChatPanelSessionRuntime(host: ChatPanelSessionRuntimeHost): Chat status.addSystemMessage(text); }, }; + const reconnectPanel = createReconnectPanelAction(reconnectHost); const reconnect = async () => { - await reconnectPanel(reconnectHost); + await reconnectPanel(); }; const reconnectForUser = host.reconnect ?? reconnect; const turn = createTurnBundle(host, { diff --git a/tests/features/chat/application/connection/reconnect-actions.test.ts b/tests/features/chat/application/connection/reconnect-actions.test.ts index 191e3444..10f83611 100644 --- a/tests/features/chat/application/connection/reconnect-actions.test.ts +++ b/tests/features/chat/application/connection/reconnect-actions.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it, vi } from "vitest"; -import { type ChatReconnectActionsHost, reconnectPanel } from "../../../../../src/features/chat/application/connection/reconnect-actions"; +import { + type ChatReconnectActionsHost, + createReconnectPanelAction, +} from "../../../../../src/features/chat/application/connection/reconnect-actions"; import { activeThreadId, createChatState } from "../../../../../src/features/chat/application/state/root-reducer"; import { createChatStateStore } from "../../../../../src/features/chat/application/state/store"; @@ -36,10 +39,7 @@ function createHost(overrides: Partial = {}) { stateStore.dispatch({ type: "thread-list/applied", threads: [{ id: "thread" } as never] }); const host: ChatReconnectActionsHost = { stateStore, - invalidateConnectionWork: vi.fn(), - invalidateThreadWork: vi.fn(), - clearDeferredDiagnostics: vi.fn(), - resetConnection: vi.fn(), + resetConnectionScope: vi.fn(), setStatus: vi.fn(), ensureConnected: vi.fn().mockResolvedValue(undefined), isConnected: vi.fn(() => true), @@ -63,20 +63,17 @@ function createHost(overrides: Partial = {}) { addSystemMessage: vi.fn(), ...overrides, }; - return { host, stateStore }; + return { host, stateStore, reconnect: createReconnectPanelAction(host) }; } -describe("reconnectPanel", () => { +describe("createReconnectPanelAction", () => { it("resets local connection work before reconnecting and retains shared thread projections", async () => { - const { host, stateStore } = createHost(); + const { host, stateStore, reconnect } = createHost(); - await expect(reconnectPanel(host)).resolves.toBe(true); + await expect(reconnect()).resolves.toBe(true); expect(stateStore.getState().ui.toolbarPanel).toBeNull(); - expect(host.invalidateConnectionWork).toHaveBeenCalledOnce(); - expect(host.invalidateThreadWork).toHaveBeenCalledOnce(); - expect(host.clearDeferredDiagnostics).toHaveBeenCalledOnce(); - expect(host.resetConnection).toHaveBeenCalledOnce(); + expect(host.resetConnectionScope).toHaveBeenCalledOnce(); expect(host.setStatus).toHaveBeenCalledWith("Reconnecting...", { kind: "connecting" }); expect(stateStore.getState().requests.pendingUserInputs).toEqual([]); expect(stateStore.getState().threadList).toEqual({ @@ -124,7 +121,7 @@ describe("reconnectPanel", () => { }, }, ] as const)("resumes the same $label thread after an unexpected exit", async ({ thread }) => { - const { host, stateStore } = createHost(); + const { host, stateStore, reconnect } = createHost(); stateStore.dispatch({ type: "active-thread/resumed", approvalPolicyKnown: true, @@ -148,14 +145,14 @@ describe("reconnectPanel", () => { provenance: thread.provenance, }); - await expect(reconnectPanel(host)).resolves.toBe(true); + await expect(reconnect()).resolves.toBe(true); expect(host.resumeThread).toHaveBeenCalledWith(thread.id); expect(activeThreadId(stateStore.getState())).toBe(thread.id); }); it("does not resume an ephemeral thread after an unexpected exit", async () => { - const { host, stateStore } = createHost(); + const { host, stateStore, reconnect } = createHost(); stateStore.dispatch({ type: "active-thread/resumed", approvalPolicyKnown: true, @@ -174,18 +171,55 @@ describe("reconnectPanel", () => { stateStore.dispatch({ type: "connection/scoped-cleared" }); expect(stateStore.getState().panelThread).toEqual({ kind: "empty" }); - await reconnectPanel(host); + await reconnect(); expect(host.resumeThread).not.toHaveBeenCalled(); }); it("reports resume failures after reconnecting", async () => { - const { host } = createHost({ + const { host, reconnect } = createHost({ resumeThread: vi.fn().mockRejectedValue(new Error("resume failed")), }); - await reconnectPanel(host); + await reconnect(); expect(host.addSystemMessage).toHaveBeenCalledWith("resume failed"); }); + + it("coalesces overlapping manual reconnect requests", async () => { + let finishConnecting: () => void = () => undefined; + const connecting = new Promise((resolve) => { + finishConnecting = resolve; + }); + const { host, reconnect } = createHost({ + ensureConnected: vi.fn(() => connecting), + }); + + const first = reconnect(); + const second = reconnect(); + + expect(host.resetConnectionScope).toHaveBeenCalledOnce(); + expect(host.ensureConnected).toHaveBeenCalledOnce(); + + finishConnecting(); + await expect(Promise.all([first, second])).resolves.toEqual([true, true]); + expect(host.resumeThread).toHaveBeenCalledOnce(); + }); + + it("does not resume the old target after navigation while reconnecting", async () => { + let finishConnecting: () => void = () => undefined; + const connecting = new Promise((resolve) => { + finishConnecting = resolve; + }); + const { host, stateStore, reconnect } = createHost({ + ensureConnected: vi.fn(() => connecting), + }); + + const operation = reconnect(); + stateStore.dispatch({ type: "active-thread/cleared" }); + finishConnecting(); + + await expect(operation).resolves.toBe(false); + expect(host.resumeThread).not.toHaveBeenCalled(); + }); });