Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ Interpret the user's request carefully:

Do not silently widen the scope unless the target is impossible to inspect otherwise. If you must widen it, mention that limitation in the final confidence note.

For targets that are only locale/i18n files, keep reviewer work proportional to that scope: check key coverage, placeholders, interpolation, formatting, and user-facing wording. Do not ask Business Logic or Architecture reviewers to chase broad call graphs or import chains unless the locale diff itself references a concrete contract change. Prefer `GetFileDiff` or a full relevant file read over repeated tiny `Read` windows.

## Tool Usage Rules

You MUST use:
Expand Down Expand Up @@ -110,10 +112,11 @@ Launch these mandatory Task tool calls in one message:
- `ReviewBusinessLogic`
- `ReviewPerformance`
- `ReviewSecurity`
- `ReviewArchitecture`

If the execution policy indicates file splitting is needed (see "File splitting for large review targets" above), launch multiple same-role instances per role in the **same message**. For example, if 3 Security instances are needed, include all three `ReviewSecurity` Task calls in the same message alongside the other reviewers.

If extra reviewers are configured, launch them in the **same message** as additional Task calls after the three mandatory reviewers.
If extra reviewers are configured, launch them in the **same message** as additional Task calls after the four mandatory reviewers.

If the execution policy says `reviewer_timeout_seconds > 0`, pass `timeout_seconds` with that value to every reviewer Task call in this batch.

Expand Down
24 changes: 23 additions & 1 deletion src/crates/core/src/agentic/agents/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ use crate::agentic::agents::custom_subagents::{
};
use crate::agentic::deep_review_policy::{
REVIEWER_ARCHITECTURE_AGENT_TYPE, REVIEWER_BUSINESS_LOGIC_AGENT_TYPE,
REVIEWER_PERFORMANCE_AGENT_TYPE, REVIEWER_SECURITY_AGENT_TYPE, REVIEW_JUDGE_AGENT_TYPE,
REVIEWER_FRONTEND_AGENT_TYPE, REVIEWER_PERFORMANCE_AGENT_TYPE, REVIEWER_SECURITY_AGENT_TYPE,
REVIEW_JUDGE_AGENT_TYPE,
};
use crate::agentic::tools::{get_all_registered_tool_names, get_readonly_registered_tool_names};
use crate::service::config::global::GlobalConfigManager;
Expand Down Expand Up @@ -148,6 +149,7 @@ fn is_review_agent_entry(entry: &AgentEntry) -> bool {
| REVIEWER_PERFORMANCE_AGENT_TYPE
| REVIEWER_SECURITY_AGENT_TYPE
| REVIEWER_ARCHITECTURE_AGENT_TYPE
| REVIEWER_FRONTEND_AGENT_TYPE
| REVIEW_JUDGE_AGENT_TYPE
)
}
Expand Down Expand Up @@ -1359,6 +1361,26 @@ mod tests {
}
}

#[test]
fn built_in_deep_review_reviewers_are_marked_as_review_agents() {
let registry = AgentRegistry::new();

for agent_type in [
"ReviewBusinessLogic",
"ReviewPerformance",
"ReviewSecurity",
"ReviewArchitecture",
"ReviewFrontend",
"ReviewJudge",
] {
assert_eq!(
registry.get_subagent_is_review(agent_type),
Some(true),
"{agent_type} must pass DeepReview Task policy validation"
);
}
}

#[test]
fn merge_dynamic_mcp_tools_appends_registered_mcp_tools_once() {
let configured_tools = vec!["Read".to_string(), "Bash".to_string()];
Expand Down
13 changes: 13 additions & 0 deletions src/crates/core/src/agentic/coordination/coordinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2465,6 +2465,19 @@ Update the persona files and delete BOOTSTRAP.md as soon as bootstrap is complet
dialog_turn_id: dialog_turn_id.clone(),
};

// Subagent sessions do not go through `start_dialog_turn_internal`, so
// they must mark their active turn here. The desktop stop action uses
// this state to find the running turn and signal the right cancel token.
self.session_manager
.update_session_state(
&session_id,
SessionState::Processing {
current_turn_id: dialog_turn_id.clone(),
phase: ProcessingPhase::Thinking,
},
)
.await?;

// Emit DialogTurnStarted with subagent_parent_info so the frontend can
// associate the subagent session ID with the parent tool (enabling the
// "ignore timeout" feature for deep-review subagents).
Expand Down
32 changes: 22 additions & 10 deletions src/web-ui/src/flow_chat/components/btw/BtwSessionPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -381,18 +381,18 @@ export const BtwSessionPanel: React.FC<BtwSessionPanelProps> = ({
const turnStatus = lastTurn?.status;
const isComplete = turnStatus === 'completed';
const isError = turnStatus === 'error' || Boolean(childSession.error);
const deepReviewInterruption = isDeepReview
? deriveDeepReviewInterruption(childSession)
: null;

const store = useReviewActionBarStore.getState();

if (isDeepReview && (!latestReviewData || latestReviewMode !== 'deep') && isError) {
const interruption = deriveDeepReviewInterruption(childSession);
if (interruption) {
store.showInterruptedActionBar({
childSessionId,
parentSessionId: parentSessionId ?? null,
interruption,
});
}
if (isDeepReview && (!latestReviewData || latestReviewMode !== 'deep') && deepReviewInterruption) {
store.showInterruptedActionBar({
childSessionId,
parentSessionId: parentSessionId ?? null,
interruption: deepReviewInterruption,
});
return;
}

Expand All @@ -405,7 +405,9 @@ export const BtwSessionPanel: React.FC<BtwSessionPanelProps> = ({
// 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' && store.phase !== 'fix_interrupted') {
if (isError && store.phase === 'resume_running') {
store.updatePhase('resume_failed', childSession.error ?? undefined);
} else 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,
Expand All @@ -425,6 +427,16 @@ export const BtwSessionPanel: React.FC<BtwSessionPanelProps> = ({
// show completion state in the action bar instead of dismissing it.
store.updatePhase('fix_completed');
}
} else if (isComplete && store.phase === 'resume_running') {
store.showActionBar({
childSessionId,
parentSessionId: parentSessionId ?? null,
reviewData: latestReviewData,
reviewMode,
phase: 'review_completed',
completedRemediationIds: store.completedRemediationIds,
});
store.minimize();
}
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ const REQUIRED_ACTION_BAR_KEYS = [
'deepReviewActionBar.fixInterrupted',
'deepReviewActionBar.continueFix',
'deepReviewActionBar.skipRemaining',
'deepReviewActionBar.switchModel',
'reviewActionBar.noIssuesFound',
];

Expand Down
28 changes: 0 additions & 28 deletions src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -672,41 +672,13 @@
line-height: 1.5;
}

&__attribution-actions {
display: flex;
align-items: center;
gap: 8px;
flex-wrap: wrap;
}

/* Recovery plan */
&__recovery-plan {
display: flex;
flex-direction: column;
gap: 6px;
}

&__recovery-plan-toggle {
display: inline-flex;
align-items: center;
gap: 6px;
padding: 4px 8px;
border-radius: 6px;
background: transparent;
border: 1px solid transparent;
color: var(--color-text-muted);
font-size: 12px;
cursor: pointer;
transition: background 0.2s, border-color 0.2s, color 0.2s;
width: fit-content;

&:hover {
background: var(--element-bg-subtle);
border-color: var(--border-base);
color: var(--color-text-primary);
}
}

&__recovery-plan-detail {
display: flex;
flex-direction: column;
Expand Down
100 changes: 97 additions & 3 deletions src/web-ui/src/flow_chat/components/btw/DeepReviewActionBar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,24 @@ import { useReviewActionBarStore } from '../../store/deepReviewActionBarStore';
const sendMessageMock = vi.hoisted(() => vi.fn());
const eventBusEmitMock = vi.hoisted(() => vi.fn());
const confirmWarningMock = vi.hoisted(() => vi.fn());
const continueDeepReviewSessionMock = vi.hoisted(() => vi.fn());
const buildRecoveryPlanMock = vi.hoisted(() => vi.fn(() => ({
willPreserve: ['ReviewSecurity'],
willRerun: ['ReviewPerformance'],
willSkip: [],
summaryText: '1 completed reviewer will be preserved; 1 reviewer will be rerun',
})));

vi.mock('react-i18next', () => ({
initReactI18next: {
type: '3rdParty',
init: vi.fn(),
},
useTranslation: () => ({
t: (_key: string, options?: { defaultValue?: string }) => options?.defaultValue ?? _key,
t: (_key: string, options?: Record<string, unknown> & { defaultValue?: string }) => {
const template = options?.defaultValue ?? _key;
return template.replace(/{{(\w+)}}/g, (_match, token: string) => String(options?.[token] ?? _match));
},
}),
}));

Expand Down Expand Up @@ -91,12 +101,12 @@ vi.mock('../../utils/deepReviewExperience', () => ({
buildReviewerProgressSummary: () => null,
extractPartialReviewData: () => null,
buildErrorAttribution: () => null,
buildRecoveryPlan: () => null,
buildRecoveryPlan: buildRecoveryPlanMock,
evaluateDegradationOptions: () => [],
}));

vi.mock('../../services/DeepReviewContinuationService', () => ({
continueDeepReviewSession: vi.fn(),
continueDeepReviewSession: continueDeepReviewSessionMock,
}));

vi.mock('@/shared/ai-errors/aiErrorPresenter', () => ({
Expand Down Expand Up @@ -148,6 +158,7 @@ describeWithJsdom('DeepReviewActionBar', () => {
sendMessageMock.mockResolvedValue(undefined);
confirmWarningMock.mockResolvedValue(true);
eventBusEmitMock.mockReturnValue(false);
continueDeepReviewSessionMock.mockResolvedValue(undefined);
useReviewActionBarStore.getState().reset();
});

Expand Down Expand Up @@ -482,4 +493,87 @@ describeWithJsdom('DeepReviewActionBar', () => {
expect(state.remainingFixIds).toEqual([]);
expect(state.activeAction).toBeNull();
});

it('keeps Deep Review interruption actions in one row without a standalone retry or recovery toggle', async () => {
const { DeepReviewActionBar } = await import('./DeepReviewActionBar');

useReviewActionBarStore.getState().showInterruptedActionBar({
childSessionId: 'deep-review-session',
parentSessionId: 'parent-session',
interruption: {
phase: 'resume_failed',
childSessionId: 'deep-review-session',
parentSessionId: 'parent-session',
originalTarget: '/DeepReview review latest commit',
errorDetail: { category: 'network', rawMessage: 'network timeout' },
canResume: true,
recommendedActions: [
{ code: 'retry', labelKey: 'errors:ai.actions.retry' },
{ code: 'switch_model', labelKey: 'errors:ai.actions.switchModel' },
{ code: 'copy_diagnostics', labelKey: 'errors:ai.actions.copyDiagnostics' },
],
reviewers: [
{ reviewer: 'ReviewSecurity', status: 'completed' },
{ reviewer: 'ReviewPerformance', status: 'timed_out' },
],
},
phase: 'resume_failed',
});

await act(async () => {
root.render(<DeepReviewActionBar />);
});

const buttonTexts = Array.from(container.querySelectorAll('button'))
.map((button) => button.textContent ?? '');

expect(buttonTexts.some((text) => text.includes('Continue review'))).toBe(true);
expect(buttonTexts.some((text) => text.includes('Switch model'))).toBe(true);
expect(buttonTexts.some((text) => text.includes('Copy diagnostics'))).toBe(true);
expect(buttonTexts.some((text) => text.includes('Retry'))).toBe(false);
expect(buttonTexts.some((text) => text.includes('Show recovery plan'))).toBe(false);
expect(container.textContent).toContain('1 completed reviewers will be preserved');
expect(container.textContent).toContain('1 reviewers will be rerun');
});

it('minimizes and disables the continue action after a resume request starts successfully', async () => {
const { DeepReviewActionBar } = await import('./DeepReviewActionBar');

useReviewActionBarStore.getState().showInterruptedActionBar({
childSessionId: 'deep-review-session',
parentSessionId: 'parent-session',
interruption: {
phase: 'review_interrupted',
childSessionId: 'deep-review-session',
parentSessionId: 'parent-session',
originalTarget: '/DeepReview review latest commit',
errorDetail: { category: 'network', rawMessage: 'network timeout' },
canResume: true,
recommendedActions: [],
reviewers: [],
},
});

await act(async () => {
root.render(<DeepReviewActionBar />);
});

const continueButton = Array.from(container.querySelectorAll('button'))
.find((button) => button.textContent?.includes('Continue review'));
expect(continueButton).toBeTruthy();

await act(async () => {
continueButton!.dispatchEvent(new dom.window.MouseEvent('click', { bubbles: true }));
await Promise.resolve();
});

const state = useReviewActionBarStore.getState();
expect(continueDeepReviewSessionMock).toHaveBeenCalledTimes(1);
expect(state.phase).toBe('resume_running');
expect(state.minimized).toBe(true);

const restoredContinueButton = Array.from(container.querySelectorAll('button'))
.find((button) => button.textContent?.includes('Continue review')) as HTMLButtonElement | undefined;
expect(restoredContinueButton?.disabled).toBe(true);
});
});
Loading
Loading