Commit Graph
2 Commits
Author SHA1 Message Date
Neil 99d9111653 fix(relay): fail an over-budget RPC response, not the connection (#17968)
The relay's control lane is a shared 1 MiB budget, and `sendResponse` admitted
responses onto it with the fatal default: once the lane was full, admission
closed the client. A ~900 KB `fs.listFiles` reply from a large remote workspace
therefore took down the whole remote session -- every terminal on it -- rather
than failing the one Quick Open request. The substitute `ResponseOverCapacity`
frame already there only covered the `legacy-response` lane, because the fatal
close beat it to the client.

A JSON-RPC response is the droppable class of control frame: it carries an id,
so one caller can be told and can retry. `pty.replay` and `notifyControl` keep
the fatal default -- they are never re-sent, and a silent drop there desyncs the
client with nothing to retry. Both response enqueues now pass
`controlOverflow: 'reject'`, so the substitute error is what the caller sees;
in the corner where even ~150 bytes will not fit, the caller's own 30s request
timeout settles it and the session survives.

Old clients are unaffected: they already decode this error code and message
generically (`ssh-channel-multiplexer.handleResponse` rejects the pending
promise with both), and the frame shape is unchanged. What changes is that a
listing which used to drop the connection now returns an error on it.
2026-09-02 01:29:26 -07:00
Brennan Benson 6415511a82 feat(remote): add a positional file read to the filesystem provider (#15517)
* feat(remote): add a positional file read to the filesystem provider

Following a growing remote file means re-reading it from the top on every
poll: the relay exposes only whole-file reads, so tailing an append-only log
over SSH costs O(size) per tick.

Adds fs.readFileRange plus a rangedReadVersion capability, and an optional
readFileRange on IFilesystemProvider -- matching how lstat/
supportsQuickOpenSearch already declare degradable capabilities.

Three deliberate choices:

- The relay loops until the requested length is satisfied or the file truly
  ends, and REJECTS an over-cap request rather than clamping it. A clamped
  read is indistinguishable from EOF, so a caller advancing a cursor by
  bytesRead would silently skip data.
- Bytes cross the wire base64-encoded. A range boundary can split a UTF-8
  sequence at either edge, and a utf-8 round trip would substitute U+FFFD and
  shift every subsequent offset.
- The provider throws a typed FileRangeReadUnsupportedError against an older
  relay instead of quietly falling back to a whole-file read. A tailing caller
  issues several reads per snapshot, so a per-call fallback is quadratic;
  callers probe supportsFileRangeRead once and snapshot instead.

The response is validated before use -- a byte count disagreeing with the
payload would shift every downstream offset while looking like success.

Terminal-artifact reads/writes move to their own module, mirroring the relay's
existing fs-handler-terminal-artifact split; the provider was at the max-lines
ceiling and this was the cohesive piece to extract.

* fix(remote): size the ranged read to what the relay writer can deliver

The 4 MiB cap was justified against MAX_MESSAGE_SIZE (16 MiB), but that is
the frame DECODER bound. Responses are gated by the writer's admission
budget: a frame over DISPATCHER_CONTROL_QUEUE_MAX_BYTES (1 MiB) is demoted
to the legacy-response lane, which is refused once the producer queue passes
2 MiB. A 4 MiB window is ~5.46 MiB of base64, so it was never admissible --
it came back as an opaque ResponseOverCapacity (-33008), which is neither of
the PR's typed errors, and above ~1.4 MiB the outcome depended on unrelated
queued traffic. Cap at STREAM_CHUNK_SIZE (256 KiB), the house per-frame
budget for file bytes, which stays in the control lane unconditionally.

Also:
- Hoist the cap and offset validation into src/shared/file-range-read.ts so
  the client rejects an out-of-contract request locally instead of paying a
  round trip for an error that does not survive the wire as a type.
- Validate filePath in the relay handler; a missing one threw a TypeError
  out of expandTilde despite the comment claiming hand-validated params.
- Collapse the two fs.getCapabilities probes onto one cached fetch per
  multiplexer. They read one document, so probing per feature spent an extra
  round trip per connection and duplicated the eviction logic.
- Reuse readFullStreamChunk instead of a second copy of the short-read fill.
- allocUnsafe the window; only subarray(0, bytesRead) escapes, so a tailing
  poll no longer memsets the whole window per call.
- Plain methods for readFileRange/supportsFileRangeRead rather than
  constructor-assigned arrows; both are unconditional, unlike downloadFolder.

Tests: cover the dispatch path and fs.getCapabilities (neither was
exercised), param validation at both boundaries, EOF at and past the end,
and a full-cap read over a real RelayDispatcher. The transport guard fails
at 4 MiB with the real -33008.

* test(remote): pin the ranged-read cap to real control-queue headroom

The cap comment claimed a full-cap window stays in the control lane
"unconditionally" and the guard test only asserted one frame fits the
lane, so a raise to 384-768 KiB stayed green while two concurrent
full-cap responses would already overflow the shared control queue --
which for a response closes the client. Pin the two-deep headroom and
state the real bound, including that widening the cap is a wire change
against a host still advertising rangedReadVersion 1.

Also cover the two behaviours the suite claimed but did not exercise: a
regular file answers a full-cap read in one syscall, so the fill loop
was untested (both mutations of readFullStreamChunk stayed green), and
the merged capability document made the abort-does-not-evict guard
load-bearing without any test reaching it.

* fix(remote): harden ranged-read validation and retry
2026-08-19 16:45:42 -07:00