diff --git a/docs/swarm/review-classifier.md b/docs/swarm/review-classifier.md index 51171e59..120f86ed 100644 --- a/docs/swarm/review-classifier.md +++ b/docs/swarm/review-classifier.md @@ -133,7 +133,12 @@ deny every tool (`OPENCODE_PERMISSION={"*":"deny"}`), which leaves the model with no tool offered at all, and refuses any permission request on top. The config it writes for opencode names one provider and model, from the settings, and turns off opencode's title call, which would send the prompt a second -time. +time. It also defines the agent the session runs as, whose prompt is the +classifier's system prompt, so opencode sends it as the system message, with +only its environment block (model, directory, platform, date) after it. The +first user message holds the PR alone. That agent has no `permission` of its +own: opencode merges an agent's permission over `OPENCODE_PERMISSION`, so one +there would offer the model tools again. The service is `swarm-review-classifier`, and its container is the one [`services.md`](services.md#review-classifier) describes. It holds no forge diff --git a/swarm-review-classifier/src/acp.rs b/swarm-review-classifier/src/acp.rs index 781f3068..5bf98a25 100644 --- a/swarm-review-classifier/src/acp.rs +++ b/swarm-review-classifier/src/acp.rs @@ -10,7 +10,12 @@ //! `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. +//! 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}; @@ -35,6 +40,13 @@ 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); @@ -81,7 +93,7 @@ impl Acp { run: &TryOut, provider: &Provider, ) -> Result { - let system = dir.join("system.md"); + let system = dir.join(SYSTEM_FILE); let config_file = dir.join("opencode.json"); tokio::fs::create_dir_all(dir) .await @@ -101,7 +113,6 @@ impl Acp { let config = Config { effort: run.effort.map(|e| e.as_str().to_owned()), cwd: Some(dir.to_owned()), - system_prompt_file: Some(system), idle_timeout: Some(IDLE_TIMEOUT), ..Config::default() }; @@ -182,7 +193,11 @@ fn opencode_config(provider: &Provider) -> Value { "share": "disabled", "model": format!("{PROVIDER_ID}/{}", provider.model), "enabled_providers": [PROVIDER_ID], - "agent": { "title": { "disable": true } }, + "default_agent": AGENT_ID, + "agent": { + "title": { "disable": true }, + AGENT_ID: { "prompt": format!("{{file:./{SYSTEM_FILE}}}") }, + }, "compaction": { "auto": false }, "provider": { PROVIDER_ID: { @@ -254,8 +269,9 @@ mod tests { use crate::run::Model as _; /// An ACP agent in plain `sh`, logging to `.*`. It writes its - /// environment and a copy of its config at start, appends each method - /// it is sent to `.methods`, and answers a prompt with a text chunk + /// environment and copies of its config and the system prompt file + /// beside it at start, appends each method it is sent to + /// `.methods` and each prompt request to `.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 `.permission`; one holding @@ -265,6 +281,7 @@ mod tests { log="$1" env > "$log.env" cp "$OPENCODE_CONFIG" "$log.config" +cp "${OPENCODE_CONFIG%/*}/system.md" "$log.system" stalled= while IFS= read -r line; do id=${line#*\"id\":}; id=${id%%[,\}]*} @@ -278,6 +295,7 @@ while IFS= read -r line; do 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' @@ -401,6 +419,26 @@ done 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(), Duration::from_mins(1)); + + acp.ask("classify", "p", &run()).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");