Repository navigation
Carry the enforced budget in sandbox timeout errors and steer agents to workflows - #1298
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughSandbox timeout errors now include enforced budgets. Executor parsing returns structured timeout details and workflow guidance. Sentry filtering recognizes annotated and legacy timeout messages while excluding wrapped or unrelated errors. ChangesExecutor timeout handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
🔎 Preview deployed: https://kody-pr-1298.kody-a99.workers.dev Worker: Mocks:
|
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
…t-workflow-hint-6e77 # Conflicts: # packages/worker/src/mcp/executor.ts Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Intent
Agents already get a proactive hint on the
executetool description that the sandbox has a ~90s hard timeout and that durableworkflows.createis the right tool for longer work — but when a call actually timed out, the agent got no next step. The timeout message also had a source-ambiguity problem: the same string was emitted by ad hoc execute (90s), workflow-step sandboxes (270s), and package services (300s), so downstream consumers could not tell which budget was actually exceeded.Summary
Execution timed out after 90s: Kody stopped observing the sandbox. …. The executor is the single component that knows the budget when the timeout fires, and the message string is the only part of the result that survives every boundary the error crosses (dynamic worker RPC, D1-persisted invocation responses,UserCodeErrorrethrows) —ExecuteResult.erroris typedstringby@cloudflare/codemode, so structured fields cannot ride along.mainrecently added to the message (fix(runtime): harden ad hoc execute and package invoke #1309):executorSandboxTimeoutMessageis now built from an exported prefix (Execution timed out) plus the explanation, and the budget slots between them.execution_timed_outcase inExecutionErrorDetails(packages/worker/src/mcp/executor.ts), matched from the timeout message (prefix-tolerant for[execution_failed]-wrapped nested causes, anywhere in the error cause chain, and backward compatible with legacy budget-less and explanation-less forms). It carries the parsedtimedOutAfterMsand aNext step:telling the agent not to retry the identical call and to submit genuinely long-running work as a durable workflow (workflows.createfromkody:runtime, progress viaworkflow_run_list) or split the work — reporting the actual exceeded budget instead of assuming 90s.isExecutorSandboxTimeoutSentryEventinpackages/worker/src/sentry-options.ts) accepts the budget and treats the explanation as optional while staying anchored, so wrapped rethrows (Error: Execution timed out…) still reach Sentry and legacy bare-form timeouts from older deployments stay muted.formatExecutionOutput) andstructuredContent.errorDetails.docs/use/execute.mdnotes the budget annotation.Testing
npm run validatepasses locally after mergingmain(lint, typecheck, format, build, 588 unit test files / 2038 tests across Node and Workers projects, e2e).packages/worker/src/mcp/executor.node.test.tscovers: real sandbox timeouts returning the budget-annotated message (host-deadline hang, permit-queue expiry, and abort-aware dispatcher paths), budget extraction for 90s and 270s messages, legacy budget-less and bare pre-explanation forms, a nested[execution_failed]-prefixed cause, and a non-matching mid-sentence mention.packages/worker/src/sentry-options.node.test.tscovers: budget-annotated messages stay filtered (bothsandmsspellings), the legacy bare form stays filtered, andError:-prefixed rethrows still pass through.System changes
System recap — extends one primitive (medium risk)
Mode: recap · Base:
main@d7f51175· Head:088bf1b8Classification: extends — the MCP endpoint's execute error contract changes (timeout messages gain an injected budget; a new structured error kind is added); no primitives added, no other primitives touched.
Primitives touched
mcp-serverexecution_timed_outkind inExecutionErrorDetailsSystem map
Sandbox timeouts now carry their enforced budget in the message, and the execute error formatter turns that into a structured next step pointing at durable workflows.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Before / after
Agent-visible output when an execute call hits the sandbox deadline:
Before:
After:
A workflow-step sandbox cut reports
after 270s; legacy budget-less messages (older stored responses) still classify withtimedOutAfterMs: nulland the generic ~90s wording.Summary by CodeRabbit
Bug Fixes
Documentation