diff --git a/packages/types/src/__tests__/skills.test.ts b/packages/types/src/__tests__/skills.test.ts new file mode 100644 index 0000000000..c215f7cdbe --- /dev/null +++ b/packages/types/src/__tests__/skills.test.ts @@ -0,0 +1,144 @@ +import { + validateSkillName, + SkillNameValidationError, + SKILL_NAME_MIN_LENGTH, + SKILL_NAME_MAX_LENGTH, + SKILL_NAME_REGEX, +} from "../skills.js" + +describe("validateSkillName", () => { + describe("valid names", () => { + it("accepts single lowercase word", () => { + expect(validateSkillName("myskill")).toEqual({ valid: true }) + }) + + it("accepts lowercase letters and numbers", () => { + expect(validateSkillName("skill123")).toEqual({ valid: true }) + }) + + it("accepts hyphenated words", () => { + expect(validateSkillName("my-skill")).toEqual({ valid: true }) + }) + + it("accepts multiple hyphenated words", () => { + expect(validateSkillName("my-awesome-skill")).toEqual({ valid: true }) + }) + + it("accepts single character", () => { + expect(validateSkillName("a")).toEqual({ valid: true }) + }) + + it("accepts single digit", () => { + expect(validateSkillName("1")).toEqual({ valid: true }) + }) + + it("accepts maximum length name (64 characters)", () => { + const maxLengthName = "a".repeat(SKILL_NAME_MAX_LENGTH) + expect(validateSkillName(maxLengthName)).toEqual({ valid: true }) + }) + }) + + describe("empty or missing names", () => { + it("rejects empty string", () => { + expect(validateSkillName("")).toEqual({ + valid: false, + error: SkillNameValidationError.Empty, + }) + }) + }) + + describe("names that are too long", () => { + it("rejects names longer than 64 characters", () => { + const tooLongName = "a".repeat(SKILL_NAME_MAX_LENGTH + 1) + expect(validateSkillName(tooLongName)).toEqual({ + valid: false, + error: SkillNameValidationError.TooLong, + }) + }) + }) + + describe("invalid format", () => { + it("rejects uppercase letters", () => { + expect(validateSkillName("MySkill")).toEqual({ + valid: false, + error: SkillNameValidationError.InvalidFormat, + }) + }) + + it("rejects leading hyphen", () => { + expect(validateSkillName("-myskill")).toEqual({ + valid: false, + error: SkillNameValidationError.InvalidFormat, + }) + }) + + it("rejects trailing hyphen", () => { + expect(validateSkillName("myskill-")).toEqual({ + valid: false, + error: SkillNameValidationError.InvalidFormat, + }) + }) + + it("rejects consecutive hyphens", () => { + expect(validateSkillName("my--skill")).toEqual({ + valid: false, + error: SkillNameValidationError.InvalidFormat, + }) + }) + + it("rejects spaces", () => { + expect(validateSkillName("my skill")).toEqual({ + valid: false, + error: SkillNameValidationError.InvalidFormat, + }) + }) + + it("rejects underscores", () => { + expect(validateSkillName("my_skill")).toEqual({ + valid: false, + error: SkillNameValidationError.InvalidFormat, + }) + }) + + it("rejects special characters", () => { + expect(validateSkillName("my@skill")).toEqual({ + valid: false, + error: SkillNameValidationError.InvalidFormat, + }) + }) + + it("rejects dots", () => { + expect(validateSkillName("my.skill")).toEqual({ + valid: false, + error: SkillNameValidationError.InvalidFormat, + }) + }) + }) +}) + +describe("SKILL_NAME_REGEX", () => { + it("matches valid names", () => { + expect(SKILL_NAME_REGEX.test("myskill")).toBe(true) + expect(SKILL_NAME_REGEX.test("my-skill")).toBe(true) + expect(SKILL_NAME_REGEX.test("skill123")).toBe(true) + expect(SKILL_NAME_REGEX.test("a1-b2-c3")).toBe(true) + }) + + it("does not match invalid names", () => { + expect(SKILL_NAME_REGEX.test("-start")).toBe(false) + expect(SKILL_NAME_REGEX.test("end-")).toBe(false) + expect(SKILL_NAME_REGEX.test("double--hyphen")).toBe(false) + expect(SKILL_NAME_REGEX.test("UPPER")).toBe(false) + expect(SKILL_NAME_REGEX.test("")).toBe(false) + }) +}) + +describe("constants", () => { + it("has correct min length", () => { + expect(SKILL_NAME_MIN_LENGTH).toBe(1) + }) + + it("has correct max length", () => { + expect(SKILL_NAME_MAX_LENGTH).toBe(64) + }) +}) diff --git a/packages/types/src/experiment.ts b/packages/types/src/experiment.ts index 4227ce4da8..dfb0db3d51 100644 --- a/packages/types/src/experiment.ts +++ b/packages/types/src/experiment.ts @@ -7,15 +7,14 @@ import type { Keys, Equals, AssertEqual } from "./type-fu.js" */ export const experimentIds = [ - "chatSearch", - "commitReview", - "alwaysIncludeFileDetails", - "useLitePrompts", "preventFocusDisruption", "imageGeneration", "runSlashCommand", - "multipleNativeToolCalls", "customTools", + "chatSearch", + "commitReview", + "alwaysIncludeFileDetails", + "useLitePrompts", "smartMistakeDetection", ] as const @@ -45,7 +44,6 @@ export const experimentsSchema = z.object({ preventFocusDisruption: z.boolean().optional(), imageGeneration: z.boolean().optional(), runSlashCommand: z.boolean().optional(), - multipleNativeToolCalls: z.boolean().optional(), customTools: z.boolean().optional(), smartMistakeDetection: z.boolean().optional(), }) diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index e9ef4ab785..e7dbb3fa00 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -22,6 +22,7 @@ export * from "./message.js" export * from "./mode.js" export * from "./model.js" export * from "./provider-settings.js" +export * from "./skills.js" export * from "./task.js" export * from "./todo.js" export * from "./telemetry.js" diff --git a/packages/types/src/providers/moonshot.ts b/packages/types/src/providers/moonshot.ts index 7ddafab76b..a825475644 100644 --- a/packages/types/src/providers/moonshot.ts +++ b/packages/types/src/providers/moonshot.ts @@ -53,6 +53,19 @@ export const moonshotModels = { defaultTemperature: 1.0, description: `The kimi-k2-thinking model is a general-purpose agentic reasoning model developed by Moonshot AI. Thanks to its strength in deep reasoning and multi-turn tool use, it can solve even the hardest problems.`, }, + "kimi-k2.5": { + maxTokens: 16_384, + contextWindow: 262_144, + supportsImages: false, + supportsPromptCache: true, + inputPrice: 0.6, // $0.60 per million tokens (cache miss) + outputPrice: 3.0, // $3.00 per million tokens + cacheReadsPrice: 0.1, // $0.10 per million tokens (cache hit) + supportsTemperature: true, + defaultTemperature: 1.0, + description: + "Kimi K2.5 is the latest generation of Moonshot AI's Kimi series, featuring improved reasoning capabilities and enhanced performance across diverse tasks.", + }, } as const satisfies Record export const MOONSHOT_DEFAULT_TEMPERATURE = 0.6 diff --git a/packages/types/src/skills.ts b/packages/types/src/skills.ts new file mode 100644 index 0000000000..2c4ac176b0 --- /dev/null +++ b/packages/types/src/skills.ts @@ -0,0 +1,71 @@ +/** + * Skill metadata for discovery (loaded at startup) + * Only name and description are required for now + */ +export interface SkillMetadata { + name: string // Required: skill identifier + description: string // Required: when to use this skill + path: string // Absolute path to SKILL.md + source: "global" | "project" // Where the skill was discovered + mode?: string // If set, skill is only available in this mode +} + +/** + * Skill name validation constants per agentskills.io specification: + * https://agentskills.io/specification + * + * Name constraints: + * - 1-64 characters + * - Lowercase letters, numbers, and hyphens only + * - Must not start or end with a hyphen + * - Must not contain consecutive hyphens + */ +export const SKILL_NAME_MIN_LENGTH = 1 +export const SKILL_NAME_MAX_LENGTH = 64 + +/** + * Regex pattern for valid skill names. + * Matches: lowercase letters/numbers, optionally followed by groups of hyphen + lowercase letters/numbers. + * This ensures no leading/trailing hyphens and no consecutive hyphens. + */ +export const SKILL_NAME_REGEX = /^[a-z0-9]+(?:-[a-z0-9]+)*$/ + +/** + * Error codes for skill name validation. + * These can be mapped to translation keys in the frontend or error messages in the backend. + */ +export enum SkillNameValidationError { + Empty = "empty", + TooLong = "too_long", + InvalidFormat = "invalid_format", +} + +/** + * Result of skill name validation. + */ +export interface SkillNameValidationResult { + valid: boolean + error?: SkillNameValidationError +} + +/** + * Validate a skill name according to agentskills.io specification. + * + * @param name - The skill name to validate + * @returns Validation result with error code if invalid + */ +export function validateSkillName(name: string): SkillNameValidationResult { + if (!name || name.length < SKILL_NAME_MIN_LENGTH) { + return { valid: false, error: SkillNameValidationError.Empty } + } + + if (name.length > SKILL_NAME_MAX_LENGTH) { + return { valid: false, error: SkillNameValidationError.TooLong } + } + + if (!SKILL_NAME_REGEX.test(name)) { + return { valid: false, error: SkillNameValidationError.InvalidFormat } + } + + return { valid: true } +} diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 55473ddad7..fbb0405ff5 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -20,6 +20,7 @@ import type { GitCommit } from "./git.js" import type { McpServer } from "./mcp.js" import type { IZgsmModelResponseData, ModelRecord, RouterModels } from "./model.js" import type { INotice } from "./notification.js" +import type { SkillMetadata } from "./skills.js" import type { OpenAiCodexRateLimitInfo } from "./providers/openai-codex-rate-limits.js" import type { WorktreeIncludeStatus } from "./worktree.js" @@ -132,6 +133,7 @@ export interface ExtensionMessage { | "worktreeIncludeStatus" | "branchWorktreeIncludeResult" | "folderSelected" + | "skills" text?: string payload?: any // eslint-disable-line @typescript-eslint/no-explicit-any checkpointWarning?: { @@ -234,6 +236,7 @@ export interface ExtensionMessage { stepIndex?: number // For browserSessionNavigate: the target step index to display tools?: SerializedCustomToolDefinition[] // For customToolsResult modes?: { slug: string; name: string }[] // For modes response + skills?: SkillMetadata[] // For skills response aggregatedCosts?: { // For taskWithAggregatedCosts response totalCost: number @@ -683,6 +686,11 @@ export interface WebviewMessage { | "createWorktreeInclude" | "checkoutBranch" | "browseForWorktreePath" + // Skills messages + | "requestSkills" + | "createSkill" + | "deleteSkill" + | "openSkillFile" text?: string // costrict-start issueId?: string @@ -723,6 +731,9 @@ export interface WebviewMessage { timeout?: number payload?: WebViewMessagePayload source?: "global" | "project" + skillName?: string // For skill operations (createSkill, deleteSkill, openSkillFile) + skillMode?: string // For skill operations (mode restriction) + skillDescription?: string // For createSkill (skill description) requestId?: string ids?: string[] hasSystemPromptOverride?: boolean diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 84c34db2bf..b5cad73bcf 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -270,7 +270,7 @@ importers: version: 0.518.0(react@18.3.1) next: specifier: ~15.2.8 - version: 15.2.9(@opentelemetry/api@1.9.0)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) + version: 15.2.9(@opentelemetry/api@1.9.0)(babel-plugin-react-compiler@1.0.0)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) next-themes: specifier: ^0.4.6 version: 0.4.6(react-dom@18.3.1(react@18.3.1))(react@18.3.1) @@ -379,7 +379,7 @@ importers: version: 0.518.0(react@18.3.1) next: specifier: ~15.2.8 - version: 15.2.9(@opentelemetry/api@1.9.0)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) + version: 15.2.9(@opentelemetry/api@1.9.0)(babel-plugin-react-compiler@1.0.0)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) next-themes: specifier: ^0.4.6 version: 0.4.6(react-dom@18.3.1(react@18.3.1))(react@18.3.1) @@ -446,7 +446,7 @@ importers: version: 10.4.23(postcss@8.5.6) next-sitemap: specifier: ^4.2.3 - version: 4.2.3(next@15.2.9(@opentelemetry/api@1.9.0)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)) + version: 4.2.3(next@15.2.9(@opentelemetry/api@1.9.0)(babel-plugin-react-compiler@1.0.0)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)) postcss: specifier: ^8.5.4 version: 8.5.6 @@ -1024,6 +1024,9 @@ importers: specifier: 3.25.76 version: 3.25.76 devDependencies: + '@ai-sdk/openai-compatible': + specifier: ^1.0.0 + version: 1.0.31(zod@3.25.76) '@openrouter/ai-sdk-provider': specifier: ^2.0.4 version: 2.1.1(ai@6.0.59(zod@3.25.76))(zod@3.25.76) @@ -1273,6 +1276,9 @@ importers: react: specifier: ^18.3.1 version: 18.3.1 + react-compiler-runtime: + specifier: ^1.0.0 + version: 1.0.0(react@18.3.1) react-dom: specifier: ^18.3.1 version: 18.3.1(react@18.3.1) @@ -1412,6 +1418,9 @@ importers: '@vitest/ui': specifier: ^3.2.3 version: 3.2.4(vitest@3.2.4) + babel-plugin-react-compiler: + specifier: ^1.0.0 + version: 1.0.0 identity-obj-proxy: specifier: ^3.0.0 version: 3.0.0 @@ -1436,18 +1445,34 @@ packages: peerDependencies: zod: 3.25.76 + '@ai-sdk/openai-compatible@1.0.31': + resolution: {integrity: sha512-znBvaVHM0M6yWNerIEy3hR+O8ZK2sPcE7e2cxfb6kYLEX3k//JH5VDnRnajseVofg7LXtTCFFdjsB7WLf1BdeQ==} + engines: {node: '>=18'} + peerDependencies: + zod: 3.25.76 + + '@ai-sdk/provider-utils@3.0.20': + resolution: {integrity: sha512-iXHVe0apM2zUEzauqJwqmpC37A5rihrStAih5Ks+JE32iTe4LZ58y17UGBjpQQTCRw9YxMeo2UFLxLpBluyvLQ==} + engines: {node: '>=18'} + peerDependencies: + zod: 3.25.76 + '@ai-sdk/provider-utils@4.0.10': resolution: {integrity: sha512-VeDAiCH+ZK8Xs4hb9Cw7pHlujWNL52RKe8TExOkrw6Ir1AmfajBZTb9XUdKOZO08RwQElIKA8+Ltm+Gqfo8djQ==} engines: {node: '>=18'} peerDependencies: zod: 3.25.76 + '@ai-sdk/provider@2.0.1': + resolution: {integrity: sha512-KCUwswvsC5VsW2PWFqF8eJgSCu5Ysj7m1TxiHTVA6g7k360bk0RNQENT8KTMAYEs+8fWPD3Uu4dEmzGHc+jGng==} + engines: {node: '>=18'} + '@ai-sdk/provider@3.0.5': resolution: {integrity: sha512-2Xmoq6DBJqmSl80U6V9z5jJSJP7ehaJJQMy2iFUqTay06wdCqTnPVBBQbtEL8RCChenL+q5DC5H5WzU3vV3v8w==} engines: {node: '>=18'} - '@alcalzone/ansi-tokenize@0.2.3': - resolution: {integrity: sha512-jsElTJ0sQ4wHRz+C45tfect76BwbTbgkgKByOzpCN9xG61N5V6u/glvg1CsNJhq2xJIFpKHSwG3D2wPPuEYOrQ==} + '@alcalzone/ansi-tokenize@0.2.4': + resolution: {integrity: sha512-HTgrrTgZ9Jgeo6Z3oqbQ7lifOVvRR14vaDuBGPPUxk9Thm+vObaO4QfYYYWw4Zo5CWQDBEfsinFA6Gre+AqwNQ==} engines: {node: '>=18'} '@alloc/quick-lru@5.2.0': @@ -4741,6 +4766,9 @@ packages: react-native-b4a: optional: true + babel-plugin-react-compiler@1.0.0: + resolution: {integrity: sha512-Ixm8tFfoKKIPYdCCKYTsqv+Fd4IJ0DQqMyEimo+pxUOMUR9cVPlwTrFt9Avu+3cb6Zp3mAzl+t1MrG2fxxKsxw==} + bail@1.0.5: resolution: {integrity: sha512-xFbRxM1tahm08yHBP16MMjVUAvDaBMD38zsM9EMAUN61omwLmKlOpB/Zku5QkjZ8TZ4vn53pj+t518cH0S03RQ==} @@ -8614,6 +8642,11 @@ packages: resolution: {integrity: sha512-y3bGgqKj3QBdxLbLkomlohkvsA8gdAiUQlSBJnBhfn+BPxg4bc62d8TcBW15wavDfgexCgccckhcZvywyQYPOw==} hasBin: true + react-compiler-runtime@1.0.0: + resolution: {integrity: sha512-rRfjYv66HlG8896yPUDONgKzG5BxZD1nV9U6rkm+7VCuvQc903C4MjcoZR4zPw53IKSOX9wMQVpA1IAbRtzQ7w==} + peerDependencies: + react: ^17.0.0 || ^18.0.0 || ^19.0.0 || ^0.0.0-experimental + react-cookie-consent@9.0.0: resolution: {integrity: sha512-Blyj+m+Zz7SFHYqT18p16EANgnSg2sIyU6Yp3vk83AnOnSW7qnehPkUe4+8+qxztJrNmCH5GP+VHsWzAKVOoZA==} engines: {node: '>=10'} @@ -9346,8 +9379,8 @@ packages: resolution: {integrity: sha512-tsaTIkKW9b4N+AEj+SVA+WhJzV7/zMhcSu78mLKWSk7cXMOSHsBKFWUs0fWwq8QyK3MgJBQRX6Gbi4kYbdvGkQ==} engines: {node: '>=18'} - string-width@8.1.0: - resolution: {integrity: sha512-Kxl3KJGb/gxkaUMOjRsQ8IrXiGW75O4E3RPjFIINOVH8AMl2SQ/yWdTzWwF3FevIX9LcMAjJW+GRwAlAbTSXdg==} + string-width@8.1.1: + resolution: {integrity: sha512-KpqHIdDL9KwYk22wEOg/VIqYbrnLeSApsKT/bSj6Ez7pn3CftUiLAv2Lccpq1ALcpLV9UX1Ppn92npZWu2w/aw==} engines: {node: '>=20'} string.prototype.codepointat@0.2.1: @@ -10466,6 +10499,19 @@ snapshots: '@vercel/oidc': 3.1.0 zod: 3.25.76 + '@ai-sdk/openai-compatible@1.0.31(zod@3.25.76)': + dependencies: + '@ai-sdk/provider': 2.0.1 + '@ai-sdk/provider-utils': 3.0.20(zod@3.25.76) + zod: 3.25.76 + + '@ai-sdk/provider-utils@3.0.20(zod@3.25.76)': + dependencies: + '@ai-sdk/provider': 2.0.1 + '@standard-schema/spec': 1.1.0 + eventsource-parser: 3.0.6 + zod: 3.25.76 + '@ai-sdk/provider-utils@4.0.10(zod@3.25.76)': dependencies: '@ai-sdk/provider': 3.0.5 @@ -10473,11 +10519,15 @@ snapshots: eventsource-parser: 3.0.6 zod: 3.25.76 + '@ai-sdk/provider@2.0.1': + dependencies: + json-schema: 0.4.0 + '@ai-sdk/provider@3.0.5': dependencies: json-schema: 0.4.0 - '@alcalzone/ansi-tokenize@0.2.3': + '@alcalzone/ansi-tokenize@0.2.4': dependencies: ansi-styles: 6.2.3 is-fullwidth-code-point: 5.1.0 @@ -14611,6 +14661,10 @@ snapshots: b4a@1.7.3: {} + babel-plugin-react-compiler@1.0.0: + dependencies: + '@babel/types': 7.28.6 + bail@1.0.5: {} bail@2.0.2: {} @@ -14944,7 +14998,7 @@ snapshots: cli-truncate@5.1.1: dependencies: slice-ansi: 7.1.2 - string-width: 8.1.0 + string-width: 8.1.1 client-only@0.0.1: {} @@ -16865,7 +16919,7 @@ snapshots: ink@6.6.0(@types/react@18.3.27)(react@19.2.4): dependencies: - '@alcalzone/ansi-tokenize': 0.2.3 + '@alcalzone/ansi-tokenize': 0.2.4 ansi-escapes: 7.2.0 ansi-styles: 6.2.3 auto-bind: 5.0.1 @@ -16883,7 +16937,7 @@ snapshots: signal-exit: 3.0.7 slice-ansi: 7.1.2 stack-utils: 2.0.6 - string-width: 8.1.0 + string-width: 8.1.1 type-fest: 4.41.0 widest-line: 5.0.0 wrap-ansi: 9.0.2 @@ -18253,20 +18307,20 @@ snapshots: netmask@2.0.2: {} - next-sitemap@4.2.3(next@15.2.9(@opentelemetry/api@1.9.0)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)): + next-sitemap@4.2.3(next@15.2.9(@opentelemetry/api@1.9.0)(babel-plugin-react-compiler@1.0.0)(react-dom@18.3.1(react@18.3.1))(react@18.3.1)): dependencies: '@corex/deepmerge': 4.0.43 '@next/env': 13.5.11 fast-glob: 3.3.3 minimist: 1.2.8 - next: 15.2.9(@opentelemetry/api@1.9.0)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) + next: 15.2.9(@opentelemetry/api@1.9.0)(babel-plugin-react-compiler@1.0.0)(react-dom@18.3.1(react@18.3.1))(react@18.3.1) next-themes@0.4.6(react-dom@18.3.1(react@18.3.1))(react@18.3.1): dependencies: react: 18.3.1 react-dom: 18.3.1(react@18.3.1) - next@15.2.9(@opentelemetry/api@1.9.0)(react-dom@18.3.1(react@18.3.1))(react@18.3.1): + next@15.2.9(@opentelemetry/api@1.9.0)(babel-plugin-react-compiler@1.0.0)(react-dom@18.3.1(react@18.3.1))(react@18.3.1): dependencies: '@next/env': 15.2.9 '@swc/counter': 0.1.3 @@ -18287,6 +18341,7 @@ snapshots: '@next/swc-win32-arm64-msvc': 15.2.5 '@next/swc-win32-x64-msvc': 15.2.5 '@opentelemetry/api': 1.9.0 + babel-plugin-react-compiler: 1.0.0 sharp: 0.33.5 transitivePeerDependencies: - '@babel/core' @@ -19045,6 +19100,10 @@ snapshots: strip-json-comments: 2.0.1 optional: true + react-compiler-runtime@1.0.0(react@18.3.1): + dependencies: + react: 18.3.1 + react-cookie-consent@9.0.0(react@18.3.1): dependencies: js-cookie: 2.2.1 @@ -19980,7 +20039,7 @@ snapshots: get-east-asian-width: 1.4.0 strip-ansi: 7.1.2 - string-width@8.1.0: + string-width@8.1.1: dependencies: get-east-asian-width: 1.4.0 strip-ansi: 7.1.2 diff --git a/src/api/index.ts b/src/api/index.ts index bb5c93fcea..84bffb9194 100644 --- a/src/api/index.ts +++ b/src/api/index.ts @@ -91,8 +91,8 @@ export interface ApiHandlerCreateMessageMetadata { tool_choice?: OpenAI.Chat.ChatCompletionCreateParams["tool_choice"] /** * Controls whether the model can return multiple tool calls in a single response. - * When true, parallel tool calls are enabled (OpenAI's parallel_tool_calls=true). - * When false (default), only one tool call is returned per response. + * When true (default), parallel tool calls are enabled (OpenAI's parallel_tool_calls=true). + * When false, only one tool call is returned per response. */ parallelToolCalls?: boolean /** diff --git a/src/api/providers/__tests__/anthropic-vertex.spec.ts b/src/api/providers/__tests__/anthropic-vertex.spec.ts index ac18e95112..abaab32003 100644 --- a/src/api/providers/__tests__/anthropic-vertex.spec.ts +++ b/src/api/providers/__tests__/anthropic-vertex.spec.ts @@ -195,7 +195,7 @@ describe("VertexHandler", () => { tools: [], tool_choice: { type: "auto", - disable_parallel_tool_use: true, + disable_parallel_tool_use: false, }, }), expect.objectContaining({ @@ -1205,7 +1205,7 @@ describe("VertexHandler", () => { }), }), ]), - tool_choice: { type: "auto", disable_parallel_tool_use: true }, + tool_choice: { type: "auto", disable_parallel_tool_use: false }, }), expect.any(Object), ) @@ -1266,7 +1266,7 @@ describe("VertexHandler", () => { }), }), ]), - tool_choice: { type: "auto", disable_parallel_tool_use: true }, + tool_choice: { type: "auto", disable_parallel_tool_use: false }, }), { signal: undefined }, ) diff --git a/src/api/providers/__tests__/anthropic.spec.ts b/src/api/providers/__tests__/anthropic.spec.ts index 88c9a867da..b4e30cdc91 100644 --- a/src/api/providers/__tests__/anthropic.spec.ts +++ b/src/api/providers/__tests__/anthropic.spec.ts @@ -483,7 +483,7 @@ describe("AnthropicHandler", () => { }), }), ]), - tool_choice: { type: "auto", disable_parallel_tool_use: true }, + tool_choice: { type: "auto", disable_parallel_tool_use: false }, }), expect.anything(), ) @@ -525,7 +525,7 @@ describe("AnthropicHandler", () => { expect(mockCreate).toHaveBeenCalledWith( expect.objectContaining({ - tool_choice: { type: "auto", disable_parallel_tool_use: true }, + tool_choice: { type: "auto", disable_parallel_tool_use: false }, }), expect.anything(), ) @@ -546,7 +546,7 @@ describe("AnthropicHandler", () => { expect(mockCreate).toHaveBeenCalledWith( expect.objectContaining({ - tool_choice: { type: "any", disable_parallel_tool_use: true }, + tool_choice: { type: "any", disable_parallel_tool_use: false }, }), expect.anything(), ) @@ -592,7 +592,7 @@ describe("AnthropicHandler", () => { expect(mockCreate).toHaveBeenCalledWith( expect.objectContaining({ - tool_choice: { type: "tool", name: "get_weather", disable_parallel_tool_use: true }, + tool_choice: { type: "tool", name: "get_weather", disable_parallel_tool_use: false }, }), expect.anything(), ) diff --git a/src/api/providers/__tests__/deepinfra.spec.ts b/src/api/providers/__tests__/deepinfra.spec.ts index 91b450fdda..c4a9275762 100644 --- a/src/api/providers/__tests__/deepinfra.spec.ts +++ b/src/api/providers/__tests__/deepinfra.spec.ts @@ -214,9 +214,9 @@ describe("DeepInfraHandler", () => { ]), }), ) - // parallel_tool_calls should be false when not explicitly set + // parallel_tool_calls should be true by default when not explicitly set const callArgs = mockCreate.mock.calls[0][0] - expect(callArgs).toHaveProperty("parallel_tool_calls", false) + expect(callArgs).toHaveProperty("parallel_tool_calls", true) }) it("should include tool_choice when provided", async () => { @@ -264,8 +264,8 @@ describe("DeepInfraHandler", () => { // Tools are now always present (minimum 6 from ALWAYS_AVAILABLE_TOOLS) expect(callArgs).toHaveProperty("tools") expect(callArgs).toHaveProperty("tool_choice") - // parallel_tool_calls should be false when not explicitly set - expect(callArgs).toHaveProperty("parallel_tool_calls", false) + // parallel_tool_calls should be true by default when not explicitly set + expect(callArgs).toHaveProperty("parallel_tool_calls", true) }) it("should yield tool_call_partial chunks during streaming", async () => { diff --git a/src/api/providers/__tests__/lmstudio-native-tools.spec.ts b/src/api/providers/__tests__/lmstudio-native-tools.spec.ts index e84f638ff1..cca543a269 100644 --- a/src/api/providers/__tests__/lmstudio-native-tools.spec.ts +++ b/src/api/providers/__tests__/lmstudio-native-tools.spec.ts @@ -82,9 +82,9 @@ describe("LmStudioHandler Native Tools", () => { ]), }), ) - // parallel_tool_calls should be false when not explicitly set + // parallel_tool_calls should be true by default when not explicitly set const callArgs = mockCreate.mock.calls[0][0] - expect(callArgs).toHaveProperty("parallel_tool_calls", false) + expect(callArgs).toHaveProperty("parallel_tool_calls", true) }) it("should include tool_choice when provided", async () => { @@ -128,8 +128,8 @@ describe("LmStudioHandler Native Tools", () => { // Tools are now always present (minimum 6 from ALWAYS_AVAILABLE_TOOLS) expect(callArgs).toHaveProperty("tools") expect(callArgs).toHaveProperty("tool_choice") - // parallel_tool_calls should be false when not explicitly set - expect(callArgs).toHaveProperty("parallel_tool_calls", false) + // parallel_tool_calls should be true by default when not explicitly set + expect(callArgs).toHaveProperty("parallel_tool_calls", true) }) it("should yield tool_call_partial chunks during streaming", async () => { diff --git a/src/api/providers/__tests__/moonshot.spec.ts b/src/api/providers/__tests__/moonshot.spec.ts index ab919c53c2..9040ed23ca 100644 --- a/src/api/providers/__tests__/moonshot.spec.ts +++ b/src/api/providers/__tests__/moonshot.spec.ts @@ -1,67 +1,28 @@ -// Mocks must come first, before imports -const mockCreate = vi.fn() -vi.mock("openai", () => { +// Use vi.hoisted to define mock functions that can be referenced in hoisted vi.mock() calls +const { mockStreamText, mockGenerateText } = vi.hoisted(() => ({ + mockStreamText: vi.fn(), + mockGenerateText: vi.fn(), +})) + +vi.mock("ai", async (importOriginal) => { + const actual = await importOriginal() return { - __esModule: true, - default: vi.fn().mockImplementation(() => ({ - chat: { - completions: { - create: mockCreate.mockImplementation(async (options) => { - if (!options.stream) { - return { - id: "test-completion", - choices: [ - { - message: { role: "assistant", content: "Test response", refusal: null }, - finish_reason: "stop", - index: 0, - }, - ], - usage: { - prompt_tokens: 10, - completion_tokens: 5, - total_tokens: 15, - cached_tokens: 2, - }, - } - } - - // Return async iterator for streaming - return { - [Symbol.asyncIterator]: async function* () { - yield { - choices: [ - { - delta: { content: "Test response" }, - index: 0, - }, - ], - usage: null, - } - yield { - choices: [ - { - delta: {}, - index: 0, - }, - ], - usage: { - prompt_tokens: 10, - completion_tokens: 5, - total_tokens: 15, - cached_tokens: 2, - }, - } - }, - } - }), - }, - }, - })), + ...actual, + streamText: mockStreamText, + generateText: mockGenerateText, } }) -import OpenAI from "openai" +vi.mock("@ai-sdk/openai-compatible", () => ({ + createOpenAICompatible: vi.fn(() => { + // Return a function that returns a mock language model + return vi.fn(() => ({ + modelId: "moonshot-chat", + provider: "moonshot", + })) + }), +})) + import type { Anthropic } from "@anthropic-ai/sdk" import { moonshotDefaultModelId } from "@roo-code/types" @@ -90,15 +51,6 @@ describe("MoonshotHandler", () => { expect(handler.getModel().id).toBe(mockOptions.apiModelId) }) - it.skip("should throw error if API key is missing", () => { - expect(() => { - new MoonshotHandler({ - ...mockOptions, - moonshotApiKey: undefined, - }) - }).toThrow("Moonshot API key is required") - }) - it("should use default model ID if not provided", () => { const handlerWithoutModel = new MoonshotHandler({ ...mockOptions, @@ -113,12 +65,6 @@ describe("MoonshotHandler", () => { moonshotBaseUrl: undefined, }) expect(handlerWithoutBaseUrl).toBeInstanceOf(MoonshotHandler) - // The base URL is passed to OpenAI client internally - expect(OpenAI).toHaveBeenCalledWith( - expect.objectContaining({ - baseURL: "https://api.moonshot.ai/v1", - }), - ) }) it("should use chinese base URL if provided", () => { @@ -128,18 +74,6 @@ describe("MoonshotHandler", () => { moonshotBaseUrl: customBaseUrl, }) expect(handlerWithCustomUrl).toBeInstanceOf(MoonshotHandler) - // The custom base URL is passed to OpenAI client - expect(OpenAI).toHaveBeenCalledWith( - expect.objectContaining({ - baseURL: customBaseUrl, - }), - ) - }) - - it("should set includeMaxTokens to true", () => { - // Create a new handler and verify OpenAI client was called with includeMaxTokens - const _handler = new MoonshotHandler(mockOptions) - expect(OpenAI).toHaveBeenCalledWith(expect.objectContaining({ apiKey: mockOptions.moonshotApiKey })) }) }) @@ -151,7 +85,7 @@ describe("MoonshotHandler", () => { expect(model.info.maxTokens).toBe(16384) expect(model.info.contextWindow).toBe(262144) expect(model.info.supportsImages).toBe(false) - expect(model.info.supportsPromptCache).toBe(true) // Should be true now + expect(model.info.supportsPromptCache).toBe(true) }) it("should return provided model ID with default model info if model does not exist", () => { @@ -162,11 +96,8 @@ describe("MoonshotHandler", () => { const model = handlerWithInvalidModel.getModel() expect(model.id).toBe("invalid-model") // Returns provided ID expect(model.info).toBeDefined() - // With the current implementation, it's the same object reference when using default model info - expect(model.info).toBe(handler.getModel().info) - // Should have the same base properties + // Should have the same base properties as default model expect(model.info.contextWindow).toBe(handler.getModel().info.contextWindow) - // And should have supportsPromptCache set to true expect(model.info.supportsPromptCache).toBe(true) }) @@ -203,6 +134,24 @@ describe("MoonshotHandler", () => { ] it("should handle streaming responses", async () => { + // Mock the fullStream async generator + async function* mockFullStream() { + yield { type: "text-delta", text: "Test response" } + } + + // Mock usage promise + const mockUsage = Promise.resolve({ + inputTokens: 10, + outputTokens: 5, + details: { cachedInputTokens: undefined }, + raw: { cached_tokens: 2 }, + }) + + mockStreamText.mockReturnValue({ + fullStream: mockFullStream(), + usage: mockUsage, + }) + const stream = handler.createMessage(systemPrompt, messages) const chunks: any[] = [] for await (const chunk of stream) { @@ -216,6 +165,22 @@ describe("MoonshotHandler", () => { }) it("should include usage information", async () => { + async function* mockFullStream() { + yield { type: "text-delta", text: "Test response" } + } + + const mockUsage = Promise.resolve({ + inputTokens: 10, + outputTokens: 5, + details: {}, + raw: { cached_tokens: 2 }, + }) + + mockStreamText.mockReturnValue({ + fullStream: mockFullStream(), + usage: mockUsage, + }) + const stream = handler.createMessage(systemPrompt, messages) const chunks: any[] = [] for await (const chunk of stream) { @@ -229,6 +194,22 @@ describe("MoonshotHandler", () => { }) it("should include cache metrics in usage information", async () => { + async function* mockFullStream() { + yield { type: "text-delta", text: "Test response" } + } + + const mockUsage = Promise.resolve({ + inputTokens: 10, + outputTokens: 5, + details: {}, + raw: { cached_tokens: 2 }, + }) + + mockStreamText.mockReturnValue({ + fullStream: mockFullStream(), + usage: mockUsage, + }) + const stream = handler.createMessage(systemPrompt, messages) const chunks: any[] = [] for await (const chunk of stream) { @@ -242,6 +223,23 @@ describe("MoonshotHandler", () => { }) }) + describe("completePrompt", () => { + it("should complete a prompt using generateText", async () => { + mockGenerateText.mockResolvedValue({ + text: "Test completion", + }) + + const result = await handler.completePrompt("Test prompt") + + expect(result).toBe("Test completion") + expect(mockGenerateText).toHaveBeenCalledWith( + expect.objectContaining({ + prompt: "Test prompt", + }), + ) + }) + }) + describe("processUsageMetrics", () => { it("should correctly process usage metrics including cache information", () => { // We need to access the protected method, so we'll create a test subclass @@ -254,10 +252,12 @@ describe("MoonshotHandler", () => { const testHandler = new TestMoonshotHandler(mockOptions) const usage = { - prompt_tokens: 100, - completion_tokens: 50, - total_tokens: 150, - cached_tokens: 20, + inputTokens: 100, + outputTokens: 50, + details: {}, + raw: { + cached_tokens: 20, + }, } const result = testHandler.testProcessUsageMetrics(usage) @@ -279,10 +279,10 @@ describe("MoonshotHandler", () => { const testHandler = new TestMoonshotHandler(mockOptions) const usage = { - prompt_tokens: 100, - completion_tokens: 50, - total_tokens: 150, - // No cached_tokens + inputTokens: 100, + outputTokens: 50, + details: {}, + raw: {}, } const result = testHandler.testProcessUsageMetrics(usage) @@ -295,31 +295,25 @@ describe("MoonshotHandler", () => { }) }) - describe("addMaxTokensIfNeeded", () => { - it("should always add max_tokens regardless of includeMaxTokens option", () => { - // Create a test subclass to access the protected method + describe("getMaxOutputTokens", () => { + it("should return maxTokens from model info", () => { class TestMoonshotHandler extends MoonshotHandler { - public testAddMaxTokensIfNeeded(requestOptions: any, modelInfo: any) { - this.addMaxTokensIfNeeded(requestOptions, modelInfo) + public testGetMaxOutputTokens() { + return this.getMaxOutputTokens() } } const testHandler = new TestMoonshotHandler(mockOptions) - const requestOptions: any = {} - const modelInfo = { - maxTokens: 32_000, - } - - // Test with includeMaxTokens set to false - should still add max tokens - testHandler.testAddMaxTokensIfNeeded(requestOptions, modelInfo) + const result = testHandler.testGetMaxOutputTokens() - expect(requestOptions.max_tokens).toBe(32_000) + // Default model maxTokens is 16384 + expect(result).toBe(16384) }) it("should use modelMaxTokens when provided", () => { class TestMoonshotHandler extends MoonshotHandler { - public testAddMaxTokensIfNeeded(requestOptions: any, modelInfo: any) { - this.addMaxTokensIfNeeded(requestOptions, modelInfo) + public testGetMaxOutputTokens() { + return this.getMaxOutputTokens() } } @@ -328,32 +322,153 @@ describe("MoonshotHandler", () => { ...mockOptions, modelMaxTokens: customMaxTokens, }) - const requestOptions: any = {} - const modelInfo = { - maxTokens: 32_000, - } - testHandler.testAddMaxTokensIfNeeded(requestOptions, modelInfo) - - expect(requestOptions.max_tokens).toBe(customMaxTokens) + const result = testHandler.testGetMaxOutputTokens() + expect(result).toBe(customMaxTokens) }) it("should fall back to modelInfo.maxTokens when modelMaxTokens is not provided", () => { class TestMoonshotHandler extends MoonshotHandler { - public testAddMaxTokensIfNeeded(requestOptions: any, modelInfo: any) { - this.addMaxTokensIfNeeded(requestOptions, modelInfo) + public testGetMaxOutputTokens() { + return this.getMaxOutputTokens() } } const testHandler = new TestMoonshotHandler(mockOptions) - const requestOptions: any = {} - const modelInfo = { - maxTokens: 16_000, + const result = testHandler.testGetMaxOutputTokens() + + // moonshot-chat has maxTokens of 16384 + expect(result).toBe(16384) + }) + }) + + describe("tool handling", () => { + const systemPrompt = "You are a helpful assistant." + const messages: Anthropic.Messages.MessageParam[] = [ + { + role: "user", + content: [{ type: "text" as const, text: "Hello!" }], + }, + ] + + it("should handle tool calls in streaming", async () => { + async function* mockFullStream() { + yield { + type: "tool-input-start", + id: "tool-call-1", + toolName: "read_file", + } + yield { + type: "tool-input-delta", + id: "tool-call-1", + delta: '{"path":"test.ts"}', + } + yield { + type: "tool-input-end", + id: "tool-call-1", + } } - testHandler.testAddMaxTokensIfNeeded(requestOptions, modelInfo) + const mockUsage = Promise.resolve({ + inputTokens: 10, + outputTokens: 5, + details: {}, + raw: {}, + }) + + mockStreamText.mockReturnValue({ + fullStream: mockFullStream(), + usage: mockUsage, + }) + + const stream = handler.createMessage(systemPrompt, messages, { + taskId: "test-task", + tools: [ + { + type: "function", + function: { + name: "read_file", + description: "Read a file", + parameters: { + type: "object", + properties: { path: { type: "string" } }, + required: ["path"], + }, + }, + }, + ], + }) + + const chunks: any[] = [] + for await (const chunk of stream) { + chunks.push(chunk) + } + + const toolCallStartChunks = chunks.filter((c) => c.type === "tool_call_start") + const toolCallDeltaChunks = chunks.filter((c) => c.type === "tool_call_delta") + const toolCallEndChunks = chunks.filter((c) => c.type === "tool_call_end") + + expect(toolCallStartChunks.length).toBe(1) + expect(toolCallStartChunks[0].id).toBe("tool-call-1") + expect(toolCallStartChunks[0].name).toBe("read_file") + + expect(toolCallDeltaChunks.length).toBe(1) + expect(toolCallDeltaChunks[0].delta).toBe('{"path":"test.ts"}') + + expect(toolCallEndChunks.length).toBe(1) + expect(toolCallEndChunks[0].id).toBe("tool-call-1") + }) + + it("should handle complete tool calls", async () => { + async function* mockFullStream() { + yield { + type: "tool-call", + toolCallId: "tool-call-1", + toolName: "read_file", + input: { path: "test.ts" }, + } + } + + const mockUsage = Promise.resolve({ + inputTokens: 10, + outputTokens: 5, + details: {}, + raw: {}, + }) + + mockStreamText.mockReturnValue({ + fullStream: mockFullStream(), + usage: mockUsage, + }) + + const stream = handler.createMessage(systemPrompt, messages, { + taskId: "test-task", + tools: [ + { + type: "function", + function: { + name: "read_file", + description: "Read a file", + parameters: { + type: "object", + properties: { path: { type: "string" } }, + required: ["path"], + }, + }, + }, + ], + }) + + const chunks: any[] = [] + for await (const chunk of stream) { + chunks.push(chunk) + } - expect(requestOptions.max_tokens).toBe(16_000) + const toolCallChunks = chunks.filter((c) => c.type === "tool_call") + expect(toolCallChunks.length).toBe(1) + expect(toolCallChunks[0].id).toBe("tool-call-1") + expect(toolCallChunks[0].name).toBe("read_file") + expect(toolCallChunks[0].arguments).toBe('{"path":"test.ts"}') }) }) }) diff --git a/src/api/providers/__tests__/openai-native-tools.spec.ts b/src/api/providers/__tests__/openai-native-tools.spec.ts index f987c43552..e0746f792e 100644 --- a/src/api/providers/__tests__/openai-native-tools.spec.ts +++ b/src/api/providers/__tests__/openai-native-tools.spec.ts @@ -61,7 +61,7 @@ describe("OpenAiHandler native tools", () => { function: expect.objectContaining({ name: "test_tool" }), }), ]), - parallel_tool_calls: false, + parallel_tool_calls: true, }), expect.anything(), ) diff --git a/src/api/providers/__tests__/openai.spec.ts b/src/api/providers/__tests__/openai.spec.ts index 97d6e5f993..128282f75c 100644 --- a/src/api/providers/__tests__/openai.spec.ts +++ b/src/api/providers/__tests__/openai.spec.ts @@ -635,7 +635,7 @@ describe("OpenAiHandler", () => { temperature: 0, tools: undefined, tool_choice: undefined, - parallel_tool_calls: false, + parallel_tool_calls: true, }, { path: "/models/chat/completions" }, ) @@ -684,7 +684,7 @@ describe("OpenAiHandler", () => { ], tools: undefined, tool_choice: undefined, - parallel_tool_calls: false, + parallel_tool_calls: true, }, { path: "/models/chat/completions" }, ) diff --git a/src/api/providers/__tests__/qwen-code-native-tools.spec.ts b/src/api/providers/__tests__/qwen-code-native-tools.spec.ts index 5e5496596d..3b470ce461 100644 --- a/src/api/providers/__tests__/qwen-code-native-tools.spec.ts +++ b/src/api/providers/__tests__/qwen-code-native-tools.spec.ts @@ -99,7 +99,7 @@ describe("QwenCodeHandler Native Tools", () => { }), }), ]), - parallel_tool_calls: false, + parallel_tool_calls: true, }), ) }) @@ -145,7 +145,7 @@ describe("QwenCodeHandler Native Tools", () => { const callArgs = mockCreate.mock.calls[mockCreate.mock.calls.length - 1][0] expect(callArgs).toHaveProperty("tools") expect(callArgs).toHaveProperty("tool_choice") - expect(callArgs).toHaveProperty("parallel_tool_calls", false) + expect(callArgs).toHaveProperty("parallel_tool_calls", true) }) it("should yield tool_call_partial chunks during streaming", async () => { diff --git a/src/api/providers/__tests__/unbound.spec.ts b/src/api/providers/__tests__/unbound.spec.ts index 122ba5a4df..c18a46b7d9 100644 --- a/src/api/providers/__tests__/unbound.spec.ts +++ b/src/api/providers/__tests__/unbound.spec.ts @@ -375,7 +375,7 @@ describe("UnboundHandler", () => { }), }), ]), - parallel_tool_calls: false, + parallel_tool_calls: true, }), expect.objectContaining({ headers: { @@ -435,7 +435,7 @@ describe("UnboundHandler", () => { const callArgs = mockCreate.mock.calls[mockCreate.mock.calls.length - 1][0] expect(callArgs).toHaveProperty("tools") expect(callArgs).toHaveProperty("tool_choice") - expect(callArgs).toHaveProperty("parallel_tool_calls", false) + expect(callArgs).toHaveProperty("parallel_tool_calls", true) }) it("should yield tool_call_partial chunks during streaming", async () => { diff --git a/src/api/providers/__tests__/vercel-ai-gateway.spec.ts b/src/api/providers/__tests__/vercel-ai-gateway.spec.ts index 23e09a97dc..1e9b2ebe72 100644 --- a/src/api/providers/__tests__/vercel-ai-gateway.spec.ts +++ b/src/api/providers/__tests__/vercel-ai-gateway.spec.ts @@ -367,7 +367,7 @@ describe("VercelAiGatewayHandler", () => { ) }) - it("should include parallel_tool_calls: false by default", async () => { + it("should include parallel_tool_calls: true by default", async () => { const handler = new VercelAiGatewayHandler(mockOptions) const messageGenerator = handler.createMessage("test prompt", [], { @@ -379,7 +379,7 @@ describe("VercelAiGatewayHandler", () => { expect(mockCreate).toHaveBeenCalledWith( expect.objectContaining({ tools: expect.any(Array), - parallel_tool_calls: false, + parallel_tool_calls: true, }), ) }) diff --git a/src/api/providers/__tests__/xai.spec.ts b/src/api/providers/__tests__/xai.spec.ts index 64e6f1dea6..c622c9d4fc 100644 --- a/src/api/providers/__tests__/xai.spec.ts +++ b/src/api/providers/__tests__/xai.spec.ts @@ -339,7 +339,7 @@ describe("XAIHandler", () => { }), }), ]), - parallel_tool_calls: false, + parallel_tool_calls: true, }), ) }) @@ -393,7 +393,7 @@ describe("XAIHandler", () => { const callArgs = mockCreate.mock.calls[mockCreate.mock.calls.length - 1][0] expect(callArgs).toHaveProperty("tools") expect(callArgs).toHaveProperty("tool_choice") - expect(callArgs).toHaveProperty("parallel_tool_calls", false) + expect(callArgs).toHaveProperty("parallel_tool_calls", true) }) it("should yield tool_call_partial chunks during streaming", async () => { diff --git a/src/api/providers/base-openai-compatible-provider.ts b/src/api/providers/base-openai-compatible-provider.ts index 4fa85c1798..9134440d08 100644 --- a/src/api/providers/base-openai-compatible-provider.ts +++ b/src/api/providers/base-openai-compatible-provider.ts @@ -95,7 +95,7 @@ export abstract class BaseOpenAiCompatibleProvider stream_options: { include_usage: true }, tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } // Add thinking parameter if reasoning is enabled and model supports it diff --git a/src/api/providers/cerebras.ts b/src/api/providers/cerebras.ts index cbf4950f24..ef67cb9775 100644 --- a/src/api/providers/cerebras.ts +++ b/src/api/providers/cerebras.ts @@ -146,7 +146,7 @@ export class CerebrasHandler extends BaseProvider implements SingleCompletionHan // Native tool calling support tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } try { diff --git a/src/api/providers/deepinfra.ts b/src/api/providers/deepinfra.ts index 9048bee333..8e051b38ec 100644 --- a/src/api/providers/deepinfra.ts +++ b/src/api/providers/deepinfra.ts @@ -74,7 +74,7 @@ export class DeepInfraHandler extends RouterProvider implements SingleCompletion prompt_cache_key, tools: this.convertToolsForOpenAI(_metadata?.tools), tool_choice: _metadata?.tool_choice, - parallel_tool_calls: _metadata?.parallelToolCalls ?? false, + parallel_tool_calls: _metadata?.parallelToolCalls ?? true, } as OpenAI.Chat.Completions.ChatCompletionCreateParamsStreaming if (this.supportsTemperature(modelId)) { diff --git a/src/api/providers/deepseek.ts b/src/api/providers/deepseek.ts index a1d594affc..17ce6e0db7 100644 --- a/src/api/providers/deepseek.ts +++ b/src/api/providers/deepseek.ts @@ -72,7 +72,7 @@ export class DeepSeekHandler extends OpenAiHandler { ...(isThinkingModel && { thinking: { type: "enabled" } }), tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } // Add max_tokens if needed diff --git a/src/api/providers/gemini-cli.ts b/src/api/providers/gemini-cli.ts index 1772d3430e..0636c5a09f 100644 --- a/src/api/providers/gemini-cli.ts +++ b/src/api/providers/gemini-cli.ts @@ -16,7 +16,7 @@ import { getModelParams } from "../transform/model-params" import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata } from "../index" import { BaseProvider } from "./base-provider" -import { getGeminiCliLiteToolGuide } from "../../core/prompts/tools/native-tools/lite-descriptions" +import { getGeminiCliLiteToolGuide } from "../../core/prompts/tools/lite-descriptions" import { TagMatcher } from "../../utils/tag-matcher" // OAuth2 Configuration (from Cline implementation) diff --git a/src/api/providers/index.ts b/src/api/providers/index.ts index a5b48cd362..3784956afc 100644 --- a/src/api/providers/index.ts +++ b/src/api/providers/index.ts @@ -20,6 +20,8 @@ export { MistralHandler } from "./mistral" export { OpenAiCodexHandler } from "./openai-codex" export { OpenAiNativeHandler } from "./openai-native" export { OpenAiHandler } from "./openai" +export { OpenAICompatibleHandler } from "./openai-compatible" +export type { OpenAICompatibleConfig } from "./openai-compatible" export { OpenRouterHandler } from "./openrouter" export { QwenCodeHandler } from "./qwen-code" export { RequestyHandler } from "./requesty" diff --git a/src/api/providers/lm-studio.ts b/src/api/providers/lm-studio.ts index 7f977d5f8b..4daa33e240 100644 --- a/src/api/providers/lm-studio.ts +++ b/src/api/providers/lm-studio.ts @@ -90,7 +90,7 @@ export class LmStudioHandler extends BaseProvider implements SingleCompletionHan stream: true, tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } if (this.options.lmStudioSpeculativeDecodingEnabled && this.options.lmStudioDraftModelId) { diff --git a/src/api/providers/moonshot.ts b/src/api/providers/moonshot.ts index d29a10a3b3..f7a849cc02 100644 --- a/src/api/providers/moonshot.ts +++ b/src/api/providers/moonshot.ts @@ -1,4 +1,3 @@ -import OpenAI from "openai" import { moonshotModels, moonshotDefaultModelId, type ModelInfo } from "@roo-code/types" import type { ApiHandlerOptions } from "../../shared/api" @@ -6,18 +5,25 @@ import type { ApiHandlerOptions } from "../../shared/api" import type { ApiStreamUsageChunk } from "../transform/stream" import { getModelParams } from "../transform/model-params" -import { OpenAiHandler } from "./openai" +import { OpenAICompatibleHandler, OpenAICompatibleConfig } from "./openai-compatible" -export class MoonshotHandler extends OpenAiHandler { +export class MoonshotHandler extends OpenAICompatibleHandler { constructor(options: ApiHandlerOptions) { - super({ - ...options, - openAiApiKey: options.moonshotApiKey ?? "not-provided", - openAiModelId: options.apiModelId ?? moonshotDefaultModelId, - openAiBaseUrl: options.moonshotBaseUrl ?? "https://api.moonshot.ai/v1", - openAiStreamingEnabled: true, - includeMaxTokens: true, - }) + const modelId = options.apiModelId ?? moonshotDefaultModelId + const modelInfo = + moonshotModels[modelId as keyof typeof moonshotModels] || moonshotModels[moonshotDefaultModelId] + + const config: OpenAICompatibleConfig = { + providerName: "moonshot", + baseURL: options.moonshotBaseUrl ?? "https://api.moonshot.ai/v1", + apiKey: options.moonshotApiKey ?? "not-provided", + modelId, + modelInfo, + modelMaxTokens: options.modelMaxTokens ?? undefined, + temperature: options.modelTemperature ?? undefined, + } + + super(options, config) } override getModel() { @@ -27,25 +33,38 @@ export class MoonshotHandler extends OpenAiHandler { return { id, info, ...params } } - // Override to handle Moonshot's usage metrics, including caching. - protected override processUsageMetrics(usage: any): ApiStreamUsageChunk { + /** + * Override to handle Moonshot's usage metrics, including caching. + * Moonshot returns cached_tokens in a different location than standard OpenAI. + */ + protected override processUsageMetrics(usage: { + inputTokens?: number + outputTokens?: number + details?: { + cachedInputTokens?: number + reasoningTokens?: number + } + raw?: Record + }): ApiStreamUsageChunk { + // Moonshot uses cached_tokens at the top level of raw usage data + const rawUsage = usage.raw as { cached_tokens?: number } | undefined + return { type: "usage", - inputTokens: usage?.prompt_tokens || 0, - outputTokens: usage?.completion_tokens || 0, + inputTokens: usage.inputTokens || 0, + outputTokens: usage.outputTokens || 0, cacheWriteTokens: 0, - cacheReadTokens: usage?.cached_tokens, + cacheReadTokens: rawUsage?.cached_tokens ?? usage.details?.cachedInputTokens, } } - // Override to always include max_tokens for Moonshot (not max_completion_tokens) - protected override addMaxTokensIfNeeded( - requestOptions: - | OpenAI.Chat.Completions.ChatCompletionCreateParamsStreaming - | OpenAI.Chat.Completions.ChatCompletionCreateParamsNonStreaming, - modelInfo: ModelInfo, - ): void { - // Moonshot uses max_tokens instead of max_completion_tokens - requestOptions.max_tokens = this.options.modelMaxTokens || modelInfo.maxTokens + /** + * Override to always include max_tokens for Moonshot (not max_completion_tokens). + * Moonshot requires max_tokens parameter to be sent. + */ + protected override getMaxOutputTokens(): number | undefined { + const modelInfo = this.config.modelInfo + // Moonshot always requires max_tokens + return this.options.modelMaxTokens || modelInfo.maxTokens || undefined } } diff --git a/src/api/providers/openai-codex.ts b/src/api/providers/openai-codex.ts index c9e804c93b..e6c294a95c 100644 --- a/src/api/providers/openai-codex.ts +++ b/src/api/providers/openai-codex.ts @@ -321,7 +321,7 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion } }), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } return body diff --git a/src/api/providers/openai-compatible.ts b/src/api/providers/openai-compatible.ts new file mode 100644 index 0000000000..d129e72452 --- /dev/null +++ b/src/api/providers/openai-compatible.ts @@ -0,0 +1,212 @@ +/** + * OpenAI-compatible provider base class using Vercel AI SDK. + * This provides a parallel implementation to OpenAiHandler using @ai-sdk/openai-compatible. + */ + +import { Anthropic } from "@anthropic-ai/sdk" +import OpenAI from "openai" +import { createOpenAICompatible } from "@ai-sdk/openai-compatible" +import { streamText, generateText, LanguageModel, ToolSet } from "ai" + +import type { ModelInfo } from "@roo-code/types" + +import type { ApiHandlerOptions } from "../../shared/api" + +import { convertToAiSdkMessages, convertToolsForAiSdk, processAiSdkStreamPart } from "../transform/ai-sdk" +import { ApiStream, ApiStreamUsageChunk } from "../transform/stream" + +import { DEFAULT_HEADERS } from "./constants" +import { BaseProvider } from "./base-provider" +import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata } from "../index" + +/** + * Configuration options for creating an OpenAI-compatible provider. + */ +export interface OpenAICompatibleConfig { + /** Provider name for identification */ + providerName: string + /** Base URL for the API endpoint */ + baseURL: string + /** API key for authentication */ + apiKey: string + /** Model ID to use */ + modelId: string + /** Model information */ + modelInfo: ModelInfo + /** Optional custom headers */ + headers?: Record + /** Whether to include max_tokens in requests (default: false uses max_completion_tokens) */ + useMaxTokens?: boolean + /** User-configured max tokens override */ + modelMaxTokens?: number + /** Temperature setting */ + temperature?: number +} + +/** + * Base class for OpenAI-compatible API providers using Vercel AI SDK. + * Extends BaseProvider and implements SingleCompletionHandler. + */ +export abstract class OpenAICompatibleHandler extends BaseProvider implements SingleCompletionHandler { + protected options: ApiHandlerOptions + protected config: OpenAICompatibleConfig + protected provider: ReturnType + + constructor(options: ApiHandlerOptions, config: OpenAICompatibleConfig) { + super() + this.options = options + this.config = config + + // Create the OpenAI-compatible provider using AI SDK + this.provider = createOpenAICompatible({ + name: config.providerName, + baseURL: config.baseURL, + apiKey: config.apiKey, + headers: { + ...DEFAULT_HEADERS, + ...(config.headers || {}), + }, + }) + } + + /** + * Get the language model for the configured model ID. + */ + protected getLanguageModel(): LanguageModel { + return this.provider(this.config.modelId) + } + + /** + * Get the model information. Must be implemented by subclasses. + */ + abstract override getModel(): { id: string; info: ModelInfo; maxTokens?: number; temperature?: number } + + /** + * Process usage metrics from the AI SDK response. + * Can be overridden by subclasses to handle provider-specific usage formats. + */ + protected processUsageMetrics(usage: { + inputTokens?: number + outputTokens?: number + details?: { + cachedInputTokens?: number + reasoningTokens?: number + } + raw?: Record + }): ApiStreamUsageChunk { + return { + type: "usage", + inputTokens: usage.inputTokens || 0, + outputTokens: usage.outputTokens || 0, + cacheReadTokens: usage.details?.cachedInputTokens, + reasoningTokens: usage.details?.reasoningTokens, + } + } + + /** + * Map OpenAI tool_choice to AI SDK toolChoice format. + */ + protected mapToolChoice( + toolChoice: OpenAI.Chat.ChatCompletionCreateParams["tool_choice"], + ): "auto" | "none" | "required" | { type: "tool"; toolName: string } | undefined { + if (!toolChoice) { + return undefined + } + + // Handle string values + if (typeof toolChoice === "string") { + switch (toolChoice) { + case "auto": + return "auto" + case "none": + return "none" + case "required": + return "required" + default: + return "auto" + } + } + + // Handle object values (OpenAI ChatCompletionNamedToolChoice format) + if (typeof toolChoice === "object" && "type" in toolChoice) { + if (toolChoice.type === "function" && "function" in toolChoice && toolChoice.function?.name) { + return { type: "tool", toolName: toolChoice.function.name } + } + } + + return undefined + } + + /** + * Get the max tokens parameter to include in the request. + */ + protected getMaxOutputTokens(): number | undefined { + const modelInfo = this.config.modelInfo + const maxTokens = this.config.modelMaxTokens || modelInfo.maxTokens + + return maxTokens ?? undefined + } + + /** + * Create a message stream using the AI SDK. + */ + override async *createMessage( + systemPrompt: string, + messages: Anthropic.Messages.MessageParam[], + metadata?: ApiHandlerCreateMessageMetadata, + ): ApiStream { + const model = this.getModel() + const languageModel = this.getLanguageModel() + + // Convert messages to AI SDK format + const aiSdkMessages = convertToAiSdkMessages(messages) + + // Convert tools to OpenAI format first, then to AI SDK format + const openAiTools = this.convertToolsForOpenAI(metadata?.tools) + const aiSdkTools = convertToolsForAiSdk(openAiTools) as ToolSet | undefined + + // Build the request options + const requestOptions: Parameters[0] = { + model: languageModel, + system: systemPrompt, + messages: aiSdkMessages, + temperature: model.temperature ?? this.config.temperature ?? 0, + maxOutputTokens: this.getMaxOutputTokens(), + tools: aiSdkTools, + toolChoice: this.mapToolChoice(metadata?.tool_choice), + } + + // Use streamText for streaming responses + const result = streamText(requestOptions) + + // Process the full stream to get all events + for await (const part of result.fullStream) { + // Use the processAiSdkStreamPart utility to convert stream parts + for (const chunk of processAiSdkStreamPart(part)) { + yield chunk + } + } + + // Yield usage metrics at the end + const usage = await result.usage + if (usage) { + yield this.processUsageMetrics(usage) + } + } + + /** + * Complete a prompt using the AI SDK generateText. + */ + async completePrompt(prompt: string): Promise { + const languageModel = this.getLanguageModel() + + const { text } = await generateText({ + model: languageModel, + prompt, + maxOutputTokens: this.getMaxOutputTokens(), + temperature: this.config.temperature ?? 0, + }) + + return text + } +} diff --git a/src/api/providers/openai-native.ts b/src/api/providers/openai-native.ts index 71280ce477..b93214b65d 100644 --- a/src/api/providers/openai-native.ts +++ b/src/api/providers/openai-native.ts @@ -378,7 +378,7 @@ export class OpenAiNativeHandler extends BaseProvider implements SingleCompletio } }), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } // Include text.verbosity only when the model explicitly supports it diff --git a/src/api/providers/openai.ts b/src/api/providers/openai.ts index b491b13c64..e17bd3d003 100644 --- a/src/api/providers/openai.ts +++ b/src/api/providers/openai.ts @@ -161,7 +161,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl ...(reasoning && reasoning), tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } // Add max_tokens if needed @@ -229,7 +229,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl // Tools are always present (minimum ALWAYS_AVAILABLE_TOOLS) tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } // Add max_tokens if needed @@ -352,7 +352,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl // Tools are always present (minimum ALWAYS_AVAILABLE_TOOLS) tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } // O3 family models do not support the deprecated max_tokens parameter @@ -386,7 +386,7 @@ export class OpenAiHandler extends BaseProvider implements SingleCompletionHandl // Tools are always present (minimum ALWAYS_AVAILABLE_TOOLS) tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } // O3 family models do not support the deprecated max_tokens parameter diff --git a/src/api/providers/qwen-code.ts b/src/api/providers/qwen-code.ts index bdbe387cee..1af946c243 100644 --- a/src/api/providers/qwen-code.ts +++ b/src/api/providers/qwen-code.ts @@ -228,7 +228,7 @@ export class QwenCodeHandler extends BaseProvider implements SingleCompletionHan max_completion_tokens: model.info.maxTokens, tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } const stream = await this.callApiWithRetry(() => client.chat.completions.create(requestOptions)) diff --git a/src/api/providers/unbound.ts b/src/api/providers/unbound.ts index 27caeb965e..5a6c7f522c 100644 --- a/src/api/providers/unbound.ts +++ b/src/api/providers/unbound.ts @@ -121,7 +121,7 @@ export class UnboundHandler extends RouterProvider implements SingleCompletionHa }, tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } if (this.supportsTemperature(modelId)) { diff --git a/src/api/providers/vercel-ai-gateway.ts b/src/api/providers/vercel-ai-gateway.ts index e78a6ebaac..e77db4785c 100644 --- a/src/api/providers/vercel-ai-gateway.ts +++ b/src/api/providers/vercel-ai-gateway.ts @@ -63,7 +63,7 @@ export class VercelAiGatewayHandler extends RouterProvider implements SingleComp stream_options: { include_usage: true }, tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } const completion = await this.client.chat.completions.create(body) diff --git a/src/api/providers/xai.ts b/src/api/providers/xai.ts index e4f37dd931..f3b93d7f75 100644 --- a/src/api/providers/xai.ts +++ b/src/api/providers/xai.ts @@ -68,7 +68,7 @@ export class XAIHandler extends BaseProvider implements SingleCompletionHandler ...(reasoning && reasoning), tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } let stream diff --git a/src/api/providers/zai.ts b/src/api/providers/zai.ts index 15a7b47b7c..a2e3740c56 100644 --- a/src/api/providers/zai.ts +++ b/src/api/providers/zai.ts @@ -103,7 +103,7 @@ export class ZAiHandler extends BaseOpenAiCompatibleProvider { thinking: useReasoning ? { type: "enabled" } : { type: "disabled" }, tools: this.convertToolsForOpenAI(metadata?.tools), tool_choice: metadata?.tool_choice, - parallel_tool_calls: metadata?.parallelToolCalls ?? false, + parallel_tool_calls: metadata?.parallelToolCalls ?? true, } return this.client.chat.completions.create(params) diff --git a/src/api/providers/zgsm.ts b/src/api/providers/zgsm.ts index 6829e1bf70..b65abc2a52 100644 --- a/src/api/providers/zgsm.ts +++ b/src/api/providers/zgsm.ts @@ -38,7 +38,7 @@ import { getEditorType } from "../../utils/getEditorType" import { ChatCompletionChunk } from "openai/resources/index.mjs" import { convertToZAiFormat } from "../transform/zai-format" import { isDebug } from "../../utils/getDebugState" -import { xmlLiteToolGuide } from "../../core/prompts/tools/native-tools/lite-descriptions" +import { xmlLiteToolGuide } from "../../core/prompts/tools/lite-descriptions" const autoModeModelId = "Auto" const isDev = process.env.NODE_ENV === "development" @@ -469,7 +469,7 @@ export class ZgsmAiHandler extends BaseProvider implements SingleCompletionHandl ...requestOptions, ...(metadata?.tools && { tools: this.convertToolsForOpenAI(metadata.tools) }), ...(metadata?.tool_choice && { tool_choice: metadata.tool_choice }), - ...(metadata?.parallelToolCalls === true && { parallel_tool_calls: true }), + ...{ parallel_tool_calls: metadata?.parallelToolCalls ?? true }, } } @@ -506,7 +506,7 @@ export class ZgsmAiHandler extends BaseProvider implements SingleCompletionHandl ? { ...(metadata?.tools && { tools: this.convertToolsForOpenAI(metadata.tools) }), ...(metadata?.tool_choice && { tool_choice: metadata.tool_choice }), - ...(metadata?.parallelToolCalls === true && { parallel_tool_calls: true }), + ...{ parallel_tool_calls: metadata?.parallelToolCalls ?? true }, } : undefined), extra_body: { @@ -997,7 +997,7 @@ export class ZgsmAiHandler extends BaseProvider implements SingleCompletionHandl ? { ...(metadata?.tools && { tools: this.convertToolsForOpenAI(metadata.tools) }), ...(metadata?.tool_choice && { tool_choice: metadata.tool_choice }), - ...(metadata?.parallelToolCalls === true && { parallel_tool_calls: true }), + ...{ parallel_tool_calls: metadata?.parallelToolCalls ?? true }, } : undefined), } @@ -1035,7 +1035,7 @@ export class ZgsmAiHandler extends BaseProvider implements SingleCompletionHandl ? { ...(metadata?.tools && { tools: this.convertToolsForOpenAI(metadata.tools) }), ...(metadata?.tool_choice && { tool_choice: metadata.tool_choice }), - ...(metadata?.parallelToolCalls === true && { parallel_tool_calls: true }), + ...{ parallel_tool_calls: metadata?.parallelToolCalls ?? true }, } : undefined), } diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index 8d60e85ebc..f0d6237bbd 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -17,7 +17,7 @@ import type { ApiStreamToolCallDeltaChunk, ApiStreamToolCallEndChunk, } from "../../api/transform/stream" -import { fixFinalToolUseResult, fixNativeToolname } from "../../utils/fixNativeToolname" +import { fixAskMultipleChoiceFinalToolUseResult, fixNativeToolname } from "../../utils/fixNativeToolname" import { MCP_TOOL_PREFIX, MCP_TOOL_SEPARATOR, parseMcpToolName, normalizeMcpToolName } from "../../utils/mcp-name" import { defaultModeSlug } from "../../shared/modes" @@ -346,7 +346,7 @@ export class NativeToolCallParser { name: toolCall.name as ToolName, arguments: (toolCall.name as ToolName) === "ask_multiple_choice" && isZgsm - ? fixFinalToolUseResult(toolCall.argumentsAccumulator) + ? fixAskMultipleChoiceFinalToolUseResult(toolCall.argumentsAccumulator) : toolCall.argumentsAccumulator, }, isZgsm, diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts index 9adac74fd2..83b6a6eeb0 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-images.spec.ts @@ -49,6 +49,7 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calling", () = closeBrowser: vi.fn().mockResolvedValue(undefined), }, recordToolUsage: vi.fn(), + recordToolError: vi.fn(), toolRepetitionDetector: { check: vi.fn().mockReturnValue({ allowExecution: true }), }, @@ -61,6 +62,7 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calling", () = }), }, say: vi.fn().mockResolvedValue(undefined), + sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing required parameter"), ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }), } @@ -85,21 +87,18 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calling", () = type: "tool_use", id: toolCallId, // ID indicates native tool calling name: "ask_followup_question", - params: { question: "What do you see?" }, - nativeArgs: { question: "What do you see?", follow_up: [] }, + params: { + question: "What do you see?", + follow_up: [{ text: "Option 1" }, { text: "Option 2" }], + }, + nativeArgs: { + question: "What do you see?", + follow_up: [{ text: "Option 1" }, { text: "Option 2" }], + }, }, ] - // Create a mock askApproval that includes images in the response - const imageBlock: Anthropic.ImageBlockParam = { - type: "image", - source: { - type: "base64", - media_type: "image/png", - data: "base64ImageData", - }, - } - + // Mock the ask method to return images mockTask.ask = vi.fn().mockResolvedValue({ response: "yesButtonClicked", text: "I see a cat", @@ -264,51 +263,6 @@ describe("presentAssistantMessage - Image Handling in Native Tool Calling", () = expect(textBlocks.length).toBe(0) }) - it("should send tool_result with is_error for skipped tools in native tool calling when didAlreadyUseTool is true", async () => { - // Simulate multiple tool calls with native protocol - const toolCallId1 = "tool_call_003" - const toolCallId2 = "tool_call_004" - - mockTask.assistantMessageContent = [ - { - type: "tool_use", - id: toolCallId1, - name: "read_file", - params: { path: "test.txt" }, - }, - { - type: "tool_use", - id: toolCallId2, - name: "write_to_file", - params: { path: "output.txt", content: "test" }, - }, - ] - - // First tool was already used - mockTask.didAlreadyUseTool = true - - // Process the second tool (should be skipped) - mockTask.currentStreamingContentIndex = 1 - await presentAssistantMessage(mockTask) - - // Find the tool_result for the second tool - const toolResult = mockTask.userMessageContent.find( - (item: any) => item.type === "tool_result" && item.tool_use_id === toolCallId2, - ) - - // Verify that a tool_result block was created (not a text block) - expect(toolResult).toBeDefined() - expect(toolResult.tool_use_id).toBe(toolCallId2) - expect(toolResult.is_error).toBe(true) - expect(toolResult.content).toContain("was not executed because a tool has already been used") - - // Ensure no text blocks were added for this rejection - const textBlocks = mockTask.userMessageContent.filter( - (item: any) => item.type === "text" && item.text.includes("was not executed because"), - ) - expect(textBlocks.length).toBe(0) - }) - it("should reject subsequent tool calls when a legacy/XML-style tool call is encountered", async () => { mockTask.assistantMessageContent = [ { diff --git a/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts b/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts index ed90127b5b..15a1e2d867 100644 --- a/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts +++ b/src/core/assistant-message/__tests__/presentAssistantMessage-unknown-tool.spec.ts @@ -217,32 +217,6 @@ describe("presentAssistantMessage - Unknown Tool Handling", () => { expect(mockTask.userMessageContentReady).toBe(true) }) - it("should still work with didAlreadyUseTool flag for unknown tool", async () => { - const toolCallId = "tool_call_already_used_test" - mockTask.assistantMessageContent = [ - { - type: "tool_use", - id: toolCallId, - name: "unknown_tool", - params: {}, - partial: false, - }, - ] - - mockTask.didAlreadyUseTool = true - - await presentAssistantMessage(mockTask) - - // When didAlreadyUseTool is true, should send error tool_result - const toolResult = mockTask.userMessageContent.find( - (item: any) => item.type === "tool_result" && item.tool_use_id === toolCallId, - ) - - expect(toolResult).toBeDefined() - expect(toolResult.is_error).toBe(true) - expect(toolResult.content).toContain("was not executed because a tool has already been used") - }) - it("should still work with didRejectTool flag for unknown tool", async () => { const toolCallId = "tool_call_rejected_test" mockTask.assistantMessageContent = [ diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 452af75506..1bf3da38df 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -133,25 +133,6 @@ export async function presentAssistantMessage(cline: Task) { break } - // Get parallel tool calling state from experiments - const mcpState = await cline.providerRef.deref()?.getState() - const mcpParallelToolCallsEnabled = mcpState?.experiments?.multipleNativeToolCalls ?? false - - if (!mcpParallelToolCallsEnabled && cline.didAlreadyUseTool) { - const toolCallId = mcpBlock.id - const errorMessage = `MCP tool [${mcpBlock.name}] was not executed because a tool has already been used in this message. Only one tool may be used per message.` - - if (toolCallId) { - cline.pushToolResultToUserContent({ - type: "tool_result", - tool_use_id: toolCallId, - content: errorMessage, - is_error: true, - }) - } - break - } - // Track if we've already pushed a tool result let hasToolResult = false const toolCallId = mcpBlock.id @@ -212,12 +193,8 @@ export async function presentAssistantMessage(cline: Task) { } hasToolResult = true - // Only set didAlreadyUseTool when parallel tool calling is disabled - if (!mcpParallelToolCallsEnabled) { - cline.didAlreadyUseTool = true - if (editTools.includes(block.name) && block.partial === false) - updateCospecMetadata(cline, block?.params?.path) - } + if (editTools.includes(block.name) && block.partial === false) + updateCospecMetadata(cline, block?.params?.path) } // const toolDescription = () => `[mcp_tool: ${mcpBlock.serverName}/${mcpBlock.toolName}]` @@ -460,24 +437,6 @@ export async function presentAssistantMessage(cline: Task) { break } - // Get parallel tool calling state from experiments (stateExperiments already fetched above) - const parallelToolCallsEnabled = stateExperiments?.multipleNativeToolCalls ?? false - - if (!parallelToolCallsEnabled && cline.didAlreadyUseTool) { - // Ignore any content after a tool has already been used. - // For native tool calling, we must send a tool_result for every tool_use to avoid API errors - const errorMessage = `Tool [${block.name}] was not executed because a tool has already been used in this message. Only one tool may be used per message. You must assess the first tool's result before proceeding to use the next tool.` - - cline.pushToolResultToUserContent({ - type: "tool_result", - tool_use_id: toolCallId, - content: errorMessage, - is_error: true, - }) - - break - } - // Track if we've already pushed a tool result for this tool call (native tool calling only) let hasToolResult = false @@ -573,12 +532,8 @@ export async function presentAssistantMessage(cline: Task) { } hasToolResult = true - // Only set didAlreadyUseTool when parallel tool calling is disabled - if (!parallelToolCallsEnabled) { - cline.didAlreadyUseTool = true - if (editTools.includes(block.name) && block.partial === false) - updateCospecMetadata(cline, block?.params?.path) - } + if (editTools.includes(block.name) && block.partial === false) + updateCospecMetadata(cline, block?.params?.path) } const askApproval = async ( diff --git a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/architect-mode-prompt.snap b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/architect-mode-prompt.snap index 968e954694..41c41cf56a 100644 --- a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/architect-mode-prompt.snap +++ b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/architect-mode-prompt.snap @@ -10,14 +10,14 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must use exactly one tool call per assistant response. Do not call zero tools or more than one tool in the same response. +You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must call at least one tool per assistant response. Prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster. # Tool Use Guidelines -1. Assess what information you have and what you need -2. Choose the most appropriate tool for the task -3. Use one tool at a time, waiting for results before proceeding. -4. Verify required parameters before calling tools +1. Assess what information you have and what you need to proceed +2. Choose the most appropriate tool based on the task +3. Use tools as needed - you may use multiple tools in one message or iteratively across messages +4. Each tool use should be informed by previous results - do not assume outcomes ==== diff --git a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/ask-mode-prompt.snap b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/ask-mode-prompt.snap index f6b81c8e6f..33254efc44 100644 --- a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/ask-mode-prompt.snap +++ b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/ask-mode-prompt.snap @@ -10,14 +10,14 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must use exactly one tool call per assistant response. Do not call zero tools or more than one tool in the same response. +You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must call at least one tool per assistant response. Prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster. # Tool Use Guidelines -1. Assess what information you have and what you need -2. Choose the most appropriate tool for the task -3. Use one tool at a time, waiting for results before proceeding. -4. Verify required parameters before calling tools +1. Assess what information you have and what you need to proceed +2. Choose the most appropriate tool based on the task +3. Use tools as needed - you may use multiple tools in one message or iteratively across messages +4. Each tool use should be informed by previous results - do not assume outcomes ==== diff --git a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/mcp-server-creation-disabled.snap b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/mcp-server-creation-disabled.snap index 889b698961..0e47c0a84b 100644 --- a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/mcp-server-creation-disabled.snap +++ b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/mcp-server-creation-disabled.snap @@ -10,14 +10,14 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must use exactly one tool call per assistant response. Do not call zero tools or more than one tool in the same response. +You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must call at least one tool per assistant response. Prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster. # Tool Use Guidelines -1. Assess what information you have and what you need -2. Choose the most appropriate tool for the task -3. Use one tool at a time, waiting for results before proceeding. -4. Verify required parameters before calling tools +1. Assess what information you have and what you need to proceed +2. Choose the most appropriate tool based on the task +3. Use tools as needed - you may use multiple tools in one message or iteratively across messages +4. Each tool use should be informed by previous results - do not assume outcomes ==== diff --git a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/partial-reads-enabled.snap b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/partial-reads-enabled.snap index 889b698961..0e47c0a84b 100644 --- a/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/partial-reads-enabled.snap +++ b/src/core/prompts/__tests__/__snapshots__/add-custom-instructions/partial-reads-enabled.snap @@ -10,14 +10,14 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must use exactly one tool call per assistant response. Do not call zero tools or more than one tool in the same response. +You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must call at least one tool per assistant response. Prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster. # Tool Use Guidelines -1. Assess what information you have and what you need -2. Choose the most appropriate tool for the task -3. Use one tool at a time, waiting for results before proceeding. -4. Verify required parameters before calling tools +1. Assess what information you have and what you need to proceed +2. Choose the most appropriate tool based on the task +3. Use tools as needed - you may use multiple tools in one message or iteratively across messages +4. Each tool use should be informed by previous results - do not assume outcomes ==== diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/consistent-system-prompt.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/consistent-system-prompt.snap index ad825c6c4c..c755bd8eab 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/consistent-system-prompt.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/consistent-system-prompt.snap @@ -10,14 +10,14 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must use exactly one tool call per assistant response. Do not call zero tools or more than one tool in the same response. +You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must call at least one tool per assistant response. Prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster. # Tool Use Guidelines -1. Assess what information you have and what you need -2. Choose the most appropriate tool for the task -3. Use one tool at a time, waiting for results before proceeding. -4. Verify required parameters before calling tools +1. Assess what information you have and what you need to proceed +2. Choose the most appropriate tool based on the task +3. Use tools as needed - you may use multiple tools in one message or iteratively across messages +4. Each tool use should be informed by previous results - do not assume outcomes ==== diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-computer-use-support.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-computer-use-support.snap index ad825c6c4c..c755bd8eab 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-computer-use-support.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-computer-use-support.snap @@ -10,14 +10,14 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must use exactly one tool call per assistant response. Do not call zero tools or more than one tool in the same response. +You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must call at least one tool per assistant response. Prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster. # Tool Use Guidelines -1. Assess what information you have and what you need -2. Choose the most appropriate tool for the task -3. Use one tool at a time, waiting for results before proceeding. -4. Verify required parameters before calling tools +1. Assess what information you have and what you need to proceed +2. Choose the most appropriate tool based on the task +3. Use tools as needed - you may use multiple tools in one message or iteratively across messages +4. Each tool use should be informed by previous results - do not assume outcomes ==== diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-different-viewport-size.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-different-viewport-size.snap index ad825c6c4c..c755bd8eab 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-different-viewport-size.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-different-viewport-size.snap @@ -10,14 +10,14 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must use exactly one tool call per assistant response. Do not call zero tools or more than one tool in the same response. +You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must call at least one tool per assistant response. Prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster. # Tool Use Guidelines -1. Assess what information you have and what you need -2. Choose the most appropriate tool for the task -3. Use one tool at a time, waiting for results before proceeding. -4. Verify required parameters before calling tools +1. Assess what information you have and what you need to proceed +2. Choose the most appropriate tool based on the task +3. Use tools as needed - you may use multiple tools in one message or iteratively across messages +4. Each tool use should be informed by previous results - do not assume outcomes ==== diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-mcp-hub-provided.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-mcp-hub-provided.snap index 587d666c3a..98c27af56b 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-mcp-hub-provided.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-mcp-hub-provided.snap @@ -10,14 +10,14 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must use exactly one tool call per assistant response. Do not call zero tools or more than one tool in the same response. +You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must call at least one tool per assistant response. Prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster. # Tool Use Guidelines -1. Assess what information you have and what you need -2. Choose the most appropriate tool for the task -3. Use one tool at a time, waiting for results before proceeding. -4. Verify required parameters before calling tools +1. Assess what information you have and what you need to proceed +2. Choose the most appropriate tool based on the task +3. Use tools as needed - you may use multiple tools in one message or iteratively across messages +4. Each tool use should be informed by previous results - do not assume outcomes ==== diff --git a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-undefined-mcp-hub.snap b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-undefined-mcp-hub.snap index ad825c6c4c..c755bd8eab 100644 --- a/src/core/prompts/__tests__/__snapshots__/system-prompt/with-undefined-mcp-hub.snap +++ b/src/core/prompts/__tests__/__snapshots__/system-prompt/with-undefined-mcp-hub.snap @@ -10,14 +10,14 @@ ALL responses MUST show ANY `language construct` OR filename reference as clicka TOOL USE -You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must use exactly one tool call per assistant response. Do not call zero tools or more than one tool in the same response. +You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must call at least one tool per assistant response. Prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster. # Tool Use Guidelines -1. Assess what information you have and what you need -2. Choose the most appropriate tool for the task -3. Use one tool at a time, waiting for results before proceeding. -4. Verify required parameters before calling tools +1. Assess what information you have and what you need to proceed +2. Choose the most appropriate tool based on the task +3. Use tools as needed - you may use multiple tools in one message or iteratively across messages +4. Each tool use should be informed by previous results - do not assume outcomes ==== diff --git a/src/core/prompts/sections/__tests__/tool-use-guidelines.spec.ts b/src/core/prompts/sections/__tests__/tool-use-guidelines.spec.ts index 26c1f77d85..6d1f4b3fbf 100644 --- a/src/core/prompts/sections/__tests__/tool-use-guidelines.spec.ts +++ b/src/core/prompts/sections/__tests__/tool-use-guidelines.spec.ts @@ -1,66 +1,39 @@ import { getToolUseGuidelinesSection } from "../tool-use-guidelines" -import { EXPERIMENT_IDS } from "../../../../shared/experiments" describe("getToolUseGuidelinesSection", () => { - describe("with MULTIPLE_NATIVE_TOOL_CALLS disabled (default)", () => { - it("should include proper numbered guidelines", () => { - const guidelines = getToolUseGuidelinesSection() - - // Check that all numbered items are present with correct numbering - expect(guidelines).toContain("1. Assess what information") - expect(guidelines).toContain("2. Choose the most appropriate tool") - expect(guidelines).toContain("3. If multiple actions are needed") - expect(guidelines).toContain("4. After each tool use") - }) - - it("should include single-tool-per-message guidance when experiment disabled", () => { - const guidelines = getToolUseGuidelinesSection({}) - - expect(guidelines).toContain("use one tool at a time per message") - expect(guidelines).not.toContain("you may use multiple tools in a single message") - expect(guidelines).not.toContain("Formulate your tool use using") - expect(guidelines).toContain("ALWAYS wait for user confirmation") - }) - - it("should include simplified iterative process guidelines", () => { - const guidelines = getToolUseGuidelinesSection() + it("should include proper numbered guidelines", () => { + const guidelines = getToolUseGuidelinesSection() - expect(guidelines).toContain("carefully considering the user's response after each tool use") - expect(guidelines).toContain("It is crucial to proceed step-by-step") - }) + expect(guidelines).toContain("1. Assess what information") + expect(guidelines).toContain("2. Choose the most appropriate tool") + expect(guidelines).toContain("3. If multiple actions are needed") }) - describe("with MULTIPLE_NATIVE_TOOL_CALLS enabled", () => { - it("should include multiple-tools-per-message guidance when experiment enabled", () => { - const guidelines = getToolUseGuidelinesSection({ - [EXPERIMENT_IDS.MULTIPLE_NATIVE_TOOL_CALLS]: true, - }) + it("should include multiple-tools-per-message guidance", () => { + const guidelines = getToolUseGuidelinesSection() - expect(guidelines).toContain("you may use multiple tools in a single message") - expect(guidelines).not.toContain("use one tool at a time per message") - expect(guidelines).not.toContain("After each tool use, the user will respond") - }) + expect(guidelines).toContain("you may use multiple tools in a single message") + expect(guidelines).not.toContain("use one tool at a time per message") + }) - it("should use simplified footer without step-by-step language", () => { - const guidelines = getToolUseGuidelinesSection({ - [EXPERIMENT_IDS.MULTIPLE_NATIVE_TOOL_CALLS]: true, - }) + it("should use simplified footer without step-by-step language", () => { + const guidelines = getToolUseGuidelinesSection() - // When multiple tools per message is enabled, we don't want the - // "step-by-step" or "after each tool use" language that would - // contradict the ability to batch tool calls. - expect(guidelines).toContain("carefully considering the user's response after tool executions") - expect(guidelines).not.toContain("It is crucial to proceed step-by-step") - expect(guidelines).not.toContain("ALWAYS wait for user confirmation after each tool use") - }) + expect(guidelines).toContain("carefully considering the user's response after tool executions") + expect(guidelines).not.toContain("It is crucial to proceed step-by-step") + expect(guidelines).not.toContain("ALWAYS wait for user confirmation after each tool use") }) it("should include common guidance", () => { const guidelines = getToolUseGuidelinesSection() expect(guidelines).toContain("Assess what information you already have") expect(guidelines).toContain("Choose the most appropriate tool") - expect(guidelines).toContain("After each tool use, the user will respond") - // No legacy XML-tag tool-calling remnants expect(guidelines).not.toContain("") }) + + it("should not include per-tool confirmation guidelines", () => { + const guidelines = getToolUseGuidelinesSection() + + expect(guidelines).not.toContain("After each tool use, the user will respond with the result") + }) }) diff --git a/src/core/prompts/sections/__tests__/tool-use.spec.ts b/src/core/prompts/sections/__tests__/tool-use.spec.ts index fc220d1acd..878db81a1c 100644 --- a/src/core/prompts/sections/__tests__/tool-use.spec.ts +++ b/src/core/prompts/sections/__tests__/tool-use.spec.ts @@ -1,52 +1,31 @@ import { getSharedToolUseSection } from "../tool-use" describe("getSharedToolUseSection", () => { - describe("with MULTIPLE_NATIVE_TOOL_CALLS disabled (default)", () => { - it("should include one tool per message requirement when experiment is disabled (default)", () => { - // No experiment flags passed (default: disabled) - const section = getSharedToolUseSection() + it("should include native tool-calling instructions", () => { + const section = getSharedToolUseSection() - expect(section).toContain("You must use exactly one tool call per assistant response") - expect(section).toContain("Do not call zero tools or more than one tool") - }) - - it("should include one tool per message requirement when experiment is explicitly disabled", () => { - const section = getSharedToolUseSection({ multipleNativeToolCalls: false }) - - expect(section).toContain("You must use exactly one tool call per assistant response") - expect(section).toContain("Do not call zero tools or more than one tool") - }) - - it("should NOT include one tool per message requirement when experiment is enabled", () => { - const section = getSharedToolUseSection({ multipleNativeToolCalls: true }) - - expect(section).not.toContain("You must use exactly one tool per message") - expect(section).not.toContain("every assistant message must include a tool call") - expect(section).toContain("You must call at least one tool per assistant response") - expect(section).toContain("Prefer calling as many tools as are reasonably needed") - }) + expect(section).toContain("provider-native tool-calling mechanism") + expect(section).toContain("Do not include XML markup or examples") + }) - it("should include native tool-calling instructions", () => { - const section = getSharedToolUseSection() + it("should include multiple tools per message guidance", () => { + const section = getSharedToolUseSection() - expect(section).toContain("provider-native tool-calling mechanism") - expect(section).toContain("Do not include XML markup or examples") - }) + expect(section).toContain("You must call at least one tool per assistant response") + expect(section).toContain("Prefer calling as many tools as are reasonably needed") + }) - it("should NOT include XML formatting instructions", () => { - const section = getSharedToolUseSection() + it("should NOT include single tool per message restriction", () => { + const section = getSharedToolUseSection() - expect(section).not.toContain("") - expect(section).not.toContain("") - }) + expect(section).not.toContain("You must use exactly one tool call per assistant response") + expect(section).not.toContain("Do not call zero tools or more than one tool") }) - describe("default (native-only)", () => { - it("should default to native tool calling when no arguments are provided", () => { - const section = getSharedToolUseSection() - expect(section).toContain("provider-native tool-calling mechanism") - // No legacy XML-tag tool-calling remnants - expect(section).not.toContain("") - }) + it("should NOT include XML formatting instructions", () => { + const section = getSharedToolUseSection() + + expect(section).not.toContain("") + expect(section).not.toContain("") }) }) diff --git a/src/core/prompts/sections/index.ts b/src/core/prompts/sections/index.ts index 7c5e53b01b..8a61dec5b1 100644 --- a/src/core/prompts/sections/index.ts +++ b/src/core/prompts/sections/index.ts @@ -11,3 +11,4 @@ export { getSkillsSection } from "./skills" // Lite versions - simplified and concise prompts export { getLiteToolUseGuidelinesSection, getLiteCapabilitiesSection, getLiteObjectiveSection } from "./lite-sections" +// export { getLiteCapabilitiesSection, getLiteObjectiveSection } from "./lite-sections" diff --git a/src/core/prompts/sections/lite-sections.ts b/src/core/prompts/sections/lite-sections.ts index e4e7d4c126..33875b8f79 100644 --- a/src/core/prompts/sections/lite-sections.ts +++ b/src/core/prompts/sections/lite-sections.ts @@ -4,22 +4,13 @@ import { McpHub } from "../../../services/mcp/McpHub" /** * Lite version of tool use guidelines - simplified and concise */ -export function getLiteToolUseGuidelinesSection(experimentFlags?: Record): string { - const isMultipleNativeToolCallsEnabled = experiments.isEnabled( - experimentFlags ?? {}, - EXPERIMENT_IDS.MULTIPLE_NATIVE_TOOL_CALLS, - ) - - const toolUsageNote = isMultipleNativeToolCallsEnabled - ? "Use tools as needed - you may use multiple tools in one message or iteratively across messages." - : "Use one tool at a time, waiting for results before proceeding." - +export function getLiteToolUseGuidelinesSection(): string { return `# Tool Use Guidelines -1. Assess what information you have and what you need -2. Choose the most appropriate tool for the task -3. ${toolUsageNote} -4. Verify required parameters before calling tools` +1. Assess what information you have and what you need to proceed +2. Choose the most appropriate tool based on the task +3. Use tools as needed - you may use multiple tools in one message or iteratively across messages +4. Each tool use should be informed by previous results - do not assume outcomes` } /** diff --git a/src/core/prompts/sections/tool-use-guidelines.ts b/src/core/prompts/sections/tool-use-guidelines.ts index 256e7aba5a..78193372cc 100644 --- a/src/core/prompts/sections/tool-use-guidelines.ts +++ b/src/core/prompts/sections/tool-use-guidelines.ts @@ -1,66 +1,9 @@ -import { experiments, EXPERIMENT_IDS } from "../../../shared/experiments" - -export function getToolUseGuidelinesSection(experimentFlags?: Record): string { - // Build guidelines array with automatic numbering - let itemNumber = 1 - const guidelinesList: string[] = [] - - // First guideline is always the same - guidelinesList.push( - `${itemNumber++}. Assess what information you already have and what information you need to proceed with the task.`, - ) - - guidelinesList.push( - `${itemNumber++}. Choose the most appropriate tool based on the task and the tool descriptions provided. Assess if you need additional information to proceed, and which of the available tools would be most effective for gathering this information. For example using the list_files tool is more effective than running a command like \`ls\` in the terminal. It's critical that you think about each available tool and use the one that best fits the current step in the task.`, - ) - - // Native-only guidelines. - // Check if multiple native tool calls is enabled via experiment. - const isMultipleNativeToolCallsEnabled = experiments.isEnabled( - experimentFlags ?? {}, - EXPERIMENT_IDS.MULTIPLE_NATIVE_TOOL_CALLS, - ) - - if (isMultipleNativeToolCallsEnabled) { - guidelinesList.push( - `${itemNumber++}. If multiple actions are needed, you may use multiple tools in a single message when appropriate, or use tools iteratively across messages. Each tool use should be informed by the results of previous tool uses. Do not assume the outcome of any tool use. Each step must be informed by the previous step's result.`, - ) - } else { - guidelinesList.push( - `${itemNumber++}. If multiple actions are needed, use one tool at a time per message to accomplish the task iteratively, with each tool use being informed by the result of the previous tool use. Do not assume the outcome of any tool use. Each step must be informed by the previous step's result.`, - ) - } - // Only add the per-tool confirmation guideline when NOT using multiple tool calls. - // When multiple tool calls are enabled, results may arrive batched (after all tools), - // so "after each tool use" would contradict the batching behavior. - if (!isMultipleNativeToolCallsEnabled) { - guidelinesList.push(`${itemNumber++}. After each tool use, the user will respond with the result of that tool use. This result will provide you with the necessary information to continue your task or make further decisions. This response may include: - - Information about whether the tool succeeded or failed, along with any reasons for failure. - - Linter errors that may have arisen due to the changes you made, which you'll need to address. - - New terminal output in reaction to the changes, which you may need to consider or act upon. - - Any other relevant feedback or information related to the tool use.`) - } - - // Only add the "wait for confirmation" guideline when NOT using multiple tool calls. - // With multiple tool calls enabled, the model is expected to batch tools and get results together. - if (!isMultipleNativeToolCallsEnabled) { - guidelinesList.push( - `${itemNumber++}. ALWAYS wait for user confirmation after each tool use before proceeding. Never assume the success of a tool use without explicit confirmation of the result from the user.`, - ) - } - - // Join guidelines and add the footer - const footer = isMultipleNativeToolCallsEnabled - ? `\n\nBy carefully considering the user's response after tool executions, you can react accordingly and make informed decisions about how to proceed with the task. This iterative process helps ensure the overall success and accuracy of your work.` - : `\n\nIt is crucial to proceed step-by-step, waiting for the user's message after each tool use before moving forward with the task. This approach allows you to: -1. Confirm the success of each step before proceeding. -2. Address any issues or errors that arise immediately. -3. Adapt your approach based on new information or unexpected results. -4. Ensure that each action builds correctly on the previous ones. - -By waiting for and carefully considering the user's response after each tool use, you can react accordingly and make informed decisions about how to proceed with the task. This iterative process helps ensure the overall success and accuracy of your work.` - +export function getToolUseGuidelinesSection(): string { return `# Tool Use Guidelines -${guidelinesList.join("\n")}${footer}` +1. Assess what information you already have and what information you need to proceed with the task. +2. Choose the most appropriate tool based on the task and the tool descriptions provided. Assess if you need additional information to proceed, and which of the available tools would be most effective for gathering this information. For example using the list_files tool is more effective than running a command like \`ls\` in the terminal. It's critical that you think about each available tool and use the one that best fits the current step in the task. +3. If multiple actions are needed, you may use multiple tools in a single message when appropriate, or use tools iteratively across messages. Each tool use should be informed by the results of previous tool uses. Do not assume the outcome of any tool use. Each step must be informed by the previous step's result. + +By carefully considering the user's response after tool executions, you can react accordingly and make informed decisions about how to proceed with the task. This iterative process helps ensure the overall success and accuracy of your work.` } diff --git a/src/core/prompts/sections/tool-use.ts b/src/core/prompts/sections/tool-use.ts index 4bc0905c56..a3def86c07 100644 --- a/src/core/prompts/sections/tool-use.ts +++ b/src/core/prompts/sections/tool-use.ts @@ -1,19 +1,7 @@ -import { experiments, EXPERIMENT_IDS } from "../../../shared/experiments" - -export function getSharedToolUseSection(experimentFlags?: Record): string { - // Check if multiple native tool calls is enabled via experiment - const isMultipleNativeToolCallsEnabled = experiments.isEnabled( - experimentFlags ?? {}, - EXPERIMENT_IDS.MULTIPLE_NATIVE_TOOL_CALLS, - ) - - const toolUseGuidance = isMultipleNativeToolCallsEnabled - ? " You must call at least one tool per assistant response. Prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster." - : " You must use exactly one tool call per assistant response. Do not call zero tools or more than one tool in the same response." - +export function getSharedToolUseSection(): string { return `==== TOOL USE -You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples.${toolUseGuidance}` +You have access to a set of tools that are executed upon the user's approval. Use the provider-native tool-calling mechanism. Do not include XML markup or examples. You must call at least one tool per assistant response. Prefer calling as many tools as are reasonably needed in a single response to reduce back-and-forth and complete tasks faster.` } diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index ab44a9acd3..91f4bc203a 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -129,9 +129,9 @@ async function generatePrompt(data: { ${markdownFormattingSection()} -${getSharedToolUseSection(experiments)}${toolsCatalog} +${getSharedToolUseSection()}${toolsCatalog} -${useLitePrompts ? getLiteToolUseGuidelinesSection(experiments) : getToolUseGuidelinesSection(experiments)} +${useLitePrompts ? getLiteToolUseGuidelinesSection() : getToolUseGuidelinesSection()} ${useLitePrompts ? getLiteCapabilitiesSection(cwd, shouldIncludeMcp ? mcpHub : undefined) : getCapabilitiesSection(cwd, shouldIncludeMcp ? mcpHub : undefined)} diff --git a/src/core/prompts/tools/native-tools/lite-descriptions.ts b/src/core/prompts/tools/lite-descriptions.ts similarity index 92% rename from src/core/prompts/tools/native-tools/lite-descriptions.ts rename to src/core/prompts/tools/lite-descriptions.ts index 0d087eeceb..c34f7d1c0f 100644 --- a/src/core/prompts/tools/native-tools/lite-descriptions.ts +++ b/src/core/prompts/tools/lite-descriptions.ts @@ -119,6 +119,17 @@ Params: command (REQUIRED), args (REQUIRED)` } getLiteRunSlashCommandDescription.toolname = "run_slash_command" +export function getLiteReadCommandOutputDescription(): string { + return `## read_command_output +Retrieve the full output from a command that was truncated in execute_command. +Params: artifact_id (REQUIRED), search (optional), offset (optional), limit (optional) +- artifact_id: The artifact filename from truncated output (e.g., "cmd-1706119234567.txt") +- search: Optional pattern to filter lines (regex or literal, case-insensitive) +- offset: Byte offset to start reading from (default: 0) +- limit: Maximum bytes to return (default: 40KB)` +} +getLiteReadCommandOutputDescription.toolname = "read_command_output" + // Native tools export function getLiteApplyDiffDescription(): string { return `## apply_diff @@ -219,6 +230,8 @@ ${getLiteListFilesDescription()} ${getLiteExecuteCommandDescription()} +${getLiteReadCommandOutputDescription()} + ${getLiteAskFollowupQuestionDescription()} ${getLiteAttemptCompletionDescription()} diff --git a/src/core/prompts/tools/native-tools/__tests__/converters.spec.ts b/src/core/prompts/tools/native-tools/__tests__/converters.spec.ts index 4c1f606754..02032346b2 100644 --- a/src/core/prompts/tools/native-tools/__tests__/converters.spec.ts +++ b/src/core/prompts/tools/native-tools/__tests__/converters.spec.ts @@ -147,14 +147,14 @@ describe("converters", () => { }) describe("convertOpenAIToolChoiceToAnthropic", () => { - it("should return auto with disabled parallel tool use when toolChoice is undefined", () => { + it("should return auto with enabled parallel tool use by default when toolChoice is undefined", () => { const result = convertOpenAIToolChoiceToAnthropic(undefined) - expect(result).toEqual({ type: "auto", disable_parallel_tool_use: true }) + expect(result).toEqual({ type: "auto", disable_parallel_tool_use: false }) }) - it("should return auto with enabled parallel tool use when parallelToolCalls is true", () => { - const result = convertOpenAIToolChoiceToAnthropic(undefined, true) - expect(result).toEqual({ type: "auto", disable_parallel_tool_use: false }) + it("should return auto with disabled parallel tool use when parallelToolCalls is false", () => { + const result = convertOpenAIToolChoiceToAnthropic(undefined, false) + expect(result).toEqual({ type: "auto", disable_parallel_tool_use: true }) }) it("should return undefined for 'none' tool choice", () => { @@ -164,17 +164,17 @@ describe("converters", () => { it("should return auto for 'auto' tool choice", () => { const result = convertOpenAIToolChoiceToAnthropic("auto") - expect(result).toEqual({ type: "auto", disable_parallel_tool_use: true }) + expect(result).toEqual({ type: "auto", disable_parallel_tool_use: false }) }) it("should return any for 'required' tool choice", () => { const result = convertOpenAIToolChoiceToAnthropic("required") - expect(result).toEqual({ type: "any", disable_parallel_tool_use: true }) + expect(result).toEqual({ type: "any", disable_parallel_tool_use: false }) }) it("should return auto for unknown string tool choice", () => { const result = convertOpenAIToolChoiceToAnthropic("unknown" as any) - expect(result).toEqual({ type: "auto", disable_parallel_tool_use: true }) + expect(result).toEqual({ type: "auto", disable_parallel_tool_use: false }) }) it("should convert function object form to tool type", () => { @@ -185,28 +185,28 @@ describe("converters", () => { expect(result).toEqual({ type: "tool", name: "get_weather", - disable_parallel_tool_use: true, + disable_parallel_tool_use: false, }) }) - it("should handle function object form with parallel tool calls enabled", () => { + it("should handle function object form with parallel tool calls disabled", () => { const result = convertOpenAIToolChoiceToAnthropic( { type: "function", function: { name: "read_file" }, }, - true, + false, ) expect(result).toEqual({ type: "tool", name: "read_file", - disable_parallel_tool_use: false, + disable_parallel_tool_use: true, }) }) it("should return auto for object without function property", () => { const result = convertOpenAIToolChoiceToAnthropic({ type: "something" } as any) - expect(result).toEqual({ type: "auto", disable_parallel_tool_use: true }) + expect(result).toEqual({ type: "auto", disable_parallel_tool_use: false }) }) }) }) diff --git a/src/core/prompts/tools/native-tools/converters.ts b/src/core/prompts/tools/native-tools/converters.ts index aec38e4025..9ad06cb2ad 100644 --- a/src/core/prompts/tools/native-tools/converters.ts +++ b/src/core/prompts/tools/native-tools/converters.ts @@ -58,7 +58,7 @@ export function convertOpenAIToolsToAnthropic(tools: OpenAI.Chat.ChatCompletionT * - { type: "function", function: { name } } → { type: "tool", name } * * @param toolChoice - OpenAI tool_choice parameter - * @param parallelToolCalls - When true, allows parallel tool calls. When false (default), disables parallel tool calls. + * @param parallelToolCalls - When true (default), allows parallel tool calls. When false, disables parallel tool calls. * @returns Anthropic ToolChoice or undefined if tools should be omitted * * @example @@ -67,16 +67,16 @@ export function convertOpenAIToolsToAnthropic(tools: OpenAI.Chat.ChatCompletionT * // Returns: { type: "auto", disable_parallel_tool_use: true } * * convertOpenAIToolChoiceToAnthropic({ type: "function", function: { name: "get_weather" } }) - * // Returns: { type: "tool", name: "get_weather", disable_parallel_tool_use: true } + * // Returns: { type: "tool", name: "get_weather", disable_parallel_tool_use: false } * ``` */ export function convertOpenAIToolChoiceToAnthropic( toolChoice: OpenAI.Chat.ChatCompletionCreateParams["tool_choice"], parallelToolCalls?: boolean, ): Anthropic.Messages.MessageCreateParams["tool_choice"] | undefined { - // Anthropic allows parallel tool calls by default. When parallelToolCalls is false or undefined, + // Parallel tool calls are enabled by default. When parallelToolCalls is explicitly false, // we disable parallel tool use to ensure one tool call at a time. - const disableParallelToolUse = !parallelToolCalls + const disableParallelToolUse = parallelToolCalls === false if (!toolChoice) { // Default to auto with parallel tool use control diff --git a/src/core/prompts/tools/native-tools/index.ts b/src/core/prompts/tools/native-tools/index.ts index dca2d22ce8..dd743faeba 100644 --- a/src/core/prompts/tools/native-tools/index.ts +++ b/src/core/prompts/tools/native-tools/index.ts @@ -50,7 +50,8 @@ import { getLiteAskMultipleChoiceDescription, getLiteSearchAndReplaceDescription, getLiteSearchReplaceDescription, -} from "./lite-descriptions" + getLiteReadCommandOutputDescription, +} from "../lite-descriptions" /** * Options for customizing the native tools array. @@ -95,6 +96,8 @@ function getLiteDescription(tool: OpenAI.Chat.ChatCompletionFunctionTool): strin return getLiteNewTaskDescription() case "read_file": return getLiteReadFileDescription() + case "read_command_output": + return getLiteReadCommandOutputDescription() case "run_slash_command": return getLiteRunSlashCommandDescription() case "edit_file": diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 74cc95d7a4..cb077e87ea 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1792,7 +1792,7 @@ export class Task extends EventEmitter implements TaskLike { ? { tools: allTools, tool_choice: "auto", - parallelToolCalls: false, + parallelToolCalls: true, } : {}), } @@ -4174,7 +4174,7 @@ export class Task extends EventEmitter implements TaskLike { ? { tools: allTools, tool_choice: "auto", - parallelToolCalls: false, + parallelToolCalls: true, } : {}), } @@ -4395,7 +4395,7 @@ export class Task extends EventEmitter implements TaskLike { ? { tools: contextMgmtTools, tool_choice: "auto", - parallelToolCalls: false, + parallelToolCalls: true, } : {}), } @@ -4555,8 +4555,6 @@ export class Task extends EventEmitter implements TaskLike { const { id } = (await ZgsmAuthService.getInstance()?.getUserInfo()) ?? {} const shouldIncludeTools = allTools.length > 0 - const parallelToolCallsEnabled = state?.experiments?.multipleNativeToolCalls ?? false - const metadata: ApiHandlerCreateMessageMetadata = { mode: mode, zgsmCodeMode, @@ -4608,7 +4606,7 @@ export class Task extends EventEmitter implements TaskLike { ? { tools: allTools, tool_choice: "auto", - parallelToolCalls: parallelToolCallsEnabled, + parallelToolCalls: true, // When mode restricts tools, provide allowedFunctionNames so providers // like Gemini can see all tools in history but only call allowed ones ...(allowedFunctionNames ? { allowedFunctionNames } : {}), diff --git a/src/core/tools/AskFollowupQuestionTool.ts b/src/core/tools/AskFollowupQuestionTool.ts index a345709b4f..d4c6dc9c77 100644 --- a/src/core/tools/AskFollowupQuestionTool.ts +++ b/src/core/tools/AskFollowupQuestionTool.ts @@ -30,6 +30,14 @@ export class AskFollowupQuestionTool extends BaseTool<"ask_followup_question"> { return } + if (!follow_up?.length) { + task.consecutiveMistakeCount++ + task.recordToolError("ask_followup_question") + task.didToolFailInCurrentTurn = true + pushToolResult(await task.sayAndCreateMissingParamError("ask_followup_question", "follow_up")) + return + } + // Transform follow_up suggestions to the format expected by task.ask const follow_up_json = { question, diff --git a/src/core/webview/__tests__/skillsMessageHandler.spec.ts b/src/core/webview/__tests__/skillsMessageHandler.spec.ts new file mode 100644 index 0000000000..900796dd6c --- /dev/null +++ b/src/core/webview/__tests__/skillsMessageHandler.spec.ts @@ -0,0 +1,334 @@ +// npx vitest run src/core/webview/__tests__/skillsMessageHandler.spec.ts + +import type { SkillMetadata, WebviewMessage } from "@roo-code/types" +import type { ClineProvider } from "../ClineProvider" + +// Mock vscode first +vi.mock("vscode", () => { + const showErrorMessage = vi.fn() + + return { + window: { + showErrorMessage, + }, + } +}) + +// Mock open-file +vi.mock("../../../integrations/misc/open-file", () => ({ + openFile: vi.fn(), +})) + +// Mock i18n +vi.mock("../../../i18n", () => ({ + t: (key: string, params?: Record) => { + const translations: Record = { + "skills:errors.missing_create_fields": "Missing required fields: skillName, source, or skillDescription", + "skills:errors.manager_unavailable": "Skills manager not available", + "skills:errors.missing_delete_fields": "Missing required fields: skillName or source", + "skills:errors.skill_not_found": `Skill "${params?.name}" not found`, + } + return translations[key] || key + }, +})) + +import * as vscode from "vscode" +import { openFile } from "../../../integrations/misc/open-file" +import { handleRequestSkills, handleCreateSkill, handleDeleteSkill, handleOpenSkillFile } from "../skillsMessageHandler" + +describe("skillsMessageHandler", () => { + const mockLog = vi.fn() + const mockPostMessageToWebview = vi.fn() + const mockGetSkillsMetadata = vi.fn() + const mockCreateSkill = vi.fn() + const mockDeleteSkill = vi.fn() + const mockGetSkill = vi.fn() + + const createMockProvider = (hasSkillsManager: boolean = true): ClineProvider => { + const skillsManager = hasSkillsManager + ? { + getSkillsMetadata: mockGetSkillsMetadata, + createSkill: mockCreateSkill, + deleteSkill: mockDeleteSkill, + getSkill: mockGetSkill, + } + : undefined + + return { + log: mockLog, + postMessageToWebview: mockPostMessageToWebview, + getSkillsManager: () => skillsManager, + } as unknown as ClineProvider + } + + const mockSkills: SkillMetadata[] = [ + { + name: "test-skill", + description: "Test skill description", + path: "/path/to/test-skill/SKILL.md", + source: "global", + }, + { + name: "project-skill", + description: "Project skill description", + path: "/project/.roo/skills/project-skill/SKILL.md", + source: "project", + mode: "code", + }, + ] + + beforeEach(() => { + vi.clearAllMocks() + }) + + describe("handleRequestSkills", () => { + it("returns skills when skills manager is available", async () => { + const provider = createMockProvider(true) + mockGetSkillsMetadata.mockReturnValue(mockSkills) + + const result = await handleRequestSkills(provider) + + expect(result).toEqual(mockSkills) + expect(mockPostMessageToWebview).toHaveBeenCalledWith({ type: "skills", skills: mockSkills }) + }) + + it("returns empty skills when skills manager is not available", async () => { + const provider = createMockProvider(false) + + const result = await handleRequestSkills(provider) + + expect(result).toEqual([]) + expect(mockPostMessageToWebview).toHaveBeenCalledWith({ type: "skills", skills: [] }) + }) + + it("handles errors and returns empty skills", async () => { + const provider = createMockProvider(true) + mockGetSkillsMetadata.mockImplementation(() => { + throw new Error("Test error") + }) + + const result = await handleRequestSkills(provider) + + expect(result).toEqual([]) + expect(mockLog).toHaveBeenCalled() + expect(mockPostMessageToWebview).toHaveBeenCalledWith({ type: "skills", skills: [] }) + }) + }) + + describe("handleCreateSkill", () => { + it("creates a skill successfully", async () => { + const provider = createMockProvider(true) + mockCreateSkill.mockResolvedValue("/path/to/new-skill/SKILL.md") + mockGetSkillsMetadata.mockReturnValue(mockSkills) + + const result = await handleCreateSkill(provider, { + type: "createSkill", + skillName: "new-skill", + source: "global", + skillDescription: "New skill description", + } as WebviewMessage) + + expect(result).toEqual(mockSkills) + expect(mockCreateSkill).toHaveBeenCalledWith("new-skill", "global", "New skill description", undefined) + expect(openFile).toHaveBeenCalledWith("/path/to/new-skill/SKILL.md") + expect(mockPostMessageToWebview).toHaveBeenCalledWith({ type: "skills", skills: mockSkills }) + }) + + it("creates a skill with mode restriction", async () => { + const provider = createMockProvider(true) + mockCreateSkill.mockResolvedValue("/path/to/new-skill/SKILL.md") + mockGetSkillsMetadata.mockReturnValue(mockSkills) + + const result = await handleCreateSkill(provider, { + type: "createSkill", + skillName: "new-skill", + source: "project", + skillDescription: "New skill description", + skillMode: "code", + } as WebviewMessage) + + expect(result).toEqual(mockSkills) + expect(mockCreateSkill).toHaveBeenCalledWith("new-skill", "project", "New skill description", "code") + }) + + it("returns undefined when required fields are missing", async () => { + const provider = createMockProvider(true) + + const result = await handleCreateSkill(provider, { + type: "createSkill", + skillName: "new-skill", + // missing source and skillDescription + } as WebviewMessage) + + expect(result).toBeUndefined() + expect(mockLog).toHaveBeenCalledWith( + "Error creating skill: Missing required fields: skillName, source, or skillDescription", + ) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to create skill: Missing required fields: skillName, source, or skillDescription", + ) + }) + + it("returns undefined when skills manager is not available", async () => { + const provider = createMockProvider(false) + + const result = await handleCreateSkill(provider, { + type: "createSkill", + skillName: "new-skill", + source: "global", + skillDescription: "New skill description", + } as WebviewMessage) + + expect(result).toBeUndefined() + expect(mockLog).toHaveBeenCalledWith("Error creating skill: Skills manager not available") + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to create skill: Skills manager not available", + ) + }) + }) + + describe("handleDeleteSkill", () => { + it("deletes a skill successfully", async () => { + const provider = createMockProvider(true) + mockDeleteSkill.mockResolvedValue(undefined) + mockGetSkillsMetadata.mockReturnValue([mockSkills[1]]) + + const result = await handleDeleteSkill(provider, { + type: "deleteSkill", + skillName: "test-skill", + source: "global", + } as WebviewMessage) + + expect(result).toEqual([mockSkills[1]]) + expect(mockDeleteSkill).toHaveBeenCalledWith("test-skill", "global", undefined) + expect(mockPostMessageToWebview).toHaveBeenCalledWith({ type: "skills", skills: [mockSkills[1]] }) + }) + + it("deletes a skill with mode restriction", async () => { + const provider = createMockProvider(true) + mockDeleteSkill.mockResolvedValue(undefined) + mockGetSkillsMetadata.mockReturnValue([mockSkills[0]]) + + const result = await handleDeleteSkill(provider, { + type: "deleteSkill", + skillName: "project-skill", + source: "project", + skillMode: "code", + } as WebviewMessage) + + expect(result).toEqual([mockSkills[0]]) + expect(mockDeleteSkill).toHaveBeenCalledWith("project-skill", "project", "code") + }) + + it("returns undefined when required fields are missing", async () => { + const provider = createMockProvider(true) + + const result = await handleDeleteSkill(provider, { + type: "deleteSkill", + skillName: "test-skill", + // missing source + } as WebviewMessage) + + expect(result).toBeUndefined() + expect(mockLog).toHaveBeenCalledWith("Error deleting skill: Missing required fields: skillName or source") + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to delete skill: Missing required fields: skillName or source", + ) + }) + + it("returns undefined when skills manager is not available", async () => { + const provider = createMockProvider(false) + + const result = await handleDeleteSkill(provider, { + type: "deleteSkill", + skillName: "test-skill", + source: "global", + } as WebviewMessage) + + expect(result).toBeUndefined() + expect(mockLog).toHaveBeenCalledWith("Error deleting skill: Skills manager not available") + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to delete skill: Skills manager not available", + ) + }) + }) + + describe("handleOpenSkillFile", () => { + it("opens a skill file successfully", async () => { + const provider = createMockProvider(true) + mockGetSkill.mockReturnValue(mockSkills[0]) + + await handleOpenSkillFile(provider, { + type: "openSkillFile", + skillName: "test-skill", + source: "global", + } as WebviewMessage) + + expect(mockGetSkill).toHaveBeenCalledWith("test-skill", "global", undefined) + expect(openFile).toHaveBeenCalledWith("/path/to/test-skill/SKILL.md") + }) + + it("opens a skill file with mode restriction", async () => { + const provider = createMockProvider(true) + mockGetSkill.mockReturnValue(mockSkills[1]) + + await handleOpenSkillFile(provider, { + type: "openSkillFile", + skillName: "project-skill", + source: "project", + skillMode: "code", + } as WebviewMessage) + + expect(mockGetSkill).toHaveBeenCalledWith("project-skill", "project", "code") + expect(openFile).toHaveBeenCalledWith("/project/.roo/skills/project-skill/SKILL.md") + }) + + it("shows error when required fields are missing", async () => { + const provider = createMockProvider(true) + + await handleOpenSkillFile(provider, { + type: "openSkillFile", + skillName: "test-skill", + // missing source + } as WebviewMessage) + + expect(mockLog).toHaveBeenCalledWith( + "Error opening skill file: Missing required fields: skillName or source", + ) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to open skill file: Missing required fields: skillName or source", + ) + }) + + it("shows error when skills manager is not available", async () => { + const provider = createMockProvider(false) + + await handleOpenSkillFile(provider, { + type: "openSkillFile", + skillName: "test-skill", + source: "global", + } as WebviewMessage) + + expect(mockLog).toHaveBeenCalledWith("Error opening skill file: Skills manager not available") + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to open skill file: Skills manager not available", + ) + }) + + it("shows error when skill is not found", async () => { + const provider = createMockProvider(true) + mockGetSkill.mockReturnValue(undefined) + + await handleOpenSkillFile(provider, { + type: "openSkillFile", + skillName: "nonexistent-skill", + source: "global", + } as WebviewMessage) + + expect(mockLog).toHaveBeenCalledWith('Error opening skill file: Skill "nonexistent-skill" not found') + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + 'Failed to open skill file: Skill "nonexistent-skill" not found', + ) + }) + }) +}) diff --git a/src/core/webview/skillsMessageHandler.ts b/src/core/webview/skillsMessageHandler.ts new file mode 100644 index 0000000000..649c036b4c --- /dev/null +++ b/src/core/webview/skillsMessageHandler.ts @@ -0,0 +1,133 @@ +import * as vscode from "vscode" + +import type { SkillMetadata, WebviewMessage } from "@roo-code/types" + +import type { ClineProvider } from "./ClineProvider" +import { openFile } from "../../integrations/misc/open-file" +import { t } from "../../i18n" + +/** + * Handles the requestSkills message - returns all skills metadata + */ +export async function handleRequestSkills(provider: ClineProvider): Promise { + try { + const skillsManager = provider.getSkillsManager() + if (skillsManager) { + const skills = skillsManager.getSkillsMetadata() + await provider.postMessageToWebview({ type: "skills", skills }) + return skills + } else { + await provider.postMessageToWebview({ type: "skills", skills: [] }) + return [] + } + } catch (error) { + provider.log(`Error fetching skills: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`) + await provider.postMessageToWebview({ type: "skills", skills: [] }) + return [] + } +} + +/** + * Handles the createSkill message - creates a new skill + */ +export async function handleCreateSkill( + provider: ClineProvider, + message: WebviewMessage, +): Promise { + try { + const skillName = message.skillName + const source = message.source + const skillDescription = message.skillDescription + const skillMode = message.skillMode + + if (!skillName || !source || !skillDescription) { + throw new Error(t("skills:errors.missing_create_fields")) + } + + const skillsManager = provider.getSkillsManager() + if (!skillsManager) { + throw new Error(t("skills:errors.manager_unavailable")) + } + + const createdPath = await skillsManager.createSkill(skillName, source, skillDescription, skillMode) + + // Open the created file in the editor + openFile(createdPath) + + // Send updated skills list + const skills = skillsManager.getSkillsMetadata() + await provider.postMessageToWebview({ type: "skills", skills }) + return skills + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error) + provider.log(`Error creating skill: ${errorMessage}`) + vscode.window.showErrorMessage(`Failed to create skill: ${errorMessage}`) + return undefined + } +} + +/** + * Handles the deleteSkill message - deletes a skill + */ +export async function handleDeleteSkill( + provider: ClineProvider, + message: WebviewMessage, +): Promise { + try { + const skillName = message.skillName + const source = message.source + const skillMode = message.skillMode + + if (!skillName || !source) { + throw new Error(t("skills:errors.missing_delete_fields")) + } + + const skillsManager = provider.getSkillsManager() + if (!skillsManager) { + throw new Error(t("skills:errors.manager_unavailable")) + } + + await skillsManager.deleteSkill(skillName, source, skillMode) + + // Send updated skills list + const skills = skillsManager.getSkillsMetadata() + await provider.postMessageToWebview({ type: "skills", skills }) + return skills + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error) + provider.log(`Error deleting skill: ${errorMessage}`) + vscode.window.showErrorMessage(`Failed to delete skill: ${errorMessage}`) + return undefined + } +} + +/** + * Handles the openSkillFile message - opens a skill file in the editor + */ +export async function handleOpenSkillFile(provider: ClineProvider, message: WebviewMessage): Promise { + try { + const skillName = message.skillName + const source = message.source + const skillMode = message.skillMode + + if (!skillName || !source) { + throw new Error(t("skills:errors.missing_delete_fields")) + } + + const skillsManager = provider.getSkillsManager() + if (!skillsManager) { + throw new Error(t("skills:errors.manager_unavailable")) + } + + const skill = skillsManager.getSkill(skillName, source, skillMode) + if (!skill) { + throw new Error(t("skills:errors.skill_not_found", { name: skillName })) + } + + openFile(skill.path) + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error) + provider.log(`Error opening skill file: ${errorMessage}`) + vscode.window.showErrorMessage(`Failed to open skill file: ${errorMessage}`) + } +} diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 4ceaf2bfb5..360c3bf5f8 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -36,6 +36,7 @@ import { ClineProvider } from "./ClineProvider" import { BrowserSessionPanelManager } from "./BrowserSessionPanelManager" import { handleCheckpointRestoreOperation } from "./checkpointRestoreHandler" import { generateErrorDiagnostics } from "./diagnosticsHandler" +import { handleRequestSkills, handleCreateSkill, handleDeleteSkill, handleOpenSkillFile } from "./skillsMessageHandler" import { changeLanguage, t } from "../../i18n" import { Package } from "../../shared/package" import { type RouterName, toRouterName } from "../../shared/api" @@ -3252,6 +3253,22 @@ export const webviewMessageHandler = async ( } break } + case "requestSkills": { + await handleRequestSkills(provider) + break + } + case "createSkill": { + await handleCreateSkill(provider, message) + break + } + case "deleteSkill": { + await handleDeleteSkill(provider, message) + break + } + case "openSkillFile": { + await handleOpenSkillFile(provider, message) + break + } case "openCommandFile": { try { if (message.text) { diff --git a/src/i18n/locales/en/skills.json b/src/i18n/locales/en/skills.json new file mode 100644 index 0000000000..9cf7369bc9 --- /dev/null +++ b/src/i18n/locales/en/skills.json @@ -0,0 +1,14 @@ +{ + "errors": { + "name_length": "Skill name must be 1-{{maxLength}} characters (got {{length}})", + "name_format": "Skill name must be lowercase letters/numbers/hyphens only (no leading/trailing hyphen, no consecutive hyphens)", + "description_length": "Skill description must be 1-1024 characters (got {{length}})", + "no_workspace": "Cannot create project skill: no workspace folder is open", + "already_exists": "Skill \"{{name}}\" already exists at {{path}}", + "not_found": "Skill \"{{name}}\" not found in {{source}}{{modeInfo}}", + "missing_create_fields": "Missing required fields: skillName, source, or skillDescription", + "manager_unavailable": "Skills manager not available", + "missing_delete_fields": "Missing required fields: skillName or source", + "skill_not_found": "Skill \"{{name}}\" not found" + } +} diff --git a/src/i18n/locales/zh-CN/skills.json b/src/i18n/locales/zh-CN/skills.json new file mode 100644 index 0000000000..629aaeb6d7 --- /dev/null +++ b/src/i18n/locales/zh-CN/skills.json @@ -0,0 +1,14 @@ +{ + "errors": { + "name_length": "技能名称必须为 1-{{maxLength}} 个字符（收到 {{length}} 个）", + "name_format": "技能名称只能包含小写字母、数字和连字符（不能有前导或尾随连字符，不能有连续连字符）", + "description_length": "技能描述必须为 1-1024 个字符（收到 {{length}} 个）", + "no_workspace": "无法创建项目技能：未打开工作区文件夹", + "already_exists": "技能 \"{{name}}\" 已存在于 {{path}}", + "not_found": "在 {{source}}{{modeInfo}} 中未找到技能 \"{{name}}\"", + "missing_create_fields": "缺少必填字段：skillName、source 或 skillDescription", + "manager_unavailable": "技能管理器不可用", + "missing_delete_fields": "缺少必填字段：skillName 或 source", + "skill_not_found": "未找到技能 \"{{name}}\"" + } +} diff --git a/src/i18n/locales/zh-TW/skills.json b/src/i18n/locales/zh-TW/skills.json new file mode 100644 index 0000000000..10705f4cce --- /dev/null +++ b/src/i18n/locales/zh-TW/skills.json @@ -0,0 +1,14 @@ +{ + "errors": { + "name_length": "技能名稱必須為 1-{{maxLength}} 個字元（收到 {{length}} 個）", + "name_format": "技能名稱只能包含小寫字母、數字和連字號（不能有前導或尾隨連字號，不能有連續連字號）", + "description_length": "技能描述必須為 1-1024 個字元（收到 {{length}} 個）", + "no_workspace": "無法建立專案技能：未開啟工作區資料夾", + "already_exists": "技能「{{name}}」已存在於 {{path}}", + "not_found": "在 {{source}}{{modeInfo}} 中找不到技能「{{name}}」", + "missing_create_fields": "缺少必填欄位：skillName、source 或 skillDescription", + "manager_unavailable": "技能管理器無法使用", + "missing_delete_fields": "缺少必填欄位：skillName 或 source", + "skill_not_found": "找不到技能「{{name}}」" + } +} diff --git a/src/package.json b/src/package.json index 96084b69ee..7f96be0e44 100644 --- a/src/package.json +++ b/src/package.json @@ -10,7 +10,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.86.2", + "vscode": "^1.93.1", "node": "20.19.2" }, "author": { @@ -929,6 +929,7 @@ "zod": "3.25.76" }, "devDependencies": { + "@ai-sdk/openai-compatible": "^1.0.0", "@openrouter/ai-sdk-provider": "^2.0.4", "@roo-code/build": "workspace:^", "@roo-code/config-eslint": "workspace:^", diff --git a/src/services/skills/SkillsManager.ts b/src/services/skills/SkillsManager.ts index b9e507e796..59c1ac4bae 100644 --- a/src/services/skills/SkillsManager.ts +++ b/src/services/skills/SkillsManager.ts @@ -1,5 +1,6 @@ import * as fs from "fs/promises" import * as path from "path" +import * as os from "os" import * as vscode from "vscode" import matter from "gray-matter" @@ -8,6 +9,12 @@ import { getGlobalRooDirectory, getGlobalCostrictDirectory } from "../roo-config import { directoryExists, fileExists } from "../roo-config" import { SkillMetadata, SkillContent } from "../../shared/skills" import { modes, getAllModes } from "../../shared/modes" +import { + validateSkillName as validateSkillNameShared, + SkillNameValidationError, + SKILL_NAME_MAX_LENGTH, +} from "@roo-code/types" +import { t } from "../../i18n" // Re-export for convenience export type { SkillMetadata, SkillContent } @@ -116,23 +123,11 @@ export class SkillsManager { return } - // Strict spec validation (https://agentskills.io/specification) - // Name constraints: - // - 1-64 chars - // - lowercase letters/numbers/hyphens only - // - must not start/end with hyphen - // - must not contain consecutive hyphens - if (effectiveSkillName.length < 1 || effectiveSkillName.length > 64) { - console.error( - `Skill name "${effectiveSkillName}" is invalid: name must be 1-64 characters (got ${effectiveSkillName.length})`, - ) - return - } - const nameFormat = /^[a-z0-9]+(?:-[a-z0-9]+)*$/ - if (!nameFormat.test(effectiveSkillName)) { - console.error( - `Skill name "${effectiveSkillName}" is invalid: must be lowercase letters/numbers/hyphens only (no leading/trailing hyphen, no consecutive hyphens)`, - ) + // Validate skill name per agentskills.io spec using shared validation + const nameValidation = validateSkillNameShared(effectiveSkillName) + if (!nameValidation.valid) { + const errorMessage = this.getSkillNameErrorMessage(effectiveSkillName, nameValidation.error!) + console.error(`Skill name "${effectiveSkillName}" is invalid: ${errorMessage}`) return } @@ -239,6 +234,146 @@ export class SkillsManager { } } + /** + * Get all skills metadata (for UI display) + * Returns skills from all sources without content + */ + getSkillsMetadata(): SkillMetadata[] { + return this.getAllSkills() + } + + /** + * Get a skill by name, source, and optionally mode + */ + getSkill(name: string, source: "global" | "project", mode?: string): SkillMetadata | undefined { + const skillKey = this.getSkillKey(name, source, mode) + return this.skills.get(skillKey) + } + + /** + * Validate skill name per agentskills.io spec using shared validation. + * Converts error codes to user-friendly error messages. + */ + private validateSkillName(name: string): { valid: boolean; error?: string } { + const result = validateSkillNameShared(name) + if (!result.valid) { + return { valid: false, error: this.getSkillNameErrorMessage(name, result.error!) } + } + return { valid: true } + } + + /** + * Convert skill name validation error code to a user-friendly error message. + */ + private getSkillNameErrorMessage(name: string, error: SkillNameValidationError): string { + switch (error) { + case SkillNameValidationError.Empty: + return t("skills:errors.name_length", { maxLength: SKILL_NAME_MAX_LENGTH, length: name.length }) + case SkillNameValidationError.TooLong: + return t("skills:errors.name_length", { maxLength: SKILL_NAME_MAX_LENGTH, length: name.length }) + case SkillNameValidationError.InvalidFormat: + return t("skills:errors.name_format") + } + } + + /** + * Create a new skill + * @param name - Skill name (must be valid per agentskills.io spec) + * @param source - "global" or "project" + * @param description - Skill description + * @param mode - Optional mode restriction (creates in skills-{mode}/ directory) + * @returns Path to created SKILL.md file + */ + async createSkill(name: string, source: "global" | "project", description: string, mode?: string): Promise { + // Validate skill name + const validation = this.validateSkillName(name) + if (!validation.valid) { + throw new Error(validation.error) + } + + // Validate description + const trimmedDescription = description.trim() + if (trimmedDescription.length < 1 || trimmedDescription.length > 1024) { + throw new Error(t("skills:errors.description_length", { length: trimmedDescription.length })) + } + + // Determine base directory + let baseDir: string + if (source === "global") { + baseDir = getGlobalRooDirectory() + } else { + const provider = this.providerRef.deref() + if (!provider?.cwd) { + throw new Error(t("skills:errors.no_workspace")) + } + baseDir = path.join(provider.cwd, ".roo") + } + + // Determine skills directory (with optional mode suffix) + const skillsDirName = mode ? `skills-${mode}` : "skills" + const skillsDir = path.join(baseDir, skillsDirName) + const skillDir = path.join(skillsDir, name) + const skillMdPath = path.join(skillDir, "SKILL.md") + + // Check if skill already exists + if (await fileExists(skillMdPath)) { + throw new Error(t("skills:errors.already_exists", { name, path: skillMdPath })) + } + + // Create the skill directory + await fs.mkdir(skillDir, { recursive: true }) + + // Generate SKILL.md content with frontmatter + const titleName = name + .split("-") + .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) + .join(" ") + + const skillContent = `--- +name: ${name} +description: ${trimmedDescription} +--- + +# ${titleName} + +## Instructions + +Add your skill instructions here. +` + + // Write the SKILL.md file + await fs.writeFile(skillMdPath, skillContent, "utf-8") + + // Refresh skills list + await this.discoverSkills() + + return skillMdPath + } + + /** + * Delete a skill + * @param name - Skill name to delete + * @param source - Where the skill is located + * @param mode - Optional mode (to locate in skills-{mode}/ directory) + */ + async deleteSkill(name: string, source: "global" | "project", mode?: string): Promise { + // Find the skill + const skill = this.getSkill(name, source, mode) + if (!skill) { + const modeInfo = mode ? ` (mode: ${mode})` : "" + throw new Error(t("skills:errors.not_found", { name, source, modeInfo })) + } + + // Get the skill directory (parent of SKILL.md) + const skillDir = path.dirname(skill.path) + + // Delete the entire skill directory + await fs.rm(skillDir, { recursive: true, force: true }) + + // Refresh skills list + await this.discoverSkills() + } + /** * Get all skills directories to scan, including mode-specific directories. */ diff --git a/src/services/skills/__tests__/SkillsManager.spec.ts b/src/services/skills/__tests__/SkillsManager.spec.ts index d568935f15..b12b5c3399 100644 --- a/src/services/skills/__tests__/SkillsManager.spec.ts +++ b/src/services/skills/__tests__/SkillsManager.spec.ts @@ -1,16 +1,29 @@ import * as path from "path" // Use vi.hoisted to ensure mocks are available during hoisting -const { mockStat, mockReadFile, mockReaddir, mockHomedir, mockDirectoryExists, mockFileExists, mockRealpath } = - vi.hoisted(() => ({ - mockStat: vi.fn(), - mockReadFile: vi.fn(), - mockReaddir: vi.fn(), - mockHomedir: vi.fn(), - mockDirectoryExists: vi.fn(), - mockFileExists: vi.fn(), - mockRealpath: vi.fn(), - })) +const { + mockStat, + mockReadFile, + mockReaddir, + mockHomedir, + mockDirectoryExists, + mockFileExists, + mockRealpath, + mockMkdir, + mockWriteFile, + mockRm, +} = vi.hoisted(() => ({ + mockStat: vi.fn(), + mockReadFile: vi.fn(), + mockReaddir: vi.fn(), + mockHomedir: vi.fn(), + mockDirectoryExists: vi.fn(), + mockFileExists: vi.fn(), + mockRealpath: vi.fn(), + mockMkdir: vi.fn(), + mockWriteFile: vi.fn(), + mockRm: vi.fn(), +})) // Platform-agnostic test paths // Use forward slashes for consistency, then normalize with path.normalize @@ -28,11 +41,17 @@ vi.mock("fs/promises", () => ({ readFile: mockReadFile, readdir: mockReaddir, realpath: mockRealpath, + mkdir: mockMkdir, + writeFile: mockWriteFile, + rm: mockRm, }, stat: mockStat, readFile: mockReadFile, readdir: mockReaddir, realpath: mockRealpath, + mkdir: mockMkdir, + writeFile: mockWriteFile, + rm: mockRm, })) // Mock os module @@ -65,6 +84,22 @@ vi.mock("../../roo-config", () => ({ fileExists: mockFileExists, })) +// Mock i18n +vi.mock("../../../i18n", () => ({ + t: (key: string, params?: Record) => { + const translations: Record = { + "skills:errors.name_length": `Skill name must be 1-${params?.maxLength} characters (got ${params?.length})`, + "skills:errors.name_format": + "Skill name must be lowercase letters/numbers/hyphens only (no leading/trailing hyphen, no consecutive hyphens)", + "skills:errors.description_length": `Skill description must be 1-1024 characters (got ${params?.length})`, + "skills:errors.no_workspace": "Cannot create project skill: no workspace folder is open", + "skills:errors.already_exists": `Skill "${params?.name}" already exists at ${params?.path}`, + "skills:errors.not_found": `Skill "${params?.name}" not found in ${params?.source}${params?.modeInfo}`, + } + return translations[key] || key + }, +})) + import { SkillsManager } from "../SkillsManager" import { ClineProvider } from "../../../core/webview/ClineProvider" @@ -829,4 +864,268 @@ description: A test skill expect(skills).toHaveLength(0) }) }) + + describe("getSkillsMetadata", () => { + it("should return all skills metadata", async () => { + const testSkillDir = p(globalSkillsDir, "test-skill") + const testSkillMd = p(testSkillDir, "SKILL.md") + + mockDirectoryExists.mockImplementation(async (dir: string) => { + return dir === globalSkillsDir + }) + + mockRealpath.mockImplementation(async (pathArg: string) => pathArg) + + mockReaddir.mockImplementation(async (dir: string) => { + if (dir === globalSkillsDir) { + return ["test-skill"] + } + return [] + }) + + mockStat.mockImplementation(async (pathArg: string) => { + if (pathArg === testSkillDir) { + return { isDirectory: () => true } + } + throw new Error("Not found") + }) + + mockFileExists.mockImplementation(async (file: string) => { + return file === testSkillMd + }) + + mockReadFile.mockResolvedValue(`--- +name: test-skill +description: A test skill +--- +Instructions`) + + await skillsManager.discoverSkills() + + const metadata = skillsManager.getSkillsMetadata() + + expect(metadata).toHaveLength(1) + expect(metadata[0].name).toBe("test-skill") + expect(metadata[0].description).toBe("A test skill") + }) + }) + + describe("getSkill", () => { + it("should return a skill by name, source, and mode", async () => { + const testSkillDir = p(globalSkillsDir, "test-skill") + const testSkillMd = p(testSkillDir, "SKILL.md") + + mockDirectoryExists.mockImplementation(async (dir: string) => { + return dir === globalSkillsDir + }) + + mockRealpath.mockImplementation(async (pathArg: string) => pathArg) + + mockReaddir.mockImplementation(async (dir: string) => { + if (dir === globalSkillsDir) { + return ["test-skill"] + } + return [] + }) + + mockStat.mockImplementation(async (pathArg: string) => { + if (pathArg === testSkillDir) { + return { isDirectory: () => true } + } + throw new Error("Not found") + }) + + mockFileExists.mockImplementation(async (file: string) => { + return file === testSkillMd + }) + + mockReadFile.mockResolvedValue(`--- +name: test-skill +description: A test skill +--- +Instructions`) + + await skillsManager.discoverSkills() + + const skill = skillsManager.getSkill("test-skill", "global") + + expect(skill).toBeDefined() + expect(skill?.name).toBe("test-skill") + expect(skill?.source).toBe("global") + }) + + it("should return undefined for non-existent skill", async () => { + mockDirectoryExists.mockResolvedValue(false) + mockRealpath.mockImplementation(async (p: string) => p) + mockReaddir.mockResolvedValue([]) + + await skillsManager.discoverSkills() + + const skill = skillsManager.getSkill("non-existent", "global") + + expect(skill).toBeUndefined() + }) + }) + + describe("createSkill", () => { + it("should create a new global skill", async () => { + // Setup: no existing skills + mockDirectoryExists.mockResolvedValue(false) + mockRealpath.mockImplementation(async (p: string) => p) + mockReaddir.mockResolvedValue([]) + mockFileExists.mockResolvedValue(false) + mockMkdir.mockResolvedValue(undefined) + mockWriteFile.mockResolvedValue(undefined) + + const createdPath = await skillsManager.createSkill("new-skill", "global", "A new skill description") + + expect(createdPath).toBe(p(GLOBAL_ROO_DIR, "skills", "new-skill", "SKILL.md")) + expect(mockMkdir).toHaveBeenCalledWith(p(GLOBAL_ROO_DIR, "skills", "new-skill"), { recursive: true }) + expect(mockWriteFile).toHaveBeenCalled() + + // Verify the content written + const writeCall = mockWriteFile.mock.calls[0] + expect(writeCall[0]).toBe(p(GLOBAL_ROO_DIR, "skills", "new-skill", "SKILL.md")) + expect(writeCall[1]).toContain("name: new-skill") + expect(writeCall[1]).toContain("description: A new skill description") + }) + + it("should create a mode-specific skill", async () => { + mockDirectoryExists.mockResolvedValue(false) + mockRealpath.mockImplementation(async (p: string) => p) + mockReaddir.mockResolvedValue([]) + mockFileExists.mockResolvedValue(false) + mockMkdir.mockResolvedValue(undefined) + mockWriteFile.mockResolvedValue(undefined) + + const createdPath = await skillsManager.createSkill("code-skill", "global", "A code skill", "code") + + expect(createdPath).toBe(p(GLOBAL_ROO_DIR, "skills-code", "code-skill", "SKILL.md")) + }) + + it("should create a project skill", async () => { + mockDirectoryExists.mockResolvedValue(false) + mockRealpath.mockImplementation(async (p: string) => p) + mockReaddir.mockResolvedValue([]) + mockFileExists.mockResolvedValue(false) + mockMkdir.mockResolvedValue(undefined) + mockWriteFile.mockResolvedValue(undefined) + + const createdPath = await skillsManager.createSkill("project-skill", "project", "A project skill") + + expect(createdPath).toBe(p(PROJECT_DIR, ".roo", "skills", "project-skill", "SKILL.md")) + }) + + it("should throw error for invalid skill name", async () => { + await expect(skillsManager.createSkill("Invalid-Name", "global", "Description")).rejects.toThrow( + "Skill name must be lowercase letters/numbers/hyphens only", + ) + }) + + it("should throw error for skill name that is too long", async () => { + const longName = "a".repeat(65) + await expect(skillsManager.createSkill(longName, "global", "Description")).rejects.toThrow( + "Skill name must be 1-64 characters", + ) + }) + + it("should throw error for skill name starting with hyphen", async () => { + await expect(skillsManager.createSkill("-invalid", "global", "Description")).rejects.toThrow( + "Skill name must be lowercase letters/numbers/hyphens only", + ) + }) + + it("should throw error for skill name ending with hyphen", async () => { + await expect(skillsManager.createSkill("invalid-", "global", "Description")).rejects.toThrow( + "Skill name must be lowercase letters/numbers/hyphens only", + ) + }) + + it("should throw error for skill name with consecutive hyphens", async () => { + await expect(skillsManager.createSkill("invalid--name", "global", "Description")).rejects.toThrow( + "Skill name must be lowercase letters/numbers/hyphens only", + ) + }) + + it("should throw error for empty description", async () => { + await expect(skillsManager.createSkill("valid-name", "global", " ")).rejects.toThrow( + "Skill description must be 1-1024 characters", + ) + }) + + it("should throw error for description that is too long", async () => { + const longDesc = "d".repeat(1025) + await expect(skillsManager.createSkill("valid-name", "global", longDesc)).rejects.toThrow( + "Skill description must be 1-1024 characters", + ) + }) + + it("should throw error if skill already exists", async () => { + mockFileExists.mockResolvedValue(true) + + await expect(skillsManager.createSkill("existing-skill", "global", "Description")).rejects.toThrow( + "already exists", + ) + }) + }) + + describe("deleteSkill", () => { + it("should delete an existing skill", async () => { + const testSkillDir = p(globalSkillsDir, "test-skill") + const testSkillMd = p(testSkillDir, "SKILL.md") + + // Setup: skill exists + mockDirectoryExists.mockImplementation(async (dir: string) => { + return dir === globalSkillsDir + }) + + mockRealpath.mockImplementation(async (pathArg: string) => pathArg) + + mockReaddir.mockImplementation(async (dir: string) => { + if (dir === globalSkillsDir) { + return ["test-skill"] + } + return [] + }) + + mockStat.mockImplementation(async (pathArg: string) => { + if (pathArg === testSkillDir) { + return { isDirectory: () => true } + } + throw new Error("Not found") + }) + + mockFileExists.mockImplementation(async (file: string) => { + return file === testSkillMd + }) + + mockReadFile.mockResolvedValue(`--- +name: test-skill +description: A test skill +--- +Instructions`) + + mockRm.mockResolvedValue(undefined) + + await skillsManager.discoverSkills() + + // Verify skill exists + expect(skillsManager.getSkill("test-skill", "global")).toBeDefined() + + // Delete the skill + await skillsManager.deleteSkill("test-skill", "global") + + expect(mockRm).toHaveBeenCalledWith(testSkillDir, { recursive: true, force: true }) + }) + + it("should throw error if skill does not exist", async () => { + mockDirectoryExists.mockResolvedValue(false) + mockRealpath.mockImplementation(async (p: string) => p) + mockReaddir.mockResolvedValue([]) + + await skillsManager.discoverSkills() + + await expect(skillsManager.deleteSkill("non-existent", "global")).rejects.toThrow("not found") + }) + }) }) diff --git a/src/shared/__tests__/experiments.spec.ts b/src/shared/__tests__/experiments.spec.ts index c735de05ba..51fca6f6dc 100644 --- a/src/shared/__tests__/experiments.spec.ts +++ b/src/shared/__tests__/experiments.spec.ts @@ -24,7 +24,6 @@ describe("experiments", () => { alwaysIncludeFileDetails: false, useLitePrompts: false, runSlashCommand: false, - multipleNativeToolCalls: false, customTools: false, smartMistakeDetection: false, } @@ -40,7 +39,6 @@ describe("experiments", () => { preventFocusDisruption: true, imageGeneration: false, runSlashCommand: false, - multipleNativeToolCalls: false, customTools: false, smartMistakeDetection: false, } @@ -56,7 +54,6 @@ describe("experiments", () => { preventFocusDisruption: false, imageGeneration: false, runSlashCommand: false, - multipleNativeToolCalls: false, customTools: false, smartMistakeDetection: false, } diff --git a/src/shared/experiments.ts b/src/shared/experiments.ts index 0cc6c5eead..dd05b1e1c6 100644 --- a/src/shared/experiments.ts +++ b/src/shared/experiments.ts @@ -10,7 +10,6 @@ export const EXPERIMENT_IDS = { PREVENT_FOCUS_DISRUPTION: "preventFocusDisruption", IMAGE_GENERATION: "imageGeneration", RUN_SLASH_COMMAND: "runSlashCommand", - MULTIPLE_NATIVE_TOOL_CALLS: "multipleNativeToolCalls", CUSTOM_TOOLS: "customTools", SMART_MISTAKE_DETECTION: "smartMistakeDetection", } as const satisfies Record @@ -32,7 +31,6 @@ export const experimentConfigsMap: Record = { PREVENT_FOCUS_DISRUPTION: { enabled: false }, IMAGE_GENERATION: { enabled: false }, RUN_SLASH_COMMAND: { enabled: false }, - MULTIPLE_NATIVE_TOOL_CALLS: { enabled: false }, CUSTOM_TOOLS: { enabled: false }, } diff --git a/src/utils/fixNativeToolname.ts b/src/utils/fixNativeToolname.ts index 136872924e..7baff38892 100644 --- a/src/utils/fixNativeToolname.ts +++ b/src/utils/fixNativeToolname.ts @@ -27,7 +27,13 @@ export const fixNativeToolname = (toolname: string | ToolName) => { return fixedToolname } -export function fixFinalToolUseResult(input?: string) { +export function fixAskMultipleChoiceFinalToolUseResult(input?: string) { if (!input) return "{}" - return input.replace(/'/g, '"').replace(/\b(False|True)\b/g, (match) => match.toLowerCase()) + + try { + JSON.parse(input as string) // check if it's a valid json + return input + } catch (error) { + return input.replace(/'/g, '"').replace(/\b(False|True)\b/g, (match) => match.toLowerCase()) + } } diff --git a/webview-ui/package.json b/webview-ui/package.json index 752fce1240..6caa4fe645 100644 --- a/webview-ui/package.json +++ b/webview-ui/package.json @@ -59,6 +59,7 @@ "qrcode": "^1.5.4", "react": "^18.3.1", "react-dom": "^18.3.1", + "react-compiler-runtime": "^1.0.0", "react-i18next": "^15.4.1", "react-icons": "^5.5.0", "react-markdown": "^9.0.3", @@ -89,6 +90,7 @@ "zod": "^3.25.61" }, "devDependencies": { + "babel-plugin-react-compiler": "^1.0.0", "@roo-code/config-eslint": "workspace:^", "@roo-code/config-typescript": "workspace:^", "@testing-library/jest-dom": "^6.6.3", diff --git a/webview-ui/src/components/settings/CreateSkillDialog.tsx b/webview-ui/src/components/settings/CreateSkillDialog.tsx new file mode 100644 index 0000000000..a4daa9989c --- /dev/null +++ b/webview-ui/src/components/settings/CreateSkillDialog.tsx @@ -0,0 +1,254 @@ +import React, { useState, useCallback, useMemo } from "react" +import { validateSkillName as validateSkillNameShared, SkillNameValidationError } from "@roo-code/types" + +import { getAllModes } from "@roo/modes" + +import { useAppTranslation } from "@/i18n/TranslationContext" +import { useExtensionState } from "@/context/ExtensionStateContext" +import { + Button, + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui" +import { vscode } from "@/utils/vscode" + +interface CreateSkillDialogProps { + open: boolean + onOpenChange: (open: boolean) => void + onSkillCreated: () => void + hasWorkspace: boolean +} + +/** + * Map skill name validation error codes to translation keys. + */ +const getSkillNameErrorTranslationKey = (error: SkillNameValidationError): string => { + switch (error) { + case SkillNameValidationError.Empty: + return "settings:skills.validation.nameRequired" + case SkillNameValidationError.TooLong: + return "settings:skills.validation.nameTooLong" + case SkillNameValidationError.InvalidFormat: + return "settings:skills.validation.nameInvalid" + } +} + +/** + * Validate skill name using shared validation from @roo-code/types. + * Returns a translation key for the error, or null if valid. + */ +const validateSkillName = (name: string): string | null => { + const result = validateSkillNameShared(name) + if (!result.valid) { + return getSkillNameErrorTranslationKey(result.error!) + } + return null +} + +/** + * Validate description according to agentskills.io spec: + * - Required field + * - 1-1024 characters + */ +const validateDescription = (description: string): string | null => { + if (!description) return "settings:skills.validation.descriptionRequired" + if (description.length > 1024) return "settings:skills.validation.descriptionTooLong" + return null +} + +// Sentinel value for "Any mode" since Radix Select doesn't allow empty string values +const MODE_ANY = "__any__" + +export const CreateSkillDialog: React.FC = ({ + open, + onOpenChange, + onSkillCreated, + hasWorkspace, +}) => { + const { t } = useAppTranslation() + const { customModes } = useExtensionState() + + const [name, setName] = useState("") + const [description, setDescription] = useState("") + const [source, setSource] = useState<"global" | "project">(hasWorkspace ? "project" : "global") + const [mode, setMode] = useState(MODE_ANY) + const [nameError, setNameError] = useState(null) + const [descriptionError, setDescriptionError] = useState(null) + + // Get available modes for the dropdown (built-in + custom modes) + const availableModes = useMemo(() => { + return getAllModes(customModes).map((m) => ({ slug: m.slug, name: m.name })) + }, [customModes]) + + const resetForm = useCallback(() => { + setName("") + setDescription("") + setSource(hasWorkspace ? "project" : "global") + setMode(MODE_ANY) + setNameError(null) + setDescriptionError(null) + }, [hasWorkspace]) + + const handleClose = useCallback(() => { + resetForm() + onOpenChange(false) + }, [resetForm, onOpenChange]) + + const handleNameChange = useCallback((e: React.ChangeEvent) => { + const value = e.target.value.toLowerCase().replace(/[^a-z0-9-]/g, "") + setName(value) + setNameError(null) + }, []) + + const handleDescriptionChange = useCallback((e: React.ChangeEvent) => { + setDescription(e.target.value) + setDescriptionError(null) + }, []) + + const handleCreate = useCallback(() => { + // Validate fields + const nameValidationError = validateSkillName(name) + const descValidationError = validateDescription(description) + + if (nameValidationError) { + setNameError(nameValidationError) + return + } + + if (descValidationError) { + setDescriptionError(descValidationError) + return + } + + // Send message to create skill + // Convert MODE_ANY sentinel value to undefined for the backend + vscode.postMessage({ + type: "createSkill", + skillName: name, + source, + skillDescription: description, + skillMode: mode === MODE_ANY ? undefined : mode, + }) + + // Close dialog and notify parent + handleClose() + onSkillCreated() + }, [name, description, source, mode, handleClose, onSkillCreated]) + + return ( + + + + {t("settings:skills.createDialog.title")} + {t("settings:skills.createDialog.description")} + + +
+ {/* Name Input */} +
+ + + + {t("settings:skills.createDialog.nameHint")} + + {nameError && {t(nameError)}} +
+ + {/* Description Input */} +
+ +