review classifier: under ACP the prompt is a real system message
The ACP runtime handed the classifier's system prompt to hive-runtime's
system_prompt_file, which prepends it to the first user message, so the
instructions and the PR text sat in the same user turn.
The rendered opencode.json now defines a review-classifier agent whose
prompt is {file:./system.md} and selects it with default_agent. opencode
1.15.10 sends an agent's prompt as the system-role message (in place of
its per-model default prompt), followed only by its environment block;
the first user message is the PR material alone. The API key runtime
keeps --system-prompt.
The agent carries no permission or tools key, and a test pins its exact
shape: opencode merges an agent's own permission after
OPENCODE_PERMISSION, so a permission there would offer tools again
(checked on real opencode: {"*":"allow"} on the agent offered all ten
tools despite OPENCODE_PERMISSION={"*":"deny"}).
Follow-up to #5214.
This commit is contained in:
parent
7d24feaf25
commit
6d42abb285
2 changed files with 50 additions and 7 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<S: CostSink> Acp<S> {
|
|||
run: &TryOut,
|
||||
provider: &Provider,
|
||||
) -> Result<String, String> {
|
||||
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<S: CostSink> Acp<S> {
|
|||
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 `<log>.*`. It writes its
|
||||
/// environment and a copy of its config at start, appends each method
|
||||
/// it is sent to `<log>.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
|
||||
/// `<log>.methods` 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
|
||||
|
|
@ -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");
|
||||
|
|
|
|||
Loading…
Reference in a new issue