Skip to content

Commit 076f032

Browse files
litang9litang9
committed
fix(auto-reply): hide recovered failed tool progress
1 parent e114001 commit 076f032

4 files changed

Lines changed: 131 additions & 33 deletions

File tree

‎src/auto-reply/reply/dispatch-from-config.test.ts‎

Lines changed: 55 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3866,13 +3866,7 @@ describe("dispatchReplyFromConfig", () => {
38663866
});
38673867

38683868
expect(result.sourceReplyDeliveryMode).toBe("message_tool_only");
3869-
expect(onCommandOutput).toHaveBeenCalledWith({
3870-
phase: "end",
3871-
title: "Exec",
3872-
name: "exec",
3873-
status: "failed",
3874-
exitCode: 1,
3875-
});
3869+
expect(onCommandOutput).not.toHaveBeenCalled();
38763870
expect(dispatcher.sendToolResult).not.toHaveBeenCalled();
38773871
expect(dispatcher.sendFinalReply).not.toHaveBeenCalled();
38783872
});
@@ -3886,6 +3880,7 @@ describe("dispatchReplyFromConfig", () => {
38863880
verboseLevel: "on",
38873881
};
38883882
const onCommandOutput = vi.fn();
3883+
const onToolResult = vi.fn();
38893884
const cfg = emptyConfig;
38903885
const dispatcher = createDispatcher();
38913886
const ctx = buildTestCtx({
@@ -3922,17 +3917,13 @@ describe("dispatchReplyFromConfig", () => {
39223917
sourceReplyDeliveryMode: "message_tool_only",
39233918
allowProgressCallbacksWhenSourceDeliverySuppressed: true,
39243919
onCommandOutput,
3920+
onToolResult,
39253921
},
39263922
});
39273923

39283924
expect(result.sourceReplyDeliveryMode).toBe("message_tool_only");
3929-
expect(onCommandOutput).toHaveBeenCalledWith({
3930-
phase: "end",
3931-
title: "Exec",
3932-
name: "exec",
3933-
status: "failed",
3934-
exitCode: 2,
3935-
});
3925+
expect(onCommandOutput).not.toHaveBeenCalled();
3926+
expect(onToolResult).not.toHaveBeenCalled();
39363927
expect(dispatcher.sendToolResult).not.toHaveBeenCalled();
39373928
expect(dispatcher.sendFinalReply).not.toHaveBeenCalled();
39383929
});
@@ -4024,7 +4015,7 @@ describe("dispatchReplyFromConfig", () => {
40244015
expect(dispatcher.sendFinalReply).not.toHaveBeenCalled();
40254016
});
40264017

4027-
it("suppresses terminal tool-error fallbacks when regular verbose progress is visible", async () => {
4018+
it("keeps terminal tool-error fallbacks available when regular verbose failed command progress is hidden", async () => {
40284019
setNoAbort();
40294020
sessionStoreMocks.currentEntry = {
40304021
sessionId: "s1",
@@ -4060,18 +4051,13 @@ describe("dispatchReplyFromConfig", () => {
40604051
replyOptions: { onCommandOutput },
40614052
});
40624053

4063-
expect(onCommandOutput).toHaveBeenCalledWith({
4064-
phase: "end",
4065-
name: "exec",
4066-
status: "failed",
4067-
exitCode: 1,
4068-
});
4054+
expect(onCommandOutput).not.toHaveBeenCalled();
40694055
expect(receivedOptions?.suppressToolErrorWarnings).toBeUndefined();
4070-
expect(receivedOptions?.shouldSuppressToolErrorWarnings?.()).toBe(true);
4056+
expect(receivedOptions?.shouldSuppressToolErrorWarnings?.()).toBeUndefined();
40714057
expect(dispatcher.sendFinalReply).toHaveBeenCalledWith({ text: "done" });
40724058
});
40734059

4074-
it("suppresses terminal tool-error fallbacks in group sessions when verbose progress is visible", async () => {
4060+
it("keeps terminal tool-error fallbacks available when regular verbose failed item progress is hidden", async () => {
40754061
setNoAbort();
40764062
sessionStoreMocks.currentEntry = {
40774063
sessionId: "s1",
@@ -4109,14 +4095,56 @@ describe("dispatchReplyFromConfig", () => {
41094095
replyOptions: { onItemEvent },
41104096
});
41114097

4112-
expect(onItemEvent).toHaveBeenCalledWith({
4113-
itemId: "item-1",
4114-
kind: "tool",
4098+
expect(onItemEvent).not.toHaveBeenCalled();
4099+
expect(receivedOptions?.suppressToolErrorWarnings).toBeUndefined();
4100+
expect(receivedOptions?.shouldSuppressToolErrorWarnings?.()).toBeUndefined();
4101+
expect(dispatcher.sendFinalReply).toHaveBeenCalledWith({ text: "done" });
4102+
});
4103+
4104+
it("keeps full verbose failed command progress visible", async () => {
4105+
setNoAbort();
4106+
sessionStoreMocks.currentEntry = {
4107+
sessionId: "s1",
4108+
updatedAt: 0,
4109+
sendPolicy: "allow",
4110+
verboseLevel: "full",
4111+
};
4112+
const dispatcher = createDispatcher();
4113+
const onCommandOutput = vi.fn();
4114+
const ctx = buildTestCtx({
4115+
Provider: "telegram",
4116+
ChatType: "direct",
4117+
SessionKey: "agent:main:telegram:direct:U1",
4118+
});
4119+
let receivedOptions: GetReplyOptions | undefined;
4120+
const replyResolver = vi.fn(async (_ctx: MsgContext, opts?: GetReplyOptions) => {
4121+
receivedOptions = opts;
4122+
expect(opts?.shouldSuppressToolErrorWarnings?.()).toBeUndefined();
4123+
await opts?.onCommandOutput?.({
4124+
phase: "end",
4125+
name: "exec",
4126+
status: "failed",
4127+
exitCode: 1,
4128+
});
4129+
return { text: "done" } satisfies ReplyPayload;
4130+
});
4131+
4132+
await dispatchReplyFromConfig({
4133+
ctx,
4134+
cfg: emptyConfig,
4135+
dispatcher,
4136+
replyResolver,
4137+
replyOptions: { onCommandOutput },
4138+
});
4139+
4140+
expect(onCommandOutput).toHaveBeenCalledWith({
4141+
phase: "end",
41154142
name: "exec",
41164143
status: "failed",
4144+
exitCode: 1,
41174145
});
41184146
expect(receivedOptions?.suppressToolErrorWarnings).toBeUndefined();
4119-
expect(receivedOptions?.shouldSuppressToolErrorWarnings?.()).toBe(true);
4147+
expect(receivedOptions?.shouldSuppressToolErrorWarnings?.()).toBeUndefined();
41204148
expect(dispatcher.sendFinalReply).toHaveBeenCalledWith({ text: "done" });
41214149
});
41224150

‎src/auto-reply/reply/dispatch-from-config.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2730,6 +2730,13 @@ export async function dispatchReplyFromConfig(
27302730
payload.status === "failed" ||
27312731
payload.status === "error" ||
27322732
(typeof payload.exitCode === "number" && payload.exitCode !== 0);
2733+
const shouldForwardFailedProgress = (options?: { force?: boolean }) =>
2734+
options?.force === true || shouldEmitFullVerboseProgress();
2735+
const shouldForwardFailedProgressStatus = (payload: {
2736+
phase?: string;
2737+
status?: string;
2738+
exitCode?: number | null;
2739+
}) => shouldForwardFailedProgress() || !hasFailedProgressStatus(payload);
27332740
const shouldSuppressToolErrorWarnings = () => {
27342741
if (params.replyOptions?.suppressToolErrorWarnings !== undefined) {
27352742
return params.replyOptions.suppressToolErrorWarnings;
@@ -2755,7 +2762,14 @@ export async function dispatchReplyFromConfig(
27552762
const shouldForwardToolResultProgressCallback = (
27562763
payload: ReplyPayload,
27572764
isFastModeAutoProgress: boolean,
2765+
isForcedToolProgress: boolean,
27582766
) => {
2767+
if (
2768+
payload.isError === true &&
2769+
!shouldForwardFailedProgress({ force: isForcedToolProgress })
2770+
) {
2771+
return false;
2772+
}
27592773
if (isFastModeAutoProgress) {
27602774
return shouldForwardProgressCallback({ forwardWhenSourceDeliverySuppressed: true });
27612775
}
@@ -2811,6 +2825,7 @@ export async function dispatchReplyFromConfig(
28112825
forwardWhenSourceDeliverySuppressed?: boolean;
28122826
requiresToolSummaryVisibility?: boolean;
28132827
onForward?: (...args: Args) => Promise<void> | void;
2828+
shouldForward?: (...args: Args) => boolean;
28142829
waitForDirectBlockReplyDelivery?: boolean;
28152830
},
28162831
): ((...args: Args) => Promise<void>) | undefined => {
@@ -2828,6 +2843,9 @@ export async function dispatchReplyFromConfig(
28282843
return;
28292844
}
28302845
}
2846+
if (options?.shouldForward?.(...args) === false) {
2847+
return;
2848+
}
28312849
if (shouldForwardProgressCallback(options)) {
28322850
await options?.onForward?.(...args);
28332851
await callback?.(...args);
@@ -2854,6 +2872,7 @@ export async function dispatchReplyFromConfig(
28542872
? wrapProgressCallback(params.replyOptions?.onItemEvent, {
28552873
...itemEventForwardingOptions,
28562874
waitForDirectBlockReplyDelivery: true,
2875+
shouldForward: shouldForwardFailedProgressStatus,
28572876
onForward: (payload) => {
28582877
if (hasFailedProgressStatus(payload)) {
28592878
markVisibleToolErrorProgress();
@@ -2939,6 +2958,7 @@ export async function dispatchReplyFromConfig(
29392958
forwardWhenSourceDeliverySuppressed: true,
29402959
requiresToolSummaryVisibility: true,
29412960
waitForDirectBlockReplyDelivery: true,
2961+
shouldForward: shouldForwardFailedProgressStatus,
29422962
onForward: (payload) => {
29432963
if (hasFailedProgressStatus(payload)) {
29442964
markVisibleToolErrorProgress();
@@ -2974,6 +2994,7 @@ export async function dispatchReplyFromConfig(
29742994
const progressCallbackForwarded = shouldForwardToolResultProgressCallback(
29752995
payload,
29762996
isFastModeAutoProgress,
2997+
isForcedToolProgress,
29772998
);
29782999
if (progressCallbackForwarded) {
29793000
await onToolResultFromReplyOptions?.(payload);
@@ -3034,6 +3055,12 @@ export async function dispatchReplyFromConfig(
30343055
if (shouldSuppressMessageToolOnlyTextErrorProgress(deliveryPayload)) {
30353056
return;
30363057
}
3058+
if (
3059+
deliveryPayload.isError === true &&
3060+
!shouldForwardFailedProgress({ force: isForcedToolProgress })
3061+
) {
3062+
return;
3063+
}
30373064
if (
30383065
shouldSuppressDefaultToolProgressMessages() &&
30393066
!isFastModeAutoProgressPayload(deliveryPayload) &&

‎src/auto-reply/reply/followup-runner.test.ts‎

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2353,6 +2353,42 @@ describe("createFollowupRunner progress forwarding", () => {
23532353
);
23542354
});
23552355

2356+
it("keeps queued regular-verbose failed tool result payloads hidden", async () => {
2357+
const queued = createQueuedRun({
2358+
originatingChannel: "discord",
2359+
originatingTo: "channel:C1",
2360+
originatingAccountId: "acct-1",
2361+
originatingThreadId: "thread-1",
2362+
run: {
2363+
messageProvider: "discord",
2364+
sourceReplyDeliveryMode: "message_tool_only",
2365+
verboseLevel: "on",
2366+
},
2367+
});
2368+
2369+
runEmbeddedAgentMock.mockImplementationOnce(
2370+
async (args: {
2371+
onToolResult?: (payload: { text: string; isError?: boolean }) => Promise<void>;
2372+
}) => {
2373+
await args.onToolResult?.({
2374+
text: "🛠️ Exec: failing queued helper",
2375+
isError: true,
2376+
});
2377+
return { payloads: [], meta: { agentMeta: {} } };
2378+
},
2379+
);
2380+
2381+
const runner = createFollowupRunner({
2382+
typing: createMockTypingController(),
2383+
typingMode: "instant",
2384+
defaultModel: "claude",
2385+
});
2386+
2387+
await runner(queued);
2388+
2389+
expect(routeReplyMock).not.toHaveBeenCalled();
2390+
});
2391+
23562392
it("drains fire-and-forget queued tool progress before final delivery", async () => {
23572393
const queued = createQueuedRun({
23582394
originatingChannel: "discord",
@@ -2568,7 +2604,7 @@ describe("createFollowupRunner progress forwarding", () => {
25682604
});
25692605
});
25702606

2571-
it("marks queued Codex command tool result errors as failed command output", async () => {
2607+
it("marks queued Codex command tool result errors as failed command output in full verbose", async () => {
25722608
const onCommandOutput = vi.fn(async () => {});
25732609
const queued = createQueuedRun({
25742610
originatingChannel: "discord",
@@ -2577,7 +2613,7 @@ describe("createFollowupRunner progress forwarding", () => {
25772613
originatingThreadId: "thread-1",
25782614
run: {
25792615
messageProvider: "discord",
2580-
verboseLevel: "on",
2616+
verboseLevel: "full",
25812617
},
25822618
});
25832619

@@ -2862,7 +2898,7 @@ describe("createFollowupRunner progress forwarding", () => {
28622898
expect(routeReplyMock).not.toHaveBeenCalled();
28632899
});
28642900

2865-
it("does not reuse dispatch-scoped tool-error suppression across queued follow-ups", async () => {
2901+
it("keeps queued regular-verbose failed progress hidden across follow-ups", async () => {
28662902
const onCommandOutput = vi.fn(async () => {});
28672903

28682904
runEmbeddedAgentMock
@@ -2882,7 +2918,7 @@ describe("createFollowupRunner progress forwarding", () => {
28822918
exitCode: 1,
28832919
},
28842920
});
2885-
expect(shouldSuppress()).toBe(true);
2921+
expect(shouldSuppress()).toBeUndefined();
28862922
return { payloads: [], meta: { agentMeta: {} } };
28872923
},
28882924
)
@@ -2920,7 +2956,7 @@ describe("createFollowupRunner progress forwarding", () => {
29202956
}),
29212957
);
29222958

2923-
expect(onCommandOutput).toHaveBeenCalledTimes(1);
2959+
expect(onCommandOutput).not.toHaveBeenCalled();
29242960
});
29252961

29262962
it("keeps queued full-verbose tool-error fallbacks available after failed progress", async () => {

‎src/auto-reply/reply/followup-runner.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,7 @@ export function createFollowupRunner(params: {
558558
shouldEmitVerboseProgress() && !shouldSuppressDefaultToolProgressMessages();
559559
const shouldEmitToolOutputProgress = () =>
560560
resolveCurrentVerboseLevel() === "full" && !shouldSuppressDefaultToolProgressMessages();
561+
const shouldEmitFailedToolProgress = () => resolveCurrentVerboseLevel() === "full";
561562
let observedVisibleToolErrorProgress = false;
562563
const markVisibleToolErrorProgress = () => {
563564
if (resolveCurrentVerboseLevel() === "on" && shouldEmitToolResultProgress()) {
@@ -936,6 +937,9 @@ export function createFollowupRunner(params: {
936937
) {
937938
return;
938939
}
940+
if (payload.isError === true && !shouldEmitFailedToolProgress()) {
941+
return;
942+
}
939943
await sendFollowupPayloads(
940944
[payload],
941945
effectiveQueued,
@@ -1224,7 +1228,9 @@ export function createFollowupRunner(params: {
12241228
evt,
12251229
opts,
12261230
detailMode: toolProgressDetail,
1227-
emitChannelProgress: shouldEmitToolResultProgress(),
1231+
emitChannelProgress:
1232+
shouldEmitToolResultProgress() &&
1233+
(!hasFailedFollowupProgressEvent(evt) || shouldEmitFailedToolProgress()),
12281234
onCompactionComplete: () => {
12291235
attemptCompactionCount += 1;
12301236
},
@@ -1235,6 +1241,7 @@ export function createFollowupRunner(params: {
12351241
});
12361242
if (
12371243
hasFailedFollowupProgressEvent(evt) &&
1244+
shouldEmitFailedToolProgress() &&
12381245
canForwardFailedFollowupProgressEvent(evt, opts)
12391246
) {
12401247
markVisibleToolErrorProgress();

0 commit comments

Comments
 (0)