@@ -11,6 +11,10 @@ vi.mock("../../../api/providers/fetchers/lmstudio", () => ({
1111 getLMStudioModels : vi . fn ( ) ,
1212} ) )
1313
14+ vi . mock ( "../../../integrations/theme/getTheme" , ( ) => ( {
15+ getTheme : vi . fn ( ) . mockResolvedValue ( { } ) ,
16+ } ) )
17+
1418vi . mock ( "../../../integrations/openai-codex/oauth" , ( ) => ( {
1519 openAiCodexOAuthManager : {
1620 getAccessToken : vi . fn ( ) ,
@@ -1817,4 +1821,112 @@ describe("webviewMessageHandler - telemetrySetting", () => {
18171821 const calls = vi . mocked ( TelemetryService . instance . updateTelemetryState ) . mock . calls
18181822 expect ( calls . at ( - 1 ) ) . toEqual ( [ true ] )
18191823 } )
1824+
1825+ // CodeRabbit follow-up on the finding #12 fix: webviewDidLaunch's telemetry init read state
1826+ // via an async provider.getStateToPostToWebview().then(...) continuation, outside
1827+ // telemetrySettingQueue -- so it could resolve after a concurrent "telemetrySetting" message
1828+ // and clobber that message's queued (correct) update with a stale value. webviewDidLaunch now
1829+ // reads getGlobalState synchronously and is routed through the same queue.
1830+ it ( "does not let webviewDidLaunch's telemetry init race and clobber a concurrent telemetrySetting message" , async ( ) => {
1831+ const { TelemetryService } = await import ( "@roo-code/telemetry" )
1832+ vi . mocked ( TelemetryService . hasInstance ) . mockReturnValue ( true )
1833+ vi . mocked ( vscode . env ) . isTelemetryEnabled = true
1834+
1835+ // webviewDidLaunch starts out "unset" (disclosed opt-out default -- opted in). Scoped to
1836+ // the "telemetrySetting" key specifically -- webviewDidLaunch also calls
1837+ // updateGlobalState("customModes", ...) through the same contextProxy mock, which must
1838+ // not clobber storedSetting.
1839+ let storedSetting : string | undefined = "unset"
1840+ vi . mocked ( mockClineProvider . contextProxy . getValue ) . mockImplementation ( ( key : string ) =>
1841+ key === "telemetrySetting" ? storedSetting : undefined ,
1842+ )
1843+ vi . mocked ( mockClineProvider . contextProxy . setValue ) . mockImplementation ( async ( key : string , value ) => {
1844+ if ( key === "telemetrySetting" ) {
1845+ storedSetting = value as string
1846+ }
1847+ } )
1848+
1849+ vi . mocked ( mockClineProvider . customModesManager . getCustomModes ) . mockResolvedValue ( [ ] )
1850+ const providerForLaunch = mockClineProvider as unknown as {
1851+ getMcpHub : ReturnType < typeof vi . fn >
1852+ providerSettingsManager : { listConfig : ReturnType < typeof vi . fn > }
1853+ getStateToPostToWebview : ReturnType < typeof vi . fn >
1854+ }
1855+ providerForLaunch . getMcpHub = vi . fn ( ) . mockReturnValue ( undefined )
1856+ providerForLaunch . providerSettingsManager = {
1857+ listConfig : vi . fn ( ) . mockResolvedValue ( undefined ) ,
1858+ }
1859+
1860+ // Deferred-promise handshake instead of setTimeout delays, so ordering is enforced
1861+ // explicitly rather than by racing real clock delays. Signals when webviewDidLaunch has
1862+ // taken its (pre-fix) state snapshot -- only fires under the *old* code path
1863+ // (provider.getStateToPostToWebview().then(...)); the fix never calls it at all.
1864+ let snapshotTaken ! : ( ) => void
1865+ const snapshotTakenPromise = new Promise < void > ( ( resolve ) => {
1866+ snapshotTaken = resolve
1867+ } )
1868+ let releaseSnapshot ! : ( ) => void
1869+ const snapshotReleased = new Promise < void > ( ( resolve ) => {
1870+ releaseSnapshot = resolve
1871+ } )
1872+
1873+ // Snapshots storedSetting at call time (mirroring the real ClineProvider building its
1874+ // state object synchronously before any internal awaits), signals it was taken, then
1875+ // waits until the test explicitly releases it -- by which point the concurrent
1876+ // telemetrySetting write below has already landed, making the snapshot genuinely stale
1877+ // once its .then() callback finally runs.
1878+ providerForLaunch . getStateToPostToWebview = vi . fn ( ) . mockImplementation ( async ( ) => {
1879+ const snapshot = storedSetting
1880+ snapshotTaken ( )
1881+ await snapshotReleased
1882+ return { telemetrySetting : snapshot }
1883+ } )
1884+
1885+ // webviewDidLaunch fires first (e.g. webview reload) -- its telemetry init is now queued
1886+ // behind telemetrySettingQueue rather than resolving independently.
1887+ const launch = webviewMessageHandler ( mockClineProvider , { type : "webviewDidLaunch" } )
1888+
1889+ // Wait for webviewDidLaunch to either take its (pre-fix) snapshot, or flush a fixed
1890+ // number of microtask turns as a same-tick fallback for the fixed code path (which never
1891+ // triggers that signal) -- enough for its synchronous prefix (await getCustomModes(),
1892+ // await updateGlobalState()) to run, without relying on a wall-clock timer.
1893+ await Promise . race ( [
1894+ snapshotTakenPromise ,
1895+ ( async ( ) => {
1896+ for ( let i = 0 ; i < 10 ; i ++ ) {
1897+ await Promise . resolve ( )
1898+ }
1899+ } ) ( ) ,
1900+ ] )
1901+
1902+ // A concurrent "telemetrySetting" message turns telemetry off, and is awaited to
1903+ // completion -- including its own updateTelemetryState(false) call -- *before* the
1904+ // deferred (pre-fix-only) snapshot below is released. Against the pre-fix code, this
1905+ // proves the snapshot it captured earlier ("unset") is genuinely stale by the time its
1906+ // .then() callback finally runs: the user's real, later choice already landed.
1907+ const disable = webviewMessageHandler ( mockClineProvider , { type : "telemetrySetting" , text : "disabled" } )
1908+ await disable
1909+
1910+ // Now release the deferred snapshot so a getStateToPostToWebview() call, if the old code
1911+ // path is exercised, resolves (with its already-captured, now-stale value) only after
1912+ // the disable write above has fully landed.
1913+ const snapshotResolved = vi . mocked ( providerForLaunch . getStateToPostToWebview ) . mock . results [ 0 ] ?. value as
1914+ | Promise < unknown >
1915+ | undefined
1916+ releaseSnapshot ( )
1917+
1918+ await Promise . all ( [ launch , snapshotResolved ] )
1919+
1920+ // webviewDidLaunch's telemetry init is fire-and-forget from the handler's own point of
1921+ // view (the "webviewDidLaunch" case doesn't await it), so even awaiting
1922+ // getStateToPostToWebview() directly isn't enough to observe its .then() callback --
1923+ // flush one more microtask turn for that callback to run.
1924+ await Promise . resolve ( )
1925+
1926+ // The user's explicit "disabled" choice must be the final state -- webviewDidLaunch's
1927+ // queued re-application of the (by-then-stale) "unset"/opted-in state must not run after
1928+ // and override it.
1929+ const calls = vi . mocked ( TelemetryService . instance . updateTelemetryState ) . mock . calls
1930+ expect ( calls . at ( - 1 ) ) . toEqual ( [ false ] )
1931+ } )
18201932} )
0 commit comments