review classifier: run on ACP when the swarm settings choose it
The swarm settings gain `runtime` (`api-key`, the default, or `acp`) and an `acp` provider record: an https base URL, the provider's model, and its prices per million tokens. A PUT now changes only the fields its body names, so the settings form that sends model, effort and limit leaves the runtime alone; `null` clears `effort` or `acp`. The body stays strict, and the stored document keeps #5192's lenient read. The controller sends the runtime in each run request. On `acp` the classifier runs one prompt on a new `opencode acp` session per classification through hive-runtime's AcpRuntime, in a scratch directory, with a config rendered from the settings. opencode runs `read` and other tools without asking, so OPENCODE_PERMISSION {"*":"deny"} takes every tool away from the model, and a deny-all policy refuses any request on top. The title call, which resends the prompt, is turned off. The deadline cancels the turn, and a turn that does not end within 15s of that is dropped, which also covers an agent stuck before its prompt. The answer is the final assistant text, as ACP has no result event. The provider key is read per run from swarm/services/review-classifier/acp-provider, which the bao UI's admins login may create and update, and reaches opencode through AcpRuntime::with_provider_key and the trampoline's allowlist. Each ACP classification's cost is computed from the configured prices and the reported tokens, and added to hyperhive.review_classifier.cost.usage labelled repo, pr, model and currency. Part of #5096
This commit is contained in:
parent
38ee68c31d
commit
1b4e764e5c
24 changed files with 1622 additions and 262 deletions
|
|
@ -1,11 +1,13 @@
|
|||
//! Answering one try-out: read the API key, ask the model one classification
|
||||
//! at a time, and turn the answers into the reply.
|
||||
//! Answering one try-out: read the key of the run's runtime, ask the model one
|
||||
//! classification at a time, and turn the answers into the reply.
|
||||
//!
|
||||
//! Each classification is one `claude --print` with no session, no tools and
|
||||
//! no MCP server, in a directory made for it and removed after it, so nothing
|
||||
//! carries from one run to the next. The API key is read from the store for
|
||||
//! every run and reaches only the child's environment, which keeps nothing of
|
||||
//! the daemon's beyond [`crate::child_env`]'s allowlist.
|
||||
//! On the API key runtime each classification is one `claude --print` with
|
||||
//! no session, no tools and no MCP server; on the ACP runtime it is one
|
||||
//! prompt on a new ACP session ([`crate::acp`]). Either way it runs in a
|
||||
//! directory made for it and removed after it, so nothing carries from one
|
||||
//! run to the next. The key is read from the store for every run and reaches
|
||||
//! only the child's environment, which keeps nothing of the daemon's beyond
|
||||
//! [`crate::child_env`]'s allowlist.
|
||||
|
||||
use std::future::Future;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
|
@ -15,33 +17,33 @@ use std::time::Duration;
|
|||
|
||||
use hive_claude::{Attach, Config, Sink};
|
||||
use swarm_queue_client::agent_inference::Effort;
|
||||
use swarm_queue_client::review_classifier::{CLASSIFICATION_DEADLINE, Reply, Row, TryOut};
|
||||
use swarm_secret_client::review_classifier::{ApiKey, api_key_path};
|
||||
use swarm_queue_client::review_classifier::{CLASSIFICATION_DEADLINE, Reply, Row, Runtime, TryOut};
|
||||
use swarm_secret_client::acp::ProviderKey;
|
||||
use swarm_secret_client::review_classifier::{ApiKey, acp_provider_key_path, api_key_path};
|
||||
|
||||
use crate::{aggregate, answer, prompt};
|
||||
|
||||
/// How long `claude` may print nothing before it is stopped.
|
||||
const IDLE_TIMEOUT: Duration = Duration::from_mins(2);
|
||||
|
||||
/// Where the run's API key comes from: the store in production, a fake in
|
||||
/// the tests.
|
||||
/// Where the run's key comes from: the store in production, a fake in the
|
||||
/// tests.
|
||||
pub trait KeySource {
|
||||
/// The key, or why there is none, in words the operator acts on.
|
||||
fn api_key(&self) -> impl Future<Output = Result<String, String>> + Send;
|
||||
/// The key `runtime` runs on, or why there is none, in words the
|
||||
/// operator acts on.
|
||||
fn key(&self, runtime: &Runtime) -> impl Future<Output = Result<String, String>> + Send;
|
||||
}
|
||||
|
||||
/// Asks the model one classification: `claude` in production, a fake in the
|
||||
/// tests.
|
||||
/// Asks the model one classification: [`Runtimes`] in production, a fake in
|
||||
/// the tests.
|
||||
pub trait Model {
|
||||
/// The final message of `model` at `effort` for `prompt`, or why there
|
||||
/// is none.
|
||||
/// The final message for `prompt` on `run`'s runtime, model and effort,
|
||||
/// run with `key`, or why there is none.
|
||||
fn ask(
|
||||
&self,
|
||||
prompt: &str,
|
||||
api_key: &str,
|
||||
resource_attributes: &str,
|
||||
model: &str,
|
||||
effort: Option<Effort>,
|
||||
key: &str,
|
||||
run: &TryOut,
|
||||
) -> impl Future<Output = Result<String, String>> + Send;
|
||||
}
|
||||
|
||||
|
|
@ -49,22 +51,15 @@ pub trait Model {
|
|||
/// run; otherwise each classification is a row, with an error where it has no
|
||||
/// answer.
|
||||
pub async fn answer(keys: &impl KeySource, model: &impl Model, run: &TryOut) -> Reply {
|
||||
let api_key = match keys.api_key().await {
|
||||
let key = match keys.key(&run.runtime).await {
|
||||
Ok(key) => key,
|
||||
Err(error) => return Reply::Failed { error },
|
||||
};
|
||||
let shared = prompt::shared_context(run);
|
||||
let attributes = resource_attributes(std::env::var("OTEL_RESOURCE_ATTRIBUTES").ok(), run);
|
||||
let mut rows: Vec<Row> = Vec::with_capacity(run.classifications.len());
|
||||
for c in &run.classifications {
|
||||
let outcome = model
|
||||
.ask(
|
||||
&prompt::classification_prompt(&shared, c),
|
||||
&api_key,
|
||||
&attributes,
|
||||
&run.model,
|
||||
run.effort,
|
||||
)
|
||||
.ask(&prompt::classification_prompt(&shared, c), &key, run)
|
||||
.await
|
||||
.and_then(|text| answer::parse(&text));
|
||||
rows.push(aggregate::row(c, outcome));
|
||||
|
|
@ -94,29 +89,72 @@ fn resource_attributes(base: Option<String>, run: &TryOut) -> String {
|
|||
}
|
||||
}
|
||||
|
||||
/// Each classification on the model its run's runtime names.
|
||||
pub struct Runtimes<C, A> {
|
||||
pub claude: C,
|
||||
pub acp: A,
|
||||
}
|
||||
|
||||
impl<C: Model + Sync, A: Model + Sync> Model for Runtimes<C, A> {
|
||||
async fn ask(&self, prompt: &str, key: &str, run: &TryOut) -> Result<String, String> {
|
||||
match run.runtime {
|
||||
Runtime::ApiKey => self.claude.ask(prompt, key, run).await,
|
||||
Runtime::Acp(_) => self.acp.ask(prompt, key, run).await,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The store, logged in to for each read: a run can come long after the last
|
||||
/// one's token expired.
|
||||
pub struct Store;
|
||||
|
||||
impl KeySource for Store {
|
||||
async fn api_key(&self) -> Result<String, String> {
|
||||
let path = api_key_path().map_err(|e| e.to_string())?;
|
||||
let store = swarm_secret_client::SecretStore::from_env(crate::CERT_ROLE)
|
||||
.await
|
||||
.map_err(|e| format!("logging in to the secret store to read the API key: {e}"))?;
|
||||
let stored: Option<ApiKey> = store
|
||||
.read_optional(&path)
|
||||
.await
|
||||
.map_err(|e| format!("reading secret/{path}: {e}"))?;
|
||||
match stored {
|
||||
Some(k) if !k.api_key.trim().is_empty() => Ok(k.api_key.trim().to_owned()),
|
||||
_ => Err(format!(
|
||||
"no API key at secret/{path}: write it in the secret store's UI, field `api_key`"
|
||||
)),
|
||||
async fn key(&self, runtime: &Runtime) -> Result<String, String> {
|
||||
match runtime {
|
||||
Runtime::ApiKey => {
|
||||
let path = api_key_path().map_err(|e| e.to_string())?;
|
||||
let key = read_key(&path, "API key", |k: ApiKey| k.api_key).await?;
|
||||
key.ok_or_else(|| {
|
||||
format!(
|
||||
"no API key at secret/{path}: write it in the secret store's UI, field \
|
||||
`api_key`"
|
||||
)
|
||||
})
|
||||
}
|
||||
Runtime::Acp(_) => {
|
||||
let path = acp_provider_key_path().map_err(|e| e.to_string())?;
|
||||
let key = read_key(&path, "ACP provider key", |k: ProviderKey| k.api_key).await?;
|
||||
key.ok_or_else(|| {
|
||||
format!(
|
||||
"no ACP provider key at secret/{path}: write it in the secret store's \
|
||||
UI, field `api_key`"
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The key `field` takes from the object at `path`, trimmed; `None` when
|
||||
/// nothing or a blank is stored.
|
||||
async fn read_key<T: serde::de::DeserializeOwned>(
|
||||
path: &str,
|
||||
what: &str,
|
||||
field: impl FnOnce(T) -> String,
|
||||
) -> Result<Option<String>, String> {
|
||||
let store = swarm_secret_client::SecretStore::from_env(crate::CERT_ROLE)
|
||||
.await
|
||||
.map_err(|e| format!("logging in to the secret store to read the {what}: {e}"))?;
|
||||
let stored: Option<T> = store
|
||||
.read_optional(path)
|
||||
.await
|
||||
.map_err(|e| format!("reading secret/{path}: {e}"))?;
|
||||
Ok(stored
|
||||
.map(field)
|
||||
.map(|key| key.trim().to_owned())
|
||||
.filter(|key| !key.is_empty()))
|
||||
}
|
||||
|
||||
/// The `claude` binary the classifications run on, started through the
|
||||
/// trampoline that clears its environment.
|
||||
pub struct Claude {
|
||||
|
|
@ -127,17 +165,11 @@ pub struct Claude {
|
|||
}
|
||||
|
||||
impl Model for Claude {
|
||||
async fn ask(
|
||||
&self,
|
||||
prompt: &str,
|
||||
api_key: &str,
|
||||
attributes: &str,
|
||||
model: &str,
|
||||
effort: Option<Effort>,
|
||||
) -> Result<String, String> {
|
||||
async fn ask(&self, prompt: &str, api_key: &str, run: &TryOut) -> Result<String, String> {
|
||||
let attributes = resource_attributes(std::env::var("OTEL_RESOURCE_ATTRIBUTES").ok(), run);
|
||||
let dir = scratch_dir();
|
||||
let outcome = self
|
||||
.ask_in(&dir, prompt, api_key, attributes, model, effort)
|
||||
.ask_in(&dir, prompt, api_key, &attributes, &run.model, run.effort)
|
||||
.await;
|
||||
if let Err(e) = tokio::fs::remove_dir_all(&dir).await {
|
||||
tracing::warn!(dir = %dir.display(), error = %e, "removing a run's directory failed");
|
||||
|
|
@ -219,7 +251,7 @@ impl Claude {
|
|||
}
|
||||
|
||||
/// A directory of its own for one classification.
|
||||
fn scratch_dir() -> PathBuf {
|
||||
pub fn scratch_dir() -> PathBuf {
|
||||
static NEXT: AtomicU64 = AtomicU64::new(0);
|
||||
std::env::temp_dir().join(format!(
|
||||
"review-classifier-{}-{}",
|
||||
|
|
@ -268,11 +300,18 @@ mod tests {
|
|||
|
||||
use super::*;
|
||||
|
||||
struct Key(Result<&'static str, &'static str>);
|
||||
/// The API key runtime's key is `k` and the ACP runtime's `p`, or both
|
||||
/// fail with the same error.
|
||||
struct Key(Result<(), &'static str>);
|
||||
|
||||
impl KeySource for Key {
|
||||
async fn api_key(&self) -> Result<String, String> {
|
||||
self.0.map(str::to_owned).map_err(str::to_owned)
|
||||
async fn key(&self, runtime: &Runtime) -> Result<String, String> {
|
||||
self.0.map_err(str::to_owned)?;
|
||||
Ok(match runtime {
|
||||
Runtime::ApiKey => "k",
|
||||
Runtime::Acp(_) => "p",
|
||||
}
|
||||
.to_owned())
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -280,17 +319,16 @@ mod tests {
|
|||
struct Scripted;
|
||||
|
||||
impl Model for Scripted {
|
||||
async fn ask(
|
||||
&self,
|
||||
prompt: &str,
|
||||
api_key: &str,
|
||||
attrs: &str,
|
||||
model: &str,
|
||||
effort: Option<Effort>,
|
||||
) -> Result<String, String> {
|
||||
assert_eq!(api_key, "k");
|
||||
assert_eq!((model, effort), ("sonnet", Some(Effort::Low)));
|
||||
assert!(attrs.ends_with("repo=o/r,pr=7"), "{attrs}");
|
||||
async fn ask(&self, prompt: &str, key: &str, run: &TryOut) -> Result<String, String> {
|
||||
let wanted = match run.runtime {
|
||||
Runtime::ApiKey => "k",
|
||||
Runtime::Acp(_) => "p",
|
||||
};
|
||||
assert_eq!(key, wanted, "the key of the run's runtime");
|
||||
assert_eq!(
|
||||
(run.model.as_str(), run.effort),
|
||||
("sonnet", Some(Effort::Low))
|
||||
);
|
||||
if prompt.contains("ANSWER-FAIL") {
|
||||
Err("claude failed: exit 1".to_owned())
|
||||
} else if prompt.contains("ANSWER-MATCH") {
|
||||
|
|
@ -321,6 +359,23 @@ mod tests {
|
|||
.collect(),
|
||||
model: "sonnet".to_owned(),
|
||||
effort: Some(Effort::Low),
|
||||
runtime: Runtime::ApiKey,
|
||||
}
|
||||
}
|
||||
|
||||
fn acp_run(descriptions: &[&str]) -> TryOut {
|
||||
TryOut {
|
||||
runtime: Runtime::Acp(swarm_queue_client::review_classifier::Acp {
|
||||
base_url: "https://llm.example/v1".to_owned(),
|
||||
model: "qwen".to_owned(),
|
||||
prices: swarm_queue_client::review_classifier::Prices {
|
||||
input: 1.0,
|
||||
output: 1.0,
|
||||
cache_read: None,
|
||||
cache_write: None,
|
||||
},
|
||||
}),
|
||||
..run(descriptions)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -343,7 +398,7 @@ mod tests {
|
|||
#[tokio::test]
|
||||
async fn a_failed_or_malformed_answer_is_an_excluded_row_beside_the_others() {
|
||||
let reply = answer(
|
||||
&Key(Ok("k")),
|
||||
&Key(Ok(())),
|
||||
&Scripted,
|
||||
&run(&["ANSWER-MATCH", "ANSWER-FAIL", "ANSWER-JUNK"]),
|
||||
)
|
||||
|
|
@ -366,7 +421,7 @@ mod tests {
|
|||
#[tokio::test]
|
||||
async fn no_answer_at_all_fails_the_run_instead_of_approving() {
|
||||
let reply = answer(
|
||||
&Key(Ok("k")),
|
||||
&Key(Ok(())),
|
||||
&Scripted,
|
||||
&run(&["ANSWER-FAIL", "ANSWER-JUNK"]),
|
||||
)
|
||||
|
|
@ -377,6 +432,37 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn an_acp_run_is_asked_with_the_acp_key() {
|
||||
let reply = answer(&Key(Ok(())), &Scripted, &acp_run(&["ANSWER-MATCH"])).await;
|
||||
assert!(matches!(reply, Reply::Done { .. }), "{reply:?}");
|
||||
}
|
||||
|
||||
/// Answers with its own name.
|
||||
struct Named(&'static str);
|
||||
|
||||
impl Model for Named {
|
||||
async fn ask(&self, _: &str, _: &str, _: &TryOut) -> Result<String, String> {
|
||||
Ok(self.0.to_owned())
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn each_run_goes_to_the_model_its_runtime_names() {
|
||||
let runtimes = Runtimes {
|
||||
claude: Named("claude"),
|
||||
acp: Named("acp"),
|
||||
};
|
||||
assert_eq!(
|
||||
runtimes.ask("p", "k", &run(&[])).await.as_deref(),
|
||||
Ok("claude")
|
||||
);
|
||||
assert_eq!(
|
||||
runtimes.ask("p", "k", &acp_run(&[])).await.as_deref(),
|
||||
Ok("acp")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn claude_is_started_through_the_trampoline() {
|
||||
let claude = Claude {
|
||||
|
|
|
|||
Loading…
Reference in a new issue