Skip to content

Commit 1c5f127

Browse files
ndycodendycode
committed
fix: clean up sync prune backups
1 parent ba92bd9 commit 1c5f127

3 files changed

Lines changed: 213 additions & 9 deletions

File tree

‎index.ts‎

Lines changed: 93 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
import { tool } from "@opencode-ai/plugin/tool";
2727
import { promises as fsPromises } from "node:fs";
28-
import { dirname } from "node:path";
28+
import { dirname, join } from "node:path";
2929
import type { Plugin, PluginInput } from "@opencode-ai/plugin";
3030
import type { Auth } from "@opencode-ai/sdk";
3131
import {
@@ -3405,9 +3405,55 @@ while (attempted.size < Math.max(1, accountCount)) {
34053405
}
34063406
};
34073407

3408+
const SYNC_PRUNE_BACKUP_PREFIX = "codex-sync-prune-backup";
3409+
const SYNC_PRUNE_BACKUP_RETAIN_COUNT = 2;
3410+
3411+
const pruneOldSyncPruneBackups = async (
3412+
backupDir: string,
3413+
keepPaths: string[] = [],
3414+
): Promise<void> => {
3415+
const entries = await fsPromises.readdir(backupDir, { withFileTypes: true }).catch((error) => {
3416+
const code = (error as NodeJS.ErrnoException).code;
3417+
if (code === "ENOENT") {
3418+
return null;
3419+
}
3420+
throw error;
3421+
});
3422+
if (!entries) return;
3423+
3424+
const keepSet = new Set(keepPaths);
3425+
const staleBackupPaths = entries
3426+
.filter(
3427+
(entry) =>
3428+
entry.isFile() &&
3429+
entry.name.startsWith(`${SYNC_PRUNE_BACKUP_PREFIX}-`) &&
3430+
entry.name.endsWith(".json"),
3431+
)
3432+
.map((entry) => join(backupDir, entry.name))
3433+
.filter((candidatePath) => !keepSet.has(candidatePath))
3434+
.sort((left, right) => right.localeCompare(left))
3435+
.slice(SYNC_PRUNE_BACKUP_RETAIN_COUNT);
3436+
3437+
for (const staleBackupPath of staleBackupPaths) {
3438+
try {
3439+
await fsPromises.unlink(staleBackupPath);
3440+
} catch (error) {
3441+
const code = (error as NodeJS.ErrnoException).code;
3442+
if (code === "ENOENT") {
3443+
continue;
3444+
}
3445+
const message = error instanceof Error ? error.message : String(error);
3446+
logWarn(
3447+
`[${PLUGIN_NAME}] Failed to prune stale sync prune backup ${staleBackupPath}: ${message}`,
3448+
);
3449+
}
3450+
}
3451+
};
3452+
34083453
const createSyncPruneBackup = async (): Promise<{
34093454
backupPath: string;
34103455
restore: () => Promise<void>;
3456+
cleanup: () => Promise<void>;
34113457
}> => {
34123458
const { accounts: loadedAccountsStorage, flagged: currentFlaggedStorage } =
34133459
await loadAccountAndFlaggedStorageSnapshot();
@@ -3419,8 +3465,9 @@ while (attempted.size < Math.max(1, accountCount)) {
34193465
activeIndex: 0,
34203466
activeIndexByFamily: {},
34213467
} satisfies AccountStorageV3);
3422-
const backupPath = createTimestampedBackupPath("codex-sync-prune-backup");
3423-
await fsPromises.mkdir(dirname(backupPath), { recursive: true });
3468+
const backupPath = createTimestampedBackupPath(SYNC_PRUNE_BACKUP_PREFIX);
3469+
const backupDir = dirname(backupPath);
3470+
await fsPromises.mkdir(backupDir, { recursive: true });
34243471
const backupPayload = createSyncPruneBackupPayload(
34253472
currentAccountsStorage,
34263473
currentFlaggedStorage,
@@ -3434,6 +3481,13 @@ while (attempted.size < Math.max(1, accountCount)) {
34343481
mode: 0o600,
34353482
});
34363483
await fsPromises.rename(tempBackupPath, backupPath);
3484+
await pruneOldSyncPruneBackups(backupDir, [backupPath]).catch((cleanupError) => {
3485+
const cleanupMessage =
3486+
cleanupError instanceof Error ? cleanupError.message : String(cleanupError);
3487+
logWarn(
3488+
`[${PLUGIN_NAME}] Failed to prune old sync prune backups in ${backupDir}: ${cleanupMessage}`,
3489+
);
3490+
});
34373491
} catch (error) {
34383492
try {
34393493
await fsPromises.unlink(tempBackupPath);
@@ -3484,6 +3538,17 @@ while (attempted.size < Math.max(1, accountCount)) {
34843538
});
34853539
invalidateAccountManagerCache();
34863540
},
3541+
cleanup: async () => {
3542+
try {
3543+
await fsPromises.unlink(backupPath);
3544+
} catch (error) {
3545+
const code = (error as NodeJS.ErrnoException).code;
3546+
if (code !== "ENOENT") {
3547+
throw error;
3548+
}
3549+
}
3550+
await pruneOldSyncPruneBackups(backupDir);
3551+
},
34873552
};
34883553
};
34893554

@@ -3665,12 +3730,35 @@ while (attempted.size < Math.max(1, accountCount)) {
36653730
return;
36663731
}
36673732

3668-
let pruneBackup: { backupPath: string; restore: () => Promise<void> } | null = null;
3733+
let pruneBackup: {
3734+
backupPath: string;
3735+
restore: () => Promise<void>;
3736+
cleanup: () => Promise<void>;
3737+
} | null = null;
36693738
const restorePruneBackup = async (): Promise<void> => {
36703739
const currentBackup = pruneBackup;
36713740
if (!currentBackup) return;
36723741
await currentBackup.restore();
36733742
pruneBackup = null;
3743+
await currentBackup.cleanup().catch((cleanupError) => {
3744+
const cleanupMessage =
3745+
cleanupError instanceof Error ? cleanupError.message : String(cleanupError);
3746+
logWarn(
3747+
`[${PLUGIN_NAME}] Failed to delete sync prune backup ${currentBackup.backupPath}: ${cleanupMessage}`,
3748+
);
3749+
});
3750+
};
3751+
const cleanupPruneBackup = async (): Promise<void> => {
3752+
const currentBackup = pruneBackup;
3753+
if (!currentBackup) return;
3754+
pruneBackup = null;
3755+
await currentBackup.cleanup().catch((cleanupError) => {
3756+
const cleanupMessage =
3757+
cleanupError instanceof Error ? cleanupError.message : String(cleanupError);
3758+
logWarn(
3759+
`[${PLUGIN_NAME}] Failed to delete sync prune backup ${currentBackup.backupPath}: ${cleanupMessage}`,
3760+
);
3761+
});
36743762
};
36753763

36763764
while (true) {
@@ -3695,7 +3783,7 @@ while (attempted.size < Math.max(1, accountCount)) {
36953783
}
36963784

36973785
const result = await syncFromCodexMultiAuth(process.cwd(), loadedSource);
3698-
pruneBackup = null;
3786+
await cleanupPruneBackup();
36993787
invalidateAccountManagerCache();
37003788
const backupLabel =
37013789
result.backupStatus === "created"

‎lib/codex-multi-auth-sync.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ interface PreparedCodexMultiAuthPreviewStorage {
121121

122122
const TEMP_CLEANUP_RETRY_DELAYS_MS = [100, 250, 500] as const;
123123
const STALE_TEMP_CLEANUP_RETRY_DELAY_MS = 150;
124-
const STALE_TEMP_SWEEP_RETRYABLE_CODES = new Set(["EBUSY", "EAGAIN", "EACCES", "EPERM", "ENOTEMPTY"]);
124+
const TEMP_CLEANUP_RETRYABLE_CODES = new Set(["EBUSY", "EAGAIN", "EACCES", "EPERM", "ENOTEMPTY"]);
125125

126126
function sleepAsync(ms: number): Promise<void> {
127127
return new Promise((resolve) => setTimeout(resolve, ms));
@@ -132,7 +132,6 @@ async function removeNormalizedImportTempDir(
132132
tempPath: string,
133133
options: NormalizedImportFileOptions,
134134
): Promise<void> {
135-
const retryableCodes = new Set(["EBUSY", "EAGAIN", "ENOTEMPTY", "EACCES", "EPERM"]);
136135
let lastMessage = "unknown cleanup failure";
137136
for (let attempt = 0; attempt <= TEMP_CLEANUP_RETRY_DELAYS_MS.length; attempt += 1) {
138137
try {
@@ -141,7 +140,7 @@ async function removeNormalizedImportTempDir(
141140
} catch (cleanupError) {
142141
const code = (cleanupError as NodeJS.ErrnoException).code;
143142
lastMessage = cleanupError instanceof Error ? cleanupError.message : String(cleanupError);
144-
if ((!code || retryableCodes.has(code)) && attempt < TEMP_CLEANUP_RETRY_DELAYS_MS.length) {
143+
if ((!code || TEMP_CLEANUP_RETRYABLE_CODES.has(code)) && attempt < TEMP_CLEANUP_RETRY_DELAYS_MS.length) {
145144
const delayMs = TEMP_CLEANUP_RETRY_DELAYS_MS[attempt];
146145
if (delayMs !== undefined) {
147146
await sleepAsync(delayMs);
@@ -307,7 +306,7 @@ async function cleanupStaleNormalizedImportTempDirs(
307306
continue;
308307
}
309308
let message = error instanceof Error ? error.message : String(error);
310-
if (code && STALE_TEMP_SWEEP_RETRYABLE_CODES.has(code)) {
309+
if (code && TEMP_CLEANUP_RETRYABLE_CODES.has(code)) {
311310
await sleepAsync(STALE_TEMP_CLEANUP_RETRY_DELAY_MS);
312311
try {
313312
await fs.rm(candidateDir, { recursive: true, force: true });
@@ -1225,6 +1224,8 @@ export async function cleanupCodexMultiAuthSyncedOverlaps(
12251224
await fs.mkdir(dirname(backupPath), { recursive: true });
12261225
const tempBackupPath = `${backupPath}.${Date.now()}.${Math.random().toString(36).slice(2, 8)}.tmp`;
12271226
try {
1227+
// Keep live tokens here so overlap cleanup can fully restore the source file;
1228+
// Windows relies on home-directory ACLs because mode 0o600 is not enforced.
12281229
await fs.writeFile(tempBackupPath, `${JSON.stringify(fallback, null, 2)}\n`, {
12291230
encoding: "utf-8",
12301231
mode: 0o600,

‎test/storage.test.ts‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
previewImportAccountsWithExistingStorage,
2323
createTimestampedBackupPath,
2424
withAccountStorageTransaction,
25+
withAccountAndFlaggedStorageTransaction,
2526
withFlaggedAccountsTransaction,
2627
loadAccountAndFlaggedStorageSnapshot,
2728
backupRawAccountsFile,
@@ -1835,6 +1836,120 @@ describe("storage", () => {
18351836
}
18361837
});
18371838

1839+
it("persists both files inside withAccountAndFlaggedStorageTransaction", async () => {
1840+
await saveAccounts({
1841+
version: 3,
1842+
activeIndex: 0,
1843+
activeIndexByFamily: {},
1844+
accounts: [
1845+
{
1846+
refreshToken: "account-refresh",
1847+
accountId: "account-id",
1848+
addedAt: 1,
1849+
lastUsed: 1,
1850+
},
1851+
],
1852+
});
1853+
await saveFlaggedAccounts({
1854+
version: 1,
1855+
accounts: [
1856+
{
1857+
refreshToken: "account-refresh",
1858+
flaggedAt: 1,
1859+
addedAt: 1,
1860+
lastUsed: 1,
1861+
},
1862+
],
1863+
});
1864+
1865+
await withAccountAndFlaggedStorageTransaction(async (current, persist) => {
1866+
await persist.flagged({
1867+
version: 1,
1868+
accounts: [
1869+
...current.flagged.accounts,
1870+
{
1871+
refreshToken: "account-next",
1872+
flaggedAt: 2,
1873+
addedAt: 2,
1874+
lastUsed: 2,
1875+
},
1876+
],
1877+
});
1878+
await persist.accounts({
1879+
version: 3,
1880+
activeIndex: 1,
1881+
activeIndexByFamily: { codex: 1 },
1882+
accounts: [
1883+
...(current.accounts?.accounts ?? []),
1884+
{
1885+
refreshToken: "account-next",
1886+
accountId: "account-next-id",
1887+
addedAt: 2,
1888+
lastUsed: 2,
1889+
},
1890+
],
1891+
});
1892+
});
1893+
1894+
const loadedAccounts = await loadAccounts();
1895+
const loadedFlagged = await loadFlaggedAccounts();
1896+
expect(loadedAccounts?.accounts.map((account) => account.refreshToken)).toEqual([
1897+
"account-refresh",
1898+
"account-next",
1899+
]);
1900+
expect(loadedAccounts?.activeIndex).toBe(1);
1901+
expect(loadedAccounts?.activeIndexByFamily?.codex).toBe(1);
1902+
expect(Object.values(loadedAccounts?.activeIndexByFamily ?? {})).toSatisfy((values) =>
1903+
values.every((value) => value === 1),
1904+
);
1905+
expect(loadedFlagged.accounts.map((account) => account.refreshToken)).toEqual([
1906+
"account-refresh",
1907+
"account-next",
1908+
]);
1909+
});
1910+
1911+
it("documents partial writes if withAccountAndFlaggedStorageTransaction throws after one persist", async () => {
1912+
await saveAccounts({
1913+
version: 3,
1914+
activeIndex: 0,
1915+
activeIndexByFamily: {},
1916+
accounts: [
1917+
{
1918+
refreshToken: "account-refresh",
1919+
accountId: "account-id",
1920+
addedAt: 1,
1921+
lastUsed: 1,
1922+
},
1923+
],
1924+
});
1925+
await saveFlaggedAccounts({
1926+
version: 1,
1927+
accounts: [
1928+
{
1929+
refreshToken: "account-refresh",
1930+
flaggedAt: 1,
1931+
addedAt: 1,
1932+
lastUsed: 1,
1933+
},
1934+
],
1935+
});
1936+
1937+
await expect(
1938+
withAccountAndFlaggedStorageTransaction(async (_current, persist) => {
1939+
await persist.flagged({
1940+
version: 1,
1941+
accounts: [],
1942+
});
1943+
throw new Error("stop after flagged write");
1944+
}),
1945+
).rejects.toThrow("stop after flagged write");
1946+
1947+
const loadedAccounts = await loadAccounts();
1948+
const loadedFlagged = await loadFlaggedAccounts();
1949+
expect(loadedAccounts?.accounts.map((account) => account.refreshToken)).toEqual(["account-refresh"]);
1950+
expect(loadedFlagged.accounts).toEqual([]);
1951+
});
1952+
18381953
it("copies the raw accounts file for backup", async () => {
18391954
await saveAccounts({
18401955
version: 3,

0 commit comments

Comments
 (0)