`|| {}` Deleted the User's Config

Here is an installer merging one entry into the user's config. Read it and decide whether you'd approve it.

function readJson(file) {
  try { return JSON.parse(fs.readFileSync(file, 'utf8')); } catch { return null; }
}
 
function registerInFile(file) {
  const existing = readJson(file) || {};
  const servers = existing.mcpServers || {};
  servers[SERVER_NAME] = serverEntry();
  writeJson(file, { ...existing, mcpServers: servers });
}

It reads, spreads, writes. It handles a missing file. It is idempotent. It is twelve lines and there is nothing clever in it.

It also destroys configs.

What || {} is actually saying

readJson returns null for every failure. Not just "file not found" — also a syntax error, a truncated write, an unreadable mount, a file that parses to an array instead of an object.

Then || {} says: treat all of that as "the user has no config yet."

So when someone's ~/.claude.json has a trailing comma — the single most common hand-edit mistake in JSON — the installer reads null, decides the config is empty, and writes back an object containing only its own entry.

Measured on a realistic config with two other MCP servers, a projects block with history, and user settings:

366 bytes in. 140 bytes out.

Other MCP servers, project entries, theme, autoUpdates — gone. No error. No warning. No backup. And this ran automatically during npx, on a path nobody thought of as risky.

The distinction the code never made

A file read has three outcomes, and this collapsed them into two:

"Absent" and "unreadable" look identical through a try/catch that returns null. They could not be more different in consequence. One means there is nothing here; the other means there is something here and I cannot see it.

function readConfig(file) {
  let raw;
  try {
    raw = fs.readFileSync(file, 'utf8');
  } catch (err) {
    if (err.code === 'ENOENT') return { state: 'absent', data: {} };
    return { state: 'unreadable', reason: err.message };
  }
  if (raw.trim() === '') return { state: 'absent', data: {} };
  try {
    const data = JSON.parse(raw);
    if (!data || typeof data !== 'object' || Array.isArray(data)) {
      return { state: 'unreadable', reason: 'top-level value is not an object' };
    }
    return { state: 'present', data };
  } catch (err) {
    return { state: 'unreadable', reason: err.message };
  }
}

Then the caller refuses:

if (cfg.state === 'unreadable') {
  throw new Error(
    `refusing to modify ${file}: it exists but could not be parsed (${cfg.reason}). ` +
    `Fix or move the file, then re-run. Nothing was written.`
  );
}

Refusing is the whole fix. There is no safe way to merge into a file whose contents you cannot reconstruct, and "best effort" here means "silently discard the parts I couldn't read."

The same rule applies on the way out. Unregistering also reads, modifies, writes — so removing an entry from a file you can't parse means writing a file you can't reconstruct. It skips instead.

The second way to lose the same data

Even with the read fixed, fs.writeFileSync truncates the target before writing. A crash, a full disk, or a kill signal partway through leaves a half-written config — the same loss by a different route.

safeFs.replaceFilePreservingMode(file, JSON.stringify(obj, null, 2) + '\n');

Write a temp file, rename it over the target. Rename is atomic on POSIX, so the file is either entirely old or entirely new. PreservingMode keeps whatever permissions the user chose rather than widening them — this is their config, and an installer has no business relaxing a 600.

Two mistakes I made fixing it

Worth including, because both were caught by testing the parts that already worked.

The first fix broke every install. I called replaceFileAtomic(file, contents) and omitted its third argument, mode. It threw on every path, including valid configs. If I'd only verified that the refusal worked — the behaviour I was adding — I'd have shipped an installer that never registered anything at all.

The mode test asserted POSIX semantics. assert.equal(mode & 0o777, 0o600) passed locally and failed CI with 438 !== 384. That's 0o666 against 0o600: the job was running on Windows, which has no POSIX permission bits, so chmod is close to a no-op and Node reports 0666 whatever you set.

The property I cared about was never the literal value — it was that the write doesn't change the mode. Capture before, compare after. True on every platform, and a better test besides.

A small aside: the CI job was labelled "Test (Node 20)" and the failing path was D:\a\.... The version in the name sent me looking at Node 20 API differences before I noticed the backslashes. Name your matrix jobs after every axis that varies.

The part that let it through

This file had no tests. Not thin ones — none.

That is the third time in this codebase that a bug and a coverage gap turned out to be the same fact. A loader with a path traversal: no tests. An excerpt helper that read whole files: no tests. A config writer that deleted configs: no tests.

Each one looked too simple to be worth testing. Each one was simple. Simple and wrong are entirely compatible.

It now has eight: the three read states, preservation of unrelated config on a valid merge, byte-identical refusal across four kinds of malformed input, creation from absent and empty, idempotence, unregister's refusal, mode preservation, and no stray temp files left behind.

If you write to a file you didn't create

Three questions, all cheap:

1. Does your reader distinguish "absent" from "unreadable"? If both paths lead to {}, you have this bug.
2. Is the write atomic? If not, a crash mid-write is a second way to lose the same data.
3. Is there a test for the malformed case? Not the missing case — the malformed case. It is the one people skip and the only one that destroys something.

Grep your codebase for || {} and ?? {} next to a file read. It is a very short grep and an uncomfortable one.

Fixed in ECC 2.24.9. Verified against the published package, not just the source: a malformed config is left byte-identical, and a valid one keeps every other server, project and setting.