diff --git a/src/features/background-agent/subagent-spawn-limits.test.ts b/src/features/background-agent/subagent-spawn-limits.test.ts index 154718dbd..85824d46c 100644 --- a/src/features/background-agent/subagent-spawn-limits.test.ts +++ b/src/features/background-agent/subagent-spawn-limits.test.ts @@ -1,6 +1,14 @@ import { describe, expect, test } from "bun:test" import type { OpencodeClient } from "./constants" -import { resolveSubagentSpawnContext } from "./subagent-spawn-limits" +import { + resolveSubagentSpawnContext, + getMaxSubagentDepth, + DEFAULT_MAX_SUBAGENT_DEPTH, + createSubagentDepthLimitError, + createSubagentDescendantLimitError, + getMaxRootSessionSpawnBudget, + DEFAULT_MAX_ROOT_SESSION_SPAWN_BUDGET, +} from "./subagent-spawn-limits" function createMockClient(sessionGet: OpencodeClient["session"]["get"]): OpencodeClient { return { @@ -41,4 +49,177 @@ describe("resolveSubagentSpawnContext", () => { await expect(result).rejects.toThrow(/background_task\.maxDescendants cannot be enforced safely.*No session data returned/) }) }) + + describe("depth calculation smoke tests (regression guard)", () => { + test("root session (no parentID) reports depth 0 and childDepth 1", async () => { + // given - a root session with no parent + const client = createMockClient(async (opts) => { + if (opts.path.id === "root-session") { + return { data: { id: "root-session", parentID: undefined } } + } + return { error: "not found", data: undefined } + }) + + // when + const result = await resolveSubagentSpawnContext(client, "root-session") + + // then + expect(result.rootSessionID).toBe("root-session") + expect(result.parentDepth).toBe(0) + expect(result.childDepth).toBe(1) + }) + + test("depth-1 child reports childDepth 2", async () => { + // given - child -> root chain + const client = createMockClient(async (opts) => { + if (opts.path.id === "child-1") { + return { data: { id: "child-1", parentID: "root-session" } } + } + if (opts.path.id === "root-session") { + return { data: { id: "root-session", parentID: undefined } } + } + return { error: "not found", data: undefined } + }) + + // when + const result = await resolveSubagentSpawnContext(client, "child-1") + + // then + expect(result.rootSessionID).toBe("root-session") + expect(result.parentDepth).toBe(1) + expect(result.childDepth).toBe(2) + }) + + test("depth-2 grandchild reports childDepth 3", async () => { + // given - grandchild -> child -> root chain + const client = createMockClient(async (opts) => { + const sessions: Record = { + "grandchild": { id: "grandchild", parentID: "child" }, + "child": { id: "child", parentID: "root" }, + "root": { id: "root", parentID: undefined }, + } + const session = sessions[opts.path.id] + if (session) return { data: session } + return { error: "not found", data: undefined } + }) + + // when + const result = await resolveSubagentSpawnContext(client, "grandchild") + + // then + expect(result.rootSessionID).toBe("root") + expect(result.parentDepth).toBe(2) + expect(result.childDepth).toBe(3) + }) + + test("depth at DEFAULT_MAX_SUBAGENT_DEPTH reports exact max childDepth", async () => { + // given - chain of exactly DEFAULT_MAX_SUBAGENT_DEPTH depth + // With default=3: session-3 -> session-2 -> session-1 -> root + const sessions: Record = { + "root": { id: "root" }, + } + for (let i = 1; i <= DEFAULT_MAX_SUBAGENT_DEPTH; i++) { + sessions[`session-${i}`] = { + id: `session-${i}`, + parentID: i === 1 ? "root" : `session-${i - 1}`, + } + } + + const client = createMockClient(async (opts) => { + const session = sessions[opts.path.id] + if (session) return { data: session } + return { error: "not found", data: undefined } + }) + + // when - resolve from the deepest session + const deepest = `session-${DEFAULT_MAX_SUBAGENT_DEPTH}` + const result = await resolveSubagentSpawnContext(client, deepest) + + // then - childDepth should be DEFAULT_MAX_SUBAGENT_DEPTH + 1 (exceeds limit) + expect(result.childDepth).toBe(DEFAULT_MAX_SUBAGENT_DEPTH + 1) + expect(result.parentDepth).toBe(DEFAULT_MAX_SUBAGENT_DEPTH) + }) + + test("detects parent cycle and throws", async () => { + // given - A -> B -> A (cycle) + const client = createMockClient(async (opts) => { + const sessions: Record = { + "session-a": { id: "session-a", parentID: "session-b" }, + "session-b": { id: "session-b", parentID: "session-a" }, + } + const session = sessions[opts.path.id] + if (session) return { data: session } + return { error: "not found", data: undefined } + }) + + // when + const result = resolveSubagentSpawnContext(client, "session-a") + + // then + await expect(result).rejects.toThrow(/session parent cycle/) + }) + }) +}) + +describe("getMaxSubagentDepth", () => { + test("returns DEFAULT_MAX_SUBAGENT_DEPTH when no config", () => { + expect(getMaxSubagentDepth()).toBe(DEFAULT_MAX_SUBAGENT_DEPTH) + expect(getMaxSubagentDepth(undefined)).toBe(DEFAULT_MAX_SUBAGENT_DEPTH) + }) + + test("returns config.maxDepth when provided", () => { + expect(getMaxSubagentDepth({ maxDepth: 5 })).toBe(5) + expect(getMaxSubagentDepth({ maxDepth: 1 })).toBe(1) + expect(getMaxSubagentDepth({ maxDepth: 0 })).toBe(0) + }) + + test("default is 3", () => { + expect(DEFAULT_MAX_SUBAGENT_DEPTH).toBe(3) + }) +}) + +describe("getMaxRootSessionSpawnBudget", () => { + test("returns DEFAULT_MAX_ROOT_SESSION_SPAWN_BUDGET when no config", () => { + expect(getMaxRootSessionSpawnBudget()).toBe(DEFAULT_MAX_ROOT_SESSION_SPAWN_BUDGET) + }) + + test("returns config.maxDescendants when provided", () => { + expect(getMaxRootSessionSpawnBudget({ maxDescendants: 10 })).toBe(10) + }) + + test("default is 50", () => { + expect(DEFAULT_MAX_ROOT_SESSION_SPAWN_BUDGET).toBe(50) + }) +}) + +describe("createSubagentDepthLimitError", () => { + test("includes childDepth, maxDepth, and session IDs in message", () => { + const error = createSubagentDepthLimitError({ + childDepth: 4, + maxDepth: 3, + parentSessionID: "parent-123", + rootSessionID: "root-456", + }) + + expect(error.message).toContain("child depth 4") + expect(error.message).toContain("maxDepth=3") + expect(error.message).toContain("parent-123") + expect(error.message).toContain("root-456") + expect(error.message).toContain("spawn blocked") + }) +}) + +describe("createSubagentDescendantLimitError", () => { + test("includes descendant count, max, and root session ID", () => { + const error = createSubagentDescendantLimitError({ + rootSessionID: "root-789", + descendantCount: 50, + maxDescendants: 50, + }) + + expect(error.message).toContain("root-789") + expect(error.message).toContain("50") + expect(error.message).toContain("maxDescendants=50") + expect(error.message).toContain("spawn blocked") + }) }) diff --git a/src/tools/delegate-task/sync-task.test.ts b/src/tools/delegate-task/sync-task.test.ts index 483a826b9..b34a3cc6c 100644 --- a/src/tools/delegate-task/sync-task.test.ts +++ b/src/tools/delegate-task/sync-task.test.ts @@ -282,6 +282,139 @@ describe("executeSyncTask - cleanup on error paths", () => { expect(deleteCalls.length).toBe(1) expect(deleteCalls[0]).toBe("ses_test_12345678") }) + + test("depth regression: blocks spawn when reserveSubagentSpawn throws depth limit error", async () => { + // This is a smoke test guarding against regressions where the depth limit + // would be silently bypassed (e.g. via a fallback path that hardcodes + // childDepth: 1). + + const mockClient = { + session: { + create: async () => ({ data: { id: "ses_test_12345678" } }), + }, + } + + const { executeSyncTask } = require("./sync-task") + + const reserveSubagentSpawn = mock(async () => { + throw new Error( + "Subagent spawn blocked: child depth 4 exceeds background_task.maxDepth=3. Parent session: parent. Root session: root. Continue in an existing subagent session instead of spawning another." + ) + }) + + const deps = { + createSyncSession: async () => ({ ok: true, sessionID: "ses_test_12345678" }), + sendSyncPrompt: async () => null, + pollSyncSession: async () => null, + fetchSyncResult: async () => ({ ok: true as const, textContent: "Result" }), + } + + const mockCtx = { + sessionID: "parent-session", + callID: "call-123", + metadata: () => {}, + } + + const mockExecutorCtx = { + manager: { reserveSubagentSpawn }, + client: mockClient, + directory: "/tmp", + onSyncSessionCreated: null, + } + + const args = { + prompt: "test prompt", + description: "test task", + category: "test", + load_skills: [], + run_in_background: false, + command: null, + } + + //#when - executeSyncTask is called from a session at max depth + const result = await executeSyncTask(args, mockCtx, mockExecutorCtx, { + sessionID: "parent-session", + }, "test-agent", undefined, undefined, undefined, undefined, deps) + + //#then - should propagate the depth limit error and NOT create the session + expect(result).toContain("Subagent spawn blocked") + expect(result).toContain("child depth 4") + expect(result).toContain("maxDepth=3") + expect(reserveSubagentSpawn).toHaveBeenCalledWith("parent-session") + // critical: createSyncSession must NOT have been called -- if it was, + // the depth guard was bypassed. + expect(addCalls.length).toBe(0) + }) + + test("depth regression: does not silently fall back to childDepth: 1 when manager methods are present", async () => { + // Guards against the dangerous fallback path in sync-task.ts that + // hardcodes childDepth: 1 if reserveSubagentSpawn / assertCanSpawn are + // not functions. With a real manager present, the fallback must NOT be + // taken. + + const mockClient = { + session: { + create: async () => ({ data: { id: "ses_test_12345678" } }), + }, + } + + const { executeSyncTask } = require("./sync-task") + + let reservedDepth: number | undefined + const commit = mock(() => 1) + const rollback = mock(() => {}) + const reserveSubagentSpawn = mock(async () => { + // Return a depth that proves the real manager was consulted + reservedDepth = 3 + return { + spawnContext: { rootSessionID: "root", parentDepth: 2, childDepth: 3 }, + descendantCount: 5, + commit, + rollback, + } + }) + + const deps = { + createSyncSession: async () => ({ ok: true, sessionID: "ses_test_12345678" }), + sendSyncPrompt: async () => null, + pollSyncSession: async () => null, + fetchSyncResult: async () => ({ ok: true as const, textContent: "Result" }), + } + + const metadataCalls: any[] = [] + const mockCtx = { + sessionID: "parent-session", + callID: "call-123", + metadata: (input: any) => { metadataCalls.push(input) }, + } + + const mockExecutorCtx = { + manager: { reserveSubagentSpawn }, + client: mockClient, + directory: "/tmp", + onSyncSessionCreated: null, + } + + const args = { + prompt: "test prompt", + description: "test task", + category: "test", + load_skills: [], + run_in_background: false, + command: null, + } + + //#when + await executeSyncTask(args, mockCtx, mockExecutorCtx, { + sessionID: "parent-session", + }, "test-agent", undefined, undefined, undefined, undefined, deps) + + //#then - the spawnDepth recorded in metadata MUST match what reserveSubagentSpawn returned + expect(reservedDepth).toBe(3) + const taskMeta = metadataCalls.find((c) => c.metadata?.spawnDepth !== undefined) + expect(taskMeta).toBeDefined() + expect(taskMeta.metadata.spawnDepth).toBe(3) // NOT 1 (the fallback value) + }) }) export {} diff --git a/src/tools/delegate-task/sync-task.ts b/src/tools/delegate-task/sync-task.ts index 07372d8a0..d1ee6ff31 100644 --- a/src/tools/delegate-task/sync-task.ts +++ b/src/tools/delegate-task/sync-task.ts @@ -37,14 +37,29 @@ export async function executeSyncTask( spawnReservation = await manager.reserveSubagentSpawn(parentContext.sessionID) } - const spawnContext = spawnReservation?.spawnContext - ?? (typeof manager?.assertCanSpawn === "function" - ? await manager.assertCanSpawn(parentContext.sessionID) - : { - rootSessionID: parentContext.sessionID, - parentDepth: 0, - childDepth: 1, - }) + // Depth/descendant guard. We must NOT silently fall back to childDepth: 1 + // when the manager is unavailable or lacks the spawn methods, because that + // would let subagents recurse without bound. The only safe fallback is + // when the manager genuinely cannot enforce limits (legacy SDK), in which + // case we still record childDepth: 1 but log a warning so regressions are + // visible. + let spawnContext: { rootSessionID: string; parentDepth: number; childDepth: number } + if (spawnReservation?.spawnContext) { + spawnContext = spawnReservation.spawnContext + } else if (typeof manager?.assertCanSpawn === "function") { + spawnContext = await manager.assertCanSpawn(parentContext.sessionID) + } else { + log( + "[task] WARNING: BackgroundManager has no spawn enforcement methods (reserveSubagentSpawn / assertCanSpawn). " + + "Depth and descendant limits cannot be enforced for this task. This indicates an old SDK or a misconfiguration.", + { parentSessionID: parentContext.sessionID } + ) + spawnContext = { + rootSessionID: parentContext.sessionID, + parentDepth: 0, + childDepth: 1, + } + } const createSessionResult = await deps.createSyncSession(client, { parentSessionID: parentContext.sessionID,