Skip to content

fix: preserve fatal tool execution propagation - #108

Merged
delete-cloud merged 6 commits into
mainfrom
fix/adr-0018-session-manager-fatal-propagation
May 5, 2026
Merged

delete-cloud merged 6 commits into
mainfrom
fix/adr-0018-session-manager-fatal-propagation

Conversation

@delete-cloud

@delete-cloud delete-cloud commented May 5, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • preserve FatalToolExecutionError through the pipeline stage wrapper, subagent summary/child-run paths, and SessionManager.run_agent() so generic fatal tool failures are not downgraded into generic pipeline/session errors
  • keep /sessions/{session_id}/prompt on the existing fail-fast contract by surfacing mid-turn fatal tool failures as SSE Error events instead of fake assistant StreamDelta plus parent TurnEnd(ERROR) output
  • update the ADR-0018 PR4 task packet to match the actual pipeline + subagent + session/http slice

Verification

  • uv run pytest tests/agentkit/runtime/test_pipeline.py tests/coding_agent/tools/test_subagent.py tests/ui/test_session_manager_runtime.py tests/ui/test_http_server.py tests/ui/test_http_server_failover.py tests/ui/test_session_manager_public_api.py -v
  • uv run python -m compileall -q src tests/agentkit/runtime tests/coding_agent/tools tests/ui
  • lsp_diagnostics error check on all changed Python files
  • manual HTTP QA via a live uvicorn server:
    • direct fatal tool path -> SSE Error(fatal tool failure) only
    • real subagent fatal-summary-publish path -> child ToolCallDelta/child output followed by SSE Error(fatal summary publish rejected) with no fake parent error turn

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Fatal tool execution errors now properly propagate through the entire pipeline and surface as error events in HTTP streams.
    • Session runtime state is correctly cleaned up when fatal tool errors occur during execution.
    • Improved subagent error handling to prevent fatal errors from being suppressed.
  • Tests

    • Added comprehensive test coverage for fatal tool error propagation and state cleanup across pipeline, session manager, and HTTP endpoints.

delete-cloud and others added 5 commits May 5, 2026 14:08
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
@coderabbitai

coderabbitai Bot commented May 5, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@delete-cloud has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 38 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d249858d-8cc5-43d0-96f1-b5de576cbb1e

📥 Commits

Reviewing files that changed from the base of the PR and between 522c4e1 and 49659e7.

📒 Files selected for processing (1)
  • coding-agent/tests/agentkit/runtime/test_pipeline.py
📝 Walkthrough

Walkthrough

This PR ensures FatalToolExecutionError is preserved and re-raised end-to-end through the pipeline, subagent tool dispatch, session manager, and HTTP SSE streaming layer, replacing prior handling of SessionOwnershipConflictError with fatal tool execution error semantics. An ADR task document specifies the requirements, while implementation and comprehensive test coverage span all affected layers.

Changes

Fatal Error Propagation Through Core Pipeline Layers

Layer / File(s) Summary
Design Specification
.opencode/prompts/tasks/adr-0018-pr4-session-manager-fatal-propagation.md
ADR task document specifies required FatalToolExecutionError re-raise points across Pipeline, subagent summary publishing, and SessionManager.run_agent(), mandates SSE Error event surfacing for mid-turn fatal tool errors, and lists target test commands.
Pipeline Exception Handling
src/agentkit/runtime/pipeline.py
Pipeline stage execution loop adds dedicated except FatalToolExecutionError: raise handler, preserving fatal tool failures rather than converting to PipelineError.
Subagent Dispatch & Publishing
src/coding_agent/tools/subagent.py
Subagent summary publishing and child run_turn() execution now catch and re-raise FatalToolExecutionError (replacing prior SessionOwnershipConflictError), ensuring fatal failures from child pipelines escape to parent.
Session Manager Turn Handling
src/coding_agent/ui/session_manager.py
Session manager's turn-level exception handler now catches FatalToolExecutionError (replacing SessionOwnershipConflictError), enabling proper cleanup and re-raising without issuing an error turn.
Pipeline Layer Tests
tests/agentkit/runtime/test_pipeline.py
Introduces FatalExecuteToolPlugin and test_run_turn_reraises_fatal_tool_execution_error_from_batch_hook, verifying pipeline re-raises fatal tool execution errors from batch hook invocation.
Subagent & Toolset Layer Tests
tests/coding_agent/tools/test_subagent.py
Adds three tests covering fatal subagent summary publish failures (direct tool and via Toolset) and fatal errors from child run_turn, confirming exceptions propagate while child adapters are closed.
Session Manager Runtime Tests
tests/ui/test_session_manager_runtime.py
New test test_run_agent_reraises_fatal_tool_execution_error_without_sending_error_turn verifies SessionManager.run_agent() re-raises the fatal error, clears cached runtime state, and does not emit an error turn delta.
HTTP SSE Integration Tests
tests/ui/test_http_server.py
Introduces two SSE tests: fatal tool execution errors and fatal subagent summary publish failures both surface as "Error" SSE events (not as fake assistant deltas or TurnEnd(ERROR)), confirming end-to-end SSE contract compliance.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 A fatal tool error hops through the pipeline clean,
No more caught by session conflicts in between,
From pipeline to subagent, up to SSE streams so bright,
Each layer reraises true—errors flow just right! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.35% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: preserving FatalToolExecutionError propagation through the pipeline, subagent, and session manager layers.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/adr-0018-session-manager-fatal-propagation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@coding-agent/tests/agentkit/runtime/test_pipeline.py`:
- Around line 132-160: The test's coverage is incorrect because the helper
plugin FatalExecuteToolPlugin only implements execute_tool (and get_tools) but
the test claims to cover the batch-hook path; update the plugin or test so the
batch path is exercised: either add an execute_tools_batch method to
FatalExecuteToolPlugin that mirrors execute_tool and raises
FatalToolExecutionError for "fatal_tool", or change the test to call the
single-tool execution path explicitly (execute_tool) instead of
execute_tools_batch; ensure the test invokes the new execute_tools_batch hook
and still registers the tool via get_tools so execute_tools_batch is used during
the from_batch_hook assertion.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a8cf9c44-a95f-4bed-8759-121f9f9b3bc9

📥 Commits

Reviewing files that changed from the base of the PR and between be3b4e7 and 522c4e1.

📒 Files selected for processing (8)
  • coding-agent/.opencode/prompts/tasks/adr-0018-pr4-session-manager-fatal-propagation.md
  • coding-agent/src/agentkit/runtime/pipeline.py
  • coding-agent/src/coding_agent/tools/subagent.py
  • coding-agent/src/coding_agent/ui/session_manager.py
  • coding-agent/tests/agentkit/runtime/test_pipeline.py
  • coding-agent/tests/coding_agent/tools/test_subagent.py
  • coding-agent/tests/ui/test_http_server.py
  • coding-agent/tests/ui/test_session_manager_runtime.py

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
@delete-cloud
delete-cloud merged commit ae256a8 into main May 5, 2026
1 check passed
@delete-cloud
delete-cloud deleted the fix/adr-0018-session-manager-fatal-propagation branch May 5, 2026 07:26
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.

1 participant