diff --git a/packages/core/src/plugin/provider/opencode.ts b/packages/core/src/plugin/provider/opencode.ts index 48f47caa3ad..4d5964cb0a1 100644 --- a/packages/core/src/plugin/provider/opencode.ts +++ b/packages/core/src/plugin/provider/opencode.ts @@ -1,4 +1,16 @@ -import { Duration, Effect, Equal, FiberHandle, Option, Schema, SchemaGetter, Scope, Semaphore, Stream } from "effect" +import { + Clock, + Duration, + Effect, + Equal, + FiberHandle, + Option, + Schema, + SchemaGetter, + Scope, + Semaphore, + Stream, +} from "effect" import type { IntegrationOAuthMethodRegistration } from "@opencode/plugin/effect/integration" import { define } from "@opencode/plugin/effect/plugin" import type { SessionHttpResponse } from "@opencode/plugin/effect/session" @@ -206,8 +218,11 @@ export const OpencodePlugin = define>[] = [] + // When the last Console config load started, so rejected inference can reuse a recent status. + let checked = Number.NEGATIVE_INFINITY const load = Effect.fn("OpencodePlugin.load")(function* () { + checked = yield* Clock.currentTimeMillis const connection = yield* ctx.integration.connection.active("opencode") if (!connection) { reported = undefined @@ -454,17 +469,24 @@ export const OpencodePlugin = define loading.withPermit(load().pipe(Effect.andThen(apply))) // Annotated: `apply` re-registers the hooks that call `check`, which would otherwise make inference circular. - const check = (): Effect.Effect => + // With `maxAge`, a load that started within it is reused; checking inside the permit lets concurrent callers + // share the load that was in flight when they arrived. + const check = (maxAge = Duration.zero): Effect.Effect => loading.withPermit( - load().pipe(Effect.flatMap((next) => (Equal.equals(snapshot, next) ? Effect.void : apply(next)))), + Effect.gen(function* () { + if ((yield* Clock.currentTimeMillis) - checked < Duration.toMillis(maxAge)) return + const next = yield* load() + if (!Equal.equals(snapshot, next)) yield* apply(next) + }), ) // Console inference rejects a missing SSO sign-in with a generic 401/403. Re-check the Console config, which // reports the reason, and replace the response with instructions when the connection needs the user. + // A client retrying rejected inference would otherwise send one config request per failed response. const explain = (event: SessionHttpResponse) => Effect.gen(function* () { if (event.response.status !== 401 && event.response.status !== 403) return - yield* check() + yield* check(Duration.seconds(30)) const status = reported if (!status) return void event.response.body?.cancel() diff --git a/packages/core/test/plugin/provider-opencode.test.ts b/packages/core/test/plugin/provider-opencode.test.ts index 832acaa71e1..c6481375ac1 100644 --- a/packages/core/test/plugin/provider-opencode.test.ts +++ b/packages/core/test/plugin/provider-opencode.test.ts @@ -1165,13 +1165,14 @@ describe("OpencodePlugin", () => { ), ) - it.live("explains rejected Console inference with the sign-in the connection needs", () => + it.effect("explains rejected Console inference from at most one config check per 30 seconds", () => Effect.acquireUseRelease( Effect.sync(() => { - const state = { sso: false } + const state = { sso: false, requests: 0 } const server = Bun.serve({ port: 0, fetch: () => { + state.requests++ if (state.sso) return Response.json( { _tag: "SsoRequired", orgId: "org_acme", connectionId: "ssoconn_1" }, @@ -1194,7 +1195,8 @@ describe("OpencodePlugin", () => { metadata: { server: server.url.origin, orgID: "org_acme", orgName: "Acme" }, }), }) - yield* addPlugin() + yield* addPlugin().pipe(inProcess(server)) + yield* drain const rejected = (providerID: Provider.ID) => hooks .trigger("session", "http.response", { @@ -1209,24 +1211,36 @@ describe("OpencodePlugin", () => { Effect.flatMap((event) => Effect.promise(async () => ({ status: event.response.status, body: await event.response.json() })), ), + inProcess(server), ) const denied = { status: 403, body: { error: { message: "Workspace access denied" } } } - - expect(yield* rejected(Provider.ID.make("remote"))).toEqual(denied) - - state.sso = true const explained = { status: 403, - body: { - error: { - type: "authentication_error", - message: "Sign in with SSO again to use Acme.", - }, - }, + body: { error: { type: "authentication_error", message: "Sign in with SSO again to use Acme." } }, } - expect(yield* rejected(Provider.ID.make("remote"))).toEqual(explained) + expect(state.requests).toBe(1) + + expect(yield* rejected(Provider.ID.make("remote"))).toEqual(denied) + expect(state.requests).toBe(1) + + state.sso = true + yield* TestClock.adjust("30 seconds") + expect( + yield* Effect.all( + Array.from({ length: 10 }, () => rejected(Provider.ID.make("remote"))), + { concurrency: "unbounded" }, + ), + ).toEqual(Array.from({ length: 10 }, () => explained)) + expect(state.requests).toBe(2) + + expect( + yield* Effect.forEach(Array.from({ length: 20 }), () => + TestClock.adjust("1 second").pipe(Effect.andThen(rejected(Provider.ID.make("remote")))), + ), + ).toEqual(Array.from({ length: 20 }, () => explained)) expect(yield* rejected(Provider.ID.opencode)).toEqual(explained) expect(yield* rejected(Provider.ID.anthropic)).toEqual(denied) + expect(state.requests).toBe(2) }), ({ server }) => Effect.promise(() => server.stop(true)), ),