feat: enhance team provisioning with member spawn auditing and improved error handling
- Simplified member spawn event handling by removing redundant checks for missing parameters. - Introduced a new audit function to verify registered members against expected members post-provisioning, flagging any discrepancies. - Updated logging to provide clearer warnings for unregistered members after provisioning. - Enhanced test cases to ensure accurate behavior of spawn handling and auditing processes.
This commit is contained in:
parent
48b485c637
commit
cba10b8ad4
5 changed files with 85 additions and 40 deletions
|
|
@ -1057,7 +1057,8 @@ function buildProvisioningPrompt(request: TeamCreateRequest): string {
|
||||||
Per-member spawn instructions:
|
Per-member spawn instructions:
|
||||||
${request.members
|
${request.members
|
||||||
.map(
|
.map(
|
||||||
(m) => ` For “${m.name}” (name: “${m.name}”):
|
(m) => ` For “${m.name}”:
|
||||||
|
- name: “${m.name}”
|
||||||
- prompt:
|
- prompt:
|
||||||
${buildMemberSpawnPrompt(
|
${buildMemberSpawnPrompt(
|
||||||
m,
|
m,
|
||||||
|
|
@ -4175,7 +4176,6 @@ export class TeamProvisioningService {
|
||||||
/**
|
/**
|
||||||
* Intercept Task tool_use blocks that spawn team members.
|
* Intercept Task tool_use blocks that spawn team members.
|
||||||
* Sets member spawn status to 'spawning' when the lead issues a Task call with team_name + name.
|
* Sets member spawn status to 'spawning' when the lead issues a Task call with team_name + name.
|
||||||
* Detects bad spawns (missing team_name/name) and marks them as 'error'.
|
|
||||||
*/
|
*/
|
||||||
private captureTeamSpawnEvents(run: ProvisioningRun, content: Record<string, unknown>[]): void {
|
private captureTeamSpawnEvents(run: ProvisioningRun, content: Record<string, unknown>[]): void {
|
||||||
for (const part of content) {
|
for (const part of content) {
|
||||||
|
|
@ -4185,39 +4185,10 @@ export class TeamProvisioningService {
|
||||||
const inp = input as Record<string, unknown>;
|
const inp = input as Record<string, unknown>;
|
||||||
const teamName = typeof inp.team_name === 'string' ? inp.team_name.trim() : '';
|
const teamName = typeof inp.team_name === 'string' ? inp.team_name.trim() : '';
|
||||||
const memberName = typeof inp.name === 'string' ? inp.name.trim() : '';
|
const memberName = typeof inp.name === 'string' ? inp.name.trim() : '';
|
||||||
|
if (!teamName || !memberName) continue;
|
||||||
if (teamName && memberName) {
|
// Only track spawns for this team
|
||||||
// Good spawn — has both required params
|
if (teamName !== run.teamName) continue;
|
||||||
if (teamName !== run.teamName) continue;
|
this.setMemberSpawnStatus(run, memberName, 'spawning');
|
||||||
this.setMemberSpawnStatus(run, memberName, 'spawning');
|
|
||||||
continue;
|
|
||||||
}
|
|
||||||
|
|
||||||
// Bad spawn — Task tool_use without team_name or name.
|
|
||||||
// Try to identify which member this was supposed to be from prompt/description text.
|
|
||||||
if (run.expectedMembers.length === 0) continue;
|
|
||||||
const desc = typeof inp.description === 'string' ? inp.description : '';
|
|
||||||
const prompt = typeof inp.prompt === 'string' ? inp.prompt : '';
|
|
||||||
const haystack = `${desc}\n${prompt}`.toLowerCase();
|
|
||||||
for (const expected of run.expectedMembers) {
|
|
||||||
if (haystack.includes(expected.toLowerCase())) {
|
|
||||||
const missing = !teamName && !memberName
|
|
||||||
? 'team_name and name'
|
|
||||||
: !teamName
|
|
||||||
? 'team_name'
|
|
||||||
: 'name';
|
|
||||||
logger.warn(
|
|
||||||
`[${run.teamName}] Bad teammate spawn for "${expected}": missing ${missing} param(s) in Task tool_use`
|
|
||||||
);
|
|
||||||
this.setMemberSpawnStatus(
|
|
||||||
run,
|
|
||||||
expected,
|
|
||||||
'error',
|
|
||||||
`Teammate spawned without ${missing} — will not join the team. Restart the team to fix.`
|
|
||||||
);
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -4237,6 +4208,63 @@ export class TeamProvisioningService {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Post-provisioning audit: read config.json members and flag any expectedMember
|
||||||
|
* that was NOT registered by Claude Code as a team member.
|
||||||
|
*
|
||||||
|
* This is the ground-truth check — when Agent(team_name=X, name=Y) succeeds,
|
||||||
|
* the CLI adds Y to config.json members[]. If a member is missing, the spawn
|
||||||
|
* was incorrect (e.g., missing team_name/name params) and the agent ran as a
|
||||||
|
* one-shot subagent instead of a persistent teammate.
|
||||||
|
*/
|
||||||
|
private async auditMemberSpawnStatuses(run: ProvisioningRun): Promise<void> {
|
||||||
|
if (!run.expectedMembers || run.expectedMembers.length === 0) return;
|
||||||
|
|
||||||
|
// Read config.json to get the actual registered members
|
||||||
|
const configPath = path.join(getTeamsBasePath(), run.teamName, 'config.json');
|
||||||
|
let registeredNames: Set<string>;
|
||||||
|
try {
|
||||||
|
const raw = await tryReadRegularFileUtf8(configPath, {
|
||||||
|
timeoutMs: TEAM_JSON_READ_TIMEOUT_MS,
|
||||||
|
maxBytes: TEAM_CONFIG_MAX_BYTES,
|
||||||
|
});
|
||||||
|
if (!raw) {
|
||||||
|
logger.warn(`[${run.teamName}] auditMemberSpawnStatuses: config.json not readable`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const config = JSON.parse(raw) as {
|
||||||
|
members?: { name?: string; agentType?: string }[];
|
||||||
|
};
|
||||||
|
registeredNames = new Set(
|
||||||
|
(config.members ?? [])
|
||||||
|
.map((m) => (typeof m.name === 'string' ? m.name.trim() : ''))
|
||||||
|
.filter(Boolean)
|
||||||
|
);
|
||||||
|
} catch {
|
||||||
|
logger.warn(`[${run.teamName}] auditMemberSpawnStatuses: failed to parse config.json`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Flag any expected member not found in config.json (excluding the lead)
|
||||||
|
for (const expected of run.expectedMembers) {
|
||||||
|
if (registeredNames.has(expected)) continue;
|
||||||
|
|
||||||
|
// Skip if already in a terminal status (e.g., previously set 'error')
|
||||||
|
const current = run.memberSpawnStatuses.get(expected);
|
||||||
|
if (current?.status === 'error') continue;
|
||||||
|
|
||||||
|
logger.warn(
|
||||||
|
`[${run.teamName}] Member "${expected}" not found in config.json members after provisioning`
|
||||||
|
);
|
||||||
|
this.setMemberSpawnStatus(
|
||||||
|
run,
|
||||||
|
expected,
|
||||||
|
'error',
|
||||||
|
'Teammate not registered after provisioning — spawned incorrectly. Restart the team to fix.'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private captureSendMessages(run: ProvisioningRun, content: Record<string, unknown>[]): void {
|
private captureSendMessages(run: ProvisioningRun, content: Record<string, unknown>[]): void {
|
||||||
for (const part of content) {
|
for (const part of content) {
|
||||||
if (part.type !== 'tool_use' || typeof part.name !== 'string') continue;
|
if (part.type !== 'tool_use' || typeof part.name !== 'string') continue;
|
||||||
|
|
@ -5504,6 +5532,9 @@ export class TeamProvisioningService {
|
||||||
/* best-effort */
|
/* best-effort */
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Audit: flag any expected member not registered in config.json after launch.
|
||||||
|
await this.auditMemberSpawnStatuses(run);
|
||||||
|
|
||||||
const readyMessage = 'Team launched — process alive and ready';
|
const readyMessage = 'Team launched — process alive and ready';
|
||||||
const progress = updateProgress(run, 'ready', readyMessage, {
|
const progress = updateProgress(run, 'ready', readyMessage, {
|
||||||
cliLogsTail: extractCliLogsFromRun(run),
|
cliLogsTail: extractCliLogsFromRun(run),
|
||||||
|
|
@ -5595,6 +5626,9 @@ export class TeamProvisioningService {
|
||||||
run.request.color
|
run.request.color
|
||||||
);
|
);
|
||||||
|
|
||||||
|
// Audit: flag any expected member not registered in config.json after provisioning.
|
||||||
|
await this.auditMemberSpawnStatuses(run);
|
||||||
|
|
||||||
const progress = updateProgress(run, 'ready', 'Team provisioned — process alive and ready', {
|
const progress = updateProgress(run, 'ready', 'Team provisioned — process alive and ready', {
|
||||||
cliLogsTail: extractCliLogsFromRun(run),
|
cliLogsTail: extractCliLogsFromRun(run),
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -274,7 +274,7 @@ describe('TeamProvisioningService relayLeadInboxMessages', () => {
|
||||||
const payload = String(writeSpy.mock.calls[0]?.[0] ?? '');
|
const payload = String(writeSpy.mock.calls[0]?.[0] ?? '');
|
||||||
expect(payload).toContain('Source: system_notification');
|
expect(payload).toContain('Source: system_notification');
|
||||||
expect(payload).toContain('summary looks like \\"Comment on #...\\"');
|
expect(payload).toContain('summary looks like \\"Comment on #...\\"');
|
||||||
expect(payload).toContain('Prefer replying on the task via task_add_comment');
|
expect(payload).toContain('REQUIRES an on-task reply via task_add_comment');
|
||||||
|
|
||||||
(service as any).handleStreamJsonMessage(run, {
|
(service as any).handleStreamJsonMessage(run, {
|
||||||
type: 'assistant',
|
type: 'assistant',
|
||||||
|
|
|
||||||
|
|
@ -44,7 +44,11 @@ describe('cli child process helpers', () => {
|
||||||
(child.spawn as unknown as Mock).mockReturnValue({} as any);
|
(child.spawn as unknown as Mock).mockReturnValue({} as any);
|
||||||
|
|
||||||
const result = spawnCli('C:\\bin\\claude.exe', ['--version'], { cwd: 'x' });
|
const result = spawnCli('C:\\bin\\claude.exe', ['--version'], { cwd: 'x' });
|
||||||
expect(child.spawn).toHaveBeenCalledWith('C:\\bin\\claude.exe', ['--version'], { cwd: 'x' });
|
expect(child.spawn).toHaveBeenCalledWith(
|
||||||
|
'C:\\bin\\claude.exe',
|
||||||
|
['--version'],
|
||||||
|
expect.objectContaining({ cwd: 'x', env: expect.objectContaining({ CLAUDE_HOOK_JUDGE_MODE: 'true' }) })
|
||||||
|
);
|
||||||
expect(result).toEqual({} as any);
|
expect(result).toEqual({} as any);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
@ -91,7 +95,11 @@ describe('cli child process helpers', () => {
|
||||||
setPlatform('linux');
|
setPlatform('linux');
|
||||||
(child.spawn as unknown as Mock).mockReturnValue({} as any);
|
(child.spawn as unknown as Mock).mockReturnValue({} as any);
|
||||||
const result = spawnCli('/usr/bin/claude', ['--help']);
|
const result = spawnCli('/usr/bin/claude', ['--help']);
|
||||||
expect(child.spawn).toHaveBeenCalledWith('/usr/bin/claude', ['--help'], {});
|
expect(child.spawn).toHaveBeenCalledWith(
|
||||||
|
'/usr/bin/claude',
|
||||||
|
['--help'],
|
||||||
|
expect.objectContaining({ env: expect.objectContaining({ CLAUDE_HOOK_JUDGE_MODE: 'true' }) })
|
||||||
|
);
|
||||||
expect(result).toEqual({} as any);
|
expect(result).toEqual({} as any);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
@ -110,7 +118,7 @@ describe('cli child process helpers', () => {
|
||||||
expect(execFileMock).toHaveBeenCalledWith(
|
expect(execFileMock).toHaveBeenCalledWith(
|
||||||
'C:\\bin\\claude.exe',
|
'C:\\bin\\claude.exe',
|
||||||
['--version'],
|
['--version'],
|
||||||
{},
|
expect.objectContaining({ env: expect.objectContaining({ CLAUDE_HOOK_JUDGE_MODE: 'true' }) }),
|
||||||
expect.any(Function)
|
expect.any(Function)
|
||||||
);
|
);
|
||||||
expect(result.stdout).toBe('ok');
|
expect(result.stdout).toBe('ok');
|
||||||
|
|
|
||||||
|
|
@ -10,7 +10,7 @@ import {
|
||||||
describe('ActivityItem legacy system message fallback', () => {
|
describe('ActivityItem legacy system message fallback', () => {
|
||||||
it('recognizes historical assignment and review message wording', () => {
|
it('recognizes historical assignment and review message wording', () => {
|
||||||
expect(getSystemMessageLabel('New task assigned to you: #abcd1234 "Implement feature".')).toBe(
|
expect(getSystemMessageLabel('New task assigned to you: #abcd1234 "Implement feature".')).toBe(
|
||||||
'Task assignment'
|
'Task'
|
||||||
);
|
);
|
||||||
expect(getSystemMessageLabel('Task #abcd1234 approved by reviewer.')).toBe('Task approved');
|
expect(getSystemMessageLabel('Task #abcd1234 approved by reviewer.')).toBe('Task approved');
|
||||||
expect(getSystemMessageLabel('Task #abcd1234 needs fixes before approval.')).toBe(
|
expect(getSystemMessageLabel('Task #abcd1234 needs fixes before approval.')).toBe(
|
||||||
|
|
|
||||||
|
|
@ -290,7 +290,10 @@ describe('changeReviewSlice task changes', () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
await store.getState().checkTaskHasChanges('team-a', '1', REVIEW_OPTIONS);
|
await store.getState().checkTaskHasChanges('team-a', '1', REVIEW_OPTIONS);
|
||||||
|
// Expire the 30s negative-cache TTL so the second call actually hits the API
|
||||||
|
vi.spyOn(Date, 'now').mockReturnValue(Date.now() + 31_000);
|
||||||
await store.getState().checkTaskHasChanges('team-a', '1', REVIEW_OPTIONS);
|
await store.getState().checkTaskHasChanges('team-a', '1', REVIEW_OPTIONS);
|
||||||
|
vi.mocked(Date.now).mockRestore();
|
||||||
|
|
||||||
expect(hoisted.getTaskChanges).toHaveBeenCalledTimes(2);
|
expect(hoisted.getTaskChanges).toHaveBeenCalledTimes(2);
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue