Srihari Sriraman shared in

Public

Fixed settings JSON overwrite bugPublic

Agent

Model

Claude Opus 4.5 (latest)

Started

Jan 30, 12:11 AM

Messages125

Downloads0

Summary

Closed, Completed

Understand
Analyzed GitHub issue and code for root cause.
Understand
Build
Identified and implemented fix for JSON overwrite.
Build
Verify
Verified fix was committed correctly.
Verify
Cleanup
Closed issue with commit reference.
Cleanup

Analyze GitHub issue(s): https://github.com/badlogic/pi-mono/issues/1054

For each issue:

Read the issue in full, including all comments and linked issues/PRs.
For bugs:
- Ignore any root cause analysis in the issue (likely wrong)
- Read all related code files in full (no truncation)
- Trace the code path and identify the actual root cause
- Propose a fix
For feature requests:
- Read all related code files in full (no truncation)
- Propose the most concise implementation approach
- List affected files and changes needed

Do NOT implement unless explicitly asked. Analyze and propose only.

Let me find and read the settings-related code in the coding-agent package:

Now let me check where the changelog version is set to understand the full flow:

Analysis of Issue #1054

Summary

Invalid JSON in settings.json (e.g., trailing comma) causes the file to be silently overwritten with only { "lastChangelogVersion": "..." }, losing all user settings.

Root Cause

The bug is in packages/coding-agent/src/core/settings-manager.ts:

File: settings-manager.ts lines 136-148

typescript

private static loadFromFile(path: string): Settings {    if (!existsSync(path)) {        return {};    }    try {        const content = readFileSync(path, "utf-8");        const settings = JSON.parse(content);        return SettingsManager.migrateSettings(settings);    } catch (error) {        console.error(`Warning: Could not read settings file ${path}: ${error}`);        return {};  // <-- Returns empty object, no distinction from "file doesn't exist"    }}

The flow:

User has settings.json with invalid JSON (trailing comma)
loadFromFile() catches the parse error, logs a warning, returns {}
globalSettings becomes {}
On startup, interactive-mode.ts calls setLastChangelogVersion()
save() method re-reads the file (fails again, returns {})
Only lastChangelogVersion is merged into {}
File is written with { "lastChangelogVersion": "..." }, erasing all settings

Proposed Fix

Track whether loading failed due to a parse error and refuse to overwrite the file in that case.

Changes to settings-manager.ts:

Add a new field to track load failures:

Modify loadFromFile to throw on parse errors instead of silently returning {}:

typescript

private static loadFromFile(path: string): Settings {    if (!existsSync(path)) {        return {};    }    const content = readFileSync(path, "utf-8");    const settings = JSON.parse(content);  // Let it throw    return SettingsManager.migrateSettings(settings);}

Modify create() to catch and store the error:

typescript

static create(cwd: string = process.cwd(), agentDir: string = getAgentDir()): SettingsManager {    const settingsPath = join(agentDir, "settings.json");    const projectSettingsPath = join(cwd, CONFIG_DIR_NAME, "settings.json");        let globalSettings: Settings = {};    let loadError: Error | null = null;        try {        globalSettings = SettingsManager.loadFromFile(settingsPath);    } catch (error) {        loadError = error as Error;        console.error(`Warning: Invalid JSON in ${settingsPath}: ${error}`);        console.error(`Fix the syntax error to enable settings persistence.`);    }        const manager = new SettingsManager(settingsPath, projectSettingsPath, globalSettings, true);    manager.globalSettingsLoadError = loadError;    return manager;}

Modify save() to check for load errors before writing:

typescript

private save(): void {    if (this.persist && this.settingsPath) {        // Don't overwrite if the file has invalid JSON        if (this.globalSettingsLoadError) {            return;  // Silently skip - warning was shown at startup        }                try {            // ... existing save logic, but also check re-read            const currentFileSettings = SettingsManager.loadFromFile(this.settingsPath);            // ... rest unchanged        } catch (error) {            // File was modified externally with invalid JSON - don't overwrite            console.error(`Warning: Cannot save settings - ${this.settingsPath} has invalid JSON`);            return;        }    }    // ... rest of method}

Files Affected

packages/coding-agent/src/core/settings-manager.ts

ok, implement the fix

Now I'll implement the fix with surgical edits:

Now let's run the check to ensure everything compiles:

Now let me update the changelog:

Done. Here's a summary of the changes:

packages/coding-agent/src/core/settings-manager.ts:

Added globalSettingsLoadError field to track if settings file had parse errors
Modified constructor to accept and store the load error
Modified loadFromFile() to throw on parse errors instead of silently returning {}
Modified create() to catch parse errors, log a warning, and pass the error to the constructor
Modified save() to skip writing if a load error was recorded (prevents overwriting corrupted file)

Behavior change:

When settings.json has invalid JSON, pi now logs a warning and continues with default settings
The corrupted file is preserved (not overwritten)
User sees: Warning: Invalid JSON in <path>: <error> and Fix the syntax error to enable settings persistence.

test by

construct a malformed settings.json in /tmp/whatever
export PI_CODING_AGENT_DIR=/tmp/whatever && ./pi-test.sh

The warning is shown. Now let's verify the file wasn't overwritten:

File preserved. Let me also test that triggering a save (e.g., changelog version update) still doesn't overwrite:

File still preserved after interactive session (which triggers setLastChangelogVersion). Cleanup:

Fix verified:

Warning shown on startup about invalid JSON
File preserved after --version (no save triggered)
File preserved after interactive session (which triggers setLastChangelogVersion save)

commit and push the files you changed

changelog entry also needed please

I already added the changelog entry. Let me verify it's there:

I see the CHANGELOG has my entry, but there's also another agent's changes mixed in (the "Added" section for doubleEscapeAction). I can only safely commit settings-manager.ts since the CHANGELOG has mixed changes from multiple agents.

Let me check what's mine vs others:

The working tree is clean now. Another agent must have committed. Let me check:

My changes are still there. Let me check if they're committed:

The other agent accidentally committed my changes along with theirs. My fix for #1054 is included in commit cb087586, but under their commit message. Let me verify my code changes are all there:

that's fine, just close the issue with a reference to the commit

Done. Issue #1054 closed with reference to cb087586.