refactor: enhance task and review tools with slimTask integration
- Updated reviewTools to utilize slimTask for lightweight task representations, improving response efficiency. - Enhanced taskTools to apply slimTaskForList for task listing, reducing data payloads. - Introduced new slimTask and slimTaskForList functions in format utility for better task data management. - Improved test coverage for slim task operations, ensuring accurate behavior in task handling and review processes.
This commit is contained in:
parent
15df012c4b
commit
0f91698fa8
5 changed files with 163 additions and 52 deletions
|
|
@ -2,7 +2,7 @@ import type { FastMCP } from 'fastmcp';
|
||||||
import { z } from 'zod';
|
import { z } from 'zod';
|
||||||
|
|
||||||
import { getController } from '../controller';
|
import { getController } from '../controller';
|
||||||
import { jsonTextContent } from '../utils/format';
|
import { jsonTextContent, slimTask } from '../utils/format';
|
||||||
|
|
||||||
const toolContextSchema = {
|
const toolContextSchema = {
|
||||||
teamName: z.string().min(1),
|
teamName: z.string().min(1),
|
||||||
|
|
@ -23,11 +23,13 @@ export function registerReviewTools(server: Pick<FastMCP, 'addTool'>) {
|
||||||
execute: async ({ teamName, claudeDir, taskId, from, reviewer, leadSessionId }) =>
|
execute: async ({ teamName, claudeDir, taskId, from, reviewer, leadSessionId }) =>
|
||||||
await Promise.resolve(
|
await Promise.resolve(
|
||||||
jsonTextContent(
|
jsonTextContent(
|
||||||
getController(teamName, claudeDir).review.requestReview(taskId, {
|
slimTask(
|
||||||
...(from ? { from } : {}),
|
getController(teamName, claudeDir).review.requestReview(taskId, {
|
||||||
...(reviewer ? { reviewer } : {}),
|
...(from ? { from } : {}),
|
||||||
...(leadSessionId ? { leadSessionId } : {}),
|
...(reviewer ? { reviewer } : {}),
|
||||||
})
|
...(leadSessionId ? { leadSessionId } : {}),
|
||||||
|
}) as Record<string, unknown>
|
||||||
|
)
|
||||||
)
|
)
|
||||||
),
|
),
|
||||||
});
|
});
|
||||||
|
|
@ -46,12 +48,14 @@ export function registerReviewTools(server: Pick<FastMCP, 'addTool'>) {
|
||||||
execute: async ({ teamName, claudeDir, taskId, from, note, notifyOwner, leadSessionId }) =>
|
execute: async ({ teamName, claudeDir, taskId, from, note, notifyOwner, leadSessionId }) =>
|
||||||
await Promise.resolve(
|
await Promise.resolve(
|
||||||
jsonTextContent(
|
jsonTextContent(
|
||||||
getController(teamName, claudeDir).review.approveReview(taskId, {
|
slimTask(
|
||||||
...(from ? { from } : {}),
|
getController(teamName, claudeDir).review.approveReview(taskId, {
|
||||||
...(note ? { note } : {}),
|
...(from ? { from } : {}),
|
||||||
...(notifyOwner !== false ? { 'notify-owner': true } : {}),
|
...(note ? { note } : {}),
|
||||||
...(leadSessionId ? { leadSessionId } : {}),
|
...(notifyOwner !== false ? { 'notify-owner': true } : {}),
|
||||||
})
|
...(leadSessionId ? { leadSessionId } : {}),
|
||||||
|
}) as Record<string, unknown>
|
||||||
|
)
|
||||||
)
|
)
|
||||||
),
|
),
|
||||||
});
|
});
|
||||||
|
|
@ -69,11 +73,13 @@ export function registerReviewTools(server: Pick<FastMCP, 'addTool'>) {
|
||||||
execute: async ({ teamName, claudeDir, taskId, from, comment, leadSessionId }) =>
|
execute: async ({ teamName, claudeDir, taskId, from, comment, leadSessionId }) =>
|
||||||
await Promise.resolve(
|
await Promise.resolve(
|
||||||
jsonTextContent(
|
jsonTextContent(
|
||||||
getController(teamName, claudeDir).review.requestChanges(taskId, {
|
slimTask(
|
||||||
...(from ? { from } : {}),
|
getController(teamName, claudeDir).review.requestChanges(taskId, {
|
||||||
...(comment ? { comment } : {}),
|
...(from ? { from } : {}),
|
||||||
...(leadSessionId ? { leadSessionId } : {}),
|
...(comment ? { comment } : {}),
|
||||||
})
|
...(leadSessionId ? { leadSessionId } : {}),
|
||||||
|
}) as Record<string, unknown>
|
||||||
|
)
|
||||||
)
|
)
|
||||||
),
|
),
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -2,7 +2,7 @@ import type { FastMCP } from 'fastmcp';
|
||||||
import { z } from 'zod';
|
import { z } from 'zod';
|
||||||
|
|
||||||
import { agentBlocks, getController } from '../controller';
|
import { agentBlocks, getController } from '../controller';
|
||||||
import { jsonTextContent, taskWriteResult, slimTask } from '../utils/format';
|
import { jsonTextContent, taskWriteResult, slimTask, slimTaskForList } from '../utils/format';
|
||||||
|
|
||||||
/** stripAgentBlocks from canonical agentBlocks module — single source of truth for the tag format. */
|
/** stripAgentBlocks from canonical agentBlocks module — single source of truth for the tag format. */
|
||||||
const { stripAgentBlocks } = agentBlocks;
|
const { stripAgentBlocks } = agentBlocks;
|
||||||
|
|
@ -228,7 +228,11 @@ export function registerTaskTools(server: Pick<FastMCP, 'addTool'>) {
|
||||||
...toolContextSchema,
|
...toolContextSchema,
|
||||||
}),
|
}),
|
||||||
execute: async ({ teamName, claudeDir }) =>
|
execute: async ({ teamName, claudeDir }) =>
|
||||||
await Promise.resolve(jsonTextContent(getController(teamName, claudeDir).tasks.listTasks())),
|
await Promise.resolve(
|
||||||
|
jsonTextContent(
|
||||||
|
(getController(teamName, claudeDir).tasks.listTasks() as Record<string, unknown>[]).map(slimTaskForList)
|
||||||
|
)
|
||||||
|
),
|
||||||
});
|
});
|
||||||
|
|
||||||
server.addTool({
|
server.addTool({
|
||||||
|
|
|
||||||
|
|
@ -25,21 +25,53 @@ export function taskWriteResult(result: Record<string, unknown>): Record<string,
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Strips heavy fields from a raw task object returned directly by status/owner
|
* Minimal task confirmation for write operations (status changes, owner
|
||||||
* mutations (not wrapped in `{ task: ... }`).
|
* assignment, comments, etc.). Uses an allowlist — only fields the caller
|
||||||
|
* needs to verify the mutation succeeded. Agents already know what they
|
||||||
|
* modified, so description/prompt/timestamps are unnecessary here.
|
||||||
*/
|
*/
|
||||||
export function slimTask(full: Record<string, unknown>): Record<string, unknown> {
|
export function slimTask(full: Record<string, unknown>): Record<string, unknown> {
|
||||||
const slim: Record<string, unknown> = {
|
return {
|
||||||
id: full.id,
|
id: full.id,
|
||||||
displayId: full.displayId,
|
displayId: full.displayId,
|
||||||
subject: full.subject,
|
subject: full.subject,
|
||||||
status: full.status,
|
status: full.status,
|
||||||
owner: full.owner,
|
owner: full.owner,
|
||||||
|
reviewState: full.reviewState,
|
||||||
|
needsClarification: full.needsClarification,
|
||||||
|
blockedBy: full.blockedBy,
|
||||||
|
blocks: full.blocks,
|
||||||
|
commentCount: Array.isArray(full.comments) ? full.comments.length : 0,
|
||||||
};
|
};
|
||||||
|
}
|
||||||
|
|
||||||
const comments = full.comments;
|
/**
|
||||||
if (Array.isArray(comments)) {
|
* Fields that grow unboundedly and dominate context usage.
|
||||||
slim.commentCount = comments.length;
|
* Everything else passes through — new task fields are included by default.
|
||||||
|
*/
|
||||||
|
const HEAVY_TASK_FIELDS = new Set(['comments', 'historyEvents', 'workIntervals']);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Lightweight task representation for task_list.
|
||||||
|
*
|
||||||
|
* Uses a BLOCKLIST approach: strips only known heavy array fields and replaces
|
||||||
|
* `comments` with `commentCount`. All other fields (including any future ones)
|
||||||
|
* pass through automatically. This avoids silently dropping new fields when
|
||||||
|
* the task schema evolves.
|
||||||
|
*/
|
||||||
|
export function slimTaskForList(full: Record<string, unknown>): Record<string, unknown> {
|
||||||
|
const slim: Record<string, unknown> = {};
|
||||||
|
|
||||||
|
for (const [key, value] of Object.entries(full)) {
|
||||||
|
if (!HEAVY_TASK_FIELDS.has(key)) {
|
||||||
|
slim[key] = value;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (Array.isArray(full.comments)) {
|
||||||
|
slim.commentCount = full.comments.length;
|
||||||
|
} else {
|
||||||
|
slim.commentCount = 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
return slim;
|
return slim;
|
||||||
|
|
|
||||||
|
|
@ -918,6 +918,98 @@ describe('agent-teams-mcp tools', () => {
|
||||||
expect(reloaded.comments[0].text).toBe('Comment should persist despite broken inbox');
|
expect(reloaded.comments[0].text).toBe('Comment should persist despite broken inbox');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('write operations return slim task (no comments/historyEvents arrays)', async () => {
|
||||||
|
const claudeDir = makeClaudeDir();
|
||||||
|
const teamName = 'slim-check';
|
||||||
|
|
||||||
|
fs.mkdirSync(path.join(claudeDir, 'tasks', teamName), { recursive: true });
|
||||||
|
writeTeamConfig(claudeDir, teamName, { members: [{ name: 'lead' }] });
|
||||||
|
|
||||||
|
const task = parseJsonToolResult(
|
||||||
|
await getTool('task_create').execute({
|
||||||
|
claudeDir,
|
||||||
|
teamName,
|
||||||
|
subject: 'Slim task test',
|
||||||
|
owner: 'lead',
|
||||||
|
notifyOwner: false,
|
||||||
|
})
|
||||||
|
);
|
||||||
|
|
||||||
|
// task_create returns full task (read operation)
|
||||||
|
expect(task.historyEvents).toBeDefined();
|
||||||
|
|
||||||
|
// Add a comment so commentCount > 0
|
||||||
|
const commented = parseJsonToolResult(
|
||||||
|
await getTool('task_add_comment').execute({
|
||||||
|
claudeDir,
|
||||||
|
teamName,
|
||||||
|
taskId: task.id,
|
||||||
|
text: 'test comment',
|
||||||
|
from: 'lead',
|
||||||
|
})
|
||||||
|
);
|
||||||
|
|
||||||
|
// task_add_comment: nested task should be slim
|
||||||
|
expect(commented.commentId).toBeTruthy();
|
||||||
|
expect(commented.comment.text).toBe('test comment');
|
||||||
|
expect(commented.task.commentCount).toBe(1);
|
||||||
|
expect(commented.task.comments).toBeUndefined();
|
||||||
|
expect(commented.task.historyEvents).toBeUndefined();
|
||||||
|
|
||||||
|
// task_start: returns slim task directly
|
||||||
|
const started = parseJsonToolResult(
|
||||||
|
await getTool('task_start').execute({
|
||||||
|
claudeDir,
|
||||||
|
teamName,
|
||||||
|
taskId: task.id,
|
||||||
|
actor: 'lead',
|
||||||
|
})
|
||||||
|
);
|
||||||
|
expect(started.status).toBe('in_progress');
|
||||||
|
expect(started.commentCount).toBe(1);
|
||||||
|
expect(started.comments).toBeUndefined();
|
||||||
|
expect(started.historyEvents).toBeUndefined();
|
||||||
|
expect(started.workIntervals).toBeUndefined();
|
||||||
|
|
||||||
|
// task_complete: returns slim task directly
|
||||||
|
const completed = parseJsonToolResult(
|
||||||
|
await getTool('task_complete').execute({
|
||||||
|
claudeDir,
|
||||||
|
teamName,
|
||||||
|
taskId: task.id,
|
||||||
|
actor: 'lead',
|
||||||
|
})
|
||||||
|
);
|
||||||
|
expect(completed.status).toBe('completed');
|
||||||
|
expect(completed.comments).toBeUndefined();
|
||||||
|
|
||||||
|
// task_list: uses blocklist, includes description but not comments array
|
||||||
|
const listed = parseJsonToolResult(
|
||||||
|
await getTool('task_list').execute({ claudeDir, teamName })
|
||||||
|
);
|
||||||
|
const listedTask = listed.find((t: { id: string }) => t.id === task.id);
|
||||||
|
expect(listedTask).toBeDefined();
|
||||||
|
expect(listedTask.subject).toBe('Slim task test');
|
||||||
|
expect(listedTask.commentCount).toBe(1);
|
||||||
|
expect(listedTask.comments).toBeUndefined();
|
||||||
|
expect(listedTask.historyEvents).toBeUndefined();
|
||||||
|
expect(listedTask.workIntervals).toBeUndefined();
|
||||||
|
// task_list preserves non-heavy fields
|
||||||
|
expect(listedTask.status).toBeDefined();
|
||||||
|
expect(listedTask.id).toBeDefined();
|
||||||
|
|
||||||
|
// task_get: still returns full task with comments
|
||||||
|
const full = parseJsonToolResult(
|
||||||
|
await getTool('task_get').execute({
|
||||||
|
claudeDir,
|
||||||
|
teamName,
|
||||||
|
taskId: task.id,
|
||||||
|
})
|
||||||
|
);
|
||||||
|
expect(full.comments).toHaveLength(1);
|
||||||
|
expect(full.historyEvents).toBeDefined();
|
||||||
|
});
|
||||||
|
|
||||||
describe('task_create_from_message', () => {
|
describe('task_create_from_message', () => {
|
||||||
function writeSentMessage(
|
function writeSentMessage(
|
||||||
claudeDir: string,
|
claudeDir: string,
|
||||||
|
|
|
||||||
|
|
@ -407,7 +407,7 @@ function buildTeammateAgentBlockReminder(): string {
|
||||||
].join('\n');
|
].join('\n');
|
||||||
}
|
}
|
||||||
|
|
||||||
function buildMemberBootstrapPrompt(
|
function buildMemberSpawnPrompt(
|
||||||
member: TeamCreateRequest['members'][number],
|
member: TeamCreateRequest['members'][number],
|
||||||
displayName: string,
|
displayName: string,
|
||||||
teamName: string,
|
teamName: string,
|
||||||
|
|
@ -438,7 +438,7 @@ ${buildTeammateAgentBlockReminder()}
|
||||||
${actionModeProtocol}`;
|
${actionModeProtocol}`;
|
||||||
}
|
}
|
||||||
|
|
||||||
function buildReconnectMemberBootstrapPrompt(
|
function buildReconnectMemberSpawnPrompt(
|
||||||
member: TeamCreateRequest['members'][number],
|
member: TeamCreateRequest['members'][number],
|
||||||
teamName: string,
|
teamName: string,
|
||||||
leadName: string,
|
leadName: string,
|
||||||
|
|
@ -482,24 +482,6 @@ ${actionModeProtocol}
|
||||||
- If you have no tasks, wait for new assignments.`;
|
- If you have no tasks, wait for new assignments.`;
|
||||||
}
|
}
|
||||||
|
|
||||||
function buildMemberSpawnPrompt(
|
|
||||||
member: TeamCreateRequest['members'][number],
|
|
||||||
displayName: string,
|
|
||||||
teamName: string,
|
|
||||||
leadName: string
|
|
||||||
): string {
|
|
||||||
return buildMemberBootstrapPrompt(member, displayName, teamName, leadName);
|
|
||||||
}
|
|
||||||
|
|
||||||
function buildReconnectMemberSpawnPrompt(
|
|
||||||
member: TeamCreateRequest['members'][number],
|
|
||||||
teamName: string,
|
|
||||||
leadName: string,
|
|
||||||
hasTasks: boolean
|
|
||||||
): string {
|
|
||||||
return buildReconnectMemberBootstrapPrompt(member, teamName, leadName, hasTasks);
|
|
||||||
}
|
|
||||||
|
|
||||||
export function buildAddMemberSpawnMessage(
|
export function buildAddMemberSpawnMessage(
|
||||||
teamName: string,
|
teamName: string,
|
||||||
displayName: string,
|
displayName: string,
|
||||||
|
|
@ -515,7 +497,7 @@ export function buildAddMemberSpawnMessage(
|
||||||
? ` Their workflow: ${member.workflow.trim()}`
|
? ` Their workflow: ${member.workflow.trim()}`
|
||||||
: '';
|
: '';
|
||||||
|
|
||||||
const prompt = buildMemberBootstrapPrompt(
|
const prompt = buildMemberSpawnPrompt(
|
||||||
{
|
{
|
||||||
name: member.name,
|
name: member.name,
|
||||||
...(member.role ? { role: member.role } : {}),
|
...(member.role ? { role: member.role } : {}),
|
||||||
|
|
@ -866,12 +848,7 @@ ${request.members
|
||||||
(m) => ` For “${m.name}”:
|
(m) => ` For “${m.name}”:
|
||||||
- name: “${m.name}”
|
- name: “${m.name}”
|
||||||
- prompt:
|
- prompt:
|
||||||
${buildMemberSpawnPrompt(
|
${buildMemberSpawnPrompt(m, displayName, request.teamName, leadName)
|
||||||
m,
|
|
||||||
displayName,
|
|
||||||
request.teamName,
|
|
||||||
leadName
|
|
||||||
)
|
|
||||||
.split('\n')
|
.split('\n')
|
||||||
.map((line) => ` ${line}`)
|
.map((line) => ` ${line}`)
|
||||||
.join('\n')}`
|
.join('\n')}`
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue