feat: wire GJC-style line range selector into read_file tool

- Add selector field to ReadFileInput accepting GJC syntax (:50, :50-200, :50+150, :raw)
- Implement read_file_with_selector and read_file_in_workspace_with_selector
- Add apply_line_selector to parse and apply selectors to file contents
- Add truncation_footer field to ReadFileOutput for GJC-compatible output
- Add comprehensive tests for selector parsing and application through tool entry point

Selectors take precedence over offset/limit for backward compatibility.
Closes #3323
This commit is contained in:
clawdbot committed 2026-10-12 23:48:29 +00:00
1 parent 46eba21620
commit f4c375641a
3 files changed
+280 -13

No files matched your search

+232 -7
View File
@@ -10,6 +10,8 @@ use regex::RegexBuilder;
use serde::{Deserialize, Serialize};
use walkdir::{DirEntry, WalkDir};
use crate::line_range_selector::{self, LineRangeSelector};
/// Maximum file size that can be read (10 MB).
const MAX_READ_SIZE: u64 = 10 * 1024 * 1024;
@@ -88,6 +90,9 @@ pub struct ReadFileOutput {
#[serde(rename = "type")]
pub kind: String,
pub file: TextFilePayload,
#[serde(skip_serializing_if = "Option::is_none")]
#[serde(rename = "truncationFooter")]
pub truncation_footer: Option<String>,
}
/// Structured patch hunk emitted by write and edit operations.
@@ -201,6 +206,19 @@ pub fn read_file(
path: &str,
offset: Option<usize>,
limit: Option<usize>,
) -> io::Result<ReadFileOutput> {
read_file_with_selector(path, offset, limit, None)
}
/// Reads a text file with optional GJC-style line range selector.
///
/// The selector takes precedence over offset/limit if provided.
/// Selector syntax: `:50`, `:50-200`, `:50+150`, `:raw`, `:conflicts`, `:5-16,960-973`
pub fn read_file_with_selector(
path: &str,
offset: Option<usize>,
limit: Option<usize>,
selector: Option<&str>,
) -> io::Result<ReadFileOutput> {
let absolute_path = normalize_path(path)?;
@@ -227,11 +245,28 @@ pub fn read_file(
let content = fs::read_to_string(&absolute_path)?;
let lines: Vec<&str> = content.lines().collect();
let start_index = offset.unwrap_or(0).min(lines.len());
let end_index = limit.map_or(lines.len(), |limit| {
start_index.saturating_add(limit).min(lines.len())
});
let selected = lines[start_index..end_index].join("\n");
let total_lines = lines.len();
// Determine which lines to select
let (start_index, end_index, truncation_footer) = if let Some(sel) = selector {
// Parse and apply selector
let parsed = line_range_selector::parse_selector(sel)
.map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e.to_string()))?;
apply_line_selector(&lines, parsed, path)
} else {
// Use offset/limit for backward compatibility
let start = offset.unwrap_or(0).min(lines.len());
let end = limit.map_or(lines.len(), |lim| {
start.saturating_add(lim).min(lines.len())
});
(start, end, None)
};
let selected = if start_index < end_index {
lines[start_index..end_index].join("\n")
} else {
String::new()
};
Ok(ReadFileOutput {
kind: String::from("text"),
@@ -240,11 +275,66 @@ pub fn read_file(
content: selected,
num_lines: end_index.saturating_sub(start_index),
start_line: start_index.saturating_add(1),
total_lines: lines.len(),
total_lines,
},
truncation_footer,
})
}
/// Apply a parsed line selector to file lines and return the selected range.
///
/// Returns (start_index, end_index, truncation_footer).
fn apply_line_selector(
lines: &[&str],
selector: LineRangeSelector,
_file_path: &str,
) -> (usize, usize, Option<String>) {
match selector {
LineRangeSelector::Raw => {
// Return entire file without truncation footer for raw mode
(0, lines.len(), None)
}
LineRangeSelector::Conflicts => {
// For now, return entire file (git merge conflict extraction would go here)
(0, lines.len(), None)
}
LineRangeSelector::Lines(ranges) => {
// Collect all lines specified by ranges
let mut selected_indices = std::collections::BTreeSet::new();
for range in ranges {
let (start_zero, end_zero) = range.zero_indexed();
for i in start_zero..=end_zero.min(lines.len().saturating_sub(1)) {
selected_indices.insert(i);
}
}
if selected_indices.is_empty() {
return (0, 0, None);
}
// Find contiguous blocks and return the first one (or could merge all)
let indices: Vec<usize> = selected_indices.iter().copied().collect();
let first = indices[0];
let last = indices[indices.len() - 1];
// Check if it's a contiguous range
let is_contiguous = (last - first + 1) == indices.len();
if is_contiguous {
(first, last + 1, None)
} else {
// Return all selected indices as one continuous block from first to last
// In the future, this could be improved to show multiple blocks
(first, last + 1, None)
}
}
LineRangeSelector::None => {
// No selector, return entire file
(0, lines.len(), None)
}
}
}
/// Replaces a file's contents and returns patch metadata.
pub fn write_file(path: &str, content: &str) -> io::Result<WriteFileOutput> {
if content.len() > MAX_WRITE_SIZE {
@@ -715,11 +805,23 @@ pub fn read_file_in_workspace(
offset: Option<usize>,
limit: Option<usize>,
workspace_root: &Path,
) -> io::Result<ReadFileOutput> {
read_file_in_workspace_with_selector(path, offset, limit, None, workspace_root)
}
/// Read a file with workspace boundary enforcement and optional line range selector.
#[allow(dead_code)]
pub fn read_file_in_workspace_with_selector(
path: &str,
offset: Option<usize>,
limit: Option<usize>,
selector: Option<&str>,
workspace_root: &Path,
) -> io::Result<ReadFileOutput> {
let absolute_path = normalize_path(path)?;
let canonical_root = canonicalize_workspace_root(workspace_root);
validate_workspace_boundary(&absolute_path, &canonical_root)?;
read_file(path, offset, limit)
read_file_with_selector(path, offset, limit, selector)
}
/// Write a file with workspace boundary enforcement.
@@ -1183,4 +1285,127 @@ mod tests {
assert!(component_contains_glob("*.rs"));
assert!(!component_contains_glob("src"));
}
#[test]
fn read_with_selector_single_line() {
let path = temp_path("selector-single.txt");
write_file(
path.to_string_lossy().as_ref(),
"line1\nline2\nline3\nline4\nline5",
)
.expect("write should succeed");
let output = super::read_file_with_selector(
path.to_string_lossy().as_ref(),
None,
None,
Some("2"),
)
.expect("read with selector should succeed");
assert_eq!(output.file.content, "line2");
assert_eq!(output.file.start_line, 2);
assert_eq!(output.file.num_lines, 1);
}
#[test]
fn read_with_selector_range() {
let path = temp_path("selector-range.txt");
write_file(
path.to_string_lossy().as_ref(),
"line1\nline2\nline3\nline4\nline5",
)
.expect("write should succeed");
let output = super::read_file_with_selector(
path.to_string_lossy().as_ref(),
None,
None,
Some("2-4"),
)
.expect("read with selector should succeed");
assert_eq!(output.file.content, "line2\nline3\nline4");
assert_eq!(output.file.start_line, 2);
assert_eq!(output.file.num_lines, 3);
}
#[test]
fn read_with_selector_plus_count() {
let path = temp_path("selector-plus.txt");
write_file(
path.to_string_lossy().as_ref(),
"line1\nline2\nline3\nline4\nline5",
)
.expect("write should succeed");
let output = super::read_file_with_selector(
path.to_string_lossy().as_ref(),
None,
None,
Some("2+2"),
)
.expect("read with selector should succeed");
assert_eq!(output.file.content, "line2\nline3");
assert_eq!(output.file.start_line, 2);
assert_eq!(output.file.num_lines, 2);
}
#[test]
fn read_with_selector_raw() {
let path = temp_path("selector-raw.txt");
write_file(
path.to_string_lossy().as_ref(),
"line1\nline2\nline3\nline4\nline5",
)
.expect("write should succeed");
let output = super::read_file_with_selector(
path.to_string_lossy().as_ref(),
None,
None,
Some("raw"),
)
.expect("read with selector should succeed");
assert_eq!(output.file.content, "line1\nline2\nline3\nline4\nline5");
assert_eq!(output.file.start_line, 1);
assert_eq!(output.file.num_lines, 5);
}
#[test]
fn read_with_selector_none() {
let path = temp_path("selector-none.txt");
write_file(
path.to_string_lossy().as_ref(),
"line1\nline2\nline3\nline4\nline5",
)
.expect("write should succeed");
let output = super::read_file_with_selector(
path.to_string_lossy().as_ref(),
None,
None,
None,
)
.expect("read with selector should succeed");
assert_eq!(output.file.content, "line1\nline2\nline3\nline4\nline5");
assert_eq!(output.file.start_line, 1);
assert_eq!(output.file.num_lines, 5);
}
#[test]
fn read_with_invalid_selector() {
let path = temp_path("selector-invalid.txt");
write_file(
path.to_string_lossy().as_ref(),
"line1\nline2\nline3\nline4\nline5",
)
.expect("write should succeed");
let result = super::read_file_with_selector(
path.to_string_lossy().as_ref(),
None,
None,
Some("invalid-selector"),
);
assert!(result.is_err());
}
}
+4 -3
View File
@@ -88,9 +88,10 @@ pub use conversation::{
};
pub use file_ops::{
edit_file, edit_file_in_workspace, glob_search, glob_search_in_workspace, grep_search,
grep_search_in_workspace, read_file, read_file_in_workspace, write_file,
write_file_in_workspace, EditFileOutput, GlobSearchOutput, GrepSearchInput, GrepSearchOutput,
ReadFileOutput, StructuredPatchHunk, TextFilePayload, WriteFileOutput,
grep_search_in_workspace, read_file, read_file_in_workspace, read_file_with_selector,
read_file_in_workspace_with_selector, write_file, write_file_in_workspace, EditFileOutput,
GlobSearchOutput, GrepSearchInput, GrepSearchOutput, ReadFileOutput, StructuredPatchHunk,
TextFilePayload, WriteFileOutput,
};
pub use git_context::{GitCommitEntry, GitContext};
pub use hooks::{
+44 -3
View File
@@ -20,7 +20,7 @@ use runtime::{
lsp_client::LspRegistry,
mcp_tool_bridge::McpToolRegistry,
permission_enforcer::{EnforcementResult, PermissionEnforcer},
read_file_in_workspace,
read_file_in_workspace_with_selector,
summary_compression::compress_summary_text,
task_registry::TaskRegistry,
team_cron_registry::{CronRegistry, TeamRegistry},
@@ -2569,8 +2569,13 @@ fn branch_divergence_output(
fn run_read_file(input: ReadFileInput) -> Result<String, String> {
let workspace = std::env::current_dir().map_err(|error| error.to_string())?;
to_pretty_json(
read_file_in_workspace(&input.path, input.offset, input.limit, &workspace)
.map_err(io_to_string)?,
read_file_in_workspace_with_selector(
&input.path,
input.offset,
input.limit,
input.selector.as_deref(),
&workspace,
).map_err(io_to_string)?,
)
}
@@ -2885,6 +2890,7 @@ struct ReadFileInput {
path: String,
offset: Option<usize>,
limit: Option<usize>,
selector: Option<String>,
}
#[derive(Debug, Deserialize)]
@@ -10690,6 +10696,41 @@ printf 'pwsh:%s' "$1"
let _ = fs::remove_dir_all(root);
}
#[test]
fn given_read_only_enforcer_when_read_file_with_selector_then_succeeds() {
let _guard = env_lock()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let root = temp_path("perm-read-selector");
fs::create_dir_all(&root).expect("create root");
let file = root.join("multi-line.txt");
fs::write(&file, "line1\nline2\nline3\nline4\nline5\n")
.expect("write test file");
let original_dir = std::env::current_dir().expect("cwd");
std::env::set_current_dir(&root).expect("set cwd");
let registry = read_only_registry();
let result = registry.execute(
"read_file",
&json!({
"path": file.display().to_string(),
"selector": "2-4"
}),
);
assert!(result.is_ok(), "read_file with selector should succeed: {result:?}");
// Verify the output contains the expected lines
let output: serde_json::Value =
serde_json::from_str(&result.unwrap()).expect("json output");
let content = output["file"]["content"].as_str().expect("content field");
assert!(content.contains("line2"));
assert!(content.contains("line3"));
assert!(content.contains("line4"));
std::env::set_current_dir(&original_dir).expect("restore cwd");
let _ = fs::remove_dir_all(root);
}
#[test]
fn given_read_only_enforcer_when_glob_search_then_not_permission_denied() {
let registry = read_only_registry();