From d507d5c05e6211cad609219c2eede2adf1a4c79e Mon Sep 17 00:00:00 2001 From: Sau1707 <83017316+Sau1707@users.noreply.github.com> Date: Tue, 13 Oct 2026 15:21:11 +0200 Subject: [PATCH] refactor(runtime): consolidate ownership and remove unused APIs Share config reload, OAuth callback, integration expiry, PTY controller, environment file, and validator workflows. Remove unread TUI state and redundant registration layers; simplify reconnect, preview, persistence, and process policies. Add regression coverage for cancellation, persistence interleavings, selection, focus, schema transforms, and error precedence. BREAKING CHANGE: remove FSUtil.globUp, ClientConnectionOptions.pageLifecycle, and foregroundIdleThreshold. External consumers must update these references and own browser lifecycle handling. No database migration is required. --- .../cli/src/services/server-connection.ts | 19 +- packages/cli/test/server-connection.test.ts | 47 ++ packages/client/src/effect/service.ts | 29 +- packages/client/src/promise/service.ts | 29 +- packages/client/src/service-probe.ts | 21 + packages/client/src/solid/connection.ts | 73 +-- packages/client/test/service-timeouts.test.ts | 59 +++ packages/client/test/solid-connection.test.ts | 73 ++- packages/core/src/config/plugin/agent.ts | 37 +- packages/core/src/config/plugin/command.ts | 37 +- packages/core/src/config/reload.ts | 30 ++ packages/core/src/environment/environment.ts | 2 +- packages/core/src/environment/index.ts | 10 +- packages/core/src/environment/make-files.ts | 8 + packages/core/src/formatter.ts | 2 +- packages/core/src/integration.ts | 44 +- packages/core/src/oauth/loopback.ts | 58 +++ packages/core/src/persistent-pty/index.ts | 32 +- packages/core/src/plugin/provider/dynamic.ts | 3 +- packages/core/src/plugin/provider/gitlab.ts | 60 +-- packages/core/src/plugin/provider/poe.ts | 58 +-- .../core/src/plugin/provider/sap-ai-core.ts | 3 - .../core/src/plugin/provider/sdk-factory.ts | 6 +- packages/core/src/plugin/websearch/exa.ts | 13 +- .../core/src/plugin/websearch/firecrawl.ts | 13 +- .../core/src/plugin/websearch/integration.ts | 11 + .../core/src/plugin/websearch/parallel.ts | 13 +- packages/core/src/plugin/websearch/tavily.ts | 13 +- .../core/src/plugin/websearch/tinyfish.ts | 13 +- packages/core/src/rpc.ts | 5 +- packages/core/src/shell.ts | 6 +- packages/core/src/tool/runtime.ts | 6 +- packages/core/src/util/standard-schema.ts | 7 + packages/core/src/workspace.ts | 7 +- .../core/test/config/reload-scheduler.test.ts | 83 ++++ .../core/test/filesystem/filesystem.test.ts | 19 - packages/core/test/formatter.test.ts | 31 ++ packages/core/test/integration.test.ts | 153 ++++++- .../core/test/persistent-pty-daemon.test.ts | 104 +++++ .../fixtures/provider-noncallable-factory.ts | 4 + .../core/test/plugin/provider-dynamic.test.ts | 42 ++ .../test/plugin/provider-gitlab-oauth.test.ts | 53 ++- .../test/plugin/provider-sap-ai-core.test.ts | 35 +- packages/core/test/plugin/websearch.test.ts | 29 ++ packages/core/test/process/process.test.ts | 18 +- packages/core/test/rpc-handler-errors.test.ts | 112 ++++- packages/core/test/shell-retention.test.ts | 7 + packages/core/test/standard-schema.test.ts | 93 ++++ packages/core/test/tool-schema.test.ts | 36 ++ packages/server/src/handlers/fs.ts | 78 ++-- packages/server/src/handlers/integration.ts | 414 +++++++++--------- packages/server/src/handlers/model.ts | 36 +- packages/server/src/handlers/provider.ts | 44 +- packages/server/src/handlers/pty.ts | 50 +-- packages/server/src/handlers/websearch.ts | 80 ++-- packages/server/test/integration.test.ts | 71 +++ packages/server/test/pty-errors.test.ts | 58 +++ packages/tui/src/component/devtools-bar.tsx | 14 +- packages/tui/src/context/editor.ts | 8 - packages/tui/src/devtools/index.ts | 41 -- .../tui/src/feature-plugins/home/footer.tsx | 2 - packages/tui/src/plugin/context.tsx | 4 +- packages/tui/src/plugin/structure.ts | 48 +- packages/tui/src/ui/dialog.tsx | 47 +- packages/tui/src/util/collapse-tool-output.ts | 19 +- packages/tui/src/util/persistence.ts | 14 +- .../test/cli/tui/collapse-tool-output.test.ts | 111 ++++- .../tui/test/cli/tui/dialog-dismiss.test.tsx | 198 +++++++++ packages/tui/test/editor-context.test.tsx | 141 ++++++ packages/tui/test/plugin-structure.test.ts | 104 +++++ packages/tui/test/prompt/persistence.test.ts | 59 ++- packages/util/src/fs-util.ts | 20 +- packages/util/src/process.ts | 13 +- 73 files changed, 2342 insertions(+), 998 deletions(-) create mode 100644 packages/client/test/service-timeouts.test.ts create mode 100644 packages/core/src/config/reload.ts create mode 100644 packages/core/src/environment/make-files.ts create mode 100644 packages/core/src/oauth/loopback.ts create mode 100644 packages/core/src/plugin/websearch/integration.ts create mode 100644 packages/core/src/util/standard-schema.ts create mode 100644 packages/core/test/config/reload-scheduler.test.ts create mode 100644 packages/core/test/plugin/fixtures/provider-noncallable-factory.ts create mode 100644 packages/core/test/standard-schema.test.ts create mode 100644 packages/server/test/integration.test.ts create mode 100644 packages/server/test/pty-errors.test.ts delete mode 100644 packages/tui/src/devtools/index.ts create mode 100644 packages/tui/test/cli/tui/dialog-dismiss.test.tsx create mode 100644 packages/tui/test/editor-context.test.tsx create mode 100644 packages/tui/test/plugin-structure.test.ts diff --git a/packages/cli/src/services/server-connection.ts b/packages/cli/src/services/server-connection.ts index db0d07ce2b..6eeba61284 100644 --- a/packages/cli/src/services/server-connection.ts +++ b/packages/cli/src/services/server-connection.ts @@ -9,7 +9,7 @@ import { Standalone } from "./standalone" export type Args = { readonly server?: string readonly standalone?: boolean - readonly mismatch?: "replace" | "ignore" | "error" + readonly mismatch?: "replace" | "ignore" readonly onStart?: EnsureOptions["onStart"] } @@ -42,10 +42,9 @@ export const resolve = Effect.fn("cli.server-connection.resolve")(function* (arg return { endpoint: yield* Standalone.start() } satisfies Resolved } - const mismatch = args.mismatch ?? "ignore" - const options = yield* ServiceConfig.options({ checkVersion: mismatch !== "ignore" }) + const options = yield* ServiceConfig.options({ checkVersion: args.mismatch === "replace" }) return { - endpoint: yield* resolveManaged({ ...options, onStart: args.onStart }, mismatch), + endpoint: yield* Service.ensure({ ...options, onStart: args.onStart }), service: managedService(options), } satisfies Resolved }) @@ -71,18 +70,6 @@ export const shutdownPersistentPty = Effect.fn("cli.server-connection.shutdown-p yield* Effect.tryPromise(() => client.experimental.persistentPty.shutdown()) }) -const resolveManaged = Effect.fnUntraced(function* (options: EnsureOptions, mismatch: NonNullable) { - if (mismatch === "replace") return yield* Service.ensure(options) - if (mismatch === "ignore") return yield* Service.ensure({ ...options, version: undefined }) - - const compatible = yield* Service.discover(options) - if (compatible !== undefined) return compatible - const existing = yield* Service.discover({ ...options, version: undefined }) - if (existing !== undefined) - return yield* Effect.fail(new Error("Background server version does not match this client")) - return yield* Service.ensure(options) -}) - function connectError(endpoint: Endpoint, cause: unknown) { if (isUnauthorizedError(cause)) { return new Error( diff --git a/packages/cli/test/server-connection.test.ts b/packages/cli/test/server-connection.test.ts index 4174fb72b2..4fdc9b8bf2 100644 --- a/packages/cli/test/server-connection.test.ts +++ b/packages/cli/test/server-connection.test.ts @@ -69,3 +69,50 @@ test("service options only require a matching version when requested", async () await fs.rm(root, { recursive: true, force: true }) } }) + +test("outdated managed registration is reused by default and ignore, but replace announces a version mismatch", async () => { + const root = await fs.mkdtemp(path.join("/tmp/uncode", "opencode-server-mismatch-")) + const version = "2.0.0-old" + const server = Bun.serve({ + port: 0, + fetch: () => Response.json({ version, pid: process.pid, urls: [] }), + }) + const registration = path.join(root, "state", ServiceConfig.filename()) + const layer = Global.layerWith({ config: path.join(root, "config"), state: path.join(root, "state") }) + const runPromise = (effect: Effect.Effect) => + Effect.runPromise(effect.pipe(Effect.provide(layer), Effect.provide(NodeFileSystem.layer), Effect.scoped)) + const starts: [string, string | undefined][] = [] + + try { + await fs.mkdir(path.dirname(registration), { recursive: true }) + await Bun.write(registration, JSON.stringify({ version, url: server.url.toString(), pid: process.pid })) + for (const mismatch of [undefined, "ignore"] as const) { + const resolved = await runPromise( + ServerConnection.resolve({ + mismatch, + onStart: (reason, previous) => starts.push([reason, previous]), + }), + ) + expect(resolved.endpoint.url).toBe(server.url.toString()) + expect(resolved.service).toBeDefined() + } + expect(starts).toEqual([]) + // Stop at admission: the registered PID belongs to this test process. + await expect( + runPromise( + ServerConnection.resolve({ + mismatch: "replace", + onStart: (reason, previous) => { + starts.push([reason, previous]) + throw new Error("Replacement admitted") + }, + }), + ), + ).rejects.toThrow("Replacement admitted") + expect(starts).toEqual([["version-mismatch", version]]) + expect((await Bun.file(registration).json()).version).toBe(version) + } finally { + await server.stop(true) + await fs.rm(root, { recursive: true, force: true }) + } +}) diff --git a/packages/client/src/effect/service.ts b/packages/client/src/effect/service.ts index 24c2b65fd3..eb13f63f54 100644 --- a/packages/client/src/effect/service.ts +++ b/packages/client/src/effect/service.ts @@ -4,7 +4,7 @@ import { contenderPool, spawnServiceContender } from "../service-contender.js" import { defaultEnsureTiming, ensureTiming, type EnsureTiming } from "../service-timing.js" import { matchesVersion } from "../service-version.js" import { PtyHandoff } from "../pty-handoff.js" -import { decide, fallback, headers, probeResult, same } from "../service-probe.js" +import { decide, fallback, headers, probeResult, same, timeoutTracker } from "../service-probe.js" export * from "../service.js" export { headers } @@ -49,7 +49,7 @@ export const incumbent = Effect.fn("service.incumbent")(function* ( export const ensure = Effect.fn("service.ensure")(function* (options: EnsureOptions = {}) { const timing = ensureTiming(options) const pool = contenderPool(timing) - let timeouts: { readonly info: Info; readonly count: number } | undefined + const timeouts = timeoutTracker() let announced = false const announce = (reason: "missing" | "version-mismatch", previousVersion?: string) => Effect.sync(() => { @@ -72,21 +72,16 @@ export const ensure = Effect.fn("service.ensure")(function* (options: EnsureOpti const registration = yield* registered(options.file, timing.requestTimeout) const info = registration.info const service = registration.service - if (registration.timedOut && info !== undefined) { - timeouts = { - info, - count: timeouts !== undefined && same(timeouts.info, info) ? timeouts.count + 1 : 1, - } - if (timeouts.count >= 3) { - yield* announce("missing") - yield* Effect.logWarning("Background service is unresponsive; recovery cannot preserve persistent terminals") - yield* Effect.tryPromise(() => PtyHandoff.clear(options.file ?? fallback())) - yield* terminate(info, options, timing) - pool.evict(info.pid) - pool.recruitNow() - timeouts = undefined - } - } else timeouts = undefined + const unresponsive = timeouts.record(registration) + if (unresponsive !== undefined) { + yield* announce("missing") + yield* Effect.logWarning("Background service is unresponsive; recovery cannot preserve persistent terminals") + yield* Effect.tryPromise(() => PtyHandoff.clear(options.file ?? fallback())) + yield* terminate(unresponsive, options, timing) + pool.evict(unresponsive.pid) + pool.recruitNow() + timeouts.reset() + } if (service !== undefined) { pool.serviceAnswered() const decision = decide(service, options) diff --git a/packages/client/src/promise/service.ts b/packages/client/src/promise/service.ts index 7526b9c10e..f3d6d9b62d 100644 --- a/packages/client/src/promise/service.ts +++ b/packages/client/src/promise/service.ts @@ -4,7 +4,7 @@ import { contenderPool, spawnServiceContender } from "../service-contender.js" import { defaultEnsureTiming, ensureTiming, type EnsureTiming } from "../service-timing.js" import { matchesVersion } from "../service-version.js" import { PtyHandoff } from "../pty-handoff.js" -import { decide, fallback, headers, probeResult, same } from "../service-probe.js" +import { decide, fallback, headers, probeResult, same, timeoutTracker } from "../service-probe.js" export * from "../service.js" export { headers } @@ -29,7 +29,7 @@ export async function ensure(options: EnsureOptions = {}): Promise { const timing = ensureTiming(options) const deadline = Date.now() + timing.promiseTimeout const pool = contenderPool(timing) - let timeouts: { readonly info: Info; readonly count: number } | undefined + const timeouts = timeoutTracker() let announced = false const announce = (reason: "missing" | "version-mismatch", previousVersion?: string) => { @@ -52,21 +52,16 @@ export async function ensure(options: EnsureOptions = {}): Promise { if (Date.now() >= deadline) throw pool.failure() ?? new Error("Timed out waiting for the background service to start") const registration = await registered(options.file, timing.requestTimeout) - if (registration.timedOut && registration.info !== undefined) { - timeouts = { - info: registration.info, - count: timeouts !== undefined && same(timeouts.info, registration.info) ? timeouts.count + 1 : 1, - } - if (timeouts.count >= 3) { - announce("missing") - console.warn("Background service is unresponsive; recovery cannot preserve persistent terminals") - await PtyHandoff.clear(options.file ?? fallback()) - await terminate(registration.info, options, timing) - pool.evict(registration.info.pid) - pool.recruitNow() - timeouts = undefined - } - } else timeouts = undefined + const unresponsive = timeouts.record(registration) + if (unresponsive !== undefined) { + announce("missing") + console.warn("Background service is unresponsive; recovery cannot preserve persistent terminals") + await PtyHandoff.clear(options.file ?? fallback()) + await terminate(unresponsive, options, timing) + pool.evict(unresponsive.pid) + pool.recruitNow() + timeouts.reset() + } if (registration.service !== undefined) { pool.serviceAnswered() diff --git a/packages/client/src/service-probe.ts b/packages/client/src/service-probe.ts index 749c1a7321..587c00ed61 100644 --- a/packages/client/src/service-probe.ts +++ b/packages/client/src/service-probe.ts @@ -45,6 +45,27 @@ export function same(left: Info, right: Info) { return left.id === right.id && left.version === right.version && left.url === right.url && left.pid === right.pid } +/** Count consecutive probe timeouts for one owner, independently for each ensure invocation. */ +export function timeoutTracker() { + let timeouts: { readonly info: Info; readonly count: number } | undefined + return { + record(registration: { readonly info?: Info; readonly timedOut: boolean }) { + if (!registration.timedOut || registration.info === undefined) { + timeouts = undefined + return undefined + } + timeouts = { + info: registration.info, + count: timeouts !== undefined && same(timeouts.info, registration.info) ? timeouts.count + 1 : 1, + } + return timeouts.count >= 3 ? registration.info : undefined + }, + reset() { + timeouts = undefined + }, + } +} + /** Ask a registered owner for its health and classify the answer. */ export async function probeResult(info: Info, timeout = defaultEnsureTiming.requestTimeout) { const endpoint = { diff --git a/packages/client/src/solid/connection.ts b/packages/client/src/solid/connection.ts index 01e6b75b28..934e8331e0 100644 --- a/packages/client/src/solid/connection.ts +++ b/packages/client/src/solid/connection.ts @@ -17,7 +17,6 @@ export type ClientConnectionOptions = { readonly reconnect?: (signal: AbortSignal) => Promise readonly onEvent: (event: OpenCodeEvent) => void readonly flushInterval?: number - readonly pageLifecycle?: boolean /** * Abort and reconnect a stream that receives no bytes for this long. The server writes a keepalive * comment every 15 seconds, so a quiet but healthy stream never trips this. @@ -33,9 +32,6 @@ const connectTimeout = 2_000 const reconnectDelay = 1_000 const connectionHistoryLimit = 50 export const defaultIdleTimeout = 45_000 -// Longer than one server keepalive interval: a stream that is silent this long when the page -// returns to the foreground is probably half-open after the device slept. -export const foregroundIdleThreshold = 20_000 export function createClientConnection(initialApi: OpenCodeClient, options: ClientConnectionOptions) { const abort = new AbortController() @@ -51,9 +47,6 @@ export function createClientConnection(initialApi: OpenCodeClient, options: Clie let flushTimer: ReturnType | undefined let stream: AbortController | undefined let current: AbortController | undefined - let run: Promise | undefined - let started = false - let generation = 0 let lastActivity = 0 let forced = false @@ -82,7 +75,7 @@ export function createClientConnection(initialApi: OpenCodeClient, options: Clie signal.addEventListener("abort", cancel, { once: true }) // Any received bytes, including keepalive comments, push the stall deadline out. A timer whose - // deadline passed while the page was suspended fires as soon as the page resumes. + // deadline passed while the runtime was suspended fires as soon as it resumes. let watchdog: ReturnType | undefined const touch = () => { lastActivity = Date.now() @@ -144,9 +137,9 @@ export function createClientConnection(initialApi: OpenCodeClient, options: Clie } } - async function runStream(active: number) { + async function runStream() { let attempt = 0 - while (!abort.signal.aborted && started && generation === active) { + while (!abort.signal.aborted) { setConnection({ status: attempt === 0 ? "connecting" : "reconnecting", attempt }) const controller = new AbortController() stream = controller @@ -154,7 +147,7 @@ export function createClientConnection(initialApi: OpenCodeClient, options: Clie abort.signal.addEventListener("abort", cancel) const result = await connect(controller.signal, attempt) abort.signal.removeEventListener("abort", cancel) - if (abort.signal.aborted || !started || generation !== active) return + if (abort.signal.aborted) return if (result.connectedAt !== undefined && Date.now() - result.connectedAt >= reconnectDelay) attempt = 0 attempt += 1 const message = errorMessage(result.error) @@ -167,7 +160,7 @@ export function createClientConnection(initialApi: OpenCodeClient, options: Clie if (!controller.signal.aborted) options.log?.info?.("server resolution failed", { attempt, error: errorMessage(error) }) }) - if (abort.signal.aborted || controller.signal.aborted || !started || generation !== active) return + if (abort.signal.aborted || controller.signal.aborted) return if (next) { api = next if (attempt === 1) continue @@ -182,67 +175,21 @@ export function createClientConnection(initialApi: OpenCodeClient, options: Clie } } - function start() { - if (started) return run - started = true - forced = false - const active = ++generation - const previous = run - const current = (async () => { - if (previous) await previous - await runStream(active) - })().finally(() => { - if (run !== current) return - run = undefined - }) - run = current - return run - } - - function stop() { - if (!started) return - started = false - generation += 1 - stream?.abort() - // Nothing is listening once stopped, so consumers must treat their data as stale until start() reconnects. - setConnection({ status: "connecting", attempt: 0, error: undefined }) - } - // Drop the live request so the reconnect loop replaces it now instead of waiting for the idle watchdog. function resync(reason: string) { - if (!started || connection.status !== "connected") return + if (abort.signal.aborted || connection.status !== "connected") return options.log?.info?.("event stream resync", { reason, idle: Date.now() - lastActivity }) forced = true current?.abort(new Error(reason)) } - if (options.pageLifecycle) { - const pagehide = () => stop() - const pageshow = () => void start() - // Locking a phone or switching apps hides the document without a pagehide; the socket usually - // dies while the page is suspended, and the browser may never report that on the hung read. - const visibility = () => { - if (document.visibilityState !== "visible") return - if (Date.now() - lastActivity < foregroundIdleThreshold) return - resync("Page returned to the foreground after the event stream went quiet") - } - const online = () => resync("Network connection restored") - window.addEventListener("pagehide", pagehide) - window.addEventListener("pageshow", pageshow) - window.addEventListener("online", online) - document.addEventListener("visibilitychange", visibility) - onCleanup(() => { - window.removeEventListener("pagehide", pagehide) - window.removeEventListener("pageshow", pageshow) - window.removeEventListener("online", online) - document.removeEventListener("visibilitychange", visibility) - }) - } - void start() + void runStream() onCleanup(() => { - stop() abort.abort() + // Resolution can still be using this controller after connect removed its abort listener. + stream?.abort() + setConnection({ status: "connecting", attempt: 0, error: undefined }) if (flushTimer) clearTimeout(flushTimer) pending = [] }) diff --git a/packages/client/test/service-timeouts.test.ts b/packages/client/test/service-timeouts.test.ts new file mode 100644 index 0000000000..d99a6e7f71 --- /dev/null +++ b/packages/client/test/service-timeouts.test.ts @@ -0,0 +1,59 @@ +import { expect, test } from "bun:test" +import { timeoutTracker } from "../src/service-probe" + +const info = { id: "owner", version: "1", url: "http://127.0.0.1:4096", pid: 4242, password: "secret" } + +test("recovery starts on the third consecutive timeout for the same owner", () => { + const tracker = timeoutTracker() + expect(tracker.record({ info, timedOut: true })).toBeUndefined() + expect(tracker.record({ info: { ...info }, timedOut: true })).toBeUndefined() + expect(tracker.record({ info, timedOut: true })).toBe(info) +}) + +test.each([{ id: "other" }, { version: "2" }, { url: "http://127.0.0.1:4097" }, { pid: 4243 }])( + "a changed registration resets the timeout count: %j", + (change) => { + const tracker = timeoutTracker() + tracker.record({ info, timedOut: true }) + tracker.record({ info, timedOut: true }) + const next = { ...info, ...change } + expect(tracker.record({ info: next, timedOut: true })).toBeUndefined() + expect(tracker.record({ info: next, timedOut: true })).toBeUndefined() + expect(tracker.record({ info: next, timedOut: true })).toBe(next) + }, +) + +test.each([ + { info, timedOut: false }, + { info: undefined, timedOut: true }, + { info: undefined, timedOut: false }, +])("a non-timeout or missing registration resets the timeout count: %j", (registration) => { + const tracker = timeoutTracker() + tracker.record({ info, timedOut: true }) + tracker.record({ info, timedOut: true }) + expect(tracker.record(registration)).toBeUndefined() + expect(tracker.record({ info, timedOut: true })).toBeUndefined() + expect(tracker.record({ info, timedOut: true })).toBeUndefined() + expect(tracker.record({ info, timedOut: true })).toBe(info) +}) + +test("password changes do not change registration identity", () => { + const tracker = timeoutTracker() + tracker.record({ info, timedOut: true }) + expect(tracker.record({ info: { ...info, password: "changed" }, timedOut: true })).toBeUndefined() + expect(tracker.record({ info, timedOut: true })).toBe(info) +}) + +test("completed recovery resets the counter and ensure invocations stay independent", () => { + const first = timeoutTracker() + const second = timeoutTracker() + first.record({ info, timedOut: true }) + first.record({ info, timedOut: true }) + expect(second.record({ info, timedOut: true })).toBeUndefined() + expect(first.record({ info, timedOut: true })).toBe(info) + first.reset() + expect(first.record({ info, timedOut: true })).toBeUndefined() + expect(first.record({ info, timedOut: true })).toBeUndefined() + expect(second.record({ info, timedOut: true })).toBeUndefined() + expect(first.record({ info, timedOut: true })).toBe(info) +}) diff --git a/packages/client/test/solid-connection.test.ts b/packages/client/test/solid-connection.test.ts index 60219ed73e..6f469f0e94 100644 --- a/packages/client/test/solid-connection.test.ts +++ b/packages/client/test/solid-connection.test.ts @@ -1,7 +1,7 @@ import { expect, test } from "bun:test" import { createRoot } from "solid-js" import { createClientConnection } from "../src/solid" -import { OpenCode, type OpenCodeEvent } from "../src/promise" +import { OpenCode, type OpenCodeClient, type OpenCodeEvent } from "../src/promise" const connected = { id: "evt_connected", created: 1, type: "server.connected", data: {} } @@ -179,3 +179,74 @@ test("a reconnect does not ask the volatile event stream to replay", async () => ctx.dispose() } }) + +test("disposing the connection aborts its stream and drops pending batched events", async () => { + const fake = server() + const events: OpenCodeEvent[] = [] + const ctx = createRoot((dispose) => ({ + dispose, + connection: createClientConnection(fake.api, { + idleTimeout: 10_000, + flushInterval: 10_000, + onEvent: (event) => events.push(event), + }), + })) + try { + await until(() => fake.streams.length === 1) + fake.streams[0].write(`data: ${JSON.stringify(connected)}\n\n`) + await until(() => ctx.connection.status() === "connected") + expect(events).toEqual([]) + + ctx.dispose() + expect(fake.streams[0].aborted).toBe(true) + ctx.connection.internal.resync("disposed") + await new Promise((resolve) => setTimeout(resolve, 20)) + expect(fake.streams).toHaveLength(1) + expect(events).toEqual([]) + } finally { + ctx.dispose() + } +}) + +test.each(["resolve", "reject"] as const)( + "disposing during server resolution aborts it and ignores a late %s", + async (outcome) => { + const fake = server() + let signal: AbortSignal | undefined + let finish!: () => void + const logs: string[] = [] + const ctx = createRoot((dispose) => ({ + dispose, + connection: createClientConnection(fake.api, { + idleTimeout: 10_000, + flushInterval: 0, + onEvent: () => {}, + log: { info: (message) => logs.push(message) }, + reconnect: (current) => { + signal = current + return new Promise((resolve, reject) => { + finish = () => (outcome === "resolve" ? resolve(fake.api) : reject(new Error("Resolution failed"))) + }) + }, + }), + })) + try { + await until(() => fake.streams.length === 1) + fake.streams[0].write(`data: ${JSON.stringify(connected)}\n\n`) + await until(() => ctx.connection.status() === "connected") + fake.streams[0].error(new Error("disconnect")) + await until(() => signal !== undefined) + expect(signal?.aborted).toBe(false) + + ctx.dispose() + expect(signal?.aborted).toBe(true) + finish() + await new Promise((resolve) => setTimeout(resolve, 20)) + expect(fake.streams).toHaveLength(1) + expect(ctx.connection.status()).toBe("connecting") + expect(logs).not.toContain("server resolution failed") + } finally { + ctx.dispose() + } + }, +) diff --git a/packages/core/src/config/plugin/agent.ts b/packages/core/src/config/plugin/agent.ts index 1d3949d015..05984febc1 100644 --- a/packages/core/src/config/plugin/agent.ts +++ b/packages/core/src/config/plugin/agent.ts @@ -4,10 +4,11 @@ import { define } from "@opencode/plugin/effect/plugin" import { Document, Info, type Entry } from "@opencode/schema/config" import { ConfigAgent } from "@opencode/schema/config/agent" import path from "path" -import { Effect, Option, PubSub, Schema, Stream } from "effect" +import { Effect, Option, Schema, Stream } from "effect" import { Agent } from "../../agent.js" import { Config } from "../../config.js" import { ConfigMarkdown } from "../markdown.js" +import { ConfigReload } from "../reload.js" import { FSUtil } from "@opencode/util/fs-util" import { ConfigAgentV1 } from "../../v1/config/agent.js" import { ConfigMigrateV1 } from "../../v1/config/migrate.js" @@ -56,28 +57,18 @@ export const Plugin = define({ Effect.tap((documents) => Effect.sync(() => (loaded.documents = documents))), Effect.andThen(ctx.agent.reload()), ) - // One trigger feed serializes reloads and shares one debounce window; - // subscribing before the initial scan means updates racing the scan still - // trigger a rebuild. Each source is subscribed eagerly on its own fiber - // (Stream.merge and Stream.debounce both open upstream a fiber hop later) - // so no update slips through while the debounce starts its pull. - const changes = yield* PubSub.sliding(1) - const notify = () => PubSub.publish(changes, undefined) - yield* config.changes().pipe( - Stream.filterEffect((update) => Effect.map(config.entries(), (entries) => isAgentSource(entries, update.path))), - Stream.runForEach(notify), - Effect.forkScoped({ startImmediately: true }), - ) - yield* ctx.event.subscribe().pipe( - Stream.filter((event) => event.type === "config.updated"), - Stream.runForEach(notify), - Effect.forkScoped({ startImmediately: true }), - ) - const updates = yield* PubSub.subscribe(changes) - yield* Stream.fromSubscription(updates).pipe( - Stream.debounce("100 millis"), - Stream.runForEach(() => reload), - Effect.forkScoped({ startImmediately: true }), + yield* ConfigReload.schedule( + [ + config + .changes() + .pipe( + Stream.filterEffect((update) => + Effect.map(config.entries(), (entries) => isAgentSource(entries, update.path)), + ), + ), + ctx.event.subscribe().pipe(Stream.filter((event) => event.type === "config.updated")), + ], + reload, ) loaded.documents = yield* load() yield* ctx.agent.transform((editor) => { diff --git a/packages/core/src/config/plugin/command.ts b/packages/core/src/config/plugin/command.ts index a029fb8ade..15fae92ef0 100644 --- a/packages/core/src/config/plugin/command.ts +++ b/packages/core/src/config/plugin/command.ts @@ -7,7 +7,7 @@ import { Model } from "@opencode/schema/model" import { Provider } from "@opencode/schema/provider" import { AppProcess } from "@opencode/util/process" import path from "path" -import { Effect, Option, PubSub, Schema, Stream } from "effect" +import { Effect, Option, Schema, Stream } from "effect" import { ChildProcess } from "effect/process" import { Agent } from "../../agent.js" import { Config } from "../../config.js" @@ -17,6 +17,7 @@ import { SubagentJob } from "../../session/subagent-job.js" import { ShellSelect } from "../../shell/select.js" import { FSUtil } from "@opencode/util/fs-util" import { ConfigMarkdown } from "../markdown.js" +import { ConfigReload } from "../reload.js" const decodeCommand = Schema.decodeUnknownOption(ConfigCommand.Info) @@ -45,28 +46,18 @@ export const Plugin = define({ Effect.tap((documents) => Effect.sync(() => (loaded.documents = documents))), Effect.andThen(ctx.command.reload()), ) - // One trigger feed serializes reloads and shares one debounce window; - // subscribing before the initial scan means updates racing the scan still - // trigger a rebuild. Each source is subscribed eagerly on its own fiber - // (Stream.merge and Stream.debounce both open upstream a fiber hop later) - // so no update slips through while the debounce starts its pull. - const changes = yield* PubSub.sliding(1) - const notify = () => PubSub.publish(changes, undefined) - yield* config.changes().pipe( - Stream.filterEffect((update) => Effect.map(config.entries(), (entries) => isCommandSource(entries, update.path))), - Stream.runForEach(notify), - Effect.forkScoped({ startImmediately: true }), - ) - yield* ctx.event.subscribe().pipe( - Stream.filter((event) => event.type === "config.updated"), - Stream.runForEach(notify), - Effect.forkScoped({ startImmediately: true }), - ) - const updates = yield* PubSub.subscribe(changes) - yield* Stream.fromSubscription(updates).pipe( - Stream.debounce("100 millis"), - Stream.runForEach(() => reload), - Effect.forkScoped({ startImmediately: true }), + yield* ConfigReload.schedule( + [ + config + .changes() + .pipe( + Stream.filterEffect((update) => + Effect.map(config.entries(), (entries) => isCommandSource(entries, update.path)), + ), + ), + ctx.event.subscribe().pipe(Stream.filter((event) => event.type === "config.updated")), + ], + reload, ) loaded.documents = yield* load() yield* ctx.command.transform((editor) => { diff --git a/packages/core/src/config/reload.ts b/packages/core/src/config/reload.ts new file mode 100644 index 0000000000..7d4fee86ee --- /dev/null +++ b/packages/core/src/config/reload.ts @@ -0,0 +1,30 @@ +export * as ConfigReload from "./reload.js" + +import { Effect, PubSub, Stream } from "effect" + +export function schedule( + triggers: readonly Stream.Stream[], + reload: Effect.Effect, +) { + return Effect.gen(function* () { + // Each call owns one feed and debounce window, serializing its reloads. + // Subscribe sources eagerly before the caller's initial scan: merge and + // debounce open upstream a fiber hop later and can miss startup updates. + const changes = yield* PubSub.sliding(1) + yield* Effect.forEach( + triggers, + (trigger) => + trigger.pipe( + Stream.runForEach(() => PubSub.publish(changes, undefined)), + Effect.forkScoped({ startImmediately: true }), + ), + { discard: true }, + ) + const updates = yield* PubSub.subscribe(changes) + yield* Stream.fromSubscription(updates).pipe( + Stream.debounce("100 millis"), + Stream.runForEach(() => reload), + Effect.forkScoped({ startImmediately: true }), + ) + }) +} diff --git a/packages/core/src/environment/environment.ts b/packages/core/src/environment/environment.ts index 416c0d0806..b22d260831 100644 --- a/packages/core/src/environment/environment.ts +++ b/packages/core/src/environment/environment.ts @@ -3,7 +3,7 @@ import { makeLocationNode } from "@opencode/util/effect/app-node" import { Context, Effect, Layer } from "effect" import { ChildProcessSpawner } from "effect/process/ChildProcessSpawner" import type { Files } from "./files.js" -import { makeFiles } from "./index.js" +import { makeFiles } from "./make-files.js" import { makeLocalDriver } from "./local.js" import { Location } from "../location.js" import { Workspace } from "../workspace.js" diff --git a/packages/core/src/environment/index.ts b/packages/core/src/environment/index.ts index 0f6564a33d..0c0f94ad5b 100644 --- a/packages/core/src/environment/index.ts +++ b/packages/core/src/environment/index.ts @@ -16,12 +16,4 @@ export { execDefaults } from "./exec-defaults.js" export { makeLocalDriver } from "./local.js" export { makeMemoryDriver, type MemoryDriver } from "./memory.js" export { type Interface, node, Service } from "./environment.js" - -import type { Driver } from "./driver.js" -import { execDefaults } from "./exec-defaults.js" -import type { Files } from "./files.js" - -export const makeFiles = (driver: Driver): Files => ({ - ...execDefaults(driver.spawner), - ...driver.overrides, -}) +export { makeFiles } from "./make-files.js" diff --git a/packages/core/src/environment/make-files.ts b/packages/core/src/environment/make-files.ts new file mode 100644 index 0000000000..7487e42f5a --- /dev/null +++ b/packages/core/src/environment/make-files.ts @@ -0,0 +1,8 @@ +import type { Driver } from "./driver.js" +import { execDefaults } from "./exec-defaults.js" +import type { Files } from "./files.js" + +export const makeFiles = (driver: Driver): Files => ({ + ...execDefaults(driver.spawner), + ...driver.overrides, +}) diff --git a/packages/core/src/formatter.ts b/packages/core/src/formatter.ts index 46bb4e6b28..c8b690a847 100644 --- a/packages/core/src/formatter.ts +++ b/packages/core/src/formatter.ts @@ -29,7 +29,7 @@ const layer = Layer.effect( Effect.gen(function* () { const location = yield* Location.Service const processes = yield* AppProcess.Service - const commands = new WeakMap() + const commands = new WeakMap() const state = State.create({ name: "formatter", initial: () => ({ formatters: [] }), diff --git a/packages/core/src/integration.ts b/packages/core/src/integration.ts index 8480e46324..1a469c8cad 100644 --- a/packages/core/src/integration.ts +++ b/packages/core/src/integration.ts @@ -292,14 +292,7 @@ type PendingCommandAttempt = { scope: Scope.Closeable time: AttemptTime } -type TerminalCommandAttempt = { - status: "complete" | "failed" | "expired" - integrationID: ID - message?: string - removeAt: number - time: AttemptTime -} -type CommandAttemptEntry = PendingCommandAttempt | TerminalCommandAttempt +type CommandAttemptEntry = PendingCommandAttempt | TerminalAttempt const layer = Layer.effect( Service, @@ -515,7 +508,7 @@ const layer = Layer.effect( value: Credential.Key.make({ type: "key", key: exit.value }), }).pipe(Effect.asVoid, Effect.exit) const settledAt = yield* Clock.currentTimeMillis - const terminal: TerminalCommandAttempt = Exit.isSuccess(persistence) + const terminal: TerminalAttempt = Exit.isSuccess(persistence) ? { status: "complete", integrationID: attempt.integrationID, @@ -533,32 +526,11 @@ const layer = Layer.effect( yield* close(attempt.scope) }, Effect.uninterruptible) - const scrub = Effect.fnUntraced(function* () { - const now = yield* Clock.currentTimeMillis - const expired = yield* SynchronizedRef.modify(attempts, (current) => { - const next = new Map(current) - const scopes: Scope.Closeable[] = [] - for (const [id, attempt] of current) { - if (attempt.status === "pending" && !attempt.persisting && attempt.time.expires <= now) { - scopes.push(attempt.scope) - next.set(id, { - status: "expired", - integrationID: attempt.integrationID, - time: attempt.time, - removeAt: now + terminalRetention, - }) - continue - } - if (attempt.status !== "pending" && attempt.removeAt <= now) next.delete(id) - } - return [scopes, next] - }) - yield* Effect.forEach(expired, close, { discard: true }) - }) - - const scrubCommands = Effect.fnUntraced(function* () { + const scrub = Effect.fnUntraced(function* ( + registry: SynchronizedRef.SynchronizedRef>, + ): Effect.fn.Return { const now = yield* Clock.currentTimeMillis - const expired = yield* SynchronizedRef.modify(commandAttempts, (current) => { + const expired = yield* SynchronizedRef.modify(registry, (current) => { const next = new Map(current) const scopes: Scope.Closeable[] = [] for (const [id, attempt] of current) { @@ -579,8 +551,8 @@ const layer = Layer.effect( yield* Effect.forEach(expired, close, { discard: true }) }) - yield* scrub().pipe(Effect.repeat(Schedule.spaced(scrubInterval)), Effect.forkIn(scope)) - yield* scrubCommands().pipe(Effect.repeat(Schedule.spaced(scrubInterval)), Effect.forkIn(scope)) + yield* scrub(attempts).pipe(Effect.repeat(Schedule.spaced(scrubInterval)), Effect.forkIn(scope)) + yield* scrub(commandAttempts).pipe(Effect.repeat(Schedule.spaced(scrubInterval)), Effect.forkIn(scope)) const connectOAuth = Effect.fn("Integration.oauth.connect")(function* (input: { readonly integrationID: ID diff --git a/packages/core/src/oauth/loopback.ts b/packages/core/src/oauth/loopback.ts new file mode 100644 index 0000000000..6dad9bd332 --- /dev/null +++ b/packages/core/src/oauth/loopback.ts @@ -0,0 +1,58 @@ +import { Deferred, Effect } from "effect" +import type { ServerResponse } from "node:http" +import { OauthCallbackPage } from "./page.js" + +// The Integration attempt owns the scope; each invocation owns its listener and admission. +export const oauthLoopback = Effect.fn(function* (input: { + provider: string + validate: (params: URLSearchParams) => string | undefined +}) { + const callback = yield* Deferred.make<{ code: string; response: ServerResponse }, Error>() + const { createServer } = yield* Effect.promise(() => import("node:http")) + const server = createServer((request, response) => { + const url = new URL(request.url ?? "/", "http://127.0.0.1") + if (request.method !== "GET" || url.pathname !== "/callback") { + response.writeHead(404).end() + return + } + const error = input.validate(url.searchParams) + if (error) { + response + .writeHead(400, { "Content-Type": "text/html" }) + .end(OauthCallbackPage.error(error, { provider: input.provider })) + Effect.runSync(Deferred.fail(callback, new Error(error))) + return + } + if (!Effect.runSync(Deferred.succeed(callback, { code: url.searchParams.get("code") ?? "", response }))) + response.writeHead(409).end("OAuth callback already received") + }) + yield* Effect.addFinalizer(() => + Effect.sync(() => { + server.close() + server.closeAllConnections() + }), + ) + return { + server, + complete: (exchange: (code: string) => Effect.Effect) => + Effect.gen(function* () { + const request = yield* Deferred.await(callback) + const respond = (error?: string) => + Effect.sync(() => + request.response + .writeHead(error ? 400 : 200, { "Content-Type": "text/html" }) + .end( + error + ? OauthCallbackPage.error(error, { provider: input.provider }) + : OauthCallbackPage.success({ provider: input.provider }), + ), + ) + return yield* exchange(request.code).pipe( + Effect.tap(() => respond()), + Effect.tapError((error) => respond(error.message)), + // Bun's server.closeAllConnections() leaves an unanswered callback response pending. + Effect.onInterrupt(() => Effect.sync(() => request.response.destroy())), + ) + }), + } +}) diff --git a/packages/core/src/persistent-pty/index.ts b/packages/core/src/persistent-pty/index.ts index 69c673b014..4aeabbe46a 100644 --- a/packages/core/src/persistent-pty/index.ts +++ b/packages/core/src/persistent-pty/index.ts @@ -191,23 +191,27 @@ const makeLayer = (options: Options = {}) => return undefined }) + const controllerOperation = Effect.fnUntraced(function* (id: Pty.ID, payload: () => object) { + const terminal = yield* get(id) + const response = yield* request(daemon, payload()) + if (response.type !== "ok") return yield* unexpected(response) + current.set(terminal.sessionID, id) + return undefined + }) + const resize = Effect.fn("PersistentPty.resize")(function* ( id: Pty.ID, cols: number, rows: number, attachmentID?: string, ) { - const terminal = yield* get(id) - const response = yield* request(daemon, { + return yield* controllerOperation(id, () => ({ op: "resize", id: fromID(id), attachment_id: attachmentID ?? null, cols, rows, - }) - if (response.type !== "ok") return yield* unexpected(response) - current.set(terminal.sessionID, id) - return undefined + })) }) const control = Effect.fn("PersistentPty.control")(function* ( @@ -216,17 +220,13 @@ const makeLayer = (options: Options = {}) => cols: number, rows: number, ) { - const terminal = yield* get(id) - const response = yield* request(daemon, { + return yield* controllerOperation(id, () => ({ op: "control", id: fromID(id), attachment_id: attachmentID, cols, rows, - }) - if (response.type !== "ok") return yield* unexpected(response) - current.set(terminal.sessionID, id) - return undefined + })) }) const input = Effect.fn("PersistentPty.input")(function* ( @@ -236,18 +236,14 @@ const makeLayer = (options: Options = {}) => rows: number, data: Uint8Array, ) { - const terminal = yield* get(id) - const response = yield* request(daemon, { + return yield* controllerOperation(id, () => ({ op: "input", id: fromID(id), attachment_id: attachmentID, cols, rows, data_base64: Buffer.from(data).toString("base64"), - }) - if (response.type !== "ok") return yield* unexpected(response) - current.set(terminal.sessionID, id) - return undefined + })) }) const snapshot = Effect.fn("PersistentPty.snapshot")(function* (id: Pty.ID) { diff --git a/packages/core/src/plugin/provider/dynamic.ts b/packages/core/src/plugin/provider/dynamic.ts index 0521f72c44..61e38c8698 100644 --- a/packages/core/src/plugin/provider/dynamic.ts +++ b/packages/core/src/plugin/provider/dynamic.ts @@ -12,7 +12,8 @@ export const DynamicProviderPlugin = define({ Effect.fn(function* (evt) { if (evt.sdk) return - evt.sdk = ((yield* loadSDKFactory(npm, evt.package)) as (options: any) => any)(evt.options) + const factory = yield* loadSDKFactory(npm, evt.package) + evt.sdk = factory(evt.options) }), ) }), diff --git a/packages/core/src/plugin/provider/gitlab.ts b/packages/core/src/plugin/provider/gitlab.ts index 45891e8d6a..ef258238b8 100644 --- a/packages/core/src/plugin/provider/gitlab.ts +++ b/packages/core/src/plugin/provider/gitlab.ts @@ -2,7 +2,7 @@ import os from "os" import { App } from "../../app.js" import { Clock, Deferred, Effect, Exit, Option, Schema, Semaphore, Stream } from "effect" import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/http" -import type { Server, ServerResponse } from "node:http" +import type { Server } from "node:http" import { define } from "@opencode/plugin/effect/plugin" import { Form } from "@opencode/schema/form" import type { DiscoveredWorkflowModel } from "gitlab-ai-provider" @@ -11,7 +11,7 @@ import { Credential } from "../../credential.js" import { Integration } from "../../integration.js" import { IntegrationConnection } from "../../integration/connection.js" import { Model } from "../../model.js" -import { OauthCallbackPage } from "../../oauth/page.js" +import { oauthLoopback } from "../../oauth/loopback.js" import { Provider } from "../../provider.js" import type { PluginInternal } from "../internal.js" @@ -186,32 +186,11 @@ export const GitLabPlugin = define({ ).toString("base64url") const state = Buffer.from(crypto.getRandomValues(new Uint8Array(32))).toString("base64url") - const callback = yield* Deferred.make<{ code: string; response: ServerResponse }, Error>() - const { createServer } = yield* Effect.promise(() => import("node:http")) - const server = createServer((request, response) => { - const url = new URL(request.url ?? "/", `http://${callbackHost}`) - if (request.method !== "GET" || url.pathname !== "/callback") { - response.writeHead(404).end() - return - } - const error = callbackError(url.searchParams, state) - if (error) { - response - .writeHead(400, { "Content-Type": "text/html" }) - .end(OauthCallbackPage.error(error, { provider: "GitLab" })) - Effect.runSync(Deferred.fail(callback, new Error(error))) - return - } - if (!Effect.runSync(Deferred.succeed(callback, { code: url.searchParams.get("code") ?? "", response }))) - response.writeHead(409).end("OAuth callback already received") + const callback = yield* oauthLoopback({ + provider: "GitLab", + validate: (params) => callbackError(params, state), }) - yield* Effect.addFinalizer(() => - Effect.sync(() => { - server.close() - server.closeAllConnections() - }), - ) - yield* listen(server) + yield* listen(callback.server) return { mode: "auto" as const, @@ -225,37 +204,20 @@ export const GitLabPlugin = define({ code_challenge_method: "S256", }).toString()}`, instructions: "Complete authorization in your browser. This window will close automatically.", - callback: Effect.gen(function* () { - const request = yield* Deferred.await(callback) - const respond = (error?: string) => - Effect.sync(() => - request.response - .writeHead(error ? 400 : 200, { "Content-Type": "text/html" }) - .end( - error - ? OauthCallbackPage.error(error, { provider: "GitLab" }) - : OauthCallbackPage.success({ provider: "GitLab" }), - ), - ) - return yield* exchange( + callback: callback.complete((code) => + exchange( http, instanceUrl, clientID, { grant_type: "authorization_code", - code: request.code, + code, redirect_uri: redirectURI, code_verifier: verifier, }, describeGrantFailure, - ).pipe( - Effect.flatMap((tokens) => credential(instanceUrl, clientID, tokens)), - Effect.tap(() => respond()), - Effect.tapError((error) => respond(error.message)), - // Bun's server.closeAllConnections() leaves an unanswered callback response pending. - Effect.onInterrupt(() => Effect.sync(() => request.response.destroy())), - ) - }), + ).pipe(Effect.flatMap((tokens) => credential(instanceUrl, clientID, tokens))), + ), } }), }) diff --git a/packages/core/src/plugin/provider/poe.ts b/packages/core/src/plugin/provider/poe.ts index 24edb14757..1638f81788 100644 --- a/packages/core/src/plugin/provider/poe.ts +++ b/packages/core/src/plugin/provider/poe.ts @@ -1,10 +1,9 @@ import { define } from "@opencode/plugin/effect/plugin" -import { Clock, Deferred, Effect, Option, Schema } from "effect" +import { Clock, Effect, Option, Schema } from "effect" import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/http" -import type { ServerResponse } from "node:http" import { Credential } from "../../credential.js" import { Integration } from "../../integration.js" -import { OauthCallbackPage } from "../../oauth/page.js" +import { oauthLoopback } from "../../oauth/loopback.js" const integrationID = Integration.ID.make("poe") const methodID = Integration.MethodID.make("browser") @@ -45,34 +44,13 @@ export const PoePlugin = define({ yield* Effect.promise(() => crypto.subtle.digest("SHA-256", new TextEncoder().encode(verifier))), ).toString("base64url") const state = Buffer.from(crypto.getRandomValues(new Uint8Array(32))).toString("base64url") - const callback = yield* Deferred.make<{ code: string; response: ServerResponse }, Error>() - const { createServer } = yield* Effect.promise(() => import("node:http")) - const { EventEmitter } = yield* Effect.promise(() => import("node:events")) - const server = createServer((request, response) => { - const url = new URL(request.url ?? "/", "http://127.0.0.1") - if (request.method !== "GET" || url.pathname !== "/callback") { - response.writeHead(404).end() - return - } - const error = callbackError(url.searchParams, state) - if (error) { - response - .writeHead(400, { "Content-Type": "text/html" }) - .end(OauthCallbackPage.error(error, { provider: "Poe" })) - Effect.runSync(Deferred.fail(callback, new Error(error))) - return - } - if (!Effect.runSync(Deferred.succeed(callback, { code: url.searchParams.get("code") ?? "", response }))) - response.writeHead(409).end("OAuth callback already received") + const callback = yield* oauthLoopback({ + provider: "Poe", + validate: (params) => callbackError(params, state), }) - yield* Effect.addFinalizer(() => - Effect.sync(() => { - server.close() - server.closeAllConnections() - }), - ) - yield* Effect.tryPromise(() => EventEmitter.once(server.listen(0, "127.0.0.1"), "listening")) - const address = server.address() + const { EventEmitter } = yield* Effect.promise(() => import("node:events")) + yield* Effect.tryPromise(() => EventEmitter.once(callback.server.listen(0, "127.0.0.1"), "listening")) + const address = callback.server.address() if (!address || typeof address === "string") return yield* Effect.fail(new Error("Missing OAuth callback port")) const redirect = `http://127.0.0.1:${address.port}/callback` @@ -88,25 +66,7 @@ export const PoePlugin = define({ state, }).toString()}`, instructions: "Complete authorization in your browser. This window will close automatically.", - callback: Effect.gen(function* () { - const request = yield* Deferred.await(callback) - const respond = (error?: string) => - Effect.sync(() => - request.response - .writeHead(error ? 400 : 200, { "Content-Type": "text/html" }) - .end( - error - ? OauthCallbackPage.error(error, { provider: "Poe" }) - : OauthCallbackPage.success({ provider: "Poe" }), - ), - ) - return yield* exchangeCode(http, { code: request.code, redirect, verifier }).pipe( - Effect.tap(() => respond()), - Effect.tapError((error) => respond(error.message)), - // Bun's server.closeAllConnections() leaves an unanswered callback response pending. - Effect.onInterrupt(() => Effect.sync(() => request.response.destroy())), - ) - }), + callback: callback.complete((code) => exchangeCode(http, { code, redirect, verifier })), } }), }) diff --git a/packages/core/src/plugin/provider/sap-ai-core.ts b/packages/core/src/plugin/provider/sap-ai-core.ts index 4085378e84..c93d7e489f 100644 --- a/packages/core/src/plugin/provider/sap-ai-core.ts +++ b/packages/core/src/plugin/provider/sap-ai-core.ts @@ -18,9 +18,6 @@ export const SapAICorePlugin = define({ if (serviceKey && !process.env.AICORE_SERVICE_KEY) process.env.AICORE_SERVICE_KEY = serviceKey const factory = yield* loadSDKFactory(npm, evt.package) - if (typeof factory !== "function") - return yield* Effect.die(new Error(`Package ${evt.package} provider factory export is not callable`)) - evt.sdk = factory( serviceKey ? { deploymentId: process.env.AICORE_DEPLOYMENT_ID, resourceGroup: process.env.AICORE_RESOURCE_GROUP } diff --git a/packages/core/src/plugin/provider/sdk-factory.ts b/packages/core/src/plugin/provider/sdk-factory.ts index 74ace1318b..8e5785c663 100644 --- a/packages/core/src/plugin/provider/sdk-factory.ts +++ b/packages/core/src/plugin/provider/sdk-factory.ts @@ -1,3 +1,4 @@ +import type { ProviderV3 } from "@ai-sdk/provider" import { Effect } from "effect" import { Npm } from "@opencode/util/npm" import { importModule, resolveModule } from "@opencode/util/runtime-import" @@ -13,7 +14,10 @@ export const loadSDKFactory = Effect.fnUntraced(function* (npm: Npm.Interface, p const mod = (yield* Effect.promise(() => importModule(installedPath))) as Record const match = Object.keys(mod).find((name) => name.startsWith("create")) if (!match) return yield* Effect.die(new Error(`Package ${packageName} has no provider factory export`)) - return mod[match] + const factory = mod[match] + if (typeof factory !== "function") + return yield* Effect.die(new Error(`Package ${packageName} provider factory export is not callable`)) + return factory as (options: Record) => ProviderV3 }) // The AI SDK bridge only supports AI SDK v6 providers. Official `@ai-sdk/*` packages now publish AI SDK v7 under diff --git a/packages/core/src/plugin/websearch/exa.ts b/packages/core/src/plugin/websearch/exa.ts index 555780f4f2..292b929fe0 100644 --- a/packages/core/src/plugin/websearch/exa.ts +++ b/packages/core/src/plugin/websearch/exa.ts @@ -4,6 +4,7 @@ import { define } from "@opencode/plugin/effect/plugin" import { Effect, Schema, Scope } from "effect" import { HttpClient } from "effect/http" import { WebSearchMcp } from "./mcp.js" +import { WebSearchIntegration } from "./integration.js" export const endpoint = "https://mcp.exa.ai/mcp" @@ -26,17 +27,7 @@ export const Plugin = define({ id: "opencode.websearch.exa", effect: Effect.fn("WebSearchExa.Plugin")(function* (ctx) { const http = yield* HttpClient.HttpClient - yield* ctx.integration.transform((editor) => { - editor.update("exa", (integration) => (integration.name = "Exa")) - editor.method.update({ - integrationID: "exa", - method: { type: "key" }, - }) - editor.method.update({ - integrationID: "exa", - method: { type: "env", names: ["EXA_API_KEY"] }, - }) - }) + yield* WebSearchIntegration.register(ctx.integration, "exa", "Exa", "EXA_API_KEY") yield* ctx.websearch.transform((editor) => { editor.add({ id: "exa", diff --git a/packages/core/src/plugin/websearch/firecrawl.ts b/packages/core/src/plugin/websearch/firecrawl.ts index 39126ffcc7..45ac0e94cf 100644 --- a/packages/core/src/plugin/websearch/firecrawl.ts +++ b/packages/core/src/plugin/websearch/firecrawl.ts @@ -5,6 +5,7 @@ import { Effect, Option, Schema, Scope } from "effect" import { HttpClient } from "effect/http" import { App } from "../../app.js" import { WebSearchMcp } from "./mcp.js" +import { WebSearchIntegration } from "./integration.js" export const endpoint = "https://mcp.firecrawl.dev/v2/mcp" @@ -37,17 +38,7 @@ export const Plugin = define({ id: "opencode.websearch.firecrawl", effect: Effect.fn("WebSearchFirecrawl.Plugin")(function* (ctx) { const http = yield* HttpClient.HttpClient - yield* ctx.integration.transform((editor) => { - editor.update("firecrawl", (integration) => (integration.name = "Firecrawl")) - editor.method.update({ - integrationID: "firecrawl", - method: { type: "key" }, - }) - editor.method.update({ - integrationID: "firecrawl", - method: { type: "env", names: ["FIRECRAWL_API_KEY"] }, - }) - }) + yield* WebSearchIntegration.register(ctx.integration, "firecrawl", "Firecrawl", "FIRECRAWL_API_KEY") yield* ctx.websearch.transform((editor) => { editor.add({ id: "firecrawl", diff --git a/packages/core/src/plugin/websearch/integration.ts b/packages/core/src/plugin/websearch/integration.ts new file mode 100644 index 0000000000..715e7e2bad --- /dev/null +++ b/packages/core/src/plugin/websearch/integration.ts @@ -0,0 +1,11 @@ +export * as WebSearchIntegration from "./integration.js" + +import type { IntegrationDomain } from "@opencode/plugin/effect/integration" + +export function register(integration: Pick, id: string, name: string, env: string) { + return integration.transform((editor) => { + editor.update(id, (integration) => (integration.name = name)) + editor.method.update({ integrationID: id, method: { type: "key" } }) + editor.method.update({ integrationID: id, method: { type: "env", names: [env] } }) + }) +} diff --git a/packages/core/src/plugin/websearch/parallel.ts b/packages/core/src/plugin/websearch/parallel.ts index 5b858aff52..29fd3ca5fb 100644 --- a/packages/core/src/plugin/websearch/parallel.ts +++ b/packages/core/src/plugin/websearch/parallel.ts @@ -5,6 +5,7 @@ import { Effect, Schema, Scope } from "effect" import { HttpClient } from "effect/http" import { App } from "../../app.js" import { WebSearchMcp } from "./mcp.js" +import { WebSearchIntegration } from "./integration.js" export const endpoint = "https://search.parallel.ai/mcp" @@ -52,17 +53,7 @@ export const Plugin = define({ id: "opencode.websearch.parallel", effect: Effect.fn("WebSearchParallel.Plugin")(function* (ctx) { const http = yield* HttpClient.HttpClient - yield* ctx.integration.transform((editor) => { - editor.update("parallel", (integration) => (integration.name = "Parallel")) - editor.method.update({ - integrationID: "parallel", - method: { type: "key" }, - }) - editor.method.update({ - integrationID: "parallel", - method: { type: "env", names: ["PARALLEL_API_KEY"] }, - }) - }) + yield* WebSearchIntegration.register(ctx.integration, "parallel", "Parallel", "PARALLEL_API_KEY") yield* ctx.websearch.transform((editor) => { editor.add({ id: "parallel", diff --git a/packages/core/src/plugin/websearch/tavily.ts b/packages/core/src/plugin/websearch/tavily.ts index 54c0477401..43e5da9efc 100644 --- a/packages/core/src/plugin/websearch/tavily.ts +++ b/packages/core/src/plugin/websearch/tavily.ts @@ -4,6 +4,7 @@ import { define } from "@opencode/plugin/effect/plugin" import { Duration, Effect, Schema, Scope } from "effect" import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/http" import { App } from "../../app.js" +import { WebSearchIntegration } from "./integration.js" export const endpoint = "https://api.tavily.com/search" @@ -28,17 +29,7 @@ export const Plugin = define({ id: "opencode.websearch.tavily", effect: Effect.fn("WebSearchTavily.Plugin")(function* (ctx) { const http = yield* HttpClient.HttpClient - yield* ctx.integration.transform((editor) => { - editor.update("tavily", (integration) => (integration.name = "Tavily")) - editor.method.update({ - integrationID: "tavily", - method: { type: "key" }, - }) - editor.method.update({ - integrationID: "tavily", - method: { type: "env", names: ["TAVILY_API_KEY"] }, - }) - }) + yield* WebSearchIntegration.register(ctx.integration, "tavily", "Tavily", "TAVILY_API_KEY") yield* ctx.websearch.transform((editor) => { editor.add({ id: "tavily", diff --git a/packages/core/src/plugin/websearch/tinyfish.ts b/packages/core/src/plugin/websearch/tinyfish.ts index 604540f373..b8af6f9376 100644 --- a/packages/core/src/plugin/websearch/tinyfish.ts +++ b/packages/core/src/plugin/websearch/tinyfish.ts @@ -5,6 +5,7 @@ import { Effect, Option, Schema, Scope } from "effect" import { HttpClient } from "effect/http" import { App } from "../../app.js" import { WebSearchMcp } from "./mcp.js" +import { WebSearchIntegration } from "./integration.js" export const endpoint = "https://agent.tinyfish.ai/mcp" @@ -33,17 +34,7 @@ export const Plugin = define({ id: "opencode.websearch.tinyfish", effect: Effect.fn("WebSearchTinyFish.Plugin")(function* (ctx) { const http = yield* HttpClient.HttpClient - yield* ctx.integration.transform((editor) => { - editor.update("tinyfish", (integration) => (integration.name = "TinyFish")) - editor.method.update({ - integrationID: "tinyfish", - method: { type: "key" }, - }) - editor.method.update({ - integrationID: "tinyfish", - method: { type: "env", names: ["TINYFISH_API_KEY"] }, - }) - }) + yield* WebSearchIntegration.register(ctx.integration, "tinyfish", "TinyFish", "TINYFISH_API_KEY") yield* ctx.websearch.transform((editor) => { editor.add({ id: "tinyfish", diff --git a/packages/core/src/rpc.ts b/packages/core/src/rpc.ts index 4e4c054ecd..3b5a5fbc0a 100644 --- a/packages/core/src/rpc.ts +++ b/packages/core/src/rpc.ts @@ -12,6 +12,7 @@ import { Context, Deferred, Effect, JsonSchema, Layer, Schema, SchemaRepresentat import { Bus } from "./bus.js" import { Location } from "./location.js" import { optional, statics } from "./schema.js" +import { invokeStandardValidator } from "./util/standard-schema.js" export interface Interface { readonly register: RpcDomain["register"] @@ -204,9 +205,7 @@ function parse(schema: Tool.ValueSchema, value: unknown): Effect.Effect schema["~standard"].validate(value), catch: (cause) => cause }) - const result = - parsed instanceof Promise ? yield* Effect.tryPromise({ try: () => parsed, catch: (cause) => cause }) : parsed + const result = yield* invokeStandardValidator(schema, value) if (result.issues) return yield* Effect.fail(new Error(result.issues.map((issue) => issue.message).join("\n"))) return result.value }) diff --git a/packages/core/src/shell.ts b/packages/core/src/shell.ts index 118146ef4a..8869e6246c 100644 --- a/packages/core/src/shell.ts +++ b/packages/core/src/shell.ts @@ -41,7 +41,6 @@ type CreateInput = Shell.CreateInput & { type Active = { // Immutable snapshot; lifecycle updates replace it via immer `produce`. info: Info - file: string size: number newlines: number // Resolves with the terminal Info once the command exits, times out, or is killed. A wait @@ -165,7 +164,7 @@ const layer = () => if (command.timeoutFiber) yield* Fiber.interrupt(command.timeoutFiber) // Unblock any wait still pending when the command is removed before it terminated. yield* Deferred.fail(command.done, new NotFoundError({ id })) - yield* Effect.promise(() => unlink(command.file).catch(() => {})) + yield* Effect.promise(() => unlink(command.info.file).catch(() => {})) yield* bus.publish(Shell.Event.Deleted, { id }) }) @@ -206,7 +205,7 @@ const layer = () => const bytesRead = yield* Effect.promise( () => new Promise((resolve) => { - const stream = createReadStream(command.file, { start, end: start + length - 1 }) + const stream = createReadStream(command.info.file, { start, end: start + length - 1 }) let offset = 0 stream.on("data", (chunk: string | Buffer) => { const bytes = Buffer.from(chunk) @@ -313,7 +312,6 @@ const layer = () => info: produce(info, (draft) => { draft.pid = handle.pid }), - file, size: 0, newlines: 0, done: Deferred.makeUnsafe(), diff --git a/packages/core/src/tool/runtime.ts b/packages/core/src/tool/runtime.ts index d1c5e29caf..79bf9c5e7d 100644 --- a/packages/core/src/tool/runtime.ts +++ b/packages/core/src/tool/runtime.ts @@ -4,6 +4,7 @@ import { Tool } from "@opencode/schema/tool" import type { StandardJSONSchemaV1, StandardSchemaV1 } from "@standard-schema/spec" import { Cache, Effect, JsonPointer, JsonSchema, Schema, SchemaIssue, SchemaRepresentation } from "effect" import { $ZodType, toJSONSchema } from "zod/v4/core" +import { invokeStandardValidator } from "../util/standard-schema.js" const formatEffectIssues = SchemaIssue.makeFormatterStandardSchemaV1() @@ -143,10 +144,7 @@ const validateStandard = ( schema: StandardSchemaV1, value: unknown, ): Effect.Effect> => - Effect.gen(function* () { - const result = yield* Effect.try({ try: () => schema["~standard"].validate(value), catch: (error) => error }) - return result instanceof Promise ? yield* Effect.tryPromise({ try: () => result, catch: (error) => error }) : result - }).pipe( + invokeStandardValidator(schema, value).pipe( Effect.match({ onFailure: (error) => ({ issues: [{ message: error instanceof Error ? error.message : String(error) }] }), onSuccess: (result) => result, diff --git a/packages/core/src/util/standard-schema.ts b/packages/core/src/util/standard-schema.ts new file mode 100644 index 0000000000..44c51cb6de --- /dev/null +++ b/packages/core/src/util/standard-schema.ts @@ -0,0 +1,7 @@ +import type { StandardSchemaV1 } from "@standard-schema/spec" +import { Effect } from "effect" + +export const invokeStandardValidator = Effect.fnUntraced(function* (schema: StandardSchemaV1, value: unknown) { + const result = yield* Effect.try({ try: () => schema["~standard"].validate(value), catch: (error) => error }) + return result instanceof Promise ? yield* Effect.tryPromise({ try: () => result, catch: (error) => error }) : result +}) diff --git a/packages/core/src/workspace.ts b/packages/core/src/workspace.ts index 04626e74a3..1b403349d2 100644 --- a/packages/core/src/workspace.ts +++ b/packages/core/src/workspace.ts @@ -7,7 +7,7 @@ import { Clock, Context, Deferred, Duration, Effect, Exit, FiberSet, Layer, Ref, import { systemError } from "effect/PlatformError" import { make } from "effect/process/ChildProcessSpawner" import type { EnvironmentDriver } from "./environment/driver.js" -import { execDefaults } from "./environment/exec-defaults.js" +import { makeFiles } from "./environment/make-files.js" import { Failed, type Files, type FilesImpl } from "./environment/files.js" import { Database } from "./database/database.js" import { KeyedMutex } from "./effect/keyed-mutex.js" @@ -169,10 +169,7 @@ const layer = (options: Options) => const connection: Connection = { driver, environment, - files: { - ...execDefaults(environment.spawner), - ...environment.overrides, - }, + files: makeFiles(environment), saveBinding: persistBinding, lastActivity: yield* Ref.make(now), active: yield* Ref.make(0), diff --git a/packages/core/test/config/reload-scheduler.test.ts b/packages/core/test/config/reload-scheduler.test.ts new file mode 100644 index 0000000000..4438ace55d --- /dev/null +++ b/packages/core/test/config/reload-scheduler.test.ts @@ -0,0 +1,83 @@ +import { describe, expect } from "bun:test" +import { ConfigReload } from "@opencode/core/config/reload" +import { Deferred, Effect, Fiber, PubSub, Stream } from "effect" +import { advance, drain } from "../lib/clock" +import { it } from "../lib/effect" + +describe("ConfigReload.schedule", () => { + it.effect("eagerly subscribes both sources and coalesces them independently for each caller", () => + Effect.gen(function* () { + const files = yield* PubSub.unbounded() + const config = yield* PubSub.unbounded() + const reloads = { agent: 0, command: 0 } + const triggers = [Stream.fromPubSub(files), Stream.fromPubSub(config)] + yield* ConfigReload.schedule( + triggers, + Effect.sync(() => reloads.agent++), + ) + yield* ConfigReload.schedule( + triggers, + Effect.sync(() => reloads.command++), + ) + + // No yield after startup: both sources must already have subscribers. + yield* PubSub.publish(files, undefined) + yield* PubSub.publish(config, undefined) + yield* advance(() => reloads.agent === 1 && reloads.command === 1) + yield* drain + expect(reloads).toEqual({ agent: 1, command: 1 }) + }), + ) + + it.effect("serializes reloads while retaining triggers arriving during a reload", () => + Effect.gen(function* () { + const changes = yield* PubSub.unbounded() + const release = yield* Deferred.make() + const reloads: number[] = [] + yield* ConfigReload.schedule( + [Stream.fromPubSub(changes)], + Effect.gen(function* () { + reloads.push(reloads.length + 1) + if (reloads.length === 1) yield* Deferred.await(release) + }), + ) + yield* PubSub.publish(changes, undefined) + yield* advance(() => reloads.length === 1) + yield* PubSub.publish(changes, undefined) + yield* PubSub.publish(changes, undefined) + yield* drain + expect(reloads).toEqual([1]) + + yield* Deferred.succeed(release, undefined) + yield* advance(() => reloads.length === 2) + yield* drain + expect(reloads).toEqual([1, 2]) + }), + ) + + it.effect("cancels source subscriptions and an active reload when its scope closes", () => + Effect.gen(function* () { + const changes = yield* PubSub.unbounded() + const ready = yield* Deferred.make() + const state = { started: 0, cancelled: false, unsubscribed: false } + const owner = yield* Effect.gen(function* () { + yield* ConfigReload.schedule( + [Stream.fromPubSub(changes).pipe(Stream.ensuring(Effect.sync(() => (state.unsubscribed = true))))], + Effect.sync(() => state.started++).pipe( + Effect.andThen(Effect.never), + Effect.ensuring(Effect.sync(() => (state.cancelled = true))), + ), + ) + yield* Deferred.succeed(ready, undefined) + yield* Effect.never + }).pipe(Effect.scoped, Effect.forkScoped) + yield* Deferred.await(ready) + yield* PubSub.publish(changes, undefined) + yield* advance(() => state.started === 1) + yield* Fiber.interrupt(owner) + yield* PubSub.publish(changes, undefined) + yield* drain + expect(state).toEqual({ started: 1, cancelled: true, unsubscribed: true }) + }), + ) +}) diff --git a/packages/core/test/filesystem/filesystem.test.ts b/packages/core/test/filesystem/filesystem.test.ts index e24dc33940..32c8cd8019 100644 --- a/packages/core/test/filesystem/filesystem.test.ts +++ b/packages/core/test/filesystem/filesystem.test.ts @@ -338,25 +338,6 @@ describe("FSUtil", () => { ) }) - describe("globUp", () => { - it( - "finds files walking up directories", - Effect.gen(function* () { - const fs = yield* FSUtil.Service - const filesys = yield* FileSystem.FileSystem - const tmp = yield* filesys.makeTempDirectoryScoped() - yield* filesys.writeFileString(path.join(tmp, "root.md"), "root") - const child = path.join(tmp, "a", "b") - yield* filesys.makeDirectory(child, { recursive: true }) - yield* filesys.writeFileString(path.join(child, "leaf.md"), "leaf") - - const result = yield* fs.globUp("*.md", child, tmp) - expect(result).toContain(path.join(child, "leaf.md")) - expect(result).toContain(path.join(tmp, "root.md")) - }), - ) - }) - describe("built-in passthrough", () => { it( "exists works", diff --git a/packages/core/test/formatter.test.ts b/packages/core/test/formatter.test.ts index 1530d93992..2779399883 100644 --- a/packages/core/test/formatter.test.ts +++ b/packages/core/test/formatter.test.ts @@ -306,6 +306,37 @@ describe("Formatter", () => { ), ) + it.live("retries disabled formatter discovery and caches the enabled command", () => + withFormatter(false, (formatter, directory) => + Effect.gen(function* () { + const discovery = { calls: 0 } + const available = path.join(directory, "formatter-available") + yield* formatter.transform((editor) => { + editor.set({ + name: "retry", + extensions: [".retry"], + enabled: Effect.gen(function* () { + discovery.calls++ + if (!(yield* Effect.promise(() => Bun.file(available).exists()))) return false + return append("'A'") + }), + }) + }) + const file = path.join(directory, "test.retry") + yield* Effect.promise(() => fs.writeFile(file, "x")) + expect(yield* formatter.file(file)).toBe(false) + expect(yield* Effect.promise(() => fs.readFile(file, "utf8"))).toBe("x") + + yield* Effect.promise(() => fs.writeFile(available, "")) + expect(yield* formatter.file(file)).toBe(true) + yield* Effect.promise(() => fs.unlink(available)) + expect(yield* formatter.file(file)).toBe(true) + expect(discovery.calls).toBe(2) + expect(yield* Effect.promise(() => fs.readFile(file, "utf8"))).toBe("xAA") + }), + ), + ) + it.live("rebuilds formatter state and clears resolved commands", () => withFormatter(false, (formatter, directory) => Effect.gen(function* () { diff --git a/packages/core/test/integration.test.ts b/packages/core/test/integration.test.ts index b5f879cafb..c0d0a2ce5e 100644 --- a/packages/core/test/integration.test.ts +++ b/packages/core/test/integration.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test" -import { Cause, Clock, Duration, Effect, Exit, Fiber, Layer, Scope, Stream } from "effect" +import { Cause, Clock, Deferred, Duration, Effect, Exit, Fiber, Layer, Schema, Scope, Stream } from "effect" import { TestClock } from "effect/testing" import { Credential } from "@opencode/core/credential" import { AppNodeBuilder } from "@opencode/core/effect/app-node-builder" @@ -522,6 +522,157 @@ describe("Integration", () => { }), ) + it.effect("expires commands and retains expired records for one minute", () => + Effect.gen(function* () { + const integrations = yield* Integration.Service + const credentials = yield* Credential.Service + const integrationID = Integration.ID.make("company") + const methodID = Integration.MethodID.make("login") + yield* integrations.transform((editor) => + editor.method.update({ + integrationID, + method: { + id: methodID, + type: "command", + label: "Log in", + command: [ + process.execPath, + "-e", + 'process.stderr.write("ready"); await Bun.sleep(60000); process.stdout.write("too late")', + ], + }, + }), + ) + const attempt = yield* integrations.command.connect({ integrationID, methodID }) + const input = { integrationID, attemptID: attempt.attemptID } + yield* eventually( + integrations.command.status(input), + (status) => status.status === "pending" && status.message === "ready", + ) + yield* TestClock.adjust(Duration.minutes(10)) + expect(yield* integrations.command.status(input)).toEqual({ status: "expired", time: attempt.time }) + yield* integrations.command.cancel(input) + yield* TestClock.adjust(Duration.seconds(59)) + expect(yield* integrations.command.status(input)).toEqual({ status: "expired", time: attempt.time }) + yield* TestClock.adjust(Duration.seconds(1)) + expect(yield* integrations.command.status(input).pipe(Effect.flip)).toBeInstanceOf( + Integration.AttemptNotFoundError, + ) + expect(yield* credentials.list(integrationID)).toEqual([]) + }), + ) + + for (const status of ["complete", "failed"] as const) { + it.effect(`retains ${status} command records and their payload until the scrub deadline`, () => + Effect.gen(function* () { + const integrations = yield* Integration.Service + const integrationID = Integration.ID.make("company") + const methodID = Integration.MethodID.make("login") + yield* integrations.transform((editor) => + editor.method.update({ + integrationID, + method: { + id: methodID, + type: "command", + label: "Log in", + command: [ + process.execPath, + "-e", + status === "complete" + ? 'process.stdout.write("secret")' + : 'process.stderr.write("login failed"); process.exit(1)', + ], + }, + }), + ) + const attempt = yield* integrations.command.connect({ integrationID, methodID }) + const input = { integrationID, attemptID: attempt.attemptID } + const terminal = + status === "failed" ? { status, message: "login failed", time: attempt.time } : { status, time: attempt.time } + expect(yield* eventually(integrations.command.status(input), (value) => value.status === status)).toEqual( + terminal, + ) + yield* TestClock.adjust(Duration.seconds(59)) + expect(yield* integrations.command.status(input)).toEqual(terminal) + yield* TestClock.adjust(Duration.seconds(1)) + expect(yield* integrations.command.status(input).pipe(Effect.flip)).toBeInstanceOf( + Integration.AttemptNotFoundError, + ) + }), + ) + } + + for (const type of ["oauth", "command"] as const) { + it.effect(`does not expire or cancel ${type} attempts while credential persistence is in flight`, () => + Effect.gen(function* () { + const integrations = yield* Integration.Service + const credentials = yield* Credential.Service + const bus = yield* Bus.Service + const integrationID = Integration.ID.make("company") + const methodID = Integration.MethodID.make("login") + const persisting = yield* Deferred.make() + const release = yield* Deferred.make() + yield* bus.listen((event) => { + if (!Schema.is(Credential.Event.Switched)(event) || event.data.integrationID !== integrationID) + return Effect.void + return Deferred.succeed(persisting, undefined).pipe(Effect.andThen(Deferred.await(release))) + }) + yield* integrations.transform((editor) => + editor.method.update( + type === "command" + ? { + integrationID, + method: { + id: methodID, + type: "command", + label: "Log in", + command: [process.execPath, "-e", 'process.stdout.write("secret")'], + }, + } + : { + integrationID, + method: { id: methodID, type: "oauth", label: "Log in" }, + authorize: () => + Effect.succeed({ + mode: "auto" as const, + url: "https://example.com/authorize", + instructions: "Sign in", + callback: Effect.succeed( + Credential.OAuth.make({ + type: "oauth", + methodID, + access: "access", + refresh: "refresh", + expires: 1, + }), + ), + }), + }, + ), + ) + const attempt = yield* integrations[type].connect({ integrationID, methodID }) + const input = { integrationID, attemptID: attempt.attemptID } + // Wait on the real credential event rather than substituting the persistence implementation. + yield* eventually(Deferred.isDone(persisting), (done) => done) + yield* integrations[type].cancel(input) + yield* TestClock.adjust(Duration.minutes(11)) + expect(yield* integrations[type].status(input)).toEqual({ status: "pending", time: attempt.time }) + yield* Deferred.succeed(release, undefined) + expect(yield* eventually(integrations[type].status(input), (value) => value.status === "complete")).toEqual({ + status: "complete", + time: attempt.time, + }) + expect(yield* credentials.list(integrationID)).toHaveLength(1) + yield* TestClock.adjust(Duration.seconds(59)) + expect(yield* integrations[type].status(input)).toEqual({ status: "complete", time: attempt.time }) + yield* TestClock.adjust(Duration.seconds(1)) + expect(yield* integrations[type].status(input).pipe(Effect.flip)).toBeInstanceOf( + Integration.AttemptNotFoundError, + ) + }), + ) + } + it.effect("uses provider-defined OAuth attempt expirations", () => Effect.gen(function* () { const integrations = yield* Integration.Service diff --git a/packages/core/test/persistent-pty-daemon.test.ts b/packages/core/test/persistent-pty-daemon.test.ts index a9657376f0..d36ac80c96 100644 --- a/packages/core/test/persistent-pty-daemon.test.ts +++ b/packages/core/test/persistent-pty-daemon.test.ts @@ -6,11 +6,115 @@ import net from "node:net" import os from "node:os" import path from "node:path" import { Effect } from "effect" +import { Global } from "@opencode/util/global" +import { Pty } from "@opencode/schema/pty" +import { Session } from "@opencode/schema/session" +import { AppNodeBuilder } from "../src/effect/app-node-builder" +import { PersistentPty } from "../src/persistent-pty/index" import { makeDaemonTransport } from "../src/persistent-pty/daemon" +import { tempGlobalLayer } from "./fixture/global" import { it } from "./lib/effect" const pong = { type: "pong", instance_id: "test", pid: process.pid, protocol: 7 } +const operations = [ + { + op: "resize", + run: (service: PersistentPty.Interface, id: Pty.ID) => service.resize(id, 100, 30), + payload: { attachment_id: null, cols: 100, rows: 30 }, + }, + { + op: "control", + run: (service: PersistentPty.Interface, id: Pty.ID) => service.control(id, "controller", 100, 30), + payload: { attachment_id: "controller", cols: 100, rows: 30 }, + }, + { + op: "input", + run: (service: PersistentPty.Interface, id: Pty.ID) => + service.input(id, "controller", 100, 30, new Uint8Array([0, 128, 255])), + payload: { attachment_id: "controller", cols: 100, rows: 30, data_base64: "AID/" }, + }, +] + +for (const operation of operations) { + for (const outcome of ["success", "missing", "error", "unexpected"] as const) { + it.live(`service ${operation.op} ${outcome} preserves acknowledgment-based selection`, () => + Effect.gen(function* () { + const runtime = yield* temporaryRuntime() + const socketPath = path.join(runtime.directory, "daemon.sock") + const requests: Record[] = [] + const sessionID = Session.ID.make("ses_test") + const first = Pty.ID.make("pty_persistent_1") + const second = Pty.ID.make("pty_persistent_2") + yield* listen(socketPath, (_socket, request) => { + if (request.op === "ping") return pong + requests.push(request) + if (request.op === "list") + return { + type: "terminals", + terminals: (outcome === "missing" ? [1] : [1, 2]).map((id) => ({ ...terminal(id), group_id: sessionID })), + } + if (request.op === "read_rows") + return { + type: "rows", + terminal: { ...terminal(Number(request.id)), group_id: sessionID }, + lines: ["selected"], + cursor_x: 0, + cursor_y: 0, + } + if (request.id === 1) return { type: "ok" } + if (outcome === "error") return { type: "error", message: "controller rejected" } + if (outcome === "unexpected") return { type: "terminals", terminals: [] } + return { type: "ok" } + }) + yield* Effect.promise(() => writeRegistration(runtime.directory, socketPath)) + yield* Effect.gen(function* () { + const service = yield* PersistentPty.Service + expect(yield* service.read(sessionID)).toBeNull() + yield* service.resize(first, 80, 24) + expect((yield* service.read(sessionID))?.ptyID).toBe(first) + requests.length = 0 + + if (outcome === "success") expect(yield* operation.run(service, second)).toBeUndefined() + if (outcome !== "success") { + const error = yield* Effect.flip(operation.run(service, second)) + expect(error).toMatchObject( + outcome === "missing" + ? { _tag: "PersistentPty.NotFoundError", ptyID: second } + : { + _tag: "PersistentPty.UnavailableError", + message: + outcome === "error" ? "controller rejected" : "unexpected opencode-pty response: terminals", + }, + ) + } + expect(requests).toEqual( + outcome === "missing" + ? [{ op: "list" }] + : [{ op: "list" }, { op: operation.op, id: 2, ...operation.payload }], + ) + expect((yield* service.read(sessionID))?.ptyID).toBe(outcome === "success" ? second : first) + }).pipe( + Effect.provide( + AppNodeBuilder.build( + PersistentPty.configured({ + root: runtime.root, + handoff: { + directory: runtime.directory, + instanceID: "test", + ticket: "ticket", + expiresAt: Date.now() + 60_000, + }, + }), + [Global.node.replace(tempGlobalLayer)], + ), + ), + ) + }), + ) + } +} + it.live("rediscovers a same-protocol daemon after its registration rotates", () => Effect.gen(function* () { const { root, directory } = yield* temporaryRuntime() diff --git a/packages/core/test/plugin/fixtures/provider-noncallable-factory.ts b/packages/core/test/plugin/fixtures/provider-noncallable-factory.ts new file mode 100644 index 0000000000..dfdbea5d3b --- /dev/null +++ b/packages/core/test/plugin/fixtures/provider-noncallable-factory.ts @@ -0,0 +1,4 @@ +export const createA = true +export function createZ() { + throw new Error("Must not skip the first create export") +} diff --git a/packages/core/test/plugin/provider-dynamic.test.ts b/packages/core/test/plugin/provider-dynamic.test.ts index c29767ba13..7a27d1370c 100644 --- a/packages/core/test/plugin/provider-dynamic.test.ts +++ b/packages/core/test/plugin/provider-dynamic.test.ts @@ -15,6 +15,7 @@ import { testEffect } from "../lib/effect" import { PluginTestLayer } from "./fixture" const fixtureProvider = new URL("./fixtures/provider-factory.ts", import.meta.url).href +const noncallableProvider = new URL("./fixtures/provider-noncallable-factory.ts", import.meta.url).href const it = testEffect(PluginTestLayer) const itWithAISDK = testEffect(Layer.mergeAll(PluginTestLayer, AppNodeBuilder.build(AISDK.node))) @@ -50,6 +51,47 @@ function tempPackage(source: string) { } describe("DynamicProviderPlugin", () => { + it.effect("rejects the first create export when it is not callable", () => + Effect.gen(function* () { + const aisdk = yield* AISDK.Service + yield* addPlugin() + const exit = yield* aisdk + .runSDK({ + model: Model.Info.default(Provider.ID.make("custom"), Model.ID.make("test-model")), + package: noncallableProvider, + options: {}, + }) + .pipe(Effect.exit) + expect(exit._tag).toBe("Failure") + if (exit._tag !== "Failure") return + expect(Cause.squash(exit.cause)).toEqual( + new Error(`Package ${noncallableProvider} provider factory export is not callable`), + ) + }), + ) + + itWithAISDK.effect("wraps a noncallable selected factory as an AISDK init error", () => + Effect.gen(function* () { + const aisdk = yield* AISDK.Service + yield* addPlugin() + const result = yield* aisdk + .language( + Model.Info.make({ + ...Model.Info.default(Provider.ID.make("custom"), Model.ID.make("test-model")), + package: Provider.aisdk(noncallableProvider), + }), + ) + .pipe(Effect.result) + expect(result._tag).toBe("Failure") + if (result._tag !== "Failure") return + expect(result.failure).toBeInstanceOf(AISDK.InitError) + expect(result.failure.providerID).toBe(Provider.ID.make("custom")) + expect(result.failure.cause).toEqual( + new Error(`Package ${noncallableProvider} provider factory export is not callable`), + ) + }), + ) + it.effect("creates an SDK from a provider factory export", () => Effect.gen(function* () { const aisdk = yield* AISDK.Service diff --git a/packages/core/test/plugin/provider-gitlab-oauth.test.ts b/packages/core/test/plugin/provider-gitlab-oauth.test.ts index 7abed9c5ba..5b81c752c8 100644 --- a/packages/core/test/plugin/provider-gitlab-oauth.test.ts +++ b/packages/core/test/plugin/provider-gitlab-oauth.test.ts @@ -1,5 +1,5 @@ import { describe, expect } from "bun:test" -import { Clock, Effect, Schedule } from "effect" +import { Clock, Deferred, Effect, Fiber, Schedule } from "effect" import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/http" import { Credential } from "@opencode/core/credential" import { Integration } from "@opencode/core/integration" @@ -390,6 +390,57 @@ describe("GitLabPlugin OAuth", () => { ) } + for (const cancel of [false, true]) { + it.live(`waits for the token exchange and consumes the callback once (cancel: ${cancel})`, () => + Effect.gen(function* () { + const test = yield* fixture() + yield* test.integrations.connection.key({ integrationID, key: "previous-key" }) + const previous = yield* test.integrations.connection.active(integrationID) + const saved = yield* test.credentials.list(integrationID) + const login = yield* test.connect + const started = yield* Deferred.make() + const token = yield* Deferred.make() + test.replies.push(Deferred.succeed(started, undefined).pipe(Effect.andThen(Deferred.await(token)))) + const page = yield* Effect.tryPromise(() => fetch(login.callback, { headers: { Connection: "close" } })).pipe( + Effect.exit, + Effect.forkScoped, + ) + yield* Deferred.await(started) + expect( + (yield* test.integrations.oauth.status({ integrationID, attemptID: login.attempt.attemptID })).status, + ).toBe("pending") + expect(yield* test.credentials.list(integrationID)).toEqual(saved) + expect(yield* test.integrations.connection.active(integrationID)).toEqual(previous) + expect((yield* Effect.promise(() => fetch(login.callback, { headers: { Connection: "close" } }))).status).toBe( + 409, + ) + expect(test.requests).toHaveLength(1) + if (cancel) { + yield* test.integrations.oauth.cancel({ integrationID, attemptID: login.attempt.attemptID }) + yield* Deferred.succeed(token, renewal()) + expect((yield* Fiber.join(page).pipe(Effect.timeout("3 seconds")))._tag).toBe("Failure") + expect(yield* test.credentials.list(integrationID)).toEqual(saved) + expect(yield* test.integrations.connection.active(integrationID)).toEqual(previous) + // A new attempt must reclaim GitLab's fixed callback port after cancellation. + const next = yield* test.connect + expect(next.callback.origin).toBe(login.callback.origin) + expect(next.url.searchParams.get("state")).not.toBe(login.url.searchParams.get("state")) + yield* test.integrations.oauth.cancel({ integrationID, attemptID: next.attempt.attemptID }) + expect(test.requests).toHaveLength(1) + return + } + yield* Deferred.succeed(token, renewal()) + const result = yield* Fiber.join(page) + const response = yield* result + expect(response.status).toBe(200) + expect(yield* Effect.promise(() => response.text())).toContain("Authorization successful") + expect((yield* test.status(login.attempt.attemptID)).status).toBe("complete") + expect(yield* test.credentials.list(integrationID)).toHaveLength(saved.length + 1) + expect(test.requests).toHaveLength(1) + }), + ) + } + it.live("reports EADDRINUSE with a clear message when the callback port is occupied", () => Effect.gen(function* () { const { createServer } = yield* Effect.promise(() => import("node:http")) diff --git a/packages/core/test/plugin/provider-sap-ai-core.test.ts b/packages/core/test/plugin/provider-sap-ai-core.test.ts index 18df46c2d0..6219307285 100644 --- a/packages/core/test/plugin/provider-sap-ai-core.test.ts +++ b/packages/core/test/plugin/provider-sap-ai-core.test.ts @@ -1,6 +1,6 @@ import { AISDK } from "@opencode/core/aisdk" import { describe, expect } from "bun:test" -import { Effect } from "effect" +import { Cause, Effect } from "effect" import { Model } from "@opencode/core/model" import { Plugin } from "@opencode/core/plugin" import { PluginHost } from "@opencode/core/plugin/host" @@ -12,6 +12,7 @@ import { testEffect } from "../lib/effect" import { PluginTestLayer } from "./fixture" const fixtureProvider = new URL("./fixtures/provider-factory.ts", import.meta.url).href +const noncallableProvider = new URL("./fixtures/provider-noncallable-factory.ts", import.meta.url).href const it = testEffect(PluginTestLayer) const npm = Npm.Service.of({ add: (name) => Effect.succeed({ directory: "", name }), @@ -36,6 +37,38 @@ function model(providerID: string) { } describe("SapAICorePlugin", () => { + it.effect("rejects the first create export when it is not callable", () => + Effect.gen(function* () { + const aisdk = yield* AISDK.Service + yield* addPlugin() + const exit = yield* aisdk + .runSDK({ model: model("sap-ai-core"), package: noncallableProvider, options: {} }) + .pipe(Effect.exit) + expect(exit._tag).toBe("Failure") + if (exit._tag !== "Failure") return + expect(Cause.squash(exit.cause)).toEqual( + new Error(`Package ${noncallableProvider} provider factory export is not callable`), + ) + }), + ) + + it.effect("wraps a noncallable selected factory as an AISDK init error", () => + Effect.gen(function* () { + const aisdk = yield* AISDK.Service + yield* addPlugin() + const result = yield* aisdk + .language(Model.Info.make({ ...model("sap-ai-core"), package: Provider.aisdk(noncallableProvider) })) + .pipe(Effect.result) + expect(result._tag).toBe("Failure") + if (result._tag !== "Failure") return + expect(result.failure).toBeInstanceOf(AISDK.InitError) + expect(result.failure.providerID).toBe(Provider.ID.make("sap-ai-core")) + expect(result.failure.cause).toEqual( + new Error(`Package ${noncallableProvider} provider factory export is not callable`), + ) + }), + ) + it.effect("copies serviceKey option into AICORE_SERVICE_KEY but keeps SDK options to deployment metadata", () => withEnv( { AICORE_SERVICE_KEY: undefined, AICORE_DEPLOYMENT_ID: "deployment", AICORE_RESOURCE_GROUP: "resource-group" }, diff --git a/packages/core/test/plugin/websearch.test.ts b/packages/core/test/plugin/websearch.test.ts index 775f576df5..6c529c57a3 100644 --- a/packages/core/test/plugin/websearch.test.ts +++ b/packages/core/test/plugin/websearch.test.ts @@ -31,6 +31,35 @@ beforeEach(() => { const it = webSearchIntegrationTest describe("built-in web search providers", () => { + ;[ + { plugin: WebSearchExa.Plugin, id: "exa", name: "Exa", env: "EXA_API_KEY" }, + { plugin: WebSearchFirecrawl.Plugin, id: "firecrawl", name: "Firecrawl", env: "FIRECRAWL_API_KEY" }, + { plugin: WebSearchParallel.Plugin, id: "parallel", name: "Parallel", env: "PARALLEL_API_KEY" }, + { plugin: WebSearchTavily.Plugin, id: "tavily", name: "Tavily", env: "TAVILY_API_KEY" }, + { plugin: WebSearchTinyFish.Plugin, id: "tinyfish", name: "TinyFish", env: "TINYFISH_API_KEY" }, + ].forEach((provider) => { + it.effect(`registers and disposes ${provider.name} authentication in its plugin scope`, () => + Effect.gen(function* () { + const integrations = yield* Integration.Service + const websearch = yield* WebSearch.Service + const integrationID = Integration.ID.make(provider.id) + const registration = { id: WebSearch.ID.make(provider.id), name: provider.name } + expect(yield* integrations.get(integrationID)).toBeUndefined() + yield* Effect.gen(function* () { + yield* provider.plugin.effect( + host({ integration: integrationHost(integrations), websearch: webSearchHost(websearch) }), + ) + const info = yield* integrations.get(integrationID) + expect(info?.id).toBe(integrationID) + expect(info?.name).toBe(provider.name) + expect(info?.methods).toEqual([{ type: "key" }, { type: "env", names: [provider.env] }]) + expect(yield* websearch.providers()).toContainEqual(registration) + }).pipe(Effect.scoped) + expect(yield* integrations.get(integrationID)).toBeUndefined() + expect(yield* websearch.providers()).not.toContainEqual(registration) + }), + ) + }) ;[ WebSearchExa.Plugin, WebSearchParallel.Plugin, diff --git a/packages/core/test/process/process.test.ts b/packages/core/test/process/process.test.ts index cd69f1b7d7..0d366031e0 100644 --- a/packages/core/test/process/process.test.ts +++ b/packages/core/test/process/process.test.ts @@ -83,12 +83,13 @@ describe("AppProcess", () => { "requireSuccess fails on non-zero exit", Effect.gen(function* () { const svc = yield* AppProcess.Service - const error = yield* svc - .run(cmd("-e", "process.exit(1)")) - .pipe(Effect.flatMap(AppProcess.requireSuccess), Effect.flip) + const script = "process.stderr.write('failure\\n'); process.exit(1)" + const error = yield* svc.run(cmd("-e", script)).pipe(Effect.flatMap(AppProcess.requireSuccess), Effect.flip) expect(error).toBeInstanceOf(AppProcess.AppProcessError) expect(error.exitCode).toBe(1) - expect(error.message).toContain("Command failed (exit 1)") + expect(error.command).toBe(`${NODE} -e ${script}`) + expect(error.stderr).toBe("failure\n") + expect(error.message).toBe(`Command failed (exit 1): ${NODE} -e ${script}: failure`) }), ) @@ -96,7 +97,8 @@ describe("AppProcess", () => { "requireSuccess succeeds on exit 0", Effect.gen(function* () { const svc = yield* AppProcess.Service - const result = yield* svc.run(cmd("-e", "process.exit(0)")).pipe(Effect.flatMap(AppProcess.requireSuccess)) + const result = yield* svc.run(cmd("-e", "process.exit(0)")) + expect(yield* AppProcess.requireSuccess(result)).toBe(result) expect(result.exitCode).toBe(0) }), ) @@ -108,11 +110,15 @@ describe("AppProcess", () => { const requireZeroOrOne = AppProcess.requireExitIn([0, 1]) const okZero = yield* svc.run(cmd("-e", "process.exit(0)")).pipe(Effect.flatMap(requireZeroOrOne)) expect(okZero.exitCode).toBe(0) - const okOne = yield* svc.run(cmd("-e", "process.exit(1)")).pipe(Effect.flatMap(requireZeroOrOne)) + const one = yield* svc.run(cmd("-e", "process.exit(1)")) + const okOne = yield* requireZeroOrOne(one) + expect(okOne).toBe(one) expect(okOne.exitCode).toBe(1) const error = yield* svc.run(cmd("-e", "process.exit(2)")).pipe(Effect.flatMap(requireZeroOrOne), Effect.flip) expect(error).toBeInstanceOf(AppProcess.AppProcessError) expect(error.exitCode).toBe(2) + expect(error.command).toBe(`${NODE} -e process.exit(2)`) + expect(error.stderr).toBe("") }), ) diff --git a/packages/core/test/rpc-handler-errors.test.ts b/packages/core/test/rpc-handler-errors.test.ts index 6dad9b0d15..d2985834bb 100644 --- a/packages/core/test/rpc-handler-errors.test.ts +++ b/packages/core/test/rpc-handler-errors.test.ts @@ -3,7 +3,7 @@ import { AppNodeBuilder } from "@opencode/core/effect/app-node-builder" import { Location } from "@opencode/core/location" import { Rpc } from "@opencode/core/rpc" import { AbsolutePath } from "@opencode/core/schema" -import { Effect, Layer, Logger, Schema } from "effect" +import { Effect, Fiber, Layer, Logger, Option, Schema, Stream } from "effect" import { location } from "./fixture/location" import { testEffect } from "./lib/effect" @@ -28,6 +28,116 @@ const Broken = Rpc.define({ events: {}, }) +it.effect("Standard Schema input, output, and declared error transforms run exactly once", () => + Effect.gen(function* () { + const rpc = yield* Rpc.Service + const calls: unknown[] = [] + const schema = { + "~standard": { + version: 1 as const, + vendor: "test", + validate: (value: unknown) => { + calls.push(value) + return { value: `${String(value)}:parsed` } + }, + }, + } + const definition = Rpc.define({ + id: "transformed", + methods: { + success: { input: schema, output: schema }, + failure: { input: schema, output: schema, errors: { known: schema } }, + }, + events: {}, + }) + yield* rpc.register(definition, { + success: (input) => Effect.succeed(input), + failure: (input, context) => Effect.fail(context.error("known", "Known", input)), + }) + expect(yield* rpc.client(definition).success("input")).toBe("input:parsed:parsed") + expect(calls).toEqual(["input", "input:parsed"]) + calls.length = 0 + expect(yield* rpc.client(definition).failure("error").pipe(Effect.flip)).toEqual({ + type: "known", + message: "Known", + data: "error:parsed:parsed", + }) + expect(calls).toEqual(["error", "error:parsed"]) + }), +) + +it.effect("Standard Schema event transforms run once before subscriber reads", () => + Effect.gen(function* () { + const rpc = yield* Rpc.Service + const calls: unknown[] = [] + const definition = Rpc.define({ + id: "transformed-event", + methods: {}, + events: { + changed: { + schema: { + "~standard": { + version: 1, + vendor: "test", + validate: (value: unknown) => { + calls.push(value) + return { value: { parsed: value } } + }, + }, + }, + }, + }, + }) + const registration = yield* rpc.register(definition, {}) + const subscriber = yield* rpc + .client(definition) + .events.subscribe("changed") + .pipe(Stream.runHead, Effect.forkChild({ startImmediately: true })) + yield* registration.events.emit("changed", { raw: true }) + expect(Option.getOrThrow(yield* Fiber.join(subscriber)).data).toEqual({ parsed: { raw: true } }) + expect(calls).toEqual([{ raw: true }]) + }), +) + +for (const mode of ["throw", "reject", "issues"] as const) { + it.effect(`Standard Schema ${mode} preserves RPC input and output failures`, () => + Effect.gen(function* () { + const rpc = yield* Rpc.Service + const schema = { + "~standard": { + version: 1 as const, + vendor: "test", + validate: () => { + if (mode === "issues") return { issues: [{ message: "first" }, { message: "second" }] } + if (mode === "reject") return Promise.reject(new Error("validator failed")) + throw new Error("validator failed") + }, + }, + } + const definition = Rpc.define({ + id: `validator-${mode}`, + methods: { + input: { input: schema, output: Schema.String }, + output: { input: Schema.String, output: schema }, + }, + events: {}, + }) + yield* rpc.register(definition, { + input: () => Effect.die("invalid input must not execute"), + output: (input) => Effect.succeed(input), + }) + expect(yield* rpc.call(definition.id, "input", "raw").pipe(Effect.flip)).toEqual({ + type: "rpc.invalid_input", + message: mode === "issues" ? "first\nsecond" : "validator failed", + }) + expect(yield* rpc.call(definition.id, "output", "raw").pipe(Effect.flip)).toEqual({ + type: "rpc.invalid_output", + message: mode === "issues" ? "first\nsecond" : "validator failed", + }) + }), + ) +} + for (const method of ["dies", "throws", "raw", "undeclared", "invalidError"] as const) { it.effect(`recovers from ${method} through the typed rpc.internal failure`, () => Effect.gen(function* () { diff --git a/packages/core/test/shell-retention.test.ts b/packages/core/test/shell-retention.test.ts index 3f776e4bb2..6a1ad26e38 100644 --- a/packages/core/test/shell-retention.test.ts +++ b/packages/core/test/shell-retention.test.ts @@ -68,7 +68,9 @@ it.live("eviction makes progress past an already-removed shell", () => const shell = yield* Shell.Service const removed = yield* shell.create({ shell: "sh", command: "removed", timeout: 0 }) const finishRemoved = yield* Queue.take(completions) + expect(yield* Effect.promise(() => Bun.file(removed.file).exists())).toBe(true) yield* shell.remove(removed.id) + expect(yield* Effect.promise(() => Bun.file(removed.file).exists())).toBe(false) expect((yield* shell.result(removed)).capture).toBeUndefined() yield* finishRemoved @@ -84,10 +86,15 @@ it.live("eviction makes progress past an already-removed shell", () => expect(yield* shell.get(oldest.id).pipe(Effect.flip)).toBeInstanceOf(Shell.NotFoundError) const survivor = yield* complete + expect(yield* shell.get(survivor.id)).toMatchObject({ status: "exited", file: survivor.file }) + expect(yield* Effect.promise(() => Bun.file(survivor.file).text())).toBe("hello") + expect((yield* shell.output(survivor.id)).output).toBe("hello") expect(yield* shell.result(survivor)).toMatchObject({ info: { status: "exited", exit: 0 }, capture: { output: "hello", truncated: false }, }) + yield* shell.remove(survivor.id) + expect(yield* Effect.promise(() => Bun.file(survivor.file).exists())).toBe(false) }).pipe( Effect.provide( AppNodeBuilder.build(Shell.node, [ diff --git a/packages/core/test/standard-schema.test.ts b/packages/core/test/standard-schema.test.ts new file mode 100644 index 0000000000..0cf728d09f --- /dev/null +++ b/packages/core/test/standard-schema.test.ts @@ -0,0 +1,93 @@ +import { expect, test } from "bun:test" +import type { StandardSchemaV1 } from "@standard-schema/spec" +import { Cause, Effect, Exit, Fiber } from "effect" +import { invokeStandardValidator } from "../src/util/standard-schema" + +test("standard validator invocation is lazy and preserves synchronous results", () => { + const calls: unknown[] = [] + const result = { value: "parsed" } + const schema: StandardSchemaV1 = { + "~standard": { + version: 1, + vendor: "test", + validate: (value) => { + calls.push(value) + return result + }, + }, + } + const validation = invokeStandardValidator(schema, "raw") + expect(calls).toEqual([]) + expect(Effect.runSync(validation)).toBe(result) + expect(calls).toEqual(["raw"]) +}) + +test.each([new Error("validator failed"), "foreign failure", undefined])( + "standard validator preserves synchronous throws and native Promise rejections: %p", + async (error) => { + for (const validate of [ + () => { + throw error + }, + () => Promise.reject(error), + ]) { + expect( + await Effect.runPromise( + Effect.flip(invokeStandardValidator({ "~standard": { version: 1, vendor: "test", validate } }, null)), + ), + ).toBe(error) + } + }, +) + +test("standard validator awaits native Promises without altering issues", async () => { + const result = { issues: [{ message: "invalid", path: ["field"] }] } + expect( + await Effect.runPromise( + invokeStandardValidator( + { "~standard": { version: 1, vendor: "test", validate: () => Promise.resolve(result) } }, + null, + ), + ), + ).toBe(result) +}) + +test("standard validator does not assimilate non-Promise thenables", () => { + const result = { + value: "parsed", + then: () => { + throw new Error("must not await a thenable") + }, + } + expect( + Effect.runSync( + invokeStandardValidator({ "~standard": { version: 1, vendor: "test", validate: () => result } }, null), + ), + ).toBe(result) +}) + +test("pending native Promise validation remains interruptible", async () => { + const started = Promise.withResolvers() + const pending = Promise.withResolvers>() + const fiber = Effect.runFork( + invokeStandardValidator( + { + "~standard": { + version: 1, + vendor: "test", + validate: () => { + started.resolve() + return pending.promise + }, + }, + }, + null, + ), + ) + await started.promise + await Effect.runPromise(Fiber.interrupt(fiber).pipe(Effect.timeout("1 second"))) + const exit = await Effect.runPromise(Fiber.await(fiber)) + expect(Exit.isFailure(exit) && Cause.hasInterruptsOnly(exit.cause)).toBe(true) + pending.resolve({ value: "late" }) + expect(await Effect.runPromise(Fiber.await(fiber))).toBe(exit) +}) diff --git a/packages/core/test/tool-schema.test.ts b/packages/core/test/tool-schema.test.ts index c70bef8df5..3f645e2b32 100644 --- a/packages/core/test/tool-schema.test.ts +++ b/packages/core/test/tool-schema.test.ts @@ -345,6 +345,42 @@ test("portable schema failures become tool failures", async () => { ) }) +test.each(["throw", "reject"])("portable validator %s failures retain tool issue policy", async (mode) => { + const schema = { + "~standard": { + version: 1, + vendor: "test", + validate: () => { + if (mode === "reject") return Promise.reject(new Error("validator failed")) + throw new Error("validator failed") + }, + }, + } + const tool: Info = { + name: "portable", + description: "Portable tool", + input: schema, + execute: () => Effect.die("invalid input must not execute"), + } + expect(await Effect.runPromise(Effect.flip(execute(tool, {}, context)))).toEqual( + new Tool.Error({ + message: + 'Invalid arguments for tool "portable":\n- root: validator failed\n\nArguments provided:\n{}\n\nUpdate the arguments and call the tool again.', + }), + ) + expect( + await Effect.runPromise( + Effect.flip( + execute( + { ...tool, input: Schema.Unknown, output: schema, execute: () => Effect.succeed({ output: "raw" }) }, + {}, + context, + ), + ), + ), + ).toEqual(new Tool.Error({ message: "Tool returned an invalid value for its output schema: validator failed" })) +}) + test("Effect schema failures use normalized input issues", async () => { const tool: Info = { name: "effect", diff --git a/packages/server/src/handlers/fs.ts b/packages/server/src/handlers/fs.ts index 11225d42b7..b33ae2737d 100644 --- a/packages/server/src/handlers/fs.ts +++ b/packages/server/src/handlers/fs.ts @@ -8,53 +8,51 @@ import { Api } from "../api" import { response } from "../location" export const FileSystemHandler = HttpApiBuilder.group(Api, "server.fs", (handlers) => - Effect.gen(function* () { - return handlers - .handleRaw("fs.read", (ctx) => + handlers + .handleRaw("fs.read", (ctx) => + Effect.gen(function* () { + const path = yield* decodeRequestPath(ctx.request.url) + const fs = yield* FileSystem.Service + const file = yield* fs + .read({ path }) + .pipe( + Effect.mapError( + (error) => new FileNotFoundError({ path: error.path, message: `File not found: ${error.path}` }), + ), + ) + return serveFile(ctx.request, file) + }), + ) + .handle("fs.list", (ctx) => + response( Effect.gen(function* () { - const path = yield* decodeRequestPath(ctx.request.url) const fs = yield* FileSystem.Service - const file = yield* fs - .read({ path }) + return yield* fs + .list(ctx.query) .pipe( Effect.mapError( - (error) => new FileNotFoundError({ path: error.path, message: `File not found: ${error.path}` }), + (error) => new FileNotFoundError({ path: error.path, message: `Directory not found: ${error.path}` }), ), ) - return serveFile(ctx.request, file) }), - ) - .handle("fs.list", (ctx) => - response( - Effect.gen(function* () { - const fs = yield* FileSystem.Service - return yield* fs - .list(ctx.query) - .pipe( - Effect.mapError( - (error) => new FileNotFoundError({ path: error.path, message: `Directory not found: ${error.path}` }), - ), - ) - }), - ), - ) - .handle("fs.find", (ctx) => - response( - Effect.gen(function* () { - const fs = yield* FileSystem.Service - return yield* fs.find(ctx.query) - }), - ), - ) - .handle("fs.write", (ctx) => - response( - Effect.gen(function* () { - const fs = yield* FileSystem.Service - return yield* fs.write({ path: ctx.query.path, data: ctx.payload }) - }), - ), - ) - }), + ), + ) + .handle("fs.find", (ctx) => + response( + Effect.gen(function* () { + const fs = yield* FileSystem.Service + return yield* fs.find(ctx.query) + }), + ), + ) + .handle("fs.write", (ctx) => + response( + Effect.gen(function* () { + const fs = yield* FileSystem.Service + return yield* fs.write({ path: ctx.query.path, data: ctx.payload }) + }), + ), + ), ) function decodeRequestPath(url: string) { diff --git a/packages/server/src/handlers/integration.ts b/packages/server/src/handlers/integration.ts index dfdbaa4476..c6accb71f6 100644 --- a/packages/server/src/handlers/integration.ts +++ b/packages/server/src/handlers/integration.ts @@ -25,243 +25,221 @@ const authorize = (effect: Effect.Effect - Effect.gen(function* () { - return handlers - .handle( - "integration.list", - Effect.fn(function* () { - yield* Plugin.awaitActivation - const service = yield* Integration.Service - return yield* response(service.list()) - }), - ) - .handle( - "integration.get", - Effect.fn(function* (ctx) { - const service = yield* Integration.Service - const integration = yield* service.get(ctx.params.integrationID) - if (!integration) - return yield* new IntegrationNotFoundError({ - integrationID: ctx.params.integrationID, - message: `Integration not found: ${ctx.params.integrationID}`, - }) - return yield* response(Effect.succeed(integration)) - }), - ) - .handle( - "integration.wellknown.add", - Effect.fn(function* (ctx) { - const wellknown = yield* WellKnown.Service - const integration = yield* Integration.Service - yield* wellknown - .add(ctx.payload.url) - .pipe( - Effect.mapError( - (error) => new InvalidRequestError({ message: error.message, kind: "well_known_discovery" }), - ), - ) - yield* integration.reload() - return HttpApiSchema.NoContent.make() - }), - ) - .handle( - "integration.connect.key", - Effect.fn(function* (ctx) { - const service = yield* Integration.Service - if (!(yield* service.get(ctx.params.integrationID))) - return yield* new IntegrationNotFoundError({ - integrationID: ctx.params.integrationID, - message: `Integration not found: ${ctx.params.integrationID}`, - }) - yield* authorize( - service.connection.key({ - integrationID: ctx.params.integrationID, - key: ctx.payload.key, - answer: ctx.payload.answer, - label: ctx.payload.label, - }), + handlers + .handle( + "integration.list", + Effect.fn(function* () { + yield* Plugin.awaitActivation + const service = yield* Integration.Service + return yield* response(service.list()) + }), + ) + .handle( + "integration.get", + Effect.fn(function* (ctx) { + const service = yield* Integration.Service + const integration = yield* requireIntegration(service, ctx.params.integrationID) + return yield* response(Effect.succeed(integration)) + }), + ) + .handle( + "integration.wellknown.add", + Effect.fn(function* (ctx) { + const wellknown = yield* WellKnown.Service + const integration = yield* Integration.Service + yield* wellknown + .add(ctx.payload.url) + .pipe( + Effect.mapError( + (error) => new InvalidRequestError({ message: error.message, kind: "well_known_discovery" }), + ), ) - return HttpApiSchema.NoContent.make() - }), - ) - .handle( - "integration.connect.external", - Effect.fn(function* (ctx) { - const service = yield* Integration.Service - if (!(yield* service.get(ctx.params.integrationID))) - return yield* new IntegrationNotFoundError({ - integrationID: ctx.params.integrationID, - message: `Integration not found: ${ctx.params.integrationID}`, - }) - yield* authorize( - service.connection.external({ + yield* integration.reload() + return HttpApiSchema.NoContent.make() + }), + ) + .handle( + "integration.connect.key", + Effect.fn(function* (ctx) { + const service = yield* Integration.Service + yield* requireIntegration(service, ctx.params.integrationID) + yield* authorize( + service.connection.key({ + integrationID: ctx.params.integrationID, + key: ctx.payload.key, + answer: ctx.payload.answer, + label: ctx.payload.label, + }), + ) + return HttpApiSchema.NoContent.make() + }), + ) + .handle( + "integration.connect.external", + Effect.fn(function* (ctx) { + const service = yield* Integration.Service + yield* requireIntegration(service, ctx.params.integrationID) + yield* authorize( + service.connection.external({ + integrationID: ctx.params.integrationID, + methodID: ctx.payload.methodID, + answer: ctx.payload.answer, + label: ctx.payload.label, + }), + ) + return HttpApiSchema.NoContent.make() + }), + ) + .handle( + "integration.oauth.connect", + Effect.fn(function* (ctx) { + const service = yield* Integration.Service + return yield* response( + authorize( + service.oauth.connect({ integrationID: ctx.params.integrationID, methodID: ctx.payload.methodID, answer: ctx.payload.answer, label: ctx.payload.label, }), - ) - return HttpApiSchema.NoContent.make() - }), - ) - .handle( - "integration.oauth.connect", - Effect.fn(function* (ctx) { - const service = yield* Integration.Service - return yield* response( - authorize( - service.oauth.connect({ - integrationID: ctx.params.integrationID, - methodID: ctx.payload.methodID, - answer: ctx.payload.answer, - label: ctx.payload.label, - }), - ), - ) - }), - ) - .handle( - "integration.oauth.status", - Effect.fn(function* (ctx) { - const service = yield* Integration.Service - if (!(yield* service.get(ctx.params.integrationID))) - return yield* new IntegrationNotFoundError({ - integrationID: ctx.params.integrationID, - message: `Integration not found: ${ctx.params.integrationID}`, - }) - return yield* response( - service.oauth - .status({ - integrationID: ctx.params.integrationID, - attemptID: ctx.params.attemptID, - }) - .pipe( - Effect.mapError( - (error) => - new IntegrationAttemptNotFoundError({ - integrationID: error.integrationID, - attemptID: error.attemptID, - message: `OAuth attempt not found: ${error.attemptID}`, - }), - ), - ), - ) - }), - ) - .handle( - "integration.oauth.complete", - Effect.fn(function* (ctx) { - const service = yield* Integration.Service - if (!(yield* service.get(ctx.params.integrationID))) - return yield* new IntegrationNotFoundError({ - integrationID: ctx.params.integrationID, - message: `Integration not found: ${ctx.params.integrationID}`, - }) - yield* service.oauth - .complete({ + ), + ) + }), + ) + .handle( + "integration.oauth.status", + Effect.fn(function* (ctx) { + const service = yield* Integration.Service + yield* requireIntegration(service, ctx.params.integrationID) + return yield* response( + service.oauth + .status({ integrationID: ctx.params.integrationID, attemptID: ctx.params.attemptID, - code: ctx.payload.code, }) .pipe( - Effect.mapError((error) => { - if (error._tag === "Integration.AttemptNotFound") - return new IntegrationAttemptNotFoundError({ + Effect.mapError( + (error) => + new IntegrationAttemptNotFoundError({ integrationID: error.integrationID, attemptID: error.attemptID, message: `OAuth attempt not found: ${error.attemptID}`, - }) - return new InvalidRequestError({ - message: - error._tag === "Integration.CodeRequired" - ? "Authorization code is required" - : "Authentication failed", - kind: - error._tag === "Integration.CodeRequired" - ? "integration_code_required" - : "integration_authorization", - }) - }), - ) - return HttpApiSchema.NoContent.make() - }), - ) - .handle( - "integration.oauth.cancel", - Effect.fn(function* (ctx) { - const service = yield* Integration.Service - yield* service.oauth.cancel({ + }), + ), + ), + ) + }), + ) + .handle( + "integration.oauth.complete", + Effect.fn(function* (ctx) { + const service = yield* Integration.Service + yield* requireIntegration(service, ctx.params.integrationID) + yield* service.oauth + .complete({ integrationID: ctx.params.integrationID, attemptID: ctx.params.attemptID, + code: ctx.payload.code, }) - return HttpApiSchema.NoContent.make() - }), - ) - .handle( - "integration.command.connect", - Effect.fn(function* (ctx) { - const service = yield* Integration.Service - const integration = yield* service.get(ctx.params.integrationID) - if (!integration) - return yield* new IntegrationNotFoundError({ - integrationID: ctx.params.integrationID, - message: `Integration not found: ${ctx.params.integrationID}`, - }) - if (!integration.methods.some((method) => method.type === "command" && method.id === ctx.payload.methodID)) - return yield* new IntegrationMethodNotFoundError({ + .pipe( + Effect.mapError((error) => { + if (error._tag === "Integration.AttemptNotFound") + return new IntegrationAttemptNotFoundError({ + integrationID: error.integrationID, + attemptID: error.attemptID, + message: `OAuth attempt not found: ${error.attemptID}`, + }) + return new InvalidRequestError({ + message: + error._tag === "Integration.CodeRequired" + ? "Authorization code is required" + : "Authentication failed", + kind: + error._tag === "Integration.CodeRequired" ? "integration_code_required" : "integration_authorization", + }) + }), + ) + return HttpApiSchema.NoContent.make() + }), + ) + .handle( + "integration.oauth.cancel", + Effect.fn(function* (ctx) { + const service = yield* Integration.Service + yield* service.oauth.cancel({ + integrationID: ctx.params.integrationID, + attemptID: ctx.params.attemptID, + }) + return HttpApiSchema.NoContent.make() + }), + ) + .handle( + "integration.command.connect", + Effect.fn(function* (ctx) { + const service = yield* Integration.Service + const integration = yield* requireIntegration(service, ctx.params.integrationID) + if (!integration.methods.some((method) => method.type === "command" && method.id === ctx.payload.methodID)) + return yield* new IntegrationMethodNotFoundError({ + integrationID: ctx.params.integrationID, + methodID: ctx.payload.methodID, + message: `Integration method not found: ${ctx.payload.methodID}`, + }) + return yield* response( + authorize( + service.command.connect({ integrationID: ctx.params.integrationID, methodID: ctx.payload.methodID, - message: `Integration method not found: ${ctx.payload.methodID}`, - }) - return yield* response( - authorize( - service.command.connect({ - integrationID: ctx.params.integrationID, - methodID: ctx.payload.methodID, - label: ctx.payload.label, - }), - ), - ) - }), - ) - .handle( - "integration.command.status", - Effect.fn(function* (ctx) { - const service = yield* Integration.Service - if (!(yield* service.get(ctx.params.integrationID))) - return yield* new IntegrationNotFoundError({ + label: ctx.payload.label, + }), + ), + ) + }), + ) + .handle( + "integration.command.status", + Effect.fn(function* (ctx) { + const service = yield* Integration.Service + yield* requireIntegration(service, ctx.params.integrationID) + return yield* response( + service.command + .status({ integrationID: ctx.params.integrationID, - message: `Integration not found: ${ctx.params.integrationID}`, + attemptID: ctx.params.attemptID, }) - return yield* response( - service.command - .status({ - integrationID: ctx.params.integrationID, - attemptID: ctx.params.attemptID, - }) - .pipe( - Effect.mapError( - (error) => - new IntegrationAttemptNotFoundError({ - integrationID: error.integrationID, - attemptID: error.attemptID, - message: `Command attempt not found: ${error.attemptID}`, - }), - ), + .pipe( + Effect.mapError( + (error) => + new IntegrationAttemptNotFoundError({ + integrationID: error.integrationID, + attemptID: error.attemptID, + message: `Command attempt not found: ${error.attemptID}`, + }), ), - ) - }), - ) - .handle( - "integration.command.cancel", - Effect.fn(function* (ctx) { - const service = yield* Integration.Service - yield* service.command.cancel({ - integrationID: ctx.params.integrationID, - attemptID: ctx.params.attemptID, - }) - return HttpApiSchema.NoContent.make() - }), - ) - }), + ), + ) + }), + ) + .handle( + "integration.command.cancel", + Effect.fn(function* (ctx) { + const service = yield* Integration.Service + yield* service.command.cancel({ + integrationID: ctx.params.integrationID, + attemptID: ctx.params.attemptID, + }) + return HttpApiSchema.NoContent.make() + }), + ), ) + +function requireIntegration(service: Integration.Interface, id: Integration.ID) { + return service + .get(id) + .pipe( + Effect.flatMap((integration) => + integration + ? Effect.succeed(integration) + : Effect.fail(new IntegrationNotFoundError({ integrationID: id, message: `Integration not found: ${id}` })), + ), + ) +} diff --git a/packages/server/src/handlers/model.ts b/packages/server/src/handlers/model.ts index 5af4334c68..b1faad3006 100644 --- a/packages/server/src/handlers/model.ts +++ b/packages/server/src/handlers/model.ts @@ -5,23 +5,21 @@ import { Api } from "../api" import { awaitPlugins, response } from "../location" export const ModelHandler = HttpApiBuilder.group(Api, "server.model", (handlers) => - Effect.gen(function* () { - return handlers - .handle( - "model.list", - Effect.fn(function* () { - yield* awaitPlugins - const models = yield* Model.Service - return yield* response(models.available()) - }), - ) - .handle( - "model.default", - Effect.fn(function* () { - yield* awaitPlugins - const models = yield* Model.Service - return yield* response(models.default()) - }), - ) - }), + handlers + .handle( + "model.list", + Effect.fn(function* () { + yield* awaitPlugins + const models = yield* Model.Service + return yield* response(models.available()) + }), + ) + .handle( + "model.default", + Effect.fn(function* () { + yield* awaitPlugins + const models = yield* Model.Service + return yield* response(models.default()) + }), + ), ) diff --git a/packages/server/src/handlers/provider.ts b/packages/server/src/handlers/provider.ts index ab9c2201a5..92ece4a884 100644 --- a/packages/server/src/handlers/provider.ts +++ b/packages/server/src/handlers/provider.ts @@ -6,27 +6,25 @@ import { Api } from "../api" import { response } from "../location" export const ProviderHandler = HttpApiBuilder.group(Api, "server.provider", (handlers) => - Effect.gen(function* () { - return handlers - .handle( - "provider.list", - Effect.fn(function* () { - const providers = yield* Provider.Service - return yield* response(providers.available()) - }), - ) - .handle( - "provider.get", - Effect.fn(function* (ctx) { - const providers = yield* Provider.Service - const provider = yield* providers.get(ctx.params.providerID) - if (!provider) - return yield* new ProviderNotFoundError({ - providerID: ctx.params.providerID, - message: `Provider not found: ${ctx.params.providerID}`, - }) - return yield* response(Effect.succeed(provider)) - }), - ) - }), + handlers + .handle( + "provider.list", + Effect.fn(function* () { + const providers = yield* Provider.Service + return yield* response(providers.available()) + }), + ) + .handle( + "provider.get", + Effect.fn(function* (ctx) { + const providers = yield* Provider.Service + const provider = yield* providers.get(ctx.params.providerID) + if (!provider) + return yield* new ProviderNotFoundError({ + providerID: ctx.params.providerID, + message: `Provider not found: ${ctx.params.providerID}`, + }) + return yield* response(Effect.succeed(provider)) + }), + ), ) diff --git a/packages/server/src/handlers/pty.ts b/packages/server/src/handlers/pty.ts index 59c0244a35..00ae414987 100644 --- a/packages/server/src/handlers/pty.ts +++ b/packages/server/src/handlers/pty.ts @@ -56,18 +56,7 @@ export const PtyHandler = HttpApiBuilder.group(Api, "server.pty", (handlers) => "pty.get", Effect.fn(function* (ctx) { const pty = yield* Pty.Service - return yield* response( - pty.get(ctx.params.ptyID).pipe( - Effect.catchTag( - "Pty.NotFoundError", - () => - new PtyNotFoundError({ - ptyID: ctx.params.ptyID, - message: `PTY session not found: ${ctx.params.ptyID}`, - }), - ), - ), - ) + return yield* response(pty.get(ctx.params.ptyID).pipe(Effect.catchTag("Pty.NotFoundError", missingPty))) }), ) .handle( @@ -80,16 +69,7 @@ export const PtyHandler = HttpApiBuilder.group(Api, "server.pty", (handlers) => ...ctx.payload, size: ctx.payload.size ? { ...ctx.payload.size } : undefined, }) - .pipe( - Effect.catchTag( - "Pty.NotFoundError", - () => - new PtyNotFoundError({ - ptyID: ctx.params.ptyID, - message: `PTY session not found: ${ctx.params.ptyID}`, - }), - ), - ), + .pipe(Effect.catchTag("Pty.NotFoundError", missingPty)), ) }), ) @@ -97,16 +77,7 @@ export const PtyHandler = HttpApiBuilder.group(Api, "server.pty", (handlers) => "pty.remove", Effect.fn(function* (ctx) { const pty = yield* Pty.Service - yield* pty.remove(ctx.params.ptyID).pipe( - Effect.catchTag( - "Pty.NotFoundError", - () => - new PtyNotFoundError({ - ptyID: ctx.params.ptyID, - message: `PTY session not found: ${ctx.params.ptyID}`, - }), - ), - ) + yield* pty.remove(ctx.params.ptyID).pipe(Effect.catchTag("Pty.NotFoundError", missingPty)) return HttpApiSchema.NoContent.make() }), ) @@ -122,16 +93,7 @@ export const PtyHandler = HttpApiBuilder.group(Api, "server.pty", (handlers) => ) return yield* new ForbiddenError({ message: "Invalid PTY connect token request" }) const pty = yield* Pty.Service - yield* pty.get(ctx.params.ptyID).pipe( - Effect.catchTag( - "Pty.NotFoundError", - () => - new PtyNotFoundError({ - ptyID: ctx.params.ptyID, - message: `PTY session not found: ${ctx.params.ptyID}`, - }), - ), - ) + yield* pty.get(ctx.params.ptyID).pipe(Effect.catchTag("Pty.NotFoundError", missingPty)) return yield* response(tickets.issue({ ptyID: ctx.params.ptyID, ...(yield* ticketScope) })) }), ) @@ -213,3 +175,7 @@ export const PtyHandler = HttpApiBuilder.group(Api, "server.pty", (handlers) => ) }), ) + +function missingPty(error: Pty.NotFoundError) { + return new PtyNotFoundError({ ptyID: error.ptyID, message: `PTY session not found: ${error.ptyID}` }) +} diff --git a/packages/server/src/handlers/websearch.ts b/packages/server/src/handlers/websearch.ts index 71808b86e2..f858b08980 100644 --- a/packages/server/src/handlers/websearch.ts +++ b/packages/server/src/handlers/websearch.ts @@ -6,45 +6,43 @@ import { Api } from "../api" import { response } from "../location" export const WebSearchHandler = HttpApiBuilder.group(Api, "server.websearch", (handlers) => - Effect.gen(function* () { - return handlers - .handle( - "websearch.providers", - Effect.fn("server.websearch.providers")(function* () { - const websearch = yield* WebSearch.Service - return yield* response(websearch.providers()) - }), - ) - .handle( - "websearch.query", - Effect.fn("server.websearch.query")(function* (request) { - const websearch = yield* WebSearch.Service - return yield* response( - websearch.query(request.payload).pipe( - Effect.catchTags({ - "WebSearch.ProviderRequired": () => - new InvalidRequestError({ - message: "Web search provider is required", - kind: "websearch_provider_required", - field: "providerID", - }), - "WebSearch.ProviderNotFound": (error) => - new InvalidRequestError({ - message: `Web search provider not found: ${error.providerID}`, - kind: "websearch_provider_not_found", - field: "providerID", - }), - "WebSearch.Disabled": () => - new InvalidRequestError({ message: "Web search is disabled", kind: "websearch_disabled" }), - "WebSearch.Request": (error) => - new ServiceUnavailableError({ - message: `Web search request failed: ${error.providerID}`, - service: error.providerID, - }), - }), - ), - ) - }), - ) - }), + handlers + .handle( + "websearch.providers", + Effect.fn("server.websearch.providers")(function* () { + const websearch = yield* WebSearch.Service + return yield* response(websearch.providers()) + }), + ) + .handle( + "websearch.query", + Effect.fn("server.websearch.query")(function* (request) { + const websearch = yield* WebSearch.Service + return yield* response( + websearch.query(request.payload).pipe( + Effect.catchTags({ + "WebSearch.ProviderRequired": () => + new InvalidRequestError({ + message: "Web search provider is required", + kind: "websearch_provider_required", + field: "providerID", + }), + "WebSearch.ProviderNotFound": (error) => + new InvalidRequestError({ + message: `Web search provider not found: ${error.providerID}`, + kind: "websearch_provider_not_found", + field: "providerID", + }), + "WebSearch.Disabled": () => + new InvalidRequestError({ message: "Web search is disabled", kind: "websearch_disabled" }), + "WebSearch.Request": (error) => + new ServiceUnavailableError({ + message: `Web search request failed: ${error.providerID}`, + service: error.providerID, + }), + }), + ), + ) + }), + ), ) diff --git a/packages/server/test/integration.test.ts b/packages/server/test/integration.test.ts new file mode 100644 index 0000000000..4ee7a4f88b --- /dev/null +++ b/packages/server/test/integration.test.ts @@ -0,0 +1,71 @@ +import { expect } from "bun:test" +import { Integration } from "@opencode/schema/integration" +import { Effect } from "effect" +import { tmpdirScoped } from "../../core/test/fixture/tmpdir" +import { it } from "../../core/test/lib/effect" +import { startServer } from "./fixture/server" + +it.live("integration routes preserve lookup errors and command method validation", () => + Effect.gen(function* () { + const tmp = yield* tmpdirScoped("opencode-integration-endpoints-") + const server = yield* startServer(tmp.path) + const request = (method: "GET" | "POST" | "DELETE", route: string, payload?: object) => + Effect.promise((signal) => { + const url = new URL(route, server.base) + url.searchParams.set("location[directory]", tmp.path) + return fetch(url, { + method, + headers: { ...server.headers, "content-type": "application/json" }, + body: payload === undefined ? undefined : JSON.stringify(payload), + signal, + }) + }) + // Listing waits for plugin activation; individual lookups read the current snapshot. + expect((yield* request("GET", "/api/integration")).status).toBe(200) + const attemptID = Integration.AttemptID.create() + const base = "/api/integration/missing-integration" + yield* Effect.forEach( + [ + { method: "GET", route: base }, + { method: "POST", route: `${base}/connect/key`, payload: { key: "secret" } }, + { method: "POST", route: `${base}/connect/external`, payload: { methodID: "missing-method" } }, + { method: "GET", route: `${base}/connect/oauth/${attemptID}` }, + { method: "POST", route: `${base}/connect/oauth/${attemptID}/complete`, payload: {} }, + { method: "POST", route: `${base}/connect/command`, payload: { methodID: "missing-method" } }, + { method: "GET", route: `${base}/connect/command/${attemptID}` }, + ] as const, + Effect.fn(function* (endpoint) { + const response = yield* request( + endpoint.method, + endpoint.route, + "payload" in endpoint ? endpoint.payload : undefined, + ) + expect(response.status).toBe(404) + expect(yield* Effect.promise(() => response.json())).toEqual({ + _tag: "IntegrationNotFoundError", + integrationID: "missing-integration", + message: "Integration not found: missing-integration", + }) + }), + ) + + const get = yield* request("GET", "/api/integration/opencode") + expect(get.status).toBe(200) + expect(yield* Effect.promise(() => get.json())).toMatchObject({ + location: { directory: tmp.path }, + data: { id: "opencode", methods: expect.any(Array) }, + }) + const command = yield* request("POST", "/api/integration/opencode/connect/command", { methodID: "missing-method" }) + expect(command.status).toBe(404) + expect(yield* Effect.promise(() => command.json())).toEqual({ + _tag: "IntegrationMethodNotFoundError", + integrationID: "opencode", + methodID: "missing-method", + message: "Integration method not found: missing-method", + }) + + // Cancellation remains a no-op for unknown integrations and attempts. + expect((yield* request("DELETE", `${base}/connect/oauth/${attemptID}`)).status).toBe(204) + expect((yield* request("DELETE", `${base}/connect/command/${attemptID}`)).status).toBe(204) + }), +) diff --git a/packages/server/test/pty-errors.test.ts b/packages/server/test/pty-errors.test.ts new file mode 100644 index 0000000000..debc5357f3 --- /dev/null +++ b/packages/server/test/pty-errors.test.ts @@ -0,0 +1,58 @@ +import { expect } from "bun:test" +import { Global } from "@opencode/util/global" +import { Effect } from "effect" +import { tempGlobalLayer } from "../../core/test/fixture/global" +import { tmpdirScoped } from "../../core/test/fixture/tmpdir" +import { it } from "../../core/test/lib/effect" +import { ServerFetch } from "../src/fetch" + +it.live("PTY REST endpoints preserve missing-terminal errors and ticket guard precedence", () => + Effect.gen(function* () { + const directory = yield* tmpdirScoped() + const handler = yield* ServerFetch.make( + { + app: { version: "test" }, + database: { path: ":memory:" }, + config: { project: false }, + fs: { filewatcher: false }, + models: { fetch: false }, + }, + { overrides: [Global.node.replace(tempGlobalLayer)] }, + ) + const ptyID = "pty_missing" + for (const method of ["GET", "PUT", "DELETE", "POST"]) { + const response = yield* Effect.promise(() => + handler( + new Request(`http://opencode.local/api/pty/${ptyID}${method === "POST" ? "/connect-token" : ""}`, { + method, + headers: { + "content-type": "application/json", + "x-opencode-directory": encodeURIComponent(directory.path), + "x-opencode-ticket": "1", + }, + body: method === "PUT" ? JSON.stringify({ title: "Changed" }) : undefined, + }), + ), + ) + expect({ method, status: response.status }).toEqual({ method, status: 404 }) + expect(yield* Effect.promise(() => response.json())).toEqual({ + _tag: "PtyNotFoundError", + ptyID, + message: `PTY session not found: ${ptyID}`, + }) + } + const forbidden = yield* Effect.promise(() => + handler( + new Request(`http://opencode.local/api/pty/${ptyID}/connect-token`, { + method: "POST", + headers: { "x-opencode-directory": encodeURIComponent(directory.path) }, + }), + ), + ) + expect(forbidden.status).toBe(403) + expect(yield* Effect.promise(() => forbidden.json())).toEqual({ + _tag: "ForbiddenError", + message: "Invalid PTY connect token request", + }) + }).pipe(Effect.scoped), +) diff --git a/packages/tui/src/component/devtools-bar.tsx b/packages/tui/src/component/devtools-bar.tsx index 752c29834d..34c9da657a 100644 --- a/packages/tui/src/component/devtools-bar.tsx +++ b/packages/tui/src/component/devtools-bar.tsx @@ -4,7 +4,7 @@ import { open } from "node:fs/promises" import { tmpdir } from "node:os" import path from "node:path" import { monitorEventLoopDelay } from "node:perf_hooks" -import { createMemo, createResource, createSignal, For, onCleanup, onMount, Show, type ParentProps } from "solid-js" +import { createMemo, createResource, createSignal, onCleanup, onMount, Show, type ParentProps } from "solid-js" import { useClient } from "../context/client" import { useConfig } from "../config" import { useData } from "../context/data" @@ -12,7 +12,6 @@ import { useLocation } from "../context/location" import { useRoute } from "../context/route" import { Keymap } from "../context/keymap" import { useTheme, useThemes } from "../context/theme" -import { DevTools } from "../devtools" import { useDialog } from "../ui/dialog" import { DialogExperiments } from "./dialog-experiments" import { usePlugin } from "../plugin/context" @@ -50,7 +49,6 @@ export function DevToolsBar() { let focus: Renderable | null const connected = createMemo(() => client.connection.status() === "connected") const serverIndicator = createMemo(() => connectionIndicator(client.connection.status(), client.connection.attempt())) - const groups = DevTools.data const [server] = createResource(connected, async () => { const info = await client.api.server.info() return { @@ -403,16 +401,6 @@ export function DevToolsBar() { - - {(group) => ( - - - {group.title} - - {(entry) => } - - )} - diff --git a/packages/tui/src/context/editor.ts b/packages/tui/src/context/editor.ts index cf2fbbf9e6..92a8638f69 100644 --- a/packages/tui/src/context/editor.ts +++ b/packages/tui/src/context/editor.ts @@ -96,7 +96,6 @@ type JsonRpcMessage = Schema.Schema.Type export type EditorSelection = Schema.Schema.Type export type EditorMention = Schema.Schema.Type export type EditorLabelState = "pending" | "sent" | "none" -type EditorServerInfo = Schema.Schema.Type type EditorConnection = { url: string @@ -125,12 +124,10 @@ export const { use: useEditorContext, provider: EditorContextProvider } = create status: "disabled" | "connecting" | "connected" selection: EditorSelection | undefined selectionSent: boolean - server: EditorServerInfo | undefined }>({ status: "disabled", selection: undefined, selectionSent: false, - server: undefined, }) let socket: WebSocket | undefined @@ -255,7 +252,6 @@ export const { use: useEditorContext, provider: EditorContextProvider } = create const initialize = method === "initialize" ? decodeEditorServerInfo(message.result) : Option.none() if (Option.isSome(initialize)) { - setStore("server", initialize.value) send({ method: "notifications/initialized" }) return } @@ -304,7 +300,6 @@ export const { use: useEditorContext, provider: EditorContextProvider } = create current.close() } setStore("status", "disabled") - setStore("server", undefined) connect() } @@ -348,9 +343,6 @@ export const { use: useEditorContext, provider: EditorContextProvider } = create mentionListeners.add(listener) return () => mentionListeners.delete(listener) }, - server() { - return store.server - }, reconnect(directory?: string) { reconnectWithDirectory(directory) }, diff --git a/packages/tui/src/devtools/index.ts b/packages/tui/src/devtools/index.ts deleted file mode 100644 index ddb84b6272..0000000000 --- a/packages/tui/src/devtools/index.ts +++ /dev/null @@ -1,41 +0,0 @@ -export * as DevTools from "." - -import { createSignal } from "solid-js" - -export type Value = string | number | boolean | null - -export type Group = Readonly<{ - id: string - title: string - entries: readonly Readonly<{ key: string; value: Value }>[] -}> - -const [groups, setGroups] = createSignal([]) - -export function register(input: { id: string; title: string }) { - setGroups((groups) => { - if (groups.some((group) => group.id === input.id)) { - return groups.map((group) => (group.id === input.id ? { ...group, title: input.title } : group)) - } - return [...groups, { ...input, entries: [] }] - }) - - return { - set(key: string, value: Value) { - setGroups((groups) => - groups.map((group) => { - if (group.id !== input.id) return group - if (group.entries.some((entry) => entry.key === key)) { - return { - ...group, - entries: group.entries.map((entry) => (entry.key === key ? { key, value } : entry)), - } - } - return { ...group, entries: [...group.entries, { key, value }] } - }), - ) - }, - } -} - -export const data = groups diff --git a/packages/tui/src/feature-plugins/home/footer.tsx b/packages/tui/src/feature-plugins/home/footer.tsx index d31888c277..9eb03bb1b4 100644 --- a/packages/tui/src/feature-plugins/home/footer.tsx +++ b/packages/tui/src/feature-plugins/home/footer.tsx @@ -107,8 +107,6 @@ function View(props: { context: Plugin.Context }) { export default Plugin.define({ id: "opencode.home.footer", setup(context) { - // Root takeover: an external plugin replacing home.footer wins (last- - // enabled) and this builtin shows as suppressed, not silently gone. // Append keeps the path open to additive plugin claims; an external // replace still takes the boundary over. context.ui.slot({ append: "home.footer", render: () => }) diff --git a/packages/tui/src/plugin/context.tsx b/packages/tui/src/plugin/context.tsx index 31c4f4cea0..881db92dd5 100644 --- a/packages/tui/src/plugin/context.tsx +++ b/packages/tui/src/plugin/context.tsx @@ -507,7 +507,7 @@ export function PluginProvider(props: ParentProps<{ packages: PackageSource; dir // order within one plugin. The resolver's last-wins rules depend on it. const claims = createMemo(() => Object.entries(store.registrations).flatMap(([id, registration]) => - Object.entries(registration.active ? registration.slots : {}).map(([key, slot]) => { + Object.values(registration.active ? registration.slots : {}).map((slot) => { // Rows downstream diff by reference; a stable claim per render // function keeps untouched plugins' slot rows (and their state) // alive across other plugins' reloads. @@ -515,7 +515,7 @@ export function PluginProvider(props: ParentProps<{ packages: PackageSource; dir if (cached) return cached // Placements are immutable once registered; unwrap the store proxy // so resolver reads don't subscribe tracked scopes. - const item = { key: `${id}/${key}`, plugin: id, placement: unwrap(slot.placement), render: slot.render } + const item = { plugin: id, placement: unwrap(slot.placement), render: slot.render } slotItems.set(slot.render, item) return item }), diff --git a/packages/tui/src/plugin/structure.ts b/packages/tui/src/plugin/structure.ts index 1a5e861767..9abbca0fc6 100644 --- a/packages/tui/src/plugin/structure.ts +++ b/packages/tui/src/plugin/structure.ts @@ -1,5 +1,5 @@ // Pure resolution of the slot tree: the mounted slot paths plus plugin claims -// in, per-path placement buckets plus diagnostics out. No solid, no I/O — +// in, per-path placement buckets out. No solid, no I/O — // every policy rule (replacement takeover, hierarchy-beats-timeline, // last-enabled-wins, missing-target degradation) is testable as a data // transform. @@ -13,7 +13,6 @@ export type Placement = { readonly kind: PlacementKind; readonly target: string // One plugin's registered slot claim, in enable order within the claims array. export type Claim = { - readonly key: string readonly plugin: string readonly placement: Placement readonly render: Render @@ -29,19 +28,6 @@ export type Slotted = { readonly replace?: Claim } -export type Suppressed = { - readonly claim: Claim - // The winning claim for a conflict or boundary suppression; absent when a - // replacement's target no longer exists (missing replacements never degrade). - readonly by?: Claim -} - -export type Degraded = { - readonly claim: Claim - // The surviving ancestor path the claim was appended to. - readonly to: string -} - const EMPTY: Slotted = { before: [], prepend: [], append: [], after: [] } export function emptySlotted(): Slotted { @@ -62,24 +48,14 @@ export function resolveSlots(input: { readonly claims: ReadonlyArray> }): { readonly slotted: ReadonlyMap> - readonly suppressed: ReadonlyArray> - readonly degraded: ReadonlyArray> } { - const suppressed: Suppressed[] = [] - const degraded: Degraded[] = [] - // Pass 1 — replacement boundaries. Per target the last-enabled claim wins; // then an accepted boundary swallows every replacement strictly inside it, // regardless of enable order (hierarchy beats timeline). const winners = new Map>() for (const claim of input.claims) { if (claim.placement.kind !== "replace") continue - if (!input.paths.has(claim.placement.target)) { - suppressed.push({ claim }) - continue - } - const prior = winners.get(claim.placement.target) - if (prior) suppressed.push({ claim: prior, by: claim }) + if (!input.paths.has(claim.placement.target)) continue winners.set(claim.placement.target, claim) } const boundaries = new Map>() @@ -89,11 +65,7 @@ export function resolveSlots(input: { } // Shallow boundaries first, so a nested replacement meets its container. for (const [path, winner] of [...winners].sort((a, b) => depth(a[0]) - depth(b[0]))) { - const outer = containing(path) - if (outer) { - suppressed.push({ claim: winner, by: outer }) - continue - } + if (containing(path)) continue boundaries.set(path, winner) } @@ -118,32 +90,24 @@ export function resolveSlots(input: { // Inside placements targeting a replaced boundary are part of its // contents; sibling placements on the boundary itself stay outside it. const inside = kind === "prepend" || kind === "append" ? boundaries.get(target) : undefined - const outer = inside ?? containing(target) - if (outer) { - suppressed.push({ claim, by: outer }) - continue - } + if (inside ?? containing(target)) continue if (input.paths.has(target)) { bucket(target)[kind].push(claim) continue } const ancestor = survivingAncestor(target, input.paths) - if (ancestor === undefined) { - suppressed.push({ claim }) - continue - } + if (ancestor === undefined) continue // The ancestor is never a replaced boundary (containing() caught that). // Appending is load-bearing: a parent slot registers before its children // and renders append after them, so the transient degradation // during a nested mount never instantiates anything. - degraded.push({ claim, to: ancestor }) bucket(ancestor).append.push(claim) } const slotted = new Map>() for (const [path, lists] of buckets) slotted.set(path, lists) for (const [path, winner] of boundaries) slotted.set(path, { ...(buckets.get(path) ?? EMPTY), replace: winner }) - return { slotted, suppressed, degraded } + return { slotted } } function depth(path: string) { diff --git a/packages/tui/src/ui/dialog.tsx b/packages/tui/src/ui/dialog.tsx index 80e4484ab2..5f8651e4f4 100644 --- a/packages/tui/src/ui/dialog.tsx +++ b/packages/tui/src/ui/dialog.tsx @@ -112,6 +112,25 @@ function init() { }, 1) } + function dismiss(clearInput: boolean) { + if (renderer.getSelection()) { + renderer.clearSelection() + return + } + if (clearInput) { + const editor = renderer.currentFocusedEditor + if (editor?.plainText) { + if (editor instanceof InputRenderable) editor.value = "" + else editor.setText("") + return + } + } + const current = store.stack.at(-1) + current?.onClose?.() + setStore("stack", store.stack.slice(0, -1)) + refocus() + } + Keymap.createLayer(() => ({ mode: "modal", enabled: store.stack.length > 0, @@ -120,37 +139,13 @@ function init() { bind: "escape", title: "Close dialog", group: "Dialog", - run: () => { - if (renderer.getSelection()) { - renderer.clearSelection() - return - } - const current = store.stack.at(-1) - current?.onClose?.() - setStore("stack", store.stack.slice(0, -1)) - refocus() - }, + run: () => dismiss(false), }, { bind: "ctrl+c", title: "Close dialog", group: "Dialog", - run: () => { - if (renderer.getSelection()) { - renderer.clearSelection() - return - } - const editor = renderer.currentFocusedEditor - if (editor?.plainText) { - if (editor instanceof InputRenderable) editor.value = "" - else editor.setText("") - return - } - const current = store.stack.at(-1) - current?.onClose?.() - setStore("stack", store.stack.slice(0, -1)) - refocus() - }, + run: () => dismiss(true), }, ], })) diff --git a/packages/tui/src/util/collapse-tool-output.ts b/packages/tui/src/util/collapse-tool-output.ts index dbdf6e5a19..c573059054 100644 --- a/packages/tui/src/util/collapse-tool-output.ts +++ b/packages/tui/src/util/collapse-tool-output.ts @@ -25,12 +25,7 @@ export function collapseToolOutput(output: string, maxLines: number, maxChars: n export function collapseShellOutput(input: string, output: string, maxLines: number, maxChars: number) { if (!input) { - const collapsed = collapseToolOutput(output, maxLines, maxChars) - return { - input, - output: collapsed.overflow ? collapseTail(output, maxLines, maxChars) : output, - overflow: collapsed.overflow, - } + return { input, ...collapseTail(output, maxLines, maxChars) } } const commandLines = Math.min(2, maxLines) @@ -40,10 +35,10 @@ export function collapseShellOutput(input: string, output: string, maxLines: num const lines = Math.max(1, maxLines - command.lines - 1) const chars = Math.max(1, maxChars - Array.from(command.output).length - 2) - const collapsed = collapseToolOutput(output, lines, chars) + const collapsed = collapseTail(output, lines, chars) return { input: command.output, - output: collapsed.overflow ? collapseTail(output, lines, chars) : output, + output: collapsed.output, overflow: command.overflow || collapsed.overflow, } } @@ -82,14 +77,14 @@ function collapseShellCommand(input: string, maxLines: number, lineWidth: number function collapseTail(output: string, maxLines: number, maxChars: number) { const lines = output.split("\n") - if (lines.length <= maxLines && Array.from(output).length <= maxChars) return output + if (lines.length <= maxLines && Array.from(output).length <= maxChars) return { output, overflow: false } const count = Math.max(1, lines.length - Math.max(0, maxLines - 1)) const label = `(${count} earlier ${count === 1 ? "line" : "lines"})` - if (maxLines <= 1) return label + if (maxLines <= 1) return { output: label, overflow: true } const preview = Array.from(lines.slice(-(maxLines - 1)).join("\n")) const available = maxChars - Array.from(label).length - 1 - if (available <= 0) return label - return `${label}\n${preview.slice(-available).join("")}` + if (available <= 0) return { output: label, overflow: true } + return { output: `${label}\n${preview.slice(-available).join("")}`, overflow: true } } diff --git a/packages/tui/src/util/persistence.ts b/packages/tui/src/util/persistence.ts index d013628218..3e492924ea 100644 --- a/packages/tui/src/util/persistence.ts +++ b/packages/tui/src/util/persistence.ts @@ -22,12 +22,10 @@ export async function appendText(filePath: string, content: string) { export async function writeJsonAtomic(filePath: string, value: unknown) { await mkdir(path.dirname(filePath), { recursive: true }) const temporary = `${filePath}.${process.pid}.${crypto.randomUUID()}.tmp` - await writeFile(temporary, JSON.stringify(value)).catch(async (error) => { - await rm(temporary, { force: true }).catch(() => undefined) - throw error - }) - await rename(temporary, filePath).catch(async (error) => { - await rm(temporary, { force: true }).catch(() => undefined) - throw error - }) + await writeFile(temporary, JSON.stringify(value)) + .then(() => rename(temporary, filePath)) + .catch(async (error) => { + await rm(temporary, { force: true }).catch(() => undefined) + throw error + }) } diff --git a/packages/tui/test/cli/tui/collapse-tool-output.test.ts b/packages/tui/test/cli/tui/collapse-tool-output.test.ts index f12697d5c8..1f94068519 100644 --- a/packages/tui/test/cli/tui/collapse-tool-output.test.ts +++ b/packages/tui/test/cli/tui/collapse-tool-output.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test" -import { collapseToolOutput } from "../../../src/util/collapse-tool-output" +import { collapseShellOutput, collapseToolOutput } from "../../../src/util/collapse-tool-output" test("limits command input and output to the same line budget", () => { const command = Array.from({ length: 8 }, (_, index) => `command ${index + 1}`).join("\n") @@ -12,3 +12,112 @@ test("limits command input and output to the same line budget", () => { expect(collapsed.output).toContain("command 8\n\noutput 1…") expect(collapsed.output).not.toContain("output 2") }) + +test.each([ + { + name: "fitting output without a command", + input: "", + output: "a\nb", + lines: 2, + chars: 3, + expected: { input: "", output: "a\nb", overflow: false }, + }, + { + name: "tail line overflow without a command", + input: "", + output: "one\ntwo\nthree\nfour", + lines: 3, + chars: 100, + expected: { input: "", output: "(2 earlier lines)\nthree\nfour", overflow: true }, + }, + { + name: "Unicode tail code-point budget", + input: "", + output: "old".repeat(12) + "😀b😺c", + lines: 3, + chars: 21, + expected: { input: "", output: "(1 earlier line)\n😀b😺c", overflow: true }, + }, + { + name: "one-line output budget", + input: "", + output: "one\ntwo\nthree", + lines: 1, + chars: 100, + expected: { input: "", output: "(3 earlier lines)", overflow: true }, + }, + { + name: "empty output with zero budgets", + input: "", + output: "", + lines: 0, + chars: 0, + expected: { input: "", output: "(1 earlier line)", overflow: true }, + }, + { + name: "zero line budget", + input: "", + output: "abc", + lines: 0, + chars: 10, + expected: { input: "", output: "(1 earlier line)", overflow: true }, + }, + { + name: "fitting command and output", + input: "echo", + output: "done", + lines: 3, + chars: 100, + expected: { input: "echo", output: "done", overflow: false }, + }, + { + name: "command-only line overflow", + input: "a\nb\nc", + output: "", + lines: 3, + chars: 100, + expected: { input: "a\nb…", output: "", overflow: true }, + }, + { + name: "command-only character overflow", + input: "abcdef", + output: "", + lines: 1, + chars: 4, + expected: { input: "abc…", output: "", overflow: true }, + }, + { + name: "command reserves output lines", + input: "echo", + output: "one\ntwo\nthree\nfour", + lines: 5, + chars: 100, + expected: { input: "echo", output: "(2 earlier lines)\nthree\nfour", overflow: true }, + }, + { + name: "combined command and output overflow", + input: "abcdefghijkl", + output: "one\ntwo", + lines: 3, + chars: 12, + expected: { input: "abcdefg…", output: "(2 earlier lines)", overflow: true }, + }, + { + name: "command overflow with fitting output", + input: "a\nb\nc", + output: "ok", + lines: 4, + chars: 100, + expected: { input: "a\nb…", output: "ok", overflow: true }, + }, + { + name: "command grapheme display-width overflow", + input: "😀😀😀", + output: "tail", + lines: 2, + chars: 6, + expected: { input: "😀😀…", output: "(1 earlier line)", overflow: true }, + }, +])("shell preview preserves $name", ({ input, output, lines, chars, expected }) => { + expect(collapseShellOutput(input, output, lines, chars)).toEqual(expected) +}) diff --git a/packages/tui/test/cli/tui/dialog-dismiss.test.tsx b/packages/tui/test/cli/tui/dialog-dismiss.test.tsx new file mode 100644 index 0000000000..a477751b71 --- /dev/null +++ b/packages/tui/test/cli/tui/dialog-dismiss.test.tsx @@ -0,0 +1,198 @@ +/** @jsxImportSource @opentui/solid */ +import { InputRenderable, TextareaRenderable } from "@opentui/core" +import { testRender } from "@opentui/solid" +import { expect, test } from "bun:test" +import { mkdir } from "node:fs/promises" +import path from "node:path" +import { onMount } from "solid-js" +import { Keymap } from "../../../src/context/keymap" +import { DialogProvider, useDialog, type DialogContext } from "../../../src/ui/dialog" +import { tmpdir } from "../../fixture/fixture" +import { TestTuiContexts } from "../../fixture/tui-environment" +import { createTuiResolvedConfig } from "../../fixture/tui-runtime" + +async function mountDialog(root: string, textarea = false) { + const state = path.join(root, "state") + await mkdir(state, { recursive: true }) + const [{ ConfigProvider }, { ThemeProvider }, { ToastProvider }] = await Promise.all([ + import("../../../src/config"), + import("../../../src/context/theme"), + import("../../../src/ui/toast"), + ]) + let dialog!: DialogContext + let keymap!: Keymap + let prior!: InputRenderable + let editor!: InputRenderable | TextareaRenderable + + function Fixture() { + dialog = useDialog() + keymap = Keymap.use() + onMount(() => prior.focus()) + return (prior = ref)} value="Prior target" /> + } + + function Content() { + onMount(() => editor.focus()) + return ( + + Dismiss dialog Alpha + {textarea ? ( +