diff --git a/src/cli/__tests__/global-config.test.ts b/src/cli/__tests__/global-config.test.ts index 6e2038973..ba00fb421 100644 --- a/src/cli/__tests__/global-config.test.ts +++ b/src/cli/__tests__/global-config.test.ts @@ -82,9 +82,9 @@ describe('global-config', () => { it('creates directory and writes config when none exists', async () => { const fresh = createTempConfig('gc-fresh'); - const ok = await updateGlobalConfig({ telemetry: { enabled: false } }, fresh.configDir, fresh.configFile); + const result = await updateGlobalConfig({ telemetry: { enabled: false } }, fresh.configDir, fresh.configFile); - expect(ok).toBe(true); + expect(result.success).toBe(true); const written = JSON.parse(await readFile(fresh.configFile, 'utf-8')); expect(written).toEqual({ telemetry: { enabled: false } }); @@ -106,14 +106,24 @@ describe('global-config', () => { }); }); - it('returns false on write failures', async () => { - const ok = await updateGlobalConfig( + it('does not overwrite malformed config and reports why', async () => { + await writeFile(tmp.configFile, '{ invalid json'); + + const result = await updateGlobalConfig({ telemetry: { enabled: true } }, tmp.configDir, tmp.configFile); + + expect(result.success).toBe(false); + expect(result.success ? '' : result.error).toMatch(/malformed/); + expect(await readFile(tmp.configFile, 'utf-8')).toBe('{ invalid json'); + }); + + it('returns an error on write failures', async () => { + const result = await updateGlobalConfig( { telemetry: { enabled: true } }, tmp.testDir + '/\0invalid', tmp.testDir + '/\0invalid/config.json' ); - expect(ok).toBe(false); + expect(result.success).toBe(false); }); }); diff --git a/src/cli/commands/telemetry/actions.ts b/src/cli/commands/telemetry/actions.ts index c83a6f425..aa8887f1a 100644 --- a/src/cli/commands/telemetry/actions.ts +++ b/src/cli/commands/telemetry/actions.ts @@ -10,18 +10,18 @@ export async function handleTelemetryDisable( configDir = GLOBAL_CONFIG_DIR, configFile = GLOBAL_CONFIG_FILE ): Promise { - const ok = await updateGlobalConfig({ telemetry: { enabled: false } }, configDir, configFile); - console.log(ok ? 'Telemetry has been disabled.' : `Warning: could not write config to ${configFile}`); - return ok; + const result = await updateGlobalConfig({ telemetry: { enabled: false } }, configDir, configFile); + console.log(result.success ? 'Telemetry has been disabled.' : `Warning: ${result.error}`); + return result.success; } export async function handleTelemetryEnable( configDir = GLOBAL_CONFIG_DIR, configFile = GLOBAL_CONFIG_FILE ): Promise { - const ok = await updateGlobalConfig({ telemetry: { enabled: true } }, configDir, configFile); - console.log(ok ? 'Telemetry has been enabled.' : `Warning: could not write config to ${configFile}`); - return ok; + const result = await updateGlobalConfig({ telemetry: { enabled: true } }, configDir, configFile); + console.log(result.success ? 'Telemetry has been enabled.' : `Warning: ${result.error}`); + return result.success; } export async function handleTelemetryStatus(configFile = GLOBAL_CONFIG_FILE): Promise { diff --git a/src/lib/schemas/io/global-config.ts b/src/lib/schemas/io/global-config.ts index fc64eb39b..6452a1df2 100644 --- a/src/lib/schemas/io/global-config.ts +++ b/src/lib/schemas/io/global-config.ts @@ -4,6 +4,7 @@ import { randomUUID } from 'node:crypto'; import { homedir } from 'os'; import { join } from 'path'; import { z } from 'zod'; +import { toError } from '../../errors/types.js'; export const GLOBAL_CONFIG_DIR = process.env.AGENTCORE_CONFIG_DIR ?? join(homedir(), '.agentcore'); export const GLOBAL_CONFIG_FILE = join(GLOBAL_CONFIG_DIR, 'config.json'); @@ -46,20 +47,55 @@ export function readGlobalConfigSync(configFile = GLOBAL_CONFIG_FILE): GlobalCon } } +export type UpdateGlobalConfigResult = { success: true } | { success: false; error: string }; + export async function updateGlobalConfig( partial: GlobalConfig, configDir = GLOBAL_CONFIG_DIR, configFile = GLOBAL_CONFIG_FILE -): Promise { - try { - const existing = await readGlobalConfig(configFile); - const merged: GlobalConfig = mergeConfig(existing, partial); +): Promise { + // Read the existing config strictly: a missing file is fine (start fresh), but a + // malformed file must not be silently overwritten with merged-in defaults. + const existing = await loadConfigForUpdate(configFile); + if (!existing.success) { + return existing; + } + try { + const merged: GlobalConfig = mergeConfig(existing.config, partial); await mkdir(configDir, { recursive: true }); await writeFile(configFile, JSON.stringify(merged, null, 2), 'utf-8'); - return true; - } catch { - return false; + return { success: true }; + } catch (error) { + return { success: false, error: `Failed to write config to ${configFile}: ${toError(error).message}` }; + } +} + +type LoadConfigResult = { success: true; config: GlobalConfig } | { success: false; error: string }; + +/** + * Reads the existing global config for an update. Distinguishes a missing file + * (treated as an empty config) from a malformed one (read/parse/schema failure), + * so the caller can avoid clobbering a config it could not understand. + */ +async function loadConfigForUpdate(configFile: string): Promise { + let data: string; + try { + data = await readFile(configFile, 'utf-8'); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return { success: true, config: {} }; + } + return { success: false, error: `Could not read config at ${configFile}: ${toError(error).message}` }; + } + + try { + return { success: true, config: GlobalConfigSchema.parse(JSON.parse(data)) }; + } catch (error) { + return { + success: false, + error: `Config at ${configFile} is malformed and was left unchanged: ${toError(error).message}`, + }; } }