[codex] add locked file updates for config writes (#785)
* add locked file updates for config writes * add glm vision turbo preset
This commit is contained in:
ekko
@@ -0,0 +1,108 @@
|
||||
import { mkdir, mkdtemp, readFile, rm, writeFile } from 'fs/promises'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import YAML from 'js-yaml'
|
||||
|
||||
const { mockGatewayManager } = vi.hoisted(() => ({
|
||||
mockGatewayManager: {
|
||||
getActiveProfile: vi.fn(() => 'default'),
|
||||
stop: vi.fn().mockResolvedValue(undefined),
|
||||
start: vi.fn().mockResolvedValue(undefined),
|
||||
},
|
||||
}))
|
||||
|
||||
vi.mock('../../packages/server/src/services/gateway-bootstrap', () => ({
|
||||
getGatewayManagerInstance: () => mockGatewayManager,
|
||||
}))
|
||||
|
||||
const originalHermesHome = process.env.HERMES_HOME
|
||||
const tempHomes: string[] = []
|
||||
let hermesHome = ''
|
||||
|
||||
async function loadController() {
|
||||
vi.resetModules()
|
||||
process.env.HERMES_HOME = hermesHome
|
||||
return import('../../packages/server/src/controllers/hermes/config')
|
||||
}
|
||||
|
||||
function makeCtx(body: unknown): any {
|
||||
return { request: { body }, query: {}, status: 200, body: undefined }
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.clearAllMocks()
|
||||
hermesHome = await mkdtemp(join(tmpdir(), 'hermes-config-controller-'))
|
||||
tempHomes.push(hermesHome)
|
||||
await mkdir(hermesHome, { recursive: true })
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
vi.resetModules()
|
||||
if (originalHermesHome === undefined) delete process.env.HERMES_HOME
|
||||
else process.env.HERMES_HOME = originalHermesHome
|
||||
await Promise.all(tempHomes.splice(0).map(dir => rm(dir, { recursive: true, force: true })))
|
||||
hermesHome = ''
|
||||
})
|
||||
|
||||
describe('config controller locked file updates', () => {
|
||||
it('deep merges a config section and restarts platform gateways', async () => {
|
||||
await writeFile(join(hermesHome, 'config.yaml'), [
|
||||
'telegram:',
|
||||
' enabled: false',
|
||||
' extra:',
|
||||
' mode: old',
|
||||
'model:',
|
||||
' default: glm-5.1',
|
||||
'',
|
||||
].join('\n'), 'utf-8')
|
||||
const { updateConfig } = await loadController()
|
||||
const ctx = makeCtx({ section: 'telegram', values: { enabled: true, extra: { token_mode: 'env' } } })
|
||||
|
||||
await updateConfig(ctx)
|
||||
|
||||
expect(ctx.body).toEqual({ success: true })
|
||||
expect(mockGatewayManager.stop).toHaveBeenCalledWith('default')
|
||||
expect(mockGatewayManager.start).toHaveBeenCalledWith('default')
|
||||
const config = YAML.load(await readFile(join(hermesHome, 'config.yaml'), 'utf-8')) as any
|
||||
expect(config.telegram.enabled).toBe(true)
|
||||
expect(config.telegram.extra).toEqual({ mode: 'old', token_mode: 'env' })
|
||||
expect(config.model.default).toBe('glm-5.1')
|
||||
})
|
||||
|
||||
it('clears credential env values and removes matching config fields without losing unrelated env keys', async () => {
|
||||
await writeFile(join(hermesHome, 'config.yaml'), [
|
||||
'platforms:',
|
||||
' weixin:',
|
||||
' token: old-token',
|
||||
' extra:',
|
||||
' account_id: old-account',
|
||||
' base_url: https://old.example',
|
||||
'model:',
|
||||
' default: glm-5.1',
|
||||
'',
|
||||
].join('\n'), 'utf-8')
|
||||
await writeFile(join(hermesHome, '.env'), [
|
||||
'OPENROUTER_API_KEY=keep',
|
||||
'WEIXIN_TOKEN=old-token',
|
||||
'WEIXIN_ACCOUNT_ID=old-account',
|
||||
'',
|
||||
].join('\n'), 'utf-8')
|
||||
const { updateCredentials } = await loadController()
|
||||
const ctx = makeCtx({ platform: 'weixin', values: { token: '', extra: { account_id: '', base_url: 'https://new.example' } } })
|
||||
|
||||
await updateCredentials(ctx)
|
||||
|
||||
expect(ctx.body).toEqual({ success: true })
|
||||
const env = await readFile(join(hermesHome, '.env'), 'utf-8')
|
||||
expect(env).toContain('OPENROUTER_API_KEY=keep')
|
||||
expect(env).not.toContain('WEIXIN_TOKEN=')
|
||||
expect(env).not.toContain('WEIXIN_ACCOUNT_ID=')
|
||||
expect(env).toContain('WEIXIN_BASE_URL=https://new.example')
|
||||
const config = YAML.load(await readFile(join(hermesHome, 'config.yaml'), 'utf-8')) as any
|
||||
expect(config.platforms.weixin.token).toBeUndefined()
|
||||
expect(config.platforms.weixin.extra.account_id).toBeUndefined()
|
||||
expect(config.platforms.weixin.extra.base_url).toBe('https://old.example')
|
||||
expect(config.model.default).toBe('glm-5.1')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,82 @@
|
||||
import { mkdir, mkdtemp, readFile, rm, writeFile } from 'fs/promises'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import YAML from 'js-yaml'
|
||||
|
||||
const originalHermesHome = process.env.HERMES_HOME
|
||||
const tempHomes: string[] = []
|
||||
let hermesHome = ''
|
||||
|
||||
async function loadHelpers() {
|
||||
vi.resetModules()
|
||||
process.env.HERMES_HOME = hermesHome
|
||||
return import('../../packages/server/src/services/config-helpers')
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
hermesHome = await mkdtemp(join(tmpdir(), 'hermes-config-helpers-'))
|
||||
tempHomes.push(hermesHome)
|
||||
await mkdir(hermesHome, { recursive: true })
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
vi.resetModules()
|
||||
if (originalHermesHome === undefined) delete process.env.HERMES_HOME
|
||||
else process.env.HERMES_HOME = originalHermesHome
|
||||
await Promise.all(tempHomes.splice(0).map(dir => rm(dir, { recursive: true, force: true })))
|
||||
hermesHome = ''
|
||||
})
|
||||
|
||||
describe('config-helpers locked file updates', () => {
|
||||
it('merges concurrent config.yaml updates by re-reading under the file lock', async () => {
|
||||
await writeFile(join(hermesHome, 'config.yaml'), 'model:\n default: old\n', 'utf-8')
|
||||
const { updateConfigYaml } = await loadHelpers()
|
||||
|
||||
await Promise.all([
|
||||
updateConfigYaml(async (cfg) => {
|
||||
await new Promise(resolve => setTimeout(resolve, 25))
|
||||
cfg.model.default = 'glm-5.1'
|
||||
return cfg
|
||||
}),
|
||||
updateConfigYaml((cfg) => {
|
||||
cfg.platforms = cfg.platforms || {}
|
||||
cfg.platforms.api_server = { extra: { port: 8648 } }
|
||||
return cfg
|
||||
}),
|
||||
])
|
||||
|
||||
const config = YAML.load(await readFile(join(hermesHome, 'config.yaml'), 'utf-8')) as any
|
||||
expect(config.model.default).toBe('glm-5.1')
|
||||
expect(config.platforms.api_server.extra.port).toBe(8648)
|
||||
await expect(readFile(join(hermesHome, 'config.yaml.bak'), 'utf-8')).resolves.toContain('model:')
|
||||
})
|
||||
|
||||
it('serializes concurrent .env updates without losing keys', async () => {
|
||||
await writeFile(join(hermesHome, '.env'), 'OPENROUTER_API_KEY=keep\n', 'utf-8')
|
||||
const { saveEnvValue } = await loadHelpers()
|
||||
|
||||
await Promise.all([
|
||||
saveEnvValue('DEEPSEEK_API_KEY', 'deepseek'),
|
||||
saveEnvValue('MOONSHOT_API_KEY', 'moonshot'),
|
||||
])
|
||||
|
||||
const env = await readFile(join(hermesHome, '.env'), 'utf-8')
|
||||
expect(env).toContain('OPENROUTER_API_KEY=keep')
|
||||
expect(env).toContain('DEEPSEEK_API_KEY=deepseek')
|
||||
expect(env).toContain('MOONSHOT_API_KEY=moonshot')
|
||||
})
|
||||
|
||||
it('skips writing config.yaml when an updater returns write false', async () => {
|
||||
const configPath = join(hermesHome, 'config.yaml')
|
||||
await writeFile(configPath, 'model:\n default: old\n', 'utf-8')
|
||||
const before = await readFile(configPath, 'utf-8')
|
||||
const { updateConfigYaml } = await loadHelpers()
|
||||
|
||||
const result = await updateConfigYaml((cfg) => ({ data: cfg, result: 'unchanged', write: false }))
|
||||
|
||||
expect(result).toBe('unchanged')
|
||||
await expect(readFile(configPath, 'utf-8')).resolves.toBe(before)
|
||||
await expect(readFile(`${configPath}.bak`, 'utf-8')).rejects.toMatchObject({ code: 'ENOENT' })
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,96 @@
|
||||
import { mkdir, mkdtemp, readFile, rm, writeFile } from 'fs/promises'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import YAML from 'js-yaml'
|
||||
|
||||
vi.mock('../../packages/server/src/services/hermes/hermes-cli', () => ({
|
||||
pinSkill: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('../../packages/server/src/db/hermes/sessions-db', () => ({
|
||||
getSkillUsageStatsFromDb: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('../../packages/server/src/db', () => ({
|
||||
getDb: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('../../packages/server/src/db/hermes/schemas', () => ({
|
||||
MODEL_CONTEXT_TABLE: 'model_context',
|
||||
}))
|
||||
|
||||
const originalHermesHome = process.env.HERMES_HOME
|
||||
const tempHomes: string[] = []
|
||||
let hermesHome = ''
|
||||
|
||||
async function loadModelsController() {
|
||||
vi.resetModules()
|
||||
process.env.HERMES_HOME = hermesHome
|
||||
return import('../../packages/server/src/controllers/hermes/models')
|
||||
}
|
||||
|
||||
async function loadSkillsController() {
|
||||
vi.resetModules()
|
||||
process.env.HERMES_HOME = hermesHome
|
||||
return import('../../packages/server/src/controllers/hermes/skills')
|
||||
}
|
||||
|
||||
function makeCtx(body: unknown): any {
|
||||
return { request: { body }, status: 200, body: undefined, query: {}, params: {} }
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
hermesHome = await mkdtemp(join(tmpdir(), 'hermes-config-controller-'))
|
||||
tempHomes.push(hermesHome)
|
||||
await mkdir(hermesHome, { recursive: true })
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
vi.resetModules()
|
||||
if (originalHermesHome === undefined) delete process.env.HERMES_HOME
|
||||
else process.env.HERMES_HOME = originalHermesHome
|
||||
await Promise.all(tempHomes.splice(0).map(dir => rm(dir, { recursive: true, force: true })))
|
||||
hermesHome = ''
|
||||
})
|
||||
|
||||
describe('config mutating controllers', () => {
|
||||
it('setConfigModel updates only the model section and preserves existing config', async () => {
|
||||
await writeFile(join(hermesHome, 'config.yaml'), [
|
||||
'terminal:',
|
||||
' backend: local',
|
||||
'model:',
|
||||
' default: old',
|
||||
' provider: old-provider',
|
||||
'',
|
||||
].join('\n'), 'utf-8')
|
||||
const { setConfigModel } = await loadModelsController()
|
||||
const ctx = makeCtx({ default: 'glm-5.1', provider: 'custom:glm' })
|
||||
|
||||
await setConfigModel(ctx)
|
||||
|
||||
expect(ctx.body).toEqual({ success: true })
|
||||
const config = YAML.load(await readFile(join(hermesHome, 'config.yaml'), 'utf-8')) as any
|
||||
expect(config.model).toEqual({ default: 'glm-5.1', provider: 'custom:glm' })
|
||||
expect(config.terminal.backend).toBe('local')
|
||||
})
|
||||
|
||||
it('skill toggle preserves unrelated config while adding and removing disabled skills', async () => {
|
||||
await writeFile(join(hermesHome, 'config.yaml'), [
|
||||
'model:',
|
||||
' default: glm-5.1',
|
||||
'skills:',
|
||||
' disabled:',
|
||||
' - old-skill',
|
||||
'',
|
||||
].join('\n'), 'utf-8')
|
||||
const { toggle } = await loadSkillsController()
|
||||
|
||||
await toggle(makeCtx({ name: 'new-skill', enabled: false }))
|
||||
await toggle(makeCtx({ name: 'old-skill', enabled: true }))
|
||||
|
||||
const config = YAML.load(await readFile(join(hermesHome, 'config.yaml'), 'utf-8')) as any
|
||||
expect(config.model.default).toBe('glm-5.1')
|
||||
expect(config.skills.disabled).toEqual(['new-skill'])
|
||||
})
|
||||
})
|
||||
@@ -5,13 +5,14 @@ vi.mock('os', async () => {
|
||||
return { ...actual, homedir: () => '/fake/home' }
|
||||
})
|
||||
|
||||
const { mockReadFile, mockWriteFile, mockMkdir, mockSaveEnvValue, mockReadConfigYaml, mockWriteConfigYaml, mockResolveWithSource, mockInvalidate, mockReadAppConfig, mockWriteAppConfig } = vi.hoisted(() => ({
|
||||
const { mockReadFile, mockWriteFile, mockMkdir, mockSaveEnvValue, mockReadConfigYaml, mockWriteConfigYaml, mockUpdateConfigYaml, mockResolveWithSource, mockInvalidate, mockReadAppConfig, mockWriteAppConfig } = vi.hoisted(() => ({
|
||||
mockReadFile: vi.fn(),
|
||||
mockWriteFile: vi.fn().mockResolvedValue(undefined),
|
||||
mockMkdir: vi.fn().mockResolvedValue(undefined),
|
||||
mockSaveEnvValue: vi.fn().mockResolvedValue(undefined),
|
||||
mockReadConfigYaml: vi.fn(),
|
||||
mockWriteConfigYaml: vi.fn().mockResolvedValue(undefined),
|
||||
mockUpdateConfigYaml: vi.fn(),
|
||||
mockResolveWithSource: vi.fn(),
|
||||
mockInvalidate: vi.fn(),
|
||||
mockReadAppConfig: vi.fn(),
|
||||
@@ -28,6 +29,7 @@ vi.mock('../../packages/server/src/services/config-helpers', () => ({
|
||||
saveEnvValue: mockSaveEnvValue,
|
||||
readConfigYaml: mockReadConfigYaml,
|
||||
writeConfigYaml: mockWriteConfigYaml,
|
||||
updateConfigYaml: mockUpdateConfigYaml,
|
||||
}))
|
||||
|
||||
vi.mock('../../packages/server/src/services/hermes/copilot-models', () => ({
|
||||
@@ -58,6 +60,17 @@ beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockReadFile.mockResolvedValue('')
|
||||
mockReadConfigYaml.mockResolvedValue({})
|
||||
mockUpdateConfigYaml.mockImplementation(async (updater: any) => {
|
||||
const cfg = await mockReadConfigYaml()
|
||||
const updated = await updater(cfg)
|
||||
if (updated && typeof updated === 'object' && Object.hasOwn(updated, 'data')) {
|
||||
if (updated.write === false) return updated.result
|
||||
await mockWriteConfigYaml(updated.data)
|
||||
return updated.result
|
||||
}
|
||||
await mockWriteConfigYaml(updated)
|
||||
return undefined
|
||||
})
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
import { mkdtemp, readFile, rm, writeFile } from 'fs/promises'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import YAML from 'js-yaml'
|
||||
import { SafeFileStore } from '../../packages/server/src/services/safe-file-store'
|
||||
|
||||
const tempDirs: string[] = []
|
||||
|
||||
async function tempFile(name: string): Promise<string> {
|
||||
const dir = await mkdtemp(join(tmpdir(), 'hermes-safe-file-store-'))
|
||||
tempDirs.push(dir)
|
||||
return join(dir, name)
|
||||
}
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(tempDirs.splice(0).map(dir => rm(dir, { recursive: true, force: true })))
|
||||
})
|
||||
|
||||
describe('SafeFileStore', () => {
|
||||
it('serializes concurrent YAML read-modify-write updates for the same file', async () => {
|
||||
const store = new SafeFileStore()
|
||||
const file = await tempFile('config.yaml')
|
||||
await writeFile(file, 'model:\n default: old\n', 'utf-8')
|
||||
|
||||
await Promise.all([
|
||||
store.updateYaml(file, async (cfg) => {
|
||||
await new Promise(resolve => setTimeout(resolve, 25))
|
||||
cfg.model.default = 'glm-5.1'
|
||||
return cfg
|
||||
}),
|
||||
store.updateYaml(file, (cfg) => {
|
||||
cfg.platforms = cfg.platforms || {}
|
||||
cfg.platforms.api_server = { extra: { port: 8648 } }
|
||||
return cfg
|
||||
}),
|
||||
])
|
||||
|
||||
const result = YAML.load(await readFile(file, 'utf-8')) as any
|
||||
expect(result.model.default).toBe('glm-5.1')
|
||||
expect(result.platforms.api_server.extra.port).toBe(8648)
|
||||
})
|
||||
|
||||
it('backs up the previous content and writes through a temporary file', async () => {
|
||||
const store = new SafeFileStore()
|
||||
const file = await tempFile('config.yaml')
|
||||
await writeFile(file, 'model:\n default: old\n', 'utf-8')
|
||||
|
||||
await store.writeText(file, 'model:\n default: new\n', { backup: true })
|
||||
|
||||
await expect(readFile(`${file}.bak`, 'utf-8')).resolves.toContain('default: old')
|
||||
await expect(readFile(file, 'utf-8')).resolves.toContain('default: new')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,77 @@
|
||||
import { mkdir, mkdtemp, readFile, rm, writeFile } from 'fs/promises'
|
||||
import { tmpdir } from 'os'
|
||||
import { join } from 'path'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { mockRestartGateway } = vi.hoisted(() => ({
|
||||
mockRestartGateway: vi.fn().mockResolvedValue(undefined),
|
||||
}))
|
||||
|
||||
vi.mock('../../packages/server/src/services/hermes/hermes-cli', () => ({
|
||||
restartGateway: mockRestartGateway,
|
||||
}))
|
||||
|
||||
const originalHermesHome = process.env.HERMES_HOME
|
||||
const tempHomes: string[] = []
|
||||
let hermesHome = ''
|
||||
|
||||
async function loadController() {
|
||||
vi.resetModules()
|
||||
process.env.HERMES_HOME = hermesHome
|
||||
return import('../../packages/server/src/controllers/hermes/weixin')
|
||||
}
|
||||
|
||||
function makeCtx(body: unknown): any {
|
||||
return { request: { body }, status: 200, body: undefined }
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.clearAllMocks()
|
||||
hermesHome = await mkdtemp(join(tmpdir(), 'hermes-weixin-controller-'))
|
||||
tempHomes.push(hermesHome)
|
||||
await mkdir(hermesHome, { recursive: true })
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
vi.resetModules()
|
||||
if (originalHermesHome === undefined) delete process.env.HERMES_HOME
|
||||
else process.env.HERMES_HOME = originalHermesHome
|
||||
await Promise.all(tempHomes.splice(0).map(dir => rm(dir, { recursive: true, force: true })))
|
||||
hermesHome = ''
|
||||
})
|
||||
|
||||
describe('weixin controller save', () => {
|
||||
it('updates .env through the locked file store and preserves unrelated keys', async () => {
|
||||
await writeFile(join(hermesHome, '.env'), [
|
||||
'OPENROUTER_API_KEY=keep',
|
||||
'WEIXIN_TOKEN=old-token',
|
||||
'',
|
||||
].join('\n'), 'utf-8')
|
||||
const { save } = await loadController()
|
||||
const ctx = makeCtx({ account_id: 'acct-1', token: 'new-token', base_url: 'https://weixin.local' })
|
||||
|
||||
await save(ctx)
|
||||
|
||||
expect(ctx.body).toEqual({ success: true })
|
||||
expect(mockRestartGateway).toHaveBeenCalled()
|
||||
const env = await readFile(join(hermesHome, '.env'), 'utf-8')
|
||||
expect(env).toContain('OPENROUTER_API_KEY=keep')
|
||||
expect(env).toContain('WEIXIN_ACCOUNT_ID=acct-1')
|
||||
expect(env).toContain('WEIXIN_TOKEN=new-token')
|
||||
expect(env).toContain('WEIXIN_BASE_URL=https://weixin.local')
|
||||
expect(env).not.toContain('old-token')
|
||||
})
|
||||
|
||||
it('rejects missing required credentials without touching .env', async () => {
|
||||
await writeFile(join(hermesHome, '.env'), 'OPENROUTER_API_KEY=keep\n', 'utf-8')
|
||||
const { save } = await loadController()
|
||||
const ctx = makeCtx({ account_id: 'acct-1' })
|
||||
|
||||
await save(ctx)
|
||||
|
||||
expect(ctx.status).toBe(400)
|
||||
expect(ctx.body).toEqual({ error: 'Missing account_id or token' })
|
||||
expect(mockRestartGateway).not.toHaveBeenCalled()
|
||||
await expect(readFile(join(hermesHome, '.env'), 'utf-8')).resolves.toBe('OPENROUTER_API_KEY=keep\n')
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user