feat: update tool output format, use plain string instead of json string (#881)

This commit is contained in:
tanzhenxin
2025-10-27 17:26:47 +08:00
committed by GitHub
parent 2a5577e5d7
commit 4328cd7f63
9 changed files with 188 additions and 117 deletions

View File

@@ -131,16 +131,14 @@ describe('ExitPlanModeTool', () => {
}
const result = await invocation.execute(signal);
const expectedLlmMessage =
'User has approved your plan. You can now start coding. Start with updating your todo list if applicable.';
expect(result).toEqual({
llmContent: expectedLlmMessage,
returnDisplay: {
type: 'plan_summary',
message: 'User approved the plan.',
plan: params.plan,
},
expect(result.llmContent).toContain(
'User has approved your plan. You can now start coding',
);
expect(result.returnDisplay).toEqual({
type: 'plan_summary',
message: 'User approved the plan.',
plan: params.plan,
});
expect(mockConfig.setApprovalMode).toHaveBeenCalledWith(
@@ -188,15 +186,12 @@ describe('ExitPlanModeTool', () => {
const result = await invocation.execute(signal);
expect(result).toEqual({
llmContent: JSON.stringify({
success: false,
plan: params.plan,
error: 'Plan execution was not approved. Remaining in plan mode.',
}),
returnDisplay:
'Plan execution was not approved. Remaining in plan mode.',
});
expect(result.llmContent).toBe(
'Plan execution was not approved. Remaining in plan mode.',
);
expect(result.returnDisplay).toBe(
'Plan execution was not approved. Remaining in plan mode.',
);
expect(mockConfig.setApprovalMode).toHaveBeenCalledWith(
ApprovalMode.PLAN,
@@ -215,50 +210,6 @@ describe('ExitPlanModeTool', () => {
);
});
it('should handle execution errors gracefully', async () => {
const params: ExitPlanModeParams = {
plan: 'Test plan',
};
const invocation = tool.build(params);
const confirmation = await invocation.shouldConfirmExecute(
new AbortController().signal,
);
if (confirmation) {
// Don't approve the plan so we go through the rejection path
await confirmation.onConfirm(ToolConfirmationOutcome.Cancel);
}
// Create a spy to simulate an error during the execution
const consoleSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
// Mock JSON.stringify to throw an error in the rejection path
const originalStringify = JSON.stringify;
vi.spyOn(JSON, 'stringify').mockImplementationOnce(() => {
throw new Error('JSON stringify error');
});
const result = await invocation.execute(new AbortController().signal);
expect(result).toEqual({
llmContent: JSON.stringify({
success: false,
error: 'Failed to present plan. Detail: JSON stringify error',
}),
returnDisplay: 'Error presenting plan: JSON stringify error',
});
expect(consoleSpy).toHaveBeenCalledWith(
'[ExitPlanModeTool] Error executing exit_plan_mode: JSON stringify error',
);
// Restore original JSON.stringify
JSON.stringify = originalStringify;
consoleSpy.mockRestore();
});
it('should return empty tool locations', () => {
const params: ExitPlanModeParams = {
plan: 'Test plan',

View File

@@ -115,17 +115,12 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation<
const rejectionMessage =
'Plan execution was not approved. Remaining in plan mode.';
return {
llmContent: JSON.stringify({
success: false,
plan,
error: rejectionMessage,
}),
llmContent: rejectionMessage,
returnDisplay: rejectionMessage,
};
}
const llmMessage =
'User has approved your plan. You can now start coding. Start with updating your todo list if applicable.';
const llmMessage = `User has approved your plan. You can now start coding. Start with updating your todo list if applicable.`;
const displayMessage = 'User approved the plan.';
return {
@@ -142,11 +137,11 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation<
console.error(
`[ExitPlanModeTool] Error executing exit_plan_mode: ${errorMessage}`,
);
const errorLlmContent = `Failed to present plan: ${errorMessage}`;
return {
llmContent: JSON.stringify({
success: false,
error: `Failed to present plan. Detail: ${errorMessage}`,
}),
llmContent: errorLlmContent,
returnDisplay: `Error presenting plan: ${errorMessage}`,
};
}

View File

@@ -241,9 +241,7 @@ describe('MemoryTool', () => {
expectedFsArgument,
);
const successMessage = `Okay, I've remembered that in global memory: "${params.fact}"`;
expect(result.llmContent).toBe(
JSON.stringify({ success: true, message: successMessage }),
);
expect(result.llmContent).toBe(successMessage);
expect(result.returnDisplay).toBe(successMessage);
});
@@ -271,9 +269,7 @@ describe('MemoryTool', () => {
expectedFsArgument,
);
const successMessage = `Okay, I've remembered that in project memory: "${params.fact}"`;
expect(result.llmContent).toBe(
JSON.stringify({ success: true, message: successMessage }),
);
expect(result.llmContent).toBe(successMessage);
expect(result.returnDisplay).toBe(successMessage);
});
@@ -298,10 +294,7 @@ describe('MemoryTool', () => {
const result = await invocation.execute(mockAbortSignal);
expect(result.llmContent).toBe(
JSON.stringify({
success: false,
error: `Failed to save memory. Detail: ${underlyingError.message}`,
}),
`Error saving memory: ${underlyingError.message}`,
);
expect(result.returnDisplay).toBe(
`Error saving memory: ${underlyingError.message}`,
@@ -319,6 +312,8 @@ describe('MemoryTool', () => {
expect(result.llmContent).toContain(
'Please specify where to save this memory',
);
expect(result.llmContent).toContain('Global:');
expect(result.llmContent).toContain('Project:');
expect(result.returnDisplay).toContain('Global:');
expect(result.returnDisplay).toContain('Project:');
});

View File

@@ -309,7 +309,7 @@ Preview of changes to be made to GLOBAL memory:
if (!fact || typeof fact !== 'string' || fact.trim() === '') {
const errorMessage = 'Parameter "fact" must be a non-empty string.';
return {
llmContent: JSON.stringify({ success: false, error: errorMessage }),
llmContent: `Error: ${errorMessage}`,
returnDisplay: `Error: ${errorMessage}`,
};
}
@@ -324,10 +324,7 @@ Global: ${globalPath} (shared across all projects)
Project: ${projectPath} (current project only)`;
return {
llmContent: JSON.stringify({
success: false,
error: 'Please specify where to save this memory',
}),
llmContent: errorMessage,
returnDisplay: errorMessage,
};
}
@@ -344,10 +341,7 @@ Project: ${projectPath} (current project only)`;
await fs.writeFile(memoryFilePath, modified_content, 'utf-8');
const successMessage = `Okay, I've updated the ${scope} memory file with your modifications.`;
return {
llmContent: JSON.stringify({
success: true,
message: successMessage,
}),
llmContent: successMessage,
returnDisplay: successMessage,
};
} else {
@@ -359,10 +353,7 @@ Project: ${projectPath} (current project only)`;
});
const successMessage = `Okay, I've remembered that in ${scope} memory: "${fact}"`;
return {
llmContent: JSON.stringify({
success: true,
message: successMessage,
}),
llmContent: successMessage,
returnDisplay: successMessage,
};
}
@@ -372,11 +363,9 @@ Project: ${projectPath} (current project only)`;
console.error(
`[MemoryTool] Error executing save_memory for fact "${fact}" in ${scope}: ${errorMessage}`,
);
return {
llmContent: JSON.stringify({
success: false,
error: `Failed to save memory. Detail: ${errorMessage}`,
}),
llmContent: `Error saving memory: ${errorMessage}`,
returnDisplay: `Error saving memory: ${errorMessage}`,
error: {
message: errorMessage,

View File

@@ -141,7 +141,12 @@ describe('TodoWriteTool', () => {
const invocation = tool.build(params);
const result = await invocation.execute(mockAbortSignal);
expect(result.llmContent).toContain('success');
expect(result.llmContent).toContain(
'Todos have been modified successfully',
);
expect(result.llmContent).toContain('<system-reminder>');
expect(result.llmContent).toContain('Your todo list has changed');
expect(result.llmContent).toContain(JSON.stringify(params.todos));
expect(result.returnDisplay).toEqual({
type: 'todo_list',
todos: [
@@ -178,7 +183,12 @@ describe('TodoWriteTool', () => {
const invocation = tool.build(params);
const result = await invocation.execute(mockAbortSignal);
expect(result.llmContent).toContain('success');
expect(result.llmContent).toContain(
'Todos have been modified successfully',
);
expect(result.llmContent).toContain('<system-reminder>');
expect(result.llmContent).toContain('Your todo list has changed');
expect(result.llmContent).toContain(JSON.stringify(params.todos));
expect(result.returnDisplay).toEqual({
type: 'todo_list',
todos: [
@@ -208,7 +218,10 @@ describe('TodoWriteTool', () => {
const invocation = tool.build(params);
const result = await invocation.execute(mockAbortSignal);
expect(result.llmContent).toContain('"success":false');
expect(result.llmContent).toContain('Failed to modify todos');
expect(result.llmContent).toContain('<system-reminder>');
expect(result.llmContent).toContain('Todo list modification failed');
expect(result.llmContent).toContain('Write failed');
expect(result.returnDisplay).toContain('Error writing todos');
});
@@ -223,7 +236,10 @@ describe('TodoWriteTool', () => {
const invocation = tool.build(params);
const result = await invocation.execute(mockAbortSignal);
expect(result.llmContent).toContain('success');
expect(result.llmContent).toContain('Todo list has been cleared');
expect(result.llmContent).toContain('<system-reminder>');
expect(result.llmContent).toContain('Your todo list is now empty');
expect(result.llmContent).toContain('no pending tasks');
expect(result.returnDisplay).toEqual({
type: 'todo_list',
todos: [],

View File

@@ -340,11 +340,30 @@ class TodoWriteToolInvocation extends BaseToolInvocation<
todos: finalTodos,
};
// Create plain string format with system reminder
const todosJson = JSON.stringify(finalTodos);
let llmContent: string;
if (finalTodos.length === 0) {
// Special message for empty todos
llmContent = `Todo list has been cleared.
<system-reminder>
Your todo list is now empty. DO NOT mention this explicitly to the user. You have no pending tasks in your todo list.
</system-reminder>`;
} else {
// Normal message for todos with items
llmContent = `Todos have been modified successfully. Ensure that you continue to use the todo list to track your progress. Please proceed with the current tasks if applicable
<system-reminder>
Your todo list has changed. DO NOT mention this explicitly to the user. Here are the latest contents of your todo list:
${todosJson}. Continue on with the tasks at hand if applicable.
</system-reminder>`;
}
return {
llmContent: JSON.stringify({
success: true,
todos: finalTodos,
}),
llmContent,
returnDisplay: todoResultDisplay,
};
} catch (error) {
@@ -353,11 +372,16 @@ class TodoWriteToolInvocation extends BaseToolInvocation<
console.error(
`[TodoWriteTool] Error executing todo_write: ${errorMessage}`,
);
// Create plain string format for error with system reminder
const errorLlmContent = `Failed to modify todos. An error occurred during the operation.
<system-reminder>
Todo list modification failed with error: ${errorMessage}. You may need to retry or handle this error appropriately.
</system-reminder>`;
return {
llmContent: JSON.stringify({
success: false,
error: `Failed to write todos. Detail: ${errorMessage}`,
}),
llmContent: errorLlmContent,
returnDisplay: `Error writing todos: ${errorMessage}`,
};
}