diff --git a/src/browser/App.tsx b/src/browser/App.tsx index 3267245f95..c6739f217e 100644 --- a/src/browser/App.tsx +++ b/src/browser/App.tsx @@ -17,6 +17,7 @@ import { buildSortedWorkspacesByProject } from "./utils/ui/workspaceFiltering"; import { useResumeManager } from "./hooks/useResumeManager"; import { useUnreadTracking } from "./hooks/useUnreadTracking"; import { useWorkspaceStoreRaw, useWorkspaceRecency } from "./stores/WorkspaceStore"; +import { setActiveWorkspace } from "./stores/GitStatusStore"; import { useStableReference, compareMaps } from "./hooks/useStableReference"; import { CommandRegistryProvider, useCommandRegistry } from "./contexts/CommandRegistryContext"; @@ -133,6 +134,11 @@ function AppInner() { prevWorkspaceRef.current = selectedWorkspace; }, [selectedWorkspace, telemetry]); + // Tell GitStatusStore which workspace is active (for prioritized 1s refresh) + useEffect(() => { + setActiveWorkspace(selectedWorkspace?.workspaceId ?? null); + }, [selectedWorkspace?.workspaceId]); + // Track last-read timestamps for unread indicators const { lastReadTimestamps, onToggleUnread } = useUnreadTracking(selectedWorkspace); diff --git a/src/browser/components/AIView.tsx b/src/browser/components/AIView.tsx index b2f4e3305e..8153cd8c9b 100644 --- a/src/browser/components/AIView.tsx +++ b/src/browser/components/AIView.tsx @@ -138,6 +138,7 @@ const AIViewInner: React.FC = ({ const storeRaw = useWorkspaceStoreRaw(); const meta = workspaceMetadata.get(workspaceId); const isQueuedAgentTask = Boolean(meta?.parentWorkspaceId) && meta?.taskStatus === "queued"; + const isReportedAgentTask = Boolean(meta?.parentWorkspaceId) && meta?.taskStatus === "reported"; const queuedAgentTaskPrompt = isQueuedAgentTask && typeof meta?.taskPrompt === "string" && meta.taskPrompt.trim().length > 0 ? meta.taskPrompt @@ -760,6 +761,11 @@ const AIViewInner: React.FC = ({ available. )} + {isReportedAgentTask && ( +
+ Completed — this agent task workspace will be deleted in 5 seconds. +
+ )} = ({ onMessageSent={handleMessageSent} onTruncateHistory={handleClearHistory} onProviderConfig={handleProviderConfig} - disabled={!projectName || !workspaceName || isQueuedAgentTask} + disabled={!projectName || !workspaceName || isQueuedAgentTask || isReportedAgentTask} disabledReason={ isQueuedAgentTask ? "Queued — waiting for an available parallel task slot. This will start automatically." - : undefined + : isReportedAgentTask + ? "Completed — this agent task workspace will be deleted in 5 seconds." + : undefined } isCompacting={isCompacting} editingMessage={editingMessage} diff --git a/src/browser/components/Messages/ToolMessage.tsx b/src/browser/components/Messages/ToolMessage.tsx index 44039a328a..f5f81d40e9 100644 --- a/src/browser/components/Messages/ToolMessage.tsx +++ b/src/browser/components/Messages/ToolMessage.tsx @@ -8,6 +8,7 @@ import { FileReadToolCall } from "../tools/FileReadToolCall"; import { AskUserQuestionToolCall } from "../tools/AskUserQuestionToolCall"; import { ProposePlanToolCall } from "../tools/ProposePlanToolCall"; import { TodoToolCall } from "../tools/TodoToolCall"; +import { AgentReportToolCall } from "../tools/AgentReportToolCall"; import { StatusSetToolCall } from "../tools/StatusSetToolCall"; import { WebFetchToolCall } from "../tools/WebFetchToolCall"; import { BashBackgroundListToolCall } from "../tools/BashBackgroundListToolCall"; @@ -436,6 +437,14 @@ export const ToolMessage: React.FC = ({ ); } + if (message.toolName === "agent_report") { + return ( +
+ +
+ ); + } + // Fallback to generic tool call return (
diff --git a/src/browser/components/RightSidebar/CodeReview/RefreshButton.tsx b/src/browser/components/RightSidebar/CodeReview/RefreshButton.tsx index fa7eb24a68..96e146a685 100644 --- a/src/browser/components/RightSidebar/CodeReview/RefreshButton.tsx +++ b/src/browser/components/RightSidebar/CodeReview/RefreshButton.tsx @@ -5,14 +5,30 @@ import React, { useState, useRef, useEffect } from "react"; import { Tooltip, TooltipTrigger, TooltipContent } from "@/browser/components/ui/tooltip"; import { formatKeybind, KEYBINDS } from "@/browser/utils/ui/keybinds"; +import { formatRelativeTimeCompact } from "@/browser/utils/ui/dateTime"; import { cn } from "@/common/lib/utils"; +import type { LastRefreshInfo, RefreshTrigger } from "@/browser/utils/RefreshController"; interface RefreshButtonProps { onClick: () => void; isLoading?: boolean; + /** Debug info about last refresh (timestamp and trigger) */ + lastRefreshInfo?: LastRefreshInfo | null; } -export const RefreshButton: React.FC = ({ onClick, isLoading = false }) => { +/** Human-readable trigger labels */ +const TRIGGER_LABELS: Record = { + manual: "manual click", + scheduled: "tool completion", + priority: "tool completion (priority)", + focus: "window focus", + visibility: "tab visible", + unpaused: "interaction ended", + "in-flight-followup": "queued followup", +}; + +export const RefreshButton: React.FC = (props) => { + const { onClick, isLoading = false, lastRefreshInfo } = props; // Track animation state for graceful stopping const [animationState, setAnimationState] = useState<"idle" | "spinning" | "stopping">("idle"); const spinOnceTimeoutRef = useRef(null); @@ -76,9 +92,19 @@ export const RefreshButton: React.FC = ({ onClick, isLoading - {animationState !== "idle" - ? "Refreshing..." - : `Refresh diff (${formatKeybind(KEYBINDS.REFRESH_REVIEW)})`} + {animationState !== "idle" ? ( + "Refreshing..." + ) : ( + + Refresh diff ({formatKeybind(KEYBINDS.REFRESH_REVIEW)}) + {lastRefreshInfo && ( + + Last: {formatRelativeTimeCompact(lastRefreshInfo.timestamp)} via{" "} + {TRIGGER_LABELS[lastRefreshInfo.trigger] ?? lastRefreshInfo.trigger} + + )} + + )} ); diff --git a/src/browser/components/RightSidebar/CodeReview/ReviewControls.tsx b/src/browser/components/RightSidebar/CodeReview/ReviewControls.tsx index 6946a1332e..beb87c4250 100644 --- a/src/browser/components/RightSidebar/CodeReview/ReviewControls.tsx +++ b/src/browser/components/RightSidebar/CodeReview/ReviewControls.tsx @@ -5,6 +5,7 @@ import React, { useState } from "react"; import { usePersistedState } from "@/browser/hooks/usePersistedState"; import type { ReviewFilters, ReviewStats } from "@/common/types/review"; +import type { LastRefreshInfo } from "@/browser/utils/RefreshController"; import { RefreshButton } from "./RefreshButton"; import { UntrackedStatus } from "./UntrackedStatus"; @@ -17,6 +18,8 @@ interface ReviewControlsProps { workspaceId: string; workspacePath: string; refreshTrigger?: number; + /** Debug info about last refresh */ + lastRefreshInfo?: LastRefreshInfo | null; } export const ReviewControls: React.FC = ({ @@ -28,6 +31,7 @@ export const ReviewControls: React.FC = ({ workspaceId, workspacePath, refreshTrigger, + lastRefreshInfo, }) => { // Local state for input value - only commit on blur/Enter const [inputValue, setInputValue] = useState(filters.diffBase); @@ -83,7 +87,13 @@ export const ReviewControls: React.FC = ({ return (
- {onRefresh && } + {onRefresh && ( + + )} [h.id, h])); + let allSame = prev.length === next.length; + + const result = next.map((hunk, i) => { + const prevHunk = prevById.get(hunk.id); + // Fast path: same ID and content means unchanged (content hash is part of ID) + if (prevHunk && prevHunk.content === hunk.content) { + if (allSame && prev[i]?.id !== hunk.id) allSame = false; + return prevHunk; + } + allSame = false; + return hunk; + }); + + // If all hunks are reused in same order, return prev array to preserve top-level reference + return allSame ? prev : result; +} + interface ReviewPanelDiffCacheValue { hunks: DiffHunk[]; truncationWarning: string | null; @@ -118,6 +143,53 @@ const reviewPanelCache = new LRUCache({ sizeCalculation: (value) => estimateJsonSizeBytes(value), }); +function getOriginBranchForFetch(diffBase: string): string | null { + const trimmed = diffBase.trim(); + if (!trimmed.startsWith("origin/")) return null; + + const branch = trimmed.slice("origin/".length); + + // Avoid shell injection; diffBase is user-controlled. + if (!/^[0-9A-Za-z._/-]+$/.test(branch)) return null; + + return branch; +} + +interface OriginFetchState { + key: string; + promise: Promise; +} + +async function ensureOriginFetched(params: { + api: APIClient; + workspaceId: string; + diffBase: string; + refreshToken: number; + originFetchRef: React.MutableRefObject; +}): Promise { + const originBranch = getOriginBranchForFetch(params.diffBase); + if (!originBranch) return; + + const key = [params.workspaceId, params.diffBase, String(params.refreshToken)].join("\u0000"); + const existing = params.originFetchRef.current; + if (existing?.key === key) { + await existing.promise; + return; + } + + // Ensure manual refresh doesn't hang on credential prompts. + const promise = params.api.workspace + .executeBash({ + workspaceId: params.workspaceId, + script: `GIT_TERMINAL_PROMPT=0 git fetch origin ${originBranch} --quiet || true`, + options: { timeout_secs: 30 }, + }) + .then(() => undefined) + .catch(() => undefined); + + params.originFetchRef.current = { key, promise }; + await promise; +} function makeReviewPanelCacheKey(params: { workspaceId: string; workspacePath: string; @@ -161,6 +233,7 @@ export const ReviewPanel: React.FC = ({ isCreating = false, onStatsChange, }) => { + const originFetchRef = useRef(null); const { api } = useAPI(); const panelRef = useRef(null); const scrollContainerRef = useRef(null); @@ -309,6 +382,15 @@ export const ReviewPanel: React.FC = ({ const loadFileTree = async () => { setIsLoadingTree(true); try { + await ensureOriginFetched({ + api, + workspaceId, + diffBase: filters.diffBase, + refreshToken: refreshTrigger, + originFetchRef, + }); + if (cancelled) return; + const tree = await executeWorkspaceBashAndCache({ api, workspaceId, @@ -420,6 +502,15 @@ export const ReviewPanel: React.FC = ({ const loadDiff = async () => { try { + await ensureOriginFetched({ + api, + workspaceId, + diffBase: filters.diffBase, + refreshToken: refreshTrigger, + originFetchRef, + }); + if (cancelled) return; + // Git-level filters (affect what data is fetched): // - diffBase: what to diff against // - includeUncommitted: include working directory changes @@ -458,10 +549,19 @@ export const ReviewPanel: React.FC = ({ if (cancelled) return; setDiagnosticInfo(data.diagnosticInfo); - setDiffState({ - status: "loaded", - hunks: data.hunks, - truncationWarning: data.truncationWarning, + + // Preserve object references for unchanged hunks to prevent unnecessary re-renders. + // HunkViewer is memoized on hunk object identity, so reusing references avoids + // re-rendering (and re-highlighting) hunks that haven't actually changed. + setDiffState((prev) => { + const prevHunks = + prev.status === "loaded" || prev.status === "refreshing" ? prev.hunks : []; + const hunks = preserveHunkReferences(prevHunks, data.hunks); + return { + status: "loaded", + hunks, + truncationWarning: data.truncationWarning, + }; }); if (data.hunks.length > 0) { @@ -811,6 +911,7 @@ export const ReviewPanel: React.FC = ({ workspaceId={workspaceId} workspacePath={workspacePath} refreshTrigger={refreshTrigger} + lastRefreshInfo={refreshController.lastRefreshInfo} /> {diffState.status === "error" ? ( diff --git a/src/browser/components/tools/AgentReportToolCall.tsx b/src/browser/components/tools/AgentReportToolCall.tsx new file mode 100644 index 0000000000..c7b0852042 --- /dev/null +++ b/src/browser/components/tools/AgentReportToolCall.tsx @@ -0,0 +1,86 @@ +import React from "react"; +import { + ToolContainer, + ToolHeader, + ExpandIcon, + ToolName, + StatusIndicator, + ToolDetails, +} from "./shared/ToolPrimitives"; +import { getStatusDisplay, useToolExpansion, type ToolStatus } from "./shared/toolUtils"; +import { CompactingMessageContent } from "../Messages/CompactingMessageContent"; +import { MarkdownRenderer } from "../Messages/MarkdownRenderer"; + +interface AgentReportToolCallProps { + args?: unknown; + result?: unknown; + status?: ToolStatus; +} + +function extractAgentReportFields(args: unknown): { reportMarkdown: string; title?: string } { + if (typeof args !== "object" || args === null) { + return { reportMarkdown: "" }; + } + + const maybe = args as { reportMarkdown?: unknown; title?: unknown }; + const reportMarkdown = typeof maybe.reportMarkdown === "string" ? maybe.reportMarkdown : ""; + const title = + typeof maybe.title === "string" && maybe.title.trim().length > 0 ? maybe.title : undefined; + + return { reportMarkdown, title }; +} + +function inferTitleFromMarkdown(markdown: string): string | undefined { + const match = /^#\s+(.+)$/m.exec(markdown); + const title = match?.[1]?.trim(); + return title && title.length > 0 ? title : undefined; +} + +export const AgentReportToolCall: React.FC = ({ + args, + result: _result, + status = "pending", +}) => { + const { reportMarkdown, title: titleFromArgs } = extractAgentReportFields(args); + const title = titleFromArgs ?? inferTitleFromMarkdown(reportMarkdown) ?? "Report"; + + // Expand by default while executing so the user sees the report being written. + const { expanded, toggleExpanded } = useToolExpansion(status === "executing"); + const statusDisplay = getStatusDisplay(status); + + const hasContent = reportMarkdown.trim().length > 0; + + const content = + status === "executing" ? ( + + {hasContent ? ( + 
+            {reportMarkdown}
+
+ ) : ( +
Writing report...
+ )} + + ) : hasContent ? ( + + ) : ( +
No report content
+ ); + + return ( + + + ▶ + agent_report + {statusDisplay} + + + {expanded && ( + +
{title}
+ {content} + + )} + + ); +}; diff --git a/src/browser/contexts/WorkspaceContext.tsx b/src/browser/contexts/WorkspaceContext.tsx index f32fd45fe5..13ca8df95e 100644 --- a/src/browser/contexts/WorkspaceContext.tsx +++ b/src/browser/contexts/WorkspaceContext.tsx @@ -31,6 +31,12 @@ import { isExperimentEnabled } from "@/browser/hooks/useExperiments"; import { EXPERIMENT_IDS } from "@/common/constants/experiments"; import { isWorkspaceArchived } from "@/common/utils/archive"; +// Keep in sync with backend auto-delete delay for reported task workspaces. +// +// Even if the backend deletes a task workspace quickly, we keep it visible for at least this +// duration after it reports so the user can read the final report/tool output. +const REPORTED_TASK_WORKSPACE_AUTO_DELETE_DELAY_MS = 5000; + /** * Seed per-workspace localStorage from backend workspace metadata. * @@ -170,6 +176,10 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { // Used by async subscription handlers to safely access the most recent metadata map // without triggering render-phase state updates. + + // When a reported task workspace is deleted, keep it visible briefly so users can read the report. + // Keyed by workspaceId. + const reportedTaskDeletionTimeoutsRef = useRef(new Map()); const workspaceMetadataRef = useRef(workspaceMetadata); useEffect(() => { workspaceMetadataRef.current = workspaceMetadata; @@ -311,6 +321,55 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { // Subscribe to metadata updates (for create/rename/delete operations) useEffect(() => { if (!api) return; + + const reportedTaskDeletionTimeouts = reportedTaskDeletionTimeoutsRef.current; + + const applyDeletedWorkspaceSelectionFallback = ( + deletedWorkspaceId: string, + deletedMeta: FrontendWorkspaceMetadata | undefined + ): void => { + // If the user is currently viewing a deleted child workspace, fall back to its parent. + // Otherwise, fall back to another workspace in the same project (best effort). + setSelectedWorkspace((current) => { + if (current?.workspaceId !== deletedWorkspaceId) { + return current; + } + + const parentWorkspaceId = deletedMeta?.parentWorkspaceId; + if (parentWorkspaceId) { + const parentMeta = workspaceMetadataRef.current.get(parentWorkspaceId); + if (parentMeta) { + return { + workspaceId: parentMeta.id, + projectPath: parentMeta.projectPath, + projectName: parentMeta.projectName, + namedWorkspacePath: parentMeta.namedWorkspacePath, + }; + } + } + + const projectPath = deletedMeta?.projectPath; + const fallbackMeta = + (projectPath + ? Array.from(workspaceMetadataRef.current.values()).find( + (meta) => meta.projectPath === projectPath && meta.id !== deletedWorkspaceId + ) + : null) ?? + Array.from(workspaceMetadataRef.current.values()).find( + (meta) => meta.id !== deletedWorkspaceId + ); + if (!fallbackMeta) { + return null; + } + + return { + workspaceId: fallbackMeta.id, + projectPath: fallbackMeta.projectPath, + projectName: fallbackMeta.projectName, + namedWorkspacePath: fallbackMeta.namedWorkspacePath, + }; + }); + }; const controller = new AbortController(); const { signal } = controller; @@ -321,52 +380,58 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { for await (const event of iterator) { if (signal.aborted) break; - if (event.metadata === null) { - // Workspace deleted - clean up workspace-scoped persisted state. - deleteWorkspaceStorage(event.workspaceId); + // If a workspace reappears after we scheduled a delayed removal, cancel it. + const pendingRemoval = reportedTaskDeletionTimeouts.get(event.workspaceId); + if (pendingRemoval && event.metadata !== null) { + window.clearTimeout(pendingRemoval); + reportedTaskDeletionTimeouts.delete(event.workspaceId); + } - // If the user is currently viewing a deleted child workspace, fall back to its parent. - // Otherwise, fall back to another workspace in the same project (best effort). + if (event.metadata === null) { const deletedMeta = workspaceMetadataRef.current.get(event.workspaceId); - setSelectedWorkspace((current) => { - if (current?.workspaceId !== event.workspaceId) { - return current; - } - const parentWorkspaceId = deletedMeta?.parentWorkspaceId; - if (parentWorkspaceId) { - const parentMeta = workspaceMetadataRef.current.get(parentWorkspaceId); - if (parentMeta) { - return { - workspaceId: parentMeta.id, - projectPath: parentMeta.projectPath, - projectName: parentMeta.projectName, - namedWorkspacePath: parentMeta.namedWorkspacePath, - }; - } - } + const isReportedTaskWorkspace = + Boolean(deletedMeta?.parentWorkspaceId) && deletedMeta?.taskStatus === "reported"; + + // Ensure reported task workspaces remain visible for at least the configured delay after reporting. + let delayRemovalMs = 0; + if (isReportedTaskWorkspace) { + const reportedAtMs = + typeof deletedMeta?.reportedAt === "string" + ? Date.parse(deletedMeta.reportedAt) + : NaN; + const elapsedSinceReportedMs = Number.isFinite(reportedAtMs) + ? Math.max(0, Date.now() - reportedAtMs) + : 0; + delayRemovalMs = Math.max( + 0, + REPORTED_TASK_WORKSPACE_AUTO_DELETE_DELAY_MS - elapsedSinceReportedMs + ); + } - const projectPath = deletedMeta?.projectPath; - const fallbackMeta = - (projectPath - ? Array.from(workspaceMetadataRef.current.values()).find( - (meta) => meta.projectPath === projectPath && meta.id !== event.workspaceId - ) - : null) ?? - Array.from(workspaceMetadataRef.current.values()).find( - (meta) => meta.id !== event.workspaceId - ); - if (!fallbackMeta) { - return null; + if (delayRemovalMs > 0) { + if (!reportedTaskDeletionTimeouts.has(event.workspaceId)) { + const handle = window.setTimeout(() => { + reportedTaskDeletionTimeouts.delete(event.workspaceId); + deleteWorkspaceStorage(event.workspaceId); + applyDeletedWorkspaceSelectionFallback(event.workspaceId, deletedMeta); + setWorkspaceMetadata((prev) => { + const updated = new Map(prev); + updated.delete(event.workspaceId); + return updated; + }); + }, delayRemovalMs); + + reportedTaskDeletionTimeouts.set(event.workspaceId, handle); } - return { - workspaceId: fallbackMeta.id, - projectPath: fallbackMeta.projectPath, - projectName: fallbackMeta.projectName, - namedWorkspacePath: fallbackMeta.namedWorkspacePath, - }; - }); + // Keep the workspace visible until the grace period elapses. + continue; + } + + // Workspace deleted - clean up workspace-scoped persisted state. + deleteWorkspaceStorage(event.workspaceId); + applyDeletedWorkspaceSelectionFallback(event.workspaceId, deletedMeta); } if (event.metadata !== null) { @@ -408,6 +473,11 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) { return () => { controller.abort(); + + for (const handle of reportedTaskDeletionTimeouts.values()) { + window.clearTimeout(handle); + } + reportedTaskDeletionTimeouts.clear(); }; }, [refreshProjects, setSelectedWorkspace, setWorkspaceMetadata, api]); diff --git a/src/browser/hooks/useReviewRefreshController.ts b/src/browser/hooks/useReviewRefreshController.ts index c8cf2291cf..f2b3613247 100644 --- a/src/browser/hooks/useReviewRefreshController.ts +++ b/src/browser/hooks/useReviewRefreshController.ts @@ -1,26 +1,12 @@ -import { useEffect, useRef } from "react"; +import { useEffect, useRef, useMemo } from "react"; import { workspaceStore } from "@/browser/stores/WorkspaceStore"; import type { APIClient } from "@/browser/contexts/API"; +import { RefreshController, type LastRefreshInfo } from "@/browser/utils/RefreshController"; +import { usePersistedState } from "@/browser/hooks/usePersistedState"; /** Debounce delay for auto-refresh after tool completion */ const TOOL_REFRESH_DEBOUNCE_MS = 3000; -/** - * Extract branch name from an "origin/..." diff base for git fetch. - * Returns null if not an origin ref or if branch name is unsafe for shell. - */ -function getOriginBranchForFetch(diffBase: string): string | null { - const trimmed = diffBase.trim(); - if (!trimmed.startsWith("origin/")) return null; - - const branch = trimmed.slice("origin/".length); - - // Avoid shell injection; diffBase is user-controlled. - if (!/^[0-9A-Za-z._/-]+$/.test(branch)) return null; - - return branch; -} - export interface UseReviewRefreshControllerOptions { workspaceId: string; api: APIClient | null; @@ -42,234 +28,79 @@ export interface ReviewRefreshController { setInteracting: (interacting: boolean) => void; /** Whether a git fetch is currently in-flight */ isRefreshing: boolean; + /** Info about the last completed refresh (for debugging) */ + lastRefreshInfo: LastRefreshInfo | null; } /** * Controls ReviewPanel auto-refresh triggered by file-modifying tool completions. * - * Handles: - * - Debouncing rapid tool completions (3s window) - * - Pausing while user is interacting (review note input focused) - * - Pausing while tab is hidden (flush on visibility change) - * - Coalescing requests while origin fetch is in-flight - * - Preserving scroll position across refreshes - * - * Architecture: - * - All refresh logic flows through a single ref-based handler to avoid stale closures - * - Pending flags track deferred refreshes for various pause conditions - * - visibilitychange listener ensures hidden-tab refreshes aren't lost + * Delegates debouncing, visibility/focus handling, and in-flight guards to RefreshController. + * Keeps ReviewPanel-specific logic: + * - Scroll position preservation + * - User interaction pause state */ export function useReviewRefreshController( options: UseReviewRefreshControllerOptions ): ReviewRefreshController { - const { workspaceId, api, isCreating, onRefresh, scrollContainerRef, onGitStatusRefresh } = - options; - - // Store diffBase in a ref so we always read the latest value - const diffBaseRef = useRef(options.diffBase); - diffBaseRef.current = options.diffBase; - - // Store onGitStatusRefresh in a ref to avoid stale closures - const onGitStatusRefreshRef = useRef(onGitStatusRefresh); - onGitStatusRefreshRef.current = onGitStatusRefresh; + const { workspaceId, api, isCreating, scrollContainerRef } = options; - // State refs (avoid re-renders, just track state for refresh logic) - const isRefreshingRef = useRef(false); - const isInteractingRef = useRef(false); - const debounceTimerRef = useRef | null>(null); + // Refs for values that executeRefresh needs at call time (avoid stale closures) + const onRefreshRef = useRef(options.onRefresh); + onRefreshRef.current = options.onRefresh; - // Pending flags - track why refresh was deferred - const pendingBecauseHiddenRef = useRef(false); - const pendingBecauseInteractingRef = useRef(false); - const pendingBecauseInFlightRef = useRef(false); + const onGitStatusRefreshRef = useRef(options.onGitStatusRefresh); + onGitStatusRefreshRef.current = options.onGitStatusRefresh; // Scroll position to restore after refresh const savedScrollTopRef = useRef(null); - // Expose isRefreshing for UI (e.g. disable refresh button) - // We use a ref but also track in a simple way for the return value - const isRefreshingForReturn = useRef(false); - - /** - * Core refresh execution - handles origin fetch if needed, then triggers onRefresh. - * Always reads latest state from refs at execution time. - */ - const executeRefresh = useRef(() => { - if (!api || isCreating) return; - - // Save scroll position before refresh - savedScrollTopRef.current = scrollContainerRef.current?.scrollTop ?? null; - - const originBranch = getOriginBranchForFetch(diffBaseRef.current); - if (originBranch) { - isRefreshingRef.current = true; - isRefreshingForReturn.current = true; - - api.workspace - .executeBash({ - workspaceId, - script: `git fetch origin ${originBranch} --quiet || true`, - options: { timeout_secs: 30 }, - }) - .catch((err) => { - console.debug("ReviewPanel origin fetch failed", err); - }) - .finally(() => { - isRefreshingRef.current = false; - isRefreshingForReturn.current = false; - onRefresh(); - onGitStatusRefreshRef.current?.(); - - // If another refresh was requested while we were fetching, do it now - if (pendingBecauseInFlightRef.current) { - pendingBecauseInFlightRef.current = false; - // Use setTimeout to avoid recursive call stack - setTimeout(() => tryRefresh("in-flight-followup"), 0); - } - }); - - return; - } - - // Local base - just trigger refresh immediately - onRefresh(); - onGitStatusRefreshRef.current?.(); - }); - - // Update executeRefresh closure dependencies - executeRefresh.current = () => { - if (!api || isCreating) return; - - savedScrollTopRef.current = scrollContainerRef.current?.scrollTop ?? null; - - const originBranch = getOriginBranchForFetch(diffBaseRef.current); - if (originBranch) { - isRefreshingRef.current = true; - isRefreshingForReturn.current = true; - - api.workspace - .executeBash({ - workspaceId, - script: `git fetch origin ${originBranch} --quiet || true`, - options: { timeout_secs: 30 }, - }) - .catch((err) => { - console.debug("ReviewPanel origin fetch failed", err); - }) - .finally(() => { - isRefreshingRef.current = false; - isRefreshingForReturn.current = false; - onRefresh(); - onGitStatusRefreshRef.current?.(); - - if (pendingBecauseInFlightRef.current) { - pendingBecauseInFlightRef.current = false; - setTimeout(() => tryRefresh("in-flight-followup"), 0); - } - }); - - return; - } - - onRefresh(); - onGitStatusRefreshRef.current?.(); - }; - - /** - * Attempt to refresh, respecting all pause conditions. - * If paused, sets the appropriate pending flag. - */ - const tryRefresh = (_reason: string) => { - if (!api || isCreating) return; - - // Check pause conditions in order of priority - - // 1. Tab hidden - queue for visibility change - if (document.hidden) { - pendingBecauseHiddenRef.current = true; - return; - } - - // 2. User interacting - queue for blur - if (isInteractingRef.current) { - pendingBecauseInteractingRef.current = true; - return; - } - - // 3. Already refreshing (origin fetch in-flight) - queue for completion - if (isRefreshingRef.current) { - pendingBecauseInFlightRef.current = true; - return; - } - - // All clear - execute refresh - executeRefresh.current(); - }; - - /** - * Schedule a debounced refresh (for tool completions). - */ - const scheduleRefresh = () => { - if (debounceTimerRef.current) { - clearTimeout(debounceTimerRef.current); - } - debounceTimerRef.current = setTimeout(() => { - debounceTimerRef.current = null; - tryRefresh("tool-completion"); - }, TOOL_REFRESH_DEBOUNCE_MS); - }; + // User interaction state (pauses auto-refresh) + const isInteractingRef = useRef(false); - /** - * Flush any pending refresh (called when pause condition clears). - */ - const flushPending = (clearedCondition: "hidden" | "interacting") => { - if (clearedCondition === "hidden" && pendingBecauseHiddenRef.current) { - pendingBecauseHiddenRef.current = false; - tryRefresh("visibility-restored"); - } else if (clearedCondition === "interacting" && pendingBecauseInteractingRef.current) { - pendingBecauseInteractingRef.current = false; - tryRefresh("interaction-ended"); - } - }; + // Track last refresh info - persisted per workspace so it survives workspace switches + const [lastRefreshInfo, setLastRefreshInfo] = usePersistedState( + `review-last-refresh:${workspaceId}`, + null + ); + + // Create RefreshController once, with stable callbacks via refs + const controller = useMemo(() => { + const ctrl = new RefreshController({ + debounceMs: TOOL_REFRESH_DEBOUNCE_MS, + isPaused: () => isInteractingRef.current, + onRefresh: () => { + if (isCreating) return; + + // Save scroll position before refresh + savedScrollTopRef.current = scrollContainerRef.current?.scrollTop ?? null; + + onRefreshRef.current(); + onGitStatusRefreshRef.current?.(); + }, + onRefreshComplete: setLastRefreshInfo, + }); + ctrl.bindListeners(); + return ctrl; + // isCreating/scrollContainerRef/setLastRefreshInfo changes require a new controller. + // Note: options.onRefresh is accessed via ref to avoid recreating controller on every render. + }, [isCreating, scrollContainerRef, setLastRefreshInfo]); + + // Cleanup on unmount or when controller changes + useEffect(() => { + return () => controller.dispose(); + }, [controller]); // Subscribe to file-modifying tool completions useEffect(() => { if (!api || isCreating) return; - const unsubscribe = workspaceStore.subscribeFileModifyingTool(scheduleRefresh, workspaceId); - - return () => { - unsubscribe(); - if (debounceTimerRef.current) { - clearTimeout(debounceTimerRef.current); - debounceTimerRef.current = null; - } - }; - // scheduleRefresh is stable (only uses refs internally) - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [api, workspaceId, isCreating]); + const unsubscribe = workspaceStore.subscribeFileModifyingTool(() => { + controller.schedule(); + }, workspaceId); - // Handle visibility/focus changes - flush pending refresh when user returns. - // Uses both visibilitychange (for browser tab hidden state) and window focus - // (for Electron app focus) since visibilitychange alone is unreliable in Electron - // when the app is behind other windows or on a different desktop/space. - useEffect(() => { - const handleReturn = () => { - // Only flush if document is actually visible - if (!document.hidden) { - flushPending("hidden"); - } - }; - - document.addEventListener("visibilitychange", handleReturn); - window.addEventListener("focus", handleReturn); - return () => { - document.removeEventListener("visibilitychange", handleReturn); - window.removeEventListener("focus", handleReturn); - }; - // flushPending is stable (only uses refs internally) - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + return unsubscribe; + }, [api, workspaceId, isCreating, controller]); // Public API const setInteracting = (interacting: boolean) => { @@ -278,25 +109,21 @@ export function useReviewRefreshController( // If interaction ended, flush any pending refresh if (wasInteracting && !interacting) { - flushPending("interacting"); + controller.notifyUnpaused(); } }; const requestManualRefresh = () => { - // Manual refresh bypasses debounce but still respects in-flight check - if (isRefreshingRef.current) { - pendingBecauseInFlightRef.current = true; - return; - } - executeRefresh.current(); + controller.requestImmediate(); }; return { requestManualRefresh, setInteracting, get isRefreshing() { - return isRefreshingForReturn.current; + return controller.isRefreshing; }, + lastRefreshInfo, }; } diff --git a/src/browser/stores/GitStatusStore.ts b/src/browser/stores/GitStatusStore.ts index 3df58aeca1..ea9494a725 100644 --- a/src/browser/stores/GitStatusStore.ts +++ b/src/browser/stores/GitStatusStore.ts @@ -19,7 +19,7 @@ import { RefreshController } from "@/browser/utils/RefreshController"; * - Lives outside React lifecycle (stable references) * - Event-driven updates (no polling): * - Initial subscription triggers immediate fetch - * - File-modifying tools trigger debounced refresh (3s) + * - File-modifying tools trigger debounced refresh (1s active, 3s background) * - Window focus triggers refresh for visible workspaces * - Explicit invalidation (branch switch, etc.) * - Manages git fetch with exponential backoff @@ -31,6 +31,7 @@ import { RefreshController } from "@/browser/utils/RefreshController"; // Configuration const MAX_CONCURRENT_GIT_OPS = 5; +const ACTIVE_WORKSPACE_DEBOUNCE_MS = 1000; // 1s for active workspace (vs 3s background) // Fetch configuration - aggressive intervals for fresh data const FETCH_BASE_INTERVAL_MS = 3 * 1000; // 3 seconds @@ -53,6 +54,9 @@ export class GitStatusStore { // File modification subscription private fileModifyUnsubscribe: (() => void) | null = null; + // Active workspace ID for prioritized refresh (1s vs 3s debounce) + private activeWorkspaceId: string | null = null; + // RefreshController handles debouncing, focus/visibility, and in-flight guards private readonly refreshController: RefreshController; @@ -64,7 +68,8 @@ export class GitStatusStore { // Create refresh controller with proactive focus refresh (catches external git changes) this.refreshController = new RefreshController({ onRefresh: () => this.updateGitStatus(), - debounceMs: 3000, // Same as TOOL_REFRESH_DEBOUNCE_MS in ReviewPanel + debounceMs: 3000, // Background workspaces + priorityDebounceMs: ACTIVE_WORKSPACE_DEBOUNCE_MS, // Active workspace gets faster refresh refreshOnFocus: true, // Proactively refresh on focus to catch external changes focusDebounceMs: 500, // Prevent spam from rapid alt-tabbing }); @@ -113,6 +118,14 @@ export class GitStatusStore { }); } + /** + * Set the active workspace for prioritized refresh (1s debounce vs 3s). + * Call when workspace selection changes. + */ + setActiveWorkspace(workspaceId: string | null): void { + this.activeWorkspaceId = workspaceId; + } + /** * Invalidate status for a workspace, clearing cache and triggering immediate refresh. * Call after operations that change git state (e.g., branch switch). @@ -490,8 +503,13 @@ export class GitStatusStore { return; } - // RefreshController handles debouncing, focus gating, and in-flight guards - this.refreshController.schedule(); + // Active workspace gets faster refresh (1s) via priority debounce + if (workspaceId === this.activeWorkspaceId) { + this.refreshController.schedulePriority(); + } else { + // Background workspaces use standard 3s debounce + this.refreshController.schedule(); + } }); } } @@ -542,3 +560,11 @@ export function invalidateGitStatus(workspaceId: string): void { const store = getGitStoreInstance(); store.invalidateWorkspace(workspaceId); } + +/** + * Set the active workspace for prioritized git status refresh (1s vs 3s debounce). + */ +export function setActiveWorkspace(workspaceId: string | null): void { + const store = getGitStoreInstance(); + store.setActiveWorkspace(workspaceId); +} diff --git a/src/browser/stores/WorkspaceStore.test.ts b/src/browser/stores/WorkspaceStore.test.ts index 34d33a418b..41c0b7b45d 100644 --- a/src/browser/stores/WorkspaceStore.test.ts +++ b/src/browser/stores/WorkspaceStore.test.ts @@ -773,4 +773,151 @@ describe("WorkspaceStore", () => { expect(live).toBeNull(); }); }); + + describe("file-modifying tool subscription", () => { + it("notifies subscribers when bash tool completes", async () => { + const workspaceId = "file-modify-test-1"; + const notifications: string[] = []; + + // Subscribe BEFORE creating workspace + const unsubscribe = store.subscribeFileModifyingTool((wsId) => { + notifications.push(wsId); + }, workspaceId); + + mockOnChat.mockImplementation(async function* (): AsyncGenerator< + WorkspaceChatMessage, + void, + unknown + > { + yield { type: "caught-up" }; + await Promise.resolve(); + yield { + type: "tool-call-end", + workspaceId, + messageId: "m1", + toolCallId: "call-1", + toolName: "bash", + result: { success: true, output: "done", exitCode: 0, wall_duration_ms: 1 }, + timestamp: 1, + }; + }); + + createAndAddWorkspace(store, workspaceId); + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(notifications).toContain(workspaceId); + expect(notifications.length).toBe(1); + + unsubscribe(); + }); + + it("notifies subscribers when file_edit tool completes", async () => { + const workspaceId = "file-modify-test-2"; + const notifications: string[] = []; + + const unsubscribe = store.subscribeFileModifyingTool((wsId) => { + notifications.push(wsId); + }, workspaceId); + + mockOnChat.mockImplementation(async function* (): AsyncGenerator< + WorkspaceChatMessage, + void, + unknown + > { + yield { type: "caught-up" }; + await Promise.resolve(); + yield { + type: "tool-call-end", + workspaceId, + messageId: "m1", + toolCallId: "call-1", + toolName: "file_edit_replace_string", + result: { success: true }, + timestamp: 1, + }; + }); + + createAndAddWorkspace(store, workspaceId); + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(notifications).toContain(workspaceId); + unsubscribe(); + }); + + it("does NOT notify for non-file-modifying tools", async () => { + const workspaceId = "file-modify-test-3"; + const notifications: string[] = []; + + const unsubscribe = store.subscribeFileModifyingTool((wsId) => { + notifications.push(wsId); + }, workspaceId); + + mockOnChat.mockImplementation(async function* (): AsyncGenerator< + WorkspaceChatMessage, + void, + unknown + > { + yield { type: "caught-up" }; + await Promise.resolve(); + yield { + type: "tool-call-end", + workspaceId, + messageId: "m1", + toolCallId: "call-1", + toolName: "web_search", // Not file-modifying + result: { success: true }, + timestamp: 1, + }; + }); + + createAndAddWorkspace(store, workspaceId); + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(notifications.length).toBe(0); + unsubscribe(); + }); + + it("only notifies subscribers for the specific workspace", async () => { + const workspaceId1 = "file-modify-test-4a"; + const workspaceId2 = "file-modify-test-4b"; + const notifications1: string[] = []; + const notifications2: string[] = []; + + const unsubscribe1 = store.subscribeFileModifyingTool((wsId) => { + notifications1.push(wsId); + }, workspaceId1); + + const unsubscribe2 = store.subscribeFileModifyingTool((wsId) => { + notifications2.push(wsId); + }, workspaceId2); + + // Only emit tool completion for workspace1 + mockOnChat.mockImplementation(async function* (): AsyncGenerator< + WorkspaceChatMessage, + void, + unknown + > { + yield { type: "caught-up" }; + await Promise.resolve(); + yield { + type: "tool-call-end", + workspaceId: workspaceId1, + messageId: "m1", + toolCallId: "call-1", + toolName: "bash", + result: { success: true, output: "done", exitCode: 0, wall_duration_ms: 1 }, + timestamp: 1, + }; + }); + + createAndAddWorkspace(store, workspaceId1); + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(notifications1.length).toBe(1); + expect(notifications2.length).toBe(0); // Not notified - different workspace + + unsubscribe1(); + unsubscribe2(); + }); + }); }); diff --git a/src/browser/stores/WorkspaceStore.ts b/src/browser/stores/WorkspaceStore.ts index 3fc72ea576..eddba112fb 100644 --- a/src/browser/stores/WorkspaceStore.ts +++ b/src/browser/stores/WorkspaceStore.ts @@ -830,6 +830,13 @@ export class WorkspaceStore { * * REQUIRES: Workspace must have been added via addWorkspace() first. */ + /** + * Get workspace name for logging/debugging. Returns "name" or falls back to ID. + */ + getWorkspaceName(workspaceId: string): string { + return this.workspaceMetadata.get(workspaceId)?.name ?? workspaceId; + } + getWorkspaceState(workspaceId: string): WorkspaceState { return this.states.get(workspaceId, () => { const aggregator = this.assertGet(workspaceId); @@ -1829,6 +1836,8 @@ export const workspaceStore = { getStoreInstance().getFileModifyingToolMs(workspaceId), clearFileModifyingToolMs: (workspaceId: string) => getStoreInstance().clearFileModifyingToolMs(workspaceId), + /** Get workspace name for logging/debugging */ + getWorkspaceName: (workspaceId: string) => getStoreInstance().getWorkspaceName(workspaceId), }; /** diff --git a/src/browser/stories/App.sidebar.stories.tsx b/src/browser/stories/App.sidebar.stories.tsx index 6307e296ca..3d39b58363 100644 --- a/src/browser/stories/App.sidebar.stories.tsx +++ b/src/browser/stories/App.sidebar.stories.tsx @@ -355,7 +355,7 @@ export const GitStatusVariations: AppStory = { /> ), play: async ({ canvasElement }: { canvasElement: HTMLElement }) => { - // Wait for git status to render (fetched async via GitStatusStore polling) + // Wait for git status to render (fetched async via GitStatusStore) await waitFor( () => { const row = canvasElement.querySelector('[data-workspace-id="ws-diverged"]'); diff --git a/src/browser/utils/RefreshController.test.ts b/src/browser/utils/RefreshController.test.ts index e4aa56babc..a976269b5f 100644 --- a/src/browser/utils/RefreshController.test.ts +++ b/src/browser/utils/RefreshController.test.ts @@ -10,7 +10,22 @@ describe("RefreshController", () => { jest.useRealTimers(); }); - it("debounces multiple schedule() calls", () => { + it("rate-limits multiple schedule() calls (doesn't reset timer)", () => { + const onRefresh = jest.fn<() => void>(); + const controller = new RefreshController({ onRefresh, debounceMs: 100 }); + + controller.schedule(); + jest.advanceTimersByTime(50); + controller.schedule(); // Shouldn't reset timer + jest.advanceTimersByTime(50); + + // Should fire at 100ms from first call, not 150ms + expect(onRefresh).toHaveBeenCalledTimes(1); + + controller.dispose(); + }); + + it("coalesces calls during rate-limit window", () => { const onRefresh = jest.fn<() => void>(); const controller = new RefreshController({ onRefresh, debounceMs: 100 }); @@ -27,7 +42,7 @@ describe("RefreshController", () => { controller.dispose(); }); - it("requestImmediate() bypasses debounce", () => { + it("requestImmediate() bypasses rate-limit timer", () => { const onRefresh = jest.fn<() => void>(); const controller = new RefreshController({ onRefresh, debounceMs: 100 }); @@ -37,7 +52,7 @@ describe("RefreshController", () => { controller.requestImmediate(); expect(onRefresh).toHaveBeenCalledTimes(1); - // Original debounce timer should be cleared + // Original timer should be cleared jest.advanceTimersByTime(100); expect(onRefresh).toHaveBeenCalledTimes(1); @@ -64,6 +79,38 @@ describe("RefreshController", () => { controller.dispose(); }); + it("schedule() during in-flight queues refresh for after completion", async () => { + let resolveRefresh: () => void; + const onRefresh = jest.fn( + () => + new Promise((resolve) => { + resolveRefresh = resolve; + }) + ); + + const controller = new RefreshController({ onRefresh, debounceMs: 100 }); + + // Start first refresh + controller.requestImmediate(); + expect(onRefresh).toHaveBeenCalledTimes(1); + + // schedule() while in-flight should queue, not start timer + controller.schedule(); + + // Complete the first refresh and let .finally() run + resolveRefresh!(); + await Promise.resolve(); + await Promise.resolve(); // Extra tick for .finally() + + // Should trigger follow-up refresh, but never more frequently than the minimum interval. + // First tick runs the post-flight setTimeout(0), then we wait out the min interval. + jest.advanceTimersByTime(0); + jest.advanceTimersByTime(500); + expect(onRefresh).toHaveBeenCalledTimes(2); + + controller.dispose(); + }); + it("isRefreshing reflects in-flight state", () => { let resolveRefresh: () => void; const refreshPromise = new Promise((resolve) => { @@ -108,4 +155,100 @@ describe("RefreshController", () => { expect(onRefresh).not.toHaveBeenCalled(); }); + + it("requestImmediate() bypasses isPaused check (for manual refresh)", () => { + const onRefresh = jest.fn<() => void>(); + const paused = true; + const controller = new RefreshController({ + onRefresh, + debounceMs: 100, + isPaused: () => paused, + }); + + // schedule() should be blocked by isPaused + controller.schedule(); + jest.advanceTimersByTime(100); + expect(onRefresh).not.toHaveBeenCalled(); + + // requestImmediate() should bypass isPaused (manual refresh) + controller.requestImmediate(); + expect(onRefresh).toHaveBeenCalledTimes(1); + + controller.dispose(); + }); + + it("schedule() respects isPaused and flushes on notifyUnpaused", () => { + const onRefresh = jest.fn<() => void>(); + let paused = true; + const controller = new RefreshController({ + onRefresh, + debounceMs: 100, + isPaused: () => paused, + }); + + // schedule() should queue but not execute while paused + controller.schedule(); + jest.advanceTimersByTime(100); + expect(onRefresh).not.toHaveBeenCalled(); + + // Unpausing should flush the pending refresh + paused = false; + controller.notifyUnpaused(); + expect(onRefresh).toHaveBeenCalledTimes(1); + + controller.dispose(); + }); + + it("lastRefreshInfo tracks trigger and timestamp", () => { + const onRefresh = jest.fn<() => void>(); + const controller = new RefreshController({ onRefresh, debounceMs: 100 }); + + expect(controller.lastRefreshInfo).toBeNull(); + + // Manual refresh should record "manual" trigger + const beforeManual = Date.now(); + controller.requestImmediate(); + expect(controller.lastRefreshInfo).not.toBeNull(); + expect(controller.lastRefreshInfo!.trigger).toBe("manual"); + expect(controller.lastRefreshInfo!.timestamp).toBeGreaterThanOrEqual(beforeManual); + + // Scheduled refresh should record "scheduled" trigger + controller.schedule(); + jest.advanceTimersByTime(500); + expect(controller.lastRefreshInfo!.trigger).toBe("scheduled"); + + // Priority refresh should record "priority" trigger + controller.schedulePriority(); + jest.advanceTimersByTime(500); + expect(controller.lastRefreshInfo!.trigger).toBe("priority"); + + controller.dispose(); + }); + + it("onRefreshComplete callback is called with refresh info", () => { + const onRefresh = jest.fn<() => void>(); + const onRefreshComplete = jest.fn<(info: { trigger: string; timestamp: number }) => void>(); + const controller = new RefreshController({ + onRefresh, + onRefreshComplete, + debounceMs: 100, + }); + + expect(onRefreshComplete).not.toHaveBeenCalled(); + + controller.requestImmediate(); + expect(onRefreshComplete).toHaveBeenCalledTimes(1); + expect(onRefreshComplete).toHaveBeenCalledWith( + expect.objectContaining({ trigger: "manual", timestamp: expect.any(Number) }) + ); + + controller.schedule(); + jest.advanceTimersByTime(500); + expect(onRefreshComplete).toHaveBeenCalledTimes(2); + expect(onRefreshComplete).toHaveBeenLastCalledWith( + expect.objectContaining({ trigger: "scheduled" }) + ); + + controller.dispose(); + }); }); diff --git a/src/browser/utils/RefreshController.ts b/src/browser/utils/RefreshController.ts index 0ba0501a75..6ad4e7e977 100644 --- a/src/browser/utils/RefreshController.ts +++ b/src/browser/utils/RefreshController.ts @@ -11,13 +11,36 @@ * Used by GitStatusStore and useReviewRefreshController. */ +/** Reason that triggered a refresh - useful for debugging */ +export type RefreshTrigger = + | "manual" // User clicked refresh button + | "scheduled" // Debounced tool completion + | "priority" // Priority debounced (active workspace) + | "focus" // Window regained focus + | "visibility" // Tab became visible + | "unpaused" // Interaction ended, flushing pending + | "in-flight-followup"; // Queued while previous refresh was running + +export interface LastRefreshInfo { + /** Timestamp of last refresh completion */ + timestamp: number; + /** What triggered the refresh */ + trigger: RefreshTrigger; +} + export interface RefreshControllerOptions { /** Called to execute the actual refresh. Can be async. */ onRefresh: () => Promise | void; + /** Called after refresh completes with info about the refresh (for state updates) */ + onRefreshComplete?: (info: LastRefreshInfo) => void; + /** Debounce delay for triggered refreshes (ms). Default: 3000 */ debounceMs?: number; + /** Priority debounce delay (ms). Used by schedulePriority(). Default: same as debounceMs */ + priorityDebounceMs?: number; + /** * Whether to proactively refresh on focus, not just flush pending. * Enable for stores that need to catch external changes (e.g., git status). @@ -27,21 +50,46 @@ export interface RefreshControllerOptions { /** Minimum interval between focus-triggered refreshes (ms). Default: 500 */ focusDebounceMs?: number; + + /** + * Optional callback to check if refresh should be paused (e.g., user is interacting). + * If returns true, refresh is deferred until the condition clears. + */ + isPaused?: () => boolean; + + /** Label for debug logging (e.g., workspace name). If set, enables debug logs. */ + debugLabel?: string; } +/** Minimum ms between refresh executions - hard guard against loops */ +const MIN_REFRESH_INTERVAL_MS = 500; + export class RefreshController { private readonly onRefresh: () => Promise | void; + private readonly onRefreshComplete: ((info: LastRefreshInfo) => void) | null; private readonly debounceMs: number; + private readonly priorityDebounceMs: number; private readonly refreshOnFocus: boolean; private readonly focusDebounceMs: number; + private readonly isPaused: (() => boolean) | null; + private readonly debugLabel: string | null; private debounceTimer: ReturnType | null = null; + private cooldownTimer: ReturnType | null = null; private inFlight = false; private pendingBecauseHidden = false; private pendingBecauseInFlight = false; + private pendingBecausePaused = false; private lastFocusRefreshMs = 0; private disposed = false; + // Track last refresh for debugging + private _lastRefreshInfo: LastRefreshInfo | null = null; + private pendingTrigger: RefreshTrigger | null = null; + + // Timestamp of last refresh START (not completion) + private lastRefreshStartMs = 0; + // Track if listeners are bound (for cleanup) private listenersBound = false; private boundHandleVisibility: (() => void) | null = null; @@ -49,29 +97,94 @@ export class RefreshController { constructor(options: RefreshControllerOptions) { this.onRefresh = options.onRefresh; + this.onRefreshComplete = options.onRefreshComplete ?? null; this.debounceMs = options.debounceMs ?? 3000; + this.priorityDebounceMs = options.priorityDebounceMs ?? this.debounceMs; this.refreshOnFocus = options.refreshOnFocus ?? false; this.focusDebounceMs = options.focusDebounceMs ?? 500; + this.isPaused = options.isPaused ?? null; + this.debugLabel = options.debugLabel ?? null; + } + + private updatePendingTrigger(trigger: RefreshTrigger): void { + const priorities: Record = { + manual: 3, + priority: 2, + scheduled: 1, + focus: 0, + visibility: 0, + unpaused: 0, + "in-flight-followup": 0, + }; + + if (!this.pendingTrigger) { + this.pendingTrigger = trigger; + return; + } + + if (priorities[trigger] >= priorities[this.pendingTrigger]) { + this.pendingTrigger = trigger; + } + } + private debug(message: string): void { + if (this.debugLabel) { + console.debug(`[RefreshController:${this.debugLabel}] ${message}`); + } } /** - * Schedule a debounced refresh. Multiple calls within debounceMs coalesce. + * Schedule a rate-limited refresh. Multiple calls within the rate limit window + * coalesce, but don't reset the timer (unlike pure debounce). + * + * Behavior: + * - First call starts timer for delayMs + * - Subsequent calls mark "pending" but don't reset timer + * - When timer fires, refresh runs + * - If calls came in during refresh, a new timer starts after completion */ schedule(): void { + this.scheduleWithDelay(this.debounceMs, "scheduled"); + } + + /** + * Schedule with priority (shorter) rate limit. Used for active workspace. + */ + schedulePriority(): void { + this.scheduleWithDelay(this.priorityDebounceMs, "priority"); + } + + private scheduleWithDelay(delayMs: number, trigger: RefreshTrigger): void { if (this.disposed) return; + // Always update pending trigger (manual > priority > scheduled) + this.updatePendingTrigger(trigger); + + // If refresh is in-flight, mark pending and let onComplete handle scheduling + if (this.inFlight) { + this.debug("in-flight, queueing for completion"); + this.pendingBecauseInFlight = true; + return; + } + + // Rate-limit: if timer already running, don't reset it if (this.debounceTimer) { - clearTimeout(this.debounceTimer); + this.debug("timer running, coalescing"); + return; } + this.debug(`starting ${delayMs}ms timer (${trigger})`); this.debounceTimer = setTimeout(() => { this.debounceTimer = null; - this.tryRefresh(); - }, this.debounceMs); + const t = this.pendingTrigger ?? trigger; + this.pendingTrigger = null; + this.debug(`timer fired, refreshing (${t})`); + this.tryRefresh({ trigger: t }); + }, delayMs); } /** - * Request immediate refresh, bypassing debounce but respecting in-flight guard. + * Request immediate refresh, bypassing debounce and pause checks. + * Use for manual refresh (user clicked button) which should always execute. */ requestImmediate(): void { if (this.disposed) return; @@ -82,48 +195,111 @@ export class RefreshController { this.debounceTimer = null; } - this.tryRefresh(); + this.tryRefresh({ bypassPause: true, bypassHidden: true, trigger: "manual" }); } /** * Attempt refresh, respecting pause conditions. */ - private tryRefresh(): void { + private tryRefresh(options?: { + bypassPause?: boolean; + bypassHidden?: boolean; + bypassMinInterval?: boolean; + trigger?: RefreshTrigger; + }): void { if (this.disposed) return; - // Hidden → queue for visibility - if (typeof document !== "undefined" && document.hidden) { + const trigger = options?.trigger ?? this.pendingTrigger ?? "scheduled"; + const bypassHidden = (options?.bypassHidden ?? false) || trigger === "manual"; + const bypassPause = (options?.bypassPause ?? false) || trigger === "manual"; + const bypassMinInterval = (options?.bypassMinInterval ?? false) || trigger === "manual"; + + // Hidden → queue for visibility (unless bypassed) + if (!bypassHidden && typeof document !== "undefined" && document.hidden) { this.pendingBecauseHidden = true; + this.updatePendingTrigger(trigger); + return; + } + + // Custom pause (e.g., user interacting) → queue for unpause + // Bypassed for manual refresh (user explicitly requested) + if (!bypassPause && this.isPaused?.()) { + this.pendingBecausePaused = true; + this.updatePendingTrigger(trigger); return; } // In-flight → queue for completion if (this.inFlight) { this.pendingBecauseInFlight = true; + this.updatePendingTrigger(trigger); return; } - this.executeRefresh(); + // Hard guard: enforce minimum interval between refresh starts. + // Rather than dropping the request, schedule it for the earliest allowed time. + // Bypassed for manual refresh (user/component explicitly requested). + if (!bypassMinInterval && this.lastRefreshStartMs > 0) { + const now = Date.now(); + const elapsed = now - this.lastRefreshStartMs; + if (elapsed < MIN_REFRESH_INTERVAL_MS) { + this.updatePendingTrigger(trigger); + + if (this.cooldownTimer) { + this.debug("cooldown timer running, coalescing"); + return; + } + + const delayMs = MIN_REFRESH_INTERVAL_MS - elapsed; + const t = this.pendingTrigger ?? trigger; + this.debug(`cooldown: delaying ${delayMs}ms (${t})`); + + this.cooldownTimer = setTimeout(() => { + this.cooldownTimer = null; + const cooldownTrigger = this.pendingTrigger ?? t; + this.pendingTrigger = null; + this.tryRefresh({ trigger: cooldownTrigger }); + }, delayMs); + + return; + } + } + + this.executeRefresh(trigger); } /** * Execute the refresh, tracking in-flight state. */ - private executeRefresh(): void { + private executeRefresh(trigger: RefreshTrigger): void { if (this.disposed) return; + // Record refresh start; min-interval enforcement happens in tryRefresh(). + this.lastRefreshStartMs = Date.now(); + + if (this.cooldownTimer) { + clearTimeout(this.cooldownTimer); + this.cooldownTimer = null; + } + this.inFlight = true; + this.pendingTrigger = null; const maybePromise = this.onRefresh(); const onComplete = () => { this.inFlight = false; + this._lastRefreshInfo = { timestamp: Date.now(), trigger }; + + // Notify listener (for React state updates) + this.onRefreshComplete?.(this._lastRefreshInfo); // Process any queued refresh if (this.pendingBecauseInFlight) { this.pendingBecauseInFlight = false; - // Defer to avoid recursive stack - setTimeout(() => this.tryRefresh(), 0); + // Defer to avoid recursive stack; use the queued trigger + const followupTrigger = this.pendingTrigger ?? "in-flight-followup"; + setTimeout(() => this.tryRefresh({ trigger: followupTrigger }), 0); } }; @@ -137,14 +313,16 @@ export class RefreshController { /** * Handle focus/visibility return. Call from visibility/focus listeners. */ - private handleReturn(): void { + private handleReturn(trigger: "focus" | "visibility"): void { if (this.disposed) return; if (typeof document !== "undefined" && document.hidden) return; // Flush pending hidden refresh if (this.pendingBecauseHidden) { this.pendingBecauseHidden = false; - this.tryRefresh(); + const pendingTrigger = this.pendingTrigger ?? trigger; + this.pendingTrigger = null; + this.tryRefresh({ trigger: pendingTrigger }); return; // Don't double-refresh with proactive } @@ -153,11 +331,25 @@ export class RefreshController { const now = Date.now(); if (now - this.lastFocusRefreshMs >= this.focusDebounceMs) { this.lastFocusRefreshMs = now; - this.tryRefresh(); + this.tryRefresh({ trigger }); } } } + /** + * Notify that a pause condition has cleared (e.g., user stopped interacting). + * Flushes any pending refresh that was deferred due to isPaused(). + */ + notifyUnpaused(): void { + if (this.disposed) return; + if (this.pendingBecausePaused) { + this.pendingBecausePaused = false; + const pendingTrigger = this.pendingTrigger ?? "unpaused"; + this.pendingTrigger = null; + this.tryRefresh({ trigger: pendingTrigger }); + } + } + /** * Bind focus/visibility listeners. Call once after construction. * Safe to call in non-browser environments (no-op). @@ -170,12 +362,12 @@ export class RefreshController { this.boundHandleVisibility = () => { if (document.visibilityState === "visible") { - this.handleReturn(); + this.handleReturn("visibility"); } }; this.boundHandleFocus = () => { - this.handleReturn(); + this.handleReturn("focus"); }; document.addEventListener("visibilitychange", this.boundHandleVisibility); @@ -189,6 +381,14 @@ export class RefreshController { return this.inFlight; } + /** + * Info about the last completed refresh (timestamp and trigger reason). + * Useful for debugging refresh behavior. + */ + get lastRefreshInfo(): LastRefreshInfo | null { + return this._lastRefreshInfo; + } + /** * Clean up timers and listeners. */ @@ -200,6 +400,11 @@ export class RefreshController { this.debounceTimer = null; } + if (this.cooldownTimer) { + clearTimeout(this.cooldownTimer); + this.cooldownTimer = null; + } + if (this.listenersBound) { if (this.boundHandleVisibility) { document.removeEventListener("visibilitychange", this.boundHandleVisibility); diff --git a/src/browser/utils/messages/StreamingMessageAggregator.agentReport.test.ts b/src/browser/utils/messages/StreamingMessageAggregator.agentReport.test.ts new file mode 100644 index 0000000000..c9da15646b --- /dev/null +++ b/src/browser/utils/messages/StreamingMessageAggregator.agentReport.test.ts @@ -0,0 +1,86 @@ +import { describe, test, expect } from "bun:test"; +import type { DisplayedMessage } from "@/common/types/message"; +import { StreamingMessageAggregator } from "./StreamingMessageAggregator"; + +const TEST_CREATED_AT = "2024-01-01T00:00:00.000Z"; + +type ToolMessage = Extract; + +function isAgentReportToolMessage(msg: DisplayedMessage, toolCallId: string): msg is ToolMessage { + return msg.type === "tool" && msg.toolCallId === toolCallId && msg.toolName === "agent_report"; +} + +describe("StreamingMessageAggregator - agent_report tool-call-delta preview", () => { + test("materializes agent_report tool part from tool-call-delta and updates it", () => { + const aggregator = new StreamingMessageAggregator(TEST_CREATED_AT); + + const workspaceId = "test-workspace"; + const messageId = "msg-1"; + const toolCallId = "tool-1"; + const baseTs = 1000; + + aggregator.handleStreamStart({ + type: "stream-start", + workspaceId, + messageId, + historySequence: 1, + model: "test-model", + startTime: baseTs, + }); + + // tool-call-delta arrives before tool-call-start. + aggregator.handleToolCallDelta({ + type: "tool-call-delta", + workspaceId, + messageId, + toolCallId, + toolName: "agent_report", + delta: '{"reportMarkdown":"Hello\\nWor', + tokens: 5, + timestamp: baseTs + 1, + }); + + let displayed = aggregator.getDisplayedMessages(); + let toolMsgs = displayed.filter((m): m is ToolMessage => + isAgentReportToolMessage(m, toolCallId) + ); + expect(toolMsgs).toHaveLength(1); + expect(toolMsgs[0].status).toBe("executing"); + expect(toolMsgs[0].args).toEqual({ reportMarkdown: "Hello\nWor" }); + + // Subsequent deltas should patch the in-flight tool args. + aggregator.handleToolCallDelta({ + type: "tool-call-delta", + workspaceId, + messageId, + toolCallId, + toolName: "agent_report", + delta: 'ld","title":"Result"}', + tokens: 5, + timestamp: baseTs + 2, + }); + + displayed = aggregator.getDisplayedMessages(); + toolMsgs = displayed.filter((m): m is ToolMessage => isAgentReportToolMessage(m, toolCallId)); + expect(toolMsgs).toHaveLength(1); + expect(toolMsgs[0].status).toBe("executing"); + expect(toolMsgs[0].args).toEqual({ reportMarkdown: "Hello\nWorld", title: "Result" }); + + // When tool-call-start arrives later, it should update the existing part instead of creating a duplicate. + aggregator.handleToolCallStart({ + type: "tool-call-start", + workspaceId, + messageId, + toolCallId, + toolName: "agent_report", + args: { reportMarkdown: "Hello\nWorld", title: "Result" }, + tokens: 10, + timestamp: baseTs + 3, + }); + + displayed = aggregator.getDisplayedMessages(); + toolMsgs = displayed.filter((m): m is ToolMessage => isAgentReportToolMessage(m, toolCallId)); + expect(toolMsgs).toHaveLength(1); + expect(toolMsgs[0].args).toEqual({ reportMarkdown: "Hello\nWorld", title: "Result" }); + }); +}); diff --git a/src/browser/utils/messages/StreamingMessageAggregator.ts b/src/browser/utils/messages/StreamingMessageAggregator.ts index 4182fbff1f..f1343b5371 100644 --- a/src/browser/utils/messages/StreamingMessageAggregator.ts +++ b/src/browser/utils/messages/StreamingMessageAggregator.ts @@ -33,6 +33,7 @@ import { z } from "zod"; import { createDeltaStorage, type DeltaRecordStorage } from "./StreamingTPSCalculator"; import { computeRecencyTimestamp } from "./recency"; import { assert } from "@/common/utils/assert"; +import { extractAgentReportArgsFromArgsText } from "@/common/utils/tools/agentReportArgsText"; import { getStatusStateKey } from "@/common/constants/storage"; // Maximum number of messages to display in the DOM for performance @@ -164,6 +165,12 @@ export class StreamingMessageAggregator { private cachedDisplayedMessages: DisplayedMessage[] | null = null; private recencyTimestamp: number | null = null; + // Track streamed tool-call args text (argsTextDelta) for in-flight rendering. + // Keyed by `${messageId}:${toolCallId}` to make cleanup on stream end trivial. + private readonly pendingToolArgsTextByToolCallKey = new Map(); + // Track which tool calls have already emitted tool-call-delta events so we can avoid + // double-counting tool input tokens when the subsequent tool-call-start arrives. + private readonly toolCallKeysWithDeltas = new Set(); // Delta history for token counting and TPS calculation private deltaHistory = new Map(); @@ -526,6 +533,20 @@ export class StreamingMessageAggregator { this.sessionTimingStats[statsKey] = modelStats; } + // Clean up any in-flight tool-call-delta arg text we may have accumulated for this stream. + // (tool-call-delta events can arrive before tool-call-start, and we keep argsText in memory + // only for live UI rendering). + const toolCallKeyPrefix = `${messageId}:`; + for (const key of Array.from(this.pendingToolArgsTextByToolCallKey.keys())) { + if (key.startsWith(toolCallKeyPrefix)) { + this.pendingToolArgsTextByToolCallKey.delete(key); + } + } + for (const key of Array.from(this.toolCallKeysWithDeltas.values())) { + if (key.startsWith(toolCallKeyPrefix)) { + this.toolCallKeysWithDeltas.delete(key); + } + } this.activeStreams.delete(messageId); // Clear todos when stream ends - they're stream-scoped state // On reload, todos will be reconstructed from completed tool_write calls in history @@ -1131,6 +1152,12 @@ export class StreamingMessageAggregator { const message = this.messages.get(data.messageId); if (!message) return; + const toolCallKey = `${data.messageId}:${data.toolCallId}`; + const hadToolCallDeltas = this.toolCallKeysWithDeltas.has(toolCallKey); + // Deltas are only useful while the call is being assembled. + this.toolCallKeysWithDeltas.delete(toolCallKey); + this.pendingToolArgsTextByToolCallKey.delete(toolCallKey); + // If this is a nested call (from PTC code_execution), add to parent's nestedCalls if (data.parentToolCallId) { const parentPart = message.parts.find( @@ -1158,16 +1185,33 @@ export class StreamingMessageAggregator { part.type === "dynamic-tool" && part.toolCallId === data.toolCallId ); - if (existingToolPart) { - console.warn(`Tool call ${data.toolCallId} already exists, skipping duplicate`); - return; - } - - // Track tool start time for execution duration calculation + // Track tool start time for execution duration calculation. + // NOTE: Some tools (agent_report) may start their timing from tool-call-delta, so preserve the + // earliest timestamp. const context = this.activeStreams.get(data.messageId); if (context) { this.updateStreamClock(context, data.timestamp); - context.pendingToolStarts.set(data.toolCallId, data.timestamp); + if (!context.pendingToolStarts.has(data.toolCallId)) { + context.pendingToolStarts.set(data.toolCallId, data.timestamp); + } + } + + if (existingToolPart) { + // If we already created a tool part from tool-call-delta, update it in-place. + if (!hadToolCallDeltas) { + console.warn(`Tool call ${data.toolCallId} already exists, skipping duplicate`); + return; + } + + assert( + existingToolPart.toolName === data.toolName, + "handleToolCallStart: existing tool part has unexpected toolName" + ); + + existingToolPart.input = data.args; + + this.invalidateCache(); + return; } // Add tool part to maintain temporal order @@ -1181,16 +1225,84 @@ export class StreamingMessageAggregator { }; message.parts.push(toolPart as never); - // Track tokens for tool input - this.trackDelta(data.messageId, data.tokens, data.timestamp, "tool-args"); + // Track tokens for tool input. + // tool-call-delta already covers streamed args text, so avoid double-counting. + if (!hadToolCallDeltas) { + this.trackDelta(data.messageId, data.tokens, data.timestamp, "tool-args"); + } this.invalidateCache(); } handleToolCallDelta(data: ToolCallDeltaEvent): void { + const toolCallKey = `${data.messageId}:${data.toolCallId}`; + this.toolCallKeysWithDeltas.add(toolCallKey); + // Track delta for token counting and TPS calculation this.trackDelta(data.messageId, data.tokens, data.timestamp, "tool-args"); - // Tool deltas are for display - args are in dynamic-tool part + + // Only agent_report gets special in-flight rendering today. + if (data.toolName !== "agent_report") { + return; + } + + // tool-call-delta payloads come from the AI SDK and should always be a string argsTextDelta, + // but stay defensive to avoid crashing the UI on unexpected inputs. + if (typeof data.delta !== "string" || data.delta.length === 0) { + return; + } + + const message = this.messages.get(data.messageId); + if (!message) return; + + const context = this.activeStreams.get(data.messageId); + if (context) { + this.updateStreamClock(context, data.timestamp); + // Start tool timing from the first delta so "report writing" time is visible in the UI. + if (!context.pendingToolStarts.has(data.toolCallId)) { + context.pendingToolStarts.set(data.toolCallId, data.timestamp); + } + } + + const nextArgsText = + (this.pendingToolArgsTextByToolCallKey.get(toolCallKey) ?? "") + data.delta; + this.pendingToolArgsTextByToolCallKey.set(toolCallKey, nextArgsText); + + const extracted = extractAgentReportArgsFromArgsText(nextArgsText); + + // Render args even while incomplete. Tool components treat empty strings as "no data yet". + const partialArgs: { reportMarkdown: string; title?: string } = { + reportMarkdown: extracted.reportMarkdown ?? "", + }; + if (typeof extracted.title === "string" && extracted.title.length > 0) { + partialArgs.title = extracted.title; + } + + // Ensure a tool part exists even before tool-call-start so the UI can stream the preview. + const existingToolPart = message.parts.find( + (part): part is DynamicToolPart => + part.type === "dynamic-tool" && part.toolCallId === data.toolCallId + ); + + if (existingToolPart) { + assert( + existingToolPart.toolName === data.toolName, + "handleToolCallDelta: existing tool part has unexpected toolName" + ); + existingToolPart.input = partialArgs; + } else { + const toolPart: DynamicToolPartPending = { + type: "dynamic-tool", + toolCallId: data.toolCallId, + toolName: data.toolName, + state: "input-available", + input: partialArgs, + timestamp: data.timestamp, + }; + message.parts.push(toolPart as never); + } + + this.invalidateCache(); } /** diff --git a/src/browser/utils/ui/dateTime.ts b/src/browser/utils/ui/dateTime.ts index a668dc4b1b..6e869981c4 100644 --- a/src/browser/utils/ui/dateTime.ts +++ b/src/browser/utils/ui/dateTime.ts @@ -75,3 +75,16 @@ export function formatRelativeTime(timestamp: number): string { return years === 1 ? "1 year ago" : `${years} years ago`; } } + +/** + * Compact relative time format for space-constrained UI (tooltips, badges). + * Examples: "5s ago", "2m ago", "3h ago", "1d ago" + */ +export function formatRelativeTimeCompact(timestamp: number): string { + const elapsed = Math.floor((Date.now() - timestamp) / 1000); + if (elapsed < 0) return "now"; + if (elapsed < 60) return `${elapsed}s ago`; + if (elapsed < 3600) return `${Math.floor(elapsed / 60)}m ago`; + if (elapsed < 86400) return `${Math.floor(elapsed / 3600)}h ago`; + return `${Math.floor(elapsed / 86400)}d ago`; +} diff --git a/src/common/utils/tools/agentReportArgsText.test.ts b/src/common/utils/tools/agentReportArgsText.test.ts new file mode 100644 index 0000000000..7999901b6c --- /dev/null +++ b/src/common/utils/tools/agentReportArgsText.test.ts @@ -0,0 +1,45 @@ +import { describe, test, expect } from "bun:test"; +import { extractAgentReportArgsFromArgsText } from "./agentReportArgsText"; + +describe("extractAgentReportArgsFromArgsText", () => { + test("extracts reportMarkdown and title from complete JSON", () => { + const argsText = JSON.stringify({ + reportMarkdown: "Hello\\nWorld", + title: "Result", + }); + + const extracted = extractAgentReportArgsFromArgsText(argsText); + expect(extracted).toEqual({ reportMarkdown: "Hello\\nWorld", title: "Result" }); + }); + + test("returns best-effort partial reportMarkdown when JSON is truncated", () => { + const partial = '{"reportMarkdown":"Hello\\nWor'; + const extracted = extractAgentReportArgsFromArgsText(partial); + expect(extracted.reportMarkdown).toBe("Hello\nWor"); + }); + + test("tolerates truncated escape sequences", () => { + const partial = '{"reportMarkdown":"Hello\\'; + const extracted = extractAgentReportArgsFromArgsText(partial); + // Trailing backslash is ignored. + expect(extracted.reportMarkdown).toBe("Hello"); + }); + + test("decodes unicode escapes when complete and ignores incomplete unicode", () => { + const complete = '{"reportMarkdown":"Hi \\u263A"}'; + expect(extractAgentReportArgsFromArgsText(complete).reportMarkdown).toBe("Hi ☺"); + + const incomplete = '{"reportMarkdown":"Hi \\u26"}'; + expect(extractAgentReportArgsFromArgsText(incomplete).reportMarkdown).toBe("Hi "); + }); + + test("does not treat key-like text inside strings as a key", () => { + const argsText = JSON.stringify({ + reportMarkdown: 'contains "reportMarkdown":"nope"', + title: "T", + }); + const extracted = extractAgentReportArgsFromArgsText(argsText); + expect(extracted.reportMarkdown).toBe('contains "reportMarkdown":"nope"'); + expect(extracted.title).toBe("T"); + }); +}); diff --git a/src/common/utils/tools/agentReportArgsText.ts b/src/common/utils/tools/agentReportArgsText.ts new file mode 100644 index 0000000000..1fbc727073 --- /dev/null +++ b/src/common/utils/tools/agentReportArgsText.ts @@ -0,0 +1,199 @@ +import { assert } from "@/common/utils/assert"; + +export interface PartialAgentReportArgsFromArgsText { + /** + * The decoded (best-effort) report markdown. + * + * This can be incomplete when the JSON args are still streaming. + */ + reportMarkdown: string | null; + /** + * The decoded (best-effort) title. + * + * This can be incomplete when the JSON args are still streaming. + */ + title: string | null; +} + +function isAsciiWhitespace(char: string): boolean { + // Keep fast + predictable (avoid regex for perf in tight loops). + return char === " " || char === "\t" || char === "\n" || char === "\r"; +} + +function decodePartialJsonString(argsText: string, startQuoteIndex: number): string { + assert( + startQuoteIndex >= 0 && startQuoteIndex < argsText.length, + "decodePartialJsonString: startQuoteIndex out of range" + ); + assert(argsText[startQuoteIndex] === '"', "decodePartialJsonString: expected opening quote"); + + const out: string[] = []; + const len = argsText.length; + let i = startQuoteIndex + 1; + + while (i < len) { + const c = argsText[i]; + + if (c === '"') { + // Closing quote. + break; + } + + if (c !== "\\") { + out.push(c); + i += 1; + continue; + } + + // Escape sequence. + i += 1; + if (i >= len) { + // Truncated escape sequence (best-effort: ignore). + break; + } + + const esc = argsText[i]; + switch (esc) { + case '"': + case "\\": + case "/": { + out.push(esc); + i += 1; + break; + } + case "b": { + out.push("\b"); + i += 1; + break; + } + case "f": { + out.push("\f"); + i += 1; + break; + } + case "n": { + out.push("\n"); + i += 1; + break; + } + case "r": { + out.push("\r"); + i += 1; + break; + } + case "t": { + out.push("\t"); + i += 1; + break; + } + case "u": { + // Unicode escape sequence: \uXXXX + const remaining = len - (i + 1); + if (remaining < 4) { + // Truncated unicode escape. Stop decoding here. + return out.join(""); + } + + const hex = argsText.slice(i + 1, i + 5); + if (!/^[0-9a-fA-F]{4}$/.test(hex)) { + // Malformed unicode escape. Best-effort: stop decoding. + return out.join(""); + } + + out.push(String.fromCharCode(Number.parseInt(hex, 16))); + i += 5; + break; + } + default: { + // Unknown escape. Best-effort: keep escaped char. + out.push(esc); + i += 1; + } + } + } + + return out.join(""); +} + +function extractJsonStringValue(argsText: string, key: string): string | null { + assert(typeof argsText === "string", "extractJsonStringValue: argsText must be a string"); + assert(key.length > 0, "extractJsonStringValue: key must be non-empty"); + + // Streaming args are a JSON object encoded as text, e.g. + // {"reportMarkdown":"...","title":"..."} + // + // We do a single pass to find a top-level string key token while skipping over + // string literals to avoid false matches inside reportMarkdown content. + const len = argsText.length; + let inString = false; + let escaped = false; + + for (let i = 0; i < len; i++) { + const c = argsText[i]; + + if (inString) { + if (escaped) { + escaped = false; + continue; + } + if (c === "\\") { + escaped = true; + continue; + } + if (c === '"') { + inString = false; + } + continue; + } + + if (c !== '"') { + continue; + } + + // Start of a JSON string token. + if (argsText.startsWith(key, i + 1) && argsText[i + 1 + key.length] === '"') { + let j = i + 1 + key.length + 1; + while (j < len && isAsciiWhitespace(argsText[j])) j += 1; + if (j >= len || argsText[j] !== ":") { + // Not a key token (could be inside a larger string, etc.). + // Continue scanning. + inString = true; + escaped = false; + continue; + } + j += 1; + while (j < len && isAsciiWhitespace(argsText[j])) j += 1; + if (j >= len || argsText[j] !== '"') { + // Value isn't a string (or it's not present yet). + return null; + } + + return decodePartialJsonString(argsText, j); + } + + // Not our key; skip over this string token. + inString = true; + escaped = false; + } + + return null; +} + +/** + * Extract best-effort `agent_report` tool args from a partially streamed JSON args string. + * + * This is intentionally tolerant to truncated JSON and truncated escape sequences. + */ +export function extractAgentReportArgsFromArgsText( + argsText: string +): PartialAgentReportArgsFromArgsText { + assert( + typeof argsText === "string", + "extractAgentReportArgsFromArgsText: argsText must be a string" + ); + + return { + reportMarkdown: extractJsonStringValue(argsText, "reportMarkdown"), + title: extractJsonStringValue(argsText, "title"), + }; +} diff --git a/src/node/services/taskService.test.ts b/src/node/services/taskService.test.ts index c6ec24cc25..2739857cc7 100644 --- a/src/node/services/taskService.test.ts +++ b/src/node/services/taskService.test.ts @@ -1553,6 +1553,17 @@ describe("TaskService", () => { expect.objectContaining({ workspaceId: childId }) ); + expect(remove).not.toHaveBeenCalled(); + + await ( + taskService as unknown as { + cleanupReportedLeafTask: ( + workspaceId: string, + options?: { skipDelay?: boolean } + ) => Promise; + } + ).cleanupReportedLeafTask(childId, { skipDelay: true }); + expect(remove).toHaveBeenCalled(); expect(resumeStream).toHaveBeenCalled(); expect(emit).toHaveBeenCalled(); @@ -1690,6 +1701,17 @@ describe("TaskService", () => { } } + expect(remove).not.toHaveBeenCalled(); + + await ( + taskService as unknown as { + cleanupReportedLeafTask: ( + workspaceId: string, + options?: { skipDelay?: boolean } + ) => Promise; + } + ).cleanupReportedLeafTask(childId, { skipDelay: true }); + expect(remove).toHaveBeenCalled(); expect(resumeStream).toHaveBeenCalled(); }); @@ -1807,6 +1829,17 @@ describe("TaskService", () => { .find((w) => w.id === childId); expect(ws?.taskStatus).toBe("reported"); + expect(remove).not.toHaveBeenCalled(); + + await ( + taskService as unknown as { + cleanupReportedLeafTask: ( + workspaceId: string, + options?: { skipDelay?: boolean } + ) => Promise; + } + ).cleanupReportedLeafTask(childId, { skipDelay: true }); + expect(remove).toHaveBeenCalled(); expect(resumeStream).toHaveBeenCalled(); }); @@ -1944,6 +1977,17 @@ describe("TaskService", () => { .find((w) => w.id === childId); expect(ws?.taskStatus).toBe("reported"); + expect(remove).not.toHaveBeenCalled(); + + await ( + taskService as unknown as { + cleanupReportedLeafTask: ( + workspaceId: string, + options?: { skipDelay?: boolean } + ) => Promise; + } + ).cleanupReportedLeafTask(childId, { skipDelay: true }); + expect(remove).toHaveBeenCalled(); expect(resumeStream).toHaveBeenCalled(); }); diff --git a/src/node/services/taskService.ts b/src/node/services/taskService.ts index 44e53c6be5..f95b746707 100644 --- a/src/node/services/taskService.ts +++ b/src/node/services/taskService.ts @@ -75,6 +75,10 @@ type AgentTaskWorkspaceEntry = WorkspaceConfigEntry & { projectPath: string }; const COMPLETED_REPORT_CACHE_TTL_MS = 60 * 60 * 1000; // 1 hour const COMPLETED_REPORT_CACHE_MAX_ENTRIES = 128; +// Delay auto-deleting reported agent task workspaces so users can see the final report/tool output +// before the workspace disappears. +const REPORTED_TASK_WORKSPACE_AUTO_DELETE_DELAY_MS = 5000; + interface AgentTaskIndex { byId: Map; childrenByParent: Map; @@ -177,6 +181,10 @@ export class TaskService { string, { reportMarkdown: string; title?: string; expiresAtMs: number } >(); + + // Best-effort delayed cleanup of reported task workspaces. + // Keyed by workspaceId to avoid scheduling duplicate deletes. + private readonly scheduledTaskWorkspaceDeletes = new Map>(); private readonly remindedAwaitingReport = new Set(); constructor( @@ -2088,9 +2096,33 @@ export class TaskService { return true; } - private async cleanupReportedLeafTask(workspaceId: string): Promise { + private async cleanupReportedLeafTask( + workspaceId: string, + options?: { skipDelay?: boolean } + ): Promise { assert(workspaceId.length > 0, "cleanupReportedLeafTask: workspaceId must be non-empty"); + // UX: Delay deletion briefly after agent_report so the user can see the final tool output. + if (!options?.skipDelay) { + if (this.scheduledTaskWorkspaceDeletes.has(workspaceId)) { + return; + } + + const timeout = setTimeout(() => { + this.scheduledTaskWorkspaceDeletes.delete(workspaceId); + void this.cleanupReportedLeafTask(workspaceId, { skipDelay: true }).catch( + (error: unknown) => { + log.error("cleanupReportedLeafTask: delayed cleanup failed", { workspaceId, error }); + } + ); + }, REPORTED_TASK_WORKSPACE_AUTO_DELETE_DELAY_MS); + + // Do not keep the process alive just for this best-effort cleanup. + timeout.unref?.(); + this.scheduledTaskWorkspaceDeletes.set(workspaceId, timeout); + return; + } + let currentWorkspaceId = workspaceId; const visited = new Set(); for (let depth = 0; depth < 32; depth++) { @@ -2116,6 +2148,13 @@ export class TaskService { ); if (hasChildren) return; + // Best-effort: if we previously scheduled deletion, clear the timer now. + const scheduled = this.scheduledTaskWorkspaceDeletes.get(currentWorkspaceId); + if (scheduled) { + clearTimeout(scheduled); + this.scheduledTaskWorkspaceDeletes.delete(currentWorkspaceId); + } + const removeResult = await this.workspaceService.remove(currentWorkspaceId, true); if (!removeResult.success) { log.error("Failed to auto-delete reported task workspace", {