Skip to content

Commit 57c89e4

Browse files
tanzhenxintanzhenxin
committed
refactor(core): generalize task_stop and send_message framing to "task"
Today every BackgroundTaskRegistry entry is a subagent, but the control-plane tools were named and described as agent-only. Generalize so future task kinds (e.g. backgrounded shells, monitors) can share the same registry without a model-facing rename. - task_stop / send_message: descriptions, error messages, and ToolError enum values drop the "agent" framing in favor of "task". - send_message: parameter to -> task_id, matching task_stop for a uniform control-plane contract. - BackgroundTaskRegistry.hasUnfinalizedAgents -> hasUnfinalizedTasks. - agent-transcript: add a TODO at getSubagentSessionDir flagging that <projectDir>/subagents/ is part of the model-facing contract via <output-file>; future kinds should migrate to <projectDir>/tasks/. - Add a test for complete()-after-finalizeCancelled no-op to pin the one-notification-per-task SDK contract through the post-notified re-entry path.
1 parent cb51311 commit 57c89e4

10 files changed

Lines changed: 110 additions & 81 deletions

‎packages/cli/src/nonInteractiveCli.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ describe('runNonInteractive', () => {
167167
setNotificationCallback: vi.fn(),
168168
setRegisterCallback: vi.fn(),
169169
getRunning: vi.fn().mockReturnValue([]),
170-
hasUnfinalizedAgents: vi.fn().mockReturnValue(false),
170+
hasUnfinalizedTasks: vi.fn().mockReturnValue(false),
171171
abortAll: vi.fn(),
172172
}),
173173
} as unknown as Config;

‎packages/cli/src/nonInteractiveCli.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -714,11 +714,11 @@ export async function runNonInteractive(
714714
await handleCancellationError(config);
715715
}
716716
await drainLocalQueue();
717-
// Wait for every agent's terminal notification, not just the
717+
// Wait for every task's terminal notification, not just the
718718
// running ones: cancel() marks status 'cancelled' synchronously
719719
// but the notification is emitted later by the natural handler,
720720
// and SDK consumers need every task_started paired with one.
721-
if (!registry.hasUnfinalizedAgents() && localQueue.length === 0)
721+
if (!registry.hasUnfinalizedTasks() && localQueue.length === 0)
722722
break;
723723
await new Promise((r) => setTimeout(r, 100));
724724
}

‎packages/core/src/agents/agent-transcript.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ function sanitizeFilenameComponent(value: string): string {
4141
/**
4242
* Returns the directory holding all subagent transcripts for a given session.
4343
* Layout: `<projectDir>/subagents/<sessionId>/`.
44+
*
45+
* TODO: this path is part of the model-facing contract via `<output-file>` in
46+
* the task-notification XML. When a second background task kind lands (e.g. a
47+
* shell pool), migrate to `<projectDir>/tasks/<sessionId>/<kind>-<id>.jsonl`
48+
* so the namespace generalizes. Update `read-file.ts` auto-allow accordingly.
4449
*/
4550
export function getSubagentSessionDir(
4651
projectDir: string,

‎packages/core/src/agents/background-tasks.test.ts‎

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -120,14 +120,14 @@ describe('BackgroundTaskRegistry', () => {
120120
expect(callback).not.toHaveBeenCalled();
121121

122122
// Pathological tool case: bgBody never emits. After the grace period
123-
// the fallback fires so hasUnfinalizedAgents() stops reporting true
123+
// the fallback fires so hasUnfinalizedTasks() stops reporting true
124124
// and the headless wait loop can exit.
125125
vi.runAllTimers();
126126

127127
expect(callback).toHaveBeenCalledOnce();
128128
const [, modelText] = callback.mock.calls[0] as [string, string];
129129
expect(modelText).toContain('<status>cancelled</status>');
130-
expect(registry.hasUnfinalizedAgents()).toBe(false);
130+
expect(registry.hasUnfinalizedTasks()).toBe(false);
131131
} finally {
132132
vi.useRealTimers();
133133
}
@@ -182,6 +182,34 @@ describe('BackgroundTaskRegistry', () => {
182182
expect(modelText).toContain('<status>cancelled</status>');
183183
});
184184

185+
it('complete() after the cancellation has already been notified is a no-op', () => {
186+
// Once finalizeCancelled has emitted the terminal notification, a
187+
// late-arriving complete() must not double-fire — the SDK contract
188+
// is one notification per task_started.
189+
const callback = vi.fn();
190+
registry.setNotificationCallback(callback);
191+
192+
registry.register({
193+
agentId: 'test-1',
194+
description: 'test agent',
195+
status: 'running',
196+
startTime: Date.now(),
197+
abortController: new AbortController(),
198+
});
199+
200+
registry.cancel('test-1');
201+
registry.finalizeCancelled('test-1', 'partial');
202+
expect(callback).toHaveBeenCalledOnce();
203+
callback.mockClear();
204+
205+
registry.complete('test-1', 'late result');
206+
207+
expect(callback).not.toHaveBeenCalled();
208+
// Status stays cancelled — the notified terminal state wins.
209+
expect(registry.get('test-1')!.status).toBe('cancelled');
210+
expect(registry.get('test-1')!.result).toBe('partial');
211+
});
212+
185213
it('does not cancel a non-running agent', () => {
186214
const abortController = new AbortController();
187215

@@ -257,7 +285,7 @@ describe('BackgroundTaskRegistry', () => {
257285
expect(callback).toHaveBeenCalledTimes(2);
258286
});
259287

260-
it('hasUnfinalizedAgents reports cancelled-but-not-notified entries', () => {
288+
it('hasUnfinalizedTasks reports cancelled-but-not-notified entries', () => {
261289
// Headless runs rely on this to keep the event loop alive after a
262290
// task_stop until the agent's natural handler has emitted the
263291
// terminal task-notification — otherwise the matching notification
@@ -269,17 +297,17 @@ describe('BackgroundTaskRegistry', () => {
269297
startTime: Date.now(),
270298
abortController: new AbortController(),
271299
});
272-
expect(registry.hasUnfinalizedAgents()).toBe(true);
300+
expect(registry.hasUnfinalizedTasks()).toBe(true);
273301

274302
registry.cancel('test-1');
275303
expect(registry.get('test-1')!.status).toBe('cancelled');
276-
expect(registry.hasUnfinalizedAgents()).toBe(true);
304+
expect(registry.hasUnfinalizedTasks()).toBe(true);
277305

278306
registry.finalizeCancelled('test-1', '');
279-
expect(registry.hasUnfinalizedAgents()).toBe(false);
307+
expect(registry.hasUnfinalizedTasks()).toBe(false);
280308
});
281309

282-
it('hasUnfinalizedAgents clears once every entry has been notified', () => {
310+
it('hasUnfinalizedTasks clears once every entry has been notified', () => {
283311
registry.register({
284312
agentId: 'a',
285313
description: 'agent a',
@@ -295,11 +323,11 @@ describe('BackgroundTaskRegistry', () => {
295323
abortController: new AbortController(),
296324
});
297325

298-
expect(registry.hasUnfinalizedAgents()).toBe(true);
326+
expect(registry.hasUnfinalizedTasks()).toBe(true);
299327
registry.complete('a', 'done');
300-
expect(registry.hasUnfinalizedAgents()).toBe(true);
328+
expect(registry.hasUnfinalizedTasks()).toBe(true);
301329
registry.fail('b', 'boom');
302-
expect(registry.hasUnfinalizedAgents()).toBe(false);
330+
expect(registry.hasUnfinalizedTasks()).toBe(false);
303331
});
304332

305333
it('complete after cancellation surfaces the real result', () => {

‎packages/core/src/agents/background-tasks.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,14 +221,14 @@ export class BackgroundTaskRegistry {
221221
}
222222

223223
/**
224-
* True if any registered agent has not yet emitted its terminal
224+
* True if any registered task has not yet emitted its terminal
225225
* task-notification. Covers `running` (still executing) and
226226
* `cancelled`-but-not-finalized (cancel requested, but the natural
227227
* handler hasn't fired finalizeCancelled() yet). Headless callers
228228
* must keep their event loop alive while this returns true, so every
229229
* task_started is paired with a matching task_notification.
230230
*/
231-
hasUnfinalizedAgents(): boolean {
231+
hasUnfinalizedTasks(): boolean {
232232
for (const entry of this.agents.values()) {
233233
if (!entry.notified) return true;
234234
}

‎packages/core/src/tools/send-message.test.ts‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ describe('SendMessageTool', () => {
2323
tool = new SendMessageTool(config);
2424
});
2525

26-
it('queues a message for a running agent', async () => {
26+
it('queues a message for a running task', async () => {
2727
registry.register({
2828
agentId: 'agent-1',
2929
description: 'test agent',
@@ -33,7 +33,7 @@ describe('SendMessageTool', () => {
3333
});
3434

3535
const result = await tool.validateBuildAndExecute(
36-
{ to: 'agent-1', message: 'do more work' },
36+
{ task_id: 'agent-1', message: 'do more work' },
3737
new AbortController().signal,
3838
);
3939

@@ -52,11 +52,11 @@ describe('SendMessageTool', () => {
5252
});
5353

5454
await tool.validateBuildAndExecute(
55-
{ to: 'agent-1', message: 'first' },
55+
{ task_id: 'agent-1', message: 'first' },
5656
new AbortController().signal,
5757
);
5858
await tool.validateBuildAndExecute(
59-
{ to: 'agent-1', message: 'second' },
59+
{ task_id: 'agent-1', message: 'second' },
6060
new AbortController().signal,
6161
);
6262

@@ -66,17 +66,17 @@ describe('SendMessageTool', () => {
6666
]);
6767
});
6868

69-
it('returns error for non-existent agent', async () => {
69+
it('returns error for non-existent task', async () => {
7070
const result = await tool.validateBuildAndExecute(
71-
{ to: 'nope', message: 'hello' },
71+
{ task_id: 'nope', message: 'hello' },
7272
new AbortController().signal,
7373
);
7474

75-
expect(result.error?.type).toBe(ToolErrorType.SEND_MESSAGE_AGENT_NOT_FOUND);
76-
expect(result.llmContent).toContain('No background agent found');
75+
expect(result.error?.type).toBe(ToolErrorType.SEND_MESSAGE_NOT_FOUND);
76+
expect(result.llmContent).toContain('No background task found');
7777
});
7878

79-
it('returns error for non-running agent', async () => {
79+
it('returns error for non-running task', async () => {
8080
registry.register({
8181
agentId: 'agent-1',
8282
description: 'test agent',
@@ -87,17 +87,15 @@ describe('SendMessageTool', () => {
8787
registry.complete('agent-1', 'done');
8888

8989
const result = await tool.validateBuildAndExecute(
90-
{ to: 'agent-1', message: 'hello' },
90+
{ task_id: 'agent-1', message: 'hello' },
9191
new AbortController().signal,
9292
);
9393

94-
expect(result.error?.type).toBe(
95-
ToolErrorType.SEND_MESSAGE_AGENT_NOT_RUNNING,
96-
);
94+
expect(result.error?.type).toBe(ToolErrorType.SEND_MESSAGE_NOT_RUNNING);
9795
expect(result.llmContent).toContain('not running');
9896
});
9997

100-
it('rejects messages for a cancelled agent', async () => {
98+
it('rejects messages for a cancelled task', async () => {
10199
// Once task_stop fires, the reasoning loop is winding down — there is
102100
// no next tool-round boundary to drain into, so the message would be
103101
// silently dropped. Reject instead of accepting a message that will
@@ -112,17 +110,15 @@ describe('SendMessageTool', () => {
112110
registry.cancel('agent-1');
113111

114112
const result = await tool.validateBuildAndExecute(
115-
{ to: 'agent-1', message: 'too late' },
113+
{ task_id: 'agent-1', message: 'too late' },
116114
new AbortController().signal,
117115
);
118116

119-
expect(result.error?.type).toBe(
120-
ToolErrorType.SEND_MESSAGE_AGENT_NOT_RUNNING,
121-
);
117+
expect(result.error?.type).toBe(ToolErrorType.SEND_MESSAGE_NOT_RUNNING);
122118
expect(registry.get('agent-1')!.pendingMessages).toEqual([]);
123119
});
124120

125-
it('includes agent description in success display', async () => {
121+
it('includes task description in success display', async () => {
126122
registry.register({
127123
agentId: 'agent-1',
128124
description: 'Search for auth code',
@@ -132,7 +128,7 @@ describe('SendMessageTool', () => {
132128
});
133129

134130
const result = await tool.validateBuildAndExecute(
135-
{ to: 'agent-1', message: 'focus on login' },
131+
{ task_id: 'agent-1', message: 'focus on login' },
136132
new AbortController().signal,
137133
);
138134

‎packages/core/src/tools/send-message.ts‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
/**
88
* @fileoverview SendMessage tool — lets the model send a text message to
9-
* a running background agent. The message is injected into the agent's
9+
* a running background task. The message is injected into the task's
1010
* reasoning loop at the next tool-round boundary.
1111
*/
1212

@@ -22,9 +22,9 @@ import {
2222
} from './tools.js';
2323

2424
export interface SendMessageParams {
25-
/** The ID of the background agent to send the message to. */
26-
to: string;
27-
/** The text message to deliver to the agent. */
25+
/** The ID of the background task to send the message to. */
26+
task_id: string;
27+
/** The text message to deliver to the task. */
2828
message: string;
2929
}
3030

@@ -40,39 +40,39 @@ class SendMessageInvocation extends BaseToolInvocation<
4040
}
4141

4242
getDescription(): string {
43-
return `Send message to agent ${this.params.to}`;
43+
return `Send message to task ${this.params.task_id}`;
4444
}
4545

4646
async execute(_signal: AbortSignal): Promise<ToolResult> {
4747
const registry = this.config.getBackgroundTaskRegistry();
48-
const entry = registry.get(this.params.to);
48+
const entry = registry.get(this.params.task_id);
4949

5050
if (!entry) {
5151
return {
52-
llmContent: `Error: No background agent found with ID "${this.params.to}".`,
53-
returnDisplay: 'Agent not found.',
52+
llmContent: `Error: No background task found with ID "${this.params.task_id}".`,
53+
returnDisplay: 'Task not found.',
5454
error: {
55-
message: `Agent not found: ${this.params.to}`,
56-
type: ToolErrorType.SEND_MESSAGE_AGENT_NOT_FOUND,
55+
message: `Task not found: ${this.params.task_id}`,
56+
type: ToolErrorType.SEND_MESSAGE_NOT_FOUND,
5757
},
5858
};
5959
}
6060

6161
if (entry.status !== 'running') {
6262
return {
63-
llmContent: `Error: Background agent "${this.params.to}" is not running (status: ${entry.status}). Cannot send messages to stopped agents.`,
64-
returnDisplay: `Agent not running (${entry.status}).`,
63+
llmContent: `Error: Background task "${this.params.task_id}" is not running (status: ${entry.status}). Cannot send messages to stopped tasks.`,
64+
returnDisplay: `Task not running (${entry.status}).`,
6565
error: {
66-
message: `Agent is ${entry.status}: ${this.params.to}`,
67-
type: ToolErrorType.SEND_MESSAGE_AGENT_NOT_RUNNING,
66+
message: `Task is ${entry.status}: ${this.params.task_id}`,
67+
type: ToolErrorType.SEND_MESSAGE_NOT_RUNNING,
6868
},
6969
};
7070
}
7171

72-
registry.queueMessage(this.params.to, this.params.message);
72+
registry.queueMessage(this.params.task_id, this.params.message);
7373

7474
return {
75-
llmContent: `Message queued for delivery to background agent "${this.params.to}". The agent will receive it at the next tool-round boundary.`,
75+
llmContent: `Message queued for delivery to background task "${this.params.task_id}". The task will receive it at the next tool-round boundary.`,
7676
returnDisplay: `Message queued for ${entry.description}`,
7777
};
7878
}
@@ -88,22 +88,22 @@ export class SendMessageTool extends BaseDeclarativeTool<
8888
super(
8989
SendMessageTool.Name,
9090
ToolDisplayNames.SEND_MESSAGE,
91-
'Send a text message to a running background agent. The message is delivered at the next tool-round boundary. Use this to provide additional instructions or context to a background agent.',
91+
'Send a text message to a running background task. The message is delivered at the next tool-round boundary. Use this to provide additional instructions or context to a background task.',
9292
Kind.Other,
9393
{
9494
type: 'object',
9595
properties: {
96-
to: {
96+
task_id: {
9797
type: 'string',
9898
description:
99-
'The ID of the running background agent (from the launch response).',
99+
'The ID of the running background task (from the launch response).',
100100
},
101101
message: {
102102
type: 'string',
103-
description: 'The text message to send to the agent.',
103+
description: 'The text message to send to the task.',
104104
},
105105
},
106-
required: ['to', 'message'],
106+
required: ['task_id', 'message'],
107107
additionalProperties: false,
108108
},
109109
);

‎packages/core/src/tools/task-stop.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,17 @@ describe('TaskStopTool', () => {
4545
expect(ac.signal.aborted).toBe(true);
4646
});
4747

48-
it('returns error for non-existent agent', async () => {
48+
it('returns error for non-existent task', async () => {
4949
const result = await tool.validateBuildAndExecute(
5050
{ task_id: 'nope' },
5151
new AbortController().signal,
5252
);
5353

54-
expect(result.error?.type).toBe(ToolErrorType.TASK_STOP_AGENT_NOT_FOUND);
55-
expect(result.llmContent).toContain('No background agent found');
54+
expect(result.error?.type).toBe(ToolErrorType.TASK_STOP_NOT_FOUND);
55+
expect(result.llmContent).toContain('No background task found');
5656
});
5757

58-
it('returns error for non-running agent', async () => {
58+
it('returns error for non-running task', async () => {
5959
registry.register({
6060
agentId: 'agent-1',
6161
description: 'test agent',
@@ -70,7 +70,7 @@ describe('TaskStopTool', () => {
7070
new AbortController().signal,
7171
);
7272

73-
expect(result.error?.type).toBe(ToolErrorType.TASK_STOP_AGENT_NOT_RUNNING);
73+
expect(result.error?.type).toBe(ToolErrorType.TASK_STOP_NOT_RUNNING);
7474
expect(result.llmContent).toContain('not running');
7575
});
7676

0 commit comments

Comments
 (0)