Add tests and infrastructure for PTY terminal#375
Merged
ghostwriternr merged 2 commits intopty-supportfrom Feb 6, 2026
Merged
Conversation
Unit tests for ring buffer, PTY spawning, and WebSocket handler. E2E tests for PTY workflow and browser terminal addon integration. Updates CI workflows and documentation for new test patterns.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
* Refactor SandboxAddon to use connect(target) * Add collaborative terminal example Demonstrates the SandboxAddon connect() API with real-time room switching, presence tracking, and session isolation across multiple terminal rooms. Co-authored-by: Naresh <naresh@cloudflare.com> * Clear bash startup warning in container PTY Bash emits 'cannot set terminal process group' warnings in containers where the shell isn't a session leader. This clutters the first lines of terminal output for users. * Add build/** to turbo output cache React Router outputs to build/ instead of dist/, causing turbo to warn about missing outputs for the collaborative-terminal example. * Update collaborative-terminal README for new API The previous README documented the old PTY API (sandbox.pty.create() with JSON WebSocket protocol) that no longer exists. Updated to reflect the current implementation using session.terminal(request) with direct WebSocket passthrough and SandboxAddon for terminal integration. --------- Co-authored-by: katereznykova <kreznykova@cloudflare.com>
ghostwriternr
added a commit
that referenced
this pull request
Feb 6, 2026
* Add PTY types to shared package
* Add PtyManager for container PTY lifecycle
* Add PTY handler and route registration
* Add PTY message handling to WebSocket adapter
* Add PTY methods to transport interface
* Add PtyClient for SDK PTY operations
* Add pty namespace to Sandbox class
* Add E2E tests for PTY workflow
* Add changeset for PTY support
* Skip PTY tests when PTY allocation fails
* Fix pty manager tests
* fix any types, logger
* fix silent logging
* fix pty tests for resizing
* update claude review yml
* revert review change
* update http tests
* more test updates
* remove the plugin for review
* Add error handling to PTY callbacks and terminal operations
* Improve
PTY error handling based on code review
* add structured exit codes
* change fire and forget strategy
* add more e2e tests
* more fixes and tests
* fix ws
* update error propagation
* update resizing tests
* add collab terminal example
* Potential fix for code scanning alert no. 40: Insecure randomness
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Potential fix for code scanning alert no. 41: Insecure randomness
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Update dependency in examples
* Add PTY listeners cleanup
* minor nits
* update tests setup
* Add logging for PTY listener registration errors and improve error handling
* Enhance error handling and logging in WebSocketTransport and PtyHandler; add tests for PTY listener registration and cleanup behavior
* Add connection-specific PTY listener cleanup on WebSocket close
* Remove outdated comment regarding connection cleanup functions in WebSocketAdapter
* Fix error handling in PTY management by updating kill method to return success status and error messages
* implement signal handling for Ctrl+C, Ctrl+Z, and Ctrl+\ in the PTY manager
* Potential fix for code scanning alert no. 43: Insecure randomness
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Changes based on review comments
* Update dependencies and improve PTY handling in collaborative terminal example
* Update PTY workflow tests to expect correct HTTP status codes for error responses
* extractPtyId method to retrieve PTY IDs from responses, and update handleRegularResponse to return parsed body for further processing
* Update PTY workflow tests to expect 'message' field in error responses instead of 'error'
* Fixed handlers for tests
* Use PR-specific Docker build cache scope to avoid cross-PR cache pollution
* Add debug logging to router for route registration and matching
* Add INFO-level route logging to debug container caching issues
* Add retry logic for WebSocket server readiness in e2e tests
The WebSocket connect tests were flaky because they didn't wait for
the echo server to be ready after /api/init. Added a helper function
that retries WebSocket connection with backoff before running tests.
* Remove debug logging added during PTY route investigation
The 404 issues were caused by stale container instances, not route
registration problems. Reverting the debug logging changes:
- Remove INFO-level route logging from router
- Remove logRegisteredRoutes() method
- Revert PR-specific Docker cache scope (not needed)
* Fix sync-docs workflow to handle PR bodies with special characters
Use quoted heredoc and printf to safely handle PR description content
that may contain backticks, code blocks, or other shell-sensitive
characters. Pass PR body via environment variable to prevent shell
interpretation during prompt construction.
* Fix sync-docs workflow shell escaping for opencode run
Use environment variable to pass prompt to opencode run, avoiding
shell interpretation of special characters like parentheses, backticks,
and dollar signs that appear in PR descriptions with code examples.
The prompt is stored in OPENCODE_PROMPT env var which GitHub Actions
sets safely, then referenced with double quotes in the shell command.
* Fix lint errors and align env type signatures
The recent env var changes in 7da85c0 introduced Record<string,
string | undefined> but missed updating getInitialEnv return type
and getSessionInfo. Also aligns vite-plugin versions across examples.
* send heartbeat events to keep container alive
* Add PTY terminal passthrough for browser clients
Enables browser-based terminal UIs to connect to sandbox shells via
WebSocket. The terminal() method proxies connections to the container's
PTY endpoint with output buffering for replay on reconnect.
* Add tests and infrastructure for PTY terminal (#375)
* Add tests and infrastructure for PTY terminal
Unit tests for ring buffer, PTY spawning, and WebSocket handler.
E2E tests for PTY workflow and browser terminal addon integration.
Updates CI workflows and documentation for new test patterns.
* Add collaborative terminal example and refine xterm addon (#376)
* Refactor SandboxAddon to use connect(target)
* Add collaborative terminal example
Demonstrates the SandboxAddon connect() API with real-time
room switching, presence tracking, and session isolation
across multiple terminal rooms.
Co-authored-by: Naresh <naresh@cloudflare.com>
* Clear bash startup warning in container PTY
Bash emits 'cannot set terminal process group' warnings in
containers where the shell isn't a session leader. This clutters
the first lines of terminal output for users.
* Add build/** to turbo output cache
React Router outputs to build/ instead of dist/, causing turbo
to warn about missing outputs for the collaborative-terminal example.
* Update collaborative-terminal README for new API
The previous README documented the old PTY API (sandbox.pty.create() with
JSON WebSocket protocol) that no longer exists. Updated to reflect the
current implementation using session.terminal(request) with direct WebSocket
passthrough and SandboxAddon for terminal integration.
---------
Co-authored-by: katereznykova <kreznykova@cloudflare.com>
---------
Co-authored-by: katereznykova <kreznykova@cloudflare.com>
---------
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Naresh <naresh@cloudflare.com>
Co-authored-by: Steve James <sjames@cloudflare.com>
Co-authored-by: Naresh <ghostwriternr@gmail.com>
Contributor
There was a problem hiding this comment.
OpenCode Review
This PR adds comprehensive testing infrastructure for the PTY terminal feature, including unit tests, E2E tests, and Playwright browser tests. The testing coverage is thorough and well-organized.
Test Infrastructure - Well Designed
The addition of browser testing with Playwright is smart for validating the PTY/xterm.js integration. The test structure separates concerns appropriately:
- Unit tests for SDK and container components
- E2E tests for WebSocket PTY lifecycle
- Browser tests for actual xterm.js terminal interaction
The CI integration is properly configured with Playwright browser installation and environment variable handling.
Collaborative Terminal Example - Good Demo
The new collaborative terminal example effectively demonstrates the PTY feature in a real-world scenario. The architecture using separate Room and RoomRegistry Durable Objects for presence management while proxying terminal I/O directly to the sandbox is sound.
Documentation Updates - Clear
The updates to testing documentation clearly explain the new browser testing capability and provide appropriate command examples. The note about test:e2e:vitest for filtering tests is helpful.
Minor Note
The generateRandomNameSuffix() function correctly uses crypto.getRandomValues() for secure randomness. However, the randomColor() function still uses Math.random() which is fine for color selection but worth noting for consistency.
Overall, this is a solid addition that properly tests the PTY feature across all layers and provides a compelling real-world example.
Contributor
🐳 Docker Images PublishedDefault: FROM cloudflare/sandbox:0.0.0-pr-375-5a91a19With Python: FROM cloudflare/sandbox:0.0.0-pr-375-5a91a19-pythonWith OpenCode: FROM cloudflare/sandbox:0.0.0-pr-375-5a91a19-opencodeVersion: Use the 📦 Standalone BinaryFor arbitrary Dockerfiles: COPY --from=cloudflare/sandbox:0.0.0-pr-375-5a91a19 /container-server/sandbox /sandbox
ENTRYPOINT ["/sandbox"]Download via GitHub CLI: gh run download 21754447599 -n sandbox-binaryExtract from Docker: docker run --rm cloudflare/sandbox:0.0.0-pr-375-5a91a19 cat /container-server/sandbox > sandbox && chmod +x sandbox |
scuffi
pushed a commit
that referenced
this pull request
Mar 3, 2026
* Add PTY types to shared package
* Add PtyManager for container PTY lifecycle
* Add PTY handler and route registration
* Add PTY message handling to WebSocket adapter
* Add PTY methods to transport interface
* Add PtyClient for SDK PTY operations
* Add pty namespace to Sandbox class
* Add E2E tests for PTY workflow
* Add changeset for PTY support
* Skip PTY tests when PTY allocation fails
* Fix pty manager tests
* fix any types, logger
* fix silent logging
* fix pty tests for resizing
* update claude review yml
* revert review change
* update http tests
* more test updates
* remove the plugin for review
* Add error handling to PTY callbacks and terminal operations
* Improve
PTY error handling based on code review
* add structured exit codes
* change fire and forget strategy
* add more e2e tests
* more fixes and tests
* fix ws
* update error propagation
* update resizing tests
* add collab terminal example
* Potential fix for code scanning alert no. 40: Insecure randomness
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Potential fix for code scanning alert no. 41: Insecure randomness
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Update dependency in examples
* Add PTY listeners cleanup
* minor nits
* update tests setup
* Add logging for PTY listener registration errors and improve error handling
* Enhance error handling and logging in WebSocketTransport and PtyHandler; add tests for PTY listener registration and cleanup behavior
* Add connection-specific PTY listener cleanup on WebSocket close
* Remove outdated comment regarding connection cleanup functions in WebSocketAdapter
* Fix error handling in PTY management by updating kill method to return success status and error messages
* implement signal handling for Ctrl+C, Ctrl+Z, and Ctrl+\ in the PTY manager
* Potential fix for code scanning alert no. 43: Insecure randomness
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Changes based on review comments
* Update dependencies and improve PTY handling in collaborative terminal example
* Update PTY workflow tests to expect correct HTTP status codes for error responses
* extractPtyId method to retrieve PTY IDs from responses, and update handleRegularResponse to return parsed body for further processing
* Update PTY workflow tests to expect 'message' field in error responses instead of 'error'
* Fixed handlers for tests
* Use PR-specific Docker build cache scope to avoid cross-PR cache pollution
* Add debug logging to router for route registration and matching
* Add INFO-level route logging to debug container caching issues
* Add retry logic for WebSocket server readiness in e2e tests
The WebSocket connect tests were flaky because they didn't wait for
the echo server to be ready after /api/init. Added a helper function
that retries WebSocket connection with backoff before running tests.
* Remove debug logging added during PTY route investigation
The 404 issues were caused by stale container instances, not route
registration problems. Reverting the debug logging changes:
- Remove INFO-level route logging from router
- Remove logRegisteredRoutes() method
- Revert PR-specific Docker cache scope (not needed)
* Fix sync-docs workflow to handle PR bodies with special characters
Use quoted heredoc and printf to safely handle PR description content
that may contain backticks, code blocks, or other shell-sensitive
characters. Pass PR body via environment variable to prevent shell
interpretation during prompt construction.
* Fix sync-docs workflow shell escaping for opencode run
Use environment variable to pass prompt to opencode run, avoiding
shell interpretation of special characters like parentheses, backticks,
and dollar signs that appear in PR descriptions with code examples.
The prompt is stored in OPENCODE_PROMPT env var which GitHub Actions
sets safely, then referenced with double quotes in the shell command.
* Fix lint errors and align env type signatures
The recent env var changes in 7da85c0 introduced Record<string,
string | undefined> but missed updating getInitialEnv return type
and getSessionInfo. Also aligns vite-plugin versions across examples.
* send heartbeat events to keep container alive
* Add PTY terminal passthrough for browser clients
Enables browser-based terminal UIs to connect to sandbox shells via
WebSocket. The terminal() method proxies connections to the container's
PTY endpoint with output buffering for replay on reconnect.
* Add tests and infrastructure for PTY terminal (#375)
* Add tests and infrastructure for PTY terminal
Unit tests for ring buffer, PTY spawning, and WebSocket handler.
E2E tests for PTY workflow and browser terminal addon integration.
Updates CI workflows and documentation for new test patterns.
* Add collaborative terminal example and refine xterm addon (#376)
* Refactor SandboxAddon to use connect(target)
* Add collaborative terminal example
Demonstrates the SandboxAddon connect() API with real-time
room switching, presence tracking, and session isolation
across multiple terminal rooms.
Co-authored-by: Naresh <naresh@cloudflare.com>
* Clear bash startup warning in container PTY
Bash emits 'cannot set terminal process group' warnings in
containers where the shell isn't a session leader. This clutters
the first lines of terminal output for users.
* Add build/** to turbo output cache
React Router outputs to build/ instead of dist/, causing turbo
to warn about missing outputs for the collaborative-terminal example.
* Update collaborative-terminal README for new API
The previous README documented the old PTY API (sandbox.pty.create() with
JSON WebSocket protocol) that no longer exists. Updated to reflect the
current implementation using session.terminal(request) with direct WebSocket
passthrough and SandboxAddon for terminal integration.
---------
Co-authored-by: katereznykova <kreznykova@cloudflare.com>
---------
Co-authored-by: katereznykova <kreznykova@cloudflare.com>
---------
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Co-authored-by: Naresh <naresh@cloudflare.com>
Co-authored-by: Steve James <sjames@cloudflare.com>
Co-authored-by: Naresh <ghostwriternr@gmail.com>
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.
Summary
Unit tests, E2E tests, and Playwright browser tests for the PTY terminal feature introduced in #310.
What's included
PtyWebSocketHandler,Ptyclass, andByteRingBuffercoverage.getSandbox()terminal proxy wiring and session wrapper tests.SandboxAddonxterm.js integration with connection state transitions and reconnection.Stacks on #310.