Skip to content

fix: abort install/uninstall on config parse errors to prevent data loss - #34

Closed
abhinavkr26104 wants to merge 1 commit into
supermemoryai:mainfrom
abhinavkr26104:config-parse-safety
Closed

fix: abort install/uninstall on config parse errors to prevent data loss#34
abhinavkr26104 wants to merge 1 commit into
supermemoryai:mainfrom
abhinavkr26104:config-parse-safety

Conversation

@abhinavkr26104

Copy link
Copy Markdown
Contributor

Summary

Prevent accidental loss of user configuration during install and uninstall when existing Codex configuration files contain invalid TOML or JSON.

Changes

  • Abort on config.toml TOML parse failures instead of falling back to an empty config.
  • Abort on hooks.json JSON parse failures instead of recreating hook configuration.
  • Validate existing Codex config files before install/uninstall writes begin.
  • Preserve existing configuration files without modification.
  • Improve CLI error messages with actionable parse diagnostics.
  • Add regression tests ensuring invalid configuration files are never overwritten.

Problem

Previously, parse failures were silently ignored in the install/uninstall merge paths. If ~/.codex/config.toml or ~/.codex/hooks.json contained a syntax error, the command could continue with an empty configuration object and overwrite the original file, potentially erasing unrelated user settings.

Result

Configuration files are now treated as authoritative. If an existing file cannot be parsed, the operation stops safely, reports the issue, and leaves the existing file untouched.

Testing

  • npm test passed
  • Added tests for invalid TOML on install/uninstall
  • Added tests for invalid JSON on install/uninstall
  • Verified failed operations preserve file contents, size, and modification time

issue fixed: #33

@Dhravya

Dhravya commented Aug 8, 2026

Copy link
Copy Markdown
Member

Thanks for this — the fail-closed approach is the right fix for #33.

We're landing a slightly simpler version on the same idea: keep parse-and-merge into the existing config, and only abort (no write) when parse fails. Same treatment for hooks.json and supermemory.json, plus a pre-mutate check so install doesn't half-apply when Codex config is invalid.

Happy to close this PR in favor of that once it's up, or we can pull any bits from here if useful. Appreciate the regression tests either way.

@Dhravya

Dhravya commented Aug 8, 2026

Copy link
Copy Markdown
Member

Landed the simpler parse-and-merge fix in #35 — fine to close this in favor of that.

@abhinavkr26104

Copy link
Copy Markdown
Contributor Author

Landed the simpler parse-and-merge fix in #35 — fine to close this in favor of that.

Thanks for the clarification. I'll close #34 in favor of #35. Glad the regression tests were useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Install/uninstall can overwrite Codex config when config parsing fails

2 participants