Skip to content

Commit 1855463

Browse files
committed
Preserve task cancel, auth, and abort contracts through the fleet wrapper
Director task() now routes through spawn_agent + wait_agents, which was misclassifying AbortError as failed:aborted, dropping Re-authenticate auth wording, and leaving the child running when the parent tool aborted. Map those outcomes back to the legacy fused-task parent contract and prettier the inherited agent-progress tip.
1 parent 470ef2f commit 1855463

2 files changed

Lines changed: 61 additions & 8 deletions

File tree

src/subagent/agent-fleet.ts

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ import type { Settings } from "../config/settings.js";
6363
import { resolveEffortForRole } from "../provider/reasoning-effort.js";
6464
import { isCodexProviderName } from "../config/codex-providers.js";
6565
import { buildDispatchBrief, type TaskIntent } from "./report.js";
66-
import type { SubAgentSessionStore } from "./session-store.js";
66+
import { DEFAULT_CANCEL_REASON, type SubAgentSessionStore } from "./session-store.js";
6767
import type {
6868
NestedDispatchDeps,
6969
RunSubAgentParams,
@@ -75,6 +75,8 @@ import { cleanupSubAgentWorktree, createSubAgentWorktree, WorktreeError } from "
7575
import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js";
7676
import { classifyAgentName } from "../telemetry/classify.js";
7777
import type { DirectorPackage } from "../agent/directors/types.js";
78+
import { formatSubAgentTaskAuthFailureMessage } from "./inference-auth-failure.js";
79+
import { isSubAgentCancelError } from "./dispose.js";
7880

7981
const log = getLogger([LOG_NAMESPACE_ROOT, "subagent", "agent-fleet"]);
8082

@@ -718,11 +720,24 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool {
718720
})
719721
.catch((err) => {
720722
// Always terminalize fleetRecords — including pre-progress cancel that
721-
// rethrows with no salvage — so wait_agents does not hang. fail()
722-
// no-ops when cancel already flipped the strip status.
723-
const message = err instanceof Error ? err.message : String(err);
724-
deps.fleetRecords.reject(session.id, message);
725-
deps.sessions.fail(session.id, message);
723+
// rethrows with no salvage — so wait_agents does not hang. Prefer
724+
// cancel semantics over fail when the strip already cancelled or the
725+
// throw is an AbortError (legacy task() parent contract).
726+
const alreadyCancelled = deps.sessions.get(session.id)?.status === "cancelled";
727+
if (alreadyCancelled || isSubAgentCancelError(err, childCtl.signal)) {
728+
if (!alreadyCancelled) {
729+
deps.sessions.cancel(session.id, DEFAULT_CANCEL_REASON);
730+
}
731+
const message = err instanceof Error ? err.message : String(err);
732+
deps.fleetRecords.reject(session.id, message);
733+
return;
734+
}
735+
// Auth failures keep the actionable Re-authenticate wording that
736+
// task()'s fused path surfaces via formatSubAgentTaskAuthFailureMessage.
737+
const authMessage = formatSubAgentTaskAuthFailureMessage(description, err);
738+
const failReason = authMessage ?? (err instanceof Error ? err.message : String(err));
739+
deps.fleetRecords.reject(session.id, failReason);
740+
deps.sessions.fail(session.id, failReason);
726741
})
727742
.finally(() => {
728743
telemetry.capture("subagent_end", {

src/subagent/task-tool.ts

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,16 +397,54 @@ async function runTaskViaFleet(input: {
397397
if (result === undefined) {
398398
return taskToolResult(input.callId, `Error: wait_agents returned no result for ${agentId}.`);
399399
}
400+
// Cancel must not be misclassified as failed:abort — strip cancel /
401+
// AbortError leaves fleetRecords as failed with "aborted" while the
402+
// session store holds cancelled + the operator reason. A cancel that
403+
// still resolved a salvage body (fleet status done) keeps the report,
404+
// matching the fused task() race contract.
405+
const session = input.sessions.get(agentId);
406+
if (session?.status === "cancelled") {
407+
if (
408+
(result.status === "done" || result.status === "interrupted") &&
409+
typeof result.report === "string" &&
410+
result.report.length > 0
411+
) {
412+
return taskToolResult(
413+
input.callId,
414+
`Sub-agent "${input.description}" reported:\n\n${result.report}`,
415+
);
416+
}
417+
return taskToolResult(
418+
input.callId,
419+
cancelledSubAgentMessage(input.description, session.error),
420+
);
421+
}
400422
if (result.status === "failed") {
423+
const errText = result.error ?? "unknown error";
424+
// Auth failures already carry the actionable Re-authenticate wording
425+
// from formatSubAgentTaskAuthFailureMessage (baked in spawn catch).
426+
if (errText.includes("Re-authenticate")) {
427+
return taskToolResult(input.callId, `Error: ${errText}`);
428+
}
401429
return taskToolResult(
402430
input.callId,
403-
`Error: sub-agent "${input.description}" failed: ${result.error ?? "unknown error"}`,
431+
`Error: sub-agent "${input.description}" failed: ${errText}`,
404432
);
405433
}
406434
const report = result.report ?? "";
407435
return taskToolResult(input.callId, `Sub-agent "${input.description}" reported:\n\n${report}`);
408436
}
409-
return taskToolResult(input.callId, `Sub-agent "${input.description}" cancelled by operator.`);
437+
// Parent tool abort must cancel the child — wait_agents itself has no
438+
// abort side effects (workers stay waitable), so task()'s fused contract
439+
// owns the cancel here.
440+
if (input.sessions.get(agentId)?.status === "running") {
441+
input.sessions.cancel(agentId);
442+
}
443+
const cancelled = input.sessions.get(agentId);
444+
return taskToolResult(
445+
input.callId,
446+
cancelledSubAgentMessage(input.description, cancelled?.error),
447+
);
410448
}
411449

412450
export function createTaskTool(deps: TaskToolDeps): AgentTool {

0 commit comments

Comments
 (0)