diff --git a/src/resources/extensions/ask-user-questions.ts b/src/resources/extensions/ask-user-questions.ts index 380c86bfe..ab7ff280a 100644 --- a/src/resources/extensions/ask-user-questions.ts +++ b/src/resources/extensions/ask-user-questions.ts @@ -107,6 +107,65 @@ export function resetAskUserQuestionsCache(): void { turnCache.clear(); } +// ─── Race helper ───────────────────────────────────────────────────────────── + +interface RaceableResult { + content: { type: "text"; text: string }[]; + details?: unknown; +} + +/** + * Race a remote channel dispatch against the local TUI. The first to produce + * a valid (non-error, non-timeout) result wins. The loser is cancelled via + * the shared AbortController. + * + * If the local TUI responds first, the remote poll is aborted (the message + * stays in Discord/Slack but polling stops). If remote responds first, the + * local TUI prompt is cancelled. + * + * Returns null only when both sides fail or are cancelled. + */ +async function raceRemoteAndLocal( + startRemote: () => Promise, + startLocal: () => Promise, + controller: AbortController, + questions: Question[], +): Promise { + // Wrap local TUI result into the same shape as remote results + const localPromise = startLocal().then((result): RaceableResult | null => { + if (!result || Object.keys(result.answers).length === 0) return null; + return { + content: [{ type: "text" as const, text: formatForLLM(result) }], + details: { questions, response: result, cancelled: false } satisfies LocalResultDetails, + }; + }).catch(() => null); + + const remotePromise = startRemote().then((result): RaceableResult | null => { + if (!result) return null; + const details = result.details as Record | undefined; + // Treat timeouts and errors as non-wins — let the local TUI win instead + if (details?.timed_out || details?.error) return null; + return result; + }).catch(() => null); + + // Race: first non-null result wins + const winner = await Promise.race([ + localPromise.then((r) => r ? { source: "local" as const, result: r } : null), + remotePromise.then((r) => r ? { source: "remote" as const, result: r } : null), + ]); + + if (winner) { + // Cancel the loser + controller.abort(); + return winner.result; + } + + // First to resolve was null — wait for the other + const [localResult, remoteResult] = await Promise.all([localPromise, remotePromise]); + controller.abort(); + return localResult ?? remoteResult; +} + // ─── Helpers ────────────────────────────────────────────────────────────────── const OTHER_OPTION_LABEL = "None of the above"; @@ -180,20 +239,53 @@ export default function AskUserQuestions(pi: ExtensionAPI) { } } - // Try remote first if configured (works in both interactive and headless modes). - // tryRemoteQuestions returns null when no remote channel is configured, so - // this is a no-op when the user has not set up Slack/Discord/Telegram. - const { tryRemoteQuestions } = await import("./remote-questions/manager.js"); - const remoteResult = await tryRemoteQuestions(params.questions, signal); - if (remoteResult) { - // Cache successful remote results to prevent duplicate Discord dispatches - const remoteDetails = remoteResult.details as Record | undefined; - if (remoteDetails && !remoteDetails.timed_out && !remoteDetails.error) { - turnCache.set(sig, remoteResult as unknown as CachedResult); + // ── Routing: race remote + local, remote-only, or local-only ──────── + const { tryRemoteQuestions, isRemoteConfigured } = await import("./remote-questions/manager.js"); + const hasRemote = isRemoteConfigured(); + + // Case 1: Both remote and local UI available — race them. + // The first response wins; the loser is cancelled via AbortController. + if (hasRemote && ctx.hasUI) { + const raceController = new AbortController(); + // Merge the parent signal so external cancellation propagates. + const onParentAbort = () => raceController.abort(); + signal?.addEventListener("abort", onParentAbort, { once: true }); + const raceSignal = raceController.signal; + + const raceResult = await raceRemoteAndLocal( + () => tryRemoteQuestions(params.questions, raceSignal), + () => showInterviewRound(params.questions, {}, ctx as any), + raceController, + params.questions, + ); + + signal?.removeEventListener("abort", onParentAbort); + + if (raceResult) { + const details = raceResult.details as Record | undefined; + if (details && !details.timed_out && !details.error && !details.cancelled) { + turnCache.set(sig, raceResult as unknown as CachedResult); + } + return { ...raceResult, details: raceResult.details as unknown }; } - return { ...remoteResult, details: remoteResult.details as unknown }; + // Both sides failed/cancelled — fall through to error + return errorResult("ask_user_questions: no response received from local UI or remote channel", params.questions); } + // Case 2: Remote configured but no local UI (headless) — remote only. + if (hasRemote && !ctx.hasUI) { + const remoteResult = await tryRemoteQuestions(params.questions, signal); + if (remoteResult) { + const remoteDetails = remoteResult.details as Record | undefined; + if (remoteDetails && !remoteDetails.timed_out && !remoteDetails.error) { + turnCache.set(sig, remoteResult as unknown as CachedResult); + } + return { ...remoteResult, details: remoteResult.details as unknown }; + } + return errorResult("Error: remote channel configured but returned no result", params.questions); + } + + // Case 3: No remote — local UI only. if (!ctx.hasUI) { return errorResult("Error: UI not available (non-interactive mode)", params.questions); } diff --git a/src/resources/extensions/gsd/tests/remote-questions.test.ts b/src/resources/extensions/gsd/tests/remote-questions.test.ts index 4cd08b339..c780e6ecc 100644 --- a/src/resources/extensions/gsd/tests/remote-questions.test.ts +++ b/src/resources/extensions/gsd/tests/remote-questions.test.ts @@ -760,6 +760,104 @@ test("ask-user-questions source-level: tryRemoteQuestions is called before the h ); }); +// ═══════════════════════════════════════════════════════════════════════════ +// Race model tests (#3810) — local TUI races against remote channel +// ═══════════════════════════════════════════════════════════════════════════ + +test("ask-user-questions source-level: raceRemoteAndLocal function exists", () => { + const src = readFileSync( + join(__dirname, "..", "..", "ask-user-questions.ts"), + "utf-8", + ); + assert.ok( + src.includes("async function raceRemoteAndLocal("), + "raceRemoteAndLocal helper should exist for racing local TUI against remote channel", + ); +}); + +test("ask-user-questions source-level: race path uses isRemoteConfigured for routing", () => { + const src = readFileSync( + join(__dirname, "..", "..", "ask-user-questions.ts"), + "utf-8", + ); + assert.ok( + src.includes("isRemoteConfigured()"), + "execute() should call isRemoteConfigured() for lightweight routing decision", + ); +}); + +test("ask-user-questions source-level: race path checks both hasRemote and ctx.hasUI", () => { + // Regression: #3810 — the race should only activate when BOTH remote and local UI + // are available. Headless mode should still use remote-only, and no-remote should + // use local-only. + const src = readFileSync( + join(__dirname, "..", "..", "ask-user-questions.ts"), + "utf-8", + ); + assert.ok( + src.includes("hasRemote && ctx.hasUI"), + "Race path should require both remote configured and local UI available", + ); + assert.ok( + src.includes("hasRemote && !ctx.hasUI"), + "Headless path should handle remote-only when no local UI", + ); +}); + +test("ask-user-questions source-level: race treats remote timeout as non-win", () => { + // Regression: the whole point of the race is that a remote timeout should NOT + // block the local TUI. The race helper must filter out timed_out results. + const src = readFileSync( + join(__dirname, "..", "..", "ask-user-questions.ts"), + "utf-8", + ); + const raceFnStart = src.indexOf("async function raceRemoteAndLocal("); + const raceFnEnd = src.indexOf("\n}", raceFnStart); + const raceFnBody = src.slice(raceFnStart, raceFnEnd); + assert.ok( + raceFnBody.includes("timed_out"), + "raceRemoteAndLocal should check for timed_out in remote results", + ); + assert.ok( + raceFnBody.includes("details?.error"), + "raceRemoteAndLocal should check for error in remote results", + ); +}); + +test("ask-user-questions source-level: race uses AbortController to cancel loser", () => { + const src = readFileSync( + join(__dirname, "..", "..", "ask-user-questions.ts"), + "utf-8", + ); + assert.ok( + src.includes("new AbortController()"), + "Race path should create an AbortController for cancellation", + ); + assert.ok( + src.includes("controller.abort()"), + "raceRemoteAndLocal should abort the controller to cancel the losing side", + ); +}); + +test("manager source-level: isRemoteConfigured export exists", () => { + const src = readFileSync( + join(__dirname, "..", "..", "remote-questions", "manager.ts"), + "utf-8", + ); + assert.ok( + src.includes("export function isRemoteConfigured()"), + "manager.ts should export isRemoteConfigured for lightweight config checking", + ); + // Must delegate to resolveRemoteConfig — no separate config parsing + const fnStart = src.indexOf("export function isRemoteConfigured()"); + const fnEnd = src.indexOf("\n}", fnStart); + const fnBody = src.slice(fnStart, fnEnd); + assert.ok( + fnBody.includes("resolveRemoteConfig()"), + "isRemoteConfigured should delegate to resolveRemoteConfig", + ); +}); + test("config source-level: removeProviderToken uses auth.remove not auth.set with empty key", () => { const commandSrc = readFileSync( join(__dirname, "..", "..", "remote-questions", "remote-command.ts"), diff --git a/src/resources/extensions/remote-questions/manager.ts b/src/resources/extensions/remote-questions/manager.ts index a1e1d10f7..338e354f1 100644 --- a/src/resources/extensions/remote-questions/manager.ts +++ b/src/resources/extensions/remote-questions/manager.ts @@ -24,6 +24,15 @@ interface QuestionInput { allowMultiple?: boolean; } +/** + * Check whether a remote channel is configured without triggering any + * side effects (no HTTP requests, no prompt records). Used by the race + * logic to decide routing before committing to a remote dispatch. + */ +export function isRemoteConfigured(): boolean { + return resolveRemoteConfig() !== null; +} + export async function tryRemoteQuestions( questions: QuestionInput[], signal?: AbortSignal,