From ec76985f4072469762388739f16b1c6904e84519 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Wed, 29 Jul 2026 11:28:57 -0700 Subject: [PATCH] fix(cli): simplify nextInstallState's dead hadFired branch (review nit) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both reviewers (Rames, Magi) independently flagged the same thing: by the time the return statement executes, hadFired is always false — the guard above already returns early for every case where hadFired was true. The merge expression wantFired || hadFired || undefined was defensively correct but misleading; it reads as "OR the two together" when the function has already established only one of them can be true here. Co-Authored-By: Claude Opus 5 (1M context) --- packages/cli/src/telemetry/config.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/telemetry/config.ts b/packages/cli/src/telemetry/config.ts index 51f3da085..7b53c6d8b 100644 --- a/packages/cli/src/telemetry/config.ts +++ b/packages/cli/src/telemetry/config.ts @@ -92,9 +92,13 @@ function writeInstallState(next: InstallState): void { function nextInstallState(state: InstallState | null, wantFired: boolean): InstallState | null { const hadFired = state?.deParallelRouterTrialFired === true; if (state !== null && (hadFired || !wantFired)) return null; + // Every path reaching here has hadFired === false (state is either null, or + // the guard above already returned when hadFired was true) — the field is + // simply wantFired, not a merge of the two (review nit, two independent + // reviewers). return { markerAt: state?.markerAt ?? new Date().toISOString(), - deParallelRouterTrialFired: wantFired || hadFired || undefined, + deParallelRouterTrialFired: wantFired || undefined, }; }