fix: stop killing goosed when a window closes - #9302
Conversation
Closing a BrowserWindow killed its dedicated goosed process, severing
sessions for all windows sharing that backend. The app.on('will-quit')
handler already calls goosedResult.cleanup() for every spawned process,
so the per-window kill was redundant and harmful.
Fixes #8711
Signed-off-by: Douwe Osinga <douwe@squareup.com>
There was a problem hiding this comment.
💡 Codex Review
Lines 1133 to 1135 in 756faf6
Removing the goosedProcess.kill() path from the closed handler leaves each chat window's spawned backend alive until full app exit, but createChat() still starts a fresh backend for every new window. In environments where closing windows does not quit the app (e.g. macOS per window-all-closed logic in ui/desktop/src/main.ts), repeatedly closing/reopening chat windows accumulates orphaned goosed processes and listeners, which can degrade performance and eventually exhaust system resources.
ℹ️ 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".
* main: (70 commits) Feat/summon subagent instructions (#9325) feat: open-plugins generalization + skills (#9112) feat(hooks): PreToolUse denial (#9304) Add support for optional api_key configuration for declarative openai-engine providers (#9202) fix(cli): use plain '> ' prompt instead of goose emoji (#9305) flag for login shell PATH (#9313) Remove popular chat topics from new chat screen (#9307) fix: stop killing goosed when a window closes (#9302) Remove vendored Windows binaries (#9318) Add Linux musl CLI builds (#9240) feat(acp): paginate session list (#9199) docs: reorganize (#9310) Structured per-provider config block, non-destructive provider switching (#8977) feat(cli): add `goose review` local code review command (#9114) feat(tui): diff viewer (#9260) fix(otel): emit trace_output as span attribute instead of event (#9255) docs: add guide for connecting goose Desktop to a remote goosed server (#9275) fix(config): check file fallback when keyring has no entry (#9279) fix(desktop): ScheduleModal error message styling (#9278) fix(ui): align sidebar hamburger in macOS fullscreen (#9257) ...
* main: (38 commits) [Prompt injection mitigation] Update pattern-based detection to reduce FPs (#9350) feat: add Harbor eval runner (#9138) chore(release): bump version to 1.35.0 (minor) (#9150) Include request URL in provider error messages (#9232) fix(databricks): ensure parallel tool image responses don't interleave tool results (#9241) Surface resolved Databricks model metadata (#9206) Add unified thinking effort control across all providers (#9242) Add Linux desktop Vulkan packages (#9323) chore: update canonical model registry (#9331) feat: slash commands (built-in, skill, recipe) in acp server (#9238) feat: add /goal command for agent self-evaluation before finishing (#9069) Feat/summon subagent instructions (#9325) feat: open-plugins generalization + skills (#9112) feat(hooks): PreToolUse denial (#9304) Add support for optional api_key configuration for declarative openai-engine providers (#9202) fix(cli): use plain '> ' prompt instead of goose emoji (#9305) flag for login shell PATH (#9313) Remove popular chat topics from new chat screen (#9307) fix: stop killing goosed when a window closes (#9302) Remove vendored Windows binaries (#9318) ...
Signed-off-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Douwe Osinga <douwe@squareup.com>
Summary
Closing a
BrowserWindowkilled its dedicatedgoosedprocess viagoosedProcess.kill()in themainWindow.on('closed')handler. This severed sessions for users who close one window while others are still open, causing the "session error" failures described in the issue.The
app.on('will-quit')handler already callsgoosedResult.cleanup()for every spawned goosed process (with graceful SIGTERM + 5s SIGKILL fallback), so the per-window kill was redundant and harmful.Changes
goosedProcess.kill()from themainWindow.on('closed')handler inui/desktop/src/main.tsprocess: goosedProcessdestructuring fromgoosedResultTesting
pnpm lintpasses cleanFixes #8711