You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(desktop): one unreadable value in desktop-settings.json no longer resets every desktop setting - #28682
If desktop-settings.json holds one value this build can't read (for example "updateChannel": "beta" from a hand edit or another build), the desktop app ignores the whole file and starts from defaults. The next settings save writes those defaults over the file, so the user's local environment, network access, Tailscale, WSL and window settings are gone for good.
Change
The strict document decode in readSettings failed before the field-by-field normalizeDesktopSettingsDocument could run. It now drops a key whose value it can't read, by mapping the existing document schema's fields through Schema.catchDecoding, so that field falls back to its default and every other field still loads. Malformed JSON, or a file that isn't an object, still falls back to defaults as before.
Scope
A bug fix for lost data. It is a small, focused fix for an obvious bug, so it needs no issue: one bad field should not throw away the other valid settings. Possibly related: #26144 (localEnvironmentEnabled: false disappeared), but that report doesn't say what the file held, so this PR doesn't claim to fix it.
Verification
To see it on Linux (desktop app):
Write { "localEnvironmentEnabled": false, "updateChannel": "beta" } to desktop-settings.json.
Start the desktop app and open Settings → Environments.
On main Local environment is on, and after a close the saved file has lost localEnvironmentEnabled; with this PR it stays off and is kept.
Linux desktop app built with vp run build:desktop from main 65c0195b46, then from this branch at d388a0a3d2 (this PR merged with that main): unpackaged Electron 44 under Xvfb, an isolated home, environment named "Test machine", the same file written before each launch. Settings opened by URL over the debugging port; the app closed with Alt+F4 through the window manager (X input). The extended DesktopAppSettings.test.ts load case fails with main's DesktopAppSettings.ts and passes here (13/13).
Main 65c0195: Local environment is on after loading the file
## desktop-settings.json after quit:
{"mainWindowBounds":{"x":170,"y":60,"width":1100,"height":780}}
This PR at d388a0a3d2:
## desktop-settings.json after quit:
{"localEnvironmentEnabled":false,"mainWindowBounds":{"x":170,"y":60,"width":1100,"height":780}}
Not checked:
macOS: not run. Windows: not run.
Packaged installers, and settings files from real older or newer builds. The runs used a hand-written unreadable value.
The change in full and the tests
normalizeDesktopSettingsDocument already falls back field by field. The strict document decode in readSettings fails first, though, and Effect.orElseSucceed swaps in the defaults for every field. The document decode now drops a key whose value it can't read (for example "updateChannel": "beta" or "wslOnly": "yes"), so that field falls back to its default and every other field still loads. Field types, defaults, the normalizer and the write path are unchanged. Mapping the fields, instead of rewriting each one, also covers any field added later. A value the build can't read is still not kept: the next save rewrites the file from the loaded settings, the same as for values the normalizer already replaces, such as an out-of-range Tailscale port.
vp test run src/settings/DesktopAppSettings.test.ts in apps/desktop: the existing lenient-JSON load case carries "updateChannel": "beta", "wslOnly": "yes" and "localEnvironmentEnabled": false, then saves a setting and reloads. With main's DesktopAppSettings.ts (from 65c0195b46) it fails, because every field comes back as its default (localEnvironmentEnabled: true, serverExposureMode: "local-only", tailscaleServePort: 443, mainWindowBounds: null); with the fix, 13/13 pass and localEnvironmentEnabled stays false after the save and reload.
On d388a0a3d2, Node 26.12.1: vp run typecheck in apps/desktop passes; vp run check:changed passes (format, lint, both knip passes, typecheck, tests).
In the desktop runs above, main's saved file held only mainWindowBounds after the close; with this PR it kept "localEnvironmentEnabled": false, and updateChannel was dropped.
Possibly related: #26144, where localEnvironmentEnabled: false disappeared from desktop-settings.json. The run above shows the same symptom from a file with one unreadable value, but that report doesn't say what the file held, so this PR doesn't claim to fix it.
Prior PRs
None make this fix. #3265, #3379 and #3432 by juliusmarminge reworked how desktop settings errors are structured and diagnosed. #26757 by t3dotgg adds a field to the same document schema and benefits from this without changes.
Generated with Claude Opus 5.5 and GPT-6.1 Terra and GPT-6.1 Sol in T3 Code Nightly
Macroscope's review found this PR approvable — Unreadable desktop settings now fall back individually, preserving other saved preferences. Existing defaults and validation remain unchanged, with no new operational behavior introduced.
You can add or adjust custom eligibility rules. Learn more.
No actionable comments were generated in the recent review.
Walkthrough
Desktop settings decoding now omits fields that cannot be decoded instead of failing the entire document. Tests check that unsupported values use defaults, valid values load, and other loaded settings remain after a server exposure mode update.
The decoder handles unreadable fields independently. Tests check defaults for unsupported updateChannel and wslOnly values, preserve localEnvironmentEnabled: false, and verify that other loaded settings remain after a server exposure mode update.
This change keeps one invalid desktop setting from resetting all the others. No actionable merge-blocking risk was found. Packaged installers and settings files from other builds were not verified.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title states the main change: one unreadable value in desktop-settings.json no longer resets every desktop setting. The raw summary describes per-field decoding middleware (DropUnreadableKey) in D…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description check
The description follows the repository template closely. It has all four sections: Problem, Change, Scope, and Verification. The Problem section states the bug: one unreadable value in desktop-setting…
Full details: Title check
Explanation
The title states the main change: one unreadable value in desktop-settings.json no longer resets every desktop setting. The raw summary describes per-field decoding middleware (DropUnreadableKey) in DesktopAppSettings.ts that drops only the unreadable key, which matches the title. The title is clear, specific, and follows the conventional commit format used in the repository template.
Full details: Description check
Explanation
The description follows the repository template closely. It has all four sections: Problem, Change, Scope, and Verification. The Problem section states the bug: one unreadable value in desktop-settings.json makes the app discard the whole file and reset to defaults. The Change section explains that the document decode now drops only the unreadable key, so other fields still load. The Scope section explains why this is a bug fix and why no issue is linked, and it mentions a possibly related issue (#26144) without claiming to fix it. The Verification section gives concrete steps, environments, before/after results, the saved file contents, the test results, and the checks that were not run (macOS, Windows, packaged installers, real older or newer settings files). The template asks for before/after screenshots for UI changes. The author includes them, but the images are hosted on an external gist link. The Verification section also ends with a model attribution line, as the template asks. Minor gaps: the conventional commit title is not shown in the provided description text, and the title is not part of the body. Those gaps are non-critical.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
verified head a3ca0bde317205c476237d86d4ea673cf21358dc as-is on linux, using the real electron 44.10.0 desktop client, dev renderer/server, and playwright from the repository. no mocks or seeded app data.
reproduced on github/main (e925dd681904) first. both launches received this same settings document in separate task homes:
main started the local backend, showed the local environment switch on, and used default 1100×780 bounds. its next save dropped the local-off choice.
this head skipped the local backend, showed "This computer is off", kept the switch off, and restored 1280×900 bounds at (120,80).
restarted with the same file: still off. cancelling enable left it off. confirming enable started the local environment after restart.
created a project through the client pointing at a real git repository. codex gpt-6.1-sol, through the local proxy, completed two client-submitted turns: cat README.md returned the repository marker; a follow-up git status --short returned a clean tree. both commands exited 0 and both turns completed without errors.
disabled the local environment through settings and restarted again: still off, no desktop backend listener, bounds preserved. the saved document retained localEnvironmentEnabled:false and removed the unreadable fields.
the development harness requests relaunch by exiting with code 75; i relaunched the identical electron command after each restart request. session d-bus was isolated to avoid configuring the vm's keyring.
vp run check:changed 8760d063e903 passed through blacksmith-check: format, lint, knip, typechecks for both affected packages, and all 12 desktop settings tests. existing ci reports 31 successful checks. no source changes were needed.
macos, windows/wsl, native ios/android, packaged installers, tailscale exposure, remote/relay connections, and update downloads were not exercised on this linux vm. linux keyring integration was not exercised. invalid update-channel and wsl values were hand-written to reproduce the reported file corruption; project, thread, and provider data were created through the real app.
@maria-rcks Since your verification at a3ca0bde31, the only change is b075ca2ea3, which edits only DesktopAppSettings.test.ts: the unreadable-field fixture now also sets "localEnvironmentEnabled": false and checks that it survives a later save. DesktopAppSettings.ts is unchanged, and CI passes on this head.
Thanks for this. Main has the change since #36168.
Main now drops only unreadable desktop-settings.json fields while preserving valid settings, including Local environment off, across saves. I compared your complete implementation and regression-test diff with the merge: both apply Schema.catchDecoding per field and retain the existing normalization, defaults, lenient JSON parsing, and write behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size:S10-29 changed lines (additions + deletions).vouch:unvouchedAuthor is not yet trusted in the VOUCHED list.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Problem
If
desktop-settings.jsonholds one value this build can't read (for example"updateChannel": "beta"from a hand edit or another build), the desktop app ignores the whole file and starts from defaults. The next settings save writes those defaults over the file, so the user's local environment, network access, Tailscale, WSL and window settings are gone for good.Change
The strict document decode in
readSettingsfailed before the field-by-fieldnormalizeDesktopSettingsDocumentcould run. It now drops a key whose value it can't read, by mapping the existing document schema's fields throughSchema.catchDecoding, so that field falls back to its default and every other field still loads. Malformed JSON, or a file that isn't an object, still falls back to defaults as before.Scope
A bug fix for lost data. It is a small, focused fix for an obvious bug, so it needs no issue: one bad field should not throw away the other valid settings. Possibly related: #26144 (
localEnvironmentEnabled: falsedisappeared), but that report doesn't say what the file held, so this PR doesn't claim to fix it.Verification
To see it on Linux (desktop app):
{ "localEnvironmentEnabled": false, "updateChannel": "beta" }todesktop-settings.json.localEnvironmentEnabled; with this PR it stays off and is kept.Linux desktop app built with
vp run build:desktopfrom main65c0195b46, then from this branch atd388a0a3d2(this PR merged with that main): unpackaged Electron 44 under Xvfb, an isolated home, environment named "Test machine", the same file written before each launch. Settings opened by URL over the debugging port; the app closed with Alt+F4 through the window manager (X input). The extendedDesktopAppSettings.test.tsload case fails with main'sDesktopAppSettings.tsand passes here (13/13).Main 65c0195: Local environment is on after loading the file
This PR at d388a0a: Local environment stays off
The saved file after the close, main
65c0195b46:This PR at
d388a0a3d2:Not checked:
The change in full and the tests
normalizeDesktopSettingsDocumentalready falls back field by field. The strict document decode inreadSettingsfails first, though, andEffect.orElseSucceedswaps in the defaults for every field. The document decode now drops a key whose value it can't read (for example"updateChannel": "beta"or"wslOnly": "yes"), so that field falls back to its default and every other field still loads. Field types, defaults, the normalizer and the write path are unchanged. Mapping the fields, instead of rewriting each one, also covers any field added later. A value the build can't read is still not kept: the next save rewrites the file from the loaded settings, the same as for values the normalizer already replaces, such as an out-of-range Tailscale port.vp test run src/settings/DesktopAppSettings.test.tsinapps/desktop: the existing lenient-JSON load case carries"updateChannel": "beta","wslOnly": "yes"and"localEnvironmentEnabled": false, then saves a setting and reloads. With main'sDesktopAppSettings.ts(from65c0195b46) it fails, because every field comes back as its default (localEnvironmentEnabled: true,serverExposureMode: "local-only",tailscaleServePort: 443,mainWindowBounds: null); with the fix, 13/13 pass andlocalEnvironmentEnabledstaysfalseafter the save and reload.d388a0a3d2, Node 26.12.1:vp run typecheckinapps/desktoppasses;vp run check:changedpasses (format, lint, both knip passes, typecheck, tests).DesktopAppSettings.tsthree times since this PR's base (chore(deps): upgrade Effect to 4.1.2 and alchemy to beta.85 #26833, fix(desktop): failed settings saves no longer leave temp files behind #29635, fix(desktop): let the window shrink to 400x360 so it fits half-screen tiles #28223);readSettingsstill ends inEffect.orElseSucceed(() => defaultSettings), and the merge was clean.In the desktop runs above, main's saved file held only
mainWindowBoundsafter the close; with this PR it kept"localEnvironmentEnabled": false, andupdateChannelwas dropped.Possibly related: #26144, where
localEnvironmentEnabled: falsedisappeared fromdesktop-settings.json. The run above shows the same symptom from a file with one unreadable value, but that report doesn't say what the file held, so this PR doesn't claim to fix it.Prior PRs
None make this fix. #3265, #3379 and #3432 by juliusmarminge reworked how desktop settings errors are structured and diagnosed. #26757 by t3dotgg adds a field to the same document schema and benefits from this without changes.
Generated with Claude Opus 5.5 and GPT-6.1 Terra and GPT-6.1 Sol in T3 Code Nightly