Skip to content

Commit b96c04c

Browse files
committed
Fix agent stop action and throttle duplicate settings saves
1 parent 6e67d9b commit b96c04c

8 files changed

Lines changed: 276 additions & 41 deletions

File tree

packages/web/src/features/agent-panes/components/session-card.test.tsx

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ const mockXtermHost = vi.fn((props: Record<string, unknown>) => (
1616
<div data-testid="mock-xterm-host" data-readonly={String(props.readOnly)} />
1717
));
1818

19+
function getLastXtermHostProps() {
20+
const lastCall = mockXtermHost.mock.calls[mockXtermHost.mock.calls.length - 1];
21+
return lastCall?.[0];
22+
}
23+
1924
vi.mock("../../terminal-panel/views/shared/xterm-host", () => ({
2025
XtermHost: (props: Record<string, unknown>) => mockXtermHost(props),
2126
}));
@@ -105,7 +110,7 @@ describe("SessionCard", () => {
105110

106111
expect(screen.queryByRole("textbox")).not.toBeInTheDocument();
107112
expect(screen.queryByRole("button", { name: "Start" })).not.toBeInTheDocument();
108-
expect(mockXtermHost.mock.calls.at(-1)?.[0]).toEqual(
113+
expect(getLastXtermHostProps()).toEqual(
109114
expect.objectContaining({
110115
terminalId: "term-live",
111116
readOnly: false,
@@ -149,7 +154,7 @@ describe("SessionCard", () => {
149154
</Provider>
150155
);
151156

152-
expect(mockXtermHost.mock.calls.at(-1)?.[0]).toEqual(
157+
expect(getLastXtermHostProps()).toEqual(
153158
expect.objectContaining({
154159
terminalId: "term-live",
155160
isActiveSession: true,
@@ -308,7 +313,7 @@ describe("SessionCard", () => {
308313
</Provider>
309314
);
310315

311-
expect(mockXtermHost.mock.calls.at(-1)?.[0]).toEqual(
316+
expect(getLastXtermHostProps()).toEqual(
312317
expect.objectContaining({
313318
terminalId: "term-live",
314319
readOnly: true,
@@ -460,7 +465,7 @@ describe("SessionCard", () => {
460465
</Provider>
461466
);
462467

463-
expect(mockXtermHost.mock.calls.at(-1)?.[0]).toEqual(
468+
expect(getLastXtermHostProps()).toEqual(
464469
expect.objectContaining({
465470
terminalId: "term-ended",
466471
readOnly: true,
@@ -509,6 +514,25 @@ describe("SessionCard", () => {
509514
);
510515
});
511516

517+
it("renders stop for running sessions and routes it through the explicit callback", () => {
518+
const { store } = createSessionStore({
519+
terminalId: "term-live",
520+
state: "running",
521+
endedAt: undefined,
522+
});
523+
const onStop = vi.fn();
524+
525+
render(
526+
<Provider store={store}>
527+
<SessionCard sessionId="sess_123456" onStop={onStop} />
528+
</Provider>
529+
);
530+
531+
fireEvent.click(screen.getByRole("button", { name: "Stop" }));
532+
533+
expect(onStop).toHaveBeenCalledTimes(1);
534+
});
535+
512536
it("routes split buttons through explicit callbacks", () => {
513537
const { store } = createSessionStore({
514538
terminalId: "term-live",
@@ -548,7 +572,12 @@ describe("SessionCard", () => {
548572
</Provider>
549573
);
550574

551-
expect(screen.queryByRole("button", { name: "Stop" })).not.toBeInTheDocument();
575+
expect(screen.getByRole("button", { name: "Stop" })).toHaveClass(
576+
"btn",
577+
"btn-ghost",
578+
"btn-sm",
579+
"session-action-btn"
580+
);
552581
expect(screen.getByRole("button", { name: "Split horizontal" })).toHaveClass(
553582
"btn",
554583
"btn-ghost",

packages/web/src/features/agent-panes/index.test.tsx

Lines changed: 52 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { Session } from "@coder-studio/core";
12
import { act, fireEvent, render, screen, waitFor } from "@testing-library/react";
23
import { createStore, Provider } from "jotai";
34
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
@@ -9,20 +10,21 @@ import { seedReadyWorkspaceState } from "../../test-utils/workspace-state";
910
import { LEGACY_PANE_LAYOUT_STORAGE_KEY_PREFIX, paneLayoutAtomFamily } from "./atoms/pane-layout";
1011
import { AgentPanes } from "./index";
1112

13+
type MockSessionCardProps = {
14+
sessionId: string;
15+
onStop?: () => void;
16+
onSplitHorizontal?: () => void;
17+
onSplitVertical?: () => void;
18+
onClose?: () => void;
19+
};
20+
1221
const mockSessionCard = vi.fn(
13-
({
14-
sessionId,
15-
onSplitHorizontal,
16-
onSplitVertical,
17-
onClose,
18-
}: {
19-
sessionId: string;
20-
onSplitHorizontal?: () => void;
21-
onSplitVertical?: () => void;
22-
onClose?: () => void;
23-
}) => (
22+
({ sessionId, onStop, onSplitHorizontal, onSplitVertical, onClose }: MockSessionCardProps) => (
2423
<div data-testid="session-card">
2524
<span>{sessionId}</span>
25+
<button type="button" onClick={onStop}>
26+
stop-{sessionId}
27+
</button>
2628
<button type="button" onClick={onSplitHorizontal}>
2729
split-{sessionId}
2830
</button>
@@ -37,7 +39,7 @@ const mockSessionCard = vi.fn(
3739
);
3840

3941
vi.mock("./views/shared/session-card", () => ({
40-
SessionCard: (props: Record<string, unknown>) => mockSessionCard(props),
42+
SessionCard: (props: MockSessionCardProps) => mockSessionCard(props),
4143
}));
4244

4345
vi.mock("./views/shared/pane-layout", () => ({
@@ -72,7 +74,7 @@ function createAgentPaneStore(
7274
| "rejected" = "connected"
7375
) {
7476
const store = createStore();
75-
const sessions = [
77+
const sessions: Session[] = [
7678
{
7779
id: "sess_1",
7880
workspaceId: "ws-1",
@@ -125,7 +127,10 @@ function createAgentPaneStore(
125127
},
126128
},
127129
});
128-
store.set(sessionsAtom, Object.fromEntries(sessions.map((session) => [session.id, session])));
130+
store.set(
131+
sessionsAtom,
132+
Object.fromEntries(sessions.map((session) => [session.id, session])) as Record<string, Session>
133+
);
129134
store.set(
130135
paneLayoutAtomFamily("ws-1"),
131136
(initialLayout as never) ?? {
@@ -339,6 +344,39 @@ describe("AgentPanes", () => {
339344
});
340345
});
341346

347+
it("stops a running session without removing its pane", async () => {
348+
const { store, sendCommand } = createAgentPaneStore();
349+
350+
render(
351+
<Provider store={store}>
352+
<AgentPanes />
353+
</Provider>
354+
);
355+
356+
await waitFor(() => {
357+
expect(store.get(paneLayoutAtomFamily("ws-1"))).toEqual({
358+
id: expect.any(String),
359+
type: "split",
360+
direction: "horizontal",
361+
ratio: 0.5,
362+
children: [
363+
expect.objectContaining({ type: "leaf", sessionId: "sess_1" }),
364+
expect.objectContaining({ type: "leaf", sessionId: "sess_2" }),
365+
],
366+
});
367+
});
368+
369+
const layoutBeforeStop = structuredClone(store.get(paneLayoutAtomFamily("ws-1")));
370+
371+
fireEvent.click(screen.getByRole("button", { name: "stop-sess_1" }));
372+
373+
await waitFor(() => {
374+
expect(sendCommand).toHaveBeenCalledWith("session.stop", { sessionId: "sess_1" }, undefined);
375+
});
376+
377+
expect(store.get(paneLayoutAtomFamily("ws-1"))).toEqual(layoutBeforeStop);
378+
});
379+
342380
it("waits for the websocket connection before requesting session.list", async () => {
343381
const sendCommand = vi.fn().mockResolvedValue([]);
344382
const { store } = createAgentPaneStore(undefined, sendCommand, "connecting");

packages/web/src/features/agent-panes/index.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ export const AgentPanes: FC<AgentPanesProps> = ({ hydrateSessions = true }) => {
8585
onAssignSession={paneActions.assignSession}
8686
onReplaceWithSession={paneActions.replaceWithSession}
8787
onCloseSessionCommand={sessionActions.closeSession}
88+
onStopSession={sessionActions.stopSession}
8889
/>
8990
</div>
9091
);
@@ -96,10 +97,11 @@ interface PaneNodeRendererProps {
9697
onAssignSession: (paneId: string, sessionId: string) => void;
9798
onCloseDraftPane: (paneId: string) => void;
9899
onCloseSession: (sessionId: string) => void;
99-
onCloseSessionCommand: (sessionId: string) => Promise<void>;
100+
onCloseSessionCommand: (sessionId: string) => Promise<boolean | void>;
100101
onReplaceWithSession: (sessionId: string) => void;
101102
onSplitDraftPane: (paneId: string, direction: "horizontal" | "vertical") => void;
102103
onSplitSession: (sessionId: string, direction: "horizontal" | "vertical") => void;
104+
onStopSession: (sessionId: string) => Promise<void>;
103105
}
104106

105107
/**
@@ -115,6 +117,7 @@ const PaneNodeRenderer: FC<PaneNodeRendererProps> = ({
115117
onReplaceWithSession,
116118
onSplitDraftPane,
117119
onSplitSession,
120+
onStopSession,
118121
}) => {
119122
if (node.type === "leaf") {
120123
// Render session card or draft launcher
@@ -128,6 +131,7 @@ const PaneNodeRenderer: FC<PaneNodeRendererProps> = ({
128131
}}
129132
onSplitHorizontal={() => onSplitSession(node.sessionId!, "horizontal")}
130133
onSplitVertical={() => onSplitSession(node.sessionId!, "vertical")}
134+
onStop={() => onStopSession(node.sessionId!)}
131135
/>
132136
);
133137
} else {
@@ -166,6 +170,7 @@ const PaneNodeRenderer: FC<PaneNodeRendererProps> = ({
166170
onReplaceWithSession={onReplaceWithSession}
167171
onSplitDraftPane={onSplitDraftPane}
168172
onSplitSession={onSplitSession}
173+
onStopSession={onStopSession}
169174
/>
170175
))}
171176
</PaneLayout>

packages/web/src/features/agent-panes/views/shared/session-card.tsx

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
import type { SessionState } from "@coder-studio/core";
99
import { useAtomValue, useSetAtom } from "jotai";
10-
import { FlipHorizontal, FlipVertical, X } from "lucide-react";
10+
import { FlipHorizontal, FlipVertical, Square, X } from "lucide-react";
1111
import type { FC, ReactNode } from "react";
1212
import { useEffect, useRef, useState } from "react";
1313
import { pendingFocusSessionAtom } from "../../../../atoms/app-ui";
@@ -33,6 +33,7 @@ interface SessionCardProps {
3333
onClose?: SessionCardAction;
3434
onSplitHorizontal?: SessionCardAction;
3535
onSplitVertical?: SessionCardAction;
36+
onStop?: SessionCardAction;
3637
}
3738

3839
/**
@@ -51,6 +52,7 @@ export const SessionCard: FC<SessionCardProps> = ({
5152
onClose,
5253
onSplitHorizontal,
5354
onSplitVertical,
55+
onStop,
5456
}) => {
5557
const session = useAtomValue(sessionByIdAtomFamily(sessionId));
5658
const workspace = useAtomValue(
@@ -150,6 +152,17 @@ export const SessionCard: FC<SessionCardProps> = ({
150152

151153
{showHeaderActions ? (
152154
<div className="session-header-actions">
155+
{session.state === "running" ? (
156+
<Tooltip content="Stop">
157+
<IconButton
158+
aria-label="Stop"
159+
className="session-action-btn"
160+
icon={<Square size={13} />}
161+
onClick={() => void onStop?.()}
162+
size="sm"
163+
/>
164+
</Tooltip>
165+
) : null}
153166
<Tooltip content="Split horizontal">
154167
<IconButton
155168
aria-label="Split horizontal"

packages/web/src/features/settings/components/settings-page.test.tsx

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,6 +1737,52 @@ describe("SettingsPage", () => {
17371737
});
17381738
});
17391739

1740+
it("throttles duplicate desktop terminal font-size commits triggered back-to-back", async () => {
1741+
const sendCommand = vi.fn().mockImplementation(async (op: string) => {
1742+
if (op === "settings.get") {
1743+
return {
1744+
"appearance.desktopTerminalFontSize": 11,
1745+
"appearance.mobileTerminalFontSize": 13,
1746+
};
1747+
}
1748+
return {};
1749+
});
1750+
const store = createConnectedStore(sendCommand);
1751+
1752+
renderSettingsPage(store);
1753+
fireEvent.click(screen.getByRole("button", { name: "外观" }));
1754+
1755+
const input = await screen.findByRole("spinbutton", { name: "桌面端终端字号" });
1756+
1757+
fireEvent.change(input, { target: { value: "15" } });
1758+
fireEvent.keyDown(input, { key: "Enter" });
1759+
fireEvent.blur(input);
1760+
1761+
await waitFor(() => {
1762+
expect(sendCommand).toHaveBeenCalledWith(
1763+
"settings.update",
1764+
{
1765+
settings: {
1766+
appearance: {
1767+
desktopTerminalFontSize: 15,
1768+
},
1769+
},
1770+
},
1771+
undefined
1772+
);
1773+
});
1774+
1775+
expect(
1776+
sendCommand.mock.calls.filter(
1777+
([op, args]) =>
1778+
op === "settings.update" &&
1779+
typeof args === "object" &&
1780+
args !== null &&
1781+
"settings" in args
1782+
)
1783+
).toHaveLength(1);
1784+
});
1785+
17401786
it("updates mobile terminal font size through the appearance input without changing desktop size", async () => {
17411787
const sendCommand = vi.fn().mockImplementation(async (op: string) => {
17421788
if (op === "settings.get") {

0 commit comments

Comments
 (0)