From 87668cd8647f9eed161a9f24ff526e3e330e877a Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Tue, 11 Aug 2026 15:11:37 -0400 Subject: [PATCH 1/2] Improve connection error guidance --- .../src/services/sync/syncAccountHelloAuth.ts | 28 +-- .../src/services/sync/syncHostService.test.ts | 4 +- .../src/services/sync/syncHostService.ts | 2 +- .../remoteRuntime/pairedRuntimeRoutes.test.ts | 16 +- .../remoteRuntime/pairedRuntimeRoutes.ts | 6 +- .../syncPairedMachineStore.test.ts | 84 ++++++++- .../remoteRuntime/syncPairedMachineStore.ts | 168 +++++++++++++----- .../webclient/sync/__tests__/sync.test.ts | 8 + .../src/renderer/webclient/sync/connection.ts | 10 +- apps/desktop/src/shared/types/sync.ts | 14 +- apps/ios/ADE/Services/SyncService.swift | 15 +- apps/ios/ADETests/ADETests.swift | 2 + docs/features/sync-and-multi-device/README.md | 34 ++-- .../sync-and-multi-device/ios-companion.md | 7 +- 14 files changed, 296 insertions(+), 102 deletions(-) diff --git a/apps/ade-cli/src/services/sync/syncAccountHelloAuth.ts b/apps/ade-cli/src/services/sync/syncAccountHelloAuth.ts index f12a5bcc0..090d47e56 100644 --- a/apps/ade-cli/src/services/sync/syncAccountHelloAuth.ts +++ b/apps/ade-cli/src/services/sync/syncAccountHelloAuth.ts @@ -30,14 +30,14 @@ import { evaluatePairedHelloDpop, syncDpopFailureMessage, type SyncDpopNonceCach export const SYNC_REPAIR_REQUIRED_MESSAGE = "This device is not paired with this machine, or its saved" + " pairing is no longer valid. Pair it again."; -export const SYNC_ACCOUNT_SESSION_CHANGED_MESSAGE = "The ADE account session on this machine changed" - + " while connecting. Try again."; +export const SYNC_ACCOUNT_SESSION_CHANGED_MESSAGE = "The ADE account session on the computer you're" + + " connecting to changed while connecting. Try again."; -export const SYNC_ACCOUNT_VERIFY_UNAVAILABLE_MESSAGE = "This machine cannot verify ADE accounts." - + " Update ADE on this computer, then try again."; +export const SYNC_ACCOUNT_VERIFY_UNAVAILABLE_MESSAGE = "The computer you're connecting to cannot verify" + + " ADE accounts. Update ADE there, then try again."; -export const SYNC_ACCOUNT_NOT_SIGNED_IN_MESSAGE = "This machine is not signed in to an ADE account." - + " Sign in on this computer, then try again."; +export const SYNC_ACCOUNT_NOT_SIGNED_IN_MESSAGE = "The computer you're connecting to is not signed in" + + " to an ADE account. Sign in on that computer, then try again."; export const SYNC_ACCOUNT_DEVICE_MISMATCH_MESSAGE = "The account identity in this connection did not" + " match the device that sent it."; @@ -48,11 +48,12 @@ export const SYNC_ACCOUNT_KEYLESS_RECORD_MESSAGE = "This device's saved pairing export const SYNC_ACCOUNT_OTHER_OWNER_MESSAGE = "This device is already paired to this machine under" + " a different ADE account."; -export const SYNC_ACCOUNT_PAIRING_WRITE_FAILED_MESSAGE = "This machine could not save the new pairing" - + " for this device. Try again."; +export const SYNC_ACCOUNT_PAIRING_WRITE_FAILED_MESSAGE = "The computer you're connecting to could not" + + " save the new pairing for this device. Try again."; -export const SYNC_ACCOUNT_VERIFY_FAILED_MESSAGE = "This machine could not verify your ADE account" - + " session. Sign out and back in on this device, then try again."; +export const SYNC_ACCOUNT_VERIFY_FAILED_MESSAGE = "The computer you're connecting to could not verify" + + " its ADE account session. Open ADE there and check that it is signed in to the same ADE account," + + " then try again."; export type SyncAccountHelloAuth = Extract; @@ -111,8 +112,9 @@ export type SyncAccountHelloAuthOptions = { /** "PIN" on the project host, "code" on the brain — same instruction, local wording. */ pairingCodeNoun: string; /** - * Code carried when this machine holds no account session at all. The brain - * answers `relay_account_required` because its only account route is Relay. + * Code carried when this machine holds no account session at all. The project + * host uses `account_not_signed_in`; the brain answers `relay_account_required` + * because its only account route is Relay. */ notSignedInCode: SyncHelloErrorPayload["code"]; }; @@ -298,6 +300,6 @@ export async function authenticateSyncAccountHello( ? (error as { code: string }).code : "verification_failed", }); - return reject(SYNC_ACCOUNT_VERIFY_FAILED_MESSAGE); + return reject(SYNC_ACCOUNT_VERIFY_FAILED_MESSAGE, "account_verification_failed"); } } diff --git a/apps/ade-cli/src/services/sync/syncHostService.test.ts b/apps/ade-cli/src/services/sync/syncHostService.test.ts index e616466ad..bb2736c65 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.test.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.test.ts @@ -4156,8 +4156,8 @@ describe("sync host account authentication", () => { "signed-out account hello_error", ); expect(signedOutRejected.payload).toMatchObject({ - code: "auth_failed", - message: expect.stringMatching(/not signed in.*Sign in on this computer/i), + code: "account_not_signed_in", + message: expect.stringMatching(/computer you're connecting to.*not signed in.*Sign in on that computer/i), }); const pinClient = await openAccountClient(port); diff --git a/apps/ade-cli/src/services/sync/syncHostService.ts b/apps/ade-cli/src/services/sync/syncHostService.ts index d687c86fd..6ddd0b8a2 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.ts @@ -7290,7 +7290,7 @@ export function createSyncHostService(args: SyncHostServiceArgs) { arbitrateConnectionAttempt(hello.peer.deviceId, peer, hello.peer), allowLegacyUpgrade: true, pairingCodeNoun: "PIN", - notSignedInCode: "auth_failed", + notSignedInCode: "account_not_signed_in", }); if (accountResult.kind === "stale") return true; if (accountResult.kind === "rejected") { diff --git a/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.test.ts b/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.test.ts index 4a5bd2fac..8b87e870b 100644 --- a/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.test.ts +++ b/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.test.ts @@ -173,16 +173,28 @@ describe("paired runtime endpoint routes", () => { // account session repaired. Both used to arrive as `auth_failed`. expect(classifyPairedRuntimeFailure( new PairedRuntimeHelloRejectedError( - "This machine cannot verify ADE accounts. Update ADE on this computer, then try again.", + "The computer you're connecting to cannot verify ADE accounts. Update ADE there, then try again.", "host_update_required", ), )).toBe("protocol"); expect(classifyPairedRuntimeFailure( new PairedRuntimeHelloRejectedError( - "The ADE account session on this machine changed while connecting. Try again.", + "The ADE account session on the computer you're connecting to changed while connecting. Try again.", "account_session_changed", ), )).toBe("authentication"); + expect(classifyPairedRuntimeFailure( + new PairedRuntimeHelloRejectedError( + "The computer you're connecting to is not signed in to an ADE account. Sign in on that computer, then try again.", + "account_not_signed_in", + ), + )).toBe("authentication"); + expect(classifyPairedRuntimeFailure( + new PairedRuntimeHelloRejectedError( + "The computer you're connecting to could not verify its ADE account session.", + "account_verification_failed", + ), + )).toBe("authentication"); }); it("diagnoses one dominant cause and says it without routes or ports", () => { diff --git a/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.ts b/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.ts index 4fe6b397f..f7fa09eeb 100644 --- a/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.ts +++ b/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.ts @@ -231,9 +231,11 @@ const HELLO_CODE_FAILURES: Record< repair_required: "pairing", // Desktop→desktop dials with `auth.kind: "paired"`, so the host's generic // `auth_failed` here is always a pairing-record rejection — including from - // hosts too old to send `repair_required`. Account problems arrive as - // `relay_account_required`, which has its own row. + // hosts too old to send `repair_required`. Account problems use their own + // authentication codes; `relay_account_required` remains the relay-only form. auth_failed: "pairing", + account_not_signed_in: "authentication", + account_verification_failed: "authentication", relay_account_required: "authentication", // The host is fine and the pairing is fine — it just cannot verify ADE // accounts yet. "Pair it again" would send the user in circles; the update diff --git a/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.test.ts b/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.test.ts index 88d2e098b..1d7add382 100644 --- a/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.test.ts +++ b/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.test.ts @@ -831,7 +831,9 @@ describe("DesktopPairedMachineStore", () => { return ws as unknown as WebSocket; }, }, - )).rejects.toThrow(/relay relay\.example:.*relay refused/i); + )).rejects.toThrow( + "Could not reach Legacy Studio. Make sure it's awake and ADE is open, then try again.", + ); } finally { warn.mockRestore(); } @@ -1031,7 +1033,11 @@ describe("DesktopPairedMachineStore", () => { }, ); - await expect(pairing).rejects.toThrow(expectedError); + await expect(pairing).rejects.toThrow( + responseAead + ? expectedError + : "Could not connect to Cipher Studio. Update ADE on that computer, then try again.", + ); expect(sentTypes).toEqual(["account_challenge"]); }); @@ -1359,8 +1365,10 @@ describe("DesktopPairedMachineStore", () => { }) as unknown as WebSocket, }, ); - // The host's real reason is surfaced, not the identity-verification error. - await expect(pairing).rejects.toThrow(/Try again in 3 minutes/); + // The host's real retry window is kept, without exposing the route details. + await expect(pairing).rejects.toThrow( + "Could not connect to Expected host. Too many failed authentication attempts. Try again in 3 minutes.", + ); await expect(pairing).rejects.not.toMatchObject({ code: "account_host_identity_verification_failed", }); @@ -1424,7 +1432,7 @@ describe("DesktopPairedMachineStore", () => { expect(sentTypes).toEqual(["account_challenge"]); }); - it("aggregates every failed adoption route with its kind and host", async () => { + it("summarizes failed adoption routes without exposing the route dump in the headline", async () => { const signing = generateKeyPairSync("ed25519"); const openedEndpoints: string[] = []; const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); @@ -1448,6 +1456,7 @@ describe("DesktopPairedMachineStore", () => { }; let failure: Error | null = null; + let warning: unknown = null; try { await new DesktopPairedMachineStore().pairWithAccountMachine( machine, @@ -1467,12 +1476,21 @@ describe("DesktopPairedMachineStore", () => { } catch (error) { failure = error instanceof Error ? error : new Error(String(error)); } finally { + warning = warn.mock.calls[0]?.[1] ?? null; warn.mockRestore(); } - expect(failure?.message).toMatch(/relay relay\.example:/); - expect(failure?.message).toMatch(/tailnet 100\.75\.20\.63:/); - expect(failure?.message).toMatch(/lan unavailable-studio\.local:/); + expect(failure?.message).toBe( + "Could not reach Unavailable Studio. Make sure it's awake and ADE is open, then try again.", + ); + expect(failure?.message).not.toMatch(/relay|tailnet|100\.75\.20\.63|unavailable-studio/i); + expect(warning).toMatchObject({ + attempts: [ + { kind: "lan", host: "unavailable-studio.local", failure: "unreachable" }, + { kind: "tailnet", host: "100.75.20.63", failure: "unreachable" }, + { kind: "relay", host: "relay.example", failure: "unreachable" }, + ], + }); expect(openedEndpoints.map(endpointWithoutCorrelation)).toEqual([ "ws://unavailable-studio.local:8787/", "ws://100.75.20.63:8787/", @@ -1483,6 +1501,56 @@ describe("DesktopPairedMachineStore", () => { expect(endpointCorrelationId(openedEndpoints[2]!)).toMatch(/^[0-9a-f-]{36}$/); }); + it("names the account machine when the target is signed out", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const machine: AdeAccountMachine = { + machineKey: "machine-signed-out", + deviceId: "host-signed-out", + name: "Arul's Mac Studio", + platform: "macOS", + deviceType: "desktop", + online: true, + lastSeenAt: Date.now(), + reachableEndpoints: [ + { kind: "relay", url: "wss://relay.example/connect/machine-signed-out" }, + ], + }; + + let failure: Error | null = null; + try { + await new DesktopPairedMachineStore().pairWithAccountMachine( + machine, + "laptop-account-token", + "MacBook Pro", + { + accountOwnerUserId: "account-user", + relayBaseUrls: ["https://relay.example"], + createWebSocket: () => new FakeWebSocket((text, ws) => { + const envelope = parseSyncEnvelope(wsDataToText(text)); + if (envelope.type !== "hello") return; + ws.receive(encodeSyncEnvelope({ + type: "hello_error", + requestId: envelope.requestId, + payload: { + code: "account_not_signed_in", + message: "The computer you're connecting to is not signed in to an ADE account. Sign in on that computer, then try again.", + }, + })); + }) as unknown as WebSocket, + }, + ); + } catch (error) { + failure = error instanceof Error ? error : new Error(String(error)); + } finally { + warn.mockRestore(); + } + + expect(failure?.message).toBe( + "Could not connect to Arul's Mac Studio. Arul's Mac Studio is not signed in to the same ADE account. Open ADE on that computer, sign in, then try again.", + ); + expect(failure?.message).not.toMatch(/sign out|this device|relay/i); + }); + it.each([ { name: "sign-out", ownerUserId: null }, { name: "account switch", ownerUserId: "account-user-2" }, diff --git a/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts b/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts index 6a3160e02..1d2d200f0 100644 --- a/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts +++ b/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts @@ -15,6 +15,7 @@ import type { import type { SyncPairingHostIdentity, SyncAccountChallengeOkPayload, + SyncHelloErrorPayload, SyncHelloPayload, SyncPairingResultPayload, SyncPeerMetadata, @@ -95,6 +96,18 @@ class AccountPairingAuthorizationError extends Error { readonly code = "account_session_changed"; } +class AccountMachineHelloRejectedError extends Error { + readonly code = "account_machine_hello_rejected"; + + constructor( + message: string, + readonly helloCode: SyncHelloErrorPayload["code"] | null, + ) { + super(message); + this.name = "AccountMachineHelloRejectedError"; + } +} + const HOST_IDENTITY_VERIFICATION_ERROR = "Host identity verification failed — the machine may be running an older ADE."; const ADOPT_CHANNEL_UPDATE_REQUIRED_ERROR = @@ -124,6 +137,7 @@ function parseDirectoryEd25519PublicKey(value: string): Buffer { type AccountMachineAdoptionFailure = { route: AccountMachineAdoptionRoute; reason: string; + helloCode: SyncHelloErrorPayload["code"] | null; }; function emitAccountMachineAdoptionStage( @@ -152,41 +166,47 @@ function accountMachineAdoptionRouteHost(route: AccountMachineAdoptionRoute): st } } -function formatAccountMachineAdoptionFailure( - failure: AccountMachineAdoptionFailure, -): string { - return `${failure.route.kind} ${accountMachineAdoptionRouteHost(failure.route)}: ${ - boundedInlineText(failure.reason, 160) - }`; -} - /** - * Adoption's own message classifier. The canonical one is - * `classifyPairedRuntimeFailure` in `pairedRuntimeRoutes.ts` — prefer it for - * anything that dials an already-paired machine, since it reads the host's - * structured `hello_error.code` before falling back to prose. - * - * Adoption is deliberately NOT expressed through it: this flow has no hello - * code to read, and it orders the prose rules differently on purpose. "Pair it - * again" is a first-class outcome here (adoption is the repair), a credential - * word outranks a transport word (a token failure mid-dial is an auth problem, - * not a dead route), and `cipher` reads as identity. Routing these strings - * through the canonical classifier would answer "unknown"/"unreachable" - * instead, so the two stay separate until one of them has tests to hold the - * merged behavior in place. + * Account adoption has a different meaning from a saved paired dial: account + * authentication is being used to establish the pairing, so a host rejection + * must describe the target account state rather than suggest that the local + * pairing is stale. Prefer the structured hello code; prose is only a fallback + * for older hosts and transport-level errors. */ function classifyAccountMachineAdoptionFailure( - reason: string, + failure: AccountMachineAdoptionFailure, ): RemoteRuntimeConnectionAttemptFailure { + switch (failure.helloCode) { + case "account_not_signed_in": + case "account_verification_failed": + case "account_session_changed": + case "relay_account_required": + return "authentication"; + case "repair_required": + return "pairing"; + case "host_update_required": + case "invalid_hello": + case "protocol_version_mismatch": + return "protocol"; + case "connection_attempt_superseded": + return "superseded"; + default: + break; + } + + const reason = failure.reason; if (/timed? out|timeout/i.test(reason)) return "timeout"; // Adoption is how a stale pairing gets repaired, so a host that says the // pairing is the problem is naming the very thing this flow fixes. if (/pair (?:it|this) again|no longer valid|not paired/i.test(reason)) { return "pairing"; } - if (/auth|token|credential|proof|forbidden|unauthorized/i.test(reason)) { + if ( + /auth|token|credential|proof|forbidden|unauthorized|sign(?:ed)?[ -]?in|account.*(?:verify|session)|(?:verify|session).*account/i.test(reason) + ) { return "authentication"; } + if (/older ADE|compatible cipher|update (?:it|ADE)/i.test(reason)) return "protocol"; if (/identity|signature|device id|cipher/i.test(reason)) return "identity"; if (/ECONN|EHOST|ENET|unreach|offline|closed|socket|websocket|refused/i.test(reason)) { return "unreachable"; @@ -194,6 +214,72 @@ function classifyAccountMachineAdoptionFailure( return "unknown"; } +const ACCOUNT_ADOPTION_FAILURE_PRECEDENCE: readonly RemoteRuntimeConnectionAttemptFailure[] = [ + "authentication", + "pairing", + "identity", + "capability", + "protocol", + "superseded", + "timeout", + "unreachable", + "unknown", +]; + +function dominantAccountMachineAdoptionFailure( + failures: readonly AccountMachineAdoptionFailure[], +): RemoteRuntimeConnectionAttemptFailure { + const seen = new Set(failures.map(classifyAccountMachineAdoptionFailure)); + return ACCOUNT_ADOPTION_FAILURE_PRECEDENCE.find((failure) => seen.has(failure)) ?? "unknown"; +} + +function accountMachineAdoptionFailureMessage( + machine: AdeAccountMachine, + failures: readonly AccountMachineAdoptionFailure[], +): string { + const machineName = accountMachineDisplayName(machine) ?? "that computer"; + const dominant = dominantAccountMachineAdoptionFailure(failures); + const targetIsSignedOut = failures.some((failure) => + failure.helloCode === "account_not_signed_in" + || failure.helloCode === "relay_account_required" + || /not signed in|signed out/i.test(failure.reason) + ); + + if (dominant === "authentication" && targetIsSignedOut) { + return `Could not connect to ${machineName}. ${machineName} is not signed in to the same ADE account. ` + + "Open ADE on that computer, sign in, then try again."; + } + const retryLaterFailure = failures.find((failure) => + /try again in\s+\d+\s+(?:second|minute|hour)/i.test(failure.reason) + ); + switch (dominant) { + case "authentication": + if (retryLaterFailure) { + return `Could not connect to ${machineName}. ${boundedInlineText(retryLaterFailure.reason, 240)}`; + } + return `Could not connect to ${machineName}. ADE could not verify the account on that computer. ` + + "Open ADE there and check that it is signed in to the same ADE account, then try again."; + case "pairing": + return `Could not connect to ${machineName}. Its saved pairing for this device is no longer valid. ` + + "Pair it again, then try again."; + case "identity": + return `Could not verify the identity of ${machineName}. Check that its account entry is current, ` + + "then try again."; + case "capability": + case "protocol": + return `Could not connect to ${machineName}. Update ADE on that computer, then try again.`; + case "superseded": + return `Another connection to ${machineName} took over. Try again.`; + case "timeout": + return `Could not connect to ${machineName} in time. Make sure ADE is open on that computer, ` + + "then try again."; + case "unreachable": + return `Could not reach ${machineName}. Make sure it's awake and ADE is open, then try again.`; + default: + return `Could not connect to ${machineName}. Open ADE on that computer and try again.`; + } +} + function nowIso(): string { return new Date().toISOString(); } @@ -966,6 +1052,7 @@ export class DesktopPairedMachineStore { failures.push({ route, reason: error instanceof Error ? error.message : String(error), + helloCode: null, }); continue; } @@ -1136,11 +1223,14 @@ export class DesktopPairedMachineStore { connection.send("hello", hello, requestId); const envelope = await response; if (envelope.type === "hello_error") { - const payload = envelope.payload as { message?: unknown }; - throw new Error( - typeof payload.message === "string" && payload.message.trim() + const payload = envelope.payload as Partial | null; + throw new AccountMachineHelloRejectedError( + typeof payload?.message === "string" && payload.message.trim() ? payload.message.trim() : "Account authentication was rejected.", + typeof payload?.code === "string" + ? payload.code as SyncHelloErrorPayload["code"] + : null, ); } let helloOk: PairedRuntimeHelloOkPayload; @@ -1229,6 +1319,9 @@ export class DesktopPairedMachineStore { failures.push({ route, reason: error instanceof Error ? error.message : String(error), + helloCode: error instanceof AccountMachineHelloRejectedError + ? error.helloCode + : null, }); } finally { connection.close(1000, "Account pairing finished."); @@ -1239,26 +1332,15 @@ export class DesktopPairedMachineStore { attempts: failures.slice(0, MAX_ROUTE_ATTEMPTS).map((failure) => ({ kind: failure.route.kind, host: accountMachineAdoptionRouteHost(failure.route), - failure: classifyAccountMachineAdoptionFailure(failure.reason), + failure: classifyAccountMachineAdoptionFailure(failure), + ...(failure.helloCode ? { helloCode: failure.helloCode } : {}), + reason: boundedInlineText(failure.reason, 200), })), omittedAttemptCount: Math.max(0, failures.length - MAX_ROUTE_ATTEMPTS), }); - // Unlike the paired dial loop, adoption's per-route reasons are specific - // and actionable in their own right ("update that computer", "try again in - // 3 minutes"), so collapsing them into one generic sentence would lose the - // instruction. They stay, bounded and joined. - const visibleFailures = failures.slice(0, MAX_ROUTE_ATTEMPTS) - .map(formatAccountMachineAdoptionFailure); - if (failures.length > MAX_ROUTE_ATTEMPTS) { - visibleFailures.push( - `${failures.length - MAX_ROUTE_ATTEMPTS} more route attempts failed`, - ); - } - throw new Error( - `Could not connect to ${accountMachineDisplayName(machine) ?? machine.machineKey} with your ADE account. ${ - visibleFailures.join("; ") - }`, - ); + // The route list is diagnostic data, not a headline. Keep it in the bounded + // warning above and give the account row one target-specific next step. + throw new Error(accountMachineAdoptionFailureMessage(machine, failures)); } private read(): DesktopPairedMachinesFile { diff --git a/apps/desktop/src/renderer/webclient/sync/__tests__/sync.test.ts b/apps/desktop/src/renderer/webclient/sync/__tests__/sync.test.ts index d6881e01a..c8341feb6 100644 --- a/apps/desktop/src/renderer/webclient/sync/__tests__/sync.test.ts +++ b/apps/desktop/src/renderer/webclient/sync/__tests__/sync.test.ts @@ -2599,6 +2599,14 @@ describe("browser sync connection and client", () => { code: "host_update_required" as const, message: "Update ADE on that machine to connect.", }, + { + code: "account_not_signed_in" as const, + message: "The computer you're connecting to is not signed in to an ADE account.", + }, + { + code: "account_verification_failed" as const, + message: "The computer you're connecting to could not verify its ADE account session.", + }, ]) { it(`keeps the pairing when the host rejects with ${scenario.code}`, async () => { const storage = new MemoryStorage(); diff --git a/apps/desktop/src/renderer/webclient/sync/connection.ts b/apps/desktop/src/renderer/webclient/sync/connection.ts index 477c625a2..b45ae0d1e 100644 --- a/apps/desktop/src/renderer/webclient/sync/connection.ts +++ b/apps/desktop/src/renderer/webclient/sync/connection.ts @@ -1334,7 +1334,7 @@ export class SyncConnection { this.setStatus({ state: "error", error: payload.message }); return new SyncConnectionError(payload.message, "relay_account_required", payload); } - // Neither of these is a reason to destroy a saved pairing, so they must not + // These account/host states are not a reason to destroy a saved pairing, so they must not // reach the `auth_failed` status that WebMachineSessionManager treats as a // signal to invalidate the environment. The account session moving under a // handshake is transient — keep reconnecting. A host too old to verify @@ -1349,6 +1349,14 @@ export class SyncConnection { this.setStatus({ state: "error", error: payload.message }); return new SyncConnectionError(payload.message, "host_update_required", payload); } + if ( + payload.code === "account_not_signed_in" + || payload.code === "account_verification_failed" + ) { + this.shouldReconnect = false; + this.setStatus({ state: "error", error: payload.message }); + return new SyncConnectionError(payload.message, payload.code, payload); + } const attributedToPairing = payload.host?.deviceId === environment.hostDeviceId; this.consecutiveAuthFailures += 1; this.emit("authFailed", { payload, attributedToPairing }); diff --git a/apps/desktop/src/shared/types/sync.ts b/apps/desktop/src/shared/types/sync.ts index 0bdcb080c..f6d797a51 100644 --- a/apps/desktop/src/shared/types/sync.ts +++ b/apps/desktop/src/shared/types/sync.ts @@ -1108,16 +1108,18 @@ export type SyncHelloErrorPayload = * pair again. Clients must classify from this code and never by matching * the host-sent message text. * - * The two codes below it exist because `auth_failed` reads as "pair it - * again" on every client, and that is the wrong instruction for a host - * that simply cannot verify accounts yet (`host_update_required` — update - * it there) or whose account session moved under the handshake - * (`account_session_changed` — sign in, then retry). Neither is a reason - * to destroy a saved pairing. + * The specific codes below it exist because `auth_failed` reads as "pair + * it again" on every client, and that is the wrong instruction for a host + * that is signed out (`account_not_signed_in`), cannot verify accounts yet + * (`host_update_required` — update it there), or whose account session + * moved under the handshake (`account_session_changed` — sign in, then + * retry). None of those is a reason to destroy a saved pairing. */ code: | "auth_failed" | "repair_required" + | "account_not_signed_in" + | "account_verification_failed" | "host_update_required" | "account_session_changed" | "invalid_hello" diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index 58ef0167a..493429a0c 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -2431,13 +2431,14 @@ private let syncRespondingHostNameKey = "ADERespondingHostName" /// the saved pairing is no longer usable — so every pairing decision reads /// this instead of comparing against a single code. /// -/// Deliberately NOT every rejection code. A host that cannot verify accounts -/// yet (`host_update_required`) or whose account session moved under the -/// handshake (`account_session_changed`) is still the machine this phone is -/// paired with, and neither is a reason to drop the pairing. Any code this -/// list does not name — including ones added to the host after this build -/// shipped — falls through to "show the host's message, keep the pairing, -/// keep retrying". +/// Deliberately NOT every rejection code. A host that is signed out +/// (`account_not_signed_in`), cannot verify accounts yet +/// (`account_verification_failed` or `host_update_required`), or whose account +/// session moved under the handshake (`account_session_changed`) is still the +/// machine this phone is paired with, and none of those is a reason to drop +/// the pairing. Any code this list does not name — including ones added to the +/// host after this build shipped — falls through to "show the host's message, +/// keep the pairing, keep retrying". private func syncCodeIsPairingRejection(_ code: String?) -> Bool { code == "auth_failed" || code == "repair_required" } diff --git a/apps/ios/ADETests/ADETests.swift b/apps/ios/ADETests/ADETests.swift index 3dd9bf1f5..b651b53a1 100644 --- a/apps/ios/ADETests/ADETests.swift +++ b/apps/ios/ADETests/ADETests.swift @@ -3791,6 +3791,8 @@ final class ADETests: XCTestCase { XCTAssertTrue(invalidates("repair_required")) XCTAssertFalse(invalidates("host_update_required")) + XCTAssertFalse(invalidates("account_not_signed_in")) + XCTAssertFalse(invalidates("account_verification_failed")) XCTAssertFalse(invalidates("account_session_changed")) XCTAssertFalse(invalidates("invalid_hello")) XCTAssertFalse(invalidates("connection_attempt_superseded")) diff --git a/docs/features/sync-and-multi-device/README.md b/docs/features/sync-and-multi-device/README.md index 410733e4d..ec514ec41 100644 --- a/docs/features/sync-and-multi-device/README.md +++ b/docs/features/sync-and-multi-device/README.md @@ -2435,6 +2435,8 @@ pattern-matching it is a defect. `SyncHelloErrorPayload` (in | --- | --- | --- | | `repair_required` | The host has no usable pairing record for this device. | Pair again. This is the one rejection the user can act on directly. | | `auth_failed` | The older, generic form of the same thing. | Treat exactly as `repair_required`. | +| `account_not_signed_in` | The target computer is not signed in to an ADE account. | Sign in to the same ADE account on that computer. Never destroy a saved pairing. | +| `account_verification_failed` | The target computer could not verify its ADE account session. | Check ADE's account state on that computer, then retry. Never destroy a saved pairing. | | `host_update_required` | The host cannot verify ADE accounts yet. | Update ADE **on that machine**. Never destroy a saved pairing. | | `account_session_changed` | The host's account session moved under the handshake, or the ingress cannot finish this sign-in shape. | Sign in / retry. Never destroy a saved pairing. | | `relay_account_required` | The route needs an account-authenticated hello. | Sign in on this device. | @@ -2442,11 +2444,13 @@ pattern-matching it is a defect. `SyncHelloErrorPayload` (in | `invalid_hello` | The payload was malformed. | Client bug or version skew. | | `protocol_version_mismatch` | Version floor/ceiling, as above. | Update the side named by `updateTarget`. | -`host_update_required` and `account_session_changed` exist because +`account_not_signed_in`, `account_verification_failed`, +`host_update_required`, and `account_session_changed` exist because `auth_failed` reads as "pair again" on every client, and that is the wrong — and -destructive — instruction for a host that simply cannot verify accounts yet or -whose session moved mid-handshake. Where a rejection *can* legitimately lead a -client to drop a saved pairing, the host also attributes itself with +destructive — instruction for a target whose account is unavailable, whose +session cannot be verified, whose host is too old, or whose session moved +mid-handshake. Where a rejection *can* legitimately lead a client to drop a +saved pairing, the host also attributes itself with `hello_error.host: { deviceId, name }`, and the client only acts when that identity matches the pairing it holds. @@ -2514,16 +2518,16 @@ is the host-observed `direct | relay` truth after authentication; controllers use it for diagnostics/policy rather than inferring the path solely from a cached candidate label. -`SyncHelloErrorPayload.code` is trimmed to `auth_failed | -invalid_hello`. An `auth_failed` payload also carries an optional -`host: { deviceId, name }` naming the machine that rejected the hello — -both the project host and the brain-level fallback handler send it. This -is the client's only safe basis for destroying a saved pairing: a phone -drops its credentials **only** when the rejecting `host.deviceId` matches -the paired machine's identity. An unattributed rejection (older host, or -a stranger machine reached over a reused DHCP lease / mDNS alias / stale -Tailscale candidate) keeps the pairing and the client moves on to other -routes. `SyncPairingResultPayload.error.code` is one of +The `hello_error` union includes the pairing, account, protocol, and route +codes in the table above. An `auth_failed` payload also carries an optional +`host: { deviceId, name }` naming the machine that rejected the hello — both +the project host and the brain-level fallback handler send it. This is the +client's only safe basis for destroying a saved pairing: a phone drops its +credentials **only** when the rejecting `host.deviceId` matches the paired +machine's identity. An unattributed rejection (older host, or a stranger +machine reached over a reused DHCP lease / mDNS alias / stale Tailscale +candidate) keeps the pairing and the client moves on to other routes. +`SyncPairingResultPayload.error.code` is one of `invalid_pin | pin_not_set | pairing_failed`. Heartbeat interval is 60 seconds. Desktop peers close after **two** @@ -2893,7 +2897,7 @@ feature is merged or because a deliberately isolated-port host is running. | Account publication + relay for a machine with no registered project | Implemented (`projectlessSyncSnapshot`, `machineRelayTunnel`, `runServe` publisher snapshot fallback) | | One hello parser + one account-hello gate chain across both ingresses | Implemented (`syncHelloProtocol`, `syncAccountHelloAuth`) | | Machine-level pairing PIN / device forget / DPoP posture on a projectless brain | Implemented (`brainMachineSyncStores`, `ProjectlessSyncControls`, `withSyncService`) | -| Code-first `hello_error` classification with non-destructive `host_update_required` / `account_session_changed` | Implemented (desktop `classifyPairedRuntimeFailure`, web `SyncConnection`, iOS `syncCodeIsPairingRejection`) | +| Code-first `hello_error` classification with non-destructive account/session/host-state codes | Implemented (`account_not_signed_in`, `account_verification_failed`, `host_update_required`, and `account_session_changed` across desktop, web, and iOS) | | Relay eviction (`4505`) suppression + surfaced outage | Implemented (bounded re-attempts, 10-minute re-arm, `routeHealth.relay.relayControlSuppressed*`, `ade doctor` relay row, desktop `relay-offline` banner) | | Sealed account adoption over direct routes (`ade-adopt-v1`, host `pubkey` identity, LAN → tailnet → Relay fallback, negotiated ChaCha20-Poly1305 / AES-256-GCM AEAD) | Implemented (`machineIdentitySigningStore` + `adoptChannelCrypto`; desktop + iOS clients) | | Legacy manual-pairing adoption into an account (DPoP-gated) + `localTrustOrigin` demotion on sign-out | Implemented (`syncPairingStore.pairPeerViaAccount` / `revokeAccountOwnedExcept`, `syncHostService` account hello) | diff --git a/docs/features/sync-and-multi-device/ios-companion.md b/docs/features/sync-and-multi-device/ios-companion.md index 58921a4fb..c687bcd6f 100644 --- a/docs/features/sync-and-multi-device/ios-companion.md +++ b/docs/features/sync-and-multi-device/ios-companion.md @@ -990,8 +990,11 @@ Sources: `apps/ios/ADE/Services/SyncService.swift` and or the generic `auth_failed` (what older hosts send for the same cause). Both mean the saved pairing is no longer usable, so every pairing decision asks that one function instead of comparing against a single code string. - The newer `host_update_required` and `account_session_changed` codes are - deliberately *not* in that set: neither is a reason to destroy a pairing. + The newer `account_not_signed_in`, `account_verification_failed`, + `host_update_required`, and `account_session_changed` codes are deliberately + *not* in that set: none is a reason to destroy a pairing. iOS keeps the + host's target-specific message so the user is told which computer needs + attention. 3. Send local `db_version` plus the per-host-DB cursor map (`remoteDbVersionBySite`); `hello_ok` returns the host DB's `serverDbSiteId` and the runtime's current project catalog when the From c638bf5a02b15a6b964b3c38d97ee910ce3cbf14 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Tue, 11 Aug 2026 15:59:59 -0400 Subject: [PATCH 2/2] =?UTF-8?q?ship:=20iteration=201=20=E2=80=94=20address?= =?UTF-8?q?=20review=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/services/sync/syncAccountHelloAuth.ts | 49 ++++-- .../src/services/sync/syncHostService.test.ts | 161 ++++++++++++++++++ .../remoteRuntime/pairedRuntimeBootstrap.ts | 17 +- .../remoteRuntime/pairedRuntimeRoutes.test.ts | 14 ++ .../remoteRuntime/pairedRuntimeRoutes.ts | 12 ++ .../syncPairedMachineStore.test.ts | 9 +- .../remoteRuntime/syncPairedMachineStore.ts | 12 +- .../webclient/sync/__tests__/sync.test.ts | 52 ++++++ .../src/renderer/webclient/sync/connection.ts | 4 + apps/desktop/src/shared/types/sync.ts | 4 +- 10 files changed, 311 insertions(+), 23 deletions(-) diff --git a/apps/ade-cli/src/services/sync/syncAccountHelloAuth.ts b/apps/ade-cli/src/services/sync/syncAccountHelloAuth.ts index 090d47e56..9cb0434ae 100644 --- a/apps/ade-cli/src/services/sync/syncAccountHelloAuth.ts +++ b/apps/ade-cli/src/services/sync/syncAccountHelloAuth.ts @@ -51,6 +51,9 @@ export const SYNC_ACCOUNT_OTHER_OWNER_MESSAGE = "This device is already paired t export const SYNC_ACCOUNT_PAIRING_WRITE_FAILED_MESSAGE = "The computer you're connecting to could not" + " save the new pairing for this device. Try again."; +export const SYNC_ACCOUNT_COMMIT_FAILED_MESSAGE = "The computer you're connecting to could not" + + " finish saving the account pairing. Try again."; + export const SYNC_ACCOUNT_VERIFY_FAILED_MESSAGE = "The computer you're connecting to could not verify" + " its ADE account session. Open ADE there and check that it is signed in to the same ADE account," + " then try again."; @@ -166,11 +169,22 @@ export async function authenticateSyncAccountHello( // `auth_failed` here reads as "pair it again" on every client. return reject(SYNC_ACCOUNT_VERIFY_UNAVAILABLE_MESSAGE, "host_update_required"); } - const attestation = await options.verifyAccountAttestation({ - token: auth.accountToken, - expectedUserId: authorization.userId, - config, - }); + let attestation: VerifiedAccountAttestation; + try { + attestation = await options.verifyAccountAttestation({ + token: auth.accountToken, + expectedUserId: authorization.userId, + config, + }); + } catch (error) { + logger.warn(`${logPrefix}.account_attestation_rejected`, { + deviceId: auth.deviceId, + reason: typeof (error as { code?: unknown } | null)?.code === "string" + ? (error as { code: string }).code + : "verification_failed", + }); + return reject(SYNC_ACCOUNT_VERIFY_FAILED_MESSAGE, "account_verification_failed"); + } if (!isPeerCurrent()) return { kind: "stale" }; const commitAuthorization = await options.captureAccountAuthorization(); if (!isPeerCurrent()) return { kind: "stale" }; @@ -268,10 +282,21 @@ export async function authenticateSyncAccountHello( // acknowledgement to arm, and staging deliberately withholds elevations, // so a staged adoption would leave the record local for exactly as long // as the bug it fixes. - const paired = pairingStore.pairPeerViaAccount(peer, attestation, { - dpopPublicKey: existingPairingRecord ? null : auth.dpop?.publicKey ?? null, - runtimeHostGrant: auth.runtimeHostGrant ?? null, - }); + let paired: ReturnType; + try { + paired = pairingStore.pairPeerViaAccount(peer, attestation, { + dpopPublicKey: existingPairingRecord ? null : auth.dpop?.publicKey ?? null, + runtimeHostGrant: auth.runtimeHostGrant ?? null, + }); + } catch (error) { + logger.warn(`${logPrefix}.account_pairing_write_failed`, { + deviceId: auth.deviceId, + reason: typeof (error as { code?: unknown } | null)?.code === "string" + ? (error as { code: string }).code + : "pairing_write_failed", + }); + return reject(SYNC_ACCOUNT_PAIRING_WRITE_FAILED_MESSAGE); + } // Read-only: this confirms the record we just wrote is readable, and must // not be mistaken for the device proving it received the secret (which is // what promotes a staged rotation). @@ -294,12 +319,12 @@ export async function authenticateSyncAccountHello( }; }); } catch (error) { - logger.warn(`${logPrefix}.account_auth_rejected`, { + logger.warn(`${logPrefix}.account_commit_failed`, { deviceId: auth.deviceId, reason: typeof (error as { code?: unknown } | null)?.code === "string" ? (error as { code: string }).code - : "verification_failed", + : "commit_failed", }); - return reject(SYNC_ACCOUNT_VERIFY_FAILED_MESSAGE, "account_verification_failed"); + return reject(SYNC_ACCOUNT_COMMIT_FAILED_MESSAGE); } } diff --git a/apps/ade-cli/src/services/sync/syncHostService.test.ts b/apps/ade-cli/src/services/sync/syncHostService.test.ts index bb2736c65..89e6f6166 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.test.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.test.ts @@ -4185,6 +4185,77 @@ describe("sync host account authentication", () => { } }); + it("keeps project-host pairing-write failures out of account verification errors", async () => { + const { projectRoot, cleanup } = createTempProjectRoot(); + const secretsDir = path.join(projectRoot, ".ade", "secrets"); + const pinStore = createSyncPinStore({ filePath: path.join(secretsDir, "sync-pin.json") }); + const pairingSecretsPath = path.join(secretsDir, "sync-paired-devices.json"); + const pairingStore = createSyncPairingStore({ filePath: pairingSecretsPath, pinStore }); + const pairPeerViaAccount = vi.spyOn(pairingStore, "pairPeerViaAccount") + .mockImplementation(() => { + throw new Error("pairing store write failed"); + }); + const listener = createSharedSyncListener({ bindHost: "127.0.0.1" }); + const baseArgs = createHostArgs(projectRoot, []); + const host = createSyncHostService({ + ...baseArgs, + ...accountDependencies(), + pinStore, + pairingStore, + pairingSecretsPath, + sharedListener: listener, + discoveryEnabled: false, + deviceRegistryService: { + ...baseArgs.deviceRegistryService, + upsertPeerMetadata: vi.fn(), + }, + } as unknown as Parameters[0]); + const clients: Array>> = []; + try { + const port = await host.waitUntilListening(); + const peer = { + deviceId: "project-host-pairing-write-failure", + deviceName: "Project host test peer", + platform: "iOS", + deviceType: "phone", + siteId: "project-host-pairing-write-failure-site", + dbVersion: 0, + } satisfies SyncPeerMetadata; + const accountToken = await mintAccountToken(); + const dpopKey = makeDpopKeyPair(); + const client = await openAccountClient(port, listener.getRelayBridgeProof()); + clients.push(client); + sendAccountHello({ + ws: client.ws, + peer, + accountToken, + dpop: signAccountDpop({ + privateKey: dpopKey.privateKey, + publicKeyX963: dpopKey.publicKeyX963, + deviceId: peer.deviceId, + accountToken, + }), + }); + const rejection = await waitForValue( + () => client.envelopes.find((envelope) => envelope.type === "hello_error"), + "project-host pairing-write hello_error", + ); + expect(rejection.payload).toMatchObject({ + code: "auth_failed", + message: expect.stringMatching(/could not save the new pairing/i), + }); + expect((rejection.payload as { message: string }).message) + .not.toMatch(/could not verify/i); + expect(pairPeerViaAccount).toHaveBeenCalledTimes(1); + } finally { + pairPeerViaAccount.mockRestore(); + for (const client of clients) client.ws.close(); + await host.dispose(); + await listener.close(); + cleanup(); + } + }); + // A re-pair used to overwrite the device's working secret the instant the // host answered, two round trips before the device could persist the reply. // Dropping the socket in that gap — the ordinary outcome on a flaky network — @@ -4442,6 +4513,96 @@ describe("sync host account authentication", () => { } }); + it("projectless brain keeps pairing-write failures out of account verification errors", async () => { + const { projectRoot, cleanup } = createTempProjectRoot(); + const secretsDir = path.join(projectRoot, "secrets"); + fs.mkdirSync(secretsDir, { recursive: true }); + resetBrainMachineSyncStoresForTests(); + const stores = resolveBrainMachineSyncStores(secretsDir); + const pairPeerViaAccount = vi.spyOn(stores.pairingStore, "pairPeerViaAccount") + .mockImplementation(() => { + throw new Error("pairing store write failed"); + }); + const deviceKey = makeDpopKeyPair(); + const accountToken = await mintAccountToken(); + const handler = createBrainProjectActionsSyncHandler({ + logger: createDiscoveryLogger(), + projectCatalogProvider: { + listProjects: vi.fn(async () => ({ projects: [] })), + prepareProjectConnection: vi.fn(), + }, + bootstrapCredentialStore: new EncryptedFileCredentialStore({ + secretsDir, + keyMaterial: { read: () => null }, + }), + secretsDir, + localDeviceIdPath: path.join(secretsDir, "sync-device-id"), + localSiteIdPath: path.join(secretsDir, "sync-site-id"), + accountAuthService: { + getStatus: () => ({ + signedIn: true, + userId: ownerUserId, + email: null, + name: null, + expiresAt: new Date(Date.now() + 10 * 60_000).toISOString(), + }), + getAccessToken: async () => "host-account-lease", + }, + getAccountAttestationConfig: () => ({ issuer, jwksUrl, oauthClientId }), + }); + const server = new WebSocketServer({ host: "127.0.0.1", port: 0 }); + server.on("connection", (ws, request) => handler({ + ws, + remoteAddress: request.socket.remoteAddress ?? null, + remotePort: request.socket.remotePort ?? null, + transportOrigin: "relay-bridge", + })); + let client: WebSocket | null = null; + try { + await new Promise((resolve, reject) => { + server.once("listening", resolve); + server.once("error", reject); + }); + const peer = { + deviceId: "projectless-pairing-write-failure", + deviceName: "Projectless test peer", + platform: "unknown", + deviceType: "browser", + siteId: "projectless-pairing-write-failure-site", + dbVersion: 0, + } satisfies SyncPeerMetadata; + const opened = await openAccountClient((server.address() as AddressInfo).port); + client = opened.ws; + sendAccountHello({ + ws: opened.ws, + peer, + accountToken, + dpop: signAccountDpop({ + privateKey: deviceKey.privateKey, + publicKeyX963: deviceKey.publicKeyX963, + deviceId: peer.deviceId, + accountToken, + }), + }); + const rejection = await waitForValue( + () => opened.envelopes.find((envelope) => envelope.type === "hello_error"), + "projectless pairing-write hello_error", + ); + expect(rejection.payload).toMatchObject({ + code: "auth_failed", + message: expect.stringMatching(/could not save the new pairing/i), + }); + expect((rejection.payload as { message: string }).message) + .not.toMatch(/could not verify/i); + expect(pairPeerViaAccount).toHaveBeenCalledTimes(1); + } finally { + pairPeerViaAccount.mockRestore(); + client?.close(); + await new Promise((resolve) => server.close(() => resolve())); + cleanup(); + } + }); + /** * Regression: the same fresh machine, reached by a phone over the LAN. * `ade sync pin generate` and the desktop's pairing card write the machine diff --git a/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeBootstrap.ts b/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeBootstrap.ts index 386804871..6561b38ab 100644 --- a/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeBootstrap.ts +++ b/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeBootstrap.ts @@ -30,10 +30,12 @@ import { orderPairedCandidates, pairedRuntimeFailureMessage, pairedRuntimeRouteHost, + type PairedRuntimeAccountHelloCode, type PairedRuntimeEndpointCandidate, } from "./pairedRuntimeRoutes"; import { PairedRuntimeCompatibilityError, + PairedRuntimeHelloRejectedError, PairedRuntimeRelayAuthRequiredError, PairedRuntimeTransportUnavailableError, } from "./pairedRuntimeErrors"; @@ -122,6 +124,7 @@ export async function bootstrapPairedRuntime(args: { const attemptRecorder = createRouteAttemptRecorder(); const { attempts, record: recordAttempt } = attemptRecorder; let relayAuthError: PairedRuntimeRelayAuthRequiredError | null = null; + let accountHelloCode: PairedRuntimeAccountHelloCode | null = null; // Keep the phases explicit even if a future candidate-builder change // accidentally reorders endpoints. const orderedCandidates = orderPairedCandidates(candidates); @@ -227,6 +230,16 @@ export async function bootstrapPairedRuntime(args: { }); continue; } + if ( + error instanceof PairedRuntimeHelloRejectedError + && ( + error.helloCode === "account_not_signed_in" + || error.helloCode === "account_verification_failed" + ) + && accountHelloCode == null + ) { + accountHelloCode = error.helloCode; + } const failure = classifyPairedRuntimeFailure(error); markEndpointFailed(candidate, failure); recordAttempt({ @@ -395,7 +408,7 @@ export async function bootstrapPairedRuntime(args: { }; // A skipped relay leg only wins when nothing more actionable was found; a // host that rejected the pairing outranks "you aren't signed in". - if (relayAuthError && failure === "authentication") { + if (relayAuthError && failure === "authentication" && accountHelloCode == null) { throw new PairedRuntimeRelayAuthRequiredError( relayAuthError.message, relayAuthError.cause, @@ -403,7 +416,7 @@ export async function bootstrapPairedRuntime(args: { ); } throw new PairedRuntimeTransportUnavailableError( - pairedRuntimeFailureMessage(failure, credentials.hostIdentity.name), + pairedRuntimeFailureMessage(failure, credentials.hostIdentity.name, accountHelloCode), undefined, diagnostic, ); diff --git a/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.test.ts b/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.test.ts index 8b87e870b..577e48fe4 100644 --- a/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.test.ts +++ b/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.test.ts @@ -222,6 +222,20 @@ describe("paired runtime endpoint routes", () => { expect(pairedRuntimeFailureMessage("authentication", "Mac Studio")).toBe( "Sign in to ADE to connect through the relay.", ); + expect(pairedRuntimeFailureMessage( + "authentication", + "Mac Studio", + "account_not_signed_in", + )).toBe( + "Mac Studio is not signed in to an ADE account. Sign in there, then try again.", + ); + expect(pairedRuntimeFailureMessage( + "authentication", + "Mac Studio", + "account_verification_failed", + )).toBe( + "Mac Studio could not verify its ADE account session. Open ADE there and check that it is signed in to the same ADE account, then try again.", + ); // No attempts recorded at all still produces a sentence, not an empty one. expect(dominantPairedRuntimeFailure([])).toBe("unknown"); expect(pairedRuntimeFailureMessage("unknown", null)).toContain("that computer"); diff --git a/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.ts b/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.ts index f7fa09eeb..7354e3fcd 100644 --- a/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.ts +++ b/apps/desktop/src/main/services/remoteRuntime/pairedRuntimeRoutes.ts @@ -299,6 +299,11 @@ const FAILURE_PRECEDENCE: readonly RemoteRuntimeConnectionAttemptFailure[] = [ "unknown", ]; +export type PairedRuntimeAccountHelloCode = Extract< + SyncHelloErrorPayload["code"], + "account_not_signed_in" | "account_verification_failed" +>; + export function dominantPairedRuntimeFailure( attempts: readonly RemoteRuntimeConnectionAttempt[], ): RemoteRuntimeConnectionAttemptFailure { @@ -316,12 +321,19 @@ export function dominantPairedRuntimeFailure( export function pairedRuntimeFailureMessage( failure: RemoteRuntimeConnectionAttemptFailure, machineNameValue: string | null | undefined, + accountHelloCode?: PairedRuntimeAccountHelloCode | null, ): string { const machine = machineNameValue?.trim() || "that computer"; switch (failure) { case "pairing": return `${machine} says this device's pairing is out of date — pair it again.`; case "authentication": + if (accountHelloCode === "account_not_signed_in") { + return `${machine} is not signed in to an ADE account. Sign in there, then try again.`; + } + if (accountHelloCode === "account_verification_failed") { + return `${machine} could not verify its ADE account session. Open ADE there and check that it is signed in to the same ADE account, then try again.`; + } return "Sign in to ADE to connect through the relay."; case "identity": return `A different computer answered at ${machine}'s saved address — try again.`; diff --git a/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.test.ts b/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.test.ts index 1d7add382..d4fd0d68c 100644 --- a/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.test.ts +++ b/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.test.ts @@ -1322,7 +1322,7 @@ describe("DesktopPairedMachineStore", () => { expect(sentTypes).toEqual(["account_challenge"]); }); - it("surfaces a host challenge decline as a route failure without leaking a hello", async () => { + it("keeps retry guidance without exposing route details from a challenge decline", async () => { // A host that declines to issue a challenge (e.g. rate-limit cooldown) is // NOT an identity-proof failure: adoption must report the host's real reason // and never send a sealed hello — but it must not be conflated with the @@ -1360,15 +1360,18 @@ describe("DesktopPairedMachineStore", () => { ws.receive(encodeSyncEnvelope({ type: "account_challenge_error", requestId: envelope.requestId, - payload: { message: "Too many failed authentication attempts. Try again in 3 minutes." }, + payload: { + message: "relay ade-tunnel-relay.arulsharma1028.workers.dev: Too many failed authentication attempts. Try again in 3 minutes.", + }, })); }) as unknown as WebSocket, }, ); // The host's real retry window is kept, without exposing the route details. await expect(pairing).rejects.toThrow( - "Could not connect to Expected host. Too many failed authentication attempts. Try again in 3 minutes.", + "Could not connect to Expected host. Try again in 3 minutes.", ); + await expect(pairing).rejects.not.toThrow(/ade-tunnel-relay|relay\.example|endpoint/i); await expect(pairing).rejects.not.toMatchObject({ code: "account_host_identity_verification_failed", }); diff --git a/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts b/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts index 1d2d200f0..28e926ab1 100644 --- a/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts +++ b/apps/desktop/src/main/services/remoteRuntime/syncPairedMachineStore.ts @@ -249,13 +249,15 @@ function accountMachineAdoptionFailureMessage( return `Could not connect to ${machineName}. ${machineName} is not signed in to the same ADE account. ` + "Open ADE on that computer, sign in, then try again."; } - const retryLaterFailure = failures.find((failure) => - /try again in\s+\d+\s+(?:second|minute|hour)/i.test(failure.reason) - ); + const retryLaterReason = failures + .map((failure) => + failure.reason.match(/\btry again in\s+\d+\s+(?:second|minute|hour)s?\b/i)?.[0], + ) + .find((reason): reason is string => reason != null); switch (dominant) { case "authentication": - if (retryLaterFailure) { - return `Could not connect to ${machineName}. ${boundedInlineText(retryLaterFailure.reason, 240)}`; + if (retryLaterReason) { + return `Could not connect to ${machineName}. ${retryLaterReason}.`; } return `Could not connect to ${machineName}. ADE could not verify the account on that computer. ` + "Open ADE there and check that it is signed in to the same ADE account, then try again."; diff --git a/apps/desktop/src/renderer/webclient/sync/__tests__/sync.test.ts b/apps/desktop/src/renderer/webclient/sync/__tests__/sync.test.ts index c8341feb6..04f508134 100644 --- a/apps/desktop/src/renderer/webclient/sync/__tests__/sync.test.ts +++ b/apps/desktop/src/renderer/webclient/sync/__tests__/sync.test.ts @@ -2645,6 +2645,58 @@ describe("browser sync connection and client", () => { }); } + for (const code of ["account_not_signed_in", "account_verification_failed"] as const) { + it(`preserves terminal account errors when a later candidate fails (${code})`, async () => { + vi.stubGlobal("location", { protocol: "http:", hostname: "example.test" }); + const storage = new MemoryStorage(); + const message = code === "account_not_signed_in" + ? "The computer you're connecting to is not signed in to an ADE account." + : "The computer you're connecting to could not verify its ADE account session."; + const environment = await makeEnvironment(storage, { + relayUrl: null, + lastGoodEndpoint: null, + explicitWssEndpoints: [], + addressCandidates: [ + { host: "192.168.1.10", kind: "lan" }, + { host: "studio.example.ts.net", kind: "tailscale" }, + ], + }); + const script = createSocketFactory((socket, envelope) => { + if (envelope.type !== "hello") return; + if (socket.url.includes("192.168.1.10")) { + socket.serverSend({ + type: "hello_error", + requestId: envelope.requestId, + payload: { + code, + message, + host: { deviceId: hostPeer.deviceId, name: hostPeer.deviceName }, + }, + }); + return; + } + socket.serverSend({ + type: "hello_error", + requestId: envelope.requestId, + payload: { + code: "auth_failed", + message: "A later route failed.", + }, + }); + }); + const connection = new SyncConnection({ socketFactory: script.factory, document: null }); + const endpoints = [ + { url: "ws://192.168.1.10:8787", kind: "candidate" as const, dialable: true }, + { url: "ws://studio.example.ts.net:8787", kind: "candidate" as const, dialable: true }, + ]; + + await expect(connection.connect(environment, endpoints)).rejects.toMatchObject({ code }); + expect(script.sockets).toHaveLength(1); + expect(connection.getStatus().error).toBe(message); + connection.dispose(); + }); + } + it("disconnects a revoked Relay lease while preserving direct reconnect trust", async () => { const storage = new MemoryStorage(); await makeEnvironment(storage, { diff --git a/apps/desktop/src/renderer/webclient/sync/connection.ts b/apps/desktop/src/renderer/webclient/sync/connection.ts index b45ae0d1e..9c5d08731 100644 --- a/apps/desktop/src/renderer/webclient/sync/connection.ts +++ b/apps/desktop/src/renderer/webclient/sync/connection.ts @@ -1382,6 +1382,10 @@ export class SyncConnection { && ( error.code === "attributed_auth_failed" || error.code === "terminal_auth_failed" + || error.code === "relay_account_required" + || error.code === "host_update_required" + || error.code === "account_not_signed_in" + || error.code === "account_verification_failed" || error.code === "invalidation_only_v1_unsupported" || error.code === "protocol_version_mismatch" ); diff --git a/apps/desktop/src/shared/types/sync.ts b/apps/desktop/src/shared/types/sync.ts index f6d797a51..d43b7d5bd 100644 --- a/apps/desktop/src/shared/types/sync.ts +++ b/apps/desktop/src/shared/types/sync.ts @@ -1110,7 +1110,9 @@ export type SyncHelloErrorPayload = * * The specific codes below it exist because `auth_failed` reads as "pair * it again" on every client, and that is the wrong instruction for a host - * that is signed out (`account_not_signed_in`), cannot verify accounts yet + * that is signed out (`account_not_signed_in`), cannot verify the account + * session (`account_verification_failed` — check sign-in and account + * configuration there), is too old to verify accounts * (`host_update_required` — update it there), or whose account session * moved under the handshake (`account_session_changed` — sign in, then * retry). None of those is a reason to destroy a saved pairing.