fix(desktop): reuse existing window for recipe deep links#9289
fix(desktop): reuse existing window for recipe deep links#9289SarthakB11 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e6743c2e0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const newSession = await createSession(getInitialWorkingDir(), { | ||
| recipeDeeplink, | ||
| allExtensions: extensionsList, |
There was a problem hiding this comment.
Pass the recipe config instead of the full URL
When a goose://recipe?... link is opened into an existing window, this passes the full URL to createSession, but createSession forwards recipeDeeplink directly to decodeRecipe, whose backend decoder expects only the base64 recipe config string (the cold-start path still passes deeplinkData?.config). As a result, any recipe/bot deep link handled through this new IPC path fails to decode instead of opening the recipe; extract and pass the config value here, or update the decode path to accept full Goose URLs.
Useful? React with 👍 / 👎.
Signed-off-by: Douwe Osinga <douwe@squareup.com> # Conflicts: # ui/desktop/src/main.ts
|
I merged During conflict resolution, I also fixed a bug that Codex flagged: the IPC payload was sending the full The fix: Items for human reviewer to check:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76ead475d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const newSession = await createSession(getInitialWorkingDir(), { | ||
| recipeDeeplink, | ||
| allExtensions: extensionsList, | ||
| }); |
There was a problem hiding this comment.
Forward deeplink recipe parameters into session creation
This handler receives recipeParameters/scheduledJobId in the IPC payload but only passes recipeDeeplink to createSession, so recipe links opened in an already-running window lose their query-parameter context. That regresses behavior compared with the new-window path (which injects recipeParameters via appConfig) and means parameterized recipe links open without prefilled values in the parameter modal.
Useful? React with 👍 / 👎.
| targetWindow.webContents.send('open-recipe-deeplink', { | ||
| recipeDeeplink: deeplinkData?.config, | ||
| recipeParameters: deeplinkData?.parameters, | ||
| scheduledJobId: scheduledJobId || undefined, | ||
| }); |
There was a problem hiding this comment.
Defer recipe deeplink IPC until renderer is ready
The main process sends open-recipe-deeplink as soon as it finds any window, but in the startup path handleProtocolUrl can create a new window and then immediately hit this branch before AppInner has registered its listener. In that case the IPC is dropped and the deeplink is effectively ignored, so recipe launch can silently fail. This should be queued/deferred until the target window reports readiness (as done for other pending deep links).
Useful? React with 👍 / 👎.
|
can you look at the codex comments |
|
Thanks @DOsinga, your On the two Codex items: P2 — P1 — IPC race when Pushing both shortly. |
…derer is ready Two follow-ups on aaif-goose#9143 per Codex review: - recipeParameters URL overrides were sent in the IPC payload but the renderer dropped them. Introduce a small per-session store in ui/desktop/src/utils/recipeParametersStore.ts that App.tsx writes from the warm-launch IPC handler and ParameterInputModal reads as initial values (falling back to window.appConfig.recipeParameters for the cold-launch path that seeds it at preload). - The bot/recipe IPC could race the renderer's listener when handleProtocolUrl created a fresh window on the same tick. Reuse the existing pendingDeepLinks/react-ready pattern: if the chosen window is still loading, queue the URL and dispatch the recipe IPC from the react-ready handler the same way extension/sessions deeplinks are already replayed. Signed-off-by: SarthakB11 <sarthak.bhardwaj21b@iiitg.ac.in>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de72a03d65
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| onSubmit={setRecipeUserParams} | ||
| onClose={() => setView('chat')} | ||
| initialValues={ | ||
| takeRecipeParametersForSession(chat.sessionId) || |
There was a problem hiding this comment.
Keep deeplink parameters stable while the modal is open
When a parameterized recipe link is opened into an existing window, this consumes and deletes the stored values during BaseChat render. On the next parent re-render while ParameterInputModal is still mounted, initialValues becomes undefined; because the modal's prefill effect depends on initialValues, it resets the form back to defaults and drops the URL-supplied values (and possibly user edits). The values should be read once into stable state/ref for the session instead of calling the destructive take... in the render path.
Useful? React with 👍 / 👎.
Summary
Clicking a
goose://recipe/...orgoose://bot/...deep link while Goose Desktop is already running was spawning a freshBrowserWindowevery time instead of routing the deeplink into the focused window. Three protocol entry points inui/desktop/src/main.ts(app.on('second-instance'),processProtocolUrl,app.on('open-url')) each calledcreateChat(app, { recipeDeeplink, ... })unconditionally.This adds an
openRecipeDeeplink(url, openDir)helper next toprocessProtocolUrlinui/desktop/src/main.ts. It picks the most-recently-focusedBrowserWindow(falling back to the newest open window, then the oldest), restores and focuses it, and forwards the deeplink as anopen-recipe-deeplinkIPC payload (recipeDeeplinkURL string,recipeParameters,scheduledJobId). When no window exists it falls back tocreateChat(...)and threadswindowDeeplinkURLso the cold-start dir-picker path keeps its prior behavior. All three protocol call sites now go through the helper.On the renderer (
ui/desktop/src/App.tsx), a newopen-recipe-deeplinkIPC listener mirrors the existingrecipeDeeplinkFromConfigbranch inPairRouteWrapper: it callscreateSession(getInitialWorkingDir(), { recipeDeeplink, allExtensions }), dispatchesADD_ACTIVE_SESSION, and navigates to/pair?resumeSessionId=.... The payload sends the raw deeplink URL string so the renderer can route through the samedecodeRecipepath used at cold launch — the IPC contract stays compatible withcreateSession's existingrecipeDeeplink: stringsignature.Reuse is unconditional rather than opt-in; matches the behavior of the surrounding
goose://extension/goose://sessionsdeeplinks.Testing
Manual only. The change is pure routing in the Electron main process plus a new renderer-side IPC listener; there is no existing harness around
main.tsdeeplink dispatch. Reviewer test path: with Goose Desktop running, triggergoose://recipe/...from an external process and confirm the recipe loads as a new session in the focused window instead of opening a new one. Worth a quick check on both macOS (open-url) and Linux/Windows (second-instance).Related Issues
Closes #9143
Screenshots/Demos (for UX changes)
N/A