mirror of
https://github.com/stablyai/orca.git
synced 2026-10-03 08:02:12 +00:00
* fix(runtime): bound the remote-runtime connect against an unreachable host A host that is powered off or firewalled black-holes the TCP SYN, so the remote-runtime WebSocket neither opens nor errors. The Node-side transports set no connect bound, leaving the caller's whole-request timeout as the only one: every `orca <cmd> --environment <unreachable>` sat silent for 60s before failing with a generic `runtime_timeout`. Measured on an unreachable paired host (win-lowspec, SYNs dropped): terminal list / worktree list / repo list / status each took 60.19-60.26s; the same command against a reachable host answered in 0.24s. So this was the shared transport, not one command. Pass `handshakeTimeout` at the three shared remote-runtime WebSocket construction sites, which `ws` applies across TCP connect and the HTTP upgrade. The value matches the bound the browser transport already used. The failure keeps code `remote_runtime_unavailable` so the existing transport-loss classification in terminal-process-inspection still applies, and the message names the endpoint and stops at "unverifiable" — per docs/reference/ssh-execution-boundary.md, loss of contact is never evidence that the host's work stopped. * fix(relay): bound the control socket's connect phase at the transport The relay control socket was constructed with no `handshakeTimeout`, the same gap fixed for the remote-runtime transports. It was not a live defect: the class-level `connectDeadlineMs` (15s) also covers a stalled connect, and that deadline does fire — its `unref()` is safe because the pending TCP connect is itself a ref'd libuv handle that holds the event loop open. Measured in a bare Node process: unref'd timer with an empty loop never fires (exit at 0ms), but the same timer alongside a black-holed connect fired at 2003ms. It was a defect waiting on a refactor. The two bounds cover different phases, and the class deadline covers the connect phase only incidentally. DO NOT REMOVE EITHER BOUND AS REDUNDANT. They are not. Proven by mutation: - Remove the transport bound -> a stalled *connect* falls through to the class deadline, rejecting with `relay_control_connect_timeout` after the full deadline instead of the transport error. - Remove the class deadline -> a stall during the *proving* phase (socket open, host proof never answered) is unbounded; the incumbent test hangs 30s. `handshakeTimeout` cannot see that phase at all. Reuses `remoteRuntimeConnectOptions` rather than forking a second helper, and moves the construction into `relay-control-socket-factory.ts` so a caller that needs a relay control socket gets the bound instead of re-deriving an unbounded one. `handshakeTimeoutMs` is settable apart from `connectDeadlineMs` so a test can stall the connect alone and assert which bound produced the rejection — error identity, not elapsed time. The connect-bound ratchet now covers the relay site and asserts the site still resolves, so an allowlist that silently stopped matching cannot pass vacuously. * fix(lint): carry SAFETY rationales for the connect-bound casts main tightened typescript/consistent-type-assertions to assertionStyle: never, which the rebase brings onto these added lines. Dropping the generic default is not typeable, so each cast keeps its own rationale. * fix(runtime): keep the bounded connect failure inside both message gates The connect bound's new wording dropped out of the two gates that classify remote-transport failures by message text, and those gates are the only ones that run on the path the bound made reachable. `subscribeRemoteRuntimeTransport` reports a connect failure by *rejecting* the subscribe promise, and that rejection crosses `ipcMain.handle`, which keeps only the message. The renderer then classifies it with `RECOVERABLE_MESSAGE_FRAGMENTS`. `Could not reach the remote Orca runtime at …` matched no fragment, so it read as fatal: `recovery.cancel()` and a red banner instead of a retry. Before the bound existed this case reached the 15s subscription-start timer, whose message did match a fragment, so introducing a 12s bound turned an auto-recovering pane into a dead-ended one — the #12650 shape. The same wording also fell outside `REMOTE_RUNTIME_UNREACHABLE_RE`, so the Tailscale remedy was dropped for precisely the unreachable-host failure it exists for. Keep the canonical phrase both gates already recognise rather than teaching each gate a second synonym for one condition, and pin it: the phrase is now a named constant, the corpus in `remote-runtime-transport-error-agreement.test.ts` grows the coded, hinted and code-stripped producers derived from the real helper, and a new subscribe-path test proves the connect bound (not the start timer) is what fires and that its message still classifies as recoverable once the code is gone. Verdict wording is unchanged: `unverifiable`, never a synonym for exited. Also states the bound in seconds, corrects the module comment (`handshakeTimeout` is a socket inactivity timer, so a slow-but-answering host is not cut off), and splits the subscription contract types out to stay under `max-lines`. * fix(relay): drop the duplicate connect bound on the control socket The claim that `connectDeadlineMs` cannot see a black-holed connect is false. `RelayControlClient.connect()` constructs the socket and arms `connectTimer` in the same synchronous call — `new WebSocket()` never blocks — and `expireConnect` fires from `opening` as well as `proving`. The class deadline was already a strict superset of a transport `handshakeTimeout` on that socket. It was also inert. Production passes neither option, so the transport bound was derived from `connectDeadlineMs` and both timers were 15_000, armed in the same tick; the ws timer is an inactivity timer armed on the later `socket` event, so it could not win. Its only reachable effect was changing which string a stalled relay connect rejects with, and it narrowed an existing test's 20ms deadline into a handshake bound it could race. So this removes the factory, the test-only `handshakeTimeoutMs` option and the source-grep test whose premise was wrong, and replaces them with a test that holds the real ground: a connect whose upgrade is never answered expires on the class deadline. Moving the timer arm after `open`, or narrowing `expireConnect` to `proving`, both turn it red — which is what a future reader needs before concluding the phase is uncovered and adding a second bound again. No behaviour change for a reachable relay, and none for the verdict: a stalled connect still rejects and still reaches `unverifiable`, never `exited`. * fix(runtime): stop the endpoint in the failure message from undoing the fix Putting the endpoint into the message created three problems the message itself caused. The Tailscale hint is idempotent by testing whether "tailscale" already appears anywhere in the message. That held while the message was fixed copy. Now a host called `tailscale-box` puts the word there itself, and the hint — the only actionable remedy on an unreachable host — is suppressed for it. Key the guard on the two hints instead of the word. The endpoint comes from a pasted pairing code, which is only length-capped; `normalizePairingUrl` rejects userinfo but nothing re-validates a stored offer. Render scheme, host and port only, so a pasted `wss://user:secret@host` cannot reach a surface the user reads. And drop the elapsed time from the wording. `handshakeTimeout` is a socket inactivity timer, so a `wss://` host that completes TCP and then goes silent re-arms it once and fails at about twice the bound; measured at 2008ms against a 1000ms bound. "within 12s" would have been wrong there, and the endpoint is the actionable part regardless. Also refuse a non-positive or non-finite bound: `ws` and `net` both gate on a truthy timeout, so `0` left the connect completely unbounded while still satisfying the connect-bound ratchet. * fix(runtime): keep the endpoint from smuggling a verdict into the message `isRemoteTerminalGoneMessage` in the pty transport substring-matches `terminal_gone` / `terminal_exited` / `no_connected_pty`, and it runs before the recoverable-connection gate: a match retires the pane's terminal id and cancels recovery. WHATWG URL accepts `_` in a special-scheme host, so once the failure message carried the endpoint, `ws://terminal_gone.example:6768` turned loss of contact into a terminal-gone verdict — the one conclusion `docs/reference/ssh-execution-boundary.md` forbids. Render the host only when it matches a hostname or IP-literal grammar that cannot carry such a token, and fall back to naming no endpoint at all. A well-formed host, including a bracketed IPv6 literal, is still shown. * docs(runtime): say why this connect bound is not the relay's removed duplicate
85 lines
3.9 KiB
TypeScript
85 lines
3.9 KiB
TypeScript
/**
|
||
* Appends an actionable Tailscale recommendation to remote-runtime connection
|
||
* failures, mirroring `withMacTailscaleDnsHint`. Lives in `shared` as a pure,
|
||
* dependency-free function so both the main process (desktop transport) and the
|
||
* renderer (web client) can route their user-facing errors through it without
|
||
* leaking presentation copy into the shared error constructors (which the CLI,
|
||
* logs, and mobile typecheck also consume).
|
||
*/
|
||
|
||
const TAILSCALE_DOWNLOAD_URL = 'https://tailscale.com/download'
|
||
|
||
// Why: only the "runtime is unreachable" family of failures has a Tailscale
|
||
// remedy; auth/protocol errors pass through untouched.
|
||
const REMOTE_RUNTIME_UNREACHABLE_RE =
|
||
/could not connect to the remote orca runtime|remote orca runtime closed the connection|timed out (?:waiting for|while connecting to) the remote orca runtime/i
|
||
|
||
const TAILSCALE_MAGIC_DNS_SUFFIX_RE = /(?:^|\.)ts\.net$/i
|
||
// Why: gate the CGNAT check on a full IPv4 literal — the range regex alone also
|
||
// matches DNS names like `100.64.0.1.example.com`, which aren't Tailscale IPs.
|
||
const IPV4_LITERAL_RE =
|
||
/^(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)(?:\.(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)){3}$/
|
||
// Tailscale assigns node IPs from the 100.64.0.0/10 CGNAT range (second octet 64–127).
|
||
const TAILSCALE_CGNAT_RE = /^100\.(?:6[4-9]|[7-9]\d|1[01]\d|12[0-7])\./
|
||
// Tailscale also assigns each node an IPv6 address from the fd7a:115c:a1e0::/48 ULA
|
||
// block, and pairing endpoints can carry an IPv6 literal (see resolvePairingEndpoint).
|
||
const TAILSCALE_IPV6_RE = /^fd7a:115c:a1e0:/i
|
||
|
||
function extractHost(endpoint: string): string | null {
|
||
let host: string | null
|
||
try {
|
||
host = new URL(endpoint).hostname || null
|
||
} catch {
|
||
// Why: a bare host (no scheme) isn't a valid URL; strip any scheme and take
|
||
// the authority up to the first port/path/query delimiter.
|
||
host = endpoint.replace(/^[a-z]+:\/\//i, '').split(/[/:?#]/, 1)[0] || null
|
||
}
|
||
if (!host) {
|
||
return null
|
||
}
|
||
// Why: WHATWG URL keeps IPv6 literals bracketed (`[fd7a:…]`) and FQDNs can carry
|
||
// a trailing dot; normalize both so the host checks below see a bare address/name.
|
||
return host.replace(/^\[|\]$/g, '').replace(/\.$/, '') || null
|
||
}
|
||
|
||
export function isTailscaleEndpoint(endpoint: string | null | undefined): boolean {
|
||
if (!endpoint) {
|
||
return false
|
||
}
|
||
const host = extractHost(endpoint)
|
||
if (!host) {
|
||
return false
|
||
}
|
||
return (
|
||
TAILSCALE_MAGIC_DNS_SUFFIX_RE.test(host) ||
|
||
(IPV4_LITERAL_RE.test(host) && TAILSCALE_CGNAT_RE.test(host)) ||
|
||
TAILSCALE_IPV6_RE.test(host)
|
||
)
|
||
}
|
||
|
||
/**
|
||
* Why: a server already reached over Tailscale fails for tailnet-specific reasons, so "use
|
||
* Tailscale" would be useless — point at the real causes. Already-paired devices keep their
|
||
* saved token across server restarts, so re-pairing only matters when adding a new device.
|
||
*/
|
||
const TAILNET_ENDPOINT_HINT =
|
||
"The server may be offline on your tailnet, or its Tailscale Funnel reverted to tailnet-only. Confirm it's reachable; re-pair only when adding a new device, since already-paired devices reconnect with their saved token."
|
||
|
||
const OTHER_NETWORK_HINT = `If the server is on another network, connect both devices to Tailscale and pair using its Tailscale address (100.x or a *.ts.net name). See ${TAILSCALE_DOWNLOAD_URL}.`
|
||
|
||
export function withRemoteRuntimeTailscaleHint(
|
||
message: string,
|
||
endpoint: string | null | undefined
|
||
): string {
|
||
if (!REMOTE_RUNTIME_UNREACHABLE_RE.test(message)) {
|
||
return message
|
||
}
|
||
// Why: keep the hint idempotent so a message routed through this helper twice (e.g. a
|
||
// re-wrapped error response) isn't suffixed with duplicate guidance. Keyed on the hints
|
||
// themselves, not on the word — messages now carry an endpoint whose host can contain it.
|
||
if (message.endsWith(TAILNET_ENDPOINT_HINT) || message.endsWith(OTHER_NETWORK_HINT)) {
|
||
return message
|
||
}
|
||
return `${message} ${isTailscaleEndpoint(endpoint) ? TAILNET_ENDPOINT_HINT : OTHER_NETWORK_HINT}`
|
||
}
|