diff --git a/src/crates/core/src/agentic/coordination/coordinator.rs b/src/crates/core/src/agentic/coordination/coordinator.rs index a0e094134..69ad48e74 100644 --- a/src/crates/core/src/agentic/coordination/coordinator.rs +++ b/src/crates/core/src/agentic/coordination/coordinator.rs @@ -1251,6 +1251,7 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet total_tools: 1, duration_ms: outcome.duration_ms, subagent_parent_info: None, + partial_recovery_reason: None, }) .await; diff --git a/src/crates/core/src/agentic/execution/execution_engine.rs b/src/crates/core/src/agentic/execution/execution_engine.rs index 99e9b6fac..7688e6cff 100644 --- a/src/crates/core/src/agentic/execution/execution_engine.rs +++ b/src/crates/core/src/agentic/execution/execution_engine.rs @@ -1213,6 +1213,7 @@ impl ExecutionEngine { let mut round_index = 0; let mut completed_rounds = 0usize; let mut total_tools = 0; + let mut last_partial_recovery_reason: Option = None; let mut last_assistant_message = Message::assistant("".to_string()); let mut finalization_reason: Option<&'static str> = None; let mut consecutive_compression_failures: u32 = 0; @@ -1565,6 +1566,11 @@ impl ExecutionEngine { total_tools += round_result.tool_calls.len(); + // Track partial recovery reason from the last round + if round_result.partial_recovery_reason.is_some() { + last_partial_recovery_reason = round_result.partial_recovery_reason.clone(); + } + // P0: Consecutive same-tool-call loop detection if !round_result.tool_calls.is_empty() { let mut sigs: Vec = round_result @@ -1754,6 +1760,7 @@ impl ExecutionEngine { total_tools, duration_ms, subagent_parent_info: event_subagent_parent_info, + partial_recovery_reason: last_partial_recovery_reason, }, None, ) diff --git a/src/crates/core/src/agentic/execution/round_executor.rs b/src/crates/core/src/agentic/execution/round_executor.rs index f43752ed6..03d26e6e5 100644 --- a/src/crates/core/src/agentic/execution/round_executor.rs +++ b/src/crates/core/src/agentic/execution/round_executor.rs @@ -208,6 +208,8 @@ impl RoundExecutor { } let no_effective_output = !result.has_effective_output; + let is_partial_recovery = result.partial_recovery_reason.is_some(); + if no_effective_output && attempt_index < max_attempts - 1 { let delay_ms = Self::retry_delay_ms(attempt_index); warn!( @@ -222,6 +224,23 @@ impl RoundExecutor { attempt_index += 1; continue; } + + if is_partial_recovery && attempt_index < max_attempts - 1 { + let delay_ms = Self::retry_delay_ms(attempt_index); + warn!( + "Retrying stream after partial recovery: session_id={}, round_id={}, attempt={}/{}, delay_ms={}, reason={}", + context.session_id, + round_id, + attempt_index + 1, + max_attempts, + delay_ms, + result.partial_recovery_reason.as_deref().unwrap_or("unknown") + ); + tokio::time::sleep(Duration::from_millis(delay_ms)).await; + attempt_index += 1; + continue; + } + break result; } Err(stream_err) => { @@ -362,6 +381,7 @@ impl RoundExecutor { finish_reason: FinishReason::Complete, usage: stream_result.usage.clone(), provider_metadata: stream_result.provider_metadata.clone(), + partial_recovery_reason: stream_result.partial_recovery_reason.clone(), }); } @@ -567,6 +587,7 @@ impl RoundExecutor { }, usage: stream_result.usage.clone(), provider_metadata: stream_result.provider_metadata.clone(), + partial_recovery_reason: stream_result.partial_recovery_reason.clone(), }) } diff --git a/src/crates/core/src/agentic/execution/types.rs b/src/crates/core/src/agentic/execution/types.rs index 7c1917ba6..d0ba86e9a 100644 --- a/src/crates/core/src/agentic/execution/types.rs +++ b/src/crates/core/src/agentic/execution/types.rs @@ -60,6 +60,9 @@ pub struct RoundResult { pub usage: Option, /// Provider-specific metadata returned by the model. pub provider_metadata: Option, + /// When set, this round's stream was partially recovered (aborted mid-way + /// but some output was already received). Contains a human-readable reason. + pub partial_recovery_reason: Option, } /// Finish reason diff --git a/src/crates/core/src/agentic/session/session_manager.rs b/src/crates/core/src/agentic/session/session_manager.rs index 49e2b200b..3ac93b213 100644 --- a/src/crates/core/src/agentic/session/session_manager.rs +++ b/src/crates/core/src/agentic/session/session_manager.rs @@ -1042,7 +1042,8 @@ impl SessionManager { // Reset session state to Idle // After application restart, previous Processing state is invalid and must be reset - if !matches!(session.state, SessionState::Idle) { + let previous_state_was_not_idle = !matches!(session.state, SessionState::Idle); + if previous_state_was_not_idle { let old_state = session.state.clone(); session.state = SessionState::Idle; debug!( @@ -1159,6 +1160,34 @@ impl SessionManager { context_msg_count ); + // Mark session as having unread completion if it was previously running (not Idle). + // This handles both normal app close and abnormal crash scenarios. + if previous_state_was_not_idle { + if let Ok(Some(mut metadata)) = self + .persistence_manager + .load_session_metadata(&session_storage_path, session_id) + .await + { + if metadata.unread_completion.is_none() { + debug!( + "Marking session as having unread completion (was interrupted during restore): session_id={}", + session_id + ); + metadata.unread_completion = Some("completed".to_string()); + if let Err(e) = self + .persistence_manager + .save_session_metadata(&session_storage_path, &metadata) + .await + { + warn!( + "Failed to save unread_completion metadata: session_id={}, error={}", + session_id, e + ); + } + } + } + } + // 4. Add to memory (will overwrite if already exists) self.sessions .insert(session_id.to_string(), session.clone()); diff --git a/src/crates/events/src/agentic.rs b/src/crates/events/src/agentic.rs index 074c3c17b..36d4776d3 100644 --- a/src/crates/events/src/agentic.rs +++ b/src/crates/events/src/agentic.rs @@ -138,6 +138,10 @@ pub enum AgenticEvent { total_tools: usize, duration_ms: u64, subagent_parent_info: Option, + /// When set, the turn finished but the last model round was a partial + /// recovery (stream aborted mid-way). Contains a human-readable reason. + #[serde(skip_serializing_if = "Option::is_none")] + partial_recovery_reason: Option, }, DialogTurnCancelled { diff --git a/src/crates/transport/src/adapters/tauri.rs b/src/crates/transport/src/adapters/tauri.rs index 0acdf181c..f88bc9ab8 100644 --- a/src/crates/transport/src/adapters/tauri.rs +++ b/src/crates/transport/src/adapters/tauri.rs @@ -194,6 +194,7 @@ impl TransportAdapter for TauriTransportAdapter { session_id, turn_id, subagent_parent_info, + partial_recovery_reason, .. } => { self.app_handle.emit( @@ -202,6 +203,7 @@ impl TransportAdapter for TauriTransportAdapter { "sessionId": session_id, "turnId": turn_id, "subagentParentInfo": subagent_parent_info, + "partialRecoveryReason": partial_recovery_reason, }), )?; } diff --git a/src/crates/transport/src/adapters/websocket.rs b/src/crates/transport/src/adapters/websocket.rs index b6f26df6a..8b0916e78 100644 --- a/src/crates/transport/src/adapters/websocket.rs +++ b/src/crates/transport/src/adapters/websocket.rs @@ -164,6 +164,7 @@ impl TransportAdapter for WebSocketTransportAdapter { session_id, turn_id, subagent_parent_info, + partial_recovery_reason, .. } => { json!({ @@ -171,6 +172,7 @@ impl TransportAdapter for WebSocketTransportAdapter { "sessionId": session_id, "turnId": turn_id, "subagentParentInfo": subagent_parent_info, + "partialRecoveryReason": partial_recovery_reason, }) } _ => return Ok(()), diff --git a/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.scss b/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.scss index ab767375a..34760b814 100644 --- a/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.scss +++ b/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.scss @@ -282,7 +282,8 @@ 0 0 0 1px color-mix(in srgb, var(--color-success, #22c55e) 35%, transparent), 0 0 6px color-mix(in srgb, var(--color-success, #22c55e) 42%, transparent); - &.is-error { + &.is-error, + &.is-interrupted { background: var(--color-error, #ef4444); box-shadow: 0 0 0 1px color-mix(in srgb, var(--color-error, #ef4444) 35%, transparent), diff --git a/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.tsx b/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.tsx index f181726c3..f62bc32c1 100644 --- a/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.tsx +++ b/src/web-ui/src/app/components/NavPanel/sections/sessions/SessionsSection.tsx @@ -532,6 +532,7 @@ const SessionsSection: React.FC = ({ className={[ 'bitfun-nav-panel__inline-item-unread-dot', attentionKind === 'error' && 'is-error', + attentionKind === 'interrupted' && 'is-interrupted', attentionKind === 'ask_user' && 'is-ask-user', attentionKind === 'tool_confirm' && 'is-tool-confirm', isHighPriority && 'is-high-priority', @@ -539,11 +540,13 @@ const SessionsSection: React.FC = ({ aria-label={ attentionKind === 'error' ? t('nav.sessions.unreadError') - : attentionKind === 'ask_user' - ? t('nav.sessions.needsUserInput') - : attentionKind === 'tool_confirm' - ? t('nav.sessions.needsToolConfirm') - : t('nav.sessions.unreadCompleted') + : attentionKind === 'interrupted' + ? t('nav.sessions.unreadInterrupted') + : attentionKind === 'ask_user' + ? t('nav.sessions.needsUserInput') + : attentionKind === 'tool_confirm' + ? t('nav.sessions.needsToolConfirm') + : t('nav.sessions.unreadCompleted') } /> ) : null} diff --git a/src/web-ui/src/flow_chat/components/btw/BtwSessionPanel.scss b/src/web-ui/src/flow_chat/components/btw/BtwSessionPanel.scss index 7f8ffddc6..32ae26e16 100644 --- a/src/web-ui/src/flow_chat/components/btw/BtwSessionPanel.scss +++ b/src/web-ui/src/flow_chat/components/btw/BtwSessionPanel.scss @@ -179,4 +179,53 @@ &--has-action-bar &__scroll-to-bottom { bottom: 140px; } + + /* Minimized review action bar indicator */ + &__minimized-indicator { + position: absolute; + bottom: 14px; + left: 14px; + z-index: 11; + pointer-events: auto; + } + + &__minimized-button { + display: flex; + align-items: center; + gap: 8px; + padding: 8px 14px; + border-radius: 20px; + background: var(--color-bg-secondary); + border: 1px solid var(--border-base); + color: var(--color-text-primary); + font-size: 12px; + font-weight: 500; + cursor: pointer; + box-shadow: 0 2px 12px rgba(0, 0, 0, 0.1); + transition: transform 0.2s ease, box-shadow 0.2s ease; + + &:hover { + transform: translateY(-1px); + box-shadow: 0 4px 16px rgba(0, 0, 0, 0.15); + } + + svg { + flex-shrink: 0; + color: var(--color-accent-500); + } + } + + &__minimized-text { + white-space: nowrap; + } + + &__minimized-count { + padding: 2px 8px; + border-radius: 10px; + background: var(--element-bg-soft); + font-size: 11px; + color: var(--color-text-secondary); + font-variant-numeric: tabular-nums; + flex-shrink: 0; + } } diff --git a/src/web-ui/src/flow_chat/components/btw/BtwSessionPanel.tsx b/src/web-ui/src/flow_chat/components/btw/BtwSessionPanel.tsx index 0422140d7..e48b3e978 100644 --- a/src/web-ui/src/flow_chat/components/btw/BtwSessionPanel.tsx +++ b/src/web-ui/src/flow_chat/components/btw/BtwSessionPanel.tsx @@ -1,7 +1,7 @@ import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; import {useTranslation} from 'react-i18next'; import path from 'path-browserify'; -import {CornerUpLeft, Link2, Square} from 'lucide-react'; +import {CornerUpLeft, Link2, Square, Sparkles} from 'lucide-react'; import {FlowChatContext} from '../modern/FlowChatContext'; import {VirtualItemRenderer} from '../modern/VirtualItemRenderer'; import {ProcessingIndicator} from '../modern/ProcessingIndicator'; @@ -23,7 +23,9 @@ import {findLatestCodeReviewResult} from '../../utils/reviewSessionSummary'; import {deriveDeepReviewInterruption} from '../../utils/deepReviewContinuation'; import {buildReviewRemediationItems, type CodeReviewRemediationData} from '../../utils/codeReviewRemediation'; import {ReviewActionBar} from './DeepReviewActionBar'; -import {type ReviewActionMode, useReviewActionBarStore} from '../../store/deepReviewActionBarStore'; +import {type ReviewActionMode, type ReviewActionPhase, useReviewActionBarStore} from '../../store/deepReviewActionBarStore'; +import {loadPersistedReviewState} from '../../services/ReviewActionBarPersistenceService'; +import type {ReviewActionPersistedState} from '@/shared/types/session-history'; import './BtwSessionPanel.scss'; export interface BtwSessionPanelProps { @@ -269,14 +271,28 @@ export const BtwSessionPanel: React.FC = ({ // ---- Review action bar integration ---- const actionBarPhase = useReviewActionBarStore((s) => s.phase); const actionBarDismissed = useReviewActionBarStore((s) => s.dismissed); + const actionBarMinimized = useReviewActionBarStore((s) => s.minimized); const actionBarChildSessionId = useReviewActionBarStore((s) => s.childSessionId); + const actionBarCompletedIds = useReviewActionBarStore((s) => s.completedRemediationIds); + const actionBarRemediationItems = useReviewActionBarStore((s) => s.remediationItems); const isDeepReview = childKind === 'deep_review'; const isReviewSession = childKind === 'review' || childKind === 'deep_review'; const showReviewActionBar = isReviewSession && actionBarChildSessionId === childSessionId && actionBarPhase !== 'idle' && - !actionBarDismissed; + !actionBarDismissed && + !actionBarMinimized; + + const showMinimizedIndicator = + isReviewSession && + actionBarChildSessionId === childSessionId && + actionBarPhase !== 'idle' && + !actionBarDismissed && + actionBarMinimized; + + const remainingCount = actionBarRemediationItems.length - actionBarCompletedIds.size; + const totalCount = actionBarRemediationItems.length; // Detect when a review completes with a remediation plan and auto-show the action bar. useEffect(() => { @@ -313,7 +329,7 @@ export const BtwSessionPanel: React.FC = ({ // Only activate if the action bar is idle or not yet shown for this session if (store.childSessionId === childSessionId && store.phase !== 'idle') { // Update phase based on turn status if currently showing - if (isError && store.phase !== 'fix_failed' && store.phase !== 'review_error') { + if (isError && store.phase !== 'fix_failed' && store.phase !== 'review_error' && store.phase !== 'fix_interrupted') { store.updatePhase( store.phase === 'fix_running' ? 'fix_failed' : 'review_error', childSession.error ?? undefined, @@ -326,6 +342,7 @@ export const BtwSessionPanel: React.FC = ({ reviewData: latestReviewData, reviewMode, phase: 'review_completed', + completedRemediationIds: store.completedRemediationIds, }); } else { // Fix completed with no further remediation needed — dismiss the action bar @@ -360,6 +377,78 @@ export const BtwSessionPanel: React.FC = ({ } }, [childSession, childSessionId, parentSessionId, isReviewSession, isDeepReview]); + // Restore persisted review action state on mount + useEffect(() => { + if (!isReviewSession || !childSessionId || !childSession) return; + + const store = useReviewActionBarStore.getState(); + // Only restore if store is idle for this session + if (store.phase !== 'idle' || store.childSessionId) return; + + const workspacePath = childSession.workspacePath; + if (!workspacePath) return; + + let cancelled = false; + + loadPersistedReviewState( + childSessionId, + workspacePath, + childSession.remoteConnectionId, + childSession.remoteSshHost, + ).then((persisted: ReviewActionPersistedState | null) => { + if (cancelled || !persisted) return; + + const latestReviewData = findLatestCodeReviewResult(childSession) as DeepReviewActionData | null; + const reviewMode: ReviewActionMode = isDeepReview ? 'deep' : 'standard'; + + // Detect fix interruption + let phase: ReviewActionPhase = persisted.phase as ReviewActionPhase; + let remainingFixIds: string[] = []; + + if (persisted.phase === 'fix_running') { + const lastTurn = childSession.dialogTurns[childSession.dialogTurns.length - 1]; + const isStillRunning = lastTurn?.status === 'processing' || lastTurn?.status === 'finishing'; + + if (!isStillRunning) { + // Fix was interrupted — determine remaining items + phase = 'fix_interrupted'; + const latestItems = latestReviewData ? buildReviewRemediationItems(latestReviewData) : []; + const latestIds = new Set(latestItems.map((i) => i.id)); + // Items that were being fixed but still exist in latest review data + remainingFixIds = persisted.completedRemediationIds.filter((id: string) => latestIds.has(id)); + } + } + + store.showActionBar({ + childSessionId, + parentSessionId: parentSessionId ?? null, + reviewData: latestReviewData ?? ({} as CodeReviewRemediationData), + reviewMode, + phase, + completedRemediationIds: new Set(persisted.completedRemediationIds), + }); + + // Apply additional restored state + store.setCustomInstructions(persisted.customInstructions); + if (persisted.minimized) { + store.minimize(); + } + if (remainingFixIds.length > 0) { + // Set remaining fix IDs in the store + // We need to access the store state directly to set this + const currentState = useReviewActionBarStore.getState(); + // Use a type-safe approach + (currentState as unknown as { remainingFixIds: string[] }).remainingFixIds = remainingFixIds; + } + }).catch(() => { + // Ignore persistence load errors + }); + + return () => { + cancelled = true; + }; + }, [childSession, childSessionId, parentSessionId, isReviewSession, isDeepReview]); + // Observe action bar height to adjust body padding dynamically useEffect(() => { if (!showReviewActionBar) { @@ -529,6 +618,29 @@ export const BtwSessionPanel: React.FC = ({ onClick={handleScrollToBottom} className="btw-session-panel__scroll-to-bottom" /> + {showMinimizedIndicator && ( +
+ +
+ )} + {showReviewActionBar && (
diff --git a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.scss b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.scss index 19c4220ca..ae43bbcc3 100644 --- a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.scss +++ b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.scss @@ -222,6 +222,27 @@ &:hover { background: var(--element-bg-subtle); } + + &--completed { + opacity: 0.55; + cursor: default; + + &:hover { + background: transparent; + } + + .deep-review-action-bar__remediation-text { + text-decoration: line-through; + color: var(--color-text-muted); + } + } + } + + &__completed-icon { + color: var(--color-success, #22c55e); + margin-right: 4px; + flex-shrink: 0; + vertical-align: text-bottom; } &__remediation-text { @@ -347,6 +368,24 @@ flex-wrap: wrap; } + /* Interruption notice */ + &__interruption-notice { + display: flex; + align-items: center; + gap: 8px; + padding: 8px 10px; + border-radius: 6px; + background: var(--element-bg-subtle); + font-size: 12px; + color: var(--color-text-secondary); + margin-bottom: 8px; + } + + &__interruption-icon { + color: var(--color-warning, #f59e0b); + flex-shrink: 0; + } + /* Animations */ @keyframes deep-review-action-bar-spin { from { diff --git a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.test.tsx b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.test.tsx index 987b8433f..c02df409c 100644 --- a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.test.tsx +++ b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.test.tsx @@ -189,4 +189,138 @@ describeWithJsdom('DeepReviewActionBar', () => { expect(displayMessage).toBe('Fix Code Review findings and re-review'); expect(agentType).toBe('CodeReview'); }); + + it('minimizes action bar when close button is clicked', async () => { + const { DeepReviewActionBar } = await import('./DeepReviewActionBar'); + + useReviewActionBarStore.getState().showActionBar({ + childSessionId: 'child-session', + parentSessionId: 'parent-session', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + }, + phase: 'review_completed', + }); + + await act(async () => { + root.render(); + }); + + const closeButton = container.querySelector('.deep-review-action-bar__close'); + expect(closeButton).toBeTruthy(); + + await act(async () => { + closeButton!.dispatchEvent(new dom.window.MouseEvent('click', { bubbles: true })); + await Promise.resolve(); + }); + + expect(useReviewActionBarStore.getState().minimized).toBe(true); + expect(useReviewActionBarStore.getState().dismissed).toBe(false); + }); + + it('marks completed remediation items when fix completes', async () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-session', + parentSessionId: 'parent-session', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + }, + phase: 'review_completed', + }); + + // Select all items + const items = store.remediationItems; + for (const item of items) { + store.toggleRemediation(item.id); + } + + store.setActiveAction('fix'); + store.updatePhase('fix_running'); + + // Simulate fix completion + store.updatePhase('fix_completed'); + + const state = useReviewActionBarStore.getState(); + expect(state.completedRemediationIds.size).toBe(2); + expect(state.phase).toBe('fix_completed'); + expect(state.fixingRemediationIds.size).toBe(0); + }); + + it('shows completed items as disabled and strikethrough', async () => { + const { DeepReviewActionBar } = await import('./DeepReviewActionBar'); + + useReviewActionBarStore.getState().showActionBar({ + childSessionId: 'child-session', + parentSessionId: 'parent-session', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + }, + phase: 'review_completed', + completedRemediationIds: new Set(['remediation-0']), + }); + + await act(async () => { + root.render(); + }); + + const completedItem = container.querySelector('.deep-review-action-bar__remediation-item--completed'); + expect(completedItem).toBeTruthy(); + + const checkboxes = container.querySelectorAll('input[type="checkbox"]'); + expect(checkboxes.length).toBeGreaterThanOrEqual(2); + }); + + it('shows continue fix UI when phase is fix_interrupted', async () => { + const { DeepReviewActionBar } = await import('./DeepReviewActionBar'); + + useReviewActionBarStore.getState().showActionBar({ + childSessionId: 'child-session', + parentSessionId: 'parent-session', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + }, + phase: 'fix_interrupted', + }); + + // Set remaining fix IDs directly on state + const store = useReviewActionBarStore.getState(); + (store as unknown as { remainingFixIds: string[] }).remainingFixIds = ['remediation-0']; + + await act(async () => { + root.render(); + }); + + const continueButton = Array.from(container.querySelectorAll('button')) + .find((button) => button.textContent?.includes('Continue fixing')); + expect(continueButton).toBeTruthy(); + + const skipButton = Array.from(container.querySelectorAll('button')) + .find((button) => button.textContent?.includes('Skip remaining')); + expect(skipButton).toBeTruthy(); + }); + + it('skips remaining fixes and returns to review_completed', async () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-session', + parentSessionId: 'parent-session', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + }, + phase: 'fix_interrupted', + }); + + store.skipRemainingFixes(); + + const state = useReviewActionBarStore.getState(); + expect(state.phase).toBe('review_completed'); + expect(state.remainingFixIds).toEqual([]); + expect(state.activeAction).toBeNull(); + }); }); diff --git a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.tsx b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.tsx index 14ecba7f9..f483f5a74 100644 --- a/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.tsx +++ b/src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.tsx @@ -12,6 +12,7 @@ import { MessageSquare, Play, Copy, + Minimize2, } from 'lucide-react'; import { Button, Checkbox, Tooltip } from '@/component-library'; import { useReviewActionBarStore, type ReviewActionPhase } from '../../store/deepReviewActionBarStore'; @@ -40,6 +41,7 @@ const PHASE_CONFIG: Record { customInstructions, errorMessage, interruption, + completedRemediationIds, + remainingFixIds, } = store; const [showCustomInput, setShowCustomInput] = useState(false); @@ -117,15 +121,17 @@ export const ReviewActionBar: React.FC = () => { store.toggleGroupRemediation(groupId as RemediationGroupId); }, [store]); - const handleStartFixing = useCallback(async (rerunReview: boolean) => { + const handleStartFixing = useCallback(async (rerunReview: boolean, overrideSelectedIds?: Set) => { if (!reviewData || !childSessionId) return; + const idsToFix = overrideSelectedIds ?? selectedRemediationIds; const action = rerunReview ? 'fix-review' : 'fix'; let prompt = buildSelectedReviewRemediationPrompt({ reviewData, - selectedIds: selectedRemediationIds, + selectedIds: idsToFix, rerunReview, reviewMode, + completedItems: [...completedRemediationIds], }); if (!prompt) return; @@ -175,7 +181,7 @@ export const ReviewActionBar: React.FC = () => { } finally { store.setActiveAction(null); } - }, [reviewData, childSessionId, selectedRemediationIds, customInstructions, reviewMode, isDeepReview, store, t]); + }, [reviewData, childSessionId, selectedRemediationIds, customInstructions, reviewMode, isDeepReview, store, t, completedRemediationIds]); const handleFillBackInput = useCallback(() => { if (!reviewData) return; @@ -205,6 +211,10 @@ export const ReviewActionBar: React.FC = () => { store.dismiss(); }, [store]); + const handleMinimize = useCallback(() => { + store.minimize(); + }, [store]); + const handleContinueReview = useCallback(async () => { if (!interruption) return; @@ -243,48 +253,76 @@ export const ReviewActionBar: React.FC = () => { } }, [childSessionId, interruption, store, t]); - const handleCopyDiagnostics = useCallback(() => { + const handleContinueFix = useCallback(async () => { + if (!reviewData || !childSessionId || remainingFixIds.length === 0) return; + + const remainingSet = new Set(remainingFixIds); + store.setSelectedRemediationIds(remainingSet); + + await handleStartFixing(false, remainingSet); + }, [reviewData, childSessionId, remainingFixIds, store, handleStartFixing]); + + const handleCopyDiagnostics = useCallback(async () => { const detail = interruption?.errorDetail; if (!detail) return; const presentation = getAiErrorPresentation(detail); - const lines: string[] = []; - lines.push(t('deepReviewActionBar.diagnosticsTitle', { defaultValue: '=== Deep Review Interruption Diagnostics ===' })); - lines.push(''); - - const categoryLabel = t(presentation.titleKey, { defaultValue: presentation.category }); - const categoryMessage = t(presentation.messageKey, { defaultValue: '' }); - lines.push(`${t('deepReviewActionBar.diagnosticsErrorType', { defaultValue: 'Error type' })}: ${categoryLabel} (${presentation.category})`); - if (categoryMessage) { - lines.push(`${t('deepReviewActionBar.diagnosticsDescription', { defaultValue: 'Description' })}: ${categoryMessage}`); - } - lines.push(''); + // Prefer the sanitized diagnostics from aiErrorPresenter if available + let diagnostics = presentation.diagnostics; + if (!diagnostics) { + const lines: string[] = []; + lines.push(t('deepReviewActionBar.diagnosticsTitle', { defaultValue: '=== Deep Review Interruption Diagnostics ===' })); + lines.push(''); - if (presentation.actions.length > 0) { - const actionLabels = presentation.actions.map((action) => { - return t(action.labelKey, { defaultValue: action.code }); - }); - lines.push(`${t('deepReviewActionBar.diagnosticsSuggestedActions', { defaultValue: 'Suggested actions' })}: ${actionLabels.join(', ')}`); + const categoryLabel = t(presentation.titleKey, { defaultValue: presentation.category }); + const categoryMessage = t(presentation.messageKey, { defaultValue: '' }); + lines.push(`${t('deepReviewActionBar.diagnosticsErrorType', { defaultValue: 'Error type' })}: ${categoryLabel} (${presentation.category})`); + if (categoryMessage) { + lines.push(`${t('deepReviewActionBar.diagnosticsDescription', { defaultValue: 'Description' })}: ${categoryMessage}`); + } lines.push(''); - } - lines.push(`${t('deepReviewActionBar.diagnosticsTechnicalDetails', { defaultValue: 'Technical details' })}:`); - lines.push(` - category: ${detail.category ?? 'unknown'}`); - if (detail.provider) lines.push(` - provider: ${detail.provider}`); - if (detail.providerCode) lines.push(` - provider code: ${detail.providerCode}`); - if (detail.providerMessage) lines.push(` - provider message: ${detail.providerMessage}`); - if (detail.httpStatus) lines.push(` - HTTP status: ${detail.httpStatus}`); - if (detail.requestId) lines.push(` - request ID: ${detail.requestId}`); - if (detail.rawMessage) { - lines.push(` - raw message: ${detail.rawMessage}`); + if (presentation.actions.length > 0) { + const actionLabels = presentation.actions.map((action) => { + return t(action.labelKey, { defaultValue: action.code }); + }); + lines.push(`${t('deepReviewActionBar.diagnosticsSuggestedActions', { defaultValue: 'Suggested actions' })}: ${actionLabels.join(', ')}`); + lines.push(''); + } + + lines.push(`${t('deepReviewActionBar.diagnosticsTechnicalDetails', { defaultValue: 'Technical details' })}:`); + lines.push(` - category: ${detail.category ?? 'unknown'}`); + if (detail.provider) lines.push(` - provider: ${detail.provider}`); + if (detail.providerCode) lines.push(` - provider code: ${detail.providerCode}`); + if (detail.providerMessage) { + const msg = detail.providerMessage.length > 500 + ? `${detail.providerMessage.slice(0, 500)}... [truncated]` + : detail.providerMessage; + lines.push(` - provider message: ${msg}`); + } + if (detail.httpStatus) lines.push(` - HTTP status: ${detail.httpStatus}`); + if (detail.requestId) lines.push(` - request ID: ${detail.requestId}`); + if (detail.rawMessage) { + const raw = detail.rawMessage.length > 500 + ? `${detail.rawMessage.slice(0, 500)}... [truncated]` + : detail.rawMessage; + lines.push(` - raw message: ${raw}`); + } + + diagnostics = lines.join('\n'); } - const diagnostics = lines.join('\n'); - void navigator.clipboard?.writeText(diagnostics); - notificationService.success(t('deepReviewActionBar.diagnosticsCopied', { - defaultValue: 'Diagnostics copied', - }), { duration: 2500 }); + try { + await navigator.clipboard.writeText(diagnostics); + notificationService.success(t('deepReviewActionBar.diagnosticsCopied', { + defaultValue: 'Diagnostics copied', + }), { duration: 2500 }); + } catch { + notificationService.error(t('deepReviewActionBar.diagnosticsCopyFailed', { + defaultValue: 'Failed to copy diagnostics', + }), { duration: 2500 }); + } }, [interruption, t]); const phaseTitle = useMemo(() => { @@ -343,8 +381,8 @@ export const ReviewActionBar: React.FC = () => { @@ -421,23 +459,35 @@ export const ReviewActionBar: React.FC = () => {
- {items.map((item: ReviewRemediationItem) => ( - - ))} + {items.map((item: ReviewRemediationItem) => { + const isCompleted = completedRemediationIds.has(item.id); + return ( + + ); + })}
); @@ -563,6 +613,38 @@ export const ReviewActionBar: React.FC = () => { )} + {phase === 'fix_interrupted' && ( + <> +
+ + + {t('deepReviewActionBar.fixInterrupted', { + defaultValue: 'Fix was interrupted. {{count}} items remain.', + count: remainingFixIds.length, + })} + +
+ + + + )} + {(phase === 'fix_completed' || phase === 'fix_failed' || phase === 'fix_timeout' || phase === 'review_error' || phase === 'resume_failed') && ( )} + + {(phase === 'review_completed' || phase === 'fix_interrupted') && ( + + )} ); diff --git a/src/web-ui/src/flow_chat/components/modern/ExploreRegion.scss b/src/web-ui/src/flow_chat/components/modern/ExploreRegion.scss index d479d5991..95b344c2e 100644 --- a/src/web-ui/src/flow_chat/components/modern/ExploreRegion.scss +++ b/src/web-ui/src/flow_chat/components/modern/ExploreRegion.scss @@ -13,7 +13,8 @@ // ==================== Non-collapsible (streaming / no auto-collapse yet) ==================== .explore-region--expanded:not(.explore-region--collapsible) { - margin: 4px 0; + margin-top: 4px; + margin-bottom: 4px; .explore-region__content { position: relative; diff --git a/src/web-ui/src/flow_chat/components/modern/ModelRoundItem.tsx b/src/web-ui/src/flow_chat/components/modern/ModelRoundItem.tsx index 1131a15cc..7d44e07f6 100644 --- a/src/web-ui/src/flow_chat/components/modern/ModelRoundItem.tsx +++ b/src/web-ui/src/flow_chat/components/modern/ModelRoundItem.tsx @@ -543,15 +543,21 @@ const SubagentItemsContainer = React.memo(({ return () => cancelAnimationFrame(rafId); }, [isCollapsed, scrollSignal]); + // Don't render the scrollable container when there are no items yet. + // This prevents an empty white area from showing before subagent content arrives. + if (items.length === 0) { + return null; + } + return (
-
{items.map((item, idx) => ( - ({ + sessionAPI: { + saveSessionMetadata: vi.fn().mockResolvedValue(undefined), + loadSessionMetadata: vi.fn().mockResolvedValue(undefined), + }, +})); + +vi.mock('../store/FlowChatStore', () => ({ + flowChatStore: { + getState: vi.fn().mockReturnValue({ + sessions: new Map(), + }), + }, +})); + +describe('ReviewActionBarPersistenceService', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + describe('persistReviewActionState', () => { + it('does nothing when childSessionId is null', async () => { + await persistReviewActionState({ + childSessionId: null, + parentSessionId: null, + reviewMode: 'deep', + phase: 'review_completed', + reviewData: null, + remediationItems: [], + selectedRemediationIds: new Set(), + dismissed: false, + minimized: false, + activeAction: null, + customInstructions: '', + errorMessage: null, + interruption: null, + completedRemediationIds: new Set(), + fixingRemediationIds: new Set(), + remainingFixIds: [], + } as any); + + expect(sessionAPI.saveSessionMetadata).not.toHaveBeenCalled(); + }); + + it('does nothing when session is not found in FlowChatStore', async () => { + await persistReviewActionState({ + childSessionId: 'session-1', + parentSessionId: null, + reviewMode: 'deep', + phase: 'review_completed', + reviewData: null, + remediationItems: [], + selectedRemediationIds: new Set(), + dismissed: false, + minimized: false, + activeAction: null, + customInstructions: '', + errorMessage: null, + interruption: null, + completedRemediationIds: new Set(), + fixingRemediationIds: new Set(), + remainingFixIds: [], + } as any); + + expect(sessionAPI.saveSessionMetadata).not.toHaveBeenCalled(); + }); + + it('saves metadata with reviewActionState when session exists', async () => { + const mockSession = { + workspacePath: '/workspace/project', + remoteConnectionId: undefined, + remoteSshHost: undefined, + }; + + (flowChatStore.getState as any).mockReturnValue({ + sessions: new Map([['session-1', mockSession]]), + }); + + await persistReviewActionState({ + childSessionId: 'session-1', + parentSessionId: null, + reviewMode: 'deep', + phase: 'review_completed', + reviewData: null, + remediationItems: [], + selectedRemediationIds: new Set(), + dismissed: false, + minimized: true, + activeAction: null, + customInstructions: 'custom instruction', + errorMessage: null, + interruption: null, + completedRemediationIds: new Set(['remediation-0']), + fixingRemediationIds: new Set(), + remainingFixIds: [], + } as any); + + expect(sessionAPI.saveSessionMetadata).toHaveBeenCalledTimes(1); + const [metadata, workspacePath] = (sessionAPI.saveSessionMetadata as any).mock.calls[0]; + expect(metadata.sessionId).toBe('session-1'); + expect(metadata.reviewActionState).toEqual({ + version: 1, + phase: 'review_completed', + completedRemediationIds: ['remediation-0'], + minimized: true, + customInstructions: 'custom instruction', + persistedAt: expect.any(Number), + }); + expect(workspacePath).toBe('/workspace/project'); + }); + + it('passes remote connection info when available', async () => { + const mockSession = { + workspacePath: '/workspace/project', + remoteConnectionId: 'remote-1', + remoteSshHost: 'ssh-host-1', + }; + + (flowChatStore.getState as any).mockReturnValue({ + sessions: new Map([['session-1', mockSession]]), + }); + + await persistReviewActionState({ + childSessionId: 'session-1', + parentSessionId: null, + reviewMode: 'deep', + phase: 'fix_running', + reviewData: null, + remediationItems: [], + selectedRemediationIds: new Set(), + dismissed: false, + minimized: false, + activeAction: null, + customInstructions: '', + errorMessage: null, + interruption: null, + completedRemediationIds: new Set(), + fixingRemediationIds: new Set(), + remainingFixIds: [], + } as any); + + expect(sessionAPI.saveSessionMetadata).toHaveBeenCalledTimes(1); + const [, , remoteConnectionId, remoteSshHost] = (sessionAPI.saveSessionMetadata as any).mock.calls[0]; + expect(remoteConnectionId).toBe('remote-1'); + expect(remoteSshHost).toBe('ssh-host-1'); + }); + }); + + describe('clearPersistedReviewState', () => { + it('saves metadata with undefined reviewActionState', async () => { + await clearPersistedReviewState('session-1', '/workspace/project'); + + expect(sessionAPI.saveSessionMetadata).toHaveBeenCalledTimes(1); + const [metadata] = (sessionAPI.saveSessionMetadata as any).mock.calls[0]; + expect(metadata.sessionId).toBe('session-1'); + expect(metadata.reviewActionState).toBeUndefined(); + }); + }); + + describe('loadPersistedReviewState', () => { + it('returns null when no metadata exists', async () => { + (sessionAPI.loadSessionMetadata as any).mockResolvedValue(undefined); + + const result = await loadPersistedReviewState('session-1', '/workspace/project'); + expect(result).toBeNull(); + }); + + it('returns null when metadata has no reviewActionState', async () => { + (sessionAPI.loadSessionMetadata as any).mockResolvedValue({ + sessionId: 'session-1', + title: 'Test Session', + }); + + const result = await loadPersistedReviewState('session-1', '/workspace/project'); + expect(result).toBeNull(); + }); + + it('returns persisted state when metadata has reviewActionState', async () => { + const persistedState = { + version: 1, + phase: 'fix_running', + completedRemediationIds: ['remediation-0'], + minimized: false, + customInstructions: 'test instruction', + persistedAt: Date.now(), + }; + + (sessionAPI.loadSessionMetadata as any).mockResolvedValue({ + sessionId: 'session-1', + reviewActionState: persistedState, + }); + + const result = await loadPersistedReviewState('session-1', '/workspace/project'); + expect(result).toEqual(persistedState); + }); + + it('passes remote connection info when loading', async () => { + (sessionAPI.loadSessionMetadata as any).mockResolvedValue(undefined); + + await loadPersistedReviewState('session-1', '/workspace/project', 'remote-1', 'ssh-host-1'); + + expect(sessionAPI.loadSessionMetadata).toHaveBeenCalledWith( + 'session-1', + '/workspace/project', + 'remote-1', + 'ssh-host-1', + ); + }); + + it('returns null and does not throw on error', async () => { + (sessionAPI.loadSessionMetadata as any).mockRejectedValue(new Error('Network error')); + + const result = await loadPersistedReviewState('session-1', '/workspace/project'); + expect(result).toBeNull(); + }); + }); +}); diff --git a/src/web-ui/src/flow_chat/services/ReviewActionBarPersistenceService.ts b/src/web-ui/src/flow_chat/services/ReviewActionBarPersistenceService.ts new file mode 100644 index 000000000..0b8b92f94 --- /dev/null +++ b/src/web-ui/src/flow_chat/services/ReviewActionBarPersistenceService.ts @@ -0,0 +1,79 @@ +/** + * Review Action Bar persistence service. + * + * Persists review action bar state to session metadata via the backend API, + * aligning with the existing session persistence architecture. + */ + +import { createLogger } from '@/shared/utils/logger'; +import { sessionAPI } from '@/infrastructure/api/service-api/SessionAPI'; +import { flowChatStore } from '../store/FlowChatStore'; +import type { ReviewActionBarState } from '../store/deepReviewActionBarStore'; +import type { ReviewActionPersistedState } from '@/shared/types/session-history'; + +const log = createLogger('ReviewActionBarPersistence'); + +export async function persistReviewActionState(state: ReviewActionBarState): Promise { + if (!state.childSessionId) return; + + const session = flowChatStore.getState().sessions.get(state.childSessionId); + if (!session?.workspacePath) return; + + const payload: ReviewActionPersistedState = { + version: 1, + phase: state.phase, + completedRemediationIds: [...state.completedRemediationIds], + minimized: state.minimized, + customInstructions: state.customInstructions, + persistedAt: Date.now(), + }; + + try { + await sessionAPI.saveSessionMetadata( + { + sessionId: state.childSessionId, + reviewActionState: payload, + } as any, + session.workspacePath, + session.remoteConnectionId, + session.remoteSshHost + ); + } catch (error) { + log.warn('Failed to persist review action state', { sessionId: state.childSessionId, error }); + throw error; + } +} + +export async function clearPersistedReviewState(sessionId: string, workspacePath: string): Promise { + try { + await sessionAPI.saveSessionMetadata( + { + sessionId, + reviewActionState: undefined, + } as any, + workspacePath + ); + } catch (error) { + log.warn('Failed to clear persisted review action state', { sessionId, error }); + } +} + +export async function loadPersistedReviewState( + sessionId: string, + workspacePath: string, + remoteConnectionId?: string, + remoteSshHost?: string +): Promise { + try { + const metadata = await sessionAPI.loadSessionMetadata( + sessionId, + workspacePath, + remoteConnectionId, + remoteSshHost + ); + return metadata?.reviewActionState ?? null; + } catch (error) { + log.warn('Failed to load persisted review action state', { sessionId, error }); + return null; + } +} diff --git a/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.ts b/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.ts index 7a8f63ff2..c6072cedf 100644 --- a/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.ts +++ b/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.ts @@ -564,13 +564,14 @@ function schedulePendingTurnCompletion( }, TURN_COMPLETION_QUIET_WINDOW_MS); } -function beginTurnCompletion(context: FlowChatContext, sessionId: string, turnId: string): void { +function beginTurnCompletion(context: FlowChatContext, sessionId: string, turnId: string, partialRecoveryReason?: string): void { clearPendingTurnCompletion(context, sessionId); context.pendingTurnCompletions.set(sessionId, { turnId, lastActivityAt: Date.now(), timer: null, + partialRecoveryReason, }); schedulePendingTurnCompletion(context, sessionId, turnId); @@ -645,7 +646,10 @@ function finalizeTurnCompletionState( // Mark unread completion for non-active sessions const activeSessionId = store.getState().activeSessionId; if (sessionId !== activeSessionId) { - context.flowChatStore.markSessionUnreadCompletion(sessionId, 'completed'); + const pending = context.pendingTurnCompletions.get(sessionId); + const isPartialRecovery = !!pending?.partialRecoveryReason; + // Partial recovery after retry failure is treated as an error state (red dot) + context.flowChatStore.markSessionUnreadCompletion(sessionId, isPartialRecovery ? 'interrupted' : 'completed'); } clearPendingTurnCompletion(context, sessionId, turnId); @@ -1513,6 +1517,8 @@ function handleDialogTurnComplete( const sessionId = event?.sessionId ?? event?.session_id; const turnId = event?.turnId ?? event?.turn_id; const subagentParentInfo = normalizeSubagentParentInfo(event); + // Partial recovery reason from backend (stream was interrupted mid-way) + const partialRecoveryReason = event?.partialRecoveryReason ?? event?.partial_recovery_reason; if (subagentParentInfo) { if (sessionId) { @@ -1561,7 +1567,7 @@ function handleDialogTurnComplete( log.debug('Skipping BACKEND_STREAM_COMPLETED transition', { currentState, sessionId, turnId }); } - beginTurnCompletion(context, sessionId, turnId); + beginTurnCompletion(context, sessionId, turnId, partialRecoveryReason); } /** diff --git a/src/web-ui/src/flow_chat/services/flow-chat-manager/MessageModule.ts b/src/web-ui/src/flow_chat/services/flow-chat-manager/MessageModule.ts index f6f3254a9..9a59e91ed 100644 --- a/src/web-ui/src/flow_chat/services/flow-chat-manager/MessageModule.ts +++ b/src/web-ui/src/flow_chat/services/flow-chat-manager/MessageModule.ts @@ -185,7 +185,8 @@ export async function sendMessage( dialogTurnId, }); if (!startOk) { - throw new Error('Session is still busy finishing the previous turn'); + const currentState = stateMachineManager.getCurrentState(sessionId); + throw new Error(`Session is still busy finishing the previous turn (current state: ${currentState})`); } if (isFirstMessage) { diff --git a/src/web-ui/src/flow_chat/services/flow-chat-manager/PersistenceModule.ts b/src/web-ui/src/flow_chat/services/flow-chat-manager/PersistenceModule.ts index b8079b709..1f814cb3a 100644 --- a/src/web-ui/src/flow_chat/services/flow-chat-manager/PersistenceModule.ts +++ b/src/web-ui/src/flow_chat/services/flow-chat-manager/PersistenceModule.ts @@ -279,12 +279,26 @@ export async function saveAllInProgressTurns(context: FlowChatContext): Promise< interruptionReason: 'app_restart', }) ); - + + // Mark session as unread if it was interrupted while not active + if (sessionId !== state.activeSessionId) { + context.flowChatStore.markSessionUnreadCompletion(sessionId, 'completed'); + } + savePromises.push( saveDialogTurnToDisk(context, sessionId, lastTurn.id).catch(error => { log.error('Failed to save in-progress turn', { sessionId, turnId: lastTurn.id, error }); }) ); + + // Also persist the unread completion metadata + if (sessionId !== state.activeSessionId) { + savePromises.push( + updateSessionMetadata(context, sessionId).catch(error => { + log.error('Failed to update session metadata for unread completion', { sessionId, error }); + }) + ); + } } } } diff --git a/src/web-ui/src/flow_chat/services/flow-chat-manager/types.ts b/src/web-ui/src/flow_chat/services/flow-chat-manager/types.ts index 2e29aecdb..fa3468aa1 100644 --- a/src/web-ui/src/flow_chat/services/flow-chat-manager/types.ts +++ b/src/web-ui/src/flow_chat/services/flow-chat-manager/types.ts @@ -18,6 +18,8 @@ export interface FlowChatContext { turnId: string; lastActivityAt: number; timer: ReturnType | null; + /** Set when the turn completed with a partial stream recovery. */ + partialRecoveryReason?: string; }>; /** In-flight historical session hydration: sessionId -> promise */ pendingHistoryLoads: Map>; diff --git a/src/web-ui/src/flow_chat/store/FlowChatStore.ts b/src/web-ui/src/flow_chat/store/FlowChatStore.ts index be9e9931f..51a2777ae 100644 --- a/src/web-ui/src/flow_chat/store/FlowChatStore.ts +++ b/src/web-ui/src/flow_chat/store/FlowChatStore.ts @@ -48,7 +48,7 @@ export class FlowChatStore { private state: FlowChatState; private listeners: Set<(state: FlowChatState) => void> = new Set(); private silentMode = false; - private onPersistUnreadCompletion?: (sessionId: string, value: 'completed' | 'error' | undefined) => void; + private onPersistUnreadCompletion?: (sessionId: string, value: 'completed' | 'error' | 'interrupted' | undefined) => void; private constructor() { this.clearOldStorage(); @@ -201,7 +201,7 @@ export class FlowChatStore { * Called by FlowChatManager during initialization. */ public registerPersistUnreadCompletionCallback( - callback: (sessionId: string, value: 'completed' | 'error' | undefined) => void + callback: (sessionId: string, value: 'completed' | 'error' | 'interrupted' | undefined) => void ): void { this.onPersistUnreadCompletion = callback; } @@ -1270,6 +1270,7 @@ export class FlowChatStore { const updatedSession: Session = { ...session, + lastActiveAt: timestamp, lastFinishedAt: timestamp, }; @@ -1285,7 +1286,7 @@ export class FlowChatStore { public markSessionUnreadCompletion( sessionId: string, - completionKind: 'completed' | 'error' + completionKind: 'completed' | 'error' | 'interrupted' ): void { this.setState(prev => { const session = prev.sessions.get(sessionId); @@ -1723,6 +1724,10 @@ export class FlowChatStore { sessions: newSessions, }; }); + + // Reset state machine to IDLE after loading history + // This handles the case where restoreSession triggered events that left the state machine in PROCESSING + stateMachineManager.reset(sessionId); } catch (error) { log.error('Failed to load session history', { sessionId, error }); throw error; @@ -1921,7 +1926,6 @@ export class FlowChatStore { const updatedSession = { ...session, todos: updatedTodos, - lastActiveAt: Date.now() }; const newSessions = new Map(prev.sessions); @@ -1966,7 +1970,6 @@ export class FlowChatStore { const updatedSession = { ...session, todos: [...todos], - lastActiveAt: Date.now() }; const newSessions = new Map(prev.sessions); diff --git a/src/web-ui/src/flow_chat/store/deepReviewActionBarStore.test.ts b/src/web-ui/src/flow_chat/store/deepReviewActionBarStore.test.ts new file mode 100644 index 000000000..3d6f9d471 --- /dev/null +++ b/src/web-ui/src/flow_chat/store/deepReviewActionBarStore.test.ts @@ -0,0 +1,233 @@ +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest'; +import { useReviewActionBarStore } from './deepReviewActionBarStore'; + +vi.mock('../services/ReviewActionBarPersistenceService', () => ({ + persistReviewActionState: vi.fn().mockResolvedValue(undefined), + clearPersistedReviewState: vi.fn().mockResolvedValue(undefined), + loadPersistedReviewState: vi.fn().mockResolvedValue(null), +})); + +describe('deepReviewActionBarStore', () => { + beforeEach(() => { + useReviewActionBarStore.getState().reset(); + }); + + afterEach(() => { + useReviewActionBarStore.getState().reset(); + vi.clearAllMocks(); + }); + + describe('showActionBar', () => { + it('initializes with default selected remediation IDs', () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-1', + parentSessionId: 'parent-1', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + }, + }); + + expect(store.childSessionId).toBe('child-1'); + expect(store.phase).toBe('review_completed'); + expect(store.selectedRemediationIds.size).toBeGreaterThan(0); + expect(store.completedRemediationIds.size).toBe(0); + expect(store.minimized).toBe(false); + }); + + it('preserves completedRemediationIds when re-showing for same session', () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-1', + parentSessionId: 'parent-1', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + }, + completedRemediationIds: new Set(['remediation-0']), + }); + + expect(store.completedRemediationIds.has('remediation-0')).toBe(true); + // Completed items should not be in selected by default + expect(store.selectedRemediationIds.has('remediation-0')).toBe(false); + }); + + it('filters out completed IDs that no longer exist in new review data', () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-1', + parentSessionId: 'parent-1', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 2'], + }, + completedRemediationIds: new Set(['remediation-0', 'remediation-1']), + }); + + // remediation-0 does not exist in new data with only 1 item + expect(store.completedRemediationIds.has('remediation-0')).toBe(false); + }); + }); + + describe('minimize and restore', () => { + it('minimizes the action bar', () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-1', + parentSessionId: 'parent-1', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1'], + }, + }); + + store.minimize(); + expect(store.minimized).toBe(true); + expect(store.phase).toBe('review_completed'); + }); + + it('restores the action bar from minimized state', () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-1', + parentSessionId: 'parent-1', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1'], + }, + }); + + store.minimize(); + store.restore(); + expect(store.minimized).toBe(false); + }); + }); + + describe('fix lifecycle', () => { + it('snapshots selected IDs when starting fix', () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-1', + parentSessionId: 'parent-1', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + }, + }); + + // Select first item + store.toggleRemediation('remediation-0'); + store.setActiveAction('fix'); + + expect(store.fixingRemediationIds.has('remediation-0')).toBe(true); + }); + + it('moves fixing IDs to completed when fix completes', () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-1', + parentSessionId: 'parent-1', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + }, + }); + + store.toggleRemediation('remediation-0'); + store.setActiveAction('fix'); + store.updatePhase('fix_running'); + store.updatePhase('fix_completed'); + + expect(store.completedRemediationIds.has('remediation-0')).toBe(true); + expect(store.fixingRemediationIds.size).toBe(0); + expect(store.phase).toBe('fix_completed'); + }); + + it('does not mark items as completed on fix_failed', () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-1', + parentSessionId: 'parent-1', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1'], + }, + }); + + store.toggleRemediation('remediation-0'); + store.setActiveAction('fix'); + store.updatePhase('fix_running'); + store.updatePhase('fix_failed', 'Something went wrong'); + + expect(store.completedRemediationIds.has('remediation-0')).toBe(false); + expect(store.phase).toBe('fix_failed'); + expect(store.errorMessage).toBe('Something went wrong'); + }); + }); + + describe('skipRemainingFixes', () => { + it('returns to review_completed and clears remaining fix IDs', () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-1', + parentSessionId: 'parent-1', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1'], + }, + phase: 'fix_interrupted', + }); + + store.skipRemainingFixes(); + + expect(store.phase).toBe('review_completed'); + expect(store.remainingFixIds).toEqual([]); + expect(store.activeAction).toBeNull(); + }); + }); + + describe('toggleRemediation with completed items', () => { + it('does not allow toggling completed items', () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-1', + parentSessionId: 'parent-1', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + }, + completedRemediationIds: new Set(['remediation-0']), + }); + + // Completed item should not be selected by default + expect(store.selectedRemediationIds.has('remediation-0')).toBe(false); + + // Toggle should work on non-completed items + store.toggleRemediation('remediation-1'); + expect(store.selectedRemediationIds.has('remediation-1')).toBe(true); + }); + }); + + describe('reset', () => { + it('clears all state back to initial', () => { + const store = useReviewActionBarStore.getState(); + store.showActionBar({ + childSessionId: 'child-1', + parentSessionId: 'parent-1', + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1'], + }, + }); + + store.minimize(); + store.reset(); + + expect(store.phase).toBe('idle'); + expect(store.childSessionId).toBeNull(); + expect(store.minimized).toBe(false); + expect(store.completedRemediationIds.size).toBe(0); + }); + }); +}); diff --git a/src/web-ui/src/flow_chat/store/deepReviewActionBarStore.ts b/src/web-ui/src/flow_chat/store/deepReviewActionBarStore.ts index 9c7034013..5c6ff3aa2 100644 --- a/src/web-ui/src/flow_chat/store/deepReviewActionBarStore.ts +++ b/src/web-ui/src/flow_chat/store/deepReviewActionBarStore.ts @@ -27,6 +27,7 @@ export type ReviewActionPhase = | 'fix_completed' | 'fix_failed' | 'fix_timeout' + | 'fix_interrupted' | 'review_interrupted' | 'resume_blocked' | 'resume_running' @@ -52,6 +53,8 @@ export interface ReviewActionBarState { selectedRemediationIds: Set; /** Whether the action bar was dismissed by the user */ dismissed: boolean; + /** Whether the action bar is minimized (collapsed to a floating button) */ + minimized: boolean; /** Which fix action is currently in flight */ activeAction: 'fix' | 'fix-review' | 'resume' | null; /** User-supplied custom instructions (from the textarea) */ @@ -60,6 +63,12 @@ export interface ReviewActionBarState { errorMessage: string | null; /** Structured interruption state used to continue an incomplete Deep Review */ interruption: DeepReviewInterruption | null; + /** IDs of remediation items that have been fixed/completed */ + completedRemediationIds: Set; + /** IDs of items being fixed in the current fix_running session (snapshot at start) */ + fixingRemediationIds: Set; + /** IDs of items remaining when a fix was interrupted */ + remainingFixIds: string[]; // ---- actions ---- showActionBar: (params: { @@ -68,6 +77,7 @@ export interface ReviewActionBarState { reviewData: CodeReviewRemediationData; reviewMode?: ReviewActionMode; phase?: ReviewActionPhase; + completedRemediationIds?: Set; }) => void; showInterruptedActionBar: (params: { childSessionId: string; @@ -81,7 +91,11 @@ export interface ReviewActionBarState { toggleGroupRemediation: (groupId: RemediationGroupId) => void; setActiveAction: (action: 'fix' | 'fix-review' | 'resume' | null) => void; setCustomInstructions: (value: string) => void; + setSelectedRemediationIds: (ids: Set) => void; dismiss: () => void; + minimize: () => void; + restore: () => void; + skipRemainingFixes: () => void; reset: () => void; } @@ -96,18 +110,34 @@ const initialState = { remediationItems: [] as ReviewRemediationItem[], selectedRemediationIds: new Set(), dismissed: false, + minimized: false, activeAction: null as 'fix' | 'fix-review' | 'resume' | null, customInstructions: '', errorMessage: null as string | null, interruption: null as DeepReviewInterruption | null, + completedRemediationIds: new Set(), + fixingRemediationIds: new Set(), + remainingFixIds: [] as string[], }; export const useReviewActionBarStore = create((set, get) => ({ ...initialState, - showActionBar: ({ childSessionId, parentSessionId, reviewData, reviewMode, phase }) => { + showActionBar: ({ childSessionId, parentSessionId, reviewData, reviewMode, phase, completedRemediationIds }) => { const items = buildReviewRemediationItems(reviewData); const defaultIds = new Set(getDefaultSelectedRemediationIds(items)); + + // If completedRemediationIds is provided, filter out items that no longer exist + const existingIds = new Set(items.map((i) => i.id)); + const preservedCompleted = completedRemediationIds + ? new Set([...completedRemediationIds].filter((id) => existingIds.has(id))) + : new Set(); + + // Remove completed items from default selection + for (const id of preservedCompleted) { + defaultIds.delete(id); + } + set({ childSessionId, parentSessionId, @@ -117,10 +147,14 @@ export const useReviewActionBarStore = create((set, get) = selectedRemediationIds: defaultIds, phase: phase ?? 'review_completed', dismissed: false, + minimized: false, activeAction: null, customInstructions: '', errorMessage: null, interruption: null, + completedRemediationIds: preservedCompleted, + fixingRemediationIds: new Set(), + remainingFixIds: [], }); }, @@ -134,15 +168,35 @@ export const useReviewActionBarStore = create((set, get) = selectedRemediationIds: new Set(), phase: phase ?? interruption.phase, dismissed: false, + minimized: false, activeAction: null, customInstructions: '', errorMessage: null, interruption, + completedRemediationIds: new Set(), + fixingRemediationIds: new Set(), + remainingFixIds: [], }); }, updatePhase: (phase, errorMessage) => { - set({ phase, errorMessage: errorMessage ?? null }); + const prevPhase = get().phase; + if (prevPhase === 'fix_running' && phase === 'fix_completed') { + const { fixingRemediationIds, completedRemediationIds } = get(); + const nextCompleted = new Set(completedRemediationIds); + for (const id of fixingRemediationIds) { + nextCompleted.add(id); + } + set({ + phase, + errorMessage: errorMessage ?? null, + completedRemediationIds: nextCompleted, + fixingRemediationIds: new Set(), + remainingFixIds: [], + }); + } else { + set({ phase, errorMessage: errorMessage ?? null }); + } }, toggleRemediation: (id) => { @@ -187,10 +241,53 @@ export const useReviewActionBarStore = create((set, get) = set({ selectedRemediationIds: next }); }, - setActiveAction: (action) => set({ activeAction: action }), + setActiveAction: (action) => { + if (action === 'fix' || action === 'fix-review') { + set({ + activeAction: action, + fixingRemediationIds: new Set(get().selectedRemediationIds), + }); + } else { + set({ activeAction: action }); + } + }, setCustomInstructions: (value) => set({ customInstructions: value }), + setSelectedRemediationIds: (ids) => set({ selectedRemediationIds: ids }), dismiss: () => set({ dismissed: true }), + minimize: () => set({ minimized: true }), + restore: () => set({ minimized: false }), + skipRemainingFixes: () => set({ + phase: 'review_completed', + remainingFixIds: [], + activeAction: null, + }), reset: () => set({ ...initialState, selectedRemediationIds: new Set() }), })); +// Subscribe to state changes and persist when relevant fields change +let persistTimer: ReturnType | null = null; +const PERSIST_DEBOUNCE_MS = 1000; + +useReviewActionBarStore.subscribe((state, prevState) => { + if (!state.childSessionId) return; + + const shouldPersist = + state.phase !== prevState.phase || + state.minimized !== prevState.minimized || + state.completedRemediationIds !== prevState.completedRemediationIds || + state.customInstructions !== prevState.customInstructions; + + if (!shouldPersist) return; + + if (persistTimer) clearTimeout(persistTimer); + + persistTimer = setTimeout(() => { + import('../services/ReviewActionBarPersistenceService').then(({ persistReviewActionState }) => { + persistReviewActionState(state).catch(() => { + // Silently ignore persistence errors + }); + }); + }, PERSIST_DEBOUNCE_MS); +}); + export const useDeepReviewActionBarStore = useReviewActionBarStore; diff --git a/src/web-ui/src/flow_chat/tool-cards/CompactToolCard.scss b/src/web-ui/src/flow_chat/tool-cards/CompactToolCard.scss index f29ea0aa0..44ffde414 100644 --- a/src/web-ui/src/flow_chat/tool-cards/CompactToolCard.scss +++ b/src/web-ui/src/flow_chat/tool-cards/CompactToolCard.scss @@ -62,7 +62,7 @@ .compact-tool-card { background: transparent !important; border: none !important; - padding: 0 8px 0 0 !important; + padding: 0 8px 0 1px !important; margin: 0 !important; box-shadow: none !important; display: flex; diff --git a/src/web-ui/src/flow_chat/types/flow-chat.ts b/src/web-ui/src/flow_chat/types/flow-chat.ts index 07647404f..e4fdb201c 100644 --- a/src/web-ui/src/flow_chat/types/flow-chat.ts +++ b/src/web-ui/src/flow_chat/types/flow-chat.ts @@ -250,9 +250,9 @@ export interface Session { /** * Set when a session finishes (completed / error / cancelled) while not the active session. * Cleared after the user switches to it and the content renders. - * 'completed' → green dot, 'error' → red dot. + * 'completed' → green dot, 'error' → red dot, 'interrupted' → red dot (partial stream recovery). */ - hasUnreadCompletion?: 'completed' | 'error'; + hasUnreadCompletion?: 'completed' | 'error' | 'interrupted'; /** * Set when a session requires user attention while not the active session. diff --git a/src/web-ui/src/flow_chat/utils/codeReviewRemediation.test.ts b/src/web-ui/src/flow_chat/utils/codeReviewRemediation.test.ts index 7886c00a6..cd4396b47 100644 --- a/src/web-ui/src/flow_chat/utils/codeReviewRemediation.test.ts +++ b/src/web-ui/src/flow_chat/utils/codeReviewRemediation.test.ts @@ -1,279 +1,198 @@ import { describe, expect, it } from 'vitest'; import { buildReviewRemediationItems, - buildSelectedRemediationPrompt, buildSelectedReviewRemediationPrompt, getDefaultSelectedRemediationIds, } from './codeReviewRemediation'; -import type { CodeReviewRemediationData } from './codeReviewRemediation'; - -function createReviewData( - overrides: Partial = {}, -): CodeReviewRemediationData { - return { - summary: {}, - issues: [], - remediation_plan: [], - ...overrides, - }; -} describe('buildReviewRemediationItems', () => { - it('returns empty array for empty plan', () => { - const items = buildReviewRemediationItems(createReviewData()); - expect(items).toEqual([]); - }); - - it('skips empty plan items', () => { - const items = buildReviewRemediationItems( - createReviewData({ remediation_plan: ['', ' ', 'valid plan'] }), - ); - expect(items).toHaveLength(1); - expect(items[0].plan).toBe('valid plan'); - }); - - it('selects critical severity by default', () => { - const items = buildReviewRemediationItems( - createReviewData({ - remediation_plan: ['Fix critical bug'], - issues: [{ severity: 'critical' }], - }), - ); - expect(items[0].defaultSelected).toBe(true); - }); - - it('selects high severity by default', () => { - const items = buildReviewRemediationItems( - createReviewData({ - remediation_plan: ['Fix high bug'], - issues: [{ severity: 'high' }], - }), - ); - expect(items[0].defaultSelected).toBe(true); - }); - - it('selects medium severity by default', () => { - const items = buildReviewRemediationItems( - createReviewData({ - remediation_plan: ['Fix medium bug'], - issues: [{ severity: 'medium' }], - }), - ); - expect(items[0].defaultSelected).toBe(true); - }); - - it('does not select low severity by default', () => { - const items = buildReviewRemediationItems( - createReviewData({ - remediation_plan: ['Fix low bug'], - issues: [{ severity: 'low' }], - }), - ); - expect(items[0].defaultSelected).toBe(false); - }); - - it('does not select info severity by default', () => { - const items = buildReviewRemediationItems( - createReviewData({ - remediation_plan: ['Fix info bug'], - issues: [{ severity: 'info' }], - }), - ); - expect(items[0].defaultSelected).toBe(false); - }); - - it('selects by confirmed certainty + suggestion even for low severity', () => { - const items = buildReviewRemediationItems( - createReviewData({ - remediation_plan: ['Fix with suggestion'], - issues: [{ severity: 'low', certainty: 'confirmed', suggestion: 'do this' }], - }), - ); - expect(items[0].defaultSelected).toBe(true); - }); - - it('does not select by suggestion alone without confirmed certainty', () => { - const items = buildReviewRemediationItems( - createReviewData({ - remediation_plan: ['Fix with suggestion'], - issues: [{ severity: 'low', certainty: 'likely', suggestion: 'do this' }], - }), - ); - expect(items[0].defaultSelected).toBe(false); - }); - - it('selects plan-only items when recommended_action is request_changes', () => { - const items = buildReviewRemediationItems( - createReviewData({ - remediation_plan: ['Plan without issue'], - summary: { recommended_action: 'request_changes' }, - }), - ); - expect(items[0].defaultSelected).toBe(true); - }); - - it('selects plan-only items when recommended_action is block', () => { - const items = buildReviewRemediationItems( - createReviewData({ - remediation_plan: ['Plan without issue'], - summary: { recommended_action: 'block' }, - }), - ); - expect(items[0].defaultSelected).toBe(true); - }); + it('builds items from remediation_plan', () => { + const result = buildReviewRemediationItems({ + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + }); - it('does not select plan-only items when recommended_action is approve', () => { - const items = buildReviewRemediationItems( - createReviewData({ - remediation_plan: ['Plan without issue'], - summary: { recommended_action: 'approve' }, - }), - ); - expect(items[0].defaultSelected).toBe(false); - }); + expect(result).toHaveLength(2); + expect(result[0].id).toBe('remediation-0'); + expect(result[0].plan).toBe('Fix issue 1'); + expect(result[1].id).toBe('remediation-1'); + expect(result[1].plan).toBe('Fix issue 2'); + }); + + it('builds items from structured remediation groups', () => { + const result = buildReviewRemediationItems({ + summary: { recommended_action: 'request_changes' }, + report_sections: { + remediation_groups: { + must_fix: ['Critical fix 1'], + should_improve: ['Improvement 1'], + needs_decision: ['Decision needed'], + verification: ['Verify fix'], + }, + }, + }); - it('links issues to plans by index', () => { - const items = buildReviewRemediationItems( - createReviewData({ - remediation_plan: ['Plan 0', 'Plan 1'], - issues: [{ title: 'Issue 0' }, { title: 'Issue 1' }], - }), - ); - expect(items).toHaveLength(2); - expect(items[0].issue?.title).toBe('Issue 0'); - expect(items[1].issue?.title).toBe('Issue 1'); + expect(result.length).toBeGreaterThan(0); + expect(result.some((item) => item.groupId === 'must_fix')).toBe(true); + expect(result.some((item) => item.groupId === 'should_improve')).toBe(true); }); - it('builds structured remediation items from report sections', () => { - const items = buildReviewRemediationItems( - createReviewData({ - remediation_plan: ['Legacy fallback should not duplicate'], - report_sections: { - remediation_groups: { - must_fix: ['Fix the blocking bug'], - should_improve: ['Clean up the helper'], - needs_decision: ['Confirm the desired product behavior'], - verification: ['Run the focused regression test'], - }, + it('marks must_fix items as default selected', () => { + const result = buildReviewRemediationItems({ + summary: { recommended_action: 'request_changes' }, + report_sections: { + remediation_groups: { + must_fix: ['Critical fix 1'], + should_improve: ['Improvement 1'], }, - }), - ); + }, + }); - expect(items.map((item) => item.plan)).toEqual([ - 'Fix the blocking bug', - 'Clean up the helper', - 'Confirm the desired product behavior', - 'Run the focused regression test', - ]); - expect(items.find((item) => item.groupId === 'needs_decision')?.defaultSelected).toBe(false); - expect(items.find((item) => item.groupId === 'needs_decision')?.requiresDecision).toBe(true); - expect(items.find((item) => item.groupId === 'verification')?.defaultSelected).toBe(false); + const mustFixItems = result.filter((item) => item.groupId === 'must_fix'); + const shouldImproveItems = result.filter((item) => item.groupId === 'should_improve'); + + expect(mustFixItems.every((item) => item.defaultSelected)).toBe(true); + expect(shouldImproveItems.every((item) => !item.defaultSelected)).toBe(true); }); }); describe('getDefaultSelectedRemediationIds', () => { - it('returns ids of default selected items', () => { - const items = [ - { id: 'a', defaultSelected: true }, - { id: 'b', defaultSelected: false }, - { id: 'c', defaultSelected: true }, - ] as any; - expect(getDefaultSelectedRemediationIds(items)).toEqual(['a', 'c']); - }); + it('returns IDs of default selected items', () => { + const items = buildReviewRemediationItems({ + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + issues: [ + { severity: 'high', title: 'Issue 1' }, + { severity: 'low', title: 'Issue 2' }, + ], + }); - it('returns empty array when no items selected', () => { - const items = [ - { id: 'a', defaultSelected: false }, - { id: 'b', defaultSelected: false }, - ] as any; - expect(getDefaultSelectedRemediationIds(items)).toEqual([]); + const selectedIds = getDefaultSelectedRemediationIds(items); + expect(selectedIds.length).toBeGreaterThan(0); + expect(selectedIds).toContain('remediation-0'); }); }); -describe('buildSelectedRemediationPrompt', () => { - it('returns empty string when no ids selected', () => { - const prompt = buildSelectedRemediationPrompt({ - reviewData: createReviewData({ remediation_plan: ['Plan'] }), +describe('buildSelectedReviewRemediationPrompt', () => { + it('returns empty string when no items selected', () => { + const prompt = buildSelectedReviewRemediationPrompt({ + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1'], + }, selectedIds: new Set(), rerunReview: false, + reviewMode: 'deep', }); - expect(prompt).toBe(''); - }); - it('returns empty string when selected ids do not match any items', () => { - const prompt = buildSelectedRemediationPrompt({ - reviewData: createReviewData({ remediation_plan: ['Plan'] }), - selectedIds: new Set(['non-existent']), - rerunReview: false, - }); expect(prompt).toBe(''); }); - it('includes selected plan items in prompt', () => { - const prompt = buildSelectedRemediationPrompt({ - reviewData: createReviewData({ - remediation_plan: ['Plan A', 'Plan B'], - }), + it('builds prompt with selected items for deep review', () => { + const prompt = buildSelectedReviewRemediationPrompt({ + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1'], + }, selectedIds: new Set(['remediation-0']), rerunReview: false, + reviewMode: 'deep', }); - expect(prompt).toContain('Plan A'); - expect(prompt).not.toContain('Plan B'); + + expect(prompt).toContain('Deep Review findings only'); + expect(prompt).toContain('Fix issue 1'); expect(prompt).toContain('Selected Remediation Plan'); }); - it('includes rerun review instruction when rerunReview is true', () => { - const prompt = buildSelectedRemediationPrompt({ - reviewData: createReviewData({ remediation_plan: ['Plan'] }), + it('builds prompt with rerun instruction when rerunReview is true', () => { + const prompt = buildSelectedReviewRemediationPrompt({ + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1'], + }, selectedIds: new Set(['remediation-0']), rerunReview: true, + reviewMode: 'deep', }); - expect(prompt).toContain('follow-up review'); + + expect(prompt).toContain('follow-up deep review'); + expect(prompt).toContain('dispatching the review team'); }); - it('includes summary instruction when rerunReview is false', () => { - const prompt = buildSelectedRemediationPrompt({ - reviewData: createReviewData({ remediation_plan: ['Plan'] }), + it('builds prompt with standard review mode', () => { + const prompt = buildSelectedReviewRemediationPrompt({ + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1'], + }, selectedIds: new Set(['remediation-0']), rerunReview: false, + reviewMode: 'standard', + }); + + expect(prompt).toContain('Code Review findings only'); + }); + + it('appends continuation context when completedItems provided', () => { + const prompt = buildSelectedReviewRemediationPrompt({ + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2', 'Fix issue 3'], + }, + selectedIds: new Set(['remediation-1', 'remediation-2']), + rerunReview: false, + reviewMode: 'deep', + completedItems: ['remediation-0'], }); - expect(prompt).toContain('summarize what changed'); + + expect(prompt).toContain('Continuation Context'); + expect(prompt).toContain('Already completed items (DO NOT re-fix)'); + expect(prompt).toContain('Fix issue 1'); + expect(prompt).toContain('Please focus only on the remaining items'); }); - it('includes issue details when issue is linked', () => { - const prompt = buildSelectedRemediationPrompt({ - reviewData: createReviewData({ - remediation_plan: ['Fix bug'], - issues: [{ - severity: 'critical', - certainty: 'confirmed', - title: 'Critical Bug', - file: 'src/main.ts', - line: 42, - description: 'Memory leak', - suggestion: 'Use WeakRef', - }], - }), + it('does not append continuation context when completedItems is empty', () => { + const prompt = buildSelectedReviewRemediationPrompt({ + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1'], + }, selectedIds: new Set(['remediation-0']), rerunReview: false, + reviewMode: 'deep', + completedItems: [], }); - expect(prompt).toContain('Critical Bug'); - expect(prompt).toContain('src/main.ts:42'); - expect(prompt).toContain('Memory leak'); - expect(prompt).toContain('Use WeakRef'); + + expect(prompt).not.toContain('Continuation Context'); }); - it('uses Code Review wording for standard review remediation prompts', () => { + it('does not append continuation context when completedItems not provided', () => { const prompt = buildSelectedReviewRemediationPrompt({ - reviewData: createReviewData({ remediation_plan: ['Fix standard issue'] }), + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1'], + }, selectedIds: new Set(['remediation-0']), - rerunReview: true, - reviewMode: 'standard', + rerunReview: false, + reviewMode: 'deep', + }); + + expect(prompt).not.toContain('Continuation Context'); + }); + + it('only includes completed items that exist in review data', () => { + const prompt = buildSelectedReviewRemediationPrompt({ + reviewData: { + summary: { recommended_action: 'request_changes' }, + remediation_plan: ['Fix issue 1', 'Fix issue 2'], + }, + selectedIds: new Set(['remediation-1']), + rerunReview: false, + reviewMode: 'deep', + completedItems: ['remediation-0', 'non-existent-id'], }); - expect(prompt).toContain('selected Code Review findings only'); - expect(prompt).toContain('follow-up standard code review'); - expect(prompt).not.toContain('Deep Review findings only'); - expect(prompt).not.toContain('parallel, followed by ReviewJudge'); + expect(prompt).toContain('Fix issue 1'); + expect(prompt).not.toContain('non-existent-id'); }); }); diff --git a/src/web-ui/src/flow_chat/utils/codeReviewRemediation.ts b/src/web-ui/src/flow_chat/utils/codeReviewRemediation.ts index 58bd03452..6c254a1a9 100644 --- a/src/web-ui/src/flow_chat/utils/codeReviewRemediation.ts +++ b/src/web-ui/src/flow_chat/utils/codeReviewRemediation.ts @@ -189,6 +189,7 @@ export function buildSelectedReviewRemediationPrompt(params: { selectedIds: Set; rerunReview: boolean; reviewMode: ReviewMode; + completedItems?: string[]; }): string { if (params.selectedIds.size === 0) { return ''; @@ -213,16 +214,40 @@ export function buildSelectedReviewRemediationPrompt(params: { ? 'After implementing fixes, run the most relevant verification. Then launch a full follow-up deep review of the fix diff by dispatching the review team (Business Logic, Performance, Security reviewers in parallel, followed by ReviewJudge). Submit the follow-up review result via submit_code_review.' : 'After implementing fixes, run the most relevant verification. Then submit a follow-up standard code review of the fix diff via submit_code_review.'; - return [ + const lines: string[] = [ `The user approved remediation for selected ${reviewLabel} findings only.`, '', 'Please implement only the selected remediation items below. Do not broaden scope beyond these selected findings unless required for correctness.', params.rerunReview ? rerunInstruction : 'After implementing fixes, summarize what changed and what verification was run.', - '', - '## Selected Remediation Plan', - planBlock, - '', - '## Selected Review Findings', - issuesBlock, - ].join('\n'); + ]; + + // Append continuation context if there are completed items + if (params.completedItems && params.completedItems.length > 0) { + const allItems = buildReviewRemediationItems(params.reviewData); + const completedItemPlans = allItems + .filter((item) => params.completedItems!.includes(item.id)) + .map((item) => item.plan); + + if (completedItemPlans.length > 0) { + lines.push(''); + lines.push('---'); + lines.push('## Continuation Context'); + lines.push(''); + lines.push('This is a continuation of a previous fix attempt that was interrupted.'); + lines.push(''); + lines.push('### Already completed items (DO NOT re-fix):'); + completedItemPlans.forEach((plan, i) => lines.push(`${i + 1}. ${plan}`)); + lines.push(''); + lines.push('Please focus only on the remaining items. Do not modify code related to already completed items unless necessary for correctness.'); + } + } + + lines.push(''); + lines.push('## Selected Remediation Plan'); + lines.push(planBlock); + lines.push(''); + lines.push('## Selected Review Findings'); + lines.push(issuesBlock); + + return lines.join('\n'); } diff --git a/src/web-ui/src/infrastructure/debug/useDebugInspector.ts b/src/web-ui/src/infrastructure/debug/useDebugInspector.ts index e19561b15..bd71bfc99 100644 --- a/src/web-ui/src/infrastructure/debug/useDebugInspector.ts +++ b/src/web-ui/src/infrastructure/debug/useDebugInspector.ts @@ -12,6 +12,7 @@ import { useCallback } from 'react'; import { useShortcut } from '@/infrastructure/hooks/useShortcut'; import { createLogger } from '@/shared/utils/logger'; +import { isTauriRuntime } from '@/infrastructure/runtime'; import { createMainWindowInspectorScript, CANCEL_MAIN_WINDOW_INSPECTOR_SCRIPT, @@ -25,7 +26,7 @@ function isDevToolsAvailable(): boolean { // In a standard web build (non-Tauri) the inspector is useless because we // already have browser DevTools. Only enable in the desktop webview. if (typeof window === 'undefined') return false; - if (!('__TAURI__' in window)) return false; + if (!isTauriRuntime()) return false; // The backend only exposes debug commands when compiled with devtools feature // or in debug builds. We optimistically enable the shortcut here; the invoke @@ -79,15 +80,19 @@ async function openNativeDevTools(): Promise { * Shortcuts: * Cmd/Ctrl + Shift + I → Toggle element inspector * Cmd/Ctrl + Shift + J → Open native DevTools + * + * Note: shortcuts are always registered (so they work even if Tauri runtime + * detection races with component mount), but the callback no-ops when not + * in a Tauri desktop environment. */ export function useDebugInspector(): void { - const available = isDevToolsAvailable(); - const handleToggleInspector = useCallback(() => { + if (!isDevToolsAvailable()) return; void toggleInspector(); }, []); const handleOpenDevTools = useCallback(() => { + if (!isDevToolsAvailable()) return; void openNativeDevTools(); }, []); @@ -98,7 +103,6 @@ export function useDebugInspector(): void { { key: 'i', ctrl: true, shift: true, scope: 'app', allowInInput: true }, handleToggleInspector, { - enabled: available, priority: 100, description: 'Toggle element inspector', } @@ -110,7 +114,6 @@ export function useDebugInspector(): void { { key: 'j', ctrl: true, shift: true, scope: 'app', allowInInput: true }, handleOpenDevTools, { - enabled: available, priority: 100, description: 'Open native DevTools', } diff --git a/src/web-ui/src/locales/en-US/common.json b/src/web-ui/src/locales/en-US/common.json index a8df072bd..28f5f02d5 100644 --- a/src/web-ui/src/locales/en-US/common.json +++ b/src/web-ui/src/locales/en-US/common.json @@ -183,6 +183,7 @@ "deepReviewRunning": "Reviewing", "unreadCompleted": "Completed — unread", "unreadError": "Failed — unread", + "unreadInterrupted": "Interrupted — retry needed", "needsUserInput": "Needs your input", "needsToolConfirm": "Needs confirmation", "badgeNeedsInput": "Waiting", diff --git a/src/web-ui/src/locales/zh-CN/common.json b/src/web-ui/src/locales/zh-CN/common.json index 96ffb2341..7775fbb73 100644 --- a/src/web-ui/src/locales/zh-CN/common.json +++ b/src/web-ui/src/locales/zh-CN/common.json @@ -183,6 +183,7 @@ "deepReviewRunning": "审核中", "unreadCompleted": "已完成 — 未读", "unreadError": "执行失败 — 未读", + "unreadInterrupted": "输出中断 — 需重试", "needsUserInput": "等待你的输入", "needsToolConfirm": "等待确认", "badgeNeedsInput": "等待中", diff --git a/src/web-ui/src/locales/zh-CN/flow-chat.json b/src/web-ui/src/locales/zh-CN/flow-chat.json index 63d2e0b37..6cca84c53 100644 --- a/src/web-ui/src/locales/zh-CN/flow-chat.json +++ b/src/web-ui/src/locales/zh-CN/flow-chat.json @@ -783,6 +783,10 @@ "errorLabel": "错误信息", "resultLabel": "执行结果", "noData": "无法加载任务数据", + "stopSubagentFailed": "无法停止该子代理。", + "stoppingSubagent": "正在停止子代理……", + "stopSubagent": "停止子代理", + "stopSubagentHint": "仅取消此审查员/子代理。父级审查仍可继续并生成摘要。", "executionContent": "执行内容", "duration": "耗时", "waitingForModelResponse": "等待模型响应...", diff --git a/src/web-ui/src/locales/zh-TW/common.json b/src/web-ui/src/locales/zh-TW/common.json index aeba88c3c..4b51da159 100644 --- a/src/web-ui/src/locales/zh-TW/common.json +++ b/src/web-ui/src/locales/zh-TW/common.json @@ -183,6 +183,7 @@ "deepReviewRunning": "審核中", "unreadCompleted": "已完成 — 未讀", "unreadError": "執行失敗 — 未讀", + "unreadInterrupted": "輸出中斷 — 需重試", "needsUserInput": "等待你的輸入", "needsToolConfirm": "等待確認", "badgeNeedsInput": "等待中", diff --git a/src/web-ui/src/locales/zh-TW/flow-chat.json b/src/web-ui/src/locales/zh-TW/flow-chat.json index 3bb870c60..a54530c46 100644 --- a/src/web-ui/src/locales/zh-TW/flow-chat.json +++ b/src/web-ui/src/locales/zh-TW/flow-chat.json @@ -738,6 +738,10 @@ "errorLabel": "錯誤信息", "resultLabel": "執行結果", "noData": "無法加載任務數據", + "stopSubagentFailed": "無法停止該子代理。", + "stoppingSubagent": "正在停止子代理……", + "stopSubagent": "停止子代理", + "stopSubagentHint": "僅取消此審查員/子代理。父級審查仍可繼續並生成摘要。", "executionContent": "執行內容", "duration": "耗時", "waitingForModelResponse": "等待模型回應...", diff --git a/src/web-ui/src/shared/types/session-history.ts b/src/web-ui/src/shared/types/session-history.ts index d97c9d4ad..e5e7a5ddf 100644 --- a/src/web-ui/src/shared/types/session-history.ts +++ b/src/web-ui/src/shared/types/session-history.ts @@ -41,8 +41,11 @@ export interface SessionMetadata { remoteSshHost?: string; /** Backend unified workspace identity field: localhost for local, SSH host for remote. */ workspaceHostname?: string; - /** Unread completion status for the session. 'completed' → green dot, 'error' → red dot. */ - unreadCompletion?: 'completed' | 'error'; + /** + * Unread completion status for the session. + * 'completed' → green dot, 'error' → red dot, 'interrupted' → red dot (partial stream recovery). + */ + unreadCompletion?: 'completed' | 'error' | 'interrupted'; /** * High-priority attention status for the session. * 'ask_user' → pending AskUserQuestion waiting for answer. @@ -50,6 +53,20 @@ export interface SessionMetadata { * Takes precedence over unreadCompletion in the UI. */ needsUserAttention?: 'ask_user' | 'tool_confirm'; + /** + * Persisted review action bar state for code review / deep review sessions. + * Allows restoring the review action bar across app restarts. + */ + reviewActionState?: ReviewActionPersistedState; +} + +export interface ReviewActionPersistedState { + version: number; + phase: string; + completedRemediationIds: string[]; + minimized: boolean; + customInstructions: string; + persistedAt: number; } export type SessionStatus = 'active' | 'archived' | 'completed'; diff --git a/tests/e2e/specs/l1-review-action-bar.spec.ts b/tests/e2e/specs/l1-review-action-bar.spec.ts new file mode 100644 index 000000000..46d904be6 --- /dev/null +++ b/tests/e2e/specs/l1-review-action-bar.spec.ts @@ -0,0 +1,181 @@ +/** + * L1 Review Action Bar spec: validates review action bar minimize/restore + * and continuation behavior for code review / deep review sessions. + */ + +import { browser, expect, $ } from '@wdio/globals'; +import { Header } from '../page-objects/components/Header'; +import { StartupPage } from '../page-objects/StartupPage'; +import { ensureWorkspaceOpen } from '../helpers/workspace-utils'; + +describe('L1 Review Action Bar', () => { + let header: Header; + let startupPage: StartupPage; + let hasWorkspace = false; + + before(async () => { + header = new Header(); + startupPage = new StartupPage(); + await browser.pause(3000); + await header.waitForLoad(); + hasWorkspace = await ensureWorkspaceOpen(startupPage); + if (!hasWorkspace) { + console.log('[L1] No workspace available - review action bar tests will be skipped'); + } + }); + + describe('Minimize and restore', () => { + it('should minimize review action bar when close button is clicked', async function () { + if (!hasWorkspace) { + this.skip(); + return; + } + + // Wait for any review action bar to appear + const actionBar = await $('.deep-review-action-bar'); + if (!(await actionBar.isExisting())) { + console.log('[L1] No review action bar visible - skipping minimize test'); + this.skip(); + return; + } + + const closeButton = await actionBar.$('.deep-review-action-bar__close'); + expect(await closeButton.isExisting()).toBe(true); + + await closeButton.click(); + await browser.pause(500); + + // Action bar should be hidden (minimized) + expect(await actionBar.isDisplayed()).toBe(false); + + // Minimized indicator should appear + const indicator = await $('.btw-session-panel__minimized-indicator'); + expect(await indicator.isExisting()).toBe(true); + expect(await indicator.isDisplayed()).toBe(true); + }); + + it('should restore review action bar when minimized indicator is clicked', async function () { + if (!hasWorkspace) { + this.skip(); + return; + } + + const indicator = await $('.btw-session-panel__minimized-indicator'); + if (!(await indicator.isExisting())) { + console.log('[L1] No minimized indicator visible - skipping restore test'); + this.skip(); + return; + } + + const restoreButton = await indicator.$('button'); + await restoreButton.click(); + await browser.pause(500); + + // Action bar should be visible again + const actionBar = await $('.deep-review-action-bar'); + expect(await actionBar.isDisplayed()).toBe(true); + + // Indicator should be hidden + expect(await indicator.isDisplayed()).toBe(false); + }); + + it('should show remaining count in minimized indicator', async function () { + if (!hasWorkspace) { + this.skip(); + return; + } + + // This test requires an active review action bar + const actionBar = await $('.deep-review-action-bar'); + if (!(await actionBar.isExisting())) { + console.log('[L1] No review action bar visible - skipping count test'); + this.skip(); + return; + } + + const closeButton = await actionBar.$('.deep-review-action-bar__close'); + await closeButton.click(); + await browser.pause(500); + + const indicator = await $('.btw-session-panel__minimized-indicator'); + const countEl = await indicator.$('.btw-session-panel__minimized-count'); + expect(await countEl.isExisting()).toBe(true); + + const countText = await countEl.getText(); + // Should be in format "X/Y" + expect(countText).toMatch(/^\d+\/\d+$/); + + // Restore for other tests + const restoreButton = await indicator.$('button'); + await restoreButton.click(); + await browser.pause(500); + }); + }); + + describe('Completed items state', () => { + it('should mark completed items with strikethrough and disabled checkbox', async function () { + if (!hasWorkspace) { + this.skip(); + return; + } + + const actionBar = await $('.deep-review-action-bar'); + if (!(await actionBar.isExisting())) { + console.log('[L1] No review action bar visible - skipping completed items test'); + this.skip(); + return; + } + + // Look for completed items + const completedItems = await actionBar.$$('.deep-review-action-bar__remediation-item--completed'); + if (completedItems.length === 0) { + console.log('[L1] No completed items visible - skipping'); + this.skip(); + return; + } + + // Verify completed item styling + const firstCompleted = completedItems[0]; + const checkbox = await firstCompleted.$('input[type="checkbox"]'); + expect(await checkbox.getAttribute('disabled')).toBe('true'); + + const text = await firstCompleted.$('.deep-review-action-bar__remediation-text'); + const textDecoration = await text.getCSSProperty('text-decoration'); + expect(textDecoration.value).toContain('line-through'); + }); + }); + + describe('Fix interruption continuation', () => { + it('should show continue fix button when fix was interrupted', async function () { + if (!hasWorkspace) { + this.skip(); + return; + } + + // This test requires the action bar to be in fix_interrupted state + // In a real scenario, this would be triggered by an interrupted fix + const actionBar = await $('.deep-review-action-bar'); + if (!(await actionBar.isExisting())) { + console.log('[L1] No review action bar visible - skipping interruption test'); + this.skip(); + return; + } + + // Check if the action bar has interruption notice + const interruptionNotice = await actionBar.$('.deep-review-action-bar__interruption-notice'); + if (!(await interruptionNotice.isExisting())) { + console.log('[L1] No interruption notice visible - skipping'); + this.skip(); + return; + } + + // Verify continue fix button exists + const continueButton = await actionBar.$('button*=Continue fixing'); + expect(await continueButton.isExisting()).toBe(true); + + // Verify skip button exists + const skipButton = await actionBar.$('button*=Skip remaining'); + expect(await skipButton.isExisting()).toBe(true); + }); + }); +});