Compare commits
3 Commits
main
...
fix/skillt
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
1e72fc94cb | ||
|
|
be7ec1b5fa | ||
|
|
cdfaea5ced |
33
src/services/tools/toolExecution.test.ts
Normal file
33
src/services/tools/toolExecution.test.ts
Normal file
@@ -0,0 +1,33 @@
|
||||
import { describe, expect, test } from 'bun:test'
|
||||
|
||||
import { SkillTool } from '../../tools/SkillTool/SkillTool.js'
|
||||
import {
|
||||
getSchemaValidationErrorOverride,
|
||||
getSchemaValidationToolUseResult,
|
||||
} from './toolExecution.js'
|
||||
|
||||
describe('getSchemaValidationErrorOverride', () => {
|
||||
test('returns actionable missing-skill error for SkillTool', () => {
|
||||
expect(getSchemaValidationErrorOverride(SkillTool, {})).toBe(
|
||||
'Missing skill name. Pass the slash command name as the skill parameter (e.g., skill: "commit" for /commit, skill: "review-pr" for /review-pr).',
|
||||
)
|
||||
})
|
||||
|
||||
test('does not override unrelated tool schema failures', () => {
|
||||
expect(getSchemaValidationErrorOverride({ name: 'Read' } as never, {})).toBe(
|
||||
null,
|
||||
)
|
||||
})
|
||||
|
||||
test('does not override SkillTool when skill is present', () => {
|
||||
expect(
|
||||
getSchemaValidationErrorOverride(SkillTool, { skill: 'commit' }),
|
||||
).toBe(null)
|
||||
})
|
||||
|
||||
test('uses the actionable override for structured toolUseResult too', () => {
|
||||
expect(getSchemaValidationToolUseResult(SkillTool, {} as never)).toBe(
|
||||
'InputValidationError: Missing skill name. Pass the slash command name as the skill parameter (e.g., skill: "commit" for /commit, skill: "review-pr" for /review-pr).',
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -43,6 +43,7 @@ import { FILE_READ_TOOL_NAME } from '../../tools/FileReadTool/prompt.js'
|
||||
import { FILE_WRITE_TOOL_NAME } from '../../tools/FileWriteTool/prompt.js'
|
||||
import { NOTEBOOK_EDIT_TOOL_NAME } from '../../tools/NotebookEditTool/constants.js'
|
||||
import { POWERSHELL_TOOL_NAME } from '../../tools/PowerShellTool/toolName.js'
|
||||
import { SKILL_TOOL_NAME } from '../../tools/SkillTool/constants.js'
|
||||
import { parseGitCommitId } from '../../tools/shared/gitOperationTracking.js'
|
||||
import {
|
||||
isDeferredTool,
|
||||
@@ -596,6 +597,31 @@ export function buildSchemaNotSentHint(
|
||||
)
|
||||
}
|
||||
|
||||
export function getSchemaValidationErrorOverride(
|
||||
tool: Tool,
|
||||
input: unknown,
|
||||
): string | null {
|
||||
if (tool.name !== SKILL_TOOL_NAME || !input || typeof input !== 'object') {
|
||||
return null
|
||||
}
|
||||
|
||||
const skill = (input as { skill?: unknown }).skill
|
||||
if (skill === undefined || skill === null) {
|
||||
return 'Missing skill name. Pass the slash command name as the skill parameter (e.g., skill: "commit" for /commit, skill: "review-pr" for /review-pr).'
|
||||
}
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
export function getSchemaValidationToolUseResult(
|
||||
tool: Tool,
|
||||
input: unknown,
|
||||
fallbackMessage?: string,
|
||||
): string {
|
||||
const override = getSchemaValidationErrorOverride(tool, input)
|
||||
return `InputValidationError: ${override ?? fallbackMessage ?? ''}`
|
||||
}
|
||||
|
||||
async function checkPermissionsAndCallTool(
|
||||
tool: Tool,
|
||||
toolUseID: string,
|
||||
@@ -614,7 +640,9 @@ async function checkPermissionsAndCallTool(
|
||||
// Validate input types with zod (surprisingly, the model is not great at generating valid input)
|
||||
const parsedInput = tool.inputSchema.safeParse(input)
|
||||
if (!parsedInput.success) {
|
||||
let errorContent = formatZodValidationError(tool.name, parsedInput.error)
|
||||
const fallbackErrorContent = formatZodValidationError(tool.name, parsedInput.error)
|
||||
let errorContent =
|
||||
getSchemaValidationErrorOverride(tool, input) ?? fallbackErrorContent
|
||||
|
||||
const schemaHint = buildSchemaNotSentHint(
|
||||
tool,
|
||||
@@ -672,7 +700,11 @@ async function checkPermissionsAndCallTool(
|
||||
tool_use_id: toolUseID,
|
||||
},
|
||||
],
|
||||
toolUseResult: `InputValidationError: ${parsedInput.error.message}`,
|
||||
toolUseResult: getSchemaValidationToolUseResult(
|
||||
tool,
|
||||
input,
|
||||
parsedInput.error.message,
|
||||
),
|
||||
sourceToolAssistantUUID: assistantMessage.uuid,
|
||||
}),
|
||||
},
|
||||
|
||||
31
src/tools/SkillTool/SkillTool.test.ts
Normal file
31
src/tools/SkillTool/SkillTool.test.ts
Normal file
@@ -0,0 +1,31 @@
|
||||
import { describe, expect, test } from 'bun:test'
|
||||
|
||||
import { SkillTool } from './SkillTool.js'
|
||||
|
||||
describe('SkillTool missing parameter handling', () => {
|
||||
test('missing skill stays required at the schema level', async () => {
|
||||
const parsed = SkillTool.inputSchema.safeParse({})
|
||||
|
||||
expect(parsed.success).toBe(false)
|
||||
})
|
||||
|
||||
test('validateInput still returns an actionable error when called with missing skill', async () => {
|
||||
const result = await SkillTool.validateInput?.({} as never, {
|
||||
options: { tools: [] },
|
||||
messages: [],
|
||||
} as never)
|
||||
|
||||
expect(result).toEqual({
|
||||
result: false,
|
||||
message:
|
||||
'Missing skill name. Pass the slash command name as the skill parameter (e.g., skill: "commit" for /commit, skill: "review-pr" for /review-pr).',
|
||||
errorCode: 1,
|
||||
})
|
||||
})
|
||||
|
||||
test('valid skill input still parses and validates', async () => {
|
||||
const parsed = SkillTool.inputSchema.safeParse({ skill: 'commit' })
|
||||
|
||||
expect(parsed.success).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -352,6 +352,16 @@ export const SkillTool: Tool<InputSchema, Output, Progress> = buildTool({
|
||||
toAutoClassifierInput: ({ skill }) => skill ?? '',
|
||||
|
||||
async validateInput({ skill }, context): Promise<ValidationResult> {
|
||||
if (!skill || typeof skill !== 'string') {
|
||||
return {
|
||||
result: false,
|
||||
message:
|
||||
'Missing skill name. Pass the slash command name as the skill parameter ' +
|
||||
'(e.g., skill: "commit" for /commit, skill: "review-pr" for /review-pr).',
|
||||
errorCode: 1,
|
||||
}
|
||||
}
|
||||
|
||||
// Skills are just skill names, no arguments
|
||||
const trimmed = skill.trim()
|
||||
if (!trimmed) {
|
||||
@@ -434,7 +444,7 @@ export const SkillTool: Tool<InputSchema, Output, Progress> = buildTool({
|
||||
context,
|
||||
): Promise<PermissionDecision> {
|
||||
// Skills are just skill names, no arguments
|
||||
const trimmed = skill.trim()
|
||||
const trimmed = skill ?? ''
|
||||
|
||||
// Remove leading slash if present (for compatibility)
|
||||
const commandName = trimmed.startsWith('/') ? trimmed.substring(1) : trimmed
|
||||
@@ -592,7 +602,7 @@ export const SkillTool: Tool<InputSchema, Output, Progress> = buildTool({
|
||||
// - Skill is a prompt-based skill
|
||||
|
||||
// Skills are just names, with optional arguments
|
||||
const trimmed = skill.trim()
|
||||
const trimmed = skill ?? ''
|
||||
|
||||
// Remove leading slash if present (for compatibility)
|
||||
const commandName = trimmed.startsWith('/') ? trimmed.substring(1) : trimmed
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { expect, test } from 'bun:test'
|
||||
import { z } from 'zod/v4'
|
||||
import { getEmptyToolPermissionContext, type Tool, type Tools } from '../Tool.js'
|
||||
import { SkillTool } from '../tools/SkillTool/SkillTool.js'
|
||||
import { toolToAPISchema } from './api.js'
|
||||
|
||||
test('toolToAPISchema preserves provider-specific schema keywords in input_schema', async () => {
|
||||
@@ -64,3 +65,16 @@ test('toolToAPISchema preserves provider-specific schema keywords in input_schem
|
||||
},
|
||||
})
|
||||
})
|
||||
|
||||
test('toolToAPISchema keeps skill required for SkillTool', async () => {
|
||||
const schema = await toolToAPISchema(SkillTool, {
|
||||
getToolPermissionContext: async () => getEmptyToolPermissionContext(),
|
||||
tools: [] as unknown as Tools,
|
||||
agents: [],
|
||||
})
|
||||
|
||||
expect((schema as { input_schema: unknown }).input_schema).toMatchObject({
|
||||
type: 'object',
|
||||
required: ['skill'],
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user