expose deleteSession

[whoiskatrin]

• expose deleteSession(sessionId) method
• prevent deletion of default session (throws error with guidance to use sandbox.destroy())
• session cleanup kills all running commands in parallel before destroying shell
• return structured SessionDeleteResult with success status, sessionId, and timestamp

[changeset-bot]

🦋 Changeset detected

Latest commit: 1019db7

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[claude]

Claude Code Review

PR #186 exposes the deleteSession API with proper safeguards. The implementation is solid overall with comprehensive testing.

Architecture & Implementation

Strengths:

• Proper three-layer flow: SDK → Handler → SessionManager → Session
• Default session protection prevents accidental deletion (throws with clear guidance)
• Session cleanup kills running commands in parallel before destroying shell (session.ts:557-560)
• Comprehensive handler validation (missing sessionId, invalid JSON)
• Good test coverage at all layers (unit + E2E)

Issues Found

1. Type mismatch in session-handler.ts:140

The handler returns the result directly, but there's a type mismatch:

const result = await this.sessionManager.deleteSession(sessionId);

if (result.success) {
  const response = {
    success: true,
    sessionId,
    timestamp: new Date().toISOString()
  };
  return this.createTypedResponse(response, context);
}

SessionManager.deleteSession() returns ServiceResult<void> (success + optional data), but the handler constructs a new response object with timestamp. The result from SessionManager doesn't have sessionId or timestamp fields - those are added in the handler layer.

Fix: This is actually correct behavior - the handler is responsible for creating the HTTP response shape. However, the naming could be clearer since result shadows the semantic meaning. Consider serviceResult or just inline the check.

2. Missing type export in packages/sandbox/src/index.ts

The changeset mentions returning SessionDeleteResult, but I don't see this type exported from the main SDK index. The types DeleteSessionRequest and DeleteSessionResponse are exported (lines 61-64), but SessionDeleteResult comes from @repo/shared.

Verification needed: Confirm whether SessionDeleteResult should be re-exported from the main SDK package for consumer convenience, or if using the HTTP client types is sufficient.

Minor Observations

• E2E test cleanup is thorough - verifies deletion works and sandbox remains operational afterward
• The any cast in unit tests (mockSessionManager.deleteSession as any) is acceptable for mocking
• TypeScript non-null assertions (currentSandboxId!) in E2E tests are fine given the test structure

Verdict

The implementation is production-ready. The type mismatch concern in #1 is actually not an issue upon closer inspection - the handler correctly transforms the service result into an HTTP response. Consider the export question in #2, otherwise this is good to merge.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/sandbox-sdk/@cloudflare/sandbox@186

commit: 1019db7

[github-actions]

🐳 Docker Image Published

FROM cloudflare/sandbox:0.0.0-pr-186-668fddb

Version: 0.0.0-pr-186-668fddb

You can use this Docker image with the preview package from this PR.

[ghostwriternr]

On a quick scan, this is looking good so far! The only gap is perhaps that we're not yet killing running commands (see packages/sandbox-container/src/session.ts) and cleaning out all related resources (labellers and pid files). Much of that logic should be present in this one file.