Keep Slack progress and final replies in one message (#119376) (#119480)

This commit is contained in:
pash-openai
2026-08-04 21:49:38 -07:00
committed by GitHub
parent a1d20ae31b
commit 75ef5aebec
3 changed files with 175 additions and 62 deletions
@@ -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("<", "&lt;")
.replaceAll(">", "&gt;");
return `\`${code}\``;
})
.join("");
return `_${content}_`;
}
@@ -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(
"💬 Im using the `monorepo` skill on Linux x86_64.",
"_Im 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 <!channel> 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 &lt;@U123&gt; in &lt;#C123&gt; and &lt;!channel&gt; 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 () => {
@@ -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;