From 9e35ef5a8cb7a479f57faf4400071df5309ca5a9 Mon Sep 17 00:00:00 2001 From: ndycode Date: Mon, 6 Apr 2026 10:35:25 +0800 Subject: [PATCH] fix: recreate live account sync on config changes Live account sync reused the same controller once created, even when its effective debounce/poll configuration changed. That leaves stale runtime state in place and keeps watching with outdated timing parameters. Track a live-sync config key and recreate the controller when the configuration changes, matching the refresh-guardian pattern. --- index.ts | 9 +++++-- lib/runtime/live-sync-entry.ts | 15 +++++++++++ lib/runtime/live-sync.ts | 23 ++++++++++++++--- lib/runtime/runtime-services.ts | 27 +++++++++++++++++-- test/live-sync-entry.test.ts | 4 +++ test/runtime-live-sync.test.ts | 46 +++++++++++++++++++++++++++++++++ test/runtime-services.test.ts | 37 +++++++++++++++++++++++--- 7 files changed, 151 insertions(+), 10 deletions(-) diff --git a/index.ts b/index.ts index e705bbb6..5e84443b 100644 --- a/index.ts +++ b/index.ts @@ -309,8 +309,9 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { let startupPrewarmTriggered = false; let lastCodexCliActiveSyncIndex: number | null = null; let perProjectStorageWarningShown = false; - let liveAccountSync: LiveAccountSync | null = null; - let liveAccountSyncPath: string | null = null; +let liveAccountSync: LiveAccountSync | null = null; +let liveAccountSyncPath: string | null = null; +let liveAccountSyncConfigKey: string | null = null; let refreshGuardian: RefreshGuardian | null = null; let refreshGuardianConfigKey: string | null = null; let refreshGuardianCleanupRegistered = false; @@ -481,7 +482,10 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { authFallback, currentSync: liveAccountSync, currentPath: liveAccountSyncPath, + currentConfigKey: liveAccountSyncConfigKey, getLiveAccountSync, + getLiveAccountSyncDebounceMs, + getLiveAccountSyncPollMs, getStoragePath, createSync: (oauthFallback) => new LiveAccountSync( @@ -502,6 +506,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { }); liveAccountSync = next.liveAccountSync; liveAccountSyncPath = next.liveAccountSyncPath; + liveAccountSyncConfigKey = next.liveAccountSyncConfigKey; }; const ensureRefreshGuardian = ( pluginConfig: ReturnType, diff --git a/lib/runtime/live-sync-entry.ts b/lib/runtime/live-sync-entry.ts index fb368d09..40f1fdf3 100644 --- a/lib/runtime/live-sync-entry.ts +++ b/lib/runtime/live-sync-entry.ts @@ -12,10 +12,17 @@ export async function ensureLiveAccountSyncEntry< authFallback?: OAuthAuthDetails; currentSync: TSync | null; currentPath: string | null; + currentConfigKey?: string | null; getLiveAccountSync: ( config: ReturnType, ) => boolean; getStoragePath: () => string; + getLiveAccountSyncDebounceMs: ( + config: ReturnType, + ) => number; + getLiveAccountSyncPollMs: ( + config: ReturnType, + ) => number; createSync: (authFallback?: OAuthAuthDetails) => TSync; registerCleanup: (cleanup: () => void) => void; logWarn: (message: string) => void; @@ -25,6 +32,8 @@ export async function ensureLiveAccountSyncEntry< targetPath: string; currentSync: TSync | null; currentPath: string | null; + currentConfigKey?: string | null; + configKey?: string | null; authFallback?: OAuthAuthDetails; createSync: (authFallback?: OAuthAuthDetails) => TSync; registerCleanup: (cleanup: () => void) => void; @@ -33,16 +42,22 @@ export async function ensureLiveAccountSyncEntry< }) => Promise<{ liveAccountSync: TSync | null; liveAccountSyncPath: string | null; + liveAccountSyncConfigKey: string | null; }>; }): Promise<{ liveAccountSync: TSync | null; liveAccountSyncPath: string | null; + liveAccountSyncConfigKey: string | null; }> { + const debounceMs = params.getLiveAccountSyncDebounceMs(params.pluginConfig); + const pollIntervalMs = params.getLiveAccountSyncPollMs(params.pluginConfig); return params.ensureLiveAccountSyncState({ enabled: params.getLiveAccountSync(params.pluginConfig), targetPath: params.getStoragePath(), currentSync: params.currentSync, currentPath: params.currentPath, + currentConfigKey: params.currentConfigKey, + configKey: `${debounceMs}:${pollIntervalMs}`, authFallback: params.authFallback, createSync: params.createSync, registerCleanup: params.registerCleanup, diff --git a/lib/runtime/live-sync.ts b/lib/runtime/live-sync.ts index c7859330..af9507fc 100644 --- a/lib/runtime/live-sync.ts +++ b/lib/runtime/live-sync.ts @@ -15,6 +15,7 @@ export async function ensureRuntimeLiveAccountSync< getStoragePath: () => string; currentSync: TSync | null; currentPath: string | null; + currentConfigKey?: string | null; currentCleanupRegistered: boolean; getCurrentSync: () => TSync | null; createSync: ( @@ -29,6 +30,7 @@ export async function ensureRuntimeLiveAccountSync< commitState: (state: { sync: TSync | null; path: string | null; + configKey: string | null; cleanupRegistered: boolean; }) => void; registerCleanup: (cleanup: () => void) => void; @@ -37,18 +39,24 @@ export async function ensureRuntimeLiveAccountSync< }): Promise<{ sync: TSync | null; path: string | null; + configKey: string | null; cleanupRegistered: boolean; }> { + const debounceMs = deps.getLiveAccountSyncDebounceMs(deps.pluginConfig); + const pollIntervalMs = deps.getLiveAccountSyncPollMs(deps.pluginConfig); + const nextConfigKey = `${debounceMs}:${pollIntervalMs}`; if (!deps.getLiveAccountSync(deps.pluginConfig)) { deps.currentSync?.stop(); deps.commitState({ sync: null, path: null, + configKey: null, cleanupRegistered: deps.currentCleanupRegistered, }); return { sync: null, path: null, + configKey: null, cleanupRegistered: deps.currentCleanupRegistered, }; } @@ -57,10 +65,18 @@ export async function ensureRuntimeLiveAccountSync< let sync = deps.currentSync; let cleanupRegistered = deps.currentCleanupRegistered; let nextPath = deps.currentPath; + let configKey = deps.currentConfigKey ?? null; + if (sync && configKey !== null && configKey !== nextConfigKey) { + sync.stop(); + sync = null; + nextPath = null; + configKey = null; + } const commitState = (): void => { deps.commitState({ sync, path: nextPath, + configKey, cleanupRegistered, }); }; @@ -70,10 +86,11 @@ export async function ensureRuntimeLiveAccountSync< await deps.reloadAccountManagerFromDisk(deps.authFallback); }, { - debounceMs: deps.getLiveAccountSyncDebounceMs(deps.pluginConfig), - pollIntervalMs: deps.getLiveAccountSyncPollMs(deps.pluginConfig), + debounceMs, + pollIntervalMs, }, ); + configKey = nextConfigKey; commitState(); if (!cleanupRegistered) { deps.registerCleanup(() => { @@ -106,5 +123,5 @@ export async function ensureRuntimeLiveAccountSync< } } - return { sync, path: nextPath, cleanupRegistered }; + return { sync, path: nextPath, configKey, cleanupRegistered }; } diff --git a/lib/runtime/runtime-services.ts b/lib/runtime/runtime-services.ts index 7bd52b1a..1b17f984 100644 --- a/lib/runtime/runtime-services.ts +++ b/lib/runtime/runtime-services.ts @@ -20,6 +20,8 @@ export async function ensureLiveAccountSyncState< targetPath: string; currentSync: TSync | null; currentPath: string | null; + currentConfigKey?: string | null; + configKey?: string | null; authFallback?: OAuthAuthDetails; createSync: (authFallback?: OAuthAuthDetails) => TSync; registerCleanup: (cleanup: () => void) => void; @@ -28,21 +30,42 @@ export async function ensureLiveAccountSyncState< }): Promise<{ liveAccountSync: TSync | null; liveAccountSyncPath: string | null; + liveAccountSyncConfigKey: string | null; }> { let liveAccountSync = params.currentSync; let liveAccountSyncPath = params.currentPath; + let liveAccountSyncConfigKey = params.currentConfigKey ?? null; if (!params.enabled) { if (liveAccountSync) { liveAccountSync.stop(); liveAccountSync = null; liveAccountSyncPath = null; + liveAccountSyncConfigKey = null; } - return { liveAccountSync, liveAccountSyncPath }; + return { + liveAccountSync, + liveAccountSyncPath, + liveAccountSyncConfigKey, + }; + } + + const nextConfigKey = params.configKey ?? null; + if ( + liveAccountSync && + nextConfigKey !== null && + liveAccountSyncConfigKey !== null && + liveAccountSyncConfigKey !== nextConfigKey + ) { + liveAccountSync.stop(); + liveAccountSync = null; + liveAccountSyncPath = null; + liveAccountSyncConfigKey = null; } if (!liveAccountSync) { liveAccountSync = params.createSync(params.authFallback); + liveAccountSyncConfigKey = nextConfigKey; params.registerCleanup(() => { liveAccountSync?.stop(); }); @@ -71,7 +94,7 @@ export async function ensureLiveAccountSyncState< } } - return { liveAccountSync, liveAccountSyncPath }; + return { liveAccountSync, liveAccountSyncPath, liveAccountSyncConfigKey }; } export function ensureRefreshGuardianState< diff --git a/test/live-sync-entry.test.ts b/test/live-sync-entry.test.ts index 60366734..f2926993 100644 --- a/test/live-sync-entry.test.ts +++ b/test/live-sync-entry.test.ts @@ -17,7 +17,10 @@ describe("live sync entry", () => { } as never, currentSync: null, currentPath: null, + currentConfigKey: null, getLiveAccountSync: () => true, + getLiveAccountSyncDebounceMs: () => 25, + getLiveAccountSyncPollMs: () => 250, getStoragePath: () => "/tmp/accounts.json", createSync: vi.fn(() => ({ stop: vi.fn(), syncToPath: vi.fn() })), registerCleanup: vi.fn(), @@ -30,6 +33,7 @@ describe("live sync entry", () => { expect.objectContaining({ enabled: true, targetPath: "/tmp/accounts.json", + configKey: "25:250", pluginName: "plugin", }), ); diff --git a/test/runtime-live-sync.test.ts b/test/runtime-live-sync.test.ts index d9861918..19b6f0f7 100644 --- a/test/runtime-live-sync.test.ts +++ b/test/runtime-live-sync.test.ts @@ -21,6 +21,7 @@ describe("runtime live sync", () => { let committedState = { sync: overrides.currentSync ?? null, path: overrides.currentPath ?? null, + configKey: null as string | null, cleanupRegistered: overrides.currentCleanupRegistered ?? false, }; let cleanupCallback: (() => void) | null = null; @@ -92,12 +93,14 @@ describe("runtime live sync", () => { await expect(ensureRuntimeLiveAccountSync(deps)).resolves.toEqual({ sync: null, path: null, + configKey: null, cleanupRegistered: true, }); expect(currentSync.stop).toHaveBeenCalledTimes(1); expect(deps.commitState).toHaveBeenCalledWith({ sync: null, path: null, + configKey: null, cleanupRegistered: true, }); }); @@ -111,6 +114,7 @@ describe("runtime live sync", () => { expect(createSync).toHaveBeenCalledTimes(1); expect(registerCleanup).toHaveBeenCalledTimes(1); expect(first.path).toBe("C:\\repo\\accounts.json"); + expect(first.configKey).toBe("25:250"); expect(first.cleanupRegistered).toBe(true); expect(first.sync?.syncToPath).toHaveBeenCalledWith( "C:\\repo\\accounts.json", @@ -120,6 +124,7 @@ describe("runtime live sync", () => { ...deps, currentSync: first.sync, currentPath: first.path, + currentConfigKey: first.configKey, currentCleanupRegistered: first.cleanupRegistered, }); @@ -151,6 +156,7 @@ describe("runtime live sync", () => { await expect(pending).resolves.toMatchObject({ sync: currentSync, path: "C:\\repo\\new.json", + configKey: null, cleanupRegistered: true, }); expect(currentSync.syncToPath).toHaveBeenCalledTimes(3); @@ -176,6 +182,7 @@ describe("runtime live sync", () => { await expect(pending).resolves.toMatchObject({ sync: currentSync, path: "C:\\repo\\old.json", + configKey: null, cleanupRegistered: true, }); expect(currentSync.syncToPath).toHaveBeenCalledTimes(3); @@ -213,6 +220,7 @@ describe("runtime live sync", () => { const committed = getCommittedState(); expect(committed.sync).toBe(createdSync); expect(committed.path).toBeNull(); + expect(committed.configKey).toBe("25:250"); expect(committed.cleanupRegistered).toBe(true); const cleanup = getCleanupCallback(); @@ -238,12 +246,14 @@ describe("runtime live sync", () => { const committed = getCommittedState(); expect(committed.sync).toBe(createdSync); + expect(committed.configKey).toBe("25:250"); expect(committed.cleanupRegistered).toBe(true); const second = ensureRuntimeLiveAccountSync({ ...deps, currentSync: committed.sync, currentPath: committed.path, + currentConfigKey: committed.configKey, currentCleanupRegistered: committed.cleanupRegistered, }); await vi.runAllTicks(); @@ -253,11 +263,13 @@ describe("runtime live sync", () => { await expect(pending).resolves.toMatchObject({ sync: createdSync, path: "C:\\repo\\accounts.json", + configKey: "25:250", cleanupRegistered: true, }); await expect(second).resolves.toMatchObject({ sync: createdSync, path: "C:\\repo\\accounts.json", + configKey: "25:250", cleanupRegistered: true, }); }); @@ -273,6 +285,7 @@ describe("runtime live sync", () => { ...deps, currentSync: first.sync, currentPath: first.path, + currentConfigKey: first.configKey, currentCleanupRegistered: first.cleanupRegistered, getLiveAccountSync: vi.fn().mockReturnValue(false), }); @@ -282,6 +295,7 @@ describe("runtime live sync", () => { ...deps, currentSync: disabled.sync, currentPath: disabled.path, + currentConfigKey: disabled.configKey, currentCleanupRegistered: disabled.cleanupRegistered, }); setLiveSync(reenabled.sync); @@ -293,4 +307,36 @@ describe("runtime live sync", () => { expect((reenabled.sync as { stop: ReturnType }).stop).toHaveBeenCalledTimes(1); expect((first.sync as { stop: ReturnType }).stop).toHaveBeenCalledTimes(1); }); + + it("recreates live sync when debounce/poll settings change", async () => { + const firstSync = { + stop: vi.fn(), + syncToPath: vi.fn().mockResolvedValue(undefined), + }; + const secondSync = { + stop: vi.fn(), + syncToPath: vi.fn().mockResolvedValue(undefined), + }; + const { deps, createSync } = createDeps({ + currentSync: firstSync, + currentPath: "C:\\repo\\accounts.json", + currentCleanupRegistered: true, + }); + createSync.mockReturnValue(secondSync); + deps.getLiveAccountSyncDebounceMs = vi.fn().mockReturnValue(50); + deps.getLiveAccountSyncPollMs = vi.fn().mockReturnValue(500); + + const result = await ensureRuntimeLiveAccountSync({ + ...deps, + currentSync: firstSync, + currentPath: "C:\\repo\\accounts.json", + currentConfigKey: "25:250", + }); + + expect(firstSync.stop).toHaveBeenCalledTimes(1); + expect(createSync).toHaveBeenCalledTimes(1); + expect(result.sync).toBe(secondSync); + expect(result.configKey).toBe("50:500"); + expect(secondSync.syncToPath).toHaveBeenCalledWith("C:\\repo\\accounts.json"); + }); }); diff --git a/test/runtime-services.test.ts b/test/runtime-services.test.ts index 4cfe1101..2cf3ad38 100644 --- a/test/runtime-services.test.ts +++ b/test/runtime-services.test.ts @@ -13,6 +13,7 @@ describe("runtime services helpers", () => { targetPath: "/tmp/a", currentSync: { stop, syncToPath: vi.fn() }, currentPath: "/tmp/old", + currentConfigKey: "old", createSync: vi.fn(), registerCleanup: vi.fn(), logWarn: vi.fn(), @@ -23,6 +24,7 @@ describe("runtime services helpers", () => { expect(result).toEqual({ liveAccountSync: null, liveAccountSyncPath: null, + liveAccountSyncConfigKey: null, }); }); @@ -34,6 +36,7 @@ describe("runtime services helpers", () => { targetPath: "/tmp/a", currentSync: null, currentPath: null, + configKey: "25:250", createSync: vi.fn(() => created), registerCleanup: vi.fn(), logWarn: vi.fn(), @@ -43,6 +46,7 @@ describe("runtime services helpers", () => { expect(syncToPath).toHaveBeenCalledWith("/tmp/a"); expect(result.liveAccountSync).toBe(created); expect(result.liveAccountSyncPath).toBe("/tmp/a"); + expect(result.liveAccountSyncConfigKey).toBe("25:250"); }); it("warns and keeps the previous path when busy retries are exhausted", async () => { @@ -59,9 +63,10 @@ describe("runtime services helpers", () => { const resultPromise = ensureLiveAccountSyncState({ enabled: true, targetPath: "/tmp/new", - currentSync, - currentPath: "/tmp/old", - createSync: vi.fn(), + currentSync, + currentPath: "/tmp/old", + currentConfigKey: "old", + createSync: vi.fn(), registerCleanup: vi.fn(), logWarn, pluginName: "plugin", @@ -77,12 +82,38 @@ describe("runtime services helpers", () => { expect(result).toEqual({ liveAccountSync: currentSync, liveAccountSyncPath: "/tmp/old", + liveAccountSyncConfigKey: "old", }); } finally { vi.useRealTimers(); } }); + it("recreates live sync when config key changes", async () => { + const oldSync = { stop: vi.fn(), syncToPath: vi.fn() }; + const newSync = { stop: vi.fn(), syncToPath: vi.fn().mockResolvedValue(undefined) }; + const createSync = vi.fn(() => newSync); + + const result = await ensureLiveAccountSyncState({ + enabled: true, + targetPath: "/tmp/a", + currentSync: oldSync, + currentPath: "/tmp/a", + currentConfigKey: "25:250", + configKey: "50:500", + createSync, + registerCleanup: vi.fn(), + logWarn: vi.fn(), + pluginName: "plugin", + }); + + expect(oldSync.stop).toHaveBeenCalledTimes(1); + expect(createSync).toHaveBeenCalledTimes(1); + expect(newSync.syncToPath).toHaveBeenCalledWith("/tmp/a"); + expect(result.liveAccountSync).toBe(newSync); + expect(result.liveAccountSyncConfigKey).toBe("50:500"); + }); + it("recreates refresh guardian when config changes and clears when disabled", () => { const oldGuardian = { stop: vi.fn(), start: vi.fn() }; const createGuardian = vi.fn(() => ({ stop: vi.fn(), start: vi.fn() }));