diff --git a/app/src/components/approvals/inline-approval.tsx b/app/src/components/approvals/inline-approval.tsx index aa52504aec..5d7c2575f1 100644 --- a/app/src/components/approvals/inline-approval.tsx +++ b/app/src/components/approvals/inline-approval.tsx @@ -149,6 +149,24 @@ export function InlineApproval({ ) : null} {pending ? null : fallback} + {/* + The conversation could not be carried on after this was decided, so the step above never got + its result and would say "Send message…" for good. What came of it is said here, from the + request itself, which is the one place that still knows. + */} + {request?.left ? ( +

+ {request.left.message} +

+ ) : null} ); } @@ -189,9 +207,13 @@ export function PendingApprovals({ toolCallIds: readonly string[]; }) { const inbox = useQuery(approvalInboxOptions()); + /* + * And one whose conversation could not be carried on after it was decided (`left`): what came of + * it is said beside the group too, since the folded line ("Used Slack") says nothing of it. + */ const waiting = new Set( (inbox.data?.requests ?? []) - .filter((request) => request.status === "pending") + .filter((request) => request.status === "pending" || request.left) .flatMap((request) => request.action.toolCallId ? [request.action.toolCallId] : [], ), diff --git a/app/src/components/channels/channel-chat.tsx b/app/src/components/channels/channel-chat.tsx index 77b57a8ef6..2891b618c5 100644 --- a/app/src/components/channels/channel-chat.tsx +++ b/app/src/components/channels/channel-chat.tsx @@ -44,6 +44,7 @@ import { answerPersonQuestionMutationOptions, approvalInboxOptions, carryingOnAfterApproval, + leftAnswers, } from "@/lib/approvals"; import { attachmentUrl } from "@/lib/channels/attachments"; import { @@ -1267,7 +1268,11 @@ export function ChannelChat({ report(trimmed || describeAttachments(attachments), null); // Providers reject later turns if prior tool calls have no result; repair before sending. - const repaired = repairUnansweredToolCalls(target.messages); + const repaired = repairUnansweredToolCalls( + target.messages, + undefined, + leftAnswers(queryClient.getQueryData(approvalInboxOptions().queryKey)), + ); if (repaired !== target.messages) { target.setMessages(repaired as typeof target.messages); } diff --git a/app/src/lib/approvals.ts b/app/src/lib/approvals.ts index 7750d1a5d1..f4bf8709be 100644 --- a/app/src/lib/approvals.ts +++ b/app/src/lib/approvals.ts @@ -67,6 +67,11 @@ export type ApprovalInboxData = { decidedAt?: string | null; /** What the person answered; null while it waits. */ decision?: ApprovalDecision | null; + /** + * Set when the conversation could not be carried on after this was decided: whether the action + * was done, the sentence that says so, and the answer the Bot never received. + */ + left?: { done: boolean; message: string; answer: string }; action: { botId: string; toolRef: string; @@ -121,6 +126,21 @@ export type ApprovalInboxData = { createdAt: string; }[]; }; +/** + * The answers the Bot never received, by the call each belongs to: actions decided whose + * conversation could not be carried on (`left`). Sent with the next message in place of "no result + * came back", so the Bot knows a post it asked for went out and does not make it again. + */ +export function leftAnswers( + inbox: ApprovalInboxData | undefined, +): ReadonlyMap { + const answers = new Map(); + for (const request of inbox?.requests ?? []) + if (request.left && request.action.toolCallId) + answers.set(request.action.toolCallId, request.left.answer); + return answers; +} + export const approvalInboxOptions = () => queryOptions({ queryKey: ["approvals"], diff --git a/app/src/lib/copilot/repair-history.ts b/app/src/lib/copilot/repair-history.ts index 02093dca40..8f37f4413c 100644 --- a/app/src/lib/copilot/repair-history.ts +++ b/app/src/lib/copilot/repair-history.ts @@ -129,6 +129,11 @@ export function repairUnansweredToolCalls( * repair branch, which every test avoided by passing its own. Biome said so, as an unused import. */ mintId: () => string = newId, + /** + * Answers known from somewhere other than the thread, by call id: an approved action whose + * conversation could not be carried on keeps its result on the request (`leftAnswers`). + */ + known: ReadonlyMap = new Map(), ): ReadonlyArray { /* * A held step's stand-in is not its answer. Counted as one, it sat right after the call, so the @@ -188,7 +193,7 @@ export function repairUnansweredToolCalls( id: mintId(), role: "tool", toolCallId: call.id, - content: UNANSWERED, + content: known.get(call.id) || UNANSWERED, } as Message), ); // A duplicated call id may only receive one repair result. diff --git a/app/tests/approval-left-not-carried-on.test.tsx b/app/tests/approval-left-not-carried-on.test.tsx new file mode 100644 index 0000000000..d9e1df5961 --- /dev/null +++ b/app/tests/approval-left-not-carried-on.test.tsx @@ -0,0 +1,164 @@ +import { afterAll, afterEach, beforeAll, expect, test } from "bun:test"; +import type { Message } from "@ag-ui/core"; +import { GlobalRegistrator } from "@happy-dom/global-registrator"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { cleanup, render, waitFor } from "@testing-library/react"; +import { + InlineApproval, + PendingApprovals, +} from "@/components/approvals/inline-approval"; +import { type ApprovalInboxData, leftAnswers } from "@/lib/approvals"; +import { + repairUnansweredToolCalls, + UNANSWERED, +} from "@/lib/copilot/repair-history"; +import { settleReactWork } from "./settle-react-work"; + +/* + * FOUND IN TESTING. A person allowed a Slack post, the post went out, and the conversation could + * not be carried on after it. The step sat on "Send message…" with nothing said, so the person + * never learned the post was made, and the next message sent the Bot "no result came back" for a + * call that had in fact run, which invites it to post again. + * + * The request now says what came of it (`left`). The step draws that sentence, and the next message + * sends the Bot the answer the conversation never received. + */ + +const originalFetch = global.fetch; +beforeAll(() => { + GlobalRegistrator.register(); +}); +afterEach(() => { + cleanup(); + global.fetch = originalFetch; +}); +afterAll(async () => { + await settleReactWork(); + GlobalRegistrator.unregister(); +}); + +const DONE = + "Done: send message on Slack. OpenBot couldn't carry on the conversation after that, so nothing else was done. Send a message to continue."; +const ANSWER = + 'Ran: the app made this change. It answered: {"ok":true,"channel":"C0LAUNCH01"}'; + +/** `GET /api/approvals`, with one request whose conversation was never carried on. */ +function inbox(left: { done: boolean; message: string; answer: string }) { + return { + enabled: true, + requests: [ + { + id: "approval-1", + status: "completed", + createdAt: "2026-11-14T04:06:40Z", + decidedAt: "2026-11-14T04:06:47Z", + decision: "allow_once", + left, + action: { + botId: "personal-assistant", + toolRef: "composio-slack/SLACK_SEND_MESSAGE", + effect: "write", + scope: "composio-slack", + threadId: "thread-1", + toolCallId: "call-send", + args: { channel: "C0LAUNCH01", markdown_text: "the deck is ready" }, + target: null, + }, + }, + ], + rules: [], + teamRules: [], + questions: [], + } as unknown as ApprovalInboxData; +} + +function drawn(data: ApprovalInboxData) { + global.fetch = (async () => Response.json(data)) as unknown as typeof fetch; + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + return render( + + Send message…

} toolCallId="call-send" /> +
, + ); +} + +test("a step whose conversation could not be carried on says the action was done and what to do next", async () => { + const view = drawn(inbox({ done: true, message: DONE, answer: ANSWER })); + const told = await waitFor(() => view.getByTestId("approval-left")); + expect(told.textContent).toBe(DONE); + // Said as a status under the step, which is still drawn: nothing replaces the step itself. + expect(told.getAttribute("role")).toBe("status"); + expect(view.getByText("Send message…")).toBeTruthy(); +}); + +test("a step folded into its group says so beside the group, where a waiting card would be", async () => { + global.fetch = (async () => + Response.json( + inbox({ done: true, message: DONE, answer: ANSWER }), + )) as unknown as typeof fetch; + const client = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + const view = render( + + + , + ); + const told = await waitFor(() => view.getByTestId("approval-left")); + expect(told.textContent).toBe(DONE); +}); + +test("one that was not done says so as an alert", async () => { + const message = + "Not done: send message on Slack. OpenBot couldn't carry on the conversation. Send a message to try again."; + const view = drawn(inbox({ done: false, message, answer: "Not done." })); + const told = await waitFor(() => view.getByTestId("approval-left")); + expect(told.textContent).toBe(message); + expect(told.getAttribute("role")).toBe("alert"); +}); + +test("the next message sends the Bot the answer it never received, not that nothing came back", () => { + const history = [ + { id: "u1", role: "user", content: "Post 'the deck is ready' in #launch." }, + { + id: "a1", + role: "assistant", + content: "", + toolCalls: [ + { + id: "call-send", + type: "function", + function: { name: "SLACK_SEND_MESSAGE", arguments: "{}" }, + }, + { + id: "call-other", + type: "function", + function: { name: "SLACK_FIND_USERS", arguments: "{}" }, + }, + ], + }, + ] as unknown as Message[]; + const known = leftAnswers( + inbox({ done: true, message: DONE, answer: ANSWER }), + ); + const repaired = repairUnansweredToolCalls(history, () => "minted", known); + const results = repaired.filter((message) => message.role === "tool") as { + toolCallId: string; + content: string; + }[]; + expect(results.map((result) => [result.toolCallId, result.content])).toEqual([ + ["call-send", ANSWER], + // A call nothing is known about keeps the words it always had. + ["call-other", UNANSWERED], + ]); + // With nothing known, the same history is repaired as before. + expect( + ( + repairUnansweredToolCalls(history, () => "minted").filter( + (message) => message.role === "tool", + ) as { content: string }[] + ).map((result) => result.content), + ).toEqual([UNANSWERED, UNANSWERED]); +});