diff --git a/ts/packages/agentServer/docs/async-clientio-design.md b/ts/packages/agentServer/docs/async-clientio-design.md index ec8a8a459..c55392af8 100644 --- a/ts/packages/agentServer/docs/async-clientio-design.md +++ b/ts/packages/agentServer/docs/async-clientio-design.md @@ -6,9 +6,9 @@ ## Overview -Three `ClientIO` methods — `askYesNo`, `proposeAction`, and `popupQuestion` — use a **non-blocking, deferred-promise pattern** in the agent-server's `SharedDispatcher`. Rather than blocking on a synchronous RPC round-trip to the originating client, the server broadcasts a fire-and-forget notification, suspends execution via a stored `Promise`, and resumes when any connected client responds. This allows commands to survive client disconnects, supports multi-client sessions, and integrates with the `DisplayLog` for session replay. +Two `ClientIO` methods — `question` and `proposeAction` — use a **non-blocking, deferred-promise pattern** in the agent-server's `SharedDispatcher`. Rather than blocking on a synchronous RPC round-trip to the originating client, the server broadcasts a fire-and-forget notification, suspends execution via a stored `Promise`, and resumes when any connected client responds. This allows commands to survive client disconnects, supports multi-client sessions, and integrates with the `DisplayLog` for session replay. -The `ClientIO` interface signatures are unchanged — callers receive a `Promise` as before. Only the server-side fulfillment mechanism differs. +The `ClientIO` interface signatures are unchanged — callers receive a `Promise` as before. Only the server-side fulfillment mechanism differs. --- @@ -17,7 +17,7 @@ The `ClientIO` interface signatures are unchanged — callers receive a `Promise ### Flow ``` -Agent code → askYesNoWithContext() → context.clientIO.askYesNo() +Agent code → askYesNoWithContext() → context.clientIO.question() → SharedDispatcher: 1. Broadcast requestInteraction (fire-and-forget) to eligible clients 2. Log pending-interaction to DisplayLog @@ -31,11 +31,11 @@ Agent code → askYesNoWithContext() → context.clientIO.askYesNo() → Agent code continues ``` -`popupQuestion` is identical except it broadcasts to all clients (it has no `requestId`). `proposeAction` follows the same pattern as `askYesNo` but throws instead of returning a default when no clients are connected. +`popupQuestion` (via `SessionContext`) follows the same path as `askYesNoWithContext` — both delegate to `clientIO.question()`. `proposeAction` follows the same deferred pattern but throws instead of returning a default when no clients are connected. ### Key invariant: always log -`logPendingInteraction` is called unconditionally for all three interaction types — even when no clients are currently connected. This ensures that a client which reconnects within the timeout window can see the pending interaction in `JoinSessionResult.pendingInteractions` and respond to it. The log and `PendingInteractionManager` entry are created before the broadcast so the interaction is visible to any client joining concurrently. +`logPendingInteraction` is called unconditionally for both interaction types — even when no clients are currently connected. This ensures that a client which reconnects within the timeout window can see the pending interaction in `JoinSessionResult.pendingInteractions` and respond to it. The log and `PendingInteractionManager` entry are created before the broadcast so the interaction is visible to any client joining concurrently. ### Routing @@ -44,7 +44,7 @@ The `broadcast()` helper respects each client's `filter` setting: - `filter: false` (default) — receives all messages, plus those routed to its own `connectionId` - `filter: true` — receives only messages routed to its own `connectionId` -`askYesNo` and `proposeAction` broadcast to clients eligible for `requestId.connectionId`. `popupQuestion` broadcasts to all clients unconditionally. +`question` and `proposeAction` broadcast to clients eligible for `requestId.connectionId`. `popupQuestion` (which passes no `requestId`) broadcasts to all clients unconditionally. ### Pending Interaction Manager @@ -52,18 +52,18 @@ The `broadcast()` helper respects each client's `filter` setting: - `create(request, timeoutMs)` — stores the deferred Promise, sets an optional timeout - `resolve(interactionId, value)` — fulfills the Promise; returns false if not found -- `cancel(interactionId, error)` — for `askYesNo`: resolves with `defaultValue` if provided, otherwise rejects; for `proposeAction` and `popupQuestion`: always rejects +- `cancel(interactionId, error)` — for `question` with a `defaultId`: resolves with that index; otherwise rejects; for `proposeAction`: always rejects - `getPending()` — returns all in-flight `PendingInteractionRequest` objects ### Timeouts -All three types use a 10-minute timeout, kept as separate constants so they can be tuned independently. On timeout, `cancel()` is called with a timeout error. +Both types use a 10-minute timeout, kept as separate constants so they can be tuned independently. On timeout, `cancel()` is called with a timeout error. ### Client Disconnect Disconnecting a client does **not** automatically cancel pending interactions. Interactions remain pending until they time out or a client explicitly calls `cancelInteraction`. This allows a reconnecting client to respond to the same interaction within the timeout window. -All three interaction types log unconditionally and survive in the pending map if no client is connected, so a later-joining client will see them in `JoinSessionResult.pendingInteractions`. However, with the current ephemeral `connectionId` design, a reconnecting client's new `connectionId` will not match `requestId.connectionId` for `askYesNo`/`proposeAction` interactions and they will be filtered out — see Open Question 5. +Both interaction types log unconditionally and survive in the pending map if no client is connected, so a later-joining client will see them in `JoinSessionResult.pendingInteractions`. However, with the current ephemeral `connectionId` design, a reconnecting client's new `connectionId` will not match the stored one for `question`/`proposeAction` interactions and they will be filtered out — see Open Question 5. Clients that want to cancel an interaction (e.g., on user dismissal) call `dispatcher.cancelInteraction(interactionId)`, which triggers: @@ -106,33 +106,22 @@ cancelInteraction(interactionId: string): void; // se ### Types (`@typeagent/dispatcher-types`) ```typescript -export type PendingInteractionType = - | "askYesNo" - | "proposeAction" - | "popupQuestion"; +export type PendingInteractionType = "question" | "proposeAction"; export type PendingInteractionRequest = { interactionId: string; type: PendingInteractionType; - requestId?: RequestId; // absent for popupQuestion + requestId?: RequestId; source: string; timestamp: number; } & ( - | { type: "askYesNo"; message: string; defaultValue?: boolean } + | { type: "question"; message: string; choices: string[]; defaultId?: number } | { type: "proposeAction"; actionTemplates: TemplateEditConfig } - | { - type: "popupQuestion"; - message: string; - choices: string[]; - defaultId?: number; - } ); -export type PendingInteractionResponse = { - interactionId: string; - type: PendingInteractionType; - value: boolean | unknown | number; -}; +export type PendingInteractionResponse = + | { interactionId: string; type: "question"; value: number } + | { interactionId: string; type: "proposeAction"; value: unknown }; ``` --- @@ -151,7 +140,7 @@ The shell and CLI `requestInteraction`/`interactionResolved`/`interactionCancell Currently broadcast to all clients; first responder wins. Other clients receive `interactionResolved` and dismiss their UI. Routing to a designated primary client would be more precise but requires adding a `connectionId` to the `popupQuestion` signature. -2. **Timeouts** — resolved: 10 minutes for all three types (separate constants). +2. **Timeouts** — resolved: 10 minutes for all types (separate constants). 3. **Unify `requestChoice`/`respondToChoice` with this system?** @@ -159,7 +148,7 @@ The shell and CLI `requestInteraction`/`interactionResolved`/`interactionCancell 4. **Grace period on disconnect** — disconnecting a client does not auto-cancel pending interactions. Interactions remain pending until they time out or a client explicitly calls `cancelInteraction`. Reconnecting clients see pending interactions in `JoinSessionResult.pendingInteractions` and can respond or cancel them. -5. **Stable client identity for reconnect routing** — `askYesNo`/`proposeAction` interactions capture `requestId.connectionId` at creation time, but `connectionId` is ephemeral: each `join()` call mints a new value. A reconnecting client therefore gets a new `connectionId` that never matches the stored one, so `getPendingInteractions()` filters those interactions out and they become permanently unresolvable until timeout. +5. **Stable client identity for reconnect routing** — `question`/`proposeAction` interactions capture `requestId.connectionId` at creation time, but `connectionId` is ephemeral: each `join()` call mints a new value. A reconnecting client therefore gets a new `connectionId` that never matches the stored one, so `getPendingInteractions()` filters those interactions out and they become permanently unresolvable until timeout. Two candidate fixes: @@ -172,25 +161,25 @@ The shell and CLI `requestInteraction`/`interactionResolved`/`interactionCancell ## Sequence Diagrams -### askYesNo: Normal Flow +### question: Normal Flow ``` Client SharedDispatcher PendingInteractionMgr DisplayLog | | | | - | (command in progress, agent calls askYesNo) | | + | (command in progress, agent calls question()) | | |<-- requestInteraction-| | | | |--- logPendingInteraction ---------------------> | | |--- create(request) -------> | - | |<-- Promise -------| | + | |<-- Promise --------| | | | (server suspends here) | | - | (user clicks Yes) | | | + | (user picks "1") | | | |-- respondToInteraction --> | | - | |--- resolve(id, true) ---->| | + | |--- resolve(id, 0) ------->| | | |--- logInteractionResolved --------------------> | | | (Promise resolves, command continues) | ``` -### askYesNo: Client Disconnect (interaction survives) +### question: Client Disconnect (interaction survives) ``` Client SharedDispatcher PendingInteractionMgr @@ -219,7 +208,7 @@ ClientA ClientB SharedDispatcher PendingInteracti | |<-- requestInteraction --| | | (user answers 1) | | | |-- respondToInteraction ---------------->| | - | | |--- resolve(id, 1) ------>| - | |<-- interactionResolved (id, 1) --| | + | | |--- resolve(id, 0) ------>| + | |<-- interactionResolved (id, 0) --| | | | (ClientB dismisses UI) | | ``` diff --git a/ts/packages/agentServer/docs/multi-client-interactions.md b/ts/packages/agentServer/docs/multi-client-interactions.md index 99f226a2d..cc55142b7 100644 --- a/ts/packages/agentServer/docs/multi-client-interactions.md +++ b/ts/packages/agentServer/docs/multi-client-interactions.md @@ -18,19 +18,19 @@ A `SharedDispatcher` is a single dispatcher instance shared among all clients co ``` Server dispatcher Client A Client B ───────────────── ──────── ──────── -askYesNo() called +question() called create PendingInteraction broadcast requestInteraction ──► show prompt show prompt await promise... - user answers "y" + user picks "1" respondToInteraction ──► server resolves promise broadcast interactionResolved ──► dismiss prompt promise resolves execution continues ``` -The same flow applies to `proposeAction` and `popupQuestion`. +The same flow applies to `proposeAction`. ### Broadcast vs. targeted routing @@ -41,9 +41,8 @@ Interaction calls are **broadcast** to all clients, regardless of which client i | ClientIO method | Routing | Notes | | ---------------------- | --------------------------------- | ------------------------------------------------------------------ | | `setDisplay` | Broadcast (filtered by requestId) | All clients see output | -| `askYesNo` | Deferred broadcast | Creates a `PendingInteraction`; broadcast via `requestInteraction` | +| `question` | Deferred broadcast | Creates a `PendingInteraction`; broadcast via `requestInteraction` | | `proposeAction` | Deferred broadcast | Same pattern | -| `popupQuestion` | Deferred broadcast | Same pattern | | `requestInteraction` | Broadcast | Sent to all clients to show the prompt | | `interactionResolved` | Broadcast | Sent to all clients after any one responds | | `interactionCancelled` | Broadcast | Sent to all clients after `cancelInteraction` or timeout | @@ -56,6 +55,29 @@ Pending interactions have a 10-minute timeout (configurable per-type in `sharedD Each `join()` call mints a new ephemeral `connectionId`. On reconnect a client receives a fresh `connectionId`, so interactions created before the disconnect (whose `requestId.connectionId` names the old connection) are not re-broadcast to the new connection. A joining client receives any still-pending interactions via `JoinSessionResult.pendingInteractions` and is responsible for displaying them and potentially responding. +## Interaction Types + +### `question` + +The `question` type is the single unified prompt type for all choice-based interactions. Choices are always explicit strings; the response is the 0-based index of the selected choice. + +```typescript +// Request +{ type: "question"; message: string; choices: string[]; defaultId?: number } + +// Response +{ interactionId: string; type: "question"; value: number } +``` + +Caller ergonomics on `SessionContext` are preserved with thin wrappers: + +- **`popupQuestion(message, choices?, defaultId?)`** — delegates directly to `clientIO.question()`. +- **`askYesNoWithContext(context, message, defaultValue?)`** — calls `clientIO.question()` with `choices: ["Yes", "No"]`, maps `defaultValue: boolean` to `defaultId: 0 | 1`, and converts the returned index back to a boolean. + +### `proposeAction` + +`proposeAction` remains intentionally separate — it renders a structured template editor rather than a text prompt, and its response type is `unknown`. It is not yet supported in the CLI client. + ## Client Responsibilities Every `ClientIO` implementation must handle three interaction-related methods. @@ -64,7 +86,7 @@ Every `ClientIO` implementation must handle three interaction-related methods. Called when the server needs the client to show a prompt. The client must: -1. Display the appropriate UI for the interaction type (`askYesNo`, `popupQuestion`, or `proposeAction`). +1. Display the appropriate UI for the interaction type (`question` or `proposeAction`). 2. Register the interaction locally (keyed by `interaction.interactionId`) so it can be dismissed later. 3. When the user responds, call `dispatcher.respondToInteraction(response)`. 4. Remove the local registration after responding or after being dismissed. @@ -93,72 +115,65 @@ The CLI (`enhancedConsole.ts`) owns stdin and renders prompts inline in the term ### Active prompt registry -A `Map void }>` keyed by `interactionId`, local to `createEnhancedClientIO`. Each entry holds a function that aborts the in-progress `question()` call for that interaction. +A `Map` keyed by `interactionId`, local to `createEnhancedClientIO`. Each entry holds an `AbortController` whose signal is passed to the `question()` call so it can be aborted remotely. ```typescript -const activeInteractions = new Map void }>(); +const activeInteractions = new Map(); ``` ### Cancellable `question()` races -`requestInteraction` wraps the `question()` call in a `Promise.race` against a cancellation promise. The cancellation promise rejects when `cancel()` is called (by `interactionResolved` or `interactionCancelled`). +`requestInteraction` creates an `AbortController`, stores it in `activeInteractions`, and passes its signal to the underlying `question()` readline call. When `abort()` is called (by `interactionResolved` or `interactionCancelled`), the pending readline rejects with the abort reason. ``` requestInteraction(interaction): - cancelled = false - cancelFn = () => { cancelled = true; rejectCancelPromise() } - activeInteractions.set(interaction.interactionId, { cancel: cancelFn }) + ac = new AbortController() + activeInteractions.set(interaction.interactionId, ac) try: - input = await Promise.race([question(prompt), cancelPromise]) - if (!cancelled): - build response - await dispatcher.respondToInteraction(response) - catch (CancelledError): - // dismissed — print notice if resolved by another client vs. cancelled + display numbered choice menu + input = await question(prompt, rl, ac.signal) + value = parse input as 1-based index + await dispatcher.respondToInteraction({ type: "question", value }) + catch (AbortError): + if reason.kind === "resolved-by-other": + print "[answered by another client]" + else: + print "Cancelled!" finally: activeInteractions.delete(interaction.interactionId) ``` -The `question()` function itself must also clean up its stdin listener when the outer race rejects, to avoid a leaked `data` listener on stdin. +The `question()` function itself must also clean up its stdin listener when the signal fires, to avoid a leaked `data` listener on stdin. ### `interactionResolved` and `interactionCancelled` ```typescript interactionResolved(interactionId, response): void { - activeInteractions.get(interactionId)?.cancel(); - // "resolved" variant — optionally print "[answered by another client]" + const ac = activeInteractions.get(interactionId); + if (ac) { + activeInteractions.delete(interactionId); + ac.abort({ kind: "resolved-by-other", response }); + } }, interactionCancelled(interactionId): void { - activeInteractions.get(interactionId)?.cancel(); - // "cancelled" variant — optionally print "[interaction cancelled]" + const ac = activeInteractions.get(interactionId); + if (ac) { + activeInteractions.delete(interactionId); + ac.abort(INTERACTION_CANCELLED); + } }, ``` -The cancel functions must distinguish resolved vs. cancelled so the client can print the appropriate notice. This can be done by passing a reason string to `cancel()`, or by using two separate rejection types. +The abort reasons distinguish resolved vs. cancelled so the client can print the appropriate notice. ## Shell Implementation Notes -The Shell (`main.ts`) is not yet implemented (stubs in place). The same pattern applies: hold a `Map` and call `dismissFn` from `interactionResolved`/`interactionCancelled`. The UI (modal dialog or inline card) should be dismissed programmatically and a toast shown if resolved by a remote client. +The Shell (`main.ts`) is not yet implemented (stubs in place). The same pattern applies: hold a `Map` and call `abort()` from `interactionResolved`/`interactionCancelled`. The UI (modal dialog or inline card) should be dismissed programmatically and a toast shown if resolved by a remote client. ## Future Work -### Unify `askYesNo` and `popupQuestion` into a single `question` type - -`askYesNo` is a special case of `popupQuestion` — a two-choice prompt where choices are implicitly `["Yes", "No"]` and the response is a boolean rather than an index. The protocol could be simplified by collapsing both into a single `question` interaction type: - -```typescript -// Unified request -{ type: "question"; message: string; choices: string[]; defaultId?: number } - -// Unified response -{ interactionId: string; type: "question"; value: number } -``` - -Caller ergonomics on `SessionContext` can be preserved with thin wrappers that map the boolean/index conversion. `proposeAction` remains intentionally separate — it renders a structured template editor rather than a text prompt, and its response type is `unknown`. - -Benefits: - -- One code path in all `ClientIO` implementations instead of two -- Simpler discriminated union in `PendingInteractionRequest` / `PendingInteractionResponse` -- Consistent rendering logic across CLI, Shell, and future clients +- **Shell `question` support**: implement `requestInteraction` in `main.ts` using the same AbortController pattern as the CLI. +- **`proposeAction` in CLI**: render the structured template editor inline in the terminal. +- **Stable client identity**: allow a reconnecting client to reclaim its old `connectionId` so in-flight interactions are re-broadcast rather than orphaned (see `TODO` in `sharedDispatcher.ts` `join()`). +- **Freeform text input**: add a `textInput` interaction type for open-ended questions where the agent needs a free-form string from the user (e.g. "What name should I use?"). This would follow the same broadcast-and-race pattern as `question`, with a `{ type: "textInput"; message: string; defaultValue?: string }` request and a `{ type: "textInput"; value: string }` response. A precedent exists in `typeagent/src/chat.ts` (`ChatUserInterface.getInput()`) and `knowledgeProcessor` which use this pattern outside the dispatcher layer. diff --git a/ts/packages/agentServer/server/src/sharedDispatcher.ts b/ts/packages/agentServer/server/src/sharedDispatcher.ts index af3710fba..e605346cb 100644 --- a/ts/packages/agentServer/server/src/sharedDispatcher.ts +++ b/ts/packages/agentServer/server/src/sharedDispatcher.ts @@ -50,9 +50,8 @@ export async function createSharedDispatcher( // Timeouts for pending interactions. All currently set to 10 minutes but // kept separate so they can be tuned independently. const INTERACTION_TIMEOUT_MS = { - askYesNo: 10 * 60 * 1000, + question: 10 * 60 * 1000, proposeAction: 10 * 60 * 1000, - popupQuestion: 10 * 60 * 1000, }; // Returns the number of clients the message was sent to. @@ -148,22 +147,21 @@ export async function createSharedDispatcher( // and broadcast the request to all clients. The first client to // respond via respondToInteraction resolves the promise. - askYesNo: async (requestId, message, defaultValue?) => { + question: async (requestId, message, choices, defaultId?, source?) => { const interactionId = randomUUID(); const request: PendingInteractionRequest = { interactionId, - type: "askYesNo", - requestId, - source: requestId.connectionId ?? "unknown", + type: "question", + ...(requestId !== undefined ? { requestId } : {}), + source: source ?? requestId?.connectionId ?? "unknown", timestamp: Date.now(), message, + choices, + ...(defaultId !== undefined ? { defaultId } : {}), }; - if (defaultValue !== undefined) { - request.defaultValue = defaultValue; - } debugInteraction( - `askYesNo created: ${interactionId} message="${message}"`, + `question created: ${interactionId} message="${message}"`, ); // Broadcast to all connected clients @@ -171,20 +169,12 @@ export async function createSharedDispatcher( cio.requestInteraction(request), ); - // Log unconditionally so the interaction survives in the DisplayLog - // and is included in JoinSessionResult.pendingInteractions on the - // next join. Cancellation is explicit-only (cancelInteraction or - // timeout) — there is no auto-cancel on disconnect. - // Note: a reconnecting client will receive a new connectionId, so - // requestId.connectionId will no longer match and the interaction - // will be filtered out of getPendingInteractions until a stable - // client identity is introduced (see TODO in join()). context.displayLog.logPendingInteraction(request); context.displayLog.saveQueued(); - return pendingInteractions.create( + return pendingInteractions.create( request, - INTERACTION_TIMEOUT_MS.askYesNo, + INTERACTION_TIMEOUT_MS.question, ); }, @@ -203,11 +193,9 @@ export async function createSharedDispatcher( `proposeAction created: ${interactionId} source="${source}"`, ); - // Log and queue unconditionally — same reasoning as askYesNo: the - // interaction must be in the DisplayLog and PendingInteractionManager - // before any broadcast so that a joining client sees it in - // JoinSessionResult.pendingInteractions even if no client is - // currently connected. + // Log and queue unconditionally so the interaction survives in + // the DisplayLog and is included in JoinSessionResult.pendingInteractions + // on the next join. context.displayLog.logPendingInteraction(request); context.displayLog.saveQueued(); @@ -221,43 +209,6 @@ export async function createSharedDispatcher( ); }, - popupQuestion: async (message, choices, defaultId, source) => { - const interactionId = randomUUID(); - const request: PendingInteractionRequest = { - interactionId, - type: "popupQuestion", - source, - timestamp: Date.now(), - message, - choices, - }; - if (defaultId !== undefined) { - request.defaultId = defaultId; - } - - debugInteraction( - `popupQuestion created: ${interactionId} message="${message}"`, - ); - - // Log and queue unconditionally — same reasoning as askYesNo: the - // interaction must be in the DisplayLog and PendingInteractionManager - // before any broadcast so that a joining client sees it in - // JoinSessionResult.pendingInteractions even if no client is - // currently connected. - context.displayLog.logPendingInteraction(request); - context.displayLog.saveQueued(); - - // popupQuestion has no requestId, so broadcast to all - broadcast("requestInteraction", undefined, (cio) => - cio.requestInteraction(request), - ); - - return pendingInteractions.create( - request, - INTERACTION_TIMEOUT_MS.popupQuestion, - ); - }, - notify: (notificationId, ...args) => { broadcast( "notify", diff --git a/ts/packages/agents/browser/src/extension/serviceWorker/dispatcherConnection.ts b/ts/packages/agents/browser/src/extension/serviceWorker/dispatcherConnection.ts index 9717af6da..8fd9b2e60 100644 --- a/ts/packages/agents/browser/src/extension/serviceWorker/dispatcherConnection.ts +++ b/ts/packages/agents/browser/src/extension/serviceWorker/dispatcherConnection.ts @@ -42,7 +42,7 @@ let connectionPromise: Promise | undefined; // RPC functions for communicating with the chat panel. // rpcSend: fire-and-forget (display updates) -// rpcInvoke: awaited (askYesNo, proposeAction) +// rpcInvoke: awaited (question, proposeAction) // Set by setChatPanelRpc() from the service worker index after the RPC server is created. let rpcSend: ((name: string, ...args: any[]) => void) | undefined; let rpcInvoke: ((name: string, ...args: any[]) => Promise) | undefined; @@ -102,18 +102,25 @@ function createChatPanelClientIO(): ClientIO { }); }, - async askYesNo(_requestId, message, defaultValue) { - if (rpcInvoke) { + async question(_requestId, message, choices, defaultId) { + // For Yes/No, delegate to the chat panel RPC. + if ( + choices.length === 2 && + choices[0] === "Yes" && + choices[1] === "No" && + rpcInvoke + ) { try { - return await rpcInvoke("chatPanelAskYesNo", { + const yes = await rpcInvoke("chatPanelAskYesNo", { message, - defaultValue, + defaultValue: defaultId === 0, }); + return yes ? 0 : 1; } catch { - return defaultValue ?? true; + return defaultId ?? 0; } } - return defaultValue ?? true; + return defaultId ?? 0; }, async proposeAction(_requestId, actionTemplates, source) { if (rpcInvoke) { @@ -130,10 +137,6 @@ function createChatPanelClientIO(): ClientIO { } return undefined; }, - async popupQuestion(_message, _choices, defaultId, _source) { - return defaultId ?? 0; - }, - notify(notificationId, event, data, source) { rpcSend?.("dispatcherNotify", { notificationId, diff --git a/ts/packages/agents/onboarding/src/testing/testingHandler.ts b/ts/packages/agents/onboarding/src/testing/testingHandler.ts index f182d7d80..6dfd8c3a7 100644 --- a/ts/packages/agents/onboarding/src/testing/testingHandler.ts +++ b/ts/packages/agents/onboarding/src/testing/testingHandler.ts @@ -513,11 +513,13 @@ function createCapturingClientIO(buffer: string[]): ClientIO { }, appendDiagnosticData: noop, setDynamicDisplay: noop, - askYesNo: async (_id: RequestId, _msg: string, def = false) => def, + question: async ( + _requestId: RequestId | undefined, + _msg: string, + _choices: string[], + defaultId?: number, + ) => defaultId ?? 0, proposeAction: async () => undefined, - popupQuestion: async () => { - throw new Error("popupQuestion not supported in test runner"); - }, notify: noop, openLocalView: async () => {}, closeLocalView: async () => {}, diff --git a/ts/packages/cli/src/enhancedConsole.ts b/ts/packages/cli/src/enhancedConsole.ts index 7b6780563..f41e64ba3 100644 --- a/ts/packages/cli/src/enhancedConsole.ts +++ b/ts/packages/cli/src/enhancedConsole.ts @@ -380,6 +380,31 @@ type InteractionResolvedReason = { }; const INTERACTION_CANCELLED = "cancelled"; +function isYesNoChoices(choices: string[]): boolean { + return choices.length === 2 && choices[0] === "Yes" && choices[1] === "No"; +} + +/** + * Parse a user's raw input string into a 0-based choice index. + * Accepts: + * - 1-based numeric input ("1", "2", …) + * - "y" / "yes" → 0 and "n" / "no" → 1 when choices are ["Yes", "No"] + * Falls back to defaultId (or 0) when the input is unrecognised. + */ +function parseChoiceInput( + input: string, + choices: string[], + defaultId: number | undefined, +): number { + const trimmed = input.trim().toLowerCase(); + if (isYesNoChoices(choices)) { + if (trimmed === "y" || trimmed === "yes") return 0; + if (trimmed === "n" || trimmed === "no") return 1; + } + const idx = parseInt(trimmed, 10) - 1; + return idx >= 0 && idx < choices.length ? idx : (defaultId ?? 0); +} + export function createEnhancedClientIO( rl?: readline.promises.Interface, dispatcherRef?: { current?: Dispatcher }, @@ -617,67 +642,12 @@ export function createEnhancedClientIO( // REVIEW: Ignored. }, - // Input - Enhanced yes/no with visual styling - async askYesNo( - requestId: RequestId, - message: string, - defaultValue?: boolean, - ): Promise { - // Pause spinner during input - const wasSpinning = currentSpinner?.isActive(); - if (wasSpinning) { - currentSpinner!.stop(); - } - - // Draw styled prompt - const width = process.stdout.columns || 80; - const line = ANSI.dim + "─".repeat(width) + ANSI.reset; - - process.stdout.write("\n"); - process.stdout.write(line + "\n"); - - const defaultHint = - defaultValue === undefined - ? "" - : defaultValue - ? " (default: yes)" - : " (default: no)"; - - const prompt = `${chalk.cyan("?")} ${message}${chalk.dim(defaultHint)} ${chalk.dim("(y/n)")} `; - - const input = await question(prompt, rl); - process.stdout.write(line + "\n"); - - // Resume spinner if it was active - if (wasSpinning) { - currentSpinner = new EnhancedSpinner({ text: "Processing..." }); - currentSpinner.start(); - } - - if (input.toLowerCase() === "y" || input.toLowerCase() === "yes") { - return true; - } - if (input.toLowerCase() === "n" || input.toLowerCase() === "no") { - return false; - } - return defaultValue ?? false; - }, - - async proposeAction( - requestId: RequestId, - actionTemplates: TemplateEditConfig, - source: string, - ): Promise { - // TODO: Not implemented - return undefined; - }, - - // Multiple choice with visual menu - async popupQuestion( + // Input — unified question prompt (handles both yes/no and multi-choice) + async question( + _requestId: RequestId | undefined, message: string, choices: string[], - defaultId: number | undefined, - source: string, + defaultId?: number, ): Promise { // Pause spinner during input const wasSpinning = currentSpinner?.isActive(); @@ -690,7 +660,7 @@ export function createEnhancedClientIO( process.stdout.write("\n"); process.stdout.write(line + "\n"); - process.stdout.write(`${chalk.cyan("?")} ${message}\n`); + process.stdout.write(`${message}\n`); process.stdout.write(line + "\n\n"); // Display choices with numbers @@ -704,7 +674,11 @@ export function createEnhancedClientIO( }); process.stdout.write("\n"); - const prompt = `${chalk.dim("Enter number (1-" + choices.length + "):")} `; + const prompt = chalk.dim( + isYesNoChoices(choices) + ? "Enter y/n or number (1-2): " + : `Enter number (1-${choices.length}): `, + ); const input = await question(prompt, rl); process.stdout.write(line + "\n"); @@ -715,15 +689,16 @@ export function createEnhancedClientIO( currentSpinner.start(); } - const selectedIndex = parseInt(input, 10) - 1; - if ( - isNaN(selectedIndex) || - selectedIndex < 0 || - selectedIndex >= choices.length - ) { - return defaultId ?? 0; - } - return selectedIndex; + return parseChoiceInput(input, choices, defaultId); + }, + + async proposeAction( + _requestId: RequestId, + _actionTemplates: TemplateEditConfig, + _source: string, + ): Promise { + // TODO: Not implemented + return undefined; }, // Notification with visual styling @@ -831,7 +806,7 @@ export function createEnhancedClientIO( let response: boolean | number[]; if (type === "yesNo") { - const prompt = `${chalk.cyan("?")} ${chalk.dim(`[${source}]`)} ${message} ${chalk.dim("(y/n)")} `; + const prompt = `${chalk.dim(`[${source}]`)} ${message} ${chalk.dim("(y/n)")} `; const input = await question(prompt, rl); response = input.toLowerCase() === "y" || @@ -839,7 +814,7 @@ export function createEnhancedClientIO( } else { // multiChoice — show numbered list, accept comma-separated process.stdout.write( - `${chalk.cyan("?")} ${chalk.dim(`[${source}]`)} ${message}\n`, + `${chalk.dim(`[${source}]`)} ${message}\n`, ); for (let i = 0; i < choices.length; i++) { process.stdout.write( @@ -900,69 +875,35 @@ export function createEnhancedClientIO( let response: PendingInteractionResponse; try { - if (interaction.type === "askYesNo") { - const defaultHint = - interaction.defaultValue === undefined - ? "" - : interaction.defaultValue - ? " (default: yes)" - : " (default: no)"; - const prompt = `${chalk.cyan("?")} ${interaction.message}${chalk.dim(defaultHint)} ${chalk.dim("(y/n)")} `; - - displayContent(line); - const input = await question(prompt, rl, ac.signal); - displayContent(line); - - let value: boolean; - if ( - input.toLowerCase() === "y" || - input.toLowerCase() === "yes" - ) { - value = true; - } else if ( - input.toLowerCase() === "n" || - input.toLowerCase() === "no" - ) { - value = false; - } else { - value = interaction.defaultValue ?? false; - } - response = { - interactionId: interaction.interactionId, - type: "askYesNo", - value, - }; - } else { - // popupQuestion - displayContent(line); - let popupText = `${chalk.cyan("?")} ${interaction.message}`; - interaction.choices.forEach((choice, i) => { - const isDefault = i === interaction.defaultId; - const prefix = chalk.cyan(`${i + 1}.`); - const suffix = isDefault - ? chalk.dim(" (default)") - : ""; - popupText += `\n ${prefix} ${choice}${suffix}`; - }); - displayContent(popupText); + // question — unified prompt for both yes/no and multi-choice + displayContent(line); + let promptText = `${interaction.message}`; + interaction.choices.forEach((choice, i) => { + const isDefault = i === interaction.defaultId; + const prefix = chalk.cyan(`${i + 1}.`); + const suffix = isDefault ? chalk.dim(" (default)") : ""; + promptText += `\n ${prefix} ${choice}${suffix}`; + }); + displayContent(promptText); - const prompt = chalk.dim( - `Enter number (1-${interaction.choices.length}): `, - ); - const input = await question(prompt, rl, ac.signal); - displayContent(line); - - const parsed = parseInt(input, 10) - 1; - const value = - parsed >= 0 && parsed < interaction.choices.length - ? parsed - : (interaction.defaultId ?? 0); - response = { - interactionId: interaction.interactionId, - type: "popupQuestion", - value, - }; - } + const prompt = chalk.dim( + isYesNoChoices(interaction.choices) + ? "Enter y/n or number (1-2): " + : `Enter number (1-${interaction.choices.length}): `, + ); + const input = await question(prompt, rl, ac.signal); + displayContent(line); + + const value = parseChoiceInput( + input, + interaction.choices, + interaction.defaultId, + ); + response = { + interactionId: interaction.interactionId, + type: "question", + value, + }; } catch { // Aborted by interactionResolved or interactionCancelled. const reason = ac.signal.reason; @@ -2105,10 +2046,7 @@ export async function replayDisplayHistory( const pending = pendingInteractions.get(entry.interactionId); if (pending?.message !== undefined) { let answerStr: string; - if (pending.interactionType === "askYesNo") { - answerStr = entry.response ? "yes" : "no"; - } else if ( - pending.interactionType === "popupQuestion" && + if ( pending.choices !== undefined && typeof entry.response === "number" ) { @@ -2116,10 +2054,10 @@ export async function replayDisplayHistory( pending.choices[entry.response] ?? String(entry.response); } else { - answerStr = String(entry.response); + answerStr = entry.response ? "yes" : "no"; } process.stdout.write( - chalk.dim(`? ${pending.message} → `) + + chalk.dim(`${pending.message} → `) + chalk.cyan(answerStr) + "\n", ); @@ -2131,7 +2069,7 @@ export async function replayDisplayHistory( const pending = pendingInteractions.get(entry.interactionId); if (pending?.message !== undefined) { process.stdout.write( - chalk.dim(`? ${pending.message} → `) + + chalk.dim(`${pending.message} → `) + chalk.yellow("[cancelled]") + "\n", ); diff --git a/ts/packages/cli/test/multiClientInteraction.spec.ts b/ts/packages/cli/test/multiClientInteraction.spec.ts index 15ed1f210..df86ecb23 100644 --- a/ts/packages/cli/test/multiClientInteraction.spec.ts +++ b/ts/packages/cli/test/multiClientInteraction.spec.ts @@ -13,6 +13,11 @@ * These tests exercise the activeInteractions map and AbortController logic * by replacing process.stdin with a controllable fake and verifying that * respondToInteraction is called (or not called) at the right times. + * + * Both yes/no prompts and multi-choice prompts use the unified `question` type. + * The caller wrapper (askYesNoWithContext) converts boolean → index on the way + * in and index → boolean on the way out; the protocol layer only ever sees + * numeric choice indices. */ import { describe, it, expect, beforeEach, afterEach } from "@jest/globals"; @@ -77,25 +82,21 @@ function makeDispatcherStub(): { dispatcher: Dispatcher; calls: CallRecord[] } { // ── Helpers ────────────────────────────────────────────────────────────────── -function makeAskYesNo(id: string): PendingInteractionRequest { +/** + * Build a `question` interaction. Pass `choices: ["Yes","No"]` to simulate + * what askYesNoWithContext sends; pass custom choices for multi-choice prompts. + */ +function makeQuestion( + id: string, + choices: string[] = ["Yes", "No"], + defaultId?: number, +): PendingInteractionRequest { return { interactionId: id, - type: "askYesNo", + type: "question", message: "Are you sure?", - defaultValue: false, - requestId: { requestId: "req-1" }, - source: "test", - timestamp: Date.now(), - }; -} - -function makePopupQuestion(id: string): PendingInteractionRequest { - return { - interactionId: id, - type: "popupQuestion", - message: "Pick one", - choices: ["alpha", "beta", "gamma"], - defaultId: 0, + choices, + ...(defaultId !== undefined && { defaultId }), requestId: { requestId: "req-1" }, source: "test", timestamp: Date.now(), @@ -152,65 +153,125 @@ afterEach(() => { // ── Tests ───────────────────────────────────────────────────────────────────── describe("requestInteraction — user answers", () => { - it("calls respondToInteraction with true when user types y", async () => { + it("calls respondToInteraction with index 0 when user picks choice 1 (Yes)", async () => { const { dispatcher, calls } = makeDispatcherStub(); const dispatcherRef = { current: dispatcher }; const clientIO = createEnhancedClientIO(undefined, dispatcherRef); - clientIO.requestInteraction(makeAskYesNo("int-1")); + clientIO.requestInteraction(makeQuestion("int-1")); await flushAsync(); // let the IIFE reach question() - fakeStdin.typeAnswer("y"); + fakeStdin.typeAnswer("1"); // 1-based → index 0 = "Yes" await flushAsync(); expect(calls).toHaveLength(1); expect(calls[0].args[0]).toMatchObject({ interactionId: "int-1", - type: "askYesNo", - value: true, + type: "question", + value: 0, }); }); - it("calls respondToInteraction with false when user types n", async () => { + it("calls respondToInteraction with index 0 when user types 'y'", async () => { const { dispatcher, calls } = makeDispatcherStub(); const clientIO = createEnhancedClientIO(undefined, { current: dispatcher, }); - clientIO.requestInteraction(makeAskYesNo("int-2")); + clientIO.requestInteraction(makeQuestion("int-1y")); await flushAsync(); - fakeStdin.typeAnswer("n"); + fakeStdin.typeAnswer("y"); + await flushAsync(); + + expect(calls[0].args[0]).toMatchObject({ value: 0 }); + }); + + it("calls respondToInteraction with index 0 when user types 'yes'", async () => { + const { dispatcher, calls } = makeDispatcherStub(); + const clientIO = createEnhancedClientIO(undefined, { + current: dispatcher, + }); + + clientIO.requestInteraction(makeQuestion("int-1yes")); + await flushAsync(); + + fakeStdin.typeAnswer("yes"); + await flushAsync(); + + expect(calls[0].args[0]).toMatchObject({ value: 0 }); + }); + + it("calls respondToInteraction with index 1 when user picks choice 2 (No)", async () => { + const { dispatcher, calls } = makeDispatcherStub(); + const clientIO = createEnhancedClientIO(undefined, { + current: dispatcher, + }); + + clientIO.requestInteraction(makeQuestion("int-2")); + await flushAsync(); + + fakeStdin.typeAnswer("2"); // 1-based → index 1 = "No" await flushAsync(); expect(calls).toHaveLength(1); - expect(calls[0].args[0]).toMatchObject({ value: false }); + expect(calls[0].args[0]).toMatchObject({ value: 1 }); }); - it("uses defaultValue when user types unrecognised input", async () => { + it("calls respondToInteraction with index 1 when user types 'n'", async () => { const { dispatcher, calls } = makeDispatcherStub(); const clientIO = createEnhancedClientIO(undefined, { current: dispatcher, }); - const interaction = makeAskYesNo("int-3"); - (interaction as any).defaultValue = true; - clientIO.requestInteraction(interaction); + clientIO.requestInteraction(makeQuestion("int-2n")); + await flushAsync(); + + fakeStdin.typeAnswer("n"); + await flushAsync(); + + expect(calls[0].args[0]).toMatchObject({ value: 1 }); + }); + + it("calls respondToInteraction with index 1 when user types 'no'", async () => { + const { dispatcher, calls } = makeDispatcherStub(); + const clientIO = createEnhancedClientIO(undefined, { + current: dispatcher, + }); + + clientIO.requestInteraction(makeQuestion("int-2no")); + await flushAsync(); + + fakeStdin.typeAnswer("no"); + await flushAsync(); + + expect(calls[0].args[0]).toMatchObject({ value: 1 }); + }); + + it("uses defaultId when user types unrecognized input", async () => { + const { dispatcher, calls } = makeDispatcherStub(); + const clientIO = createEnhancedClientIO(undefined, { + current: dispatcher, + }); + + clientIO.requestInteraction(makeQuestion("int-3", ["Yes", "No"], 0)); await flushAsync(); fakeStdin.typeAnswer("maybe"); await flushAsync(); - expect(calls[0].args[0]).toMatchObject({ value: true }); + expect(calls[0].args[0]).toMatchObject({ value: 0 }); }); - it("calls respondToInteraction with correct index for popupQuestion", async () => { + it("calls respondToInteraction with correct index for a multi-choice question", async () => { const { dispatcher, calls } = makeDispatcherStub(); const clientIO = createEnhancedClientIO(undefined, { current: dispatcher, }); - clientIO.requestInteraction(makePopupQuestion("int-4")); + clientIO.requestInteraction( + makeQuestion("int-4", ["alpha", "beta", "gamma"]), + ); await flushAsync(); fakeStdin.typeAnswer("2"); // 1-based → index 1 = "beta" @@ -218,7 +279,7 @@ describe("requestInteraction — user answers", () => { expect(calls[0].args[0]).toMatchObject({ interactionId: "int-4", - type: "popupQuestion", + type: "question", value: 1, }); }); @@ -231,11 +292,11 @@ describe("interactionResolved — dismisses pending prompt", () => { current: dispatcher, }); - clientIO.requestInteraction(makeAskYesNo("int-5")); + clientIO.requestInteraction(makeQuestion("int-5")); await flushAsync(); // IIFE reaches question(), now waiting on stdin // Another client answered — server tells this client to dismiss - clientIO.interactionResolved("int-5", true); + clientIO.interactionResolved("int-5", 0); await flushAsync(); // The user hasn't typed anything; no respondToInteraction should have fired @@ -248,10 +309,10 @@ describe("interactionResolved — dismisses pending prompt", () => { current: dispatcher, }); - clientIO.requestInteraction(makeAskYesNo("int-6")); + clientIO.requestInteraction(makeQuestion("int-6")); await flushAsync(); - clientIO.interactionResolved("int-6", true); + clientIO.interactionResolved("int-6", 0); await flushAsync(); const combined = stdoutOutput.join(""); @@ -266,7 +327,7 @@ describe("interactionCancelled — dismisses pending prompt", () => { current: dispatcher, }); - clientIO.requestInteraction(makeAskYesNo("int-7")); + clientIO.requestInteraction(makeQuestion("int-7")); await flushAsync(); clientIO.interactionCancelled("int-7"); @@ -281,7 +342,7 @@ describe("interactionCancelled — dismisses pending prompt", () => { current: dispatcher, }); - clientIO.requestInteraction(makeAskYesNo("int-8")); + clientIO.requestInteraction(makeQuestion("int-8")); await flushAsync(); clientIO.interactionCancelled("int-8"); @@ -298,7 +359,7 @@ describe("interactionResolved / interactionCancelled with unknown id", () => { current: makeDispatcherStub().dispatcher, }); expect(() => - clientIO.interactionResolved("no-such-id", true), + clientIO.interactionResolved("no-such-id", 0), ).not.toThrow(); }); @@ -318,16 +379,16 @@ describe("multiple concurrent interactions", () => { }); // Start two prompts simultaneously - clientIO.requestInteraction(makeAskYesNo("int-A")); - clientIO.requestInteraction(makeAskYesNo("int-B")); + clientIO.requestInteraction(makeQuestion("int-A")); + clientIO.requestInteraction(makeQuestion("int-B")); await flushAsync(); // Resolve int-A (answered by another client) - clientIO.interactionResolved("int-A", true); + clientIO.interactionResolved("int-A", 0); await flushAsync(); // int-B is still active — the user can still answer it - fakeStdin.typeAnswer("y"); + fakeStdin.typeAnswer("1"); await flushAsync(); // Only int-B should have called respondToInteraction @@ -341,15 +402,15 @@ describe("multiple concurrent interactions", () => { current: dispatcher, }); - clientIO.requestInteraction(makeAskYesNo("int-C")); + clientIO.requestInteraction(makeQuestion("int-C")); await flushAsync(); // User answers - fakeStdin.typeAnswer("y"); + fakeStdin.typeAnswer("1"); await flushAsync(); // Server sends interactionResolved after the fact (race condition in delivery) - expect(() => clientIO.interactionResolved("int-C", true)).not.toThrow(); + expect(() => clientIO.interactionResolved("int-C", 0)).not.toThrow(); expect(calls).toHaveLength(1); // still just the one from the user's answer }); }); diff --git a/ts/packages/commandExecutor/src/commandServer.ts b/ts/packages/commandExecutor/src/commandServer.ts index bd3d239ce..63f4d37c3 100644 --- a/ts/packages/commandExecutor/src/commandServer.ts +++ b/ts/packages/commandExecutor/src/commandServer.ts @@ -229,16 +229,30 @@ function createMcpClientIO( ); }, setDynamicDisplay(): void {}, - async askYesNo( - _requestId: RequestId, + async question( + _requestId: RequestId | undefined, message: string, - defaultValue?: boolean, - ): Promise { - if (getConfirmedFlag()) { - logger.log(`ClientIO: askYesNo - "${message}" (auto-approved)`); - return true; + choices: string[], + defaultId?: number, + ): Promise { + // For Yes/No, check the auto-confirm flag (backward compat with askYesNo). + if ( + choices.length === 2 && + choices[0] === "Yes" && + choices[1] === "No" + ) { + if (getConfirmedFlag()) { + logger.log( + `ClientIO: question - "${message}" (auto-approved)`, + ); + return 0; // "Yes" + } + throw new Error(`USER_CONFIRMATION_REQUIRED: ${message}`); } - throw new Error(`USER_CONFIRMATION_REQUIRED: ${message}`); + logger.log( + `ClientIO: question - "${message}" choices=[${choices.join(", ")}] (defaulting to ${defaultId ?? 0})`, + ); + return defaultId ?? 0; }, async proposeAction( _requestId: RequestId, @@ -250,17 +264,6 @@ function createMcpClientIO( ); return undefined; }, - async popupQuestion( - message: string, - choices: string[], - defaultId: number | undefined, - source: string, - ): Promise { - logger.log( - `ClientIO: popupQuestion(source=${source}) - "${message}" choices=[${choices.join(", ")}] (defaulting to ${defaultId ?? 0})`, - ); - return defaultId ?? 0; - }, notify( _requestId: RequestId, event: string, diff --git a/ts/packages/dispatcher/dispatcher/src/context/interactiveIO.ts b/ts/packages/dispatcher/dispatcher/src/context/interactiveIO.ts index 0cf684146..0b9817963 100644 --- a/ts/packages/dispatcher/dispatcher/src/context/interactiveIO.ts +++ b/ts/packages/dispatcher/dispatcher/src/context/interactiveIO.ts @@ -78,13 +78,17 @@ export async function askYesNoWithContext( message: string, defaultValue: boolean = false, ) { - return context?.batchMode - ? defaultValue - : context.clientIO.askYesNo( - getRequestId(context), - message, - defaultValue, - ); + if (context?.batchMode) { + return defaultValue; + } + const defaultId = defaultValue ? 0 : 1; + const index = await context.clientIO.question( + getRequestId(context), + message, + ["Yes", "No"], + defaultId, + ); + return index === 0; } export const nullClientIO: ClientIO = { @@ -96,15 +100,13 @@ export const nullClientIO: ClientIO = { appendDisplay: () => {}, appendDiagnosticData: () => {}, setDynamicDisplay: () => {}, - askYesNo: async ( - requestId: RequestId, - message: string, - defaultValue: boolean = false, - ) => defaultValue, + question: async ( + _requestId: RequestId | undefined, + _message: string, + _choices: string[], + defaultId?: number, + ) => defaultId ?? 0, proposeAction: async () => undefined, - popupQuestion: async () => { - throw new Error("popupQuestion not implemented"); - }, notify: () => {}, openLocalView: async () => {}, closeLocalView: async () => {}, diff --git a/ts/packages/dispatcher/dispatcher/src/context/pendingInteractionManager.ts b/ts/packages/dispatcher/dispatcher/src/context/pendingInteractionManager.ts index 715fbf376..75f3fba03 100644 --- a/ts/packages/dispatcher/dispatcher/src/context/pendingInteractionManager.ts +++ b/ts/packages/dispatcher/dispatcher/src/context/pendingInteractionManager.ts @@ -2,8 +2,8 @@ // Licensed under the MIT License. import type { - PendingInteractionType, PendingInteractionRequest, + PendingInteractionType, RequestId, } from "@typeagent/dispatcher-types"; @@ -13,7 +13,7 @@ type PendingEntry = { resolve: (value: any) => void; reject: (error: Error) => void; timeoutTimer?: ReturnType; - defaultValue?: boolean; // askYesNo default + defaultId?: number; // question default choice index // Full request data for replay request: PendingInteractionRequest; }; @@ -46,15 +46,11 @@ export class PendingInteractionManager { entry.requestId = request.requestId; } - // Store default value for askYesNo timeout fallback - if (request.type === "askYesNo") { - const askYesNoDefault = ( - request as PendingInteractionRequest & { - type: "askYesNo"; - } - ).defaultValue; - if (askYesNoDefault !== undefined) { - entry.defaultValue = askYesNoDefault; + // Store defaultId for question timeout fallback + if (request.type === "question") { + const q = request as { type: "question"; defaultId?: number }; + if (q.defaultId !== undefined) { + entry.defaultId = q.defaultId; } } @@ -88,7 +84,7 @@ export class PendingInteractionManager { /** * Reject/cancel a pending interaction (e.g., client disconnected, timeout). - * For askYesNo with an explicit defaultValue, resolves with that value; + * For a `question` with an explicit defaultId, resolves with that index; * otherwise rejects. Returns true if the interaction was found and cancelled. */ cancel(interactionId: string, error: Error): boolean { @@ -99,18 +95,18 @@ export class PendingInteractionManager { clearTimeout(entry.timeoutTimer); } - // For askYesNo, resolve with default if one was explicitly provided; + // For question, resolve with defaultId if one was explicitly provided; // otherwise reject — no declared safe fallback exists. - if (entry.type === "askYesNo") { - if (entry.defaultValue !== undefined) { - entry.resolve(entry.defaultValue); + if (entry.type === "question") { + if (entry.defaultId !== undefined) { + entry.resolve(entry.defaultId); } else { entry.reject(error); } return true; } - // For proposeAction and popupQuestion, reject + // For proposeAction, reject entry.reject(error); return true; } diff --git a/ts/packages/dispatcher/dispatcher/src/displayLog.ts b/ts/packages/dispatcher/dispatcher/src/displayLog.ts index d71a018bc..61f323920 100644 --- a/ts/packages/dispatcher/dispatcher/src/displayLog.ts +++ b/ts/packages/dispatcher/dispatcher/src/displayLog.ts @@ -214,12 +214,7 @@ export class DisplayLog { if (interaction.requestId !== undefined) { entry.requestId = interaction.requestId; } - if (interaction.type === "askYesNo") { - entry.message = interaction.message; - if (interaction.defaultValue !== undefined) { - entry.defaultValue = interaction.defaultValue; - } - } else if (interaction.type === "popupQuestion") { + if (interaction.type === "question") { entry.message = interaction.message; entry.choices = interaction.choices; if (interaction.defaultId !== undefined) { diff --git a/ts/packages/dispatcher/dispatcher/src/execute/sessionContext.ts b/ts/packages/dispatcher/dispatcher/src/execute/sessionContext.ts index a2749280f..2764078dc 100644 --- a/ts/packages/dispatcher/dispatcher/src/execute/sessionContext.ts +++ b/ts/packages/dispatcher/dispatcher/src/execute/sessionContext.ts @@ -159,7 +159,8 @@ export function createSessionContext( choices: string[] = ["Yes", "No"], // default choices defaultId?: number, ): Promise { - return context.clientIO.popupQuestion( + return context.clientIO.question( + undefined, message, choices, defaultId, diff --git a/ts/packages/dispatcher/dispatcher/src/helpers/console.ts b/ts/packages/dispatcher/dispatcher/src/helpers/console.ts index 926c3e3df..975ef74b7 100644 --- a/ts/packages/dispatcher/dispatcher/src/helpers/console.ts +++ b/ts/packages/dispatcher/dispatcher/src/helpers/console.ts @@ -196,13 +196,31 @@ function createConsoleClientIO( }, // Input - async askYesNo( - requestId: RequestId, + async question( + _requestId: RequestId | undefined, message: string, - defaultValue?: boolean, - ): Promise { - const input = await question(`${message} (y/n)`, rl); - return input.toLowerCase() === "y"; + choices: string[], + defaultId?: number, + ): Promise { + const isYesNo = + choices.length === 2 && + choices[0] === "Yes" && + choices[1] === "No"; + const numbered = choices + .map((c, i) => ` ${i + 1}. ${c}`) + .join("\n"); + const hint = isYesNo ? "(y/n or 1-2)" : `(1-${choices.length})`; + const input = await question( + `${message}\n${numbered}\n${hint}> `, + rl, + ); + const trimmed = input.trim().toLowerCase(); + if (isYesNo) { + if (trimmed === "y" || trimmed === "yes") return 0; + if (trimmed === "n" || trimmed === "no") return 1; + } + const idx = parseInt(trimmed, 10) - 1; + return idx >= 0 && idx < choices.length ? idx : (defaultId ?? 0); }, async proposeAction( requestId: RequestId, @@ -213,16 +231,6 @@ function createConsoleClientIO( return undefined; }, - async popupQuestion( - message: string, - choices: string[], - defaultId: number | undefined, - source: string, - ): Promise { - // REVIEW: - throw new Error("Not implemented"); - }, - // Notification (TODO: turn these in to dispatcher events) notify( requestId: RequestId, diff --git a/ts/packages/dispatcher/dispatcher/test/displayLog.spec.ts b/ts/packages/dispatcher/dispatcher/test/displayLog.spec.ts index 62cd010f4..4adcd6580 100644 --- a/ts/packages/dispatcher/dispatcher/test/displayLog.spec.ts +++ b/ts/packages/dispatcher/dispatcher/test/displayLog.spec.ts @@ -293,15 +293,16 @@ describe("DisplayLog", () => { }); describe("logPendingInteraction", () => { - it("should log askYesNo with correct fields and omit defaultValue when not provided", () => { + it("should log question with correct fields and omit defaultId when not provided", () => { const log = new DisplayLog(undefined); - const interaction: PendingInteractionRequest = { + const interaction = { interactionId: "int-1", - type: "askYesNo", + type: "question", source: "agent-a", timestamp: Date.now(), message: "Do you want to proceed?", - }; + choices: ["Yes", "No"], + } as unknown as PendingInteractionRequest; log.logPendingInteraction(interaction); @@ -310,50 +311,54 @@ describe("DisplayLog", () => { expect(entries[0].type).toBe("pending-interaction"); if (entries[0].type === "pending-interaction") { expect(entries[0].interactionId).toBe("int-1"); - expect(entries[0].interactionType).toBe("askYesNo"); + expect(entries[0].interactionType).toBe("question"); expect(entries[0].source).toBe("agent-a"); expect(entries[0].message).toBe("Do you want to proceed?"); - expect(entries[0].defaultValue).toBeUndefined(); + expect(entries[0].choices).toEqual(["Yes", "No"]); + expect(entries[0].defaultId).toBeUndefined(); } }); - it("should include defaultValue for askYesNo when provided", () => { + it("should include defaultId for question when provided", () => { const log = new DisplayLog(undefined); - const interaction: PendingInteractionRequest = { + const interaction = { interactionId: "int-2", - type: "askYesNo", + type: "question", source: "agent-b", timestamp: Date.now(), message: "Continue?", - defaultValue: true, - }; + choices: ["Yes", "No"], + defaultId: 0, + } as unknown as PendingInteractionRequest; log.logPendingInteraction(interaction); const entries = log.getEntries(); if (entries[0].type === "pending-interaction") { - expect(entries[0].defaultValue).toBe(true); + expect(entries[0].defaultId).toBe(0); } }); it("should include requestId when provided and omit when not", () => { const log = new DisplayLog(undefined); const reqId = makeRequestId("req-abc"); - const withReqId: PendingInteractionRequest = { + const withReqId = { interactionId: "int-3", - type: "askYesNo", + type: "question", source: "agent-c", timestamp: Date.now(), message: "Yes?", + choices: ["Yes", "No"], requestId: reqId, - }; - const withoutReqId: PendingInteractionRequest = { + } as unknown as PendingInteractionRequest; + const withoutReqId = { interactionId: "int-4", - type: "askYesNo", + type: "question", source: "agent-d", timestamp: Date.now(), message: "No?", - }; + choices: ["Yes", "No"], + } as unknown as PendingInteractionRequest; log.logPendingInteraction(withReqId); log.logPendingInteraction(withoutReqId); @@ -367,40 +372,40 @@ describe("DisplayLog", () => { } }); - it("should log popupQuestion with message and choices, omit defaultId when not provided", () => { + it("should log question with message and choices, omit defaultId when not provided", () => { const log = new DisplayLog(undefined); - const interaction: PendingInteractionRequest = { + const interaction = { interactionId: "int-5", - type: "popupQuestion", + type: "question", source: "agent-e", timestamp: Date.now(), message: "Pick one", choices: ["alpha", "beta", "gamma"], - }; + } as unknown as PendingInteractionRequest; log.logPendingInteraction(interaction); const entries = log.getEntries(); expect(entries).toHaveLength(1); if (entries[0].type === "pending-interaction") { - expect(entries[0].interactionType).toBe("popupQuestion"); + expect(entries[0].interactionType).toBe("question"); expect(entries[0].message).toBe("Pick one"); expect(entries[0].choices).toEqual(["alpha", "beta", "gamma"]); expect(entries[0].defaultId).toBeUndefined(); } }); - it("should include defaultId for popupQuestion when provided", () => { + it("should include defaultId for multi-choice question when provided", () => { const log = new DisplayLog(undefined); - const interaction: PendingInteractionRequest = { + const interaction = { interactionId: "int-6", - type: "popupQuestion", + type: "question", source: "agent-f", timestamp: Date.now(), message: "Choose", choices: ["x", "y"], defaultId: 1, - }; + } as unknown as PendingInteractionRequest; log.logPendingInteraction(interaction); @@ -443,20 +448,21 @@ describe("DisplayLog", () => { log.logSetDisplay(msg); // seq 0 log.logPendingInteraction({ interactionId: "int-a", - type: "askYesNo", + type: "question", source: "src", timestamp: Date.now(), message: "q?", - }); // seq 1 + choices: ["Yes", "No"], + } as unknown as PendingInteractionRequest); // seq 1 log.logAppendDisplay(msg, "block"); // seq 2 log.logPendingInteraction({ interactionId: "int-b", - type: "popupQuestion", + type: "question", source: "src", timestamp: Date.now(), message: "pick", choices: ["a"], - }); // seq 3 + } as unknown as PendingInteractionRequest); // seq 3 log.logNotify(undefined, "evt", {}, "src"); // seq 4 const entries = log.getEntries(); @@ -469,11 +475,12 @@ describe("DisplayLog", () => { const seq0 = log.logSetDisplay(makeMessage("first")); const seq1 = log.logPendingInteraction({ interactionId: "int-ret", - type: "askYesNo", + type: "question", source: "src", timestamp: Date.now(), message: "q?", - }); + choices: ["Yes", "No"], + } as unknown as PendingInteractionRequest); const seq2 = log.logPendingInteraction({ interactionId: "int-ret-2", type: "proposeAction", @@ -696,22 +703,23 @@ describe("DisplayLog", () => { log.logPendingInteraction({ interactionId: "int-p1", - type: "askYesNo", + type: "question", source: "agent-a", timestamp: Date.now(), message: "Proceed?", - defaultValue: false, + choices: ["Yes", "No"], + defaultId: 1, requestId: reqId, - }); + } as unknown as PendingInteractionRequest); log.logPendingInteraction({ interactionId: "int-p2", - type: "popupQuestion", + type: "question", source: "agent-b", timestamp: Date.now(), message: "Choose", choices: ["opt1", "opt2", "opt3"], defaultId: 2, - }); + } as unknown as PendingInteractionRequest); log.logPendingInteraction({ interactionId: "int-p3", type: "proposeAction", @@ -719,7 +727,7 @@ describe("DisplayLog", () => { timestamp: Date.now(), actionTemplates: { tpl: "data" } as any, }); - log.logInteractionResolved("int-p1", true); + log.logInteractionResolved("int-p1", 1); log.logInteractionResolved("int-p2", 1); log.logInteractionResolved("int-p3", { accepted: true }); @@ -737,20 +745,21 @@ describe("DisplayLog", () => { "interaction-resolved", ]); - // Verify askYesNo fields survived round-trip + // Verify first question fields survived round-trip if (entries[0].type === "pending-interaction") { expect(entries[0].interactionId).toBe("int-p1"); - expect(entries[0].interactionType).toBe("askYesNo"); + expect(entries[0].interactionType).toBe("question"); expect(entries[0].message).toBe("Proceed?"); - expect(entries[0].defaultValue).toBe(false); + expect(entries[0].choices).toEqual(["Yes", "No"]); + expect(entries[0].defaultId).toBe(1); expect(entries[0].source).toBe("agent-a"); expect(entries[0].requestId).toEqual(reqId); } - // Verify popupQuestion fields survived round-trip + // Verify second question fields survived round-trip if (entries[1].type === "pending-interaction") { expect(entries[1].interactionId).toBe("int-p2"); - expect(entries[1].interactionType).toBe("popupQuestion"); + expect(entries[1].interactionType).toBe("question"); expect(entries[1].message).toBe("Choose"); expect(entries[1].choices).toEqual(["opt1", "opt2", "opt3"]); expect(entries[1].defaultId).toBe(2); @@ -766,7 +775,7 @@ describe("DisplayLog", () => { // Verify interaction-resolved entries survived round-trip if (entries[3].type === "interaction-resolved") { expect(entries[3].interactionId).toBe("int-p1"); - expect(entries[3].response).toBe(true); + expect(entries[3].response).toBe(1); } if (entries[4].type === "interaction-resolved") { expect(entries[4].interactionId).toBe("int-p2"); @@ -780,11 +789,12 @@ describe("DisplayLog", () => { // Verify seq numbering resumes correctly after load loaded.logPendingInteraction({ interactionId: "int-p4", - type: "askYesNo", + type: "question", source: "agent-d", timestamp: Date.now(), message: "Another?", - }); + choices: ["Yes", "No"], + } as unknown as PendingInteractionRequest); const allEntries = loaded.getEntries(); expect(allEntries).toHaveLength(7); expect(allEntries[6].seq).toBe(6); diff --git a/ts/packages/dispatcher/dispatcher/test/pendingInteractionManager.spec.ts b/ts/packages/dispatcher/dispatcher/test/pendingInteractionManager.spec.ts index d3e0dd745..cdc4e986f 100644 --- a/ts/packages/dispatcher/dispatcher/test/pendingInteractionManager.spec.ts +++ b/ts/packages/dispatcher/dispatcher/test/pendingInteractionManager.spec.ts @@ -4,13 +4,14 @@ import type { PendingInteractionRequest } from "@typeagent/dispatcher-types"; import { PendingInteractionManager } from "../src/context/pendingInteractionManager.js"; -function makeAskYesNoRequest( - overrides: Partial = {}, -): PendingInteractionRequest & { type: "askYesNo" } { +function makeQuestionRequest( + overrides: Partial = {}, +): PendingInteractionRequest & { type: "question" } { return { interactionId: overrides.interactionId ?? `ask-${Date.now()}`, - type: "askYesNo", + type: "question", message: "Continue?", + choices: ["Yes", "No"], source: "test", timestamp: Date.now(), ...overrides, @@ -37,22 +38,6 @@ function makeProposeActionRequest( }; } -function makePopupQuestionRequest( - overrides: Partial< - PendingInteractionRequest & { type: "popupQuestion" } - > = {}, -): PendingInteractionRequest & { type: "popupQuestion" } { - return { - interactionId: overrides.interactionId ?? `popup-${Date.now()}`, - type: "popupQuestion", - message: "Pick one", - choices: ["A", "B", "C"], - source: "test", - timestamp: Date.now(), - ...overrides, - }; -} - function delay(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } @@ -69,18 +54,18 @@ describe("PendingInteractionManager", () => { // --------------------------------------------------------------- describe("create + resolve", () => { it("resolves the promise with the given value and removes the entry", async () => { - const request = makeAskYesNoRequest({ + const request = makeQuestionRequest({ interactionId: "id-1", }); - const promise = manager.create(request); + const promise = manager.create(request); expect(manager.has("id-1")).toBe(true); expect(manager.size).toBe(1); - const resolved = manager.resolve("id-1", true); + const resolved = manager.resolve("id-1", 0); expect(resolved).toBe(true); - await expect(promise).resolves.toBe(true); + await expect(promise).resolves.toBe(0); expect(manager.has("id-1")).toBe(false); expect(manager.size).toBe(0); }); @@ -124,31 +109,30 @@ describe("PendingInteractionManager", () => { }); // --------------------------------------------------------------- - // 5. cancel: all three types behave correctly - // - askYesNo resolves with defaultValue if explicitly set, rejects otherwise + // 5. cancel: both types behave correctly + // - question resolves with defaultId if explicitly set, rejects otherwise // - proposeAction rejects - // - popupQuestion rejects // --------------------------------------------------------------- - it("askYesNo cancel resolves with defaultValue instead of rejecting", async () => { - const request = makeAskYesNoRequest({ - interactionId: "yn-default-true", - defaultValue: true, + it("question cancel resolves with defaultId instead of rejecting", async () => { + const request = makeQuestionRequest({ + interactionId: "q-default-1", + defaultId: 1, }); - const promise = manager.create(request); + const promise = manager.create(request); - manager.cancel("yn-default-true", new Error("disconnected")); + manager.cancel("q-default-1", new Error("disconnected")); - // askYesNo resolves with the stored defaultValue - await expect(promise).resolves.toBe(true); + // question resolves with the stored defaultId + await expect(promise).resolves.toBe(1); }); - it("askYesNo cancel rejects with the error when no defaultValue is set", async () => { - const request = makeAskYesNoRequest({ - interactionId: "yn-no-default", + it("question cancel rejects with the error when no defaultId is set", async () => { + const request = makeQuestionRequest({ + interactionId: "q-no-default", }); - const promise = manager.create(request); + const promise = manager.create(request); - manager.cancel("yn-no-default", new Error("disconnected")); + manager.cancel("q-no-default", new Error("disconnected")); await expect(promise).rejects.toThrow("disconnected"); }); @@ -163,17 +147,6 @@ describe("PendingInteractionManager", () => { await expect(promise).rejects.toThrow("aborted"); }); - - it("popupQuestion cancel rejects with the error", async () => { - const request = makePopupQuestionRequest({ - interactionId: "pq-cancel", - }); - const promise = manager.create(request); - - manager.cancel("pq-cancel", new Error("timeout")); - - await expect(promise).rejects.toThrow("timeout"); - }); }); // --------------------------------------------------------------- @@ -192,14 +165,14 @@ describe("PendingInteractionManager", () => { expect(manager.has("timeout-1")).toBe(false); }); - it("resolves askYesNo with default on timeout instead of rejecting", async () => { - const request = makeAskYesNoRequest({ - interactionId: "timeout-yn", - defaultValue: true, + it("resolves question with defaultId on timeout instead of rejecting", async () => { + const request = makeQuestionRequest({ + interactionId: "timeout-q", + defaultId: 0, }); - const promise = manager.create(request, 10); + const promise = manager.create(request, 10); - await expect(promise).resolves.toBe(true); + await expect(promise).resolves.toBe(0); }); // --------------------------------------------------------------- @@ -245,17 +218,18 @@ describe("PendingInteractionManager", () => { // --------------------------------------------------------------- describe("getPending", () => { it("returns all current pending requests", () => { - const req1 = makeAskYesNoRequest({ + const req1 = makeQuestionRequest({ interactionId: "pending-1", }); const req2 = makeProposeActionRequest({ interactionId: "pending-2", }); - const req3 = makePopupQuestionRequest({ + const req3 = makeQuestionRequest({ interactionId: "pending-3", + choices: ["A", "B", "C"], }); - manager.create(req1); + manager.create(req1); manager.create(req2); manager.create(req3); @@ -268,7 +242,7 @@ describe("PendingInteractionManager", () => { // Verify the full request objects are returned const askReq = pending.find((r) => r.interactionId === "pending-1"); - expect(askReq?.type).toBe("askYesNo"); + expect(askReq?.type).toBe("question"); expect(askReq?.source).toBe("test"); }); @@ -276,17 +250,17 @@ describe("PendingInteractionManager", () => { // 9. getPending: returns empty after all resolved // --------------------------------------------------------------- it("returns empty array after all interactions are resolved", () => { - const req1 = makeAskYesNoRequest({ + const req1 = makeQuestionRequest({ interactionId: "r-1", }); const req2 = makeProposeActionRequest({ interactionId: "r-2", }); - manager.create(req1); + manager.create(req1); manager.create(req2); - manager.resolve("r-1", true); + manager.resolve("r-1", 0); manager.resolve("r-2", {}); expect(manager.getPending()).toEqual([]); @@ -298,15 +272,15 @@ describe("PendingInteractionManager", () => { // --------------------------------------------------------------- describe("has and size", () => { it("has returns true for pending and false after resolution", () => { - const request = makeAskYesNoRequest({ + const request = makeQuestionRequest({ interactionId: "check-has", }); - manager.create(request); + manager.create(request); expect(manager.has("check-has")).toBe(true); expect(manager.has("other")).toBe(false); - manager.resolve("check-has", false); + manager.resolve("check-has", 1); expect(manager.has("check-has")).toBe(false); }); @@ -314,26 +288,27 @@ describe("PendingInteractionManager", () => { it("size tracks additions and removals accurately", async () => { expect(manager.size).toBe(0); - const req1 = makeAskYesNoRequest({ + const req1 = makeQuestionRequest({ interactionId: "s-1", }); const req2 = makeProposeActionRequest({ interactionId: "s-2", }); - const req3 = makePopupQuestionRequest({ + const req3 = makeQuestionRequest({ interactionId: "s-3", + choices: ["A", "B", "C"], }); - const p1 = manager.create(req1); + const p1 = manager.create(req1); expect(manager.size).toBe(1); const p2 = manager.create(req2); const p3 = manager.create(req3); expect(manager.size).toBe(3); - manager.resolve("s-1", true); + manager.resolve("s-1", 0); expect(manager.size).toBe(2); - await expect(p1).resolves.toBe(true); + await expect(p1).resolves.toBe(0); manager.cancel("s-2", new Error("cancelled")); expect(manager.size).toBe(1); @@ -353,17 +328,18 @@ describe("PendingInteractionManager", () => { const req1 = makeProposeActionRequest({ interactionId: "all-1", }); - const req2 = makePopupQuestionRequest({ + const req2 = makeQuestionRequest({ interactionId: "all-2", + choices: ["A", "B", "C"], }); - const req3 = makeAskYesNoRequest({ + const req3 = makeQuestionRequest({ interactionId: "all-3", - defaultValue: true, + defaultId: 0, }); const p1 = manager.create(req1); const p2 = manager.create(req2); - const p3 = manager.create(req3); + const p3 = manager.create(req3); expect(manager.size).toBe(3); @@ -371,12 +347,12 @@ describe("PendingInteractionManager", () => { expect(manager.size).toBe(0); - // proposeAction and popupQuestion reject + // proposeAction and question without defaultId reject await expect(p1).rejects.toThrow("shutting down"); await expect(p2).rejects.toThrow("shutting down"); - // askYesNo resolves with defaultValue - await expect(p3).resolves.toBe(true); + // question with defaultId resolves with it + await expect(p3).resolves.toBe(0); }); it("is a no-op when there are no pending interactions", () => { @@ -391,12 +367,12 @@ describe("PendingInteractionManager", () => { // --------------------------------------------------------------- describe("edge cases", () => { it("stores requestId from the request", () => { - const request = makeAskYesNoRequest({ + const request = makeQuestionRequest({ interactionId: "with-rid", requestId: "req-42" as any, }); - manager.create(request); + manager.create(request); const pending = manager.getPending(); expect(pending[0].requestId).toBe("req-42"); diff --git a/ts/packages/dispatcher/dispatcher/test/sessionContext.spec.ts b/ts/packages/dispatcher/dispatcher/test/sessionContext.spec.ts index db3e848dd..54177b4bc 100644 --- a/ts/packages/dispatcher/dispatcher/test/sessionContext.spec.ts +++ b/ts/packages/dispatcher/dispatcher/test/sessionContext.spec.ts @@ -33,7 +33,7 @@ function makeContext(overrides: { }, clientIO: { notify: () => {}, - popupQuestion: async () => 0, + question: async () => 0, }, translatorCache: { clear: () => {} }, lastActionSchemaName: undefined, diff --git a/ts/packages/dispatcher/nodeProviders/test/provider.spec.ts b/ts/packages/dispatcher/nodeProviders/test/provider.spec.ts index bb87eb5fa..19d046e74 100644 --- a/ts/packages/dispatcher/nodeProviders/test/provider.spec.ts +++ b/ts/packages/dispatcher/nodeProviders/test/provider.spec.ts @@ -31,15 +31,13 @@ function createTestClientIO(data: IAgentMessage[]): ClientIO { setDisplayInfo: () => {}, appendDiagnosticData: () => {}, setDynamicDisplay: () => {}, - askYesNo: async ( - requestId: RequestId, - message: string, - defaultValue: boolean = false, - ) => defaultValue, + question: async ( + _requestId: RequestId | undefined, + _message: string, + _choices: string[], + defaultId?: number, + ) => defaultId ?? 0, proposeAction: async () => undefined, - popupQuestion: async () => { - throw new Error("popupQuestion not implemented"); - }, notify: () => {}, openLocalView: async () => {}, closeLocalView: async () => {}, diff --git a/ts/packages/dispatcher/rpc/src/clientIOClient.ts b/ts/packages/dispatcher/rpc/src/clientIOClient.ts index dc06c8f02..a5ea0091b 100644 --- a/ts/packages/dispatcher/rpc/src/clientIOClient.ts +++ b/ts/packages/dispatcher/rpc/src/clientIOClient.ts @@ -41,15 +41,12 @@ export function createClientIORpcClient(channel: RpcChannel): ClientIO { }, // Input - askYesNo(...args): Promise { - return rpc.invoke("askYesNo", ...args); + question(...args): Promise { + return rpc.invoke("question", ...args); }, proposeAction(...args): Promise { return rpc.invoke("proposeAction", ...args); }, - popupQuestion(...args): Promise { - return rpc.invoke("popupQuestion", ...args); - }, notify(...args): void { return rpc.send("notify", ...args); }, diff --git a/ts/packages/dispatcher/rpc/src/clientIOServer.ts b/ts/packages/dispatcher/rpc/src/clientIOServer.ts index e105b83ef..65a7bc4ae 100644 --- a/ts/packages/dispatcher/rpc/src/clientIOServer.ts +++ b/ts/packages/dispatcher/rpc/src/clientIOServer.ts @@ -14,15 +14,12 @@ export function createClientIORpcServer( channel: RpcChannel, ) { const clientIOInvokeFunctions: ClientIOInvokeFunctions = { - askYesNo: async (...args) => { - return clientIO.askYesNo(...args); + question: async (...args) => { + return clientIO.question(...args); }, proposeAction: async (...args) => { return clientIO.proposeAction(...args); }, - popupQuestion: async (...args) => { - return clientIO.popupQuestion(...args); - }, openLocalView: async (...args) => { return clientIO.openLocalView(...args); }, diff --git a/ts/packages/dispatcher/rpc/src/clientIOTypes.ts b/ts/packages/dispatcher/rpc/src/clientIOTypes.ts index 465f243b9..07cc197f9 100644 --- a/ts/packages/dispatcher/rpc/src/clientIOTypes.ts +++ b/ts/packages/dispatcher/rpc/src/clientIOTypes.ts @@ -10,22 +10,18 @@ import type { } from "@typeagent/dispatcher-types"; export type ClientIOInvokeFunctions = { - askYesNo( - requestId: RequestId, + question( + requestId: RequestId | undefined, message: string, - defaultValue?: boolean, - ): Promise; + choices: string[], + defaultId?: number, + source?: string, + ): Promise; proposeAction( requestId: RequestId, actionTemplates: TemplateEditConfig, source: string, ): Promise; - popupQuestion( - message: string, - choices: string[], - defaultId: number | undefined, - source: string, - ): Promise; openLocalView(requestId: RequestId, port: number): Promise; closeLocalView(requestId: RequestId, port: number): Promise; }; diff --git a/ts/packages/dispatcher/rpc/test/dispatcherRpc.spec.ts b/ts/packages/dispatcher/rpc/test/dispatcherRpc.spec.ts index b682d2bc9..fb7d94b16 100644 --- a/ts/packages/dispatcher/rpc/test/dispatcherRpc.spec.ts +++ b/ts/packages/dispatcher/rpc/test/dispatcherRpc.spec.ts @@ -82,7 +82,11 @@ function makeStubDispatcher(overrides: Partial = {}): Dispatcher & { // Helpers // --------------------------------------------------------------------------- function makeResponse(interactionId = "id-1"): PendingInteractionResponse { - return { interactionId, type: "askYesNo", value: true }; + return { + interactionId, + type: "question", + value: 0, + } as unknown as PendingInteractionResponse; } // --------------------------------------------------------------------------- diff --git a/ts/packages/dispatcher/types/src/clientIO.ts b/ts/packages/dispatcher/types/src/clientIO.ts index a778796d4..cd85c88b2 100644 --- a/ts/packages/dispatcher/types/src/clientIO.ts +++ b/ts/packages/dispatcher/types/src/clientIO.ts @@ -71,25 +71,19 @@ export interface ClientIO { ): void; // Input - askYesNo( - requestId: RequestId, + question( + requestId: RequestId | undefined, message: string, - defaultValue?: boolean, - ): Promise; + choices: string[], + defaultId?: number, + source?: string, + ): Promise; proposeAction( requestId: RequestId, actionTemplates: TemplateEditConfig, source: string, ): Promise; - // A question outside of the request - popupQuestion( - message: string, - choices: string[], - defaultId: number | undefined, - source: string, - ): Promise; - // Notification (TODO: turn these in to dispatcher events) notify( notificationId: string | RequestId | undefined, diff --git a/ts/packages/dispatcher/types/src/displayLogEntry.ts b/ts/packages/dispatcher/types/src/displayLogEntry.ts index e8e9d6066..8fefb62d5 100644 --- a/ts/packages/dispatcher/types/src/displayLogEntry.ts +++ b/ts/packages/dispatcher/types/src/displayLogEntry.ts @@ -61,10 +61,11 @@ export type PendingInteractionEntry = { interactionType: PendingInteractionType; requestId?: RequestId; source: string; + // question fields message?: string; - defaultValue?: boolean; choices?: string[]; defaultId?: number; + // proposeAction fields actionTemplates?: TemplateEditConfig; }; diff --git a/ts/packages/dispatcher/types/src/pendingInteraction.ts b/ts/packages/dispatcher/types/src/pendingInteraction.ts index 2bbbc0096..323ebd586 100644 --- a/ts/packages/dispatcher/types/src/pendingInteraction.ts +++ b/ts/packages/dispatcher/types/src/pendingInteraction.ts @@ -7,14 +7,16 @@ import type { TemplateEditConfig } from "./clientIO.js"; /** * The type of a pending interaction awaiting a client response. */ -export type PendingInteractionType = - | "askYesNo" - | "proposeAction" - | "popupQuestion"; +export type PendingInteractionType = "question" | "proposeAction"; /** * A request sent to the client for a pending interaction. * Contains all data needed for the client to render the appropriate UI. + * + * The `question` type unifies the former `askYesNo` and `popupQuestion` types: + * choices are always explicit strings; callers that want a yes/no prompt pass + * `choices: ["Yes", "No"]` and use `askYesNo()` / `popupQuestion()` wrappers + * on SessionContext to convert between boolean and index. */ export type PendingInteractionRequest = { interactionId: string; @@ -23,23 +25,21 @@ export type PendingInteractionRequest = { source: string; timestamp: number; } & ( - | { type: "askYesNo"; message: string; defaultValue?: boolean } | { - type: "proposeAction"; - actionTemplates: TemplateEditConfig; - } - | { - type: "popupQuestion"; + type: "question"; message: string; choices: string[]; defaultId?: number; } + | { + type: "proposeAction"; + actionTemplates: TemplateEditConfig; + } ); /** * A response from the client resolving a pending interaction. */ export type PendingInteractionResponse = - | { interactionId: string; type: "askYesNo"; value: boolean } - | { interactionId: string; type: "proposeAction"; value: unknown } - | { interactionId: string; type: "popupQuestion"; value: number }; + | { interactionId: string; type: "question"; value: number } + | { interactionId: string; type: "proposeAction"; value: unknown }; diff --git a/ts/packages/shell/src/main/instance.ts b/ts/packages/shell/src/main/instance.ts index 1cfcb4775..4f079c140 100644 --- a/ts/packages/shell/src/main/instance.ts +++ b/ts/packages/shell/src/main/instance.ts @@ -76,12 +76,24 @@ async function initializeDispatcher( const clientIO: ClientIO = { ...newClientIO, // Main process intercepted clientIO calls - popupQuestion: async ( + question: async ( + requestId: RequestId | undefined, message: string, choices: string[], - defaultId: number | undefined, - source: string, + defaultId?: number, ) => { + // If a requestId is present, the question is tied to an active request + // and should be displayed in the chat view (renderer) so tests and the + // UI can observe it. Only fall back to a native dialog for broadcast + // questions (no requestId). + if (requestId !== undefined) { + return newClientIO.question( + requestId, + message, + choices, + defaultId, + ); + } const result = await dialog.showMessageBox( shellWindow.mainWindow, { @@ -89,7 +101,6 @@ async function initializeDispatcher( buttons: choices, defaultId, message, - icon: source, }, ); return result.response; diff --git a/ts/packages/shell/src/renderer/src/main.ts b/ts/packages/shell/src/renderer/src/main.ts index cadc88e3e..ea8718b07 100644 --- a/ts/packages/shell/src/renderer/src/main.ts +++ b/ts/packages/shell/src/renderer/src/main.ts @@ -215,8 +215,22 @@ function registerClient( ), ); }, - askYesNo: async (requestId, message, _defaultValue) => { - return chatView.askYesNo(requestId, message, ""); + question: async (requestId, message, choices) => { + // For binary Yes/No with a known requestId, delegate to the existing chatView UI. + if ( + requestId !== undefined && + choices.length === 2 && + choices[0] === "Yes" && + choices[1] === "No" + ) { + const yes = await chatView.askYesNo(requestId, message, ""); + return yes ? 0 : 1; + } + // General multi-choice and broadcast (no requestId) are not yet implemented + // in the Shell renderer — the main process handles those via dialog.showMessageBox. + throw new Error( + "Main process should have handled multi-choice question", + ); }, requestChoice: ( requestId, @@ -240,9 +254,6 @@ function registerClient( proposeAction: async (requestId, actionTemplates, source) => { return chatView.proposeAction(requestId, actionTemplates, source); }, - popupQuestion: () => { - throw new Error("Main process should have handled popupQuestion"); - }, notify: (requestId, event, data, source, seq?) => { withReplayQueue(() => { if (seq !== undefined) { @@ -343,8 +354,8 @@ function registerClient( // TODO: Implement deferred interaction support for agent-server connect mode. // When the shell connects to a SharedDispatcher, requestInteraction is pushed // from the server. Without handling it, the server-side promise hangs for the - // full 10-minute timeout before resolving with the default value (askYesNo) or - // rejecting (proposeAction/popupQuestion). + // full 10-minute timeout before resolving with the defaultId (question) or + // rejecting (proposeAction). // // The fix (Option A): on requestInteraction, dispatch into the existing // chatView.askYesNo / chatView.proposeAction / dialog.showMessageBox UI (all