Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/core/src/agents/background-agent-resume.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { AgentTerminateMode } from './runtime/agent-types.js';
import { AgentEventEmitter } from './runtime/agent-events.js';
import { AgentHeadless } from './runtime/agent-headless.js';
import {
FORK_DEFAULT_MAX_TURNS,
FORK_SUBAGENT_TYPE,
buildChildMessage,
} from '../tools/agent/fork-subagent.js';
Expand Down Expand Up @@ -96,6 +97,7 @@ describe('BackgroundAgentResumeService', () => {
getMaxSessionTurns: () => -1,
getMaxToolCalls: () => -1,
isTrustedFolder: () => true,
isInteractive: () => false,
getProjectRoot: () => tempDir,
getCliVersion: () => 'test-version',
getGeminiClient: () => undefined,
Expand Down Expand Up @@ -1351,6 +1353,9 @@ describe('BackgroundAgentResumeService', () => {
{ role: 'model', parts: [{ text: 'Working silently' }] },
],
});
expect(createArgs?.[4]).toEqual({
max_turns: FORK_DEFAULT_MAX_TURNS,
});
expect(createArgs?.[5]).toEqual({
tools: [{ name: 'Bash' }, { name: 'Read' }],
});
Expand Down
9 changes: 3 additions & 6 deletions packages/core/src/agents/background-agent-resume.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { createApprovalModeOverride } from '../tools/agent/agent.js';
import type { ApprovalMode } from '../config/config.js';
import {
FORK_AGENT,
FORK_DEFAULT_MAX_TURNS,
FORK_SUBAGENT_TYPE,
runInForkContext,
} from '../tools/agent/fork-subagent.js';
Expand All @@ -48,11 +49,7 @@ import type { SubagentConfig } from '../subagents/types.js';
import { BUBBLE_APPROVAL_MODE } from '../subagents/types.js';
import { EXCLUDED_TOOLS_FOR_SUBAGENTS } from './runtime/agent-core.js';
import { ToolNames } from '../tools/tool-names.js';
import type {
PromptConfig,
RunConfig,
ToolConfig,
} from './runtime/agent-types.js';
import type { PromptConfig, ToolConfig } from './runtime/agent-types.js';
import type {
AgentBootstrapRecordPayload,
NotificationRecordPayload,
Expand Down Expand Up @@ -1168,7 +1165,7 @@ export class BackgroundAgentResumeService {
agentConfig,
promptConfig,
{},
{} as RunConfig,
{ max_turns: FORK_DEFAULT_MAX_TURNS },
toolConfig,
eventEmitter,
);
Expand Down
33 changes: 33 additions & 0 deletions packages/core/src/tools/agent/agent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import { ToolNames } from '../tool-names.js';
import { type Config, ApprovalMode } from '../../config/config.js';
import { SubagentManager } from '../../subagents/subagent-manager.js';
import type { SubagentConfig } from '../../subagents/types.js';
import { BUBBLE_APPROVAL_MODE } from '../../subagents/types.js';
import { FORK_AGENT, FORK_DEFAULT_MAX_TURNS } from './fork-subagent.js';
import { AgentTerminateMode } from '../../agents/runtime/agent-types.js';
import {
AgentHeadless,
Expand Down Expand Up @@ -1313,6 +1315,37 @@ describe('AgentTool', () => {
expect(AgentHeadless.create).toHaveBeenCalledTimes(1);
});

it('caps fork turns and uses bubble approval mode', async () => {
const mockLoadedSubagent: SubagentConfig = {
name: 'general-purpose',
description: 'General-purpose agent',
systemPrompt: 'You are a general-purpose agent.',
level: 'builtin',
filePath: '<builtin:general-purpose>',
};
vi.mocked(mockSubagentManager.loadSubagent).mockResolvedValue(
mockLoadedSubagent,
);

const invocation = (
agentTool as AgentToolWithProtectedMethods
).createInvocation({
description: 'some task',
prompt: 'do the thing',
subagent_type: 'fork',
});
await invocation.execute();

expect(AgentHeadless.create).toHaveBeenCalledTimes(1);
const createArgs = vi.mocked(AgentHeadless.create).mock.calls[0];
// RunConfig (5th positional) carries the detached-fork turn cap so a
// fire-and-forget fork can't loop unbounded.
expect(createArgs[4]).toEqual({ max_turns: FORK_DEFAULT_MAX_TURNS });
// The fork agent uses `bubble` approval so its permission prompts surface
// to the parent's Background-tasks UI instead of being auto-denied.
expect(FORK_AGENT.approvalMode).toBe(BUBBLE_APPROVAL_MODE);
});

it('omitting subagent_type uses general-purpose, not fork', async () => {
// Restored contract: omission resolves to the awaitable general-purpose
// subagent (inline result), never a fork — even in interactive mode.
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/tools/agent/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import { BUBBLE_APPROVAL_MODE } from '../../subagents/types.js';
import { AgentTerminateMode } from '../../agents/runtime/agent-types.js';
import type {
PromptConfig,
RunConfig,
ToolConfig,
} from '../../agents/runtime/agent-types.js';
import {
Expand All @@ -37,6 +36,7 @@ import type { AgentExternalInput } from '../../agents/runtime/agent-types.js';
import type { Content, FunctionDeclaration } from '@google/genai';
import {
FORK_AGENT,
FORK_DEFAULT_MAX_TURNS,
FORK_SUBAGENT_TYPE,
FORK_PLACEHOLDER_RESULT,
buildForkedMessages,
Expand Down Expand Up @@ -1298,7 +1298,7 @@ class AgentToolInvocation extends BaseToolInvocation<AgentParams, ToolResult> {
agentConfig,
promptConfig,
{},
{} as RunConfig,
{ max_turns: FORK_DEFAULT_MAX_TURNS },

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The RunConfig for forks includes only max_turns with no max_time_minutes. A fork doing 199 turns of expensive tool calls (long shell commands, large file I/O, network requests) could still run for hours and accumulate significant API costs. The workflow orchestrator applies both a turn cap (50) and a time cap (10 minutes) for defense in depth; the fork path has only one axis of protection.

Consider adding a time cap as a second safety net:

Suggested change
{ max_turns: FORK_DEFAULT_MAX_TURNS },
{ max_turns: FORK_DEFAULT_MAX_TURNS, max_time_minutes: 30 },

— Claude 3.5 Sonnet via Qwen Code /review

toolConfig,
eventEmitter,
);
Expand Down
10 changes: 9 additions & 1 deletion packages/core/src/tools/agent/fork-subagent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { AsyncLocalStorage } from 'node:async_hooks';
import type { Content } from '@google/genai';
import type { Config } from '../../config/config.js';
import type { SubagentConfig } from '../../subagents/types.js';
import { BUBBLE_APPROVAL_MODE } from '../../subagents/types.js';

export const FORK_SUBAGENT_TYPE = 'fork';

Expand Down Expand Up @@ -33,10 +34,17 @@ export const FORK_AGENT = {
tools: ['*'],
systemPrompt:
'You are a forked worker process. Follow the directive in the conversation history. Execute tasks directly using available tools. Do not spawn sub-agents.',
approvalMode: 'default',
// `bubble` surfaces this fork's permission prompts to the parent's Background-
// tasks UI; a detached fork has no inline UI, so 'default' would auto-deny them.
approvalMode: BUBBLE_APPROVAL_MODE,
level: 'session' as const,
} satisfies SubagentConfig;

// Turn cap for a detached fork — fire-and-forget background work nobody awaits,
// so an unbounded reasoning loop burns tokens silently. Matches claude-code's
// fork cap of 200.
export const FORK_DEFAULT_MAX_TURNS = 200;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The constant name FORK_DEFAULT_MAX_TURNS implies overridability ("default" suggests other values are possible), but the cap is applied unconditionally in createForkSubagent with no escape hatch — no environment variable, no config setting, no tool parameter. A user whose legitimate fork task exceeds 200 turns (e.g., bulk file edits across hundreds of files) will hit a silent termination with no way to raise the limit.

Either rename to FORK_MAX_TURNS (dropping the "DEFAULT" implication) to signal it is an absolute cap, or accept an optional max_turns from the AgentTool params and clamp it to a reasonable range, falling back to 200 when absent.

— Claude 3.5 Sonnet via Qwen Code /review


// Recursive-fork guard. A fork child keeps the `agent` tool in its declarations
// for byte-identical cache parity with the parent, so tool-availability
// stripping is no longer an option. Instead, mark the async frame as "inside a
Expand Down
Loading