feat(mcp): extract pure utilities and add unit tests

[Vswaroop04]

Summary

• Extracted 8 pure utility functions from the monolithic packages/mcp/src/index.ts into a new src/utils.ts module:

  • isJsonResponse
  • isLicenseError
  • licenseErrorResult
  • resolveInstanceId
  • buildQuery
  • formatProposalText
  • getArgValue
  • parseErrorResponse

• Updated index.ts to import from utils.ts while keeping thin local wrappers around the utilities so all existing call-site signatures remain unchanged and behaviour stays the same.

• Added Vitest to the package along with 40+ unit tests covering:

  • JSON detection edge cases
  • license error parsing
  • instance ID validation
  • query string generation
  • proposal text formatting
  • argument extraction and parsing flows

Why

The existing index.ts file had grown to ~1,260 lines with no automated test coverage. Extracting the stateless logic into a separate module was the smallest safe refactor that made unit testing possible without triggering top-level await side effects during imports.

Test Plan

• cd packages/mcp && pnpm install && pnpm test
• Verify all tests pass
• pnpm build still succeeds

[Vswaroop04]

I have read the CLA Document and I hereby sign the CLA

[KIvanow]

Hi @Vswaroop04 , thank you for your contribution!

Commit 004f6f5 is unsigned - please rebase and sign before re-review.

Three minor things while you're at it:
buildQuery only encodes values, not keys
parts.push(${key}=${encodeURIComponent(String(val))});
Keys are interpolated raw. No practical risk with current callers (all static identifiers), but it's incomplete and the tests don't catch it.

Misleading test description
The 'percent-encodes special characters' test uses FT.SEARCH as its first case - . isn't a reserved URI character and isn't encoded. The actual encoding being tested is a b → a%20b. Worth splitting into two tests or renaming.

resolveInstanceId silently swallows an empty-string override
resolveInstanceId(null, '') evaluates '' || null, falls through to null, and throws "No instance selected" - which is a confusing error when an override was explicitly passed. Since the function is now pure and well-tested, this edge case is easy to cover:
if (overrideId !== undefined && overrideId !== null && overrideId !== '') {
// use overrideId
}
Or at minimum add a test documenting the current behaviour.

[KIvanow]

The first 2 checks are redundant because of the third

[KIvanow]

One small very minor issue with redundant checks

[Vswaroop04]

One small very minor issue with redundant checks

Done