Merge pull request #2389 from HUQIANTAO/feat/intent-summary-before-approval
feat: show intent summary before file approval prompt (#2381)
This commit is contained in:
@@ -11,6 +11,25 @@ fn loop_guard_block_tool_result(message: String) -> ToolResult {
|
||||
ToolResult::error(message).with_metadata(json!({"loop_guard": "identical_tool_call"}))
|
||||
}
|
||||
|
||||
const MAX_APPROVAL_INTENT_SUMMARY_CHARS: usize = 2_000;
|
||||
|
||||
fn approval_intent_summary(text: &str) -> Option<String> {
|
||||
let trimmed = text.trim();
|
||||
if trimmed.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut chars = trimmed.chars();
|
||||
let mut summary = chars
|
||||
.by_ref()
|
||||
.take(MAX_APPROVAL_INTENT_SUMMARY_CHARS)
|
||||
.collect::<String>();
|
||||
if chars.next().is_some() {
|
||||
summary.push_str("...");
|
||||
}
|
||||
Some(summary)
|
||||
}
|
||||
|
||||
impl Engine {
|
||||
pub(super) async fn handle_deepseek_turn(
|
||||
&mut self,
|
||||
@@ -1344,6 +1363,22 @@ impl Engine {
|
||||
}
|
||||
active_tool_names.extend(deferred_tools_hydrated_this_batch);
|
||||
|
||||
// --- Intent summary for write tools (#2381) ---
|
||||
// When the model invokes write tools, extract its preceding text
|
||||
// as an "intent summary" so the approval view can show *why* the
|
||||
// change is being made, not just *what* will change.
|
||||
let has_write_tools = plans.iter().any(|p| {
|
||||
!p.read_only
|
||||
&& p.approval_required
|
||||
&& p.blocked_error.is_none()
|
||||
&& p.guard_result.is_none()
|
||||
});
|
||||
let intent_summary: Option<String> = if has_write_tools {
|
||||
approval_intent_summary(¤t_text_visible)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
let plan_count = plans.len();
|
||||
let batches = plan_tool_execution_batches(plans);
|
||||
let parallel_chunks = batches
|
||||
@@ -1702,6 +1737,11 @@ impl Engine {
|
||||
description: plan.approval_description.clone(),
|
||||
approval_key,
|
||||
approval_grouping_key,
|
||||
intent_summary: if plan.read_only {
|
||||
None
|
||||
} else {
|
||||
intent_summary.clone()
|
||||
},
|
||||
})
|
||||
.await;
|
||||
|
||||
@@ -2256,6 +2296,19 @@ mod tests {
|
||||
assert!(!should_hold_turn_for_subagents(0, 0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_intent_summary_trims_and_bounds_text() {
|
||||
assert_eq!(approval_intent_summary(" "), None);
|
||||
|
||||
let long_text = format!(" {} ", "x".repeat(MAX_APPROVAL_INTENT_SUMMARY_CHARS + 10));
|
||||
let summary = approval_intent_summary(&long_text).expect("summary");
|
||||
assert!(summary.ends_with("..."));
|
||||
assert_eq!(
|
||||
summary.chars().count(),
|
||||
MAX_APPROVAL_INTENT_SUMMARY_CHARS + 3
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression test for issue #1727 (P0, release-blocking).
|
||||
///
|
||||
/// When a model (e.g. gpt-oss via ollama's harmony→OpenAI shim) returns
|
||||
|
||||
@@ -257,6 +257,10 @@ pub enum Event {
|
||||
/// Lossy / arity-aware fingerprint, used to scope *approvals* so an
|
||||
/// "approve for session" covers later flag variants (v0.8.37).
|
||||
approval_grouping_key: String,
|
||||
/// The model's explanation of intent before invoking write tools (#2381).
|
||||
/// Displayed in the approval view so users understand *why* the change
|
||||
/// is being made before reviewing *what* will change.
|
||||
intent_summary: Option<String>,
|
||||
},
|
||||
|
||||
/// Request user input for a tool call
|
||||
|
||||
@@ -2683,6 +2683,7 @@ impl RuntimeThreadManager {
|
||||
id,
|
||||
tool_name,
|
||||
description,
|
||||
intent_summary,
|
||||
..
|
||||
} => {
|
||||
self.emit_event(
|
||||
@@ -2695,6 +2696,7 @@ impl RuntimeThreadManager {
|
||||
"approval_id": id,
|
||||
"tool_name": tool_name,
|
||||
"description": description,
|
||||
"intent_summary": intent_summary,
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
@@ -4217,6 +4219,7 @@ mod tests {
|
||||
tool_name: "exec_command".to_string(),
|
||||
description: "stale approval".to_string(),
|
||||
input: serde_json::json!({}),
|
||||
intent_summary: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -4291,6 +4294,7 @@ mod tests {
|
||||
tool_name: "exec_command".to_string(),
|
||||
description: "external allow".to_string(),
|
||||
input: serde_json::json!({}),
|
||||
intent_summary: Some("I will update the config file.".to_string()),
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -4300,6 +4304,20 @@ mod tests {
|
||||
}
|
||||
assert_eq!(manager.pending_approvals_count(), 1);
|
||||
|
||||
let events = manager.events_since(&thread.id, None)?;
|
||||
let approval_event = events
|
||||
.iter()
|
||||
.rev()
|
||||
.find(|event| event.event == "approval.required")
|
||||
.context("missing approval.required event")?;
|
||||
assert_eq!(
|
||||
approval_event
|
||||
.payload
|
||||
.get("intent_summary")
|
||||
.and_then(Value::as_str),
|
||||
Some("I will update the config file.")
|
||||
);
|
||||
|
||||
assert!(manager.deliver_external_approval(
|
||||
"tool_external_allow",
|
||||
ExternalApprovalDecision::Allow { remember: false },
|
||||
@@ -4369,6 +4387,7 @@ mod tests {
|
||||
tool_name: "exec_command".to_string(),
|
||||
description: "external deny".to_string(),
|
||||
input: serde_json::json!({}),
|
||||
intent_summary: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
@@ -4556,6 +4575,7 @@ mod tests {
|
||||
tool_name: "exec_command".to_string(),
|
||||
description: "remember=true".to_string(),
|
||||
input: serde_json::json!({}),
|
||||
intent_summary: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
|
||||
@@ -134,15 +134,31 @@ pub struct ApprovalRequest {
|
||||
/// Lossy / arity-aware fingerprint, used to scope *approvals* so an
|
||||
/// "approve for session" covers later flag variants (v0.8.37).
|
||||
pub approval_grouping_key: String,
|
||||
/// The model's explanation of intent before invoking write tools (#2381).
|
||||
/// Displayed in the approval view so users understand *why* the change
|
||||
/// is being made before reviewing *what* will change.
|
||||
pub intent_summary: Option<String>,
|
||||
}
|
||||
|
||||
impl ApprovalRequest {
|
||||
#[cfg(test)]
|
||||
pub fn new(
|
||||
id: &str,
|
||||
tool_name: &str,
|
||||
description: &str,
|
||||
params: &Value,
|
||||
approval_key: &str,
|
||||
) -> Self {
|
||||
Self::new_with_intent(id, tool_name, description, params, approval_key, None)
|
||||
}
|
||||
|
||||
pub fn new_with_intent(
|
||||
id: &str,
|
||||
tool_name: &str,
|
||||
description: &str,
|
||||
params: &Value,
|
||||
approval_key: &str,
|
||||
intent_summary: Option<&str>,
|
||||
) -> Self {
|
||||
let category = get_tool_category(tool_name);
|
||||
let risk = classify_risk(tool_name, category, params);
|
||||
@@ -159,6 +175,14 @@ impl ApprovalRequest {
|
||||
params: params.clone(),
|
||||
approval_key: approval_key.to_string(),
|
||||
approval_grouping_key,
|
||||
intent_summary: intent_summary.and_then(|summary| {
|
||||
let summary = summary.trim();
|
||||
if summary.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(summary.to_string())
|
||||
}
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2058,6 +2058,7 @@ async fn run_event_loop(
|
||||
input,
|
||||
approval_key,
|
||||
approval_grouping_key,
|
||||
intent_summary,
|
||||
} => {
|
||||
let session_approved =
|
||||
is_session_approved_for_tool(app, &tool_name, &approval_grouping_key);
|
||||
@@ -2109,6 +2110,7 @@ async fn run_event_loop(
|
||||
&description,
|
||||
&tool_input,
|
||||
&approval_key,
|
||||
intent_summary.as_deref(),
|
||||
);
|
||||
log_sensitive_event(
|
||||
"tool.approval.prompted",
|
||||
@@ -6806,12 +6808,20 @@ fn push_approval_request_view(
|
||||
description: &str,
|
||||
tool_input: &serde_json::Value,
|
||||
approval_key: &str,
|
||||
intent_summary: Option<&str>,
|
||||
) {
|
||||
if tool_name == "apply_patch" {
|
||||
maybe_add_patch_preview(app, tool_input);
|
||||
}
|
||||
|
||||
let request = ApprovalRequest::new(id, tool_name, description, tool_input, approval_key);
|
||||
let request = ApprovalRequest::new_with_intent(
|
||||
id,
|
||||
tool_name,
|
||||
description,
|
||||
tool_input,
|
||||
approval_key,
|
||||
intent_summary,
|
||||
);
|
||||
app.view_stack
|
||||
.push(ApprovalView::new_for_locale(request, app.ui_locale));
|
||||
}
|
||||
|
||||
@@ -5555,6 +5555,7 @@ fn approval_prompt_uses_event_input_after_message_complete_drain() {
|
||||
"Run cargo tests",
|
||||
&event_input,
|
||||
"approval-key",
|
||||
None,
|
||||
);
|
||||
|
||||
let mut view = app.view_stack.pop().expect("approval view");
|
||||
|
||||
@@ -1166,6 +1166,45 @@ impl Renderable for ApprovalWidget<'_> {
|
||||
]));
|
||||
}
|
||||
|
||||
// Intent summary — the model's explanation of why this change is needed (#2381).
|
||||
if let Some(ref summary) = self.request.intent_summary {
|
||||
let max_width = card_area.width.saturating_sub(14) as usize;
|
||||
if max_width > 0 {
|
||||
lines.push(Line::from(""));
|
||||
let intent_label = match locale {
|
||||
Locale::ZhHans => "意图:",
|
||||
_ => "Intent: ",
|
||||
};
|
||||
let summary_lines: Vec<&str> = summary.lines().collect();
|
||||
for (i, sline) in summary_lines.iter().take(3).enumerate() {
|
||||
let prefix = if i == 0 { intent_label } else { " " };
|
||||
let truncated = crate::utils::truncate_with_ellipsis(sline, max_width, "...");
|
||||
lines.push(Line::from(vec![
|
||||
Span::raw(" "),
|
||||
Span::styled(
|
||||
prefix,
|
||||
if i == 0 {
|
||||
Style::default().fg(palette::TEXT_HINT)
|
||||
} else {
|
||||
Style::default()
|
||||
},
|
||||
),
|
||||
Span::styled(truncated, Style::default().fg(palette::TEXT_SECONDARY)),
|
||||
]));
|
||||
}
|
||||
if summary_lines.len() > 3 {
|
||||
let more = match locale {
|
||||
Locale::ZhHans => format!(" … (还有 {} 行)", summary_lines.len() - 3),
|
||||
_ => format!(" … (+{} lines)", summary_lines.len() - 3),
|
||||
};
|
||||
lines.push(Line::from(vec![
|
||||
Span::raw(" "),
|
||||
Span::styled(more, Style::default().fg(palette::TEXT_HINT)),
|
||||
]));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
lines.push(Line::from(""));
|
||||
let params_str = self.request.params_display();
|
||||
let params_width = card_area.width.saturating_sub(14) as usize;
|
||||
|
||||
Reference in New Issue
Block a user