diff --git a/assets/oh-my-opencode.schema.json b/assets/oh-my-opencode.schema.json index 6cb00bd75..f2eb3fae6 100644 --- a/assets/oh-my-opencode.schema.json +++ b/assets/oh-my-opencode.schema.json @@ -69,7 +69,6 @@ "directory-readme-injector", "empty-task-response-detector", "think-mode", - "subagent-question-blocker", "anthropic-context-window-limit-recovery", "preemptive-compaction", "rules-injector", diff --git a/src/config/schema/hooks.ts b/src/config/schema/hooks.ts index bb5f6bdb0..add671887 100644 --- a/src/config/schema/hooks.ts +++ b/src/config/schema/hooks.ts @@ -13,7 +13,6 @@ export const HookNameSchema = z.enum([ "directory-readme-injector", "empty-task-response-detector", "think-mode", - "subagent-question-blocker", "anthropic-context-window-limit-recovery", "preemptive-compaction", "rules-injector", diff --git a/src/hooks/index.ts b/src/hooks/index.ts index 954475277..fcaf1dad5 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -36,7 +36,6 @@ export { createStartWorkHook } from "./start-work"; export { createAtlasHook } from "./atlas"; export { createDelegateTaskRetryHook } from "./delegate-task-retry"; export { createQuestionLabelTruncatorHook } from "./question-label-truncator"; -export { createSubagentQuestionBlockerHook } from "./subagent-question-blocker"; export { createStopContinuationGuardHook, type StopContinuationGuard } from "./stop-continuation-guard"; export { createCompactionContextInjector } from "./compaction-context-injector"; export { createCompactionTodoPreserverHook } from "./compaction-todo-preserver"; diff --git a/src/hooks/subagent-question-blocker/hook.ts b/src/hooks/subagent-question-blocker/hook.ts deleted file mode 100644 index fc64fae77..000000000 --- a/src/hooks/subagent-question-blocker/hook.ts +++ /dev/null @@ -1,29 +0,0 @@ -import type { Hooks } from "@opencode-ai/plugin" -import { subagentSessions } from "../../features/claude-code-session-state" -import { log } from "../../shared" - -export function createSubagentQuestionBlockerHook(): Hooks { - return { - "tool.execute.before": async (input) => { - const toolName = input.tool?.toLowerCase() - if (toolName !== "question" && toolName !== "askuserquestion") { - return - } - - if (!subagentSessions.has(input.sessionID)) { - return - } - - log("[subagent-question-blocker] Blocking question tool call from subagent session", { - sessionID: input.sessionID, - tool: input.tool, - }) - - throw new Error( - "Question tool is disabled for subagent sessions. " + - "Subagents should complete their work autonomously without asking questions to users. " + - "If you need clarification, return to the parent agent with your findings and uncertainties." - ) - }, - } -} diff --git a/src/hooks/subagent-question-blocker/index.test.ts b/src/hooks/subagent-question-blocker/index.test.ts deleted file mode 100644 index ea75d3cd0..000000000 --- a/src/hooks/subagent-question-blocker/index.test.ts +++ /dev/null @@ -1,82 +0,0 @@ -import { describe, test, expect, beforeEach } from "bun:test" -import { createSubagentQuestionBlockerHook } from "./index" -import { subagentSessions, _resetForTesting } from "../../features/claude-code-session-state" - -describe("createSubagentQuestionBlockerHook", () => { - const hook = createSubagentQuestionBlockerHook() - - beforeEach(() => { - _resetForTesting() - }) - - describe("tool.execute.before", () => { - test("allows question tool for non-subagent sessions", async () => { - // given - const sessionID = "ses_main" - const input = { tool: "question", sessionID, callID: "call_1" } - const output = { args: { questions: [] } } - - // when - const result = hook["tool.execute.before"]?.(input as any, output as any) - - // then - await expect(result).resolves.toBeUndefined() - }) - - test("blocks question tool for subagent sessions", async () => { - // given - const sessionID = "ses_subagent" - subagentSessions.add(sessionID) - const input = { tool: "question", sessionID, callID: "call_1" } - const output = { args: { questions: [] } } - - // when - const result = hook["tool.execute.before"]?.(input as any, output as any) - - // then - await expect(result).rejects.toThrow("Question tool is disabled for subagent sessions") - }) - - test("blocks Question tool (case insensitive) for subagent sessions", async () => { - // given - const sessionID = "ses_subagent" - subagentSessions.add(sessionID) - const input = { tool: "Question", sessionID, callID: "call_1" } - const output = { args: { questions: [] } } - - // when - const result = hook["tool.execute.before"]?.(input as any, output as any) - - // then - await expect(result).rejects.toThrow("Question tool is disabled for subagent sessions") - }) - - test("blocks AskUserQuestion tool for subagent sessions", async () => { - // given - const sessionID = "ses_subagent" - subagentSessions.add(sessionID) - const input = { tool: "AskUserQuestion", sessionID, callID: "call_1" } - const output = { args: { questions: [] } } - - // when - const result = hook["tool.execute.before"]?.(input as any, output as any) - - // then - await expect(result).rejects.toThrow("Question tool is disabled for subagent sessions") - }) - - test("ignores non-question tools for subagent sessions", async () => { - // given - const sessionID = "ses_subagent" - subagentSessions.add(sessionID) - const input = { tool: "bash", sessionID, callID: "call_1" } - const output = { args: { command: "ls" } } - - // when - const result = hook["tool.execute.before"]?.(input as any, output as any) - - // then - await expect(result).resolves.toBeUndefined() - }) - }) -}) diff --git a/src/hooks/subagent-question-blocker/index.ts b/src/hooks/subagent-question-blocker/index.ts deleted file mode 100644 index dbba20b8b..000000000 --- a/src/hooks/subagent-question-blocker/index.ts +++ /dev/null @@ -1 +0,0 @@ -export { createSubagentQuestionBlockerHook } from "./hook"; diff --git a/src/plugin/hooks/create-session-hooks.ts b/src/plugin/hooks/create-session-hooks.ts index 28a0ecc32..82a4379fc 100644 --- a/src/plugin/hooks/create-session-hooks.ts +++ b/src/plugin/hooks/create-session-hooks.ts @@ -19,7 +19,6 @@ import { createPrometheusMdOnlyHook, createSisyphusJuniorNotepadHook, createQuestionLabelTruncatorHook, - createSubagentQuestionBlockerHook, createPreemptiveCompactionHook, } from "../../hooks" import { createAnthropicEffortHook } from "../../hooks/anthropic-effort" @@ -49,7 +48,6 @@ export type SessionHooks = { prometheusMdOnly: ReturnType | null sisyphusJuniorNotepad: ReturnType | null questionLabelTruncator: ReturnType - subagentQuestionBlocker: ReturnType taskResumeInfo: ReturnType anthropicEffort: ReturnType | null } @@ -149,7 +147,6 @@ export function createSessionHooks(args: { : null const questionLabelTruncator = createQuestionLabelTruncatorHook() - const subagentQuestionBlocker = createSubagentQuestionBlockerHook() const taskResumeInfo = createTaskResumeInfoHook() const anthropicEffort = isHookEnabled("anthropic-effort") @@ -174,7 +171,6 @@ export function createSessionHooks(args: { prometheusMdOnly, sisyphusJuniorNotepad, questionLabelTruncator, - subagentQuestionBlocker, taskResumeInfo, anthropicEffort, } diff --git a/src/plugin/tool-execute-before.test.ts b/src/plugin/tool-execute-before.test.ts index 1b5b80788..95f28de16 100644 --- a/src/plugin/tool-execute-before.test.ts +++ b/src/plugin/tool-execute-before.test.ts @@ -1,10 +1,38 @@ -import { describe, expect, test } from "bun:test" -import { createToolExecuteBeforeHandler } from "./tool-execute-before" -import type { CreatedHooks } from "../create-hooks" +const { describe, expect, test } = require("bun:test") +const { createToolExecuteBeforeHandler } = require("./tool-execute-before") describe("createToolExecuteBeforeHandler", () => { + test("does not execute subagent question blocker hook for question tool", async () => { + //#given + const ctx = { + client: { + session: { + messages: async () => ({ data: [] }), + }, + }, + } + + const hooks = { + subagentQuestionBlocker: { + "tool.execute.before": async () => { + throw new Error("subagentQuestionBlocker should not run") + }, + }, + } + + const handler = createToolExecuteBeforeHandler({ ctx, hooks }) + const input = { tool: "question", sessionID: "ses_sub", callID: "call_1" } + const output = { args: { questions: [] } as Record } + + //#when + const run = handler(input, output) + + //#then + await expect(run).resolves.toBeUndefined() + }) + describe("task tool subagent_type normalization", () => { - const emptyHooks = {} as CreatedHooks + const emptyHooks = {} function createCtxWithSessionMessages(messages: Array<{ info?: { agent?: string; role?: string } }> = []) { return { @@ -13,7 +41,7 @@ describe("createToolExecuteBeforeHandler", () => { messages: async () => ({ data: messages }), }, }, - } as unknown as Parameters[0]["ctx"] + } } test("sets subagent_type to sisyphus-junior when category is provided without subagent_type", async () => { @@ -136,3 +164,5 @@ describe("createToolExecuteBeforeHandler", () => { }) }) }) + +export {} diff --git a/src/plugin/tool-execute-before.ts b/src/plugin/tool-execute-before.ts index 9d07e5efa..0a7bd38ed 100644 --- a/src/plugin/tool-execute-before.ts +++ b/src/plugin/tool-execute-before.ts @@ -17,7 +17,6 @@ export function createToolExecuteBeforeHandler(args: { const { ctx, hooks } = args return async (input, output): Promise => { - await hooks.subagentQuestionBlocker?.["tool.execute.before"]?.(input, output) await hooks.writeExistingFileGuard?.["tool.execute.before"]?.(input, output) await hooks.questionLabelTruncator?.["tool.execute.before"]?.(input, output) await hooks.claudeCodeHooks?.["tool.execute.before"]?.(input, output) diff --git a/src/tools/call-omo-agent/sync-executor.test.ts b/src/tools/call-omo-agent/sync-executor.test.ts new file mode 100644 index 000000000..744f08ffb --- /dev/null +++ b/src/tools/call-omo-agent/sync-executor.test.ts @@ -0,0 +1,91 @@ +const { describe, test, expect, mock } = require("bun:test") + +mock.module("./session-creator", () => ({ + createOrGetSession: mock(async () => ({ sessionID: "ses-test-123" })), +})) + +mock.module("./completion-poller", () => ({ + waitForCompletion: mock(async () => {}), +})) + +mock.module("./message-processor", () => ({ + processMessages: mock(async () => "agent response"), +})) + +describe("executeSync", () => { + test("passes question=false via tools parameter to block question tool", async () => { + //#given + const { executeSync } = require("./sync-executor") + + let promptArgs: any + const promptAsync = mock(async (input: any) => { + promptArgs = input + return { data: {} } + }) + + const args = { + subagent_type: "explore", + description: "test task", + prompt: "find something", + } + + const toolContext = { + sessionID: "parent-session", + messageID: "msg-1", + agent: "sisyphus", + abort: new AbortController().signal, + metadata: mock(async () => {}), + } + + const ctx = { + client: { + session: { promptAsync }, + }, + } + + //#when + await executeSync(args, toolContext, ctx as any) + + //#then + expect(promptAsync).toHaveBeenCalled() + expect(promptArgs.body.tools.question).toBe(false) + }) + + test("passes task=false via tools parameter", async () => { + //#given + const { executeSync } = require("./sync-executor") + + let promptArgs: any + const promptAsync = mock(async (input: any) => { + promptArgs = input + return { data: {} } + }) + + const args = { + subagent_type: "librarian", + description: "search docs", + prompt: "find docs", + } + + const toolContext = { + sessionID: "parent-session", + messageID: "msg-2", + agent: "sisyphus", + abort: new AbortController().signal, + metadata: mock(async () => {}), + } + + const ctx = { + client: { + session: { promptAsync }, + }, + } + + //#when + await executeSync(args, toolContext, ctx as any) + + //#then + expect(promptAsync).toHaveBeenCalled() + expect(promptArgs.body.tools.task).toBe(false) + }) +}) diff --git a/src/tools/call-omo-agent/sync-executor.ts b/src/tools/call-omo-agent/sync-executor.ts index f64310fbd..9bd0f2de7 100644 --- a/src/tools/call-omo-agent/sync-executor.ts +++ b/src/tools/call-omo-agent/sync-executor.ts @@ -35,6 +35,7 @@ export async function executeSync( tools: { ...getAgentToolRestrictions(args.subagent_type), task: false, + question: false, }, parts: [{ type: "text", text: args.prompt }], }, diff --git a/src/tools/delegate-task/sync-prompt-sender.test.ts b/src/tools/delegate-task/sync-prompt-sender.test.ts index 7b26ae239..365d796de 100644 --- a/src/tools/delegate-task/sync-prompt-sender.test.ts +++ b/src/tools/delegate-task/sync-prompt-sender.test.ts @@ -1,6 +1,45 @@ const { describe, test, expect, mock } = require("bun:test") describe("sendSyncPrompt", () => { + test("passes question=false via tools parameter", async () => { + //#given + const { sendSyncPrompt } = require("./sync-prompt-sender") + + let promptArgs: any + const promptAsync = mock(async (input: any) => { + promptArgs = input + return { data: {} } + }) + + const mockClient = { + session: { + promptAsync, + }, + } + + const input = { + sessionID: "test-session", + agentToUse: "sisyphus-junior", + args: { + description: "test task", + prompt: "test prompt", + run_in_background: false, + load_skills: [], + }, + systemContent: undefined, + categoryModel: undefined, + toastManager: null, + taskId: undefined, + } + + //#when + await sendSyncPrompt(mockClient as any, input) + + //#then + expect(promptAsync).toHaveBeenCalled() + expect(promptArgs.body.tools.question).toBe(false) + }) + test("applies agent tool restrictions for explore agent", async () => { //#given const { sendSyncPrompt } = require("./sync-prompt-sender")