mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-29 18:46:59 +00:00
fix(desktop): dedicated reader for large remote attachments
Keep Settings-configurable preview/image loads on readFileDataUrl, and route remote non-image attach through a 256 MiB IPC so uploads are not stuck on the 16 MiB default after #73221. Co-authored-by: Börje <borje@dqsverige.se>
This commit is contained in:
parent
612b23f6c0
commit
254aeda122
11 changed files with 127 additions and 23 deletions
|
|
@ -7,11 +7,13 @@ import { pathToFileURL } from 'node:url'
|
|||
import { test } from 'vitest'
|
||||
|
||||
import {
|
||||
ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES,
|
||||
clampDataUrlReadMaxMb,
|
||||
DATA_URL_READ_DEFAULT_MAX_MB,
|
||||
dataUrlReadMaxBytesFromMb,
|
||||
DEFAULT_FETCH_TIMEOUT_MS,
|
||||
encryptDesktopSecret,
|
||||
readFileDataUrlForIpc,
|
||||
resolveDirectoryForIpc,
|
||||
resolveReadableFileForIpc,
|
||||
resolveRequestedPathForIpc,
|
||||
|
|
@ -35,6 +37,41 @@ test('clampDataUrlReadMaxMb defaults and bounds the attach size preference', ()
|
|||
assert.equal(dataUrlReadMaxBytesFromMb(16), 16 * 1024 * 1024)
|
||||
})
|
||||
|
||||
test('attachment upload cap is bounded above the preview default', () => {
|
||||
assert.equal(ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES, 256 * 1024 * 1024)
|
||||
assert.ok(ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES > dataUrlReadMaxBytesFromMb(DATA_URL_READ_DEFAULT_MAX_MB))
|
||||
})
|
||||
|
||||
test('attachment data URL helper reads bytes above the preview default without changing that limit', async () => {
|
||||
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hermes-desktop-large-attachment-'))
|
||||
const source = path.join(tempDir, 'large.bin')
|
||||
const previewLimit = dataUrlReadMaxBytesFromMb(DATA_URL_READ_DEFAULT_MAX_MB)
|
||||
const content = Buffer.alloc(previewLimit + 1024, 0x5a)
|
||||
|
||||
try {
|
||||
fs.writeFileSync(source, content)
|
||||
|
||||
await assert.rejects(
|
||||
resolveReadableFileForIpc(source, {
|
||||
maxBytes: previewLimit,
|
||||
purpose: 'File preview'
|
||||
}),
|
||||
/file is too large/
|
||||
)
|
||||
|
||||
const dataUrl = await readFileDataUrlForIpc(source, {
|
||||
maxBytes: ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES,
|
||||
mimeType: 'application/octet-stream',
|
||||
purpose: 'Attachment upload'
|
||||
})
|
||||
|
||||
assert.match(dataUrl, /^data:application\/octet-stream;base64,/)
|
||||
assert.deepEqual(Buffer.from(dataUrl.slice(dataUrl.indexOf(',') + 1), 'base64'), content)
|
||||
} finally {
|
||||
fs.rmSync(tempDir, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
test('resolveTimeoutMs falls back to defaults and accepts overrides', () => {
|
||||
assert.equal(resolveTimeoutMs(undefined), DEFAULT_FETCH_TIMEOUT_MS)
|
||||
assert.equal(resolveTimeoutMs(0), DEFAULT_FETCH_TIMEOUT_MS)
|
||||
|
|
|
|||
|
|
@ -12,6 +12,10 @@ const DEFAULT_FETCH_TIMEOUT_MS = 15_000
|
|||
const DATA_URL_READ_DEFAULT_MAX_MB = 16
|
||||
const DATA_URL_READ_MIN_MAX_MB = 1
|
||||
const DATA_URL_READ_MAX_MAX_MB = 4096
|
||||
// Remote file.attach sends one base64 JSON-RPC frame. Cap the dedicated attach
|
||||
// reader so the payload still fits uvicorn's raised ws_max_size (384 MiB)
|
||||
// after base64 + framing. Preview stays on the Settings-configurable path.
|
||||
const ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES = 256 * 1024 * 1024
|
||||
const TEXT_PREVIEW_SOURCE_MAX_BYTES = 64 * 1024 * 1024
|
||||
|
||||
function clampDataUrlReadMaxMb(value) {
|
||||
|
|
@ -324,7 +328,26 @@ async function resolveReadableFileForIpc(
|
|||
return { realPath, resolvedPath, stat }
|
||||
}
|
||||
|
||||
async function readFileDataUrlForIpc(
|
||||
filePath,
|
||||
options: {
|
||||
purpose?: string
|
||||
baseDir?: fs.PathOrFileDescriptor
|
||||
fs?: typeof fs
|
||||
blockSensitive?: boolean
|
||||
maxBytes?: number
|
||||
mimeType: string
|
||||
}
|
||||
): Promise<string> {
|
||||
const fsImpl = options.fs || fs
|
||||
const { resolvedPath } = await resolveReadableFileForIpc(filePath, options)
|
||||
const data = await fsImpl.promises.readFile(resolvedPath)
|
||||
|
||||
return `data:${options.mimeType};base64,${data.toString('base64')}`
|
||||
}
|
||||
|
||||
export {
|
||||
ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES,
|
||||
clampDataUrlReadMaxMb,
|
||||
DATA_URL_READ_DEFAULT_MAX_MB,
|
||||
DATA_URL_READ_MAX_MAX_MB,
|
||||
|
|
@ -332,6 +355,7 @@ export {
|
|||
dataUrlReadMaxBytesFromMb,
|
||||
DEFAULT_FETCH_TIMEOUT_MS,
|
||||
encryptDesktopSecret,
|
||||
readFileDataUrlForIpc,
|
||||
rejectUnsafePathSyntax,
|
||||
resolveDirectoryForIpc,
|
||||
resolveReadableFileForIpc,
|
||||
|
|
|
|||
|
|
@ -115,11 +115,13 @@ import {
|
|||
switchBranch
|
||||
} from './git-worktree-ops'
|
||||
import {
|
||||
ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES,
|
||||
clampDataUrlReadMaxMb,
|
||||
DATA_URL_READ_DEFAULT_MAX_MB,
|
||||
dataUrlReadMaxBytesFromMb,
|
||||
DEFAULT_FETCH_TIMEOUT_MS,
|
||||
encryptDesktopSecret as encryptDesktopSecretStrict,
|
||||
readFileDataUrlForIpc,
|
||||
resolveReadableFileForIpc,
|
||||
resolveRequestedPathForIpc,
|
||||
resolveTimeoutMs,
|
||||
|
|
@ -10193,14 +10195,23 @@ ipcMain.handle('hermes:data-url-read-max:set', (_event, maxMb) => {
|
|||
})
|
||||
|
||||
ipcMain.handle('hermes:readFileDataUrl', async (_event, filePath) => {
|
||||
const { resolvedPath } = await resolveReadableFileForIpc(filePath, {
|
||||
return readFileDataUrlForIpc(filePath, {
|
||||
maxBytes: dataUrlReadMaxBytesFromMb(dataUrlReadMaxMb),
|
||||
mimeType: mimeTypeForPath(resolveRequestedPathForIpc(filePath, { purpose: 'File preview' })),
|
||||
purpose: 'File preview'
|
||||
})
|
||||
})
|
||||
|
||||
const data = await fs.promises.readFile(resolvedPath)
|
||||
|
||||
return `data:${mimeTypeForPath(resolvedPath)};base64,${data.toString('base64')}`
|
||||
// Remote attachment transfer is independent of the preview / Settings path.
|
||||
// Keep a finite cap so Electron + base64 memory stays bounded while archives
|
||||
// can exceed the default 16 MiB preview ceiling (and still fit the gateway
|
||||
// WebSocket frame limit after base64 expansion).
|
||||
ipcMain.handle('hermes:readFileDataUrlForAttach', async (_event, filePath) => {
|
||||
return readFileDataUrlForIpc(filePath, {
|
||||
maxBytes: ATTACHMENT_UPLOAD_DEFAULT_MAX_BYTES,
|
||||
mimeType: mimeTypeForPath(resolveRequestedPathForIpc(filePath, { purpose: 'Attachment upload' })),
|
||||
purpose: 'Attachment upload'
|
||||
})
|
||||
})
|
||||
|
||||
ipcMain.handle('hermes:readFileText', async (_event, filePath) => {
|
||||
|
|
|
|||
|
|
@ -98,6 +98,7 @@ contextBridge.exposeInMainWorld('hermesDesktop', {
|
|||
notify: payload => ipcRenderer.invoke('hermes:notify', payload),
|
||||
requestMicrophoneAccess: () => ipcRenderer.invoke('hermes:requestMicrophoneAccess'),
|
||||
readFileDataUrl: filePath => ipcRenderer.invoke('hermes:readFileDataUrl', filePath),
|
||||
readFileDataUrlForAttach: filePath => ipcRenderer.invoke('hermes:readFileDataUrlForAttach', filePath),
|
||||
dataUrlReadMax: {
|
||||
get: () => ipcRenderer.invoke('hermes:data-url-read-max:get'),
|
||||
set: maxMb => ipcRenderer.invoke('hermes:data-url-read-max:set', maxMb)
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import type { AppendMessage } from '@assistant-ui/react'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { ChatMessage } from '@/lib/chat-messages'
|
||||
|
||||
|
|
@ -12,6 +12,7 @@ import {
|
|||
isSessionBusyError,
|
||||
isSessionIdCandidate,
|
||||
isSessionNotFoundError,
|
||||
readFileDataUrlForAttach,
|
||||
renderRpcResult,
|
||||
slashStatusText,
|
||||
visibleUserIndexAtOrdinal,
|
||||
|
|
@ -86,6 +87,32 @@ describe('friendlyRemoteAttachError', () => {
|
|||
})
|
||||
})
|
||||
|
||||
describe('readFileDataUrlForAttach', () => {
|
||||
it('prefers the attachment-specific desktop reader over the preview reader', async () => {
|
||||
const previewReader = vi.fn(async () => 'preview')
|
||||
const attachmentReader = vi.fn(async () => 'data:application/zip;base64,UEs=')
|
||||
Object.defineProperty(window, 'hermesDesktop', {
|
||||
configurable: true,
|
||||
value: { readFileDataUrl: previewReader, readFileDataUrlForAttach: attachmentReader }
|
||||
})
|
||||
|
||||
await expect(readFileDataUrlForAttach('/tmp/archive.zip')).resolves.toBe('data:application/zip;base64,UEs=')
|
||||
expect(attachmentReader).toHaveBeenCalledWith('/tmp/archive.zip')
|
||||
expect(previewReader).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('falls back to the preview reader on older shells', async () => {
|
||||
const previewReader = vi.fn(async () => 'data:text/plain;base64,YQ==')
|
||||
Object.defineProperty(window, 'hermesDesktop', {
|
||||
configurable: true,
|
||||
value: { readFileDataUrl: previewReader }
|
||||
})
|
||||
|
||||
await expect(readFileDataUrlForAttach('/tmp/note.txt')).resolves.toBe('data:text/plain;base64,YQ==')
|
||||
expect(previewReader).toHaveBeenCalledWith('/tmp/note.txt')
|
||||
})
|
||||
})
|
||||
|
||||
describe('slashStatusText', () => {
|
||||
it('joins command and trimmed output', () => {
|
||||
expect(slashStatusText('/model', ' gpt ')).toBe('slash:/model\ngpt')
|
||||
|
|
|
|||
|
|
@ -155,8 +155,10 @@ export async function readImageForRemoteAttach(
|
|||
|
||||
// Read a non-image file as a data URL for upload via file.attach. Returns null
|
||||
// when the desktop bridge can't read the file (e.g. it was moved/deleted).
|
||||
// Prefer the attach-specific IPC (256 MiB) so remote uploads are not stuck on
|
||||
// the preview/Settings default; fall back for older Electron shells.
|
||||
export async function readFileDataUrlForAttach(filePath: string): Promise<string | null> {
|
||||
const reader = window.hermesDesktop?.readFileDataUrl
|
||||
const reader = window.hermesDesktop?.readFileDataUrlForAttach ?? window.hermesDesktop?.readFileDataUrl
|
||||
|
||||
if (!reader) {
|
||||
return null
|
||||
|
|
@ -167,13 +169,12 @@ export async function readFileDataUrlForAttach(filePath: string): Promise<string
|
|||
return dataUrl || null
|
||||
}
|
||||
|
||||
// The readFileDataUrl IPC base64-loads the whole file into memory and is
|
||||
// hard-capped in the main process (default 16 MB; Settings → Chat), which
|
||||
// rejects with a raw "file is too large (N bytes; limit M bytes)" string. In
|
||||
// remote mode every attachment's bytes go through that read, so a big file
|
||||
// surfaces that internal message verbatim in the failure toast. Translate it
|
||||
// into a friendly "too large to upload to the remote gateway" line, parsing the
|
||||
// limit out of the message so it tracks the real cap. Non-cap errors pass
|
||||
// The attach/preview IPC base64-loads the whole file into memory and rejects
|
||||
// with a raw "file is too large (N bytes; limit M bytes)" string when over
|
||||
// cap. In remote mode every attachment's bytes go through that read, so a big
|
||||
// file surfaces that internal message verbatim in the failure toast. Translate
|
||||
// it into a friendly "too large to upload to the remote gateway" line, parsing
|
||||
// the limit out of the message so it tracks the real cap. Non-cap errors pass
|
||||
// through unchanged.
|
||||
export function friendlyRemoteAttachError(err: unknown, label: string): Error {
|
||||
const message = err instanceof Error ? err.message : String(err)
|
||||
|
|
|
|||
2
apps/desktop/src/global.d.ts
vendored
2
apps/desktop/src/global.d.ts
vendored
|
|
@ -114,6 +114,8 @@ declare global {
|
|||
notify: (payload: HermesNotification) => Promise<boolean>
|
||||
requestMicrophoneAccess: () => Promise<boolean>
|
||||
readFileDataUrl: (filePath: string) => Promise<string>
|
||||
/** Remote non-image attach: higher dedicated cap than preview/Settings default. */
|
||||
readFileDataUrlForAttach?: (filePath: string) => Promise<string>
|
||||
/** Settings → Chat: max size for local files loaded as data URLs (attach/preview). */
|
||||
dataUrlReadMax?: {
|
||||
get: () => Promise<{ defaultMaxMb: number; maxBytes: number; maxMb: number }>
|
||||
|
|
|
|||
|
|
@ -550,11 +550,11 @@ export const en: Translations = {
|
|||
invalidJson: 'Invalid config JSON',
|
||||
keepAwakeTitle: 'Keep computer awake',
|
||||
keepAwakeDesc: 'Stop this machine from sleeping so long or overnight runs keep going. The display can still dim.',
|
||||
attachmentSizeTitle: 'Max attachment size',
|
||||
attachmentSizeTitle: 'Max preview / image load size',
|
||||
attachmentSizeDesc:
|
||||
'How big a local file Desktop will load for attach and previews, in MB. Default is 16. This is only a limit on this computer. Setting it very high loads the whole file into memory and can freeze or crash the app.',
|
||||
'How big a local file Desktop will load for previews and image attach, in MB. Default is 16. Remote non-image attach uses a separate 256 MB cap. Setting this very high loads the whole file into memory and can freeze or crash the app.',
|
||||
attachmentSizeUnit: 'MB',
|
||||
attachmentSizeLabel: 'Max attachment size in megabytes'
|
||||
attachmentSizeLabel: 'Max preview / image load size in megabytes'
|
||||
},
|
||||
quickEntry: {
|
||||
enabledTitle: 'Quick Entry',
|
||||
|
|
|
|||
|
|
@ -760,11 +760,11 @@ export const zh: Translations = {
|
|||
invalidJson: '配置 JSON 无效',
|
||||
keepAwakeTitle: '保持电脑唤醒',
|
||||
keepAwakeDesc: '阻止本机休眠,让长时间或通宵运行继续进行。屏幕仍可变暗。',
|
||||
attachmentSizeTitle: '附件大小上限',
|
||||
attachmentSizeTitle: '预览 / 图片加载大小上限',
|
||||
attachmentSizeDesc:
|
||||
'桌面端为附件和预览加载本地文件的大小上限(MB)。默认为 16。此限制仅作用于本机。设置过大会将整个文件读入内存,可能导致应用卡死或崩溃。',
|
||||
'桌面端为预览和图片附件加载本地文件的大小上限(MB)。默认为 16。远程非图片附件使用单独的 256 MB 上限。设置过大会将整个文件读入内存,可能导致应用卡死或崩溃。',
|
||||
attachmentSizeUnit: 'MB',
|
||||
attachmentSizeLabel: '附件大小上限(MB)'
|
||||
attachmentSizeLabel: '预览 / 图片加载大小上限(MB)'
|
||||
},
|
||||
quickEntry: {
|
||||
enabledTitle: '快速输入',
|
||||
|
|
|
|||
|
|
@ -120,7 +120,7 @@ describe('reportBackendContract', () => {
|
|||
})
|
||||
|
||||
it('dismisses the toast when the backend meets the contract', () => {
|
||||
reportBackendContract(4)
|
||||
reportBackendContract(5)
|
||||
expect(dismissSpy).toHaveBeenCalledWith('backend-contract-skew')
|
||||
expect(notifySpy).not.toHaveBeenCalled()
|
||||
})
|
||||
|
|
@ -160,8 +160,8 @@ describe('reportBackendContract', () => {
|
|||
lastToast().onDismiss()
|
||||
notifySpy.mockClear()
|
||||
|
||||
reportBackendContract(4) // backend updated → satisfied, snooze cleared
|
||||
reportBackendContract(3) // a later regression must warn immediately
|
||||
reportBackendContract(5) // backend updated → satisfied, snooze cleared
|
||||
reportBackendContract(4) // a later regression must warn immediately
|
||||
expect(notifySpy).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -93,7 +93,8 @@ function isUpdateToastSnoozed(): boolean {
|
|||
// v2: requires the file.attach RPC (remote-gateway non-image file upload).
|
||||
// v3: requires approvals.mode config RPCs and session.info reconciliation.
|
||||
// v4: requires explicit Fast-off session creation and session-scoped Fast edits.
|
||||
const REQUIRED_BACKEND_CONTRACT = 4
|
||||
// v5: requires raised WebSocket frame size for large one-shot file.attach.
|
||||
const REQUIRED_BACKEND_CONTRACT = 5
|
||||
const SKEW_TOAST_ID = 'backend-contract-skew'
|
||||
// The contract check runs on every session.resume (applyRuntimeInfo), so
|
||||
// without a snooze the warning re-popped on every thread the user opened, even
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue