diff --git a/CHANGELOG.md b/CHANGELOG.md index 15dd9e55b..a9f899fb9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,28 @@ file is the human-readable summary of what changed and why. ### Gateway +- **A stray request can no longer crash the app during sign-in** — while + **Login with SSO** waits for the browser, the app listens on a local port for + the callback. A request to that port carrying a malformed `Host` header (any + program on the machine can send one) threw inside the listener, and that + exception took down the whole process: the terminal app, or the desktop + app's main process. The callback path is now read without trusting the + header, and a request the parser still rejects gets a `400` instead. +- **The terminal app no longer quits over a failed sign-in when you already + have a provider** — a login that timed out (five minutes with the browser + tab left open) or a token exchange that failed once used to end the whole + app with a "setup failed" message, even from the startup key re-check or + `/configure` on a working install, where there was a perfectly good chat to + go back to. That exit is now reserved for the first-run gate, where there is + nothing to fall back to; with a provider connected the failure is a toast + and the dialog closes. `/configure` on a configured install also opens as a + dismissable dialog now, like the desktop app's **Get a new API Key**. +- **The desktop app stops asking non-gateway users to sign in on every + launch** — the startup key check treated "no gateway key stored" the same + as "the gateway rejected the key" and opened the configure dialog for both, + so anyone set up through **Custom provider** or **Connect provider** (which + never store a gateway key) got the dialog at each launch. Only a rejected + gateway key prompts now, matching the terminal app. - **A gateway that refuses your account says so, once** — when the CoDev gateway declines to issue a key for an account (today: an email domain outside its allowlist, such as a sub-domain of a listed one), **Login with diff --git a/DIVERGENCES.md b/DIVERGENCES.md index 2f65fbe0a..6c8662964 100644 --- a/DIVERGENCES.md +++ b/DIVERGENCES.md @@ -157,7 +157,7 @@ from the squash message first, or use a plain merge. | `packages/tui/src/feature-plugins/sidebar/footer.tsx` | Added the editor-context label above the `cwd:branch` line: `useEditorLabel()` (fork-only `src/context/editor-label.tsx`) supplies the effective selection, an `editorFile` memo formats it via `editorContextFileLabel(...)` at a fixed width of 36 (the sidebar's 42 columns less 2 of padding a side and 2 for the bullet), and `` renders it — both from the fork-only `./editor-context-label.tsx`. | The open file + selected range belong with the sidebar's other ambient session state rather than in the prompt footer's crowded keybind-hint row; they sit next to the cwd for a single "where am I working" group. Moved out of the prompt footer, which no longer shows it at all. | Keep the `useEditorLabel` import, the `editorFile` memo, and the `` element as the box's first child; take upstream's changes to the path display below it. | | `packages/tui/src/feature-plugins/sidebar/lsp.tsx` | Whole LSP section wrapped in `` so it renders nothing when `config.lsp` is falsy; the "LSPs are disabled" placeholder text removed (empty state is only "LSPs will activate as files are read") | A disabled feature shouldn't occupy sidebar space with a permanent placeholder. | Keep our `Show` wrapper + removed placeholder; take upstream's surrounding changes. | | `packages/opencode/src/cli/cmd/*.ts` (tui, run, serve, attach, pr, upgrade, uninstall, web) | Help `describe` strings "opencode" → "codev" | `--help` is the first thing users read. | Keep our strings. | -| `packages/tui/src/app.tsx` | Terminal title "OpenCode"/"OC \|" → "CoDev Code"/"CC \|"; the configure-gate effect is a stateless forced-setup invariant — while `sync.status === "complete" && !useConnected() && dialog.stack.length === 0` it (re)opens the fork's `DialogConfigure` (component/dialog-configure.tsx, new file), so escaping any gate-launched flow or a credential-only save bounces back to the gate instead of an unconfigured chat (gates on `!useConnected()`, not `provider.length === 0` — hosted Zen free models always register); added the `codev.configure` command (`/configure`) and a `CODEV_CONFIGURE=1` mount hook for `codev configure`; home screen skipped — plain startup seeds the `dummy` session route and auto-opens a new empty session (reusing a leftover default-titled empty one via `openNewSession`/`findEmptySession`), `session.new` and the current-session `session.deleted` handler open a new session instead of navigating home, and the `-c` resolver prefers non-default-titled sessions so it lands on the last real conversation; added the `app.exit.confirm` command + `EXIT_CONFIRM_WINDOW` const (double-press Ctrl+C to quit: first press shows a toast, second within 1.5 s exits) and gathered its `app_exit_confirm` binding alongside `app_exit`; an onMount "Session upload is paused" warning toast next to the raster-font one, shown when `@codev/upload/nudge`'s `shouldNudgeSignIn()` says the background upload stalled for lack of a CoDev SSO session (SSO-origin keys only; deliberately every launch until the sign-in happens — see the session-upload wiring row); a one-shot `createEffect` after `sync.status === "complete"` runs `@codev/gateway`'s `ensureGatewayKey()` (the startup gateway-key pre-flight shared with the desktop controller): a `refreshed` result merges the re-minted key into the auth registry for every id it names via `sdk.client.auth.set`, then `instance.dispose()` + `sync.bootstrap()`; a `needs-login` with reason `rejected` opens `` when no dialog is up (`no-key` is ignored — a non-gateway setup, or the gate's own case) | Tab title branding; first-run setup is a forced gate the user can't escape into an unconfigured chat (the invariant reopens it); `codev` starts directly in the session layout instead of the home screen; a stray Ctrl+C no longer quits on the first press. | Keep our strings, the invariant gate effect, dialog swap + command block, route seed, `openNewSession`/`findEmptySession` + call sites, the `-c` default-title filter, and the `app.exit.confirm` command + binding; take upstream's surrounding changes. | +| `packages/tui/src/app.tsx` | Terminal title "OpenCode"/"OC \|" → "CoDev Code"/"CC \|"; the configure-gate effect is a stateless forced-setup invariant — while `sync.status === "complete" && !useConnected() && dialog.stack.length === 0` it (re)opens the fork's `DialogConfigure` (component/dialog-configure.tsx, new file), so escaping any gate-launched flow or a credential-only save bounces back to the gate instead of an unconfigured chat (gates on `!useConnected()`, not `provider.length === 0` — hosted Zen free models always register); added the `codev.configure` command (`/configure`, opening `` — dismissable on a configured install, the gate otherwise) and a `CODEV_CONFIGURE=1` mount hook for `codev configure`; home screen skipped — plain startup seeds the `dummy` session route and auto-opens a new empty session (reusing a leftover default-titled empty one via `openNewSession`/`findEmptySession`), `session.new` and the current-session `session.deleted` handler open a new session instead of navigating home, and the `-c` resolver prefers non-default-titled sessions so it lands on the last real conversation; added the `app.exit.confirm` command + `EXIT_CONFIRM_WINDOW` const (double-press Ctrl+C to quit: first press shows a toast, second within 1.5 s exits) and gathered its `app_exit_confirm` binding alongside `app_exit`; an onMount "Session upload is paused" warning toast next to the raster-font one, shown when `@codev/upload/nudge`'s `shouldNudgeSignIn()` says the background upload stalled for lack of a CoDev SSO session (SSO-origin keys only; deliberately every launch until the sign-in happens — see the session-upload wiring row); a one-shot `createEffect` after `sync.status === "complete"` runs `@codev/gateway`'s `ensureGatewayKey()` (the startup gateway-key pre-flight shared with the desktop controller): a `refreshed` result merges the re-minted key into the auth registry for every id it names via `sdk.client.auth.set`, then `instance.dispose()` + `sync.bootstrap()`; a `needs-login` with reason `rejected` opens `` when no dialog is up (`no-key` is ignored — a non-gateway setup, or the gate's own case) | Tab title branding; first-run setup is a forced gate the user can't escape into an unconfigured chat (the invariant reopens it); `codev` starts directly in the session layout instead of the home screen; a stray Ctrl+C no longer quits on the first press. | Keep our strings, the invariant gate effect, dialog swap + command block, route seed, `openNewSession`/`findEmptySession` + call sites, the `-c` default-title filter, and the `app.exit.confirm` command + binding; take upstream's surrounding changes. | | `packages/tui/bunfig.toml` | `[test] preload` gains the fork-only `./test/preload.ts`, which sets `CODEV_TEST_HOME` to a fresh temp dir (unless already set) before any test runs | The app mount runs the gateway key pre-flight (`ensureGatewayKey`) and the upload sign-in nudge at startup; both read the shared auth file under `~/.codev`/`~/.codev-hub`, and the pre-flight can write a re-minted key back. View tests boot the real app, so without an isolated home a developer's test run would probe the real gateway with their real key. Same convention as core's `Global` and `packages/opencode/test/preload.ts`. | Keep our preload entry; take upstream's other bunfig changes. | | `packages/tui/src/routes/session/index.tsx` | Flow transcript: the scrollbox opens with a home-like banner (`Logo` + `home_bottom` tips slot, `placement="start"`) as its first entry for root sessions (subagent sessions keep a plain 1-row pad), its `maxHeight` is clamped to the content height (a `transcriptHeight` signal read off the content's yoga node in the renderer root's `layout-changed` handler, which then re-runs `calculateLayout` so the clamp lands in the same frame — `scrollHeight` is a frame stale there, and painting with a stale clamp flickers during streaming) so the `Prompt` rides directly below the last message, and a trailing flexGrow spacer absorbs the leftover space until content fills the screen (then the prompt is effectively bottom-anchored and the banner scrolls away with history); the session `Prompt` gets home's `placeholder` pool when the session has no messages so the input shows an `Ask anything... ""` placeholder like home | Startup lands in an empty session instead of the home screen; the transcript flows top-down like terminal scrollback — logo top-left, input following the conversation down — instead of a bottom-anchored prompt with a void above it. | Keep our banner block inside the scrollbox, the `transcriptHeight` clamp + `layout-changed` listener, the trailing spacer, and the `placeholders` prop; take upstream's surrounding changes. | | `packages/tui/src/routes/session/index.tsx` | Sidebar visibility is session-local: removed upstream's persisted `kv.signal<"auto" \| "hide">("sidebar", "auto")` (and the `sidebar() === "auto" && wide()` branch of `sidebarVisible`); visibility is driven only by the non-persisted `sidebarOpen` signal, which the `session.sidebar.toggle` command flips (supersedes the earlier `"auto"` → `"hide"` default change) | The right sidebar starts hidden in every session regardless of terminal width or what the user did last session; `ctrl+b`/`b` only affects the current session. A stale `sidebar` key in existing kv.json files is simply never read. | Keep the kv signal removed and `sidebarVisible` reading only `parentID` + `sidebarOpen`; take upstream's surrounding changes. | diff --git a/packages/app/src/components/gateway/gateway-configure-gate.tsx b/packages/app/src/components/gateway/gateway-configure-gate.tsx index d81d3ffe6..54d45fae0 100644 --- a/packages/app/src/components/gateway/gateway-configure-gate.tsx +++ b/packages/app/src/components/gateway/gateway-configure-gate.tsx @@ -164,6 +164,12 @@ export function GatewayConfigureGate() { .catch(() => undefined) return } + // "no-key" is a machine set up with some other provider (the Custom and + // Connect provider paths never write the shared auth file) or not at all — + // the gate above owns that case. Prompting for it here asked every + // non-gateway user to sign in on each launch; only a rejected gateway key + // is worth interrupting for, same as the TUI. + if (result.reason !== "rejected") return if (dialog.active !== undefined) return gateDialogOpen = true // Dismissable: unlike the gate above, a provider *is* configured here — its diff --git a/packages/app/src/gateway/types.ts b/packages/app/src/gateway/types.ts index 038521cf7..67fd4a1f1 100644 --- a/packages/app/src/gateway/types.ts +++ b/packages/app/src/gateway/types.ts @@ -57,8 +57,11 @@ export type GatewayKeyCheck = // pre-rename spellings, so a config block written before an identity rename // is re-keyed too instead of 401ing on a credential nothing updates. | { status: "refreshed"; providerIDs: string[]; apiKey: string } - // No key, or one that cannot be replaced without a browser sign-in. - | { status: "needs-login" } + // "no-key": nothing stored — a machine set up with a non-gateway provider + // (the Custom and Connect provider paths never write the shared auth file), + // or never set up. "rejected": the gateway refused the stored key and the + // SSO session could not re-mint it without a browser sign-in. + | { status: "needs-login"; reason: "no-key" | "rejected" } // The signed-in identity from the shared auth file — present whenever an SSO // session (live or refresh-revivable) is on disk, independent of the login diff --git a/packages/codev-gateway/src/sso.ts b/packages/codev-gateway/src/sso.ts index 2b8043209..10605899e 100644 --- a/packages/codev-gateway/src/sso.ts +++ b/packages/codev-gateway/src/sso.ts @@ -423,8 +423,18 @@ function getAuthCode( } const server = createServer((req, res) => { - const host = req.headers.host ?? "127.0.0.1" - const url = new URL(req.url ?? "/", `http://${host}`) + // Parsed against a fixed base, never the Host header: the server is + // bound to 127.0.0.1 and only the path and query matter. Building the + // base from Host used to throw on a malformed value (any local process + // can send `Host: [`), and an exception in the request listener is an + // uncaught exception that took the whole process down for the length of + // the sign-in wait. A target the parser still rejects is answered. + const url = URL.parse(req.url ?? "/", "http://127.0.0.1") + if (!url) { + res.writeHead(400, { "Content-Type": "text/plain" }) + res.end("Bad request") + return + } // force-login only: the IdP has just killed its session cookie and // redirected back here — bounce on to /authorize so it shows the diff --git a/packages/codev-gateway/test/sso.test.ts b/packages/codev-gateway/test/sso.test.ts index ca5b77058..a570cc0fe 100644 --- a/packages/codev-gateway/test/sso.test.ts +++ b/packages/codev-gateway/test/sso.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test" import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs" +import { connect } from "node:net" import { tmpdir } from "node:os" import { join } from "node:path" import { isLoginCancelled, login, logout, parseCallbackParams, probeSession, silentSso } from "../src/sso" @@ -217,6 +218,38 @@ describe("login", () => { expect(existsSync(authFilePath())).toBe(false) }) + // Any local process can reach the loopback port during the sign-in wait, + // and a request with a Host header that is not a valid URL authority used to + // throw inside the request listener — an uncaught exception that took the + // whole process down. + test("a malformed Host header is answered instead of crashing the process", async () => { + const abort = new AbortController() + const ready = new Promise((resolve) => { + void login({ + onLog: () => {}, + onReady: (open) => open(), + openUrl: async (url) => resolve(url), + fetchImpl: noFetch, + signal: abort.signal, + }).catch(() => {}) + }) + const port = Number(new URL(new URL(await ready).searchParams.get("redirect_uri")!).port) + const status = await new Promise((resolve, reject) => { + const socket = connect(port, "127.0.0.1", () => { + socket.write("GET /nope HTTP/1.1\r\nHost: [\r\nConnection: close\r\n\r\n") + }) + socket.once("data", (data) => { + resolve(data.toString().split("\r\n")[0]) + socket.destroy() + }) + socket.once("error", reject) + }) + expect(status).toBe("HTTP/1.1 404 Not Found") + // The login is still waiting on the real callback. + expect(await loopbackAlive(String(port))).toBe(true) + abort.abort() + }) + test("an already-aborted signal never opens a server", async () => { const abort = new AbortController() abort.abort() diff --git a/packages/desktop/src/main/gateway-sso/controller.test.ts b/packages/desktop/src/main/gateway-sso/controller.test.ts index 7f4b1ff54..4041c8a93 100644 --- a/packages/desktop/src/main/gateway-sso/controller.test.ts +++ b/packages/desktop/src/main/gateway-sso/controller.test.ts @@ -443,13 +443,16 @@ describe("gateway sso controller", () => { silentSso: async () => null, }) - expect(await controller.ensureKey()).toEqual({ status: "needs-login" }) + expect(await controller.ensureKey()).toEqual({ status: "needs-login", reason: "rejected" }) }) - test("asks for a login when there is no key at all", async () => { + // The reason travels to the renderer, whose gate only prompts for a + // rejected key: a desktop set up with some other provider must not be + // asked to sign in on every launch. + test("reports a missing key as such, not as a rejection", async () => { const controller = controllerWith({ loadApiKey: () => null }) - expect(await controller.ensureKey()).toEqual({ status: "needs-login" }) + expect(await controller.ensureKey()).toEqual({ status: "needs-login", reason: "no-key" }) }) // SSO-minted keys record no base_url, so the saved gateway URL is the only diff --git a/packages/desktop/src/main/gateway-sso/controller.ts b/packages/desktop/src/main/gateway-sso/controller.ts index a8811cd84..d2f4ff919 100644 --- a/packages/desktop/src/main/gateway-sso/controller.ts +++ b/packages/desktop/src/main/gateway-sso/controller.ts @@ -310,16 +310,15 @@ export function createGatewaySsoController(deps: Deps) { // cannot be refreshed without a browser round-trip asks for one. async ensureKey(): Promise { // The check itself is shared with the TUI (@codev/gateway ensureGatewayKey); - // this wrapper adopts a refreshed session into the controller and maps - // the result onto the wire type. Both needs-login reasons prompt here, - // as before: a desktop set up without a gateway key is still asked to - // sign in. + // this wrapper adopts a refreshed session into the controller and passes + // the rest through, needs-login reason included: the renderer's gate + // prompts for a rejected key only, like the TUI. const result = await ensureGatewayKey({ fetchImpl: deps.fetchImpl, timeoutMs: KEY_CHECK_TIMEOUT_MS, fns: gateway, }) - if (result.status !== "refreshed") return { status: result.status } + if (result.status !== "refreshed") return result session = result.auth apiKey = result.apiKey gatewayUrl = result.baseUrl diff --git a/packages/tui/src/app.tsx b/packages/tui/src/app.tsx index df548e900..236d1c603 100644 --- a/packages/tui/src/app.tsx +++ b/packages/tui/src/app.tsx @@ -954,12 +954,14 @@ function App(props: { onSnapshot?: () => Promise; pluginHost: TuiPlugi category: "Provider", }, // Fork: the CoDev gateway configure flow (SSO or own API key). + // Dismissable once a provider is connected: opened on purpose from a + // working chat, it needs a way back; unconfigured, it is the gate. { name: "codev.configure", title: "Configure CoDev gateway", slashName: "configure", run: () => { - dialog.replace(() => ) + dialog.replace(() => ) }, category: "Provider", }, diff --git a/packages/tui/src/component/dialog-configure.tsx b/packages/tui/src/component/dialog-configure.tsx index 3b9485ad0..456f17eae 100644 --- a/packages/tui/src/component/dialog-configure.tsx +++ b/packages/tui/src/component/dialog-configure.tsx @@ -22,6 +22,7 @@ import { useEpilogue } from "../context/epilogue" import { useLocal } from "../context/local" import { runCustomProviderFlow, RETRIES, StatusBox, withRetries } from "./dialog-custom-provider" import { DialogProvider } from "./dialog-provider" +import { useConnected } from "./use-connected" import { useExit } from "../context/exit" import { useSync } from "../context/sync" import { useSDK } from "../context/sdk" @@ -55,6 +56,9 @@ interface FlowContext { exit: ReturnType epilogue: ReturnType local: ReturnType + // Whether a provider is already connected — decides what a failed flow + // falls back to (see failSetup). + connected: Accessor } // `dismissable` is for the startup key pre-flight in app.tsx: a provider IS @@ -69,6 +73,7 @@ export function DialogConfigure(props: { dismissable?: boolean } = {}) { exit: useExit(), epilogue: useEpilogue(), local: useLocal(), + connected: useConnected(), } // The entry chooser is a non-dismissable setup gate: no escape, no @@ -135,10 +140,25 @@ function showSelect( }) } -// The configure flow is a forced setup gate, so an unrecoverable failure must -// not silently drop the user into an unconfigured chat. Print the reason as an -// exit epilogue (shown after the TUI tears down) and quit the app. -function failAndQuit(ctx: FlowContext, message: string) { +// On a fresh install the configure flow is a forced setup gate, so an +// unrecoverable failure must not silently drop the user into an unconfigured +// chat: print the reason as an exit epilogue (shown after the TUI tears down) +// and quit the app. With a provider already connected — the startup key +// re-check, /configure on a working install — there is a chat to go back to, +// and quitting an app the user was in the middle of using over a login timeout +// or one failed token exchange is worse than the failure itself: show the +// reason as a toast and close the dialog instead. +function failSetup(ctx: FlowContext, message: string) { + if (ctx.connected()) { + ctx.toast.show({ + variant: "error", + title: "CoDev Code setup failed", + message: `${message}. Run /configure to try again.`, + duration: 10000, + }) + ctx.dialog.clear() + return + } ctx.epilogue(`CoDev Code setup failed: ${message}\nRun \`codev configure\` to try again.`) ctx.exit() } @@ -210,7 +230,7 @@ async function runSsoFlow(ctx: FlowContext) { // A refusal — the gateway declined this account, e.g. an email domain // outside its allowlist — is neither retryable nor a reason to quit: the // manual path below works with a key issued by hand, so it lands there - // with the gateway's own reason instead of in failAndQuit's generic + // with the gateway's own reason instead of in failSetup's generic // "setup failed". Same handling as codev-hub's install step. const minted = await withRetries("Fetching gateway API key", setStatus, () => fetchApiKey(session.access_token), @@ -247,9 +267,9 @@ async function runSsoFlow(ctx: FlowContext) { if (isLoginCancelled(err)) return // login() is single-attempt (its auth code is single-use, so a silent // retry can't reuse it); the retryable backend steps have already spent - // their budget by the time they throw here. Either way, quit rather than - // fall through to chat. - failAndQuit(ctx, err instanceof Error ? err.message : String(err)) + // their budget by the time they throw here. Either way, fail the flow + // rather than fall through to an unconfigured chat. + failSetup(ctx, err instanceof Error ? err.message : String(err)) } } @@ -343,7 +363,7 @@ async function runModelFlow(ctx: FlowContext, creds: GatewayCreds) { // cached, so the static per-model table stands in. cacheModelLimits(await fetchModelWindows(creds.apiKey, creds.gatewayBaseUrl)) } catch (err) { - failAndQuit( + failSetup( ctx, `couldn't fetch models after ${RETRIES} retries: ${err instanceof Error ? err.message : String(err)}`, ) @@ -406,6 +426,6 @@ async function runModelFlow(ctx: FlowContext, creds: GatewayCreds) { spawnUploadDaemon() ctx.dialog.clear() } catch (err) { - failAndQuit(ctx, err instanceof Error ? err.message : String(err)) + failSetup(ctx, err instanceof Error ? err.message : String(err)) } }