fix(config): leave an unparseable global config untouched (#1876)

* fix(config): leave an unparseable global config untouched

A typo in config.json made getGlobalConfig() fall back to defaults, which telemetry read as consent: any command, even a read-only list, minted a new anonymous id and wrote it over the whole file, dropping a telemetry.enabled false opt-out and every other setting. config set, unset and profile likewise saved the defaults over it.

saveGlobalConfig() and telemetry's writeConfig() now refuse to overwrite a file they cannot parse, telemetry and the update check treat such a file as opted out, and config set, unset and profile exit with an error pointing to config edit. config reset --all can still replace the file, and the existing warning is unchanged.

* fix(config): treat a non-object global config as unreadable

Valid JSON that is not an object (null, an array, a string) also makes
getGlobalConfig() fall back to defaults, silently, so `config set` still
saved those defaults over the user's file. isGlobalConfigUnreadable() now
reports such a file as unreadable, which keeps telemetry off and routes
every save through the same refusal as a parse failure. This matches how
completion-tip already treats a non-object config.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs(config): document the refusal to rewrite an unparseable config

docs-lab/reference/cli.md said `config unset` always exits 0. With an
unparseable global config, `config set`, `config unset` and
`config profile` now exit 1 and leave the file unchanged; say so, show
the message and the two fixes, and note telemetry and the update check
stay off until it is fixed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(config): warn about an unparseable global config once per command

Telemetry, the update check and the command each read the global config,
and now that none of them rewrites the broken file, the "Invalid JSON"
warning printed two or three times per command. Warn once per path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(migration): skip profile migration for a config that is not a JSON object

A global config holding [] reached saveGlobalConfig, which now refuses
it, so init and update failed. null already crashed on a property read.
migrateIfNeeded now skips such a file, as it does for a parse failure.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(telemetry): refuse to write over a non-object global config

The telemetry writer had its own notion of an unreadable config: only a
JSON parse failure counted. Valid JSON that is not an object slipped
through, so updateTelemetryConfig() merged into it and replaced the file
-- an array, a number or a boolean became a bare telemetry object, a
string spread into numeric character keys, and null threw a TypeError
instead of the actionable refusal every other writer reports.

Funnel both notions through one predicate: isConfigRootObject() in
core/global-config.ts now backs isGlobalConfigUnreadable() and the
telemetry reader, so every shape the global guard rejects is classified
invalid on read and refused on write. Both writers report the same
one-line message via unreadableGlobalConfigMessage().

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(config): read a non-object global config as plain defaults

getGlobalConfig() spread the parsed root into its result before the
unreadable predicate was consulted, so the shape of the root leaked to
every caller: a config of "abc" returned defaults plus the numeric
character keys 0, 1 and 2. Check isConfigRootObject() right after
parsing and answer with plain defaults, as for a file that did not parse
at all.

Reported by CodeRabbit as an outside-the-diff finding.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(config): stop `config list` crashing on a null config root

`config list` re-reads the raw file to mark each value explicit or
default, and assigned JSON.parse() straight to rawConfig. A root of
`null` then crashed the command with a TypeError stack trace, the one
failure mode this PR is meant to remove, and it did so on a read-only
command. Normalize a non-object root to {} through the shared
isConfigRootObject() predicate so the listing shows plain defaults.

Reported by CodeRabbit as an outside-the-diff finding.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* test(config): show the telemetry notice before the first-run write check

Since #1835, nothing is tracked until the notice has been shown, so the
first-run test must show it before tracking the command.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Clay Good <hi@claygood.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

.changeset/unparseable-global-config-left-alone.md

@@ -0,0 +1,5 @@
+---
+'@fission-ai/openspec': patch
+---
+
+Stop OpenSpec rewriting a global config file it cannot parse. After a hand edit left a typo such as a trailing comma in `config.json`, the next command of any kind, including read-only ones like `openspec list`, read the fallback defaults as telemetry consent, minted a new anonymous ID and wrote it back, replacing the whole file: a `telemetry.enabled false` opt-out, the chosen profile and the workflow list were all lost, and usage events were sent. A config file that exists but does not hold a JSON object, whether it failed to parse or its root is something else such as `null`, an array or a string, is now never written implicitly, and telemetry and the update check treat it as opted out. `config set`, `config unset` and `config profile` refuse with an error that names the file and points to `openspec config edit`, and `openspec config reset --all` still replaces it. The existing "Invalid JSON" warning is unchanged, and valid or missing config files behave exactly as before.

docs-lab/reference/cli.md

@@ -302,6 +302,15 @@ Pass --allow-unknown to bypass this check.
 Error: Invalid configuration - delivery: Invalid option: expected one of "both"|"skills"|"commands"
 ```
 
+If the config file exists but does not hold a JSON object, whether because it is not valid JSON at all or because its root is something else such as `null` or an array, `config set`, `config unset` and `config profile` exit 1 and leave the file unchanged. Fix it with `openspec config edit`, or replace it with `openspec config reset --all`:
+
+```
+Error: /home/you/.config/openspec/config.json could not be parsed, so it was left unchanged.
+Fix it with "openspec config edit", or reset it with "openspec config reset --all".
+```
+
+Until it is fixed, telemetry and the update check stay off.
+
 ### openspec config unset
 
 ```bash
@@ -314,7 +323,7 @@ Removes the key so the default applies again. Keys with built-in defaults always
 Unset delivery (reverted to default)
 ```
 
-A key with no value at all prints `Key "featureFlags.nothere" was not set`. Both cases exit 0.
+A key with no value at all prints `Key "featureFlags.nothere" was not set`. Both cases exit 0. A config file that cannot be parsed exits 1 instead, as for `config set`.
 
 ### openspec config reset
 

src/commands/config.ts

@@ -6,6 +6,8 @@ import * as path from 'node:path';
 import {
   getGlobalConfigPath,
   getGlobalConfig,
+  isConfigRootObject,
+  isGlobalConfigUnreadable,
   saveGlobalConfig,
   GlobalConfig,
 } from '../core/global-config.js';
@@ -150,6 +152,21 @@ function reportEditorFailure(editor: string, outcome: EditorOutcome): void {
 
 type ProfileAction = 'both' | 'delivery' | 'workflows' | 'keep';
 
+/**
+ * A config file that exists but cannot be parsed is still the user's file:
+ * getGlobalConfig() reads it as defaults, and saving those back would erase
+ * every setting in it. Reports the fix instead, and returns true when it did.
+ */
+function refuseUnreadableConfig(): boolean {
+  if (!isGlobalConfigUnreadable()) {
+    return false;
+  }
+  console.error(`Error: ${getGlobalConfigPath()} could not be parsed, so it was left unchanged.`);
+  console.error('Fix it with "openspec config edit", or reset it with "openspec config reset --all".');
+  process.exitCode = 1;
+  return true;
+}
+
 interface ProfileState {
   profile: Profile;
   delivery: Delivery;
@@ -370,7 +387,12 @@ export function registerConfigCommand(program: Command): void {
         let rawConfig: Record<string, unknown> = {};
         try {
           if (fs.existsSync(configPath)) {
-            rawConfig = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
+            const parsed: unknown = JSON.parse(fs.readFileSync(configPath, 'utf-8'));
+            // A non-object root holds no explicit settings, and reading a key
+            // off `null` would crash this read-only command.
+            if (isConfigRootObject(parsed)) {
+              rawConfig = parsed as Record<string, unknown>;
+            }
           }
         } catch {
           // If reading fails, treat all as defaults
@@ -436,6 +458,10 @@ export function registerConfigCommand(program: Command): void {
         return;
       }
 
+      if (refuseUnreadableConfig()) {
+        return;
+      }
+
       const config = getGlobalConfig() as Record<string, unknown>;
       const coercedValue = coerceValue(value, options.string || false);
 
@@ -465,6 +491,10 @@ export function registerConfigCommand(program: Command): void {
     .command('unset <key>')
     .description('Remove a key (revert to default)')
     .action((key: string) => {
+      if (refuseUnreadableConfig()) {
+        return;
+      }
+
       const config = getGlobalConfig() as Record<string, unknown>;
       const existed = deleteNestedValue(config, key);
 
@@ -513,7 +543,8 @@ export function registerConfigCommand(program: Command): void {
         }
       }
 
-      saveGlobalConfig({ ...DEFAULT_CONFIG });
+      // A reset is the one write meant to replace a file that cannot be parsed.
+      saveGlobalConfig({ ...DEFAULT_CONFIG }, { replaceUnreadable: true });
       console.log('Configuration reset to defaults');
     });
 
@@ -574,6 +605,10 @@ export function registerConfigCommand(program: Command): void {
     .command('profile [preset]')
     .description('Configure workflow profile (interactive picker or preset shortcut)')
     .action(async (preset?: string) => {
+      if (refuseUnreadableConfig()) {
+        return;
+      }
+
       // Preset shortcut: `openspec config profile core`
       if (preset === 'core') {
         const config = getGlobalConfig();

src/core/global-config.ts

@@ -129,6 +129,10 @@ export function getGlobalConfigPath(): string {
   return path.join(getGlobalConfigDir(), GLOBAL_CONFIG_FILE_NAME);
 }
 
+// Config paths already warned about. One command reads the config several
+// times (telemetry, the update check, the command itself); warn once.
+const warnedInvalidJsonPaths = new Set<string>();
+
 /**
  * Loads the global configuration from disk.
  * Returns default configuration if file doesn't exist or is invalid.
@@ -145,6 +149,14 @@ export function getGlobalConfig(): GlobalConfig {
     const content = fs.readFileSync(configPath, 'utf-8');
     const parsed = JSON.parse(content);
 
+    // A root that is not a plain object carries no settings, and spreading it
+    // would leak its shape into the result: a string contributes numeric
+    // character keys. Answer with plain defaults, as for a file that did not
+    // parse at all. Same predicate the writers refuse to save over.
+    if (!isConfigRootObject(parsed)) {
+      return { ...DEFAULT_CONFIG };
+    }
+
     // Merge with defaults (loaded values take precedence)
     const merged: GlobalConfig = {
       ...DEFAULT_CONFIG,
@@ -167,7 +179,8 @@ export function getGlobalConfig(): GlobalConfig {
     return merged;
   } catch (error) {
     // Log warning for parse errors, but not for missing files
-    if (error instanceof SyntaxError) {
+    if (error instanceof SyntaxError && !warnedInvalidJsonPaths.has(configPath)) {
+      warnedInvalidJsonPaths.add(configPath);
       console.error(`Warning: Invalid JSON in ${configPath}, using defaults`);
     }
     return { ...DEFAULT_CONFIG };
@@ -175,13 +188,67 @@ export function getGlobalConfig(): GlobalConfig {
 }
 
 /**
- * Saves the global configuration to disk.
- * Creates the config directory if it doesn't exist.
+ * Whether a parsed JSON root can serve as a global config object.
+ *
+ * Valid JSON that is not a plain object (`null`, an array, a string, a number,
+ * a boolean) still reads as defaults, so it is just as unsafe to save over as
+ * a file that did not parse at all. Every reader and writer of the global
+ * config shares this one predicate so they cannot drift apart.
  */
-export function saveGlobalConfig(config: GlobalConfig): void {
+export function isConfigRootObject(parsed: unknown): boolean {
+  return typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed);
+}
+
+/**
+ * The one-line, actionable refusal every global-config writer reports when it
+ * declines to overwrite a file it could not read.
+ */
+export function unreadableGlobalConfigMessage(configPath: string): string {
+  return (
+    `Refusing to overwrite ${configPath}: it could not be parsed, so saving would replace every setting in it. ` +
+    'Fix it with "openspec config edit", or reset it with "openspec config reset --all".'
+  );
+}
+
+/**
+ * Whether the global config file exists but cannot be read or parsed.
+ *
+ * getGlobalConfig() answers with defaults for such a file so that reads keep
+ * working, but those defaults are not the user's settings: saving them back
+ * would erase everything the file holds, and the file may contain an opt-out
+ * such as `telemetry.enabled: false` that the defaults do not.
+ */
+export function isGlobalConfigUnreadable(): boolean {
+  const configPath = getGlobalConfigPath();
+  if (!fs.existsSync(configPath)) {
+    return false;
+  }
+
+  try {
+    return !isConfigRootObject(JSON.parse(fs.readFileSync(configPath, 'utf-8')));
+  } catch {
+    return true;
+  }
+}
+
+export interface SaveGlobalConfigOptions {
+  /** Overwrite a config file that cannot be parsed. Only a reset should. */
+  replaceUnreadable?: boolean;
+}
+
+/**
+ * Saves the global configuration to disk.
+ * Creates the config directory if it doesn't exist. Refuses to overwrite an
+ * existing file it cannot parse unless `replaceUnreadable` is set.
+ */
+export function saveGlobalConfig(config: GlobalConfig, options: SaveGlobalConfigOptions = {}): void {
   const configDir = getGlobalConfigDir();
   const configPath = getGlobalConfigPath();
 
+  if (!options.replaceUnreadable && isGlobalConfigUnreadable()) {
+    throw new Error(unreadableGlobalConfigMessage(configPath));
+  }
+
   // Create directory if it doesn't exist
   if (!fs.existsSync(configDir)) {
     fs.mkdirSync(configDir, { recursive: true });

src/core/migration.ts

@@ -6,7 +6,7 @@
  */
 
 import { AI_TOOLS, type AIToolOption } from './config.js';
-import { getGlobalConfig, getGlobalConfigPath, saveGlobalConfig, type Delivery } from './global-config.js';
+import { getGlobalConfig, getGlobalConfigPath, isGlobalConfigUnreadable, saveGlobalConfig, type Delivery } from './global-config.js';
 import { CommandAdapterRegistry } from './command-generation/index.js';
 import {
   resolveCommandInvocation,
@@ -560,6 +560,12 @@ function inferDelivery(artifacts: InstalledWorkflowArtifacts): Delivery {
  * - If profile field already exists: no-op.
  */
 export function migrateIfNeeded(projectPath: string, tools: AIToolOption[]): void {
+  // A config that cannot be parsed, or is not a JSON object, is never saved
+  // over; skip migration rather than fail init or update on it.
+  if (isGlobalConfigUnreadable()) {
+    return;
+  }
+
   const config = getGlobalConfig();
 
   // Check raw config file for profile field presence

src/core/version-check.ts

@@ -6,7 +6,7 @@ import { createRequire } from 'module';
 import chalk from 'chalk';
 import { isCiEnvironment } from '../utils/ci.js';
 import { isTelemetryOptedOutByEnv } from '../telemetry/opt-out.js';
-import { getGlobalConfig } from './global-config.js';
+import { getGlobalConfig, isGlobalConfigUnreadable } from './global-config.js';
 
 const require = createRequire(import.meta.url);
 const { name: PACKAGE_NAME, version: OPENSPEC_VERSION } = require('../../package.json');
@@ -38,6 +38,8 @@ function isCheckEnabled(): boolean {
   if (process.env.NODE_ENV === 'test') return false;
   // Same config opt-out as telemetry (env remains the hard override above).
   if (getGlobalConfig().telemetry?.enabled === false) return false;
+  // A config that cannot be parsed may be hiding that opt-out.
+  if (isGlobalConfigUnreadable()) return false;
   return true;
 }
 

src/telemetry/config.ts

@@ -9,6 +9,8 @@ import {
   GLOBAL_CONFIG_DIR_NAME,
   GLOBAL_CONFIG_FILE_NAME,
   getGlobalConfigDir,
+  isConfigRootObject,
+  unreadableGlobalConfigMessage,
   type TelemetryConfig,
 } from '../core/global-config.js';
 
@@ -40,7 +42,14 @@ function getLegacyConfigPath(): string {
 async function readConfigFile(configPath: string): Promise<ConfigReadResult> {
   try {
     const content = await fs.readFile(configPath, 'utf-8');
-    return { status: 'ok', config: JSON.parse(content) as GlobalConfig };
+    const parsed: unknown = JSON.parse(content);
+    // Valid JSON that is not an object carries no settings to merge into, and
+    // spreading it would replace the file (a string even spreads to numeric
+    // character keys). Same predicate the rest of the CLI refuses to save over.
+    if (!isConfigRootObject(parsed)) {
+      return { status: 'invalid', config: {} };
+    }
+    return { status: 'ok', config: parsed as GlobalConfig };
   } catch (error: unknown) {
     if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
       return { status: 'missing' };
@@ -150,6 +159,12 @@ export async function readConfig(): Promise<GlobalConfig> {
 export async function writeConfig(updates: Partial<GlobalConfig>): Promise<void> {
   const configPath = getConfigPath();
 
+  // Never write over a file that did not parse: the merge below would start
+  // from an empty object and replace every setting in it with these updates.
+  if ((await readConfigFile(configPath)).status === 'invalid') {
+    throw new Error(unreadableGlobalConfigMessage(configPath));
+  }
+
   // Read existing config and merge
   const existing = await readConfig();
   const merged = { ...existing, ...updates };

src/telemetry/index.ts

@@ -20,7 +20,7 @@
  * versions and broke installs (#1390).
  */
 import { randomUUID } from 'crypto';
-import { getGlobalConfig } from '../core/global-config.js';
+import { getGlobalConfig, isGlobalConfigUnreadable } from '../core/global-config.js';
 import { isCiEnvironment } from '../utils/ci.js';
 import { getTelemetryConfig, updateTelemetryConfig } from './config.js';
 import { isTelemetryOptedOutByEnv } from './opt-out.js';
@@ -69,7 +69,8 @@ async function safeTelemetryFetch(url: string, options: RequestInit): Promise<Re
  * 2. DO_NOT_TRACK set to anything but an off-value (0/false/no/off) → disabled
  * 3. CI set to a truthy/on value → disabled (same rule as version-check)
  * 4. global config telemetry.enabled === false → disabled
- * 5. otherwise enabled (unset config means on; opt-out model)
+ * 5. global config file exists but cannot be parsed → disabled
+ * 6. otherwise enabled (unset config means on; opt-out model)
  *
  * Kept synchronous so call sites need not become async. Reads config via
  * sync getGlobalConfig() rather than async getTelemetryConfig().
@@ -92,6 +93,12 @@ export function isTelemetryEnabled(): boolean {
     return false;
   }
 
+  // A config file that cannot be parsed reads as defaults, which carry no
+  // opt-out, but the file itself may hold one. Unknown is not consent.
+  if (isGlobalConfigUnreadable()) {
+    return false;
+  }
+
   return true;
 }
 

test/core/global-config.unparseable.test.ts

@@ -0,0 +1,386 @@
+import { describe, it, expect, beforeAll, beforeEach, afterEach, vi } from 'vitest';
+import * as fs from 'node:fs';
+import * as path from 'node:path';
+import * as os from 'node:os';
+import { Command } from 'commander';
+import { runCLI } from '../helpers/run-cli.js';
+
+/**
+ * A global config file with a typo is still the user's file. Reads fall back
+ * to defaults with a warning, but nothing may write those defaults back over
+ * it, and telemetry may not read them as consent: the file can hold
+ * `telemetry.enabled: false`.
+ */
+
+const SETTINGS = {
+  featureFlags: {},
+  profile: 'custom',
+  delivery: 'both',
+  telemetry: { noticeSeen: true, anonymousId: '0e249867-af5d-445b-92c7-722fbec024f1', enabled: false },
+  workflows: ['propose', 'apply', 'verify'],
+};
+const VALID = `${JSON.stringify(SETTINGS, null, 2)}\n`;
+// The trailing comma a hand edit leaves behind.
+const TYPO = VALID.replace(/\n}\n$/, ',\n}\n');
+
+describe('an unparseable global config', () => {
+  let tempDir: string;
+  let configPath: string;
+  let originalEnv: NodeJS.ProcessEnv;
+  let originalExitCode: typeof process.exitCode;
+  let consoleErrorSpy: ReturnType<typeof vi.spyOn>;
+  let fetchSpy: ReturnType<typeof vi.spyOn<typeof globalThis, 'fetch'>>;
+
+  beforeEach(() => {
+    tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-unparseable-config-'));
+    originalEnv = { ...process.env };
+    originalExitCode = process.exitCode;
+
+    process.env.XDG_CONFIG_HOME = tempDir;
+    process.env.HOME = tempDir;
+    process.env.USERPROFILE = tempDir;
+    process.env.APPDATA = path.join(tempDir, 'appdata');
+    // Telemetry stays on at the environment level, as in a user's shell, so
+    // only the config file decides.
+    delete process.env.OPENSPEC_TELEMETRY;
+    delete process.env.DO_NOT_TRACK;
+    delete process.env.CI;
+
+    configPath = path.join(tempDir, 'openspec', 'config.json');
+    fs.mkdirSync(path.dirname(configPath), { recursive: true });
+
+    consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+    vi.spyOn(console, 'log').mockImplementation(() => {});
+    fetchSpy = vi
+      .spyOn(globalThis, 'fetch')
+      .mockResolvedValue(new Response(null, { status: 200 }));
+  });
+
+  afterEach(async () => {
+    const { shutdown } = await import('../../src/telemetry/index.js');
+    await shutdown();
+    process.env = originalEnv;
+    process.exitCode = originalExitCode;
+    vi.restoreAllMocks();
+    fs.rmSync(tempDir, { recursive: true, force: true });
+  });
+
+  const read = () => fs.readFileSync(configPath, 'utf-8');
+
+  it('is detected as unreadable, while a missing or valid file is not', async () => {
+    const { isGlobalConfigUnreadable } = await import('../../src/core/global-config.js');
+
+    expect(isGlobalConfigUnreadable()).toBe(false);
+    fs.writeFileSync(configPath, VALID);
+    expect(isGlobalConfigUnreadable()).toBe(false);
+    fs.writeFileSync(configPath, TYPO);
+    expect(isGlobalConfigUnreadable()).toBe(true);
+  });
+
+  it('warns about it once, however many times it is read', async () => {
+    const { getGlobalConfig } = await import('../../src/core/global-config.js');
+    fs.writeFileSync(configPath, TYPO);
+
+    getGlobalConfig();
+    getGlobalConfig();
+    getGlobalConfig();
+
+    const warnings = consoleErrorSpy.mock.calls.filter((call) => String(call[0]).includes('Invalid JSON'));
+    expect(warnings).toHaveLength(1);
+  });
+
+  it.each(['null\n', '[]\n', '"core"\n'])('treats valid JSON that is not an object (%j) as unreadable', async (content) => {
+    const { isGlobalConfigUnreadable, saveGlobalConfig } = await import('../../src/core/global-config.js');
+    fs.writeFileSync(configPath, content);
+
+    expect(isGlobalConfigUnreadable()).toBe(true);
+    expect(() => saveGlobalConfig({ profile: 'core' })).toThrow(/Refusing to overwrite/);
+    expect(read()).toBe(content);
+  });
+
+  // Reads must answer with plain defaults, not with the shape of the root: a
+  // string used to spread into numeric character keys that reached callers.
+  it.each(['null\n', '[]\n', '["core"]\n', '"abc"\n', '42\n', 'true\n'])(
+    'reads a non-object root (%j) as plain defaults',
+    async (content) => {
+      const { getGlobalConfig } = await import('../../src/core/global-config.js');
+      fs.writeFileSync(configPath, content);
+
+      expect(getGlobalConfig()).toEqual({ featureFlags: {}, profile: 'core', delivery: 'both' });
+    },
+  );
+
+  describe('saveGlobalConfig', () => {
+    it('refuses to overwrite it, names the file and the fix, and leaves it byte-identical', async () => {
+      const { saveGlobalConfig } = await import('../../src/core/global-config.js');
+      fs.writeFileSync(configPath, TYPO);
+
+      let message = '';
+      try {
+        saveGlobalConfig({ profile: 'core' });
+      } catch (error) {
+        message = (error as Error).message;
+      }
+
+      expect(message).toContain(configPath);
+      expect(message).toContain('openspec config edit');
+      expect(read()).toBe(TYPO);
+    });
+
+    it('replaces it when asked to, as a reset does', async () => {
+      const { saveGlobalConfig } = await import('../../src/core/global-config.js');
+      fs.writeFileSync(configPath, TYPO);
+
+      saveGlobalConfig({ profile: 'core' }, { replaceUnreadable: true });
+
+      expect(JSON.parse(read())).toEqual({ profile: 'core' });
+    });
+
+    it('still creates a missing file and overwrites a valid one', async () => {
+      const { saveGlobalConfig } = await import('../../src/core/global-config.js');
+
+      saveGlobalConfig({ profile: 'custom' });
+      expect(JSON.parse(read())).toEqual({ profile: 'custom' });
+      saveGlobalConfig({ profile: 'core' });
+      expect(JSON.parse(read())).toEqual({ profile: 'core' });
+    });
+  });
+
+  describe('telemetry', () => {
+    // Fresh modules per test: telemetry caches the anonymous id it minted.
+    beforeEach(() => {
+      vi.resetModules();
+    });
+
+    it('is off when the file cannot be parsed', async () => {
+      fs.writeFileSync(configPath, TYPO);
+      const { isTelemetryEnabled } = await import('../../src/telemetry/index.js');
+
+      expect(isTelemetryEnabled()).toBe(false);
+    });
+
+    it('is on for a valid file with no opt-out', async () => {
+      fs.writeFileSync(configPath, JSON.stringify({ profile: 'core' }));
+      const { isTelemetryEnabled } = await import('../../src/telemetry/index.js');
+
+      expect(isTelemetryEnabled()).toBe(true);
+    });
+
+    it('sends nothing and writes nothing when a command runs', async () => {
+      fs.writeFileSync(configPath, TYPO);
+      const { maybeShowTelemetryNotice, trackCommand, shutdown } = await import('../../src/telemetry/index.js');
+
+      await maybeShowTelemetryNotice();
+      await trackCommand('list', '0.0.0-test');
+      await shutdown();
+
+      expect(fetchSpy).not.toHaveBeenCalled();
+      expect(read()).toBe(TYPO);
+      // The existing warning still tells the user their settings are ignored.
+      expect(consoleErrorSpy).toHaveBeenCalledWith(expect.stringContaining('Invalid JSON'));
+    });
+
+    it('still records the notice and identity in a valid file, keeping its other settings', async () => {
+      fs.writeFileSync(configPath, JSON.stringify({ profile: 'custom', workflows: ['propose'] }, null, 2));
+      const { maybeShowTelemetryNotice, trackCommand, shutdown } = await import('../../src/telemetry/index.js');
+
+      await maybeShowTelemetryNotice();
+      await trackCommand('list', '0.0.0-test');
+      await shutdown();
+
+      expect(fetchSpy).toHaveBeenCalledTimes(1);
+      const saved = JSON.parse(read());
+      expect(saved.profile).toBe('custom');
+      expect(saved.workflows).toEqual(['propose']);
+      expect(saved.telemetry.noticeSeen).toBe(true);
+      expect(saved.telemetry.anonymousId).toEqual(expect.any(String));
+    });
+
+    it('still creates the file on a first run', async () => {
+      const { maybeShowTelemetryNotice, trackCommand, shutdown } = await import('../../src/telemetry/index.js');
+
+      // Nothing is tracked until the notice has been shown, so a first run
+      // shows it before the command is tracked.
+      await maybeShowTelemetryNotice();
+      await trackCommand('list', '0.0.0-test');
+      await shutdown();
+
+      expect(JSON.parse(read()).telemetry.anonymousId).toEqual(expect.any(String));
+    });
+
+    it('never has a telemetry update written over the file', async () => {
+      fs.writeFileSync(configPath, TYPO);
+      const { updateTelemetryConfig } = await import('../../src/telemetry/config.js');
+
+      await expect(updateTelemetryConfig({ noticeSeen: true })).rejects.toThrow(/could not be parsed/);
+      expect(read()).toBe(TYPO);
+    });
+
+    // Every shape isGlobalConfigUnreadable() rejects must also be refused by
+    // the telemetry writer, which merges into whatever it reads. A non-object
+    // root used to slip past it: an array or a number replaced the file with a
+    // bare telemetry object, and a string spread into numeric character keys.
+    const NON_OBJECT_ROOTS: Array<[string, string]> = [
+      ['null', 'null\n'],
+      ['an array', '[]\n'],
+      ['a populated array', '["core", "custom"]\n'],
+      ['a string', '"core"\n'],
+      ['a number', '42\n'],
+      ['a boolean', 'true\n'],
+    ];
+
+    it.each(NON_OBJECT_ROOTS)(
+      'refuses to write telemetry over %s and leaves the file byte-identical',
+      async (_label, content) => {
+        fs.writeFileSync(configPath, content);
+        const { isGlobalConfigUnreadable } = await import('../../src/core/global-config.js');
+        const { updateTelemetryConfig } = await import('../../src/telemetry/config.js');
+
+        // The guarantee is defined by the predicate, so assert it applies here.
+        expect(isGlobalConfigUnreadable()).toBe(true);
+
+        let message = '';
+        await updateTelemetryConfig({ noticeSeen: true }).catch((error: unknown) => {
+          message = (error as Error).message;
+        });
+
+        // Not a TypeError: the same actionable one-liner every writer reports.
+        expect(message).toContain(configPath);
+        expect(message).toContain('openspec config edit');
+        expect(message).toContain('openspec config reset --all');
+        expect(read()).toBe(content);
+        expect(fetchSpy).not.toHaveBeenCalled();
+      },
+    );
+
+    it.each(NON_OBJECT_ROOTS)(
+      'reads %s as opted-out, sending and writing nothing for a whole command',
+      async (_label, content) => {
+        fs.writeFileSync(configPath, content);
+        const { isTelemetryEnabled, maybeShowTelemetryNotice, trackCommand, shutdown } = await import(
+          '../../src/telemetry/index.js'
+        );
+        const { getTelemetryConfig } = await import('../../src/telemetry/config.js');
+
+        expect(isTelemetryEnabled()).toBe(false);
+        // The reader must answer with empty settings rather than throw.
+        await expect(getTelemetryConfig()).resolves.toEqual({});
+
+        await maybeShowTelemetryNotice();
+        await trackCommand('list', '0.0.0-test');
+        await shutdown();
+
+        expect(fetchSpy).not.toHaveBeenCalled();
+        expect(read()).toBe(content);
+      },
+    );
+
+    it('still mints an anonymous id into a valid file after a non-object root is fixed', async () => {
+      fs.writeFileSync(configPath, '[]\n');
+      const { getOrCreateAnonymousId } = await import('../../src/telemetry/index.js');
+
+      // The refusal must not be silently swallowed into a corrupt write.
+      await expect(getOrCreateAnonymousId()).rejects.toThrow(/could not be parsed/);
+      expect(read()).toBe('[]\n');
+
+      fs.writeFileSync(configPath, '{}\n');
+      vi.resetModules();
+      const fixed = await import('../../src/telemetry/index.js');
+      await expect(fixed.getOrCreateAnonymousId()).resolves.toEqual(expect.any(String));
+      expect(JSON.parse(read()).telemetry.anonymousId).toEqual(expect.any(String));
+    });
+  });
+
+  describe('openspec config', () => {
+    let registerConfigCommand: typeof import('../../src/commands/config.js').registerConfigCommand;
+
+    // Imported once: the command module pulls in most of the CLI.
+    beforeAll(async () => {
+      ({ registerConfigCommand } = await import('../../src/commands/config.js'));
+    }, 60_000);
+
+    async function runConfig(args: string[]): Promise<void> {
+      const program = new Command();
+      registerConfigCommand(program);
+      await program.parseAsync(['node', 'openspec', 'config', ...args]);
+    }
+
+    const errorOutput = () => consoleErrorSpy.mock.calls.map((call) => call.join(' ')).join('\n');
+
+    it.each([
+      [['set', 'profile', 'core']],
+      [['set', 'telemetry.enabled', 'true']],
+      [['unset', 'profile']],
+      [['profile', 'core']],
+    ])('refuses `config %s` and leaves the file unchanged', async (args) => {
+      fs.writeFileSync(configPath, TYPO);
+
+      await runConfig(args);
+
+      expect(read()).toBe(TYPO);
+      expect(process.exitCode).toBe(1);
+      expect(errorOutput()).toContain(configPath);
+      expect(errorOutput()).toContain('openspec config edit');
+    });
+
+    // `config list` reads the raw file to mark values explicit vs default. A
+    // `null` root used to crash it with a TypeError stack trace.
+    it.each(['null\n', '[]\n', '"core"\n', '42\n', 'true\n', TYPO])(
+      'lists defaults without crashing for %j and leaves the file unchanged',
+      async (content) => {
+        fs.writeFileSync(configPath, content);
+
+        await expect(runConfig(['list'])).resolves.toBeUndefined();
+
+        expect(read()).toBe(content);
+        expect(process.exitCode).not.toBe(1);
+      },
+    );
+
+    it('lets `config reset --all` replace the file with defaults', async () => {
+      fs.writeFileSync(configPath, TYPO);
+
+      await runConfig(['reset', '--all', '--yes']);
+
+      const saved = JSON.parse(read());
+      expect(saved.profile).toBe('core');
+      expect(saved.telemetry).toBeUndefined();
+    });
+
+    it('still sets a value in a valid file', async () => {
+      fs.writeFileSync(configPath, VALID);
+
+      await runConfig(['set', 'profile', 'core']);
+
+      const saved = JSON.parse(read());
+      expect(saved.profile).toBe('core');
+      expect(saved.telemetry).toEqual(SETTINGS.telemetry);
+    });
+  });
+
+  it('is left byte-identical by a read-only CLI command', async () => {
+    const project = path.join(tempDir, 'project');
+    fs.mkdirSync(path.join(project, 'openspec', 'specs'), { recursive: true });
+    fs.mkdirSync(path.join(project, 'openspec', 'changes'), { recursive: true });
+    fs.writeFileSync(path.join(project, 'openspec', 'config.yaml'), 'schema: spec-driven\n');
+    fs.writeFileSync(configPath, TYPO);
+
+    const result = await runCLI(['list'], {
+      cwd: project,
+      env: {
+        XDG_CONFIG_HOME: tempDir,
+        HOME: tempDir,
+        USERPROFILE: tempDir,
+        // Telemetry on at the environment level, as for a real user.
+        OPENSPEC_TELEMETRY: '1',
+        DO_NOT_TRACK: '0',
+        CI: 'false',
+      },
+      timeoutMs: 60_000,
+    });
+
+    expect(read()).toBe(TYPO);
+    // Telemetry and the command each read the config; the warning prints once.
+    expect(result.stderr.match(/Invalid JSON/g)).toHaveLength(1);
+  }, 120_000);
+});

test/core/migration.test.ts

@@ -168,6 +168,19 @@ describe('migration', () => {
     expect(config.workflows).toEqual(['explore']);
   });
 
+  it.each(['[]\n', 'null\n', '{ "profile": "custom", }\n'])(
+    'leaves a config that is not a readable JSON object (%j) alone instead of failing',
+    async (content) => {
+      await writeSkill(projectDir, 'openspec-explore');
+      const configPath = getGlobalConfigPath();
+      await fsp.mkdir(path.dirname(configPath), { recursive: true });
+      await fsp.writeFile(configPath, content, 'utf-8');
+
+      expect(() => captureMigrationLogs(projectDir, [ensureClaudeTool()])).not.toThrow();
+      expect(fs.readFileSync(configPath, 'utf-8')).toBe(content);
+    }
+  );
+
   it('does not migrate when no managed workflow artifacts are detected', async () => {
     migrateIfNeeded(projectDir, [ensureClaudeTool()]);
 

test/core/version-check.test.ts

@@ -358,6 +358,32 @@ describe('getAvailableCliUpdate', () => {
     }
   });
 
+  it('sends nothing when the global config cannot be parsed', async () => {
+    const xdgHome = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-vc-unparseable-'));
+    const previousXdg = process.env.XDG_CONFIG_HOME;
+    vi.spyOn(console, 'error').mockImplementation(() => {});
+    try {
+      process.env.XDG_CONFIG_HOME = xdgHome;
+      const configDir = path.join(xdgHome, 'openspec');
+      fs.mkdirSync(configDir, { recursive: true });
+      // A hand-edit typo can hide the opt-out the file holds.
+      fs.writeFileSync(
+        path.join(configDir, 'config.json'),
+        '{\n  "telemetry": {\n    "enabled": false\n  },\n}\n'
+      );
+
+      await expect(getAvailableCliUpdate()).resolves.toBeNull();
+      expect(requests).toHaveLength(0);
+    } finally {
+      if (previousXdg === undefined) {
+        delete process.env.XDG_CONFIG_HOME;
+      } else {
+        process.env.XDG_CONFIG_HOME = previousXdg;
+      }
+      fs.rmSync(xdgHome, { recursive: true, force: true });
+    }
+  });
+
   it('still runs when CI is explicitly switched off', async () => {
     for (const value of ['false', '0', 'no', '']) {
       process.env.CI = value;