An AI agent built to do Ralph loops - plan mode for planning and ralph mode for implementing.

fix: address code review feedback for Phase 5

- Critical Issue: Filter node_modules and target directories in code search.
The filter_entry closure now checks skip_dirs for ALL directories, not just
those starting with '.'. This ensures node_modules and target are properly
skipped. Added unit test code_search_skips_node_modules_and_target to verify.

- Important Issue: Collapse nested if-let chains in orchestrator.rs using
Rust 2024 let-chains. The try_unblock_tasks method now uses a single
if let ... && let ... condition instead of nested blocks, resolving the
clippy collapsible_if warning.

- Minor Issue: Tighten assertion in code_search_skips_binary_files test.
Removed the vacuous disjunction - the '.' regex always matches text.txt,
so the assertion now directly checks for "text.txt" presence.

Verification:
- All 7 code_search unit tests pass
- Full test suite passes (all 100+ tests)
- cargo clippy clean (collapsible_if warning resolved)
- cargo build successful

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

+64 -29
+23 -23
src/agent/orchestrator.rs
··· 1141 1141 1142 1142 for task in &blocked_tasks { 1143 1143 // Look up blocker task ID from metadata (set during cascade) 1144 - if let Some(blocker_id) = task.metadata.get("blocker_task_id") { 1145 - if let Some(blocker) = self.graph_store.get_node(blocker_id).await? { 1146 - // If the blocker has been retried and is now completed, unblock 1147 - if blocker.status == NodeStatus::Completed { 1148 - // Remove blocker_task_id from metadata when unblocking 1149 - let mut metadata = task.metadata.clone(); 1150 - metadata.remove("blocker_task_id"); 1144 + if let Some(blocker_id) = task.metadata.get("blocker_task_id") 1145 + && let Some(blocker) = self.graph_store.get_node(blocker_id).await? 1146 + { 1147 + // If the blocker has been retried and is now completed, unblock 1148 + if blocker.status == NodeStatus::Completed { 1149 + // Remove blocker_task_id from metadata when unblocking 1150 + let mut metadata = task.metadata.clone(); 1151 + metadata.remove("blocker_task_id"); 1151 1152 1152 - self.graph_store 1153 - .update_node( 1154 - &task.id, 1155 - Some(NodeStatus::Ready), 1156 - None, // title unchanged 1157 - None, // description unchanged 1158 - Some(""), // clear blocked_reason by setting to empty string 1159 - Some(&metadata), // metadata with blocker_task_id removed 1160 - ) 1161 - .await?; 1153 + self.graph_store 1154 + .update_node( 1155 + &task.id, 1156 + Some(NodeStatus::Ready), 1157 + None, // title unchanged 1158 + None, // description unchanged 1159 + Some(""), // clear blocked_reason by setting to empty string 1160 + Some(&metadata), // metadata with blocker_task_id removed 1161 + ) 1162 + .await?; 1162 1163 1163 - tracing::info!( 1164 - task = %task.id, 1165 - blocker = %blocker_id, 1166 - "Task unblocked: dependency resolved" 1167 - ); 1168 - } 1164 + tracing::info!( 1165 + task = %task.id, 1166 + blocker = %blocker_id, 1167 + "Task unblocked: dependency resolved" 1168 + ); 1169 1169 } 1170 1170 } 1171 1171 }
+41 -6
src/tools/search.rs
··· 92 92 93 93 // Walk directory tree 94 94 for entry in WalkDir::new(&search_root).into_iter().filter_entry(|e| { 95 - // Skip hidden directories 96 - let file_name = e.file_name().to_string_lossy(); 97 - if e.file_type().is_dir() && file_name.starts_with('.') { 98 - let name_str = file_name.as_ref(); 99 - !skip_dirs.contains(&name_str) 95 + // Skip directories in skip_dirs list 96 + if e.file_type().is_dir() { 97 + let file_name = e.file_name().to_string_lossy(); 98 + !skip_dirs.contains(&file_name.as_ref()) 100 99 } else { 101 100 true 102 101 } ··· 269 268 270 269 // Should not fail, just skip the binary 271 270 let result = tool.execute(params).await.unwrap(); 272 - assert!(result.contains("text.txt") || result.contains("No matches found")); 271 + assert!(result.contains("text.txt")); 273 272 } 274 273 275 274 #[tokio::test] ··· 311 310 312 311 let result = tool.execute(params).await.unwrap(); 313 312 assert_eq!(result, "No matches found"); 313 + } 314 + 315 + #[tokio::test] 316 + async fn code_search_skips_node_modules_and_target() { 317 + let temp_dir = TempDir::new().unwrap(); 318 + let project_root = temp_dir.path().to_path_buf(); 319 + 320 + // Create files in regular directory 321 + std::fs::write(project_root.join("main.rs"), "fn main() {}").unwrap(); 322 + 323 + // Create files in node_modules (should be skipped) 324 + std::fs::create_dir_all(project_root.join("node_modules")).unwrap(); 325 + std::fs::write( 326 + project_root.join("node_modules/package.txt"), 327 + "fn should_skip", 328 + ) 329 + .unwrap(); 330 + 331 + // Create files in target (should be skipped) 332 + std::fs::create_dir_all(project_root.join("target")).unwrap(); 333 + std::fs::write( 334 + project_root.join("target/artifact.rs"), 335 + "fn should_skip", 336 + ) 337 + .unwrap(); 338 + 339 + let tool = CodeSearchTool::new(project_root); 340 + let params = json!({ 341 + "pattern": "fn" 342 + }); 343 + 344 + let result = tool.execute(params).await.unwrap(); 345 + // Should find main.rs but NOT files in node_modules or target 346 + assert!(result.contains("main.rs")); 347 + assert!(!result.contains("node_modules")); 348 + assert!(!result.contains("target")); 314 349 } 315 350 }