fix(tailscale): preserve occupied Serve handlers - #8338
Conversation
|
@codex review |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
One finding: the widened ensureTailscaleServe error channel is collapsed into a generic wrapper at the pair translation boundary, so the new occupied-port failure surfaces the wrong caller-visible message.
Posted via Macroscope — Effect Service Conventions
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 858e5eed86
ℹ️ 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".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — The change materially alters shared Tailscale Serve setup, replacement, and cleanup across pairing, server shutdown, and development sharing, including new external-status parsing and ownership decisions. Its intent is protective, but the cross-cutting stateful behavior and persistent handler side effects merit human review. You can add or adjust custom eligibility rules. Learn more. |
|
The patch-identical LastCode port is open as lastobelus#102. Downstream head |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f6d04d5f4
ℹ️ 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".
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7b7bbaf. Configure here.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b7bbaf8ae
ℹ️ 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".
There was a problem hiding this comment.
Effect service conventions review: one finding on the new error translation boundary in apps/server/src/cli/pair.ts.
Posted via Macroscope — Effect Service Conventions
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Tailscale Serve mappings persist independently of their loopback backends. LastCode previously treated a failed HTTP probe as permission to replace a configured port, and development sharing could proxy IPv4 while Vite listened only on IPv6 localhost. This ports pingdotgg#8338 exactly into LastCode. It treats the live Serve configuration as authoritative, reuses or removes only a handler whose root proxy exactly matches the current LastCode lifecycle, refuses occupied foreign handlers even when their backend is unavailable, and makes shared development bind both Vite and its proxy to `127.0.0.1`. Ordinary non-shared development remains on `localhost`. Upstream source: pingdotgg#8338 Upstream head: `858e5eed8675b2025ec39118c7c9ae23edb7a4ad` Validation: - 6 focused files, 242 tests - shared, tailscale, web, scripts, and server typechecks - scoped lint, formatting, and diff checks - normal guarded LastCode quick CI, including 2,837 server tests Authored with GPT-5.6 Sol through the Codex harness.
1 participant
T3 currently treats an unreachable Serve backend as permission to replace its port, and dev sharing can proxy IPv4 to a Vite listener that resolved
localhostas IPv6. That can overwrite another application's persistent handler and can produce a shared URL that always returns 502.This change treats live Serve configuration as the ownership authority. T3 reuses or removes only an exact root handler for its loopback target, refuses any other occupied handler, and makes shared dev choose the same explicit IPv4 loopback for both Vite and Tailscale without pinning remote HMR to loopback.
Focused validation: 241 tests across shared dev proxy, Tailscale, dev sharing/runner, pairing, and server integration; shared, Tailscale, web, scripts, and server typechecks; scoped lint and formatting.
Implemented with GPT-5.6 Sol through the Codex harness.
Note
Medium Risk
Changes remote-access and Tailscale Serve lifecycle on machines that share ports with other apps; behavior is safer but pairing/dev-share can fail where they previously overwrote handlers.
Overview
Tailscale Serve no longer blindly configures or clears HTTPS ports.
ensureTailscaleServeanddisableTailscaleServeconsulttailscale serve status --jsonand only create, reuse, or remove a mapping when the port is free or the existing handler is a single root proxy to the expected loopback URL; foreign handlers, multi-route configs, Funnel, and mismatched backends yieldTailscaleServePortOccupiedError. Verified dev pairing can opt intoreplaceVerifiedHandlerto repoint an already-proven mapping to the Vite port.Dev sharing and pairing drop the old “clear then serve” flow:
shareDevServercallsensuredirectly,t3 pairmaps occupied errors to user-facing port errors, and server shutdown passeslocalPort/localHostintodisableso only T3-owned handlers are turned off.DevServeFailedErrorloses itsstagefield and messages no longer claim a prior mapping was cleared.Shared dev over Tailscale sets
T3CODE_WEB_BIND_HOSTto127.0.0.1(via newresolveWebDevServerHostindevProxy) so Vite listens on the same IPv4 address the Serve proxy targets, while HMR stays derived from the page origin unless desktop explicitly setsHOST. Docs describe the new port-ownership rules.Reviewed by Cursor Bugbot for commit 1c54d3f. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Preserve occupied Tailscale Serve handlers in
ensureTailscaleServeanddisableTailscaleServeensureTailscaleServenow probes Serve status before configuring and only maps a port when it is absent or already points to the exact expected target; otherwise it returnsTailscaleServePortOccupiedErrordisableTailscaleServerequireslocalPortand optionallocalHostand only removes a handler that still fronts the expected proxy target, leaving unrelated or funnel-enabled mappings intactshareDevServer/unshareDevServer) drops the pre-clear step and surfaces a friendly error when the port is occupied by another handlerdev-runnerand Vite config bind toSHARED_DEV_LOOPBACK_HOST(IPv4 loopback) viaT3CODE_WEB_BIND_HOSTwhen sharing is enabled, keeping HMR host implicitrunTailscaleCommandnow returns stdout on success andTailscaleCommandExitErrorcarriesstdoutLength; callers that ignored the return value are unaffected, but any caller relying on the old single-void signature ofdisableTailscaleServemust passlocalPortandlocalHostMacroscope summarized 1c54d3f.