feat(api): force-stop endpoint for sandboxes stuck in a state change - #5
Merged
Conversation
Closes #2 Daemon (toolbox): - GET /process/exec/connect: WebSocket exec channel with SSH channel semantics. Client frames start/stdin/signal/resize/stdin_eof; server frames stdout/stderr/exit/error. With a command it streams output and reports the exit code (SIGINT on sleep 60 -> 130); without one it spawns an interactive login shell (bash -l, fallback sh) on a PTY with control-char signal delivery and resize support. The exit frame is always sent before close. Built on SessionService + cmdWrapperFormat (no parallel command runner); PTY support is additive-only. - POST /mcp: stateless streamable-HTTP MCP endpoint with tools exec_command, fs_read_file, fs_write_file, fs_list_files. Adds exactly one new dependency: github.com/modelcontextprotocol/go-sdk. Proxy: - SSH access tokens (Authorization: Bearer or ?token= query) are accepted for /{sandboxId}/process/exec/connect and /{sandboxId}/mcp. Tokens are validated per connection via /sandbox/ssh-access/validate (no caching, so revocation blocks new connections immediately) and non-started sandboxes are rejected with an explicit state message, matching the SSH gateway. Sandbox activity keepalive piggybacks on the existing last-activity polling (on connect + interval), same as ssh-gateway. API: - validateSshAccess now allows the proxy via OrGuard (same pattern as the other proxy-reachable endpoints); auth spec updated. Generated/docs: - swag init regenerated toolbox swagger docs; toolbox API clients (go, ts, java, python, python-async) regenerated with the new routes; api clients regenerated with zero diff. New docs page "SSH over HTTPS" documents the wire protocol. Verification: - go build ./apps/daemon/... ./apps/proxy/... - go test ./apps/daemon/... (incl. new unit tests for the WS frame protocol start/stdin/signal/exit and the MCP tool handlers) - golangci-lint run: 0 issues for daemon and proxy - npx nx test api: 54 suites / 562 tests pass - Live smoke over real TCP: exec stdout/stderr/exit, SIGINT -> 130, interactive shell state, concurrent connections, MCP initialize/tools/list/stateless tools/call, fs roundtrip
Daemon: - session: guard nil session.cmd in WriteInput (panic window) - session: loop syscall.Write until all stdin bytes are written (single write(2) can short-write, truncating large frames) - session: make the async stdin holder a single process (exec tail -f /dev/null) so killing the recorded PID drops the FIFO's last writer and CloseInput delivers EOF immediately; previously the sleep child kept stdin open for up to an hour - mcp: fs_read_file opens the file first, rejects non-regular files (e.g. /dev/zero) and reads through LimitReader so growth or special files cannot cause unbounded allocation Proxy: - pass the request context to ValidateSshAccess instead of context.Background() so disconnected clients cancel validation - enforce ensureSandboxStarted for regular Bearer tokens on agent-access paths, matching the SSH-token path Exec WS protocol: - emit error frames on stdin/stdin_eof/signal/resize failures instead of only debug-logging them MCP/swagger contract: - split HandleMCP into per-method handlers so the spec models POST (JSON-RPC body, json+SSE), GET (SSE stream) and DELETE with unique operation IDs; fix malformed ' text/event-stream' media type; regenerate swagger + Go/TS/Java/Python/Ruby clients (MCP clients now accept a message body) Tests: daemon (session/mcp/exec) go test green incl. new /dev/zero and demux-EOF regression tests, nx test api green, alpine/dash smoke for FIFO EOF on holder kill. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Closes #4. A sandbox whose runner crashes mid-operation (snapshotting, creating, ...) keeps pending=true and rejects every lifecycle call with 409 "Sandbox state change in progress"; recover requires state=ERROR which the sandbox never reaches, and the admin recover endpoint has no extra powers. There was no user-facing escape hatch. Add POST /sandbox/{id}/force-stop (same guard stack as /recover: OrganizationAuthContextGuard + SandboxAccessGuard + WRITE_SANDBOXES, audited as force_stop) which: - refuses sandboxes that are not actually stuck, and ones being destroyed - takes the Redis state-change lock to serialize against sync loops - moves the sandbox to ERROR / desiredState=STOPPED with an explicit errorReason — entity invariants then force pending=false, releasing the lock that 409s every other endpoint, and the sandbox becomes eligible for /recover or deletion - fails any incomplete jobs for the sandbox directly at the repository so the unique incomplete-job index no longer blocks new jobs; the job-state handler is deliberately bypassed because its completion handlers could move the sandbox to a different end state, while force-stop's contract is to land on ERROR + STOPPED Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR adds sandbox force-stop recovery, MCP and WebSocket exec access over HTTPS, PTY and command-session support, SSH-token proxy authentication, documentation, OpenAPI definitions, and generated client SDK methods. ChangesSandbox recovery
SSH over HTTPS runtime
Generated clients
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
@CodeRabbit review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
apps/api/src/sandbox/services/sandbox.service.ts (1)
2226-2229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider recording the original stuck state in
errorReason.A fixed generic message loses context (which state/desiredState the sandbox was stuck in) that would help support/debugging investigate recurring force-stops.
♻️ Proposed enhancement
state: SandboxState.ERROR, desiredState: SandboxDesiredState.STOPPED, - errorReason: 'Force-stopped by user while stuck in a state change', + errorReason: `Force-stopped by user while stuck (state=${sandbox.state}, desiredState=${sandbox.desiredState}, pending=${sandbox.pending})`,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/sandbox/services/sandbox.service.ts` around lines 2226 - 2229, Update the force-stop handling around the sandbox state update to include the original stuck state and desired state in errorReason, rather than using only the generic message. Preserve the STOPPED desiredState and existing whereCondition behavior.libs/toolbox-api-client-python-async/daytona_toolbox_api_client_async/api/mcp_api.py (1)
43-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRuff RUF036 fires on all nine
Union[None, ...]sites here, but this is verbatim OpenAPI Generator template output matching every otherapi_*.pyin the package. Hand-editing would be reverted on the next regeneration — if it's failing lint, exclude generated SDK paths in the Ruff config instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/toolbox-api-client-python-async/daytona_toolbox_api_client_async/api/mcp_api.py` around lines 43 - 50, Update the Ruff configuration to exclude the generated async SDK API modules, including mcp_api.py and the other api_*.py files, from RUF036 checks. Do not modify the generated Union declarations; preserve the OpenAPI Generator output so regeneration remains safe.Source: Linters/SAST tools
libs/toolbox-api-client-go/api/openapi.yaml (1)
849-849: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a standard numeric format for generated OpenAPI schemas.
float64is not part of OpenAPI’s standardnumberformats, wherefloatanddoubleare documented. This should be normalized in the upstream swag annotations; update the generatedopenapi.yamloccurrences at lines 849, 953, and 2692 to avoid generators treating it as a custom/unknown format.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/toolbox-api-client-go/api/openapi.yaml` at line 849, Normalize the generated OpenAPI schema numeric formats by replacing each float64 occurrence with the standard double format, including the schemas corresponding to lines 849, 953, and 2692. Update the upstream swag annotations that produce these entries so regeneration preserves the correction.libs/toolbox-api-client-ruby/lib/daytona_toolbox_api_client/api/process_api.rb (1)
409-466: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRuboCop findings on generated code are template artifacts, not new problems.
The flagged issues (ABC size, symbol style, redundant
return, hash syntax) match patterns already present throughout this auto-generated file (e.g.return data, status_code, headers,if !x.nil?guards elsewhere). Since this file is generator-owned ("Do not edit the class manually"), fixing these inline would be reverted on the next generation.Consider excluding generated API directories from RuboCop (or updating the openapi-generator Ruby template) instead of hand-editing this file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/toolbox-api-client-ruby/lib/daytona_toolbox_api_client/api/process_api.rb` around lines 409 - 466, Exclude the generated Ruby API directory from RuboCop checks, or update the OpenAPI generator template that produces ProcessApi#exec_connect_with_http_info to address the findings consistently. Do not hand-edit the generated ProcessApi methods, since regeneration would overwrite those changes.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/sandbox/services/sandbox.service.ts`:
- Around line 2222-2253: Update createSnapshotFromSandbox and the
runV0SnapshotFromSandbox path to detect force-stop cancellation before final
snapshot persistence and result propagation. Recheck the sandbox state
immediately before persisting or otherwise use a shared cancellation signal, and
abort the snapshot when it is already ERROR with desiredState STOPPED so a late
snapshot cannot restore SNAPSHOTTING or win after force-stop.
In `@apps/daemon/pkg/session/exec_support.go`:
- Around line 45-76: Update WriteInput and the commandSession.Start/Execute
stdin setup so clients cannot receive a Gone error while the wrapper is still
asynchronously attaching the FIFO; keep the FIFO open from Execute or
synchronize WriteInput with stdin attachment. Preserve the closed-stdin Gone
response only after the command has definitively closed or exited, rather than
treating transient ENXIO from syscall.Open as closure.
In `@apps/daemon/pkg/toolbox/mcp/tools.go`:
- Around line 86-118: The context-cancellation branch in the polling loop must
forcefully terminate the command before returning. In the ctx.Done() case, call
m.sessionService.SignalDescendants with sessionId and syscall.SIGKILL, then
return the context error; keep the existing timeout handling and result behavior
unchanged.
In `@apps/daemon/pkg/toolbox/process/exec/controller.go`:
- Around line 97-123: Update the writer goroutine in the frame-writing loop to
invoke the connection’s cancel function before returning on ws.WriteMessage
failure, ensuring ctx.Done() propagates to frame emitters and triggers session
cleanup. Keep the existing logging and successful-write behavior unchanged.
In `@apps/daemon/pkg/toolbox/process/exec/demux.go`:
- Around line 46-65: Update matchMarkerAt to return the length of the matched or
partially matched marker, then use that value in Write’s marker advance and
partial-prefix holdback logic instead of len(log.STDOUT_PREFIX). Preserve the
selected marker kind and ensure both STDOUT_PREFIX and STDERR_PREFIX use their
own lengths.
In `@apps/proxy/pkg/proxy/agent_access.go`:
- Around line 72-87: Update ensureSandboxStarted to classify GetSandbox failures
with common_errors.ConvertOpenAPIError and IsRetryableOpenAPIError, matching the
handling in getSshAccessTokenValid. Preserve the existing BadRequestError
mapping for non-retryable failures while propagating or returning retryable
errors so transient API and network failures retain their retryability.
In `@libs/toolbox-api-client-go/api_process.go`:
- Around line 943-1008: Update ProcessAPIService.ExecConnectExecute to support
the 101 WebSocket upgrade path: configure the request from
ProcessAPIExecConnectRequest with the required Connection, Upgrade, and
Sec-WebSocket headers, and avoid reading or replacing the response body for
status 101 so the duplex stream remains usable. Preserve the existing body
buffering and error handling for non-upgrade responses.
In `@libs/toolbox-api-client-go/api/openapi.yaml`:
- Around line 1729-1779: Update the OpenAPI definitions for /mcp
(libs/toolbox-api-client-go/api/openapi.yaml:1729-1779) and GET
/process/exec/connect (libs/toolbox-api-client-go/api/openapi.yaml:1838-1861) to
declare operation-level security with the Bearer scheme, add 401 and 403
responses, and remove the query-token authentication exposure from
/process/exec/connect. Add matching authentication-failure examples to the
corresponding handler annotations, then regenerate the SDKs.
- Around line 1848-1854: Remove the token query parameter from the
`/process/exec/connect` OpenAPI definition unless the server handler actually
supports it; update the specification to match the handler’s accepted
authentication mechanism. Do not document SSH access tokens in the WebSocket
URL, and place any supported token annotation with the daemon authentication
metadata instead.
---
Nitpick comments:
In `@apps/api/src/sandbox/services/sandbox.service.ts`:
- Around line 2226-2229: Update the force-stop handling around the sandbox state
update to include the original stuck state and desired state in errorReason,
rather than using only the generic message. Preserve the STOPPED desiredState
and existing whereCondition behavior.
In `@libs/toolbox-api-client-go/api/openapi.yaml`:
- Line 849: Normalize the generated OpenAPI schema numeric formats by replacing
each float64 occurrence with the standard double format, including the schemas
corresponding to lines 849, 953, and 2692. Update the upstream swag annotations
that produce these entries so regeneration preserves the correction.
In
`@libs/toolbox-api-client-python-async/daytona_toolbox_api_client_async/api/mcp_api.py`:
- Around line 43-50: Update the Ruff configuration to exclude the generated
async SDK API modules, including mcp_api.py and the other api_*.py files, from
RUF036 checks. Do not modify the generated Union declarations; preserve the
OpenAPI Generator output so regeneration remains safe.
In
`@libs/toolbox-api-client-ruby/lib/daytona_toolbox_api_client/api/process_api.rb`:
- Around line 409-466: Exclude the generated Ruby API directory from RuboCop
checks, or update the OpenAPI generator template that produces
ProcessApi#exec_connect_with_http_info to address the findings consistently. Do
not hand-edit the generated ProcessApi methods, since regeneration would
overwrite those changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 125f7388-a207-449e-8ad8-3ecd96e9f8ac
⛔ Files ignored due to path filters (1)
apps/daemon/go.sumis excluded by!**/*.sum
📒 Files selected for processing (61)
apps/api/src/audit/enums/audit-action.enum.tsapps/api/src/sandbox/controllers/sandbox.controller.auth.spec.tsapps/api/src/sandbox/controllers/sandbox.controller.tsapps/api/src/sandbox/services/sandbox.service.tsapps/daemon/go.modapps/daemon/pkg/session/exec_support.goapps/daemon/pkg/session/execute.goapps/daemon/pkg/toolbox/docs/docs.goapps/daemon/pkg/toolbox/docs/swagger.jsonapps/daemon/pkg/toolbox/docs/swagger.yamlapps/daemon/pkg/toolbox/mcp/server.goapps/daemon/pkg/toolbox/mcp/tools.goapps/daemon/pkg/toolbox/mcp/tools_test.goapps/daemon/pkg/toolbox/process/exec/controller.goapps/daemon/pkg/toolbox/process/exec/controller_test.goapps/daemon/pkg/toolbox/process/exec/demux.goapps/daemon/pkg/toolbox/process/exec/demux_test.goapps/daemon/pkg/toolbox/process/exec/session_exec.goapps/daemon/pkg/toolbox/process/exec/shell_exec.goapps/daemon/pkg/toolbox/process/exec/types.goapps/daemon/pkg/toolbox/process/pty/controller.goapps/daemon/pkg/toolbox/process/pty/ephemeral.goapps/daemon/pkg/toolbox/process/pty/session.goapps/daemon/pkg/toolbox/process/pty/types.goapps/daemon/pkg/toolbox/process/pty/websocket.goapps/daemon/pkg/toolbox/server.goapps/docs/src/content/docs/en/ssh-over-https.mdxapps/docs/src/content/i18n/en.jsonapps/docs/src/content/i18n/ja.jsonapps/docs/src/sidebar-config.tsapps/proxy/pkg/proxy/agent_access.goapps/proxy/pkg/proxy/auth.goapps/proxy/pkg/proxy/get_sandbox_target.golibs/toolbox-api-client-go/.openapi-generator/FILESlibs/toolbox-api-client-go/api/openapi.yamllibs/toolbox-api-client-go/api_mcp.golibs/toolbox-api-client-go/api_process.golibs/toolbox-api-client-go/client.golibs/toolbox-api-client-java/.openapi-generator/FILESlibs/toolbox-api-client-java/src/main/java/io/daytona/toolbox/client/api/McpApi.javalibs/toolbox-api-client-java/src/main/java/io/daytona/toolbox/client/api/ProcessApi.javalibs/toolbox-api-client-java/src/test/java/io/daytona/toolbox/client/api/McpApiTest.javalibs/toolbox-api-client-java/src/test/java/io/daytona/toolbox/client/api/ProcessApiTest.javalibs/toolbox-api-client-python-async/.openapi-generator/FILESlibs/toolbox-api-client-python-async/daytona_toolbox_api_client_async/__init__.pylibs/toolbox-api-client-python-async/daytona_toolbox_api_client_async/api/__init__.pylibs/toolbox-api-client-python-async/daytona_toolbox_api_client_async/api/mcp_api.pylibs/toolbox-api-client-python-async/daytona_toolbox_api_client_async/api/process_api.pylibs/toolbox-api-client-python/.openapi-generator/FILESlibs/toolbox-api-client-python/daytona_toolbox_api_client/__init__.pylibs/toolbox-api-client-python/daytona_toolbox_api_client/api/__init__.pylibs/toolbox-api-client-python/daytona_toolbox_api_client/api/mcp_api.pylibs/toolbox-api-client-python/daytona_toolbox_api_client/api/process_api.pylibs/toolbox-api-client-ruby/.openapi-generator/FILESlibs/toolbox-api-client-ruby/lib/daytona_toolbox_api_client.rblibs/toolbox-api-client-ruby/lib/daytona_toolbox_api_client/api/mcp_api.rblibs/toolbox-api-client-ruby/lib/daytona_toolbox_api_client/api/process_api.rblibs/toolbox-api-client/src/.openapi-generator/FILESlibs/toolbox-api-client/src/api.tslibs/toolbox-api-client/src/api/mcp-api.tslibs/toolbox-api-client/src/api/process-api.ts
Comment on lines
+2222
to
+2253
| try { | ||
| const updatedSandbox = await this.sandboxRepository.updateWhere(sandbox.id, { | ||
| updateData: { | ||
| state: SandboxState.ERROR, | ||
| desiredState: SandboxDesiredState.STOPPED, | ||
| errorReason: 'Force-stopped by user while stuck in a state change', | ||
| }, | ||
| whereCondition: { state: sandbox.state }, | ||
| }) | ||
|
|
||
| // Fail incomplete jobs so the unique incomplete-job index no longer | ||
| // blocks new jobs for this sandbox. Written directly (not via | ||
| // JobService.updateJobStatus) so the job-state handler does not run: | ||
| // its completion handlers could legitimately move the sandbox to a | ||
| // different end state (e.g. SNAPSHOT_SANDBOX restores the pre-snapshot | ||
| // state) while force-stop's contract is to land on ERROR + STOPPED. | ||
| const failResult = await this.jobRepository.update( | ||
| { | ||
| resourceType: ResourceType.SANDBOX, | ||
| resourceId: sandbox.id, | ||
| completedAt: null, | ||
| }, | ||
| { | ||
| status: JobStatus.FAILED, | ||
| errorMessage: 'Job superseded by force-stop', | ||
| completedAt: new Date(), | ||
| }, | ||
| ) | ||
| if (failResult.affected) { | ||
| this.logger.warn(`Force-stop of sandbox ${sandbox.id} failed ${failResult.affected} incomplete job(s)`) | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -g '*job*.service.ts' apps/api/src/job apps/api/src/sandbox 2>/dev/null
rg -n "updateJobStatus" apps/api/src -C6Repository: arrrrny/daytona
Length of output: 8212
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## job.service updateJobStatus outline"
ast-grep outline apps/api/src/sandbox/services/job.service.ts --match updateJobStatus --view expanded || true
echo "## job.service updateJobStatus implementation"
sed -n '236,380p' apps/api/src/sandbox/services/job.service.ts
echo "## sandbox forceStop and related lines"
sed -n '2180,2280p' apps/api/src/sandbox/services/sandbox.service.ts
echo "## references to runV0SnapshotFromSandbox / persistSnapshotFromSandbox"
rg -n "runV0SnapshotFromSandbox|persistSnapshotFromSandbox|create.*Sandbox|create.*Snapshot|SnapshotService" apps/api/src/sandbox -C5Repository: arrrrny/daytona
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## JobStatus isValid transition implementation"
sed -n '350,400p' apps/api/src/sandbox/services/job.service.ts
echo "## job-state-handler outline and relevant completion handling"
ast-grep outline apps/api/src/sandbox/services/job-state-handler.service.ts --view expanded | sed -n '1,220p'
rg -n "handleJobCompletion|SNAPSHOT_SANDBOX|restore|snapshot|SandboxState|SandboxDesiredState" apps/api/src/sandbox/services/job-state-handler.service.ts -C4
echo "## v0 snapshot create implementation"
sed -n '280,360p' apps/api/src/sandbox/runner-adapter/runnerAdapter.v0.ts
rg -n "runV0SnapshotFromSandbox|runV0Snapshot|createSnapshotFromSandbox" apps/api/src/sandbox -C6
echo "## snapshot persistence deps and implementation"
sed -n '1,140p' apps/api/src/sandbox/utils/persist-snapshot-from-sandbox.util.ts
sed -n '1,220p' apps/api/src/sandbox/services/snapshot.service.ts
rg -n "forceStop|snapshot.*sandbox|SandboxState.ERROR|SandboxState.*SNAPSHOT|SNAPSHOT_SANDBOX" apps/api/src/sandbox/services/*.ts apps/api/src/sandbox/managers/*.ts -C4Repository: arrrrny/daytona
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## complete forceStop implementation"
rg -n "async forceStop|const validRunners|isEphemeral|createSnapshotManager|pendingSnapshotCountIncrement|runV0SnapshotFromSandbox|force-stop|STORING_SNAPSHOT|SNAPSHOT_SANDBOX|validRunners.forEach|newPromise|pending=false" apps/api/src/sandbox/services/sandbox.service.ts -C5
echo "## runV0SnapshotFromSandbox implementation"
sed -n '1446,1535p' apps/api/src/sandbox/services/sandbox.service.ts
echo "## read-only behavioral simulation of relevant state machines and update path"
python3 - <<'PY'
from enum import Enum
class JobStatus(Enum):
PENDING = 1
IN_PROGRESS = 2
COMPLETED = 3
FAILED = 4
class SandboxState(Enum):
SNAPSHOTTING = 1
ERROR = 2
class SnapshotState(Enum):
ACTIVE = 3
def valid(current, new):
return current == new or (current == JobStatus.PENDING and new in (JobStatus.IN_PROGRESS, JobStatus.FAILED)) or (current == JobStatus.IN_PROGRESS and new in (JobStatus.COMPLETED, JobStatus.FAILED))
job_status = JobStatus.IN_PROGRESS
job_status = JobStatus.FAILED
completed_at = "2026-07-01T12:00:00Z"
print("direct failed update result:", {"status": job_status, "completed_at": completed_at})
print("if later normal callback requests FAILED status:", valid(job_status, JobStatus.FAILED))
print("if later normal callback requests COMPLETED status:", valid(job_status, JobStatus.COMPLETED))
print("v0 snapshot persistence condition in simulation (no db state passed):", True)
PYRepository: arrrrny/daytona
Length of output: 17781
Force-stop should prevent late in-flight snapshots from winning.
JobStatus rejects IN_PROGRESS → COMPLETED after forceStop has set jobs to FAILED, so a delayed job status update cannot move the sandbox back to SNAPSHOTTING. The remaining race is v0 Docker snapshots: runV0SnapshotFromSandbox persists the snapshot and returns previousState: SNAPSHOTTING without checking whether forceStop already set ERROR/STOPPED, so a successful snapshot can still be created after force-stop. Add a shared cancellation signal/check in createSnapshotFromSandbox/v0 snapshot path or gate the final persistence by the sandbox state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/sandbox/services/sandbox.service.ts` around lines 2222 - 2253,
Update createSnapshotFromSandbox and the runV0SnapshotFromSandbox path to detect
force-stop cancellation before final snapshot persistence and result
propagation. Recheck the sandbox state immediately before persisting or
otherwise use a shared cancellation signal, and abort the snapshot when it is
already ERROR with desiredState STOPPED so a late snapshot cannot restore
SNAPSHOTTING or win after force-stop.
Comment on lines
+45
to
+76
| func (s *SessionService) WriteInput(sessionId, commandId string, data []byte) error { | ||
| session, ok := s.sessions.Get(sessionId) | ||
| if !ok { | ||
| return common_errors.NewNotFoundError(errors.New("session not found")) | ||
| } | ||
|
|
||
| if session.cmd == nil || session.cmd.Process == nil { | ||
| return common_errors.NewGoneError(errors.New("session process is not running")) | ||
| } | ||
|
|
||
| if session.cmd.ProcessState != nil && session.cmd.ProcessState.Exited() { | ||
| return common_errors.NewGoneError(errors.New("session process has exited")) | ||
| } | ||
|
|
||
| command, ok := session.commands.Get(commandId) | ||
| if !ok { | ||
| return common_errors.NewNotFoundError(errors.New("command not found")) | ||
| } | ||
|
|
||
| if command.ExitCode != nil { | ||
| return common_errors.NewGoneError(fmt.Errorf("command has already completed with exit code %d", *command.ExitCode)) | ||
| } | ||
|
|
||
| inputFilePath := command.InputFilePath(session.Dir(s.configDir)) | ||
|
|
||
| fd, err := syscall.Open(inputFilePath, syscall.O_WRONLY|syscall.O_NONBLOCK, 0) | ||
| if err != nil { | ||
| if errors.Is(err, syscall.ENXIO) || os.IsNotExist(err) { | ||
| return common_errors.NewGoneError(errors.New("command stdin is closed")) | ||
| } | ||
| return common_errors.NewInternalServerError(fmt.Errorf("failed to open input pipe: %w", err)) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate exec_support.go and session_exec.go =="
fd -a 'exec_support\.go$|session_exec\.go$|execute\.go$' . | sed 's#^\./##'
echo
echo "== relevant source snippets =="
for f in apps/daemon/pkg/session/exec_support.go apps/daemon/pkg/session/session_exec.go apps/daemon/pkg/session/execute.go; do
if [ -f "$f" ]; then
echo "--- $f ($(wc -l < "$f") lines) ---"
if [ "$f" = "apps/daemon/pkg/session/exec_support.go" ]; then
sed -n '1,120p' "$f"
elif [ "$f" = "apps/daemon/pkg/session/session_exec.go" ]; then
sed -n '1,220p' "$f"
elif [ "$f" = "apps/daemon/pkg/session/execute.go" ]; then
sed -n '1,220p' "$f"
fi
fi
done
echo
echo "== search for WriteInput/CloseInput usage and wrapper symbols =="
rg -n "WriteInput|CloseInput|cmdWrapperFormat|Start\(|pump|Execute\\(" apps/daemon/pkg -SRepository: arrrrny/daytona
Length of output: 19389
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cmdWrapperFormat tail =="
sed -n '200,244p' apps/daemon/pkg/session/execute.go
echo
echo "== toolbox exec controller/session startup and pump/readiness =="
sed -n '1,220p' apps/daemon/pkg/toolbox/process/exec/controller.go
echo "--- session_exec.go ---"
sed -n '1,175p' apps/daemon/pkg/toolbox/process/exec/session_exec.go
echo
echo "== websocket/transport write path around Start/stdin frames =="
rg -n "start|StartFrame|stdin|WriteInput\\(|WriteStdin|Start\\(" apps/daemon/pkg/toolbox/process/exec apps/daemon/pkg/toolbox -S
echo
echo "== FIFO ENXIO behavior probe =="
python3 - <<'PY'
import os, subprocess, time, fcntl, select, stat
pipe = '/tmp/coderabbit_fifo_probe_{}'.format(os.getpid())
try:
os.mkfifo(pipe)
# Spawn /bin/cat /dev/null > "$pipe" like the wrapper; wait briefly.
p = subprocess.Popen(['/bin/bash', '-c', 'cat /dev/null > "$pipe"'], env={'pipe': pipe})
time.sleep(0.02)
try:
fd = os.open(pipe, os.O_WRONLY|os.O_NONBLOCK)
os.write(fd, b'hello')
print('open: SUCCESS-write')
os.close(fd)
except FileExistsError as e:
r, _, _ = select.select([p.stdout.fileno()], [], [], 0.5)
s = p.stdout.readline() or p.stderr.readline() or b''
print('open: ENXIO?', 'ENXIO' in str(e))
print('open: errno:', e.errno if hasattr(e, 'errno') else None)
time.sleep(0.05)
# Open after reader has attached.
fd = os.open(pipe, os.O_WRONLY)
os.write(fd, b'hello')
os.close(fd)
print('late_open: SUCCESS-write')
finally:
p.terminate()
p.wait(timeout=2)
finally:
os.unlink(pipe)
PYRepository: arrrrny/daytona
Length of output: 49490
Avoid treating an unready FIFO as closed stdin.
WriteInput opens the command’s stdin FIFO with O_WRONLY|O_NONBLOCK and returns "command stdin is closed" on ENXIO. In commandSession.Start, the wrapper pipes stdin through {. cmdfile; } < "$ip" asynchronously; a client that sends stdin immediately after Start() returns can see a spurious Gone error before the command has opened its stdin. Keep the FIFO open from Execute(), or synchronize WriteStdin/WriteInput to the wrapper after stdin has been attached instead of treating ENXIO as an unrecoverable gone state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/daemon/pkg/session/exec_support.go` around lines 45 - 76, Update
WriteInput and the commandSession.Start/Execute stdin setup so clients cannot
receive a Gone error while the wrapper is still asynchronously attaching the
FIFO; keep the FIFO open from Execute or synchronize WriteInput with stdin
attachment. Preserve the closed-stdin Gone response only after the command has
definitively closed or exited, rather than treating transient ENXIO from
syscall.Open as closure.
Comment on lines
+86
to
+118
| deadline := time.Now().Add(time.Duration(timeout) * time.Second) | ||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return nil, execCommandResult{}, ctx.Err() | ||
| default: | ||
| } | ||
|
|
||
| if exitCode, ok := readExitCode(exitCodePath); ok { | ||
| stdout, stderr := demuxLogFile(logPath) | ||
| out := execCommandResult{Stdout: stdout, Stderr: stderr, ExitCode: exitCode} | ||
| return &mcpsdk.CallToolResult{ | ||
| Content: []mcpsdk.Content{&mcpsdk.TextContent{Text: execResultText(out)}}, | ||
| }, out, nil | ||
| } | ||
|
|
||
| if time.Now().After(deadline) { | ||
| _ = m.sessionService.SignalDescendants(sessionId, syscall.SIGKILL) | ||
| // Give the wrapper a moment to flush output to the log. | ||
| time.Sleep(200 * time.Millisecond) | ||
| stdout, stderr := demuxLogFile(logPath) | ||
| out := execCommandResult{Stdout: stdout, Stderr: stderr, ExitCode: -1} | ||
| return &mcpsdk.CallToolResult{ | ||
| IsError: true, | ||
| Content: []mcpsdk.Content{&mcpsdk.TextContent{ | ||
| Text: fmt.Sprintf("command timed out after %ds\n%s", timeout, execResultText(out)), | ||
| }}, | ||
| }, out, nil | ||
| } | ||
|
|
||
| time.Sleep(execPollInterval) | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate relevant files =="
git ls-files | rg '(^|/)tools\.go$|session|SessionService|execCommand\(|SignalDescendants|Delete\(' || true
echo
echo "== tools.go outline =="
if [ -f apps/daemon/pkg/toolbox/mcp/tools.go ]; then
ast-grep outline apps/daemon/pkg/toolbox/mcp/tools.go || true
echo
sed -n '1,150p' apps/daemon/pkg/toolbox/mcp/tools.go | nl -ba
fi
echo
echo "== SessionService interface/impl candidates =="
rg -n "type SessionService|SignalDescendants|func .*Delete|Delete\(.*session|Exec|sessionService" apps/daemon -S || true
echo
echo "== Search for execCommandResult and related polling implementation =="
rg -n "readExitCode|execCommandResult|execPollInterval|SignalDescendants|ctx.Done\(\)" apps/daemon/pkg/toolbox/mcp -S || trueRepository: arrrrny/daytona
Length of output: 8021
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Relevant files outline =="
for f in $(git ls-files | rg '(^|/)session.*\.go$|sessions\.go$|toolbox.*\.go$' | head -50); do
echo "--- $f ---"
wc -l "$f"
ast-grep outline "$f" | sed -n '1,120p' || true
done
echo
echo "== Defer delete and execCommand bodies =="
rg -n -C 4 "defer .*Delete|Delete\(deleteCtx, sessionId\)|func .*execCommand|ctx.Done\(\)|SignalDescendants\(sessionId" apps/daemon/pkg/toolbox/mcp apps/daemon/pkg -S || true
echo
echo "== Behavioral/structural probe: source text invariant =="
python3 - <<'PY'
from pathlib import Path
p=Path('apps/daemon/pkg/toolbox/mcp/tools.go')
s=p.read_text()
needles=['case <-ctx.Done():','SignalDescendants(sessionId, syscall.SIGKILL)','time.Sleep(execPollInterval)',':= m.sessionService.Delete(deleteCtx, sessionId)']
for n in needles:
print(n, '=>', n in s)
# simple line-numberish check from text is enough
lines=s.splitlines()
print('context_done_line=', next((i+1 for i,l in enumerate(lines) if 'case <-ctx.Done():' in l), None))
print('signal_line=', next((i+1 for i,l in enumerate(lines) if 'SignalDescendants(sessionId, syscall.SIGKILL)' in l), None))
print('delete_line=', next((i+1 for i,l in enumerate(lines) if 'm.sessionService.Delete(deleteCtx, sessionId)' in l), None))
PYRepository: arrrrny/daytona
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== execCommand body =="
cat -n apps/daemon/pkg/toolbox/mcp/tools.go | sed -n '47,118p'
echo
echo "== SessionService Delete implementation =="
cat -n apps/daemon/pkg/session/delete.go | sed -n '1,150p'
echo
echo "== SessionService SignalDescendants implementation =="
cat -n apps/daemon/pkg/session/exec_support.go | sed -n '144,200p'
echo
echo "== Command PID/storage implementation =="
cat -n apps/daemon/pkg/session/types.go | sed -n '28,48p'
cat -n apps/daemon/pkg/session/command.go | sed -n '1,90p'Repository: arrrrny/daytona
Length of output: 12194
Unmanaged process handling on request cancellation.
context.Done() returns immediately, while the timeout path sends SIGKILL first. Although the deferred SessionService.Delete terminates the process tree, it uses SIGTERM/graceful wait before falling back to SIGKILL, so a cancelled request can leave command execution running longer than the explicit timeout path. Signal descendants with SIGKILL on the cancelled-context path before returning to match manual cancellation behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/daemon/pkg/toolbox/mcp/tools.go` around lines 86 - 118, The
context-cancellation branch in the polling loop must forcefully terminate the
command before returning. In the ctx.Done() case, call
m.sessionService.SignalDescendants with sessionId and syscall.SIGKILL, then
return the context error; keep the existing timeout handling and result behavior
unchanged.
Comment on lines
+97
to
+123
| writerDone := make(chan struct{}) | ||
| go func() { | ||
| defer close(writerDone) | ||
| for { | ||
| select { | ||
| case frame := <-frames: | ||
| util.WritePendingPongs(ws, pongCh, time.Second, logger) | ||
|
|
||
| data, err := json.Marshal(frame.payload) | ||
| if err != nil { | ||
| logger.Error("failed to marshal exec frame", "error", err) | ||
| continue | ||
| } | ||
| _ = ws.SetWriteDeadline(time.Now().Add(10 * time.Second)) | ||
| if err := ws.WriteMessage(websocket.TextMessage, data); err != nil { | ||
| logger.Debug("exec ws write error", "error", err) | ||
| return | ||
| } | ||
| if frame.close { | ||
| _ = ws.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, ""), time.Now().Add(time.Second)) | ||
| return | ||
| } | ||
| case <-ctx.Done(): | ||
| return | ||
| } | ||
| } | ||
| }() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Writer failure doesn't cancel the connection context, risking a goroutine/process leak.
When ws.WriteMessage fails, the writer goroutine just returns — it never calls cancel(). cancel() is only invoked in the deferred cleanup at Line 139, which only runs once the read loop's ws.ReadMessage() also errors. If the write side breaks (e.g., a stalled client tripping the 10s write deadline at Line 110) while reads keep succeeding, no one drains frames anymore. Once its 64-slot buffer fills, every subsequent emit() call (session stdout/stderr, or error frames from the read loop) blocks forever on frames <- since ctx.Done() never fires. That blocks the session's emitter goroutines indefinitely, and since sess.Kill() only runs in the same deferred cleanup, the spawned command/shell process also keeps running.
🔒 Proposed fix
_ = ws.SetWriteDeadline(time.Now().Add(10 * time.Second))
if err := ws.WriteMessage(websocket.TextMessage, data); err != nil {
logger.Debug("exec ws write error", "error", err)
+ cancel()
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| writerDone := make(chan struct{}) | |
| go func() { | |
| defer close(writerDone) | |
| for { | |
| select { | |
| case frame := <-frames: | |
| util.WritePendingPongs(ws, pongCh, time.Second, logger) | |
| data, err := json.Marshal(frame.payload) | |
| if err != nil { | |
| logger.Error("failed to marshal exec frame", "error", err) | |
| continue | |
| } | |
| _ = ws.SetWriteDeadline(time.Now().Add(10 * time.Second)) | |
| if err := ws.WriteMessage(websocket.TextMessage, data); err != nil { | |
| logger.Debug("exec ws write error", "error", err) | |
| return | |
| } | |
| if frame.close { | |
| _ = ws.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, ""), time.Now().Add(time.Second)) | |
| return | |
| } | |
| case <-ctx.Done(): | |
| return | |
| } | |
| } | |
| }() | |
| writerDone := make(chan struct{}) | |
| go func() { | |
| defer close(writerDone) | |
| for { | |
| select { | |
| case frame := <-frames: | |
| util.WritePendingPongs(ws, pongCh, time.Second, logger) | |
| data, err := json.Marshal(frame.payload) | |
| if err != nil { | |
| logger.Error("failed to marshal exec frame", "error", err) | |
| continue | |
| } | |
| _ = ws.SetWriteDeadline(time.Now().Add(10 * time.Second)) | |
| if err := ws.WriteMessage(websocket.TextMessage, data); err != nil { | |
| logger.Debug("exec ws write error", "error", err) | |
| cancel() | |
| return | |
| } | |
| if frame.close { | |
| _ = ws.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, ""), time.Now().Add(time.Second)) | |
| return | |
| } | |
| case <-ctx.Done(): | |
| return | |
| } | |
| } | |
| }() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/daemon/pkg/toolbox/process/exec/controller.go` around lines 97 - 123,
Update the writer goroutine in the frame-writing loop to invoke the connection’s
cancel function before returning on ws.WriteMessage failure, ensuring ctx.Done()
propagates to frame emitters and triggers session cleanup. Keep the existing
logging and successful-write behavior unchanged.
Comment on lines
+46
to
+65
| for i < len(buf) { | ||
| kind, isMarker, isPartial := matchMarkerAt(buf, i) | ||
| switch { | ||
| case isMarker: | ||
| d.emitRange(buf[segStart:i]) | ||
| d.current = kind | ||
| i += len(log.STDOUT_PREFIX) | ||
| segStart = i | ||
| case isPartial: | ||
| // Tail of the buffer may be a marker split across chunks — keep | ||
| // it in pending and emit everything before it. | ||
| d.emitRange(buf[segStart:i]) | ||
| d.pending = append(d.pending, buf[i:]...) | ||
| return | ||
| default: | ||
| i++ | ||
| } | ||
| } | ||
| d.emitRange(buf[segStart:]) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate demux.go and related tests =="
fd -a 'demux\.go|demux_test\.go' . | sed 's#^\./##'
echo
echo "== Inspect target file =="
target="$(fd 'demux\.go' . | head -n1 || true)"
if [ -n "$target" ]; then
wc -l "$target"
cat -n "$target"
fi
test_file="$(fd 'demux_test\.go' . | head -n1 || true)"
if [ -n "$test_file" ]; then
echo
echo "== Inspect test file =="
wc -l "$test_file"
cat -n "$test_file"
fi
echo
echo "== Common-go log prefix constants/usages =="
rg -n "STDOUT_PREFIX|STDERR_PREFIX|log\.STDOUT|log\.STDERR" .Repository: arrrrny/daytona
Length of output: 16494
Return the matched prefix length from matchMarkerAt.
Write always advances by len(log.STDOUT_PREFIX) even when STDERR_PREFIX was matched, and the partial-prefix holdback only checks against STDOUT_PREFIX. Make matchMarkerAt return the selected marker length and use it for the advance and partial-bound logic so the demux can’t silently drift if one prefix is changed in the future.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/daemon/pkg/toolbox/process/exec/demux.go` around lines 46 - 65, Update
matchMarkerAt to return the length of the matched or partially matched marker,
then use that value in Write’s marker advance and partial-prefix holdback logic
instead of len(log.STDOUT_PREFIX). Preserve the selected marker kind and ensure
both STDOUT_PREFIX and STDERR_PREFIX use their own lengths.
Comment on lines
+72
to
+87
| func (p *Proxy) ensureSandboxStarted(ctx context.Context, sandboxId string) error { | ||
| sandbox, _, err := p.apiclient.SandboxAPI.GetSandbox(ctx, sandboxId).Execute() | ||
| if err != nil { | ||
| return common_errors.NewBadRequestError(fmt.Errorf("failed to verify sandbox state: %w", err)) | ||
| } | ||
|
|
||
| if sandbox.State == nil || *sandbox.State != apiclient.SANDBOXSTATE_STARTED { | ||
| state := "unknown" | ||
| if sandbox.State != nil { | ||
| state = string(*sandbox.State) | ||
| } | ||
| return common_errors.NewBadRequestError(fmt.Errorf("sandbox is not started (state: %s). Please start the sandbox before attempting to connect", state)) | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
ensureSandboxStarted maps all GetSandbox failures to BadRequestError, including transient/server errors.
Any failure from GetSandbox — network blip, timeout, 5xx from the API — is uniformly wrapped as common_errors.NewBadRequestError (line 74-76). This loses the original error's retryability signal, unlike getSshAccessTokenValid a few lines above in this same file, which classifies errors via common_errors.ConvertOpenAPIError + IsRetryableOpenAPIError before deciding to fail vs. retry. A client (or the proxy's own retry logic) may treat a transient backend hiccup as a permanent 400 here, suppressing a retry that would otherwise succeed.
Consider reusing the same classification pattern here for consistency.
Diff sketch
func (p *Proxy) ensureSandboxStarted(ctx context.Context, sandboxId string) error {
sandbox, _, err := p.apiclient.SandboxAPI.GetSandbox(ctx, sandboxId).Execute()
if err != nil {
- return common_errors.NewBadRequestError(fmt.Errorf("failed to verify sandbox state: %w", err))
+ if openapiErr := common_errors.ConvertOpenAPIError(err); openapiErr != nil {
+ return openapiErr
+ }
+ return common_errors.NewInternalServerError(fmt.Errorf("failed to verify sandbox state: %w", err))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func (p *Proxy) ensureSandboxStarted(ctx context.Context, sandboxId string) error { | |
| sandbox, _, err := p.apiclient.SandboxAPI.GetSandbox(ctx, sandboxId).Execute() | |
| if err != nil { | |
| return common_errors.NewBadRequestError(fmt.Errorf("failed to verify sandbox state: %w", err)) | |
| } | |
| if sandbox.State == nil || *sandbox.State != apiclient.SANDBOXSTATE_STARTED { | |
| state := "unknown" | |
| if sandbox.State != nil { | |
| state = string(*sandbox.State) | |
| } | |
| return common_errors.NewBadRequestError(fmt.Errorf("sandbox is not started (state: %s). Please start the sandbox before attempting to connect", state)) | |
| } | |
| return nil | |
| } | |
| func (p *Proxy) ensureSandboxStarted(ctx context.Context, sandboxId string) error { | |
| sandbox, _, err := p.apiclient.SandboxAPI.GetSandbox(ctx, sandboxId).Execute() | |
| if err != nil { | |
| if openapiErr := common_errors.ConvertOpenAPIError(err); openapiErr != nil { | |
| return openapiErr | |
| } | |
| return common_errors.NewInternalServerError(fmt.Errorf("failed to verify sandbox state: %w", err)) | |
| } | |
| if sandbox.State == nil || *sandbox.State != apiclient.SANDBOXSTATE_STARTED { | |
| state := "unknown" | |
| if sandbox.State != nil { | |
| state = string(*sandbox.State) | |
| } | |
| return common_errors.NewBadRequestError(fmt.Errorf("sandbox is not started (state: %s). Please start the sandbox before attempting to connect", state)) | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/proxy/pkg/proxy/agent_access.go` around lines 72 - 87, Update
ensureSandboxStarted to classify GetSandbox failures with
common_errors.ConvertOpenAPIError and IsRetryableOpenAPIError, matching the
handling in getSshAccessTokenValid. Preserve the existing BadRequestError
mapping for non-retryable failures while propagating or returning retryable
errors so transient API and network failures retain their retryability.
Comment on lines
+943
to
+1008
| // Execute executes the request | ||
| func (a *ProcessAPIService) ExecConnectExecute(r ProcessAPIExecConnectRequest) (*http.Response, error) { | ||
| var ( | ||
| localVarHTTPMethod = http.MethodGet | ||
| localVarPostBody interface{} | ||
| formFiles []formFile | ||
| ) | ||
|
|
||
| localBasePath, err := a.client.cfg.ServerURLWithContext(r.ctx, "ProcessAPIService.ExecConnect") | ||
| if err != nil { | ||
| return nil, &GenericOpenAPIError{error: err.Error()} | ||
| } | ||
|
|
||
| localVarPath := localBasePath + "/process/exec/connect" | ||
|
|
||
| localVarHeaderParams := make(map[string]string) | ||
| localVarQueryParams := url.Values{} | ||
| localVarFormParams := url.Values{} | ||
|
|
||
| if r.token != nil { | ||
| parameterAddToHeaderOrQuery(localVarQueryParams, "token", r.token, "", "") | ||
| } | ||
| // to determine the Content-Type header | ||
| localVarHTTPContentTypes := []string{} | ||
|
|
||
| // set Content-Type header | ||
| localVarHTTPContentType := selectHeaderContentType(localVarHTTPContentTypes) | ||
| if localVarHTTPContentType != "" { | ||
| localVarHeaderParams["Content-Type"] = localVarHTTPContentType | ||
| } | ||
|
|
||
| // to determine the Accept header | ||
| localVarHTTPHeaderAccepts := []string{} | ||
|
|
||
| // set Accept header | ||
| localVarHTTPHeaderAccept := selectHeaderAccept(localVarHTTPHeaderAccepts) | ||
| if localVarHTTPHeaderAccept != "" { | ||
| localVarHeaderParams["Accept"] = localVarHTTPHeaderAccept | ||
| } | ||
| req, err := a.client.prepareRequest(r.ctx, localVarPath, localVarHTTPMethod, localVarPostBody, localVarHeaderParams, localVarQueryParams, localVarFormParams, formFiles) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| localVarHTTPResponse, err := a.client.callAPI(req) | ||
| if err != nil || localVarHTTPResponse == nil { | ||
| return localVarHTTPResponse, err | ||
| } | ||
|
|
||
| localVarBody, err := io.ReadAll(localVarHTTPResponse.Body) | ||
| localVarHTTPResponse.Body.Close() | ||
| localVarHTTPResponse.Body = io.NopCloser(bytes.NewBuffer(localVarBody)) | ||
| if err != nil { | ||
| return localVarHTTPResponse, err | ||
| } | ||
|
|
||
| if localVarHTTPResponse.StatusCode >= 300 { | ||
| newErr := &GenericOpenAPIError{ | ||
| body: localVarBody, | ||
| error: localVarHTTPResponse.Status, | ||
| } | ||
| return localVarHTTPResponse, newErr | ||
| } | ||
|
|
||
| return localVarHTTPResponse, nil | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
Does the OpenAPI Generator support 101 Switching Protocols / WebSocket responses specially in its go-client template, skipping the response body read/rebuild step?
💡 Result:
The OpenAPI Generator's Go client template does not natively support WebSocket 101 Switching Protocols or provide a mechanism to skip the default response body read/buffer step [1][2][3]. The generated Go client is designed primarily for RESTful interactions [3]. Its architecture relies on the Execute method, which performs the network request and, by default, reads the entire response body into memory (slurping) to handle decoding, error checking, and response parsing [4][3]. Even with recent updates to support io.Reader or []byte response types, these still operate within the framework of the existing request/response lifecycle where the body is already buffered by the client's internal Execute logic [4]. There is currently no built-in template configuration or flag to bypass this "read/rebuild" behavior for WebSocket handshakes or streaming responses [5][4][3]. While some other generator languages or tools may have specific WebSocket support, the Go client generator focuses on standard HTTP/REST patterns [6][3]. Consequently, using the generated Go client for WebSockets requires custom implementation outside of the generated code or significant modification of the standard api.mustache and client.mustache templates [2][3].
Citations:
- 1: https://github.com/OpenAPITools/openapi-generator/blob/8a97ea6d/docs/generators/go.md
- 2: https://github.com/OpenAPITools/openapi-generator/blob/8a97ea6d/modules/openapi-generator/src/main/resources/go/client.mustache
- 3: https://deepwiki.com/OpenAPITools/openapi-generator/4.8-go-client-generator
- 4: [go] support io.Reader and []byte response types in client decode OpenAPITools/openapi-generator#23789
- 5: OpenAPITools/openapi-generator@a00549c
- 6: https://github.com/OpenAPITools/openapi-generator?tab=readme-ov-file
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files around libs/toolbox-api-client-go =="
git ls-files | rg '(^|/)libs/toolbox-api-client-go/|toolbox-api-client-go' | head -100
echo
echo "== api_process.go relevant lines =="
wc -l libs/toolbox-api-client-go/api_process.go
sed -n '880,1030p' libs/toolbox-api-client-go/api_process.go
echo
echo "== execConnect occurrences in Go/OpenAPI files =="
rg -n "exec connect|execConnect|exec connect|Sec-WebSocket|Upgrade|Connection|process/exec/connect" . -g '!vendor/**' -g '!node_modules/**' | head -200Repository: arrrrny/daytona
Length of output: 39850
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== OpenAPI execConnect definition =="
sed -n '1820,2005p' libs/toolbox-api-client-go/api/openapi.yaml
echo
echo "== generated TypeScript execConnect request builder =="
sed -n '260,295p' libs/toolbox-api-client/src/api/process-api.ts
echo
echo "== client.go read/rebuild helper =="
sed -n '1,180p' libs/toolbox-api-client-go/client.go
echo
echo "== exec controller handoff =="
sed -n '1,115p' apps/daemon/pkg/toolbox/process/exec/controller.goRepository: arrrrny/daytona
Length of output: 16810
Make execConnect usable over WebSocket by avoiding the default response body rebuild.
/process/exec/connect returns 101 for the duplex exec stream, but the generated Go client always reads the response into memory and replaces Response.Body with a non-writable bytes.Buffer. Add a generated-client path for 101 responses/use a WebSocket client directly with an ExecConnectRequest that can supply Connection: Upgrade, Upgrade: websocket, and Sec-WebSocket-... headers as required by the OpenAPI spec.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/toolbox-api-client-go/api_process.go` around lines 943 - 1008, Update
ProcessAPIService.ExecConnectExecute to support the 101 WebSocket upgrade path:
configure the request from ProcessAPIExecConnectRequest with the required
Connection, Upgrade, and Sec-WebSocket headers, and avoid reading or replacing
the response body for status 101 so the duplex stream remains usable. Preserve
the existing body buffering and error handling for non-upgrade responses.
Comment on lines
+1729
to
+1779
| /mcp: | ||
| delete: | ||
| description: "Terminates the MCP session per the streamable-HTTP transport.\ | ||
| \ The handler is stateless, so this is a no-op acknowledged for transport\ | ||
| \ compliance." | ||
| operationId: MCPDelete | ||
| responses: | ||
| "202": | ||
| content: {} | ||
| description: Session terminated | ||
| summary: MCP endpoint — terminate the session (streamable HTTP) | ||
| tags: | ||
| - mcp | ||
| get: | ||
| description: "Opens the server-sent-event stream of the MCP streamable-HTTP\ | ||
| \ transport. Stateless deployments do not emit unsolicited events, so most\ | ||
| \ clients only need POST." | ||
| operationId: MCPGet | ||
| responses: | ||
| "200": | ||
| content: {} | ||
| description: SSE event stream | ||
| summary: MCP endpoint — open the SSE stream (streamable HTTP) | ||
| tags: | ||
| - mcp | ||
| post: | ||
| description: "Model Context Protocol endpoint (streamable-HTTP transport) exposing\ | ||
| \ sandbox tools: exec_command, fs_read_file, fs_write_file, fs_list_files.\ | ||
| \ The request body is a JSON-RPC 2.0 message (initialize, tools/list, tools/call,\ | ||
| \ ...); the response is a JSON-RPC response or an SSE event stream per the\ | ||
| \ transport. The handler is stateless: plain HTTP clients can call tools without\ | ||
| \ the initialize handshake. Authenticate with a scoped SSH access token (Authorization:\ | ||
| \ Bearer <token>) exactly like /process/exec/connect. NOTE: MCP clients should\ | ||
| \ speak JSON-RPC directly — generated REST clients cannot express the MCP\ | ||
| \ transport." | ||
| operationId: MCPPost | ||
| requestBody: | ||
| content: | ||
| application/json: | ||
| schema: | ||
| type: object | ||
| description: JSON-RPC 2.0 request message (e.g. tools/call) | ||
| required: true | ||
| responses: | ||
| "200": | ||
| content: {} | ||
| description: JSON-RPC response or SSE event stream | ||
| summary: MCP endpoint — send JSON-RPC messages (streamable HTTP) | ||
| tags: | ||
| - mcp | ||
| x-codegen-request-body-name: message |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the spec declares any security schemes and how other paths handle auth.
fd -t f 'openapi.yaml' libs/toolbox-api-client-go/api --exec rg -nP -C3 'securitySchemes|^ security:|security:'
# Look for security annotations in the daemon toolbox handlers.
rg -nP -C2 '`@Security`|securityDefinitions' apps/daemon/pkg/toolbox | head -40Repository: arrrrny/daytona
Length of output: 1776
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant openapi sections =="
sed -n '1700,1795p;1820,1870p;4465,4490p' libs/toolbox-api-client-go/api/openapi.yaml
echo
echo "== repo path annotations / operations for /mcp and /process/exec/connect =="
rg -n -C 4 '(/mcp|/process/exec/connect|`@Security`|//\s*security|`@Description`\(.*token|`@Param`\(.*token|Authorization|ssh access)' apps libs || true
echo
echo "== security occurrence summary in openapi =="
python3 - <<'PY'
from pathlib import Path
p=Path('libs/toolbox-api-client-go/api/openapi.yaml')
s=p.read_text().splitlines()
for i,l in enumerate(s,1):
if 'securitySchemes:' in l or ('security:' in l and 'securitySchemes' not in l):
print(f"{i}: {l}")
PYRepository: arrrrny/daytona
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmp=$(mktemp -d)
echo "== locate openapi.yaml and client api files =="
fd -t f '^openapi\\.yaml$|^mcp_api\\.(go|py|rb)$|^process_api\\.(go|py|rb)$' libs apps | sed -n '1,80p'
echo
echo "== security occurrence summary in library openapi.yaml =="
python3 - <<'PY'
from pathlib import Path
paths=list(Path('libs/toolbox-api-client-go/api').rglob('openapi.yaml'))
for p in paths:
print(f'--- {p}')
for i,l in enumerate(p.read_text().splitlines(),1):
if 'securitySchemes:' in l or (l.strip().startswith('security:') and 'securitySchemes' not in l):
print(f'{i}: {l}')
PY
echo
echo "== exact relevant openapi sections =="
sed -n '1700,1795p;1820,1870p;4465,4490p' libs/toolbox-api-client-go/api/openapi.yaml
echo
echo "== source annotations around /mcp and /process/exec/connect =="
rg -n -C 5 '(/mcp|/process/exec/connect|MCP|ExecConnect|Mcp|process/exec/connect|`@Success`|`@Failure`|`@Security`)' apps/daemon/pkg/toolbox apps/runner apps || true
echo
echo "== generated toolbox clients for /mcp and exec connect auth snippets =="
tmpdir=$(mktemp -d)
python3 - <<'PY'
from pathlib import Path
import re
for root in [Path('libs/toolbox-api-client-go/api'), Path('libs/toolbox-api-client-ruby/lib')]:
print(f'--- {root}')
for p in sorted(root.rglob('*')):
if p.is_file():
txt=p.read_text(errors='ignore')
if any(s in txt for s in ['/mcp', 'MCP', 'm_cp', 'ExecConnect', 'process/exec/connect']):
lines=txt.splitlines()
for i,l in enumerate(lines,1):
if any(s in l for s in ['/mcp', 'MCP', 'm_cp', 'ExecConnect', 'process/exec/connect', 'auth_settings', 'auth_names', 'Authorization']):
start=max(1,i-3); end=min(len(lines),i+8)
print(f'--- {p}:{i}')
for n in range(start,end+1):
print(f'{n}: {lines[n-1][:160]}')
print()
PYRepository: arrrrny/daytona
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== security occurrences in libs/toolbox-api-client-go/api/openapi.yaml =="
python3 - <<'PY'
from pathlib import Path
p=Path('libs/toolbox-api-client-go/api/openapi.yaml')
lines=p.read_text().splitlines()
for i,l in enumerate(lines,1):
if 'securitySchemes:' in l or (l.strip().startswith('security:') and 'securitySchemes' not in l):
print(f'{i}: {l}')
PY
echo
echo "== sections with line numbers =="
python3 - <<'PY'
from pathlib import Path
p=Path('libs/toolbox-api-client-go/api/openapi.yaml')
lines=p.read_text().splitlines()
for start,end in [(1700,1795),(1820,1870),(4465,4490)]:
print(f'--- {start}-{end}')
for n in range(start,end+1):
print(f'{n}: {lines[n-1]}')
PY
echo
echo "== app source annotations around exact path names =="
rg -n -C 3 '`@Path`\("/mcp"\)|'\"\"/mcp\"'|`@Path`\("/process/exec/connect"\)|/\"process/exec/connect\"|`@Security`|`@Param`\("Authorization"|`@Param`\("token"\)|Authorization|ssh access token|mcp\)' apps/daemon apps/runner -g '*.go' || true
echo
echo "== go client generated auth snippets around mcp/exec functions =="
python3 - <<'PY'
from pathlib import Path
p=Path('libs/toolbox-api-client-go/api/client.go')
if not p.exists():
raise SystemExit(1)
lines=p.read_text().splitlines()
for i,l in enumerate(lines,1):
if any(s in l for s in ['authNames', 'auth_settings', 'SetDefaultHeader', 'MCP', 'ExecConnect', 'Mcp', 'process/exec/connect']):
print(f'--- {p}:{i}')
lo=max(1,i-5); hi=min(len(lines),i+8)
for n in range(lo,hi+1):
print(f'{n}: {lines[n-1][:180]}')
PY
echo
echo "== ruby client generated auth snippets around mcp/exec connect =="
rg -n -C 4 'auth_names|auth_settings|/mcp|ExecConnect|process/exec/connect' libs/toolbox-api-client-ruby/lib/daytona_toolbox_api_client | sed -n '1,220p'Repository: arrrrny/daytona
Length of output: 7066
Add Bearer security to /mcp and /process/exec/connect and regenerate the SDKs
/mcp and GET /process/exec/connect both describe Bearer SSH token auth but have no operation-level security, so generated clients leave these calls unauthenticated; /process/exec/connect also exposes token in query instead of using a declared auth scheme. Add security: [{Bearer: []}] plus 401/403 responses, and add matching auth-failure examples in the handler annotations, then regenerate.
📍 Affects 1 file
libs/toolbox-api-client-go/api/openapi.yaml#L1729-L1779(this comment)libs/toolbox-api-client-go/api/openapi.yaml#L1838-L1861
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/toolbox-api-client-go/api/openapi.yaml` around lines 1729 - 1779, Update
the OpenAPI definitions for /mcp
(libs/toolbox-api-client-go/api/openapi.yaml:1729-1779) and GET
/process/exec/connect (libs/toolbox-api-client-go/api/openapi.yaml:1838-1861) to
declare operation-level security with the Bearer scheme, add 401 and 403
responses, and remove the query-token authentication exposure from
/process/exec/connect. Add matching authentication-failure examples to the
corresponding handler annotations, then regenerate the SDKs.
Comment on lines
+1848
to
+1854
| parameters: | ||
| - description: SSH access token (alternative to the Authorization header for | ||
| WS clients that cannot set headers) | ||
| in: query | ||
| name: token | ||
| schema: | ||
| type: string |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the exec/connect token handling and check for TTL/scoping/log redaction.
rg -nP -C5 --type=go '\bQuery\(\s*"token"\s*\)'
rg -nP -C4 --type=go 'ssh.?access.?token|AccessToken' -g '!**/*_test.go' | head -60Repository: arrrrny/daytona
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -u
echo "== repo files =="
git ls-files | sed -n '1,120p'
echo "== locate openapi token query =="
find . -path '*/openapi.yaml' -o -path '*api*' | sed -n '1,200p'
echo "== textual search token query in openapi =="
grep -R -n -C 4 'name: token' . || true
echo "== textual search websocket token/header handling =="
grep -R -n -C 4 'token' . --include='*.go' --include='*.yaml' --include='*.ts' --include='*.js' | sed -n '1,240p'Repository: arrrrny/daytona
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -u
echo "== candidate openapi files =="
git ls-files '*openapi*.yaml' '*openapi*.json' | sed -n '1,20p'
echo "== inspect target lines =="
if [ -f libs/toolbox-api-client-go/api/openapi.yaml ]; then
sed -n '1828,1870p' libs/toolbox-api-client-go/api/openapi.yaml
fi
echo "== focused token/ssh search in tracked files excluding deps =="
rg -n -C 4 --glob '!node_modules/**' --glob '!vendor/**' 'name: token|token|ssh.?access.?token|AccessToken|Authorization header|WS clients|websocket|WebSocket|Query\("token"' . | sed -n '1,260p'Repository: arrrrny/daytona
Length of output: 21980
🏁 Script executed:
#!/bin/bash
set -u
echo "== tracked go/ts files likely containing exec connect/token logic =="
git ls-files '*.go' '*.ts' '*.tsx' '**/*.go' '**/*.ts' | rg 'daytona|api|server|process|websocket|websocket|gateway|SSH|ssh' | sed -n '1,200p'
echo "== focused tracked source search, excluding lock/vendor/deps =="
rg -n -C 4 --glob '!**/node_modules/**' --glob '!**/vendor/**' --glob '!**/poetry.lock' --glob '!**/package-lock.json' --glob '!**/Gemfile.lock' \
'Query\("token"\)|token|ssh.?access.?token|AccessToken|Authorization header|WS clients|websocket|WebSocket|/process/exec/connect|ExecConnect' \
libs server app pkg . 2>/dev/null | sed -n '1,320p'Repository: arrrrny/daytona
Length of output: 32764
🏁 Script executed:
#!/bin/bash
set -u
echo "== focused code interpreter connection/token handling =="
fd -i 'code_interpreter|toolbox|openapi|generate-openapi' . | sed -n '1,200p'
echo "== inspect generated toolbox clients around connect =="
for f in $(fd -i '.*toolbox.*\.go$' .); do
echo "--- $f"
rg -n -C 5 'ExecConnect|/process/exec/connect|token|Authorization|GetConfig|Auth' "$f" | sed -n '1,220p'
done
echo "== inspect generated types around access token/security =="
rg -n -C 4 --type=go 'ContextAccessToken|AccessToken|SSH|token|X-Daytona|Bearer|Authorization' labs libs apps -g '*.go' | sed -n '1,300p' || trueRepository: arrrrny/daytona
Length of output: 27360
🏁 Script executed:
#!/bin/bash
set -u
echo "== apps/daemon/pkg/toolbox controller.go full relevant outline and content =="
if [ -f apps/daemon/pkg/toolbox/controller.go ]; then
wc -l apps/daemon/pkg/toolbox/controller.go
sed -n '1,280p' apps/daemon/pkg/toolbox/controller.go
fi
echo "== apps/daemon/pkg/toolbox all files =="
fd . apps/daemon/pkg/toolbox -x sh -c 'echo "--- $1"; wc -l "$1"; sed -n "1,240p" "$1"' sh {}
echo "== generated api-client-go toolbox api relevant content =="
if [ -f libs/api-client-go/api_toolbox.go ]; then
wc -l libs/api-client-go/api_toolbox.go
sed -n '1,260p' libs/api-client-go/api_toolbox.go
fi
echo "== generated api-client-go auth token constants/types =="
rg -n -C 4 'ContextAccessToken|AccessToken|Authorization|AddDefaultHeader|DefaultHeader|token' libs/api-client-go -g '*.go' | sed -n '1,260p'Repository: arrrrny/daytona
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -u
echo "== apps/daemon/pkg/toolbox/controller.go first 280 lines =="
wc -l apps/daemon/pkg/toolbox/controller.go
sed -n '1,280p' apps/daemon/pkg/toolbox/controller.go
echo "== exec controller first lines =="
wc -l apps/daemon/pkg/toolbox/process/exec/controller.go
sed -n '1,260p' apps/daemon/pkg/toolbox/process/exec/controller.go
echo "== generated api-client-go token/auth snippets =="
python3 - <<'PY'
import pathlib, re
for p in pathlib.Path("libs/api-client-go").glob("*.go"):
text = p.read_text(errors="replace")
if any(s in text for s in ["ContextAccessToken", "AccessToken", "Authorization", "DefaultHeader", "C", "C.Ctx"]):
print(f"--- {p}")
for i,line in enumerate(text.splitlines(),1):
if re.search(r"ContextAccessToken|AccessToken|Authorization|DefaultHeader|C\.C", line, re.I):
lo=max(1,i-4); hi=min(len(text.splitlines()), i+5)
lines=text.splitlines()
for j in range(lo,hi+1):
print(f"{j:5}: {lines[j-1]}")
print()
PY
echo "== generated api-client-go executor connect implementation snippets =="
python3 - <<'PY'
import pathlib, re
for p in pathlib.Path("libs/api-client-go").glob("*.go"):
text = p.read_text(errors="replace")
if re.search(r"ExecConnect|process/exec/connect", text):
print(f"--- {p}")
lines=text.splitlines()
for i,l in enumerate(lines,1):
if re.search(r"ExecConnect|process/exec/connect", l, re.I):
lo=max(1,i-80); hi=min(len(lines),i+160)
for j in range(lo,hi+1):
print(f"{j:5}: {lines[j-1]}")
PYRepository: arrrrny/daytona
Length of output: 38209
Avoid accepting the SSH access token as a query parameter.
The /process/exec/connect handler does not read or validate token, and ?token= is still part of the WebSocket upgrade URL, where it can appear in access logs/browser history. Unless this is a generated-only annotation that needs to match a server-side token query, move it to the daemon annotation or restrict/document it in this spec as a short-lived, sandbox-scoped, single-use secret.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/toolbox-api-client-go/api/openapi.yaml` around lines 1848 - 1854, Remove
the token query parameter from the `/process/exec/connect` OpenAPI definition
unless the server handler actually supports it; update the specification to
match the handler’s accepted authentication mechanism. Do not document SSH
access tokens in the WebSocket URL, and place any supported token annotation
with the daemon authentication metadata instead.
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.
Closes #4
Problem
When a runner crashes or Docker restarts mid-operation, a sandbox gets stuck in a transient state (
snapshotting,creating, …) withpending=true. Every user-facing endpoint then fails:start/stop/delete→409 Sandbox state change in progressrecover→400 Sandbox is not in a recoverable state(requiresstate=ERROR, which the sandbox never reaches)PUT /state→ runner-only/recover→ calls the same service method with no extra powersThe sandbox is permanently wedged short of direct DB edits.
Change
New endpoint
POST /sandbox/{sandboxIdOrName}/force-stop, guarded exactly like/recover(OrganizationAuthContextGuard+SandboxAccessGuard+WRITE_SANDBOXES, API key or JWT), audited with a newforce_stopaction.SandboxService.forceStop:state == desiredState && !pending) — the regularstopendpoint should be used instead — and sandboxes being destroyed.ERROR/desiredState=STOPPEDwith an expliciterrorReason("Force-stopped by user while stuck in a state change"). Entity invariants then forcepending=false, releasing the lock that 409s every other endpoint. The sandbox becomes eligible for/recover(in-place or from-backup where applicable) orDELETE.completedAt IS NULL) directly at the repository, so the unique incomplete-job index no longer blocks new jobs. This deliberately bypassesJobService.updateJobStatus→JobStateHandlerService.handleJobCompletion, whose per-type completion handlers could legitimately move the sandbox to a different end state (e.g.SNAPSHOT_SANDBOXrestores the pre-snapshot state) — force-stop's contract is to land onERROR+STOPPED, and the user can take it from there.Notes / design decisions
ERROR(rather than restoring a guessed prior state) is intentional: the true state of the sandbox after a crashed runner job is unknown,ERRORis the one state that both clearspendingvia invariants and surfaces the situation honestly, and it plugs into the existing recover flow instead of inventing a parallel one.recoverStaleSnapshottingSandboxessweeper (v0-only, 60 min) andhandleStaleJobs(v2, 10–120 min): this endpoint gives users an immediate escape instead of waiting for the sweepers.Verification
nx test api— 54 suites / 563 tests pass, including a newforceStopSandboxauth-guard spec mirroringrecoverSandbox.tsc --noEmiton changed files clean. (The build has 74 pre-existing TS errors inapps/api/src/interceptors/metrics.interceptor.tsfrom the express-5paramstyping — present on the base commit, same class of issue fixed for the audit decorator in eaf5e1b; out of scope here, happy to fix in a follow-up.)🤖 Generated with Claude Code
Summary by CodeRabbit