diff --git a/backend/src/index.ts b/backend/src/index.ts index 817753fc..6b6d7ba0 100644 --- a/backend/src/index.ts +++ b/backend/src/index.ts @@ -55,7 +55,8 @@ import { installAssistantWorkspace } from './services/assistant-mode' import { detectSandboxCapability } from './services/sandbox/capability' import { SandboxRuntimeService, stopWorkspaceSandboxOnShutdown } from './services/sandbox/runtime' import { getOpenCodeImportStatus, syncOpenCodeImport } from './services/opencode-import' -import { readOpenCodeConfigFile, writeOpenCodeConfigFile, OPENCODE_CONFIG_SEED } from './services/opencode-config-file' +import { readOpenCodeConfigFile } from './services/opencode-config-file' +import { seedOpenCodeConfigFile } from './services/opencode-config-apply' import { OpenCodeSupervisor } from './services/opencode-supervisor' import { OpenCodeRestartCoordinator } from './services/opencode-restart-coordinator' import { setOpenCodeRestartCoordinator } from './services/opencode-restart' @@ -133,7 +134,7 @@ async function ensureOpenCodeConfigFileExists(): Promise { } } - await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED) + await seedOpenCodeConfigFile() logger.info('Created minimal seed config') } diff --git a/backend/src/routes/opencode-config.ts b/backend/src/routes/opencode-config.ts index 6b6bc0f7..1c9dfbc0 100644 --- a/backend/src/routes/opencode-config.ts +++ b/backend/src/routes/opencode-config.ts @@ -2,8 +2,14 @@ import { Hono } from 'hono' import { z } from 'zod' import { UpdateOpenCodeConfigRequestSchema } from '@opencode-manager/shared/schemas' import type { SettingsService } from '../services/settings' -import type { OpenCodeClient } from '../services/opencode/client' -import { readOpenCodeConfigFile } from '../services/opencode-config-file' +import { UpstreamError, type OpenCodeClient } from '../services/opencode/client' +import { + OpenCodeConfigConflictError, + OpenCodeConfigShadowedRemovalError, + OpenCodeConfigSourceInvalidError, + readOpenCodeConfigFile, + withOpenCodeConfigLock, +} from '../services/opencode-config-file' import { applyOpenCodeConfigUpdate, toOpenCodeConfigApplyResponse } from '../services/opencode-config-apply' import { logger } from '../utils/logger' @@ -12,7 +18,7 @@ export function createOpenCodeConfigRoutes(settingsService: SettingsService, ope app.get('/', async (c) => { try { - const config = await readOpenCodeConfigFile() + const config = await withOpenCodeConfigLock(readOpenCodeConfigFile) if (!config) { return c.json({ error: 'No OpenCode config file found' }, 404) } @@ -23,6 +29,22 @@ export function createOpenCodeConfigRoutes(settingsService: SettingsService, ope } }) + app.get('/effective', async (c) => { + try { + const config = await openCodeClient.getJson>('/global/config') + return c.json(config) + } catch (error) { + logger.error('Failed to get effective OpenCode config:', error) + if (error instanceof UpstreamError) { + if (error.status === 502) { + 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) + } + }) + app.put('/', async (c) => { let body: unknown try { @@ -39,16 +61,33 @@ export function createOpenCodeConfigRoutes(settingsService: SettingsService, ope try { const result = await applyOpenCodeConfigUpdate({ content: parsed.data.content, - openCodeClient, + source: parsed.data.source, + expectedRevision: parsed.data.expectedRevision, settingsService, }) const { status, body: responseBody } = toOpenCodeConfigApplyResponse(result) return c.json(responseBody, status) } catch (error) { logger.error('Failed to update OpenCode config:', error) + if (error instanceof OpenCodeConfigConflictError) { + return c.json({ + error: error.message, + expectedRevision: error.expectedRevision, + actualRevision: error.actualRevision, + }, 409) + } + if (error instanceof OpenCodeConfigSourceInvalidError) { + return c.json({ error: error.message, sources: error.sources }, 400) + } + if (error instanceof OpenCodeConfigShadowedRemovalError) { + return c.json({ error: error.message, paths: error.paths, sources: error.sources }, 409) + } if (error instanceof z.ZodError) { return c.json({ error: 'Invalid config data', details: error.issues }, 400) } + if (error instanceof SyntaxError) { + return c.json({ error: 'Invalid config data', details: error.message }, 400) + } return c.json({ error: 'Failed to update OpenCode config' }, 500) } }) diff --git a/backend/src/routes/settings.ts b/backend/src/routes/settings.ts index a87504c8..920c02ba 100644 --- a/backend/src/routes/settings.ts +++ b/backend/src/routes/settings.ts @@ -7,7 +7,7 @@ import { resolve, dirname } from 'path' import type { Database } from 'bun:sqlite' import { SettingsService } from '../services/settings' import { writeFileContent, readFileContent, fileExists } from '../services/file-operations' -import { deleteOpenCodeConfigFile } from '../services/opencode-config-file' +import { archiveBrokenOpenCodeConfigFile, deleteOpenCodeConfigFile } from '../services/opencode-config-file' import { restoreLastKnownGoodOpenCodeConfig } from '../services/opencode-config-apply' import { createOpenCodeConfigRoutes } from './opencode-config' import type { OpenCodeClient } from '../services/opencode/client' @@ -517,8 +517,12 @@ export function createSettingsRoutes(db: Database, gitAuthService: GitAuthServic app.post('/opencode-reload', async (c) => { try { logger.info('OpenCode configuration reload requested') - await reloadOpenCodeConfig(openCodeSupervisor) - return c.json({ success: true, message: 'OpenCode configuration reloaded successfully' }) + const { resumedSessionIDs } = await reloadOpenCodeConfig(openCodeSupervisor) + return c.json({ + success: true, + message: 'OpenCode server restarted with the current configuration', + resumedSessions: resumedSessionIDs, + }) } catch (error) { logger.error('Failed to reload OpenCode config:', error) if (error instanceof ConfigReloadError) { @@ -529,7 +533,6 @@ export function createSettingsRoutes(db: Database, gitAuthService: GitAuthServic error: error.message, details, validationIssues: error.validationIssues, - removedFields: error.removedFields }, 500) } return c.json({ @@ -555,6 +558,7 @@ export function createSettingsRoutes(db: Database, gitAuthService: GitAuthServic } catch (reloadError) { logger.error('Rollback config reload failed, attempting restart:', reloadError) + await archiveBrokenOpenCodeConfigFile() const deleted = await deleteOpenCodeConfigFile() if (deleted) { logger.info('Deleted filesystem config, attempting restart with fallback') diff --git a/backend/src/services/assistant-mode.ts b/backend/src/services/assistant-mode.ts index 00dd894d..650835de 100644 --- a/backend/src/services/assistant-mode.ts +++ b/backend/src/services/assistant-mode.ts @@ -715,21 +715,23 @@ Reload the assistant workspace by disposing the current OpenCode instance. Use t ## OpenCode Configuration -The OpenCode configuration file on disk is the source of truth, and this endpoint is the only supported way to change it. Never edit \`opencode.json\` directly. +The global configuration files on disk are the source of truth. Use the \`ocm\` tool's \`request\` action with the endpoints below to read or change them; never edit the files directly. Global sources merge in order: \`config.json\`, \`opencode.json\`, then \`opencode.jsonc\`. ### GET /opencode-config -Read the current configuration file. Returns \`404\` when no config file exists yet. +Read the merged persisted global configuration and its source files. Returns \`404\` when no source exists. This is not the running instance configuration: project overrides and expanded environment values are not included. \`GET /opencode-config/effective\` reads the running server's effective global configuration separately; never copy that response into a save. **Response (\`OpenCodeConfigFile\`):** \`\`\`ts { - path: string // Absolute path of the configuration file - content: object // Parsed configuration - rawContent: string // Raw file content, including comments - isValid: boolean // Whether the file passes schema validation + path: string + content: object + rawContent: string + sources: Array<{ name: string, path: string, rawContent: string, content: object, isValid: boolean }> + revision: string + isValid: boolean validationIssues?: Array<{ path: string, message: string }> - updatedAt: number // Unix timestamp of the last write + updatedAt: number } \`\`\` @@ -746,11 +748,13 @@ Read the current configuration file. Returns \`404\` when no config file exists ### PUT /opencode-config -Persist a complete configuration. Read the file first, change only the keys the user asked for, and send the complete object back. +Read the merged persisted configuration first, change only the keys the user asked for, and send the complete object back with its revision. Only changed fields are patched into the preferred existing source: JSONC, JSON, then legacy config.json. New installations use opencode.jsonc. Unchanged inherited values and comments are preserved. Removing a field removes only its override in the write target; a lower-priority value can reappear. + +For a raw edit, send a string with the exact source name from \`sources\`. Never send merged JSON as raw source text. A \`409\` means the source files changed: read again and reconcile rather than retrying stale content. **Request Body:** \`\`\`ts -{ content: object } // The complete configuration to persist +{ content: object | string, expectedRevision: string, source?: "config.json" | "opencode.json" | "opencode.jsonc" } \`\`\` **Example:** @@ -761,6 +765,7 @@ Persist a complete configuration. Read the file first, change only the keys the "method": "PUT", "path": "/opencode-config", "body": { + "expectedRevision": "revision-from-get", "content": { "theme": "dark" } @@ -770,16 +775,16 @@ Persist a complete configuration. Read the file first, change only the keys the \`\`\` **Response:** -Returns the written \`OpenCodeConfigFile\`. Adds \`restartRequired: true\` when the change needs an OpenCode server restart, and \`removedFields\` when OpenCode dropped fields it does not accept. +Returns the refreshed merged configuration and source files. Adds \`restartRequired: true\` for semantic configuration changes, except changes limited to \`mcp\`, which are saved without it; to make an MCP change take effect immediately, tell the user to reconnect or reload the server from Settings → MCP. Comment-only changes do not require a restart. Saving never silently drops unsupported fields. -Returns \`400\` with \`validationIssues\` when OpenCode rejects the configuration. +Returns \`400\` for invalid configuration and \`409\` for a stale revision. When the response contains \`restartRequired: true\`, tell the user to restart the OpenCode server from Settings. Never attempt the restart yourself: it would terminate your own session. ## Safety - The settings PATCH endpoint rejects any attempt to modify credentials, API keys, or other sensitive settings; guide the user to the full UI for Git, TTS, and STT credentials -- PUT /opencode-config writes the complete OpenCode configuration, including \`plugin\`, \`mcp\`, and \`provider\` entries; change only the keys the user explicitly asked for and never add plugins, MCP servers, or provider credentials the user did not request +- PUT /opencode-config patches changed global settings, including \`plugin\`, \`mcp\`, and \`provider\` entries; change only the keys the user explicitly asked for and never add plugins, MCP servers, or provider credentials the user did not request - The settings PATCH endpoint does NOT trigger OpenCode reload or restart ` } diff --git a/backend/src/services/opencode-config-apply.ts b/backend/src/services/opencode-config-apply.ts index ccc9238e..c4b0e154 100644 --- a/backend/src/services/opencode-config-apply.ts +++ b/backend/src/services/opencode-config-apply.ts @@ -1,27 +1,35 @@ -import { OpenCodeConfigSchema } from '@opencode-manager/shared/schemas' -import { parseJsonc } from '@opencode-manager/shared/utils' -import type { OpenCodeConfigFile, OpenCodeConfigInput } from '../types/settings' -import { OPENCODE_CONFIG_SEED, normalizeOpenCodeConfigContent, readOpenCodeConfigFile, withOpenCodeConfigLock, writeOpenCodeConfigFile } from './opencode-config-file' -import { patchConfigWithRecovery, type PatchConfigValidationIssue } from './opencode/config-recovery' -import type { OpenCodeClient } from './opencode/client' +import { isDeepStrictEqual } from 'node:util' +import type { + OpenCodeConfigFile, + OpenCodeConfigSourceName, +} from '../types/settings' +import { + buildOpenCodeConfigSeedSnapshot, + readOpenCodeConfigFile, + readOpenCodeConfigSnapshot, + restoreOpenCodeConfigSnapshot, + serializeOpenCodeConfigSnapshot, + updateOpenCodeConfigFile, + withOpenCodeConfigLock, +} from './opencode-config-file' import { opencodeServerManager } from './opencode-single-server' import type { SettingsService } from './settings' export type ApplyOpenCodeConfigResult = | { status: 'restart_pending'; config: OpenCodeConfigFile } - | { status: 'applied'; config: OpenCodeConfigFile; removedFields: string[] } - | { status: 'rejected'; error: string; validationIssues: PatchConfigValidationIssue[]; removedFields: string[] } + | { status: 'applied'; config: OpenCodeConfigFile } export interface ApplyOpenCodeConfigInput { - content: OpenCodeConfigInput | string - openCodeClient: OpenCodeClient + content: Record | string + source?: OpenCodeConfigSourceName + expectedRevision?: string settingsService: SettingsService } export async function captureLastKnownGoodOpenCodeConfig(settingsService: SettingsService): Promise { const previous = await readOpenCodeConfigFile() if (previous?.isValid) { - settingsService.saveLastKnownGoodConfig(previous.rawContent) + settingsService.saveLastKnownGoodConfig(serializeOpenCodeConfigSnapshot(previous)) } return previous } @@ -32,90 +40,74 @@ export async function restoreLastKnownGoodOpenCodeConfig(settingsService: Settin return null } - const config = await withOpenCodeConfigLock(() => writeOpenCodeConfigFile(lastGood)) + const config = await withOpenCodeConfigLock(() => restoreOpenCodeConfigSnapshot(lastGood)) opencodeServerManager.clearStartupError() return config } export async function seedOpenCodeConfigFile(): Promise { - return withOpenCodeConfigLock(() => writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED)) -} - -function didConfigFieldChange( - previous: Record | undefined, - next: Record | undefined, - field: string, -): boolean { - return JSON.stringify(previous?.[field]) !== JSON.stringify(next?.[field]) -} - -function needsOpenCodeRestart( - previous: Record | undefined, - next: Record | undefined, -): boolean { - return ['agent', 'plugin', 'skills', 'provider'].some((field) => didConfigFieldChange(previous, next, field)) + return withOpenCodeConfigLock(async () => { + const config = await restoreOpenCodeConfigSnapshot(buildOpenCodeConfigSeedSnapshot()) + if (!config) { + throw new Error('Failed to seed OpenCode config') + } + return config + }) } export function toOpenCodeConfigApplyResponse( result: ApplyOpenCodeConfigResult, -): { status: 200 | 400; body: Record } { - if (result.status === 'restart_pending') { - return { status: 200, body: { ...result.config, restartRequired: true } } +): { status: 200; body: Record } { + return { + status: 200, + body: result.status === 'restart_pending' + ? { ...result.config, restartRequired: true } + : { ...result.config }, + } +} + +function requiresOpenCodeRestart(previous: OpenCodeConfigFile | null, next: OpenCodeConfigFile): boolean { + if (previous?.isValid !== next.isValid) { + return true } - if (result.status === 'applied') { - return { - status: 200, - body: result.removedFields.length > 0 - ? { ...result.config, removedFields: result.removedFields } - : { ...result.config }, + const previousContent = previous?.content ?? {} + const keys = new Set([...Object.keys(previousContent), ...Object.keys(next.content)]) + for (const key of keys) { + if (!isDeepStrictEqual(previousContent[key], next.content[key]) && key !== 'mcp') { + return true } } - return { - status: 400, - body: { - error: 'Config validation failed', - details: result.error, - validationIssues: result.validationIssues, - removedFields: result.removedFields, - }, - } + return false } export async function applyOpenCodeConfigUpdate( input: ApplyOpenCodeConfigInput, ): Promise { return withOpenCodeConfigLock(async () => { - const { content, openCodeClient, settingsService } = input + const { content, source, expectedRevision, settingsService } = input - const rawContent = normalizeOpenCodeConfigContent(content) - const nextContent = OpenCodeConfigSchema.parse(parseJsonc(rawContent)) + const snapshot = await readOpenCodeConfigSnapshot() + const previous = await readOpenCodeConfigFile(snapshot) - const previous = await captureLastKnownGoodOpenCodeConfig(settingsService) + const next = await updateOpenCodeConfigFile(content, { source, expectedRevision, snapshot }) - if (needsOpenCodeRestart(previous?.content, nextContent)) { - const config = await writeOpenCodeConfigFile(rawContent) - opencodeServerManager.markRestartPending() - return { status: 'restart_pending', config } - } - - const patchResult = await patchConfigWithRecovery(openCodeClient, nextContent) - if (!patchResult.success) { - return { - status: 'rejected', - error: patchResult.error ?? 'Config validation failed', - validationIssues: patchResult.details ?? [], - removedFields: patchResult.removedFields ?? [], + if (previous?.isValid) { + const snapshot = serializeOpenCodeConfigSnapshot(previous) + try { + settingsService.saveLastKnownGoodConfig(snapshot) + } catch (error) { + await restoreOpenCodeConfigSnapshot(snapshot) + throw error } } - const removedFields = patchResult.removedFields ?? [] - const contentToWrite = removedFields.length > 0 - ? JSON.stringify(patchResult.appliedConfig ?? nextContent, null, 2) - : rawContent - const config = await writeOpenCodeConfigFile(contentToWrite) + if (requiresOpenCodeRestart(previous, next)) { + opencodeServerManager.markRestartPending() + return { status: 'restart_pending', config: next } + } - return { status: 'applied', config, removedFields } + return { status: 'applied', config: next } }) } diff --git a/backend/src/services/opencode-config-file.ts b/backend/src/services/opencode-config-file.ts index fdc45563..585fc709 100644 --- a/backend/src/services/opencode-config-file.ts +++ b/backend/src/services/opencode-config-file.ts @@ -1,21 +1,86 @@ -import { readdir, rm, stat } from 'fs/promises' +import { createHash } from 'crypto' +import { readFile, readdir, 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 { getOpenCodeConfigFilePath, getOpenCodeHealthWatchPath } from '@opencode-manager/shared/config/env' +import { getConfigPath, getOpenCodeHealthWatchPath } from '@opencode-manager/shared/config/env' +import { + DEFAULT_OPENCODE_CONFIG_SOURCE_NAME, + OPENCODE_CONFIG_SOURCE_NAMES, + isOpenCodeConfigSourceName, + selectPreferredOpenCodeConfigSourceName, +} from '@opencode-manager/shared' import { OpenCodeConfigSchema } from '@opencode-manager/shared/schemas' import { parseJsonc } from '@opencode-manager/shared/utils' -import type { OpenCodeConfigFile, OpenCodeConfigInput, OpenCodeConfigValidationIssue } from '../types/settings' +import type { + OpenCodeConfigFile, + OpenCodeConfigSourceFile, + OpenCodeConfigSourceName, + OpenCodeConfigValidationIssue, +} from '../types/settings' import { logger } from '../utils/logger' import { withFileLock } from '../utils/atomic-json' import { existingFileMode, writeFileAtomic } from '../utils/fs-safe' -import { ensureDirectoryExists, fileExists, readFileContent, writeFileContent } from './file-operations' +import { ensureDirectoryExists } from './file-operations' export const OPENCODE_CONFIG_SEED = JSON.stringify({ $schema: 'https://opencode.ai/config.json' }, null, 2) export const HEALTH_WATCH_MAX_ENTRIES = 20 -export function withOpenCodeConfigLock(fn: () => Promise): Promise { - return withFileLock(getOpenCodeConfigFilePath(), fn) +const OPENCODE_CONFIG_SOURCE_ORDER: readonly OpenCodeConfigSourceName[] = OPENCODE_CONFIG_SOURCE_NAMES + +const OPENCODE_CONFIG_SNAPSHOT_VERSION = 1 + +const OPENCODE_CONFIG_SNAPSHOT_MARKER = 'opencode-config-snapshot' + +const OPENCODE_CONFIG_SNAPSHOT_ARTIFACT_PREFIX = 'opencode-config-broken' + +export interface UpdateOpenCodeConfigOptions { + source?: OpenCodeConfigSourceName + expectedRevision?: string + snapshot?: OpenCodeConfigSnapshot +} + +export class OpenCodeConfigConflictError extends Error { + readonly expectedRevision: string + readonly actualRevision: string + + constructor(expectedRevision: string, actualRevision: string) { + super('OpenCode config was modified by another writer') + this.name = 'OpenCodeConfigConflictError' + this.expectedRevision = expectedRevision + this.actualRevision = actualRevision + } +} + +export class OpenCodeConfigSourceInvalidError extends Error { + readonly sources: OpenCodeConfigSourceName[] + + constructor(sources: OpenCodeConfigSourceName[]) { + super(`OpenCode config source is invalid: ${sources.join(', ')}`) + this.name = 'OpenCodeConfigSourceInvalidError' + this.sources = sources + } +} + +export class OpenCodeConfigSnapshotError extends Error { + constructor(message: string) { + super(message) + this.name = 'OpenCodeConfigSnapshotError' + } +} + +export class OpenCodeConfigShadowedRemovalError extends Error { + readonly paths: string[] + readonly sources: OpenCodeConfigSourceName[] + + constructor(paths: string[], sources: OpenCodeConfigSourceName[], targetName: OpenCodeConfigSourceName) { + super(`Cannot remove ${paths.join(', ')}: defined in ${sources.join(', ')}, not in ${targetName}`) + this.name = 'OpenCodeConfigShadowedRemovalError' + this.paths = paths + this.sources = sources + } } interface OpenCodeConfigParseResult { @@ -24,8 +89,113 @@ interface OpenCodeConfigParseResult { validationIssues?: OpenCodeConfigValidationIssue[] } -export function normalizeOpenCodeConfigContent(content: OpenCodeConfigInput | string): string { - return typeof content === 'string' ? content : JSON.stringify(content, null, 2) +export interface OpenCodeConfigSnapshot { + sources: OpenCodeConfigSourceFile[] + content: Record + revision: string + updatedAt: number + path: string + rawContent: string +} + +interface OpenCodeConfigSourceFileState { + name: OpenCodeConfigSourceName + rawContent: string | null +} + +interface PreparedOpenCodeConfigSourceFileState extends OpenCodeConfigSourceFileState { + path: string + previousMode: number | undefined + previousRawContent: string | null +} + +interface AppliedOpenCodeConfigSourceFileState { + name: OpenCodeConfigSourceName + previousRawContent: string | null + previousMode: number | undefined +} + +interface OpenCodeConfigPathOperation { + path: JSONPath + value: unknown +} + +interface OpenCodeConfigSnapshotEnvelopeSource { + name: OpenCodeConfigSourceName + rawContent: string +} + +interface OpenCodeConfigSnapshotEnvelope { + marker: string + version: number + sources: OpenCodeConfigSnapshotEnvelopeSource[] +} + +function getOpenCodeConfigDirectory(): string { + return getConfigPath() +} + +function getOpenCodeConfigSourcePath(name: OpenCodeConfigSourceName): string { + return path.join(getOpenCodeConfigDirectory(), name) +} + +function assertOpenCodeConfigSourceName(value: string): OpenCodeConfigSourceName { + if (!isOpenCodeConfigSourceName(value)) { + throw new Error(`Unsupported OpenCode config source: ${value}`) + } + return value +} + +function isPlainObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value) +} + +function hasOwn(object: Record, key: string): boolean { + return Object.prototype.hasOwnProperty.call(object, key) +} + +function hasOpenCodeConfigPath(content: Record, jsonPath: JSONPath): boolean { + let current: unknown = content + for (const segment of jsonPath) { + if (typeof segment === 'number') { + if (!Array.isArray(current) || segment < 0 || segment >= current.length) return false + current = current[segment] + continue + } + if (!isPlainObject(current) || !hasOwn(current, segment)) return false + current = current[segment] + } + return true +} + +function defineOwnConfigValue(target: Record, key: string, value: unknown): void { + Object.defineProperty(target, key, { value, enumerable: true, writable: true, configurable: true }) +} + +function mergeOpenCodeConfigValues( + target: Record, + source: Record, +): Record { + const output: Record = {} + for (const key of Object.keys(target)) { + defineOwnConfigValue(output, key, target[key]) + } + for (const key of Object.keys(source)) { + const sourceValue = source[key] + const targetValue = hasOwn(output, key) ? output[key] : undefined + defineOwnConfigValue( + output, + key, + isPlainObject(targetValue) && isPlainObject(sourceValue) + ? mergeOpenCodeConfigValues(targetValue, sourceValue) + : sourceValue, + ) + } + return output +} + +export function withOpenCodeConfigLock(fn: () => Promise): Promise { + return withFileLock(getOpenCodeConfigDirectory(), fn) } export function toOpenCodeConfigValidationIssues(issues: ZodIssue[]): OpenCodeConfigValidationIssue[] { @@ -49,14 +219,12 @@ export function parseOpenCodeConfigContent(rawContent: string): OpenCodeConfigPa } } - const content = parsed && typeof parsed === 'object' && !Array.isArray(parsed) - ? parsed as Record - : {} + const content = isPlainObject(parsed) ? parsed : {} const validated = OpenCodeConfigSchema.safeParse(parsed) if (validated.success) { return { - content: validated.data as Record, + content, isValid: true, } } @@ -71,45 +239,370 @@ export function parseOpenCodeConfigContent(rawContent: string): OpenCodeConfigPa } } -export async function readOpenCodeConfigFile(): Promise { - const configPath = getOpenCodeConfigFilePath() +async function readOpenCodeConfigSourceFile(name: OpenCodeConfigSourceName): Promise { + const sourcePath = getOpenCodeConfigSourcePath(name) let updatedAt: number try { - const stats = await stat(configPath) + const stats = await stat(sourcePath) + if (!stats.isFile()) return null updatedAt = stats.mtimeMs } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - return null - } + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null throw error } - const rawContent = await readFileContent(configPath) + const rawContent = await readFile(sourcePath, 'utf8') return { - path: configPath, + name, + path: sourcePath, rawContent, ...parseOpenCodeConfigContent(rawContent), updatedAt, } } -export async function writeOpenCodeConfigFile(rawContent: string): Promise { - const parsed = OpenCodeConfigSchema.parse(parseJsonc(rawContent)) +function selectWritableSource(sources: OpenCodeConfigSourceFile[]): OpenCodeConfigSourceFile | null { + const name = selectPreferredOpenCodeConfigSourceName(sources.map((source) => source.name)) + return name ? sources.find((source) => source.name === name) ?? null : null +} - const configPath = getOpenCodeConfigFilePath() - await writeFileAtomic(configPath, rawContent, { mode: await existingFileMode(configPath) }) +function computeOpenCodeConfigRevision(sources: OpenCodeConfigSourceFile[]): string { + const hash = createHash('sha256') + const byName = new Map(sources.map((source) => [source.name, source])) + for (const name of OPENCODE_CONFIG_SOURCE_ORDER) { + const source = byName.get(name) + hash.update(name) + hash.update('\0') + hash.update(source ? '1' : '0') + hash.update('\0') + hash.update(source?.rawContent ?? '') + hash.update('\0') + } + return hash.digest('hex') +} - const stats = await stat(configPath) +export async function readOpenCodeConfigSnapshot(): Promise { + const discovered = await Promise.all(OPENCODE_CONFIG_SOURCE_ORDER.map(readOpenCodeConfigSourceFile)) + const sources = discovered.filter((source): source is OpenCodeConfigSourceFile => source !== null) + const content = sources.reduce>( + (merged, source) => mergeOpenCodeConfigValues(merged, source.content), + {}, + ) + const selected = selectWritableSource(sources) return { - path: configPath, - rawContent, - content: parsed as Record, - isValid: true, - updatedAt: stats.mtimeMs, + sources, + content, + revision: computeOpenCodeConfigRevision(sources), + updatedAt: sources.reduce((latest, source) => Math.max(latest, source.updatedAt), 0), + path: selected ? selected.path : getOpenCodeConfigSourcePath(DEFAULT_OPENCODE_CONFIG_SOURCE_NAME), + rawContent: selected ? selected.rawContent : '', + } +} + +function toOpenCodeConfigFile(snapshot: OpenCodeConfigSnapshot): OpenCodeConfigFile { + const validationIssues = snapshot.sources.flatMap((source) => source.validationIssues ?? []) + return { + path: snapshot.path, + rawContent: snapshot.rawContent, + content: snapshot.content, + isValid: snapshot.sources.every((source) => source.isValid), + validationIssues: validationIssues.length > 0 ? validationIssues : undefined, + updatedAt: snapshot.updatedAt, + sources: snapshot.sources, + revision: snapshot.revision, + } +} + +export async function readOpenCodeConfigFile( + snapshot?: OpenCodeConfigSnapshot, +): Promise { + const resolved = snapshot ?? await readOpenCodeConfigSnapshot() + if (resolved.sources.length === 0) return null + return toOpenCodeConfigFile(resolved) +} + +export async function writeOpenCodeConfigFile( + rawContent: string, + source?: OpenCodeConfigSourceName, + snapshot?: OpenCodeConfigSnapshot, +): Promise { + OpenCodeConfigSchema.parse(parseJsonc(rawContent)) + + const resolved = snapshot ?? await readOpenCodeConfigSnapshot() + const targetName = source !== undefined + ? assertOpenCodeConfigSourceName(source) + : selectWritableSource(resolved.sources)?.name ?? DEFAULT_OPENCODE_CONFIG_SOURCE_NAME + const targetPath = getOpenCodeConfigSourcePath(targetName) + + await writeFileAtomic(targetPath, rawContent, { mode: await existingFileMode(targetPath) }) + + return toOpenCodeConfigFile(await readOpenCodeConfigSnapshot()) +} + +function collectOpenCodeConfigPathOperations( + requested: Record, + current: Record, + basePath: JSONPath = [], +): OpenCodeConfigPathOperation[] { + const operations: OpenCodeConfigPathOperation[] = [] + + for (const key of Object.keys(requested)) { + const requestedValue = requested[key] + const hasCurrent = hasOwn(current, key) + const currentValue = hasCurrent ? current[key] : undefined + const nextPath = [...basePath, key] + if (isPlainObject(requestedValue) && isPlainObject(currentValue)) { + operations.push(...collectOpenCodeConfigPathOperations(requestedValue, currentValue, nextPath)) + } else if (!hasCurrent || !isDeepStrictEqual(requestedValue, currentValue)) { + operations.push({ path: nextPath, value: requestedValue }) + } + } + + for (const key of Object.keys(current)) { + if (hasOwn(requested, key)) continue + operations.push({ path: [...basePath, key], value: undefined }) } + + return operations +} + +function applyOpenCodeConfigPathOperations( + rawContent: string, + operations: OpenCodeConfigPathOperation[], +): string { + let text = rawContent + for (const operation of operations) { + const edits = modify(text, operation.path, operation.value, { + formattingOptions: { insertSpaces: true, tabSize: 2 }, + }) + text = applyEdits(text, edits) + } + return text +} + +function assertNoShadowedOpenCodeConfigRemovals( + operations: OpenCodeConfigPathOperation[], + targetSource: OpenCodeConfigSourceFile | undefined, + sources: OpenCodeConfigSourceFile[], + targetName: OpenCodeConfigSourceName, +): void { + const targetContent = targetSource?.content ?? {} + const shadowedPaths: string[] = [] + const shadowedSources = new Set() + + for (const operation of operations) { + if (operation.value !== undefined) continue + if (hasOpenCodeConfigPath(targetContent, operation.path)) continue + const definingSources = sources.filter( + (source) => source.name !== targetName && hasOpenCodeConfigPath(source.content, operation.path), + ) + if (definingSources.length === 0) continue + shadowedPaths.push(operation.path.join('.')) + for (const source of definingSources) shadowedSources.add(source.name) + } + + if (shadowedPaths.length > 0) { + throw new OpenCodeConfigShadowedRemovalError(shadowedPaths, [...shadowedSources], targetName) + } +} + +export async function updateOpenCodeConfigFile( + content: Record | string, + options: UpdateOpenCodeConfigOptions = {}, +): Promise { + const snapshot = options.snapshot ?? await readOpenCodeConfigSnapshot() + + if (options.expectedRevision !== undefined && options.expectedRevision !== snapshot.revision) { + throw new OpenCodeConfigConflictError(options.expectedRevision, snapshot.revision) + } + + const targetName = options.source !== undefined + ? assertOpenCodeConfigSourceName(options.source) + : selectWritableSource(snapshot.sources)?.name ?? DEFAULT_OPENCODE_CONFIG_SOURCE_NAME + const targetSource = snapshot.sources.find((source) => source.name === targetName) + const targetPath = getOpenCodeConfigSourcePath(targetName) + + if (typeof content === 'string') { + if (targetSource?.rawContent === content) { + return toOpenCodeConfigFile(snapshot) + } + return writeOpenCodeConfigFile(content, targetName, snapshot) + } + + const invalidSources = snapshot.sources.filter((source) => !source.isValid).map((source) => source.name) + if (invalidSources.length > 0) { + throw new OpenCodeConfigSourceInvalidError(invalidSources) + } + + OpenCodeConfigSchema.parse(content) + + const originalText = targetSource?.rawContent ?? '{}\n' + const operations = collectOpenCodeConfigPathOperations(content, snapshot.content) + assertNoShadowedOpenCodeConfigRemovals(operations, targetSource, snapshot.sources, targetName) + const updatedText = applyOpenCodeConfigPathOperations(originalText, operations) + + if (updatedText !== originalText) { + OpenCodeConfigSchema.parse(parseJsonc(updatedText)) + await writeFileAtomic(targetPath, updatedText, { mode: await existingFileMode(targetPath) }) + } + + return toOpenCodeConfigFile(await readOpenCodeConfigSnapshot()) +} + +export function serializeOpenCodeConfigSourceSnapshot( + entries: ReadonlyArray<{ name: OpenCodeConfigSourceName; rawContent: string }>, +): string { + const envelope: OpenCodeConfigSnapshotEnvelope = { + marker: OPENCODE_CONFIG_SNAPSHOT_MARKER, + version: OPENCODE_CONFIG_SNAPSHOT_VERSION, + sources: entries.map((entry) => ({ name: entry.name, rawContent: entry.rawContent })), + } + return JSON.stringify(envelope, null, 2) +} + +export function serializeOpenCodeConfigSnapshot( + config: Pick, +): string { + return serializeOpenCodeConfigSourceSnapshot(config.sources) +} + +export function buildOpenCodeConfigSeedSnapshot(): string { + return serializeOpenCodeConfigSourceSnapshot([ + { name: DEFAULT_OPENCODE_CONFIG_SOURCE_NAME, rawContent: OPENCODE_CONFIG_SEED }, + ]) +} + +function isSnapshotEnvelopeCandidate(parsed: Record): boolean { + return hasOwn(parsed, 'marker') || (hasOwn(parsed, 'sources') && hasOwn(parsed, 'version')) +} + +function parseOpenCodeConfigSnapshot(snapshot: string): Map { + let parsed: unknown + try { + parsed = parseJsonc(snapshot) + } catch { + throw new OpenCodeConfigSnapshotError('Invalid OpenCode config snapshot content') + } + + if (isPlainObject(parsed) && isSnapshotEnvelopeCandidate(parsed)) { + if (hasOwn(parsed, 'marker') && parsed.marker !== OPENCODE_CONFIG_SNAPSHOT_MARKER) { + throw new OpenCodeConfigSnapshotError('Invalid OpenCode config snapshot marker') + } + if (parsed.version !== OPENCODE_CONFIG_SNAPSHOT_VERSION) { + throw new OpenCodeConfigSnapshotError(`Unsupported OpenCode config snapshot version: ${String(parsed.version)}`) + } + if (!Array.isArray(parsed.sources)) { + throw new OpenCodeConfigSnapshotError('Invalid OpenCode config snapshot sources') + } + + const sources = new Map() + for (const entry of parsed.sources) { + if (!isPlainObject(entry)) { + throw new OpenCodeConfigSnapshotError('Invalid OpenCode config snapshot source') + } + if (typeof entry.name !== 'string' || !isOpenCodeConfigSourceName(entry.name)) { + throw new OpenCodeConfigSnapshotError(`Invalid OpenCode config snapshot source name: ${String(entry.name)}`) + } + if (typeof entry.rawContent !== 'string') { + throw new OpenCodeConfigSnapshotError('Invalid OpenCode config snapshot source content') + } + if (sources.has(entry.name)) { + throw new OpenCodeConfigSnapshotError(`Duplicate OpenCode config snapshot source: ${entry.name}`) + } + if (!parseOpenCodeConfigContent(entry.rawContent).isValid) { + throw new OpenCodeConfigSnapshotError(`Invalid OpenCode config snapshot content: ${entry.name}`) + } + sources.set(entry.name, entry.rawContent) + } + return sources + } + + if (!parseOpenCodeConfigContent(snapshot).isValid) { + throw new OpenCodeConfigSnapshotError('Invalid OpenCode config snapshot content') + } + + return new Map([['opencode.json', snapshot]]) +} + +async function rollbackOpenCodeConfigSourceFileStates( + applied: AppliedOpenCodeConfigSourceFileState[], +): Promise { + const failures: Error[] = [] + for (const entry of [...applied].reverse()) { + const sourcePath = getOpenCodeConfigSourcePath(entry.name) + try { + if (entry.previousRawContent === null) { + await rm(sourcePath, { force: true }) + } else { + await writeFileAtomic(sourcePath, entry.previousRawContent, { mode: entry.previousMode }) + } + } catch (error) { + failures.push(error instanceof Error ? error : new Error(String(error))) + } + } + return failures +} + +async function applyOpenCodeConfigSourceFileStates( + states: OpenCodeConfigSourceFileState[], + current: OpenCodeConfigSnapshot, +): Promise { + const previousByName = new Map( + current.sources.map((source) => [source.name, source.rawContent]), + ) + const prepared: PreparedOpenCodeConfigSourceFileState[] = [] + for (const state of states) { + const sourcePath = getOpenCodeConfigSourcePath(state.name) + const previousMode = await existingFileMode(sourcePath) + const previousRawContent = previousByName.get(state.name) ?? null + prepared.push({ ...state, path: sourcePath, previousMode, previousRawContent }) + } + + const applied: AppliedOpenCodeConfigSourceFileState[] = [] + try { + for (const state of prepared) { + if (state.rawContent === null) { + await rm(state.path, { force: true }) + } else { + await writeFileAtomic(state.path, state.rawContent, { mode: state.previousMode }) + } + applied.push({ + name: state.name, + previousRawContent: state.previousRawContent, + previousMode: state.previousMode, + }) + } + } catch (error) { + const rollbackFailures = await rollbackOpenCodeConfigSourceFileStates(applied) + if (rollbackFailures.length > 0) { + throw new AggregateError( + [error, ...rollbackFailures], + 'Failed to apply OpenCode config source states and roll back', + ) + } + throw error + } +} + +export async function restoreOpenCodeConfigSnapshot(snapshot: string): Promise { + const desired = parseOpenCodeConfigSnapshot(snapshot) + const current = await readOpenCodeConfigSnapshot() + const currentNames = new Set(current.sources.map((source) => source.name)) + + const states: OpenCodeConfigSourceFileState[] = [] + for (const name of OPENCODE_CONFIG_SOURCE_ORDER) { + if (desired.has(name)) { + states.push({ name, rawContent: desired.get(name) ?? '' }) + } else if (currentNames.has(name)) { + states.push({ name, rawContent: null }) + } + } + + await applyOpenCodeConfigSourceFileStates(states, current) + return readOpenCodeConfigFile() } export async function pruneHealthWatchDirectory(dirPath: string): Promise { @@ -143,42 +636,37 @@ export async function writeHealthWatchArtifact( const timestamp = new Date().toISOString().replace(/[:.]/g, '-') const artifactPath = path.join(getOpenCodeHealthWatchPath(), `${prefix}-${timestamp}.json`) await ensureDirectoryExists(getOpenCodeHealthWatchPath()) - await writeFileContent(artifactPath, buildContent(timestamp)) + await writeFileAtomic(artifactPath, buildContent(timestamp), { mode: 0o600 }) await pruneHealthWatchDirectory(getOpenCodeHealthWatchPath()) return artifactPath } export async function archiveBrokenOpenCodeConfigFile(): Promise { - const configPath = getOpenCodeConfigFilePath() - if (!(await fileExists(configPath))) { - return null - } - try { - const content = await readFileContent(configPath) - const archivePath = await writeHealthWatchArtifact('opencode-config-broken', () => content) + const snapshot = await readOpenCodeConfigSnapshot() + if (snapshot.sources.length === 0) { + return null + } + + const payload = serializeOpenCodeConfigSnapshot(toOpenCodeConfigFile(snapshot)) + const archivePath = await writeHealthWatchArtifact(OPENCODE_CONFIG_SNAPSHOT_ARTIFACT_PREFIX, () => payload) logger.warn(`Archived broken OpenCode config to ${archivePath}`) return archivePath } catch (error) { - logger.error('Failed to archive broken OpenCode config:', error) + logger.warn('Failed to archive broken OpenCode config:', error) return null } } export async function deleteOpenCodeConfigFile(): Promise { - const configPath = getOpenCodeConfigFilePath() - - if (!(await fileExists(configPath))) { - logger.warn('Config file does not exist:', configPath) - return false - } - - try { - await rm(configPath, { force: true }) - logger.info('Deleted filesystem config to allow server startup:', configPath) + return withOpenCodeConfigLock(async () => { + const snapshot = await readOpenCodeConfigSnapshot() + if (snapshot.sources.length === 0) return false + await applyOpenCodeConfigSourceFileStates( + snapshot.sources.map((source) => ({ name: source.name, rawContent: null })), + snapshot, + ) + logger.info('Deleted filesystem config to allow server startup:', snapshot.sources.map((source) => source.path).join(', ')) return true - } catch (error) { - logger.error('Failed to delete config file:', error) - return false - } + }) } diff --git a/backend/src/services/opencode-import.ts b/backend/src/services/opencode-import.ts index de609899..80f333ea 100644 --- a/backend/src/services/opencode-import.ts +++ b/backend/src/services/opencode-import.ts @@ -3,8 +3,14 @@ import path from 'path' import { existsSync } from 'node:fs' import { cp, mkdtemp, readdir, rename, rm } from 'fs/promises' import { Database as SQLiteDatabase } from 'bun:sqlite' -import { getOpenCodeConfigFilePath, getWorkspacePath } from '@opencode-manager/shared/config/env' -import { parseOpenCodeConfigContent, withOpenCodeConfigLock, writeOpenCodeConfigFile } from './opencode-config-file' +import { getConfigPath, getWorkspacePath } from '@opencode-manager/shared/config/env' +import { + DEFAULT_OPENCODE_CONFIG_SOURCE_NAME, + OPENCODE_CONFIG_SOURCE_NAMES, + isOpenCodeConfigSourceName, + selectPreferredOpenCodeConfigSourceName, +} from '@opencode-manager/shared' +import { parseOpenCodeConfigContent, restoreOpenCodeConfigSnapshot, serializeOpenCodeConfigSourceSnapshot, withOpenCodeConfigLock } from './opencode-config-file' import { captureLastKnownGoodOpenCodeConfig } from './opencode-config-apply' import { ensureDirectoryExists, fileExists, readFileContent } from './file-operations' import type { SettingsService } from './settings' @@ -13,8 +19,10 @@ const OPENCODE_STATE_DB_FILENAMES = new Set(['opencode.db', 'opencode.db-shm', ' export interface OpenCodeImportStatus { configSourcePath: string | null + configSourcePaths: string[] stateSourcePath: string | null workspaceConfigPath: string + workspaceConfigPathsToRemove: string[] workspaceStatePath: string workspaceStateExists: boolean } @@ -55,11 +63,14 @@ export function getImportPathCandidates(envKey: string, fallbackPath: string): s return Array.from(new Set(candidates)) } -export function getFirstExistingConfigSourcePath(): string | null { - return getImportPathCandidates( - 'OPENCODE_IMPORT_CONFIG_PATH', - path.join(os.homedir(), '.config', 'opencode', 'opencode.json') - ).find(candidate => existsSync(candidate)) ?? null +function getExistingConfigSourcePaths(): string[] { + const explicitPath = process.env.OPENCODE_IMPORT_CONFIG_PATH + if (explicitPath && existsSync(path.resolve(explicitPath))) { + return [path.resolve(explicitPath)] + } + return OPENCODE_CONFIG_SOURCE_NAMES + .map(name => path.join(os.homedir(), '.config', 'opencode', name)) + .filter(candidate => existsSync(candidate)) } async function getFirstExistingPathWithDatabase(paths: string[]): Promise { @@ -135,37 +146,60 @@ export async function importOpenCodeStateDirectory(sourcePath: string, targetPat } export async function getOpenCodeImportStatus(): Promise { - const workspaceConfigPath = getOpenCodeConfigFilePath() + const configDir = getConfigPath() + const workspaceConfigNames = OPENCODE_CONFIG_SOURCE_NAMES.filter(name => existsSync(path.join(configDir, name))) + const workspaceConfigPaths = workspaceConfigNames.map(name => path.join(configDir, name)) + const workspaceConfigPath = path.join( + configDir, + selectPreferredOpenCodeConfigSourceName(workspaceConfigNames) ?? DEFAULT_OPENCODE_CONFIG_SOURCE_NAME, + ) const workspaceStatePath = path.join(getWorkspacePath(), '.opencode', 'state', 'opencode') const workspaceStateExists = await fileExists(path.join(workspaceStatePath, 'opencode.db')) - const configSourcePath = getFirstExistingConfigSourcePath() + const configSourcePaths = getExistingConfigSourcePaths() + const configSourcePath = configSourcePaths.at(-1) ?? null + const hostSourceNames = new Set(configSourcePaths.map(sourcePath => path.basename(sourcePath))) + const workspaceConfigPathsToRemove = configSourcePaths.length === 0 + ? [] + : workspaceConfigPaths.filter(workspaceSourcePath => !hostSourceNames.has(path.basename(workspaceSourcePath))) const stateSourcePath = await getFirstExistingPathWithDatabase( getImportPathCandidates('OPENCODE_IMPORT_STATE_PATH', path.join(os.homedir(), '.local', 'share', 'opencode')) ) return { configSourcePath, + configSourcePaths, stateSourcePath, workspaceConfigPath, + workspaceConfigPathsToRemove, workspaceStatePath, workspaceStateExists, } } -async function importOpenCodeConfigFromSource(sourcePath: string, settingsService?: SettingsService): Promise { - const rawContent = await readFileContent(sourcePath) - const { isValid } = parseOpenCodeConfigContent(rawContent) - - if (!isValid) { - throw new Error('Importable OpenCode config is invalid') - } +async function importOpenCodeConfigFromSources(sourcePaths: string[], settingsService?: SettingsService): Promise { + const configDir = getConfigPath() + const sources = await Promise.all(sourcePaths.map(async sourcePath => { + const basename = path.basename(sourcePath) + const name = isOpenCodeConfigSourceName(basename) ? basename : DEFAULT_OPENCODE_CONFIG_SOURCE_NAME + const rawContent = await readFileContent(sourcePath) + if (!parseOpenCodeConfigContent(rawContent).isValid) { + throw new Error('Importable OpenCode config is invalid') + } + return { name, path: path.join(configDir, name), rawContent } + })) + const selected = sources.at(-1) + if (!selected) return false + if (sources.every((source, index) => path.resolve(sourcePaths[index]!) === path.resolve(source.path))) return false + const snapshot = serializeOpenCodeConfigSourceSnapshot( + sources.map(source => ({ name: source.name, rawContent: source.rawContent })), + ) await withOpenCodeConfigLock(async () => { if (settingsService) { await captureLastKnownGoodOpenCodeConfig(settingsService) } - await writeOpenCodeConfigFile(rawContent) + await restoreOpenCodeConfigSnapshot(snapshot) }) return true } @@ -183,7 +217,7 @@ export async function syncOpenCodeImport(options: SyncOpenCodeImportOptions): Pr } if (options.importConfig !== false && initialStatus.configSourcePath) { - configImported = await importOpenCodeConfigFromSource(initialStatus.configSourcePath, options.settingsService) + configImported = await importOpenCodeConfigFromSources(initialStatus.configSourcePaths, options.settingsService) } if (initialStatus.stateSourcePath && (overwriteState || !initialStatus.workspaceStateExists)) { diff --git a/backend/src/services/opencode-manager-tool-plugin.ts b/backend/src/services/opencode-manager-tool-plugin.ts index d62d169c..ff487ed0 100644 --- a/backend/src/services/opencode-manager-tool-plugin.ts +++ b/backend/src/services/opencode-manager-tool-plugin.ts @@ -8,6 +8,7 @@ export const MANAGER_TOOL_ALLOWED_ROUTES = [ 'GET /settings', 'PATCH /settings', 'GET /opencode-config', + 'GET /opencode-config/effective', 'PUT /opencode-config', 'POST /assistant/reload', 'GET /repos', diff --git a/backend/src/services/opencode-plugin-quarantine.ts b/backend/src/services/opencode-plugin-quarantine.ts index 8b518c65..c3d1f10e 100644 --- a/backend/src/services/opencode-plugin-quarantine.ts +++ b/backend/src/services/opencode-plugin-quarantine.ts @@ -4,7 +4,7 @@ import path from 'path' import { parseJsonc } from '@opencode-manager/shared/utils' import { logger } from '../utils/logger' import { existingFileMode, mkdirSafe, writeFileAtomic } from '../utils/fs-safe' -import { withFileLock } from '../utils/atomic-json' +import { withOpenCodeConfigLock } from './opencode-config-file' import { getOpenCodePluginDir } from './opencode/plugin-registry' import { isRecord, @@ -287,7 +287,7 @@ async function restoreEnforcementConfigSections(configPath: string): Promise writeFileAtomic(configPath, restoredContent, { mode })) + await withOpenCodeConfigLock(() => writeFileAtomic(configPath, restoredContent, { mode })) } await fs.rm(backupPath, { force: true }) } diff --git a/backend/src/services/opencode-restart.ts b/backend/src/services/opencode-restart.ts index 8216f0ce..7458f461 100644 --- a/backend/src/services/opencode-restart.ts +++ b/backend/src/services/opencode-restart.ts @@ -1,5 +1,6 @@ -import { opencodeServerManager } from './opencode-single-server' -import type { OpenCodeSupervisor } from './opencode-supervisor' +import { opencodeServerManager, ConfigReloadError } from './opencode-single-server' +import { readOpenCodeConfigFile } from './opencode-config-file' +import type { OpenCodeOperationReason, OpenCodeSupervisor } from './opencode-supervisor' import type { OpenCodeRestartCoordinator } from './opencode-restart-coordinator' let restartCoordinator: OpenCodeRestartCoordinator | null = null @@ -22,9 +23,9 @@ function restartFailureError(): Error { return new Error(startupError ?? 'OpenCode server restart did not complete successfully') } -async function performRestart(supervisor?: OpenCodeSupervisor): Promise { +async function performRestart(supervisor: OpenCodeSupervisor | undefined, reason: OpenCodeOperationReason): Promise { if (supervisor) { - return (await supervisor.restart('settings_restart')).healthy + return (await supervisor.restart(reason)).healthy } opencodeServerManager.clearStartupError() await opencodeServerManager.restart() @@ -43,16 +44,19 @@ async function performRestart(supervisor?: OpenCodeSupervisor): Promise * A full process restart drops in-flight sessions; resuming re-issues a * "continue" prompt once the server is healthy again. */ -export async function restartOpenCode(supervisor?: OpenCodeSupervisor): Promise<{ resumedSessionIDs: string[] }> { +export async function restartOpenCode( + supervisor?: OpenCodeSupervisor, + reason: OpenCodeOperationReason = 'settings_restart', +): Promise<{ resumedSessionIDs: string[] }> { if (restartCoordinator) { - const result = await restartCoordinator.runWithResume(() => performRestart(supervisor)) + const result = await restartCoordinator.runWithResume(() => performRestart(supervisor, reason)) if (!result.healthy) { throw restartFailureError() } return { resumedSessionIDs: result.resumedSessionIDs } } if (supervisor) { - const status = await supervisor.restart('settings_restart') + const status = await supervisor.restart(reason) if (!status.healthy) { throw restartFailureError() } @@ -67,19 +71,13 @@ export async function restartOpenCode(supervisor?: OpenCodeSupervisor): Promise< return { resumedSessionIDs: [] } } -/** - * Reloads OpenCode configuration via the non-disruptive API patch. This does - * NOT drop the server process, so active sessions keep running and there is - * nothing to resume. - */ -export async function reloadOpenCodeConfig(supervisor?: OpenCodeSupervisor): Promise { - if (supervisor) { - const status = await supervisor.reloadConfig('settings_reload') - if (!status.healthy) { - const startupError = opencodeServerManager.getLastStartupError() - throw new Error(startupError ?? 'OpenCode server reload did not complete successfully') - } - return +export async function reloadOpenCodeConfig(supervisor?: OpenCodeSupervisor): Promise<{ resumedSessionIDs: string[] }> { + const config = await readOpenCodeConfigFile() + if (!config) { + throw new ConfigReloadError('No OpenCode global configuration files found') + } + if (!config.isValid) { + throw new ConfigReloadError('OpenCode global configuration is invalid', config.validationIssues) } - await opencodeServerManager.reloadConfig() + return restartOpenCode(supervisor, 'settings_reload') } diff --git a/backend/src/services/opencode-single-server.ts b/backend/src/services/opencode-single-server.ts index 7be0c297..ce6d4e95 100644 --- a/backend/src/services/opencode-single-server.ts +++ b/backend/src/services/opencode-single-server.ts @@ -26,16 +26,11 @@ import { getOpenCodeTmpHome, ENV, } from '@opencode-manager/shared/config/env' -import { ZodError } from 'zod' import type { Database } from 'bun:sqlite' import { compareVersions } from '../utils/version-utils' -import { patchConfigWithRecovery } from './opencode/config-recovery' import type { OpenCodeClient } from './opencode/client' import { readOpenCodeConfigFile, - toOpenCodeConfigValidationIssues, - withOpenCodeConfigLock, - writeOpenCodeConfigFile, } from './opencode-config-file' import { getOrCreateInternalToken } from './internal-token' import { installManagedPlugins } from './opencode/plugin-registry' @@ -65,13 +60,11 @@ type OpenCodePluginSpec = string | [string, OpenCodePluginOptions] export class ConfigReloadError extends Error { validationIssues: StartupValidationIssue[] - removedFields: string[] - constructor(message: string, validationIssues: StartupValidationIssue[] = [], removedFields: string[] = []) { + constructor(message: string, validationIssues: StartupValidationIssue[] = []) { super(message) this.name = 'ConfigReloadError' this.validationIssues = validationIssues - this.removedFields = removedFields } } @@ -333,13 +326,6 @@ class OpenCodeServerManager { return ENV.OPENCODE.SERVER_PASSWORD } - private requireClient(): OpenCodeClient { - if (!this.openCodeClient) { - throw new Error('OpenCodeClient not configured on OpenCodeServerManager. Call setOpenCodeClient() during startup.') - } - return this.openCodeClient - } - static getInstance(): OpenCodeServerManager { if (!OpenCodeServerManager.instance) { OpenCodeServerManager.instance = new OpenCodeServerManager() @@ -636,6 +622,8 @@ class OpenCodeServerManager { delete cleanEnv.OPENCODE_PID delete cleanEnv.OPENCODE delete cleanEnv.OPENCODE_PURE + delete cleanEnv.OPENCODE_CONFIG + delete userEnvVars.OPENCODE_CONFIG this.serverProcess = spawn( openCodeExecutable, @@ -670,7 +658,6 @@ class OpenCodeServerManager { OPENCODE_SERVER_USERNAME: getOpenCodeServerUsername(), } : {}), - OPENCODE_CONFIG: openCodeConfigPath, } } ) @@ -1039,69 +1026,6 @@ class OpenCodeServerManager { } } - async reloadConfig(): Promise { - const acquired = this.acquireOp() - if (!acquired) { - throw new OpenCodeOperationBusyError() - } - - try { - logger.info('Reloading OpenCode configuration (via API)') - try { - await withOpenCodeConfigLock(async () => { - const file = await readOpenCodeConfigFile() - if (file === null) { - throw new Error(`OpenCode config file not found: ${getOpenCodeConfigFilePath()}`) - } - logger.info(`Read config from file for reload: ${file.path}`) - - const patchResult = await patchConfigWithRecovery(this.requireClient(), file.content) - if (!patchResult.success) { - const errorMessage = patchResult.error || 'Failed to reload config' - const validationIssues = patchResult.details || [] - const removedFields = patchResult.removedFields || [] - if (validationIssues.length > 0) { - const issueSummary = validationIssues.map((d) => `${d.path}: ${d.message}`).join('; ') - logger.error(`Config reload validation errors: ${issueSummary}`) - } - if (removedFields.length > 0) { - logger.info(`Removed fields during config reload: ${removedFields.join(', ')}`) - } - throw new ConfigReloadError(errorMessage, validationIssues, removedFields) - } - - if (patchResult.removedFields && patchResult.removedFields.length > 0 && patchResult.appliedConfig) { - const cleanedConfigContent = JSON.stringify(patchResult.appliedConfig, null, 2) - try { - await writeOpenCodeConfigFile(cleanedConfigContent) - } catch (error) { - if (error instanceof ZodError) { - const validationIssues = toOpenCodeConfigValidationIssues(error.issues) - const issueSummary = validationIssues.map((d) => `${d.path}: ${d.message}`).join('; ') - logger.error(`Config reload validation errors: ${issueSummary}`) - throw new ConfigReloadError('Cleaned config failed validation', validationIssues, patchResult.removedFields) - } - throw error - } - logger.info(`Persisted cleaned config to ${file.path} after removing fields: ${patchResult.removedFields.join(', ')}`) - } - }) - - logger.info('OpenCode configuration reloaded successfully') - await new Promise(r => setTimeout(r, 500)) - const healthy = await this.checkHealth() - if (!healthy) { - throw new Error('Server unhealthy after config reload') - } - } catch (error) { - logger.error('Failed to reload OpenCode config:', error) - throw error - } - } finally { - this.releaseOp(acquired) - } - } - getPort(): number { return getOpenCodeServerPort() } diff --git a/backend/src/services/opencode-supervisor.ts b/backend/src/services/opencode-supervisor.ts index 8265a593..54ab5605 100644 --- a/backend/src/services/opencode-supervisor.ts +++ b/backend/src/services/opencode-supervisor.ts @@ -130,21 +130,6 @@ export class OpenCodeSupervisor { }) } - async reloadConfig(reason: OpenCodeOperationReason): Promise { - return this.runLifecycleOperation(async () => { - this.setState('starting') - - try { - this.openCodeServerManager.clearStartupError() - await this.openCodeServerManager.reloadConfig() - return this.refreshHealthOrRecover(reason) - } catch (error) { - this.recordFailure(error) - return this.recover(reason) - } - }) - } - async checkNow(reason: OpenCodeOperationReason): Promise { if (reason === 'health_poll' && !this.isWatchEnabled()) { return this.getStatus() @@ -337,6 +322,7 @@ export class OpenCodeSupervisor { } private async seedDefaultConfig(): Promise { + await archiveBrokenOpenCodeConfigFile() await seedOpenCodeConfigFile() this.openCodeServerManager.clearStartupError() await this.openCodeServerManager.restart() diff --git a/backend/src/services/opencode/config-recovery.ts b/backend/src/services/opencode/config-recovery.ts deleted file mode 100644 index 2715722b..00000000 --- a/backend/src/services/opencode/config-recovery.ts +++ /dev/null @@ -1,262 +0,0 @@ -import type { OpenCodeClient } from './client' -import { logger } from '../../utils/logger' -import { TIMEOUTS } from '@opencode-manager/shared/config/env' -import { parseJsonc } from '@opencode-manager/shared/utils' - -export type PatchConfigValidationIssue = { - path: string - message: string -} - -export type PatchConfigResult = { - success: boolean - error?: string - details?: PatchConfigValidationIssue[] - removedFields?: string[] - appliedConfig?: Record -} - -function getIssuePath(value: unknown): string { - if (Array.isArray(value)) { - const path = value - .map((part) => typeof part === 'string' || typeof part === 'number' ? String(part) : '') - .filter(Boolean) - .join('.') - return path || 'root' - } - - if (typeof value === 'string' && value.length > 0) { - if (value.startsWith('/')) { - const pointerPath = value - .split('/') - .filter(Boolean) - .map((part) => part.replace(/~1/g, '/').replace(/~0/g, '~')) - .join('.') - return pointerPath || 'root' - } - - return value - } - - if (typeof value === 'number') { - return String(value) - } - - return 'root' -} - -function getIssueMessage(value: unknown): string { - if (typeof value === 'string' && value.length > 0) { - return value - } - - return 'Validation error' -} - -function extractValidationIssues(value: unknown): PatchConfigValidationIssue[] { - if (!Array.isArray(value)) { - return [] - } - - return value.flatMap((item) => { - if (!item || typeof item !== 'object') { - return [] - } - - const issue = item as Record - const nestedIssues = extractValidationIssues(issue.issues ?? issue.errors) - if (nestedIssues.length > 0) { - return nestedIssues - } - - if ( - typeof issue.message === 'string' - || typeof issue.path === 'string' - || Array.isArray(issue.path) - || typeof issue.instancePath === 'string' - || Array.isArray(issue.instancePath) - ) { - return [{ - path: getIssuePath(issue.path ?? issue.instancePath), - message: getIssueMessage(issue.message), - }] - } - - return [] - }) -} - -function removeFieldFromConfig(config: Record, path: string): Record { - const result = JSON.parse(JSON.stringify(config)) as Record - const parts = path.split('.') - - let current: Record = result - for (let i = 0; i < parts.length - 1; i++) { - const part = parts[i] - if (!part || !current[part] || typeof current[part] !== 'object') { - return result - } - current = current[part] as Record - } - - const lastPart = parts[parts.length - 1] - if (lastPart) { - delete current[lastPart] - } - - return result -} - -function parseErrorResponse(responseText: string): { details: PatchConfigValidationIssue[]; errorMessage: string } { - const details: PatchConfigValidationIssue[] = [] - let errorMessage = 'Unknown error' - - try { - const errorBody = parseJsonc(responseText) as Record - const structuredIssues = extractValidationIssues( - errorBody?.errors - ?? errorBody?.issues - ?? (errorBody?.data && typeof errorBody.data === 'object' - ? (errorBody.data as Record).errors ?? (errorBody.data as Record).issues - : undefined) - ) - - if (structuredIssues.length > 0) { - details.push(...structuredIssues) - errorMessage = details.map((d) => `${d.path}: ${d.message}`).join('; ') - } else if (errorBody?.name === 'ConfigInvalidError' && errorBody?.data) { - const data = errorBody.data as { issues?: Array<{ message: string; path?: string[] }> } - if (data.issues) { - for (const issue of data.issues) { - const path = issue.path ? issue.path.join('.') : 'root' - details.push({ path, message: issue.message }) - } - errorMessage = details.map((d) => `${d.path}: ${d.message}`).join('; ') - } - } else if (typeof errorBody?.error === 'string') { - errorMessage = errorBody.error - } else if (typeof errorBody?.message === 'string') { - errorMessage = errorBody.message - } else if (typeof errorBody?.success === 'boolean' && errorBody.success === false && errorBody?.data) { - errorMessage = 'Config validation failed' - } else { - const snippet = responseText.slice(0, 300) - errorMessage = `Request failed (${snippet.length < responseText.length ? 'truncated' : 'raw'} response): ${snippet}` - } - } catch { - const snippet = responseText.slice(0, 300) - errorMessage = `Parse error: ${snippet}` - } - - return { details, errorMessage } -} - -const CONFIG_PATCH_TIMEOUT_ERROR = `Timed out waiting for OpenCode config patch after ${TIMEOUTS.CONFIG_PATCH_TIMEOUT_MS}ms` - -function isTimeoutError(error: unknown): boolean { - return typeof error === 'object' - && error !== null - && 'name' in error - && (error as { name?: unknown }).name === 'TimeoutError' -} - -async function forwardConfigPatch( - client: OpenCodeClient, - config: Record, -): Promise { - const signal = AbortSignal.timeout(TIMEOUTS.CONFIG_PATCH_TIMEOUT_MS) - const response = await client.forward({ - method: 'PATCH', - path: '/config', - body: JSON.stringify(config), - headers: { 'Content-Type': 'application/json' }, - signal, - }) - - if (!response.ok && signal.aborted) { - throw new DOMException(CONFIG_PATCH_TIMEOUT_ERROR, 'TimeoutError') - } - - return response -} - -export async function patchConfigWithRecovery( - client: OpenCodeClient, - config: Record, -): Promise { - try { - const response = await forwardConfigPatch(client, config) - - if (response.ok) { - logger.info('Patched OpenCode config via API') - return { success: true, appliedConfig: config } - } - - const responseText = await response.text() - logger.warn(`OpenCode PATCH response (${response.status}): ${responseText.slice(0, 500)}`) - const { details, errorMessage: initialError } = parseErrorResponse(responseText) - - if (details.length === 0) { - logger.error(`Failed to patch OpenCode config: ${initialError}`) - return { success: false, error: initialError, details } - } - - logger.warn(`OpenCode rejected config with validation errors: ${initialError}`) - - const problematicPaths = [...new Set(details.map((d) => d.path))] - const removablePaths = problematicPaths.filter((path) => path !== 'root' && path.split('.').length <= 3) - const nonRemovablePaths = problematicPaths.filter((path) => path === 'root' || path.split('.').length > 3) - - if (nonRemovablePaths.length > 0) { - logger.error(`Failed to patch OpenCode config: ${initialError}`) - return { success: false, error: initialError, details } - } - - if (removablePaths.length === 0) { - logger.error(`Failed to patch OpenCode config: ${initialError}`) - return { success: false, error: initialError, details } - } - - let cleanedConfig = config - const removedFields: string[] = [] - - for (const path of removablePaths) { - cleanedConfig = removeFieldFromConfig(cleanedConfig, path) - removedFields.push(path) - logger.info(`Removed problematic field from config: ${path}`) - } - - logger.info(`Retrying config patch after removing ${removedFields.length} problematic field(s): ${removedFields.join(', ')}`) - const retryResponse = await forwardConfigPatch(client, cleanedConfig) - - if (retryResponse.ok) { - logger.info('Patched OpenCode config via API after removing invalid fields') - return { - success: true, - appliedConfig: cleanedConfig, - removedFields, - details - } - } - - const retryResponseText = await retryResponse.text() - const { details: retryDetails, errorMessage } = parseErrorResponse(retryResponseText) - logger.error(`Failed to patch OpenCode config even after removing invalid fields: ${errorMessage}`) - - return { - success: false, - error: errorMessage, - details: retryDetails.length > 0 ? retryDetails : details, - removedFields - } - } catch (error) { - if (isTimeoutError(error)) { - logger.error(`Failed to patch OpenCode config: ${CONFIG_PATCH_TIMEOUT_ERROR}`) - return { success: false, error: CONFIG_PATCH_TIMEOUT_ERROR } - } - - const errorMessage = error instanceof Error ? error.message : 'Unknown error' - logger.error('Failed to patch OpenCode config:', error) - return { success: false, error: errorMessage } - } -} diff --git a/backend/test/index.test.ts b/backend/test/index.test.ts index 27f51570..80d7b578 100644 --- a/backend/test/index.test.ts +++ b/backend/test/index.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' -import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -13,7 +13,6 @@ const supervisorMock = vi.hoisted(() => ({ start: vi.fn().mockResolvedValue({ healthy: true, port: 5551, state: 'running', resumedSessionIDs: [] }), stop: vi.fn().mockResolvedValue(undefined), restart: vi.fn().mockResolvedValue({ healthy: true, resumedSessionIDs: [] }), - reloadConfig: vi.fn().mockResolvedValue({ healthy: true }), getLastStartupError: vi.fn().mockReturnValue(null), })) @@ -50,11 +49,12 @@ vi.mock('../src/ipc/ipcServer', () => ({ })) vi.mock('../src/services/opencode-import', () => ({ - getFirstExistingConfigSourcePath: vi.fn().mockReturnValue(null), getOpenCodeImportStatus: vi.fn().mockResolvedValue({ configSourcePath: null, + configSourcePaths: [], stateSourcePath: null, workspaceConfigPath: '/tmp/test-workspace/.config/opencode/opencode.json', + workspaceConfigPathsToRemove: [], workspaceStatePath: '/tmp/test-workspace/.opencode/state/opencode', workspaceStateExists: true, }), @@ -65,6 +65,21 @@ vi.mock('../src/services/assistant-mode', () => ({ installAssistantWorkspace: vi.fn().mockResolvedValue(undefined), })) +const seedOpenCodeConfigFileMock = vi.hoisted(() => vi.fn().mockResolvedValue({ + path: '', + rawContent: '', + content: {}, + isValid: true, + updatedAt: 0, + sources: [], + revision: '', +})) + +vi.mock('../src/services/opencode-config-apply', async (importOriginal) => ({ + ...(await importOriginal()), + seedOpenCodeConfigFile: seedOpenCodeConfigFileMock, +})) + vi.mock('../src/services/skills', () => ({ migrateGlobalSkills: vi.fn().mockResolvedValue(undefined), })) @@ -94,7 +109,6 @@ const serverManagerMock = vi.hoisted(() => ({ clearStartupError: vi.fn(), markRestartPending: vi.fn(), isRestartPending: vi.fn().mockReturnValue(false), - reloadConfig: vi.fn().mockResolvedValue(undefined), restart: vi.fn().mockResolvedValue(undefined), checkHealth: vi.fn().mockResolvedValue(true), })) @@ -178,15 +192,19 @@ describe('backend entrypoint', () => { const { getOpenCodeImportStatus, syncOpenCodeImport } = await import('../src/services/opencode-import') vi.mocked(getOpenCodeImportStatus).mockResolvedValueOnce({ configSourcePath: '/import/opencode.json', + configSourcePaths: ['/import/opencode.json'], stateSourcePath: '/import/state', workspaceConfigPath: join(configDir, 'opencode.json'), + workspaceConfigPathsToRemove: [], workspaceStatePath: join(tempWorkspace, '.opencode', 'state', 'opencode'), workspaceStateExists: false, }) vi.mocked(syncOpenCodeImport).mockResolvedValueOnce({ configSourcePath: '/import/opencode.json', + configSourcePaths: ['/import/opencode.json'], stateSourcePath: '/import/state', workspaceConfigPath: join(configDir, 'opencode.json'), + workspaceConfigPathsToRemove: [], workspaceStatePath: join(tempWorkspace, '.opencode', 'state', 'opencode'), workspaceStateExists: false, configImported: false, @@ -201,6 +219,12 @@ describe('backend entrypoint', () => { })) }) + it('seeds the default config when no workspace or importable host config exists', async () => { + await import('../src/index') + + expect(seedOpenCodeConfigFileMock).toHaveBeenCalledTimes(1) + }) + it('answers the root route with service metadata outside production', async () => { await import('../src/index') @@ -214,6 +238,18 @@ describe('backend entrypoint', () => { expect(body.endpoints.repos).toBe('/api/repos') }) + it.each(['opencode.jsonc', 'config.json'])('does not seed opencode.json when %s already exists', async (name) => { + const configDir = join(tempWorkspace, '.config', 'opencode') + const rawContent = '{\n // preserve this source\n "model": "test/model"\n}\n' + await mkdir(configDir, { recursive: true }) + await writeFile(join(configDir, name), rawContent) + + await import('../src/index') + + expect(await readFile(join(configDir, name), 'utf8')).toBe(rawContent) + await expect(readFile(join(configDir, 'opencode.json'), 'utf8')).rejects.toMatchObject({ code: 'ENOENT' }) + }) + it('ignores unknown API routes through the not-found handler', async () => { await import('../src/index') diff --git a/backend/test/routes/internal-opencode-config.test.ts b/backend/test/routes/internal-opencode-config.test.ts index 65df8ff3..6cb11971 100644 --- a/backend/test/routes/internal-opencode-config.test.ts +++ b/backend/test/routes/internal-opencode-config.test.ts @@ -1,17 +1,17 @@ -import { describe, it, expect, beforeEach, afterEach } from 'bun:test' +import { describe, it, expect, beforeEach, afterEach, vi } from 'bun:test' import { Hono } from 'hono' import { Database } from 'bun:sqlite' -import { readFile } from 'fs/promises' +import { readFile, writeFile } from 'fs/promises' import path from 'path' import { createInternalRoutes } from '../../src/routes/internal' import { ScheduleService } from '../../src/services/schedules' import { NotificationService } from '../../src/services/notification' import { SettingsService } from '../../src/services/settings' -import { createOpenCodeClient } from '../../src/services/opencode/client' +import { UpstreamError, type OpenCodeClient } from '../../src/services/opencode/client' import { allMigrations } from '../../src/db/migrations' import { getOrCreateInternalToken } from '../../src/services/internal-token' import { migrate } from '../../src/db/migration-runner' -import { OPENCODE_CONFIG_SEED, writeOpenCodeConfigFile } from '../../src/services/opencode-config-file' +import { OPENCODE_CONFIG_SEED, readOpenCodeConfigFile, writeOpenCodeConfigFile } from '../../src/services/opencode-config-file' import { createTempAssistantWorkspace } from '../helpers/assistant-workspace' import type { ScheduleWorktreeManager } from '../../src/services/schedule-worktree' @@ -20,12 +20,27 @@ describe('internal/opencode-config routes', () => { let app: Hono let token: string let ws: Awaited> + let getJsonMock: ReturnType + let forwardMock: ReturnType + + function configPath(name: string): string { + return path.join(ws.workspacePath, '.config/opencode', name) + } + + function authHeaders(): Record { + return { authorization: `Bearer ${token}` } + } beforeEach(async () => { ws = await createTempAssistantWorkspace() db = new Database(':memory:') migrate(db, allMigrations) - const openCodeClient = createOpenCodeClient() + getJsonMock = vi.fn(() => Promise.resolve({})) + forwardMock = vi.fn(() => Promise.resolve(new Response('{}'))) + const openCodeClient = { + getJson: getJsonMock, + forward: forwardMock, + } as unknown as OpenCodeClient const stubWorktreeManager = { prepare: () => Promise.resolve(null), finalize: () => Promise.resolve({ commitHash: null }) } as unknown as ScheduleWorktreeManager const scheduleService = new ScheduleService(db, openCodeClient, stubWorktreeManager) const notificationService = new NotificationService(db) @@ -45,20 +60,16 @@ describe('internal/opencode-config routes', () => { }) it('GET /api/internal/opencode-config returns 404 when no config file exists', async () => { - const res = await app.request('/api/internal/opencode-config', { - headers: { authorization: `Bearer ${token}` }, - }) + const res = await app.request('/api/internal/opencode-config', { headers: authHeaders() }) expect(res.status).toBe(404) const body = await res.json() as { error: string } expect(body.error).toBe('No OpenCode config file found') }) - it('GET /api/internal/opencode-config returns the on-disk config state', async () => { - await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED) + it('GET /api/internal/opencode-config returns the merged persisted snapshot and sources', async () => { + await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED, 'opencode.jsonc') - const res = await app.request('/api/internal/opencode-config', { - headers: { authorization: `Bearer ${token}` }, - }) + const res = await app.request('/api/internal/opencode-config', { headers: authHeaders() }) expect(res.status).toBe(200) const body = await res.json() as { @@ -67,23 +78,24 @@ describe('internal/opencode-config routes', () => { rawContent: string isValid: boolean updatedAt: number + sources: Array<{ name: string; path: string; rawContent: string }> + revision: string } - expect(body.path).toBe(path.join(ws.workspacePath, '.config/opencode/opencode.json')) + expect(body.path).toBe(configPath('opencode.jsonc')) expect(body.rawContent).toBe(OPENCODE_CONFIG_SEED) expect(body.content).toEqual({ $schema: 'https://opencode.ai/config.json' }) expect(body.isValid).toBe(true) expect(body.updatedAt).toBeGreaterThan(0) + expect(body.sources.map((source) => source.name)).toEqual(['opencode.jsonc']) + expect(body.revision).toMatch(/^[a-f0-9]{64}$/) }) - it('PUT /api/internal/opencode-config writes the file and reports restartRequired for a plugin change', async () => { - await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED) + it('PUT /api/internal/opencode-config writes the file, reports restartRequired, and never forwards a PATCH', async () => { + await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED, 'opencode.jsonc') const res = await app.request('/api/internal/opencode-config', { method: 'PUT', - headers: { - 'content-type': 'application/json', - authorization: `Bearer ${token}`, - }, + headers: { 'content-type': 'application/json', ...authHeaders() }, body: JSON.stringify({ content: { $schema: 'https://opencode.ai/config.json', plugin: ['x'] } }), }) @@ -91,18 +103,96 @@ describe('internal/opencode-config routes', () => { const body = await res.json() as { restartRequired?: boolean; content: Record } expect(body.restartRequired).toBe(true) expect(body.content).toEqual({ $schema: 'https://opencode.ai/config.json', plugin: ['x'] }) + expect(forwardMock).not.toHaveBeenCalled() + + const onDisk = JSON.parse(await readFile(configPath('opencode.jsonc'), 'utf8')) as Record + expect(onDisk.plugin).toEqual(['x']) + }) + + it('PUT /api/internal/opencode-config does not report restartRequired for a comment-only edit', async () => { + await writeOpenCodeConfigFile('{"theme":"dark"}', 'opencode.json') + const commented = '{\n // keep this comment\n "theme": "dark"\n}\n' - const onDisk = await readFile(path.join(ws.workspacePath, '.config/opencode/opencode.json'), 'utf8') - expect(JSON.parse(onDisk).plugin).toEqual(['x']) + const res = await app.request('/api/internal/opencode-config', { + method: 'PUT', + headers: { 'content-type': 'application/json', ...authHeaders() }, + body: JSON.stringify({ content: commented, source: 'opencode.json' }), + }) + + expect(res.status).toBe(200) + const body = await res.json() as { restartRequired?: boolean } + expect(body.restartRequired).toBeUndefined() + await expect(readFile(configPath('opencode.json'), 'utf8')).resolves.toBe(commented) + }) + + it('PUT /api/internal/opencode-config forwards the requested source', async () => { + await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED, 'opencode.jsonc') + const submitted = '{\n "theme": "light"\n}\n' + + const res = await app.request('/api/internal/opencode-config', { + method: 'PUT', + headers: { 'content-type': 'application/json', ...authHeaders() }, + body: JSON.stringify({ content: submitted, source: 'config.json' }), + }) + + expect(res.status).toBe(200) + await expect(readFile(configPath('config.json'), 'utf8')).resolves.toBe(submitted) + await expect(readFile(configPath('opencode.jsonc'), 'utf8')).resolves.toBe(OPENCODE_CONFIG_SEED) + }) + + it('PUT /api/internal/opencode-config returns 409 for a stale expectedRevision', async () => { + const initial = await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED, 'opencode.jsonc') + await writeFile(configPath('config.json'), '{"model":"a/b"}', 'utf8') + + const res = await app.request('/api/internal/opencode-config', { + method: 'PUT', + headers: { 'content-type': 'application/json', ...authHeaders() }, + body: JSON.stringify({ content: { theme: 'light' }, expectedRevision: initial.revision! }), + }) + + expect(res.status).toBe(409) + const body = await res.json() as { expectedRevision: string; actualRevision: string } + expect(body.expectedRevision).toBe(initial.revision!) + expect(body.actualRevision).not.toBe(initial.revision!) + }) + + it('PUT /api/internal/opencode-config returns 409 for a shadowed removal', async () => { + const lower = '{"theme":"light","model":"a"}' + const target = '{"model":"b"}' + await writeOpenCodeConfigFile(target, 'opencode.jsonc') + await writeFile(configPath('config.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' } }), + }) + + expect(res.status).toBe(409) + const body = await res.json() as { error: string; paths: string[]; sources: string[] } + expect(body.paths).toEqual(['theme']) + expect(body.sources).toEqual(['config.json']) + expect(body.error).toContain('Cannot remove theme') + await expect(readFile(configPath('config.json'), 'utf8')).resolves.toBe(lower) + await expect(readFile(configPath('opencode.jsonc'), 'utf8')).resolves.toBe(target) + }) + + it('PUT /api/internal/opencode-config returns 400 for schema-invalid 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) }) it('PUT /api/internal/opencode-config returns 400 for an invalid JSON body', async () => { const res = await app.request('/api/internal/opencode-config', { method: 'PUT', - headers: { - 'content-type': 'application/json', - authorization: `Bearer ${token}`, - }, + headers: { 'content-type': 'application/json', ...authHeaders() }, body: '{', }) @@ -110,4 +200,26 @@ describe('internal/opencode-config routes', () => { const body = await res.json() as { error: string } expect(body.error).toBe('Invalid JSON') }) + + it('GET /api/internal/opencode-config/effective proxies the running global config without writing it back', async () => { + await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED, 'opencode.jsonc') + const effective = { theme: 'dark', model: 'effective/model' } + getJsonMock.mockImplementation(() => Promise.resolve(effective)) + + const res = await app.request('/api/internal/opencode-config/effective', { headers: authHeaders() }) + + expect(res.status).toBe(200) + expect(await res.json()).toEqual(effective) + expect(getJsonMock).toHaveBeenCalledWith('/global/config') + const persisted = await readOpenCodeConfigFile() + expect(persisted?.content).toEqual({ $schema: 'https://opencode.ai/config.json' }) + }) + + it('GET /api/internal/opencode-config/effective returns 503 when the server is unavailable', async () => { + getJsonMock.mockImplementation(() => Promise.reject(new UpstreamError(502, 'Proxy request failed'))) + + const res = await app.request('/api/internal/opencode-config/effective', { headers: authHeaders() }) + + expect(res.status).toBe(503) + }) }) diff --git a/backend/test/routes/opencode-auth-proxy.test.ts b/backend/test/routes/opencode-auth-proxy.test.ts index 507d86b0..d948b27d 100644 --- a/backend/test/routes/opencode-auth-proxy.test.ts +++ b/backend/test/routes/opencode-auth-proxy.test.ts @@ -55,7 +55,6 @@ describe('authenticated opencode proxy routes', () => { isOperationInProgress: vi.fn(() => false), checkHealth: vi.fn().mockResolvedValue(true), restart: vi.fn().mockResolvedValue(undefined), - reloadConfig: vi.fn().mockResolvedValue(undefined), clearStartupError: vi.fn(), getLastStartupError: vi.fn(() => null), isLastStartupErrorNonRecoverable: vi.fn(() => false), diff --git a/backend/test/routes/opencode-proxy.test.ts b/backend/test/routes/opencode-proxy.test.ts index 8df16538..1fd70403 100644 --- a/backend/test/routes/opencode-proxy.test.ts +++ b/backend/test/routes/opencode-proxy.test.ts @@ -668,7 +668,6 @@ describe('opencode-proxy routes', () => { isOperationInProgress: vi.fn(() => false), checkHealth: vi.fn().mockResolvedValue(true), restart: vi.fn().mockResolvedValue(undefined), - reloadConfig: vi.fn().mockResolvedValue(undefined), clearStartupError: vi.fn(), getLastStartupError: vi.fn(() => null), isLastStartupErrorNonRecoverable: vi.fn(() => false), diff --git a/backend/test/routes/settings-opencode-auth.test.ts b/backend/test/routes/settings-opencode-auth.test.ts index bd509421..d1ba7195 100644 --- a/backend/test/routes/settings-opencode-auth.test.ts +++ b/backend/test/routes/settings-opencode-auth.test.ts @@ -17,7 +17,6 @@ vi.mock('bun:sqlite', () => ({ vi.mock('../../src/services/opencode-single-server', () => ({ opencodeServerManager: { restart: vi.fn(), - reloadConfig: vi.fn(), getVersion: vi.fn(), fetchVersion: vi.fn(), clearStartupError: vi.fn(), @@ -27,7 +26,6 @@ vi.mock('../../src/services/opencode-single-server', () => ({ }, ConfigReloadError: class ConfigReloadError extends Error { validationIssues = [] - removedFields = [] }, })) @@ -242,7 +240,6 @@ describe('OpenCode Server Auth Routes', () => { isOperationInProgress: vi.fn(() => false), checkHealth: vi.fn().mockResolvedValue(true), restart: vi.fn().mockResolvedValue(undefined), - reloadConfig: vi.fn().mockResolvedValue(undefined), clearStartupError: vi.fn(), getLastStartupError: vi.fn(() => null), isLastStartupErrorNonRecoverable: vi.fn(() => false), diff --git a/backend/test/routes/settings-skills-install.test.ts b/backend/test/routes/settings-skills-install.test.ts index 6d8ca79a..cb95ff8c 100644 --- a/backend/test/routes/settings-skills-install.test.ts +++ b/backend/test/routes/settings-skills-install.test.ts @@ -51,7 +51,6 @@ vi.mock('../../src/services/file-operations', () => ({ vi.mock('../../src/services/opencode-single-server', () => { class MockConfigReloadError extends Error { validationIssues: Array<{ path: string; message: string }> = [] - removedFields: string[] = [] constructor(message: string) { super(message) this.name = 'ConfigReloadError' @@ -62,7 +61,6 @@ vi.mock('../../src/services/opencode-single-server', () => { opencodeServerManager: { getVersion: vi.fn(), fetchVersion: vi.fn(), - reloadConfig: vi.fn(), restart: vi.fn(), clearStartupError: vi.fn(), getLastStartupError: vi.fn(), @@ -85,7 +83,8 @@ vi.mock('../../src/services/skills', () => ({ installSkillFromUploadedFiles: vi.fn(), })) -vi.mock('@opencode-manager/shared/config/env', () => ({ +vi.mock('@opencode-manager/shared/config/env', async (importOriginal) => ({ + ...(await importOriginal()), getWorkspacePath: vi.fn(() => '/tmp/test-workspace'), getReposPath: vi.fn(() => '/tmp/test-repos'), getOpenCodeConfigFilePath: vi.fn(() => '/tmp/test-workspace/.config/opencode.json'), @@ -107,7 +106,7 @@ vi.mock('@opencode-manager/shared/config/env', () => ({ MAX_SIZE_BYTES: 1024 * 1024, MAX_UPLOAD_SIZE_BYTES: 10 * 1024 * 1024, }, - TIMEOUTS: { CONFIG_PATCH_TIMEOUT_MS: 15000 }, + TIMEOUTS: {}, })) import { createSettingsRoutes } from '../../src/routes/settings' diff --git a/backend/test/routes/settings.test.ts b/backend/test/routes/settings.test.ts index ba84620d..1412d24c 100644 --- a/backend/test/routes/settings.test.ts +++ b/backend/test/routes/settings.test.ts @@ -12,13 +12,15 @@ const mockResetSettings = vi.fn() const mockGetLastKnownGoodConfig = vi.fn() const { mockReadOpenCodeConfigFile, - mockWriteOpenCodeConfigFile, mockDeleteOpenCodeConfigFile, + mockArchiveBrokenOpenCodeConfigFile, + mockRestoreOpenCodeConfigSnapshot, mockApplyOpenCodeConfigUpdate, } = vi.hoisted(() => ({ mockReadOpenCodeConfigFile: vi.fn(), - mockWriteOpenCodeConfigFile: vi.fn(), mockDeleteOpenCodeConfigFile: vi.fn(), + mockArchiveBrokenOpenCodeConfigFile: vi.fn(), + mockRestoreOpenCodeConfigSnapshot: vi.fn(), mockApplyOpenCodeConfigUpdate: vi.fn(), })) @@ -154,16 +156,17 @@ vi.mock('../../src/services/file-operations', () => ({ fileExists: vi.fn(), })) -vi.mock('../../src/services/opencode/config-recovery', () => ({ - patchConfigWithRecovery: vi.fn(), -})) - -vi.mock('../../src/services/opencode-config-file', () => ({ - readOpenCodeConfigFile: mockReadOpenCodeConfigFile, - writeOpenCodeConfigFile: mockWriteOpenCodeConfigFile, - deleteOpenCodeConfigFile: mockDeleteOpenCodeConfigFile, - withOpenCodeConfigLock: (fn: () => Promise) => fn(), -})) +vi.mock('../../src/services/opencode-config-file', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + readOpenCodeConfigFile: mockReadOpenCodeConfigFile, + deleteOpenCodeConfigFile: mockDeleteOpenCodeConfigFile, + archiveBrokenOpenCodeConfigFile: mockArchiveBrokenOpenCodeConfigFile, + restoreOpenCodeConfigSnapshot: mockRestoreOpenCodeConfigSnapshot, + withOpenCodeConfigLock: (fn: () => Promise) => fn(), + } +}) vi.mock('../../src/services/opencode-config-apply', async (importOriginal) => { const actual = await importOriginal() @@ -189,13 +192,11 @@ vi.mock('../../src/services/opencode-single-server', async (importOriginal) => { class MockConfigReloadError extends Error { validationIssues: Array<{ path: string; message: string }> - removedFields: string[] - constructor(message: string, validationIssues: Array<{ path: string; message: string }> = [], removedFields: string[] = []) { + constructor(message: string, validationIssues: Array<{ path: string; message: string }> = []) { super(message) this.name = 'ConfigReloadError' this.validationIssues = validationIssues - this.removedFields = removedFields } } @@ -204,7 +205,6 @@ vi.mock('../../src/services/opencode-single-server', async (importOriginal) => { opencodeServerManager: { getVersion: vi.fn(), fetchVersion: vi.fn(), - reloadConfig: vi.fn(), restart: vi.fn(), clearStartupError: vi.fn(), getLastStartupError: vi.fn(), @@ -232,7 +232,6 @@ vi.mock('../../src/services/opencode-import', () => ({ getOpenCodeImportStatus: vi.fn(), syncOpenCodeImport: vi.fn(), getImportedSessionDirectories: vi.fn(), - getFirstExistingConfigSourcePath: vi.fn().mockReturnValue(null), })) vi.mock('../../src/services/repo', () => ({ @@ -260,9 +259,11 @@ vi.mock('@opencode-manager/shared/config/env', () => ({ getWorkspacePath: vi.fn(() => '/tmp/test-workspace'), getReposPath: vi.fn(() => '/tmp/test-repos'), getOpenCodeConfigFilePath: vi.fn(() => '/tmp/test-workspace/.config/opencode.json'), + getOpenCodeHealthWatchPath: vi.fn(() => '/tmp/test-workspace/health-watch'), getAgentsMdPath: vi.fn(() => '/tmp/test-workspace/AGENTS.md'), getDatabasePath: vi.fn(() => ':memory:'), getConfigPath: vi.fn(() => '/tmp/test-workspace/config'), + OPENCODE_CONFIG_SOURCE_NAMES: ['config.json', 'opencode.json', 'opencode.jsonc'], ENV: { SERVER: { PORT: 5003, HOST: '0.0.0.0', NODE_ENV: 'test' }, AUTH: { TRUSTED_ORIGINS: 'http://localhost:5173', SECRET: 'test-secret-for-encryption-key-32c' }, @@ -284,7 +285,7 @@ vi.mock('@opencode-manager/shared/config/env', () => ({ import { createSettingsRoutes } from '../../src/routes/settings' import { getImportedSessionDirectories, getOpenCodeImportStatus, OpenCodeImportProtectionError, syncOpenCodeImport } from '../../src/services/opencode-import' import { relinkReposFromSessionDirectories } from '../../src/services/repo' -import { opencodeServerManager, ConfigReloadError } from '../../src/services/opencode-single-server' +import { opencodeServerManager } from '../../src/services/opencode-single-server' import { detectSandboxCapability } from '../../src/services/sandbox/capability' import { forceProcessAttestation } from '../../src/services/opencode/process-identity' import { setOpenCodeRestartCoordinator } from '../../src/services/opencode-restart' @@ -293,7 +294,6 @@ import { createRepo } from '../../src/db/queries' const mockSpawnSync = spawnSync as ReturnType const mockGetVersion = opencodeServerManager.getVersion as ReturnType const mockFetchVersion = opencodeServerManager.fetchVersion as ReturnType -const mockReloadConfig = opencodeServerManager.reloadConfig as ReturnType const mockRestart = opencodeServerManager.restart as ReturnType const mockClearStartupError = opencodeServerManager.clearStartupError as ReturnType const mockGetLastStartupError = opencodeServerManager.getLastStartupError as ReturnType @@ -312,7 +312,6 @@ describe('Settings Routes - OpenCode Upgrade', () => { vi.clearAllMocks() mockGetVersion.mockReset() mockFetchVersion.mockReset() - mockReloadConfig.mockReset() mockRestart.mockReset() mockClearStartupError.mockReset() mockIsSandboxEnforced.mockReset() @@ -325,8 +324,9 @@ describe('Settings Routes - OpenCode Upgrade', () => { mockGetImportedSessionDirectories.mockReset() mockRelinkReposFromSessionDirectories.mockReset() mockReadOpenCodeConfigFile.mockReset() - mockWriteOpenCodeConfigFile.mockReset() mockDeleteOpenCodeConfigFile.mockReset() + mockArchiveBrokenOpenCodeConfigFile.mockReset() + mockRestoreOpenCodeConfigSnapshot.mockReset() mockApplyOpenCodeConfigUpdate.mockReset() mockDetectSandboxCapability.mockReset() mockDetectSandboxCapability.mockReturnValue({ available: true, msbVersion: 'msb 1.0.0' }) @@ -339,7 +339,6 @@ describe('Settings Routes - OpenCode Upgrade', () => { testDb = {} as any settingsApp = createSettingsRoutes(testDb, { getGitEnvironment: vi.fn().mockReturnValue({}) } as any, createStubOpenCodeClient()) - mockReloadConfig.mockResolvedValue(undefined) mockRestart.mockResolvedValue(undefined) mockClearStartupError.mockReturnValue(undefined) mockGetOpenCodeImportStatus.mockResolvedValue({ @@ -411,12 +410,13 @@ describe('Settings Routes - OpenCode Upgrade', () => { expect(json).toEqual({ ...config, restartRequired: true }) expect(mockApplyOpenCodeConfigUpdate).toHaveBeenCalledWith({ content: '{"plugin":["evil-plugin"]}', - openCodeClient: expect.anything(), + source: undefined, + expectedRevision: undefined, settingsService: expect.anything(), }) }) - it('maps an applied result to 200 without removedFields when none were removed', async () => { + it('maps an unchanged applied result to 200 without requesting a restart', async () => { const config = { path: '/tmp/test-workspace/.config/opencode.json', content: { theme: 'light' }, @@ -424,7 +424,7 @@ describe('Settings Routes - OpenCode Upgrade', () => { isValid: true, updatedAt: 3, } - mockApplyOpenCodeConfigUpdate.mockResolvedValueOnce({ status: 'applied', config, removedFields: [] }) + mockApplyOpenCodeConfigUpdate.mockResolvedValueOnce({ status: 'applied', config }) const res = await settingsApp.fetch(new Request('http://localhost/opencode-config', { method: 'PUT', @@ -435,64 +435,14 @@ describe('Settings Routes - OpenCode Upgrade', () => { expect(res.status).toBe(200) expect(json).toEqual(config) - expect(json.removedFields).toBeUndefined() expect(mockApplyOpenCodeConfigUpdate).toHaveBeenCalledWith({ content: { theme: 'light' }, - openCodeClient: expect.anything(), + source: undefined, + expectedRevision: undefined, settingsService: expect.anything(), }) }) - it('maps an applied result with removedFields to 200', async () => { - const config = { - path: '/tmp/test-workspace/.config/opencode.json', - content: { theme: 'light' }, - rawContent: '{"theme":"light"}', - isValid: true, - updatedAt: 3, - } - mockApplyOpenCodeConfigUpdate.mockResolvedValueOnce({ - status: 'applied', - config, - removedFields: ['command.review'], - }) - - const res = await settingsApp.fetch(new Request('http://localhost/opencode-config', { - method: 'PUT', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ content: '{"command":{"review":true},"theme":"light"}' }), - })) - const json = await res.json() as Record - - expect(res.status).toBe(200) - expect(json).toEqual({ ...config, removedFields: ['command.review'] }) - }) - - it('maps a rejected apply result to 400 with validation issues', async () => { - const validationIssues = [{ path: 'command.review', message: 'Invalid field' }] - mockApplyOpenCodeConfigUpdate.mockResolvedValueOnce({ - status: 'rejected', - error: 'command.review: Invalid field', - validationIssues, - removedFields: ['command.review'], - }) - - const res = await settingsApp.fetch(new Request('http://localhost/opencode-config', { - method: 'PUT', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ content: '{"command":{"review":true}}' }), - })) - const json = await res.json() as Record - - expect(res.status).toBe(400) - expect(json).toEqual({ - error: 'Config validation failed', - details: 'command.review: Invalid field', - validationIssues, - removedFields: ['command.review'], - }) - }) - it('returns 400 when the PUT body fails schema validation', async () => { const res = await settingsApp.fetch(new Request('http://localhost/opencode-config', { method: 'PUT', @@ -545,9 +495,16 @@ describe('Settings Routes - OpenCode Upgrade', () => { expect(json.error).toBe('Failed to update OpenCode config') }) - it('writes the last known good config and reloads on rollback', async () => { + it('writes the last known good config and restarts on rollback', async () => { mockGetLastKnownGoodConfig.mockReturnValueOnce('{"theme":"dark"}') - mockWriteOpenCodeConfigFile.mockResolvedValueOnce({ + mockRestoreOpenCodeConfigSnapshot.mockResolvedValueOnce({ + path: '/tmp/test-workspace/.config/opencode.json', + rawContent: '{"theme":"dark"}', + content: { theme: 'dark' }, + isValid: true, + updatedAt: 1, + }) + mockReadOpenCodeConfigFile.mockResolvedValueOnce({ path: '/tmp/test-workspace/.config/opencode.json', rawContent: '{"theme":"dark"}', content: { theme: 'dark' }, @@ -560,9 +517,9 @@ describe('Settings Routes - OpenCode Upgrade', () => { expect(res.status).toBe(200) expect(json).toEqual({ success: true, message: 'Server reloaded with the previous working config' }) - expect(mockWriteOpenCodeConfigFile).toHaveBeenCalledWith('{"theme":"dark"}') + expect(mockRestoreOpenCodeConfigSnapshot).toHaveBeenCalledWith('{"theme":"dark"}') expect(mockClearStartupError).toHaveBeenCalled() - expect(mockReloadConfig).toHaveBeenCalled() + expect(mockRestart).toHaveBeenCalled() }) it('returns 404 from rollback when no last known good config exists', async () => { @@ -573,19 +530,19 @@ describe('Settings Routes - OpenCode Upgrade', () => { expect(res.status).toBe(404) expect(json.error).toBe('No previous working config available for rollback') - expect(mockWriteOpenCodeConfigFile).not.toHaveBeenCalled() + expect(mockRestoreOpenCodeConfigSnapshot).not.toHaveBeenCalled() }) it('deletes the config file and restarts when the rollback reload fails', async () => { mockGetLastKnownGoodConfig.mockReturnValueOnce('{"theme":"dark"}') - mockWriteOpenCodeConfigFile.mockResolvedValueOnce({ + mockRestoreOpenCodeConfigSnapshot.mockResolvedValueOnce({ path: '/tmp/test-workspace/.config/opencode.json', rawContent: '{"theme":"dark"}', content: { theme: 'dark' }, isValid: true, updatedAt: 1, }) - mockReloadConfig.mockRejectedValueOnce(new Error('reload failed')) + mockReadOpenCodeConfigFile.mockResolvedValueOnce(null) mockDeleteOpenCodeConfigFile.mockResolvedValueOnce(true) const res = await settingsApp.fetch(new Request('http://localhost/opencode-rollback', { method: 'POST' })) @@ -597,7 +554,8 @@ describe('Settings Routes - OpenCode Upgrade', () => { message: 'Server restarted after deleting the broken config file. The previous working config remains available for rollback.', fallback: true, }) - expect(mockWriteOpenCodeConfigFile).toHaveBeenCalledWith('{"theme":"dark"}') + expect(mockRestoreOpenCodeConfigSnapshot).toHaveBeenCalledWith('{"theme":"dark"}') + expect(mockArchiveBrokenOpenCodeConfigFile).toHaveBeenCalled() expect(mockDeleteOpenCodeConfigFile).toHaveBeenCalled() expect(mockRestart).toHaveBeenCalled() }) @@ -776,7 +734,6 @@ describe('Settings Routes - OpenCode Upgrade', () => { expect(json.newVersion).toBe('1.0.1') expect(mockFetchVersion).toHaveBeenCalledTimes(1) expect(mockRestart).toHaveBeenCalledTimes(1) - expect(mockReloadConfig).not.toHaveBeenCalled() }) it('should return already up to date when version unchanged', async () => { @@ -808,7 +765,6 @@ describe('Settings Routes - OpenCode Upgrade', () => { await settingsApp.fetch(req) expect(mockRestart).toHaveBeenCalledTimes(1) - expect(mockReloadConfig).not.toHaveBeenCalled() }) it('allows upgrading while sandbox enforcement is active', async () => { @@ -1170,18 +1126,26 @@ describe('Settings Routes - OpenCode Upgrade', () => { }) describe('POST /opencode-reload', () => { + const validConfigFile = { + path: '/tmp/test-workspace/.config/opencode.json', + content: {}, + rawContent: '{}', + isValid: true, + updatedAt: 1, + } + beforeEach(() => { vi.clearAllMocks() - mockReloadConfig.mockReset() + setOpenCodeRestartCoordinator(null) + mockReadOpenCodeConfigFile.mockReset() mockRestart.mockReset() mockClearStartupError.mockReset() - mockReloadConfig.mockResolvedValue(undefined) mockRestart.mockResolvedValue(undefined) mockClearStartupError.mockReturnValue(undefined) }) - it('should return success when reload succeeds', async () => { - mockReloadConfig.mockResolvedValueOnce(undefined) + it('should return success and restart the server when the config is valid', async () => { + mockReadOpenCodeConfigFile.mockResolvedValueOnce(validConfigFile) const req = new Request('http://localhost/opencode-reload', { method: 'POST' @@ -1191,19 +1155,19 @@ describe('Settings Routes - OpenCode Upgrade', () => { expect(res.status).toBe(200) expect(json.success).toBe(true) - expect(json.message).toBe('OpenCode configuration reloaded successfully') + expect(json.message).toBe('OpenCode server restarted with the current configuration') + expect(json.resumedSessions).toEqual([]) + expect(mockRestart).toHaveBeenCalledTimes(1) + expect(mockClearStartupError).toHaveBeenCalled() }) - it('should propagate validationIssues and removedFields when ConfigReloadError is thrown', async () => { + it('should propagate validationIssues and not restart when the config is invalid', async () => { const validationIssues = [ { path: 'command.review', message: 'Invalid field' }, { path: 'agent.temperature', message: 'Temperature out of range' } ] - const removedFields = ['command.review'] - mockReloadConfig.mockRejectedValueOnce( - new ConfigReloadError('Config validation failed', validationIssues, removedFields) - ) + mockReadOpenCodeConfigFile.mockResolvedValueOnce({ ...validConfigFile, isValid: false, validationIssues }) const req = new Request('http://localhost/opencode-reload', { method: 'POST' @@ -1212,14 +1176,14 @@ describe('Settings Routes - OpenCode Upgrade', () => { const json = await res.json() as Record expect(res.status).toBe(500) - expect(json.error).toBe('Config validation failed') + expect(json.error).toBe('OpenCode global configuration is invalid') expect(json.details).toBe('command.review: Invalid field; agent.temperature: Temperature out of range') expect(json.validationIssues).toEqual(validationIssues) - expect(json.removedFields).toEqual(removedFields) + expect(mockRestart).not.toHaveBeenCalled() }) - it('should return generic error when non-ConfigReloadError is thrown', async () => { - mockReloadConfig.mockRejectedValueOnce(new Error('Some other error')) + it('should return 500 and not restart when every global config source is absent', async () => { + mockReadOpenCodeConfigFile.mockResolvedValueOnce(null) const req = new Request('http://localhost/opencode-reload', { method: 'POST' @@ -1228,14 +1192,14 @@ describe('Settings Routes - OpenCode Upgrade', () => { const json = await res.json() as Record expect(res.status).toBe(500) - expect(json.error).toBe('Failed to reload OpenCode configuration') - expect(json.details).toBe('Some other error') + expect(json.error).toBe('No OpenCode global configuration files found') + expect(json.details).toBe('No OpenCode global configuration files found') + expect(mockRestart).not.toHaveBeenCalled() }) - it('should propagate empty arrays when ConfigReloadError has no issues', async () => { - mockReloadConfig.mockRejectedValueOnce( - new ConfigReloadError('Reload failed', [], []) - ) + it('should return generic error when the restart fails', async () => { + mockReadOpenCodeConfigFile.mockResolvedValueOnce(validConfigFile) + mockRestart.mockRejectedValueOnce(new Error('Some other error')) const req = new Request('http://localhost/opencode-reload', { method: 'POST' @@ -1244,17 +1208,15 @@ describe('Settings Routes - OpenCode Upgrade', () => { const json = await res.json() as Record expect(res.status).toBe(500) - expect(json.error).toBe('Reload failed') - expect(json.details).toBe('Reload failed') - expect(json.validationIssues).toEqual([]) - expect(json.removedFields).toEqual([]) + expect(json.error).toBe('Failed to reload OpenCode configuration') + expect(json.details).toBe('Some other error') }) - it('returns 500 with the startup failure reason when a supervisor reload is unhealthy', async () => { - mockGetLastStartupError.mockReturnValue('OpenCode config reload failed after recovery') + it('returns 500 with the startup failure reason when a supervisor restart is unhealthy', async () => { + mockReadOpenCodeConfigFile.mockResolvedValueOnce(validConfigFile) + mockGetLastStartupError.mockReturnValue('OpenCode restart failed after recovery') const unhealthySupervisor = { - restart: vi.fn(), - reloadConfig: vi.fn().mockResolvedValue({ healthy: false }), + restart: vi.fn().mockResolvedValue({ healthy: false }), } const app = createSettingsRoutes( testDb, @@ -1270,14 +1232,14 @@ describe('Settings Routes - OpenCode Upgrade', () => { expect(res.status).toBe(500) expect(json.success).toBeUndefined() expect(json.error).toBe('Failed to reload OpenCode configuration') - expect(json.details).toBe('OpenCode config reload failed after recovery') - expect(unhealthySupervisor.reloadConfig).toHaveBeenCalledWith('settings_reload') + expect(json.details).toBe('OpenCode restart failed after recovery') + expect(unhealthySupervisor.restart).toHaveBeenCalledWith('settings_reload') }) - it('returns success when a supervisor reload is healthy', async () => { + it('returns success when a supervisor restart is healthy', async () => { + mockReadOpenCodeConfigFile.mockResolvedValueOnce(validConfigFile) const healthySupervisor = { - restart: vi.fn(), - reloadConfig: vi.fn().mockResolvedValue({ healthy: true }), + restart: vi.fn().mockResolvedValue({ healthy: true }), } const app = createSettingsRoutes( testDb, @@ -1292,7 +1254,7 @@ describe('Settings Routes - OpenCode Upgrade', () => { expect(res.status).toBe(200) expect(json.success).toBe(true) - expect(healthySupervisor.reloadConfig).toHaveBeenCalledWith('settings_reload') + expect(healthySupervisor.restart).toHaveBeenCalledWith('settings_reload') }) }) @@ -1310,7 +1272,6 @@ describe('Settings Routes - OpenCode Upgrade', () => { mockGetLastStartupError.mockReturnValue('OpenCode version 1.18.15 does not support sandboxed bash tool rewriting') const unhealthySupervisor = { restart: vi.fn().mockResolvedValue({ healthy: false }), - reloadConfig: vi.fn(), } const app = createSettingsRoutes( testDb, @@ -1333,7 +1294,6 @@ describe('Settings Routes - OpenCode Upgrade', () => { it('returns success when a supervisor restart is healthy', async () => { const healthySupervisor = { restart: vi.fn().mockResolvedValue({ healthy: true }), - reloadConfig: vi.fn(), } const app = createSettingsRoutes( testDb, @@ -1473,7 +1433,6 @@ describe('Settings Routes - OpenCode Upgrade', () => { expect(res.status).toBe(200) expect(json.restartRequired).toBe(true) expect(opencodeServerManager.markRestartPending).toHaveBeenCalledTimes(1) - expect(mockReloadConfig).not.toHaveBeenCalled() }) it('requires a restart when the git identity changes', async () => { @@ -1716,7 +1675,7 @@ describe('Settings Routes - OpenCode Upgrade', () => { describe('Settings Routes - versions, directory files, skills, MCP and maintenance', () => { let app: ReturnType let db: Database - let supervisor: { restart: ReturnType; reloadConfig: ReturnType } + let supervisor: { restart: ReturnType } let fetchMock: ReturnType beforeEach(() => { @@ -1750,7 +1709,6 @@ describe('Settings Routes - versions, directory files, skills, MCP and maintenan migrate(db, allMigrations) supervisor = { restart: vi.fn().mockResolvedValue({ healthy: true, resumedSessionIDs: [] }), - reloadConfig: vi.fn().mockResolvedValue({ healthy: true }), } app = createSettingsRoutes( db, diff --git a/backend/test/services/opencode-config-apply.test.ts b/backend/test/services/opencode-config-apply.test.ts index 1cc95408..4f35b401 100644 --- a/backend/test/services/opencode-config-apply.test.ts +++ b/backend/test/services/opencode-config-apply.test.ts @@ -1,19 +1,18 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { mkdtemp, readFile, rm, writeFile } from 'fs/promises' +import { 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: '' })) +const paths = vi.hoisted(() => ({ config: '', configDir: '', healthWatch: '' })) -vi.mock('@opencode-manager/shared/config/env', async (importOriginal) => { - const actual = await importOriginal() - return { - ...actual, - getOpenCodeConfigFilePath: () => paths.config, - } -}) +vi.mock('@opencode-manager/shared/config/env', () => ({ + getConfigPath: () => paths.configDir, + getOpenCodeConfigFilePath: () => paths.config, + getOpenCodeHealthWatchPath: () => paths.healthWatch, + OPENCODE_CONFIG_SOURCE_NAMES: ['config.json', 'opencode.json', 'opencode.jsonc'], +})) vi.mock('../../src/utils/logger', () => ({ logger: { @@ -23,11 +22,6 @@ vi.mock('../../src/utils/logger', () => ({ }, })) -const patchConfigWithRecoveryMock = vi.hoisted(() => vi.fn()) -vi.mock('../../src/services/opencode/config-recovery', () => ({ - patchConfigWithRecovery: patchConfigWithRecoveryMock, -})) - const markRestartPendingMock = vi.hoisted(() => vi.fn()) const clearStartupErrorMock = vi.hoisted(() => vi.fn()) vi.mock('../../src/services/opencode-single-server', () => ({ @@ -40,8 +34,20 @@ vi.mock('../../src/services/opencode-single-server', () => ({ import { migrate } from '../../src/db/migration-runner' import { allMigrations } from '../../src/db/migrations' import { SettingsService } from '../../src/services/settings' -import { applyOpenCodeConfigUpdate, captureLastKnownGoodOpenCodeConfig, restoreLastKnownGoodOpenCodeConfig, type ApplyOpenCodeConfigResult } from '../../src/services/opencode-config-apply' -import type { OpenCodeClient } from '../../src/services/opencode/client' +import { + applyOpenCodeConfigUpdate, + captureLastKnownGoodOpenCodeConfig, + restoreLastKnownGoodOpenCodeConfig, + seedOpenCodeConfigFile, + type ApplyOpenCodeConfigResult, +} from '../../src/services/opencode-config-apply' +import { + OPENCODE_CONFIG_SEED, + OpenCodeConfigConflictError, + OpenCodeConfigSourceInvalidError, + readOpenCodeConfigFile, + writeOpenCodeConfigFile, +} from '../../src/services/opencode-config-file' function expectStatus( result: ApplyOpenCodeConfigResult, @@ -51,20 +57,28 @@ function expectStatus( return result as Extract } +function parseSnapshot(snapshot: string): { version: number; sources: Array<{ name: string; rawContent: string }> } { + return JSON.parse(snapshot) as { version: number; sources: Array<{ name: string; rawContent: string }> } +} + describe('opencode-config-apply', () => { let workDir: string let db: Database let settingsService: SettingsService - let openCodeClient: OpenCodeClient + + function sourcePath(name: string): string { + return path.join(workDir, name) + } beforeEach(async () => { vi.clearAllMocks() workDir = await mkdtemp(path.join(tmpdir(), 'opencode-config-apply-')) paths.config = path.join(workDir, 'opencode.json') + paths.configDir = workDir + paths.healthWatch = path.join(workDir, 'health-watch') db = new Database(':memory:') migrate(db, allMigrations) settingsService = new SettingsService(db) - openCodeClient = { forward: vi.fn() } as unknown as OpenCodeClient }) afterEach(async () => { @@ -72,125 +86,181 @@ describe('opencode-config-apply', () => { await rm(workDir, { recursive: true, force: true }) }) - it('writes the file and marks a restart pending for a plugin change without patching the live server', async () => { - await writeFile(paths.config, '{"theme":"dark"}', 'utf8') + it.each([ + ['theme', { theme: 'light' }], + ['model', { model: 'a/b' }], + ['mcp', { mcp: { local: { type: 'local' } } }], + ['agent', { agent: { build: { model: 'a/b' } } }], + ['plugin', { plugin: ['x'] }], + ['provider', { provider: { example: { npm: 'y' } } }], + ])('marks a restart pending for a semantic %s change without patching the live server', async (_field, change) => { + await writeFile(sourcePath('opencode.json'), '{"theme":"dark"}', 'utf8') const result = expectStatus(await applyOpenCodeConfigUpdate({ - content: '{"theme":"dark","plugin":["x"]}', - openCodeClient, + content: change, settingsService, }), 'restart_pending') - expect(result.config.rawContent).toBe('{"theme":"dark","plugin":["x"]}') - await expect(readFile(paths.config, 'utf8')).resolves.toBe('{"theme":"dark","plugin":["x"]}') expect(markRestartPendingMock).toHaveBeenCalledTimes(1) - expect(patchConfigWithRecoveryMock).not.toHaveBeenCalled() + expect(result.config.content).toEqual(change) }) - it('patches a live-applied change and writes the submitted raw text verbatim', async () => { - await writeFile(paths.config, '{"theme":"dark"}', 'utf8') - patchConfigWithRecoveryMock.mockResolvedValue({ - success: true, - appliedConfig: { theme: 'dark', mcp: { local: { type: 'local' } } }, - }) + it('applies a comment-only edit without marking a restart pending', async () => { + await writeFile(sourcePath('opencode.json'), '{"theme":"dark"}', 'utf8') + const commented = '{\n // keep this comment\n "theme": "dark"\n}\n' - const submitted = '{\n // keep this comment\n "theme": "dark",\n "mcp": { "local": { "type": "local" } }\n}\n' const result = expectStatus(await applyOpenCodeConfigUpdate({ - content: submitted, - openCodeClient, + content: commented, + source: 'opencode.json', settingsService, }), 'applied') - expect(result.removedFields).toEqual([]) - await expect(readFile(paths.config, 'utf8')).resolves.toBe(submitted) + expect(result.config.rawContent).toBe(commented) + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe(commented) expect(markRestartPendingMock).not.toHaveBeenCalled() }) - it('writes the cleaned applied config and reports removed fields when recovery drops them', async () => { - await writeFile(paths.config, '{"theme":"dark"}', 'utf8') - const appliedConfig = { mcp: { local: { type: 'local' } } } - patchConfigWithRecoveryMock.mockResolvedValue({ - success: true, - appliedConfig, - removedFields: ['theme'], - }) + it('writes a raw string to the explicitly requested source verbatim', async () => { + await writeFile(sourcePath('opencode.json'), '{"model":"a/b"}', 'utf8') + const submitted = '{\n // config source\n "theme": "light"\n}\n' const result = expectStatus(await applyOpenCodeConfigUpdate({ - content: '{"theme":"dark","mcp":{"local":{"type":"local"}}}', - openCodeClient, + content: submitted, + source: 'config.json', settingsService, - }), 'applied') + }), 'restart_pending') - expect(result.removedFields).toEqual(['theme']) - await expect(readFile(paths.config, 'utf8')).resolves.toBe(JSON.stringify(appliedConfig, null, 2)) + await expect(readFile(sourcePath('config.json'), 'utf8')).resolves.toBe(submitted) + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe('{"model":"a/b"}') + expect(result.config.content).toEqual({ model: 'a/b', theme: 'light' }) }) - it('leaves the previous file byte-identical and returns rejected when the live patch fails', async () => { - const previous = '{\n // previous\n "theme": "dark"\n}\n' - await writeFile(paths.config, previous, 'utf8') - patchConfigWithRecoveryMock.mockResolvedValue({ - success: false, - error: 'model: must be string', - details: [{ path: 'model', message: 'must be string' }], - removedFields: ['model'], - }) + it('throws a conflict and preserves last known good when the expected revision is stale', async () => { + const initial = await writeOpenCodeConfigFile('{"theme":"dark"}', 'opencode.jsonc') + settingsService.saveLastKnownGoodConfig('sentinel') + await writeFile(sourcePath('opencode.json'), '{"theme":"light"}', 'utf8') - const result = expectStatus(await applyOpenCodeConfigUpdate({ - content: { model: 'x' }, - openCodeClient, + await expect(applyOpenCodeConfigUpdate({ + content: { theme: 'system' }, + expectedRevision: initial.revision, settingsService, - }), 'rejected') + })).rejects.toBeInstanceOf(OpenCodeConfigConflictError) - expect(result.error).toBe('model: must be string') - expect(result.validationIssues).toEqual([{ path: 'model', message: 'must be string' }]) - expect(result.removedFields).toEqual(['model']) - await expect(readFile(paths.config, 'utf8')).resolves.toBe(previous) + expect(settingsService.getLastKnownGoodConfig()).toBe('sentinel') + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe('{"theme":"light"}') expect(markRestartPendingMock).not.toHaveBeenCalled() }) - it('saves a valid previous file as last known good before writing', async () => { + it('saves the valid prior full snapshot as last known good after a successful write', async () => { const previous = '{"theme":"dark"}' - await writeFile(paths.config, previous, 'utf8') - patchConfigWithRecoveryMock.mockResolvedValue({ success: true }) + await writeFile(sourcePath('opencode.json'), previous, 'utf8') await applyOpenCodeConfigUpdate({ - content: { mcp: {} }, - openCodeClient, + content: { theme: 'light' }, settingsService, }) - expect(settingsService.getLastKnownGoodConfig()).toBe(previous) - expect(settingsService.getSettings().preferences.lastKnownGoodConfig).toBe(previous) + const lastGood = settingsService.getLastKnownGoodConfig() + expect(lastGood).not.toBeNull() + const parsed = parseSnapshot(lastGood as string) + expect(parsed.version).toBe(1) + expect(parsed.sources).toEqual([{ name: 'opencode.json', rawContent: previous }]) }) - it('does not capture an invalid previous file as last known good', async () => { - await writeFile(paths.config, '{"model": 5}', 'utf8') + it('rejects an object update when a source is invalid and keeps last known good', async () => { + await writeFile(sourcePath('opencode.json'), '{"model": 5}', 'utf8') settingsService.saveLastKnownGoodConfig('sentinel') - patchConfigWithRecoveryMock.mockResolvedValue({ success: true }) - const result = expectStatus(await applyOpenCodeConfigUpdate({ + await expect(applyOpenCodeConfigUpdate({ content: { theme: 'light' }, - openCodeClient, settingsService, - }), 'applied') + })).rejects.toBeInstanceOf(OpenCodeConfigSourceInvalidError) - expect(result.config.rawContent).toBe('{\n "theme": "light"\n}') expect(settingsService.getLastKnownGoodConfig()).toBe('sentinel') + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe('{"model": 5}') + expect(markRestartPendingMock).not.toHaveBeenCalled() }) - it('captures the on-disk config as last known good when it is valid', async () => { + it('rejects invalid raw content and writes nothing', async () => { const previous = '{"theme":"dark"}' - await writeFile(paths.config, previous, 'utf8') - settingsService.saveLastKnownGoodConfig('sentinel') + await writeFile(sourcePath('opencode.json'), previous, 'utf8') - const captured = await captureLastKnownGoodOpenCodeConfig(settingsService) + await expect(applyOpenCodeConfigUpdate({ + content: '{"model": 5}', + source: 'opencode.json', + settingsService, + })).rejects.toBeInstanceOf(ZodError) + + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe(previous) + expect(markRestartPendingMock).not.toHaveBeenCalled() + }) + + it('restores the prior sources if persisting last known good fails', async () => { + const lower = '{"model":"a/b"}' + const higher = '{\n // keep\n "theme":"dark"\n}' + await writeFile(sourcePath('config.json'), lower) + await writeFile(sourcePath('opencode.jsonc'), higher) + vi.spyOn(settingsService, 'saveLastKnownGoodConfig').mockImplementation(() => { + throw new Error('database write failed') + }) - expect(captured?.rawContent).toBe(previous) - expect(settingsService.getLastKnownGoodConfig()).toBe(previous) + await expect(applyOpenCodeConfigUpdate({ + content: { model: 'a/b', theme: 'light' }, + settingsService, + })).rejects.toThrow('database write failed') + + expect(await readFile(sourcePath('config.json'), 'utf8')).toBe(lower) + expect(await readFile(sourcePath('opencode.jsonc'), 'utf8')).toBe(higher) + expect(markRestartPendingMock).not.toHaveBeenCalled() + }) + + it('requires a restart after repairing an invalid source even when merged values are unchanged', async () => { + await writeFile(sourcePath('config.json'), '{"model":123}') + await writeFile(sourcePath('opencode.jsonc'), '{"model":"a/b"}') + + const result = await applyOpenCodeConfigUpdate({ + content: '{}', + source: 'config.json', + settingsService, + }) + + expect(result.status).toBe('restart_pending') + expect(markRestartPendingMock).toHaveBeenCalledOnce() }) - it('does not overwrite last known good when the on-disk config is invalid', async () => { - await writeFile(paths.config, '{"model": 5}', 'utf8') + it('preserves unknown fields sent back with the merged content', async () => { + const raw = JSON.stringify({ theme: 'dark', customTool: { enabled: true } }) + await writeFile(sourcePath('opencode.json'), raw, 'utf8') + const current = await readOpenCodeConfigFile() + + const result = expectStatus(await applyOpenCodeConfigUpdate({ + content: { ...current!.content, theme: 'light' }, + settingsService, + }), 'restart_pending') + + expect(result.config.content).toEqual({ theme: 'light', customTool: { enabled: true } }) + const onDisk = JSON.parse(await readFile(sourcePath('opencode.json'), 'utf8')) as Record + expect(onDisk).toEqual({ theme: 'light', customTool: { enabled: true } }) + }) + + it('captures every source as last known good and restores them together', async () => { + await writeFile(sourcePath('config.json'), '{"model":"a/b"}', 'utf8') + await writeFile(sourcePath('opencode.jsonc'), '{"theme":"dark"}', 'utf8') + + await captureLastKnownGoodOpenCodeConfig(settingsService) + await rm(sourcePath('config.json')) + await rm(sourcePath('opencode.jsonc')) + + const restored = await restoreLastKnownGoodOpenCodeConfig(settingsService) + + expect(restored?.content).toEqual({ model: 'a/b', theme: 'dark' }) + await expect(readFile(sourcePath('config.json'), 'utf8')).resolves.toBe('{"model":"a/b"}') + await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).resolves.toBe('{"theme":"dark"}') + expect(clearStartupErrorMock).toHaveBeenCalledTimes(1) + }) + + it('does not capture an invalid prior snapshot as last known good', async () => { + await writeFile(sourcePath('opencode.json'), '{"model": 5}', 'utf8') settingsService.saveLastKnownGoodConfig('sentinel') const captured = await captureLastKnownGoodOpenCodeConfig(settingsService) @@ -199,6 +269,16 @@ describe('opencode-config-apply', () => { expect(settingsService.getLastKnownGoodConfig()).toBe('sentinel') }) + it('restores a legacy raw last known good snapshot as an opencode.json source', async () => { + const service = { getLastKnownGoodConfig: () => '{"theme":"dark"}' } as unknown as SettingsService + + const restored = await restoreLastKnownGoodOpenCodeConfig(service) + + expect(restored?.content).toEqual({ theme: 'dark' }) + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe('{"theme":"dark"}') + expect(clearStartupErrorMock).toHaveBeenCalledTimes(1) + }) + it('returns null from restore when no last known good config exists', async () => { const service = { getLastKnownGoodConfig: () => null } as unknown as SettingsService @@ -206,69 +286,62 @@ describe('opencode-config-apply', () => { expect(clearStartupErrorMock).not.toHaveBeenCalled() }) - it('writes the last known good config and clears the startup error on restore', async () => { - const service = { getLastKnownGoodConfig: () => '{"theme":"dark"}' } as unknown as SettingsService + it('applies an mcp-only change without marking a restart pending', async () => { + await writeFile(sourcePath('opencode.json'), '{"theme":"dark"}', 'utf8') - const restored = await restoreLastKnownGoodOpenCodeConfig(service) + const result = expectStatus(await applyOpenCodeConfigUpdate({ + content: { theme: 'dark', mcp: { local: { type: 'local' } } }, + settingsService, + }), 'applied') - expect(restored?.rawContent).toBe('{"theme":"dark"}') - await expect(readFile(paths.config, 'utf8')).resolves.toBe('{"theme":"dark"}') - expect(clearStartupErrorMock).toHaveBeenCalledTimes(1) + expect(result.config.content).toEqual({ theme: 'dark', mcp: { local: { type: 'local' } } }) + expect(markRestartPendingMock).not.toHaveBeenCalled() }) - it('throws ZodError and writes nothing when the submitted content is invalid', async () => { - const previous = '{"theme":"dark"}' - await writeFile(paths.config, previous, 'utf8') + it('requires a restart when mcp changes alongside another key', async () => { + await writeFile(sourcePath('opencode.json'), '{"theme":"dark"}', 'utf8') - await expect(applyOpenCodeConfigUpdate({ - content: '{"model": 5}', - openCodeClient, + const result = expectStatus(await applyOpenCodeConfigUpdate({ + content: { theme: 'light', mcp: { local: { type: 'local' } } }, settingsService, - })).rejects.toBeInstanceOf(ZodError) + }), 'restart_pending') - await expect(readFile(paths.config, 'utf8')).resolves.toBe(previous) - expect(patchConfigWithRecoveryMock).not.toHaveBeenCalled() - expect(markRestartPendingMock).not.toHaveBeenCalled() + expect(result.config.content).toEqual({ theme: 'light', mcp: { local: { type: 'local' } } }) + expect(markRestartPendingMock).toHaveBeenCalledTimes(1) }) - it('serializes concurrent applies so their live patches do not interleave', async () => { - await writeFile(paths.config, '{"theme":"dark"}', 'utf8') - - const events: string[] = [] - let releaseFirstPatch!: () => void - patchConfigWithRecoveryMock - .mockImplementationOnce(() => { - events.push('first:start') - return new Promise((resolve) => { - releaseFirstPatch = () => { - events.push('first:end') - resolve({ success: true }) - } - }) - }) - .mockImplementationOnce(() => { - events.push('second:start') - return Promise.resolve({ success: true }) - }) - - const first = applyOpenCodeConfigUpdate({ - content: { mcp: { first: { type: 'local' } } }, - openCodeClient, + it('requires a restart for a theme-only change', async () => { + await writeFile(sourcePath('opencode.json'), '{"theme":"dark"}', 'utf8') + + expectStatus(await applyOpenCodeConfigUpdate({ + content: { theme: 'light' }, settingsService, - }) - await vi.waitFor(() => expect(events).toContain('first:start')) + }), 'restart_pending') + + expect(markRestartPendingMock).toHaveBeenCalledTimes(1) + }) + + it('applies an unchanged content edit without marking a restart pending', async () => { + await writeFile(sourcePath('opencode.json'), '{"theme":"dark"}', 'utf8') - const second = applyOpenCodeConfigUpdate({ - content: { mcp: { second: { type: 'local' } } }, - openCodeClient, + expectStatus(await applyOpenCodeConfigUpdate({ + content: { theme: 'dark' }, settingsService, - }) - await new Promise((resolve) => setTimeout(resolve, 20)) - expect(events).toEqual(['first:start']) + }), 'applied') + + expect(markRestartPendingMock).not.toHaveBeenCalled() + }) + + it('seeds a minimal opencode.jsonc snapshot and removes every other source', async () => { + await writeFile(sourcePath('config.json'), '{"model":"a/b"}', 'utf8') + await writeFile(sourcePath('opencode.json'), '{"theme":"dark"}', 'utf8') - releaseFirstPatch() - await Promise.all([first, second]) + const seeded = await seedOpenCodeConfigFile() - expect(events).toEqual(['first:start', 'first:end', 'second:start']) + expect(seeded.path).toBe(sourcePath('opencode.jsonc')) + expect(seeded.rawContent).toBe(OPENCODE_CONFIG_SEED) + expect(seeded.content).toEqual({ $schema: 'https://opencode.ai/config.json' }) + expect(seeded.isValid).toBe(true) + await expect(readdir(workDir)).resolves.toEqual(['opencode.jsonc']) }) }) diff --git a/backend/test/services/opencode-config-file.test.ts b/backend/test/services/opencode-config-file.test.ts index 4a0f2194..4bee5abd 100644 --- a/backend/test/services/opencode-config-file.test.ts +++ b/backend/test/services/opencode-config-file.test.ts @@ -4,11 +4,13 @@ import { tmpdir } from 'os' import path from 'path' import { ZodError } from 'zod' -const paths = vi.hoisted(() => ({ config: '', healthWatch: '' })) +const paths = vi.hoisted(() => ({ config: '', configDir: '', healthWatch: '' })) vi.mock('@opencode-manager/shared/config/env', () => ({ + getConfigPath: () => paths.configDir, getOpenCodeConfigFilePath: () => paths.config, getOpenCodeHealthWatchPath: () => paths.healthWatch, + OPENCODE_CONFIG_SOURCE_NAMES: ['config.json', 'opencode.json', 'opencode.jsonc'], })) vi.mock('../../src/utils/logger', () => ({ @@ -19,14 +21,59 @@ vi.mock('../../src/utils/logger', () => ({ }, })) +const writeFailures = vi.hoisted(() => ({ + paths: [] as string[], + calls: [] as string[], + readsAtWrite: [] as Array<{ stat: number; readFile: number }>, +})) + +const fsCalls = vi.hoisted(() => ({ stat: 0, readFile: 0 })) + +vi.mock('fs/promises', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + stat: vi.fn((...args: Parameters) => { + fsCalls.stat += 1 + return actual.stat(...args) + }), + readFile: vi.fn((...args: Parameters) => { + fsCalls.readFile += 1 + return actual.readFile(...args) + }), + } +}) + +vi.mock('../../src/utils/fs-safe', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + writeFileAtomic: vi.fn(async (filePath: string, content: string, options?: { mode?: number }) => { + writeFailures.calls.push(filePath) + writeFailures.readsAtWrite.push({ stat: fsCalls.stat, readFile: fsCalls.readFile }) + if (writeFailures.paths.includes(filePath)) { + throw new Error(`simulated write failure for ${filePath}`) + } + return actual.writeFileAtomic(filePath, content, options) + }), + } +}) + import { HEALTH_WATCH_MAX_ENTRIES, OPENCODE_CONFIG_SEED, + OpenCodeConfigConflictError, + OpenCodeConfigShadowedRemovalError, + OpenCodeConfigSnapshotError, archiveBrokenOpenCodeConfigFile, deleteOpenCodeConfigFile, pruneHealthWatchDirectory, readOpenCodeConfigFile, + readOpenCodeConfigSnapshot, + restoreOpenCodeConfigSnapshot, + serializeOpenCodeConfigSnapshot, toOpenCodeConfigValidationIssues, + updateOpenCodeConfigFile, writeHealthWatchArtifact, writeOpenCodeConfigFile, } from '../../src/services/opencode-config-file' @@ -34,10 +81,20 @@ import { describe('opencode-config-file', () => { let workDir: string + function sourcePath(name: string): string { + return path.join(workDir, name) + } + beforeEach(async () => { vi.clearAllMocks() + writeFailures.paths = [] + writeFailures.calls = [] + writeFailures.readsAtWrite = [] + fsCalls.stat = 0 + fsCalls.readFile = 0 workDir = await mkdtemp(path.join(tmpdir(), 'opencode-config-file-')) paths.config = path.join(workDir, 'opencode.json') + paths.configDir = workDir paths.healthWatch = path.join(workDir, 'health-watch') }) @@ -45,29 +102,31 @@ describe('opencode-config-file', () => { await rm(workDir, { recursive: true, force: true }) }) - it('returns null when the config file does not exist', async () => { + it('returns null when no config source exists', async () => { await expect(readOpenCodeConfigFile()).resolves.toBeNull() }) - it('writes a valid config and reads it back', async () => { + it('writes a valid config to the default jsonc source and reads it back', async () => { const written = await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED) - expect(written.path).toBe(paths.config) + expect(written.path).toBe(sourcePath('opencode.jsonc')) expect(written.rawContent).toBe(OPENCODE_CONFIG_SEED) expect(written.content).toEqual({ $schema: 'https://opencode.ai/config.json' }) expect(written.isValid).toBe(true) expect(written.updatedAt).toBeGreaterThan(0) + expect(written.revision).toMatch(/^[a-f0-9]{64}$/) + await expect(readdir(workDir)).resolves.toEqual(['opencode.jsonc']) }) it('preserves an existing non-default file mode when rewriting atomically', async () => { const previousUmask = process.umask(0) try { - await writeFile(paths.config, OPENCODE_CONFIG_SEED, 'utf8') - await chmod(paths.config, 0o640) + await writeFile(sourcePath('opencode.jsonc'), OPENCODE_CONFIG_SEED, 'utf8') + await chmod(sourcePath('opencode.jsonc'), 0o640) await writeOpenCodeConfigFile('{"theme":"dark"}') - const stats = await stat(paths.config) + const stats = await stat(sourcePath('opencode.jsonc')) expect(stats.mode & 0o777).toBe(0o640) } finally { process.umask(previousUmask) @@ -78,22 +137,57 @@ describe('opencode-config-file', () => { await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED) await expect(writeOpenCodeConfigFile('{"model": 5}')).rejects.toBeInstanceOf(ZodError) - await expect(readFile(paths.config, 'utf8')).resolves.toBe(OPENCODE_CONFIG_SEED) + await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).resolves.toBe(OPENCODE_CONFIG_SEED) }) it('parses JSONC comments and preserves the raw content', async () => { const rawContent = '{\n // user comment\n "theme": "dark"\n}\n' - await writeFile(paths.config, rawContent, 'utf8') + await writeFile(sourcePath('opencode.json'), rawContent, 'utf8') const file = await readOpenCodeConfigFile() + expect(file?.path).toBe(sourcePath('opencode.json')) expect(file?.content).toEqual({ theme: 'dark' }) expect(file?.rawContent).toBe(rawContent) expect(file?.isValid).toBe(true) }) + it('merges sources in config.json, opencode.json, opencode.jsonc precedence with recursive objects and replacing arrays', async () => { + await writeFile(sourcePath('config.json'), JSON.stringify({ provider: { a: { x: 1 } }, list: [1, 2] }), 'utf8') + await writeFile(sourcePath('opencode.json'), JSON.stringify({ provider: { a: { y: 2 } }, list: [3] }), 'utf8') + await writeFile(sourcePath('opencode.jsonc'), JSON.stringify({ provider: { a: { z: 3 } } }), 'utf8') + + const file = await readOpenCodeConfigFile() + + expect(file?.content).toEqual({ provider: { a: { x: 1, y: 2, z: 3 } }, list: [3] }) + expect(file?.path).toBe(sourcePath('opencode.jsonc')) + expect(file?.sources?.map((source) => source.name)).toEqual(['config.json', 'opencode.json', 'opencode.jsonc']) + }) + + it('reads and writes an existing jsonc source without creating a duplicate', async () => { + await writeFile(sourcePath('opencode.jsonc'), '{\n // comment\n "theme": "dark"\n}\n', 'utf8') + + const file = await readOpenCodeConfigFile() + expect(file?.path).toBe(sourcePath('opencode.jsonc')) + + await writeOpenCodeConfigFile('{"theme":"light"}') + + await expect(readdir(workDir)).resolves.toEqual(['opencode.jsonc']) + await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).resolves.toBe('{"theme":"light"}') + }) + + it('preserves unknown keys from raw parsed config rather than normalized schema output', async () => { + const raw = JSON.stringify({ theme: 'dark', customTool: { enabled: true }, mystery: 7 }, null, 2) + await writeFile(sourcePath('opencode.json'), raw, 'utf8') + + const file = await readOpenCodeConfigFile() + + expect(file?.content).toEqual({ theme: 'dark', customTool: { enabled: true }, mystery: 7 }) + expect(file?.sources?.[0]?.content).toEqual({ theme: 'dark', customTool: { enabled: true }, mystery: 7 }) + }) + it('reports validation issues for schema-invalid content', async () => { - await writeFile(paths.config, '{"model": 5}', 'utf8') + await writeFile(sourcePath('opencode.json'), '{"model": 5}', 'utf8') const file = await readOpenCodeConfigFile() @@ -102,7 +196,7 @@ describe('opencode-config-file', () => { }) it('reports a root validation issue when the file is not valid JSONC', async () => { - await writeFile(paths.config, '{ not json', 'utf8') + await writeFile(sourcePath('opencode.json'), '{ not json', 'utf8') const file = await readOpenCodeConfigFile() @@ -122,6 +216,307 @@ describe('opencode-config-file', () => { ]) }) + it('modifies only changed paths in the preferred source and preserves comments and untouched fields', async () => { + const lower = '{\n // lower config\n "model": "lower/model",\n "theme": "light"\n}\n' + const target = '{\n // target config\n "theme": "dark",\n "small_model": "small"\n}\n' + await writeFile(sourcePath('config.json'), lower, 'utf8') + await writeFile(sourcePath('opencode.jsonc'), target, 'utf8') + + const merged = (await readOpenCodeConfigFile())?.content ?? {} + const updated = await updateOpenCodeConfigFile({ ...merged, theme: 'light' }) + + expect(updated.content).toEqual({ model: 'lower/model', theme: 'light', small_model: 'small' }) + const targetContent = await readFile(sourcePath('opencode.jsonc'), 'utf8') + expect(targetContent).toContain('// target config') + expect(targetContent).toContain('"theme": "light"') + expect(targetContent).toContain('"small_model": "small"') + await expect(readFile(sourcePath('config.json'), 'utf8')).resolves.toBe(lower) + }) + + it('removes an override from the target source only and reveals inherited values', async () => { + const lower = JSON.stringify({ theme: 'light', model: 'a' }) + await writeFile(sourcePath('config.json'), lower, 'utf8') + await writeFile(sourcePath('opencode.jsonc'), '{\n "theme": "dark"\n}\n', 'utf8') + + await updateOpenCodeConfigFile({ model: 'a' }) + + await expect(readFile(sourcePath('config.json'), 'utf8')).resolves.toBe(lower) + const revealed = await readOpenCodeConfigFile() + expect(revealed?.content).toEqual({ theme: 'light', model: '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' + await writeFile(sourcePath('config.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) + + expect(error).toBeInstanceOf(OpenCodeConfigShadowedRemovalError) + expect((error as OpenCodeConfigShadowedRemovalError).paths).toEqual(['theme']) + expect((error as OpenCodeConfigShadowedRemovalError).sources).toEqual(['config.json']) + await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).resolves.toBe(target) + await expect(readFile(sourcePath('config.json'), 'utf8')).resolves.toBe(lower) + expect((await stat(sourcePath('opencode.jsonc'))).mtimeMs).toBe(targetStats.mtimeMs) + }) + + 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' + await writeFile(sourcePath('config.json'), lower, 'utf8') + await writeFile(sourcePath('opencode.jsonc'), target, 'utf8') + + const error = await updateOpenCodeConfigFile({ model: 'b', plugin: ['x'] }).catch((caught: unknown) => caught) + + expect(error).toBeInstanceOf(OpenCodeConfigShadowedRemovalError) + expect((error as OpenCodeConfigShadowedRemovalError).paths).toEqual(['theme']) + expect((error as OpenCodeConfigShadowedRemovalError).sources).toEqual(['config.json']) + await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).resolves.toBe(target) + await expect(readFile(sourcePath('config.json'), 'utf8')).resolves.toBe(lower) + }) + + it('writes raw content to an explicitly requested allowlisted source and rejects unknown sources', async () => { + const written = await writeOpenCodeConfigFile('{"theme":"dark"}', 'config.json') + + expect(written.path).toBe(sourcePath('config.json')) + await expect(readFile(sourcePath('config.json'), 'utf8')).resolves.toBe('{"theme":"dark"}') + await expect( + writeOpenCodeConfigFile('{"theme":"dark"}', 'nope.json' as 'config.json'), + ).rejects.toThrow(/Unsupported OpenCode config source/) + }) + + it('throws OpenCodeConfigConflictError when the expected revision is stale', async () => { + const initial = await writeOpenCodeConfigFile('{"theme":"dark"}') + + await expect( + updateOpenCodeConfigFile({ theme: 'light' }, { expectedRevision: 'stale' }), + ).rejects.toBeInstanceOf(OpenCodeConfigConflictError) + + await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).resolves.toBe('{"theme":"dark"}') + + const updated = await updateOpenCodeConfigFile({ theme: 'light' }, { expectedRevision: initial.revision }) + expect(updated.content).toEqual({ theme: 'light' }) + }) + + it('round-trips every present and absent source state through a snapshot', async () => { + const config = '{\n // config source\n "theme": "light"\n}\n' + const json = '{"model":"a/b"}' + const jsonc = '{\n // jsonc source\n "small_model": "s"\n}\n' + await writeFile(sourcePath('config.json'), config, 'utf8') + await writeFile(sourcePath('opencode.json'), json, 'utf8') + await writeFile(sourcePath('opencode.jsonc'), jsonc, 'utf8') + + const snapshot = serializeOpenCodeConfigSnapshot((await readOpenCodeConfigFile())!) + + await deleteOpenCodeConfigFile() + await expect(readOpenCodeConfigFile()).resolves.toBeNull() + + await restoreOpenCodeConfigSnapshot(snapshot) + + await expect(readFile(sourcePath('config.json'), 'utf8')).resolves.toBe(config) + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe(json) + await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).resolves.toBe(jsonc) + }) + + it('restores a legacy plain raw snapshot as an opencode.json-only source', async () => { + await writeFile(sourcePath('opencode.jsonc'), '{"theme":"dark"}', 'utf8') + + const legacy = '{\n // legacy\n "model": "a/b"\n}\n' + await restoreOpenCodeConfigSnapshot(legacy) + + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe(legacy) + await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('rolls back already applied sources when restoring a snapshot fails', async () => { + await writeFile(sourcePath('opencode.json'), '{"theme":"original"}', 'utf8') + const snapshot = serializeOpenCodeConfigSnapshot({ + sources: [{ + name: 'opencode.jsonc', + path: sourcePath('opencode.jsonc'), + rawContent: '{"theme":"restored"}', + content: { theme: 'restored' }, + isValid: true, + updatedAt: 0, + }], + }) + + writeFailures.paths.push(sourcePath('opencode.jsonc')) + + await expect(restoreOpenCodeConfigSnapshot(snapshot)).rejects.toThrow(/simulated write failure/) + + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe('{"theme":"original"}') + await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('surfaces an AggregateError when rollback also fails', async () => { + await writeFile(sourcePath('opencode.json'), '{"theme":"original"}', 'utf8') + const snapshot = serializeOpenCodeConfigSnapshot({ + sources: [{ + name: 'opencode.jsonc', + path: sourcePath('opencode.jsonc'), + rawContent: '{"theme":"restored"}', + content: { theme: 'restored' }, + isValid: true, + updatedAt: 0, + }], + }) + + writeFailures.paths.push(sourcePath('opencode.jsonc'), sourcePath('opencode.json')) + + await expect(restoreOpenCodeConfigSnapshot(snapshot)).rejects.toBeInstanceOf(AggregateError) + }) + + it('accepts a version 1 snapshot without the explicit marker', async () => { + await writeFile(sourcePath('opencode.json'), '{"model":"a/b"}', 'utf8') + const snapshot = JSON.stringify({ + version: 1, + sources: [{ name: 'opencode.jsonc', rawContent: '{"theme":"light"}' }], + }) + + await restoreOpenCodeConfigSnapshot(snapshot) + + await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).resolves.toBe('{"theme":"light"}') + await expect(readFile(sourcePath('opencode.json'), 'utf8')).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('rejects an unsupported snapshot version without changing sources', async () => { + await writeFile(sourcePath('opencode.json'), '{"model":"a/b"}', 'utf8') + const snapshot = JSON.stringify({ + version: 2, + sources: [{ name: 'config.json', rawContent: '{"theme":"dark"}' }], + }) + + await expect(restoreOpenCodeConfigSnapshot(snapshot)).rejects.toBeInstanceOf(OpenCodeConfigSnapshotError) + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe('{"model":"a/b"}') + await expect(readFile(sourcePath('config.json'), 'utf8')).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('rejects a snapshot with a malformed sources shape without changing sources', async () => { + await writeFile(sourcePath('opencode.json'), '{"model":"a/b"}', 'utf8') + const snapshot = JSON.stringify({ version: 1, sources: 'not-an-array' }) + + await expect(restoreOpenCodeConfigSnapshot(snapshot)).rejects.toBeInstanceOf(OpenCodeConfigSnapshotError) + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe('{"model":"a/b"}') + }) + + it('rejects a snapshot with an unsupported marker without changing sources', async () => { + await writeFile(sourcePath('opencode.json'), '{"model":"a/b"}', 'utf8') + const snapshot = JSON.stringify({ + marker: 'something-else', + version: 1, + sources: [{ name: 'config.json', rawContent: '{"theme":"dark"}' }], + }) + + await expect(restoreOpenCodeConfigSnapshot(snapshot)).rejects.toBeInstanceOf(OpenCodeConfigSnapshotError) + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe('{"model":"a/b"}') + }) + + it('rejects a malformed legacy raw snapshot without changing sources', async () => { + await writeFile(sourcePath('opencode.json'), '{"model":"a/b"}', 'utf8') + + await expect(restoreOpenCodeConfigSnapshot('{ not jsonc')).rejects.toBeInstanceOf(OpenCodeConfigSnapshotError) + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe('{"model":"a/b"}') + }) + + it('validates all snapshot source contents before replacing any file', async () => { + const rawContent = '{"model":"existing"}' + await writeFile(sourcePath('opencode.json'), rawContent) + const snapshot = JSON.stringify({ version: 1, sources: [ + { name: 'config.json', rawContent: '{"theme":"dark"}' }, + { name: 'opencode.jsonc', rawContent: '{"model":123}' }, + ] }) + + await expect(restoreOpenCodeConfigSnapshot(snapshot)).rejects.toBeInstanceOf(OpenCodeConfigSnapshotError) + expect(await readFile(sourcePath('opencode.json'), 'utf8')).toBe(rawContent) + await expect(readFile(sourcePath('config.json'), 'utf8')).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('rejects a schema-invalid legacy raw snapshot without changing sources', async () => { + await writeFile(sourcePath('opencode.json'), '{"model":"a/b"}', 'utf8') + + await expect(restoreOpenCodeConfigSnapshot('{"model":5}')).rejects.toBeInstanceOf(OpenCodeConfigSnapshotError) + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe('{"model":"a/b"}') + }) + + it('leaves sources untouched when an existing source cannot be read before restore', async () => { + await writeFile(sourcePath('opencode.json'), '{"model":"a/b"}', 'utf8') + await chmod(sourcePath('opencode.json'), 0o000) + const snapshot = JSON.stringify({ + version: 1, + sources: [{ name: 'config.json', rawContent: '{"theme":"dark"}' }], + }) + + try { + await expect(restoreOpenCodeConfigSnapshot(snapshot)).rejects.toThrow() + } finally { + await chmod(sourcePath('opencode.json'), 0o644) + } + + await expect(readFile(sourcePath('opencode.json'), 'utf8')).resolves.toBe('{"model":"a/b"}') + await expect(readFile(sourcePath('config.json'), 'utf8')).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('skips byte-identical structured and raw writes and preserves comments', async () => { + const raw = '{\n // keep\n "theme": "dark"\n}\n' + await writeFile(sourcePath('opencode.jsonc'), raw, 'utf8') + const merged = (await readOpenCodeConfigFile())?.content ?? {} + + writeFailures.calls = [] + await updateOpenCodeConfigFile(merged) + await updateOpenCodeConfigFile(raw) + + expect(writeFailures.calls).toEqual([]) + await expect(readFile(sourcePath('opencode.jsonc'), 'utf8')).resolves.toBe(raw) + }) + + it('uses a provided snapshot without re-reading sources before the write', async () => { + await writeFile(sourcePath('opencode.jsonc'), '{"theme":"dark"}', 'utf8') + const snapshot = await readOpenCodeConfigSnapshot() + + fsCalls.stat = 0 + fsCalls.readFile = 0 + await updateOpenCodeConfigFile({ theme: 'light' }, { snapshot }) + + expect(writeFailures.readsAtWrite).toEqual([{ stat: 0, readFile: 0 }]) + expect(fsCalls.stat).toBe(3) + expect(fsCalls.readFile).toBe(1) + }) + + it('replaces arrays atomically and merges nested objects in structured updates', async () => { + await writeFile( + sourcePath('opencode.jsonc'), + JSON.stringify({ plugin: ['a'], provider: { p: { models: { m: { name: 'one' } } } } }), + 'utf8', + ) + + const updated = await updateOpenCodeConfigFile({ + plugin: ['b'], + provider: { p: { models: { m: { name: 'two' }, n: { name: 'three' } } } }, + }) + + expect(updated.content).toEqual({ + plugin: ['b'], + provider: { p: { models: { m: { name: 'two' }, n: { name: 'three' } } } }, + }) + }) + + it('treats prototype-named config keys as plain data without polluting prototypes', async () => { + await writeFile(sourcePath('config.json'), '{"constructor":{"a":1}}', 'utf8') + await writeFile(sourcePath('opencode.json'), '{"constructor":{"b":2}}', 'utf8') + + const file = await readOpenCodeConfigFile() + expect(file?.content).toEqual({ constructor: { a: 1, b: 2 } }) + + await updateOpenCodeConfigFile({ constructor: { a: 1, b: 2 }, prototype: { x: 1 } }) + + const merged = await readOpenCodeConfigFile() + expect(merged?.content).toEqual({ constructor: { a: 1, b: 2 }, prototype: { x: 1 } }) + expect(({} as Record).x).toBeUndefined() + }) + it('writes a health-watch artifact with a shared timestamp and returns its path', async () => { const artifactPath = await writeHealthWatchArtifact('opencode-health', (timestamp) => JSON.stringify({ capturedAt: timestamp })) @@ -131,17 +526,26 @@ describe('opencode-config-file', () => { expect(content.capturedAt).toBe(path.basename(artifactPath).replace(/^opencode-health-/, '').replace(/\.json$/, '')) }) - it('archives the config file under the health-watch directory', async () => { + it('archives the full source set under the health-watch directory', async () => { await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED) + await writeFile(sourcePath('opencode.json'), '{"model":"a/b"}', 'utf8') const archivePath = await archiveBrokenOpenCodeConfigFile() expect(archivePath).toBeTruthy() expect(path.dirname(archivePath as string)).toBe(paths.healthWatch) - await expect(readFile(archivePath as string, 'utf8')).resolves.toBe(OPENCODE_CONFIG_SEED) + const archived = JSON.parse(await readFile(archivePath as string, 'utf8')) as { + version: number + sources: Array<{ name: string; rawContent: string }> + } + expect(archived.version).toBe(1) + expect(archived.sources).toEqual(expect.arrayContaining([ + { name: 'opencode.jsonc', rawContent: OPENCODE_CONFIG_SEED }, + { name: 'opencode.json', rawContent: '{"model":"a/b"}' }, + ])) }) - it('returns null when archiving with no config file present', async () => { + it('returns null when archiving with no config source present', async () => { await expect(archiveBrokenOpenCodeConfigFile()).resolves.toBeNull() }) @@ -182,10 +586,12 @@ describe('opencode-config-file', () => { expect(remaining).toContain(path.basename(archivePath as string)) }) - it('deletes the config file and reports whether it existed', async () => { + it('deletes the entire source set and reports whether any source existed', async () => { await writeOpenCodeConfigFile(OPENCODE_CONFIG_SEED) + await writeFile(sourcePath('config.json'), '{"model":"a/b"}', 'utf8') await expect(deleteOpenCodeConfigFile()).resolves.toBe(true) + await expect(readdir(workDir)).resolves.toEqual([]) await expect(deleteOpenCodeConfigFile()).resolves.toBe(false) }) }) diff --git a/backend/test/services/opencode-import.test.ts b/backend/test/services/opencode-import.test.ts index 60542310..7b5dd40a 100644 --- a/backend/test/services/opencode-import.test.ts +++ b/backend/test/services/opencode-import.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' vi.mock('node:fs', async (importOriginal) => ({ ...(await importOriginal()), @@ -29,7 +29,7 @@ vi.mock('../../src/services/file-operations', () => ({ vi.mock('../../src/services/opencode-config-file', async (importOriginal) => ({ ...(await importOriginal()), readOpenCodeConfigFile: vi.fn(), - writeOpenCodeConfigFile: vi.fn(), + restoreOpenCodeConfigSnapshot: vi.fn(), })) vi.mock('../../src/services/opencode-single-server', () => ({ @@ -38,23 +38,22 @@ vi.mock('../../src/services/opencode-single-server', () => ({ }, })) -vi.mock('../../src/services/opencode/config-recovery', () => ({ - patchConfigWithRecovery: vi.fn(), -})) - -vi.mock('@opencode-manager/shared/config/env', () => ({ +vi.mock('@opencode-manager/shared/config/env', async (importOriginal) => ({ + ...(await importOriginal()), + getConfigPath: vi.fn(() => '/tmp/workspace/.config/opencode'), getOpenCodeConfigFilePath: vi.fn(() => '/tmp/workspace/.config/opencode/opencode.json'), getWorkspacePath: vi.fn(() => '/tmp/workspace'), })) import path from 'path' +import os from 'os' import { existsSync } from 'node:fs' import { readdir, rm, cp, mkdtemp, rename } from 'fs/promises' import { Database as SQLiteDatabase } from 'bun:sqlite' import { ensureDirectoryExists, fileExists, readFileContent } from '../../src/services/file-operations' -import { readOpenCodeConfigFile, writeOpenCodeConfigFile } from '../../src/services/opencode-config-file' +import { readOpenCodeConfigFile, restoreOpenCodeConfigSnapshot, serializeOpenCodeConfigSnapshot } from '../../src/services/opencode-config-file' import type { SettingsService } from '../../src/services/settings' -import { getFirstExistingConfigSourcePath, getOpenCodeImportStatus, syncOpenCodeImport } from '../../src/services/opencode-import' +import { getOpenCodeImportStatus, syncOpenCodeImport } from '../../src/services/opencode-import' const mockReaddir = readdir as unknown as ReturnType const mockExistsSync = existsSync as ReturnType @@ -62,7 +61,7 @@ const mockFileExists = fileExists as ReturnType const mockReadFileContent = readFileContent as ReturnType const mockEnsureDirectoryExists = ensureDirectoryExists as ReturnType const mockReadOpenCodeConfigFile = readOpenCodeConfigFile as ReturnType -const mockWriteOpenCodeConfigFile = writeOpenCodeConfigFile as ReturnType +const mockWriteOpenCodeConfigFile = restoreOpenCodeConfigSnapshot as ReturnType const MockSQLiteDatabase = SQLiteDatabase as unknown as ReturnType const mockMkdtemp = mkdtemp as unknown as ReturnType const mockRename = rename as unknown as ReturnType @@ -70,6 +69,9 @@ const mockRename = rename as unknown as ReturnType describe('opencode-import service', () => { beforeEach(() => { vi.clearAllMocks() + vi.stubEnv('OPENCODE_IMPORT_CONFIG_PATH', undefined) + vi.stubEnv('OPENCODE_IMPORT_STATE_PATH', undefined) + mockReadOpenCodeConfigFile.mockResolvedValue(null) mockReadFileContent.mockResolvedValue('{"$schema":"https://opencode.ai/config.json"}') mockReaddir.mockResolvedValue([]) mockMkdtemp.mockResolvedValue('/tmp/workspace/.opencode/state/opencode-import-123') @@ -77,6 +79,37 @@ describe('opencode-import service', () => { mockExistsSync.mockImplementation((candidate: string) => candidate === process.env.OPENCODE_IMPORT_CONFIG_PATH) }) + afterEach(() => vi.unstubAllEnvs()) + + it('imports every discovered global source without flattening or renaming JSONC', async () => { + const jsonPath = path.join(os.homedir(), '.config', 'opencode', 'opencode.json') + 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}' + mockReadFileContent.mockImplementation(async (candidate: string) => candidate === jsoncPath ? rawJsonc : rawJson) + + const result = await syncOpenCodeImport({}) + + expect(result.configSourcePaths).toEqual([jsonPath, jsoncPath]) + expect(result.configSourcePath).toBe(jsoncPath) + expect(mockWriteOpenCodeConfigFile).toHaveBeenCalledTimes(1) + const snapshot = mockWriteOpenCodeConfigFile.mock.calls[0]![0] as string + expect(snapshot).toContain(JSON.stringify(rawJson)) + expect(snapshot).toContain(JSON.stringify(rawJsonc)) + expect(snapshot).toContain('opencode.jsonc') + }) + + it('preserves an explicitly imported JSONC filename', async () => { + process.env.OPENCODE_IMPORT_CONFIG_PATH = '/import/opencode.jsonc' + mockFileExists.mockResolvedValue(false) + + await syncOpenCodeImport({}) + + expect(mockWriteOpenCodeConfigFile).toHaveBeenCalledWith(expect.stringContaining('opencode.jsonc')) + }) + it('detects importable host config and state paths with opencode.db', async () => { process.env.OPENCODE_IMPORT_CONFIG_PATH = '/import/opencode-config/opencode.json' process.env.OPENCODE_IMPORT_STATE_PATH = '/import/opencode-state' @@ -101,13 +134,47 @@ describe('opencode-import service', () => { expect(status).toEqual({ configSourcePath: '/import/opencode-config/opencode.json', + configSourcePaths: ['/import/opencode-config/opencode.json'], stateSourcePath: '/import/opencode-state', - workspaceConfigPath: '/tmp/workspace/.config/opencode/opencode.json', + workspaceConfigPath: '/tmp/workspace/.config/opencode/opencode.jsonc', + workspaceConfigPathsToRemove: [], workspaceStatePath: '/tmp/workspace/.opencode/state/opencode', workspaceStateExists: true, }) }) + it('reports workspace config sources absent from the host as pending removals', async () => { + process.env.OPENCODE_IMPORT_CONFIG_PATH = '/import/opencode-config/opencode.json' + const workspaceJsoncPath = '/tmp/workspace/.config/opencode/opencode.jsonc' + mockExistsSync.mockImplementation((candidate: string) => + candidate === '/import/opencode-config/opencode.json' || candidate === workspaceJsoncPath) + + const status = await getOpenCodeImportStatus() + + expect(status.workspaceConfigPathsToRemove).toEqual([workspaceJsoncPath]) + }) + + it('reports no pending removals when workspace and host share a config basename', async () => { + process.env.OPENCODE_IMPORT_CONFIG_PATH = '/import/opencode-config/opencode.json' + const workspaceJsonPath = '/tmp/workspace/.config/opencode/opencode.json' + mockExistsSync.mockImplementation((candidate: string) => + candidate === '/import/opencode-config/opencode.json' || candidate === workspaceJsonPath) + + const status = await getOpenCodeImportStatus() + + expect(status.workspaceConfigPathsToRemove).toEqual([]) + }) + + it('reports no pending removals when no host config source is detected', async () => { + const workspaceJsoncPath = '/tmp/workspace/.config/opencode/opencode.jsonc' + mockExistsSync.mockImplementation((candidate: string) => candidate === workspaceJsoncPath) + + const status = await getOpenCodeImportStatus() + + expect(status.configSourcePaths).toEqual([]) + expect(status.workspaceConfigPathsToRemove).toEqual([]) + }) + it('imports host config and state into the workspace', async () => { process.env.OPENCODE_IMPORT_CONFIG_PATH = '/import/opencode-config/opencode.json' process.env.OPENCODE_IMPORT_STATE_PATH = '/import/opencode-state' @@ -127,7 +194,7 @@ describe('opencode-import service', () => { expect(result.stateImported).toBe(true) expect(result.workspaceStateExists).toBe(true) expect(mockWriteOpenCodeConfigFile).toHaveBeenCalledWith( - '{"$schema":"https://opencode.ai/config.json"}' + expect.stringContaining(JSON.stringify('{"$schema":"https://opencode.ai/config.json"}')) ) expect(mockEnsureDirectoryExists).toHaveBeenCalledWith('/tmp/workspace/.opencode/state') expect(MockSQLiteDatabase).toHaveBeenCalledWith('/import/opencode-state/opencode.db') @@ -141,16 +208,29 @@ describe('opencode-import service', () => { process.env.OPENCODE_IMPORT_CONFIG_PATH = '/import/opencode-config/opencode.json' mockFileExists.mockImplementation(async (candidate: string) => candidate === '/import/opencode-config/opencode.json') - mockReadOpenCodeConfigFile.mockResolvedValue({ + const previous = { + path: '/tmp/workspace/.config/opencode/opencode.json', + content: { theme: 'previous' }, isValid: true, rawContent: '{"theme":"previous"}', - }) + updatedAt: 0, + revision: 'rev-previous', + sources: [{ + name: 'opencode.json' as const, + path: '/tmp/workspace/.config/opencode/opencode.json', + rawContent: '{"theme":"previous"}', + content: { theme: 'previous' }, + isValid: true, + updatedAt: 0, + }], + } + mockReadOpenCodeConfigFile.mockResolvedValue(previous) const settingsService = { saveLastKnownGoodConfig: vi.fn() } as unknown as SettingsService await syncOpenCodeImport({ overwriteState: true, settingsService }) - expect(settingsService.saveLastKnownGoodConfig).toHaveBeenCalledWith('{"theme":"previous"}') - expect(mockWriteOpenCodeConfigFile).toHaveBeenCalledWith('{"$schema":"https://opencode.ai/config.json"}') + expect(settingsService.saveLastKnownGoodConfig).toHaveBeenCalledWith(serializeOpenCodeConfigSnapshot(previous)) + expect(mockWriteOpenCodeConfigFile).toHaveBeenCalledWith(expect.stringContaining(JSON.stringify('{"$schema":"https://opencode.ai/config.json"}'))) }) it('does not capture last known good when no previous config file exists', async () => { @@ -184,10 +264,12 @@ describe('opencode-import service', () => { expect(mockEnsureDirectoryExists).not.toHaveBeenCalled() }) - it('resolves the first existing import config candidate synchronously', () => { + it('reports the resolved import config candidate as the config source path', async () => { process.env.OPENCODE_IMPORT_CONFIG_PATH = process.execPath - expect(getFirstExistingConfigSourcePath()).toBe(process.execPath) + const status = await getOpenCodeImportStatus() + + expect(status.configSourcePath).toBe(process.execPath) }) it('rejects invalid importable config content with the existing error', async () => { diff --git a/backend/test/services/opencode-restart.test.ts b/backend/test/services/opencode-restart.test.ts index 31b48a84..313356d8 100644 --- a/backend/test/services/opencode-restart.test.ts +++ b/backend/test/services/opencode-restart.test.ts @@ -4,13 +4,30 @@ const managerMock = vi.hoisted(() => ({ getLastStartupError: vi.fn<() => string | null>(() => null), clearStartupError: vi.fn<() => void>(), restart: vi.fn<() => Promise>(), + checkHealth: vi.fn<() => Promise>(), +})) + +const configFileMock = vi.hoisted(() => ({ + readOpenCodeConfigFile: vi.fn(), })) vi.mock('../../src/services/opencode-single-server', () => ({ opencodeServerManager: managerMock, + ConfigReloadError: class ConfigReloadError extends Error { + validationIssues: Array<{ path: string; message: string }> + + constructor(message: string, validationIssues: Array<{ path: string; message: string }> = []) { + super(message) + this.name = 'ConfigReloadError' + this.validationIssues = validationIssues + } + }, })) +vi.mock('../../src/services/opencode-config-file', () => configFileMock) + import { + reloadOpenCodeConfig, restartOpenCode, setOpenCodeRestartCoordinator, } from '../../src/services/opencode-restart' @@ -20,7 +37,6 @@ import type { OpenCodeSupervisor } from '../../src/services/opencode-supervisor' function createSupervisor(healthy: boolean): OpenCodeSupervisor { return { restart: vi.fn().mockResolvedValue({ healthy }), - reloadConfig: vi.fn(), } as unknown as OpenCodeSupervisor } @@ -33,6 +49,15 @@ function createCoordinator(healthy: boolean, resumedSessionIDs: string[] = []): } as unknown as OpenCodeRestartCoordinator } +function createInvokingCoordinator(resumedSessionIDs: string[] = []): OpenCodeRestartCoordinator { + return { + runWithResume: vi.fn(async (restart: () => Promise) => ({ + healthy: await restart(), + resumedSessionIDs, + })), + } as unknown as OpenCodeRestartCoordinator +} + describe('restartOpenCode', () => { beforeEach(() => { vi.clearAllMocks() @@ -90,3 +115,85 @@ describe('restartOpenCode', () => { expect(managerMock.clearStartupError).toHaveBeenCalled() }) }) + +describe('reloadOpenCodeConfig', () => { + beforeEach(() => { + vi.clearAllMocks() + managerMock.getLastStartupError.mockReset().mockReturnValue(null) + managerMock.clearStartupError.mockReset() + managerMock.restart.mockReset() + managerMock.checkHealth.mockReset() + configFileMock.readOpenCodeConfigFile.mockReset() + setOpenCodeRestartCoordinator(null) + }) + + afterEach(() => { + setOpenCodeRestartCoordinator(null) + }) + + it('throws a ConfigReloadError without restarting when every global source is absent', async () => { + configFileMock.readOpenCodeConfigFile.mockResolvedValue(null) + const supervisor = createSupervisor(true) + + await expect(reloadOpenCodeConfig(supervisor)).rejects.toMatchObject({ + name: 'ConfigReloadError', + message: 'No OpenCode global configuration files found', + }) + expect(supervisor.restart).not.toHaveBeenCalled() + expect(managerMock.restart).not.toHaveBeenCalled() + }) + + it('throws a ConfigReloadError with validation issues without restarting when the config is invalid', async () => { + const validationIssues = [{ path: 'model', message: 'Invalid model' }] + configFileMock.readOpenCodeConfigFile.mockResolvedValue({ isValid: false, validationIssues }) + const supervisor = createSupervisor(true) + + await expect(reloadOpenCodeConfig(supervisor)).rejects.toMatchObject({ + name: 'ConfigReloadError', + message: 'OpenCode global configuration is invalid', + validationIssues, + }) + expect(supervisor.restart).not.toHaveBeenCalled() + expect(managerMock.restart).not.toHaveBeenCalled() + }) + + it('delegates a valid config to the supervisor restart path with the settings_reload reason', async () => { + configFileMock.readOpenCodeConfigFile.mockResolvedValue({ isValid: true }) + const supervisor = createSupervisor(true) + + const result = await reloadOpenCodeConfig(supervisor) + + expect(supervisor.restart).toHaveBeenCalledWith('settings_reload') + expect(result).toEqual({ resumedSessionIDs: [] }) + }) + + it('returns the resumed session IDs produced by the coordinator on a valid reload', async () => { + configFileMock.readOpenCodeConfigFile.mockResolvedValue({ isValid: true }) + const supervisor = createSupervisor(true) + setOpenCodeRestartCoordinator(createInvokingCoordinator(['session-1', 'session-2'])) + + const result = await reloadOpenCodeConfig(supervisor) + + expect(supervisor.restart).toHaveBeenCalledWith('settings_reload') + expect(result).toEqual({ resumedSessionIDs: ['session-1', 'session-2'] }) + }) + + it('restarts the manager directly without a coordinator', async () => { + configFileMock.readOpenCodeConfigFile.mockResolvedValue({ isValid: true }) + managerMock.checkHealth.mockResolvedValue(true) + + await reloadOpenCodeConfig() + + expect(managerMock.clearStartupError).toHaveBeenCalled() + expect(managerMock.restart).toHaveBeenCalledTimes(1) + expect(managerMock.checkHealth).toHaveBeenCalled() + }) + + it('throws when the manager restart leaves the server unhealthy without a coordinator', async () => { + configFileMock.readOpenCodeConfigFile.mockResolvedValue({ isValid: true }) + managerMock.checkHealth.mockResolvedValue(false) + managerMock.getLastStartupError.mockReturnValue('reload did not restore health') + + await expect(reloadOpenCodeConfig()).rejects.toThrow('reload did not restore health') + }) +}) diff --git a/backend/test/services/opencode-single-server.test.ts b/backend/test/services/opencode-single-server.test.ts index ac35b3ec..f122f659 100644 --- a/backend/test/services/opencode-single-server.test.ts +++ b/backend/test/services/opencode-single-server.test.ts @@ -23,36 +23,39 @@ vi.mock('bun:sqlite', () => ({ Database: vi.fn(), })) -vi.mock('@opencode-manager/shared/config/env', () => ({ - getWorkspacePath: vi.fn(() => '/test/workspace'), - getOpenCodeConfigFilePath: vi.fn(() => '/test/workspace/.config/opencode.json'), - getOpenCodeHealthWatchPath: vi.fn(() => '/test/workspace/health-watch'), - getOpenCodeStateHome: vi.fn(() => '/test/workspace/.opencode/state'), - getOpenCodeConfigHome: vi.fn(() => '/test/workspace/.config'), - getOpenCodeTmpHome: vi.fn(() => '/test/workspace/.opencode/tmp'), - getOpenCodeAgentTmpPath: vi.fn(() => '/test/workspace/.opencode/tmp/opencode'), - getReposPath: vi.fn(() => '/test/workspace/repos'), - getAgentsMdPath: vi.fn(() => '/test/workspace/AGENTS.md'), - getDatabasePath: vi.fn(() => ':memory:'), - getConfigPath: vi.fn(() => '/test/workspace/config'), - ENV: { - SERVER: { PORT: 5003, HOST: '0.0.0.0', NODE_ENV: 'test' }, - AUTH: { TRUSTED_ORIGINS: 'http://localhost:5173', SECRET: 'test-secret-for-encryption-key-32c' }, - WORKSPACE: { BASE_PATH: '/test/workspace', REPOS_DIR: 'repos', CONFIG_DIR: 'config', AUTH_FILE: 'auth.json' }, - OPENCODE: { PORT: 5551, HOST: '127.0.0.1', SERVER_PASSWORD: '', SERVER_USERNAME: 'opencode', PUBLIC_URL: '' }, - TIMEOUTS: { HEALTH_CHECK_TIMEOUT_MS: 50 }, - DATABASE: { PATH: ':memory:' }, - SANDBOX: { MSB_PATH: 'msb' }, - FILE_LIMITS: { - MAX_SIZE_BYTES: 1024 * 1024, - MAX_UPLOAD_SIZE_BYTES: 10 * 1024 * 1024, +vi.mock('@opencode-manager/shared/config/env', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + getWorkspacePath: vi.fn(() => '/test/workspace'), + getOpenCodeConfigFilePath: vi.fn(() => '/test/workspace/.config/opencode.json'), + getOpenCodeHealthWatchPath: vi.fn(() => '/test/workspace/health-watch'), + getOpenCodeStateHome: vi.fn(() => '/test/workspace/.opencode/state'), + getOpenCodeConfigHome: vi.fn(() => '/test/workspace/.config'), + getOpenCodeTmpHome: vi.fn(() => '/test/workspace/.opencode/tmp'), + getOpenCodeAgentTmpPath: vi.fn(() => '/test/workspace/.opencode/tmp/opencode'), + getReposPath: vi.fn(() => '/test/workspace/repos'), + getAgentsMdPath: vi.fn(() => '/test/workspace/AGENTS.md'), + getDatabasePath: vi.fn(() => ':memory:'), + getConfigPath: vi.fn(() => '/test/workspace/config'), + ENV: { + ...actual.ENV, + SERVER: { ...actual.ENV.SERVER, NODE_ENV: 'test' }, + WORKSPACE: { ...actual.ENV.WORKSPACE, BASE_PATH: '/test/workspace' }, + OPENCODE: { + ...actual.ENV.OPENCODE, + PORT: 5551, + HOST: '127.0.0.1', + SERVER_PASSWORD: '', + SERVER_USERNAME: 'opencode', + PUBLIC_URL: '', + }, + SANDBOX: { ...actual.ENV.SANDBOX, MSB_PATH: 'msb' }, + TIMEOUTS: { ...actual.ENV.TIMEOUTS, HEALTH_CHECK_TIMEOUT_MS: 50 }, + DATABASE: { PATH: ':memory:' }, }, - }, - FILE_LIMITS: { - MAX_SIZE_BYTES: 1024 * 1024, - MAX_UPLOAD_SIZE_BYTES: 10 * 1024 * 1024, - }, -})) + } +}) vi.mock('fs', () => ({ accessSync: vi.fn(() => { @@ -83,10 +86,6 @@ vi.mock('child_process', () => ({ spawnSync: spawnSyncMock, })) -vi.mock('../../src/services/opencode/config-recovery', () => ({ - patchConfigWithRecovery: vi.fn(), -})) - const writeOpenCodeConfigFileMock = vi.hoisted(() => vi.fn()) const readOpenCodeConfigFileMock = vi.hoisted(() => vi.fn()) @@ -138,7 +137,6 @@ import { promises as fs, accessSync, readdirSync } from 'fs' import { execSync, spawnSync } from 'child_process' import path from 'path' import os from 'os' -import { ZodError } from 'zod' import { ConfigReloadError, resolveOpenCodeExecutable } from '../../src/services/opencode-single-server' import { forceProcessAttestation, resetProcessIdentityProvider } from '../../src/services/opencode/process-identity' import { encryptSecret } from '../../src/utils/crypto' @@ -654,6 +652,24 @@ describe('OpenCodeServerManager - server auth', () => { } }) + it('drops inherited and user-supplied OPENCODE_CONFIG from the final child env while keeping XDG_CONFIG_HOME in the workspace', async () => { + const { OpenCodeServerManager } = await import('../../src/services/opencode-single-server') + const manager = OpenCodeServerManager.getInstance() + manager.setDatabase(createPreferencesDb({ + serverEnvVars: [{ key: 'OPENCODE_CONFIG', value: '/tmp/evil-user-config.json' }], + })) + process.env.OPENCODE_CONFIG = '/tmp/evil-inherited-config.json' + try { + await manager.start() + + const env = (spawnMock.mock.calls[0] as unknown as [unknown, unknown, { env: Record }])[2].env + expect(env.OPENCODE_CONFIG).toBeUndefined() + expect(env.XDG_CONFIG_HOME).toBe('/test/workspace/.config') + } finally { + delete process.env.OPENCODE_CONFIG + } + }) + it('honors a user-supplied HOME serverEnvVars entry while enforced', async () => { sandboxRuntimeServiceMock.SandboxRuntimeService.mockImplementation(() => ({ isEnabled: () => true, @@ -1314,7 +1330,6 @@ describe('OpenCodeServerManager - server auth', () => { ;(manager as unknown as { opInProgress: boolean }).opInProgress = true await expect(manager.restart()).rejects.toThrow('Another OpenCode server operation is already in progress') - await expect(manager.reloadConfig()).rejects.toThrow('Another OpenCode server operation is already in progress') await expect(manager.start()).rejects.toThrow('Another OpenCode server operation is already in progress') }) @@ -3042,197 +3057,20 @@ describe('OpenCodeServerManager - reinitializeBinDirectory', () => { }) describe('ConfigReloadError', () => { - it('should create error with validation issues and removed fields', () => { + it('should create error with validation issues', () => { const issues = [{ path: 'command.review', message: 'Invalid' }] - const removed = ['command.review'] - const error = new ConfigReloadError('Test error', issues, removed) + const error = new ConfigReloadError('Test error', issues) expect(error.name).toBe('ConfigReloadError') expect(error.message).toBe('Test error') expect(error.validationIssues).toEqual(issues) - expect(error.removedFields).toEqual(removed) }) - it('should default to empty arrays for issues and removed fields', () => { + it('should default to an empty array for validation issues', () => { const error = new ConfigReloadError('Test error') expect(error.validationIssues).toEqual([]) - expect(error.removedFields).toEqual([]) - }) -}) - -describe('OpenCodeServerManager - reloadConfig', () => { - const configFile = (content: Record) => ({ - path: '/test/workspace/.config/opencode.json', - rawContent: JSON.stringify(content), - content, - isValid: true, - updatedAt: 0, - }) - - beforeEach(() => { - vi.clearAllMocks() - writeOpenCodeConfigFileMock.mockReset() - readOpenCodeConfigFileMock.mockReset() - }) - - it('should read config from file before patching', async () => { - readOpenCodeConfigFileMock.mockResolvedValue(configFile({ command: { review: 'test' } })) - - const { patchConfigWithRecovery } = await import('../../src/services/opencode/config-recovery') - const mockPatchResult = { success: true } - vi.mocked(patchConfigWithRecovery).mockResolvedValue(mockPatchResult as any) - - const { opencodeServerManager } = await import('../../src/services/opencode-single-server') - const { createStubOpenCodeClient } = await import('../helpers/stub-opencode-client') - opencodeServerManager.setOpenCodeClient(createStubOpenCodeClient()) - - await opencodeServerManager.reloadConfig() - - expect(readOpenCodeConfigFileMock).toHaveBeenCalled() - expect(patchConfigWithRecovery).toHaveBeenCalled() - }) - - it('passes a config with plugins through to the live reload patch unchanged', async () => { - const { opencodeServerManager } = await import('../../src/services/opencode-single-server') - const { patchConfigWithRecovery } = await import('../../src/services/opencode/config-recovery') - vi.mocked(patchConfigWithRecovery).mockResolvedValue({ success: true } as any) - const { createStubOpenCodeClient } = await import('../helpers/stub-opencode-client') - opencodeServerManager.setOpenCodeClient(createStubOpenCodeClient()) - readOpenCodeConfigFileMock.mockResolvedValue(configFile({ plugin: ['evil-plugin'], model: 'x' })) - - await opencodeServerManager.reloadConfig() - - const patchTarget = vi.mocked(patchConfigWithRecovery).mock.calls[0]![1] - expect(patchTarget).toEqual({ plugin: ['evil-plugin'], model: 'x' }) - }) - - it('passes a plugin-free config through to the live reload patch unchanged', async () => { - const { opencodeServerManager } = await import('../../src/services/opencode-single-server') - const { patchConfigWithRecovery } = await import('../../src/services/opencode/config-recovery') - vi.mocked(patchConfigWithRecovery).mockResolvedValue({ success: true } as any) - const { createStubOpenCodeClient } = await import('../helpers/stub-opencode-client') - opencodeServerManager.setOpenCodeClient(createStubOpenCodeClient()) - readOpenCodeConfigFileMock.mockResolvedValue(configFile({ model: 'x' })) - - await opencodeServerManager.reloadConfig() - - const patchTarget = vi.mocked(patchConfigWithRecovery).mock.calls[0]![1] - expect(patchTarget).toEqual({ model: 'x' }) - }) - - it('persists the cleaned config through the config-file owner when fields are removed', async () => { - const { opencodeServerManager } = await import('../../src/services/opencode-single-server') - const { patchConfigWithRecovery } = await import('../../src/services/opencode/config-recovery') - const cleanedConfig = { model: 'x' } - vi.mocked(patchConfigWithRecovery).mockResolvedValue({ - success: true, - removedFields: ['mcp.bad'], - appliedConfig: cleanedConfig, - } as any) - const { createStubOpenCodeClient } = await import('../helpers/stub-opencode-client') - opencodeServerManager.setOpenCodeClient(createStubOpenCodeClient()) - readOpenCodeConfigFileMock.mockResolvedValue(configFile({ model: 'x', mcp: { bad: true } })) - - await opencodeServerManager.reloadConfig() - - expect(writeOpenCodeConfigFileMock).toHaveBeenCalledWith(JSON.stringify(cleanedConfig, null, 2)) - }) - - it('does not write the config file when the live patch removes nothing', async () => { - const { opencodeServerManager } = await import('../../src/services/opencode-single-server') - const { patchConfigWithRecovery } = await import('../../src/services/opencode/config-recovery') - vi.mocked(patchConfigWithRecovery).mockResolvedValue({ success: true } as any) - const { createStubOpenCodeClient } = await import('../helpers/stub-opencode-client') - opencodeServerManager.setOpenCodeClient(createStubOpenCodeClient()) - readOpenCodeConfigFileMock.mockResolvedValue(configFile({ model: 'x' })) - - await opencodeServerManager.reloadConfig() - - expect(writeOpenCodeConfigFileMock).not.toHaveBeenCalled() - }) - - it('reports a cleaned config validation failure as a ConfigReloadError with the removed fields', async () => { - const { opencodeServerManager } = await import('../../src/services/opencode-single-server') - const { patchConfigWithRecovery } = await import('../../src/services/opencode/config-recovery') - vi.mocked(patchConfigWithRecovery).mockResolvedValue({ - success: true, - removedFields: ['mcp.bad'], - appliedConfig: { model: 'x' }, - } as any) - writeOpenCodeConfigFileMock.mockRejectedValue(new ZodError([ - { code: 'custom', path: ['model'], message: 'Invalid model' }, - ])) - const { createStubOpenCodeClient } = await import('../helpers/stub-opencode-client') - opencodeServerManager.setOpenCodeClient(createStubOpenCodeClient()) - readOpenCodeConfigFileMock.mockResolvedValue(configFile({ model: 'x', mcp: { bad: true } })) - - const error = await opencodeServerManager.reloadConfig().then( - () => null, - (caught: unknown) => caught, - ) - - expect(error).toBeInstanceOf(ConfigReloadError) - const reloadError = error as ConfigReloadError - expect(reloadError.validationIssues).toEqual([{ path: 'model', message: 'Invalid model' }]) - expect(reloadError.removedFields).toEqual(['mcp.bad']) }) - - it('serializes the cleaned-config write against a concurrent apply so neither write interleaves', async () => { - const { opencodeServerManager } = await import('../../src/services/opencode-single-server') - const { patchConfigWithRecovery } = await import('../../src/services/opencode/config-recovery') - const { applyOpenCodeConfigUpdate } = await import('../../src/services/opencode-config-apply') - const { createStubOpenCodeClient } = await import('../helpers/stub-opencode-client') - - const openCodeClient = createStubOpenCodeClient() - opencodeServerManager.setOpenCodeClient(openCodeClient) - readOpenCodeConfigFileMock.mockResolvedValue(configFile({ model: 'x', mcp: { bad: true } })) - - const events: string[] = [] - let releaseReloadWrite!: () => void - const writtenConfig = { - path: '/test/workspace/.config/opencode.json', - rawContent: '{}', - content: {}, - isValid: true, - updatedAt: 0, - } - - vi.mocked(patchConfigWithRecovery) - .mockResolvedValueOnce({ success: true, removedFields: ['mcp.bad'], appliedConfig: { model: 'x' } } as any) - .mockResolvedValueOnce({ success: true } as any) - - writeOpenCodeConfigFileMock - .mockImplementationOnce(() => { - events.push('reload:write:start') - return new Promise((resolve) => { - releaseReloadWrite = () => { - events.push('reload:write:end') - resolve(writtenConfig) - } - }) - }) - .mockImplementationOnce(() => { - events.push('apply:write:start') - return Promise.resolve(writtenConfig) - }) - - const reload = opencodeServerManager.reloadConfig() - await vi.waitFor(() => expect(events).toContain('reload:write:start')) - - const apply = applyOpenCodeConfigUpdate({ - content: { theme: 'light' }, - openCodeClient, - settingsService: { saveLastKnownGoodConfig: vi.fn() } as unknown as Parameters[0]['settingsService'], - }) - await new Promise((resolve) => setTimeout(resolve, 20)) - expect(events).toEqual(['reload:write:start']) - - releaseReloadWrite() - await Promise.all([reload, apply]) - - expect(events).toEqual(['reload:write:start', 'reload:write:end', 'apply:write:start']) - }, 5000) }) describe('OpenCodeServerManager - checkHealth', () => { diff --git a/backend/test/services/opencode-supervisor.test.ts b/backend/test/services/opencode-supervisor.test.ts index 61c18b96..f5674dad 100644 --- a/backend/test/services/opencode-supervisor.test.ts +++ b/backend/test/services/opencode-supervisor.test.ts @@ -1,5 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' -import { archiveBrokenOpenCodeConfigFile, writeHealthWatchArtifact, writeOpenCodeConfigFile, OPENCODE_CONFIG_SEED } from '../../src/services/opencode-config-file' +import { archiveBrokenOpenCodeConfigFile, writeHealthWatchArtifact, restoreOpenCodeConfigSnapshot, OPENCODE_CONFIG_SEED } from '../../src/services/opencode-config-file' import { OpenCodeSupervisor } from '../../src/services/opencode-supervisor' vi.mock('../../src/utils/logger', () => ({ @@ -10,13 +10,18 @@ vi.mock('../../src/utils/logger', () => ({ }, })) -vi.mock('../../src/services/opencode-config-file', () => ({ - archiveBrokenOpenCodeConfigFile: vi.fn(), - writeHealthWatchArtifact: vi.fn(), - writeOpenCodeConfigFile: vi.fn(async (rawContent: string) => ({ rawContent, isValid: true })), - withOpenCodeConfigLock: (fn: () => Promise) => fn(), - OPENCODE_CONFIG_SEED: '{"$schema":"https://opencode.ai/config.json"}', -})) +vi.mock('../../src/services/opencode-config-file', () => { + const seed = '{"$schema":"https://opencode.ai/config.json"}' + return { + archiveBrokenOpenCodeConfigFile: vi.fn(), + writeHealthWatchArtifact: vi.fn(), + restoreOpenCodeConfigSnapshot: vi.fn(async () => ({ isValid: true })), + serializeOpenCodeConfigSnapshot: vi.fn(() => seed), + buildOpenCodeConfigSeedSnapshot: vi.fn(() => seed), + withOpenCodeConfigLock: (fn: () => Promise) => fn(), + OPENCODE_CONFIG_SEED: seed, + } +}) vi.mock('../../src/services/opencode-single-server', () => ({ opencodeServerManager: { @@ -25,9 +30,7 @@ vi.mock('../../src/services/opencode-single-server', () => ({ })) vi.mock('@opencode-manager/shared/config/env', () => ({ - TIMEOUTS: { - CONFIG_PATCH_TIMEOUT_MS: 30000, - }, + TIMEOUTS: {}, ENV: { OPENCODE: { HEALTH_POLL_MS: 200, @@ -43,7 +46,6 @@ interface FakeManager { isOperationInProgress: ReturnType checkHealth: ReturnType restart: ReturnType - reloadConfig: ReturnType clearStartupError: ReturnType getLastStartupError: ReturnType isLastStartupErrorNonRecoverable: ReturnType @@ -69,7 +71,6 @@ describe('OpenCodeSupervisor', () => { isOperationInProgress: vi.fn(() => false), checkHealth: vi.fn().mockResolvedValue(true), restart: vi.fn().mockResolvedValue(undefined), - reloadConfig: vi.fn().mockResolvedValue(undefined), clearStartupError: vi.fn(), getLastStartupError: vi.fn(() => null), isLastStartupErrorNonRecoverable: vi.fn(() => false), @@ -104,7 +105,7 @@ describe('OpenCodeSupervisor', () => { expect(manager.restart).toHaveBeenCalledTimes(3) expect(settings.getLastKnownGoodConfig).toHaveBeenCalled() expect(archiveBrokenOpenCodeConfigFile).toHaveBeenCalled() - expect(writeOpenCodeConfigFile).toHaveBeenCalledWith('{"$schema":"https://opencode.ai/config.json"}') + expect(restoreOpenCodeConfigSnapshot).toHaveBeenCalledWith('{"$schema":"https://opencode.ai/config.json"}') expect(status.watching).toBe(true) await supervisor.stop() @@ -125,7 +126,7 @@ describe('OpenCodeSupervisor', () => { expect(status.state).toBe('failed') expect(archiveBrokenOpenCodeConfigFile).toHaveBeenCalled() - expect(writeOpenCodeConfigFile).toHaveBeenCalledWith(OPENCODE_CONFIG_SEED) + expect(restoreOpenCodeConfigSnapshot).toHaveBeenCalledWith(OPENCODE_CONFIG_SEED) await supervisor.stop() }) @@ -283,7 +284,7 @@ describe('OpenCodeSupervisor', () => { expect(manager.restart).not.toHaveBeenCalled() expect(archiveBrokenOpenCodeConfigFile).not.toHaveBeenCalled() expect(settings.getLastKnownGoodConfig).not.toHaveBeenCalled() - expect(writeOpenCodeConfigFile).not.toHaveBeenCalled() + expect(restoreOpenCodeConfigSnapshot).not.toHaveBeenCalled() expect(writeHealthWatchArtifact).not.toHaveBeenCalled() await supervisor.stop() @@ -304,7 +305,7 @@ describe('OpenCodeSupervisor', () => { expect(status.state).toBe('failed') expect(archiveBrokenOpenCodeConfigFile).not.toHaveBeenCalled() expect(settings.getLastKnownGoodConfig).not.toHaveBeenCalled() - expect(writeOpenCodeConfigFile).not.toHaveBeenCalled() + expect(restoreOpenCodeConfigSnapshot).not.toHaveBeenCalled() expect(writeHealthWatchArtifact).not.toHaveBeenCalled() await supervisor.stop() @@ -331,7 +332,7 @@ describe('OpenCodeSupervisor', () => { expect(manager.restart).toHaveBeenCalledTimes(1) expect(archiveBrokenOpenCodeConfigFile).not.toHaveBeenCalled() expect(settings.getLastKnownGoodConfig).not.toHaveBeenCalled() - expect(writeOpenCodeConfigFile).not.toHaveBeenCalled() + expect(restoreOpenCodeConfigSnapshot).not.toHaveBeenCalled() expect(writeHealthWatchArtifact).not.toHaveBeenCalled() await supervisor.stop() @@ -355,7 +356,7 @@ describe('OpenCodeSupervisor', () => { expect(status.state).toBe('healthy') expect(archiveBrokenOpenCodeConfigFile).toHaveBeenCalled() expect(settings.getLastKnownGoodConfig).toHaveBeenCalled() - expect(writeOpenCodeConfigFile).toHaveBeenCalledWith('{"$schema":"https://opencode.ai/config.json"}') + expect(restoreOpenCodeConfigSnapshot).toHaveBeenCalledWith('{"$schema":"https://opencode.ai/config.json"}') await supervisor.stop() }) @@ -390,36 +391,6 @@ describe('OpenCodeSupervisor', () => { expect(secondStatus.healthy).toBe(true) }) - it('executes a reload requested during an active reload after the active reload completes', async () => { - const manager = createManager() - const settings = createSettings() - const supervisor = new OpenCodeSupervisor(manager as unknown as never, settings as unknown as never, { - failureThreshold: 1, - watchEnabled: false, - }) - - let releaseReload!: () => void - manager.reloadConfig.mockImplementationOnce( - () => new Promise((resolve) => { releaseReload = resolve }), - ) - manager.checkHealth.mockResolvedValue(true) - - const first = supervisor.reloadConfig('settings_reload') - await vi.waitFor(() => expect(manager.reloadConfig).toHaveBeenCalledTimes(1)) - - const second = supervisor.reloadConfig('manual') - await new Promise((resolve) => setTimeout(resolve, 20)) - expect(manager.reloadConfig).toHaveBeenCalledTimes(1) - - releaseReload() - - const [firstStatus, secondStatus] = await Promise.all([first, second]) - - expect(manager.reloadConfig).toHaveBeenCalledTimes(2) - expect(firstStatus.healthy).toBe(true) - expect(secondStatus.healthy).toBe(true) - }) - it('closes the proxy lifecycle gate for the whole restart transition and reopens once healthy', async () => { const manager = createManager() const settings = createSettings() @@ -448,35 +419,6 @@ describe('OpenCodeSupervisor', () => { expect(manager.setLifecycleInitialized).toHaveBeenLastCalledWith(true) }) - it('keeps the proxy lifecycle gate open across a config reload so in-flight sessions are never interrupted', async () => { - const manager = createManager() - const settings = createSettings() - const supervisor = new OpenCodeSupervisor(manager as unknown as never, settings as unknown as never, { - failureThreshold: 1, - watchEnabled: false, - }) - - await supervisor.start() - expect(manager.setLifecycleInitialized).toHaveBeenLastCalledWith(true) - manager.setLifecycleInitialized.mockClear() - - let releaseReload!: () => void - manager.reloadConfig.mockImplementationOnce( - () => new Promise((resolve) => { releaseReload = resolve }), - ) - manager.checkHealth.mockResolvedValue(true) - - const reload = supervisor.reloadConfig('settings_reload') - await vi.waitFor(() => expect(manager.reloadConfig).toHaveBeenCalledTimes(1)) - expect(manager.setLifecycleInitialized).not.toHaveBeenCalledWith(false) - - releaseReload() - const status = await reload - - expect(status.healthy).toBe(true) - expect(manager.setLifecycleInitialized).not.toHaveBeenCalledWith(false) - }) - it('closes the proxy lifecycle gate while stopping and never reopens it', async () => { const manager = createManager() const settings = createSettings() diff --git a/backend/test/services/opencode/config-recovery.test.ts b/backend/test/services/opencode/config-recovery.test.ts deleted file mode 100644 index 9c6a20da..00000000 --- a/backend/test/services/opencode/config-recovery.test.ts +++ /dev/null @@ -1,328 +0,0 @@ -/* eslint-disable @typescript-eslint/no-unused-vars */ -import { describe, it, expect, vi } from 'vitest' - -vi.mock('@opencode-manager/shared/config/env', () => ({ - getWorkspacePath: vi.fn(() => '/test/workspace'), - getOpenCodeConfigFilePath: vi.fn(() => '/test/workspace/.config/opencode.json'), - getReposPath: vi.fn(() => '/test/workspace/repos'), - getAgentsMdPath: vi.fn(() => '/test/workspace/AGENTS.md'), - getDatabasePath: vi.fn(() => ':memory:'), - getConfigPath: vi.fn(() => '/test/workspace/config'), - ENV: { - SERVER: { PORT: 5003, HOST: '0.0.0.0', NODE_ENV: 'test' }, - AUTH: { TRUSTED_ORIGINS: 'http://localhost:5173', SECRET: 'test-secret-for-encryption-key-32c' }, - WORKSPACE: { BASE_PATH: '/test/workspace', REPOS_DIR: 'repos', CONFIG_DIR: 'config', AUTH_FILE: 'auth.json' }, - OPENCODE: { PORT: 5551, HOST: '127.0.0.1' }, - DATABASE: { PATH: ':memory:' }, - FILE_LIMITS: { - MAX_SIZE_BYTES: 1024 * 1024, - MAX_UPLOAD_SIZE_BYTES: 10 * 1024 * 1024, - }, - }, - FILE_LIMITS: { - MAX_SIZE_BYTES: 1024 * 1024, - MAX_UPLOAD_SIZE_BYTES: 10 * 1024 * 1024, - }, - TIMEOUTS: { - CONFIG_PATCH_TIMEOUT_MS: 15000, - }, -})) - -vi.mock('../../../src/utils/logger', () => ({ - logger: { - info: vi.fn(), - error: vi.fn(), - warn: vi.fn(), - }, -})) - -import { patchConfigWithRecovery } from '../../../src/services/opencode/config-recovery' -import type { OpenCodeClient, ForwardRequest } from '../../../src/services/opencode/client' - -function createStubClient( - responses: Array<{ status: number; text: string }>, - capturedRequests?: ForwardRequest[], -): OpenCodeClient { - let callIndex = 0 - return { - - async forward(req: ForwardRequest) { - capturedRequests?.push(req) - const response = responses[callIndex++] ?? responses[responses.length - 1]! - return new Response(response.text, { - status: response.status, - headers: { 'Content-Type': 'application/json' }, - }) - }, - - async forwardRaw(_request: Request) { - throw new Error('not used') - }, - - async getJson(_path: string) { - throw new Error('not used') - }, - - async postJson(_path: string, _body: unknown) { - throw new Error('not used') - }, - - async setProviderAuth(_providerId: string, _apiKey: string) { - throw new Error('not used') - }, - - async deleteProviderAuth(_providerId: string) { - throw new Error('not used') - }, - } -} - -describe('patchConfigWithRecovery', () => { - it('should return success on 200 response with single forward call', async () => { - const config = { agent: { name: 'test' } } - const captured: ForwardRequest[] = [] - const client = createStubClient([ - { status: 200, text: '{}' }, - ], captured) - - const result = await patchConfigWithRecovery(client, config) - - expect(result.success).toBe(true) - expect(result.appliedConfig).toBe(config) - expect(result.error).toBeUndefined() - expect(captured).toHaveLength(1) - }) - - it('should recover by removing command.review on 400 with structured errors', async () => { - const errorResponse = { - success: false, - data: { command: { review: 'some value' } }, - errors: [ - { path: ['command', 'review'], message: 'Invalid command review field' }, - ], - } - - const captured: ForwardRequest[] = [] - const client = createStubClient([ - { status: 400, text: JSON.stringify(errorResponse) }, - { status: 200, text: '{}' }, - ], captured) - - const config = { command: { review: 'test', other: 'value' }, agent: { name: 'test' } } - const result = await patchConfigWithRecovery(client, config) - - expect(result.success).toBe(true) - expect(result.removedFields).toContain('command.review') - expect(result.details).toHaveLength(1) - expect(captured).toHaveLength(2) - - const retryBody = JSON.parse(captured[1]!.body!) as { command?: { review?: unknown; other?: unknown }; agent?: unknown } - expect(retryBody.command?.review).toBeUndefined() - expect(retryBody.command?.other).toBe('value') - expect(retryBody.agent).toEqual({ name: 'test' }) - - expect(result.appliedConfig).toBeDefined() - expect((result.appliedConfig as { command?: { review?: unknown } }).command?.review).toBeUndefined() - }) - - it('should recover from ConfigInvalidError data.issues shape', async () => { - const errorResponse = { - name: 'ConfigInvalidError', - data: { - issues: [ - { path: ['command', 'review'], message: 'Invalid review' }, - ], - }, - } - - const captured: ForwardRequest[] = [] - const client = createStubClient([ - { status: 400, text: JSON.stringify(errorResponse) }, - { status: 200, text: '{}' }, - ], captured) - - const config = { command: { review: 'test' } } - const result = await patchConfigWithRecovery(client, config) - - expect(result.success).toBe(true) - expect(result.removedFields).toContain('command.review') - expect(captured).toHaveLength(2) - - const retryBody = JSON.parse(captured[1]!.body!) as { command?: { review?: unknown } } - expect(retryBody.command?.review).toBeUndefined() - - expect((result.appliedConfig as { command?: { review?: unknown } }).command?.review).toBeUndefined() - }) - - it('should NOT retry if path depth > 3', async () => { - const errorResponse = { - success: false, - data: {}, - errors: [ - { path: ['a', 'b', 'c', 'd'], message: 'Too deep' }, - ], - } - - const captured: ForwardRequest[] = [] - const client = createStubClient([ - { status: 400, text: JSON.stringify(errorResponse) }, - ], captured) - - const config = { a: { b: { c: { d: 'value' } } } } - const result = await patchConfigWithRecovery(client, config) - - expect(result.success).toBe(false) - expect(result.removedFields).toBeUndefined() - expect(captured).toHaveLength(1) - expect(result.details).toHaveLength(1) - expect(result.details?.[0]?.message).toBe('Too deep') - }) - - it('should NOT retry if path is root', async () => { - const errorResponse = { - success: false, - data: {}, - errors: [ - { path: ['root'], message: 'Invalid configuration' }, - ], - } - - const captured: ForwardRequest[] = [] - const client = createStubClient([ - { status: 400, text: JSON.stringify(errorResponse) }, - ], captured) - - const config = { invalid: 'config' } - const result = await patchConfigWithRecovery(client, config) - - expect(result.success).toBe(false) - expect(result.removedFields).toBeUndefined() - expect(captured).toHaveLength(1) - expect(result.details).toHaveLength(1) - expect(result.details?.[0]?.path).toBe('root') - }) - - it('should return retry errors when retry also fails', async () => { - const initialError = { - success: false, - data: {}, - errors: [ - { path: ['command', 'review'], message: 'Initial error' }, - ], - } - - const retryError = { - success: false, - data: {}, - errors: [ - { path: ['agent'], message: 'Retry error - agent invalid' }, - ], - } - - let callCount = 0 - const client: OpenCodeClient = { - async forward(_req: ForwardRequest) { - callCount++ - if (callCount === 1) { - return new Response(JSON.stringify(initialError), { - status: 400, - headers: { 'Content-Type': 'application/json' }, - }) - } - return new Response(JSON.stringify(retryError), { - status: 400, - headers: { 'Content-Type': 'application/json' }, - }) - }, - async forwardRaw(_request: Request) { - throw new Error('not used') - }, - async getJson(_path: string) { - throw new Error('not used') - }, - async postJson(_path: string, _body: unknown) { - throw new Error('not used') - }, - async setProviderAuth(providerId: string, apiKey: string) { - throw new Error('not used') - }, - async deleteProviderAuth(providerId: string) { - throw new Error('not used') - }, - } - - const config = { command: { review: 'test' } } - const result = await patchConfigWithRecovery(client, config) - - expect(result.success).toBe(false) - expect(result.removedFields).toContain('command.review') - expect(result.details).toHaveLength(1) - expect(result.details?.[0]?.message).toBe('Retry error - agent invalid') - expect(callCount).toBe(2) - }) - - it('should return error with Parse error on unparseable response', async () => { - const captured: ForwardRequest[] = [] - const client = createStubClient([ - { status: 400, text: 'not valid json at all' }, - ], captured) - - const config = {} - const result = await patchConfigWithRecovery(client, config) - - expect(result.success).toBe(false) - expect(result.error).toContain('Parse error') - expect(captured).toHaveLength(1) - }) - - it('should pass an AbortSignal to every forward call', async () => { - const errorResponse = { - success: false, - data: {}, - errors: [ - { path: ['command', 'review'], message: 'Invalid command review field' }, - ], - } - - const captured: ForwardRequest[] = [] - const client = createStubClient([ - { status: 400, text: JSON.stringify(errorResponse) }, - { status: 200, text: '{}' }, - ], captured) - - const result = await patchConfigWithRecovery(client, { command: { review: 'test' } }) - - expect(result.success).toBe(true) - expect(captured).toHaveLength(2) - expect(captured[0]!.signal).toBeInstanceOf(AbortSignal) - expect(captured[1]!.signal).toBeInstanceOf(AbortSignal) - expect(captured[0]!.signal).not.toBe(captured[1]!.signal) - }) - - it('should map a TimeoutError rejection to a readable timeout error result', async () => { - const client = createStubClient([]) - client.forward = vi.fn(async () => { - throw new DOMException('The operation was aborted due to timeout', 'TimeoutError') - }) - - const result = await patchConfigWithRecovery(client, {}) - - expect(result.success).toBe(false) - expect(result.error).toMatch(/timed out/i) - }) - - it('should return error on 502 from client.forward', async () => { - const error502Response = { error: 'Proxy request failed' } - const captured: ForwardRequest[] = [] - const client = createStubClient([ - { status: 502, text: JSON.stringify(error502Response) }, - ], captured) - - const config = {} - const result = await patchConfigWithRecovery(client, config) - - expect(result.success).toBe(false) - expect(result.error).toBeDefined() - expect(captured).toHaveLength(1) - }) -}) diff --git a/docs/configuration/docker.md b/docs/configuration/docker.md index 9cee9659..9d0db453 100644 --- a/docs/configuration/docker.md +++ b/docs/configuration/docker.md @@ -309,6 +309,8 @@ services: - ${OCM_OPENCODE_STATE_HOST_PATH}:/import/opencode-state:ro ``` +`OPENCODE_IMPORT_CONFIG_PATH` imports a single file. For a host with multiple recognized config files (`config.json`, `opencode.json`, `opencode.jsonc`), omit it and mount `${OCM_OPENCODE_CONFIG_HOST_PATH}` writable at the container's OpenCode config directory (`/home/node/.config/opencode`) instead. Import mirrors the host's recognized files into the workspace and removes workspace copies absent on the host. + Why the repo mount uses the host path as the container path: - standalone OpenCode stores chats against absolute directory paths diff --git a/docs/configuration/environment.md b/docs/configuration/environment.md index c053759a..399e0455 100644 --- a/docs/configuration/environment.md +++ b/docs/configuration/environment.md @@ -105,7 +105,7 @@ When configured, users can enable push notifications in Settings → Notificatio | Variable | Description | Default | |----------|-------------|---------| -| `OPENCODE_IMPORT_CONFIG_PATH` | Existing standalone OpenCode `opencode.json` to import on first startup | - | +| `OPENCODE_IMPORT_CONFIG_PATH` | Existing standalone OpenCode config file to import on first startup. When set to a single file, only that file is imported. When unset, the host's recognized config files (`config.json`, `opencode.json`, `opencode.jsonc`) are mirrored into the workspace and workspace copies absent on the host are removed | - | | `OPENCODE_IMPORT_STATE_PATH` | Existing standalone OpenCode state directory to import on first startup | - | ## Agent Sandboxing @@ -131,7 +131,6 @@ Sandboxed agent commands run inside a microVM managed by `msb` (see [Agent Sandb | `PROCESS_START_WAIT_MS` | Wait time for OpenCode process to start | `2000` | | `PROCESS_VERIFY_WAIT_MS` | Wait time for process health verification | `1000` | | `HEALTH_CHECK_TIMEOUT_MS` | OpenCode liveness probe timeout | `30000` | -| `CONFIG_PATCH_TIMEOUT_MS` | Timeout for an OpenCode config patch request | `15000` | ## File Limits diff --git a/docs/features/assistant-internal-api.md b/docs/features/assistant-internal-api.md index 7a3eb420..c7010c67 100644 --- a/docs/features/assistant-internal-api.md +++ b/docs/features/assistant-internal-api.md @@ -204,46 +204,71 @@ Returns the updated settings object. ### OpenCode Configuration -The OpenCode configuration file at `getOpenCodeConfigFilePath()` is the source of truth, and this endpoint is the only supported way to change it. The endpoint applies the same restart and live-patch rules as the Settings UI: changes to `agent`, `plugin`, `skills`, or `provider` mark an OpenCode server restart as required, and any other change is live-patched into the running OpenCode server. +The global OpenCode configuration files in the workspace `.config/opencode/` directory are the source of truth, and these endpoints are the only supported way to change them. Up to three sources are recognized and merged in OpenCode order — `config.json`, `opencode.json`, `opencode.jsonc` — with later files overriding earlier ones. The endpoint applies the same rules as the Settings UI: any semantic change is written to disk and marks an OpenCode server restart as required; comment-only edits and changes limited to `mcp` do not. **GET `/api/internal/opencode-config`** -Read the current configuration file state. +Read the merged persisted configuration and its source files. This is not the running instance configuration: project overrides and expanded environment values are not included. **Response (`OpenCodeConfigFile`):** ```ts { - path: string // Absolute path of the configuration file - content: object // Parsed configuration - rawContent: string // Raw file content, including comments - isValid: boolean // Whether the file passes schema validation + path: string // Absolute path of the preferred write target + content: object // Merged configuration across all sources + rawContent: string // Raw content of the preferred write target + sources: Array<{ // Recognized source files, in merge order + name: 'config.json' | 'opencode.json' | 'opencode.jsonc' + path: string + rawContent: string + content: object + isValid: boolean + validationIssues?: Array<{ path: string, message: string }> + updatedAt: number + }> + revision: string // Hash of the source set; send back as expectedRevision + isValid: boolean // Whether every source passes schema validation validationIssues?: Array<{ path: string, message: string }> - updatedAt: number // Unix timestamp of the last write + updatedAt: number // Newest source mtime } ``` **Status Codes:** -- `200`: Configuration file state returned +- `200`: Configuration state returned - `401`: Missing or invalid bearer token -- `404`: No config file found +- `404`: No config source found - `500`: Server error +**GET `/api/internal/opencode-config/effective`** + +Read the running server's effective global configuration (OpenCode `GET /global/config`). Never copy this response into a save. + +**Status Codes:** +- `200`: Effective configuration returned +- `401`: Missing or invalid bearer token +- `502`: OpenCode returned an error +- `503`: OpenCode server unavailable + **PUT `/api/internal/opencode-config`** -Persist a complete configuration. Read the file first, change only the keys the user asked for, and send the complete object back. +Read the merged configuration first, change only the keys the user asked for, and send the complete object back with its `revision`. Only changed paths are patched into the preferred existing source (`opencode.jsonc` > `opencode.json` > `config.json`; a new installation gets `opencode.jsonc`); comments, unknown keys, and untouched inherited values are preserved. For a raw edit, send a string as `content` together with the exact `source` name. **Request Body:** ```ts -{ content: object } // The complete configuration to persist +{ + content: object | string // Complete merged object, or raw text for one source + expectedRevision?: string // From GET; a stale value is rejected with 409 + source?: 'config.json' | 'opencode.json' | 'opencode.jsonc' // Required for raw string edits +} ``` **Response:** -Returns the written `OpenCodeConfigFile`. Adds `restartRequired: true` when the change needs an OpenCode server restart, and `removedFields` when OpenCode dropped fields it does not accept. +Returns the refreshed `OpenCodeConfigFile`. Adds `restartRequired: true` when the change needs an OpenCode server restart. **Status Codes:** -- `200`: Configuration written (live-patched or restart pending) -- `400`: Invalid request body, or configuration rejected with `validationIssues` +- `200`: Configuration written +- `400`: Invalid request body, invalid configuration, or a source file that is not valid JSON/JSONC (`sources` lists them) - `401`: Missing or invalid bearer token +- `409`: Stale `expectedRevision` (`expectedRevision`/`actualRevision` in the body), or the save would remove a value defined only in a lower-priority source (`paths`/`sources` in the body) - `500`: Server error ### Assistant diff --git a/docs/features/server-health.md b/docs/features/server-health.md index a24a88b0..7764e0ec 100644 --- a/docs/features/server-health.md +++ b/docs/features/server-health.md @@ -42,7 +42,7 @@ Health monitoring is configured through environment variables: ## Configuration Recovery -The on-disk `opencode.json` is the source of truth. When the file exists at boot but fails validation, the Manager logs a warning and starts with the file unchanged — an invalid config file is never automatically replaced or rolled back during boot. +The on-disk global configuration files in `.config/opencode/` are the source of truth. OpenCode merges up to three recognized sources in order — `config.json`, `opencode.json`, then `opencode.jsonc` — with later files overriding matching keys from earlier ones. The Manager reads and edits the same set: saves patch only the changed paths into the preferred existing source (`opencode.jsonc` > `opencode.json` > `config.json`), preserving comments and untouched keys, and a fresh installation is seeded with `opencode.jsonc`. When a source exists at boot but fails validation, the Manager logs a warning and starts with the files unchanged — an invalid config is never automatically replaced or rolled back during boot. The health-watch ladder is the only automatic repair path. When the supervised OpenCode server fails repeated health checks, recovery runs these actions in order until the server is healthy: @@ -53,7 +53,7 @@ The health-watch ladder is the only automatic repair path. When the supervised O Because the ladder only runs after repeated failed health checks, a config file that fails validation but does not make the server unhealthy is left in place. Setting `OPENCODE_HEALTH_WATCH_ENABLED=false` disables the ladder entirely, leaving no automatic repair path. -The last known good config is captured from the current on-disk file before every write made through the Settings UI, the internal API, or a host config import, so any of those can be undone with `POST /api/settings/opencode-rollback` or by the ladder. Archived broken configs and debug snapshots are kept under `.opencode/state/health-watch/` in the workspace, pruned to the newest 20 files. +The last known good config is a snapshot of every recognized source file (including which ones exist), captured before every write made through the Settings UI, the internal API, or a host config import, so any of those can be undone with `POST /api/settings/opencode-rollback` or by the ladder. Restoring a snapshot rewrites the sources it contains and removes recognized sources it does not. Archived broken configs and debug snapshots are kept under `.opencode/state/health-watch/` in the workspace, pruned to the newest 20 files. Earlier releases stored named configuration profiles in the Manager database. On first start after upgrading, each profile is archived to `.config/opencode-configs-archive/.json` in the workspace, the default profile is restored to `opencode.json` if that file does not exist yet, and the database table is dropped. @@ -89,4 +89,6 @@ Besides the explicit **Restart** button, the server is automatically restarted w - **Config import completes** — Importing a standalone OpenCode config into the workspace - **Version upgrade** — After installing a new OpenCode version -Saving the OpenCode configuration never restarts the server on its own. Changes to `agent`, `plugin`, `skills`, or `provider` are written to disk and flagged as **restart required**; the server keeps running on the previous configuration until you restart it. Every other change is live-patched into the running server without interrupting active sessions, and is only written to disk once the server has accepted it. +Saving the OpenCode configuration never restarts the server on its own. Any change to the merged configuration is written to disk and flagged as **restart required**; the server keeps running on the previous configuration until you restart it. Two exceptions do not set the flag: comment-only edits, and changes limited to the `mcp` section, which the Settings UI applies to the running server directly. Saving a provider credential, or completing a provider OAuth flow, restarts the server through the same session-resume flow so newly configured providers are discovered. + +A save is rejected with `409` when the files changed since you loaded them (stale revision), or when it would remove a value that is defined only in a lower-priority source file — removing it from the preferred file would leave the inherited value in effect. diff --git a/frontend/src/api/fetchWrapper.ts b/frontend/src/api/fetchWrapper.ts index 4f301d5d..403abd76 100644 --- a/frontend/src/api/fetchWrapper.ts +++ b/frontend/src/api/fetchWrapper.ts @@ -51,7 +51,6 @@ async function handleResponse(response: Response): Promise { { details: data.details, validationIssues: data.validationIssues, - removedFields: data.removedFields, } ) } diff --git a/frontend/src/api/types/settings.ts b/frontend/src/api/types/settings.ts index 3a2f4934..7b668f74 100644 --- a/frontend/src/api/types/settings.ts +++ b/frontend/src/api/types/settings.ts @@ -6,9 +6,12 @@ import { DEFAULT_LEADER_KEY, BLOCKED_SERVER_ENV_KEYS, DEFAULT_SERVER_ENV_VARS, + selectPreferredOpenCodeConfigSourceName, type TTSConfig, type STTConfig, type OpenCodeConfigFile, + type OpenCodeConfigSourceFile, + type OpenCodeConfigSourceName, type UpdateOpenCodeConfigRequest, type ModelConfig, type ProviderConfig, @@ -21,9 +24,25 @@ import { type InstallSkillResponse, } from '@opencode-manager/shared' import type { NotificationPreferences } from '@opencode-manager/shared/types' +import { saveFile } from '@/lib/download' -export type { TTSConfig, STTConfig, OpenCodeConfigFile, UpdateOpenCodeConfigRequest, ModelConfig, ProviderConfig, SandboxPreferences, NotificationPreferences, SkillFileInfo, CreateSkillRequest, UpdateSkillRequest, SkillScope, InstallSkillFromGithubRequest, InstallSkillResponse } +export type { TTSConfig, STTConfig, OpenCodeConfigFile, OpenCodeConfigSourceFile, OpenCodeConfigSourceName, UpdateOpenCodeConfigRequest, ModelConfig, ProviderConfig, SandboxPreferences, NotificationPreferences, SkillFileInfo, CreateSkillRequest, UpdateSkillRequest, SkillScope, InstallSkillFromGithubRequest, InstallSkillResponse } export { DEFAULT_TTS_CONFIG, DEFAULT_STT_CONFIG, DEFAULT_KEYBOARD_SHORTCUTS, DEFAULT_USER_PREFERENCES, DEFAULT_LEADER_KEY, BLOCKED_SERVER_ENV_KEYS, DEFAULT_SERVER_ENV_VARS } +export { isOpenCodeConfigSourceName } from '@opencode-manager/shared' + +export function getOpenCodeConfigSources(config: OpenCodeConfigFile): OpenCodeConfigSourceFile[] { + return config.sources +} + +export function getPreferredOpenCodeConfigSource(config: OpenCodeConfigFile): OpenCodeConfigSourceFile | null { + const name = selectPreferredOpenCodeConfigSourceName(config.sources.map((source) => source.name)) + return name ? config.sources.find((source) => source.name === name) ?? null : null +} + +export function downloadOpenCodeConfigSource(source: OpenCodeConfigSourceFile): void { + const blob = new Blob([source.rawContent], { type: 'application/json' }) + void saveFile(blob, source.name) +} export interface CustomCommand { name: string @@ -88,13 +107,14 @@ export interface UpdateSettingsRequest { export interface OpenCodeConfigSaveResponse extends OpenCodeConfigFile { restartRequired?: boolean - removedFields?: string[] } export interface OpenCodeImportStatus { configSourcePath: string | null + configSourcePaths: string[] stateSourcePath: string | null workspaceConfigPath: string + workspaceConfigPathsToRemove: string[] workspaceStatePath: string workspaceStateExists: boolean } diff --git a/frontend/src/components/settings/AddMcpServerDialog.test.tsx b/frontend/src/components/settings/AddMcpServerDialog.test.tsx index 8a6cc946..467cb2e1 100644 --- a/frontend/src/components/settings/AddMcpServerDialog.test.tsx +++ b/frontend/src/components/settings/AddMcpServerDialog.test.tsx @@ -71,4 +71,22 @@ describe('AddMcpServerDialog', () => { }) expect(mockAddServerAsync).toHaveBeenCalledTimes(1) }) + + it('passes only the merged content to onUpdate', async () => { + const fetched = makeOpenCodeConfigFile({ revision: 'rev-B' }) + mockGetOpenCodeConfig.mockResolvedValue(fetched) + const onUpdate = vi.fn<(content: Record) => Promise>().mockResolvedValue(undefined) + const user = userEvent.setup() + renderDialog(onUpdate) + + await user.type(screen.getByLabelText('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(onUpdate).toHaveBeenCalledTimes(1)) + const [content] = onUpdate.mock.calls[0] + expect(onUpdate.mock.calls[0]).toHaveLength(1) + expect((content.mcp as Record).filesystem).toBeDefined() + expect(mockUpdateOpenCodeConfig).not.toHaveBeenCalled() + }) }) diff --git a/frontend/src/components/settings/OpenCodeConfigEditor.test.tsx b/frontend/src/components/settings/OpenCodeConfigEditor.test.tsx index 28dcf18b..8add9324 100644 --- a/frontend/src/components/settings/OpenCodeConfigEditor.test.tsx +++ b/frontend/src/components/settings/OpenCodeConfigEditor.test.tsx @@ -1,8 +1,14 @@ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { describe, it, expect, vi, beforeEach, afterEach, beforeAll } from 'vitest' import { render, screen, waitFor, fireEvent } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { OpenCodeConfigEditor } from './OpenCodeConfigEditor' -import { makeOpenCodeConfigFile } from '@/test/fixtures/opencode-config' +import { makeOpenCodeConfigFile, makeOpenCodeConfigSource } from '@/test/fixtures/opencode-config' +import { FetchError } from '@/api/fetchWrapper' +import { saveFile } from '@/lib/download' + +vi.mock('@/lib/download', () => ({ + saveFile: vi.fn().mockResolvedValue(true), +})) const RAW = `{ "$schema": "https://opencode.ai/config.json", @@ -10,13 +16,39 @@ const RAW = `{ "model": "anthropic/claude-sonnet-4" }` +const JSONC_RAW = `{ + // preferred target keeps comments + "theme": "system" +}` + +const CONFIG_JSON_RAW = `{ + "theme": "dark" +}` + const config = makeOpenCodeConfigFile({ rawContent: RAW }) +const multiSourceConfig = makeOpenCodeConfigFile({ + path: '/workspace/.config/opencode/opencode.jsonc', + rawContent: JSONC_RAW, + revision: 'rev-2', + sources: [ + makeOpenCodeConfigSource({ name: 'opencode.jsonc', path: '/workspace/.config/opencode/opencode.jsonc', rawContent: JSONC_RAW }), + makeOpenCodeConfigSource({ name: 'opencode.json', path: '/workspace/.config/opencode/opencode.json', rawContent: RAW }), + makeOpenCodeConfigSource({ name: 'config.json', path: '/workspace/.config/opencode/config.json', rawContent: CONFIG_JSON_RAW }), + ], +}) + function setContent(textarea: HTMLTextAreaElement, value: string) { fireEvent.change(textarea, { target: { value } }) } describe('OpenCodeConfigEditor', () => { + beforeAll(() => { + Element.prototype.hasPointerCapture ??= () => false + Element.prototype.setPointerCapture ??= () => {} + Element.prototype.releasePointerCapture ??= () => {} + }) + const renderEditor = ( overrides: Partial> = {}, ) => { @@ -162,7 +194,7 @@ describe('OpenCodeConfigEditor', () => { setContent(textarea, next) await user.click(screen.getByRole('button', { name: 'Update' })) await waitFor(() => { - expect(onUpdate).toHaveBeenCalledWith(next) + expect(onUpdate).toHaveBeenCalledWith({ content: next, source: 'opencode.json', expectedRevision: 'rev-1' }) }) expect(onClose).toHaveBeenCalled() }) @@ -318,7 +350,7 @@ describe('OpenCodeConfigEditor', () => { resolveSave() await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1)) expect(onUpdate).toHaveBeenCalledTimes(1) - expect(onUpdate).toHaveBeenCalledWith(RAW) + 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 () => { @@ -335,4 +367,319 @@ describe('OpenCodeConfigEditor', () => { const row = activeLine?.closest('[data-line]') expect(row).toHaveAttribute('data-line', '4') }) + + it('loads the preferred source and lists every source file', () => { + renderEditor({ config: multiSourceConfig }) + expect(screen.getByText('Edit opencode.jsonc')).toBeInTheDocument() + expect(screen.getByLabelText('Config content')).toHaveValue(JSONC_RAW) + expect(screen.getByText('/workspace/.config/opencode/opencode.jsonc')).toBeInTheDocument() + const selector = screen.getByRole('combobox', { name: 'Source file' }) + expect(selector).toBeInTheDocument() + }) + + it('switches sources without discarding and saves the selected source', async () => { + const { onUpdate } = renderEditor({ config: multiSourceConfig }) + const user = userEvent.setup() + + await user.click(screen.getByRole('combobox', { name: 'Source file' })) + await user.click(screen.getByRole('option', { name: 'config.json' })) + + expect(screen.getByText('Edit config.json')).toBeInTheDocument() + expect(screen.getByText('/workspace/.config/opencode/config.json')).toBeInTheDocument() + const textarea = screen.getByLabelText('Config content') as HTMLTextAreaElement + expect(textarea).toHaveValue(CONFIG_JSON_RAW) + + const next = '{\n "theme": "light"\n}' + setContent(textarea, next) + await user.click(screen.getByRole('button', { name: 'Update' })) + await waitFor(() => { + expect(onUpdate).toHaveBeenCalledWith({ content: next, source: 'config.json', expectedRevision: 'rev-2' }) + }) + }) + + it('disables the source selector while there are unsaved edits', async () => { + const { onClose } = renderEditor({ config: multiSourceConfig }) + const user = userEvent.setup() + const textarea = screen.getByLabelText('Config content') as HTMLTextAreaElement + setContent(textarea, JSONC_RAW + ' ') + expect(screen.getByRole('combobox', { name: 'Source file' })).toBeDisabled() + await user.click(screen.getByRole('button', { name: 'Cancel' })) + await user.click(screen.getByRole('button', { name: 'Discard' })) + expect(onClose).toHaveBeenCalledTimes(1) + }) + + it('downloads the exact selected source file', async () => { + renderEditor({ config: multiSourceConfig }) + const user = userEvent.setup() + + await user.click(screen.getByRole('combobox', { name: 'Source file' })) + await user.click(screen.getByRole('option', { name: 'config.json' })) + await user.click(screen.getByRole('button', { name: 'Download' })) + + expect(saveFile).toHaveBeenCalledWith(expect.any(Blob), 'config.json') + }) + + it('surfaces a conflict error without discarding the raw text', async () => { + const onUpdate = vi.fn().mockRejectedValue( + new FetchError('Conflict', 409, 'CONFIG_CONFLICT', 'This configuration changed since you opened it.'), + ) + renderEditor({ onUpdate }) + const user = userEvent.setup() + const textarea = screen.getByLabelText('Config content') as HTMLTextAreaElement + const next = RAW + ' ' + setContent(textarea, next) + await user.click(screen.getByRole('button', { name: 'Update' })) + + expect(await screen.findByText('This configuration changed since you opened it.')).toBeInTheDocument() + expect(textarea).toHaveValue(next) + expect(screen.getByText('Edit opencode.json')).toBeInTheDocument() + }) + + it('keeps the captured revision when the config refreshes with a newer revision while dirty', async () => { + const onUpdate = vi.fn().mockRejectedValue( + new FetchError('Conflict', 409, 'CONFIG_CONFLICT', 'This configuration changed since you opened it.'), + ) + const onClose = vi.fn() + const { rerender } = render( + , + ) + const user = userEvent.setup() + const textarea = screen.getByLabelText('Config content') as HTMLTextAreaElement + const draft = RAW + ' ' + setContent(textarea, draft) + + const newerConfig = makeOpenCodeConfigFile({ rawContent: RAW, revision: 'rev-2', updatedAt: 2 }) + rerender() + + await user.click(screen.getByRole('button', { name: 'Update' })) + await waitFor(() => { + expect(onUpdate).toHaveBeenCalledWith({ content: draft, source: 'opencode.json', expectedRevision: 'rev-1' }) + }) + expect(await screen.findByText('This configuration changed since you opened it.')).toBeInTheDocument() + expect(textarea).toHaveValue(draft) + expect(screen.getByText('Edit opencode.json')).toBeInTheDocument() + }) + + it('keeps the captured source when it disappears from the refreshed config while dirty', async () => { + const onUpdate = vi.fn().mockRejectedValue( + new FetchError('Conflict', 409, 'CONFIG_CONFLICT', 'This configuration changed since you opened it.'), + ) + const onClose = vi.fn() + const { rerender } = render( + , + ) + const user = userEvent.setup() + const textarea = screen.getByLabelText('Config content') as HTMLTextAreaElement + const draft = JSONC_RAW + '\n// draft\n' + setContent(textarea, draft) + + const sourceRemovedConfig = makeOpenCodeConfigFile({ + path: '/workspace/.config/opencode/opencode.json', + rawContent: RAW, + revision: 'rev-3', + updatedAt: 3, + sources: [ + makeOpenCodeConfigSource({ name: 'opencode.json', path: '/workspace/.config/opencode/opencode.json', rawContent: RAW }), + makeOpenCodeConfigSource({ name: 'config.json', path: '/workspace/.config/opencode/config.json', rawContent: CONFIG_JSON_RAW }), + ], + }) + rerender() + + await user.click(screen.getByRole('button', { name: 'Update' })) + await waitFor(() => { + expect(onUpdate).toHaveBeenCalledWith({ content: draft, source: 'opencode.jsonc', expectedRevision: 'rev-2' }) + }) + expect(await screen.findByText('This configuration changed since you opened it.')).toBeInTheDocument() + expect(textarea).toHaveValue(draft) + expect(screen.getByText('Edit opencode.jsonc')).toBeInTheDocument() + }) + + it('hides the multi-source notice for a single config source', () => { + renderEditor() + expect(screen.queryByText('Multiple configuration files are merged')).not.toBeInTheDocument() + }) + + it('renders the merged notice expanded with its body visible', () => { + renderEditor({ config: multiSourceConfig }) + + const details = screen.getByText('Multiple configuration files are merged').closest('details') as HTMLDetailsElement + expect(details).toHaveAttribute('open') + const paragraphs = Array.from(details.querySelectorAll('p')) + expect(paragraphs).toHaveLength(3) + paragraphs.forEach((paragraph) => { + expect(paragraph).toBeVisible() + }) + }) + + it('collapses and reopens the merged notice from its title', async () => { + const user = userEvent.setup() + renderEditor({ config: multiSourceConfig }) + + const title = screen.getByText('Multiple configuration files are merged') + const details = title.closest('details') as HTMLDetailsElement + expect(title).toBeVisible() + const paragraphs = Array.from(details.querySelectorAll('p')) + expect(paragraphs).toHaveLength(3) + + await user.click(title) + expect(details).not.toHaveAttribute('open') + paragraphs.forEach((paragraph) => { + expect(paragraph).not.toBeVisible() + }) + expect(title).toBeVisible() + + await user.click(title) + expect(details).toHaveAttribute('open') + paragraphs.forEach((paragraph) => { + expect(paragraph).toBeVisible() + }) + }) + + it('keeps the merged notice collapsed when the config refreshes', async () => { + const user = userEvent.setup() + const { rerender, onClose, onUpdate } = renderEditor({ config: multiSourceConfig }) + + const title = screen.getByText('Multiple configuration files are merged') + await user.click(title) + expect(title.closest('details')).not.toHaveAttribute('open') + + const refreshedConfig = makeOpenCodeConfigFile({ + ...multiSourceConfig, + revision: 'rev-3', + updatedAt: 2, + }) + rerender( + , + ) + + const refreshedTitle = screen.getByText('Multiple configuration files are merged') + const refreshedDetails = refreshedTitle.closest('details') as HTMLDetailsElement + expect(refreshedDetails).not.toHaveAttribute('open') + const refreshedParagraphs = Array.from(refreshedDetails.querySelectorAll('p')) + expect(refreshedParagraphs).toHaveLength(3) + refreshedParagraphs.forEach((paragraph) => { + expect(paragraph).not.toBeVisible() + }) + }) + + it('lists merged config files and names the selected source as the write target', () => { + renderEditor({ config: multiSourceConfig }) + + const notice = screen.getByText('Multiple configuration files are merged').closest('[role="alert"]') as HTMLElement + expect(notice).toBeInTheDocument() + expect(notice).toHaveTextContent('config.json, opencode.json, opencode.jsonc') + 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.') + + const scrollContainer = notice.closest('.overflow-y-auto') + expect(scrollContainer).toHaveClass('max-h-[45dvh]') + }) + + it('updates the write target when the source selection changes', async () => { + renderEditor({ config: multiSourceConfig }) + const user = userEvent.setup() + + const notice = screen.getByText('Multiple configuration files are merged').closest('[role="alert"]') as HTMLElement + expect(notice).toHaveTextContent('Saves apply only to opencode.jsonc') + + await user.click(screen.getByRole('combobox', { name: 'Source file' })) + await user.click(screen.getByRole('option', { name: 'config.json' })) + + const updatedNotice = screen.getByText('Multiple configuration files are merged').closest('[role="alert"]') as HTMLElement + expect(updatedNotice).toHaveTextContent('Saves apply only to config.json') + }) + + it('shows the file details disclosure expanded for a single source', () => { + renderEditor() + + const details = screen.getByText('File details').closest('details') as HTMLDetailsElement + expect(details).toHaveAttribute('open') + expect(screen.getByText('/workspace/.config/opencode/opencode.json')).toBeVisible() + expect(screen.getByRole('button', { name: 'Download' })).toBeVisible() + expect(screen.getByText(/Editing this file directly/)).toBeVisible() + }) + + it('hides path, guidance and download when the file details are collapsed', async () => { + const user = userEvent.setup() + renderEditor() + + const summary = screen.getByText('File details') + const details = summary.closest('details') as HTMLDetailsElement + + await user.click(summary) + + expect(details).not.toHaveAttribute('open') + expect(screen.getByText('/workspace/.config/opencode/opencode.json')).not.toBeVisible() + expect(screen.getByText(/Editing this file directly/)).not.toBeVisible() + expect(screen.getByRole('button', { name: 'Download' })).not.toBeVisible() + expect(screen.getByLabelText('Config content')).toBeVisible() + expect(screen.getByRole('button', { name: 'Update' })).toBeEnabled() + }) + + it('reopens the file details disclosure from its summary', async () => { + const user = userEvent.setup() + renderEditor() + + const summary = screen.getByText('File details') + const details = summary.closest('details') as HTMLDetailsElement + + await user.click(summary) + expect(details).not.toHaveAttribute('open') + + await user.click(summary) + expect(details).toHaveAttribute('open') + expect(screen.getByText('/workspace/.config/opencode/opencode.json')).toBeVisible() + }) + + it('keeps the file details summary keyboard focusable and natively activatable', async () => { + const user = userEvent.setup() + renderEditor() + + const label = screen.getByText('File details') + const details = label.closest('details') as HTMLDetailsElement + const summary = label.closest('summary') as HTMLElement + expect(summary).toBeInTheDocument() + + summary.focus() + expect(summary).toHaveFocus() + + await user.click(summary) + expect(details).not.toHaveAttribute('open') + + await user.click(summary) + expect(details).toHaveAttribute('open') + }) + + it('keeps the file details collapsed while editing and when the config refreshes', async () => { + const user = userEvent.setup() + const { rerender, onClose, onUpdate } = renderEditor() + + const summary = screen.getByText('File details') + await user.click(summary) + expect(summary.closest('details')).not.toHaveAttribute('open') + + const textarea = screen.getByLabelText('Config content') as HTMLTextAreaElement + setContent(textarea, RAW + ' ') + + const refreshedConfig = makeOpenCodeConfigFile({ rawContent: RAW, revision: 'rev-2', updatedAt: 2 }) + rerender() + + const refreshedDetails = screen.getByText('File details').closest('details') as HTMLDetailsElement + expect(refreshedDetails).not.toHaveAttribute('open') + expect(screen.getByText('/workspace/.config/opencode/opencode.json')).not.toBeVisible() + expect(screen.getByLabelText('Config content')).toBeVisible() + }) + + it('keeps the merged notice chevron tied to its own disclosure', () => { + renderEditor({ config: multiSourceConfig }) + + const noticeDetails = screen.getByText('Multiple configuration files are merged').closest('details') as HTMLDetailsElement + const chevron = noticeDetails.querySelector('svg') as SVGElement + expect(chevron).toHaveClass('group-open:rotate-180') + expect(chevron).not.toHaveClass('group-open/file-details:rotate-180') + + const fileDetails = screen.getByText('File details').closest('details') as HTMLDetailsElement + expect(fileDetails).toHaveClass('group/file-details') + expect(fileDetails).not.toHaveClass('group') + }) }) diff --git a/frontend/src/components/settings/OpenCodeConfigEditor.tsx b/frontend/src/components/settings/OpenCodeConfigEditor.tsx index 9293189b..4797bb28 100644 --- a/frontend/src/components/settings/OpenCodeConfigEditor.tsx +++ b/frontend/src/components/settings/OpenCodeConfigEditor.tsx @@ -1,16 +1,25 @@ -import { useState, useEffect, useCallback, useRef } from 'react' +import { useState, useEffect, useCallback, useId, useMemo, useRef } from 'react' import { Button } from '@/components/ui/button' -import { Loader2 } from 'lucide-react' +import { Label } from '@/components/ui/label' +import { Loader2, Download, ChevronDown } from 'lucide-react' import { Dialog, DialogContent, DialogHeader, DialogFooter, DialogTitle } from '@/components/ui/dialog' +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select' import { CodeEditor } from '@/components/ui/code-editor' import { EditorFindBar } from '@/components/ui/editor-find-bar' import { UnsavedChangesDialog } from '@/components/ui/unsaved-changes-dialog' +import { OpenCodeConfigSourcesNotice } from './OpenCodeConfigSourcesNotice' 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 type { OpenCodeConfigFile } from '@/api/types/settings' +import { + downloadOpenCodeConfigSource, + getOpenCodeConfigSources, + getPreferredOpenCodeConfigSource, + isOpenCodeConfigSourceName, +} from '@/api/types/settings' +import type { OpenCodeConfigFile, OpenCodeConfigSourceFile, OpenCodeConfigSourceName } from '@/api/types/settings' type ValidationIssue = { path: string @@ -22,7 +31,7 @@ interface OpenCodeConfigEditorProps { config: OpenCodeConfigFile | null isOpen: boolean onClose: () => void - onUpdate: (content: string) => Promise + onUpdate: (request: { content: string; source: OpenCodeConfigSourceName; expectedRevision?: string }) => Promise } export function OpenCodeConfigEditor({ @@ -31,6 +40,8 @@ export function OpenCodeConfigEditor({ onClose, onUpdate, }: OpenCodeConfigEditorProps) { + const [draftSource, setDraftSource] = useState(null) + const [draftRevision, setDraftRevision] = useState('') const [editConfigContent, setEditConfigContent] = useState('') const [initialContent, setInitialContent] = useState('') const [isSaving, setIsSaving] = useState(false) @@ -38,11 +49,12 @@ export function OpenCodeConfigEditor({ const [editError, setEditError] = useState('') const [editErrorLine, setEditErrorLine] = useState(null) const [validationIssues, setValidationIssues] = useState([]) - const [removedFields, setRemovedFields] = useState([]) const [activeLine, setActiveLine] = useState(null) const [revealNonce, setRevealNonce] = useState(0) const hasInitializedSessionRef = useRef(false) const isMobile = useMobile() + const sourceSelectId = useId() + const sources = useMemo(() => (config ? getOpenCodeConfigSources(config) : []), [config]) const isDirty = editConfigContent !== initialContent const { query, setQuery, matches, currentMatchIndex, hasMatches, next, prev } = useFindInText(editConfigContent) @@ -51,13 +63,23 @@ export function OpenCodeConfigEditor({ setRevealNonce((n) => n + 1) }, []) - const resetErrors = () => { + const resetErrors = useCallback(() => { setEditError('') setEditErrorLine(null) setValidationIssues([]) - setRemovedFields([]) setActiveLine(null) - } + }, []) + + const selectSource = useCallback((name: OpenCodeConfigSourceName) => { + if (!config) return + const next = sources.find((source) => source.name === name) + if (!next) return + setDraftSource(next) + setDraftRevision(config.revision) + setEditConfigContent(next.rawContent) + setInitialContent(next.rawContent) + resetErrors() + }, [sources, config, resetErrors]) useEffect(() => { if (!isOpen) { @@ -66,13 +88,22 @@ export function OpenCodeConfigEditor({ } if (hasInitializedSessionRef.current || !config) return hasInitializedSessionRef.current = true - const next = config.rawContent || JSON.stringify(config.content, null, 2) - setEditConfigContent(next) - setInitialContent(next) + const initialSource = getPreferredOpenCodeConfigSource(config) + const nextContent = initialSource?.rawContent ?? '' + setDraftSource(initialSource) + setDraftRevision(config.revision) + setEditConfigContent(nextContent) + setInitialContent(nextContent) resetErrors() setIsSaving(false) setIsDiscardPromptOpen(false) - }, [config, isOpen]) + }, [config, isOpen, resetErrors]) + + const handleSourceChange = (name: string) => { + if (isDirty || isSaving) return + if (!isOpenCodeConfigSourceName(name)) return + selectSource(name) + } const requestClose = () => { if (isSaving) return @@ -108,7 +139,7 @@ export function OpenCodeConfigEditor({ }) const updateConfig = async () => { - if (!config) return + if (!config || !draftSource) return try { resetErrors() @@ -122,7 +153,7 @@ export function OpenCodeConfigEditor({ } setIsSaving(true) - await onUpdate(editConfigContent) + await onUpdate({ content: editConfigContent, source: draftSource.name, expectedRevision: draftRevision }) onClose() } catch (error) { if (error instanceof SyntaxError) { @@ -133,8 +164,11 @@ export function OpenCodeConfigEditor({ } else if (error instanceof FetchError) { const issues = resolveIssues(error.validationIssues ?? []) setValidationIssues(issues) - setRemovedFields(error.removedFields ?? []) - setEditError(error.detail || error.message) + if (error.statusCode === 409) { + setEditError(error.detail || 'This configuration changed since you opened it. Reload the file, then reapply your edits.') + } else { + setEditError(error.detail || error.message) + } } else if (error instanceof Error) { setEditError(error.message) } else { @@ -145,7 +179,7 @@ export function OpenCodeConfigEditor({ } } - if (!config) return null + if (!config || !draftSource) return null return ( <> @@ -160,10 +194,60 @@ export function OpenCodeConfigEditor({ > - Edit opencode.json + Edit {draftSource.name} +
+ + File details + + +
+
+ {sources.length > 1 ? ( +
+ + +
+ ) : ( +

{draftSource.name}

+ )} + +
+

{draftSource.path}

+

+ Editing this file directly. Other source files and inherited values stay as they are; removing a value deletes its override so an inherited value can reappear. +

+ +
+
+ )} - {removedFields.length > 0 && ( -

- Removed invalid fields: {removedFields.join(', ')} -

- )} )} @@ -262,7 +341,7 @@ export function OpenCodeConfigEditor({ onOpenChange={(open) => !open && setIsDiscardPromptOpen(false)} onDiscard={discardAndClose} onKeepEditing={() => setIsDiscardPromptOpen(false)} - itemName="opencode.json" + itemName={draftSource.name} /> ) diff --git a/frontend/src/components/settings/OpenCodeConfigManager.test.tsx b/frontend/src/components/settings/OpenCodeConfigManager.test.tsx index a29722ff..9f763f0d 100644 --- a/frontend/src/components/settings/OpenCodeConfigManager.test.tsx +++ b/frontend/src/components/settings/OpenCodeConfigManager.test.tsx @@ -4,7 +4,7 @@ import userEvent from '@testing-library/user-event' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { OpenCodeConfigManager } from './OpenCodeConfigManager' import type { OpenCodeConfigFile } from '@/api/types/settings' -import { makeOpenCodeConfigFile } from '@/test/fixtures/opencode-config' +import { makeOpenCodeConfigFile, makeOpenCodeConfigSource } from '@/test/fixtures/opencode-config' const { mockGetOpenCodeConfig, @@ -15,6 +15,7 @@ const { mockListManagedSkills, mockListOpenCodeDirectoryFiles, mockGetAgentsMd, + mockAddServerAsync, healthState, } = vi.hoisted(() => ({ mockGetOpenCodeConfig: vi.fn(), @@ -25,6 +26,7 @@ const { mockListManagedSkills: vi.fn(), mockListOpenCodeDirectoryFiles: vi.fn(), mockGetAgentsMd: vi.fn(), + mockAddServerAsync: vi.fn(), healthState: { data: { opencode: 'healthy', opencodeRestartPending: false } as Record }, })) @@ -32,6 +34,34 @@ vi.mock('@/hooks/useServerHealth', () => ({ useServerHealth: () => healthState, })) +vi.mock('@/hooks/useMcpServers', () => ({ + useMcpServers: () => ({ + status: undefined, + isLoading: false, + isError: false, + error: null, + refetch: vi.fn(), + addServer: vi.fn(), + addServerAsync: mockAddServerAsync, + isAddingServer: false, + connect: vi.fn(), + connectAsync: vi.fn(), + isConnecting: false, + disconnect: vi.fn(), + disconnectAsync: vi.fn(), + isDisconnecting: false, + startAuth: vi.fn(), + startAuthAsync: vi.fn(), + isStartingAuth: false, + completeAuth: vi.fn(), + completeAuthAsync: vi.fn(), + isCompletingAuth: false, + removeAuth: vi.fn(), + removeAuthAsync: vi.fn(), + isRemovingAuth: false, + }), +})) + vi.mock('@/lib/toast', () => ({ showToast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), loading: vi.fn(), warning: vi.fn(), dismiss: vi.fn() }, })) @@ -93,6 +123,7 @@ describe('OpenCodeConfigManager', () => { mockUpdateOpenCodeConfig.mockResolvedValue(defaultConfig) mockRestartOpenCodeServer.mockResolvedValue({ success: true, message: 'ok' }) mockGetActiveOpenCodeSessions.mockResolvedValue({ count: 2, sessions: [] }) + mockAddServerAsync.mockResolvedValue(undefined) }) it('shows uploaded command and agent directory files in settings', async () => { @@ -234,22 +265,22 @@ describe('OpenCodeConfigManager', () => { expect(screen.getByText(`Updated: ${new Date(defaultConfig.updatedAt).toLocaleString()}`)).toBeInTheDocument() }) - it('keeps the editor mounted while the post-save config refresh is in flight', async () => { + it('sends the source and expected revision and replaces the cache with the returned snapshot for a raw save', async () => { const configWithRaw: OpenCodeConfigFile = { ...defaultConfig, rawContent: '{\n "theme": "system"\n}', } mockGetOpenCodeConfig.mockResolvedValueOnce(configWithRaw) - let resolveRefresh!: () => void - mockGetOpenCodeConfig.mockReturnValueOnce( - new Promise((resolve) => { - resolveRefresh = () => resolve(configWithRaw) - }), - ) - mockUpdateOpenCodeConfig.mockResolvedValue(configWithRaw) + const savedConfig: OpenCodeConfigFile = { + ...configWithRaw, + rawContent: '{\n "theme": "dark"\n}', + revision: 'rev-2', + updatedAt: 2, + } + mockUpdateOpenCodeConfig.mockResolvedValue(savedConfig) const user = userEvent.setup() - const { container } = renderWithQuery() + const { container, queryClient } = renderWithQuery() await screen.findByText('GPT-4o') const editIcon = container.querySelector('.lucide-square-pen') as SVGElement @@ -257,14 +288,20 @@ describe('OpenCodeConfigManager', () => { await user.click(editButton) const textarea = await screen.findByLabelText('Config content') as HTMLTextAreaElement - fireEvent.change(textarea, { target: { value: configWithRaw.rawContent + ' ' } }) + const next = configWithRaw.rawContent + ' ' + fireEvent.change(textarea, { target: { value: next } }) await user.click(screen.getByRole('button', { name: 'Update' })) - await waitFor(() => expect(mockUpdateOpenCodeConfig).toHaveBeenCalledTimes(1)) - expect(screen.getByText('Edit opencode.json')).toBeInTheDocument() - expect(screen.getByRole('button', { name: 'Update' })).toBeDisabled() - - resolveRefresh() + await waitFor(() => { + expect(mockUpdateOpenCodeConfig).toHaveBeenCalledWith({ + content: next, + source: 'opencode.json', + expectedRevision: 'rev-1', + }) + }) + const cached = queryClient.getQueryData(['opencode-config', 'file']) + expect(cached?.revision).toBe('rev-2') + expect(cached?.rawContent).toBe('{\n "theme": "dark"\n}') await waitFor(() => expect(screen.queryByText('Edit opencode.json')).not.toBeInTheDocument()) }) @@ -283,10 +320,20 @@ describe('OpenCodeConfigManager', () => { fireEvent.change(textarea, { target: { value: draft } }) const refreshedContent = { theme: 'dark' } + const refreshedRaw = JSON.stringify(refreshedContent, null, 2) const refreshedConfig: OpenCodeConfigFile = { ...defaultConfig, content: refreshedContent, - rawContent: JSON.stringify(refreshedContent, null, 2), + rawContent: refreshedRaw, + sources: [ + makeOpenCodeConfigSource({ + name: 'opencode.json', + path: defaultConfig.path, + rawContent: refreshedRaw, + content: refreshedContent, + updatedAt: 2, + }), + ], updatedAt: 2, } mockGetOpenCodeConfig.mockResolvedValue(refreshedConfig) @@ -315,7 +362,151 @@ describe('OpenCodeConfigManager', () => { await user.click(reopenButton) const reopenedTextarea = await screen.findByLabelText('Config content') as HTMLTextAreaElement - expect(reopenedTextarea).toHaveValue(JSON.stringify(refreshedContent, null, 2)) + expect(reopenedTextarea).toHaveValue(refreshedRaw) + }) + + it('sends the expected revision for structured updates', async () => { + mockUpdateOpenCodeConfig.mockResolvedValueOnce({ ...defaultConfig, restartRequired: true }) + + const user = userEvent.setup() + renderWithQuery() + + await screen.findByText('GPT-4o') + + await user.click(screen.getByRole('button', { name: /Models/i })) + + 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] + expect(payload.expectedRevision).toBe('rev-1') + expect(payload.source).toBeUndefined() + expect(payload.content.provider.openai.models).not.toHaveProperty('gpt-4o') + }) + + 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 }) + + const user = userEvent.setup() + renderWithQuery() + + await screen.findByText('GPT-4o') + 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(mockUpdateOpenCodeConfig).toHaveBeenCalledTimes(1)) + const [payload] = mockUpdateOpenCodeConfig.mock.calls[0] + expect(payload.expectedRevision).toBe('rev-A') + expect(mockAddServerAsync).toHaveBeenCalledTimes(1) + }) + + 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, + }) + mockGetOpenCodeConfig.mockResolvedValue(twoModelsConfig) + + const afterFirstDelete = { + ...twoModelsConfig, + content: { + provider: { + openai: { + name: 'OpenAI', + models: { + 'gpt-3.5': { name: 'GPT-3.5' }, + }, + }, + }, + }, + revision: 'rev-2', + } + const afterSecondDelete = { ...afterFirstDelete, revision: 'rev-3' } + mockUpdateOpenCodeConfig + .mockResolvedValueOnce(afterFirstDelete) + .mockResolvedValueOnce(afterSecondDelete) + + const user = userEvent.setup() + renderWithQuery() + + await screen.findByText('GPT-4o') + await user.click(screen.getByRole('button', { name: /Models/i })) + + 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 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') + }) + + it('reports a comment-only raw save as applied without a restart', async () => { + const configWithRaw: OpenCodeConfigFile = { + ...defaultConfig, + rawContent: '{\n // comment only\n "theme": "system"\n}', + } + mockGetOpenCodeConfig.mockResolvedValue(configWithRaw) + mockUpdateOpenCodeConfig.mockResolvedValue({ ...configWithRaw, restartRequired: false, revision: 'rev-2' }) + + const user = userEvent.setup() + const { container } = renderWithQuery() + + await screen.findByText('GPT-4o') + const editIcon = container.querySelector('.lucide-square-pen') as SVGElement + await user.click(editIcon.closest('button') as HTMLButtonElement) + + const textarea = await screen.findByLabelText('Config content') as HTMLTextAreaElement + fireEvent.change(textarea, { target: { value: configWithRaw.rawContent + '\n// another\n' } }) + await user.click(screen.getByRole('button', { name: 'Update' })) + + const { showToast } = await import('@/lib/toast') + await waitFor(() => expect(showToast.success).toHaveBeenCalledWith('Configuration updated')) + expect(showToast.success).not.toHaveBeenCalledWith('Configuration saved. Restart the server to apply changes.') + }) + + it('asks for a restart after a semantic structured change', async () => { + mockUpdateOpenCodeConfig.mockResolvedValueOnce({ ...defaultConfig, restartRequired: true }) + + const user = userEvent.setup() + renderWithQuery() + + await screen.findByText('GPT-4o') + + await user.click(screen.getByRole('button', { name: /Models/i })) + + await user.click(screen.getByLabelText('Actions for GPT-4o')) + await user.click(screen.getByText('Delete')) + + const { showToast } = await import('@/lib/toast') + await waitFor(() => { + expect(showToast.success).toHaveBeenCalledWith('Configuration saved. Restart the server to apply changes.') + }) }) it('does not reset an in-flight save when the config query refreshes in the background', async () => { @@ -384,6 +575,40 @@ describe('OpenCodeConfigManager', () => { expect(await screen.findByRole('button', { name: /Import From Host/i })).toBeInTheDocument() }) + it('warns that importing removes workspace config files absent on the host', async () => { + mockGetOpenCodeImportStatus.mockResolvedValue({ + configSourcePath: '/import/opencode-config/opencode.jsonc', + configSourcePaths: ['/import/opencode-config/opencode.json', '/import/opencode-config/opencode.jsonc'], + stateSourcePath: null, + workspaceConfigPath: '/workspace/.config/opencode/opencode.json', + workspaceConfigPathsToRemove: ['/workspace/.config/opencode/opencode.jsonc'], + workspaceStatePath: '/workspace/.opencode/state/opencode', + workspaceStateExists: false, + }) + + renderWithQuery() + + expect(await screen.findByText(/These files will be removed: opencode.jsonc\./)).toBeInTheDocument() + expect(screen.getByText(/Host files imported: opencode.json, opencode.jsonc\./)).toBeInTheDocument() + }) + + it('does not warn about removals when the host has every workspace config file', async () => { + mockGetOpenCodeImportStatus.mockResolvedValue({ + configSourcePath: '/import/opencode-config/opencode.json', + configSourcePaths: ['/import/opencode-config/opencode.json'], + stateSourcePath: null, + workspaceConfigPath: '/workspace/.config/opencode/opencode.json', + workspaceConfigPathsToRemove: [], + workspaceStatePath: '/workspace/.opencode/state/opencode', + workspaceStateExists: false, + }) + + renderWithQuery() + + await screen.findByText('opencode.json') + expect(screen.queryByText(/These files will be removed/)).not.toBeInTheDocument() + }) + it('shows the config file name and an invalid badge when the file is invalid', async () => { mockGetOpenCodeConfig.mockResolvedValue({ ...defaultConfig, @@ -398,4 +623,32 @@ describe('OpenCodeConfigManager', () => { expect(screen.getByText('Invalid Config')).toBeInTheDocument() expect(screen.getByText('model')).toBeInTheDocument() }) + + it('hides the multi-source notice when only one config file is present', async () => { + renderWithQuery() + + await screen.findByText('opencode.json') + expect(screen.queryByText('Multiple configuration files are merged')).not.toBeInTheDocument() + }) + + it('lists merged config files in override order and names the structured write target', async () => { + const multiSourceConfig = makeOpenCodeConfigFile({ + path: '/workspace/.config/opencode/opencode.jsonc', + rawContent: '{}', + sources: [ + makeOpenCodeConfigSource({ name: 'config.json', path: '/workspace/.config/opencode/config.json' }), + makeOpenCodeConfigSource({ name: 'opencode.json', path: '/workspace/.config/opencode/opencode.json' }), + makeOpenCodeConfigSource({ name: 'opencode.jsonc', path: '/workspace/.config/opencode/opencode.jsonc' }), + ], + }) + mockGetOpenCodeConfig.mockResolvedValue(multiSourceConfig) + + renderWithQuery() + + const notice = (await screen.findByText('Multiple configuration files are merged')).closest('[role="alert"]') as HTMLElement + expect(notice).toBeInTheDocument() + expect(notice).toHaveTextContent('config.json, opencode.json, opencode.jsonc') + 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.') + }) }) diff --git a/frontend/src/components/settings/OpenCodeConfigManager.tsx b/frontend/src/components/settings/OpenCodeConfigManager.tsx index ca5c0490..be277959 100644 --- a/frontend/src/components/settings/OpenCodeConfigManager.tsx +++ b/frontend/src/components/settings/OpenCodeConfigManager.tsx @@ -5,6 +5,7 @@ import { Button } from '@/components/ui/button' import { Badge } from '@/components/ui/badge' import { RestartServerDialog } from './RestartServerDialog' import { OpenCodeConfigEditor } from './OpenCodeConfigEditor' +import { OpenCodeConfigSourcesNotice } from './OpenCodeConfigSourcesNotice' import { CommandsEditor } from './CommandsEditor' import { AgentsEditor } from './AgentsEditor' import { AgentsMdEditor } from './AgentsMdEditor' @@ -18,13 +19,12 @@ 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 { hasJsoncComments } from '@/lib/jsonc' import { showToast } from '@/lib/toast' -import { saveFile } from '@/lib/download' import { invalidateConfigCaches } from '@/lib/queryInvalidation' import { getOpenCodeApiErrorMessage } from '@/lib/opencode-errors' import { FetchError } from '@/api/fetchWrapper' -import type { OpenCodeConfigFile, OpenCodeImportStatus } from '@/api/types/settings' +import { getPreferredOpenCodeConfigSource, downloadOpenCodeConfigSource } from '@/api/types/settings' +import type { OpenCodeConfigFile, OpenCodeConfigSaveResponse, OpenCodeImportStatus } from '@/api/types/settings' interface Command { template: string @@ -156,37 +156,38 @@ export function OpenCodeConfigManager() { return getApiErrorMessage(error, 'Failed to import existing OpenCode host data') } - const updateConfigContent = async (newContent: Record) => { - const previousConfig = queryClient.getQueryData(OPEN_CODE_CONFIG_QUERY_KEY) - const now = Date.now() + const applyOpenCodeConfigSave = (result: OpenCodeConfigSaveResponse) => { + queryClient.setQueryData(OPEN_CODE_CONFIG_QUERY_KEY, result) + if (result.restartRequired) { + showToast.success('Configuration saved. Restart the server to apply changes.') + } else { + showToast.success('Configuration updated') + } + invalidateConfigCaches(queryClient, { skipOpenCodeConfig: true }) + } - queryClient.setQueryData(OPEN_CODE_CONFIG_QUERY_KEY, (prev) => - prev ? { ...prev, content: newContent, updatedAt: now } : prev - ) + 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) + } - try { - const result = await settingsApi.updateOpenCodeConfig({ content: newContent }) - if (result.removedFields && result.removedFields.length > 0) { - showToast.info(`Configuration updated after removing invalid fields: ${result.removedFields.join(', ')}`) - } else if (result.restartRequired) { - showToast.success('Configuration saved. Restart the server to apply changes.') - } else { - showToast.success('Configuration updated') - } - invalidateConfigCaches(queryClient) - } catch (error) { - if (previousConfig) { - queryClient.setQueryData(OPEN_CODE_CONFIG_QUERY_KEY, previousConfig) - } + const updateConfigContentSafely = (newContent: Record) => { + void updateConfigContent(newContent).catch((error) => { showToast.error(getApiErrorMessage(error, 'Failed to update config')) - } + }) } const downloadConfig = (config: OpenCodeConfigFile) => { - const content = config.rawContent || JSON.stringify(config.content, null, 2) - const extension = config.rawContent && hasJsoncComments(config.rawContent) ? 'jsonc' : 'json' - const blob = new Blob([content], { type: 'application/json' }) - void saveFile(blob, `opencode.${extension}`) + const preferredSource = getPreferredOpenCodeConfigSource(config) + if (preferredSource) { + downloadOpenCodeConfigSource(preferredSource) + } } if (isLoading) { @@ -198,6 +199,8 @@ export function OpenCodeConfigManager() { } const canImportFromHost = Boolean(importStatus?.configSourcePath || importStatus?.stateSourcePath) + const workspaceConfigPathsToRemove = importStatus?.workspaceConfigPathsToRemove ?? [] + const hostConfigSourcePaths = importStatus?.configSourcePaths ?? [] return (
@@ -274,6 +277,12 @@ export function OpenCodeConfigManager() {
+

+ Merged persisted settings. Saves write to the preferred config file; removing a value deletes its override so an inherited value can reappear. Restart the server to apply changes. +

+ + + {!config.isValid && config.validationIssues && config.validationIssues.length > 0 && (

This configuration has validation issues

@@ -299,9 +308,13 @@ export function OpenCodeConfigManager() { config={config} isOpen={isEditDialogOpen} onClose={() => setIsEditDialogOpen(false)} - onUpdate={async (rawContent) => { - await settingsApi.updateOpenCodeConfig({ content: rawContent }) - await queryClient.invalidateQueries({ queryKey: OPEN_CODE_CONFIG_QUERY_KEY }) + onUpdate={async ({ content, source, expectedRevision }) => { + const result = await settingsApi.updateOpenCodeConfig({ + content, + source, + expectedRevision, + }) + applyOpenCodeConfigSave(result) }} /> @@ -364,7 +377,7 @@ export function OpenCodeConfigManager() { commands={(config.content.command as Record | undefined) ?? {}} directoryCommands={directoryCommands} onChange={(commands) => { - updateConfigContent({ + updateConfigContentSafely({ ...config.content, command: commands }) @@ -398,7 +411,7 @@ export function OpenCodeConfigManager() { agents={(config.content.agent as Record | undefined) ?? {}} directoryAgents={directoryAgents} onChange={(agents) => { - updateConfigContent({ + updateConfigContentSafely({ ...config.content, agent: agents }) @@ -493,7 +506,7 @@ export function OpenCodeConfigManager() { | undefined) ?? {}} onChange={(providers) => { - updateConfigContent({ + updateConfigContentSafely({ ...config.content, provider: providers }) @@ -567,6 +580,18 @@ export function OpenCodeConfigManager() {

+ {!isImportStatusLoading && workspaceConfigPathsToRemove.length > 0 && ( +

+ + + Importing replaces the workspace configuration files. These files will be removed:{' '} + {workspaceConfigPathsToRemove.map(getConfigFileName).join(', ')}. + {hostConfigSourcePaths.length > 1 && ( + <> Host files imported: {hostConfigSourcePaths.map(getConfigFileName).join(', ')}. + )} + +

+ )}

Workspace State

diff --git a/frontend/src/components/settings/OpenCodeConfigSourcesNotice.tsx b/frontend/src/components/settings/OpenCodeConfigSourcesNotice.tsx new file mode 100644 index 00000000..2662509b --- /dev/null +++ b/frontend/src/components/settings/OpenCodeConfigSourcesNotice.tsx @@ -0,0 +1,53 @@ +import { ChevronDown, Info } from 'lucide-react' +import { Alert, AlertDescription, AlertTitle } from '@/components/ui/alert' +import { OPENCODE_CONFIG_SOURCE_NAMES } from '@opencode-manager/shared' +import { + getOpenCodeConfigSources, + getPreferredOpenCodeConfigSource, +} from '@/api/types/settings' +import type { OpenCodeConfigFile, OpenCodeConfigSourceName } from '@/api/types/settings' + +interface OpenCodeConfigSourcesNoticeProps { + config: OpenCodeConfigFile + targetName?: OpenCodeConfigSourceName +} + +export function OpenCodeConfigSourcesNotice({ config, targetName }: OpenCodeConfigSourcesNoticeProps) { + const sources = getOpenCodeConfigSources(config) + if (sources.length <= 1) return null + + const orderedNames = OPENCODE_CONFIG_SOURCE_NAMES.filter((name) => + sources.some((source) => source.name === name), + ) + const writeTargetName = targetName ?? getPreferredOpenCodeConfigSource(config)?.name + if (!writeTargetName) return null + + return ( + + +

+ + Multiple configuration files are merged + + + +

+ OpenCode loads these files in order, with each later file overriding matching settings from the files before it:{' '} + {orderedNames.map((name, index) => ( + + {index > 0 ? ', ' : ''} + {name} + + ))} +

+

+ Saves apply only to {writeTargetName}. A value saved to a lower-priority file can be overridden by a higher-priority file. +

+

+ For simpler configuration, consolidate the settings you need into one file, then remove redundant files after verifying the result. +

+ +
+ + ) +} diff --git a/frontend/src/hooks/useSSE.ts b/frontend/src/hooks/useSSE.ts index 22dd0d08..14ce2551 100644 --- a/frontend/src/hooks/useSSE.ts +++ b/frontend/src/hooks/useSSE.ts @@ -21,14 +21,14 @@ const getEventDirectory = (event: SSEEvent): string | undefined => { } const handleRestartServer = async () => { - showToast.loading('Reloading OpenCode configuration...', { + showToast.loading('Restarting OpenCode server...', { id: 'restart-server', }) try { const result = await settingsApi.reloadOpenCodeConfig() if (result.success) { - showToast.success(result.message || 'OpenCode configuration reloaded successfully', { + showToast.success(result.message || 'OpenCode server restarted', { id: 'restart-server', duration: 3000, }) @@ -36,13 +36,13 @@ const handleRestartServer = async () => { window.location.reload() }, 2000) } else { - showToast.error(result.message || 'Failed to reload OpenCode configuration', { + showToast.error(result.message || 'Failed to restart OpenCode server', { id: 'restart-server', duration: 5000, }) } } catch (error) { - showToast.error(error instanceof Error ? error.message : 'Failed to reload OpenCode configuration', { + showToast.error(error instanceof Error ? error.message : 'Failed to restart OpenCode server', { id: 'restart-server', duration: 5000, }) diff --git a/frontend/src/hooks/useServerHealth.ts b/frontend/src/hooks/useServerHealth.ts index 9e6c0aa2..9182ecf1 100644 --- a/frontend/src/hooks/useServerHealth.ts +++ b/frontend/src/hooks/useServerHealth.ts @@ -44,14 +44,14 @@ export function useServerHealth(enabled = true) { }, onSuccess: () => { invalidateConfigCaches(queryClient) - toast.success('Server configuration reloaded successfully', { id: 'reload-config' }) + toast.success('OpenCode server restarted', { id: 'reload-config' }) }, onError: (error: unknown) => { const errorMessage = error && typeof error === 'object' && 'response' in error ? ((error as { response?: { data?: { details?: string; error?: string } } }).response?.data?.details || (error as { response?: { data?: { details?: string; error?: string } } }).response?.data?.error - || 'Failed to reload configuration') - : 'Failed to reload configuration' + || 'Failed to restart OpenCode server') + : 'Failed to restart OpenCode server' toast.error(errorMessage, { id: 'reload-config' }) }, }) @@ -103,7 +103,7 @@ export function useServerHealth(enabled = true) { id: 'server-health-unhealthy', duration: Infinity, action: { - label: 'Reload', + label: 'Restart', onClick: () => restartMutation.mutate(), }, }) diff --git a/frontend/src/lib/jsonc.ts b/frontend/src/lib/jsonc.ts index 8c54d925..f3395ea6 100644 --- a/frontend/src/lib/jsonc.ts +++ b/frontend/src/lib/jsonc.ts @@ -7,10 +7,3 @@ export function resolveJsoncIssueLine(content: string, path: PropertyKey[] | str : parseJsoncPathSegments(path) return findJsoncLineForPath(content, segments) } - -export function hasJsoncComments(content: string): boolean { - return content.split('\n').some(line => { - const trimmed = line.trim() - return trimmed.startsWith('//') || trimmed.startsWith('/*') - }) -} diff --git a/frontend/src/lib/opencode-errors.ts b/frontend/src/lib/opencode-errors.ts index de371c7b..598a3e3d 100644 --- a/frontend/src/lib/opencode-errors.ts +++ b/frontend/src/lib/opencode-errors.ts @@ -141,7 +141,7 @@ export function getErrorMessage(error: OpenCodeError | undefined | null): string /** * Extracts a human-readable message from a Manager REST API error, handling * both {@link FetchError} and axios-style `{ response: { data } }` shapes, - * including OpenCode config validation issues and removed fields. + * including OpenCode config validation issues. */ export function getOpenCodeApiErrorMessage(error: unknown, fallback: string): string { if (error instanceof FetchError) { @@ -154,15 +154,11 @@ export function getOpenCodeApiErrorMessage(error: unknown, fallback: string): st message = `Validation failed: ${issues}` } - if (error.removedFields && error.removedFields.length > 0) { - message += ` (removed invalid fields: ${error.removedFields.join(', ')})` - } - return message } if (error && typeof error === 'object' && 'response' in error) { - const response = (error as { response?: { data?: { details?: string; error?: string; validationIssues?: Array<{ path: string; message: string }>; removedFields?: string[] } } }).response + const response = (error as { response?: { data?: { details?: string; error?: string; validationIssues?: Array<{ path: string; message: string }> } } }).response const data = response?.data let message = data?.details || data?.error || fallback @@ -174,10 +170,6 @@ export function getOpenCodeApiErrorMessage(error: unknown, fallback: string): st message = `Validation failed: ${issues}` } - if (data?.removedFields && data.removedFields.length > 0) { - message += ` (removed invalid fields: ${data.removedFields.join(', ')})` - } - return message } diff --git a/frontend/src/lib/queryInvalidation.test.ts b/frontend/src/lib/queryInvalidation.test.ts index 13b7c9ac..364484b1 100644 --- a/frontend/src/lib/queryInvalidation.test.ts +++ b/frontend/src/lib/queryInvalidation.test.ts @@ -1,6 +1,6 @@ import { QueryClient } from '@tanstack/react-query' import { describe, expect, it, vi } from 'vitest' -import { refreshOpenCodeServerCaches } from './queryInvalidation' +import { invalidateConfigCaches, refreshOpenCodeServerCaches } from './queryInvalidation' describe('refreshOpenCodeServerCaches', () => { it('invalidates every cache that displays the installed OpenCode version', () => { @@ -27,3 +27,25 @@ describe('refreshOpenCodeServerCaches', () => { expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ['opencode-versions'] }) }) }) + +describe('invalidateConfigCaches', () => { + it('invalidates the OpenCode config file cache by default', () => { + const queryClient = new QueryClient() + const invalidateQueries = vi.spyOn(queryClient, 'invalidateQueries') + + invalidateConfigCaches(queryClient) + + expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ['opencode-config'] }) + }) + + it('skips the OpenCode config file cache while still invalidating dependents', () => { + const queryClient = new QueryClient() + const invalidateQueries = vi.spyOn(queryClient, 'invalidateQueries') + + invalidateConfigCaches(queryClient, { skipOpenCodeConfig: true }) + + expect(invalidateQueries).not.toHaveBeenCalledWith({ queryKey: ['opencode-config'] }) + expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ['opencode', 'config'] }) + expect(invalidateQueries).toHaveBeenCalledWith({ queryKey: ['health'] }) + }) +}) diff --git a/frontend/src/lib/queryInvalidation.ts b/frontend/src/lib/queryInvalidation.ts index 11bb13eb..b3be9517 100644 --- a/frontend/src/lib/queryInvalidation.ts +++ b/frontend/src/lib/queryInvalidation.ts @@ -18,11 +18,20 @@ export function invalidateProviderCaches(queryClient: QueryClient) { queryClient.invalidateQueries({ queryKey: ['providers-for-execution-model'] }) } -export function invalidateConfigCaches(queryClient: QueryClient) { +interface ConfigInvalidationOptions { + skipOpenCodeConfig?: boolean +} + +export function invalidateConfigCaches( + queryClient: QueryClient, + options: ConfigInvalidationOptions = {}, +) { queryClient.invalidateQueries({ queryKey: ['opencode', 'config'] }) queryClient.invalidateQueries({ queryKey: ['opencode', 'agents'] }) queryClient.invalidateQueries({ queryKey: ['opencode', 'commands'] }) - queryClient.invalidateQueries({ queryKey: ['opencode-config'] }) + if (!options.skipOpenCodeConfig) { + queryClient.invalidateQueries({ queryKey: ['opencode-config'] }) + } queryClient.invalidateQueries({ queryKey: ['health'] }) queryClient.invalidateQueries({ queryKey: ['mcp-status'] }) queryClient.invalidateQueries({ queryKey: ['opencode-skills'] }) diff --git a/frontend/src/test/fixtures/opencode-config.ts b/frontend/src/test/fixtures/opencode-config.ts index 145d4c98..b5e0b74a 100644 --- a/frontend/src/test/fixtures/opencode-config.ts +++ b/frontend/src/test/fixtures/opencode-config.ts @@ -1,12 +1,46 @@ -import type { OpenCodeConfigFile } from '@/api/types/settings' +import { DEFAULT_OPENCODE_CONFIG_SOURCE_NAME, isOpenCodeConfigSourceName } from '@opencode-manager/shared' +import type { OpenCodeConfigFile, OpenCodeConfigSourceFile, OpenCodeConfigSourceName } from '@/api/types/settings' -export function makeOpenCodeConfigFile(overrides: Partial = {}): OpenCodeConfigFile { +function sourceNameFromPath(path: string): OpenCodeConfigSourceName { + const fileName = path.split(/[\\/]/).filter(Boolean).pop() ?? '' + return isOpenCodeConfigSourceName(fileName) ? fileName : DEFAULT_OPENCODE_CONFIG_SOURCE_NAME +} + +export function makeOpenCodeConfigSource( + overrides: Partial = {}, +): OpenCodeConfigSourceFile { return { + name: 'opencode.json', path: '/workspace/.config/opencode/opencode.json', - content: {}, rawContent: '{}', + content: {}, isValid: true, updatedAt: 1, ...overrides, } } + +export function makeOpenCodeConfigFile(overrides: Partial = {}): OpenCodeConfigFile { + const base: OpenCodeConfigFile = { + path: '/workspace/.config/opencode/opencode.json', + content: {}, + rawContent: '{}', + isValid: true, + updatedAt: 1, + revision: 'rev-1', + sources: [], + } + const merged = { ...base, ...overrides } + return { + ...merged, + sources: overrides.sources ?? [makeOpenCodeConfigSource({ + name: sourceNameFromPath(merged.path), + path: merged.path, + rawContent: merged.rawContent, + content: merged.content, + isValid: merged.isValid, + validationIssues: merged.validationIssues, + updatedAt: merged.updatedAt, + })], + } +} diff --git a/shared/src/config/defaults.ts b/shared/src/config/defaults.ts index f48e20ce..69836f8f 100644 --- a/shared/src/config/defaults.ts +++ b/shared/src/config/defaults.ts @@ -46,7 +46,6 @@ export const DEFAULTS = { PROCESS_START_WAIT_MS: 2000, PROCESS_VERIFY_WAIT_MS: 1000, HEALTH_CHECK_TIMEOUT_MS: 30000, - CONFIG_PATCH_TIMEOUT_MS: 15000, }, FILE_LIMITS: { @@ -101,6 +100,9 @@ export const GIT_PROVIDERS = { BITBUCKET: 'bitbucket.org', } as const +export const OPENCODE_CONFIG_SOURCE_NAMES = ['config.json', 'opencode.json', 'opencode.jsonc'] as const +export type OpenCodeConfigSourceName = (typeof OPENCODE_CONFIG_SOURCE_NAMES)[number] + export type Config = typeof DEFAULTS export type AllowedMimeType = (typeof ALLOWED_MIME_TYPES)[number] export type GitProvider = (typeof GIT_PROVIDERS)[keyof typeof GIT_PROVIDERS] diff --git a/shared/src/config/env.ts b/shared/src/config/env.ts index 98254762..b3308431 100644 --- a/shared/src/config/env.ts +++ b/shared/src/config/env.ts @@ -103,7 +103,6 @@ export const ENV = { PROCESS_START_WAIT_MS: getEnvNumber('PROCESS_START_WAIT_MS', DEFAULTS.TIMEOUTS.PROCESS_START_WAIT_MS), PROCESS_VERIFY_WAIT_MS: getEnvNumber('PROCESS_VERIFY_WAIT_MS', DEFAULTS.TIMEOUTS.PROCESS_VERIFY_WAIT_MS), HEALTH_CHECK_TIMEOUT_MS: getEnvNumber('HEALTH_CHECK_TIMEOUT_MS', DEFAULTS.TIMEOUTS.HEALTH_CHECK_TIMEOUT_MS), - CONFIG_PATCH_TIMEOUT_MS: getEnvNumber('CONFIG_PATCH_TIMEOUT_MS', DEFAULTS.TIMEOUTS.CONFIG_PATCH_TIMEOUT_MS), }, FILE_LIMITS: { @@ -160,6 +159,7 @@ export const getForgeWorktreesPath = () => path.join(getOpenCodeDataPath(), 'for export const getOpenCodeTmpHome = () => path.join(ENV.WORKSPACE.BASE_PATH, '.opencode', 'tmp') export const getOpenCodeAgentTmpPath = () => path.join(getOpenCodeTmpHome(), 'opencode') export const getConfigPath = () => path.join(ENV.WORKSPACE.BASE_PATH, ENV.WORKSPACE.CONFIG_DIR) +export { OPENCODE_CONFIG_SOURCE_NAMES } from './defaults' export const getOpenCodeConfigFilePath = () => path.join(ENV.WORKSPACE.BASE_PATH, ENV.WORKSPACE.CONFIG_DIR, 'opencode.json') export const getAgentsMdPath = () => path.join(ENV.WORKSPACE.BASE_PATH, ENV.WORKSPACE.CONFIG_DIR, 'AGENTS.md') export const getAuthPath = () => path.join(ENV.WORKSPACE.BASE_PATH, ENV.WORKSPACE.AUTH_FILE) diff --git a/shared/src/config/index.ts b/shared/src/config/index.ts index a5ff9444..d7335467 100644 --- a/shared/src/config/index.ts +++ b/shared/src/config/index.ts @@ -1,2 +1,3 @@ export * from './defaults' +export * from './opencode-config-sources' export * from './client' diff --git a/shared/src/config/opencode-config-sources.ts b/shared/src/config/opencode-config-sources.ts new file mode 100644 index 00000000..dbd57e0e --- /dev/null +++ b/shared/src/config/opencode-config-sources.ts @@ -0,0 +1,17 @@ +import { OPENCODE_CONFIG_SOURCE_NAMES, type OpenCodeConfigSourceName } from './defaults' + +export const DEFAULT_OPENCODE_CONFIG_SOURCE_NAME: OpenCodeConfigSourceName = 'opencode.jsonc' + +export function isOpenCodeConfigSourceName(value: string): value is OpenCodeConfigSourceName { + return (OPENCODE_CONFIG_SOURCE_NAMES as readonly string[]).includes(value) +} + +export function selectPreferredOpenCodeConfigSourceName( + names: readonly OpenCodeConfigSourceName[], +): OpenCodeConfigSourceName | null { + const present = new Set(names) + for (const name of [...OPENCODE_CONFIG_SOURCE_NAMES].reverse()) { + if (present.has(name)) return name + } + return null +} diff --git a/shared/src/schemas/settings.ts b/shared/src/schemas/settings.ts index 888af9fe..1b9f8c8b 100644 --- a/shared/src/schemas/settings.ts +++ b/shared/src/schemas/settings.ts @@ -1,5 +1,6 @@ import { z } from "zod"; import { NotificationPreferencesSchema, DEFAULT_NOTIFICATION_PREFERENCES } from "./notifications"; +import { OPENCODE_CONFIG_SOURCE_NAMES } from "../config/defaults"; export const CustomCommandSchema = z.object({ name: z.string(), @@ -362,6 +363,18 @@ export const OpenCodeConfigValidationIssueSchema = z.object({ message: z.string(), }); +export const OpenCodeConfigSourceNameSchema = z.enum(OPENCODE_CONFIG_SOURCE_NAMES); + +export const OpenCodeConfigSourceFileSchema = z.object({ + name: OpenCodeConfigSourceNameSchema, + path: z.string(), + rawContent: z.string(), + content: z.record(z.string(), z.unknown()), + isValid: z.boolean(), + validationIssues: z.array(OpenCodeConfigValidationIssueSchema).optional(), + updatedAt: z.number(), +}); + export const OpenCodeConfigFileSchema = z.object({ path: z.string(), content: z.record(z.string(), z.unknown()), @@ -369,8 +382,12 @@ export const OpenCodeConfigFileSchema = z.object({ isValid: z.boolean(), validationIssues: z.array(OpenCodeConfigValidationIssueSchema).optional(), updatedAt: z.number(), + sources: z.array(OpenCodeConfigSourceFileSchema), + revision: z.string(), }); export const UpdateOpenCodeConfigRequestSchema = z.object({ - content: z.union([OpenCodeConfigSchema, z.string()]), + content: z.union([z.record(z.string(), z.unknown()), z.string()]), + source: OpenCodeConfigSourceNameSchema.optional(), + expectedRevision: z.string().optional(), }); diff --git a/shared/src/types/errors.ts b/shared/src/types/errors.ts index 2bf21bbb..2c32cb97 100644 --- a/shared/src/types/errors.ts +++ b/shared/src/types/errors.ts @@ -22,7 +22,6 @@ export interface ApiErrorResponse { detail?: string details?: unknown validationIssues?: Array<{ path: string; message: string }> - removedFields?: string[] } export class FetchError extends Error { @@ -31,7 +30,6 @@ export class FetchError extends Error { detail?: string details?: unknown validationIssues?: Array<{ path: string; message: string }> - removedFields?: string[] constructor( message: string, @@ -41,7 +39,6 @@ export class FetchError extends Error { options?: { details?: unknown validationIssues?: Array<{ path: string; message: string }> - removedFields?: string[] } ) { super(message) @@ -51,6 +48,5 @@ export class FetchError extends Error { this.detail = detail this.details = options?.details this.validationIssues = options?.validationIssues - this.removedFields = options?.removedFields } } diff --git a/shared/src/types/index.ts b/shared/src/types/index.ts index 868497a7..171081fb 100644 --- a/shared/src/types/index.ts +++ b/shared/src/types/index.ts @@ -6,6 +6,7 @@ import { CustomCommandSchema, OpenCodeConfigSchema, OpenCodeConfigFileSchema, + OpenCodeConfigSourceFileSchema, OpenCodeConfigValidationIssueSchema, UpdateOpenCodeConfigRequestSchema, ServerEnvVarSchema, @@ -59,6 +60,8 @@ export type UpdateSettingsRequest = z.infer export type CustomCommand = z.infer export type ServerEnvVar = z.infer export type OpenCodeConfigFile = z.infer +export type { OpenCodeConfigSourceName } from '../config/defaults' +export type OpenCodeConfigSourceFile = z.infer export type OpenCodeConfigValidationIssue = z.infer export type OpenCodeConfigInput = z.infer export type UpdateOpenCodeConfigRequest = z.infer