Files
2bdf281433 fix: avoid retaining foreign SSH file frames before metadata (#21167)
* fix: avoid retaining foreign SSH file frames before metadata

* test(ssh): exercise empty metadata through the streaming mux fixture

* fix(ssh): fail the file read when beforeResolve never runs

Moving the metadata install from .then() to beforeResolve moved it from a
mandatory callback to an optional one, and handleResponse clears the request
timer before beforeResolve runs. That left "response fulfilled, metadata never
installed" with no deadline: the read never settled, holding its notification
and dispose closures until mux disposal. Before this PR the same state failed
after the 60s inactivity deadline.

Unreachable with the concrete mux, which calls resolve on the line after
beforeResolve, but the hook is optional in the type and nothing enforces the
pairing. The guard is a no-op on every real path: empty, missing streamId,
cap-exceeded and alloc-failure all settle first, and the success path sets
metadataReady.

Found during review of #21167; raised at
https://github.com/stablyai/orca/pull/21167#issuecomment-5726058832

---------

Co-authored-by: m4air <m4air@Mac.localdomain>
Co-authored-by: Claude <noreply@anthropic.com>
2026-09-17 23:59:42 -07:00

139 lines
5.0 KiB
Diff

--- a/src/main/ssh/ssh-filesystem-stream-reader.ts
+++ b/src/main/ssh/ssh-filesystem-stream-reader.ts
@@ -77,7 +77 @@
- // Why: chunk/end/error frames may arrive in the same dispatch tick as the
- // metadata response. Queue them until streamIdRef is set, then drain.
- type PendingFrame =
- | { kind: 'chunk'; params: Record<string, unknown> }
- | { kind: 'end'; params: Record<string, unknown> }
- | { kind: 'error'; params: Record<string, unknown> }
- const pending: PendingFrame[] = []
+ // Install metadata during response dispatch, before adjacent stream frames.
@@ -232,13 +225,0 @@
- const drainPending = (): void => {
- while (!settled && pending.length > 0) {
- const frame = pending.shift()!
- if (frame.kind === 'chunk') {
- handleChunk(frame.params)
- } else if (frame.kind === 'end') {
- handleEnd(frame.params)
- } else {
- handleStreamError(frame.params)
- }
- }
- }
-
@@ -248 +228,0 @@
- pending.push({ kind: 'chunk', params })
@@ -257 +236,0 @@
- pending.push({ kind: 'end', params })
@@ -266 +244,0 @@
- pending.push({ kind: 'error', params })
@@ -287,51 +265,55 @@
- .request('fs.readFileStream', { filePath, flowControl: 'ack' })
- .then((rawMetadata) => {
- if (settled) {
- return
- }
- const metadata = rawMetadata as StreamMetadataResponse
- isBinary = metadata.isBinary
- isImage = metadata.isImage
- mimeType = metadata.mimeType
- resultEncoding = metadata.resultEncoding ?? RESULT_ENCODING_BASE64
-
- if (metadata.empty) {
- succeed({
- content: '',
- isBinary: metadata.isBinary,
- ...(metadata.isImage !== undefined ? { isImage: metadata.isImage } : {}),
- ...(metadata.mimeType !== undefined ? { mimeType: metadata.mimeType } : {})
- })
- return
- }
-
- if (typeof metadata.streamId !== 'number') {
- fail(new StreamProtocolError('Metadata missing streamId for non-empty stream'))
- return
- }
-
- const cap = sshFileStreamReadCap(metadata.isBinary, limits)
- if (metadata.totalSize < 0 || metadata.totalSize > cap) {
- streamIdRef.current = metadata.streamId
- fail(
- new FileReadCapExceededError(
- `Reported totalSize ${metadata.totalSize} exceeds client cap ${cap}`
- )
- )
- return
- }
-
- totalSize = metadata.totalSize
- totalChunks = totalSize === 0 ? 0 : Math.ceil(totalSize / STREAM_CHUNK_SIZE)
- try {
- buffer = Buffer.alloc(totalSize)
- } catch (err) {
- streamIdRef.current = metadata.streamId
- fail(new Error(`Failed to allocate ${totalSize} bytes: ${(err as Error).message}`))
- return
- }
- streamIdRef.current = metadata.streamId
- metadataReady = true
- inactivity.reset()
- drainPending()
- })
+ .request(
+ 'fs.readFileStream',
+ { filePath, flowControl: 'ack' },
+ {
+ beforeResolve: (rawMetadata) => {
+ if (settled) {
+ return
+ }
+ const metadata = rawMetadata as StreamMetadataResponse
+ isBinary = metadata.isBinary
+ isImage = metadata.isImage
+ mimeType = metadata.mimeType
+ resultEncoding = metadata.resultEncoding ?? RESULT_ENCODING_BASE64
+
+ if (metadata.empty) {
+ succeed({
+ content: '',
+ isBinary: metadata.isBinary,
+ ...(metadata.isImage !== undefined ? { isImage: metadata.isImage } : {}),
+ ...(metadata.mimeType !== undefined ? { mimeType: metadata.mimeType } : {})
+ })
+ return
+ }
+
+ if (typeof metadata.streamId !== 'number') {
+ fail(new StreamProtocolError('Metadata missing streamId for non-empty stream'))
+ return
+ }
+
+ const cap = sshFileStreamReadCap(metadata.isBinary, limits)
+ if (metadata.totalSize < 0 || metadata.totalSize > cap) {
+ streamIdRef.current = metadata.streamId
+ fail(
+ new FileReadCapExceededError(
+ `Reported totalSize ${metadata.totalSize} exceeds client cap ${cap}`
+ )
+ )
+ return
+ }
+
+ totalSize = metadata.totalSize
+ totalChunks = totalSize === 0 ? 0 : Math.ceil(totalSize / STREAM_CHUNK_SIZE)
+ try {
+ buffer = Buffer.alloc(totalSize)
+ } catch (err) {
+ streamIdRef.current = metadata.streamId
+ fail(new Error(`Failed to allocate ${totalSize} bytes: ${(err as Error).message}`))
+ return
+ }
+ streamIdRef.current = metadata.streamId
+ metadataReady = true
+ inactivity.reset()
+ }
+ }
+ )