mirror of
https://github.com/hanzoai/dev.git
synced 2026-10-17 15:34:54 +00:00
fix(dev): review and memory run on models the provider serves
On api.hanzo.ai, automatic approval review and memory extraction and consolidation ask for enso-auto, not OpenAI's ids. Off OpenAI, the reviewer counts a model as served only when the catalog lists it: the bundled OpenAI entries ride along unlisted for lookups, so codex-auto-review looked served on every provider and a LAN engine was reviewed by a model it does not have. Now it falls back to the model the run is on. Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
Hanzo Dev
Generated
+3
@@ -3408,6 +3408,7 @@ dependencies = [
|
||||
"codex-response-debug-context",
|
||||
"codex-secrets",
|
||||
"codex-utils-process",
|
||||
"hanzo-config",
|
||||
"http 1.4.0",
|
||||
"serde",
|
||||
"serde_json",
|
||||
@@ -7383,6 +7384,7 @@ dependencies = [
|
||||
"codex-mcp",
|
||||
"codex-memories-write",
|
||||
"codex-model-provider",
|
||||
"codex-model-provider-info",
|
||||
"codex-models-manager",
|
||||
"codex-plugin",
|
||||
"codex-protocol",
|
||||
@@ -7431,6 +7433,7 @@ dependencies = [
|
||||
"vt100",
|
||||
"which",
|
||||
"windows-sys 0.52.0",
|
||||
"wiremock",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -117,6 +117,18 @@ and save steps and is never exported, so no build script sees it.
|
||||
new `DEV_VERSION` alone rebuilds hanzo-dev and nothing else.
|
||||
- `sccache --show-stats` prints at the end of every Build step, pass or fail.
|
||||
|
||||
## Review and memory models
|
||||
|
||||
Automatic approval review and memory extraction and consolidation name a model
|
||||
of their own. Upstream's ids are OpenAI's; api.hanzo.ai serves none of them. On
|
||||
the Hanzo provider (`hanzo_config::is_hanzo`, base URL `API_BASE`) all three are
|
||||
`DEFAULT_MODEL`. The reviewer takes its preferred id only when the catalog serves
|
||||
it, else the parent's model, and off OpenAI only a listed entry is served: the
|
||||
bundled OpenAI entries ride along unlisted in every catalog, for lookups. So a LAN
|
||||
engine with no catalog of its own is reviewed by the model it runs. Guardian v2's
|
||||
sync reviewer runs on that same choice; its `CodexResponsesHeaders` model only
|
||||
scopes an OpenAI-backend header. `crates/hanzo-dev/tests/review.rs` holds both.
|
||||
|
||||
## House rules
|
||||
|
||||
- Hanzo service APIs use `https://api.hanzo.ai/v1/` exclusively.
|
||||
|
||||
@@ -121,3 +121,15 @@ fn a_sign_in_writes_the_shared_credential() {
|
||||
share_credential(&home);
|
||||
assert!(std::fs::symlink_metadata(home.join("auth.json")).unwrap().is_symlink());
|
||||
}
|
||||
|
||||
/// The default profile's provider is the one that serves the Hanzo catalog; a LAN
|
||||
/// engine or any other host is not, however it is named.
|
||||
#[test]
|
||||
fn only_the_hanzo_api_is_hanzo() {
|
||||
let config: toml::Value = toml::from_str(DEFAULT_CONFIG).unwrap();
|
||||
assert!(is_hanzo(config["model_providers"]["hanzo"]["base_url"].as_str()));
|
||||
assert!(is_hanzo(Some("https://api.hanzo.ai/v1/")));
|
||||
assert!(!is_hanzo(Some("http://dgx.local:1235/v1")));
|
||||
assert!(!is_hanzo(Some("https://api.hanzo.ai.example/v1")));
|
||||
assert!(!is_hanzo(None));
|
||||
}
|
||||
|
||||
@@ -24,6 +24,11 @@ pub fn home() -> PathBuf {
|
||||
pub const API_BASE: &str = "https://api.hanzo.ai/v1";
|
||||
pub const DEFAULT_MODEL: &str = "enso-auto";
|
||||
|
||||
/// Whether a provider's base URL is [`API_BASE`], the API that serves [`catalog::MODELS`].
|
||||
pub fn is_hanzo(base_url: Option<&str>) -> bool {
|
||||
base_url.is_some_and(|url| url.trim_end_matches('/') == API_BASE)
|
||||
}
|
||||
|
||||
/// Called once by the synchronous entry point, before the async runtime starts.
|
||||
pub fn initialize() -> std::io::Result<()> {
|
||||
let home = product_home(
|
||||
|
||||
@@ -113,11 +113,13 @@ windows-sys = { version = "0.52", features = [
|
||||
] }
|
||||
|
||||
[dev-dependencies]
|
||||
codex-model-provider-info = { workspace = true }
|
||||
codex-utils-pty = { workspace = true }
|
||||
insta = { workspace = true }
|
||||
vt100 = { workspace = true }
|
||||
serde_json = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
wiremock = { workspace = true }
|
||||
|
||||
[package.metadata.cargo-shear]
|
||||
# These Rust sources are intentionally Bazel-only macrobenchmarks rather than
|
||||
|
||||
@@ -0,0 +1,236 @@
|
||||
//! Automatic approval review and memory run on models the provider serves.
|
||||
//!
|
||||
//! Upstream's own ids for these jobs are OpenAI's, and the bundled catalog keeps
|
||||
//! OpenAI's entries, unlisted, in every provider's catalog. A reviewer that took
|
||||
//! "in the catalog" for "served" asked api.hanzo.ai and the LAN engines for a
|
||||
//! model neither has.
|
||||
|
||||
use std::process::Stdio;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
use std::time::Duration;
|
||||
|
||||
use codex_model_provider::create_model_provider;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
use codex_protocol::openai_models::ModelVisibility;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
use wiremock::Request;
|
||||
use wiremock::Respond;
|
||||
use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
fn provider(table: &str) -> ModelProviderInfo {
|
||||
toml::from_str(table).expect("a provider table parses")
|
||||
}
|
||||
|
||||
fn hanzo() -> ModelProviderInfo {
|
||||
let config: toml::Value =
|
||||
toml::from_str(hanzo_config::DEFAULT_CONFIG).expect("the default config parses");
|
||||
config["model_providers"]["hanzo"]
|
||||
.clone()
|
||||
.try_into()
|
||||
.expect("the default config's provider parses")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hanzo_names_a_model_it_serves_for_review_and_memory() {
|
||||
let hanzo = create_model_provider(hanzo(), None);
|
||||
for model in [
|
||||
hanzo.approval_review_preferred_model(),
|
||||
hanzo.memory_extraction_preferred_model(),
|
||||
hanzo.memory_consolidation_preferred_model(),
|
||||
] {
|
||||
assert_eq!(model, hanzo_config::DEFAULT_MODEL);
|
||||
let listed = codex_models_manager::bundled_models_response()
|
||||
.expect("the bundled catalog parses")
|
||||
.models
|
||||
.into_iter()
|
||||
.any(|known| known.slug == model && matches!(known.visibility, ModelVisibility::List));
|
||||
assert!(
|
||||
listed,
|
||||
"{model} is listed in the catalog api.hanzo.ai serves"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn only_hanzo_takes_the_hanzo_choice() {
|
||||
let openai = create_model_provider(ModelProviderInfo::create_openai_provider(None), None);
|
||||
let lan = create_model_provider(
|
||||
provider(
|
||||
"name = \"LAN\"\nbase_url = \"http://dgx.local:1235/v1\"\nwire_api = \"responses\"",
|
||||
),
|
||||
None,
|
||||
);
|
||||
for other in [openai, lan] {
|
||||
assert_ne!(
|
||||
other.approval_review_preferred_model(),
|
||||
hanzo_config::DEFAULT_MODEL
|
||||
);
|
||||
assert_ne!(
|
||||
other.memory_extraction_preferred_model(),
|
||||
hanzo_config::DEFAULT_MODEL
|
||||
);
|
||||
assert_ne!(
|
||||
other.memory_consolidation_preferred_model(),
|
||||
hanzo_config::DEFAULT_MODEL
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn sse(events: &[Value]) -> String {
|
||||
events
|
||||
.iter()
|
||||
.map(|event| {
|
||||
format!(
|
||||
"event: {}\ndata: {event}\n\n",
|
||||
event["type"].as_str().unwrap_or_default()
|
||||
)
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn created(id: &str) -> Value {
|
||||
json!({"type": "response.created", "response": {"id": id}})
|
||||
}
|
||||
|
||||
fn completed(id: &str) -> Value {
|
||||
json!({
|
||||
"type": "response.completed",
|
||||
"response": {
|
||||
"id": id,
|
||||
"usage": {"input_tokens": 0, "input_tokens_details": null, "output_tokens": 0,
|
||||
"output_tokens_details": null, "total_tokens": 0}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn message(id: &str, text: &str) -> Value {
|
||||
json!({
|
||||
"type": "response.output_item.done",
|
||||
"item": {"type": "message", "role": "assistant", "id": id,
|
||||
"content": [{"type": "output_text", "text": text}]}
|
||||
})
|
||||
}
|
||||
|
||||
/// A Responses engine that asks once for an escalated command, allows it when
|
||||
/// asked to review, and finishes once it sees the command's output.
|
||||
#[derive(Clone, Default)]
|
||||
struct Engine {
|
||||
reviews: Arc<Mutex<Vec<String>>>,
|
||||
outputs: Arc<Mutex<Vec<String>>>,
|
||||
}
|
||||
|
||||
impl Respond for Engine {
|
||||
fn respond(&self, request: &Request) -> ResponseTemplate {
|
||||
let body: Value = serde_json::from_slice(&request.body).unwrap_or_default();
|
||||
let reviewer = body["client_metadata"]["x-openai-subagent"] == "guardian"
|
||||
|| request
|
||||
.headers
|
||||
.get("x-openai-subagent")
|
||||
.and_then(|value| value.to_str().ok())
|
||||
== Some("guardian");
|
||||
let outputs: Vec<String> = body["input"]
|
||||
.as_array()
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.filter(|item| item["type"] == "function_call_output")
|
||||
.map(|item| item["output"].to_string())
|
||||
.collect();
|
||||
let events = if reviewer {
|
||||
let model = body["model"].as_str().unwrap_or_default().to_string();
|
||||
self.reviews.lock().expect("reviews").push(model);
|
||||
vec![
|
||||
created("review"),
|
||||
message("assessment", r#"{"outcome":"allow"}"#),
|
||||
completed("review"),
|
||||
]
|
||||
} else if !outputs.is_empty() {
|
||||
self.outputs.lock().expect("outputs").extend(outputs);
|
||||
vec![created("done"), message("reply", "done"), completed("done")]
|
||||
} else {
|
||||
let arguments = json!({
|
||||
"cmd": "printf reviewed",
|
||||
"sandbox_permissions": "require_escalated",
|
||||
"justification": "Run the step the user asked for.",
|
||||
});
|
||||
vec![
|
||||
created("act"),
|
||||
json!({
|
||||
"type": "response.output_item.done",
|
||||
"item": {"type": "function_call", "call_id": "act", "name": "exec_command",
|
||||
"arguments": arguments.to_string()}
|
||||
}),
|
||||
completed("act"),
|
||||
]
|
||||
};
|
||||
ResponseTemplate::new(200)
|
||||
.insert_header("content-type", "text/event-stream")
|
||||
.set_body_string(sse(&events))
|
||||
}
|
||||
}
|
||||
|
||||
/// A provider that is neither OpenAI nor Hanzo and has no catalog of its own (a
|
||||
/// LAN engine named with `-c model_provider=`) is reviewed by the model it is
|
||||
/// already running, never by an id it was not asked to serve.
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn a_provider_off_hanzo_reviews_on_the_model_it_runs() {
|
||||
let server = MockServer::start().await;
|
||||
let engine = Engine::default();
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/v1/responses"))
|
||||
.respond_with(engine.clone())
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let home = tempfile::tempdir().expect("a home");
|
||||
let work = tempfile::tempdir().expect("a project");
|
||||
std::fs::write(
|
||||
home.path().join("config.toml"),
|
||||
format!(
|
||||
"model_provider = \"lan\"\n\n[model_providers.lan]\nname = \"LAN\"\nbase_url = \"{}/v1\"\nwire_api = \"responses\"\n",
|
||||
server.uri()
|
||||
),
|
||||
)
|
||||
.expect("the profile is writable");
|
||||
|
||||
let run = tokio::process::Command::new(env!("CARGO_BIN_EXE_dev"))
|
||||
.args([
|
||||
"exec",
|
||||
"--skip-git-repo-check",
|
||||
"--approve-for-me",
|
||||
"-m",
|
||||
"zen6",
|
||||
"-C",
|
||||
])
|
||||
.arg(work.path())
|
||||
.arg("run the protected step")
|
||||
.env("DEV_HOME", home.path())
|
||||
.stdin(Stdio::null())
|
||||
.output();
|
||||
let output = tokio::time::timeout(Duration::from_secs(180), run)
|
||||
.await
|
||||
.expect("the run finishes")
|
||||
.expect("dev runs");
|
||||
let reviews = engine.reviews.lock().expect("reviews").clone();
|
||||
let outputs = engine.outputs.lock().expect("outputs").clone();
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"dev exec failed\nstdout: {}\nstderr: {}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
assert!(!reviews.is_empty(), "the escalated command was reviewed");
|
||||
assert!(
|
||||
reviews.iter().all(|model| model == "zen6"),
|
||||
"reviewed on {reviews:?}"
|
||||
);
|
||||
assert!(
|
||||
outputs.iter().any(|output| output.contains("reviewed")),
|
||||
"the allowed command ran: {outputs:?}"
|
||||
);
|
||||
}
|
||||
@@ -616,5 +616,23 @@
|
||||
"old": " separate_status_line: true,",
|
||||
"new": " separate_status_line: hanzo_tui::SEPARATE_STATUS_LINE,",
|
||||
"count": 1
|
||||
},
|
||||
{
|
||||
"file": "model-provider/Cargo.toml",
|
||||
"old": "[dependencies]",
|
||||
"new": "[dependencies]\nhanzo-config = { path = \"../../../../crates/hanzo-config\" }",
|
||||
"count": 1
|
||||
},
|
||||
{
|
||||
"file": "model-provider/src/provider.rs",
|
||||
"old": " fn approval_review_preferred_model(&self) -> &'static str {\n if self\n .auth_manager\n .as_ref()\n .and_then(|auth_manager| auth_manager.auth_cached())\n .is_some_and(|auth| auth.is_api_key_auth())\n {\n API_KEY_APPROVAL_REVIEW_PREFERRED_MODEL\n } else {\n DEFAULT_APPROVAL_REVIEW_PREFERRED_MODEL\n }\n }\n",
|
||||
"new": " fn approval_review_preferred_model(&self) -> &'static str {\n // Hanzo: api.hanzo.ai serves none of OpenAI's own ids; Enso routes the review.\n if hanzo_config::is_hanzo(self.info.base_url.as_deref()) {\n return hanzo_config::DEFAULT_MODEL;\n }\n if self\n .auth_manager\n .as_ref()\n .and_then(|auth_manager| auth_manager.auth_cached())\n .is_some_and(|auth| auth.is_api_key_auth())\n {\n API_KEY_APPROVAL_REVIEW_PREFERRED_MODEL\n } else {\n DEFAULT_APPROVAL_REVIEW_PREFERRED_MODEL\n }\n }\n\n fn memory_extraction_preferred_model(&self) -> &'static str {\n if hanzo_config::is_hanzo(self.info.base_url.as_deref()) {\n return hanzo_config::DEFAULT_MODEL;\n }\n DEFAULT_MEMORY_EXTRACTION_PREFERRED_MODEL\n }\n\n fn memory_consolidation_preferred_model(&self) -> &'static str {\n if hanzo_config::is_hanzo(self.info.base_url.as_deref()) {\n return hanzo_config::DEFAULT_MODEL;\n }\n DEFAULT_MEMORY_CONSOLIDATION_PREFERRED_MODEL\n }\n",
|
||||
"count": 1
|
||||
},
|
||||
{
|
||||
"file": "core/src/guardian/reviewer_config.rs",
|
||||
"old": " let available_models = session\n .services\n .models_manager\n .list_models(\n codex_models_manager::manager::RefreshStrategy::Offline,\n turn.config.http_client_factory(),\n )\n .await;\n let default_review_model_id = turn.provider.approval_review_preferred_model();\n",
|
||||
"new": " let mut available_models = session\n .services\n .models_manager\n .list_models(\n codex_models_manager::manager::RefreshStrategy::Offline,\n turn.config.http_client_factory(),\n )\n .await;\n // Hanzo: off OpenAI a provider serves the models its catalog lists. The bundled\n // OpenAI entries ride along unlisted, for lookups, and no other provider serves them.\n if !turn.provider.info().is_openai() {\n available_models.retain(|preset| preset.show_in_picker);\n }\n let default_review_model_id = turn.provider.approval_review_preferred_model();\n",
|
||||
"count": 1
|
||||
}
|
||||
]
|
||||
|
||||
Reference in New Issue
Block a user