Repository navigation
Stop fiber tool workers when their parent is cancelled - #1328
Conversation
A parent task stopped while collecting tool results enters ensure and waits for workers that still wait on I/O. Stop those worker tasks before waiting, then preserve the original Async stop signal. Cover host and callback cancellation, unrelated sibling tasks, and cancellation before any worker has started. Fixes crmne#1324
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Stop handling and cancellation coverage lib/ruby_llm/chat/tool_concurrency.rb, spec/ruby_llm/chat/tool_concurrency_shutdown_spec.rb |
run_with_fibers stops created tasks when Async::Stop is raised, then re-raises it. Specs check worker cleanup, result reporting, unrelated task survival, and cancellation during queue creation. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related PRs
- crmne/ruby_llm#1247: Adds ordered and concurrent tool-call execution, including the fiber-worker path changed here.
Suggested labels: bug
Suggested reviewers: crmne
Merge Risk: 
Minimal · up to 14b21
-
The fiber-worker shutdown change appears mergeable after normal checks.
Pre-merge checks: 
6 · 
1
-
Failed checks: 1 warning Passed checks: 6 passed-
Full details: Linked Issues check
-
Explanation
Issue
#1324requires fiber-mode tool workers to stop when the surrounding Async task receives Async::Stop.run_with_fibersnow stops all createdtasksin anAsync::Stoprescue and re-raises. Its existingensurewaits for those tasks. The added specs verify host stopping, callback stopping, cleanup, no result reporting, sibling-task survival, and stopping before worker creation. Thread mode remains unchanged.
Full details: Out of Scope Changes check
-
Explanation
The changes are limited to the fiber worker shutdown path in
lib/ruby_llm/chat/tool_concurrency.rband focused regression specs inspec/ruby_llm/chat/tool_concurrency_shutdown_spec.rb. The implementation and tests directly support issue#1324. No unrelated behavior or files are shown.
Full details: Dependencies From Sources The Maintainer Controls
-
Explanation
The pull request changes only
lib/ruby_llm/chat/tool_concurrency.rband adds a spec file. It does not modify a manifest, lockfile, or package-manager configuration, and the patch adds no git URL, fork, patch, override, replace entry, or alternate registry.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
What this does
When the host stopped an
Asynctask that was collecting fiber tool results, the collector'sensurewaited for every worker before the stop could finish. Async stops a task's children only after its own fiber exits, so the workers kept running until each tool returned, and the stopped task stayed alive with them.The collector now rescues
Async::Stop, stops the workers it started for this call, and re-raises. The existingensurestill waits for those workers, so their ownensureblocks finish before the stop completes. This follows what Async does for a stopped task's children, earlier and only for this call's workers. Unrelated sibling tasks keep running. Other errors, includingCancelledErrorfromchat.cancel, and thread mode keep their current path.The new examples cover:
Async::Stopstops the remaining worker.How to reproduce
Run from the repository root with
bundle exec ruby -Ilib repro.rb. Nothing goes over the network.Before:
After:
Type of change
Scope check
Related issue
Fixes #1324
Quality check
overcommit --installand all hooks passbundle exec rake vcr:record[provider_name]bundle exec rspec --tag '~live' --tag '~generator'(6026 examples; the only 2 failures, inspec/ruby_llm/sandbox/mxc_spec.rb, fail the same way onmainon macOS)models.json,aliases.json)Ownership
API changes
Summary by CodeRabbit