FEATURE: Add Session API for pipe-based bash sessions in execd#104
FEATURE: Add Session API for pipe-based bash sessions in execd#104Pangjiping wants to merge 18 commits intoalibaba:mainfrom
Conversation
…t of dependency on global locks.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b20500ddb
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
7d9e73e to
4d5372a
Compare
d2d9b9d to
62e749e
Compare
62e749e to
7d28368
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc145d1587
ℹ️ 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".
| ctx, cancel := context.WithTimeout(context.Background(), wait) | ||
| defer cancel() |
There was a problem hiding this comment.
Propagate caller cancellation into bash session execution
RunInSession passes a request-scoped context, but bashSession.run discards it and always starts from context.Background(). In practice, if the client disconnects or the server cancels the request, the spawned bash process keeps running until the internal timeout (24h by default when timeout_ms is omitted), which can leave long-running commands consuming resources after the API call is gone.
Useful? React with 👍 / 👎.
| s.started = false | ||
| s.env = nil | ||
| s.cwd = "" |
There was a problem hiding this comment.
Terminate running bash process when closing a session
Session close currently only flips in-memory flags and clears state, but never signals the active child process. Since both interrupt and delete paths call this close logic, a running command like sleep 600 will continue executing even after the API reports success, which breaks interrupt semantics and can leave orphaned work running in the container.
Useful? React with 👍 / 👎.
| func (c *Controller) createBashSession(_ *CreateContextRequest) (string, error) { | ||
| session := newBashSession(nil) |
There was a problem hiding this comment.
Apply requested cwd during bash session creation
createBashSession ignores the incoming CreateContextRequest entirely and always initializes the session with default state. As a result, POST /session with a cwd does not actually set the starting working directory, so the first command runs in an unexpected directory until callers manually cd.
Useful? React with 👍 / 👎.
| if err := c.bindJSON(&request); err != nil { | ||
| c.RespondError( |
There was a problem hiding this comment.
Accept empty request bodies for session creation
CreateSession always decodes JSON and treats decode errors as bad requests, so an empty body returns EOF and 400. Because CreateSessionRequest has no required fields and callers should be able to create a session without options, this makes a no-body POST /session fail unless clients send {} explicitly.
Useful? React with 👍 / 👎.
Summary
This PR adds a dedicated Session API to the execd web layer, exposing three endpoints for creating, running code in, and deleting pipe-based bash sessions. These sessions are independent of the existing code execution flow: the existing Bash language context (e.g.
POST /code/contextwithlanguage: bash) continues to use the Jupyter kernel; the new Session API uses the in-process, pipe-based bash implementation only.Motivation
/codeand/code/contextAPIs are built around language runtimes (Jupyter for Bash/Python/etc.). Keeping the new session lifecycle separate avoids mixing concerns and preserves backward compatibility for Bash-as-a-language.New API
All routes live under the
/sessiongroup and use the same auth middleware as other execd APIs (e.g. access token header when configured).POST/sessionsession_id.POST/session/:sessionId/run/code).DELETE/session/:sessionIdRequest / response
Create session
{ "cwd": "/optional/working/dir" }{ "session_id": "<uuid>" }Run in session
{ "code": "<shell script>", "cwd": "<optional>", "timeout_ms": <optional, non-negative> }init,stdout,stderr,execution_complete,error), same as/code.Delete session
sessionIdis not found.Runtime (codeRunner) changes
Controller
jupyterClientMap,defaultLanguageSessions, andcommandClientMapare nowsync.Mapfor concurrent access;bashSessionClientMap(alsosync.Map) was added for pipe-based bash sessions.CreateContextandExecuteforlanguage: bashstill use the Jupyter backend. No behavior change for existing/codeand/code/contextcallers.Session-only entrypoints (used only by the new API)
CreateBashSession(req *CreateContextRequest) (string, error)— creates a pipe-based bash session and returns its ID.RunInBashSession(ctx, req *ExecuteCodeRequest) error— runs code in an existing session;req.Contextmust be the session ID.DeleteBashSession(sessionID string) error— closes the session and removes it from the map.Context / cleanup
DeleteContext/GetContext/ListContextcontinue to operate only on Jupyter-backed contexts. Pipe-based sessions are not listed or deleted via these; they are managed solely via the new Session API.Web layer
pkg/web/model/session.go):CreateSessionRequest,CreateSessionResponse,RunInSessionRequest(with validation).CodeInterpretingController):CreateSession,RunInSession,DeleteSession, each calling the corresponding runtime method above.pkg/web/router.go): new/sessiongroup with the three routes.Testing
Breaking Changes
Checklist