Skip to content

Commit d89d50f

Browse files
committed
Changed "Run as Task" to use Switch instead of Checkbox (and use disabled state for required/forbidden).
1 parent f5a7a69 commit d89d50f

3 files changed

Lines changed: 59 additions & 48 deletions

File tree

docs/inspector-client-todo.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,16 @@ Better forms (test tool, etc)
116116
- Use shared functionality from core forms support
117117
- Functionality (data types, arrays, arrays of objects, etc)
118118
- UX (cleaner, maybe ditch ink-forms, see if it can be styled better?)
119+
120+
### Server port in-use behavior / bug
121+
122+
- In dev mode (in v1 and v1.5) if the attempted (specified or default) port is in use, Vite will find the next available port and use that
123+
- But we will still report that it is listening on the attempted port and will open a browser window at that URL (bug)
124+
- In prod mode, if the attempted (specified or default) port is in use, the server fails to run
125+
126+
- We should decided what the port-in-use behavior should be and make it consistent between dev and prod - proposed:
127+
- If port is specified via param or env var, only use that port (if in use, fail to start)
128+
- If default port (6274), use first available starting at that value
129+
130+
- This will require that we run Vite via API instead of spawning it (so we can get the resulting port)
131+
- It will also require progressive port listening logic for prod to mimic that behavior

web/src/components/ToolsTab.tsx

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Button } from "@/components/ui/button";
33
import { Checkbox } from "@/components/ui/checkbox";
44
import { Input } from "@/components/ui/input";
55
import { Label } from "@/components/ui/label";
6+
import { Switch } from "@/components/ui/switch";
67
import { TabsContent } from "@/components/ui/tabs";
78
import { Textarea } from "@/components/ui/textarea";
89
import {
@@ -675,31 +676,26 @@ const ToolsTab = ({
675676
<div className="flex items-center space-x-2">
676677
{selectedTool && serverSupportsTaskToolCalls && (
677678
<>
678-
{taskSupport === "forbidden" ? (
679-
<span className="text-sm font-medium text-gray-700 dark:text-gray-300">
680-
Run as Task: Forbidden
681-
</span>
682-
) : taskSupport === "required" ? (
683-
<span className="text-sm font-medium text-gray-700 dark:text-gray-300">
684-
Run as Task: Required
685-
</span>
686-
) : (
687-
<>
688-
<Checkbox
689-
id="run-as-task"
690-
checked={runAsTask}
691-
onCheckedChange={(checked) =>
692-
setRunAsTask(checked === true)
693-
}
694-
/>
695-
<Label
696-
htmlFor="run-as-task"
697-
className="text-sm font-medium text-gray-700 dark:text-gray-300 cursor-pointer"
698-
>
699-
Run as task
700-
</Label>
701-
</>
702-
)}
679+
<Switch
680+
id="run-as-task"
681+
checked={effectiveRunAsTask}
682+
disabled={
683+
taskSupport === "forbidden" ||
684+
taskSupport === "required"
685+
}
686+
onCheckedChange={(checked) =>
687+
setRunAsTask(checked === true)
688+
}
689+
/>
690+
<Label
691+
htmlFor="run-as-task"
692+
className={cn(
693+
"text-sm font-medium text-gray-700 dark:text-gray-300",
694+
taskSupport === "optional" && "cursor-pointer",
695+
)}
696+
>
697+
Run as task
698+
</Label>
703699
</>
704700
)}
705701
<Button

web/src/components/__tests__/ToolsTab.test.tsx

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -177,14 +177,12 @@ describe("ToolsTab", () => {
177177
execution: { taskSupport: "required" },
178178
};
179179

180-
it("should not show Run as task checkbox when server does not support task tool calls", () => {
180+
it("should not show Run as task switch when server does not support task tool calls", () => {
181181
renderToolsTab({
182182
selectedTool: mockTools[0],
183183
serverSupportsTaskToolCalls: false,
184184
});
185-
expect(
186-
screen.queryByRole("checkbox", { name: /run as task/i }),
187-
).toBeNull();
185+
expect(screen.queryByRole("switch", { name: /run as task/i })).toBeNull();
188186
});
189187

190188
it("should show Run as task control when server supports task tool calls and a tool is selected", () => {
@@ -193,56 +191,60 @@ describe("ToolsTab", () => {
193191
serverSupportsTaskToolCalls: true,
194192
});
195193
expect(
196-
screen.getByRole("checkbox", { name: /run as task/i }),
194+
screen.getByRole("switch", { name: /run as task/i }),
197195
).toBeInTheDocument();
198196
});
199197

200-
it("should show checkbox unchecked and enabled when tool taskSupport is optional", async () => {
198+
it("should show switch unchecked and enabled when tool taskSupport is optional", async () => {
201199
renderToolsTab({
202200
selectedTool: toolWithOptionalTask,
203201
serverSupportsTaskToolCalls: true,
204202
});
205-
const checkbox = screen.getByRole("checkbox", { name: /run as task/i });
206-
expect(checkbox).not.toBeChecked();
207-
expect(checkbox).not.toBeDisabled();
203+
const runAsTaskSwitch = screen.getByRole("switch", {
204+
name: /run as task/i,
205+
});
206+
expect(runAsTaskSwitch).not.toBeChecked();
207+
expect(runAsTaskSwitch).not.toBeDisabled();
208208
});
209209

210-
it("should show Run as Task: Forbidden when tool taskSupport is forbidden", () => {
210+
it("should show switch off and disabled when tool taskSupport is forbidden", () => {
211211
renderToolsTab({
212212
selectedTool: toolWithForbiddenTask,
213213
serverSupportsTaskToolCalls: true,
214214
});
215-
expect(screen.getByText(/run as task: forbidden/i)).toBeInTheDocument();
216-
expect(
217-
screen.queryByRole("checkbox", { name: /run as task/i }),
218-
).toBeNull();
215+
const runAsTaskSwitch = screen.getByRole("switch", {
216+
name: /run as task/i,
217+
});
218+
expect(runAsTaskSwitch).not.toBeChecked();
219+
expect(runAsTaskSwitch).toBeDisabled();
219220
});
220221

221-
it("should show Run as Task: Required when tool taskSupport is required", () => {
222+
it("should show switch on and disabled when tool taskSupport is required", () => {
222223
renderToolsTab({
223224
selectedTool: toolWithRequiredTask,
224225
serverSupportsTaskToolCalls: true,
225226
});
226-
expect(screen.getByText(/run as task: required/i)).toBeInTheDocument();
227-
expect(
228-
screen.queryByRole("checkbox", { name: /run as task/i }),
229-
).toBeNull();
227+
const runAsTaskSwitch = screen.getByRole("switch", {
228+
name: /run as task/i,
229+
});
230+
expect(runAsTaskSwitch).toBeChecked();
231+
expect(runAsTaskSwitch).toBeDisabled();
230232
});
231233

232-
it("should call callTool with runAsTask true when optional and checkbox is checked and Run Tool clicked", async () => {
234+
it("should call callTool with runAsTask true when optional and switch is on and Run Tool clicked", async () => {
233235
const mockCallTool = vi.fn(async () => {});
234236
renderToolsTab({
235237
selectedTool: toolWithOptionalTask,
236238
serverSupportsTaskToolCalls: true,
237239
callTool: mockCallTool,
238240
});
239-
const runAsTaskCheckbox = screen.getByRole("checkbox", {
241+
const runAsTaskSwitch = screen.getByRole("switch", {
240242
name: /run as task/i,
241243
});
242244
await act(async () => {
243-
fireEvent.click(runAsTaskCheckbox);
245+
fireEvent.click(runAsTaskSwitch);
244246
});
245-
expect(runAsTaskCheckbox).toBeChecked();
247+
expect(runAsTaskSwitch).toBeChecked();
246248
const runButton = screen.getByRole("button", { name: /run tool/i });
247249
await act(async () => {
248250
fireEvent.click(runButton);

0 commit comments

Comments
 (0)