Merge pull request #2617 from openchamber/feat/unexpected-message-content-edbc

fix(walkthrough): replace raw provider-login error with a readiness blocker

packages/ui/src/components/model-picker/ModelPickerList.tsx

@@ -436,7 +436,10 @@ export const ModelPickerList: React.FC<ModelPickerListProps> = ({
   );
 
   const allowedProviderSet = React.useMemo(() => {
-    if (!allowedProviderIds || allowedProviderIds.length === 0) return null;
+    // undefined = no restriction; [] = allow none. Treating empty like
+    // "unrestricted" would resurface providers without a login in pickers that
+    // intentionally pass the authenticated-only list.
+    if (!allowedProviderIds) return null;
     return new Set(allowedProviderIds);
   }, [allowedProviderIds]);
 

packages/ui/src/components/views/walkthrough/WalkthroughBlocker.tsx

@@ -150,7 +150,7 @@ export const WalkthroughBlocker = ({
             onChange={(providerId, modelId) => {
               void handleModelChange(providerId, modelId);
             }}
-            allowedProviderIds={providers}
+            allowedProviderIds={providers ?? []}
             isModelAllowed={isStructuredOutputCapable}
           />
         </div>

packages/ui/src/components/views/walkthrough/WalkthroughView.tsx

@@ -323,15 +323,36 @@ export const WalkthroughView = ({ directory }: WalkthroughViewProps) => {
   // Explicit pick first, then the model that actually produced what is on
   // screen, then whatever settings resolve to. The middle step is what makes
   // reopening a review show the model behind it rather than the default.
-  const activeModel = selectedModel
-    ?? (entry.result?.model ? `${entry.result.model.providerID}/${entry.result.model.modelID}` : undefined)
-    ?? (entry.readiness?.model ? `${entry.readiness.model.providerID}/${entry.readiness.model.modelID}` : undefined);
-  const [activeProviderId, ...activeModelParts] = (activeModel ?? '').split('/');
-  const activeModelId = activeModelParts.join('/');
-
+  // Never present a provider without a usable login as the current selection —
+  // the picker already hides them from the menu; showing one as selected was
+  // the whole "why say so?" failure mode.
   const modelsMetadata = useConfigStore((state) => state.modelsMetadata);
   const [modelProviders, setModelProviders] = useState<string[] | undefined>(undefined);
 
+  const providerIsAuthenticated = (providerId: string | undefined) => {
+    if (!providerId) return false;
+    // Until the auth list loads, do not present a candidate as selected —
+    // otherwise an unauthenticated config model flashes in the picker.
+    if (modelProviders === undefined) return false;
+    return modelProviders.includes(providerId);
+  };
+  const readinessModelRef = entry.readiness?.model
+    && entry.readiness.model.hasLogin !== false
+    && providerIsAuthenticated(entry.readiness.model.providerID)
+    ? `${entry.readiness.model.providerID}/${entry.readiness.model.modelID}`
+    : undefined;
+  const resultModelRef = entry.result?.model
+    && providerIsAuthenticated(entry.result.model.providerID)
+    ? `${entry.result.model.providerID}/${entry.result.model.modelID}`
+    : undefined;
+  const selectedModelUsable = selectedModel
+    && providerIsAuthenticated(selectedModel.split('/')[0])
+    ? selectedModel
+    : undefined;
+  const activeModel = selectedModelUsable ?? resultModelRef ?? readinessModelRef;
+  const [activeProviderId, ...activeModelParts] = (activeModel ?? '').split('/');
+  const activeModelId = activeModelParts.join('/');
+
   useEffect(() => {
     if (modelProviders !== undefined) return;
     let cancelled = false;
@@ -403,6 +424,8 @@ export const WalkthroughView = ({ directory }: WalkthroughViewProps) => {
 
   const showStages = startedFromEmptyRef.current
     && (entry.status === 'generating' || stageProgress.holding);
+  // Auth/login gaps are not a full-panel blocker: hide the unusable model and
+  // disable Generate instead of explaining a raw provider error.
   const blockedReason = entry.error?.code === 'context-too-small'
     || entry.error?.code === 'structured-output-unsupported'
     || entry.error?.code === 'no-model'
@@ -411,6 +434,7 @@ export const WalkthroughView = ({ directory }: WalkthroughViewProps) => {
     || entry.error?.code === 'output-exhausted'
     ? entry.error.code
     : entry.readiness && !entry.readiness.ready && !view
+      && entry.readiness.reason !== 'no-provider-login'
       ? entry.readiness.reason
       : undefined;
 
@@ -420,11 +444,16 @@ export const WalkthroughView = ({ directory }: WalkthroughViewProps) => {
   const blockedRequiredChars = entry.error?.requiredChars ?? entry.readiness?.requiredChars;
   const blockedAvailableChars = entry.error?.availableChars ?? entry.readiness?.availableChars;
 
+  // Not ready, or no usable selected model, means Generate must not look
+  // actionable — including when the resolved model has no login.
+  const generateDisabled = !activeModel || Boolean(entry.readiness && !entry.readiness.ready);
+
   const handleGenerate = useCallback(
     (force: boolean) => {
+      if (generateDisabled) return;
       void generate(directory, source, { force, language: activeLanguage });
     },
-    [activeLanguage, directory, generate, source]
+    [activeLanguage, directory, generate, generateDisabled, source]
   );
 
   return (
@@ -544,7 +573,8 @@ export const WalkthroughView = ({ directory }: WalkthroughViewProps) => {
             onChange={(providerId, modelId) => {
               selectModel(directory, source, providerId && modelId ? `${providerId}/${modelId}` : null);
             }}
-            allowedProviderIds={modelProviders}
+            // While the auth list is loading, allow none — not every provider.
+            allowedProviderIds={modelProviders ?? []}
             isModelAllowed={isStructuredOutputCapable}
             tooltipsEnabled={false}
             dropdownPortalToBody
@@ -592,7 +622,10 @@ export const WalkthroughView = ({ directory }: WalkthroughViewProps) => {
               type="button"
               variant="outline"
               size="sm"
-              className={WALKTHROUGH_ACTION_CLASS}
+              className={generateDisabled
+                ? 'border-border text-muted-foreground'
+                : WALKTHROUGH_ACTION_CLASS}
+              disabled={generateDisabled}
               aria-label={compactHeader
                 ? (view ? t('walkthrough.action.regenerate') : t('walkthrough.action.generate'))
                 : undefined}
@@ -646,6 +679,7 @@ export const WalkthroughView = ({ directory }: WalkthroughViewProps) => {
             variant="ghost"
             size="xs"
             className="ml-auto"
+            disabled={generateDisabled}
             // Not forced: if an entry for this exact request existed the banner
             // would not be here, and a forced run would refuse the cache it may
             // find on the way.
@@ -677,7 +711,7 @@ export const WalkthroughView = ({ directory }: WalkthroughViewProps) => {
         </div>
       )}
 
-      {entry.error && !blockedReason && (
+      {entry.error && !blockedReason && entry.error.code !== 'no-provider-login' && (
         <div className="flex shrink-0 items-start gap-2 border-b border-border/60 bg-status-error/10 px-3 py-2">
           <Icon name="error-warning" className="mt-0.5 size-4 shrink-0 text-status-error" />
           {/* Provider errors arrive as raw JSON bodies. Show a readable amount

packages/ui/src/lib/walkthrough/types.ts

@@ -91,6 +91,7 @@ export type WalkthroughStage = 'collecting' | 'asking' | 'retrying' | 'assemblin
 
 export type WalkthroughBlockedReason =
   | 'no-model'
+  | 'no-provider-login'
   | 'empty-diff'
   | 'only-generated'
   | 'context-too-small'
@@ -104,6 +105,8 @@ export interface WalkthroughReadiness {
     inputCharBudget?: number;
     contextTokens?: number;
     structuredOutput?: boolean | null;
+    /** False when the resolved provider has no usable OpenCode login. */
+    hasLogin?: boolean;
   };
   requiredChars?: number;
   availableChars?: number;

packages/web/server/lib/small-model/DOCUMENTATION.md

@@ -69,10 +69,17 @@ other runtime API.
 - `timeoutMs` overrides the 60s default per call; `signal` lets a caller abort
   a request that is no longer wanted. Both apply to every wire format.
 - `describeSmallModel()` additionally reports `inputCharBudget`,
-  `contextTokens`, `contextKnown`, and `structuredOutput`. The last is
-  tri-state: `true`/`false` from the catalog, `null` when the catalog omits the
-  field — which it does for roughly half of all models, aggregators and proxies
-  especially. Callers must treat `null` as "try it", not "unsupported".
+  `contextTokens`, `contextKnown`, `structuredOutput`, and `hasLogin`. The last
+  is whether the resolved provider has a usable credential (`auth.json` or
+  config `provider.<id>.options.apiKey`) — settings/config overrides can name a
+  provider with none, and callers such as the walkthrough refuse before the
+  request. `structuredOutput` is tri-state: `true`/`false` from the catalog,
+  `null` when the catalog omits the field — which it does for roughly half of
+  all models, aggregators and proxies especially. Callers must treat `null` as
+  "try it", not "unsupported".
+- Missing credentials throw with `statusCode: 401` and
+  `code: 'no-provider-login'` rather than a bare `Error`, so UI callers can show
+  a blocker instead of a raw 500 message.
 - `call.js` — wire formats and per-provider auth, replicating OpenCode's
   plugin auth loaders:
   - **GitHub Copilot**: fetches the requested model's authenticated `/models`

packages/web/server/lib/small-model/call.js

@@ -566,15 +566,31 @@ const readProviderConfig = (workingDirectory, providerID) => {
 // Dispatch
 // ---------------------------------------------------------------------------
 
+/**
+ * Same credential resolution the request path uses: config
+ * `provider.<id>.options.apiKey` wins, then the auth.json entry.
+ * Callers that need to refuse before spending a request (walkthrough readiness)
+ * must use this rather than inventing a second rule.
+ */
+export function resolveProviderLogin({ auth, workingDirectory, providerID }) {
+  const providerConfig = readProviderConfig(workingDirectory, providerID);
+  return providerConfig?.auth || getAuthEntryForProvider(auth, providerID) || null;
+}
+
 export async function callSmallModel({ auth, catalog, workingDirectory, providerID, modelID, prompt, system, maxOutputTokens, responseSchema, timeoutMs, signal }) {
   const tokens = Number(maxOutputTokens) > 0 ? Number(maxOutputTokens) : DEFAULT_MAX_OUTPUT_TOKENS;
   const providerConfig = readProviderConfig(workingDirectory, providerID);
   // Match OpenCode's resolveSDK precedence:
-  // config provider.<id>.options.apiKey (providerConfig.auth) wins; the
-  // auth.json entry is only a fallback.
+  // config provider.<id>.options.apiKey wins; the auth.json entry is only a fallback.
   const entry = providerConfig?.auth || getAuthEntryForProvider(auth, providerID);
   if (!entry) {
-    throw new Error(`No OpenCode login found for provider "${providerID}"`);
+    // Structured so the walkthrough (and any other caller) can show a blocker
+    // instead of a raw 500 banner with this developer-oriented sentence.
+    throw Object.assign(new Error(`No OpenCode login found for provider "${providerID}"`), {
+      statusCode: 401,
+      code: 'no-provider-login',
+      providerID,
+    });
   }
 
   if (providerID === 'github-copilot') {

packages/web/server/lib/small-model/call.test.js

@@ -171,14 +171,21 @@ describe('callSmallModel — custom provider config', () => {
         provider: { custom: { options: { baseURL: 'https://proxy.example.test/v1' } } },
       });
 
-      await expect(callSmallModel({
+      const error = await callSmallModel({
         auth: {},
         catalog: {},
         workingDirectory: '/proj',
         providerID: 'custom',
         modelID: 'gpt-4o-mini',
         prompt: 'hi',
-      })).rejects.toThrow('No OpenCode login found for provider "custom"');
+      }).then(() => null, (e) => e);
+
+      expect(error).toMatchObject({
+        message: 'No OpenCode login found for provider "custom"',
+        code: 'no-provider-login',
+        statusCode: 401,
+        providerID: 'custom',
+      });
 
       // The credential gate fires before any network call.
       expect(fetchMock).not.toHaveBeenCalled();

packages/web/server/lib/small-model/index.js

@@ -5,7 +5,7 @@ import { readAuthFile } from '../opencode/auth.js';
 import { readConfigLayers } from '../opencode/shared.js';
 import { getModelCatalog } from './catalog.js';
 import { resolveSmallModel, parseModelRef, isUsableAuthEntry, getAuthEntryForProvider } from './resolve.js';
-import { callSmallModel } from './call.js';
+import { callSmallModel, resolveProviderLogin } from './call.js';
 
 const OPENCHAMBER_SETTINGS_FILE = path.join(
   process.env.OPENCHAMBER_DATA_DIR
@@ -252,8 +252,17 @@ export async function describeSmallModel({ directory, preferredProviderID, prefe
     outputReserveTokens: reserveTokens,
   });
 
+  // Settings/config/request overrides can name a provider with no usable login.
+  // Report that here so readiness can refuse before the user pays for a 401.
+  const hasLogin = Boolean(resolveProviderLogin({
+    auth,
+    workingDirectory: directory,
+    providerID: resolved.providerID,
+  }));
+
   return {
     ...resolved,
+    hasLogin,
     inputCharBudget: maxChars,
     contextTokens,
     contextKnown,

packages/web/server/lib/small-model/index.test.js

@@ -18,7 +18,13 @@ vi.mock('./catalog.js', () => ({
   getModelCatalog: vi.fn(),
   getCatalogProvider: vi.fn(),
 }));
-vi.mock('./call.js', () => ({ callSmallModel: vi.fn() }));
+vi.mock('./call.js', () => ({
+  callSmallModel: vi.fn(),
+  resolveProviderLogin: vi.fn(({ auth, providerID }) => {
+    const entry = auth?.[providerID];
+    return entry && typeof entry === 'object' ? entry : null;
+  }),
+}));
 
 const { generateSmallModelText, describeSmallModel } = await import('./index.js');
 const { readAuthFile } = await import('../opencode/auth.js');
@@ -126,6 +132,19 @@ describe('describeSmallModel — capability reporting', () => {
       contextTokens: 8_000,
       contextKnown: true,
       structuredOutput: true,
+      hasLogin: true,
+    });
+  });
+
+  it('reports hasLogin false when the resolved provider has no usable credential', async () => {
+    readAuthFile.mockReturnValue({});
+
+    const described = await describeSmallModel({ directory: '/proj' });
+
+    expect(described).toMatchObject({
+      providerID: 'anthropic',
+      modelID: 'claude-haiku-4-5',
+      hasLogin: false,
     });
   });
 

packages/web/server/lib/walkthrough/DOCUMENTATION.md

@@ -118,6 +118,14 @@ model picker, only shows providers with a usable login. The in-panel picker on a
 blocked walkthrough writes this setting too, so recovering from a refusal never
 silently changes the model behind commit messages.
 
+A settings or `opencode.json` `small_model` override can still name a provider
+with no usable login (neither `auth.json` nor `provider.<id>.options.apiKey`).
+`describeSmallModel` reports that as `hasLogin: false`, readiness refuses with
+`reason: 'no-provider-login'` and omits the unusable model so the panel cannot
+present it as selected, and generation maps the same code to HTTP 401. The UI
+disables Generate and keeps the picker on authenticated providers only — it does
+not surface a raw auth error or a special login blocker for this case.
+
 ## Output language
 
 A walkthrough its reader cannot read is worth nothing, so the prose language is

packages/web/server/lib/walkthrough/index.js

@@ -333,6 +333,13 @@ function computeReadiness({ model, digest, files, fileCount, hunkCount, generate
     return { ready: false, reason, model, generatedFileCount };
   }
 
+  // A resolved override/config model can still have no usable login. Refuse up
+  // front and omit the model — offering an unauthenticated selection in the
+  // picker is what made the old raw auth error feel like a product bug.
+  if (model.hasLogin === false) {
+    return { ready: false, reason: 'no-provider-login' };
+  }
+
   // Built with the same language the generation would use: the instruction is
   // part of the prompt, so a readiness answer computed without it would be
   // measuring a request nobody is going to send.
@@ -392,6 +399,13 @@ async function runGeneration({ directory, source, repoRoot, key, force, explicit
   if (!model) {
     throw fail('No model is available — sign in to a provider first', 404, { code: 'no-model' });
   }
+  if (model.hasLogin === false) {
+    throw fail(
+      `No OpenCode login found for provider "${model.providerID}" — sign in or choose a different model`,
+      401,
+      { code: 'no-provider-login', model },
+    );
+  }
 
   const { digest, files, idByAlias, fileCount, hunkCount, generatedFileCount } = await loadCurrentDiff(directory, source, deps);
   setStage(repoRoot, key, 'asking');
@@ -494,6 +508,9 @@ async function runGeneration({ directory, source, repoRoot, key, force, explicit
     if (error?.code === 'output-exhausted') {
       return fail(error.message, 409, { code: 'output-exhausted', model });
     }
+    if (error?.code === 'no-provider-login') {
+      return fail(error.message, 401, { code: 'no-provider-login', model });
+    }
     return null;
   };
 

packages/web/server/lib/walkthrough/reproduce-2607.test.js

@@ -0,0 +1,148 @@
+import { execFileSync } from 'child_process';
+import fs from 'fs';
+import os from 'os';
+import path from 'path';
+import express from 'express';
+import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
+
+// ---------------------------------------------------------------------------
+// Regression for https://github.com/openchamber/openchamber/issues/2607
+// "[Bug] Why say so?" (walkthrough panel)
+//
+// Before the fix, a walkthrough small model whose provider had no usable login
+// reported readiness ready:true, then generation returned HTTP 500 with the raw
+// message `No OpenCode login found for provider "deepseek"` — shown in the
+// error banner above the "No walkthrough yet" empty state.
+//
+// After the fix: readiness refuses with `no-provider-login`, and generation
+// answers 401 with the same structured code so the UI can show a blocker.
+// ---------------------------------------------------------------------------
+
+const TEMP_HOME = fs.mkdtempSync(path.join(os.tmpdir(), 'oc-home-2607-'));
+process.env.HOME = TEMP_HOME;
+process.env.OPENCHAMBER_DATA_DIR = path.join(TEMP_HOME, '.config', 'openchamber');
+
+const CATALOG = {
+  deepseek: {
+    id: 'deepseek',
+    name: 'DeepSeek',
+    api: 'https://api.deepseek.com',
+    models: {
+      'deepseek-v4-flash': {
+        id: 'deepseek-v4-flash',
+        name: 'DeepSeek V4 Flash',
+        family: 'deepseek-flash',
+        limit: { context: 128_000 },
+      },
+    },
+  },
+};
+
+vi.mock('../../opencode/models-metadata.js', () => ({
+  getModelsMetadata: vi.fn(async () => ({ metadata: CATALOG, fromCache: false })),
+}));
+
+const SOURCE = { kind: 'working-tree', scope: 'all' };
+const REPO_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'oc-repo-2607-'));
+
+const setupGitRepo = () => {
+  const run = (args) => {
+    try {
+      return execFileSync('git', args, { cwd: REPO_DIR, encoding: 'utf8' });
+    } catch (error) {
+      throw new Error(`git ${args.join(' ')} failed: ${error.stderr?.toString() ?? error.message}`);
+    }
+  };
+
+  run(['init', '-b', 'main']);
+  run(['config', 'user.email', 'test@example.com']);
+  run(['config', 'user.name', 'Test']);
+  fs.mkdirSync(path.join(REPO_DIR, 'src'), { recursive: true });
+  fs.writeFileSync(path.join(REPO_DIR, 'src', 'a.ts'), 'export const a = 1;\n', 'utf8');
+  run(['add', 'src/a.ts']);
+  run(['commit', '-m', 'init']);
+  fs.writeFileSync(path.join(REPO_DIR, 'src', 'a.ts'), 'export const a = 1;\nexport const b = 2;\n', 'utf8');
+};
+
+let walkthrough;
+let callSmallModel;
+
+describe('issue 2607 — walkthrough blocks unauthenticated providers', () => {
+  beforeAll(async () => {
+    setupGitRepo();
+    fs.writeFileSync(
+      path.join(REPO_DIR, 'opencode.json'),
+      JSON.stringify({ small_model: 'deepseek/deepseek-v4-flash' }, null, 2),
+      'utf8',
+    );
+
+    walkthrough = await import('./index.js');
+    callSmallModel = await import('../small-model/call.js');
+  });
+
+  afterAll(() => {
+    fs.rmSync(TEMP_HOME, { recursive: true, force: true });
+    fs.rmSync(REPO_DIR, { recursive: true, force: true });
+  });
+
+  it('resolves the deepseek model but reports not ready without a login', async () => {
+    const result = await walkthrough.getWalkthrough({ directory: REPO_DIR, source: SOURCE });
+
+    expect(result.readiness.ready).toBe(false);
+    expect(result.readiness.reason).toBe('no-provider-login');
+    // Unusable models must not be offered as the current selection.
+    expect(result.readiness.model).toBeUndefined();
+  });
+
+  it('callSmallModel throws a structured no-provider-login error', async () => {
+    const error = await callSmallModel.callSmallModel({
+      auth: {},
+      catalog: CATALOG,
+      workingDirectory: REPO_DIR,
+      providerID: 'deepseek',
+      modelID: 'deepseek-v4-flash',
+      prompt: 'x',
+    }).then(() => null, (e) => e);
+
+    expect(error).toBeInstanceOf(Error);
+    expect(error.message).toBe('No OpenCode login found for provider "deepseek"');
+    expect(error.code).toBe('no-provider-login');
+    expect(error.statusCode).toBe(401);
+  });
+
+  it('generateWalkthrough rejects with structured no-provider-login', async () => {
+    const error = await walkthrough.generateWalkthrough({ directory: REPO_DIR, source: SOURCE })
+      .then(() => null, (e) => e);
+
+    expect(error).toBeInstanceOf(Error);
+    expect(error.code).toBe('no-provider-login');
+    expect(error.statusCode).toBe(401);
+    expect(error.model).toMatchObject({ providerID: 'deepseek', modelID: 'deepseek-v4-flash' });
+  });
+
+  it('answers the generate route with HTTP 401 and code no-provider-login', async () => {
+    const service = { ...walkthrough, getPullRequestDiff: async () => { throw new Error('not used'); } };
+    const app = express();
+    app.use(express.json());
+    const { registerWalkthroughRoutes } = await import('./routes.js');
+    registerWalkthroughRoutes(app, { getWalkthroughService: async () => service });
+
+    const server = app.listen(0);
+    await new Promise((resolve) => server.once('listening', resolve));
+    const base = `http://127.0.0.1:${server.address().port}`;
+    try {
+      const response = await fetch(`${base}/api/walkthrough/generate`, {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({ directory: REPO_DIR, source: SOURCE }),
+      });
+      const body = await response.json();
+
+      expect(response.status).toBe(401);
+      expect(body.code).toBe('no-provider-login');
+      expect(body.model).toMatchObject({ providerID: 'deepseek', modelID: 'deepseek-v4-flash' });
+    } finally {
+      await new Promise((resolve) => server.close(resolve));
+    }
+  });
+});