From 95dbba8f8a327f1231504fd901c49a5896167320 Mon Sep 17 00:00:00 2001 From: KazenDev Date: Tue, 15 Sep 2026 05:49:04 -0500 Subject: [PATCH] fix(cli): require two consecutive failed health checks before reporting a disconnection --- .../__tests__/use-connection-status.test.ts | 64 ++++++++++++++++++- cli/src/hooks/use-connection-status.ts | 59 ++++++++++++++--- 2 files changed, 113 insertions(+), 10 deletions(-) diff --git a/cli/src/hooks/__tests__/use-connection-status.test.ts b/cli/src/hooks/__tests__/use-connection-status.test.ts index ec188f2496..b1f4e9b36b 100644 --- a/cli/src/hooks/__tests__/use-connection-status.test.ts +++ b/cli/src/hooks/__tests__/use-connection-status.test.ts @@ -1,6 +1,9 @@ import { describe, test, expect } from 'bun:test' -import { getNextInterval } from '../use-connection-status' +import { + createProbeFailureTracker, + getNextInterval, +} from '../use-connection-status' /** * Tests for the adaptive health check interval logic. @@ -109,4 +112,63 @@ describe('useConnectionStatus - adaptive interval logic', () => { expect(getNextInterval(20)).toBe(600_000) }) }) + + describe('disconnect hysteresis', () => { + test('a single failed probe does not report a disconnection', () => { + const tracker = createProbeFailureTracker() + + expect(tracker.recordFailure()).toBe(false) + }) + + test('a sustained streak still reports a disconnection', () => { + const tracker = createProbeFailureTracker() + + expect(tracker.recordFailure()).toBe(false) + expect(tracker.recordFailure()).toBe(true) + // Stays reported while the outage lasts; it is not a one-shot signal. + expect(tracker.recordFailure()).toBe(true) + }) + + test('a sequence of isolated blips never flips the badge', () => { + const tracker = createProbeFailureTracker() + const reportedDisconnected: boolean[] = [] + + // fail → success → fail → success → fail → success + for (let i = 0; i < 3; i++) { + reportedDisconnected.push(tracker.recordFailure()) + tracker.recordSuccess() + } + + expect(reportedDisconnected).toEqual([false, false, false]) + }) + + test('recovery is immediate: one success clears the streak', () => { + const tracker = createProbeFailureTracker() + tracker.recordFailure() + tracker.recordFailure() + + tracker.recordSuccess() + + expect(tracker.consecutiveFailures).toBe(0) + // The cleared streak means the next blip is tolerated again. + expect(tracker.recordFailure()).toBe(false) + }) + + test('the streak still measures the length of the outage for backoff', () => { + const tracker = createProbeFailureTracker() + expect(tracker.consecutiveFailures).toBe(0) + + tracker.recordFailure() + tracker.recordFailure() + tracker.recordFailure() + + expect(tracker.consecutiveFailures).toBe(3) + }) + + test('threshold 1 reproduces the old notify-on-first-failure behaviour', () => { + const tracker = createProbeFailureTracker({ threshold: 1 }) + + expect(tracker.recordFailure()).toBe(true) + }) + }) }) diff --git a/cli/src/hooks/use-connection-status.ts b/cli/src/hooks/use-connection-status.ts index 20647412c3..a7b3b9df87 100644 --- a/cli/src/hooks/use-connection-status.ts +++ b/cli/src/hooks/use-connection-status.ts @@ -38,6 +38,44 @@ export function getNextInterval(consecutiveSuccesses: number): number { return HEALTH_CHECK_CONFIG.INITIAL_INTERVAL } +/** + * Number of consecutive failed health checks required before the CLI reports + * itself as disconnected. + */ +export const DISCONNECT_FAILURE_THRESHOLD = 2 + +/** + * Tracks consecutive failed health probes across the hook's lifetime. + * + * `recordFailure` returns whether the badge may flip to "connecting": a single + * failed probe is not evidence of a disconnection. Transient blips (a slow + * proxy, one dropped packet, a momentary 5xx) would otherwise paint a false + * badge and, because failures back off exponentially, leave that wrong state on + * screen for minutes. + * + * Recovery is deliberately not hysteretic: a success always clears the streak, + * so coming back feels instant. Slow to alarm, fast to clear. + * + * Exported for testing purposes. + */ +export function createProbeFailureTracker({ + threshold = DISCONNECT_FAILURE_THRESHOLD, +}: { threshold?: number } = {}) { + let consecutiveFailures = 0 + return { + get consecutiveFailures() { + return consecutiveFailures + }, + recordFailure(): boolean { + consecutiveFailures += 1 + return consecutiveFailures >= threshold + }, + recordSuccess(): void { + consecutiveFailures = 0 + }, + } +} + /** * Hook to monitor connection status to the Codebuff backend. * Jitters the adaptive healthy cadence and exponentially backs off failures so @@ -68,7 +106,7 @@ export const useConnectionStatus = ( let isMounted = true let timeoutId: NodeJS.Timeout | null = null let consecutiveSuccesses = 0 - let consecutiveFailures = 0 + const probeFailures = createProbeFailureTracker() const scheduleNextCheck = (interval: number) => { if (!isMounted) return @@ -77,18 +115,19 @@ export const useConnectionStatus = ( const scheduleFailedCheck = (message: string, error?: unknown): void => { if (!isMounted) return - setIsConnected(false) - previousConnectedRef.current = false consecutiveSuccesses = 0 - consecutiveFailures++ + if (probeFailures.recordFailure()) { + setIsConnected(false) + previousConnectedRef.current = false + } const delayMs = failedPollDelayMs({ - consecutiveFailures, + consecutiveFailures: probeFailures.consecutiveFailures, }) logger.debug( { ...(error === undefined ? {} : { error }), delayMs, - consecutiveFailures, + consecutiveFailures: probeFailures.consecutiveFailures, }, message, ) @@ -107,11 +146,11 @@ export const useConnectionStatus = ( if (!isMounted) return const prevConnected = previousConnectedRef.current - setIsConnected(connected) - previousConnectedRef.current = connected if (connected) { - consecutiveFailures = 0 + probeFailures.recordSuccess() + setIsConnected(true) + previousConnectedRef.current = true // Determine if this is the initial connection (null) or a reconnection (false) const isInitialConnection = prevConnected === null const shouldFireReconnectCallback = @@ -131,6 +170,8 @@ export const useConnectionStatus = ( }), ) } else { + // The badge is flipped by scheduleFailedCheck alone, so a single + // failed probe cannot report a disconnection by itself. scheduleFailedCheck('Health check failed, backing off') } } catch (error) {