Take placement handlers off the event loop and bound top_k at the MCP and CLI entry points#585
Merged
Conversation
…CP and CLI entry points
Two audit fixes stopped at the audited call site. The event-loop fix
offloaded one placement route but left the same blocking GPU probes
inline in the five sibling handlers; they now share one off-loop
helper that also covers the Intel util notice probe. The top_k lower
bound landed on the HTTP routes but not the MCP search tool or the
CLI search command; the tool now falls back to the configured default
with a warning (matching its scope fallback) and the CLI rejects
non-positive values at parse time.
Also: whitespace-only tool arguments now parse to {} like empty ones,
the no-op ingest early return gained the test it shipped without, and
two stale comments (a nonexistent module name, a pre-auth-middleware
claim that /api/search has no token gate) are corrected.
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.
Problem
A review of all 80 closed server/core audit findings (fixed in #570) checked each fix against its finding: does it address the root cause, is it the solution a plan would have produced, and do its tests exercise it. 73 held up with no changes needed. Two fixes had stopped at the audited call site: the event-loop fix offloaded one placement route while the five sibling handlers kept running the same blocking GPU subprocess probes inline, and the top_k lower bound covered the HTTP routes while the MCP search tool and CLI search still passed unclamped values to the store. Four smaller residues: whitespace-only tool arguments parsed inconsistently with empty ones, the no-op ingest early return had no test, and two comments went stale (a nonexistent module name; a claim that /api/search has no token gate, from before auth middleware covered every route).
Solution
All five placement handlers now run their action and its serialization through one off-loop helper, which also covers the Intel util notice probe; thread-ident tests pin each handler. The MCP search tool treats a non-positive top_k like its unknown-scope fallback, warning and using the configured default, and the CLI rejects non-positive values at parse time. Whitespace-only tool arguments parse to an empty dict, the ingest early return has a test asserting the whole-vault sync is skipped, and both stale comments are corrected.