Skip to content

Commit 7159c95

Browse files
MukundaKattavincentkoc
MukundaKatta
authored and
vincentkoc
committed
fix(webchat): suppress stale active session rows
1 parent caac973 commit 7159c95

4 files changed

Lines changed: 274 additions & 1 deletion

File tree

‎ui/src/ui/chat/run-lifecycle.test.ts‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,193 @@
1+
import { describe, expect, it } from "vitest";
2+
import { isSessionRunActive } from "../session-run-state.ts";
3+
import type { SessionsListResult } from "../types.ts";
4+
import {
5+
reconcileChatRunFromCurrentSessionRow,
6+
reconcileChatRunLifecycle,
7+
STALE_ACTIVE_ROW_RECONCILE_WINDOW_MS,
8+
} from "./run-lifecycle.ts";
9+
10+
type ReconcileHost = Parameters<typeof reconcileChatRunFromCurrentSessionRow>[0];
11+
type TestRow = { key: string; hasActiveRun?: boolean; status?: string; startedAt?: number };
12+
13+
function makeSessionsResult(rows: TestRow[]): SessionsListResult {
14+
return { sessions: rows } as unknown as SessionsListResult;
15+
}
16+
17+
function makeHost(over: Partial<ReconcileHost> = {}): ReconcileHost {
18+
return {
19+
sessionKey: "s1",
20+
chatRunId: null,
21+
chatStream: null,
22+
sessionsResult: makeSessionsResult([{ key: "s1", hasActiveRun: true, status: "running" }]),
23+
requestUpdate: () => {},
24+
...over,
25+
};
26+
}
27+
28+
function rowActive(host: ReconcileHost): boolean {
29+
const row = host.sessionsResult?.sessions.find((r) => r.key === host.sessionKey);
30+
return Boolean(row && isSessionRunActive(row));
31+
}
32+
33+
describe("reconcileChatRunFromCurrentSessionRow stale-active suppression (#87875)", () => {
34+
it("suppresses a stale active row after a recent local completion", () => {
35+
const host = makeHost({
36+
lastLocalTerminalReconcile: {
37+
sessionKey: "s1",
38+
runId: "r1",
39+
phase: "done",
40+
sessionStatus: "done",
41+
occurredAt: Date.now(),
42+
},
43+
});
44+
expect(reconcileChatRunFromCurrentSessionRow(host)).toBe(true);
45+
expect(rowActive(host)).toBe(false);
46+
expect(host.lastLocalTerminalReconcile?.runId).toBe("r1");
47+
});
48+
49+
it("does NOT clear a genuinely recovered active run with no recent local completion", () => {
50+
const host = makeHost({ lastLocalTerminalReconcile: null });
51+
expect(reconcileChatRunFromCurrentSessionRow(host)).toBe(false);
52+
expect(rowActive(host)).toBe(true);
53+
});
54+
55+
it("ignores and clears a local terminal reconcile older than the window", () => {
56+
const host = makeHost({
57+
lastLocalTerminalReconcile: {
58+
sessionKey: "s1",
59+
runId: "r1",
60+
phase: "done",
61+
sessionStatus: "done",
62+
occurredAt: Date.now() - STALE_ACTIVE_ROW_RECONCILE_WINDOW_MS - 1_000,
63+
},
64+
});
65+
expect(reconcileChatRunFromCurrentSessionRow(host)).toBe(false);
66+
expect(rowActive(host)).toBe(true);
67+
expect(host.lastLocalTerminalReconcile).toBeNull();
68+
});
69+
70+
it("does not suppress when the recent completion was for a different session", () => {
71+
const host = makeHost({
72+
sessionKey: "s2",
73+
sessionsResult: makeSessionsResult([{ key: "s2", hasActiveRun: true, status: "running" }]),
74+
lastLocalTerminalReconcile: {
75+
sessionKey: "s1",
76+
runId: "r1",
77+
phase: "done",
78+
sessionStatus: "done",
79+
occurredAt: Date.now(),
80+
},
81+
});
82+
expect(reconcileChatRunFromCurrentSessionRow(host)).toBe(false);
83+
expect(rowActive(host)).toBe(true);
84+
});
85+
86+
it("clears the flag once the server poll reports a non-active row", () => {
87+
const host = makeHost({
88+
sessionsResult: makeSessionsResult([{ key: "s1", hasActiveRun: false, status: "done" }]),
89+
lastLocalTerminalReconcile: {
90+
sessionKey: "s1",
91+
runId: "r1",
92+
phase: "done",
93+
sessionStatus: "done",
94+
occurredAt: Date.now(),
95+
},
96+
});
97+
expect(reconcileChatRunFromCurrentSessionRow(host)).toBe(false);
98+
expect(host.lastLocalTerminalReconcile).toBeNull();
99+
});
100+
101+
it("does not arm stale-row suppression from generic lifecycle cleanup", () => {
102+
const host = makeHost({
103+
chatRunId: "orphaned-run",
104+
chatStream: "stale stream",
105+
});
106+
reconcileChatRunLifecycle(host, {
107+
outcome: "interrupted",
108+
sessionStatus: "killed",
109+
runId: "orphaned-run",
110+
sessionKey: "s1",
111+
clearLocalRun: true,
112+
clearChatStream: true,
113+
publishRunStatus: false,
114+
});
115+
expect(host.lastLocalTerminalReconcile ?? null).toBeNull();
116+
host.sessionsResult = makeSessionsResult([
117+
{ key: "s1", hasActiveRun: true, status: "running" },
118+
]);
119+
expect(reconcileChatRunFromCurrentSessionRow(host)).toBe(false);
120+
expect(rowActive(host)).toBe(true);
121+
});
122+
123+
it("does not suppress a newer active row after a follow-up run starts", () => {
124+
const terminalAt = Date.now();
125+
const host = makeHost({
126+
sessionsResult: makeSessionsResult([
127+
{
128+
key: "s1",
129+
hasActiveRun: true,
130+
status: "running",
131+
startedAt: terminalAt + 1,
132+
},
133+
]),
134+
lastLocalTerminalReconcile: {
135+
sessionKey: "s1",
136+
runId: "r1",
137+
phase: "done",
138+
sessionStatus: "done",
139+
occurredAt: terminalAt,
140+
},
141+
});
142+
expect(reconcileChatRunFromCurrentSessionRow(host)).toBe(false);
143+
expect(rowActive(host)).toBe(true);
144+
expect(host.lastLocalTerminalReconcile).toBeNull();
145+
});
146+
147+
it("arms suppression on a completed turn, then suppresses the racing refresh", () => {
148+
const host = makeHost({
149+
chatRunId: "r1",
150+
chatStream: "partial...",
151+
sessionsResult: makeSessionsResult([{ key: "s1", hasActiveRun: true, status: "running" }]),
152+
});
153+
reconcileChatRunLifecycle(host, {
154+
outcome: "done",
155+
sessionStatus: "done",
156+
runId: "r1",
157+
sessionKey: "s1",
158+
clearLocalRun: true,
159+
clearChatStream: true,
160+
publishRunStatus: false,
161+
armLocalTerminalReconcile: true,
162+
});
163+
expect(host.lastLocalTerminalReconcile?.sessionKey).toBe("s1");
164+
expect(host.chatRunId ?? null).toBeNull();
165+
// A racing sessions.list refresh re-introduces a stale active row.
166+
host.sessionsResult = makeSessionsResult([
167+
{ key: "s1", hasActiveRun: true, status: "running" },
168+
]);
169+
expect(reconcileChatRunFromCurrentSessionRow(host)).toBe(true);
170+
expect(rowActive(host)).toBe(false);
171+
expect(host.lastLocalTerminalReconcile?.runId).toBe("r1");
172+
});
173+
174+
it("keeps suppressing multiple stale active refreshes within the window", () => {
175+
const terminalAt = Date.now();
176+
const host = makeHost({
177+
lastLocalTerminalReconcile: {
178+
sessionKey: "s1",
179+
runId: "r1",
180+
phase: "done",
181+
sessionStatus: "done",
182+
occurredAt: terminalAt,
183+
},
184+
});
185+
186+
expect(reconcileChatRunFromCurrentSessionRow(host)).toBe(true);
187+
host.sessionsResult = makeSessionsResult([
188+
{ key: "s1", hasActiveRun: true, status: "running", startedAt: terminalAt - 1 },
189+
]);
190+
expect(reconcileChatRunFromCurrentSessionRow(host)).toBe(true);
191+
expect(rowActive(host)).toBe(false);
192+
});
193+
});

‎ui/src/ui/chat/run-lifecycle.ts‎

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,20 @@ export type ChatRunUiStatus = {
1111
occurredAt: number;
1212
};
1313

14+
export type LocalTerminalReconcile = {
15+
sessionKey: string;
16+
runId: string | null;
17+
phase: ChatRunUiStatus["phase"];
18+
sessionStatus: SessionRunStatus;
19+
occurredAt: number;
20+
};
21+
22+
// A terminal chat event clears local run state before the periodic
23+
// sessions.list poll catches up. Within this window a stale "active" row for
24+
// the just-completed selected session is treated as poll lag and reconciled
25+
// back to terminal, so the composer does not snap back to in-progress. (#87875)
26+
export const STALE_ACTIVE_ROW_RECONCILE_WINDOW_MS = 10_000;
27+
1428
type TimerHandle = ReturnType<typeof globalThis.setTimeout>;
1529

1630
type RunLifecycleHost = Omit<Partial<Parameters<typeof resetToolStream>[0]>, "hello"> & {
@@ -26,6 +40,7 @@ type RunLifecycleHost = Omit<Partial<Parameters<typeof resetToolStream>[0]>, "he
2640
chatRunStatus?: ChatRunUiStatus | null;
2741
chatRunStatusClearTimer?: TimerHandle | number | null;
2842
sessionsResult?: SessionsListResult | null;
43+
lastLocalTerminalReconcile?: LocalTerminalReconcile | null;
2944
requestUpdate?: () => void;
3045
};
3146

@@ -42,6 +57,7 @@ type ReconcileOptions = {
4257
clearSideResultTerminalRuns?: boolean;
4358
clearRunStatus?: boolean;
4459
publishRunStatus?: boolean;
60+
armLocalTerminalReconcile?: boolean;
4561
};
4662

4763
function toSessionKey(value: string | null | undefined): string | null {
@@ -184,6 +200,15 @@ export function reconcileChatRunLifecycle(host: RunLifecycleHost, options: Recon
184200
occurredAt,
185201
};
186202
reconcileSessionRows(host, options, occurredAt);
203+
if (options.armLocalTerminalReconcile) {
204+
host.lastLocalTerminalReconcile = {
205+
sessionKey,
206+
runId,
207+
phase: options.outcome,
208+
sessionStatus: options.sessionStatus ?? (options.outcome === "done" ? "done" : "killed"),
209+
occurredAt,
210+
};
211+
}
187212
if (options.publishRunStatus !== false) {
188213
host.chatRunStatus = status;
189214
scheduleRunStatusClear(host, status);
@@ -198,12 +223,48 @@ function currentSessionRow(host: RunLifecycleHost) {
198223
return host.sessionsResult?.sessions.find((row) => row.key === host.sessionKey);
199224
}
200225

226+
// After a terminal chat event clears local run state, a racing sessions.list
227+
// refresh can still carry a stale "active" row for the session we just
228+
// finished, which would drive the composer back to in-progress. Re-apply
229+
// terminal to that row — but only while we hold a recent LOCAL terminal
230+
// reconcile for the currently selected session, so a genuinely recovered
231+
// active run (e.g. opening WebChat to a session already running elsewhere) is
232+
// never cleared. (#87875)
233+
function reconcileStaleSelectedSessionRunAfterLocalCompletion(host: RunLifecycleHost): boolean {
234+
const recent = host.lastLocalTerminalReconcile;
235+
if (!recent || recent.sessionKey !== host.sessionKey) {
236+
return false;
237+
}
238+
if (Date.now() - recent.occurredAt > STALE_ACTIVE_ROW_RECONCILE_WINDOW_MS) {
239+
host.lastLocalTerminalReconcile = null;
240+
return false;
241+
}
242+
const row = currentSessionRow(host);
243+
if (!row || !isSessionRunActive(row)) {
244+
// No row, or the server already reflects a non-active state — the poll has
245+
// caught up, so stop suppressing.
246+
host.lastLocalTerminalReconcile = null;
247+
return false;
248+
}
249+
if (typeof row.startedAt === "number" && row.startedAt > recent.occurredAt) {
250+
host.lastLocalTerminalReconcile = null;
251+
return false;
252+
}
253+
reconcileSessionRows(
254+
host,
255+
{ outcome: recent.phase, sessionStatus: recent.sessionStatus, sessionKey: recent.sessionKey },
256+
Date.now(),
257+
);
258+
host.requestUpdate?.();
259+
return true;
260+
}
261+
201262
export function reconcileChatRunFromCurrentSessionRow(
202263
host: RunLifecycleHost,
203264
options: { publishRunStatus?: boolean } = {},
204265
): boolean {
205266
if (!host.chatRunId && host.chatStream == null) {
206-
return false;
267+
return reconcileStaleSelectedSessionRunAfterLocalCompletion(host);
207268
}
208269
const row = currentSessionRow(host);
209270
if (!row) {

‎ui/src/ui/controllers/chat.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,24 @@ describe("handleChatEvent", () => {
114114
expect(handleChatEvent(state, payload)).toBe(null);
115115
});
116116

117+
it("does not arm stale active-row suppression for an unowned selected-session final", () => {
118+
const state = createState({ sessionKey: "main" }) as ChatState & {
119+
lastLocalTerminalReconcile?: unknown;
120+
};
121+
const payload: ChatEventPayload = {
122+
runId: "observed-run",
123+
sessionKey: "main",
124+
state: "final",
125+
message: {
126+
role: "assistant",
127+
content: [{ type: "text", text: "Observed reply" }],
128+
},
129+
};
130+
131+
expect(handleChatEvent(state, payload)).toBe("final");
132+
expect(state.lastLocalTerminalReconcile).toBeUndefined();
133+
});
134+
117135
it("ignores selected-agent global events for another agent", () => {
118136
const state = createState({
119137
sessionKey: "global",

‎ui/src/ui/controllers/chat.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -881,6 +881,7 @@ export function handleChatEvent(state: ChatState, payload?: ChatEventPayload) {
881881
sessionKeys: sessionMatches ? [state.sessionKey, payload.sessionKey] : [],
882882
clearLocalRun: true,
883883
clearChatStream: true,
884+
armLocalTerminalReconcile: hadActiveRunBeforeEvent && activeRunMatches,
884885
});
885886

886887
if (payload.state === "delta") {

0 commit comments

Comments
 (0)