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
This commit is contained in:
Claude
2026-09-17 23:28:31 -07:00
parent 17b57bfbc4
commit 5e9614cd84
2 changed files with 24 additions and 1 deletions
@@ -317,6 +317,13 @@ export async function readFileViaStream(
}
}
)
// Why: beforeResolve is an optional hook; if a mux ever resolves without running
// it, metadata never installs and no deadline is armed. Fail instead of hanging.
.then(() => {
if (!settled && !metadataReady) {
fail(new StreamProtocolError('Metadata response resolved without stream identity'))
}
})
.catch((err) => {
fail(err as Error)
})
@@ -1,4 +1,4 @@
import { afterEach, expect, it } from 'vitest'
import { afterEach, expect, it, vi } from 'vitest'
import { SshChannelMultiplexer } from './ssh-channel-multiplexer'
import {
FileReadCapExceededError,
@@ -203,3 +203,19 @@ it('preserves the provider fallback when an older relay has no streaming method'
provider.dispose()
}
})
// Why: the metadata install moved from the mandatory resolve path to the optional
// beforeResolve hook, and the request timer is cleared before that hook runs. A mux
// that ignores the hook must fail the read, not leave it pending with no deadline.
it('fails the read when a multiplexer resolves without running beforeResolve', async () => {
const connection = createConnection()
vi.spyOn(connection.mux, 'request').mockResolvedValue({
totalSize: 10,
isBinary: false,
streamId: 7
})
await expect(readFileViaStream(connection.mux, '/no-hook.txt')).rejects.toBeInstanceOf(
StreamProtocolError
)
})