Skip to content

Commit 4c2d79d

Browse files
committed
fix(mothership): read file contents through one adaptive command
1 parent 722af13 commit 4c2d79d

22 files changed

Lines changed: 1387 additions & 141 deletions
Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,252 @@
1+
/** @vitest-environment node */
2+
import type { Principal } from '@sim/auth/principal'
3+
import { PDFDocument } from 'pdf-lib'
4+
import sharp from 'sharp'
5+
import { SimApiError } from 'sim/embed'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
const mocks = vi.hoisted(() => ({ resolve: vi.fn(), artifact: vi.fn(), request: vi.fn() }))
9+
vi.mock('@/lib/workspace-files/application/resolve-workspace-file-reference', () => ({
10+
resolveWorkspaceFileReference: mocks.resolve,
11+
}))
12+
vi.mock('@/lib/workspace-files/application/read-workspace-file-artifact', () => ({
13+
readWorkspaceFileArtifact: { execute: mocks.artifact },
14+
}))
15+
16+
import { OrchestrationError } from '@/lib/core/orchestration/types'
17+
import { runEngine } from '@/lib/mothership/agent-cli/engines'
18+
import type { AgentCliRuntime } from '@/lib/mothership/agent-cli/types'
19+
import { ArtifactObservations } from '@/lib/mothership/generated/observations'
20+
import { fileOperations } from '@/lib/workspace-files/application/operations'
21+
22+
const workspaceId = '11111111-1111-4111-8111-111111111111'
23+
const fileId = '22222222-2222-4222-8222-222222222222'
24+
const principal: Principal = { kind: 'session', userId: 'reader', sessionId: 'session' }
25+
const runtime: AgentCliRuntime = {
26+
principal,
27+
workspaceId,
28+
userId: 'reader',
29+
chatId: 'trusted-chat',
30+
client: { request: mocks.request },
31+
}
32+
const file = {
33+
id: fileId,
34+
workspaceId,
35+
name: 'image.png',
36+
type: 'image/png',
37+
size: 100,
38+
folderPath: null,
39+
vfsNamespace: 'uploads',
40+
}
41+
42+
function textResponse(name = 'notes.txt') {
43+
return {
44+
data: {
45+
fileId,
46+
name,
47+
path: `uploads/${name}`,
48+
type: 'text/plain',
49+
text: 'read content',
50+
truncated: false,
51+
degraded: false,
52+
degradedReason: null,
53+
charCount: 12,
54+
byteCount: 12,
55+
},
56+
}
57+
}
58+
59+
describe('content-aware files read augmentation', () => {
60+
beforeEach(async () => {
61+
vi.clearAllMocks()
62+
mocks.resolve.mockResolvedValue(file)
63+
mocks.artifact.mockResolvedValue({
64+
file,
65+
contentType: 'image/png',
66+
buffer: await sharp({ create: { width: 2, height: 2, channels: 3, background: 'red' } })
67+
.png()
68+
.toBuffer(),
69+
})
70+
mocks.request.mockResolvedValue(textResponse())
71+
})
72+
73+
it.each(['image/png', 'application/octet-stream', 'image/png; charset=binary'])(
74+
'reads uploaded PNG with %s on its first call, without a text request',
75+
async (type) => {
76+
mocks.resolve.mockResolvedValue({ ...file, type })
77+
const result = await runEngine('files read', ['uploads/image.png'], runtime, {})
78+
expect(result.exitCode).toBe(0)
79+
expect(JSON.parse(result.stdout)).toMatchObject({
80+
fileId,
81+
representation: 'visual',
82+
mediaType: 'image/png',
83+
truncated: false,
84+
})
85+
expect(ArtifactObservations.parse(result.observations)[0]?.data).toBeTruthy()
86+
expect(result.resources?.[0]).toMatchObject({
87+
readOnly: true,
88+
resource: { id: fileId, type: 'file' },
89+
})
90+
expect(mocks.request).not.toHaveBeenCalled()
91+
expect(mocks.resolve).toHaveBeenCalledWith({
92+
principal,
93+
operation: fileOperations.readContent,
94+
workspaceId,
95+
reference: 'uploads/image.png',
96+
chatId: 'trusted-chat',
97+
})
98+
expect(mocks.artifact).toHaveBeenCalledWith({
99+
principal,
100+
input: {
101+
workspaceId,
102+
reference: fileId,
103+
chatId: 'trusted-chat',
104+
maxBytes: 25 * 1024 * 1024,
105+
},
106+
})
107+
}
108+
)
109+
110+
it.each([
111+
['notes.txt', 'text/plain'],
112+
['data.csv', 'text/csv'],
113+
['rows.xlsx', 'application/octet-stream'],
114+
['document.docx', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'],
115+
['slides.pptx', 'application/octet-stream'],
116+
['document.odt', 'application/octet-stream'],
117+
['main.ts', 'text/typescript'],
118+
['script', 'application/javascript'],
119+
['report.docx', 'text/x-docxjs'],
120+
])('reads %s through the existing text/provenance transport', async (name, type) => {
121+
mocks.resolve.mockResolvedValue({ ...file, name, type })
122+
mocks.request.mockResolvedValue(textResponse(name))
123+
const result = await runEngine('files read', [`uploads/${name}`], runtime, {
124+
offset: '3',
125+
limit: '2',
126+
'max-bytes': '4096',
127+
})
128+
expect(result.exitCode).toBe(0)
129+
expect(JSON.parse(result.stdout)).toMatchObject({
130+
representation: 'text',
131+
text: 'read content',
132+
degraded: false,
133+
})
134+
expect(mocks.request).toHaveBeenCalledWith(`/api/v2/files/${fileId}/text`, {
135+
query: { workspaceId, offset: '3', limit: '2', maxBytes: '4096' },
136+
})
137+
expect(mocks.artifact).not.toHaveBeenCalled()
138+
})
139+
140+
it.each(['application/pdf', 'text/x-pdflibjs'])(
141+
'reads %s PDF visually by default and text for a line request',
142+
async (type) => {
143+
const pdf = await PDFDocument.create()
144+
for (let i = 0; i < 23; i++) pdf.addPage()
145+
const pdfFile = { ...file, name: 'report.pdf', type }
146+
mocks.resolve.mockResolvedValue(pdfFile)
147+
mocks.artifact.mockResolvedValue({
148+
file: pdfFile,
149+
buffer: Buffer.from(await pdf.save()),
150+
contentType: 'application/pdf',
151+
})
152+
const visual = await runEngine('files read', ['report.pdf'], runtime, {})
153+
expect(visual.exitCode).toBe(0)
154+
expect(JSON.parse(visual.stdout)).toMatchObject({
155+
representation: 'visual',
156+
truncated: true,
157+
pages: { first: 1, last: 20, total: 23 },
158+
})
159+
expect(visual.observations?.[0]?.pageCount).toBe(20)
160+
const text = await runEngine('files read', ['report.pdf'], runtime, { limit: '50' })
161+
expect(text.exitCode).toBe(0)
162+
expect(JSON.parse(text.stdout).representation).toBe('text')
163+
}
164+
)
165+
166+
it.each([
167+
['clip.mp4', 'video/mp4'],
168+
['audio.mp3', 'audio/mpeg'],
169+
['archive.zip', 'application/zip'],
170+
['bytes.bin', 'application/octet-stream'],
171+
])(
172+
'returns honest metadata for %s without pretending to read binary bytes',
173+
async (name, type) => {
174+
mocks.resolve.mockResolvedValue({ ...file, name, type })
175+
const result = await runEngine('files read', [name], runtime, {})
176+
expect(result.exitCode).toBe(0)
177+
expect(JSON.parse(result.stdout)).toMatchObject({
178+
fileId,
179+
representation: 'binary',
180+
contentAvailable: false,
181+
note: expect.stringContaining('Content was not inspected'),
182+
})
183+
expect(result.observations).toBeUndefined()
184+
expect(mocks.artifact).not.toHaveBeenCalled()
185+
expect(mocks.request).not.toHaveBeenCalled()
186+
}
187+
)
188+
189+
it('rejects incompatible flags and invalid byte/line bounds before lookup', async () => {
190+
for (const flags of [
191+
{ render: true, limit: '2' } as const,
192+
{ pages: '1', offset: '1' },
193+
{ offset: '0' },
194+
{ offset: true } as const,
195+
{ limit: true } as const,
196+
{ 'max-bytes': true } as const,
197+
{ 'max-bytes': '999999999' },
198+
]) {
199+
expect((await runEngine('files read', ['report.pdf'], runtime, flags)).exitCode).toBe(1)
200+
}
201+
expect(mocks.resolve).not.toHaveBeenCalled()
202+
})
203+
204+
it('does not bypass denied canonical access or opaque provenance and hides unexpected failures', async () => {
205+
mocks.resolve.mockRejectedValueOnce(new OrchestrationError('forbidden', 'Access denied'))
206+
expect((await runEngine('files read', ['image.png'], runtime, {})).stderr).toContain(
207+
'Access denied'
208+
)
209+
expect(mocks.artifact).not.toHaveBeenCalled()
210+
mocks.artifact.mockRejectedValueOnce(
211+
new OrchestrationError('forbidden', 'Opaque file provenance is unsafe')
212+
)
213+
const denied = await runEngine('files read', ['image.png'], runtime, {})
214+
expect(denied.stderr).toContain('provenance')
215+
expect(denied.observations).toBeUndefined()
216+
mocks.resolve.mockRejectedValueOnce(new Error('private database connection string'))
217+
expect((await runEngine('files read', ['image.png'], runtime, {})).stderr).not.toContain(
218+
'connection string'
219+
)
220+
})
221+
222+
it('preserves actionable text extraction errors from the authenticated CLI client', async () => {
223+
mocks.resolve.mockResolvedValue({
224+
...file,
225+
name: 'report.docx',
226+
type: 'application/octet-stream',
227+
})
228+
mocks.request.mockRejectedValue(
229+
new SimApiError('Compiled artifact is pending. Retry after compilation.', 409)
230+
)
231+
expect((await runEngine('files read', ['report.docx'], runtime, {})).stderr).toContain(
232+
'Compiled artifact is pending'
233+
)
234+
})
235+
236+
it('does not silently ignore missing page values or a value on --render', async () => {
237+
for (const flags of [{ pages: true } as const, { pages: '' }, { render: 'false' }]) {
238+
expect((await runEngine('files read', ['image.png'], runtime, flags)).exitCode).toBe(1)
239+
}
240+
expect(mocks.artifact).not.toHaveBeenCalled()
241+
})
242+
243+
it('requires a principal and observes cancellation before lookup', async () => {
244+
const { principal: _principal, ...anonymous } = runtime
245+
expect((await runEngine('files read', ['image.png'], anonymous, {})).exitCode).toBe(1)
246+
const signal = AbortSignal.abort()
247+
expect(
248+
(await runEngine('files read', ['image.png'], { ...runtime, signal }, {})).exitCode
249+
).toBe(1)
250+
expect(mocks.resolve).not.toHaveBeenCalled()
251+
})
252+
})
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { v2ReadFileTextContract, v2ReadFileTextQuerySchema } from '@/lib/api/contracts/v2/files'
2+
import { fileReadFailure, readFileVisual } from '@/lib/mothership/agent-cli/engines/file-view'
3+
import { type AgentCliEngine, agentCliFail, agentCliOk } from '@/lib/mothership/agent-cli/types'
4+
import { workspaceFileVfsPath } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
5+
import {
6+
getFileExtension,
7+
getMimeTypeFromExtension,
8+
needsRenderedArtifact,
9+
resolveEffectiveMimeType,
10+
} from '@/lib/uploads/utils/file-utils'
11+
import { fileOperations } from '@/lib/workspace-files/application/operations'
12+
import { resolveWorkspaceFileReference } from '@/lib/workspace-files/application/resolve-workspace-file-reference'
13+
import { workspaceFileTextFormat } from '@/lib/workspace-files/text-format'
14+
15+
/** One agent read selects the usable representation after canonical authorization. */
16+
export const fileReadCommand: AgentCliEngine = {
17+
openReadResources: true,
18+
async execute(positionals, runtime, flags) {
19+
const reference = positionals[0]
20+
if (!reference) return agentCliFail('files read requires a workspace file reference.')
21+
if (!runtime.principal)
22+
return agentCliFail('Workspace authentication is unavailable. Retry the read.')
23+
for (const key of ['max-bytes', 'offset', 'limit']) {
24+
if (flags[key] === true) return agentCliFail(`--${key} requires a value.`)
25+
}
26+
const textRange = flags.offset !== undefined || flags.limit !== undefined
27+
const visual = flags.render !== undefined || flags.pages !== undefined
28+
if (textRange && visual)
29+
return agentCliFail('Use text line ranges or visual page options in one read, not both.')
30+
const query = v2ReadFileTextQuerySchema.safeParse({
31+
workspaceId: runtime.workspaceId,
32+
...(flags['max-bytes'] !== undefined ? { maxBytes: flags['max-bytes'] } : {}),
33+
...(flags.offset !== undefined ? { offset: flags.offset } : {}),
34+
...(flags.limit !== undefined ? { limit: flags.limit } : {}),
35+
})
36+
if (!query.success)
37+
return agentCliFail(query.error.issues.map((issue) => issue.message).join('; '))
38+
runtime.signal?.throwIfAborted()
39+
try {
40+
const file = await resolveWorkspaceFileReference({
41+
principal: runtime.principal,
42+
operation: fileOperations.readContent,
43+
workspaceId: runtime.workspaceId,
44+
reference,
45+
...(runtime.chatId !== undefined ? { chatId: runtime.chatId } : {}),
46+
})
47+
runtime.signal?.throwIfAborted()
48+
const type =
49+
(needsRenderedArtifact(file.type, file.name)
50+
? getMimeTypeFromExtension(getFileExtension(file.name))
51+
: resolveEffectiveMimeType(file.type, file.name)
52+
)
53+
.split(';')[0]
54+
?.trim()
55+
.toLowerCase() ?? ''
56+
if (visual || (!textRange && (type.startsWith('image/') || type === 'application/pdf'))) {
57+
const { metadata, ...result } = await readFileVisual(
58+
file.id,
59+
runtime,
60+
flags,
61+
query.data.maxBytes
62+
)
63+
return {
64+
...result,
65+
stdout: JSON.stringify({ ...metadata, representation: 'visual' }),
66+
}
67+
}
68+
if (workspaceFileTextFormat(file)) {
69+
const path = v2ReadFileTextContract.path.replace('[fileId]', encodeURIComponent(file.id))
70+
const params: Record<string, string> = { workspaceId: runtime.workspaceId }
71+
for (const key of ['maxBytes', 'offset', 'limit'] as const) {
72+
const value = query.data[key]
73+
if (value !== undefined) params[key] = String(value)
74+
}
75+
/** The embedded transport imports revision-bound secret provenance before exposing text. */
76+
const { data } = v2ReadFileTextContract.response.schema.parse(
77+
await runtime.client.request(path, { query: params })
78+
)
79+
runtime.signal?.throwIfAborted()
80+
return agentCliOk(JSON.stringify({ ...data, representation: 'text' }))
81+
}
82+
if (textRange)
83+
return agentCliFail(
84+
'This file has no text decoder. Omit the line range to read its available representation.'
85+
)
86+
return {
87+
...agentCliOk(
88+
JSON.stringify({
89+
fileId: file.id,
90+
name: file.name,
91+
path: workspaceFileVfsPath(file),
92+
type: file.type,
93+
representation: 'binary',
94+
bytes: file.size,
95+
contentAvailable: false,
96+
note: 'Content was not inspected: this file has no model-readable decoder. Mount its path using run_code inputs.files[].path to process the original bytes.',
97+
})
98+
),
99+
resources: [
100+
{
101+
op: 'upsert',
102+
readOnly: true,
103+
resource: { type: 'file', id: file.id, title: file.name },
104+
},
105+
],
106+
}
107+
} catch (error) {
108+
return fileReadFailure(error)
109+
}
110+
},
111+
}

0 commit comments

Comments
 (0)