diff --git a/docs/gjc-compat/tool-semantics-phase1.md b/docs/gjc-compat/tool-semantics-phase1.md new file mode 100644 index 00000000..4f4d03cf --- /dev/null +++ b/docs/gjc-compat/tool-semantics-phase1.md @@ -0,0 +1,337 @@ +# GJC Tool Semantics Alignment: Phase 1 (read/edit/search/find) + +**Status**: Implementation plan +**Issue**: #3323 +**Scope**: Slice 2.1 - Tool catalog semantic parity for read/edit/search/find +**Target**: Ship in batches with focused PRs + +--- + +## 1. EXECUTIVE SUMMARY + +This document details the semantic gaps between claw-code's current tool implementations and GJC's (gajae-code) reference implementations for the highest-value tools: **read**, **edit**, **search**, and **find**. + +### Key Gaps + +| Tool | Current (claw-code) | Target (GJC) | Gap | Complexity | +|------|-----|--------|-----|------------| +| **read** | offset/limit pagination | Line selectors (`:L-L+N`), raw mode, truncation footers | High | Medium | +| **write** | Simple file replacement | Full replacement semantics (matching GJC) | Low | Low | +| **edit** | String replacement (old→new) | String replacement with rich patch output | Low | Low | +| **find** (glob) | Pattern expansion only | Recursive glob with filtering | Low | Low | +| **search** (grep) | Regex search with context | Regex with context + advanced filtering | Low | Low | + +--- + +## 2. GJC TOOL SEMANTICS REFERENCE + +### 2.1 READ Tool + +**Source**: `gajae-code/packages/coding-agent/src/tools/read.ts` (3,500+ LOC) + +#### Input Schema +```typescript +{ + path: string; // File path (relative or absolute) + offset?: number; // Line offset (for pagination) + limit?: number; // Lines to read (for pagination) + // GJC extensions (missing in claw-code): + selector?: string; // Line range syntax: + // ":50" = line 50 onward + // ":50-200" = lines 50-200 inclusive + // ":50+150" = 150 lines starting at line 50 + // ":raw" = full file verbatim + // ":conflicts" = git merge conflicts only + truncationDirection?: "head" | "tail" | "both"; // Which end to keep + maxSummaryBytes?: number; // Summary budget (default 50KB) + maxSummaryLines?: number; // Summary budget (default 20K lines) +} +``` + +#### Output Structure +```json +{ + "file": { + "path": "src/main.rs", + "content": "...", + "totalLines": 500, + "totalBytes": 12345, + "truncatedBy": "lines" | "bytes" | "middle" | null, + "displayMode": "summarized" | "raw" | "receipt" + }, + "truncationFooter": "[Showing lines 1-50 of 500; re-read ...]" +} +``` + +#### Key Behaviors + +1. **Line Range Selectors** (`:selector` syntax) + - `:50` = start at line 50 + - `:50-200` = lines 50-200 inclusive + - `:50+150` = 150 lines starting at 50 + - `:50+1` = exactly one line + - `:5-16,960-973` = multiple ranges + - `:raw` = full verbatim + - `:conflicts` = git merge conflicts + +2. **Truncation Modes** + - Default: 50KB bytes limit, 20K lines limit + - Direction: `head` (keep start), `tail` (keep end), `both` (keep both ends) + - Footer includes recovery hint (e.g., "`re-read path:1-9999` for verbatim") + +3. **Structural Summarization** + - Code: collapse function bodies to `..` or `…` (merged brace pairs) + - Prose: preserve full lines, truncate at word boundaries + - Special files: documents (PDF, DOCX, XLSX) converted via markit + +4. **Archive Support** + - `.tar`, `.tar.gz`, `.tgz`, `.zip`: member read with `archive.ext:path/inside` + - SQLite: `.sqlite:table` or `.sqlite:table:key` queries + - Line ranges apply to archive members + +5. **Output Formatting** + - Anchored output: Line prefixes with source line numbers and hash anchors + - Optional hash lines: Marks structural boundaries (`# pragma`, function start, etc.) + - Prose column cap: Long lines wrapped/truncated at column limit + +### 2.2 EDIT Tool + +**Source**: `gajae-code/packages/coding-agent/src/tools/edit.ts` + +#### Input Schema +```typescript +{ + path: string; // File to edit + old_text: string; // Text to replace (required for edit) + new_text: string; // Replacement text + all?: boolean; // Replace all occurrences (default: first only) +} +``` + +#### Output Structure +```json +{ + "file": { + "path": "src/main.rs", + "editCount": 1, + "beforeSize": 12345, + "afterSize": 12400 + }, + "patch": [ + { + "type": "replace", + "lineStart": 50, + "lineEnd": 55, + "before": "old content", + "after": "new content", + "context": {"before": 3, "after": 3} + } + ] +} +``` + +#### Key Behaviors + +1. **Exact Match Required**: `old_text` must match exactly (no fuzzy matching) +2. **Smallest Replacement**: Use smallest unique old_text that identifies the change +3. **All-or-Nothing**: Either all occurrences replaced or none (no partial) +4. **Patch Metadata**: Line numbers, context, before/after preserved + +### 2.3 SEARCH Tool + +**Source**: `gajae-code/packages/coding-agent/src/tools/search.ts` + +#### Input Schema +```typescript +{ + pattern: string; // Regex pattern + paths?: string[]; // Files/dirs to search + context?: number; // Context lines before/after (default: 2) + i?: boolean; // Case-insensitive + glob_exclude?: string[]; // Patterns to skip + file_type?: string; // Filter by extension + limit?: number; // Max results + truncation?: "head" | "tail" | "both"; +} +``` + +#### Output Structure +```json +{ + "matches": [ + { + "file": "src/main.rs", + "lineNumber": 42, + "line": "fn process() {", + "before": ["// Process data", ""], + "after": [" // Implementation", ""] + } + ], + "summary": "Found 5 matches in 3 files" +} +``` + +### 2.4 FIND Tool + +**Source**: `gajae-code/packages/coding-agent/src/tools/find.ts` + +#### Input Schema +```typescript +{ + paths: string[]; // Glob patterns + timeout?: number; // Timeout in seconds (default: 5) + gitignore?: boolean; // Respect .gitignore (default: true) + hidden?: boolean; // Include hidden files (default: true) + limit?: number; // Max results (default: 1000) +} +``` + +#### Output Structure +```json +{ + "files": [ + { + "path": "src/main.rs", + "modified": "2026-10-13T12:34:56Z", + "size": 12345 + } + ], + "summary": "Found 42 files matching pattern", + "truncated": false +} +``` + +--- + +## 3. CURRENT CLAW-CODE IMPLEMENTATION GAPS + +### 3.1 READ Tool Gaps + +**File**: `rust/crates/runtime/src/file_ops.rs:713` + +**Current**: +```rust +pub fn read_file_in_workspace( + path: &str, + offset: Option, // Line offset only + limit: Option, // Lines only + workspace_root: &Path, +) -> io::Result +``` + +**Missing**: +- ✗ Line range selectors (`:L-L+N` syntax) +- ✗ Raw mode (`:raw`) +- ✗ Git merge conflict extraction (`:conflicts`) +- ✗ Truncation footers with recovery hints +- ✗ Structural summarization (collapse braces) +- ✗ Archive/SQLite support +- ✗ Multi-range selectors (`:5-16,960-973`) +- ✗ Prose column cap +- ✗ Hash line anchors + +**Estimated LOC to Add**: 1,200-1,500 (including tests) + +### 3.2 EDIT Tool Gaps + +**File**: `rust/crates/runtime/src/file_ops.rs:760` + +**Current**: Basic string replacement works +**Missing**: +- ✗ Rich patch metadata (line numbers, context lines) + +**Estimated LOC to Add**: 200-300 + +### 3.3 SEARCH Tool Gaps + +**File**: `rust/crates/tools/src/lib.rs` (grep search) + +**Current**: Basic regex with context +**Missing**: +- ✗ Advanced filtering (file type, glob exclude) +- ✗ Structured output format + +**Estimated LOC to Add**: 300-400 + +### 3.4 FIND Tool Gaps + +**File**: `rust/crates/tools/src/lib.rs` (glob search) + +**Current**: Basic glob expansion +**Missing**: +- ✗ Recursive globs +- ✗ Gitignore filtering +- ✗ Hidden file control +- ✗ File metadata + +**Estimated LOC to Add**: 200-300 + +--- + +## 4. IMPLEMENTATION PLAN (BATCHED) + +### Batch 1: READ Tool (Priority P0) +- Add line range selector parsing +- Implement `:raw` and `:conflicts` modes +- Add truncation footers with recovery hints +- Implement structural summarization +- Add tests for all selector combinations + +**Files to Modify**: +- `rust/crates/runtime/src/file_ops.rs` (read_file_in_workspace) +- `rust/crates/tools/src/lib.rs` (ReadFileInput struct) +- Tests in both + +**Size Estimate**: 1,200 LOC +**Complexity**: Medium (string parsing, output formatting) + +### Batch 2: EDIT Tool (Priority P1) +- Add patch metadata to output +- Ensure exact match semantics +- Add comprehensive tests + +**Files to Modify**: +- `rust/crates/runtime/src/file_ops.rs` (edit_file_in_workspace) +- Tests + +**Size Estimate**: 200 LOC +**Complexity**: Low + +### Batch 3: SEARCH/FIND Tools (Priority P1) +- Enhance grep/find with GJC-compatible filtering +- Add structured output format +- Add metadata (file type, modified time) + +**Files to Modify**: +- `rust/crates/tools/src/lib.rs` (glob_search, grep_search) +- `rust/crates/runtime/src/file_ops.rs` + +**Size Estimate**: 500 LOC +**Complexity**: Low-Medium + +--- + +## 5. VERIFICATION STRATEGY + +Each batch will include: +1. **Semantic Tests**: Verify tool inputs/outputs match GJC reference +2. **Edge Case Tests**: Line ranges, truncation, archives, etc. +3. **Integration Tests**: End-to-end with claw CLI +4. **Parity Shadow Test**: Run same inputs against both GJC and claw-code, compare outputs + +--- + +## 6. ROLLOUT SCHEDULE + +- **Batch 1 (READ)**: Week 1 - Enable most impactful tool first +- **Batch 2 (EDIT)**: Week 1-2 - Quick follow-up +- **Batch 3 (SEARCH/FIND)**: Week 2 - Lower priority but valuable for discoverability + +--- + +## 7. REFERENCES + +- GJC Read Tool: `/home/bellman/Workspace/gajae-code/packages/coding-agent/src/tools/read.ts` +- claw-code file_ops: `rust/crates/runtime/src/file_ops.rs` +- claw-code tools: `rust/crates/tools/src/lib.rs` +- Gap Matrix: `docs/gjc-compat/gap-matrix.md` diff --git a/rust/crates/runtime/src/file_ops.rs b/rust/crates/runtime/src/file_ops.rs index 7a62447a..696816cc 100644 --- a/rust/crates/runtime/src/file_ops.rs +++ b/rust/crates/runtime/src/file_ops.rs @@ -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, } /// Structured patch hunk emitted by write and edit operations. @@ -201,6 +206,19 @@ pub fn read_file( path: &str, offset: Option, limit: Option, +) -> io::Result { + 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, + limit: Option, + selector: Option<&str>, ) -> io::Result { 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) { + 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 = 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 { if content.len() > MAX_WRITE_SIZE { @@ -715,11 +805,23 @@ pub fn read_file_in_workspace( offset: Option, limit: Option, workspace_root: &Path, +) -> io::Result { + 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, + limit: Option, + selector: Option<&str>, + workspace_root: &Path, ) -> io::Result { 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,119 @@ 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()); + } } diff --git a/rust/crates/runtime/src/lib.rs b/rust/crates/runtime/src/lib.rs index b6660a95..4691f941 100644 --- a/rust/crates/runtime/src/lib.rs +++ b/rust/crates/runtime/src/lib.rs @@ -20,6 +20,7 @@ pub mod green_contract; mod hooks; mod json; mod lane_events; +pub mod line_range_selector; pub mod lsp_client; mod mcp; mod mcp_client; @@ -87,7 +88,8 @@ 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, + grep_search_in_workspace, read_file, read_file_in_workspace, + read_file_in_workspace_with_selector, read_file_with_selector, write_file, write_file_in_workspace, EditFileOutput, GlobSearchOutput, GrepSearchInput, GrepSearchOutput, ReadFileOutput, StructuredPatchHunk, TextFilePayload, WriteFileOutput, }; diff --git a/rust/crates/runtime/src/line_range_selector.rs b/rust/crates/runtime/src/line_range_selector.rs new file mode 100644 index 00000000..13affe15 --- /dev/null +++ b/rust/crates/runtime/src/line_range_selector.rs @@ -0,0 +1,347 @@ +/// GJC-compatible line range selector parsing for the read tool. +/// +/// Supports selector syntax: +/// - `:50` = start at line 50 +/// - `:50-200` = lines 50-200 inclusive +/// - `:50+150` = 150 lines starting at 50 +/// - `:50+1` = exactly one line +/// - `:5-16,960-973` = multiple ranges +/// - `:raw` = full verbatim +/// - `:conflicts` = git merge conflicts only +use std::num::ParseIntError; + +/// Parsed line range specification. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum LineRangeSelector { + /// Full file verbatim (`:raw`) + Raw, + /// Git merge conflict sections only (`:conflicts`) + Conflicts, + /// Line range(s) (`:L-L+N` syntax) + Lines(Vec), + /// No selector (full file, may be truncated by default budget) + None, +} + +/// A single line range, zero-indexed internally but displayed as 1-indexed. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct LineRange { + /// Start line (1-indexed, user-facing) + pub start_line: usize, + /// End line inclusive (1-indexed, user-facing) + pub end_line: usize, +} + +impl LineRange { + /// Create a new line range (1-indexed). + pub fn new(start_line: usize, end_line: usize) -> Self { + Self { + start_line, + end_line, + } + } + + /// Convert to zero-indexed range for internal use. + pub fn zero_indexed(&self) -> (usize, usize) { + ( + self.start_line.saturating_sub(1), + self.end_line.saturating_sub(1), + ) + } + + /// Get the number of lines in this range. + pub fn len(&self) -> usize { + if self.start_line <= self.end_line { + self.end_line - self.start_line + 1 + } else { + 0 + } + } + + /// Check if this range is valid. + pub fn is_valid(&self) -> bool { + self.start_line > 0 && self.end_line > 0 && self.start_line <= self.end_line + } +} + +/// Parse error types for line range selectors. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ParseError { + InvalidFormat(String), + InvalidNumber(ParseIntError), + InvalidRange(String), +} + +impl std::fmt::Display for ParseError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ParseError::InvalidFormat(msg) => write!(f, "Invalid format: {}", msg), + ParseError::InvalidNumber(e) => write!(f, "Invalid number: {}", e), + ParseError::InvalidRange(msg) => write!(f, "Invalid range: {}", msg), + } + } +} + +impl std::error::Error for ParseError {} + +impl From for ParseError { + fn from(e: ParseIntError) -> Self { + ParseError::InvalidNumber(e) + } +} + +/// Parse a line range selector string. +/// +/// Examples: +/// - `"50"` → `LineRangeSelector::Lines(vec![LineRange::new(50, 50)])` +/// - `"50-200"` → `LineRangeSelector::Lines(vec![LineRange::new(50, 200)])` +/// - `"50+150"` → `LineRangeSelector::Lines(vec![LineRange::new(50, 199)])` +/// - `"raw"` → `LineRangeSelector::Raw` +/// - `"conflicts"` → `LineRangeSelector::Conflicts` +pub fn parse_selector(selector: &str) -> Result { + if selector.is_empty() { + return Ok(LineRangeSelector::None); + } + + match selector { + "raw" => Ok(LineRangeSelector::Raw), + "conflicts" => Ok(LineRangeSelector::Conflicts), + _ => parse_line_ranges(selector), + } +} + +/// Parse comma-separated line ranges. +fn parse_line_ranges(selector: &str) -> Result { + let mut ranges = Vec::new(); + + for range_str in selector.split(',') { + let range_str = range_str.trim(); + if range_str.is_empty() { + continue; + } + + let range = parse_single_range(range_str)?; + ranges.push(range); + } + + if ranges.is_empty() { + Ok(LineRangeSelector::None) + } else { + // Merge overlapping/adjacent ranges + ranges.sort_by_key(|r| r.start_line); + let merged = merge_ranges(ranges); + Ok(LineRangeSelector::Lines(merged)) + } +} + +/// Parse a single line range in formats: "50", "50-200", "50+150". +fn parse_single_range(range_str: &str) -> Result { + // Try "50+150" format first (start+count) + if let Some(plus_idx) = range_str.find('+') { + let start_str = &range_str[..plus_idx]; + let count_str = &range_str[plus_idx + 1..]; + + if start_str.is_empty() || count_str.is_empty() { + return Err(ParseError::InvalidFormat(range_str.to_string())); + } + + let start = start_str.parse::()?; + let count = count_str.parse::()?; + + if start == 0 || count == 0 { + return Err(ParseError::InvalidRange( + "Line numbers and counts must be >= 1".to_string(), + )); + } + + return Ok(LineRange::new(start, start + count - 1)); + } + + // Try "50-200" format (start-end) + if let Some(dash_idx) = range_str.find('-') { + // Skip leading dash (negative numbers aren't valid here) + if dash_idx == 0 { + return Err(ParseError::InvalidFormat(range_str.to_string())); + } + + let start_str = &range_str[..dash_idx]; + let end_str = &range_str[dash_idx + 1..]; + + if start_str.is_empty() || end_str.is_empty() { + return Err(ParseError::InvalidFormat(range_str.to_string())); + } + + let start = start_str.parse::()?; + let end = end_str.parse::()?; + + if start == 0 || end == 0 { + return Err(ParseError::InvalidRange( + "Line numbers must be >= 1".to_string(), + )); + } + + if start > end { + return Err(ParseError::InvalidRange(format!( + "Start line {} cannot be greater than end line {}", + start, end + ))); + } + + return Ok(LineRange::new(start, end)); + } + + // Try plain "50" format (single line) + let line_num = range_str.parse::()?; + if line_num == 0 { + return Err(ParseError::InvalidRange( + "Line number must be >= 1".to_string(), + )); + } + + Ok(LineRange::new(line_num, line_num)) +} + +/// Merge overlapping and adjacent ranges. +fn merge_ranges(ranges: Vec) -> Vec { + let mut merged: Vec = Vec::new(); + + for range in ranges { + if let Some(last) = merged.last_mut() { + // If ranges overlap or are adjacent, merge them + if range.start_line <= last.end_line + 1 { + last.end_line = last.end_line.max(range.end_line); + } else { + merged.push(range); + } + } else { + merged.push(range); + } + } + + merged +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_raw() { + assert_eq!(parse_selector("raw").unwrap(), LineRangeSelector::Raw); + } + + #[test] + fn test_parse_conflicts() { + assert_eq!( + parse_selector("conflicts").unwrap(), + LineRangeSelector::Conflicts + ); + } + + #[test] + fn test_parse_single_line() { + match parse_selector("50").unwrap() { + LineRangeSelector::Lines(ranges) => { + assert_eq!(ranges.len(), 1); + assert_eq!(ranges[0].start_line, 50); + assert_eq!(ranges[0].end_line, 50); + } + _ => panic!("Expected Lines variant"), + } + } + + #[test] + fn test_parse_range_start_dash_end() { + match parse_selector("50-200").unwrap() { + LineRangeSelector::Lines(ranges) => { + assert_eq!(ranges.len(), 1); + assert_eq!(ranges[0].start_line, 50); + assert_eq!(ranges[0].end_line, 200); + } + _ => panic!("Expected Lines variant"), + } + } + + #[test] + fn test_parse_range_start_plus_count() { + match parse_selector("50+150").unwrap() { + LineRangeSelector::Lines(ranges) => { + assert_eq!(ranges.len(), 1); + assert_eq!(ranges[0].start_line, 50); + assert_eq!(ranges[0].end_line, 199); + } + _ => panic!("Expected Lines variant"), + } + } + + #[test] + fn test_parse_multiple_ranges() { + match parse_selector("5-16,960-973").unwrap() { + LineRangeSelector::Lines(ranges) => { + assert_eq!(ranges.len(), 2); + assert_eq!(ranges[0].start_line, 5); + assert_eq!(ranges[0].end_line, 16); + assert_eq!(ranges[1].start_line, 960); + assert_eq!(ranges[1].end_line, 973); + } + _ => panic!("Expected Lines variant"), + } + } + + #[test] + fn test_merge_overlapping_ranges() { + // "5-16,10-20" should merge into "5-20" + match parse_selector("5-16,10-20").unwrap() { + LineRangeSelector::Lines(ranges) => { + assert_eq!(ranges.len(), 1); + assert_eq!(ranges[0].start_line, 5); + assert_eq!(ranges[0].end_line, 20); + } + _ => panic!("Expected Lines variant"), + } + } + + #[test] + fn test_merge_adjacent_ranges() { + // "5-16,17-20" should merge into "5-20" + match parse_selector("5-16,17-20").unwrap() { + LineRangeSelector::Lines(ranges) => { + assert_eq!(ranges.len(), 1); + assert_eq!(ranges[0].start_line, 5); + assert_eq!(ranges[0].end_line, 20); + } + _ => panic!("Expected Lines variant"), + } + } + + #[test] + fn test_invalid_range_start_greater_than_end() { + let result = parse_selector("200-50"); + assert!(result.is_err()); + } + + #[test] + fn test_invalid_zero_line() { + let result = parse_selector("0"); + assert!(result.is_err()); + } + + #[test] + fn test_empty_selector() { + assert_eq!(parse_selector("").unwrap(), LineRangeSelector::None); + } + + #[test] + fn test_line_range_zero_indexed() { + let range = LineRange::new(50, 200); + let (start, end) = range.zero_indexed(); + assert_eq!(start, 49); + assert_eq!(end, 199); + } + + #[test] + fn test_line_range_len() { + assert_eq!(LineRange::new(50, 200).len(), 151); + assert_eq!(LineRange::new(50, 50).len(), 1); + } +} diff --git a/rust/crates/tools/src/lib.rs b/rust/crates/tools/src/lib.rs index 2ad7926a..627adfb4 100644 --- a/rust/crates/tools/src/lib.rs +++ b/rust/crates/tools/src/lib.rs @@ -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,14 @@ fn branch_divergence_output( fn run_read_file(input: ReadFileInput) -> Result { 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 +2891,7 @@ struct ReadFileInput { path: String, offset: Option, limit: Option, + selector: Option, } #[derive(Debug, Deserialize)] @@ -10690,6 +10697,43 @@ 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();