fix(ios): preserve hermes output on non-zero exit + extend remote PATH

Two related fixes that together restore Skills hub Browse / Search on
iOS over Citadel SSH.

CitadelServerTransport.asyncRunProcess was using `executeCommand`,
which throws `CommandFailed` and discards the captured ByteBuffer when
the remote process exits non-zero. `hermes skills browse` happens to
print its full table and then exit non-zero on some hosts, so iOS got
nothing while Mac (Foundation Process) got the full output with
exitCode=1. Drive `executeCommandStream` directly so stdout + stderr
are drained regardless of outcome, then catch `SSHClient.CommandFailed`
to recover the actual exit code. Network/channel-level failures still
report -1 so callers can distinguish them from a clean non-zero remote
exit.

Citadel's raw exec channel also doesn't source the user's shell rc
files, so non-interactive sessions land with a stripped PATH (typically
just /usr/bin:/bin). pipx installs `hermes` at `~/.local/bin/hermes`,
and many of hermes's sub-tools (git, curl, python) live in homebrew
prefixes that the remote sshd would otherwise add via login-shell init.
Mac's OpenSSH sshd handles this transparently; Citadel does not. Inline
PATH=$HOME/.local/bin:/opt/homebrew/bin:/usr/local/bin:$PATH on every
runProcess invocation so bare `hermes` resolves AND any subprocess it
spawns can still find its tools.

SkillsViewModel.finishBrowse now surfaces the actual stderr/stdout
snippet when the CLI exits non-zero, instead of a canned "Browse failed"
banner. ANSI-stripped + box-drawing-stripped so the message stays
readable in the one-line banner. Made diagnosing the underlying PATH
issue much easier and is a net UX improvement going forward.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alan Wizemann
2026-04-25 16:17:25 +02:00
parent 21e3cc9361
commit 850fa7a697
20 changed files with 218 additions and 24 deletions
@@ -149,7 +149,12 @@ public final class SkillsViewModel {
if source != "all" { args += ["--source", source] }
let result = Self.runHermes(executable: bin, args: args, transport: xport, timeout: 30)
let parsed = HermesSkillsHubParser.parseHubList(result.output)
await self?.finishBrowse(results: parsed, exitCode: result.exitCode, isSearch: false)
await self?.finishBrowse(
results: parsed,
exitCode: result.exitCode,
rawOutput: result.output,
isSearch: false
)
}
}
@@ -168,7 +173,12 @@ public final class SkillsViewModel {
if source != "all" { args += ["--source", source] }
let result = Self.runHermes(executable: bin, args: args, transport: xport, timeout: 30)
let parsed = HermesSkillsHubParser.parseHubList(result.output)
await self?.finishBrowse(results: parsed, exitCode: result.exitCode, isSearch: true)
await self?.finishBrowse(
results: parsed,
exitCode: result.exitCode,
rawOutput: result.output,
isSearch: true
)
}
}
@@ -244,18 +254,54 @@ public final class SkillsViewModel {
// about than the prior interleaved `MainActor.run` chains.
@MainActor
private func finishBrowse(results: [HermesHubSkill], exitCode: Int32, isSearch: Bool) async {
private func finishBrowse(
results: [HermesHubSkill],
exitCode: Int32,
rawOutput: String,
isSearch: Bool
) async {
isHubLoading = false
hubResults = results
if results.isEmpty {
hubMessage = isSearch
? "No matches"
: (exitCode == 0 ? "No results" : "Browse failed")
if exitCode == 0 {
hubMessage = isSearch ? "No matches" : "No results"
} else {
let label = isSearch ? "Search failed" : "Browse failed"
let detail = Self.firstSignificantLine(rawOutput)
hubMessage = detail.isEmpty
? "\(label) (exit \(exitCode))"
: "\(label): \(detail)"
}
} else {
hubMessage = nil
}
}
/// Extract the first non-empty, non-decorative line from CLI output
/// used to surface the actual error reason in `hubMessage` instead of a
/// canned "Browse failed". Skips Rich box-drawing chrome and ANSI noise
/// so the message stays readable in a one-line banner.
nonisolated private static func firstSignificantLine(_ output: String) -> String {
let stripped = output
.replacingOccurrences(
of: #"\u{001B}\[[0-9;]*m"#,
with: "",
options: .regularExpression
)
for raw in stripped.components(separatedBy: "\n") {
let line = raw.trimmingCharacters(in: .whitespaces)
guard !line.isEmpty else { continue }
if line.unicodeScalars.allSatisfy({ scalar in
let v = scalar.value
// Skip pure box-drawing rows (U+2500..U+257F) so the
// diagnostic surfaces the actual error text below them.
return (v >= 0x2500 && v <= 0x257F) || scalar == " "
}) { continue }
return String(line.prefix(160))
}
return ""
}
@MainActor
private func finishInstall(identifier: String, exitCode: Int32) async {
isHubLoading = false
@@ -331,29 +331,61 @@ public final class CitadelServerTransport: ServerTransport, @unchecked Sendable
timeout: TimeInterval?
) async throws -> ProcessResult {
let client = try await connectionHolder.ssh()
let cmd = Self.shellJoin([executable] + args)
// Citadel's executeCommand accumulates stdout into a ByteBuffer.
// stderr isn't separately exposed we fold it into the output
// via `2>&1` so error paths still give callers something to
// show. Exit code is similarly not directly exposed; on non-
// zero exit Citadel throws, so we map that to a commandFailed
// error with the captured output as stderr.
// Citadel's raw exec channel doesn't source the user's shell rc
// files, so non-interactive SSH sessions land with a stripped
// PATH (typically just `/usr/bin:/bin`). pipx installs `hermes`
// at `~/.local/bin/hermes`, and many of hermes's sub-tools
// (git/curl/python) live in homebrew prefixes that the remote
// sshd would otherwise add via login-shell init. Mac's OpenSSH
// sshd handles this transparently; Citadel does not. We extend
// PATH inline so bare `hermes` resolves AND any subprocess it
// spawns can still find its tools.
let cmd = "PATH=\"$HOME/.local/bin:/opt/homebrew/bin:/usr/local/bin:$PATH\" "
+ Self.shellJoin([executable] + args)
// Citadel's `executeCommand` discards captured output when the
// remote exits non-zero (it throws `CommandFailed` and the
// accumulated ByteBuffer is lost). That breaks legitimate cases
// like `hermes skills browse` printing a full table and *then*
// exiting non-zero callers see nothing and report "Browse
// failed". Drive `executeCommandStream` directly so we can
// collect stdout + stderr regardless of exit code, and surface
// the real exit status.
let stream: AsyncThrowingStream<ExecCommandOutput, Error>
do {
let buffer = try await client.executeCommand(cmd + " 2>&1")
var buf = buffer
let str = buf.readString(length: buf.readableBytes) ?? ""
return ProcessResult(
exitCode: 0,
stdout: Data(str.utf8),
stderr: Data()
)
stream = try await client.executeCommandStream(cmd)
} catch {
return ProcessResult(
exitCode: 1,
exitCode: -1,
stdout: Data(),
stderr: Data(error.localizedDescription.utf8)
)
}
var stdout = Data()
var stderr = Data()
var exitCode: Int32 = 0
do {
for try await chunk in stream {
switch chunk {
case .stdout(var buf):
if let s = buf.readString(length: buf.readableBytes) {
stdout.append(Data(s.utf8))
}
case .stderr(var buf):
if let s = buf.readString(length: buf.readableBytes) {
stderr.append(Data(s.utf8))
}
}
}
} catch let failed as SSHClient.CommandFailed {
exitCode = Int32(failed.exitCode)
} catch {
// Network / channel-level failure mid-stream preserve any
// partial output and report -1 so callers can distinguish
// from a clean non-zero remote exit.
stderr.append(Data(error.localizedDescription.utf8))
exitCode = -1
}
return ProcessResult(exitCode: exitCode, stdout: stdout, stderr: stderr)
}
private func asyncSnapshotSQLite(remotePath: String) async throws -> URL {
@@ -506,16 +538,27 @@ private actor ConnectionHolder {
if let existing = sshClient, existing.isConnected {
return existing
}
// Replacing the SSHClient invalidates any cached SFTPClient that
// was bound to the previous (now-dead) connection. Drop it here
// so the next sftp() call re-opens against the new client; without
// this, every SFTP-backed call after a reconnect throws "channel
// closed" until the app is restarted.
if let oldSftp = sftpClient {
try? await oldSftp.close()
sftpClient = nil
}
let client = try await openSSH()
sshClient = client
return client
}
func sftp() async throws -> SFTPClient {
// Pulling SSH first ensures a stale-after-reconnect cached
// sftpClient is cleared in `ssh()` before we read it here.
let client = try await ssh()
if let existing = sftpClient {
return existing
}
let client = try await ssh()
let sftp = try await client.openSFTP()
sftpClient = sftp
return sftp