Add configurable timeout to act tool in agent#1766
Open
Conversation
🦋 Changeset detectedLatest commit: 5bfac76 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 10 files
Confidence score: 2/5
- The unsanitized error message reflection in
packages/core/lib/v3/timeoutConfig.tscan expose arbitrary exception details, which is a high-severity security/privacy risk. - Given the severity (8/10) and high confidence, this issue meaningfully raises merge risk despite being a single finding.
- Pay close attention to
packages/core/lib/v3/timeoutConfig.ts- ensure error messages are sanitized for non-TimeoutErrorexceptions.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/timeoutConfig.ts">
<violation number="1" location="packages/core/lib/v3/timeoutConfig.ts:52">
P1: Custom agent: **Exception and error message sanitization**
The catch-all block reflects unsanitized error messages from arbitrary exceptions. Only `TimeoutError` has a known-safe message; any other error thrown by the underlying tool could leak sensitive information (API keys, CDP URLs, credentials, etc.) through `(error as Error)?.message`. This should either catch `TimeoutError` specifically and re-throw other errors, or sanitize the message before returning it.
For example, catch `TimeoutError` by type and return a generic message for all other errors:
```typescript
catch (error) {
if (error instanceof TimeoutError) {
return { success: false, error: error.message };
}
throw error;
}
```</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as User/SDK Client
participant Agent as V3AgentHandler
participant Registry as createAgentTools
participant Tool as Tool (e.g. act, click)
participant Utils as timeoutConfig (withTimeout)
participant V3 as V3 Engine (Browser/LLM)
Note over Client,V3: Initialization & Tool Wrapping
Client->>Agent: execute(instruction, { toolTimeout: 30000 })
Agent->>Registry: NEW: createAgentTools(options including toolTimeout)
loop For each tool (act, click, goto, etc.)
Registry->>Registry: NEW: wrapToolWithTimeout(tool, timeoutMs)
end
Registry-->>Agent: Returns timeout-guarded toolset
Note over Agent,V3: Execution Loop
Agent->>Tool: execute(args)
Tool->>Utils: NEW: withTimeout(originalExecute, timeoutMs)
opt If Tool is v3-method (act, extract, fillForm, ariaTree)
Tool->>V3: CHANGED: call method with { timeout: toolTimeout }
Note right of V3: Operation now respects internal deadline
end
alt Success Case (within timeout)
V3-->>Tool: result
Tool-->>Agent: result
else Timeout Case (Deadline reached)
Utils-->>Tool: Throw TimeoutError
Tool->>Tool: Catch Error
Tool-->>Agent: NEW: return { success: false, error: "tool timed out..." }
Note over Agent: Agent can now notify LLM to retry/adapt
end
Agent-->>Client: Final Result/Status
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
Greptile SummaryThis PR adds a configurable Key changes:
Issues found:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[agent.execute with toolTimeout] --> B[v3AgentHandler.execute]
B --> C[createTools with toolTimeout]
C --> D{toolTimeout > 0?}
D -->|Yes| E[Set toolTimeout value]
D -->|No| F[Set toolTimeout = undefined]
E --> G[Pass to Tools]
F --> G
G --> H[act tool]
G --> I[extract tool]
G --> J[fillForm tool]
G --> K[ariaTree tool]
H --> L{Timeout?}
I --> L
J --> M{Timeout in observe?}
K --> N{Timeout in extract?}
M --> L
N --> L
L -->|Yes| O[Return TimeoutError message]
L -->|No| P[Return success result]
O --> Q[Agent receives error and retries]
P --> R[Agent continues]
Last reviewed commit: 354eb79 |
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/agent/tools/fillform.ts">
<violation number="1" location="packages/core/lib/v3/agent/tools/fillform.ts:89">
P1: Custom agent: **Exception and error message sanitization**
Unsanitized error message returned from generic catch — `error?.message ?? String(error)` may leak sensitive data (agent variables, API keys, CDP URLs) from underlying `v3.act()` / `v3.observe()` calls. Return a generic sanitized message instead, or filter the error through a sanitization function before surfacing it.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
pirate
reviewed
Feb 27, 2026
Member
There was a problem hiding this comment.
we already have withLlmTimeout which wraps llmClient.createChatCompletion for act/extract/observe and only checks the LLM_MAX_MS env var currently I think.
Separately I think act/extract/observe can also accept timeout args that they enforce themselves
can you try and dedupe these / merge all these timeout mechanisms? right now we have multiple different timeout options enforced at different layers that will likely end up conflicting in some situations and emitting slightly different errors.
pirate
reviewed
Feb 27, 2026
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/agent/tools/ariaTree.ts">
<violation number="1" location="packages/core/lib/v3/agent/tools/ariaTree.ts:46">
P1: Custom agent: **Exception and error message sanitization**
Unsanitized error message is reflected back as tool content. Errors from `v3.extract()` or `v3.context.awaitActivePage()` may contain CDP URLs, API keys, or other secrets. The `TimeoutError` branch correctly returns a sanitized message, but the generic catch should do the same — return a safe, generic message without reflecting `error.message` verbatim. Consider logging the raw error internally and returning only a sanitized message to the agent.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
pirate
reviewed
Feb 27, 2026
pirate
reviewed
Feb 27, 2026
pirate
reviewed
Feb 27, 2026
pirate
reviewed
Feb 27, 2026
pirate
reviewed
Feb 27, 2026
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/agent/tools/ariaTree.ts">
<violation number="1" location="packages/core/lib/v3/agent/tools/ariaTree.ts:48">
P1: Error information is lost before reaching the LLM. Setting `content: ""` while moving the error details to `result.error` means the model only sees an empty `Accessibility Tree:\n` — it has no signal that an error occurred. The `toModelOutput` callback only reads `result.content` and ignores `error`/`success`. Either include the error message in `content` (as it was before), or update `toModelOutput` to surface the `error` field when present.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
pirate
reviewed
Feb 27, 2026
pirate
reviewed
Feb 27, 2026
pirate
approved these changes
Feb 27, 2026
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| * reported back to the LLM as a timeout error so it can retry or adjust. | ||
| * For tools that call v3 methods (act, extract, fillForm, ariaTree), the | ||
| * timeout is also forwarded to the underlying v3 call for true cancellation. | ||
| * @default 45000 (45 seconds) |
Contributor
There was a problem hiding this comment.
documentation says default is 45000, but no default is actually applied
The code in index.ts sets toolTimeout to undefined when not provided, rather than 45000:
const toolTimeout = options?.toolTimeout != null && options.toolTimeout > 0
? options.toolTimeout
: undefined;Either apply the default or update the docs to remove @default 45000.
Comment on lines
+55
to
+58
| toModelOutput: (result) => { | ||
| if (result.success === false || result.error !== undefined) { | ||
| return { | ||
| type: "content", |
Contributor
There was a problem hiding this comment.
toModelOutput doesn't handle error cases, so timeout errors won't reach the LLM
When a timeout occurs, the execute function returns { content: "", error: "...", success: false }, but toModelOutput only uses result.content, so the model sees an empty tree without the error explanation.
Other tools like click check result.success in toModelOutput:
if (result.success) {
return { type: "content", value: [{ type: "text", text: `...` }] };
}
return {
type: "content",
value: [{ type: "text", text: JSON.stringify({ success: result.success, error: result.error }) }],
};
Contributor
There was a problem hiding this comment.
2 issues found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/lib/v3/agent/tools/fillFormVision.ts">
<violation number="1" location="packages/core/lib/v3/agent/tools/fillFormVision.ts:157">
P1: Custom agent: **Exception and error message sanitization**
The error-path `JSON.stringify(result)` serialises the **entire** result object instead of explicitly picking safe fields. `FillFormVisionToolResult` can carry `playwrightArguments` (which contains variable-substituted secrets like passwords and API keys) and `screenshotBase64`. The old code deliberately allowlisted only `{ success, error }` — the new code removes that sanitisation boundary.
Replace with an explicit pick of safe fields to avoid leaking sensitive `variables` or screenshots into the model text output.</violation>
</file>
<file name="packages/core/lib/v3/agent/tools/screenshot.ts">
<violation number="1" location="packages/core/lib/v3/agent/tools/screenshot.ts:29">
P1: Custom agent: **Exception and error message sanitization**
Unsanitized error message reflected to the agent/user. `(error as Error).message` from CDP operations (e.g., `page.screenshot()`, `awaitActivePage()`) may contain sensitive information like CDP connect URLs, session identifiers, or internal endpoint details. Per the error sanitization rule, error messages must be sanitized before being surfaced, and properly typed error classes should be used instead of raw string interpolation.
Consider sanitizing the message (e.g., stripping URLs, tokens, and connection strings) or returning a generic safe message while logging the full error internally.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } catch (error) { | ||
| return { | ||
| success: false, | ||
| error: `Error taking screenshot: ${(error as Error).message}`, |
Contributor
There was a problem hiding this comment.
P1: Custom agent: Exception and error message sanitization
Unsanitized error message reflected to the agent/user. (error as Error).message from CDP operations (e.g., page.screenshot(), awaitActivePage()) may contain sensitive information like CDP connect URLs, session identifiers, or internal endpoint details. Per the error sanitization rule, error messages must be sanitized before being surfaced, and properly typed error classes should be used instead of raw string interpolation.
Consider sanitizing the message (e.g., stripping URLs, tokens, and connection strings) or returning a generic safe message while logging the full error internally.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/lib/v3/agent/tools/screenshot.ts, line 29:
<comment>Unsanitized error message reflected to the agent/user. `(error as Error).message` from CDP operations (e.g., `page.screenshot()`, `awaitActivePage()`) may contain sensitive information like CDP connect URLs, session identifiers, or internal endpoint details. Per the error sanitization rule, error messages must be sanitized before being surfaced, and properly typed error classes should be used instead of raw string interpolation.
Consider sanitizing the message (e.g., stripping URLs, tokens, and connection strings) or returning a generic safe message while logging the full error internally.</comment>
<file context>
@@ -8,22 +8,39 @@ export const screenshotTool = (v3: V3) =>
+ } catch (error) {
+ return {
+ success: false,
+ error: `Error taking screenshot: ${(error as Error).message}`,
+ };
+ }
</file context>
Suggested change
| error: `Error taking screenshot: ${(error as Error).message}`, | |
| error: "Error taking screenshot. The operation failed — please retry or check page state.", |
pirate
force-pushed
the
Add-configurable-timeout-to-act-tool-in-agent
branch
from
354eb79 to
80a4f81
Compare
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
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
2 participants
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.
Why
Agent tool calls can hang indefinitely when the underlying operation stalls (e.g. waiting on a slow LLM inference, an unresponsive page, or a network timeout). When this happens the entire agent loop blocks and the user has no recourse other than killing the process. We need a safety net that bounds how long any single tool call can take and returns control to the LLM so it can adapt.
What changed
We now pass timeout into the tools for ones that involve llm inference within the tool call
Summary by cubic
Adds a configurable per-tool timeout to prevent agent tool calls from hanging. Defaults to 45s, applies only to inference-based tools, and timed-out calls return control with clear JSON errors.
New Features
Bug Fixes
Written for commit 5bfac76. Summary will update on new commits. Review in cubic