Skip to content

Conversation

@angelahning
Copy link
Collaborator

@angelahning angelahning commented Sep 10, 2025

Addresses #4125

Screen.Recording.2025-09-10.at.3.10.37.PM.mov

@angelahning angelahning marked this pull request as ready for review September 11, 2025 18:28
@angelahning angelahning requested a review from jamadeo September 11, 2025 21:30
&self,
notification: CancelledNotificationParam,
_context: NotificationContext<RoleServer>,
) -> impl Future<Output = ()> + Send + '_ {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can change this fn to

async fn on_cancelled(
    ...
) {

and then remove the async move { part

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had that before, but on_cancelled wasn't getting triggered.

#[tool(
name = "shell",
description = "Execute a command in the shell. Returns output and error concatenated. Avoid commands with large output, use background commands for long-running processes."
description = "Execute a command in the shell.This will return the output and error concatenated into a single string, as you would see from running on the command line. There will also be an indication of if the command succeeded or failed. Avoid commands that produce a large amount of output, and consider piping those outputs to files. If you need to run a long lived command, background it - e.g. `uvicorn main:app &` so that this tool does not run indefinitely."
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this from another change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The previous rmcp refactor trimmed the tool description. I am just adding it back.

tokio::time::sleep(tokio::time::Duration::from_millis(1000)).await;

// Force kill with SIGKILL
let _sigkill_result = unsafe { libc::kill(-(pid as i32), libc::SIGKILL) };
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you need this if you always call child.kill() on the tokio process?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this kills the entire process group, vs child.kill() just kills the main process.

@angelahning angelahning merged commit 8198e1b into main Sep 22, 2025
11 checks passed
@angelahning angelahning deleted the aning/cancelfr branch September 22, 2025 21:19
michaelneale added a commit that referenced this pull request Sep 23, 2025
* main: (26 commits)
  Compact session automatically for streaming providers on Context Length Exceeded (#4565)
  When the developer extension gets a cancellation message, it should kill any running processes that it owns. (#4604)
  Remove some unused stuff (#4388)
  Add I Ching MCP to extension catalog (#4525)
  Offer to summarize or clear conversation when it has gotten too long … (#4688)
  refactor(forge): employ desktopTemplate to support absolute paths (#4084)
  fix(nix): add nixpkgs missing for existing build process & useful in dev process (#4490)
  feat(acp): add nicer formatting for tool names (#4686)
  fix gui client initialization when using an external backend (#4669)
  feat(acp): upgrade agent-client-protocol + improve tool experience (#4683)
  blog post about using subrecipes (#4647)
  fix: improve oneDark theme for comment contrast and readability (#4655)
  fix: dictation provider dropdown getting cut off (#4673)
  fix: Only metric a recipe run from ui on the first /reply call (#4679)
  fix: Don't count recipe explain or render as recipe usage (#4678)
  More robust test finder patch handling (#4667)
  fix(compaction): try to catch more context limit exceeded errors and compact (#4656)
  chore(mcp): convert memory mcp server to use the rust sdk for mcp (#4668)
  chore(mcp): convert tutorial mcp server to use the rust sdk for mcp (#4665)
  test: add test coverage for RepetitionInspector::check_tool_call (#4666)
  ...
@alexhancock alexhancock mentioned this pull request Sep 23, 2025
zanesq added a commit that referenced this pull request Sep 23, 2025
…-unification

* 'main' of github.com:block/goose:
  temp file for batch issue creation
  fix: view can recognise a dir (#4701)
  goosed standalone works with providers (#4698)
  Compact session automatically for streaming providers on Context Length Exceeded (#4565)
  When the developer extension gets a cancellation message, it should kill any running processes that it owns. (#4604)
  Remove some unused stuff (#4388)
  Add I Ching MCP to extension catalog (#4525)
  Offer to summarize or clear conversation when it has gotten too long … (#4688)
  refactor(forge): employ desktopTemplate to support absolute paths (#4084)
  fix(nix): add nixpkgs missing for existing build process & useful in dev process (#4490)
katzdave added a commit that referenced this pull request Sep 24, 2025
…ovements

* 'main' of github.com:block/goose:
  Remove now unused mcp-server crate (#4773)
  Release/1.9.0 (#4703)
  chore(mcp): convert computercontroller server to use the rust sdk (#4772)
  Docs: Delete sessions from UI and edit has changed (#4785)
  Don't load user's shell env on app startup (#4681)
  Docs: Chrome Dev Tools Extension Tutorial (#4783)
  Add Hacktoberfest 2025 Leaderboard Workflow (#4776)
  [docs] Add gotoHuman MCP server Tutorial (#4764)
  fix: adjust nightly builds to first push a tag (#4704)
  temp file for batch issue creation
  fix: view can recognise a dir (#4701)
  goosed standalone works with providers (#4698)
  Compact session automatically for streaming providers on Context Length Exceeded (#4565)
  When the developer extension gets a cancellation message, it should kill any running processes that it owns. (#4604)
  Remove some unused stuff (#4388)
  Add I Ching MCP to extension catalog (#4525)
  Offer to summarize or clear conversation when it has gotten too long … (#4688)
HikaruEgashira pushed a commit to HikaruEgashira/goose that referenced this pull request Oct 3, 2025
…ill any running processes that it owns. (block#4604)

Signed-off-by: HikaruEgashira <[email protected]>
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.

3 participants