feat(session): surface exec exit-status / exit-signal via SshSession.exitInfo#232
feat(session): surface exec exit-status / exit-signal via SshSession.exitInfo#232GlassOnTin wants to merge 1 commit into
Conversation
…exitInfo The kaitai schemas already parse the RFC 4254 §6.10 exit-status and exit-signal channel requests, but the connection's packet loop dropped them after logging, so exec consumers had no way to learn a command's exit code. Adds a sealed SessionExit (Status / Signal) and a Deferred exitInfo on SshSession, completed from the packet loop when the server reports termination and resolved to null when the channel closes without a report (servers are not required to send one). First report wins per the RFC. Tests: four FakeSshServer flow tests (status, signal, close-without- report resolves null, first-report-wins) plus two Testcontainers integration tests against OpenSSH 9.9p2 (sh -c 'exit 42' and a KILL-terminated command). Metalava signature updated; metalavaCheckCompatibility passes.
…p probe (#58) Phase 3 hit an upstream wall worth documenting: sshlib runs exec but drops the RFC 4254 §6.10 exit-status/exit-signal reports, so ExecResult.exitStatus cannot be produced honestly — and run_command automations branch on exit codes. Upstreamed as connectbot/cbssh#232 (SshSession.exitInfo: Deferred<SessionExit?>, flow + OpenSSH-9.9p2 integration tests, 1170 upstream tests green). Haven side, landable today: ExecContractTest — the engine-agnostic exec contract (stdout, stderr, exit codes, 1 MiB output, timeout shape) against MINA with an in-process scripted CommandFactory, run by JschExecContractTest as the behaviour pin; plus a spike GAP probe asserting the released sshlib has no exit API — it fails when a release carries #232, which is the signal to add SshlibExecContractTest and the engine-aware exec routing.
|
Some context on where this fits, @kruton, for whenever you next have review bandwidth (CI is green across JDK 17/21/25 + CodeQL). This closes the last gap for adopting sshlib as a second SSH engine in Haven — the migration you kindly kicked off back in GlassHaven/Haven#58 when you reached out about ssh-proto. Since then the exec (#95), subsystem (#98), SFTP (#112) and FIDO2/SK (#146) work all landed upstream, and Haven now has the per-engine building blocks built and tested against real servers:
The one thing still missing is a truthful command exit status. Haven runs Happy to reshape the API if you'd prefer something other than an exposed |
|
|
||
| /** SSH_MSG_CHANNEL_REQUEST `exit-status` (RFC 4254 section 6.10). */ | ||
| suspend fun sendChannelExitStatus(recipientChannel: Int, exitStatus: Int) { | ||
| val payload = ByteArrayOutputStream() |
There was a problem hiding this comment.
You should be able to use the SshMsgChannelRequest, ChannelRequestExitStatus, and ChannelRequestExitSignal types since they already support writing.
| coreDumped: Boolean = false, | ||
| errorMessage: String = "", | ||
| ) { | ||
| val payload = ByteArrayOutputStream() |
There was a problem hiding this comment.
You should be able to use the SshMsgChannelRequest, ChannelRequestExitStatus, and ChannelRequestExitSignal types since they already support writing.
| // process terminated to exitInfo waiters. | ||
| when (val fields = msg.requestSpecificFields()) { | ||
| is ChannelRequestExitStatus -> entry.channel.receiveExitInfo( | ||
| SessionExit.Status(fields.exitStatus().toInt()), |
There was a problem hiding this comment.
It looks like the RFC says this should be uint32, but this is converting to integer here. Prefer Long to preserve the full range. It might be prudent to add a test for the 0xffff_ffffL boundary condition as well.
2 participants
What
Adds
SshSession.exitInfo: Deferred<SessionExit?>— the RFC 4254 §6.10exit-status/exit-signalreports, which the packet loop previously parsed (the kaitai types were already wired intoSshMsgChannelRequest) but dropped after logging. Without this, an exec consumer can read stdout/stderr to EOF but never learn how the command terminated.SessionExitsealed interface:Status(code)/Signal(signalName, coreDumped, errorMessage)nullwhen the channel closes without a report (servers aren't required to send one), so waiters are never left suspendedmetalavaCheckCompatibilitypassesWhy
Haven is adopting sshlib as its second SSH engine (context in GlassHaven/Haven#58); the next slice is remote command execution, where automations branch on exit codes. This was the one missing piece for exec parity — happy to adjust the API shape (e.g. a suspend accessor instead of the exposed
Deferred) if you'd prefer.Tests
FakeSshServerflow tests: exit-status completesexitInfo; exit-signal completes it; close-without-report resolves null; first report wins over a later onesh -c 'exit 42'→Status(42); a KILL-terminated command →Signal("KILL", …):sshlib:testsuite: 1170 tests, 0 failures (unit + integration, Docker available)