Skip to content

Commit 2f01858

Browse files
committed
fix(cli): resolve model aliases before syntax validation
This patch implements the feedback from the PR review: - Swaps the order of resolution and validation to support aliases. - Fixes a bug in the validator that allowed malformed models to pass silently. - Adds debug! logging for alias resolution traceability. - Adds unit tests covering alias resolution and syntax validation.
1 parent ae2ccd9 commit 2f01858

2 files changed

Lines changed: 44 additions & 40 deletions

File tree

rust/crates/rusty-claude-cli/src/main.rs

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -15069,38 +15069,38 @@ mod alias_resolution_tests {
1506915069
#[test]
1507015070
fn test_alias_resolution_builtin() {
1507115071
// Built-in aliases should resolve to their full IDs
15072-
assert_eq!(resolve_model_alias_with_config(" opus\), \claude-opus-4-6\);
15073-
assert_eq!(resolve_model_alias_with_config(\sonnet\), \claude-sonnet-4-6\);
15074-
assert_eq!(resolve_model_alias_with_config(\haiku\), \claude-haiku-4-5-20251213\);
15075-
}
15076-
15077-
#[test]
15078-
fn test_alias_resolution_syntax_validation() {
15079-
// Resolved aliases should pass syntax validation
15080-
let resolved = resolve_model_alias_with_config(\opus\);
15081-
assert!(validate_model_syntax(&resolved).is_ok());
15082-
15083-
// Raw aliases should FAIL syntax validation (this is why we resolve first!)
15084-
assert!(validate_model_syntax(\opus\).is_err());
15085-
}
15086-
15087-
#[test]
15088-
fn test_unknown_alias_fails_validation() {
15089-
// Unknown aliases resolve to themselves
15090-
let resolved = resolve_model_alias_with_config(\unknown-alias\);
15091-
assert_eq!(resolved, \unknown-alias\);
15092-
15093-
// And then fail validation with a helpful error
15094-
let result = validate_model_syntax(&resolved);
15095-
assert!(result.is_err());
15096-
assert!(result.unwrap_err().contains(\invalid model syntax\));
15097-
}
15098-
15099-
#[test]
15100-
fn test_direct_provider_model_passes() {
15101-
// Direct provider/model strings should remain unchanged and pass
15102-
let model = \openai/gpt-4o\;
15103-
assert_eq!(resolve_model_alias_with_config(model), model);
15104-
assert!(validate_model_syntax(model).is_ok());
15105-
}
15072+
assert_eq!(resolve_model_alias_with_config("opus"), "claude-opus-4-6");
15073+
assert_eq!(resolve_model_alias_with_config("sonnet"), "claude-sonnet-4-6");
15074+
assert_eq!(resolve_model_alias_with_config("haiku"), "claude-haiku-4-5-20251213");
15075+
}
15076+
15077+
#[test]
15078+
fn test_alias_resolution_syntax_validation() {
15079+
// Resolved aliases should pass syntax validation
15080+
let resolved = resolve_model_alias_with_config("opus");
15081+
assert!(validate_model_syntax(&resolved).is_ok());
15082+
15083+
// Raw aliases should FAIL syntax validation (this is why we resolve first!)
15084+
assert!(validate_model_syntax("opus").is_err());
15085+
}
15086+
15087+
#[test]
15088+
fn test_unknown_alias_fails_validation() {
15089+
// Unknown aliases resolve to themselves
15090+
let resolved = resolve_model_alias_with_config("unknown-alias");
15091+
assert_eq!(resolved, "unknown-alias");
15092+
15093+
// And then fail validation with a helpful error
15094+
let result = validate_model_syntax(&resolved);
15095+
assert!(result.is_err());
15096+
assert!(result.unwrap_err().contains("invalid model syntax"));
15097+
}
15098+
15099+
#[test]
15100+
fn test_direct_provider_model_passes() {
15101+
// Direct provider/model strings should remain unchanged and pass
15102+
let model = "openai/gpt-4o";
15103+
assert_eq!(resolve_model_alias_with_config(model), model);
15104+
assert!(validate_model_syntax(model).is_ok());
15105+
}
1510615106
}

rust/crates/rusty-claude-cli/tests/mock_parity_harness.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::collections::BTreeMap;
22
use std::fs;
33
use std::io::Write;
4-
use std::os::unix::fs::PermissionsExt;
4+
55
use std::path::{Path, PathBuf};
66
use std::process::{Command, Output, Stdio};
77
use std::sync::atomic::{AtomicU64, Ordering};
@@ -426,11 +426,15 @@ fn prepare_plugin_fixture(workspace: &HarnessWorkspace) {
426426
"#!/bin/sh\nINPUT=$(cat)\nprintf '{\"plugin\":\"%s\",\"tool\":\"%s\",\"input\":%s}\\n' \"$CLAWD_PLUGIN_ID\" \"$CLAWD_TOOL_NAME\" \"$INPUT\"\n",
427427
)
428428
.expect("plugin script should write");
429-
let mut permissions = fs::metadata(&script_path)
430-
.expect("plugin script metadata")
431-
.permissions();
432-
permissions.set_mode(0o755);
433-
fs::set_permissions(&script_path, permissions).expect("plugin script should be executable");
429+
#[cfg(unix)]
430+
{
431+
use std::os::unix::fs::PermissionsExt;
432+
let mut permissions = fs::metadata(&script_path)
433+
.expect("plugin script metadata")
434+
.permissions();
435+
permissions.set_mode(0o755);
436+
fs::set_permissions(&script_path, permissions).expect("plugin script should be executable");
437+
}
434438

435439
fs::write(
436440
manifest_dir.join("plugin.json"),

0 commit comments

Comments
 (0)