Skip to content

fix: increase agent execute and chat completions timeouts to ≥600s#141

Open
dineshreddy91 wants to merge 1 commit into
mainfrom
devin/1780956985-increase-agent-timeouts
Open

fix: increase agent execute and chat completions timeouts to ≥600s#141
dineshreddy91 wants to merge 1 commit into
mainfrom
devin/1780956985-increase-agent-timeouts

Conversation

@dineshreddy91

@dineshreddy91 dineshreddy91 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Increase timeouts for Orion agent execute and chat completions to ≥600s to prevent premature errors and retries on long-running tasks.

Changes:

Location Before After
Agent.completions OpenAI client this.client.timeout (120s default) Math.max(timeout ?? 600000, 600000)
Executions constructor APIRequestor 120000ms 600000ms
Executions.wait() default 300s 600s
Predictions.wait() default 60s 600s

Updated unit tests in agent.test.ts to expect the new 600000ms minimum.

Related PRs: vlm-run/vlmrun-python-sdk (same changes), vlm-run/vlm-lab (server-side timeouts).

Link to Devin session: https://app.devin.ai/sessions/977e2c561b8c4ef8b72d04517946a4dc
Requested by: @dineshreddy91


Open in Devin Review

- Agent completions: enforce min 600000ms (600s) timeout for OpenAI client
- Executions: increase HTTP request timeout from 120000ms to 600000ms
- Executions.wait: increase default polling timeout from 300s to 600s
- Predictions.wait: increase default polling timeout from 60s to 600s
- Update agent tests to match new timeout behavior

Orion agent executions and chat completions can take several minutes,
especially for complex tasks. Previous timeouts caused premature errors
and retries.

Co-Authored-By: dinesh@vlm.run <dinesh.andromeda@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request increases the default timeout values across the client SDK (including OpenAI client initialization, API requestors, and execution/prediction wait times) to 10 minutes (600,000 ms or 600 seconds). However, the feedback points out that unconditionally overriding or enforcing a minimum timeout of 10 minutes prevents users from specifying shorter custom timeouts. It is recommended to respect the user's explicitly configured timeout and only fall back to the 10-minute default when it is omitted. Additionally, the unit tests should be updated to correctly assert the custom timeout values rather than the overridden default.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/client/agent.ts
apiKey: this.client.apiKey,
baseURL: baseUrl,
timeout: this.client.timeout,
timeout: Math.max(this.client.timeout ?? 600000, 600000),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Overriding the user-specified timeout with a minimum of 10 minutes (600000 ms) prevents users from setting shorter timeouts (e.g., for fast-failing requests). Standard SDK practice is to respect the user's explicitly configured timeout if provided, and only fall back to the default when it is omitted.

Additionally, the unit test "should use client timeout and maxRetries when provided" (lines 415-432 in agent.test.ts) now has a mismatch where it configures a 30000 ms timeout but asserts that 600000 ms is used.

Suggested change
timeout: Math.max(this.client.timeout ?? 600000, 600000),
timeout: this.client.timeout ?? 600000,

Comment thread src/client/executions.ts
constructor(client: Client) {
this.client = client;
this.requestor = new APIRequestor({ ...client, timeout: 120000 });
this.requestor = new APIRequestor({ ...client, timeout: 600000 });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using { ...client, timeout: 600000 } unconditionally overrides any custom timeout configured by the user on the client instance. If a user explicitly configures a custom timeout (whether shorter or longer), it should be respected.

We should default to 600000 ms only if client.timeout is not specified.

Suggested change
this.requestor = new APIRequestor({ ...client, timeout: 600000 });
this.requestor = new APIRequestor({ ...client, timeout: client.timeout ?? 600000 });

apiKey: "test-api-key",
baseURL: "https://agent.vlm.run/v1/openai",
timeout: 30000,
timeout: 600000,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The test "should use client timeout and maxRetries when provided" configures a custom timeout of 30000 ms but asserts that the timeout is 600000 ms. This contradicts the test's description and intent. Once the implementation is updated to respect the user's custom timeout, this assertion should be updated to expect the configured 30000 ms.

Suggested change
timeout: 600000,
timeout: 30000,

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment thread src/client/executions.ts
constructor(client: Client) {
this.client = client;
this.requestor = new APIRequestor({ ...client, timeout: 120000 });
this.requestor = new APIRequestor({ ...client, timeout: 600000 });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Inconsistent timeout override patterns across client classes

The timeout override approach differs across SDK client classes:

  • Agent.completions uses Math.max(client.timeout ?? 600000, 600000) — respects user timeouts above 600s
  • Executions constructor uses { ...client, timeout: 600000 } — always hardcodes 600000ms, ignoring user config entirely (even if user wants >600s)
  • Files uses { ...client, timeout: 0 } — always hardcodes 0 for unlimited
  • Artifacts uses this.client.timeout ?? 120000 — uses user config if set, 120000ms default otherwise

The Executions class at src/client/executions.ts:11 doesn't use the Math.max pattern, so a user who sets a timeout of e.g. 900000ms (15 minutes) would have it silently reduced to 600000ms for executions. This is a pre-existing pattern (previously hardcoded to 120000) but worth noting given that Agent now uses a more permissive approach.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant