Skip to content

Commit b9d7c2f

Browse files
committed
Fix storage path state save race
1 parent 72a115e commit b9d7c2f

2 files changed

Lines changed: 99 additions & 7 deletions

File tree

lib/storage/path-state.ts

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,5 @@ export async function runWithStoragePathState<T>(
3333
state: StoragePathState,
3434
fn: () => T | Promise<T>,
3535
): Promise<T> {
36-
const previousState = currentStorageState;
37-
currentStorageState = state;
38-
try {
39-
return await storagePathStateContext.run(state, fn);
40-
} finally {
41-
currentStorageState = previousState;
42-
}
36+
return await storagePathStateContext.run(state, fn);
4337
}

test/accounts.test.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1865,6 +1865,104 @@ describe("AccountManager", () => {
18651865
vi.useRealTimers();
18661866
}
18671867
});
1868+
1869+
it("keeps ambient storage path state stable during concurrent saves", async () => {
1870+
const { withAccountStorageTransaction } = await import("../lib/storage.js");
1871+
const mockWithAccountStorageTransaction = vi.mocked(
1872+
withAccountStorageTransaction,
1873+
);
1874+
const createDeferred = () => {
1875+
let resolve!: () => void;
1876+
const promise = new Promise<void>((resolvePromise) => {
1877+
resolve = resolvePromise;
1878+
});
1879+
return { promise, resolve };
1880+
};
1881+
const enteredResolvers = [createDeferred(), createDeferred()];
1882+
const releaseResolvers = [createDeferred(), createDeferred()];
1883+
const seenStates: Array<ReturnType<typeof getStoragePathState>> = [];
1884+
let callIndex = 0;
1885+
1886+
mockWithAccountStorageTransaction.mockImplementation(async (handler) => {
1887+
const currentCall = callIndex;
1888+
callIndex += 1;
1889+
seenStates.push({ ...getStoragePathState() });
1890+
enteredResolvers[currentCall]?.resolve();
1891+
if (currentCall === 0) {
1892+
await enteredResolvers[1]?.promise;
1893+
}
1894+
await releaseResolvers[currentCall]?.promise;
1895+
return handler(null, async () => undefined);
1896+
});
1897+
1898+
const now = Date.now();
1899+
const stored = {
1900+
version: 3 as const,
1901+
activeIndex: 0,
1902+
accounts: [{ refreshToken: "token-1", addedAt: now, lastUsed: now }],
1903+
};
1904+
1905+
setStoragePathState({
1906+
currentStoragePath: "/ambient/storage.json",
1907+
currentLegacyProjectStoragePath: null,
1908+
currentLegacyWorktreeStoragePath: null,
1909+
currentProjectRoot: "/ambient",
1910+
});
1911+
setStoragePathState({
1912+
currentStoragePath: "/repo-a/storage.json",
1913+
currentLegacyProjectStoragePath: null,
1914+
currentLegacyWorktreeStoragePath: null,
1915+
currentProjectRoot: "/repo-a",
1916+
});
1917+
const managerA = new AccountManager(undefined, stored);
1918+
setStoragePathState({
1919+
currentStoragePath: "/repo-b/storage.json",
1920+
currentLegacyProjectStoragePath: null,
1921+
currentLegacyWorktreeStoragePath: null,
1922+
currentProjectRoot: "/repo-b",
1923+
});
1924+
const managerB = new AccountManager(undefined, stored);
1925+
setStoragePathState({
1926+
currentStoragePath: "/ambient/storage.json",
1927+
currentLegacyProjectStoragePath: null,
1928+
currentLegacyWorktreeStoragePath: null,
1929+
currentProjectRoot: "/ambient",
1930+
});
1931+
1932+
const saveA = managerA.saveToDisk();
1933+
await enteredResolvers[0]?.promise;
1934+
const saveB = managerB.saveToDisk();
1935+
await enteredResolvers[1]?.promise;
1936+
1937+
expect(getStoragePathState()).toEqual(
1938+
expect.objectContaining({
1939+
currentStoragePath: "/ambient/storage.json",
1940+
currentProjectRoot: "/ambient",
1941+
}),
1942+
);
1943+
1944+
releaseResolvers[0]?.resolve();
1945+
await saveA;
1946+
releaseResolvers[1]?.resolve();
1947+
await saveB;
1948+
1949+
expect(seenStates).toEqual([
1950+
expect.objectContaining({
1951+
currentStoragePath: "/repo-a/storage.json",
1952+
currentProjectRoot: "/repo-a",
1953+
}),
1954+
expect.objectContaining({
1955+
currentStoragePath: "/repo-b/storage.json",
1956+
currentProjectRoot: "/repo-b",
1957+
}),
1958+
]);
1959+
expect(getStoragePathState()).toEqual(
1960+
expect.objectContaining({
1961+
currentStoragePath: "/ambient/storage.json",
1962+
currentProjectRoot: "/ambient",
1963+
}),
1964+
);
1965+
});
18681966
});
18691967

18701968
describe("constructor edge cases", () => {

0 commit comments

Comments
 (0)