Enable TypeScript checking for test files #238
Merged
Conversation
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
Added tests/e2e/ to typecheck pipeline. Created test-worker/types.ts defining response types for test infrastructure endpoints. All tests now use proper SDK types from @repo/shared for SDK operations and test-worker types for test infrastructure. Removed non-null assertions with proper type guards. Found and fixed bugs where tests used wrong field names (contextId vs id, processId vs id) that were masked by inline types.
Build output was going to dist/src/ instead of dist/ due to rootDir: "." in tsconfig, breaking Docker container startup which expects dist/index.js. Split tsconfig into separate build and typecheck configs. Build emits src only to dist/, typecheck includes tests.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Claude Code Review
Looks good. The tsconfig split pattern is correct - tsconfig.json for IDE/typecheck (includes tests), tsconfig.build.json for builds (src only). This is a standard best practice.
Type safety improvements are solid:
- Mock loggers now properly implement the
Loggerinterface withchild()method - Error codes use
ErrorCodeenum instead of string literals - New types (
VersionResult,ProxyErrorResponse.port) properly defined - Remaining
anyusage in tests is appropriate for dynamic test data
The E2E test tsconfig correctly excludes test-worker/** to prevent type conflicts between test context and Worker runtime.
No issues found. Ready to merge.
Contributor
🐳 Docker Image PublishedFROM cloudflare/sandbox:0.0.0-pr-238-5b0b863Version: You can use this Docker image with the preview package from this PR. |
ghostwriternr
added a commit
that referenced
this pull request
Nov 24, 2025
* Update default formatter * Fix memory leaks in process management Memory leaks occurred from three sources: event listeners accumulated when log streaming was cancelled, completed processes remained in memory indefinitely, and Durable Object state retained invalid references after container restarts. The ProcessStore now persists completed processes to disk while keeping active processes in memory. ReadableStream cancel handlers properly remove listeners when log streaming stops. The Sandbox onStop lifecycle hook clears container-specific state to prevent stale references. * Add changeset * Fix async cleanup and listener lifecycle The onStop lifecycle hook now properly awaits storage deletions to ensure cleanup completes before shutdown. Processes are deleted from memory before persisting to disk to prevent race conditions during concurrent access. Stream listeners are cleaned up on all close paths, not just cancellation. The list method now scans disk to include completed processes, maintaining backward compatibility. * Use structured logging for disk write failures * Fix data loss when persisting completed processes Write to disk before deleting from memory to prevent data loss if write fails. Always delete from memory regardless of write success to prevent memory leaks. * Fix list() directory check that prevented disk scanning Bun.file().exists() returns false for directories, causing list() to skip disk scanning and miss completed processes. Remove the directory existence check and rely on error handling instead. * Add comprehensive unit tests for ProcessStore Test coverage for memory/disk hybrid storage, terminal state transitions, disk persistence, write failure handling, and list() filtering. * Add E2E tests for completed process retrieval and listing Verify completed processes can be retrieved via get() with exit codes and timestamps, and that list() includes both running and completed processes. * Remove dot-prefix from process storage directory Change from /tmp/.sandbox-internal to /tmp/sandbox-internal for better debuggability. Makes it easier to discover and inspect process files during development and troubleshooting. * Fix race condition in process status updates Event handler called store.update() fire-and-forget, causing API queries to return stale data before disk writes completed. Changed SessionManager to accept async callbacks and await them, ensuring store updates complete before processing next event. This provides strong consistency while maintaining the memory leak fix that moves completed processes to disk. * Update lockfile * Clear activeMounts on container stop Applies memory leak prevention from this PR to the activeMounts feature that was merged from main. When a container stops or restarts, mount points become invalid but the Map still held stale references. Clearing it maintains consistency with portTokens and defaultSession. * Enable TypeScript checking for test files (#238) * Enable TypeScript checking for test files * Enable TypeScript checking for e2e tests Added tests/e2e/ to typecheck pipeline. Created test-worker/types.ts defining response types for test infrastructure endpoints. All tests now use proper SDK types from @repo/shared for SDK operations and test-worker types for test infrastructure. Removed non-null assertions with proper type guards. Found and fixed bugs where tests used wrong field names (contextId vs id, processId vs id) that were masked by inline types. * Fix incorrect assertions * Use public Process interface correctly * Revert enum usage * Enrich interpreter error * Fix TypeScript build output paths Build output was going to dist/src/ instead of dist/ due to rootDir: "." in tsconfig, breaking Docker container startup which expects dist/index.js. Split tsconfig into separate build and typecheck configs. Build emits src only to dist/, typecheck includes tests. * Fix type errors * Update lockfile * Fix code context e2e response checks * Prevent just-completed process appearing twice
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Enables TypeScript typechecking for test files across all packages while maintaining correct build output paths.
Implementation
Separates build and typecheck configurations:
tsconfig.json- IDE and typechecking (src + tests)tsconfig.build.json- Build only (src → dist/)Applied to
@repo/sharedand@repo/sandbox-container.Test Plan