[delete-cloud]

Summary

• drain queued ACP session updates before returning a terminal prompt response
• ignore stale ACP permission responses when the backend approval request is already gone
• surface invalid bash_run timeout values as validation errors instead of successful tool output

Tests

• uv run pytest tests/acp -q
• uv run pytest tests/coding_agent/tools/test_shell.py -q
• uv run ruff check src/coding_agent/acp/server.py src/coding_agent/tools/shell.py tests/acp/test_server.py tests/coding_agent/tools/test_shell.py

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of cancelled sessions to ensure pending messages are properly flushed and emitted.
  • Enhanced approval request error handling for edge cases.
  • Fixed timeout parameter validation to properly reject invalid values with clear error messages.

• Tests

  • Added test coverage for session prompt cancellation and error scenarios.
  • Added test coverage for timeout parameter validation.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR improves error handling in the ACP server by draining pending session updates on cancellation and silently handling unmatched approval responses, while moving shell tool timeout validation earlier to let validation errors propagate as exceptions rather than being caught and wrapped.

Changes

ACP Server Improvements

Layer / File(s)	Summary
Drain pending session updates on cancellation coding-agent/src/coding_agent/acp/server.py, coding-agent/tests/acp/test_server.py	New _drain_pending_session_updates helper consumes outgoing wire messages, filters out approval requests, converts remaining messages to session updates, and emits session/update notifications. When cancellation is detected via TurnEnd without agent_id, the server now drains these pending updates before returning the cancelled stop reason. Test verifies error chunks are drained and emitted before error response.
Silent approval response handling coding-agent/src/coding_agent/acp/server.py, coding-agent/tests/acp/test_server.py	handle_approval_request now returns silently if submit_approval_response returns None instead of raising a JsonRpcError. Test confirms that stale approval requests are submitted with approved: True and scope: "once".

Shell Tool Validation

Layer / File(s)	Summary
Timeout validation before exception handling coding-agent/src/coding_agent/tools/shell.py, coding-agent/tests/coding_agent/tools/test_shell.py	timeout_seconds computation moved outside the try/except block in bash_run, causing validation errors from _positive_int to propagate as exceptions instead of being caught and wrapped. Test verifies bash_run raises ValueError when passed invalid timeout values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• delete-cloud/Coding-Agent-Loop#384: Both PRs modify bash_run timeout validation in shell.py to change how validation errors are handled and propagated.

Poem

🐰 A rabbit hops through sessions true,
Draining updates, old and new,
Approvals silent, errors clear—
Timeouts fail fast, nothing to fear!
With validation moved just right,
The tool runs swift, the server's tight!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'fix(acp): handle stale Zed responses' is overly broad and does not clearly reflect the main changes. While it addresses one aspect (stale responses), the PR includes multiple distinct changes: draining session updates, handling missing approval requests, and fixing timeout validation.	Consider a more specific title that captures the primary change, such as 'fix(acp): drain session updates on prompt termination and handle stale responses' or break into separate PRs for clarity.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/acp-zed-error-streaming

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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