Merge pull request #73710 from NousResearch/bb/triage-large-remote-attach

fix(desktop): allow large remote attachments (supersedes #66555)
This commit is contained in:
brooklyn! 2026-07-28 19:28:01 -05:00 committed by GitHub
commit 6cccef6c10
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 151 additions and 24 deletions

View file

@ -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)

View file

@ -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,

View file

@ -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) => {

View file

@ -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)

View file

@ -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')

View file

@ -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)

View file

@ -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 }>

View file

@ -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',

View file

@ -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: '快速输入',

View file

@ -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)
})
})

View file

@ -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

View file

@ -318,6 +318,13 @@ def _apply_ssh_owner_nonce(nonce: Optional[str]) -> None:
# injection share a single, testable seam.
_DASHBOARD_EMBEDDED_CHAT_ENABLED = True
# Desktop's file.attach compatibility transport sends a complete base64 data
# URL in one JSON-RPC frame. Uvicorn defaults to 16 MiB, which rejects files at
# the preview ceiling before the dispatcher sees them. Keep the gateway
# finite while allowing the 256 MiB raw Desktop attach cap plus base64/JSON
# overhead.
_DESKTOP_ATTACHMENT_WS_MAX_BYTES = 384 * 1024 * 1024
# Simple rate limiter for the reveal endpoint
_reveal_timestamps: List[float] = []
_REVEAL_MAX_PER_WINDOW = 5
@ -20290,6 +20297,7 @@ def start_server(
# reaped via the WebSocketDisconnect → disconnect/reap path.
ws_ping_interval=None if _is_loopback else 20.0,
ws_ping_timeout=None if _is_loopback else 20.0,
ws_max_size=_DESKTOP_ATTACHMENT_WS_MAX_BYTES,
)
server = uvicorn.Server(config)

View file

@ -107,6 +107,20 @@ def test_start_server_disables_ws_ping_on_loopback(monkeypatch):
assert captured["ws_ping_timeout"] is None
def test_start_server_accepts_base64_desktop_attachments_above_preview_limit(monkeypatch):
"""The gateway frame cap must fit the Desktop attachment default after
base64 expansion and JSON framing; uvicorn's 16 MiB default would reject
the request before ``file.attach`` can stage it.
"""
captured = _stub_uvicorn(monkeypatch)
web_server.start_server(host="127.0.0.1", port=0, open_browser=False)
raw_attachment_bytes = 256 * 1024 * 1024
base64_bytes = ((raw_attachment_bytes + 2) // 3) * 4
assert captured["ws_max_size"] > base64_bytes
def test_start_server_enables_ws_ping_for_half_open_detection(monkeypatch):
"""Non-loopback (public) binds MUST keep the ws ping enabled so half-open
connections (reverse-proxy 524, dropped Cloudflare Tunnel) raise

View file

@ -4528,7 +4528,8 @@ def _current_profile_name() -> str:
# v2: adds the file.attach RPC (remote-gateway non-image file upload).
# v3: adds approvals.mode config RPCs and session.info reconciliation.
# v4: session.create fast=false is an explicit per-session normal-tier override.
DESKTOP_BACKEND_CONTRACT = 4
# v5: uvicorn ws_max_size raised for one-shot base64 file.attach frames (>16 MiB).
DESKTOP_BACKEND_CONTRACT = 5
def _session_usage_snapshot(session: dict | None) -> dict: