plugin: enforce index-first search with plugin hooks - #14
Conversation
Installing the plugin now makes the index the way agents search code, instead of only recommending it: a PreToolUse hook denies the Grep tool, and a Node hook denies standalone grep/rg/git-grep Bash commands, each with a reason redirecting the agent to the search/sql tools. The deny is self-correcting - Claude Code feeds the reason back to the agent, so a reflexive grep becomes a search call on the next step. grep as a pipe filter on other command output stays allowed; filtering logs or test output is a job the index does not do. A SessionStart hook announces the policy up front so agents reach for the index first rather than learning it from a denial. The Bash hook uses Node (already the package's requirement) rather than jq, which is not guaranteed on user machines.
Grep (tool or standalone grep/rg/git-grep in Bash) is now denied only once the repo's index fully covers the code: manifest present, vectors ready, nothing truncated by the file cap. Until then grep passes through - agents are never forced onto an index that does not exist yet or can only rank by keyword. The hook reads the codecontext.json manifest directly (CX_INDEX_DIR override honored, walking up from cwd otherwise). The redirects now point at sql with the search table functions - the search tool (still registered by older server builds) is denied with a hybrid_search/bm25_search example, and sql statements calling vector_search are denied in favor of hybrid_search. One script serves SessionStart and both PreToolUse matchers, and the session-start note adapts to index state.
Drop the search tool: sql is how agents search. Ranked retrieval is a table-valued function inside the query - hybrid_search for the fused keyword + semantic pass (bm25_search while vectors are backfilling) - so finding, understanding, counting, and ranking code are all one read-only SELECT. The instructions and the sql description now lead with the retrieval query shape (SELECT path, start_line, end_line, symbol, content FROM hybrid_search(...)) instead of presenting sql as an analytics side-tool, and vector_search is rejected with a redirect to hybrid_search - raw vector ranking loses the keyword arm for no benefit and agents kept reaching for it. Build the index at server startup instead of on the first query: the server starts with the session, so the staged build begins immediately and keyword search is typically live (vectors backfilling) before the agent asks anything. Queries still ensure the index inline as before; CX_AUTO_INDEX=0 disables both. Agent-observed failure modes this closes: choosing vector_search over hybrid_search, LIKE table scans instead of ranked TVFs, and first-query latency absorbed by an inline index build.
The search code path is gone, not deprecated: core search() and its SearchHit/SearchResult shapes, the cx search CLI command, CX_SEARCH_K, the search usage-receipt variant (searchEntry, wholeFileTokens, the per-hit ledger rendering), and every doc that described a three-tool surface. One retrieval door remains - read-only SQL whose table-valued search functions do the ranking: hybrid_search for the fused keyword + semantic pass, bm25_search for the keyword-only window while vectors backfill, GROUP BY over either for counts and rankings. Tests are ported, not deleted: every retrieval assertion now goes through runSql with the TVFs (exact-identifier bm25, meaning-aware hybrid, hybrid-over-synced-rows, staged-readiness and crash-recovery probes), truncation asserts partialIndex() directly, and the usage suite covers the sql receipt shape. The bench recall lane ranks through hybrid_search SQL; its vector-only lane keeps the raw engine call since it exists to isolate the embedder, not to model the agent surface. Verified: tsc --noEmit clean, vitest 100/100 across 13 files.
Drop the vector_search rejection added two commits ago - the policy is
that sql is the only door, not that some table functions are forbidden
inside it. vector_search('chunks','embedding', {{q}}, k) is meaning-only
ranking and occasionally the right relation; the tool description keeps
steering toward hybrid_search (which retains the keyword arm) without
refusing the query. The plugin hook likewise stops inspecting sql
statements; it still denies the legacy search tool and gates grep.
|
The hooks only exist for Claude Code plugin users, but the Since the legacy-search deny in the hook already proves the hook can steer agents to sql without the tool being deleted, could we split this? Land hooks + eager indexing now, and hold the |
|
The README today argues three tools is deliberate design ("one way to find, one way to count"); this PR rewrites that paragraph to argue two tools is deliberate design. Both can't be self-evident, so the deciding factor should be data about agent behavior, and I don't see any in the verification section. Before we delete the path, can we run even a small A/B: N fixed questions through a live session against the same repo, search-tool surface vs sql-only surface, comparing turns-to-answer, token spend, and failed/retried queries? If sql-only holds up, the deletion ships with confidence and the number goes in the PR description. If it doesn't, we just saved every non-Claude client from a regression. |
| const isGrep = | ||
| toolName === "Grep" || | ||
| (toolName === "Bash" && GREP_LAUNCH.test(input.tool_input?.command ?? "")); | ||
| if (!isGrep || indexState(input.cwd) !== "ready") return; |
There was a problem hiding this comment.
indexState walks up from the session cwd, but the deny fires on what the command targets. Two false-positive classes:
- Multi-repo workspaces: if the cwd's repo has a ready index,
rg foo ../other-repogets denied even though other-repo was never indexed. The hook should probably inspect the grep target path (or the Grep tool'spathinput) rather than assuming cwd. - Things the index excludes by design: gitignored files, files over the size cap, and edits newer than the last sync. "ready" means the index covers what it indexes, not everything grep can see. An agent grepping for a symbol it added thirty seconds ago can get denied while sql also misses it, which is the worst version of the redirect.
Related: is there an off switch? A search plugin silently disabling a built-in tool needs a documented escape hatch (CX_NO_ENFORCE=1 or similar) or the first confused user files an angry issue instead of an env var. Happy with deny as the default, just want the exit to exist.
There was a problem hiding this comment.
thanks — all three addressed in a001958:
- target, not cwd. the hook reads the command's explicit path targets (and the Grep tool's
pathand checks each against the discovered index root — a target outside the repo is allowed silently, so passes whatever this repo's index says. - index-excluded classes. gitignored (), over , dot-paths, nonexistent → allow; if sql has no rows for it, grep stays open. the edit from thirty seconds ago still reads as covered (it exists, it isn't excluded), so that case is handled by the escape below rather than the coverage check.
- off switch: . the deny message now names it, plus the per-need exit — re-run the command prefixed with and the decision becomes instead of . both in the README config table.
sixteen decision cases pinned against a fixture repo: deny/allow/ask, orphaned-manifest fail-open, grep (BSD grep, GNU compatible) 2.6.0-FreeBSD.
BODY
)
muralikpbhat
left a comment
There was a problem hiding this comment.
Reviewed across the two-tool surface, the eager index, the grep-deny hooks, the ported tests, and the docs. The core direction is good and a lot of it is executed carefully; the blocking items are the grep-deny trapping the agent, a lost readiness signal that compounds with eager indexing, two weakened tests, and two stale doc surfaces.
What's solid (no change needed)
search → sqlport is faithful. The TVFs preserve every fieldsearch()returned — path, line range, score (via TVF ordering), content — andk; content is now un-capped rather than truncated at 4000 chars, a gain. No ranking/field capability lost in the port itself.- Eager index at startup is correct.
buildIndexgoes through theexclusive()mutation lock, so a concurrent first query'sensureIndexedsees the build in flight and awaitsctx.mutation(no double-build); it'ssetImmediate-deferred so it does not block the MCP handshake; andCX_AUTO_INDEX=0correctly gates the eager block and restores the strictnoIndexerror. - Truncation + usage ports are equivalent (
partialIndex()asserts the samefilesSkipped/file-cap condition; the sql receipt is asserted, not just non-empty), andCX_SEARCH_K/DEFAULT_SEARCH_K/searchCmd/cx searchare fully removed with no dangling refs. README/AGENTS.md/llms.txt are updated to two tools. - The hook is built in the safe direction — it fails open (unparseable payload, probe throw, missing manifest → allow), only denies when
vectors === "ready", and matches an anchored regex (/^\s*(rg|grep|git\s+grep)\s/) sofoogrep,ps | grep,cmd && grepdon't over-match. Hygiene is clean (no secrets in the hooks).
Findings to address
1. The grep-deny traps the agent (blocking). Three problems, design fix below:
- Hard
denywith no per-need fallback (deny-grep.mjs:95). Once vectors are ready it returnspermissionDecision: "deny"; the only escape isCX_NO_EMBED+ a server restart (global, not something the agent can do mid-task). When SQL search genuinely comes up short, the fallback grep is walled off and the agent loops search→deny→search. - False-complete gate (
deny-grep.mjs:38readingindexer.ts:152). Completeness =truncatedFiles == 0, buttruncatedFilesonly counts the file-count cap. Files excluded byshouldIndexFile()and themaxFileBytesbyte cap (chunker.ts:110,config.ts:38-39) are silently unindexed and unrecorded — yet the hook claims "fully covers this repo." Sogrep API_KEY .env,grep <tok> Cargo.lock, a >1 MB generated JSON, a.csv→ denied, andhybrid_search/bm25have no rows for them → findable by no tool. - Over-broad Bash denial (
deny-grep.mjs:43) — scopes on command prefix only, never on the target, sogrep --version,grep pattern /var/log/system.log,grep foo ./build/output.txt,git grepin another tree are all blocked though none are repo code-search. (Also note the flip side:true && grep,LANG=C grep,cat f | grepslip the anchored regex — so enforcement is trivially bypassable for callers who don't need it, while the honest bare command is walled. Neither a reliable guardrail nor a usable fallback.)
2. The vector-readiness signal is gone, and eager indexing makes it bite (blocking). The deleted search() returned note: "vectors not ready yet — keyword-ranked only" on every call and auto-fell-back to bm25. The SQL surface surfaces readiness nowhere (except the one-time auto_indexed note when the query itself built the index). Because the index now builds eagerly at startup, an agent querying in the first seconds calls hybrid_search while vectors backfill and gets a keyword-only/partial result with no signal it's degraded — treating it as full hybrid recall. The old surface was self-correcting here; the new one is silent (searcher.ts runSql 94-114 / server.ts sql handler 226-260 pass straight to the engine regardless of manifest.vectors).
3. Two ports were weakened, so the regressions they guarded are now untested.
integration.test.ts:111-118— old "keyword ranking while vectors not ready" forced avectors:'building'handle and assertedranking==='keyword'andnote ~ /vectors not ready/. The port runs bm25 against a fully-ready handle and only assertsrows > 0— the not-ready path (finding #2) is no longer exercised.sync.test.ts:124-131— old assertedranking==='hybrid'proving sync re-embedded; dropped. The query term is bm25-satisfiable, so the test passes even if the vector stage never re-embeds the synced rows (the exact regression it was written to catch).
4. Two doc surfaces still advertise the removed search tool. skills/code-context/SKILL.md still says "three MCP tools (search, sql, reindex)", has a full ## search section, routes "how does X work / where is Y handled" to search, and tells agents to ToolSearch "search sql reindex" — it points agents at a tool that no longer exists and is actively denied by the hook. docs/faq.md still answers "Three, by design: search…" and references cx search. (README/AGENTS/llms.txt were updated; these two were missed.) Minor: a couple of stale search/sql comments (server.ts:67, the deny-grep header).
The compounding case worth calling out: on a freshly-started server an agent gets keyword-only results (vectors backfilling) → no signal it's degraded (#2) → reaches for grep to sanity-check → hard-denied with no fallback (#1) → and if it consults the skill, it's told to use search, which is gone (#4). The trap arrives from three directions at once.
Design: a grep-deny that nudges without trapping
Make the decision three-way, keyed on the target and an explicit fallback intent, so the ask fires only on a genuine fallback (not on every grep, which would just trade the trap for prompt-fatigue):
- Target the index doesn't cover →
allowsilently. A grep whose target is an unindexed file/path (excluded byshouldIndexFile/byte-cap, a log,./build/…, a path outside the repo) is auto-allowed — SQL can't answer it anyway. This alone removes most of the trap and fixes the over-broad denial. It requires the completeness/coverage check to account for the real excluded set, not just the count cap. - Reflexive grep on indexed source, no fallback signal →
deny+ redirect (as today), but the redirect names the escape. Keep nudging the first reflexive grep tohybrid_search— but the message must say how to fall back (it currently names none), e.g. "if the index search didn't find it, re-run withCX_GREP_FALLBACK=1and it will be allowed." - Grep carrying the fallback marker →
ask. The agent reaches for this only after a search came up short, so the human is prompted exactly when they should weigh in ("the agent tried search, it wasn't enough — allow this grep?") and nowhere else.
Notes: allow/deny/ask are all valid permissionDecision values and the hook already has the command string + the manifest, so this is a modest change to deny-grep.mjs, not a redesign. The marker relies on the agent's honesty — that's fine; the goal is making SQL the default, not hard-forbidding grep, and the ask is where a real human gate lives. Separately, guard the corrupted/stale-manifest case (a table deleted after the manifest went ready currently denies grep and errors sql — losing both paths); confirm the index is queryable, or fail open, before denying.
Net
The two-tool surface, SQL-as-search, and the eager-index mechanics are a good foundation and cleanly done. I'd hold on: the grep-deny redesign above, restoring the vectors-not-ready note on the SQL path, restoring the two tests' state assertions, and updating SKILL.md + faq.md to the two-tool surface.
Only conflict: src/core/config.ts - main dropped N_CENT with the 0.5.x engine's vector-spec change while this branch dropped DEFAULT_SEARCH_K with the search removal; neither constant has any remaining reference, so both go. Verified against the 0.5.1 engine: tsc clean, vitest 100/100.
…repairs Addresses the PR #14 review findings. The grep-deny no longer traps the agent. The decision is three-way and target-aware: a grep whose explicit target the index cannot answer for - outside the repo, gitignored, over the byte cap, a dot-path, or nonexistent - is allowed silently (sql has no rows for it); a reflexive grep on covered source is denied with a redirect that now names its own escape hatches; and a command prefixed with CX_GREP_FALLBACK=1 gets an 'ask' decision, putting a human on exactly the fallback moment and nowhere else. CX_NO_ENFORCE=1 disables enforcement entirely, a 'ready' manifest orphaned by a deleted table fails open instead of denying grep while sql also errors, and pattern-less invocations (grep --version) pass. All sixteen decision cases are exercised against a fixture repo. The vector-readiness signal lost with search() is restored on the SQL path: vectorsNote() surfaces 'vectors are still backfilling' on every sql result until the backfill lands - eager startup indexing made the silent window bite on the very first queries of a session. The two weakened test ports get their state assertions back: the integration suite asserts the not-ready note against a building manifest, and the sync suite proves re-embedding through vector_search, which can only return rows that actually have vectors. SKILL.md and docs/faq.md catch up to the two-tool surface (both still advertised search, which the hook itself denies), and the README documents enforcement and both escape hatches.
|
keeping the deletion, running the A/B. why not split. sql-as-the-only-door is the position, not a side effect, and a compat window recreates what this PR removes — two doors onto one index, choice per call. AGENTS.md's "near-duplicate retrieval tools worsen selection" bites hardest during a transition, not least. non-Claude clients. they lose the hooks, not the steering. for them the steering is the sql tool description, which now leads with the retrieval shape ( the A/B — agreed, and it gates the release. same bench/ lanes and question set, search-surface (0.2.0) vs sql-only, reporting turns-to-answer, tokens, failed/retried queries; table in the release notes before I tag. if comprehension regresses materially the release holds. calibration: the aggregation lane (-43% tokens, -71% calls) was already sql, and the deleted tool's job maps 1:1 onto hybrid_search in FROM — but that's the bet, not the evidence. |
|
thanks for the depth here — the coverage gap and the lost readiness signal were both real. all four on a001958 (fa43f77 merges current main; clean against v0.2.0):
compounding case end to end now: a first-seconds agent gets bm25 rows carrying the backfilling note, its sanity-check grep passes (deny gates on gates: |
The grep-deny's coverage check re-derived the indexer's skip rules and
got them wrong in both directions: it denied greps on files the index
never holds (lockfiles, .csv, LICENSE, vendor/, symlinks - sql answers
none of them, so the agent lost both paths), and it allowed indexed
dotfiles. Now the indexer records how many chunks each file produced
(filestate entries gain an optional count, stamped by both the build and
the sync, 0 included so a sync does not re-hash) and the hook covers
exactly the files with rows - verified equal to SELECT DISTINCT path
FROM chunks on four fixtures built by the real indexer, every file,
directory and glob form. A wholly count-less (pre-upgrade) state fails
open until a rebuild stamps it. The manifest records the repo root so a
CX_INDEX_DIR-relocated index still scopes targets, trusted only when it
agrees with where the index was found (a copied repo's stale root would
otherwise turn enforcement off silently).
The command reader now holds against the shapes agents actually type:
operands end at an unquoted pipe or comment (rg pat src | head was
allowed - downstream words read as uncoverable targets), heredoc bodies
are data, shell control words and group openers are stepped over
(if/while/for/{/(/!, backgrounding &), launchers match by basename
(/usr/bin/rg), git's global flags are walked (-C also moves the scope),
wrapper value-flags are per-wrapper (env -i ate the launcher), unquoted
trailing group-closers are syntax rather than targets, and a glob's
star-runs collapse before compiling (a 24-star token wedged the regex
past the hook timeout). Everything unparseable still fails open.
61 -> 73 enforcement cases, including ground-truth agreement driven
through the real indexer on both write paths.
…ient The plugin ships the hooks, but a hand-wired MCP server (npx, claude mcp add-json) has no hook surface at all - the tools arrive with no steering, agents keep grepping, and restarting changes nothing. 0.1.0 solved this with cx install and the command dropped out in 0.1.4; this brings it back for the two-tool surface: copy hooks/deny-grep.mjs to ~/.claude/hooks and merge SessionStart + PreToolUse entries into ~/.claude/settings.json, embedding the absolute node running the install because the client process often has no node on PATH and a hook that cannot find node fails silently. The settings file belongs to the user, so the edit is careful: ownership is per hook command keyed on the resolved script path (a foreign hook sharing our entry survives; a wrapper naming our basename is never touched), the rewrite is a same-directory temp file + rename that follows symlinks (dotfile-managed settings keep working) and writes hardlinks in place (a rename would quietly unlink the other name), uninstall asks sibling settings files before deleting the shared script and is a byte-identical round trip, a non-absolute HOME fails loudly instead of writing into the cwd, a project-scoped .claude target is refused (machine-specific absolute paths must not land in a version-controlled file; --force overrides), and malformed settings are refused rather than overwritten. Claude Code only, and says so - other MCP clients expose no hook surface to configure. package.json ships hooks/ in the tarball; cx --version reads package.json so it cannot drift again. 39 install cases; docs cover the plugin-vs-install split, both escape hatches, and the FAQ answer for making an agent actually use the index.
|
Correcting my two earlier replies before anything else: both overstated the coverage fix, and adversarial re-verification against real indexes proved it. What I claimed vs what was true. I said "if sql has no rows for it, grep stays open" and that coverage was checked "exactly where shouldIndexFile / the byte cap / gitignore bite." Executed counter-examples: on a fixture whose table held two files, the hook denied What replaced it (65b8093). Coverage is no longer modelled; it is read from the index's own record. Filestate entries now carry the chunk count each file produced (stamped by build and sync, 0 included), and the hook covers exactly the files with rows. Ground truth is the decisive gate now: on four fixtures built by the real indexer, the hook's deny set equals Parser, same commit. Operands end at an unquoted The distribution gap (088ab81). The hooks only ever reached plugin installs; a hand-wired server ( Gates: tsc clean, vitest 199/199 across 15 files (73 enforcement, 39 install), ground-truth equality on fx1/fx3/fx5/fxpy, wildcard-flood answers in ~125ms, install round-trips byte-identical through symlinked/hardlinked/foreign-hook settings. The verification harness runs the real indexer and compares against the table — the standard any future change to the hook should be held to. |
Installing the plugin now makes the index the way agents search code, and the
index has exactly one query surface: read-only SQL with ranked search
table-valued functions.
The MCP surface (breaking)
searchtool is gone. Two tools remain:sqlandreindex.sqlis the search surface:vector searchfor semantic,hybrid_searchfor the fused keyword + semantic pass,
bm25_search(...)for the keyword-onlywindow while vectors backfill, GROUP BY over either for counts/rankings,
regexp_likein WHERE. The instructions and tool description lead with theretrieval query shape instead of presenting sql as an analytics side-tool.
a query that beats the build still triggers it inline (
CX_AUTO_INDEX=0restores the strict error).
The search path is removed, not deprecated
Core
search()and its result shapes, thecx searchCLI command,CX_SEARCH_K, and the search-flavored usage receipt are deleted. Tests areported, not dropped: every retrieval assertion now goes through
runSqlwiththe TVFs; truncation asserts
partialIndex()directly; the bench recall laneranks through
hybrid_searchSQL (its vector-only lane keeps the raw enginecall — it isolates the embedder, not the agent surface). README, AGENTS.md,
and llms.txt describe the two-tool surface.
Enforcement hooks (ship with the plugin)
hooks/hooks.json+hooks/deny-grep.mjs, auto-loaded on install:grep/rg/git grepBash commands witha reason redirecting to the sql TVFs — but only once the index fully covers
the repo (manifest present, vectors ready, nothing truncated by the file
cap). Until then grep passes through: agents are never forced onto an index
that can't answer yet.
grepas a pipe filter on other command output always passes — filteringlogs or test output is a job the index does not do.
searchtool (older server builds) andvector_searchinside sql statements, same redirects.
isn't guaranteed on user machines.
The denials are self-correcting: Claude Code feeds the reason back to the
agent, so a reflexive grep becomes a sql query on the next step.
Verification
tsc --noEmitclean;vitest100/100 across 13 files.Code session: standalone
rgdenied with the redirect, pipeline grepuntouched, and the redirected query auto-built the index and answered in
one step.
Ships to users as the next npm release (needs a version bump + publish);
the hooks take effect on plugin update as soon as this merges.