Skip to content

Commit 558d96d

Browse files
authored
fix(search): return actionable live read errors and drop unreadable Confluence matches (#8888)
## Summary - `read_document` turned every provider-side read failure into the generic "Knowledge operation failed", because `NativeSearchError` escaped `readLiveDocument` without being classified. Reads now return errors the model can act on: - a 404 or 410 becomes `not_found`, with a hint to search again or read a different result; - a revoked grant becomes `unauthorized`, naming the provider to reconnect; - rate limits, 5xx responses, provider timeouts, MCP request timeouts and the 15 s read deadline become a retryable `LiveReadError`. - The Assistant `read_document` tool now reports `retryable` (and `retryAfterSeconds` when set), the same way `search_workspace` does. The Search MCP `read_document` returns the classified message instead of the generic text. - Confluence search labeled every non-blogpost CQL hit a `page`. Native CQL that matched attachments, comments, whiteboards, folders or databases therefore produced references whose v2 page read returns 404. Those kinds are now dropped, and the result message says so. ## Test plan - [x] `application.test.ts`: covers 404, reconnect, rate limit, 503 and MCP timeout. All five fail with the fix reverted. - [x] `atlassian.test.ts`: covers native CQL matches that a page read can't open. It fails with the fix reverted. - [x] Focused suites pass locally: application, atlassian, policy, workspace-search, mcp server. - [ ] CI
1 parent 3492598 commit 558d96d

8 files changed

Lines changed: 206 additions & 10 deletions

File tree

‎apps/sim/lib/knowledge/mcp/server.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
import { liveCitationId } from '@/lib/knowledge/search/citation'
2727
import { toolError } from '@/lib/mcp/tool-result'
2828
import { readLiveDocument, searchLiveKnowledge } from '@/lib/sim-search/live/application'
29+
import { LiveReadError } from '@/lib/sim-search/live/read-error'
2930
import { v2CaughtOrchestrationError } from '@/app/api/v2/lib/response'
3031
import { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
3132

@@ -67,6 +68,7 @@ export function createKnowledgeMcpServer(context: KnowledgeMcpContext): McpServe
6768
return result
6869
} catch (error) {
6970
if (signal.aborted) outcome = 'cancelled'
71+
if (error instanceof LiveReadError) return toolError(error.message)
7072
const response = v2CaughtOrchestrationError(error)
7173
if (response) {
7274
const body: unknown = await response.json()

‎apps/sim/lib/mothership/tools/server/knowledge/workspace-search.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
import type { BaseServerTool, ServerToolContext } from '@/lib/mothership/tools/server/base-tool'
2020
import { connectorDisplayName } from '@/lib/sim-search/connectors'
2121
import { readLiveDocument, searchLiveKnowledge } from '@/lib/sim-search/live/application'
22+
import { LiveReadError } from '@/lib/sim-search/live/read-error'
2223
import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection'
2324

2425
const logger = createLogger('WorkspaceSearchTool')
@@ -165,8 +166,16 @@ export const readDocumentServerTool: BaseServerTool = {
165166
}
166167
} catch (error) {
167168
logger.error('Document read failed', { error })
169+
if (error instanceof LiveReadError)
170+
return {
171+
success: false,
172+
retryable: error.retryable,
173+
...(error.retryAfterSeconds ? { retryAfterSeconds: error.retryAfterSeconds } : {}),
174+
message: error.message,
175+
}
168176
return {
169177
success: false,
178+
retryable: false,
170179
message:
171180
error instanceof z.ZodError
172181
? 'Invalid document arguments'

‎apps/sim/lib/sim-search/live/application.test.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'
12
import { dbChainMockFns, resetDbChainMock } from '@sim/testing'
23
import { createSessionPrincipal } from '@sim/testing/factories/principal.factory'
34
import { setEnv } from '@sim/testing/mocks/env.mock'
@@ -769,6 +770,62 @@ describe('authorized live retrieval', () => {
769770
).rejects.toThrow('Revoked')
770771
expect(mocks.read).not.toHaveBeenCalled()
771772
})
773+
it.each([
774+
[
775+
'a missing or non-readable document',
776+
new NativeSearchError('unavailable', 'Provider request failed (404).', undefined, 404),
777+
{ code: 'not_found', message: expect.stringContaining('Search again') },
778+
],
779+
[
780+
'a revoked grant',
781+
new NativeSearchError('reconnect', 'The provider denied access.'),
782+
{ code: 'unauthorized', message: expect.stringContaining('Reconnect Google Drive') },
783+
],
784+
[
785+
'a provider rate limit',
786+
new NativeSearchError('rate_limited', 'Provider rate limit reached.', 30),
787+
{ retryable: true, retryAfterSeconds: 30 },
788+
],
789+
[
790+
'a provider outage',
791+
new NativeSearchError('unavailable', 'Provider request failed (503).', undefined, 503),
792+
{ retryable: true, message: expect.stringContaining('Try again') },
793+
],
794+
[
795+
'an MCP request timeout',
796+
new McpError(ErrorCode.RequestTimeout, 'TimeoutError'),
797+
{ retryable: true, message: expect.stringContaining('took too long') },
798+
],
799+
[
800+
'a provider request timeout response',
801+
new NativeSearchError('unavailable', 'Provider request failed (408).', undefined, 408),
802+
{ retryable: true, message: expect.stringContaining('timed out') },
803+
],
804+
[
805+
'a native request socket timeout',
806+
Object.assign(new Error('Request timed out after 10000ms'), { code: 'ETIMEDOUT' }),
807+
{ retryable: true, message: expect.stringContaining('Try again') },
808+
],
809+
[
810+
'a native request dispatcher timeout',
811+
Object.assign(new Error('Headers Timeout Error'), { code: 'UND_ERR_HEADERS_TIMEOUT' }),
812+
{ retryable: true, message: expect.stringContaining('Try again') },
813+
],
814+
])('classifies %s during a read so the caller can act on it', async (_, failure, expected) => {
815+
const search = await searchLiveKnowledge.execute({ principal, input })
816+
mocks.read.mockRejectedValueOnce(failure)
817+
await expect(
818+
readLiveDocument.execute({
819+
principal,
820+
input: {
821+
workspaceId: 'workspace',
822+
documentId: search.results[0]?.documentId ?? '',
823+
limit: 1,
824+
resultSecretRegistry: new ResolvedSecretTraceRegistry([]),
825+
},
826+
})
827+
).rejects.toMatchObject(expected)
828+
})
772829
it.each([
773830
['a stop reason', 'user_stop:test'],
774831
['an AbortError', new DOMException('The operation was aborted.', 'AbortError')],

‎apps/sim/lib/sim-search/live/application.ts‎

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { ErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'
12
import { requirePrincipalSubjectUserId } from '@sim/auth/principal'
23
import { safeCompare } from '@sim/security/compare'
34
import { hmacSha256Hex } from '@sim/security/hmac'
@@ -17,6 +18,7 @@ import {
1718
} from '@/lib/api/contracts/mothership-assistant-tools'
1819
import { canonicalJson, fingerprint, instantScopePart } from '@/lib/api/cursor-binding'
1920
import { env } from '@/lib/core/config/env'
21+
import { isRetryableNetworkError } from '@/lib/core/errors/retryable-infrastructure'
2022
import { OrchestrationError } from '@/lib/core/orchestration/types'
2123
import {
2224
type ResourceOwner,
@@ -25,6 +27,7 @@ import {
2527
} from '@/lib/core/resource-scope'
2628
import { createPinnedConnectionPool } from '@/lib/core/security/input-validation.server'
2729
import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
30+
import { MANAGED_MCP_CONNECTORS } from '@/lib/credential-groups/managed-mcp-connectors'
2831
import { requireOrganizationSearchAvailable } from '@/lib/knowledge/access/availability'
2932
import { defineAuthorizedKnowledgeUseCase } from '@/lib/knowledge/application/authorized-knowledge-use-case'
3033
import { resolveKnowledgeOwnerContext } from '@/lib/knowledge/application/contexts'
@@ -33,6 +36,7 @@ import { measureSearchStage } from '@/lib/knowledge/search/diagnostics'
3336
import { RRF_K } from '@/lib/knowledge/search/recency'
3437
import { matchPassage } from '@/lib/knowledge/search/snippet'
3538
import { isKnowledgeSourceUrl } from '@/lib/knowledge/search/source-url'
39+
import { connectorDisplayName } from '@/lib/sim-search/connectors'
3640
import {
3741
type LiveAccountSession,
3842
openLiveAccountSession,
@@ -51,10 +55,12 @@ import {
5155
withImpliedListingBound,
5256
} from '@/lib/sim-search/live/dates'
5357
import { NativeSearchError } from '@/lib/sim-search/live/http'
58+
import { isManagedSearchMcpProvider } from '@/lib/sim-search/live/managed-mcp-config'
5459
import { joinMessages } from '@/lib/sim-search/live/pages'
5560
import { loadLiveSearchPolicies } from '@/lib/sim-search/live/policy-store'
5661
import { LIVE_SEARCH_PROVIDER_IDS } from '@/lib/sim-search/live/provider-catalog'
5762
import { liveSearchGuidance } from '@/lib/sim-search/live/providers'
63+
import { LiveReadError } from '@/lib/sim-search/live/read-error'
5864
import type { LiveAccount, NativeDocument } from '@/lib/sim-search/live/types'
5965
import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection'
6066
import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry'
@@ -746,6 +752,58 @@ export type LiveReadInput = ResourceOwner & {
746752
signal?: AbortSignal
747753
}
748754

755+
/** Milliseconds one document read may spend on provider calls, after account resolution. */
756+
const READ_DEADLINE_MS = 15_000
757+
758+
function liveProviderName(provider: string): string {
759+
return isManagedSearchMcpProvider(provider)
760+
? MANAGED_MCP_CONNECTORS[provider].name
761+
: connectorDisplayName(provider)
762+
}
763+
764+
/**
765+
* Classifies a provider failure during a read so the caller can act on it: a missing or
766+
* non-readable document, a grant to reconnect, or a transient failure worth retrying.
767+
* Classified and unrecognized errors pass through unchanged.
768+
*/
769+
function liveReadFailure(error: unknown, provider: string, deadline?: AbortSignal): unknown {
770+
if (error instanceof OrchestrationError) return error
771+
const name = liveProviderName(provider)
772+
if (error instanceof NativeSearchError) {
773+
if (error.status === 'reconnect')
774+
return new OrchestrationError(
775+
'unauthorized',
776+
`Reconnect ${name} to read this document. ${error.message}`
777+
)
778+
if (error.status === 'rate_limited')
779+
return new LiveReadError(error.message, true, error.retryAfterSeconds)
780+
if (error.status === 'timeout' || error.httpStatus === 408)
781+
return new LiveReadError(`${name} timed out reading this document. Try again.`, true)
782+
if (error.httpStatus === 404 || error.httpStatus === 410)
783+
return new OrchestrationError(
784+
'not_found',
785+
`${name} could not find this document: it was deleted, moved, or is not a readable page. Search again or read a different result.`
786+
)
787+
if (error.httpStatus !== undefined && error.httpStatus >= 500)
788+
return new LiveReadError(
789+
`${name} is temporarily unavailable (${error.httpStatus}). Try again shortly.`,
790+
true
791+
)
792+
return new LiveReadError(error.message, false)
793+
}
794+
if (deadline?.aborted || (error instanceof McpError && error.code === ErrorCode.RequestTimeout))
795+
return new LiveReadError(
796+
`${name} took too long to return this document. Try again, or read a different result.`,
797+
true
798+
)
799+
if (isRetryableNetworkError(error))
800+
return new LiveReadError(
801+
`${name} did not respond while reading this document. Try again shortly.`,
802+
true
803+
)
804+
return error
805+
}
806+
749807
export const readLiveDocument = defineAuthorizedKnowledgeUseCase({
750808
operation: knowledgeOperations.readDocument,
751809
resolveContext: ({ input }: { input: LiveReadInput }) => resolveKnowledgeOwnerContext(input),
@@ -765,15 +823,19 @@ export const readLiveDocument = defineAuthorizedKnowledgeUseCase({
765823
(input.filters?.documentIds && !input.filters.documentIds.includes(input.documentId))
766824
)
767825
throw new OrchestrationError('not_found', 'Document is outside the selected search filters')
826+
/** Cancellation by the caller passes through unclassified. */
827+
const readFailure = (error: unknown, deadline?: AbortSignal) =>
828+
input.signal?.aborted ? error : liveReadFailure(error, reference.provider, deadline)
768829
const [resolved, policies] = await Promise.all([
769830
resolveLiveAccount(input, userId, reference.account),
770831
loadLiveSearchPolicies(input),
771-
])
832+
]).catch((error: unknown) => {
833+
throw readFailure(error)
834+
})
772835
if (resolved.account.provider !== reference.provider)
773836
throw new OrchestrationError('not_found', 'Document account changed')
774-
const signal = input.signal
775-
? AbortSignal.any([input.signal, AbortSignal.timeout(15_000)])
776-
: AbortSignal.timeout(15_000)
837+
const deadline = AbortSignal.timeout(READ_DEADLINE_MS)
838+
const signal = input.signal ? AbortSignal.any([input.signal, deadline]) : deadline
777839
const pool = createPinnedConnectionPool()
778840
let document: NativeDocument
779841
let session: LiveAccountSession | undefined
@@ -805,6 +867,8 @@ export const readLiveDocument = defineAuthorizedKnowledgeUseCase({
805867
'not_found',
806868
'Document is outside your organization’s search scope'
807869
)
870+
} catch (error) {
871+
throw readFailure(error, deadline)
808872
} finally {
809873
try {
810874
await session?.close()

‎apps/sim/lib/sim-search/live/atlassian.test.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,4 +155,30 @@ describe('Confluence live documents', () => {
155155
expect(page.documents[0]?.accessMetadata).toEqual({ spaceKey: 'ENG' })
156156
expect(page.documents[2]?.accessMetadata).toEqual({ spaceKey: 'ENG' })
157157
})
158+
159+
it('drops native CQL matches that the page and blog post reads cannot open', async () => {
160+
const api = client({
161+
'/ex/confluence/cloud/wiki/rest/api/search': {
162+
results: [
163+
{ content: { id: '123', type: 'page', title: 'Runbook' } },
164+
{ content: { id: '77', type: 'attachment', title: 'diagram.png' } },
165+
{ content: { id: '78', type: 'comment', title: 'Re: Runbook' } },
166+
{ content: { id: '79', type: 'whiteboard', title: 'Planning' } },
167+
{ content: { id: '80', type: 'folder', title: 'Archive' } },
168+
],
169+
_links: {},
170+
},
171+
})
172+
const page = await searchAtlassian(api, 'confluence', {
173+
query: '',
174+
native: { provider: 'confluence', query: 'title ~ "runbook"' },
175+
limit: 10,
176+
scopes: [],
177+
})
178+
expect(page.documents.map(({ id, kind }) => ({ id, kind }))).toEqual([
179+
{ id: '123', kind: 'page' },
180+
])
181+
expect(page.message).toContain('cannot be read')
182+
expect(page.partial).toBe(true)
183+
})
158184
})

‎apps/sim/lib/sim-search/live/atlassian.ts‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,16 @@ function issue(row: Record<string, unknown>, cloudId: string, site: string): Nat
6565
author: string(object(fields.creator).displayName),
6666
}
6767
}
68-
function page(row: Record<string, unknown>, cloudId: string, site: string): NativeDocument {
68+
/**
69+
* Maps a CQL search hit to a readable document, or `undefined` for kinds the v2 page and blog
70+
* post reads cannot open. Native CQL can match attachments, comments, whiteboards, folders,
71+
* databases, and users; returning those would hand the caller a reference whose read is a 404.
72+
*/
73+
function page(
74+
row: Record<string, unknown>,
75+
cloudId: string,
76+
site: string
77+
): NativeDocument | undefined {
6978
if (string(row.entityType) === 'space') {
7079
const space = object(row.space)
7180
return {
@@ -83,9 +92,11 @@ function page(row: Record<string, unknown>, cloudId: string, site: string): Nati
8392
const links = object(content._links)
8493
const version = object(content.version)
8594
const spaceKey = string(object(content.space).key)
95+
const kind = string(content.type)
96+
if ((kind !== 'page' && kind !== 'blogpost') || !string(content.id)) return undefined
8697
return {
8798
id: string(content.id),
88-
kind: string(content.type) === 'blogpost' ? 'blogpost' : 'page',
99+
kind,
89100
...(spaceKey ? { accessMetadata: { spaceKey } } : {}),
90101
container: cloudId,
91102
title: string(content.title) || string(row.title),
@@ -164,6 +175,7 @@ export async function searchAtlassian(
164175
)
165176
return {
166177
documents: array(data.issues).map((row) => issue(row, cloudId, origin)),
178+
excluded: false,
167179
next: string(data.nextPageToken) || undefined,
168180
}
169181
}
@@ -183,22 +195,31 @@ export async function searchAtlassian(
183195
})
184196
)
185197
const next = string(object(data._links).next)
198+
const rows = array(data.results)
199+
const documents = rows
200+
.map((row) => page(row, cloudId, origin))
201+
.filter((document) => document !== undefined)
186202
return {
187-
documents: array(data.results).map((row) => page(row, cloudId, origin)),
203+
documents,
204+
excluded: documents.length < rows.length,
188205
next: next
189206
? (new URL(next, 'https://api.atlassian.com').searchParams.get('cursor') ?? undefined)
190207
: undefined,
191208
}
192209
})
193210
)
194211
const documents = interleaveByRank(pages.map((result) => result.documents))
212+
const excluded = pages.some((result) => result.excluded)
195213
return {
196214
documents,
197-
partial: !input.native?.project && allSites.length > selected.length,
215+
partial: excluded || (!input.native?.project && allSites.length > selected.length),
198216
hasMore: pages.some((result) => Boolean(result.next)),
199217
nextCursor: single ? pages[0]?.next : undefined,
200218
message:
201-
'Searches up to four accessible Atlassian sites. For a specific site and pagination, set project to its cloud ID.',
219+
'Searches up to four accessible Atlassian sites. For a specific site and pagination, set project to its cloud ID.' +
220+
(excluded
221+
? ' Matches other than pages, blog posts, and spaces (attachments, comments, whiteboards, folders, databases) were excluded because they cannot be read.'
222+
: ''),
202223
}
203224
}
204225

‎apps/sim/lib/sim-search/live/managed-mcp.integration.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,9 @@ describe('managed Search operation sessions', () => {
739739
.where(eq(credential.id, actors[0].credentialId))
740740
}
741741
try {
742-
await expect(read(found.results[0].documentId)).rejects.toMatchObject({ status: 'reconnect' })
742+
await expect(read(found.results[0].documentId)).rejects.toMatchObject({
743+
code: 'unauthorized',
744+
})
743745
expect(openTransports.size).toBe(0)
744746
} finally {
745747
onTool = undefined
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/**
2+
* A provider-side read failure whose message tells the caller what to do next, and whether the
3+
* same read may succeed if retried. Messages are written by Search code, never copied from a
4+
* provider response body.
5+
*/
6+
export class LiveReadError extends Error {
7+
constructor(
8+
message: string,
9+
readonly retryable: boolean,
10+
readonly retryAfterSeconds?: number
11+
) {
12+
super(message)
13+
this.name = 'LiveReadError'
14+
}
15+
}

0 commit comments

Comments
 (0)