fix: persist deviceId on first call when no config exists#454
fix: persist deviceId on first call when no config exists#454HiddenPuppy wants to merge 1 commit intomainfrom
Conversation
ensureDeviceId() previously generated a random UUID but only wrote it when a config file already existed. On fresh installs (readConfig() returns null), every call returned a new UUID until something else wrote the config. Now we construct a minimal AppConfig with the required fields (workspacePath, gateways) and always call writeConfig() so the deviceId is stable from the very first invocation. Fixes #383
|
Hi @HiddenPuppy, DetailsInstructions for interacting with me using comments are available here. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where the device ID was not being correctly persisted during the initial application launch. By ensuring that a default configuration is created and written to disk immediately upon generating a new device ID, the application now maintains a consistent identity across subsequent calls. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the ensureDeviceId function to ensure that a deviceId is persisted even if the configuration file is missing, by initializing it with default values. A review comment suggests refactoring this implementation to use the existing updateConfig helper, which would avoid duplicating default configuration logic and keep persistence centralized.
| const updated = config ?? { | ||
| workspacePath: getDefaultWorkspacePath(), | ||
| gateways: [], | ||
| }; | ||
| updated.deviceId = deviceId; | ||
| writeConfig(updated); |
There was a problem hiding this comment.
Instead of manually re-implementing the logic to handle a missing configuration, you can use the existing updateConfig helper. This avoids duplicating the default configuration object (workspacePath and gateways) and keeps the persistence logic centralized. Note that using this helper will make the config variable on line 204 unused, so you may want to refactor the initial check to readConfig()?.deviceId.
updateConfig({ deviceId });1 participant
Problem
On first launch, before a config file exists,
ensureDeviceId()generates a random UUID but only persists it when config is non-null. WhenreadConfig()returns null (no config file yet), the function returns a fresh UUID without writing it, so every subsequent call produces a different device ID until something else writes the config.Fix
When config is null, construct a minimal
AppConfigwithworkspacePath: getDefaultWorkspacePath(),gateways: [], and the newdeviceId, then always callwriteConfig(...)before returning.Verification
pnpm checkpasses (40 test files, 243 tests)ensureDeviceId()twice → same UUID both timesFixes #383