fix: reconcile companion progress ownership

This commit is contained in:
Shakker
2026-08-11 22:27:01 +02:00
parent 70fbc35da2
commit 6932cd6980
5 changed files with 73 additions and 10 deletions
+3 -8
View File
@@ -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(),
+11 -1
View File
@@ -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.
}
}
+33 -1
View File
@@ -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(
@@ -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 {
@@ -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;