fix(walkthrough): block unauthenticated providers with a friendly refusal

When the walkthrough small model resolves to a provider with no usable login, readiness was still ready and generate returned a raw 500 message. Refuse up front with no-provider-login and surface a blocker instead. Closes openchamber/openchamber#2607 Co-authored-by: Serhii Dziupin <makeittech@users.noreply.github.com>
2026-08-04 11:29:58 +00:00
parent f47110c66f
commit abb396e080
22 changed files with 286 additions and 12 deletions
@@ -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`
@@ -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') {
@@ -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();
@@ -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,
@@ -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,
    });
  });

@@ -118,6 +118,13 @@ 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
+`code: 'no-provider-login'`, and generation maps the same code to HTTP 401 —
+so the panel shows a blocker with a model picker instead of looking ready and
+then dumping the raw `No OpenCode login found for provider "…"` string.
+
 ## Output language

 A walkthrough its reader cannot read is worth nothing, so the prose language is
@@ -333,6 +333,12 @@ 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 so the panel does not look ready and then dump a raw auth error.
+  if (model.hasLogin === false) {
+    return { ready: false, reason: 'no-provider-login', model };
+  }
+
  // 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 +398,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 +507,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;
  };

@@ -0,0 +1,151 @@
+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');
+    expect(result.readiness.model).toMatchObject({
+      providerID: 'deepseek',
+      modelID: 'deepseek-v4-flash',
+      hasLogin: false,
+    });
+  });
+
+  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));
+    }
+  });
+});