Add agentic retrieval: session state, vector grep, virtual files, related command, telemetry#9
Conversation
BashOptionsSchema and BashCacheConfigSchema in types.ts, persistent CWD tracking with SessionStateManager and WorkspaceTracker in bash-session.ts, plus comprehensive tests for session state, workspace, tool config, and bash tool integration.
parseGrepCommand and vectorGrep in bash-grep.ts with semantic + ILIKE text + dedup search passes, textSearchChunks with proper ILIKE escaping in queries.ts, and comprehensive grep tests.
Virtual files (INDEX.md, SEARCH_TIPS.md) in bash-virtual-files.ts, file metadata with buildFileMetadata and formatLsLong in bash-fs.ts, related command with self-exclusion fix and grep-miss suggestions in bash-related.ts, rebuildBashInstance webhook refresh, and tests for all features.
BashTelemetry in bash-telemetry.ts with buffer overflow protection and comprehensive telemetry tests.
Integrate all bash tool features (cd interception, grep interception, multi-source search, path validation) in bash.ts, session state wiring in server.ts and index.ts, bash tool options in mcp-docs.yaml, and documentation updates in README.md.
mme
left a comment
There was a problem hiding this comment.
PR Review Summary
Reviewed by 5 specialized agents (code-reviewer, test-analyzer, silent-failure-hunter, type-design-analyzer, comment-analyzer) across all 33 changed files.
Critical Issues (3 found — must fix before merge)
1. buildBashFilesMap doesn't resolve git-based source paths
File: src/mcp/tools/bash-fs.ts:55
buildBashFilesMap resolves source paths using path.resolve(source.path), which resolves relative to process.cwd(). Production sources have repo: fields and are cloned by the indexing orchestrator into /tmp/mcp-repos/<repoName>/. The function doesn't account for this — it needs to prepend the clone directory and repo name for git-based sources.
Both explore-docs and explore-code in mcp-docs.yaml reference git-based sources, so both bash tools will have zero files in production.
Fix: Accept a cloneDir parameter and, for sources with a repo field, construct the path as path.join(cloneDir, repoName, source.path) — mirroring how SourceIndexer resolves repo paths.
2. Bash grep_strategy: vector/hybrid without search tools crashes at runtime
Files: src/index.ts:318-324, src/config.ts:49-53
DB initialization (initializeSchema()) is gated on hasSearchTools() || hasCollectTools(). But a bash-only config with grep_strategy: vector calls vectorGrep → searchChunks → getPool(), which throws because the pool was never created.
Fix: Add a hasBashVectorGrep() helper and include it in the DB init condition:
const needsRag = hasSearchTools() || hasBashVectorGrep();
const needsDb = needsRag || hasCollectTools();3. Shared mutable Bash filesystem across sessions
Files: src/index.ts:36, src/mcp/server.ts:54
A single Bash instance per tool name is shared across all MCP sessions. The just-bash filesystem persists writes across exec() calls, so one session's echo "x" > /docs/file.mdx or rm /INDEX.md corrupts the shared instance for all concurrent and future sessions.
Fix: Consider per-session OverlayFs copy-on-write layers, per-session instances, or at minimum document this as intentional and accept that the FS is shared.
Important Issues (11 found — should fix)
4. vectorGrep silently degrades to single-backend search
src/mcp/tools/bash-grep.ts:72-87 — When one of semantic/text search fails, results are silently incomplete. Only a console.warn is emitted. The agent gets a normal-looking grep result with no indication it's partial.
5. Empty file map from bad paths silently produces useless tool
src/mcp/tools/bash-fs.ts:54-59 — Misconfigured paths or bad volume mounts → 0 files loaded with only a console.warn. The tool appears healthy but returns empty results.
6. refreshBashInstances is fire-and-forget with unreliable 5s timeout
src/index.ts:73-77 — Webhook returns 200, then refresh is scheduled via setTimeout(5000). If refresh fails, bash serves stale data indefinitely. The 5s heuristic is also likely too short for actual reindexing.
7. Config.databaseUrl set to empty string '' when not needed
src/config.ts:72-73 — Creates a latent crash if any code path reaches getPool() unexpectedly. Should use undefined instead.
8. parseGrepCommand mishandles flags with arguments
src/mcp/tools/bash-grep.ts:36-47 — grep -m 5 "pattern" treats 5 as the pattern. Flags like -e, -m, -f take arguments that aren't consumed by the parser.
9. Command injection in shell strings via unescaped paths
src/mcp/tools/bash.ts:88,111 — related command interpolates user paths into cat "${path}" without escaping. Limited blast radius (WASM sandbox) but still a bug. Use bash.exec('cat', { args: [resolvedPath] }) instead.
10. Misleading comment: "stateless per-request"
src/mcp/server.ts:10-14 — createMcpServer JSDoc says "stateless per-request" but the function creates per-session servers with shared state.
11. Misleading comment: "each exec starts from /"
src/index.ts:330 — False when session_state is enabled; commands start from the session's persisted CWD.
12. README documents workspace & telemetry as features, but neither is wired up
WorkspaceTracker and BashTelemetry are defined and tested but never instantiated in the actual tool execution path. The README presents them as working features.
13. BashOptionsSchema .default().partial() anti-pattern
src/types.ts:61-68 — The .default() calls on inner fields are unreachable after .partial() on the outer object. Consumers must use ?? everywhere. The schema promises defaults it cannot deliver.
14. Piped grep silently bypasses vector search
src/mcp/tools/bash.ts:124-139 — cat file | grep pattern falls through to in-memory bash, completely bypassing vector infrastructure with no indication to the agent.
Suggestions (8 found — nice to have)
TelemetryEventshould be a discriminated union instead of optional fields — currently allows invalid combinations (bash-telemetry.ts:1-7)ParsedGrepshould use discriminated union instead ofisGrep: boolean— TypeScript can't narrow the type (bash-grep.ts:4-9)- No integration test for vector grep interception through
registerBashTool— unit tests exist for both sides but wiring is untested (bash.ts:119-135) - No test for
createMcpServerbash tool wiring — all MCP tests callregisterBashTooldirectly (server.ts:51-73) - No test for
relatedcommand interception through the registered tool (bash.ts:102-117) textSearchChunksILIKE escaping has no unit test (db/queries.ts:85-116)SessionStateManagerhas no size cap — unbounded memory growth under burst load (bash-session.ts:22-45)- Vector grep ignores file path arguments from grep command — not documented (
bash.ts:123-125)
Strengths
- Clean discriminated union for tool types (
AnyToolConfigSchema) with exhaustive switch - Thorough Zod cross-field validation in
superRefine - Strong
BashSessionStateencapsulation with proper POSIX path handling - Excellent degradation testing in
vectorGrep— all 4 backend combinations covered - Good MCP protocol integration tests using
InMemoryTransport - 154 tests covering features, edge cases, and error paths
- Config validation tests are especially thorough
Cross-cutting Theme
The most dangerous pattern across this PR is silent degradation presented as normal operation. Issues #4, #5, #6, and #14 all share the same problem: when infrastructure fails or is misconfigured, the system continues with reduced capability while giving the agent zero indication that results are incomplete or stale. In a retrieval system where the agent makes decisions based on what it finds, silently incomplete results lead to wrong conclusions with no reason to suspect the search infrastructure.
🤖 Reviewed with Claude Code
|
In addition to the previous preview: Review feedback
The
The review notes "vector grep ignores file path arguments" only in the suggestions section, but the flag issue is equally concrete. Workspace and telemetry are uninstantiated This is the point I'd most want to echo from the existing review. The README presents both as working features, but neither is hooked into the execution path. |
|
Worth considering not to mess around with Instead, expose a command line tool that is used in the real world for semantic search. https://github.com/tobi/qmd is well known, used in the memory engine of OpenClaw and feels like a great candidate. |
…n and command injection - Remove grep interception from bash tool; grep now passes through to bash unchanged - Add qmd command interception for vector-backed semantic search (inspired by tobi/qmd) - Add parseQmdCommand() to bash-grep.ts with support for quoted and unquoted queries - Fix parseGrepCommand to skip argument tokens for flags -e, -m, -f, -A, -B, -C - Fix command injection in related command by escaping path before interpolation - Fix related self-exclusion false positive: use exact vPath match instead of basename suffix - Update grep-miss suggestion to recommend qmd alongside search tools - Add tests for parseQmdCommand, flag-with-argument parsing, self-exclusion fix, grep passthrough
- buildBashFilesMap now resolves git-based source paths via cloneDir parameter - Add hasBashSemanticSearch() and include it in needsDb condition - Change Config.databaseUrl to string|undefined, remove empty-string fallback - Add undefined guards in db/client.ts getPool/initializePGlite/initializeSchema - Remove misleading .default() calls from BashOptionsSchema (negated by .partial()) - Fix stale comments in server.ts and index.ts about stateless behavior - Increase webhook bash refresh delay from 5s to 30s with TODO for event-based
…e unadvertised features - Replace grep_strategy description to document the qmd semantic search command instead of claiming grep is intercepted/routed through vector search - Add qmd command usage example and description alongside existing related command - Remove workspace and telemetry bullet points (WorkspaceTracker and BashTelemetry are defined but never instantiated in the execution path) - Remove workspace: true from the YAML example - Add shared filesystem known limitation note - Update grep_strategy default comment in YAML example
…dup order, partial failure warnings, and self-exclusion test
- config.ts: parseConfig() needsDb now includes hasBashSemanticSearch() to match index.ts logic
- bash-grep.ts: iterate semantic results first in dedup to preserve similarity scores
- bash-grep.ts: add stderr warnings when one search backend fails but the other succeeds
- bash-related.test.ts: fix self-exclusion test filePath to match vPath construction (/${source_name}/${file_path})
- server.ts: Fix JSDoc to reflect that each bash tool gets its own FS instance - README: Fix cd example (only bare cd persists CWD, not compound commands) - README: Fix 3-pass to 2-pass search description - README: Fix shared FS note (filesystem is read-only, no write path) - README: Remove max_file_size and cache from bash YAML (dead/ignored config)
… example config, clipboard fallback
Summary
Evolves mcp-docs bash tools from static in-memory filesystem exploration into an integrated agentic retrieval system that matches and exceeds Mintlify's ChromaFS approach. All features are opt-in via the
bash:config subsection — existing configs work unchanged.cd /docs && lsworks across separate tool calls. Path validation rejects nonexistent directories.INDEX.md(file listing) andSEARCH_TIPS.md(usage guidance) injected into the virtual filesystem at startup.related /path/to/filecommand finds semantically similar files across all sources. Grep misses suggest companion search tools.buildFileMetadataandformatLsLongforls -lstyle output with sizes and line counts./workspace/directory with 1MB size cap.Full design: Proposal on Notion
Test plan
npx vitest run)npx tsc --noEmit)relatedcommand returns meaningful results against CopilotKit docs🤖 Generated with Claude Code