MCP bridge loses upstream isError, logs failures as SUCCESS and caches them #1203

Description

When an upstream MCP tool returns a normal MCP tool-error result (isError: true, rather than throwing a JSON-RPC/HTTP error), the bridge engine preserves it, but DynamicMcpTools treats the entire result envelope as an ordinary API response body.

The client-facing executor result loses the top-level error flag. The upstream error is JSON-encoded inside a text item, the invocation is recorded as SUCCESS, and, when responseMapping.cacheTtl is enabled, that error is cached and served on subsequent calls without retrying the remote tool. An AI client relying on the protocol's error flag sees a successful call.

Steps to Reproduce

At main commit 59c2a06e331aa58584167d17c0692e956a862335 (v0.45.3):

1. Use a registered MCP bridge tool whose upstream returns this valid tool result:

{"content":[{"type":"text","text":"Customer does not exist"}],"isError":true}

2. Invoke it through DynamicMcpTools.executeTool with responseMapping: { cacheTtl: 60 }.
3. Check the returned top-level isError, the invocation's audit status and the Redis write.
4. Invoke the same tool again with the same arguments. The second invocation reads the cached error.

// Run from the repo root. The executor is real; external dependencies are doubles.
const assert = require('node:assert/strict');
const { resolve } = require('node:path');
require('ts-node').register({ project: 'packages/backend/tsconfig.json', transpileOnly: true,
  compilerOptions: { emitDecoratorMetadata: false } });
const metricsPath = require.resolve(resolve('packages/backend/src/common/metrics.ts'));
require.cache[metricsPath] = { id: metricsPath, filename: metricsPath, loaded: true,
  exports: { recordToolCall: () => {} } };
const { DynamicMcpTools } = require(resolve('packages/backend/src/mcp-server/dynamic-mcp-tools.ts'));
const remoteResult = { content: [{ type: 'text', text: 'Customer does not exist' }], isError: true };
const tool = {
  id: 't', connectorId: 'c', organizationId: 'o', name: 'lookup_customer',
  description: 'Reproduction', parameters: { type: 'object', properties: {} },
  connectorType: 'MCP', connectorConfig: { baseUrl: 'https://mcp.example.com/mcp', authType: 'NONE' },
  endpointMapping: { method: 'lookup_customer', path: '/mcp' }, responseMapping: { cacheTtl: 60 },
};
(async () => {
  for (const cloud of [false, true]) for (const cacheTtl of [0, 60]) {
    tool.responseMapping = { cacheTtl };
    let engineCalls = 0;
    const cache = new Map(), auditRows = [];
    const executor = new DynamicMcpTools(
      { getTool: () => tool, getToolForOrg: () => tool },
      { logInvocation: async row => auditRows.push(row) },
      { get: async key => cache.get(key), set: async (key, value) => cache.set(key, value) },
      { checkLicenseActive: async () => {} }, { isCloud: () => cloud }, {}, {}, {}, {},
      { execute: async () => { engineCalls++; return remoteResult; } }, {},
      { scheduleObservationalIngest: async () => {} },
    );
    const context = { organizationId: 'o', connectorIds: ['c'] };
    const first = await executor.executeTool(tool.name, {}, context);
    const second = await executor.executeTool(tool.name, {}, context);
    assert.equal(first.isError, undefined);
    assert.equal(second.isError, undefined);
    assert.equal(JSON.parse(first.content[0].text).isError, true);
    assert.equal(auditRows[0].status, 'SUCCESS');
    assert.equal(cache.size, cacheTtl ? 1 : 0);
    assert.equal(engineCalls, cacheTtl ? 1 : 2);
    console.log(JSON.stringify({ mode: cloud ? 'cloud' : 'self-hosted', cacheTtl,
      outerIsError: first.isError ?? null, innerIsError: JSON.parse(first.content[0].text).isError,
      auditStatus: auditRows[0].status, cacheEntries: cache.size, engineCallsForTwoInvocations: engineCalls }));
  }
})().catch(error => { console.error(error); process.exitCode = 1; });

Expected Behavior

• The client-facing MCP result retains isError: true and the upstream error content.
• The invocation is classified as an error in audit/observability.
• A failed tool result does not enter the normal successful-response cache.

Actual Behavior

The client-facing executor result has the following shape. The error flag is nested inside the serialized body and the executor's internal structured field, instead of being on the returned MCP result:

{
  "content": [{"type":"text","text":"{\n  \"content\": [ ... ],\n  \"isError\": true\n}"}],
  "structured": {"content":[{"type":"text","text":"Customer does not exist"}],"isError":true}
}

Condensed observed output from the reproduction:

{"mode":"self-hosted","cacheTtl":0,"outerIsError":null,"innerIsError":true,"auditStatus":"SUCCESS","cacheEntries":0,"engineCallsForTwoInvocations":2}
{"mode":"self-hosted","cacheTtl":60,"outerIsError":null,"innerIsError":true,"auditStatus":"SUCCESS","cacheEntries":1,"engineCallsForTwoInvocations":1}
{"mode":"cloud","cacheTtl":0,"outerIsError":null,"innerIsError":true,"auditStatus":"SUCCESS","cacheEntries":0,"engineCallsForTwoInvocations":2}
{"mode":"cloud","cacheTtl":60,"outerIsError":null,"innerIsError":true,"auditStatus":"SUCCESS","cacheEntries":1,"engineCallsForTwoInvocations":1}

Here outerIsError: null means the property is absent (the script prints first.isError ?? null). This also happens without caching; enabling caching additionally makes the failure persist until expiry.

Relevant Code

• McpClientEngine.execute returns the upstream CallToolResult intact.
• The MCP dispatch branch wraps that result as body.
• The common success path audits/caches it as success, and renderResult serializes it without lifting the MCP error flag.

This concerns output/error handling, separate from the input-schema flattening work in #961. The fix should be specific to MCP result envelopes: an arbitrary REST response containing a business-data field named isError should not change classification.

Connector Type

MCP Bridge

Deployment Method / Environment

Local development; Windows, Node.js v24.17.0. Source files used in the reproduction are unchanged from the main commit above.

I also exercised the real McpClientEngine against a local HTTP fake MCP server: the engine returned isError: true, and the executor then exhibited the same missing flag, success audit entry and cached error. Audit, Redis, licensing, metrics and KG were doubles; I did not run a full Docker Compose stack or a deployed Cloud instance. The shared executor path was exercised with both Cloud/self-hosted deployment doubles.

Suggested Regression Coverage

• An MCP tool-error result stays an error, with caching disabled and enabled.
• Audit records an error and the successful-response cache receives no write.
• A second invocation can reach the remote tool after a failed first call.
• A REST payload with an isError data field keeps its existing behavior.

Reporting for triage; no assignment requested.