From 5e9614cd847f8ff4b6857e7e96b85da76bc06822 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 23:27:09 -0700 Subject: [PATCH] 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 --- src/main/ssh/ssh-filesystem-stream-reader.ts | 7 +++++++ .../ssh-filesystem-stream-retention.test.ts | 18 +++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/main/ssh/ssh-filesystem-stream-reader.ts b/src/main/ssh/ssh-filesystem-stream-reader.ts index 9743005a466..8c5b8632fa7 100644 --- a/src/main/ssh/ssh-filesystem-stream-reader.ts +++ b/src/main/ssh/ssh-filesystem-stream-reader.ts @@ -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) }) diff --git a/src/main/ssh/ssh-filesystem-stream-retention.test.ts b/src/main/ssh/ssh-filesystem-stream-retention.test.ts index 1614afa3910..15400212f78 100644 --- a/src/main/ssh/ssh-filesystem-stream-retention.test.ts +++ b/src/main/ssh/ssh-filesystem-stream-retention.test.ts @@ -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 + ) +})