fix(desktop): harden browser and runtime hygiene - #7152
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
cc99802 to
05649b1
Compare
Greptile SummaryThis follow-up hardens desktop server switching by separating teardown readiness, durable configuration persistence, and recovery-marker finalization.
Confidence Score: 4/5The PR is not yet safe to merge because an overlapping failed account teardown can cancel the required relaunch after the new server origin has already been committed. The server picker closes and reports success after committing the new origin, but the application quit handler can prevent the requested relaunch when concurrent account teardown fails, leaving the live process on its old partition and runtime identity. Files Needing Attention: apps/desktop/src/main/server-window.ts and apps/desktop/src/main/index.ts
|
| Filename | Overview |
|---|---|
| apps/desktop/src/main/server-window.ts | The revised commit/finalization order fixes early marker removal, but assumes a requested relaunch cannot be canceled after origin persistence. |
| apps/desktop/src/main/index.ts | The lifecycle composition correctly blocks quitting on unsafe account teardown, which exposes the server-window success-path mismatch. |
| apps/desktop/src/main/account-data-generation.ts | Teardown-kind escalation preserves the stronger account recovery marker and causes deployment finalization to fail safely. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Clear deployment-scoped state] --> B{Deployment teardown still completable?}
B -->|No| C[Return failure]
B -->|Yes| D[Persist scoped clears and new origin]
D --> E{Persistence succeeded?}
E -->|No| C
E -->|Yes| F[Finalize deployment recovery]
F --> G[Request relaunch]
G --> H{Concurrent account teardown clean?}
H -->|Yes| I[Quit and relaunch]
H -->|No| J[Quit cancelled after origin commit]
Reviews (3): Last reviewed commit: "fix(desktop): close review edge cases" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 83 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
05649b1 to
0926849
Compare
| if (!deps.completeDeploymentScopedStateChange()) { | ||
| logger.error('Server changed while account-data recovery remained active') | ||
| } | ||
| } catch (error) { | ||
| // The origin is already durably committed, so returning without a | ||
| // relaunch would mix the old process partition with the new origin. | ||
| // Keep the marker for startup recovery and finish the committed move. | ||
| logger.error('Server changed but deployment-scoped recovery remains pending', { | ||
| error: getErrorMessage(error), | ||
| }) | ||
| } | ||
| logger.info('Server origin changed; relaunching', { from: current, to: origin }) | ||
| close() | ||
| deps.relaunch() |
There was a problem hiding this comment.
If account sign-out starts after the deployment readiness check and its teardown fails, completeDeploymentScopedStateChange() returns false but setOrigin() still closes the picker, requests a relaunch, and reports success. The before-quit handler then cancels that relaunch, leaving the running process on its old partition while the persisted configuration names the new server.
Knowledge Base Used:
1 participant
Summary
Type of Change
Testing
Validated with the full monorepo test and type-check suites, lint, 36 audits, source Electron E2E, and fresh packaged Electron fuse and launch smoke tests.
Checklist