From 2fb6b39f9f3f7ac9de8393fb5b8da419a3000e9e Mon Sep 17 00:00:00 2001 From: atlas Date: Wed, 14 Oct 2026 14:06:55 +0200 Subject: [PATCH] review classifier: every ACP run gets one shared, empty cwd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit opencode writes the session's cwd into the system message's environment block ("Working directory: …", "Workspace root folder: …"), ahead of the PR's user message. Each ACP classification ran with its per-call scratch directory, /tmp/review-classifier--, as cwd, so every call sent a different system message and the provider's prompt cache could not match past it. The session's cwd is now one fixed directory, /review-classifier-acp-cwd, shared by every run. HOME (the config, system.md and opencode's own state) stays a directory per run, removed after it, as before. With no tool and no project config nothing writes into the cwd: a live opencode 1.15.10 run through initialize, session/new and a prompt left it empty, with its state all under HOME. So concurrent runs can share it, and no state carries from one run to the next. Refs #5245 --- docs/swarm/review-classifier.md | 6 ++- swarm-review-classifier/src/acp.rs | 64 ++++++++++++++++++++++++++--- swarm-review-classifier/src/main.rs | 1 + swarm-review-classifier/src/run.rs | 10 ++--- 4 files changed, 70 insertions(+), 11 deletions(-) diff --git a/docs/swarm/review-classifier.md b/docs/swarm/review-classifier.md index a8ebec4d..79e910ce 100644 --- a/docs/swarm/review-classifier.md +++ b/docs/swarm/review-classifier.md @@ -124,7 +124,11 @@ resume, no tools (`--tools ""`), no MCP server (`--strict-mcp-config` and no config) and an empty directory of its own. On the ACP runtime, each classification is one prompt on a new `opencode acp` -session in an empty directory of its own. The service drives it through +session. Its config and opencode's state live in a directory of its own, +removed after it. Its working directory is one empty directory that every +classification shares: opencode writes that path into the system message, so +one path keeps the system message the same from one classification to the +next, and the provider's prompt cache can hold. The service drives it through [`hive-runtime`](../../hive-runtime/README.md)'s `AcpRuntime`, as the subagent daemon does a one-run subagent, and stops the agent when the classification ends. The agent has no tool. opencode runs some tools, `read` among them, diff --git a/swarm-review-classifier/src/acp.rs b/swarm-review-classifier/src/acp.rs index 7c5d7475..905b8659 100644 --- a/swarm-review-classifier/src/acp.rs +++ b/swarm-review-classifier/src/acp.rs @@ -1,6 +1,8 @@ //! One classification on the ACP runtime: `opencode acp`, driven through -//! [`hive_runtime::AcpRuntime`] for one prompt on a new session, in a -//! directory made for it and removed after it. +//! [`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: @@ -68,6 +70,11 @@ pub struct Acp { /// [`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 crate::run::Model for Acp { @@ -97,6 +104,7 @@ impl Acp { 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}"))?; @@ -112,7 +120,7 @@ impl Acp { .ok_or("the ACP runtime offers no cancel handle")?; let config = Config { effort: run.effort.map(|e| e.as_str().to_owned()), - cwd: Some(dir.to_owned()), + cwd: Some(self.cwd.clone()), idle_timeout: Some(IDLE_TIMEOUT), ..Config::default() }; @@ -271,8 +279,10 @@ mod tests { /// An ACP agent in plain `sh`, logging to `.*`. It writes its /// 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 + /// beside it and appends its `$HOME` to `.homes` at start, appends + /// each method it is sent to `.methods`, each `session/new` request + /// to `.sessions` 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 @@ -283,6 +293,7 @@ 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%%[,\}]*} @@ -292,6 +303,7 @@ while IFS= read -r line; do 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" ;; @@ -338,6 +350,7 @@ done deadline, cancel_wait: Duration::from_millis(500), cost: Arc::clone(&costs), + cwd: dir.join("cwd"), }; (acp, costs) } @@ -507,4 +520,45 @@ done assert!(acp.ask("classify", "p", &claude).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 { + 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(), Duration::from_mins(1)); + + let (run_one, run_two) = (run(), run()); + let (one, two) = tokio::join!(acp.ask("one", "p", &run_one), acp.ask("two", "p", &run_two)); + one.expect("an answer"); + two.expect("an answer"); + acp.ask("three", "p", &run()).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(), Duration::from_mins(1)); + + acp.ask("one", "p", &run()).await.expect("an answer"); + acp.ask("two", "p", &run()).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}"); + } + } } diff --git a/swarm-review-classifier/src/main.rs b/swarm-review-classifier/src/main.rs index a07ce623..5e07e176 100644 --- a/swarm-review-classifier/src/main.rs +++ b/swarm-review-classifier/src/main.rs @@ -70,6 +70,7 @@ async fn serve() -> Result<()> { deadline: swarm_queue_client::review_classifier::CLASSIFICATION_DEADLINE, cancel_wait: acp::CANCEL_WAIT, cost: cost::Otel, + cwd: std::env::temp_dir().join("review-classifier-acp-cwd"), }, }; let secret = client_secret_with_retry(&client_id).await?; diff --git a/swarm-review-classifier/src/run.rs b/swarm-review-classifier/src/run.rs index 10659cd8..2be5a726 100644 --- a/swarm-review-classifier/src/run.rs +++ b/swarm-review-classifier/src/run.rs @@ -3,11 +3,11 @@ //! //! 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. +//! prompt on a new ACP session ([`crate::acp`]). Either way its files live +//! 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};