From 3fcf8c0fd04140cbb627dc6f278de56324f2f2df Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 16 Nov 2025 17:44:43 -0600 Subject: [PATCH 1/3] fix: prevent retry barrier flashing --- docs/AGENTS.md | 1 + src/browser/components/ChatInput/index.tsx | 10 ++++++++++ src/browser/stores/WorkspaceStore.ts | 14 ++++++++++++++ .../messages/StreamingMessageAggregator.ts | 12 ++++++++++++ .../utils/messages/retryEligibility.test.ts | 19 ++++++++++--------- .../utils/messages/retryEligibility.ts | 10 ++++++---- 6 files changed, 53 insertions(+), 13 deletions(-) diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 496e283a4..1eb92520f 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -152,4 +152,5 @@ gh pr view --json mergeable,mergeStateStatus | jq '.' ## Mode: Plan +- When Plan Mode is requested, assume the user wants the actual completed plan; do not merely describe how you would devise one. - Attach a net LoC estimate (product code only) to each recommended approach. diff --git a/src/browser/components/ChatInput/index.tsx b/src/browser/components/ChatInput/index.tsx index 2dd851af9..eda896645 100644 --- a/src/browser/components/ChatInput/index.tsx +++ b/src/browser/components/ChatInput/index.tsx @@ -62,6 +62,7 @@ import { setTelemetryEnabled } from "@/common/telemetry"; import { getTokenCountPromise } from "@/browser/utils/tokenizer/rendererClient"; import { CreationCenterContent } from "./CreationCenterContent"; import { CreationControls } from "./CreationControls"; +import { useWorkspaceStoreRaw } from "@/browser/stores/WorkspaceStore"; import { useCreationWorkspace } from "./useCreationWorkspace"; type TokenCountReader = () => number; @@ -136,6 +137,7 @@ export const ChatInput: React.FC = (props) => { const [mode, setMode] = useMode(); const { recentModels, addModel, evictModel } = useModelLRU(); const commandListId = useId(); + const workspaceStore = useWorkspaceStoreRaw(); const telemetry = useTelemetry(); const [vimEnabled, setVimEnabled] = usePersistedState(VIM_ENABLED_KEY, false, { listener: true, @@ -443,6 +445,12 @@ export const ChatInput: React.FC = (props) => { // Workspace variant: full command handling + message send if (variant !== "workspace") return; // Type guard + const notifyPendingSendFailed = () => { + setTimeout(() => { + workspaceStore.markPendingStreamFailed(props.workspaceId); + }, 0); + }; + try { // Parse command const parsed = parseCommand(messageText); @@ -716,6 +724,7 @@ export const ChatInput: React.FC = (props) => { setToast(createErrorToast(result.error)); // Restore input and images on error so user can try again setInput(messageText); + notifyPendingSendFailed(); setImageAttachments(previousImageAttachments); } else { // Track telemetry for successful message send @@ -736,6 +745,7 @@ export const ChatInput: React.FC = (props) => { raw: error instanceof Error ? error.message : "Failed to send message", }) ); + notifyPendingSendFailed(); setInput(messageText); setImageAttachments(previousImageAttachments); } finally { diff --git a/src/browser/stores/WorkspaceStore.ts b/src/browser/stores/WorkspaceStore.ts index 7dfe3be2d..d81ef143b 100644 --- a/src/browser/stores/WorkspaceStore.ts +++ b/src/browser/stores/WorkspaceStore.ts @@ -365,6 +365,20 @@ export class WorkspaceStore { return allStates; } + /** + * Notify the aggregator that a pending stream never started (renderer send failure). + */ + markPendingStreamFailed(workspaceId: string): void { + const aggregator = this.aggregators.get(workspaceId); + if (!aggregator) { + return; + } + + if (aggregator.markPendingStreamStartFailed()) { + this.states.bump(workspaceId); + } + } + /** * Get recency timestamps for all workspaces (for sorting in command palette). * Derived on-demand from individual workspace states. diff --git a/src/browser/utils/messages/StreamingMessageAggregator.ts b/src/browser/utils/messages/StreamingMessageAggregator.ts index fcdd02d91..f4483a58e 100644 --- a/src/browser/utils/messages/StreamingMessageAggregator.ts +++ b/src/browser/utils/messages/StreamingMessageAggregator.ts @@ -267,6 +267,18 @@ export class StreamingMessageAggregator { return this.pendingStreamStartTime; } + + /** + * Clear pending stream-start tracking when send fails before we ever receive stream-start. + * Returns true when the internal state changed so callers can trigger updates. + */ + markPendingStreamStartFailed(): boolean { + if (this.pendingStreamStartTime === null) { + return false; + } + this.pendingStreamStartTime = null; + return true; + } private setPendingStreamStartTime(time: number | null): void { this.pendingStreamStartTime = time; } diff --git a/src/browser/utils/messages/retryEligibility.test.ts b/src/browser/utils/messages/retryEligibility.test.ts index fb8d1be51..403f1488a 100644 --- a/src/browser/utils/messages/retryEligibility.test.ts +++ b/src/browser/utils/messages/retryEligibility.test.ts @@ -3,6 +3,7 @@ import { hasInterruptedStream, isEligibleForAutoRetry, isNonRetryableSendError, + PENDING_STREAM_START_GRACE_PERIOD_MS, } from "./retryEligibility"; import type { DisplayedMessage } from "@/common/types/message"; import type { SendMessageError } from "@/common/types/errors"; @@ -165,7 +166,7 @@ describe("hasInterruptedStream", () => { expect(hasInterruptedStream(messages, null)).toBe(true); }); - it("returns false when message was sent very recently (< 3s)", () => { + it("returns false when message was sent very recently (within grace period)", () => { const messages: DisplayedMessage[] = [ { type: "user", @@ -194,8 +195,8 @@ describe("hasInterruptedStream", () => { historySequence: 3, }, ]; - // Message sent 1 second ago - still within 3s window - const recentTimestamp = Date.now() - 1000; + // Message sent 1 second ago - still within grace window + const recentTimestamp = Date.now() - (PENDING_STREAM_START_GRACE_PERIOD_MS - 1000); expect(hasInterruptedStream(messages, recentTimestamp)).toBe(false); }); @@ -212,7 +213,7 @@ describe("hasInterruptedStream", () => { expect(hasInterruptedStream(messages, null)).toBe(true); }); - it("returns false when user message just sent (< 3s ago)", () => { + it("returns false when user message just sent (within grace period)", () => { const messages: DisplayedMessage[] = [ { type: "user", @@ -222,11 +223,11 @@ describe("hasInterruptedStream", () => { historySequence: 1, }, ]; - const justSent = Date.now() - 500; // 0.5s ago + const justSent = Date.now() - (PENDING_STREAM_START_GRACE_PERIOD_MS - 500); expect(hasInterruptedStream(messages, justSent)).toBe(false); }); - it("returns true when message sent over 3s ago (stream likely hung)", () => { + it("returns true when message sent beyond grace period (stream likely hung)", () => { const messages: DisplayedMessage[] = [ { type: "user", @@ -236,7 +237,7 @@ describe("hasInterruptedStream", () => { historySequence: 1, }, ]; - const longAgo = Date.now() - 4000; // 4s ago - past 3s threshold + const longAgo = Date.now() - (PENDING_STREAM_START_GRACE_PERIOD_MS + 1000); expect(hasInterruptedStream(messages, longAgo)).toBe(true); }); @@ -545,7 +546,7 @@ describe("isEligibleForAutoRetry", () => { expect(isEligibleForAutoRetry(messages, null)).toBe(true); }); - it("returns false when user message sent very recently (< 3s)", () => { + it("returns false when user message sent very recently (within grace period)", () => { const messages: DisplayedMessage[] = [ { type: "user", @@ -555,7 +556,7 @@ describe("isEligibleForAutoRetry", () => { historySequence: 1, }, ]; - const justSent = Date.now() - 500; // 0.5s ago + const justSent = Date.now() - (PENDING_STREAM_START_GRACE_PERIOD_MS - 500); expect(isEligibleForAutoRetry(messages, justSent)).toBe(false); }); }); diff --git a/src/browser/utils/messages/retryEligibility.ts b/src/browser/utils/messages/retryEligibility.ts index 3d5f6fff2..eed8ec69f 100644 --- a/src/browser/utils/messages/retryEligibility.ts +++ b/src/browser/utils/messages/retryEligibility.ts @@ -18,6 +18,8 @@ declare global { } } +export const PENDING_STREAM_START_GRACE_PERIOD_MS = 15000; // 15 seconds + /** * Check if the debug flag to force all errors to be retryable is enabled */ @@ -69,7 +71,7 @@ export function isNonRetryableSendError(error: SendMessageError): boolean { * 3. Last message is a user message (indicating we sent it but never got a response) * - This handles app restarts during slow model responses (models can take 30-60s to first token) * - User messages are only at the end when response hasn't started/completed - * - EXCEPT: Not if recently sent (<3s ago) - prevents flash during normal send flow + * - EXCEPT: Not if recently sent (within PENDING_STREAM_START_GRACE_PERIOD_MS) - prevents flash during normal send flow */ export function hasInterruptedStream( messages: DisplayedMessage[], @@ -77,12 +79,12 @@ export function hasInterruptedStream( ): boolean { if (messages.length === 0) return false; - // Don't show retry barrier if user message was sent very recently (< 3s) + // Don't show retry barrier if user message was sent very recently (within the grace period) // This prevents flash during normal send flow while stream-start event arrives - // After 3s, we assume something is wrong and show the barrier + // After the grace period, assume something is wrong and show the barrier if (pendingStreamStartTime !== null) { const elapsed = Date.now() - pendingStreamStartTime; - if (elapsed < 3000) return false; + if (elapsed < PENDING_STREAM_START_GRACE_PERIOD_MS) return false; } const lastMessage = messages[messages.length - 1]; From bf803a5a1bd9c755831e4ccb2fba831638037541 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 16 Nov 2025 17:57:57 -0600 Subject: [PATCH 2/3] chore: format StreamingMessageAggregator --- src/browser/utils/messages/StreamingMessageAggregator.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/browser/utils/messages/StreamingMessageAggregator.ts b/src/browser/utils/messages/StreamingMessageAggregator.ts index f4483a58e..709c61df5 100644 --- a/src/browser/utils/messages/StreamingMessageAggregator.ts +++ b/src/browser/utils/messages/StreamingMessageAggregator.ts @@ -267,7 +267,6 @@ export class StreamingMessageAggregator { return this.pendingStreamStartTime; } - /** * Clear pending stream-start tracking when send fails before we ever receive stream-start. * Returns true when the internal state changed so callers can trigger updates. From 34b71eca3a7b262087d601ff73b3682b4732f1db Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 16 Nov 2025 18:09:32 -0600 Subject: [PATCH 3/3] fix: keep pending stream timestamp until retry --- src/browser/components/ChatInput/index.tsx | 10 ---------- src/browser/stores/WorkspaceStore.ts | 14 -------------- .../utils/messages/StreamingMessageAggregator.ts | 13 ++----------- 3 files changed, 2 insertions(+), 35 deletions(-) diff --git a/src/browser/components/ChatInput/index.tsx b/src/browser/components/ChatInput/index.tsx index eda896645..2dd851af9 100644 --- a/src/browser/components/ChatInput/index.tsx +++ b/src/browser/components/ChatInput/index.tsx @@ -62,7 +62,6 @@ import { setTelemetryEnabled } from "@/common/telemetry"; import { getTokenCountPromise } from "@/browser/utils/tokenizer/rendererClient"; import { CreationCenterContent } from "./CreationCenterContent"; import { CreationControls } from "./CreationControls"; -import { useWorkspaceStoreRaw } from "@/browser/stores/WorkspaceStore"; import { useCreationWorkspace } from "./useCreationWorkspace"; type TokenCountReader = () => number; @@ -137,7 +136,6 @@ export const ChatInput: React.FC = (props) => { const [mode, setMode] = useMode(); const { recentModels, addModel, evictModel } = useModelLRU(); const commandListId = useId(); - const workspaceStore = useWorkspaceStoreRaw(); const telemetry = useTelemetry(); const [vimEnabled, setVimEnabled] = usePersistedState(VIM_ENABLED_KEY, false, { listener: true, @@ -445,12 +443,6 @@ export const ChatInput: React.FC = (props) => { // Workspace variant: full command handling + message send if (variant !== "workspace") return; // Type guard - const notifyPendingSendFailed = () => { - setTimeout(() => { - workspaceStore.markPendingStreamFailed(props.workspaceId); - }, 0); - }; - try { // Parse command const parsed = parseCommand(messageText); @@ -724,7 +716,6 @@ export const ChatInput: React.FC = (props) => { setToast(createErrorToast(result.error)); // Restore input and images on error so user can try again setInput(messageText); - notifyPendingSendFailed(); setImageAttachments(previousImageAttachments); } else { // Track telemetry for successful message send @@ -745,7 +736,6 @@ export const ChatInput: React.FC = (props) => { raw: error instanceof Error ? error.message : "Failed to send message", }) ); - notifyPendingSendFailed(); setInput(messageText); setImageAttachments(previousImageAttachments); } finally { diff --git a/src/browser/stores/WorkspaceStore.ts b/src/browser/stores/WorkspaceStore.ts index d81ef143b..7dfe3be2d 100644 --- a/src/browser/stores/WorkspaceStore.ts +++ b/src/browser/stores/WorkspaceStore.ts @@ -365,20 +365,6 @@ export class WorkspaceStore { return allStates; } - /** - * Notify the aggregator that a pending stream never started (renderer send failure). - */ - markPendingStreamFailed(workspaceId: string): void { - const aggregator = this.aggregators.get(workspaceId); - if (!aggregator) { - return; - } - - if (aggregator.markPendingStreamStartFailed()) { - this.states.bump(workspaceId); - } - } - /** * Get recency timestamps for all workspaces (for sorting in command palette). * Derived on-demand from individual workspace states. diff --git a/src/browser/utils/messages/StreamingMessageAggregator.ts b/src/browser/utils/messages/StreamingMessageAggregator.ts index 709c61df5..e0d1193e1 100644 --- a/src/browser/utils/messages/StreamingMessageAggregator.ts +++ b/src/browser/utils/messages/StreamingMessageAggregator.ts @@ -96,6 +96,8 @@ export class StreamingMessageAggregator { // Track when we're waiting for stream-start after user message // Prevents retry barrier flash during normal send flow // Stores timestamp of when user message was sent (null = no pending stream) + // IMPORTANT: We intentionally keep this timestamp until a stream actually starts + // (or the user retries) so retry UI/backoff logic doesn't misfire on send failures. private pendingStreamStartTime: number | null = null; // Workspace creation timestamp (used for recency calculation) @@ -267,17 +269,6 @@ export class StreamingMessageAggregator { return this.pendingStreamStartTime; } - /** - * Clear pending stream-start tracking when send fails before we ever receive stream-start. - * Returns true when the internal state changed so callers can trigger updates. - */ - markPendingStreamStartFailed(): boolean { - if (this.pendingStreamStartTime === null) { - return false; - } - this.pendingStreamStartTime = null; - return true; - } private setPendingStreamStartTime(time: number | null): void { this.pendingStreamStartTime = time; }