From f5c6c1d94ccca731ee44a50c320da8b8b6d35863 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Thu, 9 Apr 2026 15:49:27 -0500 Subject: [PATCH 1/2] fix(gsd): suppress workflow stderr during /gsd --- .../extensions/gsd/commands/index.ts | 8 ++++++- .../gsd/tests/workflow-logger.test.ts | 16 ++++++++++++++ .../extensions/gsd/workflow-logger.ts | 22 ++++++++++++++++--- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/resources/extensions/gsd/commands/index.ts b/src/resources/extensions/gsd/commands/index.ts index 38f55e0bb..c07476532 100644 --- a/src/resources/extensions/gsd/commands/index.ts +++ b/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); + } }, }); } diff --git a/src/resources/extensions/gsd/tests/workflow-logger.test.ts b/src/resources/extensions/gsd/tests/workflow-logger.test.ts index 68ef6710f..9af623bd5 100644 --- a/src/resources/extensions/gsd/tests/workflow-logger.test.ts +++ b/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, []); + }); }); }); diff --git a/src/resources/extensions/gsd/workflow-logger.ts b/src/resources/extensions/gsd/workflow-logger.ts index e4d62b39b..cdff396a3 100644 --- a/src/resources/extensions/gsd/workflow-logger.ts +++ b/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) From c666ff55eb0dce206fc8bc412601709333b81f67 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Thu, 9 Apr 2026 16:02:45 -0500 Subject: [PATCH 2/2] group gsd model picker by provider --- .../extensions/gsd/commands/handlers/core.ts | 77 ++++++++----- .../gsd/tests/core-overlay-fallback.test.ts | 101 ++++++++++++++++++ 2 files changed, 153 insertions(+), 25 deletions(-) diff --git a/src/resources/extensions/gsd/commands/handlers/core.ts b/src/resources/extensions/gsd/commands/handlers/core.ts index ae8da6c60..9b608f166 100644 --- a/src/resources/extensions/gsd/commands/handlers/core.ts +++ b/src/resources/extensions/gsd/commands/handlers/core.ts @@ -194,6 +194,56 @@ function sortModelsForSelection(models: Model[], currentModel: Model | }); } +function buildProviderModelGroups( + models: Model[], + currentModel: Model | undefined, +): Map[]> { + const byProvider = new Map[]>(); + + 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[], + ctx: ExtensionCommandContext, + currentModel: Model | undefined, +): Promise | 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>(); + 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>(); - 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 { @@ -247,18 +285,7 @@ async function handleModel(trimmedArgs: string, ctx: ExtensionCommandContext, pi return; } - const optionToModel = new Map>(); - 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); } diff --git a/src/resources/extensions/gsd/tests/core-overlay-fallback.test.ts b/src/resources/extensions/gsd/tests/core-overlay-fallback.test.ts index a6c2dc6d9..9a7a21d16 100644 --- a/src/resources/extensions/gsd/tests/core-overlay-fallback.test.ts +++ b/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/); +});