fix(sdk): stop killing agent runs on a 180s silent tool call - #6982
Aman-goel-04 wants to merge 4 commits into
Conversation
|
✅ Thanks @Aman-goel-04! This PR now meets the contribution requirements and has been reopened. A maintainer will review it soon. |
github-actions
Bot
added
the
incomplete-pr
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (17)
🚧 Files skipped from review as they are similar to previous changes (5)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe PR raises the shared runner timeout to 1920 seconds, aligns related documentation and environment examples, reports stalled stream errors without internal details, and propagates pre-admission failure reasons into chat refusal messages. ChangesRunner timeout handling
Chat refusal reason handling
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant Runner
participant deliver_http_stream
participant SandboxAgentBackend
Runner->>deliver_http_stream: emit stream records
deliver_http_stream->>SandboxAgentBackend: report completed or stalled stream
SandboxAgentBackend->>Runner: return result or transport error
sequenceDiagram
participant RunStream
participant useServerSessionInputs
participant useAgentChatQueue
participant LiveConversation
RunStream->>useServerSessionInputs: send error frame
useServerSessionInputs->>useAgentChatQueue: call onFailed(reason)
useAgentChatQueue->>LiveConversation: restore refused send with reason
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 22.22% which is insufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 15 files. (5 skipped: 5 unsupported.)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The changes are tested and no blocking issues remain; one documentation nit is non-blocking.
Review effort: Lite
Findings: None
What changed in this PR
Updates SDK runner timeouts to prevent silent tool calls from failing after 180 seconds and improves stalled-stream diagnostics.
Changes:
- Centralizes the 1920-second default timeout.
- Adds explicit HTTP stream stall errors.
- Updates tests, deployment examples, and documentation.
| File | Summary |
|---|---|
sdks/python/oss/tests/pytest/unit/agents/test_runner_transport_timeout.py |
Adds timeout regression coverage. |
sdks/python/oss/tests/pytest/integration/agents/_fake_runner_backend.py |
Aligns fake backend timeout defaults. |
sdks/python/agenta/sdk/agents/utils/ts_runner.py |
Defines shared timeout and handles stream timeouts. |
sdks/python/agenta/sdk/agents/utils/__init__.py |
Exports the shared timeout constant. |
sdks/python/agenta/sdk/agents/adapters/sandbox_agent.py |
Uses the shared timeout default. |
hosting/docker-compose/oss/env.oss.gh.example |
Updates timeout example. |
hosting/docker-compose/oss/env.oss.dev.example |
Updates timeout example. |
hosting/docker-compose/ee/env.ee.gh.example |
Updates timeout example. |
hosting/docker-compose/ee/env.ee.dev.example |
Updates timeout example. |
docs/docs/self-host/reference/01-configuration.mdx |
Updates configuration documentation. |
docs/design/agent-workflows/projects/runner-interface/README.md |
Updates runner timeout documentation. |
docs/design/agent-workflows/interfaces/in-service/backend-adapter.md |
Updates adapter documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
github-actions
Bot
removed
the
incomplete-pr
|
qa found one issue on 329b24f. the primary fix works: a silent shell tool completed after 181 seconds instead of being cut off at the old 180-second boundary. video proof: https://www.browserbase.com/sessions/0e8687c2-990d-4681-a4e4-5a89f47d0772 the new stall detail does not reach the user. with a 3-second test timeout, services logged `Agent runner stream stalled: no record for 3.0s`, but the browser only showed `Message wasn't sent — try again.` video proof: https://www.browserbase.com/sessions/db9602e1-6df2-4ed3-9653-c653e4c45d27 please make the sanitized stream error visible in the failed turn and add regression coverage for that path. |
9a3d336 to
977762f
Compare
|
Thanks for the QA. Found it: with the 3s timeout the stall hits before the runner admits the turn, so the web treats the error frame as a refused send and throws away its text. That's why you only saw "Message wasn't sent". What I changed:
Tests: the reason is passed on for a pre-turn error (both frame types), no reason when the frame has no text, and an error after the turn id still goes to the transcript. The queue forwards the reason to the composer, lateRefusedSendRejections is covered in sendRefusal.test.tsx, and the timeout tests check the URL and command stay out of the message. I also merged main, since the branch had conflicts. Two questions:
I couldn't reproduce it in the browser, so it's only covered by unit tests. Could you re-run your 3s scenario when you get a chance? |
Fixes #6976
Summary
A tool call that stayed silent for more than 180 seconds killed the whole agent run with a generic "agent run failed", even though the runner has much wider limits (30 min idle, 30 min per tool call).
Root cause:
SandboxAgentBackenddefaultedAGENTA_RUNNER_TIMEOUT_SECONDSto 180 with its own literal, overriding the wider default ints_runner.py. httpx applies that float as a per-read idle timeout on the NDJSON stream, so any 180s gap between frames raisedhttpx.ReadTimeout. That exception has an empty message, so the wire layer fell back to "agent run failed".Changes:
AGENT_DEFAULT_TIMEOUT(1920s, above the runner's 30 min limits so the runner always trips first), exported fromagenta.sdk.agents.utilsand used by the adapter and the fake test backend. The duplicate 180 literal is gone.deliver_http_streamnow catcheshttpx.ReadTimeoutand raises a named "Agent runner stream stalled: no record for Ns" error, matching the subprocess transport's wording.Trade-off: a genuinely dead runner connection now takes about 32 minutes to give up on instead of 3. This is the runner-as-authority design, where the runner emits a terminal record when its own limits trip.
Not included: a runner-side heartbeat frame during silent tool calls (a larger runner change).
Testing
Verified locally
pytest oss/tests/pytest/unit/agents/test_runner_transport_timeout.py: 6 passed.ruff format --checkis clean on the changed Python files.Added or updated tests
ReadTimeoutin the HTTP stream produces the named "stalled" error.QA follow-up
N/A
Demo
main:
this branch:
Checklist