diff --git a/CHANGELOG.md b/CHANGELOG.md index bab0419..bf9a73f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Agent contract**: verb templates gain a `{session_name}` placeholder, and + the built-in `claude` agent now names its session with it instead of spelling + `voro-{task_id}` itself. Every session Voro launches carries a name it + composed — `voro-` for a dispatch (unchanged), `voro--refine` for a + refine, `voro-plan-` for a planning session — so a refine no longer + launches under the literal, shared name `voro-{task_id}` and can be found and + attached to in `claude agents`. `{session_name}` is honoured on `dispatch` and + `plan` and refused elsewhere, the same rule `{model}` follows; `{task_id}` is + now refused on `plan`, whose target may be a project with no task. No verb was + added: an agent defining only `dispatch` (or `cmd`) refines exactly as before. + A wholesale `[agents.claude]` override copied from an older `voro.toml` keeps + working, and keeps its old naming. +- Prompts and command lines are filled by a single-pass renderer, so a task + body, branch name, document title or project name containing `{task_id}`, + `{db}`, `{note}`, `{seed}`, `{branch}` or `{docs}` now reaches the agent + verbatim instead of being rewritten before it is read. - `projects.path` is gone. Existing databases convert in place: every project's old path becomes its default repo, and existing tasks resolve to exactly the checkouts they had before. `voro project add` and `voro project path` keep diff --git a/crates/voro-core/src/agent.rs b/crates/voro-core/src/agent.rs index b0c6ec8..a219634 100644 --- a/crates/voro-core/src/agent.rs +++ b/crates/voro-core/src/agent.rs @@ -19,16 +19,25 @@ use serde::Deserialize; use crate::error::{Error, Result}; use crate::scheduler::{AttentionCosts, DEFAULT_MAX_RUNNING}; +use crate::template::{render, shell_quote}; /// The prompt-file substitution in the `dispatch` template. The working /// directory is handled by the spawner, not the template. pub const PROMPT_FILE_PLACEHOLDER: &str = "{prompt_file}"; /// The task-id substitution in the `dispatch` template, the numeric id of the -/// task. Optional — a template that omits it dispatches unchanged — so agents -/// with a session-naming flag can tie the session back to its task. +/// task. Optional — a template that omits it dispatches unchanged — so a +/// template can put the id somewhere other than the session name. Refused on +/// `plan`, which serves targets that have no task id to bind. pub const TASK_ID_PLACEHOLDER: &str = "{task_id}"; +/// The session-name substitution in the `dispatch` and `plan` templates: the +/// name Voro composes for the session a launch opens ([`Launch::session_name`]), +/// so every backgrounded session is findable by a name Voro chose. Optional, +/// and refused on the session verbs for the same reason [`MODEL_PLACEHOLDER`] +/// is — they act on a session that already exists and has its name. +pub const SESSION_NAME_PLACEHOLDER: &str = "{session_name}"; + /// The session-reference substitution in the `attach` and `resume` templates: /// the agent-opaque reference captured at dispatch (a Claude session UUID, a /// Codex session id, a tmux session name). @@ -64,6 +73,12 @@ pub const VIEWER_BASE_PLACEHOLDER: &str = "{base}"; /// plans interactively in the foreground; `codex` covers the headless-resume /// shape. Must parse and pass [`validate_agent`]. /// +/// Both claude verbs name their session from `{session_name}` rather than +/// spelling `voro-{task_id}` themselves, so every launch Voro makes — a +/// dispatch, a refine, a planning session — carries a distinct Voro-composed +/// name in `claude agents` and the `/resume` picker (DESIGN.md §8). `--name` is +/// not a background-only flag, so the foreground `plan` verb takes it too. +/// /// The claude verbs take their model from `{model}` rather than a baked-in /// flag, so the model varies per purpose and per task: `model` is the workhorse /// a normal dispatch runs, `model_deep` the stronger one a `deep` task earns, @@ -75,11 +90,11 @@ pub const VIEWER_BASE_PLACEHOLDER: &str = "{base}"; /// no-model-direction case: a deep task dispatches with it unchanged. const BUILTIN_AGENTS: &str = "\ [agents.claude] -dispatch = \"claude --bg --name \\\"voro-{task_id}\\\" --permission-mode auto --model {model} \\\"$(cat {prompt_file})\\\"\" +dispatch = \"claude --bg --name \\\"{session_name}\\\" --permission-mode auto --model {model} \\\"$(cat {prompt_file})\\\"\" sessions = \"claude agents --json\" attach = \"claude attach {session}\" resume = \"claude --resume {session}\" -plan = \"claude --permission-mode auto --model {model} \\\"$(cat {prompt_file})\\\"\" +plan = \"claude --name \\\"{session_name}\\\" --permission-mode auto --model {model} \\\"$(cat {prompt_file})\\\"\" model = \"opus\" model_deep = \"fable\" model_plan = \"fable\" @@ -119,13 +134,18 @@ const STARTER_HEADER: &str = r#"# Voro configuration (~/.config/voro/voro.toml). # # * add your own agent — a new [agents.] table. Only `dispatch` is # required (`cmd` is an alias): it starts a session on a task, with -# `{prompt_file}` replaced by the prompt file's path and the optional -# `{task_id}` by the task's numeric id. The optional session verbs unlock -# attachable dispatch, and each degrades gracefully when absent: +# `{prompt_file}` replaced by the prompt file's path, the optional +# `{session_name}` by the name Voro composes for the session (`voro-` +# for a dispatch, `voro--refine` for a refine, `voro-plan-` +# for planning), and the optional `{task_id}` by the task's numeric id. +# The optional session verbs unlock attachable dispatch, and each degrades +# gracefully when absent: # sessions list the agent's sessions as JSON (liveness + ref capture) # attach open a running session interactively ({session}) # resume reopen a finished session interactively ({session}) # plan run an interactive foreground planning session ({prompt_file}) +# `plan` may carry `{session_name}` too, but not `{task_id}`: a planning +# session drafts a task rather than naming one. # `dispatch` and `plan` may also carry `{model}`, filled from this agent's # own model keys: `model` normally, `model_deep` for a task flagged deep # (`voro set --deep`), and `model_plan` when planning — the last two @@ -201,8 +221,9 @@ fn starter_config() -> String { /// A named set of verb templates from `voro.toml`. `dispatch` (or its alias /// `cmd`) is required and always contains [`PROMPT_FILE_PLACEHOLDER`]; it may -/// also carry the optional [`TASK_ID_PLACEHOLDER`]. The rest are optional, with -/// their `{session}`/`{prompt_file}` placeholders validated at parse time. +/// also carry the optional [`SESSION_NAME_PLACEHOLDER`] and +/// [`TASK_ID_PLACEHOLDER`]. The rest are optional, with their +/// `{session}`/`{prompt_file}` placeholders validated at parse time. #[derive(Debug, Clone, Deserialize)] #[serde(deny_unknown_fields)] pub struct AgentTemplate { @@ -266,15 +287,97 @@ impl AgentTemplate { } } -/// Substitute [`MODEL_PLACEHOLDER`] in a verb template. `model` is the name -/// that verb resolved to, or `None` for an agent that declares none — in which -/// case the template carries no placeholder either (`validate_agent` enforces -/// that pairing), so this is a no-op. -fn render_model(template: &str, model: Option<&str>) -> String { - match model { - Some(model) => template.replace(MODEL_PLACEHOLDER, model), - None => template.to_string(), +/// What a launch *is* (DESIGN.md §8): the one place a backgrounded or +/// foreground agent session's identity is composed. A launch names its session, +/// its prompt and log files, and its line in the launch log from this single +/// value, so a new flavour of launch cannot inherit one of those and forget +/// another — which is exactly how a refine came to be named `voro-{task_id}`, +/// literally, on every task at once. +/// +/// The invariant it carries: every session Voro launches has a Voro-composed +/// name, `voro-` for a dispatch and `voro--` for anything else +/// pointed at that task, so nothing Voro starts shows up anonymous or +/// duplicately named in the agent's own session listing. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Launch { + /// A task dispatched to a headless session (DESIGN.md §8). + Dispatch { task_id: i64 }, + /// A proposed task's body rewritten by an agent (DESIGN.md §6), at either + /// intensity — the headless note-driven one and the interactive one are the + /// same operation, and only one of them is ever backgrounded. + Refine { task_id: i64 }, + /// An interactive planning session drafting a new task for a project. + Plan { project_id: i64 }, +} + +impl Launch { + /// The name the agent's session carries, filling [`SESSION_NAME_PLACEHOLDER`]. + /// `voro-` for a dispatch is the published contract + /// (docs/agent-integration.md) and what `attach` and the `/resume` picker + /// are read by, so anything else pointed at the same task suffixes a kind + /// rather than colliding with it. + pub fn session_name(&self) -> String { + match self { + Launch::Dispatch { task_id } => format!("voro-{task_id}"), + Launch::Refine { task_id } => format!("voro-{task_id}-refine"), + Launch::Plan { project_id } => format!("voro-plan-{project_id}"), + } + } + + /// The stem of this launch's prompt and log files, and the label its + /// launch-log lines carry. + pub fn slug(&self) -> String { + match self { + Launch::Dispatch { task_id } => format!("task-{task_id}"), + Launch::Refine { task_id } => format!("refine-{task_id}"), + Launch::Plan { project_id } => format!("plan-{project_id}"), + } } + + /// The task this launch is pointed at, if any — `None` for a planning + /// session, which drafts a task rather than naming one, and why + /// [`TASK_ID_PLACEHOLDER`] is refused on the `plan` verb. + pub fn task_id(&self) -> Option { + match self { + Launch::Dispatch { task_id } | Launch::Refine { task_id } => Some(*task_id), + Launch::Plan { .. } => None, + } + } +} + +/// Everything a verb template needs bound to become a command line: which +/// launch this is, the prompt file written for it, and whether the task earns +/// the deeper model. Assembled by the caller that wrote the prompt, rendered by +/// [`ResolvedAgent::launch_command`] or +/// [`ResolvedAgent::plan_launch_command`](ResolvedAgent::plan_launch_command). +#[derive(Debug, Clone, Copy)] +pub struct LaunchSpec<'a> { + pub launch: Launch, + pub prompt_file: &'a Path, + /// Whether the task carries the `deep` flag; ignored by the plan template, + /// which has no depth to read. + pub deep: bool, +} + +/// Bind every launch placeholder a verb template may carry, in one pass, so no +/// value's own braces are re-scanned. `{task_id}` goes unbound for a launch that +/// has none, which only a `plan` template could contain — and that is refused at +/// config load. +fn render_launch(template: &str, spec: &LaunchSpec, model: Option<&str>) -> String { + let prompt_file = shell_quote(spec.prompt_file); + let session_name = spec.launch.session_name(); + let task_id = spec.launch.task_id().map(|id| id.to_string()); + let mut bindings = vec![ + (PROMPT_FILE_PLACEHOLDER, prompt_file.as_str()), + (SESSION_NAME_PLACEHOLDER, session_name.as_str()), + ]; + if let Some(task_id) = &task_id { + bindings.push((TASK_ID_PLACEHOLDER, task_id.as_str())); + } + if let Some(model) = model { + bindings.push((MODEL_PLACEHOLDER, model)); + } + render(template, &bindings) } /// A viewer command template from `voro.toml` (DESIGN.md §11a): a shell command @@ -423,22 +526,44 @@ fn validate_agent(name: &str, agent: &AgentTemplate, path: &Path) -> Result<()> "agent '{name}' plan is missing the {PROMPT_FILE_PLACEHOLDER} placeholder" ))); } - // `{model}` is resolved only where a command launches work, so it is - // meaningful on `dispatch` and `plan` and nowhere else; anywhere else it - // would reach the shell unsubstituted. + // `{model}`, `{session_name}` and `{task_id}` are all resolved only where a + // command launches work, so they are meaningful on `dispatch` and `plan` + // and nowhere else; on a session verb they would reach the shell as literal + // braces. No launch placeholder may survive to a command line: either a + // renderer binds it or it is refused here. for (verb, template) in [ ("sessions", &agent.sessions), ("attach", &agent.attach), ("resume", &agent.resume), ] { - if let Some(template) = template - && template.contains(MODEL_PLACEHOLDER) - { + let Some(template) = template else { continue }; + if template.contains(MODEL_PLACEHOLDER) { return Err(invalid(format!( "agent '{name}' {verb} carries {MODEL_PLACEHOLDER}, which is resolved only on \ dispatch and plan — a session verb reuses the model its session started with" ))); } + for placeholder in [SESSION_NAME_PLACEHOLDER, TASK_ID_PLACEHOLDER] { + if template.contains(placeholder) { + return Err(invalid(format!( + "agent '{name}' {verb} carries {placeholder}, which is resolved only on \ + dispatch and plan — a session verb names its session with {SESSION_PLACEHOLDER}, \ + the reference Voro captured at launch" + ))); + } + } + } + // `plan` serves a target that has no task: a planning session drafts a task + // rather than naming one, so `{task_id}` there has nothing to bind to. A + // template must render for every target its verb serves. + if let Some(template) = &agent.plan + && template.contains(TASK_ID_PLACEHOLDER) + { + return Err(invalid(format!( + "agent '{name}' plan carries {TASK_ID_PLACEHOLDER}, but a planning session drafts a \ + task rather than naming one — use {SESSION_NAME_PLACEHOLDER}, which Voro composes \ + for every launch" + ))); } // The model keys are inert without the placeholder (a wholesale override // that drops `{model}` keeps loading), but the placeholder without them @@ -471,10 +596,11 @@ fn binary_on_path(name: &str) -> bool { /// has one, otherwise the config's global default, with every verb template /// resolved. /// -/// The `dispatch` and `plan` fields still hold `{model}` unresolved, because -/// which model they name depends on the task: reach them through -/// [`dispatch_command`](Self::dispatch_command) and -/// [`plan_command`](Self::plan_command). +/// The `dispatch` and `plan` fields still hold their placeholders unresolved, +/// because what they bind to depends on the launch: reach them through +/// [`launch_command`](Self::launch_command) and +/// [`plan_launch_command`](Self::plan_launch_command), which return a command +/// line with nothing left to substitute. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ResolvedAgent { pub name: String, @@ -489,28 +615,31 @@ pub struct ResolvedAgent { } impl ResolvedAgent { - /// The dispatch command with `{model}` resolved for a task of this depth - /// (DESIGN.md §8): `model_deep` for a deep task, falling back to `model` - /// when the agent names no deeper one, and `model` otherwise. An agent - /// whose template carries no placeholder renders the same string either - /// way — the graceful degradation of the `deep` flag. - pub fn dispatch_command(&self, deep: bool) -> String { - let model = if deep { + /// The dispatch template rendered into a runnable command line: the prompt + /// file shell-quoted, the launch's session name and task id bound, and + /// `{model}` resolved for the task's depth (DESIGN.md §8) — `model_deep` for + /// a deep task, falling back to `model` when the agent names no deeper one, + /// and `model` otherwise. An agent whose template carries no placeholder + /// renders the same string either way, the graceful degradation of the + /// `deep` flag. + pub fn launch_command(&self, spec: &LaunchSpec) -> String { + let model = if spec.deep { self.model_deep.as_deref().or(self.model.as_deref()) } else { self.model.as_deref() }; - render_model(&self.dispatch, model) + render_launch(&self.dispatch, spec, model) } - /// The plan command with `{model}` resolved — `model_plan`, falling back to - /// `model` — when the agent defines the verb. Planning has no depth: it is - /// interactive reasoning either way. - pub fn plan_command(&self) -> Option { + /// The plan template rendered the same way, when the agent defines the + /// verb, with `{model}` resolved to `model_plan` falling back to `model`. + /// Planning has no depth: it is interactive reasoning either way, so + /// `spec.deep` is not read. + pub fn plan_launch_command(&self, spec: &LaunchSpec) -> Option { let model = self.model_plan.as_deref().or(self.model.as_deref()); self.plan .as_deref() - .map(|template| render_model(template, model)) + .map(|template| render_launch(template, spec, model)) } } @@ -1499,7 +1628,7 @@ mod tests { let claude = config.agent("claude").unwrap(); assert!(claude.dispatch().contains("--bg"), "{}", claude.dispatch()); assert!( - claude.dispatch().contains("voro-{task_id}"), + claude.dispatch().contains(SESSION_NAME_PLACEHOLDER), "{}", claude.dispatch() ); @@ -1571,7 +1700,80 @@ mod tests { assert!(agents["codex"].resume().is_some()); } - // --- the model map and {model} (task #241) --- + // --- launch identity and rendered commands (task #326) --- + + /// A dispatch of task 7 with a fixed prompt file, so a rendered command is + /// a stable string to assert on. + fn spec(deep: bool) -> LaunchSpec<'static> { + LaunchSpec { + launch: Launch::Dispatch { task_id: 7 }, + prompt_file: Path::new("/tmp/p.md"), + deep, + } + } + + #[test] + fn a_launch_names_its_session_and_its_files() { + let dispatch = Launch::Dispatch { task_id: 42 }; + let refine = Launch::Refine { task_id: 42 }; + let plan = Launch::Plan { project_id: 3 }; + // The dispatch name is the published contract; anything else pointed at + // the same task suffixes a kind rather than colliding with it. + assert_eq!(dispatch.session_name(), "voro-42"); + assert_eq!(refine.session_name(), "voro-42-refine"); + assert_eq!(plan.session_name(), "voro-plan-3"); + assert_ne!(dispatch.session_name(), refine.session_name()); + // The file slugs are exactly what the three paths computed before the + // identity was factored out, so no prompt or log filename moved. + assert_eq!(dispatch.slug(), "task-42"); + assert_eq!(refine.slug(), "refine-42"); + assert_eq!(plan.slug(), "plan-3"); + assert_eq!(dispatch.task_id(), Some(42)); + assert_eq!(refine.task_id(), Some(42)); + assert_eq!(plan.task_id(), None); + } + + #[test] + fn builtin_claude_names_the_session_from_the_launch() { + let config = AgentsConfig::builtin_only(Path::new("/tmp/voro.toml")); + let claude = config.resolve(Some("claude")).unwrap(); + let dispatch = claude.launch_command(&spec(false)); + assert!(dispatch.contains("--name \"voro-7\""), "{dispatch}"); + + let refined = claude.launch_command(&LaunchSpec { + launch: Launch::Refine { task_id: 7 }, + ..spec(false) + }); + assert!(refined.contains("--name \"voro-7-refine\""), "{refined}"); + assert_ne!(dispatch, refined); + + // A planning session is named too, and `--name` is not a --bg-only + // flag, so the foreground plan verb carries it. + let planned = claude + .plan_launch_command(&LaunchSpec { + launch: Launch::Plan { project_id: 3 }, + ..spec(false) + }) + .unwrap(); + assert!(planned.contains("--name \"voro-plan-3\""), "{planned}"); + assert!(!planned.contains("--bg"), "{planned}"); + + // Nothing reaches the shell as literal braces on any of them. + for rendered in [dispatch, refined, planned] { + assert!(!rendered.contains('{'), "unsubstituted: {rendered}"); + } + } + + #[test] + fn the_prompt_file_is_shell_quoted_into_the_command() { + let config = AgentsConfig::builtin_only(Path::new("/tmp/voro.toml")); + let claude = config.resolve(Some("claude")).unwrap(); + let rendered = claude.launch_command(&LaunchSpec { + prompt_file: Path::new("/tmp/a dir/p.md"), + ..spec(false) + }); + assert!(rendered.contains("cat '/tmp/a dir/p.md'"), "{rendered}"); + } #[test] fn builtin_claude_renders_a_model_per_purpose_and_depth() { @@ -1581,24 +1783,21 @@ mod tests { // deep task and for interactive planning; all `claude` model aliases, // so none churns with a release. assert!( - claude.dispatch_command(false).contains("--model opus"), + claude.launch_command(&spec(false)).contains("--model opus"), "{}", - claude.dispatch_command(false) + claude.launch_command(&spec(false)) ); assert!( - claude.dispatch_command(true).contains("--model fable"), + claude.launch_command(&spec(true)).contains("--model fable"), "{}", - claude.dispatch_command(true) - ); - assert!( - claude.plan_command().unwrap().contains("--model fable"), - "{:?}", - claude.plan_command() + claude.launch_command(&spec(true)) ); + let planned = claude.plan_launch_command(&spec(false)).unwrap(); + assert!(planned.contains("--model fable"), "{planned}"); for rendered in [ - claude.dispatch_command(false), - claude.dispatch_command(true), - claude.plan_command().unwrap(), + claude.launch_command(&spec(false)), + claude.launch_command(&spec(true)), + planned, ] { assert!( !rendered.contains(MODEL_PLACEHOLDER), @@ -1611,9 +1810,15 @@ mod tests { fn an_agent_without_the_placeholder_ignores_depth_entirely() { let config = AgentsConfig::builtin_only(Path::new("/tmp/voro.toml")); let codex = config.resolve(Some("codex")).unwrap(); - assert_eq!(codex.dispatch_command(true), codex.dispatch_command(false)); - assert_eq!(codex.dispatch_command(true), codex.dispatch); - assert_eq!(codex.plan_command(), None); + assert_eq!( + codex.launch_command(&spec(true)), + codex.launch_command(&spec(false)) + ); + assert_eq!( + codex.launch_command(&spec(true)), + "codex exec \"$(cat '/tmp/p.md')\"" + ); + assert_eq!(codex.plan_launch_command(&spec(false)), None); } #[test] @@ -1627,16 +1832,16 @@ mod tests { let config = AgentsConfig::parse(text, Path::new("/tmp/voro.toml")).unwrap(); let a = config.resolve(Some("a")).unwrap(); assert_eq!( - a.dispatch_command(false), - "run --model workhorse {prompt_file}" + a.launch_command(&spec(false)), + "run --model workhorse '/tmp/p.md'" ); assert_eq!( - a.dispatch_command(true), - "run --model workhorse {prompt_file}" + a.launch_command(&spec(true)), + "run --model workhorse '/tmp/p.md'" ); assert_eq!( - a.plan_command().unwrap(), - "run -i --model workhorse {prompt_file}" + a.plan_launch_command(&spec(false)).unwrap(), + "run -i --model workhorse '/tmp/p.md'" ); } @@ -1674,8 +1879,8 @@ mod tests { "#; let config = AgentsConfig::parse(text, Path::new("/tmp/voro.toml")).unwrap(); let claude = config.resolve(Some("claude")).unwrap(); - assert_eq!(claude.dispatch_command(true), "claude -p {prompt_file}"); - assert_eq!(claude.dispatch_command(false), "claude -p {prompt_file}"); + assert_eq!(claude.launch_command(&spec(true)), "claude -p '/tmp/p.md'"); + assert_eq!(claude.launch_command(&spec(false)), "claude -p '/tmp/p.md'"); } #[test] @@ -1693,6 +1898,63 @@ mod tests { assert!(message.contains("{model}"), "{message}"); } + /// No launch placeholder may survive to a command line: one a renderer does + /// not bind on that verb is refused at load rather than reaching the shell + /// as literal braces (DESIGN.md §8). + #[test] + fn launch_placeholders_are_refused_on_the_verbs_that_cannot_bind_them() { + for (verb, template) in [ + ("sessions", "list --name {session_name}"), + ("attach", "reopen --name {session_name} {session}"), + ("resume", "reopen {session} --for {task_id}"), + ] { + let text = + format!("[agents.a]\ndispatch = \"run {{prompt_file}}\"\n{verb} = '{template}'\n"); + let message = AgentsConfig::parse(&text, Path::new("/tmp/voro.toml")) + .unwrap_err() + .to_string(); + assert!(message.contains(verb), "{verb}: {message}"); + assert!(message.contains("dispatch and plan"), "{verb}: {message}"); + } + + // `plan` serves a target that has no task, so `{task_id}` is refused + // there even though `dispatch` still honours it. + let text = r#" + [agents.a] + dispatch = "run {prompt_file}" + plan = "run -i --name \"voro-{task_id}\" {prompt_file}" + "#; + let message = AgentsConfig::parse(text, Path::new("/tmp/voro.toml")) + .unwrap_err() + .to_string(); + assert!(message.contains("plan"), "{message}"); + assert!(message.contains(TASK_ID_PLACEHOLDER), "{message}"); + assert!(message.contains(SESSION_NAME_PLACEHOLDER), "{message}"); + } + + #[test] + fn the_session_name_placeholder_is_accepted_on_dispatch_and_plan() { + let text = r#" + [agents.a] + dispatch = "run --name {session_name} --for {task_id} {prompt_file}" + plan = "run -i --name {session_name} {prompt_file}" + "#; + let config = AgentsConfig::parse(text, Path::new("/tmp/voro.toml")).unwrap(); + let a = config.resolve(Some("a")).unwrap(); + assert_eq!( + a.launch_command(&spec(false)), + "run --name voro-7 --for 7 '/tmp/p.md'" + ); + assert_eq!( + a.plan_launch_command(&LaunchSpec { + launch: Launch::Plan { project_id: 2 }, + ..spec(false) + }) + .unwrap(), + "run -i --name voro-plan-2 '/tmp/p.md'" + ); + } + #[test] fn default_agent_key_sets_the_default() { let text = r#" diff --git a/crates/voro-core/src/lib.rs b/crates/voro-core/src/lib.rs index fe47cb6..ce35268 100644 --- a/crates/voro-core/src/lib.rs +++ b/crates/voro-core/src/lib.rs @@ -11,12 +11,14 @@ mod model; mod pr; pub mod scheduler; mod store; +mod template; mod transition; pub use agent::{ - AgentSessionEntry, AgentTemplate, AgentsConfig, PROMPT_FILE_PLACEHOLDER, Provenance, - ResolvedAgent, SESSION_PLACEHOLDER, TASK_ID_PLACEHOLDER, VIEWER_BASE_PLACEHOLDER, - VIEWER_BRANCH_PLACEHOLDER, VIEWER_PATH_PLACEHOLDER, ViewerTemplate, parse_sessions_json, + AgentSessionEntry, AgentTemplate, AgentsConfig, Launch, LaunchSpec, PROMPT_FILE_PLACEHOLDER, + Provenance, ResolvedAgent, SESSION_NAME_PLACEHOLDER, SESSION_PLACEHOLDER, TASK_ID_PLACEHOLDER, + VIEWER_BASE_PLACEHOLDER, VIEWER_BRANCH_PLACEHOLDER, VIEWER_PATH_PLACEHOLDER, ViewerTemplate, + parse_sessions_json, }; pub use error::{Error, Result}; pub use import::{GithubIssue, already_imported, issue_new_task, issue_task_body}; @@ -30,6 +32,7 @@ pub use scheduler::{ ScoreBreakdown, StateCounts, WipGate, }; pub use store::{NewTask, Store, TaskEdit}; +pub use template::{render, shell_quote}; pub use transition::{Action, Triage}; #[cfg(test)] diff --git a/crates/voro-core/src/template.rs b/crates/voro-core/src/template.rs new file mode 100644 index 0000000..df6478e --- /dev/null +++ b/crates/voro-core/src/template.rs @@ -0,0 +1,103 @@ +//! The one substitution routine (DESIGN.md §8). Every template Voro fills — +//! agent command lines, dispatch preambles, planning and refine prompts — goes +//! through [`render`], because chained `String::replace` calls re-scan values +//! they have already emitted: whichever untrusted value goes in last, the ones +//! before it were searched for the placeholders that came after. A task body +//! discussing `{task_id}` is then silently rewritten before the agent sees it. +//! +//! Shell quoting lives here too, so command assembly owns it rather than +//! borrowing it from the TUI crate. + +use std::path::Path; + +/// Fill `template` from `bindings` in a single left-to-right pass: each bound +/// placeholder's value is emitted verbatim and never re-scanned, so a value +/// containing another placeholder survives whatever order the bindings are +/// given in. An unrecognised `{…}` is copied through untouched, which is what +/// makes composing nested blocks safe — render the inner block first, then bind +/// the finished text. +pub fn render(template: &str, bindings: &[(&str, &str)]) -> String { + let mut out = String::with_capacity(template.len()); + let mut rest = template; + while let Some(open) = rest.find('{') { + out.push_str(&rest[..open]); + rest = &rest[open..]; + // Longest match wins, so one placeholder that prefixes another cannot + // shadow it whichever order the bindings arrive in. + let matched = bindings + .iter() + .filter(|(name, _)| rest.starts_with(name)) + .max_by_key(|(name, _)| name.len()); + match matched { + Some((name, value)) => { + out.push_str(value); + rest = &rest[name.len()..]; + } + None => { + out.push('{'); + rest = &rest[1..]; + } + } + } + out.push_str(rest); + out +} + +/// Single-quote a path for safe substitution into an `sh -c` command line. +pub fn shell_quote(path: &Path) -> String { + format!("'{}'", path.to_string_lossy().replace('\'', "'\\''")) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn bound_placeholders_are_filled_and_unknown_ones_copied_through() { + assert_eq!( + render("run {a} then {b} and {c}", &[("{a}", "1"), ("{b}", "2")]), + "run 1 then 2 and {c}" + ); + assert_eq!(render("nothing to do", &[("{a}", "1")]), "nothing to do"); + assert_eq!(render("{a}{a}{a}", &[("{a}", "x")]), "xxx"); + assert_eq!(render("a { b } c", &[("{b}", "x")]), "a { b } c"); + } + + /// The defect this routine exists for: a value that itself contains a + /// placeholder reaches the output as written, whichever order the bindings + /// are given in. Chained `String::replace` cannot promise that. + #[test] + fn a_value_containing_another_placeholder_is_emitted_verbatim() { + let body = "the note says {task_id} is wrong, and {db} too"; + for bindings in [ + vec![("{seed}", body), ("{task_id}", "42"), ("{db}", " --db x")], + vec![("{task_id}", "42"), ("{db}", " --db x"), ("{seed}", body)], + ] { + assert_eq!( + render("task {task_id}:\n{seed}\n{db}", &bindings), + format!("task 42:\n{body}\n --db x") + ); + } + } + + #[test] + fn the_longest_matching_placeholder_wins() { + for bindings in [ + vec![("{project}", "p"), ("{project_arg}", "'p'")], + vec![("{project_arg}", "'p'"), ("{project}", "p")], + ] { + assert_eq!(render("{project} {project_arg}", &bindings), "p 'p'"); + } + } + + #[test] + fn unicode_before_an_unbound_brace_is_not_split() { + assert_eq!(render("— {x} — {y}", &[("{x}", "ok")]), "— ok — {y}"); + } + + #[test] + fn shell_quote_wraps_and_escapes() { + assert_eq!(shell_quote(Path::new("/tmp/a b")), "'/tmp/a b'"); + assert_eq!(shell_quote(Path::new("/tmp/it's")), "'/tmp/it'\\''s'"); + } +} diff --git a/crates/voro/src/dispatch.rs b/crates/voro/src/dispatch.rs index bd66f8b..f2066b1 100644 --- a/crates/voro/src/dispatch.rs +++ b/crates/voro/src/dispatch.rs @@ -18,11 +18,15 @@ use std::process::{Command, Stdio}; use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use voro_core::{ - AgentsConfig, PROMPT_FILE_PLACEHOLDER, ReviewAction, Store, TASK_ID_PLACEHOLDER, TaskState, + AgentsConfig, Launch, LaunchSpec, ResolvedAgent, ReviewAction, Store, TaskState, VIEWER_BASE_PLACEHOLDER, VIEWER_BRANCH_PLACEHOLDER, VIEWER_PATH_PLACEHOLDER, - parse_sessions_json, + parse_sessions_json, render, }; +/// Command assembly owns its own quoting, so this is voro-core's; re-exported +/// here because the TUI reaches it through this module. +pub(crate) use voro_core::shell_quote; + /// Prepended to every dispatched prompt so the agent learns the return-path /// verbs (DESIGN.md §8). [`render_preamble`] fills the `{task_id}` and `{db}` /// placeholders literally rather than relying on inherited `VORO_TASK_ID`/ @@ -245,20 +249,34 @@ fn render_preamble( docs: &[(String, String)], ) -> String { let db_flag = db_flag(db_path); + let task_id = task_id.to_string(); let name = branch.unwrap_or(""); - let register = BRANCH_REGISTER_SENTENCE.replace("{name}", name); - let isolate = BRANCH_ISOLATE_SENTENCE.replace("{name}", name); - let branch_block = match branch { - Some(name) => ASSIGNED_BRANCH_TEMPLATE - .replace("{isolate}", &isolate) - .replace("{register}", ®ister) - .replace("{rebase}", BRANCH_REBASE_SENTENCE) - .replace("{name}", name), - None => UNASSIGNED_BRANCH_TEMPLATE - .replace("{isolate}", &isolate) - .replace("{register}", ®ister) - .replace("{rebase}", BRANCH_REBASE_SENTENCE), + // Rendered innermost-first, each block finished before it is bound into the + // next: a single pass emits a value verbatim, so a branch name spelling + // `{task_id}` survives composition rather than being rewritten by the outer + // template's own bindings. + let leaf = [ + ("{name}", name), + ("{task_id}", task_id.as_str()), + ("{db}", db_flag.as_str()), + ]; + let register = render(BRANCH_REGISTER_SENTENCE, &leaf); + let isolate = render(BRANCH_ISOLATE_SENTENCE, &leaf); + let branch_template = match branch { + Some(_) => ASSIGNED_BRANCH_TEMPLATE, + None => UNASSIGNED_BRANCH_TEMPLATE, }; + let branch_block = render( + branch_template, + &[ + ("{isolate}", isolate.as_str()), + ("{register}", register.as_str()), + ("{rebase}", BRANCH_REBASE_SENTENCE), + ("{name}", name), + ("{task_id}", task_id.as_str()), + ("{db}", db_flag.as_str()), + ], + ); // A task with no linked document renders no block at all, so an unlinked // dispatch's prompt is byte-for-byte what it was before docs existed. let docs_block = if docs.is_empty() { @@ -275,13 +293,17 @@ fn render_preamble( }) .collect::>() .join("\n"); - LINKED_DOCS_TEMPLATE.replace("{list}", &list) + render(LINKED_DOCS_TEMPLATE, &[("{list}", list.as_str())]) }; - RETURN_PATH_PREAMBLE_TEMPLATE - .replace("{branch}", &branch_block) - .replace("{docs}", &docs_block) - .replace("{task_id}", &task_id.to_string()) - .replace("{db}", &db_flag) + render( + RETURN_PATH_PREAMBLE_TEMPLATE, + &[ + ("{branch}", branch_block.as_str()), + ("{docs}", docs_block.as_str()), + ("{task_id}", task_id.as_str()), + ("{db}", db_flag.as_str()), + ], + ) } /// Fill [`PLANNING_PROMPT_TEMPLATE`] for a concrete project: the `voro add` @@ -289,10 +311,14 @@ fn render_preamble( /// [`render_preamble`] does, a `--db` flag only when the database is not the /// default one the verb resolves to unaided. fn render_planning_prompt(project: &str, db_path: &Path) -> String { - PLANNING_PROMPT_TEMPLATE - .replace("{project_arg}", &shell_quote(Path::new(project))) - .replace("{project}", project) - .replace("{db}", &db_flag(db_path)) + render( + PLANNING_PROMPT_TEMPLATE, + &[ + ("{project_arg}", shell_quote(Path::new(project)).as_str()), + ("{project}", project), + ("{db}", db_flag(db_path).as_str()), + ], + ) } /// The seed context both refine flavours open with: the task as it stands, the @@ -346,7 +372,10 @@ fn refine_seed(store: &Store, task: &voro_core::Task) -> Result /// Fill a refine template for a concrete task: the literal task id in the `set` /// command, the same conditional `--db` flag the other prompts render, the seed -/// context, and — for the note-driven flavour — the operator's note. +/// context, and — for the note-driven flavour — the operator's note. The seed +/// carries the task's own body, so it goes through the single-pass renderer: +/// a body that discusses `{task_id}` or `{db}` must reach the agent as written, +/// and rewriting the subject of a rewrite is the one thing this must not do. fn render_refine_prompt( template: &str, task_id: i64, @@ -354,11 +383,15 @@ fn render_refine_prompt( seed: &str, note: &str, ) -> String { - template - .replace("{seed}", seed) - .replace("{note}", note.trim()) - .replace("{task_id}", &task_id.to_string()) - .replace("{db}", &db_flag(db_path)) + render( + template, + &[ + ("{seed}", seed), + ("{note}", note.trim()), + ("{task_id}", task_id.to_string().as_str()), + ("{db}", db_flag(db_path).as_str()), + ], + ) } /// The assembled launch of a planning session: the agent's `plan` template @@ -403,7 +436,7 @@ pub fn plan_session( ) -> Result { let config = AgentsConfig::load(&ctx.agents_path).map_err(|e| e.to_string())?; let agent = config.resolve(None).map_err(|e| e.to_string())?; - let Some(plan_cmd) = agent.plan_command() else { + if agent.plan.is_none() { return Err(format!( "agent '{}' defines no plan template in {} — add plan = \" \ {{prompt_file}}\" to its [agents.{}] table to plan tasks with it", @@ -411,14 +444,14 @@ pub fn plan_session( ctx.agents_path.display(), agent.name )); - }; - let (label, name, cwd, prompt) = match target { + } + let (label, launch, cwd, prompt) = match target { PlanTarget::Create { project_id } => { let project = store.project(project_id).map_err(|e| e.to_string())?; let repo = store.default_repo(project_id).map_err(|e| e.to_string())?; ( "plan", - format!("plan-{project_id}"), + Launch::Plan { project_id }, repo.path, render_planning_prompt(&project.name, &ctx.db_path), ) @@ -429,7 +462,7 @@ pub fn plan_session( let seed = refine_seed(store, &task)?; ( "refine", - format!("refine-{task_id}"), + Launch::Refine { task_id }, repo.path, render_refine_prompt( REFINE_PLAN_PROMPT_TEMPLATE, @@ -441,9 +474,16 @@ pub fn plan_session( ) } }; - let prompt_path = write_prompt(ctx, &name, &prompt)?; + let prompt_path = write_prompt(ctx, &launch.slug(), &prompt)?; + let command = agent + .plan_launch_command(&LaunchSpec { + launch, + prompt_file: &prompt_path, + deep: false, + }) + .expect("the plan verb is checked above"); Ok(PlanLaunch { - command: plan_cmd.replace(PROMPT_FILE_PLACEHOLDER, &shell_quote(&prompt_path)), + command, cwd, label, }) @@ -485,26 +525,35 @@ fn write_prompt(ctx: &DispatchCtx, name: &str, prompt: &str) -> Result { + /// Who this launch is, which names its session, its prompt and log files, + /// and its launch-log lines (DESIGN.md §8). + launch: Launch, + /// The agent that will run it — the *default* one, since expanding an + /// intent is not executing the task. + agent: &'a ResolvedAgent, + /// Whether the task carries `deep`, so the brief is written with the same + /// model the work would be. + deep: bool, prompt: String, cwd: String, } /// Spawn an [`Expansion`] detached, with its output captured to a log beside the -/// session logs, and return a summary line naming that log. The child is reaped -/// in a thread — nothing waits on it, and a zombie would otherwise linger for -/// the life of a long-running TUI (DESIGN.md §8). +/// session logs, and return the tail of a summary line naming the session and +/// that log. Both halves matter: the launcher exits at birth, so the log holds +/// its banner rather than the rewrite, and the session name is what the operator +/// attaches to. The child is reaped in a thread — nothing waits on it, and a +/// zombie would otherwise linger for the life of a long-running TUI +/// (DESIGN.md §8). fn spawn_expansion(ctx: &DispatchCtx, exp: Expansion) -> Result { - let prompt_path = write_prompt(ctx, &exp.label, &exp.prompt)?; + let label = exp.launch.slug(); + let prompt_path = write_prompt(ctx, &label, &exp.prompt)?; let stamp = prompt_path .file_stem() .and_then(|s| s.to_str()) .map(|s| s.trim_end_matches(".prompt").to_string()) - .unwrap_or_else(|| exp.label.clone()); + .unwrap_or_else(|| label.clone()); let log_path = ctx.runtime_dir.join(format!("{stamp}.log")); let log = File::create(&log_path) .map_err(|e| format!("cannot create log {}: {e}", log_path.display()))?; @@ -512,13 +561,15 @@ fn spawn_expansion(ctx: &DispatchCtx, exp: Expansion) -> Result .try_clone() .map_err(|e| format!("cannot open log {}: {e}", log_path.display()))?; - let command = exp - .command - .replace(PROMPT_FILE_PLACEHOLDER, &shell_quote(&prompt_path)); + let command = exp.agent.launch_command(&LaunchSpec { + launch: exp.launch, + prompt_file: &prompt_path, + deep: exp.deep, + }); let launch_log = ctx.launch_log_path(); append_launch_log( &launch_log, - &format!("{}: {command} (cwd {})", exp.label, exp.cwd), + &format!("{label}: {command} (cwd {})", exp.cwd), ); let mut child = Command::new("sh") .arg("-c") @@ -533,7 +584,7 @@ fn spawn_expansion(ctx: &DispatchCtx, exp: Expansion) -> Result .map_err(|e| format!("cannot spawn agent in {}: {e}", exp.cwd))?; let pid = i64::from(child.id()); - let reap_label = exp.label.clone(); + let reap_label = label.clone(); std::thread::spawn(move || { let line = match child.wait() { Ok(status) => format!("{reap_label}: exited with {status}"), @@ -543,8 +594,8 @@ fn spawn_expansion(ctx: &DispatchCtx, exp: Expansion) -> Result }); Ok(format!( - "{} launched (pid {pid}) — log {}", - exp.label, + "as {} (pid {pid}) — log {}", + exp.launch.session_name(), log_path.display() )) } @@ -559,6 +610,11 @@ fn spawn_expansion(ctx: &DispatchCtx, exp: Expansion) -> Result /// `deep` flag still applies, because a task worth the strongest model is worth /// a brief written with it. A human task refines like any other: no agent can /// *execute* it, but its brief is still text an agent can write. +/// +/// The session is named `voro--refine` ([`Launch::Refine`]), so it is +/// findable in the agent's fleet listing and cannot collide with the dispatch of +/// the same task; it still opens no session row, because naming is fleet +/// legibility and a session row is Voro's bookkeeping (DESIGN.md §6). pub fn refine( store: &mut Store, ctx: &DispatchCtx, @@ -582,24 +638,26 @@ pub fn refine( let seed = refine_seed(store, &task)?; let prompt = render_refine_prompt(REFINE_PROMPT_TEMPLATE, task_id, &ctx.db_path, &seed, note); - let summary = spawn_expansion( + let launched = spawn_expansion( ctx, Expansion { - label: format!("refine-{task_id}"), - command: agent.dispatch_command(task.deep), + launch: Launch::Refine { task_id }, + agent: &agent, + deep: task.deep, prompt, cwd: repo.path, }, )?; + let summary = format!( + "task {task_id} sent for refinement by '{}' {launched}", + agent.name + ); // Recorded after the spawn, so a refine that never launched leaves no // marker; the agent rewriting the body is what the marker is about. store .record_refine(task_id, note) .map_err(|e| format!("{summary}, but recording the refine event failed: {e}"))?; - Ok(format!( - "task {task_id} sent for refinement by '{}' — {summary}", - agent.name - )) + Ok(summary) } /// Where dispatch finds its inputs and puts its artefacts. Built from the @@ -858,9 +916,10 @@ fn spawn_session( .duration_since(UNIX_EPOCH) .map(|d| d.as_nanos()) .unwrap_or(0); + let launch = Launch::Dispatch { task_id }; let prompt_path = ctx .runtime_dir - .join(format!("task-{task_id}-{stamp}.prompt.md")); + .join(format!("{}-{stamp}.prompt.md", launch.slug())); let body = if task.body.is_empty() { format!("# {}\n", task.title) } else { @@ -886,20 +945,25 @@ fn spawn_session( std::fs::write(&prompt_path, prompt) .map_err(|e| format!("cannot write prompt {}: {e}", prompt_path.display()))?; - let log_path = ctx.runtime_dir.join(format!("task-{task_id}-{stamp}.log")); + let log_path = ctx + .runtime_dir + .join(format!("{}-{stamp}.log", launch.slug())); let log = File::create(&log_path) .map_err(|e| format!("cannot create log {}: {e}", log_path.display()))?; let log_err = log .try_clone() .map_err(|e| format!("cannot open log {}: {e}", log_path.display()))?; - // `dispatch_command` resolves `{model}` from the task's depth: the deeper - // model for a deep task, the workhorse otherwise, and nothing at all for an - // agent whose template names no model (DESIGN.md §8). - let command = agent - .dispatch_command(task.deep) - .replace(PROMPT_FILE_PLACEHOLDER, &shell_quote(&prompt_path)) - .replace(TASK_ID_PLACEHOLDER, &task_id.to_string()); + // `launch_command` binds every placeholder the template may carry in one + // pass: the prompt file, the session name `voro-`, the task id, and + // `{model}` resolved from the task's depth — the deeper model for a deep + // task, the workhorse otherwise, and nothing at all for an agent whose + // template names no model (DESIGN.md §8). + let command = agent.launch_command(&LaunchSpec { + launch, + prompt_file: &prompt_path, + deep: task.deep, + }); let spawn_ms = SystemTime::now() .duration_since(UNIX_EPOCH) .map(|d| d.as_millis() as i64) @@ -1124,11 +1188,6 @@ fn default_base_branch(repo_path: &str) -> String { } } -/// Single-quote a path for safe substitution into the `sh -c` command line. -pub(crate) fn shell_quote(path: &Path) -> String { - format!("'{}'", path.to_string_lossy().replace('\'', "'\\''")) -} - #[cfg(test)] mod tests { use super::*; @@ -2144,6 +2203,127 @@ mod tests { assert!(prompt.contains("docs/PLAN.md"), "{prompt}"); } + // --- launch identity (task #326) --- + + /// Wait for a file the stub agent wrote to hold `until`, so an assertion + /// about a detached launch races neither the spawn nor the write: the shell + /// creates a redirect target before the command behind it produces + /// anything, so mere existence is not enough. + fn read_marker(path: &Path, until: &str) -> String { + for _ in 0..200 { + if let Ok(text) = std::fs::read_to_string(path) + && text.contains(until) + { + return text.trim().to_string(); + } + std::thread::sleep(Duration::from_millis(20)); + } + panic!("timed out waiting for {} to be written", path.display()) + } + + /// The defect this task fixes: a refine borrowed the dispatch template and + /// left `{task_id}` unsubstituted, so every refine of every task launched a + /// session called, literally, `voro-{task_id}`. Now each launch composes its + /// own name, and the refine of a task cannot collide with its dispatch. + #[test] + fn a_refine_and_a_dispatch_of_one_task_carry_distinct_rendered_names() { + // Each launch writes a marker named for its own session, so the two + // cannot race over one file and the filenames themselves prove the + // names differ. + let (mut store, ctx, project) = fixture( + "echo --name {session_name} --for {task_id} > {session_name}.txt; : {prompt_file}", + ); + let id = proposal(&mut store, &project, false); + + refine(&mut store, &ctx, id, "sharpen it").unwrap(); + let refine_marker = read_marker(&project.join(format!("voro-{id}-refine.txt")), "--for"); + assert_eq!(refine_marker, format!("--name voro-{id}-refine --for {id}")); + + store + .apply(id, voro_core::Action::Triage(voro_core::Triage::Ready)) + .unwrap(); + dispatch(&mut store, &ctx, id, None).unwrap(); + let dispatch_marker = read_marker(&project.join(format!("voro-{id}.txt")), "--for"); + assert_eq!(dispatch_marker, format!("--name voro-{id} --for {id}")); + + assert_ne!(refine_marker, dispatch_marker); + for rendered in [&refine_marker, &dispatch_marker] { + assert!(!rendered.contains('{'), "unsubstituted: {rendered}"); + } + + // The refine's whole command line is in the launch log, named for the + // session the operator can attach to. + let log = std::fs::read_to_string(ctx.launch_log_path()).unwrap(); + let launched = log + .lines() + .find(|l| l.contains(&format!("refine-{id}: echo"))) + .unwrap_or_else(|| panic!("no launch line in {log}")); + assert!( + launched.contains(&format!("voro-{id}-refine")), + "{launched}" + ); + assert!(!launched.contains('{'), "unsubstituted: {launched}"); + } + + /// Defect 3: the refine prompt used to substitute into the seed, so a task + /// body discussing a command template was rewritten before the agent read + /// it — corrupting the very subject of the rewrite. + #[test] + fn a_refine_hands_the_agent_a_body_full_of_placeholders_unchanged() { + let (mut store, ctx, project) = fixture("cat {prompt_file} > refine-prompt.txt"); + let id = proposal(&mut store, &project, false); + let body = "the template says voro-{task_id} and the flag is {db}, verbatim"; + let task = store.task(id).unwrap(); + store + .update_task( + id, + voro_core::TaskEdit { + title: task.title, + body: body.into(), + priority: task.priority, + agent: task.agent, + human: task.human, + deep: task.deep, + }, + ) + .unwrap(); + + refine(&mut store, &ctx, id, "keep the {task_id} in the body").unwrap(); + + let prompt = read_captured_prompt(&project.join("refine-prompt.txt")); + assert!(prompt.contains(body), "{prompt}"); + assert!( + prompt.contains("keep the {task_id} in the body"), + "{prompt}" + ); + // ...while the template's own placeholders still resolved around it. + assert!(prompt.contains(&format!("voro set {id} --db")), "{prompt}"); + } + + /// The same guarantee on the dispatch preamble, whose untrusted values are + /// the branch name and the linked documents' titles. + #[test] + fn a_dispatch_hands_the_agent_branch_and_doc_names_unchanged() { + let (mut store, ctx, project) = fixture("cat {prompt_file} > dispatch-prompt.txt"); + let id = ready_task(&mut store, &project); + let project_id = store.task(id).unwrap().project_id; + store.set_branch(id, Some("feat/{task_id}")).unwrap(); + let doc = store + .create_doc(project_id, None, "docs/{db}.md", Some("Plan for {task_id}")) + .unwrap(); + store.link_doc(id, doc.id).unwrap(); + + dispatch(&mut store, &ctx, id, None).unwrap(); + + let prompt = read_marker(&project.join("dispatch-prompt.txt"), "Detailed prompt."); + assert!(prompt.contains("git branch `feat/{task_id}`"), "{prompt}"); + assert!(prompt.contains("Plan for {task_id}"), "{prompt}"); + assert!(prompt.contains("docs/{db}.md"), "{prompt}"); + // and the preamble's own placeholders still resolved + assert!(prompt.contains(&format!("voro ask {id} --db")), "{prompt}"); + assert!(prompt.contains("--branch feat/{task_id}"), "{prompt}"); + } + #[test] fn refine_is_refused_off_proposed_and_without_a_note() { let (mut store, ctx, project) = fixture("cat {prompt_file}"); diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 32d0c07..fc79d00 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -158,7 +158,7 @@ The **docs** tables (§3) are purely additive — no existing row changes shape A project that has stopped mattering is **archived** rather than deleted: `voro project archive` (and the projects screen's `A` key) sets the flag, and every cockpit view — the queue, `voro next`, the state counts and `stats`, the running strip — excludes the project and *all* of its tasks, whatever state each holds. This is retirement, not a transition: no task is moved or closed, the event log is untouched, and unarchiving restores the pre-archive view exactly. It is deliberately distinct from weight 0, which is a snooze — a parked project is expected back and its row sits untagged among the rest — whereas an archived project remains only on the projects screen and `voro project list`, dimmed under an `[archived]` tag, so it can be found and unarchived. The flag also closes the side doors: dispatch and redispatch refuse a task in an archived project, and `add`/`propose`/import refuse to create new work there — the refusals live in `voro-core` beside the human-task guards, so no interface can smuggle work into a retired project. Deleting a project outright stays reserved for one with no tasks at all; removing a project *and* its history is a separate, deliberate purge. -Agent definitions are command templates, not state, so they live outside the database. Voro *owns* the common ones — `claude` and `codex` are compiled into `voro-core`, so they version with the binary and every upgrade carries the current verb set (the session verbs of §8) to every install with no re-init. The user's `~/.config/voro/voro.toml` is then layered on top and is for extensions, overrides, and app options: it may add a new agent, replace a built-in wholesale (a `[agents.claude]` table overrides the built-in claude *entirely*, not per-verb — predictable over a partial merge), and set `default_agent` and the viewers. Viewers are command templates too, and live in the same file for the same reason: named `[viewers.]` tables define how a task's diff is shown locally (§8/§11a), `default_viewer` names the one used when nothing picks a viewer by name, and the older single anonymous `[viewer]` table stays valid as that default (a sole named viewer also serves as the default without being named). Which viewer a *project* uses is state, so it lives in the database (`projects.review_action`, §8), referencing these templates by name. A viewer command carries up to three optional placeholders (§8): `{path}` — the task's worktree, or the project checkout when it has none — plus `{branch}` (the task's branch, empty when it has none) and `{base}` (the checkout's default branch), so `{base}...{branch}` spells the review diff's range rather than opening a bare directory. An agent table may also carry a small **model map** beside its verbs — `model`, `model_deep`, and `model_plan` — whose values fill the `{model}` placeholder in the `dispatch` and `plan` templates (§8). They are plain strings, opaque to Voro, which is why they live in the same file as the templates they are pasted into rather than in the schema: the model is part of how a command is spelled, not state about a task. It also carries the queue's two pricing options — `max_running`, the dispatch WIP cap, and a `[costs]` table overriding the per-action attention divisors (§7) — for the same reason the viewers live here: they are operator preference about how the tool behaves, not state about a task, and a divisor is meaningless to anything but the rendering of the queue. Both are optional and both are validated at load, since a non-positive divisor or a negative cap would produce a nonsense order rather than an obvious error. Because it carries app options like the viewers and not just agents, the file is named `voro.toml`. A missing file is not an error; the built-ins alone are a working config, so a fresh install with `claude` on PATH dispatches without any TOML. `voro agent list` shows the effective set with each agent's provenance — built-in, user, or user-override — and warns when a user override of a built-in drops verbs the built-in defined, the one staleness case layering cannot fix; `voro viewer list` does the same for viewers, flagging the default. +Agent definitions are command templates, not state, so they live outside the database. Voro *owns* the common ones — `claude` and `codex` are compiled into `voro-core`, so they version with the binary and every upgrade carries the current verb set (the session verbs of §8) to every install with no re-init. The user's `~/.config/voro/voro.toml` is then layered on top and is for extensions, overrides, and app options: it may add a new agent, replace a built-in wholesale (a `[agents.claude]` table overrides the built-in claude *entirely*, not per-verb — predictable over a partial merge), and set `default_agent` and the viewers. Viewers are command templates too, and live in the same file for the same reason: named `[viewers.]` tables define how a task's diff is shown locally (§8/§11a), `default_viewer` names the one used when nothing picks a viewer by name, and the older single anonymous `[viewer]` table stays valid as that default (a sole named viewer also serves as the default without being named). Which viewer a *project* uses is state, so it lives in the database (`projects.review_action`, §8), referencing these templates by name. A viewer command carries up to three optional placeholders (§8): `{path}` — the task's worktree, or the project checkout when it has none — plus `{branch}` (the task's branch, empty when it has none) and `{base}` (the checkout's default branch), so `{base}...{branch}` spells the review diff's range rather than opening a bare directory. An agent table may also carry a small **model map** beside its verbs — `model`, `model_deep`, and `model_plan` — whose values fill the `{model}` placeholder in the `dispatch` and `plan` templates (§8). They are plain strings, opaque to Voro, which is why they live in the same file as the templates they are pasted into rather than in the schema: the model is part of how a command is spelled, not state about a task. Two further placeholders in those templates are filled from the launch rather than from this file: `{session_name}`, the name Voro composes for the session a launch opens, and `{task_id}`, the task's numeric id. Like `{model}` they are meaningful only where a command starts work, so both are refused on the session verbs, and `{task_id}` on `plan` as well, whose target may be a project with no task to name (§8). It also carries the queue's two pricing options — `max_running`, the dispatch WIP cap, and a `[costs]` table overriding the per-action attention divisors (§7) — for the same reason the viewers live here: they are operator preference about how the tool behaves, not state about a task, and a divisor is meaningless to anything but the rendering of the queue. Both are optional and both are validated at load, since a non-positive divisor or a negative cap would produce a nonsense order rather than an obvious error. Because it carries app options like the viewers and not just agents, the file is named `voro.toml`. A missing file is not an error; the built-ins alone are a working config, so a fresh install with `claude` on PATH dispatches without any TOML. `voro agent list` shows the effective set with each agent's provenance — built-in, user, or user-override — and warns when a user override of a built-in drops verbs the built-in defined, the one staleness case layering cannot fix; `voro viewer list` does the same for viewers, flagging the default. The file is no longer read-only to Voro. The TUI's Config screen (§9) and the `voro viewer add`/`viewer remove` verbs *edit* it in place — adding, changing, and deleting `[viewers.]` tables and setting `default_viewer`/`default_agent` — through a single write helper (`voro-core::config_edit`) built on `toml_edit`, so a machine write preserves the file's existing content, formatting, and comments and touches only the key it changes. A missing file is created on first edit. The user-owned surface is all that is writable this way: agents stay read-only in the TUI (editing a built-in means writing a wholesale override table, a sharper knife deferred here), and deleting a viewer that a project's `review_action` still names as `viewer:` is refused with the projects named, while deleting the default viewer clears `default_viewer`. @@ -199,11 +199,11 @@ review = 1.4 Three deliberate choices. Any triaged, non-terminal state can be *abandoned* straight to `rejected` — obsolete work must not need walking through the rest of the machine to close, and parking (`ready` → `parked`) has a manual inverse for the same reason. Second, `needs-input`, `review`, and `stalled` are all human-attention states but are kept distinct because they sort differently: at equal score an unanswered question outranks a completed diff, which outranks a dead dispatch, which outranks startable work, which outranks an untriaged proposal — a question stalls in-flight work; a proposal's priority is agent-asserted and untrusted until triage, so it wins nothing but ties it deserves. Third, `proposed` exists precisely so agent-generated tasks can be captured freely without granting them anything: each proposed task competes in the queue on the same score as everything else and cannot be dispatched until a human triages it. Surfacing proposals in the queue rather than behind an approval step keeps the generation pipeline honest without automating it — triage is one keypress away. Under the queue's uniform cap (§7) a low-scoring proposal can fall past the visible rows into the browser, so an always-visible untriaged count is what keeps the pipeline felt when the individual rows drop off. -Triage has a fourth outcome that is deliberately *not* a verdict. A proposal whose body is sub-standard leaves the operator three bad options — accept it as it stands, reject it and lose the work, or pay the manual edit cost — and accepting wins by default, which exports the quality problem downstream to dispatch and review. **Refine** is the fourth: `voro triage refine --note "..."` hands the body, the operator's one-line note, the task's linked documents (§3), and the body and completion summary of the task it was `discovered-from` to a headless agent, whose whole job is to rewrite the body as a dispatchable prompt honouring the note and apply it with `voro set --body-file` — the CLI as the agent's interface, exactly as in dispatch and planning. That seed context is pulled in rather than left to the agent to hunt because it is usually precisely what a sloppy proposal is missing: the plan it was meant to implement, and the work it fell out of. The task stays `proposed` throughout: refine is an *event* on a proposed task, not a state, so nothing about the state machine, the queue, or the score changes, and the improved version comes back round for a real verdict on the next pass. A `refined` event carrying the note marks the row `↻ refined` in the task browser, `list`, and `show` until triage takes the task out of `proposed`, so the operator can see which rows have moved since they last read them. In the queue, where proposals collapse into a per-project digest (§7), the constituent rows carry the marker once the digest is folded open and the digest itself carries the count — `↻ 2 refined` — since a collapsed digest would otherwise hide the very fact that its bodies have improved. The event is recorded at the moment the agent is launched rather than when the body lands — Voro is told, not observing (§8), and the same principle that keeps dispatch agent-agnostic applies here: what the marker promises is that a rewrite was asked for, and the rewritten body is what the operator reads. Refine runs on the *default* agent whatever override the task carries, since an agent override picks who executes a task, not who writes its brief, and it opens no session row: like a planning session it is not a dispatch, it moves no task state, and a run that dies having applied nothing leaves the proposal exactly as it was. +Triage has a fourth outcome that is deliberately *not* a verdict. A proposal whose body is sub-standard leaves the operator three bad options — accept it as it stands, reject it and lose the work, or pay the manual edit cost — and accepting wins by default, which exports the quality problem downstream to dispatch and review. **Refine** is the fourth: `voro triage refine --note "..."` hands the body, the operator's one-line note, the task's linked documents (§3), and the body and completion summary of the task it was `discovered-from` to a headless agent, whose whole job is to rewrite the body as a dispatchable prompt honouring the note and apply it with `voro set --body-file` — the CLI as the agent's interface, exactly as in dispatch and planning. That seed context is pulled in rather than left to the agent to hunt because it is usually precisely what a sloppy proposal is missing: the plan it was meant to implement, and the work it fell out of. The task stays `proposed` throughout: refine is an *event* on a proposed task, not a state, so nothing about the state machine, the queue, or the score changes, and the improved version comes back round for a real verdict on the next pass. A `refined` event carrying the note marks the row `↻ refined` in the task browser, `list`, and `show` until triage takes the task out of `proposed`, so the operator can see which rows have moved since they last read them. In the queue, where proposals collapse into a per-project digest (§7), the constituent rows carry the marker once the digest is folded open and the digest itself carries the count — `↻ 2 refined` — since a collapsed digest would otherwise hide the very fact that its bodies have improved. The event is recorded at the moment the agent is launched rather than when the body lands — Voro is told, not observing (§8), and the same principle that keeps dispatch agent-agnostic applies here: what the marker promises is that a rewrite was asked for, and the rewritten body is what the operator reads. Refine runs on the *default* agent whatever override the task carries, since an agent override picks who executes a task, not who writes its brief, and it opens no session row: like a planning session it is not a dispatch, it moves no task state, and a run that dies having applied nothing leaves the proposal exactly as it was. The session it launches is *named* all the same — `voro--refine` (§8) — so the operator can find it in the agent's own fleet listing and attach to it, which matters precisely because the launcher exits at birth and the log holds its banner rather than the rewrite. Naming is fleet legibility and a session row is Voro's bookkeeping; refine wants the first without the second. It could not have the second cheaply in any case: a refine runs on a `proposed` task, one open session per task is an invariant, so a refine row would be aborted by the very dispatch it might have informed — and it would tie the task to the refining agent rather than the one the task's own override names. Refine has a second, interactive intensity for the case where a note is not enough. Given no note it opens the planning session of §8 seeded with the task that already exists — the same `plan` verb and the same foreground round-trip as `N`, ending in `set --body-file` rather than `add`, so it edits in place and creates nothing. Because it is a conversation with an agent it is TUI-only for the same reason planning sessions are: the CLI is how an LLM drives Voro, so a note-less `refine` there errors and points at the TUI. Both intensities answer over a selected proposal in the queue — `r` collects a note, `R` opens the conversation — and *only* there, not from behind the triage menu, because that menu collects *verdicts* and refine is deliberately not one (above): a proposal that leaves it is still `proposed`, so putting refine there filed an event under a decision it does not make, and hid it one keypress behind the very menu whose three bad options it exists to escape. The operator notices a sub-standard body while reading it in the queue, which is where the key is. The menu does not keep a second copy: one key in one place is the whole point of moving it, and a duplicate would reintroduce the claim that refine is something the verdict menu does. Refresh moves to `ctrl-r` to free the letter, the manual counterpart to the refresh every mutating action already performs. The two intensities share the note-driven path's guards — both are refused on anything but a `proposed` task, before a prompt is written or a process spawned. -The note-driven path is one instance of a general shape: a terse human intent, expanded by an agent into a formal artefact, applied back through an ordinary CLI verb. Expanding a review rejection's one-line feedback the same way is the obvious next instance, so the seed-context-plus-note → agent → apply-via-verb plumbing is factored (`Expansion` in the `voro` crate) rather than written into refine alone. +The note-driven path is one instance of a general shape: a terse human intent, expanded by an agent into a formal artefact, applied back through an ordinary CLI verb. Expanding a review rejection's one-line feedback the same way is the obvious next instance, so the seed-context-plus-note → agent → apply-via-verb plumbing is factored (`Expansion` in the `voro` crate) rather than written into refine alone. Its identity comes from the same `Launch` value every launch uses (§8), so the next instance inherits a session name, a prompt/log file slug and a launch-log label by adding a variant rather than computing each of them again. A **human task** (§3) walks a shortened path through the same machine rather than earning states of its own. Its executor is the human, which collapses two states: `needs-input` is unreachable because the executor cannot be blocked on their own decision — real-world verification of a human task's output is a downstream `blocks`-dependent task (often agent work), not a sub-state of this one — and `review` is unreachable because the human is both executor and acceptor, so completion goes `running → done` directly. The transition API enforces this: `ask` on a human task is refused, `complete` lands in `done`, and dispatch and redispatch both refuse to open an agent session on one — the same shape as dispatch's refusal of a non-git checkout, an error saying why rather than a silent skip. Setting an agent override on a human task (or flagging human a task that carries one) is refused too, since the override exists only to pick a dispatch agent. To keep the unreachability total, a task currently sitting in `needs-input` or `review`, or one with an agent session still open, cannot be flagged human as it stands — such a task is demonstrably agent-executed; resolve it first. @@ -255,7 +255,7 @@ Cheap actions need one further guard, or the pricing swaps one swamping for anot **Agent resolution**, in order: the task's `agent` override if set (the capability case — a task that needs image generation is inherently a task for the agent that has it, so the choice is worth persisting on the task); otherwise the global default. The default itself resolves layered too: the user's `default_agent` in `voro.toml` when set, otherwise a PATH probe of the built-ins in a fixed order (claude, then codex) picking the first installed — so a fresh install needs no `default_agent` line. Only when nothing resolves is it an error, with guidance to install an agent or set `default_agent`. The TUI offers two dispatch actions: dispatch-with-resolved-agent on one key, dispatch-via-picker on another — the picker existing mostly for the cap case, where the default is temporarily unusable but nothing about the task has changed. Because the built-ins make a fresh install dispatchable, `voro agent init` is demoted to an optional convenience that writes a commented extension/override skeleton (refusing to clobber an existing one); `voro agent list` shows the effective set with provenance and the default flagged, and `voro agent path` prints where the user file is read from. -**Model selection** rides the same template mechanism rather than a parallel one, and keeps Voro model-blind. A verb template may carry a `{model}` placeholder, filled from a small map of keys on the agent's own table — `model`, `model_deep`, `model_plan` — whose values are opaque strings Voro pastes in and never interprets, so nothing in the tool knows what a model *is* or which is stronger; the operator's config asserts the ordering. A task carries a persistent boolean `deep` (schema §5) that chooses between them at dispatch: `dispatch` renders `model_deep` for a deep task and `model` otherwise, with `model_deep` falling back to `model` when an agent names only one, and `plan` renders `model_plan` (falling back the same way) whatever the queue holds, since a planning session belongs to no task and so has no depth to read. The placeholder is meaningful only on those two verbs — the ones that launch work — and is refused on the session verbs, where a session already runs on whatever model started it. The flag is orthogonal to both of its neighbours: ordering is priority's job and `deep` never touches the score (§7), and the per-task agent override picks *which* agent runs while `deep` picks only how hard it runs. It degrades exactly as the optional verbs do: an agent whose templates carry no `{model}` — the built-in `codex` — takes no model direction, and a deep task dispatches with it identically to any other, no error and no warning beyond the flag `voro show` already prints. The one config error is the inverse pairing, a template carrying `{model}` on an agent that names no model, caught at load; model keys *without* the placeholder are inert rather than an error, so a wholesale override written before this existed keeps loading unchanged. The flag is set with `--deep`/`--no-deep` on `add` and `set`, and toggled in the TUI with `!` on the cockpit and the task browser, where a deep task's row carries a `!` beside its priority. +**Model selection** rides the same template mechanism rather than a parallel one, and keeps Voro model-blind. A verb template may carry a `{model}` placeholder, filled from a small map of keys on the agent's own table — `model`, `model_deep`, `model_plan` — whose values are opaque strings Voro pastes in and never interprets, so nothing in the tool knows what a model *is* or which is stronger; the operator's config asserts the ordering. A task carries a persistent boolean `deep` (schema §5) that chooses between them at dispatch: `dispatch` renders `model_deep` for a deep task and `model` otherwise, with `model_deep` falling back to `model` when an agent names only one, and `plan` renders `model_plan` (falling back the same way) whatever the queue holds, since a planning session belongs to no task and so has no depth to read. The placeholder is meaningful only on those two verbs — the ones that launch work — and is refused on the session verbs, where a session already runs on whatever model started it. The flag is orthogonal to both of its neighbours: ordering is priority's job and `deep` never touches the score (§7), and the per-task agent override picks *which* agent runs while `deep` picks only how hard it runs. It degrades exactly as the optional verbs do: an agent whose templates carry no `{model}` — the built-in `codex` — takes no model direction, and a deep task dispatches with it identically to any other, no error and no warning beyond the flag `voro show` already prints. The one config error is the inverse pairing, a template carrying `{model}` on an agent that names no model, caught at load; model keys *without* the placeholder are inert rather than an error, so a wholesale override written before this existed keeps loading unchanged. The flag is set with `--deep`/`--no-deep` on `add` and `set`, and toggled in the TUI with `!` on the cockpit and the task browser, where a deep task's row carries a `!` beside its priority. `{session_name}` is governed by the same rule as `{model}` — meaningful on the two verbs that launch work and refused on the session verbs — and it expresses the naming invariant: **every session Voro launches into the background carries a Voro-composed name** — `voro-` for a dispatch, `voro--` for anything else pointed at that task, `voro-plan-` for a planning session — so nothing Voro starts shows up anonymous or duplicately named in the agent's own listing, and a refine of task 42 (`voro-42-refine`) cannot collide with its dispatch (`voro-42`), which `attach` and the `/resume` picker are read by. `{task_id}` remains substituted on `dispatch`, for a template that wants the id somewhere other than the name, but is refused on `plan`, which serves a target that has no task id to bind — a template must render for every target its verb serves. Behind all three is one rule: no launch placeholder may survive to a command line. Each is either bound by the single renderer below or refused at config load, never left to reach the shell as literal braces — which is not hypothetical, since a note-driven refine once borrowed the dispatch template without substituting `{task_id}` and so launched every refine of every task under one literal name, `voro-{task_id}`. **Redispatch** is a first-class action born of the same cap case: a dispatch that dies `capped` or `failed` lands its task in `stalled` (the reconciler performs `running → stalled` itself — see *Observing the end of a session* below), and redispatching it offers the agent picker plus the previous session's notes/log so the successor agent does not start cold; dispatch's precondition accepts `stalled → running` beside `ready → running`. `stalled` is a first-class state, not a flag derived from session history: the state machine carries it, the scheduler scores it (§7), and a stalled row's *redispatch* verb comes from the same next-action derivation (§3) as every other row's, reading the state alone. @@ -295,7 +295,7 @@ Naming the id literally is what makes the return path survive the launch style t **Resolving a stale review branch** stays a prompt convention rather than a Voro action, matching the "Voro runs no git during dispatch" boundary above: the task's agent session is still open, so the operator attaches to it (the same jump-in as answering `needs-input`) and asks the agent to rebase. The dispatch preamble already tells the agent how — if its branch conflicts with the base, it runs `git fetch origin ` from inside its own worktree and rebases or merges onto `origin/` there. Fetching only updates remote-tracking refs in the shared ref store and never touches a working tree, including the primary checkout, so it leaves the one trust rule — a dispatched agent cannot push — intact. The task never leaves `review`; the PR simply updates. Voro performs no git for any of this, and automating it away (an operator-git rebase behind a CLI verb and a review → running round trip) was rejected as machinery for a situation that is not a rejection — the work is fine, the branch is merely stale. Detection of the staleness is the `[branch conflicts]` marker above (#138). -The prompt is the task's title and body written to a file outside the checkout, and the agent's command template — the `{prompt_file}` line from `voro.toml` — is run through `sh -c` in the task's resolved repo with the process detached into its own process group and its output captured to a per-session log. Spawning happens only after every check that can fail (task is `ready`, agent resolves, path is a git repository, prompt writes); the `ready → running` transition and the session insert then land together in one transaction, so a running task always has a session and a session always names a live dispatch. A path that is not a git repository is refused, since the dispatched agent does its work in a git worktree of the checkout; a checkout with uncommitted changes is *not* refused, because `git worktree add` snapshots HEAD rather than the working tree, so the operator's in-progress work never enters the agent's diff. +The prompt is the task's title and body written to a file outside the checkout, and the agent's command template — the `{prompt_file}` line from `voro.toml` — is run through `sh -c` in the task's resolved repo with the process detached into its own process group and its output captured to a per-session log. Spawning happens only after every check that can fail (task is `ready`, agent resolves, path is a git repository, prompt writes); the `ready → running` transition and the session insert then land together in one transaction, so a running task always has a session and a session always names a live dispatch. Every template Voro fills — those command lines, the dispatch preamble, the planning and refine prompts — goes through one substitution routine that makes a single left-to-right pass and emits each bound value verbatim, never re-scanning what it has already written. Chained replacements cannot promise that: whichever untrusted value goes in last, the ones before it were searched for the placeholders that came after, so a task body, branch name, document title or project name that *discusses* `{task_id}` or `{db}` was silently rewritten before the agent read it — the one thing a body-rewriting flow must not do. A launch's identity is likewise computed once, from a single `Launch` value naming what is being started, and used for the session name, the stem of the prompt and log files, and the launch-log label together, so a new flavour of launch inherits all three rather than deriving each ad hoc and forgetting one. A path that is not a git repository is refused, since the dispatched agent does its work in a git worktree of the checkout; a checkout with uncommitted changes is *not* refused, because `git worktree add` snapshots HEAD rather than the working tree, so the operator's in-progress work never enters the agent's diff. Because these are plain CLI calls writing to a local SQLite file, they work identically for Claude Code, Codex, or anything that can run a shell command — no per-agent integration beyond the command template in `voro.toml`. An MCP server wrapping the same three verbs is a later nicety, not a requirement. Note these verbs are a thin second consumer of `voro-core`, not a prerequisite for the TUI — they arrive in the milestone that closes the agent loop. @@ -303,7 +303,7 @@ The return path depends on the agent remembering to call it, and for Claude Code Dispatch runs in the **task's resolved repo** (§3/§5) — its own repo when it names one, the project's default otherwise — which must be a git repository; the dispatched agent does its work in a throwaway worktree it creates (the preamble instructs this), so the operator's uncommitted changes never enter its diff. Everything downstream of the dispatch resolves the same way, through the same helper: the git guard and the spawn's cwd, the session-ref capture, `pr`'s push and `gh pr create`, `open`'s worktree lookup and `{base}` branch, and the accept-time worktree cleanup all read the *task's* repo rather than the project's default, because a task dispatched into a second repo has its branch, its worktree, and its PR there. The per-project review action (`projects.review_action`) is untouched by this: the action stays a property of the project, and only its `auto` probe — "is this checkout a GitHub repo?" — runs against the task's checkout, so a multi-repo project can answer differently per task without configuring anything twice. Two consumers deliberately stay on the default repo, because neither executes a task: a planning session (`N`) runs in the default checkout and the task it drafts picks its own repo with `voro add --repo`, and `voro import` defaults there while taking `--repo ` to import from another (the tasks it creates then carry that repo, so an imported issue dispatches where it lives). Voro-managed per-dispatch worktrees are deferred until parallel dispatch within one project is actually wanted (§11). -**Planning sessions** are the same machinery pointed at the *front* of a task's life: agent-assisted task creation, where the operator plans a task interactively with an agent and the deliverable of the session is a Voro task, not a PR. This is TUI-only by design — the CLI is how an LLM drives Voro, so an LLM-drafting verb there would be circular — and it is interactive by design: a one-shot variant (agent expands a description into a pre-filled editor form) was considered and rejected, because task planning is usually a back-and-forth and an interactive session subsumes the one-shot case (say what you want, confirm, exit). From the TUI, `N` (beside `n`'s manual editor, which stays first-class — the same lowercase-default, uppercase-variant pairing as `d`/`D`) picks a project and suspends the terminal in the same round-trip used for `$EDITOR` and attach/resume, launching the default agent's **`plan` verb** in the project's default repo (§3): an optional agent template alongside dispatch/sessions/attach/resume — an interactive *foreground* command carrying `{prompt_file}`, built in for `claude` — that degrades like the other optional verbs, an agent without one yielding a status line saying what to configure. The prompt seeds the session with its job: it is drafting a task for that project; interview the operator as needed; write the body as a self-contained dispatchable prompt (named files, acceptance criteria); and when the operator confirms, create the task with `voro add` — the CLI is the agent's interface exactly as in dispatch, down to the rendered `--db` flag for a non-default store, so Voro gains no new store write path and parses no agent output. When the session exits the TUI refreshes, and the new task appears in the queue as `proposed` for ordinary triage — the human already saw the content, but triage stays uniform. A session that exits without creating a task is a no-op, not an error; no session row is recorded and none of dispatch's guards apply, since planning only reads the checkout and writes nothing to it. The built-in claude verbs reach their per-purpose models through the `{model}` map above — a stronger reasoning model on `plan` and on a deep dispatch, a workhorse on an ordinary one — naming the `claude` model aliases (`fable`, `opus`) rather than pinned ids so they track the current model of each class without churning; an operator overrides the agent wholesale in `voro.toml` to change them (docs/agent-integration.md). The same session serves the *middle* of a proposal's life as well: an interactive refine (§6) is this exact machinery pointed at a task that already exists — same `plan` verb, same foreground round-trip, same absence of a session row — seeded with the current body and ending in `voro set --body-file` instead of `voro add`, so it rewrites in place rather than creating anything. A planning session runs in the project's default repo because the task it drafts has not chosen one; a refine runs in the task's *resolved* repo, since the code its body must name is there. +**Planning sessions** are the same machinery pointed at the *front* of a task's life: agent-assisted task creation, where the operator plans a task interactively with an agent and the deliverable of the session is a Voro task, not a PR. This is TUI-only by design — the CLI is how an LLM drives Voro, so an LLM-drafting verb there would be circular — and it is interactive by design: a one-shot variant (agent expands a description into a pre-filled editor form) was considered and rejected, because task planning is usually a back-and-forth and an interactive session subsumes the one-shot case (say what you want, confirm, exit). From the TUI, `N` (beside `n`'s manual editor, which stays first-class — the same lowercase-default, uppercase-variant pairing as `d`/`D`) picks a project and suspends the terminal in the same round-trip used for `$EDITOR` and attach/resume, launching the default agent's **`plan` verb** in the project's default repo (§3): an optional agent template alongside dispatch/sessions/attach/resume — an interactive *foreground* command carrying `{prompt_file}`, built in for `claude` — that degrades like the other optional verbs, an agent without one yielding a status line saying what to configure. The prompt seeds the session with its job: it is drafting a task for that project; interview the operator as needed; write the body as a self-contained dispatchable prompt (named files, acceptance criteria); and when the operator confirms, create the task with `voro add` — the CLI is the agent's interface exactly as in dispatch, down to the rendered `--db` flag for a non-default store, so Voro gains no new store write path and parses no agent output. When the session exits the TUI refreshes, and the new task appears in the queue as `proposed` for ordinary triage — the human already saw the content, but triage stays uniform. A session that exits without creating a task is a no-op, not an error; no session row is recorded and none of dispatch's guards apply, since planning only reads the checkout and writes nothing to it. The built-in claude verbs reach their per-purpose models through the `{model}` map above — a stronger reasoning model on `plan` and on a deep dispatch, a workhorse on an ordinary one — naming the `claude` model aliases (`fable`, `opus`) rather than pinned ids so they track the current model of each class without churning; an operator overrides the agent wholesale in `voro.toml` to change them (docs/agent-integration.md). The same session serves the *middle* of a proposal's life as well: an interactive refine (§6) is this exact machinery pointed at a task that already exists — same `plan` verb, same foreground round-trip, same absence of a session row — seeded with the current body and ending in `voro set --body-file` instead of `voro add`, so it rewrites in place rather than creating anything. A planning session runs in the project's default repo because the task it drafts has not chosen one; a refine runs in the task's *resolved* repo, since the code its body must name is there. The verb roster stops at `dispatch`/`sessions`/`attach`/`resume`/`plan`: an `expand` verb for the headless refine was considered and rejected, because it would have differed from `dispatch` only in the session name and the model — arguments, not verbs — and every third-party agent defining only `dispatch` would have stopped refining until its config gained one. A launch flavour is an argument. The two launching verbs that do stay distinct differ by *mode of interaction*, detached versus owning the terminal, which is a real difference in the process contract rather than a difference of label. The subprocesses Voro launches that are *not* dispatches — the viewer open (§11a), the attach/resume round-trip (§11a), and the planning session above — do not each earn a session row and its per-session log, but their failures are just as easily swallowed: a detached viewer's output would otherwise go to `/dev/null`, and an attach failure is painted over the instant the TUI reinitialises. They share one append-only `launches.log` beside the per-session logs, recording each launch's command, cwd, and exit status. A failing attach or planning launch additionally holds its own error output on screen until a keypress before the TUI redraws over it. Single rolling file, no rotation — the same single-operator argument as the per-session logs above. diff --git a/docs/agent-integration.md b/docs/agent-integration.md index 5effe16..4bfb732 100644 --- a/docs/agent-integration.md +++ b/docs/agent-integration.md @@ -73,11 +73,11 @@ agent init` writes the same built-ins into a fresh `voro.toml`, commented out. ```toml [agents.claude] -dispatch = "claude --bg --name \"voro-{task_id}\" --permission-mode auto --model {model} \"$(cat {prompt_file})\"" +dispatch = "claude --bg --name \"{session_name}\" --permission-mode auto --model {model} \"$(cat {prompt_file})\"" sessions = "claude agents --json" attach = "claude attach {session}" resume = "claude --resume {session}" -plan = "claude --permission-mode auto --model {model} \"$(cat {prompt_file})\"" +plan = "claude --name \"{session_name}\" --permission-mode auto --model {model} \"$(cat {prompt_file})\"" model = "opus" model_deep = "fable" model_plan = "fable" @@ -87,11 +87,20 @@ dispatch = "codex exec \"$(cat {prompt_file})\"" resume = "codex resume {session}" ``` -- `dispatch` may also carry `{task_id}`, replaced with the task's numeric id. - It is optional — a template that omits it dispatches unchanged — and is used - above to name the session `voro-` (via Claude's `--name` flag) so it is - identifiable in `claude agents` and the `/resume` picker. Agents with no - session-naming flag simply leave it out. +- `dispatch` and `plan` may also carry `{session_name}`, replaced with the name + Voro composes for the session that launch opens. It is used above with + Claude's `--name` flag so every session Voro starts is identifiable in + `claude agents` and the `/resume` picker; agents with no session-naming flag + simply leave it out. The scheme is `voro-` for a dispatch of task ``, + `voro--refine` for a refine of it (DESIGN.md §6), and + `voro-plan-` for a planning session. A dispatch's name is a + stable contract — anything else pointed at the same task suffixes a kind + rather than colliding with it. +- `dispatch` may also carry `{task_id}`, replaced with the task's numeric id, + for a template that wants the id somewhere other than the session name. It is + optional — a template that omits it dispatches unchanged — and it is refused + on `plan`, which serves a target that has no task id to bind, and on the + session verbs, which name their session with `{session}`. - `sessions` prints the agent's sessions as a JSON array; Voro reads `sessionId` (or `id`), `cwd`, `startedAt` (ms epoch), and `state` (`"done"` once finished) from each object and ignores the rest. @@ -102,7 +111,11 @@ resume = "codex resume {session}" task creation (DESIGN.md §8): `{prompt_file}` holds the planning brief, and the command owns the terminal until the conversation ends, so it must not background itself. It carries no `{session}` — a planning session belongs to - no task and records no session row. + no task and records no session row — and no `{task_id}`, since it may be + drafting a task that does not exist yet. It may carry `{session_name}`, as + the built-in does: Claude's `--name` is not a background-only flag, so a + planning or interactive-refine session is findable in the `/resume` picker + afterwards. The **model map** is the last three keys, and it is what `{model}` resolves to. `dispatch` and `plan` may each carry the placeholder; Voro fills it from this