diff --git a/changelog.d/757.fixed.md b/changelog.d/757.fixed.md new file mode 100644 index 00000000..14151cfe --- /dev/null +++ b/changelog.d/757.fixed.md @@ -0,0 +1,8 @@ +- **`omni init --hook` registered an MCP server it said it would not (#757)**: the flag has + read "Only install hooks" since it shipped and installed both halves anyway, and then + `omni doctor` called the resulting state a broken install and `omni doctor --fix` undid + it. Three ways of overruling a choice the CLI offered in the first place. `--hook` and + `--mcp` now install the half they name, naming the host or both halves still gets both, + and an absent MCP registration on Claude Code is reported as what it is rather than as a + fault: the hooks are what shortens output there, and the server is a convenience whose + tool definitions sit in the prefix of every request. diff --git a/docs/website/src-id/reference/agents.md b/docs/website/src-id/reference/agents.md index d6c5bee3..b61c1c63 100644 --- a/docs/website/src-id/reference/agents.md +++ b/docs/website/src-id/reference/agents.md @@ -61,11 +61,17 @@ JSON di awal setiap request, dan host membuang seluruh cache ketika sebuah serve tersambung atau terputus sementara perkakasnya sudah dimuat, yang bisa terjadi sendiri saat proses server keluar lalu tersambung lagi di tengah sesi tanpa Anda melakukan apa pun. `omni init --claude` mendaftarkannya. Jalur plugin tidak menambah definisi perkakas -sama sekali. Untuk tetap memakai hook tanpa pertukaran itu, hapus entri `omni` dari -`mcpServers` di `~/.claude.json`, dan ketahui bahwa penghapusannya tidak bertahan: -`omni doctor` melaporkan ketiadaannya sebagai peringatan, sedangkan `omni doctor --fix` -dan `omni init` berikutnya akan mendaftarkannya lagi. Belum ada flag khusus hook saja -([#757](https://github.com/fajarhide/omni/issues/757)). +sama sekali. Untuk tetap memakai hook tanpa pertukaran itu, pasang +separuhnya saja: + +```sh +omni init --hook # hook saja, tanpa pendaftaran MCP +omni init --mcp # server MCP saja, kalau Anda berubah pikiran +``` + +`omni doctor` kemudian melaporkan server MCP sebagai tidak terdaftar, bukan sebagai +kesalahan, dan baik ia maupun `--fix` tidak akan memasangnya kembali (#757). Menyebut +host-nya, `omni init --claude`, tetap memasang keduanya. **OpenClaw** Penuh di giliran berikutnya, bukan giliran saat ini. Hook `tool_result_persist`-nya menulis ulang hasil tool yang disimpan OpenClaw, jadi model diff --git a/docs/website/src/reference/agents.md b/docs/website/src/reference/agents.md index 9b9ff230..57b3cbb4 100644 --- a/docs/website/src/reference/agents.md +++ b/docs/website/src/reference/agents.md @@ -57,10 +57,16 @@ in the prefix of every request, and the host discards the whole cache when an MC connects or disconnects with its tools loaded, which a server process can do by exiting and reconnecting mid-session without you touching anything. `omni init --claude` registers it. The plugin route adds no tool definitions at all. To keep the hooks and drop -the trade, remove the `omni` entry from `mcpServers` in `~/.claude.json`, and know that -the removal does not stick: `omni doctor` reports the absence as a warning, and both -`omni doctor --fix` and the next `omni init` register it again. There is no hooks-only -flag yet ([#757](https://github.com/fajarhide/omni/issues/757)). +the trade, install that half on its own: + +```sh +omni init --hook # hooks, no MCP registration +omni init --mcp # the MCP server on its own, if you change your mind +``` + +`omni doctor` then reports the MCP server as not registered rather than as a fault, and +neither it nor `--fix` puts it back (#757). Naming the host, `omni init --claude`, still +installs both. **OpenClaw** is Full on a later turn, not the current one. Its `tool_result_persist` hook rewrites the tool result OpenClaw persists, so the model reads the distilled bytes diff --git a/src/agents/claude.rs b/src/agents/claude.rs index 0e378214..b5bc91c3 100644 --- a/src/agents/claude.rs +++ b/src/agents/claude.rs @@ -28,25 +28,13 @@ impl AgentIntegration for ClaudeIntegration { } fn install(&self, exe_path: &str) -> anyhow::Result<()> { - let (path, mut val) = initialize_settings()?; - let _ = backup_settings(&path); - - install_omni_hooks(&mut val, exe_path); - let new_content = serde_json::to_string_pretty(&val)?; - fs::write(&path, new_content)?; - crate::agent_report!( - " {} {} installed in Claude settings", - "✓".green(), - "Hooks".bold() - ); - + install_hooks(exe_path)?; install_mcp_server(exe_path)?; crate::agent_report!( " {} {} registered in .claude.json", "✓".green(), "MCP Server".bold() ); - Ok(()) } @@ -256,24 +244,24 @@ impl AgentIntegration for ClaudeIntegration { } } if !mcp_found { - if fix_mode { - if let Ok(exe_path) = std::env::current_exe() { - crate::agents::report_fix( - "MCP Server:", - "registered", - self.install(&exe_path.to_string_lossy()), - warnings, - ); - } - } else { - crate::agent_report!( - " {:<15} {}", - "MCP Server:".bright_black(), - "[WARNING] no MCP server found".yellow().bold() - ); - warnings.push("MCP Server is not configured. Run `omni init`.".to_string()); - all_ok = false; - } + // #757. Absence is a state here, not a fault. The hooks are what + // shortens output on this host; the MCP server is a convenience whose + // two tool definitions sit in the prefix of every request, and the + // host discards the whole prompt cache when an MCP server connects or + // disconnects with its tools loaded. Someone who ran + // `omni init --hook`, or who removed the entry on purpose after + // reading that, was being told their install was broken and then + // having the decision undone by the next `doctor --fix`. + // + // So it reports, and says how to add it, and neither fails the check + // nor repairs what nobody broke. + let _ = fix_mode; + crate::agent_report!( + " {:<15} {} {}", + "MCP Server:".bright_black(), + "[not registered]".bright_black(), + "optional here, `omni init --mcp` adds it".bright_black() + ); } all_ok @@ -558,6 +546,28 @@ pub fn install_omni_hooks(val: &mut Value, exe_path: &str) { ); } +/// The half of the install that does the work. +/// +/// Separate from the MCP registration because `omni init --hook` has promised +/// "Only install hooks" since it shipped and installed both anyway, which is the +/// shape of #151: a flag the parser accepts and the code ignores. On this host +/// the hooks are what shortens output and the MCP server is a convenience whose +/// tool definitions sit in the prefix of every request, so wanting one without +/// the other is an ordinary preference and not a broken install (#757). +pub fn install_hooks(exe_path: &str) -> anyhow::Result<()> { + let (path, mut val) = initialize_settings()?; + let _ = backup_settings(&path); + + install_omni_hooks(&mut val, exe_path); + fs::write(&path, serde_json::to_string_pretty(&val)?)?; + crate::agent_report!( + " {} {} installed in Claude settings", + "✓".green(), + "Hooks".bold() + ); + Ok(()) +} + pub fn install_mcp_server(exe_path: &str) -> anyhow::Result<()> { let path = get_claude_json_path(); let mut val = if path.exists() { diff --git a/src/cli/init.rs b/src/cli/init.rs index 6ad21e8b..00a725e8 100644 --- a/src/cli/init.rs +++ b/src/cli/init.rs @@ -127,6 +127,22 @@ fn non_interactive_host() -> anyhow::Result<&'static str> { }) } +/// Which halves of the Claude Code install a flag set asks for, as (hooks, mcp). +/// +/// #757. `--hook` has said "Only install hooks" since it shipped and installed +/// the MCP server as well, which is the shape of #151: a flag the parser accepts +/// and the code ignores. On this host the hooks are what shortens output and the +/// MCP server is a convenience whose tool definitions sit in the prefix of every +/// request, so one without the other is a preference somebody can hold. +/// +/// Asking for both, by naming the host or by naming both halves, gets both. +fn claude_halves(is_claude: bool, is_all: bool, is_hook: bool, is_mcp: bool) -> (bool, bool) { + if is_claude || is_all || (is_hook && is_mcp) || (!is_hook && !is_mcp) { + return (true, true); + } + (is_hook, is_mcp) +} + pub fn run_init(args: &[String]) -> anyhow::Result<()> { if super::wants_help(args) { print_help(); @@ -426,7 +442,21 @@ pub fn run_init(args: &[String]) -> anyhow::Result<()> { if target_ids.contains(&agent.id()) { println!("{}", format!("🤖 {} Setup", agent.name()).bold().cyan()); - match agent.install(&exe_path) { + // #757. `--hook` and `--mcp` have promised "Only install hooks" and + // "Only register MCP server" since they shipped, and both installed + // the pair. On Claude Code the hooks are what shortens output and the + // MCP server is a convenience that puts two tool definitions in the + // prefix of every request, so one without the other is a preference + // somebody can hold, and the flag that says so has to mean it. + let outcome = match ( + agent.id(), + claude_halves(is_claude, is_all, is_hook, is_mcp), + ) { + ("claude", (true, false)) => crate::agents::claude::install_hooks(&exe_path), + ("claude", (false, true)) => crate::agents::claude::install_mcp_server(&exe_path), + _ => agent.install(&exe_path), + }; + match outcome { Err(e) => eprintln!(" {} Failed: {}", "✗".red(), e), // #684. The success line was the same shape on every host, so // configuring an MCP-only one read as "OMNI is now shortening @@ -462,6 +492,33 @@ pub fn run_init(args: &[String]) -> anyhow::Result<()> { #[cfg(test)] mod tests { + + /// #757. Both flags have promised a half since they shipped and delivered + /// the pair, so `omni init --hook` registered an MCP server the user had + /// just said they did not want. The parser accepted the flag and the code + /// ignored it, which is #151 wearing a different name. + /// + /// The table is the point: naming the host, or naming both halves, or naming + /// neither, all still get both. Only a single half means a single half. + #[test] + fn a_half_flag_installs_that_half_and_nothing_else() { + // (is_claude, is_all, is_hook, is_mcp) -> (hooks, mcp) + for (args, want) in [ + ((false, false, true, false), (true, false)), + ((false, false, false, true), (false, true)), + ((false, false, true, true), (true, true)), + ((true, false, true, false), (true, true)), + ((false, true, true, false), (true, true)), + ((false, false, false, false), (true, true)), + ] { + let (is_claude, is_all, is_hook, is_mcp) = args; + assert_eq!( + claude_halves(is_claude, is_all, is_hook, is_mcp), + want, + "claude_halves{args:?} has to be {want:?}" + ); + } + } use super::*; /// Every host the no-tty path can pick has to be a real install target, or