diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3fde4f7d5b..3b0a5e2f45 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -28,6 +28,7 @@ ### Fixed - ACP session configuration now emits the spec-defined `category` field on the Mode, Model, and Thinking select options (`mode`, `model`, `thought_level`), so standards-compliant ACP clients such as Paseo discover models, modes, and thinking levels instead of an empty model picker (#3922). - The ACP session model catalog is now filtered to active providers via `providers.list/active`, falling back to the full catalog on older session hosts, so ACP clients no longer list models for providers without usable credentials (#3922). +- Workflow-gate asks (ralplan approval, deep-interview questions) now surface through the ACP permission channel when the client does not advertise ACP form elicitation, so plain ACP clients such as Paseo can answer selector gates; free-text asks remain unanswered and the richer `ui` channel stays preferred when advertised (#3925). - `todo_write` now rejects malformed raw arguments with bounded, authority-controlled correction codes instead of a generic rejection: unknown root keys, unknown operation-entry keys, done/drop entries without a task or phase target, and unknown init list-entry keys each surface a fixed message naming the accepted shape without echoing the offending input, while recoverable payloads keep the passthrough/coercion path and the existing ask-tool codes are untouched (#3916). - The Alibaba Token Plan onboarding preset and `alibaba-token-plan-qwen-deepseek` profile now reference the provider-supported `qwen3.8-max` model id instead of `qwen-3.8-max`, preventing the built-in profile from selecting an HTTP 400 unsupported model (#3909). diff --git a/packages/coding-agent/src/modes/acp/acp-agent.ts b/packages/coding-agent/src/modes/acp/acp-agent.ts index ee4a8fcae5..9eb2a57ea0 100644 --- a/packages/coding-agent/src/modes/acp/acp-agent.ts +++ b/packages/coding-agent/src/modes/acp/acp-agent.ts @@ -737,7 +737,13 @@ export function acpRequestFailure(error: unknown): unknown { } } -/** Registers a permission provider only when the ACP client requires prompts. */ +/** + * Registers the permission reverse channel whenever a form-less client needs + * to answer selector asks. The permission mode (prompt vs allow) only gates + * tool-authorization prompts via `permission_mode.set`; workflow questions + * still need a channel, so form-less clients always get the permission + * capability and the bus installs the permission-backed ask source on it. + */ export function acpProviderRegistrations( capabilities: ClientCapabilities | undefined, env: NodeJS.ProcessEnv = process.env, @@ -757,7 +763,7 @@ export function acpProviderRegistrations( ] : []), ...(capabilities?.terminal ? [{ capability: "terminal", definitions: [] }] : []), - ...(resolveAcpPermissionMode(capabilities, env) === "prompt" + ...(resolveAcpPermissionMode(capabilities, env) === "prompt" || !capabilities?.elicitation?.form ? [{ capability: "permission", definitions: [] }] : []), ...(capabilities?.elicitation?.form ? [{ capability: "ui", definitions: [] }] : []), diff --git a/packages/coding-agent/src/sdk/broker/ensure.ts b/packages/coding-agent/src/sdk/broker/ensure.ts index 0ec3f29331..814ecd94e6 100644 --- a/packages/coding-agent/src/sdk/broker/ensure.ts +++ b/packages/coding-agent/src/sdk/broker/ensure.ts @@ -279,6 +279,25 @@ async function ensureBrokerOnce(settings: EnsureBrokerSettings, initiator: Ensur await sleep(50); } const exitedBeforeDiscovery = child.exitCode !== null || child.signalCode !== null; + if (exitedBeforeDiscovery && child.exitCode === 0) { + // A clean exit means another broker won the ownership lock (two ACP + // processes racing a cold broker state, e.g. a provider probe and an + // agent launch). The winner may publish its discovery right after our + // last poll; reuse it instead of failing the caller. Transient discovery + // read failures fall through to the common cleanup + failure path below. + try { + for (let retry = 0; retry < 20; retry++) { + const winner = await readBrokerDiscovery(settings.agentDir, settings.heartbeatTtlMs); + if (winner) { + await owner.stop(); + return { kind: "external-discovery", discovery: winner }; + } + await sleep(50); + } + } catch { + // fall through to cleanup + failure + } + } const failure = spawnError ? new Error(`Failed to spawn detached SDK broker: ${spawnError.message}`) : exitedBeforeDiscovery diff --git a/packages/coding-agent/src/sdk/bus/index.ts b/packages/coding-agent/src/sdk/bus/index.ts index b53e4d0182..9e62152733 100644 --- a/packages/coding-agent/src/sdk/bus/index.ts +++ b/packages/coding-agent/src/sdk/bus/index.ts @@ -57,7 +57,12 @@ import type { AskSettlement, AskSettlementResult, } from "../../tools"; -import { registerAskAnswerSource, registerWorkflowGateEmitterListener } from "../../tools/ask-answer-registry"; +import { RECOMMENDED_SUFFIX } from "../../tools/ask"; +import { + GJC_ASK_TIMEOUT_CODE, + registerAskAnswerSource, + registerWorkflowGateEmitterListener, +} from "../../tools/ask-answer-registry"; import { acpFinalTextFromMessage } from "../acp/final-text"; import { ensureBroker } from "../broker/ensure"; import { SessionIndex } from "../broker/session-index"; @@ -1783,6 +1788,158 @@ function createSdkUiAskAnswerSource( awaitAnswerRequest, }; } + +/** + * Ask-answer source that bridges workflow-gate asks to the ACP permission + * channel (`session/request_permission`). Used when the client does not + * advertise ACP form elicitation (e.g. Paseo): the gate question is sent as + * a permission request whose options are the answer choices, and the + * selected optionId maps back to the answer. Only selector asks are bridged; + * free-text asks have no permission-option representation and stay + * unanswered (unchanged from today). Auto-approval follows the client's + * permission mode, so gates never self-approve under `prompt`. + */ +export function createSdkPermissionAskAnswerSource( + requestPermission: (params: Record, signal?: AbortSignal) => Promise, +): AskAnswerSource { + const awaitAnswerRequest = async ( + request: AskAnswerRequest, + signal?: AbortSignal, + ): Promise => { + if (signal?.aborted) return undefined; + if (request.interaction !== "selector") return undefined; + // AskTool appends its synthetic "Other"/clarification transition entries + // at the end; a free-text editor this channel cannot complete. Remove + // exactly those trailing entries so a legitimate option that happens to + // share a transition label is preserved and recommendedIndex stays valid. + const transitionCount = + typeof request.transitionCount === "number" && + Number.isInteger(request.transitionCount) && + request.transitionCount > 0 + ? Math.min(request.transitionCount, request.options.length) + : 0; + const bridgedOptions = + transitionCount > 0 ? request.options.slice(0, request.options.length - transitionCount) : request.options; + // An ask with no model-supplied choices leaves only the synthetic + // transition entries; do not send an unanswerable permission request. + // An ask with no model-supplied choices leaves only the synthetic + // transition entries; do not send an unanswerable permission request + // unless an enabled control can still commit something. + if (bridgedOptions.length === 0 && !request.controls.some(control => control.enabled)) return undefined; + const recommendedLabel = + typeof request.recommendedIndex === "number" && + Number.isInteger(request.recommendedIndex) && + request.recommendedIndex >= 0 && + request.recommendedIndex < bridgedOptions.length + ? bridgedOptions[request.recommendedIndex] + : undefined; + const selectedOptions = request.selectedOptions; + const markSelection = selectedOptions !== undefined && selectedOptions.length > 0; + const choices = new Map(); + const options: Array> = bridgedOptions.map((label, index) => { + const optionId = `option:${index}`; + choices.set(optionId, { kind: "value", value: label }); + const selected = markSelection && selectedOptions?.includes(label) === true; + const name = markSelection ? `${selected ? "[x] " : "[ ] "}${label}` : label; + return { + optionId, + name: label === recommendedLabel ? `${name}${RECOMMENDED_SUFFIX}` : name, + kind: "allow_once", + }; + }); + for (const control of request.controls) { + if (!control.enabled) continue; + const optionId = `control:${control.id}`; + choices.set(optionId, { kind: "control", controlId: control.id }); + options.push({ optionId, name: control.label, kind: "allow_once" }); + } + const requestController = new AbortController(); + const onRequestAbort = () => requestController.abort(); + signal?.addEventListener("abort", onRequestAbort, { once: true }); + const { + promise: requestPromise, + resolve: resolveRequest, + reject: rejectRequest, + } = Promise.withResolvers(); + const timeoutTimer = + request.timeoutMs === undefined + ? undefined + : setTimeout(() => { + requestController.abort(); + resolveRequest(undefined); + }, request.timeoutMs); + void requestPermission( + { + toolCall: { + toolCallId: crypto.randomUUID(), + toolName: "ask", + title: request.question, + rawInput: { question: request.question }, + }, + options, + }, + requestController.signal, + ).then( + value => { + resolveRequest(value); + }, + error => { + rejectRequest(error); + }, + ); + const askTimeoutError = Object.assign(new Error("ask timed out"), { code: GJC_ASK_TIMEOUT_CODE }); + let response: unknown; + try { + response = await requestPromise; + } catch (error) { + if (requestController.signal.aborted && !signal?.aborted) throw askTimeoutError; + throw error; + } finally { + if (timeoutTimer !== undefined) clearTimeout(timeoutTimer); + signal?.removeEventListener("abort", onRequestAbort); + } + if (signal?.aborted) return undefined; + // The configured ask timeout elapsed with no answer: throw the marked + // timeout error so the ask tool distinguishes it from a genuine + // cancellation (which must never auto-select) and its own + // auto-selection-on-timeout policy stays authoritative. + if (requestController.signal.aborted) throw askTimeoutError; + if (!isRecord(response)) return undefined; + // ACP clients (e.g. Paseo) return `{ outcome: { outcome, optionId } }`; + // accept the flat legacy shape as well. + const outcome = isRecord(response.outcome) ? response.outcome : response; + if (outcome.outcome === "cancelled") return undefined; + if (outcome.outcome !== "selected" || typeof outcome.optionId !== "string") return undefined; + const interaction = choices.get(outcome.optionId); + if (!interaction) return undefined; + if (interaction.kind === "value") return interaction.value; + let settled: Promise | undefined; + return { + source: "remote", + interaction, + settle(settlement) { + if (!settled) { + settled = Promise.resolve( + settlement.kind === "commit" + ? { kind: "committed", ack: { status: "failed", reason: "unsupported" } } + : settlement.kind === "invalid" + ? { kind: "invalid_closed" } + : { kind: "resolved_without_commit" }, + ); + } + return settled; + }, + }; + }; + return { + async awaitAnswer(question, options, signal) { + const answer = await awaitAnswerRequest({ question, options, interaction: "selector", controls: [] }, signal); + if (!answer || typeof answer === "string") return answer; + return answer.interaction.kind === "value" ? answer.interaction.value : undefined; + }, + awaitAnswerRequest, + }; +} /** Register the interactive `ask` answer source for a session (the ask tool * races the local UI against a remote reply). Returns the deregister disposer. */ function registerInteractiveAnswerSource( @@ -3631,6 +3788,17 @@ export function createNotificationsExtension( const revisions = new RevisionStore(id, Date.now, { storageDir: stateRoot }); let host: SessionSdkHost | undefined; let disposeUiAnswerSource: (() => void) | undefined; + let disposePermissionAnswerSource: (() => void) | undefined; + let permissionCapabilityActive = false; + const installPermissionAnswerSource = () => { + if (disposeUiAnswerSource || disposePermissionAnswerSource) return; + disposePermissionAnswerSource = registerAskAnswerSource( + id, + createSdkPermissionAskAnswerSource( + async (params, signal) => await host!.reverse.request("permission", "request", params, signal), + ), + ); + }; const installProviderDefinitions = (capability: string, definitions: unknown) => { validateProviderDefinitions(capability, definitions); if (capability === "permission") { @@ -3658,6 +3826,11 @@ export function createNotificationsExtension( }; throw new Error("permission provider returned an invalid response"); }); + permissionCapabilityActive = true; + // Clients without ACP form elicitation (e.g. Paseo) still surface + // workflow-gate asks through the permission channel; a client that + // advertises `elicitation.form` keeps the richer `ui` source instead. + installPermissionAnswerSource(); return; } if (capability === "ui") { @@ -3668,6 +3841,8 @@ export function createNotificationsExtension( async (params, signal) => await host!.reverse.request("ui", "ui.elicit", params, signal), ), ); + disposePermissionAnswerSource?.(); + disposePermissionAnswerSource = undefined; return; } if (capability !== "fs") return; @@ -3704,11 +3879,19 @@ export function createNotificationsExtension( ctx.setSdkClientBridge?.(bridge); }; const removeProviderDefinitions = (capability: string) => { - if (capability === "permission") ctx.setSdkPermissionProvider?.(undefined); + if (capability === "permission") { + ctx.setSdkPermissionProvider?.(undefined); + permissionCapabilityActive = false; + disposePermissionAnswerSource?.(); + disposePermissionAnswerSource = undefined; + } if (capability === "fs") ctx.setSdkClientBridge?.(undefined); if (capability === "ui") { disposeUiAnswerSource?.(); disposeUiAnswerSource = undefined; + // The permission lease may still be live; restore its ask source so + // later headless asks keep an ACP answer channel. + if (permissionCapabilityActive) installPermissionAnswerSource(); } }; diff --git a/packages/coding-agent/src/tools/ask-answer-registry.ts b/packages/coding-agent/src/tools/ask-answer-registry.ts index ddd38a0ab0..c26dd2e5c5 100644 --- a/packages/coding-agent/src/tools/ask-answer-registry.ts +++ b/packages/coding-agent/src/tools/ask-answer-registry.ts @@ -7,6 +7,9 @@ * source; registering returns a disposer. */ +/** Error code a remote ask source uses to signal that its own timeout fired. */ +export const GJC_ASK_TIMEOUT_CODE = "gjc.ask.timeout"; + import { logger } from "@gajae-code/utils"; import type { WorkflowGateEmitter } from "../modes/shared/agent-wire/workflow-gate-broker"; import type { AskAnswerSource } from "./index"; diff --git a/packages/coding-agent/src/tools/ask.ts b/packages/coding-agent/src/tools/ask.ts index d5441a87cf..ec87798bed 100644 --- a/packages/coding-agent/src/tools/ask.ts +++ b/packages/coding-agent/src/tools/ask.ts @@ -61,6 +61,7 @@ import type { AskSettlementResult, ToolSession, } from "."; +import { GJC_ASK_TIMEOUT_CODE } from "./ask-answer-registry"; import { formatErrorMessage, formatMeta, formatTitle } from "./render-utils"; import { ToolAbortError } from "./tool-errors"; @@ -573,10 +574,12 @@ export interface AskToolDetails { // Constants // ============================================================================= -const OTHER_OPTION = "Other (type your own)"; -const ASK_CLARIFICATION_OPTION = "Ask about these choices"; -const RECOMMENDED_SUFFIX = " (Recommended)"; +export const OTHER_OPTION = "Other (type your own)"; +export const ASK_CLARIFICATION_OPTION = "Ask about these choices"; +export const RECOMMENDED_SUFFIX = " (Recommended)"; const REMOTE_NAVIGATION_FORWARD = "\u0000ask-navigation-forward"; +const HEADLESS_CHECKBOX_CHECKED = "[x]"; +const HEADLESS_CHECKBOX_UNCHECKED = "[ ]"; const DEEP_INTERVIEW_SELECTOR_SCROLL_TITLE_ROWS = Number.MAX_SAFE_INTEGER; const DEEP_INTERVIEW_RECORDER_AWAIT_TIMEOUT_MS = 250; @@ -584,6 +587,10 @@ function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } +function isAskTimeoutError(error: unknown): boolean { + return typeof error === "object" && error !== null && (error as { code?: unknown }).code === GJC_ASK_TIMEOUT_CODE; +} + async function awaitDeepInterviewRecorderPersistence(persistence: Promise, required: boolean): Promise { if (required) { await persistence; @@ -609,7 +616,8 @@ async function awaitDeepInterviewRecorderPersistence(persistence: Promise, } function getDoneOptionLabel(): string { - return `${theme.status.success} Done selecting`; + const success = theme?.status?.success; + return success ? `${success} Done selecting` : "Done selecting"; } function validRecommendedIndex(recommended: number | undefined, optionCount: number): number | undefined { @@ -885,9 +893,22 @@ async function askSingleQuestion( : undefined, }; const startMs = Date.now(); - const choice = signal - ? await untilAborted(signal, () => ui.select(prompt, optionsToShow, dialogOptions)) - : await ui.select(prompt, optionsToShow, dialogOptions); + let choice: string | undefined; + try { + choice = signal + ? await untilAborted(signal, () => ui.select(prompt, optionsToShow, dialogOptions)) + : await ui.select(prompt, optionsToShow, dialogOptions); + } catch (error) { + // A remote source signals its own timeout with the marked error; a + // genuine cancellation (even past the deadline) is not a timeout and + // must not auto-select. + if (!signal?.aborted && isAskTimeoutError(error)) { + timeoutTriggered = true; + choice = undefined; + } else { + throw error; + } + } if (!timeoutTriggered && choice === undefined && typeof timeout === "number") { timeoutTriggered = Date.now() - startMs >= timeout; } @@ -928,6 +949,8 @@ async function askSingleQuestion( const promptWithProgress = navigation?.progressText ? `${question} (${navigation.progressText})` : question; if (multi) { const selected = new Set(selectedOptions); + const checkedCheckbox = theme?.checkbox?.checked ?? HEADLESS_CHECKBOX_CHECKED; + const uncheckedCheckbox = theme?.checkbox?.unchecked ?? HEADLESS_CHECKBOX_UNCHECKED; let cursorIndex = Math.min(Math.max(recommended ?? 0, 0), Math.max(optionLabels.length - 1, 0)); const firstSelected = selectedOptions[0]; if (firstSelected) { @@ -938,7 +961,7 @@ async function askSingleQuestion( const opts: string[] = []; for (const opt of optionLabels) { - const checkbox = selected.has(opt) ? theme.checkbox.checked : theme.checkbox.unchecked; + const checkbox = selected.has(opt) ? checkedCheckbox : uncheckedCheckbox; opts.push(`${checkbox} ${opt}`); } @@ -1011,13 +1034,17 @@ async function askSingleQuestion( cursorIndex = selectedIdx; } - const checkedPrefix = `${theme.checkbox.checked} `; - const uncheckedPrefix = `${theme.checkbox.unchecked} `; + const checkedPrefix = `${checkedCheckbox} `; + const uncheckedPrefix = `${uncheckedCheckbox} `; let opt: string | undefined; if (choice.startsWith(checkedPrefix)) { opt = choice.slice(checkedPrefix.length); } else if (choice.startsWith(uncheckedPrefix)) { opt = choice.slice(uncheckedPrefix.length); + } else if (optionLabels.includes(choice)) { + // A headless remote source (e.g. the ACP permission bridge) returns + // the raw label without checkbox prefixes. + opt = choice; } if (opt) { if (selected.has(opt)) { @@ -1344,19 +1371,23 @@ export class AskTool implements AgentTool { } const remoteValue = receipt.interaction.kind === "value" ? receipt.interaction.value : undefined; const value = remoteValue ?? REMOTE_NAVIGATION_FORWARD; + const checkboxPrefixes = theme?.checkbox + ? [theme.checkbox.checked, theme.checkbox.unchecked].filter( + (prefix): prefix is string => typeof prefix === "string", + ) + : [HEADLESS_CHECKBOX_CHECKED, HEADLESS_CHECKBOX_UNCHECKED]; const selectedValue = remoteValue === undefined ? value : (options.find( option => option === remoteValue || - option === `${theme.checkbox.checked} ${remoteValue}` || - option === `${theme.checkbox.unchecked} ${remoteValue}`, + checkboxPrefixes.some(prefix => option === `${prefix} ${remoteValue}`), ) ?? value); - const normalizedRemoteValue = remoteValue?.startsWith(`${theme.checkbox.checked} `) - ? remoteValue.slice(`${theme.checkbox.checked} `.length) - : remoteValue?.startsWith(`${theme.checkbox.unchecked} `) - ? remoteValue.slice(`${theme.checkbox.unchecked} `.length) + const checkboxPrefix = checkboxPrefixes.find(prefix => remoteValue?.startsWith(`${prefix} `)); + const normalizedRemoteValue = + remoteValue !== undefined && checkboxPrefix + ? remoteValue.slice(checkboxPrefix.length + 1) : remoteValue; const semanticRemoteValue = normalizedRemoteValue?.replace(/^\s*\d+[.)]\s+/, ""); const transitionReason = @@ -1597,6 +1628,8 @@ export class AskTool implements AgentTool { options: remoteSelectorOptions, interaction: "selector", ...(recommendedIndex === undefined ? {} : { recommendedIndex }), + ...(timeout === undefined || timeout === null ? {} : { timeoutMs: timeout }), + ...(clarificationOptionLabel ? { transitionCount: 2 } : { transitionCount: 1 }), multi: q.multi === true, selectedOptions: [...(initialSelection?.selectedOptions ?? [])], controls: askRemoteControls({ @@ -1623,7 +1656,10 @@ export class AskTool implements AgentTool { navigation: options?.navigation, scrollTitleRows: DEEP_INTERVIEW_SELECTOR_SCROLL_TITLE_ROWS, otherOptionLabel, - autoSelectOnTimeout: !intentContract(q.deepInterview) && !intentReview(q.deepInterview), + autoSelectOnTimeout: + !intentContract(q.deepInterview) && + !intentReview(q.deepInterview) && + (q.workflowGate === undefined || q.workflowGate.kind === "question"), clarificationOptionLabel, onRemoteState: state => { activeRemoteRequest = { @@ -1633,6 +1669,14 @@ export class AskTool implements AgentTool { ...(state.interaction === "selector" && recommendedIndex !== undefined ? { recommendedIndex } : {}), + ...(state.interaction === "selector" && timeout !== undefined && timeout !== null + ? { timeoutMs: timeout } + : {}), + ...(state.interaction === "selector" + ? clarificationOptionLabel + ? { transitionCount: 2 } + : { transitionCount: 1 } + : {}), multi: q.multi === true, selectedOptions: [...state.selectedOptions], controls: diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts index 2f7a03c628..b0af366099 100644 --- a/packages/coding-agent/src/tools/index.ts +++ b/packages/coding-agent/src/tools/index.ts @@ -146,6 +146,10 @@ export interface AskAnswerRequest { * labels. Remote transports render the selection state so a toggle is visible. */ selectedOptions?: readonly string[]; + /** Milliseconds before a remote source auto-selects; absent means no timeout. */ + timeoutMs?: number; + /** Number of trailing synthetic transition entries (Other/clarification) appended by the ask tool. */ + transitionCount?: number; } export type AskRemoteInteraction = diff --git a/packages/coding-agent/test/acp-startup-options.test.ts b/packages/coding-agent/test/acp-startup-options.test.ts index 071d4a0fd9..a3fe0b41bd 100644 --- a/packages/coding-agent/test/acp-startup-options.test.ts +++ b/packages/coding-agent/test/acp-startup-options.test.ts @@ -26,13 +26,20 @@ function providerNames(capabilities: unknown, env: NodeJS.ProcessEnv = {}): stri return acpProviderRegistrations(capabilities as never, env).map(provider => provider.capability); } -test("ACP registers a permission provider only for prompt handling", () => { +test("ACP registers the permission channel for form-less clients regardless of permission mode", () => { expect(providerNames({ _meta: { gjc: { permissionHandling: "prompt" } } })).toContain("permission"); - expect(providerNames({ _meta: { gjc: { permissionHandling: "auto" } } })).not.toContain("permission"); - expect(providerNames({ _meta: { gjc: { permissionHandling: "always-allow" } } })).not.toContain("permission"); + // Form-less clients always get the permission channel so selector asks can + // be answered even in auto/always-allow mode (the mode only gates tools). + expect(providerNames({ _meta: { gjc: { permissionHandling: "auto" } } })).toContain("permission"); + expect(providerNames({ _meta: { gjc: { permissionHandling: "always-allow" } } })).toContain("permission"); expect(providerNames(undefined, { GJC_ACP_PERMISSION_MODE: "prompt" })).toContain("permission"); - expect(providerNames(undefined, { GJC_ACP_PERMISSION_MODE: "auto" })).not.toContain("permission"); + expect(providerNames(undefined, { GJC_ACP_PERMISSION_MODE: "auto" })).toContain("permission"); expect(providerNames({ _meta: { gjc: { permissionHandling: "invalid" } } })).toContain("permission"); + // A form-eliciting client in allow mode keeps only the ui channel. + expect(providerNames({ _meta: { gjc: { permissionHandling: "auto" } }, elicitation: { form: {} } })).not.toContain( + "permission", + ); + expect(providerNames({ _meta: { gjc: { permissionHandling: "auto" } }, elicitation: { form: {} } })).toContain("ui"); }); test("ACP registers the SDK UI provider only for clients with form elicitation", () => { diff --git a/packages/coding-agent/test/sdk-acp-ask-permission-source.test.ts b/packages/coding-agent/test/sdk-acp-ask-permission-source.test.ts new file mode 100644 index 0000000000..f4bb048af0 --- /dev/null +++ b/packages/coding-agent/test/sdk-acp-ask-permission-source.test.ts @@ -0,0 +1,225 @@ +import { expect, test } from "bun:test"; +import { createSdkPermissionAskAnswerSource } from "../src/sdk/bus/index"; +import { GJC_ASK_TIMEOUT_CODE } from "../src/tools/ask-answer-registry"; + +test("bridges selector asks to the ACP permission channel and maps optionId to the answer", async () => { + const requests: Array> = []; + const source = createSdkPermissionAskAnswerSource(async params => { + requests.push(params); + // Paseo returns the nested `{ outcome: { outcome, optionId } }` shape. + return { outcome: { outcome: "selected", optionId: "option:1" } }; + }); + const answer = await source.awaitAnswer("Approve the plan?", ["Approve", "Revise"], undefined); + expect(answer).toBe("Revise"); + expect(requests).toHaveLength(1); + const request = requests[0] as { + toolCall: { toolName: string; title: string; toolCallId: string }; + options: Array>; + }; + expect(request.toolCall.toolName).toBe("ask"); + expect(request.toolCall.title).toBe("Approve the plan?"); + expect(request.toolCall.toolCallId).toBeTypeOf("string"); + expect(request.options).toEqual([ + { optionId: "option:0", name: "Approve", kind: "allow_once" }, + { optionId: "option:1", name: "Revise", kind: "allow_once" }, + ]); +}); + +test("accepts the flat legacy permission outcome as well", async () => { + const source = createSdkPermissionAskAnswerSource(async () => ({ outcome: "selected", optionId: "option:0" })); + const answer = await source.awaitAnswer("Continue?", ["Yes", "No"], undefined); + expect(answer).toBe("Yes"); +}); +test("omits the synthetic trailing transition options from the permission request", async () => { + const requests: Array> = []; + const source = createSdkPermissionAskAnswerSource(async params => { + requests.push(params); + return { outcome: { outcome: "selected", optionId: "option:0" } }; + }); + await source.awaitAnswerRequest!({ + question: "Pick one:", + options: ["Yes", "No", "Other (type your own)", "Ask about these choices"], + interaction: "selector", + controls: [], + transitionCount: 2, + }); + const options = (requests[0] as { options: Array<{ name: string }> }).options; + expect(options.map(option => option.name)).toEqual(["Yes", "No"]); +}); + +test("preserves legitimate options that match transition labels", async () => { + const requests: Array> = []; + const source = createSdkPermissionAskAnswerSource(async params => { + requests.push(params); + return { outcome: { outcome: "selected", optionId: "option:1" } }; + }); + // A legit option named like a transition is preserved; only the single + // synthetic trailing entry is removed, and recommendedIndex stays valid. + await source.awaitAnswerRequest!({ + question: "Pick one:", + options: ["Yes", "Ask about these choices", "Other (type your own)"], + interaction: "selector", + controls: [], + transitionCount: 1, + recommendedIndex: 1, + }); + const options = (requests[0] as { options: Array<{ optionId: string; name: string }> }).options; + expect(options.map(option => option.name)).toEqual(["Yes", "Ask about these choices (Recommended)"]); +}); +test("skips the permission request when only synthetic transitions remain", async () => { + let called = false; + const source = createSdkPermissionAskAnswerSource(async () => { + called = true; + return { outcome: { outcome: "selected", optionId: "option:0" } }; + }); + const result = await source.awaitAnswerRequest!({ + question: "Describe the change", + options: ["Other (type your own)"], + interaction: "selector", + controls: [], + transitionCount: 1, + }); + expect(result).toBeUndefined(); + expect(called).toBe(false); +}); +test("sends the request when an enabled control can commit an empty selection", async () => { + const requests: Array> = []; + const source = createSdkPermissionAskAnswerSource(async params => { + requests.push(params); + return { outcome: { outcome: "selected", optionId: "control:navigation_forward" } }; + }); + const result = await source.awaitAnswerRequest!({ + question: "Select any:", + options: ["Other (type your own)"], + interaction: "selector", + controls: [{ id: "navigation_forward", kind: "navigation", label: "Done", enabled: true }], + transitionCount: 1, + }); + expect(requests).toHaveLength(1); + const options = (requests[0] as { options: Array<{ optionId: string }> }).options; + expect(options.map(option => option.optionId)).toEqual(["control:navigation_forward"]); + expect(result && typeof result === "object" ? result.interaction : undefined).toEqual({ + kind: "control", + controlId: "navigation_forward", + }); +}); + +test("marks selected options in multi-select reissues", async () => { + const requests: Array> = []; + const source = createSdkPermissionAskAnswerSource(async params => { + requests.push(params); + return { outcome: { outcome: "selected", optionId: "control:navigation_forward" } }; + }); + await source.awaitAnswerRequest!({ + question: "Select any:", + options: ["A", "B"], + interaction: "selector", + controls: [{ id: "navigation_forward", kind: "navigation", label: "Done", enabled: true }], + multi: true, + selectedOptions: ["A"], + }); + const options = (requests[0] as { options: Array<{ name: string }> }).options; + expect(options.map(option => option.name)).toEqual(["[x] A", "[ ] B", "Done"]); +}); + +test("cancelled permission responses leave the ask unanswered", async () => { + const source = createSdkPermissionAskAnswerSource(async () => ({ outcome: { outcome: "cancelled" } })); + await expect(source.awaitAnswer("Proceed?", ["Yes", "No"], undefined)).resolves.toBeUndefined(); +}); + +test("maps enabled navigation controls to permission options and returns a control interaction", async () => { + const source = createSdkPermissionAskAnswerSource(async params => { + const options = (params as { options: Array<{ optionId: string }> }).options; + expect(options.map(option => option.optionId)).toEqual(["option:0", "option:1", "control:navigation_forward"]); + return { outcome: { outcome: "selected", optionId: "control:navigation_forward" } }; + }); + const result = await source.awaitAnswerRequest!({ + question: "Select any:", + options: ["A", "B"], + interaction: "selector", + controls: [{ id: "navigation_forward", kind: "navigation", label: "Done", enabled: true }], + }); + expect(result && typeof result === "object" ? result.interaction : undefined).toEqual({ + kind: "control", + controlId: "navigation_forward", + }); +}); + +test("non-selector asks are not bridged to the permission channel", async () => { + let called = false; + const source = createSdkPermissionAskAnswerSource(async () => { + called = true; + return { outcome: { outcome: "selected", optionId: "option:0" } }; + }); + const result = await source.awaitAnswerRequest!({ + question: "Describe the change", + options: [], + interaction: "custom_editor", + controls: [], + }); + expect(result).toBeUndefined(); + expect(called).toBe(false); +}); +test("decorates the recommended option name", async () => { + const requests: Array> = []; + const source = createSdkPermissionAskAnswerSource(async params => { + requests.push(params); + return { outcome: { outcome: "selected", optionId: "option:1" } }; + }); + await source.awaitAnswerRequest!({ + question: "Approve the plan?", + options: ["Approve", "Revise"], + interaction: "selector", + controls: [], + recommendedIndex: 1, + }); + const options = (requests[0] as { options: Array<{ optionId: string; name: string }> }).options; + expect(options[0].name).toBe("Approve"); + expect(options[1].name).toBe("Revise (Recommended)"); + expect(requests[0].options).toEqual([ + { optionId: "option:0", name: "Approve", kind: "allow_once" }, + { optionId: "option:1", name: "Revise (Recommended)", kind: "allow_once" }, + ]); +}); + +test("signals its own timeout with the marked error", async () => { + const { promise: neverAnswer } = Promise.withResolvers(); + const source = createSdkPermissionAskAnswerSource(() => neverAnswer); + await expect( + source.awaitAnswerRequest!({ + question: "Proceed?", + options: ["Yes", "No"], + interaction: "selector", + controls: [], + recommendedIndex: 0, + timeoutMs: 50, + }), + ).rejects.toMatchObject({ code: GJC_ASK_TIMEOUT_CODE }); +}); + +test("aborts the underlying permission request on timeout", async () => { + let sawAbortSignal = false; + const { promise: aborted, resolve: resolveAborted } = Promise.withResolvers(); + const source = createSdkPermissionAskAnswerSource(async (_params, signal) => { + signal?.addEventListener( + "abort", + () => { + sawAbortSignal = true; + resolveAborted(); + }, + { once: true }, + ); + await aborted; + return { outcome: { outcome: "selected", optionId: "option:0" } }; + }); + await expect( + source.awaitAnswerRequest!({ + question: "Proceed?", + options: ["Yes", "No"], + interaction: "selector", + controls: [], + timeoutMs: 50, + }), + ).rejects.toMatchObject({ code: GJC_ASK_TIMEOUT_CODE }); + expect(sawAbortSignal).toBe(true); +}); diff --git a/packages/coding-agent/test/tools/ask.test.ts b/packages/coding-agent/test/tools/ask.test.ts index e302d89cd6..3783d8d9ce 100644 --- a/packages/coding-agent/test/tools/ask.test.ts +++ b/packages/coding-agent/test/tools/ask.test.ts @@ -2674,6 +2674,68 @@ describe("AskTool deep-interview recorder persistence", () => { expect(reviewResult.details?.selectedOptions).toEqual([]); expect(recorder).not.toHaveBeenCalled(); }); + it("does not auto-select a ralplan approval gate on ask timeout", async () => { + const tool = new AskTool( + createSession({ + settings: Settings.isolated({ "ask.timeout": 0.001 }), + getSessionId: () => "session-ask", + }), + ); + const context = createContext({ + select: async (_prompt, _options, dialogOptions) => { + const timeout = dialogOptions?.timeout ?? 1; + await Bun.sleep(timeout + 5); + dialogOptions?.onTimeout?.(); + return _options[0]; + }, + }); + const approvalQuestion = { + id: "ralplan-approval-timeout", + question: "Approve the plan?", + options: [{ label: "Approve" }, { label: "Revise" }], + workflowGate: { stage: "ralplan", kind: "approval" } as const, + }; + const result = await tool.execute( + "ralplan-approval-timeout", + { questions: [approvalQuestion] }, + undefined, + undefined, + context, + ); + // A timeout is not consent: the plan approval must stay unselected. + expect(result.details?.selectedOptions).toEqual([]); + }); + it("does not auto-select an execution gate on ask timeout", async () => { + const tool = new AskTool( + createSession({ + settings: Settings.isolated({ "ask.timeout": 0.001 }), + getSessionId: () => "session-ask", + }), + ); + const context = createContext({ + select: async (_prompt, _options, dialogOptions) => { + const timeout = dialogOptions?.timeout ?? 1; + await Bun.sleep(timeout + 5); + dialogOptions?.onTimeout?.(); + return _options[0]; + }, + }); + const executionQuestion = { + id: "ultragoal-execution-timeout", + question: "Approve execution?", + options: [{ label: "Approve" }, { label: "Hold" }], + workflowGate: { stage: "ultragoal", kind: "execution" } as const, + }; + const result = await tool.execute( + "ultragoal-execution-timeout", + { questions: [executionQuestion] }, + undefined, + undefined, + context, + ); + // A timeout is not execution authorization. + expect(result.details?.selectedOptions).toEqual([]); + }); it("discards focused intent choices before multi-question timeout navigation", async () => { const recorder = spyOn(deepInterviewRecorder, "appendOrMergeDeepInterviewRound").mockResolvedValue({