Files
Termind/TermindTests/AgentConnectionPoolTests.swift
T
Kaede AkinoandClaude Opus 5 4b861eae26 Reuse one SSH session per host, not one per command
`ssh_exec` built and tore down a complete SSH connection every time it
ran. Its `defer` freed the session and closed the transport before
returning, so each command paid a TCP connect, a handshake, a host-key
check, and a public-key or Secure Enclave signature. With a jump host
the whole route was rebuilt per hop, per command: an agent turn running
five commands behind one jump performed ten handshakes to do work that
needs one connection and five channels.

SFTP was pooled per `(runID, hostID)`, which is barely a pool — a run is
short — and it had a reentrancy hole. Two tool calls that both reached
the `await` on `connectSFTP` before either stored its client each built
a connection, and one was never disconnected.

The pool is now keyed by host alone and outlives runs and conversations,
which is what makes an idle timeout meaningful: run-scoped connections
are already bounded, so a sweep would rarely have fired. A follow-up
message reuses the connection; three idle minutes gives it back. Exec
and SFTP stay on separate sessions so a long command cannot block a
directory listing — at most two connections per host, against one per
command before.

Two boundaries make reuse safe rather than merely cheap.

A retry only happens while the command is provably not running.
`channelUnavailable` is thrown for a dead session, a channel that would
not open, or a `process_startup` that failed; everything after startup
succeeds is rethrown unchanged. A socket that drops mid-read must not
silently run half an `rm` a second time.

A command that timed out or was cancelled leaves the channel mid-stream
with the remote possibly still working. Closing it courteously means a
blocking `libssh2_channel_close` on the queue every later command for
that host has to wait behind, so the session is abandoned instead and
`libssh2_session_free` reclaims the channel with it.

Reuse also has to keep answering the same question. Each entry carries a
route fingerprint — host, port, username, known host keys, proxy, jump
chain — built only from records the store already holds. No Keychain
read: this runs before every command, and a rotated credential on an
unchanged identity is not a mismatch, because the live session is still
the same box and the same login. Editing a host's address retires the
connection bound to the old one rather than running the next command on
a machine the user no longer means.

No connect or disconnect tool. The model does not manage this, so
putting four more definitions in every request would buy nothing.

`AgentRuntime.runCleanup` goes with it: closing pooled connections was
its only caller, and leaving it wired to a no-op invites someone to hang
per-run teardown off a pool that is no longer per-run.

The unit tests run against stubs and prove the pool's behaviour, not
libssh2's. That one connection now carries five commands still has to be
confirmed against a real host, in its `auth.log`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-25 22:58:31 +08:00

346 lines
12 KiB
Swift

import Foundation
import Testing
@testable import Termind
@Suite
struct AgentConnectionPoolTests {
@Test
func repeatedCommandsOnOneHostShareASingleConnection() async throws {
let backend = StubBackend()
let pool = makePool(backend)
let host = UUID()
for index in 0..<5 {
let result = try await pool.execute(
"echo \(index)", workingDirectory: nil, stdin: nil,
timeout: 60, outputLimit: 1_024, hostID: host
)
#expect(result.exitCode == 0)
}
// The whole point of the pool: five commands, one handshake.
#expect(await backend.execOpens == 1)
#expect(await backend.execDisconnects == 0)
}
@Test
func concurrentCallersShareOneConnectionInsteadOfLeakingOne() async throws {
let backend = StubBackend()
let gate = Gate()
let pool = makePool(backend, openerGate: gate)
let host = UUID()
async let first = pool.exec(hostID: host)
// Only start the second caller once the first is parked mid-connect, so
// it is guaranteed to find work already in flight rather than racing it.
while await gate.arrivals == 0 { await Task.yield() }
async let second = pool.exec(hostID: host)
await gate.open()
let sessions = try await [first, second]
#expect(try sessions.map(serial) == [1, 1])
#expect(await backend.execOpens == 1)
}
@Test
func execAndSFTPAreSeparateConnectionsThatCloseIndependently() async throws {
let backend = StubBackend()
let pool = makePool(backend)
let host = UUID()
_ = try await pool.exec(hostID: host)
_ = try await pool.sftp(hostID: host)
#expect(await pool.openConnectionCount == 2)
#expect(await pool.disconnect(hostID: host, kind: .exec))
#expect(await pool.isConnected(hostID: host, kind: .sftp))
#expect(await backend.execDisconnects == 1)
#expect(await backend.sftpDisconnects == 0)
}
@Test
func disconnectingATwiceReportsThatNothingWasOpenTheSecondTime() async throws {
let backend = StubBackend()
let pool = makePool(backend)
let host = UUID()
_ = try await pool.exec(hostID: host)
#expect(await pool.disconnect(hostID: host, kind: .exec))
#expect(await pool.disconnect(hostID: host, kind: .exec) == false)
#expect(await backend.execDisconnects == 1)
}
@Test
func idleConnectionsCloseAndTheNextCommandReconnects() async throws {
let backend = StubBackend()
let pool = makePool(backend)
let host = UUID()
_ = try await pool.exec(hostID: host)
let remaining = await pool.sweepIdle(
now: .now.advanced(by: AgentConnectionPool.defaultIdleTimeout + .seconds(1))
)
#expect(remaining == 0)
#expect(await backend.execDisconnects == 1)
#expect(await pool.isConnected(hostID: host, kind: .exec) == false)
_ = try await pool.exec(hostID: host)
#expect(await backend.execOpens == 2)
}
@Test
func aRecentlyUsedConnectionSurvivesTheSweep() async throws {
let backend = StubBackend()
let pool = makePool(backend)
let host = UUID()
_ = try await pool.exec(hostID: host)
let remaining = await pool.sweepIdle(now: .now)
#expect(remaining == 1)
#expect(await backend.execDisconnects == 0)
}
@Test
func editingAHostsRouteRetiresTheConnectionBoundToTheOldOne() async throws {
let backend = StubBackend()
let pool = makePool(backend)
let host = UUID()
let before = try await pool.exec(hostID: host)
await backend.setFingerprint("route-b", for: host)
let after = try await pool.exec(hostID: host)
// Not the same machine any more, so not the same session — otherwise a
// command aimed at the edited host would run on the old one.
#expect(try serial(before) == 1)
#expect(try serial(after) == 2)
#expect(await backend.execOpens == 2)
#expect(await pool.openConnectionCount == 1)
}
@Test
func aHostThatNoLongerResolvesHasItsConnectionClosed() async throws {
let backend = StubBackend()
let pool = makePool(backend)
let host = UUID()
_ = try await pool.exec(hostID: host)
await backend.forget(host)
await #expect(throws: AgentToolError.self) { _ = try await pool.exec(hostID: host) }
#expect(await backend.execDisconnects == 1)
#expect(await pool.openConnectionCount == 0)
}
@Test
func aSessionThatDiedBetweenCommandsIsReplacedTransparently() async throws {
let backend = StubBackend()
let pool = makePool(backend)
let host = UUID()
await backend.queueExecFailures([
SSHExecSessionError.channelUnavailable(TermindSSHError.disconnected)
])
let result = try await pool.execute(
"uptime", workingDirectory: nil, stdin: nil,
timeout: 60, outputLimit: 1_024, hostID: host
)
#expect(result.exitCode == 0)
#expect(await backend.execOpens == 2)
#expect(await backend.execDisconnects == 1)
}
@Test
func aCommandThatFailedAfterItStartedIsNeverRunTwice() async throws {
let backend = StubBackend()
let pool = makePool(backend)
let host = UUID()
// Not `channelUnavailable`: the command reached the remote shell, so a
// silent retry could delete something a second time.
await backend.queueExecFailures([TermindSSHError.library("connection reset")])
await #expect(throws: TermindSSHError.self) {
_ = try await pool.execute(
"rm -rf /srv//old", workingDirectory: nil, stdin: nil,
timeout: 60, outputLimit: 1_024, hostID: host
)
}
#expect(await backend.execOpens == 1)
#expect(await backend.totalExecuteCount() == 1)
}
@Test
func closeAllReleasesEveryHost() async throws {
let backend = StubBackend()
let pool = makePool(backend)
for _ in 0..<3 {
let host = UUID()
_ = try await pool.exec(hostID: host)
_ = try await pool.sftp(hostID: host)
}
await pool.closeAll()
#expect(await pool.openConnectionCount == 0)
#expect(await backend.execDisconnects == 3)
#expect(await backend.sftpDisconnects == 3)
}
}
/// Which stub connection the pool handed back. Distinguishing "reused" from
/// "reconnected" is the whole subject of these tests.
private func serial(_ session: any SSHExecSessionProtocol) throws -> Int {
try #require(session as? StubExecSession).stubSerial
}
/// `sweepInterval: nil` keeps the background sweeper out of the tests; every
/// case here drives `sweepIdle` itself with a synthetic instant.
private func makePool(
_ backend: StubBackend, openerGate: Gate? = nil
) -> AgentConnectionPool {
AgentConnectionPool(
configuration: { hostID in
SSHConnectionConfiguration(
hostID: hostID, host: "host.example", port: 22, username: "root",
credentials: [], expectedHostKeys: [], proxy: nil, jumps: []
)
},
routeFingerprint: { try await backend.fingerprint(for: $0) },
openExec: { _ in
if let openerGate { await openerGate.wait() }
return await backend.openExec()
},
openSFTP: { _ in await backend.openSFTP() },
sweepInterval: nil
)
}
private actor Gate {
private(set) var arrivals = 0
private var isOpen = false
private var waiting: [CheckedContinuation<Void, Never>] = []
func wait() async {
arrivals += 1
guard !isOpen else { return }
await withCheckedContinuation { waiting.append($0) }
}
func open() {
isOpen = true
for continuation in waiting { continuation.resume() }
waiting.removeAll()
}
}
private actor StubBackend {
private(set) var execOpens = 0
private(set) var sftpOpens = 0
private(set) var execDisconnects = 0
private(set) var sftpDisconnects = 0
private var fingerprints: [UUID: String] = [:]
private var forgotten: Set<UUID> = []
private var nextExecFailures: [any Error] = []
private var execSessions: [StubExecSession] = []
func fingerprint(for hostID: UUID) throws -> String {
guard !forgotten.contains(hostID) else { throw AgentToolError.invalidHost }
return fingerprints[hostID] ?? "route-a"
}
func setFingerprint(_ value: String, for hostID: UUID) { fingerprints[hostID] = value }
func forget(_ hostID: UUID) { forgotten.insert(hostID) }
/// Applies to the next session opened, so a test can arrange for a pooled
/// connection to fail once and the replacement to succeed.
func queueExecFailures(_ errors: [any Error]) { nextExecFailures = errors }
func openExec() -> StubExecSession {
execOpens += 1
let session = StubExecSession(
serial: execOpens, backend: self, failures: nextExecFailures
)
nextExecFailures = []
execSessions.append(session)
return session
}
func openSFTP() -> StubSFTPClient {
sftpOpens += 1
return StubSFTPClient(backend: self)
}
func noteExecDisconnect() { execDisconnects += 1 }
func noteSFTPDisconnect() { sftpDisconnects += 1 }
func totalExecuteCount() async -> Int {
var total = 0
for session in execSessions { total += await session.executeCount }
return total
}
}
private actor StubExecSession: SSHExecSessionProtocol {
nonisolated let stubSerial: Int
private let backend: StubBackend
private var failures: [any Error]
private(set) var executeCount = 0
init(serial: Int, backend: StubBackend, failures: [any Error]) {
stubSerial = serial
self.backend = backend
self.failures = failures
}
func connect(using configuration: SSHConnectionConfiguration) async throws {}
func execute(
_ command: String, workingDirectory: String?, stdin: Data?,
timeout: TimeInterval, outputLimit: Int
) async throws -> SSHExecResult {
executeCount += 1
if !failures.isEmpty { throw failures.removeFirst() }
return SSHExecResult(
stdout: command, stderr: "", exitCode: 0, signal: nil,
stdoutTruncated: false, stderrTruncated: false
)
}
func disconnect() async { await backend.noteExecDisconnect() }
}
private actor StubSFTPClient: SFTPClientProtocol {
private let backend: StubBackend
init(backend: StubBackend) { self.backend = backend }
func connect(
using configuration: SSHConnectionConfiguration,
progress: SSHRouteProgressHandler?
) async throws {}
func disconnect() async { await backend.noteSFTPDisconnect() }
func homeDirectory() async throws -> String { "/root" }
func listDirectory(at path: String) async throws -> [RemoteFile] { [] }
func attributes(at path: String) async throws -> RemoteFile? { nil }
func setPermissions(at path: String, to permissions: UInt32) async throws {}
func createDirectory(at path: String) async throws {}
func rename(from source: String, to destination: String, overwrite: Bool) async throws {}
func removeFile(at path: String) async throws {}
func removeDirectory(at path: String) async throws {}
func uploadFile(
from localURL: URL, to remotePath: String, overwrite: Bool,
cancellation: TransferCancellationToken,
progress: @escaping @Sendable (SFTPProgress) -> Void
) async throws {}
func downloadFile(
from remotePath: String, to localURL: URL, overwrite: Bool,
cancellation: TransferCancellationToken,
progress: @escaping @Sendable (SFTPProgress) -> Void
) async throws {}
}