diff --git a/docs/config-reference.md b/docs/config-reference.md index b8e0ff0..4af091b 100644 --- a/docs/config-reference.md +++ b/docs/config-reference.md @@ -111,3 +111,37 @@ are `source`, `dest`, `trigger`, `extract`, and `prefix`. Local LLMs require `endpoint` and `model`; `system` and `api_key` are optional. + +- `tool_dedup_secs` (default `45`) — repeat-call suppression window for the orchestrator's tools. `0` disables. + +## `[[personas]]` + +Named behavioural presets layered over `[orchestrator]`. Every field is +optional; omitted fields inherit from `[orchestrator]`. Set +`[orchestrator].persona` to pick the one applied at startup, or switch at +runtime with `/persona ` (history is preserved). + +Personas modulate autonomy and eagerness only. Repeat-call suppression, +tool-result elision stubs and `send_input` evidence are unconditional — a +persona cannot turn correctness off. + +- `name` — the name used by `/persona`. Matching a builtin replaces it. +- `events`, `event_cooldown_secs` — which session transitions wake the agent, and how often. +- `approval`, `auto_approve` — propose-mode gating. +- `allowed_tools` — tools present in the schema at all. `list_sessions` is always kept. +- `max_tool_iterations`, `tool_dedup_secs`, `max_context_tokens`, `event_tail_lines`. +- `note` — appended to the system prompt under a `## Persona` heading. + +Builtins: `assistant` (reactive, read-only, propose), `monitor` (watches and +reports, writes gated), `orchestrator` (acts autonomously, tightest dedup +window). + +```toml +[orchestrator] +persona = "monitor" + +[[personas]] +name = "monitor" +event_cooldown_secs = 90 +note = "Be terse. One report per incident." +``` diff --git a/src/app.rs b/src/app.rs index 83e0974..b1c12a2 100644 --- a/src/app.rs +++ b/src/app.rs @@ -506,6 +506,11 @@ pub struct App { /// Live progress of the orchestrator's current turn, plus when it was /// set (drives the chat-pane spinner). None while idle. pub orchestrator_status: Option<(String, std::time::Instant)>, + /// Name of the active persona (empty = bare [orchestrator] config). + pub orchestrator_persona: String, + /// Pristine [orchestrator] config. Personas layer over *this*, never over + /// an already-layered config, so swapping A -> B -> A is idempotent. + pub orchestrator_base: Option, /// Per-(session_id, state label) cooldown for proactive orchestrator events orch_event_cooldowns: HashMap<(usize, &'static str), std::time::Instant>, /// Session behind the most recent permission request surfaced in chat; @@ -604,6 +609,8 @@ impl App { orchestrator_stats: crate::session::TokenStats::default(), orchestrator_ctx_max: None, orchestrator_status: None, + orchestrator_persona: String::new(), + orchestrator_base: None, orch_event_cooldowns: HashMap::new(), last_permission_request: None, chat: ChatState::default(), @@ -2310,6 +2317,29 @@ impl App { anyhow::bail!("orchestrator already running"); } self.orchestrator_paused = false; + // Remember the unlayered config once, then apply the configured + // default persona (if any) before anything reads cfg. + // Not get_or_insert_with: the closure would borrow self while + // orchestrator_base is already mutably borrowed. + let base = match &self.orchestrator_base { + Some(b) => b.clone(), + None => { + let b = self.config.orchestrator.clone(); + self.orchestrator_base = Some(b.clone()); + b + } + }; + let want = base.persona.clone(); + if !want.is_empty() && self.orchestrator_persona != want { + if let Some(p) = self.personas().into_iter().find(|p| p.name == want) { + let mut cfg = (*self.config).clone(); + cfg.orchestrator = p.apply(&base); + self.config = Arc::new(cfg); + self.orchestrator_persona = want; + } else { + self.chat_system(format!("unknown persona \"{}\" in config; ignoring", want)); + } + } let cfg = self.config.orchestrator.clone(); match cfg.class()? { crate::config::OrchestratorClass::Api(_) => { @@ -2472,8 +2502,36 @@ impl App { .insert(session_id, (response_tx, line_offset)); return; // reply arrives via check_ipc_replies on READY } - s.write_bytes(shape_injected_input(&text)); - serde_json::json!({"ok": true}) + // Fire-and-forget, but never a bare {"ok": true}: an + // unconditional success is indistinguishable from a send into + // a paused or dead session, so the model cannot tell a no-op + // from a real action and retries. Report what is knowable + // synchronously and point at wait_ready for the rest. + if s.paused { + serde_json::json!({ + "error": "session is paused (SIGSTOP); resume_session first — \ + input would sit unread in the terminal buffer", + "session_id": session_id + }) + } else if s.state_label().eq_ignore_ascii_case("dead") { + serde_json::json!({ + "error": "session is dead; input was not sent", + "session_id": session_id + }) + } else { + let before = s.output_lines.len(); + s.write_bytes(shape_injected_input(&text)); + serde_json::json!({ + "ok": true, + "session_id": session_id, + "state_at_send": s.state_label(), + "output_lines_at_send": before, + "note": "input was written to the terminal; this is not \ + confirmation it was accepted or acted on. Use \ + wait_ready=true, or read_output later and compare \ + against output_lines_at_send, to verify." + }) + } } OrchestratorReq::PipeAdd { source, @@ -3771,6 +3829,8 @@ impl App { .to_string() }; } + ["persona"] => self.report_personas(), + ["persona", name] => self.set_persona(name), ["confirm-kill"] => self.resolve_pending_kill(true), ["deny-kill"] => self.resolve_pending_kill(false), ["interrupt"] | ["stop"] => self.interrupt_orchestrator(), @@ -3785,6 +3845,78 @@ impl App { } } + /// All personas: user-defined entries in [[personas]] shadow builtins of + /// the same name. + fn personas(&self) -> Vec { + let mut out = crate::config::builtin_personas(); + for p in &self.config.personas { + match out.iter_mut().find(|b| b.name == p.name) { + Some(slot) => *slot = p.clone(), + None => out.push(p.clone()), + } + } + out + } + + fn report_personas(&mut self) { + let names: Vec = self + .personas() + .iter() + .map(|p| { + if p.name == self.orchestrator_persona { + format!("{}*", p.name) + } else { + p.name.clone() + } + }) + .collect(); + self.command_result = format!("personas: {} (* = active)", names.join(", ")); + } + + /// Swap the active persona. History is preserved — a persona changes how + /// the agent behaves from here, not what it knows. In-flight tool calls + /// complete under the old config; the allowlist applies from the next turn. + fn set_persona(&mut self, name: &str) { + let Some(persona) = self.personas().into_iter().find(|p| p.name == name) else { + self.command_result = format!("unknown persona \"{}\"; try /persona", name); + return; + }; + // Not get_or_insert_with: the closure would borrow self while + // orchestrator_base is already mutably borrowed. + let base = match &self.orchestrator_base { + Some(b) => b.clone(), + None => { + let b = self.config.orchestrator.clone(); + self.orchestrator_base = Some(b.clone()); + b + } + }; + let cfg = persona.apply(&base); + let Some(h) = &self.orchestrator else { + // No API orchestrator running: still record it so a later + // /orchestrator start picks the persona up. + let mut config = (*self.config).clone(); + config.orchestrator = cfg; + self.config = Arc::new(config); + self.orchestrator_persona = persona.name.clone(); + self.command_result = format!("persona set to {} (takes effect on start)", name); + return; + }; + let tx = h.tx.clone(); + let boxed = Box::new(cfg.clone()); + let mut config = (*self.config).clone(); + config.orchestrator = cfg; + self.config = Arc::new(config); + self.orchestrator_persona = persona.name.clone(); + tokio::spawn(async move { + let _ = tx + .send(crate::orchestrator::OrchestratorMsg::SetConfig(boxed)) + .await; + }); + self.chat_system(format!("persona → {}", name)); + self.command_result = format!("persona set to {}", name); + } + /// Break the orchestrator out of its current turn (/interrupt, /stop). /// The turn stops at the next safe point: immediately if it's blocked in /// a tool call, otherwise before the next tool iteration. diff --git a/src/config.rs b/src/config.rs index d23d49c..7ac4097 100644 --- a/src/config.rs +++ b/src/config.rs @@ -22,6 +22,9 @@ pub struct Config { pub orchestrator: OrchestratorConfig, pub chat: ChatConfig, pub profiles: Vec, + /// User-defined personas; a name matching a builtin replaces it. + #[serde(default)] + pub personas: Vec, } // ── [orchestrator] ──────────────────────────────────────────────────────── @@ -112,6 +115,20 @@ pub struct OrchestratorConfig { /// Lines of session output inlined into a [linkshell event] /// notification. The orchestrator can always read_output for more. pub event_tail_lines: usize, + /// Tool names the orchestrator may call. Empty means the full set. This + /// is the mechanical half of a persona: a system-prompt instruction to be + /// cautious is a suggestion to a small local model, whereas omitting + /// `send_input` from the schema is a guarantee. + pub allowed_tools: Vec, + /// Extra text appended to the system prompt by the active persona. + pub persona_note: String, + /// Name of the active persona (informational; shown in the status row). + pub persona: String, + /// Seconds during which an identical (tool, arguments) call is answered + /// with a duplicate_call error instead of being re-executed. Bounds the + /// cross-turn loops that max_tool_iterations (per-turn) cannot see. + /// 0 disables. + pub tool_dedup_secs: u64, /// Cap on lines returned by send_input wait_ready / `input --wait`. /// Longer replies are truncated to the last N lines with a marker. /// 0 disables. @@ -156,6 +173,10 @@ impl Default for OrchestratorConfig { max_context_tokens: 60_000, tool_result_keep_turns: 3, event_tail_lines: 5, + allowed_tools: Vec::new(), + persona_note: String::new(), + persona: String::new(), + tool_dedup_secs: 45, wait_ready_max_lines: 80, } } @@ -400,6 +421,136 @@ impl Default for NotificationsConfig { } } +/// A named behavioural preset layered over `[orchestrator]`. +/// +/// Personas modulate *autonomy and eagerness*, not correctness: the loop +/// suppressor, the elision stubs and the send_input evidence are +/// unconditional. Every field is optional, and `None` means "inherit from +/// `[orchestrator]`" — an explicit setting there still wins unless the +/// persona overrides it. +/// +/// [[personas]] +/// name = "assistant" +/// events = [] +/// approval = "propose" +/// allowed_tools = ["list_sessions", "read_output", "use_skill", "remember"] +/// max_tool_iterations = 4 +/// tool_dedup_secs = 300 +/// note = "You observe and advise. You do not drive sessions." +#[derive(serde::Deserialize, serde::Serialize, Clone, Debug, Default)] +#[serde(default)] +pub struct Persona { + pub name: String, + pub events: Option>, + pub event_cooldown_secs: Option, + pub approval: Option, + pub auto_approve: Option>, + pub allowed_tools: Option>, + pub max_tool_iterations: Option, + pub tool_dedup_secs: Option, + pub max_context_tokens: Option, + pub event_tail_lines: Option, + /// Appended to the system prompt. + pub note: String, +} + +impl Persona { + /// Layer this persona over a base orchestrator config. + pub fn apply(&self, base: &OrchestratorConfig) -> OrchestratorConfig { + let mut cfg = base.clone(); + if let Some(v) = &self.events { + cfg.events = v.clone(); + } + if let Some(v) = self.event_cooldown_secs { + cfg.event_cooldown_secs = v; + } + if let Some(v) = &self.approval { + cfg.approval = v.clone(); + } + if let Some(v) = &self.auto_approve { + cfg.auto_approve = v.clone(); + } + if let Some(v) = &self.allowed_tools { + cfg.allowed_tools = v.clone(); + } + if let Some(v) = self.max_tool_iterations { + cfg.max_tool_iterations = v; + } + if let Some(v) = self.tool_dedup_secs { + cfg.tool_dedup_secs = v; + } + if let Some(v) = self.max_context_tokens { + cfg.max_context_tokens = v; + } + if let Some(v) = self.event_tail_lines { + cfg.event_tail_lines = v; + } + cfg.persona_note = self.note.clone(); + cfg.persona = self.name.clone(); + cfg + } +} + +/// The three shipped personas, used when no `[[personas]]` entry matches. +/// Ordered by autonomy: assistant looks, monitor reports, orchestrator acts. +pub fn builtin_personas() -> Vec { + let read_only = vec![ + "list_sessions".to_string(), + "read_output".to_string(), + "use_skill".to_string(), + "remember".to_string(), + ]; + vec![ + Persona { + name: "assistant".into(), + events: Some(Vec::new()), + approval: Some("propose".into()), + allowed_tools: Some(read_only.clone()), + max_tool_iterations: Some(4), + tool_dedup_secs: Some(300), + note: "You are a reactive assistant. You answer when spoken to. You can \ + inspect sessions but cannot drive them; if something needs doing, \ + say so and let the user do it." + .into(), + ..Default::default() + }, + Persona { + name: "monitor".into(), + events: Some(vec!["waiting".into(), "error".into(), "dead".into()]), + event_cooldown_secs: Some(60), + approval: Some("propose".into()), + allowed_tools: None, // full set, but writes are gated by propose + auto_approve: Some(read_only), + max_tool_iterations: Some(8), + tool_dedup_secs: Some(120), + note: "You watch sessions and report. Investigate freely with read-only \ + tools; anything that changes a session is proposed for approval \ + first. Prefer one clear report over a stream of updates." + .into(), + ..Default::default() + }, + Persona { + name: "orchestrator".into(), + events: Some(vec![ + "ready".into(), + "waiting".into(), + "error".into(), + "dead".into(), + ]), + event_cooldown_secs: Some(15), + approval: Some("auto".into()), + max_tool_iterations: Some(12), + tool_dedup_secs: Some(45), + note: "You actively route work between sessions. Act without asking for \ + routine steps. Before repeating an action, check whether the \ + previous one had an effect; if you cannot tell, say so rather \ + than trying again." + .into(), + ..Default::default() + }, + ] +} + #[derive(serde::Deserialize, serde::Serialize, Clone, Debug)] pub struct Profile { pub name: String, diff --git a/src/events.rs b/src/events.rs index bda40e9..c1a6ae3 100644 --- a/src/events.rs +++ b/src/events.rs @@ -186,6 +186,9 @@ pub enum AppEvent { /// pane ("thinking (3/12)", "running read_output", ...). None clears it /// when the turn ends. OrchestratorStatus(Option), + /// The orchestrator task acknowledged a persona swap (name, or empty for + /// the bare [orchestrator] config). + OrchestratorPersona(String), /// Token usage from the orchestrator's own API calls OrchestratorUsage { input: u64, diff --git a/src/main.rs b/src/main.rs index bc94475..270f26d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -672,6 +672,10 @@ fn handle_event(app: &mut App, event: AppEvent) { } => { app.handle_orchestrator_proposal(tool, detail, response_tx); } + AppEvent::OrchestratorPersona(name) => { + app.orchestrator_persona = name; + app.needs_redraw = true; + } AppEvent::OrchestratorStatus(status) => { app.orchestrator_status = status.map(|s| (s, std::time::Instant::now())); app.needs_redraw = true; diff --git a/src/orchestrator/anthropic.rs b/src/orchestrator/anthropic.rs index 88862a6..5ee2fe5 100644 --- a/src/orchestrator/anthropic.rs +++ b/src/orchestrator/anthropic.rs @@ -37,6 +37,7 @@ pub async fn run_turn( cfg: &OrchestratorConfig, client: &reqwest::Client, history: &mut Vec, + calls: &mut super::CallLog, user_text: &str, event_tx: &mpsc::Sender, interrupt: &mut super::Interrupt, @@ -52,7 +53,7 @@ pub async fn run_turn( // a moving breakpoint on the last message of each request so the 12 // iterations of a busy turn re-serve the shared history prefix instead // of re-billing it in full every call. - let mut tools = super::anthropic_tools(); + let mut tools = super::anthropic_tools(cfg); if let Some(last) = tools.as_array_mut().and_then(|a| a.last_mut()) { last["cache_control"] = serde_json::json!({"type": "ephemeral"}); } @@ -136,7 +137,7 @@ pub async fn run_turn( super::INTERRUPTED_RESULT.to_string() } else { tokio::select! { - r = super::exec_tool(cfg, event_tx, name, &b["input"]) => r, + r = super::exec_tool(cfg, event_tx, calls, name, &b["input"]) => r, _ = interrupt.wait() => { interrupted = true; super::INTERRUPTED_RESULT.to_string() diff --git a/src/orchestrator/mod.rs b/src/orchestrator/mod.rs index f9b5306..61e28ca 100644 --- a/src/orchestrator/mod.rs +++ b/src/orchestrator/mod.rs @@ -67,6 +67,68 @@ impl Interrupt { } } +/// Cross-turn repeat-call suppressor. +/// +/// `max_tool_iterations` bounds a single turn; nothing bounds the *sequence* +/// of turns, so a flapping session can drive the model through the same tool +/// call indefinitely. The log remembers recent (name, args) pairs and turns +/// an exact repeat into a tool result that says so — which is information the +/// model can act on, rather than a silent loop. +pub(crate) struct CallLog { + seen: Vec<(u64, std::time::Instant)>, + window: std::time::Duration, +} + +impl CallLog { + pub(crate) fn new(window_secs: u64) -> Self { + Self { + seen: Vec::new(), + window: std::time::Duration::from_secs(window_secs), + } + } + + /// Re-window on a persona swap (autonomous personas legitimately re-read + /// more often; the window narrows, it never reaches zero unless the user + /// asks for that explicitly). + pub(crate) fn set_window(&mut self, secs: u64) { + self.window = std::time::Duration::from_secs(secs); + } + + /// Drop the history (used by /reset, so a fresh context is not haunted by + /// the calls of the previous one). + pub(crate) fn clear(&mut self) { + self.seen.clear(); + } + + fn key(name: &str, args: &serde_json::Value) -> u64 { + use std::hash::{Hash, Hasher}; + let mut h = std::collections::hash_map::DefaultHasher::new(); + name.hash(&mut h); + // Serialized form: serde_json preserves insertion order, and both + // providers hand us arguments the model just generated, so identical + // calls serialize identically in practice. + args.to_string().hash(&mut h); + h.finish() + } + + /// Record a call. Returns Some(age) when the identical call was already + /// made inside the window. + fn check(&mut self, name: &str, args: &serde_json::Value) -> Option { + if self.window.is_zero() { + return None; + } + let now = std::time::Instant::now(); + self.seen + .retain(|(_, t)| now.duration_since(*t) < self.window); + let key = Self::key(name, args); + if let Some((_, t)) = self.seen.iter().find(|(k, _)| *k == key) { + return Some(now.duration_since(*t)); + } + self.seen.push((key, now)); + None + } +} + /// Tool result injected for calls cut short by /interrupt. const INTERRUPTED_RESULT: &str = "{\"error\": \"interrupted by user\"}"; /// Turn text surfaced in the chat pane after an interrupt. @@ -86,6 +148,10 @@ pub enum OrchestratorMsg { }, /// Out-of-band note (kill approved/denied, etc.). SystemNote(String), + /// Swap the active persona (config layer) without dropping history. + /// In-flight tool calls finish under the old config; the new one applies + /// from the next turn. + SetConfig(Box), /// Drop the conversation history (/reset). Anything queued before the /// reset is discarded with it; messages after it start the fresh context. Reset, @@ -115,7 +181,7 @@ impl OrchestratorMsg { ) } OrchestratorMsg::SystemNote(note) => format!("[linkshell] {}", note), - OrchestratorMsg::Reset => String::new(), + OrchestratorMsg::SetConfig(_) | OrchestratorMsg::Reset => String::new(), } } @@ -166,6 +232,7 @@ fn coalesce_batch( /// Spawn the orchestrator task. Only valid for API-class providers. pub fn spawn(cfg: OrchestratorConfig, event_tx: mpsc::Sender) -> OrchestratorHandle { + let mut cfg = cfg; cfg.ensure_agent_files(); let (tx, mut rx) = mpsc::channel::(64); let (interrupt_tx, interrupt_rx) = tokio::sync::watch::channel(0u64); @@ -175,6 +242,7 @@ pub fn spawn(cfg: OrchestratorConfig, event_tx: mpsc::Sender) -> Orche // Provider-native message history (anthropic and openai shapes differ, // but both are serde_json Values in a flat Vec). let mut history: Vec = Vec::new(); + let mut calls = CallLog::new(cfg.tool_dedup_secs); while let Some(first) = rx.recv().await { // Coalesce whatever queued up while we were idle or mid-turn into // a single user turn — an event storm becomes one API call. @@ -182,7 +250,32 @@ pub fn spawn(cfg: OrchestratorConfig, event_tx: mpsc::Sender) -> Orche while let Ok(next) = rx.try_recv() { msgs.push(next); } + // Persona swaps are config, not conversation: apply the last one + // in the batch and drop them before the batch becomes a turn. + let mut new_cfg = None; + msgs.retain(|m| match m { + OrchestratorMsg::SetConfig(c) => { + new_cfg = Some((**c).clone()); + false + } + _ => true, + }); + if let Some(c) = new_cfg { + let window = c.tool_dedup_secs; + cfg = c; + calls.set_window(window); + let _ = event_tx + .send(AppEvent::OrchestratorPersona(cfg.persona.clone())) + .await; + } + if msgs.is_empty() { + continue; + } + let had_reset = msgs.iter().any(|m| matches!(m, OrchestratorMsg::Reset)); let (user_text, any_user, any_actionable) = coalesce_batch(&msgs, &mut history); + if had_reset { + calls.clear(); + } let Some(user_text) = user_text else { // Pure reset, nothing to say to the model. continue; @@ -200,6 +293,7 @@ pub fn spawn(cfg: OrchestratorConfig, event_tx: mpsc::Sender) -> Orche &cfg, &client, &mut history, + &mut calls, &user_text, &event_tx, &mut interrupt, @@ -211,6 +305,7 @@ pub fn spawn(cfg: OrchestratorConfig, event_tx: mpsc::Sender) -> Orche &cfg, &client, &mut history, + &mut calls, &user_text, &event_tx, &mut interrupt, @@ -261,6 +356,22 @@ pub fn spawn(cfg: OrchestratorConfig, event_tx: mpsc::Sender) -> Orche /// One logical tool set; converted per provider wire format below. /// (name, description, JSON Schema for the arguments) +/// Filter the tool set by the active persona's allowlist. An empty allowlist +/// means the full set; `list_sessions` is always kept so the model is never +/// completely blind. +fn allowed_specs(cfg: &OrchestratorConfig) -> Vec<(&'static str, &'static str, serde_json::Value)> { + let specs = tool_specs(); + if cfg.allowed_tools.is_empty() { + return specs; + } + specs + .into_iter() + .filter(|(name, _, _)| { + *name == "list_sessions" || cfg.allowed_tools.iter().any(|a| a == name) + }) + .collect() +} + fn tool_specs() -> Vec<(&'static str, &'static str, serde_json::Value)> { vec![ ( @@ -402,8 +513,8 @@ const EXHAUSTION_NUDGE: &str = "[linkshell] Tool iteration budget for this turn Do not call any more tools. Summarize what you did, what you learned from the tool results \ above, and what (if anything) remains to be done."; -fn anthropic_tools() -> serde_json::Value { - tool_specs() +fn anthropic_tools(cfg: &OrchestratorConfig) -> serde_json::Value { + allowed_specs(cfg) .into_iter() .map(|(name, desc, schema)| { serde_json::json!({"name": name, "description": desc, "input_schema": schema}) @@ -412,8 +523,8 @@ fn anthropic_tools() -> serde_json::Value { } /// OpenAI `tools` array shape. -fn openai_tools() -> serde_json::Value { - tool_specs() +fn openai_tools(cfg: &OrchestratorConfig) -> serde_json::Value { + allowed_specs(cfg) .into_iter() .map(|(name, desc, schema)| { serde_json::json!({ @@ -474,9 +585,42 @@ fn proposal_detail(name: &str, args: &serde_json::Value) -> String { async fn exec_tool( cfg: &OrchestratorConfig, event_tx: &mpsc::Sender, + calls: &mut CallLog, name: &str, args: &serde_json::Value, ) -> String { + if !cfg.allowed_tools.is_empty() + && name != "list_sessions" + && !cfg.allowed_tools.iter().any(|a| a == name) + { + return serde_json::json!({ + "error": "tool_not_available", + "detail": format!( + "the active persona ({}) does not have {}. Available: {}.", + if cfg.persona.is_empty() { "default" } else { &cfg.persona }, + name, + cfg.allowed_tools.join(", ") + ), + }) + .to_string(); + } + // Repeat-call suppression, before the approval gate: an identical call + // inside the window is answered rather than executed, so the user is not + // asked to approve the same proposal twice either. + if let Some(age) = calls.check(name, args) { + return serde_json::json!({ + "error": "duplicate_call", + "detail": format!( + "you already called {} with these exact arguments {}s ago and the result is \ + above in this conversation. Nothing was re-run. If you are waiting for a \ + session to change, use send_input with wait_ready=true, or tell the user \ + what you are blocked on.", + name, + age.as_secs() + ), + }) + .to_string(); + } // Propose mode: gated tools block here until the human answers in the // chat pane (/approve, /deny [reason]) or the timeout fires. Only the // orchestrator's own tokio task waits — no HTTP request is held open and @@ -699,6 +843,13 @@ and continue; otherwise report what you wanted to do and why.\n", if let Some(memory) = memory_section(cfg) { p.push_str(&memory); } + // Persona note last of the static text (before memory), so a persona + // swap invalidates as little of the cached prefix as possible. + if !cfg.persona_note.trim().is_empty() { + p.push_str("\n\n## Persona\n\n"); + p.push_str(cfg.persona_note.trim()); + p.push('\n'); + } p } @@ -813,10 +964,71 @@ fn skills_section(cfg: &OrchestratorConfig, with_paths: bool) -> Option Some(skills::skill_list(&list, with_paths)) } -/// Stub text substituted for aged-out tool results. +/// Stub text substituted for aged-out tool results whose originating call +/// could not be identified. Also the minimum length worth eliding. const ELIDED_RESULT: &str = "[elided to save context — re-run the tool if this result is still needed]"; +/// Map tool-call id -> (tool name, arguments) across both provider shapes. +/// +/// Aged-out results are replaced by a stub that *names the call it answered*. +/// A bare "[elided]" leaves the model reading its own `tool_use` block +/// followed by nothing, which reads as an unfinished action and invites an +/// immediate re-call; naming the call turns it into a completed one. +fn call_index( + history: &[serde_json::Value], +) -> std::collections::HashMap { + let mut map = std::collections::HashMap::new(); + for m in history.iter().filter(|m| m["role"] == "assistant") { + // Anthropic: content array of blocks, tool_use carries id/name/input. + if let Some(blocks) = m["content"].as_array() { + for b in blocks.iter().filter(|b| b["type"] == "tool_use") { + if let Some(id) = b["id"].as_str() { + map.insert( + id.to_string(), + ( + b["name"].as_str().unwrap_or("tool").to_string(), + b["input"].clone(), + ), + ); + } + } + } + // OpenAI: tool_calls array, arguments is a JSON-encoded string. + if let Some(calls) = m["tool_calls"].as_array() { + for c in calls { + if let Some(id) = c["id"].as_str() { + let args: serde_json::Value = c["function"]["arguments"] + .as_str() + .and_then(|a| serde_json::from_str(a).ok()) + .unwrap_or(serde_json::json!({})); + map.insert( + id.to_string(), + ( + c["function"]["name"].as_str().unwrap_or("tool").to_string(), + args, + ), + ); + } + } + } + } + map +} + +/// Stub naming the call whose result was dropped, with the original size. +fn elision_stub(call: Option<&(String, serde_json::Value)>, original_len: usize) -> String { + match call { + Some((name, args)) => format!( + "[elided: {} {} returned {} chars — re-run only if you still need the detail]", + name, + proposal_detail(name, args), + original_len + ), + None => ELIDED_RESULT.to_string(), + } +} + /// Count plain user-text turns (the boundaries trim_history cuts at). fn user_turns(history: &[serde_json::Value]) -> usize { history @@ -842,6 +1054,7 @@ fn age_tool_results(history: &mut [serde_json::Value], keep_turns: usize) { if keep_turns == 0 { return; } + let calls = call_index(history); // Index of the keep_turns-th plain user turn from the end; everything // before it is "old". let mut seen = 0; @@ -861,11 +1074,10 @@ fn age_tool_results(history: &mut [serde_json::Value], keep_turns: usize) { for m in history[..boundary].iter_mut() { // OpenAI shape: {"role": "tool", "content": "..."} if m["role"] == "tool" { - if m["content"] - .as_str() - .is_some_and(|s| s.len() > ELIDED_RESULT.len()) - { - m["content"] = serde_json::json!(ELIDED_RESULT); + let len = m["content"].as_str().map_or(0, |s| s.len()); + if len > ELIDED_RESULT.len() { + let call = m["tool_call_id"].as_str().and_then(|id| calls.get(id)); + m["content"] = serde_json::json!(elision_stub(call, len)); } continue; } @@ -874,12 +1086,13 @@ fn age_tool_results(history: &mut [serde_json::Value], keep_turns: usize) { if m["role"] == "user" { if let Some(blocks) = m["content"].as_array_mut() { for b in blocks.iter_mut().filter(|b| b["type"] == "tool_result") { - let long = match &b["content"] { - serde_json::Value::String(s) => s.len() > ELIDED_RESULT.len(), - v => v.to_string().len() > ELIDED_RESULT.len(), + let len = match &b["content"] { + serde_json::Value::String(s) => s.len(), + v => v.to_string().len(), }; - if long { - b["content"] = serde_json::json!(ELIDED_RESULT); + if len > ELIDED_RESULT.len() { + let call = b["tool_use_id"].as_str().and_then(|id| calls.get(id)); + b["content"] = serde_json::json!(elision_stub(call, len)); } } } @@ -946,8 +1159,9 @@ mod tests { #[test] fn tool_specs_convert_to_both_provider_shapes() { - let a = anthropic_tools(); - let o = openai_tools(); + let cfg = OrchestratorConfig::default(); + let a = anthropic_tools(&cfg); + let o = openai_tools(&cfg); let n = tool_specs().len(); assert_eq!(a.as_array().unwrap().len(), n); assert_eq!(o.as_array().unwrap().len(), n); @@ -986,6 +1200,7 @@ mod tests { let out = exec_tool( &cfg, &tx, + &mut CallLog::new(0), "remember", &serde_json::json!({"text": "user prefers rebase\nover merge"}), ) @@ -1025,6 +1240,7 @@ mod tests { exec_tool( &cfg, &tx, + &mut CallLog::new(0), "send_input", &serde_json::json!({"session_id": 2, "text": "cargo test"}), ) @@ -1086,7 +1302,14 @@ mod tests { let (tx, mut rx) = mpsc::channel::(8); // use_skill with no skills dir: fails fast without a main-loop trip, // but the status announcement must still come first. - let _ = exec_tool(&cfg, &tx, "use_skill", &serde_json::json!({"name": "x"})).await; + let _ = exec_tool( + &cfg, + &tx, + &mut CallLog::new(0), + "use_skill", + &serde_json::json!({"name": "x"}), + ) + .await; match rx.try_recv() { Ok(AppEvent::OrchestratorStatus(Some(s))) => { assert!(s.contains("use_skill"), "status names the tool: {}", s) @@ -1161,7 +1384,9 @@ mod tests { let big = "x".repeat(500); let mut h = vec![ serde_json::json!({"role": "user", "content": "one"}), - serde_json::json!({"role": "assistant", "content": [{"type": "tool_use", "id": "a"}]}), + serde_json::json!({"role": "assistant", "content": [ + {"type": "tool_use", "id": "a", "name": "read_output", + "input": {"session_id": 2}}]}), serde_json::json!({"role": "user", "content": [ {"type": "tool_result", "tool_use_id": "a", "content": big.clone()}]}), serde_json::json!({"role": "assistant", "content": "reply"}), @@ -1172,7 +1397,12 @@ mod tests { ]; age_tool_results(&mut h, 1); // Old result stubbed, id preserved - assert_eq!(h[2]["content"][0]["content"], ELIDED_RESULT); + let stub = h[2]["content"][0]["content"].as_str().unwrap(); + assert!(stub.starts_with("[elided:"), "{stub}"); + // The stub names the call it answered, so the model reads a completed + // action rather than a tool_use followed by nothing. + assert!(stub.contains("read_output"), "{stub}"); + assert!(stub.contains("500 chars"), "{stub}"); assert_eq!(h[2]["content"][0]["tool_use_id"], "a"); // Result within the keep window untouched assert_eq!(h[6]["content"][0]["content"], big); @@ -1182,6 +1412,72 @@ mod tests { assert_eq!(h, before); } + #[test] + fn a_persona_allowlist_shrinks_the_tool_schema() { + let mut cfg = OrchestratorConfig { + allowed_tools: vec!["read_output".into()], + ..Default::default() + }; + let names: Vec<&str> = allowed_specs(&cfg).iter().map(|(n, _, _)| *n).collect(); + assert!(names.contains(&"read_output")); + // Always kept: the model is never left with no way to see anything. + assert!(names.contains(&"list_sessions")); + // Removed from the schema entirely, not merely discouraged. + assert!(!names.contains(&"send_input")); + // Empty allowlist means the full set. + cfg.allowed_tools.clear(); + assert_eq!(allowed_specs(&cfg).len(), tool_specs().len()); + } + + #[test] + fn builtin_personas_layer_over_the_base_config() { + let base = OrchestratorConfig { + model: "local-qwen".into(), + event_cooldown_secs: 30, + ..Default::default() + }; + let personas = crate::config::builtin_personas(); + let assistant = personas.iter().find(|p| p.name == "assistant").unwrap(); + let cfg = assistant.apply(&base); + // Overridden by the persona... + assert!(cfg.events.is_empty()); + assert_eq!(cfg.approval, "propose"); + assert!(!cfg.allowed_tools.contains(&"send_input".to_string())); + // ...while untouched fields are inherited. + assert_eq!(cfg.model, "local-qwen"); + assert_eq!(cfg.persona, "assistant"); + // The most autonomous persona still cannot disable loop suppression. + let orch = personas.iter().find(|p| p.name == "orchestrator").unwrap(); + assert!(orch.apply(&base).tool_dedup_secs > 0); + } + + #[test] + fn repeat_calls_are_suppressed_within_the_window() { + let mut log = CallLog::new(60); + let args = serde_json::json!({"session_id": 2, "lines": 50}); + assert!(log.check("read_output", &args).is_none()); + // Identical call: reported as a repeat rather than executed again. + assert!(log.check("read_output", &args).is_some()); + // Different arguments are a different call. + assert!(log + .check( + "read_output", + &serde_json::json!({"session_id": 3, "lines": 50}) + ) + .is_none()); + // Reset clears the log so a fresh context is not haunted by the old one. + log.clear(); + assert!(log.check("read_output", &args).is_none()); + } + + #[test] + fn a_zero_window_disables_suppression() { + let mut log = CallLog::new(0); + let args = serde_json::json!({}); + assert!(log.check("list_sessions", &args).is_none()); + assert!(log.check("list_sessions", &args).is_none()); + } + #[test] fn aging_stubs_openai_tool_messages() { let big = "y".repeat(500); @@ -1191,6 +1487,7 @@ mod tests { serde_json::json!({"role": "user", "content": "two"}), ]; age_tool_results(&mut h, 1); + // No matching tool_call_id in history: falls back to the bare stub. assert_eq!(h[1]["content"], ELIDED_RESULT); } @@ -1315,6 +1612,7 @@ mod tests { &cfg, &client, &mut history, + &mut CallLog::new(0), "do something", &event_tx, &mut interrupt, @@ -1359,6 +1657,7 @@ mod tests { &cfg, &client, &mut history, + &mut CallLog::new(0), "what's running?", &event_tx, &mut interrupt, @@ -1408,6 +1707,7 @@ mod tests { &cfg, &client, &mut history, + &mut CallLog::new(0), "what's running?", &event_tx, &mut interrupt, diff --git a/src/orchestrator/openai.rs b/src/orchestrator/openai.rs index 373072c..14d3b76 100644 --- a/src/orchestrator/openai.rs +++ b/src/orchestrator/openai.rs @@ -9,6 +9,7 @@ pub async fn run_turn( cfg: &OrchestratorConfig, client: &reqwest::Client, history: &mut Vec, + calls: &mut super::CallLog, user_text: &str, event_tx: &mpsc::Sender, interrupt: &mut super::Interrupt, @@ -18,7 +19,7 @@ pub async fn run_turn( anyhow::bail!("no endpoint configured for provider {}", cfg.provider); } let url = crate::agent_llm::completions_url(&endpoint); - let tools = super::openai_tools(); + let tools = super::openai_tools(cfg); history.push(serde_json::json!({"role": "user", "content": user_text})); super::compact_history(history, cfg); @@ -92,7 +93,7 @@ pub async fn run_turn( super::INTERRUPTED_RESULT.to_string() } else { tokio::select! { - r = super::exec_tool(cfg, event_tx, name, &args) => r, + r = super::exec_tool(cfg, event_tx, calls, name, &args) => r, _ = interrupt.wait() => { interrupted = true; super::INTERRUPTED_RESULT.to_string() diff --git a/src/ui.rs b/src/ui.rs index 3e592b5..243c383 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -1484,7 +1484,17 @@ fn draw_chat_in(f: &mut Frame<'_>, app: &App, popup: Rect, focused: bool) -> Cha const FRAMES: [char; 10] = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏']; let frame = FRAMES[(since.elapsed().as_millis() / 120) as usize % FRAMES.len()]; lines.push(Line::from(Span::styled( - format!("{} {}: {}", frame, app.config.orchestrator.name, status), + format!( + "{} {}{}: {}", + frame, + app.config.orchestrator.name, + if app.orchestrator_persona.is_empty() { + String::new() + } else { + format!(" [{}]", app.orchestrator_persona) + }, + status + ), Style::default().fg(Color::Yellow), ))); }