feat(sdk): websocket (JS + Python) - #225
Open
alitariksahin wants to merge 9 commits into
Open
Conversation
Client for the exec-session WebSocket (backend DX-2945). Where exec.command and exec.stream run a command and return its result, a session hands back a handle to a process that is still running: - `argv` runs a program with no shell; `cmd` runs one via `bash -lc`. - `write()` feeds stdin and `endStdin()` closes it, so a command that reads to EOF (`sort`, `cat`) finishes on its own instead of needing a kill. - `onStdout` / `onStderr` receive output as it arrives — separate streams unless `tty` is set, which allocates a real PTY sized by `rows`/`cols` with `resize()` for later changes. - `kill(signal)` sends an allowlisted signal; `terminate(graceMs)` asks the server for SIGTERM then SIGKILL after the grace; `wait()` resolves with the exit code; `close()` hangs up, which also stops the process. The client rejects an empty command locally, times out the handshake rather than hanging, validates signals before sending, and guards write/resize/kill after the session has ended. `ws` becomes a runtime dependency; a minimal structural type keeps `@types/ws` out of the published types. Node-only: authentication uses a request header, which a browser cannot set on a WebSocket handshake. Python has no counterpart yet. check_parity.py does not catch this — its extractor walks one level deep, so nested namespace methods like `exec.session` (and `files.stat`) are outside the gate — so the gap and the blind spot are both recorded in PARITY.md.
- `pid` no longer claims it may be 0 just after start. The server fails the
handshake instead of starting a session it cannot signal, so a client guard on
pid === 0 would be dead code.
- Fix a malformed {@link Box.exec}`.session()` reference.
- State on the handle that the session owns the process: losing the connection
kills it, and sessions cannot be reattached. That is only implied by close()
today, and it is the behavior most likely to surprise.
Drives the session API end to end through the deployed coordinator and agent, alongside the existing integration suite (gated on UPSTASH_BOX_API_KEY, skipped without it). Covers the behaviors a subprocess consumer depends on: split stdout/stderr with the exit code, argv running without a shell while cmd goes through one, stdin plus endStdin() finishing an EOF-reading command, cwd and env overlay, blocked env keys being dropped, terminate() stopping a long-running process, kill() reaping the whole tree, a real PTY at the requested size with interactive input, a long-lived process over multiple round-trips, concurrent sessions without crosstalk, no env leaking between sessions, close() stopping the process, local rejection of an empty command and an unsupported signal, and session writes being visible through the files API.
Closes the last namespace-level gap against @upstash/box. `exec.command` returns after a command finishes; `exec.session` returns as soon as it starts, so a caller can write to stdin, resize a PTY, and signal the process while it runs. session = await box.exec.session(argv=["sort"], on_stdout=chunks.append) await session.write("banana\napple\n") await session.end_stdin() assert await session.wait() == 0 `argv` runs a program with no shell and takes precedence over `cmd`, which goes through `bash -lc`. `tty=True` (with `rows`/`cols`) allocates a PTY sized correctly from the first read. `cwd` resolves against the box's current directory, and `env` overlays `KEY=VALUE` entries. The handle exposes `pid`, `exec_id`, `write`, `end_stdin`, `resize`, `kill`, `terminate`, `wait`, and `close`, and works as a context manager. The session owns the process: closing the handle or losing the connection kills the command. Both handles are hand-written in upstash_box/_exec_session.py rather than generated. The async handle pumps frames with an asyncio task and the sync handle with a reader thread, an asymmetry generate_sync.py cannot produce by token substitution; the generator maps the async names onto the sync pair instead. Frame construction, signal validation, and decoding are shared between them so the wire protocol has a single definition. Adds a websockets>=13 dependency, imported lazily so it only loads when a session is opened, and bumps the package to 0.4.0.
Unit tests drive both handles against a scripted local WebSocket server, so the pump task/thread, exit settling, and teardown run over a real socket rather than a mock. The server's reply table is shared between the async and sync suites, holding both flavors to identical wire behavior. Pure protocol helpers (start frame assembly, signal normalization, URL scheme) are tested directly. Integration tests mirror the @upstash/box suite one-for-one against a real box, sharing a single box across the module: split streams with exit code, argv without a shell, cmd through one, stdin plus EOF, cwd and env overlay, blocked env keys, terminate, tree kill, PTY sizing with interactive input, long-lived round-trips, concurrency, env isolation, close() and context-manager teardown, local validation, and session writes seen through the files API. The sync smoke test gains a session round-trip so the reader thread is exercised against the real server too. Adds a module-scoped `module_opts` fixture for suites that share one box.
The feature shipped without a README entry, so the only description of it lived in the type docs. Documents it alongside exec.command in the same signature style: the stdin/EOF round-trip, argv vs cmd, PTY sizing, cwd/env, the handle surface, and the ownership rule that closing the handle or losing the connection kills the command.
Both handles documented pid as always non-zero while still accepting a started frame that carried pid 0 or no pid at all, handing back a handle whose kill() and terminate() would go nowhere. The backend already fails the handshake rather than starting a session it cannot signal, so make the client hold the same line instead of weakening the claim: a started frame without a usable pid now fails the handshake and closes the socket, in TypeScript and both Python flavors. Also documents where callbacks run. The async handle dispatches on the task pumping the socket and awaits async callbacks; the sync handle dispatches on the reader thread, so calling wait() from inside a callback deadlocks, because the exit it waits for can only arrive on the thread it is blocking. Moves the Python pid/exec_id docs onto class-level annotations. A string literal after an assignment in __init__ is discarded, so those docs reached neither help() nor an IDE.
The filesystem work and exec.session ship as one release, so exec.session does not get a version of its own. Reverts the 0.4.0 bump and moves its changelog entry into the unreleased 0.3.0 section, which stays the tag to cut. (The earlier feat commit message still says it bumps to 0.4.0; leaving that history alone rather than rewriting it.)
There was a problem hiding this comment.
Pull request overview
Adds live WebSocket command sessions to the JavaScript and Python SDKs.
Changes:
- Adds interactive execution handles with stdin, streaming output, PTY resizing, signals, and teardown.
- Implements async/sync Python parity and runtime dependencies.
- Adds documentation, release notes, and extensive tests.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.changeset/exec-session.md |
Records the JavaScript SDK release change. |
pnpm-lock.yaml |
Moves ws to runtime dependencies. |
packages/sdk/package.json |
Adds runtime WebSocket support. |
packages/sdk/README.md |
Documents JavaScript sessions. |
packages/sdk/src/types.ts |
Defines session public types. |
packages/sdk/src/index.ts |
Exports session types. |
packages/sdk/src/client.ts |
Implements JavaScript sessions. |
packages/sdk/src/__tests__/box-exec-session.test.ts |
Tests JavaScript protocol behavior. |
packages/sdk/src/__tests__/integration/exec-session.integration.test.ts |
Tests JavaScript sessions end-to-end. |
packages/python-sdk/pyproject.toml |
Adds the Python WebSocket dependency. |
packages/python-sdk/README.md |
Documents Python sessions. |
packages/python-sdk/PARITY.md |
Records JavaScript/Python parity. |
packages/python-sdk/CHANGELOG.md |
Records the Python SDK feature. |
packages/python-sdk/upstash_box/__init__.py |
Exports Python session handles. |
packages/python-sdk/upstash_box/_exec_session.py |
Implements shared async/sync protocol handling. |
packages/python-sdk/upstash_box/_async/client.py |
Exposes async sessions. |
packages/python-sdk/upstash_box/_sync/client.py |
Exposes generated sync sessions. |
packages/python-sdk/scripts/generate_sync.py |
Maps hand-written session primitives. |
packages/python-sdk/tests/exec_session_server.py |
Provides a shared test server. |
packages/python-sdk/tests/_async/test_exec_session.py |
Tests async session behavior. |
packages/python-sdk/tests/_sync/test_exec_session_sync.py |
Tests sync session behavior. |
packages/python-sdk/tests/integration/conftest.py |
Adds module-scoped integration configuration. |
packages/python-sdk/tests/integration/test_exec_session_async.py |
Tests async sessions end-to-end. |
packages/python-sdk/tests/integration/test_sync_subset.py |
Adds sync integration coverage. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The error branch was the one path that never hung up. Before start it rejected session() and left the connection open, leaking the socket for a session the caller never received. After start it settled wait() with -1 while the socket stayed live, which breaks the handle's ownership contract: the caller sees the session as finished, but the connection keeping the process alive is still open. Every other terminal path (exit, handshake timeout, unusable pid) already closed. The Python client already closed on both paths, so this is TypeScript catching up rather than a shared bug. Adds regression tests on both sides: the two new TypeScript tests hang and fail without the fix, and the Python ones assert the behavior that was already correct so the two stay in step. Reported by Copilot on #239.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (3)
packages/python-sdk/upstash_box/_exec_session.py:474
- Like the async path, this resets the full handshake timeout after every ignored frame. An endpoint that keeps sending malformed or unknown frames can therefore prevent
session()from ever timing out. Use one monotonic deadline and supply the remaining budget to eachrecv()call.
while True:
try:
raw = conn.recv(timeout=timeout_s)
packages/python-sdk/upstash_box/_exec_session.py:283
timeout_sis reapplied to everyrecv(), so malformed or unknown frames arriving more often than the timeout reset the handshake budget forever. This differs from the JS implementation's single handshake timer and can leavesession()hung indefinitely against a noisy or version-mismatched endpoint. Compute one monotonic deadline for the handshake and pass only its remaining time to each receive.
This issue also appears on line 472 of the same file.
while True:
try:
raw = await asyncio.wait_for(conn.recv(), timeout_s)
packages/python-sdk/upstash_box/_exec_session.py:432
- This public
timeoutis passed directly tothreading.Event.wait(), which interprets it as seconds, while the SDK contract says all timeout values are milliseconds (README.md:328-331,PARITY.md:95). A caller following that contract could wait 1,000× longer than intended. Convert milliseconds to seconds here (and update the new tests to pass millisecond values), or explicitly define and document this as an exception before release.
def wait(self, timeout: Optional[float] = None) -> int:
"""Wait for the process to finish and return its exit code (``-1`` if it
was still running at a forced teardown). Raises ``TimeoutError`` if
``timeout`` elapses first."""
if not self._exited.wait(timeout):
raise TimeoutError("exec.session wait timed out")
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.
No description provided.