Skip to content

Commit 9c51348

Browse files
committed
fix(search): return actionable live read errors and drop unreadable Confluence matches
read_document collapsed every provider-side read failure into the generic 'Knowledge operation failed' because NativeSearchError escaped readLiveDocument unclassified. Reads now map a 404/410 to not_found with a search-again hint, a revoked grant to unauthorized naming the provider, and rate limits, 5xx, provider timeouts, MCP request timeouts and the read deadline to a retryable LiveReadError that the Assistant tool reports with retryable (and retryAfterSeconds) like search_workspace. Confluence search labeled every non-blogpost CQL hit a page, so native CQL matching attachments, comments, whiteboards, folders or databases produced references whose v2 page read 404s. Those kinds are now dropped and the page message says so.
1 parent cf4e2d4 commit 9c51348

7 files changed

Lines changed: 219 additions & 45 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: 42 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,47 @@ 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+
])('classifies %s during a read so the caller can act on it', async (_, failure, expected) => {
800+
const search = await searchLiveKnowledge.execute({ principal, input })
801+
mocks.read.mockRejectedValueOnce(failure)
802+
await expect(
803+
readLiveDocument.execute({
804+
principal,
805+
input: {
806+
workspaceId: 'workspace',
807+
documentId: search.results[0]!.documentId,
808+
limit: 1,
809+
resultSecretRegistry: new ResolvedSecretTraceRegistry([]),
810+
},
811+
})
812+
).rejects.toMatchObject(expected)
813+
})
772814
it.each([
773815
['a stop reason', 'user_stop:test'],
774816
['an AbortError', new DOMException('The operation was aborted.', 'AbortError')],

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

Lines changed: 101 additions & 41 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'
@@ -25,6 +26,7 @@ import {
2526
} from '@/lib/core/resource-scope'
2627
import { createPinnedConnectionPool } from '@/lib/core/security/input-validation.server'
2728
import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
29+
import { MANAGED_MCP_CONNECTORS } from '@/lib/credential-groups/managed-mcp-connectors'
2830
import { requireOrganizationSearchAvailable } from '@/lib/knowledge/access/availability'
2931
import { defineAuthorizedKnowledgeUseCase } from '@/lib/knowledge/application/authorized-knowledge-use-case'
3032
import { resolveKnowledgeOwnerContext } from '@/lib/knowledge/application/contexts'
@@ -33,12 +35,14 @@ import { measureSearchStage } from '@/lib/knowledge/search/diagnostics'
3335
import { RRF_K } from '@/lib/knowledge/search/recency'
3436
import { matchPassage } from '@/lib/knowledge/search/snippet'
3537
import { isKnowledgeSourceUrl } from '@/lib/knowledge/search/source-url'
38+
import { connectorDisplayName } from '@/lib/sim-search/connectors'
3639
import {
3740
type LiveAccountSession,
3841
openLiveAccountSession,
3942
} from '@/lib/sim-search/live/account-session'
4043
import {
4144
listLiveAccounts,
45+
type ResolvedLiveAccount,
4246
resolveListedLiveAccount,
4347
resolveLiveAccount,
4448
} from '@/lib/sim-search/live/accounts'
@@ -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'
@@ -742,6 +748,53 @@ export type LiveReadInput = ResourceOwner & {
742748
signal?: AbortSignal
743749
}
744750

751+
/** Milliseconds one document read may spend on provider calls, after account resolution. */
752+
const READ_DEADLINE_MS = 15_000
753+
754+
function liveProviderName(provider: string): string {
755+
return isManagedSearchMcpProvider(provider)
756+
? MANAGED_MCP_CONNECTORS[provider].name
757+
: connectorDisplayName(provider)
758+
}
759+
760+
/**
761+
* Classifies a provider failure during a read so the caller can act on it: a missing or
762+
* non-readable document, a grant to reconnect, or a transient failure worth retrying.
763+
* Classified and unrecognized errors pass through unchanged.
764+
*/
765+
function liveReadFailure(error: unknown, provider: string, deadline?: AbortSignal): unknown {
766+
if (error instanceof OrchestrationError) return error
767+
const name = liveProviderName(provider)
768+
if (error instanceof NativeSearchError) {
769+
if (error.status === 'reconnect')
770+
return new OrchestrationError(
771+
'unauthorized',
772+
`Reconnect ${name} to read this document. ${error.message}`
773+
)
774+
if (error.status === 'rate_limited')
775+
return new LiveReadError(error.message, true, error.retryAfterSeconds)
776+
if (error.status === 'timeout')
777+
return new LiveReadError(`${name} timed out reading this document. Try again.`, true)
778+
if (error.httpStatus === 404 || error.httpStatus === 410)
779+
return new OrchestrationError(
780+
'not_found',
781+
`${name} could not find this document: it was deleted, moved, or is not a readable page. Search again or read a different result.`
782+
)
783+
if (error.httpStatus !== undefined && error.httpStatus >= 500)
784+
return new LiveReadError(
785+
`${name} is temporarily unavailable (${error.httpStatus}). Try again shortly.`,
786+
true
787+
)
788+
return new LiveReadError(error.message, false)
789+
}
790+
if (deadline?.aborted || (error instanceof McpError && error.code === ErrorCode.RequestTimeout))
791+
return new LiveReadError(
792+
`${name} took too long to return this document. Try again, or read a different result.`,
793+
true
794+
)
795+
return error
796+
}
797+
745798
export const readLiveDocument = defineAuthorizedKnowledgeUseCase({
746799
operation: knowledgeOperations.readDocument,
747800
resolveContext: ({ input }: { input: LiveReadInput }) => resolveKnowledgeOwnerContext(input),
@@ -761,52 +814,59 @@ export const readLiveDocument = defineAuthorizedKnowledgeUseCase({
761814
(input.filters?.documentIds && !input.filters.documentIds.includes(input.documentId))
762815
)
763816
throw new OrchestrationError('not_found', 'Document is outside the selected search filters')
764-
const [resolved, policies] = await Promise.all([
765-
resolveLiveAccount(input, userId, reference.account),
766-
loadLiveSearchPolicies(input),
767-
])
768-
if (resolved.account.provider !== reference.provider)
769-
throw new OrchestrationError('not_found', 'Document account changed')
770-
const signal = input.signal
771-
? AbortSignal.any([input.signal, AbortSignal.timeout(15_000)])
772-
: AbortSignal.timeout(15_000)
773-
const pool = createPinnedConnectionPool()
817+
let resolved: ResolvedLiveAccount
774818
let document: NativeDocument
775-
let session: LiveAccountSession | undefined
819+
let deadline: AbortSignal | undefined
776820
try {
777-
session = await openLiveAccountSession({
778-
owner: input,
779-
userId,
780-
resolved,
781-
policies,
782-
signal,
783-
pool,
784-
})
785-
if (!(await session.verify(reference)))
786-
throw new OrchestrationError(
787-
'not_found',
788-
'Document is outside your organization’s search scope'
789-
)
790-
const currentSession = session
791-
document = await measureSearchStage('live.read', () =>
792-
currentSession.read(reference, input.filters)
793-
)
794-
/** Readers degrade section failures to warnings, so the signal decides cancellation. */
795-
signal.throwIfAborted()
796-
const current = await session.verifyCurrent(document)
797-
/** A verifier may report a check cut short by cancellation as a normal result. */
798-
signal.throwIfAborted()
799-
if (!current)
800-
throw new OrchestrationError(
801-
'not_found',
802-
'Document is outside your organization’s search scope'
803-
)
804-
} finally {
821+
const [account, policies] = await Promise.all([
822+
resolveLiveAccount(input, userId, reference.account),
823+
loadLiveSearchPolicies(input),
824+
])
825+
resolved = account
826+
if (resolved.account.provider !== reference.provider)
827+
throw new OrchestrationError('not_found', 'Document account changed')
828+
deadline = AbortSignal.timeout(READ_DEADLINE_MS)
829+
const signal = input.signal ? AbortSignal.any([input.signal, deadline]) : deadline
830+
const pool = createPinnedConnectionPool()
831+
let session: LiveAccountSession | undefined
805832
try {
806-
await session?.close()
833+
session = await openLiveAccountSession({
834+
owner: input,
835+
userId,
836+
resolved,
837+
policies,
838+
signal,
839+
pool,
840+
})
841+
if (!(await session.verify(reference)))
842+
throw new OrchestrationError(
843+
'not_found',
844+
'Document is outside your organization’s search scope'
845+
)
846+
const currentSession = session
847+
document = await measureSearchStage('live.read', () =>
848+
currentSession.read(reference, input.filters)
849+
)
850+
/** Readers degrade section failures to warnings, so the signal decides cancellation. */
851+
signal.throwIfAborted()
852+
const current = await session.verifyCurrent(document)
853+
/** A verifier may report a check cut short by cancellation as a normal result. */
854+
signal.throwIfAborted()
855+
if (!current)
856+
throw new OrchestrationError(
857+
'not_found',
858+
'Document is outside your organization’s search scope'
859+
)
807860
} finally {
808-
pool.destroy()
861+
try {
862+
await session?.close()
863+
} finally {
864+
pool.destroy()
865+
}
809866
}
867+
} catch (error) {
868+
if (input.signal?.aborted) throw error
869+
throw liveReadFailure(error, reference.provider, deadline)
810870
}
811871
if (!matchesLiveFilters(document, input.documentId, reference.provider, input.filters))
812872
throw new OrchestrationError('not_found', 'Document is outside the selected search filters')

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,4 +155,29 @@ 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+
})
158183
})

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

Lines changed: 25 additions & 4 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,
197215
partial: !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

0 commit comments

Comments
 (0)