diff --git a/AGENTS.md b/AGENTS.md index 20e0bee..43638f7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -213,6 +213,8 @@ Analysis backend coordinates and the public gateway base URL (`gateway_url`) are - Tests that exercise real `login()` must mock `POST /codev-backend/config` if (and only if) the caller also calls `refreshCodevConfig`. 2. **`runUpload` retries once on a "refreshable" error.** `isRefreshableError` (in `src/lib/upload.ts`) is deliberately narrow: `Missing supabase_…` from the cache accessors, or HTTP `401`/`403` from any analysis backend or CoDev backend fetch. `5xx`, `404`, network errors, and timeouts are NOT retried — refreshing won't help and we'd amplify the outage. Per-file upload errors stay in `summary.errors` and don't trigger the pipeline-level retry. If you change `runAnalysisBackendUpload`'s shape, keep that boundary intact. +**A key refusal is not an outage.** The backend's `/auth/exchange` answers `403` with `code: "key_refused"` when the gateway's mint endpoint rejects the *user* (a 400/422 on the mint call — today an email domain outside the gateway's allowlist, e.g. a sub-domain of a listed one). Gateway 5xx and the backend's own credential failures stay `502`. `backend.ts#fetchApiKey` throws a `BackendError` carrying the backend's `reason` and `code`, and `isKeyRefusal` is what `FetchApiKey` branches on: a refusal renders the gateway's sentence with **no retry affordance** and Enter goes straight to the manual-credentials step, because nothing the user can do on this machine changes the answer. The `code` string is a wire contract with `codev-backend/src/litellm.ts` — rename it in both or the hub silently falls back to the generic retry prompt. `FetchApiKey` also switches its key listener off once it has succeeded or handed off (`isActive`): the parent keeps the Step mounted as history, and a live listener there used to answer every later Enter by calling `onFallback` again. + **The vendor name survives only on the wire.** The service is called the *analysis backend* in every comment, doc, identifier and user-visible string. Three things keep their historical `supabase` spelling because they are contracts CoDev doesn't own: the on-disk / `POST /config` keys `supabase_url` / `supabase_anon_key` (and the response's `supabaseUrl` / `supabaseAnonKey`, mapped onto `CodevConfig`'s `analysisBackendUrl` / `analysisBackendAnonKey` at the parse boundary in `backend.ts`), the backend route `POST /supabase/exchange`, and the `Missing supabase_…` text those accessors throw — which is exactly what `isRefreshableError` matches on. Renaming any of them silently stops parsing a live response, hits a 404, or breaks the retry trigger. ## TLS trust (corporate proxies) diff --git a/package.json b/package.json index b35d549..b859855 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "codev-ai", - "version": "0.6.3", + "version": "0.6.4", "description": "CoDev — AI Coding Agent Hub. Install, configure, and manage multiple AI coding agents.", "keywords": [ "ai", diff --git a/src/components/FetchApiKey.tsx b/src/components/FetchApiKey.tsx index 2bf8d14..592e9dd 100644 --- a/src/components/FetchApiKey.tsx +++ b/src/components/FetchApiKey.tsx @@ -2,7 +2,7 @@ import { Box, Text, useInput } from "ink"; import Spinner from "ink-spinner"; import { useEffect, useState } from "react"; import type { AuthData } from "@/lib/auth.js"; -import { fetchApiKey } from "@/lib/backend.js"; +import { fetchApiKey, isKeyRefusal } from "@/lib/backend.js"; import { BACKEND_URL } from "@/lib/const.js"; import { describeFailure } from "@/lib/doctor.js"; @@ -20,8 +20,19 @@ interface FetchApiKeyProps { export function FetchApiKey({ auth, onDone, onFallback }: FetchApiKeyProps) { const [pending, setPending] = useState(true); const [error, setError] = useState(null); + // The gateway declined this user outright (backend `key_refused`). Held + // apart from `error` because it gets no retry affordance: the answer will + // not change, and "Press Enter to retry" under it sent users looping on a + // failure only the gateway team can fix. + const [refused, setRefused] = useState(null); const [emptyCount, setEmptyCount] = useState(0); const [succeeded, setSucceeded] = useState(false); + // Set once this component has handed the flow to the manual-credentials + // step. Together with `succeeded` it switches the key listener off: the + // parent keeps this Step mounted as read-only history, and a listener left + // live here would keep answering every later Enter (model pick, smoke-test + // spinner) by calling onFallback again and yanking the wizard backwards. + const [handedOff, setHandedOff] = useState(false); const [attempt, setAttempt] = useState(0); // `attempt` is the retry trigger — bumping it re-runs the effect. @@ -42,6 +53,13 @@ export function FetchApiKey({ auth, onDone, onFallback }: FetchApiKeyProps) { }) .catch((err: Error) => { setPending(false); + if (isKeyRefusal(err)) { + // The backend's `reason` is the gateway's own sentence (which + // domains it accepts); the surrounding "Backend … failed (403)" + // wrapper is noise to the person reading it. + setRefused(err.reason); + return; + } // A transport failure here (proxy/TLS/DNS) gets the full diagnosis; // a backend HTTP error keeps its own already-precise message. setError( @@ -53,21 +71,33 @@ export function FetchApiKey({ auth, onDone, onFallback }: FetchApiKeyProps) { }); }, [auth.access_token, onDone, attempt]); - useInput((_input, key) => { - if (pending) return; - if (!key.return) return; - if (error) { - setAttempt((n) => n + 1); - return; - } - if (emptyCount === 1) { - setAttempt((n) => n + 1); - return; - } - if (emptyCount >= 2) { - onFallback(); - } - }); + const fallBack = () => { + setHandedOff(true); + onFallback(); + }; + + useInput( + (_input, key) => { + if (pending) return; + if (!key.return) return; + if (refused) { + fallBack(); + return; + } + if (error) { + setAttempt((n) => n + 1); + return; + } + if (emptyCount === 1) { + setAttempt((n) => n + 1); + return; + } + if (emptyCount >= 2) { + fallBack(); + } + }, + { isActive: !succeeded && !handedOff }, + ); return ( @@ -82,6 +112,21 @@ export function FetchApiKey({ auth, onDone, onFallback }: FetchApiKeyProps) { {succeeded && ( {"✓ API key obtained successfully."} )} + {refused && ( + <> + {`✗ ${refused}`} + + { + "Retrying won't change this — ask the gateway team to enable your account, or use an API key issued to you by hand." + } + + {!handedOff && ( + + {"Press Enter to enter your own API key, Ctrl-C to quit"} + + )} + + )} {error && ( <> {/* One-line reasons stay inline; a multi-line transport @@ -109,9 +154,11 @@ export function FetchApiKey({ auth, onDone, onFallback }: FetchApiKeyProps) { {"Gateway returned an empty API key again."} - - {"Press Enter to enter credentials manually, Ctrl-C to quit"} - + {!handedOff && ( + + {"Press Enter to enter credentials manually, Ctrl-C to quit"} + + )} )} diff --git a/src/lib/backend.ts b/src/lib/backend.ts index 905928e..7c1e7ff 100644 --- a/src/lib/backend.ts +++ b/src/lib/backend.ts @@ -26,6 +26,9 @@ interface ConfigResponse { interface ErrorResponse { error?: string; + // Machine-readable reason the backend attaches to some failures — see + // BackendError below. + code?: string; } const VALIDATE_TIMEOUT_MS = 5_000; @@ -48,6 +51,31 @@ export interface AnalysisBackendSession { }; } +// A non-2xx from the CoDev backend. Beyond the message every caller already +// prints, it keeps the backend's own `error` text (`reason`) and, when the +// backend sends one, its machine-readable `code` — so a component can branch +// on *what kind* of failure this was without pattern-matching prose. +export class BackendError extends Error { + constructor( + message: string, + public status: number, + public reason: string, + public code?: string, + ) { + super(message); + this.name = "BackendError"; + } +} + +// The gateway looked at this user and declined to issue a key — today, an +// email domain outside its allowlist. The backend answers 403 with +// `code: "key_refused"`. Nothing the user can do on this machine changes the +// answer: retrying is pointless, and the way forward is a key issued by hand +// (the manual-credentials path) or a fix on the gateway side. +export function isKeyRefusal(err: unknown): err is BackendError { + return err instanceof BackendError && err.code === "key_refused"; +} + export async function fetchApiKey(accessToken: string): Promise { const res = await loggedFetch( "backend.auth-exchange", @@ -62,7 +90,12 @@ export async function fetchApiKey(accessToken: string): Promise { if (!res.ok) { const body = (await res.json().catch(() => ({}))) as ErrorResponse; const reason = body.error || res.statusText; - throw new Error(`Backend /auth/exchange failed (${res.status}): ${reason}`); + throw new BackendError( + `Backend /auth/exchange failed (${res.status}): ${reason}`, + res.status, + reason, + body.code, + ); } const data = (await res.json()) as ExchangeResponse; diff --git a/tests/InstallApp.test.tsx b/tests/InstallApp.test.tsx index 8199dbe..9a04e36 100644 --- a/tests/InstallApp.test.tsx +++ b/tests/InstallApp.test.tsx @@ -702,6 +702,63 @@ describe("InstallApp fail-stop invariant", () => { }); }); + test("a gateway key refusal drops into manual creds and reaches the done screen", async () => { + stubExecFile(() => ({ stdout: "ok" })); + stubModels(); + vi.spyOn(auth, "login").mockResolvedValue(fakeAuth()); + const fetchApiKeySpy = vi + .spyOn(backend, "fetchApiKey") + .mockRejectedValue( + new backend.BackendError( + "Backend /auth/exchange failed (403): The gateway declined to issue a key for test@os.example.com: Only @example.com emails are supported", + 403, + "The gateway declined to issue a key for test@os.example.com: Only @example.com emails are supported", + "key_refused", + ), + ); + const configureSpy = vi + .spyOn(configure, "configureClaudeCode") + .mockReturnValue([ + { + kind: "claude-settings", + sourcePath: "/tmp/x", + backupPath: "/tmp/x.b", + created: true, + }, + ]); + + const { stdin, frames } = render(); + await advanceThroughConfirm(stdin, frames); + await pickNewKey(stdin, frames); + + await waitForFrame(frames, "Only @example.com emails are supported"); + expect(allFrames(frames)).toContain( + "Press Enter to enter your own API key", + ); + expect(allFrames(frames)).not.toContain("Press Enter to retry"); + + stdin.write("\r"); + await typeManualCreds( + stdin, + frames, + "https://byhand.example.com/v1", + "sk-byhand-123", + ); + await pickFirstModel(stdin, frames); + await waitForFrame(frames, "Happy coding"); + + expect(allFrames(frames)).toContain("Happy coding"); + expect(fetchApiKeySpy).toHaveBeenCalledTimes(1); + expect(configureSpy).toHaveBeenCalledTimes(1); + expect(configureSpy).toHaveBeenCalledWith( + expect.objectContaining({ + apiKey: "sk-byhand-123", + baseUrl: "https://byhand.example.com/v1", + model: "m-alpha", + }), + ); + }); + test("fetch-key retry after failure reaches the done screen", async () => { stubExecFile(() => ({ stdout: "ok" })); stubModels(); diff --git a/tests/components/FetchApiKey.test.tsx b/tests/components/FetchApiKey.test.tsx index fae1332..e53b60e 100644 --- a/tests/components/FetchApiKey.test.tsx +++ b/tests/components/FetchApiKey.test.tsx @@ -3,6 +3,7 @@ import { afterEach, describe, expect, test, vi } from "vitest"; import { FetchApiKey } from "@/components/FetchApiKey.js"; import type * as auth from "@/lib/auth.js"; import * as backend from "@/lib/backend.js"; +import { BackendError } from "@/lib/backend.js"; afterEach(() => { cleanup(); @@ -120,6 +121,75 @@ describe("FetchApiKey", () => { expect(onDone).not.toHaveBeenCalled(); }); + test("a gateway refusal shows the reason with no retry hint; Enter hands off to manual entry", async () => { + const fetchSpy = vi + .spyOn(backend, "fetchApiKey") + .mockRejectedValue( + new BackendError( + "Backend /auth/exchange failed (403): The gateway declined to issue a key for u@os.example.com: Only @example.com emails are supported", + 403, + "The gateway declined to issue a key for u@os.example.com: Only @example.com emails are supported", + "key_refused", + ), + ); + // This file never restores mocks, so the spy carries every earlier + // test's calls; clear before counting. + fetchSpy.mockClear(); + + const onDone = vi.fn(); + const onFallback = vi.fn(); + const { stdin, lastFrame } = render( + , + ); + + await new Promise((r) => setTimeout(r, 100)); + const output = lastFrame() ?? ""; + expect(output).toContain( + "The gateway declined to issue a key for u@os.example.com: Only @example.com emails are supported", + ); + // The user reads the gateway's sentence, not the HTTP wrapper around it. + expect(output).not.toContain("Backend /auth/exchange failed"); + expect(output).toContain("Retrying won't change this"); + expect(output).toContain("Press Enter to enter your own API key"); + expect(output).not.toContain("Press Enter to retry"); + + stdin.write("\r"); + await new Promise((r) => setTimeout(r, 100)); + expect(onFallback).toHaveBeenCalledTimes(1); + expect(onDone).not.toHaveBeenCalled(); + // Enter must not have re-fetched: the refusal is not retryable. + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + + test("stops listening once it has handed off, so later Enters don't re-fire onFallback", async () => { + vi.spyOn(backend, "fetchApiKey").mockResolvedValue(""); + + const onDone = vi.fn(); + const onFallback = vi.fn(); + const { stdin, lastFrame } = render( + , + ); + + await new Promise((r) => setTimeout(r, 100)); + stdin.write("\r"); // first empty -> retry + await new Promise((r) => setTimeout(r, 100)); + stdin.write("\r"); // second empty -> fallback + await new Promise((r) => setTimeout(r, 100)); + expect(onFallback).toHaveBeenCalledTimes(1); + + // The parent keeps this Step mounted as history while the next step + // (manual creds, model pick, smoke test) owns the keyboard. Every Enter + // the user presses there used to land here too. + stdin.write("\r"); + stdin.write("\r"); + await new Promise((r) => setTimeout(r, 100)); + expect(onFallback).toHaveBeenCalledTimes(1); + // And the prompt that invited the Enter is gone from the history frame. + expect(lastFrame() ?? "").not.toContain( + "Press Enter to enter credentials manually", + ); + }); + test("shows error and retry prompt on fetchApiKey rejection", async () => { vi.spyOn(backend, "fetchApiKey").mockRejectedValue( new Error("Backend /auth/exchange failed (502): boom"), diff --git a/tests/lib/backend.test.ts b/tests/lib/backend.test.ts index 5a9d9e3..f18a346 100644 --- a/tests/lib/backend.test.ts +++ b/tests/lib/backend.test.ts @@ -3,12 +3,14 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { + BackendError, fetchAnalysisBackendSession, fetchApiKey, fetchCodevConfig, fetchModels, fetchModelWindows, isInvalidKeyError, + isKeyRefusal, smokeTestModel, validateApiKey, } from "@/lib/backend.js"; @@ -88,6 +90,33 @@ describe("fetchApiKey", () => { ); }); + test("a 403 key_refused surfaces as a BackendError carrying the code and reason", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue( + jsonResponse(403, { + error: + "The gateway declined to issue a key for u@os.example.com: Only @example.com emails are supported", + code: "key_refused", + }), + ); + let caught: unknown; + try { + await fetchApiKey("token"); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(BackendError); + const err = caught as BackendError; + expect(err.status).toBe(403); + expect(err.code).toBe("key_refused"); + expect(err.reason).toBe( + "The gateway declined to issue a key for u@os.example.com: Only @example.com emails are supported", + ); + expect(isKeyRefusal(err)).toBe(true); + // A plain 502 is a BackendError too, just not a refusal. + expect(isKeyRefusal(new BackendError("x", 502, "x"))).toBe(false); + expect(isKeyRefusal(new Error("x"))).toBe(false); + }); + test("throws on a non-2xx response with no JSON body, using statusText", async () => { vi.spyOn(globalThis, "fetch").mockResolvedValue( new Response("not json", { status: 500, statusText: "Server Error" }),