[Donnykk]

Closes #2108.

Start the example interpreter through a launcher that reserves its listener, starts envd and Jupyter, and reaps dependency processes on failure or shutdown. Preserve tini as PID 1. Validate envd payloads and bound readiness waits before reporting startup complete; clean up known sessions even when WebSocket cleanup fails.

Verify real Uvicorn readiness with a prebound listener and delayed lifespan startup. Empty ENVD_LOG_FILE and JUPYTER_LOG_FILE values use the same defaults as the image entrypoint. Keep startup behavior documentation above the build instructions.

Testing: all 38 offline Python tests passed. The derived image and live envd sandbox contract have not been exercised. Session-creation requests are not replayed after an ambiguous timeout; an unknown session ID can remain until sandbox destruction.

[cubesandboxbot]

Starting envd is fire-and-forget: Popen succeeds as soon as the exec does, and this code prints "envd started" without checking poll() or probing 127.0.0.1:$ENVD_PORT/health. If envd exits right after exec (e.g. EADDRINUSE because the base image's entrypoint or CubeMaster's cubebox wrapper already owns ENVD_PORT, or a rejected flag), the launcher continues and server.py only discovers it on the first run_code that sends env vars (502). The -isnotfc log-exporter fix this PR targets then silently does not take effect. Please wait for envd readiness and fail startup (so the finally reaps the children) when it never becomes ready.

[cubesandboxbot]

Only CMD is set, so the base image's ENTRYPOINT still runs. If it is cube-entrypoint.sh (the documented Cube contract — it always starts envd on ${ENVD_PORT}), launch.py starts a second envd on the same port; if the base ENTRYPOINT starts Jupyter/uvicorn itself, launch.py may never run at all. Since this PR does no image build or sandbox acceptance, please check the base image's Config.Entrypoint/Config.Cmd (e.g. docker inspect) and reset the entrypoint (e.g. ENTRYPOINT []) if the launcher is meant to own startup.

[cubesandboxbot]

The session DELETE is only attempted when kernel.close() returns normally. Any exception from close() (websocket already closed, cancellation, a failing cleanup task) skips it and leaves the Jupyter session/kernel that POST /api/sessions just created — the failed-session leak this change is meant to prevent. Run the DELETE in its own try/except so it is independent of the websocket teardown result.

[cubesandboxbot]

ENVD_LOG_FILE is documented (docker/cube-entrypoint.sh, docs/guide/tutorials/bring-your-own-image.md) to accept - for "inherit container stdio". Here open("-", "ab") would create a file literally named - in the process cwd and swallow envd's output. The launcher also ignores the documented ENVD_BIN and ENVD_EXTRA_ARGS. If this launcher replaces the entrypoint's role, it should honor those conventions or document the deviation.

[cubesandboxbot]

Automated review — AI-generated, not a human approval

PR #2109 · fix(examples): make interpreter startup and session cleanup reliable @ 886790f4
Base: master · Reviewed statically from review-input/pr.diff + the base tree (no checkout of PR head, no test execution).

What the change does

• Adds lightweight-code-interpreter/launch.py: binds/listens on the interpreter port before starting envd/Jupyter/Uvicorn and passes the pre-bound socket to uvicorn.Server.run(sockets=[...]), so kernel/HTTP-client ephemeral ports cannot steal 49999 (which is inside the default Linux ephemeral range). Manages both children, escalates SIGTERM→SIGKILL, preserves an in-flight startup error over cleanup errors, and installs a SIGTERM handler so shutdown exits through Python cleanup.
• Dockerfile: replaces the inherited entrypoint with tini -- + python launch.py, adds build-time checks for tini, python, jupyter, and imports.
• server.py: CODE_INTERPRETER_CONTEXT_CREATE_TIMEOUT (default 60 s) for POST /api/sessions, the kernel WebSocket open_timeout, and the %cd initialization; a bounded, error-classifying _wait_for_envd() readiness gate in the lifespan; cleanup of the known session (and websocket) when connect() fails, without replaying creation; default_context_id reset in the lifespan finally.
• New offline tests (test_launch.py, test_startup.py, incl. a real Uvicorn pre-bound-socket test) and a path-filtered CI workflow; README updates.

Verdict

No blocking correctness defects found. The core design is sound and unusually well covered by tests. Three issues worth addressing, all small, are listed below; the rest are nits/observations.

Findings

1. Dockerfile:10 — the build check does not validate "server imports" (medium-low). It verifies uvicorn and jupyter_server, but server.py imports fastapi, httpx, starlette, pydantic and the legacy websockets.client API. Since the README supports overriding SANDBOX_CODE_IMAGE, an incompatible/missing module passes the build and fails at Config.load() inside the container. Importing the copied module would cover the whole set. (inline comment posted)
2. README.md:28 — README_zh.md not updated (low-medium). The zh file still claims an unavailable envd//envs yields HTTP 502 from run_code, which is now false (startup fails, the app never becomes ready), and it lacks the new launcher/timeout/readiness/JUPYTER_LOG_FILE sections. The two files are cross-linked and normally kept in sync. (inline comment posted)
3. server.py:503 — compensating DELETE /api/sessions/{id} uses the httpx default 5 s timeout (low). That branch runs exactly when Jupyter was slow enough to exceed the new 60 s creation timeout, so the cleanup request is likely to time out as well and be swallowed, leaving the session/kernel alive until teardown — the outcome this block is meant to prevent. (inline comment posted)

Nits / observations

• tests/test_launch.py (test_listener_reserved_before_dependencies_and_lifespan): patch.object(launch.socket, 'socket', ...) patches the stdlib module itself, so socket.create_connection() inside assert_listening() also goes through new_socket and appends client sockets to the same sockets list the test later treats as sockets[0] → listener. It only holds today because the listener is created first; a refactor that opens any socket earlier would silently assert on the wrong socket. Capturing the listener explicitly (e.g. from the run(sockets=...) argument only) would make the test self-evident.
• launch.py: -isnotfc is hard-coded and ENVD_EXTRA_ARGS is appended after -port <ENVD_PORT>, unlike cube-entrypoint.sh which de-dupes -isnotfc. Duplicate boolean flags are harmless in Go's flag package, but an ENVD_EXTRA_ARGS="-port X" would move envd while server.py keeps probing ENVD_PORT.
• Readiness budget vs. platform probe: cube tpl create-from-image --probe 49999 configures a ~30 s window (timeout_ms=30000, failure_threshold=300 × period_ms=100), while the new budgets are 60 s each. Typical startup is far below both, but a degraded envd/Jupyter will fail the template-build probe before the interpreter's own budget is used. A sentence in the README would set expectations.
• _wait_for_envd()'s timeout branch (RuntimeError("envd did not become ready")) has no direct test — the existing tests either patch the function out or exercise the retry/success/fatal paths. A case with a tiny patched CONTEXT_CREATE_TIMEOUT and an always-503 transport would cover it.
• As the PR description already discloses, the derived image and the live envd contract were not exercised. Because launch.py now replaces both the inherited ENTRYPOINT and CMD, the manual jupyter server invocation must reproduce whatever the base sandbox-code command did (paths, config, /envs availability before /init). Worth confirming once against the real base image; a mismatched jupyter binary/venv is also only partially covered by the build check (command -v jupyter vs. python -c "import jupyter_server").

What looks good

• The reserved-listener reasoning is correct (the ephemeral-port collision on 49999 is real), and PreboundUvicornTest proves the ordering with a real Uvicorn instead of asserting on mocks.
• stop_children() preserves the original startup exception over cleanup errors, tears down in reverse order, and escalates to kill() with a second bounded wait().
• _wait_for_envd() classifies transport/5xx/429 as retryable and everything else (e.g. 404, invalid payload) as fatal, matching the documented fail-fast contract.
• Session cleanup on a known ID without replaying creation is the right call for an ambiguous timeout; the original handshake exception is preserved and the contexts map is never polluted.
• Test coverage of failure paths (partial child launch, bind conflict, lifespan failure, stubborn child, SIGTERM, cleanup-throws) is thorough and readable.

[cubesandboxbot]

Verify the inherited ENTRYPOINT before relying on this CMD.

This file replaces only CMD; ENTRYPOINT is inherited from sandbox-code. CubeSandbox base images use docker/Dockerfile.cube-base:97 (ENTRYPOINT ["/usr/bin/tini","--","/usr/local/bin/cube-entrypoint.sh"]), and docker/cube-entrypoint.sh:50 unconditionally starts envd -port ${ENVD_PORT:-49983} (with -isnotfc appended) before running the CMD. If sandbox-code keeps that entrypoint, envd is already listening on 49983 when launch.py runs, so the launcher's own envd loses the bind and exits while the launcher still prints envd started on :49983 — and the exporter/log fix described in the README would apply to a process that is not the one serving /envs.

Please confirm with docker inspect --format '{{json .Config.Entrypoint}}' <registry>/sandbox-code:latest. If an entrypoint already starts envd, drop the envd spawn from launch.py; if not, keep it. This is exactly the base-image interaction the PR says was not acceptance-tested.

Also worth confirming: overriding CMD replaces the base image's entire startup command, so launch.py must replicate everything else it did (extra Jupyter/uvicorn flags, any init step), not just the interpreter.

[cubesandboxbot]

envd startup is never verified, so a failed envd is silent.

Popen succeeds even when envd exits immediately (port already in use, missing binary, rejected flag); the launcher then prints envd started and continues. The template probe targets 49999, so the sandbox looks ready while the first run_code fails with a 502 from /envs (server.py:24, ENVD_TIMEOUT). Consider checking process.poll() after a short wait, or polling http://127.0.0.1:{envd_port}/health, and failing with a clear exit code — the same way _wait_for_jupyter handles Jupyter readiness on the app side.

Related: this hardcodes /usr/bin/envd and ignores ENVD_BIN, ENVD_EXTRA_ARGS, and ENVD_LOG_FILE=- (inherit stdio, per docs/guide/tutorials/bring-your-own-image.md). Note that open(os.getenv("ENVD_LOG_FILE", ...)) would create a file literally named -. If launch.py replaces the entrypoint contract, it should accept at least these variables.

[cubesandboxbot]

The session DELETE is skipped if kernel.close() raises.

Both cleanup steps share one try, so a failing self.ws.close() (broken connection, aborted close, cancellation during shutdown) prevents client.delete(...) from running — leaving behind the session/kernel created by the successful POST, which is the leak this block is meant to prevent. WebSocketClientProtocol.close() can raise, so this is not a hypothetical. Split the two steps (or wrap delete in finally) so the DELETE is always attempted:

try:
    await kernel.close()
except Exception as cleanup_error:
    logger.warning("failed context close: %s", cleanup_error)
try:
    await client.delete(f"{JUPYTER_BASE_URL}/api/sessions/{session['id']}")
except Exception as cleanup_error:
    logger.warning("failed session cleanup: %s", cleanup_error)
raise

The README claim "closes the connection and attempts to delete that session" then holds unconditionally.

[cubesandboxbot]

A POST timeout can still leave an orphan session/kernel.

httpx.ReadTimeout only means the client gave up; the Jupyter server may still create the session, and nothing reclaims it. The README correctly states the request is not replayed, but it doesn't mention that the result is leaked — a timed-out creation under exactly the cold-start burst this PR targets leaves a kernel running until the sandbox dies.

The request already sends unique UUIDs for path and name, so cleanup is possible without guessing: best-effort GET /api/sessions and delete the single session whose path/name matches those UUIDs. If that is out of scope, please at least document the residual kernel leak in the README limitation paragraph.

[cubesandboxbot]

This CMD replaces whatever startup command the base sandbox-code image ships, but the PR does not build or run the derived image (the description says so). The inherited ENTRYPOINT is still in effect: CubeSandbox images use cube-entrypoint.sh, which already starts envd ... -isnotfc in the background (docker/cube-entrypoint.sh:37-50, docs/guide/tutorials/template-build-practice.md:279-291). If sandbox-code follows that contract, launch.py will start a second envd on the same ENVD_PORT, and the one actually serving :49983 is still the entrypoint's (unsupervised) process.

Please confirm the base image's ENTRYPOINT/CMD split, and run the docker build + docker run smoke check from the README before relying on this. If the entrypoint already launches envd, drop the new Popen (or skip it when envd is already reachable) instead of racing it.

[cubesandboxbot]

The "envd started" line is printed unconditionally: the launcher never checks poll() or /health after spawning envd. If this child exits during initialization (e.g. :49983 already bound, per the entrypoint concern, or a bad envd invocation), main() still starts Jupyter/Uvicorn and /health on :49999 reports ready — so the template goes READY with envd down, and the first run_code fails later with the 502 from _fetch_sandbox_envs.

Since the whole point of this change is reliable startup, consider waiting for envd /health (like _wait_for_jupyter does) or at least failing fast when envd.poll() is not None before starting Uvicorn.

[cubesandboxbot]

Jupyter's default ServerApp.port_retries is 50, so if :8888 is already occupied Jupyter will silently bind 8889+ while JUPYTER_BASE_URL (and hence _wait_for_jupyter) still points at 8888. Startup then spends the full 120×0.5 s ≈ 60 s wait loop and fails with the misleading "Jupyter Server did not become ready".

Since the launcher now owns Jupyter startup, pass --ServerApp.port_retries=0 so a port conflict fails immediately with a clear log line (or parse the actual bound port from Jupyter's output).

[cubesandboxbot]

kernel.close() and the session DELETE share one try. If kernel.close() raises (the WebSocket close path is not guaranteed to succeed), the surrounding except Exception swallows it and client.delete(...) is skipped — leaking the very session this block was added to clean up. The DELETE response is also never checked, so a non-2xx cleanup failure is silent.

Wrap the two steps separately (or use try/finally) so the session deletion is always attempted, and log non-2xx responses.

[cubesandboxbot]

Documentation parity: README_zh.md is the linked translation of this file but none of the new behavior/limits (context-create timeout, entrypoint/launcher, port reservation, -isnotfc) were added there. The Korean/Chinese readers will still see the old build-and-run story, including the old "envd must be available, otherwise run_code returns 502" wording, which no longer matches the new startup-time envd gate. Please mirror this block in README_zh.md (or drop the translation link until it is updated).

[cubesandboxbot]

Both log files are opened directly here, but unlike docker/cube-entrypoint.sh (which runs mkdir -p "$(dirname "${ENVD_LOG_FILE}")" before starting envd) the launcher never creates the parent directory. A user who sets ENVD_LOG_FILE (or the new JUPYTER_LOG_FILE) to a path whose directory does not exist, e.g. ENVD_LOG_FILE=/var/log/e2b/envd.log, now gets FileNotFoundError from open(..., "ab") before any service starts, so the sandbox never boots; the same setting worked with the inherited entrypoint. Consider creating the parent (Path(name).parent.mkdir(parents=True, exist_ok=True)) when the value is not -.

[cubesandboxbot]

_wait_for_envd retries on every HTTPException, but _fetch_sandbox_envs collapses all failures into a 502 HTTPException: HTTP status errors (including a 404 for an envd without /envs — the "endpoint unsupported" case the README calls out), connection/timeouts, and invalid payloads. For the permanent ones the loop can never make progress, so startup spins for the full CONTEXT_CREATE_TIMEOUT (60 s default) and then raises the generic envd did not become ready, discarding the actual 404/invalid-payload cause. Before this change an unsupported /envs produced an immediate 502 on run_code; now it silently costs a minute at every startup. Consider retrying only connection/timeout/5xx failures and failing fast (or surfacing the status/detail) for 4xx or invalid payloads.

[cubesandboxbot]

Making interpreter readiness depend on envd /envs changes the failure mode from degraded to unbootable. With _wait_for_envd in the lifespan, an envd that is missing, that lacks /envs, or that returns a non-retryable error now aborts uvicorn startup: launch.py exits non-zero and /health is never served, so the template's --probe 49999 --probe-path /health can never pass. Before this change the interpreter started and only run_code returned 502 (README, "Environment semantics").

The part that needs verification is timing: this now runs at container boot, before the sandbox is initialized via POST :49983/init (create-time env vars are applied at /init — see docs/guide/tutorials/bring-your-own-image.md:18). If envd answers /envs with 4xx/401/403 before /init, _wait_for_envd classifies it as non-retryable and startup fails. The added tests mock the HTTP layer, and the PR body states no live acceptance was run, so this path is unverified.

Could you confirm that /envs returns 2xx before /init in the target sandbox-code image? If it may not, consider gating readiness on /health and keeping the /envs capability check lazy, or retrying 4xx until the timeout rather than failing immediately.

[cubesandboxbot]

ENTRYPOINT [] also drops the base entrypoint's tini wrapper, so launch.py becomes PID 1. The repo's base image uses ENTRYPOINT ["/usr/bin/tini", "--", "/usr/local/bin/cube-entrypoint.sh"] (docker/Dockerfile.cube-base:97), and the README says the inherited entrypoint is cleared here.

launch.py only wait()s on its two direct children (envd, jupyter). Grandchildren that outlive their parent — e.g. Jupyter kernel processes reparented to PID 1 if the Jupyter server dies, or kernels/commands orphaned when stop_child falls back to kill() after the 5 s timeout — are never reaped and stay as zombies for the container's lifetime. Keeping tini in the chain (e.g. ENTRYPOINT ["/usr/bin/tini", "--"] with the launcher as CMD) would preserve the base image's reaping semantics while still letting the launcher own dependency startup. If the launcher must be PID 1, this limitation should at least be documented/verified.

[cubesandboxbot]

Replacing both the inherited ENTRYPOINT and CMD means the base sandbox-code image's own command no longer runs, so launch.py has to reproduce it exactly. The base image is external to this repo and the PR says the derived image was never built or run, so a mismatch here would only show up as a sandbox that never starts. Please confirm before merge:

• python is on PATH in the base image, and is the interpreter that actually has uvicorn/jupyter installed (not, e.g., only /opt/venv/bin/python).
• /usr/bin/tini exists in sandbox-code (it does in cubesandbox-base, but sandbox-code is built elsewhere).
• The uvicorn server:app invocation in launch.py (host/port, log_level="info", timeout_keep_alive=640) and the Jupyter flag set (--IdentityProvider.token=, --ServerApp.port_retries=0, --ServerApp.root_dir, --ServerApp.allow_root, --ServerApp.disable_check_xsrf, --ServerApp.open_browser) match what the base command used, and nothing else that command did (extra setup, user/working-dir handling, other sidecars) is lost.

A docker run --rm -p 49999:49999 -p 49983:49983 <derived image> plus curl localhost:49999/health and curl localhost:49983/health would settle all of this cheaply.

[cubesandboxbot]

Calling listen() here means TCP connections to the interpreter port now succeed from process start — i.e. throughout the entire lifespan startup, which can be up to _wait_for_jupyter (60 s) + _wait_for_envd (60 s) + session POST/WS/%cd (60 s each) in the worst case. Previously the port was not bound until uvicorn's startup() finished, so connections were refused until the app was ready.

HTTP probes (template creation in the README uses --probe 49999 --probe-path /health) are not fooled — nothing replies until uvicorn accepts — but they will now connect and then wait out their timeout instead of failing fast, and any deployment that probes 49999 with a TCP-connect-only check would report ready before the interpreter is up. Please confirm which probe semantics CubeMaster uses for this template; if TCP-only is ever used, the early listen() turns this fix into a false-positive readiness signal.

[cubesandboxbot]

README_zh.md is the mirrored, cross-linked Chinese version of this file but is not updated here. It still says run_code returns 502 when envd is unavailable and documents neither the new startup semantics, CODE_INTERPRETER_CONTEXT_CREATE_TIMEOUT, the entrypoint/launcher change, nor the -isnotfc rationale. The docs-bilingual-check workflow only covers docs/guide, not examples/, but every other example keeps README.md/README_zh.md in sync — please update README_zh.md in this PR.

Also, the launcher honors JUPYTER_LOG_FILE (default /var/log/jupyter.log) but only the ENVD_* knobs are listed in the "honors ..." sentence above; consider mentioning it.

[cubesandboxbot]

The PR's central mechanism — real uvicorn.Server adopting the pre-bound socket via run(sockets=[...]) — is never exercised: every launch.py test replaces uvicorn.Server with a Mock. If the image's uvicorn version behaves differently (signature, socket ownership/create_server(sock=...) requirements), CI stays green and the container fails at start. Since uvicorn is already installed here, a small test using a trivial ASGI app (uvicorn.Config(app, host="127.0.0.1", port=0)) with Server.run(sockets=[prebound]) in a thread and a GET / would close that gap without needing Jupyter.

Separately, these pins are inline in the workflow while the README recommends running the tests inside the built image to use the same dependencies as the runtime; a shared requirements file (or running the suite in the image) would keep the two from silently diverging.

[cubesandboxbot]

These four new behavior paragraphs are inserted after the ## Build and test the image heading and before From the repository root:, so the build section now starts with startup/launcher prose before its command. They belong in the environment-semantics section above, next to the changed ENVD_TIMEOUT paragraph.

[cubesandboxbot]

README_zh.md is not updated by this PR, so the Chinese mirror still documents the old contract (e.g. that run_code returns 502 whenever envd is unavailable, with no mention of the new startup gate / retry semantics / session cleanup). Please mirror the new paragraphs so the two translations agree.

[cubesandboxbot]

os.getenv(..., default) returns "" when the variable is set but empty, so ENVD_LOG_FILE= or JUPYTER_LOG_FILE= reaches open("", "ab") below and raises FileNotFoundError before any dependency starts. docker/cube-entrypoint.sh uses ${VAR:-default} semantics, i.e. empty means "use the default". Use os.getenv("ENVD_LOG_FILE") or "/var/log/envd.log" (same for JUPYTER_LOG_FILE) so the launcher matches the entrypoint contract it replaces.

[cubesandboxbot]

The build check does not actually validate the "server imports" it is meant to guard. launch.py hands server:app to Uvicorn, and server.py imports fastapi, httpx, starlette, pydantic and from websockets.client import WebSocketClientProtocol, connect — none of which are covered by import uvicorn; import jupyter_server.

That matters here because the README explicitly supports overriding SANDBOX_CODE_IMAGE (international registry, pinned base). A base image with an incompatible/moved websockets (the legacy websockets.client API is the fragile one) or a missing fastapi passes this check and then dies at Config.load() inside Uvicorn — exactly the failure this build-time check exists to catch. Importing the copied module covers the full runtime dependency set:

RUN test -x /usr/bin/tini && command -v python && command -v jupyter \
    && python -c "import sys; sys.path.insert(0, '/opt/lightweight-code-interpreter'); import server"

[cubesandboxbot]

This new paragraph (and the ones below on context-creation timeouts, the tini launcher, and JUPYTER_LOG_FILE) has no counterpart in README_zh.md, which these docs otherwise keep in sync (the two files link to each other). More importantly the Chinese text is now wrong: it still says an unavailable envd or unsupported /envs makes run_code return HTTP 502, whereas startup now fails and the interpreter never becomes ready. Please mirror the update so the two READMEs describe the same startup contract.

[cubesandboxbot]

The compensating DELETE runs on the shared client with the library default timeout (5s), even though this failure branch is entered precisely when Jupyter is slow enough to blow the newly-raised CONTEXT_CREATE_TIMEOUT (60s). Under that same cold-start pressure the cleanup request is likely to time out too, and the exception is swallowed by the except BaseException below, so the session/kernel still leaks until sandbox teardown — which is the outcome this block is trying to avoid.

Worth passing an explicit, smaller-but-not-default timeout, e.g. reuse a short bound so the compensating delete does not add 5s to every failed creation:

await client.delete(
    f"{JUPYTER_BASE_URL}/api/sessions/{session['id']}",
    timeout=min(CONTEXT_CREATE_TIMEOUT, 30),
)

(Not blocking; the current best-effort behaviour is already documented in the PR description.)