Conversation
Create shared crate with fts_query, MatchMode, Bm25Weights, clamped_limit. Refactor gateway-registry and skill-registry to import from it instead of duplicating the sanitization logic.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesAdds the Shared search primitives
Estimated code review effort: 3 (Moderate) | ~20 minutes 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 |
… impl Satisfies clippy::derivable_impls under -D warnings, which CI's clippy job enforces.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/flare-search-kit/src/lib.rs (1)
76-87: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider bounding token count/length for defense-in-depth.
fts_querysanitizes arbitrary-length input with no cap on token count or overall size. Since callers (gateway/skill registries) pass user-supplied query strings straight through, a pathological huge input could still build an unbounded MATCH expression even thoughclamped_limitprotects the result-set size on the other end.♻️ Example hardening
pub fn fts_query(query: &str, mode: MatchMode) -> Option<String> { let tokens: Vec<String> = query .split_whitespace() .map(|t| t.replace('"', "")) .filter(|t| !t.is_empty()) .map(|t| format!("\"{t}\"")) + .take(MAX_TOKENS) .collect(); if tokens.is_empty() { return None; } Some(tokens.join(mode.joiner())) }🤖 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 `@crates/flare-search-kit/src/lib.rs` around lines 76 - 87, Update fts_query to enforce bounds on both the number of tokens and the total generated query length before constructing the final MATCH expression. Use defined limits, stop or reject input that exceeds them, and preserve the existing None result for empty input and join behavior for valid queries.crates/skill-registry/src/search.rs (1)
60-167: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo huge-limit test for skill-registry, unlike gateway-registry.
Once
clamped_limitis wired in (see the comment on lines 4-5/37), add a test mirroring gateway-registry'ssearch_with_a_huge_limit_does_not_panic_and_still_returns_resultsto lock in the fix.🤖 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 `@crates/skill-registry/src/search.rs` around lines 60 - 167, The search tests lack coverage for oversized limits. After `clamped_limit` is wired into `search`, add a test near the existing search tests that calls `search` with a very large limit and verifies it does not panic and still returns expected results, mirroring gateway-registry’s `search_with_a_huge_limit_does_not_panic_and_still_returns_results` behavior.
🤖 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 `@crates/skill-registry/src/search.rs`:
- Around line 4-5: Update the search query setup in search.rs to pass
clamped_limit(limit) rather than casting the raw limit to i64 before binding it.
Reuse the existing clamped_limit helper and preserve the current query behavior
while ensuring oversized inputs remain capped.
---
Nitpick comments:
In `@crates/flare-search-kit/src/lib.rs`:
- Around line 76-87: Update fts_query to enforce bounds on both the number of
tokens and the total generated query length before constructing the final MATCH
expression. Use defined limits, stop or reject input that exceeds them, and
preserve the existing None result for empty input and join behavior for valid
queries.
In `@crates/skill-registry/src/search.rs`:
- Around line 60-167: The search tests lack coverage for oversized limits. After
`clamped_limit` is wired into `search`, add a test near the existing search
tests that calls `search` with a very large limit and verifies it does not panic
and still returns expected results, mirroring gateway-registry’s
`search_with_a_huge_limit_does_not_panic_and_still_returns_results` behavior.
🪄 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: 0b5a957a-5bc6-444f-b6f7-0731fbddb866
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
Cargo.tomlcrates/flare-search-kit/Cargo.tomlcrates/flare-search-kit/src/lib.rscrates/gateway-registry/Cargo.tomlcrates/gateway-registry/src/search.rscrates/skill-registry/Cargo.tomlcrates/skill-registry/src/search.rs
…egression test CodeRabbit caught it on #189: search() still cast the raw usize limit to i64 directly instead of using flare-search-kit's clamped_limit, leaving the negative-LIMIT-on-usize::MAX bug (already fixed in gateway-registry) unguarded here.
getappz
added a commit
that referenced
this pull request
Jul 15, 2026
- Don't hold any lock (skills_registry mutex or gateway Registry's conn mutex) across the fallback HTTP call to registry.modelcontextprotocol.io: local search now runs and releases its lock before the network fallback fires, in both the skill and tool MCP actions. - Add a 3s timeout to the registry HTTP request so a slow/unreachable registry can't hang a search indefinitely. - Wire the fallback into tool_search (the "tool"/"search" MCP action) too, matching the item's original scope -- gateway_registry::Registry's fallback method existed but was dead code, never called anywhere. - Fix score semantics: local bm25() scores are ascending (lower = better, no DESC), but registry hits got a flat 0.5 with a comment claiming that ranked them below local results -- backwards, since 0.5 > every negative local score. Registry hits now score f64::MAX, so they always sort last under the same ascending convention if the merged list is ever re-sorted. - Resolve merge conflicts from master's flare-search-kit consolidation (#189) by rebuilding gateway-registry/search.rs and skill-registry's merge logic against the new shared MatchMode/fts_query/clamped_limit.
This was referenced Jul 22, 2026
2 tasks
5 tasks
getappz
added a commit
that referenced
this pull request
Aug 25, 2026
…tch (#601) execute_work chdir'd into an item's worktree via std::env::set_current_dir, which mutates the whole process's cwd, not per-thread. The daemon dispatches multiple in-process work-item jobs concurrently by design (work_max_concurrency), so two jobs racing here could have one item's pipeline run against a different item's checked-out worktree -- observed live, twice, with two different item pairs (items #189/#70 and #189/#188). Add run_in_worktree (src/cli/work_cwd_lock.rs), which serializes the chdir -> run -> restore critical section behind a mutex, same mitigation shape as flare_git_core::worktree::WORKTREE_ADD_LOCK. Only this section is serialized -- claiming, agent resolution, and DB reads above it still run concurrently. Split into a satellite file (and the new regression test into another) to keep work.rs under the repo's LOC gate. Regression test dispatches two items concurrently on separate threads and asserts neither thread's cwd drifts into the other's worktree mid-run; confirmed it reliably reproduces the race (3/3 runs) with the lock removed and passes reliably (3/3) with it restored, both before and after factoring the lock+chdir logic into run_in_worktree. Agentflare-Agent: claude-code Agentflare-Branch: fix/execute-work-cwd-race Co-authored-by: shiva <shiva@gosysinfo.tech>
This was referenced Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auto-opened on
item donefor 019f62aa-6020-72f3-b346-5ec86af244e7.Summary by CodeRabbit