fix(code-cli): resolve data-loss, credential-leak, and behavior-regression findings from PR review

Addresses a local code review of the CLI-config subsystem covering 18 findings
across 2 CRITICAL, 9 IMPORTANT, and 7 SUGGESTION items:

- Distinguish missing-file from read-error in readExternal/snapshotFile so a
  transient read failure (EACCES/EBUSY) no longer gets misread as "file
  doesn't exist," which previously caused the build path to wipe unmanaged
  config keys and the rollback path to trash real user config files.
- Stop using the MANAGED (full-wipe) key set on incremental build paths for
  Gemini/Qwen/Kimi, which was silently deleting fields Cherry has no UI for.
- Quote dotenv values containing `#` or leading/trailing whitespace so
  Gemini's .env file survives round-tripping through external parsers.
- Fail closed (instead of silently defaulting) when OpenCode provider
  resolution fails, and when Claude Code is enabled without an API key.
- Restore the custom OpenClaw gateway port preference and fix an initial-load
  race in the config draft controller that could misclassify a foreign
  config as managed before apiKeys finished loading.
- Redact likely secrets (including multiline TOML triple-quoted values and
  Bearer tokens) from parser error messages before they reach logs/UI.
- Write CLI config files with 0600 permissions instead of the platform
  default, since they contain credentials.
- Avoid a Codex provider display-name collision with the literal "OpenAI"
  sentinel used to encode remoteCompaction.
- Remove the dead Codex "Write Common Config" toggle and its dead OpenClaw
  IPC routes (check_health/get_channels/check_update/perform_update), which
  had no consumers.
- Harden rollback error logging (don't mask the original write error with a
  restore error) and null out empty-shell connection objects instead of
  treating them as valid foreign configs.

Each fix ships with a regression test verified to fail without its
corresponding change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: eeee0717 <chentao020717Work@outlook.com>
This commit is contained in:
eeee0717
2026-07-05 17:05:11 +08:00
parent 32202e4650
commit 4fc8948ab0
37 changed files with 969 additions and 496 deletions

View File

@@ -19,18 +19,8 @@ export const openclawHandlers: IpcHandlersFor<typeof openclawRequestSchemas> = {
'openclaw.get_status': async () => {
return application.get('OpenClawService').getStatus()
},
'openclaw.check_health': async () => {
return application.get('OpenClawService').checkHealth()
},
'openclaw.get_dashboard_url': async () => {
return application.get('OpenClawService').getDashboardUrl()
},
'openclaw.sync_config': (input) => asOperationResult(() => application.get('OpenClawService').syncConfig(input)),
'openclaw.get_channels': async () => {
return application.get('OpenClawService').getChannelStatus()
},
'openclaw.check_update': async () => {
return application.get('OpenClawService').checkUpdate()
},
'openclaw.perform_update': () => asOperationResult(() => application.get('OpenClawService').performUpdate())
'openclaw.sync_config': (input) => asOperationResult(() => application.get('OpenClawService').syncConfig(input))
}

View File

@@ -501,9 +501,12 @@ class FileStorage {
public writeFile = async (
_: Electron.IpcMainInvokeEvent,
filePath: string,
data: Uint8Array | string
data: Uint8Array | string,
mode?: number
): Promise<void> => {
await fs.promises.writeFile(filePath, data)
await fs.promises.writeFile(filePath, data, mode !== undefined ? { mode } : undefined)
// `writeFile`'s `mode` option only applies when creating a new file; an existing file keeps its old mode.
if (mode !== undefined) await fs.promises.chmod(filePath, mode)
}
public fileNameGuard = async (

View File

@@ -13,8 +13,7 @@ import { isWin } from '@main/core/platform'
import type { Model, Provider, ProviderType, VertexProvider } from '@main/data/migration/v2/legacyTypes'
import { getBinaryPath } from '@main/utils/binaryResolver'
import { findExecutableInEnv } from '@main/utils/commandResolver'
import { crossPlatformSpawn } from '@main/utils/processRunner'
import { getShellEnv, refreshShellEnv } from '@main/utils/shellEnv'
import { refreshShellEnv } from '@main/utils/shellEnv'
import type { EndpointType, Model as DataModel } from '@shared/data/types/model'
import { ENDPOINT_TYPE, parseUniqueModelId, UniqueModelIdSchema } from '@shared/data/types/model'
import type { Provider as DataProvider } from '@shared/data/types/provider'
@@ -26,40 +25,6 @@ import { vertexAiService } from './VertexAiService'
const logger = loggerService.withContext('OpenClawService')
/**
* Parse the current version from `openclaw --version` output.
* Example input: "OpenClaw 2026.3.9 (fe96034)"
*/
export function parseCurrentVersion(versionOutput: string): string | null {
const match = versionOutput.match(/OpenClaw\s+([\d.]+)/i)
return match?.[1] ?? null
}
/**
* Parse the update status from `openclaw update status` output.
* Returns the latest version string if a **binary** update is available, otherwise null.
*
* Cherry Studio installs OpenClaw as a standalone binary, so we only care about
* binary-channel updates. npm/pkg-channel updates are ignored because they
* require a different upgrade path (`npm update -g`).
*
* The table output contains a row like:
* │ Update │ available · binary · 2026.3.12 │
* And a summary line like:
* Update available (binary 2026.3.12). Run: openclaw update
*/
export function parseUpdateStatus(statusOutput: string): string | null {
// Match binary-channel update from table row: "available · binary · <version>"
const tableMatch = statusOutput.match(/available\s*·\s*binary\s*·?\s*([\d.]+)/i)
if (tableMatch) return tableMatch[1]
// Match binary-channel update from summary line: "Update available (binary <version>)"
const summaryMatch = statusOutput.match(/Update available\s*\(binary\s+([\d.]+)\)/i)
if (summaryMatch) return summaryMatch[1]
return null
}
const openclawConfigDir = () => application.getPath('external.openclaw.config')
const openclawConfigPath = () => path.join(openclawConfigDir(), 'openclaw.json')
const openclawConfigBakPath = () => path.join(openclawConfigDir(), 'openclaw.json.bak')
@@ -73,13 +38,6 @@ export interface HealthInfo {
gatewayPort: number
}
export interface ChannelInfo {
id: string
name: string
type: string
status: 'connected' | 'disconnected' | 'error'
}
export interface OpenClawConfig {
gateway?: {
mode?: 'local' | 'remote'
@@ -300,7 +258,7 @@ export class OpenClawService extends BaseService {
}
}
// Refresh shell env first so crossPlatformSpawn uses a fresh env
// Refresh shell env first so the gateway process spawns with a fresh env
const shellEnv = await refreshShellEnv()
const openclawPath = await this.findOpenClawBinary()
if (!openclawPath) {
@@ -488,45 +446,6 @@ export class OpenClawService extends BaseService {
return true
}
private async execOpenClawCommandWithResult(
openclawPath: string,
args: string[],
env: Record<string, string>,
timeoutMs = 20000
): Promise<{ code: number | null; stdout: string; stderr: string }> {
return new Promise((resolve) => {
const proc = crossPlatformSpawn(openclawPath, args, { env })
let stdout = ''
let stderr = ''
proc.stdout?.on('data', (data) => {
stdout += data.toString()
})
proc.stderr?.on('data', (data) => {
stderr += data.toString()
})
const timeout = setTimeout(() => {
logger.warn(`Gateway command timed out: ${args.join(' ')}`)
proc.kill('SIGKILL')
resolve({ code: null, stdout, stderr })
}, timeoutMs)
proc.on('exit', (code) => {
clearTimeout(timeout)
logger.info(`Gateway command [${args.join(' ')}]:`, { code, stdout: stdout.trim(), stderr: stderr.trim() })
resolve({ code, stdout, stderr })
})
proc.on('error', (err) => {
clearTimeout(timeout)
logger.error(`Gateway command error [${args.join(' ')}]:`, err)
resolve({ code: null, stdout, stderr: err.message })
})
})
}
/**
* Get Gateway status. Probes the port when idle to detect externally-started gateways.
*/
@@ -550,22 +469,6 @@ export class OpenClawService extends BaseService {
}
}
/**
* Check Gateway health (public API).
* Returns unhealthy immediately if we know the gateway is not running.
*/
public async checkHealth(): Promise<HealthInfo> {
if (this.gatewayStatus !== 'running') {
return { status: 'unhealthy', gatewayPort: this.gatewayPort }
}
const healthInfo = await this.checkGatewayHealth()
if (healthInfo.status === 'unhealthy') {
logger.warn(`Gateway health check failed, marking as stopped`)
this.gatewayStatus = 'stopped'
}
return healthInfo
}
/**
* Probe gateway health via HTTP request to the health endpoint.
* This is faster than spawning the openclaw binary.
@@ -916,100 +819,6 @@ export class OpenClawService extends BaseService {
}
}
/**
* Check for OpenClaw updates by comparing the installed version with the latest GitHub release.
*/
public async checkUpdate(): Promise<{
hasUpdate: boolean
currentVersion: string | null
latestVersion: string | null
message?: string
}> {
try {
const openclawPath = await this.findOpenClawBinary()
if (!openclawPath) {
return { hasUpdate: false, currentVersion: null, latestVersion: null, message: 'OpenClaw binary not found' }
}
const shellEnv = await getShellEnv()
// Get current version via `openclaw --version`
const versionResult = await this.execOpenClawCommandWithResult(openclawPath, ['--version'], shellEnv, 10000)
const currentVersion = parseCurrentVersion(versionResult.stdout)
// Check for updates via `openclaw update status`
const { code, stdout, stderr } = await this.execOpenClawCommandWithResult(
openclawPath,
['update', 'status'],
shellEnv,
15000
)
if (code !== 0) {
const errMsg = stderr.trim() || `Command exited with code ${code}`
return { hasUpdate: false, currentVersion, latestVersion: null, message: errMsg }
}
const latestVersion = parseUpdateStatus(stdout)
if (latestVersion) {
return { hasUpdate: true, currentVersion, latestVersion }
}
// No update available
return { hasUpdate: false, currentVersion, latestVersion: currentVersion }
} catch (error) {
const errorMessage = error instanceof Error ? error.message : String(error)
logger.error('Failed to check for updates:', error as Error)
return { hasUpdate: false, currentVersion: null, latestVersion: null, message: errorMessage }
}
}
/**
* Perform OpenClaw update through BinaryManager so mise remains the owner.
*/
public async performUpdate(): Promise<OperationResult> {
try {
const openclawPath = await this.findOpenClawBinary()
if (!openclawPath) {
return { success: false, message: 'OpenClaw binary not found' }
}
// Stop gateway before updating
if (this.gatewayStatus === 'running') {
await this.stopGateway()
}
await application.get('BinaryManager').installTool({ name: 'openclaw', tool: 'npm:openclaw' })
void refreshShellEnv()
logger.info('OpenClaw updated successfully')
return { success: true }
} catch (error) {
const errorMessage = error instanceof Error ? error.message : String(error)
logger.error('Failed to update OpenClaw:', error as Error)
return { success: false, message: errorMessage }
}
}
/**
* Get connected channel status
*/
public async getChannelStatus(): Promise<ChannelInfo[]> {
try {
const response = await fetch(`http://127.0.0.1:${this.gatewayPort}/api/channels`, {
signal: AbortSignal.timeout(5000)
})
if (response.ok) {
const data = (await response.json()) as { channels?: ChannelInfo[] }
return data.channels || []
}
} catch (error) {
logger.debug('Failed to get channel status:', error as Error)
}
return []
}
/**
* Like checkGatewayHealth but also returns error message when unhealthy.
* Uses HTTP request for faster health checks.

View File

@@ -1,5 +1,8 @@
import { dialog } from 'electron'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import * as fs from 'fs'
import * as os from 'os'
import * as path from 'path'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
// `t` pulls in i18n + preference machinery that isn't initialized under test; the
// dialog title it produces is irrelevant to these contracts, so stub it to the key.
@@ -39,4 +42,40 @@ describe('FileStorage', () => {
await expect(fileStorage.showInFolder(event, '/no/such/path/x.txt')).rejects.toThrow('/no/such/path/x.txt')
})
})
describe('writeFile', () => {
let tmpFile: string
beforeEach(() => {
tmpFile = path.join(os.tmpdir(), `filestorage-test-${uniqueId()}.txt`)
})
afterEach(() => {
fs.rmSync(tmpFile, { force: true })
})
// CLI config files carry secrets (API keys, tokens) — callers that pass a mode must get that
// exact mode back, not the platform default (0644, world-readable).
it('chmods the file to the requested mode', async () => {
await fileStorage.writeFile(event, tmpFile, 'secret content', 0o600)
expect(fs.statSync(tmpFile).mode & 0o777).toBe(0o600)
})
// `fs.writeFile`'s `mode` option only takes effect when the file is created — an already-existing
// file (e.g. a settings file being re-saved) keeps its prior mode unless explicitly chmod'd.
it('chmods an already-existing file to the requested mode, not just new ones', async () => {
fs.writeFileSync(tmpFile, 'old content', { mode: 0o644 })
await fileStorage.writeFile(event, tmpFile, 'new content', 0o600)
expect(fs.statSync(tmpFile).mode & 0o777).toBe(0o600)
})
it('does not touch the file mode when no mode is given', async () => {
await fileStorage.writeFile(event, tmpFile, 'content')
expect(fs.readFileSync(tmpFile, 'utf-8')).toBe('content')
})
})
})
function uniqueId(): string {
return `${process.pid}-${Math.floor(Math.random() * 1e9)}`
}

View File

@@ -3,8 +3,6 @@ import type { Provider as DataProvider } from '@shared/data/types/provider'
import type { OperationResult } from '@shared/types/codeTools'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { parseCurrentVersion, parseUpdateStatus } from '../OpenClawService'
// --- Mocks for OpenClawService dependencies ---
vi.mock('@main/core/lifecycle', () => {
@@ -50,11 +48,6 @@ vi.mock('@main/utils/commandResolver', () => ({
findExecutableInEnv: vi.fn()
}))
vi.mock('@main/utils/processRunner', () => ({
crossPlatformSpawn: vi.fn(),
runInstallScript: vi.fn()
}))
vi.mock('@data/services/ModelService', () => ({
modelService: {
getByKey: vi.fn(),
@@ -70,7 +63,6 @@ vi.mock('@data/services/ProviderService', () => ({
}))
vi.mock('@main/utils/shellEnv', () => ({
getShellEnv: vi.fn(() => Promise.resolve({ PATH: '/usr/bin' })),
refreshShellEnv: vi.fn(() => Promise.resolve({ PATH: '/usr/bin' }))
}))
@@ -246,57 +238,6 @@ describe('OpenClawService gateway status state machine', () => {
})
})
// ─── checkHealth ─────────────────────────────────────────────
describe('checkHealth', () => {
it('returns unhealthy immediately when status is stopped', async () => {
;(service as any).gatewayStatus = 'stopped'
const result = await service.checkHealth()
expect(result).toEqual({ status: 'unhealthy', gatewayPort: 18790 })
expect(checkHealthSpy).not.toHaveBeenCalled()
})
it('returns unhealthy immediately when status is error', async () => {
;(service as any).gatewayStatus = 'error'
const result = await service.checkHealth()
expect(result).toEqual({ status: 'unhealthy', gatewayPort: 18790 })
expect(checkHealthSpy).not.toHaveBeenCalled()
})
it('returns unhealthy immediately when status is starting', async () => {
;(service as any).gatewayStatus = 'starting'
const result = await service.checkHealth()
expect(result).toEqual({ status: 'unhealthy', gatewayPort: 18790 })
expect(checkHealthSpy).not.toHaveBeenCalled()
})
it('probes and returns healthy when gateway is running and reachable', async () => {
;(service as any).gatewayStatus = 'running'
checkHealthSpy.mockResolvedValue({ status: 'healthy', gatewayPort: 18790 })
const result = await service.checkHealth()
expect(result).toEqual({ status: 'healthy', gatewayPort: 18790 })
expect((service as any).gatewayStatus).toBe('running')
})
it('transitions running → stopped when probe returns unhealthy', async () => {
;(service as any).gatewayStatus = 'running'
checkHealthSpy.mockResolvedValue({ status: 'unhealthy', gatewayPort: 18790 })
const result = await service.checkHealth()
expect(result).toEqual({ status: 'unhealthy', gatewayPort: 18790 })
expect((service as any).gatewayStatus).toBe('stopped')
})
})
// ─── startGateway ────────────────────────────────────────────
describe('startGateway', () => {
@@ -845,134 +786,18 @@ describe('OpenClawService gateway status state machine', () => {
expect(status.status).toBe('running')
})
it('running → checkHealth unhealthy → stopped → getStatus healthy → running', async () => {
it('running → getStatus unhealthy → stopped → getStatus healthy → running', async () => {
;(service as any).gatewayStatus = 'running'
// checkHealth detects crash
// getStatus detects crash
checkHealthSpy.mockResolvedValue({ status: 'unhealthy', gatewayPort: 18790 })
await service.checkHealth()
expect((service as any).gatewayStatus).toBe('stopped')
const crashed = await service.getStatus()
expect(crashed.status).toBe('stopped')
// getStatus detects recovery
checkHealthSpy.mockResolvedValue({ status: 'healthy', gatewayPort: 18790 })
const status = await service.getStatus()
expect(status.status).toBe('running')
const recovered = await service.getStatus()
expect(recovered.status).toBe('running')
})
})
})
// ─── Parser tests (preserved from original) ─────────────────────
describe('parseCurrentVersion', () => {
const cases = [
{ name: 'standard version output', input: 'OpenClaw 2026.3.9 (fe96034)', expected: '2026.3.9' },
{ name: 'version without commit hash', input: 'OpenClaw 2026.3.11', expected: '2026.3.11' },
{ name: 'lowercase prefix', input: 'openclaw 1.0.0 (abc1234)', expected: '1.0.0' },
{ name: 'semver format', input: 'OpenClaw 0.12.3 (deadbeef)', expected: '0.12.3' },
{ name: 'empty string', input: '', expected: null },
{ name: 'unrelated output', input: 'some random text', expected: null },
{ name: 'version with extra whitespace', input: ' OpenClaw 2026.3.9 ', expected: '2026.3.9' }
]
it.each(cases)('$name: "$input"', ({ input, expected }) => {
expect(parseCurrentVersion(input)).toBe(expected)
})
it('snapshot: all cases', () => {
const results = Object.fromEntries(cases.map((c) => [c.name, parseCurrentVersion(c.input)]))
expect(results).toMatchInlineSnapshot(`
{
"empty string": null,
"lowercase prefix": "1.0.0",
"semver format": "0.12.3",
"standard version output": "2026.3.9",
"unrelated output": null,
"version with extra whitespace": "2026.3.9",
"version without commit hash": "2026.3.11",
}
`)
})
})
describe('parseUpdateStatus', () => {
const cases = [
{
name: 'binary update via summary line',
input: 'Update available (binary 2026.3.12). Run: openclaw update',
expected: '2026.3.12'
},
{
name: 'binary update via table row',
input: 'available · binary · 2026.3.12',
expected: '2026.3.12'
},
{
name: 'binary update with semver',
input: 'Update available (binary 1.2.3). Run: openclaw update',
expected: '1.2.3'
},
{
name: 'full table output with binary update',
input: [
'OpenClaw update status',
'┌──────────┬─────────────────────────────────┐',
'│ Install │ binary (~/.cherrystudio/bin) │',
'│ Channel │ stable (default) │',
'│ Update │ available · binary · 2026.3.12 │',
'└──────────┴─────────────────────────────────┘',
'',
'Update available (binary 2026.3.12). Run: openclaw update'
].join('\n'),
expected: '2026.3.12'
},
{
name: 'ignores npm update (summary)',
input: 'Update available (npm 2026.3.11). Run: openclaw update',
expected: null
},
{
name: 'ignores pkg update (table row)',
input: 'Update available · pkg · npm update 2026.3.11',
expected: null
},
{
name: 'ignores pkg update in full table output',
input: [
'OpenClaw update status',
'┌──────────┬─────────────────────────────────┐',
'│ Install │ binary (~/.cherrystudio/bin) │',
'│ Channel │ stable (default) │',
'│ Update │ available · pkg · npm update 2026.3.11 │',
'└──────────┴─────────────────────────────────┘',
'',
'Update available (npm 2026.3.11). Run: openclaw update'
].join('\n'),
expected: null
},
{ name: 'no update available', input: 'Already up to date', expected: null },
{ name: 'empty string', input: '', expected: null },
{ name: 'unrelated output', input: 'some random text', expected: null }
]
it.each(cases)('$name', ({ input, expected }) => {
expect(parseUpdateStatus(input)).toBe(expected)
})
it('snapshot: all cases', () => {
const results = Object.fromEntries(cases.map((c) => [c.name, parseUpdateStatus(c.input)]))
expect(results).toMatchInlineSnapshot(`
{
"binary update via summary line": "2026.3.12",
"binary update via table row": "2026.3.12",
"binary update with semver": "1.2.3",
"empty string": null,
"full table output with binary update": "2026.3.12",
"ignores npm update (summary)": null,
"ignores pkg update (table row)": null,
"ignores pkg update in full table output": null,
"no update available": null,
"unrelated output": null,
}
`)
})
})

View File

@@ -626,12 +626,14 @@ export class CodeCliService extends BaseService {
// OpenCode reads its provider from the opencode.json written above; here we only select the model
// at launch (matching the written provider key) and disable its own auto-update.
if (cliTool === CodeCli.OPEN_CODE) {
let providerName = 'Studio'
let providerName: string
try {
const provider = providerService.getByProviderId(providerId)
providerName = sanitizeProviderName(provider.name, provider.id)
} catch {
/* keep default */
} catch (error) {
const message = `OpenCode provider not found: ${providerId}`
logger.error(message, error as Error)
return { success: false, message, command: '' }
}
baseCommand = `${baseCommand} --model cherry-${providerName}/${model}`
env.OPENCODE_DISABLE_AUTOUPDATE = 'true'

View File

@@ -1,5 +1,5 @@
import { CodeCli } from '@shared/types/codeCli'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
vi.mock('@application', () => ({
application: {
@@ -11,10 +11,19 @@ vi.mock('@application', () => ({
}
}
return {}
})
}),
getPath: vi.fn().mockReturnValue('/mock/binary-data')
}
}))
const providerServiceMock = vi.hoisted(() => ({
getByProviderId: vi.fn()
}))
vi.mock('@main/data/services/ProviderService', () => ({
providerService: providerServiceMock
}))
const loggerMock = vi.hoisted(() => ({
info: vi.fn(),
warn: vi.fn(),
@@ -197,4 +206,37 @@ describe('CodeCliService', () => {
await expect(svc.getPackageName(cliTool)).resolves.toBe(installTool.slice('npm:'.length))
}
})
// A stale/deleted provider must fail the launch outright — previously the
// lookup failure was swallowed, launching OpenCode with a default provider
// name ("Studio") that doesn't match the provider key written into
// opencode.json, while still reporting success.
describe('run (OpenCode provider resolution)', () => {
beforeEach(async () => {
const fs = (await import('node:fs')).default
vi.mocked(fs.existsSync).mockReturnValue(true)
const resolver = await import('@main/utils/binaryResolver')
vi.mocked(resolver.isBinaryExists).mockResolvedValue(true)
vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network disabled in test')))
})
afterEach(() => {
vi.unstubAllGlobals()
})
it('fails the launch instead of defaulting to a wrong provider name', async () => {
providerServiceMock.getByProviderId.mockImplementation(() => {
throw new Error('Provider not found: ghost')
})
const { codeCliService } = await loadModules()
const result = await codeCliService.run(CodeCli.OPEN_CODE, 'gpt-4o', 'ghost', '/tmp/project')
expect(result).toEqual({
success: false,
message: expect.stringContaining('OpenCode provider not found: ghost'),
command: ''
})
})
})
})

View File

@@ -195,7 +195,8 @@ const api = {
get: (filePath: string): Promise<FileMetadata | null> => ipcRenderer.invoke(IpcChannel.File_Get, filePath),
createTempFile: (fileName: string): Promise<string> => ipcRenderer.invoke(IpcChannel.File_CreateTempFile, fileName),
mkdir: (dirPath: string) => ipcRenderer.invoke(IpcChannel.File_Mkdir, dirPath),
write: (filePath: string, data: Uint8Array | string) => ipcRenderer.invoke(IpcChannel.File_Write, filePath, data),
write: (filePath: string, data: Uint8Array | string, mode?: number) =>
ipcRenderer.invoke(IpcChannel.File_Write, filePath, data, mode),
open: (options?: OpenDialogOptions) => ipcRenderer.invoke(IpcChannel.File_Open, options),
openPath: (path: string) => ipcRenderer.invoke(IpcChannel.File_OpenPath, path),
save: (path: string, content: string | NodeJS.ArrayBufferView, options?: any): Promise<string | null> =>

View File

@@ -1949,7 +1949,6 @@
"sonnet_model": "Sonnet"
},
"codex": {
"common_config": "Write Common Config",
"disable_response_storage": "Disable Response Storage",
"goal_mode": "Enable Goal Mode",
"remote_compaction": "Enable Remote Compaction"

View File

@@ -1949,7 +1949,6 @@
"sonnet_model": "Sonnet"
},
"codex": {
"common_config": "写入通用配置",
"disable_response_storage": "禁用响应存储",
"goal_mode": "启用 Goal 模式",
"remote_compaction": "启用远程压缩"

View File

@@ -1949,7 +1949,6 @@
"sonnet_model": "Sonnet"
},
"codex": {
"common_config": "寫入通用設定",
"disable_response_storage": "禁用回應儲存",
"goal_mode": "啟用 Goal 模式",
"remote_compaction": "啟用遠端壓縮"

View File

@@ -6,10 +6,12 @@ import { clearCliConfig } from '../index'
let existing: Record<string, string>
let writes: Record<string, string>
let modes: Record<string, number | undefined>
beforeEach(() => {
existing = {}
writes = {}
modes = {}
Object.defineProperty(window, 'api', {
configurable: true,
value: {
@@ -20,8 +22,9 @@ beforeEach(() => {
throw new Error(`File does not exist: ${p}`)
}),
mkdir: vi.fn(async () => undefined),
write: vi.fn(async (p: string, content: string) => {
write: vi.fn(async (p: string, content: string, mode?: number) => {
writes[p] = content
modes[p] = mode
})
}
}
@@ -184,4 +187,63 @@ describe('clearCliConfig', () => {
await clearCliConfig({ cliTool: CodeCli.OPENCLAW })
expect(writes).toEqual({})
})
// Cleared configs still hold non-Cherry secrets on disk — every rewrite must stay 0600, not fall
// back to the platform default (0644, world-readable) once it goes through the generic file IPC.
it('writes every cleared config file with 0600 permissions', async () => {
existing['/resolved~/.codex/config.toml'] = 'model_provider = "cherry-deepseek"\nuser_key = "keep"'
existing['/resolved~/.codex/auth.json'] = JSON.stringify({ OPENAI_API_KEY: 'sk', user: 'keep' })
await clearCliConfig({ cliTool: CodeCli.OPENAI_CODEX })
existing['/resolved~/.gemini/.env'] = 'CHERRY_KEY=sk\nUSER_KEY=keep'
existing['/resolved~/.gemini/settings.json'] = JSON.stringify({ general: { userSetting: 'keep' } })
await clearCliConfig({ cliTool: CodeCli.GEMINI_CLI })
expect(modes['/resolved~/.codex/config.toml']).toBe(0o600)
expect(modes['/resolved~/.codex/auth.json']).toBe(0o600)
expect(modes['/resolved~/.gemini/.env']).toBe(0o600)
expect(modes['/resolved~/.gemini/settings.json']).toBe(0o600)
})
// S6: clear must never overwrite a config it can't fully understand — a malformed on-disk file
// should abort the whole operation rather than silently rewriting it as if it were empty.
describe('aborts instead of overwriting a malformed existing config file', () => {
it('claude: rejects and writes nothing', async () => {
existing['/resolved~/.claude/settings.json'] = '{ not valid json'
await expect(clearCliConfig({ cliTool: CodeCli.CLAUDE_CODE })).rejects.toThrow(/Failed to parse/)
expect(writes).toEqual({})
})
it('codex: rejects and writes nothing (config.toml malformed, secret redacted)', async () => {
existing['/resolved~/.codex/config.toml'] = 'api_key = "sk-ant-real-secret"\nbroken====='
existing['/resolved~/.codex/auth.json'] = JSON.stringify({ OPENAI_API_KEY: 'sk', user: 'keep' })
await expect(clearCliConfig({ cliTool: CodeCli.OPENAI_CODEX })).rejects.toThrow(/Failed to parse/)
await expect(clearCliConfig({ cliTool: CodeCli.OPENAI_CODEX })).rejects.not.toThrow(/sk-ant-real-secret/)
expect(writes).toEqual({})
})
it('opencode: rejects and writes nothing', async () => {
existing['/resolved~/.config/opencode/opencode.json'] = '{ not valid json'
await expect(clearCliConfig({ cliTool: CodeCli.OPEN_CODE })).rejects.toThrow(/Failed to parse/)
expect(writes).toEqual({})
})
it('gemini: rejects and writes nothing (settings.json malformed)', async () => {
existing['/resolved~/.gemini/settings.json'] = '{ not valid json'
await expect(clearCliConfig({ cliTool: CodeCli.GEMINI_CLI })).rejects.toThrow(/Failed to parse/)
expect(writes).toEqual({})
})
it('qwen: rejects and writes nothing', async () => {
existing['/resolved~/.qwen/settings.json'] = '{ not valid json'
await expect(clearCliConfig({ cliTool: CodeCli.QWEN_CODE })).rejects.toThrow(/Failed to parse/)
expect(writes).toEqual({})
})
it('kimi: rejects and writes nothing', async () => {
existing['/resolved~/.kimi-code/config.toml'] = 'broken====='
await expect(clearCliConfig({ cliTool: CodeCli.KIMI_CODE })).rejects.toThrow(/Failed to parse/)
expect(writes).toEqual({})
})
})
})

View File

@@ -0,0 +1,60 @@
import { describe, expect, it } from 'vitest'
import { parseDotenv, renderDotenvFile } from '../dotenv'
describe('renderDotenvFile', () => {
it('writes plain values without quotes', () => {
expect(renderDotenvFile(new Map([['KEY', 'value']]))).toBe('KEY=value\n')
})
it('quotes a value containing #', () => {
expect(renderDotenvFile(new Map([['HTTPS_PROXY', 'http://user:p#ss@host']]))).toBe(
'HTTPS_PROXY="http://user:p#ss@host"\n'
)
})
it('quotes a value with leading/trailing whitespace', () => {
expect(renderDotenvFile(new Map([['KEY', ' value ']]))).toBe('KEY=" value "\n')
})
it('quotes and escapes a value containing a double quote', () => {
expect(renderDotenvFile(new Map([['KEY', 'say "hi"']]))).toBe('KEY="say \\"hi\\""\n')
})
it('quotes an empty value', () => {
expect(renderDotenvFile(new Map([['KEY', '']]))).toBe('KEY=""\n')
})
})
describe('parseDotenv', () => {
it('parses a plain unquoted value', () => {
expect(parseDotenv('KEY=value\n')).toEqual(new Map([['KEY', 'value']]))
})
it('unquotes a double-quoted value without truncating at #', () => {
expect(parseDotenv('HTTPS_PROXY="http://user:p#ss@host"\n')).toEqual(
new Map([['HTTPS_PROXY', 'http://user:p#ss@host']])
)
})
it('unquotes a single-quoted value', () => {
expect(parseDotenv("KEY='value'\n")).toEqual(new Map([['KEY', 'value']]))
})
it('unescapes a double-quoted value containing an escaped quote', () => {
expect(parseDotenv('KEY="say \\"hi\\""\n')).toEqual(new Map([['KEY', 'say "hi"']]))
})
it('skips comment lines and blank lines', () => {
expect(parseDotenv('# comment\n\nKEY=value\n')).toEqual(new Map([['KEY', 'value']]))
})
})
describe('round-trip', () => {
const cases = ['plain-value', 'http://user:p#ss@host', ' leading-and-trailing ', 'say "hi"', '', "it's fine"]
it.each(cases)('renders and re-parses %j unchanged', (value) => {
const rendered = renderDotenvFile(new Map([['KEY', value]]))
expect(parseDotenv(rendered).get('KEY')).toBe(value)
})
})

View File

@@ -56,6 +56,18 @@ describe('writeCliConfigDraft', () => {
expect(mkdirMock.mock.invocationCallOrder[1]).toBeLessThan(writeMock.mock.invocationCallOrder[1])
})
// Config files carry secrets (auth tokens, API keys) — they must never be written world-readable.
it('writes every config file with 0600 permissions', async () => {
await writeCliConfigDraft({
cliTool: CodeCli.OPENAI_CODEX,
files: [codexConfigDraft, codexAuthDraft]
})
const writeMock = vi.mocked(window.api.file.write)
expect(writeMock).toHaveBeenCalledWith(codexConfigDraft.path, codexConfigDraft.content, 0o600)
expect(writeMock).toHaveBeenCalledWith(codexAuthDraft.path, codexAuthDraft.content, 0o600)
})
it('ensures parent directories before rollback writes', async () => {
existing[codexConfigDraft.path] = 'user_key = "keep"\n'
vi.mocked(window.api.file.write).mockImplementation(async (p: string) => {
@@ -71,7 +83,44 @@ describe('writeCliConfigDraft', () => {
const mkdirMock = vi.mocked(window.api.file.mkdir)
expect(window.api.file.deleteExternalFile).toHaveBeenCalledWith(codexAuthDraft.path)
expect(window.api.file.write).toHaveBeenLastCalledWith(codexConfigDraft.path, 'user_key = "keep"\n')
expect(window.api.file.write).toHaveBeenLastCalledWith(codexConfigDraft.path, 'user_key = "keep"\n', 0o600)
expect(mkdirMock).toHaveBeenLastCalledWith('/tmp/cherry/.codex')
})
// A restore write can itself fail (disk full, permission denied, etc.) — that failure must be
// logged, not thrown in place of the original write error, and must not abort the remaining
// rollbacks in the chain (S1: the restore branch previously had no .catch, unlike delete).
it('surfaces the original write error (not a restore failure) and still rolls back the rest', async () => {
const openCodeConfigDraft: CliConfigFileDraft = {
target: 'opencode-config',
label: 'OpenCode config',
path: '/tmp/cherry/.config/opencode/opencode.json',
language: 'json',
content: '{ "new": true }\n'
}
existing[codexConfigDraft.path] = 'user_key = "keep"\n'
existing[codexAuthDraft.path] = '{ "user": "keep" }\n'
// Write order: codexConfigDraft (ok), codexAuthDraft (ok), openCodeConfigDraft (fails) → rollback
// in reverse: openCodeConfigDraft (delete, new file), codexAuthDraft (restore, fails), then
// codexConfigDraft (restore, must still run despite the previous restore failure).
vi.mocked(window.api.file.write).mockImplementationOnce(async () => undefined) // codexConfigDraft write
vi.mocked(window.api.file.write).mockImplementationOnce(async () => undefined) // codexAuthDraft write
vi.mocked(window.api.file.write).mockImplementationOnce(async () => {
throw new Error('disk full')
}) // openCodeConfigDraft write fails
vi.mocked(window.api.file.write).mockImplementationOnce(async () => {
throw new Error('restore failed: disk still full')
}) // rollback restore of codexAuthDraft fails
await expect(
writeCliConfigDraft({
cliTool: CodeCli.OPENAI_CODEX,
files: [codexConfigDraft, codexAuthDraft, openCodeConfigDraft]
})
).rejects.toThrow('disk full')
// codexConfigDraft (rolled back last, since rollback order is reversed) must still be restored
// even though codexAuthDraft's restore failed first in the chain.
expect(window.api.file.write).toHaveBeenLastCalledWith(codexConfigDraft.path, 'user_key = "keep"\n', 0o600)
})
})

View File

@@ -0,0 +1,25 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { readAndParseDraftFile } from '../draftFiles'
import { parseTomlOrThrow } from '../file'
describe('readAndParseDraftFile (secret redaction on parse failure)', () => {
beforeEach(() => {
Object.defineProperty(window, 'api', {
configurable: true,
value: {
resolvePath: vi.fn(async (p: string) => `/resolved${p}`),
file: {
readExternal: vi.fn(async () => 'api_key = "sk-ant-real-secret"\nbroken=====')
}
}
})
})
it('does not leak the raw secret from a malformed TOML file into the thrown error', async () => {
await expect(readAndParseDraftFile('kimi-config', parseTomlOrThrow)).rejects.toThrow(
/Failed to parse .*api_key = "<redacted>"/s
)
await expect(readAndParseDraftFile('kimi-config', parseTomlOrThrow)).rejects.not.toThrow(/sk-ant-real-secret/)
})
})

View File

@@ -39,6 +39,12 @@ const geminiProvider = {
name: 'Gemini',
endpointConfigs: { 'google-generate-content': { baseUrl: 'https://generativelanguage.googleapis.com' } }
} as unknown as Provider
/** Display name is literally "OpenAI" — the exact literal Codex reserves for remote-compaction mode. */
const openaiNamedProvider = {
id: 'openai-official',
name: 'OpenAI',
endpointConfigs: { 'openai-responses': { baseUrl: 'https://api.openai.com/v1' } }
} as unknown as Provider
beforeEach(() => {
Object.defineProperty(window, 'api', {
@@ -96,6 +102,43 @@ describe('extractConnectionFromCliConfigDraft', () => {
}
expect(extractConnectionFromCliConfigDraft(CodeCli.CLAUDE_CODE, [badClaude])).toBeNull()
})
// S4: an existing-but-empty config file parses to an all-`undefined` connection object, which
// is truthy — callers doing `if (!connection)` must not misread that as a real foreign connection.
const emptyFileCases: Array<[string, CodeCli, CliConfigFileDraft[]]> = [
[
'claude',
CodeCli.CLAUDE_CODE,
[{ target: 'claude-settings', label: '', path: '', language: 'json', content: '{}' }]
],
[
'codex',
CodeCli.OPENAI_CODEX,
[
{ target: 'codex-config', label: '', path: '', language: 'toml', content: '' },
{ target: 'codex-auth', label: '', path: '', language: 'json', content: '{}' }
]
],
[
'opencode',
CodeCli.OPEN_CODE,
[{ target: 'opencode-config', label: '', path: '', language: 'json', content: '{}' }]
],
[
'gemini',
CodeCli.GEMINI_CLI,
[
{ target: 'gemini-env', label: '', path: '', language: 'dotenv', content: '' },
{ target: 'gemini-settings', label: '', path: '', language: 'json', content: '{}' }
]
],
['qwen', CodeCli.QWEN_CODE, [{ target: 'qwen-settings', label: '', path: '', language: 'json', content: '{}' }]],
['kimi', CodeCli.KIMI_CODE, [{ target: 'kimi-config', label: '', path: '', language: 'toml', content: '' }]]
]
it.each(emptyFileCases)('returns null for an existing-but-empty %s config', (_name, cliTool, files) => {
expect(extractConnectionFromCliConfigDraft(cliTool, files)).toBeNull()
})
})
describe('extractConfigFromCliConfigDraft', () => {
@@ -115,6 +158,16 @@ describe('extractConfigFromCliConfigDraft', () => {
})
})
// Regression: a provider literally named "OpenAI" must not collide with the
// "OpenAI" literal Codex reserves for remote-compaction mode.
it('round-trips remoteCompaction for a provider literally named "OpenAI"', async () => {
const filesOff = await buildDraft(CodeCli.OPENAI_CODEX, openaiNamedProvider, 'gpt-5', {})
expect(extractConfigFromCliConfigDraft(CodeCli.OPENAI_CODEX, filesOff)).toEqual({})
const filesOn = await buildDraft(CodeCli.OPENAI_CODEX, openaiNamedProvider, 'gpt-5', { remoteCompaction: true })
expect(extractConfigFromCliConfigDraft(CodeCli.OPENAI_CODEX, filesOn)).toEqual({ remoteCompaction: true })
})
it('round-trips supported Claude managed settings from the config blob', async () => {
const blob = {
effortLevel: 'xhigh',

View File

@@ -0,0 +1,45 @@
import { describe, expect, it } from 'vitest'
import { redactSecretsInMessage } from '../redact'
describe('redactSecretsInMessage', () => {
it('redacts a quoted TOML-style api_key assignment', () => {
expect(redactSecretsInMessage('unexpected character at line 3: api_key = "sk-ant-real-secret"')).toBe(
'unexpected character at line 3: api_key = "<redacted>"'
)
})
it('redacts a quoted JSON-style "apiKey" field', () => {
expect(redactSecretsInMessage('invalid JSONC near "apiKey": "sk-ant-real-secret"')).toBe(
'invalid JSONC near "apiKey": "<redacted>"'
)
})
it('redacts a bare dotenv-style token value', () => {
expect(redactSecretsInMessage('bad line: AUTH_TOKEN=sk-ant-real-secret')).toBe('bad line: AUTH_TOKEN="<redacted>"')
})
it('redacts secret and password variants', () => {
expect(redactSecretsInMessage('client_secret = "abc123"')).toBe('client_secret = "<redacted>"')
expect(redactSecretsInMessage('password: "hunter2"')).toBe('password: "<redacted>"')
})
it('leaves a message with no sensitive-looking keys unchanged', () => {
const message = 'unexpected character at line 3, column 5: expected "," or "}"'
expect(redactSecretsInMessage(message)).toBe(message)
})
it('fully redacts a multiline TOML triple-quoted secret', () => {
const message = 'unexpected character: api_key = """\nsk-ant-real-secret\nmore-secret-lines\n"""'
const result = redactSecretsInMessage(message)
expect(result).not.toContain('sk-ant-real-secret')
expect(result).not.toContain('more-secret-lines')
expect(result).toBe('unexpected character: api_key = "<redacted>"')
})
it('redacts a Bearer token instead of only stripping the word "Bearer"', () => {
const message = 'request failed: Authorization: Bearer sk-ant-real-secret'
const result = redactSecretsInMessage(message)
expect(result).not.toContain('sk-ant-real-secret')
})
})

View File

@@ -0,0 +1,14 @@
import { CodeCli } from '@shared/types/codeCli'
import { describe, expect, it } from 'vitest'
import { sanitizeCliConfigBlob } from '../sanitize'
describe('sanitizeCliConfigBlob (codex)', () => {
// `commonConfig` was a dead toggle (buildCodexConfig/parser.ts never read or wrote it) and was
// removed from the UI — pin it as filtered so a future field addition can't silently revive it.
it('drops the removed commonConfig field', () => {
const result = sanitizeCliConfigBlob(CodeCli.OPENAI_CODEX, { commonConfig: true, goalMode: true })
expect(result).not.toHaveProperty('commonConfig')
expect(result).toEqual({ goalMode: true })
})
})

View File

@@ -415,6 +415,43 @@ describe('writeCliConfigDraft', () => {
expect(parsed.model_providers['cherry-DeepSeek'].name).toBe('DeepSeek')
})
// Regression: Codex treats a model_providers[...].name of exactly "OpenAI" as a signal
// that remote compaction is on, regardless of the actual toggle — so a provider whose
// display name really is "OpenAI" must never be written verbatim unless that mode is on.
it('avoids the "OpenAI" name collision when the provider is actually named OpenAI (remote compaction off)', async () => {
const openaiNamedProvider = { ...codexProvider, name: 'OpenAI' } as unknown as Provider
mockGet({
'/providers/deepseek': () => openaiNamedProvider,
'/providers/deepseek/api-keys': () => ({ keys: [enabledKey] }),
'/models/': () => null
})
await writeCliConfigDraft({ cliTool: CodeCli.OPENAI_CODEX, modelId: 'deepseek::deepseek-chat' })
const { parse: parseToml } = await import('smol-toml')
const parsed = parseToml(findWrite('config.toml')!.content) as Record<string, any>
expect(parsed.model_providers['cherry-OpenAI'].name).toBe('OpenAI (Cherry)')
})
it('writes the literal "OpenAI" name when remote compaction is actually on', async () => {
const openaiNamedProvider = { ...codexProvider, name: 'OpenAI' } as unknown as Provider
mockGet({
'/providers/deepseek': () => openaiNamedProvider,
'/providers/deepseek/api-keys': () => ({ keys: [enabledKey] }),
'/models/': () => null
})
await writeCliConfigDraft({
cliTool: CodeCli.OPENAI_CODEX,
modelId: 'deepseek::deepseek-chat',
configBlob: { remoteCompaction: true }
})
const { parse: parseToml } = await import('smol-toml')
const parsed = parseToml(findWrite('config.toml')!.content) as Record<string, any>
expect(parsed.model_providers['cherry-OpenAI'].name).toBe('OpenAI')
})
it('uses the responses endpoint even when a chat-completions endpoint is also present', async () => {
const responsesProvider = {
...openaiCompatProvider,
@@ -636,6 +673,26 @@ describe('writeCliConfigDraft', () => {
expect(settings.tools).toBeUndefined()
expect(settings.advanced).toBeUndefined()
})
it('preserves a field Cherry has no UI for instead of silently deleting it', async () => {
// `general.preferredEditor` is MANAGED (clear.ts wipes it) but not WRITABLE
// (no UI control writes it), so a save must leave it untouched.
existing['/resolved~/.gemini/settings.json'] = JSON.stringify({ general: { preferredEditor: 'vim' } })
mockGet({
'/providers/gemini': () => geminiProvider,
'/providers/gemini/api-keys': () => ({ keys: [enabledKey] }),
'/models/': () => null
})
await writeCliConfigDraft({
cliTool: CodeCli.GEMINI_CLI,
modelId: 'gemini::gemini-2.5-pro',
configBlob: { general: { vimMode: true } }
})
const settings = JSON.parse(findWrite('settings.json').content)
expect(settings.general).toEqual({ vimMode: true, preferredEditor: 'vim' })
})
})
describe('qwen-code (~/.qwen/settings.json)', () => {
@@ -687,6 +744,26 @@ describe('writeCliConfigDraft', () => {
envKey: 'CHERRY_QWEN_API_KEY'
})
})
it('preserves a field Cherry has no UI for instead of silently deleting it', async () => {
// `context.fileName` is MANAGED (clear.ts wipes it) but not WRITABLE
// (no UI control writes it), so a save must leave it untouched.
existing['/resolved~/.qwen/settings.json'] = JSON.stringify({ context: { fileName: ['QWEN.md'] } })
mockGet({
'/providers/deepseek': () => openaiCompatProvider,
'/providers/deepseek/api-keys': () => ({ keys: [enabledKey] }),
'/models/': () => ({ id: 'deepseek-chat', name: 'DeepSeek Chat' })
})
await writeCliConfigDraft({
cliTool: CodeCli.QWEN_CODE,
modelId: 'deepseek::deepseek-chat',
configBlob: { general: { vimMode: true } }
})
const parsed = JSON.parse(written!.content)
expect(parsed.context).toEqual({ fileName: ['QWEN.md'] })
})
})
describe('kimi-code (~/.kimi-code/config.toml)', () => {
@@ -728,6 +805,27 @@ describe('writeCliConfigDraft', () => {
expect(parsed.models['cherry-DeepSeek'].max_context_size).toBe(65536)
})
it('preserves a field Cherry has no UI for instead of silently deleting it', async () => {
// `loop_control.*` is MANAGED (clear.ts wipes it) but not WRITABLE
// (no UI control writes it), so a save must leave it untouched.
existing['/resolved~/.kimi-code/config.toml'] = 'loop_control = { max_steps_per_turn = 12 }'
mockGet({
'/providers/deepseek': () => openaiCompatProvider,
'/providers/deepseek/api-keys': () => ({ keys: [enabledKey] }),
'/models/': () => ({ id: 'deepseek-chat', contextWindow: 65536 })
})
await writeCliConfigDraft({
cliTool: CodeCli.KIMI_CODE,
modelId: 'deepseek::deepseek-chat',
configBlob: { thinking: { enabled: true } }
})
const { parse: parseToml } = await import('smol-toml')
const parsed = parseToml(written!.content) as Record<string, any>
expect(parsed.loop_control).toEqual({ max_steps_per_turn: 12 })
})
it('does not write when parent directory creation fails', async () => {
mockGet({
'/providers/deepseek': () => openaiCompatProvider,
@@ -791,4 +889,132 @@ describe('writeCliConfigDraft', () => {
expect(writes).toEqual([])
})
})
describe('assertCliConfigCredentials via writeCliConfigDraft (never write an unauthenticated config)', () => {
it('rejects claude-code with no API key and writes nothing', async () => {
mockGet({
'/providers/anthropic': () => anthropicProvider,
'/providers/anthropic/api-keys': () => ({ keys: [] }),
'/models/': () => null
})
await expect(
writeCliConfigDraft({ cliTool: CodeCli.CLAUDE_CODE, modelId: 'anthropic::claude-sonnet-4-5' })
).rejects.toThrow(/missing the API key/)
expect(writes).toEqual([])
})
it('rejects codex with no API key and writes nothing', async () => {
mockGet({
'/providers/deepseek': () => codexProvider,
'/providers/deepseek/api-keys': () => ({ keys: [] }),
'/models/': () => null
})
await expect(
writeCliConfigDraft({ cliTool: CodeCli.OPENAI_CODEX, modelId: 'deepseek::deepseek-chat' })
).rejects.toThrow(/missing the API key/)
expect(writes).toEqual([])
})
it('rejects opencode with no API key and writes nothing', async () => {
mockGet({
'/providers/deepseek': () => openaiCompatProvider,
'/providers/deepseek/api-keys': () => ({ keys: [] }),
'/models/': () => ({ id: 'deepseek-chat', contextWindow: 65536 })
})
await expect(
writeCliConfigDraft({ cliTool: CodeCli.OPEN_CODE, modelId: 'deepseek::deepseek-chat' })
).rejects.toThrow(/missing required fields/)
expect(writes).toEqual([])
})
it('rejects gemini-cli with no API key and writes nothing', async () => {
mockGet({
'/providers/gemini': () => geminiProvider,
'/providers/gemini/api-keys': () => ({ keys: [] }),
'/models/': () => null
})
await expect(
writeCliConfigDraft({ cliTool: CodeCli.GEMINI_CLI, modelId: 'gemini::gemini-2.5-pro' })
).rejects.toThrow(/missing the API key/)
expect(writes).toEqual([])
})
it('rejects qwen-code with no API key and writes nothing', async () => {
mockGet({
'/providers/deepseek': () => openaiCompatProvider,
'/providers/deepseek/api-keys': () => ({ keys: [] }),
'/models/': () => ({ id: 'deepseek-chat', name: 'DeepSeek Chat' })
})
await expect(
writeCliConfigDraft({ cliTool: CodeCli.QWEN_CODE, modelId: 'deepseek::deepseek-chat' })
).rejects.toThrow(/missing the API key/)
expect(writes).toEqual([])
})
it('rejects kimi-code with no API key and writes nothing', async () => {
mockGet({
'/providers/deepseek': () => openaiCompatProvider,
'/providers/deepseek/api-keys': () => ({ keys: [] }),
'/models/': () => ({ id: 'deepseek-chat', contextWindow: 65536 })
})
await expect(
writeCliConfigDraft({ cliTool: CodeCli.KIMI_CODE, modelId: 'deepseek::deepseek-chat' })
).rejects.toThrow(/missing the API key/)
expect(writes).toEqual([])
})
})
describe('read-error safety (a real read failure must not be treated as "file missing")', () => {
it('aborts instead of treating a permission-denied read as an empty/new file', async () => {
// The file exists, but reading it fails transiently (e.g. EACCES/EBUSY).
// Before the fix this was swallowed and treated as "file doesn't exist
// yet", which would silently wipe every unmanaged key from the real file.
existing['/resolved~/.claude/settings.json'] = JSON.stringify({ hooks: { foo: 'bar' } })
vi.mocked(window.api.file.readExternal).mockImplementationOnce(async () => {
throw new Error('EACCES: permission denied')
})
mockGet({
'/providers/anthropic': () => anthropicProvider,
'/providers/anthropic/api-keys': () => ({ keys: [enabledKey] }),
'/models/': () => null
})
await expect(
writeCliConfigDraft({ cliTool: CodeCli.CLAUDE_CODE, modelId: 'anthropic::claude-sonnet-4-5' })
).rejects.toThrow(/EACCES/)
// Nothing was written — the real file (and its unmanaged keys) is untouched.
expect(writes).toEqual([])
})
it('does not delete a real config file during rollback when its snapshot read fails', async () => {
// Drive the write with explicit `files` so the snapshot/rollback path is
// exercised directly, independent of the build-time read (covered above).
// codex writes config.toml then auth.json; a real read error while
// snapshotting config.toml must abort before either file is touched, not
// be recorded as "didn't exist" and later trash-deleted during rollback.
const files = [
{
target: 'codex-config' as const,
label: 'Codex config',
path: '/resolved~/.codex/config.toml',
language: 'toml' as const,
content: 'model = "new-model"'
},
{
target: 'codex-auth' as const,
label: 'Codex auth',
path: '/resolved~/.codex/auth.json',
language: 'json' as const,
content: '{"OPENAI_API_KEY":"sk-secret"}'
}
]
vi.mocked(window.api.file.readExternal).mockImplementationOnce(async () => {
throw new Error('EBUSY: resource busy or locked')
})
await expect(writeCliConfigDraft({ cliTool: CodeCli.OPENAI_CODEX, files })).rejects.toThrow(/EBUSY/)
expect(writes).toEqual([])
expect(window.api.file.deleteExternalFile).not.toHaveBeenCalled()
})
})
})

View File

@@ -1,15 +1,15 @@
import { CHERRY_PROVIDER_PREFIX, OPENCODE_SCHEMA } from './constants'
import {
applyManagedJsonSettings,
applyManagedTomlSettings,
applyWritableTomlSettings,
CLAUDE_MANAGED_ENV_KEYS,
CLAUDE_MANAGED_PERMISSION_KEYS,
CLAUDE_MANAGED_TOP_LEVEL_KEYS,
CODEX_MANAGED_TOP_LEVEL_KEYS,
GEMINI_MANAGED_ENV_KEYS,
GEMINI_MANAGED_SETTINGS_KEYS,
GEMINI_WRITABLE_SETTINGS_KEYS,
OPEN_CODE_MANAGED_TOP_LEVEL_KEYS,
QWEN_MANAGED_SETTINGS_KEYS
QWEN_WRITABLE_SETTINGS_KEYS
} from './managedKeys'
import {
codexPermissionModeToConfig,
@@ -71,6 +71,18 @@ export function buildClaudeConfig(
return merged
}
/**
* Codex's own CLI has special-cased behavior when a model_providers[...].name is
* exactly "OpenAI" (reserved for the remote-compaction mode). When the user's
* actual provider display name is literally "OpenAI" but remote compaction is
* off, we must not emit the same literal or the config readback (parser.ts)
* would misinterpret it as remoteCompaction being on.
*/
function resolveCodexProviderDisplayName(providerName: string, remoteCompaction: boolean): string {
if (remoteCompaction) return 'OpenAI'
return providerName === 'OpenAI' ? 'OpenAI (Cherry)' : providerName
}
export function buildCodexConfig(
existingToml: Record<string, any>,
resolved: { baseUrl: string; providerName: string; model: string },
@@ -93,7 +105,7 @@ export function buildCodexConfig(
model_providers: {
...preservedProviders,
[providerKey]: {
name: options.remoteCompaction === true ? 'OpenAI' : resolved.providerName,
name: resolveCodexProviderDisplayName(resolved.providerName, options.remoteCompaction === true),
base_url: normalizeUrl(resolved.baseUrl),
wire_api: 'responses',
requires_openai_auth: true
@@ -198,7 +210,7 @@ export function buildGeminiSettingsConfig(
configBlob: Record<string, any>
): Record<string, any> {
const next = { ...settings }
applyManagedJsonSettings(next, sanitizeGeminiConfigBlob(configBlob), GEMINI_MANAGED_SETTINGS_KEYS)
applyManagedJsonSettings(next, sanitizeGeminiConfigBlob(configBlob), GEMINI_WRITABLE_SETTINGS_KEYS)
next.model = { ...asRecord(next.model), name: resolved.model }
const security = asRecord(next.security)
next.security = { ...security, auth: { ...asRecord(security.auth), selectedType: 'gemini-api-key' } }
@@ -229,7 +241,7 @@ export function buildQwenConfig(
},
model: { name: resolved.model }
}
applyManagedJsonSettings(merged, sanitizedConfigBlob, QWEN_MANAGED_SETTINGS_KEYS)
applyManagedJsonSettings(merged, sanitizedConfigBlob, QWEN_WRITABLE_SETTINGS_KEYS)
return merged
}
@@ -251,6 +263,6 @@ export function buildKimiConfig(
modelsTable[resolved.modelKey] = modelConfig
const merged = { ...existing, default_model: resolved.modelKey, providers: providerTable, models: modelsTable }
applyManagedTomlSettings(merged, sanitizedConfigBlob)
applyWritableTomlSettings(merged, sanitizedConfigBlob)
return merged
}

View File

@@ -2,12 +2,11 @@ import { CodeCli } from '@shared/types/codeCli'
import { stringify as stringifyToml } from 'smol-toml'
import { CHERRY_PROVIDER_PREFIX } from './constants'
import { parseDotenv } from './dotenv'
import { parseDotenv, renderDotenvFile } from './dotenv'
import {
readExternalOrNull,
readValidatedJsonOrNull,
readValidatedTomlOrNull,
renderDotenvFile,
renderJsonFile,
resolveAbs
} from './file'
@@ -71,7 +70,7 @@ export async function clearCliConfig(args: ClearCliConfigArgs): Promise<void> {
for (const key of CLAUDE_MANAGED_ENV_KEYS) delete env[key]
next.env = env
}
await window.api.file.write(absPath, renderJsonFile(next))
await window.api.file.write(absPath, renderJsonFile(next), 0o600)
return
}
case CodeCli.OPENAI_CODEX: {
@@ -90,12 +89,12 @@ export async function clearCliConfig(args: ClearCliConfigArgs): Promise<void> {
next.model_providers = omitKeysByPrefix(next.model_providers as Record<string, any>, CHERRY_PROVIDER_PREFIX)
}
dropFeatureGoalsIfEmpty(next)
await window.api.file.write(absPath, stringifyToml(next))
await window.api.file.write(absPath, stringifyToml(next), 0o600)
}
if (existingAuth?.OPENAI_API_KEY !== undefined) {
const nextAuth = { ...existingAuth }
delete nextAuth.OPENAI_API_KEY
await window.api.file.write(authAbsPath, renderJsonFile(nextAuth))
await window.api.file.write(authAbsPath, renderJsonFile(nextAuth), 0o600)
}
return
}
@@ -108,7 +107,7 @@ export async function clearCliConfig(args: ClearCliConfigArgs): Promise<void> {
if (next.provider && typeof next.provider === 'object') {
next.provider = omitKeysByPrefix(next.provider as Record<string, any>, CHERRY_PROVIDER_PREFIX)
}
await window.api.file.write(absPath, renderJsonFile(next))
await window.api.file.write(absPath, renderJsonFile(next), 0o600)
return
}
case CodeCli.GEMINI_CLI: {
@@ -117,7 +116,7 @@ export async function clearCliConfig(args: ClearCliConfigArgs): Promise<void> {
if (envText !== null) {
const envMap = parseDotenv(envText)
for (const key of GEMINI_MANAGED_ENV_KEYS) envMap.delete(key)
await window.api.file.write(envAbsPath, renderDotenvFile(envMap))
await window.api.file.write(envAbsPath, renderDotenvFile(envMap), 0o600)
}
const settingsAbsPath = await resolveAbs(GEMINI_SETTINGS_PATH)
@@ -129,7 +128,7 @@ export async function clearCliConfig(args: ClearCliConfigArgs): Promise<void> {
delete settings.model.name
if (Object.keys(settings.model as Record<string, any>).length === 0) delete settings.model
}
await window.api.file.write(settingsAbsPath, renderJsonFile(settings))
await window.api.file.write(settingsAbsPath, renderJsonFile(settings), 0o600)
return
}
case CodeCli.QWEN_CODE: {
@@ -147,7 +146,7 @@ export async function clearCliConfig(args: ClearCliConfigArgs): Promise<void> {
applyManagedJsonSettings(next, {}, QWEN_MANAGED_SETTINGS_KEYS)
dropSecurityAuthSelectedTypeIfEmpty(next)
delete next.model
await window.api.file.write(absPath, renderJsonFile(next))
await window.api.file.write(absPath, renderJsonFile(next), 0o600)
return
}
case CodeCli.KIMI_CODE: {
@@ -162,7 +161,7 @@ export async function clearCliConfig(args: ClearCliConfigArgs): Promise<void> {
}
applyManagedTomlSettings(next, {})
delete next.default_model
await window.api.file.write(absPath, stringifyToml(next))
await window.api.file.write(absPath, stringifyToml(next), 0o600)
return
}
default:

View File

@@ -8,10 +8,29 @@ export function parseDotenv(content: string): Map<string, string> {
if (eq === -1) continue
const key = line.slice(0, eq).trim()
let value = line.slice(eq + 1).trim()
if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith("'") && value.endsWith("'"))) {
if (value.startsWith('"') && value.endsWith('"')) {
value = value.slice(1, -1).replace(/\\(["\\])/g, '$1')
} else if (value.startsWith("'") && value.endsWith("'")) {
value = value.slice(1, -1)
}
out.set(key, value)
}
return out
}
// Bare (unquoted) dotenv values are truncated by standard dotenv parsers at the
// first `#`, and lose leading/trailing whitespace — quote whenever that would
// otherwise corrupt the value on read-back by the CLI tool's own loader.
function needsDotenvQuoting(value: string): boolean {
return value === '' || /^\s|\s$|["'#\\]/.test(value)
}
function quoteDotenvValue(value: string): string {
return `"${value.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"`
}
export function renderDotenvFile(envMap: Map<string, string>): string {
return `${[...envMap.entries()]
.map(([key, value]) => `${key}=${needsDotenvQuoting(value) ? quoteDotenvValue(value) : value}`)
.join('\n')}\n`
}

View File

@@ -18,12 +18,12 @@ import {
buildQwenConfig
} from './builders'
import { CHERRY_PROVIDER_PREFIX } from './constants'
import { parseDotenv } from './dotenv'
import { parseDotenv, renderDotenvFile } from './dotenv'
import { makeDraftFile, readAndParseDraftFile, readDraftFileText, validateCliConfigDraftForWrite } from './draftFiles'
import {
parseJsonOrThrow,
parseTomlOrThrow,
renderDotenvFile,
readExternalOrNull,
renderJsonFile,
resolveAbs,
writeExternalConfigFile
@@ -70,19 +70,8 @@ interface FileSnapshot {
}
async function snapshotFile(path: string): Promise<FileSnapshot> {
try {
return {
path,
existed: true,
previousContent: await window.api.file.readExternal(path)
}
} catch {
return {
path,
existed: false,
previousContent: ''
}
}
const previousContent = await readExternalOrNull(path)
return { path, existed: previousContent !== null, previousContent: previousContent ?? '' }
}
interface ResolvedCliConfigContext {
@@ -264,6 +253,9 @@ async function buildCliConfigDraftFiles(
function assertCliConfigCredentials(cliTool: string, context: ResolvedCliConfigContext): void {
const { provider, apiKey, modelRecord } = context
switch (cliTool) {
case CodeCli.CLAUDE_CODE:
if (!apiKey) throw new Error('Claude Code config is missing the API key')
return
case CodeCli.OPENAI_CODEX:
if (!apiKey) throw new Error('Codex config is missing the API key')
return
@@ -335,9 +327,13 @@ export async function writeCliConfigDraft(args: {
for (const snapshot of snapshots.slice().reverse()) {
rollbackQueue = rollbackQueue.then(async () => {
if (snapshot.existed) {
await writeExternalConfigFile(snapshot.path, snapshot.previousContent)
await writeExternalConfigFile(snapshot.path, snapshot.previousContent).catch((rollbackError) => {
logger.error(`Failed to roll back ${snapshot.path} after write failure`, rollbackError as Error)
})
} else {
await window.api.file.deleteExternalFile(snapshot.path).catch(() => undefined)
await window.api.file.deleteExternalFile(snapshot.path).catch((rollbackError) => {
logger.error(`Failed to delete ${snapshot.path} during rollback`, rollbackError as Error)
})
}
})
}

View File

@@ -1,5 +1,6 @@
import { parseDotenv } from './dotenv'
import { parseJsonOrThrow, parseTomlOrThrow, readExternal, resolveAbs } from './file'
import { redactSecretsInMessage } from './redact'
import { CLI_CONFIG_FILE_SPECS } from './targets'
import type { CliConfigFileDraft, CliConfigTarget } from './types'
@@ -40,7 +41,8 @@ export async function readAndParseDraftFile<T>(
} catch (err) {
const spec = CLI_CONFIG_FILE_SPECS[target]
const path = getDraftFile(files, target)?.path ?? (await resolveAbs(spec.path))
throw new Error(`Failed to parse ${spec.label} at ${path}: ${err instanceof Error ? err.message : String(err)}`)
const rawMessage = err instanceof Error ? err.message : String(err)
throw new Error(`Failed to parse ${spec.label} at ${path}: ${redactSecretsInMessage(rawMessage)}`)
}
}

View File

@@ -12,9 +12,9 @@ import {
buildQwenConfig
} from './builders'
import { CHERRY_PROVIDER_PREFIX } from './constants'
import { parseDotenv } from './dotenv'
import { parseDotenv, renderDotenvFile } from './dotenv'
import { getDraftFile } from './draftFiles'
import { parseJsonOrThrow, parseTomlOrThrow, renderDotenvFile, renderJsonFile } from './file'
import { parseJsonOrThrow, parseTomlOrThrow, renderJsonFile } from './file'
import { extractConnectionFromCliConfigDraft } from './parser'
import { openCodeNpmInfoFromNpmPackage } from './resolvers'
import { sanitizeCliConfigBlob } from './sanitize'

View File

@@ -1,6 +1,8 @@
import { parse as parseJsonc, type ParseError } from 'jsonc-parser'
import { parse as parseToml } from 'smol-toml'
import { redactSecretsInMessage } from './redact'
/** Resolve `~`/relative paths to absolute (renderer cannot call application.getPath). */
export async function resolveAbs(p: string): Promise<string> {
return window.api.resolvePath(p)
@@ -21,16 +23,7 @@ export async function writeExternalConfigFile(absPath: string, content: Uint8Arr
if (parentDir) {
await window.api.file.mkdir(parentDir)
}
await window.api.file.write(absPath, content)
}
/** Read an external file as text; returns '' when missing or unreadable. */
export async function readExternal(absPath: string): Promise<string> {
try {
return await window.api.file.readExternal(absPath)
} catch {
return ''
}
await window.api.file.write(absPath, content, 0o600)
}
function isMissingFileError(error: unknown): boolean {
@@ -48,11 +41,17 @@ export async function readExternalOrNull(absPath: string): Promise<string | null
}
}
/** Read an external file as text; returns '' when missing, throws on other read errors. */
export async function readExternal(absPath: string): Promise<string> {
return (await readExternalOrNull(absPath)) ?? ''
}
function parseOrThrow<T>(content: string, label: string, absPath: string, parseFn: (content: string) => T): T {
try {
return parseFn(content)
} catch (err) {
throw new Error(`Failed to parse ${label} at ${absPath}: ${err instanceof Error ? err.message : String(err)}`)
const rawMessage = err instanceof Error ? err.message : String(err)
throw new Error(`Failed to parse ${label} at ${absPath}: ${redactSecretsInMessage(rawMessage)}`)
}
}
@@ -99,7 +98,3 @@ export function parseJsonOrThrow(content: string): Record<string, any> {
export function renderJsonFile(value: Record<string, any>): string {
return `${JSON.stringify(value, null, 2)}\n`
}
export function renderDotenvFile(envMap: Map<string, string>): string {
return `${[...envMap.entries()].map(([key, value]) => `${key}=${value}`).join('\n')}\n`
}

View File

@@ -141,3 +141,8 @@ export function applyManagedJsonSettings(
export function applyManagedTomlSettings(target: Record<string, any>, source: Record<string, any>): void {
applyManagedJsonSettings(target, source, KIMI_MANAGED_SECTION_KEYS, KIMI_MANAGED_TOP_LEVEL_KEYS)
}
/** Kimi's TOML config build path: only clears/restores the WRITABLE subset (unlike clear.ts's full wipe). */
export function applyWritableTomlSettings(target: Record<string, any>, source: Record<string, any>): void {
applyManagedJsonSettings(target, source, KIMI_WRITABLE_SECTION_KEYS, KIMI_WRITABLE_TOP_LEVEL_KEYS)
}

View File

@@ -18,6 +18,18 @@ import { asRecord, findCherryProviderKey, isCherryManagedModel, stringValue } fr
export function extractConnectionFromCliConfigDraft(
cliTool: string,
files: CliConfigFileDraft[]
): CliConfigConnection | null {
const connection = extractConnectionFromCliConfigDraftInternal(cliTool, files)
if (!connection) return null
// An existing-but-empty config file (e.g. `{}`) parses to an all-undefined connection object,
// which is truthy — callers doing `if (!connection)` would otherwise misread it as a real,
// non-matching foreign connection instead of "no connection info here".
return connection.baseUrl || connection.apiKey || connection.model ? connection : null
}
function extractConnectionFromCliConfigDraftInternal(
cliTool: string,
files: CliConfigFileDraft[]
): CliConfigConnection | null {
try {
switch (cliTool) {

View File

@@ -0,0 +1,14 @@
const BEARER_TOKEN_PATTERN = /\bBearer\s+\S+/gi
// Triple-quoted alternatives must come before the single-quote-pair alternative — otherwise that
// alternative matches only up to the first quote inside the triple-quoted block, leaving the rest
// of a multiline TOML secret unredacted.
const SENSITIVE_MESSAGE_PATTERN =
/(["']?(?:api[_-]?key|token|secret|password|auth)\w*["']?\s*[:=]\s*)("""[\s\S]*?"""|'''[\s\S]*?'''|["'][^"']*["']|\S+)/gi
/** Redact likely-sensitive key=value / "key": value / Bearer-token fragments embedded in a raw parser error message. */
export function redactSecretsInMessage(message: string): string {
// Bearer must be redacted before the key=value pass, which would otherwise consume the literal
// word "Bearer" as the "value" for a preceding "Authorization:" key and leave the real token intact.
return message.replace(BEARER_TOKEN_PATTERN, 'Bearer <redacted>').replace(SENSITIVE_MESSAGE_PATTERN, '$1"<redacted>"')
}

View File

@@ -53,7 +53,7 @@ export function sanitizeClaudeConfigBlob(configBlob: Record<string, unknown> | u
export function sanitizeCodexConfigBlob(configBlob: Record<string, unknown> | undefined): Record<string, any> {
const blob = asRecord(configBlob)
const next = pickTopLevel(blob, ['goalMode', 'remoteCompaction', 'commonConfig', 'disableResponseStorage'])
const next = pickTopLevel(blob, ['goalMode', 'remoteCompaction', 'disableResponseStorage'])
if (isCodexPermissionMode(blob.permissionMode)) next.permissionMode = blob.permissionMode
if (isCodexReasoningEffort(blob.reasoningEffort)) next.reasoningEffort = blob.reasoningEffort
return next

View File

@@ -0,0 +1,118 @@
import type { CliConfigConnection, CliConfigFileDraft } from '@renderer/pages/code/cliConfig'
import type { ApiKeyEntry, Provider } from '@shared/data/types/provider'
import { CodeCli } from '@shared/types/codeCli'
import { act, renderHook } from '@testing-library/react'
import { beforeEach, describe, expect, it, vi } from 'vitest'
const mocks = vi.hoisted(() => ({
extractConnectionFromCliConfigDraft: vi.fn(),
readCliConfigFiles: vi.fn()
}))
vi.mock('@renderer/pages/code/cliConfig', async (importOriginal) => {
// oxlint-disable-next-line consistent-type-imports
const actual = await importOriginal<typeof import('@renderer/pages/code/cliConfig')>()
return {
...actual,
extractConnectionFromCliConfigDraft: mocks.extractConnectionFromCliConfigDraft,
readCliConfigFiles: mocks.readCliConfigFiles,
readCliConfigDraft: vi.fn().mockResolvedValue([]),
updateCliConfigDraftConfig: vi.fn(),
validateCliConfigDraftForWrite: vi.fn()
}
})
const { useConfigDraftController } = await import('../useConfigDraftController')
const codexProvider = {
id: 'deepseek',
name: 'DeepSeek',
endpointConfigs: { 'openai-responses': { baseUrl: 'https://api.deepseek.com/v1' } },
defaultChatEndpoint: 'openai-responses'
} as unknown as Provider
const rawFiles: CliConfigFileDraft[] = [
{ target: 'codex-config', label: 'Codex config', path: '/home/.codex/config.toml', language: 'toml', content: '' }
]
// A connection whose baseUrl/model match the enabled provider but whose apiKey does not —
// the exact shape `connectionMatchesProvider` must reject once `apiKeys` has actually resolved.
const foreignConnection: CliConfigConnection = {
baseUrl: 'https://api.deepseek.com/v1',
apiKey: 'sk-foreign',
model: 'deepseek-chat'
}
function renderController(apiKeys: ApiKeyEntry[] | undefined) {
return renderHook(
(props: { apiKeys: ApiKeyEntry[] | undefined }) =>
useConfigDraftController({
cliTool: CodeCli.OPENAI_CODEX,
provider: codexProvider,
providerConfig: { modelId: 'deepseek::deepseek-chat' },
isCurrentProvider: true,
apiKeys: props.apiKeys,
onSubmit: vi.fn(),
onClose: vi.fn()
}),
{ initialProps: { apiKeys } }
)
}
describe('useConfigDraftController (initial load vs. apiKeys race)', () => {
beforeEach(() => {
vi.clearAllMocks()
mocks.readCliConfigFiles.mockResolvedValue(rawFiles)
mocks.extractConnectionFromCliConfigDraft.mockReturnValue(foreignConnection)
})
it('does not judge managed/foreign until the apiKeys query resolves', async () => {
const { result } = renderController(undefined)
// Flush any pending microtasks; the load effect must not have fired at all.
await act(async () => {
await Promise.resolve()
})
expect(mocks.readCliConfigFiles).not.toHaveBeenCalled()
expect(result.current.draft.mode).toBe('managed')
})
it('judges foreign correctly once apiKeys resolves to a non-matching set', async () => {
const { result, rerender } = renderController(undefined)
await act(async () => {
await Promise.resolve()
})
expect(mocks.readCliConfigFiles).not.toHaveBeenCalled()
rerender({ apiKeys: [{ id: 'k1', key: 'sk-real', isEnabled: true }] })
await act(async () => {
await Promise.resolve()
await Promise.resolve()
})
expect(mocks.readCliConfigFiles).toHaveBeenCalledTimes(1)
expect(result.current.draft.mode).toBe('foreign')
})
it('does not re-run the initial load when apiKeys changes reference after it already ran', async () => {
const { rerender } = renderController([{ id: 'k1', key: 'sk-real', isEnabled: true }])
await act(async () => {
await Promise.resolve()
await Promise.resolve()
})
expect(mocks.readCliConfigFiles).toHaveBeenCalledTimes(1)
// New array reference, same content — must not trigger a second load.
rerender({ apiKeys: [{ id: 'k1', key: 'sk-real', isEnabled: true }] })
await act(async () => {
await Promise.resolve()
})
expect(mocks.readCliConfigFiles).toHaveBeenCalledTimes(1)
})
})

View File

@@ -13,7 +13,7 @@ export interface CodexConfigFieldsProps {
section?: 'all' | 'basic' | 'advanced'
}
type CodexFlag = 'goalMode' | 'remoteCompaction' | 'commonConfig' | 'disableResponseStorage'
type CodexFlag = 'goalMode' | 'remoteCompaction' | 'disableResponseStorage'
const PERMISSION_MODE_LABEL_KEYS: Record<(typeof CODEX_PERMISSION_MODES)[number], string> = {
readOnly: 'code.adv.permission_modes.read_only',
@@ -35,7 +35,6 @@ export const CodexConfigFields: FC<CodexConfigFieldsProps> = ({ config, onChange
const goalMode = config.goalMode === true
const remoteCompaction = config.remoteCompaction === true
const commonConfig = config.commonConfig === true
const disableResponseStorage = config.disableResponseStorage === true
const toggle = useCallback(
@@ -93,11 +92,6 @@ export const CodexConfigFields: FC<CodexConfigFieldsProps> = ({ config, onChange
active={disableResponseStorage}
onClick={() => toggle('disableResponseStorage', !disableResponseStorage)}
/>
<TogglePill
label={t('code.adv.codex.common_config')}
active={commonConfig}
onClick={() => toggle('commonConfig', !commonConfig)}
/>
</div>
</div>
)

View File

@@ -90,7 +90,6 @@ describe('CLI config provider fields', () => {
expect(screen.getByText('code.adv.codex.goal_mode')).toBeInTheDocument()
expect(screen.getByText('code.adv.codex.remote_compaction')).toBeInTheDocument()
expect(screen.getByText('code.adv.codex.disable_response_storage')).toBeInTheDocument()
expect(screen.getByText('code.adv.codex.common_config')).toBeInTheDocument()
expect(screen.getByText('code.adv.permission_mode')).toBeInTheDocument()
expect(screen.getByText('code.adv.permission_modes.full_access_high_risk')).toBeInTheDocument()
expect(screen.getByText('code.adv.reasoning_effort')).toBeInTheDocument()

View File

@@ -69,6 +69,7 @@ export function useConfigDraftController({
const initialClaudeModelModeRef = useRef<ClaudeModelMode>(initialClaudeModelMode)
const loadIdRef = useRef(0)
const apiKeysRef = useRef<Parameters<typeof cliConfigConnectionMatchesProvider>[3]>(undefined)
const initialLoadHasRunRef = useRef(false)
if (initialDraftSnapshotRef.current === undefined) {
initialDraftSnapshotRef.current = createDraftSnapshot(initialDraftSeed)
@@ -164,6 +165,10 @@ export function useConfigDraftController({
})
useEffect(() => {
if (initialLoadHasRunRef.current) return
if (apiKeys === undefined) return // wait for the apiKeys query to resolve (even to an empty array) before judging managed/foreign
initialLoadHasRunRef.current = true
const {
isCurrentProvider,
cliTool,
@@ -194,7 +199,7 @@ export function useConfigDraftController({
if (loadId !== loadIdRef.current) return
commitLoadedDraft(nextDraft)
})
}, [])
}, [apiKeys])
/* oxlint-enable react-doctor/no-pass-data-to-parent */
const canSubmit = isForeignDraft ? draft.files.length > 0 && !draft.error : !draft.error

View File

@@ -0,0 +1,91 @@
import type { Provider } from '@shared/data/types/provider'
import { CodeCli } from '@shared/types/codeCli'
import { act, renderHook } from '@testing-library/react'
import { beforeEach, describe, expect, it, vi } from 'vitest'
const mocks = vi.hoisted(() => ({
gatewayPort: undefined as number | undefined,
requestMock: vi.fn()
}))
vi.mock('@data/hooks/usePreference', () => ({
usePreference: () => [mocks.gatewayPort, vi.fn()]
}))
vi.mock('@renderer/hooks/useMiniAppPopup', () => ({
useMiniAppPopup: () => ({ openSmartMiniApp: vi.fn() })
}))
vi.mock('@renderer/ipc', () => ({
ipcApi: { request: mocks.requestMock }
}))
vi.mock('@renderer/services/LoggerService', () => ({
loggerService: {
withContext: () => ({ info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() })
}
}))
vi.mock('react-i18next', () => ({
useTranslation: () => ({ t: (key: string) => key })
}))
const { useOpenClawGatewayController } = await import('../useOpenClawGatewayController')
const enabledProvider = { id: 'anthropic', name: 'Anthropic' } as Provider
describe('useOpenClawGatewayController', () => {
beforeEach(() => {
vi.clearAllMocks()
mocks.gatewayPort = undefined
window.toast = { error: vi.fn() } as any
mocks.requestMock.mockImplementation((route: string) => {
if (route === 'openclaw.get_status') return Promise.resolve({ status: 'stopped' })
if (route === 'openclaw.sync_config') return Promise.resolve({ success: true })
if (route === 'openclaw.start_gateway') return Promise.resolve({ success: true })
if (route === 'openclaw.get_dashboard_url') return Promise.resolve('https://dashboard.local')
return Promise.resolve({ success: true })
})
})
// Regression: the standalone OpenClaw page used to read `feature.openclaw.gateway_port`
// and forward it to `start_gateway`; that wiring was dropped when the controller moved here,
// silently pinning every launch to the gateway's hardcoded default port.
it('forwards the configured gateway port to openclaw.start_gateway instead of undefined', async () => {
mocks.gatewayPort = 18888
const { result } = renderHook(() =>
useOpenClawGatewayController({
selectedCliTool: CodeCli.OPENCLAW,
enabledProvider,
currentProviderConfig: { modelId: 'anthropic::claude-sonnet-4-5' },
upsertProviderConfig: vi.fn(),
setCurrentProvider: vi.fn()
})
)
await act(async () => {
await result.current.onLaunch()
})
expect(mocks.requestMock).toHaveBeenCalledWith('openclaw.start_gateway', 18888)
})
it('passes undefined when no custom gateway port preference is set', async () => {
const { result } = renderHook(() =>
useOpenClawGatewayController({
selectedCliTool: CodeCli.OPENCLAW,
enabledProvider,
currentProviderConfig: { modelId: 'anthropic::claude-sonnet-4-5' },
upsertProviderConfig: vi.fn(),
setCurrentProvider: vi.fn()
})
)
await act(async () => {
await result.current.onLaunch()
})
expect(mocks.requestMock).toHaveBeenCalledWith('openclaw.start_gateway', undefined)
})
})

View File

@@ -1,3 +1,4 @@
import { usePreference } from '@data/hooks/usePreference'
import { useMiniAppPopup } from '@renderer/hooks/useMiniAppPopup'
import { ipcApi } from '@renderer/ipc'
import { loggerService } from '@renderer/services/LoggerService'
@@ -43,6 +44,7 @@ export function useOpenClawGatewayController({
}: UseOpenClawGatewayControllerOptions): OpenClawGatewayController {
const { t } = useTranslation()
const { openSmartMiniApp } = useMiniAppPopup()
const [gatewayPort] = usePreference('feature.openclaw.gateway_port')
const [status, setStatus] = useState<OpenClawGatewayStatus>('stopped')
const [launching, setLaunching] = useState(false)
const [stopping, setStopping] = useState(false)
@@ -88,7 +90,7 @@ export function useOpenClawGatewayController({
return
}
const startResult = await ipcApi.request('openclaw.start_gateway', undefined)
const startResult = await ipcApi.request('openclaw.start_gateway', gatewayPort)
if (!startResult.success) {
setStatus('error')
window.toast.error(startResult.message || t('code.launch.error'))
@@ -107,6 +109,7 @@ export function useOpenClawGatewayController({
}, [
currentProviderConfig,
enabledProvider,
gatewayPort,
openDashboard,
selectedCliTool,
setCurrentProvider,

View File

@@ -11,18 +11,6 @@ const gatewayStatusResultSchema = z.object({
message: z.string().optional()
})
const healthInfoSchema = z.object({
status: z.enum(['healthy', 'unhealthy']),
gatewayPort: z.number()
})
const channelInfoSchema = z.object({
id: z.string(),
name: z.string(),
type: z.string(),
status: z.enum(['connected', 'disconnected', 'error'])
})
// ── Request schemas ──
export const openclawRequestSchemas = {
'openclaw.start_gateway': defineRoute({
@@ -37,10 +25,6 @@ export const openclawRequestSchemas = {
input: z.void(),
output: z.object({ status: z.enum(['stopped', 'starting', 'running', 'error']), port: z.number() })
}),
'openclaw.check_health': defineRoute({
input: z.void(),
output: healthInfoSchema
}),
'openclaw.get_dashboard_url': defineRoute({
input: z.void(),
output: z.string()
@@ -48,22 +32,5 @@ export const openclawRequestSchemas = {
'openclaw.sync_config': defineRoute({
input: z.string(),
output: gatewayStatusResultSchema
}),
'openclaw.get_channels': defineRoute({
input: z.void(),
output: z.array(channelInfoSchema)
}),
'openclaw.check_update': defineRoute({
input: z.void(),
output: z.object({
hasUpdate: z.boolean(),
currentVersion: z.string().nullable(),
latestVersion: z.string().nullable(),
message: z.string().optional()
})
}),
'openclaw.perform_update': defineRoute({
input: z.void(),
output: gatewayStatusResultSchema
})
}