Skip to content

Add Tailscale share command#89

Open
brahimhamichan wants to merge 2 commits into
EvanBacon:mainfrom
brahimhamichan:bridge-tailscale-share
Open

Add Tailscale share command#89
brahimhamichan wants to merge 2 commits into
EvanBacon:mainfrom
brahimhamichan:bridge-tailscale-share

Conversation

@brahimhamichan

@brahimhamichan brahimhamichan commented Jun 14, 2026

Copy link
Copy Markdown

Summary

  • add serve-sim share [device...] to start the normal preview server and expose it through Tailscale Serve
  • keep private tailnet Serve as the default; require explicit --public for Funnel
  • clean up the Tailscale service on exit only when the current Serve config still targets this preview URL
  • document that the shared URL exposes simulator video and controls

Testing

  • bun run typecheck
  • bun run lint
  • bun run --filter serve-sim build

Summary by CodeRabbit

  • New Features

    • Added a serve-sim share CLI command to enable remote access to the preview server via Tailscale, supporting private tailnet Serve and public Funnel modes (including an optional --public flag).
  • Documentation

    • Updated the documentation with new CLI usage examples for serve-sim share and added security guidance about exposed video and browser controls.

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5ec4b538-b376-45d4-87c6-9257b7d6e88b

📥 Commits

Reviewing files that changed from the base of the PR and between 14a5e81 and cfb507d.

📒 Files selected for processing (2)
  • packages/serve-sim/README.md
  • packages/serve-sim/src/index.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/serve-sim/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/serve-sim/src/index.ts

📝 Walkthrough

Walkthrough

Adds a serve-sim share CLI subcommand that starts the preview server and exposes it remotely via Tailscale Serve (private tailnet) or Tailscale Funnel (--public). The serve() function is refactored to accept an onReady callback. Cleanup handlers remove the Serve configuration on process exit. README is updated accordingly.

Changes

Tailscale Share Command

Layer / File(s) Summary
serve() onReady callback refactor
packages/serve-sim/src/index.ts
Adds execFileSync import, a typed PreviewReadyInfo interface, and an optional onReady callback to serve(), which is invoked with { host, port, localUrl } immediately after the preview port binds.
Tailscale share() implementation and helpers
packages/serve-sim/src/index.ts
Adds helper functions for Tailscale status parsing, name resolution, Serve target matching, and exit-signal cleanup; implements share() to invoke tailscale serve/funnel in the onReady callback and print the share URL with a security warning.
CLI subcommand registration
packages/serve-sim/src/index.ts
Registers the serve-sim share [device...] subcommand with --public, --host, and --port options, wiring it to the share() function.
README documentation
packages/serve-sim/README.md
Adds serve-sim share [device...] to the command list, extends usage examples with --public, and inserts a "Remote access with Tailscale" section covering private tailnet Serve vs public Funnel with a security warning.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • EvanBacon/serve-sim#59: Modifies the same CLI subcommand dispatch layer in packages/serve-sim/src/index.ts, directly adjacent to where the new serve-sim share subcommand is registered.

Poem

A rabbit hops through the tailnet at night,
Sharing the sim with a Funnel of light.
--public flag waves, the preview is live,
(Don't expose your controls — that's survival advice!)
🐰📡 The burrow is open, but guard what you give.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add Tailscale share command' directly describes the main change: introducing a new serve-sim share command powered by Tailscale. It is concise, specific, and clearly summarizes the primary feature addition.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 14a5e814cc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/serve-sim/src/index.ts Outdated
Comment on lines +1891 to +1892
if (tailscaleServeTargets(target)) {
execFileSync("tailscale", ["serve", "clear", "https:443"], { stdio: "ignore" });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use the matching off command for Serve cleanup

When a share session exits, this cleanup never removes the background Tailscale Serve/Funnel entry because tailscale serve clear only accepts a Tailscale Service name (svc:...), while this code passes https:443 after creating a normal port share with tailscale serve|funnel --bg <target>. The error is swallowed, so the Funnel/Serve config persists and can unexpectedly expose whatever later binds to the same local port; Tailscale's documented disable path for these shares is rerunning the same serve/funnel mode/port with off, not clear <service>.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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 `@packages/serve-sim/README.md`:
- Line 127: The documentation for the serve-sim share command incorrectly shows
[device] in singular form, but the command actually accepts multiple devices as
variadic arguments. Update the notation in the README from `serve-sim share
[device]` to `serve-sim share [devices...]` to accurately reflect the CLI
contract and match the actual implementation that accepts multiple device
arguments.

In `@packages/serve-sim/src/index.ts`:
- Around line 1870-1873: The execFileSync call to "tailscale status --json" can
block indefinitely if the tailscale daemon is unresponsive, causing the process
to hang during cleanup operations. Add a timeout option (in milliseconds) to the
options object passed to execFileSync in this invocation. Set a reasonable
timeout value that allows the command to complete under normal circumstances
while preventing indefinite blocking. Apply the same timeout hardening to all
other tailscale subprocess calls in the file (mentioned at lines 1946, 1892, and
1909-1912) to ensure consistent protection against unresponsive daemon states.
- Around line 1942-1945: The target URL construction for Tailscale does not
properly handle IPv6 addresses. When shareHost is an IPv6 address (which will
contain colons), the current string interpolation in the target variable
assignment produces an invalid URL like http://::1:port instead of the RFC 3986
compliant http://[::1]:port. Modify the target URL construction to detect if
shareHost contains colons (indicating an IPv6 address) and wrap it in brackets
before inserting it into the URL. This ensures IPv6 literals are properly
formatted as per the standard.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d4ac17f9-4cd9-4315-aae8-b67236f6e591

📥 Commits

Reviewing files that changed from the base of the PR and between 20fe098 and 14a5e81.

📒 Files selected for processing (2)
  • packages/serve-sim/README.md
  • packages/serve-sim/src/index.ts

Comment thread packages/serve-sim/README.md Outdated
Comment thread packages/serve-sim/src/index.ts
Comment thread packages/serve-sim/src/index.ts
@brahimhamichan

Copy link
Copy Markdown
Author

Addressed the review feedback in cfb507d:

  • Switched cleanup to matching tailscale serve|funnel --bg <target> off instead of serve clear
  • Added bounded timeouts to new tailscale subprocess calls
  • Bracketed IPv6 literal hosts when building Tailscale target URLs
  • Updated README notation to serve-sim share [devices...]

Validation run:

  • bun run typecheck
  • bun run lint
  • bun run --filter serve-sim build

CodeRabbit is now green and there are no active unresolved review threads.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant