mirror of
https://github.com/stablyai/orca.git
synced 2026-09-23 16:02:24 +00:00
* 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>
139 lines
5.0 KiB
Diff
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()
|
|
+ }
|
|
+ }
|
|
+ )
|