| 1 | //! The workspace must contain exactly one turn loop. |
| 2 | //! |
| 3 | //! `crates/core` carried a placeholder `engine/` tree whose `Engine::run` |
| 4 | //! accepted `Op::SendMessage`, appended to a journal, and emitted |
| 5 | //! `TurnComplete { status: "completed" }` without ever contacting a model. It |
| 6 | //! had no callers, but its doc comments ("the real turn loop is wired here in |
| 7 | //! the next slice") were load-bearing for `docs/ARCHITECTURE.md`'s claim that |
| 8 | //! core owns the agent loop, and a reader could reasonably have built on it. |
| 9 | //! |
| 10 | //! This guard is deliberately a source scan rather than a type check: the thing |
| 11 | //! being prevented is a *second implementation*, which by definition would not |
| 12 | //! be reachable from the first. |
| 13 | //! |
| 14 | //! # What changed, and why (#6242) |
| 15 | //! |
| 16 | //! Until #6242 this scan matched a function *name* — `async fn run_turn`. A |
| 17 | //! name is not the rule. `acp_server.rs` grew a full second turn loop called |
| 18 | //! `run_agentic_prompt_turn`, and the guard stayed green for as long as it |
| 19 | //! existed; nobody picked that name to evade anything, which is exactly the |
| 20 | //! problem. A guard satisfied by spelling is satisfied by accident. |
| 21 | //! |
| 22 | //! So the scan now matches the *shape*. A turn loop is a loop whose body does |
| 23 | //! all three of these in one iteration: |
| 24 | //! |
| 25 | //! 1. **drives a model** — calls something that opens or consumes a provider |
| 26 | //! message stream / completion: an identifier ending in `stream`, |
| 27 | //! `create_message`, `completion`, or `complete`; *starting* with |
| 28 | //! `create_message` (the `LlmClient` method family — `create_message`, |
| 29 | //! `create_message_stream`, `create_message_boxed`, …); or spelled |
| 30 | //! `request_…model…` (a wrapper that requests a model response); |
| 31 | //! 2. **dispatches tool calls** — calls something that executes tools |
| 32 | //! (`execute_*tool*`, `dispatch_*tool*`, `run_*tool*`, …), or runs |
| 33 | //! model-written code in a REPL/kernel (`repl.run(…)`, `kernel.execute(…)`) |
| 34 | //! — a code round is a tool round whatever the executor is called; |
| 35 | //! 3. **assembles its own prompt** — pushes onto a message history |
| 36 | //! (`…messages…`/`…history…`/`…conversation…`/`…prompt…`.`push`/`extend`). |
| 37 | //! |
| 38 | //! Anything doing all three per iteration *is* a turn loop, whatever it is |
| 39 | //! called. Renaming it does not hide it; only [`ALLOWED_TURN_LOOPS`] does, and |
| 40 | //! that list is read back at the end of the test so a stale entry fails too. |
| 41 | //! |
| 42 | //! # #6511: suffix-only matching was a spelling guard too |
| 43 | //! |
| 44 | //! After #6242 the model-call marker was still a *suffix* list, so two loops |
| 45 | //! passed by spelling: the sub-agent loop calls |
| 46 | //! `request_subagent_model_response_with_retries(…)` and the RLM loop calls |
| 47 | //! `client.create_message_boxed(…)` and runs its code rounds through |
| 48 | //! `repl.run(…)`. CI said "exactly one turn loop" while three existed. The |
| 49 | //! markers cover both spellings. Both the child and RLM producer/code loops |
| 50 | //! now enter the canonical Engine; neither has a migration exception. |
| 51 | //! |
| 52 | //! # Known limitations |
| 53 | //! |
| 54 | //! - It is a lexical scan, not a type check. A turn loop that reaches the |
| 55 | //! provider through an indirection matching none of the markers above, or |
| 56 | //! that never mutates a message history, would not be seen. The markers are |
| 57 | //! deliberately broad rather than exact for that reason, and |
| 58 | //! [`detector_sees_a_renamed_turn_loop`] pins the detector against going |
| 59 | //! vacuously blind. |
| 60 | //! - `#[cfg(test)]` modules and `tests/`, `benches/`, `examples/` sources are |
| 61 | //! skipped: a test harness that drives rounds is not a shipped turn loop. |
| 62 | //! - It reports one loop per (file, enclosing function). A function with two |
| 63 | //! turn loops in it is one finding, not two. |
| 64 | |
| 65 | use std::collections::{BTreeMap, BTreeSet}; |
| 66 | use std::path::{Path, PathBuf}; |
| 67 | |
| 68 | /// A loop that is permitted to drive model/tool rounds, and the reason. |
| 69 | /// |
| 70 | /// Every entry must be *named*, *documented*, and *reachable* — an entry whose |
| 71 | /// loop no longer exists fails the test, so an exception has to be consciously |
| 72 | /// renewed instead of quietly outliving its reason. |
| 73 | struct AllowedTurnLoop { |
| 74 | /// Workspace-relative path, `/`-separated. |
| 75 | path: &'static str, |
| 76 | /// Name of the function that lexically encloses the loop. |
| 77 | owner: &'static str, |
| 78 | /// Why this loop is allowed to exist. Exceptions cite their issue. |
| 79 | why: &'static str, |
| 80 | } |
| 81 | |
| 82 | const ALLOWED_TURN_LOOPS: &[AllowedTurnLoop] = &[AllowedTurnLoop { |
| 83 | path: "crates/tui/src/core/engine/turn_loop.rs", |
| 84 | owner: "run_turn", |
| 85 | why: "THE turn loop. `Engine::run_turn` is the single agent loop; \ |
| 86 | docs/ARCHITECTURE.md and AGENTS.md both name it as the owner.", |
| 87 | }]; |
| 88 | |
| 89 | // --------------------------------------------------------------------------- |
| 90 | // Detection |
| 91 | // --------------------------------------------------------------------------- |
| 92 | |
| 93 | /// A call to an identifier ending in one of these, i.e. "this iteration talks |
| 94 | /// to a model". |
| 95 | const MODEL_CALL_SUFFIXES: &[&str] = &["stream", "create_message", "completion", "complete"]; |
| 96 | |
| 97 | /// A call to an identifier *starting* with one of these also talks to a model: |
| 98 | /// the `LlmClient` method family (`create_message_boxed`, …) is spelled by |
| 99 | /// prefix, not suffix (#6511). |
| 100 | const MODEL_CALL_PREFIXES: &[&str] = &["create_message"]; |
| 101 | |
| 102 | /// Receiver-name fragments for "this iteration runs model-written code": |
| 103 | /// `repl.run(…)` / `kernel.execute(…)` is a tool round by another name. |
| 104 | const CODE_RUNNER_RECEIVER_FRAGMENTS: &[&str] = &["repl", "kernel"]; |
| 105 | |
| 106 | /// Prefix/infix pairs for "this iteration dispatches tool calls". |
| 107 | const TOOL_DISPATCH_VERBS: &[&str] = &[ |
| 108 | "execute", "dispatch", "run", "invoke", "perform", "handle", "call", |
| 109 | ]; |
| 110 | |
| 111 | /// Receiver-name fragments for "this iteration assembles its own prompt". |
| 112 | const HISTORY_RECEIVER_FRAGMENTS: &[&str] = &["messages", "history", "conversation", "prompt"]; |
| 113 | |
| 114 | #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] |
| 115 | struct TurnLoopSite { |
| 116 | path: String, |
| 117 | owner: String, |
| 118 | line: usize, |
| 119 | keyword: String, |
| 120 | } |
| 121 | |
| 122 | fn is_ident_char(c: char) -> bool { |
| 123 | c.is_ascii_alphanumeric() || c == '_' |
| 124 | } |
| 125 | |
| 126 | /// Blank out comments, string literals, and char literals, preserving byte |
| 127 | /// offsets and newlines. Brace matching is only trustworthy over source that |
| 128 | /// cannot contain a `{` inside a comment or a `"{"` literal. |
| 129 | fn sanitize(src: &str) -> String { |
| 130 | let bytes: Vec<char> = src.chars().collect(); |
| 131 | let mut out: Vec<char> = bytes.clone(); |
| 132 | let blank = |out: &mut Vec<char>, from: usize, to: usize| { |
| 133 | for slot in out.iter_mut().take(to.min(bytes.len())).skip(from) { |
| 134 | if *slot != '\n' { |
| 135 | *slot = ' '; |
| 136 | } |
| 137 | } |
| 138 | }; |
| 139 | let mut i = 0usize; |
| 140 | while i < bytes.len() { |
| 141 | let c = bytes[i]; |
| 142 | let next = bytes.get(i + 1).copied(); |
| 143 | if c == '/' && next == Some('/') { |
| 144 | let start = i; |
| 145 | while i < bytes.len() && bytes[i] != '\n' { |
| 146 | i += 1; |
| 147 | } |
| 148 | blank(&mut out, start, i); |
| 149 | } else if c == '/' && next == Some('*') { |
| 150 | let start = i; |
| 151 | let mut depth = 0usize; |
| 152 | while i < bytes.len() { |
| 153 | if bytes[i] == '/' && bytes.get(i + 1) == Some(&'*') { |
| 154 | depth += 1; |
| 155 | i += 2; |
| 156 | } else if bytes[i] == '*' && bytes.get(i + 1) == Some(&'/') { |
| 157 | depth -= 1; |
| 158 | i += 2; |
| 159 | if depth == 0 { |
| 160 | break; |
| 161 | } |
| 162 | } else { |
| 163 | i += 1; |
| 164 | } |
| 165 | } |
| 166 | blank(&mut out, start, i); |
| 167 | } else if c == 'r' && matches!(next, Some('"') | Some('#')) { |
| 168 | // Raw string `r"..."` / `r#"..."#`, but not the identifier `red`. |
| 169 | let mut hashes = 0usize; |
| 170 | let mut j = i + 1; |
| 171 | while bytes.get(j) == Some(&'#') { |
| 172 | hashes += 1; |
| 173 | j += 1; |
| 174 | } |
| 175 | if bytes.get(j) != Some(&'"') || (i > 0 && is_ident_char(bytes[i - 1])) { |
| 176 | i += 1; |
| 177 | continue; |
| 178 | } |
| 179 | let start = i; |
| 180 | j += 1; |
| 181 | loop { |
| 182 | if j >= bytes.len() { |
| 183 | break; |
| 184 | } |
| 185 | if bytes[j] == '"' { |
| 186 | let closing = (1..=hashes).all(|k| bytes.get(j + k) == Some(&'#')); |
| 187 | if closing { |
| 188 | j += hashes + 1; |
| 189 | break; |
| 190 | } |
| 191 | } |
| 192 | j += 1; |
| 193 | } |
| 194 | i = j; |
| 195 | blank(&mut out, start, i); |
| 196 | } else if c == '"' { |
| 197 | let start = i; |
| 198 | i += 1; |
| 199 | while i < bytes.len() { |
| 200 | if bytes[i] == '\\' { |
| 201 | i += 2; |
| 202 | continue; |
| 203 | } |
| 204 | if bytes[i] == '"' { |
| 205 | i += 1; |
| 206 | break; |
| 207 | } |
| 208 | i += 1; |
| 209 | } |
| 210 | blank(&mut out, start, i); |
| 211 | } else if c == '\'' { |
| 212 | // `'a'` / `'\n'` are char literals; `'a` alone is a lifetime. |
| 213 | let is_char_lit = if bytes.get(i + 1) == Some(&'\\') { |
| 214 | true |
| 215 | } else { |
| 216 | bytes.get(i + 2) == Some(&'\'') |
| 217 | }; |
| 218 | if is_char_lit { |
| 219 | let start = i; |
| 220 | i += 1; |
| 221 | while i < bytes.len() { |
| 222 | if bytes[i] == '\\' { |
| 223 | i += 2; |
| 224 | continue; |
| 225 | } |
| 226 | if bytes[i] == '\'' { |
| 227 | i += 1; |
| 228 | break; |
| 229 | } |
| 230 | i += 1; |
| 231 | } |
| 232 | blank(&mut out, start, i); |
| 233 | } else { |
| 234 | i += 1; |
| 235 | } |
| 236 | } else { |
| 237 | i += 1; |
| 238 | } |
| 239 | } |
| 240 | out.into_iter().collect() |
| 241 | } |
| 242 | |
| 243 | /// Strip `#[cfg(test)]`-gated items. A test harness that drives rounds is not |
| 244 | /// a shipped turn loop. |
| 245 | fn strip_cfg_test(src: &str) -> String { |
| 246 | let chars: Vec<char> = sanitize(src).chars().collect(); |
| 247 | let mut out: Vec<char> = src.chars().collect(); |
| 248 | let mut search_from = 0usize; |
| 249 | let needle = "cfg(test)"; |
| 250 | while let Some(rel) = src[search_from..].find(needle) { |
| 251 | let at = search_from + rel; |
| 252 | search_from = at + needle.len(); |
| 253 | // Require a `#[` immediately before (allowing whitespace). |
| 254 | let prefix = &src[..at]; |
| 255 | let Some(hash) = prefix.rfind("#[") else { |
| 256 | continue; |
| 257 | }; |
| 258 | if !prefix[hash + 2..].trim().is_empty() { |
| 259 | continue; |
| 260 | } |
| 261 | let hash_ci = src[..hash].chars().count(); |
| 262 | let mut i = src[..search_from].chars().count(); |
| 263 | // Skip the cfg attribute's close, then find this item's boundary. |
| 264 | // Commas end gated struct fields/initializers, while commas inside a |
| 265 | // function signature or generic type do not. Braces in strings and |
| 266 | // comments were already blanked by the same existing sanitizer. |
| 267 | while i < chars.len() && chars[i] != ']' { |
| 268 | i += 1; |
| 269 | } |
| 270 | i += 1; |
| 271 | let mut nesting = 0usize; |
| 272 | let mut angles = 0usize; |
| 273 | while i < chars.len() { |
| 274 | match chars[i] { |
| 275 | '(' | '[' => nesting += 1, |
| 276 | ')' | ']' => nesting = nesting.saturating_sub(1), |
| 277 | '<' if nesting == 0 => angles += 1, |
| 278 | '>' if nesting == 0 => angles = angles.saturating_sub(1), |
| 279 | '{' | ';' | ',' if nesting == 0 && angles == 0 => break, |
| 280 | _ => {} |
| 281 | } |
| 282 | i += 1; |
| 283 | } |
| 284 | if i >= chars.len() { |
| 285 | continue; |
| 286 | } |
| 287 | if matches!(chars[i], ';' | ',') { |
| 288 | for slot in out.iter_mut().take(i + 1).skip(hash_ci) { |
| 289 | if *slot != '\n' { |
| 290 | *slot = ' '; |
| 291 | } |
| 292 | } |
| 293 | continue; |
| 294 | } |
| 295 | let mut depth = 0usize; |
| 296 | let mut j = i; |
| 297 | while j < chars.len() { |
| 298 | if chars[j] == '{' { |
| 299 | depth += 1; |
| 300 | } else if chars[j] == '}' { |
| 301 | depth -= 1; |
| 302 | if depth == 0 { |
| 303 | break; |
| 304 | } |
| 305 | } |
| 306 | j += 1; |
| 307 | } |
| 308 | for slot in out.iter_mut().take((j + 1).min(chars.len())).skip(hash_ci) { |
| 309 | if *slot != '\n' { |
| 310 | *slot = ' '; |
| 311 | } |
| 312 | } |
| 313 | } |
| 314 | out.into_iter().collect() |
| 315 | } |
| 316 | |
| 317 | /// Every `loop`/`for … in …`/`while …` block, as (keyword, body char range). |
| 318 | fn loop_bodies(chars: &[char]) -> Vec<(String, usize, usize)> { |
| 319 | let mut found = Vec::new(); |
| 320 | let mut i = 0usize; |
| 321 | while i < chars.len() { |
| 322 | if !is_ident_char(chars[i]) { |
| 323 | i += 1; |
| 324 | continue; |
| 325 | } |
| 326 | let start = i; |
| 327 | while i < chars.len() && is_ident_char(chars[i]) { |
| 328 | i += 1; |
| 329 | } |
| 330 | if start > 0 && is_ident_char(chars[start - 1]) { |
| 331 | continue; |
| 332 | } |
| 333 | let word: String = chars[start..i].iter().collect(); |
| 334 | if word != "loop" && word != "for" && word != "while" { |
| 335 | continue; |
| 336 | } |
| 337 | // Walk to the body `{`, tracking paren/bracket depth so `for x in v[0] {` |
| 338 | // and `while let Some(x) = it.next() {` resolve correctly. |
| 339 | let mut j = i; |
| 340 | let mut nesting = 0i32; |
| 341 | let mut body_open = None; |
| 342 | let mut saw_in = false; |
| 343 | while j < chars.len() { |
| 344 | match chars[j] { |
| 345 | '(' | '[' => nesting += 1, |
| 346 | ')' | ']' => nesting -= 1, |
| 347 | ';' if nesting == 0 => break, |
| 348 | '{' if nesting == 0 => { |
| 349 | body_open = Some(j); |
| 350 | break; |
| 351 | } |
| 352 | _ => {} |
| 353 | } |
| 354 | if word == "for" |
| 355 | && nesting == 0 |
| 356 | && chars[j] == 'i' |
| 357 | && chars.get(j + 1) == Some(&'n') |
| 358 | && !is_ident_char(chars[j - 1]) |
| 359 | && chars.get(j + 2).is_some_and(|c| !is_ident_char(*c)) |
| 360 | { |
| 361 | saw_in = true; |
| 362 | } |
| 363 | j += 1; |
| 364 | } |
| 365 | let Some(open) = body_open else { continue }; |
| 366 | // `impl Trait for Type {` and `for<'a> Fn(..)` are the `for` keyword |
| 367 | // without a loop. A `for` loop always has an `in` before its body. |
| 368 | if word == "for" && !saw_in { |
| 369 | continue; |
| 370 | } |
| 371 | let mut depth = 0usize; |
| 372 | let mut k = open; |
| 373 | while k < chars.len() { |
| 374 | if chars[k] == '{' { |
| 375 | depth += 1; |
| 376 | } else if chars[k] == '}' { |
| 377 | depth -= 1; |
| 378 | if depth == 0 { |
| 379 | break; |
| 380 | } |
| 381 | } |
| 382 | k += 1; |
| 383 | } |
| 384 | found.push((word, open, k.min(chars.len().saturating_sub(1)))); |
| 385 | } |
| 386 | found |
| 387 | } |
| 388 | |
| 389 | /// Every identifier in `body` that is called as a function or method, i.e. |
| 390 | /// immediately followed (modulo whitespace) by `(`. Macros (`name!(`) and |
| 391 | /// bare parentheses are not identifiers and are skipped. |
| 392 | fn called_identifiers(body: &str) -> impl Iterator<Item = &str> { |
| 393 | body.match_indices('(').filter_map(|(at, _)| { |
| 394 | let before = body[..at].trim_end(); |
| 395 | let start = before |
| 396 | .rfind(|c: char| !is_ident_char(c)) |
| 397 | .map_or(0, |i| i + 1); |
| 398 | let ident = &before[start..]; |
| 399 | (!ident.is_empty() && !ident.starts_with(|c: char| c.is_ascii_digit())).then_some(ident) |
| 400 | }) |
| 401 | } |
| 402 | |
| 403 | /// True when `ident` names something that requests a model response. |
| 404 | fn is_model_call(ident: &str) -> bool { |
| 405 | MODEL_CALL_SUFFIXES |
| 406 | .iter() |
| 407 | .any(|suffix| ident.ends_with(suffix)) |
| 408 | || MODEL_CALL_PREFIXES |
| 409 | .iter() |
| 410 | .any(|prefix| ident.starts_with(prefix)) |
| 411 | || (ident.starts_with("request_") && ident.contains("model")) |
| 412 | } |
| 413 | |
| 414 | /// True when `body` calls something that drives a model. |
| 415 | fn drives_a_model(body: &str) -> bool { |
| 416 | called_identifiers(body).any(is_model_call) |
| 417 | } |
| 418 | |
| 419 | /// The identifier a `.method(` call at byte `at` is invoked on, when the |
| 420 | /// receiver is a plain identifier (`messages.push(`, `repl.run(`). |
| 421 | fn method_receiver(body: &str, at: usize) -> Option<&str> { |
| 422 | let before = body[..at].trim_end().strip_suffix('.')?; |
| 423 | let receiver_end = before.trim_end(); |
| 424 | let ident_start = receiver_end |
| 425 | .rfind(|c: char| !is_ident_char(c)) |
| 426 | .map_or(0, |i| i + 1); |
| 427 | Some(&receiver_end[ident_start..]) |
| 428 | } |
| 429 | |
| 430 | /// True when `body` calls `.method(` on a receiver whose name contains one of |
| 431 | /// `fragments`. |
| 432 | fn calls_method_on(body: &str, methods: &[&str], fragments: &[&str]) -> bool { |
| 433 | methods.iter().any(|method| { |
| 434 | body.match_indices(method).any(|(at, _)| { |
| 435 | let after = &body[at + method.len()..]; |
| 436 | if !after.trim_start().starts_with('(') { |
| 437 | return false; |
| 438 | } |
| 439 | method_receiver(body, at) |
| 440 | .is_some_and(|receiver| fragments.iter().any(|f| receiver.contains(f))) |
| 441 | }) |
| 442 | }) |
| 443 | } |
| 444 | |
| 445 | /// True when `body` calls something like `execute_tool_calls(…)`, or runs |
| 446 | /// model-written code on a REPL/kernel (`repl.run(…)`). |
| 447 | fn dispatches_tool_calls(body: &str) -> bool { |
| 448 | let named_tool_dispatch = TOOL_DISPATCH_VERBS.iter().any(|verb| { |
| 449 | body.match_indices(verb).any(|(at, _)| { |
| 450 | if at > 0 && is_ident_char(body[..at].chars().next_back().unwrap_or(' ')) { |
| 451 | return false; |
| 452 | } |
| 453 | let rest = &body[at + verb.len()..]; |
| 454 | if !rest.starts_with('_') { |
| 455 | return false; |
| 456 | } |
| 457 | let ident_end = rest.find(|c: char| !is_ident_char(c)).unwrap_or(rest.len()); |
| 458 | rest[..ident_end].contains("tool") |
| 459 | }) |
| 460 | }); |
| 461 | named_tool_dispatch |
| 462 | || calls_method_on(body, &["run", "execute"], CODE_RUNNER_RECEIVER_FRAGMENTS) |
| 463 | } |
| 464 | |
| 465 | /// True when `body` pushes onto something whose name reads like a prompt or |
| 466 | /// message history — the loop assembling its own conversation. |
| 467 | fn assembles_prompt_history(body: &str) -> bool { |
| 468 | calls_method_on(body, &["push", "extend"], HISTORY_RECEIVER_FRAGMENTS) |
| 469 | } |
| 470 | |
| 471 | /// Name of the function that lexically encloses char offset `at`. |
| 472 | fn enclosing_fn(chars: &[char], at: usize) -> String { |
| 473 | let head: String = chars[..at].iter().collect(); |
| 474 | let mut best = None; |
| 475 | let mut search = 0usize; |
| 476 | while let Some(rel) = head[search..].find("fn ") { |
| 477 | let idx = search + rel; |
| 478 | search = idx + 3; |
| 479 | let before_ok = idx == 0 || !is_ident_char(head[..idx].chars().next_back().unwrap_or(' ')); |
| 480 | if !before_ok { |
| 481 | continue; |
| 482 | } |
| 483 | let rest = head[idx + 3..].trim_start(); |
| 484 | let name_end = rest.find(|c: char| !is_ident_char(c)).unwrap_or(rest.len()); |
| 485 | if name_end > 0 { |
| 486 | best = Some(rest[..name_end].to_string()); |
| 487 | } |
| 488 | } |
| 489 | best.unwrap_or_else(|| "<unknown>".to_string()) |
| 490 | } |
| 491 | |
| 492 | /// A resolved local method, including the syntactic impl receiver. Matching |
| 493 | /// `self.phase()` by its spelling alone would join unrelated impls. |
| 494 | #[derive(Clone)] |
| 495 | struct LocalMethod { |
| 496 | receiver: String, |
| 497 | name: String, |
| 498 | body: String, |
| 499 | open: usize, |
| 500 | close: usize, |
| 501 | } |
| 502 | |
| 503 | fn matching_brace(chars: &[char], open: usize) -> usize { |
| 504 | let mut depth = 0usize; |
| 505 | for (at, ch) in chars.iter().enumerate().skip(open) { |
| 506 | match ch { |
| 507 | '{' => depth += 1, |
| 508 | '}' => { |
| 509 | depth -= 1; |
| 510 | if depth == 0 { |
| 511 | return at; |
| 512 | } |
| 513 | } |
| 514 | _ => {} |
| 515 | } |
| 516 | } |
| 517 | panic!("unbalanced source block at {open}"); |
| 518 | } |
| 519 | |
| 520 | /// Reuse the sanitized, balanced source rather than phase names or markers. |
| 521 | /// Only inherent impls with literal receiver names are resolved. Unknown or |
| 522 | /// ambiguous calls are refused when needed, rather than guessed across types. |
| 523 | fn local_methods(prepared: &str) -> Vec<LocalMethod> { |
| 524 | let chars: Vec<char> = prepared.chars().collect(); |
| 525 | let mut methods = Vec::new(); |
| 526 | for (byte, _) in prepared.match_indices("impl ") { |
| 527 | if byte > 0 && is_ident_char(prepared[..byte].chars().next_back().unwrap()) { |
| 528 | continue; |
| 529 | } |
| 530 | let start = prepared[..byte].chars().count(); |
| 531 | let Some(open_rel) = chars[start..].iter().position(|c| *c == '{') else { |
| 532 | continue; |
| 533 | }; |
| 534 | let open = start + open_rel; |
| 535 | let header: String = chars[start + 5..open].iter().collect(); |
| 536 | // Trait impls and generic/path receivers require more resolution than |
| 537 | // this literal local graph provides; direct shape detection remains. |
| 538 | let receiver = header.trim(); |
| 539 | if receiver.is_empty() || !receiver.chars().all(is_ident_char) { |
| 540 | continue; |
| 541 | } |
| 542 | let close = matching_brace(&chars, open); |
| 543 | let impl_body: String = chars[open + 1..close].iter().collect(); |
| 544 | for (fn_byte, _) in impl_body.match_indices("fn ") { |
| 545 | if fn_byte > 0 && is_ident_char(impl_body[..fn_byte].chars().next_back().unwrap()) { |
| 546 | continue; |
| 547 | } |
| 548 | let rest = &impl_body[fn_byte + 3..]; |
| 549 | let name_end = rest.find(|c: char| !is_ident_char(c)).unwrap_or(rest.len()); |
| 550 | if name_end == 0 { |
| 551 | continue; |
| 552 | } |
| 553 | let fn_start = open + 1 + impl_body[..fn_byte].chars().count(); |
| 554 | let Some(body_rel) = chars[fn_start..close].iter().position(|c| *c == '{') else { |
| 555 | continue; |
| 556 | }; |
| 557 | let body_open = fn_start + body_rel; |
| 558 | let signature: String = chars[fn_start..body_open].iter().collect(); |
| 559 | if signature.contains(';') |
| 560 | || !signature |
| 561 | .split(|c: char| !is_ident_char(c)) |
| 562 | .any(|v| v == "self") |
| 563 | { |
| 564 | continue; |
| 565 | } |
| 566 | let body_close = matching_brace(&chars, body_open); |
| 567 | methods.push(LocalMethod { |
| 568 | receiver: receiver.to_string(), |
| 569 | name: rest[..name_end].to_string(), |
| 570 | body: chars[body_open..=body_close].iter().collect(), |
| 571 | open: body_open, |
| 572 | close: body_close, |
| 573 | }); |
| 574 | } |
| 575 | } |
| 576 | methods |
| 577 | } |
| 578 | |
| 579 | type MethodGraph = BTreeMap<(String, String), Vec<String>>; |
| 580 | |
| 581 | fn method_graph(sources: &[String]) -> MethodGraph { |
| 582 | let mut graph = MethodGraph::new(); |
| 583 | for (index, source) in sources.iter().enumerate() { |
| 584 | let prepared = sanitize(&strip_cfg_test(source)); |
| 585 | let compact: String = prepared.chars().filter(|c| !c.is_whitespace()).collect(); |
| 586 | let tokens: Vec<&str> = prepared |
| 587 | .split(|c: char| !is_ident_char(c)) |
| 588 | .filter(|s| !s.is_empty()) |
| 589 | .collect(); |
| 590 | for method in local_methods(&prepared) { |
| 591 | if index + 1 != sources.len() { |
| 592 | // A child contributes to its parent's inherent impl only |
| 593 | // through a literal parent binding, with no local same-named |
| 594 | // type. Unrelated imports/local types are scanned at their |
| 595 | // own file but cannot impersonate the parent's phase owner. |
| 596 | let parent_binding = compact.contains("usesuper::*;") |
| 597 | || compact.contains(&format!("usesuper::{};", method.receiver)); |
| 598 | let local_type = tokens.windows(2).any(|pair| { |
| 599 | matches!(pair[0], "struct" | "enum" | "type" | "union") |
| 600 | && pair[1] == method.receiver |
| 601 | }); |
| 602 | if !parent_binding || local_type { |
| 603 | continue; |
| 604 | } |
| 605 | } |
| 606 | graph |
| 607 | .entry((method.receiver, method.name)) |
| 608 | .or_default() |
| 609 | .push(method.body); |
| 610 | } |
| 611 | } |
| 612 | graph |
| 613 | } |
| 614 | |
| 615 | /// A helper's own loops are independently scanned at their lexical owner. |
| 616 | /// Do not transfer a delegated loop's effects into its caller and thereby |
| 617 | /// conceal a second owner. Ordinary blocks/async expressions stay intact. |
| 618 | fn without_owned_loops(body: &str) -> String { |
| 619 | let mut chars: Vec<char> = body.chars().collect(); |
| 620 | for (_, open, close) in loop_bodies(&chars) { |
| 621 | for ch in &mut chars[open..=close] { |
| 622 | if *ch != '\n' { |
| 623 | *ch = ' '; |
| 624 | } |
| 625 | } |
| 626 | } |
| 627 | chars.into_iter().collect() |
| 628 | } |
| 629 | |
| 630 | /// Once a `self` call resolves locally, its method name is not evidence of a |
| 631 | /// provider call: inspect its real body. For example an actual snapshot |
| 632 | /// `*_before_complete` helper must not turn a human-operation dispatcher into |
| 633 | /// a model loop merely because it has the old broad suffix. Unresolved calls |
| 634 | /// keep the original conservative marker rule. |
| 635 | fn without_resolved_model_names(body: &str, receiver: &str, graph: &MethodGraph) -> String { |
| 636 | let mut edits = Vec::new(); |
| 637 | for (at, _) in body.match_indices('(') { |
| 638 | let before = body[..at].trim_end(); |
| 639 | let start = before |
| 640 | .rfind(|c: char| !is_ident_char(c)) |
| 641 | .map_or(0, |i| i + 1); |
| 642 | let name = &before[start..]; |
| 643 | if is_model_call(name) |
| 644 | && method_receiver(body, start) == Some("self") |
| 645 | && graph.contains_key(&(receiver.to_string(), name.to_string())) |
| 646 | { |
| 647 | edits.push((start, start + name.len())); |
| 648 | } |
| 649 | } |
| 650 | let mut result = body.as_bytes().to_vec(); |
| 651 | for (start, end) in edits { |
| 652 | result[start..end].fill(b' '); |
| 653 | } |
| 654 | String::from_utf8(result).unwrap() |
| 655 | } |
| 656 | |
| 657 | fn resolved_loop_body( |
| 658 | body: &str, |
| 659 | receiver: &str, |
| 660 | graph: &MethodGraph, |
| 661 | delegates: &BTreeSet<(String, String)>, |
| 662 | ) -> (String, Vec<String>) { |
| 663 | let mut resolved = without_resolved_model_names(body, receiver, graph); |
| 664 | let mut pending = vec![body.to_string()]; |
| 665 | let mut visited = BTreeSet::new(); |
| 666 | let mut ambiguous = Vec::new(); |
| 667 | while let Some(current) = pending.pop() { |
| 668 | for (at, _) in current.match_indices('(') { |
| 669 | let before = current[..at].trim_end(); |
| 670 | let name_start = before |
| 671 | .rfind(|c: char| !is_ident_char(c)) |
| 672 | .map_or(0, |i| i + 1); |
| 673 | let name = &before[name_start..]; |
| 674 | if method_receiver(¤t, name_start) != Some("self") |
| 675 | || !visited.insert(name.to_string()) |
| 676 | { |
| 677 | continue; |
| 678 | } |
| 679 | let key = (receiver.to_string(), name.to_string()); |
| 680 | if delegates.contains(&key) { |
| 681 | continue; |
| 682 | } |
| 683 | let Some(bodies) = graph.get(&key) else { |
| 684 | continue; // external/parent methods cannot be invented here |
| 685 | }; |
| 686 | if bodies.len() != 1 { |
| 687 | ambiguous.push(format!("{receiver}::{name}")); |
| 688 | } |
| 689 | // Conditional platform impls may have several definitions. Union |
| 690 | // their possible effects conservatively, then refuse ambiguity |
| 691 | // only if those effects could constitute a turn loop. |
| 692 | for body in bodies { |
| 693 | let contribution = without_owned_loops(body); |
| 694 | resolved.push_str(&without_resolved_model_names( |
| 695 | &contribution, |
| 696 | receiver, |
| 697 | graph, |
| 698 | )); |
| 699 | pending.push(contribution); |
| 700 | } |
| 701 | } |
| 702 | } |
| 703 | (resolved, ambiguous) |
| 704 | } |
| 705 | |
| 706 | /// Entry dispatchers that call an independently owned turn loop are |
| 707 | /// delegation, not phases. Find those owners by the same shape rule, then |
| 708 | /// propagate only this boundary through actual same-receiver call edges. |
| 709 | fn delegated_loop_boundaries(graph: &MethodGraph) -> BTreeSet<(String, String)> { |
| 710 | let mut boundaries = BTreeSet::new(); |
| 711 | for (key, bodies) in graph { |
| 712 | for body in bodies { |
| 713 | let chars: Vec<char> = body.chars().collect(); |
| 714 | for (_, open, close) in loop_bodies(&chars) { |
| 715 | let direct: String = chars[open..=close].iter().collect(); |
| 716 | let (resolved, _) = resolved_loop_body(&direct, &key.0, graph, &BTreeSet::new()); |
| 717 | if drives_a_model(&resolved) |
| 718 | && dispatches_tool_calls(&resolved) |
| 719 | && assembles_prompt_history(&resolved) |
| 720 | { |
| 721 | boundaries.insert(key.clone()); |
| 722 | } |
| 723 | } |
| 724 | } |
| 725 | } |
| 726 | loop { |
| 727 | let before = boundaries.len(); |
| 728 | for (key, bodies) in graph { |
| 729 | if bodies.iter().any(|body| { |
| 730 | called_identifiers(body).any(|name| { |
| 731 | body.match_indices(name) |
| 732 | .any(|(at, _)| method_receiver(body, at) == Some("self")) |
| 733 | && boundaries.contains(&(key.0.clone(), name.to_string())) |
| 734 | }) |
| 735 | }) { |
| 736 | boundaries.insert(key.clone()); |
| 737 | } |
| 738 | } |
| 739 | if boundaries.len() == before { |
| 740 | return boundaries; |
| 741 | } |
| 742 | } |
| 743 | } |
| 744 | |
| 745 | /// Find every turn loop by shape, following only resolved same-receiver local |
| 746 | /// calls in its declared source closure. No method name is an exemption. |
| 747 | fn detect_turn_loops_with_graph( |
| 748 | src: &str, |
| 749 | rel_path: &str, |
| 750 | graph: &MethodGraph, |
| 751 | ) -> Vec<TurnLoopSite> { |
| 752 | let prepared = sanitize(&strip_cfg_test(src)); |
| 753 | let methods = local_methods(&prepared); |
| 754 | let delegates = delegated_loop_boundaries(graph); |
| 755 | let chars: Vec<char> = prepared.chars().collect(); |
| 756 | let mut sites: Vec<TurnLoopSite> = Vec::new(); |
| 757 | for (keyword, open, close) in loop_bodies(&chars) { |
| 758 | let direct: String = chars[open..=close.max(open)].iter().collect(); |
| 759 | let (body, ambiguous) = methods |
| 760 | .iter() |
| 761 | .find(|m| m.open <= open && close <= m.close) |
| 762 | .map_or_else( |
| 763 | || (direct.clone(), Vec::new()), |
| 764 | |m| resolved_loop_body(&direct, &m.receiver, graph, &delegates), |
| 765 | ); |
| 766 | if !(drives_a_model(&body) |
| 767 | && dispatches_tool_calls(&body) |
| 768 | && assembles_prompt_history(&body)) |
| 769 | { |
| 770 | continue; |
| 771 | } |
| 772 | assert!( |
| 773 | ambiguous.is_empty(), |
| 774 | "ambiguous local turn-loop phase resolution: {ambiguous:?}" |
| 775 | ); |
| 776 | let owner = enclosing_fn(&chars, open); |
| 777 | let line = chars[..open].iter().filter(|c| **c == '\n').count() + 1; |
| 778 | if sites.iter().any(|s| s.owner == owner) { |
| 779 | continue; |
| 780 | } |
| 781 | sites.push(TurnLoopSite { |
| 782 | path: rel_path.to_string(), |
| 783 | owner, |
| 784 | line, |
| 785 | keyword, |
| 786 | }); |
| 787 | } |
| 788 | sites |
| 789 | } |
| 790 | |
| 791 | fn detect_turn_loops(src: &str, rel_path: &str) -> Vec<TurnLoopSite> { |
| 792 | detect_turn_loops_with_graph(src, rel_path, &method_graph(&[src.to_string()])) |
| 793 | } |
| 794 | |
| 795 | /// Follow ordinary literal out-of-line modules, not every file in a directory. |
| 796 | /// This is a finite local closure. Test-only declarations are blanked first; |
| 797 | /// missing declared files fail instead of silently dropping a phase. |
| 798 | fn local_module_sources(file: &Path, seen: &mut BTreeSet<PathBuf>, out: &mut Vec<String>) { |
| 799 | let file = file.canonicalize().expect("declared module source exists"); |
| 800 | if !seen.insert(file.clone()) { |
| 801 | return; |
| 802 | } |
| 803 | let source = std::fs::read_to_string(&file).expect("read declared module source"); |
| 804 | let prepared = sanitize(&strip_cfg_test(&source)); |
| 805 | let base = if matches!( |
| 806 | file.file_name().and_then(|n| n.to_str()), |
| 807 | Some("lib.rs" | "main.rs" | "mod.rs") |
| 808 | ) { |
| 809 | file.parent().unwrap().to_path_buf() |
| 810 | } else { |
| 811 | file.with_extension("") |
| 812 | }; |
| 813 | for (at, _) in prepared.match_indices("mod ") { |
| 814 | if at > 0 && is_ident_char(prepared[..at].chars().next_back().unwrap()) { |
| 815 | continue; |
| 816 | } |
| 817 | // Inline-module declarations need their own lexical directory and |
| 818 | // are not folded into this file's local method namespace. |
| 819 | if prepared[..at].chars().fold(0isize, |depth, ch| match ch { |
| 820 | '{' => depth + 1, |
| 821 | '}' => depth - 1, |
| 822 | _ => depth, |
| 823 | }) != 0 |
| 824 | { |
| 825 | continue; |
| 826 | } |
| 827 | let rest = prepared[at + 4..].trim_start(); |
| 828 | let end = rest.find(|c: char| !is_ident_char(c)).unwrap_or(rest.len()); |
| 829 | if end == 0 || !rest[end..].trim_start().starts_with(';') { |
| 830 | continue; |
| 831 | } |
| 832 | // An explicit path is not guessed as a conventional module. Leave it |
| 833 | // unresolved so the required owner's stale-entry assertion can fail. |
| 834 | let line_prefix = prepared[..at] |
| 835 | .rsplit_once([';', '{', '}']) |
| 836 | .map_or(&prepared[..at], |(_, p)| p); |
| 837 | if line_prefix.contains("path") { |
| 838 | continue; |
| 839 | } |
| 840 | let named = base.join(format!("{}.rs", &rest[..end])); |
| 841 | let directory = base.join(&rest[..end]).join("mod.rs"); |
| 842 | let child = match (named.is_file(), directory.is_file()) { |
| 843 | (true, false) => named, |
| 844 | (false, true) => directory, |
| 845 | _ => panic!( |
| 846 | "missing or ambiguous declared module {} in {}", |
| 847 | &rest[..end], |
| 848 | file.display() |
| 849 | ), |
| 850 | }; |
| 851 | local_module_sources(&child, seen, out); |
| 852 | } |
| 853 | out.push(source); |
| 854 | } |
| 855 | |
| 856 | // --------------------------------------------------------------------------- |
| 857 | // Source discovery |
| 858 | // --------------------------------------------------------------------------- |
| 859 | |
| 860 | fn workspace_root() -> PathBuf { |
| 861 | // crates/core/tests -> crates/core -> crates -> <root> |
| 862 | Path::new(env!("CARGO_MANIFEST_DIR")) |
| 863 | .ancestors() |
| 864 | .nth(2) |
| 865 | .expect("workspace root above crates/core") |
| 866 | .to_path_buf() |
| 867 | } |
| 868 | |
| 869 | fn rust_sources(dir: &Path, out: &mut Vec<PathBuf>) { |
| 870 | let Ok(entries) = std::fs::read_dir(dir) else { |
| 871 | return; |
| 872 | }; |
| 873 | for entry in entries.flatten() { |
| 874 | let path = entry.path(); |
| 875 | let name = entry.file_name(); |
| 876 | let name = name.to_string_lossy(); |
| 877 | if path.is_dir() { |
| 878 | if name == "target" || name == ".git" || name == "node_modules" { |
| 879 | continue; |
| 880 | } |
| 881 | rust_sources(&path, out); |
| 882 | } else if path.extension().is_some_and(|e| e == "rs") { |
| 883 | out.push(path); |
| 884 | } |
| 885 | } |
| 886 | } |
| 887 | |
| 888 | /// Shipped source only: test, bench, and example trees drive rounds on |
| 889 | /// purpose. |
| 890 | fn is_shipped_source(rel: &Path) -> bool { |
| 891 | let components: Vec<String> = rel |
| 892 | .components() |
| 893 | .map(|c| c.as_os_str().to_string_lossy().into_owned()) |
| 894 | .collect(); |
| 895 | let Some((file, dirs)) = components.split_last() else { |
| 896 | return false; |
| 897 | }; |
| 898 | if dirs |
| 899 | .iter() |
| 900 | .any(|d| d == "tests" || d == "benches" || d == "examples") |
| 901 | { |
| 902 | return false; |
| 903 | } |
| 904 | !(file == "tests.rs" || file.ends_with("_tests.rs")) |
| 905 | } |
| 906 | |
| 907 | fn rel_slash(root: &Path, path: &Path) -> String { |
| 908 | path.strip_prefix(root) |
| 909 | .unwrap_or(path) |
| 910 | .components() |
| 911 | .map(|c| c.as_os_str().to_string_lossy().into_owned()) |
| 912 | .collect::<Vec<_>>() |
| 913 | .join("/") |
| 914 | } |
| 915 | |
| 916 | // --------------------------------------------------------------------------- |
| 917 | // Tests |
| 918 | // --------------------------------------------------------------------------- |
| 919 | |
| 920 | #[test] |
| 921 | fn workspace_declares_exactly_one_turn_loop() { |
| 922 | let root = workspace_root(); |
| 923 | let crates = root.join("crates"); |
| 924 | assert!(crates.is_dir(), "expected {} to exist", crates.display()); |
| 925 | |
| 926 | let mut files = Vec::new(); |
| 927 | rust_sources(&crates, &mut files); |
| 928 | assert!( |
| 929 | files.len() > 100, |
| 930 | "source scan found too few files to trust" |
| 931 | ); |
| 932 | |
| 933 | let mut scanned = 0usize; |
| 934 | let mut sites: Vec<TurnLoopSite> = Vec::new(); |
| 935 | for file in &files { |
| 936 | let rel = file.strip_prefix(&root).unwrap_or(file).to_path_buf(); |
| 937 | if !is_shipped_source(&rel) { |
| 938 | continue; |
| 939 | } |
| 940 | let Ok(text) = std::fs::read_to_string(file) else { |
| 941 | continue; |
| 942 | }; |
| 943 | scanned += 1; |
| 944 | let prepared = sanitize(&strip_cfg_test(&text)); |
| 945 | let chars: Vec<char> = prepared.chars().collect(); |
| 946 | let needs_local_closure = loop_bodies(&chars).iter().any(|(_, open, close)| { |
| 947 | let body: String = chars[*open..=*close].iter().collect(); |
| 948 | body.contains("self") |
| 949 | }); |
| 950 | if needs_local_closure { |
| 951 | let mut sources = Vec::new(); |
| 952 | local_module_sources(file, &mut BTreeSet::new(), &mut sources); |
| 953 | let resolved = std::panic::catch_unwind(|| { |
| 954 | detect_turn_loops_with_graph( |
| 955 | &text, |
| 956 | &rel_slash(&root, file), |
| 957 | &method_graph(&sources), |
| 958 | ) |
| 959 | }) |
| 960 | .unwrap_or_else(|_| panic!("local turn-loop resolution failed in {}", file.display())); |
| 961 | sites.extend(resolved); |
| 962 | } else { |
| 963 | sites.extend(detect_turn_loops(&text, &rel_slash(&root, file))); |
| 964 | } |
| 965 | } |
| 966 | assert!( |
| 967 | scanned > 100, |
| 968 | "only {scanned} shipped sources scanned — the exclusion rules are \ |
| 969 | swallowing the workspace, so a pass here proves nothing" |
| 970 | ); |
| 971 | sites.sort(); |
| 972 | |
| 973 | let allowed: BTreeSet<(&str, &str)> = ALLOWED_TURN_LOOPS |
| 974 | .iter() |
| 975 | .map(|entry| (entry.path, entry.owner)) |
| 976 | .collect(); |
| 977 | |
| 978 | let unlisted: Vec<&TurnLoopSite> = sites |
| 979 | .iter() |
| 980 | .filter(|site| !allowed.contains(&(site.path.as_str(), site.owner.as_str()))) |
| 981 | .collect(); |
| 982 | assert!( |
| 983 | unlisted.is_empty(), |
| 984 | "found {} turn loop(s) that are not on ALLOWED_TURN_LOOPS:\n{}\n\n\ |
| 985 | A turn loop is a loop that, in one iteration, drives a model stream, \ |
| 986 | dispatches tool calls, and appends to its own prompt history. There \ |
| 987 | is supposed to be exactly one of those — `Engine::run_turn`. If you \ |
| 988 | are migrating the runtime, move the loop that exists instead of \ |
| 989 | adding another beside it. If this genuinely must exist for now, add \ |
| 990 | a named, documented ALLOWED_TURN_LOOPS entry citing the issue that \ |
| 991 | deletes it, so the exception is visible and has to be renewed.", |
| 992 | unlisted.len(), |
| 993 | unlisted |
| 994 | .iter() |
| 995 | .map(|s| format!( |
| 996 | " - {}:{} (`{}`, `{}` loop)", |
| 997 | s.path, s.line, s.owner, s.keyword |
| 998 | )) |
| 999 | .collect::<Vec<_>>() |
| 1000 | .join("\n"), |
| 1001 | ); |
| 1002 | |
| 1003 | // The allowlist is read back: an entry whose loop is gone (or renamed, or |
| 1004 | // moved) fails, so no exception outlives its reason unnoticed. |
| 1005 | let detected: BTreeSet<(&str, &str)> = sites |
| 1006 | .iter() |
| 1007 | .map(|site| (site.path.as_str(), site.owner.as_str())) |
| 1008 | .collect(); |
| 1009 | let stale: Vec<&AllowedTurnLoop> = ALLOWED_TURN_LOOPS |
| 1010 | .iter() |
| 1011 | .filter(|entry| !detected.contains(&(entry.path, entry.owner))) |
| 1012 | .collect(); |
| 1013 | assert!( |
| 1014 | stale.is_empty(), |
| 1015 | "ALLOWED_TURN_LOOPS has {} entr(y/ies) with no matching turn loop in \ |
| 1016 | the tree:\n{}\n\n\ |
| 1017 | Either the loop was deleted (good — delete the entry too), or it \ |
| 1018 | moved/was renamed (update the entry), or the detector stopped seeing \ |
| 1019 | it, which is the #6242 failure all over again and must be fixed \ |
| 1020 | rather than papered over.", |
| 1021 | stale.len(), |
| 1022 | stale |
| 1023 | .iter() |
| 1024 | .map(|e| format!(" - {} :: {} — {}", e.path, e.owner, e.why)) |
| 1025 | .collect::<Vec<_>>() |
| 1026 | .join("\n"), |
| 1027 | ); |
| 1028 | } |
| 1029 | |
| 1030 | /// The detector must find a turn loop it has never been told the name of. |
| 1031 | /// |
| 1032 | /// This is the regression for #6242: the old guard matched `run_turn` and was |
| 1033 | /// green for as long as `run_agentic_prompt_turn` existed. If the shape match |
| 1034 | /// ever degrades back into a name match, this fails. |
| 1035 | #[test] |
| 1036 | fn detector_sees_a_renamed_turn_loop() { |
| 1037 | let src = r#" |
| 1038 | async fn absolutely_not_called_run_turn(&mut self) -> Result<()> { |
| 1039 | let mut messages = self.history.clone(); |
| 1040 | loop { |
| 1041 | let stream = self.client.create_message_stream(request).await?; |
| 1042 | let (text, calls) = self.consume(stream).await?; |
| 1043 | messages.push(Message::assistant(text)); |
| 1044 | if calls.is_empty() { |
| 1045 | return Ok(()); |
| 1046 | } |
| 1047 | let results = self.execute_tool_calls(calls).await?; |
| 1048 | messages.extend(results); |
| 1049 | } |
| 1050 | } |
| 1051 | "#; |
| 1052 | let sites = detect_turn_loops(src, "crates/whatever/src/sneaky.rs"); |
| 1053 | assert_eq!( |
| 1054 | sites.len(), |
| 1055 | 1, |
| 1056 | "shape detector missed a turn loop that avoids the name `run_turn`: {sites:#?}" |
| 1057 | ); |
| 1058 | assert_eq!(sites[0].owner, "absolutely_not_called_run_turn"); |
| 1059 | |
| 1060 | // …and must not fire on a loop that only does part of the job. |
| 1061 | let not_a_turn_loop = r#" |
| 1062 | async fn render(&mut self) -> Result<()> { |
| 1063 | for event in events { |
| 1064 | messages.push(event.text); |
| 1065 | self.paint(event); |
| 1066 | } |
| 1067 | Ok(()) |
| 1068 | } |
| 1069 | "#; |
| 1070 | assert!( |
| 1071 | detect_turn_loops(not_a_turn_loop, "crates/whatever/src/ui.rs").is_empty(), |
| 1072 | "shape detector fired on a loop that never drives a model or tools" |
| 1073 | ); |
| 1074 | } |
| 1075 | |
| 1076 | /// #6511: the two spellings that hid the sub-agent and RLM loops from the |
| 1077 | /// suffix-only marker must both be seen. |
| 1078 | #[test] |
| 1079 | fn detector_sees_prefix_spelled_model_calls_and_repl_rounds() { |
| 1080 | // Sub-agent shape: a `request_…model…` wrapper plus a tool runner. |
| 1081 | let subagent = r#" |
| 1082 | async fn child_loop(&mut self) { |
| 1083 | loop { |
| 1084 | let api = request_subagent_model_response_with_retries(&client, request).await; |
| 1085 | messages.push(api.message); |
| 1086 | let output = run_tool_with_person_aware_timeout(call).await; |
| 1087 | messages.push(output); |
| 1088 | } |
| 1089 | } |
| 1090 | "#; |
| 1091 | let sites = detect_turn_loops(subagent, "crates/whatever/src/child.rs"); |
| 1092 | assert_eq!(sites.len(), 1, "missed the sub-agent spelling: {sites:#?}"); |
| 1093 | assert_eq!(sites[0].owner, "child_loop"); |
| 1094 | |
| 1095 | // RLM shape: `create_message_boxed` plus a REPL code round. |
| 1096 | let rlm = r#" |
| 1097 | async fn recursive_loop(client: Arc<dyn Client>) { |
| 1098 | for iteration in 0..LIMIT { |
| 1099 | let response = client.create_message_boxed(request).await; |
| 1100 | let round = repl.run(&code, Some(&bridge)).await; |
| 1101 | messages.push(metadata(round)); |
| 1102 | } |
| 1103 | } |
| 1104 | "#; |
| 1105 | let sites = detect_turn_loops(rlm, "crates/whatever/src/rlm.rs"); |
| 1106 | assert_eq!(sites.len(), 1, "missed the RLM spelling: {sites:#?}"); |
| 1107 | assert_eq!(sites[0].owner, "recursive_loop"); |
| 1108 | |
| 1109 | // A REPL round with no model call is not a turn loop. |
| 1110 | let replay = r#" |
| 1111 | async fn replay(&mut self) { |
| 1112 | for block in blocks { |
| 1113 | let round = repl.run(&block.code, None).await; |
| 1114 | history.push(round.stdout); |
| 1115 | } |
| 1116 | } |
| 1117 | "#; |
| 1118 | assert!( |
| 1119 | detect_turn_loops(replay, "crates/whatever/src/replay.rs").is_empty(), |
| 1120 | "a REPL replay that never calls a model is not a turn loop" |
| 1121 | ); |
| 1122 | } |
| 1123 | |
| 1124 | #[test] |
| 1125 | fn core_does_not_reintroduce_a_placeholder_engine_module() { |
| 1126 | let core_src = Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); |
| 1127 | assert!( |
| 1128 | !core_src.join("engine").exists(), |
| 1129 | "crates/core/src/engine/ is back. It was removed in v0.9.11 because it \ |
| 1130 | emitted TurnComplete without calling a model and had no consumers; a \ |
| 1131 | boundary type that does real work belongs in a named module, not a \ |
| 1132 | second `engine`." |
| 1133 | ); |
| 1134 | } |
| 1135 | |
| 1136 | #[test] |
| 1137 | fn detector_follows_resolved_phases_across_declared_modules() { |
| 1138 | let root = r#"impl DifferentName { async fn outer(&mut self) { |
| 1139 | loop { self.request_phase().await; self.apply_phase().await; } |
| 1140 | } }"#; |
| 1141 | let request = r#"use super::*; impl DifferentName { async fn request_phase(&mut self) { |
| 1142 | messages.push(input); client.create_message_stream(request).await; |
| 1143 | } }"#; |
| 1144 | let apply = r#"use super::*; impl DifferentName { async fn apply_phase(&mut self) { |
| 1145 | self.execute_tool_calls(calls).await; |
| 1146 | } }"#; |
| 1147 | let graph = method_graph(&[request.into(), apply.into(), root.into()]); |
| 1148 | let sites = detect_turn_loops_with_graph(root, "arbitrary.rs", &graph); |
| 1149 | assert_eq!(sites.len(), 1); |
| 1150 | assert_eq!(sites[0].owner, "outer"); |
| 1151 | // A missing phase cannot produce a green approximation of the owner. |
| 1152 | let missing = method_graph(&[request.into(), root.into()]); |
| 1153 | assert!(detect_turn_loops_with_graph(root, "arbitrary.rs", &missing).is_empty()); |
| 1154 | } |
| 1155 | |
| 1156 | #[test] |
| 1157 | fn detector_local_call_cycles_are_finite_and_do_not_join_unrelated_impls() { |
| 1158 | let src = r#" |
| 1159 | impl A { |
| 1160 | async fn renamed(&mut self) { loop { self.first().await; } } |
| 1161 | async fn first(&mut self) { self.second().await; } |
| 1162 | async fn second(&mut self) { self.first().await; |
| 1163 | client.create_message_stream(req).await; |
| 1164 | self.execute_tool_calls(calls).await; messages.push(msg); |
| 1165 | } |
| 1166 | } |
| 1167 | impl B { fn first(&mut self) { history.push(msg); } } |
| 1168 | "#; |
| 1169 | let sites = detect_turn_loops(src, "cycles.rs"); |
| 1170 | assert_eq!(sites.len(), 1); |
| 1171 | assert_eq!(sites[0].owner, "renamed"); |
| 1172 | let unrelated = r#" |
| 1173 | impl A { fn outer(&mut self) { loop { self.first(); } } |
| 1174 | fn first(&mut self) { messages.push(msg); } } |
| 1175 | impl B { fn first(&mut self) { |
| 1176 | client.create_message_stream(req); execute_tool_calls(calls); |
| 1177 | } } |
| 1178 | "#; |
| 1179 | assert!(detect_turn_loops(unrelated, "unrelated.rs").is_empty()); |
| 1180 | } |
| 1181 | |
| 1182 | #[test] |
| 1183 | fn detector_does_not_hide_two_phased_owners_or_a_foreign_inline_loop() { |
| 1184 | let src = r#"impl Owner { |
| 1185 | fn one(&mut self) { loop { self.model(); self.results(); } } |
| 1186 | fn two(&mut self) { loop { self.model(); self.results(); } } |
| 1187 | fn model(&mut self) { client.create_message_stream(req); } |
| 1188 | fn results(&mut self) { execute_tool_calls(calls); history.push(msg); } |
| 1189 | } |
| 1190 | fn foreign() { loop { client.create_message_stream(req); |
| 1191 | execute_tool_calls(calls); history.push(msg); } } |
| 1192 | "#; |
| 1193 | let sites = detect_turn_loops(src, "more_than_one.rs"); |
| 1194 | let owners: BTreeSet<&str> = sites.iter().map(|s| s.owner.as_str()).collect(); |
| 1195 | assert_eq!(owners, BTreeSet::from(["one", "two", "foreign"])); |
| 1196 | } |
| 1197 | |
| 1198 | #[test] |
| 1199 | fn detector_does_not_transfer_a_delegated_loop_to_its_caller() { |
| 1200 | let src = r#"impl Owner { |
| 1201 | fn transport(&mut self) { loop { self.other_owner(); } } |
| 1202 | fn other_owner(&mut self) { loop { self.request(); self.results(); } } |
| 1203 | fn request(&mut self) { client.create_message_stream(req); } |
| 1204 | fn results(&mut self) { execute_tool_calls(calls); history.push(msg); } |
| 1205 | }"#; |
| 1206 | let sites = detect_turn_loops(src, "delegated.rs"); |
| 1207 | assert_eq!(sites.len(), 1); |
| 1208 | assert_eq!(sites[0].owner, "other_owner"); |
| 1209 | } |
| 1210 | |
| 1211 | #[test] |
| 1212 | fn detector_ignores_test_only_effects_and_refuses_ambiguous_methods() { |
| 1213 | let root = r#"impl Owner { fn outer(&mut self) { loop { self.phase(); } } } |
| 1214 | #[cfg(test)] impl Owner { fn phase(&mut self) { |
| 1215 | client.create_message_stream(req); execute_tool_calls(calls); messages.push(msg); |
| 1216 | } }"#; |
| 1217 | assert!(detect_turn_loops(root, "tests.rs").is_empty()); |
| 1218 | let one = "use super::*; impl Owner { fn phase(&mut self) { client.create_message_stream(req); execute_tool_calls(calls); messages.push(msg); } }"; |
| 1219 | let graph = method_graph(&[one.into(), one.into(), root.into()]); |
| 1220 | assert!( |
| 1221 | std::panic::catch_unwind(|| detect_turn_loops_with_graph(root, "ambiguous.rs", &graph)) |
| 1222 | .is_err() |
| 1223 | ); |
| 1224 | } |
| 1225 | |
| 1226 | #[test] |
| 1227 | fn local_module_closure_reads_only_declared_production_sources() { |
| 1228 | let dir = std::env::temp_dir().join(format!( |
| 1229 | "cw-turn-phases-{}-{}", |
| 1230 | std::process::id(), |
| 1231 | std::time::SystemTime::now() |
| 1232 | .duration_since(std::time::UNIX_EPOCH) |
| 1233 | .unwrap() |
| 1234 | .as_nanos() |
| 1235 | )); |
| 1236 | std::fs::create_dir_all(dir.join("outer")).unwrap(); |
| 1237 | let root = dir.join("outer.rs"); |
| 1238 | std::fs::write(&root, "mod phase; #[cfg(test)] mod absent; impl Owner { fn outer(&mut self) { loop { self.phase(); } } }").unwrap(); |
| 1239 | std::fs::write(dir.join("outer/phase.rs"), "use super::*; impl Owner { fn phase(&mut self) { client.create_message_stream(req); execute_tool_calls(calls); messages.push(msg); } }").unwrap(); |
| 1240 | std::fs::write( |
| 1241 | dir.join("outer/undeclared.rs"), |
| 1242 | "impl Owner { fn phase(&mut self) { } }", |
| 1243 | ) |
| 1244 | .unwrap(); |
| 1245 | let mut sources = Vec::new(); |
| 1246 | local_module_sources(&root, &mut BTreeSet::new(), &mut sources); |
| 1247 | assert_eq!(sources.len(), 2); |
| 1248 | assert_eq!( |
| 1249 | detect_turn_loops_with_graph(&sources[1], "outer.rs", &method_graph(&sources)).len(), |
| 1250 | 1 |
| 1251 | ); |
| 1252 | std::fs::remove_dir_all(dir).unwrap(); |
| 1253 | } |
| 1254 | |
| 1255 | #[test] |
| 1256 | fn gated_fields_and_initializers_do_not_unbalance_the_local_graph() { |
| 1257 | let src = r#"struct Owner { |
| 1258 | #[cfg(test)] hidden: std::collections::BTreeMap<String, String>, |
| 1259 | actual: bool, |
| 1260 | } |
| 1261 | impl Owner { |
| 1262 | fn new() -> Self { Self { #[cfg(test)] hidden: value, actual: true } } |
| 1263 | #[cfg(test)] fn fixture(&self, first: bool, second: bool) { |
| 1264 | let text = "{not a brace}"; |
| 1265 | } |
| 1266 | fn outer(&mut self) { loop { self.phase(); } } |
| 1267 | fn phase(&mut self) { client.create_message_stream(req); execute_tool_calls(calls); history.push(msg); } |
| 1268 | }"#; |
| 1269 | let sites = detect_turn_loops(src, "field.rs"); |
| 1270 | assert_eq!(sites.len(), 1); |
| 1271 | assert_eq!(sites[0].owner, "outer"); |
| 1272 | } |
| 1273 | |
| 1274 | #[test] |
| 1275 | fn resolved_completion_named_snapshot_helper_is_not_a_model_call() { |
| 1276 | let src = r#"impl Owner { |
| 1277 | fn operations(&mut self) { loop { self.snapshot_before_complete(); self.results(); } } |
| 1278 | fn snapshot_before_complete(&mut self) { persist_snapshot(); } |
| 1279 | fn results(&mut self) { execute_tool_calls(calls); history.push(msg); } |
| 1280 | }"#; |
| 1281 | assert!(detect_turn_loops(src, "completion.rs").is_empty()); |
| 1282 | // An unresolved provider helper remains conservatively visible, and a |
| 1283 | // locally resolved provider helper is followed to its real client call. |
| 1284 | let unknown = src.replace( |
| 1285 | "fn snapshot_before_complete(&mut self) { persist_snapshot(); }", |
| 1286 | "", |
| 1287 | ); |
| 1288 | assert_eq!(detect_turn_loops(&unknown, "unknown.rs").len(), 1); |
| 1289 | let actual = src.replace("persist_snapshot();", "client.create_message_stream(req);"); |
| 1290 | assert_eq!(detect_turn_loops(&actual, "actual.rs").len(), 1); |
| 1291 | } |
| 1292 | |
| 1293 | #[test] |
| 1294 | fn same_named_child_type_or_unrelated_import_cannot_impersonate_a_phase() { |
| 1295 | let root = "impl Owner { fn outer(&mut self) { loop { self.phase(); } } }"; |
| 1296 | let effects = "impl Owner { fn phase(&mut self) { client.create_message_stream(req); execute_tool_calls(calls); history.push(msg); } }"; |
| 1297 | let shadow = format!("use super::*; struct Owner; {effects}"); |
| 1298 | let unrelated = format!("use unrelated::Owner; {effects}"); |
| 1299 | for child in [shadow, unrelated, effects.to_string()] { |
| 1300 | let graph = method_graph(&[child, root.to_string()]); |
| 1301 | assert!(detect_turn_loops_with_graph(root, "owner.rs", &graph).is_empty()); |
| 1302 | } |
| 1303 | // False-green counterpart: an unrelated child's non-provider method |
| 1304 | // cannot erase a real unresolved model marker on the root receiver. |
| 1305 | let actual_root = "impl Owner { fn outer(&mut self) { loop { self.create_message_stream(req); execute_tool_calls(calls); history.push(msg); } } }"; |
| 1306 | for child in [ |
| 1307 | "use super::*; struct Owner; impl Owner { fn create_message_stream(&mut self) { } }", |
| 1308 | "use unrelated::Owner; impl Owner { fn create_message_stream(&mut self) { } }", |
| 1309 | ] { |
| 1310 | let graph = method_graph(&[child.to_string(), actual_root.to_string()]); |
| 1311 | assert_eq!( |
| 1312 | detect_turn_loops_with_graph(actual_root, "actual.rs", &graph).len(), |
| 1313 | 1 |
| 1314 | ); |
| 1315 | } |
| 1316 | for import in ["use super::*;", "use super::Owner;"] { |
| 1317 | let child = format!("{import} {effects}"); |
| 1318 | let graph = method_graph(&[child, root.to_string()]); |
| 1319 | assert_eq!( |
| 1320 | detect_turn_loops_with_graph(root, "owner.rs", &graph).len(), |
| 1321 | 1 |
| 1322 | ); |
| 1323 | } |
| 1324 | } |
| 1325 | |
| 1326 | #[test] |
| 1327 | fn rlm_cannot_reintroduce_client_only_authority_or_a_direct_provider_producer() { |
| 1328 | let root = Path::new(env!("CARGO_MANIFEST_DIR")) |
| 1329 | .parent() |
| 1330 | .unwrap() |
| 1331 | .join("tui/src"); |
| 1332 | for path in [ |
| 1333 | "rlm/bridge.rs", |
| 1334 | "rlm/turn.rs", |
| 1335 | "tools/rlm.rs", |
| 1336 | "core/engine/rlm_host.rs", |
| 1337 | ] { |
| 1338 | let source = std::fs::read_to_string(root.join(path)).unwrap(); |
| 1339 | let production = sanitize(&strip_cfg_test(&source)); |
| 1340 | for retired in [ |
| 1341 | "ModelClientRlmAdapter", |
| 1342 | "RlmLlmClient", |
| 1343 | "run_rlm_turn_impl", |
| 1344 | "create_message_boxed", |
| 1345 | "create_message_stream", |
| 1346 | ] { |
| 1347 | assert!( |
| 1348 | !production.contains(retired), |
| 1349 | "{path} reintroduced retired model authority {retired}" |
| 1350 | ); |
| 1351 | } |
| 1352 | } |
| 1353 | } |
| 1354 |