From a3f062b52287436c408919c0e70571c9b4ec4f63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Fri, 17 Jul 2026 20:15:56 -0400 Subject: [PATCH] =?UTF-8?q?F6:=20kimi-cli=20command=20parity=20=E2=80=94?= =?UTF-8?q?=20kigi=20acp,=20mcp=20auth,=20--mcp-config-file?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `kigi acp`: top-level alias for the stdio ACP server (kimi-cli `acp`). AgentArgs.mode is now optional — bare `kigi agent` and `kigi acp` both default to stdio at dispatch; `kigi acp ` is rejected. - `kigi mcp auth `: authorize an OAuth-enabled remote MCP server (kimi-cli `mcp auth`). Reuses the doctor's interactive connection path: starts the named server with OauthInteractivity::Interactive (browser flow when required), completes the handshake, and reports the tool count. Stdio servers and unknown names fail with actionable errors. - `--mcp-config-file ` (repeatable, global): extra MCP config files in the .mcp.json shape ({"mcpServers": {...}}), kimi-cli semantics. Files are validated at parse time (fail fast on unreadable/invalid JSON), carried across the TUI -> shell boundary via KIGI_MCP_CONFIG_FILES, and merged at HIGHEST priority — an explicitly passed file overrides every config scope. Covered by loader unit tests and verified live: an injected server surfaces in the session's x.ai/mcp/servers_updated notification via both the flag and the env. The stale bare-agent-requires-mode CLI test is re-contracted to the new default-to-stdio behavior. --- crates/codegen/kigi-bin/src/main.rs | 42 ++++++++- crates/codegen/kigi-shell/src/mcp_doctor.rs | 44 +++++++++ .../codegen/kigi-shell/src/util/config/mcp.rs | 92 +++++++++++++++++++ crates/codegen/kigi-tui/src/app/cli.rs | 42 ++++++++- crates/codegen/kigi-tui/src/app/mod.rs | 13 ++- crates/codegen/kigi-tui/src/mcp_cmd.rs | 25 +++++ 6 files changed, 246 insertions(+), 12 deletions(-) diff --git a/crates/codegen/kigi-bin/src/main.rs b/crates/codegen/kigi-bin/src/main.rs index aa442d0..a860edc 100644 --- a/crates/codegen/kigi-bin/src/main.rs +++ b/crates/codegen/kigi-bin/src/main.rs @@ -896,8 +896,9 @@ async fn run_agent_command( drop(kigi_shell::agent::models::start_early_prefetch(None)); kigi_shell::agent::mvp_agent::warm_async_http_client(); tokio::task::spawn_blocking(|| {}); - let is_stdio = matches!(agent_args.mode, AgentCmd::Stdio); - let is_leader = matches!(agent_args.mode, AgentCmd::Leader(_)); + let mode = agent_args.mode.clone().unwrap_or(AgentCmd::Stdio); + let is_stdio = matches!(mode, AgentCmd::Stdio); + let is_leader = matches!(mode, AgentCmd::Leader(_)); if !is_stdio && !is_leader { eprintln!( "Kigi v{}", @@ -965,7 +966,7 @@ async fn run_agent_command( storage_mode: None, }); let agent_memory_config = agent_config.memory_config.clone(); - let leader_eligible = matches!(&agent_args.mode, AgentCmd::Stdio); + let leader_eligible = is_stdio; let (use_leader, policy_disable_reason) = resolve_use_leader( agent_args.leader, agent_args.no_leader, @@ -1128,7 +1129,7 @@ async fn run_agent_command( } } } - match agent_args.mode { + match mode { AgentCmd::Stdio => run_stdio_agent(&agent_config, None, agent_memory_config).await, AgentCmd::Serve(a) => { let secret = a.get_secret(); @@ -1439,6 +1440,21 @@ async fn async_main() -> Result<()> { if let Some(ref socket) = args.leader_socket { unsafe { std::env::set_var(kigi_shell::leader::LEADER_SOCKET_ENV, socket) }; } + if !args.mcp_config_file.is_empty() { + // Fail fast on unusable files: an explicitly passed config that + // cannot be read must abort, not silently degrade the session. + for path in &args.mcp_config_file { + let content = std::fs::read_to_string(path) + .map_err(|e| anyhow::anyhow!("--mcp-config-file {}: {e}", path.display()))?; + serde_json::from_str::(&content).map_err(|e| { + anyhow::anyhow!("--mcp-config-file {}: invalid JSON: {e}", path.display()) + })?; + } + let joined = std::env::join_paths(&args.mcp_config_file).map_err(|e| { + anyhow::anyhow!("--mcp-config-file paths cannot be joined into the env: {e}") + })?; + unsafe { std::env::set_var(kigi_shell::util::config::MCP_CONFIG_FILES_ENV, joined) }; + } if let Some(ref path) = args.debug_file { unsafe { std::env::set_var("KIGI_DEBUG_LOG", path); @@ -1505,6 +1521,24 @@ async fn async_main() -> Result<()> { } return Ok(()); } + Command::Acp(agent_args) => { + if let Some(mode) = &agent_args.mode { + anyhow::bail!( + "`kigi acp` always runs the stdio server; `{mode:?}` does not apply. \ + Use `kigi agent ...` for other runtime modes." + ); + } + enforce_minimum_version_or_exit(&update_config).await; + return run_agent_command( + agent_args, + args.permission_mode_flag.clone(), + args.trust, + args.no_auto_update, + args.disable_web_search, + &update_config, + ) + .await; + } Command::Agent(agent_args) => { if args.leader || args.no_leader { let flag = if args.leader { diff --git a/crates/codegen/kigi-shell/src/mcp_doctor.rs b/crates/codegen/kigi-shell/src/mcp_doctor.rs index dfd16a5..c4d786c 100644 --- a/crates/codegen/kigi-shell/src/mcp_doctor.rs +++ b/crates/codegen/kigi-shell/src/mcp_doctor.rs @@ -513,6 +513,50 @@ async fn check_server( // ── Entry point ───────────────────────────────────────────────── +/// `kigi mcp auth ` (kimi-cli `mcp auth` parity): start the named +/// remote server with interactive OAuth — the browser flow opens when the +/// server requires it — complete the handshake, and return the discovered +/// tool count. Errors are user-facing strings. +pub async fn run_auth(cwd: &Path, name: &str) -> Result { + let (_sources, discovered) = discover_servers(cwd); + let all_names: Vec = discovered + .iter() + .map(|d| mcp_servers::mcp_server_name(&d.server).to_string()) + .collect(); + let Some(found) = discovered + .into_iter() + .find(|d| mcp_servers::mcp_server_name(&d.server) == name) + else { + let hint = if all_names.is_empty() { + String::new() + } else { + format!(" Available servers: {}", all_names.join(", ")) + }; + return Err(format!("MCP server '{name}' not found.{hint}")); + }; + if matches!(found.server, agent_client_protocol::McpServer::Stdio(_)) { + return Err(format!( + "MCP server '{name}' is a local stdio server; OAuth applies to \ + remote (http/sse) servers only." + )); + } + let fail = |c: Check| { + let mut msg = c.label; + if let Some(detail) = c.detail { + msg = format!("{msg}: {detail}"); + } + msg + }; + let (client, _started) = check_server_start(found.server, cwd).await.map_err(fail)?; + let (service, _handshake) = check_handshake(&client).await.map_err(fail)?; + use kigi_mcp::rmcp::model::PaginatedRequestParams; + let tools = service + .list_tools(Some(PaginatedRequestParams::default())) + .await + .map_err(|e| format!("tools/list failed after authorization: {e}"))?; + Ok(tools.tools.len()) +} + pub async fn run_doctor(cwd: &Path, name_filter: Option<&str>) -> DoctorReport { let (mut sources, mut discovered) = discover_servers(cwd); diff --git a/crates/codegen/kigi-shell/src/util/config/mcp.rs b/crates/codegen/kigi-shell/src/util/config/mcp.rs index 76467cc..c5d4fad 100644 --- a/crates/codegen/kigi-shell/src/util/config/mcp.rs +++ b/crates/codegen/kigi-shell/src/util/config/mcp.rs @@ -138,6 +138,12 @@ pub fn load_mcp_servers_with_oauth( servers_map.entry(name).or_insert(config); } + // `--mcp-config-file` entries override everything: an explicitly passed + // file is the most specific intent (kimi-cli parity, F6). + for (name, config) in load_cli_mcp_config_files_as_configs() { + servers_map.insert(name, config); + } + let mut oauth_configs = McpOAuthConfigMap::new(); let mut acp_servers = Vec::new(); @@ -267,6 +273,11 @@ pub(crate) fn reload_mcp_servers_merged( servers.entry(name).or_insert(config); } + // `--mcp-config-file` entries override everything (kimi-cli parity, F6). + for (name, config) in load_cli_mcp_config_files_as_configs() { + servers.insert(name, config); + } + let sub = &crate::config::expand_env_vars_in_string; servers .into_iter() @@ -966,6 +977,32 @@ fn load_claude_json_mcp_servers_from( } /// Read and parse a JSON file. Returns `None` on I/O or parse errors (logged). +/// Env var carrying `--mcp-config-file` paths across the TUI → shell +/// process boundary (joined with the platform path separator). kimi-cli +/// parity (F6): each file uses the `.mcp.json` shape +/// (`{"mcpServers": {...}}`) and its entries override every config-scope +/// source — an explicitly passed file is the most specific intent. +pub const MCP_CONFIG_FILES_ENV: &str = "KIGI_MCP_CONFIG_FILES"; + +/// Servers from `--mcp-config-file` paths (via [`MCP_CONFIG_FILES_ENV`]), +/// later files winning on name conflicts. Unreadable/unparsable files fail +/// loudly at CLI parse time; here they only warn (the env may outlive the +/// file). +pub(crate) fn load_cli_mcp_config_files_as_configs() -> IndexMap { + let mut result = IndexMap::new(); + let Some(paths) = std::env::var_os(MCP_CONFIG_FILES_ENV) else { + return result; + }; + for path in std::env::split_paths(&paths) { + if let Some(config) = read_mcp_json(&path) { + for (name, cfg) in config.mcp_servers { + result.insert(name, cfg); + } + } + } + result +} + pub(crate) fn read_mcp_json(path: &std::path::Path) -> Option { let content = std::fs::read_to_string(path) .map_err(|e| { @@ -1148,6 +1185,61 @@ pub fn session_registry_from_toml_opt(root: &TomlValue) -> Option { #[cfg(test)] mod tests { + /// `--mcp-config-file` env plumbing: later files override earlier ones, + /// and entries parse with the `.mcp.json` shape. Serialized on the env + /// var via the kigi_env guard (process-global state). + #[test] + #[serial_test::serial(kigi_mcp_config_files_env)] + fn cli_mcp_config_files_load_in_order_with_later_files_winning() { + let dir = tempfile::tempdir().unwrap(); + let first = dir.path().join("first.json"); + let second = dir.path().join("second.json"); + std::fs::write( + &first, + serde_json::json!({ "mcpServers": { + "shared": { "command": "first-cmd" }, + "only-first": { "command": "keep-me" } + }}) + .to_string(), + ) + .unwrap(); + std::fs::write( + &second, + serde_json::json!({ "mcpServers": { + "shared": { "command": "second-cmd" } + }}) + .to_string(), + ) + .unwrap(); + let joined = std::env::join_paths([&first, &second]).unwrap(); + let _guard = + kigi_env::EnvVarGuard::set(super::MCP_CONFIG_FILES_ENV, joined.to_str().unwrap()); + + let command_of = |c: &McpServerConfig| match &c.transport { + McpServerTransportConfig::Stdio { command, .. } => command.clone(), + other => panic!("expected stdio transport, got {other:?}"), + }; + let configs = super::load_cli_mcp_config_files_as_configs(); + assert_eq!(configs.len(), 2); + assert_eq!( + configs.get("shared").map(command_of).as_deref(), + Some("second-cmd"), + "the later file must win on name conflicts" + ); + assert_eq!( + configs.get("only-first").map(command_of).as_deref(), + Some("keep-me") + ); + } + + /// Without the env var the loader is inert. + #[test] + #[serial_test::serial(kigi_mcp_config_files_env)] + fn cli_mcp_config_files_absent_env_is_empty() { + let _guard = kigi_env::EnvVarGuard::remove(super::MCP_CONFIG_FILES_ENV); + assert!(super::load_cli_mcp_config_files_as_configs().is_empty()); + } + use super::*; use toml::Value as TomlValue; diff --git a/crates/codegen/kigi-tui/src/app/cli.rs b/crates/codegen/kigi-tui/src/app/cli.rs index d8adbb9..63259de 100644 --- a/crates/codegen/kigi-tui/src/app/cli.rs +++ b/crates/codegen/kigi-tui/src/app/cli.rs @@ -7,6 +7,8 @@ use std::path::PathBuf; /// Top-level commands for the pager binary. #[derive(Debug, Clone, Subcommand)] pub enum Command { + /// Run the ACP server over stdio (alias of `agent stdio`; kimi-cli parity) + Acp(Box), /// Run Kigi without the interactive UI Agent(Box), /// Show the configuration Kigi discovers for this directory @@ -270,9 +272,10 @@ pub struct AgentArgs { /// Override the public xAI API base URL. #[arg(long = "xai-api-base-url")] pub xai_api_base_url: Option, - /// Agent runtime mode + /// Agent runtime mode. Optional: bare `kigi agent` (and the `kigi acp` + /// alias) default to stdio. #[command(subcommand)] - pub mode: AgentCmd, + pub mode: Option, } impl AgentArgs { /// Canonicalized `--plugin-dir` paths, warning to stderr and skipping @@ -387,6 +390,15 @@ pub struct PagerArgs { /// Working directory. #[arg(long)] pub cwd: Option, + /// MCP config file to load (`{"mcpServers": {...}}`). Repeat the option + /// for multiple files; entries override config.toml-scoped servers. + #[arg( + long = "mcp-config-file", + value_name = "PATH", + global = true, + value_hint = ValueHint::FilePath + )] + pub mcp_config_file: Vec, /// Use a custom leader socket path instead of the default `~/.kigi/leader.sock`. #[arg( long = "leader-socket", @@ -923,6 +935,30 @@ mod tests { let err = PagerArgs::try_parse_from(["grok", "--minimal", "--fullscreen"]).unwrap_err(); assert_eq!(err.kind(), clap::error::ErrorKind::ArgumentConflict); } + /// kimi-cli parity (F6): bare `kigi acp` runs the stdio ACP server, and + /// bare `kigi agent` defaults to stdio at dispatch (mode is optional). + #[test] + fn acp_alias_and_bare_agent_default_to_stdio() { + let args = PagerArgs::try_parse_from(["kigi", "acp"]).unwrap(); + let Some(Command::Acp(agent)) = args.command else { + panic!("expected acp subcommand"); + }; + assert!(agent.mode.is_none(), "acp must not carry a runtime mode"); + + let args = PagerArgs::try_parse_from(["kigi", "agent"]).unwrap(); + let Some(Command::Agent(agent)) = args.command else { + panic!("expected agent subcommand"); + }; + assert!(agent.mode.is_none(), "bare agent defaults to stdio"); + + // Explicit modes still parse under `agent`. + let args = PagerArgs::try_parse_from(["kigi", "agent", "stdio"]).unwrap(); + let Some(Command::Agent(agent)) = args.command else { + panic!("expected agent subcommand"); + }; + assert!(matches!(agent.mode, Some(AgentCmd::Stdio))); + } + #[test] fn agent_plugin_dir_repeatable_and_canonicalized() { let tmp = tempfile::tempdir().unwrap(); @@ -948,7 +984,7 @@ mod tests { panic!("expected agent subcommand"); }; assert_eq!(agent.plugin_dirs, vec![dir.clone(), file, missing]); - assert!(matches!(agent.mode, AgentCmd::Stdio)); + assert!(matches!(agent.mode, Some(AgentCmd::Stdio))); assert!(agent.no_leader); assert_eq!( agent.canonical_plugin_dirs(), diff --git a/crates/codegen/kigi-tui/src/app/mod.rs b/crates/codegen/kigi-tui/src/app/mod.rs index 8f06a7f..c72cf95 100644 --- a/crates/codegen/kigi-tui/src/app/mod.rs +++ b/crates/codegen/kigi-tui/src/app/mod.rs @@ -1359,12 +1359,15 @@ mod tests { assert!(args.no_leader); assert!(matches!(args.command, Some(Command::Agent(_)))); } - /// The `agent` subcommand requires an explicit mode (stdio|serve|leader); - /// the old bare `agent` headless form was removed. + /// Bare `agent` parses with no mode and defaults to stdio at dispatch + /// (kimi-cli parity, F6 — the `acp` alias relies on the same default). #[test] - fn cli_bare_agent_subcommand_requires_mode() { - assert!(try_parse_pager(&["grok-pager", "--leader", "agent"]).is_err()); - assert!(try_parse_pager(&["grok-pager", "agent"]).is_err()); + fn cli_bare_agent_subcommand_defaults_to_stdio() { + let args = try_parse_pager(&["grok-pager", "agent"]).unwrap(); + let Some(Command::Agent(agent)) = args.command else { + panic!("expected agent subcommand"); + }; + assert!(agent.mode.is_none(), "bare agent carries no explicit mode"); } #[test] fn leader_defaults_off_without_config() { diff --git a/crates/codegen/kigi-tui/src/mcp_cmd.rs b/crates/codegen/kigi-tui/src/mcp_cmd.rs index b6827f7..b241bed 100644 --- a/crates/codegen/kigi-tui/src/mcp_cmd.rs +++ b/crates/codegen/kigi-tui/src/mcp_cmd.rs @@ -80,6 +80,11 @@ pub enum McpCommand { #[arg(short = 's', long, value_enum)] scope: Option, }, + /// Authorize with an OAuth-enabled remote MCP server + Auth { + /// Name of the MCP server to authorize + name: String, + }, /// Diagnose MCP server configuration and connectivity Doctor { /// Emit machine-readable JSON output @@ -142,10 +147,30 @@ pub async fn run(mcp_args: McpArgs) -> Result<()> { McpCommand::List { json } => run_list(json), McpCommand::Add(args) => run_add(args).await, McpCommand::Remove { name, scope } => run_remove(&name, scope).await, + McpCommand::Auth { name } => run_auth(&name).await, McpCommand::Doctor { json, name } => run_doctor(json, name).await, } } +/// kimi-cli `mcp auth` parity: start the named remote server with the +/// interactive OAuth flow and report the tool count on success. +async fn run_auth(name: &str) -> Result<()> { + let cwd = current_dir_or_exit(); + println!("Authorizing with '{name}'..."); + println!("A browser window will open if authorization is required."); + match kigi_shell::mcp_doctor::run_auth(&cwd, name).await { + Ok(tool_count) => { + println!("Successfully authorized with '{name}'."); + println!("Available tools: {tool_count}"); + Ok(()) + } + Err(e) => { + eprintln!("Authorization failed: {e}"); + std::process::exit(1); + } + } +} + fn run_list(json: bool) -> Result<()> { // Include project-scoped servers (nearest definition wins), matching what // a session started in this directory would load from config.toml files.