Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions changelog.d/757.fixed.md
Original file line number Diff line number Diff line change
@@ -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.
16 changes: 11 additions & 5 deletions docs/website/src-id/reference/agents.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions docs/website/src/reference/agents.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
72 changes: 41 additions & 31 deletions src/agents/claude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand Down
59 changes: 58 additions & 1 deletion src/cli/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down