From 01d8f0825abef0c3a6b61a6e525640e1138c6273 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Mon, 2 Nov 2026 09:51:53 -0500 Subject: [PATCH 01/20] section 1: Phase 1: Shared reconnecting OpenCode event subscription and --- .../src/services/opencode-event-routing.ts | 87 +++-- .../services/opencode-event-subscription.ts | 156 ++++++++ .../services/opencode-event-routing.test.ts | 121 +++++++ .../opencode-event-subscription.test.ts | 342 ++++++++++++++++++ 4 files changed, 683 insertions(+), 23 deletions(-) create mode 100644 backend/src/services/opencode-event-subscription.ts create mode 100644 backend/test/services/opencode-event-routing.test.ts create mode 100644 backend/test/services/opencode-event-subscription.test.ts diff --git a/backend/src/services/opencode-event-routing.ts b/backend/src/services/opencode-event-routing.ts index bed6f3d70..f533a330d 100644 --- a/backend/src/services/opencode-event-routing.ts +++ b/backend/src/services/opencode-event-routing.ts @@ -1,12 +1,29 @@ import { EventSource } from 'eventsource' -import { openCodeLocation, sessionIDFromEvent, type OpenCodeApi, type V2Event } from '@opencode-manager/shared/opencode' +import { + openCodeLocation, + sessionIDFromEvent, + type FormInfo, + type OpenCodeApi, + type V2Event, +} from '@opencode-manager/shared/opencode' import { logger } from '../utils/logger' const MULTILINE_PATTERN = /[\r\n]/ -export type ReplayEventType = 'permission.asked' | 'form.created' | 'session.status' +type PermissionAskedEvent = Extract +type FormCreatedEvent = Extract +type SessionStatusEvent = Extract -export type ReplayEventBuilder = (type: ReplayEventType, directory: string, data: unknown) => V2Event +export type ReplayEvent = PermissionAskedEvent | FormCreatedEvent | SessionStatusEvent + +export type ReplayEventType = ReplayEvent['type'] + +type ReplayEventArguments = + | [type: PermissionAskedEvent['type'], directory: string, data: PermissionAskedEvent['data']] + | [type: FormCreatedEvent['type'], directory: string, data: { form: FormInfo }] + | [type: SessionStatusEvent['type'], directory: string, data: SessionStatusEvent['data']] + +export type ReplayEventBuilder = (...args: ReplayEventArguments) => ReplayEvent export interface RoutedOpenCodeEvent { event: V2Event @@ -81,29 +98,24 @@ export function applySessionActivity(busySessions: Set, event: V2Event): export function createReplayEventBuilder(idPrefix: string): ReplayEventBuilder { let count = 0 - return (type, directory, data) => { + return (...args) => { count += 1 - return { - id: `${idPrefix}${count}`, - created: Date.now(), - type, - location: { directory }, - data, - } as V2Event + const base = { id: `${idPrefix}${count}`, created: Date.now() } + switch (args[0]) { + case 'permission.asked': + return { ...base, type: args[0], location: { directory: args[1] }, data: args[2] } + case 'form.created': + return { ...base, type: args[0], location: { directory: args[1] }, data: args[2] as FormCreatedEvent['data'] } + case 'session.status': + return { ...base, type: args[0], location: { directory: args[1] }, data: args[2] } + } } } -export function parseOpenCodeEvent(data: string, sessionDirectories: Map): RoutedOpenCodeEvent | null { - let event: V2Event - try { - event = JSON.parse(data) as V2Event - } catch { - return null - } - - if (!event?.type) return null - - const payloadJson = MULTILINE_PATTERN.test(data) ? JSON.stringify(event) : data +function resolveEventRouting( + event: V2Event, + sessionDirectories: Map, +): Omit { const directory = event.location?.directory const sessionID = sessionIDFromEvent(event) const forkParentId = event.type === 'session.forked' ? event.data.parentID : undefined @@ -117,7 +129,36 @@ export function parseOpenCodeEvent(data: string, sessionDirectories: Map, +): RoutedOpenCodeEvent { + return { ...resolveEventRouting(event, sessionDirectories), payloadJson: JSON.stringify(event) } +} + +export function parseOpenCodeEvent(data: string, sessionDirectories: Map): RoutedOpenCodeEvent | null { + let event: V2Event + try { + event = JSON.parse(data) as V2Event + } catch { + return null + } + + if (!event?.type) return null + + const routed = resolveEventRouting(event, sessionDirectories) + const payloadJson = MULTILINE_PATTERN.test(data) ? JSON.stringify(event) : data + return { ...routed, payloadJson } } export async function collectPendingReplayEvents(request: PendingReplayRequest): Promise { diff --git a/backend/src/services/opencode-event-subscription.ts b/backend/src/services/opencode-event-subscription.ts new file mode 100644 index 000000000..0bf823c69 --- /dev/null +++ b/backend/src/services/opencode-event-subscription.ts @@ -0,0 +1,156 @@ +import { DEFAULTS } from '@opencode-manager/shared/config' +import type { OpenCodeApi, V2Event } from '@opencode-manager/shared/opencode' +import { logger } from '../utils/logger' +import { nextBackoffDelay } from './opencode-event-routing' + +export interface OpenCodeEventSubscriptionOptions { + api: () => OpenCodeApi + label: string + onOpen: () => void + onEvent: (event: V2Event) => void + onDisconnect: (error: unknown) => void + reconnectDelayMs?: number + maxReconnectDelayMs?: number + stallThresholdMs?: number + watchdogTickMs?: number +} + +export interface OpenCodeEventSubscription { + close: () => void + reconnect: () => void +} + +interface SubscriptionAttempt { + controller: AbortController + watchdog: ReturnType | null +} + +/** + * Subscribes to an OpenCode event stream and owns its reconnect, backoff and stall detection. + * + * Every attempt yields a fresh subscription. The first `server.connected` event marks the attempt + * as established, every event reaches `onEvent` in order, and an attempt that ends, throws or + * exceeds the stall threshold disconnects, waits the current backoff delay and reconnects with the + * delay doubled up to `maxReconnectDelayMs`. + * + * @param options Stream access, callbacks and optional timing overrides. + * @returns Handles to close the subscription or force an immediate reconnect. + */ +export function subscribeOpenCodeEvents(options: OpenCodeEventSubscriptionOptions): OpenCodeEventSubscription { + const reconnectDelayMs = options.reconnectDelayMs ?? DEFAULTS.SSE.RECONNECT_DELAY_MS + const maxReconnectDelayMs = options.maxReconnectDelayMs ?? DEFAULTS.SSE.MAX_RECONNECT_DELAY_MS + const stallThresholdMs = options.stallThresholdMs ?? DEFAULTS.SSE.STALL_THRESHOLD_MS + const watchdogTickMs = options.watchdogTickMs ?? DEFAULTS.SSE.WATCHDOG_TICK_MS + + let closed = false + let currentDelay = reconnectDelayMs + let loopToken = 0 + let currentAttempt: SubscriptionAttempt | null = null + let delayTimer: ReturnType | null = null + let resolveDelay: (() => void) | null = null + + const stopAttempt = (): void => { + const attempt = currentAttempt + currentAttempt = null + if (!attempt) return + if (attempt.watchdog) { + clearInterval(attempt.watchdog) + attempt.watchdog = null + } + attempt.controller.abort() + } + + const stopDelay = (): void => { + if (delayTimer) { + clearTimeout(delayTimer) + delayTimer = null + } + const resolve = resolveDelay + resolveDelay = null + resolve?.() + } + + const wait = (delayMs: number): Promise => new Promise((resolve) => { + resolveDelay = resolve + delayTimer = setTimeout(() => { + delayTimer = null + resolveDelay = null + resolve() + }, delayMs) + }) + + const run = async (token: number): Promise => { + while (!closed && token === loopToken) { + const attempt: SubscriptionAttempt = { controller: new AbortController(), watchdog: null } + currentAttempt = attempt + let lastActivityAt = Date.now() + let opened = false + let stallError: Error | null = null + let failure: unknown + + attempt.watchdog = setInterval(() => { + if (Date.now() - lastActivityAt <= stallThresholdMs) return + stallError = new Error(`${options.label}: OpenCode event stream stalled`) + attempt.controller.abort(stallError) + }, watchdogTickMs) + + try { + const stream = options.api().event.subscribe({ + signal: attempt.controller.signal, + onActivity: () => { + lastActivityAt = Date.now() + }, + }) + + for await (const event of stream) { + if (closed || token !== loopToken) break + if (!opened && event.type === 'server.connected') { + opened = true + currentDelay = reconnectDelayMs + options.onOpen() + } + try { + options.onEvent(event) + } catch (error) { + logger.error(`${options.label}: failed to handle ${event.type} event`, error) + } + } + } catch (error) { + failure = error + } finally { + if (attempt.watchdog) { + clearInterval(attempt.watchdog) + attempt.watchdog = null + } + if (currentAttempt === attempt) currentAttempt = null + } + + if (closed || token !== loopToken) return + + options.onDisconnect(stallError ?? failure) + await wait(currentDelay) + if (closed || token !== loopToken) return + currentDelay = nextBackoffDelay(currentDelay, maxReconnectDelayMs) + } + } + + void run(loopToken) + + return { + close: () => { + if (closed) return + closed = true + loopToken += 1 + stopDelay() + stopAttempt() + }, + reconnect: () => { + if (closed) return + loopToken += 1 + stopDelay() + stopAttempt() + currentDelay = reconnectDelayMs + void run(loopToken) + }, + } +} diff --git a/backend/test/services/opencode-event-routing.test.ts b/backend/test/services/opencode-event-routing.test.ts new file mode 100644 index 000000000..916c98c25 --- /dev/null +++ b/backend/test/services/opencode-event-routing.test.ts @@ -0,0 +1,121 @@ +import { describe, expect, it } from 'vitest' +import type { V2Event } from '@opencode-manager/shared/opencode' +import { createReplayEventBuilder, routeOpenCodeEvent } from '../../src/services/opencode-event-routing' + +const locatedStatus = { + id: 'evt_status_located', + created: 1, + type: 'session.status', + location: { directory: '/work/alpha' }, + data: { sessionID: 'ses_1', status: { type: 'busy' } }, +} satisfies V2Event + +const unlocatedStatus = { + id: 'evt_status_unlocated', + created: 2, + type: 'session.status', + data: { sessionID: 'ses_1', status: { type: 'idle' } }, +} satisfies V2Event + +const forkedSession = { + id: 'evt_forked', + created: 3, + type: 'session.forked', + durable: { aggregateID: 'ses_1', seq: 1, version: 2 }, + data: { + sessionID: 'ses_child', + parentID: 'ses_1', + boundary: { type: 'before', messageID: 'msg_1' }, + }, +} satisfies V2Event + +const childStatus = { + id: 'evt_status_child', + created: 4, + type: 'session.status', + data: { sessionID: 'ses_child', status: { type: 'busy' } }, +} satisfies V2Event + +const deletedSession = { + id: 'evt_deleted', + created: 5, + type: 'session.deleted', + durable: { aggregateID: 'ses_child', seq: 2, version: 2 }, + data: { sessionID: 'ses_child' }, +} satisfies V2Event + +describe('routeOpenCodeEvent', () => { + it('serializes the event payload and remembers its directory by session', () => { + const directories = new Map() + + const routed = routeOpenCodeEvent(locatedStatus, directories) + + expect(routed.event).toBe(locatedStatus) + expect(routed.payloadJson).toBe(JSON.stringify(locatedStatus)) + expect(routed.directory).toBe('/work/alpha') + expect(routed.resolvedDirectory).toBe('/work/alpha') + expect(directories.get('ses_1')).toBe('/work/alpha') + }) + + it('resolves a session directory remembered from an earlier event', () => { + const directories = new Map([['ses_1', '/work/alpha']]) + + const routed = routeOpenCodeEvent(unlocatedStatus, directories) + + expect(routed.directory).toBeUndefined() + expect(routed.resolvedDirectory).toBe('/work/alpha') + }) + + it('inherits the parent directory for a forked session', () => { + const directories = new Map([['ses_1', '/work/alpha']]) + + const routed = routeOpenCodeEvent(forkedSession, directories) + + expect(routed.directory).toBeUndefined() + expect(routed.resolvedDirectory).toBe('/work/alpha') + expect(directories.get('ses_child')).toBe('/work/alpha') + }) + + it('forgets the directory of a deleted session', () => { + const directories = new Map([['ses_child', '/work/alpha']]) + + const routed = routeOpenCodeEvent(deletedSession, directories) + expect(routed.resolvedDirectory).toBe('/work/alpha') + expect(directories.has('ses_child')).toBe(false) + + const afterDelete = routeOpenCodeEvent(childStatus, directories) + expect(afterDelete.resolvedDirectory).toBeUndefined() + expect(directories.has('ses_child')).toBe(false) + }) +}) + +describe('createReplayEventBuilder', () => { + it('builds typed replay events with incremental ids', () => { + const buildReplayEvent = createReplayEventBuilder('replay_') + + const permission = buildReplayEvent('permission.asked', '/work/alpha', { + id: 'perm_1', + sessionID: 'ses_1', + action: 'edit', + resources: ['/work/alpha/file.ts'], + }) + const form = buildReplayEvent('form.created', '/work/alpha', { + form: { + id: 'form_1', + sessionID: 'ses_1', + title: 'Question', + fields: [{ key: 'answer', type: 'string' }], + }, + }) + const status = buildReplayEvent('session.status', '/work/alpha', { + sessionID: 'ses_1', + status: { type: 'idle' }, + }) + + expect(permission.type).toBe('permission.asked') + expect(permission.location?.directory).toBe('/work/alpha') + expect(form.type).toBe('form.created') + expect(status.type).toBe('session.status') + expect([permission.id, form.id, status.id]).toEqual(['replay_1', 'replay_2', 'replay_3']) + }) +}) diff --git a/backend/test/services/opencode-event-subscription.test.ts b/backend/test/services/opencode-event-subscription.test.ts new file mode 100644 index 000000000..f23e0b71e --- /dev/null +++ b/backend/test/services/opencode-event-subscription.test.ts @@ -0,0 +1,342 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('../../src/utils/logger', () => ({ + logger: { + info: vi.fn(), + error: vi.fn(), + warn: vi.fn(), + }, +})) + +import { ClientError, type OpenCodeApi, type V2Event } from '@opencode-manager/shared/opencode' +import { subscribeOpenCodeEvents } from '../../src/services/opencode-event-subscription' + +const serverConnected = { + id: 'evt_connected', + type: 'server.connected', + data: {}, +} satisfies V2Event + +const sessionBusy = { + id: 'evt_busy', + created: 1, + type: 'session.status', + data: { sessionID: 'ses_1', status: { type: 'busy' } }, +} satisfies V2Event + +interface ControlledEventStream { + iterable: AsyncIterable + signal: AbortSignal + push: (event: V2Event) => void + end: () => void + fail: (error: unknown) => void +} + +interface FakeSubscription { + stream: ControlledEventStream + onActivity: (() => void) | undefined +} + +function createControlledEventStream(): ControlledEventStream { + const queue: V2Event[] = [] + let finished = false + let failure: { value: unknown } | null = null + let signal: AbortSignal + let notify: (() => void) | null = null + + const wake = (): void => { + const current = notify + notify = null + current?.() + } + + const iterable: AsyncIterable = { + [Symbol.asyncIterator]: () => ({ + async next() { + for (;;) { + if (queue.length > 0) return { value: queue.shift()!, done: false } + if (failure) { + const current = failure.value + failure = null + throw current + } + if (finished) return { value: undefined, done: true } + await new Promise((resolve) => { + notify = resolve + }) + } + }, + }), + } + + return { + iterable, + get signal() { + return signal + }, + set signal(next: AbortSignal) { + signal = next + next.addEventListener('abort', () => { + failure = { value: next.reason ?? new Error('aborted') } + wake() + }, { once: true }) + }, + push: (event) => { + queue.push(event) + wake() + }, + end: () => { + finished = true + wake() + }, + fail: (error) => { + failure = { value: error } + wake() + }, + } as ControlledEventStream +} + +function createFakeApi(): { api: OpenCodeApi; subscriptions: FakeSubscription[] } { + const subscriptions: FakeSubscription[] = [] + const api = { + event: { + subscribe: (options: { signal?: AbortSignal; onActivity?: () => void }) => { + const stream = createControlledEventStream() + stream.signal = options.signal! + subscriptions.push({ stream, onActivity: options.onActivity }) + return stream.iterable + }, + }, + } as unknown as OpenCodeApi + return { api, subscriptions } +} + +const flush = async (): Promise => { + await vi.advanceTimersByTimeAsync(0) +} + +beforeEach(() => { + vi.useFakeTimers() +}) + +afterEach(() => { + vi.useRealTimers() + vi.clearAllMocks() +}) + +describe('subscribeOpenCodeEvents', () => { + it('opens once on server.connected and forwards events in order', async () => { + const { api, subscriptions } = createFakeApi() + const onOpen = vi.fn() + const received: string[] = [] + const onDisconnect = vi.fn() + + const subscription = subscribeOpenCodeEvents({ + api: () => api, + label: 'test', + onOpen, + onEvent: (event) => { + received.push(event.type) + }, + onDisconnect, + }) + + expect(subscriptions).toHaveLength(1) + + subscriptions[0]!.stream.push(serverConnected) + subscriptions[0]!.stream.push(sessionBusy) + await flush() + + expect(onOpen).toHaveBeenCalledTimes(1) + expect(received).toEqual(['server.connected', 'session.status']) + expect(onDisconnect).not.toHaveBeenCalled() + + subscription.close() + }) + + it('keeps streaming when an event handler throws', async () => { + const { api, subscriptions } = createFakeApi() + const received: string[] = [] + const onDisconnect = vi.fn() + + const subscription = subscribeOpenCodeEvents({ + api: () => api, + label: 'test', + onOpen: () => undefined, + onEvent: (event) => { + received.push(event.type) + if (event.type === 'server.connected') throw new Error('handler failure') + }, + onDisconnect, + }) + + subscriptions[0]!.stream.push(serverConnected) + subscriptions[0]!.stream.push(sessionBusy) + await flush() + + expect(received).toEqual(['server.connected', 'session.status']) + expect(onDisconnect).not.toHaveBeenCalled() + + subscription.close() + }) + + it('reconnects after the stream ends once the reconnect delay elapses', async () => { + const { api, subscriptions } = createFakeApi() + const onOpen = vi.fn() + const onDisconnect = vi.fn() + + const subscription = subscribeOpenCodeEvents({ + api: () => api, + label: 'test', + onOpen, + onEvent: () => undefined, + onDisconnect, + reconnectDelayMs: 100, + maxReconnectDelayMs: 1000, + }) + + subscriptions[0]!.stream.push(serverConnected) + await flush() + expect(onOpen).toHaveBeenCalledTimes(1) + + subscriptions[0]!.stream.end() + await flush() + expect(onDisconnect).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(99) + expect(subscriptions).toHaveLength(1) + + await vi.advanceTimersByTimeAsync(1) + expect(subscriptions).toHaveLength(2) + + subscriptions[1]!.stream.push(serverConnected) + await flush() + expect(onOpen).toHaveBeenCalledTimes(2) + + subscription.close() + }) + + it('doubles the reconnect delay after a thrown transport error and caps it', async () => { + const { api, subscriptions } = createFakeApi() + const onDisconnect = vi.fn() + + const subscription = subscribeOpenCodeEvents({ + api: () => api, + label: 'test', + onOpen: () => undefined, + onEvent: () => undefined, + onDisconnect, + reconnectDelayMs: 100, + maxReconnectDelayMs: 150, + }) + + subscriptions[0]!.stream.fail(new ClientError('Transport')) + await flush() + expect(onDisconnect).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(99) + expect(subscriptions).toHaveLength(1) + await vi.advanceTimersByTimeAsync(1) + expect(subscriptions).toHaveLength(2) + + subscriptions[1]!.stream.fail(new ClientError('UnexpectedStatus')) + await flush() + expect(onDisconnect).toHaveBeenCalledTimes(2) + + await vi.advanceTimersByTimeAsync(149) + expect(subscriptions).toHaveLength(2) + await vi.advanceTimersByTimeAsync(1) + expect(subscriptions).toHaveLength(3) + + subscription.close() + }) + + it('aborts a stalled attempt and starts a new one', async () => { + const { api, subscriptions } = createFakeApi() + const onDisconnect = vi.fn() + + const subscription = subscribeOpenCodeEvents({ + api: () => api, + label: 'test', + onOpen: () => undefined, + onEvent: () => undefined, + onDisconnect, + reconnectDelayMs: 10, + stallThresholdMs: 100, + watchdogTickMs: 50, + }) + + subscriptions[0]!.stream.push(serverConnected) + await flush() + + await vi.advanceTimersByTimeAsync(100) + expect(subscriptions[0]!.stream.signal.aborted).toBe(false) + + await vi.advanceTimersByTimeAsync(50) + expect(subscriptions[0]!.stream.signal.aborted).toBe(true) + expect(onDisconnect).toHaveBeenCalledTimes(1) + + await vi.advanceTimersByTimeAsync(10) + expect(subscriptions).toHaveLength(2) + + subscription.close() + }) + + it('stops reconnecting after close', async () => { + const { api, subscriptions } = createFakeApi() + const onDisconnect = vi.fn() + + const subscription = subscribeOpenCodeEvents({ + api: () => api, + label: 'test', + onOpen: () => undefined, + onEvent: () => undefined, + onDisconnect, + reconnectDelayMs: 100, + }) + + subscriptions[0]!.stream.push(serverConnected) + await flush() + subscriptions[0]!.stream.end() + await flush() + expect(onDisconnect).toHaveBeenCalledTimes(1) + + subscription.close() + await vi.advanceTimersByTimeAsync(10_000) + + expect(subscriptions).toHaveLength(1) + }) + + it('reconnect aborts the current attempt and restarts with the base delay', async () => { + const { api, subscriptions } = createFakeApi() + + const subscription = subscribeOpenCodeEvents({ + api: () => api, + label: 'test', + onOpen: () => undefined, + onEvent: () => undefined, + onDisconnect: () => undefined, + reconnectDelayMs: 100, + maxReconnectDelayMs: 1000, + }) + + subscriptions[0]!.stream + .fail(new ClientError('Transport')) + await flush() + await vi.advanceTimersByTimeAsync(100) + expect(subscriptions).toHaveLength(2) + + subscription.reconnect() + expect(subscriptions).toHaveLength(3) + expect(subscriptions[1]!.stream.signal.aborted).toBe(true) + + subscriptions[2]!.stream.end() + await flush() + await vi.advanceTimersByTimeAsync(99) + expect(subscriptions).toHaveLength(3) + await vi.advanceTimersByTimeAsync(1) + expect(subscriptions).toHaveLength(4) + + subscription.close() + }) +}) From d7490841637864329f83a675cc7a4aaf6d89600c Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Mon, 2 Nov 2026 09:57:21 -0500 Subject: [PATCH 02/20] section 2: Phase 2: Local SSE aggregator streams through the local Open --- backend/src/index.ts | 3 +- backend/src/services/sse-aggregator.ts | 182 +++------ backend/test/index.test.ts | 3 +- .../services/remote-server-events.test.ts | 12 +- .../services/sse-aggregator-remote.test.ts | 35 +- backend/test/services/sse-aggregator.test.ts | 354 +++++++++++++----- 6 files changed, 338 insertions(+), 251 deletions(-) diff --git a/backend/src/index.ts b/backend/src/index.ts index 019a1f3c5..83ccc8584 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -359,8 +359,7 @@ if (ENV.VAPID.PUBLIC_KEY && ENV.VAPID.PRIVATE_KEY) { }) } -sseAggregator.setPendingActionsFetcher(openCodeClient) -sseAggregator.setPasswordResolver(() => new SettingsService(db).getOpenCodeServerPassword()) +sseAggregator.setOpenCodeClient(openCodeClient) sseAggregator.onServerConnected((serverId) => { void sessionPermissionModeService.acceptPendingRequestsForActiveSessions(serverId).catch((err) => { logger.error(`Failed to accept pending permission requests for server ${serverId}:`, err) diff --git a/backend/src/services/sse-aggregator.ts b/backend/src/services/sse-aggregator.ts index 5e75e9d59..68df187e3 100644 --- a/backend/src/services/sse-aggregator.ts +++ b/backend/src/services/sse-aggregator.ts @@ -1,20 +1,16 @@ -import { EventSource } from 'eventsource' import { logger } from '../utils/logger' -import { DEFAULTS } from '@opencode-manager/shared/config' import { buildServerDirectoryKey, LOCAL_OPENCODE_SERVER_ID, parseServerDirectoryKey } from '@opencode-manager/shared/schemas' -import { type OpenCodeApi, type V2Event } from '@opencode-manager/shared/opencode' -import { getOpenCodeBasicAuthHeader, type OpenCodePasswordResolver } from './opencode/auth' -import { getOpenCodeUpstreamBaseUrl } from './opencode/upstream' +import { type V2Event } from '@opencode-manager/shared/opencode' +import type { OpenCodeClient } from './opencode/client' import { encodeSSEFrame } from '../utils/sse-frame' import { collectPendingReplayEvents, - createAuthorizedEventSourceInit, createReplayEventBuilder, - nextBackoffDelay, - parseOpenCodeEvent, planSessionStatusReplay, + routeOpenCodeEvent, type ReplayEventBuilder, } from './opencode-event-routing' +import { subscribeOpenCodeEvents, type OpenCodeEventSubscription } from './opencode-event-subscription' type SSEClientCallback = (event: string, data: string) => void type SSEClientFrameWriter = (frame: Uint8Array) => void @@ -37,16 +33,11 @@ interface SSEClient { export type SSEEvent = V2Event -export interface PendingActionsFetcher { - api: OpenCodeApi -} - export interface ScheduledSessionRef { sessionID: string directory: string } -const { RECONNECT_DELAY_MS, MAX_RECONNECT_DELAY_MS } = DEFAULTS.SSE const LOCATION_CATALOG_EVENT_TYPES = new Set(['command.updated', 'agent.updated', 'skill.updated', 'model.updated', 'provider.updated', 'integration.updated']) function serializeEnvelope(directory: string | null, payloadJson: string, serverId?: string): string { @@ -66,26 +57,19 @@ class SSEAggregator { private serverConnectedListeners: Set = new Set() private remoteSubagentResolver: RemoteSubagentResolver | null = null private subagentSessions: Map> = new Map() - private upstream: EventSource | null = null - private reconnectTimeout: ReturnType | null = null - private reconnectDelay: number = RECONNECT_DELAY_MS + private subscription: OpenCodeEventSubscription | null = null private upstreamConnected = false private everConnected = false private started = false - private pendingActionsFetcher: PendingActionsFetcher | null = null - private passwordResolver: OpenCodePasswordResolver | null = null + private openCodeClient: Pick | null = null private scheduledSessionsResolver: (() => ScheduledSessionRef[]) | null = null private remoteReplayHandler: RemoteReplayHandler | null = null private readonly buildReplayEvent: ReplayEventBuilder = createReplayEventBuilder('ocm_replay_') private constructor() {} - setPendingActionsFetcher(fetcher: PendingActionsFetcher | null): void { - this.pendingActionsFetcher = fetcher - } - - setPasswordResolver(resolver: OpenCodePasswordResolver | null): void { - this.passwordResolver = resolver + setOpenCodeClient(client: Pick | null): void { + this.openCodeClient = client } setScheduledSessionsResolver(resolver: () => ScheduledSessionRef[]): void { @@ -99,12 +83,7 @@ class SSEAggregator { reconnect(): void { if (!this.started) return logger.info('SSE forcing upstream reconnect (auth changed)') - if (this.reconnectTimeout) { - clearTimeout(this.reconnectTimeout) - this.reconnectTimeout = null - } - this.reconnectDelay = RECONNECT_DELAY_MS - void this.connectUpstream() + this.subscription?.reconnect() } static getInstance(): SSEAggregator { @@ -116,8 +95,26 @@ class SSEAggregator { start(): void { if (this.started) return + const client = this.openCodeClient + if (!client) { + logger.warn('SSE aggregator cannot start without an OpenCode client') + return + } this.started = true - void this.connectUpstream() + this.subscription = subscribeOpenCodeEvents({ + api: () => client.api, + label: 'SSE aggregator', + onOpen: () => { + this.handleUpstreamOpen(this.everConnected) + }, + onEvent: (event) => { + this.handleUpstreamEvent(event) + }, + onDisconnect: (error) => { + this.upstreamConnected = false + logger.warn(`SSE upstream disconnected: ${String(error)}`) + }, + }) } addClient(id: string, callback: SSEClientCallback, writeFrame: SSEClientFrameWriter, directories: string[]): () => void { @@ -250,21 +247,21 @@ class SSEAggregator { } private async replayPendingActionsForClient(clientId: string, directories: string[]): Promise { - const fetcher = this.pendingActionsFetcher - if (!fetcher) return + const client = this.openCodeClient + if (!client) return await Promise.allSettled(directories.map(directory => - this.replayPendingActionsForDirectory(clientId, directory, fetcher) + this.replayPendingActionsForDirectory(clientId, directory, client) )) } private async replayPendingActionsForDirectory( clientId: string, directory: string, - fetcher: PendingActionsFetcher, + client: Pick, ): Promise { const events = await collectPendingReplayEvents({ - api: fetcher.api, + api: client.api, directory, buildEvent: this.buildReplayEvent, logScope: 'replay', @@ -294,12 +291,12 @@ class SSEAggregator { } private async replaySessionStatusesForTrackedDirectories(): Promise { - const fetcher = this.pendingActionsFetcher - if (!fetcher) return + const client = this.openCodeClient + if (!client) return let running: Record try { - running = await fetcher.api.session.active() + running = await client.api.session.active() } catch (error) { logger.warn(`replay: failed to fetch active sessions: ${String(error)}`) return @@ -317,7 +314,7 @@ class SSEAggregator { const runningDirectories = await Promise.all(plan.busy.map(async (sessionID) => ({ sessionID, - directory: tracked.get(sessionID) ?? await this.resolveSessionDirectory(fetcher, sessionID), + directory: tracked.get(sessionID) ?? await this.resolveSessionDirectory(client, sessionID), }))) for (const { sessionID, directory } of runningDirectories) { @@ -343,9 +340,9 @@ class SSEAggregator { return tracked } - private async resolveSessionDirectory(fetcher: PendingActionsFetcher, sessionID: string): Promise { + private async resolveSessionDirectory(client: Pick, sessionID: string): Promise { try { - const session = await fetcher.api.session.get({ sessionID }) + const session = await client.api.session.get({ sessionID }) this.sessionDirectories.set(sessionID, session.location.directory) return session.location.directory } catch (error) { @@ -354,53 +351,9 @@ class SSEAggregator { } } - private async connectUpstream(): Promise { - if (!this.started) return - if (this.upstream) { - this.upstream.close() - this.upstream = null - } - - const url = `${getOpenCodeUpstreamBaseUrl()}/api/event` - const wasConnectedBefore = this.everConnected - logger.info(`SSE connecting to OpenCode global stream: ${url}`) - - const authHeader = this.passwordResolver - ? await getOpenCodeBasicAuthHeader(this.passwordResolver) - : null - - if (!this.started) return - - const init: ConstructorParameters[1] = createAuthorizedEventSourceInit(authHeader) - - const es = new EventSource(url, init) - this.upstream = es - - es.onopen = () => { - this.handleUpstreamOpen(wasConnectedBefore) - } - - es.onerror = (event) => { - this.upstreamConnected = false - if (es === this.upstream) { - const code = (event as { code?: number }).code - const message = (event as { message?: string }).message - logger.warn(`SSE upstream error${code ? ` (code=${code})` : ''}${message ? `: ${message}` : ''}`) - es.close() - this.upstream = null - this.scheduleReconnect() - } - } - - es.onmessage = (event) => { - this.handleUpstreamMessage(event.data) - } - } - private handleUpstreamOpen(wasConnectedBefore: boolean): void { logger.info('SSE global stream connected') this.upstreamConnected = true - this.reconnectDelay = RECONNECT_DELAY_MS this.everConnected = true this.notifyUpstreamConnected() this.broadcastResync() @@ -420,15 +373,6 @@ class SSEAggregator { this.broadcastToAll('resync', JSON.stringify({ timestamp: Date.now() })) } - private scheduleReconnect(): void { - if (!this.started || this.reconnectTimeout) return - this.reconnectTimeout = setTimeout(() => { - this.reconnectTimeout = null - this.reconnectDelay = nextBackoffDelay(this.reconnectDelay, MAX_RECONNECT_DELAY_MS) - void this.connectUpstream() - }, this.reconnectDelay) - } - onEvent(listener: SSEEventListener): () => void { this.eventListeners.add(listener) return () => { this.eventListeners.delete(listener) } @@ -465,32 +409,25 @@ class SSEAggregator { this.remoteSubagentResolver = resolver } - private handleUpstreamMessage(data: string): void { - const routed = parseOpenCodeEvent(data, this.sessionDirectories) - if (!routed) return + private handleUpstreamEvent(event: V2Event): void { + const { payloadJson, directory, resolvedDirectory } = routeOpenCodeEvent(event, this.sessionDirectories) - const { event, payloadJson, directory, resolvedDirectory } = routed - - try { - if (directory) { - if (LOCATION_CATALOG_EVENT_TYPES.has(event.type)) { - this.deliverResolvedEvent(directory, event, payloadJson) - } else { - this.deliverEvent(directory, event, payloadJson) - } - return - } - - if (resolvedDirectory) { - this.deliverResolvedEvent(resolvedDirectory, event, payloadJson) - return + if (directory) { + if (LOCATION_CATALOG_EVENT_TYPES.has(event.type)) { + this.deliverResolvedEvent(directory, event, payloadJson) + } else { + this.deliverEvent(directory, event, payloadJson) } + return + } - this.notifyListeners(null, event) - this.writeEnvelopeToClients(this.clients.keys(), null, payloadJson) - } catch (error) { - logger.error(`SSE failed to handle ${event.type} event:`, error) + if (resolvedDirectory) { + this.deliverResolvedEvent(resolvedDirectory, event, payloadJson) + return } + + this.notifyListeners(null, event) + this.writeEnvelopeToClients(this.clients.keys(), null, payloadJson) } private deliverEvent(directory: string, event: SSEEvent, payloadJson?: string): void { @@ -731,15 +668,10 @@ class SSEAggregator { shutdown(): void { this.started = false + this.everConnected = false - if (this.reconnectTimeout) { - clearTimeout(this.reconnectTimeout) - this.reconnectTimeout = null - } - if (this.upstream) { - this.upstream.close() - this.upstream = null - } + this.subscription?.close() + this.subscription = null this.upstreamConnected = false this.activeSessions.clear() diff --git a/backend/test/index.test.ts b/backend/test/index.test.ts index 2772bab6f..c2d60ad03 100644 --- a/backend/test/index.test.ts +++ b/backend/test/index.test.ts @@ -125,8 +125,7 @@ const sseAggregatorMock = vi.hoisted(() => ({ onServerEvent: vi.fn(), onUpstreamConnected: vi.fn(), onServerConnected: vi.fn(), - setPendingActionsFetcher: vi.fn(), - setPasswordResolver: vi.fn(), + setOpenCodeClient: vi.fn(), setScheduledSessionsResolver: vi.fn(), setRemoteReplayHandler: vi.fn(), setRemoteSubagentResolver: vi.fn(), diff --git a/backend/test/services/remote-server-events.test.ts b/backend/test/services/remote-server-events.test.ts index 97248a439..5b70b3695 100644 --- a/backend/test/services/remote-server-events.test.ts +++ b/backend/test/services/remote-server-events.test.ts @@ -219,7 +219,7 @@ describe('RemoteServerEventHub event delivery', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) fake = createRegistry([SERVER_A, SERVER_B]) sources = createSourceFactory() @@ -434,7 +434,7 @@ describe('RemoteServerEventHub reconnection', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) fake = createRegistry([SERVER_A]) sources = createSourceFactory() @@ -654,7 +654,7 @@ describe('RemoteServerEventHub failure handling', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) fake = createRegistry([SERVER_A]) sources = createSourceFactory() @@ -768,7 +768,7 @@ describe('RemoteServerEventHub pending action replay', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) fake = createRegistry([SERVER_A], { permissions: { '/repo': [{ id: 'perm-1', sessionID: 'ses-1', action: 'shell', resources: ['ls'] }] }, @@ -858,7 +858,7 @@ describe('RemoteServerEventHub pending action replay', () => { it('does not use the local pending action fetcher for remote keys', async () => { const localApi = makeApi({ permissions: { '/repo': [{ id: 'local-perm' }] } }) - sseAggregator.setPendingActionsFetcher({ api: localApi }) + sseAggregator.setOpenCodeClient({ api: localApi }) hub.start() trackSessionStatus('ses-1', '/repo', 'busy') @@ -913,7 +913,7 @@ describe('RemoteServerEventHub session status replay', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) sources = createSourceFactory() }) diff --git a/backend/test/services/sse-aggregator-remote.test.ts b/backend/test/services/sse-aggregator-remote.test.ts index 5e9fbbb61..f854717f6 100644 --- a/backend/test/services/sse-aggregator-remote.test.ts +++ b/backend/test/services/sse-aggregator-remote.test.ts @@ -14,8 +14,9 @@ vi.mock('../../src/utils/logger', () => ({ }, })) -import type { OpenCodeApi } from '@opencode-manager/shared/opencode' -import { sseAggregator, type PendingActionsFetcher } from '../../src/services/sse-aggregator' +import type { OpenCodeApi, V2Event } from '@opencode-manager/shared/opencode' +import { sseAggregator } from '../../src/services/sse-aggregator' +import type { OpenCodeClient } from '../../src/services/opencode/client' interface CapturedClient { events: Array<{ event: string; data: string }> @@ -40,7 +41,7 @@ function parseFrame(frame: string): { directory: string | null; serverId?: strin return JSON.parse(frame.replace(/^event: message\ndata: /, '').trim()) } -function makeFetcher(): PendingActionsFetcher { +function makeClient(): Pick { const api = { permission: { request: { @@ -68,10 +69,12 @@ async function flushReplay(): Promise { const PAYLOAD = '{"id":"evt_1","created":1,"type":"session.idle","location":{"directory":"/repo"},"data":{"sessionID":"ses_1"}}' +const LOCAL_EVENT = JSON.parse(PAYLOAD) as V2Event + describe('SSEAggregator remote envelope delivery', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) }) @@ -159,16 +162,16 @@ describe('SSEAggregator remote envelope delivery', () => { const client = createCapturingClient() sseAggregator.addClient('local-envelope', client.callback, client.writeFrame, ['/repo']) - ;(sseAggregator as unknown as { handleUpstreamMessage(data: string): void }).handleUpstreamMessage(PAYLOAD) + ;(sseAggregator as unknown as { handleUpstreamEvent(event: V2Event): void }).handleUpstreamEvent(LOCAL_EVENT) - expect(client.frames).toEqual([`event: message\ndata: {"directory":"/repo","payload":${PAYLOAD}}\n\n`]) + expect(client.frames).toEqual([`event: message\ndata: {"directory":"/repo","payload":${JSON.stringify(LOCAL_EVENT)}}\n\n`]) expect(client.frames[0]).not.toContain('serverId') expect(parseFrame(client.frames[0]!).serverId).toBeUndefined() }) - it('routes remote subscriptions to the remote replay hook instead of the local fetcher', async () => { - const fetcher = makeFetcher() - sseAggregator.setPendingActionsFetcher(fetcher) + it('routes remote subscriptions to the remote replay hook instead of the local client', async () => { + const openCodeClient = makeClient() + sseAggregator.setOpenCodeClient(openCodeClient) const remoteReplay = vi.fn(async () => {}) sseAggregator.addClient('mixed', () => {}, () => {}, ['/repo', 'srv_a|/repo']) @@ -179,21 +182,21 @@ describe('SSEAggregator remote envelope delivery', () => { expect(remoteReplay).toHaveBeenCalledTimes(1) expect(remoteReplay).toHaveBeenCalledWith('mixed', 'srv_a', '/new') - expect(fetcher.api.permission.request.list).toHaveBeenCalledTimes(1) - expect(fetcher.api.permission.request.list).toHaveBeenCalledWith({ location: { directory: '/repo' } }) + expect(openCodeClient.api.permission.request.list).toHaveBeenCalledTimes(1) + expect(openCodeClient.api.permission.request.list).toHaveBeenCalledWith({ location: { directory: '/repo' } }) }) - it('does not replay remote keys through the local fetcher on connect', async () => { - const fetcher = makeFetcher() - sseAggregator.setPendingActionsFetcher(fetcher) + it('does not replay remote keys through the local client on connect', async () => { + const openCodeClient = makeClient() + sseAggregator.setOpenCodeClient(openCodeClient) const remoteReplay = vi.fn() sseAggregator.setRemoteReplayHandler(remoteReplay) sseAggregator.addClient('remote-only', () => {}, () => {}, ['srv_a|/repo']) await flushReplay() - expect(fetcher.api.permission.request.list).not.toHaveBeenCalled() - expect(fetcher.api.form.list).not.toHaveBeenCalled() + expect(openCodeClient.api.permission.request.list).not.toHaveBeenCalled() + expect(openCodeClient.api.form.list).not.toHaveBeenCalled() expect(remoteReplay).toHaveBeenCalledWith('remote-only', 'srv_a', '/repo') }) diff --git a/backend/test/services/sse-aggregator.test.ts b/backend/test/services/sse-aggregator.test.ts index 5ea5cd275..7e05599b3 100644 --- a/backend/test/services/sse-aggregator.test.ts +++ b/backend/test/services/sse-aggregator.test.ts @@ -4,6 +4,7 @@ vi.mock('@opencode-manager/shared/config/env', () => ({ ENV: { OPENCODE: { PORT: 5551, HOST: '127.0.0.1' }, }, + getWorkspacePath: () => '/workspace', })) vi.mock('../../src/utils/logger', () => ({ @@ -14,9 +15,11 @@ vi.mock('../../src/utils/logger', () => ({ }, })) -import type { OpenCodeApi } from '@opencode-manager/shared/opencode' -import { sessionIDFromEvent } from '@opencode-manager/shared/opencode' -import { sseAggregator, type PendingActionsFetcher } from '../../src/services/sse-aggregator' +import type { OpenCodeApi, V2Event } from '@opencode-manager/shared/opencode' +import { buildOpenCodeBasicAuth, sessionIDFromEvent } from '@opencode-manager/shared/opencode' +import { sseAggregator } from '../../src/services/sse-aggregator' +import { FetchOpenCodeClient } from '../../src/services/opencode/client' +import { getOpenCodeUpstreamBaseUrl } from '../../src/services/opencode/upstream' interface CapturedEvent { event: string @@ -36,7 +39,7 @@ function createCapturingClient() { return { callback, writeFrame, events, frames } } -interface RawEventInput { +interface EventInput { id?: string created?: number type: string @@ -44,14 +47,14 @@ interface RawEventInput { data?: unknown } -function rawEvent(input: RawEventInput): string { - return JSON.stringify({ +function eventOf(input: EventInput): V2Event { + return { id: input.id ?? 'evt_1', created: input.created ?? 1, type: input.type, ...(input.location ? { location: input.location } : {}), data: input.data ?? {}, - }) + } as unknown as V2Event } function parseFrame(frame: string): { directory: string | null; payload: { id?: string; type: string; data: Record } } { @@ -94,12 +97,12 @@ function makeApi(fixture: ApiFixture = {}): OpenCodeApi { } as unknown as OpenCodeApi } -function makeFetcher(fixture: ApiFixture = {}): PendingActionsFetcher { +function makeClient(fixture: ApiFixture = {}): { api: OpenCodeApi } { return { api: makeApi(fixture) } } -function emitRawEvent(input: RawEventInput): void { - ;(sseAggregator as unknown as { handleUpstreamMessage(data: string): void }).handleUpstreamMessage(rawEvent(input)) +function emitEvent(input: EventInput): void { + ;(sseAggregator as unknown as { handleUpstreamEvent(event: V2Event): void }).handleUpstreamEvent(eventOf(input)) } async function flushReplay(): Promise { @@ -108,20 +111,20 @@ async function flushReplay(): Promise { } } -describe('SSEAggregator raw V2 event handling', () => { +describe('SSEAggregator V2 event handling', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) }) - it('marks a session active from a raw V2 session.status busy event and routes it only to subscribed clients', () => { + it('marks a session active from a V2 session.status busy event and routes it only to subscribed clients', () => { const clientA = createCapturingClient() const clientB = createCapturingClient() sseAggregator.addClient('active-a', clientA.callback, clientA.writeFrame, ['/r']) sseAggregator.addClient('active-b', clientB.callback, clientB.writeFrame, ['/other']) - emitRawEvent({ + emitEvent({ id: 'evt_1', type: 'session.status', location: { directory: '/r' }, @@ -149,7 +152,7 @@ describe('SSEAggregator raw V2 event handling', () => { sseAggregator.addClient('global-a', subscribed.callback, subscribed.writeFrame, ['/r']) sseAggregator.addClient('global-b', unsubscribed.callback, unsubscribed.writeFrame, []) - emitRawEvent({ id: 'evt_global', type: 'server.connected', data: {} }) + emitEvent({ id: 'evt_global', type: 'server.connected', data: {} }) const expected = { directory: null, @@ -160,56 +163,51 @@ describe('SSEAggregator raw V2 event handling', () => { expect(sseAggregator.getActiveSessions()).toEqual({}) }) - it('embeds the raw upstream payload in the client envelope without re-serializing it', () => { - const client = createCapturingClient() - sseAggregator.addClient('raw-a', client.callback, client.writeFrame, ['/r']) - const raw = '{"type":"session.idle","id":"evt_raw","created":7,"location":{"directory":"/r"},"data":{"sessionID":"ses_1","extra":{"b":2,"a":1}}}' - - ;(sseAggregator as unknown as { handleUpstreamMessage(data: string): void }).handleUpstreamMessage(raw) - - expect(client.frames).toEqual([`event: message\ndata: {"directory":"/r","payload":${raw}}\n\n`]) - expect(parseFrame(client.frames[0]!)).toEqual({ directory: '/r', payload: JSON.parse(raw) }) - }) - - it('re-serializes a multi-line upstream payload so the client frame stays a single data line', () => { + it('delivers the parsed event payload in a single data line', () => { const client = createCapturingClient() - sseAggregator.addClient('raw-b', client.callback, client.writeFrame, ['/r']) - const event = { type: 'session.idle', id: 'evt_ml', created: 3, location: { directory: '/r' }, data: { sessionID: 'ses_1' } } + sseAggregator.addClient('payload-a', client.callback, client.writeFrame, ['/r']) + const event = eventOf({ + id: 'evt_raw', + created: 7, + type: 'session.idle', + location: { directory: '/r' }, + data: { sessionID: 'ses_1', extra: { b: 2, a: 1 } }, + }) - ;(sseAggregator as unknown as { handleUpstreamMessage(data: string): void }).handleUpstreamMessage(JSON.stringify(event, null, 2)) + ;(sseAggregator as unknown as { handleUpstreamEvent(event: V2Event): void }).handleUpstreamEvent(event) - expect(client.frames).toHaveLength(1) - expect(client.frames[0]!.split('\n').filter(line => line.startsWith('data: '))).toHaveLength(1) + expect(client.frames).toEqual([`event: message\ndata: {"directory":"/r","payload":${JSON.stringify(event)}}\n\n`]) expect(parseFrame(client.frames[0]!)).toEqual({ directory: '/r', payload: event }) + expect(client.frames[0]!.split('\n').filter(line => line.startsWith('data: '))).toHaveLength(1) }) it('marks a session idle on session.idle and on terminal execution events', () => { - emitRawEvent({ type: 'session.status', location: { directory: '/r' }, data: { sessionID: 'ses_1', status: { type: 'busy' } } }) - emitRawEvent({ type: 'session.execution.started', location: { directory: '/r' }, data: { sessionID: 'ses_2' } }) + emitEvent({ type: 'session.status', location: { directory: '/r' }, data: { sessionID: 'ses_1', status: { type: 'busy' } } }) + emitEvent({ type: 'session.execution.started', location: { directory: '/r' }, data: { sessionID: 'ses_2' } }) expect(sseAggregator.getActiveSessions()).toEqual({ '/r': ['ses_1', 'ses_2'] }) - emitRawEvent({ type: 'session.execution.succeeded', location: { directory: '/r' }, data: { sessionID: 'ses_1' } }) + emitEvent({ type: 'session.execution.succeeded', location: { directory: '/r' }, data: { sessionID: 'ses_1' } }) expect(sseAggregator.getActiveSessions()).toEqual({ '/r': ['ses_2'] }) - emitRawEvent({ type: 'session.execution.failed', location: { directory: '/r' }, data: { sessionID: 'ses_2', error: { type: 'unknown', message: 'boom' } } }) - emitRawEvent({ type: 'session.status', location: { directory: '/r' }, data: { sessionID: 'ses_3', status: { type: 'retry', attempt: 1, message: 'retrying', next: 1 } } }) + emitEvent({ type: 'session.execution.failed', location: { directory: '/r' }, data: { sessionID: 'ses_2', error: { type: 'unknown', message: 'boom' } } }) + emitEvent({ type: 'session.status', location: { directory: '/r' }, data: { sessionID: 'ses_3', status: { type: 'retry', attempt: 1, message: 'retrying', next: 1 } } }) expect(sseAggregator.getActiveSessions()).toEqual({ '/r': ['ses_3'] }) - emitRawEvent({ type: 'session.idle', location: { directory: '/r' }, data: { sessionID: 'ses_3' } }) + emitEvent({ type: 'session.idle', location: { directory: '/r' }, data: { sessionID: 'ses_3' } }) expect(sseAggregator.getActiveSessions()).toEqual({}) }) it('marks a session idle on session.execution.interrupted', () => { - emitRawEvent({ type: 'session.execution.started', location: { directory: '/r' }, data: { sessionID: 'ses_1' } }) - emitRawEvent({ type: 'session.execution.interrupted', location: { directory: '/r' }, data: { sessionID: 'ses_1', reason: 'shutdown' } }) + emitEvent({ type: 'session.execution.started', location: { directory: '/r' }, data: { sessionID: 'ses_1' } }) + emitEvent({ type: 'session.execution.interrupted', location: { directory: '/r' }, data: { sessionID: 'ses_1', reason: 'shutdown' } }) expect(sseAggregator.getActiveSessions()).toEqual({}) }) it('records subagent sessions from session.created parentID and forgets deleted ones', () => { - emitRawEvent({ + emitEvent({ type: 'session.created', location: { directory: '/r' }, data: { sessionID: 'ses_sub', parentID: 'ses_root', projectID: 'p', location: { directory: '/r' }, slug: 'sub', version: '2.0.15' }, @@ -217,35 +215,21 @@ describe('SSEAggregator raw V2 event handling', () => { expect(sseAggregator.isSubagentSession('ses_sub')).toBe(true) - emitRawEvent({ type: 'session.deleted', location: { directory: '/r' }, data: { sessionID: 'ses_sub' } }) + emitEvent({ type: 'session.deleted', location: { directory: '/r' }, data: { sessionID: 'ses_sub' } }) expect(sseAggregator.isSubagentSession('ses_sub')).toBe(false) }) - - it('ignores malformed upstream payloads', () => { - const client = createCapturingClient() - sseAggregator.addClient('malformed-a', client.callback, client.writeFrame, ['/r']) - - ;(sseAggregator as unknown as { handleUpstreamMessage(data: string): void }).handleUpstreamMessage('not-json') - ;(sseAggregator as unknown as { handleUpstreamMessage(data: string): void }).handleUpstreamMessage(rawEvent({ - type: 'session.status', - location: { directory: '/r' }, - data: null, - })) - - expect(client.frames).toHaveLength(0) - }) }) describe('SSEAggregator pending replay on connect', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) }) it('replays pending permissions and forms from the V2 api per subscribed directory', async () => { - sseAggregator.setPendingActionsFetcher(makeFetcher({ + sseAggregator.setOpenCodeClient(makeClient({ permissions: { '/repo/a': [ { id: 'perm-1', sessionID: 'ses-a', action: 'shell', resources: ['rm -rf node_modules'] }, @@ -301,7 +285,7 @@ describe('SSEAggregator pending replay on connect', () => { }) it('does not replay to other clients', async () => { - sseAggregator.setPendingActionsFetcher(makeFetcher({ + sseAggregator.setOpenCodeClient(makeClient({ permissions: { '/repo/a': [{ id: 'perm-1', sessionID: 'ses-a', action: 'shell', resources: ['ls'] }] }, })) @@ -318,7 +302,7 @@ describe('SSEAggregator pending replay on connect', () => { }) it('replays only newly added directories on addDirectories', async () => { - sseAggregator.setPendingActionsFetcher(makeFetcher({ + sseAggregator.setOpenCodeClient(makeClient({ permissions: { '/repo/a': [{ id: 'perm-1', sessionID: 'ses-a', action: 'shell', resources: ['ls'] }], '/repo/b': [{ id: 'perm-2', sessionID: 'ses-b', action: 'shell', resources: ['ls'] }], @@ -353,7 +337,7 @@ describe('SSEAggregator pending replay on connect', () => { if (input?.location?.directory === '/repo/broken') throw new Error('upstream down') return { location: { directory: input?.location?.directory ?? '' }, data: [{ id: 'perm-ok', sessionID: 'ses-ok', action: 'shell', resources: ['ls'] }] } }) - sseAggregator.setPendingActionsFetcher({ api }) + sseAggregator.setOpenCodeClient({ api }) const { callback, writeFrame, events } = createCapturingClient() sseAggregator.addClient('client-4', callback, writeFrame, ['/repo/broken', '/repo/ok']) @@ -372,7 +356,7 @@ describe('SSEAggregator pending replay on connect', () => { vi.mocked(list).mockImplementation(() => new Promise((resolve) => { resolvePermissions = resolve as (value: { location: { directory: string }; data: unknown[] }) => void })) - sseAggregator.setPendingActionsFetcher({ api }) + sseAggregator.setOpenCodeClient({ api }) const { callback, writeFrame, events } = createCapturingClient() sseAggregator.addClient('client-5', callback, writeFrame, ['/repo/a']) @@ -389,7 +373,7 @@ describe('SSEAggregator pending replay on connect', () => { describe('SSEAggregator session status replay on upstream reconnect', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) }) @@ -397,9 +381,9 @@ describe('SSEAggregator session status replay on upstream reconnect', () => { const client = createCapturingClient() sseAggregator.addClient('status-1', client.callback, client.writeFrame, ['/repo/a']) - emitRawEvent({ type: 'session.status', location: { directory: '/repo/a' }, data: { sessionID: 'ses-1', status: { type: 'busy' } } }) + emitEvent({ type: 'session.status', location: { directory: '/repo/a' }, data: { sessionID: 'ses-1', status: { type: 'busy' } } }) - sseAggregator.setPendingActionsFetcher(makeFetcher({ active: { 'ses-1': { type: 'running' } } })) + sseAggregator.setOpenCodeClient(makeClient({ active: { 'ses-1': { type: 'running' } } })) await (sseAggregator as unknown as { replaySessionStatusesForTrackedDirectories(): Promise }).replaySessionStatusesForTrackedDirectories() await flushReplay() @@ -413,9 +397,9 @@ describe('SSEAggregator session status replay on upstream reconnect', () => { const client = createCapturingClient() sseAggregator.addClient('status-2', client.callback, client.writeFrame, ['/repo/a']) - emitRawEvent({ type: 'session.status', location: { directory: '/repo/a' }, data: { sessionID: 'ses-9', status: { type: 'busy' } } }) + emitEvent({ type: 'session.status', location: { directory: '/repo/a' }, data: { sessionID: 'ses-9', status: { type: 'busy' } } }) - sseAggregator.setPendingActionsFetcher(makeFetcher({ active: {} })) + sseAggregator.setOpenCodeClient(makeClient({ active: {} })) await (sseAggregator as unknown as { replaySessionStatusesForTrackedDirectories(): Promise }).replaySessionStatusesForTrackedDirectories() await flushReplay() @@ -431,7 +415,7 @@ describe('SSEAggregator session status replay on upstream reconnect', () => { active: { 'ses-new': { type: 'running' } }, sessions: { 'ses-new': { directory: '/repo/new' } }, }) - sseAggregator.setPendingActionsFetcher({ api }) + sseAggregator.setOpenCodeClient({ api }) const client = createCapturingClient() sseAggregator.addClient('status-3', client.callback, client.writeFrame, ['/repo/new']) @@ -453,7 +437,7 @@ describe('SSEAggregator session status replay on upstream reconnect', () => { const sessionID = (input as { sessionID: string }).sessionID pending.push(() => resolve({ location: { directory: `/repo/${sessionID}` } } as unknown as Awaited>)) })) - sseAggregator.setPendingActionsFetcher({ api }) + sseAggregator.setOpenCodeClient({ api }) const client = createCapturingClient() sseAggregator.addClient('status-5', client.callback, client.writeFrame, ['/repo/ses-x', '/repo/ses-y']) @@ -469,10 +453,10 @@ describe('SSEAggregator session status replay on upstream reconnect', () => { }) it('does not replay pending actions to already connected clients on upstream reconnect', async () => { - const fetcher = makeFetcher({ + const fetcher = makeClient({ permissions: { '/repo/a': [{ id: 'perm-1', sessionID: 'ses-a', action: 'shell', resources: ['ls'] }] }, }) - sseAggregator.setPendingActionsFetcher(fetcher) + sseAggregator.setOpenCodeClient(fetcher) const client = createCapturingClient() sseAggregator.addClient('status-6', client.callback, client.writeFrame, ['/repo/a']) await flushReplay() @@ -503,12 +487,12 @@ describe('SSEAggregator session status replay on upstream reconnect', () => { describe('SSEAggregator scheduled session replay without a connected client', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) }) it('replays a scheduled directory that no browser client is subscribed to', async () => { - sseAggregator.setPendingActionsFetcher(makeFetcher({ + sseAggregator.setOpenCodeClient(makeClient({ active: { 'ses-sched': { type: 'running' } }, })) sseAggregator.setScheduledSessionsResolver(() => [ @@ -535,7 +519,7 @@ describe('SSEAggregator scheduled session replay without a connected client', () }) it('emits idle for a scheduled session that finished while the stream was down and was never marked active', async () => { - sseAggregator.setPendingActionsFetcher(makeFetcher({ active: {} })) + sseAggregator.setOpenCodeClient(makeClient({ active: {} })) sseAggregator.setScheduledSessionsResolver(() => [ { sessionID: 'ses-finished', directory: '/worktrees/run-2' }, ]) @@ -557,7 +541,7 @@ describe('SSEAggregator scheduled session replay without a connected client', () describe('SSEAggregator upstream resynchronization signal', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) }) @@ -616,7 +600,7 @@ describe('SSEAggregator upstream resynchronization signal', () => { describe('SSEAggregator directory-indexed broadcast', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) }) @@ -626,7 +610,7 @@ describe('SSEAggregator directory-indexed broadcast', () => { sseAggregator.addClient('index-a', clientA.callback, clientA.writeFrame, ['/a']) sseAggregator.addClient('index-b', clientB.callback, clientB.writeFrame, ['/b']) - emitRawEvent({ type: 'session.idle', location: { directory: '/a' }, data: { sessionID: 'ses-1' } }) + emitEvent({ type: 'session.idle', location: { directory: '/a' }, data: { sessionID: 'ses-1' } }) expect(clientA.frames).toHaveLength(1) expect(clientB.frames).toHaveLength(0) @@ -636,7 +620,7 @@ describe('SSEAggregator directory-indexed broadcast', () => { const clientA = createCapturingClient() sseAggregator.addClient('index-c', clientA.callback, clientA.writeFrame, ['/a']) - emitRawEvent({ type: 'session.idle', location: { directory: '/z' }, data: { sessionID: 'ses-1' } }) + emitEvent({ type: 'session.idle', location: { directory: '/z' }, data: { sessionID: 'ses-1' } }) expect(clientA.frames).toHaveLength(0) }) @@ -646,7 +630,7 @@ describe('SSEAggregator directory-indexed broadcast', () => { sseAggregator.addClient('index-d', clientA.callback, clientA.writeFrame, ['/a']) sseAggregator.removeClient('index-d') - emitRawEvent({ type: 'session.idle', location: { directory: '/a' }, data: { sessionID: 'ses-1' } }) + emitEvent({ type: 'session.idle', location: { directory: '/a' }, data: { sessionID: 'ses-1' } }) expect(clientA.frames).toHaveLength(0) }) @@ -656,7 +640,7 @@ describe('SSEAggregator directory-indexed broadcast', () => { sseAggregator.addClient('index-e', clientA.callback, clientA.writeFrame, []) sseAggregator.addDirectories('index-e', ['/a']) - emitRawEvent({ type: 'session.idle', location: { directory: '/a' }, data: { sessionID: 'ses-1' } }) + emitEvent({ type: 'session.idle', location: { directory: '/a' }, data: { sessionID: 'ses-1' } }) expect(clientA.frames).toHaveLength(1) }) @@ -668,11 +652,11 @@ describe('SSEAggregator directory-indexed broadcast', () => { sseAggregator.addClient('index-f', clientA.callback, clientA.writeFrame, ['/a']) sseAggregator.addClient('index-f', clientB.callback, clientB.writeFrame, ['/b']) - emitRawEvent({ type: 'session.idle', location: { directory: '/a' }, data: { sessionID: 'ses-1' } }) + emitEvent({ type: 'session.idle', location: { directory: '/a' }, data: { sessionID: 'ses-1' } }) expect(clientA.frames).toHaveLength(0) expect(clientB.frames).toHaveLength(0) - emitRawEvent({ type: 'session.idle', location: { directory: '/b' }, data: { sessionID: 'ses-1' } }) + emitEvent({ type: 'session.idle', location: { directory: '/b' }, data: { sessionID: 'ses-1' } }) expect(clientB.frames).toHaveLength(1) }) }) @@ -680,7 +664,7 @@ describe('SSEAggregator directory-indexed broadcast', () => { describe('SSEAggregator directory-less session event routing', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) }) @@ -695,10 +679,10 @@ describe('SSEAggregator directory-less session event routing', () => { seen.push({ directory, type: event.type, sessionID: sessionIDFromEvent(event) ?? '' }) }) - emitRawEvent({ type: 'session.execution.started', location: { directory: '/r' }, data: { sessionID: 'ses_1' } }) + emitEvent({ type: 'session.execution.started', location: { directory: '/r' }, data: { sessionID: 'ses_1' } }) expect(sseAggregator.getActiveSessions()).toEqual({ '/r': ['ses_1'] }) - emitRawEvent({ type: 'session.execution.succeeded', data: { sessionID: 'ses_1' } }) + emitEvent({ type: 'session.execution.succeeded', data: { sessionID: 'ses_1' } }) expect(seen).toEqual([ { directory: '/r', type: 'session.execution.started', sessionID: 'ses_1' }, @@ -731,7 +715,7 @@ describe('SSEAggregator directory-less session event routing', () => { const seen: Array<{ directory: string | null; type: string }> = [] sseAggregator.onEvent((directory, event) => { seen.push({ directory, type: event.type }) }) - emitRawEvent({ type: 'session.execution.succeeded', data: { sessionID: 'ses_unknown' } }) + emitEvent({ type: 'session.execution.succeeded', data: { sessionID: 'ses_unknown' } }) expect(seen).toEqual([{ directory: null, type: 'session.execution.succeeded' }]) expect(sseAggregator.getActiveSessions()).toEqual({}) @@ -752,7 +736,7 @@ describe('SSEAggregator directory-less session event routing', () => { seen.push({ directory, type: event.type, sessionID: sessionIDFromEvent(event) ?? '' }) }) - emitRawEvent({ type: 'session.deleted', data: { sessionID: 'ses_unknown' } }) + emitEvent({ type: 'session.deleted', data: { sessionID: 'ses_unknown' } }) expect(seen).toEqual([{ directory: null, type: 'session.deleted', sessionID: 'ses_unknown' }]) expect(client.frames.map(parseFrame)).toEqual([ @@ -770,7 +754,7 @@ describe('SSEAggregator directory-less session event routing', () => { const seen: Array<{ directory: string | null; type: string }> = [] sseAggregator.onEvent((directory, event) => { seen.push({ directory, type: event.type }) }) - emitRawEvent({ type: 'session.deleted', location: { directory: '/r' }, data: { sessionID: 'ses_located' } }) + emitEvent({ type: 'session.deleted', location: { directory: '/r' }, data: { sessionID: 'ses_located' } }) expect(seen).toEqual([{ directory: '/r', type: 'session.deleted' }]) expect(client.frames.map(parseFrame)).toEqual([ @@ -788,11 +772,11 @@ describe('SSEAggregator directory-less session event routing', () => { const seen: Array = [] sseAggregator.onEvent((directory) => { seen.push(directory) }) - emitRawEvent({ type: 'session.created', location: { directory: '/r' }, data: { sessionID: 'ses_del' } }) - emitRawEvent({ type: 'session.deleted', data: { sessionID: 'ses_del' } }) + emitEvent({ type: 'session.created', location: { directory: '/r' }, data: { sessionID: 'ses_del' } }) + emitEvent({ type: 'session.deleted', data: { sessionID: 'ses_del' } }) seen.length = 0 - emitRawEvent({ type: 'session.execution.succeeded', data: { sessionID: 'ses_del' } }) + emitEvent({ type: 'session.execution.succeeded', data: { sessionID: 'ses_del' } }) expect(seen).toEqual([null]) expect(sseAggregator.getActiveSessions()).toEqual({}) @@ -813,10 +797,10 @@ describe('SSEAggregator directory-less session event routing', () => { seen.push({ directory, type: event.type, sessionID: sessionIDFromEvent(event) ?? '' }) }) - emitRawEvent({ type: 'session.created', location: { directory: '/r' }, data: { sessionID: 'ses_source' } }) + emitEvent({ type: 'session.created', location: { directory: '/r' }, data: { sessionID: 'ses_source' } }) seen.length = 0 - emitRawEvent({ type: 'session.forked', data: { sessionID: 'ses_fork', parentID: 'ses_source', boundary: { messageID: 'msg_1' } } }) + emitEvent({ type: 'session.forked', data: { sessionID: 'ses_fork', parentID: 'ses_source', boundary: { messageID: 'msg_1' } } }) expect(seen).toEqual([{ directory: '/r', type: 'session.forked', sessionID: 'ses_fork' }]) @@ -830,7 +814,7 @@ describe('SSEAggregator directory-less session event routing', () => { expect(global.frames.map(parseFrame).at(-1)).toEqual({ directory: null, payload: forkPayload }) seen.length = 0 - emitRawEvent({ type: 'session.execution.succeeded', data: { sessionID: 'ses_fork' } }) + emitEvent({ type: 'session.execution.succeeded', data: { sessionID: 'ses_fork' } }) expect(seen).toEqual([{ directory: '/r', type: 'session.execution.succeeded', sessionID: 'ses_fork' }]) }) @@ -839,7 +823,7 @@ describe('SSEAggregator directory-less session event routing', () => { describe('SSEAggregator server-aware event feed', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) }) @@ -849,8 +833,8 @@ describe('SSEAggregator server-aware event feed', () => { seen.push({ serverId, directory, type: event.type }) }) - emitRawEvent({ type: 'session.created', location: { directory: '/r' }, data: { sessionID: 'ses_1' } }) - emitRawEvent({ type: 'session.execution.succeeded', data: { sessionID: 'ses_1' } }) + emitEvent({ type: 'session.created', location: { directory: '/r' }, data: { sessionID: 'ses_1' } }) + emitEvent({ type: 'session.execution.succeeded', data: { sessionID: 'ses_1' } }) expect(seen).toEqual([ { serverId: 'local', directory: '/r', type: 'session.created' }, @@ -929,7 +913,7 @@ describe('SSEAggregator server-aware event feed', () => { describe('SSEAggregator location-scoped catalog event broadcast', () => { beforeEach(() => { sseAggregator.shutdown() - sseAggregator.setPendingActionsFetcher(null) + sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) }) @@ -946,7 +930,7 @@ describe('SSEAggregator location-scoped catalog event broadcast', () => { seen.push({ directory, type: event.type }) }) - emitRawEvent({ id: 'evt_catalog', type, location: { directory: '/repo-c' }, data: {} }) + emitEvent({ id: 'evt_catalog', type, location: { directory: '/repo-c' }, data: {} }) const expected = { directory: null, @@ -956,7 +940,7 @@ describe('SSEAggregator location-scoped catalog event broadcast', () => { expect(unsubscribed.frames.map(parseFrame)).toEqual([expected]) expect(seen).toEqual([{ directory: '/repo-c', type }]) - emitRawEvent({ + emitEvent({ id: 'evt_status', type: 'session.status', location: { directory: '/repo-c' }, @@ -968,3 +952,173 @@ describe('SSEAggregator location-scoped catalog event broadcast', () => { }, ) }) + +interface ControlledEventStream { + iterable: AsyncIterable + signal: AbortSignal + push: (event: V2Event) => void + end: () => void +} + +function createControlledEventStream(): ControlledEventStream { + const queue: V2Event[] = [] + let finished = false + let signal: AbortSignal + let notify: (() => void) | null = null + + const wake = (): void => { + const current = notify + notify = null + current?.() + } + + const iterable: AsyncIterable = { + [Symbol.asyncIterator]: () => ({ + async next() { + for (;;) { + if (queue.length > 0) return { value: queue.shift()!, done: false } + if (finished) return { value: undefined, done: true } + await new Promise((resolve) => { + notify = resolve + }) + } + }, + }), + } + + return { + iterable, + get signal() { + return signal + }, + set signal(next: AbortSignal) { + signal = next + next.addEventListener('abort', wake, { once: true }) + }, + push: (event) => { + queue.push(event) + wake() + }, + end: () => { + finished = true + wake() + }, + } as ControlledEventStream +} + +function makeStreamingApi(active: Record = {}) { + const subscriptions: ControlledEventStream[] = [] + const api = { + ...makeApi({ active }), + event: { + subscribe: (options: { signal?: AbortSignal }) => { + const stream = createControlledEventStream() + stream.signal = options.signal! + subscriptions.push(stream) + return stream.iterable + }, + }, + } as unknown as OpenCodeApi + return { api, subscriptions } +} + +describe('SSEAggregator upstream transport', () => { + beforeEach(() => { + sseAggregator.shutdown() + sseAggregator.setOpenCodeClient(null) + sseAggregator.setScheduledSessionsResolver(() => []) + }) + + it('streams through the OpenCode client host override and resolved password', async () => { + let streamController: ReadableStreamDefaultController | null = null + const fetchFn = vi.fn(async () => new Response( + new ReadableStream({ + start(controller) { + streamController = controller + }, + }), + { headers: { 'content-type': 'text/event-stream' } }, + )) + const client = new FetchOpenCodeClient({ + baseUrl: getOpenCodeUpstreamBaseUrl('10.20.30.40'), + basicAuth: null, + passwordResolver: () => 'resolved-secret', + fetchFn: fetchFn as unknown as typeof fetch, + }) + + const captured = createCapturingClient() + sseAggregator.addClient('transport-a', captured.callback, captured.writeFrame, ['/repo']) + sseAggregator.setOpenCodeClient(client) + sseAggregator.start() + await flushReplay() + + expect(fetchFn).toHaveBeenCalledTimes(1) + const [url, init] = fetchFn.mock.calls[0] as unknown as [URL, RequestInit] + expect(String(url)).toBe('http://10.20.30.40:5551/api/event') + expect(new Headers(init.headers).get('Authorization')).toBe(buildOpenCodeBasicAuth('resolved-secret')) + + const encoder = new TextEncoder() + streamController!.enqueue(encoder.encode(`data: ${JSON.stringify(eventOf({ type: 'server.connected', data: {} }))}\n\n`)) + const busy = eventOf({ + type: 'session.status', + location: { directory: '/repo' }, + data: { sessionID: 'ses-1', status: { type: 'busy' } }, + }) + streamController!.enqueue(encoder.encode(`data: ${JSON.stringify(busy)}\n\n`)) + await flushReplay() + + expect(captured.events.filter(event => event.event === 'resync')).toHaveLength(1) + expect(captured.frames.map(parseFrame).filter(frame => frame.payload.type === 'session.status')).toEqual([ + { directory: '/repo', payload: busy }, + ]) + + streamController!.close() + sseAggregator.shutdown() + }) +}) + +describe('SSEAggregator upstream reconnection', () => { + beforeEach(() => { + sseAggregator.shutdown() + sseAggregator.setOpenCodeClient(null) + sseAggregator.setScheduledSessionsResolver(() => []) + }) + + it('reconnects after the stream ends, broadcasting resync and replaying tracked statuses', async () => { + vi.useFakeTimers() + try { + const { api, subscriptions } = makeStreamingApi({ 'ses-1': { type: 'running' } }) + const client = createCapturingClient() + sseAggregator.addClient('reconnect-a', client.callback, client.writeFrame, ['/repo/a']) + sseAggregator.setOpenCodeClient({ api }) + sseAggregator.start() + + expect(subscriptions).toHaveLength(1) + subscriptions[0]!.push(eventOf({ type: 'server.connected', data: {} })) + subscriptions[0]!.push(eventOf({ + type: 'session.status', + location: { directory: '/repo/a' }, + data: { sessionID: 'ses-1', status: { type: 'busy' } }, + })) + await flushReplay() + + expect(client.events.filter(event => event.event === 'resync')).toHaveLength(1) + expect(sseAggregator.getActiveSessions()).toEqual({ '/repo/a': ['ses-1'] }) + + subscriptions[0]!.end() + await vi.advanceTimersByTimeAsync(0) + await vi.advanceTimersByTimeAsync(1000) + + expect(subscriptions).toHaveLength(2) + subscriptions[1]!.push(eventOf({ type: 'server.connected', data: {} })) + await flushReplay() + + expect(client.events.filter(event => event.event === 'resync')).toHaveLength(2) + expect(api.session.active).toHaveBeenCalledTimes(1) + const statuses = client.frames.map(parseFrame).filter(frame => frame.payload.type === 'session.status') + expect(statuses.at(-1)?.payload.data).toEqual({ sessionID: 'ses-1', status: { type: 'busy' } }) + } finally { + vi.useRealTimers() + } + }) +}) From 680a336a24b5f8b106d06ab6c42ef0fef036baac Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:08:00 -0500 Subject: [PATCH 03/20] section 3: Phase 3: Remote server events use the registry client; remov --- backend/package.json | 2 - .../src/services/opencode-event-routing.ts | 34 -- backend/src/services/remote-server-events.ts | 150 ++---- backend/test/routes/repo-git.test.ts | 4 - backend/test/routes/ssh.test.ts | 4 - backend/test/services/git-auth.test.ts | 4 - .../services/remote-server-events.test.ts | 475 ++++++++++-------- pnpm-lock.yaml | 30 +- 8 files changed, 327 insertions(+), 376 deletions(-) diff --git a/backend/package.json b/backend/package.json index 79fb4bd82..900ba0324 100644 --- a/backend/package.json +++ b/backend/package.json @@ -24,7 +24,6 @@ "archiver": "^7.0.1", "better-auth": "^1.7.12", "croner": "^10.0.1", - "eventsource": "^4.1.0", "hono": "^4.13.13", "jsonc-parser": "^3.3.1", "web-push": "^3.6.7", @@ -35,7 +34,6 @@ "@types/archiver": "^7.0.0", "@types/better-sqlite3": "^7.6.13", "@types/bun": "latest", - "@types/eventsource": "^3.0.0", "@types/web-push": "^3.6.4", "@vitest/coverage-v8": "^3.2.4", "@vitest/ui": "^3.2.4", diff --git a/backend/src/services/opencode-event-routing.ts b/backend/src/services/opencode-event-routing.ts index f533a330d..65123f11c 100644 --- a/backend/src/services/opencode-event-routing.ts +++ b/backend/src/services/opencode-event-routing.ts @@ -1,4 +1,3 @@ -import { EventSource } from 'eventsource' import { openCodeLocation, sessionIDFromEvent, @@ -8,8 +7,6 @@ import { } from '@opencode-manager/shared/opencode' import { logger } from '../utils/logger' -const MULTILINE_PATTERN = /[\r\n]/ - type PermissionAskedEvent = Extract type FormCreatedEvent = Extract type SessionStatusEvent = Extract @@ -50,8 +47,6 @@ export interface SessionStatusReplayPlan { busy: string[] } -export type EventSourceInit = ConstructorParameters[1] - /** * Partitions tracked sessions into those that must be replayed as idle and the * active sessions that must be replayed as busy. @@ -146,21 +141,6 @@ export function routeOpenCodeEvent( return { ...resolveEventRouting(event, sessionDirectories), payloadJson: JSON.stringify(event) } } -export function parseOpenCodeEvent(data: string, sessionDirectories: Map): RoutedOpenCodeEvent | null { - let event: V2Event - try { - event = JSON.parse(data) as V2Event - } catch { - return null - } - - if (!event?.type) return null - - const routed = resolveEventRouting(event, sessionDirectories) - const payloadJson = MULTILINE_PATTERN.test(data) ? JSON.stringify(event) : data - return { ...routed, payloadJson } -} - export async function collectPendingReplayEvents(request: PendingReplayRequest): Promise { const { api, directory, buildEvent, logScope, logTarget } = request const [permissionsResult, formsResult] = await Promise.allSettled([ @@ -189,20 +169,6 @@ export async function collectPendingReplayEvents(request: PendingReplayRequest): return { permissions, forms } } -export function createAuthorizedEventSourceInit(authorization: string | null): EventSourceInit { - if (!authorization) return undefined - return { - fetch: (input, init) => - fetch(input, { - ...init, - headers: { - ...(init?.headers ?? {}), - Authorization: authorization, - }, - }), - } -} - export function nextBackoffDelay(currentDelayMs: number, maxDelayMs: number): number { return Math.min(currentDelayMs * 2, maxDelayMs) } diff --git a/backend/src/services/remote-server-events.ts b/backend/src/services/remote-server-events.ts index c81fbcf7c..20b81eef9 100644 --- a/backend/src/services/remote-server-events.ts +++ b/backend/src/services/remote-server-events.ts @@ -1,4 +1,3 @@ -import { EventSource } from 'eventsource' import { DEFAULTS } from '@opencode-manager/shared/config' import { buildServerDirectoryKey, @@ -10,19 +9,17 @@ import { sessionIDFromEvent, type OpenCodeApi, type V2Event } from '@opencode-ma import { logger } from '../utils/logger' import type { OpenCodeServerChange, - OpenCodeServerConnection, OpenCodeServerRegistry, } from './opencode-server-registry' import { applySessionActivity, collectPendingReplayEvents, - createAuthorizedEventSourceInit, createReplayEventBuilder, - nextBackoffDelay, - parseOpenCodeEvent, planSessionStatusReplay, + routeOpenCodeEvent, type ReplayEventBuilder, } from './opencode-event-routing' +import { subscribeOpenCodeEvents, type OpenCodeEventSubscription } from './opencode-event-subscription' import type { RemoteReplayHandler } from './sse-aggregator' const { RECONNECT_DELAY_MS, MAX_RECONNECT_DELAY_MS } = DEFAULTS.SSE @@ -38,26 +35,13 @@ export interface RemoteEventAggregator { setRemoteSubagentResolver(resolver: ((serverId: string, sessionId: string) => boolean) | null): void } -export interface RemoteEventSource { - onopen: ((event: unknown) => void) | null - onerror: ((event: unknown) => void) | null - onmessage: ((event: { data: string }) => void) | null - close(): void -} - -export type RemoteEventSourceInit = ConstructorParameters[1] -export type RemoteEventSourceFactory = (url: string, init?: RemoteEventSourceInit) => RemoteEventSource - export interface RemoteServerEventHubOptions { - eventSourceFactory?: RemoteEventSourceFactory reconnectDelayMs?: number maxReconnectDelayMs?: number } interface RemoteServerStream { - source: RemoteEventSource | null - reconnectTimeout: ReturnType | null - reconnectDelay: number + subscription: OpenCodeEventSubscription | null connected: boolean error: string | null version: string | null @@ -68,10 +52,6 @@ interface RemoteServerStream { subagentSessions: Set } -function createDefaultEventSource(url: string, init?: RemoteEventSourceInit): RemoteEventSource { - return new EventSource(url, init) as unknown as RemoteEventSource -} - function describeStreamError(error: unknown): string { if (typeof error === 'string' && error) return error if (error instanceof Error && error.message) return error.message @@ -85,7 +65,6 @@ function describeStreamError(error: unknown): string { export class RemoteServerEventHub { private readonly streams = new Map() - private readonly eventSourceFactory: RemoteEventSourceFactory private readonly reconnectDelayMs: number private readonly maxReconnectDelayMs: number private unsubscribeRegistry: (() => void) | null = null @@ -98,7 +77,6 @@ export class RemoteServerEventHub { private readonly aggregator: RemoteEventAggregator, options: RemoteServerEventHubOptions = {}, ) { - this.eventSourceFactory = options.eventSourceFactory ?? createDefaultEventSource this.reconnectDelayMs = options.reconnectDelayMs ?? RECONNECT_DELAY_MS this.maxReconnectDelayMs = options.maxReconnectDelayMs ?? MAX_RECONNECT_DELAY_MS } @@ -214,97 +192,68 @@ export class RemoteServerEventHub { if (!this.started) return this.closeStream(serverId) - let connection: OpenCodeServerConnection try { - connection = this.registry.getConnection(serverId) + this.registry.getConnection(serverId) } catch (error) { this.recordError(serverId, describeStreamError(error)) logger.warn(`Remote event stream unavailable for ${serverId}: ${describeStreamError(error)}`) return } - const authorization = connection.authorization - const init: RemoteEventSourceInit = createAuthorizedEventSourceInit(authorization) - const stream = this.ensureStream(serverId) - let source: RemoteEventSource - try { - source = this.eventSourceFactory(`${connection.baseUrl}/api/event`, init) - } catch (error) { - this.recordError(serverId, describeStreamError(error)) - this.scheduleReconnect(serverId) - return - } - - stream.source = source stream.error = null - source.onopen = () => this.handleOpen(serverId, source) - source.onerror = (event) => this.handleStreamError(serverId, source, event) - source.onmessage = (event) => this.handleMessage(serverId, event.data) + + const subscription = subscribeOpenCodeEvents({ + api: () => this.registry.getClient(serverId).api, + label: serverId, + onOpen: () => this.handleOpen(serverId, subscription), + onEvent: (event) => this.handleMessage(serverId, event), + onDisconnect: (error) => this.handleStreamError(serverId, error), + reconnectDelayMs: this.reconnectDelayMs, + maxReconnectDelayMs: this.maxReconnectDelayMs, + }) + + stream.subscription = subscription logger.info(`Remote event stream connecting: ${serverId}`) } - private handleOpen(serverId: string, source: RemoteEventSource): void { + private handleOpen(serverId: string, subscription: OpenCodeEventSubscription): void { const stream = this.streams.get(serverId) - if (!stream || stream.source !== source) return + if (!stream || stream.subscription !== subscription) return stream.connected = true stream.error = null - stream.reconnectDelay = this.reconnectDelayMs logger.info(`Remote event stream connected: ${serverId}`) this.aggregator.notifyServerConnected(serverId) this.aggregator.sendRemoteResync(serverId) void this.refreshVersion(serverId) - void this.replaySessionStatuses(serverId, source) + void this.replaySessionStatuses(serverId, subscription) } - private handleStreamError(serverId: string, source: RemoteEventSource, error: unknown): void { + private handleStreamError(serverId: string, error: unknown): void { const stream = this.streams.get(serverId) - if (!stream || stream.source !== source) return + if (!stream) return stream.connected = false stream.error = describeStreamError(error) logger.warn(`Remote event stream error for ${serverId}: ${stream.error}`) - this.closeSource(stream) - this.scheduleReconnect(serverId) } - private scheduleReconnect(serverId: string): void { - const stream = this.streams.get(serverId) - if (!this.started || !stream || stream.reconnectTimeout) return - - const delay = stream.reconnectDelay - stream.reconnectTimeout = setTimeout(() => { - const current = this.streams.get(serverId) - if (!current) return - current.reconnectTimeout = null - current.reconnectDelay = nextBackoffDelay(current.reconnectDelay, this.maxReconnectDelayMs) - this.connectServer(serverId) - }, delay) - } - - private handleMessage(serverId: string, data: string): void { + private handleMessage(serverId: string, event: V2Event): void { const stream = this.streams.get(serverId) if (!stream) return - const routed = parseOpenCodeEvent(data, stream.sessionDirectories) - if (!routed) return - - try { - const { event, resolvedDirectory, payloadJson } = routed - const sessionID = sessionIDFromEvent(event) - if (sessionID) applySessionActivity(stream.busySessions, event) - if (sessionID && event.type === 'session.deleted') { - stream.subagentSessions.delete(sessionID) - } else if (event.type === 'session.created' && event.data.parentID) { - stream.subagentSessions.add(event.data.sessionID) - } - - this.aggregator.notifyServerEvent(serverId, resolvedDirectory ?? null, event) - this.aggregator.deliverRemoteEnvelope(serverId, resolvedDirectory ?? null, payloadJson) - } catch (error) { - logger.error(`Remote event stream failed to handle ${routed.event.type} for ${serverId}:`, error) + const { resolvedDirectory, payloadJson } = routeOpenCodeEvent(event, stream.sessionDirectories) + const sessionID = sessionIDFromEvent(event) + if (sessionID) applySessionActivity(stream.busySessions, event) + if (sessionID && event.type === 'session.deleted') { + stream.subagentSessions.delete(sessionID) + } else if (event.type === 'session.created' && event.data.parentID) { + stream.subagentSessions.add(event.data.sessionID) } + + this.aggregator.notifyServerEvent(serverId, resolvedDirectory ?? null, event) + this.aggregator.deliverRemoteEnvelope(serverId, resolvedDirectory ?? null, payloadJson) } private handleRegistryChange(change: OpenCodeServerChange): void { @@ -331,7 +280,6 @@ export class RemoteServerEventHub { const stream = this.streams.get(change.serverId) if (stream) { - stream.reconnectDelay = this.reconnectDelayMs stream.version = null stream.versionRequested = false stream.versionProbeToken = 0 @@ -339,13 +287,17 @@ export class RemoteServerEventHub { this.connectServer(change.serverId) } - private isStreamCurrent(serverId: string, stream: RemoteServerStream, source: RemoteEventSource): boolean { - return this.streams.get(serverId) === stream && stream.source === source && stream.connected + private isStreamCurrent( + serverId: string, + stream: RemoteServerStream, + subscription: OpenCodeEventSubscription, + ): boolean { + return this.streams.get(serverId) === stream && stream.subscription === subscription && stream.connected } - private async replaySessionStatuses(serverId: string, source: RemoteEventSource): Promise { + private async replaySessionStatuses(serverId: string, subscription: OpenCodeEventSubscription): Promise { const stream = this.streams.get(serverId) - if (!stream || !this.isStreamCurrent(serverId, stream, source)) return + if (!stream || !this.isStreamCurrent(serverId, stream, subscription)) return let api: OpenCodeApi try { @@ -363,10 +315,10 @@ export class RemoteServerEventHub { return } - if (!this.isStreamCurrent(serverId, stream, source)) return + if (!this.isStreamCurrent(serverId, stream, subscription)) return await this.resolveActiveSessionDirectories(api, serverId, stream, Object.keys(active ?? {})) - if (!this.isStreamCurrent(serverId, stream, source)) return + if (!this.isStreamCurrent(serverId, stream, subscription)) return const plan = planSessionStatusReplay(active, stream.sessionDirectories) let replayed = 0 @@ -442,9 +394,7 @@ export class RemoteServerEventHub { if (existing) return existing const stream: RemoteServerStream = { - source: null, - reconnectTimeout: null, - reconnectDelay: this.reconnectDelayMs, + subscription: null, connected: false, error: null, version: null, @@ -464,20 +414,16 @@ export class RemoteServerEventHub { stream.error = error } - private closeSource(stream: RemoteServerStream): void { - if (!stream.source) return - stream.source.close() - stream.source = null + private closeSubscription(stream: RemoteServerStream): void { + if (!stream.subscription) return + stream.subscription.close() + stream.subscription = null } private closeStream(serverId: string): void { const stream = this.streams.get(serverId) if (!stream) return - if (stream.reconnectTimeout) { - clearTimeout(stream.reconnectTimeout) - stream.reconnectTimeout = null - } - this.closeSource(stream) + this.closeSubscription(stream) stream.connected = false } diff --git a/backend/test/routes/repo-git.test.ts b/backend/test/routes/repo-git.test.ts index 781e180a7..94ad49414 100644 --- a/backend/test/routes/repo-git.test.ts +++ b/backend/test/routes/repo-git.test.ts @@ -52,10 +52,6 @@ vi.mock('@opencode-manager/shared/config', () => ({ }, })) -vi.mock('eventsource', () => ({ - EventSource: vi.fn(), -})) - const getRepoByIdMock = db.getRepoById as MockedFunction describe('Repo Git Routes', () => { diff --git a/backend/test/routes/ssh.test.ts b/backend/test/routes/ssh.test.ts index 717e84487..95c938236 100644 --- a/backend/test/routes/ssh.test.ts +++ b/backend/test/routes/ssh.test.ts @@ -32,10 +32,6 @@ vi.mock('@opencode-manager/shared/config', () => ({ }, })) -vi.mock('eventsource', () => ({ - EventSource: vi.fn(), -})) - vi.mock('../../src/utils/logger', () => ({ logger: { info: vi.fn(), diff --git a/backend/test/services/git-auth.test.ts b/backend/test/services/git-auth.test.ts index f182c931b..012821b76 100644 --- a/backend/test/services/git-auth.test.ts +++ b/backend/test/services/git-auth.test.ts @@ -41,10 +41,6 @@ vi.mock('@opencode-manager/shared/config', () => ({ }, })) -vi.mock('eventsource', () => ({ - EventSource: vi.fn(), -})) - vi.mock('../../src/utils/ssh-key-manager', () => ({ writeTemporarySSHKey: vi.fn().mockResolvedValue('/tmp/test-workspace/.ssh-keys/test-key'), cleanupSSHKey: vi.fn().mockResolvedValue(undefined), diff --git a/backend/test/services/remote-server-events.test.ts b/backend/test/services/remote-server-events.test.ts index 5b70b3695..38f64ca73 100644 --- a/backend/test/services/remote-server-events.test.ts +++ b/backend/test/services/remote-server-events.test.ts @@ -8,15 +8,10 @@ vi.mock('../../src/utils/logger', () => ({ }, })) -import type { OpenCodeApi } from '@opencode-manager/shared/opencode' +import type { OpenCodeApi, V2Event } from '@opencode-manager/shared/opencode' import type { OpenCodeServer, OpenCodeServerTestResult } from '@opencode-manager/shared/schemas' import { sseAggregator } from '../../src/services/sse-aggregator' -import { - RemoteServerEventHub, - type RemoteEventSource, - type RemoteEventSourceFactory, - type RemoteEventSourceInit, -} from '../../src/services/remote-server-events' +import { RemoteServerEventHub } from '../../src/services/remote-server-events' import type { OpenCodeServerChange, OpenCodeServerConnection, @@ -45,32 +40,73 @@ const SERVER_B: OpenCodeServer = { updatedAt: 1, } -class FakeEventSource implements RemoteEventSource { - onopen: ((event: unknown) => void) | null = null - onerror: ((event: unknown) => void) | null = null - onmessage: ((event: { data: string }) => void) | null = null - closed = false +const SERVER_CONNECTED: V2Event = { id: 'evt_connected', type: 'server.connected', data: {} } - constructor( - readonly url: string, - readonly init?: RemoteEventSourceInit, - ) {} - - close(): void { - this.closed = true - } +interface ControlledEventStream { + iterable: AsyncIterable + signal: AbortSignal + push: (event: V2Event) => void + end: () => void + fail: (error: unknown) => void +} - open(): void { - this.onopen?.({}) +function createControlledEventStream(): ControlledEventStream { + const queue: V2Event[] = [] + let finished = false + let failure: { value: unknown } | null = null + let signal: AbortSignal + let notify: (() => void) | null = null + + const wake = (): void => { + const current = notify + notify = null + current?.() } - fail(event: unknown = { message: 'connection lost' }): void { - this.onerror?.(event) + const iterable: AsyncIterable = { + [Symbol.asyncIterator]: () => ({ + async next() { + for (;;) { + if (queue.length > 0) return { value: queue.shift()!, done: false } + if (failure) { + const current = failure.value + failure = null + throw current + } + if (finished) return { value: undefined, done: true } + await new Promise((resolve) => { + notify = resolve + }) + } + }, + }), } - message(data: string): void { - this.onmessage?.({ data }) - } + return { + iterable, + get signal() { + return signal + }, + set signal(next: AbortSignal) { + signal = next + next.addEventListener('abort', () => { + failure = { value: next.reason ?? new Error('aborted') } + wake() + }, { once: true }) + }, + push: (event) => { + queue.push(event) + wake() + }, + end: () => { + finished = true + wake() + }, + fail: (error) => { + failure = { value: error } + wake() + }, + } as ControlledEventStream } interface CapturedClient { @@ -107,10 +143,18 @@ interface ApiFixture { sessionDirectories?: Record } -function makeApi(fixture: ApiFixture = {}): OpenCodeApi { +function makeApi(fixture: ApiFixture, subscriptions: ControlledEventStream[]): OpenCodeApi { const directoryOf = (input?: { location?: { directory?: string } }): string => input?.location?.directory ?? '' return { + event: { + subscribe: (options: { signal?: AbortSignal; onActivity?: () => void }) => { + const stream = createControlledEventStream() + stream.signal = options.signal! + subscriptions.push(stream) + return stream.iterable + }, + }, permission: { request: { list: vi.fn(async (input?: { location?: { directory?: string } }) => ({ @@ -134,21 +178,40 @@ function makeApi(fixture: ApiFixture = {}): OpenCodeApi { } as unknown as OpenCodeApi } +interface FakeServerTransport { + api: OpenCodeApi + subscriptions: ControlledEventStream[] +} + interface FakeRegistry { registry: OpenCodeServerRegistry api: OpenCodeApi + transport: (serverId: string) => FakeServerTransport + latest: (serverId: string) => ControlledEventStream emit: (change: OpenCodeServerChange) => void setServer: (server: OpenCodeServer) => void removeServer: (serverId: string) => void listCalls: () => number + clientCalls: string[] } function createRegistry(servers: OpenCodeServer[], fixture: ApiFixture = {}): FakeRegistry { const byId = new Map(servers.map(server => [server.id, server])) const listeners = new Set<(change: OpenCodeServerChange) => void>() - const api = makeApi(fixture) + const transports = new Map() + const clientCalls: string[] = [] let listCalls = 0 + const transport = (serverId: string): FakeServerTransport => { + const existing = transports.get(serverId) + if (existing) return existing + + const subscriptions: ControlledEventStream[] = [] + const created: FakeServerTransport = { api: makeApi(fixture, subscriptions), subscriptions } + transports.set(serverId, created) + return created + } + const registry = { list: () => { listCalls += 1 @@ -164,7 +227,10 @@ function createRegistry(servers: OpenCodeServer[], fixture: ApiFixture = {}): Fa authorization: server.hasPassword ? 'Basic dXNlcjpwYXNz' : null, } }, - getClient: () => ({ api }), + getClient: (serverId: string) => { + clientCalls.push(serverId) + return { api: transport(serverId).api } + }, onChange: (listener: (change: OpenCodeServerChange) => void) => { listeners.add(listener) return () => { listeners.delete(listener) } @@ -174,25 +240,19 @@ function createRegistry(servers: OpenCodeServer[], fixture: ApiFixture = {}): Fa return { registry, - api, + api: transport(servers[0]!.id).api, + transport, + latest: (serverId) => { + const subscriptions = transport(serverId).subscriptions + const latest = subscriptions[subscriptions.length - 1] + if (!latest) throw new Error(`No subscription for ${serverId}`) + return latest + }, emit: (change) => { listeners.forEach(listener => listener(change)) }, setServer: (server) => { byId.set(server.id, server) }, removeServer: (serverId) => { byId.delete(serverId) }, listCalls: () => listCalls, - } -} - -function createSourceFactory() { - const sources: FakeEventSource[] = [] - const factory: RemoteEventSourceFactory = (url, init) => { - const source = new FakeEventSource(url, init) - sources.push(source) - return source - } - return { - sources, - factory, - latest: () => sources[sources.length - 1]!, + clientCalls, } } @@ -202,28 +262,26 @@ async function flushPromises(): Promise { } } -function remoteEvent(type: string, data: unknown, directory?: string): string { - return JSON.stringify({ +function remoteEvent(type: string, data: unknown, directory?: string): V2Event { + return { id: 'evt_1', created: 1, type, ...(directory ? { location: { directory } } : {}), data, - }) + } as V2Event } describe('RemoteServerEventHub event delivery', () => { let hub: RemoteServerEventHub let fake: FakeRegistry - let sources: ReturnType beforeEach(() => { sseAggregator.shutdown() sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) fake = createRegistry([SERVER_A, SERVER_B]) - sources = createSourceFactory() - hub = new RemoteServerEventHub(fake.registry, sseAggregator, { eventSourceFactory: sources.factory }) + hub = new RemoteServerEventHub(fake.registry, sseAggregator) }) afterEach(() => { @@ -231,18 +289,15 @@ describe('RemoteServerEventHub event delivery', () => { sseAggregator.shutdown() }) - it('opens one stream per enabled server with the connection authorization', () => { + it('opens one subscription per enabled server through the registry client', () => { hub.start() - expect(sources.sources.map(source => source.url)).toEqual([ - 'https://studio.example.com/api/event', - 'https://laptop.example.com/api/event', - ]) - expect(sources.sources[0]?.init?.fetch).toBeTypeOf('function') - expect(sources.sources[1]?.init).toBeUndefined() + expect(fake.clientCalls).toEqual(['srv_a', 'srv_b']) + expect(fake.transport('srv_a').subscriptions).toHaveLength(1) + expect(fake.transport('srv_b').subscriptions).toHaveLength(1) }) - it('broadcasts a located event to every client with a serverId envelope', () => { + it('broadcasts a located event to every client with a serverId envelope', async () => { const remoteClient = createCapturingClient() const otherServerClient = createCapturingClient() const localClient = createCapturingClient() @@ -254,11 +309,15 @@ describe('RemoteServerEventHub event delivery', () => { sseAggregator.onEvent(listener) hub.start() - const source = sources.sources[0]! - source.open() + const stream = fake.latest('srv_a') + stream.push(SERVER_CONNECTED) + await flushPromises() remoteClient.frames.length = 0 + otherServerClient.frames.length = 0 + localClient.frames.length = 0 - source.message(remoteEvent('session.idle', { sessionID: 'ses_1' }, '/repo')) + stream.push(remoteEvent('session.idle', { sessionID: 'ses_1' }, '/repo')) + await flushPromises() const envelope = { directory: '/repo', @@ -278,16 +337,18 @@ describe('RemoteServerEventHub event delivery', () => { expect(sseAggregator.getActiveSessions()).toEqual({}) }) - it('resolves a directory-less event through the per-server session map', () => { + it('resolves a directory-less event through the per-server session map', async () => { const client = createCapturingClient() sseAggregator.addClient('session-map', client.callback, client.writeFrame, ['srv_a|/repo']) hub.start() - const source = sources.sources[0]! - source.message(remoteEvent('session.created', { sessionID: 'ses_1', parentID: null }, '/repo')) + const stream = fake.latest('srv_a') + stream.push(remoteEvent('session.created', { sessionID: 'ses_1', parentID: null }, '/repo')) + await flushPromises() client.frames.length = 0 - source.message(remoteEvent('session.execution.succeeded', { sessionID: 'ses_1' })) + stream.push(remoteEvent('session.execution.succeeded', { sessionID: 'ses_1' })) + await flushPromises() expect(client.frames.map(parseFrame)).toEqual([{ directory: '/repo', @@ -301,22 +362,26 @@ describe('RemoteServerEventHub event delivery', () => { }]) }) - it('inherits the parent directory for a forked session and forgets deleted sessions', () => { + it('inherits the parent directory for a forked session and forgets deleted sessions', async () => { const client = createCapturingClient() sseAggregator.addClient('fork', client.callback, client.writeFrame, ['srv_a|/repo']) hub.start() - const source = sources.sources[0]! - source.message(remoteEvent('session.created', { sessionID: 'ses_parent' }, '/repo')) - source.message(remoteEvent('session.forked', { sessionID: 'ses_fork', parentID: 'ses_parent' })) + const stream = fake.latest('srv_a') + stream.push(remoteEvent('session.created', { sessionID: 'ses_parent' }, '/repo')) + stream.push(remoteEvent('session.forked', { sessionID: 'ses_fork', parentID: 'ses_parent' })) + await flushPromises() client.frames.length = 0 - source.message(remoteEvent('session.execution.succeeded', { sessionID: 'ses_fork' })) + stream.push(remoteEvent('session.execution.succeeded', { sessionID: 'ses_fork' })) + await flushPromises() expect(client.frames.map(parseFrame).map(frame => frame.directory)).toEqual(['/repo']) - source.message(remoteEvent('session.deleted', { sessionID: 'ses_fork' })) + stream.push(remoteEvent('session.deleted', { sessionID: 'ses_fork' })) + await flushPromises() client.frames.length = 0 - source.message(remoteEvent('session.execution.succeeded', { sessionID: 'ses_fork' })) + stream.push(remoteEvent('session.execution.succeeded', { sessionID: 'ses_fork' })) + await flushPromises() expect(client.frames.map(parseFrame)).toEqual([{ directory: null, @@ -330,7 +395,7 @@ describe('RemoteServerEventHub event delivery', () => { }]) }) - it('broadcasts to every client when the directory is unknown', () => { + it('broadcasts to every client when the directory is unknown', async () => { const subscribedA = createCapturingClient() const otherDirectoryA = createCapturingClient() const subscribedB = createCapturingClient() @@ -339,7 +404,8 @@ describe('RemoteServerEventHub event delivery', () => { sseAggregator.addClient('any-b', subscribedB.callback, subscribedB.writeFrame, ['srv_b|/repo']) hub.start() - sources.sources[0]!.message(remoteEvent('server.connected', {})) + fake.latest('srv_a').push(remoteEvent('server.connected', {})) + await flushPromises() const envelope = { directory: null, @@ -351,46 +417,31 @@ describe('RemoteServerEventHub event delivery', () => { expect(subscribedB.frames.map(parseFrame)).toEqual([envelope]) }) - it('keeps the raw upstream payload in the remote envelope and re-serializes multi-line payloads', () => { + it('serializes the routed event into a single envelope data line', async () => { const client = createCapturingClient() sseAggregator.addClient('raw-remote', client.callback, client.writeFrame, ['srv_a|/repo']) hub.start() - const source = sources.sources[0]! - const raw = '{"type":"session.idle","id":"evt_raw","created":7,"location":{"directory":"/repo"},"data":{"sessionID":"ses_1","extra":{"b":2,"a":1}}}' - source.message(raw) - - expect(client.frames).toEqual([`event: message\ndata: {"directory":"/repo","serverId":"srv_a","payload":${raw}}\n\n`]) - - const multiLine = JSON.stringify(JSON.parse(raw), null, 2) - source.message(multiLine) - - expect(client.frames[1]!.split('\n').filter(line => line.startsWith('data: '))).toHaveLength(1) - expect(parseFrame(client.frames[1]!).payload).toEqual(JSON.parse(raw)) - }) - - it('ignores malformed remote payloads', () => { - const client = createCapturingClient() - sseAggregator.addClient('malformed-remote', client.callback, client.writeFrame, ['srv_a|/repo']) - - hub.start() - const source = sources.sources[0]! - source.message('not-json') - source.message(JSON.stringify({ id: 'evt', created: 1, data: {} })) + const event = remoteEvent('session.idle', { sessionID: 'ses_1', extra: { b: 2, a: 1 } }, '/repo') + fake.latest('srv_a').push(event) + await flushPromises() - expect(client.frames).toHaveLength(0) + expect(client.frames).toHaveLength(1) + expect(client.frames[0]!.split('\n').filter(line => line.startsWith('data: '))).toHaveLength(1) + expect(parseFrame(client.frames[0]!).payload).toEqual(event) }) - it('notifies server event listeners with the remote server id and resolved directory', () => { + it('notifies server event listeners with the remote server id and resolved directory', async () => { const seen: Array<{ serverId: string; directory: string | null; type: string }> = [] sseAggregator.onServerEvent((serverId, directory, event) => { seen.push({ serverId, directory, type: event.type }) }) hub.start() - const source = sources.sources[0]! - source.message(remoteEvent('session.created', { sessionID: 'ses_1' }, '/repo')) - source.message(remoteEvent('session.execution.succeeded', { sessionID: 'ses_1' })) + const stream = fake.latest('srv_a') + stream.push(remoteEvent('session.created', { sessionID: 'ses_1' }, '/repo')) + stream.push(remoteEvent('session.execution.succeeded', { sessionID: 'ses_1' })) + await flushPromises() expect(seen).toEqual([ { serverId: 'srv_a', directory: '/repo', type: 'session.created' }, @@ -399,29 +450,32 @@ describe('RemoteServerEventHub event delivery', () => { expect(sseAggregator.getActiveSessions()).toEqual({}) }) - it('tracks remote subagent sessions per server', () => { + it('tracks remote subagent sessions per server', async () => { hub.start() - const source = sources.sources[0]! + const stream = fake.latest('srv_a') - source.message(remoteEvent('session.created', { sessionID: 'ses_sub', parentID: 'ses_root' }, '/repo')) + stream.push(remoteEvent('session.created', { sessionID: 'ses_sub', parentID: 'ses_root' }, '/repo')) + await flushPromises() expect(hub.isSubagentSession('srv_a', 'ses_sub')).toBe(true) expect(sseAggregator.isSubagentSession('ses_sub', 'srv_a')).toBe(true) expect(sseAggregator.isSubagentSession('ses_sub')).toBe(false) expect(sseAggregator.isSubagentSession('ses_sub', 'srv_b')).toBe(false) - source.message(remoteEvent('session.deleted', { sessionID: 'ses_sub' })) + stream.push(remoteEvent('session.deleted', { sessionID: 'ses_sub' })) + await flushPromises() expect(hub.isSubagentSession('srv_a', 'ses_sub')).toBe(false) expect(sseAggregator.isSubagentSession('ses_sub', 'srv_a')).toBe(false) }) - it('notifies server connected listeners when a remote stream opens', () => { + it('notifies server connected listeners when a remote stream opens', async () => { const seen: string[] = [] sseAggregator.onServerConnected(serverId => { seen.push(serverId) }) hub.start() - sources.sources[1]!.open() + fake.latest('srv_b').push(SERVER_CONNECTED) + await flushPromises() expect(seen).toEqual(['srv_b']) }) @@ -430,16 +484,13 @@ describe('RemoteServerEventHub event delivery', () => { describe('RemoteServerEventHub reconnection', () => { let hub: RemoteServerEventHub let fake: FakeRegistry - let sources: ReturnType beforeEach(() => { sseAggregator.shutdown() sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) fake = createRegistry([SERVER_A]) - sources = createSourceFactory() hub = new RemoteServerEventHub(fake.registry, sseAggregator, { - eventSourceFactory: sources.factory, reconnectDelayMs: 1000, maxReconnectDelayMs: 4000, }) @@ -453,22 +504,24 @@ describe('RemoteServerEventHub reconnection', () => { async function openThenReconnect(): Promise { hub.start() - sources.sources[0]!.open() + fake.latest('srv_a').push(SERVER_CONNECTED) + await flushPromises() + fake.latest('srv_a').fail(new Error('connection lost')) await flushPromises() - sources.sources[0]!.fail({ message: 'connection lost' }) await vi.advanceTimersByTimeAsync(1000) - sources.sources[1]!.open() + fake.latest('srv_a').push(SERVER_CONNECTED) await flushPromises() } - it('sends a resync to the subscribers of the server on connect', () => { + it('sends a resync to the subscribers of the server on connect', async () => { const clientA = createCapturingClient() const clientB = createCapturingClient() sseAggregator.addClient('resync-a', clientA.callback, clientA.writeFrame, ['srv_a|/repo']) sseAggregator.addClient('resync-b', clientB.callback, clientB.writeFrame, ['srv_b|/repo']) hub.start() - sources.sources[0]!.open() + fake.latest('srv_a').push(SERVER_CONNECTED) + await flushPromises() const resyncs = clientA.events.filter(event => event.event === 'resync') expect(resyncs).toHaveLength(1) @@ -476,40 +529,69 @@ describe('RemoteServerEventHub reconnection', () => { expect(clientB.events.filter(event => event.event === 'resync')).toHaveLength(0) }) - it('closes the failed stream and reconnects with exponential backoff', async () => { + it('reconnects with exponential backoff after a stream error', async () => { vi.useFakeTimers() hub.start() - const first = sources.sources[0]! - first.open() + const first = fake.latest('srv_a') + first.push(SERVER_CONNECTED) await flushPromises() first.fail({ message: 'Unauthorized', code: 401 }) + await flushPromises() - expect(first.closed).toBe(true) expect(hub.getStatuses()).toEqual([ { serverId: 'srv_a', connected: false, version: '2.0.15', error: 'Unauthorized' }, ]) await vi.advanceTimersByTimeAsync(999) - expect(sources.sources).toHaveLength(1) + expect(fake.transport('srv_a').subscriptions).toHaveLength(1) await vi.advanceTimersByTimeAsync(1) - expect(sources.sources).toHaveLength(2) + expect(fake.transport('srv_a').subscriptions).toHaveLength(2) - const second = sources.sources[1]! + const second = fake.latest('srv_a') second.fail({ message: 'Unauthorized', code: 401 }) + await flushPromises() + + await vi.advanceTimersByTimeAsync(1000) + expect(fake.transport('srv_a').subscriptions).toHaveLength(2) await vi.advanceTimersByTimeAsync(1000) - expect(sources.sources).toHaveLength(2) + expect(fake.transport('srv_a').subscriptions).toHaveLength(3) + }) + + it('reconnects after the stream errors and after it ends', async () => { + vi.useFakeTimers() + hub.start() + const first = fake.latest('srv_a') + first.push(SERVER_CONNECTED) + await flushPromises() + + first.end() + await flushPromises() + expect(hub.getStatuses()[0]?.connected).toBe(false) await vi.advanceTimersByTimeAsync(1000) - expect(sources.sources).toHaveLength(3) + expect(fake.transport('srv_a').subscriptions).toHaveLength(2) + + const second = fake.latest('srv_a') + second.push(SERVER_CONNECTED) + await flushPromises() + expect(hub.getStatuses()[0]?.connected).toBe(true) + + second.fail(new Error('connection reset')) + await flushPromises() + await vi.advanceTimersByTimeAsync(2000) + + expect(fake.transport('srv_a').subscriptions).toHaveLength(3) + expect(hub.getStatuses()[0]?.error).toBe('connection reset') }) it('reports the version lazily from the registry without polling', async () => { hub.start() - sources.sources[0]!.open() + const stream = fake.latest('srv_a') + stream.push(SERVER_CONNECTED) await flushPromises() - sources.sources[0]!.open() + stream.push(SERVER_CONNECTED) await flushPromises() expect(fake.registry.test).toHaveBeenCalledTimes(1) @@ -548,7 +630,7 @@ describe('RemoteServerEventHub reconnection', () => { it('clears the version and probes again when the server is updated', async () => { hub.start() - sources.sources[0]!.open() + fake.latest('srv_a').push(SERVER_CONNECTED) await flushPromises() expect(hub.getStatuses()[0]?.version).toBe('2.0.15') @@ -557,7 +639,7 @@ describe('RemoteServerEventHub reconnection', () => { expect(hub.getStatuses()[0]?.version).toBeNull() - sources.sources[1]!.open() + fake.latest('srv_a').push(SERVER_CONNECTED) await flushPromises() expect(fake.registry.test).toHaveBeenCalledTimes(2) @@ -571,13 +653,13 @@ describe('RemoteServerEventHub reconnection', () => { })) hub.start() - sources.sources[0]!.open() + fake.latest('srv_a').push(SERVER_CONNECTED) await flushPromises() expect(fake.registry.test).toHaveBeenCalledTimes(1) vi.mocked(fake.registry.test).mockResolvedValueOnce({ ok: true, version: '2.1.0', versionSupported: true, error: null }) fake.emit({ type: 'upserted', serverId: 'srv_a' }) - sources.sources[1]!.open() + fake.latest('srv_a').push(SERVER_CONNECTED) await flushPromises() resolveFirstProbe({ ok: true, version: '1.0.0', versionSupported: true, error: null }) @@ -587,59 +669,60 @@ describe('RemoteServerEventHub reconnection', () => { expect(hub.getStatuses()[0]?.version).toBe('2.1.0') }) - it('closes and forgets the stream when the server is disabled', () => { + it('closes and forgets the stream when the server is disabled', async () => { const client = createCapturingClient() sseAggregator.addClient('disable', client.callback, client.writeFrame, ['srv_a|/repo']) hub.start() - const source = sources.sources[0]! - source.open() + const stream = fake.latest('srv_a') + stream.push(SERVER_CONNECTED) + await flushPromises() client.frames.length = 0 fake.setServer({ ...SERVER_A, enabled: false }) fake.emit({ type: 'upserted', serverId: 'srv_a' }) - expect(source.closed).toBe(true) + expect(stream.signal.aborted).toBe(true) expect(hub.getStatuses()).toEqual([ { serverId: 'srv_a', connected: false, version: null, error: null }, ]) - source.message(remoteEvent('session.idle', { sessionID: 'ses_1' }, '/repo')) + stream.push(remoteEvent('session.idle', { sessionID: 'ses_1' }, '/repo')) + await flushPromises() expect(client.frames).toHaveLength(0) }) - it('reconnects with a fresh stream when the server is updated', () => { + it('reconnects with a fresh client when the server is updated', () => { hub.start() - const first = sources.sources[0]! - first.open() + const first = fake.latest('srv_a') fake.setServer({ ...SERVER_A, baseUrl: 'https://moved.example.com' }) fake.emit({ type: 'upserted', serverId: 'srv_a' }) - expect(first.closed).toBe(true) - expect(sources.sources).toHaveLength(2) - expect(sources.sources[1]!.url).toBe('https://moved.example.com/api/event') + expect(first.signal.aborted).toBe(true) + expect(fake.transport('srv_a').subscriptions).toHaveLength(2) + expect(fake.clientCalls).toEqual(['srv_a', 'srv_a']) }) it('closes the stream when the server is deleted', () => { hub.start() - const source = sources.sources[0]! - source.open() + const stream = fake.latest('srv_a') fake.removeServer('srv_a') fake.emit({ type: 'deleted', serverId: 'srv_a' }) - expect(source.closed).toBe(true) + expect(stream.signal.aborted).toBe(true) expect(hub.getStatuses()).toEqual([]) }) - it('stops reconnecting and clears the replay hook after shutdown', () => { + it('stops reconnecting and clears the replay hook after shutdown', async () => { hub.start() - const source = sources.sources[0]! - source.open() + const stream = fake.latest('srv_a') + stream.push(SERVER_CONNECTED) + await flushPromises() hub.shutdown() - expect(source.closed).toBe(true) + expect(stream.signal.aborted).toBe(true) const client = createCapturingClient() sseAggregator.addClient('after-shutdown', client.callback, client.writeFrame, ['srv_a|/repo']) @@ -650,16 +733,13 @@ describe('RemoteServerEventHub reconnection', () => { describe('RemoteServerEventHub failure handling', () => { let hub: RemoteServerEventHub let fake: FakeRegistry - let sources: ReturnType beforeEach(() => { sseAggregator.shutdown() sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) fake = createRegistry([SERVER_A]) - sources = createSourceFactory() hub = new RemoteServerEventHub(fake.registry, sseAggregator, { - eventSourceFactory: sources.factory, reconnectDelayMs: 1000, maxReconnectDelayMs: 4000, }) @@ -675,47 +755,48 @@ describe('RemoteServerEventHub failure handling', () => { vi.useFakeTimers() hub.start() - const first = sources.sources[0]! - first.fail(new Error('boom')) + fake.latest('srv_a').fail(new Error('boom')) + await flushPromises() expect(hub.getStatuses()[0]?.error).toBe('boom') await vi.advanceTimersByTimeAsync(1000) - sources.sources[1]!.fail('plain failure') + fake.latest('srv_a').fail('plain failure') + await flushPromises() expect(hub.getStatuses()[0]?.error).toBe('plain failure') await vi.advanceTimersByTimeAsync(2000) - sources.sources[2]!.fail({ code: 500 }) + fake.latest('srv_a').fail({ code: 500 }) + await flushPromises() expect(hub.getStatuses()[0]?.error).toBe('event stream closed (code=500)') await vi.advanceTimersByTimeAsync(4000) - sources.sources[3]!.fail({}) + fake.latest('srv_a').fail({}) + await flushPromises() expect(hub.getStatuses()[0]?.error).toBe('event stream disconnected') expect(hub.getStatuses()[0]?.error).not.toContain('Basic') expect(hub.getStatuses()[0]?.error).not.toContain('studio.example.com') }) - it('records a connection failure without credentials and without opening a stream', () => { + it('records a connection failure without credentials and without subscribing', () => { ;(fake.registry as unknown as { getConnection: (serverId: string) => never }).getConnection = () => { throw new Error('OpenCode server is disabled: srv_a') } hub.start() - expect(sources.sources).toHaveLength(0) + expect(fake.transport('srv_a').subscriptions).toHaveLength(0) expect(hub.getStatuses()).toEqual([ { serverId: 'srv_a', connected: false, version: null, error: 'OpenCode server is disabled: srv_a' }, ]) }) - it('retries with backoff when the event source cannot be created', async () => { + it('retries with backoff when the client cannot be created', async () => { vi.useFakeTimers() let attempts = 0 - const factory: RemoteEventSourceFactory = () => { + ;(fake.registry as unknown as { getClient: (serverId: string) => never }).getClient = () => { attempts += 1 throw new Error('bad event source') } - hub.shutdown() - hub = new RemoteServerEventHub(fake.registry, sseAggregator, { eventSourceFactory: factory, reconnectDelayMs: 1000 }) hub.start() expect(attempts).toBe(1) @@ -735,24 +816,25 @@ describe('RemoteServerEventHub failure handling', () => { expect(hub.getStatuses()).toEqual([]) }) - it('forgets the stream when the server cannot be read on change', () => { + it('forgets the stream when the server cannot be read on change', async () => { hub.start() - const source = sources.sources[0]! - source.open() + const stream = fake.latest('srv_a') + stream.push(SERVER_CONNECTED) + await flushPromises() ;(fake.registry as unknown as { get: (serverId: string) => never }).get = () => { throw new Error('db down') } fake.emit({ type: 'upserted', serverId: 'srv_a' }) - expect(source.closed).toBe(true) + expect(stream.signal.aborted).toBe(true) }) it('keeps the version null when the version probe rejects', async () => { vi.mocked(fake.registry.test).mockRejectedValue(new Error('no info endpoint')) hub.start() - sources.sources[0]!.open() + fake.latest('srv_a').push(SERVER_CONNECTED) await flushPromises() expect(hub.getStatuses()).toEqual([ @@ -764,7 +846,6 @@ describe('RemoteServerEventHub failure handling', () => { describe('RemoteServerEventHub pending action replay', () => { let hub: RemoteServerEventHub let fake: FakeRegistry - let sources: ReturnType beforeEach(() => { sseAggregator.shutdown() @@ -774,8 +855,7 @@ describe('RemoteServerEventHub pending action replay', () => { permissions: { '/repo': [{ id: 'perm-1', sessionID: 'ses-1', action: 'shell', resources: ['ls'] }] }, forms: { '/repo': [{ id: 'form-1', sessionID: 'ses-1', title: 'Continue?', fields: [] }] }, }) - sources = createSourceFactory() - hub = new RemoteServerEventHub(fake.registry, sseAggregator, { eventSourceFactory: sources.factory }) + hub = new RemoteServerEventHub(fake.registry, sseAggregator) }) afterEach(() => { @@ -783,13 +863,14 @@ describe('RemoteServerEventHub pending action replay', () => { sseAggregator.shutdown() }) - function trackSessionStatus(sessionID: string, directory: string, status: string): void { - sources.sources[0]!.message(remoteEvent('session.status', { sessionID, status: { type: status } }, directory)) + async function trackSessionStatus(sessionID: string, directory: string, status: string): Promise { + fake.latest('srv_a').push(remoteEvent('session.status', { sessionID, status: { type: status } }, directory)) + await flushPromises() } it('replays pending remote actions to the subscribing client only', async () => { hub.start() - trackSessionStatus('ses-1', '/repo', 'busy') + await trackSessionStatus('ses-1', '/repo', 'busy') const client = createCapturingClient() const other = createCapturingClient() sseAggregator.addClient('replay-a', client.callback, client.writeFrame, ['srv_a|/repo']) @@ -825,8 +906,8 @@ describe('RemoteServerEventHub pending action replay', () => { it('stops replaying for a linked directory once its session goes idle', async () => { hub.start() - trackSessionStatus('ses-1', '/repo', 'busy') - trackSessionStatus('ses-1', '/repo', 'idle') + await trackSessionStatus('ses-1', '/repo', 'busy') + await trackSessionStatus('ses-1', '/repo', 'idle') const client = createCapturingClient() sseAggregator.addClient('settled', client.callback, client.writeFrame, ['srv_a|/repo']) @@ -842,9 +923,9 @@ describe('RemoteServerEventHub pending action replay', () => { sessionDirectories: { 'ses-1': '/repo' }, permissions: { '/repo': [{ id: 'perm-1', sessionID: 'ses-1', action: 'shell', resources: ['ls'] }] }, }) - hub = new RemoteServerEventHub(fake.registry, sseAggregator, { eventSourceFactory: sources.factory }) + hub = new RemoteServerEventHub(fake.registry, sseAggregator) hub.start() - sources.sources[0]!.open() + fake.latest('srv_a').push(SERVER_CONNECTED) await flushPromises() const client = createCapturingClient() @@ -857,11 +938,11 @@ describe('RemoteServerEventHub pending action replay', () => { }) it('does not use the local pending action fetcher for remote keys', async () => { - const localApi = makeApi({ permissions: { '/repo': [{ id: 'local-perm' }] } }) + const localApi = makeApi({ permissions: { '/repo': [{ id: 'local-perm' }] } }, []) sseAggregator.setOpenCodeClient({ api: localApi }) hub.start() - trackSessionStatus('ses-1', '/repo', 'busy') + await trackSessionStatus('ses-1', '/repo', 'busy') const client = createCapturingClient() sseAggregator.addClient('mixed', client.callback, client.writeFrame, ['/repo', 'srv_a|/repo']) @@ -875,7 +956,7 @@ describe('RemoteServerEventHub pending action replay', () => { it('replays remote actions when a client subscribes later', async () => { hub.start() - trackSessionStatus('ses-1', '/repo', 'busy') + await trackSessionStatus('ses-1', '/repo', 'busy') const client = createCapturingClient() sseAggregator.addClient('late', client.callback, client.writeFrame, []) @@ -894,7 +975,7 @@ describe('RemoteServerEventHub pending action replay', () => { vi.mocked(fake.api.form.list).mockResolvedValue({ location: { directory: '/repo' }, data: [] }) hub.start() - trackSessionStatus('ses-1', '/repo', 'busy') + await trackSessionStatus('ses-1', '/repo', 'busy') const client = createCapturingClient() sseAggregator.addClient('unsubscribed', client.callback, client.writeFrame, ['srv_a|/repo']) sseAggregator.removeDirectories('unsubscribed', ['srv_a|/repo']) @@ -909,13 +990,11 @@ describe('RemoteServerEventHub pending action replay', () => { describe('RemoteServerEventHub session status replay', () => { let hub: RemoteServerEventHub let fake: FakeRegistry - let sources: ReturnType beforeEach(() => { sseAggregator.shutdown() sseAggregator.setOpenCodeClient(null) sseAggregator.setScheduledSessionsResolver(() => []) - sources = createSourceFactory() }) afterEach(() => { @@ -925,7 +1004,7 @@ describe('RemoteServerEventHub session status replay', () => { function createHub(fixture: ApiFixture = {}): void { fake = createRegistry([SERVER_A], fixture) - hub = new RemoteServerEventHub(fake.registry, sseAggregator, { eventSourceFactory: sources.factory }) + hub = new RemoteServerEventHub(fake.registry, sseAggregator) } function statusFrames(client: CapturedClient) { @@ -934,10 +1013,10 @@ describe('RemoteServerEventHub session status replay', () => { async function trackSessionThenReconnect(sessionID: string, directory: string): Promise { hub.start() - const first = sources.sources[0]! - first.open() + fake.latest('srv_a').push(SERVER_CONNECTED) + await flushPromises() + fake.latest('srv_a').push(remoteEvent('session.created', { sessionID }, directory)) await flushPromises() - first.message(remoteEvent('session.created', { sessionID }, directory)) fake.emit({ type: 'upserted', serverId: 'srv_a' }) await flushPromises() } @@ -948,7 +1027,7 @@ describe('RemoteServerEventHub session status replay', () => { const client = createCapturingClient() sseAggregator.addClient('status-idle', client.callback, client.writeFrame, ['srv_a|/repo']) - sources.sources[1]!.open() + fake.latest('srv_a').push(SERVER_CONNECTED) await flushPromises() const statuses = statusFrames(client) @@ -965,7 +1044,7 @@ describe('RemoteServerEventHub session status replay', () => { const client = createCapturingClient() sseAggregator.addClient('status-busy', client.callback, client.writeFrame, ['srv_a|/repo']) - sources.sources[1]!.open() + fake.latest('srv_a').push(SERVER_CONNECTED) await flushPromises() const statuses = statusFrames(client) @@ -983,7 +1062,7 @@ describe('RemoteServerEventHub session status replay', () => { const client = createCapturingClient() sseAggregator.addClient('status-failure', client.callback, client.writeFrame, ['srv_a|/repo']) - sources.sources[1]!.open() + fake.latest('srv_a').push(SERVER_CONNECTED) await flushPromises() expect(fake.api.session.active).toHaveBeenCalledTimes(2) @@ -996,10 +1075,11 @@ describe('RemoteServerEventHub session status replay', () => { sseAggregator.addClient('status-stale', client.callback, client.writeFrame, ['srv_a|/repo']) hub.start() - const first = sources.sources[0]! - first.open() + const first = fake.latest('srv_a') + first.push(SERVER_CONNECTED) + await flushPromises() + first.push(remoteEvent('session.created', { sessionID: 'ses_1' }, '/repo')) await flushPromises() - first.message(remoteEvent('session.created', { sessionID: 'ses_1' }, '/repo')) let resolveActive: (value: Record) => void = () => {} vi.mocked(fake.api.session.active).mockReturnValueOnce(new Promise>((resolve) => { @@ -1007,12 +1087,13 @@ describe('RemoteServerEventHub session status replay', () => { }) as never) fake.emit({ type: 'upserted', serverId: 'srv_a' }) - const second = sources.sources[1]! - second.open() + const second = fake.latest('srv_a') + second.push(SERVER_CONNECTED) await flushPromises() expect(fake.api.session.active).toHaveBeenCalledTimes(2) - second.fail({ message: 'connection lost' }) + second.fail(new Error('connection lost')) + await flushPromises() resolveActive({}) await flushPromises() diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 89f9f9d3e..30d6f6978 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -47,9 +47,6 @@ importers: croner: specifier: ^10.0.1 version: 10.0.1 - eventsource: - specifier: ^4.1.0 - version: 4.1.0 hono: specifier: ^4.13.13 version: 4.13.13 @@ -78,9 +75,6 @@ importers: '@types/bun': specifier: latest version: 1.3.10 - '@types/eventsource': - specifier: ^3.0.0 - version: 3.0.0 '@types/web-push': specifier: ^3.6.4 version: 3.6.4 @@ -2474,10 +2468,6 @@ packages: '@types/estree@1.0.8': resolution: {integrity: sha512-dWHzHa2WqEXI/O1E9OjrocMTKJl2mSrEolh1Iomrv6U+JuNwaHXsXx9bLu5gG7BUWFIN0skIQJQ/L1rIex4X6w==} - '@types/eventsource@3.0.0': - resolution: {integrity: sha512-yEhFj31FTD29DtNeqePu+A+lD6loRef6YOM5XfN1kUwBHyy2DySGlA3jJU+FbQSkrfmlBVluf2Dub/OyReFGKA==} - deprecated: This is a stub types definition. eventsource provides its own type definitions, so you do not need this installed. - '@types/geojson@7946.0.16': resolution: {integrity: sha512-6C8nqWur3j98U6+lXDfTUWIfgvZU+EumvpHKcYjujKH7woYyLj2sUmff0tRhrqM7BohUw7Pz3ZB1jj2gW9Fvmg==} @@ -3493,14 +3483,6 @@ packages: resolution: {integrity: sha512-mQw+2fkQbALzQ7V0MY0IqdnXNOeTtP4r0lN9z7AAawCXgqea7bDii20AYrIBrFd/Hx0M2Ocz6S111CaFkUcb0Q==} engines: {node: '>=0.8.x'} - eventsource-parser@3.0.6: - resolution: {integrity: sha512-Vo1ab+QXPzZ4tCa8SwIHJFaSzy4R6SHf7BY79rFBDf0idraZWAkYrDjDj8uWaSm3S2TK+hJ7/t1CEmZ7jXw+pg==} - engines: {node: '>=18.0.0'} - - eventsource@4.1.0: - resolution: {integrity: sha512-2GuF51iuHX6A9xdTccMTsNb7VO0lHZihApxhvQzJB5A03DvHDd2FQepodbMaztPBmBcE/ox7o2gqaxGhYB9LhQ==} - engines: {node: '>=20.0.0'} - expand-template@2.0.3: resolution: {integrity: sha512-XYfuKMvj4O35f/pOXLObndIRvyQ+/+6AhODh+OKWj9S9498pHHn/IMszH+gt0fBCRWMNfk1ZSp5x3AifmnI2vg==} engines: {node: '>=6'} @@ -7905,10 +7887,6 @@ snapshots: '@types/estree@1.0.8': {} - '@types/eventsource@3.0.0': - dependencies: - eventsource: 4.1.0 - '@types/geojson@7946.0.16': {} '@types/hast@3.0.4': @@ -8135,7 +8113,7 @@ snapshots: sirv: 3.0.2 tinyglobby: 0.2.15 tinyrainbow: 2.0.0 - vitest: 3.2.4(@types/debug@4.1.12)(@types/node@24.10.4)(@vitest/ui@3.2.4)(jiti@2.6.1)(jsdom@27.4.0)(lightningcss@1.30.2)(terser@5.46.0)(yaml@2.9.0) + vitest: 3.2.4(@types/debug@4.1.12)(@types/node@22.19.19)(@vitest/ui@3.2.4)(jiti@2.6.1)(jsdom@27.4.0)(lightningcss@1.30.2)(terser@5.46.0)(yaml@2.9.0) '@vitest/utils@3.2.4': dependencies: @@ -9001,12 +8979,6 @@ snapshots: events@3.3.0: {} - eventsource-parser@3.0.6: {} - - eventsource@4.1.0: - dependencies: - eventsource-parser: 3.0.6 - expand-template@2.0.3: {} expect-type@1.3.0: {} From 2b00825b0a92d28dc855fd8e4b5a36d200a03a39 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:11:15 -0500 Subject: [PATCH 04/20] section 4: Phase 4: One streaming proxy forwarder used by both proxy ro --- backend/src/routes/opencode-proxy.ts | 15 +- backend/src/routes/opencode-server-proxy.ts | 57 ++----- backend/src/utils/proxy-forward.ts | 79 ++++++++++ backend/test/routes/opencode-proxy.test.ts | 12 ++ backend/test/utils/proxy-forward.test.ts | 160 ++++++++++++++++++++ 5 files changed, 270 insertions(+), 53 deletions(-) create mode 100644 backend/src/utils/proxy-forward.ts create mode 100644 backend/test/utils/proxy-forward.test.ts diff --git a/backend/src/routes/opencode-proxy.ts b/backend/src/routes/opencode-proxy.ts index d3be10b56..02dd2c009 100644 --- a/backend/src/routes/opencode-proxy.ts +++ b/backend/src/routes/opencode-proxy.ts @@ -16,6 +16,7 @@ import { getRepoById } from '../db/queries' import { getTrustedOrigins, getWorkspacePath } from '@opencode-manager/shared/config/env' import { isPathWithinRoot } from '../utils/fs-safe' import { buildProxyResponseHeaders, filterProxyHeaders } from '../utils/proxy-headers' +import { forwardProxyRequest } from '../utils/proxy-forward' import { connectUpstreamWebSocket, createBridgedSocketEvents, @@ -192,20 +193,12 @@ export function createOpenCodeProxyRoutes(db: Database, settingsService: Setting const upstreamUrl = `${getOpenCodeUpstreamBaseUrl()}${pathSuffix}${url.search}` try { - const upstreamResponse = await fetch(upstreamUrl, { + return await forwardProxyRequest({ + url: upstreamUrl, method: c.req.method, headers, body: requestBody, - redirect: 'manual', - duplex: 'half', - }) - - const responseHeaders = buildProxyResponseHeaders(upstreamResponse.headers) - - return new Response(upstreamResponse.body, { - status: upstreamResponse.status, - statusText: upstreamResponse.statusText, - headers: responseHeaders, + buildResponseHeaders: buildProxyResponseHeaders, }) } catch { return c.json({ error: 'Proxy request failed' }, 502) diff --git a/backend/src/routes/opencode-server-proxy.ts b/backend/src/routes/opencode-server-proxy.ts index 38c68b310..38050678c 100644 --- a/backend/src/routes/opencode-server-proxy.ts +++ b/backend/src/routes/opencode-server-proxy.ts @@ -6,7 +6,8 @@ import { type OpenCodeServerConnection, type OpenCodeServerRegistry, } from '../services/opencode-server-registry' -import { filterProxyHeaders, NULL_BODY_STATUSES } from '../utils/proxy-headers' +import { filterProxyHeaders } from '../utils/proxy-headers' +import { forwardProxyRequest, ProxyHeadersTimeoutError } from '../utils/proxy-forward' import { logger } from '../utils/logger' const MOUNT_PREFIX = '/api/opencode/servers/' @@ -117,58 +118,30 @@ export function createOpenCodeServerProxyRoutes( headers.Authorization = connection.authorization } - const hasBody = c.req.method !== 'GET' && c.req.method !== 'HEAD' - - let headersTimeoutTimer: ReturnType | null = null - let headersTimedOut = false - let upstreamSignal = c.req.raw.signal - if (!hasBody) { - const timeoutController = new AbortController() - headersTimeoutTimer = setTimeout(() => { - headersTimedOut = true - timeoutController.abort() - }, upstreamHeadersTimeoutMs) - upstreamSignal = AbortSignal.any([c.req.raw.signal, timeoutController.signal]) - } + const requestBody = c.req.method !== 'GET' && c.req.method !== 'HEAD' + ? c.req.raw.body + : null try { - const upstreamResponse = await fetchFn(upstream.url.toString(), { + return await forwardProxyRequest({ + url: upstream.url.toString(), method: c.req.method, headers, - body: hasBody ? c.req.raw.body : undefined, - redirect: 'manual', - signal: upstreamSignal, - duplex: 'half', - }) - - if (headersTimeoutTimer !== null) { - clearTimeout(headersTimeoutTimer) - headersTimeoutTimer = null - } - - const responseHeaders = new Headers( - filterProxyHeaders(upstreamResponse.headers, RESPONSE_EXCLUDED_HEADERS), - ) - const responseBody = NULL_BODY_STATUSES.has(upstreamResponse.status) - ? null - : upstreamResponse.body - - return new Response(responseBody, { - status: upstreamResponse.status, - statusText: upstreamResponse.statusText, - headers: responseHeaders, + body: requestBody, + signal: c.req.raw.signal, + headersTimeoutMs: requestBody === null ? upstreamHeadersTimeoutMs : undefined, + fetchFn, + buildResponseHeaders: (upstreamHeaders) => new Headers( + filterProxyHeaders(upstreamHeaders, RESPONSE_EXCLUDED_HEADERS), + ), }) } catch (error) { - if (headersTimedOut) { + if (error instanceof ProxyHeadersTimeoutError) { logger.warn(`OpenCode server proxy timed out waiting for headers: ${c.req.method} ${upstream.url.host}${upstreamPath.path}`, error) return c.json({ error: 'Remote OpenCode server timed out' }, 504) } logger.warn(`OpenCode server proxy request failed: ${c.req.method} ${upstream.url.host}${upstreamPath.path}`, error) return c.json({ error: 'Remote OpenCode server unreachable' }, 502) - } finally { - if (headersTimeoutTimer !== null) { - clearTimeout(headersTimeoutTimer) - } } }) diff --git a/backend/src/utils/proxy-forward.ts b/backend/src/utils/proxy-forward.ts new file mode 100644 index 000000000..55a70a095 --- /dev/null +++ b/backend/src/utils/proxy-forward.ts @@ -0,0 +1,79 @@ +import { NULL_BODY_STATUSES } from './proxy-headers' + +export class ProxyHeadersTimeoutError extends Error { + constructor(timeoutMs: number) { + super(`Upstream did not send response headers within ${timeoutMs}ms`) + this.name = 'ProxyHeadersTimeoutError' + } +} + +export interface ForwardProxyRequestInput { + url: string | URL + method: string + headers: Record + body: RequestInit['body'] + signal?: AbortSignal + headersTimeoutMs?: number + fetchFn?: typeof fetch + buildResponseHeaders: (upstream: Headers) => Headers +} + +/** + * Forwards one HTTP request to an upstream server and returns its response with the + * body left unread so it keeps streaming to the caller. + * + * When `headersTimeoutMs` is set, requests without a body are aborted if the upstream + * does not send response headers in time; that abort surfaces as a + * `ProxyHeadersTimeoutError`. Every other fetch failure is rethrown unchanged. + */ +export async function forwardProxyRequest({ + url, + method, + headers, + body, + signal, + headersTimeoutMs, + fetchFn = fetch, + buildResponseHeaders, +}: ForwardProxyRequestInput): Promise { + const timeoutMs = body === null || body === undefined ? headersTimeoutMs : undefined + const timeoutController = timeoutMs === undefined ? null : new AbortController() + let headersTimedOut = false + const timeoutTimer = timeoutController === null + ? null + : setTimeout(() => { + headersTimedOut = true + timeoutController.abort() + }, timeoutMs) + + const requestSignal = timeoutController === null + ? signal + : signal + ? AbortSignal.any([signal, timeoutController.signal]) + : timeoutController.signal + + let upstream: Response + try { + upstream = await fetchFn(url, { + method, + headers, + body: body ?? undefined, + redirect: 'manual', + duplex: 'half', + signal: requestSignal, + }) + } catch (error) { + if (headersTimedOut && timeoutMs !== undefined) { + throw new ProxyHeadersTimeoutError(timeoutMs) + } + throw error + } finally { + if (timeoutTimer !== null) clearTimeout(timeoutTimer) + } + + return new Response(NULL_BODY_STATUSES.has(upstream.status) ? null : upstream.body, { + status: upstream.status, + statusText: upstream.statusText, + headers: buildResponseHeaders(upstream.headers), + }) +} diff --git a/backend/test/routes/opencode-proxy.test.ts b/backend/test/routes/opencode-proxy.test.ts index cb491644f..2ca982a9b 100644 --- a/backend/test/routes/opencode-proxy.test.ts +++ b/backend/test/routes/opencode-proxy.test.ts @@ -323,6 +323,18 @@ describe('opencode-proxy routes', () => { expect(body.error).toBe('Proxy request failed') }) + it('returns a null body for bodyless upstream statuses', async () => { + const upstreamFetch = vi.fn().mockResolvedValue(new Response(null, { status: 204 })) + globalThis.fetch = upstreamFetch as unknown as typeof fetch + + const res = await app.request('/api/opencode-proxy/doc', { + headers: { Authorization: 'Bearer test-internal-token' }, + }) + + expect(res.status).toBe(204) + expect(await res.text()).toBe('') + }) + it('preserves query string in upstream URL', async () => { const upstreamFetch = vi.fn().mockResolvedValue( new Response('ok', { status: 200, headers: { 'content-type': 'text/plain' } }) diff --git a/backend/test/utils/proxy-forward.test.ts b/backend/test/utils/proxy-forward.test.ts new file mode 100644 index 000000000..50a81838d --- /dev/null +++ b/backend/test/utils/proxy-forward.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, it, vi } from 'vitest' +import { + forwardProxyRequest, + ProxyHeadersTimeoutError, + type ForwardProxyRequestInput, +} from '../../src/utils/proxy-forward' + +function forward(overrides: Partial = {}): Promise { + return forwardProxyRequest({ + url: 'http://upstream.example/api/event', + method: 'GET', + headers: { 'x-test': '1' }, + body: null, + buildResponseHeaders: (headers) => new Headers(headers), + ...overrides, + }) +} + +function deferred() { + let resolve!: () => void + const promise = new Promise((settle) => { + resolve = settle + }) + return { promise, resolve } +} + +async function readWithin( + reader: ReadableStreamDefaultReader, + timeoutMs = 500, +) { + let timer: ReturnType | undefined + try { + return await Promise.race([ + reader.read(), + new Promise((_resolve, reject) => { + timer = setTimeout(() => reject(new Error('upstream chunk was buffered instead of streamed')), timeoutMs) + }), + ]) + } finally { + if (timer !== undefined) clearTimeout(timer) + } +} + +describe('forwardProxyRequest', () => { + it('streams the upstream body instead of buffering it', async () => { + const encoder = new TextEncoder() + const secondChunk = deferred() + let secondChunkEnqueued = false + const upstream = new Response(new ReadableStream({ + async start(controller) { + controller.enqueue(encoder.encode('first')) + await secondChunk.promise + secondChunkEnqueued = true + controller.enqueue(encoder.encode('second')) + controller.close() + }, + })) + const fetchFn = vi.fn().mockResolvedValue(upstream) + + const response = await forward({ fetchFn: fetchFn as unknown as typeof fetch }) + const reader = response.body?.getReader() + expect(reader).toBeDefined() + + const decoder = new TextDecoder() + const first = await readWithin(reader as ReadableStreamDefaultReader) + expect(decoder.decode(first.value)).toBe('first') + expect(secondChunkEnqueued).toBe(false) + + secondChunk.resolve() + const readerRef = reader as ReadableStreamDefaultReader + expect(decoder.decode((await readerRef.read()).value)).toBe('second') + expect((await readerRef.read()).done).toBe(true) + }) + + it.each([204, 304])('returns a null body for upstream status %i', async (status) => { + const fetchFn = vi.fn().mockResolvedValue(new Response(null, { status })) + + const response = await forward({ fetchFn: fetchFn as unknown as typeof fetch }) + + expect(response.status).toBe(status) + expect(response.body).toBeNull() + }) + + it('aborts a body-less request when the upstream sends no headers in time', async () => { + let receivedSignal: AbortSignal | undefined + const fetchFn = vi.fn((_url: string | URL, init: RequestInit) => new Promise((_resolve, reject) => { + receivedSignal = init.signal ?? undefined + init.signal?.addEventListener('abort', () => reject(new DOMException('Aborted', 'AbortError'))) + })) + + await expect( + forward({ fetchFn: fetchFn as unknown as typeof fetch, headersTimeoutMs: 10 }), + ).rejects.toBeInstanceOf(ProxyHeadersTimeoutError) + expect(receivedSignal?.aborted).toBe(true) + }) + + it('does not apply the headers timeout to a request with a body', async () => { + const client = new AbortController() + let receivedSignal: AbortSignal | undefined + const fetchFn = vi.fn((_url: string | URL, init: RequestInit) => new Promise((resolve, reject) => { + receivedSignal = init.signal ?? undefined + init.signal?.addEventListener('abort', () => reject(new DOMException('Aborted', 'AbortError'))) + setTimeout(() => resolve(new Response('ok')), 40) + })) + + const response = await forward({ + method: 'POST', + body: '{"title":"x"}', + signal: client.signal, + headersTimeoutMs: 10, + fetchFn: fetchFn as unknown as typeof fetch, + }) + + expect(response.status).toBe(200) + expect(receivedSignal).toBe(client.signal) + }) + + it('rethrows non-timeout fetch failures unchanged', async () => { + const failure = new Error('connect ECONNREFUSED') + const fetchFn = vi.fn().mockRejectedValue(failure) + + await expect(forward({ fetchFn: fetchFn as unknown as typeof fetch })).rejects.toBe(failure) + }) + + it('forwards method, headers, and body with a manual redirect and half duplex', async () => { + const fetchFn = vi.fn().mockResolvedValue(new Response('ok', { status: 201, statusText: 'Created' })) + + const response = await forward({ + url: 'http://upstream.example/api/session', + method: 'POST', + headers: { 'content-type': 'application/json', 'x-test': '1' }, + body: '{"title":"x"}', + fetchFn: fetchFn as unknown as typeof fetch, + }) + + const [calledUrl, init] = fetchFn.mock.calls[0] as [string | URL, RequestInit] + expect(calledUrl).toBe('http://upstream.example/api/session') + expect(init.method).toBe('POST') + expect(init.headers).toEqual({ 'content-type': 'application/json', 'x-test': '1' }) + expect(init.body).toBe('{"title":"x"}') + expect(init.redirect).toBe('manual') + expect(init.duplex).toBe('half') + expect(response.status).toBe(201) + expect(response.statusText).toBe('Created') + }) + + it('builds the returned response headers from the upstream response', async () => { + const fetchFn = vi.fn().mockResolvedValue(new Response('ok', { + status: 200, + headers: { 'content-type': 'text/event-stream' }, + })) + + const response = await forward({ + fetchFn: fetchFn as unknown as typeof fetch, + buildResponseHeaders: (headers) => new Headers({ 'x-proxied': headers.get('content-type') ?? '' }), + }) + + expect(response.headers.get('x-proxied')).toBe('text/event-stream') + }) +}) From 66b5d1ac06be7aa7030faea2c77968051d9bfe52 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:15:37 -0500 Subject: [PATCH 05/20] section 5: Phase 5: Stop buffering in /api/opencode/* --- backend/src/services/opencode/client.ts | 71 +++++-------------- .../test/routes/opencode-auth-proxy.test.ts | 56 ++++++++++++++- backend/test/services/opencode/client.test.ts | 16 +++-- 3 files changed, 82 insertions(+), 61 deletions(-) diff --git a/backend/src/services/opencode/client.ts b/backend/src/services/opencode/client.ts index 8dc63166a..4d89cf07d 100644 --- a/backend/src/services/opencode/client.ts +++ b/backend/src/services/opencode/client.ts @@ -1,7 +1,8 @@ import { logger } from '../../utils/logger' import { ENV } from '@opencode-manager/shared/config/env' import { createOpenCodeApi, type OpenCodeApi } from '@opencode-manager/shared/opencode' -import { filterProxyHeaders, NULL_BODY_STATUSES } from '../../utils/proxy-headers' +import { filterProxyHeaders } from '../../utils/proxy-headers' +import { forwardProxyRequest } from '../../utils/proxy-forward' import { getOpenCodeBasicAuthHeader, type OpenCodePasswordResolver } from './auth' import { getOpenCodeUpstreamBaseUrl, withDefaultOpenCodeDirectory } from './upstream' @@ -61,76 +62,36 @@ export class FetchOpenCodeClient implements OpenCodeClient { return this.fetchFn(input, { ...init, headers }) } - private async request(req: { method: string; path: string; body?: string; headers?: Record }): Promise { - const url = new URL(this.resolveBaseUrl() + req.path) - - const headers: Record = this.withDirectoryHeader({ ...(req.headers ?? {}) }) - + async forwardRaw(request: Request): Promise { + const url = new URL(request.url) + const path = url.pathname.replace(/^\/api\/opencode/, '') + url.search + const headers = this.withDirectoryHeader(filterProxyHeaders(request.headers)) const basicAuth = await this.getBasicAuth() if (basicAuth) { headers.Authorization = basicAuth } + const hasBody = request.method !== 'GET' && request.method !== 'HEAD' + try { - const response = await this.fetchFn(url, { - method: req.method, + return await forwardProxyRequest({ + url: this.resolveBaseUrl() + path, + method: request.method, headers, - body: req.body, - }) - - const filteredHeaders = filterProxyHeaders(response.headers) - - if (NULL_BODY_STATUSES.has(response.status)) { - return new Response(null, { - status: response.status, - statusText: response.statusText, - headers: filteredHeaders, - }) - } - - const body = await response.text() - return new Response(body, { - status: response.status, - statusText: response.statusText, - headers: filteredHeaders, + body: hasBody ? request.body : null, + signal: request.signal, + fetchFn: this.fetchFn, + buildResponseHeaders: (upstream) => new Headers(filterProxyHeaders(upstream)), }) } catch (error) { - logger.error(`Proxy request failed for ${req.path}:`, error) + logger.error(`Proxy request failed for ${path}:`, error) return new Response(JSON.stringify({ error: 'Proxy request failed' }), { status: 502, headers: { 'Content-Type': 'application/json' }, }) } } - - async forwardRaw(request: Request): Promise { - const url = new URL(request.url) - const cleanPathname = url.pathname.replace(/^\/api\/opencode/, '') - - if (url.pathname.includes('/permissions/')) { - logger.info(`Proxying permission request: ${url.pathname}${url.search} -> ${cleanPathname}${url.search}`) - } - - const headers: Record = {} - request.headers.forEach((value, key) => { - const lowerKey = key.toLowerCase() - if (!['host', 'connection', 'authorization'].includes(lowerKey)) { - headers[key] = value - } - }) - - const body = request.method !== 'GET' && request.method !== 'HEAD' - ? await request.text() - : undefined - - return this.request({ - method: request.method, - path: cleanPathname + url.search, - body, - headers, - }) - } } export function createOpenCodeClient( diff --git a/backend/test/routes/opencode-auth-proxy.test.ts b/backend/test/routes/opencode-auth-proxy.test.ts index 40d1afa35..fa550fae6 100644 --- a/backend/test/routes/opencode-auth-proxy.test.ts +++ b/backend/test/routes/opencode-auth-proxy.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { Hono } from 'hono' import type { MiddlewareHandler } from 'hono' import { createAuthenticatedOpenCodeProxyRoutes } from '../../src/routes/opencode-auth-proxy' -import type { OpenCodeClient } from '../../src/services/opencode/client' +import { FetchOpenCodeClient, type OpenCodeClient } from '../../src/services/opencode/client' import { OpenCodeSupervisor } from '../../src/services/opencode-supervisor' import type { SettingsService } from '../../src/services/settings' @@ -14,7 +14,8 @@ vi.mock('../../src/services/opencode-single-server', () => ({ const forwardRawMock = vi.hoisted(() => vi.fn(async () => new Response('ok', { status: 200 }))) -vi.mock('../../src/services/opencode/client', () => ({ +vi.mock('../../src/services/opencode/client', async (importOriginal) => ({ + ...(await importOriginal()), createOpenCodeClient: vi.fn(), })) @@ -344,3 +345,54 @@ describe('authenticated opencode proxy routes', () => { expect(forwarded).toEqual(JSON.parse(body)) }) }) + +describe('authenticated opencode proxy streaming', () => { + it('streams the upstream body chunk by chunk while forwarding the request body as a stream', async () => { + const encoder = new TextEncoder() + const decoder = new TextDecoder() + let upstreamController!: ReadableStreamDefaultController + const upstreamBody = new ReadableStream({ + start(controller) { + upstreamController = controller + }, + }) + + let capturedInit: RequestInit | undefined + const fetchFn = async (_input: URL | Request | string, init?: RequestInit) => { + capturedInit = init + return new Response(upstreamBody, { status: 200, headers: { 'Content-Type': 'text/event-stream' } }) + } + const client = new FetchOpenCodeClient({ + baseUrl: 'http://127.0.0.1:5551', + basicAuth: '', + fetchFn: fetchFn as unknown as typeof fetch, + }) + + const app = new Hono() + app.route('/api/opencode', createAuthenticatedOpenCodeProxyRoutes(client, passThroughAuth)) + + const responsePromise = app.request('/api/opencode/event', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ hello: 'world' }), + }) + + upstreamController.enqueue(encoder.encode('first-chunk')) + const response = await responsePromise + expect(response.status).toBe(200) + expect(response.headers.get('content-type')).toBe('text/event-stream') + + const reader = response.body!.getReader() + const first = await reader.read() + expect(decoder.decode(first.value)).toBe('first-chunk') + + upstreamController.enqueue(encoder.encode('second-chunk')) + const second = await reader.read() + expect(decoder.decode(second.value)).toBe('second-chunk') + upstreamController.close() + await reader.cancel() + + expect(capturedInit?.body).toBeInstanceOf(ReadableStream) + expect(capturedInit?.duplex).toBe('half') + }) +}) diff --git a/backend/test/services/opencode/client.test.ts b/backend/test/services/opencode/client.test.ts index 0587efbbc..abeca5c50 100644 --- a/backend/test/services/opencode/client.test.ts +++ b/backend/test/services/opencode/client.test.ts @@ -36,6 +36,11 @@ import { createOpenCodeClient, FetchOpenCodeClient } from '../../../src/services import { ENV } from '@opencode-manager/shared/config/env' import { openCodeLocation } from '@opencode-manager/shared/opencode' +async function readBodyText(body: RequestInit['body']): Promise { + if (!(body instanceof ReadableStream)) return null + return await new Response(body).text() +} + describe('OpenCodeClient', () => { const baseUrl = 'http://127.0.0.1:5551' const basicAuth = 'Basic dXNlcjpwYXNz' @@ -91,9 +96,9 @@ describe('OpenCodeClient', () => { expect((capturedInit?.headers as Record)['x-opencode-directory']).toBe('%2Frepo%2Fb') }) - it('should read body for POST but not for GET/HEAD', async () => { + it('should stream the body for POST but not for GET/HEAD', async () => { const mockResponse = new Response(JSON.stringify({}), { status: 200 }) - const capturedBodies: unknown[] = [] + const capturedBodies: Array = [] const fetchFn = async (_: URL | Request | string, init?: RequestInit) => { capturedBodies.push(init?.body) return mockResponse @@ -119,7 +124,8 @@ describe('OpenCodeClient', () => { await client.forwardRaw(headRequest) - expect(capturedBodies[0]).toBe('test body') + expect(capturedBodies[0]).toBeInstanceOf(ReadableStream) + expect(await readBodyText(capturedBodies[0])).toBe('test body') expect(capturedBodies[1]).toBeUndefined() expect(capturedBodies[2]).toBeUndefined() }) @@ -177,7 +183,9 @@ describe('OpenCodeClient', () => { expect(capturedUrl?.toString()).toBe(baseUrl + '/config?foo=bar') expect(capturedInit?.method).toBe('POST') - expect(capturedInit?.body).toBe('request-body-content') + expect(capturedInit?.body).toBeInstanceOf(ReadableStream) + expect(await readBodyText(capturedInit?.body)).toBe('request-body-content') + expect(capturedInit?.duplex).toBe('half') expect(capturedInit?.headers).toEqual(expect.objectContaining({ Authorization: basicAuth, 'x-caller': 'caller-value', From 48ca38c4e3b63f50c1d95fc546561a32c868af93 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:23:14 -0500 Subject: [PATCH 06/20] section 6: Phase 6: Config.Info validation owner and v2 config types in --- backend/src/routes/opencode-config.ts | 6 +- backend/src/services/opencode-config-file.ts | 33 ++++++----- .../routes/internal-opencode-config.test.ts | 24 +++++++- .../services/opencode-config-apply.test.ts | 4 +- .../services/opencode-config-file.test.ts | 52 ++++++++---------- .../opencode-config-validation.test.ts | 55 +++++++++++++++++++ backend/test/services/opencode-import.test.ts | 6 +- .../settings/OpenCodeConfigEditor.test.tsx | 40 ++++++++++---- .../settings/OpenCodeConfigEditor.tsx | 10 +--- pnpm-lock.yaml | 5 +- shared/package.json | 2 + shared/src/opencode/config-validation.ts | 41 ++++++++++++++ shared/src/opencode/config.ts | 13 +++++ shared/src/opencode/index.ts | 10 ++++ shared/src/opencode/mcp.ts | 7 +-- shared/src/opencode/modelRef.ts | 3 +- 16 files changed, 232 insertions(+), 79 deletions(-) create mode 100644 backend/test/services/opencode-config-validation.test.ts create mode 100644 shared/src/opencode/config-validation.ts create mode 100644 shared/src/opencode/config.ts diff --git a/backend/src/routes/opencode-config.ts b/backend/src/routes/opencode-config.ts index 027a633f5..1525228cb 100644 --- a/backend/src/routes/opencode-config.ts +++ b/backend/src/routes/opencode-config.ts @@ -1,5 +1,4 @@ import { Hono, type Context } from 'hono' -import { z } from 'zod' import { UpdateOpenCodeConfigPatchRequestSchema, UpdateOpenCodeConfigRequestSchema, @@ -20,6 +19,7 @@ import { OpenCodeConfigRedactedValueError, OpenCodeConfigShadowedRemovalError, OpenCodeConfigSourceInvalidError, + OpenCodeConfigValidationError, readOpenCodeConfigFile, withOpenCodeConfigLock, } from '../services/opencode-config-file' @@ -198,8 +198,8 @@ async function applyUpdate( if (error instanceof OpenCodeConfigRedactedValueError) { return c.json({ error: error.message, paths: error.paths }, 400) } - if (error instanceof z.ZodError) { - return c.json({ error: 'Invalid config data', details: error.issues }, 400) + if (error instanceof OpenCodeConfigValidationError) { + return c.json({ error: 'Invalid config data', validationIssues: error.issues }, 400) } if (error instanceof SyntaxError) { return c.json({ error: 'Invalid config data', details: error.message }, 400) diff --git a/backend/src/services/opencode-config-file.ts b/backend/src/services/opencode-config-file.ts index e0044b9d5..5574e6a49 100644 --- a/backend/src/services/opencode-config-file.ts +++ b/backend/src/services/opencode-config-file.ts @@ -4,7 +4,6 @@ import { readFile, readdir, rename, rm, stat } from 'fs/promises' import path from 'path' import { isDeepStrictEqual } from 'node:util' import { applyEdits, modify, type JSONPath } from 'jsonc-parser' -import type { ZodIssue } from 'zod' import { getConfigPath, getOpenCodeConfigHome, getOpenCodeHealthWatchPath } from '@opencode-manager/shared/config/env' import { DEFAULT_OPENCODE_CONFIG_SOURCE_NAME, @@ -13,7 +12,7 @@ import { isOpenCodeConfigSourceName, selectPreferredOpenCodeConfigSourceName, } from '@opencode-manager/shared' -import { OpenCodeConfigSchema } from '@opencode-manager/shared/schemas' +import { validateOpenCodeConfig } from '@opencode-manager/shared/opencode/config-validation' import { parseJsonc } from '@opencode-manager/shared/utils' import type { OpenCodeConfigFile, @@ -107,6 +106,16 @@ export class OpenCodeConfigRedactedValueError extends Error { } } +export class OpenCodeConfigValidationError extends Error { + readonly issues: OpenCodeConfigValidationIssue[] + + constructor(issues: OpenCodeConfigValidationIssue[]) { + super('OpenCode config is invalid') + this.name = 'OpenCodeConfigValidationError' + this.issues = issues + } +} + interface OpenCodeConfigParseResult { content: Record isValid: boolean @@ -271,11 +280,9 @@ export function withOpenCodeConfigLock(fn: () => Promise): Promise { return withFileLock(getOpenCodeConfigDirectory(), fn) } -export function toOpenCodeConfigValidationIssues(issues: ZodIssue[]): OpenCodeConfigValidationIssue[] { - return issues.map((issue) => ({ - path: issue.path.length > 0 ? issue.path.join('.') : 'root', - message: issue.message, - })) +function assertValidOpenCodeConfig(value: unknown): void { + const result = validateOpenCodeConfig(value) + if (!result.success) throw new OpenCodeConfigValidationError(result.issues) } export function parseOpenCodeConfigContent(rawContent: string): OpenCodeConfigParseResult { @@ -294,7 +301,7 @@ export function parseOpenCodeConfigContent(rawContent: string): OpenCodeConfigPa const content = isPlainObject(parsed) ? parsed : {} - const validated = OpenCodeConfigSchema.safeParse(parsed) + const validated = validateOpenCodeConfig(parsed) if (validated.success) { return { content, @@ -302,7 +309,7 @@ export function parseOpenCodeConfigContent(rawContent: string): OpenCodeConfigPa } } - const validationIssues = toOpenCodeConfigValidationIssues(validated.error.issues) + const validationIssues = validated.issues logger.error(`Failed to validate OpenCode config: ${validationIssues.map((issue) => `${issue.path}: ${issue.message}`).join('; ')}`) return { @@ -402,7 +409,7 @@ export async function writeOpenCodeConfigFile( source?: OpenCodeConfigSourceName, snapshot?: OpenCodeConfigSnapshot, ): Promise { - OpenCodeConfigSchema.parse(parseJsonc(rawContent)) + assertValidOpenCodeConfig(parseJsonc(rawContent)) const resolved = snapshot ?? await readOpenCodeConfigSnapshot() const targetName = source !== undefined @@ -526,7 +533,7 @@ export async function updateOpenCodeConfigFile( const mode = options.mode ?? 'replace' const operations = collectOpenCodeConfigPathOperations(content, snapshot.content, mode) - OpenCodeConfigSchema.parse( + assertValidOpenCodeConfig( mode === 'merge' ? mergeOpenCodeConfigValues(snapshot.content, content, { deleteNullValues: true }) : content, ) @@ -535,7 +542,7 @@ export async function updateOpenCodeConfigFile( const updatedText = applyOpenCodeConfigPathOperations(originalText, operations) if (updatedText !== originalText) { - OpenCodeConfigSchema.parse(parseJsonc(updatedText)) + assertValidOpenCodeConfig(parseJsonc(updatedText)) await writeFileAtomic(targetPath, updatedText, { mode: await existingFileMode(targetPath) }) } @@ -755,7 +762,7 @@ export async function foldLegacyConfigJsonSource(): Promise { .filter((operation) => operation.value !== undefined && !hasOpenCodeConfigPath(lowestPrecedenceSource.content, operation.path)) if (operations.length > 0) { const updatedText = applyOpenCodeConfigPathOperations(lowestPrecedenceSource.rawContent, operations) - OpenCodeConfigSchema.parse(parseJsonc(updatedText)) + assertValidOpenCodeConfig(parseJsonc(updatedText)) await writeFileAtomic(lowestPrecedenceSource.path, updatedText, { mode: await existingFileMode(lowestPrecedenceSource.path) }) } await rm(legacyPath, { force: true }) diff --git a/backend/test/routes/internal-opencode-config.test.ts b/backend/test/routes/internal-opencode-config.test.ts index 5cd6307c4..55a5316d0 100644 --- a/backend/test/routes/internal-opencode-config.test.ts +++ b/backend/test/routes/internal-opencode-config.test.ts @@ -415,15 +415,15 @@ describe('internal/opencode-config routes', () => { }) it('PUT /api/internal/opencode-config returns 409 for a shadowed removal', async () => { - const lower = '{"theme":"light","model":"a"}' - const target = '{"model":"b"}' + const lower = '{"theme":"light","model":"test/a"}' + const target = '{"model":"test/b"}' await writeOpenCodeConfigFile(target, 'opencode.jsonc') await writeFile(configPath('opencode.json'), lower, 'utf8') const res = await app.request('/api/internal/opencode-config', { method: 'PUT', headers: { 'content-type': 'application/json', ...authHeaders() }, - body: JSON.stringify({ content: { model: 'b' } }), + body: JSON.stringify({ content: { model: 'test/b' } }), }) expect(res.status).toBe(409) @@ -445,6 +445,24 @@ describe('internal/opencode-config routes', () => { }) expect(res.status).toBe(400) + const body = await res.json() as { error: string; validationIssues: Array<{ path: string; message: string }> } + expect(body.error).toBe('Invalid config data') + expect(body.validationIssues.map((issue) => issue.path)).toEqual(['model']) + }) + + it('PUT /api/internal/opencode-config returns 400 with validationIssues for invalid raw content', async () => { + await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED, 'opencode.jsonc') + + const res = await app.request('/api/internal/opencode-config', { + method: 'PUT', + headers: { 'content-type': 'application/json', ...authHeaders() }, + body: JSON.stringify({ content: '{"model": 5}' }), + }) + + expect(res.status).toBe(400) + const body = await res.json() as { error: string; validationIssues: Array<{ path: string; message: string }> } + expect(body.error).toBe('Invalid config data') + expect(body.validationIssues.map((issue) => issue.path)).toEqual(['model']) }) it('PUT /api/internal/opencode-config returns 400 for an invalid JSON body', async () => { diff --git a/backend/test/services/opencode-config-apply.test.ts b/backend/test/services/opencode-config-apply.test.ts index 624ff7f83..70a64baa5 100644 --- a/backend/test/services/opencode-config-apply.test.ts +++ b/backend/test/services/opencode-config-apply.test.ts @@ -3,7 +3,6 @@ import { mkdir, mkdtemp, readFile, readdir, rm, writeFile } from 'fs/promises' import { tmpdir } from 'os' import path from 'path' import { Database } from 'bun:sqlite' -import { ZodError } from 'zod' const paths = vi.hoisted(() => ({ config: '', configDir: '', configHome: '', healthWatch: '' })) @@ -48,6 +47,7 @@ import { OPENCODE_CONFIG_SEED, OpenCodeConfigConflictError, OpenCodeConfigSourceInvalidError, + OpenCodeConfigValidationError, readOpenCodeConfigFile, writeOpenCodeConfigFile, } from '../../src/services/opencode-config-file' @@ -231,7 +231,7 @@ describe('opencode-config-apply', () => { source: 'opencode.json', settingsService, openCodeClient, - })).rejects.toBeInstanceOf(ZodError) + })).rejects.toBeInstanceOf(OpenCodeConfigValidationError) await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe(previous) expect(markRestartPendingMock).not.toHaveBeenCalled() diff --git a/backend/test/services/opencode-config-file.test.ts b/backend/test/services/opencode-config-file.test.ts index 49a417f37..d90676961 100644 --- a/backend/test/services/opencode-config-file.test.ts +++ b/backend/test/services/opencode-config-file.test.ts @@ -2,7 +2,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { access, chmod, mkdir, mkdtemp, readFile, readdir, rm, stat, utimes, writeFile } from 'fs/promises' import { tmpdir } from 'os' import path from 'path' -import { ZodError } from 'zod' const paths = vi.hoisted(() => ({ config: '', configDir: '', configHome: '', healthWatch: '' })) @@ -79,6 +78,7 @@ import { OpenCodeConfigRedactedValueError, OpenCodeConfigShadowedRemovalError, OpenCodeConfigSnapshotError, + OpenCodeConfigValidationError, archiveBrokenOpenCodeConfigFile, deleteOpenCodeConfigFile, foldLegacyConfigJsonSource, @@ -88,7 +88,6 @@ import { restoreLegacyOpenCodeConfigBackup, restoreOpenCodeConfigSnapshot, serializeOpenCodeConfigSnapshot, - toOpenCodeConfigValidationIssues, updateOpenCodeConfigFile, writeHealthWatchArtifact, writeOpenCodeConfigFile, @@ -166,10 +165,10 @@ describe('opencode-config-file', () => { } }) - it('rejects invalid content with a ZodError and leaves the previous file untouched', async () => { + it('rejects invalid content with a validation error and leaves the previous file untouched', async () => { await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED) - await expect(writeOpenCodeConfigFile('{"model": 5}')).rejects.toBeInstanceOf(ZodError) + await expect(writeOpenCodeConfigFile('{"model": 5}')).rejects.toBeInstanceOf(OpenCodeConfigValidationError) await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).resolves.toBe(OPENCODE_CONFIG_SEED) }) @@ -236,16 +235,13 @@ describe('opencode-config-file', () => { expect(file?.validationIssues?.[0]?.path).toBe('root') }) - it('maps Zod issues to config validation issues with a root fallback', () => { - const issues = new ZodError([ - { code: 'custom', path: ['model'], message: 'Invalid model' }, - { code: 'custom', path: [], message: 'Invalid root' }, - ]).issues + it('maps validation issues to dotted paths with a root fallback', async () => { + await writeFile(sourcePath('opencode.json'), JSON.stringify({ agents: { a: { mode: 'boss' } } }), 'utf8') - expect(toOpenCodeConfigValidationIssues(issues)).toEqual([ - { path: 'model', message: 'Invalid model' }, - { path: 'root', message: 'Invalid root' }, - ]) + const file = await readOpenCodeConfigFile() + + expect(file?.isValid).toBe(false) + expect(file?.validationIssues?.[0]?.path).toBe('agents.a.mode') }) it('modifies only changed paths in the preferred source and preserves comments and untouched fields', async () => { @@ -338,14 +334,14 @@ describe('opencode-config-file', () => { const raw = '{"theme":"dark"}' await writeFile(sourcePath('opencode.json'), raw, 'utf8') - await expect(updateOpenCodeConfigFile({ model: 5 }, { mode: 'merge' })).rejects.toBeInstanceOf(ZodError) + await expect(updateOpenCodeConfigFile({ model: 5 }, { mode: 'merge' })).rejects.toBeInstanceOf(OpenCodeConfigValidationError) await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe(raw) }) it('rejects a merge patch that removes a value only a lower-priority source defines', async () => { const lower = '{"theme":"light"}' - const target = '{"model":"b"}' + const target = '{"model":"test/b"}' await writeFile(sourcePath('opencode.json'), lower, 'utf8') await writeFile(sourcePath('opencode.jsonc'), target, 'utf8') @@ -360,7 +356,7 @@ describe('opencode-config-file', () => { it('merges into an existing nested object while deleting sibling keys with null', async () => { const raw = JSON.stringify({ - mcp: { servers: { linear: { url: 'https://linear.example.com', headers: { Authorization: 'Bearer old', 'X-Keep': '1' } } } }, + mcp: { servers: { linear: { type: 'remote', url: 'https://linear.example.com', headers: { Authorization: 'Bearer old', 'X-Keep': '1' } } } }, }) await writeFile(sourcePath('opencode.json'), raw, 'utf8') @@ -370,7 +366,7 @@ describe('opencode-config-file', () => { ) expect(updated.content).toEqual({ - mcp: { servers: { linear: { url: 'https://linear.example.com', headers: { 'X-Keep': '1' } } } }, + mcp: { servers: { linear: { type: 'remote', url: 'https://linear.example.com', headers: { 'X-Keep': '1' } } } }, }) const onDisk = JSON.parse(await readFile(sourcePath('opencode.json'), 'utf8')) as Record expect(onDisk).toEqual(updated.content) @@ -380,13 +376,13 @@ describe('opencode-config-file', () => { await writeFile(sourcePath('opencode.json'), '{"theme":"dark"}', 'utf8') const updated = await updateOpenCodeConfigFile( - { mcp: { servers: { linear: { url: 'https://linear.example.com', headers: { Authorization: null, 'X-Trace': 'on' } } } } }, + { mcp: { servers: { linear: { type: 'remote', url: 'https://linear.example.com', headers: { Authorization: null, 'X-Trace': 'on' } } } } }, { mode: 'merge' }, ) expect(updated.content).toEqual({ theme: 'dark', - mcp: { servers: { linear: { url: 'https://linear.example.com', headers: { 'X-Trace': 'on' } } } }, + mcp: { servers: { linear: { type: 'remote', url: 'https://linear.example.com', headers: { 'X-Trace': 'on' } } } }, }) const onDisk = await readFile(sourcePath('opencode.json'), 'utf8') expect(onDisk).not.toContain('null') @@ -436,25 +432,25 @@ describe('opencode-config-file', () => { }) it('removes an override from the target source only and reveals inherited values', async () => { - const lower = JSON.stringify({ theme: 'light', model: 'a' }) + const lower = JSON.stringify({ theme: 'light', model: 'test/a' }) await writeFile(sourcePath('opencode.json'), lower, 'utf8') await writeFile(sourcePath('opencode.jsonc'), '{\n "theme": "dark"\n}\n', 'utf8') - await updateOpenCodeConfigFile({ model: 'a' }) + await updateOpenCodeConfigFile({ model: 'test/a' }) await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe(lower) const revealed = await readOpenCodeConfigFile() - expect(revealed?.content).toEqual({ theme: 'light', model: 'a' }) + expect(revealed?.content).toEqual({ theme: 'light', model: 'test/a' }) }) it('rejects a removal of a value that only a lower-priority source defines and writes nothing', async () => { - const lower = '{\n "theme": "light",\n "model": "a"\n}\n' - const target = '{\n "model": "b"\n}\n' + const lower = '{\n "theme": "light",\n "model": "test/a"\n}\n' + const target = '{\n "model": "test/b"\n}\n' await writeFile(sourcePath('opencode.json'), lower, 'utf8') await writeFile(sourcePath('opencode.jsonc'), target, 'utf8') const targetStats = await stat(sourcePath('opencode.jsonc')) - const error = await updateOpenCodeConfigFile({ model: 'b' }).catch((caught: unknown) => caught) + const error = await updateOpenCodeConfigFile({ model: 'test/b' }).catch((caught: unknown) => caught) expect(error).toBeInstanceOf(OpenCodeConfigShadowedRemovalError) expect((error as OpenCodeConfigShadowedRemovalError).paths).toEqual(['theme']) @@ -465,12 +461,12 @@ describe('opencode-config-file', () => { }) it('rejects a mixed shadowed removal and legitimate change without writing anything', async () => { - const lower = '{\n "theme": "light",\n "model": "a"\n}\n' - const target = '{\n "model": "b"\n}\n' + const lower = '{\n "theme": "light",\n "model": "test/a"\n}\n' + const target = '{\n "model": "test/b"\n}\n' await writeFile(sourcePath('opencode.json'), lower, 'utf8') await writeFile(sourcePath('opencode.jsonc'), target, 'utf8') - const error = await updateOpenCodeConfigFile({ model: 'b', plugin: ['x'] }).catch((caught: unknown) => caught) + const error = await updateOpenCodeConfigFile({ model: 'test/b', plugin: ['x'] }).catch((caught: unknown) => caught) expect(error).toBeInstanceOf(OpenCodeConfigShadowedRemovalError) expect((error as OpenCodeConfigShadowedRemovalError).paths).toEqual(['theme']) diff --git a/backend/test/services/opencode-config-validation.test.ts b/backend/test/services/opencode-config-validation.test.ts new file mode 100644 index 000000000..c212802d2 --- /dev/null +++ b/backend/test/services/opencode-config-validation.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest' +import { validateOpenCodeConfig } from '@opencode-manager/shared/opencode/config-validation' + +describe('validateOpenCodeConfig', () => { + it('accepts a valid v2 config document', () => { + expect( + validateOpenCodeConfig({ + $schema: 'https://opencode.ai/config.json', + model: 'anthropic/claude-sonnet-4', + share: 'manual', + agents: { assistant: { mode: 'primary' } }, + }), + ).toEqual({ success: true }) + }) + + it('accepts v2-only fields', () => { + expect( + validateOpenCodeConfig({ + shell: '/bin/zsh', + lsp: true, + timezone: 'Europe/Madrid', + policies: [{ action: 'tool.use', resource: 'bash', effect: 'deny' }], + }), + ).toEqual({ success: true }) + }) + + it('rejects invalid nested values with dotted paths', () => { + const result = validateOpenCodeConfig({ agents: { a: { mode: 'boss' } } }) + + expect(result.success).toBe(false) + if (result.success) return + expect(result.issues.map((issue) => issue.path)).toContain('agents.a.mode') + }) + + it('rejects invalid enum values', () => { + const result = validateOpenCodeConfig({ share: 'sometimes' }) + + expect(result.success).toBe(false) + if (result.success) return + expect(result.issues).toEqual([ + expect.objectContaining({ path: 'share' }), + ]) + }) + + it('accepts v1-only keys because unknown keys are ignored', () => { + expect( + validateOpenCodeConfig({ + agent: { build: { mode: 'primary' } }, + command: { deploy: { template: 'ship it' } }, + provider: { openai: { options: {} } }, + permission: { edit: 'allow' }, + }), + ).toEqual({ success: true }) + }) +}) diff --git a/backend/test/services/opencode-import.test.ts b/backend/test/services/opencode-import.test.ts index 756d805d9..0bac4a3a6 100644 --- a/backend/test/services/opencode-import.test.ts +++ b/backend/test/services/opencode-import.test.ts @@ -86,8 +86,8 @@ describe('opencode-import service', () => { const jsoncPath = path.join(os.homedir(), '.config', 'opencode', 'opencode.jsonc') mockExistsSync.mockImplementation((candidate: string) => [jsonPath, jsoncPath].includes(candidate)) mockFileExists.mockResolvedValue(false) - const rawJson = '{"model":"lower"}' - const rawJsonc = '{\n// keep\n"model":"higher"\n}' + const rawJson = '{"model":"test/lower"}' + const rawJsonc = '{\n// keep\n"model":"test/higher"\n}' mockReadFileContent.mockImplementation(async (candidate: string) => candidate === jsoncPath ? rawJsonc : rawJson) const result = await syncOpenCodeImport({}) @@ -115,7 +115,7 @@ describe('opencode-import service', () => { const legacyPath = path.join(os.homedir(), '.config', 'opencode', 'config.json') mockExistsSync.mockImplementation((candidate: string) => [jsonPath, legacyPath].includes(candidate)) mockFileExists.mockResolvedValue(false) - const rawJson = '{"model":"lower"}' + const rawJson = '{"model":"test/lower"}' const rawLegacy = '{"theme":"dark"}' mockReadFileContent.mockImplementation(async (candidate: string) => candidate === legacyPath ? rawLegacy : rawJson) diff --git a/frontend/src/components/settings/OpenCodeConfigEditor.test.tsx b/frontend/src/components/settings/OpenCodeConfigEditor.test.tsx index 64462e6c4..04f3378de 100644 --- a/frontend/src/components/settings/OpenCodeConfigEditor.test.tsx +++ b/frontend/src/components/settings/OpenCodeConfigEditor.test.tsx @@ -155,9 +155,14 @@ describe('OpenCodeConfigEditor', () => { expect(scrollTopSets).toBeGreaterThanOrEqual(2) }) - it('lists schema validation issues as line jumps', async () => { + it('lists backend validation issues as line jumps', async () => { + const onUpdate = vi.fn().mockRejectedValue( + new FetchError('Invalid config data', 400, undefined, undefined, { + validationIssues: [{ path: 'theme', message: 'Expected string' }], + }), + ) + renderEditor({ onUpdate }) const user = userEvent.setup() - renderEditor() const textarea = screen.getByLabelText('Config content') as HTMLTextAreaElement setContent(textarea, '{\n "theme": 42\n}') await user.click(screen.getByRole('button', { name: 'Update' })) @@ -169,16 +174,22 @@ describe('OpenCodeConfigEditor', () => { expect(row).toHaveAttribute('data-line', '2') }) - it('does not call onUpdate when local validation fails', async () => { - const { onUpdate } = renderEditor() + it('sends content the browser cannot validate to the backend instead of rejecting it locally', async () => { + const onUpdate = vi.fn().mockRejectedValue( + new FetchError('Invalid config data', 400, undefined, undefined, { + validationIssues: [{ path: 'theme', message: 'Expected string' }], + }), + ) + renderEditor({ onUpdate }) const user = userEvent.setup() const textarea = screen.getByLabelText('Config content') as HTMLTextAreaElement - setContent(textarea, '{\n "theme": 42\n}') + const invalid = '{\n "theme": 42\n}' + setContent(textarea, invalid) await user.click(screen.getByRole('button', { name: 'Update' })) await waitFor(() => { - expect(screen.getByText(/Configuration validation failed/)).toBeInTheDocument() + expect(onUpdate).toHaveBeenCalledWith({ content: invalid, source: 'opencode.json', expectedRevision: 'rev-1' }) }) - expect(onUpdate).not.toHaveBeenCalled() + expect(await screen.findByText(/theme: Expected string/)).toBeInTheDocument() }) it('saves the raw text including comments', async () => { @@ -348,14 +359,19 @@ describe('OpenCodeConfigEditor', () => { expect(onUpdate).toHaveBeenCalledWith({ content: RAW, source: 'opencode.json', expectedRevision: 'rev-1' }) }) - it('resolves a validation issue under a dotted provider key to its line', async () => { + it('resolves a nested backend validation issue to its line', async () => { + const onUpdate = vi.fn().mockRejectedValue( + new FetchError('Invalid config data', 400, undefined, undefined, { + validationIssues: [{ path: 'agents.assistant.mode', message: 'Expected a known mode' }], + }), + ) + renderEditor({ onUpdate }) const user = userEvent.setup() - renderEditor() const textarea = screen.getByLabelText('Config content') as HTMLTextAreaElement - const dotted = '{\n "provider": {\n "api.example.com": {\n "key": 123\n }\n }\n}' - setContent(textarea, dotted) + const nested = '{\n "agents": {\n "assistant": {\n "mode": 42\n }\n }\n}' + setContent(textarea, nested) await user.click(screen.getByRole('button', { name: 'Update' })) - const issueButton = await screen.findByRole('button', { name: /api\.example\.com/i }) + const issueButton = await screen.findByRole('button', { name: /agents\.assistant\.mode/i }) expect(issueButton).toBeInTheDocument() await user.click(issueButton) const activeLine = document.body.querySelector('[data-active-line]') diff --git a/frontend/src/components/settings/OpenCodeConfigEditor.tsx b/frontend/src/components/settings/OpenCodeConfigEditor.tsx index 97816e6b7..d763123f2 100644 --- a/frontend/src/components/settings/OpenCodeConfigEditor.tsx +++ b/frontend/src/components/settings/OpenCodeConfigEditor.tsx @@ -12,7 +12,6 @@ import { useMobile } from '@/hooks/useMobile' import { useFindInText } from '@/lib/useFindInText' import { parseJsonc, parseJsoncErrorLine, resolveJsoncIssueLine } from '@/lib/jsonc' import { FetchError } from '@/api/fetchWrapper' -import { OpenCodeConfigSchema } from '@opencode-manager/shared' import { downloadOpenCodeConfigSource, getOpenCodeConfigSources, @@ -143,14 +142,7 @@ export function OpenCodeConfigEditor({ try { resetErrors() - const parsedConfig = parseJsonc>(editConfigContent) - const validationResult = OpenCodeConfigSchema.safeParse(parsedConfig) - if (!validationResult.success) { - const issues = resolveIssues(validationResult.error.issues) - setValidationIssues(issues) - setEditError(`Configuration validation failed: ${issues.map(getIssueText).join('; ')}`) - return - } + parseJsonc>(editConfigContent) setIsSaving(true) await onUpdate({ content: editConfigContent, source: draftSource.name, expectedRevision: draftRevision }) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 30d6f6978..80fb8e5a4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -352,6 +352,9 @@ importers: '@opencode/schema': specifier: 2.2.1 version: 2.2.1 + effect: + specifier: 4.0.1 + version: 4.0.1 jsonc-parser: specifier: ^3.3.1 version: 3.3.1 @@ -8113,7 +8116,7 @@ snapshots: sirv: 3.0.2 tinyglobby: 0.2.15 tinyrainbow: 2.0.0 - vitest: 3.2.4(@types/debug@4.1.12)(@types/node@22.19.19)(@vitest/ui@3.2.4)(jiti@2.6.1)(jsdom@27.4.0)(lightningcss@1.30.2)(terser@5.46.0)(yaml@2.9.0) + vitest: 3.2.4(@types/debug@4.1.12)(@types/node@24.10.4)(@vitest/ui@3.2.4)(jiti@2.6.1)(jsdom@27.4.0)(lightningcss@1.30.2)(terser@5.46.0)(yaml@2.9.0) '@vitest/utils@3.2.4': dependencies: diff --git a/shared/package.json b/shared/package.json index aaa65f8e8..546368d3c 100644 --- a/shared/package.json +++ b/shared/package.json @@ -18,11 +18,13 @@ "./notifications": "./src/notifications/index.ts", "./project-id": "./src/project-id-resolver.ts", "./opencode": "./src/opencode/index.ts", + "./opencode/config-validation": "./src/opencode/config-validation.ts", "./themes": "./src/themes/index.ts" }, "dependencies": { "@opencode/client": "2.2.1", "@opencode/schema": "2.2.1", + "effect": "4.0.1", "jsonc-parser": "^3.3.1", "zod": "^4.1.12" }, diff --git a/shared/src/opencode/config-validation.ts b/shared/src/opencode/config-validation.ts new file mode 100644 index 000000000..c2a86b4ec --- /dev/null +++ b/shared/src/opencode/config-validation.ts @@ -0,0 +1,41 @@ +import { Info } from '@opencode/schema/config' +import { Schema } from 'effect' + +export interface OpenCodeConfigValidationIssue { + path: string + message: string +} + +export type OpenCodeConfigValidationResult = + | { success: true } + | { success: false; issues: OpenCodeConfigValidationIssue[] } + +const standardSchema = Schema.toStandardSchemaV1(Info, { + parseOptions: { errors: 'all', onExcessProperty: 'ignore' }, +}) + +function toIssuePath(path: ReadonlyArray | undefined): string { + if (!path || path.length === 0) return 'root' + return path + .map((segment) => String(typeof segment === 'object' ? segment.key : segment)) + .join('.') +} + +/** + * Validates a parsed OpenCode config document against the v2 `Config.Info` schema, ignoring + * unknown keys so v1-only fields in existing files stay loadable. + */ +export function validateOpenCodeConfig(value: unknown): OpenCodeConfigValidationResult { + const result = standardSchema['~standard'].validate(value) + if (result instanceof Promise) { + throw new Error('OpenCode config validation must be synchronous') + } + if (!result.issues) return { success: true } + return { + success: false, + issues: result.issues.map((issue) => ({ + path: toIssuePath(issue.path), + message: issue.message, + })), + } +} diff --git a/shared/src/opencode/config.ts b/shared/src/opencode/config.ts new file mode 100644 index 000000000..7ae6373bf --- /dev/null +++ b/shared/src/opencode/config.ts @@ -0,0 +1,13 @@ +import type { ConfigEntry } from '@opencode/client' + +export type OpenCodeConfigInfo = Extract['info'] + +export type OpenCodeAgentConfig = NonNullable[string] + +export type OpenCodeCommandConfig = NonNullable[string] + +export type OpenCodeProviderConfig = NonNullable[string] + +export type OpenCodeProviderModelConfig = NonNullable[string] + +export type OpenCodePermissionRule = NonNullable[number] diff --git a/shared/src/opencode/index.ts b/shared/src/opencode/index.ts index 00fef098c..8298fb758 100644 --- a/shared/src/opencode/index.ts +++ b/shared/src/opencode/index.ts @@ -22,6 +22,7 @@ export type { ModelInfo, ModelRef, PermissionRequest, + PermissionReply, PromptAgentAttachment, PromptFileAttachment, PromptMention, @@ -51,6 +52,15 @@ export { ClientError, isIntegrationNotFoundError, isMcpServerNotFoundError, isSe export { assistantText, sessionIDFromEvent, toolContentText } from './content' +export type { + OpenCodeAgentConfig, + OpenCodeCommandConfig, + OpenCodeConfigInfo, + OpenCodePermissionRule, + OpenCodeProviderConfig, + OpenCodeProviderModelConfig, +} from './config' + export { GOAL_CHANGED_EVENT_TYPE, GOAL_COMMAND_NAME, diff --git a/shared/src/opencode/mcp.ts b/shared/src/opencode/mcp.ts index 368b4634e..9f14c2c74 100644 --- a/shared/src/opencode/mcp.ts +++ b/shared/src/opencode/mcp.ts @@ -1,11 +1,10 @@ -import type { ConfigEntry, McpServer } from '@opencode/client' +import type { McpServer } from '@opencode/client' +import type { OpenCodeConfigInfo } from './config' import { isRecord } from '../utils/record' export const MCP_OAUTH_CALLBACK_PATH = '/api/mcp-oauth-proxy/callback' -type ConfigDocumentInfo = Extract['info'] - -type McpConfig = NonNullable +type McpConfig = NonNullable export type McpServerConfig = NonNullable[string] diff --git a/shared/src/opencode/modelRef.ts b/shared/src/opencode/modelRef.ts index d1a05623a..9ec275e8a 100644 --- a/shared/src/opencode/modelRef.ts +++ b/shared/src/opencode/modelRef.ts @@ -1,6 +1,7 @@ import type { ConfigEntry, ModelRef } from '@opencode/client' +import type { OpenCodeConfigInfo } from './config' -type ConfigDocumentModel = Extract['info']['model'] +type ConfigDocumentModel = OpenCodeConfigInfo['model'] export function parseOpenCodeModelRef(model: string): ModelRef | undefined { const providerEnd = model.indexOf('/') From 19d4ae25b61dc7a28aeac81babd5805be21a81e5 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:32:28 -0500 Subject: [PATCH 07/20] section 7: Phase 7: Entity saves replace the matching v1 entry in the s --- backend/src/services/opencode-config-file.ts | 203 +++++++++++++++++- .../services/opencode-config-file.test.ts | 171 +++++++++++++++ 2 files changed, 364 insertions(+), 10 deletions(-) diff --git a/backend/src/services/opencode-config-file.ts b/backend/src/services/opencode-config-file.ts index 5574e6a49..faaf0d3aa 100644 --- a/backend/src/services/opencode-config-file.ts +++ b/backend/src/services/opencode-config-file.ts @@ -3,7 +3,7 @@ import { promises as fs } from 'fs' import { readFile, readdir, rename, rm, stat } from 'fs/promises' import path from 'path' import { isDeepStrictEqual } from 'node:util' -import { applyEdits, modify, type JSONPath } from 'jsonc-parser' +import { applyEdits, findNodeAtLocation, modify, parseTree, type Edit, type JSONPath } from 'jsonc-parser' import { getConfigPath, getOpenCodeConfigHome, getOpenCodeHealthWatchPath } from '@opencode-manager/shared/config/env' import { DEFAULT_OPENCODE_CONFIG_SOURCE_NAME, @@ -148,11 +148,30 @@ interface AppliedOpenCodeConfigSourceFileState { previousMode: number | undefined } -interface OpenCodeConfigPathOperation { +export interface OpenCodeConfigPathOperation { path: JSONPath value: unknown } +interface OpenCodeConfigV1EntityPath { + v2Path: string[] + v1Path: string[] + named: boolean +} + +interface OpenCodeConfigV1EntityRemoval { + path: JSONPath + container?: JSONPath +} + +const V1_ENTITY_PATHS: readonly OpenCodeConfigV1EntityPath[] = [ + { v2Path: ['agents'], v1Path: ['agent'], named: true }, + { v2Path: ['commands'], v1Path: ['command'], named: true }, + { v2Path: ['providers'], v1Path: ['provider'], named: true }, + { v2Path: ['mcp', 'servers'], v1Path: ['mcp'], named: true }, + { v2Path: ['permissions'], v1Path: ['permission'], named: false }, +] + interface OpenCodeConfigSnapshotEnvelopeSource { name: OpenCodeConfigRestorableSourceName rawContent: string @@ -187,18 +206,40 @@ function hasOwn(object: Record, key: string): boolean { return Object.prototype.hasOwnProperty.call(object, key) } -function hasOpenCodeConfigPath(content: Record, jsonPath: JSONPath): boolean { +function readOpenCodeConfigPath( + content: Record, + jsonPath: JSONPath, +): { found: boolean; value: unknown } { let current: unknown = content for (const segment of jsonPath) { if (typeof segment === 'number') { - if (!Array.isArray(current) || segment < 0 || segment >= current.length) return false + if (!Array.isArray(current) || segment < 0 || segment >= current.length) { + return { found: false, value: undefined } + } current = current[segment] continue } - if (!isPlainObject(current) || !hasOwn(current, segment)) return false + if (!isPlainObject(current) || !hasOwn(current, segment)) return { found: false, value: undefined } + current = current[segment] + } + return { found: true, value: current } +} + +function hasOpenCodeConfigPath(content: Record, jsonPath: JSONPath): boolean { + return readOpenCodeConfigPath(content, jsonPath).found +} + +function readPatchedOpenCodeConfigPath( + patch: Record, + jsonPath: readonly string[], +): { found: boolean; value: unknown } { + let current: unknown = patch + for (const segment of jsonPath) { + if (current === null) return { found: true, value: null } + if (!isPlainObject(current) || !hasOwn(current, segment)) return { found: false, value: undefined } current = current[segment] } - return true + return { found: true, value: current } } function defineOwnConfigValue(target: Record, key: string, value: unknown): void { @@ -459,7 +500,136 @@ function collectOpenCodeConfigPathOperations( return operations } -function applyOpenCodeConfigPathOperations( +function collectOpenCodeConfigV1EntityRemovals( + patch: Record, +): OpenCodeConfigV1EntityRemoval[] { + const removals: OpenCodeConfigV1EntityRemoval[] = [] + for (const mapping of V1_ENTITY_PATHS) { + const patched = readPatchedOpenCodeConfigPath(patch, mapping.v2Path) + if (!patched.found) continue + if (!mapping.named || patched.value === null) { + removals.push({ path: mapping.v1Path }) + continue + } + if (!isPlainObject(patched.value)) continue + for (const name of Object.keys(patched.value)) { + removals.push({ path: [...mapping.v1Path, name], container: mapping.v1Path }) + } + } + return removals +} + +function findOpenCodeConfigSeparatorComma(rawContent: string, start: number, end: number): number { + let index = start + while (index < end) { + if (rawContent.startsWith('//', index)) { + const newline = rawContent.indexOf('\n', index) + if (newline < 0 || newline >= end) return -1 + index = newline + 1 + continue + } + if (rawContent.startsWith('/*', index)) { + const close = rawContent.indexOf('*/', index + 2) + if (close < 0 || close >= end) return -1 + index = close + 2 + continue + } + if (rawContent[index] === ',') return index + index += 1 + } + return -1 +} + +function findOpenCodeConfigLineEnd(rawContent: string, offset: number): number | null { + let index = offset + while (index < rawContent.length && (rawContent[index] === ' ' || rawContent[index] === '\t')) { + index += 1 + } + if (rawContent.startsWith('\r\n', index)) return index + 2 + if (rawContent[index] === '\n') return index + 1 + return null +} + +function buildOpenCodeConfigCommentPreservingRemovalEdits( + rawContent: string, + jsonPath: JSONPath, +): Edit[] | null { + const root = parseTree(rawContent) + const node = root ? findNodeAtLocation(root, jsonPath) : undefined + const property = node?.parent + const container = property?.parent + if (!property || !container?.children) return null + + const index = container.children.indexOf(property) + const previous = index > 0 ? container.children[index - 1] : undefined + const next = container.children[index + 1] + const propertyEnd = property.offset + property.length + const lineStart = rawContent.lastIndexOf('\n', property.offset - 1) + 1 + const aloneOnLine = rawContent.slice(lineStart, property.offset).trim() === '' + const removeStart = aloneOnLine ? lineStart : property.offset + + if (next) { + const separatorOffset = findOpenCodeConfigSeparatorComma(rawContent, propertyEnd, next.offset) + if (separatorOffset < 0) return null + if (rawContent.slice(propertyEnd, separatorOffset).includes('/')) { + return [ + { offset: removeStart, length: propertyEnd - removeStart, content: '' }, + { offset: separatorOffset, length: 1, content: '' }, + ] + } + const lineEnd = aloneOnLine ? findOpenCodeConfigLineEnd(rawContent, separatorOffset + 1) : null + const removeEnd = lineEnd ?? separatorOffset + 1 + return [{ offset: removeStart, length: removeEnd - removeStart, content: '' }] + } + + const edits: Edit[] = [] + if (previous) { + const separatorOffset = findOpenCodeConfigSeparatorComma( + rawContent, + previous.offset + previous.length, + property.offset, + ) + if (separatorOffset < 0) return null + edits.push({ offset: separatorOffset, length: 1, content: '' }) + } + const lineEnd = aloneOnLine ? findOpenCodeConfigLineEnd(rawContent, propertyEnd) : null + const removeEnd = lineEnd ?? propertyEnd + edits.push({ offset: removeStart, length: removeEnd - removeStart, content: '' }) + return edits +} + +function removeOpenCodeConfigProperty(rawContent: string, jsonPath: JSONPath): string { + const edits = buildOpenCodeConfigCommentPreservingRemovalEdits(rawContent, jsonPath) + if (!edits) return rawContent + return applyEdits(rawContent, edits) +} + +function collectEmptyOpenCodeConfigContainers( + rawContent: string, + removals: OpenCodeConfigV1EntityRemoval[], +): JSONPath[] { + const parsed = parseJsonc(rawContent) + if (!isPlainObject(parsed)) return [] + + const containers: JSONPath[] = [] + const visited = new Set() + for (const removal of removals) { + if (!removal.container) continue + const key = removal.container.join('\0') + if (visited.has(key)) continue + visited.add(key) + const container = readOpenCodeConfigPath(parsed, removal.container) + if (isPlainObject(container.value) && Object.keys(container.value).length === 0) { + containers.push(removal.container) + } + } + return containers +} + +/** + * Applies path edits to raw JSONC config text, preserving comments and formatting. + */ +export function applyOpenCodeConfigEdits( rawContent: string, operations: OpenCodeConfigPathOperation[], ): string { @@ -532,14 +702,27 @@ export async function updateOpenCodeConfigFile( const mode = options.mode ?? 'replace' const operations = collectOpenCodeConfigPathOperations(content, snapshot.content, mode) + const v1EntityRemovals = mode === 'merge' ? collectOpenCodeConfigV1EntityRemovals(content) : [] assertValidOpenCodeConfig( mode === 'merge' ? mergeOpenCodeConfigValues(snapshot.content, content, { deleteNullValues: true }) : content, ) const originalText = targetSource?.rawContent ?? '{}\n' - assertNoShadowedOpenCodeConfigRemovals(operations, targetSource, snapshot.sources, targetName) - const updatedText = applyOpenCodeConfigPathOperations(originalText, operations) + assertNoShadowedOpenCodeConfigRemovals( + [...operations, ...v1EntityRemovals.map((removal) => ({ path: removal.path, value: undefined }))], + targetSource, + snapshot.sources, + targetName, + ) + + let updatedText = applyOpenCodeConfigEdits(originalText, operations) + for (const removal of v1EntityRemovals) { + updatedText = removeOpenCodeConfigProperty(updatedText, removal.path) + } + for (const container of collectEmptyOpenCodeConfigContainers(updatedText, v1EntityRemovals)) { + updatedText = removeOpenCodeConfigProperty(updatedText, container) + } if (updatedText !== originalText) { assertValidOpenCodeConfig(parseJsonc(updatedText)) @@ -761,7 +944,7 @@ export async function foldLegacyConfigJsonSource(): Promise { const operations = collectOpenCodeConfigPathOperations(legacyContent, lowestPrecedenceSource.content, 'replace') .filter((operation) => operation.value !== undefined && !hasOpenCodeConfigPath(lowestPrecedenceSource.content, operation.path)) if (operations.length > 0) { - const updatedText = applyOpenCodeConfigPathOperations(lowestPrecedenceSource.rawContent, operations) + const updatedText = applyOpenCodeConfigEdits(lowestPrecedenceSource.rawContent, operations) assertValidOpenCodeConfig(parseJsonc(updatedText)) await writeFileAtomic(lowestPrecedenceSource.path, updatedText, { mode: await existingFileMode(lowestPrecedenceSource.path) }) } diff --git a/backend/test/services/opencode-config-file.test.ts b/backend/test/services/opencode-config-file.test.ts index d90676961..e920bd92a 100644 --- a/backend/test/services/opencode-config-file.test.ts +++ b/backend/test/services/opencode-config-file.test.ts @@ -790,6 +790,177 @@ describe('opencode-config-file', () => { await expect(deleteOpenCodeConfigFile()).resolves.toBe(false) }) + describe('v1 entity replacement', () => { + it.each([ + { + label: 'agents', + v1Content: { agent: { reviewer: { description: 'Legacy reviewer' } } }, + patch: { agents: { reviewer: { description: 'New reviewer' } } }, + expected: { agents: { reviewer: { description: 'New reviewer' } } }, + }, + { + label: 'commands', + v1Content: { command: { build: { template: 'legacy template' } } }, + patch: { commands: { build: { template: 'new template' } } }, + expected: { commands: { build: { template: 'new template' } } }, + }, + { + label: 'providers', + v1Content: { provider: { example: { name: 'Legacy provider' } } }, + patch: { providers: { example: { name: 'Example provider' } } }, + expected: { providers: { example: { name: 'Example provider' } } }, + }, + { + label: 'mcp servers', + v1Content: { mcp: { linear: { type: 'remote', url: 'https://legacy.example.com' } } }, + patch: { mcp: { servers: { linear: { type: 'remote', url: 'https://current.example.com' } } } }, + expected: { mcp: { servers: { linear: { type: 'remote', url: 'https://current.example.com' } } } }, + }, + { + label: 'permissions', + v1Content: { permission: { edit: 'deny' } }, + patch: { permissions: [{ action: 'edit', resource: '*', effect: 'deny' }] }, + expected: { permissions: [{ action: 'edit', resource: '*', effect: 'deny' }] }, + }, + ])('replaces the v1 $label entry when saving its v2 entity', async ({ v1Content, patch, expected }) => { + const raw = `{\n // keep this comment\n ${JSON.stringify(v1Content).slice(1, -1)}\n}\n` + await writeFile(sourcePath('opencode.json'), raw, 'utf8') + + const updated = await updateOpenCodeConfigFile(patch, { mode: 'merge' }) + + expect(updated.content).toEqual(expected) + const onDisk = await readFile(sourcePath('opencode.json'), 'utf8') + expect(onDisk).toContain('// keep this comment') + }) + + it('keeps a comment that leads the sibling after the removed v1 container', async () => { + const raw = '{\n // lead\n "agent": {"reviewer":{"description":"Legacy reviewer"}},\n // theme comment\n "theme": "dark"\n}\n' + await writeFile(sourcePath('opencode.json'), raw, 'utf8') + + const updated = await updateOpenCodeConfigFile( + { agents: { reviewer: { description: 'New reviewer' } } }, + { mode: 'merge' }, + ) + + expect(updated.content).toEqual({ + theme: 'dark', + agents: { reviewer: { description: 'New reviewer' } }, + }) + const onDisk = await readFile(sourcePath('opencode.json'), 'utf8') + expect(onDisk).toContain('// lead') + expect(onDisk).toContain('// theme comment') + expect(onDisk).not.toContain('"agent"') + }) + + it('keeps the following sibling comment when the v1 container has no leading comment', async () => { + const raw = '{\n "agent": {"reviewer":{"description":"Legacy reviewer"}},\n // theme comment\n "theme": "dark"\n}\n' + await writeFile(sourcePath('opencode.json'), raw, 'utf8') + + await updateOpenCodeConfigFile({ agents: { reviewer: { description: 'New reviewer' } } }, { mode: 'merge' }) + + const onDisk = await readFile(sourcePath('opencode.json'), 'utf8') + expect(onDisk).toContain('// theme comment') + expect(onDisk).not.toContain('"agent"') + }) + + it('keeps a comment that leads the next sibling inside the v1 container', async () => { + const raw = [ + '{', + ' "agent": {', + ' "reviewer": { "description": "Legacy reviewer" }, // keep', + ' "keeper": { "description": "Keeper" }', + ' }', + '}', + ].join('\n') + await writeFile(sourcePath('opencode.json'), raw, 'utf8') + + const updated = await updateOpenCodeConfigFile( + { agents: { reviewer: { description: 'New reviewer' } } }, + { mode: 'merge' }, + ) + + expect(updated.content).toEqual({ + agent: { keeper: { description: 'Keeper' } }, + agents: { reviewer: { description: 'New reviewer' } }, + }) + const onDisk = await readFile(sourcePath('opencode.json'), 'utf8') + expect(onDisk).toContain('// keep') + expect(onDisk).not.toContain('Legacy reviewer') + }) + + it('removes both definitions when the v2 entity is patched with null', async () => { + const raw = JSON.stringify({ + agent: { reviewer: { description: 'Legacy reviewer' } }, + agents: { reviewer: { description: 'Current reviewer' } }, + }, null, 2) + await writeFile(sourcePath('opencode.json'), raw, 'utf8') + + const updated = await updateOpenCodeConfigFile({ agents: { reviewer: null } }, { mode: 'merge' }) + + expect(updated.content).toEqual({ agents: {} }) + const onDisk = JSON.parse(await readFile(sourcePath('opencode.json'), 'utf8')) as Record + expect(onDisk).toEqual({ agents: {} }) + expect(onDisk).not.toHaveProperty('agent') + }) + + it('keeps the v2 definition when the same entity exists in both shapes', async () => { + const raw = JSON.stringify({ + agent: { reviewer: { description: 'Legacy reviewer' } }, + agents: { reviewer: { description: 'Current reviewer' } }, + }, null, 2) + await writeFile(sourcePath('opencode.json'), raw, 'utf8') + + const updated = await updateOpenCodeConfigFile( + { agents: { reviewer: { description: 'Current reviewer' } } }, + { mode: 'merge' }, + ) + + expect(updated.content).toEqual({ agents: { reviewer: { description: 'Current reviewer' } } }) + const onDisk = await readFile(sourcePath('opencode.json'), 'utf8') + expect(JSON.parse(onDisk)).toEqual({ agents: { reviewer: { description: 'Current reviewer' } } }) + expect(onDisk).not.toContain('Legacy reviewer') + }) + + it('keeps other v1 entities in the container', async () => { + const raw = JSON.stringify({ + agent: { + reviewer: { description: 'Legacy reviewer' }, + keeper: { description: 'Keeper' }, + }, + }, null, 2) + await writeFile(sourcePath('opencode.json'), raw, 'utf8') + + const updated = await updateOpenCodeConfigFile( + { agents: { reviewer: { description: 'New reviewer' } } }, + { mode: 'merge' }, + ) + + expect(updated.content).toEqual({ + agent: { keeper: { description: 'Keeper' } }, + agents: { reviewer: { description: 'New reviewer' } }, + }) + const onDisk = JSON.parse(await readFile(sourcePath('opencode.json'), 'utf8')) as Record + expect(onDisk).toHaveProperty('agent.keeper') + }) + + it('rejects the save when the v1 entity only a lower-priority source defines', async () => { + const lower = JSON.stringify({ agent: { reviewer: { description: 'Legacy reviewer' } } }) + const target = '{"theme":"dark"}' + await writeFile(sourcePath('opencode.json'), lower, 'utf8') + await writeFile(sourcePath('opencode.jsonc'), target, 'utf8') + + const error = await updateOpenCodeConfigFile( + { agents: { reviewer: { description: 'New reviewer' } } }, + { mode: 'merge' }, + ).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(OpenCodeConfigShadowedRemovalError) + expect((error as OpenCodeConfigShadowedRemovalError).paths).toEqual(['agent.reviewer']) + expect((error as OpenCodeConfigShadowedRemovalError).sources).toEqual(['opencode.json']) + await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).resolves.toBe(target) + }) + }) + describe('restoreLegacyOpenCodeConfigBackup', () => { const backupPath = (configPath: string) => `${configPath}.ocm-sandbox-backup` From 6578e11eeb0abf253b49f1d06a275876c1e8f684 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:36:56 -0500 Subject: [PATCH 08/20] section 8: Phase 8: Managed effective config read through api.config.ge --- backend/src/routes/opencode-config.ts | 47 +++++-- backend/src/services/assistant-mode.ts | 7 +- .../src/services/opencode-config-effective.ts | 32 +++++ backend/src/services/opencode-config-file.ts | 2 +- .../src/services/opencode-config-redact.ts | 12 +- .../routes/internal-opencode-config.test.ts | 119 ++++++++++++++++-- backend/test/routes/settings.test.ts | 113 +++++++++++++++++ .../opencode-config-effective.test.ts | 94 ++++++++++++++ .../services/opencode-config-redact.test.ts | 12 +- backend/test/shared/opencode-mcp.test.ts | 13 +- shared/src/opencode/mcp.ts | 24 +--- 11 files changed, 406 insertions(+), 69 deletions(-) create mode 100644 backend/src/services/opencode-config-effective.ts create mode 100644 backend/test/services/opencode-config-effective.test.ts diff --git a/backend/src/routes/opencode-config.ts b/backend/src/routes/opencode-config.ts index 1525228cb..9f497147d 100644 --- a/backend/src/routes/opencode-config.ts +++ b/backend/src/routes/opencode-config.ts @@ -21,8 +21,10 @@ import { OpenCodeConfigSourceInvalidError, OpenCodeConfigValidationError, readOpenCodeConfigFile, + readOpenCodeConfigSnapshot, withOpenCodeConfigLock, } from '../services/opencode-config-file' +import { readManagedOpenCodeConfig } from '../services/opencode-config-effective' import { applyOpenCodeConfigUpdate, toOpenCodeConfigApplyResponse } from '../services/opencode-config-apply' import { redactOpenCodeConfigContent, redactOpenCodeConfigFile } from '../services/opencode-config-redact' import { logger } from '../utils/logger' @@ -53,6 +55,16 @@ function mergeMcpServerStatus(views: McpServerView[], status: McpStatusMap) { }) } +function toClientReadErrorResponse(c: Context, error: unknown, message: string) { + if (error instanceof ClientError) { + if (error.reason === 'Transport') { + return c.json({ error: 'OpenCode server unavailable' }, 503) + } + return c.json({ error: message }, 502) + } + return c.json({ error: message }, 500) +} + export function createOpenCodeConfigRoutes( settingsService: SettingsService, openCodeClient: OpenCodeClient, @@ -90,30 +102,41 @@ export function createOpenCodeConfigRoutes( }) } catch (error) { logger.error('Failed to get effective OpenCode config:', error) - if (error instanceof ClientError) { - if (error.reason === 'Transport') { - return c.json({ error: 'OpenCode server unavailable' }, 503) - } - return c.json({ error: 'Failed to get effective OpenCode config' }, 502) - } - return c.json({ error: 'Failed to get effective OpenCode config' }, 500) + return toClientReadErrorResponse(c, error, 'Failed to get effective OpenCode config') + } + }) + + app.get('/managed', async (c) => { + try { + const snapshot = await withOpenCodeConfigLock(readOpenCodeConfigSnapshot) + const info = await readManagedOpenCodeConfig(openCodeClient.api, snapshot.sources) + return c.json({ + revision: snapshot.revision, + info: options.redactSecrets + ? redactOpenCodeConfigContent(info as Record).content + : info, + }) + } catch (error) { + logger.error('Failed to get managed OpenCode config:', error) + return toClientReadErrorResponse(c, error, 'Failed to get managed OpenCode config') } }) app.get('/mcp', async (c) => { try { - const [config, status] = await Promise.all([ - withOpenCodeConfigLock(readOpenCodeConfigFile), + const snapshot = await withOpenCodeConfigLock(readOpenCodeConfigSnapshot) + const [info, status] = await Promise.all([ + readManagedOpenCodeConfig(openCodeClient.api, snapshot.sources), readMcpStatus(openCodeClient), ]) - const views = mcpServerViewsFromConfig(config?.content.mcp) + const views = mcpServerViewsFromConfig(info.mcp) return c.json({ - revision: config?.revision ?? null, + revision: snapshot.revision, servers: mergeMcpServerStatus(views, status), }) } catch (error) { logger.error('Failed to get OpenCode MCP servers:', error) - return c.json({ error: 'Failed to get OpenCode MCP servers' }, 500) + return toClientReadErrorResponse(c, error, 'Failed to get OpenCode MCP servers') } }) diff --git a/backend/src/services/assistant-mode.ts b/backend/src/services/assistant-mode.ts index d6195daea..cb91fc987 100644 --- a/backend/src/services/assistant-mode.ts +++ b/backend/src/services/assistant-mode.ts @@ -822,21 +822,20 @@ Returns \`400\` for invalid configuration or a \`\` value (\`paths\` l ### GET /opencode-config/mcp -List the configured MCP servers with their stored shape, enabled state, and live connection status. Use this to answer questions about MCP servers instead of reading the whole configuration. Header and environment values are never returned. +List the configured MCP servers with their enabled state and live connection status. Use this to answer questions about MCP servers instead of reading the whole configuration. Header and environment values are never returned. -\`type\` is \`local\` or \`remote\`, and \`command\` or \`url\` is included for the matching type. \`enabled\` reflects the stored flag. \`shape\` is \`servers\` for a native \`mcp.servers.\` entry or \`legacy\` for a flat \`mcp.\` entry, so you can address the entry by the path it actually uses. \`status\` is the live OpenCode status (\`connected\`, \`pending\`, \`disabled\`, \`failed\`, or \`needs_auth\`) when the server is reachable, and \`error\` carries the failure reason. +\`type\` is \`local\` or \`remote\`, and \`command\` or \`url\` is included for the matching type. \`enabled\` reflects the stored flag. Every entry comes from a native \`mcp.servers.\` entry, so address it by that path; saving \`mcp.servers.\` replaces a flat \`mcp.\` entry left by an older config. \`status\` is the live OpenCode status (\`connected\`, \`pending\`, \`disabled\`, \`failed\`, or \`needs_auth\`) when the server is reachable, and \`error\` carries the failure reason. **Response:** \`\`\`ts { - revision: string | null + revision: string servers: Array<{ name: string type: 'local' | 'remote' command?: string[] url?: string enabled: boolean - shape: 'servers' | 'legacy' status?: 'connected' | 'pending' | 'disabled' | 'failed' | 'needs_auth' error?: string }> diff --git a/backend/src/services/opencode-config-effective.ts b/backend/src/services/opencode-config-effective.ts new file mode 100644 index 000000000..1af2ccdbc --- /dev/null +++ b/backend/src/services/opencode-config-effective.ts @@ -0,0 +1,32 @@ +import path from 'path' +import { getWorkspacePath } from '@opencode-manager/shared/config/env' +import { + openCodeLocation, + type OpenCodeApi, + type OpenCodeConfigInfo, +} from '@opencode-manager/shared/opencode' +import { mergeOpenCodeConfigValues } from './opencode-config-file' + +interface ManagedConfigSource { + path: string +} + +export async function readManagedOpenCodeConfig( + api: OpenCodeApi, + sources: ReadonlyArray, +): Promise { + const entries = await api.config.get(openCodeLocation(getWorkspacePath())) + const infoByPath = new Map>() + + for (const entry of entries) { + if (entry.type !== 'document' || !entry.path) continue + infoByPath.set(path.resolve(entry.path), entry.info as Record) + } + + const merged = sources.reduce>((current, source) => { + const info = infoByPath.get(path.resolve(source.path)) + return info ? mergeOpenCodeConfigValues(current, info) : current + }, {}) + + return merged as OpenCodeConfigInfo +} diff --git a/backend/src/services/opencode-config-file.ts b/backend/src/services/opencode-config-file.ts index faaf0d3aa..49b8b44dc 100644 --- a/backend/src/services/opencode-config-file.ts +++ b/backend/src/services/opencode-config-file.ts @@ -288,7 +288,7 @@ interface MergeOpenCodeConfigValuesOptions { deleteNullValues?: boolean } -function mergeOpenCodeConfigValues( +export function mergeOpenCodeConfigValues( target: Record, source: Record, options: MergeOpenCodeConfigValuesOptions = {}, diff --git a/backend/src/services/opencode-config-redact.ts b/backend/src/services/opencode-config-redact.ts index 2b91bbdcc..7c742d42b 100644 --- a/backend/src/services/opencode-config-redact.ts +++ b/backend/src/services/opencode-config-redact.ts @@ -55,22 +55,18 @@ function isSecretKey(key: string): boolean { function keysAreNames(path: string[]): boolean { const [head] = path - if (head === 'provider') { + if (head === 'providers') { return path.length === 1 || (path.length === 3 && path[2] === 'models') } - if (head === 'agent') { + if (head === 'agents' || head === 'commands') { return path.length === 1 } if (head === 'mcp') { - return path.length === 1 || (path.length === 2 && path[1] === 'servers') + return path.length === 2 && path[1] === 'servers' } return false } -function isNameMapException(key: string): boolean { - return key === 'servers' || key === 'timeout' -} - function redactOpaqueValue(value: unknown, path: string[], redactedPaths: string[]): unknown { if (Array.isArray(value)) { return value.map((item, index) => redactOpaqueValue(item, [...path, String(index)], redactedPaths)) @@ -95,7 +91,7 @@ function redactValue(value: unknown, path: string[], redactedPaths: string[]): u return Object.fromEntries( Object.entries(value).map(([key, child]) => { const childPath = [...path, key] - if (names && !isNameMapException(key)) { + if (names) { return [key, redactValue(child, childPath, redactedPaths)] } if (OPAQUE_VALUE_KEYS.has(key)) { diff --git a/backend/test/routes/internal-opencode-config.test.ts b/backend/test/routes/internal-opencode-config.test.ts index 55a5316d0..6fe82295b 100644 --- a/backend/test/routes/internal-opencode-config.test.ts +++ b/backend/test/routes/internal-opencode-config.test.ts @@ -253,7 +253,7 @@ describe('internal/opencode-config routes', () => { expect(body.actualRevision).not.toBe(initial.revision!) }) - it('GET /api/internal/opencode-config/mcp lists configured servers with redacted config and live status', async () => { + it('GET /api/internal/opencode-config/mcp lists managed servers with live status and no shape', async () => { await writeOpenCodeConfigFile( JSON.stringify({ mcp: { @@ -265,11 +265,24 @@ describe('internal/opencode-config routes', () => { }, local: { type: 'local', command: ['npx', 'local'], disabled: true }, }, - legacy: { type: 'local', command: ['npx', 'legacy'], enabled: false }, }, }), 'opencode.jsonc', ) + configGetMock.mockImplementation(() => Promise.resolve([ + { + type: 'document', + path: configPath('opencode.jsonc'), + info: { + mcp: { + servers: { + linear: { type: 'remote', url: 'https://linear.example.com' }, + local: { type: 'local', command: ['npx', 'local'], disabled: true }, + }, + }, + }, + }, + ])) mcpListMock.mockResolvedValue({ data: [ { name: 'linear', status: { status: 'connected' } }, @@ -281,31 +294,37 @@ describe('internal/opencode-config routes', () => { expect(res.status).toBe(200) const body = await res.json() as { - revision: string | null + revision: string servers: Array<{ name: string type: string command?: string[] url?: string enabled: boolean - shape: string status?: string }> } expect(body.revision).toMatch(/^[a-f0-9]{64}$/) expect(body.servers).toEqual([ - { name: 'legacy', type: 'local', command: ['npx', 'legacy'], enabled: false, shape: 'legacy' }, - { name: 'linear', type: 'remote', url: 'https://linear.example.com', enabled: true, shape: 'servers', status: 'connected' }, - { name: 'local', type: 'local', command: ['npx', 'local'], enabled: false, shape: 'servers', status: 'disabled' }, + { name: 'linear', type: 'remote', url: 'https://linear.example.com', enabled: true, status: 'connected' }, + { name: 'local', type: 'local', command: ['npx', 'local'], enabled: false, status: 'disabled' }, ]) + expect(body.servers.every((server) => !('shape' in server))).toBe(true) expect(mcpListMock).toHaveBeenCalledWith({ location: { directory: ws.workspacePath } }) }) - it('GET /api/internal/opencode-config/mcp returns config-only when the server is unreachable', async () => { + it('GET /api/internal/opencode-config/mcp returns managed servers when the status list is unreachable', async () => { await writeOpenCodeConfigFile( JSON.stringify({ mcp: { servers: { local: { type: 'local', command: ['npx', 'local'] } } } }), 'opencode.jsonc', ) + configGetMock.mockImplementation(() => Promise.resolve([ + { + type: 'document', + path: configPath('opencode.jsonc'), + info: { mcp: { servers: { local: { type: 'local', command: ['npx', 'local'] } } } }, + }, + ])) mcpListMock.mockRejectedValue(new Error('unreachable')) const res = await app.request('/api/internal/opencode-config/mcp', { headers: authHeaders() }) @@ -313,10 +332,20 @@ describe('internal/opencode-config routes', () => { expect(res.status).toBe(200) const body = await res.json() as { servers: Array> } expect(body.servers).toEqual([ - { name: 'local', type: 'local', command: ['npx', 'local'], enabled: true, shape: 'servers' }, + { name: 'local', type: 'local', command: ['npx', 'local'], enabled: true }, ]) }) + it('GET /api/internal/opencode-config/mcp returns 503 when OpenCode is unreachable', async () => { + configGetMock.mockImplementation(() => Promise.reject(new ClientError('Transport'))) + + const res = await app.request('/api/internal/opencode-config/mcp', { headers: authHeaders() }) + + expect(res.status).toBe(503) + const body = await res.json() as { error: string } + expect(body.error).toBe('OpenCode server unavailable') + }) + it('PUT /api/internal/opencode-config writes the file and applies it through a location reload without a restart', async () => { await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED, 'opencode.jsonc') @@ -526,4 +555,76 @@ describe('internal/opencode-config routes', () => { const body = await res.json() as { error: string } expect(body.error).toBe('Failed to get effective OpenCode config') }) + + it('GET /api/internal/opencode-config/managed merges only Manager source documents and redacts secrets', async () => { + await writeOpenCodeConfigFile(JSON.stringify({ theme: 'light' }), 'opencode.json') + const snapshot = await writeOpenCodeConfigFile(JSON.stringify({ theme: 'dark' }), 'opencode.jsonc') + configGetMock.mockImplementation(() => Promise.resolve([ + { + type: 'document', + path: configPath('opencode.json'), + info: { theme: 'light', providers: { example: { apiKey: 'secret-key' } } }, + }, + { + type: 'document', + path: configPath('opencode.jsonc'), + info: { + theme: 'dark', + mcp: { servers: { local: { type: 'local', command: ['npx', 'local'] } } }, + }, + }, + { + type: 'document', + path: path.join(ws.workspacePath, 'unmanaged', 'opencode.json'), + info: { theme: 'ignored' }, + }, + { type: 'directory', path: path.join(ws.workspacePath, '.config', 'opencode') }, + ])) + + const res = await app.request('/api/internal/opencode-config/managed', { headers: authHeaders() }) + + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ + revision: snapshot.revision, + info: { + theme: 'dark', + providers: { example: { apiKey: '' } }, + mcp: { servers: { local: { type: 'local', command: ['npx', 'local'] } } }, + }, + }) + expect(configGetMock).toHaveBeenCalledWith({ location: { directory: ws.workspacePath } }) + }) + + it('GET /api/internal/opencode-config/managed reports a v1-only file through the converted entries the client returns', async () => { + await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED, 'opencode.json') + await writeFile(configPath('opencode.json'), JSON.stringify({ + agent: { reviewer: { mode: 'subagent' } }, + }), 'utf8') + configGetMock.mockImplementation(() => Promise.resolve([ + { + type: 'document', + path: configPath('opencode.json'), + info: { agents: { reviewer: { mode: 'subagent' } } }, + }, + ])) + + const res = await app.request('/api/internal/opencode-config/managed', { headers: authHeaders() }) + + expect(res.status).toBe(200) + const body = await res.json() as { info: Record } + expect(body.info).toEqual({ agents: { reviewer: { mode: 'subagent' } } }) + const persisted = JSON.parse(await readFile(configPath('opencode.json'), 'utf8')) as Record + expect(persisted.agent).toEqual({ reviewer: { mode: 'subagent' } }) + expect((await readOpenCodeConfigFile())?.path).toBe(configPath('opencode.json')) + }) + + it('GET /api/internal/opencode-config/managed returns 503 when OpenCode is unreachable', async () => { + configGetMock.mockImplementation(() => Promise.reject(new ClientError('Transport'))) + + const res = await app.request('/api/internal/opencode-config/managed', { headers: authHeaders() }) + + expect(res.status).toBe(503) + const body = await res.json() as { error: string } + expect(body.error).toBe('OpenCode server unavailable') + }) }) diff --git a/backend/test/routes/settings.test.ts b/backend/test/routes/settings.test.ts index 649c19295..19451e376 100644 --- a/backend/test/routes/settings.test.ts +++ b/backend/test/routes/settings.test.ts @@ -12,12 +12,14 @@ const mockResetSettings = vi.fn() const mockGetLastKnownGoodConfig = vi.fn() const { mockReadOpenCodeConfigFile, + mockReadOpenCodeConfigSnapshot, mockDeleteOpenCodeConfigFile, mockArchiveBrokenOpenCodeConfigFile, mockRestoreOpenCodeConfigSnapshot, mockApplyOpenCodeConfigUpdate, } = vi.hoisted(() => ({ mockReadOpenCodeConfigFile: vi.fn(), + mockReadOpenCodeConfigSnapshot: vi.fn(), mockDeleteOpenCodeConfigFile: vi.fn(), mockArchiveBrokenOpenCodeConfigFile: vi.fn(), mockRestoreOpenCodeConfigSnapshot: vi.fn(), @@ -183,6 +185,7 @@ vi.mock('../../src/services/opencode-config-file', async (importOriginal) => { return { ...actual, readOpenCodeConfigFile: mockReadOpenCodeConfigFile, + readOpenCodeConfigSnapshot: mockReadOpenCodeConfigSnapshot, deleteOpenCodeConfigFile: mockDeleteOpenCodeConfigFile, archiveBrokenOpenCodeConfigFile: mockArchiveBrokenOpenCodeConfigFile, restoreOpenCodeConfigSnapshot: mockRestoreOpenCodeConfigSnapshot, @@ -327,6 +330,7 @@ import { detectSandboxCapability } from '../../src/services/sandbox/capability' import { forceProcessAttestation } from '../../src/services/opencode/process-identity' import { sseAggregator } from '../../src/services/sse-aggregator' import { createRepo } from '../../src/db/queries' +import { ClientError } from '@opencode-manager/shared/opencode' import type { OpenCodeClient } from '../../src/services/opencode/client' const mockSpawnSync = spawnSync as ReturnType @@ -363,6 +367,7 @@ describe('Settings Routes - OpenCode Upgrade', () => { mockGetImportedSessionDirectories.mockReset() mockRelinkReposFromSessionDirectories.mockReset() mockReadOpenCodeConfigFile.mockReset() + mockReadOpenCodeConfigSnapshot.mockReset() mockDeleteOpenCodeConfigFile.mockReset() mockArchiveBrokenOpenCodeConfigFile.mockReset() mockRestoreOpenCodeConfigSnapshot.mockReset() @@ -443,6 +448,114 @@ describe('Settings Routes - OpenCode Upgrade', () => { expect(json.error).toBe('No OpenCode config file found') }) + it('returns the managed config merged from Manager source documents', async () => { + mockReadOpenCodeConfigSnapshot.mockResolvedValueOnce({ + sources: [ + { name: 'opencode.json', path: '/tmp/test-workspace/config/opencode.json', rawContent: '{}', content: {}, isValid: true, updatedAt: 1 }, + { name: 'opencode.jsonc', path: '/tmp/test-workspace/config/opencode.jsonc', rawContent: '{}', content: {}, isValid: true, updatedAt: 1 }, + ], + content: {}, + revision: 'rev-managed', + updatedAt: 1, + path: '/tmp/test-workspace/config/opencode.jsonc', + rawContent: '{}', + }) + const configGet = vi.fn(async () => [ + { type: 'document', path: '/tmp/test-workspace/config/opencode.json', info: { theme: 'light', model: 'a/b' } }, + { type: 'document', path: '/tmp/test-workspace/config/opencode.jsonc', info: { theme: 'dark' } }, + { type: 'document', path: '/tmp/test-workspace/unmanaged.json', info: { theme: 'ignored' } }, + { type: 'directory', path: '/tmp/test-workspace' }, + ]) + const app = createSettingsRoutes( + testDb, + { getGitEnvironment: vi.fn().mockReturnValue({}) } as any, + createStubOpenCodeClient({ api: { config: { get: configGet } } as unknown as OpenCodeClient['api'] }), + ) + + const res = await app.fetch(new Request('http://localhost/opencode-config/managed')) + + expect(res.status).toBe(200) + expect(await res.json()).toEqual({ + revision: 'rev-managed', + info: { theme: 'dark', model: 'a/b' }, + }) + expect(configGet).toHaveBeenCalledWith({ location: { directory: '/tmp/test-workspace' } }) + }) + + it('returns 503 from GET /opencode-config/managed when OpenCode is unavailable', async () => { + mockReadOpenCodeConfigSnapshot.mockResolvedValueOnce({ + sources: [ + { name: 'opencode.jsonc', path: '/tmp/test-workspace/config/opencode.jsonc', rawContent: '{}', content: {}, isValid: true, updatedAt: 1 }, + ], + content: {}, + revision: 'rev-managed', + updatedAt: 1, + path: '/tmp/test-workspace/config/opencode.jsonc', + rawContent: '{}', + }) + const app = createSettingsRoutes( + testDb, + { getGitEnvironment: vi.fn().mockReturnValue({}) } as any, + createStubOpenCodeClient({ + api: { + config: { + get: vi.fn(async () => { + throw new ClientError('Transport') + }), + }, + } as unknown as OpenCodeClient['api'], + }), + ) + + const res = await app.fetch(new Request('http://localhost/opencode-config/managed')) + const json = await res.json() as Record + + expect(res.status).toBe(503) + expect(json.error).toBe('OpenCode server unavailable') + }) + + it('returns MCP servers from the managed config without a shape field', async () => { + mockReadOpenCodeConfigSnapshot.mockResolvedValueOnce({ + sources: [ + { name: 'opencode.jsonc', path: '/tmp/test-workspace/config/opencode.jsonc', rawContent: '{}', content: {}, isValid: true, updatedAt: 1 }, + ], + content: {}, + revision: 'rev-managed', + updatedAt: 1, + path: '/tmp/test-workspace/config/opencode.jsonc', + rawContent: '{}', + }) + const configGet = vi.fn(async () => [ + { + type: 'document', + path: '/tmp/test-workspace/config/opencode.jsonc', + info: { mcp: { servers: { local: { type: 'local', command: ['npx', 'local'] } } } }, + }, + ]) + const mcpList = vi.fn(async () => ({ + data: [{ name: 'local', status: { status: 'connected' } }], + })) + const app = createSettingsRoutes( + testDb, + { getGitEnvironment: vi.fn().mockReturnValue({}) } as any, + createStubOpenCodeClient({ + api: { config: { get: configGet }, mcp: { list: mcpList } } as unknown as OpenCodeClient['api'], + }), + ) + + const res = await app.fetch(new Request('http://localhost/opencode-config/mcp')) + const json = await res.json() as { revision: string; servers: Array> } + + expect(res.status).toBe(200) + expect(json).toEqual({ + revision: 'rev-managed', + servers: [ + { name: 'local', type: 'local', command: ['npx', 'local'], enabled: true, status: 'connected' }, + ], + }) + expect(json.servers.every((server) => !('shape' in server))).toBe(true) + }) + it('maps a restart-pending apply result to 200 with restartRequired', async () => { const config = { path: '/tmp/test-workspace/.config/opencode.json', diff --git a/backend/test/services/opencode-config-effective.test.ts b/backend/test/services/opencode-config-effective.test.ts new file mode 100644 index 000000000..049cae891 --- /dev/null +++ b/backend/test/services/opencode-config-effective.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it, vi } from 'vitest' +import { getWorkspacePath } from '@opencode-manager/shared/config/env' +import { ClientError, openCodeLocation, type OpenCodeApi } from '@opencode-manager/shared/opencode' +import { readManagedOpenCodeConfig } from '../../src/services/opencode-config-effective' + +function stubApi(entries: unknown[]) { + const get = vi.fn(async () => entries) + return { api: { config: { get } } as unknown as OpenCodeApi, get } +} + +describe('readManagedOpenCodeConfig', () => { + it('merges only the documents whose path is a Manager source, in source order', async () => { + const { api, get } = stubApi([ + { + type: 'document', + path: '/workspace/config/opencode.json', + info: { theme: 'light', mcp: { servers: { local: { type: 'local', command: ['npx', 'local'] } } } }, + }, + { + type: 'document', + path: '/workspace/config/opencode.jsonc', + info: { theme: 'dark', mcp: { servers: { remote: { type: 'remote', url: 'https://remote.example.com' } } } }, + }, + { type: 'document', path: '/workspace/unmanaged/opencode.json', info: { theme: 'ignored' } }, + { type: 'document', info: { theme: 'pathless' } }, + { type: 'directory', path: '/workspace/config' }, + ]) + + const info = await readManagedOpenCodeConfig(api, [ + { path: '/workspace/config/opencode.json' }, + { path: '/workspace/config/opencode.jsonc' }, + ]) + + expect(info).toEqual({ + theme: 'dark', + mcp: { + servers: { + local: { type: 'local', command: ['npx', 'local'] }, + remote: { type: 'remote', url: 'https://remote.example.com' }, + }, + }, + }) + expect(get).toHaveBeenCalledWith(openCodeLocation(getWorkspacePath())) + }) + + it('resolves both the source and reported path before matching', async () => { + const { api } = stubApi([ + { + type: 'document', + path: '/workspace/config/./opencode.json', + info: { theme: 'dark' }, + }, + ]) + + await expect( + readManagedOpenCodeConfig(api, [{ path: '/workspace/config/opencode.json' }]), + ).resolves.toEqual({ theme: 'dark' }) + }) + + it('returns the converted v1 entries the server reports for a Manager file', async () => { + const { api } = stubApi([ + { + type: 'document', + path: '/workspace/config/opencode.json', + info: { agents: { reviewer: { mode: 'subagent' } } }, + }, + ]) + + await expect( + readManagedOpenCodeConfig(api, [{ path: '/workspace/config/opencode.json' }]), + ).resolves.toEqual({ agents: { reviewer: { mode: 'subagent' } } }) + }) + + it('returns an empty config when no reported document matches a Manager source', async () => { + const { api } = stubApi([ + { type: 'document', path: '/workspace/other/opencode.json', info: { theme: 'dark' } }, + ]) + + await expect( + readManagedOpenCodeConfig(api, [{ path: '/workspace/config/opencode.json' }]), + ).resolves.toEqual({}) + }) + + it('propagates client errors', async () => { + const get = vi.fn(async () => { + throw new ClientError('Transport') + }) + const api = { config: { get } } as unknown as OpenCodeApi + + await expect( + readManagedOpenCodeConfig(api, [{ path: '/workspace/config/opencode.json' }]), + ).rejects.toBeInstanceOf(ClientError) + }) +}) diff --git a/backend/test/services/opencode-config-redact.test.ts b/backend/test/services/opencode-config-redact.test.ts index 80234ff40..bb25eb590 100644 --- a/backend/test/services/opencode-config-redact.test.ts +++ b/backend/test/services/opencode-config-redact.test.ts @@ -96,7 +96,7 @@ describe('redactOpenCodeConfigFile', () => { expect(redacted.redactedPaths).toEqual([]) }) - it('does not mask server, provider, or model names that look like secret keys', () => { + it('does not mask server, provider, agent, command, or model names that look like secret keys', () => { const redacted = redactOpenCodeConfigFile( buildConfig({ mcp: { @@ -108,12 +108,14 @@ describe('redactOpenCodeConfigFile', () => { }, }, }, - provider: { + providers: { 'secret-provider': { apiKey: 'provider-secret', models: { 'token-model': { name: 'gpt' } }, }, }, + agents: { 'token-agent': { model: 'openai/gpt' } }, + commands: { 'secret-command': { template: 'hello' } }, }), ) @@ -127,16 +129,18 @@ describe('redactOpenCodeConfigFile', () => { }, }, }, - provider: { + providers: { 'secret-provider': { apiKey: '', models: { 'token-model': { name: 'gpt' } }, }, }, + agents: { 'token-agent': { model: 'openai/gpt' } }, + commands: { 'secret-command': { template: 'hello' } }, }) expect(redacted.redactedPaths).toEqual([ 'mcp.servers.github-token.headers.Authorization', - 'provider.secret-provider.apiKey', + 'providers.secret-provider.apiKey', ]) }) diff --git a/backend/test/shared/opencode-mcp.test.ts b/backend/test/shared/opencode-mcp.test.ts index b6ad03a30..b3f3138a9 100644 --- a/backend/test/shared/opencode-mcp.test.ts +++ b/backend/test/shared/opencode-mcp.test.ts @@ -49,37 +49,28 @@ describe('mcpServersFromConfig', () => { }) describe('mcpServerViewsFromConfig', () => { - it('normalizes native and legacy servers, reporting each shape and enabled state', () => { + it('normalizes native servers and their enabled state', () => { expect( mcpServerViewsFromConfig({ timeout: { catalog: 5000 }, - legacy: { type: 'local', command: ['npx', 'legacy'], enabled: false }, + flat: { type: 'local', command: ['npx', 'flat'], enabled: false }, servers: { remote: { type: 'remote', url: 'https://native.example.com', disabled: true }, local: { type: 'local', command: ['npx', 'native'] }, }, }), ).toEqual([ - { - name: 'legacy', - type: 'local', - command: ['npx', 'legacy'], - enabled: false, - shape: 'legacy', - }, { name: 'local', type: 'local', command: ['npx', 'native'], enabled: true, - shape: 'servers', }, { name: 'remote', type: 'remote', url: 'https://native.example.com', enabled: false, - shape: 'servers', }, ]) }) diff --git a/shared/src/opencode/mcp.ts b/shared/src/opencode/mcp.ts index 9f14c2c74..df8a5cee6 100644 --- a/shared/src/opencode/mcp.ts +++ b/shared/src/opencode/mcp.ts @@ -27,43 +27,27 @@ export function mcpServersFromConfig(mcp: unknown): Record isMcpServerConfig(entry[1]))) } -export type McpServerShape = 'servers' | 'legacy' - export interface McpServerView { name: string type: 'local' | 'remote' command?: string[] url?: string enabled: boolean - shape: McpServerShape } -function toMcpServerView( - name: string, - config: McpServerConfig, - enabled: boolean, - shape: McpServerShape, -): McpServerView { +function toMcpServerView(name: string, config: McpServerConfig, enabled: boolean): McpServerView { return { name, type: config.type, ...(config.type === 'local' ? { command: config.command } : { url: config.url }), enabled, - shape, } } export function mcpServerViewsFromConfig(mcp: unknown): McpServerView[] { - if (!isRecord(mcp)) return [] - const nativeViews = Object.entries(mcpServersFromConfig(mcp)).map(([name, config]) => - toMcpServerView(name, config, config.disabled !== true, 'servers'), - ) - const legacyViews = Object.entries(mcp).flatMap(([name, value]) => { - if (name === 'servers' || name === 'timeout' || !isMcpServerConfig(value)) return [] - const enabled = (value as { enabled?: unknown }).enabled !== false - return [toMcpServerView(name, value, enabled, 'legacy')] - }) - return [...nativeViews, ...legacyViews].sort((left, right) => left.name.localeCompare(right.name)) + return Object.entries(mcpServersFromConfig(mcp)) + .map(([name, config]) => toMcpServerView(name, config, config.disabled !== true)) + .sort((left, right) => left.name.localeCompare(right.name)) } export function mcpStatusByName(servers: McpServer[]): McpStatusMap { From 81725fb34568837655f35c018372011af7ae9144 Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:47:37 -0500 Subject: [PATCH 09/20] section 9: Phase 9: Settings agents and commands read managed v2 config --- frontend/src/api/settings.ts | 16 ++ frontend/src/api/types/settings.ts | 32 +++ .../components/settings/AgentDialog.test.tsx | 90 ++++++++- .../src/components/settings/AgentDialog.tsx | 191 ++++++------------ .../components/settings/AgentsEditor.test.tsx | 34 ++-- .../src/components/settings/AgentsEditor.tsx | 63 ++---- .../src/components/settings/CommandDialog.tsx | 93 ++++----- .../settings/CommandsEditor.test.tsx | 38 ++-- .../components/settings/CommandsEditor.tsx | 52 ++--- .../settings/OpenCodeConfigManager.test.tsx | 164 +++++++++++++++ .../settings/OpenCodeConfigManager.tsx | 120 ++++++----- .../src/hooks/useOpenCodeManagedConfig.ts | 11 + frontend/src/lib/queryInvalidation.ts | 3 + 13 files changed, 534 insertions(+), 373 deletions(-) create mode 100644 frontend/src/hooks/useOpenCodeManagedConfig.ts diff --git a/frontend/src/api/settings.ts b/frontend/src/api/settings.ts index 6357bf568..c7ec8428a 100644 --- a/frontend/src/api/settings.ts +++ b/frontend/src/api/settings.ts @@ -14,6 +14,8 @@ import type { InstallSkillResponse, OpenCodeDirectoryFileInfo, OpenCodeRestartResponse, + ManagedOpenCodeConfig, + UpdateOpenCodeConfigPatchRequest, } from './types/settings' import { API_BASE_URL } from '@/config' import { fetchWrapper } from './fetchWrapper' @@ -73,6 +75,20 @@ export const settingsApi = { }) }, + getManagedOpenCodeConfig: async (): Promise => { + return fetchWrapper(`${API_BASE_URL}/api/settings/opencode-config/managed`) + }, + + patchOpenCodeConfig: async ( + request: UpdateOpenCodeConfigPatchRequest + ): Promise => { + return fetchWrapper(`${API_BASE_URL}/api/settings/opencode-config`, { + method: 'PATCH', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(request), + }) + }, + discoverOpenCodeModels: async ( baseUrl: string, apiKey?: string, diff --git a/frontend/src/api/types/settings.ts b/frontend/src/api/types/settings.ts index dd20feaa8..941286635 100644 --- a/frontend/src/api/types/settings.ts +++ b/frontend/src/api/types/settings.ts @@ -26,6 +26,11 @@ import { type InstallSkillResponse, type SettingsResponse as SharedSettingsResponse, } from '@opencode-manager/shared' +import type { OpenCodeConfigInfo } from '@opencode-manager/shared/opencode' +import type { + OpenCodeAgentConfig, + OpenCodeCommandConfig, +} from '@opencode-manager/shared/opencode' import type { NotificationPreferences } from '@opencode-manager/shared/types' import { saveFile } from '@/lib/download' @@ -90,3 +95,30 @@ export interface OpenCodeDirectoryFileInfo { name: string relativePath: string } + +export interface ManagedOpenCodeConfig { + revision: string | null + info: OpenCodeConfigInfo +} + +/** + * Merge-patch value for one agent: an explicit null clears the stored field, + * while an omitted field stays untouched. + */ +export type OpenCodeAgentPatch = { + [K in keyof OpenCodeAgentConfig]?: OpenCodeAgentConfig[K] | null +} + +/** + * Merge-patch value for one command: an explicit null clears the stored field, + * while an omitted field stays untouched. + */ +export type OpenCodeCommandPatch = { + [K in keyof OpenCodeCommandConfig]?: OpenCodeCommandConfig[K] | null +} + +export interface UpdateOpenCodeConfigPatchRequest { + patch: Record + source?: OpenCodeConfigSourceName + expectedRevision?: string +} diff --git a/frontend/src/components/settings/AgentDialog.test.tsx b/frontend/src/components/settings/AgentDialog.test.tsx index 0a8ae007d..d3fb89c93 100644 --- a/frontend/src/components/settings/AgentDialog.test.tsx +++ b/frontend/src/components/settings/AgentDialog.test.tsx @@ -43,7 +43,7 @@ describe('AgentDialog model field', () => { open onOpenChange={vi.fn()} onSubmit={vi.fn()} - editingAgent={{ name: 'reviewer', agent: { prompt: 'Review code', model: 'openai/gpt-4o' } }} + editingAgent={{ name: 'reviewer', agent: { system: 'Review code', model: 'openai/gpt-4o' } }} />, ) @@ -58,7 +58,7 @@ describe('AgentDialog model field', () => { open onOpenChange={vi.fn()} onSubmit={onSubmit} - editingAgent={{ name: 'reviewer', agent: { prompt: 'Review code', model: 'openai/gpt-4o#high' } }} + editingAgent={{ name: 'reviewer', agent: { system: 'Review code', model: 'openai/gpt-4o#high' } }} />, ) @@ -80,7 +80,7 @@ describe('AgentDialog model field', () => { onSubmit={onSubmit} editingAgent={{ name: 'reviewer', - agent: { prompt: 'Review code', model: { providerID: 'openai', model: 'gpt-4o', variant: 'high' } }, + agent: { system: 'Review code', model: { providerID: 'openai', model: 'gpt-4o', variant: 'high' } }, }} />, ) @@ -95,7 +95,7 @@ describe('AgentDialog model field', () => { ) }) - it('submits no model when the field is empty', async () => { + it('submits null for a cleared model', async () => { const onSubmit = vi.fn() const user = userEvent.setup() render( @@ -103,14 +103,39 @@ describe('AgentDialog model field', () => { open onOpenChange={vi.fn()} onSubmit={onSubmit} - editingAgent={{ name: 'reviewer', agent: { prompt: 'Review code' } }} + editingAgent={{ name: 'reviewer', agent: { system: 'Review code' } }} />, ) await user.click(screen.getByRole('button', { name: 'Update' })) const agent = onSubmit.mock.calls[0][1] - expect(agent.model).toBeUndefined() + expect(agent.model).toBeNull() + }) + + it('sends null for cleared description and model so the merge patch deletes them', async () => { + const onSubmit = vi.fn() + const user = userEvent.setup() + render( + , + ) + + await user.clear(screen.getByLabelText('Description')) + await user.clear(screen.getByLabelText('Model')) + await user.click(screen.getByRole('button', { name: 'Update' })) + + expect(onSubmit).toHaveBeenCalledWith( + 'reviewer', + expect.objectContaining({ description: null, model: null }), + ) }) it('saves the OpenCode key of an aliased model on create', async () => { @@ -129,4 +154,57 @@ describe('AgentDialog model field', () => { expect.objectContaining({ model: 'anthropic/claude-opus-5-5-fast' }), ) }) + + it('maps v2 request body values and permission rules into the form and preserves unrelated fields on save', async () => { + const onSubmit = vi.fn() + const user = userEvent.setup() + render( + , + ) + + expect(screen.getByLabelText('Temperature')).toHaveValue(0.2) + expect(screen.getByLabelText('Top P')).toHaveValue(0.5) + expect(screen.getByText('Deny')).toBeInTheDocument() + + await user.click(screen.getByRole('button', { name: 'Update' })) + + const agent = onSubmit.mock.calls[0][1] + expect(agent.request).toEqual({ + headers: { 'x-test': '1' }, + body: { temperature: 0.2, top_p: 0.5, reasoning_effort: 'high' }, + }) + expect(agent.permissions).toEqual([ + { action: 'edit', resource: 'src/**', effect: 'ask' }, + { action: 'edit', resource: '*', effect: 'allow' }, + { action: 'shell', resource: '*', effect: 'deny' }, + { action: 'webfetch', resource: '*', effect: 'allow' }, + ]) + expect(agent.hidden).toBe(true) + expect(agent.color).toBe('#fff') + expect(agent.steps).toBe(7) + }) }) diff --git a/frontend/src/components/settings/AgentDialog.tsx b/frontend/src/components/settings/AgentDialog.tsx index e8772c245..54ced3944 100644 --- a/frontend/src/components/settings/AgentDialog.tsx +++ b/frontend/src/components/settings/AgentDialog.tsx @@ -11,6 +11,27 @@ import { Switch } from '@/components/ui/switch' import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select' import { ModelCombobox } from '@/components/model/ModelCombobox' import { configModelRef, formatOpenCodeModelRef } from '@opencode-manager/shared/opencode' +import type { OpenCodeAgentConfig, OpenCodePermissionRule } from '@opencode-manager/shared/opencode' +import type { OpenCodeAgentPatch } from '@/api/types/settings' + +const MANAGED_PERMISSION_ACTIONS = ['edit', 'shell', 'webfetch'] as const + +type ManagedPermissionAction = typeof MANAGED_PERMISSION_ACTIONS[number] + +type PermissionEffect = OpenCodePermissionRule['effect'] + +const permissionEffect = ( + permissions: OpenCodePermissionRule[] | undefined, + action: ManagedPermissionAction, +): PermissionEffect => { + const rule = permissions?.find((entry) => entry.action === action && entry.resource === '*') + return rule?.effect ?? 'allow' +} + +const requestBodyNumber = (agent: OpenCodeAgentConfig | undefined, key: string): number | undefined => { + const value = agent?.request?.body?.[key] + return typeof value === 'number' ? value : undefined +} const agentFormSchema = z.object({ name: z.string().min(1, 'Agent name is required').regex(/^[a-z0-9-]+$/, 'Must be lowercase letters, numbers, and hyphens only'), @@ -20,70 +41,42 @@ const agentFormSchema = z.object({ temperature: z.number().min(0).max(2), topP: z.number().min(0).max(1), model: z.string().optional(), - write: z.boolean(), - edit: z.boolean(), - bash: z.boolean(), - webfetch: z.boolean(), editPermission: z.enum(['ask', 'allow', 'deny']), - bashPermission: z.enum(['ask', 'allow', 'deny']), + shellPermission: z.enum(['ask', 'allow', 'deny']), webfetchPermission: z.enum(['ask', 'allow', 'deny']), - disable: z.boolean() + disable: z.boolean(), }) type AgentFormValues = z.infer -interface Agent { - prompt?: string - description?: string - mode?: 'subagent' | 'primary' | 'all' - temperature?: number - topP?: number - top_p?: number - model?: TModel - tools?: Record - permission?: { - edit?: 'ask' | 'allow' | 'deny' - bash?: 'ask' | 'allow' | 'deny' | Record - webfetch?: 'ask' | 'allow' | 'deny' - } - disable?: boolean - [key: string]: unknown -} - -type StoredAgent = Agent - interface AgentDialogProps { open: boolean onOpenChange: (open: boolean) => void - onSubmit: (name: string, agent: Agent) => void - editingAgent?: { name: string; agent: StoredAgent } | null + onSubmit: (name: string, agent: OpenCodeAgentPatch) => void + editingAgent?: { name: string; agent: OpenCodeAgentConfig } | null } export function AgentDialog({ open, onOpenChange, onSubmit, editingAgent }: AgentDialogProps) { - const getDefaultValues = (agent?: { name: string; agent: StoredAgent } | null): AgentFormValues => { + const getDefaultValues = (agent?: { name: string; agent: OpenCodeAgentConfig } | null): AgentFormValues => { const modelRef = configModelRef(agent?.agent.model) return { name: agent?.name || '', description: agent?.agent.description || '', - prompt: agent?.agent.prompt || '', + prompt: agent?.agent.system || '', mode: agent?.agent.mode || 'subagent', - temperature: agent?.agent.temperature ?? 0.7, - topP: agent?.agent.topP ?? agent?.agent.top_p ?? 1, + temperature: requestBodyNumber(agent?.agent, 'temperature') ?? 0.7, + topP: requestBodyNumber(agent?.agent, 'top_p') ?? 1, model: modelRef ? formatOpenCodeModelRef(modelRef) : '', - write: agent?.agent.tools?.write ?? true, - edit: agent?.agent.tools?.edit ?? true, - bash: agent?.agent.tools?.bash ?? true, - webfetch: agent?.agent.tools?.webfetch ?? true, - editPermission: agent?.agent.permission?.edit ?? 'allow', - bashPermission: typeof agent?.agent.permission?.bash === 'string' ? agent.agent.permission.bash : 'allow', - webfetchPermission: agent?.agent.permission?.webfetch ?? 'allow', - disable: agent?.agent.disable ?? false + editPermission: permissionEffect(agent?.agent.permissions, 'edit'), + shellPermission: permissionEffect(agent?.agent.permissions, 'shell'), + webfetchPermission: permissionEffect(agent?.agent.permissions, 'webfetch'), + disable: agent?.agent.disabled ?? false, } } const form = useForm({ resolver: zodResolver(agentFormSchema), - defaultValues: getDefaultValues(editingAgent) + defaultValues: getDefaultValues(editingAgent), }) useEffect(() => { @@ -93,28 +86,31 @@ export function AgentDialog({ open, onOpenChange, onSubmit, editingAgent }: Agen }, [open, editingAgent, form]) const handleSubmit = (values: AgentFormValues) => { - const agent: Agent = { - prompt: values.prompt, - description: values.description || undefined, + const previous = editingAgent?.agent + const preservedPermissions = (previous?.permissions ?? []).filter( + (rule) => rule.resource !== '*' || !MANAGED_PERMISSION_ACTIONS.includes(rule.action as ManagedPermissionAction), + ) + const agent: OpenCodeAgentPatch = { + ...previous, + system: values.prompt, + description: values.description || null, mode: values.mode, - temperature: values.temperature, - topP: values.topP, - disable: values.disable, - tools: { - write: values.write, - edit: values.edit, - bash: values.bash, - webfetch: values.webfetch + model: values.model?.trim() || null, + disabled: values.disable, + permissions: [ + ...preservedPermissions, + { action: 'edit', resource: '*', effect: values.editPermission }, + { action: 'shell', resource: '*', effect: values.shellPermission }, + { action: 'webfetch', resource: '*', effect: values.webfetchPermission }, + ], + request: { + ...previous?.request, + body: { + ...previous?.request?.body, + temperature: values.temperature, + top_p: values.topP, + }, }, - permission: { - edit: values.editPermission, - bash: values.bashPermission, - webfetch: values.webfetchPermission - } - } - - if (values.model?.trim()) { - agent.model = values.model.trim() } onSubmit(values.name, agent) @@ -287,75 +283,6 @@ export function AgentDialog({ open, onOpenChange, onSubmit, editingAgent }: Agen )} /> -
-
Tools Configuration
-
- ( - - - - - Write - - )} - /> - - ( - - - - - Edit - - )} - /> - - ( - - - - - Bash - - )} - /> - - ( - - - - - Web Fetch - - )} - /> -
-
-
Permissions
@@ -384,10 +311,10 @@ export function AgentDialog({ open, onOpenChange, onSubmit, editingAgent }: Agen ( - Bash + Shell - field.onChange(parseFloat(e.target.value)) - } - /> - - - - )} - /> (
- Run as subtask + Run as subagent - Execute this command as a separate subtask + Execute this command in a subagent session
@@ -262,4 +246,3 @@ export function CommandDialog({ ); } - diff --git a/frontend/src/components/settings/CommandsEditor.test.tsx b/frontend/src/components/settings/CommandsEditor.test.tsx index 2b0b9e9bb..fb2a8eca1 100644 --- a/frontend/src/components/settings/CommandsEditor.test.tsx +++ b/frontend/src/components/settings/CommandsEditor.test.tsx @@ -54,16 +54,14 @@ describe('CommandsEditor', () => { }) it('renders empty state when no commands configured', () => { - const onChange = vi.fn() - render(, { wrapper: createWrapper() }) + render(, { wrapper: createWrapper() }) expect(screen.getByText('No commands configured')).toBeInTheDocument() expect(screen.getByText('Add your first command to get started.')).toBeInTheDocument() }) it('renders command names without a leading slash', () => { - const onChange = vi.fn() - render(, { wrapper: createWrapper() }) + render(, { wrapper: createWrapper() }) expect(screen.getByText('review')).toBeInTheDocument() expect(screen.getByText('build')).toBeInTheDocument() @@ -71,12 +69,12 @@ describe('CommandsEditor', () => { }) it('renders uploaded directory command files', () => { - const onChange = vi.fn() render( , { wrapper: createWrapper() }, ) @@ -89,8 +87,7 @@ describe('CommandsEditor', () => { it('opens CommandDialog when clicking Edit on a row', async () => { const user = userEvent.setup() - const onChange = vi.fn() - render(, { wrapper: createWrapper() }) + render(, { wrapper: createWrapper() }) const editButtons = screen.getAllByText('Edit') await user.click(editButtons[0]) @@ -101,8 +98,7 @@ describe('CommandsEditor', () => { it('opens CommandDialog when clicking row body', async () => { const user = userEvent.setup() - const onChange = vi.fn() - render(, { wrapper: createWrapper() }) + render(, { wrapper: createWrapper() }) await user.click(screen.getByText('review')) @@ -111,8 +107,7 @@ describe('CommandsEditor', () => { it('opens CommandDialog and displays Create Command text when clicking Add Command button', async () => { const user = userEvent.setup() - const onChange = vi.fn() - render(, { wrapper: createWrapper() }) + render(, { wrapper: createWrapper() }) await user.click(screen.getByText('Add Command')) @@ -120,33 +115,31 @@ describe('CommandsEditor', () => { expect(screen.getByText('Create Command')).toBeInTheDocument() }) - it('calls onChange with command removed when Delete is clicked from overflow menu', async () => { + it('calls onDelete with the command name when Delete is clicked from overflow menu', async () => { const user = userEvent.setup() - const onChange = vi.fn() - render(, { wrapper: createWrapper() }) + const onDelete = vi.fn() + render(, { wrapper: createWrapper() }) await user.click(screen.getByLabelText('Actions for review')) await user.click(screen.getByText('Delete')) - expect(onChange).toHaveBeenCalledTimes(1) - expect(onChange).toHaveBeenCalledWith({ build: mockCommands.build }) + expect(onDelete).toHaveBeenCalledTimes(1) + expect(onDelete).toHaveBeenCalledWith('review') }) it('renders agent badge when command has agent', () => { - const onChange = vi.fn() - render(, { wrapper: createWrapper() }) + render(, { wrapper: createWrapper() }) expect(screen.getByText('code-reviewer')).toBeInTheDocument() }) it('uploads only markdown files from a selected commands folder', async () => { - const onChange = vi.fn() const markdownFile = new File(['commit body'], 'commit.md', { type: 'text/markdown' }) const systemFile = new File(['metadata'], '.DS_Store') Object.defineProperty(markdownFile, 'webkitRelativePath', { value: 'commands/git/commit.md' }) Object.defineProperty(systemFile, 'webkitRelativePath', { value: 'commands/.DS_Store' }) - const { container } = render(, { wrapper: createWrapper() }) + const { container } = render(, { wrapper: createWrapper() }) const input = container.querySelector('input[type="file"]') as HTMLInputElement fireEvent.change(input, { target: { files: [markdownFile, systemFile] } }) @@ -158,11 +151,10 @@ describe('CommandsEditor', () => { }) it('shows an error when a selected commands folder has no markdown files', async () => { - const onChange = vi.fn() const systemFile = new File(['metadata'], '.DS_Store') Object.defineProperty(systemFile, 'webkitRelativePath', { value: 'commands/.DS_Store' }) - const { container } = render(, { wrapper: createWrapper() }) + const { container } = render(, { wrapper: createWrapper() }) const input = container.querySelector('input[type="file"]') as HTMLInputElement fireEvent.change(input, { target: { files: [systemFile] } }) diff --git a/frontend/src/components/settings/CommandsEditor.tsx b/frontend/src/components/settings/CommandsEditor.tsx index 721d37dc0..f76e1867b 100644 --- a/frontend/src/components/settings/CommandsEditor.tsx +++ b/frontend/src/components/settings/CommandsEditor.tsx @@ -1,58 +1,34 @@ import { useState } from 'react' import { Plus } from 'lucide-react' -import { Button } from '@/components/ui/button' import { Badge } from '@/components/ui/badge' +import { Button } from '@/components/ui/button' import { Dialog, DialogTrigger } from '@/components/ui/dialog' import { SettingsList, SettingsListRow } from '@/components/ui/settings-list' +import type { OpenCodeCommandConfig } from '@opencode-manager/shared/opencode' +import type { OpenCodeCommandPatch, OpenCodeDirectoryFileInfo } from '@/api/types/settings' import { CommandDialog } from './CommandDialog' import { UploadFolderButton } from './UploadFolderButton' import { DirectoryFilesList } from './DirectoryFilesList' -import type { OpenCodeDirectoryFileInfo } from '@/api/types/settings' - -interface Command { - template: string - description?: string - agent?: string - model?: string - subtask?: boolean - topP?: number -} interface CommandsEditorProps { - commands: Record + commands: Record directoryCommands?: OpenCodeDirectoryFileInfo[] - onChange: (commands: Record) => void + onSave: (name: string, command: OpenCodeCommandPatch, previousName?: string) => void + onDelete: (name: string) => void } -export function CommandsEditor({ commands, directoryCommands = [], onChange }: CommandsEditorProps) { +export function CommandsEditor({ commands, directoryCommands = [], onSave, onDelete }: CommandsEditorProps) { const [isCreateDialogOpen, setIsCreateDialogOpen] = useState(false) - const [editingCommand, setEditingCommand] = useState<{ name: string; command: Command } | null>(null) + const [editingCommand, setEditingCommand] = useState<{ name: string; command: OpenCodeCommandConfig } | null>(null) const hasCommands = Object.keys(commands).length > 0 || directoryCommands.length > 0 - const handleCommandSubmit = (name: string, command: Command) => { - if (editingCommand) { - const updatedCommands = { ...commands } - delete updatedCommands[editingCommand.name] - updatedCommands[name] = command - onChange(updatedCommands) - setEditingCommand(null) - } else { - const updatedCommands = { - ...commands, - [name]: command - } - onChange(updatedCommands) - setIsCreateDialogOpen(false) - } - } - - const deleteCommand = (name: string) => { - const updatedCommands = { ...commands } - delete updatedCommands[name] - onChange(updatedCommands) + const handleCommandSubmit = (name: string, command: OpenCodeCommandPatch) => { + onSave(name, command, editingCommand?.name) + setEditingCommand(null) + setIsCreateDialogOpen(false) } - const startEdit = (name: string, command: Command) => { + const startEdit = (name: string, command: OpenCodeCommandConfig) => { setEditingCommand({ name, command }) } @@ -91,7 +67,7 @@ export function CommandsEditor({ commands, directoryCommands = [], onChange }: C } onClick={() => startEdit(name, command)} primaryAction={{ label: 'Edit', onClick: () => startEdit(name, command) }} - actions={[{ label: 'Delete', destructive: true, onClick: () => deleteCommand(name) }]} + actions={[{ label: 'Delete', destructive: true, onClick: () => onDelete(name) }]} actionsLabel={`Actions for ${name}`} /> ))} diff --git a/frontend/src/components/settings/OpenCodeConfigManager.test.tsx b/frontend/src/components/settings/OpenCodeConfigManager.test.tsx index 844eff83d..9088dcc9c 100644 --- a/frontend/src/components/settings/OpenCodeConfigManager.test.tsx +++ b/frontend/src/components/settings/OpenCodeConfigManager.test.tsx @@ -3,12 +3,15 @@ import { render, screen, waitFor, fireEvent, act } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { OpenCodeConfigManager } from './OpenCodeConfigManager' +import { FetchError } from '@/api/fetchWrapper' import type { OpenCodeConfigFile } from '@/api/types/settings' import { makeOpenCodeConfigFile, makeOpenCodeConfigSource } from '@/test/fixtures/opencode-config' const { mockGetOpenCodeConfig, mockUpdateOpenCodeConfig, + mockGetManagedOpenCodeConfig, + mockPatchOpenCodeConfig, mockRestartOpenCodeServer, mockGetActiveOpenCodeSessions, mockGetOpenCodeImportStatus, @@ -19,6 +22,8 @@ const { } = vi.hoisted(() => ({ mockGetOpenCodeConfig: vi.fn(), mockUpdateOpenCodeConfig: vi.fn(), + mockGetManagedOpenCodeConfig: vi.fn(), + mockPatchOpenCodeConfig: vi.fn(), mockRestartOpenCodeServer: vi.fn(), mockGetActiveOpenCodeSessions: vi.fn(), mockGetOpenCodeImportStatus: vi.fn(), @@ -65,6 +70,8 @@ vi.mock('@/api/settings', () => ({ settingsApi: { getOpenCodeConfig: mockGetOpenCodeConfig, updateOpenCodeConfig: mockUpdateOpenCodeConfig, + getManagedOpenCodeConfig: mockGetManagedOpenCodeConfig, + patchOpenCodeConfig: mockPatchOpenCodeConfig, restartOpenCodeServer: mockRestartOpenCodeServer, getActiveOpenCodeSessions: mockGetActiveOpenCodeSessions, getOpenCodeImportStatus: mockGetOpenCodeImportStatus, @@ -76,6 +83,25 @@ vi.mock('@/api/settings', () => ({ }, })) +vi.mock('@/components/model/ModelCombobox', () => ({ ModelCombobox: () => null })) + +vi.mock('./AgentDialog', () => ({ + AgentDialog: ({ open, onSubmit, editingAgent }: { + open: boolean + onSubmit: (name: string, agent: Record) => void + editingAgent?: { name: string } | null + }) => open ? ( +
+ + +
+ ) : null, +})) + const defaultContent = { provider: { openai: { @@ -108,6 +134,8 @@ describe('OpenCodeConfigManager', () => { vi.clearAllMocks() healthState.data = { opencode: 'healthy', opencodeRestartPending: false } mockGetOpenCodeConfig.mockResolvedValue(defaultConfig) + mockGetManagedOpenCodeConfig.mockResolvedValue({ revision: 'rev-1', info: {} }) + mockPatchOpenCodeConfig.mockResolvedValue({ ...defaultConfig, revision: 'rev-2' }) mockGetOpenCodeImportStatus.mockResolvedValue({}) mockGetAgentsMd.mockResolvedValue({ content: '# Agent Instructions' }) mockListManagedSkills.mockResolvedValue([]) @@ -643,4 +671,140 @@ describe('OpenCodeConfigManager', () => { expect(notice).toHaveTextContent('Saves apply only to opencode.jsonc') expect(notice).toHaveTextContent('For simpler configuration, consolidate the settings you need into one file, then remove redundant files after verifying the result.') }) + + it('lists agents and commands from the managed config query', async () => { + mockGetManagedOpenCodeConfig.mockResolvedValue({ + revision: 'rev-1', + info: { + agents: { reviewer: { system: 'Review code', description: 'Reviews code', mode: 'subagent' } }, + commands: { deploy: { template: 'Deploy it', description: 'Deploys the app' } }, + }, + }) + + const user = userEvent.setup() + renderWithQuery() + + await screen.findByText('opencode.json') + await user.click(screen.getByRole('button', { name: /Commands/i })) + expect(await screen.findByText('deploy')).toBeInTheDocument() + + const agentsButton = screen.getAllByRole('button', { name: /Agents/i }).find(button => button.textContent?.startsWith('Agents')) + expect(agentsButton).toBeDefined() + await user.click(agentsButton!) + expect(await screen.findByText('reviewer')).toBeInTheDocument() + }) + + it('shows the unavailable notice and keeps the raw editor reachable when OpenCode is down', async () => { + mockGetManagedOpenCodeConfig.mockRejectedValue(new FetchError('OpenCode server unavailable', 503)) + + const user = userEvent.setup() + const { container } = renderWithQuery() + + await screen.findByText('opencode.json') + await user.click(screen.getByRole('button', { name: /Commands/i })) + expect(await screen.findByText(/OpenCode is not running, so the configured commands cannot be listed/)).toBeInTheDocument() + + const agentsButton = screen.getAllByRole('button', { name: /Agents/i }).find(button => button.textContent?.startsWith('Agents')) + expect(agentsButton).toBeDefined() + await user.click(agentsButton!) + expect(await screen.findByText(/OpenCode is not running, so the configured agents cannot be listed/)).toBeInTheDocument() + + const editIcon = container.querySelector('.lucide-square-pen') as SVGElement + await user.click(editIcon.closest('button') as HTMLButtonElement) + expect(await screen.findByLabelText('Config content')).toBeInTheDocument() + }) + + it('saves an agent with an entity patch through the managed config endpoint', async () => { + mockGetManagedOpenCodeConfig.mockResolvedValue({ + revision: 'rev-1', + info: { agents: { reviewer: { system: 'Review code', mode: 'subagent' } } }, + }) + + const user = userEvent.setup() + renderWithQuery() + + await screen.findByText('opencode.json') + const agentsButton = screen.getAllByRole('button', { name: /Agents/i }).find(button => button.textContent?.startsWith('Agents')) + expect(agentsButton).toBeDefined() + await user.click(agentsButton!) + await user.click(await screen.findByText('reviewer')) + await user.click(await screen.findByRole('button', { name: 'Submit Agent' })) + + await waitFor(() => expect(mockPatchOpenCodeConfig).toHaveBeenCalledWith({ + patch: { agents: { reviewer: { system: 'Updated', mode: 'subagent' } } }, + expectedRevision: 'rev-1', + })) + }) + + it('deletes the previous agent name in the same patch when renaming', async () => { + mockGetManagedOpenCodeConfig.mockResolvedValue({ + revision: 'rev-1', + info: { agents: { reviewer: { system: 'Review code', mode: 'subagent' } } }, + }) + + const user = userEvent.setup() + renderWithQuery() + + await screen.findByText('opencode.json') + const agentsButton = screen.getAllByRole('button', { name: /Agents/i }).find(button => button.textContent?.startsWith('Agents')) + expect(agentsButton).toBeDefined() + await user.click(agentsButton!) + await user.click(await screen.findByText('reviewer')) + await user.click(await screen.findByRole('button', { name: 'Submit Renamed Agent' })) + + await waitFor(() => expect(mockPatchOpenCodeConfig).toHaveBeenCalledWith({ + patch: { + agents: { + 'renamed-agent': { system: 'Renamed', mode: 'subagent' }, + reviewer: null, + }, + }, + expectedRevision: 'rev-1', + })) + }) + + it('sends null for cleared command fields and a disabled subagent', async () => { + mockGetManagedOpenCodeConfig.mockResolvedValue({ + revision: 'rev-1', + info: { + commands: { + review: { + template: 'Review this code', + description: 'Reviews changes', + agent: 'reviewer', + model: 'openai/gpt-5', + subagent: true, + }, + }, + }, + }) + + const user = userEvent.setup() + renderWithQuery() + + await screen.findByText('opencode.json') + await user.click(screen.getByRole('button', { name: /Commands/i })) + await user.click(await screen.findByText('review')) + + await user.clear(screen.getByLabelText('Description')) + await user.clear(screen.getByLabelText('Agent (optional)')) + await user.clear(screen.getByLabelText('Model (optional)')) + await user.click(screen.getByRole('switch')) + await user.click(screen.getByRole('button', { name: 'Update' })) + + await waitFor(() => expect(mockPatchOpenCodeConfig).toHaveBeenCalledWith({ + patch: { + commands: { + review: { + template: 'Review this code', + description: null, + agent: null, + model: null, + subagent: null, + }, + }, + }, + expectedRevision: 'rev-1', + })) + }) }) diff --git a/frontend/src/components/settings/OpenCodeConfigManager.tsx b/frontend/src/components/settings/OpenCodeConfigManager.tsx index e02d4852f..4a455ab85 100644 --- a/frontend/src/components/settings/OpenCodeConfigManager.tsx +++ b/frontend/src/components/settings/OpenCodeConfigManager.tsx @@ -19,41 +19,15 @@ import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query' import { useServerHealth } from '@/hooks/useServerHealth' import { useOpenCodeServerActions } from '@/hooks/useOpenCodeServerActions' import { useOpenCodeConfigFile, OPEN_CODE_CONFIG_QUERY_KEY } from '@/hooks/useOpenCodeConfigFile' +import { useOpenCodeManagedConfig } from '@/hooks/useOpenCodeManagedConfig' import { useOpenCodeApplyFeedback } from '@/hooks/useOpenCodeApplyFeedback' import { showToast } from '@/lib/toast' -import { invalidateConfigCaches } from '@/lib/queryInvalidation' +import { invalidateConfigCaches, OPEN_CODE_MANAGED_CONFIG_QUERY_KEY } from '@/lib/queryInvalidation' import { getOpenCodeApiErrorMessage } from '@/lib/opencode-errors' import { mcpServersFromConfig } from '@opencode-manager/shared/opencode' import { FetchError } from '@/api/fetchWrapper' import { getPreferredOpenCodeConfigSource, downloadOpenCodeConfigSource } from '@/api/types/settings' -import type { OpenCodeConfigFile, OpenCodeConfigSaveResponse, OpenCodeImportStatus } from '@/api/types/settings' - -interface Command { - template: string - description?: string - agent?: string - model?: string - subtask?: boolean - topP?: number -} - -interface Agent { - prompt?: string - description?: string - mode?: 'subagent' | 'primary' | 'all' - temperature?: number - topP?: number - top_p?: number - model?: string - tools?: Record - permission?: { - edit?: 'ask' | 'allow' | 'deny' - bash?: 'ask' | 'allow' | 'deny' | Record - webfetch?: 'ask' | 'allow' | 'deny' - } - disable?: boolean - [key: string]: unknown -} +import type { OpenCodeConfigFile, OpenCodeConfigSaveResponse, OpenCodeImportStatus, OpenCodeAgentPatch, OpenCodeCommandPatch } from '@/api/types/settings' type ConfigSectionKey = 'agentsMd' | 'commands' | 'agents' | 'skills' | 'mcp' | 'models' | 'hostImport' @@ -108,6 +82,10 @@ export function OpenCodeConfigManager() { const hostImportRef = useRef(null) const { data: config, isLoading } = useOpenCodeConfigFile() + const { data: managedConfig, error: managedConfigError } = useOpenCodeManagedConfig() + const managedAgents = managedConfig?.info.agents ?? {} + const managedCommands = managedConfig?.info.commands ?? {} + const isManagedConfigUnavailable = managedConfigError instanceof FetchError && managedConfigError.statusCode === 503 const { data: managedSkills = [] } = useQuery({ queryKey: ['managed-skills'], @@ -184,6 +162,42 @@ export function OpenCodeConfigManager() { }) } + const patchManagedConfig = async (patch: Record) => { + const expectedRevision = queryClient.getQueryData( + OPEN_CODE_CONFIG_QUERY_KEY, + )?.revision + const result = await settingsApi.patchOpenCodeConfig({ patch, expectedRevision }) + queryClient.setQueryData(OPEN_CODE_CONFIG_QUERY_KEY, result) + applyOpenCodeSaveFeedback({ + appliedMessage: 'Configuration updated', + restartRequired: result.restartRequired, + }) + queryClient.invalidateQueries({ queryKey: OPEN_CODE_MANAGED_CONFIG_QUERY_KEY }) + } + + const patchManagedConfigSafely = (patch: Record) => { + void patchManagedConfig(patch).catch((error) => { + showToast.error(getApiErrorMessage(error, 'Failed to update config')) + }) + } + + const saveManagedEntity = ( + key: 'agents' | 'commands', + name: string, + value: OpenCodeAgentPatch | OpenCodeCommandPatch, + previousName?: string, + ) => { + const entries: Record = { [name]: value } + if (previousName && previousName !== name) { + entries[previousName] = null + } + patchManagedConfigSafely({ [key]: entries }) + } + + const deleteManagedEntity = (key: 'agents' | 'commands', name: string) => { + patchManagedConfigSafely({ [key]: { [name]: null } }) + } + const downloadConfig = (config: OpenCodeConfigFile) => { const preferredSource = getPreferredOpenCodeConfigSource(config) if (preferredSource) { @@ -365,7 +379,7 @@ export function OpenCodeConfigManager() {

Commands

- {Object.keys((config.content.command as Record | undefined) ?? {}).length + directoryCommands.length} configured + {Object.keys(managedCommands).length + directoryCommands.length} configured
@@ -374,16 +388,18 @@ export function OpenCodeConfigManager() { id={commandsContentId} className={cn(SECTION_CONTENT_CLASS, expandedSections.commands ? 'block' : 'hidden')} > - | undefined) ?? {}} - directoryCommands={directoryCommands} - onChange={(commands) => { - updateConfigContentSafely({ - ...config.content, - command: commands - }) - }} - /> + {isManagedConfigUnavailable ? ( +

+ OpenCode is not running, so the configured commands cannot be listed. Use Edit to change the config file directly. +

+ ) : ( + saveManagedEntity('commands', name, command, previousName)} + onDelete={(name) => deleteManagedEntity('commands', name)} + /> + )}
@@ -399,7 +415,7 @@ export function OpenCodeConfigManager() {

Agents

- {Object.keys((config.content.agent as Record | undefined) ?? {}).length + directoryAgents.length} configured + {Object.keys(managedAgents).length + directoryAgents.length} configured
@@ -408,16 +424,18 @@ export function OpenCodeConfigManager() { id={agentsContentId} className={cn(SECTION_CONTENT_CLASS, expandedSections.agents ? 'block' : 'hidden')} > - | undefined) ?? {}} - directoryAgents={directoryAgents} - onChange={(agents) => { - updateConfigContentSafely({ - ...config.content, - agent: agents - }) - }} - /> + {isManagedConfigUnavailable ? ( +

+ OpenCode is not running, so the configured agents cannot be listed. Use Edit to change the config file directly. +

+ ) : ( + saveManagedEntity('agents', name, agent, previousName)} + onDelete={(name) => deleteManagedEntity('agents', name)} + /> + )} diff --git a/frontend/src/hooks/useOpenCodeManagedConfig.ts b/frontend/src/hooks/useOpenCodeManagedConfig.ts new file mode 100644 index 000000000..a127ad6c2 --- /dev/null +++ b/frontend/src/hooks/useOpenCodeManagedConfig.ts @@ -0,0 +1,11 @@ +import { useQuery } from '@tanstack/react-query' +import { settingsApi } from '@/api/settings' +import { OPEN_CODE_MANAGED_CONFIG_QUERY_KEY } from '@/lib/queryInvalidation' +import type { ManagedOpenCodeConfig } from '@/api/types/settings' + +export function useOpenCodeManagedConfig() { + return useQuery({ + queryKey: OPEN_CODE_MANAGED_CONFIG_QUERY_KEY, + queryFn: () => settingsApi.getManagedOpenCodeConfig(), + }) +} diff --git a/frontend/src/lib/queryInvalidation.ts b/frontend/src/lib/queryInvalidation.ts index dbdc51468..ec5b99fac 100644 --- a/frontend/src/lib/queryInvalidation.ts +++ b/frontend/src/lib/queryInvalidation.ts @@ -3,6 +3,8 @@ import { LOCAL_OPENCODE_SERVER_ID } from '@opencode-manager/shared/schemas' import { serverScopedKey } from '@/lib/serverScope' import type { GitStatusResponse } from '@/types/git' +export const OPEN_CODE_MANAGED_CONFIG_QUERY_KEY = ['opencode-config', 'managed'] as const + export function sessionTranscriptQueryKey(sessionID: string | null | undefined) { return ['opencode', 'transcript', sessionID] } @@ -78,6 +80,7 @@ export function invalidateConfigCaches( queryClient.invalidateQueries({ queryKey: ['opencode', 'commands'] }) if (!options.skipOpenCodeConfig) { queryClient.invalidateQueries({ queryKey: ['opencode-config'] }) + queryClient.invalidateQueries({ queryKey: OPEN_CODE_MANAGED_CONFIG_QUERY_KEY }) } queryClient.invalidateQueries({ queryKey: ['health'] }) queryClient.invalidateQueries({ queryKey: ['mcp-status'] }) From 1802cdc7ded2e7970edf92c0a2d8b4efb6c368fd Mon Sep 17 00:00:00 2001 From: Chris Scott <99081550+chriswritescode-dev@users.noreply.github.com> Date: Mon, 2 Nov 2026 10:57:51 -0500 Subject: [PATCH 10/20] section 10: Phase 10: Model/provider editor and MCP manager on v2 provid --- frontend/src/api/types/settings.ts | 24 +- .../components/settings/McpManager.test.tsx | 54 ++- .../src/components/settings/McpManager.tsx | 44 +-- .../settings/OpenCodeConfigManager.test.tsx | 236 ++++++++++--- .../settings/OpenCodeConfigManager.tsx | 69 ++-- .../settings/OpenCodeModelDialog.tsx | 312 +++++++++--------- .../settings/OpenCodeModelsEditor.test.tsx | 271 +++++++++++---- .../settings/OpenCodeModelsEditor.tsx | 109 +++--- 8 files changed, 679 insertions(+), 440 deletions(-) diff --git a/frontend/src/api/types/settings.ts b/frontend/src/api/types/settings.ts index 941286635..306dd0356 100644 --- a/frontend/src/api/types/settings.ts +++ b/frontend/src/api/types/settings.ts @@ -15,8 +15,6 @@ import { type OpenCodeConfigSourceFile, type OpenCodeConfigSourceName, type UpdateOpenCodeConfigRequest, - type ModelConfig, - type ProviderConfig, type SandboxPreferences, type SkillFileInfo, type CreateSkillRequest, @@ -30,11 +28,13 @@ import type { OpenCodeConfigInfo } from '@opencode-manager/shared/opencode' import type { OpenCodeAgentConfig, OpenCodeCommandConfig, + OpenCodeProviderConfig, + OpenCodeProviderModelConfig, } from '@opencode-manager/shared/opencode' import type { NotificationPreferences } from '@opencode-manager/shared/types' import { saveFile } from '@/lib/download' -export type { TTSConfig, STTConfig, OpenCodeConfigFile, OpenCodeConfigSourceFile, OpenCodeConfigSourceName, UpdateOpenCodeConfigRequest, ModelConfig, ProviderConfig, SandboxPreferences, NotificationPreferences, SkillFileInfo, CreateSkillRequest, UpdateSkillRequest, SkillScope, InstallSkillFromGithubRequest, InstallSkillResponse } +export type { TTSConfig, STTConfig, OpenCodeConfigFile, OpenCodeConfigSourceFile, OpenCodeConfigSourceName, UpdateOpenCodeConfigRequest, SandboxPreferences, NotificationPreferences, SkillFileInfo, CreateSkillRequest, UpdateSkillRequest, SkillScope, InstallSkillFromGithubRequest, InstallSkillResponse } export type { UserPreferences, UpdateSettingsRequest, CustomCommand, GitCredential, GitIdentity, GitIdentityProfile, KeyboardShortcutAction } from '@opencode-manager/shared' export { DEFAULT_TTS_CONFIG, DEFAULT_STT_CONFIG, DEFAULT_KEYBOARD_SHORTCUTS, DEFAULT_USER_PREFERENCES, DEFAULT_LEADER_KEY, DEFAULT_DIRECT_SHORTCUTS, BLOCKED_SERVER_ENV_KEYS, DEFAULT_ACTIVITY_WINDOW_MINUTES, ACTIVITY_WINDOW_MINUTE_OPTIONS } export { isOpenCodeConfigSourceName } from '@opencode-manager/shared' @@ -117,6 +117,24 @@ export type OpenCodeCommandPatch = { [K in keyof OpenCodeCommandConfig]?: OpenCodeCommandConfig[K] | null } +/** + * Merge-patch value for one provider model: an explicit null clears the stored + * field, while an omitted field stays untouched. + */ +export type OpenCodeProviderModelPatch = { + [K in keyof OpenCodeProviderModelConfig]?: OpenCodeProviderModelConfig[K] | null +} + +/** + * Merge-patch value for one provider: an explicit null clears the stored field, + * a null model entry removes that model, and omitted entries stay untouched. + */ +export type OpenCodeProviderPatch = { + [K in keyof OpenCodeProviderConfig]?: K extends 'models' + ? Record | null + : OpenCodeProviderConfig[K] | null +} + export interface UpdateOpenCodeConfigPatchRequest { patch: Record source?: OpenCodeConfigSourceName diff --git a/frontend/src/components/settings/McpManager.test.tsx b/frontend/src/components/settings/McpManager.test.tsx index f026c0ade..4012f56d8 100644 --- a/frontend/src/components/settings/McpManager.test.tsx +++ b/frontend/src/components/settings/McpManager.test.tsx @@ -3,6 +3,7 @@ import { render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { FetchError } from '@opencode-manager/shared' +import type { McpServerConfig } from '@opencode-manager/shared/opencode' import { McpManager } from './McpManager' const { mockRemoveServer, mockToastError, mockStatus } = vi.hoisted(() => ({ @@ -34,21 +35,20 @@ vi.mock('@/lib/toast', () => ({ showToast: { success: vi.fn(), error: mockToastError, info: vi.fn(), loading: vi.fn(), warning: vi.fn(), dismiss: vi.fn() }, })) -const config = { - content: { - mcp: { - servers: { - configured: { type: 'local', command: ['npx', 'server'] }, - }, - }, +const mcp = { + servers: { + configured: { type: 'local', command: ['npx', 'server'] }, }, } -function renderManager(onUpdate: (content: Record) => Promise) { +function renderManager( + onSave: (serverId: string, server: McpServerConfig) => Promise, + onDelete: (serverId: string) => Promise, +) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }) return render( - + , ) } @@ -66,43 +66,61 @@ describe('McpManager', () => { }) it('lists servers reported by the running server that are not in mcp.servers', () => { - renderManager(vi.fn()) + renderManager(vi.fn(), vi.fn()) expect(screen.getByText('Configured')).toBeInTheDocument() expect(screen.getByText('Legacy server')).toBeInTheDocument() }) it('removes only the mcp.servers entry when deleting a configured server', async () => { - const onUpdate = vi.fn<(content: Record) => Promise>().mockResolvedValue(undefined) + const onSave = vi.fn().mockResolvedValue(undefined) + const onDelete = vi.fn().mockResolvedValue(undefined) const user = userEvent.setup() - renderManager(onUpdate) + renderManager(onSave, onDelete) await deleteServer(user, 'Configured') - await waitFor(() => expect(onUpdate).toHaveBeenCalledWith({ mcp: { servers: {} } })) + await waitFor(() => expect(onDelete).toHaveBeenCalledWith('configured')) expect(mockRemoveServer).not.toHaveBeenCalled() }) it('skips the config update for a server that is not in mcp.servers', async () => { - const onUpdate = vi.fn<(content: Record) => Promise>().mockResolvedValue(undefined) + const onSave = vi.fn().mockResolvedValue(undefined) + const onDelete = vi.fn().mockResolvedValue(undefined) const user = userEvent.setup() - renderManager(onUpdate) + renderManager(onSave, onDelete) await deleteServer(user, 'Legacy server') await waitFor(() => expect(mockRemoveServer).toHaveBeenCalledWith('legacy-server')) - expect(onUpdate).not.toHaveBeenCalled() + expect(onDelete).not.toHaveBeenCalled() }) it('surfaces the config save error when deleting a configured server', async () => { const message = 'Cannot remove mcp.servers.configured: defined in opencode.json, not in opencode.jsonc' - const onUpdate = vi.fn<(content: Record) => Promise>().mockRejectedValue(new FetchError(message, 409)) + const onDelete = vi.fn().mockRejectedValue(new FetchError(message, 409)) const user = userEvent.setup() - renderManager(onUpdate) + renderManager(vi.fn().mockResolvedValue(undefined), onDelete) await deleteServer(user, 'Configured') await waitFor(() => expect(mockToastError).toHaveBeenCalledWith(message)) expect(mockRemoveServer).not.toHaveBeenCalled() }) + + it('saves a new server through the config patch callback', async () => { + const onSave = vi.fn().mockResolvedValue(undefined) + const user = userEvent.setup() + renderManager(onSave, vi.fn().mockResolvedValue(undefined)) + + await user.click(screen.getByRole('button', { name: /Add Server/i })) + await user.type(await screen.findByLabelText('Server ID'), 'filesystem') + await user.type(screen.getByLabelText('Command'), 'npx server-filesystem /tmp') + await user.click(screen.getByRole('button', { name: 'Add MCP Server' })) + + await waitFor(() => expect(onSave).toHaveBeenCalledWith('filesystem', expect.objectContaining({ + type: 'local', + command: ['npx', 'server-filesystem', '/tmp'], + }))) + }) }) diff --git a/frontend/src/components/settings/McpManager.tsx b/frontend/src/components/settings/McpManager.tsx index 04b72a7fc..821a4ff8e 100644 --- a/frontend/src/components/settings/McpManager.tsx +++ b/frontend/src/components/settings/McpManager.tsx @@ -13,26 +13,17 @@ import { invalidateConfigCaches } from '@/lib/queryInvalidation' import type { McpAuthStartResponse } from '@/api/mcp' import { mcpApi } from '@/api/mcp' import { mcpServersFromConfig, type McpServerConfig } from '@opencode-manager/shared/opencode' -import { settingsApi } from '@/api/settings' import { showToast } from '@/lib/toast' import { getOpenCodeApiErrorMessage } from '@/lib/opencode-errors' import { formatMcpServerName } from '@/lib/mcp' interface McpManagerProps { - config: { - content: Record - } | null - onUpdate: (content: Record) => Promise + mcp: unknown + onSave: (serverId: string, config: McpServerConfig) => Promise + onDelete: (serverId: string) => Promise } -function withoutMcpServer(mcp: unknown, serverId: string): Record { - const current = (mcp && typeof mcp === 'object' ? mcp : {}) as Record - const servers = { ...(current.servers as Record | undefined) } - delete servers[serverId] - return { ...current, servers } -} - -export function McpManager({ config, onUpdate }: McpManagerProps) { +export function McpManager({ mcp, onSave, onDelete }: McpManagerProps) { const [isAddDialogOpen, setIsAddDialogOpen] = useState(false) const [deleteConfirmServer, setDeleteConfirmServer] = useState<{ id: string; name: string } | null>(null) const [togglingServerId, setTogglingServerId] = useState(null) @@ -50,7 +41,7 @@ export function McpManager({ config, onUpdate }: McpManagerProps) { isRemovingAuth } = useMcpServers() - const mcpServers = useMemo(() => mcpServersFromConfig(config?.content?.mcp), [config]) + const mcpServers = useMemo(() => mcpServersFromConfig(mcp), [mcp]) const serverIds = useMemo( () => [...new Set([...Object.keys(mcpServers), ...Object.keys(mcpStatus ?? {})])], [mcpServers, mcpStatus], @@ -58,11 +49,8 @@ export function McpManager({ config, onUpdate }: McpManagerProps) { const deleteServerMutation = useMutation({ mutationFn: async (serverId: string) => { - if (config && mcpServers[serverId]) { - await onUpdate({ - ...config.content, - mcp: withoutMcpServer(config.content.mcp, serverId), - }) + if (mcpServers[serverId]) { + await onDelete(serverId) return } @@ -79,15 +67,7 @@ export function McpManager({ config, onUpdate }: McpManagerProps) { }) const addServer = async (serverId: string, serverConfig: McpServerConfig) => { - const latest = await settingsApi.getOpenCodeConfig() - const mcp = (latest.content.mcp as Record | undefined) ?? {} - await onUpdate({ - ...latest.content, - mcp: { - ...mcp, - servers: { ...(mcp.servers as Record | undefined), [serverId]: serverConfig }, - }, - }) + await onSave(serverId, serverConfig) } const isAnyOperationPending = deleteServerMutation.isPending || togglingServerId !== null @@ -161,14 +141,6 @@ export function McpManager({ config, onUpdate }: McpManagerProps) { return null } - if (!config) { - return ( -
-

No OpenCode configuration file found.

-
- ) - } - return (
{isAnyOperationPending && ( diff --git a/frontend/src/components/settings/OpenCodeConfigManager.test.tsx b/frontend/src/components/settings/OpenCodeConfigManager.test.tsx index 9088dcc9c..09b0ac9eb 100644 --- a/frontend/src/components/settings/OpenCodeConfigManager.test.tsx +++ b/frontend/src/components/settings/OpenCodeConfigManager.test.tsx @@ -78,6 +78,7 @@ vi.mock('@/api/settings', () => ({ listManagedSkills: mockListManagedSkills, listOpenCodeDirectoryFiles: mockListOpenCodeDirectoryFiles, getAgentsMd: mockGetAgentsMd, + discoverOpenCodeModels: vi.fn().mockResolvedValue({ models: [], cached: false }), syncOpenCodeImport: vi.fn(), upgradeOpenCode: vi.fn(), }, @@ -113,6 +114,17 @@ const defaultContent = { }, } +const defaultManagedInfo = { + providers: { + openai: { + name: 'OpenAI', + models: { + 'gpt-4o': { name: 'GPT-4o', modelID: 'gpt-4o' }, + }, + }, + }, +} + const defaultConfig = makeOpenCodeConfigFile({ path: '/workspace/.opencode/opencode.json', rawContent: JSON.stringify(defaultContent, null, 2), @@ -134,7 +146,7 @@ describe('OpenCodeConfigManager', () => { vi.clearAllMocks() healthState.data = { opencode: 'healthy', opencodeRestartPending: false } mockGetOpenCodeConfig.mockResolvedValue(defaultConfig) - mockGetManagedOpenCodeConfig.mockResolvedValue({ revision: 'rev-1', info: {} }) + mockGetManagedOpenCodeConfig.mockResolvedValue({ revision: 'rev-1', info: defaultManagedInfo }) mockPatchOpenCodeConfig.mockResolvedValue({ ...defaultConfig, revision: 'rev-2' }) mockGetOpenCodeImportStatus.mockResolvedValue({}) mockGetAgentsMd.mockResolvedValue({ content: '# Agent Instructions' }) @@ -174,7 +186,7 @@ describe('OpenCodeConfigManager', () => { }) it('optimistic delete saves without eager restart modal', async () => { - mockUpdateOpenCodeConfig.mockResolvedValueOnce({ ...defaultConfig, restartRequired: true }) + mockPatchOpenCodeConfig.mockResolvedValueOnce({ ...defaultConfig, restartRequired: true }) const user = userEvent.setup() renderWithQuery() @@ -186,9 +198,9 @@ describe('OpenCodeConfigManager', () => { await user.click(screen.getByLabelText('Actions for GPT-4o')) await user.click(screen.getByText('Delete')) - expect(mockUpdateOpenCodeConfig).toHaveBeenCalledTimes(1) - const [payload] = mockUpdateOpenCodeConfig.mock.calls[0] - expect(payload.content.provider.openai.models).not.toHaveProperty('gpt-4o') + expect(mockPatchOpenCodeConfig).toHaveBeenCalledTimes(1) + const [payload] = mockPatchOpenCodeConfig.mock.calls[0] + expect(payload.patch.providers.openai.models).toEqual({ 'gpt-4o': null }) expect(screen.queryByText('Restart OpenCode Server?')).not.toBeInTheDocument() }) @@ -209,7 +221,7 @@ describe('OpenCodeConfigManager', () => { }) it('rollback on failure', async () => { - mockUpdateOpenCodeConfig.mockRejectedValueOnce(new Error('boom')) + mockPatchOpenCodeConfig.mockRejectedValueOnce(new Error('boom')) const user = userEvent.setup() renderWithQuery() @@ -221,7 +233,7 @@ describe('OpenCodeConfigManager', () => { await user.click(screen.getByLabelText('Actions for GPT-4o')) await user.click(screen.getByText('Delete')) - expect(mockUpdateOpenCodeConfig).toHaveBeenCalledTimes(1) + expect(mockPatchOpenCodeConfig).toHaveBeenCalledTimes(1) const { showToast } = await import('@/lib/toast') await vi.waitFor(() => { @@ -388,7 +400,7 @@ describe('OpenCodeConfigManager', () => { }) it('sends the expected revision for structured updates', async () => { - mockUpdateOpenCodeConfig.mockResolvedValueOnce({ ...defaultConfig, restartRequired: true }) + mockPatchOpenCodeConfig.mockResolvedValueOnce({ ...defaultConfig, restartRequired: true }) const user = userEvent.setup() renderWithQuery() @@ -400,22 +412,17 @@ describe('OpenCodeConfigManager', () => { await user.click(screen.getByLabelText('Actions for GPT-4o')) await user.click(screen.getByText('Delete')) - await waitFor(() => expect(mockUpdateOpenCodeConfig).toHaveBeenCalledTimes(1)) - const [payload] = mockUpdateOpenCodeConfig.mock.calls[0] + await waitFor(() => expect(mockPatchOpenCodeConfig).toHaveBeenCalledTimes(1)) + const [payload] = mockPatchOpenCodeConfig.mock.calls[0] expect(payload.expectedRevision).toBe('rev-1') expect(payload.source).toBeUndefined() - expect(payload.content.provider.openai.models).not.toHaveProperty('gpt-4o') + expect(payload.patch.providers.openai.models).toEqual({ 'gpt-4o': null }) }) it('resolves the add-server expected revision from the query cache', async () => { const cachedConfig = { ...defaultConfig, revision: 'rev-A' } - const dialogFetchedConfig = { ...defaultConfig, revision: 'rev-B' } - let configRequests = 0 - mockGetOpenCodeConfig.mockImplementation(() => { - configRequests += 1 - return Promise.resolve(configRequests === 1 ? cachedConfig : dialogFetchedConfig) - }) - mockUpdateOpenCodeConfig.mockResolvedValue({ ...cachedConfig, restartRequired: true }) + mockGetOpenCodeConfig.mockResolvedValue(cachedConfig) + mockPatchOpenCodeConfig.mockResolvedValue({ ...cachedConfig, restartRequired: true }) const user = userEvent.setup() renderWithQuery() @@ -427,46 +434,55 @@ describe('OpenCodeConfigManager', () => { await user.type(screen.getByLabelText('Command'), 'npx server-filesystem /tmp') await user.click(screen.getByRole('button', { name: 'Add MCP Server' })) - await waitFor(() => expect(mockUpdateOpenCodeConfig).toHaveBeenCalledTimes(1)) - const [payload] = mockUpdateOpenCodeConfig.mock.calls[0] + await waitFor(() => expect(mockPatchOpenCodeConfig).toHaveBeenCalledTimes(1)) + const [payload] = mockPatchOpenCodeConfig.mock.calls[0] expect(payload.expectedRevision).toBe('rev-A') + expect(payload.patch.mcp.servers.filesystem).toEqual(expect.objectContaining({ + type: 'local', + command: ['npx', 'server-filesystem', '/tmp'], + })) }) - it('sends the revision from the previous save on the next structured save', async () => { - const twoModelsContent = { - provider: { - openai: { - name: 'OpenAI', - models: { - 'gpt-4o': { name: 'GPT-4o' }, - 'gpt-3.5': { name: 'GPT-3.5' }, - }, - }, - }, - } - const twoModelsConfig = makeOpenCodeConfigFile({ - path: '/workspace/.opencode/opencode.json', - rawContent: JSON.stringify(twoModelsContent, null, 2), - content: twoModelsContent, + it('deletes a configured MCP server through an mcp.servers patch', async () => { + mockGetManagedOpenCodeConfig.mockResolvedValue({ + revision: 'rev-1', + info: { mcp: { servers: { filesystem: { type: 'local', command: ['npx', 'server'] } } } }, }) - mockGetOpenCodeConfig.mockResolvedValue(twoModelsConfig) + mockPatchOpenCodeConfig.mockResolvedValue({ ...defaultConfig, revision: 'rev-2' }) - const afterFirstDelete = { - ...twoModelsConfig, - content: { - provider: { + const user = userEvent.setup() + renderWithQuery() + + await screen.findByText('opencode.json') + await user.click(screen.getByRole('button', { name: /MCP Servers/i })) + await user.click(await screen.findByLabelText('Actions for Filesystem')) + await user.click(screen.getByText('Delete Server')) + await user.click(screen.getByRole('button', { name: 'Delete' })) + + await waitFor(() => expect(mockPatchOpenCodeConfig).toHaveBeenCalledWith({ + patch: { mcp: { servers: { filesystem: null } } }, + expectedRevision: 'rev-1', + })) + }) + + it('sends the revision from the previous save on the next structured save', async () => { + mockGetManagedOpenCodeConfig.mockResolvedValue({ + revision: 'rev-1', + info: { + providers: { openai: { name: 'OpenAI', models: { + 'gpt-4o': { name: 'GPT-4o' }, 'gpt-3.5': { name: 'GPT-3.5' }, }, }, }, }, - revision: 'rev-2', - } - const afterSecondDelete = { ...afterFirstDelete, revision: 'rev-3' } - mockUpdateOpenCodeConfig + }) + const afterFirstDelete = { ...defaultConfig, revision: 'rev-2' } + const afterSecondDelete = { ...defaultConfig, revision: 'rev-3' } + mockPatchOpenCodeConfig .mockResolvedValueOnce(afterFirstDelete) .mockResolvedValueOnce(afterSecondDelete) @@ -478,13 +494,133 @@ describe('OpenCodeConfigManager', () => { await user.click(screen.getByLabelText('Actions for GPT-4o')) await user.click(screen.getByText('Delete')) - await waitFor(() => expect(mockUpdateOpenCodeConfig).toHaveBeenCalledTimes(1)) - expect(mockUpdateOpenCodeConfig.mock.calls[0][0].expectedRevision).toBe('rev-1') + await waitFor(() => expect(mockPatchOpenCodeConfig).toHaveBeenCalledTimes(1)) + expect(mockPatchOpenCodeConfig.mock.calls[0][0].expectedRevision).toBe('rev-1') await user.click(screen.getByLabelText('Actions for GPT-3.5')) await user.click(screen.getByText('Delete')) - await waitFor(() => expect(mockUpdateOpenCodeConfig).toHaveBeenCalledTimes(2)) - expect(mockUpdateOpenCodeConfig.mock.calls[1][0].expectedRevision).toBe('rev-2') + await waitFor(() => expect(mockPatchOpenCodeConfig).toHaveBeenCalledTimes(2)) + expect(mockPatchOpenCodeConfig.mock.calls[1][0].expectedRevision).toBe('rev-2') + }) + + it('adds a model to a new API provider through a providers patch', async () => { + mockGetManagedOpenCodeConfig.mockResolvedValue({ revision: 'rev-1', info: {} }) + mockPatchOpenCodeConfig.mockResolvedValue({ ...defaultConfig, revision: 'rev-2' }) + + const user = userEvent.setup() + renderWithQuery() + + await screen.findByText('opencode.json') + await user.click(screen.getByRole('button', { name: /Models/i })) + await user.click(screen.getByRole('button', { name: /Add Model/i })) + + fireEvent.change(await screen.findByPlaceholderText('e.g., my-provider'), { target: { value: 'my-provider' } }) + fireEvent.change(screen.getByPlaceholderText('e.g., My Provider'), { target: { value: 'My Provider' } }) + fireEvent.change(screen.getByPlaceholderText('e.g., https://api.openai.com/v1'), { target: { value: 'https://api.example.com/v1' } }) + fireEvent.change(screen.getByPlaceholderText('e.g., qwen-3.5-27b'), { target: { value: 'gpt-5' } }) + fireEvent.change(screen.getByPlaceholderText('e.g., MiniMax-M2.7'), { target: { value: 'gpt-5-model' } }) + fireEvent.change(screen.getByPlaceholderText('e.g., Qwen3.5-27B'), { target: { value: 'GPT-5' } }) + + const createButton = screen.getByRole('button', { name: /create/i }) + await waitFor(() => expect(createButton).not.toBeDisabled()) + await user.click(createButton) + + await waitFor(() => expect(mockPatchOpenCodeConfig).toHaveBeenCalledWith({ + patch: { + providers: { + 'my-provider': { + name: 'My Provider', + settings: { baseURL: 'https://api.example.com/v1' }, + models: { + 'gpt-5': expect.objectContaining({ modelID: 'gpt-5-model', name: 'GPT-5' }), + }, + }, + }, + }, + expectedRevision: 'rev-1', + })) + }) + + it('saves an edited model under providers when the managed response converted a v1 entry', async () => { + mockGetManagedOpenCodeConfig.mockResolvedValue({ + revision: 'rev-1', + info: { + providers: { + openai: { + name: 'OpenAI', + models: { 'gpt-4o': { name: 'GPT-4o', modelID: 'gpt-4o' } }, + }, + }, + }, + }) + mockPatchOpenCodeConfig.mockResolvedValue({ ...defaultConfig, revision: 'rev-2' }) + + const user = userEvent.setup() + renderWithQuery() + + await screen.findByText('opencode.json') + await user.click(screen.getByRole('button', { name: /Models/i })) + await user.click(await screen.findByText('GPT-4o')) + + fireEvent.change(await screen.findByPlaceholderText('e.g., Qwen3.5-27B'), { target: { value: 'GPT-4o Updated' } }) + await user.click(screen.getByRole('button', { name: /update/i })) + + await waitFor(() => expect(mockPatchOpenCodeConfig).toHaveBeenCalledWith({ + patch: { + providers: { + openai: { + name: 'OpenAI', + models: { + 'gpt-4o': expect.objectContaining({ name: 'GPT-4o Updated', modelID: 'gpt-4o' }), + }, + }, + }, + }, + expectedRevision: 'rev-1', + })) + }) + + it('keeps a converted provider and its sibling models when deleting one model', async () => { + mockGetManagedOpenCodeConfig.mockResolvedValue({ + revision: 'rev-1', + info: { + providers: { + p: { + name: 'Provider P', + settings: { baseURL: 'http://x' }, + models: { + a: { name: 'Model A' }, + b: { name: 'Model B' }, + }, + }, + }, + }, + }) + mockPatchOpenCodeConfig.mockResolvedValue({ ...defaultConfig, revision: 'rev-2' }) + + const user = userEvent.setup() + renderWithQuery() + + await screen.findByText('opencode.json') + await user.click(screen.getByRole('button', { name: /Models/i })) + await user.click(await screen.findByLabelText('Actions for Model A')) + await user.click(screen.getByText('Delete')) + + await waitFor(() => expect(mockPatchOpenCodeConfig).toHaveBeenCalledWith({ + patch: { + providers: { + p: { + name: 'Provider P', + settings: { baseURL: 'http://x' }, + models: { + a: null, + b: { name: 'Model B' }, + }, + }, + }, + }, + expectedRevision: 'rev-1', + })) }) it('reports a comment-only raw save as applied without a restart', async () => { @@ -512,7 +648,7 @@ describe('OpenCodeConfigManager', () => { }) it('asks for a restart after a semantic structured change', async () => { - mockUpdateOpenCodeConfig.mockResolvedValueOnce({ ...defaultConfig, restartRequired: true }) + mockPatchOpenCodeConfig.mockResolvedValueOnce({ ...defaultConfig, restartRequired: true }) const user = userEvent.setup() renderWithQuery() diff --git a/frontend/src/components/settings/OpenCodeConfigManager.tsx b/frontend/src/components/settings/OpenCodeConfigManager.tsx index 4a455ab85..2211d9bce 100644 --- a/frontend/src/components/settings/OpenCodeConfigManager.tsx +++ b/frontend/src/components/settings/OpenCodeConfigManager.tsx @@ -11,7 +11,7 @@ import { AgentsEditor } from './AgentsEditor' import { AgentsMdEditor } from './AgentsMdEditor' import { McpManager } from './McpManager' import { SkillsEditor } from './SkillsEditor' -import { OpenCodeModelsEditor, type ConfigProvider } from './OpenCodeModelsEditor' +import { OpenCodeModelsEditor } from './OpenCodeModelsEditor' import { scrollSectionIntoView } from '@/lib/settingsScroll' import { VersionSelectDialog } from './VersionSelectDialog' import { settingsApi } from '@/api/settings' @@ -85,6 +85,8 @@ export function OpenCodeConfigManager() { const { data: managedConfig, error: managedConfigError } = useOpenCodeManagedConfig() const managedAgents = managedConfig?.info.agents ?? {} const managedCommands = managedConfig?.info.commands ?? {} + const managedProviders = managedConfig?.info.providers ?? {} + const managedMcpServers = mcpServersFromConfig(managedConfig?.info.mcp) const isManagedConfigUnavailable = managedConfigError instanceof FetchError && managedConfigError.statusCode === 503 const { data: managedSkills = [] } = useQuery({ @@ -145,23 +147,6 @@ export function OpenCodeConfigManager() { }) } - const updateConfigContent = async (newContent: Record) => { - const expectedRevision = queryClient.getQueryData( - OPEN_CODE_CONFIG_QUERY_KEY, - )?.revision - const result = await settingsApi.updateOpenCodeConfig({ - content: newContent, - expectedRevision, - }) - applyOpenCodeConfigSave(result) - } - - const updateConfigContentSafely = (newContent: Record) => { - void updateConfigContent(newContent).catch((error) => { - showToast.error(getApiErrorMessage(error, 'Failed to update config')) - }) - } - const patchManagedConfig = async (patch: Record) => { const expectedRevision = queryClient.getQueryData( OPEN_CODE_CONFIG_QUERY_KEY, @@ -478,7 +463,7 @@ export function OpenCodeConfigManager() {

MCP Servers

- {Object.keys(mcpServersFromConfig(config.content.mcp)).length} configured + {Object.keys(managedMcpServers).length} configured
@@ -487,10 +472,17 @@ export function OpenCodeConfigManager() { id={mcpContentId} className={cn(SECTION_CONTENT_CLASS, expandedSections.mcp ? 'block' : 'hidden')} > - + {isManagedConfigUnavailable ? ( +

+ OpenCode is not running, so the configured MCP servers cannot be listed. Use Edit to change the config file directly. +

+ ) : ( + patchManagedConfig({ mcp: { servers: { [serverId]: server } } })} + onDelete={(serverId) => patchManagedConfig({ mcp: { servers: { [serverId]: null } } })} + /> + )}
@@ -506,14 +498,10 @@ export function OpenCodeConfigManager() {

Models

- {(() => { - const provider = config.content.provider as Record | undefined - if (!provider) return 0 - return Object.values(provider).reduce((acc, p) => { - const models = (p as { models?: Record })?.models - return acc + (models ? Object.keys(models).length : 0) - }, 0) - })()} configured + {Object.values(managedProviders).reduce( + (acc, provider) => acc + Object.keys(provider.models ?? {}).length, + 0, + )} configured
@@ -522,15 +510,16 @@ export function OpenCodeConfigManager() { id={modelsContentId} className={cn(SECTION_CONTENT_CLASS, expandedSections.models ? 'block' : 'hidden')} > - | undefined) ?? {}} - onChange={(providers) => { - updateConfigContentSafely({ - ...config.content, - provider: providers - }) - }} - /> + {isManagedConfigUnavailable ? ( +

+ OpenCode is not running, so the configured models cannot be listed. Use Edit to change the config file directly. +

+ ) : ( + patchManagedConfigSafely({ providers })} + /> + )} diff --git a/frontend/src/components/settings/OpenCodeModelDialog.tsx b/frontend/src/components/settings/OpenCodeModelDialog.tsx index 7583e622a..0e1dbc119 100644 --- a/frontend/src/components/settings/OpenCodeModelDialog.tsx +++ b/frontend/src/components/settings/OpenCodeModelDialog.tsx @@ -13,70 +13,99 @@ import { Combobox } from '@/components/ui/combobox' import { Label } from '@/components/ui/label' import { RefreshCw, Loader2 } from 'lucide-react' import { settingsApi } from '@/api/settings' -import type { ModelConfig, ProviderConfig } from '@/api/types/settings' - -type ConfigModel = Partial & { - limit?: { - context?: number - input?: number - output?: number - } -} & Record - -type ConfigProvider = Omit, 'models' | 'env'> & { - api?: string - npm?: string - env?: string[] - models?: Record -} & Record +import type { OpenCodeProviderConfig, OpenCodeProviderModelConfig } from '@opencode-manager/shared/opencode' +import type { OpenCodeProviderModelPatch } from '@/api/types/settings' const handledModelKeys = new Set([ - 'id', - 'providerID', - 'api', + 'modelID', 'name', 'family', + 'releaseDate', + 'package', + 'settings', + 'headers', 'capabilities', 'cost', - 'limit', - 'status', - 'options', - 'headers', - 'release_date', 'variants', + 'limit', ]) -function parseJsonObject(value: string): Record | null { +type JsonObject = Record + +function parseJson(value: string): unknown { try { - const parsed = JSON.parse(value) as unknown - if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { - return parsed as Record - } - return null + return JSON.parse(value) as unknown } catch { return null } } +function parseJsonObject(value: string): JsonObject | null { + const parsed = parseJson(value) + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { + return parsed as JsonObject + } + return null +} + +function parseJsonContainer(value: string): JsonObject | unknown[] | null { + const parsed = parseJson(value) + if (Array.isArray(parsed)) return parsed + if (parsed && typeof parsed === 'object') return parsed as JsonObject + return null +} + +function parseJsonArray(value: string): unknown[] | null { + const parsed = parseJson(value) + return Array.isArray(parsed) ? parsed : null +} + function stringifyJson(value: unknown): string { - if (!value || (typeof value === 'object' && !Array.isArray(value) && Object.keys(value as Record).length === 0)) { + if (!value || (typeof value === 'object' && !Array.isArray(value) && Object.keys(value as JsonObject).length === 0)) { return '' } return JSON.stringify(value, null, 2) } -function parseOptionalJsonField(value: string): Record | undefined { +function parseOptionalJsonField(value: string): JsonObject | undefined { if (!value.trim()) return undefined return parseJsonObject(value) ?? undefined } +function parseOptionalJsonContainerField(value: string): JsonObject | unknown[] | undefined { + if (!value.trim()) return undefined + return parseJsonContainer(value) ?? undefined +} + +function parseOptionalJsonArrayField(value: string): unknown[] | undefined { + if (!value.trim()) return undefined + return parseJsonArray(value) ?? undefined +} + function parseOptionalNumber(value: string): number | undefined { if (!value.trim()) return undefined const parsed = Number(value) return Number.isFinite(parsed) ? parsed : undefined } +function isPlainObject(value: unknown): value is Record { + return Boolean(value) && typeof value === 'object' && !Array.isArray(value) +} + +function withRemovedKeysAsNull(previous: unknown, next: unknown): unknown { + if (!isPlainObject(previous) || !isPlainObject(next)) return next + + const patch: Record = {} + for (const key of Object.keys(previous)) { + if (!(key in next)) patch[key] = null + } + for (const [key, value] of Object.entries(next)) { + patch[key] = withRemovedKeysAsNull(previous[key], value) + } + return patch +} + function sanitizeModelId(modelId: string): string { return modelId.replace(/[^a-zA-Z0-9._-]/g, '-') } @@ -101,32 +130,53 @@ function jsonObjectField(label: string) { }) } +function jsonContainerField(label: string) { + return z.string().superRefine((value, ctx) => { + if (!value.trim()) return + if (!parseJsonContainer(value)) { + ctx.addIssue({ + code: 'custom', + message: `${label} must be a valid JSON object or array`, + }) + } + }) +} + +function jsonArrayField(label: string) { + return z.string().superRefine((value, ctx) => { + if (!value.trim()) return + if (!parseJsonArray(value)) { + ctx.addIssue({ + code: 'custom', + message: `${label} must be a valid JSON array`, + }) + } + }) +} + const modelFormSchema = z.object({ providerId: z.string(), modelId: z.string().min(1, 'Model ID is required').regex(/^[a-zA-Z0-9._-]+$/, 'Must use only letters, numbers, dots, hyphens, and underscores'), backingModelId: z.string(), - providerModelProviderId: z.string(), displayName: z.string(), family: z.string(), - status: z.enum(['none', 'alpha', 'beta', 'deprecated', 'active']), releaseDate: z.string(), - apiUrl: z.string(), - apiNpm: z.string(), + modelPackage: z.string(), contextLimit: z.string(), inputLimit: z.string(), outputLimit: z.string(), - capabilitiesJson: jsonObjectField('Capabilities'), - costJson: jsonObjectField('Cost'), - optionsJson: jsonObjectField('Options'), + settingsJson: jsonObjectField('Settings'), headersJson: jsonObjectField('Headers'), - variantsJson: jsonObjectField('Variants'), + capabilitiesJson: jsonObjectField('Capabilities'), + costJson: jsonContainerField('Cost'), + variantsJson: jsonArrayField('Variants'), extraJson: jsonObjectField('Advanced fields'), createNewProvider: z.boolean(), newProviderType: z.enum(['api', 'npm']), newProviderId: z.string(), newProviderName: z.string().optional(), newProviderBaseUrl: z.string().optional(), - newProviderNpm: z.string().optional(), + newProviderPackage: z.string().optional(), }).superRefine((data, ctx) => { if (data.createNewProvider) { if (!data.newProviderId?.trim()) { @@ -137,8 +187,8 @@ const modelFormSchema = z.object({ if (data.newProviderType === 'api' && !data.newProviderBaseUrl?.trim()) { ctx.addIssue({ code: 'custom', message: 'Base URL is required for API providers', path: ['newProviderBaseUrl'] }) } - if (data.newProviderType === 'npm' && !data.newProviderNpm?.trim()) { - ctx.addIssue({ code: 'custom', message: 'NPM package is required for npm providers', path: ['newProviderNpm'] }) + if (data.newProviderType === 'npm' && !data.newProviderPackage?.trim()) { + ctx.addIssue({ code: 'custom', message: 'NPM package is required for npm providers', path: ['newProviderPackage'] }) } } else { if (!data.providerId?.trim()) { @@ -165,20 +215,20 @@ export interface NewProviderConfig { type: 'api' | 'npm' name?: string baseUrl?: string - npm?: string + package?: string } interface OpenCodeModelDialogProps { open: boolean onOpenChange: (open: boolean) => void - onSubmit: (providerId: string, modelId: string, model: ConfigModel, newProvider?: NewProviderConfig) => void + onSubmit: (providerId: string, modelId: string, model: OpenCodeProviderModelPatch, newProvider?: NewProviderConfig) => void availableProviders: string[] - existingProviders?: Record + existingProviders?: Record selectedProviderId: string editingModel?: { providerId: string modelId: string - model: ConfigModel + model: OpenCodeProviderModelConfig } } @@ -200,21 +250,18 @@ export function OpenCodeModelDialog({ return { providerId: editingModel.providerId, modelId: editingModel.modelId, - backingModelId: typeof editingModel.model.id === 'string' ? editingModel.model.id : '', - providerModelProviderId: typeof editingModel.model.providerID === 'string' ? editingModel.model.providerID : '', + backingModelId: editingModel.model.modelID || '', displayName: editingModel.model.name || '', family: editingModel.model.family || '', - status: (editingModel.model.status as ModelFormValues['status']) || 'none', - releaseDate: editingModel.model.release_date || '', - apiUrl: editingModel.model.api?.url || '', - apiNpm: editingModel.model.api?.npm || '', + releaseDate: editingModel.model.releaseDate || '', + modelPackage: editingModel.model.package || '', contextLimit: editingModel.model.limit?.context?.toString() || '', inputLimit: editingModel.model.limit?.input?.toString() || '', outputLimit: editingModel.model.limit?.output?.toString() || '', + settingsJson: stringifyJson(editingModel.model.settings), + headersJson: stringifyJson(editingModel.model.headers), capabilitiesJson: stringifyJson(editingModel.model.capabilities), costJson: stringifyJson(editingModel.model.cost), - optionsJson: stringifyJson(editingModel.model.options), - headersJson: stringifyJson(editingModel.model.headers), variantsJson: stringifyJson(editingModel.model.variants), extraJson: stringifyJson(extraEntries), createNewProvider: false, @@ -222,7 +269,7 @@ export function OpenCodeModelDialog({ newProviderId: '', newProviderName: '', newProviderBaseUrl: '', - newProviderNpm: '', + newProviderPackage: '', } } @@ -230,20 +277,17 @@ export function OpenCodeModelDialog({ providerId: selectedProviderId || availableProviders[0] || '', modelId: '', backingModelId: '', - providerModelProviderId: '', displayName: '', family: '', - status: 'none', releaseDate: '', - apiUrl: '', - apiNpm: '', + modelPackage: '', contextLimit: '', inputLimit: '', outputLimit: '', + settingsJson: '', + headersJson: '', capabilitiesJson: '', costJson: '', - optionsJson: '', - headersJson: '', variantsJson: '', extraJson: '', createNewProvider: availableProviders.length === 0, @@ -251,7 +295,7 @@ export function OpenCodeModelDialog({ newProviderId: '', newProviderName: '', newProviderBaseUrl: '', - newProviderNpm: '', + newProviderPackage: '', } }, [editingModel, selectedProviderId, availableProviders]) @@ -280,9 +324,8 @@ export function OpenCodeModelDialog({ if (createNewProvider) { return watchedNewProviderBaseUrl?.trim() || '' } - const provider = existingProviders?.[watchedProviderId] - const options = provider?.options as { baseURL?: string } | undefined - return (options?.baseURL || provider?.api || '').trim() + const baseURL = existingProviders?.[watchedProviderId]?.settings?.baseURL + return typeof baseURL === 'string' ? baseURL.trim() : '' }, [createNewProvider, watchedNewProviderBaseUrl, watchedProviderId, existingProviders]) const discoverModels = useCallback(async (forceRefresh = false) => { @@ -353,43 +396,41 @@ export function OpenCodeModelDialog({ const handleSubmit = (values: ModelFormValues) => { const extra = parseOptionalJsonField(values.extraJson) - const capabilities = parseOptionalJsonField(values.capabilitiesJson) - const cost = parseOptionalJsonField(values.costJson) - const options = parseOptionalJsonField(values.optionsJson) + const settings = parseOptionalJsonField(values.settingsJson) const headers = parseOptionalJsonField(values.headersJson) - const variants = parseOptionalJsonField(values.variantsJson) - - const model: ConfigModel = { - ...(extra || {}), - } - - if (values.backingModelId.trim()) model.id = values.backingModelId.trim() - if (values.providerModelProviderId.trim()) model.providerID = values.providerModelProviderId.trim() - if (values.displayName.trim()) model.name = values.displayName.trim() - if (values.family.trim()) model.family = values.family.trim() - if (values.status !== 'none') model.status = values.status - if (values.releaseDate.trim()) model.release_date = values.releaseDate.trim() - - if (values.apiUrl.trim() || values.apiNpm.trim()) { - model.api = { url: values.apiUrl.trim(), ...(values.apiNpm.trim() ? { npm: values.apiNpm.trim() } : {}) } + const capabilities = parseOptionalJsonField(values.capabilitiesJson) + const cost = parseOptionalJsonContainerField(values.costJson) + const variants = parseOptionalJsonArrayField(values.variantsJson) + + const builtModel: OpenCodeProviderModelPatch = { + ...(extra ?? {}), + modelID: values.backingModelId.trim() || null, + name: values.displayName.trim() || null, + family: values.family.trim() || null, + releaseDate: values.releaseDate.trim() || null, + package: values.modelPackage.trim() || null, } const contextLimit = parseOptionalNumber(values.contextLimit) const inputLimit = parseOptionalNumber(values.inputLimit) const outputLimit = parseOptionalNumber(values.outputLimit) - if (contextLimit !== undefined || inputLimit !== undefined || outputLimit !== undefined) { - model.limit = { - ...(contextLimit !== undefined ? { context: contextLimit } : {}), - ...(inputLimit !== undefined ? { input: inputLimit } : {}), - ...(outputLimit !== undefined ? { output: outputLimit } : {}), - } as ConfigModel['limit'] - } - - if (capabilities) model.capabilities = capabilities as ConfigModel['capabilities'] - if (cost) model.cost = cost as ConfigModel['cost'] - if (options) model.options = options - if (headers) model.headers = headers as Record - if (variants) model.variants = variants as ConfigModel['variants'] + builtModel.limit = contextLimit !== undefined || inputLimit !== undefined || outputLimit !== undefined + ? { + ...(contextLimit !== undefined ? { context: contextLimit } : {}), + ...(inputLimit !== undefined ? { input: inputLimit } : {}), + ...(outputLimit !== undefined ? { output: outputLimit } : {}), + } + : null + + builtModel.settings = (settings as OpenCodeProviderModelConfig['settings'] | undefined) ?? null + builtModel.headers = (headers as Record | undefined) ?? null + builtModel.capabilities = (capabilities as OpenCodeProviderModelConfig['capabilities'] | undefined) ?? null + builtModel.cost = (cost as OpenCodeProviderModelConfig['cost'] | undefined) ?? null + builtModel.variants = (variants as OpenCodeProviderModelConfig['variants'] | undefined) ?? null + + const model = editingModel + ? withRemovedKeysAsNull(editingModel.model, builtModel) as OpenCodeProviderModelPatch + : builtModel let newProvider: NewProviderConfig | undefined if (values.createNewProvider) { @@ -398,7 +439,7 @@ export function OpenCodeModelDialog({ type: values.newProviderType, name: values.newProviderName || undefined, baseUrl: values.newProviderBaseUrl || undefined, - npm: values.newProviderNpm || undefined, + package: values.newProviderPackage || undefined, } } @@ -516,7 +557,7 @@ export function OpenCodeModelDialog({ )} {newProviderType === 'npm' && ( - ( + ( NPM Package @@ -596,7 +637,7 @@ export function OpenCodeModelDialog({ )} /> -
+
( Display Name @@ -612,64 +653,23 @@ export function OpenCodeModelDialog({ )} /> -
-
- ( + ( - Status - - - - )} /> - - ( - - Release Date - + Package + )} />
-
- ( - - Model Provider ID - - - - )} /> - -
- ( - - API URL - - - - )} /> - - ( - - API NPM - - - - )} /> -
-
+ ( + + Release Date + + + + )} />
( @@ -700,9 +700,9 @@ export function OpenCodeModelDialog({

Structured JSON fields

- ( + ( - Options JSON + Settings JSON