fix(oauth): honor connectionId on token refresh so email-less providers don't duplicate (#8062)

persistOAuthConnection gated its whole dedup step behind if(tokenData.email).
The matcher (findExistingOAuthConnectionMatch) already matches by explicit
connectionId first, but it was never reached when the payload had no top-level
email. GitHub Copilot's device-code flow keeps identity under
providerSpecificData.githubEmail, so tokenData.email is undefined — a refresh
(which passes the existing connectionId) skipped the match and fell through to
createProviderConnection, producing a duplicate connection.

- Widen the gate to if(connectionId || tokenData.email) so an explicit
  connectionId is honored regardless of email.
- Guard the matcher's email branch with if(!tokenData.email) return false, so a
  widened gate can't false-match an email-less connection via
  safeEqual(undefined, undefined).

Fixes #8059.

changelog.d/fixes/8059-oauth-refresh-connection-dedup.md

@@ -0,0 +1 @@
+- **fix(oauth):** Refreshing the token on an email-less OAuth connection (e.g. GitHub Copilot, whose identity lives under `providerSpecificData`) no longer creates a duplicate connection. `persistOAuthConnection` gated its whole dedup behind `if (tokenData.email)`, so the explicit `connectionId` passed on refresh was discarded and a new connection was created every time; the connectionId is now honored regardless of email, and email dedup only applies when a non-empty email is present. ([#8059](https://github.com/diegosouzapw/OmniRoute/issues/8059))

src/lib/oauth/connectionPersistence.ts

@@ -68,6 +68,10 @@ export function findExistingOAuthConnectionMatch(
 ): Record<string, any> | undefined {
   return existing.find((c) => {
     if (c.id && safeEqual(connectionId, c.id)) return true;
+    // Email dedup only when the payload actually carries an email. Without this
+    // guard `safeEqual(undefined, undefined)` is true, so an email-less payload
+    // would false-match the first email-less connection of the provider.
+    if (!tokenData.email) return false;
     if (!safeEqual(c.email, tokenData.email) || c.authType !== "oauth") return false;
     if (provider === "codex") {
       return isSameCodexAccount(c.providerSpecificData, tokenData.providerSpecificData);
@@ -130,7 +134,12 @@ export async function persistOAuthConnection(
     : null;
 
   let connection: any;
-  if (tokenData.email) {
+  // A connectionId is an explicit "update THIS connection" signal (token refresh
+  // / re-auth of a known connection); honor it even when the payload has no
+  // top-level email. Some providers (e.g. GitHub Copilot) keep identity under
+  // providerSpecificData, so gating dedup on tokenData.email alone created a
+  // duplicate connection on every refresh (#8059).
+  if (connectionId || tokenData.email) {
     const existing = await getProviderConnections({ provider });
     const match = findExistingOAuthConnectionMatch(existing, provider, tokenData, connectionId);
     const matchId = typeof match?.id === "string" ? match.id : null;

tests/unit/oauth-refresh-connection-dedup-8059.test.ts

@@ -0,0 +1,90 @@
+/**
+ * #8059 — token refresh must not duplicate an email-less OAuth connection.
+ *
+ * persistOAuthConnection gated its whole dedup step behind `if (tokenData.email)`.
+ * GitHub Copilot's device-code flow keeps identity under
+ * providerSpecificData.githubEmail, so the top-level `tokenData.email` is
+ * undefined — a refresh (which passes the existing connectionId) skipped the
+ * match entirely and fell through to createProviderConnection, producing a
+ * duplicate. The fix honors an explicit connectionId regardless of email, and
+ * only applies email dedup when a non-empty email is present.
+ */
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-oauth-refresh-dedup-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "oauth-refresh-dedup-test-secret";
+
+const core = await import("../../src/lib/db/core.ts");
+const providersDb = await import("../../src/lib/db/providers.ts");
+const { persistOAuthConnection, findExistingOAuthConnectionMatch } = await import(
+  "../../src/lib/oauth/connectionPersistence.ts"
+);
+
+async function resetStorage() {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+}
+
+test.beforeEach(resetStorage);
+test.after(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+async function oauthConns(provider: string) {
+  const all = await providersDb.getProviderConnections({});
+  return all.filter((c: any) => c.provider === provider && c.authType === "oauth");
+}
+
+// GitHub Copilot: no top-level email, identity under providerSpecificData.
+const copilotToken = (refresh: string) => ({
+  refreshToken: refresh,
+  providerSpecificData: {
+    copilotToken: `cop_${refresh}`,
+    githubLogin: "octocat",
+    githubName: "The Octocat",
+    githubEmail: "octocat@github.com",
+  },
+});
+
+test("token refresh with matching connectionId updates the connection, no duplicate (#8059)", async () => {
+  const created = await persistOAuthConnection("github", copilotToken("r1"));
+  assert.equal((await oauthConns("github")).length, 1, "initial connect creates one connection");
+
+  // Refresh: same connection, new token, still no top-level email.
+  const refreshed = await persistOAuthConnection("github", copilotToken("r2"), created.id);
+
+  const conns = await oauthConns("github");
+  assert.equal(conns.length, 1, "refresh must not create a duplicate");
+  assert.equal(refreshed.id, created.id, "refresh updates the existing connection");
+  assert.equal(conns[0].refreshToken, "r2", "the token was actually updated");
+});
+
+test("findExistingOAuthConnectionMatch: connectionId wins even without an email", () => {
+  const existing = [
+    { id: "conn-1", provider: "github", authType: "oauth", email: null },
+    { id: "conn-2", provider: "github", authType: "oauth", email: null },
+  ];
+  const match = findExistingOAuthConnectionMatch(existing, "github", copilotToken("x"), "conn-2");
+  assert.equal(match?.id, "conn-2");
+});
+
+test("findExistingOAuthConnectionMatch: an email-less payload does not false-match an email-less connection", () => {
+  const existing = [{ id: "conn-1", provider: "github", authType: "oauth", email: null }];
+  // No connectionId, no top-level email -> must not match (would clobber a
+  // different account whose email is likewise absent).
+  const match = findExistingOAuthConnectionMatch(existing, "github", copilotToken("x"));
+  assert.equal(match, undefined);
+});
+
+test("findExistingOAuthConnectionMatch: email dedup still works when an email is present", () => {
+  const existing = [{ id: "conn-1", provider: "claude", authType: "oauth", email: "a@b.com" }];
+  const match = findExistingOAuthConnectionMatch(existing, "claude", { email: "a@b.com" });
+  assert.equal(match?.id, "conn-1");
+});