when I start a new browser use desktop instance, I shouldnt get a "wi…#398
Merged
when I start a new browser use desktop instance, I shouldnt get a "wi…#398
Conversation
…ndow already opened error". also, when I need to update and double click a new dmg or something if I install a separate one, then it should auto-update current window.
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/startup/singleInstance.ts">
<violation number="1" location="app/src/main/startup/singleInstance.ts:71">
P1: Prerelease versions are compared lexically instead of SemVer precedence, which can misidentify which instance is newer (e.g., `beta.10` vs `beta.2`).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
…o reagan/fix-duplicate-instance-opening
The version-aware handoff depended on a newer app process reliably reaching Electron's second-instance path, which is not guaranteed for normal macOS app launches. Keep duplicate launches simple: the lock loser exits, and the primary instance restores or recreates the appropriate window. Constraint: macOS may activate the existing app instead of starting a second packaged app process Rejected: Newer-binary handoff | relies on launch behavior we cannot prove with unit tests Confidence: high Scope-risk: narrow Directive: Keep duplicate-launch handling version-agnostic unless packaged two-version smoke tests cover takeover behavior Tested: npm run typecheck Tested: npm run lint Tested: npm run test
Collaborator
Author
|
@cubic review this entire pr pls |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/index.ts">
<violation number="1" location="app/src/main/index.ts:46">
P1: This change removes newer-instance handoff and always keeps the currently running instance, so manual installs/updates cannot take over when an older process is already open.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic.
| // start, surface the existing window instead. | ||
| // Enforce a single running instance. The lock loser exits, while the primary | ||
| // process handles `second-instance` by focusing or recreating its main window. | ||
| if (!app.requestSingleInstanceLock()) { |
There was a problem hiding this comment.
P1: This change removes newer-instance handoff and always keeps the currently running instance, so manual installs/updates cannot take over when an older process is already open.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/index.ts, line 46:
<comment>This change removes newer-instance handoff and always keeps the currently running instance, so manual installs/updates cannot take over when an older process is already open.</comment>
<file context>
@@ -48,22 +41,9 @@ crashReporter.start({
-if (!app.requestSingleInstanceLock(singleInstanceLaunchData)) {
+// Enforce a single running instance. The lock loser exits, while the primary
+// process handles `second-instance` by focusing or recreating its main window.
+if (!app.requestSingleInstanceLock()) {
app.exit(0);
}
</file context>
Tip: Review your code locally with the cubic CLI to iterate faster.
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
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.
…ndow already opened error".
also, when I need to update and double click a new dmg or something if I install a separate one, then it should auto-update current window.
Summary
Test plan
npm run lintpassesnpm run typecheckpassesnpm run testpasses (unit + integration)src/main/src/sharedmodules)pytest+ruff check+mypypassnpm run devfor UI-facing changesScreenshots / recordings
Risk + rollback
Summary by cubic
Stops the “window already opened” alert on duplicate launches. Second launches now exit, and the primary instance focuses an existing window or recreates the right one.
Bug Fixes
electron’sapp.requestSingleInstanceLock; lock loser callsapp.exit(0).second-instance, focus the best available window (shell, onboarding, or last focused); if none exist, open shell or create onboarding based on account state.Refactors
Written for commit 7e41114. Summary will update on new commits.