fix(web): Tailscale HTTPS stays on, and can be turned off, when this machine can't reach its own URL #15027 · pingdotgg/t3code

Ccopilot-pull-request-reviewerBotreviewed26d ago

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Approvability

Verdict: Approved at 17c9a99

Macroscope's review found this PR approvable — This is a narrowly scoped Connections settings bug fix that reconciles the Tailscale switch with the saved Serve state while preserving pairing health checks and existing disable behavior. It adds focused tests without changing backend contracts, defaults, deployment, or sensitive packages.

^{You can add or adjust custom eligibility rules. Learn more.}

MmacroscopeappBotapproved these changes26d ago

CcoderabbitaiBot26d ago

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.config.ts
Review profile: CHILL
Plan: Advanced
Run ID: 105e14e8-362b-4c68-89b5-b0c0243c54b7

📥 Commits

Reviewing files that changed from the base of the PR and between b8dabcc519f65f0718b51ebfdc9cc0d6263aa069 and 594eade81df0a8a7d366ed9fab7fa028d242fadd.

📒 Files selected for processing: 3

apps/web/src/components/settings/ConnectionsSettings.logic.test.ts
apps/web/src/components/settings/ConnectionsSettings.logic.ts
apps/web/src/components/settings/ConnectionsSettings.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The Tailscale HTTPS settings row now uses the saved Serve setting for its switch state. Endpoint availability determines reachability and whether the row displays the endpoint URL.

Changes: Tailscale HTTPS settings row

Layer / File(s)	Summary
Resolve the row state `apps/web/src/components/settings/ConnectionsSettings.logic.ts`, `apps/web/src/components/settings/ConnectionsSettings.logic.test.ts`	`resolveTailscaleHttpsRowState` derives switch visibility, checked state, URL, and reachability. Tests cover available, unavailable, and absent endpoints with Serve enabled or disabled.
Apply resolved state in settings `apps/web/src/components/settings/ConnectionsSettings.tsx`	The row uses the resolved state for its description, reachability warning, switch visibility, and checked value. Disabling Serve calls the handler without an endpoint argument.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: utkarshusername

Merge Risk: ⚪ Minimal · up to 594ea

The Tailscale HTTPS switch now follows the saved setting, so users can turn it off when the machine cannot reach its own URL. Pairing still requires a reachable endpoint. No merge-blocking risk was found.

Security architecture risk: 🔵 Low · up to 594ea

The change restores access to an existing exposure control without changing pairing eligibility or management permissions. No newly expanded access was identified. The remaining uncertainty concerns pre-existing shutdown and recovery behavior: a saved disabled setting is not confirmation that the external Serve mapping was removed.

Retained concerns No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The affected authority remains management of the local desktop backend's configured Serve port. The change does not add an attacker-supplied destination, broaden the configured network audience, or create cross-environment management authority.

Trust Boundaries and Controls

observed — Separating saved configuration from health does not promote an unavailable URL into a pairing destination. Default endpoint selection and endpoint copy options still reject unavailable endpoints before constructing credential-bearing pairing URLs. Disable also retains its desktop-bridge and environment-maintenance prerequisites.

Resilience and Maintainability Implications

inferred — Pre-existing persistence and cleanup are not one atomic revocation operation. Persistence can finish before detached shutdown, and cleanup is not guaranteed after failed setup or removal. The comparison does not establish that this PR worsens those failure paths, but interrupted shutdown and stale external mappings remain incompletely verified recovery cases.

Pre-merge checks: ^Passed 3 · ^Failed 1

^Failed Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Linked Issues check	Warning	The change satisfies the main `#14746` row behavior. `resolveTailscaleHttpsRowState` uses `tailscaleServeEnabled` for switch state, keeps the switch visible without an endpoint, reports probe health sep…	Update the Authorized clients visibility condition to include `desktopServerExposureState?.tailscaleServeEnabled === true`. Add a regression test for Network access off with saved Serve enabled.

^Passed Passed checks: 3 passed

Check name	Status	Explanation
Out of Scope Changes check	Passed	The whole-PR diff is limited to the Tailscale HTTPS row resolver, its Connections settings integration, and focused regression tests. The handler signature change supports the same disable flow. No un…
Title check	Passed	The title is concise, specific, and accurately summarizes the primary fix: preserving the saved Tailscale HTTPS state and allowing it to be disabled when the local URL is unreachable.
Description check	Passed	The description provides the problem, implemented change, scope and approval context, verification results, UI screenshots, known limitations, issue links, and agent details. It does not use the templ…

Full details: Out of Scope Changes check

Explanation

The whole-PR diff is limited to the Tailscale HTTPS row resolver, its Connections settings integration, and focused regression tests. The handler signature change supports the same disable flow. No unrelated desktop, server, contract, pairing, or Tailscale IP probing changes are present.

Full details: Description check

Explanation

The description provides the problem, implemented change, scope and approval context, verification results, UI screenshots, known limitations, issue links, and agent details. It does not use the template's exact Problem, Change, or Scope and approval headings, but the required information is present and sufficiently complete.

Full details: Linked Issues check

Explanation

The change satisfies the main #14746 row behavior. resolveTailscaleHttpsRowState uses tailscaleServeEnabled for switch state, keeps the switch visible without an endpoint, reports probe health separately, and leaves pairing selection health-gated. However, the Authorized clients visibility condition is unchanged. It still depends on Network access or an available Tailscale endpoint, not on the saved tailscaleServeEnabled flag. Authorized clients can therefore remain hidden while Serve is configured and the probe fails.

Resolution

Update the Authorized clients visibility condition to include desktopServerExposureState?.tailscaleServeEnabled === true. Add a regression test for Network access off with saved Serve enabled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

CcoderabbitaiBotapproved these changes21d ago

Ttiliakoos12d ago

Merged upstream/main (70d60e03a7) in b8dabcc519, so this is mergeable again. #20515 rewrote this component for the React Compiler, so the conflicts were the handleStartTailscaleServeDisable signature (now a plain function with no endpoint argument, since the switch can be turned off without one) and the memoized endpoint derivations (now plain expressions, with resolveTailscaleHttpsRowState among them). The row in renderTailscaleRow() merged by itself. The diff against main is still the three files (+104 −11).

Rerun on the merged branch: ConnectionsSettings.logic.test.ts 21 passed; lint, format and the web typecheck clean. The dev desktop app checks in the description are from the 2026-10-07 run and were not retaken.

#18758 edits the same row's no-endpoint text; the two changes do not overlap in behaviour, and whichever lands second has a one-line conflict in that ternary.

Ttiliakoos10d ago

Merged upstream/main (ab6be13d43) in 594eade81d, so this is mergeable again. #18758 landed on the same row and added missingTailscaleEndpointDescription for the no-endpoint text. The row keeps this fix's description while the switch is on and uses #18758's helper while it is off with no endpoint, so "Turn on Network access to detect Tailscale." still shows when detection was skipped. The other conflict was the two imports. The diff against main is still the three files (+104 −11).

Rerun on the merged branch: ConnectionsSettings.logic.test.ts 24 passed, and the two regression cases still fail with upstream's switch logic put back in the helper; lint, format and the web typecheck clean. The dev desktop app checks in the description are from the 2026-10-07 run and were not retaken.

JJulius Marminge10d ago

Taking over this one for batch ssh-relay-remote-05. I will preserve contributor history, merge current main, reduce the regression tests, and verify the saved-state switch and disable behavior with independent reviews. This remains unmerged.

Ttheo15h ago

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

Superseded by #26757, which folds in the Tailscale HTTPS switch following the saved setting. You are credited with a Co-authored-by trailer. Closing this so the two don't conflict.

fix(web): Tailscale HTTPS stays on, and can be turned off, when this machine can't reach its own URL#15027

Verification

Proof

Limits

Checks

Approvability