Watch
0
0
Fork
You've already forked hyperhive
0
hyperhive/swarm-review-classifier/src/acp.rs
atlas ce7b3e055f review-classifier: ask a check once more when its answer isn't valid JSON
An answer that fails to validate is asked for a second time, for that
classification only, with a reminder to reply with the JSON object alone.
The second ask comes out of the classification's CLASSIFICATION_DEADLINE:
it gets what the first ask left, and is skipped when less than 20 seconds
remain, so RUN_DEADLINE's bound still holds. A failed ask (runtime error,
timeout) is not repeated. If the second answer does not validate either,
the row has no answer, as before.

Model::ask takes the deadline for the ask; the ACP runtime no longer holds
its own.

The parser also takes the first fenced block of a reply that has prose
around the fence; prose around a bare object is still rejected.
2026-10-16 16:55:07 +02:00

598 lines
23 KiB
Rust

//! One classification on the ACP runtime: `opencode acp`, driven through
//! [`hive_runtime::AcpRuntime`] for one prompt on a new session. Its config,
//! system prompt and opencode's state live in a directory made for it and
//! removed after it, its `HOME`; its working directory is [`Acp::cwd`],
//! shared by every run.
//!
//! The agent has no tool at all. opencode runs some tools, `read` among them,
//! without asking, so a deny-all permission policy alone would never see them:
//! [`DENY_ALL`] in `OPENCODE_PERMISSION` makes opencode offer the model no
//! tool, and the policy refuses whatever is still asked. A tool would let a
//! PR's diff talk the model into reading this unit's credentials into its
//! `reason`, which the controller posts on the forge.
//!
//! The rendered config names one provider and model, from the run, and turns
//! off opencode's title call, which would send the prompt a second time. It
//! also defines the agent the session runs as, [`AGENT_ID`], whose prompt is
//! the classifier's system prompt: opencode sends an agent's prompt as the
//! system message, so the first user message holds only the PR. That agent
//! must carry no `permission` or `tools` key: opencode merges an agent's own
//! permission over `OPENCODE_PERMISSION`, so one there could offer tools again.
use std::collections::BTreeMap;
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex};
use std::time::Duration;
use hive_runtime::{
AcpCommand, AcpRuntime, Config, PermissionAsk, PermissionPolicy, Runtime as _, Sink,
};
use serde_json::{Value, json};
use swarm_queue_client::review_classifier::{Acp as Provider, Runtime, TryOut};
use crate::cost::{Cost, CostSink};
/// The variable the provider key reaches `opencode` in.
pub const PROVIDER_KEY_VAR: &str = "ACP_PROVIDER_API_KEY";
/// `OPENCODE_PERMISSION`: every tool denied, which opencode merges over its
/// config.
const DENY_ALL: &str = r#"{"*":"deny"}"#;
/// The provider's id in the rendered config.
const PROVIDER_ID: &str = "review-classifier";
/// The agent the session runs as, which carries the system prompt.
const AGENT_ID: &str = "review-classifier";
/// The system prompt's file, beside the rendered config. opencode reads it
/// through a `{file:…}` reference, relative to the config's directory.
const SYSTEM_FILE: &str = "system.md";
/// How long the agent may send nothing before its turn is cancelled.
const IDLE_TIMEOUT: Duration = Duration::from_mins(2);
/// How long a turn cancelled at the deadline has to end before it is dropped,
/// which interrupts the agent: past `hive-runtime`'s own grace before it sends
/// SIGINT. A cancel reaches only a turn whose prompt has been sent, so an
/// agent stuck before that needs the drop.
pub const CANCEL_WAIT: Duration = Duration::from_secs(15);
/// The `opencode` binary the classifications run on, started through the
/// trampoline that clears its environment.
pub struct Acp<S> {
pub program: PathBuf,
/// This binary, which execs [`Self::program`] when started as the
/// trampoline.
pub trampoline: PathBuf,
/// [`CANCEL_WAIT`] outside the tests.
pub cancel_wait: Duration,
pub cost: S,
/// The empty working directory every run's session gets. opencode writes
/// it into the system message, so one path for every run keeps that
/// message the same and the provider's prompt cache usable; with no tool
/// and no project config, nothing writes into it.
pub cwd: PathBuf,
}
impl<S: CostSink> crate::run::Model for Acp<S> {
async fn ask(
&self,
prompt: &crate::prompt::Prompt<'_>,
key: &str,
run: &TryOut,
deadline: Duration,
) -> Result<String, String> {
let Runtime::Acp(provider) = &run.runtime else {
return Err("not an ACP run".to_owned());
};
let dir = crate::run::scratch_dir();
let outcome = self
.ask_in(&dir, &prompt.text(), key, run, provider, deadline)
.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");
}
outcome
}
}
impl<S: CostSink> Acp<S> {
async fn ask_in(
&self,
dir: &Path,
prompt: &str,
key: &str,
run: &TryOut,
provider: &Provider,
deadline: Duration,
) -> Result<String, String> {
let system = dir.join(SYSTEM_FILE);
let config_file = dir.join("opencode.json");
tokio::fs::create_dir_all(dir)
.await
.and(tokio::fs::create_dir_all(&self.cwd).await)
.and(tokio::fs::write(&system, crate::prompt::SYSTEM).await)
.and(tokio::fs::write(&config_file, opencode_config(provider).to_string()).await)
.map_err(|e| format!("preparing the run's directory: {e}"))?;
let runtime = AcpRuntime::new(
self.command(dir, &config_file),
dir.join("session"),
deny_all(),
hive_claude::NeverCompact,
)
.with_provider_key(key.to_owned());
let canceller = runtime
.canceller()
.ok_or("the ACP runtime offers no cancel handle")?;
let config = Config {
effort: run.effort.map(|e| e.as_str().to_owned()),
cwd: Some(self.cwd.clone()),
idle_timeout: Some(IDLE_TIMEOUT),
..Config::default()
};
let sink = FinalText::default();
let turn = runtime.run(&config, prompt, &sink);
tokio::pin!(turn);
match tokio::time::timeout(deadline, &mut turn).await {
Ok(Ok(progress)) => {
self.record_cost(run, provider, &progress.telemetry.cost);
sink.take()
}
Ok(Err(e)) => Err(format!("the ACP agent failed: {e}")),
Err(_) => {
let _ = canceller.cancel();
let _ = tokio::time::timeout(self.cancel_wait, turn).await;
Err(format!("no answer within {} seconds", deadline.as_secs()))
}
}
}
/// One classification's invocation: the trampoline as the program, with
/// the real `opencode` and the run's own variables in its environment.
fn command(&self, dir: &Path, config_file: &Path) -> AcpCommand {
let env: BTreeMap<String, String> = [
(
crate::child_env::EXEC_OPENCODE,
self.program.display().to_string(),
),
("HOME", dir.display().to_string()),
("OPENCODE_CONFIG", config_file.display().to_string()),
("OPENCODE_PERMISSION", DENY_ALL.to_owned()),
("OPENCODE_DISABLE_PROJECT_CONFIG", "1".to_owned()),
]
.into_iter()
.map(|(k, v)| (k.to_owned(), v))
.collect();
AcpCommand {
command: self.trampoline.display().to_string(),
args: vec!["acp".to_owned()],
env,
api_key_env: Some(PROVIDER_KEY_VAR.to_owned()),
}
}
/// Record what `usage` cost at the run's prices. An agent that reports no
/// tokens leaves the classification's cost unknown, not zero.
fn record_cost(&self, run: &TryOut, provider: &Provider, usage: &hive_runtime::TokenUsage) {
if *usage == hive_runtime::TokenUsage::default() {
tracing::warn!(
repo = %run.repo, pr = run.pr,
"the ACP agent reported no token counts, so this classification's cost is unknown"
);
return;
}
self.cost.record(&Cost {
repo: run.repo.clone(),
pr: run.pr,
model: provider.model.clone(),
amount: crate::cost::amount(&provider.prices, usage),
usage: *usage,
});
}
}
/// Refuses every permission request.
fn deny_all() -> PermissionPolicy {
Arc::new(|_: &PermissionAsk<'_>| false)
}
/// The `opencode.json` of a run on `provider`.
fn opencode_config(provider: &Provider) -> Value {
let prices = &provider.prices;
json!({
"$schema": "https://opencode.ai/config.json",
"autoupdate": false,
"share": "disabled",
"model": format!("{PROVIDER_ID}/{}", provider.model),
"enabled_providers": [PROVIDER_ID],
"default_agent": AGENT_ID,
"agent": {
"title": { "disable": true },
AGENT_ID: { "prompt": format!("{{file:./{SYSTEM_FILE}}}") },
},
"compaction": { "auto": false },
"provider": {
PROVIDER_ID: {
"npm": "@ai-sdk/openai-compatible",
"name": "review classifier provider",
"options": {
"baseURL": provider.base_url,
// opencode substitutes it from its environment.
"apiKey": format!("{{env:{PROVIDER_KEY_VAR}}}"),
},
"models": {
provider.model.as_str(): {
"name": provider.model,
"cost": {
"input": prices.input,
"output": prices.output,
"cache_read": prices.cache_read.unwrap_or(prices.input),
"cache_write": prices.cache_write.unwrap_or(prices.input),
},
},
},
},
},
})
}
/// Keeps the turn's last assistant text: the model's final message. An ACP
/// turn has no `result` event to take it from.
#[derive(Default)]
struct FinalText(Mutex<Option<String>>);
impl FinalText {
fn take(&self) -> Result<String, String> {
self.0
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.take()
.ok_or_else(|| "the ACP agent ended without an answer".to_owned())
}
}
impl Sink for FinalText {
fn on_event(&self, event: &Value) {
if event["type"] != "assistant" {
return;
}
let mut texts = event["message"]["content"]
.as_array()
.into_iter()
.flatten()
.filter(|block| block["type"] == "text")
.filter_map(|block| block["text"].as_str());
if let Some(text) = texts.next_back() {
*self
.0
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner) = Some(text.to_owned());
}
}
}
#[cfg(test)]
mod tests {
use std::os::unix::fs::PermissionsExt as _;
use swarm_queue_client::review_classifier::Prices;
use super::*;
use crate::prompt::Prompt;
use crate::run::Model as _;
/// A deadline no test reaches.
const LONG: Duration = Duration::from_mins(1);
/// A deadline the stalled agents run into.
const SHORT: Duration = Duration::from_millis(500);
/// An ACP agent in plain `sh`, logging to `<log>.*`. It writes its
/// environment and copies of its config and the system prompt file
/// beside it and appends its `$HOME` to `<log>.homes` at start, appends
/// each method it is sent to `<log>.methods`, each `session/new` request
/// to `<log>.sessions` and each prompt request to `<log>.prompts`, and
/// answers a prompt with a text chunk
/// holding a verdict, then `end_turn` with 1000 input and 200 output
/// tokens. A prompt holding `ASK-READ` first asks permission for a
/// `read` and appends the reply to `<log>.permission`; one holding
/// `STALL` is answered only once it is cancelled; one holding `MUTE`
/// gets the answer with no token counts.
const AGENT: &str = r#"#!/bin/sh
log="$1"
env > "$log.env"
cp "$OPENCODE_CONFIG" "$log.config"
cp "${OPENCODE_CONFIG%/*}/system.md" "$log.system"
printf '%s\n' "$HOME" >> "$log.homes"
stalled=
while IFS= read -r line; do
id=${line#*\"id\":}; id=${id%%[,\}]*}
method=$(printf '%s' "$line" | sed -n 's/.*"method":"\([^"]*\)".*/\1/p')
printf '%s\n' "$method" >> "$log.methods"
case "$method" in
initialize)
printf '{"jsonrpc":"2.0","id":%s,"result":{"protocolVersion":1,"agentCapabilities":{"mcpCapabilities":{"http":true}}}}\n' "$id" ;;
session/new)
printf '%s\n' "$line" >> "$log.sessions"
printf '{"jsonrpc":"2.0","id":%s,"result":{"sessionId":"s1"}}\n' "$id" ;;
session/cancel)
[ -n "$stalled" ] && printf '{"jsonrpc":"2.0","id":%s,"result":{"stopReason":"cancelled"}}\n' "$stalled" ;;
session/prompt)
printf '%s\n' "$line" >> "$log.prompts"
case "$line" in *STALL*) stalled=$id; continue ;; esac
case "$line" in *ASK-READ*)
printf '{"jsonrpc":"2.0","id":900,"method":"session/request_permission","params":{"sessionId":"s1","toolCall":{"toolCallId":"t1","kind":"read","title":"read"},"options":[{"optionId":"yes","kind":"allow_once","name":"Allow"},{"optionId":"no","kind":"reject_once","name":"Reject"}]}}\n'
read -r reply
printf '%s\n' "$reply" >> "$log.permission" ;;
esac
printf '{"jsonrpc":"2.0","method":"session/update","params":{"sessionId":"s1","update":{"sessionUpdate":"agent_message_chunk","content":{"type":"text","text":"{\\"verdict\\":\\"match\\",\\"certainty\\":90,\\"reason\\":\\"ok\\"}"}}}}\n'
case "$line" in
*MUTE*) printf '{"jsonrpc":"2.0","id":%s,"result":{"stopReason":"end_turn"}}\n' "$id" ;;
*) printf '{"jsonrpc":"2.0","id":%s,"result":{"stopReason":"end_turn","usage":{"inputTokens":1000,"outputTokens":200}}}\n' "$id" ;;
esac ;;
esac
done
"#;
#[derive(Default)]
struct Costs(Mutex<Vec<Cost>>);
impl CostSink for Arc<Costs> {
fn record(&self, cost: &Cost) {
self.0.lock().expect("unpoisoned").push(cost.clone());
}
}
/// An [`Acp`] whose trampoline is [`AGENT`], logging under `dir`.
fn acp(dir: &Path) -> (Acp<Arc<Costs>>, Arc<Costs>) {
let script = dir.join("agent.sh");
let log = dir.join("log");
std::fs::write(
&script,
AGENT.replacen("log=\"$1\"", &format!("log='{}'", log.display()), 1),
)
.expect("written");
std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).expect("chmod");
let costs = Arc::new(Costs::default());
let acp = Acp {
program: PathBuf::from("/nix/store/x-opencode/bin/opencode"),
trampoline: script,
cancel_wait: Duration::from_millis(500),
cost: Arc::clone(&costs),
cwd: dir.join("cwd"),
};
(acp, costs)
}
fn read(dir: &Path, suffix: &str) -> String {
std::fs::read_to_string(dir.join(format!("log.{suffix}"))).unwrap_or_default()
}
fn run() -> TryOut {
TryOut {
operator: "alice".to_owned(),
repo: "o/r".to_owned(),
pr: 7,
head_sha: "abc".to_owned(),
commits: Vec::new(),
diff: String::new(),
classifications: Vec::new(),
model: "haiku".to_owned(),
effort: None,
runtime: Runtime::Acp(Provider {
base_url: "https://llm.example/v1".to_owned(),
model: "qwen".to_owned(),
prices: Prices {
input: 3.0,
output: 15.0,
cache_read: None,
cache_write: None,
},
}),
}
}
#[tokio::test]
async fn the_answer_is_the_final_text_and_its_cost_is_labelled_with_the_pr() {
let dir = tempfile::tempdir().expect("a dir");
let (acp, costs) = acp(dir.path());
let answer = acp.ask(&Prompt::of("classify"), "p", &run(), LONG).await;
assert_eq!(
answer.as_deref(),
Ok(r#"{"verdict":"match","certainty":90,"reason":"ok"}"#)
);
let costs = costs.0.lock().expect("unpoisoned");
assert_eq!(costs.len(), 1);
let cost = &costs[0];
assert_eq!(
(cost.repo.as_str(), cost.pr, cost.model.as_str()),
("o/r", 7, "qwen")
);
assert!((cost.amount - 0.006).abs() < 1e-12, "{}", cost.amount);
assert_eq!(
(cost.usage.input_tokens, cost.usage.output_tokens),
(1000, 200)
);
}
#[tokio::test]
async fn opencode_gets_the_key_no_tools_and_the_runs_provider() {
let dir = tempfile::tempdir().expect("a dir");
let (acp, _) = acp(dir.path());
acp.ask(&Prompt::of("classify"), "p", &run(), LONG)
.await
.expect("an answer");
let env = read(dir.path(), "env");
for line in [
"ACP_PROVIDER_API_KEY=p",
r#"OPENCODE_PERMISSION={"*":"deny"}"#,
"OPENCODE_DISABLE_PROJECT_CONFIG=1",
"SWARM_REVIEW_CLASSIFIER_EXEC_OPENCODE=/nix/store/x-opencode/bin/opencode",
] {
assert!(env.lines().any(|l| l == line), "{line} not in:\n{env}");
}
let config: Value = serde_json::from_str(&read(dir.path(), "config")).expect("JSON");
let provider = &config["provider"][PROVIDER_ID];
assert_eq!(config["model"], "review-classifier/qwen");
assert_eq!(provider["options"]["baseURL"], "https://llm.example/v1");
assert_eq!(provider["options"]["apiKey"], "{env:ACP_PROVIDER_API_KEY}");
assert_eq!(provider["models"]["qwen"]["cost"]["output"], 15.0);
assert_eq!(config["agent"]["title"]["disable"], true);
// The run's directory is gone with the run.
let home = env
.lines()
.find_map(|l| l.strip_prefix("HOME="))
.expect("a HOME");
assert!(!Path::new(home).exists(), "{home}");
}
#[tokio::test]
async fn the_system_prompt_is_the_agents_and_the_prompt_is_the_pr_alone() {
let dir = tempfile::tempdir().expect("a dir");
let (acp, _) = acp(dir.path());
acp.ask(&Prompt::of("classify"), "p", &run(), LONG)
.await
.expect("an answer");
let config: Value = serde_json::from_str(&read(dir.path(), "config")).expect("JSON");
assert_eq!(config["default_agent"], AGENT_ID);
// Only a prompt: a `permission` or `tools` key here would be merged
// over `OPENCODE_PERMISSION`.
assert_eq!(
config["agent"][AGENT_ID],
json!({ "prompt": "{file:./system.md}" })
);
assert_eq!(read(dir.path(), "system"), crate::prompt::SYSTEM);
let prompts = read(dir.path(), "prompts");
assert!(prompts.contains("classify"), "{prompts}");
assert!(!prompts.contains("You check one pull request"), "{prompts}");
}
#[tokio::test]
async fn a_permission_request_is_refused() {
let dir = tempfile::tempdir().expect("a dir");
let (acp, _) = acp(dir.path());
acp.ask(&Prompt::of("ASK-READ"), "p", &run(), LONG)
.await
.expect("an answer");
let reply: Value =
serde_json::from_str(read(dir.path(), "permission").trim()).expect("a JSON reply");
assert_eq!(reply["result"]["outcome"]["optionId"], "no");
}
#[tokio::test]
async fn the_deadline_cancels_the_turn() {
let dir = tempfile::tempdir().expect("a dir");
let (acp, costs) = acp(dir.path());
let answer = acp.ask(&Prompt::of("STALL"), "p", &run(), SHORT).await;
assert_eq!(answer, Err("no answer within 0 seconds".to_owned()));
assert!(
read(dir.path(), "methods")
.lines()
.any(|m| m == "session/cancel")
);
assert!(costs.0.lock().expect("unpoisoned").is_empty());
}
#[tokio::test]
async fn an_agent_stuck_before_its_prompt_is_dropped_at_the_deadline() {
let dir = tempfile::tempdir().expect("a dir");
let (acp, _) = acp(dir.path());
let script = dir.path().join("agent.sh");
let unanswered = std::fs::read_to_string(&script).expect("written").replacen(
" initialize)",
" never)",
1,
);
std::fs::write(&script, unanswered).expect("written");
let answer = tokio::time::timeout(
Duration::from_secs(10),
acp.ask(&Prompt::of("one"), "p", &run(), SHORT),
)
.await
.expect("the deadline ends the ask");
assert_eq!(answer, Err("no answer within 0 seconds".to_owned()));
}
#[tokio::test]
async fn no_token_counts_record_no_cost() {
let dir = tempfile::tempdir().expect("a dir");
let (acp, costs) = acp(dir.path());
acp.ask(&Prompt::of("MUTE"), "p", &run(), LONG)
.await
.expect("an answer");
assert!(costs.0.lock().expect("unpoisoned").is_empty());
}
#[tokio::test]
async fn a_claude_run_is_refused() {
let dir = tempfile::tempdir().expect("a dir");
let (acp, _) = acp(dir.path());
let mut claude = run();
claude.runtime = Runtime::ApiKey;
assert!(
acp.ask(&Prompt::of("classify"), "p", &claude, LONG)
.await
.is_err()
);
assert_eq!(read(dir.path(), "methods"), "", "no agent started");
}
/// The cwd of each session the agent was asked for, in order.
fn cwds(dir: &Path) -> Vec<String> {
read(dir, "sessions")
.lines()
.map(|l| {
let request: Value = serde_json::from_str(l).expect("JSON");
request["params"]["cwd"].as_str().expect("a cwd").to_owned()
})
.collect()
}
#[tokio::test]
async fn every_run_gets_the_same_cwd_even_at_once() {
let dir = tempfile::tempdir().expect("a dir");
let (acp, _) = acp(dir.path());
let (run_one, run_two) = (run(), run());
let (prompt_one, prompt_two) = (Prompt::of("one"), Prompt::of("two"));
let (one, two) = tokio::join!(
acp.ask(&prompt_one, "p", &run_one, LONG),
acp.ask(&prompt_two, "p", &run_two, LONG)
);
one.expect("an answer");
two.expect("an answer");
acp.ask(&Prompt::of("three"), "p", &run(), LONG)
.await
.expect("an answer");
let cwd = dir.path().join("cwd").display().to_string();
assert_eq!(cwds(dir.path()), [cwd.clone(), cwd.clone(), cwd]);
}
#[tokio::test]
async fn each_run_has_a_home_of_its_own_removed_after_it() {
let dir = tempfile::tempdir().expect("a dir");
let (acp, _) = acp(dir.path());
acp.ask(&Prompt::of("one"), "p", &run(), LONG)
.await
.expect("an answer");
acp.ask(&Prompt::of("two"), "p", &run(), LONG)
.await
.expect("an answer");
let homes = read(dir.path(), "homes");
let homes: Vec<&str> = homes.lines().collect();
assert_eq!(homes.len(), 2, "{homes:?}");
assert_ne!(homes[0], homes[1]);
for home in homes {
assert!(!Path::new(home).exists(), "{home}");
}
}
}