From dadb0b136eb2c0a889d759ac61cdd246de015736 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Wed, 8 Apr 2026 15:58:06 -0500 Subject: [PATCH 1/2] fix(remote-questions): race local TUI against remote channel instead of remote-only routing When a remote channel (Discord/Slack/Telegram) is configured, ask_user_questions now races the local TUI against the remote dispatch. The first response wins and the loser is cancelled. Previously, remote completely preempted the local TUI, meaning terminal users never saw the question prompt when remote was configured. Closes #3801 --- .../extensions/ask-user-questions.ts | 114 ++++++++++++++++-- .../extensions/remote-questions/manager.ts | 9 ++ 2 files changed, 112 insertions(+), 11 deletions(-) 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/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, From c1a68a48a9eaa788dd7d4d63a093a3398980ccf7 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Wed, 8 Apr 2026 16:20:13 -0500 Subject: [PATCH 2/2] test(remote-questions): add regression tests for race model (#3810) Validates the race routing logic: raceRemoteAndLocal helper exists, routing checks both hasRemote and ctx.hasUI, remote timeouts are treated as non-wins, AbortController cancels the loser, and isRemoteConfigured is exported from manager.ts. --- .../gsd/tests/remote-questions.test.ts | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) 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"),