Skip to content
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

Fix background process termination on stop button #1090

Closed

Conversation

jasonkneen
Copy link

@jasonkneen jasonkneen commented Feb 5, 2025

Fixes #1062

Add logic to terminate background processes when the stop button is clicked.

  • ui/desktop/src/ChatWindow.tsx

    • Add logic to terminate background processes in the onStopGoose function.
    • Update the stop function to include background process termination.
  • ui/desktop/src/ai-sdk-fork/call-custom-chat-api.ts

    • Add a stop function to terminate background processes.
  • ui/desktop/src/main.ts

    • Update the stop-power-save-blocker function to stop background processes.

For more details, open the Copilot Workspace session.

Fixes block#1062

Add logic to terminate background processes when the stop button is clicked.

* **ui/desktop/src/ChatWindow.tsx**
  - Add logic to terminate background processes in the `onStopGoose` function.
  - Update the `stop` function to include background process termination.

* **ui/desktop/src/ai-sdk-fork/call-custom-chat-api.ts**
  - Add a `stop` function to terminate background processes.

* **ui/desktop/src/main.ts**
  - Update the `stop-power-save-blocker` function to stop background processes.

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/block/goose/issues/1062?shareId=XXXX-XXXX-XXXX-XXXX).
@@ -90,3 +90,12 @@ export async function callCustomChatApi({
}
}
}

export function stop() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already exported from the ai-sdk-fork here and we use it here.

It does abort the request to the goosed server but I suspect the child processes are not properly stopped. We need to investigate it on the rust side.

@@ -189,6 +189,9 @@ export function ChatContent({
const updatedMessages = [...messages.slice(0, -1), newLastMessage];
setMessages(updatedMessages);
}

// Terminate any background processes
window.electron.terminateBackgroundProcesses();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to be defined

@alexhancock
Copy link
Collaborator

The frontend is already aborting the request properly. The agent side needs to reap the child processes when the connection is cut off

// Kill any running processes when the client disconnects
// TODO is this used? I suspect post MCP this is on the server instead
// goose::process_store::kill_processes();

Will look into why this was disabled

@alexhancock alexhancock closed this Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stopping GUI Goose via the stop button does not terminate background processes
2 participants