fix(envd): support opt-in descendant signals - #3617
tttboy123 wants to merge 4 commits into
Conversation
Sorry, something went wrong.
|
We require contributors to sign our Contributor License Agreement, and we don't have @tttboy123 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' |
Sorry, something went wrong.
|
We require contributors to sign our Contributor License Agreement, and we don't have @tttboy123 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' |
Sorry, something went wrong.
|
We require contributors to sign our Contributor License Agreement, and we don't have @tttboy123 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' |
Sorry, something went wrong.
|
Follow-up hardening is pushed in 0a364e308. During final review I found that SendSignal canceled output pumps before validating or delivering a terminal signal. That meant a rejected group-scoped kill could leave the process alive while disconnecting its output. The handler now validates and sends first, returns without touching output on failure, and cancels output only after successful SIGKILL/SIGTERM delivery. The regression test now asserts both sides of that contract: a process that does not own its group remains alive and its output remains connected after the request is rejected. Fresh validation:
|
Sorry, something went wrong.
|
The cla-bot has been summoned, and re-checked this pull request! |
Sorry, something went wrong.
Preserve the injectable envd version and bump the proposed behavior version to 0.9.1. Document explicit signal scope without changing automatic timeout semantics.
|
Refreshed this branch against current runtime main 92197909d in merge commit 9c53269f0, without rewriting history. The version-file conflict is resolved: the branch preserves upstream's injectable var Version and proposes envd 0.9.1. I also documented the process-group and lifecycle boundaries in docs/ARCHITECTURE.md. Validation completed on October 5 with Go 1.26.8 / golangci-lint 2.13.2 (published on October 6 after resuming the interrupted push):
Coverage boundary: the unfiltered Linux run was attempted, but those unchanged upstream tests require mount privileges, a writable cgroup hierarchy, or bindfs, which this container lacks. The full native macOS run also hits upstream's Linux-only newTreeFixture reference. No tests were edited/disabled, and this is not a claim of full CI or hosted deployment validation. The scope stays explicit opt-in group signaling, preserving default leader-only kill and existing automatic-timeout behavior. I proposed handling the automatic-timeout case separately in e2b-dev/E2B#1031 (comment) and would appreciate confirmation of that contract. SDK e2b-dev/E2B#1801 remains Draft and untouched until this backend merges, as requested. Its historical 0.7.1 feature floor will need to move to the actual backend feature release before it is marked ready; the proposed 0.9.1 here is not yet a release guarantee. This backend PR is ready for another look when convenient. |
Sorry, something went wrong.
This branch has not been deployed
Reviewers
At least 1 approving review is required to merge this pull request.
Problem
CommandHandle.kill() currently signals only the managed leader PID. Child processes can remain alive and are then correctly restored by a later memory-preserving pause/resume. I reproduced this against hosted E2B with Python SDK 2.46.4: the kill RPC returned true in 0.322s and removed the leader, while both child processes remained alive before and after pause/resume.
This PR adds the envd/protocol half of an opt-in descendant-kill API while preserving the existing PID-only default. It relates to e2b-dev/E2B#1034.
Solution
The process-group scope intentionally covers descendants that stay in the command group. Descendants that call setsid() can escape it; a per-command cgroup / cgroup.kill guarantee would be a separate, larger lifecycle change.
Compatibility
Existing clients omit descendants, so their behavior is unchanged. SDK PR e2b-dev/E2B#1801 exposes an explicit Python/JS kill scope and remains Draft until this backend PR merges. Its feature-version gate must be aligned with the actual backend release before readiness; its current historical 0.7.1 floor is not valid for the refreshed backend.
Automatic finite-timeout cleanup is a separate proposed follow-up under e2b-dev/E2B#1031, awaiting maintainer agreement. This PR does not claim to resolve that timeout case or to destroy the sandbox.
Validation (original submission)
At the original submission, unfiltered local lint also reported a pre-existing unused-parameter warning in internal/services/process/dup3_other.go; CI uses only-new-issues. Current refresh evidence is recorded below.
October 5 upstream refresh
Merged upstream main 92197909d without rewriting the feature branch. The only content conflict was the envd version file. Fresh checks use the current repository tool versions: Go 1.26.8 and golangci-lint 2.13.2.
The unfiltered Linux suite was attempted but cannot pass in this unprivileged container: mount-dependent upload cases need mount privileges, real cgroup tests need a writable cgroup hierarchy, and FUSE watcher cases need bindfs. Those test files are unchanged from upstream. This is limited local coverage, not an all-tests-green or full hosted-integration claim.
The full native macOS suite is blocked by unchanged upstream cgroups/memory_test.go referencing the Linux-only newTreeFixture; other reported packages pass. No hosted deployment or new cloud pause/resume test is claimed for this refresh.