From 6932cd6980e6b0d5b4d1df6ab60b5fc514b30422 Mon Sep 17 00:00:00 2001 From: Shakker Date: Tue, 11 Aug 2026 22:27:01 +0200 Subject: [PATCH] fix: reconcile companion progress ownership --- src/gateway/session-companion-ask.ts | 11 ++----- src/gateway/session-companion-progress.ts | 12 +++++++- src/gateway/session-companion-rpc.test.ts | 34 ++++++++++++++++++++- ui/src/pages/chat/chat-session-companion.ts | 7 +++++ ui/src/pages/chat/chat-session-rail.test.ts | 19 ++++++++++++ 5 files changed, 73 insertions(+), 10 deletions(-) diff --git a/src/gateway/session-companion-ask.ts b/src/gateway/session-companion-ask.ts index 34f96b2d8f0b..310c7944cf10 100644 --- a/src/gateway/session-companion-ask.ts +++ b/src/gateway/session-companion-ask.ts @@ -460,14 +460,9 @@ export function createSessionCompanionAskRuntime(params: SessionCompanionAskRunt } const thread: SessionCompanionThread = { exchanges: [], - seed: { - messages: result.context.messages.flatMap((message): SessionCompanionSeedMessage[] => - message.role === "summary" - ? [] - : [{ role: message.role, text: message.text, ts: message.ts }], - ), - digestJson: "null", - }, + // Public type compatibility only; authoritative prepared context stays + // in the private WeakMap so one thread never retains duplicate payloads. + seed: { messages: [], digestJson: "null" }, lastNoteSequence: 0, busy: false, lastUsedAt: params.now(), diff --git a/src/gateway/session-companion-progress.ts b/src/gateway/session-companion-progress.ts index 7813a393d192..5014b22f9f8b 100644 --- a/src/gateway/session-companion-progress.ts +++ b/src/gateway/session-companion-progress.ts @@ -12,6 +12,11 @@ export function registerSessionCompanionProgress(params: { listener: SessionCompanionProgressListener; }): () => void { const key = progressKey(params.connId, params.sessionKey); + // A duplicate busy ask must not steal the accepted phase from the request + // that already owns this connection/session slot. + if (listeners.has(key)) { + return () => {}; + } listeners.set(key, params.listener); return () => { if (listeners.get(key) === params.listener) { @@ -25,5 +30,10 @@ export function notifySessionCompanionPrepared(params: { empty: boolean; sessionKey: string; }): void { - listeners.get(progressKey(params.connId, params.sessionKey))?.({ empty: params.empty }); + try { + listeners.get(progressKey(params.connId, params.sessionKey))?.({ empty: params.empty }); + } catch { + // Progress presentation is advisory; a callback failure cannot abort the + // authoritative companion request after context is ready. + } } diff --git a/src/gateway/session-companion-rpc.test.ts b/src/gateway/session-companion-rpc.test.ts index 90de91d2bb9f..5b25337be8e5 100644 --- a/src/gateway/session-companion-rpc.test.ts +++ b/src/gateway/session-companion-rpc.test.ts @@ -3,7 +3,10 @@ import { GatewayErrorDetailCodes } from "../../packages/gateway-protocol/src/ind import { CONTROL_UI_SESSION_COMPANION_PROGRESS_CAP } from "../shared/control-ui-capabilities.js"; import { SessionCompanionAskError } from "./session-companion-ask.js"; import { attachSessionCompanionErrorDetail } from "./session-companion-error-detail.js"; -import { notifySessionCompanionPrepared } from "./session-companion-progress.js"; +import { + notifySessionCompanionPrepared, + registerSessionCompanionProgress, +} from "./session-companion-progress.js"; import { sessionCompanionHandlers } from "./session-companion-rpc.js"; async function invoke( @@ -30,6 +33,35 @@ async function invoke( } describe("session companion RPC", () => { + it("keeps the first progress owner and isolates callback failures", () => { + const first = vi.fn(() => { + throw new Error("presentation failed"); + }); + const second = vi.fn(); + const clearFirst = registerSessionCompanionProgress({ + connId: "conn-1", + sessionKey: "agent:main:main", + listener: first, + }); + const clearSecond = registerSessionCompanionProgress({ + connId: "conn-1", + sessionKey: "agent:main:main", + listener: second, + }); + + expect(() => + notifySessionCompanionPrepared({ + connId: "conn-1", + empty: false, + sessionKey: "agent:main:main", + }), + ).not.toThrow(); + expect(first).toHaveBeenCalledOnce(); + expect(second).not.toHaveBeenCalled(); + clearSecond(); + clearFirst(); + }); + it("dispatches a valid ask and returns its timestamp", async () => { const ask = vi.fn(async () => ({ answer: "It is checking the fix.", ts: 123 })); const respond = await invoke( diff --git a/ui/src/pages/chat/chat-session-companion.ts b/ui/src/pages/chat/chat-session-companion.ts index cbff4599aaf7..d0c934317c19 100644 --- a/ui/src/pages/chat/chat-session-companion.ts +++ b/ui/src/pages/chat/chat-session-companion.ts @@ -102,6 +102,13 @@ export class ChatSessionCompanionThreads { answer, ts, })); + if ( + thread.failedQuestion && + thread.exchanges.some((exchange) => exchange.question === thread.failedQuestion) + ) { + thread.failedQuestion = null; + thread.hint = null; + } thread.revision += 1; this.notify(); } catch { diff --git a/ui/src/pages/chat/chat-session-rail.test.ts b/ui/src/pages/chat/chat-session-rail.test.ts index 621f52a42be5..a56698dc2003 100644 --- a/ui/src/pages/chat/chat-session-rail.test.ts +++ b/ui/src/pages/chat/chat-session-rail.test.ts @@ -350,6 +350,25 @@ describe("ChatSessionCompanionThreads", () => { }); }); + it("clears a retry error when hydration confirms that exact answer committed", async () => { + const threads = new ChatSessionCompanionThreads(); + await threads.submit("one", "What changed?", async () => { + throw Object.assign(new Error("disconnected"), { + details: { reason: "context-unavailable" }, + }); + }); + + await threads.hydrate("one", async () => ({ + exchanges: [{ question: "What changed?", answer: "The fix committed.", ts: 4 }], + })); + + expect(threads.view("one")).toMatchObject({ + failedQuestion: null, + hint: null, + exchanges: [{ question: "What changed?", answer: "The fix committed.", ts: 4 }], + }); + }); + it("rejects an answer that settles after the owning connection changes", async () => { let current = true; let resolveAnswer!: (value: { answer: string; ts: number }) => void;