From f8849c4536f106eea7bc2fe00531858a5269adfc Mon Sep 17 00:00:00 2001 From: jnjpng Date: Mon, 16 Mar 2026 17:40:01 -0700 Subject: [PATCH] fix(statusline): re-arm polling when config changes (#1416) Co-authored-by: Letta Code --- src/cli/hooks/useConfigurableStatusLine.ts | 71 +++++++++++++++++---- src/tests/cli/statusline-controller.test.ts | 41 ++++++++++++ 2 files changed, 101 insertions(+), 11 deletions(-) diff --git a/src/cli/hooks/useConfigurableStatusLine.ts b/src/cli/hooks/useConfigurableStatusLine.ts index f201c62..249970c 100644 --- a/src/cli/hooks/useConfigurableStatusLine.ts +++ b/src/cli/hooks/useConfigurableStatusLine.ts @@ -56,6 +56,31 @@ export interface StatusLineInputs { /** ASCII Record Separator used to split left/right column output. */ const RS = "\x1e"; +export interface RefreshIntervalPlan { + shouldClearExistingInterval: boolean; + shouldArmInterval: boolean; + nextRefreshIntervalMs: number | null; +} + +export function buildRefreshIntervalPlan( + armedRefreshIntervalMs: number | null, + desiredRefreshIntervalMs: number | null, +): RefreshIntervalPlan { + if (armedRefreshIntervalMs === desiredRefreshIntervalMs) { + return { + shouldClearExistingInterval: false, + shouldArmInterval: false, + nextRefreshIntervalMs: armedRefreshIntervalMs, + }; + } + + return { + shouldClearExistingInterval: armedRefreshIntervalMs !== null, + shouldArmInterval: desiredRefreshIntervalMs !== null, + nextRefreshIntervalMs: desiredRefreshIntervalMs, + }; +} + export interface StatusLineState { text: string; rightText: string; @@ -116,6 +141,8 @@ export function useConfigurableStatusLine( const refreshIntervalRef = useRef | null>( null, ); + const armedRefreshIntervalMsRef = useRef(null); + const scheduleDebouncedRunRef = useRef<() => void>(() => {}); useEffect(() => { inputsRef.current = inputs; @@ -133,8 +160,31 @@ export function useConfigurableStatusLine( clearInterval(refreshIntervalRef.current); refreshIntervalRef.current = null; } + armedRefreshIntervalMsRef.current = null; }, []); + const reconcileRefreshInterval = useCallback( + (config: NormalizedStatusLineConfig | null) => { + const desiredRefreshIntervalMs = config?.refreshIntervalMs ?? null; + const plan = buildRefreshIntervalPlan( + armedRefreshIntervalMsRef.current, + desiredRefreshIntervalMs, + ); + + if (plan.shouldClearExistingInterval) { + clearRefreshInterval(); + } + + if (plan.shouldArmInterval && plan.nextRefreshIntervalMs !== null) { + refreshIntervalRef.current = setInterval(() => { + scheduleDebouncedRunRef.current(); + }, plan.nextRefreshIntervalMs); + armedRefreshIntervalMsRef.current = plan.nextRefreshIntervalMs; + } + }, + [clearRefreshInterval], + ); + const resolveActiveConfig = useCallback(() => { const workingDirectory = inputsRef.current.currentDirectory; const config = resolveStatusLineConfig(workingDirectory); @@ -151,14 +201,16 @@ export function useConfigurableStatusLine( setText(""); setRightText(""); setPadding(0); + reconcileRefreshInterval(null); return null; } configRef.current = config; setActive(true); setPadding(config.padding); + reconcileRefreshInterval(config); return config; - }, []); + }, [reconcileRefreshInterval]); const executeNow = useCallback(async () => { const config = configRef.current ?? resolveActiveConfig(); @@ -223,6 +275,11 @@ export function useConfigurableStatusLine( const triggerVersion = inputs.triggerVersion; + // Keep polling callbacks pointed at the latest debounced scheduler. + useEffect(() => { + scheduleDebouncedRunRef.current = scheduleDebouncedRun; + }, [scheduleDebouncedRun]); + // Event-driven trigger updates. useEffect(() => { // tie this effect explicitly to triggerVersion for lint + semantics @@ -232,18 +289,11 @@ export function useConfigurableStatusLine( const currentDirectory = inputs.currentDirectory; - // Re-resolve config and optional polling whenever working directory changes. + // Re-resolve config whenever working directory changes. useEffect(() => { // tie this effect explicitly to currentDirectory for lint + semantics void currentDirectory; - const config = resolveActiveConfig(); - - clearRefreshInterval(); - if (config?.refreshIntervalMs) { - refreshIntervalRef.current = setInterval(() => { - scheduleDebouncedRun(); - }, config.refreshIntervalMs); - } + resolveActiveConfig(); return () => { clearRefreshInterval(); @@ -255,7 +305,6 @@ export function useConfigurableStatusLine( clearDebounceTimer, clearRefreshInterval, resolveActiveConfig, - scheduleDebouncedRun, currentDirectory, ]); diff --git a/src/tests/cli/statusline-controller.test.ts b/src/tests/cli/statusline-controller.test.ts index 92a6c7f..7bf111b 100644 --- a/src/tests/cli/statusline-controller.test.ts +++ b/src/tests/cli/statusline-controller.test.ts @@ -3,6 +3,7 @@ import { DEFAULT_STATUS_LINE_DEBOUNCE_MS, normalizeStatusLineConfig, } from "../../cli/helpers/statusLineConfig"; +import { buildRefreshIntervalPlan } from "../../cli/hooks/useConfigurableStatusLine"; describe("statusline controller-related config", () => { test("normalizes debounce and refresh interval defaults", () => { @@ -29,3 +30,43 @@ describe("statusline controller-related config", () => { expect(normalized.debounceMs).toBe(50); }); }); + +describe("buildRefreshIntervalPlan", () => { + test("returns no-op when interval is unchanged", () => { + expect(buildRefreshIntervalPlan(null, null)).toEqual({ + shouldClearExistingInterval: false, + shouldArmInterval: false, + nextRefreshIntervalMs: null, + }); + + expect(buildRefreshIntervalPlan(5000, 5000)).toEqual({ + shouldClearExistingInterval: false, + shouldArmInterval: false, + nextRefreshIntervalMs: 5000, + }); + }); + + test("arms interval when moving from off to polling", () => { + expect(buildRefreshIntervalPlan(null, 5000)).toEqual({ + shouldClearExistingInterval: false, + shouldArmInterval: true, + nextRefreshIntervalMs: 5000, + }); + }); + + test("re-arms interval when polling cadence changes", () => { + expect(buildRefreshIntervalPlan(5000, 1000)).toEqual({ + shouldClearExistingInterval: true, + shouldArmInterval: true, + nextRefreshIntervalMs: 1000, + }); + }); + + test("clears interval when polling is disabled", () => { + expect(buildRefreshIntervalPlan(5000, null)).toEqual({ + shouldClearExistingInterval: true, + shouldArmInterval: false, + nextRefreshIntervalMs: null, + }); + }); +});