Add serve command: combined local server and client in one by vitorbaptista · Pull Request #101 · vitorbaptista/shellshare

vitorbaptista · 2026-02-06T18:56:19Z

Summary

Adds a new shellshare serve command that starts a local server and immediately connects to it, allowing users to share their terminal with a single command. This provides a simpler alternative to running separate server and client processes.

Key Changes

New serve subcommand (src/cli/serve.rs): Implements the combined server+client functionality
- Spawns a local Tokio-based server in the background
- Waits for server readiness via TCP connection polling
- Connects as a client to the local server with auto-generated credentials
- Supports two modes:
  - --stdin: Reads from stdin (prints sharing URL to stderr)
  - Default (PTY): Spawns an interactive shell (prints sharing URL to stdout)
- Handles graceful shutdown with Ctrl+C and room cleanup
CLI argument parsing (src/main.rs): Added Serve command variant with options:
- --host (default: 127.0.0.1): Bind address for the server
- --port (default: 5000): Port to listen on
- --stdin: Enable stdin mode instead of PTY mode
Module exports (src/cli/mod.rs): Made stream_stdin public to allow reuse by serve mode
Comprehensive E2E tests (e2e/test_cli_serve.py): 373 lines of test coverage including:
- Basic stdin mode functionality and URL printing
- Message streaming and ordering
- Late joiner history retrieval
- Terminal size event handling
- Clean exit and room cleanup
- PTY/script mode operation

Implementation Details

Uses a fixed room name ("terminal") since only one room is needed per serve session
Generates random numeric passwords for internal client-server authentication
Maps 0.0.0.0 to 127.0.0.1 for connection attempts (platform compatibility)
Displays localhost in user-facing URLs for better UX
Waits up to 5 seconds for server startup with 100ms polling intervals
Properly cleans up room on exit via DELETE request
Shuts down Tokio runtime in background to avoid blocking

https://claude.ai/code/session_01HsviQhQjYdFSDSU7mpZmXw

This adds a new `serve` subcommand that starts a local server and broadcasts the terminal to it in a single command, eliminating the need to run server and client separately for local sharing. Options: --host/-H: Bind address (default: 127.0.0.1) --port/-p: Listen port (default: 5000) --stdin: Read from stdin instead of spawning a shell The server automatically shuts down when the session ends. https://claude.ai/code/session_01HsviQhQjYdFSDSU7mpZmXw

When running `shellshare serve`, GET / now serves the room viewer page instead of the home page. This means visitors can go to http://localhost:5000 and see the terminal directly. Implementation: - Add `ServerConfig` struct with optional `serve_room` field - In serve mode, inject `window.SHELLSHARE_ROOM` into room.html so the JS knows which room to join regardless of URL path - Display clean base URL (http://localhost:5000) in output - Regular `shellshare server` is unaffected (serve_room: None) https://claude.ai/code/session_01HsviQhQjYdFSDSU7mpZmXw

Copilot

Pull request overview

Adds a new shellshare serve CLI subcommand that runs a local server and immediately connects a local client to it, enabling “one command” terminal sharing (with / showing the terminal viewer directly).

Changes:

Introduces shellshare serve with --host, --port, and --stdin options to run a combined server+client flow.
Extends server routing to optionally serve a specific room viewer at / by injecting a room override into room.html.
Adds a comprehensive E2E test suite for serve mode (stdin + PTY behaviors).

Reviewed changes

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

Show a summary per file

File	Description
templates/room.html	Allows room selection via an injected `window.SHELLSHARE_ROOM` override.
src/server/mod.rs	Adds `ServerConfig` and serve-mode `/` routing that serves the room viewer directly.
src/main.rs	Wires the new `Serve` subcommand and updates server startup to use `ServerConfig`.
src/cli/serve.rs	Implements combined server+client execution, readiness polling, URL printing, and cleanup.
src/cli/mod.rs	Exposes `stream_stdin` for reuse by serve mode.
e2e/test_cli_serve.py	Adds E2E coverage for serve mode behavior and lifecycle.

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

Copilot · 2026-02-06T20:25:10Z

+            // Inject the room override before the main script
+            let override_script =
+                format!("<script>window.SHELLSHARE_ROOM=\"/r/{room}\";</script>");
+            let html = html.replacen("<script>", &format!("{override_script}\n  <script>"), 1);


serve_room_index_handler injects room directly into a JS string (window.SHELLSHARE_ROOM="/r/{room}") without escaping. If ServerConfig.serve_room ever comes from user input (or contains quotes/</script>), this can become an XSS vector. Consider normalizing the room name (e.g., stripping leading /r/) and escaping via JSON string encoding before embedding into HTML.

Copilot · 2026-02-06T20:25:10Z

+            // Inject the room override before the main script
+            let override_script =
+                format!("<script>window.SHELLSHARE_ROOM=\"/r/{room}\";</script>");
+            let html = html.replacen("<script>", &format!("{override_script}\n  <script>"), 1);
+


Room injection currently relies on html.replacen("<script>", ...) to find the first inline <script> tag. This is brittle (breaks if the template changes to <script type=...> or rearranges scripts) and silently serves the wrong page without the override. Consider adding an explicit placeholder in room.html for the override (or using a minimal templating step) instead of string-search replacement.

Suggested change

// Inject the room override before the main script

let override_script =

format!("<script>window.SHELLSHARE_ROOM=\"/r/{room}\";</script>");

let html = html.replacen("<script>", &format!("{override_script}\n <script>"), 1);

// Inject the room override script using an explicit placeholder if present,

// otherwise fall back to inserting before the first <script> tag, then </body>,

// and finally at the end of the document.

let override_script =

format!("<script>window.SHELLSHARE_ROOM=\"/r/{room}\";</script>");

let html = {

const PLACEHOLDER: &str = "{{ROOM_OVERRIDE_SCRIPT}}";

if html.contains(PLACEHOLDER) {

// Preferred: template provides an explicit placeholder.

html.replacen(PLACEHOLDER, &override_script, 1)

} else if let Some(script_pos) = html.find("<script") {

// Fallback: insert before the first <script...> tag (handles attributes).

let mut result =

String::with_capacity(html.len() + override_script.len() + 1);

result.push_str(&html[..script_pos]);

result.push_str(&override_script);

result.push('\n');

result.push_str(&html[script_pos..]);

result

} else if let Some(body_pos) = html.rfind("</body>") {

// Fallback: insert before closing </body> if there are no script tags.

let mut result =

String::with_capacity(html.len() + override_script.len() + 1);

result.push_str(&html[..body_pos]);

result.push_str(&override_script);

result.push('\n');

result.push_str(&html[body_pos..]);

result

} else {

// Last resort: append at the end.

let mut result =

String::with_capacity(html.len() + override_script.len() + 1);

result.push_str(&html);

result.push('\n');

result.push_str(&override_script);

result

}

};

Copilot · 2026-02-06T20:25:11Z

+    let server_config = crate::server::ServerConfig {
+        host: args.host.clone(),
+        port: args.port,
+        cleanup_interval_secs: 3600,
+        room_ttl_secs: 21600,
+        serve_room: Some(ROOM_NAME.to_string()),
+    };


cleanup_interval_secs / room_ttl_secs are hard-coded (3600/21600), duplicating the server subcommand defaults. This can drift if defaults change. Consider reusing shared constants (or wiring these through CLI args) so behavior stays consistent across commands.

Suggested change

let server_config = crate::server::ServerConfig {

host: args.host.clone(),

port: args.port,

cleanup_interval_secs: 3600,

room_ttl_secs: 21600,

serve_room: Some(ROOM_NAME.to_string()),

};

let mut server_config = crate::server::ServerConfig::default();

server_config.host = args.host.clone();

server_config.port = args.port;

server_config.serve_room = Some(ROOM_NAME.to_string());

Copilot · 2026-02-06T20:25:11Z

+    let display_host = display_host(&args.host);
+    let base_url = format!("http://{display_host}:{}", args.port);
+
+    // Create HTTP client pointing to the local server
+    let server_url = format!("http://{connect_host}:{}", args.port);


base_url / server_url are built with format!("http://{host}:{port}"), which produces invalid URLs for IPv6 addresses (needs brackets) and mis-handles hosts containing :. Consider validating --host to an IpAddr/SocketAddr, or using a URL builder that formats IPv6 hosts correctly.

Copilot · 2026-02-06T20:25:11Z

+            stop_serve(proc)
+
+        # After exit, a new serve on the same port should work
+        # (room was cleaned up, not locked by stale password)
+        port2 = get_free_port()


This test stops the process via stop_serve(proc) (which sends terminate() by default), so it may not exercise the clean-exit/DELETE cleanup path it claims to test. Also, the comment says “same port”, but the follow-up run uses port2. Consider either exiting cleanly and asserting behavior that depends on DELETE/room cleanup, or renaming/updating the test to match what it actually verifies.

Copilot · 2026-02-06T20:25:12Z

+    fn test_wait_for_server_fails_on_closed_port() {
+        // Port 1 is almost certainly not listening
+        let result = wait_for_server("127.0.0.1", 1);
+        assert!(result.is_err());
+    }


test_wait_for_server_fails_on_closed_port assumes port 1 is not listening, which can be flaky on some environments. Consider selecting a guaranteed-closed port by binding an ephemeral port, reading it, closing it, and then asserting connect fails (or otherwise ensuring the chosen port is unused).

Copilot · 2026-02-06T20:25:12Z

+    // Shut down the runtime without blocking
+    runtime.shutdown_background();
+


runtime.shutdown_background() is only called on the normal success path. If any ? early-returns after the server task is spawned, dropping the Tokio runtime can block while shutting down the still-running server task. Consider using a Drop/scope guard so shutdown happens on all exit paths (and optionally do best-effort room cleanup on errors).

claude added 2 commits February 6, 2026 16:00

vitorbaptista requested a review from Copilot February 6, 2026 20:20

Copilot started reviewing on behalf of vitorbaptista February 6, 2026 20:20 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add serve command: combined local server and client in one#101

Add serve command: combined local server and client in one#101
vitorbaptista wants to merge 2 commits intomasterfrom
claude/add-shellshare-serve-command-q963A

vitorbaptista commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Copilot AI Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-            // Inject the room override before the main script
-            let override_script =
-                format!("<script>window.SHELLSHARE_ROOM=\"/r/{room}\";</script>");
-            let html = html.replacen("<script>", &format!("{override_script}\n  <script>"), 1);
+            // Inject the room override script using an explicit placeholder if present,
+            // otherwise fall back to inserting before the first <script> tag, then </body>,
+            // and finally at the end of the document.
+            let override_script =
+                format!("<script>window.SHELLSHARE_ROOM=\"/r/{room}\";</script>");
+            let html = {
+                const PLACEHOLDER: &str = "{{ROOM_OVERRIDE_SCRIPT}}";
+                if html.contains(PLACEHOLDER) {
+                    // Preferred: template provides an explicit placeholder.
+                    html.replacen(PLACEHOLDER, &override_script, 1)
+                } else if let Some(script_pos) = html.find("<script") {
+                    // Fallback: insert before the first <script...> tag (handles attributes).
+                    let mut result =
+                        String::with_capacity(html.len() + override_script.len() + 1);
+                    result.push_str(&html[..script_pos]);
+                    result.push_str(&override_script);
+                    result.push('\n');
+                    result.push_str(&html[script_pos..]);
+                    result
+                } else if let Some(body_pos) = html.rfind("</body>") {
+                    // Fallback: insert before closing </body> if there are no script tags.
+                    let mut result =
+                        String::with_capacity(html.len() + override_script.len() + 1);
+                    result.push_str(&html[..body_pos]);
+                    result.push_str(&override_script);
+                    result.push('\n');
+                    result.push_str(&html[body_pos..]);
+                    result
+                } else {
+                    // Last resort: append at the end.
+                    let mut result =
+                        String::with_capacity(html.len() + override_script.len() + 1);
+                    result.push_str(&html);
+                    result.push('\n');
+                    result.push_str(&override_script);
+                    result
+                }
+            };

		// Shut down the runtime without blocking
		runtime.shutdown_background();

Conversation

vitorbaptista commented Feb 6, 2026

Summary

Key Changes

Implementation Details

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants