Skip to content

feat(sdks/kotlin): add interactive PTY session support#1052

Open
ferponse wants to merge 5 commits into
opensandbox-group:mainfrom
ferponse:feat/sdk-kotlin-pty-sessions
Open

feat(sdks/kotlin): add interactive PTY session support#1052
ferponse wants to merge 5 commits into
opensandbox-group:mainfrom
ferponse:feat/sdk-kotlin-pty-sessions

Conversation

@ferponse

Copy link
Copy Markdown

Summary

execd already implements interactive PTY sessions — the /pty REST endpoints (create / status / delete) and the /pty/{sessionId}/ws WebSocket — but the Kotlin SDK had no way to reach them. This PR exposes the PTY session lifecycle in the SDK and builds the WebSocket URL clients connect to.

Adds a Pty service, accessible via sandbox.pty():

  • createSession(cwd?, command?)POST /pty (the shell starts on first WebSocket attach).
  • getSession(sessionId)GET /pty/{id} (running, outputOffset for replay).
  • deleteSession(sessionId)DELETE /pty/{id}.
  • webSocketUrl(sessionId, mode = PTY, since?, takeover?) — derives the ws/wss URL with the pty=0 / since= / takeover=1 query params, no network call.
PtySession session = sandbox.pty().createSession();
String wsUrl = sandbox.pty().webSocketUrl(session.getSessionId()); // attach with any WS client

Notes / scope

  • The PTY routes are not part of the OpenAPI specs (the interactive channel is a WebSocket), so PtyAdapter is handwritten OkHttp transport, mirroring the existing SSE command stream. No spec/server/generated-code changes.
  • This lands the session lifecycle + URL. Driving the binary WebSocket protocol (stdin/stdout frames, resize, takeover handling) is intentionally left to the caller for now; a follow-up can add an in-SDK WebSocket client if maintainers want it.
  • This is the first client SDK to surface PTY — happy to align Python/Go/JS/C# in follow-ups if you'd like parity.

Testing

  • ./gradlew spotlessCheck :sandbox:test — green (full :sandbox suite: 184 tests, 0 failures).
  • New PtyAdapterTest (MockWebServer) covers create/get/delete request shape and parsing, error status mapping, endpoint-header forwarding, and the webSocketUrl builder (params + wss for https). SandboxTest covers the pty() accessor.

Breaking Changes

None for SDK consumers (additive API). Note: the internal Sandbox constructor gains a ptyService parameter — it is internal, not part of the public surface.

Checklist

  • Conventional Commits
  • Unit tests added
  • Java interop preserved (builders/accessors, no coroutines, java.time unaffected)
  • No generated code edited
  • Open to adding the /pty paths to specs/execd-api.yaml + an E2E in tests/java/ if preferred

Co-authored-by: Atenea Agent srv_atenea_gitlab@ofidona.net

Expose execd's interactive pseudo-terminal sessions in the Kotlin SDK.
execd already serves the /pty endpoints and the /pty/{id}/ws WebSocket;
the SDK had no way to reach them.

- Add a Pty service (createSession / getSession / deleteSession) plus a
  webSocketUrl(sessionId, mode, since, takeover) builder that derives the
  ws/wss URL (PtyMode PTY|PIPE, since-offset replay, takeover) without a
  network call.
- Add PtyAdapter as handwritten OkHttp transport, since the PTY routes are
  not part of the OpenAPI specs (same pattern as the SSE command stream).
- Wire it through AdapterFactory and expose it as Sandbox.pty().

No spec, server, or generated-code changes. Driving the WebSocket itself
is left to the caller; this lands the session lifecycle and URL.

Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ This PR has no labels. Please add one based on the changes.

Changed directories: sdks.

📋 Recommended labels (based on changed files):

  • sdks ⬅️

Other available labels:

  • bug - Something isn't working
  • dependencies - Pull requests that update a dependency file
  • documentation - Improvements or additions to documentation
  • feature - New feature or request
  • packages - Changes for package, image and configuration

💡 Tip: Use feature for new functionality or improvements, bug for fixes.

cc @ferponse

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 889b764cb5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…eaders

- Replace Kotlin default arguments on the Pty interface with explicit
  overloads (createSession / webSocket) so Java callers get usable no-arg
  and partial overloads instead of having to pass every nullable argument.
- Return the WebSocket target as PtyWebSocket(url, headers) instead of a
  bare URL, so the routing/auth headers resolved for the execd endpoint are
  sent on the WebSocket handshake too (header-mode ingress / secure access),
  matching what the REST calls already do.

Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 15fb425238

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

- Document the /pty create/status/delete REST routes in specs/execd-api.yaml
  so the contract the SDK depends on is tracked and generatable (the
  interactive channel stays a WebSocket and is out of scope for codegen).
- Preserve the Sandbox JVM constructor signature: wire the PTY service via an
  internal bindPtyService() after construction instead of adding a parameter,
  avoiding a source/ABI break for already-compiled consumers.
- Map structured execd errors (e.g. 404 CONTEXT_NOT_FOUND, 501 NOT_SUPPORTED)
  to SandboxApiException with the parsed error code and X-Request-ID, like the
  other adapters, instead of a bare status code.
- Honor https domains when building PTY REST/WebSocket URLs even if `protocol`
  is left at its default, mirroring lifecycle base-URL resolution.
- Include the configured ConnectionConfig.headers in the WebSocket handshake
  headers (endpoint headers take precedence on conflicts).

Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7da8490ae

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread specs/execd-api.yaml
Comment thread specs/execd-api.yaml Outdated
Comment thread specs/execd-api.yaml
…apters

- Use ConnectionConfig.protocol for the resolved execd endpoint (REST + ws/wss),
  exactly like CommandsAdapter/HealthAdapter. Deriving https from the lifecycle
  domain broke direct-endpoint mode, where execd is reached on a raw pod IP /
  host-mapped port that serves plain HTTP.
- Revert the /pty additions to specs/execd-api.yaml. Formalizing the PTY REST
  contract in the shared spec is a cross-SDK change (regenerating the committed
  JS/Python/Kotlin clients, aligning the server's real 200 delete status and the
  501 NOT_SUPPORTED responses) and belongs in a dedicated spec PR with parity,
  not this Kotlin SDK feature. The adapter stays handwritten transport for the
  WebSocket-centric PTY protocol.

Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bfdb7e0c91

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

…spec

- Resolve the PTY endpoint scheme by mode: server-proxy endpoints go back
  through the lifecycle server and inherit its scheme (honoring an https
  domain with the default protocol), while direct raw endpoints stay on
  config.protocol like CommandsAdapter. Fixes wrong http/ws for https
  server-proxy deployments without breaking direct deployments.
- Re-add the /pty create/status/delete routes to specs/execd-api.yaml so the
  REST contract the SDK depends on is tracked and generatable, this time
  matching execd's real behavior: DELETE returns 200 (empty success body) and
  all three operations document the 501 NOT_SUPPORTED platform response. The
  interactive channel remains a WebSocket (out of scope for codegen).

Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8fe18b3c1a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread specs/execd-api.yaml
"500":
$ref: "#/components/responses/InternalServerError"

/pty:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Regenerate the execd clients for PTY

Fresh evidence since the earlier thread: this diff re-adds /pty to the source execd spec, but the committed generated clients are still stale (rg "createPtySession|CreatePtySessionRequest" sdks/sandbox/javascript/src/api/execd.ts sdks/sandbox/python/src/opensandbox/api/execd finds no PTY output). Because these generated API packages are published/used downstream, releases or generator checks can omit the new contract or produce uncommitted diffs; regenerate the affected execd clients alongside the spec addition.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant