Merge pull request #3901 from jeremymcs/codex/fix-gsd-tui-stderr-corruption

[codex] fix(gsd): suppress workflow stderr during /gsd

src/resources/extensions/gsd/commands/handlers/core.ts

@@ -194,6 +194,56 @@ function sortModelsForSelection(models: Model<any>[], currentModel: Model<any> |
   });
 }
 
+function buildProviderModelGroups(
+  models: Model<any>[],
+  currentModel: Model<any> | undefined,
+): Map<string, Model<any>[]> {
+  const byProvider = new Map<string, Model<any>[]>();
+
+  for (const model of sortModelsForSelection(models, currentModel)) {
+    let group = byProvider.get(model.provider);
+    if (!group) {
+      group = [];
+      byProvider.set(model.provider, group);
+    }
+    group.push(model);
+  }
+  return byProvider;
+}
+
+async function selectModelByProvider(
+  title: string,
+  models: Model<any>[],
+  ctx: ExtensionCommandContext,
+  currentModel: Model<any> | undefined,
+): Promise<Model<any> | undefined> {
+  const byProvider = buildProviderModelGroups(models, currentModel);
+  const providerOptions = Array.from(byProvider.entries()).map(([provider, group]) =>
+    `${provider} (${group.length} model${group.length === 1 ? "" : "s"})`,
+  );
+  providerOptions.push("(cancel)");
+
+  const providerChoice = await ctx.ui.select(`${title} — choose provider:`, providerOptions);
+  if (!providerChoice || typeof providerChoice !== "string" || providerChoice === "(cancel)") return undefined;
+
+  const providerName = providerChoice.replace(/ \(\d+ models?\)$/, "");
+  const providerModels = byProvider.get(providerName);
+  if (!providerModels || providerModels.length === 0) return undefined;
+
+  const optionToModel = new Map<string, Model<any>>();
+  const modelOptions = providerModels.map((model) => {
+    const isCurrent = currentModel && model.provider === currentModel.provider && model.id === currentModel.id;
+    const label = `${isCurrent ? "* " : ""}${model.id}`;
+    optionToModel.set(label, model);
+    return label;
+  });
+  modelOptions.push("(cancel)");
+
+  const modelChoice = await ctx.ui.select(`${title} — ${providerName}:`, modelOptions);
+  if (!modelChoice || typeof modelChoice !== "string" || modelChoice === "(cancel)") return undefined;
+  return optionToModel.get(modelChoice);
+}
+
 async function resolveRequestedModel(
   query: string,
   ctx: ExtensionCommandContext,
@@ -211,19 +261,7 @@ async function resolveRequestedModel(
 
   if (partialMatches.length === 1) return partialMatches[0];
   if (partialMatches.length === 0 || !ctx.hasUI) return undefined;
-
-  const sorted = sortModelsForSelection(partialMatches, ctx.model);
-  const optionToModel = new Map<string, Model<any>>();
-  const options = sorted.map((model) => {
-    const label = `${model.provider}/${model.id}`;
-    optionToModel.set(label, model);
-    return label;
-  });
-  options.push("(cancel)");
-
-  const choice = await ctx.ui.select(`Multiple models match "${query}" — choose one:`, options);
-  if (!choice || typeof choice !== "string" || choice === "(cancel)") return undefined;
-  return optionToModel.get(choice);
+  return selectModelByProvider(`Multiple models match "${query}"`, partialMatches, ctx, ctx.model);
 }
 
 async function handleModel(trimmedArgs: string, ctx: ExtensionCommandContext, pi: ExtensionAPI | undefined): Promise<void> {
@@ -247,18 +285,7 @@ async function handleModel(trimmedArgs: string, ctx: ExtensionCommandContext, pi
       return;
     }
 
-    const optionToModel = new Map<string, Model<any>>();
-    const options = sortModelsForSelection(availableModels, ctx.model).map((model) => {
-      const isCurrent = ctx.model && model.provider === ctx.model.provider && model.id === ctx.model.id;
-      const label = `${isCurrent ? "* " : ""}${model.provider}/${model.id}`;
-      optionToModel.set(label, model);
-      return label;
-    });
-    options.push("(cancel)");
-
-    const choice = await ctx.ui.select("Select session model:", options);
-    if (!choice || typeof choice !== "string" || choice === "(cancel)") return;
-    targetModel = optionToModel.get(choice);
+    targetModel = await selectModelByProvider("Select session model:", availableModels, ctx, ctx.model);
   } else {
     targetModel = await resolveRequestedModel(trimmed, ctx);
   }

src/resources/extensions/gsd/commands/index.ts

@@ -8,7 +8,13 @@ export function registerGSDCommand(pi: ExtensionAPI): void {
     getArgumentCompletions: getGsdArgumentCompletions,
     handler: async (args: string, ctx: ExtensionCommandContext) => {
       const { handleGSDCommand } = await import("./dispatcher.js");
-      await handleGSDCommand(args, ctx, pi);
+      const { setStderrLoggingEnabled } = await import("../workflow-logger.js");
+      const previousStderrSetting = setStderrLoggingEnabled(false);
+      try {
+        await handleGSDCommand(args, ctx, pi);
+      } finally {
+        setStderrLoggingEnabled(previousStderrSetting);
+      }
     },
   });
 }

src/resources/extensions/gsd/tests/core-overlay-fallback.test.ts

@@ -74,3 +74,104 @@ test("model command resolves and persists exact provider-qualified selection", a
   assert.deepEqual(applied, selectedModel);
   assert.match(notices[0]!.message, /openai\/gpt-5\.4/);
 });
+
+test("interactive model picker chooses provider first, then model", async () => {
+  const selectedModel = { provider: "openai", id: "gpt-5.4" };
+  let applied: typeof selectedModel | null = null;
+  const selects: Array<{ title: string; options: string[] }> = [];
+  const notices: Array<{ message: string; type?: string }> = [];
+
+  const ctx = {
+    hasUI: true,
+    model: { provider: "anthropic", id: "claude-sonnet-4-6" },
+    modelRegistry: {
+      getAvailable: () => [
+        { provider: "openai", id: "gpt-5.4" },
+        { provider: "anthropic", id: "claude-opus-4-6" },
+        { provider: "openai", id: "gpt-5.3-mini" },
+        { provider: "anthropic", id: "claude-sonnet-4-6" },
+      ],
+    },
+    ui: {
+      select: async (title: string, options: string[]) => {
+        selects.push({ title, options });
+        return selects.length === 1 ? "openai (2 models)" : "gpt-5.4";
+      },
+      notify: (message: string, type?: string) => {
+        notices.push({ message, type });
+      },
+    },
+  } as any;
+
+  const pi = {
+    setModel: async (model: typeof selectedModel) => {
+      applied = model;
+      return true;
+    },
+  } as any;
+
+  const handled = await handleCoreCommand("model", ctx, pi);
+  assert.equal(handled, true);
+  assert.deepEqual(selects, [
+    {
+      title: "Select session model: — choose provider:",
+      options: ["anthropic (2 models)", "openai (2 models)", "(cancel)"],
+    },
+    {
+      title: "Select session model: — openai:",
+      options: ["gpt-5.3-mini", "gpt-5.4", "(cancel)"],
+    },
+  ]);
+  assert.deepEqual(applied, selectedModel);
+  assert.match(notices[0]!.message, /openai\/gpt-5\.4/);
+});
+
+test("ambiguous typed model selection chooses provider first, then model", async () => {
+  const selectedModel = { provider: "github-copilot", id: "gpt-5" };
+  let applied: typeof selectedModel | null = null;
+  const selects: Array<{ title: string; options: string[] }> = [];
+  const notices: Array<{ message: string; type?: string }> = [];
+
+  const ctx = {
+    hasUI: true,
+    model: { provider: "anthropic", id: "claude-sonnet-4-6" },
+    modelRegistry: {
+      getAvailable: () => [
+        { provider: "openai", id: "gpt-5" },
+        { provider: "github-copilot", id: "gpt-5" },
+        { provider: "openai", id: "gpt-5-mini" },
+      ],
+    },
+    ui: {
+      select: async (title: string, options: string[]) => {
+        selects.push({ title, options });
+        return selects.length === 1 ? "github-copilot (1 model)" : "gpt-5";
+      },
+      notify: (message: string, type?: string) => {
+        notices.push({ message, type });
+      },
+    },
+  } as any;
+
+  const pi = {
+    setModel: async (model: typeof selectedModel) => {
+      applied = model;
+      return true;
+    },
+  } as any;
+
+  const handled = await handleCoreCommand("model gpt", ctx, pi);
+  assert.equal(handled, true);
+  assert.deepEqual(selects, [
+    {
+      title: "Multiple models match \"gpt\" — choose provider:",
+      options: ["github-copilot (1 model)", "openai (2 models)", "(cancel)"],
+    },
+    {
+      title: "Multiple models match \"gpt\" — github-copilot:",
+      options: ["gpt-5", "(cancel)"],
+    },
+  ]);
+  assert.deepEqual(applied, selectedModel);
+  assert.match(notices[0]!.message, /github-copilot\/gpt-5/);
+});

src/resources/extensions/gsd/tests/workflow-logger.test.ts

@@ -18,6 +18,7 @@ import {
   summarizeLogs,
   formatForNotification,
   setLogBasePath,
+  setStderrLoggingEnabled,
   _resetLogs,
 } from "../workflow-logger.ts";
 
@@ -375,5 +376,20 @@ describe("workflow-logger", () => {
       logError("tool", "failed", { cmd: "complete_task" });
       assert.ok(written[0].includes('"cmd":"complete_task"'));
     });
+
+    test("suppresses stderr when disabled", (t) => {
+      const written: string[] = [];
+      const orig = process.stderr.write.bind(process.stderr);
+      const previous = setStderrLoggingEnabled(false);
+      // @ts-ignore — patching for test
+      process.stderr.write = (chunk: string) => { written.push(chunk); return true; };
+      t.after(() => {
+        process.stderr.write = orig;
+        setStderrLoggingEnabled(previous);
+      });
+
+      logWarning("engine", "hidden warning");
+      assert.deepEqual(written, []);
+    });
   });
 });

src/resources/extensions/gsd/workflow-logger.ts

@@ -67,6 +67,7 @@ export interface LogEntry {
 const MAX_BUFFER = 100;
 let _buffer: LogEntry[] = [];
 let _auditBasePath: string | null = null;
+let _stderrEnabled = true;
 
 /**
  * Set the base path for persistent audit log writes.
@@ -77,6 +78,16 @@ export function setLogBasePath(basePath: string): void {
   _auditBasePath = basePath;
 }
 
+/**
+ * Enable or disable immediate stderr writes for workflow logs.
+ * Returns the previous setting so callers can restore it.
+ */
+export function setStderrLoggingEnabled(enabled: boolean): boolean {
+  const previous = _stderrEnabled;
+  _stderrEnabled = enabled;
+  return previous;
+}
+
 // ─── Public API ─────────────────────────────────────────────────────────
 
 /**
@@ -245,7 +256,7 @@ function _push(
   // Always forward to stderr so terminal watchers see it (see module header for policy)
   const prefix = severity === "error" ? "ERROR" : "WARN";
   const ctxStr = context ? ` ${JSON.stringify(context)}` : "";
-  process.stderr.write(`[gsd:${component}] ${prefix}: ${message}${ctxStr}\n`);
+  _writeStderr(`[gsd:${component}] ${prefix}: ${message}${ctxStr}\n`);
 
   // Persist to notification store (both warnings and errors)
   try {
@@ -255,7 +266,7 @@ function _push(
       "workflow-logger",
     );
   } catch (notifErr) {
-    process.stderr.write(`[gsd:workflow-logger] notification-store append failed: ${(notifErr as Error).message}\n`);
+    _writeStderr(`[gsd:workflow-logger] notification-store append failed: ${(notifErr as Error).message}\n`);
   }
 
   // Buffer for auto-loop to drain
@@ -275,11 +286,16 @@ function _push(
       appendFileSync(join(auditDir, "audit-log.jsonl"), JSON.stringify(sanitized) + "\n", "utf-8");
     } catch (auditErr) {
       // Best-effort — never let audit write failures bubble up
-      process.stderr.write(`[gsd:audit] failed to persist log entry: ${(auditErr as Error).message}\n`);
+      _writeStderr(`[gsd:audit] failed to persist log entry: ${(auditErr as Error).message}\n`);
     }
   }
 }
 
+function _writeStderr(message: string): void {
+  if (!_stderrEnabled) return;
+  process.stderr.write(message);
+}
+
 /**
  * Sanitize a log entry before persisting to the audit JSONL file.
  * Strips potentially sensitive context (raw paths, cwd, full error text)