Merge pull request #3811 from jeremymcs/worktree-fix+race-remote-local-questions

fix(remote-questions): race local TUI against remote channel

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<RaceableResult | null>,
+	startLocal: () => Promise<RoundResult | null | undefined>,
+	controller: AbortController,
+	questions: Question[],
+): Promise<RaceableResult | null> {
+	// 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<string, unknown> | 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<string, unknown> | 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<string, unknown> | 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<string, unknown> | 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);
 			}

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"),

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,