feat(sdk): add TypeScript SDK (@nvidia/openshell-sdk)

[maxdubrinsky]

Summary

Adds the first native TypeScript SDK for the OpenShell gateway (@nvidia/openshell-sdk), generated from the protobufs over connect-es with no FFI or napi. It covers the v0.1 surface (sandbox lifecycle, health, streamed exec) and wires up codegen, CI, and tagged publishing to GitHub Packages.

Related Issue

Supersedes the shared-FFI-core direction from RFC-0008 (#1764): the frequently-changing surface (RPCs and messages) is already shared through proto/, so each language builds a native client over the generated stubs instead of binding a common Rust core.

Changes

• sdk/typescript/ — new package. OpenShellClient (create/get/list/delete plus waitReady/waitDeleted, health, streamed exec), transport/auth (h2c + Node TLS + OIDC bearer / CF-Access), and typed errors. Stubs are generated with protoc + @bufbuild/protoc-gen-es (pinned protoc 29.6); src/gen/ and dist/ are gitignored, and dist/ ships the compiled stubs so consumers never regenerate.
• Tasks / CI — tasks/typescript.toml (install/proto/typecheck/build/ci/publish); sdk:ts:typecheck added to check; new sdk-typescript job in branch-checks.yml running typecheck, build, and a --dry-run publish that validates the release path.
• Licensing — SPDX enforcement extended to .ts/.tsx/.mts/.cts (skips node_modules and generated gen/); two back-filled headers.
• Release — release.py gains an npm version format; release-tag.yml publishes to GitHub Packages on tag, stamping the version from the tag (repo keeps a 0.0.0 placeholder). Prerelease builds publish under the next dist-tag, stable under latest.

Distribution note: GitHub Packages forces the @nvidia scope and requires consumers to add a project .npmrc and a token even for public installs. The public API matches the eventual GA package (@openshell/sdk on public npm), so only the install specifier changes at GA.

Open decision (for review)

Package name/scope is unsettled; see the resolvable thread on package.json (the name field). Pre-GA is @nvidia/openshell-sdk; the proposed GA name is @openshell/sdk on public npm.

Testing

• TS SDK: protoc codegen, tsc typecheck, and build all green; npm publish --dry-run produces a valid 30-file tarball (dist/** + README + package.json) and restores the placeholder.
• License check green (625 files); ruff clean on changed Python.
• mise run pre-commit: validated in CI. (Locally the alias also builds/tests the Rust workspace, which the sandbox blocks; that workspace is untouched here.)

Not included: a vitest unit suite and a gateway-gated live e2e test, both follow-ups.

Checklist

• Follows Conventional Commits
• Commits are signed off (DCO)
• Architecture docs — n/a (no published subsystem doc added)

[maxdubrinsky]

Naming decision to settle here (resolvable thread).

GH Packages forces the @nvidia scope, so pre-GA ships as @nvidia/openshell-sdk. Proposed GA target: @openshell/sdk on public npm — and reserve the @openshell org now to pre-empt squatting. Public API is identical across the move, so GA is an install-specifier change only.

👍 to confirm, or propose an alternative scope/name.

[github-actions]

🌿 Preview your docs: https://nvidia-preview-pr-2122.docs.buildwithfern.com/openshell

[benoitf]

question: can it generate with namespaces ?

like one sandbox with list() method vs tons or root methods like listSandboxes, createSandbox

const myClient = OpenShellClient.connect(...);
await myClient.sandbox.create(...)
await myClient.sandbox.list(...)
await myClient.gateway.add(...)
await myClient.gateway.list(...)

like the CLI where we have verbs gateway, sandbox, policy, provider , etc.

[maxdubrinsky]

question: can it generate with namespaces ?

like one sandbox with list() method vs tons or root methods like listSandboxes, createSandbox

const myClient = OpenShellClient.connect(...);
await myClient.sandbox.create(...)
await myClient.sandbox.list(...)
await myClient.gateway.add(...)
await myClient.gateway.list(...)

like the CLI where we have verbs gateway, sandbox, policy, provider , etc.

@benoitf good question, no we can't but the client isn't generated so namespacing is something we can just do. Refactored this to be closer to the existing Python SDK with a SandboxClient which gets exported from a OpenShellClient class.

[MarsKubeX]

Question: The v0.1 surface covers non-interactive exec() (server-streaming). Are ExecSandboxInteractive (bidi streaming with TTY, stdin, and window resize) and CreateSshSession + ForwardTcp planned for future TS SDK releases? I saw that the Go SDK prototype already wraps these RPCs.

[maxdubrinsky]

Question: The v0.1 surface covers non-interactive exec() (server-streaming). Are ExecSandboxInteractive (bidi streaming with TTY, stdin, and window resize) and CreateSshSession + ForwardTcp planned for future TS SDK releases? I saw that the Go SDK prototype already wraps these RPCs.

@MarsKubeX ForwardTcp for sure, this was just a first pass at an SDK with more work to come as needed.

[drew]

Feedback from my review agent

1. Support the default mTLS gateway

Anchor: sdk/typescript/src/transport.ts:16-27

Could we include client certificate and private-key material in ConnectOptions before publishing this API? caCert only verifies the server; it cannot authenticate the caller. OpenShell's default local Docker, VM, Homebrew, and Linux-package gateway uses mTLS user authentication, so the SDK currently cannot connect to the standard installed gateway at all. createGrpcTransport can pass cert and key through nodeOptions; I would expose those as a validated pair and add an mTLS transport test.

Validation:

• ConnectOptions exposes only caCert, and buildTransport() passes only ca and rejectUnauthorized to Node TLS.
• architecture/gateway.md:26-31 defines mTLS user authentication as the default local deployment mode.
• docs/about/installation.mdx:41-43 and :60-62 document the client mTLS bundle used by installed macOS and Linux gateways.
• The Rust client passes CA, client certificate, and private key for standard mTLS in crates/openshell-cli/src/tls.rs:399-429.

Confidence: high.

2. Do not make create-time sandbox policy inexpressible

Anchor: sdk/typescript/src/client.ts:46-53

How is a TypeScript caller supposed to create a sandbox with its own filesystem, process, Landlock, or initial network policy? The curated SandboxSpec drops policy, and create() consequently always omits it. setPolicy() cannot repair this later because sandbox-scoped updates require static fields to match the create-time policy. For a sandbox SDK, the safety boundary needs to be expressible at creation; I would expose at least policy, and preferably an advanced full generated spec shape so new template and resource fields do not require an SDK redesign.

Validation:

• SandboxSpec exposes only name, image, labels, environment, providers, and a boolean GPU request.
• SandboxClient.create() at sdk/typescript/src/client.ts:363-374 does not populate spec.policy.
• proto/openshell.proto:313-377 includes policy plus template/runtime/resource controls that the curated type drops.
• The SDK's own README states at sdk/typescript/README.md:100 that static policy fields must match the create-time policy.
• The server only permits sandbox-scoped policy differences described in proto/openshell.proto:1203-1208.

Confidence: high.

3. Make streamed command completion observable in normal TypeScript

Anchor: sdk/typescript/src/client.ts:445-483

Could we change the streaming contract so the exit status is observable from idiomatic for await usage? JavaScript discards an async generator's return value when consumed with for await, which is exactly how the README demonstrates execStream(). A failing pytest therefore produces output and then looks successful to the caller. A { output, done } session like the interactive API, or a discriminated stream event that includes the terminal exit, would make failure impossible to miss. The completion path should also reject if the gateway closes without an exit event instead of returning -1.

Validation:

• execStream() declares its exit-bearing ExecResult as the async generator return value.
• The README example at sdk/typescript/README.md:58-62 uses for await, so it cannot receive that return value.
• An isolated Node experiment confirmed that for await sees yielded chunks but discards the generator's terminal return value; only manual next() calls expose it.
• Both execStream() and execInteractive() initialize the exit code to -1 and accept a clean stream end without a required exit event. The Python SDK rejects this protocol violation at python/openshell/sandbox.py:578-579.

Confidence: high.

4. Make waitReady and waitDeleted honor their timeout

Anchor: sdk/typescript/src/client.ts:412-438

Would these waits ever time out if an individual get() stalls? The deadline is checked only after an unbounded RPC returns, and none of the calls accept an AbortSignal, so a network partition can leave waitReady(name, 30) pending forever. I would pass the remaining deadline into each Connect call and expose cancellation on waits and streams; the timeout argument should bound the returned promise, not just the sleep loop.

Validation:

• Each loop awaits this.get(name) before checking its wall-clock deadline.
• get() calls the Connect client without call options, a deadline, or a signal.
• The same cancellation gap affects exec streams, interactive exec, and forwards, but the wait methods make an explicit timeout promise that is currently not guaranteed.

Confidence: high.

5. Enforce the SSH response trust-boundary contract

Anchor: sdk/typescript/src/client.ts:723-735

Can we validate the CreateSshSession response before returning it? The protobuf contract explicitly requires clients to reject invalid sandbox IDs, tokens, hosts, ports, schemes, and fingerprints because these values feed OpenSSH ProxyCommand construction. This method currently forwards every string from the gateway unchanged, leaving each SDK consumer to rediscover a security invariant that exists only in the proto comments.

Validation:

• proto/openshell.proto:544-575 says clients must reject responses outside the specified character sets and ranges.
• createSshSession() performs no response validation.
• The README presents createSshSession() as the SSH transport primitive, so downstream callers are expected to use these values.

Confidence: high.

6. Respect backpressure on forwarded responses

Anchor: sdk/typescript/src/client.ts:701-705

Could we stop reading the gRPC stream when socket.write() returns false and resume after drain? The current loop keeps pulling sandbox data into Node's socket buffer even when the local client is slow. A large download or long-lived high-throughput forward can therefore grow memory without a bound and eventually take down the SDK process; the tiny echo test does not exercise this path.

Validation:

• The server-to-local relay ignores the boolean returned by net.Socket.write().
• Node uses that return value to signal that callers should wait for drain before writing more.
• The reverse direction has an explicit queue threshold and pause()/resume(), confirming that flow control is part of this implementation's responsibility.

Confidence: high.

Additional API feedback

These do not independently drive the request-changes recommendation, but they are worth settling before the public API becomes expensive to change.

Export a genuinely typed error contract

The PR describes typed errors, but the package exports neither SdkError nor SdkErrorCode; errorCode() returns string | null. Consumers cannot use instanceof, exhaustively switch over supported codes, inspect a preserved Connect status, or distinguish optimistic-concurrency aborts. Export the error class/code union (or a public type guard), preserve the cause/status, and consider a specific conflict/aborted code.

Relevant code: sdk/typescript/src/errors.ts:11-53, sdk/typescript/src/index.ts:35.

Use literal unions for finite SDK states

SandboxRef.phase, Health.status, EffectiveSettingView.scope, and SandboxConfig.policySource are all plain strings even though each comes from a finite protobuf enum. Lowercase literal unions would retain the friendly API while allowing exhaustive TypeScript handling and preventing typos.

Relevant code: sdk/typescript/src/client.ts:41-44, :55-62, :166-181.

Plan for credential lifetime, not only credential injection

oidcToken is captured as one static string. Long-lived agent processes will start failing when it expires, and must replace the whole client. Even if full OIDC refresh remains a follow-up, accepting a per-call async token provider now would avoid freezing a short-lived-token API into the first release.

Relevant code: sdk/typescript/src/transport.ts:16-39.

Do not log any part of an SSH bearer token

The demo prints the first eight characters of the SSH session token. Examples become production code; log only non-secret session metadata.

Relevant code: sdk/typescript/src/demo.ts:86-91.

Align the package-name documentation

The README says the public npm release will use the same package name, while the PR description proposes @openshell/sdk. Resolve the existing naming thread and make the README and release plan agree before publication.

Relevant code: sdk/typescript/README.md:5; existing PR thread on sdk/typescript/package.json:2.

[drew]

Cross-SDK API comparison with Go PR #2271

Compared against PR #2271 at dab9329d983324c90147b0e04f845d02deec01b4.

Conclusion

The approaches are architecturally similar but not yet API-aligned.

Both SDKs use one root client, resource-scoped sub-clients, curated domain types, hidden name-to-ID translation, a shared connection/auth layer, and higher-level operations over generated gRPC clients. That is the same overall SDK pattern.

They diverge at the level consumers will organize applications around. The Go API treats exec, files, SSH, TCP, configuration, policy, services, providers, health, and sandboxes as peer domains. The TypeScript API puts lifecycle, exec, TCP listening, SSH sessions, provider attachments, sandbox configuration, and policy updates on one SandboxClient, leaving only health at the root. This is more than naming or language idiom; it is a different capability taxonomy.

PR #2271 also declares the intended full Go surface upfront while implementing only sandbox operations in this PR. Exec, files, health, providers, policy, services, TCP, SSH, and configuration currently return Unimplemented and are scheduled for follow-up PRs. The comparison below therefore uses the Go interfaces as the intended API contract, not as a claim that every operation works at this commit.

Capability mapping

Area	TypeScript PR #2122	Go PR #2271 intended API	Alignment
Root composition	Root client plus one sandbox sub-client; health stays top-level	Root client with peer sub-clients for every resource domain	Same pattern, different taxonomy
Sandbox lifecycle	Create, get, list, delete, wait-ready, wait-deleted	Create, get, list, delete, wait-ready, watch, logs	Core CRUD matches; observability and deletion waiting differ
Sandbox result	Lean SandboxRef: ID, name, phase, labels, resource version	Full Sandbox: metadata, complete spec, status, conditions, current policy version	Materially different
Create input	Flattened image/environment/providers/GPU boolean; no policy or advanced template fields	Full policy, template, runtime class, resources, user namespaces, driver config, GPU count	Materially different
Name versus ID	Public methods consistently accept sandbox names and resolve IDs internally	Mostly name-based with internal resolution; raw SSH session creation is explicitly ID-based	Largely aligned
Buffered exec	Returns exit code and byte buffers	Returns exit code and byte slices	Aligned
Streaming exec	Async iterable whose hidden generator return carries the exit code	Stream object with explicit Next, ExitCode, and Close lifecycle	Same capability, different completion semantics
Interactive exec	Output iterable plus write, resize, close, and completion promise	Read/write session plus resize, close, and explicit exit status	Closely aligned
TCP forwarding	sandbox.forward() creates a process-local listener and returns its bound address	Separates one tunneled connection (TCP.Forward) from a local listener (TCP.Listen)	Materially different abstraction
SSH	Create/revoke session by sandbox name under the sandbox client	Separate SSH domain: raw ID-based session plus name-based managed tunnel	Partial overlap
Provider attachment	Attach, detach, list on the sandbox client	Same operations on the sandbox client	Aligned
Provider management	Not exposed	Separate provider CRUD/ensure, profiles, and refresh domains	Missing in TypeScript
Configuration	Sandbox get/set-policy/set-setting methods	Separate sandbox/global configuration domain with a general update model	Partial overlap; different ownership
Policy	Only full sandbox policy update through the sandbox client	Initial policy plus separate draft, approval, status, and revision-history domain	Initial-policy gap plus much broader Go surface
Services	Not exposed	Separate HTTP service endpoint domain	Missing in TypeScript
Files	Explicitly out of scope	First-class upload/download domain planned	Product-boundary disagreement
Health	Top-level health()	Health sub-client with Check	Semantically aligned
Watch and logs	Polling waits only	Typed sandbox watch and filtered log retrieval	Missing in TypeScript
Cancellation/deadlines	No public cancellation or per-call deadline	Every operation receives cancellation/deadline context	Material behavioral difference
Client lifecycle	No close/dispose contract	Idempotent client close	Material lifecycle difference
TLS	CA trust and insecure verification; no client certificate/key	CA plus validated client certificate/key pair	Go covers default OpenShell mTLS; TypeScript does not
Auth lifetime	Static OIDC or edge token	Pluggable per-call auth, static token, refreshable token, extra headers	Same layer, substantially different capability
Errors	Internal SdkError; public string parser	Public typed status error, stable code set, predicate helpers, details	Different public contract
Wire isolation	Most types curated, but policy and setting messages are generated protobuf types	Domain package has no protobuf imports	Similar intent, different boundary strength

Shared high-level concepts worth preserving

These concepts are consistent enough to become a cross-language SDK contract:

• A single root client owns one shared gateway connection and composes resource clients.
• Sandbox-facing operations accept the canonical sandbox name and hide RPCs that require the sandbox ID.
• Lifecycle has create/get/list/delete plus a readiness primitive.
• Exec has buffered, incremental, and interactive forms, all reporting a required terminal exit status.
• Provider attachment is a sandbox operation and supports optimistic concurrency.
• Port forwarding and SSH session credentials are managed resources with explicit cleanup.
• Configuration and policy expose revision metadata and preserve 64-bit values without lossy numeric conversion.
• Generated transport types should not become the default public application model.

Decisions needed for alignment

1. Choose one resource taxonomy. The Go domain split scales better as the API grows. TypeScript can still use idiomatic property names, but exec, TCP, SSH, configuration, policy, providers, services, files, health, and sandboxes should have the same conceptual ownership across SDKs.

2. Standardize the sandbox domain object. Decide whether SDK CRUD returns a lightweight reference or the full desired/observed sandbox. Returning different information in each language will force cross-language examples and orchestration code to behave differently.

3. Standardize the create contract. Every SDK needs to express the full safety-relevant create-time policy and an escape hatch for current template/resource fields. The Go shape demonstrates the semantic surface missing from TypeScript.

4. Define streaming completion independently of iteration syntax. Every streaming exec must expose an unavoidable terminal exit status, cancellation, and cleanup. The exact iterator mechanics can remain language-specific.

5. Define two TCP primitives explicitly. A single tunneled byte stream and a bound local multi-connection listener are different resources. The current TypeScript forward corresponds to Go Listen, not Go Forward.

6. Agree on operational parity. Default mTLS, refreshable auth, cancellation/deadlines, typed errors, and client shutdown are baseline SDK behavior rather than language-specific conveniences.

7. Settle the file-transfer boundary. Go declares upload/download as first-class SDK operations while TypeScript explicitly excludes them because the gateway has no file RPC. Either choice can work, but cross-language SDKs should not disagree about whether tar-over-SSH belongs in the SDK contract.

[cv]

Thanks for pushing this forward. The native TypeScript/Connect direction looks great, and the recent additions (mTLS, create-time policy plus rawSpec, bounded waits, and explicit exec exit events) address several important production concerns.

I’m working on a TypeScript project that runs a long-lived worker inside an OpenShell sandbox. It currently maintains its own generated protobuf/gRPC adapter. Ideally, we could replace that adapter with this SDK rather than retain a parallel raw client.

A few capabilities would make that possible:

1. An advanced evidence/raw surface on the shared transport

   The curated SandboxRef is a good default application model, but infrastructure clients sometimes need the complete gateway response. Our use case needs:

   • the full observed Sandbox, including image, policy, environment, template/resources, providers, phase, and resource version;
   • sandbox policy revision/load status (GetSandboxPolicyStatus);
   • cluster inference route/provider/model/version evidence (GetClusterInference);
   • the sandbox watch stream rather than polling readiness.

   Would you consider an explicitly advanced escape hatch? Perhaps raw scoped clients or service factories sharing OpenShellClient’s transport? That would preserve the curated default API without forcing consumers to deep-import generated files or open a second connection.

   It would also be useful for this layer to preserve protobuf distinctions such as an omitted optional Struct versus an explicitly present empty map.

2. Refreshable authentication and complete cancellation

   For long-lived processes, could ConnectOptions accept an async token provider instead of only a static oidcToken? Ideally it would support expiry-aware, single-flight refresh and receive an AbortSignal.

   It would also help if every unary, polling, and streaming operation consistently accepted a signal/deadline, not only selected methods.

3. Long-lived interactive exec lifecycle

   For using execInteractive() as a persistent non-TTY transport, it would be helpful to have:

   • an awaitable/backpressured write();
   • distinct closeInput() and cancel() operations;
   • bounded input buffering;
   • caller cancellation/deadline support;
   • explicit terminal/abnormal-stream completion semantics.

4. Explicit client shutdown and stable errors

   Could the root client expose close() or AsyncDisposable to shut down its shared transport deterministically?

   For errors, retaining stable code, operation, and gRPC status fields separately from the raw gateway message would let applications sanitize user-facing errors without parsing message text.

Finally, documenting the supported gateway/protobuf compatibility range and adding a gateway-backed e2e covering standard mTLS plus interactive-exec cancellation/backpressure would make adopting the SDK as a sole transport much easier.

Would this kind of advanced, evidence-preserving use case fit the intended SDK boundary? It need not widen the normal curated surface; a deliberately advanced but supported layer would be sufficient.

[MarsKubeX]

SDK scope questions from a consumer perspective

Thanks for the SDK work, the TypeScript surface and the recent additions (mTLS, create-time policy via rawSpec, bounded waits) are looking great.

I've been evaluating the SDK as a replacement for our current CLI-based integration, and a few scope questions came up. @drew cross-SDK comparison with the Go PR (#2271) already touches on some of these, but I wanted to ask about them explicitly from a consumer adoption standpoint.

1. Gateway-level provider operations

The SDK exposes sandbox.attach(), sandbox.detach(), and sandbox.listProviders() for provider management scoped to a single sandbox. The proto also defines gateway-level CreateProvider, DeleteProvider, and ListProviders RPCs for managing providers across the entire gateway. Are gateway-level provider operations planned for the TypeScript SDK surface, or is that intentionally left to the CLI?

2. Gateway-level configuration and settings

Similarly, sandbox.getConfig() and sandbox.setSetting() operate at the sandbox scope. For use cases that need to read or write gateway-global settings (e.g. feature flags, inference routing configuration), are gateway-scoped GetGatewayConfig / UpdateConfig RPCs expected to be part of the SDK, or would consumers be expected to use a raw/advanced escape hatch like the one @cv proposed?

3. File transfer

@drew comparison flagged the file-transfer boundary as a cross-SDK alignment decision — Go declares upload/download as first-class, while the TypeScript SDK explicitly scopes it out. For consumers whose workflows depend on injecting files at sandbox creation time (currently handled via --upload in the CLI), is there a recommended SDK-level path? Would the intent be to chain create() + waitReady() + execStream() with a tar pipe, use SSH-based transfer via createSshSession() + forward(), or wait for RFC-0007 (#1590) to land a dedicated RPC?

4. Resource allocation fields in SandboxSpec

The curated SandboxSpec covers name, image, labels, environment, providers, and gpu. Are fields like GPU device selection, CPU, and memory allocation constraints expected to be added to the curated spec, or would those go through a rawSpec / advanced surface?

Thanks for the context — understanding the intended SDK boundary will help plan which integrations can move off the CLI and which should wait for future releases.

[rhuss]

Great work, like it. My agent says: Clean architecture (scoped clients, typed errors, SSH trust-boundary validation, enum drift detection), thorough test suite, and well-structured CI/release automation. Inline comments on specific spots.

[rhuss]

Nit: no coverage thresholds or reporters configured. Adding a coverage.thresholds.lines gate (even modest, like 70%) would prevent regressions as new RPCs are added. Follow-up material.

[rhuss]

For the golang SDK I've enabled a threshold of 80%, with a current coverage of 89%. A good test coverage is really essential in the agentic times. (doesn't have to be that high though)

[rhuss]

Missing waitDeleted test coverage.

waitReady has two tests (timeout, abort) but waitDeleted has zero. It has its own poll loop, its own error mapping, and a different terminal condition (not_found instead of ready). At minimum, test the happy path (get returns NotFound) and the timeout path.

[rhuss]

end(error) resolves waiters before they can observe the error.

This sets this.error, then resolves all waiting promises with {done: true}. In the async iterator, next.done === true triggers the error throw. But if a consumer calls iterator.next() once (not for await), they get {value: undefined, done: true} and might treat it as clean termination, missing the error entirely. The for await path works correctly because the generator checks this.error on re-entry. Subtle edge case.

[rhuss]

Minor: the constructor creates this.grpc = createClient(...) (used only for health()) and this.sandbox = new SandboxClient(transport) which internally creates its own createClient(...). Since connect-es clients are stateless wrappers over the transport this is harmless, but could pass the client instance into SandboxClient to avoid the duplication.

[rhuss]

Nit: connect() is async but synchronous (just calls the constructor). The async signature is forward-compatible (OIDC discovery, cert loading), but a doc note would help callers who might expect connection establishment to happen here.

[rhuss]

[codex:gpt-5.4] OIDC/CF tokens sent over plain HTTP to non-loopback gateways.

The auth interceptor attaches bearer/CF-Access tokens regardless of scheme. A gateway: 'http://remote-host:8080' config silently transmits credentials unencrypted. Consider rejecting token options over http:// unless the host is loopback (127.0.0.1, ::1, localhost), or require an explicit allowInsecureAuth opt-in.

[rhuss]

[codex:gpt-5.4] No error listener on accepted socket during createSshSession() await.

The socket has no error handler between acceptance in the createServer callback and the first await in forwardConnection(). If the client resets the connection during the createSshSession() RPC, Node emits an unhandled error event on the socket, which by default crashes the process. Attach socket.on('error', ...) synchronously before the first await.

[rhuss]

[codex:gpt-5.4] oidcToken and edgeToken are documented as mutually exclusive, but supplying both silently sends only the OIDC token. Consider rejecting configs where both are set:

if (opts.oidcToken !== undefined && opts.edgeToken !== undefined) {
  throw new SdkError('invalid_config', 'oidcToken and edgeToken are mutually exclusive');
}

[rhuss]

[codex:gpt-5.4] SSH response validation checks syntax but not that resp.sandboxId matches the requested sandboxId. A valid token for a different sandbox would be returned to callers or combined with the wrong ID in ForwardTcp. Whether this permits cross-sandbox access depends on server-side token binding, but a client-side mismatch check is cheap defense-in-depth.

[rhuss]

[codex:gpt-5.4] sandboxRef() converts missing metadata into empty IDs/names and resource version "0" instead of throwing. Downstream exec, SSH, and config calls then send a fabricated empty sandbox ID rather than reporting a malformed gateway response. Consider requiring metadata, metadata.id, and metadata.name and throwing invalid_config when absent.

[rhuss]

[codex:gpt-5.4] Two related issues with done settlement:

1. done is resolved after the exit event is yielded. If a caller breaks from the output loop upon receiving exit, the generator never resumes past the yield, so done remains pending forever.

2. A stream failure both throws from output and rejects done. A caller that handles only the output error leaves done as an unhandled rejection.

Consider settling done before yielding exit and rejecting it from finally if still unsettled.

[rhuss]

[codex:gpt-5.4] close() destroys sockets and closes the listener but neither cancels nor awaits active createSshSession/forwardTcp RPCs. closed can resolve while RPCs remain in-flight and tokens remain temporarily unrevoked. Consider giving each connection an AbortController, passing its signal to both RPCs, and awaiting Promise.allSettled() during teardown.

[rhuss]

[codex:gpt-5.4] Response types silently drop proto fields with no raw escape hatch. create/get/list reduce the full proto Sandbox to ID, name, phase, labels, and resource version. This drops created_at_ms, the complete spec, conditions, runtime endpoints, and current_policy_version. Unlike create input (which has rawSpec), responses have no equivalent. Consider exposing a raw field or surfacing the omitted fields explicitly.

[rhuss]

[codex:gpt-5.6-sol] Forward failures are silently discarded. The server callback launches forwardConnection() with void, and the method catches every error without recording or exposing it. Auth failures, invalid tokens, gateway errors, and malformed responses simply destroy the client socket while the ForwardHandle remains healthy. There is no error event or rejected promise through which callers can distinguish a remote failure from an ordinary disconnect. Consider emitting an error event on the handle or exposing a per-connection error callback.

[rhuss]

[codex:gpt-5.6-sol] close() is not idempotent and races with abort. AbortSignal invokes void teardown(), while callers may concurrently call close(). A second server.close() can throw ERR_SERVER_NOT_RUNNING; the abort path discards the resulting rejection. The abort listener is also retained after manual closure. Consider guarding teardown with a once flag and removing the abort listener after close.

[rhuss]

[codex:gpt-5.6-sol] Edge token values are not validated before cookie interpolation. Beyond the mutual-exclusion issue (see earlier comment), an edgeToken containing ; changes the Cookie header structure, and one containing \r\n can inject arbitrary headers. Consider validating token characters or using a proper cookie serializer.

[rhuss]

[codex:gpt-5.6-sol] Host validation enforces a character bag, not the documented host grammar. Values like "[", "::::", or "a]b" pass despite the proto requiring a DNS hostname, IPv4 address, or bracketed IPv6 address. These don't enable shell injection through the currently allowed charset, but malformed values still reach consumers constructing ProxyCommand. The validation also omits the Rust client's 256-byte fingerprint limit.

[rhuss]

[coderabbit] versionPin() leaks a native SyntaxError on malformed input. BigInt(value) throws a raw SyntaxError when value is not a valid integer string (e.g. "abc", "7.5"). This bypasses the SdkError taxonomy, so callers' errorCode() checks won't match. Wrap the conversion:

function versionPin(value: string | undefined): bigint {
  if (!value) return 0n;
  try {
    return BigInt(value);
  } catch {
    throw new SdkError('invalid_config', `expectedResourceVersion is not a u64: '${value}'`);
  }
}

[rhuss]

[coderabbit] AbortSignal.any() requires Node >= 20.3. The package declares engines: >=20, but AbortSignal.any() was added in Node 20.3.0. Consumers on 20.0-20.2 would hit a runtime TypeError. Either bump the engine floor to >=20.3 or add a polyfill/fallback that combines signals manually.

[rhuss]

@maxdubrinsky I've put my review agents army on the PR, and there are some findings. I would recommend to put your agent on that comments and assess them whether these are valid findings (i hopes so, I did a quick review but not necessarily 100% sure) or reject those with some reasoning (so that it convinces my agent :). If there is some contentious, let's discuss it on the human level.

(and also, if those findings are not helpful to you or only noise, let me know, I can do it differently the next time)