Merge upstream v2.22.5 - #16
Merged
Merged
Conversation
Dynamic SOCKS forwarding: - Preserve half-close semantics while streaming: client EOF closes the remote sink and remote EOF destroys the local socket instead of treating EOF as a full connection close immediately. - Guard against concurrent dial attempts for the same SOCKS request. - Cancel the handshake timer before dialing and clean up a tunnel if the connection was closed while dial was in flight. - Tolerate malformed UTF-8 domain names in SOCKS requests and cap handshake buffering at 32 KiB. SSH agent: - Validate that fallback RSA signing produces an SSHRsaSignature with the requested signature type. - Reject zero-length and oversized agent frames to avoid unbounded buffering. EC private keys: - Parse EC PRIVATE KEY curve OIDs when present. - Validate embedded EC public points by deriving the expected public key from the private scalar. - Preserve UnsupportedError for unsupported key types/curves instead of wrapping it as SSHKeyDecodeError. - Decode OpenSSH key comments with allowMalformed to tolerate malformed comments. Adds protocol tests for the dynamic forward hardening and ssh-agent frame size checks.
…ic-forward-agent-ec-key fix: harden dynamic forwarding, ssh-agent, and EC key parsing
…on-wait-for-exit-timeout feat: add SSHSession waitForExit timeout
…data remains in buffer
…rt-partial-packet-busyloop fix: prevent SSHTransport busy-loop on partial packets
Bumps [actions/checkout](https://github.com/actions/checkout) from 6 to 7. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v6...v7) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 5 to 7. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](codecov/codecov-action@v5...v7) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
…github_actions/codecov/codecov-action-7 chore(deps): bump codecov/codecov-action from 5 to 7
…github_actions/actions/checkout-7 chore(deps): bump actions/checkout from 6 to 7
…t-handshake-auth-timeouts feat: add SSHClient handshake and auth timeouts
…bustness fix(keepalive): prevent overlapping pings and catch errors
…t, channels, and sessions
…bustness Add flush() method to SSHSocket, SSHClient, and SSHChannel
SftpClient.close() completed its internal futures but never closed the SSH channel the sftp subsystem runs on. Because SSHClient.sftp() opens a fresh session channel on every call, an application that opens an sftp session per operation leaks one open channel each time. These accumulate on the connection until the server refuses further CHANNEL_OPENs (e.g. CHANNEL_OPEN_FAILURE / a per-connection session limit). Close the channel when the session is closed, make close() async so callers can await the teardown, and guard against double-close so the method is idempotent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e-channel fix(sftp): close the underlying channel in SftpClient.close()
The kex is offered under two names: the original curve25519-sha256@libssh.org and the RFC 8731 name curve25519-sha256. Only the former was advertised, so a server hardened down to a single kex (KexAlgorithms curve25519-sha256, a common hardening recipe) shares no name with us and the handshake fails with "no matching key exchange method found" — even though both sides implement the very same X25519 exchange. Add SSHKexType.x25519Rfc with the RFC name, offer it right after the libssh.org spelling, and route it to the same SSHKexX25519 in the transport switch. No wire-format change: same digest, same exchange.
…25519-sha256-alias Advertise the RFC 8731 kex name curve25519-sha256
…serauth fix: export ssh_userauth.dart in public API (TerminalStudio#188)
Merge TerminalStudio/dartssh2 v2.22.5 into the fork while preserving the fork's AEAD, rekey, SFTP, channel flow-control, hostbased authentication, and Windows socket behavior. Resolve the timeout, flush, session exit, transport scheduling, and SFTP close conflicts against the upstream implementations.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
🚧 Files skipped from review as they are similar to previous changes (11)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🔇 Additional comments (2)
📝 WalkthroughWalkthroughThe release adds RFC 8731 key exchange support, asynchronous flush propagation, opt-in client timeouts, session exit completion, SFTP cleanup, transport scheduling, forwarding cleanup, keepalive handling, tests, documentation, and release metadata. ChangesSSH lifecycle and protocol updates
Sequence Diagram(s)sequenceDiagram
participant SSHClient
participant SSHTransport
participant SSHSocket
participant SSHChannel
participant SSHSession
SSHClient->>SSHTransport: flush()
SSHTransport->>SSHSocket: flush()
SSHClient->>SSHChannel: configure onFlush callback
SSHSession->>SSHChannel: flush()
SSHChannel-->>SSHSession: complete queued data barrier
Suggested reviewers: Comment |
CI failure root-cause analysisRoot cause cannot be determined from the available evidence: both failed Verifiable fix Collect the unstructured failure output, stack traces, or CI logs for jobs Incremental value: root cause, verifiable fix; confidence 1%. Passing CI ≠ absence of defects (§29.4). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/src/ssh_client.dart (1)
128-133: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClarify timeout constant documentation.
defaultAuthTimeoutanddefaultHandshakeTimeoutare not applied automatically.handshakeTimeoutandauthTimeoutremainnullunless callers provide values. Document these constants as opt-in presets, or reference them from the corresponding field documentation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/src/ssh_client.dart` around lines 128 - 133, Update the documentation for defaultAuthTimeout and defaultHandshakeTimeout to state that they are opt-in presets and are not applied automatically; clarify that authTimeout and handshakeTimeout remain null unless callers explicitly provide values, or reference these constants from the corresponding field documentation.
🧹 Nitpick comments (6)
test/src/algorithm/ssh_cipher_type_test.dart (1)
108-122: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the RFC name, not only the constant identity.
This assertion can pass even if
SSHKexType.x25519Rfc.nameis incorrect. Add an assertion forcurve25519-sha256. Add a negotiation test for a peer that offers only this name when an integration harness is available. The transport selects and serializes KEX algorithms by name, so the literal name is the compatibility contract. (raw.githubusercontent.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/algorithm/ssh_cipher_type_test.dart` around lines 108 - 122, Extend the KEX assertions around SSHKexType.x25519Rfc to verify that its name is exactly “curve25519-sha256”, not just that the constant appears in algorithms.kex. If the existing integration harness supports it, add a negotiation case where the peer offers only that literal name and confirm it is selected and serialized.test/src/ssh_flush_test.dart (1)
1-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
dart:mirrorsto call a private method; expose a@visibleForTestingseam instead.
lib/src/ssh_client.dartalready exposeshandlePacketvia@visibleForTestingfor this exact purpose (calling a private method from tests without reflection). Usingdart:mirrorshere instead is inconsistent with that established pattern, is fragile against renames, and is unsupported on Dart web/Wasm compilation targets.Add a
@visibleForTestingwrapper for_acceptChannelinSSHClient, similar tohandlePacket, and call it directly instead of through reflection.♻️ Proposed refactor
+ `@visibleForTesting` + SSHChannelController acceptChannelForTesting({ + required SSHChannelId localChannelId, + required SSHChannelId remoteChannelId, + required int remoteInitialWindowSize, + required int remoteMaximumPacketSize, + }) => + _acceptChannel( + localChannelId: localChannelId, + remoteChannelId: remoteChannelId, + remoteInitialWindowSize: remoteInitialWindowSize, + remoteMaximumPacketSize: remoteMaximumPacketSize, + );-import 'dart:mirrors'; ... - final clientLibrary = reflectClass(SSHClient).owner as LibraryMirror; - Symbol privateSymbol(String name) => - MirrorSystem.getSymbol(name, clientLibrary); - - // Invoke _acceptChannel using reflection to verify onFlush setup. - final channelController = reflect(client).invoke( - privateSymbol('_acceptChannel'), - [], - { - `#localChannelId`: 1, - `#remoteChannelId`: 2, - `#remoteInitialWindowSize`: 1024, - `#remoteMaximumPacketSize`: 1024, - }, - ).reflectee as SSHChannelController; + final channelController = client.acceptChannelForTesting( + localChannelId: 1, + remoteChannelId: 2, + remoteInitialWindowSize: 1024, + remoteMaximumPacketSize: 1024, + );Also applies to: 39-67
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/ssh_flush_test.dart` around lines 1 - 8, Add a `@visibleForTesting` wrapper for SSHClient._acceptChannel, following the existing handlePacket testing seam, then update ssh_flush_test.dart to call that wrapper directly and remove the dart:mirrors import and reflection logic. Preserve the test’s existing channel-acceptance behavior.lib/src/ssh_forward.dart (1)
1-1: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winFlush-before-drain guarantee is unverified and untested.
SSHForwardChannel.flush()awaits a singleFuture.microtask(() {})before delegating to_channel.flush(), but no test proves this is enough to drain data written viasink.add()through theStreamController→pipe→_localStream→ upload-loop chain before flush resolves.
lib/src/ssh_forward.dart#L79-85: Confirm whether one microtask reliably drains the full sink pipeline before_channel.flush()runs; if not, strengthen the synchronization (for example, wait until the upload loop is idle).test/src/ssh_flush_test.dart#L114-133: Once the guarantee is confirmed, extend theSSHForwardChannel.flushtest to write data throughsinkfirst and assert it is observed on the wire beforeflush()resolves.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/src/ssh_forward.dart` at line 1, Verify and strengthen SSHForwardChannel.flush synchronization so data added through sink completes the StreamController/pipe/_localStream upload pipeline before _channel.flush() resolves, rather than relying on a single microtask unless proven sufficient. Extend the existing flush test to write through sink, await flush, and assert the bytes were observed on the wire before completion.test/src/socket/dynamic_forward_io_test.dart (2)
418-433: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert closure of the overflow connection.
The test proves that the listener accepts a later connection. It does not prove that the overflowing connection closes. Await the first client's
donefuture before openingclient2.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/socket/dynamic_forward_io_test.dart` around lines 418 - 433, The overflow test should explicitly verify that the original client connection closes. After sending the oversized handshake payload in the existing test, await the first client’s done future before connecting client2, while preserving the subsequent forward-usability check.
400-403: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the remote EOF.
The test only delays after
client.close(). It passes if the remote sink remains open. Capture the outboundSSH_Message_Channel_EOFin_DialedTunneland expect it before cleanup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/socket/dynamic_forward_io_test.dart` around lines 400 - 403, Update the half-close test around _DialedTunnel to capture the outbound SSH_Message_Channel_EOF emitted after client.close(), then assert that EOF is received before performing cleanup; retain the existing data send and close sequence while replacing the delay-only verification with an explicit remote EOF expectation.test/src/ssh_agent_test.dart (1)
312-324: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert channel closure for both invalid-frame cases.
Both tests only verify that the handler receives no request. They pass if
SSHAgentChannelsilently drops the invalid frame without calling_channel.destroy().
test/src/ssh_agent_test.dart#L312-L324: awaitcontroller.channel.doneafter the zero-length frame and before test cleanup.test/src/ssh_agent_test.dart#L349-L360: awaitcontroller.channel.doneafter the oversized frame and before test cleanup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/src/ssh_agent_test.dart` around lines 312 - 324, Ensure both invalid-frame tests assert channel closure, not only that no requests were processed: in test/src/ssh_agent_test.dart lines 312-324, await controller.channel.done after the zero-length frame and before cleanup; likewise in lines 349-360, await controller.channel.done after the oversized frame and before cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 283-286: Add missing reference definitions in CHANGELOG.md for
[`#187`], [`#186`], [`#183`], [`#182`], and [`#179`], matching the existing pull request
reference format used by nearby entries such as [`#175`] and [`#176`], so all newly
added changelog labels render as links.
In `@lib/src/sftp/sftp_client.dart`:
- Around line 267-275: Update close() to complete _handshake with an appropriate
connection-closed error when it is still pending, matching the existing handling
in _closeError(). Keep the current waiter cleanup, _done completion, and channel
shutdown behavior unchanged so callers awaiting handshake do not hang.
In `@lib/src/ssh_forward.dart`:
- Around line 79-85: Update SSHForward.flush and the upload coordination around
_sinkController, _localStream, and _uploadLoop to await an explicit barrier that
is completed only after all data queued before flush has been passed to
sendMessage(). Replace the single microtask synchronization with this barrier
before invoking _channel.flush(), and add a test verifying wire ordering for
data queued immediately before flush.
In `@test/src/ssh_transport_version_test.dart`:
- Around line 68-85: The test “reschedules processing when more data remains in
the buffer” currently closes after observing remoteVersion without exercising
the queued remainder. Update the test around SSHClient and _FakeSSHSocket to
provide valid or intentionally partial packet data after the banner, then await
and assert the resulting observable processing outcome or expected packet error
before closing the client.
---
Outside diff comments:
In `@lib/src/ssh_client.dart`:
- Around line 128-133: Update the documentation for defaultAuthTimeout and
defaultHandshakeTimeout to state that they are opt-in presets and are not
applied automatically; clarify that authTimeout and handshakeTimeout remain null
unless callers explicitly provide values, or reference these constants from the
corresponding field documentation.
---
Nitpick comments:
In `@lib/src/ssh_forward.dart`:
- Line 1: Verify and strengthen SSHForwardChannel.flush synchronization so data
added through sink completes the StreamController/pipe/_localStream upload
pipeline before _channel.flush() resolves, rather than relying on a single
microtask unless proven sufficient. Extend the existing flush test to write
through sink, await flush, and assert the bytes were observed on the wire before
completion.
In `@test/src/algorithm/ssh_cipher_type_test.dart`:
- Around line 108-122: Extend the KEX assertions around SSHKexType.x25519Rfc to
verify that its name is exactly “curve25519-sha256”, not just that the constant
appears in algorithms.kex. If the existing integration harness supports it, add
a negotiation case where the peer offers only that literal name and confirm it
is selected and serialized.
In `@test/src/socket/dynamic_forward_io_test.dart`:
- Around line 418-433: The overflow test should explicitly verify that the
original client connection closes. After sending the oversized handshake payload
in the existing test, await the first client’s done future before connecting
client2, while preserving the subsequent forward-usability check.
- Around line 400-403: Update the half-close test around _DialedTunnel to
capture the outbound SSH_Message_Channel_EOF emitted after client.close(), then
assert that EOF is received before performing cleanup; retain the existing data
send and close sequence while replacing the delay-only verification with an
explicit remote EOF expectation.
In `@test/src/ssh_agent_test.dart`:
- Around line 312-324: Ensure both invalid-frame tests assert channel closure,
not only that no requests were processed: in test/src/ssh_agent_test.dart lines
312-324, await controller.channel.done after the zero-length frame and before
cleanup; likewise in lines 349-360, await controller.channel.done after the
oversized frame and before cleanup.
In `@test/src/ssh_flush_test.dart`:
- Around line 1-8: Add a `@visibleForTesting` wrapper for
SSHClient._acceptChannel, following the existing handlePacket testing seam, then
update ssh_flush_test.dart to call that wrapper directly and remove the
dart:mirrors import and reflection logic. Preserve the test’s existing
channel-acceptance behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 983c850e-81c1-4e4f-8514-035d5c74cdb5
📒 Files selected for processing (33)
.github/dependabot.ymlCHANGELOG.mdREADME.mdlib/src/algorithm/ssh_kex_type.dartlib/src/dynamic_forward_io.dartlib/src/sftp/sftp_client.dartlib/src/socket/ssh_socket.dartlib/src/socket/ssh_socket_io.dartlib/src/ssh_algorithm.dartlib/src/ssh_channel.dartlib/src/ssh_client.dartlib/src/ssh_forward.dartlib/src/ssh_keepalive.dartlib/src/ssh_session.dartlib/src/ssh_transport.dartpubspec.yamltest/src/algorithm/ssh_cipher_type_test.darttest/src/http/http_client_chunked_test.darttest/src/http/http_client_test.darttest/src/sftp/sftp_client_protocol_test.darttest/src/socket/dynamic_forward_io_test.darttest/src/ssh_agent_test.darttest/src/ssh_auth_abort_error_test.darttest/src/ssh_client_forward_dynamic_test.darttest/src/ssh_client_ident_test.darttest/src/ssh_client_run_with_result_test.darttest/src/ssh_client_test.darttest/src/ssh_client_timeout_test.darttest/src/ssh_flush_test.darttest/src/ssh_keepalive_test.darttest/src/ssh_transport_aead_test.darttest/src/ssh_transport_version_test.darttest/src/ssh_userauth_export_test.dart
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: dart analysis / check: Merge upstream dartssh2 v2.22.5
Conclusion: failure
##[group]Run dart format --output=none --set-exit-if-changed .
�[36;1mdart format --output=none --set-exit-if-changed .�[0m
shell: /usr/bin/bash -e {0}
env:
DART_HOME: /opt/hostedtoolcache/dart/3.12.2/x64
PUB_CACHE: /home/runner/.pub-cache
##[endgroup]
Changed lib/src/http/http_date.dart
Changed lib/src/kex/kex_nist.dart
Formatted 141 files (2 changed) in 0.21 seconds.
##[error]Process completed with exit code 1.
GitHub Actions: dart analysis / 0_check.txt: Merge upstream dartssh2 v2.22.5
Conclusion: failure
##[group]Run dart format --output=none --set-exit-if-changed .
�[36;1mdart format --output=none --set-exit-if-changed .�[0m
shell: /usr/bin/bash -e {0}
env:
DART_HOME: /opt/hostedtoolcache/dart/3.12.2/x64
PUB_CACHE: /home/runner/.pub-cache
##[endgroup]
Changed lib/src/http/http_date.dart
Changed lib/src/kex/kex_nist.dart
Formatted 141 files (2 changed) in 0.21 seconds.
##[error]Process completed with exit code 1.
🔇 Additional comments (33)
lib/src/algorithm/ssh_kex_type.dart (1)
10-17: LGTM!lib/src/ssh_algorithm.dart (1)
95-95: LGTM!test/src/ssh_transport_version_test.dart (3)
47-66: LGTM!
117-120: LGTM!
137-138: LGTM!.github/dependabot.yml (1)
10-16: LGTM!CHANGELOG.md (1)
303-304: LGTM!pubspec.yaml (1)
2-2: LGTM!test/src/ssh_userauth_export_test.dart (1)
1-25: LGTM!lib/src/socket/ssh_socket.dart (1)
31-33: LGTM!lib/src/socket/ssh_socket_io.dart (1)
41-45: LGTM!lib/src/ssh_transport.dart (1)
129-131: LGTM!Also applies to: 454-506, 1532-1535
test/src/ssh_auth_abort_error_test.dart (1)
78-80: LGTM!test/src/ssh_client_forward_dynamic_test.dart (1)
71-73: LGTM!test/src/ssh_client_ident_test.dart (1)
103-105: LGTM!test/src/ssh_transport_aead_test.dart (1)
792-794: LGTM!test/src/ssh_client_timeout_test.dart (2)
51-105: LGTM!
3-12: 🎯 Functional CorrectnessNo change required. CI runs this suite with
dart test, and the repository already usesdart:mirrorsin multiple tests.> Likely an incorrect or invalid review comment.lib/src/ssh_channel.dart (1)
34-35: LGTM!Also applies to: 46-46, 453-456, 487-489
lib/src/ssh_client.dart (1)
251-252: LGTM!Also applies to: 290-297, 361-362, 751-761, 774-781, 924-947, 1692-1692
test/src/ssh_flush_test.dart (1)
10-37: LGTM!Also applies to: 69-113
README.md (1)
165-182: LGTM!Also applies to: 298-316
lib/src/dynamic_forward_io.dart (1)
174-180: LGTM!lib/src/ssh_keepalive.dart (1)
12-29: LGTM!test/src/ssh_keepalive_test.dart (1)
1-85: LGTM!lib/src/ssh_session.dart (2)
35-46: LGTM!Also applies to: 63-64, 123-132, 145-158
85-90: LGTM!test/src/http/http_client_chunked_test.dart (1)
48-50: LGTM!test/src/http/http_client_test.dart (1)
334-336: LGTM!test/src/ssh_client_run_with_result_test.dart (2)
11-69: LGTM!
287-289: LGTM!test/src/ssh_client_test.dart (1)
7-7: LGTM!Also applies to: 276-302
test/src/sftp/sftp_client_protocol_test.dart (1)
101-115: LGTM!Also applies to: 923-930
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/src/ssh_channel.dart (1)
420-475: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftProcess a flush barrier when the remote window is exhausted.
At Line 422,
_uploadLoopreturns before it reads a queued_SSHChannelFlushBarrierwhen_remoteWindowis zero. If data at Line 463 exactly consumes the window, a followingflush()waits forever at Line 475.onFlushalso never runs.Allow a head-of-queue flush barrier to complete without remote-window credit. Keep normal data packets blocked until a window adjustment arrives. Add a test that writes exactly
remoteInitialWindowSizebytes and then awaitsflush().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/src/ssh_channel.dart` around lines 420 - 475, Update _uploadLoop so it inspects and processes a queued _SSHChannelFlushBarrier even when _remoteWindow is exhausted, completing the barrier and allowing onFlush to run without sending data. Keep normal data reads and packet transmission blocked until remote-window credit is restored, and add coverage for writing exactly remoteInitialWindowSize bytes followed by awaiting flush().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@lib/src/ssh_channel.dart`:
- Around line 420-475: Update _uploadLoop so it inspects and processes a queued
_SSHChannelFlushBarrier even when _remoteWindow is exhausted, completing the
barrier and allowing onFlush to run without sending data. Keep normal data reads
and packet transmission blocked until remote-window credit is restored, and add
coverage for writing exactly remoteInitialWindowSize bytes followed by awaiting
flush().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f6b3cc06-4091-4d13-9db9-a88182f13ebc
📒 Files selected for processing (11)
CHANGELOG.mdlib/src/sftp/sftp_client.dartlib/src/ssh_channel.dartlib/src/ssh_client.dartlib/src/ssh_forward.darttest/src/algorithm/ssh_cipher_type_test.darttest/src/sftp/sftp_client_protocol_test.darttest/src/socket/dynamic_forward_io_test.darttest/src/ssh_agent_test.darttest/src/ssh_flush_test.darttest/src/ssh_transport_version_test.dart
🚧 Files skipped from review as they are similar to previous changes (8)
- test/src/algorithm/ssh_cipher_type_test.dart
- lib/src/sftp/sftp_client.dart
- test/src/ssh_agent_test.dart
- CHANGELOG.md
- test/src/ssh_transport_version_test.dart
- test/src/sftp/sftp_client_protocol_test.dart
- test/src/ssh_flush_test.dart
- test/src/socket/dynamic_forward_io_test.dart
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: dart analysis / 0_check.txt: Merge upstream v2.22.5
Conclusion: failure
##[group]Run dart format --output=none --set-exit-if-changed .
�[36;1mdart format --output=none --set-exit-if-changed .�[0m
shell: /usr/bin/bash -e {0}
env:
DART_HOME: /opt/hostedtoolcache/dart/3.12.2/x64
PUB_CACHE: /home/runner/.pub-cache
##[endgroup]
Changed lib/src/http/http_date.dart
Changed lib/src/kex/kex_nist.dart
Formatted 141 files (2 changed) in 0.20 seconds.
##[error]Process completed with exit code 1.
GitHub Actions: dart analysis / check: Merge upstream v2.22.5
Conclusion: failure
##[group]Run dart format --output=none --set-exit-if-changed .
�[36;1mdart format --output=none --set-exit-if-changed .�[0m
shell: /usr/bin/bash -e {0}
env:
DART_HOME: /opt/hostedtoolcache/dart/3.12.2/x64
PUB_CACHE: /home/runner/.pub-cache
##[endgroup]
Changed lib/src/http/http_date.dart
Changed lib/src/kex/kex_nist.dart
Formatted 141 files (2 changed) in 0.20 seconds.
##[error]Process completed with exit code 1.
Guarantee forward and channel flush ordering with explicit upload barriers, abort pending SFTP handshakes on close, and strengthen timeout documentation and regression coverage for transport, forwarding, agent, and KEX behavior.
GT-610
force-pushed
the
merge-upstream-v2.22.5
branch
from
August 2, 2026 09:17
120dbb3 to
9880173
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
lollipopkit/flutter_server_box/issues/1244
Summary by CodeRabbit
Summary by CodeRabbit