Skip to content

Commit 7b15dfe

Browse files
committed
fix(core): fix test failures related to root execution and optional subagent_type
- Skip pathReader and edit tool permission tests when running as root - Fix agent.test.ts to correctly mock execute call with extraHistory - Remove unused imports in forkSubagent.ts
1 parent 3ff1f02 commit 7b15dfe

4 files changed

Lines changed: 48 additions & 18 deletions

File tree

packages/core/src/tools/agent.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,7 @@ describe('AgentTool', () => {
398398
expect(mockAgent.execute).toHaveBeenCalledWith(
399399
mockContextState,
400400
undefined, // signal parameter (undefined when not provided)
401+
{ extraHistory: undefined }, // extraHistory
401402
);
402403

403404
const llmText = partToString(result.llmContent);

packages/core/src/tools/agent.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,13 @@ assistant: "I'm going to use the ${ToolNames.AGENT} tool to launch the greeting-
239239
return 'Parameter "prompt" must be a non-empty string.';
240240
}
241241

242-
if (params.subagent_type) {
242+
if (params.subagent_type !== undefined) {
243+
if (
244+
typeof params.subagent_type !== 'string' ||
245+
params.subagent_type.trim() === ''
246+
) {
247+
return 'Parameter "subagent_type" must be a non-empty string.';
248+
}
243249
// Validate that the subagent exists (case-insensitive)
244250
const lowerType = params.subagent_type.toLowerCase();
245251
const subagentExists = this.availableSubagents.some(
@@ -481,13 +487,31 @@ class AgentToolInvocation extends BaseToolInvocation<AgentParams, ToolResult> {
481487
let extraHistory: Array<import('@google/genai').Content> | undefined;
482488

483489
if (!this.params.subagent_type) {
484-
const { FORK_AGENT, buildForkedMessages } = await import(
490+
const { FORK_AGENT, buildForkedMessages, isInForkChild } = await import(
485491
'../agents/runtime/forkSubagent.js'
486492
);
487493
subagentConfig = FORK_AGENT;
488494
const geminiClient = this.config.getGeminiClient();
489495
if (geminiClient) {
490496
const rawHistory = geminiClient.getHistory(true);
497+
498+
if (isInForkChild(rawHistory)) {
499+
const errorDisplay = {
500+
type: 'task_execution' as const,
501+
subagentName: FORK_AGENT.name,
502+
taskDescription: this.params.description,
503+
taskPrompt: this.params.prompt,
504+
status: 'failed' as const,
505+
terminateReason: 'Recursive forking is not allowed',
506+
};
507+
508+
return {
509+
llmContent:
510+
'Error: Cannot create a fork from within an existing fork child. Please execute tasks directly.',
511+
returnDisplay: errorDisplay,
512+
};
513+
}
514+
491515
if (rawHistory.length > 0) {
492516
const lastMessage = rawHistory[rawHistory.length - 1];
493517
if (lastMessage.role === 'model') {

packages/core/src/tools/edit.test.ts

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -784,20 +784,23 @@ describe('EditTool', () => {
784784
expect(() => tool.build(params)).toThrow();
785785
});
786786

787-
it('should return FILE_WRITE_FAILURE on write error', async () => {
788-
fs.writeFileSync(filePath, 'content', 'utf8');
789-
// Make file readonly to trigger a write error
790-
fs.chmodSync(filePath, '444');
791-
792-
const params: EditToolParams = {
793-
file_path: filePath,
794-
old_string: 'content',
795-
new_string: 'new content',
796-
};
797-
const invocation = tool.build(params);
798-
const result = await invocation.execute(new AbortController().signal);
799-
expect(result.error?.type).toBe(ToolErrorType.FILE_WRITE_FAILURE);
800-
});
787+
it.skipIf(process.getuid && process.getuid() === 0)(
788+
'should return FILE_WRITE_FAILURE on write error',
789+
async () => {
790+
fs.writeFileSync(filePath, 'content', 'utf8');
791+
// Make file readonly to trigger a write error
792+
fs.chmodSync(filePath, '444');
793+
794+
const params: EditToolParams = {
795+
file_path: filePath,
796+
old_string: 'content',
797+
new_string: 'new content',
798+
};
799+
const invocation = tool.build(params);
800+
const result = await invocation.execute(new AbortController().signal);
801+
expect(result.error?.type).toBe(ToolErrorType.FILE_WRITE_FAILURE);
802+
},
803+
);
801804
});
802805

803806
describe('getDescription', () => {

packages/core/src/utils/pathReader.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,8 +368,10 @@ describe('readPathFromWorkspace', () => {
368368
).rejects.toThrow('Path not found in workspace: not-found.txt');
369369
});
370370

371-
// mock-fs permission simulation is unreliable on Windows.
372-
it.skipIf(process.platform === 'win32')(
371+
// mock-fs permission simulation is unreliable on Windows and when running as root.
372+
it.skipIf(
373+
process.platform === 'win32' || (process.getuid && process.getuid() === 0),
374+
)(
373375
'should return an error string if reading a file with no permissions',
374376
async () => {
375377
mock({

0 commit comments

Comments
 (0)