Skip to content

fix(sdk): disable timeout on background commands by default#1391

Open
mishushakov wants to merge 5 commits into
mainfrom
mishushakov/disable-background-cmd-timeout
Open

fix(sdk): disable timeout on background commands by default#1391
mishushakov wants to merge 5 commits into
mainfrom
mishushakov/disable-background-cmd-timeout

Conversation

@mishushakov
Copy link
Copy Markdown
Member

@mishushakov mishushakov commented Jun 5, 2026

Summary

Background commands (commands.run(..., background=True) in Python / { background: true } in JS) were killed once the default 60s command timeout elapsed. They now default to no timeout, while foreground commands keep the 60s default, and an explicit timeout/timeoutMs still takes precedence.

The "disable" value differs per transport: Python's e2b_connect treats 0 as no-limit, while @connectrpc/connect treats timeoutMs <= 0 as an immediate abort and only undefined as no-limit — so the JS side resolves to undefined and the Python side to 0. Both SDKs use their natural "not set" marker (undefined / None) to decide the default, and the changeset bumps e2b and @e2b/python-sdk as patches.

Usage

# Python — runs indefinitely now (previously killed after 60s)
handle = sandbox.commands.run("python -m http.server", background=True)
# explicit timeout still honored
sandbox.commands.run("sleep 10", background=True, timeout=2)  # TimeoutException
// JS/TS — no timeout by default for background commands
const handle = await sandbox.commands.run('python -m http.server', { background: true })
await sandbox.commands.run('sleep 10', { background: true, timeoutMs: 2000 }) // still times out

Tests

Integration tests (sync + async Python, JS) against live sandboxes assert that a background sleep 20 is capped when given timeout=10 (raises TimeoutException in under 20s) and completes successfully when no timeout is set.

🤖 Generated with Claude Code

@cla-bot cla-bot Bot added the cla-signed label Jun 5, 2026
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jun 5, 2026

🦋 Changeset detected

Latest commit: 12850f4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@e2b/python-sdk Patch
e2b Patch

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

@cursor
Copy link
Copy Markdown

cursor Bot commented Jun 5, 2026

PR Summary

Medium Risk
Default timeout behavior changes for background commands, which may affect callers that implicitly relied on the old 60s kill.

Overview
Background commands.run no longer applies the 60s default command timeout when background is true, so long-running background processes are not killed at 60s unless you pass timeout / timeoutMs. Foreground runs still default to 60s; explicit timeouts still apply.

In JS, background starts pass undefined for timeoutMs instead of the default connection timeout. In Python (sync and async), omitted timeout resolves to no limit for background and 60s for foreground via an _Unset sentinel; timeout=None is treated as no limit. Patch bumps and integration tests cover capped background runs, uncapped background runs past 70s, and Python timeout=None on foreground.

Reviewed by Cursor Bugbot for commit 12850f4. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 5, 2026

Package Artifacts

Built from 7a0477a. Download artifacts from this workflow run.

JS SDK (e2b@2.27.2-mishushakov-disable-background-cmd-timeout.0):

npm install ./e2b-2.27.2-mishushakov-disable-background-cmd-timeout.0.tgz

CLI (@e2b/cli@2.10.4-mishushakov-disable-background-cmd-timeout.0):

npm install ./e2b-cli-2.10.4-mishushakov-disable-background-cmd-timeout.0.tgz

Python SDK (e2b==2.25.1+mishushakov-disable-background-cmd-timeout):

pip install ./e2b-2.25.1+mishushakov.disable.background.cmd.timeout-py3-none-any.whl

@mishushakov mishushakov force-pushed the mishushakov/disable-background-cmd-timeout branch from 005ac68 to 19deb22 Compare June 5, 2026 18:20
@mishushakov mishushakov changed the title fix(sdk): disable connection timeout on background commands by default fix(sdk): disable timeout on background commands by default Jun 5, 2026
Background commands (`commands.run(..., background=True)` / `{ background: true }`)
were killed after the default 60s command timeout. They now default to no
timeout, while foreground commands keep the 60s default. An explicit
`timeout`/`timeoutMs` still takes precedence.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mishushakov mishushakov force-pushed the mishushakov/disable-background-cmd-timeout branch from 19deb22 to d79fa59 Compare June 5, 2026 18:29
@mishushakov mishushakov marked this pull request as ready for review June 5, 2026 18:31
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6647fb6cab

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread packages/python-sdk/e2b/sandbox_sync/commands/command.py Outdated
Comment thread packages/python-sdk/e2b/sandbox_async/commands/command.py Outdated
Comment thread packages/python-sdk/tests/sync/sandbox_sync/commands/test_run.py Outdated
mishushakov and others added 3 commits June 5, 2026 20:54
…x connect timeoutMs JSDoc

Addresses PR review:
- background no-timeout integration tests now sleep 70s (> the previous 60s
  default) so they actually guard the regression rather than passing under both
  old and new behavior.
- `CommandConnectOpts` gets its own `timeoutMs` JSDoc so `connect()` tooltips no
  longer inherit the background-specific wording from `CommandStartOpts`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Distinguish an omitted `timeout` from an explicit `None` via a private sentinel:
omitted resolves to the default (60s foreground, 0/no-timeout background), while
an explicit `None` means no timeout (0). This restores the pre-change behavior
where `run(..., timeout=None)` ran uncapped, addressing the PR review.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ng the type

Revert the `CommandConnectOpts` type split and the background note on the shared
`CommandStartOpts.timeoutMs` field. The background no-timeout behavior is
documented on the `run` background overload only, so `connect()`'s `timeoutMs`
tooltip stays generic without any type churn.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant