feat(gateway): put lean-ctx behind the gateway, rename gateway_search/execute to tool_search/tool_execute - #184
Conversation
…/execute to tool_search/tool_execute
📝 WalkthroughWalkthroughThe PR renames downstream MCP discovery and execution APIs to ChangesDownstream tool API surface
Leanctx gateway integration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ComponentApply
participant Leanctx
participant AgentflareGateway
participant ClaudeConfig
ComponentApply->>Leanctx: install lean-ctx
ComponentApply->>AgentflareGateway: register leanctx integration
ComponentApply->>ClaudeConfig: remove native lean-ctx entry for claude-code
AgentflareGateway-->>ComponentApply: return registration and cleanup status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components.rs (1)
364-367: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
"leanctx"duplicates the idempotency key defined ingateway_integrations::LEANCTX.name.Reference
crate::gateway_integrations::LEANCTX.nameinstead of the literal string, so a future rename of the integration's key can't silently desync thischeck.♻️ Proposed fix
check: Box::new(|| { crate::tool_install::installed(&crate::tool_install::LEAN_CTX) - && crate::gateway_integrations::already_registered("leanctx") + && crate::gateway_integrations::already_registered(crate::gateway_integrations::LEANCTX.name) }),🤖 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 364 - 367, Update the check closure in the component definition to pass crate::gateway_integrations::LEANCTX.name to already_registered instead of the hardcoded "leanctx" literal, while preserving the existing installation check.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/components.rs`:
- Around line 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.
---
Nitpick comments:
In `@src/components.rs`:
- Around line 364-367: Update the check closure in the component definition to
pass crate::gateway_integrations::LEANCTX.name to already_registered instead of
the hardcoded "leanctx" literal, while preserving the existing installation
check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a094837-df06-4631-b70f-9421963077d7
📒 Files selected for processing (11)
crates/gateway-registry/src/audit.rscrates/gateway-registry/src/error.rscrates/gateway-registry/src/mcp_stdio.rscrates/gateway-registry/src/sanitize.rscrates/gateway-registry/src/search.rscrates/gateway-registry/src/truncate.rscrates/gateway-registry/tests/mcp_stdio_call.rssrc/components.rssrc/gateway_integrations.rssrc/mcp_server.rssrc/rule_text.rs
| // 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 |
There was a problem hiding this comment.
🎯 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.
| // 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.
# Conflicts: # src/mcp_server.rs
…refix, fix flaky nanoid test Review of task/184 (item #186) found the PR didn't actually cover its own motivating case: item_get -- the exact call from #184's bug report (item(get, id="178")) -- was never wired to resolve_item_id, and resolve_id's numeric parse didn't strip a leading '#', so #-prefixed sequence ids silently fell through to the UUID passthrough branch instead of resolving, despite both the commit message and the tool's own schema description claiming that support. - item_get now resolves through resolve_item_id, closing the original gap - resolve_id strips a leading '#' before parsing as numeric - Added the tests #184 explicitly required and that were missing: bare numeric, #-prefixed numeric, not-found numeric, project-scoped lookup, end-to-end via the item MCP tool - Fixed a flaky test: handoff_tool_requires_recipient_and_assigns_item asserted a filename via item_id.to_lowercase(), which doesn't match production's actual AgentflareMcp::slugify() transform -- slugify also collapses '_' to '-', which to_lowercase() doesn't, so the assertion failed whenever a randomly-generated nanoid id happened to contain an underscore. Now asserts against the real transform. cargo test --workspace (632 passing, 0 failed), cargo fmt --check, and cargo clippy --workspace --all-features -D warnings all clean.
…tion to nanoid (#257) * item/claim: accept numeric sequence_id or #-prefixed id; switch id generation to nanoid item/claim MCP tools (get, update, update_state, delete, claim, heartbeat, release, done, add_label, remove_label; claim's target param) now accept either a UUID or a numeric sequence_id (bare or #-prefixed), resolved via agentflare_backend::item::resolve_id scoped to the repo's linked project. Not-found numeric ids return the same not-found shape as an unmatched UUID. Closes #184. Also switches db_kit::ids::new_id() from uuid::Uuid::now_v7() to nanoid::nanoid!(), updating every caller across agentflare-artifacts and agentflare-backend (asset/comment/label/project/state/webhook/workspace). cargo build --workspace --all-features, cargo test --workspace (630+ passing across the bin plus every crate), cargo fmt --check, and cargo clippy --workspace --all-features -D warnings all clean (the one remaining clippy hit is the pre-existing Windows-only agent_launch.rs test import, tracked separately as item #169, unrelated to this change). * item/claim: wire item_get through sequence_id resolution, support #-prefix, fix flaky nanoid test Review of task/184 (item #186) found the PR didn't actually cover its own motivating case: item_get -- the exact call from #184's bug report (item(get, id="178")) -- was never wired to resolve_item_id, and resolve_id's numeric parse didn't strip a leading '#', so #-prefixed sequence ids silently fell through to the UUID passthrough branch instead of resolving, despite both the commit message and the tool's own schema description claiming that support. - item_get now resolves through resolve_item_id, closing the original gap - resolve_id strips a leading '#' before parsing as numeric - Added the tests #184 explicitly required and that were missing: bare numeric, #-prefixed numeric, not-found numeric, project-scoped lookup, end-to-end via the item MCP tool - Fixed a flaky test: handoff_tool_requires_recipient_and_assigns_item asserted a filename via item_id.to_lowercase(), which doesn't match production's actual AgentflareMcp::slugify() transform -- slugify also collapses '_' to '-', which to_lowercase() doesn't, so the assertion failed whenever a randomly-generated nanoid id happened to contain an underscore. Now asserts against the real transform. cargo test --workspace (632 passing, 0 failed), cargo fmt --check, and cargo clippy --workspace --all-features -D warnings all clean.
Discovery tick dispatches purely on the ready-for-work label, so items #184/#185/#186/#187 (go/no-go candidates from #166's spec) whose own description says "Decision pending — not dispatched" got auto-dispatched and re-dispatched across multiple agents anyway -- the prose was never actually enforced. Add a needs-decision label that blocks run_discovery_tick even while ready-for-work is also present. Stripping ready-for-work alone wouldn't have been durable: redispatch unconditionally re-attaches it, so the new label has to keep gating on its own until a human clears it. Agentflare-Agent: claude-code Agentflare-Branch: fix/dispatch-failure-ceiling-any-reason
Follow-up to a manual spike this session that proved viable: routing lean-ctx's ~80
ctx_*MCP tools through the agentflare gateway instead of declaring them natively in the host's tool list.Changes
src/gateway_integrations.rs: newLEANCTXGatewayIntegration (mcp_stdio backend,command = "lean-ctx") alongside the existingGITHUBone. Registers lean-ctx behind~/.agentflare/gateway.tomlon consent.src/components.rs: the"leanctx"component now (a) installs the binary as before, (b) registers it behind the gateway viagateway_integrations::register, and (c) for claude-code, removes any nativemcpServers.lean-ctxentry lean-ctx's own onboarder created, via a newremove_claude_mcp_serverhelper — so the same tools don't end up declared twice.check()now requires both the binary being installed and gateway registration.gateway_search/gateway_executeMCP tools totool_search/tool_execute(mcp_server.rs) — clearer, more obvious naming per user request. Updated all references/tests/doc-comments repo-wide, including stale mentions incrates/gateway-registry's doc comments.src/rule_text.rs: added a@fallbackline to the lean-ctx usage rule pointing attool_search/tool_executefor whenctx_*tools aren't natively present — old wording preserved inLEANCTX_SUPERSEDEDso existing installs get an automatic refresh offer via the existing stale-rule mechanism.Verified
gateway_search/gateway_execute(pre-rename) discovered and executed lean-ctx's tools correctly end-to-end.cargo test: 437/437 passed on a clean, uncontended run.cargo clippy --all-targets -- -D warnings -A unsafe_code -A clippy::pedantic: clean.cargo fmt --check: clean.Known local flakiness (not from this diff)
worktree::tests::run_output_timeout_kills_the_child_not_just_abandons_itfailed intermittently on later re-runs on this dev machine (2 of 3 attempts), but:worktree.rsoragent_launch.rs.taskkill: process not found) is consistent with the test's polling thread getting starved under heavy local concurrent-build load (multiple simultaneouscargo test/clippy/checkruns across worktrees on this machine during this session), not a functional regression — a real kill_tree bug would fail deterministically, not 1-in-3.Follow-up needed (separate, not in this PR)
Item #49 (opencode, uncommitted work on
task/46) consolidatesgateway_search/gateway_executeinto a singlegateway(action=...)dispatch tool — a different direction than this PR's simple rename. Left a review comment on #49 flagging the conflict; once this merges, that work should retargettool_search/tool_execute(ideally as a singletool(action=...)dispatch, matching the rest of the action-dispatch family) instead of the old names.Summary by CodeRabbit
New Features
tool_searchandtool_execute.Documentation