Fix review issues from Claude's code review
- Default taskSupport fallback to "forbidden" per MCP spec (was "optional") - Fix import path to use @/ alias consistently - Fix misleading "type guard" comment on getTaskSupport - Extract getTaskSupport to a variable to avoid duplicate calls in render - Export ExtendedTool and import in tests instead of duplicating Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
4bcffe456b
commit
f64a7ea6e3
2 changed files with 9 additions and 14 deletions
|
|
@ -51,12 +51,12 @@ import {
|
||||||
hasValidMetaName,
|
hasValidMetaName,
|
||||||
hasValidMetaPrefix,
|
hasValidMetaPrefix,
|
||||||
isReservedMetaKey,
|
isReservedMetaKey,
|
||||||
} from "../utils/metaUtils";
|
} from "@/utils/metaUtils";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Extended Tool type that includes optional fields used by the inspector.
|
* Extended Tool type that includes optional fields used by the inspector.
|
||||||
*/
|
*/
|
||||||
interface ExtendedTool extends Tool, WithIcons {
|
export interface ExtendedTool extends Tool, WithIcons {
|
||||||
_meta?: Record<string, unknown>;
|
_meta?: Record<string, unknown>;
|
||||||
execution?: {
|
execution?: {
|
||||||
taskSupport?: "forbidden" | "required" | "optional";
|
taskSupport?: "forbidden" | "required" | "optional";
|
||||||
|
|
@ -69,7 +69,7 @@ const hasMeta = (
|
||||||
): tool is ExtendedTool & { _meta: Record<string, unknown> } =>
|
): tool is ExtendedTool & { _meta: Record<string, unknown> } =>
|
||||||
typeof (tool as ExtendedTool)._meta !== "undefined";
|
typeof (tool as ExtendedTool)._meta !== "undefined";
|
||||||
|
|
||||||
// Type guard to detect execution.taskSupport
|
// Returns the execution.taskSupport value for a tool, defaulting to "forbidden" per MCP spec
|
||||||
const getTaskSupport = (
|
const getTaskSupport = (
|
||||||
tool: Tool | null,
|
tool: Tool | null,
|
||||||
): "forbidden" | "required" | "optional" => {
|
): "forbidden" | "required" | "optional" => {
|
||||||
|
|
@ -83,7 +83,7 @@ const getTaskSupport = (
|
||||||
) {
|
) {
|
||||||
return taskSupport;
|
return taskSupport;
|
||||||
}
|
}
|
||||||
return "optional";
|
return "forbidden";
|
||||||
};
|
};
|
||||||
|
|
||||||
// Type guard to safely detect the optional annotations field
|
// Type guard to safely detect the optional annotations field
|
||||||
|
|
@ -264,6 +264,8 @@ const ToolsTab = ({
|
||||||
return trimmedKey !== "" && !hasValidMetaName(trimmedKey);
|
return trimmedKey !== "" && !hasValidMetaName(trimmedKey);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const taskSupport = getTaskSupport(selectedTool);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<TabsContent value="tools">
|
<TabsContent value="tools">
|
||||||
<div className="grid grid-cols-2 gap-4">
|
<div className="grid grid-cols-2 gap-4">
|
||||||
|
|
@ -777,7 +779,7 @@ const ToolsTab = ({
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
{getTaskSupport(selectedTool) !== "forbidden" && (
|
{taskSupport !== "forbidden" && (
|
||||||
<div className="flex items-center space-x-2">
|
<div className="flex items-center space-x-2">
|
||||||
<Checkbox
|
<Checkbox
|
||||||
id="run-as-task"
|
id="run-as-task"
|
||||||
|
|
@ -785,7 +787,7 @@ const ToolsTab = ({
|
||||||
onCheckedChange={(checked: boolean) =>
|
onCheckedChange={(checked: boolean) =>
|
||||||
setRunAsTask(checked)
|
setRunAsTask(checked)
|
||||||
}
|
}
|
||||||
disabled={getTaskSupport(selectedTool) === "required"}
|
disabled={taskSupport === "required"}
|
||||||
/>
|
/>
|
||||||
<Label
|
<Label
|
||||||
htmlFor="run-as-task"
|
htmlFor="run-as-task"
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,7 @@
|
||||||
import { render, screen, fireEvent, act } from "@testing-library/react";
|
import { render, screen, fireEvent, act } from "@testing-library/react";
|
||||||
import "@testing-library/jest-dom";
|
import "@testing-library/jest-dom";
|
||||||
import { describe, it, jest, beforeEach } from "@jest/globals";
|
import { describe, it, jest, beforeEach } from "@jest/globals";
|
||||||
import ToolsTab from "../ToolsTab";
|
import ToolsTab, { ExtendedTool } from "../ToolsTab";
|
||||||
import { Tool } from "@modelcontextprotocol/sdk/types.js";
|
import { Tool } from "@modelcontextprotocol/sdk/types.js";
|
||||||
import { Tabs } from "../ui/tabs";
|
import { Tabs } from "../ui/tabs";
|
||||||
import { cacheToolOutputSchemas } from "../../utils/schemaUtils";
|
import { cacheToolOutputSchemas } from "../../utils/schemaUtils";
|
||||||
|
|
@ -12,13 +12,6 @@ import {
|
||||||
RESERVED_NAMESPACE_MESSAGE,
|
RESERVED_NAMESPACE_MESSAGE,
|
||||||
} from "../../utils/metaUtils";
|
} from "../../utils/metaUtils";
|
||||||
|
|
||||||
interface ExtendedTool extends Tool {
|
|
||||||
_meta?: Record<string, unknown>;
|
|
||||||
execution?: {
|
|
||||||
taskSupport?: "forbidden" | "required" | "optional";
|
|
||||||
};
|
|
||||||
}
|
|
||||||
|
|
||||||
describe("ToolsTab", () => {
|
describe("ToolsTab", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
// Clear the output schema cache before each test
|
// Clear the output schema cache before each test
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue