diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d4dbbf..a9ec9c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added -- Child-agent handoff schema and verify gate (#383, part 1): `tole_core::handoff` defines the typed handoff document (role `explore` / `implement` / `verify` / `fix`, artifact references by path and rev, verification evidence), strict validation of the model-authored input (workspace-relative paths, option-safe refs, bounded fields), the close gate (an implement/fix child without passing verification is `Unverified` / `VerifyFailed`, never closed silently) and the prompt for the follow-up `fix` child. Design and anti-goals (no DAG engine) in `docs/child-agents-handoff.md`; the child tool and the `agent_poll` gate follow. +- Child-agent handoff schema and verify gate (#383): `tole_core::handoff` defines the typed handoff document (role `explore` / `implement` / `verify` / `fix`, artifact references by path and rev, verification evidence), strict validation of the model-authored input (workspace-relative paths, option-safe refs, bounded fields), the close gate (an implement/fix child without passing verification is `Unverified` / `VerifyFailed`, never closed silently) and the prompt for the follow-up `fix` child. `agent_start` takes `role` and `consumes` (artifact references the child reads itself), a role child gets a `handoff_submit` tool, and `agent_poll` reports the gate and a ready-made `fix_prompt` (the parent decides whether to start the fix child; nothing is spawned automatically). Design and anti-goals (no DAG engine) in `docs/child-agents-handoff.md`. - Sibling change notices, core (#380): `sibling_watch` tracks which files each child agent reads and writes in one shared append-only log (owner-only, bounded) and tells a child, once per stale path, when a sibling changed a file it had read or written; the `Tracked` tool decorator records `read_file` / `write_file` / `edit_file` / `delete_file` on success. Interrupts gain a `source`: a notice is delivered at the next safe point as an attributed entry (`source: notice`) and never withdraws output or supersedes a final answer. Wiring into `tole run` children follows separately. - Sibling change notices (#380): `sibling_watch` tracks which files each child agent reads and writes in one shared append-only log (owner-only, bounded) and tells a child, once per stale path, when a sibling changed a file it had read or written; the `Tracked` tool decorator records `read_file` / `write_file` / `edit_file` / `delete_file` on success. Interrupts gain a `source`: a notice is delivered at the next safe point as an attributed entry (`source: notice`) and never withdraws output or supersedes a final answer. Children spawned by `agent_start` get `TOLE_AGENT_ID` and a shared `TOLE_AGENT_TOUCH_LOG`; in a child, `tole run` wraps its file tools and runs the watcher, so a sibling's change reaches the model at its next safe point. - `tole doctor` coverage ledger (#378): every run is recorded per provider + model (level reached, first blocker, fix command, timestamp) in an owner-only `doctor-ledger.json` (atomic write, a corrupt file is an error, never replaced); `--summary` prints `READY n-of-m` + the first blocker and its fix, `--no-ledger` skips recording. diff --git a/README.md b/README.md index f3fe6e1..94e0162 100644 --- a/README.md +++ b/README.md @@ -252,7 +252,7 @@ hard error. | `verify_package` | RO | crates.io / npm registry check before any install — hallucinated names get NOT FOUND + candidates, edit-distance-1 candidates get a typo-squat warning | | `load_skill` | RO | loads a discovered SKILL.md on demand (`--skill` pins one upfront; `--no-skills` disables) | | `gitea` | Write | Gitea counterpart of `gh` over the instance REST API — registers when `origin` is a Gitea remote AND `TOLE_GITEA_TOKEN`/`GITEA_TOKEN` is set; same six ops | -| `agent_start`, `agent_poll` | Write / Write | depth-1 child agents: spawn durable child sessions (structurally no grandchildren), results via per-child uteke mailboxes (ephemeral by default); `agent_poll` is Write because a settled poll consumes (cleans) the mailbox, and `--trust internal` allowlists it by name; `--agents-worktree` gives each child its own git worktree; siblings are told (#380) when another child changes a file they read or wrote, at their next safe point | +| `agent_start`, `agent_poll` | Write / Write | depth-1 child agents: spawn durable child sessions (structurally no grandchildren), results via per-child uteke mailboxes (ephemeral by default); `agent_poll` is Write because a settled poll consumes (cleans) the mailbox, and `--trust internal` allowlists it by name; `--agents-worktree` gives each child its own git worktree; siblings are told (#380) when another child changes a file they read or wrote, at their next safe point; `role` (`explore`/`implement`/`verify`/`fix`) and `consumes` (artifacts by reference) give children a typed handoff, and an implement/fix child without passing verification is not closed (#383) | | `systemone_decide` | RO | typed decisions (choice/score/noul + confidence) from a System One backend — active when `SYSTEMONE_API_KEY` is set; `SYSTEMONE_BASE_URL` picks the backend (hosted Jev default, self-hosted compatible) | | `job_start`, `job_poll` | Write / RO | detached long-running jobs with log tailing | | `cora_search` | RO | hybrid codebase search via `cora brain`; native fallback — skipped when the cora MCP surface is attached | diff --git a/crates/tole-cli/src/handoff.rs b/crates/tole-cli/src/handoff.rs new file mode 100644 index 0000000..0947b12 --- /dev/null +++ b/crates/tole-cli/src/handoff.rs @@ -0,0 +1,52 @@ +//! Host wiring for the child-agent handoff (#383): a child started with a +//! role finds the role and the handoff path in its environment and gets the +//! `handoff_submit` tool. Outside such a child this is a no-op. + +use std::path::PathBuf; +use tole_core::handoff::{HandoffSubmitTool, Role}; + +pub const ENV_ROLE: &str = "TOLE_AGENT_ROLE"; +pub const ENV_HANDOFF: &str = "TOLE_AGENT_HANDOFF"; + +/// The `handoff_submit` tool for this process, when it is a role child. +/// Both variables must be present and the role must parse; anything else +/// (including a role set without a path) registers nothing. +pub fn tool_from_env(env: &dyn Fn(&str) -> Option) -> Option { + let role = Role::parse(env(ENV_ROLE)?.trim())?; + let path = env(ENV_HANDOFF)?.trim().to_string(); + if path.is_empty() { + return None; + } + Some(HandoffSubmitTool::new(PathBuf::from(path), role)) +} + +#[cfg(test)] +mod tests { + use super::*; + use tole_core::tool::Tool; + + fn env<'a>(pairs: &'a [(&'a str, &'a str)]) -> impl Fn(&str) -> Option + 'a { + move |k| { + pairs + .iter() + .find(|(n, _)| *n == k) + .map(|(_, v)| v.to_string()) + } + } + + #[test] + fn a_role_child_gets_the_tool_and_nothing_else_does() { + let t = tool_from_env(&env(&[(ENV_ROLE, "fix"), (ENV_HANDOFF, "/x/handoff.json")])); + assert_eq!( + t.map(|t| t.name().to_string()), + Some("handoff_submit".into()) + ); + assert!(tool_from_env(&env(&[])).is_none()); + assert!( + tool_from_env(&env(&[(ENV_ROLE, "fix")])).is_none(), + "no path" + ); + assert!(tool_from_env(&env(&[(ENV_ROLE, "boss"), (ENV_HANDOFF, "/x")])).is_none()); + assert!(tool_from_env(&env(&[(ENV_ROLE, "fix"), (ENV_HANDOFF, " ")])).is_none()); + } +} diff --git a/crates/tole-cli/src/lib.rs b/crates/tole-cli/src/lib.rs index 8e14581..8ec164d 100644 --- a/crates/tole-cli/src/lib.rs +++ b/crates/tole-cli/src/lib.rs @@ -8,6 +8,7 @@ pub mod config; pub mod config_apply; pub mod config_trust; pub mod doctor; +pub mod handoff; pub mod session_host; pub mod session_tools; pub mod sibling; diff --git a/crates/tole-cli/src/main.rs b/crates/tole-cli/src/main.rs index e9d6142..2b08461 100644 --- a/crates/tole-cli/src/main.rs +++ b/crates/tole-cli/src/main.rs @@ -1473,6 +1473,11 @@ fn build_registry( let file_root = resolve_workspace_root(workspace)?; // Child agents (#380): present only when spawned by `agent_start`. let tracking = tole_cli::sibling::tracking_from_env(&tole_cli::config_apply::real_env); + // Role children (#383) get the handoff tool; everyone else does not. + if let Some(tool) = tole_cli::handoff::tool_from_env(&tole_cli::config_apply::real_env) { + reg.register(Box::new(tool)) + .map_err(|e| anyhow::anyhow!("registering handoff_submit: {e}"))?; + } // Uteke first-class (B4): recall (read) + document (write), behind // startup probing — a missing uteke binary degrades to a warning, // not phantom tools. @@ -1644,6 +1649,11 @@ fn build_server_registry( let file_root = resolve_workspace_root(workspace)?; // Child agents (#380): present only when spawned by `agent_start`. let tracking = tole_cli::sibling::tracking_from_env(&tole_cli::config_apply::real_env); + // Role children (#383) get the handoff tool; everyone else does not. + if let Some(tool) = tole_cli::handoff::tool_from_env(&tole_cli::config_apply::real_env) { + reg.register(Box::new(tool)) + .map_err(|e| anyhow::anyhow!("registering handoff_submit: {e}"))?; + } let count = |reg: &ToolRegistry| reg.specs().len(); #[cfg(feature = "shell-tools")] diff --git a/crates/tole-core/src/agents.rs b/crates/tole-core/src/agents.rs index f833aad..a6d77a7 100644 --- a/crates/tole-core/src/agents.rs +++ b/crates/tole-core/src/agents.rs @@ -249,6 +249,30 @@ pub fn valid_agent_id(id: &str) -> bool { crate::sibling_watch::valid_agent_id(id) } +/// File name of a child's handoff inside its agent dir. +pub const HANDOFF_FILE: &str = "handoff.json"; + +/// The child's mission: what was asked, plus (when a role is set) its brief +/// and the artifacts to read by reference. Unchanged when neither is given. +fn compose_prompt( + prompt: &str, + role: Option, + consumes: &[crate::handoff::ArtifactRef], +) -> String { + let mut out = String::new(); + if let Some(r) = role { + out.push_str(&crate::handoff::role_brief(r)); + out.push_str("\n\n"); + } + out.push_str(prompt); + let inputs = crate::handoff::consume_preamble(consumes); + if !inputs.is_empty() { + out.push_str("\n\n"); + out.push_str(&inputs); + } + out +} + fn agents_root(root: &std::path::Path) -> PathBuf { root.join(AGENTS_DIR) } @@ -290,6 +314,9 @@ pub struct AgentMeta { /// poll re-recalled the mailbox after cleanup — flapping summaries. #[serde(default)] pub mailbox_consumed: bool, + /// Handoff role (#383), when the child was started with one. + #[serde(default)] + pub role: Option, } fn read_meta(dir: &std::path::Path) -> Result { @@ -512,7 +539,21 @@ impl Tool for AgentStartTool { "type": "array", "items": { "type": "string" }, "description": "The child's Write-tool allow patterns (glob). Keep minimal — the human approves this policy here." }, - "keep_mailbox": { "type": "boolean", "description": "Keep the uteke mailbox after the result is consumed (default false — ephemeral). The child's session JSONL is always kept." } + "keep_mailbox": { "type": "boolean", "description": "Keep the uteke mailbox after the result is consumed (default false — ephemeral). The child's session JSONL is always kept." }, + "role": { "type": "string", "enum": ["explore", "implement", "verify", "fix"], "description": "Handoff role (#383). The child submits a typed handoff when it finishes; an implement/fix child that does not pass verification is not closed — agent_poll reports it with a ready-made prompt for a fix child." }, + "consumes": { + "type": "array", + "description": "Artifacts from earlier agents the child should read itself, by reference (workspace-relative path, optional rev) — not relayed through you", + "items": { + "type": "object", + "properties": { + "path": { "type": "string" }, + "rev": { "type": "string" }, + "note": { "type": "string" } + }, + "required": ["path"] + } + } }, "required": ["prompt"] })) @@ -536,6 +577,32 @@ impl Tool for AgentStartTool { .get("keep_mailbox") .and_then(Value::as_bool) .unwrap_or(false); + // Handoff role + the artifacts to consume (#383). Both optional; + // without them the child's prompt is exactly what was asked for. + let role = match input.get("role") { + None | Some(Value::Null) => None, + Some(v) => Some( + v.as_str() + .and_then(crate::handoff::Role::parse) + .ok_or("agent_start: 'role' must be explore, implement, verify or fix")?, + ), + }; + let consumes: Vec = match input.get("consumes") { + None | Some(Value::Null) => Vec::new(), + Some(v) => serde_json::from_value(v.clone()) + .map_err(|e| format!("agent_start: bad 'consumes': {e}"))?, + }; + if consumes.len() > crate::handoff::MAX_ARTIFACTS { + return Err(format!( + "agent_start: at most {} artifacts in 'consumes'", + crate::handoff::MAX_ARTIFACTS + )); + } + for a in &consumes { + a.validate() + .map_err(|e| format!("agent_start: bad 'consumes' entry: {e}"))?; + } + let prompt = compose_prompt(&prompt, role, &consumes); let requested: Vec = input .get("allow") .and_then(Value::as_array) @@ -575,6 +642,13 @@ impl Tool for AgentStartTool { } requested }; + // A role child gets exactly one extra, harness-owned tool name: + // `handoff_submit` can only write one validated, bounded file at + // the path chosen here, so it cannot widen what the child may do. + let mut allows = allows; + if role.is_some() && !allows.iter().any(|a| a == "handoff_submit") { + allows.push("handoff_submit".to_string()); + } // Issue #234: hold the flock across count + register — the // count-then-spawn window is the TOCTOU the scan flagged. The // guard releases when `_lock` drops (spawn errors included). @@ -642,6 +716,7 @@ impl Tool for AgentStartTool { worktree: worktree.clone(), branch: branch.clone(), mailbox_consumed: false, + role: role.map(|r| r.as_str().to_string()), }; write_meta(&dir, &meta)?; @@ -677,6 +752,10 @@ impl Tool for AgentStartTool { .arg(&sessions_dir) .arg("--workspace") .arg(&child_cwd); + if let Some(r) = role { + cmd.env("TOLE_AGENT_ROLE", r.as_str()) + .env("TOLE_AGENT_HANDOFF", dir.join(HANDOFF_FILE)); + } for pat in &allows { cmd.arg("--allow").arg(pat); } @@ -720,6 +799,7 @@ impl Tool for AgentStartTool { "mailbox": mailbox_ns, "worktree": worktree, "branch": branch, + "role": role.map(|r| r.as_str()), "note": "running detached; poll with agent_poll (the result summary arrives via the mailbox)", })) } @@ -850,11 +930,63 @@ impl Tool for AgentPollTool { // cleaned mailbox. The in-process Mutex is held across the // ENTIRE consume section (issue #260, rescan #155: dropping it // after the read let two concurrent polls both recall+clean). + attach_handoff(&dir, &meta, &mut out); self.consume_mailbox(&dir, &mut meta, &mut out, crate::memory::recall); Ok(out) } } +/// The #383 verify gate for a settled child started with a role: attach its +/// handoff and whether the work may be treated as closed. An implement/fix +/// child that submitted nothing, or whose verification is missing or failed, +/// is reported as such with the prompt for the fix child that should follow +/// (the parent model decides whether to start it; nothing is spawned here). +fn attach_handoff(dir: &std::path::Path, meta: &AgentMeta, out: &mut Value) { + use crate::handoff::{Gate, Handoff, Role}; + let Some(role) = meta.role.as_deref().and_then(Role::parse) else { + return; + }; + out["role"] = json!(role.as_str()); + let raw = match std::fs::read_to_string(dir.join(HANDOFF_FILE)) { + Ok(raw) => raw, + Err(_) => { + // Nothing submitted. + if role.requires_verification() { + out["gate"] = json!("unverified"); + out["gate_reason"] = json!("the child closed without submitting a handoff"); + out["fix_prompt"] = json!(crate::handoff::missing_handoff_prompt(role)); + } else { + out["gate"] = json!("no_handoff"); + } + return; + } + }; + match Handoff::from_json(&raw) { + Err(e) => { + // A malformed document is as good as none for a gated role. + out["handoff_error"] = json!(e); + if role.requires_verification() { + out["gate"] = json!("unverified"); + out["gate_reason"] = json!("the submitted handoff is invalid"); + out["fix_prompt"] = json!(crate::handoff::missing_handoff_prompt(role)); + } else { + out["gate"] = json!("no_handoff"); + } + } + Ok(h) => { + out["gate"] = json!(match h.gate() { + Gate::Closed => "closed", + Gate::Unverified => "unverified", + Gate::VerifyFailed { .. } => "verify_failed", + }); + if let Some(p) = h.fix_prompt() { + out["fix_prompt"] = json!(p); + } + out["handoff"] = serde_json::to_value(&h).unwrap_or(Value::Null); + } + } +} + impl AgentPollTool { /// Consume-once mailbox handling for a settled child. The mailbox is /// marked consumed and cleaned ONLY when the recall succeeded (hits or @@ -1247,6 +1379,225 @@ mod tests { let _ = std::fs::remove_dir_all(&d); } + // ---- #383 handoff roles ---- + + /// Start a child through a script that dumps its argv and the handoff + /// env, and return (argv lines, env lines). + #[cfg(unix)] + fn start_and_dump( + d: &std::path::Path, + input: Value, + allows: Vec, + ) -> (Value, Vec, Vec) { + let p = d.join("dump-all"); + let argv = d.join("argv.txt"); + let envf = d.join("env.txt"); + let script = format!( + "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"{}\"\nprintf '%s\\n%s\\n' \"$TOLE_AGENT_ROLE\" \"$TOLE_AGENT_HANDOFF\" > \"{}\"\nexit 0\n", + argv.display(), + envf.display() + ); + std::fs::write(&p, script).unwrap(); + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions(&p, std::fs::Permissions::from_mode(0o755)); + let out = AgentStartTool::new(&p, d.to_path_buf()) + .with_parent_allows(allows) + .execute(input) + .unwrap(); + let mut got = None; + for _ in 0..50 { + std::thread::sleep(std::time::Duration::from_millis(100)); + if let (Ok(a), Ok(e)) = ( + std::fs::read_to_string(&argv), + std::fs::read_to_string(&envf), + ) { + if e.lines().count() >= 2 { + got = Some((a, e)); + break; + } + } + } + let (a, e) = got.expect("child never dumped"); + ( + out, + a.lines().map(str::to_string).collect(), + e.lines().map(str::to_string).collect(), + ) + } + + #[cfg(unix)] + #[test] + fn a_role_child_gets_its_brief_the_artifacts_to_read_the_env_and_the_handoff_tool() { + let d = tmp("role"); + let (out, argv, env) = start_and_dump( + &d, + json!({ + "prompt": "make the parser faster", + "role": "implement", + "consumes": [{"path": "notes/plan.md", "rev": "abc1234", "note": "the plan"}] + }), + vec!["write_*".into()], + ); + let id = out["agent"].as_str().unwrap(); + assert_eq!(out["role"], "implement"); + let prompt = argv.last().unwrap(); + // The whole multi-line prompt arrives as one argv element after `--`. + let joined = argv.join("\n"); + assert!(joined.contains("You are an `implement` agent"), "{joined}"); + assert!(joined.contains("make the parser faster")); + assert!( + joined.contains("notes/plan.md @ abc1234 (the plan)"), + "{joined}" + ); + assert!(joined.contains("read them yourself")); + let _ = prompt; + // The harness-owned tool is allowlisted next to the parent's patterns. + let allows: Vec<&String> = argv + .iter() + .enumerate() + .filter(|(i, a)| *i > 0 && argv[*i - 1] == "--allow" && !a.is_empty()) + .map(|(_, a)| a) + .collect(); + assert!( + allows.iter().any(|a| *a == "write_*") && allows.iter().any(|a| *a == "handoff_submit"), + "{argv:?}" + ); + assert_eq!(env[0], "implement"); + assert_eq!( + std::path::Path::new(&env[1]), + agents_root(&d).join(id).join(HANDOFF_FILE) + ); + let meta = read_meta(&agents_root(&d).join(id)).unwrap(); + assert_eq!(meta.role.as_deref(), Some("implement")); + let _ = std::fs::remove_dir_all(&d); + } + + #[cfg(unix)] + #[test] + fn without_a_role_nothing_changes() { + let d = tmp("norole"); + let (out, argv, env) = start_and_dump(&d, json!({"prompt": "just do it"}), vec![]); + assert_eq!(out["role"], Value::Null); + assert_eq!(argv.last().map(String::as_str), Some("just do it")); + assert!(!argv.iter().any(|a| a == "handoff_submit")); + assert_eq!( + env, + vec!["".to_string(), "".to_string()], + "no handoff env: {env:?}" + ); + let _ = std::fs::remove_dir_all(&d); + } + + #[test] + fn bad_roles_and_consumes_are_refused_before_anything_is_spawned() { + let d = tmp("badrole"); + let t = AgentStartTool::new(fake_child(&d), d.clone()); + for bad in [ + json!({"prompt": "x", "role": "boss"}), + json!({"prompt": "x", "role": 7}), + json!({"prompt": "x", "consumes": [{"path": "../etc/passwd"}]}), + json!({"prompt": "x", "consumes": [{"path": "a", "rev": "--upload-pack=x"}]}), + json!({"prompt": "x", "consumes": [{"nope": 1}]}), + json!({"prompt": "x", "consumes": "a.txt"}), + ] { + assert!(t.execute(bad.clone()).is_err(), "{bad}"); + } + let many: Vec = (0..=crate::handoff::MAX_ARTIFACTS) + .map(|i| json!({"path": format!("f{i}")})) + .collect(); + assert!(t.execute(json!({"prompt": "x", "consumes": many})).is_err()); + assert!( + !agents_root(&d).exists() || std::fs::read_dir(agents_root(&d)).unwrap().count() == 0 + ); + let _ = std::fs::remove_dir_all(&d); + } + + /// A settled agent dir built by hand: dead pid, role, optional handoff. + #[cfg(unix)] + fn settled(d: &std::path::Path, role: Option<&str>, handoff: Option<&str>) -> Value { + let id = format!("a-{}-gate", role.unwrap_or("none")); + let dir = agents_root(d).join(&id); + std::fs::create_dir_all(&dir).unwrap(); + let mut c = Command::new("sh").args(["-c", "exit 0"]).spawn().unwrap(); + let pid = c.id(); + c.wait().unwrap(); + std::fs::write(dir.join("pid"), pid.to_string()).unwrap(); + write_meta( + &dir, + &AgentMeta { + prompt: "p".into(), + mailbox_ns: "agent-x".into(), + keep_mailbox: true, + spawned_at_ms: 0, + worktree: None, + branch: None, + mailbox_consumed: true, + role: role.map(str::to_string), + }, + ) + .unwrap(); + if let Some(h) = handoff { + std::fs::write(dir.join(HANDOFF_FILE), h).unwrap(); + } + AgentPollTool::new(d.to_path_buf()) + .execute(json!({"agent": id})) + .unwrap() + } + + #[cfg(unix)] + #[test] + fn the_poll_gate_blocks_an_unverified_close_and_offers_a_fix_prompt() { + let d = tmp("gate"); + let doc = |verify: &str| { + format!( + r#"{{"version":1,"role":"implement","summary":"built","artifacts":[{{"path":"out.txt","rev":"abc1234"}}]{verify}}}"# + ) + }; + // Nothing submitted by a gated role: blocked. + let o = settled(&d, Some("implement"), None); + assert_eq!(o["gate"], "unverified"); + assert!(o["fix_prompt"] + .as_str() + .unwrap() + .contains("without submitting a handoff")); + // Submitted without verification. + let o = settled(&d, Some("fix"), Some(&doc("").replace("implement", "fix"))); + assert_eq!(o["gate"], "unverified"); + assert!(o["fix_prompt"] + .as_str() + .unwrap() + .contains("reported no verification")); + // Verification failed: the prompt names the failure and the artifact. + let failing = + doc(r#","verify":{"command":"cargo test","exit_code":101,"summary":"1 failed"}"#); + let o = settled(&d, Some("implement"), Some(&failing)); + assert_eq!(o["gate"], "verify_failed"); + let fp = o["fix_prompt"].as_str().unwrap(); + assert!( + fp.contains("cargo test") + && fp.contains("exit 101") + && fp.contains("out.txt @ abc1234"), + "{fp}" + ); + assert_eq!(o["handoff"]["artifacts"][0]["path"], "out.txt"); + // Verified: closed, no fix prompt. + let ok = doc(r#","verify":{"command":"cargo test","exit_code":0,"summary":"ok"}"#); + let o = settled(&d, Some("implement"), Some(&ok)); + assert_eq!(o["gate"], "closed"); + assert!(o.get("fix_prompt").is_none()); + // A malformed document is as good as none for a gated role. + let o = settled(&d, Some("implement"), Some("{not json")); + assert_eq!(o["gate"], "unverified"); + assert!(o["handoff_error"] + .as_str() + .unwrap() + .contains("invalid handoff")); + // Ungated roles and role-less agents are not blocked. + assert_eq!(settled(&d, Some("explore"), None)["gate"], "no_handoff"); + assert!(settled(&d, None, None).get("gate").is_none()); + let _ = std::fs::remove_dir_all(&d); + } + #[test] fn depth_one_refuses_to_spawn() { let d = tmp("depth"); diff --git a/crates/tole-core/src/handoff.rs b/crates/tole-core/src/handoff.rs index 6533712..ecd49e9 100644 --- a/crates/tole-core/src/handoff.rs +++ b/crates/tole-core/src/handoff.rs @@ -24,7 +24,10 @@ //! workspace or look like command-line options, and the helpers that render //! text for another model escape control characters. +use crate::tool::{Risk, Tool}; use serde::{Deserialize, Serialize}; +use serde_json::{json, Value}; +use std::path::PathBuf; /// Schema version written and accepted. pub const VERSION: u32 = 1; @@ -183,6 +186,21 @@ fn check_text(what: &str, s: &str, max: usize, allow_empty: bool) -> Result<(), Ok(()) } +impl ArtifactRef { + /// Check one reference (also used for the `consumes` input of + /// `agent_start`). + pub fn validate(&self) -> Result<(), String> { + check_path(&self.path)?; + if let Some(rev) = &self.rev { + check_rev(rev)?; + } + if let Some(note) = &self.note { + check_text("artifact note", note, MAX_NOTE, true)?; + } + Ok(()) + } +} + impl Handoff { /// Check every field's bounds and shapes. pub fn validate(&self) -> Result<(), String> { @@ -197,13 +215,7 @@ impl Handoff { return bad(format!("at most {MAX_ARTIFACTS} artifacts")); } for a in &self.artifacts { - check_path(&a.path)?; - if let Some(rev) = &a.rev { - check_rev(rev)?; - } - if let Some(note) = &a.note { - check_text("artifact note", note, MAX_NOTE, true)?; - } + a.validate()?; } if let Some(v) = &self.verify { check_text("verify command", &v.command, MAX_COMMAND, false)?; @@ -268,6 +280,41 @@ impl Handoff { } } +/// What a child of `role` is told at the top of its mission. +pub fn role_brief(role: Role) -> String { + let job = match role { + Role::Explore => "You are an `explore` agent: find out what is asked, change nothing.", + Role::Implement => "You are an `implement` agent: build what is asked.", + Role::Verify => { + "You are a `verify` agent: check the work you are pointed at; do not change it." + } + Role::Fix => "You are a `fix` agent: repair what is broken, then prove it.", + }; + let verify = if role.requires_verification() { + " Run the verification yourself (tests, build, whatever proves the work) and \ + report its exact command, exit code and a one-line result in `verify` — \ + without passing verification your work is not closed." + } else { + "" + }; + format!( + "{job}{verify} Before you finish, call `handoff_submit` once with a short summary and \ + the files you produced or examined as `artifacts` (workspace-relative path, plus \ + the commit if there is one); the next agent reads them from there." + ) +} + +/// Prompt for the `fix` child after an `implement`/`fix` child closed +/// without ever submitting a handoff. +pub fn missing_handoff_prompt(role: Role) -> String { + format!( + "The previous {} agent closed without submitting a handoff, so its work cannot be \ + verified or closed. Inspect what it left in the workspace, finish or repair it, run \ + the verification yourself, and only then finish.", + role.as_str() + ) +} + /// Escape control characters so a model-authored string cannot break out of /// a single line of text shown to another model. fn escape(s: &str) -> String { @@ -304,6 +351,95 @@ pub fn consume_preamble(artifacts: &[ArtifactRef]) -> String { out } +/// Child-side tool: validates and writes the handoff document to the path +/// the parent chose (`TOLE_AGENT_HANDOFF`). The role is fixed by the parent +/// at spawn time, never taken from the model. Write-tier: the parent adds +/// exactly this tool name to the child's allowlist, which is safe because it +/// can only write one validated, size-bounded file at a fixed location. +pub struct HandoffSubmitTool { + path: PathBuf, + role: Role, +} + +impl HandoffSubmitTool { + pub fn new(path: impl Into, role: Role) -> Self { + Self { + path: path.into(), + role, + } + } +} + +impl Tool for HandoffSubmitTool { + fn name(&self) -> &str { + "handoff_submit" + } + fn risk(&self) -> Risk { + Risk::Write + } + fn summary(&self) -> String { + "Submit this agent's handoff: summary, artifact references and (for implement/fix) its verification result.".into() + } + fn describe(&self, _input: &Value) -> String { + format!("submit the {} handoff", self.role.as_str()) + } + fn spec(&self) -> Option { + Some(json!({ + "type": "object", + "properties": { + "summary": { "type": "string", "description": "What you did and found, briefly" }, + "artifacts": { + "type": "array", + "description": "Files you produced or examined, by reference", + "items": { + "type": "object", + "properties": { + "path": { "type": "string", "description": "Workspace-relative path" }, + "rev": { "type": "string", "description": "Commit (or ref) it was built in" }, + "note": { "type": "string", "description": "One line on what it is" } + }, + "required": ["path"] + } + }, + "verify": { + "type": "object", + "description": "Required for implement/fix: how you verified your work", + "properties": { + "command": { "type": "string" }, + "exit_code": { "type": "integer" }, + "summary": { "type": "string" } + }, + "required": ["command", "exit_code"] + } + }, + "required": ["summary"] + })) + } + fn execute(&self, input: Value) -> Result { + let mut doc = input; + let obj = doc + .as_object_mut() + .ok_or("handoff_submit: input must be an object")?; + // The parent decides the role and the version; the model does not. + obj.insert("version".into(), json!(VERSION)); + obj.insert("role".into(), json!(self.role.as_str())); + if let Some(v) = obj.get_mut("verify").and_then(Value::as_object_mut) { + v.entry("summary").or_insert_with(|| json!("")); + } + let h: Handoff = serde_json::from_value(doc).map_err(|e| format!("handoff_submit: {e}"))?; + let body = h.to_json().map_err(|e| format!("handoff_submit: {e}"))?; + let tmp = self.path.with_extension("json.tmp"); + std::fs::write(&tmp, body).map_err(|e| format!("handoff_submit: write: {e}"))?; + std::fs::rename(&tmp, &self.path).map_err(|e| format!("handoff_submit: replace: {e}"))?; + let gate = match h.gate() { + Gate::Closed => "closed", + Gate::Unverified => "unverified", + Gate::VerifyFailed { .. } => "verify_failed", + }; + Ok(json!({"submitted": true, "role": self.role.as_str(), "gate": gate})) + } +} + #[cfg(test)] mod tests { use super::*; @@ -462,4 +598,64 @@ mod tests { assert!(p.contains("\\u{a}"), "{p:?}"); assert_eq!(consume_preamble(&[]), ""); } + + #[test] + fn role_briefs_tell_gated_roles_to_verify() { + assert!(role_brief(Role::Implement).contains("Run the verification yourself")); + assert!(role_brief(Role::Fix).contains("not closed")); + assert!(!role_brief(Role::Explore).contains("Run the verification")); + assert!(role_brief(Role::Verify).contains("handoff_submit")); + assert!(missing_handoff_prompt(Role::Implement).contains("without submitting a handoff")); + } + + fn tool_in(tag: &str, role: Role) -> (HandoffSubmitTool, PathBuf) { + let d = std::env::temp_dir().join(format!("tole-handoff-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + let p = d.join("handoff.json"); + (HandoffSubmitTool::new(&p, role), p) + } + + #[test] + fn the_submit_tool_writes_a_validated_document_with_the_parents_role() { + let (t, p) = tool_in("ok", Role::Implement); + let out = t + .execute(json!({ + "summary": "built it", + "role": "explore", + "version": 99, + "artifacts": [{"path": "out.txt", "rev": "abc1234"}], + "verify": {"command": "cargo test", "exit_code": 1} + })) + .unwrap(); + assert_eq!(out["gate"], "verify_failed"); + let h = Handoff::from_json(&std::fs::read_to_string(&p).unwrap()).unwrap(); + assert_eq!(h.role, Role::Implement, "the model cannot choose its role"); + assert_eq!(h.version, VERSION); + assert_eq!(h.artifacts[0].path, "out.txt"); + assert_eq!(t.risk(), Risk::Write); + assert!(t.spec().is_some()); + let _ = std::fs::remove_dir_all(p.parent().unwrap()); + } + + #[test] + fn the_submit_tool_refuses_bad_input_and_writes_nothing() { + let (t, p) = tool_in("bad", Role::Explore); + for bad in [ + json!("not an object"), + json!({"summary": ""}), + json!({"summary": "x", "artifacts": [{"path": "../etc/passwd"}]}), + json!({"summary": "x", "artifacts": [{"path": "a", "rev": "--upload-pack=x"}]}), + json!({"summary": "x", "unknown": 1}), + ] { + assert!(t.execute(bad.clone()).is_err(), "{bad}"); + assert!(!p.exists(), "{bad}"); + } + // Closed explore handoff without verification is fine. + assert_eq!( + t.execute(json!({"summary": "found it"})).unwrap()["gate"], + "closed" + ); + let _ = std::fs::remove_dir_all(p.parent().unwrap()); + } } diff --git a/docs/child-agents-handoff.md b/docs/child-agents-handoff.md index e5f4554..31771b3 100644 --- a/docs/child-agents-handoff.md +++ b/docs/child-agents-handoff.md @@ -1,8 +1,8 @@ # Child-agent handoff and verify gate (#383) -Status: the schema, validation and close gate exist (`tole_core::handoff`); -how a child submits it and how `agent_poll` enforces the gate follow in a -second change. This page is the design both parts follow. +Status: implemented — the schema, validation and close gate +(`tole_core::handoff`), the child-side `handoff_submit` tool, the `role` / +`consumes` inputs of `agent_start` and the gate in `agent_poll`. ## Problem @@ -66,6 +66,29 @@ Neither failure is closed silently. `Handoff::fix_prompt()` builds the prompt of the `fix` child that should follow: what failed, the evidence the child reported, and the artifact references to read. +## How it is wired + +- **Start**: `agent_start` takes an optional `role` and an optional + `consumes` list of artifact references. The child's mission is the + requested prompt, preceded by a short role brief and followed by the + artifacts to read (rendered with control characters escaped). With neither + input the child's prompt is exactly what was asked for. +- **Submit**: a role child gets the `handoff_submit` tool (the parent sets + `TOLE_AGENT_ROLE` and `TOLE_AGENT_HANDOFF`). The tool validates the input + and writes `/handoff.json`; the role comes from the parent, not + from the model. It is Write-tier and the parent adds exactly this name to + the child's allowlist: it can only write that one validated, bounded file, + so it does not widen what the child may do. +- **Poll**: for a settled role child `agent_poll` adds `role`, `gate` + (`closed`, `unverified`, `verify_failed`, or `no_handoff` for roles that + are not gated), the `handoff` itself, and — whenever the work is not + closed — a `fix_prompt`. A gated child that submitted nothing, or an + invalid document, is `unverified`. +- **Fix**: nothing is spawned automatically. The parent model sees the + failed gate and the ready-made `fix_prompt` and decides whether to start a + `fix` child (an `agent_start` with `role: fix`), so spawning, approvals and + budgets stay under the parent's policy. + ## Anti-goals - **No DAG scheduler or dependency engine.** The dependency edge is the data diff --git a/docs/threat-model.md b/docs/threat-model.md index 7799f30..dbb47fa 100644 --- a/docs/threat-model.md +++ b/docs/threat-model.md @@ -200,6 +200,18 @@ tier as the session system prompt. Child sessions are ordinary durable sessions: approval gates, risk tiers, and secret redaction apply unchanged inside them. +Typed handoff (#383) and sibling notices (#380) add two child-to-agent +channels, both carrying model-authored text that is treated as untrusted. A +role child gets a Write-tier `handoff_submit` tool that `agent_start` +allowlists by exact name; it can only write one schema-validated, bounded +file at the path the parent chose (the role is fixed by the parent), so it +does not widen what the child may do. Artifact paths and refs are validated +(workspace-relative, no `..`, backslash, colon or leading `-`; plain ref +names), and any handoff or sibling-log text shown to another model is +escaped and bounded. The shared sibling log is a same-user channel between +processes that already share the parent's authority; a forged line can only +add notices, and ids read from it must match the agent-id charset. + ## `systemone_decide` (#172/#173) ReadOnly tool, active only when `SYSTEMONE_API_KEY` is set