Merge pull request #1831 from code-yeongyu/fix/issue-1701-load-skills-string
fix(delegate-task): parse load_skills when passed as JSON string
This commit is contained in:
commit
a8d26e3f74
@ -3,10 +3,12 @@ const { describe, test, expect, beforeEach, afterEach, spyOn, mock } = require("
|
|||||||
import { DEFAULT_CATEGORIES, CATEGORY_PROMPT_APPENDS, CATEGORY_DESCRIPTIONS, isPlanAgent, PLAN_AGENT_NAMES, isPlanFamily, PLAN_FAMILY_NAMES } from "./constants"
|
import { DEFAULT_CATEGORIES, CATEGORY_PROMPT_APPENDS, CATEGORY_DESCRIPTIONS, isPlanAgent, PLAN_AGENT_NAMES, isPlanFamily, PLAN_FAMILY_NAMES } from "./constants"
|
||||||
import { resolveCategoryConfig } from "./tools"
|
import { resolveCategoryConfig } from "./tools"
|
||||||
import type { CategoryConfig } from "../../config/schema"
|
import type { CategoryConfig } from "../../config/schema"
|
||||||
|
import type { DelegateTaskArgs } from "./types"
|
||||||
import { __resetModelCache } from "../../shared/model-availability"
|
import { __resetModelCache } from "../../shared/model-availability"
|
||||||
import { clearSkillCache } from "../../features/opencode-skill-loader/skill-content"
|
import { clearSkillCache } from "../../features/opencode-skill-loader/skill-content"
|
||||||
import { __setTimingConfig, __resetTimingConfig } from "./timing"
|
import { __setTimingConfig, __resetTimingConfig } from "./timing"
|
||||||
import * as connectedProvidersCache from "../../shared/connected-providers-cache"
|
import * as connectedProvidersCache from "../../shared/connected-providers-cache"
|
||||||
|
import * as executor from "./executor"
|
||||||
|
|
||||||
const SYSTEM_DEFAULT_MODEL = "anthropic/claude-sonnet-4-5"
|
const SYSTEM_DEFAULT_MODEL = "anthropic/claude-sonnet-4-5"
|
||||||
|
|
||||||
@ -21,6 +23,10 @@ const TEST_AVAILABLE_MODELS = new Set([
|
|||||||
"openai/gpt-5.3-codex",
|
"openai/gpt-5.3-codex",
|
||||||
])
|
])
|
||||||
|
|
||||||
|
type DelegateTaskArgsWithSerializedSkills = Omit<DelegateTaskArgs, "load_skills"> & {
|
||||||
|
load_skills: string
|
||||||
|
}
|
||||||
|
|
||||||
function createTestAvailableModels(): Set<string> {
|
function createTestAvailableModels(): Set<string> {
|
||||||
return new Set(TEST_AVAILABLE_MODELS)
|
return new Set(TEST_AVAILABLE_MODELS)
|
||||||
}
|
}
|
||||||
@ -256,6 +262,134 @@ describe("sisyphus-task", () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("load_skills parsing", () => {
|
||||||
|
test("parses valid JSON string into array before validation", async () => {
|
||||||
|
//#given
|
||||||
|
const { createDelegateTask } = require("./tools")
|
||||||
|
|
||||||
|
const mockManager = {
|
||||||
|
launch: async () => ({
|
||||||
|
id: "task-123",
|
||||||
|
status: "pending",
|
||||||
|
description: "Parse test",
|
||||||
|
agent: "sisyphus-junior",
|
||||||
|
sessionID: "test-session",
|
||||||
|
}),
|
||||||
|
}
|
||||||
|
|
||||||
|
const mockClient = {
|
||||||
|
app: { agents: async () => ({ data: [] }) },
|
||||||
|
config: { get: async () => ({}) },
|
||||||
|
provider: { list: async () => ({ data: { connected: ["openai"] } }) },
|
||||||
|
model: { list: async () => ({ data: [{ provider: "openai", id: "gpt-5.3-codex" }] }) },
|
||||||
|
session: {
|
||||||
|
create: async () => ({ data: { id: "test-session" } }),
|
||||||
|
prompt: async () => ({ data: {} }),
|
||||||
|
promptAsync: async () => ({ data: {} }),
|
||||||
|
messages: async () => ({ data: [] }),
|
||||||
|
status: async () => ({ data: {} }),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
const tool = createDelegateTask({
|
||||||
|
manager: mockManager,
|
||||||
|
client: mockClient,
|
||||||
|
connectedProvidersOverride: TEST_CONNECTED_PROVIDERS,
|
||||||
|
availableModelsOverride: createTestAvailableModels(),
|
||||||
|
})
|
||||||
|
|
||||||
|
const toolContext = {
|
||||||
|
sessionID: "parent-session",
|
||||||
|
messageID: "parent-message",
|
||||||
|
agent: "sisyphus",
|
||||||
|
abort: new AbortController().signal,
|
||||||
|
}
|
||||||
|
|
||||||
|
const resolveSkillContentSpy = spyOn(executor, "resolveSkillContent").mockResolvedValue({
|
||||||
|
content: "resolved skill content",
|
||||||
|
error: null,
|
||||||
|
})
|
||||||
|
|
||||||
|
const args: DelegateTaskArgsWithSerializedSkills = {
|
||||||
|
description: "Parse valid string",
|
||||||
|
prompt: "Load skill parsing test",
|
||||||
|
category: "quick",
|
||||||
|
run_in_background: true,
|
||||||
|
load_skills: '["playwright", "git-master"]',
|
||||||
|
}
|
||||||
|
|
||||||
|
//#when
|
||||||
|
await tool.execute(args as unknown as DelegateTaskArgs, toolContext)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(args.load_skills).toEqual(["playwright", "git-master"])
|
||||||
|
expect(resolveSkillContentSpy).toHaveBeenCalledWith(["playwright", "git-master"], expect.any(Object))
|
||||||
|
}, { timeout: 10000 })
|
||||||
|
|
||||||
|
test("defaults to [] when load_skills is malformed JSON", async () => {
|
||||||
|
//#given
|
||||||
|
const { createDelegateTask } = require("./tools")
|
||||||
|
|
||||||
|
const mockManager = {
|
||||||
|
launch: async () => ({
|
||||||
|
id: "task-456",
|
||||||
|
status: "pending",
|
||||||
|
description: "Parse test",
|
||||||
|
agent: "sisyphus-junior",
|
||||||
|
sessionID: "test-session",
|
||||||
|
}),
|
||||||
|
}
|
||||||
|
|
||||||
|
const mockClient = {
|
||||||
|
app: { agents: async () => ({ data: [] }) },
|
||||||
|
config: { get: async () => ({}) },
|
||||||
|
provider: { list: async () => ({ data: { connected: ["openai"] } }) },
|
||||||
|
model: { list: async () => ({ data: [{ provider: "openai", id: "gpt-5.3-codex" }] }) },
|
||||||
|
session: {
|
||||||
|
create: async () => ({ data: { id: "test-session" } }),
|
||||||
|
prompt: async () => ({ data: {} }),
|
||||||
|
promptAsync: async () => ({ data: {} }),
|
||||||
|
messages: async () => ({ data: [] }),
|
||||||
|
status: async () => ({ data: {} }),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
const tool = createDelegateTask({
|
||||||
|
manager: mockManager,
|
||||||
|
client: mockClient,
|
||||||
|
connectedProvidersOverride: TEST_CONNECTED_PROVIDERS,
|
||||||
|
availableModelsOverride: createTestAvailableModels(),
|
||||||
|
})
|
||||||
|
|
||||||
|
const toolContext = {
|
||||||
|
sessionID: "parent-session",
|
||||||
|
messageID: "parent-message",
|
||||||
|
agent: "sisyphus",
|
||||||
|
abort: new AbortController().signal,
|
||||||
|
}
|
||||||
|
|
||||||
|
const resolveSkillContentSpy = spyOn(executor, "resolveSkillContent").mockResolvedValue({
|
||||||
|
content: "resolved skill content",
|
||||||
|
error: null,
|
||||||
|
})
|
||||||
|
|
||||||
|
const args: DelegateTaskArgsWithSerializedSkills = {
|
||||||
|
description: "Parse malformed string",
|
||||||
|
prompt: "Load skill parsing test",
|
||||||
|
category: "quick",
|
||||||
|
run_in_background: true,
|
||||||
|
load_skills: '["playwright", "git-master"',
|
||||||
|
}
|
||||||
|
|
||||||
|
//#when
|
||||||
|
await tool.execute(args as unknown as DelegateTaskArgs, toolContext)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(args.load_skills).toEqual([])
|
||||||
|
expect(resolveSkillContentSpy).toHaveBeenCalledWith([], expect.any(Object))
|
||||||
|
}, { timeout: 10000 })
|
||||||
|
})
|
||||||
|
|
||||||
describe("category delegation config validation", () => {
|
describe("category delegation config validation", () => {
|
||||||
test("fills subagent_type as sisyphus-junior when category is provided without subagent_type", async () => {
|
test("fills subagent_type as sisyphus-junior when category is provided without subagent_type", async () => {
|
||||||
// given
|
// given
|
||||||
|
|||||||
@ -103,6 +103,14 @@ Prompts MUST be in English.`
|
|||||||
if (args.run_in_background === undefined) {
|
if (args.run_in_background === undefined) {
|
||||||
throw new Error(`Invalid arguments: 'run_in_background' parameter is REQUIRED. Use run_in_background=false for task delegation, run_in_background=true only for parallel exploration.`)
|
throw new Error(`Invalid arguments: 'run_in_background' parameter is REQUIRED. Use run_in_background=false for task delegation, run_in_background=true only for parallel exploration.`)
|
||||||
}
|
}
|
||||||
|
if (typeof args.load_skills === "string") {
|
||||||
|
try {
|
||||||
|
const parsed = JSON.parse(args.load_skills)
|
||||||
|
args.load_skills = Array.isArray(parsed) ? parsed : []
|
||||||
|
} catch {
|
||||||
|
args.load_skills = []
|
||||||
|
}
|
||||||
|
}
|
||||||
if (args.load_skills === undefined) {
|
if (args.load_skills === undefined) {
|
||||||
throw new Error(`Invalid arguments: 'load_skills' parameter is REQUIRED. Pass [] if no skills needed, but IT IS HIGHLY RECOMMENDED to pass proper skills like ["playwright"], ["git-master"] for best results.`)
|
throw new Error(`Invalid arguments: 'load_skills' parameter is REQUIRED. Pass [] if no skills needed, but IT IS HIGHLY RECOMMENDED to pass proper skills like ["playwright"], ["git-master"] for best results.`)
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user