[WRONG BRANCH] oauth: remove legacy auth backup on destructive mutations (logout/account delete)

src/oauth/store.ts

@@ -4,8 +4,10 @@
  * Multiauth shape (260706): each provider value is a ProviderAccountSet
  * `{ activeAccountId, accounts: [{ id, credential, needsReauth?, addedAt? }] }`.
  * Legacy single-credential values (`{ access, refresh, expires, ... }`) normalize on load,
- * and the first new-shape persist writes a one-time `auth.json.pre-multiauth` backup so a
- * downgraded loader (which silently drops unknown shapes) cannot destroy refresh tokens.
+ * and the first non-destructive new-shape persist writes a one-time
+ * `auth.json.pre-multiauth` backup so a downgraded loader (which silently drops unknown
+ * shapes) cannot destroy refresh tokens. Destructive mutations remove that backup so
+ * logout and account deletion do not retain the deleted credentials.
  *
  * Exceptions:
  * - `chatgpt` stays single-slot (always replaced): codex-auth-api uses it as a scratch slot
@@ -235,6 +237,14 @@ function backupLegacyOnce(): void {
   } catch { /* best-effort */ }
 }
 
+function removeLegacyBackup(): void {
+  try {
+    unlinkSync(`${getAuthStorePath()}.pre-multiauth`);
+  } catch (error) {
+    if (errorCode(error) !== "ENOENT") throw error;
+  }
+}
+
 function isCredentialSource(value: unknown): value is OAuthCredentialSource {
   return value === "oauth" || value === "local-cli" || value === "credential-file" || value === "environment" || value === "manual";
 }
@@ -450,11 +460,12 @@ function serializeMutation<T>(work: () => Promise<T>, retainedValues: readonly u
   drainOAuthMutations();
   return result;
 }
-export function mutateStore<T>(fn:(store:AuthStore)=>T|Promise<T>, retainedValues: readonly unknown[] = [], options?: { waitMs?: number }):Promise<T>{return serializeMutation(async()=>{const guard=await createOAuthFileLock({path:getAuthStoreLockPath(),staleAfterMs:30000}).acquire();try{
+export function mutateStore<T>(fn:(store:AuthStore)=>T|Promise<T>, retainedValues: readonly unknown[] = [], options?: { waitMs?: number; removeLegacyBackup?: boolean }):Promise<T>{return serializeMutation(async()=>{const guard=await createOAuthFileLock({path:getAuthStoreLockPath(),staleAfterMs:30000}).acquire();try{
     const { store, hadLegacy } = loadAuthStoreInternal();
-    if (hadLegacy) backupLegacyOnce();
+    if (hadLegacy && !options?.removeLegacyBackup) backupLegacyOnce();
     const result = await fn(store);
     persist(store);
+    if (options?.removeLegacyBackup) removeLegacyBackup();
     return result;
   }finally{guard.release();}}, retainedValues, options?.waitMs);
 }
@@ -534,7 +545,7 @@ export async function removeCredential(provider: string): Promise<void> {
       return;
     }
     set.activeAccountId = set.accounts[0]!.id;
-  }, [provider]);
+  }, [provider], { removeLegacyBackup: true });
 }
 
 // ---------------------------------------------------------------------------
@@ -609,7 +620,7 @@ export async function removeAccount(provider: string, accountId: string): Promis
     }
     if (set.activeAccountId === accountId) set.activeAccountId = set.accounts[0]!.id;
     return true;
-  }, [provider, accountId]);
+  }, [provider, accountId], { removeLegacyBackup: true });
   return removed;
 }
 

tests/oauth-store-multi.test.ts

@@ -67,14 +67,41 @@ describe("multi-account auth store", () => {
       xai: { access: "legacy-access", refresh: "legacy-refresh", expires: Date.now() + 1000, email: "old@example.com" },
     }));
     expect(getCredential("xai")?.access).toBe("legacy-access");
-    // Any mutation persists the new shape + writes the downgrade backup.
+    // A non-destructive mutation persists the new shape + writes the downgrade backup.
     await saveCredential("xai", cred({ email: "old@example.com", access: "new-access" }));
     expect(getCredential("xai")?.access).toBe("new-access");
     const raw = JSON.parse(readFileSync(authPath, "utf-8"));
     expect(Array.isArray(raw.xai.accounts)).toBe(true);
     expect(existsSync(`${authPath}.pre-multiauth`)).toBe(true);
   });
 
+  test("logout migrates a legacy store without retaining its credential backup", async () => {
+    const authPath = join(TEST_DIR, "auth.json");
+    writeFileSync(authPath, JSON.stringify({
+      xai: { access: "legacy-access", refresh: "legacy-refresh", expires: Date.now() + 1000 },
+    }));
+
+    await removeCredential("xai");
+
+    expect(JSON.parse(readFileSync(authPath, "utf-8"))).toEqual({});
+    expect(existsSync(`${authPath}.pre-multiauth`)).toBe(false);
+  });
+
+  test("account deletion removes an existing legacy credential backup", async () => {
+    const authPath = join(TEST_DIR, "auth.json");
+    const legacy = {
+      xai: { access: "legacy-access", refresh: "legacy-refresh", expires: Date.now() + 1000 },
+    };
+    writeFileSync(authPath, JSON.stringify(legacy));
+    writeFileSync(`${authPath}.pre-multiauth`, JSON.stringify(legacy));
+    const accountId = getAccountSet("xai")!.activeAccountId;
+
+    expect(await removeAccount("xai", accountId)).toBe(true);
+
+    expect(JSON.parse(readFileSync(authPath, "utf-8"))).toEqual({});
+    expect(existsSync(`${authPath}.pre-multiauth`)).toBe(false);
+  });
+
   test("legacy credential WITHOUT identity gets a deterministic account id across loads", async () => {
     // Legacy stores are re-normalized on EVERY load without being persisted, so the
     // derived id must be stable: a time-salted id would make getAccountSet and