close
Skip to content

fix(release): backport one-click Tailnet mobile access - #5980

Merged
bolichen97 merged 2 commits into
release/0.4.0from
codex/backport-tailnet-mobile-release
Aug 26, 2026
Merged

fix(release): backport one-click Tailnet mobile access#5980
bolichen97 merged 2 commits into
release/0.4.0from
codex/backport-tailnet-mobile-release

Conversation

@bolichen97

@bolichen97 bolichen97 commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Summary Backports #5961 to release/0.4.0 as the same two patches, in order: - feat(tailnet): make phone access one click - fix(gateway): harden phone access and restart safety ## What changes - Adds the Gateway one-click Tailscale mobile setup flow and QR display. - Detects Tailscale/MagicDNS/HTTPS-consent prerequisites and provides actionable states. - Applies Tailnet trust only across the formal Gateway restart boundary. - Coalesces concurrent restart requests and waits for response flush, process exit, port release, and ownership handoff before rebind/reexec. - Preserves Windows process-tree handling, macOS/Linux service-manager grace, and symlink/realpath safety. - Keeps audit work off the async request loop. ## Backport integrity Both cherry-picks applied without conflicts. Their stable patch IDs exactly match the source commits from #5961. ## Targeted validation - Backend Tailnet/governance/health/restart/venv tests: 187 passed - TailnetMobileCard tests: 40 passed The source PR is fully green, including Windows/macOS/Linux, Python 3.10/3.12, frontend, packaging, security, AI review, and coverage gates. ## Platform boundary The guided flow works on Windows, macOS, and Linux once Tailscale is installed, running, signed in, MagicDNS and tailnet HTTPS consent are enabled, the OS grants any required permissions, and the phone is permitted by the same tailnet ACL. Those external security prerequisites cannot be bypassed silently by Gateway.

Screenshots

Phone access before setup

Phone access after one click

@bolichen97
bolichen97 requested a review from a team as a code owner August 26, 2026 02:32
@bolichen97
bolichen97 requested review from iamwhatever and removed request for a team August 26, 2026 02:32
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 26, 2026
@bolichen97 bolichen97 changed the title [release/0.4.0] Backport one-click Tailnet mobile access fix(release): backport one-click Tailnet mobile access Aug 26, 2026
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Aug 26, 2026
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 03d6577e55cfd9a76ece0d151a4b8b71d82ed5bb and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 03d6577

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 03d6577e55cfd9a76ece0d151a4b8b71d82ed5bb: <one-sentence reason>

@bolichen97
bolichen97 merged commit eb286fc into release/0.4.0 Aug 26, 2026
23 of 27 checks passed
@bolichen97
bolichen97 deleted the codex/backport-tailnet-mobile-release branch August 26, 2026 02:35
@github-actions github-actions Bot removed the readiness: checking Automated validation is still running label Aug 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

UX-level review of 03d6577e55cfd9a76ece0d151a4b8b71d82ed5bb — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: CONCERNS

One click now silently restarts the whole gateway — "Set up & show QR" never discloses the interruption, and the old manual copy still narrates the steps it automates.

Watch

  • Undisclosed gateway restart behind "Set up & show QR". On trust_off, the setup mutation runs api.restartGateway() — the process replaces itself and close_all() drops every live session — but neither the button label nor trust_off_body ("This opens no port to the internet and does not change how you sign in.") mentions a restart; the removed "Restart now"/"Restarting — this page will reconnect." was the only disclosure. A user with an agent mid-task gets it interrupted by a button that promised a QR code. Once-per-machine × work-interruption impact → add one line near the button: "This restarts Kiro Crew briefly."
  • Manual instructions render during the automated flow. accept() writes each intermediate step into the query cache, so while "Setting up phone access…" is pending the card flips to restart_gateway_title/body — "Restart Kiro Crew once and the last step will be ready" — instructing the user to do the thing the click is already doing; a literal reader restarts concurrently. Every one-click run × confusion/duplicate action → reword restart_gateway_body (and its title) to the automated reality, e.g. "Kiro Crew is restarting to trust the new name."

Suggestions

  • setup_timeout "Re-check the setup, then try again." — name the actual control: "…press Re-check, then try again," matching the visible Re-check button.
  • The after.jpg/before.jpg helper line ("…no certificate setup is required.") doesn't match the shipped automatic_https string — re-capture so screenshots show the final copy.

[UX-REVIEWED] 03d6577

@github-actions

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of 03d6577e55cfd9a76ece0d151a4b8b71d82ed5bb — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound client-owned orchestration across the restart boundary, but the one-click button silently restarts the whole gateway, and the description claims restart mechanics the diff doesn't contain.

Watch

  • A full gateway restart is hidden behind "Set up & show QR." The old flow had an explicit "Restart now" button (removed: restart_action / restarting keys deleted); the new setup mutation calls api.restartGateway() as an intermediate step. An operator with an in-flight agent turn or unattended task clicks a QR button and their sessions are drained and the process re-exec'd with no warning in the button label or card body — trust_off_body says only "Allow its own private-network name". Say "restarts Kiro Crew" in the visible copy for the trust_off/restart_gateway states.
  • Description → diff mismatch on restart safety. The description claims the restart path "waits for … process exit, port release, and ownership handoff before rebind/reexec" and "preserves Windows process-tree handling, macOS/Linux service-manager grace, and symlink/realpath safety." The updates.py hunk contains only the coalescing latch and the pre-existing flush sleep; reexec_python_module is a plain in-process os.execv with no port-release or handoff wait anywhere in this diff. Either the backport dropped code the description promises, or the description is inflated — verify which before merging a release-branch backport on the strength of it.

Suggestions

  • waitForRestartedGateway swallows every fetch error identically; a persistent 401/403 after the successor binds (auth not surviving restart) spins silently to the 60s timeout — distinguishing auth failures from connection-refused would turn a dead-end timeout into an actionable error.

[DESIGN-REVIEWED] 03d6577

@github-actions

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 03d6577e55cfd9a76ece0d151a4b8b71d82ed5bb — this comment is updated in place on each push.

Review details

Both candidates are self-rated low confidence. Let me verify the key falsification points before concluding.

For Candidate 1: startup_trusted has no rendering consumer (grep confirms only the TS interface at client.ts:910 and test fixtures reference it — TailnetMobileCard.tsx never reads it). Moreover, when startup_host == "" and probe.name == "", _derive_step returns enable_magicdns at line 146 (guard if not probe.name) before any startup_host comparison — so the empty==empty case never even reaches a state where the flipped field would matter to the derived step. No observable wrong outcome (c) exists. Dropped.

For Candidate 2: _gateway_restart_in_progress is True only during an actively-executing restart, which ends in reexec_python_module replacing the process. For _restart_gateway to return False on the policy-update path, a concurrent restart must be in flight — and that in-flight restart execs the (already-pulled) new image anyway, so a restart does occur. The interleaving is rare and the fallback is not unsafe. (c) does not hold as an observable wrong outcome at the 80+ bar. Dropped.

No new grounded defects surfaced during falsification (the restart claim/release, the enable_https gating with the published is not True exception, and the startup_host != probe.name change are all correct and improve on prior behavior).

No findings.

[OPUS-REVIEWED] 03d6577

Verdict parsed from the review's SHA-scoped output markers for commit 03d6577e55cfd9a76ece0d151a4b8b71d82ed5bb.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 03d6577e55cfd9a76ece0d151a4b8b71d82ed5bb: <one-sentence reason>

@github-actions

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 03d6577e55cfd9a76ece0d151a4b8b71d82ed5bb — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: CONCERNS

A feat is being backported onto a soaking release branch, and its one-click button merge is a relabel with no named failing user.

What this change ships

Intent: let an operator get the dashboard onto a phone without walking three separate steps — an ADDITION carrying real fixes.

  1. Three buttons ("Allow this name", "Restart now", "Turn on phone access") collapse into one "Set up & show QR" — unjustified move
  2. Setup now continues on its own across the gateway restart and mints the QR — oversized
  3. New "Enable HTTPS certificates in Tailscale" prerequisite state — justified
  4. QR refused with https_not_enabled when CertDomains omits the host — justified
  5. restart_gateway now also fires when the tailnet name changed, not only when absent — justified
  6. Repeat restart requests coalesce instead of racing two successors — justified
  7. already_in_progress in the restart response — zero consumers
  8. _restart_gateway returns a bool — zero consumers
  9. Owner-only denial audits moved off the event loop — justified
  10. Two test files reformatted out of the black baseline; two review JPEGs added — rides along, undeclared

Watch

  • Branch carries __version__ = "0.4.0-rc.9"; CONTRIBUTING.md:186 scopes a cut branch to bug fixes and release.md:98 wants the soaked bytes stable. Items 1–2 are the feature half and can wait for main; items 3–6, 9 stand alone as fixes.
  • Item 1 replaces four catalog keys across 12 locales. The description names no operator who reached for the wrong button.
  • Restart coalescing ships two latches for one job: _gateway_restart_in_progress (updates.py:1176, claimed before the first await) already prevents the double drain, so the _gateway_restart_task latch adds only a skipped 0.25 s task.

Subtractions

  • Drop already_in_progress (updates.py:1927) — grepped repo-wide: 2 hits, the definition and test_update_channel_and_restart.py:516; website/src reads it nowhere.
  • Restore _restart_gateway -> None — all 3 call sites (updates.py:1240, 1414, 1936) discard the bool.
  • Drop waitForRestartedGateway + RESTART_WAIT_MS + setup_timeout and keep the restart_gateway step's own button: useDashboardHealthProbe already force-reconnects and the card's React Query poll re-renders the next step.

[FIRST-PRINCIPLES-REVIEWED] 03d6577

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