Skip to content

Commit 9a99db3

Browse files
committed
Surface config read/parse errors from updateGlobalConfig
Return a {success, error} result instead of a bare boolean so callers can distinguish a missing config (start fresh) from a malformed one (left unchanged, with the parse/read error surfaced). Telemetry enable/disable now report the actual failure reason rather than a generic write warning.
1 parent 799da57 commit 9a99db3

3 files changed

Lines changed: 51 additions & 26 deletions

File tree

src/cli/__tests__/global-config.test.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,9 @@ describe('global-config', () => {
8282
it('creates directory and writes config when none exists', async () => {
8383
const fresh = createTempConfig('gc-fresh');
8484

85-
const ok = await updateGlobalConfig({ telemetry: { enabled: false } }, fresh.configDir, fresh.configFile);
85+
const result = await updateGlobalConfig({ telemetry: { enabled: false } }, fresh.configDir, fresh.configFile);
8686

87-
expect(ok).toBe(true);
87+
expect(result.success).toBe(true);
8888
const written = JSON.parse(await readFile(fresh.configFile, 'utf-8'));
8989
expect(written).toEqual({ telemetry: { enabled: false } });
9090

@@ -106,23 +106,24 @@ describe('global-config', () => {
106106
});
107107
});
108108

109-
it('does not overwrite malformed config', async () => {
109+
it('does not overwrite malformed config and reports why', async () => {
110110
await writeFile(tmp.configFile, '{ invalid json');
111111

112-
const ok = await updateGlobalConfig({ telemetry: { enabled: true } }, tmp.configDir, tmp.configFile);
112+
const result = await updateGlobalConfig({ telemetry: { enabled: true } }, tmp.configDir, tmp.configFile);
113113

114-
expect(ok).toBe(false);
114+
expect(result.success).toBe(false);
115+
expect(result.success ? '' : result.error).toMatch(/malformed/);
115116
expect(await readFile(tmp.configFile, 'utf-8')).toBe('{ invalid json');
116117
});
117118

118-
it('returns false on write failures', async () => {
119-
const ok = await updateGlobalConfig(
119+
it('returns an error on write failures', async () => {
120+
const result = await updateGlobalConfig(
120121
{ telemetry: { enabled: true } },
121122
tmp.testDir + '/\0invalid',
122123
tmp.testDir + '/\0invalid/config.json'
123124
);
124125

125-
expect(ok).toBe(false);
126+
expect(result.success).toBe(false);
126127
});
127128
});
128129

src/cli/commands/telemetry/actions.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,18 @@ export async function handleTelemetryDisable(
1010
configDir = GLOBAL_CONFIG_DIR,
1111
configFile = GLOBAL_CONFIG_FILE
1212
): Promise<boolean> {
13-
const ok = await updateGlobalConfig({ telemetry: { enabled: false } }, configDir, configFile);
14-
console.log(ok ? 'Telemetry has been disabled.' : `Warning: could not write config to ${configFile}`);
15-
return ok;
13+
const result = await updateGlobalConfig({ telemetry: { enabled: false } }, configDir, configFile);
14+
console.log(result.success ? 'Telemetry has been disabled.' : `Warning: ${result.error}`);
15+
return result.success;
1616
}
1717

1818
export async function handleTelemetryEnable(
1919
configDir = GLOBAL_CONFIG_DIR,
2020
configFile = GLOBAL_CONFIG_FILE
2121
): Promise<boolean> {
22-
const ok = await updateGlobalConfig({ telemetry: { enabled: true } }, configDir, configFile);
23-
console.log(ok ? 'Telemetry has been enabled.' : `Warning: could not write config to ${configFile}`);
24-
return ok;
22+
const result = await updateGlobalConfig({ telemetry: { enabled: true } }, configDir, configFile);
23+
console.log(result.success ? 'Telemetry has been enabled.' : `Warning: ${result.error}`);
24+
return result.success;
2525
}
2626

2727
export async function handleTelemetryStatus(configFile = GLOBAL_CONFIG_FILE): Promise<void> {

src/lib/schemas/io/global-config.ts

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { randomUUID } from 'node:crypto';
44
import { homedir } from 'os';
55
import { join } from 'path';
66
import { z } from 'zod';
7+
import { toError } from '../../errors/types.js';
78

89
export const GLOBAL_CONFIG_DIR = process.env.AGENTCORE_CONFIG_DIR ?? join(homedir(), '.agentcore');
910
export const GLOBAL_CONFIG_FILE = join(GLOBAL_CONFIG_DIR, 'config.json');
@@ -46,32 +47,55 @@ export function readGlobalConfigSync(configFile = GLOBAL_CONFIG_FILE): GlobalCon
4647
}
4748
}
4849

50+
export type UpdateGlobalConfigResult = { success: true } | { success: false; error: string };
51+
4952
export async function updateGlobalConfig(
5053
partial: GlobalConfig,
5154
configDir = GLOBAL_CONFIG_DIR,
5255
configFile = GLOBAL_CONFIG_FILE
53-
): Promise<boolean> {
54-
try {
55-
const existing = await readGlobalConfigForUpdate(configFile);
56-
const merged: GlobalConfig = mergeConfig(existing, partial);
56+
): Promise<UpdateGlobalConfigResult> {
57+
// Read the existing config strictly: a missing file is fine (start fresh), but a
58+
// malformed file must not be silently overwritten with merged-in defaults.
59+
const existing = await loadConfigForUpdate(configFile);
60+
if (!existing.success) {
61+
return existing;
62+
}
5763

64+
try {
65+
const merged: GlobalConfig = mergeConfig(existing.config, partial);
5866
await mkdir(configDir, { recursive: true });
5967
await writeFile(configFile, JSON.stringify(merged, null, 2), 'utf-8');
60-
return true;
61-
} catch {
62-
return false;
68+
return { success: true };
69+
} catch (error) {
70+
return { success: false, error: `Failed to write config to ${configFile}: ${toError(error).message}` };
6371
}
6472
}
6573

66-
async function readGlobalConfigForUpdate(configFile: string): Promise<GlobalConfig> {
74+
type LoadConfigResult = { success: true; config: GlobalConfig } | { success: false; error: string };
75+
76+
/**
77+
* Reads the existing global config for an update. Distinguishes a missing file
78+
* (treated as an empty config) from a malformed one (read/parse/schema failure),
79+
* so the caller can avoid clobbering a config it could not understand.
80+
*/
81+
async function loadConfigForUpdate(configFile: string): Promise<LoadConfigResult> {
82+
let data: string;
6783
try {
68-
const data = await readFile(configFile, 'utf-8');
69-
return GlobalConfigSchema.parse(JSON.parse(data));
84+
data = await readFile(configFile, 'utf-8');
7085
} catch (error) {
7186
if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
72-
return {};
87+
return { success: true, config: {} };
7388
}
74-
throw error;
89+
return { success: false, error: `Could not read config at ${configFile}: ${toError(error).message}` };
90+
}
91+
92+
try {
93+
return { success: true, config: GlobalConfigSchema.parse(JSON.parse(data)) };
94+
} catch (error) {
95+
return {
96+
success: false,
97+
error: `Config at ${configFile} is malformed and was left unchanged: ${toError(error).message}`,
98+
};
7599
}
76100
}
77101

0 commit comments

Comments
 (0)