feat(execd): PTY session lifecycle — spec, regenerated clients, and SDK services#1054
Open
ferponse wants to merge 7 commits into
Open
feat(execd): PTY session lifecycle — spec, regenerated clients, and SDK services#1054ferponse wants to merge 7 commits into
ferponse wants to merge 7 commits into
Conversation
…lients
execd already serves the interactive PTY session lifecycle, but the routes
were never in the source contract, so SDK clients could drift from the server.
Add them to the source of truth and regenerate the generated clients.
- spec: add POST /pty, GET /pty/{sessionId}, DELETE /pty/{sessionId} to
specs/execd-api.yaml, matching execd's real behavior (DELETE returns 200;
all three document the 501 NOT_SUPPORTED platform response). The interactive
/pty/{id}/ws channel stays a WebSocket and is intentionally not modelled.
- python: regenerate the execd client (openapi-python-client) -> adds the pty
API module and Create/Status request/response models.
- javascript: regenerate execd.ts (openapi-typescript) -> adds the /pty path
and schema types.
Kotlin regenerates its execd client from the spec at build time. The Go and C#
execd clients are hand-written (not generated from this spec) and can add PTY
as a follow-up feature when those SDKs choose to expose it.
Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>
ferponse
requested review from
Pangjiping,
hittyt,
jwx0925 and
ninan-nn
as code owners
Contributor
|
Changed directories: sdks、specs. 📋 Recommended labels (based on changed files):
Other available labels:
💡 Tip: Use cc @ferponse |
5 tasks
…YApi Expose execd's PTY session lifecycle in the Kotlin SDK as `sandbox.pty()`, implemented on top of the OpenAPI-generated PTYApi (this PR adds /pty to the execd spec): createSession / getSession / deleteSession. No handwritten transport — request/response mapping and error handling go through the generated client and the shared exception converter, like the other execd adapters. The PTY service is wired after construction via an internal bindPtyService() so the public Sandbox constructor signature is unchanged. The interactive WebSocket attach helper (ws/wss URL + handshake headers) is intentionally out of scope here and will follow in a branch stacked on this one. Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>
…Y client Expose execd's PTY session lifecycle in the Python SDK as `sandbox.pty` (async and sync variants): create_session / get_session / delete_session, implemented on the openapi-python-client generated PTY API. Adds the Pty / PtySync service protocols, PtyAdapter / PtyAdapterSync, PtySession / PtySessionStatus domain models, factory wiring and the Sandbox accessor. Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>
Add CreatePtySession / GetPtySession / DeletePtySession to the hand-written execd client and expose them on the Sandbox, mirroring the existing session helpers. The PtySession / PtySessionStatus types map execd's /pty REST responses (session_id / running / output_offset). The interactive WebSocket channel is driven separately and is out of scope here. Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>
ferponse
changed the title
Author
|
Scope update: this PR now carries the full auto-generatable PTY lifecycle:
|
Expose execd's PTY session lifecycle (create / status / delete) in the remaining SDKs, on top of the clients generated/hand-written from the /pty spec: - JavaScript: ExecdPty service + PtyAdapter (openapi-fetch over execd.ts), wired into the execd stack and exposed as `sandbox.pty`. - C#: IExecdPty + PtyAdapter (HttpClientWrapper), wired into ExecdStack and exposed as `Sandbox.Pty`. Adds PtySession / PtySessionStatus models and unit tests in both SDKs. DELETE is read as a stream in JS to avoid JSON-parsing its empty 200 body. Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2a31a6282
ℹ️ 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".
Address Codex review on opensandbox-group#1054: - Make the execd stack's `pty`/`Pty` member optional in the public `AdapterFactory` / `IAdapterFactory` contract, and install an unavailable-PTY fallback in `Sandbox` when a custom factory omits it (mirrors the existing unavailable-credential-vault fallback). Existing factories and test doubles keep compiling and running; `sandbox.pty` stays defined and fails loudly only on use. - JS: stop forcing the PTY DELETE response through the stream parser. The empty 200 body (Content-Length: 0) is already skipped by openapi-fetch, while error responses (404 CONTEXT_NOT_FOUND, 501 NOT_SUPPORTED) keep their JSON code/message for throwOnOpenApiFetchError to surface. Tests cover the JSON-error path, the empty-success path, and the unavailable-PTY fallback; the C# stack tests no longer set Pty, proving the change is additive. Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9908a47b3f
ℹ️ 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".
`sandbox.pty` is part of the root public API, so export `ExecdPty`, `PtySession`, and `PtySessionStatus` alongside `ExecdCommands` instead of leaving them reachable only via inference or internal paths. Co-authored-by: Atenea Agent <srv_atenea_gitlab@ofidona.net>
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
1 participant
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.
Summary
Closes #1078.
execd already serves the interactive PTY session lifecycle (
POST /pty,GET /pty/{sessionId},DELETE /pty/{sessionId}, plus the/pty/{id}/wsWebSocket), but the REST routes were never inspecs/execd-api.yaml, so the SDK clients could drift from the server protocol. This adds the routes to the source of truth, regenerates the clients that are spec-driven, and exposes a PTY session service across all SDKs.Spec
/ptyREST routes, matching execd's real behavior —DELETEreturns 200 (empty success body) and all three document the 501NOT_SUPPORTEDplatform response. The interactive/pty/{id}/wschannel stays a WebSocket and is intentionally not modelled by OpenAPI.Generated clients (regenerated from the spec)
openapi-python-client) — adds theapi/execd/api/ptymodule and theCreatePtySessionRequest/CreatePtySessionResponse/PtySessionStatusResponsemodels.execd.ts(openapi-typescript@7.13.0, the pinned version) — adds the/ptypath and schema types.PTYApi) from the spec at build time (no committed artifact).Hand-written clients (no spec codegen — PTY transport added by hand)
execd.go) is hand-written, so the/ptytransport was added directly (sandbox_pty.go).Internal/HttpClientWrapper.cs) is hand-written, so the/ptycalls were added directly in the PTY adapter.PTY session service (create / status / delete) — wired into the execd stack and exposed on the sandbox facade in every SDK:
sandbox.pty()(kotlin),sandbox.pty(python async+sync, javascript),Sandbox.CreatePtySession/...(go),Sandbox.Pty(csharp). The PTY stack member is optional in the public adapter-factory contract, with an unavailable-PTY fallback installed bySandbox, so the change stays additive for existing custom factories.Testing
Per SDK (all green):
:sandbox-api:generateExecdApi :sandbox-api:compileKotlin(the generator accepts the spec andPTYApicompiles) +:sandbox:test.uv run ruff check/pyright/pytest(async + sync) after regeneration.build/vet/test.dotnet test.Breaking Changes
Additive routes, types, and an optional stack member (existing adapter factories keep compiling via the unavailable-PTY fallback).
Checklist
sandbox-lifecycle.ymlonly, so the execd spec change needs no docs regenerationNOT_SUPPORTEDon non-Unix); no new auth surfaceCo-authored-by: Atenea Agent srv_atenea_gitlab@ofidona.net