Add multi-source Pinecone MCP support - #187
Conversation
📝 WalkthroughWalkthroughThis PR adds multi-source Pinecone support: source parsing and config resolution, source-aware runtime routing, updated MCP tool contracts and handlers, CLI/startup wiring, redacted error handling, and supporting documentation and examples. ChangesMulti-source mode implementation
Sequence Diagram(s)sequenceDiagram
participant CLI
participant resolveConfig
participant resolveSourceDefinitions
participant ServerContext
participant SourceRegistry
CLI->>resolveConfig: overrides + env
resolveConfig->>resolveSourceDefinitions: sources/configFile
resolveConfig-->>ServerContext: CoreServerConfig
ServerContext->>SourceRegistry: buildSourceRegistry(sources)
SourceRegistry-->>ServerContext: registry
sequenceDiagram
participant Tool
participant resolveSourceForTool
participant ServerContext
participant SourceRegistry
participant PineconeClient
Tool->>resolveSourceForTool: source, namespace
resolveSourceForTool->>ServerContext: resolveSource(source, namespace)
ServerContext->>SourceRegistry: lookup namespaces
SourceRegistry-->>ServerContext: source match / ambiguity / partial failure
ServerContext-->>resolveSourceForTool: ok(source) / error code
resolveSourceForTool-->>Tool: activeCtx, activeSource
Tool->>PineconeClient: getClientForResolvedSource(activeCtx, activeSource)
PineconeClient-->>Tool: results
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #187 +/- ##
=======================================
Coverage ? 83.86%
=======================================
Files ? 45
Lines ? 2306
Branches ? 768
=======================================
Hits ? 1934
Misses ? 372
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review again |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 18
🤖 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 `@examples/alliance/suggest-flow-demo.ts`:
- Line 58: The current main().catch(exitOnDemoFailure('suggest-flow-demo')) path
drops the rejected error, so all failures collapse into one generic exit. Update
exitOnDemoFailure to accept the caught error and use it when logging from
suggest-flow-demo so the original failure reason is preserved. Make sure the
helper redacts sensitive details before emitting the error, and keep the
existing identifier exitOnDemoFailure as the entry point for the fix.
In `@examples/lib/exit-on-failure.ts`:
- Around line 4-7: The exitOnDemoFailure helper currently calls process.exit(1)
immediately after logging, which can prevent stderr from flushing in piped/CI
runs. Update the exitOnDemoFailure function to set process.exitCode = 1 instead
of exiting right away, and let Node terminate naturally after the error message
is written. If a hard shutdown is still required, ensure stderr is flushed
before any explicit exit.
In `@scripts/test-search.ts`:
- Around line 227-228: The outer catch in test() is swallowing startup error
details, so failures from initialization in the test-search flow lose their
message. Update the catch around test() to log the same redacted error details
used elsewhere in the script, and make sure the logging path still covers early
failures like new PineconeClient(...) without exposing credentials.
In `@src/alliance/config.ts`:
- Around line 51-57: The Alliance config setup is ignoring the resolved defaults
and still hard-coding the built-in fallback values. Update
`allianceParseOptions` in `resolveConfig` to use the already-resolved
`indexName` and `rerankModel` values from the Alliance defaults logic, so
sources missing those fields inherit the caller-selected defaults instead of
`ALLIANCE_DEFAULT_INDEX_NAME` and `ALLIANCE_DEFAULT_RERANK_MODEL`.
In `@src/core/config.ts`:
- Around line 200-222: The multi-source config branch is pulling top-level
fields from multi.sources[0] instead of the resolved defaultSource, which makes
the returned config inconsistent. Update the logic in
resolveSourceDefinitions/brandCoreConfig handling so apiKey, indexName,
sparseIndexName, and rerankModel are copied from multi.defaultSource (or the
matching default source object), while still preserving sources and
defaultSource in the returned config. Ensure src/index.ts and any other
consumers read the default source’s values consistently.
In `@src/core/server/format-query-result.ts`:
- Around line 82-85: The row-level source field in format-query-result should
only be emitted in multi-source mode, not whenever options.source is present.
Update the object construction in format-query-result to gate the source
property the same way the rest of the tool surface does, using
ctx.isMultiSource() (or the equivalent multi-source check) alongside
options.source so legacy single-source results stay source-less. Keep the change
localized around the formatQueryResult logic and the source spread handling.
In `@src/core/server/multi-source.context.test.ts`:
- Around line 41-49: The test callbacks in multi-source.context.test.ts are
returning an invalid UrlGenerationResult.method value; replace the placeholder
string in the ctx.registerUrlGenerator calls with a valid union literal such as
generated.custom so the type check passes. Use the registerUrlGenerator setup in
the shared context test as the location to update both generators consistently.
In `@src/core/server/server-context.ts`:
- Around line 324-345: The namespace resolution path in server-context.ts should
not trust partial results from getNamespacesWithCache() when source_errors are
present. Update resolveSource to detect degraded aggregation and, instead of
returning NAMESPACE_NOT_FOUND or a single-source match from the surviving data,
return an unresolved/retry-style response that forces the caller to provide
source explicitly or retry. Use the existing resolveSource,
getNamespacesWithCache, and source_errors handling to locate and gate this
branch.
- Around line 387-409: Source-scoped URL generators are only being registered
under source-qualified keys, but ServerContext lookup/removal APIs still use
namespace-only keys. Update registerUrlGenerator, unregisterUrlGenerator, and
hasUrlGenerator in ServerContext to handle source-aware keys consistently by
using the source argument when provided and, when omitted in multi-source mode,
checking/removing all source-qualified entries for the namespace. Make sure the
logic stays aligned with urlRegistryKey and the existing urlGenerators map
behavior.
In `@src/core/server/source-config.ts`:
- Around line 203-212: The source selection logic in source-config.ts is
prioritizing PINECONE_SOURCES over an explicit configFile override, so the CLI
cannot honor --config-file as intended. Update the config resolution in the
source parsing path around the inline/configFile checks to prefer
overrides.configFile before falling back to env['PINECONE_SOURCES'], keeping the
parseSourcesConfigFile and parseInlineSources behavior unchanged otherwise.
Ensure the explicit configFile branch wins whenever it is provided, regardless
of whether inline sources are also present in the environment.
In `@src/core/server/source-tool-utils.ts`:
- Around line 46-58: Update sourceValidationError in source-tool-utils.ts so
namespace resolution failures are attached to the namespace field instead of
defaulting to source. Use the existing field parameter and ensure
ctx.resolveSource() callers pass 'namespace' when the code is
NAMESPACE_NOT_FOUND, while keeping UNKNOWN_SOURCE and AMBIGUOUS_NAMESPACE
aligned with source/namespace as appropriate. This change should be localized
around sourceValidationError and the ResolveSourceFailureCode handling so
downstream validation highlights the correct input.
In `@src/core/server/tools/guided-query-tool.ts`:
- Around line 122-149: The auto-routing flow in guided-query-tool still looks up
namespace metadata by namespace only, which can mix sources when the same
namespace exists in multiple projects. Update the metadata lookup in
suggestQueryParams to also match against selectedSource, and ensure the resolved
source from resolveSourceForTool/ctx.resolveSource is carried through to the
lookup before building the query params.
In `@src/core/server/tools/keyword-search-tool.ts`:
- Around line 78-92: The keyword_search tool currently ignores a provided source
when ctx is undefined, causing the legacy registerKeywordSearchTool(server) path
to fall back to the default client. Update the keyword-search-tool flow around
resolveSourceForTool and getClientForResolvedSource so that source is either
rejected on the ctx-less path or explicitly resolved into a source-aware default
context before client selection. Make sure the behavior is enforced in the
keyword_search entrypoint, not just when ServerContext is present.
In `@src/core/server/tools/list-namespaces-tool.ts`:
- Around line 25-27: The fallback path in list-namespaces-tool.ts drops the
requested source because `getNamespacesWithCache()` is called without it when
`ctx` is absent. Update `listNamespacesTool` to preserve `source` on the no-ctx
path by routing through a source-aware helper or by rejecting `source` when
`ctx` is missing, so `registerListNamespacesTool` and `list_namespaces({ source
})` behave consistently.
In `@src/core/server/tools/namespace-router-tool.ts`:
- Around line 55-57: The namespace_router tool currently ignores the input
source when no ServerContext is provided, so legacy
registerNamespaceRouterTool(server) calls can rank against all namespaces.
Update the NamespaceRouterTool execution path in namespace-router-tool.ts so the
fallback branch is source-aware by passing source into getNamespacesWithCache,
or explicitly reject source when ctx is absent. Make sure the
ctx.getNamespacesWithCache(source) and fallback behavior both preserve the same
recommended_namespace and recommended_source semantics.
In `@src/core/setup.ts`:
- Around line 168-170: The conditional around registerListSourcesTool in
setupCoreServerOnContext is checking ctx.isMultiSource() before configuration
has been resolved, so new ServerContext instances can skip list_sources
incorrectly. Update the setup flow to force config resolution first (through
getConfig or an equivalent eager path) and then decide whether to register
list_sources based on the resolved source count. Keep the fix localized to
setupCoreServerOnContext and the ctx.isMultiSource logic so multi-source
deployments always register the tool when expected.
In `@src/index.ts`:
- Around line 88-105: The `config.checkIndexes` branch in `src/index.ts` only
exits on failure, so a successful `ctx.checkAllIndexes()` run still falls
through and starts the stdio server. Update the `checkIndexes` flow to terminate
immediately after printing the success message, using the existing
`ctx.checkAllIndexes()` / `config.checkIndexes` block as the place to return or
exit with code 0 so this mode stays one-shot.
- Around line 91-93: The stderr reporting in the index-checking flow is printing
raw `checkIndexes()` errors, which can leak Pinecone secrets. Update the
`checkAllIndexes()` error path in `src/index.ts` so the loop redacts each `err`
with `redactApiKey()` before calling `process.stderr.write(...)`, or sanitize
the errors earlier in the checker. Keep the fix localized to the
`checkAllIndexes()` / `checkIndexes()` flow and ensure no raw API key text is
emitted.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4cf4f787-97d9-4138-b7ba-c0c95d1e4619
📒 Files selected for processing (52)
.env.exampleCHANGELOG.mdREADME.mdbenchmarks/latency.tsdocs/CONFIGURATION.mddocs/MIGRATION.mddocs/README.mddocs/SECURITY.mddocs/TOOLS.mdexamples/alliance/.env.exampleexamples/alliance/custom-url-generator.tsexamples/alliance/guided-query-demo.tsexamples/alliance/library-embedding-demo.tsexamples/alliance/suggest-flow-demo.tsexamples/lib/exit-on-failure.tsexamples/multi-source/pinecone-sources.json.exampleexamples/quickstart/mcp-demo.tsexamples/quickstart/seed-data.tsscripts/test-search.tssrc/alliance/config.tssrc/alliance/tools/suggest-query-params-tool.tssrc/cli.tssrc/constants.tssrc/core/config.tssrc/core/index.tssrc/core/pinecone-client.tssrc/core/pinecone/indexes.tssrc/core/server/format-query-result.tssrc/core/server/multi-source.context.test.tssrc/core/server/namespace-router.tssrc/core/server/response-schemas.tssrc/core/server/server-context.tssrc/core/server/source-config.test.tssrc/core/server/source-config.tssrc/core/server/source-registry.test.tssrc/core/server/source-registry.tssrc/core/server/source-tool-utils.tssrc/core/server/tool-error.tssrc/core/server/tools/count-tool.tssrc/core/server/tools/generate-urls-tool.tssrc/core/server/tools/guided-query-tool.multi-source.test.tssrc/core/server/tools/guided-query-tool.tssrc/core/server/tools/keyword-search-tool.tssrc/core/server/tools/list-namespaces-tool.context.test.tssrc/core/server/tools/list-namespaces-tool.tssrc/core/server/tools/list-sources-tool.tssrc/core/server/tools/namespace-router-tool.tssrc/core/server/tools/query-documents-tool.tssrc/core/server/tools/query-tool.tssrc/core/server/tools/test-helpers.tssrc/core/setup.tssrc/index.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/core/server/source-config.test.ts (1)
44-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting a shared temp-config-file test helper.
The
mkdtempSync(join(tmpdir(), ...))+writeFileSync(..., JSON.stringify({...}))boilerplate is repeated 5x in this file and again insrc/alliance/config.test.ts. The PR summary mentions a shared test fixture was introduced — consolidating this pattern into a small helper (e.g.,writeSourcesConfigFile(sources, defaultSource)) would reduce duplication across both test files.Also applies to: 70-81, 111-133, 135-154, 156-172
🤖 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 `@src/core/server/source-config.test.ts` around lines 44 - 56, Extract the repeated temp JSON config setup in source-config.test.ts into a small shared helper, since mkdtempSync(join(tmpdir(), ...)) and writeFileSync(JSON.stringify(...)) are duplicated across multiple parseSourcesConfigFile tests and in alliance/config.test.ts. Add a reusable helper like writeSourcesConfigFile (or similar) near the test utilities and update the affected tests to call it instead of inlining the temp-file boilerplate.
🤖 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.
Nitpick comments:
In `@src/core/server/source-config.test.ts`:
- Around line 44-56: Extract the repeated temp JSON config setup in
source-config.test.ts into a small shared helper, since
mkdtempSync(join(tmpdir(), ...)) and writeFileSync(JSON.stringify(...)) are
duplicated across multiple parseSourcesConfigFile tests and in
alliance/config.test.ts. Add a reusable helper like writeSourcesConfigFile (or
similar) near the test utilities and update the affected tests to call it
instead of inlining the temp-file boilerplate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2c5b1aa-450d-4238-b44f-d6ac0536ea0e
📒 Files selected for processing (22)
examples/lib/exit-on-failure.tsscripts/test-search.tssrc/alliance/config.test.tssrc/alliance/config.tssrc/core/config.tssrc/core/server/format-query-result.context.test.tssrc/core/server/format-query-result.tssrc/core/server/multi-source.context.test.tssrc/core/server/server-context.tssrc/core/server/source-config.test.tssrc/core/server/source-config.tssrc/core/server/source-tool-utils.tssrc/core/server/tools/guided-query-tool.multi-source.test.tssrc/core/server/tools/guided-query-tool.tssrc/core/server/tools/keyword-search-tool.tssrc/core/server/tools/list-namespaces-tool.test.tssrc/core/server/tools/list-namespaces-tool.tssrc/core/server/tools/namespace-router-tool.tssrc/core/server/url-registry.tssrc/core/setup-multi-instance.test.tssrc/core/setup.tssrc/index.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- src/core/server/tools/guided-query-tool.multi-source.test.ts
- src/core/setup.ts
- src/core/server/multi-source.context.test.ts
- src/core/server/tools/list-namespaces-tool.ts
- src/core/server/format-query-result.ts
- scripts/test-search.ts
- src/core/server/tools/namespace-router-tool.ts
- src/alliance/config.ts
- src/core/server/source-config.ts
- src/index.ts
- src/core/server/tools/keyword-search-tool.ts
- src/core/server/server-context.ts
- src/core/server/tools/guided-query-tool.ts
- src/core/config.ts
- Redact non-Error rejections in test-search catch handlers - Fix TOOLS.md: selected_source is experimental, not stable - Reject source param on legacy tool paths without ServerContext - Treat JSON null sparseIndexName/rerankModel as omitted in config file Co-authored-by: Cursor <cursoragent@cursor.com>
524961d to
7cfaa65
Compare
Summary
PINECONE_SOURCES,PINECONE_CONFIG_FILE, or CLI--sources/--config-file.SourceRegistry,resolveSource(), per-source namespace caches, compound suggest-flow keys (source:namespace), and per-source URL generators.list_sourceswhen more than one source is configured; adds optionalsourceon discovery and execution tools with provenance on responses.PINECONE_API_KEY+PINECONE_INDEX_NAMEunchanged).TOOLS.md/CONFIGURATION.md.guided_querytrace,list_namespacessmoke) and sharedcreateMultiSourceTestContext()fixture.Test plan
npm run buildnpm test(319 tests)PINECONE_SOURCESorPINECONE_CONFIG_FILE—list_sources, aggregatedlist_namespaces,guided_querywithselected_sourcesourcefields, nolist_sources)Security / deployment notes
Related
Summary by CodeRabbit
--sources, or JSONPINECONE_CONFIG_FILE) with a newlist_sourcestool when multiple sources are configured.sourcewhere applicable.guided_queryvalidation/routing.