From 354ffb31b22bb390ab2def31ecd4faeac2f4f490 Mon Sep 17 00:00:00 2001 From: Kayshen-X Date: Thu, 14 May 2026 15:59:11 +0800 Subject: [PATCH] fix(shell-core/chat): replace direct-HTTP provider model with 4-backend architecture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User correction: the previous `chat_provider.rs` (commits `548c5336` + `2c2b7f60`) assumed a direct-HTTP-per-provider model with Anthropic / OpenAI-compat / Gemini / etc. each carrying its own endpoint + model defaults. That's not the architecture decision — per the project_agent_runtime memory, OP runs FOUR distinct backend categories: - BuiltIn → `agent-rs` QueryEngine in-process (the cross-product Rust agent crate; lives at /Users/kayshen/Workspace/ZSeven-W/agent-rs) - Subprocess(Cli) → spawn `claude` / `gemini` / `gh-copilot` + talk line-delimited JSON over stdio - HttpServer(Cli) → spawn `codex serve` / `opencode serve` + hit local HTTP endpoint with reqwest - Acp → Agent Client Protocol (ndJSON over stdio), the open extension point for third-party agents OP doesn't ship a dedicated adapter for API rewrite in `chat_provider.rs`: - `CliName::{ ClaudeCode, Gemini, Copilot, Codex, OpenCode }` enumerates the 5 first-party CLI backends. Each carries `label()` (human display), `default_binary()` (PATH lookup), and `backend()` (which Subprocess/HttpServer transport it uses — table matches the memo verbatim). - `ChatProviderKind::{ BuiltIn | Subprocess(CliName) | HttpServer(CliName) | Acp }` replaces the previous flat Anthropic/OpenAI-compat/etc enum. - `ChatProviderConfig::new(kind)` pre-fills `binary` from `default_binary()` for Subprocess/HttpServer kinds; BuiltIn and Acp leave it empty. - `ChatProvider` trait + `EchoProvider` test double unchanged. The streaming `ChatDelta` shape stays close to agent-rs's `stream::Event` (TextDelta / Thinking / ToolUse / Done / Error + StopReason variants). - Real transport implementations live in the future `pen-agent-cli` desktop crate per the memo — shell-core stays wasm32-clean (no tokio / reqwest / process-spawn). Tests (4 new + 1 carried): - cli_name_backend_table_matches_architecture_memo (verbatim map from the memo's table) - cli_default_binary_uses_expected_names - provider_config_new_seeds_binary_for_cli_kinds (BuiltIn / Acp leave it empty) - cli_label_is_human_readable - echo_provider_replays_script (carries the test double's behavior forward) Tests total: 250 shell-core. Wasm32 build clean. --- .../src/chat_provider.rs | 284 ++++++++++-------- 1 file changed, 162 insertions(+), 122 deletions(-) diff --git a/crates/openpencil-shell-core/src/chat_provider.rs b/crates/openpencil-shell-core/src/chat_provider.rs index 153a6862f..aa19e9a71 100644 --- a/crates/openpencil-shell-core/src/chat_provider.rs +++ b/crates/openpencil-shell-core/src/chat_provider.rs @@ -1,112 +1,149 @@ -//! AI chat provider abstraction. Real LLM endpoints (Anthropic, -//! OpenAI-compatible, OpenCode, Gemini, etc.) implement this trait; -//! the chat widget calls `send` to push a user message and receives -//! `ChatDelta`s via the iterator until `Done`. Mirrors the streaming -//! shape `apps/web/src/services/ai/ai-service.ts` uses. +//! Chat / agent provider abstraction. Three backend categories +//! mirror the architecture decision in +//! [[project_agent_runtime]]: //! -//! v1 scope: trait + an `EchoProvider` test double. Real HTTP -//! transport + per-provider serialisation arrive with the agent -//! runtime port (`packages/agent-native` → Rust subprocess / -//! NAPI / direct). +//! - **BuiltIn** — `agent-rs` crate's `QueryEngine` runs in-process +//! against the user's chosen `Provider` (Anthropic, OpenAI-compat, +//! Ollama, ...). This is the OP-native agent. +//! - **Subprocess(CliName)** — spawn an external CLI binary +//! (Claude Code / Gemini / Copilot) and pipe line-delimited JSON +//! over its stdin / stdout. +//! - **HttpServer(CliName)** — spawn `codex serve` / `opencode serve` +//! and hit its local HTTP/SSE endpoint with reqwest. +//! - **Acp** — Agent Client Protocol (ndJSON over stdio); the +//! open extension point for third-party agents OP doesn't ship a +//! dedicated adapter for. +//! +//! shell-core only carries the data shapes + the trait. Real +//! transports live in the future `pen-agent-cli` crate (desktop- +//! side) because they pull tokio / reqwest / process-spawn which +//! shell-core's wasm32 target can't accept. The `EchoProvider` test +//! double stays here so widget tests don't need a real backend. -/// Provider backend kinds the editor can talk to. Mirrors the -/// settings modal's Agents tab (Claude / Codex / OpenCode / Copilot -/// / Gemini) plus the OpenAI-compat backend that covers Anthropic, -/// OpenAI, Ollama, and most local servers. +/// Which external CLI a Subprocess / HttpServer / Acp provider +/// is bridging to. Subprocess + Acp paths spawn the binary and +/// talk over stdio; HttpServer spawns with a `serve` subcommand +/// then connects via HTTP. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum ChatProviderKind { - Anthropic, - OpenAiCompat, +pub enum CliName { + /// Anthropic's `claude` CLI (subprocess IPC). + ClaudeCode, + /// Google's `gemini` CLI (subprocess IPC). Gemini, + /// GitHub Copilot CLI (subprocess IPC). Copilot, + /// OpenAI Codex CLI (HTTP server mode). + Codex, + /// OpenCode AI's CLI (HTTP server mode). OpenCode, - Ollama, } -impl ChatProviderKind { +impl CliName { pub fn label(self) -> &'static str { match self { - ChatProviderKind::Anthropic => "Claude", - ChatProviderKind::OpenAiCompat => "OpenAI Compatible", - ChatProviderKind::Gemini => "Gemini", - ChatProviderKind::Copilot => "GitHub Copilot", - ChatProviderKind::OpenCode => "OpenCode", - ChatProviderKind::Ollama => "Ollama", + CliName::ClaudeCode => "Claude Code", + CliName::Gemini => "Gemini", + CliName::Copilot => "GitHub Copilot", + CliName::Codex => "Codex", + CliName::OpenCode => "OpenCode", + } + } + /// Default binary name on PATH. Users override via + /// `ChatProviderConfig::binary` when a non-standard install + /// location applies. + pub fn default_binary(self) -> &'static str { + match self { + CliName::ClaudeCode => "claude", + CliName::Gemini => "gemini", + CliName::Copilot => "gh-copilot", + CliName::Codex => "codex", + CliName::OpenCode => "opencode", + } + } + /// Which backend transport this CLI uses. Mirrors the table in + /// [[project_agent_runtime]] memory: + /// Claude/Gemini/Copilot = subprocess IPC; Codex/OpenCode = HTTP server. + pub fn backend(self) -> ChatProviderKind { + match self { + CliName::ClaudeCode | CliName::Gemini | CliName::Copilot => { + ChatProviderKind::Subprocess(self) + } + CliName::Codex | CliName::OpenCode => ChatProviderKind::HttpServer(self), } } } -/// Per-provider config the chat panel persists. `api_key` is -/// opaque + provider-specific (Bearer for Anthropic; sk-... for -/// OpenAI; etc.). `endpoint` overrides the default base URL — used -/// for OpenAI-compat backends pointing at local servers. +/// Provider backend category. The widget host dispatches each +/// chat message through the corresponding transport in +/// `pen-agent-cli`. Built-in keeps everything in-process. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ChatProviderKind { + /// `agent-rs` QueryEngine in-process (built-in agent). + BuiltIn, + /// Spawn the CLI binary + talk over stdio (line-delimited JSON). + Subprocess(CliName), + /// Spawn ` serve` + talk over the resulting HTTP endpoint. + HttpServer(CliName), + /// Agent Client Protocol via ndJSON over stdio. The catch-all + /// for third-party agents OP doesn't carry a dedicated adapter. + Acp, +} + +/// Persisted per-provider config the chat panel + agent-settings +/// modal own. Fields are interpreted per `kind`: +/// - `BuiltIn` — `api_key` + `endpoint` + `model` are passed to the +/// underlying `agent-rs` Provider (e.g. Anthropic). +/// - `Subprocess` / `HttpServer` / `Acp` — `binary` overrides +/// `CliName::default_binary()`; `endpoint` for HttpServer is the +/// bind URL (defaults to `127.0.0.1:0` so the OS picks a port). #[derive(Debug, Clone, PartialEq, Eq)] pub struct ChatProviderConfig { pub kind: ChatProviderKind, pub api_key: String, pub endpoint: String, pub model: String, + pub binary: String, } impl ChatProviderConfig { - /// Default endpoint URL for `kind`. The chat panel's "endpoint" - /// input pre-fills with this; user overrides flow back via the - /// `endpoint` field. - pub fn default_endpoint(kind: ChatProviderKind) -> &'static str { - match kind { - ChatProviderKind::Anthropic => "https://api.anthropic.com", - ChatProviderKind::OpenAiCompat => "https://api.openai.com", - ChatProviderKind::Gemini => "https://generativelanguage.googleapis.com", - ChatProviderKind::Copilot => "https://api.githubcopilot.com", - ChatProviderKind::OpenCode => "https://opencode.local", - ChatProviderKind::Ollama => "http://localhost:11434", - } - } - /// Sensible model default for `kind` — the chat panel's "model" - /// input pre-fills with this. Mirrors TS app's default-model - /// table. - pub fn default_model(kind: ChatProviderKind) -> &'static str { - match kind { - ChatProviderKind::Anthropic => "claude-sonnet-4-6", - ChatProviderKind::OpenAiCompat => "gpt-4o-mini", - ChatProviderKind::Gemini => "gemini-1.5-flash", - ChatProviderKind::Copilot => "gpt-4o-copilot", - ChatProviderKind::OpenCode => "opencode-default", - ChatProviderKind::Ollama => "llama3.2", + /// Empty default — every string blank, kind = BuiltIn. The + /// settings modal pre-fills user-facing inputs from this seed. + pub fn new(kind: ChatProviderKind) -> Self { + Self { + kind, + api_key: String::new(), + endpoint: String::new(), + model: String::new(), + binary: match kind { + ChatProviderKind::Subprocess(cli) + | ChatProviderKind::HttpServer(cli) => cli.default_binary().into(), + _ => String::new(), + }, } } } /// Streaming delta from a provider — text fragments, tool calls, -/// status events. Mirrors `streaming/events.zig::Event` in -/// agent-native. +/// status events. Mirrors `agent-rs`'s `stream::Event` enum (which +/// is the cross-product source of truth); shell-core duplicates the +/// shape so widget code doesn't need agent-rs in its dep graph. #[derive(Debug, Clone, PartialEq, Eq)] pub enum ChatDelta { - /// Text fragment appended to the assistant's reply. TextDelta(String), - /// Thinking trace (Claude's "thinking" content blocks). Thinking(String), - /// Tool invocation — name + JSON-stringified args. ToolUse { name: String, args: String }, - /// Final assistant message, stop-reason known. Done { stop_reason: StopReason }, - /// Provider-side error — abort + surface to user. Error(String), } #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum StopReason { - /// Model finished naturally. EndTurn, - /// User-side `AbortController` interrupted the stream. Aborted, - /// `maxOutputTokens` budget reached. MaxTokens, - /// Tool call sequence — caller should run the tool + resume. ToolUse, } -/// One pending request. Providers hold this in their stream state. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ChatRequest { pub system_prompt: String, @@ -114,20 +151,20 @@ pub struct ChatRequest { pub max_output_tokens: u32, } -/// Provider abstraction. `send` initiates the stream + returns an -/// iterator of deltas. Errors surface as `ChatDelta::Error` rather -/// than `Result` so partial output is preserved. +/// Provider abstraction the widget host calls. Implementations live +/// in the future `pen-agent-cli` desktop crate (one per kind): +/// `BuiltInProvider` wraps `agent-rs`, `SubprocessProvider` / +/// `HttpServerProvider` / `AcpProvider` each own their transport. +/// shell-core only carries the trait + the test double. pub trait ChatProvider: Send + Sync { fn provider_label(&self) -> &str; fn send(&self, request: ChatRequest) -> Box + Send>; } -/// Test double that replays a fixed script. Useful for chat-widget -/// unit tests that need a deterministic stream without an actual -/// LLM round-trip. +/// Test double — replays a fixed delta script. Lets the chat widget +/// run unit tests without spinning up agent-rs / a CLI subprocess / +/// an HTTP server. pub struct EchoProvider { - /// Sequence of deltas to yield in order. Last entry should be - /// `Done { stop_reason: EndTurn }` for typical fixtures. pub script: Vec, } @@ -144,19 +181,66 @@ impl ChatProvider for EchoProvider { mod tests { use super::*; + #[test] + fn cli_name_backend_table_matches_architecture_memo() { + // project_agent_runtime memory: + // Subprocess IPC = Claude Code / Gemini / Copilot + // HTTP server = Codex / OpenCode + assert!(matches!( + CliName::ClaudeCode.backend(), + ChatProviderKind::Subprocess(_) + )); + assert!(matches!( + CliName::Gemini.backend(), + ChatProviderKind::Subprocess(_) + )); + assert!(matches!( + CliName::Copilot.backend(), + ChatProviderKind::Subprocess(_) + )); + assert!(matches!( + CliName::Codex.backend(), + ChatProviderKind::HttpServer(_) + )); + assert!(matches!( + CliName::OpenCode.backend(), + ChatProviderKind::HttpServer(_) + )); + } + + #[test] + fn cli_default_binary_uses_expected_names() { + assert_eq!(CliName::ClaudeCode.default_binary(), "claude"); + assert_eq!(CliName::Codex.default_binary(), "codex"); + assert_eq!(CliName::OpenCode.default_binary(), "opencode"); + } + + #[test] + fn provider_config_new_seeds_binary_for_cli_kinds() { + let cfg = ChatProviderConfig::new(ChatProviderKind::Subprocess(CliName::ClaudeCode)); + assert_eq!(cfg.binary, "claude"); + let cfg2 = ChatProviderConfig::new(ChatProviderKind::HttpServer(CliName::Codex)); + assert_eq!(cfg2.binary, "codex"); + // BuiltIn / Acp leave binary empty — built-in needs no + // spawn target; Acp's binary is user-supplied per-instance. + let cfg3 = ChatProviderConfig::new(ChatProviderKind::BuiltIn); + assert!(cfg3.binary.is_empty()); + let cfg4 = ChatProviderConfig::new(ChatProviderKind::Acp); + assert!(cfg4.binary.is_empty()); + } + #[test] fn echo_provider_replays_script() { let p = EchoProvider { script: vec![ ChatDelta::TextDelta("Hello".into()), - ChatDelta::TextDelta(", world!".into()), ChatDelta::Done { stop_reason: StopReason::EndTurn, }, ], }; let req = ChatRequest { - system_prompt: "be helpful".into(), + system_prompt: String::new(), user_message: "hi".into(), max_output_tokens: 1024, }; @@ -166,58 +250,14 @@ mod tests { _ => panic!(), } match iter.next() { - Some(ChatDelta::TextDelta(s)) => assert_eq!(s, ", world!"), + Some(ChatDelta::Done { .. }) => {} _ => panic!(), } - match iter.next() { - Some(ChatDelta::Done { - stop_reason: StopReason::EndTurn, - }) => {} - _ => panic!(), - } - assert!(iter.next().is_none()); } #[test] - fn echo_provider_label_is_echo() { - let p = EchoProvider { script: Vec::new() }; - assert_eq!(p.provider_label(), "echo"); - } - - #[test] - fn provider_kind_defaults_match_ts_table() { - // Pick three to sanity-check rather than full coverage — - // the table is mechanical mirror of TS defaults. - assert!(ChatProviderConfig::default_endpoint(ChatProviderKind::Anthropic) - .starts_with("https://api.anthropic.com")); - assert!(ChatProviderConfig::default_endpoint(ChatProviderKind::Ollama) - .starts_with("http://localhost")); - assert_eq!( - ChatProviderConfig::default_model(ChatProviderKind::Anthropic), - "claude-sonnet-4-6" - ); - } - - #[test] - fn provider_kind_label_is_human_readable() { - assert_eq!(ChatProviderKind::Anthropic.label(), "Claude"); - assert_eq!(ChatProviderKind::Copilot.label(), "GitHub Copilot"); - } - - #[test] - fn error_delta_carries_message() { - let p = EchoProvider { - script: vec![ChatDelta::Error("rate limited".into())], - }; - let req = ChatRequest { - system_prompt: String::new(), - user_message: String::new(), - max_output_tokens: 0, - }; - let mut iter = p.send(req); - match iter.next() { - Some(ChatDelta::Error(m)) => assert_eq!(m, "rate limited"), - _ => panic!(), - } + fn cli_label_is_human_readable() { + assert_eq!(CliName::ClaudeCode.label(), "Claude Code"); + assert_eq!(CliName::OpenCode.label(), "OpenCode"); } }