Skip to content

Commit eaaaee3

Browse files
committed
Fix remaining Windows sync review issues
1 parent e5b5b36 commit eaaaee3

File tree

4 files changed

+103
-7
lines changed

4 files changed

+103
-7
lines changed

index.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,6 @@ import {
127127
StorageError,
128128
formatStorageErrorHint,
129129
withAccountAndFlaggedStorageTransaction,
130-
withFlaggedAccountsTransaction,
131130
type AccountStorageV3,
132131
type FlaggedAccountMetadataV1,
133132
} from "./lib/storage.js";
@@ -3428,7 +3427,7 @@ while (attempted.size < Math.max(1, accountCount)) {
34283427
return;
34293428
} catch (error) {
34303429
const code = (error as NodeJS.ErrnoException).code;
3431-
if ((code === "EPERM" || code === "EBUSY")) {
3430+
if ((code === "EPERM" || code === "EBUSY" || code === "EACCES")) {
34323431
lastError = error as NodeJS.ErrnoException;
34333432
const delayMs = SYNC_PRUNE_BACKUP_RENAME_RETRY_DELAYS_MS[attempt];
34343433
if (delayMs !== undefined) {

lib/codex-multi-auth-sync.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ interface PreparedCodexMultiAuthPreviewStorage {
122122
const TEMP_CLEANUP_RETRY_DELAYS_MS = [100, 250, 500] as const;
123123
const STALE_TEMP_CLEANUP_RETRY_DELAY_MS = 150;
124124
const TEMP_CLEANUP_RETRYABLE_CODES = new Set(["EBUSY", "EAGAIN", "EACCES", "EPERM", "ENOTEMPTY"]);
125+
const BACKUP_RENAME_RETRY_DELAYS_MS = [100, 250, 500] as const;
126+
const BACKUP_RENAME_RETRYABLE_CODES = new Set(["EBUSY", "EACCES", "EPERM"]);
125127

126128
function sleepAsync(ms: number): Promise<void> {
127129
return new Promise((resolve) => setTimeout(resolve, ms));
@@ -257,6 +259,37 @@ async function scrubStaleNormalizedImportTempFile(candidateDir: string): Promise
257259
if (code === "ENOENT" || code === "EACCES" || code === "EPERM" || code === "EBUSY" || code === "EAGAIN") {
258260
return;
259261
}
262+
logWarn(
263+
`Failed to scrub stale codex sync temp file ${tempPath}: ${
264+
error instanceof Error ? error.message : String(error)
265+
}`,
266+
);
267+
}
268+
}
269+
270+
async function renameOverlapCleanupBackupWithRetry(sourcePath: string, destinationPath: string): Promise<void> {
271+
let lastError: NodeJS.ErrnoException | null = null;
272+
273+
for (let attempt = 0; attempt < BACKUP_RENAME_RETRY_DELAYS_MS.length; attempt += 1) {
274+
try {
275+
await fs.rename(sourcePath, destinationPath);
276+
return;
277+
} catch (error) {
278+
const code = (error as NodeJS.ErrnoException).code;
279+
if (code && BACKUP_RENAME_RETRYABLE_CODES.has(code)) {
280+
lastError = error as NodeJS.ErrnoException;
281+
const delayMs = BACKUP_RENAME_RETRY_DELAYS_MS[attempt];
282+
if (delayMs !== undefined) {
283+
await sleepAsync(delayMs);
284+
continue;
285+
}
286+
}
287+
throw error;
288+
}
289+
}
290+
291+
if (lastError) {
292+
throw lastError;
260293
}
261294
}
262295

@@ -1245,7 +1278,7 @@ export async function cleanupCodexMultiAuthSyncedOverlaps(
12451278
mode: 0o600,
12461279
flag: "wx",
12471280
});
1248-
await fs.rename(tempBackupPath, backupPath);
1281+
await renameOverlapCleanupBackupWithRetry(tempBackupPath, backupPath);
12491282
writtenBackupPath = backupPath;
12501283
} catch (error) {
12511284
try {

test/codex-multi-auth-sync.test.ts

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,6 +1080,66 @@ describe("codex-multi-auth sync", () => {
10801080
}
10811081
});
10821082

1083+
it("warns when stale sync temp scrubbing hits an unexpected truncate error", async () => {
1084+
const rootDir = join(process.cwd(), ".tmp-codex-multi-auth");
1085+
const fakeHome = await fs.promises.mkdtemp(join(os.tmpdir(), "codex-sync-home-"));
1086+
process.env.CODEX_MULTI_AUTH_DIR = rootDir;
1087+
process.env.HOME = fakeHome;
1088+
process.env.USERPROFILE = fakeHome;
1089+
const globalPath = join(rootDir, "openai-codex-accounts.json");
1090+
const tempRoot = join(fakeHome, ".opencode", "tmp");
1091+
const staleDir = join(tempRoot, "oc-chatgpt-multi-auth-sync-stale-scrub-warning");
1092+
const staleFile = join(staleDir, "accounts.json");
1093+
mockExistsSync.mockImplementation((candidate) => String(candidate) === globalPath);
1094+
mockSourceStorageFile(
1095+
globalPath,
1096+
JSON.stringify({
1097+
version: 3,
1098+
activeIndex: 0,
1099+
activeIndexByFamily: {},
1100+
accounts: [{ accountId: "org-source", organizationId: "org-source", refreshToken: "rt-source", addedAt: 1, lastUsed: 1 }],
1101+
}),
1102+
);
1103+
1104+
const originalRm = fs.promises.rm.bind(fs.promises);
1105+
const rmSpy = vi.spyOn(fs.promises, "rm").mockImplementation(async (path, options) => {
1106+
if (String(path) === staleDir) {
1107+
throw Object.assign(new Error("still locked"), { code: "EBUSY" });
1108+
}
1109+
return originalRm(path, options as never);
1110+
});
1111+
const truncateSpy = vi.spyOn(fs.promises, "truncate").mockImplementation(async (path, len) => {
1112+
if (String(path) === staleFile) {
1113+
throw Object.assign(new Error("disk io failed"), { code: "EIO" });
1114+
}
1115+
return undefined;
1116+
});
1117+
const loggerModule = await import("../lib/logger.js");
1118+
1119+
try {
1120+
await fs.promises.mkdir(staleDir, { recursive: true });
1121+
await fs.promises.writeFile(staleFile, "sensitive-refresh-token", "utf8");
1122+
const oldTime = new Date(Date.now() - (15 * 60 * 1000));
1123+
await fs.promises.utimes(staleDir, oldTime, oldTime);
1124+
await fs.promises.utimes(staleFile, oldTime, oldTime);
1125+
1126+
const { previewSyncFromCodexMultiAuth } = await import("../lib/codex-multi-auth-sync.js");
1127+
await expect(previewSyncFromCodexMultiAuth(process.cwd())).resolves.toMatchObject({
1128+
rootDir,
1129+
accountsPath: globalPath,
1130+
scope: "global",
1131+
});
1132+
1133+
expect(vi.mocked(loggerModule.logWarn)).toHaveBeenCalledWith(
1134+
expect.stringContaining(`Failed to scrub stale codex sync temp file ${staleFile}: disk io failed`),
1135+
);
1136+
} finally {
1137+
rmSpy.mockRestore();
1138+
truncateSpy.mockRestore();
1139+
await fs.promises.rm(fakeHome, { recursive: true, force: true });
1140+
}
1141+
});
1142+
10831143
it("skips source accounts whose emails already exist locally during sync", async () => {
10841144
const rootDir = join(process.cwd(), ".tmp-codex-multi-auth");
10851145
process.env.CODEX_MULTI_AUTH_DIR = rootDir;
@@ -1657,14 +1717,17 @@ describe("codex-multi-auth sync", () => {
16571717
expect(persist).not.toHaveBeenCalled();
16581718
});
16591719

1660-
it("writes overlap cleanup backups via a temp file before rename", async () => {
1720+
it("writes overlap cleanup backups via a temp file and retries transient EACCES renames", async () => {
16611721
const storageModule = await import("../lib/storage.js");
16621722
vi.mocked(storageModule.withAccountStorageTransaction).mockImplementationOnce(async (handler) =>
16631723
handler(defaultTransactionalStorage(), vi.fn(async () => {})),
16641724
);
16651725
const mkdirSpy = vi.spyOn(fs.promises, "mkdir").mockResolvedValue(undefined);
16661726
const writeSpy = vi.spyOn(fs.promises, "writeFile").mockResolvedValue(undefined);
1667-
const renameSpy = vi.spyOn(fs.promises, "rename").mockResolvedValue(undefined);
1727+
const renameSpy = vi
1728+
.spyOn(fs.promises, "rename")
1729+
.mockRejectedValueOnce(Object.assign(new Error("rename locked"), { code: "EACCES" }))
1730+
.mockResolvedValueOnce(undefined);
16681731
const unlinkSpy = vi.spyOn(fs.promises, "unlink").mockResolvedValue(undefined);
16691732

16701733
try {
@@ -1675,6 +1738,7 @@ describe("codex-multi-auth sync", () => {
16751738
const tempBackupPath = writeSpy.mock.calls[0]?.[0];
16761739
expect(String(tempBackupPath)).toMatch(/^\/tmp\/overlap-cleanup-backup\.json\.\d+\.[a-z0-9]+\.tmp$/);
16771740
expect(renameSpy).toHaveBeenCalledWith(tempBackupPath, "/tmp/overlap-cleanup-backup.json");
1741+
expect(renameSpy).toHaveBeenCalledTimes(2);
16781742
expect(unlinkSpy).not.toHaveBeenCalled();
16791743
} finally {
16801744
mkdirSpy.mockRestore();

test/index.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2116,7 +2116,7 @@ describe("OpenAIOAuthPlugin", () => {
21162116
});
21172117

21182118
describe("sync maintenance flows", () => {
2119-
it("shows overlap cleanup backup hints only when cleanup reports a written backup", async () => {
2119+
it("shows the locally tracked overlap cleanup backup path when cleanup fails", async () => {
21202120
const cliModule = await import("../lib/cli.js");
21212121
const confirmModule = await import("../lib/ui/confirm.js");
21222122
const syncModule = await import("../lib/codex-multi-auth-sync.js");
@@ -2341,7 +2341,7 @@ describe("OpenAIOAuthPlugin", () => {
23412341
const writeSpy = vi.spyOn(nodeFsPromises, "writeFile").mockResolvedValue(undefined);
23422342
const renameSpy = vi
23432343
.spyOn(nodeFsPromises, "rename")
2344-
.mockRejectedValueOnce(Object.assign(new Error("rename locked"), { code: "EPERM" }))
2344+
.mockRejectedValueOnce(Object.assign(new Error("rename locked"), { code: "EACCES" }))
23452345
.mockResolvedValueOnce(undefined);
23462346
const normalizePath = (value: string) => value.replace(/\\/g, "/");
23472347
const now = Date.now();

0 commit comments

Comments
 (0)