Skip to content

Commit a82622f

Browse files
committed
fix(copilot): harden docs context retrieval
1 parent 935edc4 commit a82622f

8 files changed

Lines changed: 177 additions & 34 deletions

File tree

apps/sim/lib/copilot/chat/post.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,8 @@ describe('handleUnifiedChatPost', () => {
384384
'user-1',
385385
'Hello',
386386
'ws-1',
387-
expect.anything()
387+
expect.anything(),
388+
expect.any(ResolvedSecretTraceRegistry)
388389
)
389390
})
390391

@@ -448,7 +449,8 @@ describe('handleUnifiedChatPost', () => {
448449
'user-1',
449450
'Explain these selections',
450451
'ws-1',
451-
'chat-1'
452+
'chat-1',
453+
expect.any(ResolvedSecretTraceRegistry)
452454
)
453455
})
454456

apps/sim/lib/copilot/chat/process-contents.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,34 @@ describe('processContextsServer - docs contexts', () => {
373373
expect.objectContaining({ workspaceId: 'ws-1', chatId: 'chat-1' })
374374
)
375375
})
376+
377+
it('preserves an explicit unavailable note when docs search fails', async () => {
378+
searchDocsExecute.mockRejectedValue(new Error('embedding service unavailable'))
379+
380+
const result = await processContextsServer(
381+
[{ kind: 'docs', label: 'Docs' }],
382+
'user-1',
383+
'@Docs explain schedules',
384+
'ws-1',
385+
'chat-1',
386+
new ResolvedSecretTraceRegistry()
387+
)
388+
389+
expect(result).toEqual([
390+
{
391+
type: 'docs',
392+
tag: '@Docs',
393+
content: JSON.stringify({
394+
results: [],
395+
note: 'Documentation search is temporarily unavailable. Do not infer that the docs lack this topic; retry search_docs or browse docs/** later.',
396+
}),
397+
},
398+
])
399+
expect(mockProcessContentsLogger.error).toHaveBeenCalledWith(
400+
'Failed to process docs context',
401+
expect.any(Error)
402+
)
403+
})
376404
})
377405

378406
describe('processContextsServer - MCP contexts', () => {

apps/sim/lib/copilot/chat/process-contents.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,14 @@ export async function processContextsServer(
338338
return { type: 'docs', tag: ctx.label ? `@${ctx.label}` : '@', content }
339339
} catch (e) {
340340
logger.error('Failed to process docs context', e)
341-
return null
341+
return {
342+
type: 'docs',
343+
tag: ctx.label ? `@${ctx.label}` : '@',
344+
content: JSON.stringify({
345+
results: [],
346+
note: 'Documentation search is temporarily unavailable. Do not infer that the docs lack this topic; retry search_docs or browse docs/** later.',
347+
}),
348+
}
342349
}
343350
}
344351
return null

apps/sim/lib/copilot/docs/docs-corpus.test.ts

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@
33
*/
44
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
55

6+
const { mockSleep } = vi.hoisted(() => ({
7+
mockSleep: vi.fn(() => Promise.resolve()),
8+
}))
9+
610
vi.mock('@sim/utils/helpers', () => ({
7-
sleep: vi.fn(() => Promise.resolve()),
11+
sleep: mockSleep,
812
}))
913

1014
import {
@@ -19,6 +23,15 @@ import { DOCS_MANIFEST } from '@/lib/copilot/generated/docs-manifest'
1923

2024
const SAMPLE_PAGE = DOCS_MANIFEST.find((path) => path === 'workflows/blocks/agent.mdx')
2125

26+
function fetchResponse(status: number, content = '', headers: HeadersInit = {}) {
27+
return {
28+
ok: status >= 200 && status < 300,
29+
status,
30+
headers: new Headers(headers),
31+
text: async () => content,
32+
}
33+
}
34+
2235
describe('docs corpus scoping', () => {
2336
it('recognizes docs paths', () => {
2437
expect(isDocsPath('docs/workflows.mdx')).toBe(true)
@@ -75,6 +88,8 @@ describe('readDocsPage', () => {
7588

7689
beforeEach(() => {
7790
fetchMock.mockReset()
91+
mockSleep.mockReset()
92+
mockSleep.mockResolvedValue(undefined)
7893
vi.stubGlobal('fetch', fetchMock)
7994
})
8095

@@ -84,7 +99,7 @@ describe('readDocsPage', () => {
8499

85100
it('fetches the manifest path verbatim from the docs site', async () => {
86101
expect(SAMPLE_PAGE).toBeDefined()
87-
fetchMock.mockResolvedValue({ ok: true, status: 200, text: async () => '# Agent\n\nbody' })
102+
fetchMock.mockResolvedValue(fetchResponse(200, '# Agent\n\nbody'))
88103

89104
const page = await readDocsPage(`docs/${SAMPLE_PAGE}`)
90105

@@ -104,7 +119,7 @@ describe('readDocsPage', () => {
104119
})
105120

106121
it('surfaces a docs-site outage as a retryable error after exhausting retries', async () => {
107-
fetchMock.mockResolvedValue({ ok: false, status: 502, text: async () => '' })
122+
fetchMock.mockResolvedValue(fetchResponse(502))
108123
await expect(readDocsPage(`docs/${SAMPLE_PAGE}`)).rejects.toThrow(/could not be reached/)
109124
expect(fetchMock).toHaveBeenCalledTimes(3)
110125
})
@@ -118,7 +133,7 @@ describe('readDocsPage', () => {
118133
it('recovers when a transient failure clears on retry', async () => {
119134
fetchMock
120135
.mockRejectedValueOnce(new Error('socket hang up'))
121-
.mockResolvedValue({ ok: true, status: 200, text: async () => '# Agent\n\nbody' })
136+
.mockResolvedValue(fetchResponse(200, '# Agent\n\nbody'))
122137

123138
const page = await readDocsPage(`docs/${SAMPLE_PAGE}`)
124139

@@ -127,7 +142,7 @@ describe('readDocsPage', () => {
127142
})
128143

129144
it('reports a page the site no longer serves as permanent, without retrying', async () => {
130-
fetchMock.mockResolvedValue({ ok: false, status: 404, text: async () => '' })
145+
fetchMock.mockResolvedValue(fetchResponse(404))
131146
const error = await readDocsPage(`docs/${SAMPLE_PAGE}`).catch((e) => e)
132147
expect(error).toBeInstanceOf(DocsCorpusError)
133148
expect(error.message).toMatch(/does not serve it/)
@@ -136,17 +151,50 @@ describe('readDocsPage', () => {
136151
expect(fetchMock).toHaveBeenCalledOnce()
137152
})
138153

139-
it('still treats 429 as retryable rather than permanent', async () => {
140-
fetchMock.mockResolvedValue({ ok: false, status: 429, text: async () => '' })
154+
it('honors Retry-After while retrying a 429 response', async () => {
155+
fetchMock.mockResolvedValue(fetchResponse(429, '', { 'Retry-After': '7' }))
141156
await expect(readDocsPage(`docs/${SAMPLE_PAGE}`)).rejects.toThrow(/could not be reached/)
142157
expect(fetchMock).toHaveBeenCalledTimes(3)
158+
expect(mockSleep).toHaveBeenNthCalledWith(1, 7_000)
159+
expect(mockSleep).toHaveBeenNthCalledWith(2, 7_000)
143160
})
144161

145162
it('treats 408 as retryable rather than a missing page', async () => {
146-
fetchMock.mockResolvedValue({ ok: false, status: 408, text: async () => '' })
163+
fetchMock.mockResolvedValue(fetchResponse(408))
147164
await expect(readDocsPage(`docs/${SAMPLE_PAGE}`)).rejects.toThrow(/could not be reached/)
148165
expect(fetchMock).toHaveBeenCalledTimes(3)
149166
})
167+
168+
it('aborts an in-flight fetch without retrying', async () => {
169+
const controller = new AbortController()
170+
fetchMock.mockImplementation((_url: string, init: RequestInit) => {
171+
const signal = init.signal as AbortSignal
172+
return new Promise((_resolve, reject) => {
173+
signal.addEventListener('abort', () => reject(signal.reason), { once: true })
174+
})
175+
})
176+
177+
const request = readDocsPage(`docs/${SAMPLE_PAGE}`, controller.signal)
178+
await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledOnce())
179+
controller.abort(new Error('user stopped docs read'))
180+
181+
await expect(request).rejects.toThrow('user stopped docs read')
182+
expect(fetchMock).toHaveBeenCalledOnce()
183+
expect(mockSleep).not.toHaveBeenCalled()
184+
})
185+
186+
it('aborts retry backoff before starting another fetch', async () => {
187+
const controller = new AbortController()
188+
fetchMock.mockResolvedValue(fetchResponse(502))
189+
mockSleep.mockImplementationOnce(() => new Promise<void>(() => {}))
190+
191+
const request = readDocsPage(`docs/${SAMPLE_PAGE}`, controller.signal)
192+
await vi.waitFor(() => expect(mockSleep).toHaveBeenCalledOnce())
193+
controller.abort(new Error('user stopped docs retry'))
194+
195+
await expect(request).rejects.toThrow('user stopped docs retry')
196+
expect(fetchMock).toHaveBeenCalledOnce()
197+
})
150198
})
151199

152200
describe('grepDocs', () => {
@@ -163,11 +211,7 @@ describe('grepDocs', () => {
163211
})
164212

165213
it('greps exactly one page for a page path', async () => {
166-
fetchMock.mockResolvedValue({
167-
ok: true,
168-
status: 200,
169-
text: async () => 'intro line\nsystemPrompt matters\ntail',
170-
})
214+
fetchMock.mockResolvedValue(fetchResponse(200, 'intro line\nsystemPrompt matters\ntail'))
171215

172216
const matches = await grepDocs(`docs/${SAMPLE_PAGE}`, 'systemPrompt')
173217

apps/sim/lib/copilot/docs/docs-corpus.ts

Lines changed: 52 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { toError } from '@sim/utils/errors'
33
import { sleep } from '@sim/utils/helpers'
4-
import { backoffWithJitter } from '@sim/utils/retry'
4+
import { backoffWithJitter, parseRetryAfter } from '@sim/utils/retry'
55
import { foldDocsIndexPath } from '@/lib/copilot/docs/docs-path'
66
import { DOCS_MANIFEST } from '@/lib/copilot/generated/docs-manifest'
77
import type { GrepCountEntry, GrepMatch, GrepOptions } from '@/lib/copilot/vfs/operations'
@@ -121,12 +121,43 @@ type DocsFetchResult =
121121
/** The site will not serve this path however many times we ask. */
122122
| { outcome: 'missing' }
123123
/** Transient: 5xx, 429, network error, or timeout. */
124-
| { outcome: 'unavailable' }
124+
| { outcome: 'unavailable'; retryAfterMs: number | null }
125+
126+
function throwIfAborted(signal?: AbortSignal): void {
127+
if (signal?.aborted) {
128+
throw toError(signal.reason ?? 'Docs request aborted')
129+
}
130+
}
131+
132+
async function sleepForRetry(delayMs: number, signal?: AbortSignal): Promise<void> {
133+
if (!signal) {
134+
await sleep(delayMs)
135+
return
136+
}
137+
138+
throwIfAborted(signal)
139+
let abortListener: (() => void) | undefined
140+
const aborted = new Promise<never>((_resolve, reject) => {
141+
abortListener = () => reject(toError(signal.reason ?? 'Docs request aborted'))
142+
signal.addEventListener('abort', abortListener, { once: true })
143+
if (signal.aborted) abortListener()
144+
})
145+
146+
try {
147+
await Promise.race([sleep(delayMs), aborted])
148+
} finally {
149+
if (abortListener) signal.removeEventListener('abort', abortListener)
150+
}
151+
}
152+
153+
async function fetchDocsPageOnce(url: string, signal?: AbortSignal): Promise<DocsFetchResult> {
154+
throwIfAborted(signal)
155+
const timeoutSignal = AbortSignal.timeout(FETCH_ATTEMPT_TIMEOUT_MS)
156+
const requestSignal = signal ? AbortSignal.any([signal, timeoutSignal]) : timeoutSignal
125157

126-
async function fetchDocsPageOnce(url: string): Promise<DocsFetchResult> {
127158
try {
128159
const response = await fetch(url, {
129-
signal: AbortSignal.timeout(FETCH_ATTEMPT_TIMEOUT_MS),
160+
signal: requestSignal,
130161
headers: { Accept: 'text/markdown, text/plain' },
131162
})
132163
if (!response.ok) {
@@ -136,23 +167,30 @@ async function fetchDocsPageOnce(url: string): Promise<DocsFetchResult> {
136167
response.status < 500 &&
137168
response.status !== 408 &&
138169
response.status !== 429
139-
return { outcome: permanent ? 'missing' : 'unavailable' }
170+
if (permanent) return { outcome: 'missing' }
171+
return {
172+
outcome: 'unavailable',
173+
retryAfterMs: parseRetryAfter(response.headers.get('retry-after')),
174+
}
140175
}
141176
return { outcome: 'ok', content: await response.text() }
142177
} catch (err) {
178+
throwIfAborted(signal)
143179
logger.warn('Docs page fetch failed', { url, error: toError(err).message })
144-
return { outcome: 'unavailable' }
180+
return { outcome: 'unavailable', retryAfterMs: null }
145181
}
146182
}
147183

148-
async function fetchDocsPage(path: string): Promise<DocsFetchResult> {
184+
async function fetchDocsPage(path: string, signal?: AbortSignal): Promise<DocsFetchResult> {
149185
const key = normalizeDocsPath(path)
150186
if (!docsKeyView.has(key)) return { outcome: 'missing' }
151187
const url = `${DOCS_BASE_URL}/${key.slice(DOCS_PREFIX.length)}`
152188
for (let attempt = 1; ; attempt++) {
153-
const result = await fetchDocsPageOnce(url)
189+
throwIfAborted(signal)
190+
const result = await fetchDocsPageOnce(url, signal)
191+
throwIfAborted(signal)
154192
if (result.outcome !== 'unavailable' || attempt >= FETCH_MAX_ATTEMPTS) return result
155-
await sleep(backoffWithJitter(attempt, null))
193+
await sleepForRetry(backoffWithJitter(attempt, result.retryAfterMs), signal)
156194
}
157195
}
158196

@@ -161,7 +199,7 @@ async function fetchDocsPage(path: string): Promise<DocsFetchResult> {
161199
* conditions (directory path, unknown page, site unreachable) so the handler can
162200
* surface the message verbatim.
163201
*/
164-
export async function readDocsPage(path: string): Promise<DocsPage> {
202+
export async function readDocsPage(path: string, signal?: AbortSignal): Promise<DocsPage> {
165203
const key = normalizeDocsPath(path)
166204
if (!docsKeyView.has(key)) {
167205
if (isDocsDir(key)) {
@@ -172,7 +210,7 @@ export async function readDocsPage(path: string): Promise<DocsPage> {
172210
`Docs page not found: ${path}. Use glob("docs/**") to list the docs corpus.`
173211
)
174212
}
175-
const result = await fetchDocsPage(key)
213+
const result = await fetchDocsPage(key, signal)
176214
if (result.outcome === 'missing') {
177215
throw new DocsCorpusError(
178216
`${key} is in the docs index but ${DOCS_BASE_URL} does not serve it — the page was likely moved or removed. Use glob("docs/**") to find the current path; retrying will not help.`
@@ -194,7 +232,8 @@ export async function readDocsPage(path: string): Promise<DocsPage> {
194232
export async function grepDocs(
195233
path: string,
196234
pattern: string,
197-
options?: GrepOptions
235+
options?: GrepOptions,
236+
signal?: AbortSignal
198237
): Promise<GrepMatch[] | string[] | GrepCountEntry[]> {
199238
const key = normalizeDocsPath(path)
200239
if (!docsKeyView.has(key)) {
@@ -207,6 +246,6 @@ export async function grepDocs(
207246
`"${path}" is not a docs page. Use glob("docs/**") to list the docs corpus.`
208247
)
209248
}
210-
const page = await readDocsPage(key)
249+
const page = await readDocsPage(key, signal)
211250
return grepReadResult(key, page, pattern, key, options)
212251
}

0 commit comments

Comments
 (0)