diff --git a/designdocs/agents/STYLE_GUIDE.md b/designdocs/agents/STYLE_GUIDE.md index 8126a83b..602bb4bc 100644 --- a/designdocs/agents/STYLE_GUIDE.md +++ b/designdocs/agents/STYLE_GUIDE.md @@ -48,7 +48,14 @@ carry the **why** — the things a reader cannot recover by reading the code. - **No arbitrary font-size values**: Never use Tailwind's arbitrary-value syntax for typography (e.g. `tw-text-[10.5px]`, `tw-text-[13px]`). Stick to the configured `fontSize` tokens (`tw-text-ui-smaller`, `tw-text-ui-small`, `tw-text-xs`, `tw-text-smallest`, etc.) so type stays consistent with Obsidian's CSS variables. If none of the existing tokens fit, extend the `fontSize` scale in `tailwind.config.js` rather than hard-coding a pixel value at the call site. - **No inline `style={{ ... }}`**: Reserve the `style` prop for values that must change dynamically at runtime (computed positions, animated transforms). Static visual changes belong in Tailwind classes or the shared component (e.g. `Button` variants/sizes). - **Always wrap Tailwind class strings with `cn()`** (from `@/lib/utils`) whenever the classes live anywhere other than a literal `className=` attribute on a JSX element — variable assignments, ternaries, function returns, props passed to other components, etc. `eslint-plugin-tailwindcss` only lints classes it can statically see inside JSX `className` literals or inside calls to its registered callees (`cn`, `clsx`, `classnames`, `ctl`, `cva`). Use `cn()` for composition too — instead of a ternary between two whole class strings, merge a shared base with conditional fragments: `cn("tw-flex tw-text-sm", expandable && "tw-cursor-pointer")`. -- **Never use a raw ` - - - - ); -}; - -/** - * Open the AskUserQuestion modal for one Claude SDK `canUseTool` invocation. - * Resolves with the answers map (or `{}` on cancel — the bridge maps empty - * to a deny-with-cancelled message). - */ -export function openAskUserQuestionModal(app: App, questions: Questions): Promise { - return new Promise((resolve) => { - const modal = new AskUserQuestionModal(app, questions, resolve); - modal.open(); - }); -} - -class AskUserQuestionModal extends Modal { - private root: Root | null = null; - private settled = false; - - constructor( - app: App, - private readonly questions: Questions, - private readonly onSettle: (answers: Answers) => void - ) { - super(app); - this.titleEl.setText("Agent Mode — Question from Claude"); - } - - onOpen(): void { - const { contentEl } = this; - contentEl.empty(); - this.root = createPluginRoot(contentEl, this.app); - this.root.render( - { - this.settled = true; - this.onSettle(answers); - this.close(); - }} - onCancel={() => { - this.settled = true; - this.onSettle({}); - this.close(); - }} - /> - ); - } - - onClose(): void { - this.root?.unmount(); - this.root = null; - this.contentEl.empty(); - if (!this.settled) { - this.settled = true; - this.onSettle({}); - } - } -} diff --git a/src/agentMode/backends/claude/descriptor.ts b/src/agentMode/backends/claude/descriptor.ts index 1211d700..873afa0a 100644 --- a/src/agentMode/backends/claude/descriptor.ts +++ b/src/agentMode/backends/claude/descriptor.ts @@ -28,7 +28,6 @@ import type { BackendDescriptor, BackendProcess, InstallState } from "@/agentMod import { ClaudeInstallModal } from "./ClaudeInstallModal"; import ClaudeLogo from "./logo.svg"; import { ClaudeSettingsPanel } from "./ClaudeSettingsPanel"; -import { openAskUserQuestionModal } from "./AskUserQuestionModal"; export const CLAUDE_INSTALL_COMMAND = "npm install -g @anthropic-ai/claude-code"; @@ -201,7 +200,6 @@ export const ClaudeBackendDescriptor: BackendDescriptor = { app: args.app, clientVersion: args.clientVersion, descriptor: args.descriptor, - askUserQuestion: (questions) => openAskUserQuestionModal(args.app, questions), getEnableThinking: () => Boolean(getSettings().agentMode?.backends?.claude?.enableThinking), getEnvOverrides: () => getSettings().agentMode?.backends?.claude?.envOverrides, isPlanModePlanFilePath: isClaudePlanModePlanFilePath, diff --git a/src/agentMode/index.ts b/src/agentMode/index.ts index 97621b73..fb828e45 100644 --- a/src/agentMode/index.ts +++ b/src/agentMode/index.ts @@ -10,7 +10,10 @@ import { AgentChatPersistenceManager } from "./session/AgentChatPersistenceManag import { AgentModelPreloader } from "./session/AgentModelPreloader"; import { AgentSessionManager } from "./session/AgentSessionManager"; import { SkillManager } from "./skills"; -import { createDefaultPermissionPrompter } from "./ui/permissionPrompter"; +import { + createDefaultAskUserQuestionPrompter, + createDefaultPermissionPrompter, +} from "./ui/permissionPrompter"; export { AGENT_CHAT_MODE } from "./session/AgentChatPersistenceManager"; export { AgentModeChat } from "./ui/AgentModeChat"; @@ -126,8 +129,12 @@ export function createAgentSessionManager(app: App, plugin: CopilotPlugin): Agen const prompter = createDefaultPermissionPrompter( (id) => managerRef?.getSessionByBackendId(id) ?? null ); + const askUserQuestionPrompter = createDefaultAskUserQuestionPrompter( + (id) => managerRef?.getSessionByBackendId(id) ?? null + ); const manager = new AgentSessionManager(app, plugin, { permissionPrompter: prompter, + askUserQuestionPrompter, resolveDescriptor: (id) => backendRegistry[id], modelPreloader: preloader, persistenceManager, diff --git a/src/agentMode/sdk/ClaudeSdkBackendProcess.ts b/src/agentMode/sdk/ClaudeSdkBackendProcess.ts index 8a2ad752..c499bf5c 100644 --- a/src/agentMode/sdk/ClaudeSdkBackendProcess.ts +++ b/src/agentMode/sdk/ClaudeSdkBackendProcess.ts @@ -52,7 +52,7 @@ import type { } from "@/agentMode/session/types"; import { MethodUnsupportedError } from "@/agentMode/session/errors"; import { createTranslatorState, mapStopReason, translateSdkMessage } from "./sdkMessageTranslator"; -import { PermissionBridge, type AskUserQuestionHandler } from "./permissionBridge"; +import { PermissionBridge, type AskUserQuestionPrompter } from "./permissionBridge"; import { getCachedSdkCatalog, probeClaudeSdkCatalog, @@ -101,7 +101,6 @@ export interface ClaudeSdkBackendProcessOptions { app: App; clientVersion: string; descriptor: BackendDescriptor; - askUserQuestion?: AskUserQuestionHandler; /** * Read at the start of every `prompt()` so a settings change live-applies on * the next turn. @@ -155,6 +154,7 @@ export class ClaudeSdkBackendProcess implements BackendProcess { private readonly sessions = new Map(); private permissionPrompter: ((req: PermissionPrompt) => Promise) | null = null; + private askUserQuestionPrompter: AskUserQuestionPrompter | null = null; private exitListeners = new Set<() => void>(); private shuttingDown = false; private readonly bridge: PermissionBridge; @@ -169,7 +169,7 @@ export class ClaudeSdkBackendProcess implements BackendProcess { constructor(private readonly opts: ClaudeSdkBackendProcessOptions) { this.bridge = new PermissionBridge({ getPrompter: () => this.permissionPrompter, - askUserQuestion: opts.askUserQuestion, + getAskUserQuestionPrompter: () => this.askUserQuestionPrompter, isPlanModePlanFilePath: opts.isPlanModePlanFilePath, }); logInfo( @@ -190,6 +190,10 @@ export class ClaudeSdkBackendProcess implements BackendProcess { this.permissionPrompter = fn; } + setAskUserQuestionPrompter(fn: AskUserQuestionPrompter): void { + this.askUserQuestionPrompter = fn; + } + registerSessionHandler(sessionId: SessionId, handler: SessionUpdateHandler): () => void { this.sessionHandlers.set(sessionId, handler); const buffered = this.pendingUpdates.get(sessionId); diff --git a/src/agentMode/sdk/permissionBridge.test.ts b/src/agentMode/sdk/permissionBridge.test.ts index 8871a15b..c824b919 100644 --- a/src/agentMode/sdk/permissionBridge.test.ts +++ b/src/agentMode/sdk/permissionBridge.test.ts @@ -1,22 +1,45 @@ -import type { PermissionDecision, PermissionPrompt } from "@/agentMode/session/types"; -import { PermissionBridge } from "./permissionBridge"; +import type { + AgentQuestionAnswers, + AskUserQuestionPrompt, + PermissionDecision, + PermissionPrompt, +} from "@/agentMode/session/types"; +import { PermissionBridge, type AskUserQuestionPrompter } from "./permissionBridge"; describe("PermissionBridge.canUseTool", () => { function makeBridge( prompter: ((req: PermissionPrompt) => Promise) | null, - askUserQuestion?: ( - questions: Array<{ question: string; options: Array<{ label: string }> }> - ) => Promise<{ [q: string]: string }> + askUserQuestionPrompter?: AskUserQuestionPrompter ) { const bridge = new PermissionBridge({ getPrompter: () => prompter, - - askUserQuestion: askUserQuestion, + getAskUserQuestionPrompter: askUserQuestionPrompter + ? () => askUserQuestionPrompter + : undefined, }); bridge.setSessionContext("session-1"); return bridge; } + /** + * Minimal stand-in for an `AgentSession`'s ask-question resolver path: holds + * the in-flight request and a `resolve` handle so a test can drive the + * inline-card "submit" / "cancel" transitions the bridge awaits. + */ + class FakeQuestionSession { + pending: AskUserQuestionPrompt | null = null; + private resolver: ((answers: AgentQuestionAnswers) => void) | null = null; + readonly handle: AskUserQuestionPrompter = (req) => { + this.pending = req; + return new Promise((resolve) => { + this.resolver = resolve; + }); + }; + resolve(answers: AgentQuestionAnswers): void { + this.resolver?.(answers); + } + } + const ctx = { signal: new AbortController().signal, toolUseID: "toolu_test_id", @@ -133,8 +156,10 @@ describe("PermissionBridge.canUseTool", () => { expect(result.behavior).toBe("deny"); }); - it("routes AskUserQuestion to the dedicated handler with answers", async () => { - const handler = jest.fn(async () => ({ "What's your favorite color?": "Blue" })); + it("routes AskUserQuestion to the ask-question prompter with a session-domain request", async () => { + const handler = jest.fn, [AskUserQuestionPrompt]>(async () => ({ + "What's your favorite color?": "Blue", + })); const bridge = makeBridge(null, handler); const result = await bridge.canUseTool( "AskUserQuestion", @@ -143,7 +168,11 @@ describe("PermissionBridge.canUseTool", () => { }, ctx ); - expect(handler).toHaveBeenCalled(); + expect(handler).toHaveBeenCalledWith({ + sessionId: "session-1", + requestId: "toolu_test_id", + questions: [{ question: "What's your favorite color?", options: [{ label: "Blue" }] }], + }); expect(result.behavior).toBe("allow"); if (result.behavior === "allow") { expect(result.updatedInput).toMatchObject({ @@ -152,7 +181,7 @@ describe("PermissionBridge.canUseTool", () => { } }); - it("denies AskUserQuestion when no handler is configured", async () => { + it("denies AskUserQuestion when no ask-question prompter is configured", async () => { const bridge = makeBridge(async () => ({ outcome: { outcome: "cancelled" } })); const result = await bridge.canUseTool( "AskUserQuestion", @@ -162,15 +191,48 @@ describe("PermissionBridge.canUseTool", () => { expect(result.behavior).toBe("deny"); }); - it("treats empty AskUserQuestion answers as cancelled", async () => { - const handler = jest.fn(async () => ({})); - const bridge = makeBridge(null, handler); - const result = await bridge.canUseTool( + it("submitting answers resolves AskUserQuestion with allow + the { questions, answers } payload", async () => { + // End-to-end inline-card resolver path: the bridge awaits the session's + // pending question, then maps the submitted answers back to a SDK allow — + // the same payload the old modal produced. + const fake = new FakeQuestionSession(); + const bridge = makeBridge(null, fake.handle); + const questions = [ + { question: "Pick a fruit", options: [{ label: "Apple" }, { label: "Pear" }] }, + ]; + const resultPromise = bridge.canUseTool("AskUserQuestion", { questions }, ctx); + // The card is pending until the user submits — the prompter saw the + // session-domain request keyed by the SDK tool_use_id. + expect(fake.pending).toEqual({ + sessionId: "session-1", + requestId: "toolu_test_id", + questions, + }); + + fake.resolve({ "Pick a fruit": "Pear" }); + const result = await resultPromise; + expect(result).toEqual({ + behavior: "allow", + updatedInput: { questions, answers: { "Pick a fruit": "Pear" } }, + }); + }); + + it("cancelling AskUserQuestion (empty answers) resolves with deny + the cancellation message", async () => { + const fake = new FakeQuestionSession(); + const bridge = makeBridge(null, fake.handle); + const resultPromise = bridge.canUseTool( "AskUserQuestion", { questions: [{ question: "Q", options: [{ label: "A" }] }] }, ctx ); + + // Dismissing the card resolves the resolver with `{}`. + fake.resolve({}); + const result = await resultPromise; expect(result.behavior).toBe("deny"); + if (result.behavior === "deny") { + expect(result.message).toBe("User cancelled the question"); + } }); describe("Write tool gating", () => { diff --git a/src/agentMode/sdk/permissionBridge.ts b/src/agentMode/sdk/permissionBridge.ts index 5d18ee98..577db600 100644 --- a/src/agentMode/sdk/permissionBridge.ts +++ b/src/agentMode/sdk/permissionBridge.ts @@ -1,9 +1,10 @@ /** * Bridge between the Claude SDK's `canUseTool` callback and Agent Mode's - * session-domain permission prompter. Each `canUseTool` invocation is - * translated to a `PermissionPrompt`, dispatched through the prompter, then - * translated back to a SDK `PermissionResult`. AskUserQuestion gets a - * separate branch that opens a dedicated multi-choice modal. + * session-domain prompters. Each `canUseTool` invocation is translated to a + * `PermissionPrompt`, dispatched through the permission prompter, then + * translated back to a SDK `PermissionResult`. AskUserQuestion gets a separate + * branch that dispatches through the ask-question prompter — the session + * surfaces an inline card and returns the answers map. */ import type { CanUseTool, @@ -11,6 +12,9 @@ import type { PermissionUpdate, } from "@anthropic-ai/claude-agent-sdk"; import type { + AgentQuestion, + AgentQuestionAnswers, + AskUserQuestionPrompt, PermissionDecision, PermissionOption, PermissionOptionKind, @@ -25,22 +29,26 @@ import { deriveToolKind, deriveToolTitle, vendorMetaFields } from "./toolMeta"; export type Prompter = (req: PermissionPrompt) => Promise; -export type AskUserQuestionHandler = ( - questions: AskUserQuestionInput["questions"] -) => Promise<{ [questionText: string]: string }>; +/** + * Session-domain handler for the SDK's `AskUserQuestion` tool. Mirrors the + * permission `Prompter`: the bridge fetches it lazily via + * `getAskUserQuestionPrompter` so it can be registered after construction. + */ +export type AskUserQuestionPrompter = (req: AskUserQuestionPrompt) => Promise; +/** SDK-side shape of the `AskUserQuestion` tool input. */ export interface AskUserQuestionInput { - questions: Array<{ - question: string; - header?: string; - options: Array<{ label: string; description?: string }>; - multiSelect?: boolean; - }>; + questions: AgentQuestion[]; } export interface PermissionBridgeOptions { getPrompter: () => Prompter | null; - askUserQuestion?: AskUserQuestionHandler; + /** + * Lazily fetch the session-domain ask-question prompter. Absent / returning + * `null` makes AskUserQuestion deny with "not yet supported", matching the + * pre-inline behavior when no handler was wired. + */ + getAskUserQuestionPrompter?: () => AskUserQuestionPrompter | null; /** * Predicate identifying plan-mode plan files. When provided, the bridge * auto-allows `Write` calls whose `file_path` satisfies the predicate so @@ -71,7 +79,7 @@ export class PermissionBridge { canUseTool: CanUseTool = async (toolName, input, ctx) => { if (toolName === "AskUserQuestion") { - return this.handleAskUserQuestion(input as unknown as AskUserQuestionInput); + return this.handleAskUserQuestion(input as unknown as AskUserQuestionInput, ctx); } const sessionId = this.currentSessionId; @@ -105,10 +113,14 @@ export class PermissionBridge { return result; }; - private async handleAskUserQuestion(input: AskUserQuestionInput): Promise { + private async handleAskUserQuestion( + input: AskUserQuestionInput, + ctx: Parameters[2] + ): Promise { const sessionId = this.currentSessionId; logSdkInbound("askUserQuestion:request", input, sessionId); - if (!this.opts.askUserQuestion) { + const prompter = this.opts.getAskUserQuestionPrompter?.() ?? null; + if (!prompter || !sessionId) { return this.deny( "askUserQuestion:response", "AskUserQuestion is not yet supported", @@ -116,7 +128,14 @@ export class PermissionBridge { ); } try { - const answers = await this.opts.askUserQuestion(input.questions); + // Reuse the SDK's `tool_use_id` as the requestId so the inline card's + // resolver pairs the answer with this call, mirroring the permission + // prompt's `toolCallId`. + const answers = await prompter({ + sessionId, + requestId: ctx.toolUseID, + questions: input.questions, + }); if (Object.keys(answers).length === 0) { return this.deny("askUserQuestion:response", "User cancelled the question", sessionId); } diff --git a/src/agentMode/session/AgentChatBackend.ts b/src/agentMode/session/AgentChatBackend.ts index 4fdcb10c..f8d22cf2 100644 --- a/src/agentMode/session/AgentChatBackend.ts +++ b/src/agentMode/session/AgentChatBackend.ts @@ -1,6 +1,8 @@ import type { MessageContext } from "@/types/message"; import type { AgentChatMessage, + AgentQuestionAnswers, + AskUserQuestionPrompt, BackendState, CurrentPlan, PermissionPrompt, @@ -94,4 +96,20 @@ export interface AgentChatBackend { * SDK turn unblocks. No-op when no permission is pending for the given id. */ resolveToolPermission(toolCallId: string, optionId: string): void; + + /** + * Snapshot of every pending AskUserQuestion request waiting on the user. + * Rendered as inline `AskUserQuestionCard`s at the tail of the chat scroll + * container, alongside any `ToolPermissionCard`s. Empty list when none. + */ + getPendingAskUserQuestions(): AskUserQuestionPrompt[]; + + /** + * Resolve a pending AskUserQuestion with the user's answers. The card is + * removed from `getPendingAskUserQuestions()` synchronously and the SDK turn + * unblocks. An empty map signals cancellation (the backend produces the + * "User cancelled the question" deny). No-op when no question is pending for + * the given id. + */ + resolveAskUserQuestion(requestId: string, answers: AgentQuestionAnswers): void; } diff --git a/src/agentMode/session/AgentChatUIState.ts b/src/agentMode/session/AgentChatUIState.ts index 0272d252..b4efd790 100644 --- a/src/agentMode/session/AgentChatUIState.ts +++ b/src/agentMode/session/AgentChatUIState.ts @@ -3,6 +3,8 @@ import type { AgentChatBackend } from "@/agentMode/session/AgentChatBackend"; import type { AgentSession } from "@/agentMode/session/AgentSession"; import type { AgentChatMessage, + AgentQuestionAnswers, + AskUserQuestionPrompt, BackendState, CurrentPlan, PermissionPrompt, @@ -130,6 +132,15 @@ export class AgentChatUIState implements AgentChatBackend { this.notifyListeners(); } + getPendingAskUserQuestions(): AskUserQuestionPrompt[] { + return this.session.getPendingAskUserQuestions(); + } + + resolveAskUserQuestion(requestId: string, answers: AgentQuestionAnswers): void { + this.session.resolveAskUserQuestion(requestId, answers); + this.notifyListeners(); + } + getCurrentPlan(): CurrentPlan | null { return this.session.getCurrentPlan(); } diff --git a/src/agentMode/session/AgentSession.test.ts b/src/agentMode/session/AgentSession.test.ts index 3ec6a76f..c3093cb1 100644 --- a/src/agentMode/session/AgentSession.test.ts +++ b/src/agentMode/session/AgentSession.test.ts @@ -1725,6 +1725,92 @@ describe("AgentSession plan proposal lifecycle", () => { await turn; }); + it("surfaces a pending AskUserQuestion and resolves it with the submitted answers", async () => { + const mock = makeMockBackend(); + let resolvePrompt: ((v: { stopReason: "end_turn" }) => void) | null = null; + mock.prompt.mockImplementation( + () => new Promise((resolve) => (resolvePrompt = resolve as typeof resolvePrompt)) + ); + const session = new AgentSession({ + backend: mock.asBackend, + backendSessionId: "acp-1", + internalId: "internal-1", + backendId: "claude", + }); + const statusChanges: string[] = []; + session.subscribe({ + onMessagesChanged: () => {}, + onStatusChanged: (status) => statusChanges.push(status), + }); + const { turn } = session.sendPrompt("ask me something"); + expect(session.getStatus()).toBe("running"); + + const answersPromise = session.handleAskUserQuestion({ + sessionId: "acp-1", + requestId: "tc-ask", + questions: [{ question: "Pick a fruit", options: [{ label: "Apple" }, { label: "Pear" }] }], + }); + + expect(session.getStatus()).toBe("awaiting_permission"); + expect(session.getPendingAskUserQuestions()).toHaveLength(1); + expect(statusChanges).toContain("awaiting_permission"); + + session.resolveAskUserQuestion("tc-ask", { "Pick a fruit": "Pear" }); + await expect(answersPromise).resolves.toEqual({ "Pick a fruit": "Pear" }); + expect(session.getPendingAskUserQuestions()).toHaveLength(0); + expect(session.getStatus()).toBe("running"); + + resolvePrompt!({ stopReason: "end_turn" }); + await turn; + }); + + it("returns the shared empty array when no AskUserQuestion is pending", () => { + const mock = makeMockBackend(); + const session = new AgentSession({ + backend: mock.asBackend, + backendSessionId: "acp-1", + internalId: "internal-1", + backendId: "claude", + }); + expect(session.getPendingAskUserQuestions()).toHaveLength(0); + // Stable reference across idle ticks so React subscribers don't re-render. + expect(session.getPendingAskUserQuestions()).toBe(session.getPendingAskUserQuestions()); + }); + + it("flushes a pending AskUserQuestion with empty answers when the turn is cancelled", async () => { + const mock = makeMockBackend(); + let resolvePrompt: ((v: { stopReason: "cancelled" }) => void) | null = null; + let answersPromise: Promise = Promise.resolve(); + mock.prompt.mockImplementation( + () => new Promise((resolve) => (resolvePrompt = resolve as typeof resolvePrompt)) + ); + mock.cancel.mockImplementation(() => answersPromise.then(() => undefined)); + const session = new AgentSession({ + backend: mock.asBackend, + backendSessionId: "acp-1", + internalId: "internal-1", + backendId: "claude", + }); + const { turn } = session.sendPrompt("ask me something"); + + answersPromise = session.handleAskUserQuestion({ + sessionId: "acp-1", + requestId: "tc-ask", + questions: [{ question: "Pick a fruit", options: [{ label: "Apple" }] }], + }); + expect(session.getStatus()).toBe("awaiting_permission"); + + const cancelPromise = session.cancel(); + // Cancellation resolves the card's resolver with `{}` (the cancel signal) + // so the SDK turn unblocks instead of dangling. + await expect(answersPromise).resolves.toEqual({}); + expect(session.getPendingAskUserQuestions()).toHaveLength(0); + await cancelPromise; + + resolvePrompt!({ stopReason: "cancelled" }); + await turn; + }); + it("forwards the optional denyMessage on resolvePlanProposalPermission to the resolved decision", async () => { const mock = makeMockBackend(); let resolvePrompt: ((v: { stopReason: "end_turn" }) => void) | null = null; diff --git a/src/agentMode/session/AgentSession.ts b/src/agentMode/session/AgentSession.ts index 30bd88bf..d9fa7867 100644 --- a/src/agentMode/session/AgentSession.ts +++ b/src/agentMode/session/AgentSession.ts @@ -3,7 +3,9 @@ import { logInfo, logWarn } from "@/logger"; import { AgentMessageStore } from "@/agentMode/session/AgentMessageStore"; import { AgentMessagePart, + AgentQuestionAnswers, AgentToolCallOutput, + AskUserQuestionPrompt, BackendDescriptor, BackendId, BackendProcess, @@ -43,6 +45,11 @@ const MAX_TOOL_OUTPUT_TEXT_CHARS = 12_000; // when nothing is pending — preserves React `useState` setter bail-out // behavior on idle subscription ticks. const EMPTY_PERMISSIONS: PermissionPrompt[] = []; +// Same idea for `getPendingAskUserQuestions()`. +const EMPTY_QUESTIONS: AskUserQuestionPrompt[] = []; +// Canonical "no answers" map. Resolving an in-flight question with this on +// cancel/dispose makes the bridge treat it as a user cancellation. +const EMPTY_ANSWERS: AgentQuestionAnswers = Object.freeze({}); /** * Optimistically swap `state.model.current.baseModelId` for the persisted @@ -241,6 +248,17 @@ export class AgentSession { resolve: (resp: PermissionDecision) => void; } >(); + // Pending AskUserQuestion resolvers keyed by requestId. Populated by + // `handleAskUserQuestion` when the wrapped ask-question prompter routes a + // request to this session; the inline `AskUserQuestionCard` resolves them + // through `resolveAskUserQuestion`. + private pendingQuestionResolvers = new Map< + string, + { + request: AskUserQuestionPrompt; + resolve: (answers: AgentQuestionAnswers) => void; + } + >(); // Singleton "current plan" for the floating card. At most one per session // while in canonical plan mode and a plan has been proposed; cleared on a // terminal user decision or when the canonical mode flips out of plan. @@ -509,7 +527,12 @@ export class AgentSession { if (this.backendSessionId === null) { return this.startupFailed ? "error" : "starting"; } - if (this.pendingPlanResolvers.size + this.pendingToolResolvers.size > 0) { + if ( + this.pendingPlanResolvers.size + + this.pendingToolResolvers.size + + this.pendingQuestionResolvers.size > + 0 + ) { return "awaiting_permission"; } if (this.abortController !== null) return "running"; @@ -711,9 +734,10 @@ export class AgentSession { * session events before the prompt promise resolves with * `stopReason: "cancelled"` — that's expected. * - * Flushes any pending tool-permission resolvers as rejects so the inline - * cards disappear immediately (and the SDK sees a deny rather than a - * dangling promise) instead of waiting for the user to click them. + * Flushes any pending tool-permission and AskUserQuestion resolvers (rejects + * / empty answers respectively) so the inline cards disappear immediately + * (and the SDK sees a deny rather than a dangling promise) instead of waiting + * for the user to click them. */ async cancel(): Promise { const status = this.getStatus(); @@ -723,6 +747,10 @@ export class AgentSession { this.flushResolvers(this.pendingToolResolvers); this.notifyMessages(); } + if (this.pendingQuestionResolvers.size > 0) { + this.flushQuestionResolvers(); + this.notifyMessages(); + } try { await this.backend.cancel({ sessionId: this.backendSessionId }); } catch (e) { @@ -738,6 +766,7 @@ export class AgentSession { this.unregisterSessionHandler = null; this.flushResolvers(this.pendingPlanResolvers); this.flushResolvers(this.pendingToolResolvers); + this.flushQuestionResolvers(); this.decidedPlanToolCallIds.clear(); this.currentPlan = null; // Fire the `"closed"` transition before clearing listeners so @@ -935,6 +964,46 @@ export class AgentSession { return Array.from(this.pendingToolResolvers.values(), (e) => e.request); } + /** + * Called by the wrapped ask-question prompter when the backend invokes its + * inline-question surface (Claude SDK's `AskUserQuestion`). The returned + * promise resolves when the inline `AskUserQuestionCard` calls + * `resolveAskUserQuestion` with the user's answers — or with `EMPTY_ANSWERS` + * if the turn is cancelled/disposed, which the bridge maps to a deny. + */ + handleAskUserQuestion(request: AskUserQuestionPrompt): Promise { + const requestId = request.requestId; + return new Promise((resolve) => { + this.pendingQuestionResolvers.set(requestId, { request, resolve }); + this.recomputeStatusIfChanged(); + this.notifyMessages(); + }); + } + + /** + * Resolve a pending AskUserQuestion with the user's answers. An empty map + * signals cancellation (the bridge turns it into the "User cancelled the + * question" deny). No-op when no question is pending for the given id. + */ + resolveAskUserQuestion(requestId: string, answers: AgentQuestionAnswers): void { + const entry = this.pendingQuestionResolvers.get(requestId); + if (!entry) return; + this.pendingQuestionResolvers.delete(requestId); + entry.resolve(answers); + this.recomputeStatusIfChanged(); + this.notifyMessages(); + } + + /** + * Snapshot of every pending AskUserQuestion request, in arrival order so the + * UI renders cards stably. Returns a shared empty array when nothing is + * pending so React subscribers don't re-render on unrelated ticks. + */ + getPendingAskUserQuestions(): AskUserQuestionPrompt[] { + if (this.pendingQuestionResolvers.size === 0) return EMPTY_QUESTIONS; + return Array.from(this.pendingQuestionResolvers.values(), (e) => e.request); + } + /** * Reject every pending resolver in `map` with the canonical deny decision * derived from the request's offered options, then clear the map. Used by @@ -951,6 +1020,19 @@ export class AgentSession { this.recomputeStatusIfChanged(); } + /** + * Resolve every pending AskUserQuestion with `EMPTY_ANSWERS` (the + * cancellation signal) and clear the map. The answer-shaped resolvers can't + * reuse `flushResolvers`, which deals in `PermissionDecision`. + */ + private flushQuestionResolvers(): void { + for (const { resolve } of this.pendingQuestionResolvers.values()) { + resolve(EMPTY_ANSWERS); + } + this.pendingQuestionResolvers.clear(); + this.recomputeStatusIfChanged(); + } + private handleSessionEvent(event: SessionEvent): void { const update = event.update; diff --git a/src/agentMode/session/AgentSessionManager.ts b/src/agentMode/session/AgentSessionManager.ts index 8225d8f5..934bf3b4 100644 --- a/src/agentMode/session/AgentSessionManager.ts +++ b/src/agentMode/session/AgentSessionManager.ts @@ -13,6 +13,8 @@ import type { AgentModelPreloader, WarmBackend } from "./AgentModelPreloader"; import { MethodUnsupportedError } from "./errors"; import { resolveMcpServers } from "./mcpResolver"; import type { + AgentQuestionAnswers, + AskUserQuestionPrompt, BackendDescriptor, BackendId, BackendProcess, @@ -28,12 +30,27 @@ const AUTOSAVE_DEBOUNCE_MS = 500; export type PermissionPrompter = (req: PermissionPrompt) => Promise; +/** + * Session-domain handler for inline multiple-choice questions, the sibling of + * `PermissionPrompter`. Routes a backend's `AskUserQuestion` request to its + * owning session, which surfaces an inline card and resolves with the answers + * (or `{}` when the user cancels / no session owns the request). + */ +export type AskUserQuestionPrompter = (req: AskUserQuestionPrompt) => Promise; + // Injected by the barrel so `session/` doesn't have to import // `backends/registry` directly (would breach the layer boundary). export type DescriptorResolver = (id: BackendId) => BackendDescriptor | undefined; export interface AgentSessionManagerOptions { permissionPrompter: PermissionPrompter; + /** + * Handler the Claude SDK backend calls for its inline `AskUserQuestion` + * surface. Optional only so legacy callers (tests) can omit it; production + * wiring always supplies one via the barrel in `agentMode/index.ts`. Wired + * onto each backend that advertises `setAskUserQuestionPrompter`. + */ + askUserQuestionPrompter?: AskUserQuestionPrompter; resolveDescriptor: DescriptorResolver; modelPreloader: AgentModelPreloader; /** @@ -1059,6 +1076,19 @@ export class AgentSessionManager { * (e.g. `tryResumeSessionFromHistory`) can ignore `warm` — the probe * session simply sits unused on the proc. */ + /** + * Register the session-domain prompters on a freshly-adopted backend. The + * permission prompter is required; the ask-question prompter is wired only + * when both the manager was configured with one and the backend advertises + * the optional `setAskUserQuestionPrompter` surface (Claude SDK today). + */ + private wirePrompters(proc: BackendProcess): void { + proc.setPermissionPrompter(this.opts.permissionPrompter); + if (this.opts.askUserQuestionPrompter) { + proc.setAskUserQuestionPrompter?.(this.opts.askUserQuestionPrompter); + } + } + private async ensureBackend( backendId: BackendId, descriptor: BackendDescriptor @@ -1072,7 +1102,7 @@ export class AgentSessionManager { if (warm) { // Probe subprocess is already started + initialize-handshaken — // wire it into the manager without paying either cost again. - warm.proc.setPermissionPrompter(this.opts.permissionPrompter); + this.wirePrompters(warm.proc); this.installBackendExitHandler(backendId, warm.proc, descriptor); this.backends.set(backendId, warm.proc); return { proc: warm.proc, warm }; @@ -1088,7 +1118,7 @@ export class AgentSessionManager { // ACP backends declare `start()` to spawn the subprocess and run the // initialize handshake. In-process adapters (Claude SDK) omit it. if (proc.start) await proc.start(); - proc.setPermissionPrompter(this.opts.permissionPrompter); + this.wirePrompters(proc); this.installBackendExitHandler(backendId, proc, descriptor); this.backends.set(backendId, proc); return proc; diff --git a/src/agentMode/session/index.ts b/src/agentMode/session/index.ts index 0825d940..05c0ddd0 100644 --- a/src/agentMode/session/index.ts +++ b/src/agentMode/session/index.ts @@ -1,6 +1,7 @@ export { AgentSessionManager, type AgentSessionManagerOptions, + type AskUserQuestionPrompter, type PermissionPrompter, } from "./AgentSessionManager"; export { AgentSession, type AgentSessionStatus, type AgentSessionListener } from "./AgentSession"; @@ -16,6 +17,9 @@ export type { AgentToolKind, AgentToolStatus, AgentPlanEntry, + AgentQuestion, + AgentQuestionAnswers, + AskUserQuestionPrompt, NewAgentChatMessage, PermissionDecision, PermissionPrompt, diff --git a/src/agentMode/session/types.ts b/src/agentMode/session/types.ts index 96877fef..28362d4e 100644 --- a/src/agentMode/session/types.ts +++ b/src/agentMode/session/types.ts @@ -489,6 +489,42 @@ export interface PermissionDecision { denyMessage?: string; } +// ---- Ask-user-question (inline multiple-choice prompt) ----------------- + +/** + * One question in an `AskUserQuestionPrompt` — a `header`/`question` pair plus + * a single- or multi-select option list. Mirrors the Claude SDK's + * `AskUserQuestion` tool input; backends translate to this shape at the + * boundary so the inline card stays backend-agnostic. + */ +export interface AgentQuestion { + question: string; + header?: string; + options: Array<{ label: string; description?: string }>; + multiSelect?: boolean; +} + +/** + * Answer map keyed by question text. Single-select values are the chosen + * option label; multi-select values are the chosen labels joined with `, `. + * An empty map signals cancellation (the bridge maps it to a deny). + */ +export type AgentQuestionAnswers = { [questionText: string]: string }; + +/** + * A request from the backend asking the user to answer one or more inline + * multiple-choice questions (Claude SDK's `AskUserQuestion` tool). Routed + * through the session-domain ask-question prompter and rendered as an inline + * card at the tail of the chat — the sibling of `PermissionPrompt`. + * `requestId` reuses the backend's tool-call id so the resolver can pair the + * answer with the originating call. + */ +export interface AskUserQuestionPrompt { + sessionId: SessionId; + requestId: string; + questions: AgentQuestion[]; +} + // ---- MCP server spec (neutral) ----------------------------------------- /** @@ -595,6 +631,17 @@ export interface BackendProcess { isRunning(): boolean; onExit(listener: () => void): () => void; setPermissionPrompter(fn: (req: PermissionPrompt) => Promise): void; + /** + * Optional: register the session-domain handler the backend calls when it + * needs the user to answer inline multiple-choice questions (Claude SDK's + * `AskUserQuestion`). Mirrors `setPermissionPrompter`; the prompter routes + * each request to its owning session, which surfaces an inline card and + * resolves the returned promise with the answers (or `{}` on cancel). + * Backends with no equivalent surface (ACP today) omit it. + */ + setAskUserQuestionPrompter?( + fn: (req: AskUserQuestionPrompt) => Promise + ): void; registerSessionHandler(sessionId: SessionId, handler: SessionUpdateHandler): () => void; newSession(params: OpenSessionInput): Promise; prompt(params: PromptInput): Promise; diff --git a/src/agentMode/ui/AgentChat.tsx b/src/agentMode/ui/AgentChat.tsx index 1749c9c7..5207d610 100644 --- a/src/agentMode/ui/AgentChat.tsx +++ b/src/agentMode/ui/AgentChat.tsx @@ -16,6 +16,7 @@ import { expandCustomCommandPrefix } from "@/agentMode/session/expandCustomComma import { resolveActiveNoteToken } from "@/agentMode/session/resolveActiveNoteToken"; import type { AgentChatMessage, + AskUserQuestionPrompt, CurrentPlan, PermissionPrompt, PromptContent, @@ -159,6 +160,9 @@ const AgentChatInternal: React.FC = ({ const [pendingToolPermissions, setPendingToolPermissions] = useState(() => backend.getPendingToolPermissions() ); + const [pendingAskUserQuestions, setPendingAskUserQuestions] = useState( + () => backend.getPendingAskUserQuestions() + ); const [chatHistoryItems, setChatHistoryItems] = useState([]); const [queuedMessages, setQueuedMessages] = useState([]); const [selectedTextContexts] = useSelectedTextContexts(); @@ -188,6 +192,7 @@ const AgentChatInternal: React.FC = ({ setHasPendingPlanPermission(backend.hasPendingPlanPermission()); setCurrentPlan(backend.getCurrentPlan()); setPendingToolPermissions(backend.getPendingToolPermissions()); + setPendingAskUserQuestions(backend.getPendingAskUserQuestions()); }; sync(); return backend.subscribe(() => { @@ -508,6 +513,7 @@ const AgentChatInternal: React.FC = ({ onDelete={handleDelete} currentPlan={currentPlan} pendingToolPermissions={pendingToolPermissions} + pendingAskUserQuestions={pendingAskUserQuestions} chatBackend={backend} isLoading={loading} /> diff --git a/src/agentMode/ui/AgentChatMessages.tsx b/src/agentMode/ui/AgentChatMessages.tsx index 92cf8fe0..0056c2b9 100644 --- a/src/agentMode/ui/AgentChatMessages.tsx +++ b/src/agentMode/ui/AgentChatMessages.tsx @@ -1,4 +1,5 @@ import { AgentTrail } from "@/agentMode/ui/AgentTrailView"; +import { AskUserQuestionCard } from "@/agentMode/ui/AskUserQuestionCard"; import { PlanProposalCard } from "@/agentMode/ui/PlanProposalCard"; import { ToolPermissionCard } from "@/agentMode/ui/ToolPermissionCard"; import { BottomLoadingIndicator } from "@/components/chat-components/BottomLoadingIndicator"; @@ -6,7 +7,12 @@ import ChatSingleMessage from "@/components/chat-components/ChatSingleMessage"; import { USER_SENDER } from "@/constants"; import { useChatScrolling } from "@/hooks/useChatScrolling"; import type { AgentChatBackend } from "@/agentMode/session/AgentChatBackend"; -import type { AgentChatMessage, CurrentPlan, PermissionPrompt } from "@/agentMode/session/types"; +import type { + AgentChatMessage, + AskUserQuestionPrompt, + CurrentPlan, + PermissionPrompt, +} from "@/agentMode/session/types"; import type { ChatMessage } from "@/types/message"; import { App } from "obsidian"; import React, { memo, useMemo } from "react"; @@ -17,6 +23,7 @@ interface AgentChatMessagesProps { onDelete: (messageId: string) => void; currentPlan: CurrentPlan | null; pendingToolPermissions: PermissionPrompt[]; + pendingAskUserQuestions: AskUserQuestionPrompt[]; chatBackend: AgentChatBackend; /** True while a turn is in flight. The last assistant message in the * visible list is treated as the streaming placeholder. */ @@ -48,6 +55,7 @@ const AgentChatMessages = memo( onDelete, currentPlan, pendingToolPermissions, + pendingAskUserQuestions, chatBackend, isLoading, }: AgentChatMessagesProps) => { @@ -68,7 +76,15 @@ const AgentChatMessages = memo( onResolve={chatBackend.resolveToolPermission.bind(chatBackend)} /> )); - const hasTailCards = showPlanCard || pendingToolPermissions.length > 0; + const inlineAskUserQuestionCards = pendingAskUserQuestions.map((req) => ( + + )); + const hasTailCards = + showPlanCard || pendingToolPermissions.length > 0 || pendingAskUserQuestions.length > 0; // The last visible assistant message is the streaming placeholder while // a turn is in flight — drives the reasoning-block timer/spinner and the @@ -100,6 +116,7 @@ const AgentChatMessages = memo( {isLoading && } {inlinePlanCard} {inlineToolPermissionCards} + {inlineAskUserQuestionCards} ); } @@ -169,6 +186,7 @@ const AgentChatMessages = memo( })} {inlinePlanCard} {inlineToolPermissionCards} + {inlineAskUserQuestionCards} ); diff --git a/src/agentMode/ui/AskUserQuestionCard.tsx b/src/agentMode/ui/AskUserQuestionCard.tsx new file mode 100644 index 00000000..fc534fc4 --- /dev/null +++ b/src/agentMode/ui/AskUserQuestionCard.tsx @@ -0,0 +1,200 @@ +import { Button } from "@/components/ui/button"; +import { cn } from "@/lib/utils"; +import type { + AgentQuestion, + AgentQuestionAnswers, + AskUserQuestionPrompt, +} from "@/agentMode/session/types"; +import { MessageCircleQuestion } from "lucide-react"; +import React, { useState } from "react"; + +interface AskUserQuestionCardProps { + request: AskUserQuestionPrompt; + onResolve: (requestId: string, answers: AgentQuestionAnswers) => void; +} + +/** A single-select pick is "answered" once a label is chosen; multi-select is always satisfiable. */ +function isAnswered(question: AgentQuestion, selection: string | Set | undefined): boolean { + if (question.multiSelect) return true; + return typeof selection === "string" && selection !== ""; +} + +/** + * Inline card rendered at the tail of the chat scroll container while the + * agent's `AskUserQuestion` tool waits on the user — the sibling of + * `ToolPermissionCard`. Replaces the old `AskUserQuestionModal`: modals steal + * focus and resolve as a cancel on accidental click-outside, which is + * inconsistent with the rest of Agent Mode's inline-card model. + * + * A single call may carry several questions; each renders under its own tab so + * the card stays compact, while the answers still submit together to honor the + * SDK's single-response contract. Submitting routes the answers map through the + * ask-question prompter's happy path; Cancel resolves with `{}`, which the + * bridge maps to the "User cancelled the question" deny. + */ +export const AskUserQuestionCard: React.FC = ({ request, onResolve }) => { + const { questions, requestId } = request; + const [busy, setBusy] = useState(false); + const [activeTab, setActiveTab] = useState(0); + // Per-question selection: a single label for radio, a Set of labels for checkbox. + const [selections, setSelections] = useState>>({}); + + // Gate Submit until every single-select question has a pick. Multi-select + // questions may be left empty (the user can decline every option). + const canSubmit = questions.every((q, idx) => isAnswered(q, selections[idx])); + + const submit = (): void => { + if (busy || !canSubmit) return; + setBusy(true); + const answers: AgentQuestionAnswers = {}; + for (let i = 0; i < questions.length; i++) { + const q = questions[i]; + const sel = selections[i]; + if (q.multiSelect) { + answers[q.question] = sel instanceof Set ? Array.from(sel).join(", ") : ""; + } else { + answers[q.question] = typeof sel === "string" ? sel : ""; + } + } + onResolve(requestId, answers); + }; + + const cancel = (): void => { + if (busy) return; + setBusy(true); + onResolve(requestId, {}); + }; + + const showTabs = questions.length > 1; + const active = questions[activeTab] ?? questions[0]; + const activeIdx = questions[activeTab] ? activeTab : 0; + + return ( +
+
+ +
Question from agent
+
+ +
+ {showTabs ? ( +
+ {questions.map((q, idx) => { + const selected = idx === activeIdx; + return ( + + ); + })} +
+ ) : null} + + + setSelections((prev) => { + if (active.multiSelect) { + const cur = prev[activeIdx]; + const next = new Set(cur instanceof Set ? cur : []); + if (next.has(label)) next.delete(label); + else next.add(label); + return { ...prev, [activeIdx]: next }; + } + return { ...prev, [activeIdx]: label }; + }) + } + /> +
+ +
+ + +
+
+ ); +}; + +interface QuestionPanelProps { + question: AgentQuestion; + /** Radio-group name; namespaced by requestId + index so cards don't collide. */ + name: string; + selection: string | Set | undefined; + disabled: boolean; + onToggle: (label: string) => void; +} + +/** The active question's prompt text plus its single- or multi-select option list. */ +const QuestionPanel: React.FC = ({ + question, + name, + selection, + disabled, + onToggle, +}) => { + return ( +
+
{question.question}
+
+ {question.options.map((opt) => { + const checked = question.multiSelect + ? selection instanceof Set && selection.has(opt.label) + : selection === opt.label; + return ( + + ); + })} +
+
+ ); +}; diff --git a/src/agentMode/ui/index.ts b/src/agentMode/ui/index.ts index 9b4030e5..ee6d6c2e 100644 --- a/src/agentMode/ui/index.ts +++ b/src/agentMode/ui/index.ts @@ -7,4 +7,7 @@ export { useBackendInstallState, useSessionBackendDescriptor, } from "./useBackendDescriptor"; -export { createDefaultPermissionPrompter } from "./permissionPrompter"; +export { + createDefaultAskUserQuestionPrompter, + createDefaultPermissionPrompter, +} from "./permissionPrompter"; diff --git a/src/agentMode/ui/permissionPrompter.ts b/src/agentMode/ui/permissionPrompter.ts index 23ea11e5..d644a4b3 100644 --- a/src/agentMode/ui/permissionPrompter.ts +++ b/src/agentMode/ui/permissionPrompter.ts @@ -1,5 +1,8 @@ import type { AgentSession } from "@/agentMode/session/AgentSession"; -import type { PermissionPrompter } from "@/agentMode/session/AgentSessionManager"; +import type { + AskUserQuestionPrompter, + PermissionPrompter, +} from "@/agentMode/session/AgentSessionManager"; import type { SessionId } from "@/agentMode/session/types"; /** @@ -21,3 +24,20 @@ export function createDefaultPermissionPrompter( return session.handleToolPermission(req); }; } + +/** + * AskUserQuestion requests route into the owning session so the user answers + * via an inline card in the chat instead of a modal — the sibling of + * `createDefaultPermissionPrompter`. Returns `{}` (the cancellation signal) + * when no session owns the request, so the SDK turn unblocks with the standard + * cancellation deny instead of hanging on a dangling promise. + */ +export function createDefaultAskUserQuestionPrompter( + resolveSession: (backendSessionId: SessionId) => AgentSession | null +): AskUserQuestionPrompter { + return (req) => { + const session = resolveSession(req.sessionId); + if (!session) return Promise.resolve({}); + return session.handleAskUserQuestion(req); + }; +} diff --git a/src/styles/tailwind.css b/src/styles/tailwind.css index 31680654..52fcccb0 100644 --- a/src/styles/tailwind.css +++ b/src/styles/tailwind.css @@ -20,12 +20,16 @@ If your plugin does not need CSS, delete this file. z-index: 9999 !important; } -/* Bottom-only divider. Preflight is off, so `tw-border-b tw-border-solid` +/* Single-side dividers. Preflight is off, so `tw-border-b tw-border-solid` leaks `border-style: solid` to all sides and renders all four borders. */ .copilot-divider-b { border-bottom: 1px solid var(--background-modifier-border); } +.copilot-divider-t { + border-top: 1px solid var(--background-modifier-border); +} + .button-container { display: flex; justify-content: space-between;