diff --git a/packages/opencode/src/account/index.ts b/packages/opencode/src/account/index.ts index a1bb614ce41f..bc2c7a799994 100644 --- a/packages/opencode/src/account/index.ts +++ b/packages/opencode/src/account/index.ts @@ -1,9 +1,10 @@ import { Cache, Clock, Duration, Effect, Layer, Option, Schema, SchemaGetter, ServiceMap } from "effect" -import { FetchHttpClient, HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http" +import { FetchHttpClient, HttpClient, HttpClientError, HttpClientRequest, HttpClientResponse } from "effect/unstable/http" import { makeRuntime } from "@/effect/run-service" import { withTransientReadRetry } from "@/util/effect-http-client" import { AccountRepo, type AccountRow } from "./repo" +import { normalizeServerUrl } from "./url" import { type AccountError, AccessToken, @@ -12,6 +13,7 @@ import { Info, RefreshToken, AccountServiceError, + AccountTransportError, Login, Org, OrgID, @@ -30,6 +32,7 @@ export { type AccountError, AccountRepoError, AccountServiceError, + AccountTransportError, AccessToken, RefreshToken, DeviceCode, @@ -132,13 +135,30 @@ const isTokenFresh = (tokenExpiry: number | null, now: number) => const mapAccountServiceError = (message = "Account service operation failed") => - (effect: Effect.Effect): Effect.Effect => + (effect: Effect.Effect): Effect.Effect => effect.pipe( - Effect.mapError((cause) => - cause instanceof AccountServiceError ? cause : new AccountServiceError({ message, cause }), - ), + Effect.mapError((cause) => accountErrorFromCause(cause, message)), ) +const accountErrorFromCause = (cause: unknown, message: string): AccountError => { + if (cause instanceof AccountServiceError || cause instanceof AccountTransportError) { + return cause + } + + if (HttpClientError.isHttpClientError(cause)) { + switch (cause.reason._tag) { + case "TransportError": { + return AccountTransportError.fromHttpClientError(cause.reason) + } + default: { + return new AccountServiceError({ message, cause }) + } + } + } + + return new AccountServiceError({ message, cause }) +} + export namespace Account { export interface Interface { readonly active: () => Effect.Effect, AccountError> @@ -346,8 +366,9 @@ export namespace Account { }) const login = Effect.fn("Account.login")(function* (server: string) { + const normalizedServer = normalizeServerUrl(server) const response = yield* executeEffectOk( - HttpClientRequest.post(`${server}/auth/device/code`).pipe( + HttpClientRequest.post(`${normalizedServer}/auth/device/code`).pipe( HttpClientRequest.acceptJson, HttpClientRequest.schemaBodyJson(ClientId)(new ClientId({ client_id: clientId })), ), @@ -359,8 +380,8 @@ export namespace Account { return new Login({ code: parsed.device_code, user: parsed.user_code, - url: `${server}${parsed.verification_uri_complete}`, - server, + url: `${normalizedServer}${parsed.verification_uri_complete}`, + server: normalizedServer, expiry: parsed.expires_in, interval: parsed.interval, }) diff --git a/packages/opencode/src/account/repo.ts b/packages/opencode/src/account/repo.ts index d02cf1b637f8..4dbb9cab43d6 100644 --- a/packages/opencode/src/account/repo.ts +++ b/packages/opencode/src/account/repo.ts @@ -4,6 +4,7 @@ import { Effect, Layer, Option, Schema, ServiceMap } from "effect" import { Database } from "@/storage/db" import { AccountStateTable, AccountTable } from "./account.sql" import { AccessToken, AccountID, AccountRepoError, Info, OrgID, RefreshToken } from "./schema" +import { normalizeServerUrl } from "./url" export type AccountRow = (typeof AccountTable)["$inferSelect"] @@ -125,11 +126,13 @@ export class AccountRepo extends ServiceMap.Service tx((db) => { + const url = normalizeServerUrl(input.url) + db.insert(AccountTable) .values({ id: input.id, email: input.email, - url: input.url, + url, access_token: input.accessToken, refresh_token: input.refreshToken, token_expiry: input.expiry, @@ -138,7 +141,7 @@ export class AccountRepo extends ServiceMap.Service()("AccountTransportError", { + method: Schema.String, + url: Schema.String, + description: Schema.optional(Schema.String), + cause: Schema.optional(Schema.Defect), +}) { + static fromHttpClientError(error: HttpClientError.TransportError): AccountTransportError { + return new AccountTransportError({ + method: error.request.method, + url: error.request.url, + description: error.description, + cause: error.cause, + }) + } + + override get message(): string { + return [ + `Could not reach ${this.method} ${this.url}.`, + `This failed before the server returned an HTTP response.`, + this.description, + `Check your network, proxy, or VPN configuration and try again.`, + ] + .filter(Boolean) + .join("\n") + } +} + +export type AccountError = AccountRepoError | AccountServiceError | AccountTransportError export class Login extends Schema.Class("Login")({ code: DeviceCode, diff --git a/packages/opencode/src/account/url.ts b/packages/opencode/src/account/url.ts new file mode 100644 index 000000000000..36afd6d8e99b --- /dev/null +++ b/packages/opencode/src/account/url.ts @@ -0,0 +1,8 @@ +export const normalizeServerUrl = (input: string): string => { + const url = new URL(input) + url.search = "" + url.hash = "" + + const pathname = url.pathname.replace(/\/+$/, "") + return pathname.length === 0 ? url.origin : `${url.origin}${pathname}` +} diff --git a/packages/opencode/src/cli/error.ts b/packages/opencode/src/cli/error.ts index 52bad892eb82..cd67635e9ba8 100644 --- a/packages/opencode/src/cli/error.ts +++ b/packages/opencode/src/cli/error.ts @@ -1,3 +1,4 @@ +import { AccountServiceError, AccountTransportError } from "@/account" import { ConfigMarkdown } from "@/config/markdown" import { errorFormat } from "@/util/error" import { Config } from "../config/config" @@ -8,6 +9,9 @@ import { UI } from "./ui" export function FormatError(input: unknown) { if (MCP.Failed.isInstance(input)) return `MCP server "${input.data.name}" failed. Note, opencode does not support MCP authentication yet.` + if (input instanceof AccountTransportError || input instanceof AccountServiceError) { + return input.message + } if (Provider.ModelNotFoundError.isInstance(input)) { const { providerID, modelID, suggestions } = input.data return [ diff --git a/packages/opencode/test/account/repo.test.ts b/packages/opencode/test/account/repo.test.ts index 460c47443f3b..2f17d1b22fc0 100644 --- a/packages/opencode/test/account/repo.test.ts +++ b/packages/opencode/test/account/repo.test.ts @@ -56,6 +56,32 @@ it.live("persistAccount inserts and getRow retrieves", () => }), ) +it.live("persistAccount normalizes trailing slashes in stored server URLs", () => + Effect.gen(function* () { + const id = AccountID.make("user-1") + + yield* AccountRepo.use((r) => + r.persistAccount({ + id, + email: "test@example.com", + url: "https://control.example.com/", + accessToken: AccessToken.make("at_123"), + refreshToken: RefreshToken.make("rt_456"), + expiry: Date.now() + 3600_000, + orgID: Option.none(), + }), + ) + + const row = yield* AccountRepo.use((r) => r.getRow(id)) + const active = yield* AccountRepo.use((r) => r.active()) + const list = yield* AccountRepo.use((r) => r.list()) + + expect(Option.getOrThrow(row).url).toBe("https://control.example.com") + expect(Option.getOrThrow(active).url).toBe("https://control.example.com") + expect(list[0]?.url).toBe("https://control.example.com") + }), +) + it.live("persistAccount sets the active account and org", () => Effect.gen(function* () { const id1 = AccountID.make("user-1") diff --git a/packages/opencode/test/account/service.test.ts b/packages/opencode/test/account/service.test.ts index 85ab259f1da6..265fdbbe1ac5 100644 --- a/packages/opencode/test/account/service.test.ts +++ b/packages/opencode/test/account/service.test.ts @@ -1,10 +1,20 @@ import { expect } from "bun:test" import { Duration, Effect, Layer, Option, Schema } from "effect" -import { HttpClient, HttpClientResponse } from "effect/unstable/http" +import { HttpClient, HttpClientError, HttpClientResponse } from "effect/unstable/http" import { AccountRepo } from "../../src/account/repo" import { Account } from "../../src/account" -import { AccessToken, AccountID, DeviceCode, Login, Org, OrgID, RefreshToken, UserCode } from "../../src/account/schema" +import { + AccessToken, + AccountID, + AccountTransportError, + DeviceCode, + Login, + Org, + OrgID, + RefreshToken, + UserCode, +} from "../../src/account/schema" import { Database } from "../../src/storage/db" import { testEffect } from "../lib/effect" @@ -57,6 +67,57 @@ const deviceTokenClient = (body: unknown, status = 400) => const poll = (body: unknown, status = 400) => Account.Service.use((s) => s.poll(login())).pipe(Effect.provide(live(deviceTokenClient(body, status)))) +it.live("login normalizes trailing slashes in the provided server URL", () => + Effect.gen(function* () { + const seen: Array = [] + const client = HttpClient.make((req) => + Effect.gen(function* () { + seen.push(`${req.method} ${req.url}`) + + if (req.url === "https://one.example.com/auth/device/code") { + return json(req, { + device_code: "device-code", + user_code: "user-code", + verification_uri_complete: "/device?user_code=user-code", + expires_in: 600, + interval: 5, + }) + } + + return json(req, {}, 404) + }), + ) + + const result = yield* Account.Service.use((s) => s.login("https://one.example.com/")).pipe(Effect.provide(live(client))) + + expect(seen).toEqual(["POST https://one.example.com/auth/device/code"]) + expect(result.server).toBe("https://one.example.com") + expect(result.url).toBe("https://one.example.com/device?user_code=user-code") + }), +) + +it.live("login maps transport failures to account transport errors", () => + Effect.gen(function* () { + const client = HttpClient.make((req) => + Effect.fail( + new HttpClientError.HttpClientError({ + reason: new HttpClientError.TransportError({ request: req }), + }), + ), + ) + + const error = yield* Effect.flip( + Account.Service.use((s) => s.login("https://one.example.com")).pipe(Effect.provide(live(client))), + ) + + expect(error).toBeInstanceOf(AccountTransportError) + if (error instanceof AccountTransportError) { + expect(error.method).toBe("POST") + expect(error.url).toBe("https://one.example.com/auth/device/code") + } + }), +) + it.live("orgsByAccount groups orgs per account", () => Effect.gen(function* () { yield* AccountRepo.use((r) => diff --git a/packages/opencode/test/cli/error.test.ts b/packages/opencode/test/cli/error.test.ts new file mode 100644 index 000000000000..6af2633ce622 --- /dev/null +++ b/packages/opencode/test/cli/error.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, test } from "bun:test" +import { AccountTransportError } from "../../src/account/schema" +import { FormatError } from "../../src/cli/error" + +describe("cli.error", () => { + test("formats account transport errors clearly", () => { + const error = new AccountTransportError({ + method: "POST", + url: "https://console.opencode.ai/auth/device/code", + }) + + const formatted = FormatError(error) + + expect(formatted).toContain("Could not reach POST https://console.opencode.ai/auth/device/code.") + expect(formatted).toContain("This failed before the server returned an HTTP response.") + expect(formatted).toContain("Check your network, proxy, or VPN configuration and try again.") + }) +})