fix(remote-questions): empty-key entry in auth.json shadows valid Discord bot token (#2737)

* fix(remote-questions): empty-key entry in auth.json shadows valid Discord bot token

removeProviderToken() called auth.set(provider, { key: '' }) instead of
auth.remove(provider). Since AuthStorage.set() appends for api_key type
(deduplicating by exact key match), this inserted an empty-key entry at
index 0. Every credential lookup (.get(), .find()) matched the empty
entry first, shadowing valid tokens at later indices.

Fixes:
- remote-command.ts: use auth.remove() instead of auth.set() with empty key
- config.ts: hydrateRemoteTokensFromAuth .find() now requires non-empty key
- wizard.ts: loadStoredEnvKeys uses getCredentialsForProvider + .find()
  instead of .get() which returns creds[0]
- onboarding.ts: check existing tokens via .some() over full credentials
  array instead of .get() which only returns first entry
- key-manager.ts: filter empty-key entries in getAllKeyStatuses, add/remove/
  rotate provider pickers, and doctor env-conflict check

Tests: 3186 pass, 0 fail across full GSD test suite

* fix(config): ignore empty shadowing tool keys

src/onboarding.ts

@@ -669,10 +669,12 @@ async function runRemoteQuestionsStep(
   pc: PicoModule,
   authStorage: AuthStorage,
 ): Promise<string | null> {
-  // Check existing config
-  const hasDiscord = authStorage.has('discord_bot') && !!(authStorage.get('discord_bot') as any)?.key
-  const hasSlack = authStorage.has('slack_bot') && !!(authStorage.get('slack_bot') as any)?.key
-  const hasTelegram = authStorage.has('telegram_bot') && !!(authStorage.get('telegram_bot') as any)?.key
+  // Check existing config — use getCredentialsForProvider to skip empty-key entries
+  const hasValidKey = (provider: string) =>
+    authStorage.getCredentialsForProvider(provider).some((c: any) => c.type === 'api_key' && c.key)
+  const hasDiscord = hasValidKey('discord_bot')
+  const hasSlack = hasValidKey('slack_bot')
+  const hasTelegram = hasValidKey('telegram_bot')
   const existingChannel = hasDiscord ? 'Discord' : hasSlack ? 'Slack' : hasTelegram ? 'Telegram' : null
 
   type RemoteOption = { value: string; label: string; hint?: string }

src/resources/extensions/gsd/commands-config.ts

@@ -22,6 +22,12 @@ export const TOOL_KEYS = [
   { id: "groq",     env: "GROQ_API_KEY",      label: "Groq Voice",        hint: "console.groq.com" },
 ] as const;
 
+function getStoredToolKey(auth: AuthStorage, providerId: string): string | undefined {
+  const creds = auth.getCredentialsForProvider(providerId);
+  const cred = creds.find((c) => c.type === "api_key" && c.key);
+  return cred?.type === "api_key" ? cred.key : undefined;
+}
+
 /**
  * Load tool API keys from auth.json into environment variables.
  * Called at session startup to ensure tools have access to their credentials.
@@ -33,9 +39,9 @@ export function loadToolApiKeys(): void {
 
     const auth = AuthStorage.create(authPath);
     for (const tool of TOOL_KEYS) {
-      const cred = auth.get(tool.id);
-      if (cred && cred.type === "api_key" && cred.key && !process.env[tool.env]) {
-        process.env[tool.env] = cred.key;
+      const key = getStoredToolKey(auth, tool.id);
+      if (key && !process.env[tool.env]) {
+        process.env[tool.env] = key;
       }
     }
   } catch {
@@ -55,14 +61,14 @@ export async function handleConfig(ctx: ExtensionCommandContext): Promise<void>
   // Show current status
   const statusLines = ["GSD Tool Configuration\n"];
   for (const tool of TOOL_KEYS) {
-    const hasKey = !!process.env[tool.env] || !!(auth.get(tool.id) as { key?: string })?.key;
+    const hasKey = !!process.env[tool.env] || !!getStoredToolKey(auth, tool.id);
     statusLines.push(`  ${hasKey ? "\u2713" : "\u2717"} ${tool.label}${hasKey ? "" : ` \u2014 get key at ${tool.hint}`}`);
   }
   ctx.ui.notify(statusLines.join("\n"), "info");
 
   // Ask which tools to configure
   const options = TOOL_KEYS.map(t => {
-    const hasKey = !!process.env[t.env] || !!(auth.get(t.id) as { key?: string })?.key;
+    const hasKey = !!process.env[t.env] || !!getStoredToolKey(auth, t.id);
     return `${t.label} ${hasKey ? "(configured \u2713)" : "(not set)"}`;
   });
   options.push("(done)");

src/resources/extensions/gsd/key-manager.ts

@@ -150,22 +150,13 @@ export interface KeyStatus {
  */
 export function getAllKeyStatuses(auth: AuthStorage): KeyStatus[] {
   return PROVIDER_REGISTRY.map((provider) => {
-    const creds = auth.getCredentialsForProvider(provider.id);
+    const rawCreds = auth.getCredentialsForProvider(provider.id);
+    // Filter out empty-key entries (left by legacy removeProviderToken or skipped onboarding)
+    const creds = rawCreds.filter((c) => !(c.type === "api_key" && !(c as ApiKeyCredential).key));
     const envKey = provider.envVar ? process.env[provider.envVar] : undefined;
 
     if (creds.length > 0) {
       const firstCred = creds[0];
-      // Skip empty keys (from skipped onboarding)
-      if (firstCred.type === "api_key" && !(firstCred as ApiKeyCredential).key) {
-        return {
-          provider,
-          configured: false,
-          source: "none" as const,
-          credentialCount: 0,
-          description: "empty key (skipped setup)",
-          backedOff: false,
-        };
-      }
       const desc =
         creds.length > 1
           ? `${creds.length} keys (round-robin)`
@@ -275,7 +266,7 @@ export async function handleAddKey(
   } else {
     // Interactive provider picker
     const options = PROVIDER_REGISTRY.map((p) => {
-      const creds = auth.getCredentialsForProvider(p.id);
+      const creds = auth.getCredentialsForProvider(p.id).filter((c) => !(c.type === "api_key" && !(c as ApiKeyCredential).key));
       const existing = creds.length > 0 ? " (configured)" : "";
       return `[${p.category}] ${p.label}${existing}`;
     });
@@ -360,7 +351,7 @@ export async function handleRemoveKey(
   } else {
     // Show only configured providers
     const configured = PROVIDER_REGISTRY.filter((p) => {
-      const creds = auth.getCredentialsForProvider(p.id);
+      const creds = auth.getCredentialsForProvider(p.id).filter((c) => !(c.type === "api_key" && !(c as ApiKeyCredential).key));
       return creds.length > 0;
     });
 
@@ -619,7 +610,7 @@ export async function handleRotateKey(
     // Show only configured API key providers
     const configured = PROVIDER_REGISTRY.filter((p) => {
       const creds = auth.getCredentialsForProvider(p.id);
-      return creds.some((c) => c.type === "api_key");
+      return creds.some((c) => c.type === "api_key" && (c as ApiKeyCredential).key);
     });
 
     if (configured.length === 0) {
@@ -788,7 +779,7 @@ export function runKeyDoctor(auth: AuthStorage): DoctorFinding[] {
     if (!envValue) continue;
 
     const creds = auth.getCredentialsForProvider(provider.id);
-    const apiKey = creds.find((c) => c.type === "api_key") as ApiKeyCredential | undefined;
+    const apiKey = creds.find((c) => c.type === "api_key" && (c as ApiKeyCredential).key) as ApiKeyCredential | undefined;
     if (apiKey?.key && apiKey.key !== envValue) {
       findings.push({
         severity: "warning",

src/resources/extensions/gsd/tests/commands-config.test.ts

@@ -0,0 +1,24 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { readFileSync } from "node:fs";
+import { dirname, join } from "node:path";
+import { fileURLToPath } from "node:url";
+
+const __filename = fileURLToPath(import.meta.url);
+const __dirname = dirname(__filename);
+
+test("commands-config source-level: tool key lookup skips empty api_key entries", () => {
+  const source = readFileSync(join(__dirname, "..", "commands-config.ts"), "utf-8");
+  assert.ok(
+    source.includes('getCredentialsForProvider(providerId)'),
+    "commands-config should read the full credential list",
+  );
+  assert.ok(
+    source.includes('c.type === "api_key" && c.key'),
+    "commands-config should require a non-empty api_key when resolving stored tool keys",
+  );
+  assert.ok(
+    !source.includes("auth.get(tool.id)"),
+    "commands-config should not rely on auth.get(tool.id), which can return an empty shadowing entry",
+  );
+});

src/resources/extensions/gsd/tests/key-manager.test.ts

@@ -189,7 +189,22 @@ test("getAllKeyStatuses detects empty keys as not configured", () => {
   const statuses = getAllKeyStatuses(auth);
   const groq = statuses.find((s) => s.provider.id === "groq");
   assert.equal(groq?.configured, false);
-  assert.ok(groq?.description.includes("empty"));
+  // Empty-key entries are filtered out, so provider appears unconfigured
+  assert.equal(groq?.source, "none");
+});
+
+test("getAllKeyStatuses finds valid keys even when empty-key entry exists at index 0", () => {
+  const auth = makeAuth({
+    groq: [
+      { type: "api_key", key: "" },
+      { type: "api_key", key: "gsk-real-key" },
+    ],
+  });
+  const statuses = getAllKeyStatuses(auth);
+  const groq = statuses.find((s) => s.provider.id === "groq");
+  assert.equal(groq?.configured, true);
+  assert.equal(groq?.source, "auth.json");
+  assert.equal(groq?.credentialCount, 1); // only the valid key counts
 });
 
 test("getAllKeyStatuses detects env var keys", () => {

src/resources/extensions/gsd/tests/remote-questions.test.ts

@@ -724,3 +724,32 @@ test("resolveRemoteConfig returns null when preferences are absent (no env side-
     if (savedTelegram !== undefined) process.env.TELEGRAM_BOT_TOKEN = savedTelegram;
   }
 });
+
+test("config source-level: hydration skips api_key entries with empty keys", () => {
+  const configSrc = readFileSync(
+    join(__dirname, "..", "..", "remote-questions", "config.ts"),
+    "utf-8",
+  );
+  // The find() call in hydrateRemoteTokensFromAuth must filter for non-empty keys,
+  // not just match on type === "api_key". This prevents stale empty-key entries
+  // (left by removeProviderToken) from shadowing valid tokens.
+  assert.ok(
+    configSrc.includes('c.type === "api_key" && !!c.key'),
+    "hydrateRemoteTokensFromAuth find() should require a non-empty key",
+  );
+});
+
+test("config source-level: removeProviderToken uses auth.remove not auth.set with empty key", () => {
+  const commandSrc = readFileSync(
+    join(__dirname, "..", "..", "remote-questions", "remote-command.ts"),
+    "utf-8",
+  );
+  // removeProviderToken should call auth.remove(provider), not auth.set(provider, { key: "" }).
+  // Setting an empty key pollutes the credentials array and shadows valid tokens.
+  const fnStart = commandSrc.indexOf("function removeProviderToken");
+  assert.ok(fnStart !== -1, "removeProviderToken should exist");
+  const fnEnd = commandSrc.indexOf("\n}", fnStart);
+  const fnBody = commandSrc.slice(fnStart, fnEnd);
+  assert.ok(fnBody.includes("auth.remove("), "removeProviderToken should call auth.remove()");
+  assert.ok(!fnBody.includes('key: ""'), "removeProviderToken should not set an empty key");
+});

src/resources/extensions/remote-questions/config.ts

@@ -59,7 +59,7 @@ function hydrateRemoteTokensFromAuth(): void {
     for (const [providerId, envVar] of needed) {
       try {
         const creds = auth.getCredentialsForProvider(providerId);
-        const apiKeyCred = creds.find((c: { type: string }) => c.type === "api_key") as
+        const apiKeyCred = creds.find((c: { type: string; key?: string }) => c.type === "api_key" && !!c.key) as
           | { type: "api_key"; key: string }
           | undefined;
         if (apiKeyCred?.key) {

src/resources/extensions/remote-questions/remote-command.ts

@@ -312,7 +312,7 @@ function saveProviderToken(provider: string, token: string): void {
 
 function removeProviderToken(provider: string): void {
   const auth = getAuthStorage();
-  auth.set(provider, { type: "api_key", key: "" });
+  auth.remove(provider);
 }
 
 export function saveRemoteQuestionsConfig(channel: "slack" | "discord" | "telegram", channelId: string): void {

src/wizard.ts

@@ -23,9 +23,12 @@ export function loadStoredEnvKeys(authStorage: AuthStorage): void {
   ]
   for (const [provider, envVar] of providers) {
     if (!process.env[envVar]) {
-      const cred = authStorage.get(provider)
-      if (cred?.type === 'api_key' && cred.key) {
-        process.env[envVar] = cred.key as string
+      // Use getCredentialsForProvider to skip empty-key entries at index 0
+      // (left by legacy removeProviderToken which used set() with empty key)
+      const creds = authStorage.getCredentialsForProvider(provider)
+      const cred = creds.find((c: any) => c.type === 'api_key' && c.key)
+      if (cred?.type === 'api_key' && (cred as any).key) {
+        process.env[envVar] = (cred as any).key as string
       }
     }
   }