fix(tui): show background subagent notifications immediately when idle (#1242)
This commit is contained in:
@@ -276,7 +276,10 @@ import {
|
|||||||
flushEligibleLinesBeforeReentry,
|
flushEligibleLinesBeforeReentry,
|
||||||
shouldClearCompletedSubagentsOnTurnStart,
|
shouldClearCompletedSubagentsOnTurnStart,
|
||||||
} from "./helpers/subagentTurnStart";
|
} from "./helpers/subagentTurnStart";
|
||||||
import { extractTaskNotificationsForDisplay } from "./helpers/taskNotifications";
|
import {
|
||||||
|
appendTaskNotificationEventsToBuffer,
|
||||||
|
extractTaskNotificationsForDisplay,
|
||||||
|
} from "./helpers/taskNotifications";
|
||||||
import {
|
import {
|
||||||
getRandomPastTenseVerb,
|
getRandomPastTenseVerb,
|
||||||
getRandomThinkingVerb,
|
getRandomThinkingVerb,
|
||||||
@@ -1894,23 +1897,19 @@ export default function App({
|
|||||||
);
|
);
|
||||||
}, [isExecutingTool]);
|
}, [isExecutingTool]);
|
||||||
|
|
||||||
|
// Ref indirection: refreshDerived is declared later in the component but
|
||||||
|
// appendTaskNotificationEvents needs to call it. Using a ref avoids a
|
||||||
|
// forward-declaration error while keeping the deps array empty.
|
||||||
|
const refreshDerivedRef = useRef<(() => void) | null>(null);
|
||||||
|
|
||||||
const appendTaskNotificationEvents = useCallback(
|
const appendTaskNotificationEvents = useCallback(
|
||||||
(summaries: string[]): boolean => {
|
(summaries: string[]): boolean =>
|
||||||
if (summaries.length === 0) return false;
|
appendTaskNotificationEventsToBuffer(
|
||||||
for (const summary of summaries) {
|
summaries,
|
||||||
const eventId = uid("event");
|
buffersRef.current,
|
||||||
buffersRef.current.byId.set(eventId, {
|
() => uid("event"),
|
||||||
kind: "event",
|
() => refreshDerivedRef.current?.(),
|
||||||
id: eventId,
|
),
|
||||||
eventType: "task_notification",
|
|
||||||
eventData: {},
|
|
||||||
phase: "finished",
|
|
||||||
summary,
|
|
||||||
});
|
|
||||||
buffersRef.current.order.push(eventId);
|
|
||||||
}
|
|
||||||
return true;
|
|
||||||
},
|
|
||||||
[],
|
[],
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -2702,6 +2701,7 @@ export default function App({
|
|||||||
setLines(newLines);
|
setLines(newLines);
|
||||||
commitEligibleLines(b);
|
commitEligibleLines(b);
|
||||||
}, [commitEligibleLines]);
|
}, [commitEligibleLines]);
|
||||||
|
refreshDerivedRef.current = refreshDerived;
|
||||||
|
|
||||||
const recordCommandReminder = useCallback((event: CommandFinishedEvent) => {
|
const recordCommandReminder = useCallback((event: CommandFinishedEvent) => {
|
||||||
const input = event.input.trim();
|
const input = event.input.trim();
|
||||||
|
|||||||
@@ -106,6 +106,47 @@ export function extractTaskNotificationsForDisplay(message: string): {
|
|||||||
return { notifications, cleanedText };
|
return { notifications, cleanedText };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ============================================================================
|
||||||
|
// Buffer Helpers
|
||||||
|
// ============================================================================
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Append task-notification events to a transcript buffer and flush.
|
||||||
|
*
|
||||||
|
* This is the pure-function core of App.tsx's `appendTaskNotificationEvents`
|
||||||
|
* useCallback. Extracting it here makes the behavioral contract testable:
|
||||||
|
* buffer writes MUST be followed by a flush so that notifications from
|
||||||
|
* background subagent onComplete callbacks (which run outside React's render
|
||||||
|
* cycle) appear immediately instead of waiting for the next unrelated render.
|
||||||
|
*/
|
||||||
|
export type NotificationBuffer = Pick<
|
||||||
|
import("./accumulator").Buffers,
|
||||||
|
"byId" | "order"
|
||||||
|
>;
|
||||||
|
|
||||||
|
export function appendTaskNotificationEventsToBuffer(
|
||||||
|
summaries: string[],
|
||||||
|
buffer: NotificationBuffer,
|
||||||
|
generateId: () => string,
|
||||||
|
flush?: () => void,
|
||||||
|
): boolean {
|
||||||
|
if (summaries.length === 0) return false;
|
||||||
|
for (const summary of summaries) {
|
||||||
|
const eventId = generateId();
|
||||||
|
buffer.byId.set(eventId, {
|
||||||
|
kind: "event",
|
||||||
|
id: eventId,
|
||||||
|
eventType: "task_notification",
|
||||||
|
eventData: {},
|
||||||
|
phase: "finished",
|
||||||
|
summary,
|
||||||
|
});
|
||||||
|
buffer.order.push(eventId);
|
||||||
|
}
|
||||||
|
flush?.();
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Format multiple notifications as XML string.
|
* Format multiple notifications as XML string.
|
||||||
* @deprecated Use formatTaskNotification and queue individually
|
* @deprecated Use formatTaskNotification and queue individually
|
||||||
|
|||||||
174
src/tests/cli/task-notification-flush.test.ts
Normal file
174
src/tests/cli/task-notification-flush.test.ts
Normal file
@@ -0,0 +1,174 @@
|
|||||||
|
import { describe, expect, mock, test } from "bun:test";
|
||||||
|
import { readFileSync } from "node:fs";
|
||||||
|
import { fileURLToPath } from "node:url";
|
||||||
|
import type { NotificationBuffer } from "../../cli/helpers/taskNotifications";
|
||||||
|
import { appendTaskNotificationEventsToBuffer } from "../../cli/helpers/taskNotifications";
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// Helper-level behavioral tests
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe("appendTaskNotificationEventsToBuffer", () => {
|
||||||
|
const makeBuffer = (): NotificationBuffer => ({
|
||||||
|
byId: new Map(),
|
||||||
|
order: [],
|
||||||
|
});
|
||||||
|
|
||||||
|
let idCounter = 0;
|
||||||
|
const generateId = () => `event_${++idCounter}`;
|
||||||
|
|
||||||
|
test("writes events to buffer and calls flush", () => {
|
||||||
|
const buffer = makeBuffer();
|
||||||
|
const flush = mock(() => {});
|
||||||
|
|
||||||
|
const result = appendTaskNotificationEventsToBuffer(
|
||||||
|
["Agent completed", "Reflection done"],
|
||||||
|
buffer,
|
||||||
|
generateId,
|
||||||
|
flush,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result).toBe(true);
|
||||||
|
expect(buffer.order).toHaveLength(2);
|
||||||
|
expect(buffer.byId.size).toBe(2);
|
||||||
|
expect(flush).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
// Verify event shape
|
||||||
|
const firstId = buffer.order[0];
|
||||||
|
expect(firstId).toBeDefined();
|
||||||
|
const first = buffer.byId.get(firstId as string) as Record<string, unknown>;
|
||||||
|
expect(first.kind).toBe("event");
|
||||||
|
expect(first.eventType).toBe("task_notification");
|
||||||
|
expect(first.phase).toBe("finished");
|
||||||
|
expect(first.summary).toBe("Agent completed");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("flush is called exactly once even with multiple summaries", () => {
|
||||||
|
const buffer = makeBuffer();
|
||||||
|
const flush = mock(() => {});
|
||||||
|
|
||||||
|
appendTaskNotificationEventsToBuffer(
|
||||||
|
["one", "two", "three"],
|
||||||
|
buffer,
|
||||||
|
generateId,
|
||||||
|
flush,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(buffer.order).toHaveLength(3);
|
||||||
|
expect(flush).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("returns false and skips flush for empty summaries", () => {
|
||||||
|
const buffer = makeBuffer();
|
||||||
|
const flush = mock(() => {});
|
||||||
|
|
||||||
|
const result = appendTaskNotificationEventsToBuffer(
|
||||||
|
[],
|
||||||
|
buffer,
|
||||||
|
generateId,
|
||||||
|
flush,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result).toBe(false);
|
||||||
|
expect(buffer.order).toHaveLength(0);
|
||||||
|
expect(flush).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
test("works without flush callback (non-background caller)", () => {
|
||||||
|
const buffer = makeBuffer();
|
||||||
|
|
||||||
|
const result = appendTaskNotificationEventsToBuffer(
|
||||||
|
["Agent completed"],
|
||||||
|
buffer,
|
||||||
|
generateId,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(result).toBe(true);
|
||||||
|
expect(buffer.order).toHaveLength(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// Integration wiring: onComplete callbacks → appendTaskNotificationEvents
|
||||||
|
//
|
||||||
|
// These verify that the actual background subagent completion sites in App.tsx
|
||||||
|
// route through the flush-equipped appendTaskNotificationEvents, and that the
|
||||||
|
// ref indirection connecting it to refreshDerived is wired correctly.
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe("background onComplete → flush wiring in App.tsx", () => {
|
||||||
|
const readSource = () =>
|
||||||
|
readFileSync(
|
||||||
|
fileURLToPath(new URL("../../cli/App.tsx", import.meta.url)),
|
||||||
|
"utf-8",
|
||||||
|
);
|
||||||
|
|
||||||
|
test("appendTaskNotificationEvents delegates to appendTaskNotificationEventsToBuffer with a flush arg", () => {
|
||||||
|
const source = readSource();
|
||||||
|
|
||||||
|
// The useCallback must call the extracted helper
|
||||||
|
expect(source).toContain("appendTaskNotificationEventsToBuffer(");
|
||||||
|
|
||||||
|
// It must pass refreshDerivedRef as the flush callback (4th arg).
|
||||||
|
// Match the delegation pattern: the flush lambda references refreshDerivedRef.
|
||||||
|
expect(source).toContain("refreshDerivedRef.current?.");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("refreshDerivedRef is assigned after refreshDerived is defined", () => {
|
||||||
|
const source = readSource();
|
||||||
|
|
||||||
|
const refDecl = source.indexOf("const refreshDerivedRef = useRef");
|
||||||
|
const derivedDecl = source.indexOf("const refreshDerived = useCallback");
|
||||||
|
const refAssign = source.indexOf(
|
||||||
|
"refreshDerivedRef.current = refreshDerived",
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(refDecl).toBeGreaterThan(-1);
|
||||||
|
expect(derivedDecl).toBeGreaterThan(-1);
|
||||||
|
expect(refAssign).toBeGreaterThan(-1);
|
||||||
|
|
||||||
|
// Declaration order: ref declared before refreshDerived, assignment after
|
||||||
|
expect(refDecl).toBeLessThan(derivedDecl);
|
||||||
|
expect(refAssign).toBeGreaterThan(derivedDecl);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("/init onComplete calls appendTaskNotificationEvents", () => {
|
||||||
|
const source = readSource();
|
||||||
|
|
||||||
|
// Find the /init silentCompletion block and verify its onComplete
|
||||||
|
// calls appendTaskNotificationEvents (not addToMessageQueue or inline buffer writes)
|
||||||
|
const initBlock = source.indexOf('subagentType: "init"');
|
||||||
|
expect(initBlock).toBeGreaterThan(-1);
|
||||||
|
|
||||||
|
const onCompleteIdx = source.indexOf("onComplete:", initBlock);
|
||||||
|
expect(onCompleteIdx).toBeGreaterThan(-1);
|
||||||
|
|
||||||
|
// The appendTaskNotificationEvents call must be within the onComplete body
|
||||||
|
// (before the next closing of spawnBackgroundSubagentTask)
|
||||||
|
const callIdx = source.indexOf(
|
||||||
|
"appendTaskNotificationEvents(",
|
||||||
|
onCompleteIdx,
|
||||||
|
);
|
||||||
|
const blockEnd = source.indexOf("});", onCompleteIdx);
|
||||||
|
expect(callIdx).toBeGreaterThan(onCompleteIdx);
|
||||||
|
expect(callIdx).toBeLessThan(blockEnd);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("reflection onComplete calls appendTaskNotificationEvents", () => {
|
||||||
|
const source = readSource();
|
||||||
|
|
||||||
|
const reflectionBlock = source.indexOf('subagentType: "reflection"');
|
||||||
|
expect(reflectionBlock).toBeGreaterThan(-1);
|
||||||
|
|
||||||
|
const onCompleteIdx = source.indexOf("onComplete:", reflectionBlock);
|
||||||
|
expect(onCompleteIdx).toBeGreaterThan(-1);
|
||||||
|
|
||||||
|
const callIdx = source.indexOf(
|
||||||
|
"appendTaskNotificationEvents(",
|
||||||
|
onCompleteIdx,
|
||||||
|
);
|
||||||
|
const blockEnd = source.indexOf("});", onCompleteIdx);
|
||||||
|
expect(callIdx).toBeGreaterThan(onCompleteIdx);
|
||||||
|
expect(callIdx).toBeLessThan(blockEnd);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user