From 772ef5f912c93e8a1b7f9879b92091b886f29f29 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 21 Oct 2026 17:26:49 -0700 Subject: [PATCH 1/8] optchat-chief: test that chief.engine.get offers every harness (red) The engine picker needs offered.harnesses[]: each acpmux harness a Chief turn can run on, with displayName, family, capabilities and why it is unavailable. Tests only; the brain does not answer it yet. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cfc-5b39-7e21-bc3d-02eb374c820c&host=cmux16 --- .../optchat-chief/tests/engine_offer.rs | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/Native/OptChat/optchat-chief/tests/engine_offer.rs b/Native/OptChat/optchat-chief/tests/engine_offer.rs index 171e61b254fc..afa995df7330 100644 --- a/Native/OptChat/optchat-chief/tests/engine_offer.rs +++ b/Native/OptChat/optchat-chief/tests/engine_offer.rs @@ -46,6 +46,7 @@ fn serve(listener: UnixListener) { "cost": {"input": 15}}, ]}, ]}), + "_acpmux/harnesses" => harnesses(), _ => json!({}), }; writeln!( @@ -60,6 +61,34 @@ fn serve(listener: UnixListener) { }); } +/// The fake's `_acpmux/harnesses`: Claude Code through sr and directly, +/// a Claude ACP adapter the gate refuses, codex, an OpenCode whose launcher +/// is missing, the built-in UniiChat, and a terminal-only harness. +fn harnesses() -> Value { + json!({ + "harnesses": { + "claude-sr": {"kind": "claude-stdio", "argv": ["sr", "claude", "proxy"], "family": "claude"}, + "claude": {"kind": "claude-stdio", "argv": ["claude"], "family": "claude"}, + "claude-acp": {"kind": "acp", "argv": ["claude-code-acp"], "family": "claude"}, + "codex": {"kind": "acp", "argv": ["codex-acp"], "family": "codex"}, + "opencode": {"kind": "acp", "argv": ["opencode", "acp"], "family": "opencode", + "displayName": "OpenCode", "icon": "opencode", + "capabilities": {"effort": ["low", "high"], "resume": true, "permissionModes": true}, + "auth": {"login": "opencode auth login"}, + "unavailable": "opencode is not on PATH"}, + "unii": {"kind": "acp", "argv": ["/Applications/cmux.app/acpmux", "uniichat-agent"], + "family": "unii", "displayName": "UniiChat", + "description": "UniiChat: one chat that never ends (uniichat.com)", + "capabilities": {"resume": true}}, + "aider": {"kind": "terminal", "argv": ["aider"], "family": "aider"}, + }, + "catalog": {"harnesses": [ + {"id": "claude", "name": "Claude Code", "profiles": ["claude", "claude-sr"]}, + {"id": "codex", "name": "Codex", "profiles": ["codex"]}, + ]}, + }) +} + /// A brain over the fake acpmux, its engine.json in `dir`, knowing /// `families` (`_acpmux/harnesses` at host start). fn brain(dir: &std::path::Path, families: &[(&str, optchat_chief::acpmux::Family)]) -> Brain { @@ -287,3 +316,94 @@ fn a_chief_switches_its_turn_harness_to_uniichat_and_back() { None ); } + +/// The engine picker lists every harness a Chief turn can run on, from +/// acpmux's `_acpmux/harnesses` (Leo #89: "users choose the Chief's engine +/// ... each one lists what it supports"): its name, family, what it takes, +/// and why it cannot run when it cannot. A terminal-only harness never runs +/// a Chief turn and is not offered. +#[test] +fn the_engine_answer_offers_every_harness_with_what_it_supports() { + let dir = tempfile::tempdir().unwrap(); + let mut brain = brain(dir.path(), &[]); + let answer = engine(&mut brain, EngineRequest::Show); + let offered = answer["offered"]["harnesses"] + .as_array() + .unwrap_or_else(|| panic!("no offered harnesses: {answer:#}")); + let ids: Vec<&str> = offered.iter().filter_map(|h| h["id"].as_str()).collect(); + assert_eq!( + ids, + [ + "claude", + "claude-acp", + "claude-sr", + "codex", + "opencode", + "unii" + ], + "{answer:#}" + ); + let by_id = |id: &str| offered.iter().find(|h| h["id"] == id).unwrap(); + + let codex = by_id("codex"); + assert_eq!(codex["displayName"], "Codex", "{codex:#}"); + assert_eq!(codex["family"], "codex"); + assert_eq!(codex["chiefFamily"], "codex"); + assert_eq!(codex["kind"], "acp"); + assert_eq!(codex["unavailable"], false); + assert_eq!(codex.get("unavailableReason"), None); + assert_eq!(codex["capabilities"]["speeds"], json!(["default", "fast"])); + assert_eq!( + codex["capabilities"]["efforts"], + json!(["minimal", "low", "medium", "high", "xhigh", "max"]) + ); + + // Two profiles of one catalog harness keep apart by their ids. + let sr = by_id("claude-sr"); + assert_eq!(sr["displayName"], "Claude Code (claude-sr)", "{sr:#}"); + assert_eq!(by_id("claude")["displayName"], "Claude Code (claude)"); + assert_eq!(sr["chiefFamily"], "claude"); + assert_eq!(sr["capabilities"]["speeds"], json!(["default"])); + assert_eq!(sr["unavailable"], false); + + // A launcher acpmux could not find: listed, with acpmux's reason, the + // profile's declared efforts and flags, and how to sign in. + let opencode = by_id("opencode"); + assert_eq!(opencode["displayName"], "OpenCode", "{opencode:#}"); + assert_eq!(opencode["family"], "opencode"); + assert_eq!(opencode["chiefFamily"], "other"); + assert_eq!(opencode["icon"], "opencode"); + assert_eq!(opencode["unavailable"], true); + assert!( + opencode["unavailableReason"] + .as_str() + .is_some_and(|r| r.contains("opencode is not on PATH")), + "{opencode:#}" + ); + assert_eq!(opencode["capabilities"]["efforts"], json!(["low", "high"])); + assert_eq!(opencode["capabilities"]["resume"], true); + assert_eq!(opencode["capabilities"]["permissionModes"], true); + assert_eq!(opencode["capabilities"]["speeds"], json!(["default"])); + assert_eq!(opencode["login"], "opencode auth login"); + + // A Claude ACP adapter: the harness gate refuses it, and says why. + let refused = by_id("claude-acp"); + assert_eq!(refused["unavailable"], true, "{refused:#}"); + assert!( + refused["unavailableReason"] + .as_str() + .is_some_and(|r| r.contains("claude-stdio")), + "{refused:#}" + ); + + // UniiChat: its own family, laid out as Other, with its description. + let unii = by_id("unii"); + assert_eq!(unii["displayName"], "UniiChat", "{unii:#}"); + assert_eq!(unii["family"], "unii"); + assert_eq!(unii["chiefFamily"], "other"); + assert_eq!( + unii["description"], + "UniiChat: one chat that never ends (uniichat.com)" + ); + assert_eq!(unii["unavailable"], false); +} From b0a3ef387ba93d6d862fd122477e8a5b002b1dbd Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 21 Oct 2026 17:30:40 -0700 Subject: [PATCH 2/8] CmuxNextDaemon: test the decode of chief.engine.get's offered harnesses (red) Tests only; ChiefEngineHarnessOffer does not exist yet. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cfc-5b39-7e21-bc3d-02eb374c820c&host=cmux16 --- .../ChiefEngineHarnessOfferTests.swift | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 Packages/macOS/CmuxNext/Tests/CmuxNextDaemonTests/ChiefEngineHarnessOfferTests.swift diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextDaemonTests/ChiefEngineHarnessOfferTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextDaemonTests/ChiefEngineHarnessOfferTests.swift new file mode 100644 index 000000000000..0185cfcd83a1 --- /dev/null +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextDaemonTests/ChiefEngineHarnessOfferTests.swift @@ -0,0 +1,79 @@ +import Foundation +import Testing +@testable import CmuxNextDaemon + +/// The engines `chief.engine.get` offers the Chief picker (`offered.harnesses`, +/// optchat-chief brain/harness_offer.rs): each with its name, family, what it +/// supports and why it cannot run when it cannot. +struct ChiefEngineHarnessOfferTests { + static let answer = Data(#""" + {"engine": {"harness": "claude-sr"}, "offered": {"models": [], "efforts": [], "speeds": ["default"], "harnesses": [ + {"id": "codex", "displayName": "Codex", "family": "codex", "chiefFamily": "codex", "kind": "acp", + "capabilities": {"efforts": ["minimal", "low", "medium", "high", "xhigh", "max"], "speeds": ["default", "fast"]}, + "unavailable": false}, + {"id": "opencode", "displayName": "OpenCode", "family": "opencode", "chiefFamily": "other", "kind": "acp", + "capabilities": {"efforts": ["low", "high"], "speeds": ["default"], "resume": true, "permissionModes": true}, + "unavailable": true, "unavailableReason": "acpmux's opencode is unavailable: opencode is not on PATH", + "icon": "opencode", "login": "opencode auth login"}, + {"id": "unii", "displayName": "UniiChat", "family": "unii", "chiefFamily": "other", "kind": "acp", + "capabilities": {"efforts": ["minimal"], "speeds": ["default"], "resume": true, "someFutureFlag": 3}, + "unavailable": false, "description": "UniiChat: one chat that never ends (uniichat.com)", + "someFutureField": {"x": 1}}, + {"id": 7} + ]}} + """#.utf8) + + @Test func everyOfferedEngineDecodesWithWhatItSupports() throws { + let offer = try JSONDecoder().decode(ChiefEngineHarnessOffer.self, from: Self.answer) + #expect(offer.harnesses.map(\.id) == ["codex", "opencode", "unii"]) + + let codex = try #require(offer.harness(id: "codex")) + #expect(codex.displayName == "Codex") + #expect(codex.family == "codex") + #expect(codex.chiefFamily == "codex") + #expect(codex.isAvailable) + #expect(codex.unavailableReason == nil) + #expect(codex.capabilities.speeds == ["default", "fast"]) + #expect(codex.capabilities.efforts.count == 6) + #expect(codex.capabilities.resume == nil) + + let opencode = try #require(offer.harness(id: "opencode")) + #expect(!opencode.isAvailable) + #expect(opencode.unavailableReason?.contains("not on PATH") == true) + #expect(opencode.capabilities.efforts == ["low", "high"]) + #expect(opencode.capabilities.resume == true) + #expect(opencode.capabilities.permissionModes == true) + #expect(opencode.login == "opencode auth login") + #expect(opencode.icon == "opencode") + + let unii = try #require(offer.harness(id: "unii")) + #expect(unii.family == "unii") + #expect(unii.chiefFamily == "other") + #expect(unii.description == "UniiChat: one chat that never ends (uniichat.com)") + } + + /// The picker lists the engines that can run a turn first, in the brain's order. + @Test func thePickerOrderPutsAvailableEnginesFirst() throws { + let offer = try JSONDecoder().decode(ChiefEngineHarnessOffer.self, from: Self.answer) + #expect(offer.pickerOrder.map(\.id) == ["codex", "unii", "opencode"]) + } + + /// An older brain answers no `harnesses`: the picker keeps its own list. + @Test func anOlderBrainOffersNone() throws { + let old = Data(#"{"engine": {"harness": "claude"}, "offered": {"models": [], "efforts": [], "speeds": []}}"#.utf8) + #expect(try JSONDecoder().decode(ChiefEngineHarnessOffer.self, from: old).harnesses.isEmpty) + let older = Data(#"{"engine": {"harness": "claude"}}"#.utf8) + #expect(try JSONDecoder().decode(ChiefEngineHarnessOffer.self, from: older).harnesses.isEmpty) + } + + /// A row without a reason still reads as unavailable when the brain says so. + @Test func anUnavailableRowWithoutAReasonIsStillUnavailable() throws { + let row = Data(#""" + {"id": "pi", "displayName": "Pi", "family": "pi", "chiefFamily": "other", "kind": "acp", + "capabilities": {"efforts": [], "speeds": ["default"]}, "unavailable": true} + """#.utf8) + let pi = try JSONDecoder().decode(ChiefEngineHarness.self, from: row) + #expect(!pi.isAvailable) + #expect(pi.unavailableReason == nil) + } +} From 50730ebcd6d66ad3ac252d014489a74ed279bd83 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Wed, 21 Oct 2026 17:32:14 -0700 Subject: [PATCH 3/8] Chief engine picker: chief.engine.get offers every harness with what it supports offered.harnesses[] lists each acpmux harness a Chief turn can run on (_acpmux/harnesses, read at each answer): id, displayName, acpmux's family and the Chief's layout family, kind, capabilities (efforts, speeds, the profile's resume/permissionModes/fast), unavailable with the reason (acpmux's own, or the harness gate's refusal), and description, icon and login when the profile has them. Terminal-only harnesses are left out. ChiefEngineHarnessOffer decodes it on the Mac; the picker UI wires it in after #23883. Co-Authored-By: Claude Opus 5.5 Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cfc-5b39-7e21-bc3d-02eb374c820c&host=cmux16 --- .../optchat-chief/src/brain/engine_control.rs | 16 +- .../optchat-chief/src/brain/harness_offer.rs | 147 ++++++++++++++++++ Native/OptChat/optchat-chief/src/brain/mod.rs | 1 + .../Chief/ChiefEngineHarnessOffer.swift | 125 +++++++++++++++ 4 files changed, 287 insertions(+), 2 deletions(-) create mode 100644 Native/OptChat/optchat-chief/src/brain/harness_offer.rs create mode 100644 Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Chief/ChiefEngineHarnessOffer.swift diff --git a/Native/OptChat/optchat-chief/src/brain/engine_control.rs b/Native/OptChat/optchat-chief/src/brain/engine_control.rs index 7ca03ed9647b..c27904ddf536 100644 --- a/Native/OptChat/optchat-chief/src/brain/engine_control.rs +++ b/Native/OptChat/optchat-chief/src/brain/engine_control.rs @@ -9,7 +9,10 @@ //! Answer: `{"engine": {harness, model, effort, compactor_harness, //! compactor_model}, "choice": , "last_turn": |null, //! "recent": [...], "offered": {"models": [{id, name, family?, -//! efforts?, fast?, aliases?, ...}...], "efforts": [...], "speeds": [...]}}`, or `{"error": {"code", "message"}}`. +//! efforts?, fast?, aliases?, ...}...], "efforts": [...], "speeds": [...], +//! "harnesses": [{id, displayName, family, chiefFamily, kind, capabilities, +//! unavailable, unavailableReason?, ...}...]}}` (harness_offer.rs), or +//! `{"error": {"code", "message"}}`. use serde_json::{Value, json}; @@ -166,6 +169,15 @@ impl Brain { .copied() .filter(|speed| crate::engine::check_speed(speed, family).is_ok()) .collect(); + // Every engine the picker can switch the next turn to (harness_offer.rs). + let harnesses = self + .agents + .harness_catalog() + .map(|answer| super::harness_offer::offered_harnesses(&answer)) + .unwrap_or_else(|e| { + (self.log)(&format!("engine answer: acpmux harnesses: {e}")); + Vec::new() + }); json!({ "engine": { "harness": engine.harness, @@ -179,7 +191,7 @@ impl Brain { "choice": choice, "last_turn": recent.first().cloned().unwrap_or(Value::Null), "recent": recent, - "offered": {"models": models, "efforts": EFFORTS, "speeds": speeds}, + "offered": {"models": models, "efforts": EFFORTS, "speeds": speeds, "harnesses": harnesses}, }) } diff --git a/Native/OptChat/optchat-chief/src/brain/harness_offer.rs b/Native/OptChat/optchat-chief/src/brain/harness_offer.rs new file mode 100644 index 000000000000..c05061c8ad40 --- /dev/null +++ b/Native/OptChat/optchat-chief/src/brain/harness_offer.rs @@ -0,0 +1,147 @@ +//! chief.engine.get's `offered.harnesses`: the engines the Chief's picker +//! lists (Leo #89 item 3: Claude Code via sr2, Codex, OpenCode, the other +//! acpmux harnesses, UniiChat), each with what it supports and, when it +//! cannot run a turn, why. Read from acpmux's `_acpmux/harnesses` at each +//! answer, so a harness installed or signed in since is seen at once. +//! +//! Row: `{"id", "displayName", "family", "chiefFamily", "kind", +//! "capabilities": {"efforts", "speeds", "resume"?, "permissionModes"?, +//! "fast"?}, "unavailable": bool, "unavailableReason"?, "description"?, +//! "icon"?, "login"?}`, by id. `family` is acpmux's (opencode, unii, ...); +//! `chiefFamily` is the layout the Chief gives it (claude, codex, other), +//! which decides the speeds. A terminal-only harness never runs a Chief +//! turn and is not listed. + +use std::collections::BTreeMap; + +use serde_json::{Map, Value, json}; + +use crate::acpmux::Family; + +/// The rows for an `_acpmux/harnesses` answer. +pub(super) fn offered_harnesses(answer: &Value) -> Vec { + let Some(profiles) = answer.get("harnesses").and_then(Value::as_object) else { + return Vec::new(); + }; + // The curated catalog's name of each profile (`Claude Code` for both + // `claude` and `claude-sr`). + let mut catalog_names: BTreeMap<&str, &str> = BTreeMap::new(); + for h in answer + .pointer("/catalog/harnesses") + .and_then(Value::as_array) + .into_iter() + .flatten() + { + let Some(name) = h.get("name").and_then(Value::as_str) else { + continue; + }; + for p in h + .get("profiles") + .and_then(Value::as_array) + .into_iter() + .flatten() + { + if let Some(p) = p.as_str() { + catalog_names.insert(p, name); + } + } + } + let mut rows: Vec = profiles + .iter() + .filter(|(_, p)| p.get("kind").and_then(Value::as_str) != Some("terminal")) + .map(|(id, p)| row(answer, id, p, catalog_names.get(id.as_str()).copied())) + .collect(); + rows.sort_by(|a, b| a["id"].as_str().cmp(&b["id"].as_str())); + // Two profiles under one name keep apart by their ids. + let mut counts: BTreeMap = BTreeMap::new(); + for r in &rows { + *counts + .entry(r["displayName"].as_str().unwrap_or_default().to_owned()) + .or_default() += 1; + } + for r in &mut rows { + let name = r["displayName"].as_str().unwrap_or_default().to_owned(); + let id = r["id"].as_str().unwrap_or_default().to_owned(); + if counts.get(&name).copied().unwrap_or(0) > 1 && name != id { + r["displayName"] = json!(format!("{name} ({id})")); + } + } + rows +} + +fn row(answer: &Value, id: &str, profile: &Value, catalog_name: Option<&str>) -> Value { + let str_of = |key: &str| { + profile + .get(key) + .and_then(Value::as_str) + .filter(|s| !s.is_empty()) + }; + // The gate the turn passes (harness_gate): acpmux's own unavailable + // reason, a Claude adapter that is not acpmux's, a route with no profile. + let plan = crate::harness_gate::plan(answer, id); + let chief_family = plan.family; + let reason = plan.admitted.err(); + let family = str_of("family").unwrap_or(family_name(chief_family)); + let display = str_of("displayName").or(catalog_name).unwrap_or(id); + + let mut capabilities = Map::new(); + let declared = profile.get("capabilities").and_then(Value::as_object); + for (key, value) in declared.into_iter().flatten() { + if key != "effort" && !value.is_null() { + capabilities.insert(key.clone(), value.clone()); + } + } + // The efforts the profile declares, else every one a set accepts. + let efforts = declared + .and_then(|c| c.get("effort")) + .and_then(Value::as_array) + .filter(|e| !e.is_empty()) + .cloned() + .unwrap_or_else(|| { + super::engine_control::EFFORTS + .iter() + .map(|e| json!(e)) + .collect() + }); + capabilities.insert("efforts".into(), Value::Array(efforts)); + let speeds: Vec<&str> = crate::engine::SPEEDS + .iter() + .copied() + .filter(|speed| crate::engine::check_speed(speed, chief_family).is_ok()) + .collect(); + capabilities.insert("speeds".into(), json!(speeds)); + + let mut row = json!({ + "id": id, + "displayName": display, + "family": family, + "chiefFamily": family_name(chief_family), + "kind": str_of("kind").unwrap_or("acp"), + "capabilities": capabilities, + "unavailable": reason.is_some(), + }); + if let Some(reason) = reason { + row["unavailableReason"] = json!(reason); + } + for (key, value) in [ + ("description", str_of("description")), + ("icon", str_of("icon")), + ( + "login", + profile.pointer("/auth/login").and_then(Value::as_str), + ), + ] { + if let Some(value) = value { + row[key] = json!(value); + } + } + row +} + +fn family_name(family: Family) -> &'static str { + match family { + Family::Claude => "claude", + Family::Codex => "codex", + Family::Other => "other", + } +} diff --git a/Native/OptChat/optchat-chief/src/brain/mod.rs b/Native/OptChat/optchat-chief/src/brain/mod.rs index 24ef48caf231..3f1a5b41dca9 100644 --- a/Native/OptChat/optchat-chief/src/brain/mod.rs +++ b/Native/OptChat/optchat-chief/src/brain/mod.rs @@ -28,6 +28,7 @@ mod desk; mod drafts; mod engine_control; pub mod files; +mod harness_offer; mod image_fit; pub mod images; mod inbox; diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Chief/ChiefEngineHarnessOffer.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Chief/ChiefEngineHarnessOffer.swift new file mode 100644 index 000000000000..e90135330d5b --- /dev/null +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextDaemon/Chief/ChiefEngineHarnessOffer.swift @@ -0,0 +1,125 @@ +public import Foundation + +/// One engine `chief.engine.get` offers the Chief picker (`offered.harnesses`, +/// optchat-chief brain/harness_offer.rs): an acpmux harness a Chief turn can +/// run on, with what it supports and, when it cannot run, why. A +/// `chief.engine.set {harness: id}` switches the Chief to it from the next turn. +public struct ChiefEngineHarness: Decodable, Sendable, Equatable, Identifiable { + /// What the engine takes, as the brain checks a set. + public struct Capabilities: Decodable, Sendable, Equatable { + /// The efforts it takes: the profile's own list, else every one the brain accepts. + public var efforts: [String] + /// The speeds its turns run (`fast` only on a codex-family harness). + public var speeds: [String] + /// Whether its sessions resume after a restart; nil when the profile does not say. + public var resume: Bool? + /// Whether it has permission modes; nil when the profile does not say. + public var permissionModes: Bool? + /// The profile's own fast flag; nil when it does not say. + public var fast: Bool? + + public init(efforts: [String] = [], speeds: [String] = [], resume: Bool? = nil, + permissionModes: Bool? = nil, fast: Bool? = nil) { + self.efforts = efforts + self.speeds = speeds + self.resume = resume + self.permissionModes = permissionModes + self.fast = fast + } + + enum CodingKeys: String, CodingKey { case efforts, speeds, resume, permissionModes, fast } + + public init(from decoder: any Decoder) throws { + let c = try decoder.container(keyedBy: CodingKeys.self) + efforts = try c.decodeIfPresent([String].self, forKey: .efforts) ?? [] + speeds = try c.decodeIfPresent([String].self, forKey: .speeds) ?? [] + resume = try? c.decodeIfPresent(Bool.self, forKey: .resume) + permissionModes = try? c.decodeIfPresent(Bool.self, forKey: .permissionModes) + fast = try? c.decodeIfPresent(Bool.self, forKey: .fast) + } + } + + /// The acpmux harness id (`claude-sr`, `codex`, `opencode`, `unii`): `chief.engine.set`'s `harness`. + public var id: String + /// The profile's or the catalog's name; two profiles of one name carry their ids. + public var displayName: String + /// acpmux's family (`claude`, `codex`, `opencode`, `unii`, ...). + public var family: String + /// The layout the Chief gives it: `claude`, `codex` or `other`. + public var chiefFamily: String + /// acpmux's kind (`claude-stdio`, `acp`). + public var kind: String + public var capabilities: Capabilities + /// Whether a turn on it would be refused (its launcher is missing, the harness gate refuses it). + public var unavailable: Bool + /// Why it cannot run, as the brain says it; nil when it can (or the brain gave no reason). + public var unavailableReason: String? + public var description: String? + /// A built-in icon name, or an absolute path to the icon file. + public var icon: String? + /// The command that signs in to it (`unii login`). + public var login: String? + + /// Whether a turn can run on it now. + public var isAvailable: Bool { !unavailable } + + enum CodingKeys: String, CodingKey { + case id, displayName, family, chiefFamily, kind, capabilities, unavailable, unavailableReason + case description, icon, login + } + + public init(from decoder: any Decoder) throws { + let c = try decoder.container(keyedBy: CodingKeys.self) + id = try c.decode(String.self, forKey: .id) + displayName = try c.decodeIfPresent(String.self, forKey: .displayName) ?? id + chiefFamily = try c.decodeIfPresent(String.self, forKey: .chiefFamily) ?? "other" + family = try c.decodeIfPresent(String.self, forKey: .family) ?? chiefFamily + kind = try c.decodeIfPresent(String.self, forKey: .kind) ?? "acp" + capabilities = try c.decodeIfPresent(Capabilities.self, forKey: .capabilities) ?? Capabilities() + unavailableReason = try c.decodeIfPresent(String.self, forKey: .unavailableReason) + unavailable = try c.decodeIfPresent(Bool.self, forKey: .unavailable) ?? (unavailableReason != nil) + description = try c.decodeIfPresent(String.self, forKey: .description) + icon = try c.decodeIfPresent(String.self, forKey: .icon) + login = try c.decodeIfPresent(String.self, forKey: .login) + } +} + +/// `offered.harnesses` of a `chief.engine.get` (or `chief.engine.set`) answer, +/// decoded from the whole answer (empty from a brain that offers none). A row +/// this app cannot read is skipped, not the whole list. +public struct ChiefEngineHarnessOffer: Decodable, Sendable, Equatable { + public var harnesses: [ChiefEngineHarness] + + public init(harnesses: [ChiefEngineHarness] = []) { + self.harnesses = harnesses + } + + enum CodingKeys: String, CodingKey { case offered, harnesses } + + public init(from decoder: any Decoder) throws { + let answer = try decoder.container(keyedBy: CodingKeys.self) + harnesses = [] + guard answer.contains(.offered), + let offered = try? answer.nestedContainer(keyedBy: CodingKeys.self, forKey: .offered), + offered.contains(.harnesses), + var rows = try? offered.nestedUnkeyedContainer(forKey: .harnesses) else { return } + while !rows.isAtEnd { + if let row = try? rows.decode(ChiefEngineHarness.self) { + harnesses.append(row) + } else { + // Step past the row this app cannot read. + _ = try rows.decode(JSONValue.self) + } + } + } + + /// The engine whose harness id is `id`. + public func harness(id: String) -> ChiefEngineHarness? { + harnesses.first { $0.id == id } + } + + /// The picker's order: the engines that can run a turn, then the rest, each in the brain's order. + public var pickerOrder: [ChiefEngineHarness] { + harnesses.filter(\.isAvailable) + harnesses.filter { !$0.isAvailable } + } +} From cc42a8b49912d4b380c105a2eb82d39f3be3c99a Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 25 Oct 2026 21:41:14 +0000 Subject: [PATCH 4/8] Chief engine picker: a row never offers what a set would refuse The review (codex gpt-6-astra xhigh at 0a245f449e9) found two ways the picker could offer an engine chief.engine.set answers with an error: - Discovery read acpmux's live catalog while a set checks the families the brain read at its start, so a harness acpmux gained since showed as available and then answered unknown_harness. Such a row is now listed unavailable with that reason ("restart the Chief"), which is step 4 of the PR's remaining work. - A profile may declare an effort acpmux takes and a Chief set does not ("none"), which answered invalid_effort. Declared efforts are now filtered to the ones EFFORTS accepts; none left falls back to the full list. Test: an_offer_never_names_a_harness_or_effort_a_set_would_refuse asserts both rows and then asserts the set answers the refusal each row names. Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a174e4-3868-7f51-8f3d-21b34ad270b2&host=huge-blue --- .../optchat-chief/src/brain/engine_control.rs | 2 +- .../optchat-chief/src/brain/harness_offer.rs | 70 +++++++++++++++---- .../optchat-chief/tests/engine_offer.rs | 51 +++++++++++++- 3 files changed, 107 insertions(+), 16 deletions(-) diff --git a/Native/OptChat/optchat-chief/src/brain/engine_control.rs b/Native/OptChat/optchat-chief/src/brain/engine_control.rs index 214ebd5dc16c..15bfe4b41314 100644 --- a/Native/OptChat/optchat-chief/src/brain/engine_control.rs +++ b/Native/OptChat/optchat-chief/src/brain/engine_control.rs @@ -209,7 +209,7 @@ impl Brain { let harnesses = self .agents .harness_catalog() - .map(|answer| super::harness_offer::offered_harnesses(&answer)) + .map(|answer| super::harness_offer::offered_harnesses(&answer, &self.settings.families)) .unwrap_or_else(|e| { (self.log)(&format!("engine answer: acpmux harnesses: {e}")); Vec::new() diff --git a/Native/OptChat/optchat-chief/src/brain/harness_offer.rs b/Native/OptChat/optchat-chief/src/brain/harness_offer.rs index c05061c8ad40..3da3d6dcd3d1 100644 --- a/Native/OptChat/optchat-chief/src/brain/harness_offer.rs +++ b/Native/OptChat/optchat-chief/src/brain/harness_offer.rs @@ -11,6 +11,11 @@ //! `chiefFamily` is the layout the Chief gives it (claude, codex, other), //! which decides the speeds. A terminal-only harness never runs a Chief //! turn and is not listed. +//! +//! A row never offers what `chief.engine.set` would refuse, since the picker +//! acts on this list: a harness the brain did not read at its start is marked +//! unavailable ("restart the Chief"), because the set checks the start-up +//! families; and a declared effort the set does not take is left out. use std::collections::BTreeMap; @@ -18,8 +23,13 @@ use serde_json::{Map, Value, json}; use crate::acpmux::Family; -/// The rows for an `_acpmux/harnesses` answer. -pub(super) fn offered_harnesses(answer: &Value) -> Vec { +/// The rows for an `_acpmux/harnesses` answer. `started_with` is the families +/// the brain read at its start (``Settings::families``), which is what a set +/// checks a harness against; empty means the set checks nothing. +pub(super) fn offered_harnesses( + answer: &Value, + started_with: &BTreeMap, +) -> Vec { let Some(profiles) = answer.get("harnesses").and_then(Value::as_object) else { return Vec::new(); }; @@ -49,7 +59,15 @@ pub(super) fn offered_harnesses(answer: &Value) -> Vec { let mut rows: Vec = profiles .iter() .filter(|(_, p)| p.get("kind").and_then(Value::as_str) != Some("terminal")) - .map(|(id, p)| row(answer, id, p, catalog_names.get(id.as_str()).copied())) + .map(|(id, p)| { + row( + answer, + id, + p, + catalog_names.get(id.as_str()).copied(), + started_with, + ) + }) .collect(); rows.sort_by(|a, b| a["id"].as_str().cmp(&b["id"].as_str())); // Two profiles under one name keep apart by their ids. @@ -69,7 +87,13 @@ pub(super) fn offered_harnesses(answer: &Value) -> Vec { rows } -fn row(answer: &Value, id: &str, profile: &Value, catalog_name: Option<&str>) -> Value { +fn row( + answer: &Value, + id: &str, + profile: &Value, + catalog_name: Option<&str>, + started_with: &BTreeMap, +) -> Value { let str_of = |key: &str| { profile .get(key) @@ -80,7 +104,15 @@ fn row(answer: &Value, id: &str, profile: &Value, catalog_name: Option<&str>) -> // reason, a Claude adapter that is not acpmux's, a route with no profile. let plan = crate::harness_gate::plan(answer, id); let chief_family = plan.family; - let reason = plan.admitted.err(); + // A set checks the harness against the families this brain read at its + // start, so a harness acpmux gained since would be refused with + // `unknown_harness`. Say that here instead of offering a row the picker + // cannot switch to. + let reason = plan.admitted.err().or_else(|| { + (!started_with.is_empty() && !started_with.contains_key(id)).then(|| { + format!("acpmux gained {id} after this Chief started; restart the Chief to run on it") + }) + }); let family = str_of("family").unwrap_or(family_name(chief_family)); let display = str_of("displayName").or(catalog_name).unwrap_or(id); @@ -91,18 +123,28 @@ fn row(answer: &Value, id: &str, profile: &Value, catalog_name: Option<&str>) -> capabilities.insert(key.clone(), value.clone()); } } - // The efforts the profile declares, else every one a set accepts. - let efforts = declared + // The efforts the profile declares, less any a set would refuse + // (`invalid_effort`); none left, or none declared, offers every one a set + // accepts. + let efforts: Vec = declared .and_then(|c| c.get("effort")) .and_then(Value::as_array) - .filter(|e| !e.is_empty()) + .into_iter() + .flatten() + .filter(|e| { + e.as_str() + .is_some_and(|e| super::engine_control::EFFORTS.contains(&e)) + }) .cloned() - .unwrap_or_else(|| { - super::engine_control::EFFORTS - .iter() - .map(|e| json!(e)) - .collect() - }); + .collect(); + let efforts = if efforts.is_empty() { + super::engine_control::EFFORTS + .iter() + .map(|e| json!(e)) + .collect() + } else { + efforts + }; capabilities.insert("efforts".into(), Value::Array(efforts)); let speeds: Vec<&str> = crate::engine::SPEEDS .iter() diff --git a/Native/OptChat/optchat-chief/tests/engine_offer.rs b/Native/OptChat/optchat-chief/tests/engine_offer.rs index 42afe441404b..2feabe958575 100644 --- a/Native/OptChat/optchat-chief/tests/engine_offer.rs +++ b/Native/OptChat/optchat-chief/tests/engine_offer.rs @@ -73,7 +73,8 @@ fn harnesses() -> Value { "codex": {"kind": "acp", "argv": ["codex-acp"], "family": "codex"}, "opencode": {"kind": "acp", "argv": ["opencode", "acp"], "family": "opencode", "displayName": "OpenCode", "icon": "opencode", - "capabilities": {"effort": ["low", "high"], "resume": true, "permissionModes": true}, + // `none` is an effort acpmux takes and a Chief set does not. + "capabilities": {"effort": ["low", "none", "high"], "resume": true, "permissionModes": true}, "auth": {"login": "opencode auth login"}, "unavailable": "opencode is not on PATH"}, "unii": {"kind": "acp", "argv": ["/Applications/cmux.app/acpmux", "uniichat-agent"], @@ -383,6 +384,8 @@ fn the_engine_answer_offers_every_harness_with_what_it_supports() { .is_some_and(|r| r.contains("opencode is not on PATH")), "{opencode:#}" ); + // `none` is dropped: a set refuses it, and the picker never offers what a + // set refuses. assert_eq!(opencode["capabilities"]["efforts"], json!(["low", "high"])); assert_eq!(opencode["capabilities"]["resume"], true); assert_eq!(opencode["capabilities"]["permissionModes"], true); @@ -411,6 +414,52 @@ fn the_engine_answer_offers_every_harness_with_what_it_supports() { assert_eq!(unii["unavailable"], false); } +/// Every offered row is one a set would take: a harness acpmux gained after +/// this Chief started is listed unavailable, with how to get it, rather than +/// offered for a set that answers `unknown_harness`; and a declared effort a +/// set refuses is left out rather than offered for an `invalid_effort`. +#[test] +fn an_offer_never_names_a_harness_or_effort_a_set_would_refuse() { + use optchat_chief::acpmux::Family; + let dir = tempfile::tempdir().unwrap(); + // The brain read only these at its start; acpmux lists unii and the rest now. + let mut brain = brain( + dir.path(), + &[("claude-sr", Family::Claude), ("codex", Family::Codex)], + ); + let answer = engine(&mut brain, EngineRequest::Show); + let offered = answer["offered"]["harnesses"].as_array().unwrap(); + let by_id = |id: &str| offered.iter().find(|h| h["id"] == id).unwrap(); + + // Still listed, so the picker can say why, and still never offered as runnable. + let unii = by_id("unii"); + assert_eq!(unii["unavailable"], true, "{unii:#}"); + assert!( + unii["unavailableReason"] + .as_str() + .is_some_and(|r| r.contains("after this Chief started")), + "{unii:#}" + ); + assert_eq!(by_id("codex")["unavailable"], false, "{answer:#}"); + + let set = |harness: Option<&str>, effort: Option<&str>| EngineRequest::Set { + harness: harness.map(str::to_owned), + model: None, + effort: effort.map(str::to_owned), + speed: None, + compactor_speed: None, + compactor_harness: None, + compactor_model: None, + reactions: Default::default(), + }; + // The reason the row gives is the refusal the set would answer. + let refused = engine(&mut brain, set(Some("unii"), None)); + assert_eq!(refused["error"]["code"], "unknown_harness", "{refused:#}"); + // And the effort the offer dropped is the one the set refuses. + let invalid = engine(&mut brain, set(None, Some("none"))); + assert_eq!(invalid["error"]["code"], "invalid_effort", "{invalid:#}"); +} + /// The reaction classifier's fields (a416): a set saves them in engine.json, /// the answer names the classifier this Chief runs (Claude Haiku and the /// built-in prompt, "", by default), `default` clears them, and a From 518488cf66a667ec5a2309bc624bb3aee13f2a11 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 25 Oct 2026 22:02:21 +0000 Subject: [PATCH 5/8] Chief engine picker: the row's speeds and name match what a set takes The second review (codex gpt-6-astra xhigh at cc42a8b4991) found two more ways the offer could name something chief.engine.set refuses: - Speeds came from the live profile family while a set checks the start-up family (Brain::family_of): a harness acpmux re-families keeps its row available and offers Fast, and the set answers invalid_speed. offered_harnesses now takes the set's own family_of and filters with it. - The id disambiguation could still collide: two profiles named X and a third already named "X (a)" leave two rows called "X (a)". A name still shared after that pass falls back to the id, which is unique. Tests: an_offer_never_names_a_harness_effort_or_speed_a_set_would_refuse (renamed) adds the speed case and asserts the set answers invalid_speed for it; no_two_rows_leave_the_picker_with_the_same_name covers the collision, with a claude-vm profile in the fake that is already called "Claude Code (claude)". Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a174e4-3868-7f51-8f3d-21b34ad270b2&host=huge-blue --- .../optchat-chief/src/brain/engine_control.rs | 6 +- .../optchat-chief/src/brain/harness_offer.rs | 58 +++++++++---- .../optchat-chief/tests/engine_offer.rs | 86 ++++++++++++++----- 3 files changed, 111 insertions(+), 39 deletions(-) diff --git a/Native/OptChat/optchat-chief/src/brain/engine_control.rs b/Native/OptChat/optchat-chief/src/brain/engine_control.rs index 15bfe4b41314..d5c3b85f26d2 100644 --- a/Native/OptChat/optchat-chief/src/brain/engine_control.rs +++ b/Native/OptChat/optchat-chief/src/brain/engine_control.rs @@ -209,7 +209,11 @@ impl Brain { let harnesses = self .agents .harness_catalog() - .map(|answer| super::harness_offer::offered_harnesses(&answer, &self.settings.families)) + .map(|answer| { + super::harness_offer::offered_harnesses(&answer, &self.settings.families, |h| { + self.family_of(h) + }) + }) .unwrap_or_else(|e| { (self.log)(&format!("engine answer: acpmux harnesses: {e}")); Vec::new() diff --git a/Native/OptChat/optchat-chief/src/brain/harness_offer.rs b/Native/OptChat/optchat-chief/src/brain/harness_offer.rs index 3da3d6dcd3d1..44239aef6754 100644 --- a/Native/OptChat/optchat-chief/src/brain/harness_offer.rs +++ b/Native/OptChat/optchat-chief/src/brain/harness_offer.rs @@ -23,12 +23,17 @@ use serde_json::{Map, Value, json}; use crate::acpmux::Family; -/// The rows for an `_acpmux/harnesses` answer. `started_with` is the families -/// the brain read at its start (``Settings::families``), which is what a set -/// checks a harness against; empty means the set checks nothing. +/// The rows for an `_acpmux/harnesses` answer. +/// +/// - `started_with`: the families the brain read at its start +/// (``Settings::families``), which is what a set checks a harness against; +/// empty means the set checks no harness. +/// - `set_family`: the family a set would check a harness's speed against +/// (``Brain::family_of``), which is the start-up family, not the live one. pub(super) fn offered_harnesses( answer: &Value, started_with: &BTreeMap, + set_family: impl Fn(&str) -> Family, ) -> Vec { let Some(profiles) = answer.get("harnesses").and_then(Value::as_object) else { return Vec::new(); @@ -66,25 +71,42 @@ pub(super) fn offered_harnesses( p, catalog_names.get(id.as_str()).copied(), started_with, + &set_family, ) }) .collect(); rows.sort_by(|a, b| a["id"].as_str().cmp(&b["id"].as_str())); - // Two profiles under one name keep apart by their ids. - let mut counts: BTreeMap = BTreeMap::new(); - for r in &rows { - *counts - .entry(r["displayName"].as_str().unwrap_or_default().to_owned()) - .or_default() += 1; - } - for r in &mut rows { - let name = r["displayName"].as_str().unwrap_or_default().to_owned(); - let id = r["id"].as_str().unwrap_or_default().to_owned(); - if counts.get(&name).copied().unwrap_or(0) > 1 && name != id { + disambiguate(&mut rows); + rows +} + +/// Two profiles under one name keep apart by their ids, and no two rows leave +/// with the same name: `X (a)` is taken when a third profile is already called +/// that, so a name that is still shared after the first pass becomes the id, +/// which is unique (it keys acpmux's map). +fn disambiguate(rows: &mut [Value]) { + let name_of = |r: &Value| r["displayName"].as_str().unwrap_or_default().to_owned(); + let id_of = |r: &Value| r["id"].as_str().unwrap_or_default().to_owned(); + let counts = |rows: &[Value]| { + let mut counts: BTreeMap = BTreeMap::new(); + for r in rows { + *counts.entry(name_of(r)).or_default() += 1; + } + counts + }; + let shared = counts(rows); + for r in rows.iter_mut() { + let (name, id) = (name_of(r), id_of(r)); + if shared.get(&name).copied().unwrap_or(0) > 1 && name != id { r["displayName"] = json!(format!("{name} ({id})")); } } - rows + let shared = counts(rows); + for r in rows.iter_mut() { + if shared.get(&name_of(r)).copied().unwrap_or(0) > 1 { + r["displayName"] = json!(id_of(r)); + } + } } fn row( @@ -93,6 +115,7 @@ fn row( profile: &Value, catalog_name: Option<&str>, started_with: &BTreeMap, + set_family: &impl Fn(&str) -> Family, ) -> Value { let str_of = |key: &str| { profile @@ -146,10 +169,13 @@ fn row( efforts }; capabilities.insert("efforts".into(), Value::Array(efforts)); + // The speeds a set would take, so the row never offers one it refuses: a + // set checks against the start-up family (`Brain::family_of`), which a + // harness whose family changed since no longer matches live. let speeds: Vec<&str> = crate::engine::SPEEDS .iter() .copied() - .filter(|speed| crate::engine::check_speed(speed, chief_family).is_ok()) + .filter(|speed| crate::engine::check_speed(speed, set_family(id)).is_ok()) .collect(); capabilities.insert("speeds".into(), json!(speeds)); diff --git a/Native/OptChat/optchat-chief/tests/engine_offer.rs b/Native/OptChat/optchat-chief/tests/engine_offer.rs index 2feabe958575..de8f91f8cd05 100644 --- a/Native/OptChat/optchat-chief/tests/engine_offer.rs +++ b/Native/OptChat/optchat-chief/tests/engine_offer.rs @@ -81,6 +81,10 @@ fn harnesses() -> Value { "family": "unii", "displayName": "UniiChat", "description": "UniiChat: one chat that never ends (uniichat.com)", "capabilities": {"resume": true}}, + // The name `claude` and `claude-sr` get when they keep apart by + // their ids, so the second pass has something to resolve. + "claude-vm": {"kind": "claude-stdio", "argv": ["claude"], "family": "claude", + "displayName": "Claude Code (claude)"}, "aider": {"kind": "terminal", "argv": ["aider"], "family": "aider"}, }, "catalog": {"harnesses": [ @@ -341,6 +345,7 @@ fn the_engine_answer_offers_every_harness_with_what_it_supports() { "claude", "claude-acp", "claude-sr", + "claude-vm", "codex", "opencode", "unii" @@ -362,10 +367,13 @@ fn the_engine_answer_offers_every_harness_with_what_it_supports() { json!(["minimal", "low", "medium", "high", "xhigh", "max"]) ); - // Two profiles of one catalog harness keep apart by their ids. + // Two profiles of one catalog harness keep apart by their ids. `claude` + // would become "Claude Code (claude)", which claude-vm already is, so + // both of those fall back to their ids and no two rows share a name. let sr = by_id("claude-sr"); assert_eq!(sr["displayName"], "Claude Code (claude-sr)", "{sr:#}"); - assert_eq!(by_id("claude")["displayName"], "Claude Code (claude)"); + assert_eq!(by_id("claude")["displayName"], "claude", "{answer:#}"); + assert_eq!(by_id("claude-vm")["displayName"], "claude-vm", "{answer:#}"); assert_eq!(sr["chiefFamily"], "claude"); assert_eq!(sr["capabilities"]["speeds"], json!(["default"])); assert_eq!(sr["unavailable"], false); @@ -416,16 +424,20 @@ fn the_engine_answer_offers_every_harness_with_what_it_supports() { /// Every offered row is one a set would take: a harness acpmux gained after /// this Chief started is listed unavailable, with how to get it, rather than -/// offered for a set that answers `unknown_harness`; and a declared effort a -/// set refuses is left out rather than offered for an `invalid_effort`. +/// offered for a set that answers `unknown_harness`; a declared effort a set +/// refuses is left out rather than offered for an `invalid_effort`; and the +/// speeds are the start-up family's, which is the family a set checks, not +/// the family acpmux reports now. #[test] -fn an_offer_never_names_a_harness_or_effort_a_set_would_refuse() { +fn an_offer_never_names_a_harness_effort_or_speed_a_set_would_refuse() { use optchat_chief::acpmux::Family; let dir = tempfile::tempdir().unwrap(); - // The brain read only these at its start; acpmux lists unii and the rest now. + // The brain read only these at its start; acpmux lists unii and the rest + // now, and it calls codex a Codex harness, which this brain did not read + // it as. let mut brain = brain( dir.path(), - &[("claude-sr", Family::Claude), ("codex", Family::Codex)], + &[("claude-sr", Family::Claude), ("codex", Family::Other)], ); let answer = engine(&mut brain, EngineRequest::Show); let offered = answer["offered"]["harnesses"].as_array().unwrap(); @@ -440,24 +452,54 @@ fn an_offer_never_names_a_harness_or_effort_a_set_would_refuse() { .is_some_and(|r| r.contains("after this Chief started")), "{unii:#}" ); - assert_eq!(by_id("codex")["unavailable"], false, "{answer:#}"); + // acpmux calls codex a Codex harness, whose Fast a set takes; this brain + // read it as Other, which is what a set checks, so the row offers no Fast. + let codex = by_id("codex"); + assert_eq!(codex["unavailable"], false, "{codex:#}"); + assert_eq!( + codex["capabilities"]["speeds"], + json!(["default"]), + "{codex:#}" + ); - let set = |harness: Option<&str>, effort: Option<&str>| EngineRequest::Set { - harness: harness.map(str::to_owned), - model: None, - effort: effort.map(str::to_owned), - speed: None, - compactor_speed: None, - compactor_harness: None, - compactor_model: None, - reactions: Default::default(), - }; - // The reason the row gives is the refusal the set would answer. - let refused = engine(&mut brain, set(Some("unii"), None)); + let set = + |harness: Option<&str>, effort: Option<&str>, speed: Option<&str>| EngineRequest::Set { + harness: harness.map(str::to_owned), + model: None, + effort: effort.map(str::to_owned), + speed: speed.map(str::to_owned), + compactor_speed: None, + compactor_harness: None, + compactor_model: None, + reactions: Default::default(), + }; + // The reason each row gives is the refusal the set would answer. + let refused = engine(&mut brain, set(Some("unii"), None, None)); assert_eq!(refused["error"]["code"], "unknown_harness", "{refused:#}"); - // And the effort the offer dropped is the one the set refuses. - let invalid = engine(&mut brain, set(None, Some("none"))); + let invalid = engine(&mut brain, set(None, Some("none"), None)); assert_eq!(invalid["error"]["code"], "invalid_effort", "{invalid:#}"); + let slow = engine(&mut brain, set(Some("codex"), None, Some("fast"))); + assert_eq!(slow["error"]["code"], "invalid_speed", "{slow:#}"); +} + +/// Two profiles under one name keep apart by their ids, and a third profile +/// already called `Claude Code (claude)` does not leave two rows with that +/// name: the ones that would still share it fall back to their ids. +#[test] +fn no_two_rows_leave_the_picker_with_the_same_name() { + use optchat_chief::acpmux::Family; + let dir = tempfile::tempdir().unwrap(); + let mut brain = brain(dir.path(), &[("claude-sr", Family::Claude)]); + let answer = engine(&mut brain, EngineRequest::Show); + let offered = answer["offered"]["harnesses"].as_array().unwrap(); + let mut names: Vec<&str> = offered + .iter() + .filter_map(|h| h["displayName"].as_str()) + .collect(); + let seen = names.len(); + names.sort_unstable(); + names.dedup(); + assert_eq!(names.len(), seen, "duplicate picker names: {answer:#}"); } /// The reaction classifier's fields (a416): a set saves them in engine.json, From 9b77d6bbc3b6c5cb6d6f771e7afa4758ead62ca5 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 25 Oct 2026 22:09:03 +0000 Subject: [PATCH 6/8] Chief engine picker: the broad offer test starts with the families it needs the_engine_answer_offers_every_harness_with_what_it_supports ran with an empty start-up families map, so with speeds now following the set's own family_of (the previous commit) it read every harness as Other and codex offered no Fast. The test gives the brain the families acpmux's fake reports, which is what a real brain reads at its start, and codex offers Fast again. Local: cargo test --test engine_offer, 9 passed, 0 failed. Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a174e4-3868-7f51-8f3d-21b34ad270b2&host=huge-blue --- .../OptChat/optchat-chief/tests/engine_offer.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/Native/OptChat/optchat-chief/tests/engine_offer.rs b/Native/OptChat/optchat-chief/tests/engine_offer.rs index de8f91f8cd05..d15049d1159c 100644 --- a/Native/OptChat/optchat-chief/tests/engine_offer.rs +++ b/Native/OptChat/optchat-chief/tests/engine_offer.rs @@ -332,8 +332,22 @@ fn a_chief_switches_its_turn_harness_to_uniichat_and_back() { /// a Chief turn and is not offered. #[test] fn the_engine_answer_offers_every_harness_with_what_it_supports() { + use optchat_chief::acpmux::Family; let dir = tempfile::tempdir().unwrap(); - let mut brain = brain(dir.path(), &[]); + // The families this brain read at its start, which is what a set checks a + // harness and its speed against, so the rows say what a set would take. + let mut brain = brain( + dir.path(), + &[ + ("claude", Family::Claude), + ("claude-acp", Family::Claude), + ("claude-sr", Family::Claude), + ("claude-vm", Family::Claude), + ("codex", Family::Codex), + ("opencode", Family::Other), + ("unii", Family::Other), + ], + ); let answer = engine(&mut brain, EngineRequest::Show); let offered = answer["offered"]["harnesses"] .as_array() From 3c54c4ad06b985f637e274852de3939744973e96 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 25 Oct 2026 22:27:40 +0000 Subject: [PATCH 7/8] Chief engine picker: one pass gives every row a name no other row has The third review (codex gpt-6-astra xhigh at 9b77d6bbc3b) found the two-pass disambiguation still produced duplicates: rows (a, X), (b, X), (c, "X (a)"), (d, "a") left two rows called "a", because the fallback to the id did not check the ids against names no pass had touched. disambiguate() is now one walk in id order over a taken set. A row wants its name, or "name (id)" when another row wants the same name; if that is taken it gives up the name for its id, and that for "id (2)", "id (3)"... So every row leaves with a name no other row has, whatever acpmux calls its profiles. An empty catalog name is also no longer a name, so no row leaves blank. Tests: no_two_rows_leave_the_picker_with_the_same_name builds exactly that four-row shape out of claude, claude-sr, claude-vm ("Claude Code (claude)") and claude-box ("claude"), asserts every final name and that none repeats or is blank, and a nameless profile with an empty catalog name falls back to its id. Red without the source change (cargo test --test engine_offer: 7 passed, 2 failed), green with it (9 passed, 0 failed). Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a174e4-3868-7f51-8f3d-21b34ad270b2&host=huge-blue --- .../optchat-chief/src/brain/harness_offer.rs | 58 ++++++++++++------- .../optchat-chief/tests/engine_offer.rs | 55 ++++++++++++++---- 2 files changed, 82 insertions(+), 31 deletions(-) diff --git a/Native/OptChat/optchat-chief/src/brain/harness_offer.rs b/Native/OptChat/optchat-chief/src/brain/harness_offer.rs index 44239aef6754..cd65d2140863 100644 --- a/Native/OptChat/optchat-chief/src/brain/harness_offer.rs +++ b/Native/OptChat/optchat-chief/src/brain/harness_offer.rs @@ -17,7 +17,7 @@ //! unavailable ("restart the Chief"), because the set checks the start-up //! families; and a declared effort the set does not take is left out. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use serde_json::{Map, Value, json}; @@ -47,7 +47,13 @@ pub(super) fn offered_harnesses( .into_iter() .flatten() { - let Some(name) = h.get("name").and_then(Value::as_str) else { + // An empty catalog name is no name: the row falls back to its id, + // never to a blank picker label. + let Some(name) = h + .get("name") + .and_then(Value::as_str) + .filter(|n| !n.is_empty()) + else { continue; }; for p in h @@ -81,31 +87,41 @@ pub(super) fn offered_harnesses( } /// Two profiles under one name keep apart by their ids, and no two rows leave -/// with the same name: `X (a)` is taken when a third profile is already called -/// that, so a name that is still shared after the first pass becomes the id, -/// which is unique (it keys acpmux's map). +/// the picker with the same name. +/// +/// A row wants `name`, or `name (id)` when another row wants the same `name`. +/// What it wants can be taken anyway, by a profile acpmux literally calls +/// `Claude Code (claude)` or `claude`, so the wanted name is then given up for +/// the id, and that for `id (2)`, `id (3)`... Rows are walked in id order +/// (``offered_harnesses`` sorted them), so the result does not depend on the +/// order acpmux listed them in. fn disambiguate(rows: &mut [Value]) { let name_of = |r: &Value| r["displayName"].as_str().unwrap_or_default().to_owned(); let id_of = |r: &Value| r["id"].as_str().unwrap_or_default().to_owned(); - let counts = |rows: &[Value]| { - let mut counts: BTreeMap = BTreeMap::new(); - for r in rows { - *counts.entry(name_of(r)).or_default() += 1; - } - counts - }; - let shared = counts(rows); - for r in rows.iter_mut() { - let (name, id) = (name_of(r), id_of(r)); - if shared.get(&name).copied().unwrap_or(0) > 1 && name != id { - r["displayName"] = json!(format!("{name} ({id})")); - } + let mut shared: BTreeMap = BTreeMap::new(); + for r in rows.iter() { + *shared.entry(name_of(r)).or_default() += 1; } - let shared = counts(rows); + let mut taken: BTreeSet = BTreeSet::new(); for r in rows.iter_mut() { - if shared.get(&name_of(r)).copied().unwrap_or(0) > 1 { - r["displayName"] = json!(id_of(r)); + let (name, id) = (name_of(r), id_of(r)); + let wanted = if shared.get(&name).copied().unwrap_or(0) > 1 && name != id { + format!("{name} ({id})") + } else { + name + }; + let mut pick = wanted; + if taken.contains(&pick) { + pick = id.clone(); + for n in 2.. { + if !taken.contains(&pick) { + break; + } + pick = format!("{id} ({n})"); + } } + r["displayName"] = json!(pick); + taken.insert(pick); } } diff --git a/Native/OptChat/optchat-chief/tests/engine_offer.rs b/Native/OptChat/optchat-chief/tests/engine_offer.rs index d15049d1159c..92daa9bc43dd 100644 --- a/Native/OptChat/optchat-chief/tests/engine_offer.rs +++ b/Native/OptChat/optchat-chief/tests/engine_offer.rs @@ -82,14 +82,21 @@ fn harnesses() -> Value { "description": "UniiChat: one chat that never ends (uniichat.com)", "capabilities": {"resume": true}}, // The name `claude` and `claude-sr` get when they keep apart by - // their ids, so the second pass has something to resolve. + // their ids, so the fallback has something to resolve... "claude-vm": {"kind": "claude-stdio", "argv": ["claude"], "family": "claude", "displayName": "Claude Code (claude)"}, + // ...and `claude`'s own fallback to its id is taken too, so even + // that has to give way. + "claude-box": {"kind": "claude-stdio", "argv": ["claude"], "family": "claude", + "displayName": "claude"}, + // An empty catalog name is no name: this row falls back to its id. + "nameless": {"kind": "acp", "argv": ["nameless"], "family": "other"}, "aider": {"kind": "terminal", "argv": ["aider"], "family": "aider"}, }, "catalog": {"harnesses": [ {"id": "claude", "name": "Claude Code", "profiles": ["claude", "claude-sr"]}, {"id": "codex", "name": "Codex", "profiles": ["codex"]}, + {"id": "nameless", "name": "", "profiles": ["nameless"]}, ]}, }) } @@ -341,9 +348,11 @@ fn the_engine_answer_offers_every_harness_with_what_it_supports() { &[ ("claude", Family::Claude), ("claude-acp", Family::Claude), + ("claude-box", Family::Claude), ("claude-sr", Family::Claude), ("claude-vm", Family::Claude), ("codex", Family::Codex), + ("nameless", Family::Other), ("opencode", Family::Other), ("unii", Family::Other), ], @@ -358,9 +367,11 @@ fn the_engine_answer_offers_every_harness_with_what_it_supports() { [ "claude", "claude-acp", + "claude-box", "claude-sr", "claude-vm", "codex", + "nameless", "opencode", "unii" ], @@ -381,12 +392,12 @@ fn the_engine_answer_offers_every_harness_with_what_it_supports() { json!(["minimal", "low", "medium", "high", "xhigh", "max"]) ); - // Two profiles of one catalog harness keep apart by their ids. `claude` - // would become "Claude Code (claude)", which claude-vm already is, so - // both of those fall back to their ids and no two rows share a name. + // Two profiles of one catalog harness keep apart by their ids, and + // claude-vm, which acpmux already calls "Claude Code (claude)", gives way + // to its own id, so no two rows share a name. let sr = by_id("claude-sr"); assert_eq!(sr["displayName"], "Claude Code (claude-sr)", "{sr:#}"); - assert_eq!(by_id("claude")["displayName"], "claude", "{answer:#}"); + assert_eq!(by_id("claude")["displayName"], "Claude Code (claude)"); assert_eq!(by_id("claude-vm")["displayName"], "claude-vm", "{answer:#}"); assert_eq!(sr["chiefFamily"], "claude"); assert_eq!(sr["capabilities"]["speeds"], json!(["default"])); @@ -506,14 +517,38 @@ fn no_two_rows_leave_the_picker_with_the_same_name() { let mut brain = brain(dir.path(), &[("claude-sr", Family::Claude)]); let answer = engine(&mut brain, EngineRequest::Show); let offered = answer["offered"]["harnesses"].as_array().unwrap(); - let mut names: Vec<&str> = offered + let names: Vec<&str> = offered .iter() .filter_map(|h| h["displayName"].as_str()) .collect(); - let seen = names.len(); - names.sort_unstable(); - names.dedup(); - assert_eq!(names.len(), seen, "duplicate picker names: {answer:#}"); + let mut unique = names.clone(); + unique.sort_unstable(); + unique.dedup(); + assert_eq!( + unique.len(), + names.len(), + "duplicate picker names {names:?}: {answer:#}" + ); + assert!( + !names.iter().any(|n| n.is_empty()), + "a blank picker name in {names:?}" + ); + let by_id = |id: &str| offered.iter().find(|h| h["id"] == id).unwrap(); + // The shape the old two-pass disambiguation left two rows called + // "claude" in: claude and claude-sr share "Claude Code", claude-vm is + // already called "Claude Code (claude)", and claude-box is already called + // "claude", the id claude would have fallen back to. Rows are taken in id + // order, so claude keeps the parenthesised name and claude-vm gives way. + assert_eq!( + by_id("claude-sr")["displayName"], + "Claude Code (claude-sr)", + "{answer:#}" + ); + assert_eq!(by_id("claude")["displayName"], "Claude Code (claude)"); + assert_eq!(by_id("claude-vm")["displayName"], "claude-vm", "{answer:#}"); + assert_eq!(by_id("claude-box")["displayName"], "claude"); + // An empty catalog name is no name. + assert_eq!(by_id("nameless")["displayName"], "nameless"); } /// The reaction classifier's fields (a416): a set saves them in engine.json, From d7ea6dfb8911dcace9137efe92b60bd20fd361b3 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 25 Oct 2026 22:58:38 +0000 Subject: [PATCH 8/8] Chief engine picker: a profile acpmux calls `default` is not offered as a choice Review finding (codex gpt-6-astra xhigh): `chief.engine.set` reads an empty harness value, or the literal `default`, as "clear the choice" (the `set` closure in engine_control). A profile acpmux actually calls `default` is a legitimate harness, so the picker offered it as runnable; selecting it refuses nothing and switches nothing, and when its family takes Fast, a set of `{harness: "default", speed: "fast"}` is checked against the harness the Chief already runs and can come back `invalid_speed`. The rule this PR is about is that a row never offers what a set would refuse, so such a row is now unavailable, with the reason naming what to do (rename the profile in acpmux). The fix is in the offer, not in the sentinel: changing what `default` means in a set would break every caller that clears a field with it. Covered by an_offer_never_names_a_harness_effort_or_speed_a_set_would_refuse, which also asserts the dead set: no refusal, and the harness does not change. Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a174e4-3868-7f51-8f3d-21b34ad270b2&host=huge-blue --- .../optchat-chief/src/brain/harness_offer.rs | 30 ++++++++++++++--- .../optchat-chief/tests/engine_offer.rs | 32 ++++++++++++++++++- 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/Native/OptChat/optchat-chief/src/brain/harness_offer.rs b/Native/OptChat/optchat-chief/src/brain/harness_offer.rs index cd65d2140863..80639d1b30b8 100644 --- a/Native/OptChat/optchat-chief/src/brain/harness_offer.rs +++ b/Native/OptChat/optchat-chief/src/brain/harness_offer.rs @@ -125,6 +125,22 @@ fn disambiguate(rows: &mut [Value]) { } } +/// Why a set could not name this harness at all: `chief.engine.set` reads an +/// empty value, or the literal `default`, as "clear the choice" (its `set` +/// closure in ``engine_control``), so a profile acpmux calls either one can +/// never be switched to. The row says so rather than offer a choice that +/// silently keeps the harness the Chief already runs. +fn unnameable(id: &str) -> Option { + let trimmed = id.trim(); + (trimmed.is_empty() || trimmed == "default").then(|| { + format!( + "this Chief cannot be switched to a harness acpmux calls {:?}: a set reads that as \ + clearing the choice. Rename the profile in acpmux.", + id + ) + }) +} + fn row( answer: &Value, id: &str, @@ -147,11 +163,17 @@ fn row( // start, so a harness acpmux gained since would be refused with // `unknown_harness`. Say that here instead of offering a row the picker // cannot switch to. - let reason = plan.admitted.err().or_else(|| { - (!started_with.is_empty() && !started_with.contains_key(id)).then(|| { - format!("acpmux gained {id} after this Chief started; restart the Chief to run on it") + let reason = plan + .admitted + .err() + .or_else(|| { + (!started_with.is_empty() && !started_with.contains_key(id)).then(|| { + format!( + "acpmux gained {id} after this Chief started; restart the Chief to run on it" + ) + }) }) - }); + .or_else(|| unnameable(id)); let family = str_of("family").unwrap_or(family_name(chief_family)); let display = str_of("displayName").or(catalog_name).unwrap_or(id); diff --git a/Native/OptChat/optchat-chief/tests/engine_offer.rs b/Native/OptChat/optchat-chief/tests/engine_offer.rs index 92daa9bc43dd..554267bacd92 100644 --- a/Native/OptChat/optchat-chief/tests/engine_offer.rs +++ b/Native/OptChat/optchat-chief/tests/engine_offer.rs @@ -91,6 +91,8 @@ fn harnesses() -> Value { "displayName": "claude"}, // An empty catalog name is no name: this row falls back to its id. "nameless": {"kind": "acp", "argv": ["nameless"], "family": "other"}, + // A profile acpmux calls `default`, the word a set reads as clearing the choice. + "default": {"kind": "acp", "argv": ["default-acp"], "family": "codex"}, "aider": {"kind": "terminal", "argv": ["aider"], "family": "aider"}, }, "catalog": {"harnesses": [ @@ -352,6 +354,7 @@ fn the_engine_answer_offers_every_harness_with_what_it_supports() { ("claude-sr", Family::Claude), ("claude-vm", Family::Claude), ("codex", Family::Codex), + ("default", Family::Codex), ("nameless", Family::Other), ("opencode", Family::Other), ("unii", Family::Other), @@ -371,6 +374,7 @@ fn the_engine_answer_offers_every_harness_with_what_it_supports() { "claude-sr", "claude-vm", "codex", + "default", "nameless", "opencode", "unii" @@ -462,7 +466,11 @@ fn an_offer_never_names_a_harness_effort_or_speed_a_set_would_refuse() { // it as. let mut brain = brain( dir.path(), - &[("claude-sr", Family::Claude), ("codex", Family::Other)], + &[ + ("claude-sr", Family::Claude), + ("codex", Family::Other), + ("default", Family::Codex), + ], ); let answer = engine(&mut brain, EngineRequest::Show); let offered = answer["offered"]["harnesses"].as_array().unwrap(); @@ -505,6 +513,28 @@ fn an_offer_never_names_a_harness_effort_or_speed_a_set_would_refuse() { assert_eq!(invalid["error"]["code"], "invalid_effort", "{invalid:#}"); let slow = engine(&mut brain, set(Some("codex"), None, Some("fast"))); assert_eq!(slow["error"]["code"], "invalid_speed", "{slow:#}"); + + // A profile acpmux calls `default`: a set reads that value as clearing the + // choice, so it never switches to the profile and keeps the harness the + // Chief already runs. The row says so instead of offering a dead choice. + let sentinel = by_id("default"); + assert_eq!(sentinel["unavailable"], true, "{sentinel:#}"); + assert!( + sentinel["unavailableReason"] + .as_str() + .is_some_and(|r| r.contains("clearing the choice")), + "{sentinel:#}" + ); + // The set's answer is the dead choice the row warns about: no refusal, and + // the harness it ran on before is still the one it runs on. + let before = engine(&mut brain, EngineRequest::Show)["engine"]["harness"].clone(); + let cleared = engine(&mut brain, set(Some("default"), None, None)); + assert_eq!(cleared.get("error"), None, "{cleared:#}"); + assert_eq!( + engine(&mut brain, EngineRequest::Show)["engine"]["harness"], + before, + "a set of the `default` profile switched nothing: {cleared:#}" + ); } /// Two profiles under one name keep apart by their ids, and a third profile