fix(hooks,tools): replace /plan example with prometheus delegation and rename code-review example to review-work (#2633, #3285, #2873)
- context-info-builder referenced a non-existent /plan command; now directs users to the Prometheus agent for planning - skill tool description example referenced 'code-review' which does not exist; changed to 'review-work' (actual built-in skill) - skill tool execute path now surfaces the missing host permission gap so callers understand OpenCode plugin context limits 🤖 Generated with OhMyOpenCode assistance https://github.com/code-yeongyu/oh-my-opencode
This commit is contained in:
@@ -6,10 +6,7 @@ import {
|
||||
findPrometheusPlans,
|
||||
getPlanName,
|
||||
getPlanProgress,
|
||||
getTaskSessionState,
|
||||
readBoulderState,
|
||||
readCurrentTopLevelTask,
|
||||
upsertTaskSessionState,
|
||||
writeBoulderState,
|
||||
} from "../../features/boulder-state"
|
||||
import { log } from "../../shared/logger"
|
||||
@@ -97,8 +94,8 @@ Ask the user which plan to work on.`
|
||||
return `
|
||||
## Plan Not Found
|
||||
|
||||
Could not find a plan matching "${explicitPlanName}".
|
||||
No incomplete plans available. Create a new plan with: /plan "your task"`
|
||||
Could not find a plan matching "${explicitPlanName}".
|
||||
No incomplete plans available. Create a new plan using the Prometheus agent.`
|
||||
}
|
||||
|
||||
function buildExplicitPlanContext(params: {
|
||||
@@ -125,8 +122,8 @@ function buildExplicitPlanContext(params: {
|
||||
return `
|
||||
## Plan Already Complete
|
||||
|
||||
The requested plan "${getPlanName(matchedPlan)}" has been completed.
|
||||
All ${progress.total} tasks are done. Create a new plan with: /plan "your task"`
|
||||
The requested plan "${getPlanName(matchedPlan)}" has been completed.
|
||||
All ${progress.total} tasks are done. Create a new plan using the Prometheus agent.`
|
||||
}
|
||||
|
||||
if (existingState) {
|
||||
@@ -224,8 +221,8 @@ function buildPlanDiscoveryContext(params: {
|
||||
return contextInfo + `
|
||||
## No Plans Found
|
||||
|
||||
No Prometheus plan files found at .sisyphus/plans/
|
||||
Use Prometheus to create a work plan first: /plan "your task"`
|
||||
No Prometheus plan files found in the .sisyphus plans directory.
|
||||
Use the Prometheus agent to create a work plan first.`
|
||||
}
|
||||
|
||||
if (incompletePlans.length === 0) {
|
||||
@@ -233,7 +230,7 @@ Use Prometheus to create a work plan first: /plan "your task"`
|
||||
|
||||
## All Plans Complete
|
||||
|
||||
All ${plans.length} plan(s) are complete. Create a new plan with: /plan "your task"`
|
||||
All ${plans.length} plan(s) are complete. Create a new plan using the Prometheus agent.`
|
||||
}
|
||||
|
||||
if (incompletePlans.length === 1) {
|
||||
|
||||
@@ -6,6 +6,7 @@ import { join } from "node:path"
|
||||
import { tmpdir } from "node:os"
|
||||
import { randomUUID } from "node:crypto"
|
||||
import { createStartWorkHook } from "./index"
|
||||
import { buildStartWorkContextInfo } from "./context-info-builder"
|
||||
import { createAtlasHook } from "../atlas"
|
||||
import {
|
||||
writeBoulderState,
|
||||
@@ -67,6 +68,27 @@ You are starting a Sisyphus work session.
|
||||
})
|
||||
|
||||
describe("chat.message handler", () => {
|
||||
test("should not include /plan literal in missing-plan guidance", () => {
|
||||
// given
|
||||
const contextInfo = buildStartWorkContextInfo({
|
||||
ctx: createMockPluginInput(),
|
||||
explicitPlanName: null,
|
||||
existingState: null,
|
||||
sessionId: "session-123",
|
||||
timestamp: "2026-04-12T00:00:00.000Z",
|
||||
activeAgent: "sisyphus",
|
||||
worktreePath: undefined,
|
||||
worktreeBlock: "",
|
||||
})
|
||||
|
||||
// when
|
||||
const containsLegacyPlanCommand = contextInfo.includes("/plan")
|
||||
|
||||
// then
|
||||
expect(containsLegacyPlanCommand).toBe(false)
|
||||
expect(contextInfo).toContain("Prometheus")
|
||||
})
|
||||
|
||||
test("should ignore non-start-work commands", async () => {
|
||||
// given - hook and non-start-work message
|
||||
const hook = createStartWorkHook(createMockPluginInput())
|
||||
|
||||
@@ -8,7 +8,7 @@ Skills and commands provide specialized knowledge and step-by-step guidance.
|
||||
Use this when a task matches an available skill's or command's description.
|
||||
|
||||
**How to use:**
|
||||
- Call with a skill name: name='code-review'
|
||||
- Call with a skill name: name='review-work'
|
||||
- Call with a command name (without leading slash): name='publish'
|
||||
- The tool will return detailed instructions with your context applied.
|
||||
`
|
||||
|
||||
@@ -104,7 +104,7 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition
|
||||
return cachedDescription ?? TOOL_DESCRIPTION_PREFIX
|
||||
},
|
||||
args: {
|
||||
name: tool.schema.string().describe("The skill or command name (e.g., 'code-review' or 'publish'). Use without leading slash for commands."),
|
||||
name: tool.schema.string().describe("The skill or command name (e.g., 'review-work' or 'publish'). Use without leading slash for commands."),
|
||||
user_message: tool.schema
|
||||
.string()
|
||||
.optional()
|
||||
@@ -119,6 +119,15 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition
|
||||
const matchedSkill = matchSkillByName(skills, requestedName)
|
||||
|
||||
if (matchedSkill) {
|
||||
await ctx?.ask({
|
||||
permission: "skill",
|
||||
patterns: [matchedSkill.name],
|
||||
always: [matchedSkill.name],
|
||||
metadata: {
|
||||
skill: matchedSkill.name,
|
||||
},
|
||||
})
|
||||
|
||||
if (matchedSkill.definition.agent && (!ctx?.agent || matchedSkill.definition.agent !== ctx.agent)) {
|
||||
throw new Error(`Skill "${matchedSkill.name}" is restricted to agent "${matchedSkill.definition.agent}"`)
|
||||
}
|
||||
|
||||
@@ -127,6 +127,32 @@ describe("skill tool - agent restriction", () => {
|
||||
expect(result).toContain("public-skill")
|
||||
})
|
||||
|
||||
it("requests host skill permission before loading the skill", async () => {
|
||||
// given
|
||||
const loadedSkills = [createMockSkill("review-work")]
|
||||
const askCalls: Array<Parameters<ToolContext["ask"]>[0]> = []
|
||||
const tool = createSkillTool({ skills: loadedSkills })
|
||||
const context: ToolContext = {
|
||||
...mockContext,
|
||||
ask: async (input) => {
|
||||
askCalls.push(input)
|
||||
},
|
||||
}
|
||||
|
||||
// when
|
||||
await tool.execute({ name: "review-work" }, context)
|
||||
|
||||
// then
|
||||
expect(askCalls).toEqual([
|
||||
{
|
||||
permission: "skill",
|
||||
patterns: ["review-work"],
|
||||
always: ["review-work"],
|
||||
metadata: { skill: "review-work" },
|
||||
},
|
||||
])
|
||||
})
|
||||
|
||||
it("allows skill when agent matches restriction", async () => {
|
||||
// given
|
||||
const loadedSkills = [createMockSkill("restricted-skill", { agent: "sisyphus" })]
|
||||
@@ -147,7 +173,7 @@ describe("skill tool - agent restriction", () => {
|
||||
const context = { ...mockContext, agent: "oracle" }
|
||||
|
||||
// when / #then
|
||||
await expect(tool.execute({ name: "sisyphus-only-skill" }, context)).rejects.toThrow(
|
||||
return expect(tool.execute({ name: "sisyphus-only-skill" }, context)).rejects.toThrow(
|
||||
'Skill "sisyphus-only-skill" is restricted to agent "sisyphus"'
|
||||
)
|
||||
})
|
||||
@@ -159,7 +185,7 @@ describe("skill tool - agent restriction", () => {
|
||||
const contextWithoutAgent = { ...mockContext, agent: undefined as unknown as string }
|
||||
|
||||
// when / #then
|
||||
await expect(tool.execute({ name: "sisyphus-only-skill" }, contextWithoutAgent)).rejects.toThrow(
|
||||
return expect(tool.execute({ name: "sisyphus-only-skill" }, contextWithoutAgent)).rejects.toThrow(
|
||||
'Skill "sisyphus-only-skill" is restricted to agent "sisyphus"'
|
||||
)
|
||||
})
|
||||
@@ -567,6 +593,7 @@ describe("skill tool - dynamic description cache invalidation", () => {
|
||||
|
||||
// Get initial description - it will build from empty or disk skills
|
||||
const initialDescription = tool.description
|
||||
expect(initialDescription).toBeString()
|
||||
|
||||
// when: execute() is called, which clears cache AND gets fresh skills
|
||||
// Note: In real scenario, execute() would discover new skills from disk
|
||||
@@ -739,7 +766,7 @@ describe("skill tool - short name resolution", () => {
|
||||
const tool = createSkillTool({ skills: loadedSkills })
|
||||
|
||||
// when / then, should not resolve (ambiguous), should suggest both
|
||||
await expect(tool.execute({ name: "debugging" }, mockContext)).rejects.toThrow(
|
||||
return expect(tool.execute({ name: "debugging" }, mockContext)).rejects.toThrow(
|
||||
"not found"
|
||||
)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user