mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
mmarabelandNeil
68f0b2e835
* feat(runtime): stream file uploads instead of buffering whole files Staging read each dropped file whole with readFile(), base64-encoded it (a 4/3 expansion), and passed the string through IPC to the renderer, which re-chunked it. Peak memory was ~2.3x the file size before a byte moved, so a 25 MB per-file cap existed to protect the heap. Staging now records identity only. The byte pump moves into main, where the file handle and the runtime socket both live: 384 KiB slices (512 KiB once base64-encoded, matching the chunk size the renderer used) appended through the existing files.writeBase64Chunk RPC. Peak memory is one slice regardless of file size, so the ceilings become user-safety limits on an unattended transfer — 2 GB per file, 8 GB per drop — and over-limit errors name both the size and the limit. Because staging and streaming are separate calls, the staged entry carries size, inode, device and mtime, and the streamer re-checks all four against the pre-open lstat and against the handle it actually reads. A source replaced or rewritten at the same size between the two calls is refused rather than uploaded under the original name. The post-read check compares mtime as well as size, so an in-place rewrite mid-transfer aborts before commitUpload renames anything into place. O_NOFOLLOW, realpath containment and stat identity are preserved, and the pairing revision plus the runtime id ride every chunk, so a re-pair or a replacement runtime aborts instead of appending the rest of the file to a different host. No wire change: files.writeBase64Chunk and its params are untouched, so old and new hosts behave identically. The SSH import path is separate and unchanged. The web client has no local filesystem to stream from and says so instead of failing obscurely. * fix(runtime): close the empty-upload and per-drop budget holes Two gaps the first pass left open. A zero-byte source returned before the post-transfer identity check, so a file that gained content during the empty write's round trip committed as an empty file at the user's chosen name. The empty chunk now falls through to the same final check the slice loop uses. Each staged source also started its own byte counter, so the 8 GB ceiling capped one source rather than the drop: five 2 GB files staged cleanly at 10 GB total. The IPC handler now carries one budget across sourcePaths and adds only what each source actually staged. The per-file ceiling is still re-enforced where the bytes move; the drop total holds at staging because identity enforcement means each file streams exactly the bytes measured. * docs(runtime): name the invariants the upload helpers carry * fix(runtime): name the source in errors and stop uploads with their window Three problems an independent review turned up. A dropped file's relative path is '', so the over-limit error read "'' is 3 GB, over the 2 GB per-file remote import limit" — the message this change exists to fix, naming nothing. Errors now fall back to the file's own name; the staged entry keeps '' so the destination path is unaffected. The streamer had the same shape, falling back to the hidden .orca-upload-<nonce> temp destination, a path the user never chose. The byte loop used to live in the renderer and died with it. Moving it into main meant closing or reloading the window left the rest of a multi-GB transfer running, with the renderer's temp cleanup never reaching its finally. An AbortSignal now rides the caller's lifetime and every chunk, is re-checked per slice, and main sweeps the abandoned temp path itself when the renderer is no longer there to do it. Upload failures also reached the import result wrapped in Electron's "Error invoking remote method '...'" prefix, because the throw crossed IPC instead of happening in-renderer; extractIpcErrorMessage unwraps it. An existing staging test asserted the empty-name message, so it encoded the bug rather than catching it; it now asserts the file name. * test(runtime): cover the containment check and the per-chunk host guards The "escapes the dropped root" test only reached the lstat symlink guard, so assertEntryInsideRoot had no coverage at all. The shape that actually needs it is a regular file under a symlinked intermediate directory: lstat sees a plain file, and realpath containment is the only thing that refuses it. Disabling the guard now fails this test and nothing else. Nothing asserted that the SSH target, connection generation and execution host reach the writeBase64Chunk params either — the renderer tests stop at the IPC boundary, so the streamer's half of that contract was untested. * fix(runtime): survive a straggling append when sweeping an aborted upload Aborting rejects the in-flight chunk locally, but the host may still apply that append, and appends open with flag 'a' — which recreates the file the sweep just deleted. The delete and the straggler also race: they are separate calls on a queue that is not ordered between them. Slices are strictly sequential, so at most one append can be outstanding. A second pass after it has had time to land is therefore sufficient, not merely a heuristic. The sweep moves out of filesystem-mutations.ts into its own module so the behaviour is testable directly. Found by an independent review pass, which also pointed out that the "escapes the dropped root" test only reached the lstat symlink guard. * fix(runtime): abort uploads only when the document commits, and honour manual disconnect per chunk did-start-navigation fires before will-navigate blocks an external link or a stray file drop, and the renderer survives those (verified against Electron 43 with a hidden window). Aborting there killed a healthy upload with a misleading 'window went away' error. did-navigate fires only once a new document has replaced the caller. The renderer's per-chunk calls used to go through the IPC handler that refuses a manually disconnected environment; the loop in main made no such check, so a disconnect mid-upload kept pushing the rest of the file. The handler now resolves the selector to an environment id and the streamer checks it per slice. Adds slice-boundary coverage against the real chunk schema and host write flags, staging-to-stream on a real filesystem, and handler-level lifetime tests. --------- Co-authored-by: Neil <neil@stably.ai>
34 lines
1.1 KiB
TypeScript
34 lines
1.1 KiB
TypeScript
import { describe, expect, it } from 'vitest'
|
|
import {
|
|
formatByteCeiling,
|
|
REMOTE_IMPORT_MAX_FILE_BYTES,
|
|
REMOTE_IMPORT_MAX_TOTAL_BYTES
|
|
} from './runtime-import-limits'
|
|
|
|
describe('formatByteCeiling', () => {
|
|
it('renders a size one byte over a ceiling as larger than the ceiling', () => {
|
|
// "is 2 GB, over the 2 GB limit" reads like a broken check, not a big file.
|
|
expect(formatByteCeiling(REMOTE_IMPORT_MAX_FILE_BYTES)).toBe('2 GB')
|
|
expect(formatByteCeiling(REMOTE_IMPORT_MAX_FILE_BYTES + 1)).toBe('2.1 GB')
|
|
})
|
|
|
|
it('leaves an exact ceiling as a whole number', () => {
|
|
expect(formatByteCeiling(REMOTE_IMPORT_MAX_TOTAL_BYTES)).toBe('8 GB')
|
|
expect(formatByteCeiling(1024)).toBe('1 KB')
|
|
})
|
|
|
|
it('scales through the units', () => {
|
|
expect(formatByteCeiling(512)).toBe('512 B')
|
|
expect(formatByteCeiling(1024 * 1024)).toBe('1 MB')
|
|
expect(formatByteCeiling(1024 ** 4)).toBe('1 TB')
|
|
})
|
|
|
|
it('rounds up rather than to nearest', () => {
|
|
expect(formatByteCeiling(1024 * 1024 + 1)).toBe('1.1 MB')
|
|
})
|
|
|
|
it('does not crash on zero', () => {
|
|
expect(formatByteCeiling(0)).toBe('0 B')
|
|
})
|
|
})
|