From 3f86293c5d57fd214cbae03f6fa77a6eb8fbe1aa Mon Sep 17 00:00:00 2001 From: Yu Xie Date: Tue, 27 Oct 2026 17:33:49 +0100 Subject: [PATCH 1/7] fix(chat): retain verified Copilot history on failed refreshes Reuse existing failed-request retention for verified Copilot page reads and gap-fill failures. Bind retention to the displayed history, reject changed identities, and keep late timed-out responses from evicting current cache entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fad73aa-cbc5-42dd-916b-64e5198bc512 --- changelog.d/copilot-refresh-retention.md | 4 + scripts/chat-history-browser-qa.ts | 54 ++++++++++- server/conversation-shared.ts | 10 ++- server/conversation.ts | 28 +++++- server/copilot.contract.test.ts | 110 +++++++++++++++++++++++ server/copilot.test.ts | 2 +- server/pane-routes.ts | 4 +- shared/protocol.ts | 8 +- src/components/ChatView.tsx | 9 +- src/lib/api.ts | 13 ++- src/lib/conversationFetch.test.ts | 36 +++++++- 11 files changed, 260 insertions(+), 18 deletions(-) create mode 100644 changelog.d/copilot-refresh-retention.md diff --git a/changelog.d/copilot-refresh-retention.md b/changelog.d/copilot-refresh-retention.md new file mode 100644 index 0000000000..add66156e8 --- /dev/null +++ b/changelog.d/copilot-refresh-retention.md @@ -0,0 +1,4 @@ +### Fixed +- Copilot Chat keeps its messages while retrying a failed transcript page read when the same + conversation is still verified, instead of briefly showing the terminal fallback. +- A failed read of the gap between Chat pages no longer discards the messages already displayed. diff --git a/scripts/chat-history-browser-qa.ts b/scripts/chat-history-browser-qa.ts index 3a53a0bf1b..4ab6932f1e 100644 --- a/scripts/chat-history-browser-qa.ts +++ b/scripts/chat-history-browser-qa.ts @@ -60,6 +60,7 @@ try { let holdOlder = false; let product = false; let unavailable = false, terminalFails = false; + let readPending = false, gapPending = false; let holdTerminal = false; let terminalText = "Fictional terminal fallback"; /** product panes ("machine/pane") whose conversation requests wait for the script, and those that fail */ @@ -105,6 +106,19 @@ try { await route.fulfill({ json: { source: "scrollback", turns: [] } }); return; } + if (readPending || (gapPending && url.searchParams.has("before"))) { + const displayed = url.searchParams.get("history_id"); + if (displayed !== null && displayed !== epoch) { + await route.fulfill({ status: 409, json: { error: { code: "history_changed", message: "reload history" } } }); + return; + } + if (displayed === null && !url.searchParams.has("before") && !url.searchParams.has("from")) { + await route.fulfill({ json: { source: "scrollback", turns: [] } }); + return; + } + await route.fulfill({ status: 503, json: { error: { code: "conversation_read_pending", message: "The conversation could not be read; try again shortly." } } }); + return; + } const captured = epoch; const cursor = url.searchParams.get("before") ?? url.searchParams.get("from"); if (cursor && !cursor.startsWith(`${epoch}:`)) { @@ -114,7 +128,7 @@ try { if (url.searchParams.has("before")) { routeEvents.dispatchEvent(new Event("older")); if (holdOlder) await new Promise((resolve) => releases.push(resolve)); - await route.fulfill({ json: { source: "omp-transcript", history_id: captured, cursor: null, turns: [user("old earlier")] } }); + await route.fulfill({ json: { source: "omp-transcript", history_id: captured, cursor: url.searchParams.get("since"), turns: [user("old earlier")] } }); } else { const tool = { role: "assistant", ts: "reused", parts: [{ kind: "tool", name: "Read", summary: "same", input: "{}", output: "preview", output_ref: "reused", output_size: 9000 }] }; await route.fulfill({ json: { source: "omp-transcript", history_id: epoch, cursor: before, turns: newest ? [user(newest), tool] : [] } }); @@ -123,6 +137,44 @@ try { await page.goto(`http://127.0.0.1:${server.port}/`); await page.getByText("old newest", { exact: true }).waitFor(); const refresh = () => page.evaluate(() => window.qa.refresh()); + await page.locator(".work-block-head").click(); + await page.locator(".work-row-head").click(); + const retainedTurn = await page.getByText("old newest", { exact: true }).elementHandle(); + for (const gap of [false, true]) { + readPending = !gap; gapPending = gap; + if (gap) before = "one:200"; + const failed = page.waitForResponse((response) => response.url().includes("/conversation?") && response.status() === 503); + await refresh(); + await failed; + await page.getByText("Couldn't load the conversation", { exact: true }).waitFor(); + assert.equal(await retainedTurn!.evaluate((node) => node.isConnected), true, "failed newest/gap reads keep the same mounted turn"); + assert.equal(await page.locator(".work-block-head").getAttribute("aria-expanded"), "true", "failed reads keep opened work"); + assert.equal(await page.locator(".work-row-head").getAttribute("aria-expanded"), "true", "failed reads keep opened tool details"); + assert.equal(await page.locator(".chat-terminal-fallback").count(), 0, "a retryable read does not replace the conversation"); + readPending = false; gapPending = false; + await refresh(); + await page.getByText("Couldn't load the conversation", { exact: true }).waitFor({ state: "hidden" }); + assert.equal(await page.getByText("old newest", { exact: true }).isVisible(), true); + } + // A session switched before an unpaged read fails: retrying it must not retain the old session. + await page.evaluate(() => window.qa.chat("switched-before-read")); + await page.waitForFunction(() => window.qa.answered.includes("local/switched-before-read")); + await page.getByText("old newest", { exact: true }).waitFor(); + const switchedTurn = await page.getByText("old newest", { exact: true }).elementHandle(); + epoch = "different-session"; before = null; readPending = true; + const rejectedHistory = page.waitForResponse((response) => response.url().includes("/conversation?") && response.status() === 409); + const freshFallback = page.waitForResponse((response) => response.url().includes("/conversation?") && !new URL(response.url()).searchParams.has("history_id") && response.status() === 200); + await refresh(); + await rejectedHistory; await freshFallback; + await page.getByText("old newest", { exact: true }).waitFor({ state: "hidden" }); + assert.equal(await switchedTurn!.evaluate((node) => node.isConnected), false, "an unpaged history mismatch clears old turns even when its fresh retry fails"); + assert.equal(await page.getByText("old newest", { exact: true }).count(), 0); + readPending = false; + // Restore the original single-page fixture before the existing history-change cases. + epoch = "one-reset"; before = "one-reset:100"; + await refresh(); + await page.getByText("old newest", { exact: true }).waitFor(); + console.log("PASS newest and gap read failures retain mounted messages, opened work and tool details, then recover"); const output = page.locator("#output"); await page.locator("#fetch-output").click(); assert.equal(await page.evaluate(() => window.qa.requests.length), 1, "duplicate load deduplicated"); diff --git a/server/conversation-shared.ts b/server/conversation-shared.ts index 210d5a6f9b..4305c155e2 100644 --- a/server/conversation-shared.ts +++ b/server/conversation-shared.ts @@ -18,6 +18,14 @@ export class ConversationUnavailable extends Error { } } +/** A read failed, but the pane still proves the same current conversation: retain the view and retry. */ +export class ConversationReadPending extends Error { + constructor() { + super("The conversation could not be read; try again shortly."); + this.name = "ConversationReadPending"; + } +} + /** The pane's agent holds a session it has not written yet: a conversation with no turns, not a missing one. */ export class ConversationNotStarted extends ConversationUnavailable { constructor(readonly sessionId: string, readonly source: RecognizedConversation["source"] = "omo-transcript", readonly identity: string = sessionId) { @@ -70,4 +78,4 @@ export function answerVersion(key: string, signature: string): string { * while it is still inside the newest page); with `before`, the page ending there, * never reaching back past `since`. */ -export type ConversationPage = { before?: string; since?: string; from?: string }; +export type ConversationPage = { before?: string; since?: string; from?: string; history_id?: string }; diff --git a/server/conversation.ts b/server/conversation.ts index f35c71efdb..5556a681ae 100644 --- a/server/conversation.ts +++ b/server/conversation.ts @@ -64,7 +64,7 @@ import { forgetAllPiIndexes, forgetPiIndex, piAbandonedTurns, piBranchRecords, p import { copilotOffsetOutput, copilotOutput, copilotRootRecord, parseCopilotTranscript } from "./copilot.ts"; import { copilotExplicitResume, copilotHandoffSession, copilotProcessSession, copilotTranscriptFile, copilotUnwrittenSession, openCopilotFile } from "./copilot-store.ts"; import { parseConversationMetadata } from "./conversation-metadata.ts"; -import { answerVersion, ConversationNotStarted, ConversationUnavailable, type ConversationPage, HistoryChanged, type RecognizedConversation, type StreamSource, TRANSCRIPT_WINDOW_BYTES } from "./conversation-shared.ts"; +import { answerVersion, ConversationNotStarted, ConversationReadPending, ConversationUnavailable, type ConversationPage, HistoryChanged, type RecognizedConversation, type StreamSource, TRANSCRIPT_WINDOW_BYTES } from "./conversation-shared.ts"; import { forgetHermesTranscriptState, hermesConversationPage, hermesToolOutput, hermesTranscriptForPane } from "./hermes.ts"; import { createOmpTranscriptParser, isContextClear, MAX_TURNS, omoTaskResults, parseOmpTranscript, PI_IMAGE_REF, PI_PROMPT_IMAGE_REF, piImageBlock, piMessage, piNotice, piPromptImages, piResults, type TranscriptParser } from "./transcript-records.ts"; @@ -80,7 +80,7 @@ import { devinSessionForPane, isDevinProcess } from "./devin-session.ts"; export { isOmoProcess } from "./omo.ts"; export { devinSessionForPane, isDevinProcess } from "./devin-session.ts"; export { createClaudeTranscriptParser, parseClaudeTranscript, unwrapPastes } from "./claude-transcript.ts"; -export { TRANSCRIPT_WINDOW_BYTES, ConversationUnavailable, ConversationNotStarted, HistoryChanged, answerVersion, type RecognizedConversation, type ConversationPage } from "./conversation-shared.ts"; +export { TRANSCRIPT_WINDOW_BYTES, ConversationReadPending, ConversationUnavailable, ConversationNotStarted, HistoryChanged, answerVersion, type RecognizedConversation, type ConversationPage } from "./conversation-shared.ts"; const PI_PAGE_CACHE_BYTES = 4 * 1024 * 1024; @@ -1468,7 +1468,27 @@ export async function paneConversation(paneId: string, codexHome?: string, page: rememberPaneRead(paneId, resolved.path); const history = resolved.source === "copilot-transcript" ? copilotHistoryIdentity(resolved.path) : null; const prefix = history !== null && resolved.source === "copilot-transcript" ? copilotPrefix(paneId, `${resolved.binding}\0${history}`) : null; - const answer = transcriptPage(resolved.source, resolved.path, prefix === null ? page : copilotPage(page, prefix), resolved.codexHome ?? codexHome, resolved.piAgentDir); + let answer: RecognizedConversation; + try { + answer = transcriptPage(resolved.source, resolved.path, prefix === null ? page : copilotPage(page, prefix), resolved.codexHome ?? codexHome, resolved.piAgentDir); + } catch (error) { + if (resolved.source !== "copilot-transcript" || prefix === null || !(error instanceof ConversationUnavailable) || error.message !== "transcript_missing") throw error; + // A read failure is not an empty conversation. Only a fresh, matching binding + // and file generation permit retaining the view; never reuse peer claims here. + const fresh = await sessionSnapshot(); + const pane = fresh.panes.find((candidate) => candidate.pane_id === paneId); + if (pane === undefined) throw new ConversationUnavailable("no_session_id"); + const current = await copilotTranscriptPath(pane, fresh.panes); + if (current.path !== resolved.path || current.binding !== resolved.binding || copilotHistoryIdentity(resolved.path) !== history) { + if (page.before !== undefined || page.since !== undefined || page.from !== undefined) throw new HistoryChanged(); + throw new ConversationUnavailable("no_session_path"); + } + // Unpaged refreshes otherwise carry no proof of which history is on screen. + // Older clients without it keep their fail-closed fallback behavior. + if (page.history_id !== undefined && !page.history_id.startsWith(prefix)) throw new HistoryChanged(); + if (page.history_id === undefined && page.before === undefined && page.since === undefined && page.from === undefined) throw error; + throw new ConversationReadPending(); + } const status = snapshot.panes.find((pane) => pane.pane_id === paneId)?.agent_status; if (resolved.source === "grok-transcript" && answer.turns.length === 0 && page.before === undefined && (status === "working" || status === "blocked")) { @@ -1732,7 +1752,7 @@ function transcriptPageRead(source: StreamSource, path: string, page: Conversati if (live === null && page.before === undefined && start > stream.floor) head = metadataHead(path, stream, source, start); cursor = formatCursor(stream, start); } catch (error) { - if (error instanceof HistoryChanged) throw error; + if (error instanceof HistoryChanged || error instanceof ConversationUnavailable) throw error; throw new ConversationUnavailable("transcript_missing"); } diff --git a/server/copilot.contract.test.ts b/server/copilot.contract.test.ts index 22972ca7bb..628c0f0fa8 100644 --- a/server/copilot.contract.test.ts +++ b/server/copilot.contract.test.ts @@ -327,6 +327,116 @@ it.skipIf(process.platform !== "linux")("binds only the reported UUID to the for expect((await read()).source).toBe("scrollback"); }, 20_000); +it.skipIf(process.platform !== "linux")("retries failed Copilot page reads only while fresh ownership and file generation still agree", async () => { + const session = "12aa83d8-48bc-41f2-a498-7055c5c44a07"; + const switched = "12aa83d8-48bc-41f2-a498-7055c5c44a08"; + for (const id of [session, switched]) { + mkdirSync(join(home, "session-state", id), { recursive: true }); + writeFileSync(file(id), fixture(id, "retained history") + + Array.from({ length: 55 }, (_, i) => event("user.message", { content: `prompt ${i}` })).join("")); + } + const created = await workspaceCreate({ cwd: root, label: "herdr-web-ui-test-copilot-read-retry" }); + const current = created.root_pane.pane_id; + const realRead = fs.readSync; + let failingFile = file(session); + let fail = false; + let onFailure = () => {}; + function failedRead(fd: number, buffer: NodeJS.ArrayBufferView, offset: number, length: number, position: fs.ReadPosition | null): number; + function failedRead(fd: number, buffer: NodeJS.ArrayBufferView, options?: fs.ReadSyncOptions): number; + function failedRead(fd: number, buffer: NodeJS.ArrayBufferView, offset?: number | fs.ReadSyncOptions, length?: number, position?: fs.ReadPosition | null): number { + // Leave the bounded header/identity checks real; fail only a transcript page read. + if (fail && (length ?? 0) > 64 * 1024 && fs.readlinkSync(`/proc/self/fd/${fd}`) === failingFile) { + fail = false; + onFailure(); + throw Object.assign(new Error("synthetic private path must not escape"), { code: "EIO" }); + } + return typeof offset === "number" ? realRead(fd, buffer, offset, length!, position ?? null) : realRead(fd, buffer, offset); + } + let reads: ReturnType> | undefined; + const append = () => appendFileSync(failingFile, event("assistant.message", { content: "fixture ".repeat(10_000) })); + try { + await startFixture(current, session, "--lifecycle"); + const info = await paneProcessInfo(current); + const pid = info?.foreground_processes?.find((process) => process.argv?.includes(executable))?.pid; + expect(pid).toBeDefined(); + const log = join(home, "logs", `process-100-${pid}.log`); + const lifecycle = (id: string, action = "Registering") => + appendFileSync(log, `${new Date().toISOString()} [INFO] ${action} foreground session: ${id}\n`); + const url = `http://127.0.0.1:${app.port}/api/pane/conversation?pane_id=${encodeURIComponent(current)}`; + reads = spyOn(fs, "readSync").mockImplementation(failedRead); + const initial = await (await fetch(url)).json() as { history_id: string; cursor: string }; + expect(initial.cursor).toBeString(); + append(); fail = true; + const legacyFailure = await fetch(url); + expect(legacyFailure.status).toBe(200); + expect(await legacyFailure.json()).toEqual({ source: "scrollback", turns: [] }); + expect(fail).toBe(false); + for (const query of [`&history_id=${encodeURIComponent(initial.history_id)}`, `&from=${encodeURIComponent(initial.cursor)}`, `&before=${encodeURIComponent(initial.cursor)}`]) { + append(); fail = true; + const failed = await fetch(url + query, { headers: { "if-none-match": '"old-answer"' } }); + expect(failed.status).toBe(503); + expect(failed.headers.get("etag")).toBeNull(); + expect(failed.headers.get("cache-control")).toBe("no-store"); + expect(await failed.json()).toEqual({ error: { + code: "conversation_read_pending", message: "The conversation could not be read; try again shortly.", + } }); + expect(fail).toBe(false); + const recovered = await fetch(url); + expect(recovered.status).toBe(200); + expect((await recovered.json()).history_id).toBe(initial.history_id); + } + lifecycle(switched); + failingFile = file(switched); + append(); fail = true; + const switchedBeforeRead = await fetch(`${url}&history_id=${encodeURIComponent(initial.history_id)}`); + expect(fail).toBe(false); + expect(switchedBeforeRead.status).toBe(409); + expect((await (await fetch(url)).json()).history_id).not.toBe(initial.history_id); + failingFile = file(session); + lifecycle(session); + appendFileSync(file(session), "malformed complete record\n"); + expect((await (await fetch(url)).json()).source).toBe("scrollback"); + writeFileSync(file(session), fixture(session, "recovered history") + + Array.from({ length: 55 }, (_, i) => event("user.message", { content: `recovered prompt ${i}` })).join("")); + await fetch(url); + append(); fail = true; onFailure = () => lifecycle(session, "Unregistering"); + const revoked = await fetch(url); + expect(fail).toBe(false); + expect(revoked.status).toBe(200); + expect(await revoked.json()).toEqual({ source: "scrollback", turns: [] }); + for (const paged of [false, true]) { + lifecycle(session); + const before = await (await fetch(url)).json() as { cursor: string }; + expect(before.cursor).toBeString(); + append(); fail = true; onFailure = () => lifecycle(switched); + const changed = await fetch(url + (paged ? `&from=${encodeURIComponent(before.cursor)}` : "")); + expect(fail).toBe(false); + expect(changed.status).toBe(paged ? 409 : 200); + if (!paged) expect(await changed.json()).toEqual({ source: "scrollback", turns: [] }); + } + lifecycle(session); + await fetch(url); + append(); fail = true; onFailure = () => { + rmSync(file(session)); + writeFileSync(file(session), fixture(session, "replacement file")); + }; + const replaced = await fetch(url); + expect(fail).toBe(false); + expect(replaced.status).toBe(200); + expect(await replaced.json()).toEqual({ source: "scrollback", turns: [] }); + onFailure = () => {}; + const displayed = await (await fetch(url)).json() as { history_id: string }; + expect(displayed.history_id).toBeString(); + fs.renameSync(file(session), `${file(session)}.replaced`); + writeFileSync(file(session), fixture(session, "replaced before the next refresh")); + append(); fail = true; + const replacedBeforeRead = await fetch(`${url}&history_id=${encodeURIComponent(displayed.history_id)}`); + expect(fail).toBe(false); + expect(replacedBeforeRead.status).toBe(409); + expect((await (await fetch(url)).json()).history_id).not.toBe(displayed.history_id); + } finally { reads?.mockRestore(); await workspaceClose(created.workspace.workspace_id); } +}, 20_000); + it.skipIf(process.platform !== "linux")("reads a resumed CLI without marker or report and follows foreground switches rather than stale reports", async () => { const resumed = "12aa83d8-48bc-41f2-a498-7055c5c44a03"; const forked = "12aa83d8-48bc-41f2-a498-7055c5c44a04"; diff --git a/server/copilot.test.ts b/server/copilot.test.ts index 9cc332e513..bf20e6f9a7 100644 --- a/server/copilot.test.ts +++ b/server/copilot.test.ts @@ -922,7 +922,7 @@ it.each([ writeFileSync(path, start); expect(transcriptPage("copilot-transcript", path).turns).toEqual([]); appendFileSync(path, record); - expect(() => transcriptPage("copilot-transcript", path)).toThrow(ConversationUnavailable); + expect(() => transcriptPage("copilot-transcript", path)).toThrow("transcript_unreadable"); } finally { forgetTranscriptState(); rmSync(dir, { recursive: true, force: true }); } }); diff --git a/server/pane-routes.ts b/server/pane-routes.ts index 1d10a3e002..12f86f47be 100644 --- a/server/pane-routes.ts +++ b/server/pane-routes.ts @@ -9,7 +9,7 @@ import { mergeCatalogSources } from "./catalog-sources.ts"; import { paneCommands, rememberPaneCommands } from "./commands.ts"; import { paneFiles } from "./files.ts"; import { badRequest, conflictResponse, errorResponse, isCount, isJsonObject, jsonResponse, readJsonObject, setRequestRoute } from "./http.ts"; -import { conversationImage, ConversationUnavailable, forgetPaneTranscriptState, HistoryChanged, paneConversation, paneProcessRoots, paneTranscript, toolOutput } from "./conversation.ts"; +import { conversationImage, ConversationReadPending, ConversationUnavailable, forgetPaneTranscriptState, HistoryChanged, paneConversation, paneProcessRoots, paneTranscript, toolOutput } from "./conversation.ts"; import { OPENCODE_TOOL_REF } from "./opencode.ts"; import { openCodeInstalledVersion } from "./opencode-service.ts"; import { DevinHistoryChanged } from "./devin.ts"; @@ -256,6 +256,7 @@ export async function handlePaneRequest(request: Request, url: URL, pathname: st before: url.searchParams.get("before") ?? undefined, since: url.searchParams.get("since") ?? undefined, from: url.searchParams.get("from") ?? undefined, + history_id: url.searchParams.get("history_id") ?? undefined, }; try { const { version, ...conversation } = await paneConversation(paneId, options.codexHome, page, options.devinDbPath, options.opencodeDb, options.hermesHome, options.grokHome, options.museData); @@ -267,6 +268,7 @@ export async function handlePaneRequest(request: Request, url: URL, pathname: st return jsonResponse(conversation, 200, headers); } catch (error) { if (error instanceof HistoryChanged || error instanceof DevinHistoryChanged) return jsonResponse({ error: { code: "history_changed", message: error.message } }, 409); + if (error instanceof ConversationReadPending) return jsonResponse({ error: { code: "conversation_read_pending", message: error.message } }, 503, { "cache-control": "no-store" }); // an unrecognized pane is not an error: the client falls back to the // scrollback transcript, exactly like chatmux's terminal fallback. Its metadata is only // ever what the agent's own status line states diff --git a/shared/protocol.ts b/shared/protocol.ts index fee03520c9..1e5311180f 100644 --- a/shared/protocol.ts +++ b/shared/protocol.ts @@ -199,6 +199,9 @@ export type { DevicePairedEvent, Machine, MachineEvent, MachineRoster, PaneTarge * GET /api/pane/conversation?pane_id= -> ConversationResponse (structured agent * transcript turns - claude, codex, omp, omo, gjc, pi, devin, OpenCode, hermes, grok or agy; * source:"scrollback" when the pane has no recognized, readable store. + * Copilot page reads that fail while fresh ownership, file generation and the requested + * history_id (or bound cursor) still match answer 503 conversation_read_pending: + * retain the displayed view and retry, not fallback. A mismatched history_id answers 409. * Grok requires exact live process/session evidence, Agy live process/session evidence; * rewinds invalidate cursors. * Grok/Agy optional activity describes the latest native turn, independently of pane status.) @@ -502,7 +505,10 @@ export interface PlanSummary { current: string | null; } -/** GET /api/pane/conversation: native conversation with settings, or scrollback fallback. */ +/** + * GET /api/pane/conversation: native conversation with settings, or scrollback fallback. + * A refresh may echo history_id as a query parameter to identify the view it would retain on failure. + */ export interface ConversationResponse { /** Grok/Agy latest native turn; display feedback only, never input-delivery authority. */ activity?: "thinking" | "working" | "idle"; diff --git a/src/components/ChatView.tsx b/src/components/ChatView.tsx index 8e4e08734d..7a90a7a152 100644 --- a/src/components/ChatView.tsx +++ b/src/components/ChatView.tsx @@ -1409,7 +1409,10 @@ export const ChatView = memo(function ChatView({ paneId, presentationKey, finish pages.unshift(page.turns); before = page.cursor; } - } catch { + } catch (cause) { + // Failed gap reads keep the same timeline, just as a failed newest-page + // read does. Only a rejected cursor means these pages cannot be joined. + if (!(cause instanceof ApiError) || cause.status !== 409) throw cause; return null; } return before === held ? pages.flat() : null; @@ -1418,7 +1421,7 @@ export const ChatView = memo(function ChatView({ paneId, presentationKey, finish const readKey = presentationRef.current; const finish = finishRef.current; const previousAnswer = lastAnswer.current ?? undefined; - const page = { ...(heldFrom.current === null ? {} : { from: heldFrom.current }), fresh: readKey !== validatedPresentation.current }; + const page = { ...(heldFrom.current === null ? {} : { from: heldFrom.current }), history_id: previousAnswer?.history_id, fresh: readKey !== validatedPresentation.current }; let generation = olderGeneration.current; try { let conversation; @@ -1426,7 +1429,7 @@ export const ChatView = memo(function ChatView({ paneId, presentationKey, finish conversation = await fetchPaneConversation(paneId, page); } catch (cause) { // a new session or a Codex backtrack replaced the transcript the older pages came from - if (heldFrom.current === null || !(cause instanceof ApiError) || cause.status !== 409) throw cause; + if (!(cause instanceof ApiError) || cause.status !== 409) throw cause; if (cancelled || generation !== olderGeneration.current) return; dropOlder(); setState(EMPTY_STATE); setLoaded(false); signature.current = ""; lastAnswer.current = null; generation = olderGeneration.current; diff --git a/src/lib/api.ts b/src/lib/api.ts index 9c0be07379..4b0c48a858 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -289,7 +289,7 @@ export async function fetchPaneTranscript(paneId: string, lines: number, machine } /** Which turns (ConversationResponse.cursor): the page `before` a cursor, not past `since`; the newest ones `from` a held start. */ -export type ConversationPageQuery = { before?: string; since?: string; from?: string; /** Revalidate a new finish without borrowing another key's 304. */ fresh?: boolean }; +export type ConversationPageQuery = { before?: string; since?: string; from?: string; /** History currently displayed, even if its cached response was evicted. */ history_id?: string; /** Revalidate a new finish without borrowing another key's 304. */ fresh?: boolean }; /** * The last answer per newest-page conversation URL and its ETag. The chat reads it again on @@ -359,12 +359,19 @@ export async function fetchPaneConversation(paneId: string, machineId = "local", const url = conversationUrl(paneId, machineId, page); const polled = page.before === undefined; const known = polled ? conversationAnswers.get(url) : undefined; + const requestUrl = page.history_id === undefined ? url : `${url}&history_id=${encodeURIComponent(page.history_id)}`; const { response, body } = await withChatReadDeadline(async (signal) => { - const response = await fetch(url, { cache: "no-store", signal, ...(known && !page.fresh ? { headers: { "if-none-match": known.etag } } : {}) }); + const response = await fetch(requestUrl, { cache: "no-store", signal, ...(known && !page.fresh ? { headers: { "if-none-match": known.etag } } : {}) }); if (response.status === 304 && known) return { response, body: known.body }; if (!response.ok) throw await errorFrom(url, response); return { response, body: (await response.json()) as ConversationResponse }; - }, chatReadDeadlineMs(machineId)); + }, chatReadDeadlineMs(machineId)).catch((cause: unknown) => { + if (cause instanceof ApiError && cause.status === 409) { + conversationAnswers.delete(url); + conversationAnswers.delete(conversationUrl(paneId, machineId, {})); + } + throw cause; + }); // Cache changes occur after the bounded read: a late, aborted answer cannot replace it. if (response.status === 304 && known) { conversationAnswers.delete(url); diff --git a/src/lib/conversationFetch.test.ts b/src/lib/conversationFetch.test.ts index 56837c21c2..a8c437d751 100644 --- a/src/lib/conversationFetch.test.ts +++ b/src/lib/conversationFetch.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, spyOn } from "bun:test"; -import { fetchPaneConversation, conversationAnswerBytes, peekPaneConversation } from "./api.ts"; +import { ApiError, fetchPaneConversation, conversationAnswerBytes, peekPaneConversation } from "./api.ts"; import { ConversationRefresh } from "./conversationRefresh.ts"; import type { ConversationResponse } from "../../shared/protocol.ts"; @@ -16,6 +16,36 @@ afterEach(() => { }); describe("conversation polling", () => { + it("echoes displayed history, retains it on a retryable read, and forgets it on a rejected history", async () => { + const requests: { history: string | null; etag: string | null }[] = []; + let status = 200; + let history = "history-a"; + globalThis.fetch = (async (url: string, init?: RequestInit) => { + requests.push({ history: new URL(url, "http://fixture").searchParams.get("history_id"), etag: new Headers(init?.headers).get("if-none-match") }); + if (status === 304) return new Response(null, { status }); + if (status !== 200) return Response.json({ error: { code: status === 409 ? "history_changed" : "conversation_read_pending", message: "retry" } }, { status }); + return Response.json({ source: "copilot-transcript", history_id: history, cursor: null, turns: [] }, { headers: { etag: `"${history}"` } }); + }) as typeof fetch; + const first = await fetchPaneConversation("retained-history"); + const displayed = { history_id: first.history_id }; + status = 503; + await expect(fetchPaneConversation("retained-history", "local", displayed)).rejects.toBeInstanceOf(ApiError); + expect(peekPaneConversation("retained-history")).toBe(first); + status = 304; + expect(await fetchPaneConversation("retained-history", "local", displayed)).toBe(first); + status = 409; + await expect(fetchPaneConversation("retained-history", "local", { ...displayed, from: "held-a" })).rejects.toBeInstanceOf(ApiError); + expect(peekPaneConversation("retained-history")).toBeUndefined(); + status = 200; history = "history-b"; + expect((await fetchPaneConversation("retained-history")).history_id).toBe(history); + expect(requests).toEqual([ + { history: null, etag: null }, + ...Array.from({ length: 2 }, () => ({ history: "history-a", etag: '"history-a"' })), + { history: "history-a", etag: null }, + { history: null, etag: null }, + ]); + }); + it("revalidates a new finish without borrowing the previous finish's ETag", async () => { const sent: (string | null)[] = []; const body: ConversationResponse = { source: "claude-transcript", turns: [] }; @@ -552,7 +582,7 @@ describe("conversation invalidation refresh", () => { }); }); -it("a timed-out JSON body cannot replace the last conversation ETag, even when it answers late", async () => { +it.each([200, 409])("a timed-out read cannot replace or clear the last conversation ETag after a late %s response", async (status) => { const originalTimer = globalThis.setTimeout; const clock = spyOn(globalThis, "setTimeout").mockImplementation(((callback: () => void, delay: number) => originalTimer(callback, delay === 15_000 ? 10 : delay)) as typeof setTimeout); try { @@ -561,7 +591,7 @@ it("a timed-out JSON body cannot replace the last conversation ETag, even when i const current = await fetchPaneConversation(pane); let resolve!: (body: ConversationResponse) => void; const late = new Promise((done) => { resolve = done; }); - const lateResponse = Response.json({}, { headers: { etag: '"obsolete"' } }); + const lateResponse = Response.json({}, { status, headers: { etag: '"obsolete"' } }); lateResponse.json = () => late; globalThis.fetch = (async (_url: string) => lateResponse) as typeof fetch; await expect(fetchPaneConversation(pane)).rejects.toThrow("timed out"); From 591ef1a4f44579fd5ccd624f339715bbd47b95cb Mon Sep 17 00:00:00 2001 From: Yu Xie Date: Tue, 27 Oct 2026 20:56:36 +0100 Subject: [PATCH 2/7] fix(chat): reject revoked paginated Copilot histories consistently Map unavailable post-read ownership and file revalidation to history_changed for cursor requests, preserving unpaged fallback and unrelated errors. Cover revoked ownership and disappearing files during from/before reads. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fad73aa-cbc5-42dd-916b-64e5198bc512 --- server/conversation.ts | 18 +++++++++++------- server/copilot.contract.test.ts | 28 ++++++++++++++++++++++------ 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/server/conversation.ts b/server/conversation.ts index 5556a681ae..f581fd65fd 100644 --- a/server/conversation.ts +++ b/server/conversation.ts @@ -1475,13 +1475,17 @@ export async function paneConversation(paneId: string, codexHome?: string, page: if (resolved.source !== "copilot-transcript" || prefix === null || !(error instanceof ConversationUnavailable) || error.message !== "transcript_missing") throw error; // A read failure is not an empty conversation. Only a fresh, matching binding // and file generation permit retaining the view; never reuse peer claims here. - const fresh = await sessionSnapshot(); - const pane = fresh.panes.find((candidate) => candidate.pane_id === paneId); - if (pane === undefined) throw new ConversationUnavailable("no_session_id"); - const current = await copilotTranscriptPath(pane, fresh.panes); - if (current.path !== resolved.path || current.binding !== resolved.binding || copilotHistoryIdentity(resolved.path) !== history) { - if (page.before !== undefined || page.since !== undefined || page.from !== undefined) throw new HistoryChanged(); - throw new ConversationUnavailable("no_session_path"); + try { + const fresh = await sessionSnapshot(); + const pane = fresh.panes.find((candidate) => candidate.pane_id === paneId); + if (pane === undefined) throw new ConversationUnavailable("no_session_id"); + const current = await copilotTranscriptPath(pane, fresh.panes); + if (current.path !== resolved.path || current.binding !== resolved.binding || copilotHistoryIdentity(resolved.path) !== history) { + throw new ConversationUnavailable("no_session_path"); + } + } catch (cause) { + if (cause instanceof ConversationUnavailable && (page.before !== undefined || page.since !== undefined || page.from !== undefined)) throw new HistoryChanged(); + throw cause; } // Unpaged refreshes otherwise carry no proof of which history is on screen. // Older clients without it keep their fail-closed fallback behavior. diff --git a/server/copilot.contract.test.ts b/server/copilot.contract.test.ts index 1adc5998a2..2e236eac10 100644 --- a/server/copilot.contract.test.ts +++ b/server/copilot.contract.test.ts @@ -400,12 +400,16 @@ it.skipIf(process.platform !== "linux")("retries failed Copilot page reads only expect((await (await fetch(url)).json()).source).toBe("scrollback"); writeFileSync(file(session), fixture(session, "recovered history") + Array.from({ length: 55 }, (_, i) => event("user.message", { content: `recovered prompt ${i}` })).join("")); - await fetch(url); - append(); fail = true; onFailure = () => lifecycle(session, "Unregistering"); - const revoked = await fetch(url); - expect(fail).toBe(false); - expect(revoked.status).toBe(200); - expect(await revoked.json()).toEqual({ source: "scrollback", turns: [] }); + for (const selector of ["", "from", "before"]) { + lifecycle(session); + const before = await (await fetch(url)).json() as { cursor: string }; + expect(before.cursor).toBeString(); + append(); fail = true; onFailure = () => lifecycle(session, "Unregistering"); + const revoked = await fetch(url + (selector ? `&${selector}=${encodeURIComponent(before.cursor)}` : "")); + expect(fail).toBe(false); + expect(revoked.status).toBe(selector ? 409 : 200); + if (!selector) expect(await revoked.json()).toEqual({ source: "scrollback", turns: [] }); + } for (const paged of [false, true]) { lifecycle(session); const before = await (await fetch(url)).json() as { cursor: string }; @@ -436,6 +440,18 @@ it.skipIf(process.platform !== "linux")("retries failed Copilot page reads only expect(fail).toBe(false); expect(replacedBeforeRead.status).toBe(409); expect((await (await fetch(url)).json()).history_id).not.toBe(displayed.history_id); + for (const selector of ["from", "before"]) { + writeFileSync(file(session), fixture(session, "file removed during the read") + + Array.from({ length: 55 }, (_, i) => event("user.message", { content: `prompt ${i}` })).join("")); + lifecycle(session); + const before = await (await fetch(url)).json() as { cursor: string }; + expect(before.cursor).toBeString(); + append(); fail = true; onFailure = () => rmSync(file(session)); + const missing = await fetch(`${url}&${selector}=${encodeURIComponent(before.cursor)}`); + expect(fail).toBe(false); + expect(missing.status).toBe(409); + expect((await missing.json()).error.code).toBe("history_changed"); + } } finally { reads?.mockRestore(); await workspaceClose(created.workspace.workspace_id); } }, 20_000); From 43535fdd9c03fab521fbc3534dcf20e5bf1c23f3 Mon Sep 17 00:00:00 2001 From: devswha Date: Wed, 28 Oct 2026 08:05:22 +0900 Subject: [PATCH 3/7] fix(chat): recheck Copilot ownership after peer checks before retaining history A failed page read revalidated the pane's binding before awaiting its peers' process checks, so a session switch during those checks passed revalidation and the 503 kept the previous session on screen. Read the pane's own binding again once the peer claims are in. --- server/conversation.ts | 5 ++++- server/copilot.test.ts | 50 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/server/conversation.ts b/server/conversation.ts index f581fd65fd..aa2233b9b2 100644 --- a/server/conversation.ts +++ b/server/conversation.ts @@ -1479,7 +1479,10 @@ export async function paneConversation(paneId: string, codexHome?: string, page: const fresh = await sessionSnapshot(); const pane = fresh.panes.find((candidate) => candidate.pane_id === paneId); if (pane === undefined) throw new ConversationUnavailable("no_session_id"); - const current = await copilotTranscriptPath(pane, fresh.panes); + // Peer checks await herdr, and the session can switch meanwhile: read this + // pane's own binding again once they are in, so the comparison sees the latest. + const { peerClaims } = await copilotTranscriptPath(pane, fresh.panes); + const current = await copilotTranscriptPath(pane, fresh.panes, peerClaims); if (current.path !== resolved.path || current.binding !== resolved.binding || copilotHistoryIdentity(resolved.path) !== history) { throw new ConversationUnavailable("no_session_path"); } diff --git a/server/copilot.test.ts b/server/copilot.test.ts index 765af08a38..b63b16e971 100644 --- a/server/copilot.test.ts +++ b/server/copilot.test.ts @@ -475,6 +475,56 @@ it.skipIf(process.platform !== "linux")("reuses peer claims across conversation } }); +it.skipIf(process.platform !== "linux")("does not retain the old history when the session switches while a failed read rechecks its peers", async () => { + const root = mkdtempSync(join(tmpdir(), "copilot-recheck-")); + const transcript = (id: string) => join(root, "session-state", id, "events.jsonl"); + for (const id of [session, other]) { + mkdirSync(join(root, "session-state", id), { recursive: true }); + writeFileSync(transcript(id), start + prompt(id)); + } + mkdirSync(join(root, "logs")); + const log = join(root, "logs", `process-100-${process.pid}.log`); + // The lifecycle log counts only while this process holds it open, as Copilot does. + const held = fs.openSync(log, "a"); + appendFileSync(log, register(session)); + let failing = false; + let failed = false; + let switched = false; + const rpc = spyOn(herdr, "herdrRpc").mockResolvedValue({ process_info: { foreground_processes: [] } }); + const info = spyOn(herdr, "paneProcessInfo").mockImplementation(async (paneId) => { + // The peer check of the recheck, after the read failed, is where the switch lands. + if (paneId === "peer") { + if (failed && !switched) { appendFileSync(log, register(other)); switched = true; } + return { pane_id: paneId, foreground_processes: [] }; + } + return { pane_id: paneId, foreground_processes: [{ pid: process.pid, name: "copilot", argv: ["copilot"] }] }; + }); + const environment = spyOn(omo, "processEnviron").mockReturnValue([`COPILOT_HOME=${root}`]); + const snapshots = spyOn(herdr, "sessionSnapshot").mockResolvedValue({ + panes: ["own", "peer"].map((pane_id) => ({ pane_id, agent: "copilot", cwd: root })), agents: [], + } as unknown as SessionSnapshot); + const readSync = fs.readSync; + const reads = spyOn(fs, "readSync").mockImplementation(((fd: number, ...rest: [NodeJS.ArrayBufferView, number, number, number | null]) => { + if (failing && rest[2] > 65536 && fs.readlinkSync(`/proc/self/fd/${fd}`) === transcript(session)) { + failed = true; + throw Object.assign(new Error("EIO"), { code: "EIO" }); + } + return readSync(fd, ...rest); + }) as typeof fs.readSync); + try { + const shown = await paneConversation("own"); + expect(shown.source).toBe("copilot-transcript"); + appendFileSync(transcript(session), answer("long ".repeat(15000))); + failing = true; + await expect(paneConversation("own", undefined, { history_id: shown.history_id })).rejects.toMatchObject({ name: "ConversationUnavailable" }); + expect(switched).toBe(true); + } finally { + reads.mockRestore(); snapshots.mockRestore(); environment.mockRestore(); info.mockRestore(); rpc.mockRestore(); + fs.closeSync(held); forgetTranscriptState(); + rmSync(root, { recursive: true, force: true }); + } +}); + it.skipIf(process.platform !== "linux")("uses only the process home and a canonical regular file with the matching native header", async () => { const root = mkdtempSync(join(tmpdir(), "copilot-store-")); const custom = join(root, "custom"); From cad580460a33a4cda738236eb314c9f147bd9963 Mon Sep 17 00:00:00 2001 From: devswha Date: Wed, 28 Oct 2026 08:13:39 +0900 Subject: [PATCH 4/7] fix(chat): read Copilot peer claims again after the pane's own recheck The recheck reused the peer claims it had gathered before its last own-process call, so a peer that claimed the session during that call was missed and the 503 kept history whose ownership was already disputed. Keep the peers' process info from the first pass and read their lifecycle claims again right after the pane's own binding, with no herdr call between those reads and the comparison. --- server/conversation.ts | 17 +++++++++----- server/copilot.test.ts | 50 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/server/conversation.ts b/server/conversation.ts index aa2233b9b2..4952d4c664 100644 --- a/server/conversation.ts +++ b/server/conversation.ts @@ -1121,16 +1121,19 @@ async function ompTranscriptPath(paneId: string, cwd: string): Promise<{ path: s } /** A process-owned lifecycle registration, or a trusted report plus a unique held marker. */ -async function copilotTranscriptPath(pane: HerdrPane, panes: readonly HerdrPane[], previousClaims?: ReadonlySet): Promise<{ path: string; binding: string; peerClaims: ReadonlySet }> { +async function copilotTranscriptPath(pane: HerdrPane, panes: readonly HerdrPane[], previousClaims?: ReadonlySet, peerInfo?: Map> | null>): Promise<{ path: string; binding: string; peerClaims: ReadonlySet }> { const report = pane.agent_session; // A request shares its peer snapshot across the read; the pane's own process // and lifecycle evidence are still validated afresh before returning history. + // With `peerInfo`, peers' processes come from (and fill) that cache, while their + // lifecycle logs are read again here, after this pane's own binding. let peerClaims: Promise> | undefined = previousClaims === undefined ? undefined : Promise.resolve(previousClaims); const gatherPeerClaims = async (): Promise> => { const claims = new Set(); for (const peer of panes) { if (peer.pane_id === pane.pane_id || peer.agent !== "copilot") continue; - const info = await paneProcessInfo(peer.pane_id).catch(() => null); + const info = peerInfo?.has(peer.pane_id) ? peerInfo.get(peer.pane_id) : await paneProcessInfo(peer.pane_id).catch(() => null); + peerInfo?.set(peer.pane_id, info ?? null); const processes = agentProcesses(info?.foreground_processes ?? [], "copilot"); if (processes.length === 1 && typeof processes[0]?.pid === "number") { const live = copilotProcessSession(processEnviron(processes[0].pid), processes[0].argv ?? [], processes[0].pid); @@ -1479,10 +1482,12 @@ export async function paneConversation(paneId: string, codexHome?: string, page: const fresh = await sessionSnapshot(); const pane = fresh.panes.find((candidate) => candidate.pane_id === paneId); if (pane === undefined) throw new ConversationUnavailable("no_session_id"); - // Peer checks await herdr, and the session can switch meanwhile: read this - // pane's own binding again once they are in, so the comparison sees the latest. - const { peerClaims } = await copilotTranscriptPath(pane, fresh.panes); - const current = await copilotTranscriptPath(pane, fresh.panes, peerClaims); + // Peer checks await herdr, and either side can switch meanwhile: once every + // process is known, read this pane's binding and the peers' claims again with no + // herdr call between those reads and the comparison. + const peerInfo = new Map> | null>(); + await copilotTranscriptPath(pane, fresh.panes, undefined, peerInfo); + const current = await copilotTranscriptPath(pane, fresh.panes, undefined, peerInfo); if (current.path !== resolved.path || current.binding !== resolved.binding || copilotHistoryIdentity(resolved.path) !== history) { throw new ConversationUnavailable("no_session_path"); } diff --git a/server/copilot.test.ts b/server/copilot.test.ts index b63b16e971..8a710e8010 100644 --- a/server/copilot.test.ts +++ b/server/copilot.test.ts @@ -525,6 +525,56 @@ it.skipIf(process.platform !== "linux")("does not retain the old history when th } }); +it.skipIf(process.platform !== "linux")("does not retain the old history when a peer claims its session during the failed read's recheck", async () => { + const root = mkdtempSync(join(tmpdir(), "copilot-peer-recheck-")); + const transcript = join(root, "session-state", session, "events.jsonl"); + mkdirSync(join(root, "session-state", session), { recursive: true }); + mkdirSync(join(root, "logs")); + writeFileSync(transcript, start + prompt("history")); + const ownLog = join(root, "logs", `process-100-${process.pid}.log`); + const held = fs.openSync(ownLog, "a"); + appendFileSync(ownLog, register(session)); + // A peer Copilot that holds its own lifecycle log open, on another session. + const peer = Bun.spawn([process.execPath, "-e", `require("node:fs").openSync(process.argv[1] + "/logs/process-100-" + process.pid + ".log", "a"); console.log("ready"); process.stdin.resume();`, root], { stdin: "pipe", stdout: "pipe" }); + expect(new TextDecoder().decode((await peer.stdout.getReader().read()).value)).toContain("ready"); + const peerLog = join(root, "logs", `process-100-${peer.pid}.log`); + appendFileSync(peerLog, register(other)); + let failing = false; + let failed = false; + let ownChecks = 0; + let switched = false; + const rpc = spyOn(herdr, "herdrRpc").mockResolvedValue({ process_info: { foreground_processes: [] } }); + const info = spyOn(herdr, "paneProcessInfo").mockImplementation(async (paneId) => { + // The recheck's last own-process call comes after the peers were asked: the peer claims the session then. + if (failed && paneId === "own" && ++ownChecks === 2) { appendFileSync(peerLog, register(session)); switched = true; } + return { pane_id: paneId, foreground_processes: [{ pid: paneId === "own" ? process.pid : peer.pid, name: "copilot", argv: ["copilot"] }] }; + }); + const environment = spyOn(omo, "processEnviron").mockReturnValue([`COPILOT_HOME=${root}`]); + const snapshots = spyOn(herdr, "sessionSnapshot").mockResolvedValue({ + panes: ["own", "peer"].map((pane_id) => ({ pane_id, agent: "copilot", cwd: root })), agents: [], + } as unknown as SessionSnapshot); + const readSync = fs.readSync; + const reads = spyOn(fs, "readSync").mockImplementation(((fd: number, ...rest: [NodeJS.ArrayBufferView, number, number, number | null]) => { + if (failing && rest[2] > 65536 && fs.readlinkSync(`/proc/self/fd/${fd}`) === transcript) { + failed = true; + throw Object.assign(new Error("EIO"), { code: "EIO" }); + } + return readSync(fd, ...rest); + }) as typeof fs.readSync); + try { + const shown = await paneConversation("own"); + expect(shown.source).toBe("copilot-transcript"); + appendFileSync(transcript, answer("long ".repeat(15000))); + failing = true; + await expect(paneConversation("own", undefined, { history_id: shown.history_id })).rejects.toMatchObject({ name: "ConversationUnavailable" }); + expect(switched).toBe(true); + } finally { + reads.mockRestore(); snapshots.mockRestore(); environment.mockRestore(); info.mockRestore(); rpc.mockRestore(); + fs.closeSync(held); peer.kill(); await peer.exited; forgetTranscriptState(); + rmSync(root, { recursive: true, force: true }); + } +}); + it.skipIf(process.platform !== "linux")("uses only the process home and a canonical regular file with the matching native header", async () => { const root = mkdtempSync(join(tmpdir(), "copilot-store-")); const custom = join(root, "custom"); From f282ae5517f91d0f94d5689eca87b32e835420c4 Mon Sep 17 00:00:00 2001 From: devswha Date: Wed, 28 Oct 2026 08:25:38 +0900 Subject: [PATCH 5/7] fix(chat): fail a Copilot recheck closed when a cached peer process has exited The recheck reads peers' claims from the process info it cached in its first pass. A peer whose Copilot exited during the pane's last process call may have been replaced by one that claims this session, and the cache never names it: such a recheck now rejects the read instead of retaining history. --- server/conversation.ts | 7 +++++-- server/copilot.test.ts | 29 +++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/server/conversation.ts b/server/conversation.ts index 4952d4c664..b19e46d6d7 100644 --- a/server/conversation.ts +++ b/server/conversation.ts @@ -47,7 +47,7 @@ import { CODEX_IMAGE_REF, CODEX_TOOL_IMAGE_REF, codexOutputPicture, codexTranscr import { AmbiguousClaudeStore, claudeAttachSession, claudeProcessSession, claudeTranscriptFile, defaultClaudeConfigDir, forgetClaudeSessionFile, forgetClaudeSessions, isClaudeProcess, processClaudeConfigDir } from "./claude-store.ts"; import { forgetGjcPane, forgetGjcState, gjcPidUnderShell, gjcTranscriptForPane, isGjcProcess, storeRelative } from "./gjc-runtime.ts"; import { isOmoProcess, omoProcessIdentity, omoSessionForPane, processEnviron } from "./omo.ts"; -import { processStartedAtAsync } from "./process-start.ts"; +import { processAlive, processStartedAtAsync } from "./process-start.ts"; import { ompBlobStore, ompPaneRoute, ompRouteOwns, ompRouteStore, ompRouteTranscript } from "./omp.ts"; import { forgetOpencodeRead, forgetOpencodeState, OPENCODE_IMAGE_REF, opencodeConversation, opencodeDatabasePath, opencodeImage, opencodeReadKey, opencodeSessionId, opencodeToolOutput } from "./opencode.ts"; import { piTranscriptPath, unwrittenSession } from "./pi.ts"; @@ -1132,9 +1132,12 @@ async function copilotTranscriptPath(pane: HerdrPane, panes: readonly HerdrPane[ const claims = new Set(); for (const peer of panes) { if (peer.pane_id === pane.pane_id || peer.agent !== "copilot") continue; - const info = peerInfo?.has(peer.pane_id) ? peerInfo.get(peer.pane_id) : await paneProcessInfo(peer.pane_id).catch(() => null); + const cached = peerInfo?.has(peer.pane_id) === true; + const info = cached ? peerInfo!.get(peer.pane_id) : await paneProcessInfo(peer.pane_id).catch(() => null); peerInfo?.set(peer.pane_id, info ?? null); const processes = agentProcesses(info?.foreground_processes ?? [], "copilot"); + // A cached peer process that has exited may have been replaced by one we never asked about. + if (cached && processes.some((entry) => typeof entry.pid === "number" && !processAlive(entry.pid))) throw new ConversationUnavailable("no_session_path"); if (processes.length === 1 && typeof processes[0]?.pid === "number") { const live = copilotProcessSession(processEnviron(processes[0].pid), processes[0].argv ?? [], processes[0].pid); if (live?.logged) { diff --git a/server/copilot.test.ts b/server/copilot.test.ts index 8a710e8010..4c183b4a5c 100644 --- a/server/copilot.test.ts +++ b/server/copilot.test.ts @@ -525,7 +525,7 @@ it.skipIf(process.platform !== "linux")("does not retain the old history when th } }); -it.skipIf(process.platform !== "linux")("does not retain the old history when a peer claims its session during the failed read's recheck", async () => { +it.skipIf(process.platform !== "linux").each([false, true])("does not retain the old history when a peer claims its session during the failed read's recheck (peer replaced: %p)", async (replaced) => { const root = mkdtempSync(join(tmpdir(), "copilot-peer-recheck-")); const transcript = join(root, "session-state", session, "events.jsonl"); mkdirSync(join(root, "session-state", session), { recursive: true }); @@ -535,10 +535,17 @@ it.skipIf(process.platform !== "linux")("does not retain the old history when a const held = fs.openSync(ownLog, "a"); appendFileSync(ownLog, register(session)); // A peer Copilot that holds its own lifecycle log open, on another session. - const peer = Bun.spawn([process.execPath, "-e", `require("node:fs").openSync(process.argv[1] + "/logs/process-100-" + process.pid + ".log", "a"); console.log("ready"); process.stdin.resume();`, root], { stdin: "pipe", stdout: "pipe" }); - expect(new TextDecoder().decode((await peer.stdout.getReader().read()).value)).toContain("ready"); - const peerLog = join(root, "logs", `process-100-${peer.pid}.log`); - appendFileSync(peerLog, register(other)); + const copilot = async () => { + const child = Bun.spawn([process.execPath, "-e", `require("node:fs").openSync(process.argv[1] + "/logs/process-100-" + process.pid + ".log", "a"); console.log("ready"); process.stdin.resume();`, root], { stdin: "pipe", stdout: "pipe" }); + expect(new TextDecoder().decode((await child.stdout.getReader().read()).value)).toContain("ready"); + const log = join(root, "logs", `process-100-${child.pid}.log`); + appendFileSync(log, register(other)); + return { child, log }; + }; + const peer = await copilot(); + // With `replaced`, the peer pane's Copilot exits and a new one there claims the session. + const successor = await copilot(); + let active = peer; let failing = false; let failed = false; let ownChecks = 0; @@ -546,8 +553,12 @@ it.skipIf(process.platform !== "linux")("does not retain the old history when a const rpc = spyOn(herdr, "herdrRpc").mockResolvedValue({ process_info: { foreground_processes: [] } }); const info = spyOn(herdr, "paneProcessInfo").mockImplementation(async (paneId) => { // The recheck's last own-process call comes after the peers were asked: the peer claims the session then. - if (failed && paneId === "own" && ++ownChecks === 2) { appendFileSync(peerLog, register(session)); switched = true; } - return { pane_id: paneId, foreground_processes: [{ pid: paneId === "own" ? process.pid : peer.pid, name: "copilot", argv: ["copilot"] }] }; + if (failed && paneId === "own" && ++ownChecks === 2) { + if (replaced) { peer.child.kill(); await peer.child.exited; active = successor; } + appendFileSync(active.log, register(session)); + switched = true; + } + return { pane_id: paneId, foreground_processes: [{ pid: paneId === "own" ? process.pid : active.child.pid, name: "copilot", argv: ["copilot"] }] }; }); const environment = spyOn(omo, "processEnviron").mockReturnValue([`COPILOT_HOME=${root}`]); const snapshots = spyOn(herdr, "sessionSnapshot").mockResolvedValue({ @@ -570,7 +581,9 @@ it.skipIf(process.platform !== "linux")("does not retain the old history when a expect(switched).toBe(true); } finally { reads.mockRestore(); snapshots.mockRestore(); environment.mockRestore(); info.mockRestore(); rpc.mockRestore(); - fs.closeSync(held); peer.kill(); await peer.exited; forgetTranscriptState(); + fs.closeSync(held); + for (const { child } of [peer, successor]) { child.kill(); await child.exited; } + forgetTranscriptState(); rmSync(root, { recursive: true, force: true }); } }); From 5d4058054e7db344fa0446d3e0ec6e20fbb9c2fa Mon Sep 17 00:00:00 2001 From: devswha Date: Wed, 28 Oct 2026 08:54:39 +0900 Subject: [PATCH 6/7] fix(chat): keep an unpaged Copilot refresh without a history on its fallback before any recheck A request that names no displayed history and no cursor can never be retained, yet it still waited on the herdr recheck, and a herdr error there replaced its scrollback fallback with an error. Such a request now takes the fallback before the recheck. Suggested by CodeRabbit. --- server/conversation.ts | 11 ++++++----- server/copilot.test.ts | 7 +++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/server/conversation.ts b/server/conversation.ts index b19e46d6d7..54cf0db80e 100644 --- a/server/conversation.ts +++ b/server/conversation.ts @@ -1479,8 +1479,12 @@ export async function paneConversation(paneId: string, codexHome?: string, page: answer = transcriptPage(resolved.source, resolved.path, prefix === null ? page : copilotPage(page, prefix), resolved.codexHome ?? codexHome, resolved.piAgentDir); } catch (error) { if (resolved.source !== "copilot-transcript" || prefix === null || !(error instanceof ConversationUnavailable) || error.message !== "transcript_missing") throw error; + const paged = page.before !== undefined || page.since !== undefined || page.from !== undefined; + // Unpaged refreshes otherwise carry no proof of which history is on screen: older + // clients without it keep their fail-closed fallback, with no recheck to wait on. + if (page.history_id === undefined && !paged) throw error; // A read failure is not an empty conversation. Only a fresh, matching binding - // and file generation permit retaining the view; never reuse peer claims here. + // and file generation permit retaining the view; never reuse the request's earlier peer claims. try { const fresh = await sessionSnapshot(); const pane = fresh.panes.find((candidate) => candidate.pane_id === paneId); @@ -1495,13 +1499,10 @@ export async function paneConversation(paneId: string, codexHome?: string, page: throw new ConversationUnavailable("no_session_path"); } } catch (cause) { - if (cause instanceof ConversationUnavailable && (page.before !== undefined || page.since !== undefined || page.from !== undefined)) throw new HistoryChanged(); + if (cause instanceof ConversationUnavailable && paged) throw new HistoryChanged(); throw cause; } - // Unpaged refreshes otherwise carry no proof of which history is on screen. - // Older clients without it keep their fail-closed fallback behavior. if (page.history_id !== undefined && !page.history_id.startsWith(prefix)) throw new HistoryChanged(); - if (page.history_id === undefined && page.before === undefined && page.since === undefined && page.from === undefined) throw error; throw new ConversationReadPending(); } const status = snapshot.panes.find((pane) => pane.pane_id === paneId)?.agent_status; diff --git a/server/copilot.test.ts b/server/copilot.test.ts index 4c183b4a5c..20847667b0 100644 --- a/server/copilot.test.ts +++ b/server/copilot.test.ts @@ -516,6 +516,13 @@ it.skipIf(process.platform !== "linux")("does not retain the old history when th expect(shown.source).toBe("copilot-transcript"); appendFileSync(transcript(session), answer("long ".repeat(15000))); failing = true; + // A client that names no history keeps its fallback, with no recheck that herdr can fail. + const answered = await herdr.sessionSnapshot(); + snapshots.mockImplementation(async () => { throw new Error("herdr is busy"); }); + snapshots.mockResolvedValueOnce(answered); + await expect(paneConversation("own")).rejects.toMatchObject({ name: "ConversationUnavailable", message: "transcript_missing" }); + expect(switched).toBe(false); + snapshots.mockResolvedValue(answered); await expect(paneConversation("own", undefined, { history_id: shown.history_id })).rejects.toMatchObject({ name: "ConversationUnavailable" }); expect(switched).toBe(true); } finally { From b76515cd318d8ab1cb26fae3c4085d04b2e16224 Mon Sep 17 00:00:00 2001 From: devswha Date: Wed, 28 Oct 2026 09:07:31 +0900 Subject: [PATCH 7/7] test(chat): keep the Copilot session-switch case switching inside the recheck The legacy-fallback assertion left the read-failure flag set, so the next request switched sessions during its first resolution instead of the recheck, and the case no longer caught a recheck that reads the binding once. --- server/copilot.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/copilot.test.ts b/server/copilot.test.ts index 20847667b0..419d448680 100644 --- a/server/copilot.test.ts +++ b/server/copilot.test.ts @@ -523,6 +523,8 @@ it.skipIf(process.platform !== "linux")("does not retain the old history when th await expect(paneConversation("own")).rejects.toMatchObject({ name: "ConversationUnavailable", message: "transcript_missing" }); expect(switched).toBe(false); snapshots.mockResolvedValue(answered); + // the switch must land in the next request's recheck, not in its first resolution + failed = false; await expect(paneConversation("own", undefined, { history_id: shown.history_id })).rejects.toMatchObject({ name: "ConversationUnavailable" }); expect(switched).toBe(true); } finally {