From fd5b0269817506ca20899a02899d84254cf3d1b2 Mon Sep 17 00:00:00 2001 From: Manu MA Date: Mon, 10 Aug 2026 11:51:53 +0200 Subject: [PATCH 1/2] feat: implement endsTurn behavior for ask-question action and enhance guided question handling --- .../core/src/agent/production-agent.spec.ts | 131 ++++++++++++++++++ packages/core/src/agent/production-agent.ts | 68 +++++++++ packages/core/src/client/AssistantChat.tsx | 4 +- .../src/client/guided-questions.flow.spec.tsx | 73 ++++++++++ packages/core/src/client/guided-questions.tsx | 62 +++++++-- .../src/server/agent-chat/context-tools.ts | 23 ++- 6 files changed, 345 insertions(+), 16 deletions(-) diff --git a/packages/core/src/agent/production-agent.spec.ts b/packages/core/src/agent/production-agent.spec.ts index d64d50b490..b7cdecc374 100644 --- a/packages/core/src/agent/production-agent.spec.ts +++ b/packages/core/src/agent/production-agent.spec.ts @@ -9591,6 +9591,137 @@ describe("runAgentLoop", () => { }); }); +// ─── endsTurn (actions that hand control back to the user) ─────────────────── + +describe("runAgentLoop endsTurn", () => { + /** + * Emits `ask-question` plus a second tool call in ONE assistant message, then + * a plain text completion on every later stream. The extra call reproduces the + * reported "it keeps asking questions": a second `ask-question` overwrites the + * first card before anyone can answer it. + */ + const yieldEngine = (): { + engine: AgentEngine; + streamCalls: () => number; + } => { + let streamCalls = 0; + const engine: AgentEngine = { + name: "test", + label: "Test", + defaultModel: "test-model", + supportedModels: ["test-model"], + capabilities: { + thinking: false, + promptCaching: false, + vision: false, + computerUse: false, + parallelToolCalls: true, + }, + async *stream(): AsyncIterable { + streamCalls += 1; + if (streamCalls === 1) { + yield { + type: "assistant-content", + parts: [ + { + type: "tool-call" as const, + id: "ask-1", + name: "ask-question", + input: { question: "Which range?" }, + }, + { + type: "tool-call" as const, + id: "ask-2", + name: "ask-question", + input: { question: "Which grain?" }, + }, + ], + }; + yield { type: "stop", reason: "tool_use" }; + return; + } + yield { type: "text-delta", text: "kept working" }; + yield { + type: "assistant-content", + parts: [{ type: "text" as const, text: "kept working" }], + }; + yield { type: "stop", reason: "end_turn" }; + }, + }; + return { engine, streamCalls: () => streamCalls }; + }; + + it("stops the turn after the action runs and skips later calls in the same message", async () => { + const { engine, streamCalls } = yieldEngine(); + const run = vi.fn(async () => "asked"); + const events: any[] = []; + const outcomes: AgentLoopOutcome[] = []; + + await runAgentLoop({ + engine, + model: "test-model", + systemPrompt: "system", + tools: [], + messages: [{ role: "user", content: [{ type: "text", text: "go" }] }], + actions: { + "ask-question": { + ...actionEntry({ readOnly: false }), + endsTurn: true, + run, + }, + }, + send: (event) => events.push(event), + onOutcome: (outcome) => outcomes.push(outcome), + signal: new AbortController().signal, + }); + + // The first question ran; the second never did. + expect(run).toHaveBeenCalledOnce(); + expect(events).toContainEqual( + expect.objectContaining({ + type: "tool_done", + id: "ask-2", + result: expect.stringContaining("Not executed"), + }), + ); + // The model was never asked for another step. + expect(streamCalls()).toBe(1); + expect(events.some((event) => event.type === "done")).toBe(false); + expect(events.some((event) => event.text === "kept working")).toBe(false); + expect(outcomes).toEqual([ + { + state: "input_required", + code: "awaiting_user_input", + message: "Waiting for your answer before continuing.", + }, + ]); + }); + + it("leaves a turn running when the action is not marked endsTurn", async () => { + const { engine, streamCalls } = yieldEngine(); + const run = vi.fn(async () => "asked"); + const outcomes: AgentLoopOutcome[] = []; + + await runAgentLoop({ + engine, + model: "test-model", + systemPrompt: "system", + tools: [], + messages: [{ role: "user", content: [{ type: "text", text: "go" }] }], + actions: { + "ask-question": { ...actionEntry({ readOnly: false }), run }, + }, + send: () => {}, + onOutcome: (outcome) => outcomes.push(outcome), + signal: new AbortController().signal, + }); + + expect(run).toHaveBeenCalledTimes(2); + expect(streamCalls()).toBe(2); + expect(outcomes).toEqual([{ state: "completed" }]); + }); +}); + // ─── isContextTooLongError ──────────────────────────────────────────────────── describe("isContextTooLongError", () => { diff --git a/packages/core/src/agent/production-agent.ts b/packages/core/src/agent/production-agent.ts index 69191ef603..31bdbc6863 100644 --- a/packages/core/src/agent/production-agent.ts +++ b/packages/core/src/agent/production-agent.ts @@ -733,6 +733,14 @@ export interface ActionEntry { args: any, ctx?: import("../action.js").ActionRunContext, ) => boolean | Promise); + /** + * The action hands control back to the user: once it succeeds the loop stops + * the turn instead of asking the model for another step, and any remaining + * tool calls in the same assistant message do not execute. Only for actions + * whose whole purpose is to wait on a human (`ask-question`) — telling the + * model to stop in the tool result does not make it stop. + */ + endsTurn?: boolean; /** Which framework tool group contributed this action. Set by the framework, * never by an app: apps own their action names, and a tagged action is one * the app can switch off wholesale through `frameworkTools`. Tagged actions @@ -5153,6 +5161,10 @@ export async function runAgentLoop(opts: { let requestedActionStop: { message: string; errorCode?: string } | null = null; + // An `endsTurn` action ran and handed control to the user. Distinct from + // `requestedActionStop`, which also covers failure stops that must not + // suppress the remaining tool calls. + let turnYieldedToUser = false; const noteRepeatedToolCall = (toolName: string, input: unknown) => { const key = toolCallCacheKey(toolName, input); @@ -6074,6 +6086,13 @@ export async function runAgentLoop(opts: { ...(actionEntry.chatUI ? { chatUI: actionEntry.chatUI } : {}), }); recordToolResult(result, isError); + if (!isError && actionEntry.endsTurn === true) { + turnYieldedToUser = true; + requestedActionStop ??= { + message: "Waiting for your answer before continuing.", + errorCode: "awaiting-user-input", + }; + } if (!isError) { if (cacheKey) { readOnlyToolResultCache.set(cacheKey, result); @@ -6130,7 +6149,50 @@ export async function runAgentLoop(opts: { toolResultParts.push(...(await Promise.all(batch.map(runToolCall)))); }; + // An `endsTurn` action already handed control to the user, so the rest of + // this assistant message belongs to a turn that is over. Report those calls + // as not executed rather than running them: a second `ask-question` would + // overwrite the first one's card before anyone could answer it. + const skipToolCallAfterYield = ( + toolCall: import("./engine/types.js").EngineToolCallPart, + ): EngineContentPart => { + const result = + `Not executed: ${toolCall.name} was called after an action that ends the turn. ` + + `The turn is paused for the user's answer — call it again on a later turn if still needed.`; + send({ + type: "tool_start", + id: toolCall.id, + tool: toolCall.name, + input: toolCall.input as Record, + }); + send({ + type: "tool_done", + id: toolCall.id, + tool: toolCall.name, + input: toolCall.input as Record, + result, + completedSideEffect: false, + }); + toolResultHistory.push({ + name: toolCall.name, + content: result, + isError: false, + }); + return { + type: "tool-result" as const, + toolCallId: toolCall.id, + toolName: toolCall.name, + toolInput: JSON.stringify(toolCall.input ?? {}), + content: result, + }; + }; + for (const toolCall of toolCallParts) { + if (turnYieldedToUser) { + await flushParallelBatch(); + toolResultParts.push(skipToolCallAfterYield(toolCall)); + continue; + } const batchKind = getParallelBatchKind(toolCall); if (batchKind) { if (parallelBatchKind && parallelBatchKind !== batchKind) { @@ -6256,6 +6318,12 @@ export async function runAgentLoop(opts: { code: "needs_approval", message: terminalActionStop.message, }); + } else if (terminalActionStop?.errorCode === "awaiting-user-input") { + reportOutcome({ + state: "input_required", + code: "awaiting_user_input", + message: terminalActionStop.message, + }); } else if (terminalActionStop) { reportOutcome({ state: "failed", diff --git a/packages/core/src/client/AssistantChat.tsx b/packages/core/src/client/AssistantChat.tsx index aa377db885..84d6bed796 100644 --- a/packages/core/src/client/AssistantChat.tsx +++ b/packages/core/src/client/AssistantChat.tsx @@ -5352,7 +5352,8 @@ const AssistantChatInner = forwardRef< // GuidedQuestionPayload to application_state under "guided-questions". The // hook polls that key, and on submit/skip composes the answer as a normal // user turn (via the shared sendToAgentChat) and clears the persisted key so - // the question does not reappear. + // the question does not reappear. The key is per browser tab, so `threadId` + // is what keeps a pending question in the chat that asked it. const { questions: guidedQuestions, title: guidedQuestionsTitle, @@ -5365,6 +5366,7 @@ const AssistantChatInner = forwardRef< stateKey: "guided-questions", queryKey: ["guided-questions"], ...(browserTabId ? { browserTabId } : {}), + ...(threadId ? { threadId } : {}), }); const hasComposerAccessoryAboveStack = Boolean( composerError || diff --git a/packages/core/src/client/guided-questions.flow.spec.tsx b/packages/core/src/client/guided-questions.flow.spec.tsx index c0323f7cfc..11e3efa4f6 100644 --- a/packages/core/src/client/guided-questions.flow.spec.tsx +++ b/packages/core/src/client/guided-questions.flow.spec.tsx @@ -184,6 +184,79 @@ describe("useGuidedQuestionFlow scoped reads", () => { expect(requestedKeys).not.toContain("guided-questions:undefined"); }); + // The per-tab key is shared by every chat in that browser tab, so an + // agent-written payload names the thread that asked. Without this the same + // card followed the user into every other conversation. + it("hides a question asked in another chat", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL) => + readResponse(String(input), (key) => + key === "guided-questions:tab123" + ? JSON.stringify({ ...payload, threadId: "chat-a" }) + : "", + ), + ), + ); + + const result = await renderFlow({ + stateKey: "guided-questions", + queryKey: ["guided-questions"], + browserTabId: "tab123", + threadId: "chat-b", + refetchInterval: false, + }); + + expect(result.current().questions).toBeNull(); + expect(result.current().payload).toBeNull(); + }); + + it("renders a question in the chat that asked it", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL) => + readResponse(String(input), (key) => + key === "guided-questions:tab123" + ? JSON.stringify({ ...payload, threadId: "chat-a" }) + : "", + ), + ), + ); + + const result = await renderFlow({ + stateKey: "guided-questions", + queryKey: ["guided-questions"], + browserTabId: "tab123", + threadId: "chat-a", + refetchInterval: false, + }); + + expect(result.current().questions?.length).toBe(1); + }); + + it("renders a payload with no threadId in any chat", async () => { + // Client-initiated `askUserQuestion` and deterministic writes are not + // thread-bound; they must keep rendering wherever the flow is mounted. + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL) => + readResponse(String(input), (key) => + key === "guided-questions:tab123" ? JSON.stringify(payload) : "", + ), + ), + ); + + const result = await renderFlow({ + stateKey: "guided-questions", + queryKey: ["guided-questions"], + browserTabId: "tab123", + threadId: "chat-b", + refetchInterval: false, + }); + + expect(result.current().questions?.length).toBe(1); + }); + it("does not read application state when disabled", async () => { const fetchMock = vi.fn(); vi.stubGlobal("fetch", fetchMock); diff --git a/packages/core/src/client/guided-questions.tsx b/packages/core/src/client/guided-questions.tsx index 400cb2db15..d394ad9553 100644 --- a/packages/core/src/client/guided-questions.tsx +++ b/packages/core/src/client/guided-questions.tsx @@ -77,6 +77,13 @@ export interface GuidedQuestionPayload { * promise with the answer instead of forwarding it to the agent chat. */ clientResolveId?: string; + /** + * Chat thread that asked. Set by the agent's `ask-question` tool. A payload + * carrying this only renders in that conversation; one without it (app code + * calling {@link askUserQuestion}, deterministic writes) is not thread-bound + * and renders wherever the flow is mounted. + */ + threadId?: string; } const OTHER_OPTION_PREFIX = "__other__:"; @@ -973,6 +980,13 @@ export interface UseGuidedQuestionFlowOptions { * (which it almost always does — see `sessionBrowserTabId`). */ browserTabId?: string; + /** + * The conversation this flow is mounted in. Agent-written payloads name the + * thread that asked, and a pending question belongs to that conversation + * only — the application-state key is per browser tab, so without this the + * same card follows the user into every other chat in the tab. + */ + threadId?: string; queryKey?: readonly unknown[]; refetchInterval?: number | false; submitMessage?: string; @@ -984,10 +998,25 @@ export interface UseGuidedQuestionFlowOptions { buildSkipContext?: () => string; } +/** + * Whether a stored payload belongs to the conversation currently on screen. + * Payloads with no `threadId` are not thread-bound and always match. + */ +function payloadBelongsToThread( + payload: GuidedQuestionPayload, + threadId: string | undefined, +): boolean { + const asker = + typeof payload.threadId === "string" ? payload.threadId.trim() : ""; + if (!asker) return true; + return asker === (threadId ?? "").trim(); +} + export function useGuidedQuestionFlow({ enabled = true, stateKey = "show-questions", browserTabId, + threadId, queryKey = ["show-questions"], refetchInterval = false, submitMessage = "Here are my answers — go ahead.", @@ -1076,6 +1105,12 @@ export function useGuidedQuestionFlow({ } }, [data]); + // A question asked in another conversation stays in application state — the + // user can still answer it by going back to that chat — but it must not + // render here. + const visiblePayload = + payload && payloadBelongsToThread(payload, threadId) ? payload : null; + const clear = useCallback(() => { setPayload(null); queryClient.setQueryData(resolvedQueryKey, null); @@ -1090,15 +1125,16 @@ export function useGuidedQuestionFlow({ (answers: GuidedQuestionAnswers) => { // Client-initiated question (askUserQuestion): resolve the caller's // promise with the answer instead of forwarding it to the agent chat. - const resolveId = payload?.clientResolveId; + const resolveId = visiblePayload?.clientResolveId; if (resolveId) { - const firstId = payload?.questions?.[0]?.id ?? "q1"; + const firstId = visiblePayload?.questions?.[0]?.id ?? "q1"; resolveClientQuestion(resolveId, extractSingleAnswer(answers, firstId)); clear(); return; } const formattedAnswers = formatGuidedAnswersForAgent(answers); - const resolvedSubmitMessage = payload?.submitMessage ?? submitMessage; + const resolvedSubmitMessage = + visiblePayload?.submitMessage ?? submitMessage; const context = buildSubmitContext?.({ answers, formattedAnswers }) ?? defaultGuidedSubmitContext(formattedAnswers); @@ -1109,31 +1145,31 @@ export function useGuidedQuestionFlow({ }); clear(); }, - [buildSubmitContext, clear, payload, submitMessage], + [buildSubmitContext, clear, visiblePayload, submitMessage], ); const handleSkip = useCallback(() => { - const resolveId = payload?.clientResolveId; + const resolveId = visiblePayload?.clientResolveId; if (resolveId) { resolveClientQuestion(resolveId, null); clear(); return; } sendToAgentChat({ - message: payload?.skipMessage ?? skipMessage, + message: visiblePayload?.skipMessage ?? skipMessage, context: buildSkipContext?.(), submit: true, }); clear(); - }, [buildSkipContext, clear, payload, skipMessage]); + }, [buildSkipContext, clear, visiblePayload, skipMessage]); return { - payload, - questions: payload?.questions ?? null, - title: payload?.title, - description: payload?.description, - skipLabel: payload?.skipLabel, - submitLabel: payload?.submitLabel, + payload: visiblePayload, + questions: visiblePayload?.questions ?? null, + title: visiblePayload?.title, + description: visiblePayload?.description, + skipLabel: visiblePayload?.skipLabel, + submitLabel: visiblePayload?.submitLabel, clear, handleSubmit, handleSkip, diff --git a/packages/core/src/server/agent-chat/context-tools.ts b/packages/core/src/server/agent-chat/context-tools.ts index e03b5e3e22..076ca72f09 100644 --- a/packages/core/src/server/agent-chat/context-tools.ts +++ b/packages/core/src/server/agent-chat/context-tools.ts @@ -428,9 +428,13 @@ export function createUrlTools(): Record { }, }, "ask-question": { + // The turn is over once the question is on screen. Without this the loop + // asks the model for another step, and it keeps working (and re-asking) + // over an unanswered question no matter what the tool result says. + endsTurn: true, tool: { description: - "Ask the user a multiple-choice clarifying question and render it inline in the chat. Use this ONLY when you are genuinely blocked on a decision you cannot resolve from context and a wrong guess would be costly — an ambiguous metric, date range, or grain; a real fork in approach. Present 2-5 concrete options and mark the most likely one recommended. Do NOT use it for confirmations, for things the user already specified, or to dodge easy work you could just do. Ask at most once per turn. Calling this yields the turn: stop and wait for the user's answer.", + "Ask the user a multiple-choice clarifying question and render it inline in the chat. Use this ONLY when you are genuinely blocked on a decision you cannot resolve from context and a wrong guess would be costly — an ambiguous metric, date range, or grain; a real fork in approach. Present 2-5 concrete options and mark the most likely one recommended. Do NOT use it for confirmations, for things the user already specified, or to dodge easy work you could just do. Calling this ends the turn: one question per turn, and any other tool call you emit alongside it will not run.", parameters: { type: "object", properties: { @@ -523,7 +527,20 @@ export function createUrlTools(): Record { // carry `value`, with `multiSelect` for multi-pick and `allowOther` for // free text. The renderer otherwise injects "Explore"/"Decide" options, // which would be noise for a focused clarifying question, so disable them. + // The application-state key is scoped per browser tab, not per chat, so + // the payload has to name the thread that asked. The client hides a + // pending question in every other conversation instead of following the + // user from chat to chat. + // A request that carried no thread id falls back to the run id (see + // `onRunStart`), and a per-run value would never match the chat the user + // is looking at. Leave those surfaces unbound — they have one chat. + const askingRunCtx = getRequestRunContext(); + const askingThreadId = + askingRunCtx?.threadId && askingRunCtx.threadId !== askingRunCtx.runId + ? askingRunCtx.threadId + : undefined; const payload = { + ...(askingThreadId ? { threadId: askingThreadId } : {}), questions: [ { id: "q1", @@ -549,7 +566,9 @@ export function createUrlTools(): Record { ), payload, ); - return "Asked the user a clarifying question and rendered it in the chat. Stop here and wait for their answer — do not proceed or assume an answer."; + // The `startsWith` of this text is how integration surfaces recognize a + // delivered question (see `extractSlackInputRequest`). Keep the prefix. + return "Asked the user a clarifying question and rendered it in the chat. This turn is over — their answer arrives as a new message."; }, }, }; From 8e1160782fef375df35f0d19df211490efe67c8c Mon Sep 17 00:00:00 2001 From: Manu MA Date: Mon, 10 Aug 2026 12:42:23 +0200 Subject: [PATCH 2/2] feat: add ask-question card to chat and end turn upon rendering --- .changeset/scope-ask-question-to-its-chat.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/scope-ask-question-to-its-chat.md diff --git a/.changeset/scope-ask-question-to-its-chat.md b/.changeset/scope-ask-question-to-its-chat.md new file mode 100644 index 0000000000..0d66ab5ae2 --- /dev/null +++ b/.changeset/scope-ask-question-to-its-chat.md @@ -0,0 +1,5 @@ +--- +"@agent-native/core": patch +--- + +Keep an `ask-question` card in the chat that asked it, and end the turn once it renders instead of letting the agent keep working over an unanswered question.