From 66ec47afbe6366db76f510af621b02e22836bada Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Thu, 20 Aug 2026 12:53:36 +0100 Subject: [PATCH] Stop firing the agent's stop verb from the reconciler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reconciliation ran two background stops. The rest-stop released any session on a needs-input/review/waiting task whose listing entry read done, on every pass; stop_finalised retired the registry entry of every session a pass finalised. Both fired unprompted, at whatever the pass found, on every read of the store — which is every TUI refresh and every CLI verb. At the operator's desk that reaches into sessions nobody asked about. The separate Claude window flickers, moves and gets reset even when the task being reconciled is not the one in focus, and a stop landing in a live session is a plausible source of prompt-cache churn. No reading of a listing is worth doing that to a desk, so both call sites go. Every stop Voro makes now hangs off something the operator just did: a closing verdict (Accept, Abort, Abandon, via stop_closed_session) or the send that needs the hold released. The quick-message path already released a held session inline, waited on, because the operator could outrun a reconcile tick. That inline release is now the only one, and it covers the same ground: the rest test — task at rest, listing entry reads done — is read on the keypress that needs the hold gone rather than swept in the background, and the invariant that a message only ever reaches a session that has explicitly handed back is unchanged. Mid-turn the liveness gate still refuses the send; between turns the send releases the hold itself and refuses outright if that release fails. The capped-session sweep's unconditional release is untouched, as are the reconciler's read-only probes. The accepted cost is what Voro did before it stopped anything: a session reconciliation finalises keeps its entry in the agent's own listing until the verdict that closes the task, and a handed-back session keeps its entry until the first message into it. That is a lingering listing entry and nothing broken — the same degradation an agent defining no stop verb has always had — and the operator's close still clears the common case. DESIGN.md §8 moves with the code: the release paragraph, the rest rule, the delivery invariant, the recorded trade, and which closes stop. Verified: cargo test --workspace (916 pass), clippy --all-targets -D warnings clean, fmt clean. The reconcile tests that pinned the removed behaviour are replaced by ones pinning its absence — a finalised dead session and a healed stale row both close without a stop, and a task at rest in all three states keeps its registration — and the send-path tests (a_send_releases_a_session_the_agent_still_holds_at_rest, a_send_makes_no_stop_when_the_session_is_not_listed_at_rest) still pass unchanged. Co-authored-by: Claude Opus 5 --- crates/voro-core/src/agent.rs | 10 +- crates/voro/src/app.rs | 34 ++-- crates/voro/src/dispatch.rs | 32 ++-- crates/voro/src/reconcile.rs | 292 +++++-------------------------- crates/voro/src/session_probe.rs | 14 +- docs/DESIGN.md | 125 +++++++------ 6 files changed, 152 insertions(+), 355 deletions(-) diff --git a/crates/voro-core/src/agent.rs b/crates/voro-core/src/agent.rs index 5c6dfd8..9ff4aac 100644 --- a/crates/voro-core/src/agent.rs +++ b/crates/voro-core/src/agent.rs @@ -1659,7 +1659,7 @@ impl AgentSessionEntry { } /// Whether this entry says its session's turn has *ended* — the narrow - /// reading the rest-stop rule acts on (DESIGN.md §8), which is not the same + /// reading the rest rule acts on (DESIGN.md §8), which is not the same /// question as [`liveness`](Self::liveness). Only `done` answers yes. /// `blocked` is the case that makes the distinction load-bearing: it reads /// dead without a live pid, but it is also what a permission prompt and a @@ -2348,9 +2348,9 @@ mod tests { } /// The built-in `claude` message verb resumes in place (DESIGN.md §8): the - /// supervisor that refuses a headless resume has been released by the - /// rest-stop before any send is made, so the send addresses the session's own - /// reference and the conversation stays under the name Voro composed for it. + /// supervisor that refuses a headless resume is released before the send is + /// made, so the send addresses the session's own reference and the + /// conversation stays under the name Voro composed for it. #[test] fn the_builtin_claude_message_verb_resumes_in_place() { let message = builtin_agents()["claude"].message().unwrap(); @@ -2663,7 +2663,7 @@ mod tests { assert_eq!(entry(r#""state": "idle""#), SessionLiveness::Dead); } - /// Rest is a narrower reading than death (DESIGN.md §8): the rest-stop acts + /// Rest is a narrower reading than death (DESIGN.md §8): the release acts /// on a turn that has *ended*, and only `done` says so. `blocked` is the /// separation that matters — dead to the liveness question, yet a turn still /// under way (a permission prompt, a supervisor mid-turn) that a stop would diff --git a/crates/voro/src/app.rs b/crates/voro/src/app.rs index 1140ee9..52fd2d2 100644 --- a/crates/voro/src/app.rs +++ b/crates/voro/src/app.rs @@ -378,8 +378,8 @@ struct CapTarget { 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). Carried whole - /// rather than as an id, because the inline rest-stop the send may have to - /// make first is addressed at the session itself. + /// rather than as an id, because the release the send may have to make + /// first is addressed at the session itself. session: voro_core::Session, /// The reference the send is addressed to: the row's, already established to /// be present. @@ -2514,11 +2514,10 @@ impl App { } /// Release the agent's hold on a session, waiting for the answer (DESIGN.md - /// §8), so a headless resume into it can land. The reconciler's rest-stop - /// makes this call off the send path on every pass; the two senders make it - /// inline where that pass cannot have covered them — the quick-message key - /// for the window between an agent handing back and the next pass noticing, - /// the capped-session sweep because no pass will ever release its target. + /// §8), so a headless resume into it can land. Both senders call it inline, + /// immediately before the send it makes deliverable: the quick-message key + /// where the target's listing entry says the agent is holding a session + /// that has come to rest, the capped-session sweep unconditionally. /// /// Nothing about the row changes either way — the session stays the task's /// conversation — so a config that will not load costs the release and @@ -2622,13 +2621,11 @@ impl App { )); return; } - // Normally reconcile has already released a handed-back session - // (DESIGN.md §8), and this finds nothing to do. It fires when the - // operator has outrun a pass — messaging within the same tick the agent - // reported in — and the hold that would refuse an in-place resume is - // still there. Failing to release it refuses the send outright rather - // than spawning one that cannot land, so the task is left exactly where - // it was. + // The agent still holds a session it has finished a turn on, and that + // hold refuses an in-place resume, so it is released here and waited on + // before the send goes out (DESIGN.md §8). Failing to release it + // refuses the send outright rather than spawning one that cannot land, + // so the task is left exactly where it was. if verdict.at_rest && let Err(e) = self.release_session(&target.session) { @@ -7173,11 +7170,10 @@ mod tests { } } - /// The inline half of the rest rule (DESIGN.md §8). Normally reconcile has - /// already released a handed-back session and this finds nothing to do; when - /// the operator outruns a pass, the send releases the session itself, at its - /// own reference, and then resumes it in place. Without that the agent's hold - /// would refuse the resume and the feedback would go nowhere. + /// The rest rule (DESIGN.md §8): a session the agent is still holding with + /// its turn ended is released by the send itself, at its own reference, and + /// then resumed in place. Without that the hold would refuse the resume and + /// the feedback would go nowhere. #[test] fn a_send_releases_a_session_the_agent_still_holds_at_rest() { let (mut app, task_id, root, paths) = send_env(FINISHED_LISTING); diff --git a/crates/voro/src/dispatch.rs b/crates/voro/src/dispatch.rs index cda16f5..5328f87 100644 --- a/crates/voro/src/dispatch.rs +++ b/crates/voro/src/dispatch.rs @@ -1028,11 +1028,9 @@ 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; -/// How long the send path's inline rest-stop is waited on before the send is -/// refused ([`stop_session_now`]). Generous next to the sub-second call it -/// covers, and it is only ever paid when the operator has outrun a reconcile -/// tick — but bounded, because a stop that hangs must not take the cockpit with -/// it. +/// How long the send path's release is waited on before the send is refused +/// ([`stop_session_now`]). Generous next to the sub-second call it covers, but +/// bounded, because a stop that hangs must not take the cockpit with it. const STOP_WAIT: Duration = Duration::from_secs(5); /// One line said into a session that already exists (DESIGN.md §8), assembled by @@ -1271,11 +1269,10 @@ pub(crate) fn append_launch_log(path: &Path, line: &str) { } } -/// Retire an agent's own registry entry for a session Voro has just closed -/// (DESIGN.md §8) — the process half of the rule that a session's entry follows -/// its row. `voro-core` decides *whether* a close should stop (the session comes -/// from `Store::apply_closing` or a reconciler finalisation); this supplies the -/// spawn. +/// Retire an agent's own registry entry for a session an operator's verdict has +/// just closed (DESIGN.md §8) — the process half of the rule that a session's +/// entry follows its row. `voro-core` decides *whether* a close should stop (the +/// session comes back from `Store::apply_closing`); this supplies the spawn. /// /// Best-effort in every direction, which is what makes it safe to fire from /// inside a transition that has already committed: an agent that defines no @@ -1298,12 +1295,11 @@ pub fn stop_session(ctx: &DispatchCtx, config: &AgentsConfig, session: &Session) }); } -/// The same stop, waited on: the send path's inline fallback (DESIGN.md §8), -/// where the operator has outrun a reconcile tick and the session is still -/// registered at rest. Unlike the detached form the answer matters — a session -/// whose hold was not released cannot be resumed in place — so this reports -/// rather than merely logging, and the caller refuses the send on an `Err` -/// having committed nothing. +/// The same stop, waited on: the release a send makes of the session it is +/// about to resume (DESIGN.md §8). Unlike the detached form the answer matters +/// — a session whose hold was not released cannot be resumed in place — so +/// this reports rather than merely logging, and the caller refuses the send on +/// an `Err` having committed nothing. /// /// "Nothing to stop" is `Ok(())`, not a failure: an agent that defines no `stop` /// verb, or a session with no captured reference, is one Voro was never going to @@ -1390,8 +1386,8 @@ fn spawn_stop( /// Load the agents config for a one-off [`stop_session`], best-effort: a missing /// or malformed `voro.toml` costs the stop, never the transition that asked for -/// it. Callers that already hold a config — the reconciler, sweeping many -/// sessions — pass their own rather than reloading per session. +/// it. Callers that already hold a config pass their own rather than reloading +/// per session. pub fn stop_closed_session(ctx: &DispatchCtx, session: &Session) { if let Ok(config) = AgentsConfig::load(&ctx.agents_path) { stop_session(ctx, &config, session); diff --git a/crates/voro/src/reconcile.rs b/crates/voro/src/reconcile.rs index d175f4b..52d596b 100644 --- a/crates/voro/src/reconcile.rs +++ b/crates/voro/src/reconcile.rs @@ -57,17 +57,14 @@ //! — is not taken here at all: it is slow enough to need its own off-loop //! runner ([`crate::probe::CapProbe`]), and it changes nothing in the database. //! -//! Whatever a pass finalises, it also stops: the agent's `stop` verb -//! is fired at the closed session's reference so its own listing loses the entry -//! along with the row, best-effort and unwaited-on. -//! -//! A session left *open* is stopped too, on a narrower test — the rest-stop -//! ([`rest_stop`]). A task in `needs-input`, `review` or `waiting` -//! has handed back, and once its listing entry agrees that the turn is over, the -//! agent's hold on the session is released. Its row stays open and stays the -//! task's conversation; only the registration goes, and with it the lock that -//! made a headless quick message impossible to deliver in place. The operator -//! still answers and rejects into that session, and `A` still opens it. +//! A pass fires no `stop`. It writes to the database and reads through the +//! agent's verbs, and it retires nothing on the agent's side: the sessions it +//! finalises keep their listing entries, as do the ones it leaves open. +//! Retiring an entry belongs to the paths the operator drives — the closing +//! verdict, and the send that needs a hold released (DESIGN.md §8) — because a +//! pass runs unprompted on every read, where a `stop` reaches into a session the +//! operator may be working in. The cost is a listing that keeps entries for +//! sessions Voro knows are over until a close clears them. //! //! There is no daemon watching for process exit. Reconciliation runs on read: //! `App::refresh` and every CLI verb call [`reconcile_live_sessions`] before @@ -81,9 +78,9 @@ use voro_core::{ AgentSessionEntry, AgentsConfig, LivenessSource, Result, Store, TaskState, read_cap, }; -use crate::dispatch::{DispatchCtx, stop_session}; +use crate::dispatch::DispatchCtx; use crate::session_probe::{ - listing_says_at_rest, listing_says_live, pid_is_alive, read_session_cap, run_sessions_command, + listing_says_live, pid_is_alive, read_session_cap, run_sessions_command, }; /// An agent's session listing for this pass, keyed by agent name. `None` against @@ -117,14 +114,8 @@ pub fn reconcile_live_sessions(store: &mut Store, ctx: &DispatchCtx) -> Result Result( .as_deref() } -/// Release a session that has handed back (DESIGN.md §8): a task at rest — -/// `needs-input`, `review`, `waiting` — whose session the agent still holds -/// registered with its turn ended is stopped, so the lock a headless message -/// resumes through is already gone by the time the operator sends one. -/// -/// The rest state is only half the test, because a task reaches it the moment -/// the agent calls `voro done`/`ask` — from inside a turn still running, whose -/// tail a stop would cut off. The listing supplies the other half: the entry -/// must itself read `at_rest`, which it does only once that turn is over. -/// Everything else is left alone — a `blocked` entry is mid-turn or waiting on a -/// permission prompt, and an absent one was never registered or has been stopped -/// already. -/// -/// Nothing about the row changes: the session is still the task's conversation, -/// `A` still reopens it with its full context, and only the agent-side -/// registration goes. Which is also why firing this on every pass converges -/// rather than repeating — a stopped session leaves the listing, so the next -/// pass finds nothing at rest to stop. -fn rest_stop( - ctx: &DispatchCtx, - config: Option<&AgentsConfig>, - listings: &mut Listings, - session: &voro_core::Session, -) { - let Some(session_ref) = session.session_ref.as_deref() else { - return; - }; - let handed_back = listing(config, listings, &session.agent) - .is_some_and(|entries| listing_says_at_rest(entries, session_ref)); - if !handed_back { - return; - } - // `handed_back` can only be true with a config loaded, since the listing - // came out of one; the agent defining no `stop` verb is skipped in there. - if let Some(config) = config { - stop_session(ctx, config, session); - } -} - -/// Retire the agent's registry entry for a session reconciliation has just -/// finalised (DESIGN.md §8) — both flavours, the dead dispatch it stalls and the -/// stale row it heals, since either way the session is over and Voro has said so. -/// Skipped in silence when the config would not load, which is the same cost a -/// missing config already carries here. -fn stop_finalised(ctx: &DispatchCtx, config: Option<&AgentsConfig>, closed: &voro_core::Session) { - if let Some(config) = config { - stop_session(ctx, config, closed); - } -} - /// Best-effort usage-cap detector for an agent with no `logs` verb (DESIGN.md /// §8): read the launch log's tail and put it through the same classifier the /// session-output path uses, so both channels agree on what a cap looks like. @@ -770,12 +713,11 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } - // --- finalising a session stops it --- + // --- reconciliation stops nothing --- /// A `voro.toml` whose `claude` agent lists `listing` and whose `stop` verb /// records the reference it was fired at, plus that marker's path — so a - /// test can tell a stop that happened from one that did not, and read which - /// session it named. + /// test can tell a stop that happened from one that did not. fn stop_fixture_listing(name: &str, listing: &str) -> (DispatchCtx, PathBuf, PathBuf) { let dir = std::env::temp_dir().join(format!("voro-reconcile-stop-{name}-{}", std::process::id())); @@ -799,33 +741,25 @@ mod tests { (ctx_at(agents_path), marker, dir) } - /// The same fixture with an empty listing, which is what every close-time - /// stop test wants: the session is finalised on its own terms and the - /// listing has nothing to say about it either way. + /// The same fixture with an empty listing, which is what the finalisation + /// tests want: the session is finalised on its own terms and the listing + /// has nothing to say about it either way. fn stop_fixture(name: &str) -> (DispatchCtx, PathBuf, PathBuf) { stop_fixture_listing(name, "[]") } - /// Wait for a detached stop to have written its marker, since nothing in the - /// reconcile path waits on it. Returns what it wrote, or `None` if it never - /// ran. - fn stopped_ref(marker: &std::path::Path) -> Option { - for _ in 0..100 { - if let Ok(text) = std::fs::read_to_string(marker) - && !text.is_empty() - { - return Some(text); - } - std::thread::sleep(std::time::Duration::from_millis(30)); - } - None + /// Assert no stop was spawned. A stop would be detached, so its absence + /// only means anything once there has been time for one to write. + fn no_stop(marker: &std::path::Path, name: &str) { + std::thread::sleep(std::time::Duration::from_millis(150)); + assert!(!marker.exists(), "{name}: the reconciler fired a stop"); } - /// The dead-dispatch path: a session the reconciler finalises is also - /// retired from the agent's own listing, at the reference Voro captured for - /// it, so the entry goes when the row does. + /// The dead-dispatch path: a session the reconciler finalises keeps its + /// entry in the agent's own listing (DESIGN.md §8). The row closes and the + /// task stalls; retiring the entry waits for the operator's close. #[test] - fn a_finalised_dead_session_is_stopped() { + fn a_finalised_dead_session_is_not_stopped() { let (ctx, marker, dir) = stop_fixture("dead"); let (mut s, task_id) = running_task(); let session = s @@ -841,15 +775,15 @@ mod tests { assert_eq!(reconcile_live_sessions(&mut s, &ctx).unwrap(), 1); assert_eq!(s.task(task_id).unwrap().state, TaskState::Stalled); - assert_eq!(stopped_ref(&marker).as_deref(), Some("full-uuid-1")); + no_stop(&marker, "dead"); let _ = std::fs::remove_dir_all(&dir); } - /// The stale-row heal: a session still open on a task that has already - /// closed is finalised without a probe, and stopped on the same terms. + /// The stale-row heal, on the same terms: a session still open on a task + /// that has already closed is finalised without a probe and without a stop. #[test] - fn a_healed_stale_session_is_stopped() { + fn a_healed_stale_session_is_not_stopped() { let (ctx, marker, dir) = stop_fixture("stale"); let (mut s, task_id) = running_task(); let session = s @@ -866,13 +800,11 @@ mod tests { s.set_session_ref(stranded.id, "stale-uuid").unwrap(); assert_eq!(reconcile_live_sessions(&mut s, &ctx).unwrap(), 1); - assert_eq!(stopped_ref(&marker).as_deref(), Some("stale-uuid")); + no_stop(&marker, "stale"); let _ = std::fs::remove_dir_all(&dir); } - // --- the rest-stop releases a session that has handed back --- - /// The transitions that put a task at rest with its session still open, and /// the name each test labels its fixture with. fn rest_states() -> Vec<(&'static str, Vec)> { @@ -902,174 +834,36 @@ mod tests { (s, task_id, session.id) } - /// The rule itself (DESIGN.md §8): a task that has handed back, whose - /// session's own listing entry agrees the turn is over, has that session - /// released — so the lock a headless quick message resumes through is gone - /// before the operator sends one. Every state the message key serves. + /// A task that has handed back keeps its session registered, listing entry + /// and all. `done` is the release trigger (DESIGN.md §8), but it is read on + /// the send path, on the keypress that needs the hold gone; a pass reading + /// the same entry acts on nothing. Every state the message key serves, and + /// nothing moves in the store either. #[test] - fn a_handed_back_session_at_rest_is_stopped() { + fn a_session_at_rest_is_left_registered() { for (name, actions) in rest_states() { let (ctx, marker, dir) = stop_fixture_listing(name, r#"[{"sessionId": "full-uuid-1", "state": "done"}]"#); let (mut s, task_id, session_id) = task_at_rest(&actions); let before = s.task(task_id).unwrap().state; + let events = s.events_for(task_id).unwrap().len(); assert_eq!(reconcile_live_sessions(&mut s, &ctx).unwrap(), 0, "{name}"); - assert_eq!( - stopped_ref(&marker).as_deref(), - Some("full-uuid-1"), - "{name}" - ); - // Nothing about the task or the row moves: the session is still the - // task's conversation, and only the agent-side registration went. + no_stop(&marker, name); let session = s.session(session_id).unwrap(); assert!(session.ended_at.is_none(), "{name}"); - assert_eq!(session.outcome, None, "{name}"); assert_eq!( session.session_ref.as_deref(), Some("full-uuid-1"), "{name}" ); assert_eq!(s.task(task_id).unwrap().state, before, "{name}"); + assert_eq!(s.events_for(task_id).unwrap().len(), events, "{name}"); let _ = std::fs::remove_dir_all(&dir); } } - /// The guard that makes the rule safe in the other direction. A `blocked` - /// entry is a turn still under way — a permission prompt, a supervisor - /// mid-turn — and the handover verbs fire from *inside* the turn that - /// reports, so a task reaches `review` while its agent is still finishing - /// the sentence. Reading rest off the task state alone would cut that off. - /// An absent entry has nothing to release. - #[test] - fn a_session_not_at_rest_is_left_registered() { - for (name, listing, actions) in [ - ( - "blocked", - r#"[{"sessionId": "full-uuid-1", "state": "blocked"}]"#, - vec![Action::Complete(None)], - ), - ( - "working", - r#"[{"sessionId": "full-uuid-1", "state": "working"}]"#, - vec![Action::Ask("A or B?".into())], - ), - ("absent", "[]", vec![Action::Complete(None)]), - ] { - let (ctx, marker, dir) = stop_fixture_listing(name, listing); - let (mut s, ..) = task_at_rest(&actions); - - assert_eq!(reconcile_live_sessions(&mut s, &ctx).unwrap(), 0, "{name}"); - std::thread::sleep(std::time::Duration::from_millis(150)); - assert!(!marker.exists(), "{name}: stopped a session mid-turn"); - - let _ = std::fs::remove_dir_all(&dir); - } - } - - /// A `running` task is mid-work by definition and is never released, - /// whatever its listing entry happens to say — the entry going `done` there - /// is a session that died without reporting, which the liveness arm - /// finalises and stops on its own terms rather than leaving the row open. - #[test] - fn a_running_task_is_not_rest_stopped() { - let (ctx, marker, dir) = stop_fixture_listing( - "running", - r#"[{"sessionId": "full-uuid-1", "state": "working"}]"#, - ); - let (mut s, task_id) = running_task(); - let session = s - .create_session(task_id, "claude", None, LivenessSource::Listing, None) - .unwrap(); - s.set_session_ref(session.id, "full-uuid-1").unwrap(); - - assert_eq!(reconcile_live_sessions(&mut s, &ctx).unwrap(), 0); - std::thread::sleep(std::time::Duration::from_millis(150)); - assert!(!marker.exists()); - assert_eq!(s.task(task_id).unwrap().state, TaskState::Running); - - let _ = std::fs::remove_dir_all(&dir); - } - - /// An agent with no `stop` verb skips in silence, as it does at close time: - /// its sessions stay registered and nothing errors. `codex` needs none. - #[test] - fn a_stopless_agent_is_not_rest_stopped() { - // The same `done` listing the rule fires on — only the `stop` verb is - // missing, so this is the verb's absence deciding and nothing else. - let (ctx, dir) = sessions_fixture( - "rest-stopless", - r#"[{"sessionId": "full-uuid-1", "state": "done"}]"#, - ); - let (mut s, task_id, session_id) = task_at_rest(&[Action::Complete(None)]); - - assert_eq!(reconcile_live_sessions(&mut s, &ctx).unwrap(), 0); - assert!(s.session(session_id).unwrap().ended_at.is_none()); - assert_eq!(s.task(task_id).unwrap().state, TaskState::Review); - - let _ = std::fs::remove_dir_all(&dir); - } - - /// A session with no captured reference has nothing to name in a stop, and - /// the rest-stop is skipped rather than guessed at. - #[test] - fn a_refless_session_at_rest_is_not_stopped() { - let (ctx, marker, dir) = stop_fixture_listing( - "rest-refless", - r#"[{"sessionId": "full-uuid-1", "state": "done"}]"#, - ); - let (mut s, task_id) = running_task(); - s.create_session(task_id, "claude", None, LivenessSource::Listing, None) - .unwrap(); - s.apply(task_id, Action::Complete(None)).unwrap(); - - assert_eq!(reconcile_live_sessions(&mut s, &ctx).unwrap(), 0); - std::thread::sleep(std::time::Duration::from_millis(150)); - assert!(!marker.exists()); - - let _ = std::fs::remove_dir_all(&dir); - } - - /// The rest-stop writes nothing, so a pass that makes one leaves the event - /// log exactly as it found it — the session's registration is agent-side - /// state, not Voro's. - #[test] - fn a_rest_stop_records_no_event() { - let (ctx, marker, dir) = stop_fixture_listing( - "rest-events", - r#"[{"sessionId": "full-uuid-1", "state": "done"}]"#, - ); - let (mut s, task_id, _) = task_at_rest(&[Action::Complete(None)]); - let before = s.events_for(task_id).unwrap().len(); - - assert_eq!(reconcile_live_sessions(&mut s, &ctx).unwrap(), 0); - assert_eq!(stopped_ref(&marker).as_deref(), Some("full-uuid-1")); - assert_eq!(s.events_for(task_id).unwrap().len(), before); - - let _ = std::fs::remove_dir_all(&dir); - } - - /// An agent naming no `stop` verb degrades to what Voro did before: the row - /// closes, the entry lingers, and nothing errors. - #[test] - fn a_stopless_agent_finalises_without_stopping() { - let (mut s, task_id) = running_task(); - let session = s - .create_session( - task_id, - "manual", - Some(dead_pid()), - LivenessSource::Pid, - None, - ) - .unwrap(); - s.set_session_ref(session.id, "full-uuid-1").unwrap(); - - assert_eq!(reconcile_live_sessions(&mut s, &no_config()).unwrap(), 1); - assert_eq!(s.task(task_id).unwrap().state, TaskState::Stalled); - } - // --- capped deaths read the session's own output --- /// A `voro.toml` whose `claude` agent lists no sessions and prints diff --git a/crates/voro/src/session_probe.rs b/crates/voro/src/session_probe.rs index 3a3efcf..e2c88ca 100644 --- a/crates/voro/src/session_probe.rs +++ b/crates/voro/src/session_probe.rs @@ -63,10 +63,10 @@ pub fn listing_says_live(entries: &[AgentSessionEntry], session_ref: &str) -> bo } /// Whether a listing shows this ref as a session whose turn has ended and which -/// the agent is therefore still holding registered — the rest-stop's trigger -/// (DESIGN.md §8). A ref that has dropped out of the listing answers no: it was -/// never registered, or it has already been stopped, and either way there is -/// nothing to release. +/// the agent is therefore still holding registered — the send path's trigger to +/// release it (DESIGN.md §8). A ref that has dropped out of the listing answers +/// no: it was never registered, or it has already been stopped, and either way +/// there is nothing to release. pub fn listing_says_at_rest(entries: &[AgentSessionEntry], session_ref: &str) -> bool { entries .iter() @@ -204,7 +204,7 @@ pub struct SessionVerdict { /// Whether the session is still going, `None` when that is unknowable. pub live: Option, /// Whether the agent still holds it registered with its turn ended - /// ([`listing_says_at_rest`]). Unknowable reads as no, since the rest-stop + /// ([`listing_says_at_rest`]). Unknowable reads as no, since the release /// acts only on positive evidence. pub at_rest: bool, } @@ -387,8 +387,8 @@ mod tests { assert!(apart <= 1, "{label} vs {minutes} minutes past midnight"); } - /// The rest reading the send path and the reconciler act on: a session the - /// agent still holds registered with its turn ended. `blocked` — a + /// The rest reading the send path acts on: a session the agent still holds + /// registered with its turn ended. `blocked` — a /// permission prompt, a supervisor mid-turn — is the one that must not /// answer yes, and an entry that has left the listing has nothing to /// release. diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 8a79feb..becacaa 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -1183,9 +1183,9 @@ is a server process registered with a daemon, and `-p --resume` bypasses that daemon to own the transcript file — which is why the registry refuses it while a supervisor lives. Voro's answer is not to route around that hold but to remove it: the built-in `claude` message verb is a plain `claude -p --resume -{session}`, and the session is stopped, through the same optional `stop` verb -the close path uses, as soon as it comes to rest — so the hold is already -gone by the time any message is sent. One session id, one Voro-composed name, +{session}`, and the send releases the session first, through the same optional +`stop` verb the close path uses, waiting for the answer — so the hold is gone +by the time the message goes out. One session id, one Voro-composed name, one linear transcript for the task's whole life, and no kill step anywhere near a send. @@ -1227,44 +1227,54 @@ 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. -**The release happens at rest, not at the transition that hands back.** The -handover verbs fire mid-turn: `voro done` and `voro ask` run inside the agent's +**The release happens at rest, and on the send that needs it.** The handover +verbs fire mid-turn: `voro done` and `voro ask` run inside the agent's still-executing turn, so stopping at the transition itself would kill the tail -of the very turn that is reporting. The trigger is therefore *rest*, judged from -the two sources reconciliation already reads — a task in a between-turns state +of the very turn that is reporting. The trigger is therefore *rest*, judged +from the two sources a send already reads — a task in a between-turns state (`needs-input`, `review`, `waiting`) whose open session's listing entry reports -its turn ended (state `done`) is stopped, best-effort, exactly as a closing row -is. The guard is load-bearing in both directions. A session at `blocked` — a +its turn ended (state `done`) is released, and only then does the message go +out. The guard is load-bearing in both directions. A session at `blocked` — a permission prompt, a supervisor mid-turn — never reads `done`, so it is never stopped; an entry that is absent was never registered or has already been stopped, so there is nothing to do, and the stop is idempotent and safe on a dead session. Nothing about the row changes: it stays open, it stays the task's conversation, and no event and no transition is recorded. Only the agent-side -registration goes, which is also why firing the rule on every pass converges -rather than repeating — a stopped session leaves the listing, so the next pass -finds nothing at rest to stop. +registration goes. + +Reconciliation makes no release of its own, and the reason is the operator's +desk rather than the model. A pass runs on every read, unprompted and in the +background, so a rule swept there fires `stop` at whatever it finds — including +sessions the operator is sitting in front of, in a window belonging to a task +they are not even looking at, and again on every pass. That cost is real rather +than hypothetical: such a stop resets the window it lands in, and no reading of +a listing is worth doing that to a desk. Every stop Voro makes therefore hangs +off something the operator just did — a closing verdict, or the send this +release covers. What emerges is an invariant worth stating on its own: **a message can only ever be delivered to a session that has explicitly handed back to Voro.** Mid-turn, the liveness gate refuses the send honestly; between turns, the -rest-stop has already released the hold. Consecutive messages fall under the -same rule whether or not a finished `-p` turn puts the session back in the -agent's registry: where it does, the entry reads `done` again and the next pass -releases it again; where it does not, there was never a hold to release. -Stacked mid-turn sends are structurally impossible rather than merely -discouraged. The send path carries no unconditional stop of its own: it gates -on liveness as before, and only where the target's listing entry still reads -`done` — the operator outrunning a reconcile tick — does it make the same -release inline and waited-on, refusing the send outright if that release fails. -A message that could not be made deliverable commits nothing and leaves the -task exactly where it was. - -The trade, recorded rather than discovered later: releasing at rest retires the -session's entry from the agent's own view at *handover* rather than at close, so -a review task's named row lives in Voro's queue and not in the agent's session -list. Attach still opens the stopped session with its full context — a stop -keeps the conversation — so jumping in, answering in-session and reading the -output are all unaffected. +send releases the hold itself and waits for the answer before spawning +anything. Consecutive messages fall under the same rule whether or not a +finished `-p` turn puts the session back in the agent's registry: where it does, +the entry reads `done` again and the next send releases it again; where it does +not, there was never a hold to release. Stacked mid-turn sends are structurally +impossible rather than merely discouraged. The stop is not unconditional: the +send gates on liveness first, releases only where the target's listing entry +reads `done`, and refuses outright if that release fails. A message that could +not be made deliverable commits nothing and leaves the task exactly where it +was. + +The trade, recorded rather than discovered later: an entry lingers in the +agent's own view until something the operator does clears it. A handed-back +session keeps its entry until the first message into it, and a session +reconciliation finalises keeps its entry until the verdict that closes the task. +That is a lingering listing entry and nothing broken — the same degradation an +agent defining no `stop` verb has always had — and the operator's close still +clears the common case. Attach is unaffected either way: a stop keeps the +conversation, so jumping in, answering in-session and reading the output all +open the session with its full context. **A permission mode is a property of a launch, not of a verb.** `--permission-mode` is per invocation rather than something the session @@ -1327,19 +1337,20 @@ agent that defines none — or one whose `stop` fails — degrades to exactly what Voro did before, a lingering listing entry and nothing broken. The transition never waits on it and never rolls back for it. Which closes stop is deliberately narrower than which closes happen: the operator's closing verdicts -(`Accept`, `Abort`, `Abandon`) stop, and so do the reconciler's finalisations, -both the dead-dispatch stall and the stale-row heal, since by then the session -is over and Voro has said so. Sessions that stay open — `needs-input`, -`review`, `waiting` — are not stopped by a *close*, because there is no close; -they are released by the rest-stop above instead, on its own narrower test, and -that release takes the registration without taking the row, so the operator -still answers and rejects into them. A refine round's conclusion is the one -close that does not stop, because its commonest trigger is the rewriting agent's -own `voro set --body-file`, a call made from inside the session and mid-turn: -stopping there would kill the agent that just reported. `voro-core` decides -*whether* a close stops (`Store::apply_closing` hands back the session a verdict -retired, `Store::reconcile_session` the one a pass finalised); the `voro` crate -supplies the spawn, beside the other process seams. +(`Accept`, `Abort`, `Abandon`) stop, and nothing else does. The reconciler's +finalisations — the dead-dispatch stall, the stale-row heal — close the row +and leave the entry, because a pass fires in the background at sessions nobody +asked about (above); the entry they leave is retired by the verdict that +eventually closes the task. Sessions that stay open — `needs-input`, `review`, +`waiting` — are not stopped by a *close* either, because there is no close; +they are released by the send that needs them released, above, and that release +takes the registration without taking the row, so the operator still answers and +rejects into them. A refine round's conclusion is the one close that does not +stop, because its commonest trigger is the rewriting agent's own `voro set +--body-file`, a call made from inside the session and mid-turn: stopping there +would kill the agent that just reported. `voro-core` decides *whether* a close +stops (`Store::apply_closing` hands back the session a verdict retired); the +`voro` crate supplies the spawn, beside the other process seams. **Worktree lifecycle.** A dispatched agent does its work in a throwaway git worktree of the project checkout it creates itself — the dispatch preamble @@ -1634,18 +1645,18 @@ It goes out through the existing `message` verb rather than through any new channel, and it is the one send that releases its target *unconditionally* first. A supervisor-owned session refuses a plain headless `--resume` for as long as its supervisor lives, and a capped session's supervisor is alive by -definition, so something has to remove that hold. The rest-stop above never -will: it fires on a listing entry that reads `done`, and a capped session reads -`blocked` — the same word a permission prompt earns — for as long as it -sits there. So the sweep stops the session itself, waits for the answer, and -resumes it in place, abandoning the nudge if the release fails rather than -spawning a send that could only be refused. No `tmux send-keys` channel or -supervisor IPC is needed, and none is built. The send is otherwise recorded -exactly as a quick message is, with the pid now carrying the turn, so a nudged -session stays as visible to the reconciler as a messaged one; the badge is -dropped the moment the send lands, so a second press cannot put a second agent -on the same worktree, and it returns on the next reading if the session is -still held. +definition, so something has to remove that hold. The rest test above never +covers it: it releases on a listing entry that reads `done`, and a capped +session reads `blocked` — the same word a permission prompt earns — for as +long as it sits there. So the sweep stops the session itself, waits for the +answer, and resumes it in place, abandoning the nudge if the release fails +rather than spawning a send that could only be refused. No `tmux send-keys` +channel or supervisor IPC is needed, and none is built. The send is otherwise +recorded exactly as a quick message is, with the pid now carrying the turn, so +a nudged session stays as visible to the reconciler as a messaged one; the +badge is dropped the moment the send lands, so a second press cannot put a +second agent on the same worktree, and it returns on the next reading if the +session is still held. Two costs come with that unconditional stop, priced rather than discovered. The stop is exactly as safe as the cap reading is right: a nudge into a session @@ -1661,8 +1672,8 @@ is mid-turn, and refused again when the session is listed live — but a capped session is `running`, listed live, and *not* mid-turn, which is the one combination nothing else in the cockpit can recognise. Nothing else may skip those guards, and standing them down is what obliges the sweep to release its -own target rather than trusting the rest rule to have done it (above). The -sweep fires only when pressed — a staging decision rather than a principle: +own target unconditionally rather than on the rest test (above). The sweep +fires only when pressed — a staging decision rather than a principle: automatic resumption once the window reopens is wanted, and manual first buys the evidence automation needs, that a nudge reliably lands and that the badge does not false-positive, while a wrong reading still costs one keypress instead