Skip to content

Commit 6e37c58

Browse files
RomneyDaRomneyDa
authored
Merge 75cbcc2 into 36d9241 
2 parents 36d9241 + 75cbcc2 commit 6e37c58

10 files changed

Lines changed: 322 additions & 40 deletions

‎src/tui/components/chat-log.test.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,15 +174,19 @@ describe("ChatLog", () => {
174174
expect(chatLog.render(120).join("\n")).toContain("queued hello");
175175
});
176176

177-
it("stops counting a pending user message once the run is committed", () => {
177+
it("re-keys a pending user in place without moving its position", () => {
178178
const chatLog = new ChatLog(40);
179179

180-
chatLog.addPendingUser("run-1", "hello");
181-
expect(chatLog.countPendingUsers()).toBe(1);
180+
chatLog.addPendingUser("local", "queued hello", 1_000);
181+
chatLog.startAssistant("hi there", "r-accepted");
182+
183+
expect(chatLog.rekeyPendingUser("local", "r-accepted")).toBe(true);
182184

183-
expect(chatLog.commitPendingUser("run-1")).toBe(true);
185+
const rendered = chatLog.render(120).join("\n");
186+
expect(rendered.indexOf("queued hello")).toBeLessThan(rendered.indexOf("hi there"));
187+
// The row is now addressable by the gateway-assigned runId.
188+
expect(chatLog.dropPendingUser("r-accepted")).toBe(true);
184189
expect(chatLog.countPendingUsers()).toBe(0);
185-
expect(chatLog.render(120).join("\n")).toContain("hello");
186190
});
187191

188192
it("reconciles pending users against rebuilt history using timestamps", () => {

‎src/tui/components/chat-log.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -198,10 +198,6 @@ export class ChatLog extends Container {
198198
return component;
199199
}
200200

201-
commitPendingUser(runId: string) {
202-
return this.pendingUsers.delete(runId);
203-
}
204-
205201
dropPendingUser(runId: string) {
206202
const existing = this.pendingUsers.get(runId);
207203
if (!existing) {
@@ -212,8 +208,20 @@ export class ChatLog extends Container {
212208
return true;
213209
}
214210

215-
hasPendingUser(runId: string) {
216-
return this.pendingUsers.has(runId);
211+
// Re-key in place: the gateway can assign its own runId after the optimistic
212+
// row is rendered. Swap the map key without re-mounting the component so the
213+
// row keeps its transcript position even if a reply already rendered below it.
214+
rekeyPendingUser(fromRunId: string, toRunId: string) {
215+
if (fromRunId === toRunId) {
216+
return false;
217+
}
218+
const existing = this.pendingUsers.get(fromRunId);
219+
if (!existing) {
220+
return false;
221+
}
222+
this.pendingUsers.delete(fromRunId);
223+
this.pendingUsers.set(toRunId, existing);
224+
return true;
217225
}
218226

219227
reconcilePendingUsers(

‎src/tui/tui-command-handlers.test.ts‎

Lines changed: 97 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ function createHarness(params?: {
9090
currentSessionKey?: string;
9191
abortActive?: AbortActiveMock;
9292
consumeCompletedRunForPendingSend?: ConsumeCompletedRunMock;
93+
isRunObserved?: (runId: string) => boolean;
9394
flushPendingHistoryRefreshIfIdle?: FlushPendingHistoryRefreshMock;
9495
}) {
9596
const sendChat = params?.sendChat ?? vi.fn().mockResolvedValue({ runId: "r1" });
@@ -103,6 +104,9 @@ function createHarness(params?: {
103104
const setEmptySession =
104105
params?.setEmptySession ?? (vi.fn().mockResolvedValue(undefined) as SetEmptySessionMock);
105106
const addUser = vi.fn();
107+
const addPendingUser = vi.fn();
108+
const dropPendingUser = vi.fn();
109+
const rekeyPendingUser = vi.fn();
106110
const addSystem = vi.fn();
107111
const clearTools = vi.fn();
108112
const reserveAssistantSlot = vi.fn();
@@ -133,6 +137,7 @@ function createHarness(params?: {
133137
activeChatRunId: params?.activeChatRunId ?? null,
134138
pendingOptimisticUserMessage: params?.pendingOptimisticUserMessage ?? false,
135139
pendingChatRunId: params?.pendingChatRunId ?? null,
140+
pendingSubmitDraft: null as { runId: string; text: string } | null,
136141
activityStatus: params?.activityStatus ?? "idle",
137142
isConnected: params?.isConnected ?? true,
138143
sessionInfo: {},
@@ -148,7 +153,15 @@ function createHarness(params?: {
148153
resetSession,
149154
runGoalCommand,
150155
} as never,
151-
chatLog: { addUser, addSystem, clearTools, reserveAssistantSlot } as never,
156+
chatLog: {
157+
addUser,
158+
addPendingUser,
159+
dropPendingUser,
160+
rekeyPendingUser,
161+
addSystem,
162+
clearTools,
163+
reserveAssistantSlot,
164+
} as never,
152165
tui: { requestRender } as never,
153166
opts: params?.opts ?? {},
154167
state: state as never,
@@ -170,6 +183,7 @@ function createHarness(params?: {
170183
forgetLocalRunId,
171184
forgetLocalBtwRunId: vi.fn(),
172185
consumeCompletedRunForPendingSend: params?.consumeCompletedRunForPendingSend,
186+
isRunObserved: params?.isRunObserved,
173187
flushPendingHistoryRefreshIfIdle: params?.flushPendingHistoryRefreshIfIdle,
174188
runAuthFlow,
175189
requestExit,
@@ -190,6 +204,9 @@ function createHarness(params?: {
190204
setSession,
191205
setEmptySession,
192206
addUser,
207+
addPendingUser,
208+
dropPendingUser,
209+
rekeyPendingUser,
193210
addSystem,
194211
clearTools,
195212
reserveAssistantSlot,
@@ -264,19 +281,62 @@ describe("tui command handlers", () => {
264281
});
265282

266283
it("forwards unknown slash commands to the gateway", async () => {
267-
const { handleCommand, sendChat, addUser, addSystem, requestRender } = createHarness();
284+
const { handleCommand, sendChat, addPendingUser, addSystem, requestRender } = createHarness();
268285

269286
await handleCommand("/unregistered-command");
270287

271288
expect(addSystem).not.toHaveBeenCalled();
272-
expect(addUser).toHaveBeenCalledWith("/unregistered-command");
289+
expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), "/unregistered-command");
273290
expectSendChatFields(sendChat, {
274291
sessionKey: "agent:main:main",
275292
message: "/unregistered-command",
276293
});
277294
expect(requestRender).toHaveBeenCalled();
278295
});
279296

297+
it("re-keys the optimistic pending row to the gateway-accepted runId in place", async () => {
298+
const sendChat = vi.fn().mockResolvedValue({ runId: "r-accepted" });
299+
const harness = createHarness({ sendChat });
300+
301+
await harness.handleCommand("hello");
302+
303+
const localRunId = harness.addPendingUser.mock.calls[0]?.[0];
304+
expect(localRunId).toEqual(expect.any(String));
305+
expect(localRunId).not.toBe("r-accepted");
306+
// Re-key happens in place (no drop/re-add) so the row keeps its position.
307+
expect(harness.rekeyPendingUser).toHaveBeenCalledWith(localRunId, "r-accepted");
308+
expect(harness.addPendingUser).toHaveBeenCalledTimes(1);
309+
expect(harness.dropPendingUser).not.toHaveBeenCalled();
310+
expect(harness.state.pendingSubmitDraft).toEqual({ runId: "r-accepted", text: "hello" });
311+
});
312+
313+
it("does not re-arm the submit draft when the accepted run already emitted events", async () => {
314+
const sendChat = vi.fn().mockResolvedValue({ runId: "r-accepted" });
315+
const isRunObserved = vi.fn((runId: string) => runId === "r-accepted");
316+
const harness = createHarness({ sendChat, isRunObserved });
317+
318+
await harness.handleCommand("hello");
319+
320+
// The accepted run already registered, so the draft must not be re-armed —
321+
// otherwise a later abort would drop a row whose reply already rendered.
322+
expect(harness.rekeyPendingUser).toHaveBeenCalledWith(expect.any(String), "r-accepted");
323+
expect(harness.state.pendingSubmitDraft).toBeNull();
324+
});
325+
326+
it("clears the submit draft when the accepted run already completed", async () => {
327+
const sendChat = vi.fn().mockResolvedValue({ runId: "r-accepted" });
328+
const consumeCompletedRunForPendingSend = vi
329+
.fn()
330+
.mockReturnValue(true) as ConsumeCompletedRunMock;
331+
const harness = createHarness({ sendChat, consumeCompletedRunForPendingSend });
332+
333+
await harness.handleCommand("hello");
334+
335+
expect(harness.addPendingUser).toHaveBeenCalledTimes(1);
336+
expect(harness.dropPendingUser).not.toHaveBeenCalled();
337+
expect(harness.state.pendingSubmitDraft).toBeNull();
338+
});
339+
280340
it("passes the current backing session id when sending to the gateway", async () => {
281341
const { handleCommand, sendChat } = createHarness({
282342
currentSessionId: "session-before-relaunch",
@@ -293,10 +353,11 @@ describe("tui command handlers", () => {
293353

294354
it("starts local goals and sends the objective to the model", async () => {
295355
const runGoalCommand = vi.fn().mockResolvedValue({ text: "Goal started: ship" });
296-
const { handleCommand, sendChat, addSystem, refreshSessionInfo, addUser } = createHarness({
297-
opts: { local: true },
298-
runGoalCommand,
299-
});
356+
const { handleCommand, sendChat, addSystem, refreshSessionInfo, addPendingUser } =
357+
createHarness({
358+
opts: { local: true },
359+
runGoalCommand,
360+
});
300361

301362
await handleCommand("/goal start ship");
302363

@@ -309,7 +370,7 @@ describe("tui command handlers", () => {
309370
sessionKey: "agent:main:main",
310371
message: "ship",
311372
});
312-
expect(addUser).toHaveBeenCalledWith("ship");
373+
expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), "ship");
313374
expect(addSystem).toHaveBeenCalledWith("Goal started: ship");
314375
expect(refreshSessionInfo).toHaveBeenCalled();
315376
});
@@ -327,7 +388,7 @@ describe("tui command handlers", () => {
327388
sessionKey: "agent:main:main",
328389
message: slashPrompt,
329390
});
330-
expect(slashHarness.addUser).toHaveBeenCalledWith(slashPrompt);
391+
expect(slashHarness.addPendingUser).toHaveBeenCalledWith(expect.any(String), slashPrompt);
331392

332393
const bangRunGoalCommand = vi.fn().mockResolvedValue({ text: "Goal started" });
333394
const bangHarness = createHarness({
@@ -341,7 +402,7 @@ describe("tui command handlers", () => {
341402
sessionKey: "agent:main:main",
342403
message: bangPrompt,
343404
});
344-
expect(bangHarness.addUser).toHaveBeenCalledWith(bangPrompt);
405+
expect(bangHarness.addPendingUser).toHaveBeenCalledWith(expect.any(String), bangPrompt);
345406
});
346407

347408
it("keeps local goal status as a control command", async () => {
@@ -359,7 +420,7 @@ describe("tui command handlers", () => {
359420

360421
it("wraps command-prefixed local goal resume notes before sending", async () => {
361422
const runGoalCommand = vi.fn().mockResolvedValue({ text: "Goal resumed: ship" });
362-
const { handleCommand, sendChat, addUser } = createHarness({
423+
const { handleCommand, sendChat, addPendingUser } = createHarness({
363424
opts: { local: true },
364425
runGoalCommand,
365426
});
@@ -371,7 +432,7 @@ describe("tui command handlers", () => {
371432
sessionKey: "agent:main:main",
372433
message: prompt,
373434
});
374-
expect(addUser).toHaveBeenCalledWith(prompt);
435+
expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), prompt);
375436
});
376437

377438
it("passes the selected agent for local global goal commands", async () => {
@@ -468,12 +529,12 @@ describe("tui command handlers", () => {
468529
});
469530

470531
it("forwards /status to the shared gateway command path", async () => {
471-
const { handleCommand, sendChat, addUser, addSystem } = createHarness();
532+
const { handleCommand, sendChat, addPendingUser, addSystem } = createHarness();
472533

473534
await handleCommand("/status");
474535

475536
expect(addSystem).not.toHaveBeenCalled();
476-
expect(addUser).toHaveBeenCalledWith("/status");
537+
expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), "/status");
477538
expectSendChatFields(sendChat, {
478539
sessionKey: "agent:main:main",
479540
message: "/status",
@@ -608,10 +669,19 @@ describe("tui command handlers", () => {
608669

609670
it("clears the pending runId if sendChat fails", async () => {
610671
const sendChat = vi.fn().mockRejectedValue(new Error("boom"));
611-
const { handleCommand, state } = createHarness({ sendChat });
672+
const {
673+
handleCommand,
674+
sendChat: sendChatMock,
675+
dropPendingUser,
676+
state,
677+
} = createHarness({
678+
sendChat,
679+
});
612680

613681
await handleCommand("hello");
614682

683+
const sentRunId = (firstMockArg(sendChatMock, "sendChat") as { runId: string }).runId;
684+
expect(dropPendingUser).toHaveBeenCalledWith(sentRunId);
615685
expect(state.pendingChatRunId).toBeNull();
616686
expect(state.pendingOptimisticUserMessage).toBe(false);
617687
});
@@ -837,7 +907,7 @@ describe("tui command handlers", () => {
837907
const {
838908
handleCommand,
839909
sendChat,
840-
addUser,
910+
addPendingUser,
841911
addSystem,
842912
reserveAssistantSlot,
843913
requestRender,
@@ -857,9 +927,9 @@ describe("tui command handlers", () => {
857927
});
858928
expect(reserveAssistantSlot).toHaveBeenCalledWith("run-active");
859929
const reserveCallOrder = reserveAssistantSlot.mock.invocationCallOrder[0];
860-
const addUserCallOrder = addUser.mock.invocationCallOrder[0];
861-
expect(reserveCallOrder).toBeLessThan(addUserCallOrder);
862-
expect(addUser).toHaveBeenCalledWith("/context detail");
930+
const addPendingUserCallOrder = addPendingUser.mock.invocationCallOrder[0];
931+
expect(reserveCallOrder).toBeLessThan(addPendingUserCallOrder);
932+
expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), "/context detail");
863933
expect(addSystem).not.toHaveBeenCalledWith(
864934
"agent is busy — press Esc to abort before sending a new message",
865935
);
@@ -869,15 +939,15 @@ describe("tui command handlers", () => {
869939
});
870940

871941
it("blocks gateway slash prompts while a run is active", async () => {
872-
const { handleCommand, sendChat, addUser, addSystem } = createHarness({
942+
const { handleCommand, sendChat, addPendingUser, addSystem } = createHarness({
873943
activeChatRunId: "run-active",
874944
activityStatus: "streaming",
875945
});
876946

877947
await handleCommand("/context detail");
878948

879949
expect(sendChat).not.toHaveBeenCalled();
880-
expect(addUser).not.toHaveBeenCalled();
950+
expect(addPendingUser).not.toHaveBeenCalled();
881951
expect(addSystem).toHaveBeenCalledWith(
882952
"agent is busy — press Esc to abort before sending a new message",
883953
);
@@ -900,7 +970,7 @@ describe("tui command handlers", () => {
900970

901971
it("sends slash stop to the backend when there is no tracked run", async () => {
902972
const abortActive = vi.fn().mockResolvedValue(undefined);
903-
const { handleCommand, sendChat, addUser } = createHarness({ abortActive });
973+
const { handleCommand, sendChat, addPendingUser } = createHarness({ abortActive });
904974

905975
await handleCommand("/stop");
906976

@@ -910,18 +980,18 @@ describe("tui command handlers", () => {
910980
message: "/stop",
911981
sessionKey: "agent:main:main",
912982
});
913-
expect(addUser).toHaveBeenCalledWith("/stop");
983+
expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), "/stop");
914984
});
915985

916986
it("sends broad stop-like text as a normal prompt when idle", async () => {
917987
const abortActive = vi.fn().mockResolvedValue(undefined);
918-
const { handleCommand, sendChat, addUser } = createHarness({ abortActive });
988+
const { handleCommand, sendChat, addPendingUser } = createHarness({ abortActive });
919989

920990
await handleCommand("do not do that");
921991

922992
expect(abortActive).not.toHaveBeenCalled();
923993
expect(sendChat).toHaveBeenCalledTimes(1);
924-
expect(addUser).toHaveBeenCalledWith("do not do that");
994+
expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), "do not do that");
925995
});
926996

927997
it("rejects normal sends while a queued submit is pending registration", async () => {
@@ -941,7 +1011,7 @@ describe("tui command handlers", () => {
9411011
});
9421012

9431013
it("allows local sends to queue while the current run is finishing", async () => {
944-
const { handleCommand, sendChat, addUser, addSystem } = createHarness({
1014+
const { handleCommand, sendChat, addPendingUser, addSystem } = createHarness({
9451015
opts: { local: true },
9461016
activeChatRunId: "run-active",
9471017
activityStatus: "finishing context",
@@ -950,7 +1020,7 @@ describe("tui command handlers", () => {
9501020
await handleCommand("/context detail");
9511021

9521022
expect(sendChat).toHaveBeenCalledTimes(1);
953-
expect(addUser).toHaveBeenCalledWith("/context detail");
1023+
expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), "/context detail");
9541024
expect(addSystem).not.toHaveBeenCalledWith(
9551025
"agent is busy — press Esc to abort before sending a new message",
9561026
);

0 commit comments

Comments
 (0)