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
2 changes: 1 addition & 1 deletion crates/gateway-registry/src/audit.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Minimal append-only audit trail for `gateway_execute` calls: timestamp,
//! Minimal append-only audit trail for `tool_execute` calls: timestamp,
//! server, tool, a hash of the args (never the raw args — they may contain
//! secrets), and outcome. Same category of guard forgemax's
//! `forge-audit` documents; written fresh here, not ported — see
Expand Down
2 changes: 1 addition & 1 deletion crates/gateway-registry/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub enum GatewayError {
/// A local, pre-flight validation failure (e.g. malformed `args`) that
/// happens entirely before any downstream I/O — distinct from
/// `Upstream`, which is reserved for the downstream server itself
/// failing. Callers (see `src/mcp_server.rs::gateway_execute`) map this
/// failing. Callers (see `src/mcp_server.rs::tool_execute`) map this
/// to `invalid_params` like `ServerNotFound`/`ToolNotFound`, since it's
/// a caller-fixable mistake, not an infrastructure failure.
#[error("{0}")]
Expand Down
2 changes: 1 addition & 1 deletion crates/gateway-registry/src/mcp_stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ impl McpStdioBackend {
// Local, pre-flight validation — happens before any
// downstream I/O, so this is the caller's mistake, not the
// downstream server's. `InvalidArgument`, not `Upstream`,
// so `gateway_execute` maps it to `invalid_params`.
// so `tool_execute` maps it to `invalid_params`.
return Err(GatewayError::InvalidArgument(format!(
"args must be a JSON object, got {other}"
)));
Expand Down
6 changes: 3 additions & 3 deletions crates/gateway-registry/src/sanitize.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Bounds and cleans up whatever a downstream `tools/list` response hands
//! back before it's indexed and surfaced to the LLM via `gateway_search`.
//! back before it's indexed and surfaced to the LLM via `tool_search`.
//! A downstream server (especially a third-party or compromised one) fully
//! controls its own tool names/descriptions — this is the one place that
//! data crosses into our own storage and eventually into LLM context, so
Expand All @@ -15,7 +15,7 @@ const MAX_DESCRIPTION_LEN: usize = 1024;

/// Restrict a tool name to `[A-Za-z0-9._-]`, truncated to [`MAX_NAME_LEN`].
/// Falls back to `"unnamed"` if nothing safe survives — an empty server-
/// or tool-name would otherwise break `gateway_execute`'s addressing.
/// or tool-name would otherwise break `tool_execute`'s addressing.
fn sanitize_name(name: &str) -> String {
let cleaned: String = name
.chars()
Expand All @@ -32,7 +32,7 @@ fn sanitize_name(name: &str) -> String {
/// Strip control characters (kept: newline/tab, since descriptions are
/// legitimately multi-line) and truncate to [`MAX_DESCRIPTION_LEN`] chars —
/// bounds how much of a single tool's description a hostile downstream
/// server can push into `gateway_search` results / LLM context.
/// server can push into `tool_search` results / LLM context.
fn sanitize_description(description: &str) -> String {
description
.chars()
Expand Down
2 changes: 1 addition & 1 deletion crates/gateway-registry/src/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub struct ToolHit {

/// Ceiling applied to a caller-supplied `limit` before it's used in
/// SQLite's `LIMIT` clause. Two reasons: (1) casting a huge `usize` (e.g.
/// `usize::MAX`, reachable via `gateway_search`'s MCP request) straight to
/// `usize::MAX`, reachable via `tool_search`'s MCP request) straight to
/// `i64` can wrap around to a negative number in two's-complement, and
/// SQLite treats a negative `LIMIT` as "no limit" — silently defeating the
/// cap; (2) even ignoring the cast, tool search results are meant to be a
Expand Down
4 changes: 2 additions & 2 deletions crates/gateway-registry/src/truncate.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Caps oversized `gateway_execute` results so one chatty downstream tool
//! Caps oversized `tool_execute` results so one chatty downstream tool
//! can't blow out the LLM's context. Same motivation as forgemax's
//! `MAX_RESULT_CHARS` envelope, written fresh (no code shared — FSL).

Expand Down Expand Up @@ -51,7 +51,7 @@ fn build_envelope(json: &str, cut: usize) -> Value {
}

/// The ACTUAL serialized size of a candidate envelope — matches
/// `gateway_execute` (`src/mcp_server.rs`), which serializes the capped
/// `tool_execute` (`src/mcp_server.rs`), which serializes the capped
/// value with `serde_json::to_string_pretty` before returning it.
fn envelope_len(envelope: &Value) -> usize {
serde_json::to_string_pretty(envelope)
Expand Down
2 changes: 1 addition & 1 deletion crates/gateway-registry/tests/mcp_stdio_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ async fn call_with_non_object_args_is_invalid_argument_not_upstream() {
// Malformed `args` (not a JSON object or null) is rejected entirely
// locally, before any downstream I/O — a caller mistake, not a
// downstream/infrastructure failure, so it must be `InvalidArgument`
// (which `gateway_execute` maps to `invalid_params`), not `Upstream`
// (which `tool_execute` maps to `invalid_params`), not `Upstream`
// (which maps to `internal_error`).
let backend = McpStdioBackend::new(fixture_path(), vec![], HashMap::new());
let err = backend
Expand Down
98 changes: 82 additions & 16 deletions src/components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,27 @@ fn claude_json() -> Value {
.unwrap_or(Value::Null)
}

/// Removes a server entry from `~/.claude.json`'s `mcpServers` map, if
/// present — used to undo a native MCP registration another tool's own
/// installer created (e.g. lean-ctx's `onboard`) once that server has been
/// re-registered behind the agentflare gateway instead. Returns true only if
/// an entry was actually found and removed.
fn remove_claude_mcp_server(name: &str) -> bool {
let path = home().join(".claude.json");
let mut root = claude_json();
let Some(servers) = root.get_mut("mcpServers").and_then(|v| v.as_object_mut()) else {
return false;
};
if servers.remove(name).is_none() {
return false;
}
fs::write(
&path,
serde_json::to_string_pretty(&root).unwrap_or_default() + "\n",
)
.is_ok()
}

fn json_at(path: &PathBuf) -> Value {
fs::read_to_string(path)
.ok()
Expand Down Expand Up @@ -333,26 +354,40 @@ pub fn get_components(host: &str) -> Vec<Component> {
id: "leanctx",
needs_consent: true,
// lean-ctx's own installer (and `onboard`) wires MCP into whichever
// supported tool it detects, so no per-host branching needed here —
// trust the upstream tool's own setup. Installed via its native
// prebuilt-binary installer (see tool_install), not mise:
// lean-ctx ships a proper `curl | sh` that downloads, verifies, and
// onboards on its own.
describe: "lean-ctx (context compression) — native installer (curl | sh, or brew) + onboard".to_string(),
check: Box::new(|| crate::tool_install::installed(&crate::tool_install::LEAN_CTX)),
// supported tool it detects natively — exactly the always-on
// tool-list bloat the agentflare gateway exists to avoid. Right
// after installing, register it behind the gateway instead
// (`gateway_integrations::LEANCTX`) and, for claude-code, strip
// whatever native entry the upstream onboarder already created so
// the same ~80 ctx_* tools aren't declared twice.
describe: "lean-ctx (context compression) — native installer (curl | sh, or brew), registered behind the agentflare gateway (tool_search/tool_execute), not the host's native tool list".to_string(),
check: Box::new(|| {
crate::tool_install::installed(&crate::tool_install::LEAN_CTX)
&& crate::gateway_integrations::already_registered("leanctx")
}),
apply: {
let log = leanctx_log.clone();
let host = host_owned.clone();
Box::new(move || {
if log.exists() {
return format!("lean-ctx install already triggered — check {}", log.display());
}
let _ = fs::create_dir_all(log.parent().unwrap());
let outcome = crate::tool_install::install(&crate::tool_install::LEAN_CTX);
let _ = fs::write(&log, format!("{:?}", std::time::SystemTime::now()));
match outcome {
Ok(m) => format!("{m} + onboarded"),
Err(e) => e,
let mut msg = if log.exists() {
format!("lean-ctx install already triggered — check {}", log.display())
} else {
let _ = fs::create_dir_all(log.parent().unwrap());
let outcome = crate::tool_install::install(&crate::tool_install::LEAN_CTX);
let _ = fs::write(&log, format!("{:?}", std::time::SystemTime::now()));
match outcome {
Ok(m) => m,
Err(e) => return e,
}
};
msg = format!(
"{msg} + {}",
crate::gateway_integrations::register(&crate::gateway_integrations::LEANCTX)
);
if host == "claude-code" && remove_claude_mcp_server("lean-ctx") {
msg = format!("{msg} + removed native claude-code MCP entry (now gateway-only)");
}
msg
Comment on lines +357 to +390

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Gateway registration proceeds even after a previously-failed lean-ctx install, masking the failure as success.

The leanctx-install.log is written unconditionally right after calling install(), regardless of outcome. On the run where install fails, return e correctly skips registration — but on every subsequent run, log.exists() is true, so apply() skips reinstalling and falls straight through to gateway_integrations::register(&LEANCTX) (which doesn't verify the binary exists) and the claude-code cleanup. The result: gateway.toml gets a [servers.leanctx] entry pointing at a binary that was never actually installed, apply() reports it as "ok ... registered", and check() keeps failing forever with no way to retry the install short of manually deleting the log file. The gateway will fail at spawn time the first time a ctx_* tool is actually invoked.

🛡️ Proposed fix — don't register a dangling entry when the binary is still missing
                     match outcome {
                         Ok(m) => m,
                         Err(e) => return e,
                     }
                 };
+                if !crate::tool_install::installed(&crate::tool_install::LEAN_CTX) {
+                    return msg; // install genuinely failed previously — don't register a dangling gateway entry
+                }
                 msg = format!(
                     "{msg} + {}",
                     crate::gateway_integrations::register(&crate::gateway_integrations::LEANCTX)
                 );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// supported tool it detects natively — exactly the always-on
// tool-list bloat the agentflare gateway exists to avoid. Right
// after installing, register it behind the gateway instead
// (`gateway_integrations::LEANCTX`) and, for claude-code, strip
// whatever native entry the upstream onboarder already created so
// the same ~80 ctx_* tools aren't declared twice.
describe: "lean-ctx (context compression) — native installer (curl | sh, or brew), registered behind the agentflare gateway (tool_search/tool_execute), not the host's native tool list".to_string(),
check: Box::new(|| {
crate::tool_install::installed(&crate::tool_install::LEAN_CTX)
&& crate::gateway_integrations::already_registered("leanctx")
}),
apply: {
let log = leanctx_log.clone();
let host = host_owned.clone();
Box::new(move || {
if log.exists() {
return format!("lean-ctx install already triggered — check {}", log.display());
}
let _ = fs::create_dir_all(log.parent().unwrap());
let outcome = crate::tool_install::install(&crate::tool_install::LEAN_CTX);
let _ = fs::write(&log, format!("{:?}", std::time::SystemTime::now()));
match outcome {
Ok(m) => format!("{m} + onboarded"),
Err(e) => e,
let mut msg = if log.exists() {
format!("lean-ctx install already triggered — check {}", log.display())
} else {
let _ = fs::create_dir_all(log.parent().unwrap());
let outcome = crate::tool_install::install(&crate::tool_install::LEAN_CTX);
let _ = fs::write(&log, format!("{:?}", std::time::SystemTime::now()));
match outcome {
Ok(m) => m,
Err(e) => return e,
}
};
msg = format!(
"{msg} + {}",
crate::gateway_integrations::register(&crate::gateway_integrations::LEANCTX)
);
if host == "claude-code" && remove_claude_mcp_server("lean-ctx") {
msg = format!("{msg} + removed native claude-code MCP entry (now gateway-only)");
}
msg
// supported tool it detects natively — exactly the always-on
// tool-list bloat the agentflare gateway exists to avoid. Right
// after installing, register it behind the gateway instead
// (`gateway_integrations::LEANCTX`) and, for claude-code, strip
// whatever native entry the upstream onboarder already created so
// the same ~80 ctx_* tools aren't declared twice.
describe: "lean-ctx (context compression) — native installer (curl | sh, or brew), registered behind the agentflare gateway (tool_search/tool_execute), not the host's native tool list".to_string(),
check: Box::new(|| {
crate::tool_install::installed(&crate::tool_install::LEAN_CTX)
&& crate::gateway_integrations::already_registered("leanctx")
}),
apply: {
let log = leanctx_log.clone();
let host = host_owned.clone();
Box::new(move || {
let mut msg = if log.exists() {
format!("lean-ctx install already triggered — check {}", log.display())
} else {
let _ = fs::create_dir_all(log.parent().unwrap());
let outcome = crate::tool_install::install(&crate::tool_install::LEAN_CTX);
let _ = fs::write(&log, format!("{:?}", std::time::SystemTime::now()));
match outcome {
Ok(m) => m,
Err(e) => return e,
}
};
if !crate::tool_install::installed(&crate::tool_install::LEAN_CTX) {
return msg; // install genuinely failed previously — don't register a dangling gateway entry
}
msg = format!(
"{msg} + {}",
crate::gateway_integrations::register(&crate::gateway_integrations::LEANCTX)
);
if host == "claude-code" && remove_claude_mcp_server("lean-ctx") {
msg = format!("{msg} + removed native claude-code MCP entry (now gateway-only)");
}
msg
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components.rs` around lines 357 - 390, Update the lean-ctx installation
flow in the apply closure to distinguish a successful install from a prior
failed attempt: write the completion marker only after install succeeds, and
allow subsequent runs to retry when the installed binary is still missing.
Register LEANCTX and remove the native Claude entry only after confirming the
tool is installed, so failed installations cannot produce a dangling gateway
registration or success message.

})
},
},
Expand Down Expand Up @@ -956,4 +991,35 @@ mod tests {
assert_eq!(apply_skill_overrides(&names, &mut settings).unwrap(), 1);
assert_eq!(apply_skill_overrides(&names, &mut settings).unwrap(), 0);
}

#[test]
fn remove_claude_mcp_server_removes_only_the_named_entry() {
crate::paths::test_support::with_temp_home(|| {
let path = home().join(".claude.json");
fs::write(
&path,
serde_json::json!({
"mcpServers": {
"lean-ctx": {"command": "lean-ctx"},
"flare": {"command": "agentflare"}
}
})
.to_string(),
)
.unwrap();

assert!(remove_claude_mcp_server("lean-ctx"));

let value = json_at(&path);
assert!(value["mcpServers"]["lean-ctx"].is_null());
assert!(value["mcpServers"]["flare"].is_object());
});
}

#[test]
fn remove_claude_mcp_server_is_a_noop_when_absent() {
crate::paths::test_support::with_temp_home(|| {
assert!(!remove_claude_mcp_server("lean-ctx"));
});
}
}
68 changes: 65 additions & 3 deletions src/gateway_integrations.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// During `init`, detect project context (e.g. a GitHub remote) and, with the
// user's OK, register the matching MCP server BEHIND agentflare's own gateway
// (`~/.agentflare/gateway.toml`) — so its tools stay reachable through
// `gateway_search`/`gateway_execute` instead of bloating the host's always-on
// `tool_search`/`tool_execute` instead of bloating the host's always-on
// tool list. Adding another gateway-fronted MCP later is one more entry in
// `INTEGRATIONS`; the plumbing (detect → consent → idempotent append) is shared.
use crate::paths::home;
Expand All @@ -24,12 +24,12 @@ pub struct GatewayIntegration {
pub post_note: fn() -> Vec<String>,
}

pub const INTEGRATIONS: &[GatewayIntegration] = &[GITHUB];
pub const INTEGRATIONS: &[GatewayIntegration] = &[GITHUB, LEANCTX];

const GITHUB: GatewayIntegration = GatewayIntegration {
name: "github",
detect: git_remote_is_github,
prompt: "⚑ GitHub repo detected. github-mcp-server can sit behind the agentflare gateway\n (its tools stay under gateway_search/gateway_execute, not the host's tool list).",
prompt: "⚑ GitHub repo detected. github-mcp-server can sit behind the agentflare gateway\n (its tools stay under tool_search/tool_execute, not the host's tool list).",
// Remote HTTP backend — zero-install (no docker/binary). The gateway
// sends `auth_header` verbatim, so the stored secret is the full header
// value (`Bearer <token>`), see `post_note`.
Expand All @@ -45,6 +45,34 @@ fn github_post_note() -> Vec<String> {
]
}

/// lean-ctx's own installer/`onboard` wires its ~80 `ctx_*` tools straight
/// into the host's native MCP config — exactly the always-on tool-list bloat
/// this gateway exists to avoid. Once the binary is on PATH, this puts it
/// behind the gateway instead; `components.rs`'s `"leanctx"` component also
/// strips whatever native registration the upstream onboarder already
/// created, so the same tools don't end up declared twice.
pub const LEANCTX: GatewayIntegration = GatewayIntegration {
name: "leanctx",
detect: leanctx_installed,
prompt: "⚑ lean-ctx detected. Its ~80 ctx_* tools can sit behind the agentflare gateway\n (reachable via tool_search/tool_execute) instead of bloating the host's tool list.",
// Local stdio backend — same binary lean-ctx's own installer already put
// on PATH; the gateway just spawns it instead of the host declaring it
// natively. No auth needed (local process).
toml_block: "[servers.leanctx]\nkind = \"mcp_stdio\"\ncommand = \"lean-ctx\"\nargs = []",
post_note: leanctx_post_note,
};

fn leanctx_installed() -> bool {
crate::tool_install::installed(&crate::tool_install::LEAN_CTX)
}

fn leanctx_post_note() -> Vec<String> {
vec![
" next its ctx_* tools are now reached via tool_search/tool_execute, not called natively"
.to_string(),
]
}

pub fn gateway_toml_path() -> PathBuf {
home().join(".agentflare").join("gateway.toml")
}
Expand Down Expand Up @@ -195,4 +223,38 @@ mod tests {
assert!(cfg.servers.contains_key("github"));
});
}

#[test]
fn register_writes_a_valid_parseable_leanctx_server() {
with_temp_home(|| {
assert!(!already_registered("leanctx"));
let msg = register(&LEANCTX);
assert!(msg.starts_with("ok"), "unexpected: {msg}");

let content = fs::read_to_string(gateway_toml_path()).unwrap();
assert!(content.contains("[servers.leanctx]"));
let cfg = gateway_registry::parse_config(&content).unwrap();
assert!(cfg.servers.contains_key("leanctx"));
assert!(already_registered("leanctx"));
});
}

#[test]
fn register_leanctx_is_idempotent_and_never_duplicates() {
with_temp_home(|| {
let msg1 = register(&LEANCTX);
assert!(msg1.starts_with("ok"), "first: {msg1}");
let msg2 = register(&LEANCTX);
assert!(msg2.starts_with("skip"), "second: {msg2}");
let content = fs::read_to_string(gateway_toml_path()).unwrap();
assert_eq!(content.matches("[servers.leanctx]").count(), 1);
});
}

#[test]
fn integrations_list_includes_github_and_leanctx() {
let names: Vec<&str> = INTEGRATIONS.iter().map(|i| i.name).collect();
assert!(names.contains(&"github"));
assert!(names.contains(&"leanctx"));
}
}
Loading
Loading