Merge pull request #1812 from code-yeongyu/refactor/remove-subagent-question-blocker-hook

refactor: remove redundant subagent-question-blocker hook

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",

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",

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";

src/hooks/subagent-question-blocker/hook.ts

@@ -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."
-      )
-    },
-  }
-}

src/hooks/subagent-question-blocker/index.test.ts

@@ -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()
-    })
-  })
-})

src/hooks/subagent-question-blocker/index.ts

@@ -1 +0,0 @@
-export { createSubagentQuestionBlockerHook } from "./hook";

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<typeof createPrometheusMdOnlyHook> | null
   sisyphusJuniorNotepad: ReturnType<typeof createSisyphusJuniorNotepadHook> | null
   questionLabelTruncator: ReturnType<typeof createQuestionLabelTruncatorHook>
-  subagentQuestionBlocker: ReturnType<typeof createSubagentQuestionBlockerHook>
   taskResumeInfo: ReturnType<typeof createTaskResumeInfoHook>
   anthropicEffort: ReturnType<typeof createAnthropicEffortHook> | 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,
   }

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<string, unknown> }
+
+    //#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<typeof createToolExecuteBeforeHandler>[0]["ctx"]
+      }
     }
 
     test("sets subagent_type to sisyphus-junior when category is provided without subagent_type", async () => {
@@ -136,3 +164,5 @@ describe("createToolExecuteBeforeHandler", () => {
     })
   })
 })
+
+export {}

src/plugin/tool-execute-before.ts

@@ -17,7 +17,6 @@ export function createToolExecuteBeforeHandler(args: {
   const { ctx, hooks } = args
 
   return async (input, output): Promise<void> => {
-    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)

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)
+  })
+})

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 }],
       },

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")