From 548512f92d29c101486ab80b98ea12113e22cd6f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 3 Nov 2026 12:24:00 -0800 Subject: [PATCH] improvement(agent-cli): skip non-text files in grep instead of marking the search incomplete --- .../agent-cli/engines/universal-grep.test.ts | 115 ++++++++++++++++++ .../agent-cli/engines/universal-grep.ts | 75 +++++++++--- .../agent-cli/project-file-grep.test.ts | 17 +++ .../mothership/agent-cli/project-file-grep.ts | 97 +++++++++------ 4 files changed, 246 insertions(+), 58 deletions(-) diff --git a/apps/sim/lib/mothership/agent-cli/engines/universal-grep.test.ts b/apps/sim/lib/mothership/agent-cli/engines/universal-grep.test.ts index d29e49c6878..3f829e50166 100644 --- a/apps/sim/lib/mothership/agent-cli/engines/universal-grep.test.ts +++ b/apps/sim/lib/mothership/agent-cli/engines/universal-grep.test.ts @@ -524,6 +524,121 @@ describe('search authority and completeness', () => { expect(result.stderr).toContain('broken') }) + it('skips files with no text extractor without fetching them or failing the search', async () => { + const requested: string[] = [] + const result = await runEngine( + 'grep', + ['needle'], + runtimeWith( + { + '/api/v2/files': { + data: [ + { id: 'notes', name: 'notes.md', type: 'text/markdown' }, + { id: 'report', name: 'report.pdf', type: 'application/pdf' }, + { id: 'font', name: 'brand.woff2', type: 'font/woff2' }, + { id: 'clip', name: 'demo.mp4', type: 'video/mp4' }, + ], + nextCursor: null, + }, + '/api/v2/files/notes/text': { data: { text: 'needle', degraded: false } }, + '/api/v2/files/report/text': { data: { text: 'a needle', degraded: false } }, + }, + requested + ), + { scope: 'files' } + ) + expect(requested.filter((path) => path !== '/api/v2/files').sort()).toEqual([ + '/api/v2/files/notes/text', + '/api/v2/files/report/text', + ]) + expect(result.exitCode).toBe(0) + expect(result.stdout).toBe( + 'files/notes.md (notes):1: needle\nfiles/report.pdf (report):1: a needle' + ) + expect(result.stderr).toBe( + 'Skipped 2 files with no searchable text (images, media, archives, and other binary formats).' + ) + }) + + it('keeps real read failures incomplete beside skipped non-text files', async () => { + const result = await runEngine( + 'grep', + ['needle'], + runtimeWith({ + '/api/v2/files': { + data: [ + { id: 'broken', name: 'broken.txt', type: 'text/plain' }, + { id: 'logo', name: 'logo.png', type: 'image/png' }, + ], + nextCursor: null, + }, + }), + { scope: 'files' } + ) + expect(result.exitCode).toBe(1) + expect(result.stderr).toMatch(/^Search incomplete \(1 issue\)/) + expect(result.stderr).toContain('files/broken.txt (broken): could not read text') + expect(result.stderr).toContain('Skipped 1 file with no searchable text') + }) + + it('names a selected non-text file as searched-and-skipped, not an unknown selector', async () => { + const requested: string[] = [] + const result = await runEngine( + 'grep', + ['needle'], + runtimeWith( + { + '/api/v2/files': { + data: [{ id: 'logo', name: 'logo.png', type: 'image/png' }], + nextCursor: null, + }, + }, + requested + ), + { scope: 'files', in: 'files/logo' } + ) + expect(requested).toEqual(['/api/v2/files']) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('No matches') + expect(result.stderr).toContain('Skipped 1 file with no searchable text') + }) + + it('spends the file-read limit only on files with text', async () => { + const files = [ + ...Array.from({ length: 300 }, (_, i) => ({ + id: `img${i}`, + name: `img${i}.png`, + type: 'image/png', + })), + { id: 'notes', name: 'notes.txt', type: 'text/plain' }, + ] + const result = await runEngine( + 'grep', + ['needle'], + { + ...runtimeWith({}), + client: { + request: async ( + path: string, + options?: { query?: Record } + ): Promise => { + if (path === '/api/v2/files') { + const offset = Number(options?.query?.cursor ?? 0) + return { + data: files.slice(offset, offset + 100), + nextCursor: offset + 100 < files.length ? String(offset + 100) : null, + } as T + } + if (path === '/api/v2/files/notes/text') return { data: { text: 'needle' } } as T + throw new Error(`Unexpected request: ${path}`) + }, + }, + }, + { scope: 'files', count: true } + ) + expect(result).toMatchObject({ exitCode: 0, stdout: '1 (files=1)' }) + }) + it('reports truncated extraction even when a returned prefix has a match', async () => { const result = await runEngine( 'grep', diff --git a/apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts b/apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts index 83a668f8cb3..9fd8a5e35d8 100644 --- a/apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts +++ b/apps/sim/lib/mothership/agent-cli/engines/universal-grep.ts @@ -11,9 +11,9 @@ import { type AgentCliFlags, type AgentCliRuntime, agentCliFail, - agentCliOk, } from '@/lib/mothership/agent-cli/types' import { buildIntegrationToolSchemas } from '@/lib/mothership/chat/payload' +import { workspaceFileTextFormat } from '@/lib/workspace-files/text-format' import { projectResolvedSecretModelContent } from '@/executor/utils/resolved-secret-content-projection' import { OPAQUE_RESOLVED_SECRET_REPLACEMENT } from '@/executor/utils/resolved-secret-matcher' import type { ResolvedSecretTraceRegistry } from '@/executor/utils/resolved-secret-trace-registry' @@ -61,6 +61,7 @@ const PLATFORM_SCOPES: ReadonlySet = new Set(['blocks', 'tools']) interface GrepRuntime extends AgentCliRuntime { reportIncomplete(message: string): void + reportNonText(): void } /** @@ -370,8 +371,14 @@ const FETCHERS: Record< credentials: async (_runtime, item) => render('credentials', item.id, item.label, item.raw), } -function concurrencyFor(scope: Scope): number { - return scope === 'files' ? FILE_READ_CONCURRENCY : FETCH_CONCURRENCY +/** + * A listed file the agent's text read has no extractor for (fonts, media, archives, images), by + * the same gate that read applies. A listing without a name cannot say, so that file is read. + */ +function lacksText(file: unknown): boolean { + if (!isRecordLike(file)) return false + const name = str(file.name) + return name !== undefined && !workspaceFileTextFormat({ name, type: str(file.type) ?? '' }) } async function fetchAll( @@ -379,13 +386,29 @@ async function fetchAll( scope: Scope, entries: IndexEntry[] ): Promise { - if (scope === 'files' && entries.length > MAX_GREP_FILES) { + if (scope !== 'files') { + return mapConcurrent(entries, FETCH_CONCURRENCY, (item) => FETCHERS[scope](runtime, item)) + } + // Skipped files keep their identity, so `--in` naming one is not an unknown selector. + const skipped: GrepMaterialized[] = [] + const readable: IndexEntry[] = [] + for (const item of entries) { + if (lacksText(item.raw)) { + runtime.reportNonText() + skipped.push({ scope, id: item.id, label: item.label, text: null }) + } else readable.push(item) + } + if (readable.length > MAX_GREP_FILES) { runtime.reportIncomplete( - `files: searched the first ${MAX_GREP_FILES} of ${entries.length} selected files; narrow with --in files/.` + `files: searched the first ${MAX_GREP_FILES} of ${readable.length} selected files; narrow with --in files/.` ) } - const selected = scope === 'files' ? entries.slice(0, MAX_GREP_FILES) : entries - return mapConcurrent(selected, concurrencyFor(scope), (item) => FETCHERS[scope](runtime, item)) + const read = await mapConcurrent( + readable.slice(0, MAX_GREP_FILES), + FILE_READ_CONCURRENCY, + (item) => FETCHERS.files(runtime, item) + ) + return [...read, ...skipped] } /** A whole world, for a search with no `--in`: every resource the index lists. */ @@ -566,10 +589,15 @@ export interface GrepSource { registry?: AgentCliRuntime['resolvedSecretTraceRegistry'] /** The only scopes this corpus holds, and the refusal for a search that names another. */ scopeLimit?: { scopes: readonly Scope[]; refusal: string } + /** + * `reportNonText` counts a file skipped unread because it has no text to extract: a note, + * not an incomplete search. The skipped file is still returned with null text. + */ materialize( scopes: Scope[], nameFilter: string | undefined, - reportIncomplete: (message: string) => void + reportIncomplete: (message: string) => void, + reportNonText: () => void ): Promise } @@ -624,15 +652,24 @@ export async function executeGrep(positionals: string[], flags: AgentCliFlags, s issueCount++ if (issues.length < 10) issues.push(clip(message)) } - const finish = (stdout: string) => - issueCount === 0 - ? agentCliOk(stdout) - : { - exitCode: 1, - stdout, - stderr: `Search incomplete (${issueCount} issue${issueCount === 1 ? '' : 's'}); matches and counts cover only the content read.\n${issues.join('\n')}${issueCount > issues.length ? `\n${issueCount - issues.length} further issues omitted.` : ''}`, - } - const candidates = await source.materialize(searched, nameFilter, reportIncomplete) + let nonTextCount = 0 + const reportNonText = () => { + nonTextCount++ + } + const finish = (stdout: string) => { + const skipped = nonTextCount + ? `Skipped ${nonTextCount} file${nonTextCount === 1 ? '' : 's'} with no searchable text (images, media, archives, and other binary formats).` + : '' + const incomplete = + issueCount && + `Search incomplete (${issueCount} issue${issueCount === 1 ? '' : 's'}); matches and counts cover only the content read.\n${issues.join('\n')}${issueCount > issues.length ? `\n${issueCount - issues.length} further issues omitted.` : ''}` + return { + exitCode: issueCount ? 1 : 0, + stdout, + stderr: [incomplete, skipped].filter(Boolean).join('\n'), + } + } + const candidates = await source.materialize(searched, nameFilter, reportIncomplete, reportNonText) source.signal?.throwIfAborted() /** * A resource nothing in the searched worlds answers to is a wrong selector, not a @@ -721,8 +758,8 @@ export const universalGrepCommand: AgentCliEngine = { executeGrep(positionals, flags, { signal: runtime.signal, registry: runtime.resolvedSecretTraceRegistry, - async materialize(scopes, nameFilter, reportIncomplete) { - const bounded = gatedRuntime({ ...runtime, reportIncomplete }) + async materialize(scopes, nameFilter, reportIncomplete, reportNonText) { + const bounded = gatedRuntime({ ...runtime, reportIncomplete, reportNonText }) return nameFilter ? materializeWithin(bounded, scopes, nameFilter) : (await settleAll(scopes.map((scope) => materializeScope(bounded, scope)))).flat() diff --git a/apps/sim/lib/mothership/agent-cli/project-file-grep.test.ts b/apps/sim/lib/mothership/agent-cli/project-file-grep.test.ts index 9af8f96bfdb..df26ac7c1b9 100644 --- a/apps/sim/lib/mothership/agent-cli/project-file-grep.test.ts +++ b/apps/sim/lib/mothership/agent-cli/project-file-grep.test.ts @@ -105,6 +105,23 @@ describe('Project grep corpus', () => { expect(list).toHaveBeenCalledTimes(1) expect(result.stderr).toContain(`searched the first ${MAX_GREP_FILES} selected files`) }) + it('skips a file with no text extractor without reading it or failing the search', async () => { + list.mockResolvedValue({ + files: [ + { id: 'file', name: 'notes.txt', type: 'text/plain', folderPath: '/' }, + { id: 'clip', name: 'notes-demo.mp4', type: 'video/mp4', folderPath: '/' }, + ], + nextKeys: null, + }) + const result = await grepProjectFiles(invocation, context(), 'project') + expect(textRead).toHaveBeenCalledTimes(1) + expect(result).toEqual({ + exitCode: 0, + stdout: 'files/notes.txt (file):2: needle', + stderr: + 'Skipped 1 file with no searchable text (images, media, archives, and other binary formats).', + }) + }) it('marks truncated inventory incomplete instead of claiming a missing selector has no matches', async () => { list.mockResolvedValue({ files: [], nextKeys: [{ value: 'cursor', direction: 'asc' }] }) const result = await grepProjectFiles(invocation, context(), 'project') diff --git a/apps/sim/lib/mothership/agent-cli/project-file-grep.ts b/apps/sim/lib/mothership/agent-cli/project-file-grep.ts index 6667059df6c..f1241512921 100644 --- a/apps/sim/lib/mothership/agent-cli/project-file-grep.ts +++ b/apps/sim/lib/mothership/agent-cli/project-file-grep.ts @@ -14,6 +14,8 @@ import { admitProjectFileProvenance } from '@/lib/mothership/agent-cli/project-f import { executeCopilotProjectFileUseCase } from '@/lib/mothership/application/execute-project-file-use-case' import type { AgentCliAugmentationInvocation } from '@/lib/mothership/generated/agent-cli' import { listProjectFiles, readProjectFileText } from '@/lib/projects/files/application' +import { observedMediaType } from '@/lib/uploads/utils/file' +import { workspaceFileTextFormat } from '@/lib/workspace-files/text-format' /** Project grep materializes only freshly authorized files and imports evidence before matching. */ export function grepProjectFiles( @@ -33,7 +35,9 @@ export function grepProjectFiles( scopes: ['files'], refusal: 'Project grep supports only --scope files. Search workspace resources separately.', }, - async materialize(_scopes, nameFilter, reportIncomplete) { + async materialize(_scopes, nameFilter, reportIncomplete, reportNonText) { + // Kept so `--in` naming an unread non-text file is not an unknown selector. + const skipped: GrepMaterialized[] = [] const selectedFiles: { item: Awaited>['files'][number] label: string @@ -61,6 +65,17 @@ export function grepProjectFiles( !label.toLowerCase().includes(nameFilter) ) continue + // Same gate as the Project text read, so a skipped file is one it would refuse. + if ( + !workspaceFileTextFormat({ + name: item.name, + type: observedMediaType(item.type, item.name), + }) + ) { + reportNonText() + skipped.push({ scope: 'files', id: item.id, label, text: null }) + continue + } if (selectedFiles.length >= MAX_GREP_FILES) { reportIncomplete( `files: searched the first ${MAX_GREP_FILES} selected files; narrow with --in files/.` @@ -74,46 +89,50 @@ export function grepProjectFiles( if (pageNumber === MAX_GREP_LIST_PAGES - 1) reportIncomplete(grepListingStoppedMessage('files')) } - return mapGrepFileReads(selectedFiles, async ({ item, label }): Promise => { - context.signal?.throwIfAborted() - const target = { projectId, fileId: item.id } - let read: Awaited> - try { - read = await withGrepReadSlot( - () => - executeCopilotProjectFileUseCase( - context, - readProjectFileText, - { ...target, maxBytes: MAX_GREP_BYTES_PER_FILE, skipFolderPath: true }, - target - ), - context.signal - ) - } catch { + const searched = await mapGrepFileReads( + selectedFiles, + async ({ item, label }): Promise => { context.signal?.throwIfAborted() - reportIncomplete( - `files/${label} (${item.id}): could not read text; use files read to inspect the error.` - ) - return { scope: 'files', id: item.id, label, text: null } - } - if ( - !(await admitProjectFileProvenance( - context.resolvedSecretTraceRegistry, - read.secretProvenance - )) - ) { - reportIncomplete( - `files/${label} (${item.id}): text withheld without its secret provenance; use files read to inspect it.` - ) - return { scope: 'files', id: item.id, label, text: null } - } - if (read.degraded || read.truncated) { - reportIncomplete( - `files/${label} (${item.id}): text extraction is ${read.degraded ? 'degraded' : 'truncated'}; use files read or files get to inspect it.` - ) + const target = { projectId, fileId: item.id } + let read: Awaited> + try { + read = await withGrepReadSlot( + () => + executeCopilotProjectFileUseCase( + context, + readProjectFileText, + { ...target, maxBytes: MAX_GREP_BYTES_PER_FILE, skipFolderPath: true }, + target + ), + context.signal + ) + } catch { + context.signal?.throwIfAborted() + reportIncomplete( + `files/${label} (${item.id}): could not read text; use files read to inspect the error.` + ) + return { scope: 'files', id: item.id, label, text: null } + } + if ( + !(await admitProjectFileProvenance( + context.resolvedSecretTraceRegistry, + read.secretProvenance + )) + ) { + reportIncomplete( + `files/${label} (${item.id}): text withheld without its secret provenance; use files read to inspect it.` + ) + return { scope: 'files', id: item.id, label, text: null } + } + if (read.degraded || read.truncated) { + reportIncomplete( + `files/${label} (${item.id}): text extraction is ${read.degraded ? 'degraded' : 'truncated'}; use files read or files get to inspect it.` + ) + } + return { scope: 'files', id: item.id, label, text: read.degraded ? null : read.text } } - return { scope: 'files', id: item.id, label, text: read.degraded ? null : read.text } - }) + ) + return [...searched, ...skipped] }, }) }