Repository navigation
fix(browser): apply core's timeout rules to tool calls - #95
Merged
frontegg-david merged 2 commits intoOct 4, 2026
Merged
Conversation
|
Warning Review limit reachedYour organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Next included review available in 19 minutes. View limit detailsLimit details: You’ve used all 3 included reviews currently available. Your 43 included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (11)
Comment |
This was referenced Oct 3, 2026
frontegg-david
added this pull request to stack #97
October 3, 2026 23:55
frontegg-david
force-pushed
the
fix/browser-timeout-tool-calls
branch
from
October 3, 2026 23:56
1f3045c to
484ece7
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Timeout state is not propagated to all runtime gates, and adjustable wall clocks make the new deadline accounting unreliable.
Review effort: Balanced
Findings: 3
Open (3)
What changed in this PR
Aligns browser tool-call timeout behavior with core while preserving execution-time limits.
Changes:
- Adds pausable browser timeout handling for active tool calls.
- Adds core and browser regression tests.
- Documents shared timeout semantics.
| File | Description |
|---|---|
libs/core/src/types.ts |
Documents core timeout semantics. |
libs/core/src/__tests__/enclave.timeout-tool-calls.spec.ts |
Tests core behavior with slow tools. |
libs/browser/src/types.ts |
Documents browser timeout semantics. |
libs/browser/src/adapters/pausable-timer.ts |
Adds a pausable timer utility. |
libs/browser/src/adapters/outer-iframe-bootstrap.ts |
Pauses outer-frame timeout during tool calls. |
libs/browser/src/adapters/inner-iframe-bootstrap.ts |
Rejects post-deadline tool calls. |
libs/browser/src/adapters/iframe-adapter.ts |
Pauses the host backup timeout. |
libs/browser/e2e/timeout.spec.ts |
Covers slow, parallel, and sequential calls. |
docs/enclave/core-libraries/enclave-vm/configuration.mdx |
Updates core configuration documentation. |
docs/enclave/core-libraries/enclave-browser/overview.mdx |
Adds timeout parity information. |
docs/enclave/core-libraries/enclave-browser/configuration.mdx |
Updates browser timeout documentation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Oct 4, 2026
alexmercerpo
approved these changes
Oct 4, 2026
BrowserEnclave's timeout was a wall-clock timer that also ran while a tool call waited for toolHandler, so a script with a slow tool timed out in the browser and succeeded on @enclave-vm/core. Core's rules: a tool call in progress when the timeout passes is not cut off and its result is delivered; the script's next tool call then fails with "Execution aborted". The browser now does the same: the outer iframe's and the host's hard timeouts pause while a tool call is waiting for the host (a new PausableTimer on the host side), and the inner iframe refuses a tool call once the timeout has passed since the start. The script's own running time stays bounded by the timeout. A core spec pins the same rules, and the timeout docs of both packages describe them, including that a handler that never settles keeps run() waiting. Fixes #90 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review follow-up. The inner iframe's deadline, the outer iframe's pause accounting and the host's PausableTimer measured elapsed time with Date.now(), so a system clock change during a run moved the budget: a jump forward expired the host timer at once (the new test fails with EXECUTION_TIMEOUT before this change), a jump back stretched it. All three now use performance.now(); the inner iframe captures it before STRICT removes performance. Date.now() is kept only for the reported timestamps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
frontegg-david
force-pushed
the
fix/browser-timeout-tool-calls
branch
from
October 4, 2026 00:21
484ece7 to
f8b7c6a
Compare
This was referenced Oct 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack 4/10:
main← #92 ← #93 ← #94 ← this ← #96 ← #98 ← #99 ← #100 ← #101 ← #102Fixes #90.
Validation
Reproduced: with
timeout: 1000and a tool that takes 2.5 s, core returns"done"and the browser fails at 1.0 s withEXECUTION_TIMEOUT.The issue says core's timer doesn't count time spent in tool calls. Not quite: core's timer does run during a tool call; it just never cuts off a call in progress. When it fires, core only sets
aborted, which the nextcallTool()checks. Measured on core withtimeout: 1000and a 2.5 s tool:callTool"done"callTools in sequenceExecution aborted(the second call is refused)The browser had two wall-clock timers (outer iframe at
timeout, host attimeout + parentTimeoutBuffer), and both kept running while a tool call waited fortoolHandler.Fix
The browser now follows core's rules exactly, instead of only leaving tool time out of the timer:
PausableTimeron the host side; counters in the outer iframe). A tool call in progress is never cut off.timeoutms of wall-clock time have passed since the start:Execution aborted, as in core. The clock is captured before user code runs.timeout(a test covers a script that sleeps after a tool call).Docs: the
timeoutJSDoc in both packages, the configuration tables and the overview's "Browser vs Node.js" table now describe these rules. That includes the one caveat the two packages share: atoolHandlerthat never settles keepsrun()waiting, so bound slow tools in the handler.Red → green
libs/browser/e2e/timeout.spec.ts(+4 tests)libs/core/src/__tests__/enclave.timeout-tool-calls.spec.ts(new, 2 tests)Review follow-ups (f8b7c6a)
Date.now()(Copilot, 3 comments), so a system clock change moved the budget. The inner deadline (captured before STRICT removesperformance), the outer iframe's pause accounting and the host'sPausableTimernow useperformance.now();Date.now()is kept for the reported stats. New e2e test: the page'sDate.now()jumps an hour ahead between two tool calls. Before, the host timer fired at once (EXECUTION_TIMEOUT); now the run completes (10/10 with--repeat-each 10).Verification
nx run browser:test-e2e: 83 passed🤖 Generated with Claude Code