Skip to content

Commit 3fc850f

Browse files
bdjbenBenjamin Badejojesse-merhi
authored
fix(matrix): replace recovered command progress lines (#89920)
* fix(matrix): replace recovered command progress lines * fix(matrix): replace recovered command progress lines * fix(matrix): share command progress identity * fix(channels): share command progress identity * fix command progress draft replacement * fix command progress ids without changing public line ids * test(telegram): assert command progress preview update * fix(telegram): keep progress preview test typed --------- Co-authored-by: Benjamin Badejo <[email protected]> Co-authored-by: jesse-merhi <[email protected]>
1 parent ba1be23 commit 3fc850f

22 files changed

Lines changed: 1209 additions & 31 deletions

‎extensions/discord/src/monitor/message-handler.process.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ type DispatchInboundParams = {
142142
}) => Promise<void> | void;
143143
onItemEvent?: (payload: {
144144
itemId?: string;
145+
toolCallId?: string;
145146
kind?: string;
146147
progressText?: string;
147148
summary?: string;
@@ -156,6 +157,8 @@ type DispatchInboundParams = {
156157
}) => Promise<void> | void;
157158
onApprovalEvent?: (payload: { phase?: string; command?: string }) => Promise<void> | void;
158159
onCommandOutput?: (payload: {
160+
itemId?: string;
161+
toolCallId?: string;
159162
phase?: string;
160163
name?: string;
161164
title?: string;
@@ -2954,6 +2957,45 @@ describe("processDiscordMessage draft streaming", () => {
29542957
expectPreviewEditContent("done");
29552958
});
29562959

2960+
it("replaces Discord command progress items with matching command output", async () => {
2961+
const draftStream = createMockDraftStreamForTest();
2962+
2963+
dispatchInboundMessage.mockImplementationOnce(async (params?: DispatchInboundParams) => {
2964+
await params?.replyOptions?.onItemEvent?.({
2965+
itemId: "tool:call-1",
2966+
toolCallId: "call-1",
2967+
kind: "command",
2968+
name: "exec",
2969+
progressText: "install dependencies",
2970+
});
2971+
await params?.replyOptions?.onCommandOutput?.({
2972+
itemId: "tool:call-1-output",
2973+
toolCallId: "call-1",
2974+
phase: "end",
2975+
name: "exec",
2976+
exitCode: 0,
2977+
});
2978+
return createNoQueuedDispatchResult();
2979+
});
2980+
2981+
const ctx = await createAutomaticSourceDeliveryContext({
2982+
discordConfig: {
2983+
streaming: {
2984+
mode: "progress",
2985+
progress: {
2986+
label: "Shelling",
2987+
},
2988+
},
2989+
},
2990+
});
2991+
2992+
await runProcessDiscordMessage(ctx);
2993+
2994+
const lastUpdate = draftStream.update.mock.calls.at(-1)?.[0];
2995+
expect(lastUpdate).toContain("completed");
2996+
expect(lastUpdate).not.toContain("install dependencies");
2997+
});
2998+
29572999
it("drops later tool warning finals after progress preview final replies", async () => {
29583000
const draftStream = createMockDraftStreamForTest();
29593001

‎extensions/discord/src/monitor/message-handler.process.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,6 +1027,8 @@ async function processDiscordMessageInner(
10271027
discordConfig,
10281028
{
10291029
event: "tool",
1030+
itemId: payload.itemId,
1031+
toolCallId: payload.toolCallId,
10301032
name: payload.name,
10311033
phase: payload.phase,
10321034
args: payload.args,
@@ -1052,6 +1054,7 @@ async function processDiscordMessageInner(
10521054
buildChannelProgressDraftLineForEntry(discordConfig, {
10531055
event: "item",
10541056
itemId: payload.itemId,
1057+
toolCallId: payload.toolCallId,
10551058
itemKind: payload.kind,
10561059
title: payload.title,
10571060
name: payload.name,
@@ -1099,6 +1102,8 @@ async function processDiscordMessageInner(
10991102
await draftPreview.pushToolProgress(
11001103
buildChannelProgressDraftLine({
11011104
event: "command-output",
1105+
itemId: payload.itemId,
1106+
toolCallId: payload.toolCallId,
11021107
phase: payload.phase,
11031108
title: payload.title,
11041109
name: payload.name,
@@ -1114,6 +1119,8 @@ async function processDiscordMessageInner(
11141119
await draftPreview.pushToolProgress(
11151120
buildChannelProgressDraftLine({
11161121
event: "patch",
1122+
itemId: payload.itemId,
1123+
toolCallId: payload.toolCallId,
11171124
phase: payload.phase,
11181125
title: payload.title,
11191126
name: payload.name,

‎extensions/matrix/src/matrix/monitor/handler.test.ts‎

Lines changed: 183 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2950,12 +2950,24 @@ describe("matrix monitor handler draft streaming", () => {
29502950
) => Promise<void> | void;
29512951
onAssistantMessageStart?: () => void;
29522952
suppressDefaultToolProgressMessages?: boolean;
2953-
onToolStart?: (payload: { name?: string }) => Promise<void>;
2953+
onToolStart?: (payload: {
2954+
itemId?: string;
2955+
toolCallId?: string;
2956+
name?: string;
2957+
phase?: string;
2958+
args?: Record<string, unknown>;
2959+
detailMode?: "explain" | "raw";
2960+
}) => Promise<void>;
29542961
onItemEvent?: (payload: {
2962+
itemId?: string;
2963+
toolCallId?: string;
29552964
progressText?: string;
29562965
summary?: string;
29572966
title?: string;
29582967
name?: string;
2968+
kind?: string;
2969+
phase?: string;
2970+
status?: string;
29592971
}) => Promise<void>;
29602972
onPlanUpdate?: (payload: {
29612973
phase: string;
@@ -2964,15 +2976,24 @@ describe("matrix monitor handler draft streaming", () => {
29642976
}) => Promise<void>;
29652977
onApprovalEvent?: (payload: { phase: string; command?: string }) => Promise<void>;
29662978
onCommandOutput?: (payload: {
2979+
itemId?: string;
2980+
toolCallId?: string;
29672981
phase: string;
29682982
name?: string;
29692983
exitCode?: number;
2984+
status?: string;
29702985
title?: string;
29712986
}) => Promise<void>;
29722987
onPatchSummary?: (payload: {
2988+
itemId?: string;
2989+
toolCallId?: string;
29732990
phase: string;
2991+
name?: string;
29742992
summary?: string;
29752993
title?: string;
2994+
added?: string[];
2995+
modified?: string[];
2996+
deleted?: string[];
29762997
}) => Promise<void>;
29772998
disableBlockStreaming?: boolean;
29782999
};
@@ -3138,6 +3159,167 @@ describe("matrix monitor handler draft streaming", () => {
31383159
await finish();
31393160
});
31403161

3162+
it("replaces recovered Matrix command progress instead of leaving stale failed text", async () => {
3163+
const { dispatch } = createStreamingHarness({
3164+
streaming: "progress",
3165+
previewToolProgressEnabled: true,
3166+
accountConfig: {
3167+
streaming: { mode: "progress", progress: { label: "Working" } },
3168+
} as never,
3169+
});
3170+
const { opts, finish } = await dispatch();
3171+
3172+
await opts.onItemEvent?.({
3173+
itemId: "command-1",
3174+
kind: "command",
3175+
name: "exec",
3176+
phase: "end",
3177+
status: "failed",
3178+
progressText: "run openclaw cron -> run jq (agent) failed",
3179+
});
3180+
await opts.onItemEvent?.({
3181+
itemId: "command-1",
3182+
kind: "command",
3183+
name: "exec",
3184+
phase: "end",
3185+
status: "failed",
3186+
progressText: "run openclaw cron -> run jq (agent) failed",
3187+
});
3188+
3189+
await vi.waitFor(() => {
3190+
expect(sendSingleTextMessageMatrixMock).toHaveBeenCalledTimes(1);
3191+
});
3192+
expect(singleTextMessageBody()).toContain("failed");
3193+
3194+
await opts.onCommandOutput?.({
3195+
itemId: "command-1",
3196+
toolCallId: "call-1",
3197+
phase: "end",
3198+
name: "exec",
3199+
status: "completed",
3200+
exitCode: 0,
3201+
});
3202+
3203+
await finish();
3204+
expect(editMessageMatrixMock).toHaveBeenCalledWith(
3205+
"!room:example.org",
3206+
"$draft1",
3207+
expect.stringContaining("completed"),
3208+
expect.any(Object),
3209+
);
3210+
const recoveredEdit = mockCalls(editMessageMatrixMock, "editMessageMatrix").find(
3211+
([, eventId, body]) =>
3212+
eventId === "$draft1" && typeof body === "string" && body.includes("completed"),
3213+
);
3214+
expect(recoveredEdit?.[2]).not.toContain("failed");
3215+
expect(recoveredEdit?.[2]).not.toContain("run openclaw cron -> run jq");
3216+
});
3217+
3218+
it("replaces Matrix tool-start progress when command output completes", async () => {
3219+
const { dispatch } = createStreamingHarness({
3220+
streaming: "progress",
3221+
previewToolProgressEnabled: true,
3222+
accountConfig: {
3223+
streaming: { mode: "progress", progress: { label: "Working" } },
3224+
} as never,
3225+
});
3226+
const { opts, finish } = await dispatch();
3227+
3228+
await opts.onToolStart?.({
3229+
itemId: "fc-call-2",
3230+
toolCallId: "call-2",
3231+
name: "exec",
3232+
phase: "start",
3233+
args: { command: "npm install" },
3234+
});
3235+
await opts.onToolStart?.({
3236+
itemId: "fc-call-2",
3237+
toolCallId: "call-2",
3238+
name: "exec",
3239+
phase: "update",
3240+
args: { command: "npm install" },
3241+
});
3242+
3243+
await vi.waitFor(() => {
3244+
expect(sendSingleTextMessageMatrixMock).toHaveBeenCalledTimes(1);
3245+
});
3246+
expect(singleTextMessageBody()).toContain("install dependencies");
3247+
3248+
await opts.onItemEvent?.({
3249+
itemId: "fc-call-2",
3250+
toolCallId: "call-2",
3251+
kind: "command",
3252+
name: "exec",
3253+
phase: "update",
3254+
progressText: "install dependencies",
3255+
});
3256+
3257+
await opts.onCommandOutput?.({
3258+
itemId: "fc-call-2-output",
3259+
toolCallId: "call-2",
3260+
phase: "end",
3261+
name: "exec",
3262+
status: "completed",
3263+
exitCode: 0,
3264+
});
3265+
3266+
await finish();
3267+
const completedEdit = mockCalls(editMessageMatrixMock, "editMessageMatrix").find(
3268+
([, eventId, body]) =>
3269+
eventId === "$draft1" && typeof body === "string" && body.includes("completed"),
3270+
);
3271+
expect(completedEdit?.[2]).not.toContain("install dependencies");
3272+
});
3273+
3274+
it("replaces Matrix patch progress when the patch summary completes", async () => {
3275+
const { dispatch } = createStreamingHarness({
3276+
streaming: "progress",
3277+
previewToolProgressEnabled: true,
3278+
accountConfig: {
3279+
streaming: { mode: "progress", progress: { label: "Working" } },
3280+
} as never,
3281+
});
3282+
const { opts, finish } = await dispatch();
3283+
3284+
await opts.onItemEvent?.({
3285+
itemId: "patch:call-3",
3286+
toolCallId: "call-3",
3287+
kind: "patch",
3288+
name: "apply_patch",
3289+
phase: "update",
3290+
progressText: "updating Matrix progress handling",
3291+
});
3292+
await opts.onItemEvent?.({
3293+
itemId: "patch:call-3",
3294+
toolCallId: "call-3",
3295+
kind: "patch",
3296+
name: "apply_patch",
3297+
phase: "update",
3298+
progressText: "updating Matrix progress handling",
3299+
});
3300+
3301+
await vi.waitFor(() => {
3302+
expect(sendSingleTextMessageMatrixMock).toHaveBeenCalledTimes(1);
3303+
});
3304+
expect(singleTextMessageBody()).toContain("updating Matrix progress handling");
3305+
3306+
await opts.onPatchSummary?.({
3307+
itemId: "patch:call-3",
3308+
toolCallId: "call-3",
3309+
phase: "end",
3310+
name: "apply_patch",
3311+
modified: ["extensions/matrix/src/matrix/monitor/handler.ts"],
3312+
summary: "1 file modified",
3313+
});
3314+
3315+
await finish();
3316+
const patchEdit = mockCalls(editMessageMatrixMock, "editMessageMatrix").find(
3317+
([, eventId, body]) =>
3318+
eventId === "$draft1" && typeof body === "string" && body.includes("1 file modified"),
3319+
);
3320+
expect(patchEdit?.[2]).not.toContain("updating Matrix progress handling");
3321+
});
3322+
31413323
it("keeps Matrix tool progress mentions inside code formatting", async () => {
31423324
const { dispatch } = createStreamingHarness({
31433325
previewToolProgressEnabled: true,

‎extensions/matrix/src/matrix/monitor/handler.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import {
1616
createChannelProgressDraftGate,
1717
type ChannelProgressDraftLine,
1818
formatChannelProgressDraftLine,
19-
formatChannelProgressDraftLineForEntry,
2019
formatChannelProgressDraftText,
2120
isChannelProgressDraftWorkToolName,
2221
mergeChannelProgressDraftLine,
@@ -1933,10 +1932,12 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam
19331932
onToolStart: async (payload) => {
19341933
const toolName = payload.name?.trim();
19351934
await pushPreviewToolProgress(
1936-
formatChannelProgressDraftLineForEntry(
1935+
buildChannelProgressDraftLineForEntry(
19371936
progressConfigEntry,
19381937
{
19391938
event: "tool",
1939+
itemId: payload.itemId,
1940+
toolCallId: payload.toolCallId,
19401941
name: toolName,
19411942
phase: payload.phase,
19421943
args: payload.args,
@@ -1951,6 +1952,7 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam
19511952
buildChannelProgressDraftLineForEntry(progressConfigEntry, {
19521953
event: "item",
19531954
itemId: payload.itemId,
1955+
toolCallId: payload.toolCallId,
19541956
itemKind: payload.kind,
19551957
title: payload.title,
19561958
name: payload.name,
@@ -1996,8 +1998,10 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam
19961998
return;
19971999
}
19982000
await pushPreviewToolProgress(
1999-
formatChannelProgressDraftLine({
2001+
buildChannelProgressDraftLineForEntry(progressConfigEntry, {
20002002
event: "command-output",
2003+
itemId: payload.itemId,
2004+
toolCallId: payload.toolCallId,
20012005
phase: payload.phase,
20022006
title: payload.title,
20032007
name: payload.name,
@@ -2011,8 +2015,10 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam
20112015
return;
20122016
}
20132017
await pushPreviewToolProgress(
2014-
formatChannelProgressDraftLine({
2018+
buildChannelProgressDraftLineForEntry(progressConfigEntry, {
20152019
event: "patch",
2020+
itemId: payload.itemId,
2021+
toolCallId: payload.toolCallId,
20162022
phase: payload.phase,
20172023
title: payload.title,
20182024
name: payload.name,

0 commit comments

Comments
 (0)