fix(cli): bound Tailscale Serve invocation and surface stdout in diagnostic - #10
Draft
andromarces wants to merge 1 commit into
Draft
fix(cli): bound Tailscale Serve invocation and surface stdout in diagnostic#10andromarces wants to merge 1 commit into
andromarces wants to merge 1 commit into
Conversation
…nostic `codor install` hangs indefinitely at "configuring Tailscale" when the tailnet has no HTTPS certificates enabled: `tailscale serve --bg` does not fail in that case, it blocks on an interactive browser-consent flow and prints the consent URL to stdout, which `execFileSync` captures into a buffer nobody ever sees, with no timeout to bound the wait. Bound the `serve --bg` invocation to 20s (only that call; `serve status` stays unbounded, and every other `defaultExec` caller is unaffected since the new options parameter is optional), and surface stdout alongside stderr in the thrown diagnostic so a bounded failure is actionable instead of a bare timeout.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
codor install, after choosing "On this computer and remotely", can hang indefinitely at "configuring Tailscale" with no prompt, no error, no timeout, and no exit — the only recovery is Ctrl-C.This happens when the tailnet does not have HTTPS certificates enabled. In that case
tailscale serve --bgdoes not fail; it enters Tailscale's interactive feature-enablement flow, printing a consent URL to stdout and blocking until a human completes it in a browser. Two things inconfigureTailscaleServe(setup.ts) turn that into a silent freeze:execFileSync, so the wait is unbounded.Fix
serve --bginvocation with a 20s timeout.serve status(the next call, which never blocks this way) stays unbounded, and every otherdefaultExeccaller is unaffected since the newoptionsparameter is optional — verified by adding it to the sharedexectype rather than special-casing one call site.stdoutalongsidestderrin the thrown diagnostic, so a bounded failure explains itself instead of surfacing as a bare timeout.Both changes are scoped to
configureTailscaleServeand the sharedexecsignature;defaultExec's behavior is unchanged for every other call site (plutil,launchctl,wslinfo,icacls,systemctl,loginctl,schtasks,where/which).Not in this PR
Preflighting the actual precondition (checking
CertDomainsviatailscale status --jsonbefore ever calling Serve, to turn the 20s wait into an immediate message) is a larger, separately-reviewable change that adds a dependency on a Tailscale JSON field. Proposing it as a follow-up once this lands.Testing
Result: same 5 pre-existing failures as the unmodified baseline at this commit (
runtime-install.spec.ts:116,setup-tailscale.spec.ts›resolveTailscale, two POSIX-path/JSON-escaping failures and a timeout inindex.spec.ts, one unrelatedartifact.spec.tsassertion) — all pnpm-11/Windows-path portability issues unrelated to this change. 177 passed (was 175), the 2 new tests insetup-tailscale.spec.tsboth pass, no new failures.pnpm exec vitest runis used directly rather thanpnpm --filter @codor/cli test/pnpm -r test, which currently cannot reach or complete on Windows for unrelated reasons at this commit.Not independently reproduced: the hang itself was not triggered by actually executing
tailscale serve --bg— doing so publishes an endpoint to a tailnet and issues a publicly-logged HTTPS certificate, which is an irreversible disclosure. The blocking-consent mechanism is documented Tailscale behavior, confirmed by a second reviewer againsttailscalev1.98.9 (ipn/ipnlocal/serve_legacy.go,enableFeatureInteractive). The preconditions codor checks (CertDomains: null,BackendState: Running,Health: [],serve --helpexits 0) were all verified on the affected machine (Windows 11, Tailscale 1.98.9). Only tested on Windows 11; the macOS/Linux paths through this function are unchanged but not independently re-verified here.Harn
Touches the
setup-resolves-and-capability-probes-tailscale-serveassumption (setup.ts, "Setup resolves Tailscale by path and app location and probes Serve support"). The assumption's substance is unchanged — Tailscale is still resolved and probed the same way; this only bounds and better-diagnoses the subsequent Serve call.