diff --git a/docs/concepts/qa-e2e-automation.md b/docs/concepts/qa-e2e-automation.md index 55300af4c22f..e85b9a33c8e8 100644 --- a/docs/concepts/qa-e2e-automation.md +++ b/docs/concepts/qa-e2e-automation.md @@ -614,6 +614,11 @@ Imperative Slack scenarios (`extensions/qa-lab/src/live-transports/slack/slack-l - `slack-allowlist-block` - `slack-top-level-reply-shape` - `slack-restart-resume` +- `slack-progress-commentary-true`, `slack-progress-commentary-false`, + `slack-progress-commentary-omitted`, and + `slack-progress-commentary-verbose-dedupe` - opt-in real-Slack probes for + independent commentary/tool-progress controls, the omitted-key legacy + default, and single-delivery behavior when durable verbose progress is on. - `slack-reaction-glyph-native` - opt-in live message-tool reaction scenario. Instructs the agent to pass the exact `✅` glyph and confirms Slack stored `white_check_mark` for the SUT bot on the target message. diff --git a/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.test.ts b/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.test.ts index b255cf06b82b..6bff67efc37b 100644 --- a/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.test.ts +++ b/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.test.ts @@ -103,6 +103,10 @@ describe("Slack live QA runtime helpers", () => { "slack-chart-presentation-native", "slack-table-presentation-native", "slack-table-invalid-blocks-fallback", + "slack-progress-commentary-true", + "slack-progress-commentary-false", + "slack-progress-commentary-omitted", + "slack-progress-commentary-verbose-dedupe", "slack-reaction-glyph-native", "slack-approval-exec-native", "slack-approval-plugin-native", @@ -111,6 +115,10 @@ describe("Slack live QA runtime helpers", () => { ]) .map((scenario) => scenario.id), ).toEqual([ + "slack-progress-commentary-true", + "slack-progress-commentary-false", + "slack-progress-commentary-omitted", + "slack-progress-commentary-verbose-dedupe", "slack-chart-presentation-native", "slack-table-presentation-native", "slack-table-invalid-blocks-fallback", @@ -133,6 +141,9 @@ describe("Slack live QA runtime helpers", () => { expect(testing.findScenario().map((scenario) => scenario.id)).not.toContain( "slack-table-invalid-blocks-fallback", ); + expect(testing.findScenario().map((scenario) => scenario.id)).not.toContain( + "slack-progress-commentary-true", + ); expect(testing.findScenario(["slack-codex-approval-exec-native"])[0]?.forcedRuntime).toBe( "codex", ); @@ -271,6 +282,247 @@ describe("Slack live QA runtime helpers", () => { expect(account?.channels?.C123456789?.users).toEqual(["U_NEVER_ALLOWED"]); }); + it("builds the Slack progress commentary true, false, omitted, and dedupe configs", () => { + const buildScenarioConfig = (scenarioId: string) => { + const scenario = testing.findScenario([scenarioId])[0]; + if (!scenario) { + throw new Error(`missing Slack QA scenario: ${scenarioId}`); + } + return testing.buildSlackQaConfig( + { + agents: { + defaults: { verboseDefault: "off" }, + list: [{ id: "qa", identity: { name: "C-3PO QA" } }], + }, + }, + { + channelId: "C123456789", + driverBotUserId: "U999999999", + overrides: scenario.configOverrides, + sutAccountId: "sut", + sutAppToken: "xapp-sut", + sutBotToken: "xoxb-sut", + }, + ); + }; + const progressConfig = (scenarioId: string) => + buildScenarioConfig(scenarioId).channels?.slack?.accounts?.sut?.streaming?.progress; + + expect(progressConfig("slack-progress-commentary-true")).toMatchObject({ + commentary: true, + toolProgress: false, + }); + expect(progressConfig("slack-progress-commentary-false")).toMatchObject({ + commentary: false, + toolProgress: false, + }); + expect( + buildScenarioConfig("slack-progress-commentary-false").agents?.defaults?.verboseDefault, + ).toBe("off"); + const omitted = progressConfig("slack-progress-commentary-omitted"); + expect(omitted).toMatchObject({ toolProgress: true }); + expect(Object.hasOwn(omitted ?? {}, "commentary")).toBe(false); + expect( + buildScenarioConfig("slack-progress-commentary-verbose-dedupe").agents?.defaults + ?.verboseDefault, + ).toBe("on"); + expect(buildScenarioConfig("slack-progress-commentary-true").agents?.list?.[0]?.identity).toBe( + undefined, + ); + }); + + it("verifies progress commentary by Slack message identity", () => { + const cases = [ + { + id: "slack-progress-commentary-true", + commentaryTs: "2.000000", + toolProgress: "absent", + }, + { + id: "slack-progress-commentary-false", + commentaryTs: undefined, + toolProgress: "absent", + }, + { + id: "slack-progress-commentary-omitted", + commentaryTs: "2.000000", + toolProgress: "draft", + }, + { + id: "slack-progress-commentary-verbose-dedupe", + commentaryTs: "1.500000", + toolProgress: "standalone", + }, + ] as const; + + for (const testCase of cases) { + const scenario = testing.findScenario([testCase.id])[0]; + const run = scenario?.buildRun("U999999999"); + const input = run && "input" in run ? run.input : ""; + const commentaryMarker = input.match(/SLACK-QA-COMMENTARY-[0-9A-F]{8}/u)?.[0]; + const toolMarker = input.match(/SLACK-QA-TOOL-[0-9A-F]{8}/u)?.[0]; + const finalMarker = input.match(/SLACK-QA-COMMENTARY-DONE-[0-9A-F]{8}/u)?.[0]; + const verifyObserved = run && "verifyObserved" in run ? run.verifyObserved : undefined; + if (!commentaryMarker || !toolMarker || !finalMarker || !verifyObserved) { + throw new Error(`missing Slack progress verifier: ${testCase.id}`); + } + const messages = [ + { + channelId: "C123456789", + text: finalMarker, + ts: "2.000000", + }, + ...(testCase.commentaryTs + ? [ + { + channelId: "C123456789", + text: `💬 ${commentaryMarker}`, + ts: testCase.commentaryTs, + }, + ] + : []), + ...(testCase.toolProgress === "absent" + ? [] + : [ + { + channelId: "C123456789", + text: `🛠️ Exec ${toolMarker}`, + ts: testCase.toolProgress === "draft" ? "2.000000" : "1.750000", + }, + ]), + ]; + expect( + verifyObserved({ + finalMessage: { text: finalMarker, ts: "2.000000" }, + messages, + }), + ).toContain("verified"); + } + }); + + it("rejects commentary when false and mismatched tool progress", () => { + const verify = ( + scenarioId: string, + mutate: (markers: string[]) => string[], + finalText: "echo" | "exact" = "exact", + ) => { + const scenario = testing.findScenario([scenarioId])[0]; + const run = scenario?.buildRun("U999999999"); + const input = run && "input" in run ? run.input : ""; + const markers = [ + input.match(/SLACK-QA-COMMENTARY-[0-9A-F]{8}/u)?.[0], + input.match(/SLACK-QA-TOOL-[0-9A-F]{8}/u)?.[0], + input.match(/SLACK-QA-COMMENTARY-DONE-[0-9A-F]{8}/u)?.[0], + ]; + const verifyObserved = run && "verifyObserved" in run ? run.verifyObserved : undefined; + if (markers.some((marker) => !marker) || !verifyObserved) { + throw new Error(`missing Slack progress verifier: ${scenarioId}`); + } + const completeMarkers = markers as string[]; + return () => + verifyObserved({ + finalMessage: { + text: + finalText === "exact" + ? completeMarkers[2] + : `${completeMarkers[0]} ${completeMarkers[2]}`, + ts: "2.000000", + }, + messages: mutate(completeMarkers).map((text) => ({ + channelId: "C123456789", + text, + ts: "2.000000", + })), + }); + }; + + expect( + verify("slack-progress-commentary-false", ([commentary, , final]) => [commentary, final]), + ).toThrow("commentary to stay out"); + expect( + verify("slack-progress-commentary-true", ([commentary, tool, final]) => [ + commentary, + tool, + final, + ]), + ).toThrow("tool progress to stay out"); + expect( + verify("slack-progress-commentary-omitted", ([commentary, , final]) => [commentary, final]), + ).toThrow("tool progress on the progress draft"); + expect( + verify( + "slack-progress-commentary-true", + ([commentary, , final]) => [`${commentary} ${final}`], + "echo", + ), + ).toThrow("only the final marker"); + }); + + it("rejects duplicate durable and draft commentary identities", () => { + const scenario = testing.findScenario(["slack-progress-commentary-verbose-dedupe"])[0]; + const run = scenario?.buildRun("U999999999"); + const input = run && "input" in run ? run.input : ""; + const marker = input.match(/SLACK-QA-COMMENTARY-[0-9A-F]{8}/u)?.[0]; + const finalMarker = input.match(/SLACK-QA-COMMENTARY-DONE-[0-9A-F]{8}/u)?.[0]; + const verifyObserved = run && "verifyObserved" in run ? run.verifyObserved : undefined; + if (!marker || !finalMarker || !verifyObserved) { + throw new Error("missing Slack progress dedupe verifier"); + } + + expect(() => + verifyObserved({ + finalMessage: { text: finalMarker, ts: "2.000000" }, + messages: [ + { channelId: "C123456789", text: `💬 ${marker}`, ts: "1.500000" }, + { channelId: "C123456789", text: `• ${marker}`, ts: "2.000000" }, + ], + }), + ).toThrow("exactly one Slack message identity containing commentary"); + }); + + it("settles complete channel and thread observations after the final reply", async () => { + let historyCalls = 0; + const observedMessages: Array<{ text: string }> = []; + await testing.observeSlackScenarioMessages({ + channelId: "C123456789", + client: { + conversations: { + history: async () => { + historyCalls += 1; + return { + messages: + historyCalls === 1 + ? [ + { text: "FINAL_MARKER", ts: "3.000000", user: "U999999999" }, + { text: "EARLIER_COMMENTARY", ts: "2.000000", user: "U999999999" }, + ] + : [ + { text: "LATE_DUPLICATE", ts: "4.000000", user: "U999999999" }, + { text: "FINAL_MARKER", ts: "3.000000", user: "U999999999" }, + ], + }; + }, + replies: async () => ({ + messages: [{ text: "THREAD_DUPLICATE", ts: "5.000000", user: "U999999999" }], + }), + }, + } as never, + matchText: "FINAL_MARKER", + observedMessages: observedMessages as never, + observationScenarioId: "slack-progress-commentary-verbose-dedupe", + observationScenarioTitle: "Slack commentary dedupe", + sentTs: "1.000000", + settleMs: 10, + sutIdentity: { userId: "U999999999" }, + threadTs: "1.000000", + }); + + expect(historyCalls).toBeGreaterThanOrEqual(2); + expect(new Set(observedMessages.map((message) => message.text))).toEqual( + new Set(["FINAL_MARKER", "EARLIER_COMMENTARY", "LATE_DUPLICATE", "THREAD_DUPLICATE"]), + ); + }); + it("extracts typed Slack approval button values from blocks", () => { const actionValue = 'openclaw:approval:v1:{"approvalId":"plugin:abc","approvalKind":"plugin","decision":"allow-once"}'; diff --git a/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.ts b/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.ts index 5bb0b65d18fc..39f8453afdb3 100644 --- a/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.ts +++ b/extensions/qa-lab/src/live-transports/slack/slack-live.runtime.ts @@ -140,6 +140,10 @@ type SlackQaScenarioId = | "slack-codex-approval-plugin-native" | "slack-chart-presentation-native" | "slack-mention-gating" + | "slack-progress-commentary-false" + | "slack-progress-commentary-omitted" + | "slack-progress-commentary-true" + | "slack-progress-commentary-verbose-dedupe" | "slack-reaction-glyph-native" | "slack-table-invalid-blocks-fallback" | "slack-table-presentation-native" @@ -178,7 +182,12 @@ type SlackQaMessageScenarioRun = { expectReply: boolean; input: string; matchText: string; + settleObservedMs?: number; verify?: (message: SlackMessage, context: { requestThreadTs: string; sentTs: string }) => void; + verifyObserved?: (params: { + finalMessage: SlackMessage; + messages: readonly SlackObservedMessage[]; + }) => string | void; beforeRun?: (context: Omit) => Promise; afterReply?: (message: SlackMessage, context: SlackQaScenarioContext) => Promise; }; @@ -243,6 +252,11 @@ type SlackQaConfigOverrides = { }; codexApproval?: boolean; messageTool?: boolean; + progress?: { + commentary?: boolean; + toolProgress: boolean; + verboseDefault?: "off" | "on" | "full"; + }; replyToMode?: "all" | "off"; users?: string[]; }; @@ -516,6 +530,95 @@ function buildSlackInvalidBlocksTableProbe() { }; } +type SlackProgressCommentaryExpectation = { + commentary: "absent" | "draft" | "standalone"; + toolProgress: "absent" | "draft" | "standalone"; +}; + +function buildSlackProgressCommentaryRun( + sutUserId: string, + expectation: SlackProgressCommentaryExpectation, +): SlackQaMessageScenarioRun { + const suffix = randomUUID().slice(0, 8).toUpperCase(); + // Slack mrkdwn escapes underscores in progress drafts. Hyphenated markers + // stay byte-identical across draft edits and final-message reads. + const commentaryMarker = `SLACK-QA-COMMENTARY-${suffix}`; + const toolMarker = `SLACK-QA-TOOL-${suffix}`; + const finalMarker = `SLACK-QA-COMMENTARY-DONE-${suffix}`; + return { + expectReply: true, + input: [ + `<@${sutUserId}> This is a Slack progress protocol test. First, emit an assistant commentary message whose entire text is exactly ${commentaryMarker}.`, + "Do not call any tool until that commentary message is complete.", + `Then use the exec tool exactly once to run: grep '${toolMarker}' /dev/null || sleep 5.`, + `After the command finishes, reply with only this exact marker: ${finalMarker}`, + ].join(" "), + matchText: finalMarker, + settleObservedMs: 3_000, + verifyObserved: ({ finalMessage, messages }) => { + if (!finalMessage.ts) { + throw new Error("Slack progress commentary final message had no ts"); + } + if ((finalMessage.text ?? "").trim() !== finalMarker) { + throw new Error("expected the Slack final answer to contain only the final marker"); + } + const progressMessages = messages.filter((message) => !message.text.includes(finalMarker)); + const commentaryMessages = progressMessages.filter((message) => + message.text.includes(commentaryMarker), + ); + const commentaryTimestamps = new Set(commentaryMessages.map((message) => message.ts)); + if (expectation.commentary === "absent" && commentaryTimestamps.size !== 0) { + throw new Error("expected commentary to stay out of Slack progress messages"); + } + if (expectation.commentary !== "absent" && commentaryTimestamps.size !== 1) { + throw new Error( + `expected exactly one Slack message identity containing commentary; got ${commentaryTimestamps.size}`, + ); + } + const commentaryTs = [...commentaryTimestamps][0]; + if (expectation.commentary === "draft" && commentaryTs !== finalMessage.ts) { + throw new Error("expected commentary on the progress draft finalized as the answer"); + } + if (expectation.commentary === "standalone" && commentaryTs === finalMessage.ts) { + throw new Error("expected commentary only in the standalone verbose message"); + } + const toolTimestamps = new Set( + progressMessages + .filter((message) => message.text.includes(toolMarker)) + .map((message) => message.ts), + ); + if (expectation.toolProgress === "draft") { + if (toolTimestamps.size !== 1 || !toolTimestamps.has(finalMessage.ts)) { + throw new Error("expected tool progress on the progress draft finalized as the answer"); + } + } else if (expectation.toolProgress === "standalone") { + if (toolTimestamps.size === 0 || toolTimestamps.has(finalMessage.ts)) { + throw new Error("expected tool progress only in standalone verbose messages"); + } + } else if (toolTimestamps.size !== 0) { + throw new Error("expected tool progress to stay out of Slack progress messages"); + } + const finalTimestamps = new Set( + messages + .filter((message) => message.text.includes(finalMarker)) + .map((message) => message.ts), + ); + if (finalTimestamps.size !== 1 || !finalTimestamps.has(finalMessage.ts)) { + throw new Error( + "expected one final-marker Slack message identity matching the final answer", + ); + } + const commentaryDetails = + expectation.commentary === "draft" + ? "commentary on progress/final identity" + : expectation.commentary === "standalone" + ? "one standalone commentary identity" + : "commentary absent from Slack progress"; + return `verified ${commentaryDetails}; tool progress ${expectation.toolProgress}; final identity unique`; + }, + }; +} + const SLACK_QA_SCENARIOS: SlackQaScenarioDefinition[] = [ { id: "slack-canary", @@ -585,6 +688,62 @@ const SLACK_QA_SCENARIOS: SlackQaScenarioDefinition[] = [ }; }, }, + { + id: "slack-progress-commentary-true", + title: "Slack progress commentary true is independent from tool progress", + defaultEnabled: false, + timeoutMs: 90_000, + configOverrides: { + progress: { commentary: true, toolProgress: false }, + }, + buildRun: (sutUserId) => + buildSlackProgressCommentaryRun(sutUserId, { + commentary: "draft", + toolProgress: "absent", + }), + }, + { + id: "slack-progress-commentary-false", + title: "Slack progress commentary false stays out of the progress draft", + defaultEnabled: false, + timeoutMs: 90_000, + configOverrides: { + progress: { commentary: false, toolProgress: false }, + }, + buildRun: (sutUserId) => + buildSlackProgressCommentaryRun(sutUserId, { + commentary: "absent", + toolProgress: "absent", + }), + }, + { + id: "slack-progress-commentary-omitted", + title: "Slack omitted progress commentary preserves the tool-progress default", + defaultEnabled: false, + timeoutMs: 90_000, + configOverrides: { + progress: { toolProgress: true }, + }, + buildRun: (sutUserId) => + buildSlackProgressCommentaryRun(sutUserId, { + commentary: "draft", + toolProgress: "draft", + }), + }, + { + id: "slack-progress-commentary-verbose-dedupe", + title: "Slack explicit commentary yields to durable verbose progress", + defaultEnabled: false, + timeoutMs: 90_000, + configOverrides: { + progress: { commentary: true, toolProgress: false, verboseDefault: "on" }, + }, + buildRun: (sutUserId) => + buildSlackProgressCommentaryRun(sutUserId, { + commentary: "standalone", + toolProgress: "standalone", + }), + }, { id: "slack-chart-presentation-native", title: "Slack portable chart renders as a native data visualization", @@ -911,6 +1070,7 @@ function buildSlackQaConfig( }, ): OpenClawConfig { const codexApprovalConfig = params.overrides?.codexApproval === true; + const progressOverrides = params.overrides?.progress; const primaryModel = params.primaryModel; const pluginAllow = uniqueStrings([ ...(baseCfg.plugins?.allow ?? []), @@ -960,6 +1120,26 @@ function buildSlackQaConfig( }, } : baseCfg.agents?.defaults; + const qaAgentDefaults = progressOverrides + ? { + ...codexAgentDefaults, + ...(progressOverrides.verboseDefault + ? { verboseDefault: progressOverrides.verboseDefault } + : {}), + } + : codexAgentDefaults; + const qaAgentList = progressOverrides + ? baseCfg.agents?.list?.map((agent) => { + if (agent.id !== "qa") { + return agent; + } + // Slack draft edits cannot preserve custom authorship. Remove the + // synthetic QA identity so progress scenarios reach the draft path. + const qaAgent = { ...agent }; + delete qaAgent.identity; + return qaAgent; + }) + : baseCfg.agents?.list; const execApprovalsConfig = approvalOverrides ? { enabled: true, @@ -1017,11 +1197,12 @@ function buildSlackQaConfig( : {}), }, }, - ...(codexApprovalConfig + ...(codexApprovalConfig || progressOverrides ? { agents: { ...baseCfg.agents, - ...(codexAgentDefaults ? { defaults: codexAgentDefaults } : {}), + ...(qaAgentDefaults ? { defaults: qaAgentDefaults } : {}), + ...(qaAgentList ? { list: qaAgentList } : {}), }, } : {}), @@ -1047,6 +1228,21 @@ function buildSlackQaConfig( groupPolicy: "allowlist", allowBots: true, replyToMode: params.overrides?.replyToMode ?? "off", + ...(progressOverrides + ? { + streaming: { + mode: "progress" as const, + progress: { + label: false, + maxLines: 4, + toolProgress: progressOverrides.toolProgress, + ...(progressOverrides.commentary === undefined + ? {} + : { commentary: progressOverrides.commentary }), + }, + }, + } + : {}), ...(execApprovalsConfig ? { execApprovals: execApprovalsConfig } : {}), channels: { [params.channelId]: { @@ -1425,51 +1621,64 @@ async function runSlackTableInvalidBlocksFallbackScenario( }; } -async function waitForSlackScenarioReply(params: { +type SlackScenarioObservationContext = { channelId: string; - client: WebClient; matchText: string; observedMessages: SlackObservedMessage[]; observationScenarioId: string; observationScenarioTitle: string; sentTs: string; - threadTs?: string; sutIdentity: SlackAuthIdentity; - timeoutMs: number; -}) { +}; + +function recordSlackScenarioMessages( + params: SlackScenarioObservationContext & { messages: SlackMessage[] }, +) { + let matchedMessage: SlackMessage | undefined; + for (const message of params.messages) { + const text = message.text ?? ""; + if ( + !message.ts || + message.ts === params.sentTs || + !isSutSlackMessage(message, params.sutIdentity) + ) { + continue; + } + const matchedScenario = text.includes(params.matchText); + params.observedMessages.push({ + actionValues: collectSlackActionValues(message.blocks), + blockText: collectSlackBlockText(message.blocks), + botId: message.bot_id, + channelId: params.channelId, + matchedScenario, + scenarioId: params.observationScenarioId, + scenarioTitle: params.observationScenarioTitle, + text, + threadTs: message.thread_ts, + ts: message.ts, + userId: message.user, + }); + if (matchedScenario && !matchedMessage) { + matchedMessage = message; + } + } + return matchedMessage; +} + +async function waitForSlackScenarioReply( + params: SlackScenarioObservationContext & { + client: WebClient; + threadTs?: string; + timeoutMs: number; + }, +) { + const observationContext: SlackScenarioObservationContext = params; const startedAt = Date.now(); const inspectMessages = (messages: SlackMessage[]) => { - for (const message of messages) { - const text = message.text ?? ""; - if ( - !message.ts || - message.ts === params.sentTs || - !isSutSlackMessage(message, params.sutIdentity) - ) { - continue; - } - const matchedScenario = text.includes(params.matchText); - params.observedMessages.push({ - actionValues: collectSlackActionValues(message.blocks), - blockText: collectSlackBlockText(message.blocks), - botId: message.bot_id, - channelId: params.channelId, - matchedScenario, - scenarioId: params.observationScenarioId, - scenarioTitle: params.observationScenarioTitle, - text, - threadTs: message.thread_ts, - ts: message.ts, - userId: message.user, - }); - if (matchedScenario) { - return { - message, - observedAt: new Date().toISOString(), - }; - } - } - return undefined; + const matchedMessage = recordSlackScenarioMessages({ ...observationContext, messages }); + return matchedMessage + ? { message: matchedMessage, observedAt: new Date().toISOString() } + : undefined; }; while (Date.now() - startedAt < params.timeoutMs) { @@ -1506,6 +1715,50 @@ async function waitForSlackScenarioReply(params: { throw new Error(`timed out after ${params.timeoutMs}ms waiting for Slack message`); } +async function observeSlackScenarioMessages( + params: SlackScenarioObservationContext & { + client: WebClient; + settleMs: number; + threadTs?: string; + }, +) { + const observationContext: SlackScenarioObservationContext = params; + const startedAt = Date.now(); + + while (true) { + recordSlackScenarioMessages({ + ...observationContext, + messages: await listSlackMessages({ + channelId: params.channelId, + client: params.client, + oldestTs: params.sentTs, + }), + }); + try { + recordSlackScenarioMessages({ + ...observationContext, + messages: await listSlackThreadMessages({ + channelId: params.channelId, + client: params.client, + threadTs: params.threadTs ?? params.sentTs, + }), + }); + } catch (error) { + throw new Error( + `Slack conversations.replies failed while settling ${params.observationScenarioId}: ${formatErrorMessage(error)}`, + { cause: error }, + ); + } + const remainingMs = params.settleMs - (Date.now() - startedAt); + if (remainingMs <= 0) { + return; + } + await new Promise((resolve) => { + setTimeout(resolve, Math.min(1_000, remainingMs)); + }); + } +} + async function waitForSlackNoReply(params: { channelId: string; client: WebClient; @@ -3145,6 +3398,8 @@ export async function runSlackQaLive(params: { const beforeRunResult = await scenarioRun.beforeRun?.(baseScenarioContext); const beforeRunDetails = typeof beforeRunResult === "string" ? beforeRunResult : beforeRunResult?.details; + // Keep identity checks attempt-local so earlier scenario traffic cannot mask duplicates. + const observedMessageStartIndex = observedMessages.length; const requestStartedAt = new Date(); const sent = await sendSlackChannelMessage({ channelId: activeRuntimeEnv.channelId, @@ -3170,6 +3425,25 @@ export async function runSlackQaLive(params: { timeoutMs: scenario.timeoutMs, }); scenarioRun.verify?.(reply.message, { requestThreadTs, sentTs: sent.ts }); + if (scenarioRun.settleObservedMs) { + // Negative and dedupe checks need late Slack deliveries, not only the first final hit. + await observeSlackScenarioMessages({ + channelId: activeRuntimeEnv.channelId, + client: sutReadClient, + matchText: scenarioRun.matchText, + observedMessages, + observationScenarioId: scenario.id, + observationScenarioTitle: scenario.title, + sentTs: sent.ts, + settleMs: scenarioRun.settleObservedMs, + sutIdentity, + threadTs: requestThreadTs, + }); + } + const observedDetails = scenarioRun.verifyObserved?.({ + finalMessage: reply.message, + messages: observedMessages.slice(observedMessageStartIndex), + }); const responseObservedAt = new Date(reply.observedAt); const rttMs = responseObservedAt.getTime() - requestStartedAt.getTime(); const afterReplyDetails = await scenarioRun.afterReply?.(reply.message, { @@ -3184,6 +3458,7 @@ export async function runSlackQaLive(params: { details: [ `reply matched in ${rttMs}ms`, beforeRunDetails, + observedDetails, afterReplyDetails, scenarioAttempt > 1 ? `retried ${scenarioAttempt - 1}x` : undefined, ] @@ -3421,6 +3696,7 @@ export const testing = { isSlackChannelReadyForQa, matchesSlackApprovalResolvedUpdate, matchesSlackApprovalPromptText, + observeSlackScenarioMessages, parseSlackNativeApprovalAction, parseSlackQaCredentialPayload, preserveSlackGatewayDebugArtifacts, diff --git a/extensions/slack/src/draft-stream.test.ts b/extensions/slack/src/draft-stream.test.ts index adbac993ad55..3d41c5d1e4d2 100644 --- a/extensions/slack/src/draft-stream.test.ts +++ b/extensions/slack/src/draft-stream.test.ts @@ -32,6 +32,7 @@ function createDraftStreamHarness( maxChars?: number; send?: DraftSendFn; edit?: DraftEditFn; + eventScope?: DraftStreamParams["eventScope"]; remove?: DraftRemoveFn; warn?: DraftWarnFn; } = {}, @@ -46,6 +47,7 @@ function createDraftStreamHarness( token: "xoxb-test", throttleMs: 250, maxChars: params.maxChars, + eventScope: params.eventScope, send, edit, remove, @@ -72,6 +74,37 @@ describe("createSlackDraftStream", () => { }); }); + it("uses the enterprise event client for draft writes", async () => { + const client = {} as NonNullable["client"]; + const eventScope = { + apiAppId: "A_TEST", + enterpriseId: "E_TEST", + isEnterpriseInstall: true as const, + teamId: "T_TEST", + client, + }; + const { stream, send, edit, remove } = createDraftStreamHarness({ eventScope }); + + stream.update("hello"); + await stream.flush(); + stream.update("hello world"); + await stream.flush(); + await stream.clear(); + + expect(send).toHaveBeenCalledWith( + "channel:C123", + "hello", + expect.objectContaining({ client, enterpriseEventScope: eventScope }), + ); + expect(edit).toHaveBeenCalledWith( + "C123", + "111.222", + "hello world", + expect.objectContaining({ client }), + ); + expect(remove).toHaveBeenCalledWith("C123", "111.222", expect.objectContaining({ client })); + }); + it("sends and edits rich draft blocks with text fallback", async () => { const { stream, send, edit } = createDraftStreamHarness(); const blocks = [{ type: "divider" }] as const; diff --git a/extensions/slack/src/draft-stream.ts b/extensions/slack/src/draft-stream.ts index b3843c12e3da..2bac853e5722 100644 --- a/extensions/slack/src/draft-stream.ts +++ b/extensions/slack/src/draft-stream.ts @@ -6,6 +6,7 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { deleteSlackMessage, editSlackMessage } from "./actions.js"; import { formatSlackError } from "./errors.js"; import { SLACK_TEXT_LIMIT } from "./limits.js"; +import type { SlackEventScope } from "./monitor/event-scope.js"; import type { SlackSendIdentity } from "./send.js"; import { sendMessageSlack } from "./send.js"; @@ -35,6 +36,7 @@ export function createSlackDraftStream(params: { cfg: OpenClawConfig; token: string; accountId?: string; + eventScope?: SlackEventScope; identity?: SlackSendIdentity; maxChars?: number; throttleMs?: number; @@ -88,6 +90,7 @@ export function createSlackDraftStream(params: { cfg: params.cfg, token: params.token, accountId: params.accountId, + ...(params.eventScope ? { client: params.eventScope.client } : {}), ...(blocks ? { blocks } : {}), }); return; @@ -98,6 +101,9 @@ export function createSlackDraftStream(params: { accountId: params.accountId, threadTs: params.resolveThreadTs?.(), identity: params.identity, + ...(params.eventScope + ? { client: params.eventScope.client, enterpriseEventScope: params.eventScope } + : {}), ...(params.metadata ? { metadata: params.metadata } : {}), ...(blocks ? { blocks } : {}), }); @@ -145,6 +151,7 @@ export function createSlackDraftStream(params: { await remove(channelId, messageId, { token: params.token, accountId: params.accountId, + ...(params.eventScope ? { client: params.eventScope.client } : {}), }); } catch (err) { params.warn?.(`slack stream preview cleanup failed: ${formatSlackError(err)}`); 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 b2ad2136431f..7b94be57b411 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 @@ -48,6 +48,8 @@ let capturedReplyOptions: | { disableBlockStreaming?: boolean; suppressDefaultToolProgressMessages?: boolean; + commentaryProgressEnabled?: boolean; + onVerboseProgressVisibility?: (isActive: () => boolean) => void; allowProgressCallbacksWhenSourceDeliverySuppressed?: boolean; allowToolLifecycleWhenProgressHidden?: boolean; onAssistantMessageStart?: () => Promise | void; @@ -3296,6 +3298,210 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { await requireCapturedItemEventHandler()({ progressText: "hidden progress" }); }); + it("keeps only the latest Slack commentary when tool progress is disabled", async () => { + const draftStream = createDraftStreamStub(); + createSlackDraftStreamMock.mockReturnValueOnce(draftStream); + mockedSlackStreamingMode = "progress"; + mockedSlackDraftMode = "status_final"; + mockedDispatchSequence = []; + mockedReplyOptionEvents = [ + { + kind: "tool_start", + itemId: "tool-1", + name: "bash", + phase: "start", + args: { command: "pnpm test" }, + }, + { + kind: "item", + itemKind: "preamble", + itemId: "preamble-1", + progressText: "Checking the Slack event path", + }, + { + kind: "item", + itemKind: "preamble", + itemId: "preamble-2", + progressText: "Preparing the smallest fix", + }, + ]; + + await dispatchPreparedSlackMessage( + createPreparedSlackMessage({ + accountConfig: { + streaming: { + mode: "progress", + progress: { label: false, commentary: true, toolProgress: false, maxLines: 1 }, + }, + }, + }), + ); + + expect(capturedReplyOptions?.commentaryProgressEnabled).toBe(true); + expect(capturedReplyOptions?.suppressDefaultToolProgressMessages).toBe(true); + 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; + capturedReplyOptions?.onVerboseProgressVisibility?.(() => true); + await requireCapturedItemEventHandler()({ + kind: "preamble", + itemId: "preamble-3", + progressText: "Delivered by the verbose lane", + }); + expect(draftStream.update).toHaveBeenCalledTimes(updateCount); + }); + + it("uses the enterprise event client for Slack commentary drafts", async () => { + const draftStream = createDraftStreamStub(); + createSlackDraftStreamMock.mockReturnValueOnce(draftStream); + mockedSlackStreamingMode = "progress"; + mockedSlackDraftMode = "status_final"; + mockedDispatchSequence = []; + mockedReplyOptionEvents = [ + { + kind: "item", + itemKind: "preamble", + itemId: "preamble-1", + progressText: "Checking the Enterprise event path", + }, + { + kind: "item", + itemKind: "preamble", + itemId: "preamble-2", + progressText: "Using the scoped listener client", + }, + ]; + const eventClient = { + chat: { postMessage: postMessageMock, update: chatUpdateMock }, + }; + const eventScope = { + apiAppId: "A_TEST", + enterpriseId: "E_TEST", + isEnterpriseInstall: true as const, + teamId: "T_ENTERPRISE", + client: eventClient, + }; + + await dispatchPreparedSlackMessage( + createPreparedSlackMessage({ + accountConfig: { + enterpriseOrgInstall: true, + streaming: { + mode: "progress", + progress: { label: false, commentary: true, toolProgress: false, maxLines: 1 }, + }, + }, + eventScope, + }), + ); + + expect(createSlackDraftStreamMock).toHaveBeenCalledWith( + expect.objectContaining({ eventScope }), + ); + expect(draftStream.update).toHaveBeenLastCalledWith("• Using the scoped listener client"); + }); + + it("preserves legacy Slack preambles when commentary is omitted", async () => { + const draftStream = createDraftStreamStub(); + createSlackDraftStreamMock.mockReturnValueOnce(draftStream); + mockedSlackStreamingMode = "progress"; + mockedSlackDraftMode = "status_final"; + mockedDispatchSequence = []; + mockedReplyOptionEvents = [ + { + kind: "item", + itemKind: "preamble", + itemId: "preamble-1", + progressText: "Checking the legacy Slack path", + }, + { + kind: "item", + itemKind: "preamble", + itemId: "preamble-2", + progressText: "Keeping the released behavior", + }, + ]; + + await dispatchPreparedSlackMessage( + createPreparedSlackMessage({ + accountConfig: { + streaming: { mode: "progress", progress: { label: false, maxLines: 1 } }, + }, + }), + ); + + expect(capturedReplyOptions?.commentaryProgressEnabled).toBeUndefined(); + expect(capturedReplyOptions?.onVerboseProgressVisibility).toBeUndefined(); + expect(draftStream.update).toHaveBeenLastCalledWith("• Keeping the released behavior"); + }); + + it("preserves Slack preamble previews outside progress mode", async () => { + const draftStream = createDraftStreamStub(); + createSlackDraftStreamMock.mockReturnValueOnce(draftStream); + mockedSlackStreamingMode = "partial"; + mockedSlackDraftMode = "replace"; + mockedDispatchSequence = []; + mockedReplyOptionEvents = [ + { + kind: "item", + itemKind: "preamble", + itemId: "preamble-1", + progressText: "Keeping the partial preview path", + }, + ]; + + await dispatchPreparedSlackMessage( + createPreparedSlackMessage({ + accountConfig: { + streaming: { mode: "partial", progress: { label: false } }, + }, + }), + ); + + expect(capturedReplyOptions?.commentaryProgressEnabled).toBeUndefined(); + expect(draftStream.update).toHaveBeenLastCalledWith("• Keeping the partial preview path"); + }); + + it("lets Slack hide commentary without hiding tool progress", async () => { + const draftStream = createDraftStreamStub(); + createSlackDraftStreamMock.mockReturnValueOnce(draftStream); + mockedSlackStreamingMode = "progress"; + mockedSlackDraftMode = "status_final"; + mockedDispatchSequence = []; + mockedReplyOptionEvents = [ + { + kind: "tool_start", + itemId: "tool-1", + name: "bash", + phase: "start", + args: { command: "pnpm test" }, + }, + { + kind: "item", + itemKind: "preamble", + itemId: "preamble-1", + progressText: "Hidden commentary", + }, + ]; + + await dispatchPreparedSlackMessage( + createPreparedSlackMessage({ + accountConfig: { + streaming: { + mode: "progress", + progress: { label: false, commentary: false, toolProgress: true }, + }, + }, + }), + ); + + const updates = draftStream.update.mock.calls.flat().join("\n"); + expect(capturedReplyOptions?.commentaryProgressEnabled).toBeUndefined(); + expect(updates).toContain("pnpm test"); + expect(updates).not.toContain("Hidden commentary"); + }); + it("does not create a blank Slack progress draft when label and lines are disabled", async () => { const draftStream = createDraftStreamStub(); createSlackDraftStreamMock.mockReturnValueOnce(draftStream); diff --git a/extensions/slack/src/monitor/message-handler/dispatch.ts b/extensions/slack/src/monitor/message-handler/dispatch.ts index 1aa2aa3d9181..cce570481b0c 100644 --- a/extensions/slack/src/monitor/message-handler/dispatch.ts +++ b/extensions/slack/src/monitor/message-handler/dispatch.ts @@ -37,6 +37,7 @@ import { resolveChannelStreamingBlockEnabled, resolveChannelStreamingNativeTransport, resolveChannelStreamingPreviewToolProgress, + resolveChannelStreamingProgressCommentary, resolveChannelStreamingSuppressDefaultToolProgressMessages, type ChannelProgressDraftLine, } from "openclaw/plugin-sdk/channel-outbound"; @@ -694,7 +695,6 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag // chat.update cannot preserve custom authorship. Use native streaming when // possible; otherwise keep identity intact with one final postMessage. const shouldUseDraftStream = - !prepared.eventScope && !hasSlackCustomIdentity && shouldInitializeSlackDraftStream({ previewStreamingEnabled, @@ -1491,6 +1491,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag cfg, token: ctx.botToken, accountId: account.accountId, + ...(prepared.eventScope ? { eventScope: prepared.eventScope } : {}), identity: slackIdentity, ...(slackMessageMetadata ? { metadata: slackMessageMetadata } : {}), maxChars: Math.min(ctx.textLimit, SLACK_TEXT_LIMIT), @@ -1508,9 +1509,19 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag let hasStreamedMessage = false; const streamMode = slackStreaming.draftMode; const useNativeProgressStreaming = useStreaming && slackStreaming.mode === "progress"; + const progressDraftActive = Boolean(draftStream) || useNativeProgressStreaming; const previewToolProgressEnabled = - (Boolean(draftStream) || useNativeProgressStreaming) && - resolveChannelStreamingPreviewToolProgress(account.config); + progressDraftActive && resolveChannelStreamingPreviewToolProgress(account.config); + const commentaryProgressEnabled = + progressDraftActive && resolveChannelStreamingProgressCommentary(account.config); + // Slack shipped Codex preambles through toolProgress before commentary gained + // an independent switch. Omitted commentary preserves that released behavior. + const commentaryDraftEnabled = + progressDraftActive && + (slackStreaming.mode === "progress" + ? resolveChannelStreamingProgressCommentary(account.config, previewToolProgressEnabled) + : previewToolProgressEnabled); + let shouldYieldDraftProgress: () => boolean = () => false; const suppressDefaultToolProgressMessages = resolveChannelStreamingSuppressDefaultToolProgressMessages(account.config, { draftStreamActive: Boolean(draftStream) || useNativeProgressStreaming, @@ -1707,13 +1718,19 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag } }; - const pushPreviewToolProgress = async ( + const pushPreviewProgress = async ( line?: ChannelProgressDraftLine, - options?: { toolName?: string }, + options?: { toolName?: string; lane?: "tool" | "commentary" }, ) => { + const lane = options?.lane ?? "tool"; + const progressEnabled = + lane === "commentary" ? commentaryDraftEnabled : previewToolProgressEnabled; if (!draftStream && !useNativeProgressStreaming) { return; } + if (lane === "commentary" && commentaryProgressEnabled && shouldYieldDraftProgress()) { + return; + } if (options?.toolName !== undefined && !isChannelProgressDraftWorkToolName(options.toolName)) { return; } @@ -1730,7 +1747,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag return; } if (streamMode !== "status_final") { - if (!previewToolProgressEnabled || previewToolProgressSuppressed) { + if (!progressEnabled || previewToolProgressSuppressed) { return; } const nextLines = mergeChannelProgressDraftLine(previewToolProgressLines, line, { @@ -1751,7 +1768,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag hasStreamedMessage = true; return; } - if (previewToolProgressEnabled && !previewToolProgressSuppressed) { + if (progressEnabled && !previewToolProgressSuppressed) { previewToolProgressLines = mergeChannelProgressDraftLine(previewToolProgressLines, line, { maxLines: resolveChannelProgressDraftMaxLines(account.config), }); @@ -1834,7 +1851,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag if (!normalized) { return; } - await pushPreviewToolProgress({ + await pushPreviewProgress({ id: "reasoning", kind: "item", text: normalized, @@ -1891,6 +1908,12 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag disableBlockStreaming, onModelSelected, suppressDefaultToolProgressMessages: suppressDefaultToolProgressMessages ? true : undefined, + commentaryProgressEnabled: commentaryProgressEnabled ? true : undefined, + onVerboseProgressVisibility: commentaryProgressEnabled + ? (isActive) => { + shouldYieldDraftProgress = isActive; + } + : undefined, allowProgressCallbacksWhenSourceDeliverySuppressed: sourceReplyDeliveryMode === "message_tool_only" && statusReactionsEnabled ? true @@ -1919,7 +1942,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag if (statusReactionsEnabled) { await statusReactions.setTool(payload.name); } - await pushPreviewToolProgress( + await pushPreviewProgress( buildChannelProgressDraftLineForEntry( account.config, { @@ -1936,7 +1959,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag ); }, onItemEvent: async (payload) => { - await pushPreviewToolProgress( + await pushPreviewProgress( buildChannelProgressDraftLineForEntry(account.config, { event: "item", itemId: payload.itemId, @@ -1950,13 +1973,14 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag progressText: payload.progressText, meta: payload.meta, }), + payload.kind === "preamble" ? { lane: "commentary" } : undefined, ); }, onPlanUpdate: async (payload) => { if (payload.phase !== "update") { return; } - await pushPreviewToolProgress( + await pushPreviewProgress( buildChannelProgressDraftLine({ event: "plan", phase: payload.phase, @@ -1970,7 +1994,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag if (payload.phase !== "requested") { return; } - await pushPreviewToolProgress( + await pushPreviewProgress( buildChannelProgressDraftLine({ event: "approval", phase: payload.phase, @@ -1985,7 +2009,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag if (payload.phase !== "end") { return; } - await pushPreviewToolProgress( + await pushPreviewProgress( buildChannelProgressDraftLine({ event: "command-output", itemId: payload.itemId, @@ -2002,7 +2026,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag if (payload.phase !== "end") { return; } - await pushPreviewToolProgress( + await pushPreviewProgress( buildChannelProgressDraftLine({ event: "patch", itemId: payload.itemId,