Merge pull request #1554 from code-yeongyu/fix/1187-dynamic-skill-reminder
Fix category-skill-reminder to prioritize user-installed skills
This commit is contained in:
YeonGyu-Kim
GitHub
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, test, beforeEach, afterEach, spyOn } from "bun:test"
|
||||
import { createCategorySkillReminderHook } from "./index"
|
||||
import { updateSessionAgent, clearSessionAgent, _resetForTesting } from "../../features/claude-code-session-state"
|
||||
import type { AvailableSkill } from "../../agents/dynamic-agent-prompt-builder"
|
||||
import * as sharedModule from "../../shared"
|
||||
|
||||
describe("category-skill-reminder hook", () => {
|
||||
@@ -29,10 +30,14 @@ describe("category-skill-reminder hook", () => {
|
||||
} as any
|
||||
}
|
||||
|
||||
function createHook(availableSkills: AvailableSkill[] = []) {
|
||||
return createCategorySkillReminderHook(createMockPluginInput(), availableSkills)
|
||||
}
|
||||
|
||||
describe("target agent detection", () => {
|
||||
test("should inject reminder for sisyphus agent after 3 tool calls", async () => {
|
||||
// given - sisyphus agent session with multiple tool calls
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "sisyphus-session"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
|
||||
@@ -52,7 +57,7 @@ describe("category-skill-reminder hook", () => {
|
||||
|
||||
test("should inject reminder for atlas agent", async () => {
|
||||
// given - atlas agent session
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "atlas-session"
|
||||
updateSessionAgent(sessionID, "Atlas")
|
||||
|
||||
@@ -71,7 +76,7 @@ describe("category-skill-reminder hook", () => {
|
||||
|
||||
test("should inject reminder for sisyphus-junior agent", async () => {
|
||||
// given - sisyphus-junior agent session
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "junior-session"
|
||||
updateSessionAgent(sessionID, "sisyphus-junior")
|
||||
|
||||
@@ -90,7 +95,7 @@ describe("category-skill-reminder hook", () => {
|
||||
|
||||
test("should NOT inject reminder for non-target agents", async () => {
|
||||
// given - librarian agent session (not a target)
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "librarian-session"
|
||||
updateSessionAgent(sessionID, "librarian")
|
||||
|
||||
@@ -109,7 +114,7 @@ describe("category-skill-reminder hook", () => {
|
||||
|
||||
test("should detect agent from input.agent when session state is empty", async () => {
|
||||
// given - no session state, agent provided in input
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "input-agent-session"
|
||||
|
||||
const output = { title: "", output: "result", metadata: {} }
|
||||
@@ -127,7 +132,7 @@ describe("category-skill-reminder hook", () => {
|
||||
describe("delegation tool tracking", () => {
|
||||
test("should NOT inject reminder if delegate_task is used", async () => {
|
||||
// given - sisyphus agent that uses delegate_task
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "delegation-session"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
|
||||
@@ -147,7 +152,7 @@ describe("category-skill-reminder hook", () => {
|
||||
|
||||
test("should NOT inject reminder if call_omo_agent is used", async () => {
|
||||
// given - sisyphus agent that uses call_omo_agent
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "omo-agent-session"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
|
||||
@@ -167,7 +172,7 @@ describe("category-skill-reminder hook", () => {
|
||||
|
||||
test("should NOT inject reminder if task tool is used", async () => {
|
||||
// given - sisyphus agent that uses task tool
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "task-session"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
|
||||
@@ -189,7 +194,7 @@ describe("category-skill-reminder hook", () => {
|
||||
describe("tool call counting", () => {
|
||||
test("should NOT inject reminder before 3 tool calls", async () => {
|
||||
// given - sisyphus agent with only 2 tool calls
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "few-calls-session"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
|
||||
@@ -207,7 +212,7 @@ describe("category-skill-reminder hook", () => {
|
||||
|
||||
test("should only inject reminder once per session", async () => {
|
||||
// given - sisyphus agent session
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "once-session"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
|
||||
@@ -231,7 +236,7 @@ describe("category-skill-reminder hook", () => {
|
||||
|
||||
test("should only count delegatable work tools", async () => {
|
||||
// given - sisyphus agent with mixed tool calls
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "mixed-tools-session"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
|
||||
@@ -252,7 +257,7 @@ describe("category-skill-reminder hook", () => {
|
||||
describe("event handling", () => {
|
||||
test("should reset state on session.deleted event", async () => {
|
||||
// given - sisyphus agent with reminder already shown
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "delete-session"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
|
||||
@@ -278,7 +283,7 @@ describe("category-skill-reminder hook", () => {
|
||||
|
||||
test("should reset state on session.compacted event", async () => {
|
||||
// given - sisyphus agent with reminder already shown
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "compact-session"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
|
||||
@@ -306,7 +311,7 @@ describe("category-skill-reminder hook", () => {
|
||||
describe("case insensitivity", () => {
|
||||
test("should handle tool names case-insensitively", async () => {
|
||||
// given - sisyphus agent with mixed case tool names
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "case-session"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
|
||||
@@ -325,7 +330,7 @@ describe("category-skill-reminder hook", () => {
|
||||
|
||||
test("should handle delegation tool names case-insensitively", async () => {
|
||||
// given - sisyphus agent using DELEGATE_TASK in uppercase
|
||||
const hook = createCategorySkillReminderHook(createMockPluginInput())
|
||||
const hook = createHook()
|
||||
const sessionID = "case-delegate-session"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
|
||||
@@ -343,4 +348,71 @@ describe("category-skill-reminder hook", () => {
|
||||
clearSessionAgent(sessionID)
|
||||
})
|
||||
})
|
||||
|
||||
describe("dynamic skills reminder message", () => {
|
||||
test("shows built-in skills when only built-in skills are available", async () => {
|
||||
// given
|
||||
const availableSkills: AvailableSkill[] = [
|
||||
{ name: "frontend-ui-ux", description: "Frontend UI/UX work", location: "plugin" },
|
||||
{ name: "git-master", description: "Git operations", location: "plugin" },
|
||||
{ name: "playwright", description: "Browser automation", location: "plugin" },
|
||||
]
|
||||
const hook = createHook(availableSkills)
|
||||
const sessionID = "builtins-only"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
const output = { title: "", output: "result", metadata: {} }
|
||||
|
||||
// when
|
||||
await hook["tool.execute.after"]({ tool: "edit", sessionID, callID: "1" }, output)
|
||||
await hook["tool.execute.after"]({ tool: "edit", sessionID, callID: "2" }, output)
|
||||
await hook["tool.execute.after"]({ tool: "edit", sessionID, callID: "3" }, output)
|
||||
|
||||
// then
|
||||
expect(output.output).toContain("**Built-in**:")
|
||||
expect(output.output).toContain("frontend-ui-ux")
|
||||
expect(output.output).toContain("**⚡ YOUR SKILLS (PRIORITY)**")
|
||||
expect(output.output).toContain("load_skills=[\"frontend-ui-ux\"")
|
||||
})
|
||||
|
||||
test("emphasizes user skills with PRIORITY and uses first user skill in example", async () => {
|
||||
// given
|
||||
const availableSkills: AvailableSkill[] = [
|
||||
{ name: "frontend-ui-ux", description: "Frontend UI/UX work", location: "plugin" },
|
||||
{ name: "react-19", description: "React 19 expertise", location: "user" },
|
||||
{ name: "web-designer", description: "Visual design", location: "user" },
|
||||
]
|
||||
const hook = createHook(availableSkills)
|
||||
const sessionID = "user-skills"
|
||||
updateSessionAgent(sessionID, "Atlas")
|
||||
const output = { title: "", output: "result", metadata: {} }
|
||||
|
||||
// when
|
||||
await hook["tool.execute.after"]({ tool: "bash", sessionID, callID: "1" }, output)
|
||||
await hook["tool.execute.after"]({ tool: "bash", sessionID, callID: "2" }, output)
|
||||
await hook["tool.execute.after"]({ tool: "bash", sessionID, callID: "3" }, output)
|
||||
|
||||
// then
|
||||
expect(output.output).toContain("**⚡ YOUR SKILLS (PRIORITY)**")
|
||||
expect(output.output).toContain("react-19")
|
||||
expect(output.output).toContain("> User-installed skills OVERRIDE")
|
||||
expect(output.output).toContain("load_skills=[\"react-19\"")
|
||||
})
|
||||
|
||||
test("still injects a generic reminder when no skills are provided", async () => {
|
||||
// given
|
||||
const hook = createHook([])
|
||||
const sessionID = "no-skills"
|
||||
updateSessionAgent(sessionID, "Sisyphus")
|
||||
const output = { title: "", output: "result", metadata: {} }
|
||||
|
||||
// when
|
||||
await hook["tool.execute.after"]({ tool: "read", sessionID, callID: "1" }, output)
|
||||
await hook["tool.execute.after"]({ tool: "read", sessionID, callID: "2" }, output)
|
||||
await hook["tool.execute.after"]({ tool: "read", sessionID, callID: "3" }, output)
|
||||
|
||||
// then
|
||||
expect(output.output).toContain("[Category+Skill Reminder]")
|
||||
expect(output.output).toContain("load_skills=[]")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type { PluginInput } from "@opencode-ai/plugin"
|
||||
import type { AvailableSkill } from "../../agents/dynamic-agent-prompt-builder"
|
||||
import { getSessionAgent } from "../../features/claude-code-session-state"
|
||||
import { log } from "../../shared"
|
||||
|
||||
@@ -34,33 +35,41 @@ const DELEGATION_TOOLS = new Set([
|
||||
"task",
|
||||
])
|
||||
|
||||
const REMINDER_MESSAGE = `
|
||||
[Category+Skill Reminder]
|
||||
function formatSkillNames(skills: AvailableSkill[], limit: number): string {
|
||||
if (skills.length === 0) return "(none)"
|
||||
const shown = skills.slice(0, limit).map((s) => s.name)
|
||||
const remaining = skills.length - shown.length
|
||||
const suffix = remaining > 0 ? ` (+${remaining} more)` : ""
|
||||
return shown.join(", ") + suffix
|
||||
}
|
||||
|
||||
You are an orchestrator agent. Consider whether this work should be delegated:
|
||||
function buildReminderMessage(availableSkills: AvailableSkill[]): string {
|
||||
const builtinSkills = availableSkills.filter((s) => s.location === "plugin")
|
||||
const customSkills = availableSkills.filter((s) => s.location !== "plugin")
|
||||
|
||||
**DELEGATE when:**
|
||||
- UI/Frontend work → category: "visual-engineering", skills: ["frontend-ui-ux"]
|
||||
- Complex logic/architecture → category: "ultrabrain"
|
||||
- Quick/trivial tasks → category: "quick"
|
||||
- Git operations → skills: ["git-master"]
|
||||
- Browser automation → skills: ["playwright"] or ["agent-browser"]
|
||||
const builtinText = formatSkillNames(builtinSkills, 8)
|
||||
const customText = formatSkillNames(customSkills, 8)
|
||||
|
||||
**DO IT YOURSELF when:**
|
||||
- Gathering context/exploring codebase
|
||||
- Simple edits that are part of a larger task you're coordinating
|
||||
- Tasks requiring your full context understanding
|
||||
const exampleSkillName = customSkills[0]?.name ?? builtinSkills[0]?.name
|
||||
const loadSkills = exampleSkillName ? `["${exampleSkillName}"]` : "[]"
|
||||
|
||||
Example delegation:
|
||||
\`\`\`
|
||||
delegate_task(
|
||||
category="visual-engineering",
|
||||
load_skills=["frontend-ui-ux"],
|
||||
description="Implement responsive navbar with animations",
|
||||
run_in_background=true
|
||||
)
|
||||
\`\`\`
|
||||
`
|
||||
const lines = [
|
||||
"",
|
||||
"[Category+Skill Reminder]",
|
||||
"",
|
||||
`**Built-in**: ${builtinText}`,
|
||||
`**⚡ YOUR SKILLS (PRIORITY)**: ${customText}`,
|
||||
"",
|
||||
"> User-installed skills OVERRIDE built-in defaults. ALWAYS prefer YOUR SKILLS when domain matches.",
|
||||
"",
|
||||
"```typescript",
|
||||
`delegate_task(category=\"visual-engineering\", load_skills=${loadSkills}, run_in_background=true)`,
|
||||
"```",
|
||||
"",
|
||||
]
|
||||
|
||||
return lines.join("\n")
|
||||
}
|
||||
|
||||
interface ToolExecuteInput {
|
||||
tool: string
|
||||
@@ -81,8 +90,12 @@ interface SessionState {
|
||||
toolCallCount: number
|
||||
}
|
||||
|
||||
export function createCategorySkillReminderHook(_ctx: PluginInput) {
|
||||
export function createCategorySkillReminderHook(
|
||||
_ctx: PluginInput,
|
||||
availableSkills: AvailableSkill[] = []
|
||||
) {
|
||||
const sessionStates = new Map<string, SessionState>()
|
||||
const reminderMessage = buildReminderMessage(availableSkills)
|
||||
|
||||
function getOrCreateState(sessionID: string): SessionState {
|
||||
if (!sessionStates.has(sessionID)) {
|
||||
@@ -130,7 +143,7 @@ export function createCategorySkillReminderHook(_ctx: PluginInput) {
|
||||
state.toolCallCount++
|
||||
|
||||
if (state.toolCallCount >= 3 && !state.delegationUsed && !state.reminderShown) {
|
||||
output.output += REMINDER_MESSAGE
|
||||
output.output += reminderMessage
|
||||
state.reminderShown = true
|
||||
log("[category-skill-reminder] Reminder injected", {
|
||||
sessionID,
|
||||
|
||||
12
src/index.ts
12
src/index.ts
@@ -1,4 +1,5 @@
|
||||
import type { Plugin, ToolDefinition } from "@opencode-ai/plugin";
|
||||
import type { AvailableSkill } from "./agents/dynamic-agent-prompt-builder";
|
||||
import {
|
||||
createTodoContinuationEnforcer,
|
||||
createContextWindowMonitorHook,
|
||||
@@ -59,7 +60,6 @@ import {
|
||||
import type { SkillScope } from "./features/opencode-skill-loader/types";
|
||||
import { createBuiltinSkills } from "./features/builtin-skills";
|
||||
import { getSystemMcpServerNames } from "./features/claude-code-mcp-loader";
|
||||
import type { AvailableSkill } from "./agents/dynamic-agent-prompt-builder";
|
||||
import {
|
||||
setMainSession,
|
||||
getMainSessionID,
|
||||
@@ -122,6 +122,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => {
|
||||
|
||||
const pluginConfig = loadPluginConfig(ctx.directory, ctx);
|
||||
const disabledHooks = new Set(pluginConfig.disabled_hooks ?? []);
|
||||
|
||||
const firstMessageVariantGate = createFirstMessageVariantGate();
|
||||
|
||||
const tmuxConfig = {
|
||||
@@ -249,9 +250,7 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => {
|
||||
? createThinkingBlockValidatorHook()
|
||||
: null;
|
||||
|
||||
const categorySkillReminder = isHookEnabled("category-skill-reminder")
|
||||
? createCategorySkillReminderHook(ctx)
|
||||
: null;
|
||||
let categorySkillReminder: ReturnType<typeof createCategorySkillReminderHook> | null = null;
|
||||
|
||||
const ralphLoop = isHookEnabled("ralph-loop")
|
||||
? createRalphLoopHook(ctx, {
|
||||
@@ -483,6 +482,11 @@ const OhMyOpenCodePlugin: Plugin = async (ctx) => {
|
||||
});
|
||||
},
|
||||
});
|
||||
|
||||
categorySkillReminder = isHookEnabled("category-skill-reminder")
|
||||
? createCategorySkillReminderHook(ctx, availableSkills)
|
||||
: null;
|
||||
|
||||
const skillMcpManager = new SkillMcpManager();
|
||||
const getSessionIDForMcp = () => getMainSessionID() || "";
|
||||
const skillTool = createSkillTool({
|
||||
|
||||
Reference in New Issue
Block a user