diff --git a/CHANGELOG.md b/CHANGELOG.md index bd46db6..3ac506a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,11 +15,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 it straight into the task's recorded agent session, headlessly, without suspending the cockpit — for a `needs-input`, `review`, or `waiting` task whose session is between turns. On a review or waiting task the message *is* - the rejection: the feedback is appended to the body and logged before - anything is sent. The interactive jump-in moves from `a` to `A`. Agents - declare the capability with a new optional `message` verb (`{session}` plus - `{prompt_file}`), built in for `claude`; an agent without one, such as - `codex`, reports so on the status line and keeps its jump-in. + the rejection: the send goes first and the feedback is appended to the body + and logged behind it, so a message the agent refuses leaves the task + untouched rather than recording feedback nobody received. The interactive + jump-in moves from `a` to `A`. Agents declare the capability with a new + optional `message` verb (`{session}` plus `{prompt_file}`, and the optional + `{new_session}` for an agent that can only be joined by forking — which is + how the built-in `claude` verb reaches a session its supervisor still holds), + built in for `claude`; an agent without one, such as `codex`, reports so on + the status line and keeps its jump-in. - `voro set --unlink :` drops a single dependency edge — `related:7`, `discovered-from:4`, `blocks:9` — named as `voro show` lists it. A pair of tasks carrying two edges keeps the one not named, so an edge diff --git a/Cargo.lock b/Cargo.lock index 922c5c1..5b8f690 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1729,6 +1729,7 @@ dependencies = [ "thiserror 2.0.18", "toml", "toml_edit", + "uuid", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index ea9ae4d..cf60df1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ serde_json = "1" toml = "0.8" toml_edit = "0.22" ratatui = { version = "0.30", features = ["unstable-rendered-line-info"] } +uuid = { version = "1", features = ["v4"] } # The profile that 'dist' will build with [profile.dist] diff --git a/crates/voro-core/Cargo.toml b/crates/voro-core/Cargo.toml index 4a639da..986110c 100644 --- a/crates/voro-core/Cargo.toml +++ b/crates/voro-core/Cargo.toml @@ -16,3 +16,4 @@ serde_json.workspace = true thiserror.workspace = true toml.workspace = true toml_edit.workspace = true +uuid.workspace = true diff --git a/crates/voro-core/src/agent.rs b/crates/voro-core/src/agent.rs index f448d04..2431875 100644 --- a/crates/voro-core/src/agent.rs +++ b/crates/voro-core/src/agent.rs @@ -44,6 +44,16 @@ pub const SESSION_NAME_PLACEHOLDER: &str = "{session_name}"; /// UUID, a Codex session id, a tmux session name). pub const SESSION_PLACEHOLDER: &str = "{session}"; +/// The fresh-reference substitution in the `message` template, bound to a v4 +/// UUID Voro generates for the send (DESIGN.md §8). An agent whose sessions are +/// held by a supervisor cannot be resumed headlessly while that supervisor +/// lives; it can be *forked*, which continues the same conversation under a +/// reference the caller names up front. A `message` template carrying this +/// placeholder is declaring that shape, and the session row follows the fork: +/// what Voro binds here becomes the session's reference. Optional — a template +/// without it resumes in place and keeps the reference it had. +pub const NEW_SESSION_PLACEHOLDER: &str = "{new_session}"; + /// The model substitution in a verb template, resolved from the agent's own /// `model`/`model_deep`/`model_plan` keys (DESIGN.md §8). Voro is model-blind: /// the values are opaque strings it pastes into the command and never @@ -94,13 +104,17 @@ pub const VIEWER_BASE_PLACEHOLDER: &str = "{base}"; /// deliberate: a verb is an opaque per-agent contract, which is exactly what /// lets an agent define a subset of them and degrade per-verb. `codex` defines /// no `message` and the TUI's quick-message key says so on the status line. +/// It forks rather than resumes in place ([`NEW_SESSION_PLACEHOLDER`]): a +/// `claude --bg` session keeps its supervisor process after finishing its turn, +/// and that supervisor refuses a headless `--resume` for as long as it lives, +/// so the plain resume was a send that could never land (DESIGN.md §8). const BUILTIN_AGENTS: &str = "\ [agents.claude] 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}\" -message = \"claude -p --resume {session} \\\"$(cat {prompt_file})\\\"\" +message = \"claude -p --resume {session} --fork-session --session-id {new_session} \\\"$(cat {prompt_file})\\\"\" plan = \"claude --name \\\"{session_name}\\\" --permission-mode auto --model {model} \\\"$(cat {prompt_file})\\\"\" model = \"opus\" model_deep = \"fable\" @@ -155,7 +169,9 @@ const STARTER_HEADER: &str = r#"# Voro configuration (~/.config/voro/voro.toml). # attach open a running session interactively ({session}) # resume reopen a finished session interactively ({session}) # message say one thing into a session headlessly, no terminal -# ({session} and {prompt_file}) +# ({session} and {prompt_file}, plus the optional +# {new_session}: a fresh reference for an agent that can only +# be joined by forking, which the session row then follows) # 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. @@ -425,20 +441,44 @@ fn render_launch(template: &str, spec: &LaunchSpec, model: Option<&str>) -> Stri render(template, &bindings) } -/// Bind a `message` template's two placeholders in one pass, so neither value's -/// own braces are re-scanned: the session reference Voro captured at dispatch -/// and the file holding the message. Both are shell-quoted — the reference is -/// agent-opaque text, not a token Voro may assume is bare. -pub fn render_message(template: &str, session_ref: &str, prompt_file: &Path) -> String { +/// A `message` template rendered into a runnable command line, plus the +/// reference the session will answer to afterwards where the agent forks +/// ([`NEW_SESSION_PLACEHOLDER`]). The caller records that reference only once +/// the send is under way, so a command that never ran leaves the session +/// pointing where it did. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RenderedMessage { + pub command: String, + /// The fresh reference bound to `{new_session}`, or `None` for a template + /// that resumes its session in place. + pub new_session_ref: Option, +} + +/// Bind a `message` template's placeholders in one pass, so no value's own +/// braces are re-scanned: the session reference Voro captured at dispatch, the +/// file holding the message, and — for a template that forks — a freshly +/// generated v4 UUID for the session the send opens. All are shell-quoted; the +/// references are agent-opaque text, not tokens Voro may assume are bare. +pub fn render_message(template: &str, session_ref: &str, prompt_file: &Path) -> RenderedMessage { let session = shell_quote(Path::new(session_ref)); let prompt_file = shell_quote(prompt_file); - render( - template, - &[ - (SESSION_PLACEHOLDER, session.as_str()), - (PROMPT_FILE_PLACEHOLDER, prompt_file.as_str()), - ], - ) + let new_session_ref = template + .contains(NEW_SESSION_PLACEHOLDER) + .then(|| uuid::Uuid::new_v4().to_string()); + let new_session = new_session_ref + .as_deref() + .map(|r| shell_quote(Path::new(r))); + let mut bindings = vec![ + (SESSION_PLACEHOLDER, session.as_str()), + (PROMPT_FILE_PLACEHOLDER, prompt_file.as_str()), + ]; + if let Some(new_session) = &new_session { + bindings.push((NEW_SESSION_PLACEHOLDER, new_session.as_str())); + } + RenderedMessage { + command: render(template, &bindings), + new_session_ref, + } } /// A viewer command template from `voro.toml` (DESIGN.md §11a): a shell command @@ -623,6 +663,23 @@ fn validate_agent(name: &str, agent: &AgentTemplate, path: &Path) -> Result<()> } } } + // `{new_session}` names the session a *send* opens, so `message` is the one + // verb that can bind it; anywhere else it would reach the shell as literal + // braces. + for (verb, template) in [ + ("dispatch", Some(dispatch.as_str())), + ("sessions", agent.sessions.as_deref()), + ("attach", agent.attach.as_deref()), + ("resume", agent.resume.as_deref()), + ("plan", agent.plan.as_deref()), + ] { + if template.is_some_and(|t| t.contains(NEW_SESSION_PLACEHOLDER)) { + return Err(invalid(format!( + "agent '{name}' {verb} carries {NEW_SESSION_PLACEHOLDER}, which is bound only on \ + message — it names the session a headless send forks into" + ))); + } + } // `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. @@ -1680,9 +1737,74 @@ mod tests { Path::new("/run/msg-1.md"), ); assert_eq!( - rendered, + rendered.command, "claude -p --resume '3f6c-1111' \"$(cat '/run/msg-1.md')\"" ); + // A template that resumes in place keeps the reference it was given. + assert_eq!(rendered.new_session_ref, None); + } + + /// A `message` template that forks names the session it forks into, and + /// Voro supplies that name: a fresh v4 UUID, shell-quoted like the rest, + /// handed back so the session row can follow the fork (DESIGN.md §8). + #[test] + fn render_message_binds_a_fresh_reference_for_a_forking_verb() { + let rendered = render_message( + "claude -p --resume {session} --fork-session --session-id {new_session} \ + \"$(cat {prompt_file})\"", + "3f6c-1111", + Path::new("/run/msg-1.md"), + ); + let new_ref = rendered.new_session_ref.expect("a fresh reference"); + assert_ne!(new_ref, "3f6c-1111"); + assert_eq!(new_ref.len(), 36, "a v4 uuid: {new_ref}"); + assert!( + rendered + .command + .contains(&format!("--session-id '{new_ref}'")), + "{}", + rendered.command + ); + // and a second send forks somewhere else again + let again = render_message( + "claude --session-id {new_session} --resume {session} {prompt_file}", + "3f6c-1111", + Path::new("/run/msg-2.md"), + ); + assert_ne!(again.new_session_ref, Some(new_ref)); + } + + /// The placeholder is bound only where a send happens; on any other verb it + /// would reach the shell as literal braces, so it is refused at load. + #[test] + fn new_session_is_refused_outside_the_message_verb() { + for (verb, template) in [ + ("attach", "join {session} {new_session}"), + ("resume", "reopen {session} {new_session}"), + ("plan", "plan --session-id {new_session} {prompt_file}"), + ] { + let text = format!( + "[agents.a]\ndispatch = \"run {{prompt_file}}\"\n{verb} = \"{template}\"\n" + ); + let e = parse(&text).unwrap_err().to_string(); + assert!(e.contains("{new_session}"), "{verb}: {e}"); + assert!(e.contains(verb), "{verb}: {e}"); + } + // and on the dispatch template itself, which starts a session rather + // than joining one + let e = parse("[agents.a]\ndispatch = \"run --session-id {new_session} {prompt_file}\"\n") + .unwrap_err() + .to_string(); + assert!(e.contains("dispatch carries {new_session}"), "{e}"); + } + + /// The built-in `claude` message verb forks, because a `--bg` session's + /// supervisor refuses a headless resume while it lives (DESIGN.md §8). + #[test] + fn the_builtin_claude_message_verb_forks() { + let message = builtin_agents()["claude"].message().unwrap(); + assert!(message.contains("--fork-session"), "{message}"); + assert!(message.contains(NEW_SESSION_PLACEHOLDER), "{message}"); } /// The one-pass rule (§8): a value carrying its own braces reaches the @@ -1694,7 +1816,7 @@ mod tests { "{prompt_file}", Path::new("/run/m.md"), ); - assert_eq!(rendered, "say '{prompt_file}' '/run/m.md'"); + assert_eq!(rendered.command, "say '{prompt_file}' '/run/m.md'"); } /// A stale `continue` line — from a pre-pivot config or the old codex diff --git a/crates/voro-core/src/lib.rs b/crates/voro-core/src/lib.rs index 44c546f..eb69969 100644 --- a/crates/voro-core/src/lib.rs +++ b/crates/voro-core/src/lib.rs @@ -16,10 +16,11 @@ mod template; mod transition; pub use agent::{ - AgentSessionEntry, AgentTemplate, AgentsConfig, Launch, LaunchSpec, PROMPT_FILE_PLACEHOLDER, - Provenance, ResolvedAgent, SESSION_NAME_PLACEHOLDER, SESSION_PLACEHOLDER, SessionLiveness, - TASK_ID_PLACEHOLDER, VIEWER_BASE_PLACEHOLDER, VIEWER_BRANCH_PLACEHOLDER, - VIEWER_PATH_PLACEHOLDER, ViewerTemplate, parse_sessions_json, render_message, + AgentSessionEntry, AgentTemplate, AgentsConfig, Launch, LaunchSpec, NEW_SESSION_PLACEHOLDER, + PROMPT_FILE_PLACEHOLDER, Provenance, RenderedMessage, ResolvedAgent, SESSION_NAME_PLACEHOLDER, + SESSION_PLACEHOLDER, SessionLiveness, TASK_ID_PLACEHOLDER, VIEWER_BASE_PLACEHOLDER, + VIEWER_BRANCH_PLACEHOLDER, VIEWER_PATH_PLACEHOLDER, ViewerTemplate, parse_sessions_json, + render_message, }; pub use error::{Error, Result}; pub use import::{GithubIssue, already_imported, issue_new_task, issue_task_body}; diff --git a/crates/voro-core/src/store.rs b/crates/voro-core/src/store.rs index f99d0d3..94253b7 100644 --- a/crates/voro-core/src/store.rs +++ b/crates/voro-core/src/store.rs @@ -1241,6 +1241,28 @@ impl Store { self.session(id) } + /// Record what a confirmed headless send did to a session (DESIGN.md §8): + /// the process now carrying the turn, and — where the agent forked rather + /// than resumed in place — the reference the conversation continues under. + /// One statement, so a reconcile in another window never reads the new + /// reference beside the old process or the reverse. + pub fn record_session_send( + &mut self, + id: i64, + session_ref: Option<&str>, + pid: i64, + ) -> Result { + let changed = self.conn.execute( + "UPDATE sessions SET pid = ?1, session_ref = COALESCE(?2, session_ref) + WHERE id = ?3", + params![pid, session_ref, id], + )?; + if changed == 0 { + return Err(Error::SessionNotFound(id)); + } + self.session(id) + } + /// Close a session with its outcome, stamping `ended_at`. pub fn end_session(&mut self, id: i64, outcome: SessionOutcome) -> Result { if set_session_outcome(&self.conn, id, outcome)? == 0 { @@ -3521,6 +3543,35 @@ mod tests { )); } + /// A confirmed send moves the session's process to the one carrying the + /// turn, and follows the fork when the agent opened a new reference — but a + /// send that resumed in place must not blank the reference it already had. + #[test] + fn record_session_send_moves_the_pid_and_follows_a_fork() { + let mut s = Store::open_in_memory().unwrap(); + let task_id = task_fixture(&mut s); + let opened = s + .create_session(task_id, "claude", Some(1234), None) + .unwrap(); + s.set_session_ref(opened.id, "first-ref").unwrap(); + + let resumed = s.record_session_send(opened.id, None, 4321).unwrap(); + assert_eq!(resumed.pid, Some(4321)); + assert_eq!(resumed.session_ref.as_deref(), Some("first-ref")); + + let forked = s + .record_session_send(opened.id, Some("forked-ref"), 5678) + .unwrap(); + assert_eq!(forked.pid, Some(5678)); + assert_eq!(forked.session_ref.as_deref(), Some("forked-ref")); + assert_eq!(s.session(opened.id).unwrap(), forked); + + assert!(matches!( + s.record_session_send(999, None, 1), + Err(Error::SessionNotFound(999)) + )); + } + #[test] fn end_session_rejects_unknown_id() { let mut s = Store::open_in_memory().unwrap(); diff --git a/crates/voro/src/app.rs b/crates/voro/src/app.rs index 567dae8..e8cc73f 100644 --- a/crates/voro/src/app.rs +++ b/crates/voro/src/app.rs @@ -291,6 +291,9 @@ pub struct AttachRequest { /// the line lands in, the template that puts it there, and the listing the /// liveness probe reads to be sure the session is between turns. struct MessageTarget { + /// The session row the send updates once it is confirmed — its process, and + /// its reference where the agent's verb forks (DESIGN.md §8). + session_id: i64, session_ref: String, template: String, sessions_cmd: Option, @@ -1861,6 +1864,7 @@ impl App { return None; }; Some(MessageTarget { + session_id: session.id, session_ref, template: template.to_string(), sessions_cmd: agent.and_then(|a| a.sessions()).map(str::to_string), @@ -1868,11 +1872,15 @@ impl App { } /// Send the collected line into the task's session (DESIGN.md §8). A - /// review or waiting task's message *is* its rejection: the transition runs - /// first, so the feedback is in the body and the event log before anything - /// is said, and a refused transition sends nothing. A `needs-input` task - /// transitions not at all — the answer lives in the transcript and the - /// agent's own `voro resume` moves it back to `running` (DESIGN.md §6). + /// review or waiting task's message *is* its rejection, and the send goes + /// first: a message that never left would otherwise leave the feedback in + /// the body, the task back in `running`, and the agent none the wiser — + /// which is exactly the state a redispatch cannot tell from a stall. So the + /// spawn is confirmed, then the session row and the transition commit + /// together, and a refused send leaves the task precisely where it was. A + /// `needs-input` task transitions not at all — the answer lives in the + /// transcript and the agent's own `voro resume` moves it back to `running` + /// (DESIGN.md §6). fn send_session_message(&mut self, task_id: i64, message: &str) { if message.trim().is_empty() { self.status = Some("a message is required".into()); @@ -1912,20 +1920,6 @@ impl App { } }; let rejected = matches!(state, TaskState::Review | TaskState::Waiting); - if rejected - && let Err(e) = self - .store - .apply(task_id, Action::RejectWork(message.to_string())) - { - self.status = Some(format!("{e} — nothing was sent")); - return; - } - if rejected { - // What the operator just judged, so the re-review can be narrowed to - // the rework (DESIGN.md §8) — off the loop, so the message is sent - // without waiting on `gh`. - self.capture_reviewed(task_id); - } // A rejection reaches the session framed as one: the feedback, plus the // instruction to answer it point by point at `done` (DESIGN.md §8). An // ordinary message is said as written. @@ -1941,13 +1935,47 @@ impl App { cwd, }, ); - self.status = Some(match (sent, rejected) { - (Ok(summary), true) => format!("{summary} — task returned to running"), - (Ok(summary), false) => summary, - (Err(e), true) => format!( - "{e} — the feedback is recorded and the task is running, but nothing was sent" - ), - (Err(e), false) => e, + let sent = match sent { + Ok(sent) => sent, + Err(e) => { + self.status = Some(format!("{e} — task {task_id} is unchanged")); + return; + } + }; + // The send is under way, so the session row follows it — the process + // now carrying the turn, and the reference the agent forked into where + // its verb does that — and the rejection commits behind it. A store + // failure here takes the agent down with it rather than leaving it + // working on feedback no state records. + let pid = sent.pid(); + if let Err(e) = + self.store + .record_session_send(target.session_id, sent.new_session_ref(), pid) + { + sent.abandon(); + self.status = Some(format!( + "recording the send failed ({e}); the spawned agent (pid {pid}) was killed" + )); + return; + } + if rejected { + if let Err(e) = self + .store + .apply(task_id, Action::RejectWork(message.to_string())) + { + sent.abandon(); + self.status = Some(format!("{e}; the spawned agent (pid {pid}) was killed")); + return; + } + // What the operator just judged, so the re-review can be narrowed to + // the rework (DESIGN.md §8) — off the loop, so nothing waits on `gh`. + self.capture_reviewed(task_id); + } + let summary = sent.confirm(&self.dispatch_ctx); + self.status = Some(if rejected { + format!("{summary} — task returned to running") + } else { + summary }); let refreshed = self.refresh(); self.report(refreshed); @@ -3218,6 +3246,7 @@ mod tests { agents_path, runtime_dir: root.join("sessions"), ref_capture_timeout: std::time::Duration::ZERO, + message_grace: std::time::Duration::from_millis(300), }; (store, ctx, project_path) } @@ -3269,6 +3298,7 @@ mod tests { agents_path, runtime_dir: root.join("sessions"), ref_capture_timeout: std::time::Duration::ZERO, + message_grace: std::time::Duration::from_millis(300), }; let project = store .create_project("demo", project_path.to_str().unwrap()) @@ -4507,6 +4537,12 @@ mod tests { /// session verbs the stub agent defines, so a test can take one away. The /// stub lingers after printing its prompt, so a verb-less agent — whose /// liveness is the pid — keeps its task `running` through reconcile. + /// + /// The `message` verb lingers too: a send that exits non-zero inside its + /// grace window is a send that did not happen (task #390), so the stub has + /// to be a command that survives. It says what it is in a trailing comment, + /// which the launch log records verbatim — that is what the assertions + /// below read the rendered `{session}` out of. fn jump_in_env(verbs: &[&str], listing_json: &str) -> JumpIn { let (mut store, ctx, project_path) = scratch_env("jumpin", None); let listing = project_path.parent().unwrap().join("listing.json"); @@ -4515,7 +4551,10 @@ mod tests { ("sessions", format!("cat '{}'", listing.display())), ("attach", "agent attach {session}".into()), ("resume", "agent resume {session}".into()), - ("message", "agent message {session} {prompt_file}".into()), + ( + "message", + "sleep 30 # agent message {session} {prompt_file}".into(), + ), ] .into_iter() .filter(|(verb, _)| verbs.contains(verb)) @@ -4924,7 +4963,8 @@ mod tests { /// A `needs-input` task transitions not at all — per DESIGN.md §6 the /// answer lives in the transcript, and the agent's own `voro resume` moves - /// the task back to `running`. + /// the task back to `running`. Its session row still follows the send, so + /// the answer's process is what the reconciler reads. #[test] fn message_on_a_needs_input_task_sends_without_transitioning() { let mut env = jump_in_env(&["attach", "resume", "message"], FINISHED_LISTING); @@ -4941,6 +4981,107 @@ mod tests { assert_eq!(task.state, TaskState::NeedsInput); assert!(!task.body.contains("voro-core"), "{}", task.body); assert!(launches(&root).contains("agent message 'ref-1'")); + let session = app.store.sessions_for(task_id).unwrap().remove(0); + assert!(crate::session_probe::pid_is_alive(session.pid.unwrap())); + + let _ = std::fs::remove_dir_all(&root); + } + + /// Rewrite the stub agent's `message` verb — loaded fresh on every send, so + /// a test can change what a send *does* after the environment is built. + fn set_message_verb(env: &JumpIn, template: &str) { + let config = std::fs::read_to_string(&env.ctx.agents_path).unwrap(); + let rewritten = config + .lines() + .map(|line| match line.starts_with("message = ") { + true => format!("message = \"{template}\"\n"), + false => format!("{line}\n"), + }) + .collect::(); + std::fs::write(&env.ctx.agents_path, rewritten).unwrap(); + } + + /// The defect this ordering exists for (task #390): the send is what the + /// rejection hangs off, so a message the agent refuses — a supervisor-held + /// session, a stale reference — leaves the task in `review` with its body + /// untouched and the refusal on the status line. Recording feedback the + /// agent never received, and returning the task to `running` on the + /// strength of it, is the one outcome worse than not sending. + #[test] + fn a_refused_send_leaves_the_review_task_exactly_where_it_was() { + let mut env = jump_in_env(all_verbs(), FINISHED_LISTING); + env.store + .apply(env.task_id, Action::Complete(None)) + .unwrap(); + set_message_verb( + &env, + "printf 'Session is currently running as a background agent' >&2; \ + exit 1 # {session} {prompt_file}", + ); + let (project_path, task_id) = (env.project_path.clone(), env.task_id); + let root = project_path.parent().unwrap().to_path_buf(); + + let mut app = App::new(env.store, env.ctx).unwrap(); + app.toggle_screen(); + send_message(&mut app, "the tests are missing"); + + let task = app.store.task(task_id).unwrap(); + assert_eq!(task.state, TaskState::Review); + assert!(!task.body.contains("Feedback"), "{}", task.body); + assert!( + !task.body.contains("the tests are missing"), + "{}", + task.body + ); + assert!( + !app.store + .events_for(task_id) + .unwrap() + .iter() + .any(|e| e.kind == "feedback"), + "no rejection is logged for a message that never landed" + ); + // the agent's own account of the refusal, out of the log and onto the + // status line + let status = app.status.as_deref().unwrap_or("").to_string(); + assert!(status.contains("background agent"), "{status}"); + assert!(status.contains("unchanged"), "{status}"); + + let _ = std::fs::remove_dir_all(&root); + } + + /// A verb that forks rather than resuming in place: the session row follows + /// the fork, so the next message and the next jump-in address the + /// conversation where it actually continued — and the rejection lands as + /// usual behind the confirmed send. + #[test] + fn a_forking_send_moves_the_session_to_the_reference_it_opened() { + let mut env = jump_in_env(all_verbs(), FINISHED_LISTING); + env.store + .apply(env.task_id, Action::Complete(None)) + .unwrap(); + set_message_verb( + &env, + "sleep 30 # agent message {session} --session-id {new_session} {prompt_file}", + ); + let (project_path, task_id) = (env.project_path.clone(), env.task_id); + let root = project_path.parent().unwrap().to_path_buf(); + + let mut app = App::new(env.store, env.ctx).unwrap(); + app.toggle_screen(); + send_message(&mut app, "the tests are missing"); + + let task = app.store.task(task_id).unwrap(); + assert_eq!(task.state, TaskState::Running); + assert!(task.body.contains("the tests are missing"), "{}", task.body); + let session = app.store.sessions_for(task_id).unwrap().remove(0); + let new_ref = session.session_ref.expect("a reference"); + assert_ne!(new_ref, "ref-1", "the row followed the fork"); + assert!(crate::session_probe::pid_is_alive(session.pid.unwrap())); + assert!( + launches(&root).contains(&format!("--session-id '{new_ref}'")), + "the recorded reference is the one the command was given" + ); let _ = std::fs::remove_dir_all(&root); } @@ -4976,10 +5117,11 @@ mod tests { } /// The refusal's other half (task #376): a pid-less `blocked` zombie is - /// not a session still running, so the message goes headlessly — the - /// rejection lands first, then the send. The reconcile that follows finds - /// the same zombie and stalls the task, exactly as a `done` entry does - /// below, with the feedback already in the body for the redispatch. + /// not a session still running, so the message goes headlessly — and the + /// send that lands is what the task then rides on. The reconcile that + /// follows finds the same zombie in the listing but the send's own process + /// on the row, so the task stays `running` rather than being stalled out + /// from under the agent now answering (task #390). #[test] fn message_sends_into_a_zombie_session() { let mut env = jump_in_env(all_verbs(), ZOMBIE_LISTING); @@ -4995,7 +5137,7 @@ mod tests { let task = app.store.task(task_id).unwrap(); assert!(task.body.contains("the tests are missing"), "{}", task.body); - assert_eq!(task.state, TaskState::Stalled); + assert_eq!(task.state, TaskState::Running); assert!(launches(&root).contains("agent message 'ref-1'")); let _ = std::fs::remove_dir_all(&root); @@ -5030,14 +5172,14 @@ mod tests { let _ = std::fs::remove_dir_all(&root); } - /// The reject edge's documented tail (DESIGN.md §8), inherited unchanged: - /// when the agent's listing still reports the session finished, the - /// reconcile that follows the transition stalls the task. The feedback is - /// in the body either way, so the redispatch that stalling offers carries - /// it — and the headless send that did land reports back on the stalled - /// session's behalf if it gets there first. + /// The reject edge's tail, corrected (task #390): a session the listing + /// reports finished used to be stalled by the very next reconcile, seconds + /// after the rejection was sent into it — the operator's feedback recorded, + /// the task queued for redispatch, and the agent that received it ignored. + /// The send's own process is now on the row, so the task rides `running` + /// for as long as the turn takes. #[test] - fn message_to_a_finished_session_leaves_the_reject_edge_to_reconcile() { + fn message_to_a_finished_session_keeps_the_task_running() { let mut env = jump_in_env(all_verbs(), FINISHED_LISTING); env.store .apply(env.task_id, Action::Complete(None)) @@ -5050,9 +5192,15 @@ mod tests { send_message(&mut app, "the tests are missing"); let task = app.store.task(task_id).unwrap(); - assert_eq!(task.state, TaskState::Stalled); + assert_eq!(task.state, TaskState::Running); assert!(task.body.contains("the tests are missing"), "{}", task.body); assert!(launches(&root).contains("agent message 'ref-1'")); + let session = app.store.sessions_for(task_id).unwrap().remove(0); + assert!(session.ended_at.is_none()); + assert!( + crate::session_probe::pid_is_alive(session.pid.unwrap()), + "the row carries the send's own process, not the dispatch launcher" + ); let _ = std::fs::remove_dir_all(&root); } @@ -5551,6 +5699,7 @@ mod tests { agents_path, runtime_dir: dir.join("sessions"), ref_capture_timeout: std::time::Duration::ZERO, + message_grace: std::time::Duration::from_millis(300), }; app } diff --git a/crates/voro/src/cli.rs b/crates/voro/src/cli.rs index 0d4bb87..b348571 100644 --- a/crates/voro/src/cli.rs +++ b/crates/voro/src/cli.rs @@ -2451,6 +2451,7 @@ mod tests { agents_path: agents_path.clone(), runtime_dir: dir.join("sessions"), ref_capture_timeout: std::time::Duration::ZERO, + message_grace: std::time::Duration::from_millis(300), }; let mut s = store(); let call = |s: &mut Store, args: &[&str]| { @@ -2496,6 +2497,7 @@ mod tests { agents_path, runtime_dir: dir.join("sessions"), ref_capture_timeout: std::time::Duration::ZERO, + message_grace: std::time::Duration::from_millis(300), }; let mut s = store(); let call = |s: &mut Store, args: &[&str]| { @@ -4019,6 +4021,7 @@ mod tests { agents_path, runtime_dir: root.join("sessions"), ref_capture_timeout: std::time::Duration::ZERO, + message_grace: std::time::Duration::from_millis(300), } } @@ -4491,6 +4494,7 @@ mod tests { agents_path, runtime_dir: root.join("sessions"), ref_capture_timeout: std::time::Duration::ZERO, + message_grace: std::time::Duration::from_millis(300), }; ok( &mut store, @@ -4570,6 +4574,7 @@ mod tests { agents_path, runtime_dir: root.join("sessions"), ref_capture_timeout: std::time::Duration::ZERO, + message_grace: std::time::Duration::from_millis(300), }; (store, ctx, project) } diff --git a/crates/voro/src/dispatch.rs b/crates/voro/src/dispatch.rs index 8042c0c..3f0c00b 100644 --- a/crates/voro/src/dispatch.rs +++ b/crates/voro/src/dispatch.rs @@ -11,7 +11,7 @@ //! log — and records it on the session row for later attach/resume. use std::fs::{File, OpenOptions}; -use std::io::Write; +use std::io::{Read, Seek, SeekFrom, Write}; use std::os::unix::process::CommandExt; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Stdio}; @@ -879,11 +879,20 @@ pub fn refine( )) } +/// How long [`send_message`] watches a spawned send before calling it started. +const MESSAGE_GRACE: Duration = Duration::from_secs(2); + +/// How often that window is polled. +const MESSAGE_POLL_INTERVAL: Duration = Duration::from_millis(25); + +/// How much of a failed send's log to look at for the line to quote back. +const LOG_TAIL_BYTES: u64 = 4096; + /// One line said into a session that already exists (DESIGN.md §8), assembled by /// the TUI's quick-message key. Unlike an [`Expansion`] this opens no session -/// row and tracks no pid: it joins a conversation Voro already knows about -/// rather than starting one, so there is no new identity to compose and nothing -/// for the reconciler to observe. +/// row: it joins a conversation Voro already knows about rather than starting +/// one, so there is no new identity to compose — it updates the row that +/// conversation already has (DESIGN.md §8). pub struct SessionMessage<'a> { /// The task whose session is being messaged — names the prompt and log /// files, and the launch-log line. @@ -899,32 +908,143 @@ pub struct SessionMessage<'a> { pub cwd: String, } -/// Fire a message into a task's agent session and return, leaving it to run -/// (DESIGN.md §8). Fire-and-forget by design: the send is one turn appended to a -/// transcript the operator watches elsewhere, so an agent-side refusal lands in -/// the log rather than back in the UI, and nothing here waits for a reply. -pub fn send_message(ctx: &DispatchCtx, msg: SessionMessage) -> Result { +/// A quick message whose send is under way: the spawn survived its grace window +/// ([`send_message`]), so the caller may now record what it changed about the +/// session and — for a rejection — apply the transition the message carries. +/// The child is still held, so a caller whose recording fails can take the +/// agent down with it ([`abandon`](Self::abandon)) rather than leaving it acting +/// on a message no state reflects. +pub struct SentMessage { + spawned: Spawned, + task_id: i64, + new_session_ref: Option, +} + +impl SentMessage { + /// The process carrying the turn. Unlike a dispatch's launcher pid this is + /// the send itself, so it is alive for as long as the agent is answering — + /// which is what keeps the reconciler off a task whose forked turn does not + /// appear in the agent's own listing (DESIGN.md §8). + pub fn pid(&self) -> i64 { + self.spawned.pid + } + + /// The reference the session answers to from now on, when the agent's verb + /// forked rather than resuming in place. + pub fn new_session_ref(&self) -> Option<&str> { + self.new_session_ref.as_deref() + } + + /// Let the send run: hand the child to the reaper and report it. From here + /// the turn is fire-and-forget — it is appended to a transcript the operator + /// watches elsewhere, so an agent-side refusal after this point lands in the + /// log rather than back in the UI. + pub fn confirm(self, ctx: &DispatchCtx) -> String { + let summary = format!( + "message sent to task {}'s session — log {}", + self.task_id, + self.spawned.log_path.display() + ); + reap_expansion(self.spawned, ctx.launch_log_path()); + summary + } + + /// Kill the send's process group, for a caller whose store write failed + /// after the spawn: an agent must not go on acting on a message the + /// database does not record. + pub fn abandon(self) { + kill_expansion(self.spawned); + } +} + +/// Fire a message into a task's agent session and wait just long enough to know +/// it started (DESIGN.md §8). A headless send can be refused outright — a +/// supervisor-held session, a stale reference — and that refusal exits in well +/// under a second, so the spawn is given a grace window to fail in and an early +/// non-zero exit is reported as a send that did not happen. What comes back is +/// a [`SentMessage`] the caller records against the session before letting it +/// run; the transition a rejection carries hangs off that, so nothing is +/// committed against a message that never left. +pub fn send_message(ctx: &DispatchCtx, msg: SessionMessage) -> Result { if msg.message.trim().is_empty() { return Err("a message is required".into()); } let label = format!("message-{}", msg.task_id); let prompt_path = write_prompt(ctx, &label, msg.message)?; - let command = voro_core::render_message(msg.template, msg.session_ref, &prompt_path); - let spawned = spawn_logged( + let rendered = voro_core::render_message(msg.template, msg.session_ref, &prompt_path); + let mut spawned = spawn_logged( ctx, label, &prompt_path, - &command, + &rendered.command, &msg.cwd, msg.session_ref.to_string(), )?; - let summary = format!( - "message sent to task {}'s session — log {}", - msg.task_id, - spawned.log_path.display() - ); - reap_expansion(spawned, ctx.launch_log_path()); - Ok(summary) + // An exit inside the window is only a failure if it failed: a verb that + // says its piece and returns cleanly has delivered the message. + if let Some(status) = wait_for_early_exit(&mut spawned.child, ctx.message_grace) + && !status.success() + { + append_launch_log( + &ctx.launch_log_path(), + &format!("{}: exited with {status}", spawned.label), + ); + return Err(format!( + "the message was refused by '{}' ({status}){}", + msg.template + .split_whitespace() + .next() + .unwrap_or("the agent"), + log_tail_note(&spawned.log_path) + )); + } + Ok(SentMessage { + spawned, + task_id: msg.task_id, + new_session_ref: rendered.new_session_ref, + }) +} + +/// Poll a freshly spawned child for up to `grace`, returning its status if it +/// exits in that window and `None` if it is still going (or could not be +/// waited on, which is not evidence either way). A zero grace is a single look. +fn wait_for_early_exit(child: &mut Child, grace: Duration) -> Option { + let deadline = Instant::now() + grace; + loop { + match child.try_wait() { + Ok(Some(status)) => return Some(status), + Ok(None) => {} + Err(_) => return None, + } + if Instant::now() >= deadline { + return None; + } + std::thread::sleep(MESSAGE_POLL_INTERVAL.min(grace)); + } +} + +/// The last line a failed send wrote, for the status line — the agent's own +/// account of why it refused, which is otherwise only in the log file. Empty +/// when there is nothing to quote. +fn log_tail_note(path: &Path) -> String { + let Ok(mut file) = File::open(path) else { + return String::new(); + }; + let len = file.metadata().map(|m| m.len()).unwrap_or(0); + if file + .seek(SeekFrom::Start(len.saturating_sub(LOG_TAIL_BYTES))) + .is_err() + { + return String::new(); + } + let mut tail = String::new(); + if file.read_to_string(&mut tail).is_err() { + return String::new(); + } + match tail.lines().rev().find(|l| !l.trim().is_empty()) { + Some(line) => format!(": {}", line.trim().chars().take(160).collect::()), + None => String::new(), + } } /// Where dispatch finds its inputs and puts its artefacts. Built from the @@ -942,6 +1062,11 @@ pub struct DispatchCtx { /// agent that defines a `sessions` verb, before giving up (the ref stays /// NULL and the summary says so). Zero means a single attempt. pub ref_capture_timeout: Duration, + /// How long a quick message's process gets to prove it started before the + /// send is treated as having landed (DESIGN.md §8). Long enough for a + /// refusal — which is immediate — to be caught, short enough that the TUI + /// does not visibly stall. Zero means a single look. + pub message_grace: Duration, } impl DispatchCtx { @@ -958,6 +1083,7 @@ impl DispatchCtx { agents_path: AgentsConfig::default_path(), runtime_dir, ref_capture_timeout: Duration::from_secs(5), + message_grace: MESSAGE_GRACE, } } @@ -1516,6 +1642,7 @@ mod tests { agents_path, runtime_dir: root.join("sessions"), ref_capture_timeout: Duration::ZERO, + message_grace: std::time::Duration::from_millis(300), }; (store, ctx, project) } @@ -2098,6 +2225,7 @@ mod tests { // give the stub time to write the line before capture's single poll let ctx = DispatchCtx { ref_capture_timeout: Duration::from_secs(3), + message_grace: std::time::Duration::from_millis(300), ..ctx }; let summary = dispatch(&mut store, &ctx, id, None).unwrap(); diff --git a/crates/voro/src/reconcile.rs b/crates/voro/src/reconcile.rs index a15b869..55b3509 100644 --- a/crates/voro/src/reconcile.rs +++ b/crates/voro/src/reconcile.rs @@ -25,6 +25,14 @@ //! from reading them all as live. Agents without a `sessions` verb keep the //! spawned-pid check. //! +//! The row's own pid is still read in one direction, for every agent: a pid +//! that is *alive* proves the session is (task #390). A quick message replaces +//! that pid with the process carrying its turn, and where the agent had to fork +//! to be joined at all, that turn is a `-p` run the agent's listing never shows +//! — so the listing would report the session gone while the message it was just +//! sent is still being worked on. A dead pid still proves nothing and falls +//! back to the listing. +//! //! A refine round reads the same way, because it is launched the same way: the //! headless flavour renders the agent's own `dispatch` template, so under a //! `--bg` launcher its recorded pid lies exactly as a dispatch's does, and its @@ -108,6 +116,16 @@ pub fn reconcile_live_sessions(store: &mut Store, agents_path: &Path) -> Result< // recorded means liveness can't be checked. None => session.pid.map(pid_is_alive), }; + // A recorded process that is still there proves the session is live + // whatever the listing says: a quick message forks a `-p` turn that + // never appears in `claude agents`, so listing-absence alone must not + // finalise the session under it (DESIGN.md §8). The check is + // directional — a dead pid proves nothing, since a dispatch's pid is a + // launcher that exits at birth, and falls back to the listing verdict. + let alive = match alive { + Some(false) if session.pid.is_some_and(pid_is_alive) => Some(true), + verdict => verdict, + }; let Some(alive) = alive else { continue }; if alive { continue; @@ -441,8 +459,11 @@ mod tests { ] { let (agents_path, dir) = sessions_fixture(name, listing); let (mut s, task_id) = running_task(); + // The launcher pid of a `--bg` dispatch, dead as it always is by + // now, so the listing is what decides (a *live* pid would override + // it — see `a_live_pid_outlives_its_absence_from_the_listing`). let session = s - .create_session(task_id, "claude", Some(std::process::id() as i64), None) + .create_session(task_id, "claude", Some(dead_pid()), None) .unwrap(); s.set_session_ref(session.id, "full-uuid-1").unwrap(); @@ -475,7 +496,7 @@ mod tests { ); let (mut s, task_id) = running_task(); let session = s - .create_session(task_id, "claude", Some(std::process::id() as i64), None) + .create_session(task_id, "claude", Some(dead_pid()), None) .unwrap(); s.set_session_ref(session.id, "full-uuid-1").unwrap(); @@ -513,6 +534,26 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// The quick-message case (task #390): a forked `-p` turn does not appear + /// in the agent's listing at all, so the ref the session now carries reads + /// as gone — while the process answering the message is right there on the + /// row. A live pid outranks the listing, or the send the operator just made + /// would stall the task under the agent working on it. + #[test] + fn a_live_pid_outlives_its_absence_from_the_listing() { + let (agents_path, dir) = sessions_fixture("message-fork", "[]"); + let (mut s, task_id) = running_task(); + let session = s.create_session(task_id, "claude", None, None).unwrap(); + s.record_session_send(session.id, Some("forked-uuid"), std::process::id() as i64) + .unwrap(); + + assert_eq!(reconcile_live_sessions(&mut s, &agents_path).unwrap(), 0); + assert!(s.session(session.id).unwrap().ended_at.is_none()); + assert_eq!(s.task(task_id).unwrap().state, TaskState::Running); + + let _ = std::fs::remove_dir_all(&dir); + } + /// With a `sessions` verb configured but no captured ref, liveness is /// unknowable: the session is left alone (pid-checking a supervisor-owned /// launch would wrongly flag it), matching the no-pid case above. @@ -593,7 +634,7 @@ mod tests { #[test] fn a_refine_round_gone_from_the_listing_is_marked_failed() { let (agents_path, dir) = sessions_fixture("refine-gone", "[]"); - let (mut s, task_id, session_id) = refining_task("claude", Some(std::process::id() as i64)); + let (mut s, task_id, session_id) = refining_task("claude", Some(dead_pid())); s.set_session_ref(session_id, "refine-uuid").unwrap(); assert_eq!(reconcile_live_sessions(&mut s, &agents_path).unwrap(), 1); diff --git a/crates/voro/src/ui.rs b/crates/voro/src/ui.rs index d1560df..d494db5 100644 --- a/crates/voro/src/ui.rs +++ b/crates/voro/src/ui.rs @@ -2130,6 +2130,7 @@ mod tests { agents_path, runtime_dir: dir.join("sessions"), ref_capture_timeout: std::time::Duration::ZERO, + message_grace: std::time::Duration::from_millis(300), }; let mut app = App::new(store, ctx).unwrap(); app.on_key(KeyEvent::from(KeyCode::Char('4'))); @@ -4317,6 +4318,7 @@ mod tests { agents_path, runtime_dir: dir.join("sessions"), ref_capture_timeout: std::time::Duration::ZERO, + message_grace: std::time::Duration::from_millis(300), }; let mut app = crate::app::App::new(store, ctx).unwrap(); let mut terminal = TestTerminal::new(ratatui::backend::TestBackend::new(100, 24)).unwrap(); diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 35bce45..6fd8f92 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -223,7 +223,7 @@ A **human task** (§3) walks a shortened path through the same machine rather th `waiting` is the state for work in flight on *someone else's* move. Once the operator has run `pr` and the PR is up awaiting another person's review or merge, there is nothing the operator can do, yet `review` is an attention state that would keep the task occupying a queue row (with a state bonus, §7) indefinitely. `waiting` says "in flight, but not my move": it earns no state bonus and is excluded from the queue entirely, like `parked`. It derives no next-action verb (§3). Being out of the queue is not the same as being out of sight, though, and it was originally both: a handed-off task surfaced only in the task browser and the state counts, so merged PRs sat unaccepted for days and work gated behind a `blocks` edge stayed gated with nothing saying so — `waiting` earns no score, so not even the `unblock_bonus` (§7) could lift it back into view. It therefore rides the cockpit's running strip (§9), which is where the operator already reads what is in flight: to them a handed-off task is the same fact as a dispatched one — something else owns the work — and the strip filters on state rather than on sessions, so carrying it costs no new machinery. The row is badged with what the hand-off is holding up (`blocks N`, counted by the `unblock_bonus` rule) and with whether a PR tracks it, and its elapsed time counts from the hand-off rather than from the session underneath, which opened when the agent started work and says nothing about how long the PR has been sitting there. It is reached only from `review`, via the *hand off* transition (`voro wait`), and leaves by four manual moves: *accept* (the PR merged) → `done`, *reject with feedback* (changes requested) → `running` — reusing the review→running feedback path, which keeps the same agent session open (§8) — *reclaim* (it is the operator's move again) → `review`, and *abandon* → `rejected`. Entering `waiting` only from `review` is deliberate: waiting on a person *before* work starts is what `parked` plus a blocker already expresses, so the more general "blocked on an external party at any point" state is deferred until a concrete need for it appears. Return-path automation — a reconcile that polls `gh pr view` and pulls a merged PR to `done` or a change-requested one back to `review` — is likewise deferred; today every exit is a manual operator move. -**Every message to a review or waiting task is a rejection.** The `review → running` feedback edge is the only way a sentence from the operator reaches work already reported done, so the cockpit's quick message (§8) routes through it rather than beside it: the `RejectWork` transition is applied *before* the send, appending the feedback to the body under `## Feedback` and logging the `feedback` event, and a refused transition sends nothing. There is deliberately no "just asking" mode — a second channel that said something to the agent without recording it would put the task's body and its session out of step, which is precisely the drift the return-path verbs exist to prevent. A `needs-input` task is the mirror and transitions not at all: the answer belongs to the transcript, and the agent's own `voro resume` is what moves the task back to `running`. +**Every message to a review or waiting task is a rejection.** The `review → running` feedback edge is the only way a sentence from the operator reaches work already reported done, so the cockpit's quick message (§8) routes through it rather than beside it: a send confirmed to have started is followed by the `RejectWork` transition, appending the feedback to the body under `## Feedback` and logging the `feedback` event, and a send the agent refuses transitions nothing (§8). There is deliberately no "just asking" mode — a second channel that said something to the agent without recording it would put the task's body and its session out of step, which is precisely the drift the return-path verbs exist to prevent. A `needs-input` task is the mirror and transitions not at all: the answer belongs to the transcript, and the agent's own `voro resume` is what moves the task back to `running`. ## 7. Scoring @@ -275,7 +275,11 @@ Cheap actions need one further guard, or the pricing swaps one swamping for anot **Answering a question happens in the session, not through Voro.** When an agent hits a blocker it calls `voro ask`, landing the task in `needs-input` with its `question` set. Voro's job from there is to be an accurate *signpost* — which task is blocked, on what question, surfaced on the inbox row and in the detail pane — not the door into the conversation. The operator opens the agent's own session directly (the `voro-` session it runs under, in a `claude agents` pane or the equivalent) and answers there, where the agent still has its full context; Voro records no answer text, since the exchange lives in the session transcript, addressable through the session ref already captured on the session row. Once answered, `voro resume ` moves the task `needs-input → running` and nothing more: the dispatch preamble tells the agent to run it in-session after its question is answered (the primary path), and Enter on a `needs-input` inbox row is the operator's backstop for the same transition. This replaces an earlier headless-continuation design — a fresh session re-sent the whole task body carrying the appended answer — whose only reliable path for the built-in `claude`, which has no headless *continue* verb, was to *restart* the task rather than resume the conversation; a human already watching the session is a strictly better place to answer than a text box in Voro. `voro reject` is the symmetric move on the `review → running` (and `waiting → running`) edge: it appends the feedback to the task body under a `## Feedback` heading and returns the task to `running`. Because `review`/`waiting` keep the session open (see *Session lifecycle* below), the feedback lands back on the *same* agent session when its process is still alive — the operator having stayed attached — and otherwise the task stalls on the next reconcile and is redispatched with the feedback now in its body (the redispatch prompt carries it). Both `resume` and `reject` route through the transition API and change only state, so neither can smuggle a task into `running` outside the machine; a task never dispatched still answers or rejects as a plain transition, since nothing about them depends on a prior session. -**Steering a session without entering it** is the cheap half of that door. Answering in the session is right, but suspending the whole cockpit for a full attach round-trip to say one sentence is not, so the agent verb set gains an optional `message`: a *headless* send carrying both `{session}` and `{prompt_file}`, which appends one turn to an existing session's transcript and returns without owning the terminal. It is the only session verb Voro backgrounds, and the only one that is fire-and-forget — Voro reads no reply, so an agent-side refusal lands in the launch log rather than in the UI, and the exchange is simply there on any later attach. The built-in `claude` spells it as its `resume` plus `-p`; the near-duplication between verb bodies is accepted rather than factored, because a verb is an opaque per-agent contract, and that opacity is exactly what lets an agent define a subset of the verbs and degrade one at a time. It applies to the three states whose session is open and between turns — `needs-input`, `review`, `waiting` — and is refused on the rest: `running` and `refining` are mid-turn with no injection channel, and `stalled` has a dead session, where a headless resume would restart the work with no tracked pid and no session row, invisible to the reconciler. Redispatch is the honest path there. A liveness probe refuses a session still running for the same reason the state gate does, and the send opens no session row and tracks no pid: it joins a conversation Voro already knows about rather than starting one. In the cockpit this is `a`, with the interactive jump-in moving to `A` — the lowercase-quick, uppercase-interactive pairing the `r`/`R` refine keys already use (§9). +**Steering a session without entering it** is the cheap half of that door. Answering in the session is right, but suspending the whole cockpit for a full attach round-trip to say one sentence is not, so the agent verb set gains an optional `message`: a *headless* send carrying both `{session}` and `{prompt_file}`, which appends one turn to an existing session's transcript and returns without owning the terminal. It is the only session verb Voro backgrounds, and — once its delivery is confirmed, below — the only one that is fire-and-forget: Voro reads no reply, so what the agent says afterwards lands in the launch log rather than in the UI, and the exchange is simply there on any later attach. The built-in `claude` spells it as its `resume` plus `-p`, forked; the near-duplication between verb bodies is accepted rather than factored, because a verb is an opaque per-agent contract, and that opacity is exactly what lets an agent define a subset of the verbs and degrade one at a time. It applies to the three states whose session is open and between turns — `needs-input`, `review`, `waiting` — and is refused on the rest: `running` and `refining` are mid-turn with no injection channel, and `stalled` has a dead session, where a headless resume would restart the work with no tracked pid and no session row, invisible to the reconciler. Redispatch is the honest path there. A liveness probe refuses a session still running for the same reason the state gate does, and the send opens no session row: it joins a conversation Voro already knows about rather than starting one. In the cockpit this is `a`, with the interactive jump-in moving to `A` — the lowercase-quick, uppercase-interactive pairing the `r`/`R` refine keys already use (§9). + +**A send that is refused must change nothing, so delivery is confirmed before the rejection commits.** A headless send can be refused outright, and the case that matters is not exotic: a `claude --bg` session that has finished its turn is still owned by a live supervisor process, and that supervisor refuses `--resume` for as long as it lives. Fire-and-forget hid it — the refusal exited in under a second into the launch log while the transition had already appended the feedback and returned the task to `running`, leaving a task nobody was working on, a body claiming otherwise, and a reconcile pass that stalled it for redispatch a moment later. So the ordering is inverted: the send is spawned first and watched for a short grace window, an early non-zero exit is reported as a message that did not happen — with the agent's own last log line quoted on the status line — and the task stays exactly where it was, feedback unwritten. Only a send still running past the window is followed by the session-row update and the `RejectWork` transition, together, so no other window's reconcile reads one without the other. A clean exit inside the window is a delivery, not a failure: a verb that says its piece and returns has done its job. This trades a lost transition for a lost send in the rare case where the store write fails after the spawn — and that case takes the agent down with it (the process group is killed) rather than leaving it working on feedback nothing records. + +**Where a session cannot be resumed, it is forked, and the session reference follows the fork.** The `message` template may carry a third, optional placeholder, `{new_session}`, which Voro binds to a freshly generated v4 UUID: an agent declares by using it that its sessions are joined by forking rather than resumed in place, and the built-in `claude` message verb does exactly that (`--fork-session --session-id {new_session}`), because forking is the one scriptable channel into a supervisor-held session. The fork continues the same conversation under a reference the caller names up front, which is what makes it usable here — Voro records that reference on the session row once the send is confirmed, so the next message, the next jump-in, and the reconciler all address the conversation where it actually continued. A verb without the placeholder resumes in place and keeps the reference it had, so the headless-resume agents are unaffected. One consequence reaches reconciliation: a forked `-p` turn does not appear in the agent's own session listing at all, so the listing would report the session gone while the message it was just sent is still being answered. The row's recorded pid settles it, in one direction only — a *live* pid proves the session is live whatever the listing says, since the quick message replaces that pid with the process carrying its turn, while a dead pid still proves nothing (a dispatch's pid is a launcher that exits at birth) and falls back to the listing verdict. **Session lifecycle.** A session's life follows the *task*, not the agent's process listing. An open session therefore no longer implies the task is *executing*: a refine round (§6) opens one too, in the same transaction as `proposed → refining`, and closes it on the transition back — `completed` when the rewritten body landed, `failed` when the agent died, `aborted` when the round was quit or cancelled. What a session means is "an agent Voro launched is working on this task", and which kind of work it is comes from the task's state, which is why every session-consuming query reads that state rather than the session's existence: the running strip lists `running` and `refining` (§9), reconciliation probes those two and leaves the rest alone, and dispatch's preconditions never look at sessions at all. The dispatch half of that life is unchanged: a session is opened at dispatch (in the same transaction as `ready → running`), stays open across `running → needs-input → review` — `needs-input` keeps it open so the operator answers in that same session, and `review` keeps it open so a reject-with-feedback returns the work to it — and is closed by the terminal transition that tears the running work down, stamped with the matching outcome in the same transaction: `Accept` closes it `completed`, `Abort` and `Abandon` close it `aborted`. `Resume` and `RejectWork` deliberately leave it open (the task returns to `running` on the session it already had). `waiting` (§6) behaves exactly as `review` here: the hand-off keeps the session open, so a change-requested `RejectWork` from `waiting` returns to the same agent session, and `Accept`/`Abandon` from `waiting` close it (`completed`/`aborted`) precisely as they do from `review`. Reconciliation therefore leaves a `waiting` task's open session untouched regardless of process liveness, the same treatment it gives `needs-input` and `review`. A task holds **at most one open session** as an invariant: opening a redispatch first closes any predecessor still open in the same transaction, enforced by a partial unique index on `sessions(task_id) WHERE ended_at IS NULL`. Rows stay one-per-attempt — each keeps its own pid, log, and outcome, and the redispatch flag still derives from the latest one — but two can never be open at once, so a task can never render twice in the running strip. diff --git a/docs/agent-integration.md b/docs/agent-integration.md index 0e1e22f..e7cb4ab 100644 --- a/docs/agent-integration.md +++ b/docs/agent-integration.md @@ -77,7 +77,7 @@ dispatch = "claude --bg --name \"{session_name}\" --permission-mode auto --mod sessions = "claude agents --json" attach = "claude attach {session}" resume = "claude --resume {session}" -message = "claude -p --resume {session} \"$(cat {prompt_file})\"" +message = "claude -p --resume {session} --fork-session --session-id {new_session} \"$(cat {prompt_file})\"" plan = "claude --name \"{session_name}\" --permission-mode auto --model {model} \"$(cat {prompt_file})\"" model = "opus" model_deep = "fable" @@ -118,13 +118,29 @@ resume = "codex resume {session}" - `message` says one thing into a session *headlessly*: it takes both `{session}` and `{prompt_file}`, appends the file's contents to that session's transcript as the next turn, and returns without owning the terminal. It is - the only session verb Voro backgrounds, and it is fire-and-forget — Voro reads - no reply, so an agent-side refusal lands in the launch's log rather than in - the UI. The built-in above is `resume` plus `-p`, and that near-duplication is - deliberate: a verb is an opaque per-agent contract, which is what lets an - agent define a subset of the verbs and degrade one at a time. `codex` names no - `message`, so the quick-message key reports that on the status line and the - jump-in still works. + the only session verb Voro backgrounds. Voro watches the spawned command for + about two seconds and treats an exit with a non-zero status in that window as + a message that was never delivered: nothing is transitioned, the task stays + where it was, and the command's last log line is quoted on the status line. A + send still running past the window — or one that finished cleanly inside it — + has landed, and from there it is fire-and-forget: Voro reads no reply, so + anything the agent says afterwards is in the launch's log rather than the UI. + A rejection's `review → running` transition (DESIGN.md §6) hangs off that + confirmation, so feedback is never recorded against a message the agent + refused. `codex` names no `message`, so the quick-message key reports that on + the status line and the jump-in still works. +- `message` may also carry `{new_session}`, replaced with a fresh v4 UUID Voro + generates for the send. Use it when your agent's sessions cannot be resumed + headlessly but can be *forked*: the built-in `claude` message verb is + `--resume` plus `--fork-session --session-id {new_session}`, because a + `claude --bg` session keeps a supervisor process after finishing its turn and + that supervisor refuses a plain `--resume` while it lives. The fork continues + the same conversation under the reference Voro named, and Voro records that + reference on the session row once the send is confirmed — so later messages, + the jump-in keys, and reconciliation all follow the conversation to where it + continued. A `message` template without the placeholder resumes in place and + keeps the reference it had. `{new_session}` is refused on every other verb: + it names the session a send opens, and nothing else opens one. - `plan` runs an interactive *foreground* session for the TUI's agent-assisted 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 @@ -192,7 +208,11 @@ leaves dead sessions at `blocked` forever) would otherwise read as a fleet of running agents. A session that drops out, finishes, or zombies there without calling `voro done`/`ask` stalls its task, exactly as pid-death does for plain agents (DESIGN.md §8). When liveness is unknowable (no ref, listing failed) the -session is left alone. +session is left alone. The row's own pid is still read in one direction, for +every agent: a pid that is *alive* proves the session is, whatever the listing +says. That is what a quick message leaves behind — the process carrying its turn +— and a forked send never appears in the listing at all, so without this rule +the next reconcile would stall a task whose agent is mid-answer. **Jump-in.** In the TUI, `A` on a running task runs the agent's `attach` command with the TUI suspended — the real session, full control, including answering @@ -211,11 +231,15 @@ standing. It applies to the three states whose session is open and between turns session that redispatch, not a headless resume, is the honest answer for. Voro probes liveness first and refuses a session still running, since that one wants the terminal. On a `review` or `waiting` task the message *is* a -reject-with-feedback: the transition runs first, so the feedback is appended to -the body and logged before anything is said, and a refused transition sends -nothing. On a `needs-input` task nothing transitions — the answer lives in the -transcript, and the agent's own `voro resume` moves the task back (DESIGN.md -§6). +reject-with-feedback: the send goes first and the transition follows it, so +feedback is appended to the body and logged only once the message is known to +have started, and a send the agent refuses leaves the task untouched. On a +`needs-input` task nothing transitions — the answer lives in the transcript, and +the agent's own `voro resume` moves the task back (DESIGN.md §6). Either way the +session row follows the send: it records the process now carrying the turn, so +reconciliation leaves the task `running` while the agent answers, and — for a +verb that forks (`{new_session}`) — the reference the conversation continued +under. The same jump-in resolves a **stale review branch**. A task can sit in `review` while other work merges, leaving its branch in conflict with the moved base