Skip to content

feat: CLI/MCP search parity, MCP host contract, doctor diagnostics, install drift checks#24

Closed
m-szymanska wants to merge 7 commits into
developfrom
agent/aicx-search-mcp-closure
Closed

feat: CLI/MCP search parity, MCP host contract, doctor diagnostics, install drift checks#24
m-szymanska wants to merge 7 commits into
developfrom
agent/aicx-search-mcp-closure

Conversation

@m-szymanska

Copy link
Copy Markdown
Contributor

What

Closes the review gaps from the search/MCP/Sztudio session audit. One coherent slice across search retrieval, MCP transport, doctor diagnostics, installer, and docs.

Search parity (CLI ↔ MCP)

  • Shared retrieval primitives in search_engine: fuzzy_search_with_post_filters, finalize_fuzzy_results, fuzzy_fetch_limit. CLI aicx search and MCP aicx_search (success + fuzzy fallback) now route through the same code path — three duplicated inline copies removed.
  • MCP aicx_search is semantic-first with an explicit semantic_fallback payload and a strict_semantic opt-out for the old fail-fast behavior.

MCP HTTP host contract

  • --host / --allowed-host / --allow-any-host with a safe loopback default. Distinguishes bind host from Host-header validation (rmcp DNS-rebind guard). Symmetric across aicx serve and aicx-mcp.

Stale honesty

  • Hybrid search results carry an index_snapshot hint (freshness_verified: false, source_chunks) pointing at aicx index status. Zero search hot-path cost — freshness scanning stays off the query path.

Doctor diagnostics

  • New informational checks: aicx_home (resolved home vs default, store/indexed presence), binary_pair (aicx ↔ aicx-mcp version parity on PATH), http_auth_token (token source, never the value). Deliberately excluded from overall severity (diagnostic, not a health gate).
  • auth::probe_token_source: non-mutating token-source probe (no generation, no value exposure).

Installer drift

  • install.sh + npm install.js + 3 platform postinstalls now scan/verify both aicx and aicx-mcp as a versioned pair — catches the "fresh CLI, stale MCP" drift class.

Docs

  • MCP search contract, host validation (bind vs Host header), the single HTTP auth-token resolution cascade, and the index_snapshot signal. Public-safe examples only.

Test plan

  • New tests/cli_mcp_search_parity.rs: drives the real aicx binary (subprocess, fuzzy path) and the in-process MCP render path, asserts identical items.
  • Full gate green locally: cargo fmt, cargo clippy --all-targets -D warnings (clean), 1043 tests / 0 failed, git diff --check clean.

Known limitations

  • The three new doctor checks are informational only — CI gating solely on overall: green will not catch binary-pair drift (folding them in would false-fail when aicx-mcp is absent from PATH, e.g. in CI).
  • Semantic/hybrid retrieval via remote backend was not exercised in this slice (fuzzy fallback path is what the parity test covers).

Merge intentionally left to the maintainers.

🤖 Generated with Claude Code

m-szymanska and others added 5 commits June 27, 2026 22:47
Shadow scan and post-install path resolution were aicx-only, so a fresh CLI could ship next to a stale aicx-mcp without warning. Scan and verify both binaries (install.sh + npm install.js + 3 platform postinstalls) and report dry-run intent for shadow cleanup.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…stics

- Shared fuzzy primitives in search_engine (fuzzy_search_with_post_filters, finalize_fuzzy_results, fuzzy_fetch_limit) so the CLI and MCP fuzzy fallback cannot drift; remove the duplicated inline logic in both surfaces.
- MCP aicx_search is semantic-first with an explicit semantic_fallback payload and a strict_semantic opt-out for the old fail-fast behavior.
- HTTP transport gains --host / --allowed-host / --allow-any-host with a safe loopback default (distinguishing bind host from Host-header validation).
- Hybrid results carry an index_snapshot honesty hint (freshness_verified=false) pointing at `aicx index status`; zero search hot-path cost.
- doctor gains informational aicx_home / binary_pair / http_auth_token checks (not folded into overall severity); auth::probe_token_source reports the token source without generating or printing it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…shot

Document semantic-first search + fallback, --host/--allowed-host (bind host vs Host header), the single HTTP auth-token resolution cascade, and the index_snapshot freshness signal. Public-safe examples only (mcp.example.internal, localhost).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drive the real aicx binary (fuzzy path) as a subprocess and reproduce the exact public retrieval+render path that MCP aicx_search uses for its fuzzy fallback in-process, then assert the rendered items are identical. Closes the North Star gap: same query + same store => same results on both surfaces.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rsplit-on-whitespace took the last token, which a trailing build suffix ('0.9.4 (abc)') would corrupt. Select the first token starting with a digit instead so the CLI/MCP version comparison stays correct.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 29, 2026 01:11

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request unifies the search and post-filtering logic between the CLI and MCP surfaces, ensuring strict result parity and introducing a semantic-first search with a graceful fuzzy fallback. It also expands the MCP HTTP transport options with bind host, allowed host, and any-host validation configurations, accompanied by new informational diagnostics in the doctor command. Feedback on the changes highlights a compilation issue in the new parity test where the unstable let_chains feature is used; this should be simplified to nested if statements to ensure compatibility with stable Rust toolchains.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread tests/cli_mcp_search_parity.rs

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens CLI ↔ MCP behavioral parity for search by extracting shared fuzzy retrieval/finalization primitives, adds an explicit MCP semantic-fallback contract (with strict_semantic opt-back-in), formalizes an MCP HTTP bind/Host-validation configuration surface, and extends aicx doctor + installers to detect and explain CLI/MCP binary drift.

Changes:

  • Extract shared fuzzy retrieval + post-filter + finalize helpers into search_engine, and route both CLI aicx search and MCP aicx_search through them.
  • Add MCP HTTP host contract (--host, --allowed-host, --allow-any-host) and surface hybrid index_snapshot honesty hints in both CLI and MCP JSON payloads.
  • Add informational doctor checks (aicx_home, binary_pair, http_auth_token) and extend installers (shell + npm) to check both aicx and aicx-mcp drift.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/cli_mcp_search_parity.rs New integration test asserting CLI and MCP fuzzy-rendered items match byte-for-byte.
src/search_engine.rs Adds shared fuzzy fetch-limit + post-filter primitive, shared finalization, and index_snapshot injection helper + unit tests.
src/mcp.rs Implements semantic-first MCP search with explicit fuzzy fallback payload + strict_semantic, plus HTTP bind/Host-validation config plumbing.
src/main/tests.rs Updates tests to call search_engine::fuzzy_fetch_limit and verifies serve host-related CLI flags/help output.
src/main.rs Routes CLI search through shared fuzzy primitives + finalizer; adds serve host/allowed-host/allow-any-host flags and wires McpHttpConfig.
src/doctor/types.rs Extends DoctorReport schema with new informational checks.
src/doctor/tests.rs Adds unit tests covering new doctor checks.
src/doctor/report.rs Includes new informational checks in the text report output.
src/doctor/checks.rs Implements new doctor checks and keeps them out of overall gating.
src/bin/aicx_mcp.rs Adds MCP binary flags for HTTP bind/Host validation + tests for parsing defaults/overrides.
src/auth.rs Adds non-mutating probe_token_source and TokenSourceProbe to report token source without generating/reading it.
install.sh Extends shadow scanning and PATH-resolution verification to cover both aicx and aicx-mcp.
docs/ORACLE_CORPUS.md Updates operator guidance to semantic-first search + explicit fallback + index_snapshot signal.
docs/install-paths.md Documents drift class, dual-binary scan/verification, and host validation requirements for remote MCP.
docs/COMMANDS.md Documents aicx_search semantic-first contract, new serve host flags, and auth-token resolution cascade.
docs/ARCHITECTURE.md Updates MCP tool surface description and documents HTTP bind vs Host-header validation model.
distribution/npm/aicx/platform-packages/win32-x64-gnu/postinstall.js Extends npm postinstall shadow scanning to include aicx-mcp.
distribution/npm/aicx/platform-packages/linux-x64-gnu/postinstall.js Extends npm postinstall shadow scanning to include aicx-mcp.
distribution/npm/aicx/platform-packages/darwin-arm64/postinstall.js Extends npm postinstall shadow scanning to include aicx-mcp.
distribution/npm/aicx/install.js Extends npm install-time drift/shadow warnings to cover aicx-mcp in addition to aicx.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/search_engine.rs Outdated
Comment thread src/mcp.rs
m-szymanska and others added 2 commits June 28, 2026 19:45
Unknown sort tokens fall back to newest (timestamp/date desc), not
descending score. Align the doc with the implementation and unit test.

PR #24 review (copilot-pull-request-reviewer)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MCP semantic-success now passes kind_filter_dir to finalize_fuzzy_results
instead of None, matching the CLI tail. Kind is already pushed into the
semantic query; the retain is a defensive guard so an off-kind hit cannot
slip through if the semantic backend regresses, keeping CLI/MCP parity.

PR #24 review (copilot-pull-request-reviewer)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Szowesgad

Copy link
Copy Markdown
Contributor

Superseded by #26 (merged): the entire agent/aicx-search-mcp-closure branch was merged into fix/aicx-signals-revalidation (merge commit e44e7a2, union resolution documented there) and landed in develop via #26. Verified by containment: git log origin/agent/aicx-search-mcp-closure ^fix/aicx-signals-revalidation is empty — nothing from this PR is missing.

@Szowesgad Szowesgad closed this Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants