Skip to content

fix(codex): persist desktop settings in atom state - #2166

Merged
shunkakinoki merged 1 commit into
mainfrom
codex/fix-codex-desktop-atom-state
Jul 26, 2026
Merged

fix(codex): persist desktop settings in atom state#2166
shunkakinoki merged 1 commit into
mainfrom
codex/fix-codex-desktop-atom-state

Conversation

@shunkakinoki

@shunkakinoki shunkakinoki commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • store managed Codex Desktop preferences in electron-persisted-atom-state, which version 26.721.41059 actually reads
  • remove ignored top-level duplicates while preserving unrelated global and atom state
  • add regression coverage for app-owned nesting, stale top-level cleanup, rewrite recovery, and idempotency

Verification

  • shellspec spec/activate_config_spec.sh — 55 examples, 0 failures (3 failures before the fix)
  • full ShellSpec phase — 1,720 examples, 0 failures
  • make nix-format-check — 533 files, 0 changed
  • bash -n and ShellCheck on the synchronizer
  • make build — successful Galactica Darwin generation
  • live Home Manager activation — nested keep count 300, top-level copy absent, LaunchAgent last exit 0

Note

  • the Fish sub-suite did not start because its nix develop environment evaluation stalled and was interrupted; no Fish assertion failed
  • unrelated worktrunk/yek macOS Cargo activation failures remain tracked in shunkakinokisoftware-uvkd

Summary by cubic

Persist Codex Desktop managed settings in electron-persisted-atom-state and remove stale top‑level copies. Matches current Desktop behavior, preserves unrelated state, and avoids unnecessary rewrites.

  • Bug Fixes
    • Store managed keys under electron-persisted-atom-state (UI ignores top-level keys).
    • Merge managed keys into atom state and keep unrelated keys; strip stale top-level duplicates.
    • Skip writes when values already match to prevent launchd churn; add regression tests for merge, rewrite recovery, and idempotency.

Written for commit 1ad11da. Summary will update on new commits.

Review in cubic

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@indent-zero

indent-zero Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor
PR Summary

Corrects where the Codex Desktop settings synchronizer persists managed values so that the app-controlled UI actually reads them. Previously the managed keys were written at the top level of ~/.codex/.codex-global-state.json and the nested electron-persisted-atom-state copies were being stripped as "legacy" — Codex Desktop actually reads from the atom-state object, so the UI was ignoring managed values and effectively reverting them. This PR inverts the write target, treats top-level copies as stale, and updates the ShellSpec suite accordingly.

  • Rewrites sync-desktop-settings.sh to merge managed keys into electron-persisted-atom-state (via $atoms + $managed) and strip managed keys from the top level.
  • Inverts the idempotency guard to require managed values in atom-state AND their absence at the top level, keeping the launchd watch loop stable.
  • Changes the initial-file branch to write {"electron-persisted-atom-state": $settings[0]} when no state file exists.
  • Updates the default.nix activation comment to describe atom-state persistence.
  • Updates four ShellSpec cases (merge, restore-after-app-rewrite, remove-stale-top-level, idempotency) with new fixtures and has(...) assertions confirming top-level cleanup.

Issues

No issues found.

CI Checks

Waiting for CI checks...

@mesa-dot-dev

mesa-dot-dev Bot commented Jul 26, 2026

Copy link
Copy Markdown

You do not have enough credits to review this pull request. Please purchase more credits to continue.

@shunkakinoki
shunkakinoki merged commit 52fa213 into main Jul 26, 2026
29 of 31 checks passed
@shunkakinoki
shunkakinoki deleted the codex/fix-codex-desktop-atom-state branch July 26, 2026 17:13
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 62c2ac88-e617-4c22-8a4d-c65524f4cffd

📥 Commits

Reviewing files that changed from the base of the PR and between 6221a9d and 1ad11da.

📒 Files selected for processing (3)
  • config/codex/default.nix
  • config/codex/sync-desktop-settings.sh
  • spec/activate_config_spec.sh

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved synchronization of Codex Desktop preferences by storing managed settings in the app’s persisted state.
    • Preserved unrelated application state during preference updates.
    • Prevented unnecessary rewrites when preferences are already synchronized.
    • Removed outdated top-level preference entries after migration to the persisted state structure.

Walkthrough

Codex Desktop preference synchronization now stores managed values in persisted electron-persisted-atom-state, removes conflicting top-level copies, preserves unrelated atom state, and updates activation tests for nested persistence and no-op behavior.

Changes

Codex Desktop state synchronization

Layer / File(s) Summary
Persist managed preferences in atom state
config/codex/sync-desktop-settings.sh, config/codex/default.nix
Synchronization reads and writes managed preferences under electron-persisted-atom-state, removes overlapping top-level keys, preserves unrelated state, and documents the updated behavior.
Validate activation and no-op behavior
spec/activate_config_spec.sh
Activation tests verify nested merging, restoration, stale top-level key removal, and avoidance of rewrites when persisted values already match.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Poem

A bunny tucked the settings deep,
Where atom-state can safely sleep.
Old top-level copies hopped away,
While matching values skip the write today.
Merge the state, preserve the rest—
Codex preferences pass the test!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-codex-desktop-atom-state

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant