fix: update pause() JSDoc and add regression tests for apiKey propagation by passionworkeer · Pull Request #1219 · e2b-dev/E2B

passionworkeer · 2026-03-20T16:07:18Z

Summary

Fixes Copilot review feedback on PR #1218:

JSDoc fix: pause() method was documented as returning "sandbox ID" but the actual return type is Promise<boolean> (true=paused, false=already paused). Updated @returns to reflect the correct boolean semantics.
Regression tests: Added tests verifying that Sandbox.connect() with an explicit apiKey correctly propagates credentials to pause() even when E2B_API_KEY is not set in the environment.

Changes

packages/js-sdk/src/sandbox/index.ts: Updated pause() JSDoc @returns from "sandbox ID" to "true if paused successfully, false if already paused"
packages/js-sdk/tests/sandbox/pause.test.ts: New test file with 3 regression tests

Test coverage

pause() uses apiKey from connectionConfig when E2B_API_KEY is not set
pause() returns false when sandbox is already paused
pause() works on connected sandbox with apiKey in connectionConfig

When using Sandbox.connect() with an apiKey, the pause() method was not passing the apiKey to the API call, causing auth failures. This fix ensures pause() (and deprecated betaPause()) use the same pattern as kill() - merging connectionConfig with provided opts. Fixes: e2b-dev#1215

@returns

…tion - Update @returns JSDoc: pause() returns boolean (true=paused, false=already paused) matching SandboxApi.pause semantics, not a sandbox ID - Add regression tests for apiKey propagation in Sandbox.connect(): - pause() succeeds when E2B_API_KEY is unset but apiKey is passed to connect() - pause() returns false when sandbox is already paused - pause() works on connected sandbox with apiKey in connectionConfig Refs: Copilot PR e2b-dev#1218 review comments

changeset-bot · 2026-03-20T16:07:23Z

🦋 Changeset detected

Latest commit: 6221c8e

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

This PR includes changesets to release 1 package

Name	Type
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

Copilot

Pull request overview

This PR corrects the pause() JSDoc return semantics and adds regression tests ensuring apiKey passed via Sandbox.connect() propagates to pause() even when E2B_API_KEY is unset.

Changes:

Fix pause() JSDoc @returns to accurately describe Promise<boolean> semantics.
Ensure Sandbox.pause() / betaPause() pass merged connectionConfig + per-call opts to the underlying API.
Add integration regression tests covering apiKey propagation and paused-state return behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
packages/js-sdk/src/sandbox/index.ts	Corrects `pause()` documentation and merges `connectionConfig` into `pause()`/`betaPause()` API calls.
packages/js-sdk/tests/sandbox/pause.test.ts	Adds regression tests for `apiKey` propagation and `pause()` boolean return behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-20T16:09:04Z

packages/js-sdk/tests/sandbox/pause.test.ts

+import { assert, expect } from 'vitest'
+
+import { Sandbox } from '../../src'
+import { isDebug, sandboxTest, template } from '../setup'


template is imported but not used anywhere in this new test file. Removing it will keep the test file cleaner and avoid lint failures in stricter configurations.

Suggested change

import { isDebug, sandboxTest, template } from '../setup'

import { isDebug, sandboxTest } from '../setup'

Copilot · 2026-03-20T16:09:05Z

packages/js-sdk/tests/sandbox/pause.test.ts

+      if (savedApiKey) {
+        process.env.E2B_API_KEY = savedApiKey


Environment restoration is currently conditional on savedApiKey being truthy; if it was set to an empty string, the test will not restore the original value and could leak mutated process state into subsequent tests. Prefer checking savedApiKey !== undefined (restore), and otherwise explicitly delete process.env.E2B_API_KEY.

Suggested change

if (savedApiKey) {

process.env.E2B_API_KEY = savedApiKey

if (savedApiKey !== undefined) {

process.env.E2B_API_KEY = savedApiKey

} else {

delete process.env.E2B_API_KEY

Copilot · 2026-03-20T16:09:05Z

packages/js-sdk/tests/sandbox/pause.test.ts

+      const apiKey = process.env.E2B_API_KEY || savedApiKey
+      expect(apiKey).toBeDefined()
+
+      // Connect to the sandbox with an explicit apiKey (E2B_API_KEY is now unset)
+      const connected = Sandbox.connect(sandbox.sandboxId, { apiKey })


expect(apiKey).toBeDefined() is a runtime check but does not help TypeScript narrow apiKey to string at compile time. To avoid accidentally passing undefined into Sandbox.connect (and to satisfy TS if apiKey is typed as required), assign with an explicit guard (throw/assert) and/or pass apiKey! after the assertion.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 725d362766

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-20T16:10:47Z

packages/js-sdk/tests/sandbox/pause.test.ts

+      expect(apiKey).toBeDefined()
+
+      // Connect to the sandbox with an explicit apiKey (E2B_API_KEY is now unset)
+      const connected = Sandbox.connect(sandbox.sandboxId, { apiKey })


Await Sandbox.connect before calling instance methods

Sandbox.connect(...) is async and returns a Promise<Sandbox>, but this test stores it without await and then calls connected.pause()/connected.isRunning(). In Node this means connected is a Promise, so method calls fail with TypeError before the pause behavior is exercised; the same pattern appears again later in this file (line 63).

Useful? React with 👍 / 👎.

- Remove unused template import - Change savedApiKey truthy check to explicit undefined check - Add await before Sandbox.connect() calls (P1 - async Promise fix) - Add explicit non-null assertion after toBeDefined() for TypeScript

- Use savedApiKey directly instead of re-reading from process.env after deletion - Update pause() JSDoc to say 'Pause this sandbox' (instance method, not static) Co-Authored-By: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

passionworkeer · 2026-03-21T13:37:10Z

Hi, checking in - this PR updates pause() JSDoc and adds regression tests for apiKey propagation. Copilot reviewed with no issues. Is there anything needed to move forward?

passionworkeer · 2026-03-21T16:39:58Z

The P1 bug (missing �wait on Sandbox.connect()) has been fixed and pushed to a new PR #1223 targeting main. The fix addresses:

All Sandbox.connect() calls now properly use �wait
Env restoration logic corrected: if (savedApiKey !== undefined) with proper else branch to delete the env var
Redundant apiKey variable removed

Please review the new PR #1223.

…n tests Re-format the spread expressions in pause()/betaPause() to comply with the project’s prettier/eslint rules, and confirm all review feedback from Copilot and Codex is fully addressed: - Remove unused `template` import (Copilot #1) - Use `savedApiKey !== undefined` guard for env restoration (Copilot e2b-dev#2) - Add explicit throw guard for TypeScript narrowing on apiKey (Copilot e2b-dev#3) - `await Sandbox.connect()` for async correctness (Codex P1) No functional changes — only formatting adjustments and lint compliance.

Add explicit isRunning() verification between the two pause() calls in the "returns false when already paused" test, consistent with the pattern used in the other two pause() tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ression tests - Remove unused 'expect' import from vitest - Replace expect(apiKey).toBeDefined() + apiKey! with explicit if-check + throw in test 3, consistent with test 1 pattern Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Replace the runtime-only `expect(apiKey).toBeDefined()` + `apiKey!` pattern with an explicit if-guard that narrows the type to `string` at compile time. This matches the pattern already used in the first regression test and satisfies TypeScript's type checker when apiKey is typed as optional. Also removes the now-unused `expect` vitest import. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…nConfig Use nullish coalescing (??) instead of logical OR (||) when reading apiKey and accessToken from opts, so that empty-string values are preserved rather than incorrectly falling back to the environment variable. Also simplify E2B_API_KEY restoration in the pause regression test to use the same ?? pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…n pause tests - ConnectionConfig: use nullish coalescing (??) instead of || for apiKey and accessToken to correctly preserve empty-string values - pause.test.ts: simplify E2B_API_KEY restoration using ?? operator - Resolve merge conflict in third pause regression test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…sion test Add an explicit isRunning() assertion before calling pause() in the apiKey-propagation regression test, matching the defensive guard already present in the third pause regression test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

passionworkeer added 2 commits March 20, 2026 05:33

Copilot AI reviewed Mar 20, 2026

View reviewed changes

chatgpt-codex-connector bot reviewed Mar 20, 2026

View reviewed changes

passionworkeer and others added 2 commits March 21, 2026 00:33

ValentaTomas removed their request for review March 21, 2026 06:40

TRIX and others added 8 commits March 22, 2026 02:41

chore: add changeset for pause() JSDoc fix

2f9021c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update pause() JSDoc and add regression tests for apiKey propagation#1219

fix: update pause() JSDoc and add regression tests for apiKey propagation#1219
passionworkeer wants to merge 12 commits intoe2b-dev:mainfrom
passionworkeer:fix/pause-method-jsdoc-and-test

passionworkeer commented Mar 20, 2026

Uh oh!

changeset-bot bot commented Mar 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 20, 2026

Uh oh!

passionworkeer commented Mar 21, 2026

Uh oh!

passionworkeer commented Mar 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	import { isDebug, sandboxTest, template } from '../setup'
	import { isDebug, sandboxTest } from '../setup'

Conversation

passionworkeer commented Mar 20, 2026

Summary

Changes

Test coverage

Uh oh!

changeset-bot bot commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

passionworkeer commented Mar 21, 2026

Uh oh!

passionworkeer commented Mar 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot bot commented Mar 20, 2026 •

edited

Loading