From 5d5e56817da78181c0c9e2864c45319fe8d1ca5e Mon Sep 17 00:00:00 2001 From: Quine <219950802+Quine-rq@users.noreply.github.com> Date: Fri, 18 Sep 2026 13:59:03 +0800 Subject: [PATCH] fix(chat): preserve approval replies across early turn completion --- lat.md/approval-completion.md | 15 +++ lat.md/chat-commands.md | 2 + lat.md/lat.md | 2 + .../hooks/useDashboardChatTransport.test.tsx | 120 ++++++++++++++++++ .../Chat/hooks/useDashboardChatTransport.ts | 38 ++++-- 5 files changed, 169 insertions(+), 8 deletions(-) create mode 100644 lat.md/approval-completion.md diff --git a/lat.md/approval-completion.md b/lat.md/approval-completion.md new file mode 100644 index 000000000..439e58d9c --- /dev/null +++ b/lat.md/approval-completion.md @@ -0,0 +1,15 @@ +--- +lat: + require-code-mention: true +--- +# Approval completion ordering + +A dashboard turn can complete before its approval response RPC is acknowledged. [[src/renderer/src/screens/Chat/hooks/useDashboardChatTransport.ts#useDashboardChatTransport]] keeps an in-flight decision addressable until the reply settles. + +## Confirmed decisions + +Allow and deny responses succeed after early completion when the gateway resolves exactly one request. Unanswered queued approvals still expire; duplicate submission remains blocked while the response is pending. + +## Failed and obsolete decisions + +A lost or unresolved acknowledgement cannot revive a completed approval. Abort, disconnect, connection changes and a new prompt invalidate retained requests; a late reply cannot stop a new turn on the same runtime session. diff --git a/lat.md/chat-commands.md b/lat.md/chat-commands.md index 693bcc4c3..7dcd08017 100644 --- a/lat.md/chat-commands.md +++ b/lat.md/chat-commands.md @@ -104,6 +104,8 @@ The legacy approval responses `/approve` and `/deny` (the `RENDERER_NATIVE_SLASH Dangerous commands pause the current turn until the user explicitly allows or denies them; the desktop never auto-approves or replays a prompt after an approval request. +Response acknowledgements may follow turn completion; lifecycle regression coverage is documented in [[approval-completion]]. + [[src/shared/chat-approval.ts#normalizeApprovalRequest]] limits choices to the gateway's offered permissions and preserves a deny path. Dashboard chat renders [[src/renderer/src/screens/Chat/ApprovalCard.tsx#ApprovalCard]] from `approval.request`. Responses include the gateway-issued `request_id` and runtime session ID through [[src/renderer/src/screens/Chat/hooks/useDashboardChatTransport.ts#useDashboardChatTransport]]. Cards queue in arrival order, with an in-flight guard. Only an acknowledgment resolving exactly one request succeeds. Network failures can retry the same ID; a missing ID or unresolved acknowledgment invalidates the cards and interrupts the turn. Gateway-only WebSocket chat registers opaque renderer IDs through [[src/main/hermes.ts#registerPendingApproval]], bound to the originating renderer and run by [[src/main/ipc/register.ts#registerIpcHandlers]]. These IDs map to the upstream request IDs; they never substitute for them. Completion, cancellation, renderer destruction, or connection loss clears pending requests. Run cleanup retains the originating connection key; cancelled IPC calls settle, and runs that finish before their handle arrives are not registered as active. A transport failure after any approval request cannot replay the prompt. Callers without an approval UI fail closed. `Always allow` requires a second confirmation because the gateway persists that permission. diff --git a/lat.md/lat.md b/lat.md/lat.md index 69dea45ce..64a899920 100644 --- a/lat.md/lat.md +++ b/lat.md/lat.md @@ -37,3 +37,5 @@ This directory defines the high-level concepts, business logic, and architecture - [[scheduled-jobs]] — schedule state normalization across local files, remote API responses, and named SSH profiles. - [[dashboard-clarify]] — Interactive WebSocket clarification cards and answer delivery tests. + +- [[approval-completion]] — In-flight approval replies remain valid across early turn completion, with failure and lifecycle isolation. diff --git a/src/renderer/src/screens/Chat/hooks/useDashboardChatTransport.test.tsx b/src/renderer/src/screens/Chat/hooks/useDashboardChatTransport.test.tsx index aed45d302..6a834d757 100644 --- a/src/renderer/src/screens/Chat/hooks/useDashboardChatTransport.test.tsx +++ b/src/renderer/src/screens/Chat/hooks/useDashboardChatTransport.test.tsx @@ -1030,6 +1030,126 @@ describe("useDashboardChatTransport approvals", () => { ).toBe(false); }); + // @lat: [[approval-completion#Confirmed decisions]] + it.each(["once", "deny"] as const)( + "accepts a confirmed %s decision when completion arrives before its acknowledgement", + async (choice) => { + const api: HarnessApi = {}; + render(); + await act(async () => { + await api.send?.("hello"); + dashboardMock.onEvent?.({ + type: "approval.request", + session_id: "live-1", + payload: { request_id: "in-flight", choices: ["once", "deny"] }, + }); + dashboardMock.onEvent?.({ + type: "approval.request", + session_id: "live-1", + payload: { request_id: "unanswered", choices: ["once"] }, + }); + }); + let acknowledge!: (value: { resolved: number }) => void; + dashboardMock.request.mockImplementationOnce( + () => + new Promise((resolve) => { + acknowledge = resolve; + }), + ); + let response!: Promise; + act(() => { + response = api.respondApproval!("in-flight", choice); + }); + await act(async () => { + dashboardMock.onEvent?.({ + type: "message.complete", + session_id: "live-1", + payload: { text: "Done" }, + }); + }); + expect( + api.messages?.find( + (m) => m.kind === "approval" && m.requestId === "in-flight", + ), + ).not.toMatchObject({ unavailable: true }); + expect( + api.messages?.find( + (m) => m.kind === "approval" && m.requestId === "unanswered", + ), + ).toMatchObject({ unavailable: true }); + await act(async () => { + acknowledge({ resolved: 1 }); + expect(await response).toBe(true); + }); + expect(await api.respondApproval!("unanswered", "once")).toBe(false); + expect(await api.respondApproval!("in-flight", choice)).toBe(false); + }, + ); + + // @lat: [[approval-completion#Failed and obsolete decisions]] + it.each([ + "reject", + "unresolved", + "abort", + "disconnect", + "connection", + "new turn", + ])("does not revive completed approvals after %s", async (outcome) => { + const api: HarnessApi = {}; + const view = render(); + await act(async () => { + await api.send?.("hello"); + dashboardMock.onEvent?.({ + type: "approval.request", + session_id: "live-1", + payload: { request_id: "late", choices: ["once"] }, + }); + }); + let acknowledge!: (value: { resolved: number }) => void; + let reject!: (reason: Error) => void; + dashboardMock.request.mockImplementationOnce( + () => + new Promise((resolve, fail) => { + acknowledge = resolve; + reject = fail; + }), + ); + let response!: Promise; + act(() => { + response = api.respondApproval!("late", "once"); + }); + await act(async () => { + dashboardMock.onEvent?.({ + type: "message.complete", + session_id: "live-1", + payload: { text: "Done" }, + }); + }); + await act(async () => { + if (outcome === "abort") api.abort?.(); + if (outcome === "disconnect") dashboardMock.onClose?.(); + if (outcome === "new turn") await api.send?.("next turn"); + }); + if (outcome === "connection") + view.rerender(); + const loadingBeforeAck = api.isLoading; + await act(async () => { + if (outcome === "reject") reject(new Error("acknowledgement lost")); + else + acknowledge({ + resolved: outcome === "unresolved" || outcome === "new turn" ? 0 : 1, + }); + expect(await response).toBe(false); + }); + expect( + api.messages?.find( + (m) => m.kind === "approval" && m.requestId === "late", + ), + ).toMatchObject({ unavailable: true }); + expect(await api.respondApproval!("late", "once")).toBe(false); + if (outcome === "new turn") expect(api.isLoading).toBe(loadingBeforeAck); + }); + it("clears pending approval on completion and abort", async () => { const api: HarnessApi = {}; render(); diff --git a/src/renderer/src/screens/Chat/hooks/useDashboardChatTransport.ts b/src/renderer/src/screens/Chat/hooks/useDashboardChatTransport.ts index 4f58a6dbd..cd29c6760 100644 --- a/src/renderer/src/screens/Chat/hooks/useDashboardChatTransport.ts +++ b/src/renderer/src/screens/Chat/hooks/useDashboardChatTransport.ts @@ -147,6 +147,7 @@ interface PendingDashboardApproval { gatewayRequestId: string | null; requestId: string; responding: boolean; + completed?: boolean; sessionId: string; } @@ -1049,15 +1050,27 @@ export function useDashboardChatTransport({ ); }; - const expirePendingApprovalsRef = useRef<(failActiveTurn?: boolean) => void>( - () => undefined, - ); - expirePendingApprovalsRef.current = (failActiveTurn = false): void => { + const expirePendingApprovalsRef = useRef< + (failActiveTurn?: boolean, preserveResponding?: boolean) => void + >(() => undefined); + expirePendingApprovalsRef.current = ( + failActiveTurn = false, + preserveResponding = false, + ): void => { if (pendingApprovalsRef.current.length === 0) return; - const pendingIds = new Set( - pendingApprovalsRef.current.map(({ requestId }) => requestId), + const pendingIds = new Set(); + pendingApprovalsRef.current = pendingApprovalsRef.current.filter( + (pending) => { + // Turn completion can precede the response RPC acknowledgement. Keep + // only decisions already in flight; unanswered requests still expire. + if (preserveResponding && pending.responding) { + pending.completed = true; + return true; + } + pendingIds.add(pending.requestId); + return false; + }, ); - pendingApprovalsRef.current = []; setMessages((current) => { const unavailable = current.map((message) => message.kind === "approval" && @@ -1263,7 +1276,7 @@ export function useDashboardChatTransport({ // A reply RPC can be acknowledged after the resumed turn finishes. if (!pendingClarifyRef.current?.responding) expirePendingClarifyRef.current(); - expirePendingApprovalsRef.current(); + expirePendingApprovalsRef.current(false, true); if (failed) { appliedModelRef.current = null; recreateRuntimeSessionRef.current = true; @@ -1803,6 +1816,11 @@ export function useDashboardChatTransport({ return true; } } + // A late reply from the completed turn must not affect a new prompt + // using the same runtime session, especially on an unresolved ACK. + if (pendingApprovalsRef.current.some((pending) => pending.completed)) { + expirePendingApprovalsRef.current(); + } const dashboardText = dashboardPromptTextForAttachments( text, attachments, @@ -2025,6 +2043,10 @@ export function useDashboardChatTransport({ pendingApprovalsRef.current = pendingApprovalsRef.current.slice(1); return true; } catch { + if (pending.completed && pendingApprovalsRef.current[0] === pending) { + // A completed turn cannot accept a retry after an unconfirmed reply. + expirePendingApprovalsRef.current(); + } return false; } finally { if (pendingApprovalsRef.current[0] === pending) {