fix: critical bugs across execd, server, and SDKs by wishhyt · Pull Request #468 · alibaba/OpenSandbox

wishhyt · 2026-03-17T14:55:36Z

Summary

Fix copy-paste bug in execd auth client where Post() sent PUT requests
Fix panic on empty SQL query and missing rows.Err() check
Fix goroutine/fd leaks in runCommand when cmd.Start() fails
Fix background command stdin still reading from real stdin instead of /dev/null
Exit with non-zero code when execd server fails to start
Remove duplicate sandbox_service instantiation in server lifespan
Add missing execd_endpoint.headers in code-interpreter adapter
Add OSSFS volume backend support in SDK model converter

Testing

Unit tests (existing tests not broken)
Manual verification of each fix

Breaking Changes

None

Checklist

Clearly described motivation per commit
Each commit is atomic and focused
Security impact considered
Backward compatibility considered

Post() was using http.MethodPut instead of http.MethodPost due to a copy-paste error from the Put() method. This caused all Jupyter API calls that used client.Post() to send PUT requests instead of POST. Made-with: Cursor

getQueryType() would panic with index-out-of-range when given an empty or whitespace-only query string because strings.Fields() returns an empty slice. Add a length check before accessing the first element. Also add the missing rows.Err() check after the SELECT iteration loop, which is required to detect errors that cause rows.Next() to return false prematurely. Made-with: Cursor

When cmd.Start() fails, the done channel was never closed, causing the two tailStdPipe goroutines to block forever on <-done. Close the channel and wait for the goroutines before returning. Also defer-close stdout and stderr file descriptors returned by stdLogDescriptor(), which were previously never closed. Made-with: Cursor

os.NewFile(uintptr(syscall.Stdin), os.DevNull) wraps the real stdin (fd 0) with a misleading name, so background commands still read from the process stdin. Use os.Open(os.DevNull) to actually open /dev/null. Made-with: Cursor

When engine.Run() fails (e.g., port already in use), the process logged the error but exited with code 0, making it invisible to process supervisors and container orchestrators. Made-with: Cursor

create_sandbox_service() was called twice: once at module level in lifecycle.py (used by all routes) and once in the lifespan handler in main.py (stored in app.state but never referenced). The duplicate instance maintained separate internal state (expiration timers, OSSFS ref counts, Docker client) that was never synchronized with the one actually serving requests. Made-with: Cursor

CodesAdapter was not including self.execd_endpoint.headers when building HTTP headers, unlike CommandsAdapter in the sandbox SDK which correctly spreads them. This omission could cause requests to fail in server-proxy mode where routing headers are required. Made-with: Cursor

CLAassistant · 2026-03-17T14:55:44Z

All committers have signed the CLA.

Copilot

Pull request overview

This PR addresses several correctness and reliability issues in the execd runtime, the lifecycle server startup flow, and Python SDK adapters/model conversion.

Changes:

Fix execd runtime robustness (SQL empty-query panic prevention, rows.Err() handling, improved command startup/IO handling, non-zero exit on server start failure).
Fix HTTP method bug in execd Jupyter auth client (Post() now actually uses POST).
Improve Python SDKs: propagate execd_endpoint.headers in the code-interpreter adapter and add OSSFS volume backend conversion in the sandbox SDK.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
server/src/main.py	Removes duplicate sandbox service initialization from server lifespan startup path.
sdks/sandbox/python/src/opensandbox/adapters/converter/sandbox_model_converter.py	Adds OSSFS backend support when converting domain `Volume` to API `Volume`.
sdks/code-interpreter/python/src/code_interpreter/adapters/code_adapter.py	Ensures endpoint-required headers are included in execd requests.
components/execd/pkg/runtime/sql.go	Prevents panic on empty SQL; adds `rows.Err()` handling after iteration.
components/execd/pkg/runtime/command.go	Improves command file descriptor cleanup and background stdin handling.
components/execd/pkg/jupyter/auth/client.go	Fixes `Post()` to use `http.MethodPost` instead of PUT.
components/execd/main.go	Exits with non-zero status when execd server fails to start.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

components/execd/pkg/runtime/command.go

components/execd/pkg/runtime/sql.go

components/execd/pkg/jupyter/auth/client.go

components/execd/pkg/runtime/command.go

Pangjiping · 2026-03-17T15:15:47Z

Thanks for submission, please sign the CLA. 😊

Pangjiping · 2026-03-18T04:33:06Z

It looks like there are some linter errors. Could you fix them it?

https://github.com/alibaba/OpenSandbox/actions/runs/23200575894/job/67517987478?pr=468

to_api_volume() only handled host and pvc backends, silently dropping the ossfs configuration. OSSFS volumes created by users would be sent to the API without their backend config, causing mount failures. Map all OSSFS domain fields (bucket, endpoint, credentials, version, options) to the corresponding API model. Made-with: Cursor

wishhyt · 2026-03-18T04:41:43Z

It looks like there are some linter errors. Could you fix them it?

https://github.com/alibaba/OpenSandbox/actions/runs/23200575894/job/67517987478?pr=468

Fixed. The pyright errors were caused by passing str | None fields to a parameter expecting str. Added explicit None checks to narrow the types before constructing the API model.

Pangjiping

LGTM. @ninan-nn Please take a look at this when you have some free time.

ninan-nn

LGTM : >

wishhyt added 7 commits March 17, 2026 22:48

fix(execd): use correct HTTP method in auth client Post()

48695cb

Post() was using http.MethodPut instead of http.MethodPost due to a copy-paste error from the Put() method. This caused all Jupyter API calls that used client.Post() to send PUT requests instead of POST. Made-with: Cursor

fix(execd): fix stdin redirection in background commands

6651047

os.NewFile(uintptr(syscall.Stdin), os.DevNull) wraps the real stdin (fd 0) with a misleading name, so background commands still read from the process stdin. Use os.Open(os.DevNull) to actually open /dev/null. Made-with: Cursor

fix(execd): exit with non-zero code on server startup failure

d759b6a

When engine.Run() fails (e.g., port already in use), the process logged the error but exited with code 0, making it invisible to process supervisors and container orchestrators. Made-with: Cursor

Copilot AI review requested due to automatic review settings March 17, 2026 14:55

wishhyt requested review from Generalwin, Pangjiping, hittyt, jwx0925 and ninan-nn as code owners March 17, 2026 14:55

Copilot started reviewing on behalf of wishhyt March 17, 2026 14:56 View session

Copilot AI reviewed Mar 17, 2026

View reviewed changes

components/execd/pkg/runtime/command.go Show resolved Hide resolved

components/execd/pkg/runtime/sql.go Show resolved Hide resolved

components/execd/pkg/runtime/sql.go Show resolved Hide resolved

Pangjiping reviewed Mar 17, 2026

View reviewed changes

components/execd/pkg/jupyter/auth/client.go Show resolved Hide resolved

components/execd/pkg/runtime/command.go Show resolved Hide resolved

Pangjiping assigned Pangjiping and ninan-nn Mar 17, 2026

Pangjiping added bug Something isn't working component/server component/execd sdk/python labels Mar 17, 2026

wishhyt force-pushed the fix/critical-bugs branch from 5555915 to 8d40208 Compare March 18, 2026 04:36

Pangjiping approved these changes Mar 18, 2026

View reviewed changes

ninan-nn approved these changes Mar 18, 2026

View reviewed changes

ninan-nn merged commit 02cc2fd into alibaba:main Mar 18, 2026
18 checks passed

ninan-nn mentioned this pull request Mar 18, 2026

SDK parity gaps: endpoint headers and OSSFS support are inconsistent across languages #476

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: critical bugs across execd, server, and SDKs#468

fix: critical bugs across execd, server, and SDKs#468
ninan-nn merged 8 commits intoalibaba:mainfrom
wishhyt:fix/critical-bugs

wishhyt commented Mar 17, 2026

Uh oh!

CLAassistant commented Mar 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Pangjiping commented Mar 17, 2026

Uh oh!

Pangjiping commented Mar 18, 2026 •

edited

Loading

Uh oh!

wishhyt commented Mar 18, 2026

Uh oh!

Pangjiping left a comment

Uh oh!

ninan-nn left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

wishhyt commented Mar 17, 2026

Summary

Testing

Breaking Changes

Checklist

Uh oh!

CLAassistant commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Pangjiping commented Mar 17, 2026

Uh oh!

Pangjiping commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wishhyt commented Mar 18, 2026

Uh oh!

Pangjiping left a comment

Choose a reason for hiding this comment

Uh oh!

ninan-nn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CLAassistant commented Mar 17, 2026 •

edited

Loading

Pangjiping commented Mar 18, 2026 •

edited

Loading