feat(core): provider-aware errors + named-step logging in generate

apps/desktop/src/main/index.ts

@@ -1,7 +1,7 @@
 import { stat } from 'node:fs/promises';
 import { basename, dirname, join } from 'node:path';
 import { fileURLToPath } from 'node:url';
-import { applyComment, generate } from '@open-codesign/core';
+import { type CoreLogger, applyComment, generate } from '@open-codesign/core';
 import { detectProviderFromKey } from '@open-codesign/providers';
 import {
   ApplyCommentPayload,
@@ -10,6 +10,7 @@ import {
   CodesignError,
   GeneratePayload,
   GeneratePayloadV1,
+  isSupportedOnboardingProvider,
 } from '@open-codesign/shared';
 import type { BrowserWindow as ElectronBrowserWindow } from 'electron';
 import { autoUpdater } from 'electron-updater';
@@ -69,6 +70,14 @@ function createWindow(): void {
 function registerIpcHandlers(): void {
   const logIpc = getLogger('main:ipc');
 
+  /** Adapter so `core` can log step events through the same scoped electron-log
+   * sink the IPC handler uses. Keeps a single timeline per generation in the
+   * log file without forcing `core` to depend on electron-log. */
+  const coreLoggerFor = (id: string): CoreLogger => ({
+    info: (event, data) => logIpc.info(event, { id, ...(data ?? {}) }),
+    error: (event, data) => logIpc.error(event, { id, ...(data ?? {}) }),
+  });
+
   /** In-flight requests: generationId → AbortController */
   const inFlight = new Map<string, AbortController>();
 
@@ -134,11 +143,43 @@ function registerIpcHandlers(): void {
     const controller = new AbortController();
     const id = payload.generationId;
     inFlight.set(id, controller);
+    const coreLogger = coreLoggerFor(id);
+    const stepCtx = { id, provider: payload.model.provider, modelId: payload.model.modelId };
 
+    coreLogger.info('[generate] step=load_config');
+    const loadStart = Date.now();
     const apiKey = getApiKeyForProvider(payload.model.provider);
     const storedBaseUrl = getBaseUrlForProvider(payload.model.provider);
     const baseUrl = payload.baseUrl ?? storedBaseUrl;
     const cfg = getCachedConfig();
+    coreLogger.info('[generate] step=load_config.ok', {
+      ms: Date.now() - loadStart,
+      hasApiKey: apiKey.length > 0,
+      baseUrl: baseUrl ?? '<default>',
+    });
+
+    coreLogger.info('[generate] step=validate_provider', stepCtx);
+    if (apiKey.length === 0) {
+      coreLogger.error('[generate] step=validate_provider.fail', {
+        provider: payload.model.provider,
+        reason: 'missing_api_key',
+      });
+      inFlight.delete(id);
+      throw new CodesignError(
+        `No API key configured for provider "${payload.model.provider}". Open Settings to add one.`,
+        'PROVIDER_AUTH_MISSING',
+      );
+    }
+    if (!isSupportedOnboardingProvider(payload.model.provider)) {
+      // Non-shortlist providers still work (any ProviderId is accepted by the
+      // payload schema), just warn so the log timeline shows we noticed.
+      coreLogger.info('[generate] step=validate_provider.warn', {
+        provider: payload.model.provider,
+        reason: 'not_in_onboarding_shortlist',
+      });
+    }
+    coreLogger.info('[generate] step=validate_provider.ok', { provider: payload.model.provider });
+
     const promptContext = await preparePromptContext({
       attachments: payload.attachments,
       referenceUrl: payload.referenceUrl,
@@ -169,6 +210,7 @@ function registerIpcHandlers(): void {
         designSystem: promptContext.designSystem ?? null,
         ...(baseUrl !== undefined ? { baseUrl } : {}),
         signal: controller.signal,
+        logger: coreLogger,
       });
       logIpc.info('generate.ok', {
         id,

packages/core/src/errors.test.ts

@@ -0,0 +1,95 @@
+import { CodesignError } from '@open-codesign/shared';
+import { describe, expect, it } from 'vitest';
+import { remapProviderError, rewriteUpstreamMessage } from './errors';
+
+const LEAKED =
+  'Incorrect API key provided: sk-AAA. You can find your API key at https://platform.openai.com/account/api-keys.';
+
+function httpError(status: number, message: string): Error & { status: number } {
+  const err = new Error(message) as Error & { status: number };
+  err.status = status;
+  return err;
+}
+
+describe('rewriteUpstreamMessage', () => {
+  it('keeps openai URL when active provider is openai', () => {
+    const result = rewriteUpstreamMessage(LEAKED, 'openai', 401);
+    expect(result.rewritten).toBe(false);
+    expect(result.message).toContain('platform.openai.com/account/api-keys');
+  });
+
+  it('rewrites leaked openai URL to anthropic billing URL', () => {
+    const result = rewriteUpstreamMessage(LEAKED, 'anthropic', 401);
+    expect(result.rewritten).toBe(true);
+    expect(result.message).not.toContain('openai.com');
+    expect(result.message).toContain('console.anthropic.com/settings/keys');
+  });
+
+  it('rewrites leaked openai URL to openrouter URL', () => {
+    const result = rewriteUpstreamMessage(LEAKED, 'openrouter', 401);
+    expect(result.message).toContain('openrouter.ai/settings/keys');
+  });
+
+  it('rewrites to deepseek URL even though it is not in the typed enum', () => {
+    const result = rewriteUpstreamMessage(LEAKED, 'deepseek', 401);
+    expect(result.message).toContain('platform.deepseek.com/api_keys');
+  });
+
+  it('strips URL and adds generic hint for unknown providers', () => {
+    const result = rewriteUpstreamMessage(LEAKED, 'mystery-llm', 401);
+    expect(result.rewritten).toBe(true);
+    expect(result.message).not.toContain('openai.com');
+    expect(result.message).toContain("Check your provider's API key settings");
+  });
+
+  it('does not rewrite 5xx errors', () => {
+    const result = rewriteUpstreamMessage(LEAKED, 'anthropic', 503);
+    expect(result.rewritten).toBe(false);
+  });
+
+  it('does not rewrite when no openai URL is present', () => {
+    const result = rewriteUpstreamMessage('Bad request: model not found', 'anthropic', 400);
+    expect(result.rewritten).toBe(false);
+  });
+});
+
+describe('remapProviderError', () => {
+  it('passes openai 401 through verbatim', () => {
+    const err = httpError(401, LEAKED);
+    const out = remapProviderError(err, 'openai');
+    expect(out).toBe(err);
+  });
+
+  it('rewrites anthropic 401 with leaked openai URL into a CodesignError', () => {
+    const err = httpError(401, LEAKED);
+    const out = remapProviderError(err, 'anthropic');
+    expect(out).toBeInstanceOf(CodesignError);
+    expect((out as CodesignError).message).toContain('console.anthropic.com/settings/keys');
+    expect((out as CodesignError).message).not.toContain('openai.com');
+    expect((out as CodesignError).code).toBe('PROVIDER_HTTP_4XX');
+  });
+
+  it('strips the URL when provider is unknown', () => {
+    const err = httpError(401, LEAKED);
+    const out = remapProviderError(err, 'mystery-llm');
+    expect(out).toBeInstanceOf(CodesignError);
+    expect((out as CodesignError).message).not.toContain('openai.com');
+    expect((out as CodesignError).message).toContain("Check your provider's API key settings");
+  });
+
+  it('passes 5xx errors through unchanged', () => {
+    const err = httpError(503, 'upstream unavailable');
+    const out = remapProviderError(err, 'anthropic');
+    expect(out).toBe(err);
+  });
+
+  it('extracts status code from CodesignError messages that embed it', () => {
+    const err = new CodesignError(
+      'HTTP 401 — see https://platform.openai.com/account/api-keys',
+      'PROVIDER_ERROR',
+    );
+    const out = remapProviderError(err, 'anthropic');
+    expect(out).toBeInstanceOf(CodesignError);
+    expect((out as CodesignError).message).toContain('console.anthropic.com/settings/keys');
+  });
+});

packages/core/src/errors.ts

@@ -0,0 +1,110 @@
+/**
+ * Provider-aware error remapping.
+ *
+ * Upstream SDKs (pi-ai, OpenAI client, etc.) embed an OpenAI key-help URL in
+ * 4xx error messages — even when the *active* provider is something else
+ * (e.g. DeepSeek behind an OpenAI-compatible base URL, or OpenRouter). That
+ * URL is misleading: clicking it sends the user to the wrong dashboard.
+ *
+ * We rewrite leaked openai.com URLs in 4xx upstream messages to the active
+ * provider's billing/key-help URL. Unknown providers get the URL stripped and
+ * a generic hint appended.
+ *
+ * This runs only on 4xx errors. 5xx and network errors pass through verbatim
+ * because their messages are usually already provider-neutral and the retry
+ * layer logs them with `reason`.
+ */
+
+import type { ProviderId } from '@open-codesign/shared';
+import { CodesignError } from '@open-codesign/shared';
+
+export const PROVIDER_KEY_HELP_URL: Partial<Record<ProviderId, string>> = {
+  openai: 'https://platform.openai.com/account/api-keys',
+  anthropic: 'https://console.anthropic.com/settings/keys',
+  openrouter: 'https://openrouter.ai/settings/keys',
+  google: 'https://aistudio.google.com/app/apikey',
+};
+
+// deepseek is not in the ProviderId enum yet but commonly used via openai-
+// compatible base URL — keyed by string so callers passing a free-form string
+// still get the rewrite. Kept separate so the typed map stays exhaustive-checked.
+const EXTRA_KEY_HELP_URL: Record<string, string> = {
+  deepseek: 'https://platform.deepseek.com/api_keys',
+};
+
+const OPENAI_URL_PATTERN = /https?:\/\/(?:platform\.openai\.com|openai\.com)\/[^\s)<>"']*/gi;
+const GENERIC_HINT = "Check your provider's API key settings.";
+
+function statusFromError(err: unknown): number | undefined {
+  if (typeof err !== 'object' || err === null) return undefined;
+  const candidates = [
+    (err as { status?: unknown }).status,
+    (err as { statusCode?: unknown }).statusCode,
+    (err as { response?: { status?: unknown } }).response?.status,
+  ];
+  for (const c of candidates) {
+    if (typeof c === 'number' && Number.isFinite(c)) return c;
+  }
+  if (err instanceof Error) {
+    const m = /\b(\d{3})\b/.exec(err.message);
+    if (m?.[1]) {
+      const n = Number(m[1]);
+      if (n >= 400 && n < 600) return n;
+    }
+  }
+  return undefined;
+}
+
+function lookupKeyHelpUrl(provider: string | undefined): string | undefined {
+  if (!provider) return undefined;
+  const typed = PROVIDER_KEY_HELP_URL[provider as ProviderId];
+  if (typed) return typed;
+  return EXTRA_KEY_HELP_URL[provider];
+}
+
+export interface RewriteResult {
+  message: string;
+  rewritten: boolean;
+  status?: number;
+}
+
+export function rewriteUpstreamMessage(
+  rawMessage: string,
+  provider: string | undefined,
+  status: number | undefined,
+): RewriteResult {
+  const result: RewriteResult = { message: rawMessage, rewritten: false };
+  if (status !== undefined) result.status = status;
+  if (status === undefined || status < 400 || status >= 500) return result;
+  if (!OPENAI_URL_PATTERN.test(rawMessage)) {
+    OPENAI_URL_PATTERN.lastIndex = 0;
+    return result;
+  }
+  OPENAI_URL_PATTERN.lastIndex = 0;
+  const target = lookupKeyHelpUrl(provider);
+  if (provider === 'openai') return result;
+  if (target) {
+    result.message = rawMessage.replace(OPENAI_URL_PATTERN, target);
+  } else {
+    const stripped = rawMessage.replace(OPENAI_URL_PATTERN, '').replace(/\s+/g, ' ').trim();
+    result.message = `${stripped} ${GENERIC_HINT}`.trim();
+  }
+  result.rewritten = true;
+  return result;
+}
+
+/**
+ * Wrap an upstream error so its message is safe to surface to the user. Only
+ * 4xx errors are rewritten — everything else is rethrown unchanged so the
+ * retry/network layer keeps its own taxonomy.
+ */
+export function remapProviderError(err: unknown, provider: string | undefined): unknown {
+  if (!(err instanceof Error)) return err;
+  if (err instanceof CodesignError && err.code === 'PROVIDER_ABORTED') return err;
+  const status = statusFromError(err);
+  if (status === undefined || status < 400 || status >= 500) return err;
+  const { message, rewritten } = rewriteUpstreamMessage(err.message, provider, status);
+  if (!rewritten) return err;
+  const code = err instanceof CodesignError ? err.code : 'PROVIDER_HTTP_4XX';
+  return new CodesignError(message, code, { cause: err });
+}