-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix deeplink recipe launch cold start #6210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
ui/desktop/src/main.ts
Outdated
| } | ||
| const scheduledJobId = parsedUrl.searchParams.get('scheduledJob'); | ||
|
|
||
| // Wait for app to be ready before creating window (critical for cold start) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd nuke the comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
goose run --text "look at the current branch and compare with main. remove any comments that were introduced that were introduce that just say what the code underneath is doing, i.e. commenting on what or how and not on why."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure will do, actually I did review these and they seem relevant for the cold start edge case for future devs
…ased * 'main' of github.com:block/goose: (23 commits) Use Intl.NumberFormat for token formatting in SessionsInsights (#6466) feat(ui): format large and small token counts for readability (#6449) fix: apply subrecipes when using slash commands (#6460) Fix: exclude platform_schedule_tool in CLI (#6442) Fix: Small update in how ML-based prompt injection determines final result (#6439) docs: remove SSE transport and rename to Streamable HTTP (#6319) fix: correct Cloudinary extension command and env variable (#6453) fix: add gap between buttons in MacDesktopInstallButtons.js (#6452) refactor: include hidden dotfiles folders in file picker search (#6315) upgraded safe npm packages (#6450) chore(deps): bump react-router and react-router-dom in /ui/desktop (#6408) chore(deps): bump lru from 0.12.5 to 0.16.3 (#6379) chore(deps-dev): bump @modelcontextprotocol/sdk from 1.24.0 to 1.25.2 in /ui/desktop (#6375) fix: inconsistent API url requirement between desktop and CLI versions (#6419) feat(vertexai): Add streaming support (#6409) fix deeplink recipe launch cold start (#6210) Spell check setting (#6446) File bug directly (#6413) fix(cli): incorrect bin name in shell completions (#6444) Use crunchy from crates instead of git fork (#6415) ...
Summary
Fix deep link handling on cold start (app not running) for extension, session, and recipe deep links.
Also noticed deeplinks stopped launching from cold start after nextcamp.
Goose created this but I wasn't able to test cold start launch it would only launch my goose app in applications and copying to Applications wasn't getting around it so we'll need to see after the next release. Verified deeplinks still open when app is running so there should be no regression here only hopeful improvement :)
fixes #3972
Problem
When clicking a
goose://deep link while Goose is not running:open-urlhandler andappMain()would create windowsRoot Cause
Two issues in the
open-urlevent handler:Missing
app.whenReady()call: On macOS,open-urlcan fire before the app is ready. The handler was callingcreateChat()(which creates aBrowserWindow) without waiting for the app to be ready, causing the window creation to fail silently.Race condition for IPC messages: For extension/session deep links, IPC messages (
add-extension,open-shared-session) were sent immediately after window creation, before React had mounted and registered its listeners. The messages were lost.Solution
await app.whenReady()in theopen-urlhandler before creating any windows (critical fix for cold start)openUrlHandledLaunchflag to track whenopen-urlhas already created a window for a deep linkopen-urlhandler to defer IPC messages for extension/session deep links until React is ready (stored inpendingDeepLink)appMain()to skip window creation ifopenUrlHandledLaunchis truereact-readyIPC handler to processpendingDeepLinkby sending the appropriate IPC message to the now-ready windowResult