Skip to content

Mobile registry + services + MCP/OAuth, and multi-operation HTTP services - #94

Merged
ddutchie merged 9 commits into
mainfrom
ddutchie/mobiletoolsandmcp
Jul 25, 2026
Merged

ddutchie merged 9 commits into
mainfrom
ddutchie/mobiletoolsandmcp

Conversation

@ddutchie

@ddutchie ddutchie commented Jul 24, 2026 •

Copy link
Copy Markdown
Owner

What does this PR do?

Brings Cairn's community connector registry, HTTP services, and MCP client (with OAuth) to the mobile app so it converges with desktop, and adds multi-operation HTTP services (one connector → many tools, shared base URL/auth) to the shared core consumed by both platforms. Integrations can now ship via the cairn-community registry without an app release.

Type of change

  • New feature
  • Bug fix
  • Refactor / code quality
  • Docs / changelog
  • Tests

Summary of the work (7 commits, staged as tracks)

  • Track 0 — shared JSON-safety (parseToolArgs + external-ref); fixed a silent empty-note write.
  • Track 1 — shared HTTP-service model + registry validation (registry-schema.ts).
  • Track 2 — mobile community registry + HTTP services + device-global tool toggles.
  • Track 2 follow-up — built-in tools made read-only to converge with desktop; Read/Write split. Retired the built-in mobile web tools (web search is now registry-only: Tavily MCP, Brave HTTP service).
  • Track 3 — MCP servers + OAuth on mobile via CIMD (client-id metadata document); friendly "desktop-only" message for loopback/allowlist servers (Canva/Asana/Vercel).
  • Multi-operation HTTP services — shared service-exec gained baseUrl + operations[], {placeholder} path templating, explicit paramLocations (path/query/body), and connector-supplied static query params. Desktop custom_services gained base_url/operations columns (auto-migrated); desktop + mobile both route each operation through the shared resolveOperation + buildOperationRequest. Backward-compatible: legacy single-op services normalize to one operation internally.
  • Open-Meteo weather bug fix — function-calling defaults aren't auto-applied, so the model omitted current and the API returned only metadata. Fixed via static operation query params in the cairn-community connector (shipped separately in that repo, now live).

Screenshots / recording

UI changes are mobile-only (Settings → Tools: expandable per-operation service cards with per-tool switches; MCP server management; tool-run spinner). Screen recording to be attached.

Checklist

  • npm run type-check:all passes
  • npm run lint passes
  • npm test passes (root vitest: 1410 pass / 12 skip)
  • npm run test:e2e passes (no desktop UI change beyond Tools subtitle; will run before release)
  • No hardcoded colours — CSS variables only
  • No text-[Npx] pixel font classes — rem equivalents only
  • New IPC handlers wrapped in handle() and return IpcResult<T> (no new IPC handlers)
  • New DB migrations appended (not edited) in schema.ts (base_url/operations via ensure())
  • New SQL goes in electron/db/queries.ts — single source of truth
  • No new dependencies/devDependencies
  • MCP tools unchanged (no new tools registered)
  • No --external:<pkg> flag changes in the compile script

Notes for reviewer

  • The shared core is the single source of truth for both desktop and mobile — start review in shared/chat/service-exec.ts and shared/chat/registry-schema.ts.
  • The cairn-community registry migration (connectors → operations[], Open-Meteo static-query fix) is a separate repo and is already committed + live at the raw manifest URL.
  • Backward compatibility: verify already-installed single-op services and legacy rows still work (they normalize to one operation).

Summary by CodeRabbit

  • New Features
    • Added mobile Tools & Services settings for managing community services, MCP servers, and individual tool toggles.
    • Added support for multi-operation connectors sharing one connection and API key.
    • Added mobile MCP server discovery, HTTP connections, OAuth sign-in, refresh, and management.
    • Tool activity now shows running status and tappable external links in chat.
  • Bug Fixes
    • Improved recovery from malformed tool arguments without silently losing content.
    • Improved OAuth callback handling and MCP connection error guidance.
    • Added support for clickable links in tool results.

ddutchie added 7 commits July 24, 2026 13:42
… silent empty-note write

Moves the two pure JSON-safety modules from the desktop PR into @cairn/shared so
desktop and mobile share one implementation, and adopts them on mobile.

- shared/chat/parse-tool-args.ts + external-ref.ts (moved from electron/lib;
  electron keeps thin re-export shims so desktop imports are unchanged). Tests
  moved alongside; run in the root vitest 'shared/**' project.
- mobile providers/apple.ts: the on-device tool path did
  JSON.parse(inputJson) catch {} = {} — a malformed ensure_note/patch_note
  (markdown content with raw newlines) SILENTLY ran with {} and wrote an EMPTY
  note. Now uses shared parseToolArgs (strict-first + lossless repair) and, on
  genuine parse failure, returns an error the model can re-issue against instead
  of destroying content. Display parse in the stream uses the same tolerant path.
- mobile chat: web/MCP/service tool results now yield a tappable external-link
  chip. ToolCall gains externalRef; chat/index.tsx extracts it via shared
  extractExternalRef (only when there's no in-app note/card ref); ToolTrail
  renders an https-guarded chip opening via Linking.openURL.

Verify: root 1373 pass/12 skip; type-check:all + lint green; mobile type-check
+ lint green.
Establishes ONE shared definition of a custom HTTP service and the community
registry manifest, so desktop and mobile don't diverge (and mobile Track 2 can
consume the same core). Pure logic only — platform I/O stays per-platform.

- shared/chat/service-exec.ts: pure request-shaping extracted from
  electron/lib/custom-services.ts (namespacing, parseToolDefinition, coerceArgs,
  buildRequest, filterResponse, sampleArgsFromSchema, serviceToOpenAI) + shared
  types CustomServiceRuntimeConfig / BearerResolver / OpenAIToolDef. electron's
  custom-services.ts now re-exports these and keeps ONLY the impure parts
  (callService/testService/withOAuthBearer/fetchWithTimeout — node fetch +
  keychain resolveSecrets).
- shared/chat/registry-schema.ts: manifest TYPES + Zod schemas + parseManifest
  (entry-level fail-soft). electron's community-registry.ts imports + re-exports
  these and keeps only fetch/ETag/fs-cache. zod 4.4.3 on both platforms.
- shared/chat/service-exec.test.ts (10). Existing custom-services.test.ts (28)
  + community-registry.test.ts (9) still pass via the re-exports.
- mobile changelog v0.1.3: silent-empty-note fix + tappable web results (Track 0)
  + under-the-hood sharing note.

Verify: root 1383 pass/12 skip; type-check:all + lint + compile green; mobile
type-check + lint green. Desktop behavior unchanged (pure refactor).
…ol toggles

Mobile can now consume the SAME community connector registry as desktop, install
HTTP services with just an API key (no app update), and toggle which tools the
assistant may use — all device-global (works across every project) and unsynced.

New (mobile/src/chat):
- registry.ts: fetch manifest.json via expo/fetch with a conditional GET (stored
  ETag), validate with the shared parseManifest, cache in the device-global meta
  DB. getCachedManifest/getRegistryServices are fail-soft (corrupt cache → none).
- tool-toggles.ts: device-global enabled map in meta (tools.enabled). Tools
  default ON; only an explicit false disables one. This is the mobile equivalent
  of desktop's disabledTools/ToolAttachment gating, which mobile never had.
- services.ts: installed-service store in meta (services.installed) + per-service
  API key in expo-secure-store (svc.<id>.apiKey). Runs each service through the
  SHARED pure core (serviceToOpenAI/buildRequest/filterResponse) + expo/fetch
  with a 20s timeout. resolveHeaders substitutes <API_KEY> and DROPS an unfilled
  placeholder header (never sends a raw token — matches desktop resolveSecrets).
  installService rejects authMode:oauth (deferred to Track 3).

Wiring:
- tools.ts gains allTools()/allToolMap() = built-in TOOLS + installed services,
  filtered by the toggle map (recomputed per run — install/toggle takes effect on
  the next agent run). toolsForAgent() uses allTools().
- agent.ts resolves allToolMap() ONCE per run so advertised == executable.
- apple.ts runToolToJson resolves via allToolMap() so PCC (32K) executes
  installed services too. (services.ts imports ToolDef as type-only → no cycle.)

Settings UI (app/settings/tools.tsx, linked from AI settings):
- Installed services (toggle + uninstall), Built-in tools (toggle list), and
  Browse-and-install (Alert.prompt for the key; OAuth entries shown but disabled).

Brave consolidation (toward a fully registry-driven web stack):
- web-tools.ts is now Tavily-only; the hardcoded Brave client is removed. Brave
  now installs from the registry as the generic HTTP service, so there's ONE
  Brave definition instead of two. AI-settings web-search simplifies to a single
  Tavily key + a pointer to Tools & Services for Brave; provider pinned to
  'tavily' on save. An existing Brave key is preserved for a later migration.
  (Fully dropping built-in web_search/web_extract waits for Track 3, once the
  mobile MCP client + a registry Tavily connector exist — otherwise mobile would
  lose Tavily search/extract with no replacement.)

Verify: mobile type-check + lint green; root vitest 1383 pass/12 skip. Shared
service-exec/registry-schema unchanged (already tested). Changelog v0.1.3:
corrected the stale Tavily/Brave web-search entry, added Tools & Services, and an
under-the-hood registry note.
…ktop) + Read/Write split

User feedback: (1) the Tools & Services view was large with every built-in tool
shown, and (2) letting mobile disable built-in tools diverged from desktop —
desktop's external-tools.ts gates ONLY external MCP/HTTP tools, never Cairn's
own. Converge with desktop and fix the size in one change.

- tools.ts allTools(): built-in TOOLS are always exposed now; only installed
  services are filtered by the toggle map. Disabling a built-in (e.g.
  get_cairn_context / search_notes) would silently break the assistant, so it's
  no longer possible — matching desktop.
- tools.ts: add WRITE_TOOL_NAMES (the 15 mutating built-ins) for the UI's
  Read/Write grouping.
- tool-toggles.ts: doc clarified — governs installed services only.
- settings/tools.tsx: built-in tools are now a READ-ONLY, collapsed-by-default
  disclosure (chevron + count) split into Read vs Write, no switches. Only
  installed services keep on/off switches — keeps the screen compact and
  informational. Removed the now-unused isToolEnabled import.
- changelog v0.1.3: corrected the entry to reflect built-ins are always-on /
  read-only and match desktop.

Verify: mobile type-check + lint green; root vitest 1383 pass/12 skip.
Mobile can now connect MCP servers from the community registry — including
OAuth ones — and use their tools in chat, with per-server tool management.
Confirmed end-to-end on-device against Tavily (OAuth MCP).

Transport (spiked + confirmed on Hermes):
- Reuse the stock @modelcontextprotocol/sdk Client + StreamableHTTPClientTransport
  with expo/fetch injected (its Response.body is a real ReadableStream, so the
  SDK's TextDecoderStream/pipeThrough SSE pipeline works on RN via Expo's
  web-streams-polyfill + winter runtime). No vendored transport needed.
- mobile/src/chat/mcp-client.ts: connection cache + idle dispose + timeouts;
  listTools/callTool/testConnection. mcp-store.ts: installed servers + a
  device-global cache of discovered tool defs so allTools() stays synchronous.

OAuth (deep-link, no loopback):
- mobile/src/chat/mcp-oauth.ts: SecureStoreOAuthProvider (async expo-secure-store
  storage) drives the SDK's transport-agnostic auth(). Sign-in returns via an
  https bounce page (gerardbuilds.com/cairn/oauth) that forwards to the app's
  cairn:// deep link, opened with expo-web-browser openAuthSessionAsync — a phone
  can't run desktop's 127.0.0.1 loopback listener.
- Client registration uses CIMD (SEP-991): our client_id is a static metadata
  JSON URL, so no per-provider registration/secret. SDK falls back to DCR where
  CIMD is unsupported. crypto-polyfill.ts shims crypto.subtle/getRandomValues
  (via expo-crypto) so the SDK's PKCE (pkce-challenge) works on Hermes.
- parseOAuthCallback fix: state is optional (Tavily returns empty state); code is
  the only required param. Providers that require a pre-registered/allow-listed
  redirect (e.g. Canva) can't be connected from mobile yet — the UI notes this.

Shared (desktop re-exports; +12 tests):
- shared/chat/mcp-namespace.ts: namespace/parse/isMcp/mcpToolsToOpenAI/
  stringifyToolResult moved out of electron/lib/mcp-client.
- shared/chat/oauth-callback.ts: parseOAuthCallback + cairn:// constants moved
  out of electron/lib/mcp-oauth.

Tools wiring + UI:
- tools.ts allTools()/allToolMap() fold in cached MCP tool defs (toggle-gated);
  mcp__ calls route to mcp-client.callTool. Built-ins stay always-on.
- settings/tools.tsx: MCP servers section — install/Connect (OAuth), sign in/out,
  uninstall, and an expandable per-server tool manager (per-tool switches +
  re-discover). Connector brand logos via new ConnectorLogo (react-native-svg
  SvgXml + desktop's looksSafeSvg guard); fixed built-in tools list spacing.
- Chat shows a running spinner chip while a tool executes: agent.ts emits a
  tool-start event before tool.run(); the chip finalizes in place on result.

Retired built-in web tools: deleted web-tools.ts + web-config.ts and the
web_search/web_extract built-ins — web search is now a registry connector
(Tavily via MCP, Brave via HTTP service). AI-settings Web-search section removed;
its nav points to Tools & Services.

Deps (mobile): @modelcontextprotocol/sdk, expo-web-browser (+ config plugin),
expo-auth-session, expo-crypto.

Verify: mobile type-check + lint green; desktop type-check:all + compile green;
root vitest 1403 pass / 12 skip. Changelog v0.1.3 updated.
Some providers only accept an OAuth sign-in from a desktop/loopback client and
reject the mobile redirect at registration (confirmed: Asana, Vercel — their
/register only allows http://127.0.0.1, not our https bounce or cairn://). The
SDK surfaced this as a raw, scary error (e.g. 'Invalid OAuth error response:
SyntaxError: JSON Parse error' from the provider's non-JSON 400 body).

- mcp-oauth.ts: friendlyAuthError() classifies redirect/registration rejections
  as desktopOnly and returns human copy; AuthStartResult gains an optional
  desktopOnly flag. Generic failures get a clean 'try again / use desktop'
  message instead of the raw SDK text.
- settings/tools.tsx: titles the alert 'Connect on desktop' for desktopOnly
  failures, 'Sign-in failed' otherwise.
- User cancels still return 'cancelled' (no alert); network errors fall to the
  generic branch (not mislabeled desktop-only).

Verify: mobile type-check + lint green; oauth tests unaffected (31 pass).
Changelog v0.1.3 updated.
One community service connector can now expose SEVERAL tools sharing a base URL
+ headers/auth, instead of one endpoint per connector (each re-keyed). Consumed
identically by desktop and mobile via the shared @cairn/shared core.

Shared (@cairn/shared):
- service-exec.ts: ServiceOperation + ResolvedOperation; normalizeOperations
  (legacy single-op → one op), serviceOperationsToOpenAI (one tool per op),
  resolveOperation, buildOperationRequest — path {placeholder} templating,
  per-arg paramLocations (path/query/body), and static per-op  params
  (connector-supplied; model args override). buildRequest kept as a
  backward-compatible single-op wrapper. +17 tests.
- registry-schema.ts: service definition gained baseUrl + operations[] (with
  query), legacy fields optional, refine requires legacy-trio OR baseUrl+ops.

Desktop:
- CustomServiceConfig + RegistryServiceDefinition + ServiceOperationConfig
  (src/types) gained baseUrl/operations; apiUrl/method/toolDefinition optional.
- custom_services table: base_url + operations columns (auto-migrated via
  ensure()); saveCustomService + toCustomService round-trip them.
- external-tools.ts lists one tool per operation (serviceOperationsToOpenAI) and
  routes calls through the shared config; custom-services.ts callService/
  testService are operation-aware (resolveOperation + buildOperationRequest).
- Community install (store slice) copies baseUrl/operations; Settings shows a
  multi-op subtitle; BrowseCommunity uses baseUrl ?? apiUrl.

Mobile:
- services.ts: serviceToolDefs emits one ToolDef per operation; runService
  routes via resolveOperation + buildOperationRequest; uninstall clears every
  op's toggle. settings/tools.tsx: installed services render as an expandable
  per-operation tool list with per-tool switches (mirrors MCP servers).

Backward-compatible: existing single-op services + already-installed rows keep
working (normalized to one operation internally).

Verify: root vitest 1410 pass / 12 skip; type-check:all + lint + compile green;
mobile type-check + lint green. cairn-community migrated + manifest live
(commit 2b65854). Changelogs: desktop v2.5.8, mobile v0.1.3.
@coderabbitai

coderabbitai Bot commented Jul 24, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@ddutchie, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 33 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8a316748-c61f-4451-b1ae-709c1cd8f68d

📥 Commits

Reviewing files that changed from the base of the PR and between 7bfdbe8 and 66547fb.

📒 Files selected for processing (11)
  • changelogs/v2.5.8.md
  • electron/lib/custom-services.ts
  • mobile/app/settings/tools.tsx
  • mobile/changelogs/v0.1.3.md
  • mobile/src/chat/mcp-client.ts
  • mobile/src/chat/registry.ts
  • mobile/src/chat/services.ts
  • mobile/src/components/AiSettingsForm.tsx
  • shared/chat/registry-schema.test.ts
  • shared/chat/registry-schema.ts
  • shared/chat/service-exec.ts
📝 Walkthrough

Walkthrough

This change adds shared multi-operation HTTP connector support, desktop persistence, mobile registry-based services, MCP connectivity, OAuth handling, dynamic tool aggregation, tolerant argument parsing, and richer tool-result presentation.

Changes

Connector execution and contracts

Layer / File(s) Summary
Shared schemas and request execution
shared/chat/*, src/types/index.ts
Defines multi-operation service schemas, manifest validation, operation resolution, path/query/body request construction, response filtering, argument parsing, MCP utilities, and external-reference extraction with tests.
Desktop persistence and wiring
electron/db/*, electron/lib/*, electron/shared/db-mappers.ts
Persists base_url and operations, normalizes legacy services, routes execution through shared helpers, and re-exports shared registry and utility implementations.

Mobile tools and connectivity

Layer / File(s) Summary
Mobile registry and runtimes
mobile/src/chat/registry.ts, mobile/src/chat/services.ts, mobile/src/chat/mcp-*
Adds cached registry loading, device-local service installation, API-key storage, MCP discovery and connection caching, OAuth flows, token storage, and PKCE crypto support.
Tool aggregation and settings
mobile/src/chat/tools.ts, mobile/app/settings/tools.tsx, mobile/src/components/*
Aggregates enabled built-in, service, and MCP tools; adds the Tools & Services screen; supports per-operation toggles, server management, connector logos, and navigation.
Chat execution and presentation
mobile/src/chat/agent.ts, mobile/src/chat/providers/apple.ts, mobile/app/(tabs)/chat/index.tsx, mobile/src/components/chat/ToolTrail.tsx
Correlates tool-start and completion events, reports malformed arguments, shows running chips, and opens extracted HTTP references.
Release and compatibility updates
changelogs/*, mobile/package.json, mobile/app.json, mobile/eslint.config.js
Documents the connector and MCP changes and adds the mobile packages, Expo plugin, and resolver configuration required by the new runtime.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 72.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the PR’s main work: mobile registry/services, MCP/OAuth, and multi-operation HTTP services.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ddutchie/mobiletoolsandmcp

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
electron/lib/custom-services.ts (1)

67-94: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

resolveOperation call sits outside the try block — malformed operation JSON becomes an unhandled rejection, not a graceful Error: ... string.

resolveOperation (and the normalizeOperations/parseToolDefinition it calls internally for every operation of the service, not just the target one) throws on invalid JSON by design. Because that call is made before the try at line 77, a corrupted toolDefinition on any operation of a multi-op service turns callService's returned promise into a rejection instead of the resolved "Error: ..." string every other failure path here produces — breaking the consistent error-surfacing contract relied on by the model/chat loop.

Also, the !res.ok branch on line 84 reports op.url, which per ResolvedOperation's own doc comment still has "placeholders... present" — it should report the actual filled url built on line 80, not the unfilled template.

🐛 Proposed fix
 export async function callService(
   cfg: CustomServiceRuntimeConfig,
   namespaced: string,
   args: Record<string, unknown>,
   resolveBearer?: BearerResolver,
 ): Promise<string> {
-  const op = resolveOperation(cfg, namespaced);
-  if (!op) {
-    return `Error: "${namespaced}" is not a tool of service ${cfg.id}`;
-  }
   try {
+    const op = resolveOperation(cfg, namespaced);
+    if (!op) {
+      return `Error: "${namespaced}" is not a tool of service ${cfg.id}`;
+    }
     const headers = await withOAuthBearer(cfg, resolveSecrets(cfg.headers ?? {}), resolveBearer);
     const { parameters } = parseToolDefinition(op.toolDefinition);
     const { url, init } = buildOperationRequest(op, args, headers, parameters);
     const res = await fetchWithTimeout(url, init, CALL_TIMEOUT_MS);
     const text = await res.text();
     if (!res.ok) {
-      return `Error: ${op.method} ${op.url} returned ${res.status} ${res.statusText}\n${text.slice(0, 1000)}`;
+      return `Error: ${op.method} ${url} returned ${res.status} ${res.statusText}\n${text.slice(0, 1000)}`;
     }
🤖 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 `@electron/lib/custom-services.ts` around lines 67 - 94, Move the
resolveOperation call inside callService’s existing try block so malformed
operation definitions from any service operation are converted into the
established Error string and logged rather than rejected. In the !res.ok branch,
report the resolved request url returned by buildOperationRequest instead of
op.url, while preserving the existing status and response-body details.
🧹 Nitpick comments (7)
mobile/app/settings/tools.tsx (1)

141-166: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Hoist oauthCfg out of the component so onConnect's dep array is honest.

oauthCfg is re-created every render and captured by onConnect without appearing in its deps — harmless today because it is pure over s, but it will trip react-hooks/exhaustive-deps and invites a real stale closure later.

♻️ Proposed refactor
-  const oauthCfg = (s: InstalledMcpServer): OAuthServerConfig => ({
-    id: s.id,
-    serverUrl: s.baseUrl,
-    scope: s.oauthScope,
-  });

Move to module scope, above the component:

const oauthCfg = (s: InstalledMcpServer): OAuthServerConfig => ({
  id: s.id,
  serverUrl: s.baseUrl,
  scope: s.oauthScope,
});
🤖 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 `@mobile/app/settings/tools.tsx` around lines 141 - 166, Move the pure oauthCfg
helper to module scope above the component, then keep onConnect focused on
invoking it without capturing a render-scoped function. Preserve the existing
OAuthServerConfig mapping and ensure the useCallback dependency array remains
accurate.
mobile/src/components/ConnectorLogo.tsx (1)

26-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move looksSafeSvg into shared rather than re-implementing the desktop guard.

This PR consolidates the namespacing, external-ref and registry-validation logic into shared/chat/*; this SVG sanitiser is the same class of security-relevant pure logic and is now maintained twice. A tightened rule on desktop won't reach mobile.

Rest of the component reads well — sizing memoised, deterministic fallback glyphs.

🤖 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 `@mobile/src/components/ConnectorLogo.tsx` around lines 26 - 38, Move the
looksSafeSvg sanitizer from ConnectorLogo.tsx into the shared chat utilities
alongside the existing SVG safety logic, then import and reuse that shared
implementation in the mobile component. Remove the local duplicate while
preserving its current fallback behavior and validation rules.
shared/chat/registry-schema.ts (2)

49-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The multi-operation connector contract is declared three times instead of derived once. The Zod schemas in shared/chat/registry-schema.ts are the only real validator, yet the same field list is hand-maintained as interfaces there and again in src/types/index.ts, with parseManifest's as CommunityManifest cast hiding any divergence.

  • shared/chat/registry-schema.ts#L49-L59: replace the hand-written RegistryServiceOperation (and RegistryServiceEntry["definition"]) with z.infer<typeof serviceOperation> / z.infer<typeof serviceDefinition>, moving the schema declarations above the type exports so the cast in parseManifest becomes unnecessary.
  • src/types/index.ts#L170-L180: drop the local ServiceOperationConfig body and re-export the shared operation type; apply the same treatment to RegistryServiceDefinition at Lines 268-293, which mirrors the shared service definition.
🤖 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 `@shared/chat/registry-schema.ts` around lines 49 - 59, Derive the
multi-operation connector types from the Zod schemas instead of maintaining
duplicate field lists: in shared/chat/registry-schema.ts, move serviceOperation
and serviceDefinition schema declarations before the exports and define
RegistryServiceOperation and RegistryServiceEntry["definition"] with z.infer,
then remove the unnecessary as CommunityManifest cast in parseManifest. In
src/types/index.ts lines 170-180 and 268-293, remove the local
ServiceOperationConfig and RegistryServiceDefinition bodies and re-export the
corresponding shared inferred types.

98-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Zod 4 legacy APIs: z.string().url(), .passthrough(), and { message }.

All three still work but are deprecated in Zod 4 and slated for removal in the next major. The method equivalents (z.string().email(), etc.) are still available but have been deprecated. They'll be removed in the next major version. ZodObject.passthrough() is deprecated — "Use z.looseObject() or .loose() instead." The message parameter is still supported but deprecated in favour of error.

♻️ Modernised forms
-  baseUrl: z.string().url().startsWith("https://"),
+  baseUrl: z.url().startsWith("https://"),
-const mcpEntry = z.object({ ...entryMeta, definition: mcpDefinition }).passthrough();
-const serviceEntry = z.object({ ...entryMeta, definition: serviceDefinition }).passthrough();
+const mcpEntry = z.looseObject({ ...entryMeta, definition: mcpDefinition });
+const serviceEntry = z.looseObject({ ...entryMeta, definition: serviceDefinition });
-    { message: "service must define either baseUrl+operations or apiUrl+method+toolDefinition" }
+    { error: "service must define either baseUrl+operations or apiUrl+method+toolDefinition" }

(the same z.string().url() pattern appears at Lines 108, 111-112, 135, 140, 143, 168)

Also applies to: 174-175

🤖 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 `@shared/chat/registry-schema.ts` at line 98, Modernize the Zod 4 schema
definitions in the registry schema: replace deprecated string URL validation
methods such as those used by baseUrl and the other URL fields with the
supported validator form, replace any ZodObject.passthrough() usage with
z.looseObject() or .loose(), and change deprecated { message } validation
options to { error }. Apply these updates consistently to all matching fields
and options in the schema.
shared/chat/service-exec.ts (2)

193-220: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

ResolvedOperation.toolName can silently diverge from the name actually used for namespacing/resolution.

normalizeOperations sets toolName: op.name (line 211), but serviceOperationsToOpenAI (line 229) and resolveOperation (line 248) both derive the real tool name from parseToolDefinition(op.toolDefinition).name, never from op.name. If a connector author ever sets a ServiceOperation.name that doesn't match the name embedded in its own toolDefinition JSON, the two names silently disagree — the toolName field on ResolvedOperation (used for display/tests) would no longer reflect what's exposed to the model or what resolveOperation actually matches on.

Consider deriving toolName from parseToolDefinition(op.toolDefinition).name for consistency, or asserting the two stay in sync.

🤖 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 `@shared/chat/service-exec.ts` around lines 193 - 220, The toolName assigned in
normalizeOperations can differ from the name used by serviceOperationsToOpenAI
and resolveOperation. Derive ResolvedOperation.toolName from
parseToolDefinition(op.toolDefinition).name, or validate that it matches op.name
and enforce the existing name consistently across all three flows.

31-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated namespace-parsing algorithm between service-exec.ts and mcp-namespace.ts. Both parseServiceToolName and parseToolName implement the identical prefix-check + first-separator-scan + bounds-check algorithm, differing only in the prefix constant and returned field names (serviceId vs serverId). Since both modules are explicitly pure/framework-free and shared across desktop and mobile, this is a good candidate to extract into one shared parametrized helper so future fixes (e.g. to separator-collision handling) don't need to be applied twice.

  • shared/chat/service-exec.ts#L31-L39: replace parseServiceToolName's body with a call into a shared parseNamespacedName(NS_PREFIX, NS_SEP, namespaced) helper (returning { id, toolName }, aliased to serviceId here).
  • shared/chat/mcp-namespace.ts#L25-L34: replace parseToolName's body with the same shared helper, aliasing its id result to serverId.
🤖 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 `@shared/chat/service-exec.ts` around lines 31 - 39, Extract the duplicated
prefix/separator parsing logic into a shared pure helper named
parseNamespacedName that accepts the prefix, separator, and namespaced value and
returns { id, toolName } or null. In shared/chat/service-exec.ts lines 31-39,
update parseServiceToolName to delegate to it and map id to serviceId; in
shared/chat/mcp-namespace.ts lines 25-34, update parseToolName to delegate to it
and map id to serverId, preserving existing validation behavior.
mobile/src/components/AiSettingsForm.tsx (1)

419-432: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add accessibility metadata to the new "Tools & Services" nav row.

This Pressable lacks accessibilityRole/accessibilityLabel, unlike the equivalent tappable rows elsewhere (e.g. ToolTrail.tsx's chips set accessibilityRole="button" + accessibilityLabel). Screen readers won't announce it as an actionable navigation item.

♿ Proposed fix
             <Pressable
               style={styles.navRow}
+              accessibilityRole="button"
+              accessibilityLabel="Tools & Services"
               onPress={() => {
                 haptics.selection();
                 router.push("/settings/tools");
               }}
             >
🤖 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 `@mobile/src/components/AiSettingsForm.tsx` around lines 419 - 432, Add
accessibilityRole="button" and a descriptive accessibilityLabel to the Tools &
Services Pressable in the AI settings form, matching the metadata used by
equivalent tappable navigation rows such as ToolTrail chips.
🤖 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 `@mobile/app/settings/tools.tsx`:
- Line 359: Add descriptive accessibilityLabel values to the icon-only
Pressables in the settings tools UI: the uninstall handlers onUninstall, the
per-server refresh control, and the disclosure controls near those sections.
Match the existing toolbar accessibility-label pattern, using labels that
clearly identify remove/uninstall, refresh tools, and expand/collapse actions.
- Around line 262-272: Update the API-key prompt flow around Alert.prompt so it
does not invoke the iOS-only API when Platform.OS is Android. Add a
cross-platform text-input modal or an explicit Android fallback that collects
the key and then calls finish/installService, while preserving the existing iOS
prompt behavior.
- Around line 108-125: Update the loading-state initialization used by the
screen so it starts as not loading when cached catalog data is present in
services, mcpEntries, or installed, while retaining loading for an empty cache.
Keep loadRegistry(false) as a background refresh and preserve its finally
cleanup for active requests.

In `@mobile/src/chat/mcp-client.ts`:
- Around line 97-114: Update connect so failures from client.connect(transport)
or withTimeout are caught and the newly created client/transport is closed
before rethrowing the original error. Keep caching and touch behavior unchanged
for successful connections, and ensure cleanup also covers timeout failures.

In `@mobile/src/chat/registry.ts`:
- Around line 52-55: Update mobile/src/chat/registry.ts at lines 52-55 to pass
an AbortController signal to expoFetch, enforce the existing FETCH_TIMEOUT_MS
deadline, and clear the timer in finally while preserving cached-manifest
fallback with a soft error. Update mobile/app/settings/tools.tsx at lines
108-125 to initialize loading from getCachedManifest() === null, allowing cached
tools to render immediately while the fetch refreshes them.

In `@mobile/src/chat/services.ts`:
- Around line 176-189: Move the parseToolDefinition call and parameter
extraction inside runService’s existing try/catch so malformed toolDefinition
values are handled by the function’s standard { error } response path. Preserve
the current successful request flow and never-throws contract.

In `@shared/chat/registry-schema.ts`:
- Around line 130-156: Update the serviceOperation schema used by
serviceDefinition to validate each operations[].path as a proper relative path
before it is joined with baseUrl: reject absolute URLs, scheme-relative values,
and paths that can escape the pinned origin, while preserving valid
connector-relative paths. Ensure malformed paths fail schema parsing before URL
construction and credential/header attachment.

---

Outside diff comments:
In `@electron/lib/custom-services.ts`:
- Around line 67-94: Move the resolveOperation call inside callService’s
existing try block so malformed operation definitions from any service operation
are converted into the established Error string and logged rather than rejected.
In the !res.ok branch, report the resolved request url returned by
buildOperationRequest instead of op.url, while preserving the existing status
and response-body details.

---

Nitpick comments:
In `@mobile/app/settings/tools.tsx`:
- Around line 141-166: Move the pure oauthCfg helper to module scope above the
component, then keep onConnect focused on invoking it without capturing a
render-scoped function. Preserve the existing OAuthServerConfig mapping and
ensure the useCallback dependency array remains accurate.

In `@mobile/src/components/AiSettingsForm.tsx`:
- Around line 419-432: Add accessibilityRole="button" and a descriptive
accessibilityLabel to the Tools & Services Pressable in the AI settings form,
matching the metadata used by equivalent tappable navigation rows such as
ToolTrail chips.

In `@mobile/src/components/ConnectorLogo.tsx`:
- Around line 26-38: Move the looksSafeSvg sanitizer from ConnectorLogo.tsx into
the shared chat utilities alongside the existing SVG safety logic, then import
and reuse that shared implementation in the mobile component. Remove the local
duplicate while preserving its current fallback behavior and validation rules.

In `@shared/chat/registry-schema.ts`:
- Around line 49-59: Derive the multi-operation connector types from the Zod
schemas instead of maintaining duplicate field lists: in
shared/chat/registry-schema.ts, move serviceOperation and serviceDefinition
schema declarations before the exports and define RegistryServiceOperation and
RegistryServiceEntry["definition"] with z.infer, then remove the unnecessary as
CommunityManifest cast in parseManifest. In src/types/index.ts lines 170-180 and
268-293, remove the local ServiceOperationConfig and RegistryServiceDefinition
bodies and re-export the corresponding shared inferred types.
- Line 98: Modernize the Zod 4 schema definitions in the registry schema:
replace deprecated string URL validation methods such as those used by baseUrl
and the other URL fields with the supported validator form, replace any
ZodObject.passthrough() usage with z.looseObject() or .loose(), and change
deprecated { message } validation options to { error }. Apply these updates
consistently to all matching fields and options in the schema.

In `@shared/chat/service-exec.ts`:
- Around line 193-220: The toolName assigned in normalizeOperations can differ
from the name used by serviceOperationsToOpenAI and resolveOperation. Derive
ResolvedOperation.toolName from parseToolDefinition(op.toolDefinition).name, or
validate that it matches op.name and enforce the existing name consistently
across all three flows.
- Around line 31-39: Extract the duplicated prefix/separator parsing logic into
a shared pure helper named parseNamespacedName that accepts the prefix,
separator, and namespaced value and returns { id, toolName } or null. In
shared/chat/service-exec.ts lines 31-39, update parseServiceToolName to delegate
to it and map id to serviceId; in shared/chat/mcp-namespace.ts lines 25-34,
update parseToolName to delegate to it and map id to serverId, preserving
existing validation behavior.
🪄 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 Plus

Run ID: a2ffcee3-9908-4f08-b30f-26390e449401

📥 Commits

Reviewing files that changed from the base of the PR and between f551d68 and 7bfdbe8.

⛔ Files ignored due to path filters (1)
  • mobile/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (52)
  • changelogs/v2.5.8.md
  • electron/db/queries.ts
  • electron/db/schema.ts
  • electron/lib/community-registry.ts
  • electron/lib/custom-services.test.ts
  • electron/lib/custom-services.ts
  • electron/lib/external-ref.ts
  • electron/lib/external-tools.ts
  • electron/lib/mcp-client.ts
  • electron/lib/mcp-oauth.test.ts
  • electron/lib/mcp-oauth.ts
  • electron/lib/parse-tool-args.ts
  • electron/shared/db-mappers.ts
  • mobile/app.json
  • mobile/app/(tabs)/chat/index.tsx
  • mobile/app/_layout.tsx
  • mobile/app/settings/tools.tsx
  • mobile/changelogs/v0.1.3.md
  • mobile/eslint.config.js
  • mobile/package.json
  • mobile/src/chat/agent.ts
  • mobile/src/chat/crypto-polyfill.ts
  • mobile/src/chat/mcp-client.ts
  • mobile/src/chat/mcp-oauth.ts
  • mobile/src/chat/mcp-store.ts
  • mobile/src/chat/providers/apple.ts
  • mobile/src/chat/registry.ts
  • mobile/src/chat/services.ts
  • mobile/src/chat/tool-toggles.ts
  • mobile/src/chat/tools.ts
  • mobile/src/chat/web-config.ts
  • mobile/src/chat/web-tools.ts
  • mobile/src/components/AiSettingsForm.tsx
  • mobile/src/components/ConnectorLogo.tsx
  • mobile/src/components/ai-settings/styles.ts
  • mobile/src/components/chat/ToolTrail.tsx
  • mobile/src/db/chat-store.ts
  • shared/chat/external-ref.test.ts
  • shared/chat/external-ref.ts
  • shared/chat/mcp-namespace.test.ts
  • shared/chat/mcp-namespace.ts
  • shared/chat/oauth-callback.test.ts
  • shared/chat/oauth-callback.ts
  • shared/chat/parse-tool-args.test.ts
  • shared/chat/parse-tool-args.ts
  • shared/chat/registry-schema.ts
  • shared/chat/service-exec.test.ts
  • shared/chat/service-exec.ts
  • src/components/settings/ToolsSettings.tsx
  • src/components/settings/tools/BrowseCommunityModal.tsx
  • src/store/slices/tools.ts
  • src/types/index.ts
💤 Files with no reviewable changes (2)
  • mobile/src/chat/web-config.ts
  • mobile/src/chat/web-tools.ts

Comment thread mobile/app/settings/tools.tsx
Comment thread mobile/app/settings/tools.tsx Outdated
Comment thread mobile/app/settings/tools.tsx Outdated
Comment thread mobile/src/chat/mcp-client.ts
Comment thread mobile/src/chat/registry.ts
Comment thread mobile/src/chat/services.ts
Comment thread shared/chat/registry-schema.ts
ddutchie added 2 commits July 24, 2026 18:32
Ports desktop's Browse Community filtering to the mobile Tools & Services
screen. Adds a 'Browse community' block above the two Add lists:

- A search box (reuses the shared SearchField) matching connector name, blurb,
  category, and freeform tags — same fields as desktop's BrowseCommunityModal.
- Horizontally-scrolling category chips ('All' + the categories present across
  everything still installable), single-select, toggle-off on re-tap.

Both filters apply to the installable services AND MCP servers at once, so the
whole catalog narrows together. Category chip set is the union of the two kinds
and shrinks as you install things. Distinct empty states: 'installed everything'
vs 'no matches for your search'. The refresh control moved into the new block's
header; the Add sections are relabelled Services / MCP servers.

Verify: mobile type-check + lint clean; expo export (ios) bundles.
Changelog: mobile v0.1.3.
Verified each finding against current code; fixed the still-valid ones,
skipped the rest (reasons below). All changes minimal + validated.

Security / correctness:
- registry-schema: reject service-operation paths that carry a scheme/host
  ('https://…', '//host', embedded '://') so a connector can't redirect a
  request off its pinned baseUrl origin. New registry-schema.test.ts (+5).
- service-exec: derive ResolvedOperation.toolName from the toolDefinition name
  (falling back to op.name) so the identifier is consistent across tool listing,
  resolveOperation, and execution — previously toolName=op.name could diverge
  from the name the model actually calls.
- mcp-client (mobile): on a failed/timed-out connect, close the half-open
  client + transport before rethrowing so the socket + refresh timers don't leak.
- services (mobile) + custom-services (desktop): move parseToolDefinition /
  resolveOperation inside the try so a malformed operation/tool definition
  becomes the standard { error } / error-string path instead of throwing into
  the tool loop. custom-services error now reports the resolved request url.

Mobile UX:
- registry: AbortController + 20s FETCH_TIMEOUT_MS on the manifest fetch, timer
  cleared in finally, cached-manifest fallback preserved (soft error).
- tools screen: initialise loading from getCachedManifest()===null so a cached
  catalog renders instantly while it refreshes in the background.
- tools screen: Android has no Alert.prompt — add an in-app Modal key prompt for
  key-based installs (iOS keeps the native prompt).
- accessibility labels on the icon-only controls (uninstall service/server,
  connect/sign-out, expand/collapse tools, refresh server, refresh catalog,
  built-ins disclosure) + the AiSettingsForm Tools nav row.
- hoist the pure oauthCfg helper to module scope.

Skipped (with reason):
- Move ConnectorLogo.looksSafeSvg into shared: no existing shared SVG-safety
  module (the finding's premise); creating shared/chat SVG code for an 8-line
  RN-render helper is scope-creep vs 'keep minimal'.
- Extract parseNamespacedName shared helper: two 5-line parsers in separate
  modules with different NS constants; indirection > benefit here.
- Zod-4 modernization (.url()/.passthrough()/{message}): deprecated-but-working;
  a broad schema sweep risks behaviour changes and is out of scope for minimal.
- Derive registry types from Zod (+ remove src/types bodies): large refactor for
  a nitpick; skipped as scope-creep.

Verify: type-check:all + root vitest (1415 pass / 12 skip) + compile green;
mobile type-check + lint + expo export (ios) green. Changelogs: desktop v2.5.8,
mobile v0.1.3.
@ddutchie
ddutchie merged commit 13568a7 into main Jul 25, 2026
6 checks passed
@ddutchie
ddutchie deleted the ddutchie/mobiletoolsandmcp branch July 25, 2026 12:16
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.

1 participant