| 1 | |
| 2 | #[test] |
| 3 | fn review_failure_after_publication_preserves_all_usage_and_post_state() { |
| 4 | let mut usage = codewhale_models::Usage::default(); |
| 5 | for response_usage in [ |
| 6 | codewhale_models::Usage { |
| 7 | input_tokens: 21, |
| 8 | output_tokens: 5, |
| 9 | reasoning_tokens: Some(2), |
| 10 | ..Default::default() |
| 11 | }, |
| 12 | codewhale_models::Usage { |
| 13 | input_tokens: 34, |
| 14 | output_tokens: 8, |
| 15 | reasoning_tokens: Some(3), |
| 16 | ..Default::default() |
| 17 | }, |
| 18 | ] { |
| 19 | crate::tools::review::add_review_usage(&mut usage, &response_usage); |
| 20 | } |
| 21 | |
| 22 | for (publication, message) in [ |
| 23 | ( |
| 24 | ReviewPublication::Uncertain, |
| 25 | "PR review publication failed after request dispatch", |
| 26 | ), |
| 27 | ( |
| 28 | ReviewPublication::Posted, |
| 29 | "review posted but receipt write failed", |
| 30 | ), |
| 31 | ] { |
| 32 | let payload = review_failure_payload( |
| 33 | "fixture-provider", |
| 34 | "fixture-model", |
| 35 | &usage, |
| 36 | 2, |
| 37 | 2, |
| 38 | publication, |
| 39 | message, |
| 40 | ); |
| 41 | assert_eq!(payload["success"], false); |
| 42 | assert_eq!(payload["complete"], false); |
| 43 | assert_eq!(payload["publication"], publication.as_str()); |
| 44 | assert_eq!(payload["usage"]["input_tokens"], 55); |
| 45 | assert_eq!(payload["usage"]["output_tokens"], 13); |
| 46 | assert_eq!(payload["usage"]["reasoning_tokens"], 5); |
| 47 | assert!(payload.get("review").is_none()); |
| 48 | assert!(payload.get("receipt").is_none()); |
| 49 | } |
| 50 | } |
| 51 | |
| 52 | #[tokio::test] |
| 53 | async fn review_provider_flag_pins_route_for_multi_route_model() { |
| 54 | // A genuinely multi-route model: `shared-review-model` is the |
| 55 | // configured model of BOTH named routes below, while the active route |
| 56 | // (custom-a) does not offer it. (Two custom providers can never be |
| 57 | // ambiguous — the route inventory resolves only the active custom |
| 58 | // entry — so the multi-route pair has to be named providers.) |
| 59 | // Without `--provider`, an explicit `--model` lets cross-provider |
| 60 | // inventory inference run, and a model offered by more than one |
| 61 | // configured route hard-errors in `resolve_cli_auto_route` |
| 62 | // ("available from configured provider route(s): ..."). That error |
| 63 | // already tells the user to "Pass `--provider <provider>`" — until |
| 64 | // now `codewhale review` had no such flag to pass. |
| 65 | let mut config = custom_exec_config("custom-a"); |
| 66 | { |
| 67 | let providers = config.providers.as_mut().expect("providers"); |
| 68 | for entry in [&mut providers.deepseek, &mut providers.openrouter] { |
| 69 | *entry = crate::config::ProviderConfig { |
| 70 | model: Some("shared-review-model".to_string()), |
| 71 | api_key: Some("local-test-key".to_string()), |
| 72 | ..Default::default() |
| 73 | }; |
| 74 | } |
| 75 | } |
| 76 | |
| 77 | let inferred = review_args(&["codewhale", "review", "--model", "shared-review-model"]); |
| 78 | let (inferred_config, inferred_force) = |
| 79 | review_execution_route(&config, &inferred).expect("model-only route"); |
| 80 | assert!( |
| 81 | !inferred_force, |
| 82 | "an explicit model with no provider stays open to inventory inference" |
| 83 | ); |
| 84 | assert_eq!(inferred_config.provider.as_deref(), Some("custom-a")); |
| 85 | |
| 86 | // Without the flag the multi-route model must refuse to guess. |
| 87 | let err = resolve_cli_exec_route( |
| 88 | &inferred_config, |
| 89 | &resolve_review_model(&inferred_config, inferred.model.as_deref()), |
| 90 | "review diff", |
| 91 | inferred_force, |
| 92 | ) |
| 93 | .await |
| 94 | .expect_err("a model offered by two configured routes must hard-error"); |
| 95 | let message = err.to_string(); |
| 96 | assert!( |
| 97 | message.contains("available from configured provider route(s)"), |
| 98 | "unexpected error: {message}" |
| 99 | ); |
| 100 | assert!( |
| 101 | message.contains("deepseek") && message.contains("openrouter"), |
| 102 | "both candidate routes must be named: {message}" |
| 103 | ); |
| 104 | |
| 105 | // With the flag the same model pins to the named route and resolves. |
| 106 | let pinned = review_args(&[ |
| 107 | "codewhale", |
| 108 | "review", |
| 109 | "--provider", |
| 110 | "deepseek", |
| 111 | "--model", |
| 112 | "shared-review-model", |
| 113 | ]); |
| 114 | let (pinned_config, pinned_force) = |
| 115 | review_execution_route(&config, &pinned).expect("pinned route"); |
| 116 | assert!(pinned_force, "--provider makes the route authoritative"); |
| 117 | assert_eq!(pinned_config.provider.as_deref(), Some("deepseek")); |
| 118 | |
| 119 | let route = resolve_cli_exec_route( |
| 120 | &pinned_config, |
| 121 | &resolve_review_model(&pinned_config, pinned.model.as_deref()), |
| 122 | "review diff", |
| 123 | pinned_force, |
| 124 | ) |
| 125 | .await |
| 126 | .expect("pinned review route"); |
| 127 | let execution = config_for_cli_route(&pinned_config, &route).expect("admitted execution route"); |
| 128 | |
| 129 | assert_eq!(route.provider.provider, crate::config::ProviderKind::Deepseek); |
| 130 | assert_eq!(route.model, "shared-review-model"); |
| 131 | assert_eq!( |
| 132 | execution.active_provider_identity().unwrap().key.as_str(), |
| 133 | "deepseek", |
| 134 | "the review runs on the provider the flag named" |
| 135 | ); |
| 136 | |
| 137 | // Pinning a configured custom provider (the workflow's |
| 138 | // CODEWHALE_REVIEW_PROVIDER="my-proxy" case) is equally authoritative |
| 139 | // for the same multi-route model. |
| 140 | let pinned_custom = review_args(&[ |
| 141 | "codewhale", |
| 142 | "review", |
| 143 | "--provider", |
| 144 | "custom-b", |
| 145 | "--model", |
| 146 | "shared-review-model", |
| 147 | ]); |
| 148 | let (custom_config, custom_force) = |
| 149 | review_execution_route(&config, &pinned_custom).expect("pinned custom route"); |
| 150 | assert!(custom_force); |
| 151 | let custom_route = resolve_cli_exec_route( |
| 152 | &custom_config, |
| 153 | &resolve_review_model(&custom_config, pinned_custom.model.as_deref()), |
| 154 | "review diff", |
| 155 | custom_force, |
| 156 | ) |
| 157 | .await |
| 158 | .expect("pinned custom review route"); |
| 159 | let custom_execution = config_for_cli_route(&custom_config, &custom_route).expect("admitted execution route"); |
| 160 | |
| 161 | assert_eq!(custom_route.provider.provider, crate::config::ProviderKind::Custom); |
| 162 | assert_eq!(custom_route.model, "shared-review-model"); |
| 163 | assert_eq!( |
| 164 | custom_execution |
| 165 | .active_provider_identity() |
| 166 | .unwrap() |
| 167 | .key |
| 168 | .as_str(), |
| 169 | "custom-b", |
| 170 | "the review runs on the custom provider the flag named" |
| 171 | ); |
| 172 | } |
| 173 | |
| 174 | #[test] |
| 175 | fn review_provider_flag_rejects_unknown_provider() { |
| 176 | let config = custom_exec_config("custom-a"); |
| 177 | let args = review_args(&["codewhale", "review", "--provider", "not-a-provider"]); |
| 178 | |
| 179 | let err = review_execution_route(&config, &args) |
| 180 | .expect_err("unknown provider must fail before any diff is fetched"); |
| 181 | assert!( |
| 182 | err.to_string().contains("Unrecognized --provider"), |
| 183 | "unexpected error: {err}" |
| 184 | ); |
| 185 | } |
| 186 | |
| 187 | #[test] |
| 188 | fn review_without_provider_flag_keeps_configured_route_authoritative() { |
| 189 | let config = custom_exec_config("custom-a"); |
| 190 | let args = review_args(&["codewhale", "review"]); |
| 191 | |
| 192 | let (resolved, force) = review_execution_route(&config, &args).expect("default route"); |
| 193 | assert!( |
| 194 | force, |
| 195 | "the configured/default review route is authoritative" |
| 196 | ); |
| 197 | assert_eq!(resolved.provider.as_deref(), Some("custom-a")); |
| 198 | } |
| 199 | |
| 200 | fn review_issue( |
| 201 | severity: &str, |
| 202 | path: Option<&str>, |
| 203 | line: Option<u32>, |
| 204 | ) -> crate::tools::review::ReviewIssue { |
| 205 | crate::tools::review::ReviewIssue { |
| 206 | severity: severity.to_string(), |
| 207 | title: format!("{severity} finding"), |
| 208 | description: "detail".to_string(), |
| 209 | path: path.map(str::to_string), |
| 210 | line, |
| 211 | } |
| 212 | } |
| 213 | |
| 214 | fn review_suggestion( |
| 215 | path: Option<&str>, |
| 216 | line: Option<u32>, |
| 217 | replacement: Option<&str>, |
| 218 | ) -> crate::tools::review::ReviewSuggestion { |
| 219 | crate::tools::review::ReviewSuggestion { |
| 220 | path: path.map(str::to_string), |
| 221 | line, |
| 222 | start_line: None, |
| 223 | end_line: None, |
| 224 | suggestion: "Use the checked variant".to_string(), |
| 225 | replacement: replacement.map(str::to_string), |
| 226 | } |
| 227 | } |
| 228 | |
| 229 | fn review_with( |
| 230 | issues: Vec<crate::tools::review::ReviewIssue>, |
| 231 | suggestions: Vec<crate::tools::review::ReviewSuggestion>, |
| 232 | ) -> crate::tools::review::ReviewOutput { |
| 233 | crate::tools::review::ReviewOutput { |
| 234 | summary: "summary".to_string(), |
| 235 | issues, |
| 236 | suggestions, |
| 237 | overall_assessment: String::new(), |
| 238 | } |
| 239 | } |
| 240 | |
| 241 | /// Two hunks in one file: right lines 10..=12 and 40..=41. |
| 242 | /// |
| 243 | /// Built from a slice, not one string literal: a `\` continuation strips |
| 244 | /// the leading space that marks a context line. |
| 245 | fn review_test_diff() -> String { |
| 246 | [ |
| 247 | "diff --git a/crates/tui/src/lib.rs b/crates/tui/src/lib.rs", |
| 248 | "index 1111111..2222222 100644", |
| 249 | "--- a/crates/tui/src/lib.rs", |
| 250 | "+++ b/crates/tui/src/lib.rs", |
| 251 | "@@ -10,2 +10,3 @@ fn head() {", |
| 252 | " let a = 1;", |
| 253 | "+let b = a.unwrap();", |
| 254 | " let c = 2;", |
| 255 | "@@ -40,2 +40,2 @@ fn tail() {", |
| 256 | " let d = 3;", |
| 257 | "-let e = d;", |
| 258 | "+let e = d + 1;", |
| 259 | "", |
| 260 | ] |
| 261 | .join("\n") |
| 262 | } |
| 263 | |
| 264 | #[cfg(unix)] |
| 265 | #[test] |
| 266 | fn local_review_collects_raw_changes_without_running_diff_helpers() { |
| 267 | use std::os::unix::fs::PermissionsExt; |
| 268 | |
| 269 | for helper_kind in ["textconv", "external", "clean", "process"] { |
| 270 | for mode in ["working", "staged", "base", "path"] { |
| 271 | let workspace = tempfile::tempdir().unwrap(); |
| 272 | let path = workspace.path(); |
| 273 | let git = |args: &[&str]| { |
| 274 | let output = crate::dependencies::Git::command() |
| 275 | .expect("git") |
| 276 | .current_dir(path) |
| 277 | .args([ |
| 278 | "-c", |
| 279 | "core.hooksPath=/dev/null", |
| 280 | "-c", |
| 281 | "commit.gpgSign=false", |
| 282 | ]) |
| 283 | .args(args) |
| 284 | .output() |
| 285 | .unwrap(); |
| 286 | assert!( |
| 287 | output.status.success(), |
| 288 | "{args:?}: {}", |
| 289 | String::from_utf8_lossy(&output.stderr) |
| 290 | ); |
| 291 | String::from_utf8(output.stdout).unwrap() |
| 292 | }; |
| 293 | git(&["init", "-q"]); |
| 294 | git(&["config", "user.name", "Review fixture"]); |
| 295 | git(&["config", "user.email", "fixture@example.invalid"]); |
| 296 | std::fs::write( |
| 297 | path.join(".gitattributes"), |
| 298 | "*.txt diff=fixture filter=fixture=odd\n", |
| 299 | ) |
| 300 | .unwrap(); |
| 301 | for name in ["- source.txt", "other.txt"] { |
| 302 | std::fs::write(path.join(name), "before\n").unwrap(); |
| 303 | } |
| 304 | std::fs::write(path.join("binary.dat"), b"\0before").unwrap(); |
| 305 | git(&["add", "."]); |
| 306 | git(&["commit", "-qm", "before"]); |
| 307 | for name in ["- source.txt", "other.txt"] { |
| 308 | std::fs::write(path.join(name), "after\n").unwrap(); |
| 309 | } |
| 310 | std::fs::write(path.join("binary.dat"), b"\0after").unwrap(); |
| 311 | if matches!(mode, "staged" | "base") { |
| 312 | git(&["add", "."]); |
| 313 | } |
| 314 | if mode == "base" { |
| 315 | git(&["commit", "-qm", "after"]); |
| 316 | } |
| 317 | let script = match helper_kind { |
| 318 | "external" => "#!/bin/sh\nprintf touched > helper-marker\nprintf external-diff\n", |
| 319 | "clean" => "#!/bin/sh\nprintf touched > helper-marker\ncat\n", |
| 320 | "process" => "#!/bin/sh\nprintf touched > helper-marker\nexit 1\n", |
| 321 | _ => "#!/bin/sh\nprintf touched > helper-marker\ncat < \"$1\"\n", |
| 322 | }; |
| 323 | let helper = path.join("helper.sh"); |
| 324 | std::fs::write(&helper, script).unwrap(); |
| 325 | std::fs::set_permissions(&helper, std::fs::Permissions::from_mode(0o700)).unwrap(); |
| 326 | git(&[ |
| 327 | "config", |
| 328 | match helper_kind { |
| 329 | "external" => "diff.external", |
| 330 | "clean" => "filter.fixture=odd.clean", |
| 331 | "process" => "filter.fixture=odd.process", |
| 332 | _ => "diff.fixture.textconv", |
| 333 | }, |
| 334 | "./helper.sh", |
| 335 | ]); |
| 336 | if matches!(helper_kind, "clean" | "process") { |
| 337 | git(&["config", "filter.fixture=odd.required", "true"]); |
| 338 | } |
| 339 | let (review_flags, diff_flags): (&[&str], &[&str]) = match mode { |
| 340 | "staged" => (&["--staged"], &["--cached"]), |
| 341 | "base" => (&["--base", "HEAD^"], &["HEAD^...HEAD"]), |
| 342 | "path" => (&["--path=- source.txt"], &["--", "- source.txt"]), |
| 343 | _ => (&[], &[]), |
| 344 | }; |
| 345 | // Positive control: the same repository really can invoke its helper. |
| 346 | let mut baseline = vec!["diff", "--ext-diff"]; |
| 347 | baseline.extend_from_slice(diff_flags); |
| 348 | let baseline = crate::dependencies::Git::command() |
| 349 | .unwrap() |
| 350 | .current_dir(path) |
| 351 | .args(&baseline) |
| 352 | .output() |
| 353 | .unwrap(); |
| 354 | let marker = path.join("helper-marker"); |
| 355 | let conversion = matches!(helper_kind, "clean" | "process"); |
| 356 | let reads_worktree = matches!(mode, "working" | "path"); |
| 357 | assert_eq!( |
| 358 | marker.exists(), |
| 359 | !conversion || reads_worktree, |
| 360 | "positive control: {mode}, {helper_kind}" |
| 361 | ); |
| 362 | assert_eq!( |
| 363 | baseline.status.success(), |
| 364 | helper_kind != "process" || !reads_worktree |
| 365 | ); |
| 366 | if marker.exists() { |
| 367 | std::fs::remove_file(&marker).unwrap(); |
| 368 | } |
| 369 | |
| 370 | let mut argv = vec!["codewhale", "review"]; |
| 371 | argv.extend_from_slice(review_flags); |
| 372 | let mut args = review_args(&argv); |
| 373 | let diff = collect_diff(&args, None, path).unwrap(); |
| 374 | assert!(!marker.exists(), "{mode}, {helper_kind}"); |
| 375 | assert!(diff.contains("-before\n+after"), "{diff}"); |
| 376 | assert!(diff.contains("- source.txt")); |
| 377 | if mode == "path" { |
| 378 | assert!(!diff.contains("other.txt")); |
| 379 | assert!(!diff.contains("binary.dat")); |
| 380 | } else { |
| 381 | assert!(diff.contains("other.txt")); |
| 382 | assert!(diff.contains("Binary files") && diff.contains("binary.dat")); |
| 383 | } |
| 384 | args.max_chars = 1; |
| 385 | assert!( |
| 386 | collect_diff(&args, None, path) |
| 387 | .unwrap_err() |
| 388 | .to_string() |
| 389 | .contains("No review was run") |
| 390 | ); |
| 391 | assert!(!marker.exists()); |
| 392 | } |
| 393 | } |
| 394 | } |
| 395 | |
| 396 | #[test] |
| 397 | fn local_review_budget_rejects_changes_beyond_a_shared_prefix() { |
| 398 | let prefix = review_test_diff(); |
| 399 | let limit = prefix.chars().count(); |
| 400 | for tail in ["+safe_change();\n", "+dangerous_change();\n"] { |
| 401 | let diff = format!("{prefix}{tail}"); |
| 402 | let error = ensure_local_review_diff_fits(&diff, limit).unwrap_err(); |
| 403 | assert!(error.to_string().contains("No review was run")); |
| 404 | assert!( |
| 405 | error |
| 406 | .to_string() |
| 407 | .contains("no receipt was written or accepted") |
| 408 | ); |
| 409 | } |
| 410 | } |
| 411 | |
| 412 | #[test] |
| 413 | fn review_in_a_bare_repository_is_not_called_outside_git() { |
| 414 | let bare = tempfile::tempdir().expect("tempdir"); |
| 415 | let init = std::process::Command::new("git") |
| 416 | .args(["init", "--bare", "-q"]) |
| 417 | .current_dir(bare.path()) |
| 418 | .status() |
| 419 | .expect("git init --bare"); |
| 420 | assert!(init.success()); |
| 421 | let error = ensure_review_workspace_is_git_repo(bare.path()) |
| 422 | .expect_err("a bare repository has no work tree"); |
| 423 | let text = error.to_string(); |
| 424 | assert!(text.starts_with("Not inside a git work tree"), "{text}"); |
| 425 | } |
| 426 | |
| 427 | #[test] |
| 428 | fn review_outside_a_git_repository_says_so_in_one_line() { |
| 429 | let outside = tempfile::tempdir().expect("tempdir"); |
| 430 | let error = ensure_review_workspace_is_git_repo(outside.path()) |
| 431 | .expect_err("a plain directory is not a work tree"); |
| 432 | assert_eq!( |
| 433 | error.to_string(), |
| 434 | format!( |
| 435 | "Not inside a git repository (cwd: {})", |
| 436 | outside.path().display() |
| 437 | ) |
| 438 | ); |
| 439 | assert_eq!( |
| 440 | first_stderr_line("\nfatal: bad revision 'nope...HEAD'\nusage: git diff\n --stat\n"), |
| 441 | "fatal: bad revision 'nope...HEAD'" |
| 442 | ); |
| 443 | } |
| 444 | |
| 445 | #[test] |
| 446 | fn exec_resume_error_redacts_the_typed_id_and_names_the_list_command() { |
| 447 | let id = "sk-live-pasted-by-mistake"; |
| 448 | let text = exec_resume_load_error(id); |
| 449 | assert!(!text.contains(id), "{text}"); |
| 450 | assert!(text.contains("<redacted:"), "{text}"); |
| 451 | assert!( |
| 452 | text.ends_with("Run `codewhale sessions` to list ids."), |
| 453 | "{text}" |
| 454 | ); |
| 455 | } |
| 456 | |
| 457 | #[test] |
| 458 | fn doctor_shows_plain_value_errors_with_a_fix_and_hides_the_rest() { |
| 459 | let invalid = crate::config::Config { |
| 460 | verbosity: Some("chatty".to_string()), |
| 461 | ..Default::default() |
| 462 | } |
| 463 | .validate() |
| 464 | .expect_err("unknown verbosity"); |
| 465 | let wrapped = invalid.context("Failed to load config file /tmp/config.toml"); |
| 466 | let text = doctor_config_error_text(&wrapped); |
| 467 | assert_eq!( |
| 468 | text, |
| 469 | "doctor configuration validation failed: Invalid verbosity (value not shown): expected normal or concise.\nfix: codewhale config set verbosity normal (if a profile or managed config sets it, correct it there)" |
| 470 | ); |
| 471 | assert!(!text.contains("chatty"), "{text}"); |
| 472 | |
| 473 | let opaque = anyhow::anyhow!("TOML parse error near api_key = \"sk-live-secret\""); |
| 474 | let text = doctor_config_error_text(&opaque); |
| 475 | assert!(text.contains("details omitted"), "{text}"); |
| 476 | assert!(!text.contains("sk-live-secret"), "{text}"); |
| 477 | } |
| 478 | |
| 479 | #[test] |
| 480 | fn local_review_budget_counts_unicode_characters_without_cutting_input() { |
| 481 | let diff = format!("{}+鲸鱼\n", review_test_diff()); |
| 482 | let limit = diff.chars().count(); |
| 483 | assert!(diff.len() > limit); |
| 484 | ensure_local_review_diff_fits(&diff, limit).unwrap(); |
| 485 | assert!(ensure_local_review_diff_fits(&diff, limit - 1).is_err()); |
| 486 | } |
| 487 | |
| 488 | #[test] |
| 489 | fn inline_review_comments_keep_only_hunk_locatable_issues() { |
| 490 | let review = review_with( |
| 491 | vec![ |
| 492 | review_issue("error", Some("./crates/tui/src/lib.rs"), Some(11)), |
| 493 | review_issue("warning", Some("docs/NOT_IN_DIFF.md"), Some(3)), |
| 494 | review_issue("info", Some("crates/tui/src/lib.rs"), None), |
| 495 | review_issue("error", None, Some(7)), |
| 496 | ], |
| 497 | Vec::new(), |
| 498 | ); |
| 499 | let comments = plan_inline_review_comments(&review, &review_test_diff()).comments; |
| 500 | // GitHub rejects the entire review (422) when a comment's path is not |
| 501 | // part of the diff, and inline comments need a line to anchor to. |
| 502 | assert_eq!(comments.len(), 1); |
| 503 | assert_eq!(comments[0]["path"], "crates/tui/src/lib.rs"); |
| 504 | assert_eq!(comments[0]["line"], 11); |
| 505 | assert_eq!(comments[0]["side"], "RIGHT"); |
| 506 | } |
| 507 | |
| 508 | #[test] |
| 509 | fn inline_review_comments_drop_one_out_of_hunk_anchor_not_the_whole_review() { |
| 510 | // Line 25 is inside the file but between the two hunks. Before the |
| 511 | // hunk filter this single bad anchor 422'd the whole review request. |
| 512 | let review = review_with( |
| 513 | vec![ |
| 514 | review_issue("error", Some("crates/tui/src/lib.rs"), Some(11)), |
| 515 | review_issue("error", Some("crates/tui/src/lib.rs"), Some(25)), |
| 516 | review_issue("warning", Some("crates/tui/src/lib.rs"), Some(41)), |
| 517 | ], |
| 518 | Vec::new(), |
| 519 | ); |
| 520 | let plan = plan_inline_review_comments(&review, &review_test_diff()); |
| 521 | let lines: Vec<_> = plan.comments.iter().map(|c| c["line"].clone()).collect(); |
| 522 | assert_eq!(lines, vec![serde_json::json!(11), serde_json::json!(41)]); |
| 523 | // The loss is counted and reported, never silent. |
| 524 | assert_eq!(plan.dropped_out_of_hunk, 1); |
| 525 | assert_eq!(plan.dropped_untouched_file, 0); |
| 526 | assert!( |
| 527 | plan.receipt() |
| 528 | .expect("receipt") |
| 529 | .contains("no line inside a diff hunk") |
| 530 | ); |
| 531 | } |
| 532 | |
| 533 | #[test] |
| 534 | fn review_suggestion_with_replacement_posts_a_committable_block() { |
| 535 | let review = review_with( |
| 536 | Vec::new(), |
| 537 | vec![review_suggestion( |
| 538 | Some("crates/tui/src/lib.rs"), |
| 539 | Some(11), |
| 540 | Some("let b = a.unwrap_or_default();"), |
| 541 | )], |
| 542 | ); |
| 543 | let comments = plan_inline_review_comments(&review, &review_test_diff()).comments; |
| 544 | assert_eq!(comments.len(), 1); |
| 545 | let body = comments[0]["body"].as_str().expect("body"); |
| 546 | assert!(body.starts_with("Use the checked variant"), "{body}"); |
| 547 | assert!( |
| 548 | body.contains("```suggestion\nlet b = a.unwrap_or_default();\n```"), |
| 549 | "{body}" |
| 550 | ); |
| 551 | assert_eq!(comments[0]["side"], "RIGHT"); |
| 552 | // Single-line suggestions must not carry start_line. |
| 553 | assert!(comments[0].get("start_line").is_none()); |
| 554 | } |
| 555 | |
| 556 | #[test] |
| 557 | fn multi_line_review_suggestion_spans_start_line_to_line() { |
| 558 | let mut suggestion = review_suggestion( |
| 559 | Some("crates/tui/src/lib.rs"), |
| 560 | Some(12), |
| 561 | Some("let b = a.unwrap_or_default();\nlet c = 2;"), |
| 562 | ); |
| 563 | suggestion.start_line = Some(11); |
| 564 | suggestion.end_line = Some(12); |
| 565 | let review = review_with(Vec::new(), vec![suggestion]); |
| 566 | let comments = plan_inline_review_comments(&review, &review_test_diff()).comments; |
| 567 | assert_eq!(comments.len(), 1); |
| 568 | assert_eq!(comments[0]["start_line"], 11); |
| 569 | assert_eq!(comments[0]["start_side"], "RIGHT"); |
| 570 | assert_eq!(comments[0]["line"], 12); |
| 571 | assert_eq!(comments[0]["side"], "RIGHT"); |
| 572 | } |
| 573 | |
| 574 | #[test] |
| 575 | fn review_suggestion_without_replacement_degrades_to_prose() { |
| 576 | // SAFETY: a committable suggestion is one click from being merged, so |
| 577 | // a suggestion the model did not back with literal code must never |
| 578 | // render as an applicable block. |
| 579 | let review = review_with( |
| 580 | Vec::new(), |
| 581 | vec![review_suggestion( |
| 582 | Some("crates/tui/src/lib.rs"), |
| 583 | Some(11), |
| 584 | None, |
| 585 | )], |
| 586 | ); |
| 587 | let plan = plan_inline_review_comments(&review, &review_test_diff()); |
| 588 | assert_eq!(plan.comments.len(), 1); |
| 589 | let body = plan.comments[0]["body"].as_str().expect("body"); |
| 590 | assert_eq!(body, "Use the checked variant"); |
| 591 | assert!(!body.contains("```suggestion"), "{body}"); |
| 592 | assert_eq!(plan.degraded_to_prose, 1); |
| 593 | } |
| 594 | |
| 595 | #[test] |
| 596 | fn review_suggestion_spanning_outside_a_hunk_degrades_to_prose() { |
| 597 | // Line 12 is in a hunk but line 13 is not, so the span cannot be |
| 598 | // committed. Keep the finding, drop the one-click apply. |
| 599 | let mut suggestion = review_suggestion( |
| 600 | Some("crates/tui/src/lib.rs"), |
| 601 | Some(12), |
| 602 | Some("let c = 2;\nlet d = 3;"), |
| 603 | ); |
| 604 | suggestion.start_line = Some(12); |
| 605 | suggestion.end_line = Some(13); |
| 606 | let review = review_with(Vec::new(), vec![suggestion]); |
| 607 | let comments = plan_inline_review_comments(&review, &review_test_diff()).comments; |
| 608 | // end_line 13 is outside every hunk, so there is no anchor at all. |
| 609 | assert!(comments.is_empty()); |
| 610 | |
| 611 | // Same replacement anchored at an in-hunk end line but with a start |
| 612 | // line outside the hunk: anchor is valid, span is not -> prose. |
| 613 | let mut suggestion = review_suggestion( |
| 614 | Some("crates/tui/src/lib.rs"), |
| 615 | Some(11), |
| 616 | Some("let a = 1;\nlet b = a.unwrap_or_default();"), |
| 617 | ); |
| 618 | suggestion.start_line = Some(9); |
| 619 | suggestion.end_line = Some(11); |
| 620 | let review = review_with(Vec::new(), vec![suggestion]); |
| 621 | let comments = plan_inline_review_comments(&review, &review_test_diff()).comments; |
| 622 | assert_eq!(comments.len(), 1); |
| 623 | let body = comments[0]["body"].as_str().expect("body"); |
| 624 | assert!(!body.contains("```suggestion"), "{body}"); |
| 625 | assert!(comments[0].get("start_line").is_none()); |
| 626 | } |
| 627 | |
| 628 | #[test] |
| 629 | fn review_suggestion_anchored_to_a_deleted_line_is_not_committable() { |
| 630 | // Right line 41 is `+let e = d + 1;`; the deleted `-let e = d;` has no |
| 631 | // RIGHT-side number, so a model that anchors at the pre-image line 42 |
| 632 | // gets nothing rather than a suggestion on the wrong line. |
| 633 | let review = review_with( |
| 634 | Vec::new(), |
| 635 | vec![review_suggestion( |
| 636 | Some("crates/tui/src/lib.rs"), |
| 637 | Some(42), |
| 638 | Some("let e = d + 2;"), |
| 639 | )], |
| 640 | ); |
| 641 | assert!( |
| 642 | plan_inline_review_comments(&review, &review_test_diff()) |
| 643 | .comments |
| 644 | .is_empty() |
| 645 | ); |
| 646 | } |
| 647 | |
| 648 | #[test] |
| 649 | fn review_suggestion_replacement_containing_backticks_is_fenced_safely() { |
| 650 | let review = review_with( |
| 651 | Vec::new(), |
| 652 | vec![review_suggestion( |
| 653 | Some("crates/tui/src/lib.rs"), |
| 654 | Some(11), |
| 655 | Some("let b = \"```\";"), |
| 656 | )], |
| 657 | ); |
| 658 | let comments = plan_inline_review_comments(&review, &review_test_diff()).comments; |
| 659 | let body = comments[0]["body"].as_str().expect("body"); |
| 660 | assert!(body.contains("````suggestion\n"), "{body}"); |
| 661 | assert!(body.ends_with("\n````"), "{body}"); |
| 662 | } |
| 663 | |
| 664 | #[test] |
| 665 | fn review_suggestion_multi_line_replacement_without_span_degrades_to_prose() { |
| 666 | // SAFETY: with no start_line/end_line, GitHub would insert these two |
| 667 | // lines at line 11 and leave the original line 11 duplicated below. |
| 668 | let review = review_with( |
| 669 | Vec::new(), |
| 670 | vec![review_suggestion( |
| 671 | Some("crates/tui/src/lib.rs"), |
| 672 | Some(11), |
| 673 | Some("let b = a.unwrap_or_default();\nlet c = 2;"), |
| 674 | )], |
| 675 | ); |
| 676 | let plan = plan_inline_review_comments(&review, &review_test_diff()); |
| 677 | assert_eq!(plan.comments.len(), 1); |
| 678 | assert!( |
| 679 | !plan.comments[0]["body"] |
| 680 | .as_str() |
| 681 | .expect("body") |
| 682 | .contains("```suggestion") |
| 683 | ); |
| 684 | assert_eq!(plan.degraded_to_prose, 1); |
| 685 | } |
| 686 | |
| 687 | #[test] |
| 688 | fn review_suggestion_prose_fence_is_never_committable() { |
| 689 | // SAFETY: only the replacement this reviewer validated against the |
| 690 | // diff may be one click from merging. A fence the model wrote inside |
| 691 | // its explanation is downgraded to a plain code block. |
| 692 | let mut suggestion = review_suggestion(Some("crates/tui/src/lib.rs"), Some(11), None); |
| 693 | suggestion.suggestion = |
| 694 | "Try this:\n\n```suggestion\nlet b = a.unwrap_or_default();\n```".to_string(); |
| 695 | let review = review_with(Vec::new(), vec![suggestion]); |
| 696 | let comments = plan_inline_review_comments(&review, &review_test_diff()).comments; |
| 697 | let body = comments[0]["body"].as_str().expect("body"); |
| 698 | assert!(!body.contains("```suggestion"), "{body}"); |
| 699 | assert!(body.contains("```text"), "{body}"); |
| 700 | assert!(body.contains("let b = a.unwrap_or_default();"), "{body}"); |
| 701 | } |
| 702 | |
| 703 | #[test] |
| 704 | fn review_suggestion_tilde_prose_fence_is_neutralized() { |
| 705 | let mut suggestion = review_suggestion(Some("crates/tui/src/lib.rs"), Some(11), None); |
| 706 | suggestion.suggestion = " ~~~suggestion\n x\n ~~~".to_string(); |
| 707 | let review = review_with(Vec::new(), vec![suggestion]); |
| 708 | let comments = plan_inline_review_comments(&review, &review_test_diff()).comments; |
| 709 | let body = comments[0]["body"].as_str().expect("body"); |
| 710 | assert!(body.contains(" ~~~text"), "{body}"); |
| 711 | } |
| 712 | |
| 713 | #[test] |
| 714 | fn suggestion_fence_behind_list_marker_or_blockquote_is_neutralized() { |
| 715 | // Whether GitHub renders a fence nested behind a list marker, |
| 716 | // blockquote cue, or ordered-list marker applicable is unverified, |
| 717 | // so those shapes are treated as live and downgraded too. Ordinary |
| 718 | // prose lines that merely start with a marker stay untouched. |
| 719 | let neutralized = neutralize_model_suggestion_fences( |
| 720 | "```suggestion\n- ```suggestion\n rm -rf /\n> ```suggestion\n1. ```suggestion\n> - ~~~suggestion\n- fix the `foo` call", |
| 721 | ); |
| 722 | assert_eq!( |
| 723 | neutralized, |
| 724 | "```text\n- ```text\n rm -rf /\n> ```text\n1. ```text\n> - ~~~text\n- fix the `foo` call" |
| 725 | ); |
| 726 | } |
| 727 | |
| 728 | #[test] |
| 729 | fn inline_issue_comment_neutralizes_model_suggestion_fences_in_issue_text() { |
| 730 | // SAFETY: regression for a proven hole — the issue title and |
| 731 | // description are raw model text, and a ```suggestion fence in |
| 732 | // either used to reach the inline comment body verbatim: a |
| 733 | // one-click mergeable block that bypassed every span and size |
| 734 | // check. Only a replacement validated against the diff hunks may |
| 735 | // ever be committable. |
| 736 | let mut fenced_description = review_issue("error", Some("crates/tui/src/lib.rs"), Some(11)); |
| 737 | fenced_description.title = "Cleanup script".to_string(); |
| 738 | fenced_description.description = "Run this:\n\n```suggestion\nrm -rf /\n```\n".to_string(); |
| 739 | let mut fenced_title = review_issue("warning", Some("crates/tui/src/lib.rs"), Some(12)); |
| 740 | fenced_title.title = "Fix all\n```suggestion\nrm -rf ~\n```".to_string(); |
| 741 | fenced_title.description = "detail".to_string(); |
| 742 | let review = review_with(vec![fenced_description, fenced_title], Vec::new()); |
| 743 | |
| 744 | let comments = plan_inline_review_comments(&review, &review_test_diff()).comments; |
| 745 | assert_eq!(comments.len(), 2); |
| 746 | for comment in &comments { |
| 747 | let body = comment["body"].as_str().expect("body"); |
| 748 | assert!(!body.contains("```suggestion"), "{body}"); |
| 749 | } |
| 750 | let description_body = comments[0]["body"].as_str().expect("body"); |
| 751 | assert!( |
| 752 | description_body.contains("```text\nrm -rf /"), |
| 753 | "{description_body}" |
| 754 | ); |
| 755 | let title_body = comments[1]["body"].as_str().expect("body"); |
| 756 | assert!(title_body.contains("```text\nrm -rf ~"), "{title_body}"); |
| 757 | } |
| 758 | |
| 759 | #[test] |
| 760 | fn pr_prompt_preserves_the_last_patch_beyond_the_old_200kib_cutoff() { |
| 761 | let diff = format!( |
| 762 | "{}\ndiff --git a/last.rs b/last.rs\n+LAST_PATCH\n", |
| 763 | "x".repeat(210 * 1024) |
| 764 | ); |
| 765 | let view = GhPullRequest { |
| 766 | head_sha: "b".repeat(40), |
| 767 | base_sha: "a".repeat(40), |
| 768 | changed_files: 301, |
| 769 | ..Default::default() |
| 770 | }; |
| 771 | let prompt = format_pr_prompt(6002, &view, &diff); |
| 772 | assert!(prompt.contains(&diff)); |
| 773 | assert!(prompt.contains("+LAST_PATCH")); |
| 774 | assert!(prompt.contains(&view.head_sha)); |
| 775 | assert!(!prompt.contains("diff truncated")); |
| 776 | } |
| 777 | |
| 778 | #[test] |
| 779 | fn pr_review_unreviewed_note_names_budget_stops_and_plan_skips() { |
| 780 | assert_eq!(pr_review_unreviewed_note(None, 0), "the entire diff"); |
| 781 | fn patch(name: &str, content: &str) -> String { |
| 782 | format!( |
| 783 | "diff --git a/{name} b/{name}\nnew file mode 100644\n--- /dev/null\n+++ b/{name}\n@@ -0,0 +1 @@\n+{content}\n" |
| 784 | ) |
| 785 | } |
| 786 | let patches = [ |
| 787 | patch("a.txt", "alpha"), |
| 788 | patch("b.txt", "bravo"), |
| 789 | patch("c.txt", "charlie"), |
| 790 | ]; |
| 791 | let diff = patches.concat(); |
| 792 | let max_chars = patches |
| 793 | .iter() |
| 794 | .map(|patch| patch.chars().count()) |
| 795 | .max() |
| 796 | .unwrap(); |
| 797 | let view = GhPullRequest { |
| 798 | changed_files: 3, |
| 799 | ..Default::default() |
| 800 | }; |
| 801 | // Two passes of budget: a and b are planned, c is skipped by the plan. |
| 802 | let plan = crate::tools::review::plan_pr_review(&diff, &view, max_chars, 2).unwrap(); |
| 803 | let note = pr_review_unreviewed_note(Some(&plan), 0); |
| 804 | assert!(note.contains("were never read"), "{note}"); |
| 805 | assert!(note.contains("a/b.txt b/b.txt"), "{note}"); |
| 806 | assert!(note.contains("the plan never scheduled"), "{note}"); |
| 807 | assert!(note.contains("a/c.txt b/c.txt"), "{note}"); |
| 808 | // One pass done: only b is left unread, but the plan skip still stands. |
| 809 | let note = pr_review_unreviewed_note(Some(&plan), 1); |
| 810 | assert!(!note.contains("a/a.txt b/a.txt"), "{note}"); |
| 811 | assert!(note.contains("a/b.txt b/b.txt"), "{note}"); |
| 812 | assert!(note.contains("a/c.txt b/c.txt"), "{note}"); |
| 813 | // A complete plan with every pass done names nothing. |
| 814 | let complete = crate::tools::review::plan_pr_review(&diff, &view, max_chars, 3).unwrap(); |
| 815 | assert_eq!( |
| 816 | pr_review_unreviewed_note(Some(&complete), 3), |
| 817 | "no planned file was left unread" |
| 818 | ); |
| 819 | } |
| 820 | |
| 821 | #[tokio::test(flavor = "current_thread")] |
| 822 | async fn actual_cli_review_report_host_matches_legacy_and_core_keeps_inline_authority() { |
| 823 | let _home = crate::test_support::SealedHome::new(); |
| 824 | let _policy = crate::plugins::activation::TestPolicyGuard::extension_host(false); |
| 825 | let Some(node) = crate::extension_host::tests::node_for_tests("cli_review_report_parity") |
| 826 | else { |
| 827 | return; |
| 828 | }; |
| 829 | let root = tempfile::tempdir().unwrap(); |
| 830 | let manager = std::sync::Arc::new(crate::extension_host::ExtensionHostManager::new( |
| 831 | crate::extension_host::ExtensionHostOptions { |
| 832 | runtime: crate::config::ExtensionHostRuntime::Node, |
| 833 | node_override: Some(node), |
| 834 | root: Some(root.path().join("host")), |
| 835 | ..Default::default() |
| 836 | }, |
| 837 | )); |
| 838 | let _manager = |
| 839 | crate::extension_host::TestManagerGuard::install(std::sync::Arc::clone(&manager)); |
| 840 | let mut flags = crate::features::Features::with_defaults(); |
| 841 | flags.enable(crate::features::Feature::ReviewHost); |
| 842 | let context = crate::tools::spec::ToolContext::new(root.path()).with_features(flags); |
| 843 | let mut issue = review_issue("warning", Some("crates/tui/src/lib.rs"), Some(11)); |
| 844 | issue.description = "malicious body\n> - ```suggestion\nnot approved\n```".into(); |
| 845 | let valid = review_suggestion( |
| 846 | Some("crates/tui/src/lib.rs"), |
| 847 | Some(11), |
| 848 | Some("let b = a.unwrap_or_default();"), |
| 849 | ); |
| 850 | let invalid = review_suggestion(Some("untouched.rs"), Some(11), Some("unowned replacement")); |
| 851 | let review = review_with(vec![issue], vec![valid, invalid]); |
| 852 | let view = GhPullRequest { |
| 853 | head_sha: "a".repeat(40), |
| 854 | base_sha: "b".repeat(40), |
| 855 | ..Default::default() |
| 856 | }; |
| 857 | let local = render_review_report_for_context(Some(&review), "unused", None, &context) |
| 858 | .await |
| 859 | .unwrap(); |
| 860 | assert_eq!(local, render_review_markdown(&review, None)); |
| 861 | let posted = |
| 862 | render_review_report_for_context(Some(&review), "unused", Some((7, &view)), &context) |
| 863 | .await |
| 864 | .unwrap(); |
| 865 | assert_eq!(posted, render_review_markdown(&review, Some((7, &view)))); |
| 866 | assert!(posted.ends_with(&review_advisory_footer(7, &view))); |
| 867 | let safe = neutralize_model_suggestion_fences(&posted); |
| 868 | assert!(!safe.contains("```suggestion")); |
| 869 | assert!(safe.contains("> - ```text")); |
| 870 | let plan = plan_inline_review_comments(&review, &review_test_diff()); |
| 871 | assert_eq!(plan.comments.len(), 2); |
| 872 | assert_eq!(plan.dropped_untouched_file, 1); |
| 873 | assert!( |
| 874 | plan.comments[1]["body"] |
| 875 | .as_str() |
| 876 | .unwrap() |
| 877 | .contains("```suggestion") |
| 878 | ); |
| 879 | assert!( |
| 880 | !plan.comments[0]["body"] |
| 881 | .as_str() |
| 882 | .unwrap() |
| 883 | .contains("```suggestion") |
| 884 | ); |
| 885 | assert_eq!( |
| 886 | render_review_report_for_context(None, "plain prose", None, &context) |
| 887 | .await |
| 888 | .unwrap(), |
| 889 | "plain prose" |
| 890 | ); |
| 891 | let prompt = crate::tools::review_host::interactive(7, &view, &review_test_diff(), &context) |
| 892 | .await |
| 893 | .unwrap(); |
| 894 | assert_eq!(prompt, format_pr_prompt(7, &view, &review_test_diff())); |
| 895 | manager.shutdown().await; |
| 896 | } |
| 897 | |
| 898 | #[test] |
| 899 | fn pr_review_markdown_shows_the_computed_replacement() { |
| 900 | // A plain `codewhale review --pr` (no --post) computes and validates |
| 901 | // the literal fix; the local report must show it, not just the prose. |
| 902 | let view = GhPullRequest { |
| 903 | title: "T".to_string(), |
| 904 | body: String::new(), |
| 905 | base: "main".to_string(), |
| 906 | head: "feature".to_string(), |
| 907 | url: "https://example.invalid/pr/1".to_string(), |
| 908 | head_sha: "abc123".to_string(), |
| 909 | ..Default::default() |
| 910 | }; |
| 911 | let mut single = review_suggestion( |
| 912 | Some("crates/tui/src/lib.rs"), |
| 913 | Some(11), |
| 914 | Some("let b = a.unwrap_or_default();"), |
| 915 | ); |
| 916 | single.suggestion = "Use the checked variant".to_string(); |
| 917 | let mut multi = review_suggestion( |
| 918 | Some("crates/tui/src/lib.rs"), |
| 919 | Some(11), |
| 920 | Some("let b = a.unwrap_or_default();\nlet c = 2;"), |
| 921 | ); |
| 922 | multi.suggestion = "Replace both lines".to_string(); |
| 923 | multi.start_line = Some(11); |
| 924 | multi.end_line = Some(12); |
| 925 | let review = review_with(Vec::new(), vec![single, multi]); |
| 926 | |
| 927 | let local = render_review_markdown(&review, None); |
| 928 | assert!(local.contains("### Suggestions"), "{local}"); |
| 929 | assert!( |
| 930 | local.contains("\n ```suggestion\n let b = a.unwrap_or_default();\n ```\n"), |
| 931 | "{local}" |
| 932 | ); |
| 933 | assert!( |
| 934 | local |
| 935 | .contains("\n ```suggestion\n let b = a.unwrap_or_default();\n let c = 2;\n ```\n"), |
| 936 | "{local}" |
| 937 | ); |
| 938 | |
| 939 | // The posted body keeps the fix visible but never as a live |
| 940 | // one-click block: only diff-validated inline suggestion comments |
| 941 | // may carry those to GitHub. |
| 942 | let posted = render_review_markdown(&review, Some((1, &view))); |
| 943 | assert!( |
| 944 | posted.contains("let b = a.unwrap_or_default();"), |
| 945 | "{posted}" |
| 946 | ); |
| 947 | assert!(!posted.contains("```suggestion"), "{posted}"); |
| 948 | assert!(posted.contains("```text"), "{posted}"); |
| 949 | } |
| 950 | |
| 951 | /// #6510: a plain-diff review now asks for the one structured review |
| 952 | /// contract. A reply that keeps it renders as the Markdown report; one |
| 953 | /// that ignores it is printed verbatim, as the old prose path did. |
| 954 | #[test] |
| 955 | fn plain_diff_review_renders_structured_reply_and_keeps_prose() { |
| 956 | let json = r#"{"summary":"One risky unwrap.","issues":[{"severity":"high","title":"Unchecked unwrap","description":"Panics on None.","path":"src/lib.rs","line":11}],"suggestions":[],"overall_assessment":"request changes"}"#; |
| 957 | let structured = crate::tools::review::ReviewOutput::from_structured_str(json); |
| 958 | let report = plain_diff_review_report(structured.as_ref(), json); |
| 959 | assert!(report.starts_with("## Codewhale review"), "{report}"); |
| 960 | assert!(report.contains("One risky unwrap."), "{report}"); |
| 961 | assert!(report.contains("### Findings"), "{report}"); |
| 962 | assert!(report.contains("`src/lib.rs:11`"), "{report}"); |
| 963 | assert!( |
| 964 | !report.contains("\"issues\""), |
| 965 | "raw JSON must not leak: {report}" |
| 966 | ); |
| 967 | |
| 968 | let prose = "Looks fine overall; consider a test for the empty case."; |
| 969 | let structured = crate::tools::review::ReviewOutput::from_structured_str(prose); |
| 970 | assert!(structured.is_none()); |
| 971 | assert_eq!(plain_diff_review_report(structured.as_ref(), prose), prose); |
| 972 | } |
| 973 | |
| 974 | #[test] |
| 975 | fn oversized_review_suggestion_degrades_to_prose() { |
| 976 | use crate::tools::review::MAX_COMMITTABLE_SUGGESTION_LINES; |
| 977 | |
| 978 | // A whole-hunk rewrite is not a mechanical fix, so it must not be |
| 979 | // committable even though every line is inside the diff. |
| 980 | let span = MAX_COMMITTABLE_SUGGESTION_LINES + 5; |
| 981 | let added: String = (1..=span).map(|i| format!("+line {i}\n")).collect(); |
| 982 | let diff = format!( |
| 983 | "diff --git a/big.rs b/big.rs\nnew file mode 100644\n--- /dev/null\n\ |
| 984 | +++ b/big.rs\n@@ -0,0 +1,{span} @@\n{added}" |
| 985 | ); |
| 986 | let replacement: String = (1..=span) |
| 987 | .map(|i| format!("line {i} fixed")) |
| 988 | .collect::<Vec<_>>() |
| 989 | .join("\n"); |
| 990 | |
| 991 | let mut suggestion = review_suggestion(Some("big.rs"), Some(span), Some(&replacement)); |
| 992 | suggestion.start_line = Some(1); |
| 993 | suggestion.end_line = Some(span); |
| 994 | let review = review_with(Vec::new(), vec![suggestion]); |
| 995 | let comments = plan_inline_review_comments(&review, &diff).comments; |
| 996 | assert_eq!(comments.len(), 1); |
| 997 | let body = comments[0]["body"].as_str().expect("body"); |
| 998 | assert!(!body.contains("```suggestion"), "{body}"); |
| 999 | |
| 1000 | // The same shape, trimmed to the budget, stays committable. |
| 1001 | let mut suggestion = review_suggestion( |
| 1002 | Some("big.rs"), |
| 1003 | Some(MAX_COMMITTABLE_SUGGESTION_LINES), |
| 1004 | Some("line 1 fixed"), |
| 1005 | ); |
| 1006 | suggestion.start_line = Some(1); |
| 1007 | suggestion.end_line = Some(MAX_COMMITTABLE_SUGGESTION_LINES); |
| 1008 | let review = review_with(Vec::new(), vec![suggestion]); |
| 1009 | let comments = plan_inline_review_comments(&review, &diff).comments; |
| 1010 | assert!( |
| 1011 | comments[0]["body"] |
| 1012 | .as_str() |
| 1013 | .expect("body") |
| 1014 | .contains("```suggestion"), |
| 1015 | "a span at the budget limit stays committable" |
| 1016 | ); |
| 1017 | } |
| 1018 | |
| 1019 | #[tokio::test] |
| 1020 | async fn configured_workflow_default_keeps_named_custom_route() { |
| 1021 | let config = custom_exec_config("custom-a"); |
| 1022 | let model = config.default_model(); |
| 1023 | |
| 1024 | let route = resolve_cli_exec_route( |
| 1025 | &config, |
| 1026 | &model, |
| 1027 | "Run a checked-in Workflow through the host runtime", |
| 1028 | true, |
| 1029 | ) |
| 1030 | .await |
| 1031 | .expect("configured workflow route"); |
| 1032 | let execution = config_for_cli_route(&config, &route).expect("admitted execution route"); |
| 1033 | |
| 1034 | assert_eq!(route.provider.provider, crate::config::ProviderKind::Custom); |
| 1035 | assert_eq!( |
| 1036 | execution.active_provider_identity().unwrap().key.as_str(), |
| 1037 | "custom-a" |
| 1038 | ); |
| 1039 | assert_eq!( |
| 1040 | execution.active_route_base_url(), |
| 1041 | "http://127.0.0.1:18181/v1" |
| 1042 | ); |
| 1043 | let client = crate::client::CodewhaleClient::new(&execution).expect("workflow client"); |
| 1044 | assert_eq!(client.base_url(), "http://127.0.0.1:18181/v1"); |
| 1045 | } |
| 1046 | |
| 1047 | #[test] |
| 1048 | fn exec_json_receipts_keep_exact_named_custom_provider() { |
| 1049 | let config = custom_exec_config("custom-a"); |
| 1050 | let identity = config.active_provider_identity().unwrap(); |
| 1051 | let provider = identity.key.as_str(); |
| 1052 | // #6510: plain exec is an Engine turn now; its `--json` receipt keeps |
| 1053 | // the documented one-shot fields (docs/LIVE_SMOKE.md step 5). |
| 1054 | let mut one_shot = ExecSummary { |
| 1055 | mode: "one-shot".to_string(), |
| 1056 | provider: provider.to_string(), |
| 1057 | model: "model-a".to_string(), |
| 1058 | output: "done".to_string(), |
| 1059 | status: Some("completed".to_string()), |
| 1060 | ..ExecSummary::default() |
| 1061 | }; |
| 1062 | one_shot.record_one_shot_outcome(Some(codewhale_models::Usage { |
| 1063 | input_tokens: 12, |
| 1064 | output_tokens: 3, |
| 1065 | ..Default::default() |
| 1066 | })); |
| 1067 | let one_shot = serde_json::to_value(&one_shot).expect("one-shot receipt"); |
| 1068 | assert_eq!(one_shot["mode"], "one-shot"); |
| 1069 | assert_eq!(one_shot["provider"], "custom-a"); |
| 1070 | assert_eq!(one_shot["model"], "model-a"); |
| 1071 | assert_eq!(one_shot["output"], "done"); |
| 1072 | assert_eq!(one_shot["success"], true); |
| 1073 | assert_eq!(one_shot["usage"]["input_tokens"], 12); |
| 1074 | assert_eq!(one_shot["usage"]["output_tokens"], 3); |
| 1075 | |
| 1076 | let mut failed = ExecSummary { |
| 1077 | mode: "one-shot".to_string(), |
| 1078 | provider: provider.to_string(), |
| 1079 | model: "model-a".to_string(), |
| 1080 | status: Some("failed".to_string()), |
| 1081 | error: Some("Model response incomplete".to_string()), |
| 1082 | ..ExecSummary::default() |
| 1083 | }; |
| 1084 | failed.record_one_shot_outcome(None); |
| 1085 | let failed = serde_json::to_value(&failed).expect("failed one-shot receipt"); |
| 1086 | assert_eq!(failed["success"], false); |
| 1087 | assert!( |
| 1088 | failed.get("usage").is_none(), |
| 1089 | "no usage is never zero usage" |
| 1090 | ); |
| 1091 | |
| 1092 | let agent = serde_json::to_value(ExecSummary { |
| 1093 | mode: "agent".to_string(), |
| 1094 | provider: provider.to_string(), |
| 1095 | model: "model-a".to_string(), |
| 1096 | ..ExecSummary::default() |
| 1097 | }) |
| 1098 | .expect("agent exec JSON receipt"); |
| 1099 | assert_eq!(agent["provider"], "custom-a"); |
| 1100 | assert!( |
| 1101 | agent.get("success").is_none() && agent.get("usage").is_none(), |
| 1102 | "agent receipts keep their pre-#6510 shape" |
| 1103 | ); |
| 1104 | let serialized = serde_json::to_string(&agent).expect("serialize receipt"); |
| 1105 | assert!(!serialized.contains("127.0.0.1")); |
| 1106 | assert!(!serialized.contains("local-test-key")); |
| 1107 | } |
| 1108 | |
| 1109 | #[test] |
| 1110 | fn exec_stream_provider_pair_preserves_named_literal_and_root_custom_provenance() { |
| 1111 | let mut config = Config::default(); |
| 1112 | for key in ["lm-studio", "custom"] { |
| 1113 | config.providers.get_or_insert_with(Default::default).custom.insert( |
| 1114 | key.to_string(), |
| 1115 | crate::config::ProviderConfig { |
| 1116 | kind: Some("openai-compatible".to_string()), |
| 1117 | base_url: Some("http://localhost:1234/v1".to_string()), |
| 1118 | ..Default::default() |
| 1119 | }, |
| 1120 | ); |
| 1121 | } |
| 1122 | let named = config.resolve_provider_identity("lm-studio").expect("exact named table"); |
| 1123 | let literal = config.resolve_provider_identity("custom").expect("exact literal table"); |
| 1124 | let root_config = Config { |
| 1125 | provider: Some("custom".to_string()), |
| 1126 | ..Config::default() |
| 1127 | }.with_legacy_root(None, Some("http://localhost:1234/v1".to_string())); |
| 1128 | let root = root_config.active_provider_identity().expect("captured root migration"); |
| 1129 | let built_in = Config::default().active_provider_identity().expect("built-in fixture"); |
| 1130 | |
| 1131 | assert_eq!( |
| 1132 | exec_stream_provider_route(&named), |
| 1133 | ("custom".to_string(), Some("lm-studio".to_string())) |
| 1134 | ); |
| 1135 | assert_eq!( |
| 1136 | exec_stream_provider_route(&literal), |
| 1137 | ("custom".to_string(), Some("custom".to_string())) |
| 1138 | ); |
| 1139 | assert_eq!( |
| 1140 | exec_stream_provider_route(&root), |
| 1141 | ("custom".to_string(), None) |
| 1142 | ); |
| 1143 | assert_eq!( |
| 1144 | exec_stream_provider_route(&built_in), |
| 1145 | ("deepseek".to_string(), None) |
| 1146 | ); |
| 1147 | } |
| 1148 | |
| 1149 | #[test] |
| 1150 | fn resumed_exec_persistence_updates_provider_and_model_as_one_route() { |
| 1151 | let saved_a = saved_exec_session("custom-a", crate::config::ZAI_GLM_5_2_MODEL); |
| 1152 | let mut config = custom_exec_config("custom-a"); |
| 1153 | apply_exec_provider_override(&mut config, "custom-b").expect("custom B"); |
| 1154 | let model = resolve_exec_resume_route(&mut config, &saved_a, true, None) |
| 1155 | .expect("explicit provider route"); |
| 1156 | let mut persisted = saved_a; |
| 1157 | stamp_exec_session_metadata( |
| 1158 | &mut persisted, |
| 1159 | &model, |
| 1160 | crate::config::ProviderKind::Custom.as_str(), |
| 1161 | Some("custom-b"), |
| 1162 | Path::new("/tmp/exec-resume"), |
| 1163 | ); |
| 1164 | |
| 1165 | let mut next_config = custom_exec_config("custom-a"); |
| 1166 | let resumed_model = resolve_exec_resume_route(&mut next_config, &persisted, false, None) |
| 1167 | .expect("next plain resume"); |
| 1168 | |
| 1169 | assert_eq!(persisted.metadata.model_provider, "custom"); |
| 1170 | assert_eq!( |
| 1171 | persisted.metadata.model_provider_id.as_deref(), |
| 1172 | Some("custom-b") |
| 1173 | ); |
| 1174 | assert_eq!(persisted.metadata.model, "model-b"); |
| 1175 | assert_eq!(next_config.provider.as_deref(), Some("custom-b")); |
| 1176 | assert_eq!(resumed_model, "model-b"); |
| 1177 | } |
| 1178 | |
| 1179 | #[test] |
| 1180 | fn exec_persistence_omits_id_for_legacy_root_custom_route() { |
| 1181 | let mut saved = session_manager::create_saved_session_with_mode( |
| 1182 | &[], |
| 1183 | "legacy-root-model", |
| 1184 | Path::new("/tmp/exec-root"), |
| 1185 | 0, |
| 1186 | None, |
| 1187 | Some("exec"), |
| 1188 | ); |
| 1189 | stamp_exec_session_metadata( |
| 1190 | &mut saved, |
| 1191 | "legacy-root-model", |
| 1192 | crate::config::ProviderKind::Custom.as_str(), |
| 1193 | None, |
| 1194 | Path::new("/tmp/exec-root"), |
| 1195 | ); |
| 1196 | |
| 1197 | assert_eq!(saved.metadata.model_provider, "custom"); |
| 1198 | assert_eq!(saved.metadata.model_provider_id, None); |
| 1199 | assert!( |
| 1200 | !serde_json::to_string(&saved) |
| 1201 | .expect("serialize exec session") |
| 1202 | .contains("model_provider_id") |
| 1203 | ); |
| 1204 | } |
| 1205 | |
| 1206 | #[test] |
| 1207 | fn exec_parses_reasoning_effort_flag_alongside_provider() { |
| 1208 | let cli = parse_cli(&[ |
| 1209 | "codewhale", |
| 1210 | "exec", |
| 1211 | "--provider", |
| 1212 | "openrouter", |
| 1213 | "--model", |
| 1214 | "glm-5.2", |
| 1215 | "--reasoning-effort", |
| 1216 | "max", |
| 1217 | "audit", |
| 1218 | ]); |
| 1219 | let Some(Commands::Exec(args)) = cli.command else { |
| 1220 | panic!("expected exec command"); |
| 1221 | }; |
| 1222 | |
| 1223 | assert_eq!(args.provider.as_deref(), Some("openrouter")); |
| 1224 | assert_eq!(args.model.as_deref(), Some("glm-5.2")); |
| 1225 | assert_eq!(args.reasoning_effort.as_deref(), Some("max")); |
| 1226 | assert_eq!(args.prompt, vec!["audit"]); |
| 1227 | } |
| 1228 | |
| 1229 | #[test] |
| 1230 | fn cli_reasoning_effort_normalizes_aliases_and_rejects_typos() { |
| 1231 | // The thinking ladder split these: `xhigh` is a tier the CLI can now |
| 1232 | // name, `ultracode` is still an alias and resolves to `ultra`. |
| 1233 | assert_eq!( |
| 1234 | normalize_cli_reasoning_effort("xhigh").unwrap().as_deref(), |
| 1235 | Some("xhigh") |
| 1236 | ); |
| 1237 | assert_eq!( |
| 1238 | normalize_cli_reasoning_effort("ultracode") |
| 1239 | .unwrap() |
| 1240 | .as_deref(), |
| 1241 | Some("ultra") |
| 1242 | ); |
| 1243 | assert_eq!(normalize_cli_reasoning_effort("default").unwrap(), None); |
| 1244 | assert!(normalize_cli_reasoning_effort("expensive").is_err()); |
| 1245 | } |
| 1246 | |
| 1247 | #[test] |
| 1248 | fn cli_auto_resolves_to_the_declared_default_before_k3_route_normalization() { |
| 1249 | let config = Config { |
| 1250 | provider: Some("moonshot".to_string()), |
| 1251 | providers: Some(crate::config::ProvidersConfig { |
| 1252 | moonshot: crate::config::ProviderConfig { |
| 1253 | base_url: Some(crate::config::DEFAULT_KIMI_CODE_BASE_URL.to_string()), |
| 1254 | model: Some(crate::config::KIMI_CODE_K3_MODEL.to_string()), |
| 1255 | ..Default::default() |
| 1256 | }, |
| 1257 | ..Default::default() |
| 1258 | }), |
| 1259 | ..Default::default() |
| 1260 | }; |
| 1261 | |
| 1262 | // #6290 rework: Auto no longer classifies the prompt. Any wording |
| 1263 | // resolves the declared policy tier, normalized for the K3 route. |
| 1264 | assert_eq!( |
| 1265 | cli_reasoning_effort_value_for_prompt( |
| 1266 | &config, |
| 1267 | crate::config::KIMI_CODE_K3_MODEL, |
| 1268 | crate::reasoning_preference::ReasoningEffort::Auto, |
| 1269 | ) |
| 1270 | .as_deref(), |
| 1271 | Some("high"), |
| 1272 | "Auto resolves the declared default, not a classification" |
| 1273 | ); |
| 1274 | |
| 1275 | assert_eq!( |
| 1276 | cli_reasoning_effort_value_for_prompt( |
| 1277 | &config, |
| 1278 | crate::config::KIMI_CODE_K3_MODEL, |
| 1279 | crate::reasoning_preference::ReasoningEffort::Off, |
| 1280 | ) |
| 1281 | .as_deref(), |
| 1282 | Some("low"), |
| 1283 | "membership K3 still applies its exact-route always-thinking floor" |
| 1284 | ); |
| 1285 | } |
| 1286 | |
| 1287 | #[test] |
| 1288 | fn cli_route_tracks_auto_reasoning_independently_from_auto_model() { |
| 1289 | use crate::reasoning_preference::ReasoningEffort; |
| 1290 | |
| 1291 | let fixed_model_auto_reasoning = CliAutoRoute { |
| 1292 | provider: Config::default().test_identity_for_kind(crate::config::ProviderKind::Deepseek), |
| 1293 | model: crate::config::DEFAULT_TEXT_MODEL.to_string(), |
| 1294 | reasoning_effort: Some(ReasoningEffort::Auto), |
| 1295 | auto_controls_reasoning: true, |
| 1296 | auto_model: false, |
| 1297 | }; |
| 1298 | let auto_model_fixed_reasoning = CliAutoRoute { |
| 1299 | provider: Config::default().test_identity_for_kind(crate::config::ProviderKind::OpenaiCodex), |
| 1300 | model: crate::config::DEFAULT_OPENAI_CODEX_MODEL.to_string(), |
| 1301 | reasoning_effort: Some(ReasoningEffort::High), |
| 1302 | auto_controls_reasoning: false, |
| 1303 | auto_model: true, |
| 1304 | }; |
| 1305 | |
| 1306 | assert!(fixed_model_auto_reasoning.auto_controls_reasoning); |
| 1307 | assert!(!fixed_model_auto_reasoning.auto_model); |
| 1308 | assert!(!auto_model_fixed_reasoning.auto_controls_reasoning); |
| 1309 | assert!(auto_model_fixed_reasoning.auto_model); |
| 1310 | } |
| 1311 | |
| 1312 | #[test] |
| 1313 | fn saved_reasoning_preference_overrides_config_for_non_tui_runtimes() { |
| 1314 | let mut config = Config { |
| 1315 | reasoning_effort: Some("max".to_string()), |
| 1316 | reasoning_effort_inferred_from_legacy_alias: true, |
| 1317 | ..Default::default() |
| 1318 | }; |
| 1319 | let settings = crate::settings::Settings { |
| 1320 | reasoning_effort: Some("low".to_string()), |
| 1321 | ..Default::default() |
| 1322 | }; |
| 1323 | |
| 1324 | apply_saved_reasoning_preference(&mut config, &settings); |
| 1325 | |
| 1326 | assert_eq!(config.reasoning_effort(), Some("low")); |
| 1327 | assert!(config.reasoning_effort_is_explicit()); |
| 1328 | } |
| 1329 | |
| 1330 | /// `run_exec_agent` must hand the engine a concrete tier, never the literal |
| 1331 | /// `"auto"` sentinel, for a fixed-model Auto launch. |
| 1332 | #[test] |
| 1333 | fn fixed_model_exec_auto_resolves_to_a_concrete_tier_not_the_auto_sentinel() { |
| 1334 | let config = Config { |
| 1335 | provider: Some("zai".to_string()), |
| 1336 | ..Default::default() |
| 1337 | }; |
| 1338 | |
| 1339 | let resolved = cli_reasoning_effort_value_for_prompt( |
| 1340 | &config, |
| 1341 | crate::config::ZAI_GLM_5_2_MODEL, |
| 1342 | crate::reasoning_preference::ReasoningEffort::Auto, |
| 1343 | ) |
| 1344 | .expect("Auto must resolve to a concrete tier"); |
| 1345 | |
| 1346 | assert_ne!( |
| 1347 | resolved, "auto", |
| 1348 | "the literal auto sentinel must never reach a provider" |
| 1349 | ); |
| 1350 | assert!( |
| 1351 | matches!(resolved.as_str(), "off" | "low" | "medium" | "high" | "max"), |
| 1352 | "unexpected resolved tier: {resolved}" |
| 1353 | ); |
| 1354 | } |
| 1355 | |
| 1356 | #[test] |
| 1357 | fn exec_accepts_resume_session_flags_for_harnesses() { |
| 1358 | let cli = parse_cli(&[ |
| 1359 | "codewhale", |
| 1360 | "exec", |
| 1361 | "--resume", |
| 1362 | "abc123", |
| 1363 | "--output-format", |
| 1364 | "stream-json", |
| 1365 | "follow up", |
| 1366 | ]); |
| 1367 | let Some(Commands::Exec(args)) = cli.command else { |
| 1368 | panic!("expected exec command"); |
| 1369 | }; |
| 1370 | |
| 1371 | assert_eq!(args.resume.as_deref(), Some("abc123")); |
| 1372 | assert_eq!(args.output_format, ExecOutputFormat::StreamJson); |
| 1373 | assert_eq!(args.prompt, vec!["follow up"]); |
| 1374 | assert!(!args.hooks, "headless hooks stay opt-in by default"); |
| 1375 | } |
| 1376 | |
| 1377 | #[test] |
| 1378 | fn exec_accepts_session_id_alias() { |
| 1379 | let cli = parse_cli(&["codewhale", "exec", "--session-id", "abc123", "follow up"]); |
| 1380 | let Some(Commands::Exec(args)) = cli.command else { |
| 1381 | panic!("expected exec command"); |
| 1382 | }; |
| 1383 | |
| 1384 | assert_eq!(args.session_id.as_deref(), Some("abc123")); |
| 1385 | assert_eq!(args.output_format, ExecOutputFormat::Text); |
| 1386 | } |
| 1387 | |
| 1388 | #[test] |
| 1389 | fn exec_parses_tool_gate_and_hardening_flags() { |
| 1390 | let envelope = r#"{"schema_version":1,"owner":"fleet-worker-1","authority":"read_only"}"#; |
| 1391 | let cli = parse_cli(&[ |
| 1392 | "codewhale", |
| 1393 | "exec", |
| 1394 | "--allowed-tools", |
| 1395 | "File,Git", |
| 1396 | "--disallowed-tools", |
| 1397 | "Bash", |
| 1398 | "--max-turns", |
| 1399 | "7", |
| 1400 | "--max-tool-calls", |
| 1401 | "9", |
| 1402 | "--append-system-prompt", |
| 1403 | "extra rules", |
| 1404 | "--tool-authority-json", |
| 1405 | envelope, |
| 1406 | "--hooks", |
| 1407 | "do the thing", |
| 1408 | ]); |
| 1409 | let Some(Commands::Exec(args)) = cli.command else { |
| 1410 | panic!("expected exec command"); |
| 1411 | }; |
| 1412 | |
| 1413 | assert_eq!( |
| 1414 | args.allowed_tools.as_deref(), |
| 1415 | Some(&["File".to_string(), "Git".to_string()][..]) |
| 1416 | ); |
| 1417 | assert_eq!( |
| 1418 | args.disallowed_tools.as_deref(), |
| 1419 | Some(&["Bash".to_string()][..]) |
| 1420 | ); |
| 1421 | assert_eq!(args.max_turns, Some(7)); |
| 1422 | assert_eq!(args.max_tool_calls, Some(9)); |
| 1423 | assert_eq!(args.append_system_prompt.as_deref(), Some("extra rules")); |
| 1424 | assert_eq!(args.tool_authority_json.as_deref(), Some(envelope)); |
| 1425 | assert!(args.hooks); |
| 1426 | assert_eq!(args.prompt, vec!["do the thing"]); |
| 1427 | } |
| 1428 | |
| 1429 | #[test] |
| 1430 | fn exec_rejects_zero_max_tool_calls() { |
| 1431 | let err = Cli::try_parse_from(["codewhale", "exec", "--max-tool-calls", "0", "do the thing"]) |
| 1432 | .expect_err("max-tool-calls must be >= 1"); |
| 1433 | assert_eq!(err.kind(), clap::error::ErrorKind::ValueValidation); |
| 1434 | } |
| 1435 | |
| 1436 | #[test] |
| 1437 | fn fleet_tool_authority_cannot_cross_an_exec_resume_boundary() { |
| 1438 | assert!(validate_exec_tool_authority_resume(None, true).is_ok()); |
| 1439 | assert!(validate_exec_tool_authority_resume(Some("{}"), false).is_ok()); |
| 1440 | let error = validate_exec_tool_authority_resume(Some("{}"), true) |
| 1441 | .expect_err("authority must remain bound to its fresh Fleet launch") |
| 1442 | .to_string(); |
| 1443 | assert!(error.contains("cannot be combined with exec --resume")); |
| 1444 | } |
| 1445 | |
| 1446 | #[test] |
| 1447 | fn exec_auto_does_not_authorize_sandbox_elevation() { |
| 1448 | let cli = parse_cli(&["codewhale", "exec", "--auto", "run it"]); |
| 1449 | let Some(Commands::Exec(args)) = cli.command else { |
| 1450 | panic!("expected exec command"); |
| 1451 | }; |
| 1452 | |
| 1453 | assert!(!exec_sandbox_elevation_authorized( |
| 1454 | args.allow_sandbox_elevation, |
| 1455 | args.sandbox.as_deref() |
| 1456 | )); |
| 1457 | } |
| 1458 | |
| 1459 | #[test] |
| 1460 | fn exec_explicit_sandbox_elevation_opt_ins_authorize_retry() { |
| 1461 | let danger = parse_cli(&[ |
| 1462 | "codewhale", |
| 1463 | "exec", |
| 1464 | "--auto", |
| 1465 | "--sandbox", |
| 1466 | "danger-full-access", |
| 1467 | "run it", |
| 1468 | ]); |
| 1469 | let Some(Commands::Exec(args)) = danger.command else { |
| 1470 | panic!("expected exec command"); |
| 1471 | }; |
| 1472 | assert!(exec_sandbox_elevation_authorized( |
| 1473 | args.allow_sandbox_elevation, |
| 1474 | args.sandbox.as_deref() |
| 1475 | )); |
| 1476 | |
| 1477 | let flag = parse_cli(&[ |
| 1478 | "codewhale", |
| 1479 | "exec", |
| 1480 | "--auto", |
| 1481 | "--allow-sandbox-elevation", |
| 1482 | "run it", |
| 1483 | ]); |
| 1484 | let Some(Commands::Exec(args)) = flag.command else { |
| 1485 | panic!("expected exec command"); |
| 1486 | }; |
| 1487 | assert!(exec_sandbox_elevation_authorized( |
| 1488 | args.allow_sandbox_elevation, |
| 1489 | args.sandbox.as_deref() |
| 1490 | )); |
| 1491 | } |
| 1492 | |
| 1493 | #[test] |
| 1494 | fn exec_sandbox_denial_stream_event_is_typed() { |
| 1495 | let event = ExecStreamEvent::SandboxDenied { |
| 1496 | tool_id: "call_1".to_string(), |
| 1497 | tool_name: "exec_shell".to_string(), |
| 1498 | reason: "write blocked".to_string(), |
| 1499 | outcome: "approval_required".to_string(), |
| 1500 | }; |
| 1501 | let value: serde_json::Value = |
| 1502 | serde_json::from_str(&serde_json::to_string(&event).expect("serializes")) |
| 1503 | .expect("valid json"); |
| 1504 | assert_eq!(value["type"], "sandbox_denied"); |
| 1505 | assert_eq!(value["outcome"], "approval_required"); |
| 1506 | } |
| 1507 | |
| 1508 | #[test] |
| 1509 | fn exec_help_separates_agent_mode_from_sandbox_elevation() { |
| 1510 | let mut cli = Cli::command(); |
| 1511 | let help = cli |
| 1512 | .find_subcommand_mut("exec") |
| 1513 | .expect("exec command") |
| 1514 | .render_help() |
| 1515 | .to_string(); |
| 1516 | assert!(help.contains("--auto")); |
| 1517 | assert!(help.contains("--sandbox")); |
| 1518 | assert!(help.contains("--allow-sandbox-elevation")); |
| 1519 | assert!(help.contains("does not change the")); |
| 1520 | assert!(help.contains("explicitly authorize sandbox elevation")); |
| 1521 | } |
| 1522 | |
| 1523 | #[test] |
| 1524 | fn exec_shell_only_tool_surface_env_sets_shell_allowlist() { |
| 1525 | let _env_lock = crate::test_support::lock_test_env(); |
| 1526 | let _surface = |
| 1527 | crate::test_support::EnvVarGuard::set(CODEWHALE_TOOL_SURFACE_ENV, " shell-only "); |
| 1528 | |
| 1529 | let allowed_tools = resolve_exec_allowed_tools(None, exec_tool_surface_from_env()) |
| 1530 | .expect("shell-only surface should set an allowlist"); |
| 1531 | |
| 1532 | assert_eq!(allowed_tools, vec!["bash".to_string()]); |
| 1533 | } |
| 1534 | |
| 1535 | #[test] |
| 1536 | fn exec_explicit_allowed_tools_override_shell_only_env() { |
| 1537 | let _env_lock = crate::test_support::lock_test_env(); |
| 1538 | let _surface = crate::test_support::EnvVarGuard::set(CODEWHALE_TOOL_SURFACE_ENV, "shell-only"); |
| 1539 | let explicit = vec![" File ".to_string(), "GIT".to_string()]; |
| 1540 | |
| 1541 | let allowed_tools = resolve_exec_allowed_tools(Some(&explicit), exec_tool_surface_from_env()) |
| 1542 | .expect("explicit allowlist should be preserved"); |
| 1543 | |
| 1544 | assert_eq!(allowed_tools, vec!["file".to_string(), "git".to_string()]); |
| 1545 | } |
| 1546 | |
| 1547 | #[test] |
| 1548 | fn exec_full_tool_surface_env_leaves_allowlist_unset() { |
| 1549 | let _env_lock = crate::test_support::lock_test_env(); |
| 1550 | let _surface = crate::test_support::EnvVarGuard::set(CODEWHALE_TOOL_SURFACE_ENV, "full"); |
| 1551 | |
| 1552 | assert_eq!( |
| 1553 | resolve_exec_allowed_tools(None, exec_tool_surface_from_env()), |
| 1554 | None |
| 1555 | ); |
| 1556 | } |
| 1557 | |
| 1558 | #[test] |
| 1559 | fn exec_unknown_tool_surface_env_warns_without_allowlist() { |
| 1560 | assert!(should_warn_unknown_exec_tool_surface("shell_onyl")); |
| 1561 | assert!(!should_warn_unknown_exec_tool_surface("shell-only")); |
| 1562 | assert!(!should_warn_unknown_exec_tool_surface("native-tools")); |
| 1563 | assert!(!should_warn_unknown_exec_tool_surface("full")); |
| 1564 | assert!(!should_warn_unknown_exec_tool_surface(" ")); |
| 1565 | assert_eq!(parse_exec_tool_surface("shell_onyl"), None); |
| 1566 | } |
| 1567 | |
| 1568 | #[test] |
| 1569 | fn exec_rejects_zero_max_turns() { |
| 1570 | let err = Cli::try_parse_from(["codewhale", "exec", "--max-turns", "0", "hello"]) |
| 1571 | .expect_err("max-turns must be >= 1"); |
| 1572 | assert_eq!(err.kind(), clap::error::ErrorKind::ValueValidation); |
| 1573 | } |
| 1574 |