From 342cee41a8469101860b5ade379d14580432982a Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Fri, 14 Aug 2026 14:52:52 +0100 Subject: [PATCH] List every optional verb an agent defines, message included MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agent list` and the Config screen assembled each agent's bracketed verb list from four hand-written entries — sessions, attach, resume, plan — while the warning under the row read a seven-verb roster. So the built-in claude listed as `[sessions attach resume plan]` while the same rows warned that an override had dropped `message`, `logs` or `stop`: the two lines disagreed about the same agent. Quick message is the verb that suffers most from the omission, being entirely per-agent and otherwise invisible until `a` is pressed and a status line read. Both lines now read one `OPTIONAL_VERBS` roster in voro-core, so no verb can be named by one and not the other, and a verb added to the set later appears in both without a second edit. `AgentTemplate::verbs()` renders it; the listing gains `message`, `logs` and `stop`. A `message` carrying `{new_session}` reads `message(fork)`. Forking is not a spelling detail: it is what makes a send land in a session whose supervisor still holds it, and it moves the reference the row addresses afterwards, so the row distinguishes an agent Voro can steer between turns from one it cannot. A message that resumes in place still reads `message`. DESIGN.md §11a and docs/agent-integration.md record both. Verified with `cargo test --workspace` (836 tests), `cargo clippy --workspace --all-targets -- -D warnings`, `voro agent list`, and the Config screen in a scratch tmux at 110 columns, where the built-in claude row reads `[sessions attach resume message(fork) logs stop plan]`. --- crates/voro-core/src/agent.rs | 110 +++++++++++++++++++++++++++++----- crates/voro/src/app.rs | 10 +--- crates/voro/src/cli.rs | 16 +++-- docs/DESIGN.md | 2 +- docs/agent-integration.md | 4 +- 5 files changed, 106 insertions(+), 36 deletions(-) diff --git a/crates/voro-core/src/agent.rs b/crates/voro-core/src/agent.rs index 475fecb..f41c1af 100644 --- a/crates/voro-core/src/agent.rs +++ b/crates/voro-core/src/agent.rs @@ -464,8 +464,46 @@ impl AgentTemplate { pub fn model_plan(&self) -> Option<&str> { self.model_plan.as_deref() } + + /// The optional verbs this agent defines, in roster order, as `agent list` + /// and the Config screen name them (DESIGN.md §8). A `message` that carries + /// [`NEW_SESSION_PLACEHOLDER`] reads `message(fork)`, because forking is + /// what a send into a supervisor-held session needs and it moves the + /// session reference the row afterwards addresses. + pub fn verbs(&self) -> Vec<&'static str> { + OPTIONAL_VERBS + .iter() + .filter_map(|(verb, defined)| { + let template = defined(self)?; + Some( + if *verb == "message" && template.contains(NEW_SESSION_PLACEHOLDER) { + "message(fork)" + } else { + *verb + }, + ) + }) + .collect() + } } +/// A verb's name beside the accessor for its template. +type VerbAccessor = (&'static str, fn(&AgentTemplate) -> Option<&str>); + +/// Every verb an agent may define beyond `dispatch`, in the order they are +/// listed to the operator. One roster serves both the positive listing and the +/// dropped-verb warning under it, so the two lines cannot disagree about the +/// same agent. +const OPTIONAL_VERBS: [VerbAccessor; 7] = [ + ("sessions", AgentTemplate::sessions), + ("attach", AgentTemplate::attach), + ("resume", AgentTemplate::resume), + ("message", AgentTemplate::message), + ("logs", AgentTemplate::logs), + ("stop", AgentTemplate::stop), + ("plan", AgentTemplate::plan), +]; + /// What a launch *is* (DESIGN.md §8): the one place a backgrounded or /// foreground agent session's identity is composed. A launch names its session, /// its prompt and log files, and its line in the launch log from this single @@ -1400,22 +1438,11 @@ impl AgentsConfig { else { return Vec::new(); }; - [ - ( - "sessions", - builtin.sessions.is_some(), - user.sessions.is_some(), - ), - ("attach", builtin.attach.is_some(), user.attach.is_some()), - ("resume", builtin.resume.is_some(), user.resume.is_some()), - ("message", builtin.message.is_some(), user.message.is_some()), - ("logs", builtin.logs.is_some(), user.logs.is_some()), - ("stop", builtin.stop.is_some(), user.stop.is_some()), - ("plan", builtin.plan.is_some(), user.plan.is_some()), - ] - .into_iter() - .filter_map(|(verb, in_builtin, in_user)| (in_builtin && !in_user).then_some(verb)) - .collect() + OPTIONAL_VERBS + .iter() + .filter(|(_, defined)| defined(builtin).is_some() && defined(user).is_none()) + .map(|(verb, _)| *verb) + .collect() } /// Every agent as `(name, template, provenance)`, sorted by name, for @@ -2043,6 +2070,57 @@ mod tests { ); } + /// The listing and the dropped-verb warning read the same roster, so an + /// agent cannot be listed as lacking a verb the warning says it dropped. + #[test] + fn verbs_lists_every_optional_verb_and_marks_a_forking_message() { + let agents = builtin_agents(); + assert_eq!( + agents["claude"].verbs(), + vec![ + "sessions", + "attach", + "resume", + "message(fork)", + "logs", + "stop", + "plan" + ] + ); + assert_eq!(agents["codex"].verbs(), vec!["resume"]); + + // A message that resumes in place keeps the reference it had, so it is + // named plainly; the roster still lists it. + let text = r#" + [agents.a] + dispatch = "run {prompt_file}" + message = "say --into {session} {prompt_file}" + "#; + let config = AgentsConfig::parse(text, Path::new("/tmp/voro.toml")).unwrap(); + assert_eq!(config.agent("a").unwrap().verbs(), vec!["message"]); + } + + /// Every verb the warning can name is a verb the listing can name, which is + /// the invariant that kept the two lines disagreeing before they shared a + /// roster: a wholesale override of claude that drops everything reports the + /// same set the built-in row lists. + #[test] + fn the_listing_and_the_dropped_verb_warning_cover_the_same_verbs() { + let text = r#" + [agents.claude] + cmd = "claude -p {prompt_file}" + "#; + let config = AgentsConfig::parse(text, Path::new("/tmp/voro.toml")).unwrap(); + let dropped = config.override_missing_verbs("claude"); + let listed: Vec<&str> = builtin_agents()["claude"] + .verbs() + .into_iter() + .map(|verb| verb.split('(').next().expect("a verb name")) + .collect(); + assert_eq!(dropped, listed); + assert!(config.agent("claude").unwrap().verbs().is_empty()); + } + #[test] fn render_message_binds_both_placeholders_shell_quoted() { let rendered = render_message( diff --git a/crates/voro/src/app.rs b/crates/voro/src/app.rs index ef6d35f..31d0e34 100644 --- a/crates/voro/src/app.rs +++ b/crates/voro/src/app.rs @@ -910,15 +910,7 @@ impl App { self.config_agents = config .entries() .map(|(name, template, provenance)| { - let verbs = [ - ("sessions", template.sessions()), - ("attach", template.attach()), - ("resume", template.resume()), - ("plan", template.plan()), - ] - .into_iter() - .filter_map(|(verb, defined)| defined.map(|_| verb)) - .collect(); + let verbs = template.verbs(); ConfigAgentRow { name: name.to_string(), dispatch: template.dispatch().to_string(), diff --git a/crates/voro/src/cli.rs b/crates/voro/src/cli.rs index 198db6b..2c48597 100644 --- a/crates/voro/src/cli.rs +++ b/crates/voro/src/cli.rs @@ -1306,15 +1306,7 @@ fn agent_verb(cmd: AgentCmd, ctx: &DispatchCtx) -> Result { } else { " " }; - let verbs: Vec<&str> = [ - ("sessions", template.sessions()), - ("attach", template.attach()), - ("resume", template.resume()), - ("plan", template.plan()), - ] - .into_iter() - .filter_map(|(verb, defined)| defined.map(|_| verb)) - .collect(); + let verbs = template.verbs(); let suffix = if verbs.is_empty() { String::new() } else { @@ -2589,6 +2581,12 @@ mod tests { assert!(listed.contains("claude"), "{listed}"); assert!(listed.contains("codex"), "{listed}"); assert!(listed.contains("built-in"), "{listed}"); + // every optional verb the agent defines, the quick message included and + // marked as the forking send it is + assert!( + listed.contains("[sessions attach resume message(fork) logs stop plan]"), + "{listed}" + ); // init writes an optional skeleton let out = call(&mut s, &["agent", "init"]).unwrap(); diff --git a/docs/DESIGN.md b/docs/DESIGN.md index be26fd1..9fbab51 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -179,7 +179,7 @@ The **docs** tables (§3) are purely additive — no existing row changes shape A project that has stopped mattering is **archived** rather than deleted: `voro project archive` (and the projects screen's `A` key) sets the flag, and every cockpit view — the queue, `voro next`, the state counts and `stats`, the running strip — excludes the project and *all* of its tasks, whatever state each holds. This is retirement, not a transition: no task is moved or closed, the event log is untouched, and unarchiving restores the pre-archive view exactly. It is deliberately distinct from weight 0, which is a snooze — a parked project is expected back and its row sits untagged among the rest — whereas an archived project remains only on the projects screen and `voro project list`, dimmed under an `[archived]` tag, so it can be found and unarchived. The flag also closes the side doors: dispatch and redispatch refuse a task in an archived project, and `add`/`propose`/import refuse to create new work there — the refusals live in `voro-core` beside the human-task guards, so no interface can smuggle work into a retired project. Deleting a project outright stays reserved for one with no tasks at all; removing a project *and* its history is a separate, deliberate purge. -Agent definitions are command templates, not state, so they live outside the database. Voro *owns* the common ones — `claude` and `codex` are compiled into `voro-core`, so they version with the binary and every upgrade carries the current verb set (the session verbs of §8) to every install with no re-init. The user's `~/.config/voro/voro.toml` is then layered on top and is for extensions, overrides, and app options: it may add a new agent, replace a built-in wholesale (a `[agents.claude]` table overrides the built-in claude *entirely*, not per-verb — predictable over a partial merge), and set `default_agent` and the viewers. Viewers are command templates too, and live in the same file for the same reason — and Voro owns the common ones exactly as it owns the common agents: `code`, `cursor` and `zed` are compiled in and probed against PATH in that order, so a fresh install with any editor CLI installed opens a task's checkout with no configuration at all, which is what the review step of a first session needs. A user `[viewers.]` table then layers on top: named for a built-in it replaces that built-in wholesale, named for anything else it adds a viewer. The built-ins take `{path}` alone rather than a diff range, because that is the shape they can honour — an editor cannot open `{base}...{branch}` from its command line — and because a viewer is spawned detached with no terminal (§8), which is also why no built-in is a pager-driven command like `git difftool -d`: it would have nothing to draw on. `default_viewer` names the one used when nothing picks a viewer by name, and the older single anonymous `[viewer]` table stays valid as that default (a sole named viewer also serves as the default without being named). Resolution therefore runs user-first and probe-last: with a name, the user's table for it, else the built-in of that name; without one, `default_viewer`, else the anonymous `[viewer]`, else the sole named table, else the first built-in found on PATH. When even that finds nothing, what the operator is asked for is to *register the viewer they already use* — `voro viewer add ''` — not to install one of Voro's; the probed built-ins follow as diagnosis, after the action, so what to do is what reads first (the status line wraps rather than truncating, §9, so the diagnosis is not paid for in lost advice). The failure never reports the config file as invalid, since on a fresh install there is no file to be invalid. In the TUI it is not reported at all but answered: `o` with nothing resolving raises the add-viewer form itself (§5), because the operator is two fields away from the diff they asked for and the Config screen would only ask for the same two. Saving does not then open the task — pressing `o` again does — so the key keeps doing one thing. Which viewer a *project* uses is state, so it lives in the database (`projects.viewer`, §8), naming one of these templates — which, since the review keys split (§8), is all that setting decides, and is why the column holds a viewer name and nothing else. A viewer command carries up to three optional placeholders (§8): `{path}` — the task's worktree, or the project checkout when it has none — plus `{branch}` (the task's branch, empty when it has none) and `{base}` (the checkout's default branch), so `{base}...{branch}` spells the review diff's range rather than opening a bare directory. An agent table may also carry a small **model map** beside its verbs — `model`, `model_deep`, and `model_plan` — whose values fill the `{model}` placeholder in the `dispatch` and `plan` templates (§8). They are plain strings, opaque to Voro, which is why they live in the same file as the templates they are pasted into rather than in the schema: the model is part of how a command is spelled, not state about a task. Two further placeholders in those templates are filled from the launch rather than from this file: `{session_name}`, the name Voro composes for the session a launch opens, and `{task_id}`, the task's numeric id. Like `{model}` they are meaningful only where a command starts work, so both are refused on the session verbs, and `{task_id}` on `plan` as well, whose target may be a project with no task to name (§8). It also carries the queue's two pricing options — `max_running`, the dispatch WIP cap, and a `[costs]` table overriding the per-action attention divisors (§7) — for the same reason the viewers live here: they are operator preference about how the tool behaves, not state about a task, and a divisor is meaningless to anything but the rendering of the queue. Both are optional and both are validated at load, since a non-positive divisor or a negative cap would produce a nonsense order rather than an obvious error. Because it carries app options like the viewers and not just agents, the file is named `voro.toml`. A missing file is not an error; the built-ins alone are a working config, so a fresh install with `claude` and an editor on PATH both dispatches and reviews without any TOML. `voro agent list` shows the effective set with each agent's provenance — built-in, user, or user-override — and warns when a user override of a built-in drops verbs the built-in defined, the one staleness case layering cannot fix; `voro viewer list` does the same for viewers, flagging the default. +Agent definitions are command templates, not state, so they live outside the database. Voro *owns* the common ones — `claude` and `codex` are compiled into `voro-core`, so they version with the binary and every upgrade carries the current verb set (the session verbs of §8) to every install with no re-init. The user's `~/.config/voro/voro.toml` is then layered on top and is for extensions, overrides, and app options: it may add a new agent, replace a built-in wholesale (a `[agents.claude]` table overrides the built-in claude *entirely*, not per-verb — predictable over a partial merge), and set `default_agent` and the viewers. Viewers are command templates too, and live in the same file for the same reason — and Voro owns the common ones exactly as it owns the common agents: `code`, `cursor` and `zed` are compiled in and probed against PATH in that order, so a fresh install with any editor CLI installed opens a task's checkout with no configuration at all, which is what the review step of a first session needs. A user `[viewers.]` table then layers on top: named for a built-in it replaces that built-in wholesale, named for anything else it adds a viewer. The built-ins take `{path}` alone rather than a diff range, because that is the shape they can honour — an editor cannot open `{base}...{branch}` from its command line — and because a viewer is spawned detached with no terminal (§8), which is also why no built-in is a pager-driven command like `git difftool -d`: it would have nothing to draw on. `default_viewer` names the one used when nothing picks a viewer by name, and the older single anonymous `[viewer]` table stays valid as that default (a sole named viewer also serves as the default without being named). Resolution therefore runs user-first and probe-last: with a name, the user's table for it, else the built-in of that name; without one, `default_viewer`, else the anonymous `[viewer]`, else the sole named table, else the first built-in found on PATH. When even that finds nothing, what the operator is asked for is to *register the viewer they already use* — `voro viewer add ''` — not to install one of Voro's; the probed built-ins follow as diagnosis, after the action, so what to do is what reads first (the status line wraps rather than truncating, §9, so the diagnosis is not paid for in lost advice). The failure never reports the config file as invalid, since on a fresh install there is no file to be invalid. In the TUI it is not reported at all but answered: `o` with nothing resolving raises the add-viewer form itself (§5), because the operator is two fields away from the diff they asked for and the Config screen would only ask for the same two. Saving does not then open the task — pressing `o` again does — so the key keeps doing one thing. Which viewer a *project* uses is state, so it lives in the database (`projects.viewer`, §8), naming one of these templates — which, since the review keys split (§8), is all that setting decides, and is why the column holds a viewer name and nothing else. A viewer command carries up to three optional placeholders (§8): `{path}` — the task's worktree, or the project checkout when it has none — plus `{branch}` (the task's branch, empty when it has none) and `{base}` (the checkout's default branch), so `{base}...{branch}` spells the review diff's range rather than opening a bare directory. An agent table may also carry a small **model map** beside its verbs — `model`, `model_deep`, and `model_plan` — whose values fill the `{model}` placeholder in the `dispatch` and `plan` templates (§8). They are plain strings, opaque to Voro, which is why they live in the same file as the templates they are pasted into rather than in the schema: the model is part of how a command is spelled, not state about a task. Two further placeholders in those templates are filled from the launch rather than from this file: `{session_name}`, the name Voro composes for the session a launch opens, and `{task_id}`, the task's numeric id. Like `{model}` they are meaningful only where a command starts work, so both are refused on the session verbs, and `{task_id}` on `plan` as well, whose target may be a project with no task to name (§8). It also carries the queue's two pricing options — `max_running`, the dispatch WIP cap, and a `[costs]` table overriding the per-action attention divisors (§7) — for the same reason the viewers live here: they are operator preference about how the tool behaves, not state about a task, and a divisor is meaningless to anything but the rendering of the queue. Both are optional and both are validated at load, since a non-positive divisor or a negative cap would produce a nonsense order rather than an obvious error. Because it carries app options like the viewers and not just agents, the file is named `voro.toml`. A missing file is not an error; the built-ins alone are a working config, so a fresh install with `claude` and an editor on PATH both dispatches and reviews without any TOML. `voro agent list` shows the effective set with each agent's provenance — built-in, user, or user-override — names the optional verbs each agent defines, and warns when a user override of a built-in drops verbs the built-in defined, the one staleness case layering cannot fix. The listing and that warning read one roster of the optional verbs, so no agent can be listed as lacking a verb the line below it says was dropped — the failure the two had while the listing named a hand-written subset of them. Where the row says more than presence it is because the verb's *spelling* changes what Voro can do with it: a `message` carrying `{new_session}` reads `message(fork)`, since forking is the difference between an agent Voro can steer while a supervisor holds the session and one it cannot, and it is what moves the session reference the row afterwards addresses (below). `voro viewer list` does the same for viewers, flagging the default. The file is no longer read-only to Voro. The TUI's Config screen (§9) and the `voro viewer add`/`viewer remove` verbs *edit* it in place — adding, changing, and deleting `[viewers.]` tables and setting `default_viewer`/`default_agent` — through a single write helper (`voro-core::config_edit`) built on `toml_edit`, so a machine write preserves the file's existing content, formatting, and comments and touches only the key it changes. A missing file is created on first edit. What a viewer *is* asks one thing of the operator that a first-time one cannot answer — the command line, `zed {path}` or `code -n {path}`, where the name of the editor is the easy half and the placeholder is not — so the command is optional at both surfaces and defaults to ` {path}`, which is what nearly every editor CLI wants. Naming a *built-in* defaults to that built-in's own command instead, so overriding one starts from what it replaces rather than from a worse guess at the same thing, which is what makes `a` the answer to `e` being refused on a built-in row. In the form the command does not merely default but *follows*: it is rewritten from the name on every keystroke, so the operator watches the line they are about to save assemble itself instead of reading a hint about it. Writing in the command field takes it over — the first character replaces the suggestion whole rather than landing on the end of a line nobody typed, and backspace leaves a suggestion alone, since there is nothing there the operator put — and deleting what they wrote back to empty hands it to the name again, which is the undo. The two states are told apart on sight rather than in words: a following command is dim, focused or not. An edit never follows; that command exists and is theirs. The user-owned surface is all that is writable this way: agents stay read-only in the TUI (editing a built-in means writing a wholesale override table, a sharper knife deferred here), and deleting a viewer that a project's `viewer` still names is refused with the projects named, while deleting the default viewer clears `default_viewer`. A built-in viewer is read-only for the same reason an agent is — it lives in the binary, not the file — so the Config screen lists the built-ins beside the user's tables with their provenance but refuses to edit or delete one, and `voro viewer remove code` says the same, naming the *add* of that name that overrides it. Choosing one is not writing one, so both a `default_viewer` and a project's own viewer may name a built-in with no table defining it. diff --git a/docs/agent-integration.md b/docs/agent-integration.md index dee9941..c707ecc 100644 --- a/docs/agent-integration.md +++ b/docs/agent-integration.md @@ -147,7 +147,9 @@ resume = "codex resume {session}" 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. + it names the session a send opens, and nothing else opens one. Which of the + two a configured agent got is visible without reading the file back: `voro + agent list` names a forking send `message(fork)` and a resuming one `message`. - A `message` template should carry whatever permission flag its agent's `dispatch` carries — the built-in `claude` one carries `--permission-mode auto`. A resumed turn does real work, and on agents where the flag is per