refactor(core): share the oauth state failure union and fold callback narrowing

packages/core/src/analytics/oauth-callback.analytics.test.ts

@@ -55,4 +55,63 @@ describe("oauth-callback.analytics", () => {
       provider_error: "exchangeFailed",
     });
   });
+
+  it("omits provider_error when an exchange failure carries no reason", () => {
+    expect(
+      oauthCallbackFailureReason({
+        outcome: "error",
+        state: "signed-state",
+        code: "auth-code",
+        oauthError: undefined,
+        exchangeError: Object.assign(new Error("exchange failed"), {
+          name: "ProviderAuthError",
+        }),
+      }),
+    ).toEqual({ reason: "token_exchange_error" });
+  });
+
+  it("ignores a non-provider error on the exchange path", () => {
+    expect(
+      oauthCallbackFailureReason({
+        outcome: "error",
+        state: "signed-state",
+        code: "auth-code",
+        oauthError: undefined,
+        exchangeError: new TypeError("unrelated"),
+      }),
+    ).toEqual({ reason: "other" });
+  });
+
+  it("treats a blank state or code as absent", () => {
+    expect(
+      oauthCallbackFailureReason({
+        outcome: "error",
+        state: "",
+        code: "auth-code",
+        oauthError: undefined,
+      }),
+    ).toEqual({ reason: "invalid_state" });
+    expect(
+      oauthCallbackFailureReason({
+        outcome: "error",
+        state: "signed-state",
+        code: "",
+        oauthError: undefined,
+      }),
+    ).toEqual({ reason: "missing_code" });
+  });
+
+  it("maps a state verification failure to invalid_state", () => {
+    for (const stateVerifyReason of ["invalidSignature", "expired"] as const) {
+      expect(
+        oauthCallbackFailureReason({
+          outcome: "error",
+          state: "signed-state",
+          code: "auth-code",
+          oauthError: undefined,
+          stateVerifyReason,
+        }),
+      ).toEqual({ reason: "invalid_state" });
+    }
+  });
 });

packages/core/src/analytics/oauth-callback.analytics.ts

@@ -2,6 +2,7 @@ import {
   type OAuthFailedReason,
   oauthFailedReasonFromCallbackQuery,
 } from "@core/analytics/oauth-signup.events";
+import { type OAuthStateFailure } from "@core/types/sync/connection.contracts";
 
 export type SyncOAuthCallbackOutcome =
   | "connected"
@@ -42,32 +43,28 @@ export function oauthCallbackFailureReason(input: {
   code: unknown;
   oauthError: unknown;
   oauthErrorDescription?: unknown;
-  stateVerifyReason?:
-    | "malformed"
-    | "invalidSignature"
-    | "expired"
-    | "stateMismatch"
-    | undefined;
+  stateVerifyReason?: OAuthStateFailure | undefined;
   exchangeError?: unknown;
 }): { reason: OAuthFailedReason; provider_error?: string } | undefined {
   if (input.outcome === "connected") return undefined;
 
-  const providerErrorRaw =
-    typeof input.oauthError === "string" ? input.oauthError : undefined;
+  // Callback query values reach here as `unknown` straight off the request, so
+  // each one is narrowed once up front rather than at every branch that reads it.
+  const state = asNonEmptyString(input.state);
+  const code = asNonEmptyString(input.code);
+  const providerErrorRaw = asNonEmptyString(input.oauthError);
   const providerError = sanitizeOAuthProviderError(providerErrorRaw);
   const providerDescription = sanitizeOAuthProviderError(
-    typeof input.oauthErrorDescription === "string"
-      ? input.oauthErrorDescription
-      : undefined,
+    asNonEmptyString(input.oauthErrorDescription),
   );
   const mergedProviderError = providerError ?? providerDescription;
 
   if (input.outcome === "declined") {
     const reason =
       oauthFailedReasonFromCallbackQuery({
-        state: typeof input.state === "string" ? input.state : null,
-        code: typeof input.code === "string" ? input.code : null,
-        providerError: providerErrorRaw ?? null,
+        state,
+        code,
+        providerError: providerErrorRaw,
       }) ?? "other";
     return withProviderError(reason, providerError);
   }
@@ -77,13 +74,10 @@ export function oauthCallbackFailureReason(input: {
   }
 
   if (input.outcome === "error") {
-    const hasState = typeof input.state === "string" && input.state.length > 0;
-    const hasCode = typeof input.code === "string" && input.code.length > 0;
-
-    if (!hasState || input.stateVerifyReason === "malformed") {
+    if (!state || input.stateVerifyReason === "malformed") {
       return withProviderError("invalid_state", providerError);
     }
-    if (!hasCode) {
+    if (!code) {
       return withProviderError("missing_code", providerError);
     }
     if (
@@ -92,11 +86,9 @@ export function oauthCallbackFailureReason(input: {
     ) {
       return { reason: "invalid_state" };
     }
-    if (isTokenExchangeFailure(input.exchangeError)) {
-      return {
-        reason: "token_exchange_error",
-        provider_error: providerErrorFromExchange(input.exchangeError),
-      };
+    const exchange = tokenExchangeFailure(input.exchangeError);
+    if (exchange) {
+      return withProviderError("token_exchange_error", exchange.providerError);
     }
     return withProviderError("other", mergedProviderError);
   }
@@ -114,17 +106,22 @@ function withProviderError(
   };
 }
 
-function isTokenExchangeFailure(error: unknown): boolean {
-  if (!error || typeof error !== "object") return false;
-  const name = (error as { name?: unknown }).name;
-  return name === "ProviderAuthError";
+/** Blank query values mean "absent", the same way a missing param does. */
+function asNonEmptyString(value: unknown): string | undefined {
+  return typeof value === "string" && value.length > 0 ? value : undefined;
 }
 
-function providerErrorFromExchange(error: unknown): string | undefined {
-  if (!error || typeof error !== "object") return undefined;
-  const reason = (error as { reason?: unknown }).reason;
-  if (typeof reason === "string" && reason.length > 0) {
-    return sanitizeOAuthProviderError(reason);
-  }
-  return undefined;
+/**
+ * A provider token-exchange failure, matched structurally because core cannot
+ * import sync's `ProviderAuthError` without inverting the package dependency.
+ * Returns its sanitized reason when it matches, else null.
+ */
+function tokenExchangeFailure(
+  error: unknown,
+): { providerError?: string } | null {
+  if (!error || typeof error !== "object") return null;
+  const { name, reason } = error as { name?: unknown; reason?: unknown };
+  if (name !== "ProviderAuthError") return null;
+  const providerError = sanitizeOAuthProviderError(asNonEmptyString(reason));
+  return { ...(providerError ? { providerError } : {}) };
 }

packages/core/src/types/sync/connection.contracts.ts

@@ -71,6 +71,18 @@ export type ConnectionBeginFeatures = z.infer<
   typeof ConnectionBeginFeaturesSchema
 >;
 
+// Why a signed OAuth state token failed verification on the public callback.
+// Sync mints the state at `begin` and verifies the echo; callback analytics map
+// these onto funnel reasons. Shared here so the verifier and the analytics
+// mapper cannot drift apart.
+export const OAuthStateFailureSchema = z.enum([
+  "malformed",
+  "invalidSignature",
+  "expired",
+  "stateMismatch",
+]);
+export type OAuthStateFailure = z.infer<typeof OAuthStateFailureSchema>;
+
 // Opaque authenticated-encryption envelope for credentials sent between the
 // Compass API and Sync services. The browser never sees this transport type.
 export const EncryptedCredentialEnvelopeSchema = z.strictObject({

packages/sync/src/oauth/oauth-state.ts

@@ -1,3 +1,4 @@
+import { type OAuthStateFailure } from "@core/types/sync/connection.contracts";
 import {
   type ConnectionId,
   ConnectionIdSchema,
@@ -56,12 +57,6 @@ export function signOAuthState(
   return `${encoded}.${sign(secret, encoded)}`;
 }
 
-export type OAuthStateFailure =
-  | "malformed"
-  | "invalidSignature"
-  | "expired"
-  | "stateMismatch";
-
 export type OAuthStateResult =
   | { readonly ok: true; readonly payload: OAuthStatePayload }
   | { readonly ok: false; readonly reason: OAuthStateFailure };

packages/sync/src/server/connection.routes.ts

@@ -32,6 +32,7 @@ import {
   ConnectionRefreshResponseSchema,
   ForegroundRefreshRequestSchema,
   GoogleConnectionAdoptionRequestSchema,
+  type OAuthStateFailure,
   type ProviderCalendar,
   ProviderCalendarSchema,
   type ProviderConnection,
@@ -72,7 +73,6 @@ import {
   HORIZON_PAST_MONTHS,
 } from "@sync/domain/horizon";
 import {
-  type OAuthStateFailure,
   peekOAuthStatePayload,
   signOAuthState,
   verifyOAuthState,