From f70946c5ff5f80fb2a3619af5292517181fd90e5 Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 20:19:04 -0600 Subject: [PATCH 1/8] feat(engine): answer the SDK's AskUserQuestion through canUseTool (#4) buildEngineOptions turns on the SDK's own question tool with toolConfig.askUserQuestion.previewFormat 'html' and settings.askUserQuestionTimeout 'never' (a Settings field, not an Option). Read off the installed SDK 0.3.278 / CLI 2.1.278: the tool's permission check always asks, so every call reaches canUseTool, and the host answers by allowing with updatedInput.answers (question text -> answer, multi-select comma-separated), which the CLI echoes back as tool_use_result.answers. The engine hands the call to the run's askUser host ($lib/engine/ask-user-question): no tool frames, refused at once with a reason when there is no host or a subagent asked, settled on Stop, and never left pending. The chat's host records the question on chat_runs.pending_questions (opening the /review item and needs-input push as before), emits the ask_user frame under the SDK's tool_use id, and waits; the wait now ends early on abort. Saved blocks keep the answers as details { kind: 'ask_user_question' }. A question is exempt from approval settings and modes but not from an agent's tool scope; ask_user is kept as an alias so an agent listing it can still ask, and every subagent disallows AskUserQuestion. Co-Authored-By: Claude Opus 5.5 --- src/lib/chat/ask-user-host.server.ts | 88 +++++ src/lib/engine/agent-definitions.ts | 14 +- src/lib/engine/ask-user-question.ts | 408 ++++++++++++++++++++ src/lib/engine/builtin-tools.ts | 28 +- src/lib/engine/options.server.ts | 8 + src/lib/engine/permission-mode.ts | 6 +- src/lib/engine/stream.server.ts | 59 ++- src/lib/engine/tool-decision.ts | 6 + src/lib/engine/tool-result-details.ts | 44 ++- src/lib/runs/questions.server.ts | 29 +- src/lib/runs/runs.schema.ts | 19 +- src/routes/chat/[id]/stream/+server.ts | 38 +- tests/chat.ask-user-host.spec.ts | 96 +++++ tests/engine.agent-definitions.spec.ts | 6 +- tests/engine.ask-user-question.spec.ts | 490 +++++++++++++++++++++++++ tests/engine.stream-approvals.spec.ts | 45 ++- 16 files changed, 1290 insertions(+), 94 deletions(-) create mode 100644 src/lib/chat/ask-user-host.server.ts create mode 100644 src/lib/engine/ask-user-question.ts create mode 100644 tests/chat.ask-user-host.spec.ts create mode 100644 tests/engine.ask-user-question.spec.ts diff --git a/src/lib/chat/ask-user-host.server.ts b/src/lib/chat/ask-user-host.server.ts new file mode 100644 index 00000000..360f351a --- /dev/null +++ b/src/lib/chat/ask-user-host.server.ts @@ -0,0 +1,88 @@ +/** + * The chat's answer to the SDK's AskUserQuestion (#4): put the question in front of the user + * and wait for what they choose. + * + * The engine hands a question over from `canUseTool` (`$lib/engine/ask-user-question`); this + * is the chat run's side of that hand-off, and it does what the old `ask_user` host did: + * + * 1. record the question on the run (`chat_runs.pending_questions`) and move the run to + * `waiting_user_input`. Recording it is what opens its /review inbox item and schedules + * the "needs input" notification, so a user who is not watching the chat can still find + * it and answer it there (`$lib/runs/questions.server`); + * 2. send the `ask_user` frame, which the chat page renders as the question card. The frame + * keeps its old name — it means "the agent is asking you", not "this tool ran" — and now + * carries the SDK's tool_use id, so the card, the frame and the call's result share one id; + * 3. wait for the answer, from the card, the composer or /review. The wait is bounded like + * an approval's (`QUESTION_TIMEOUT_MS`) and cut short by a stop; after that the model is + * told nobody answered. Nothing ever answers on the user's behalf. + * + * The store is injectable so a spec can drive the hand-off without a database. + */ + +import { + ASK_USER_QUESTION_TOOL, + answerKey, + type AskQuestion, + type AskUserHost, +} from '$lib/engine/ask-user-question' +import type { PendingQuestionEntry } from '$lib/runs/runs.schema' +import { awaitQuestionAnswers, enqueuePendingQuestion, QUESTION_TIMEOUT_MS } from '$lib/runs/questions.server' +import { markChatRunRunning } from '$lib/runs/run-lifecycle.server' + +export type AskUserStore = { + enqueue: ( + runId: string, + entry: Omit, + transition: { state: 'waiting_user_input'; label: string }, + ) => Promise + awaitAnswers: (runId: string, token: string, signal: AbortSignal) => Promise | null> + markRunning: (runId: string) => Promise +} + +const databaseStore: AskUserStore = { + enqueue: (runId, entry, transition) => enqueuePendingQuestion(runId, entry, transition), + awaitAnswers: (runId, token, signal) => awaitQuestionAnswers(runId, token, QUESTION_TIMEOUT_MS, signal), + markRunning: (runId) => markChatRunRunning(runId), +} + +/** + * The token an answer has to present. One per question call, so two questions in one turn + * never share one, and readable in a log next to the approval tokens (`:`). + */ +export function askUserToken(runId: string, toolUseId: string): string { + return `${runId}:ask:${toolUseId}` +} + +/** A question as it is recorded and sent: keyed, and with "Other" spelled out. */ +export function pendingQuestionsFor(questions: AskQuestion[]): AskQuestion[] { + return questions.map((question) => ({ + ...question, + key: answerKey(question), + allowFreeformInput: question.allowFreeformInput ?? true, + })) +} + +export function createAskUserHost(input: { + runId: string + /** The stream's frame writer — persisted as a run event, so a reconnecting page replays it. */ + emit: (event: string, payload: unknown) => Promise + store?: AskUserStore +}): AskUserHost { + const store = input.store ?? databaseStore + return async ({ toolUseId, questions, signal }) => { + const token = askUserToken(input.runId, toolUseId) + const pending = pendingQuestionsFor(questions) + + await store.enqueue( + input.runId, + { token, questions: pending, requestedAt: new Date().toISOString() }, + { state: 'waiting_user_input', label: 'Waiting for your answer' }, + ) + await input.emit('ask_user', { id: toolUseId, name: ASK_USER_QUESTION_TOOL, token, questions: pending }) + + const answers = await store.awaitAnswers(input.runId, token, signal) + // A stopped run is ending; putting it back to "running" would only flicker. + if (!signal.aborted) await store.markRunning(input.runId) + return { answers } + } +} diff --git a/src/lib/engine/agent-definitions.ts b/src/lib/engine/agent-definitions.ts index 938a57c2..29905ccb 100644 --- a/src/lib/engine/agent-definitions.ts +++ b/src/lib/engine/agent-definitions.ts @@ -18,9 +18,10 @@ * `idle` and `active` both available — lives in `$lib/agents/agent-status`, which the Pause * button, the automation gate and this filter all read. * - * **`ask_user` is disallowed for every subagent.** The old loop refused it for - * non-orchestrators — a child has no stream to ask down — and that rule has to survive the - * port or a delegated agent will hang waiting for an answer nobody is being shown. + * **`AskUserQuestion` is disallowed for every subagent.** The old loop refused `ask_user` for + * non-orchestrators — a child has no stream to ask down — and that rule survived the port to + * the SDK's own question tool (#4). The engine refuses a child's question in `canUseTool` as + * well, for the SDK's built-in agents, which no definition here reaches. * * **The model is inherited unless the row asks for a Claude model.** A run against the * gateway sets `ANTHROPIC_MODEL` for the whole process, so a subagent naming a different @@ -31,6 +32,7 @@ import { isAgentPaused } from '$lib/agents/agent-status' import { BUILTIN_TOOL_SET } from './builtin-tools' +import { ASK_USER_QUESTION_TOOL } from './ask-user-question' import { OWN_MCP_SERVER } from './permission-mode' /** The subset of `AgentDefinition` this app populates. Mirrors the SDK type structurally. */ @@ -72,9 +74,9 @@ export function qualifyAgentTools(allowedTools: readonly string[]): string[] { /** Tools no subagent may call, whatever its own allow-list says. */ export const SUBAGENT_DISALLOWED_TOOLS: readonly string[] = [ - // A child has no stream to ask down. The old loop refused this for non-orchestrators; - // without it a delegated agent hangs on a question nobody is shown. - 'mcp__agentstudio__ask_user', + // A child has no stream to ask down. The old loop refused its `ask_user` for + // non-orchestrators; the SDK's question tool is refused the same way. + ASK_USER_QUESTION_TOOL, ] /** diff --git a/src/lib/engine/ask-user-question.ts b/src/lib/engine/ask-user-question.ts new file mode 100644 index 00000000..8a15bf27 --- /dev/null +++ b/src/lib/engine/ask-user-question.ts @@ -0,0 +1,408 @@ +/** + * The Agent SDK's native AskUserQuestion (#4): the questions as data, how the host answers + * one, and how an option's preview is rendered without trusting it. + * + * AgentStudio used to register its own `ask_user` tool on the in-process MCP server and + * special-case it in three places. The SDK ships a better one — a preview per option, + * multi-select, and a free-text "Other" the CLI adds itself — and it reaches the host the + * way every built-in call does: through `canUseTool`. The host answers it there, by + * returning the call's input with an `answers` map added. No tool of ours runs at all. + * + * Read off the installed SDK (`@anthropic-ai/claude-agent-sdk` 0.3.278, bundled CLI + * 2.1.278), not assumed: + * + * - `sdk-tools.d.ts` `AskUserQuestionInput`: 1–4 questions, each a `question`, a short + * `header` (the CLI calls it a chip, max 12 characters), 2–4 options of + * `{ label, description, preview? }` and `multiSelect`. `answers` is typed "User answers + * collected by the permission component", keyed by question text. The CLI's `call()` + * reads exactly that field back and echoes it on `tool_use_result` + * (`AskUserQuestionOutput.answers`, "multi-select answers are comma-separated"). + * - The CLI's own tool prompt says a recommended option goes first with "(Recommended)" at + * the end of its label, and that "Other" is never an option: "Users will always be able + * to select Other". + * - In `previewFormat: 'html'` the CLI rejects a preview that is a whole document or holds + * `
', + ) + expect(doc).toContain(``) + expect(PREVIEW_CSP).toContain("default-src 'none'") + expect(PREVIEW_CSP).not.toMatch(/script-src|https?:/) + // Inline styles survive — they are the point of a preview. + expect(doc).toContain('

hi

') + const body = doc.slice(doc.indexOf('')) + for (const tag of [' { + // No `allow-scripts`, no `allow-same-origin`: an opaque origin with scripts off, so a + // preview can neither run code nor reach the app, its cookies or its remote functions. + const source = readFileSync(resolve('src/lib/chat/AskUserPreview.svelte'), 'utf8') + expect(source).toMatch(/ { + test('HTML previews, and a question that never answers itself', () => { + expect(ASK_USER_QUESTION_TOOL_CONFIG).toEqual({ askUserQuestion: { previewFormat: 'html' } }) + expect(ASK_USER_QUESTION_SETTINGS).toEqual({ askUserQuestionTimeout: 'never' }) + // Read as source: `options.server.ts` imports `$env`, which this runtime cannot resolve. + const source = readFileSync(resolve('src/lib/engine/options.server.ts'), 'utf8') + expect(source).toMatch(/toolConfig:\s*ASK_USER_QUESTION_TOOL_CONFIG/) + expect(source).toMatch(/settings:\s*ASK_USER_QUESTION_SETTINGS/) + }) +}) + +// ── Driven through the engine ──────────────────────────────────────────────────────────── + +type Frame = { event: string; payload: Record } +type Call = { id: string; input: Record; parent?: string | null } + +const WS = process.platform === 'win32' ? 'C:\\sandbox\\u1\\runs\\r1' : '/sandbox/u1/runs/r1' + +/** + * A scripted SDK for question calls: announce the call, run the CLI's permission pipeline + * (the PreToolUse hook, then `canUseTool` — AskUserQuestion's own check always asks), then + * yield the result the CLI would: on an allow, the text the model reads plus the + * `tool_use_result` echoing the answers it was given. + */ +async function driveQuestions( + input: { askUser?: AskUserHost; requiresApproval?: () => boolean; toolScope?: ToolScope | null }, + calls: Call[], +) { + const { runEngineStream } = await import('../src/lib/engine/stream.server') + const frames: Frame[] = [] + const permissions: PermissionResult[] = [] + const hooks: string[] = [] + const summary = await runEngineStream({ + prompt: 'go', + options: {}, + workspaceRoot: WS, + bashPolicy: 'sandboxed', + requiresApproval: input.requiresApproval ?? (() => false), + ...(input.askUser ? { askUser: input.askUser } : {}), + ...(input.toolScope ? { toolScope: input.toolScope } : {}), + createQuery: ({ options }: { options: Options }) => ({ + async *[Symbol.asyncIterator]() { + for (const call of calls) { + const parent = call.parent ?? null + yield { + type: 'assistant', + parent_tool_use_id: parent, + message: { content: [{ type: 'tool_use', id: call.id, name: ASK_USER_QUESTION_TOOL, input: call.input }] }, + } as never + let hook = 'none' + for (const matcher of (options.hooks?.PreToolUse ?? []) as HookCallbackMatcher[]) { + for (const fn of matcher.hooks) { + const out = (await fn( + { hook_event_name: 'PreToolUse', session_id: 's', transcript_path: '', cwd: WS, tool_name: ASK_USER_QUESTION_TOOL, tool_input: call.input, tool_use_id: call.id } as never, + call.id, + { signal: new AbortController().signal }, + )) as { hookSpecificOutput?: { permissionDecision?: string; permissionDecisionReason?: string } } + hook = out.hookSpecificOutput?.permissionDecision ?? hook + } + } + hooks.push(hook) + const permission: PermissionResult = + hook === 'deny' + ? { behavior: 'deny', message: 'refused by the hook' } + : ((await options.canUseTool!(ASK_USER_QUESTION_TOOL, call.input, { + signal: new AbortController().signal, + toolUseID: call.id, + requestId: 'r', + ...(parent ? { agentID: 'agent-1' } : {}), + })) as PermissionResult) + permissions.push(permission) + const answers = permission.behavior === 'allow' ? (permission.updatedInput?.answers as Record) : null + yield { + type: 'user', + parent_tool_use_id: parent, + message: { + content: [ + { + type: 'tool_result', + tool_use_id: call.id, + content: answers + ? `User has answered your questions: ${Object.entries(answers).map(([q, a]) => `"${q}"="${a}"`).join(', ')}.` + : permission.behavior === 'deny' ? permission.message : '', + is_error: !answers, + }, + ], + }, + ...(answers ? { tool_use_result: { questions: call.input.questions, answers } } : {}), + } as never + } + yield { type: 'result', usage: {}, duration_ms: 1, num_turns: 1 } as never + }, + }), + emit: async (event: string, payload: unknown) => { + frames.push({ event, payload: (payload ?? {}) as Record }) + }, + }) + return { frames, permissions, hooks, summary } +} + +test.describe('the engine hands AskUserQuestion to the host', () => { + const call = { id: 'toolu_ask1', input: { questions: [{ question: Q2.question, header: 'Features', multiSelect: true, options: Q2.options }] } } + + test('answered: allowed with the answers, no approval card, and a saved block that shows them after a reload', async () => { + const { frames, permissions, summary } = await driveQuestions( + { + // Every approval setting on: none of them reaches a question. + requiresApproval: () => true, + askUser: async () => ({ answers: { [Q2.question]: 'Search, Export' } }), + }, + [call], + ) + expect(permissions[0]).toEqual({ behavior: 'allow', updatedInput: { ...call.input, answers: { [Q2.question]: 'Search, Export' } } }) + const own = frames.filter((f) => f.payload.id === call.id).map((f) => f.event) + expect(own).toEqual(['tool_result']) + const result = frames.find((f) => f.event === 'tool_result')! + expect(result.payload).toMatchObject({ name: 'AskUserQuestion', success: true, details: { kind: 'ask_user_question', answers: { [Q2.question]: 'Search, Export' } } }) + expect(summary.blocks).toContainEqual( + expect.objectContaining({ + kind: 'tool', + name: 'AskUserQuestion', + arguments: call.input, + success: true, + details: { kind: 'ask_user_question', answers: { [Q2.question]: 'Search, Export' } }, + }), + ) + }) + + test('no host (a run nobody can answer): refused with a reason the model can act on', async () => { + const { permissions, frames } = await driveQuestions({}, [call]) + expect(permissions[0]).toEqual({ behavior: 'deny', message: ASK_USER_REFUSALS.noHost }) + expect(frames.find((f) => f.event === 'tool_result')?.payload).toMatchObject({ success: false }) + }) + + test('an agent scoped without it is refused in the hook at once, and nobody is asked', async () => { + let asked = 0 + const started = Date.now() + const { hooks, permissions } = await driveQuestions( + { + toolScope: resolveToolScope(['Read'], { delegation: false }), + askUser: async () => { + asked += 1 + return { answers: {} } + }, + }, + [call], + ) + expect(hooks).toEqual(['deny']) + expect(permissions[0].behavior).toBe('deny') + expect(asked).toBe(0) + // The call was announced, so the refusal does not sit out the announcement wait. + expect(Date.now() - started).toBeLessThan(1_500) + }) + + test('an old agent listing ask_user keeps the ability to ask', async () => { + const { hooks, permissions } = await driveQuestions( + { + toolScope: resolveToolScope(['Read', 'ask_user'], { delegation: false }), + askUser: async () => ({ answers: { [Q2.question]: 'Share' } }), + }, + [call], + ) + expect(hooks).toEqual(['none']) + expect(permissions[0].behavior).toBe('allow') + }) + + test("a subagent's question is refused even with a host, and nobody is asked", async () => { + let asked = 0 + const { permissions } = await driveQuestions( + { + askUser: async () => { + asked += 1 + return { answers: { [Q2.question]: 'Search' } } + }, + }, + [{ ...call, id: 'toolu_child', parent: 'toolu_task' }], + ) + expect(asked).toBe(0) + expect(permissions[0]).toEqual({ behavior: 'deny', message: ASK_USER_REFUSALS.subagent }) + }) +}) diff --git a/tests/engine.stream-approvals.spec.ts b/tests/engine.stream-approvals.spec.ts index a8db0e1b..83f5d569 100644 --- a/tests/engine.stream-approvals.spec.ts +++ b/tests/engine.stream-approvals.spec.ts @@ -177,24 +177,43 @@ test.describe('every call meets the gate', () => { }) test.describe('the tools the host owns', () => { - test('skip both gates, and are exactly the set the settings list and MCP leave out', async () => { - // `builtin-tools`' HOST_OWNED_TOOLS is what the settings approval list and /api/mcp - // filter by, on the promise that the engine hands those calls to the host before any - // gate runs. The engine keeps its own copy of the set, so drive it: with every tool set - // to ask, a host-owned call gets no 'ask', no card and no tool_call frame (the host - // renders its own), and every other registry tool is asked about. + test('no registry tool skips the gate: with every tool set to ask, each one is asked about', async () => { + // `builtin-tools`' HOST_OWNED_TOOLS is the set the engine hands to the host ungated. It + // used to hold the in-house `ask_user`, which the settings approval list and /api/mcp + // had to leave out. Since #4 it holds only the SDK's AskUserQuestion, which is not a + // registry tool — so every registry tool must now meet the gate, and the settings list + // may offer a tick for each one. const calls = allToolNames.map((name, i) => ({ id: `t${i}`, name: `mcp__agentstudio__${name}`, input: {} })) - const { outcomes, frames } = await drive({ requiresApproval: () => true }, calls) + const { outcomes } = await drive({ requiresApproval: () => true }, calls) const handedOver = allToolNames.filter( (_, i) => outcomes[i].hook === 'none' && outcomes[i].permission?.behavior === 'allow', ) - expect([...handedOver].sort()).toEqual([...HOST_OWNED_TOOLS].sort()) - for (const [i, name] of allToolNames.entries()) { - if (!HOST_OWNED_TOOLS.has(name)) continue - const seen = events(frames, `t${i}`) - for (const frame of ['tool_call', 'tool_pending', 'tool_denied']) expect(seen, name).not.toContain(frame) - } + expect(handedOver).toEqual([]) + for (const name of HOST_OWNED_TOOLS) expect(allToolNames as readonly string[], name).not.toContain(name) + }) + + test('AskUserQuestion skips both gates and goes to the host, with no tool frame of its own', async () => { + // Every approval setting on, and plan mode: a question is answered by the user in its + // own card, so no approval card may appear, and plan mode is exactly when one is wanted. + const asked: string[] = [] + const input = { questions: [{ question: 'Which?', header: 'Pick', multiSelect: false, options: [{ label: 'A', description: 'a' }, { label: 'B', description: 'b' }] }] } + const { outcomes, frames } = await drive( + { + requiresApproval: () => true, + permissionMode: 'plan', + askUser: async ({ toolUseId }) => { + asked.push(toolUseId) + return { answers: { 'Which?': 'B' } } + }, + }, + [{ id: 'toolu_q1', name: 'AskUserQuestion', input }], + ) + expect([...HOST_OWNED_TOOLS]).toEqual(['AskUserQuestion']) + expect(outcomes[0].hook).toBe('none') + expect(outcomes[0].permission).toEqual({ behavior: 'allow', updatedInput: { ...input, answers: { 'Which?': 'B' } } }) + expect(asked).toEqual(['toolu_q1']) + for (const frame of ['tool_call', 'tool_pending', 'tool_denied']) expect(events(frames, 'toolu_q1')).not.toContain(frame) }) }) From ac7a6b0d8880825521f1a66cb7956725cccb9e06 Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 20:19:12 -0600 Subject: [PATCH 2/8] refactor(tools): retire the in-house ask_user tool (#4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing live needs it once the SDK's AskUserQuestion is on: its schema, description and "cannot run directly" handler leave the registry, the engine's MCP server no longer special-cases it, and the old loop's ask_user branch (and the trimToolResult helper only it used) goes with the isOrchestrator flag that gated it — that loop's callers are unattended and were never offered it; a model that asks anyway meets the offered-list gate. /api/mcp and the settings approval list no longer have a host-owned registry tool to leave out. The read-only agents' allow-list carries AskUserQuestion in place of ask_user, and the orchestrator prompt tells the model "Other" is automatic and when to give an HTML preview. Co-Authored-By: Claude Opus 5.5 --- src/lib/agents/builtin-agents.server.ts | 4 +- src/lib/chat/chat.remote.ts | 2 +- src/lib/chat/chat.ts | 39 +----- src/lib/chat/creation-flow.ts | 2 +- src/lib/chat/run-controls.ts | 2 +- src/lib/chat/stream-slots.server.ts | 13 +- src/lib/engine/tools.server.ts | 18 --- src/lib/runs/needs-input.server.ts | 2 +- src/lib/runs/prompt-review-items.server.ts | 4 +- src/lib/runtime/agent-definition.server.ts | 8 +- src/lib/runtime/detached-tools.ts | 3 +- src/lib/runtime/loop.server.ts | 1 - src/lib/runtime/tool-handlers.server.ts | 141 +-------------------- src/lib/runtime/types.ts | 2 +- src/lib/tools/handlers/meta.server.ts | 14 -- src/lib/tools/tool-schemas.ts | 23 ---- src/lib/tools/tools.server.ts | 3 - src/lib/tools/tools.ts | 14 +- tests/chat.agent-tool-policy.spec.ts | 10 +- tests/engine.tool-surface.spec.ts | 1 - tests/runs.live.spec.ts | 23 ++-- tests/runtime.detached-tools.spec.ts | 9 +- tests/runtime.offered-tools.spec.ts | 21 +-- tests/settings.tool-approval.spec.ts | 23 ++-- tests/tools.mcp-surface.spec.ts | 22 ++-- 25 files changed, 99 insertions(+), 305 deletions(-) diff --git a/src/lib/agents/builtin-agents.server.ts b/src/lib/agents/builtin-agents.server.ts index 7a81ba40..69a19003 100644 --- a/src/lib/agents/builtin-agents.server.ts +++ b/src/lib/agents/builtin-agents.server.ts @@ -45,8 +45,8 @@ export const BUILTIN_AGENT_IDS: Record = { * `Write` from Research would break its only workflow. See docs/agents/spec.md. */ export const READ_ONLY_TOOL_NAMES: readonly string[] = [ - // Essentials. - 'ask_user', + // Essentials. `AskUserQuestion` is the SDK's question tool (#4), which replaced `ask_user`. + 'AskUserQuestion', 'web_search', // Plan authoring + handoff. The plan lives on disk now: the planner writes a markdown // file with Write and hands off via request_plan_approval. Write is the one diff --git a/src/lib/chat/chat.remote.ts b/src/lib/chat/chat.remote.ts index d37d478c..9ba3b7b2 100644 --- a/src/lib/chat/chat.remote.ts +++ b/src/lib/chat/chat.remote.ts @@ -100,7 +100,7 @@ export const getConversation = query(conversationIdSchema, async (conversationId findLiveChatRun(conversationId, user.id), ]) - // Surface the first un-decided ask_user entry so a hard refresh during a paused question + // Surface the first un-decided question so a hard refresh during a paused question // can resume — the stream path owns updates while connected; this is the resume seed. const undecided = (activeRun?.pendingQuestions ?? []).find( (entry): entry is PendingQuestionEntry => !!entry?.token && !entry.decidedAt, diff --git a/src/lib/chat/chat.ts b/src/lib/chat/chat.ts index be1c8dd9..7dcc07b0 100644 --- a/src/lib/chat/chat.ts +++ b/src/lib/chat/chat.ts @@ -65,7 +65,7 @@ marked.setOptions({ /* * Everything rendered here is model output — replies, thinking, subagent results, - * ask_user questions — and it goes straight into `{@html}` on a page that can call every + * the agent's questions — and it goes straight into `{@html}` on a page that can call every * remote function as the user. So it is sanitized at the renderer (see * `util/safe-markdown.ts` for the rules). This must be an object literal: `marked` only * honours own enumerable properties of `use({ renderer })`. @@ -300,40 +300,3 @@ export function getWebSearchPreview(toolName: string, rawResult: unknown): WebSe export function faviconUrl(hostname: string) { return `https://www.google.com/s2/favicons?domain=${encodeURIComponent(hostname)}&sz=32` } - -/* ── Tool result caps ──────────────────────────────────────── */ - -export function trimToolResult(toolName: string, resultStr: string): string { - const limits: Record = { - web_search: 6000, - Read: 32000, - Bash: 16000, - browser_screenshot: Infinity, - run_subagent: 16000, - } - - const limit = limits[toolName] ?? 16000 - - if (resultStr.length <= limit) return resultStr - - try { - const parsed = JSON.parse(resultStr) - - if (toolName === 'web_search' && Array.isArray(parsed)) { - const trimmed = parsed.slice(0, 5).map((r: Record) => ({ - ...r, - snippet: typeof r.snippet === 'string' ? r.snippet.slice(0, 500) : r.snippet, - content: typeof r.content === 'string' ? r.content.slice(0, 500) : r.content, - })) - return JSON.stringify(trimmed) - } - - const s = JSON.stringify(parsed) - if (s.length > limit) { - return s.slice(0, limit) + `\n... [truncated from ${s.length} chars]` - } - return s - } catch { - return resultStr.slice(0, limit) + `\n... [truncated from ${resultStr.length} chars]` - } -} diff --git a/src/lib/chat/creation-flow.ts b/src/lib/chat/creation-flow.ts index cada2d01..6508e8ab 100644 --- a/src/lib/chat/creation-flow.ts +++ b/src/lib/chat/creation-flow.ts @@ -12,7 +12,7 @@ type GuidedCreationInput = { function buildPrompt({ kind, context = {} }: GuidedCreationInput) { const commonRules = [ 'Help me create this using a cooperative execution flow.', - 'First ask clarifying questions one at a time if needed, using ask_user when options are helpful.', + 'First ask clarifying questions one at a time if needed, using AskUserQuestion when options are helpful.', 'Execute tool calls directly as needed once requirements are clear.', 'If user input is missing, ask for it and then continue execution.', 'Summarize what was created and any follow-up options.', diff --git a/src/lib/chat/run-controls.ts b/src/lib/chat/run-controls.ts index e276a41b..1de8c24a 100644 --- a/src/lib/chat/run-controls.ts +++ b/src/lib/chat/run-controls.ts @@ -67,7 +67,7 @@ export function approvalAnswerProblem(ok: boolean, status: number, body: unknown } /** - * What to tell the user when an `ask_user` answer was not recorded, or null when it was. + * What to tell the user when an answer to the agent's question was not recorded, or null when it was. * * `/ask-user` answers a token it cannot find with a 200 and `resolved: false`, and the page * read only the status: the modal closed as if the answer had been taken, and the answer diff --git a/src/lib/chat/stream-slots.server.ts b/src/lib/chat/stream-slots.server.ts index fd5eb65a..5fd6714b 100644 --- a/src/lib/chat/stream-slots.server.ts +++ b/src/lib/chat/stream-slots.server.ts @@ -242,22 +242,23 @@ export async function buildProjectContextSlot(input: { const ORCHESTRATOR_TOOL_POLICY = [ 'Tool usage policy:', - '- If the user asks you to ask questions, gather preferences with options, or confirm choices before continuing, you MUST call the ask_user tool.', - "- Do not only say you'll ask a question in plain text when ask_user is appropriate.", - '- Use concise questions with clear option labels, and allow freeform input when the request is open-ended.', - '- For ask_user: aim for ~3 prefilled answer options per question. Prefer asking more focused questions (split complex choices across multiple questions) rather than listing many options in one question.', + '- If the user asks you to ask questions, gather preferences with options, or confirm choices before continuing, you MUST call the AskUserQuestion tool.', + "- Do not only say you'll ask a question in plain text when AskUserQuestion is appropriate.", + '- Use concise questions with clear option labels. The user can always pick "Other" and type their own answer, so never add an "Other" option yourself.', + '- For AskUserQuestion: aim for ~3 options per question. Prefer asking more focused questions (split complex choices across multiple questions) rather than listing many options in one question. Set multiSelect when the choices are not mutually exclusive.', + '- When the options are things to compare by eye (layouts, snippets, configurations), give each one an HTML preview. The user sees it in a sandboxed pane, so keep it self-contained with inline styles.', '', ...SUBAGENT_RESULT_POLICY_LINES, ].join('\n') const AGENT_TOOL_POLICY = [ 'Tool usage policy:', - '- You cannot call ask_user directly in agent conversations.', + '- You cannot ask the user questions directly (AskUserQuestion) in agent conversations.', '- If you need user input, summarize missing information and return control to orchestrator for follow-up.', ].join('\n') /** - * The tool-usage policy slot. Orchestrator agents get the ask_user-encouraging + * The tool-usage policy slot. Orchestrator agents get the AskUserQuestion-encouraging * variant; sub-agents get the variant that tells them they can't ask the user * directly. Priority 90 — high enough to be near the top, below identity and * project context. diff --git a/src/lib/engine/tools.server.ts b/src/lib/engine/tools.server.ts index d7cc01f2..6299c506 100644 --- a/src/lib/engine/tools.server.ts +++ b/src/lib/engine/tools.server.ts @@ -41,22 +41,10 @@ export type ToolExecutionRecord = { error?: string } -export type AskUserQuestion = { - header: string - question: string - options?: Array<{ label: string; description?: string; recommended?: boolean }> -} - export type ToolServerContext = { userId: string runId: string | null workspace?: WorkspaceOptions - /** - * Fulfils `ask_user`. The registry's handler is a deliberate stub — the old - * chat loop special-cased the tool, so without a host implementation the - * model just gets "cannot run directly" back. Resolves once the user answers. - */ - onAskUser?: (questions: AskUserQuestion[]) => Promise /** * Fired after every tool finishes. The stream layer uses this to emit * `tool_result` frames without having to re-derive the outcome. @@ -86,12 +74,6 @@ export function buildToolServer(ctx: ToolServerContext, only?: ReadonlySet) => { - if (name === 'ask_user' && ctx.onAskUser) { - const questions = (args.questions ?? []) as AskUserQuestion[] - const answer = await ctx.onAskUser(questions) - return { content: [{ type: 'text' as const, text: answer }] } - } - const outcome = await executeTool({ name, arguments: args }, ctx.userId, ctx.runId, ctx.workspace) ctx.onExecuted?.({ diff --git a/src/lib/runs/needs-input.server.ts b/src/lib/runs/needs-input.server.ts index 602d7d76..b2b2ccb0 100644 --- a/src/lib/runs/needs-input.server.ts +++ b/src/lib/runs/needs-input.server.ts @@ -7,7 +7,7 @@ import { logger } from '$lib/observability/logger' * "Needs input" notifications: a run has been waiting on the user for a while. * * The toggle existed in Settings and nothing ever sent one. A run that pauses on a tool - * approval or an `ask_user` question waits five minutes (`DECISION_TIMEOUT_MS`) and then + * approval or a question from the agent waits five minutes (`DECISION_TIMEOUT_MS`) and then * gives up — denying the tool, or answering "the user did not answer in time" — so a user * who has walked away from the chat loses the turn without knowing it was waiting. * diff --git a/src/lib/runs/prompt-review-items.server.ts b/src/lib/runs/prompt-review-items.server.ts index 5c1c85e9..c2353f92 100644 --- a/src/lib/runs/prompt-review-items.server.ts +++ b/src/lib/runs/prompt-review-items.server.ts @@ -5,8 +5,8 @@ import { reviewItems } from '$lib/observability/observability.schema' import { logger } from '$lib/observability/logger' /** - * The review-inbox items a paused run opens — one per tool approval, one per `ask_user` - * question — and how they close. + * The review-inbox items a paused run opens — one per tool approval, one per question the + * agent asks (AskUserQuestion, #4) — and how they close. * * They used to be opened and never closed. Approving a call in the chat left its item open * in /review for good, each approval added another, and the inbox's open count and the diff --git a/src/lib/runtime/agent-definition.server.ts b/src/lib/runtime/agent-definition.server.ts index 2be650a2..a3b4c39e 100644 --- a/src/lib/runtime/agent-definition.server.ts +++ b/src/lib/runtime/agent-definition.server.ts @@ -40,8 +40,8 @@ export type BuildAgentDefinitionInput = { */ intent?: string /** - * The agent collaboration policy text — varies by caller. Sub-agents say "you cannot - * ask_user, return a handoff"; automations say "no human in the loop, summarize what you + * The agent collaboration policy text — varies by caller. Sub-agents say "you cannot ask + * the user, return a handoff"; automations say "no human in the loop, summarize what you * did"; tasks say similar. Caller picks the right one. */ toolPolicy: string @@ -173,8 +173,8 @@ export async function buildAgentDefinition(input: BuildAgentDefinitionInput): Pr const assembled = assembleSystemPrompt(slots) - // Tool surface: the unattended-run list, narrowed by allowedTools when set. ask_user is - // never on it — the loop's `isOrchestrator: false` would refuse it anyway. + // Tool surface: the unattended-run list, narrowed by allowedTools when set. Nothing on it + // asks the user: nobody is there to answer. const tools = getToolDefinitions( detachedRunToolNames(Array.isArray(config?.allowedTools) ? config.allowedTools : null), ) diff --git a/src/lib/runtime/detached-tools.ts b/src/lib/runtime/detached-tools.ts index 24ee4a90..08cea5b2 100644 --- a/src/lib/runtime/detached-tools.ts +++ b/src/lib/runtime/detached-tools.ts @@ -9,7 +9,8 @@ import type { ToolName } from '../tools/tool-schemas' * This used to be the registry's "always loaded" tier (web_search, ask_user, run_code, * search_tools), with the rest of the registry reachable only through `search_tools`. #69 * retired `run_code`, #8 deleted `search_tools` and the tier with it, and these runs never - * get `ask_user` because nobody is there to answer. That leaves `web_search`. + * got `ask_user` because nobody is there to answer — #4 then deleted it for the SDK's own + * question tool, which only the chat engine has. That leaves `web_search`. * * Kept exactly that narrow on purpose. These runs have no approval surface, and deciding * what else they may do belongs to moving them onto the engine, not to deleting two tools. diff --git a/src/lib/runtime/loop.server.ts b/src/lib/runtime/loop.server.ts index cce46fc2..749cd8c6 100644 --- a/src/lib/runtime/loop.server.ts +++ b/src/lib/runtime/loop.server.ts @@ -71,7 +71,6 @@ async function runChatLoopRounds(input: RunChatLoopInput): Promise approvalRequiredTools: ReadonlySet - isOrchestrator: boolean } /** - * Run one planned call through every gate, in order: the offered list, approval, then - * `ask_user` or the registry. A name the run did not offer is refused before anything else, - * so it never reaches an approval card or `executeTool` (see `./offered-tools`). + * Run one planned call through every gate, in order: the offered list, approval, then the + * registry. A name the run did not offer is refused before anything else, so it never reaches + * an approval card or `executeTool` (see `./offered-tools`). */ export async function dispatchToolCall(ctx: DispatchContext, tc: PlannedToolCall): Promise { const { session } = ctx @@ -67,9 +60,6 @@ export async function dispatchToolCall(ctx: DispatchContext, tc: PlannedToolCall const approval = await checkToolApproval(session, tc, ctx.approvalRequiredTools) if (approval.kind === 'denied') return approval.outcome - // ── ask_user (orchestrator-only). Self-contained: emits + pushes block inside. - if (tc.name === 'ask_user') return handleAskUserCall(session, tc, ctx.isOrchestrator) - await session.updateRun({ state: 'running', label: `Executing ${tc.name}`, @@ -170,125 +160,6 @@ export async function checkToolApproval( } } -/** Handle an `ask_user` tool call. Routes through pendingQuestions + awaits answers. */ -export async function handleAskUserCall( - session: Session, - tc: PlannedToolCall, - isOrchestrator: boolean, -): Promise { - if (!isOrchestrator) { - const resultStr = trimToolResult( - tc.name, - JSON.stringify({ - error: - 'Agents cannot ask users directly. Return this question to the orchestrator to gather user input, then resume the agent with those answers.', - }), - ) - await session.emit('tool_result', { - id: tc.id, - name: tc.name, - success: false, - executionMs: 0, - result: resultStr, - }) - await session.pushBlock({ - kind: 'tool', - name: tc.name, - arguments: tc.parsedArgs, - result: { denied: true, reason: 'ask_user is restricted to orchestrator conversations' }, - success: false, - executionMs: 0, - }) - return { - toolResult: { call_id: tc.id, name: tc.name, result: resultStr }, - allToolCallsEntry: { - name: tc.name, - arguments: tc.parsedArgs, - result: { denied: true, reason: 'ask_user is restricted to orchestrator conversations' }, - executionMs: 0, - }, - } - } - - let askInput: ReturnType - try { - askInput = toolSchemas.ask_user.parse(tc.parsedArgs) - } catch { - const errorMessage = 'ask_user received invalid arguments.' - const resultStr = trimToolResult(tc.name, JSON.stringify({ error: errorMessage })) - await session.emit('tool_result', { - id: tc.id, - name: tc.name, - success: false, - executionMs: 0, - result: resultStr, - }) - return { - toolResult: { call_id: tc.id, name: tc.name, result: resultStr }, - allToolCallsEntry: { - name: tc.name, - arguments: tc.parsedArgs, - result: { error: errorMessage }, - executionMs: 0, - }, - } - } - - const questionToken = crypto.randomUUID() - await enqueuePendingQuestion( - session.runId, - { - token: questionToken, - questions: askInput.questions, - requestedAt: new Date().toISOString(), - }, - { state: 'waiting_user_input', label: 'Waiting for user input' }, - ) - await session.emit('ask_user', { - token: questionToken, - id: tc.id, - name: tc.name, - questions: askInput.questions, - }) - const answers = await awaitQuestionAnswers(session.runId, questionToken) - await session.updateRun({ - state: 'running', - label: 'User input received', - heartbeat: true, - }) - - const questionResult = { - questions: askInput.questions, - answers, - timedOut: answers === null, - } - const resultStr = trimToolResult(tc.name, JSON.stringify(questionResult)) - await session.emit('tool_result', { - id: tc.id, - name: tc.name, - success: answers !== null, - executionMs: 0, - result: resultStr, - }) - await session.pushBlock({ - kind: 'tool', - name: tc.name, - arguments: tc.parsedArgs, - result: questionResult, - success: answers !== null, - executionMs: 0, - }) - return { - toolResult: { call_id: tc.id, name: tc.name, result: resultStr }, - allToolCallsEntry: { - name: tc.name, - arguments: tc.parsedArgs, - result: questionResult, - executionMs: 0, - }, - } -} - export type NormalToolContext = { session: Session userId: string diff --git a/src/lib/runtime/types.ts b/src/lib/runtime/types.ts index ad29d181..b82dd771 100644 --- a/src/lib/runtime/types.ts +++ b/src/lib/runtime/types.ts @@ -92,7 +92,7 @@ export type RunChatLoopInput = { maxRounds: number /** Tools requiring explicit user approval (or the wildcard "*"). */ approvalRequiredTools: ReadonlySet - /** True when this is the orchestrator (controls ask_user permission). */ + /** True when this is the orchestrator — the `before_run` hook's `source`. */ isOrchestrator: boolean /** Wave 3 #13 phase 4 — owning agent so per-agent hook config (`agents.config.hooks`) can dispatch. Null for unowned chat runs. */ agentId?: string | null diff --git a/src/lib/tools/handlers/meta.server.ts b/src/lib/tools/handlers/meta.server.ts index 85d73e7c..10358714 100644 --- a/src/lib/tools/handlers/meta.server.ts +++ b/src/lib/tools/handlers/meta.server.ts @@ -2,9 +2,6 @@ * "Meta" tool handlers — tools that operate on the runtime/loop itself rather than * an external resource: * - request_plan_approval: planner→implementer agent handoff (mandatory approval) - * - ask_user: only the chat-stream loop fulfills this; the dispatcher reaches it as a - * defensive fallback (e.g. someone executes the tool directly without going through - * the loop) — return a 'not directly executable' error. * - run_subagent: stateless one-shot LLM call. The chat engine does not register it * (delegation is the SDK's Task tool); the MCP endpoint still lists it. */ @@ -18,17 +15,6 @@ import { logger } from '$lib/observability/logger' import type { ToolHandler } from '../handler-types' export const metaHandlers: Record = { - ask_user: async (call, { startedAt }) => { - const input = toolSchemas.ask_user.parse(call.arguments) - return { - success: false, - tool: call.name, - input, - error: 'ask_user must be handled by chat streaming flow and cannot run directly.', - executionMs: Date.now() - startedAt, - } - }, - request_plan_approval: async (call, { startedAt }) => { const input = toolSchemas.request_plan_approval.parse(call.arguments) // Mandatory-approval tool — by the time the executor runs, the user has approved diff --git a/src/lib/tools/tool-schemas.ts b/src/lib/tools/tool-schemas.ts index 19301898..8ac6036c 100644 --- a/src/lib/tools/tool-schemas.ts +++ b/src/lib/tools/tool-schemas.ts @@ -189,27 +189,6 @@ export const toolSchemas = { additionalDays: z.number().min(0).max(MONITOR_MAX_DEADLINE_DAYS).optional(), additionalChecks: z.number().int().min(0).max(MONITOR_HARD_MAX_CHECKS).optional(), }), - ask_user: z.object({ - questions: z - .array( - z.object({ - header: z.string().min(1), - question: z.string().min(1), - options: z - .array( - z.object({ - label: z.string().min(1), - description: z.string().optional(), - recommended: z.boolean().optional(), - }), - ) - .default([]), - allowFreeformInput: z.boolean().default(true), - }), - ) - .min(1) - .max(8), - }), list_skills: z.object({}), read_skill: z.object({ name: z.string().min(1) }), read_skill_file: z.object({ skillName: z.string().min(1), fileName: z.string().min(1) }), @@ -311,8 +290,6 @@ export const toolDescriptions: Record = { cancel_monitor: 'Cancel a monitor by id. Terminal — it stops being checked immediately and cannot be resumed; create a new one instead.', extend_monitor: 'Push a monitor\'s deadline out and/or top up its check budget. Extension is deliberately explicit — monitors expire on purpose. `additionalDays` is measured from NOW and re-capped at 30 days, so repeated extensions cannot compound into an immortal monitor; `additionalChecks` is added to the existing budget and re-capped. A monitor that expired or exhausted its budget becomes active again if the extension leaves it with both time and budget. A canceled monitor cannot be extended.', - ask_user: - 'Ask the user one or more focused clarifying questions with prefilled answer options. Each question should have ~3 prefilled options — prefer splitting a broad inquiry into multiple focused questions rather than providing many options in a single question. Use when you need explicit user input before proceeding.', list_skills: 'List all available skills with their names, descriptions, and nested file names. Use this to discover what skills are available.', read_skill: diff --git a/src/lib/tools/tools.server.ts b/src/lib/tools/tools.server.ts index d5349018..1a4cc3a5 100644 --- a/src/lib/tools/tools.server.ts +++ b/src/lib/tools/tools.server.ts @@ -133,6 +133,3 @@ export async function executeTool( }, ) } - -export type AskUserQuestion = z.infer['questions'][number] -export type AskUserAnswers = Record diff --git a/src/lib/tools/tools.ts b/src/lib/tools/tools.ts index ea801239..28669534 100644 --- a/src/lib/tools/tools.ts +++ b/src/lib/tools/tools.ts @@ -1,5 +1,5 @@ import { allToolNames, toolDescriptions } from './tool-schemas' -import { ENGINE_EXCLUDED_TOOLS, HOST_OWNED_TOOLS } from '$lib/engine/builtin-tools' +import { ENGINE_EXCLUDED_TOOLS } from '$lib/engine/builtin-tools' type ToolName = string @@ -48,9 +48,10 @@ export function estimateTokens(text: string): number { * can be marked as needing approval, one by one. * * Derived rather than listed: names and descriptions come from `tool-schemas.ts`, and the - * tools the engine does not register (`ENGINE_EXCLUDED_TOOLS`) or never gates - * (`HOST_OWNED_TOOLS` — `ask_user` is the question itself, handed to the host before any - * gate runs) are left out, because a setting that cannot take effect is a false promise. + * tools the engine does not register (`ENGINE_EXCLUDED_TOOLS`) are left out, because a + * setting that cannot take effect is a false promise. The one tool the engine never gates — + * the SDK's `AskUserQuestion`, the question itself (#4) — is not a registry tool, so there is + * nothing to leave out on its account. * * The list used to be grouped into an "always loaded" tier, whose chips could not be * toggled, and a "searchable" tier — both described the old loop's deferred loading, which @@ -62,7 +63,7 @@ export type BuiltinTool = { } export const BUILTIN_TOOLS: BuiltinTool[] = allToolNames - .filter((name) => !ENGINE_EXCLUDED_TOOLS.has(name) && !HOST_OWNED_TOOLS.has(name)) + .filter((name) => !ENGINE_EXCLUDED_TOOLS.has(name)) .map((name) => ({ name, description: toolDescriptions[name] ?? '' })) .sort((a, b) => a.name.localeCompare(b.name)) @@ -70,15 +71,12 @@ export const BUILTIN_TOOLS: BuiltinTool[] = allToolNames * Registry tools that only work inside a chat run, so the MCP endpoint neither lists nor runs * them. `/api/mcp` calls a tool with no run at all, and each of these refuses without one: * - * - `HOST_OWNED_TOOLS` (`ask_user`): the question is answered in the chat's own card; run - * directly, the handler can only say it is not directly executable. * - `MANDATORY_APPROVAL_TOOLS`: they refuse anywhere nobody can press Allow, which is * every call that is not part of an interactive chat run. * - `set_project_context`: it changes the project of the conversation the run belongs to, * and an MCP call belongs to none. */ export const CHAT_RUN_ONLY_TOOLS: ReadonlySet = new Set([ - ...HOST_OWNED_TOOLS, ...MANDATORY_APPROVAL_TOOLS, 'set_project_context', ]) diff --git a/tests/chat.agent-tool-policy.spec.ts b/tests/chat.agent-tool-policy.spec.ts index 1b5478d6..709bcad7 100644 --- a/tests/chat.agent-tool-policy.spec.ts +++ b/tests/chat.agent-tool-policy.spec.ts @@ -76,7 +76,8 @@ test.describe('agent-tool-policy — readOnly policy (Research / Plan built-ins) test('readOnly keeps allow-listed tools (web_search, Read, Write, request_plan_approval)', async () => { const policy = await readOnlyPolicy() const kept = [ - 'ask_user', + // The SDK's question tool (#4), which replaced the in-house `ask_user`. + 'AskUserQuestion', 'Read', 'Write', 'get_pull_request', @@ -172,11 +173,8 @@ test.describe('agent-tool-policy — the real allow-list is not quietly widened' import('../src/lib/engine/builtin-tools'), ]) - const known = new Set([ - ...allToolNames, - ...builtins.BUILTIN_FILE_TOOLS, - ...builtins.BUILTIN_SHELL_TOOLS, - ]) + // The whole built-in set: since #4 the question tool is the SDK's AskUserQuestion. + const known = new Set([...allToolNames, ...builtins.BUILTIN_TOOL_SET]) const unknown = READ_ONLY_TOOL_NAMES.filter((name) => !known.has(name)) expect(unknown, 'allow-listed tools that no longer exist').toEqual([]) }) diff --git a/tests/engine.tool-surface.spec.ts b/tests/engine.tool-surface.spec.ts index e7556890..12382dea 100644 --- a/tests/engine.tool-surface.spec.ts +++ b/tests/engine.tool-surface.spec.ts @@ -61,7 +61,6 @@ test('the exclusions stay narrow — everything else is still exposed', () => { const mustBeExposed = [ 'web_search', 'web_fetch', - 'ask_user', 'create_monitor', 'create_automation', 'push_branch', diff --git a/tests/runs.live.spec.ts b/tests/runs.live.spec.ts index 9b46fd77..5cd9b467 100644 --- a/tests/runs.live.spec.ts +++ b/tests/runs.live.spec.ts @@ -267,7 +267,7 @@ test.describe('runs/live — durable runs through real LLM calls', () => { } }) - test('ask_user flow persists, resolves via endpoint, and emits a gapless event log', async ({ context }) => { + test('an AskUserQuestion persists, resolves via endpoint, and emits a gapless event log (#4)', async ({ context }) => { test.setTimeout(STREAM_TIMEOUT_MS + 30_000) const prefix = uniquePrefix('runs-live-askuser') await cleanupPrefixedRecords(prefix) @@ -276,13 +276,15 @@ test.describe('runs/live — durable runs through real LLM calls', () => { await setApprovalRequiredTools(userId, []) const conversationId = await seedConversationOwnedBy(userId, prefix) - const prompt = `Use the ask_user tool exactly once to ask me which color I prefer with options "red" and "blue". Header should be "Color". Do not write any other text first.` + const prompt = `Use the AskUserQuestion tool exactly once to ask me which color I prefer with options "red" and "blue". Header should be "Color". Do not write any other text first.` const cookie = await buildCookieHeader(context) const abort = new AbortController() try { const body = await postChatStream(cookie, conversationId, prompt, abort.signal) let questionToken: string | null = null + // The SDK takes answers keyed by question text; the frame's `key` carries it. + let questionKey: string | null = null const collected: SseEvent[] = [] await readSseUntil( body, @@ -290,6 +292,8 @@ test.describe('runs/live — durable runs through real LLM calls', () => { const last = events[events.length - 1] if (last?.type === 'ask_user' && typeof last.data.token === 'string') { questionToken = last.data.token + const questions = last.data.questions as Array<{ key?: string }> | undefined + questionKey = questions?.[0]?.key ?? null return true } return false @@ -297,6 +301,7 @@ test.describe('runs/live — durable runs through real LLM calls', () => { { onEvent: (e) => collected.push(e) }, ) expect(questionToken, 'expected ask_user event with token').toBeTruthy() + expect(questionKey, 'expected the question keyed by its text').toBeTruthy() const runMid = await readChatRun(conversationId) expect(runMid.state).toBe('waiting_user_input') @@ -304,7 +309,7 @@ test.describe('runs/live — durable runs through real LLM calls', () => { expect(runMid.pending_questions.find((e) => e.token === questionToken)?.answers).toBeUndefined() const answerRes = await context.request.post(`/chat/${conversationId}/ask-user`, { - data: { token: questionToken, answers: { Color: 'blue' } }, + data: { token: questionToken, answers: { [questionKey!]: 'blue' } }, }) expect(await answerRes.json()).toEqual({ resolved: true }) @@ -316,7 +321,7 @@ test.describe('runs/live — durable runs through real LLM calls', () => { const runFinal = await readChatRun(conversationId) expect(runFinal.pending_questions).toEqual([]) - expect(runFinal.stream_blocks.some((b) => b.kind === 'tool' && b.name === 'ask_user')).toBeTruthy() + expect(runFinal.stream_blocks.some((b) => b.kind === 'tool' && b.name === 'AskUserQuestion')).toBeTruthy() const events = await listRunEvents(runFinal.id) expect(events.map((e) => e.seq)).toEqual(events.map((_, i) => i + 1)) @@ -338,15 +343,15 @@ test.describe('runs/live — durable runs through real LLM calls', () => { const assistantMsg = persisted.find((m) => m.role === 'assistant') expect(assistantMsg).toBeDefined() const persistedBlocks = assistantMsg!.metadata.blocks ?? [] - const askBlock = persistedBlocks.find((b) => b.kind === 'tool' && b.name === 'ask_user') - expect(askBlock, 'ask_user must remain visible as a tool block in the saved message').toBeDefined() + const askBlock = persistedBlocks.find((b) => b.kind === 'tool' && b.name === 'AskUserQuestion') + expect(askBlock, 'the question must remain visible as a tool block in the saved message').toBeDefined() const askArgs = askBlock!.arguments as { questions?: Array<{ header?: string }> } expect(askArgs.questions?.[0]?.header).toBe('Color') - const askResult = askBlock!.result as { answers?: Record; timedOut?: boolean } - expect(askResult.answers).toEqual({ Color: 'blue' }) - expect(askResult.timedOut).toBe(false) + // The answers ride on `details`, distilled from the CLI's tool_use_result. + const askDetails = (askBlock as { details?: { kind?: string; answers?: Record } }).details + expect(askDetails).toEqual({ kind: 'ask_user_question', answers: { [questionKey!]: 'blue' } }) expect(askBlock!.success).toBe(true) // Persisted streamBlocks (run row) and rendered blocks (message row) agree on order. diff --git a/tests/runtime.detached-tools.spec.ts b/tests/runtime.detached-tools.spec.ts index 1cf1545c..5f097c77 100644 --- a/tests/runtime.detached-tools.spec.ts +++ b/tests/runtime.detached-tools.spec.ts @@ -27,11 +27,16 @@ test('allowedTools narrows the list and cannot widen it', () => { expect(detachedRunToolNames(['web_fetch', 'delete_file'])).toEqual([]) }) -test('the retired tools and ask_user are never offered', () => { +test('the retired tools and the question tool are never offered', () => { + // Nobody is there to answer. `ask_user` has left the registry for the SDK's own + // AskUserQuestion (#4), which only the chat engine has; an agent listing either name in + // `allowedTools` still gets neither here. const offered = new Set(detachedRunToolNames()) - for (const name of ['run_code', 'search_tools', 'ask_user']) { + for (const name of ['run_code', 'search_tools', 'ask_user', 'AskUserQuestion']) { expect(offered.has(name), name).toBe(false) } + expect(detachedRunToolNames(['web_search', 'ask_user', 'AskUserQuestion'])).toEqual(['web_search']) + expect(allToolNames as readonly string[]).not.toContain('ask_user') }) test('every name is a real registry tool', () => { diff --git a/tests/runtime.offered-tools.spec.ts b/tests/runtime.offered-tools.spec.ts index e270c0a5..e4def16b 100644 --- a/tests/runtime.offered-tools.spec.ts +++ b/tests/runtime.offered-tools.spec.ts @@ -83,7 +83,6 @@ test.describe('runtime/offered-tools — dispatch refuses before anything runs', projectId: null, offeredTools: offeredToolNames(definitions(opts.offered ?? detachedRunToolNames())), approvalRequiredTools: new Set(opts.approval ?? []), - isOrchestrator: false, }, { id: 't1', name, arguments: '{}', parsedArgs: {} }, ) @@ -119,15 +118,21 @@ test.describe('runtime/offered-tools — dispatch refuses before anything runs', expect(patches).toEqual([]) }) - test('ask_user is refused the same way when the run did not offer it', async () => { - const { outcome } = await dispatch('ask_user') - expect(JSON.parse(outcome.toolResult.result)).toEqual({ error: notOfferedMessage('ask_user') }) + test('a question to the user is refused the same way — nobody is there to answer', async () => { + // The old loop's `ask_user` branch went with the tool (#4); a model that still asks, by + // either name, meets the offered-list gate like any other tool the run did not offer. + for (const name of ['ask_user', 'AskUserQuestion']) { + const { outcome, emits } = await dispatch(name) + expect(JSON.parse(outcome.toolResult.result), name).toEqual({ error: notOfferedMessage(name) }) + expect(emits.map((e) => e.event), name).toEqual(['tool_result']) + } }) test('an offered name passes the gate to the next stage', async () => { - // ask_user offered to a run that is not the orchestrator reaches its own handler, which - // refuses it for its own reason: the gate let it through, and nothing ran. - const { outcome } = await dispatch('ask_user', { offered: ['ask_user'] }) - expect(outcome.toolResult.result).toContain('Agents cannot ask users directly') + // Offered here only to prove the gate lets an offered name through: the registry no + // longer has an `ask_user` (#4), so the next stage answers "unknown tool", and nothing ran. + const { outcome, emits } = await dispatch('ask_user', { offered: ['ask_user'] }) + expect(emits.map((e) => e.event)).toEqual(['tool_call', 'tool_result']) + expect(outcome.toolResult.result).toContain('Unknown tool: ask_user') }) }) diff --git a/tests/settings.tool-approval.spec.ts b/tests/settings.tool-approval.spec.ts index f56bb429..863a8bcb 100644 --- a/tests/settings.tool-approval.spec.ts +++ b/tests/settings.tool-approval.spec.ts @@ -1,6 +1,7 @@ import { expect, test } from '@playwright/test' import { authenticateContext } from './helpers' import { ENGINE_EXCLUDED_TOOLS, HOST_OWNED_TOOLS } from '../src/lib/engine/builtin-tools' +import { ASK_USER_QUESTION_TOOL } from '../src/lib/engine/ask-user-question' import { allToolNames } from '../src/lib/tools/tool-schemas' import { BUILTIN_TOOLS, MANDATORY_APPROVAL_TOOLS } from '../src/lib/tools/tools' @@ -17,8 +18,9 @@ import { BUILTIN_TOOLS, MANDATORY_APPROVAL_TOOLS } from '../src/lib/tools/tools' * - and the "Always loaded" chips were rendered disabled, so `web_search`, which the engine * registers and gates by name, could only be made to ask through the all-tools wildcard. * - * Unlocking the chips must not swap one false promise for another: `ask_user` is handed to - * the host before any approval gate runs, so a tick on it could never take effect. And the + * Unlocking the chips must not swap one false promise for another: a question to the user is + * answered by the user, so no approval setting reaches it — and since #4 it is the SDK's own + * `AskUserQuestion`, not a registry tool, so it has no row to tick at all. And the * mandatory-approval tools ask whatever is stored, so an untick on them could not either. * * The first block is pure. The second opens the page but saves nothing: settings are a @@ -28,20 +30,19 @@ import { BUILTIN_TOOLS, MANDATORY_APPROVAL_TOOLS } from '../src/lib/tools/tools' test.describe('settings/tool-approval — the list is the engine surface', () => { test('it lists exactly the registry tools the engine registers and gates', () => { // `buildToolServer` registers every registry tool outside ENGINE_EXCLUDED_TOOLS for an - // unscoped run, and the engine's gate sees all of them but HOST_OWNED_TOOLS, which - // `runEngineStream` hands to the host before its PreToolUse hook and `canUseTool`. A - // setting for anything else could never take effect. That the engine hands over - // exactly this set is pinned where the engine is driven: engine.stream-approvals. - const gated = allToolNames - .filter((name) => !ENGINE_EXCLUDED_TOOLS.has(name) && !HOST_OWNED_TOOLS.has(name)) - .sort() + // unscoped run, and the engine's gate sees all of them. The one call it hands to the + // host ungated, AskUserQuestion (HOST_OWNED_TOOLS), is the SDK's and not in the + // registry, so there is nothing to leave out on its account. That the engine gates + // every registry tool is pinned where the engine is driven: engine.stream-approvals. + const gated = allToolNames.filter((name) => !ENGINE_EXCLUDED_TOOLS.has(name)).sort() expect(BUILTIN_TOOLS.map((t) => t.name)).toEqual(gated) - expect(HOST_OWNED_TOOLS.has('ask_user')).toBe(true) + expect([...HOST_OWNED_TOOLS]).toEqual([ASK_USER_QUESTION_TOOL]) + for (const name of HOST_OWNED_TOOLS) expect(allToolNames as readonly string[], name).not.toContain(name) }) test('no entry promises a tool that is gone, hidden or never gated', () => { const names = new Set(BUILTIN_TOOLS.map((t) => t.name)) - for (const gone of ['run_code', 'search_tools', 'run_subagent', 'ask_user']) { + for (const gone of ['run_code', 'search_tools', 'run_subagent', 'ask_user', 'AskUserQuestion']) { expect(names.has(gone), gone).toBe(false) } expect(names.has('web_search')).toBe(true) diff --git a/tests/tools.mcp-surface.spec.ts b/tests/tools.mcp-surface.spec.ts index ad1d8ffd..04c99723 100644 --- a/tests/tools.mcp-surface.spec.ts +++ b/tests/tools.mcp-surface.spec.ts @@ -10,7 +10,8 @@ import { CHAT_RUN_ONLY_TOOLS, MANDATORY_APPROVAL_TOOLS, mcpExposedToolNames } fr * The endpoint calls a tool with no chat run behind it. It used to list the whole registry, * so it advertised tools that always refuse without a run (#69 triage, step 0.3): `ask_user` * ("must be handled by chat streaming flow"), the mandatory-approval tools (nobody to press - * Allow) and `set_project_context` (no conversation to bind). + * Allow) and `set_project_context` (no conversation to bind). `ask_user` has since left the + * registry altogether, for the SDK's own AskUserQuestion (#4), which MCP never had. * * The first block is pure. The second calls the endpoint; nothing it calls writes. */ @@ -18,15 +19,17 @@ import { CHAT_RUN_ONLY_TOOLS, MANDATORY_APPROVAL_TOOLS, mcpExposedToolNames } fr test.describe('mcp surface — which registry tools are offered', () => { test('the chat-run-only tools are named, and are real registry tools', () => { expect([...CHAT_RUN_ONLY_TOOLS].sort()).toEqual( - ['ask_user', 'create_pull_request', 'push_branch', 'request_plan_approval', 'set_project_context'].sort(), + ['create_pull_request', 'push_branch', 'request_plan_approval', 'set_project_context'].sort(), ) // A stale name here would quietly re-expose the renamed tool. for (const name of CHAT_RUN_ONLY_TOOLS) expect(allToolNames, name).toContain(name) }) - test('they are derived from the engine and approval sets, not copied', () => { - for (const name of HOST_OWNED_TOOLS) expect(CHAT_RUN_ONLY_TOOLS.has(name), name).toBe(true) + test('they are derived from the approval set, not copied', () => { for (const name of MANDATORY_APPROVAL_TOOLS) expect(CHAT_RUN_ONLY_TOOLS.has(name), name).toBe(true) + // The engine's one host-owned call is the SDK's AskUserQuestion — not a registry tool, + // so the endpoint never listed it and has nothing to leave out. + for (const name of HOST_OWNED_TOOLS) expect(allToolNames as readonly string[], name).not.toContain(name) }) test('everything else in the registry stays exposed', () => { @@ -64,13 +67,16 @@ test.describe('mcp surface — the endpoint', () => { test('tools/call refuses them as invalid params, before any handler runs', async ({ page }) => { await authenticateContext(page.context()) - for (const name of ['ask_user', 'push_branch', 'set_project_context']) { + for (const name of ['push_branch', 'set_project_context']) { const body = await rpc(page, 'tools/call', { name, arguments: {} }) expect(body.error?.code, name).toBe(-32602) expect(body.error?.message, name).toContain('only runs inside a chat conversation') } - const unknown = await rpc(page, 'tools/call', { name: 'no_such_tool', arguments: {} }) - expect(unknown.error?.code).toBe(-32602) - expect(unknown.error?.message).toContain('Unknown tool') + // A retired name is simply unknown now. + for (const name of ['no_such_tool', 'ask_user']) { + const unknown = await rpc(page, 'tools/call', { name, arguments: {} }) + expect(unknown.error?.code, name).toBe(-32602) + expect(unknown.error?.message, name).toContain('Unknown tool') + } }) }) From 70b34be72c49beafad99741b084a3865021cd5ce Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 20:19:22 -0600 Subject: [PATCH 3/8] feat(chat): rebuild the question card around AskUserQuestion (#4) The card now shows each question with its header chip, option cards (label, description, a Recommended badge in place of the model's "(Recommended)" suffix), a preview pane for the option under the pointer or keyboard, multi-select, and an automatic free-text Other. The same AskUserQuestionCard serves the inline card, the modal a reloaded page opens and the /review inbox; the click rules and the answer string (labels in option order, then Other, comma-separated) live in the pure $lib/engine/ask-user-question so the specs reach them. Previews are model HTML, so they only ever render in an + diff --git a/src/lib/chat/AskUserQuestionCard.svelte b/src/lib/chat/AskUserQuestionCard.svelte index c02f95f2..c65c014a 100644 --- a/src/lib/chat/AskUserQuestionCard.svelte +++ b/src/lib/chat/AskUserQuestionCard.svelte @@ -1,77 +1,160 @@ -
- {#if question.options.length > 0} -
+
+
+ {#if question.header} + + {question.header} + + {/if} + {#if multi} + Choose any that apply + {/if} +
+ +

{question.question}

+ +
+
{#each question.options as option (option.label)} + {@const picked = selection.selected.includes(option.label)} {/each} + + {#if allowOther} +
+ + +
+ {/if}
- {/if} - {#if question.allowFreeformInput ?? true} -
- -
- {/if} + {#if shown?.preview} +
+ +
+ {/if} +
diff --git a/src/lib/chat/MessageBlocks.svelte b/src/lib/chat/MessageBlocks.svelte index e0b595f5..a129ed4d 100644 --- a/src/lib/chat/MessageBlocks.svelte +++ b/src/lib/chat/MessageBlocks.svelte @@ -7,11 +7,11 @@ import ThinkingBlockCard from './ThinkingBlockCard.svelte'; import TodoListCard from './TodoListCard.svelte'; import ToolCallCard from './ToolCallCard.svelte'; + import SavedAskUserBlock from './SavedAskUserBlock.svelte'; + import { isAskUserToolName } from '$lib/engine/ask-user-question'; import { askQuestionAlreadyInMessage, blockHasRenderableOutput, - getAskUserAnswer, - getAskUserQuestions, type SavedBlock, } from './message-bubble-helpers'; @@ -21,7 +21,7 @@ * `MessageBubble.svelte` so the bubble can be a thin layout shell. * * Block-kind dispatch: - * - tool/ask_user → inline question + (if available) the answer bubble + * - tool/AskUserQuestion (or a retired ask_user) → the question + its answer (SavedAskUserBlock) * - tool + details → the card for that shape (diff / terminal / todo) * - tool (other) → ToolCallCard * - notice → RunNoticeCard @@ -30,7 +30,7 @@ * - text → rendered markdown * * `messageId` is used to compose stable per-block keys; `messageContent` is - * the raw assistant text (used to suppress redundant ask_user prompts that + * the raw assistant text (used to suppress redundant question prompts that * already appear in the message body). */ @@ -58,25 +58,8 @@ {#each blocks as block, idx (`${messageId}-block-${idx}`)} - {#if block.kind === 'tool' && block.name === 'ask_user'} - {@const askQuestions = getAskUserQuestions(block.arguments, block.result)} - {#if askQuestions.length > 0} - {#each askQuestions as q} - {#if !askQuestionAlreadyHere(q.question ?? q.header)} -
-
{@html renderMarkdown(q.question ?? q.header)}
-
- {/if} - {@const answer = getAskUserAnswer(block.result, q.header, askQuestions.map((question) => question.header))} - {#if answer} -
-
-

{answer}

-
-
- {/if} - {/each} - {/if} + {#if block.kind === 'tool' && isAskUserToolName(block.name)} + {:else if block.kind === 'tool' && block.details?.kind === 'file_edit'}
@@ -89,7 +72,7 @@
- {:else if block.kind === 'tool' && block.name !== 'ask_user'} + {:else if block.kind === 'tool'}
0}
{#each normalizedToolCalls as call, idx (`${message.id}-${idx}`)} - {#if call.name === 'ask_user'} + {#if isAskUserToolName(call.name)} {@const askQuestions = getAskUserQuestions(call.arguments, call.result)} {#if askQuestions.length > 0} {#each askQuestions as q} diff --git a/src/lib/chat/SavedAskUserBlock.svelte b/src/lib/chat/SavedAskUserBlock.svelte new file mode 100644 index 00000000..7e3bf601 --- /dev/null +++ b/src/lib/chat/SavedAskUserBlock.svelte @@ -0,0 +1,41 @@ + + +{#each exchanges as exchange, i (i)} + {#if !alreadyShown(exchange.question)} +
+
{@html renderMarkdown(exchange.question)}
+
+ {/if} + {#if exchange.answer} +
+
+

{exchange.answer}

+
+
+ {/if} +{/each} +{#if unanswered && exchanges.length > 0} +

Not answered.

+{/if} diff --git a/src/lib/chat/ask-user-answers.ts b/src/lib/chat/ask-user-answers.ts index a3f2356b..fffa4662 100644 --- a/src/lib/chat/ask-user-answers.ts +++ b/src/lib/chat/ask-user-answers.ts @@ -1,11 +1,13 @@ /** - * Reading the user's answers back out of an `ask_user` tool result (#81). + * Reading the user's answers back out of a question block's result (#81). * - * The host hands the model its answers as plain text, one `Header: answer` per question - * (`fulfilAskUser` in the chat stream). That text is also what the transcript records as the - * call's result, live and after a reload. The cards only understood a JSON - * `{ answers: { … } }` result, which nothing produces any more, so an answered question never - * showed its answer. Both shapes are read here; JSON wins when it is there. + * The retired `ask_user` host handed the model its answers as plain text, one + * `Header: answer` per question, and that text is what those transcripts record as the call's + * result. The SDK's AskUserQuestion (#4) carries its answers on the block's `details` + * instead (`./tool-block-helpers`), but a card answered a moment ago still holds the JSON + * `{ answers }` the chat recorded, and old transcripts are full of the text form. The cards + * once understood only the JSON form, so an answered `ask_user` question never showed its + * answer. Both shapes are read here; JSON wins when it is there. * * No imports on purpose: the transcript helpers load this in the plain Playwright loader too. */ diff --git a/src/lib/chat/message-bubble-helpers.ts b/src/lib/chat/message-bubble-helpers.ts index 4e3cc01b..a09bd45b 100644 --- a/src/lib/chat/message-bubble-helpers.ts +++ b/src/lib/chat/message-bubble-helpers.ts @@ -15,6 +15,8 @@ import type { ToolResultDetails } from '../engine/tool-result-details' import type { RunNotice } from '../engine/sdk-notices' import { readAskUserAnswers } from './ask-user-answers' +import { getAskUserAnswersFromTool, getAskUserQuestionsFromTool } from './tool-block-helpers' +import { answerKey } from '../engine/ask-user-question' export type SavedBlock = | { kind: 'text'; content: string } @@ -115,6 +117,33 @@ export function getAskUserAnswer(resultValue: unknown, header: string, headers: return trimmed.length > 0 ? trimmed : null } +/** + * A saved question block as the transcript shows it: each question, and the answer under it + * when there is one. Reads an AskUserQuestion block (#4) — answers on its `details`, keyed by + * question text — and a retired `ask_user` block, whose answers are in its result. + */ +export function savedAskUserExchanges(block: { + name: string + arguments: unknown + result: unknown + details?: unknown +}): Array<{ question: string; answer: string | null }> { + const toText = (value: unknown) => + value === undefined || value === null ? null : typeof value === 'string' ? value : JSON.stringify(value) + const like = { + name: block.name, + arguments: toText(block.arguments) ?? '', + result: toText(block.result), + details: block.details, + } + const questions = getAskUserQuestionsFromTool(like) + const answers = getAskUserAnswersFromTool(like) + return questions.map((question) => { + const answer = answers?.[answerKey(question)]?.trim() + return { question: question.question || question.header, answer: answer ? answer : null } + }) +} + /** Lowercase + collapse whitespace for the dedupe-text comparison. */ export function normalizeText(value: string): string { return value.toLowerCase().replace(/\s+/g, ' ').trim() diff --git a/src/lib/chat/streaming-blocks.ts b/src/lib/chat/streaming-blocks.ts index 2cf4efb0..a2da94eb 100644 --- a/src/lib/chat/streaming-blocks.ts +++ b/src/lib/chat/streaming-blocks.ts @@ -10,6 +10,7 @@ */ import { parseJsonFallback } from '$lib/chat/tool-block-helpers' +import { ASK_USER_QUESTION_TOOL, LEGACY_ASK_USER_TOOL, isAskUserToolName } from '../engine/ask-user-question' import type { ToolResultDetails } from '../engine/tool-result-details' import type { RunNotice } from '../engine/sdk-notices' @@ -693,15 +694,18 @@ export function applyToolResult( const finalStatus = payload.success ? ('completed' as const) : ('failed' as const) const resultText = payload.result ?? (payload.success ? 'Success' : 'Tool execution failed') /* - * #81 — an ask_user card is keyed by the host's answer token (the `ask_user` frame) and - * its result by the SDK's tool_use id, so the two never met: the card stayed open with its - * Submit button and an empty block was appended instead. The result belongs to the oldest - * card still waiting for one — a card whose id is still its token — and the card takes the - * SDK's id with it, so a replayed result finds it directly. + * #81 — a retired `ask_user` card was keyed by the host's answer token (the `ask_user` + * frame) and its result by the SDK's tool_use id, so the two never met: the card stayed + * open with its Submit button and an empty block was appended instead. The result belongs + * to the oldest card still waiting for one — a card whose id is still its token — and the + * card takes the SDK's id with it, so a replayed result finds it directly. + * + * An AskUserQuestion card (#4) is opened under the tool_use id itself, so its result finds + * it by id like any other call's. */ const askUserCard = - payload.name === 'ask_user' && !blocks.some((b) => b.kind === 'tool' && b.id === payload.id) - ? blocks.findIndex((b) => b.kind === 'tool' && b.name === 'ask_user' && !!b.token && b.id === b.token) + payload.name === LEGACY_ASK_USER_TOOL && !blocks.some((b) => b.kind === 'tool' && b.id === payload.id) + ? blocks.findIndex((b) => b.kind === 'tool' && b.name === LEGACY_ASK_USER_TOOL && !!b.token && b.id === b.token) : -1 const idx = askUserCard !== -1 ? askUserCard : blocks.findIndex((b) => b.kind === 'tool' && b.id === payload.id) if (idx === -1) { @@ -728,10 +732,11 @@ export function applyToolResult( } } const existing = blocks[idx] - // An answered ask_user card is already marked completed by `applyAskUserAnswered`. + // An answered question card is already marked completed by `applyAskUserAnswered`. const unexpected = askUserCard === -1 && existing.kind === 'tool' && + !isAskUserToolName(existing.name) && existing.status !== 'executing' && existing.status !== 'approved' return { @@ -753,9 +758,9 @@ export function applyToolResult( } /** - * The server recorded the user's answers to an ask_user card (#81): show it answered now, + * The server recorded the user's answers to a question card (#81): show it answered now, * rather than with its Submit button still live until the call's `tool_result` arrives. The - * card keeps its token as its id, so that result still finds it (`applyToolResult`). + * card keeps its id, so that result still finds it (`applyToolResult`). */ export function applyAskUserAnswered( blocks: StreamingBlock[], @@ -763,7 +768,7 @@ export function applyAskUserAnswered( answers: Record, ): StreamingBlock[] { return blocks.map((b) => - b.kind === 'tool' && b.name === 'ask_user' && b.token === token + b.kind === 'tool' && isAskUserToolName(b.name) && b.token === token ? { ...b, status: 'completed' as const, result: JSON.stringify({ answers }) } : b, ) @@ -773,6 +778,9 @@ export function applyAskUserAnswered( * `ask_user` event — append a synthetic executing tool block carrying the * questions so the AskUserCard can render inline. Skips the append when a tool * block with the same id already exists (idempotent re-emit). + * + * The frame names the tool: `AskUserQuestion`, with the SDK's tool_use id as its id (#4), or + * `ask_user` from a run that predates it. A frame that names none is the former. */ export function applyAskUser( blocks: StreamingBlock[], @@ -788,7 +796,7 @@ export function applyAskUser( { kind: 'tool' as const, id: payload.id, - name: payload.name ?? 'ask_user', + name: payload.name ?? ASK_USER_QUESTION_TOOL, arguments: askUserArgs, status: 'executing' as const, expanded: true, diff --git a/src/lib/chat/tool-block-helpers.ts b/src/lib/chat/tool-block-helpers.ts index efcdf825..1bec1660 100644 --- a/src/lib/chat/tool-block-helpers.ts +++ b/src/lib/chat/tool-block-helpers.ts @@ -5,32 +5,46 @@ * `result` coming out). These helpers shape that raw payload into the * structured forms the UI cards expect, with permissive fallbacks so a * partially-formed block still renders something useful. + * + * Relative imports only: `./message-bubble-helpers` imports this, and specs load both in the + * plain Playwright loader, where the SvelteKit alias is not guaranteed to resolve. */ -export type AskUserOption = { - label: string - description?: string - recommended?: boolean -} +// Re-export so call sites that already import `parseJsonFallback` keep working; +// the shared implementation lives in `$lib/util/json` (parseJsonRecord — also +// guards against non-object JSON values like null or arrays). +import { parseJsonRecord as parseJsonFallback } from '../util/json' +import { readAskUserAnswers } from './ask-user-answers' +import { + LEGACY_ASK_USER_TOOL, + answerKey, + isAskUserToolName, + readAskQuestions, + type AskOption, + type AskQuestion, +} from '../engine/ask-user-question' +export { parseJsonFallback } -export type AskUserQuestion = { - header: string - question: string - options: AskUserOption[] - allowFreeformInput?: boolean -} +/** + * A question card's question: the SDK's AskUserQuestion (#4) or a retired `ask_user` one. + * Both read into one shape — see `$lib/engine/ask-user-question`. + */ +export type AskUserOption = AskOption +export type AskUserQuestion = AskQuestion +export { isAskUserToolName } type ToolBlockLike = { + /** `AskUserQuestion`, or `ask_user` for a block from before #4. Absent reads as the latter. */ + name?: string arguments: string result?: string | null + /** The call's distilled result; an AskUserQuestion's carries its answers. */ + details?: unknown } -// Re-export so call sites that already import `parseJsonFallback` keep working; -// the shared implementation lives in `$lib/util/json` (parseJsonRecord — also -// guards against non-object JSON values like null or arrays). -import { parseJsonRecord as parseJsonFallback } from '$lib/util/json' -import { readAskUserAnswers } from './ask-user-answers' -export { parseJsonFallback } +function isLegacyBlock(block: ToolBlockLike): boolean { + return block.name === undefined || block.name === LEGACY_ASK_USER_TOOL +} export function getAskUserQuestionsFromTool(block: ToolBlockLike): AskUserQuestion[] { const args = parseJsonFallback(block.arguments) @@ -38,34 +52,31 @@ export function getAskUserQuestionsFromTool(block: ToolBlockLike): AskUserQuesti const fromArgs = Array.isArray(args.questions) ? args.questions : [] const fromResult = Array.isArray(result.questions) ? result.questions : [] const source = fromArgs.length > 0 ? fromArgs : fromResult + return readAskQuestions(source, { legacy: isLegacyBlock(block) }) +} - return source - .map((entry) => { - const row = (entry ?? {}) as Record - const header = typeof row.header === 'string' ? row.header : '' - const question = typeof row.question === 'string' ? row.question : header - const options = Array.isArray(row.options) - ? (row.options as Array>) - .map((opt) => ({ - label: typeof opt.label === 'string' ? opt.label : '', - description: typeof opt.description === 'string' ? opt.description : undefined, - recommended: typeof opt.recommended === 'boolean' ? opt.recommended : undefined, - })) - .filter((opt) => opt.label.length > 0) - : [] - const allowFreeformInput = - typeof row.allowFreeformInput === 'boolean' ? row.allowFreeformInput : true - return { header, question, options, allowFreeformInput } - }) - .filter((row) => row.question.trim().length > 0) +/** The answers on an AskUserQuestion block's distilled result, keyed by question text. */ +function detailsAnswers(details: unknown): Record | null { + const record = details && typeof details === 'object' ? (details as { kind?: unknown; answers?: unknown }) : null + if (record?.kind !== 'ask_user_question' || !record.answers || typeof record.answers !== 'object') return null + const out: Record = {} + for (const [key, value] of Object.entries(record.answers as Record)) { + if (typeof value === 'string' && value.trim()) out[key] = value + } + return Object.keys(out).length > 0 ? out : null } /** - * The answers recorded on an ask_user block's result, or null while it has none. Reads the - * host's `Header: answer` text as well as a JSON `{ answers }` object — see `./ask-user-answers`. + * The answers recorded on a question block, keyed like its questions (`answerKey`), or null + * while it has none. An AskUserQuestion block carries them on its `details` once the call's + * result is in; before that — the moment the server records them — as a JSON `{ answers }` + * result. A retired `ask_user` block also reads the host's `Header: answer` text; see + * `./ask-user-answers`. */ export function getAskUserAnswersFromTool(block: ToolBlockLike): Record | null { + const fromDetails = detailsAnswers(block.details) + if (fromDetails) return fromDetails if (!block.result) return null - const headers = getAskUserQuestionsFromTool(block).map((question) => question.header) - return readAskUserAnswers(block.result, headers) + const keys = getAskUserQuestionsFromTool(block).map((question) => answerKey(question)) + return readAskUserAnswers(block.result, keys) } diff --git a/src/lib/observability/review.remote.ts b/src/lib/observability/review.remote.ts index 9fd2c903..4b83a004 100644 --- a/src/lib/observability/review.remote.ts +++ b/src/lib/observability/review.remote.ts @@ -91,13 +91,15 @@ export const decideApprovalReviewItemCommand = command(decideApprovalSchema, asy const answerQuestionSchema = z.object({ itemId: z.string().uuid(), - // Keyed by question header, as the chat's answer card sends them. + // Keyed as the answer card sends them: by question text for the SDK's AskUserQuestion (#4), + // by header for a question the retired `ask_user` left open. A question is a sentence, so + // the key may be longer than a header ever was. answers: z - .record(z.string().trim().min(1).max(200), z.string().trim().min(1).max(4000)) + .record(z.string().trim().min(1).max(2000), z.string().trim().min(1).max(4000)) .refine((answers) => Object.keys(answers).length > 0, 'Answer at least one question'), }) -/** Answer, from /review, the questions a paused run asked with ask_user. */ +/** Answer, from /review, the questions a paused run asked (AskUserQuestion, #4). */ export const answerQuestionReviewItemCommand = command(answerQuestionSchema, async (input) => { const user = requireAuthenticatedRequestUser() const { answerQuestionFromReview } = await import('$lib/runs/review-decisions.server') diff --git a/src/routes/chat/[id]/+page.svelte b/src/routes/chat/[id]/+page.svelte index c665158b..96cd7671 100644 --- a/src/routes/chat/[id]/+page.svelte +++ b/src/routes/chat/[id]/+page.svelte @@ -43,6 +43,7 @@ parseJsonFallback, getAskUserQuestionsFromTool, getAskUserAnswersFromTool, + isAskUserToolName, type AskUserOption, type AskUserQuestion, } from '$lib/chat/tool-block-helpers'; @@ -1007,7 +1008,7 @@ } if (eventName === 'tool_result') { - if (payload.name === 'ask_user') { + if (isAskUserToolName(payload.name)) { pendingAskUser = null; askUserModalOpen = false; } @@ -1153,7 +1154,7 @@ const fullText = getPartialText(streamingBlocks); const completedToolCalls = getCompletedToolCalls(streamingBlocks); const hasAskUserTool = completedToolCalls.some( - (call) => String(call.name ?? '') === 'ask_user' + (call) => isAskUserToolName(call.name) ); if (!hasAskUserTool && (fullText.trim() || completedToolCalls.length > 0)) { pendingAssistantDrafts = [ @@ -1626,7 +1627,7 @@ --> {:else if streaming && !pendingMessageId} {#each streamingBlocks as block (block.id)} - {#if block.kind === 'tool' && block.name === 'ask_user'} + {#if block.kind === 'tool' && isAskUserToolName(block.name)} {@const askQuestions = getAskUserQuestionsFromTool(block)} {@const askAnswers = getAskUserAnswersFromTool(block)} {@const askLive = block.status === 'pending' || block.status === 'approved' || block.status === 'executing'} @@ -1652,7 +1653,7 @@ {:else if block.kind === 'notice'} - {:else if block.kind === 'tool' && block.name !== 'ask_user'} + {:else if block.kind === 'tool' && !isAskUserToolName(block.name)} >; @@ -107,14 +108,8 @@ } } - type QuestionPayload = { - questions?: Array<{ - header: string; - question: string; - options: Array<{ label: string; description?: string; recommended?: boolean }>; - allowFreeformInput?: boolean; - }>; - }; + /** As the chat run recorded them (`chat_runs.pending_questions`) — previews and multi-select included (#4). */ + type QuestionPayload = { questions?: AskQuestion[] }; function fmtDate(d: Date | string) { return new Date(d).toLocaleString(); diff --git a/tests/chat.ask-user-question-card.spec.ts b/tests/chat.ask-user-question-card.spec.ts new file mode 100644 index 00000000..a6529290 --- /dev/null +++ b/tests/chat.ask-user-question-card.spec.ts @@ -0,0 +1,368 @@ +import { randomUUID } from 'node:crypto' +import { expect, test, type Locator } from '@playwright/test' +import { authenticateContext, cleanupPrefixedRecords, getActiveUserId, getSql, seedConversation, uniquePrefix } from './helpers' +import { openAndSend, scriptHeldRun } from './chat-stream-script' +import { readAskQuestions } from '../src/lib/engine/ask-user-question' + +/** + * #4 — the question card, rebuilt around the SDK's AskUserQuestion: a header chip, option + * cards (label, description, recommended badge), a sandboxed HTML preview of the option in + * focus, multi-select, and a free-text "Other" that is always there. + * + * Every surface a question reaches is driven here: the live card in a turn (scripted — no + * model), the saved transcript after a reload, the modal a reloaded page opens for a question + * still waiting, and the /review inbox. Answers are keyed by question text throughout — + * what the SDK takes back. + */ + +const LAYOUT = { + question: 'Which layout should the dashboard use?', + header: 'Layout', + multiSelect: false, + options: [ + { label: 'Sidebar (Recommended)', description: 'Navigation down the left edge', preview: '
side
' }, + { label: 'Top bar', description: 'Navigation across the top', preview: '
top
' }, + { label: 'No navigation', description: 'A single page' }, + ], +} + +const FEATURES = { + question: 'Which features should it ship with?', + header: 'Features', + multiSelect: true, + options: [ + { label: 'Search', description: 'Full-text search' }, + { label: 'Export', description: 'CSV export' }, + { label: 'Share', description: 'Share links' }, + ], +} + +/** The questions as the chat host records and sends them. */ +const PENDING = readAskQuestions({ questions: [LAYOUT, FEATURES] }) + +/** Guards the layout bug mobile keeps catching: a flex child collapsing to zero width. */ +async function expectReadableWidth(locator: Locator) { + const box = await locator.boundingBox() + expect(box?.width ?? 0).toBeGreaterThan(24) +} + +test.describe('the live question card', () => { + test('chip, recommended badge, sandboxed preview, multi-select and Other — answered by question text', async ({ page }) => { + test.setTimeout(90_000) + const prefix = uniquePrefix('chat-auq-live') + await cleanupPrefixedRecords(prefix) + await authenticateContext(page.context()) + const conversation = await seedConversation(prefix, { userId: await getActiveUserId() }) + const runId = randomUUID() + const token = `${runId}:ask:toolu_auq1` + const { release } = await scriptHeldRun(page, conversation.id, [ + { id: 1, event: 'context_stats', data: { runId, tokenEstimate: 10, contextWindow: 200_000 } }, + { id: 2, event: 'ask_user', data: { id: 'toolu_auq1', name: 'AskUserQuestion', token, questions: PENDING } }, + ]) + const posted: Array<{ token: string; answers: Record }> = [] + await page.route( + (url) => url.pathname === `/chat/${conversation.id}/ask-user`, + (route) => { + posted.push(route.request().postDataJSON()) + return route.fulfill({ json: { resolved: true } }) + }, + ) + + try { + await openAndSend(page, conversation.id, `${prefix} build it`) + const card = page.getByRole('main').locator('.ask-user-card').first() + await expect(card.getByText(LAYOUT.question)).toBeVisible({ timeout: 30_000 }) + + // Header chip, and a counter because there are two questions. + await expect(card.locator('.ask-question__chip')).toHaveText('Layout') + await expect(card.getByText('Question 1/2')).toBeVisible() + + // "(Recommended)" is shown as a badge, not as part of the label. + const sidebar = card.getByRole('button', { name: /^Sidebar/ }) + await expect(sidebar).toContainText('Recommended') + await expect(card).not.toContainText('(Recommended)') + await expectReadableWidth(sidebar.getByText('Sidebar', { exact: true })) + await expectReadableWidth(card.getByText('Navigation down the left edge')) + + // The recommended option's preview shows first — in a frame with no permissions. + const frame = card.locator('iframe') + await expect(frame).toHaveAttribute('sandbox', '') + await expect(frame).toHaveAttribute('srcdoc', /side-preview/) + await expect(frame).toHaveAttribute('srcdoc', /Content-Security-Policy/) + await expectReadableWidth(frame) + // The preview's markup never reaches the page itself. + await expect(page.locator('#side-preview')).toHaveCount(0) + + // Choosing an option shows its preview. + await card.getByRole('button', { name: /^Top bar/ }).click() + await expect(card.getByRole('button', { name: /^Top bar/ })).toHaveAttribute('aria-pressed', 'true') + await expect(frame).toHaveAttribute('srcdoc', /top-preview/) + + await card.getByRole('button', { name: 'Next', exact: true }).click() + + // The multi-select question: no previews, so no frame. + await expect(card.locator('.ask-question__chip')).toHaveText('Features') + await expect(card.getByText('Choose any that apply')).toBeVisible() + await expect(card.locator('iframe')).toHaveCount(0) + const submit = card.getByRole('button', { name: 'Submit', exact: true }) + await expect(submit).toBeDisabled() + await card.getByRole('button', { name: /^Share/ }).click() + await card.getByRole('button', { name: /^Search/ }).click() + await card.getByPlaceholder('Type your own answer').fill('Dark mode') + await expect(card.getByRole('button', { name: /^Search/ })).toHaveAttribute('aria-pressed', 'true') + await expect(card.getByRole('button', { name: /^Share/ })).toHaveAttribute('aria-pressed', 'true') + await expect(submit).toBeEnabled() + await submit.click() + + await expect.poll(() => posted.length).toBe(1) + expect(posted[0]).toEqual({ + token, + answers: { [LAYOUT.question]: 'Top bar', [FEATURES.question]: 'Search, Share, Dark mode' }, + }) + + // Shown answered at once, each answer under its question. + await expect(card.locator('.user-bubble', { hasText: 'Search, Share, Dark mode' })).toBeVisible() + await expect(card.locator('.user-bubble', { hasText: /^Top bar$/ })).toBeVisible() + await expect(card.getByRole('button', { name: 'Submit' })).toHaveCount(0) + } finally { + release() + await page.unrouteAll({ behavior: 'ignoreErrors' }) + await cleanupPrefixedRecords(prefix) + } + }) + + test("a single-select question answered with Other sends the text, and the call's result closes the card", async ({ page }) => { + test.setTimeout(90_000) + const prefix = uniquePrefix('chat-auq-other') + await cleanupPrefixedRecords(prefix) + await authenticateContext(page.context()) + const conversation = await seedConversation(prefix, { userId: await getActiveUserId() }) + const runId = randomUUID() + const token = `${runId}:ask:toolu_auq2` + const [layout] = PENDING + const { release } = await scriptHeldRun(page, conversation.id, [ + { id: 1, event: 'context_stats', data: { runId, tokenEstimate: 10, contextWindow: 200_000 } }, + { id: 2, event: 'ask_user', data: { id: 'toolu_auq2', name: 'AskUserQuestion', token, questions: [layout] } }, + ]) + const posted: Array<{ answers: Record }> = [] + await page.route( + (url) => url.pathname === `/chat/${conversation.id}/ask-user`, + (route) => { + posted.push(route.request().postDataJSON()) + return route.fulfill({ json: { resolved: true } }) + }, + ) + + try { + await openAndSend(page, conversation.id, `${prefix} lay it out`) + const card = page.getByRole('main').locator('.ask-user-card').first() + await expect(card.getByText(LAYOUT.question)).toBeVisible({ timeout: 30_000 }) + // One question: no counter. + await expect(card.getByText(/Question \d\/\d/)).toHaveCount(0) + + await card.getByRole('button', { name: /^Top bar/ }).click() + // Typing in Other replaces the option on a single-select question. + await card.getByPlaceholder('Type your own answer').fill('Tabs along the bottom') + await expect(card.getByRole('button', { name: /^Top bar/ })).toHaveAttribute('aria-pressed', 'false') + await card.getByRole('button', { name: 'Submit', exact: true }).click() + await expect.poll(() => posted.length).toBe(1) + expect(posted[0].answers).toEqual({ [LAYOUT.question]: 'Tabs along the bottom' }) + await expect(card.locator('.user-bubble', { hasText: 'Tabs along the bottom' })).toBeVisible() + } finally { + release() + await page.unrouteAll({ behavior: 'ignoreErrors' }) + await cleanupPrefixedRecords(prefix) + } + }) + + test("an answer that arrives only as the call's result still shows on the card", async ({ page }) => { + test.setTimeout(90_000) + const prefix = uniquePrefix('chat-auq-result') + await cleanupPrefixedRecords(prefix) + await authenticateContext(page.context()) + const conversation = await seedConversation(prefix, { userId: await getActiveUserId() }) + const runId = randomUUID() + const [layout] = PENDING + const { release } = await scriptHeldRun(page, conversation.id, [ + { id: 1, event: 'context_stats', data: { runId, tokenEstimate: 10, contextWindow: 200_000 } }, + { id: 2, event: 'ask_user', data: { id: 'toolu_auq3', name: 'AskUserQuestion', token: `${runId}:ask:toolu_auq3`, questions: [layout] } }, + // Answered from /review or another tab: the engine's result, under the same id, with + // the answers distilled from the CLI's tool_use_result. + { + id: 3, + event: 'tool_result', + data: { + id: 'toolu_auq3', + name: 'AskUserQuestion', + success: true, + result: `User has answered your questions: "${LAYOUT.question}"="No navigation".`, + details: { kind: 'ask_user_question', answers: { [LAYOUT.question]: 'No navigation' } }, + }, + }, + ]) + + try { + await openAndSend(page, conversation.id, `${prefix} lay it out`) + const main = page.getByRole('main') + await expect(main.getByText(LAYOUT.question).first()).toBeVisible({ timeout: 30_000 }) + await expect(main.locator('.user-bubble', { hasText: /^No navigation$/ }).first()).toBeVisible() + await expect(main.getByRole('button', { name: 'Submit' })).toHaveCount(0) + } finally { + release() + await page.unrouteAll({ behavior: 'ignoreErrors' }) + await cleanupPrefixedRecords(prefix) + } + }) +}) + +test('a saved AskUserQuestion shows its question and answer after a reload; an unanswered one says so', async ({ page }) => { + test.setTimeout(60_000) + const prefix = uniquePrefix('chat-auq-saved') + await cleanupPrefixedRecords(prefix) + await authenticateContext(page.context()) + const conversation = await seedConversation(prefix, { userId: await getActiveUserId() }) + // As the engine saves them: the model's input as arguments, the CLI's text as the result, + // and the answers on `details`. + const blocks = [ + { + kind: 'tool', + name: 'AskUserQuestion', + arguments: { questions: [LAYOUT, FEATURES] }, + result: `User has answered your questions: "${LAYOUT.question}"="Top bar", "${FEATURES.question}"="Search, Export".`, + success: true, + executionMs: 0, + details: { kind: 'ask_user_question', answers: { [LAYOUT.question]: 'Top bar', [FEATURES.question]: 'Search, Export' } }, + }, + { + kind: 'tool', + name: 'AskUserQuestion', + arguments: { questions: [{ question: `${prefix} which colour?`, header: 'Colour', multiSelect: false, options: [{ label: 'Red', description: 'r' }, { label: 'Blue', description: 'b' }] }] }, + result: 'The user did not answer this question before the run stopped waiting.', + success: false, + executionMs: 0, + }, + { kind: 'text', content: `${prefix} built it` }, + ] + const sql = getSql() + await sql` + insert into messages (conversation_id, role, content, metadata, tool_calls, sequence) + values (${conversation.id}, 'assistant', ${`${prefix} built it`}, ${sql.json({ blocks })}, '[]'::jsonb, 3) + ` + + try { + await page.goto('/', { waitUntil: 'domcontentloaded' }) + await page.goto(`/chat/${conversation.id}`, { waitUntil: 'domcontentloaded' }) + const main = page.getByRole('main') + await expect(main.getByText(`${prefix} built it`).first()).toBeVisible({ timeout: 30_000 }) + await expect(main.getByText(LAYOUT.question).first()).toBeVisible() + await expect(main.locator('.user-bubble', { hasText: /^Top bar$/ }).first()).toBeVisible() + await expect(main.locator('.user-bubble', { hasText: /^Search, Export$/ }).first()).toBeVisible() + await expect(main.getByText(`${prefix} which colour?`).first()).toBeVisible() + await expect(main.getByText('Not answered.').first()).toBeVisible() + // A saved question is never a live form. + await expect(main.getByRole('button', { name: 'Submit' })).toHaveCount(0) + } finally { + await cleanupPrefixedRecords(prefix) + } +}) + +test('a question still waiting after a reload opens in the modal and is answered by question text', async ({ page }) => { + test.setTimeout(60_000) + const prefix = uniquePrefix('chat-auq-resume') + await cleanupPrefixedRecords(prefix) + await authenticateContext(page.context()) + const sql = getSql() + const userId = await getActiveUserId() + const conversation = await seedConversation(prefix, { userId, userMessage: `${prefix} pick features` }) + const token = `${randomUUID()}:ask:toolu_auq4` + const [, features] = PENDING + const [run] = await sql<{ id: string }[]>` + insert into chat_runs (conversation_id, user_id, state, source, label, pending_questions) + values ( + ${conversation.id}, ${userId}, 'waiting_user_input', 'chat_stream', ${`${prefix} run`}, + ${sql.json([{ token, questions: [features], requestedAt: new Date().toISOString() }])} + ) + returning id + ` + + try { + await page.goto('/', { waitUntil: 'domcontentloaded' }) + await page.goto(`/chat/${conversation.id}`, { waitUntil: 'domcontentloaded' }) + await page.getByText(`${prefix} pick features`, { exact: true }).waitFor({ state: 'visible', timeout: 30_000 }) + + await expect(page.getByText(FEATURES.question).first()).toBeVisible({ timeout: 10_000 }) + await expect(page.locator('.ask-question__chip', { hasText: 'Features' }).first()).toBeVisible() + await page.getByRole('button', { name: /^Export/ }).first().click() + await page.getByRole('button', { name: /^Search/ }).first().click() + + const answered = page.waitForResponse( + (r) => r.url().includes(`/chat/${conversation.id}/ask-user`) && r.status() === 200, + { timeout: 15_000 }, + ) + await page.getByRole('button', { name: /^Submit$/ }).first().click() + expect((await (await answered).json()).resolved).toBe(true) + + const [row] = await sql<{ pending_questions: Array<{ token: string; answers?: Record }> }[]>` + select pending_questions from chat_runs where id = ${run.id} + ` + const entry = row.pending_questions.find((e) => e.token === token) + // Multi-select answers in option order, keyed by the question text the SDK takes back. + expect(entry?.answers).toEqual({ [FEATURES.question]: 'Search, Export' }) + } finally { + await cleanupPrefixedRecords(prefix) + } +}) + +test('the /review inbox shows the same card — previews sandboxed — and its answer reaches the run', async ({ page }) => { + test.setTimeout(60_000) + const prefix = uniquePrefix('review-auq') + await cleanupPrefixedRecords(prefix) + await authenticateContext(page.context()) + const sql = getSql() + const userId = await getActiveUserId() + const conversation = await seedConversation(prefix, { userId }) + const token = `${randomUUID()}:ask:toolu_auq5` + const [layout] = PENDING + const [run] = await sql<{ id: string }[]>` + insert into chat_runs (conversation_id, user_id, state, source, label, pending_questions) + values ( + ${conversation.id}, ${userId}, 'waiting_user_input', 'chat_stream', ${`${prefix} run`}, + ${sql.json([{ token, questions: [layout], requestedAt: new Date().toISOString() }])} + ) + returning id + ` + const { openReviewItem } = await import('../src/lib/observability/review.server') + await openReviewItem({ + type: 'user_question', + summary: `${prefix} Agent asked: ${LAYOUT.question}`, + payload: { token, questions: [layout] }, + runId: run.id, + dedupeKey: `question:${token}`, + }) + + try { + await page.goto('/review', { waitUntil: 'domcontentloaded' }) + await page.getByText(`${prefix} Agent asked`).first().click({ timeout: 30_000 }) + const card = page.locator('.ask-user-card').first() + await expect(card.locator('.ask-question__chip')).toHaveText('Layout') + await expect(card.locator('iframe')).toHaveAttribute('sandbox', '') + await expect(card.locator('iframe')).toHaveAttribute('srcdoc', /side-preview/) + await expect(page.locator('#side-preview')).toHaveCount(0) + await expectReadableWidth(card.getByText('Navigation across the top')) + + await card.getByRole('button', { name: /^No navigation/ }).click() + await card.getByRole('button', { name: 'Submit', exact: true }).click() + + await expect + .poll(async () => { + const [row] = await sql<{ pending_questions: Array<{ token: string; answers?: Record }> }[]>` + select pending_questions from chat_runs where id = ${run.id} + ` + return row.pending_questions.find((e) => e.token === token)?.answers ?? null + }) + .toEqual({ [LAYOUT.question]: 'No navigation' }) + } finally { + await sql`delete from review_items where summary like ${`${prefix}%`}` + await cleanupPrefixedRecords(prefix) + } +}) From 65d46c64328484802ed1b706391860ff60e110aa Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 20:19:22 -0600 Subject: [PATCH 4/8] docs: questions from the agent are the SDK's AskUserQuestion (#4) The chat spec describes the new card, how and where a question is answered, what happens when nobody answers, who may ask, why previews are safe, and what was verified in the SDK rather than assumed. Runs, tools, settings, hooks, notifications, observability, UI and structure docs drop ask_user; the README says what the card does; the parity doc gains a row and the re-audit marks #4 shipped. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 ++- docs/chat/spec.md | 36 ++++++++++++++++++--- docs/hooks/spec.md | 2 +- docs/notifications/spec.md | 4 +-- docs/observability/spec.md | 2 +- docs/runs/spec.md | 11 +++++-- docs/runtime/spec.md | 2 +- docs/settings/spec.md | 2 +- docs/structure/boundaries.md | 2 +- docs/structure/claude-desktop-parity.md | 1 + docs/structure/parity-reaudit-2026-09-22.md | 32 ++++++++++++++++-- docs/structure/spec.md | 2 +- docs/tools/spec.md | 2 +- docs/tools/tools.md | 7 ++-- docs/ui/spec.md | 8 ++--- 15 files changed, 90 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index b9947a7b..93323cff 100644 --- a/README.md +++ b/README.md @@ -8,9 +8,11 @@ Self-hosted autonomous AI agent platform for a single owner, with a sandboxed wo AgentStudio provides a streaming chat interface where the assistant can call tools such as web search and sandboxed code execution. The filesystem toolset supports ranged file reads, full writes, unified-diff patch apply, deterministic string replace, recursive directory listing, search, move/rename, delete, and file metadata lookups. Chat supports editing and branching, interleaved tool and thinking blocks, per-message performance and cost metrics, model selection, and per-prompt reasoning effort selection. +When the agent needs a decision it asks with the Agent SDK's own AskUserQuestion, shown as a question card: a header chip, option cards with a recommended badge, a sandboxed HTML preview of each option where one helps, multi-select, and a free-text "Other". It can be answered in the chat, from the composer, or from the /review inbox, and never answers itself. See [docs/chat/spec.md](docs/chat/spec.md#questions-from-the-agent). + Replies can be read aloud: a speaker button on each reply plays it through an OpenRouter text-to-speech model (code blocks are skipped), and an opt-in, per-device **Auto-read** switch above the message box reads each new reply when its turn finishes, for hands-free use. The model and voice are set in Settings → Model & AI; spend is recorded in the usage ledger under "Read Aloud" and counts toward budget limits. See [docs/speech/speech.md](docs/speech/speech.md). -Creation workflows are chat-led: New Agent and New Skill actions launch a fresh conversation with a seeded creation prompt. The assistant gathers missing requirements (optionally with ask_user), then executes directly with tool-level approvals where configured. +Creation workflows are chat-led: New Agent and New Skill actions launch a fresh conversation with a seeded creation prompt. The assistant gathers missing requirements (optionally with a question card), then executes directly with tool-level approvals where configured. ### Agents diff --git a/docs/chat/spec.md b/docs/chat/spec.md index 61d9f37a..bf1157e5 100644 --- a/docs/chat/spec.md +++ b/docs/chat/spec.md @@ -192,7 +192,35 @@ On mobile, the right panel collapses into a bottom sheet tab drawer. ### Inline approvals and answers -Approval requests and `ask_user` questions render inline in the thread, but are also reflected in the global Review Inbox. Resolving from either place updates the same durable state. +Approval requests and the agent's questions render inline in the thread, but are also reflected in the global Review Inbox. Resolving from either place updates the same durable state. + +### Questions from the agent + +When the agent needs a decision from you before it carries on, it asks with **AskUserQuestion** — the Agent SDK's own question tool, which replaced AgentStudio's home-made `ask_user` in September 2026 (#4). A call holds one to four questions. + +**What the card shows.** Each question gets: + +| Part | What it is | +| --- | --- | +| Header chip | A short label for the question ("Layout", "Auth method") | +| Question | The full question, in a sentence | +| Option cards | Two to four choices, each with a label and a line saying what it means. The one the agent recommends carries a **Recommended** badge | +| Preview | For options that are easier to compare by eye (a layout, a snippet, a configuration), a small rendered picture of what the choice produces. It shows the option under your pointer or keyboard, otherwise the one you chose, otherwise the recommended one. Beside the options on a wide screen, below them on a phone | +| Other | Always there: type your own answer instead of — or, on a "choose any" question, as well as — the options | + +Some questions say **Choose any that apply**: tick as many options as you like. The rest take one answer, and typing in Other replaces the option you picked. With several questions the card steps through them (Question 1/3, Next, Submit), and Submit waits until every question has an answer. + +**How you answer.** In the card, in the chat composer (typing a reply while a question is waiting answers it), or from the /review inbox, which shows the same card. Whichever comes first counts. Once the server has recorded it, the card shows each question with your answer under it, and so does the conversation after a reload. + +**If nobody answers.** The question waits five minutes, like a tool approval. After a minute you get a "Needs input" notification. After five, the agent is told nobody answered and that it must not assume an answer — it carries on only with what does not depend on it, or ends its turn and says what it needs. Stopping the run settles the question at once. A question never answers itself: the SDK's own idle auto-continue (`askUserQuestionTimeout`) is set to `never`, because on a box that runs while you sleep an auto-picked option is a decision you never saw. + +**Who can ask.** Only the agent in a chat. A delegated subagent cannot — it is told to put the question in its result so the agent that delegated to it can ask. Automations, monitors and CI-fix runs have no one to ask and are never offered the tool; if the model tries anyway the call is refused, and the run does not wait. + +**Previews are safe to show.** A preview is HTML the model wrote, and the model may be repeating text from a web page or a repository. So it is never put into the page. It is shown in a sealed frame that cannot run scripts, cannot reach the app or your session, cannot submit forms or open windows, and cannot load anything from the internet — an image pointing at someone's server is simply not fetched. Tags that could act on the frame itself are shown as text. A preview over 12,000 characters is dropped. + +**What was checked in the SDK** (`@anthropic-ai/claude-agent-sdk` 0.3.278, bundled CLI 2.1.278), rather than assumed: `toolConfig.askUserQuestion.previewFormat` exists and is set to `html` (the default is markdown, for a terminal); `askUserQuestionTimeout` is a Settings field, reached through the `settings` option; the tool always asks permission, so each call reaches the app's permission callback, which answers it by returning the call's input with the answers added (question text to answer, several choices comma-separated); and the tool is only switched on when a permission callback is installed, which the chat engine always does. The CLI also reads back and reports the answers it was given, which is what the saved transcript shows. + +Transcripts from before the change keep their `ask_user` blocks, and those still show each question with its answer. ### Diff and artifact preview @@ -241,7 +269,7 @@ How the controls that exist today behave: - **Coming back to a running turn.** Opening a conversation whose turn is still running — after a reload, or from another tab — shows that turn streaming again, with its tool and approval cards and the Stop button. Text written before you came back appears once the turn finishes. - **One turn at a time.** A message sent while a turn is still running is not sent. The page says so, keeps the message for Retry, and shows the running turn instead. - **Allow / Deny.** An approval card only shows a call as approved or denied once the server has recorded the answer. If it could not be recorded (the approval timed out, or was answered in another tab), the card keeps its buttons and says why. -- **Answering a question.** When the agent asks a question (`ask_user`), the answer only counts once the server has recorded it. If the question is no longer waiting (it timed out, was answered in another tab, or its turn ended), the page says so and shows the conversation as the server has it, rather than closing the question as if the answer had gone through. An answered question shows the answer under it straight away, and again after a reload. It no longer keeps a live Submit button that does nothing. +- **Answering a question.** When the agent asks a question (see [Questions from the agent](#questions-from-the-agent)), the answer only counts once the server has recorded it. If the question is no longer waiting (it timed out, was answered in another tab, or its turn ended), the page says so and shows the conversation as the server has it, rather than closing the question as if the answer had gone through. An answered question shows the answer under it straight away, and again after a reload. It no longer keeps a live Submit button that does nothing. - **Switching conversations mid-turn.** Opening another conversation while a reply is streaming shows only the other conversation. Nothing from the first one comes along: not its reply, its tool cards, its Stop button, its error message or its Retry. Leaving is not a Stop. The first turn keeps running, saves its own reply, and shows again with Stop when you go back to it. - **A turn that ends in an error.** Some turns end in an error after the reply was already saved, for example when the agent reaches its maximum number of steps or the model provider is overloaded. The page shows the error and keeps the one saved reply. It used to save a second, partial copy of the same reply. - **Background tasks.** A command the agent starts in the background (a dev server, a watcher) shows as a chip in the header while the turn runs, with a button to stop it. The chips go away when the turn's stream ends, because ending a turn also ends the commands it started. If a stop does not work, a short message under the header says why — for example that the turn had already ended. @@ -272,7 +300,7 @@ Every assistant reply has a speaker button beside Copy. It reads the reply throu ### How replies are displayed safely -Assistant replies, thinking, subagent results and `ask_user` questions are written by the model, and the model may be repeating text it picked up from a web page, a repository or a tool result. Someone who plants instructions there can try to make the model write HTML that would run inside the app, or an image link that quietly sends data to their server the moment the reply is shown. So replies are displayed as formatted markdown, but under these rules: +Assistant replies, thinking, subagent results and the agent's questions are written by the model, and the model may be repeating text it picked up from a web page, a repository or a tool result. Someone who plants instructions there can try to make the model write HTML that would run inside the app, or an image link that quietly sends data to their server the moment the reply is shown. So replies are displayed as formatted markdown, but under these rules: | Content in a reply | What the reader sees | | ------------------ | -------------------- | @@ -324,7 +352,7 @@ This domain follows [../ui/spec.md](../ui/spec.md) and defines the primary app-s - Surfaces: session list (with agent filter, project grouping, and expandable run tree), chat thread canvas, composer, mode selector, live run HUD, inline action cards, and mode-aware right panel tabs. - States and badges: running, blocked, needs-input, queued interjection, completed, failed, and pending approvals count. -- Blocking actions: plan approvals, tool approvals, and ask_user responses must resolve through durable review items. +- Blocking actions: plan approvals, tool approvals, and answers to the agent's questions must resolve through durable review items. - Mobile behavior: right panel collapses to bottom-sheet tabs; blocking cards remain visible near composer; session tree uses progressive disclosure to avoid deep nested panes. ## References diff --git a/docs/hooks/spec.md b/docs/hooks/spec.md index ce4258fa..3d108b1a 100644 --- a/docs/hooks/spec.md +++ b/docs/hooks/spec.md @@ -53,7 +53,7 @@ Not every event below is raised yet. Which ones fire today, on the chat path and | `on_evaluator` | `runId`, `verdict`, `findings` | On evaluator result | | `on_subagent_spawn` | `parentRunId`, `childRunId`, `agentId` | On `run_subagent` call | | `on_approval_required` | `runId`, `toolName`, `args`, `token` | On pending approval | -| `on_user_question` | `runId`, `questions`, `token` | On `ask_user` call | +| `on_user_question` | `runId`, `questions`, `token` | When the agent asks a question (AskUserQuestion) | | `on_run_failed` | `runId`, `error` | On run error | | `on_skill_loaded` | `runId`, `skillSlug`, `loadKind` | When a skill summary or body is loaded | | `on_tool_output_archived` | `runId`, `toolName`, `handle`, `wasSummarized` | When large tool output is offloaded | diff --git a/docs/notifications/spec.md b/docs/notifications/spec.md index 16e428e5..4978db7c 100644 --- a/docs/notifications/spec.md +++ b/docs/notifications/spec.md @@ -55,7 +55,7 @@ Notification preferences are stored in `appSettings.notificationPrefs` and contr | Key | Switch in Settings | What it sends today | | --------------- | ------------------ | ------------------- | | `taskCompleted` | Task completed | A research run finished its report | -| `needsInput` | Needs input | A chat run has been waiting for a tool approval or an answer to an `ask_user` question for a minute. Not straight away: someone watching the chat answers in seconds. The run gives up after five minutes, so this leaves about four to answer. The notification links to the chat | +| `needsInput` | Needs input | A chat run has been waiting for a tool approval or an answer to one of the agent's questions for a minute. Not straight away: someone watching the chat answers in seconds. The run gives up after five minutes, so this leaves about four to answer. The notification links to the chat | | `agentErrors` | Agent errors | An automation run failed for good, or a check failed on a pull request an agent opened | Two kinds of notification are sent whatever these switches say, because the user set each one up on purpose and turns it off where they set it up: @@ -87,7 +87,7 @@ All notifications are persisted to the `notifications` table regardless of wheth - Mark individual or all as read - Notifications older than 90 days are soft-hidden (not deleted) -The inbox is the fallback for users without push subscriptions. For approvals and `ask_user` blocks, the chat session UI is the primary surface — the notification is a secondary signal for users who are not actively watching the chat. +The inbox is the fallback for users without push subscriptions. For approvals and the agent's questions, the chat session UI is the primary surface — the notification is a secondary signal for users who are not actively watching the chat. ## Web Push Setup diff --git a/docs/observability/spec.md b/docs/observability/spec.md index 5ea6b9f3..d34ecec2 100644 --- a/docs/observability/spec.md +++ b/docs/observability/spec.md @@ -66,7 +66,7 @@ Time-series metrics storage. Rolled up from run events, job logs, and hook invoc | Item type | Severity | Triggered by | | ------------------------- | -------- | ----------------------------------------------------------- | | `approval_request` | critical | Tool call requires human approval | -| `user_question` | critical | `ask_user` tool call waiting for answer | +| `user_question` | critical | A question from the agent (AskUserQuestion) waiting for an answer — answerable from the inbox with the same card as the chat | | `evaluation_failure` | warning | Evaluator returned `fail` verdict | | `job_failure` | warning | Job exhausted all retry attempts | | `job_stuck` | warning | Job whose worker kept dying mid-run, out of attempts | diff --git a/docs/runs/spec.md b/docs/runs/spec.md index f694fbda..1a8190eb 100644 --- a/docs/runs/spec.md +++ b/docs/runs/spec.md @@ -21,7 +21,7 @@ A run is the durable, resumable record of one agent execution attempt. Every mes | `model` | string | Model slug used | | `state` | enum | `pending`, `running`, `awaiting_approval`, `awaiting_answer`, `completed`, `failed`, `canceled` | | `pendingApprovals` | jsonb[] | Outstanding tool approval requests (token, toolName, args, requestedAt) | -| `pendingQuestions` | jsonb[] | Outstanding `ask_user` requests (token, questions, requestedAt) | +| `pendingQuestions` | jsonb[] | Outstanding questions from the agent (token, questions, requestedAt, then answers and decidedAt) | | `streamBlocks` | jsonb[] | Incremental content blocks appended as the loop runs | | `currentRound` | integer | Tool loop round counter for resume | | `cursor` | jsonb | Loop resume pointer (last persisted message index, last tool call) | @@ -115,11 +115,16 @@ The chat's Allow / Deny card can appear a moment before step 1 has happened. An ### Pending user questions -`ask_user` follows the same pattern as approvals, written to `runs.pendingQuestions`, resolved via the answer endpoint. +A question from the agent (the SDK's AskUserQuestion, #4) follows the same pattern as approvals: written to `runs.pendingQuestions`, resolved via the answer endpoint or /review, and the run moves to `waiting_user_input` while it waits. + +- Each entry keeps the questions as the card shows them — header, question text, options with descriptions, the recommended marker, any HTML previews, and whether several choices are allowed — so a reloaded page and the /review inbox show the same card. It is JSON, so none of this needed a migration. +- Answers are recorded keyed by each question's text, which is what the SDK takes back. An entry left by the retired `ask_user` is keyed by its header instead, and still reads. +- The token is `:ask:`, one per question call. +- The run waits five minutes, then the agent is told nobody answered. A Stop ends the wait at once and closes the question's inbox item. ### Approvals and questions in the review inbox -Each pending approval and each `ask_user` question also opens an item in the /review inbox, so a user who is not watching the chat can find it (and, after a minute, gets a "Needs input" notification). The item can be answered from /review — Approve, Deny, or the answer card — with the same effect as answering in the chat. +Each pending approval and each question from the agent also opens an item in the /review inbox, so a user who is not watching the chat can find it (and, after a minute, gets a "Needs input" notification). The item can be answered from /review — Approve, Deny, or the answer card — with the same effect as answering in the chat. The item closes by itself when the prompt is settled: answered in either place (resolved, with who answered), timed out after five minutes (dismissed), or left behind by a run that ended or was reaped (dismissed by the check that runs with the reaper every five minutes). Before 2026-09-23 these items were never closed, and answering one in /review did not reach the run. diff --git a/docs/runtime/spec.md b/docs/runtime/spec.md index 36daf988..53657b34 100644 --- a/docs/runtime/spec.md +++ b/docs/runtime/spec.md @@ -57,7 +57,7 @@ A stateful event bus for one run. The Session is the only thing that touches SSE | `getMessages` | () => Promise\ | Loads current message history | | `appendMessage` | (m) => Promise\ | Persists a message | | `pendingApproval` | (req) => Promise\ | Blocks until user approves or denies a tool call | -| `pendingQuestion` | (req) => Promise\ | Blocks until user answers via `ask_user` | +| `pendingQuestion` | (req) => Promise\ | Retired with `ask_user` (#4): these runs are unattended and never ask; chat questions are the SDK's AskUserQuestion | ## Features diff --git a/docs/settings/spec.md b/docs/settings/spec.md index 384d3f9a..c85f1b32 100644 --- a/docs/settings/spec.md +++ b/docs/settings/spec.md @@ -72,7 +72,7 @@ Each limit is enforced as a budget limit (see [../cost/spec.md](../cost/spec.md) } ``` -The Tool Approval panel lists every AgentStudio tool a chat can call, and each one can be ticked on its own, except three that always ask: `push_branch`, `create_pull_request` and `request_plan_approval`. Those show ticked, marked "always asks", and cannot be unticked, and the **All** and **None** buttons leave them alone, because they ask for approval in every mode whatever is stored. `ask_user` is left off, because it is a question to you rather than an action, and no approval setting ever reaches it. The panel's old "Always loaded" and "Searchable" groups and its "Programmatic tool calling" switch are gone, because none of them did anything in a chat (#8, #69). A stored row may still carry `programmaticToolCallingEnabled`; nothing reads it, and the next save drops it. See [../tools/tools.md](../tools/tools.md). +The Tool Approval panel lists every AgentStudio tool a chat can call, and each one can be ticked on its own, except three that always ask: `push_branch`, `create_pull_request` and `request_plan_approval`. Those show ticked, marked "always asks", and cannot be unticked, and the **All** and **None** buttons leave them alone, because they ask for approval in every mode whatever is stored. Questions from the agent are not on the list: they are the SDK's own AskUserQuestion rather than an AgentStudio tool (#4), and a question to you is not an action, so no approval setting ever reaches it. The panel's old "Always loaded" and "Searchable" groups and its "Programmatic tool calling" switch are gone, because none of them did anything in a chat (#8, #69). A stored row may still carry `programmaticToolCallingEnabled`; nothing reads it, and the next save drops it. See [../tools/tools.md](../tools/tools.md). **`memoryConfig`** diff --git a/docs/structure/boundaries.md b/docs/structure/boundaries.md index da676ce7..4021ecf9 100644 --- a/docs/structure/boundaries.md +++ b/docs/structure/boundaries.md @@ -21,7 +21,7 @@ The narrow LLM-provider surface. Only this module knows OpenRouter exists. ### `src/lib/runtime/` The pre-engine agent loop. Interactive chat runs on the Claude Agent SDK (`src/lib/engine/`); what still runs here is unattended — automations with an agent attached, a monitor's start_conversation, and CI fix runs (#8). - `loop.server.ts` — `runChatLoop()`. Drives streaming, multi-round tool execution and approval gates. Calls `streamChat()` from `llm/`. -- `tool-handlers.server.ts` — per-tool-call dispatch (ask_user, normal tool). Owns the emit / pushBlock / approval-await flow. +- `tool-handlers.server.ts` — per-tool-call dispatch: the offered-list gate, approval, then the registry. Owns the emit / pushBlock / approval-await flow. (Its `ask_user` branch went with the tool, #4 — these runs are unattended and were never offered it.) - `types.ts` — `Session`, `RunChatLoopInput`, `LoopMessage`. The contract the three callers implement. - `agent-definition.server.ts` — slot assembly + workspace context resolution. - `detached-tools.ts` — the short tool list an unattended run is offered (`web_search`). diff --git a/docs/structure/claude-desktop-parity.md b/docs/structure/claude-desktop-parity.md index dd4c58de..56e48c81 100644 --- a/docs/structure/claude-desktop-parity.md +++ b/docs/structure/claude-desktop-parity.md @@ -71,6 +71,7 @@ This is the Cowork comparison, and it is the one I got wrong in the first draft: | Web search + fetch | **even** | `web_search`, `web_fetch`, `pdf_read` | same | | Code execution | **n/a** — removed (#69) | `run_code` never ran on the engine path and was deleted rather than rebuilt. The agent writes a script into the run's workspace and runs it with the SDK's `Bash`, sandboxed by bubblewrap in production and approval-gated where bubblewrap is missing. No in-script tool calls, no chart output — a chart is a file shown in the Preview tab | analysis tool / sandboxed Python, renders charts | | File attachments | **even** (#36 fixed) | images inline as base64 content blocks on the SDK's streaming-input prompt; PDFs and other files are staged into the run's sandbox workspace and read with `pdf_read` / `file_read`; anything undeliverable (video, oversized or unsupported images) warns on the message instead of being dropped; files attached on the new-chat page go with the first message (they were silently dropped until 2026-09-23) | images, PDFs, office docs, with extraction | +| Agent asks you a question | **even** (#4 landed) | the SDK's own AskUserQuestion, shown as a card with a header chip, option cards and a recommended badge, sandboxed HTML previews of each option, multi-select and a free-text Other; answerable in the chat, the composer or the /review inbox, and it never answers itself (`askUserQuestionTimeout: 'never'`). Until #4 it was a home-made `ask_user` with no previews or multi-select | the same tool, with previews and an idle auto-continue | | Voice dictation | **even** | record → `/api/transcribe`; no live transcript while speaking | same | | Text-to-speech | **even** (#27 fixed) | speaker button on every reply and an opt-in, per-device auto-read for hands-free use, through an OpenRouter speech model and voice chosen in Settings; code is skipped, long replies are chunked, spend is on the ledger and under budget limits | shipped | | Deep research | **even** | in a chat, the Research agent writes a plan the user must approve, then hands it to Chat, which does the research as ordinary turns with the web tools — steerable, but no cited-report page or notification. The background pipeline (planner, fetch, reflection, cited report, notification) runs only from research-mode automations, and it is fixed: a run cannot be steered once started. The composer's Research button that would start one is not switched on. (Until 2026-09-23 every background run failed at its first model call — the bare SDK model id was sent to OpenRouter — and would have failed again at its last, marking the cited sources; a canceled run was retried to completion, or saved its report over a Cancel pressed during the write-up.) | agentic, steerable | diff --git a/docs/structure/parity-reaudit-2026-09-22.md b/docs/structure/parity-reaudit-2026-09-22.md index cfb214b1..aa03c3ea 100644 --- a/docs/structure/parity-reaudit-2026-09-22.md +++ b/docs/structure/parity-reaudit-2026-09-22.md @@ -253,7 +253,7 @@ plan), **fold** (belongs inside another issue), **delete** (close it). | #17 | Connect external MCP servers | **as filed** | plumbing confirmed trivial; the policy layer is the actual work | | #32 | Multi-agent orchestration | **rebuild** | use SDK `agents` + the Task tool instead of a bespoke fan-out tool | | #5 | Port subagents to SDK subagents | **as filed** — shipped | keystone; `Options.agents` + `Task`, `run_subagent` retired | -| #4 | Native AskUserQuestion | **as filed** | `toolConfig.askUserQuestion.previewFormat` confirmed present | +| #4 | Native AskUserQuestion | **as filed** — shipped | the SDK's tool answered through `canUseTool`; HTML previews, multi-select, Other, timeout `never` | | #18 | Conversation pin/archive/search/export | **as filed**, trimmed | all four are cheap; make archive the default action, not delete | | #22 | Slash commands and `@`-mentions | **split** | build `@` now; `/` should wait for `settingSources` | | #38 | Usage digest | **rebuild** | fix the ledger first, then ship the header strip; the digest agent is the last 20% | @@ -477,6 +477,34 @@ timeout: `never` is the right default for a self-hosted single-user box where th may be asleep — an auto-continue that picks an option unattended is exactly the failure this app's `/review` inbox exists to avoid. +**Shipped (#4).** The in-house `ask_user` is gone — schema, handler, the old loop's branch and +the engine's special cases — and the SDK's own AskUserQuestion is on for every chat run: +`toolConfig.askUserQuestion.previewFormat: 'html'` and `settings.askUserQuestionTimeout: +'never'` (a Settings field, not an Option). What the installed SDK (0.3.278, CLI 2.1.278) was +read for rather than assumed: + +- the tool's own permission check always asks, so every call reaches `canUseTool`; the host + answers by allowing with `updatedInput.answers` (question text → answer, multi-select + comma-separated), which the CLI's `call()` reads back and echoes as + `tool_use_result.answers`; +- the CLI only enables the tool in SDK mode when a permission prompt tool is set, which + `canUseTool` always provides (`--permission-prompt-tool stdio`); +- in `html` mode the CLI rejects previews that are whole documents or carry + ` @@ -123,7 +134,18 @@ ? pickedClasses : idleClasses}" > -
diff --git a/src/lib/engine/ask-user-question.ts b/src/lib/engine/ask-user-question.ts index 8a15bf27..7d331fef 100644 --- a/src/lib/engine/ask-user-question.ts +++ b/src/lib/engine/ask-user-question.ts @@ -198,19 +198,29 @@ export function toggleOption(question: AskQuestion, selection: AskSelection, lab return { ...selection, selected } } -/** Type into "Other". On a single-select question that is choosing it over the options. */ +/** + * Type into "Other". On a single-select question, text there is choosing it over the options; + * an edit that leaves the box blank does not take away an option the user chose. + */ export function writeOther(question: AskQuestion, selection: AskSelection, text: string): AskSelection { const otherChosen = text.trim().length > 0 || (!question.multiSelect && selection.otherChosen) return { - selected: question.multiSelect ? selection.selected : [], + selected: question.multiSelect || !otherChosen ? selection.selected : [], other: text, otherChosen, } } -/** Focus "Other": a single-select question drops its option, so the answer is what gets typed. */ -export function focusOther(question: AskQuestion, selection: AskSelection): AskSelection { - if (question.multiSelect) return selection +/** + * Click "Other" itself: a single-select question drops its option, so the answer is what gets + * typed; a multi-select one toggles it, like any other checkbox. + * + * Only an explicit click does this — never focus. "Other" sits between the options and + * Submit in tab order, so choosing it on focus cleared a keyboard user's option on their way + * to Submit and left the question unanswerable from the keyboard. + */ +export function chooseOther(question: AskQuestion, selection: AskSelection): AskSelection { + if (question.multiSelect) return { ...selection, otherChosen: !selection.otherChosen } return { selected: [], other: selection.other, otherChosen: true } } diff --git a/tests/chat.ask-user-question-card.spec.ts b/tests/chat.ask-user-question-card.spec.ts index a6529290..3d8e0f19 100644 --- a/tests/chat.ask-user-question-card.spec.ts +++ b/tests/chat.ask-user-question-card.spec.ts @@ -160,11 +160,26 @@ test.describe('the live question card', () => { // One question: no counter. await expect(card.getByText(/Question \d\/\d/)).toHaveCount(0) - await card.getByRole('button', { name: /^Top bar/ }).click() - // Typing in Other replaces the option on a single-select question. + const topBar = card.getByRole('button', { name: /^Top bar/ }) + const other = card.getByRole('button', { name: 'Other', exact: true }) + const submit = card.getByRole('button', { name: 'Submit', exact: true }) + await topBar.click() + // Clicking "Other" itself chooses it: the option goes, the caret goes to the box, and + // there is nothing to send until something is typed. + await other.click() + await expect(other).toHaveAttribute('aria-pressed', 'true') + await expect(topBar).toHaveAttribute('aria-pressed', 'false') + await expect(card.getByPlaceholder('Type your own answer')).toBeFocused() + await expect(submit).toBeDisabled() + await expectReadableWidth(other.getByText('Other', { exact: true })) + + // Back to the option; then typing in Other replaces it on a single-select question. + await topBar.click() + await expect(other).toHaveAttribute('aria-pressed', 'false') await card.getByPlaceholder('Type your own answer').fill('Tabs along the bottom') - await expect(card.getByRole('button', { name: /^Top bar/ })).toHaveAttribute('aria-pressed', 'false') - await card.getByRole('button', { name: 'Submit', exact: true }).click() + await expect(topBar).toHaveAttribute('aria-pressed', 'false') + await expect(other).toHaveAttribute('aria-pressed', 'true') + await submit.click() await expect.poll(() => posted.length).toBe(1) expect(posted[0].answers).toEqual({ [LAYOUT.question]: 'Tabs along the bottom' }) await expect(card.locator('.user-bubble', { hasText: 'Tabs along the bottom' })).toBeVisible() @@ -175,6 +190,64 @@ test.describe('the live question card', () => { } }) + test('a keyboard user can choose an option and Tab past "Other" to Submit', async ({ page }) => { + // Focusing "Other" used to choose it, which cleared the option on the way to Submit and + // left the question unanswerable from the keyboard. + test.setTimeout(90_000) + const prefix = uniquePrefix('chat-auq-keys') + await cleanupPrefixedRecords(prefix) + await authenticateContext(page.context()) + const conversation = await seedConversation(prefix, { userId: await getActiveUserId() }) + const runId = randomUUID() + const [layout] = PENDING + const { release } = await scriptHeldRun(page, conversation.id, [ + { id: 1, event: 'context_stats', data: { runId, tokenEstimate: 10, contextWindow: 200_000 } }, + { id: 2, event: 'ask_user', data: { id: 'toolu_keys', name: 'AskUserQuestion', token: `${runId}:ask:toolu_keys`, questions: [layout] } }, + ]) + const posted: Array<{ answers: Record }> = [] + await page.route( + (url) => url.pathname === `/chat/${conversation.id}/ask-user`, + (route) => { + posted.push(route.request().postDataJSON()) + return route.fulfill({ json: { resolved: true } }) + }, + ) + + try { + await openAndSend(page, conversation.id, `${prefix} lay it out`) + const card = page.getByRole('main').locator('.ask-user-card').first() + await expect(card.getByText(LAYOUT.question)).toBeVisible({ timeout: 30_000 }) + const topBar = card.getByRole('button', { name: /^Top bar/ }) + const otherBox = card.getByPlaceholder('Type your own answer') + const submit = card.getByRole('button', { name: 'Submit', exact: true }) + + await topBar.focus() + await page.keyboard.press('Space') + await expect(topBar).toHaveAttribute('aria-pressed', 'true') + + // Tab through the rest of the options, "Other", its box and the preview to Submit. + let passedOtherBox = false + for (let presses = 0; presses < 12; presses += 1) { + if (await submit.evaluate((el) => el === document.activeElement)) break + await page.keyboard.press('Tab') + if (await otherBox.evaluate((el) => el === document.activeElement)) passedOtherBox = true + } + expect(passedOtherBox).toBe(true) + await expect(submit).toBeFocused() + await expect(topBar).toHaveAttribute('aria-pressed', 'true') + await expect(card.getByRole('button', { name: 'Other', exact: true })).toHaveAttribute('aria-pressed', 'false') + await expect(submit).toBeEnabled() + + await page.keyboard.press('Enter') + await expect.poll(() => posted.length).toBe(1) + expect(posted[0].answers).toEqual({ [LAYOUT.question]: 'Top bar' }) + } finally { + release() + await page.unrouteAll({ behavior: 'ignoreErrors' }) + await cleanupPrefixedRecords(prefix) + } + }) + test("an answer that arrives only as the call's result still shows on the card", async ({ page }) => { test.setTimeout(90_000) const prefix = uniquePrefix('chat-auq-result') diff --git a/tests/engine.ask-user-question.spec.ts b/tests/engine.ask-user-question.spec.ts index 216e94f2..71653557 100644 --- a/tests/engine.ask-user-question.spec.ts +++ b/tests/engine.ask-user-question.spec.ts @@ -11,7 +11,7 @@ import { answerAskUserQuestion, answerKey, answersByQuestion, - focusOther, + chooseOther, isAskUserToolName, optionLabel, previewDocument, @@ -143,7 +143,7 @@ test.describe('what the user picked', () => { s = toggleOption(Q1, s, 'Sidebar (Recommended)') expect(selectionAnswer(Q1, s)).toBe('Sidebar (Recommended)') - s = focusOther(Q1, s) + s = chooseOther(Q1, s) expect(s.selected).toEqual([]) expect(selectionAnswer(Q1, s)).toBe('') // chose Other, typed nothing yet s = writeOther(Q1, s, ' A hamburger menu ') @@ -160,13 +160,28 @@ test.describe('what the user picked', () => { s = toggleOption(Q2, s, 'Search') s = toggleOption(Q2, s, 'Export') s = toggleOption(Q2, s, 'Export') - expect(focusOther(Q2, s)).toBe(s) s = writeOther(Q2, s, 'Dark mode') expect(selectionAnswer(Q2, s)).toBe('Search, Share, Dark mode') + // Clicking "Other" is a checkbox here: off drops the text from the answer, on brings it back. + s = chooseOther(Q2, s) + expect(selectionAnswer(Q2, s)).toBe('Search, Share') + s = chooseOther(Q2, s) + expect(selectionAnswer(Q2, s)).toBe('Search, Share, Dark mode') s = writeOther(Q2, s, ' ') expect(selectionAnswer(Q2, s)).toBe('Search, Share') }) + test('single-select: an option survives an empty "Other" until the user types or clicks it', () => { + // The card no longer chooses "Other" on focus (a keyboard user tabs through it on the way + // to Submit); only typing or clicking "Other" itself replaces the option. + let s = toggleOption(Q1, EMPTY_SELECTION, 'Top bar') + s = writeOther(Q1, s, '') + expect(selectionAnswer(Q1, s)).toBe('Top bar') + s = writeOther(Q1, s, 'Tabs') + expect(s.selected).toEqual([]) + expect(selectionAnswer(Q1, s)).toBe('Tabs') + }) + test('the card submits only answered questions, keyed by question text', () => { const selections = { [answerKey(Q2)]: toggleOption(Q2, EMPTY_SELECTION, 'Export') } expect(selectionAnswers([Q1, Q2], selections)).toEqual({ 'Which features do you want?': 'Export' }) From 0a6ed9458b71ade1b17a84a4512984258a8bc18b Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 20:40:38 -0600 Subject: [PATCH 6/8] fix(chat): answer each question card under its own token (#4) The bundled CLI marks AskUserQuestion concurrency-safe, so one assistant message can open two or three cards at once. Every inline card sent its answers under the page's single pendingAskUser token, which is always the newest card's: answering the first card recorded its answers against the last question, the engine then refused that question as unanswered, and the first one waited out the five minutes. Each inline card now submits with its own block's token. pendingAskUser is only what the composer and the modal answer; settlePendingAskUser keeps it when another card settles and, when its own card is answered or gets its result, hands the role to the newest card still waiting. A tool_result settles the card it landed on (askUserTokenFor), not whichever question was pending, and a retry resends to the same token. Co-Authored-By: Claude Opus 5.5 --- src/lib/chat/streaming-blocks.ts | 46 +++++++++++++- src/routes/chat/[id]/+page.svelte | 36 +++++++---- tests/chat.ask-user-answers.spec.ts | 47 +++++++++++++- tests/chat.ask-user-question-card.spec.ts | 77 +++++++++++++++++++++++ 4 files changed, 190 insertions(+), 16 deletions(-) diff --git a/src/lib/chat/streaming-blocks.ts b/src/lib/chat/streaming-blocks.ts index a2da94eb..39c68dd4 100644 --- a/src/lib/chat/streaming-blocks.ts +++ b/src/lib/chat/streaming-blocks.ts @@ -9,8 +9,13 @@ * stats, or building the persistence payload on stop / error). */ -import { parseJsonFallback } from '$lib/chat/tool-block-helpers' -import { ASK_USER_QUESTION_TOOL, LEGACY_ASK_USER_TOOL, isAskUserToolName } from '../engine/ask-user-question' +import { getAskUserQuestionsFromTool, parseJsonFallback } from '$lib/chat/tool-block-helpers' +import { + ASK_USER_QUESTION_TOOL, + LEGACY_ASK_USER_TOOL, + isAskUserToolName, + type AskQuestion, +} from '../engine/ask-user-question' import type { ToolResultDetails } from '../engine/tool-result-details' import type { RunNotice } from '../engine/sdk-notices' @@ -774,6 +779,43 @@ export function applyAskUserAnswered( ) } +/** The question the chat page's composer and modal answer: one card's token and questions. */ +export type PendingAskUser = { token: string; questions: AskQuestion[] } + +/** The answer token of the question card a call's result landed on, if it is one. */ +export function askUserTokenFor(blocks: StreamingBlock[], id: string): string | null { + const block = blocks.find((b) => b.kind === 'tool' && b.id === id) + return block?.kind === 'tool' && isAskUserToolName(block.name) ? (block.token ?? null) : null +} + +/** + * Which question the page treats as pending once the card for `settledToken` is answered or + * has its call's result (#4). + * + * The CLI runs AskUserQuestion calls concurrently (`isConcurrencySafe`), so one assistant + * message can put two cards on screen, each answered under its own token. The page keeps one + * of them as *the* pending question — the one the composer's free-text reply and the modal + * answer. Settling another card leaves it alone. Settling it hands the role to the newest + * card still waiting, so a composer reply still has a question to go to; with none, nothing + * is pending. An unknown `settledToken` (null) just re-reads which card is still waiting. + */ +export function settlePendingAskUser( + blocks: StreamingBlock[], + current: PendingAskUser | null, + settledToken: string | null, +): PendingAskUser | null { + if (current && settledToken !== null && current.token !== settledToken) return current + for (let i = blocks.length - 1; i >= 0; i -= 1) { + const block = blocks[i] + if (block.kind !== 'tool' || !isAskUserToolName(block.name) || !block.token) continue + if (block.token === settledToken) continue + if (block.status !== 'pending' && block.status !== 'approved' && block.status !== 'executing') continue + const questions = getAskUserQuestionsFromTool(block) + if (questions.length > 0) return { token: block.token, questions } + } + return null +} + /** * `ask_user` event — append a synthetic executing tool block carrying the * questions so the AskUserCard can render inline. Skips the append when a tool diff --git a/src/routes/chat/[id]/+page.svelte b/src/routes/chat/[id]/+page.svelte index 96cd7671..9b2027b2 100644 --- a/src/routes/chat/[id]/+page.svelte +++ b/src/routes/chat/[id]/+page.svelte @@ -51,6 +51,8 @@ appendThinking, applyAskUser, applyAskUserAnswered, + askUserTokenFor, + settlePendingAskUser, applyDeltaStart, applySubagentDelta, applySubagentDone, @@ -172,6 +174,7 @@ | { kind: 'askUser'; answers: Record; + token: string; } | { kind: 'edit'; @@ -236,7 +239,7 @@ return; } if (intent.kind === 'askUser') { - await resolveAskUser(intent.answers); + await resolveAskUser(intent.answers, intent.token); return; } if (intent.kind === 'regenerate') { @@ -750,9 +753,14 @@ ); } - async function resolveAskUser(answers: Record) { - if (!pendingAskUser) return; - const { token } = pendingAskUser; + /** + * #4 — `cardToken` is the answering card's own token: AskUserQuestion calls run + * concurrently, so two cards can be open and `pendingAskUser` holds only the newest. + * The modal and the composer answer that one. + */ + async function resolveAskUser(answers: Record, cardToken?: string | null) { + const token = cardToken ?? pendingAskUser?.token; + if (!token) return; try { const response = await fetch(`/chat/${conversationId}/ask-user`, { method: 'POST', @@ -776,19 +784,24 @@ // Do not create optimistic user bubbles for ask_user to avoid ordering/race issues. // The card itself shows the recorded answers (#81). streamingBlocks = applyAskUserAnswered(streamingBlocks, token, answers); - - pendingAskUser = null; - askUserModalOpen = false; + settleAskUser(token); } catch (error) { setRecoverableError( error instanceof Error ? error.message : 'Failed to submit ask_user answers', - { kind: 'askUser', answers }, + { kind: 'askUser', answers, token }, { token, answerCount: Object.keys(answers).length, action: 'resolveAskUser' } ); throw error; } } + /** A question card was answered or got its result: keep or hand over the pending question. */ + function settleAskUser(settledToken: string | null) { + const next = settlePendingAskUser(streamingBlocks, pendingAskUser, settledToken); + if (!next || next.token !== pendingAskUser?.token) askUserModalOpen = false; + pendingAskUser = next; + } + function closeAskUserModal() { askUserModalOpen = false; } @@ -1008,10 +1021,6 @@ } if (eventName === 'tool_result') { - if (isAskUserToolName(payload.name)) { - pendingAskUser = null; - askUserModalOpen = false; - } const outcome = applyToolResult(streamingBlocks, payload as Parameters[1]); if (outcome.missing) { logChatUi('warn', 'tool_result without matching tool block', { @@ -1025,6 +1034,7 @@ }); } streamingBlocks = outcome.blocks; + if (isAskUserToolName(payload.name)) settleAskUser(askUserTokenFor(streamingBlocks, payload.id)); } if (eventName === 'tool_denied') { @@ -1636,7 +1646,7 @@ questions={askQuestions} status={block.status} answers={askAnswers} - onSubmit={resolveAskUser} + onSubmit={(answers) => resolveAskUser(answers, block.token)} /> {/if}