fix(desktop): honor XDG_CONFIG_HOME in locale-ipc and preferences-ipc#71
Merged
fix(desktop): honor XDG_CONFIG_HOME in locale-ipc and preferences-ipc#71
Conversation
locale-ipc and preferences-ipc previously hardcoded join(homedir(), '.config', 'open-codesign') for their persisted JSON files, while config.ts honored XDG_CONFIG_HOME. Setting XDG_CONFIG_HOME=/custom caused config.toml to land under the custom path while locale.json / preferences.json stayed under ~/.config/open-codesign — split state. Switch both modules to call configDir() lazily inside read/write fns so the env var is read at call time, matching config.ts. Adds a test in each module asserting that XDG_CONFIG_HOME redirects the persisted file path.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Findings
- [Minor] Test cleanup leaks
XDG_CONFIG_HOMEas literal string — assigningprocess.env['XDG_CONFIG_HOME'] = undefineddoes not unset env vars in Node; it leaves the string"undefined", which can contaminate later tests and hide path-resolution regressions, evidenceapps/desktop/src/main/locale-ipc.test.ts:42,apps/desktop/src/main/preferences-ipc.test.ts:57.
Suggested fix:if (prev === undefined) delete process.env['XDG_CONFIG_HOME']; else process.env['XDG_CONFIG_HOME'] = prev;
Summary
- Review mode: initial
- 1 issue found in added test cleanup logic.
- Required context docs
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.
Testing
- Not run (automation)
open-codesign Bot
| const firstCall = writeFileMock.mock.calls[0]; | ||
| expect(firstCall?.[0]).toBe('/tmp/xdg-locale-test/open-codesign/locale.json'); | ||
| } finally { | ||
| if (prev === undefined) process.env['XDG_CONFIG_HOME'] = undefined; |
Contributor
There was a problem hiding this comment.
process.env['XDG_CONFIG_HOME'] = undefined does not unset the variable; Node stores the literal string "undefined". This can leak into following tests.
Suggested fix:
if (prev === undefined) delete process.env['XDG_CONFIG_HOME'];
else process.env['XDG_CONFIG_HOME'] = prev;| 'utf8', | ||
| ); | ||
| } finally { | ||
| if (prev === undefined) process.env['XDG_CONFIG_HOME'] = undefined; |
Contributor
There was a problem hiding this comment.
Same cleanup issue here: assigning undefined leaves a string value in process.env. Use deletion when the previous value was absent.
Suggested fix:
if (prev === undefined) delete process.env['XDG_CONFIG_HOME'];
else process.env['XDG_CONFIG_HOME'] = prev;
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Bug
apps/desktop/src/main/config.tshonorsXDG_CONFIG_HOMEviaconfigDir(), butlocale-ipc.tsandpreferences-ipc.tshardcodedjoin(homedir(), '.config', 'open-codesign'). WithXDG_CONFIG_HOME=/custom/path,config.tomllands under/custom/path/open-codesign/whilelocale.jsonandpreferences.jsonstay under~/.config/open-codesign/— split state. Violates CLAUDE.md hard constraint: "respect XDG base dirs / Electronapp.getPath()".Fix
Both modules now call
configDir()lazily inside the read/write functions (env var read at call time, matchingconfig.ts). Dropped the no-longer-neededhomedirimport and theCONFIG_DIRwrapper constants.Tests
preferences-ipc.test.ts: added a case assertingXDG_CONFIG_HOMEredirects thereadFilepath.locale-ipc.test.ts(new): single test assertingwriteFileuses the XDG path whenlocale:setruns.PRINCIPLES §5b
XDG_CONFIG_HOMEis unset; existing~/.config/open-codesign/files keep working.schemaVersionfields preserved; no on-disk format changes.configDir()) for all on-disk config paths.