fix(#83): default MCP_SERVER_URL to dev Kapa AI endpoint

[imtushar01]

This PR fully resolves #83 by not only setting a default MCP_SERVER_URL, but also implementing a working MCP integration so the research workflow functions end-to-end.

Previously:

• MCP_SERVER_URL had no default and required manual .env configuration
• useMockMCP: false caused a crash because callKapaAI() was not implemented
• The system fell back to mocked responses instead of real Kapa AI results

What this PR does

• Defaults MCP_SERVER_URL to:
  https://mcp-docs.dev.medicmobile.org/mcp
• Allows users to override via .env
• Implements MCP client integration for real API calls
• Wires DocumentationSearchAgent.callKapaAI() to use the MCP client
• Ensures npm run research works out-of-the-box without crashing
• Adds logging for the MCP server URL being used

Result

• No manual configuration required
• No crashes when using real MCP
• Research workflow returns real Kapa AI documentation results

Fixes#83

[sugat009]

Thanks for wiring up the real MCP integration! I confirmed npm run research works end-to-end against a local MCP server and returns real Kapa AI documentation results. The MCPClient implementation (JSON-RPC 2.0, timeout handling, error handling, response parsing) is solid.

Heads up: We've set up SonarCloud on this repo. Newer commits will also need to pass SonarCloud quality checks in CI.

A few items to address below.

[sugat009]

issue (blocking): The default URL 'https://mcp-docs.dev.medicmobile.org/mcp' is hardcoded here and in src/mcp/client.ts:19. If the default ever changes, one will get missed.

suggestion: Extract this into a shared constant in a new src/constants.ts file (e.g., DEFAULT_MCP_SERVER_URL). Both this fallback and the DEFAULT_CONFIG in client.ts can reference it. This also gives us a single place for project-wide constants going forward.

[sugat009]

question (blocking): topics is hardcoded to [] here, which means relatedTopics in the returned response is always empty, and identifyRelatedDomains() returns nothing for real MCP calls. Mock mode still returns populated topics, so existing tests pass, but the real path has non-functional topic/domain detection.

Similarly, MCPToolName (src/types/index.ts:297) is defined but never referenced anywhere, and 'mock' | 'error' were added to the source union type (src/types/index.ts:161) but are never assigned (mock returns 'cached', error returns 'kapa-ai').

These all appear to come from PR #86. The scope outlined in the PR #85 review was to pull MCPClient, the related types, and the wiring needed to make the research agent work end-to-end with a real MCP server. These three items seem to have been pulled from #86 as well but without the implementations that give them purpose. Were these intended for future use, or were the corresponding implementations missed during the cherry-pick?

[sugat009]

suggestion (non-blocking): sinon.stub(globalThis, 'fetch' as any) — the as any cast hides type mismatches silently. Consider a type-safe approach or at minimum adding a comment explaining why the cast is needed.

[sugat009]

suggestion (non-blocking): process.env.MCP_SERVER_URL is saved and restored inside the test body. If an assertion fails between the mutation on line 13 and the restore on line 18, the env stays polluted for subsequent tests. Consider moving the save/restore into beforeEach/afterEach to guarantee cleanup.

[imtushar01]

Hey @sugat009
Thank you for the review I will shortly commit the changes

[sugat009]

Hey @sugat009 Thank you for the review I will shortly commit the changes

Cool, thanks!

[imtushar01]

Hey @sugat009
Can you please suggest how should i resolve the topic : [] issue with this pr
Thank you

[sugat009]

You can extract topics from the document content using keyword matching. Here's how:

1. Add this private method to DocumentationSearchAgent:

private extractTopics(content: string): string[] {
  const keywords = [
    'contact', 'hierarchy', 'form', 'report', 'task', 'target',
    'permission', 'role', 'sync', 'replication', 'offline',
    'sentinel', 'transition', 'workflow', 'validation',
  ];

  const lowerContent = content.toLowerCase();
  const topics = keywords.filter(keyword => lowerContent.includes(keyword));
  return [...new Set(topics)];
}

2. Replace topics: [] on line 88 with:

topics: this.extractTopics(doc.content),

This is the same approach used in #86.

[imtushar01]

Thank you, @sugat009, for the guidance. I’ll address the review comments and let you know once all checks pass successfully.

[imtushar01]

Hey @sugat009!
I have successfully made the changes and passed all the checks. Can you please review it suggest if any thing still needs to be changed.
Thank you

[sugat009]

Re-review: PR #90 (commit df720fe)

Good news first — all items from the prior round are resolved:

• DEFAULT_MCP_SERVER_URL extracted to src/constants/index.ts and imported in both client.ts and the agent ✓
• topics: [] replaced with extractTopics() ✓
• MCPToolName removed; 'mock' | 'error' gone from the source union ✓
• Test env save/restore moved into beforeEach/afterEach ✓
• as any cast on fetch stub now has an explanatory comment ✓

Also confirmed end-to-end: ran npm run research tickets/contact-search-feature.md against the dev MCP endpoint (both with MCP_SERVER_URL explicitly set and unset to verify the default fallback). 10 real docs come back, source: kapa-ai, no crashes. The PR's core claim holds.

One thing the test suite is failing on, though. Details below.

---

Blocker: 4 failing tests on HEAD

139 passing (90ms)
4 failing

1) DocumentationSearchAgent
     search
       should return research findings for forms-and-reports domain:
   AssertionError: expected [ 'form', 'validation' ] to include 'forms'
    at test/agents/documentation-search-agent.spec.ts:58:59

2) DocumentationSearchAgent
     processMCPResponse
       should return empty findings for unsuccessful response:
   TypeError: agent.processMCPResponse is not a function
    at test/agents/documentation-search-agent.spec.ts:356:39

3) DocumentationSearchAgent
     processMCPResponse
       should extract unique code examples:
   TypeError: agent.processMCPResponse is not a function
    at test/agents/documentation-search-agent.spec.ts:376:39

4) DocumentationSearchAgent
     processMCPResponse
       should use "cached" source when using mock MCP:
   TypeError: agent.processMCPResponse is not a function
    at test/agents/documentation-search-agent.spec.ts:393:39

The 3 processMCPResponse tests — processMCPResponse was renamed to buildFindings in this PR, but the test file wasn't updated. They also need their mock data reshaped from the old MCPResponse { success, data: { references: [...] } } shape to the MCPParsedDocument[] shape that buildFindings accepts (or deleted if buildFindings is already covered by the domain-specific search() tests above).

The forms-and-reports test — assertion expects topics to include 'forms' (plural) but extractTopics has 'form' (singular) in its keyword list, so the matched value is 'form'. Either the keyword list should include the plural, or the test should assert 'form'.

Heads up on why CI didn't catch this: the CI checks on this PR are only Lint PR title and SonarCloud — there's no npm test job. Worth running npm test locally before the next push. (Adding a test job to CI is probably a good follow-up — I can open an issue for that separately.)

---

Question: is relevantExamples intentionally always empty now?

DocumentationReference.codeExamples is declared in the type and populated in mockData (lines 130, 145, 160, 191, 208 of documentation-search-agent.ts), but the new flow drops it end-to-end:

• mockKapaAIResponse (line 216-221) converts DocumentationReference[] → MCPParsedDocument[] without carrying codeExamples over
• parseDocumentSection in client.ts doesn't extract codeExamples from the markdown either
• buildFindings (line 239) reads ref.codeExamples || [], but ref is built from MCPParsedDocument, which has no codeExamples field — so it's always undefined and relevantExamples is always []

Empirically confirmed — after a real MCP call that returned 10 documents, findings.relevantExamples.length === 0.

On main, relevantExamples was populated in mock mode via the old processMCPResponse reading ref.codeExamples directly from the mock references. PR #86 (the fuller code-gen PR) also populates it, via an extractCodeExamples(content) method that pulls first lines from markdown code blocks and wires it into ref-building (src/agents/documentation-search-agent.ts on that branch).

Was leaving this out of #90 intentional (deferred so the scope stays minimal and the extraction comes with #86), or an oversight during the cherry-pick? If deferred, worth either deleting codeExamples from mockData to avoid implying mock mode populates it, or leaving a TODO pointing to where the extraction will get wired in.

---

Nit: Legacy MCP types

MCPToolCall (src/types/index.ts:335) and MCPResponse (src/types/index.ts:348) are marked @deprecated but no longer referenced anywhere in src/ or test/ (I grepped). The latest commit title is literally "replace deprecated MCPResponse with MCPParsedDocument". Can these just be deleted, or is there an external consumer to keep them around for?

Nit: Indentation in parseDocumentSection

src/mcp/client.ts:182-194 — the body mixes 2-space and 4-space indentation. The const title / sectionPath / sourceUrl block and the if (!sourceUrl) / contentStart / contentEnd / let content block are at 2-space, while the method signature and the closing return { ... } are at 4-space. Prettier will likely flag this.

---

Summary

• Fix the 4 failing tests (please run npm test locally before pushing)
• Clarify codeExamples / relevantExamples intent (wire up or remove from mocks)
• Confirm whether MCPToolCall / MCPResponse can be removed
• Normalize indentation in parseDocumentSection

Thanks for the iterations — the MCP integration itself is shaping up well.

[imtushar01]

Hey @sugat009, thanks for pointing these out!

I’ve deferred this to keep the scope of this PR minimal. I’ve added a TODO and Note to wire in extractCodeExamples() in #86, since that PR already handles the markdown extraction. However, SonarCloud is currently flagging the TODO.

I’ve also removed MCPToolCall and MCPResponse as they are deprecated, and updated the tests by replacing processMCPResponse with buildFindings.

Please let me know if any further improvements are needed.

Thanks again for the review

[sugat009]

Re-review: PR #90 (commit 401cf64)

All prior items resolved cleanly:

• 4 failing tests: 3 processMCPResponse tests rewritten as targeted buildFindings tests, 1 forms-and-reports topic assertion fixed via keyword swap ('form' → 'forms'). Local run: 143 passing, 0 failing ✓
• relevantExamples deferred to #86: clear Note: on mockKapaAIResponse and TODO(#86): on buildFindings, exactly the deferral path I'd suggested ✓
• Legacy MCPToolCall / MCPResponse: removed ✓
• parseDocumentSection indentation: fixed (lines 182-194 now consistent 4-space) ✓

TypeScript typechecks clean. CI green.

Verified end-to-end with npm run research tickets/contact-search-feature.md on this commit:

• With MCP_SERVER_URL set: hits the configured server, 10 docs back.
• With MCP_SERVER_URL unset: falls back to https://mcp-docs.dev.medicmobile.org/mcp (the new default), 10 docs back.
• Topics output now correctly shows forms (plural) on real data, confirming the keyword swap works in production code, not just tests.

Approving. Thanks for the iteration!