fix: task subagent static race main (#843)
This commit is contained in:
@@ -212,6 +212,10 @@ import {
|
||||
interruptActiveSubagents,
|
||||
subscribe as subscribeToSubagents,
|
||||
} from "./helpers/subagentState";
|
||||
import {
|
||||
flushEligibleLinesBeforeReentry,
|
||||
shouldClearCompletedSubagentsOnTurnStart,
|
||||
} from "./helpers/subagentTurnStart";
|
||||
import { extractTaskNotificationsForDisplay } from "./helpers/taskNotifications";
|
||||
import {
|
||||
getRandomPastTenseVerb,
|
||||
@@ -2960,9 +2964,13 @@ export default function App({
|
||||
// Reset interrupted flag since we're starting a fresh stream
|
||||
buffersRef.current.interrupted = false;
|
||||
|
||||
// Clear completed subagents from the UI when starting a new turn,
|
||||
// but only if no subagents are still running.
|
||||
if (!hasActiveSubagents()) {
|
||||
// Clear completed subagents only on true new turns.
|
||||
if (
|
||||
shouldClearCompletedSubagentsOnTurnStart(
|
||||
allowReentry,
|
||||
hasActiveSubagents(),
|
||||
)
|
||||
) {
|
||||
clearCompletedSubagents();
|
||||
}
|
||||
|
||||
@@ -8773,6 +8781,13 @@ ${SYSTEM_REMINDER_CLOSE}
|
||||
} else if (hadNotifications) {
|
||||
refreshDerived();
|
||||
}
|
||||
// Flush finished items synchronously before reentry. This avoids a
|
||||
// race where deferred non-Task commits delay Task grouping while the
|
||||
// reentry path continues.
|
||||
flushEligibleLinesBeforeReentry(
|
||||
commitEligibleLines,
|
||||
buffersRef.current,
|
||||
);
|
||||
toolResultsInFlightRef.current = true;
|
||||
await processConversation(input, { allowReentry: true });
|
||||
toolResultsInFlightRef.current = false;
|
||||
@@ -8808,6 +8823,7 @@ ${SYSTEM_REMINDER_CLOSE}
|
||||
syncTrajectoryElapsedBase,
|
||||
closeTrajectorySegment,
|
||||
openTrajectorySegment,
|
||||
commitEligibleLines,
|
||||
],
|
||||
);
|
||||
|
||||
|
||||
27
src/cli/helpers/subagentTurnStart.ts
Normal file
27
src/cli/helpers/subagentTurnStart.ts
Normal file
@@ -0,0 +1,27 @@
|
||||
import type { Buffers } from "./accumulator.js";
|
||||
|
||||
/**
|
||||
* Completed subagents should only be cleared on true new turns.
|
||||
* During allowReentry (post-approval continuation), completed subagents
|
||||
* must remain available so deferred Task grouping can still resolve.
|
||||
*/
|
||||
export function shouldClearCompletedSubagentsOnTurnStart(
|
||||
allowReentry: boolean,
|
||||
hasActiveSubagents: boolean,
|
||||
): boolean {
|
||||
return !allowReentry && !hasActiveSubagents;
|
||||
}
|
||||
|
||||
/**
|
||||
* Flush static-eligible lines before reentry so Task grouping is not delayed
|
||||
* by deferred non-Task tool commits.
|
||||
*/
|
||||
export function flushEligibleLinesBeforeReentry(
|
||||
commitEligibleLines: (
|
||||
b: Buffers,
|
||||
opts?: { deferToolCalls?: boolean },
|
||||
) => void,
|
||||
buffers: Buffers,
|
||||
): void {
|
||||
commitEligibleLines(buffers, { deferToolCalls: false });
|
||||
}
|
||||
241
src/tests/cli/subagentTurnStart.test.ts
Normal file
241
src/tests/cli/subagentTurnStart.test.ts
Normal file
@@ -0,0 +1,241 @@
|
||||
import { beforeEach, describe, expect, test } from "bun:test";
|
||||
import {
|
||||
collectFinishedTaskToolCalls,
|
||||
hasInProgressTaskToolCalls,
|
||||
} from "../../cli/helpers/subagentAggregation";
|
||||
import {
|
||||
clearAllSubagents,
|
||||
clearCompletedSubagents,
|
||||
completeSubagent,
|
||||
getSubagentByToolCallId,
|
||||
registerSubagent,
|
||||
} from "../../cli/helpers/subagentState";
|
||||
import {
|
||||
flushEligibleLinesBeforeReentry,
|
||||
shouldClearCompletedSubagentsOnTurnStart,
|
||||
} from "../../cli/helpers/subagentTurnStart";
|
||||
|
||||
type MinimalToolCallLine = {
|
||||
kind: "tool_call";
|
||||
id: string;
|
||||
name: string;
|
||||
phase: "finished";
|
||||
toolCallId: string;
|
||||
resultOk: boolean;
|
||||
};
|
||||
|
||||
function simulateCommitPass(
|
||||
order: string[],
|
||||
byId: Map<string, MinimalToolCallLine>,
|
||||
emitted: Set<string>,
|
||||
deferredCommits: Map<string, number>,
|
||||
now: number,
|
||||
deferToolCalls: boolean,
|
||||
): {
|
||||
blockedByDeferred: boolean;
|
||||
grouped: boolean;
|
||||
taskFallbackCommitted: boolean;
|
||||
} {
|
||||
const hasInProgress = hasInProgressTaskToolCalls(
|
||||
order,
|
||||
byId as unknown as Map<string, never>,
|
||||
emitted,
|
||||
);
|
||||
const finishedTaskToolCalls = collectFinishedTaskToolCalls(
|
||||
order,
|
||||
byId as unknown as Map<string, never>,
|
||||
emitted,
|
||||
hasInProgress,
|
||||
);
|
||||
|
||||
let blockedByDeferred = false;
|
||||
let taskFallbackCommitted = false;
|
||||
const TASK_DEFER_MS = 50;
|
||||
|
||||
for (const id of order) {
|
||||
if (emitted.has(id)) continue;
|
||||
const ln = byId.get(id);
|
||||
if (!ln) continue;
|
||||
|
||||
if (ln.name === "Task" || ln.name === "task") {
|
||||
const hasSubagentData = finishedTaskToolCalls.some(
|
||||
(tc) => tc.lineId === id,
|
||||
);
|
||||
if (!hasSubagentData) {
|
||||
emitted.add(id);
|
||||
taskFallbackCommitted = true;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
if (deferToolCalls) {
|
||||
const commitAt = deferredCommits.get(id);
|
||||
if (commitAt === undefined) {
|
||||
deferredCommits.set(id, now + TASK_DEFER_MS);
|
||||
blockedByDeferred = true;
|
||||
break;
|
||||
}
|
||||
if (commitAt > now) {
|
||||
blockedByDeferred = true;
|
||||
break;
|
||||
}
|
||||
deferredCommits.delete(id);
|
||||
}
|
||||
|
||||
emitted.add(id);
|
||||
}
|
||||
|
||||
const grouped = !blockedByDeferred && finishedTaskToolCalls.length > 0;
|
||||
if (grouped) {
|
||||
for (const tc of finishedTaskToolCalls) {
|
||||
emitted.add(tc.lineId);
|
||||
}
|
||||
}
|
||||
|
||||
return { blockedByDeferred, grouped, taskFallbackCommitted };
|
||||
}
|
||||
|
||||
describe("subagent turn-start reentry safeguards", () => {
|
||||
beforeEach(() => {
|
||||
clearAllSubagents();
|
||||
});
|
||||
|
||||
test("shouldClearCompletedSubagentsOnTurnStart preserves completed agents during allowReentry", () => {
|
||||
expect(shouldClearCompletedSubagentsOnTurnStart(true, false)).toBe(false);
|
||||
expect(shouldClearCompletedSubagentsOnTurnStart(false, false)).toBe(true);
|
||||
expect(shouldClearCompletedSubagentsOnTurnStart(false, true)).toBe(false);
|
||||
});
|
||||
|
||||
test("deferred first pass + explicit pre-reentry flush preserves Task grouping", () => {
|
||||
registerSubagent("sub-1", "explore", "Find symbols", "tc-task", false);
|
||||
completeSubagent("sub-1", { success: true, totalTokens: 42 });
|
||||
|
||||
const order = ["line-read", "line-task"];
|
||||
const byId = new Map<string, MinimalToolCallLine>([
|
||||
[
|
||||
"line-read",
|
||||
{
|
||||
kind: "tool_call",
|
||||
id: "line-read",
|
||||
name: "Read",
|
||||
phase: "finished",
|
||||
toolCallId: "tc-read",
|
||||
resultOk: true,
|
||||
},
|
||||
],
|
||||
[
|
||||
"line-task",
|
||||
{
|
||||
kind: "tool_call",
|
||||
id: "line-task",
|
||||
name: "Task",
|
||||
phase: "finished",
|
||||
toolCallId: "tc-task",
|
||||
resultOk: true,
|
||||
},
|
||||
],
|
||||
]);
|
||||
const emitted = new Set<string>();
|
||||
const deferred = new Map<string, number>();
|
||||
|
||||
// First commit pass gets blocked by deferred non-Task tool call.
|
||||
const first = simulateCommitPass(
|
||||
order,
|
||||
byId,
|
||||
emitted,
|
||||
deferred,
|
||||
1_000,
|
||||
true,
|
||||
);
|
||||
expect(first.blockedByDeferred).toBe(true);
|
||||
expect(first.grouped).toBe(false);
|
||||
expect(getSubagentByToolCallId("tc-task")).toBeDefined();
|
||||
|
||||
// During reentry we should preserve completed subagents.
|
||||
if (shouldClearCompletedSubagentsOnTurnStart(true, false)) {
|
||||
clearCompletedSubagents();
|
||||
}
|
||||
expect(getSubagentByToolCallId("tc-task")).toBeDefined();
|
||||
|
||||
// Explicit non-deferred flush before reentry should request deferToolCalls=false.
|
||||
let flushCalled = false;
|
||||
let capturedOpts: { deferToolCalls?: boolean } | undefined;
|
||||
flushEligibleLinesBeforeReentry((_b, opts) => {
|
||||
flushCalled = true;
|
||||
capturedOpts = opts;
|
||||
}, {} as never);
|
||||
expect(flushCalled).toBe(true);
|
||||
expect(capturedOpts?.deferToolCalls).toBe(false);
|
||||
|
||||
// And with defer disabled, Task grouping should happen instead of fallback.
|
||||
const flushed = simulateCommitPass(
|
||||
order,
|
||||
byId,
|
||||
emitted,
|
||||
deferred,
|
||||
1_001,
|
||||
false,
|
||||
);
|
||||
expect(flushed.blockedByDeferred).toBe(false);
|
||||
expect(flushed.grouped).toBe(true);
|
||||
expect(flushed.taskFallbackCommitted).toBe(false);
|
||||
});
|
||||
|
||||
test("clearing completed agents before second pass reproduces Task fallback", () => {
|
||||
registerSubagent("sub-1", "explore", "Find symbols", "tc-task", false);
|
||||
completeSubagent("sub-1", { success: true });
|
||||
|
||||
const order = ["line-read", "line-task"];
|
||||
const byId = new Map<string, MinimalToolCallLine>([
|
||||
[
|
||||
"line-read",
|
||||
{
|
||||
kind: "tool_call",
|
||||
id: "line-read",
|
||||
name: "Read",
|
||||
phase: "finished",
|
||||
toolCallId: "tc-read",
|
||||
resultOk: true,
|
||||
},
|
||||
],
|
||||
[
|
||||
"line-task",
|
||||
{
|
||||
kind: "tool_call",
|
||||
id: "line-task",
|
||||
name: "Task",
|
||||
phase: "finished",
|
||||
toolCallId: "tc-task",
|
||||
resultOk: true,
|
||||
},
|
||||
],
|
||||
]);
|
||||
const emitted = new Set<string>();
|
||||
const deferred = new Map<string, number>();
|
||||
|
||||
const first = simulateCommitPass(
|
||||
order,
|
||||
byId,
|
||||
emitted,
|
||||
deferred,
|
||||
2_000,
|
||||
true,
|
||||
);
|
||||
expect(first.blockedByDeferred).toBe(true);
|
||||
|
||||
// This mirrors the old problematic behavior.
|
||||
clearCompletedSubagents();
|
||||
expect(getSubagentByToolCallId("tc-task")).toBeUndefined();
|
||||
|
||||
const second = simulateCommitPass(
|
||||
order,
|
||||
byId,
|
||||
emitted,
|
||||
deferred,
|
||||
2_100,
|
||||
false,
|
||||
);
|
||||
expect(second.grouped).toBe(false);
|
||||
expect(second.taskFallbackCommitted).toBe(true);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user