Merge pull request #1896 from code-yeongyu/fix/1283-review-code-silent-fail

fix: report silent subagent delegation failures (#1283)

src/tools/delegate-task/subagent-resolver.test.ts

@@ -0,0 +1,82 @@
+declare const require: (name: string) => any
+const { describe, test, expect, beforeEach, afterEach, spyOn, mock } = require("bun:test")
+import { resolveSubagentExecution } from "./subagent-resolver"
+import type { DelegateTaskArgs } from "./types"
+import type { ExecutorContext } from "./executor-types"
+import * as logger from "../../shared/logger"
+
+function createBaseArgs(overrides?: Partial<DelegateTaskArgs>): DelegateTaskArgs {
+  return {
+    description: "Run review",
+    prompt: "Review the current changes",
+    run_in_background: false,
+    load_skills: [],
+    subagent_type: "oracle",
+    ...overrides,
+  }
+}
+
+function createExecutorContext(agentsFn: () => Promise<unknown>): ExecutorContext {
+  const client = {
+    app: {
+      agents: agentsFn,
+    },
+  } as ExecutorContext["client"]
+
+  return {
+    client,
+    manager: {} as ExecutorContext["manager"],
+    directory: "/tmp/test",
+  }
+}
+
+describe("resolveSubagentExecution", () => {
+  let logSpy: ReturnType<typeof spyOn> | undefined
+
+  beforeEach(() => {
+    mock.restore()
+    logSpy = spyOn(logger, "log").mockImplementation(() => {})
+  })
+
+  afterEach(() => {
+    logSpy?.mockRestore()
+  })
+
+  test("returns delegation error when agent discovery fails instead of silently proceeding", async () => {
+    //#given
+    const resolverError = new Error("agents API unavailable")
+    const args = createBaseArgs()
+    const executorCtx = createExecutorContext(async () => {
+      throw resolverError
+    })
+
+    //#when
+    const result = await resolveSubagentExecution(args, executorCtx, "sisyphus", "deep")
+
+    //#then
+    expect(result.agentToUse).toBe("")
+    expect(result.categoryModel).toBeUndefined()
+    expect(result.error).toBe("Failed to delegate to agent \"oracle\": agents API unavailable")
+  })
+
+  test("logs failure details when subagent resolution throws", async () => {
+    //#given
+    const args = createBaseArgs({ subagent_type: "review" })
+    const executorCtx = createExecutorContext(async () => {
+      throw new Error("network timeout")
+    })
+
+    //#when
+    await resolveSubagentExecution(args, executorCtx, "sisyphus", "deep")
+
+    //#then
+    expect(logSpy).toHaveBeenCalledTimes(1)
+    const callArgs = logSpy?.mock.calls[0]
+    expect(callArgs?.[0]).toBe("[delegate-task] Failed to resolve subagent execution")
+    expect(callArgs?.[1]).toEqual({
+      requestedAgent: "review",
+      parentAgent: "sisyphus",
+      error: "network timeout",
+    })
+  })
+})

src/tools/delegate-task/subagent-resolver.ts

@@ -6,6 +6,7 @@ import { parseModelString } from "./model-string-parser"
 import { AGENT_MODEL_REQUIREMENTS } from "../../shared/model-requirements"
 import { getAgentDisplayName, getAgentConfigKey } from "../../shared/agent-display-names"
 import { normalizeSDKResponse } from "../../shared"
+import { log } from "../../shared/logger"
 import { getAvailableModelsForDelegateTask } from "./available-models"
 import { resolveModelForDelegateTask } from "./model-selection"
 
@@ -119,8 +120,19 @@ Create the work plan directly - that's your job as the planning agent.`,
     if (!categoryModel && matchedAgent.model) {
       categoryModel = matchedAgent.model
     }
-  } catch {
-    // Proceed anyway - session.prompt will fail with clearer error if agent doesn't exist
+  } catch (error) {
+    const errorMessage = error instanceof Error ? error.message : String(error)
+    log("[delegate-task] Failed to resolve subagent execution", {
+      requestedAgent: agentToUse,
+      parentAgent,
+      error: errorMessage,
+    })
+
+    return {
+      agentToUse: "",
+      categoryModel: undefined,
+      error: `Failed to delegate to agent "${agentToUse}": ${errorMessage}`,
+    }
   }
 
   return { agentToUse, categoryModel }