diff --git a/crates/merry-cli/src/tui/process_output.rs b/crates/merry-cli/src/tui/process_output.rs index ce7ad672..7e9be03d 100644 --- a/crates/merry-cli/src/tui/process_output.rs +++ b/crates/merry-cli/src/tui/process_output.rs @@ -1,4 +1,4 @@ -use crate::tui::state::ProcessOutputPreview; +use crate::tui::state::ToolOutputPreview; use merry_runtime::ArtifactContent; use serde::Deserialize; use serde_json::Value; @@ -68,7 +68,7 @@ impl CapturedOutput { } } -pub(crate) fn process_output_preview(output: &str) -> Option { +pub(crate) fn process_output_preview(output: &str) -> Option { let value = serde_json::from_str::(output).ok()?; if value.get("kind").and_then(Value::as_str) != Some("process_action") { return None; @@ -83,7 +83,10 @@ pub(crate) fn process_output_preview(output: &str) -> Option Option { diff --git a/crates/merry-cli/src/tui/projector.rs b/crates/merry-cli/src/tui/projector.rs index 392bcafb..8328b5a4 100644 --- a/crates/merry-cli/src/tui/projector.rs +++ b/crates/merry-cli/src/tui/projector.rs @@ -4,17 +4,17 @@ use crate::tui::{ process_output::process_exit_code, projector::tool_output::{ compact_tool_output, completed_process_view, completed_tool_title, expanded_tool_title, - failed_tool_body, parse_apply_patch_view, started_tool_title_and_detail, - success_tool_bodies, tool_output_text, + failed_tool_body, parse_apply_patch_view, permission_output_bodies, + read_text_output_preview, started_tool_title_and_detail, tool_output_text, }, - state::{CommandView, QueuePreview, TimelineItem, TuiState}, + state::{CommandView, QueuePreview, TimelineItem, ToolOutputPreview, TuiState}, }; use merry_core::{ PlanAttemptOutcome, PlanDirectiveStatus, PlanPhase, RuntimeEvent, TOOL_CANCELLED_BY_USER_CODE, ToolCallId, ToolCallResultStatus, ToolName, }; use merry_runtime::SessionTranscriptItem; -use merry_tools::APPLY_PATCH_TOOL; +use merry_tools::{APPLY_PATCH_TOOL, READ_TEXT_TOOL}; use serde_json::Value; use std::collections::HashMap; use tokio::time::Instant; @@ -209,7 +209,19 @@ impl TuiProjector { state.push_timeline_item(patch_item); } } else if let Some(tool) = tool.as_ref() - && let Some(preview) = success_tool_bodies(tool.name.as_str(), &text) + && tool.name.as_str() == READ_TEXT_TOOL + { + state.replace_timeline_item( + tool.timeline_index, + TimelineItem::Read { + title: expanded_tool_title(tool), + preview: read_text_output_preview(&text) + .unwrap_or_else(|| ToolOutputPreview::new(text.lines(), false)), + }, + ); + } else if let Some(tool) = tool.as_ref() + && tool.name.as_str() == "request_permissions" + && let Some(preview) = permission_output_bodies(&text) { state.replace_timeline_item( tool.timeline_index, diff --git a/crates/merry-cli/src/tui/projector/tool_output.rs b/crates/merry-cli/src/tui/projector/tool_output.rs index 8768a1d3..2acd789d 100644 --- a/crates/merry-cli/src/tui/projector/tool_output.rs +++ b/crates/merry-cli/src/tui/projector/tool_output.rs @@ -8,9 +8,8 @@ use crate::{ projector::StartedToolView, state::{ CommandFailure, CommandView, PatchChangeView, PatchLineKind, PatchLineView, - PatchOperationView, ProcessOutputPreview, TimelineItem, + PatchOperationView, TimelineItem, ToolOutputPreview, }, - text_wrap::truncate_chars, tool_error::compact_failed_tool_body, }, }; @@ -23,11 +22,6 @@ use serde::Deserialize; use serde_json::Value; use std::collections::HashMap; -// This bounds the timeline preview only; the workspace tool and Focus retain the full file. -pub(super) const READ_FILE_PREVIEW_MAX_LINES: usize = 120; - -pub(super) const READ_FILE_PREVIEW_MAX_CHARS: usize = 180; - pub(super) fn expanded_tool_title(tool: &StartedToolView) -> String { if tool.detail.is_empty() { return tool.title.clone(); @@ -111,14 +105,6 @@ pub(super) fn parse_mcp_tool_name(name: &str) -> Option<(&str, &str)> { Some((server, tool)) } -pub(super) fn success_tool_bodies(name: &str, output: &str) -> Option { - match name { - "request_permissions" => permission_output_bodies(output), - "read_text" => read_text_output_bodies(output), - _ => None, - } -} - pub(super) fn permission_output_bodies(output: &str) -> Option { let value = serde_json::from_str::(output).ok()?; if value.get("ok").and_then(Value::as_bool) != Some(true) @@ -138,25 +124,22 @@ pub(super) fn permission_output_bodies(output: &str) -> Option { Some(body) } -pub(super) fn read_text_output_bodies(output: &str) -> Option { +pub(super) fn read_text_output_preview(output: &str) -> Option { let output = serde_json::from_str::(output).ok()?; if !output.ok || output.tool.as_deref() != Some("read_text") { return None; } - let mut lines = output - .content - .lines() - .take(READ_FILE_PREVIEW_MAX_LINES) - .map(|line| truncate_chars(line, READ_FILE_PREVIEW_MAX_CHARS)) - .collect::>(); - if output.truncated { - lines.push("... truncated".to_owned()); - } - if lines.is_empty() { - lines.push(format!("{} is empty", output.path)); + if output.content.is_empty() { + return Some(ToolOutputPreview::new( + std::iter::once(format!("{} is empty", output.path).as_str()), + output.truncated, + )); } - Some(lines.join("\n")) + Some(ToolOutputPreview::new( + output.content.lines(), + output.truncated, + )) } #[derive(Debug, Deserialize)] @@ -188,11 +171,10 @@ pub(super) fn completed_process_view( None }; let mut preview = process_output_preview(output) - .unwrap_or_else(|| ProcessOutputPreview::new(&compact_tool_output(output), "", false)); + .unwrap_or_else(|| ToolOutputPreview::new(compact_tool_output(output).lines(), false)); if failure == Some(CommandFailure::Failed) { - preview = ProcessOutputPreview::new( - &failed_tool_body(result.diagnostic(), output), - "", + preview = ToolOutputPreview::new( + failed_tool_body(result.diagnostic(), output).lines(), preview.truncated, ); } diff --git a/crates/merry-cli/src/tui/render/timeline.rs b/crates/merry-cli/src/tui/render/timeline.rs index 73814293..0c623ed1 100644 --- a/crates/merry-cli/src/tui/render/timeline.rs +++ b/crates/merry-cli/src/tui/render/timeline.rs @@ -6,7 +6,8 @@ use crate::tui::{ markdown::{RenderedMarkdown, markdown_lines}, render::command_style::command_spans, state::{ - CommandFailure, CommandView, PatchChangeView, PatchOperationView, TimelineItem, TuiState, + CommandFailure, CommandView, PatchChangeView, PatchOperationView, TimelineItem, + ToolOutputPreview, TuiState, }, text_interaction::TextSelection, text_wrap::{ @@ -31,9 +32,6 @@ use super::timeline_layout::{ TimelineLayout, timeline_layout, timeline_scroll_start, timeline_viewport, }; -// Keep prefix eviction below the viewport start when Paragraph scroll exceeds u16. -pub(super) const TOOL_RESULT_PREVIEW_MAX_LINES: usize = 5; - pub(super) fn render_timeline_pane(frame: &mut Frame<'_>, state: &TuiState, region: Rect) { let mut block = Block::default() .borders(Borders::BOTTOM) @@ -345,12 +343,38 @@ pub(super) fn expanded_timeline_lines( return lines; } + lines.extend(output_preview_lines( + state, + &ToolOutputPreview::new(body.lines(), false), + true, + region_width, + )); + lines +} + +pub(super) fn read_output_lines( + state: &TuiState, + title: &str, + preview: &ToolOutputPreview, + region_width: u16, +) -> Vec> { + let mut lines = vec![expanded_title_line(state, title)]; + lines.extend(output_preview_lines(state, preview, false, region_width)); + lines +} + +fn output_preview_lines( + state: &TuiState, + preview: &ToolOutputPreview, + always_show: bool, + region_width: u16, +) -> Vec> { + let mut lines = Vec::new(); + if !always_show && !state.show_successful_command_output() { + return lines; + } let body_width = usize::from(region_width).saturating_sub(2).max(4); - for line in body - .lines() - .filter(|line| !line.trim().is_empty()) - .take(TOOL_RESULT_PREVIEW_MAX_LINES) - { + for line in &preview.lines { let clean = crate::text::without_control_chars(line); let clean = clean.trim(); if clean.is_empty() { @@ -361,6 +385,12 @@ pub(super) fn expanded_timeline_lines( semantic_style(state, SemanticColor::Muted), ))); } + if preview.truncated { + lines.push(Line::from(Span::styled( + " ...", + semantic_style(state, SemanticColor::Muted), + ))); + } lines } @@ -414,21 +444,8 @@ pub(super) fn command_lines( } append_command_elapsed(state, &mut title, *elapsed); let mut lines = wrap_command_title(title, region_width); - if failed || (failure.is_none() && state.show_successful_command_output()) { - let body_width = usize::from(region_width).saturating_sub(2).max(4); - for line in &preview.lines { - let clean = crate::text::without_control_chars(line); - lines.push(Line::from(Span::styled( - format!(" {}", truncate_chars(clean.trim(), body_width)), - semantic_style(state, SemanticColor::Muted), - ))); - } - if preview.truncated { - lines.push(Line::from(Span::styled( - " ...", - semantic_style(state, SemanticColor::Muted), - ))); - } + if failed || failure.is_none() { + lines.extend(output_preview_lines(state, preview, failed, region_width)); } lines } diff --git a/crates/merry-cli/src/tui/render/timeline_layout.rs b/crates/merry-cli/src/tui/render/timeline_layout.rs index 24198cd3..934ff2c9 100644 --- a/crates/merry-cli/src/tui/render/timeline_layout.rs +++ b/crates/merry-cli/src/tui/render/timeline_layout.rs @@ -2,7 +2,7 @@ use super::timeline::{ assistant_lines, command_lines, compact_patch_lines, diagnostic_lines, expanded_timeline_lines, - local_command_lines, muted_lines, user_lines, + local_command_lines, muted_lines, read_output_lines, user_lines, }; use crate::tui::{ copy_controls::CopyTarget, @@ -67,6 +67,12 @@ pub(super) fn timeline_layout(state: &TuiState, region: Rect) -> TimelineLayout .into_iter() .map(|line| TranscriptRow::new(line, SelectionPolicy::Keep)) .collect(), + TimelineItem::Read { title, preview } => { + read_output_lines(state, title, preview, region.width) + .into_iter() + .map(|line| TranscriptRow::new(line, SelectionPolicy::Keep)) + .collect() + } TimelineItem::LocalCommand { title, body } => { let rendered = local_command_lines(state, title, body, region.width); item_targets = rendered.copy_targets; diff --git a/crates/merry-cli/src/tui/state.rs b/crates/merry-cli/src/tui/state.rs index 1c930804..3e1bc1c6 100644 --- a/crates/merry-cli/src/tui/state.rs +++ b/crates/merry-cli/src/tui/state.rs @@ -19,7 +19,7 @@ use std::{ pub(crate) use timeline::TimelineAnchor; pub(crate) use views::{ CommandFailure, CommandView, PatchChangeView, PatchLineKind, PatchLineView, PatchOperationView, - ProcessOutputPreview, QueuePreview, QueuePreviewItem, QueuePreviewState, TimelineItem, + QueuePreview, QueuePreviewItem, QueuePreviewState, TimelineItem, ToolOutputPreview, }; mod overlays; @@ -132,7 +132,7 @@ impl TuiState { self } - /// Whether completed successful commands show their bounded output preview. + /// Whether successful process commands and file reads show their bounded output preview. pub(crate) fn show_successful_command_output(&self) -> bool { self.show_successful_command_output } diff --git a/crates/merry-cli/src/tui/state/views.rs b/crates/merry-cli/src/tui/state/views.rs index 438a1f1d..c555225d 100644 --- a/crates/merry-cli/src/tui/state/views.rs +++ b/crates/merry-cli/src/tui/state/views.rs @@ -2,30 +2,36 @@ use merry_core::{ArtifactRef, QueuedInputLane, QueuedInputView}; use std::time::Duration; use tokio::time::Instant; -const PROCESS_PREVIEW_MAX_LINES: usize = 5; +const TOOL_PREVIEW_MAX_LINES: usize = 5; +const TOOL_PREVIEW_MAX_LINE_CHARS: usize = 180; -/// A bounded display preview; complete process output remains in runtime artifacts. +/// A bounded display preview; complete tool output remains in runtime artifacts. #[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct ProcessOutputPreview { +pub(crate) struct ToolOutputPreview { pub(crate) lines: Vec, pub(crate) truncated: bool, } -impl ProcessOutputPreview { - /// Keeps five nonempty stdout/stderr lines and records local or upstream truncation. - pub(crate) fn new(stdout: &str, stderr: &str, source_truncated: bool) -> Self { - let mut output_lines = stdout - .lines() - .chain(stderr.lines()) - .filter(|line| !line.trim().is_empty()); - let lines = output_lines - .by_ref() - .take(PROCESS_PREVIEW_MAX_LINES) - .map(str::to_owned) - .collect(); +impl ToolOutputPreview { + /// Keeps five nonempty lines of at most 180 Unicode scalar values each. + /// Records character, line, or upstream truncation without retaining full output. + pub(crate) fn new<'a>(lines: impl Iterator, source_truncated: bool) -> Self { + let mut output_lines = lines.filter(|line| !line.trim().is_empty()); + let mut lines = Vec::new(); + let mut truncated = source_truncated; + for line in output_lines.by_ref().take(TOOL_PREVIEW_MAX_LINES) { + let mut characters = line.chars(); + lines.push( + characters + .by_ref() + .take(TOOL_PREVIEW_MAX_LINE_CHARS) + .collect(), + ); + truncated |= characters.next().is_some(); + } Self { lines, - truncated: source_truncated || output_lines.next().is_some(), + truncated: truncated || output_lines.next().is_some(), } } } @@ -48,7 +54,7 @@ pub(crate) enum CommandView { detail: String, exit_code: Option, failure: Option, - preview: ProcessOutputPreview, + preview: ToolOutputPreview, command: String, cwd: String, artifact: ArtifactRef, @@ -203,12 +209,37 @@ pub(crate) struct PatchChangeView { #[derive(Debug, Clone, PartialEq, Eq)] #[allow(dead_code)] pub(crate) enum TimelineItem { - User { text: String, lane: QueuedInputLane }, - Assistant { text: String }, - Muted { title: String, detail: String }, - Command { view: CommandView }, - LocalCommand { title: String, body: String }, - Expanded { title: String, body: String }, - Diagnostic { title: String, body: String }, - Patch { changes: Vec }, + User { + text: String, + lane: QueuedInputLane, + }, + Assistant { + text: String, + }, + Muted { + title: String, + detail: String, + }, + Command { + view: CommandView, + }, + Read { + title: String, + preview: ToolOutputPreview, + }, + LocalCommand { + title: String, + body: String, + }, + Expanded { + title: String, + body: String, + }, + Diagnostic { + title: String, + body: String, + }, + Patch { + changes: Vec, + }, } diff --git a/crates/merry-cli/src/tui/tests.rs b/crates/merry-cli/src/tui/tests.rs index aeb0d119..aa1ea098 100644 --- a/crates/merry-cli/src/tui/tests.rs +++ b/crates/merry-cli/src/tui/tests.rs @@ -112,10 +112,14 @@ mod input_controls; mod layout; +mod output_preview; + mod patch_projection; mod process_projection; +mod read_output; + mod provider_interactions; mod settings; diff --git a/crates/merry-cli/src/tui/tests/event_projection.rs b/crates/merry-cli/src/tui/tests/event_projection.rs index 39191798..bad052fe 100644 --- a/crates/merry-cli/src/tui/tests/event_projection.rs +++ b/crates/merry-cli/src/tui/tests/event_projection.rs @@ -47,7 +47,8 @@ fn projector_rebuilds_resume_transcript_history() { "gpt-test".to_owned(), Keymap::default(), TuiTheme::default(), - ); + ) + .with_successful_command_output(true); let mut projector = TuiProjector::default(); let call = pending_call_with_args("call-read", "read_text", json!({"path": "hello_world.py"})); let result = ToolCallResult::succeeded( @@ -99,12 +100,9 @@ fn projector_rebuilds_resume_transcript_history() { &state.timeline()[1], TimelineItem::Assistant { text } if text == "我先读一下文件。" )); - assert!(matches!( - &state.timeline()[2], - TimelineItem::Expanded { title, body } - if title == "Read read_text path=hello_world.py" - && body.contains("print('hi')") - )); + let rendered = render_to_text(&state, 120, 24); + assert!(rendered.contains("Read read_text path=hello_world.py")); + assert!(rendered.contains("print('hi')")); } #[test] diff --git a/crates/merry-cli/src/tui/tests/output_preview.rs b/crates/merry-cli/src/tui/tests/output_preview.rs new file mode 100644 index 00000000..fa28e78c --- /dev/null +++ b/crates/merry-cli/src/tui/tests/output_preview.rs @@ -0,0 +1,39 @@ +use crate::tui::state::ToolOutputPreview; + +#[test] +fn output_preview_bounds_long_ascii_and_multibyte_lines() { + for content in ["x".repeat(900_000), "界🚀".repeat(150_000)] { + let preview = ToolOutputPreview::new(content.lines(), false); + + assert!(preview.lines.iter().map(String::len).sum::() <= 720); + assert_eq!( + preview.lines, + [content.chars().take(180).collect::()] + ); + assert!(preview.truncated); + } +} + +#[test] +fn output_preview_preserves_exact_character_boundary_and_source_truncation() { + let content = "🚀".repeat(180); + for source_truncated in [false, true] { + let preview = ToolOutputPreview::new(content.lines(), source_truncated); + + assert_eq!(preview.lines.as_slice(), std::slice::from_ref(&content)); + assert_eq!(preview.truncated, source_truncated); + } +} + +#[test] +fn output_preview_bounds_total_retained_text_and_keeps_short_lines() { + let long_line = "界🚀".repeat(200); + let content = format!("\nshort\n{long_line}\n{long_line}\n{long_line}\n{long_line}\nsixth"); + let preview = ToolOutputPreview::new(content.lines(), false); + + assert_eq!(preview.lines.len(), 5); + assert_eq!(preview.lines[0], "short"); + assert!(preview.lines.iter().all(|line| line.chars().count() <= 180)); + assert!(preview.lines.iter().map(String::len).sum::() <= 3_600); + assert!(preview.truncated); +} diff --git a/crates/merry-cli/src/tui/tests/patch_projection.rs b/crates/merry-cli/src/tui/tests/patch_projection.rs index e28bc0fa..b7ce3a66 100644 --- a/crates/merry-cli/src/tui/tests/patch_projection.rs +++ b/crates/merry-cli/src/tui/tests/patch_projection.rs @@ -10,47 +10,6 @@ use merry_core::{ErrorInfo, RuntimeEvent, ToolCallId, ToolCallResult, ToolOutput use merry_tools::APPLY_PATCH_TOOL; use serde_json::json; -#[test] -fn projector_keeps_non_patch_tool_results_compact_without_raw_json() { - let mut state = TuiState::new( - "/repo".into(), - "gpt-test".to_owned(), - Keymap::default(), - TuiTheme::default(), - ); - let mut projector = TuiProjector::default(); - - projector.apply( - RuntimeEvent::ToolCallStarted { - call: pending_call("call-read", "read_text"), - source: source(), - }, - &mut state, - ); - projector.apply( - RuntimeEvent::ToolCallFinished { - result: ToolCallResult::succeeded( - ToolCallId::new("call-read").unwrap(), - text_artifact("read-output"), - ), - output: Some(ToolOutput::Json { - json: r#"{"ok":true,"tool":"read_text","path":"AGENTS.md","bytes":19704,"content":"large raw content"}"#.to_owned(), - }), - source: source(), - }, - &mut state, - ); - - assert_eq!(state.timeline().len(), 1); - let TimelineItem::Expanded { title, body } = &state.timeline()[0] else { - panic!("read tool result should expand to a compact preview"); - }; - assert_eq!(title, "Read read_text"); - assert!(!body.contains("AGENTS.md:1")); - assert!(body.contains("large raw content")); - assert!(!body.contains(r#""content":"#)); -} - #[test] fn projector_projects_apply_patch_using_patch_tool_format() { let mut state = TuiState::new( @@ -416,13 +375,14 @@ fn projector_expands_apply_patch_by_tool_name() { } #[test] -fn projector_keeps_diff_like_non_patch_output_muted() { +fn projector_keeps_diff_like_non_patch_output_as_a_read_preview() { let mut state = TuiState::new( "/repo".into(), "gpt-test".to_owned(), Keymap::default(), TuiTheme::default(), - ); + ) + .with_successful_command_output(true); let mut projector = TuiProjector::default(); projector.apply( @@ -447,7 +407,10 @@ fn projector_keeps_diff_like_non_patch_output_muted() { ); assert_eq!(state.timeline().len(), 1); - assert!(matches!(state.timeline()[0], TimelineItem::Expanded { .. })); + assert!(!matches!(state.timeline()[0], TimelineItem::Patch { .. })); + let rendered = render_to_text(&state, 120, 24); + assert!(rendered.contains("Read read_text")); + assert!(rendered.contains("+not a patch result")); } #[test] diff --git a/crates/merry-cli/src/tui/tests/read_output.rs b/crates/merry-cli/src/tui/tests/read_output.rs new file mode 100644 index 00000000..25590b75 --- /dev/null +++ b/crates/merry-cli/src/tui/tests/read_output.rs @@ -0,0 +1,283 @@ +use crate::tui::{ + keymap::Keymap, + projector::TuiProjector, + render::render_to_text, + state::{TimelineItem, TuiState}, + tests::{pending_call_with_args, source}, + theme::TuiTheme, +}; +use merry_core::{ + ArtifactId, ArtifactKind, ArtifactRef, ErrorInfo, RuntimeEvent, ToolCallId, ToolCallResult, + ToolOutput, +}; +use merry_runtime::SessionTranscriptItem; +use serde_json::json; + +#[test] +fn successful_read_output_follows_display_policy_live_and_on_resume() { + for show_output in [false, true] { + for replay in [false, true] { + for output in [ + ToolOutput::Json { + json: json!({ + "ok": true, + "tool": "read_text", + "path": "notes.txt", + "start_line": 2, + "end_line": 4, + "content": "read-output-marker\nsecond line\nthird line\n", + "truncated": true, + }) + .to_string(), + }, + ToolOutput::Text { + text: "read-output-marker".to_owned(), + }, + ] { + let mut state = read_state(show_output); + let kind = match &output { + ToolOutput::Json { .. } => ArtifactKind::Json, + ToolOutput::Text { .. } => ArtifactKind::Text, + }; + let result = successful_read_result(kind); + + project_read_result( + &mut state, + replay, + result, + output, + json!({"path": "notes.txt", "start_line": 2, "max_lines": 3}), + ); + + let rendered = render_to_text(&state, 120, 24); + assert!(rendered.contains("Read read_text")); + assert!(rendered.contains("path=notes.txt")); + assert!(rendered.contains("start_line=2")); + assert!(rendered.contains("max_lines=3")); + assert_eq!( + rendered.contains("read-output-marker"), + show_output, + "read output policy must hold for live and replayed results: replay={replay}" + ); + } + } + } +} + +#[test] +fn failed_read_output_stays_visible_live_and_on_resume() { + for show_output in [false, true] { + for replay in [false, true] { + let mut state = read_state(show_output); + let result = ToolCallResult::failed( + ToolCallId::new("read-call").unwrap(), + ArtifactRef::new(ArtifactId::new("read-result").unwrap(), ArtifactKind::Json), + ErrorInfo::new("file_not_found", "workspace file was not found").unwrap(), + ); + let output = ToolOutput::Json { + json: json!({ + "ok": false, + "tool": "read_text", + "path": "notes.txt", + "error": { + "code": "file_not_found", + "message": "workspace file was not found", + }, + }) + .to_string(), + }; + + project_read_result( + &mut state, + replay, + result, + output, + json!({"path": "notes.txt", "start_line": 2, "max_lines": 3}), + ); + + let rendered = render_to_text(&state, 120, 24); + assert!(rendered.contains("path=notes.txt")); + assert!(rendered.contains("failed")); + assert!(rendered.contains("workspace file was not found")); + } + } +} + +#[test] +fn read_preview_preserves_truncation_and_follows_render_time_settings() { + for replay in [false, true] { + for (content, source_truncated, preview_truncated) in [ + ("first\nsecond\nthird\nfourth\nfifth\n", false, false), + ("first\nsecond\nthird\nfourth\nfifth\n", true, true), + ("first\nsecond\nthird\nfourth\nfifth\nsixth\n", false, true), + ( + "\nfirst\n\nsecond\nthird\nfourth\nfifth\nsixth\n", + false, + true, + ), + ("first\n", true, true), + ] { + let mut state = read_state(false); + let result = successful_read_result(ArtifactKind::Json); + let output = ToolOutput::Json { + json: json!({ + "ok": true, + "tool": "read_text", + "path": "notes.txt", + "content": content, + "truncated": source_truncated, + }) + .to_string(), + }; + project_read_result( + &mut state, + replay, + result, + output, + json!({"path": "notes.txt", "start_line": 2, "max_lines": 3}), + ); + + assert!(!render_to_text(&state, 120, 24).contains("first")); + state = state.with_successful_command_output(true); + let rendered = render_to_text(&state, 120, 24); + assert!(rendered.contains("first")); + assert!(!rendered.contains("sixth")); + assert_eq!(rendered.contains("..."), preview_truncated); + state = state.with_successful_command_output(false); + assert!(!render_to_text(&state, 120, 24).contains("first")); + } + } +} + +#[test] +fn successful_read_shows_decoded_content_and_arguments_without_completion_noise() { + for arguments in [json!({}), json!({"path": "AGENTS.md"})] { + let has_path = arguments.get("path").is_some(); + let mut state = read_state(true); + let result = successful_read_result(ArtifactKind::Json); + let output = ToolOutput::Json { + json: json!({ + "ok": true, + "tool": "read_text", + "path": "AGENTS.md", + "bytes": 19704, + "content": "large raw content", + }) + .to_string(), + }; + + project_read_result(&mut state, false, result, output, arguments); + + assert_eq!(state.timeline().len(), 1); + let rendered = render_to_text(&state, 120, 24); + assert!(rendered.contains("Read read_text")); + assert_eq!(rendered.contains("path=AGENTS.md"), has_path); + assert!(rendered.contains("large raw content")); + assert!(!rendered.contains("AGENTS.md:1")); + assert!(!rendered.contains("completed")); + assert!(!rendered.contains(r#""content":"#)); + } +} + +#[test] +fn read_projection_retains_only_bounded_previews_for_json_text_and_empty_files() { + let content = "界🚀".repeat(150_000); + for replay in [false, true] { + for output in [ + ToolOutput::Text { + text: content.clone(), + }, + ToolOutput::Json { + json: json!({ + "ok": true, "tool": "read_text", "path": "notes.txt", "content": content, + }) + .to_string(), + }, + ToolOutput::Json { + json: json!({ + "ok": true, "tool": "read_text", "path": content, "content": "", + }) + .to_string(), + }, + ] { + let kind = match &output { + ToolOutput::Json { .. } => ArtifactKind::Json, + ToolOutput::Text { .. } => ArtifactKind::Text, + }; + let mut state = read_state(false); + project_read_result( + &mut state, + replay, + successful_read_result(kind), + output, + json!({"path": "notes.txt"}), + ); + + let TimelineItem::Read { preview, .. } = &state.timeline()[0] else { + panic!("read result must retain a bounded preview"); + }; + assert!(preview.lines.iter().map(String::len).sum::() <= 720); + assert!(preview.truncated); + assert!(!render_to_text(&state, 400, 24).contains('界')); + state = state.with_successful_command_output(true); + let rendered = render_to_text(&state, 400, 24); + assert!(rendered.contains('界')); + assert!(rendered.contains("...")); + } + } +} + +fn read_state(show_output: bool) -> TuiState { + TuiState::new( + "/repo".into(), + "test-model".to_owned(), + Keymap::default(), + TuiTheme::default(), + ) + .with_successful_command_output(show_output) +} + +fn successful_read_result(kind: ArtifactKind) -> ToolCallResult { + ToolCallResult::succeeded( + ToolCallId::new("read-call").unwrap(), + ArtifactRef::new(ArtifactId::new("read-result").unwrap(), kind), + ) +} + +fn project_read_result( + state: &mut TuiState, + replay: bool, + result: ToolCallResult, + output: ToolOutput, + arguments: serde_json::Value, +) { + let mut projector = TuiProjector::default(); + let call = pending_call_with_args(result.call_id().as_str(), "read_text", arguments); + if replay { + projector.apply_transcript_item(SessionTranscriptItem::ToolCall { call }, state); + projector.apply_transcript_item( + SessionTranscriptItem::ToolResult { + call_id: result.call_id().clone(), + result, + output: Some(output), + }, + state, + ); + } else { + projector.apply( + RuntimeEvent::ToolCallStarted { + call, + source: source(), + }, + state, + ); + projector.apply( + RuntimeEvent::ToolCallFinished { + result, + output: Some(output), + source: source(), + }, + state, + ); + } +} diff --git a/crates/merry-cli/src/tui/tests/tool_projection.rs b/crates/merry-cli/src/tui/tests/tool_projection.rs index a8c413ef..98b24ea4 100644 --- a/crates/merry-cli/src/tui/tests/tool_projection.rs +++ b/crates/merry-cli/src/tui/tests/tool_projection.rs @@ -65,7 +65,9 @@ fn projector_keeps_successful_non_patch_tool_compact_and_expands_patch_tool() { ); assert_eq!(state.timeline().len(), 2); - assert!(!matches!(state.timeline()[0], TimelineItem::Muted { .. })); + let rendered = render_to_text(&state, 120, 24); + assert!(rendered.contains("Read read_text")); + assert!(!rendered.contains("file contents")); assert!(matches!(state.timeline()[1], TimelineItem::Expanded { .. })); } @@ -328,47 +330,6 @@ fn projector_expands_tool_batches_in_model_order() { assert!(!rendered.contains("Ran ")); } -#[test] -fn projector_shows_tool_call_arguments_without_completed_noise() { - let mut state = TuiState::new( - "/repo".into(), - "gpt-test".to_owned(), - Keymap::default(), - TuiTheme::default(), - ); - let mut projector = TuiProjector::default(); - - projector.apply( - RuntimeEvent::ToolCallStarted { - call: pending_call_with_args("call-read", "read_text", json!({ "path": "AGENTS.md" })), - source: source(), - }, - &mut state, - ); - projector.apply( - RuntimeEvent::ToolCallFinished { - result: ToolCallResult::succeeded( - ToolCallId::new("call-read").unwrap(), - text_artifact("read-output"), - ), - output: Some(ToolOutput::Json { - json: r#"{"ok":true,"tool":"read_text","path":"AGENTS.md","bytes":19704,"content":"large raw content"}"#.to_owned(), - }), - source: source(), - }, - &mut state, - ); - - assert_eq!(state.timeline().len(), 1); - let TimelineItem::Expanded { title, body } = &state.timeline()[0] else { - panic!("read tool call should expand to a compact preview"); - }; - assert_eq!(title, "Read read_text path=AGENTS.md"); - assert!(!body.contains("AGENTS.md:1")); - assert!(body.contains("large raw content")); - assert!(!body.contains("completed")); -} - #[test] fn renderer_shows_tool_result_preview_below_tool_call() { let mut state = TuiState::new( diff --git a/crates/merry-coding/src/lib.rs b/crates/merry-coding/src/lib.rs index 772cba8b..0bf0c9e4 100644 --- a/crates/merry-coding/src/lib.rs +++ b/crates/merry-coding/src/lib.rs @@ -7,6 +7,7 @@ //! second coding policy. mod child_runtime; +mod policy_prompt; mod profile_hash; mod project_capabilities; mod project_rules; @@ -17,6 +18,7 @@ mod workspace; #[cfg(test)] mod tests; +pub use policy_prompt::CODING_AGENT_POLICY_PROMPT; pub use profile_hash::CodingAgentProfileHash; pub use project_rules::{ MAX_ROOT_PROJECT_RULES_BYTES, ProjectRulesLoadError, ROOT_PROJECT_RULES_FILE, @@ -55,21 +57,6 @@ pub const CODING_AGENT_STABLE_PREFIX_LAYOUT: &str = pub const CODING_AGENT_DYNAMIC_CONTEXT_LAYOUT: &str = "checkpoint|task-anchor|plan-control|compiled-context|transcript|tool-results|user-input"; -/// Stable coding-specific policy block inserted after runtime instructions. -pub const CODING_AGENT_POLICY_PROMPT: &str = r#" -This is a coding-agent run. Inspect the repository and its governing rules before changing files. Keep runtime state, task progress, artifacts, checkpoints, permissions, and tool results in their owning runtime contracts; do not treat a raw transcript as the source of truth. - -Use the registered file and process tools according to their typed schemas. Use `read_text` for a bounded line range from a known text file; never request complete-file content when a focused range is enough. Use `run_process` for repository discovery and verification when it is available, preferring the installed modern search tools the workspace capability facts report, bounded commands such as `rg --files`, a focused literal `rg` search, or `sed -n ',p'`. Avoid broad recursive output, `cat` on large files, and repeated exploratory calls. Use `apply_patch` for edits, keep hunks localized, and include only the smallest unique context needed. Permission, phase, role, and path scope are runtime admission decisions; do not invent tools or request broader capability than the exact action needs. - -A sandboxed action starts with no network access and no access beyond what trusted global configuration already granted. Decide what a command needs before running it rather than after it fails: a command that authenticates, installs, downloads, publishes, or otherwise reaches a remote service needs network requested in the same `run_process` call, while a command that only touches the workspace and the configured local baseline needs no additional capability. Paths and host integrations enabled by trusted global configuration are already available to sandboxed commands. If a process command needs access that is still missing, such as network, a reviewed path, or an unconfigured host integration, include all required capabilities in that same `run_process` call under `permissions`; Merry reviews them before execution and runs the exact command through the permissioned backend when approved. Use `reason` to explain the minimum required access. Use `request_permissions` only when the needed capability is discovered after a failed sandboxed attempt or when the action is not being retried through `run_process`. Treat the sandbox as the first explanation when a process command fails: a capability it withheld often surfaces as a credentials, authentication, or connectivity error, so re-run the same action with the missing capability before concluding anything about the user's machine, account, or local setup. - -When a tool fails, preserve the failure evidence, determine whether the cause is validation, missing permission, unavailable capability, or an implementation error, and then either make a bounded recovery attempt or report the blocker. Do not repeat an identical failed action without new evidence or an explicit reviewed admission. - -For delegated coding work, each child max_model_turns covers its full lifecycle and must be at least 2048. The configured maximum may be larger. Reaching the limit is recoverable when that maximum leaves room: inspect the child status, then spawn a replacement with the same plan_client_key and a larger budget so it can continue from the shared workspace. - -Before finishing, verify the requested behavior with the narrowest deterministic checks that prove it. The final report is an evidence-backed summary: state what changed or was answered, name the checks that actually ran and their outcomes, distinguish skipped or blocked checks, and call out remaining risks. Never claim an unrun check succeeded. -"#; - /// Minimum model-turn budget for a coding-profile child agent. pub const MIN_CODING_SUBAGENT_MODEL_TURNS: u32 = 2048; diff --git a/crates/merry-coding/src/policy_prompt.rs b/crates/merry-coding/src/policy_prompt.rs new file mode 100644 index 00000000..14433019 --- /dev/null +++ b/crates/merry-coding/src/policy_prompt.rs @@ -0,0 +1,18 @@ +//! Stable coding policy, separate from tool mechanics and workspace capability facts. + +/// Stable coding-specific policy block inserted after runtime instructions. +pub const CODING_AGENT_POLICY_PROMPT: &str = r#" +This is a coding-agent run. Inspect the repository and its governing rules before changing files. Keep runtime state, task progress, artifacts, checkpoints, permissions, and tool results in their owning runtime contracts; do not treat a raw transcript as the source of truth. + +Use the registered file and process tools according to their typed schemas. Choose file-read scope by the task: prefer a focused range when it answers the question, but read a larger range or the whole file when understanding or changing its overall structure requires it. Avoid fragmenting necessary whole-file inspection into many tiny reads. Use `read_text` for known text files within its configured per-call limits. Reuse unchanged content already available in context, whether from file tools, process output, or successful edits; re-read only when relevant content may have changed, needed lines were not returned or are no longer available after compaction, or a concrete verification or patch mismatch requires fresh evidence. + +Use `run_process` for repository discovery and verification when it is available, preferring the installed modern search tools the workspace capability facts report: `rg --files` to list files, a focused literal `rg` search for content, and `fd`/`fdfind` for paths by name; fall back to `grep -r` and `find` only for a tool the facts say is missing. Scope each search to the directories that own the behavior instead of the repository root, and exclude build output such as `target/`, `node_modules/`, and `.venv/`. Use `sed -n ',p'` for focused reads. Avoid broad recursive output, unnecessary whole-file dumps, and redundant exploratory calls. Use `apply_patch` for edits, keep hunks localized, and include only the smallest unique context needed. Permission, phase, role, and path scope are runtime admission decisions; do not invent tools or request broader capability than the exact action needs. + +A sandboxed action starts with no network access and no access beyond what trusted global configuration already granted. Decide what a command needs before running it rather than after it fails: a command that authenticates, installs, downloads, publishes, or otherwise reaches a remote service needs network requested in the same `run_process` call, while a command that only touches the workspace and the configured local baseline needs no additional capability. Paths and host integrations enabled by trusted global configuration are already available to sandboxed commands. If a process command needs access that is still missing, such as network, a reviewed path, or an unconfigured host integration, include all required capabilities in that same `run_process` call under `permissions`; Merry reviews them before execution and runs the exact command through the permissioned backend when approved. Use `reason` to explain the minimum required access. Use `request_permissions` only when the needed capability is discovered after a failed sandboxed attempt or when the action is not being retried through `run_process`. Treat the sandbox as the first explanation when a process command fails: a capability it withheld often surfaces as a credentials, authentication, or connectivity error, so re-run the same action with the missing capability before concluding anything about the user's machine, account, or local setup. + +When a tool fails, preserve the failure evidence, determine whether the cause is validation, missing permission, unavailable capability, or an implementation error, and then either make a bounded recovery attempt or report the blocker. Do not repeat an identical failed action without new evidence or an explicit reviewed admission. + +For delegated coding work, each child max_model_turns covers its full lifecycle and must be at least 2048. The configured maximum may be larger. Reaching the limit is recoverable when that maximum leaves room: inspect the child status, then spawn a replacement with the same plan_client_key and a larger budget so it can continue from the shared workspace. + +Before finishing, verify the requested behavior with the narrowest deterministic checks that prove it. The final report is an evidence-backed summary: state what changed or was answered, name the checks that actually ran and their outcomes, distinguish skipped or blocked checks, and call out remaining risks. Never claim an unrun check succeeded. +"#; diff --git a/crates/merry-coding/src/workspace.rs b/crates/merry-coding/src/workspace.rs index 7b8dd187..7d5e932f 100644 --- a/crates/merry-coding/src/workspace.rs +++ b/crates/merry-coding/src/workspace.rs @@ -20,9 +20,9 @@ use thiserror::Error; const PROJECT_CAPABILITY_CONTEXT_ID: &str = "project-capabilities"; const CODING_WORKSPACE_CAPABILITY_SUMMARY: &str = concat!( "Coding file capabilities:\n", - "- `read_text` reads a bounded one-based line range from a known UTF-8 text path. Omit the range only to use the small configured default; use multiple focused reads instead of requesting a whole file. A path is relative to the workspace root or absolute, every spelling including dot-prefixed components is accepted, and a relative path also resolves under the read-only skill/resource roots. An absolute path may name a file outside the workspace, where the sandbox decides what is reachable.\n", + "- `read_text` reads UTF-8 text within the configured line and byte limits. A path is relative to the workspace root or absolute, every spelling including dot-prefixed components is accepted, and a relative path also resolves under the read-only skill/resource roots. An absolute path may name a file outside the workspace, where the sandbox decides what is reachable.\n", "- `apply_patch` is the only file-edit tool: one `*** Begin Patch` envelope with `*** Add File:`, `*** Update File:`, or `*** Delete File:` sections, at most one Add or Delete section per file (repeated `*** Update File:` sections for one file merge into a single change), and localized hunks instead of whole-file content. Every hunk must match the current file bytes exactly and uniquely; when one does not, the failure names the closest line and the first difference, so re-read that line before retrying. Runtime admission, write scope, forbidden paths, and current-file preimages are enforced before writes. A section path is relative to the workspace root or absolute: an absolute path inside the workspace root and the matching relative path address the same file, and an absolute path may name a file outside the workspace, where the sandbox decides what is reachable and writable.\n", - "- `run_process` is the discovery and verification lane when configured. Prefer the modern search tools the environment facts below report: `rg --files` to list files, a focused literal `rg` search for content, and `fd`/`fdfind` for paths by name; fall back to `grep -r` and `find` only for a tool the facts say is missing. Scope each search to the directories that own the behavior instead of the repository root, and exclude build output such as `target/`, `node_modules/`, and `.venv/`. Read files with bounded `sed -n ',p'`. Avoid broad recursive output, `cat` on large files, and repeated exploratory calls.\n", + "- `run_process` is the discovery and verification lane when configured. The environment facts below report available search tools.\n", "- Process execution runs through Merry runtime policy and the configured sandbox/profile, so filesystem and network access may be intentionally restricted; environment and host IPC access may also be intentionally restricted. Network is withheld from every action that does not request it, so a command that reaches a remote service needs `network: true` in the same call's `permissions`. Paths and host integrations enabled by trusted global configuration are already available to actions. If a command needs access that is still missing, such as network, a reviewed path, or an unconfigured endpoint, put all minimum required capabilities in that same `run_process` call under `permissions`; runtime reviews before executing the exact command through the permissioned backend.\n", "- If a capability is discovered only after a sandboxed failure, call `request_permissions` for that exact action before retrying it. Approved paths and host integrations remain available for later actions in this runtime session; network access must be requested again for every action that needs it.\n", "- Linux Unix sockets are filesystem paths. If a host resource is not represented by a named integration, request its exact socket/file path through `permissions.paths` or `requested.paths`; the outer sandbox must already expose the path.\n", diff --git a/crates/merry-py/src/builder.rs b/crates/merry-py/src/builder.rs index c390b008..15d658ea 100644 --- a/crates/merry-py/src/builder.rs +++ b/crates/merry-py/src/builder.rs @@ -89,22 +89,23 @@ impl PyAgentBuilder { enable_patch: bool, patch_write_scope: Option>, forbidden_paths: Vec, - max_read_bytes: usize, - max_read_lines: usize, - max_write_bytes: usize, - max_patch_bytes: usize, + max_read_bytes: Option, + max_read_lines: Option, + max_write_bytes: Option, + max_patch_bytes: Option, ) -> PyResult<()> { if root.is_empty() { return Err(error::config_message_to_py("workspace requires a root")); } + let defaults = WorkspaceToolLimits::default(); let mut profile_builder = merry::profiles::CodingAgentProfileBuilder::new(PathBuf::from(root)) .readonly_resource_roots(readonly_resource_roots.into_iter().map(PathBuf::from)) .limits(WorkspaceToolLimits { - max_read_bytes, - max_read_lines, - max_write_bytes, - max_patch_bytes, + max_read_bytes: max_read_bytes.unwrap_or(defaults.max_read_bytes), + max_read_lines: max_read_lines.unwrap_or(defaults.max_read_lines), + max_write_bytes: max_write_bytes.unwrap_or(defaults.max_write_bytes), + max_patch_bytes: max_patch_bytes.unwrap_or(defaults.max_patch_bytes), }) .forbidden_paths(forbidden_paths.into_iter().map(PathBuf::from)); if enable_patch { diff --git a/crates/merry-tools/src/config.rs b/crates/merry-tools/src/config.rs index d64dac97..14648e62 100644 --- a/crates/merry-tools/src/config.rs +++ b/crates/merry-tools/src/config.rs @@ -10,7 +10,8 @@ use thiserror::Error; pub struct WorkspaceToolLimits { /// Maximum bytes returned or scanned by one `read_text` request. pub max_read_bytes: usize, - /// Maximum lines returned by one `read_text` request. + /// Maximum lines returned by one `read_text` request, also used when omitted. + /// Defaults to 2,000 lines; the independent byte limit still applies. pub max_read_lines: usize, /// Maximum bytes written to one file by `apply_patch`. pub max_write_bytes: usize, @@ -22,7 +23,7 @@ impl Default for WorkspaceToolLimits { fn default() -> Self { Self { max_read_bytes: 1024 * 1024, - max_read_lines: 200, + max_read_lines: 2_000, max_write_bytes: 1024 * 1024, max_patch_bytes: 128 * 1024, } diff --git a/crates/merry-tools/src/read/input.rs b/crates/merry-tools/src/read/input.rs index 27c817d7..e286afc1 100644 --- a/crates/merry-tools/src/read/input.rs +++ b/crates/merry-tools/src/read/input.rs @@ -7,7 +7,7 @@ use serde::Deserialize; #[merry_tools_macros::tool( crate = "crate", name = "read_text", - description = "Read a bounded one-based line range from a UTF-8 text file. Omit start_line to begin at line 1 and omit max_lines to use the configured limit. Use multiple focused reads for larger files; do not request or assume complete-file content." + description = "Read UTF-8 text using one-based lines. Omit start_line to begin at line 1 and omit max_lines to use the configured per-call limit. A successful result contains the complete returned start_line..end_line range; truncated means more lines follow. Continue at end_line + 1 to read the next range, within the configured limits." )] #[derive(Debug, Deserialize, JsonSchema)] #[serde(deny_unknown_fields)] diff --git a/crates/merry-tools/src/tests/mod.rs b/crates/merry-tools/src/tests/mod.rs index 6a918f0c..e44ccc11 100644 --- a/crates/merry-tools/src/tests/mod.rs +++ b/crates/merry-tools/src/tests/mod.rs @@ -186,6 +186,7 @@ fn workspace_tool_schemas_describe_bounded_file_operations() { assert_eq!(read_schema["properties"]["path"]["minLength"], 1); assert_eq!(read_schema["properties"]["start_line"]["minimum"], 1); assert_eq!(read_schema["properties"]["max_lines"]["minimum"], 1); + assert_eq!(read_schema["properties"]["max_lines"]["maximum"], 2_000); assert_eq!(patch_schema["properties"]["patch"]["minLength"], 1); } diff --git a/crates/merry-tools/src/tests/read.rs b/crates/merry-tools/src/tests/read.rs index 633ce386..76d56ef7 100644 --- a/crates/merry-tools/src/tests/read.rs +++ b/crates/merry-tools/src/tests/read.rs @@ -31,10 +31,34 @@ fn read_text_returns_requested_line_window_without_host_root() { ); } +#[test] +fn read_text_defaults_to_complete_file_when_it_fits_line_budget() { + let temp = TempWorkspace::new("read-complete-file"); + let tools = tools_for(temp.path()); + + for line_count in [559, 2_000] { + let content = (1..=line_count) + .map(|line| format!("line-{line}\n")) + .collect::(); + temp.write_text("note.txt", &content); + + let outcome = read_outcome(&tools, "note.txt"); + let payload = json_content(&outcome); + + assert_eq!(outcome.status(), ToolCallResultStatus::Succeeded); + assert_eq!(payload["start_line"], 1); + assert_eq!(payload["end_line"], line_count); + assert_eq!(payload["lines"], line_count); + assert_eq!(payload["bytes"], content.len()); + assert_eq!(payload["content"], content); + assert_eq!(payload["truncated"], false); + } +} + #[test] fn read_text_defaults_to_a_bounded_first_window() { let temp = TempWorkspace::new("read-default-window"); - let content = (1..=205) + let content = (1..=2_005) .map(|line| format!("line-{line}\n")) .collect::(); temp.write_text("large.txt", &content); @@ -45,20 +69,20 @@ fn read_text_defaults_to_a_bounded_first_window() { assert_eq!(outcome.status(), ToolCallResultStatus::Succeeded); assert_eq!(payload["start_line"], 1); - assert_eq!(payload["lines"], 200); - assert_eq!(payload["end_line"], 200); + assert_eq!(payload["lines"], 2_000); + assert_eq!(payload["end_line"], 2_000); assert_eq!(payload["truncated"], true); assert!( payload["content"] .as_str() .expect("content text") - .contains("line-200\n") + .contains("line-2000\n") ); assert!( !payload["content"] .as_str() .expect("content text") - .contains("line-201\n") + .contains("line-2001\n") ); } diff --git a/examples/config.toml b/examples/config.toml index a2ef561f..791a1835 100644 --- a/examples/config.toml +++ b/examples/config.toml @@ -205,10 +205,13 @@ max_model_turns = 2048 # tools = ["resolve-library-id", "get-library-docs"] [tui] -# Show up to five output lines for successful commands. Failed commands always -# show their output preview; an extra ... line indicates omitted output. -# Ctrl+T inspects captured output independently of this default. In the viewer, -# Left/Right selects a command, C copies its command, Y copies captured output, +# Show up to five output lines for successful process commands and read_text calls. +# Timeline previews retain at most 180 Unicode characters per line; captured +# process output remains available in full through Ctrl+T. +# Failed commands always show their output preview; failed reads show diagnostics. +# An extra ... line indicates omitted output. +# Ctrl+T inspects captured process output independently of this default. In the +# viewer, Left/Right selects a command, C copies its command, Y copies captured output, # and Esc closes. Copy uses OSC 52 and requires terminal clipboard support. show_successful_command_output = false diff --git a/sdks/python/README.md b/sdks/python/README.md index 04e1dc17..e5393ab3 100644 --- a/sdks/python/README.md +++ b/sdks/python/README.md @@ -71,7 +71,13 @@ agent = ( `WorkspaceConfig` maps to the Rust coding profile. A workspace has one root, which may be absolute or relative; patch and forbidden paths are root-relative normalized paths. Every workspace limit is positive and is enforced again by -Rust. +Rust. `WorkspaceLimits` fields default to `None`, meaning the native builder uses +Rust's current defaults rather than a separate set of Python defaults. Set only +the fields you want to override; other fields continue to inherit Rust defaults. +Currently, `read_text` returns up to 2,000 lines per call, subject to the independent +1 MiB scan/return limit. `WorkspaceLimits(max_read_lines=...)` overrides both the +default read window and the maximum accepted `max_lines` argument. The Python +configuration retains `None` for inherited fields; it is not a resolved snapshot. Anthropic Messages uses the same builder: diff --git a/sdks/python/merry/_config.py b/sdks/python/merry/_config.py index ce616ef6..a0ff90ea 100644 --- a/sdks/python/merry/_config.py +++ b/sdks/python/merry/_config.py @@ -83,12 +83,12 @@ def __repr__(self) -> str: @dataclass(frozen=True, slots=True) class WorkspaceLimits: - """Positive bounds applied by Rust-owned workspace tools.""" + """Positive workspace bound overrides; None inherits the Rust default.""" - max_read_bytes: int = 1024 * 1024 - max_read_lines: int = 200 - max_write_bytes: int = 1024 * 1024 - max_patch_bytes: int = 128 * 1024 + max_read_bytes: int | None = None + max_read_lines: int | None = None + max_write_bytes: int | None = None + max_patch_bytes: int | None = None def __post_init__(self) -> None: for name, value in ( @@ -97,7 +97,8 @@ def __post_init__(self) -> None: ("max_write_bytes", self.max_write_bytes), ("max_patch_bytes", self.max_patch_bytes), ): - require_positive_int(name, value) + if value is not None: + require_positive_int(name, value) @dataclass(frozen=True, slots=True, init=False) diff --git a/sdks/python/merry/_merry.pyi b/sdks/python/merry/_merry.pyi index bdc17549..e91881e3 100644 --- a/sdks/python/merry/_merry.pyi +++ b/sdks/python/merry/_merry.pyi @@ -22,10 +22,10 @@ class AgentBuilder: enable_patch: bool, patch_write_scope: list[str] | None, forbidden_paths: list[str], - max_read_bytes: int, - max_read_lines: int, - max_write_bytes: int, - max_patch_bytes: int, + max_read_bytes: int | None, + max_read_lines: int | None, + max_write_bytes: int | None, + max_patch_bytes: int | None, ) -> None: ... def register_bridge_tool( self, name: str, description: str, schema_json: str diff --git a/sdks/python/tests/test_builder.py b/sdks/python/tests/test_builder.py index c631b66d..602f54d0 100644 --- a/sdks/python/tests/test_builder.py +++ b/sdks/python/tests/test_builder.py @@ -121,6 +121,33 @@ def test_workspace_and_patch_configuration_are_explicit(tmp_path: Path) -> None: assert workspace.limits.max_read_bytes == 2048 +def test_workspace_limits_defer_defaults_to_rust_and_allow_partial_overrides( + tmp_path: Path, +) -> None: + workspace = merry.WorkspaceConfig(root=tmp_path) + custom = merry.WorkspaceConfig( + root=tmp_path, + limits=merry.WorkspaceLimits(max_read_lines=17), + ) + + assert workspace.limits == merry.WorkspaceLimits() + assert workspace.limits.max_read_lines is None + assert workspace.limits.max_read_bytes is None + assert workspace.limits.max_write_bytes is None + assert workspace.limits.max_patch_bytes is None + assert custom.limits.max_read_lines == 17 + assert custom.limits.max_read_bytes is None + + for config in (workspace, custom): + agent = ( + merry.AgentBuilder("workspace-defaults") + .provider(openai_provider()) + .workspace(config) + .build() + ) + assert agent.session_id == "workspace-defaults" + + def test_invalid_workspace_and_limits_fail_before_native_build(tmp_path: Path) -> None: with pytest.raises(ValueError, match="write_scope"): merry.PatchConfig(write_scope=[]) diff --git a/sdks/python/tests/test_workspace_limits.py b/sdks/python/tests/test_workspace_limits.py new file mode 100644 index 00000000..bc972ed6 --- /dev/null +++ b/sdks/python/tests/test_workspace_limits.py @@ -0,0 +1,321 @@ +from __future__ import annotations + +import asyncio +import json +from collections.abc import Callable, Iterator +from contextlib import contextmanager +from http.server import BaseHTTPRequestHandler, HTTPServer +from pathlib import Path +from queue import Queue +from threading import Thread +from typing import Literal + +import pytest +from pydantic import BaseModel + +import merry + + +class SchemaProperty(BaseModel): + maximum: int | None = None + + +class ToolSchema(BaseModel): + properties: dict[str, SchemaProperty] + + +class ToolFunction(BaseModel): + name: str + parameters: ToolSchema + + +class RequestTool(BaseModel): + function: ToolFunction + + +class ModelRequest(BaseModel): + tools: list[RequestTool] + + +class ReadArguments(BaseModel): + path: str + max_lines: int | None = None + + +class PatchArguments(BaseModel): + patch: str + + +class ReadOutput(BaseModel): + content: str + lines: int + truncated: bool + + +@contextmanager +def capture_model_request( + tool_response: bytes | None = None, +) -> Iterator[tuple[str, Queue[bytes]]]: + final_response = ( + b'data: {"choices":[{"index":0,"delta":{"content":"done"},' + b'"finish_reason":"stop"}]}\n\n' + b"data: [DONE]\n\n" + ) + responses: Queue[bytes] = Queue(maxsize=2) + if tool_response is not None: + responses.put_nowait(tool_response) + responses.put_nowait(final_response) + requests: Queue[bytes] = Queue(maxsize=responses.qsize()) + + class Handler(BaseHTTPRequestHandler): + timeout = 5 + + def do_POST(self) -> None: + content_length = int(self.headers["Content-Length"]) + requests.put_nowait(self.rfile.read(content_length)) + response = responses.get_nowait() + self.send_response(200) + self.send_header("Content-Type", "text/event-stream") + self.send_header("Content-Length", str(len(response))) + self.end_headers() + self.wfile.write(response) + + def log_message(self, format: str, *args: str | int) -> None: + pass + + with HTTPServer(("127.0.0.1", 0), Handler) as server: + worker = Thread( + target=server.serve_forever, + kwargs={"poll_interval": 0.05}, + name="workspace-limits-provider", + ) + worker.start() + try: + yield f"http://127.0.0.1:{server.server_port}/v1", requests + finally: + server.shutdown() + worker.join(timeout=10) + if worker.is_alive(): + raise RuntimeError("workspace limit fixture did not stop") + + +@pytest.mark.parametrize( + ("limits", "expected_max_lines"), + [ + (merry.WorkspaceLimits(), 2_000), + (merry.WorkspaceLimits(max_read_lines=17), 17), + (merry.WorkspaceLimits(max_read_bytes=2048), 2_000), + ], +) +def test_workspace_limits_reach_rust_tool_schema( + tmp_path: Path, + limits: merry.WorkspaceLimits, + expected_max_lines: int, +) -> None: + with capture_model_request() as (base_url, requests): + agent = ( + merry.AgentBuilder("workspace-limit-contract") + .provider( + merry.OpenAICompatible( + api_key="test-key", + model="test-model", + base_url=base_url, + protocol="chat_completions", + ) + ) + .workspace(merry.WorkspaceConfig(root=tmp_path, limits=limits)) + .build() + ) + + async def run() -> None: + await asyncio.wait_for(agent.run("Reply with done."), timeout=5) + + asyncio.run(run()) + request = ModelRequest.model_validate_json(requests.get(timeout=1)) + + read_tool = next( + tool.function for tool in request.tools if tool.function.name == "read_text" + ) + assert read_tool.parameters.properties["max_lines"].maximum == expected_max_lines + + +def run_workspace_tool( + root: Path, + limits: merry.WorkspaceLimits, + name: Literal["read_text", "apply_patch"], + arguments: ReadArguments | PatchArguments, +) -> merry.ToolCallFinishedPayload: + chunk = json.dumps( + { + "choices": [ + { + "index": 0, + "delta": { + "tool_calls": [ + { + "index": 0, + "id": "workspace-tool-call", + "type": "function", + "function": { + "name": name, + "arguments": arguments.model_dump_json( + exclude_none=True + ), + }, + } + ] + }, + "finish_reason": "tool_calls", + } + ] + } + ) + response = f"data: {chunk}\n\ndata: [DONE]\n\n".encode() + with capture_model_request(response) as (base_url, requests): + agent = ( + merry.AgentBuilder("workspace-tool-limits") + .provider( + merry.OpenAICompatible( + api_key="test-key", + model="test-model", + base_url=base_url, + protocol="chat_completions", + ) + ) + .workspace( + merry.WorkspaceConfig( + root=root, + limits=limits, + patch=merry.PatchConfig(write_scope=["note.txt"]), + ) + ) + .build() + ) + + async def run() -> tuple[merry.Event, ...]: + result = await asyncio.wait_for(agent.run("Execute the tool."), timeout=5) + assert result.status is merry.RunStatus.COMPLETED + return result.events + + events = asyncio.run(run()) + assert requests.qsize() == 2 + + finished = [ + event.payload + for event in events + if isinstance(event.payload, merry.ToolCallFinishedPayload) + ] + assert len(finished) == 1 + return finished[0] + + +@pytest.mark.parametrize( + ("limits", "expected_error"), + [ + (merry.WorkspaceLimits(), None), + (merry.WorkspaceLimits(max_read_bytes=3), "workspace_file_too_large"), + (merry.WorkspaceLimits(max_read_bytes=4), None), + (merry.WorkspaceLimits(max_write_bytes=1, max_patch_bytes=1), None), + ], +) +def test_read_byte_limits_are_enforced_without_changing_other_limits( + tmp_path: Path, limits: merry.WorkspaceLimits, expected_error: str | None +) -> None: + path = tmp_path / "note.txt" + path.write_text("界\n", encoding="utf-8") + finished = run_workspace_tool( + tmp_path, limits, "read_text", ReadArguments(path="note.txt") + ) + + assert_tool_status(finished, expected_error) + if expected_error is None: + assert finished.output is not None + output = ReadOutput.model_validate_json(finished.output.value) + assert output.content == "界\n" + assert output.lines == 1 + assert not output.truncated + assert path.read_text(encoding="utf-8") == "界\n" + + +@pytest.mark.parametrize("requested_lines", [None, 2, 3]) +def test_read_line_override_controls_default_window_and_explicit_limit( + tmp_path: Path, requested_lines: int | None +) -> None: + (tmp_path / "note.txt").write_text("one\ntwo\nthree\n", encoding="utf-8") + finished = run_workspace_tool( + tmp_path, + merry.WorkspaceLimits(max_read_lines=2), + "read_text", + ReadArguments(path="note.txt", max_lines=requested_lines), + ) + + expected_error = "tool_input_schema_invalid" if requested_lines == 3 else None + assert_tool_status(finished, expected_error) + if expected_error is None: + assert finished.output is not None + output = ReadOutput.model_validate_json(finished.output.value) + assert output.content == "one\ntwo\n" + assert output.lines == 2 + assert output.truncated + + +PATCH = ( + "*** Begin Patch\n*** Update File: note.txt\n@@\n-old\n+changed\n*** End Patch\n" +) + + +@pytest.mark.parametrize( + ("limits", "expected_error"), + [ + (merry.WorkspaceLimits(), None), + (merry.WorkspaceLimits(max_write_bytes=7), "workspace_file_too_large"), + (merry.WorkspaceLimits(max_write_bytes=8), None), + ( + merry.WorkspaceLimits(max_patch_bytes=len(PATCH.encode()) - 1), + "tool_input_schema_invalid", + ), + (merry.WorkspaceLimits(max_patch_bytes=len(PATCH.encode())), None), + (merry.WorkspaceLimits(max_read_bytes=4, max_read_lines=1), None), + ], +) +def test_patch_limits_are_enforced_before_writes( + tmp_path: Path, limits: merry.WorkspaceLimits, expected_error: str | None +) -> None: + path = tmp_path / "note.txt" + path.write_text("old\n", encoding="utf-8") + finished = run_workspace_tool( + tmp_path, limits, "apply_patch", PatchArguments(patch=PATCH) + ) + + assert_tool_status(finished, expected_error) + expected_content = "changed\n" if expected_error is None else "old\n" + assert path.read_text(encoding="utf-8") == expected_content + + +@pytest.mark.parametrize( + "make_limits", + [ + lambda value: merry.WorkspaceLimits(max_read_bytes=value), + lambda value: merry.WorkspaceLimits(max_read_lines=value), + lambda value: merry.WorkspaceLimits(max_write_bytes=value), + lambda value: merry.WorkspaceLimits(max_patch_bytes=value), + ], +) +@pytest.mark.parametrize("invalid_value", [0, -1]) +def test_invalid_limits_are_rejected_instead_of_inheriting_defaults( + make_limits: Callable[[int], merry.WorkspaceLimits], invalid_value: int +) -> None: + with pytest.raises(ValueError, match="must be greater than zero"): + make_limits(invalid_value) + + +def assert_tool_status( + finished: merry.ToolCallFinishedPayload, expected_error: str | None +) -> None: + if expected_error is None: + assert finished.result.status is merry.RuntimeToolResultStatus.SUCCEEDED + assert finished.result.diagnostic is None + else: + assert finished.result.status is merry.RuntimeToolResultStatus.FAILED + assert finished.result.diagnostic is not None + assert finished.result.diagnostic.code == expected_error