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
57 changes: 0 additions & 57 deletions packages/cli/src/acp-integration/session/Session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,6 @@ describe('Session', () => {

mockToolRegistry = {
getTool: vi.fn(),
// #executePrompt → #buildInitialSystemReminders calls
// getToolRegistry().ensureTool(ToolNames.AGENT) on every session.prompt(),
// so the default mock must provide it (#1151 / #3479).
ensureTool: vi.fn().mockResolvedValue(true),
};
const fileService = { shouldGitIgnoreFile: vi.fn().mockReturnValue(false) };
Expand All @@ -239,12 +236,6 @@ describe('Session', () => {
.fn()
.mockReturnValue(mockChatRecordingService),
getToolRegistry: vi.fn().mockReturnValue(mockToolRegistry),
// #buildInitialSystemReminders iterates listSubagents() on every
// session.prompt(). Default to an empty list so tests that don't
// exercise subagent reminders don't need to stub it (#1151 / #3479).
getSubagentManager: vi.fn().mockReturnValue({
listSubagents: vi.fn().mockResolvedValue([]),
}),
getFileService: vi.fn().mockReturnValue(fileService),
getFileFilteringRespectGitIgnore: vi.fn().mockReturnValue(true),
getEnableRecursiveFileSearch: vi.fn().mockReturnValue(false),
Expand Down Expand Up @@ -2874,20 +2865,7 @@ describe('Session', () => {
return capture;
};

const stubEmptySubagents = () => {
(mockConfig as unknown as Record<string, unknown>)[
'getSubagentManager'
] = vi.fn().mockReturnValue({
listSubagents: vi.fn().mockResolvedValue([]),
});
// ensureTool is called on the result of getToolRegistry(); add it.
(
mockToolRegistry as unknown as { ensureTool: () => Promise<boolean> }
).ensureTool = vi.fn().mockResolvedValue(true);
};

it('prepends plan-mode reminder when approval mode is PLAN (#1151)', async () => {
stubEmptySubagents();
mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.PLAN);
const capture = captureFirstTurnMessage();

Expand All @@ -2910,7 +2888,6 @@ describe('Session', () => {
});

it('does not prepend plan-mode reminder in default approval mode', async () => {
stubEmptySubagents();
mockConfig.getApprovalMode = vi
.fn()
.mockReturnValue(ApprovalMode.DEFAULT);
Expand All @@ -2926,40 +2903,6 @@ describe('Session', () => {
);
expect(hasPlanReminder).toBe(false);
});

it('prepends subagent reminder when user-level subagents exist', async () => {
(mockConfig as unknown as Record<string, unknown>)[
'getSubagentManager'
] = vi.fn().mockReturnValue({
listSubagents: vi.fn().mockResolvedValue([
{ name: 'researcher', level: 'user' },
{ name: 'planner', level: 'project' },
// builtin entries are filtered out, matching client.ts:853.
{ name: 'builtin-helper', level: 'builtin' },
]),
});
(
mockToolRegistry as unknown as { ensureTool: () => Promise<boolean> }
).ensureTool = vi.fn().mockResolvedValue(true);
mockConfig.getApprovalMode = vi
.fn()
.mockReturnValue(ApprovalMode.DEFAULT);
const capture = captureFirstTurnMessage();

await session.prompt({
sessionId: 'test-session-id',
prompt: [{ type: 'text', text: 'hi' }],
});

const reminder = capture.parts.find(
(p) =>
p.text &&
p.text.includes('researcher') &&
p.text.includes('planner'),
);
expect(reminder).toBeTruthy();
expect(reminder!.text).not.toContain('builtin-helper');
});
});
});
});
11 changes: 0 additions & 11 deletions packages/cli/src/acp-integration/session/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import {
generateToolUseId,
MessageBusType,
getPlanModeSystemReminder,
getSubagentSystemReminder,
getArenaSystemReminder,
STARTUP_CONTEXT_MODEL_ACK,
evaluatePermissionFlow,
Expand Down Expand Up @@ -1738,16 +1737,6 @@ export class Session implements SessionContext {
async #buildInitialSystemReminders(): Promise<Part[]> {
const reminders: Part[] = [];

Comment thread
DennisYu07 marked this conversation as resolved.
const hasAgentTool = await this.config
.getToolRegistry()
.ensureTool(ToolNames.AGENT);
const subagents = (await this.config.getSubagentManager().listSubagents())
.filter((subagent) => subagent.level !== 'builtin')
.map((subagent) => subagent.name);
if (hasAgentTool && subagents.length > 0) {
reminders.push({ text: getSubagentSystemReminder(subagents) });
}

if (this.config.getApprovalMode() === ApprovalMode.PLAN) {
reminders.push({
text: getPlanModeSystemReminder(this.config.getSdkMode?.()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,8 @@ describe('Session.pendingWorktreeNotice', () => {
}),
getToolRegistry: vi.fn().mockReturnValue({
getTool: vi.fn(),
// Called on every prompt() via #buildInitialSystemReminders
ensureTool: vi.fn().mockResolvedValue(true),
}),
// Called on every prompt() to check subagent system reminders
getSubagentManager: vi.fn().mockReturnValue({
listSubagents: vi.fn().mockResolvedValue([]),
}),
getFileService: vi.fn().mockReturnValue({
shouldGitIgnoreFile: vi.fn().mockReturnValue(false),
}),
Expand Down
5 changes: 0 additions & 5 deletions packages/core/src/core/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -418,10 +418,6 @@ describe('Gemini Client (client.ts)', () => {
vertexai: false,
authType: AuthType.USE_GEMINI,
};
const mockSubagentManager = {
listSubagents: vi.fn().mockResolvedValue([]),
addChangeListener: vi.fn().mockReturnValue(() => {}),
};
mockConfig = {
getContentGeneratorConfig: vi
.fn()
Expand Down Expand Up @@ -473,7 +469,6 @@ describe('Gemini Client (client.ts)', () => {
},
getContentGenerator: vi.fn().mockReturnValue(mockContentGenerator),
getBaseLlmClient: vi.fn(),
getSubagentManager: vi.fn().mockReturnValue(mockSubagentManager),
getSkipLoopDetection: vi.fn().mockReturnValue(false),
getChatRecordingService: vi.fn().mockReturnValue(undefined),
getResumedSessionData: vi.fn().mockReturnValue(undefined),
Expand Down
15 changes: 0 additions & 15 deletions packages/core/src/core/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import {
getCoreSystemPrompt,
getCustomSystemPrompt,
getPlanModeSystemReminder,
getSubagentSystemReminder,
} from './prompts.js';
import {
CompressionStatus,
Expand Down Expand Up @@ -1610,20 +1609,6 @@ export class GeminiClient {
) {
const systemReminders = [];

Comment thread
DennisYu07 marked this conversation as resolved.
// add subagent system reminder if there are subagents
const hasAgentTool = await this.config
.getToolRegistry()
.ensureTool(ToolNames.AGENT);
const subagents = (
await this.config.getSubagentManager().listSubagents()
)
.filter((subagent) => subagent.level !== 'builtin')
.map((subagent) => subagent.name);

if (hasAgentTool && subagents.length > 0) {
systemReminders.push(getSubagentSystemReminder(subagents));
}

// add plan mode system reminder if approval mode is plan
if (this.config.getApprovalMode() === ApprovalMode.PLAN) {
systemReminders.push(
Expand Down
26 changes: 0 additions & 26 deletions packages/core/src/core/prompts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
buildDeferredToolsSection,
getCoreSystemPrompt,
getCustomSystemPrompt,
getSubagentSystemReminder,
getPlanModeSystemReminder,
resolvePathFromEnv,
} from './prompts.js';
Expand Down Expand Up @@ -454,31 +453,6 @@ describe('getCustomSystemPrompt', () => {
});
});

describe('getSubagentSystemReminder', () => {
it('should format single agent type correctly', () => {
const result = getSubagentSystemReminder(['python']);

expect(result).toMatch(/^<system-reminder>.*<\/system-reminder>$/);
expect(result).toContain('available agent types are: python');
expect(result).toContain('PROACTIVELY use the');
});

it('should join multiple agent types with commas', () => {
const result = getSubagentSystemReminder(['python', 'web', 'analysis']);

expect(result).toContain(
'available agent types are: python, web, analysis',
);
});

it('should handle empty array', () => {
const result = getSubagentSystemReminder([]);

expect(result).toContain('available agent types are: ');
expect(result).toContain('<system-reminder>');
});
});

describe('buildDeferredToolsSection', () => {
it('returns an empty string when no deferred tools are passed', () => {
expect(buildDeferredToolsSection([])).toBe('');
Expand Down
20 changes: 0 additions & 20 deletions packages/core/src/core/prompts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -896,26 +896,6 @@ function getToolCallExamples(model?: string): string {
return generalToolCallExamples;
}

/**
* Generates a system reminder message about available subagents for the AI assistant.
*
* This function creates an internal system message that informs the AI about specialized
* agents it can delegate tasks to. The reminder encourages proactive use of the TASK tool
* when user requests match agent capabilities.
*
* @param agentTypes - Array of available agent type names (e.g., ['python', 'web', 'analysis'])
* @returns A formatted system reminder string wrapped in XML tags for internal AI processing
*
* @example
* ```typescript
* const reminder = getSubagentSystemReminder(['python', 'web']);
* // Returns: "<system-reminder>You have powerful specialized agents..."
* ```
*/
export function getSubagentSystemReminder(agentTypes: string[]): string {
return `<system-reminder>You have powerful specialized agents at your disposal, available agent types are: ${agentTypes.join(', ')}. PROACTIVELY use the ${ToolNames.AGENT} tool to delegate user's task to appropriate agent when user's task matches agent capabilities. Ignore this message if user's task is not relevant to any agent. This message is for internal use only. Do not mention this to user in your response.</system-reminder>`;
}

/**
* Generates a system reminder message for plan mode operation.
*
Expand Down
Loading