Fix release branch review follow-ups

Author: ndycode

lib/named-backup-export.ts

@@ -229,12 +229,12 @@ export async function exportNamedBackupFile(
 	const destination = resolveNamedBackupPath(name, storagePath);
 	const backupRoot = getNamedBackupRoot(storagePath);
 	const exportKey = normalizePathForComparison(destination);
-	if (!options?.force && inFlightNamedBackupExports.has(exportKey)) {
+	if (inFlightNamedBackupExports.has(exportKey)) {
 		throw new Error(`File already exists: ${destination}`);
 	}
+	inFlightNamedBackupExports.add(exportKey);
 	assertWithinDirectory(dirname(backupRoot), backupRoot);
 	await fs.mkdir(backupRoot, { recursive: true });
-	inFlightNamedBackupExports.add(exportKey);
 	try {
 		await dependencies.exportAccounts(
 			destination,

lib/runtime/auth-facade.ts

@@ -19,12 +19,28 @@ export async function runRuntimeOAuthFlow(
 		pluginName: string;
 	},
 ): Promise<TokenResult> {
+	const pluginPrefix = `[${deps.pluginName}]`;
+	const prefixLogMessage = (
+		message: string,
+		options?: { leadingNewline?: boolean },
+	): string => {
+		if (
+			message.startsWith(pluginPrefix) ||
+			message.startsWith(`\n${pluginPrefix}`)
+		) {
+			return message;
+		}
+		return options?.leadingNewline
+			? `\n${pluginPrefix} ${message}`
+			: `${pluginPrefix} ${message}`;
+	};
 	return deps.runBrowserOAuthFlow({
 		forceNewLogin,
 		manualModeLabel: deps.manualModeLabel,
 		logInfo: deps.logInfo,
-		logDebug: deps.logDebug,
-		logWarn: deps.logWarn,
+		logDebug: (message) => deps.logDebug(prefixLogMessage(message)),
+		logWarn: (message) =>
+			deps.logWarn(prefixLogMessage(message, { leadingNewline: true })),
 	});
 }
 

lib/storage/import-export.ts

@@ -130,13 +130,16 @@ export function mergeImportedAccounts(params: {
 	}
 
 	const deduplicatedAccounts = params.deduplicateAccounts(merged);
+	const deduplicatedExistingAccounts =
+		params.deduplicateAccounts(existingAccounts);
 	const newStorage: AccountStorageV3 = {
 		version: 3,
 		accounts: deduplicatedAccounts,
 		activeIndex: existingActiveIndex,
 		activeIndexByFamily: params.existing?.activeIndexByFamily,
 	};
-	const importedCount = deduplicatedAccounts.length - existingAccounts.length;
+	const importedCount =
+		deduplicatedAccounts.length - deduplicatedExistingAccounts.length;
 	const skippedCount = params.imported.accounts.length - importedCount;
 	return {
 		newStorage,

test/import-export.test.ts

@@ -32,6 +32,32 @@ describe("import export helpers", () => {
 		expect(result.imported).toBe(1);
 	});
 
+	it("counts imports against deduplicated existing storage", () => {
+		const result = mergeImportedAccounts({
+			existing: {
+				version: 3,
+				accounts: [{ refreshToken: "a" }, { refreshToken: "a" }],
+				activeIndex: 0,
+				activeIndexByFamily: {},
+			},
+			imported: {
+				version: 3,
+				accounts: [{ refreshToken: "b" }],
+				activeIndex: 0,
+				activeIndexByFamily: {},
+			},
+			maxAccounts: 10,
+			deduplicateAccounts: (accounts) =>
+				Array.from(
+					new Map(accounts.map((account) => [account.refreshToken, account])).values(),
+				),
+		});
+
+		expect(result.total).toBe(2);
+		expect(result.imported).toBe(1);
+		expect(result.skipped).toBe(0);
+	});
+
 	it("throws for invalid import payloads and empty exports", async () => {
 		await expect(
 			readImportFile({

test/index-retry.test.ts

@@ -324,15 +324,25 @@ vi.mock("../lib/accounts.js", async () => {
 	};
 });
 
-vi.mock("../lib/storage.js", () => ({
-	getStoragePath: () => "",
-	loadAccounts: async () => null,
-	saveAccounts: async () => {},
-	setStoragePath: () => {},
-	setStorageBackupEnabled: () => {},
-	exportAccounts: async () => {},
-	importAccounts: async () => ({ imported: 0, total: 0 }),
-}));
+vi.mock("../lib/storage.js", async () => {
+	const actual = await vi.importActual("../lib/storage.js");
+	return {
+		...actual,
+		getStoragePath: () => "",
+		loadAccounts: async () => null,
+		saveAccounts: async () => {},
+		withAccountStorageTransaction: async <T>(
+			handler: (
+				loadedStorage: null,
+				persist: (storage: unknown) => Promise<void>,
+			) => Promise<T>,
+		): Promise<T> => handler(null, async (_storage: unknown) => {}),
+		setStoragePath: () => {},
+		setStorageBackupEnabled: () => {},
+		exportAccounts: async () => {},
+		importAccounts: async () => ({ imported: 0, total: 0 }),
+	};
+});
 
 vi.mock("../lib/recovery.js", () => ({
 	createSessionRecoveryHook: () => null,

test/runtime-account-selection.test.ts

@@ -24,6 +24,15 @@ function createTokens(): TokenSuccess {
 	};
 }
 
+function createDeps(logInfo = vi.fn()) {
+	return {
+		envAccountId: process.env.CODEX_AUTH_ACCOUNT_ID,
+		logInfo,
+		getAccountIdCandidates,
+		selectBestAccountCandidate,
+	};
+}
+
 describe("resolveAccountSelection", () => {
 	beforeEach(() => {
 		vi.clearAllMocks();
@@ -39,7 +48,7 @@ describe("resolveAccountSelection", () => {
 		const tokens = createTokens();
 		const logInfo = vi.fn();
 
-		const result = resolveAccountSelection(tokens, { logInfo });
+		const result = resolveAccountSelection(tokens, createDeps(logInfo));
 
 		expect(result).toEqual({
 			...tokens,
@@ -58,7 +67,7 @@ describe("resolveAccountSelection", () => {
 		vi.mocked(getAccountIdCandidates).mockReturnValueOnce([]);
 		const tokens = createTokens();
 
-		const result = resolveAccountSelection(tokens, { logInfo: vi.fn() });
+		const result = resolveAccountSelection(tokens, createDeps());
 
 		expect(result).toBe(tokens);
 		expect(selectBestAccountCandidate).not.toHaveBeenCalled();
@@ -75,7 +84,7 @@ describe("resolveAccountSelection", () => {
 		]);
 		const tokens = createTokens();
 
-		const result = resolveAccountSelection(tokens, { logInfo: vi.fn() });
+		const result = resolveAccountSelection(tokens, createDeps());
 
 		expect(result).toEqual({
 			...tokens,
@@ -112,7 +121,7 @@ describe("resolveAccountSelection", () => {
 		vi.mocked(selectBestAccountCandidate).mockReturnValueOnce(candidates[1]);
 		const tokens = createTokens();
 
-		const result = resolveAccountSelection(tokens, { logInfo: vi.fn() });
+		const result = resolveAccountSelection(tokens, createDeps());
 
 		expect(selectBestAccountCandidate).toHaveBeenCalledWith(candidates);
 		expect(result).toEqual({
@@ -154,7 +163,7 @@ describe("resolveAccountSelection", () => {
 		vi.mocked(selectBestAccountCandidate).mockReturnValueOnce(null);
 		const tokens = createTokens();
 
-		const result = resolveAccountSelection(tokens, { logInfo: vi.fn() });
+		const result = resolveAccountSelection(tokens, createDeps());
 
 		expect(selectBestAccountCandidate).toHaveBeenCalledWith(candidates);
 		expect(result).toBe(tokens);