Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have @zegging on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check' |
zegging
marked this pull request as ready for review
September 20, 2026 09:46
zegging
requested review from
ValentaTomas,
dobrac and
jakubno
as code owners
September 20, 2026 09:46
Author
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
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.
Why
Closes #3650.
An invalid
Keepalive-Ping-Intervalcan panic intime.NewTicker. In the process streaming paths this terminates envd, interrupting command streams and losing its in-memory process tracking. It does not imply that the whole sandbox VM exits.This was found while developing a Java client that calls the API directly, rather than through the official SDKs. The normal official JavaScript/TypeScript and Python command paths send a fixed positive interval, so this is a lower-priority input-robustness fix, not a claim that typical SDK usage is affected.
What Changed
time.Duration: it must be positive and representable without overflow. Otherwise, keep the existing 90-second fallback used for missing or non-numeric values.0.9.1as required for a behavioral change.The two commits deliberately separate the regression tests from the fix.
Verification
The new envd regression tests were run with
-raceagainst both revisions. Before the fix, zero, negative, and overflowing intervals fail: three cases panic, and positive wraparound produces a roughly 290 ms interval. After the fix, all cases pass, including reset behavior and the maximum valid interval.The E2E test compiles, but the updated scenario has not been validated against rebuilt envd end to end: the local automated runner was blocked before executing that scenario. This PR remains a draft pending that verification; it does not claim E2E green.
The full envd suite is also not fully validated locally:
TestIsPathOnNetworkMount_FuseMountandTestCreateWatcherOnNetworkMountfail at the FUSE mount setup, and tests requiring root are skipped. These environment limitations have not been worked around by changing or disabling tests.