Skip to content
This repository was archived by the owner on Mar 30, 2026. It is now read-only.

fix: reduce excessive file writes from account state updates #459

Closed
mrm007 wants to merge 1 commit into from
+48 −15
Closed

fix: reduce excessive file writes from account state updates #459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1699,8 +1699,6 @@ export const createAntigravityPlugin = (providerId: string) => async (
accountManager.markToastShown(account.index);
}

accountManager.requestSaveToDisk();

let authRecord = accountManager.toAuthDetails(account);

if (accessTokenExpired(authRecord)) {
Expand Down Expand Up @@ -2330,7 +2328,8 @@ export const createAntigravityPlugin = (providerId: string) => async (
account.consecutiveFailures = 0;
getHealthTracker().recordSuccess(account.index);
accountManager.markAccountUsed(account.index);

accountManager.requestSaveToDisk()
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolon

The call to requestSaveToDisk() is missing a trailing semicolon, inconsistent with the rest of the codebase style.

Suggested change
accountManager.requestSaveToDisk()
accountManager.requestSaveToDisk();

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/plugin.ts
Line: 2331:2331

Comment:
**Missing semicolon**

The call to `requestSaveToDisk()` is missing a trailing semicolon, inconsistent with the rest of the codebase style.

```suggestion
                  accountManager.requestSaveToDisk();
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.


void triggerAsyncQuotaRefreshForAccount(
accountManager,
account.index,
Expand Down
35 changes: 31 additions & 4 deletions src/plugin/accounts.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,7 @@ describe("AccountManager", () => {

expect(saveSpy).not.toHaveBeenCalled();

await vi.advanceTimersByTimeAsync(1500);
await vi.advanceTimersByTimeAsync(5500);

expect(saveSpy).toHaveBeenCalledTimes(1);

Expand All @@ -1112,7 +1112,7 @@ describe("AccountManager", () => {

const flushPromise = manager.flushSaveToDisk();

await vi.advanceTimersByTimeAsync(1500);
await vi.advanceTimersByTimeAsync(5500);
await flushPromise;

expect(saveSpy).toHaveBeenCalledTimes(1);
Expand All @@ -1135,16 +1135,43 @@ describe("AccountManager", () => {
const saveSpy = vi.spyOn(manager, "saveToDisk").mockResolvedValue();

manager.requestSaveToDisk();
await vi.advanceTimersByTimeAsync(1500);
await vi.advanceTimersByTimeAsync(5500);

expect(saveSpy).toHaveBeenCalledTimes(1);

await vi.advanceTimersByTimeAsync(3000);
await vi.advanceTimersByTimeAsync(6000);

expect(saveSpy).toHaveBeenCalledTimes(1);

saveSpy.mockRestore();
});

it("skips write when account state has not changed", async () => {
vi.useFakeTimers();

const stored: AccountStorageV4 = {
version: 4,
accounts: [
{ refreshToken: "r1", projectId: "p1", addedAt: 1, lastUsed: 0 },
],
activeIndex: 0,
};

const manager = new AccountManager(undefined, stored);
const saveSpy = vi.spyOn(manager, "saveToDisk").mockResolvedValue();

// First save — should write
manager.requestSaveToDisk();
await vi.advanceTimersByTimeAsync(5500);
expect(saveSpy).toHaveBeenCalledTimes(1);

// Second save with no state change — should skip
manager.requestSaveToDisk();
await vi.advanceTimersByTimeAsync(5500);
expect(saveSpy).toHaveBeenCalledTimes(1);

saveSpy.mockRestore();
});
Comment on lines +1149 to +1174
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deduplication test mocks saveToDisk, bypassing the real snapshot-write path

The new test mocks saveToDisk via vi.spyOn(...).mockResolvedValue(). This means executeSave takes a snapshot, sees a diff from null, calls the mock (which does nothing), then stores the snapshot as lastSavedSnapshot. The second call correctly sees equality and skips.

However, this doesn't test the case where the real saveToDisk (which calls buildStorageState() again independently) could write a state that differs from the comparison snapshot. Consider adding a test that exercises state mutation during the debounce window to confirm the snapshot reflects what's actually on disk.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/plugin/accounts.test.ts
Line: 1149:1174

Comment:
**Deduplication test mocks saveToDisk, bypassing the real snapshot-write path**

The new test mocks `saveToDisk` via `vi.spyOn(...).mockResolvedValue()`. This means `executeSave` takes a snapshot, sees a diff from `null`, calls the mock (which does nothing), then stores the snapshot as `lastSavedSnapshot`. The second call correctly sees equality and skips.

However, this doesn't test the case where the *real* `saveToDisk` (which calls `buildStorageState()` again independently) could write a state that differs from the comparison snapshot. Consider adding a test that exercises state mutation *during* the debounce window to confirm the snapshot reflects what's actually on disk.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

});

describe("Rate Limit Reason Classification", () => {
Expand Down
23 changes: 15 additions & 8 deletions src/plugin/accounts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ export class AccountManager {
private savePending = false;
private saveTimeout: ReturnType<typeof setTimeout> | null = null;
private savePromiseResolvers: Array<() => void> = [];
private lastSavedSnapshot: string | null = null;

static async loadFromDisk(authFallback?: OAuthAuthDetails): Promise<AccountManager> {
const stored = await loadAccounts();
Expand Down Expand Up @@ -973,11 +974,11 @@ export class AccountManager {
return [...this.accounts];
}

async saveToDisk(): Promise<void> {
private buildStorageState(): AccountStorageV4 {
const claudeIndex = Math.max(0, this.currentAccountIndexByFamily.claude);
const geminiIndex = Math.max(0, this.currentAccountIndexByFamily.gemini);
const storage: AccountStorageV4 = {

return {
version: 4,
accounts: this.accounts.map((a) => ({
email: a.email,
Expand Down Expand Up @@ -1005,9 +1006,11 @@ export class AccountManager {
claude: claudeIndex,
gemini: geminiIndex,
},
};
}
}

await saveAccounts(storage);
async saveToDisk(): Promise<void> {
await saveAccounts(this.buildStorageState());
}

requestSaveToDisk(): void {
Expand All @@ -1017,7 +1020,7 @@ export class AccountManager {
this.savePending = true;
this.saveTimeout = setTimeout(() => {
void this.executeSave();
}, 1000);
}, 5000);
}

async flushSaveToDisk(): Promise<void> {
Expand All @@ -1032,9 +1035,13 @@ export class AccountManager {
private async executeSave(): Promise<void> {
this.savePending = false;
this.saveTimeout = null;

try {
await this.saveToDisk();
const snapshot = JSON.stringify(this.buildStorageState());
if (snapshot !== this.lastSavedSnapshot) {
await this.saveToDisk();
this.lastSavedSnapshot = snapshot;
Comment on lines +1040 to +1043
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale snapshot stored as last-saved state

lastSavedSnapshot is set to the snapshot taken before saveToDisk() is awaited, but saveToDisk() internally calls buildStorageState() again. If any state mutation happens during the await saveAccounts(...) call (e.g., a rate-limit update arriving while the lockfile/rename cycle is in progress), the snapshot stored in lastSavedSnapshot will diverge from what was actually written to disk. The next executeSave call will then compare against a snapshot that doesn't match the file on disk, potentially skipping a write that should have happened.

A safer approach is to capture the snapshot once, pass it to saveToDisk, and only update lastSavedSnapshot after a confirmed successful write of that exact snapshot:

const snapshot = JSON.stringify(this.buildStorageState());
if (snapshot !== this.lastSavedSnapshot) {
  await saveAccounts(JSON.parse(snapshot) as AccountStorageV4);
  this.lastSavedSnapshot = snapshot;
}

This ensures the snapshot stored is exactly what was written, not a concurrent re-read.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/plugin/accounts.ts
Line: 1040:1043

Comment:
**Stale snapshot stored as last-saved state**

`lastSavedSnapshot` is set to the snapshot taken *before* `saveToDisk()` is awaited, but `saveToDisk()` internally calls `buildStorageState()` again. If any state mutation happens during the `await saveAccounts(...)` call (e.g., a rate-limit update arriving while the lockfile/rename cycle is in progress), the snapshot stored in `lastSavedSnapshot` will diverge from what was actually written to disk. The next `executeSave` call will then compare against a snapshot that doesn't match the file on disk, potentially skipping a write that should have happened.

A safer approach is to capture the snapshot once, pass it to `saveToDisk`, and only update `lastSavedSnapshot` after a confirmed successful write of that exact snapshot:

```
const snapshot = JSON.stringify(this.buildStorageState());
if (snapshot !== this.lastSavedSnapshot) {
  await saveAccounts(JSON.parse(snapshot) as AccountStorageV4);
  this.lastSavedSnapshot = snapshot;
}
```

This ensures the snapshot stored is exactly what was written, not a concurrent re-read.

How can I resolve this? If you propose a fix, please make it concise.

}
} catch {
// best-effort persistence; avoid unhandled rejection from timer-driven saves
} finally {
Expand Down