diff --git a/extensions/slack/src/monitor/message-handler/dispatch-progress.ts b/extensions/slack/src/monitor/message-handler/dispatch-progress.ts index 9e01dd8afdff..220fd7c5a27a 100644 --- a/extensions/slack/src/monitor/message-handler/dispatch-progress.ts +++ b/extensions/slack/src/monitor/message-handler/dispatch-progress.ts @@ -324,9 +324,9 @@ export function createSlackProgressRuntime(runtimeParams: { seed: progressSeed, formatLine: formatSlackProgressDraftLine, reasoningLinePrefix: "🧠 ", - commentaryLinePrefix: "💬 ", + commentaryLinePrefix: "", reasoningGate: previewToolProgressEnabled, - commentaryItalics: false, + commentaryItalics: true, buildProgressEventLine: (input, options) => input.event === "tool" || input.event === "item" ? buildChannelProgressDraftLineForEntry(account.config, input, options) @@ -658,5 +658,29 @@ export function createSlackProgressRuntime(runtimeParams: { } function formatSlackProgressDraftLine(line: string): string { - return /^(?:🧠|💬)\s/u.test(line) ? line : escapeSlackMrkdwn(line); + if (/^(?:🧠|💬)\s/u.test(line)) { + return line; + } + + const italicCommentary = /^_(.*)_$/su.exec(line); + if (!italicCommentary) { + return escapeSlackMrkdwn(line); + } + + const content = italicCommentary[1]! + .split(/(`[^`\n]+`)/u) + .map((segment, index) => { + if (index % 2 === 0) { + return escapeSlackMrkdwn(segment); + } + const code = segment + .slice(1, -1) + .replaceAll("&", "&") + .replaceAll("<", "<") + .replaceAll(">", ">"); + return `\`${code}\``; + }) + .join(""); + + return `_${content}_`; } diff --git a/extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts b/extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts index 9eabec7ab189..908275dfc772 100644 --- a/extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts +++ b/extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts @@ -2149,9 +2149,9 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { expectMockCallArgFields(finalizeSlackPreviewEditMock, 0, "preview edit params", { channelId: "C123", messageId: "171234.567", - text: expect.stringMatching(/⏱️ \d+s$/), + text: FINAL_REPLY_TEXT, }); - expect(deliverRepliesMock).toHaveBeenCalledTimes(1); + expect(deliverRepliesMock).not.toHaveBeenCalled(); expect(draftStream.clear).not.toHaveBeenCalled(); }); @@ -2245,10 +2245,10 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { ], }); expect(finalizeSlackPreviewEditMock).toHaveBeenCalledTimes(1); - expect(deliverRepliesMock).toHaveBeenCalledTimes(1); + expect(deliverRepliesMock).not.toHaveBeenCalled(); }); - it("sends a progress final fresh before collapsing its draft to a receipt", async () => { + it("replaces the progress draft with the final answer without posting a receipt", async () => { const draftStream = createDraftStreamStub(); createSlackDraftStreamMock.mockReturnValueOnce(draftStream); finalizeSlackPreviewEditMock.mockResolvedValueOnce(undefined); @@ -2272,20 +2272,55 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { }), ); - expect(deliverRepliesMock).toHaveBeenCalledTimes(1); - expectDeliverReplyCall(0, FINAL_REPLY_TEXT); - expectMockCallArgFields(finalizeSlackPreviewEditMock, 0, "progress receipt edit", { + expect(deliverRepliesMock).not.toHaveBeenCalled(); + expectMockCallArgFields(finalizeSlackPreviewEditMock, 0, "progress final edit", { channelId: "C123", messageId: "171234.567", - text: expect.stringMatching(/^🛠️ 1 tool call · ⏱️ \d+s$/), + text: FINAL_REPLY_TEXT, }); - expect(deliverRepliesMock.mock.invocationCallOrder[0]).toBeLessThan( - finalizeSlackPreviewEditMock.mock.invocationCallOrder[0] ?? Number.POSITIVE_INFINITY, - ); + expect(finalizeSlackPreviewEditMock).toHaveBeenCalledTimes(1); expect(draftStream.clear).not.toHaveBeenCalled(); }); - it("leaves a progress draft untouched when the fresh final send fails", async () => { + it.each([ + { description: "plain text", finalText: "x".repeat(4001) }, + { description: "multibyte text", finalText: "é".repeat(2001) }, + ])( + "delivers oversized $description intact through the normal chunked sender", + async ({ finalText }) => { + const draftStream = createDraftStreamStub(); + createSlackDraftStreamMock.mockReturnValueOnce(draftStream); + mockedSlackStreamingMode = "progress"; + mockedSlackDraftMode = "status_final"; + mockedDispatchSequence = [{ kind: "final", payload: { text: finalText } }]; + mockedReplyOptionEvents = [ + { + kind: "item", + itemKind: "preamble", + itemId: "preamble-1", + progressText: "Checking the full answer before replying.", + }, + ]; + + await dispatchPreparedSlackMessage( + createPreparedSlackMessage({ + accountConfig: { + streaming: { + mode: "progress", + progress: { label: false, commentary: true, toolProgress: false }, + }, + }, + }), + ); + + expect(finalizeSlackPreviewEditMock).not.toHaveBeenCalled(); + expect(deliverRepliesMock).toHaveBeenCalledTimes(1); + expectDeliverReplyCall(0, finalText); + expect(draftStream.clear).toHaveBeenCalledTimes(1); + }, + ); + + it("retains the progress draft when both the final edit and fallback send fail", async () => { const draftStream = createDraftStreamStub(); createSlackDraftStreamMock.mockReturnValueOnce(draftStream); deliverRepliesMock.mockRejectedValueOnce(new Error("final send failed")); @@ -2303,11 +2338,14 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { ).rejects.toThrow("final send failed"); expect(draftStream.update).toHaveBeenCalled(); - expect(finalizeSlackPreviewEditMock).not.toHaveBeenCalled(); + expectMockCallArgFields(finalizeSlackPreviewEditMock, 0, "progress final edit", { + messageId: "171234.567", + text: FINAL_REPLY_TEXT, + }); expect(draftStream.clear).not.toHaveBeenCalled(); }); - it("keeps a progress draft for an error final without creating a receipt", async () => { + it("replaces a progress draft with an error final without creating a receipt", async () => { const draftStream = createDraftStreamStub(); createSlackDraftStreamMock.mockReturnValueOnce(draftStream); mockedSlackStreamingMode = "progress"; @@ -2323,7 +2361,7 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { expect(deliverRepliesMock).toHaveBeenCalledTimes(1); expect(finalizeSlackPreviewEditMock).not.toHaveBeenCalled(); - expect(draftStream.clear).not.toHaveBeenCalled(); + expect(draftStream.clear).toHaveBeenCalledTimes(1); }); it("mandatory E2E: streams native Slack progress with the newest meaningful plan title when no explicit label exists", async () => { @@ -3532,7 +3570,7 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { ], }); expect(finalizeSlackPreviewEditMock).toHaveBeenCalledTimes(1); - expect(deliverRepliesMock).toHaveBeenCalledTimes(1); + expect(deliverRepliesMock).not.toHaveBeenCalled(); }); it("preserves text Slack progress lines after a draft boundary status update", async () => { @@ -3736,7 +3774,7 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { expect(capturedReplyOptions?.commentaryProgressEnabled).toBe(true); expect(capturedReplyOptions?.suppressDefaultToolProgressMessages).toBe(true); - expect(draftStream.update).toHaveBeenLastCalledWith("💬 Preparing the smallest fix"); + expect(draftStream.update).toHaveBeenLastCalledWith("_Preparing the smallest fix_"); expect(draftStream.update.mock.calls.flat().join("\n")).not.toContain("pnpm test"); const updateCount = draftStream.update.mock.calls.length; @@ -3776,10 +3814,96 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { ); expect(draftStream.update).toHaveBeenLastCalledWith( - "💬 I’m using the `monorepo` skill on Linux x86_64.", + "_I’m using the `monorepo` skill on Linux x86\\_64._", ); }); + it("escapes Slack mentions and formatting in commentary without losing outer italics or inline code", async () => { + const draftStream = createDraftStreamStub(); + createSlackDraftStreamMock.mockReturnValueOnce(draftStream); + mockedSlackStreamingMode = "progress"; + mockedSlackDraftMode = "status_final"; + mockedDispatchSequence = []; + mockedReplyOptionEvents = [ + { + kind: "item", + itemKind: "preamble", + itemId: "preamble-1", + progressText: + "checking <@U123> in <#C123> and with *urgent* _context_ `src/one.ts`", + }, + ]; + + await dispatchPreparedSlackMessage( + createPreparedSlackMessage({ + accountConfig: { + streaming: { + mode: "progress", + progress: { label: false, commentary: true, toolProgress: false }, + }, + }, + }), + ); + + expect(draftStream.update).toHaveBeenLastCalledWith( + "_checking <@U123> in <#C123> and <!channel> with \\*urgent\\* \\_context\\_ `src/one.ts`_", + ); + }); + + it("keeps the full latest preamble and turns the same Slack message into the final answer", async () => { + const draftStream = createDraftStreamStub(); + createSlackDraftStreamMock.mockReturnValueOnce(draftStream); + finalizeSlackPreviewEditMock.mockResolvedValueOnce(undefined); + mockedSlackStreamingMode = "progress"; + mockedSlackDraftMode = "status_final"; + mockedDispatchSequence = [{ kind: "final", payload: { text: FINAL_REPLY_TEXT } }]; + const firstPreamble = "Checking the previous conversation before replying."; + const latestPreamble = + "I found the earlier decision and am checking the owner, the current rollout, and the original feedback before deciding what would actually be useful here."; + mockedReplyOptionEvents = [ + { + kind: "item", + itemKind: "preamble", + itemId: "preamble-1", + progressText: firstPreamble, + }, + { + kind: "item", + itemKind: "preamble", + itemId: "preamble-2", + progressText: latestPreamble, + }, + ]; + + await dispatchPreparedSlackMessage( + createPreparedSlackMessage({ + accountConfig: { + streaming: { + mode: "progress", + progress: { + label: false, + commentary: true, + toolProgress: false, + maxLines: 1, + maxLineChars: 4000, + }, + }, + }, + }), + ); + + expect(draftStream.update).toHaveBeenCalledWith(`_${firstPreamble}_`); + expect(draftStream.update).toHaveBeenLastCalledWith(`_${latestPreamble}_`); + expect(finalizeSlackPreviewEditMock).toHaveBeenCalledTimes(1); + expectMockCallArgFields(finalizeSlackPreviewEditMock, 0, "progress final edit", { + channelId: "C123", + messageId: "171234.567", + text: FINAL_REPLY_TEXT, + }); + expect(deliverRepliesMock).not.toHaveBeenCalled(); + expect(draftStream.update.mock.calls.flat().join("\n")).not.toMatch(/Working|💬|•|⏱️/u); + }); + it("uses the enterprise event client for Slack commentary drafts", async () => { const draftStream = createDraftStreamStub(); createSlackDraftStreamMock.mockReturnValueOnce(draftStream); @@ -3827,7 +3951,7 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { expect(createSlackDraftStreamMock).toHaveBeenCalledWith( expect.objectContaining({ eventScope }), ); - expect(draftStream.update).toHaveBeenLastCalledWith("💬 Using the scoped listener client"); + expect(draftStream.update).toHaveBeenLastCalledWith("_Using the scoped listener client_"); }); it("renders the latest Slack preamble as the status headline by default", async () => { @@ -4025,7 +4149,7 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { ], }); expect(finalizeSlackPreviewEditMock).toHaveBeenCalledTimes(1); - expect(deliverRepliesMock).toHaveBeenCalledTimes(1); + expect(deliverRepliesMock).not.toHaveBeenCalled(); }); it("suppresses standalone Slack tool progress when partial preview lines are disabled", async () => { diff --git a/extensions/slack/src/monitor/message-handler/dispatch.ts b/extensions/slack/src/monitor/message-handler/dispatch.ts index 72ad38a3ddbf..ff0a3f477b8d 100644 --- a/extensions/slack/src/monitor/message-handler/dispatch.ts +++ b/extensions/slack/src/monitor/message-handler/dispatch.ts @@ -110,45 +110,6 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag } return; } - - if (hadProgressDraft) { - // Best-effort settle of the working draft; a flush failure must never - // suppress the fresh final send below. - try { - await draftStream?.flush(); - } catch (err) { - logVerbose(`slack: progress draft flush before final failed (${formatSlackError(err)})`); - } - } - const receiptChannelId = hadProgressDraft ? draftStream?.channelId() : undefined; - const receiptMessageId = hadProgressDraft ? draftStream?.messageId() : undefined; - // The draft already selected the reply thread; re-planning here could - // route the fresh final elsewhere under stateful replyToMode values. - const draftThreadTs = hadProgressDraft - ? (delivery.usedReplyThreadTs ?? statusThreadTs) - : undefined; - await delivery.deliverNormally({ - payload, - kind: info.kind, - ...(draftThreadTs ? { forcedThreadTs: draftThreadTs } : {}), - }); - progress.progressDraft.markFinalReplyDelivered(); - if ( - !payload.isError && - receiptChannelId && - receiptMessageId && - !progress.progressReceiptCollapsed - ) { - // Collapse only after the fresh final lands; a failed send leaves the - // working draft untouched as the turn record. - await progress.collapseProgressReceipt({ - channelId: receiptChannelId, - messageId: receiptMessageId, - text: progress.progressReceipt.buildSummaryLine(), - threadTs: delivery.usedReplyThreadTs ?? statusThreadTs, - }); - } - return; } if (progress.useNativeProgressStreaming) { await delivery.deliverNormally({ @@ -259,6 +220,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag forcedThreadTs: finalThreadTs, }); delivery.markPreviewPayloadDelivered({ kind: info.kind, payload, threadTs: finalThreadTs }); + progress.progressDraft.markFinalReplyDelivered(); return; } } @@ -377,6 +339,9 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag }); }, }); + if (info.kind === "final") { + progress.progressDraft.markFinalReplyDelivered(); + } }; let dispatchError: unknown; let queuedFinal = false;