mirror of
https://github.com/awizemann/scarf.git
synced 2026-05-10 10:36:35 +00:00
fix(diagnostics): treat config.yaml absence as informational, not failure
Same root cause as the connection-pill fix in 511726e: Hermes v0.11+
doesn't materialize config.yaml until the user changes a setting from
defaults, so a healthy fresh install was reporting "12/14 passing"
forever even though everything that mattered worked.
Probe.Status becomes tri-state (.pass / .fail / .skipped). The shell
script emits SKIP for the "config.yaml absent" branch (Hermes creates
it lazily); only "exists but unreadable" still emits FAIL. The view
renders .skipped with a grey info-circle and excludes those probes
from the summary's denominator — "12/12 passing (2 optional skipped)"
instead of the misleading "12/14."
Probe titles relabeled to "config.yaml readable (optional)" and
"config.yaml content (optional)" so users see the file is not
load-bearing at a glance. The failure hint for the genuine
permission-denied case explicitly notes that absence is fine.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -50,8 +50,8 @@ final class RemoteDiagnosticsViewModel {
|
|||||||
case .hermesHomeConfigured: return "Hermes home directory"
|
case .hermesHomeConfigured: return "Hermes home directory"
|
||||||
case .hermesDirExists: return "Hermes directory exists"
|
case .hermesDirExists: return "Hermes directory exists"
|
||||||
case .hermesDirReadable: return "Hermes directory readable"
|
case .hermesDirReadable: return "Hermes directory readable"
|
||||||
case .configYAMLReadable: return "config.yaml readable"
|
case .configYAMLReadable: return "config.yaml readable (optional)"
|
||||||
case .configYAMLContents: return "config.yaml actually readable (content)"
|
case .configYAMLContents: return "config.yaml content (optional)"
|
||||||
case .stateDBReadable: return "state.db readable"
|
case .stateDBReadable: return "state.db readable"
|
||||||
case .sqlite3Installed: return "sqlite3 binary installed on remote"
|
case .sqlite3Installed: return "sqlite3 binary installed on remote"
|
||||||
case .sqlite3CanOpenStateDB: return "sqlite3 can open state.db"
|
case .sqlite3CanOpenStateDB: return "sqlite3 can open state.db"
|
||||||
@@ -75,9 +75,13 @@ final class RemoteDiagnosticsViewModel {
|
|||||||
case .hermesDirReadable:
|
case .hermesDirReadable:
|
||||||
return "The SSH user can see `~/.hermes` but can't list it. Check permissions: `ls -ld ~/.hermes` on the remote — the SSH user needs at least `r-x`."
|
return "The SSH user can see `~/.hermes` but can't list it. Check permissions: `ls -ld ~/.hermes` on the remote — the SSH user needs at least `r-x`."
|
||||||
case .configYAMLReadable, .configYAMLContents:
|
case .configYAMLReadable, .configYAMLContents:
|
||||||
return "Scarf can't read `config.yaml`. This usually means the SSH user is different from the user Hermes runs as. Either (a) run Hermes as the SSH user, (b) `chmod a+r ~/.hermes/config.yaml`, or (c) configure Scarf to SSH as the Hermes user."
|
// Reached only when the file EXISTS but is unreadable —
|
||||||
|
// a real permission issue. The "file absent" case emits
|
||||||
|
// SKIP (Hermes v0.11+ creates config.yaml lazily, only
|
||||||
|
// when the user changes a setting from defaults).
|
||||||
|
return "`config.yaml` exists on the remote but the SSH user can't read it. Either (a) run Hermes as the SSH user, (b) `chmod a+r ~/.hermes/config.yaml`, or (c) configure Scarf to SSH as the Hermes user. If `config.yaml` is missing entirely, that's fine — Hermes only creates it when you change a setting from the defaults."
|
||||||
case .stateDBReadable:
|
case .stateDBReadable:
|
||||||
return "Scarf can't read `state.db` — Sessions, Activity, Dashboard stats all depend on this. Same fix pattern as config.yaml."
|
return "Scarf can't read `state.db` — Sessions, Activity, Dashboard stats all depend on this. Either (a) run Hermes as the SSH user, (b) `chmod a+r ~/.hermes/state.db`, or (c) configure Scarf to SSH as the Hermes user."
|
||||||
case .sqlite3Installed:
|
case .sqlite3Installed:
|
||||||
return "Scarf pulls a snapshot of state.db via `sqlite3 .backup`, so sqlite3 must be installed on the remote AND visible to non-interactive SSH sessions. The probe sources `~/.zshenv` / `.zprofile` / `.bash_profile` / `.profile` and falls back to `/usr/bin`, `/usr/local/bin`, `/opt/homebrew/bin`, and `/opt/local/bin` — if it's still not found, either install via your package manager (`sudo apt install sqlite3` / `sudo yum install sqlite` / `apk add sqlite`) or symlink the existing binary into a location the probe checks (e.g. `sudo ln -s /your/path/sqlite3 /usr/local/bin/sqlite3`)."
|
return "Scarf pulls a snapshot of state.db via `sqlite3 .backup`, so sqlite3 must be installed on the remote AND visible to non-interactive SSH sessions. The probe sources `~/.zshenv` / `.zprofile` / `.bash_profile` / `.profile` and falls back to `/usr/bin`, `/usr/local/bin`, `/opt/homebrew/bin`, and `/opt/local/bin` — if it's still not found, either install via your package manager (`sudo apt install sqlite3` / `sudo yum install sqlite` / `apk add sqlite`) or symlink the existing binary into a location the probe checks (e.g. `sudo ln -s /your/path/sqlite3 /usr/local/bin/sqlite3`)."
|
||||||
case .sqlite3CanOpenStateDB:
|
case .sqlite3CanOpenStateDB:
|
||||||
@@ -92,10 +96,26 @@ final class RemoteDiagnosticsViewModel {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Tri-state probe outcome. `.skipped` covers checks that didn't
|
||||||
|
/// run because they aren't applicable (e.g. config.yaml absence on
|
||||||
|
/// a fresh Hermes v0.11+ install — the file is created lazily, so
|
||||||
|
/// missing is normal). UI renders skipped probes with a grey info
|
||||||
|
/// icon and excludes them from "X/Y failing" tallies.
|
||||||
|
enum ProbeStatus: Sendable, Equatable {
|
||||||
|
case pass
|
||||||
|
case fail
|
||||||
|
case skipped
|
||||||
|
}
|
||||||
|
|
||||||
struct Probe: Identifiable, Sendable {
|
struct Probe: Identifiable, Sendable {
|
||||||
let id: ProbeID
|
let id: ProbeID
|
||||||
let passed: Bool
|
let status: ProbeStatus
|
||||||
let detail: String
|
let detail: String
|
||||||
|
|
||||||
|
/// Back-compat for callers (Copy Full Report, view counters)
|
||||||
|
/// that still think in pass/fail. Skipped probes report `true`
|
||||||
|
/// so they don't count as failures.
|
||||||
|
var passed: Bool { status != .fail }
|
||||||
}
|
}
|
||||||
|
|
||||||
private(set) var probes: [Probe] = []
|
private(set) var probes: [Probe] = []
|
||||||
@@ -135,10 +155,10 @@ final class RemoteDiagnosticsViewModel {
|
|||||||
rawStderr = msg
|
rawStderr = msg
|
||||||
rawExitCode = -1
|
rawExitCode = -1
|
||||||
probes = [
|
probes = [
|
||||||
Probe(id: .connectivity, passed: false, detail: msg)
|
Probe(id: .connectivity, status: .fail, detail: msg)
|
||||||
] + ProbeID.allCases
|
] + ProbeID.allCases
|
||||||
.filter { $0 != .connectivity }
|
.filter { $0 != .connectivity }
|
||||||
.map { Probe(id: $0, passed: false, detail: "(skipped — SSH didn't connect)") }
|
.map { Probe(id: $0, status: .fail, detail: "(skipped — SSH didn't connect)") }
|
||||||
case .completed(let stdout, let stderr, let exitCode):
|
case .completed(let stdout, let stderr, let exitCode):
|
||||||
rawStdout = stdout
|
rawStdout = stdout
|
||||||
rawStderr = stderr
|
rawStderr = stderr
|
||||||
@@ -151,18 +171,37 @@ final class RemoteDiagnosticsViewModel {
|
|||||||
Self.logger.info("Diagnostics for \(self.context.displayName, privacy: .public) finished — \(self.passingCount)/\(self.probes.count) passing")
|
Self.logger.info("Diagnostics for \(self.context.displayName, privacy: .public) finished — \(self.passingCount)/\(self.probes.count) passing")
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Quick summary string, e.g. "9/14 passing". Used in the header.
|
/// Quick summary string. Skipped probes (e.g. config.yaml absent
|
||||||
|
/// on a fresh Hermes v0.11+ install) are excluded from the
|
||||||
|
/// denominator so the user sees "12/12 passing" instead of a
|
||||||
|
/// misleading "12/14 passing." When any probe is skipped we
|
||||||
|
/// append a parenthetical so it's still visible at a glance.
|
||||||
var summary: String {
|
var summary: String {
|
||||||
guard !probes.isEmpty else { return "Not yet run." }
|
guard !probes.isEmpty else { return "Not yet run." }
|
||||||
return "\(passingCount)/\(probes.count) checks passing"
|
let total = probes.filter { $0.status != .skipped }.count
|
||||||
|
var s = "\(passingCount)/\(total) checks passing"
|
||||||
|
if skippedCount > 0 {
|
||||||
|
s += " (\(skippedCount) optional skipped)"
|
||||||
|
}
|
||||||
|
return s
|
||||||
}
|
}
|
||||||
|
|
||||||
var passingCount: Int {
|
var passingCount: Int {
|
||||||
probes.filter { $0.passed }.count
|
probes.filter { $0.status == .pass }.count
|
||||||
}
|
}
|
||||||
|
|
||||||
|
var skippedCount: Int {
|
||||||
|
probes.filter { $0.status == .skipped }.count
|
||||||
|
}
|
||||||
|
|
||||||
|
var failingCount: Int {
|
||||||
|
probes.filter { $0.status == .fail }.count
|
||||||
|
}
|
||||||
|
|
||||||
|
/// True iff every applicable probe passed — skipped probes don't
|
||||||
|
/// block the green-banner state because they're informational.
|
||||||
var allPassed: Bool {
|
var allPassed: Bool {
|
||||||
!probes.isEmpty && passingCount == probes.count
|
!probes.isEmpty && failingCount == 0
|
||||||
}
|
}
|
||||||
|
|
||||||
// MARK: - Script + parsing
|
// MARK: - Script + parsing
|
||||||
@@ -210,21 +249,32 @@ final class RemoteDiagnosticsViewModel {
|
|||||||
emit hermesDirReadable FAIL "cannot read/enter $H (check perms on the dir)"
|
emit hermesDirReadable FAIL "cannot read/enter $H (check perms on the dir)"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
|
# config.yaml is OPTIONAL on Hermes v0.11+ — the file is created
|
||||||
|
# lazily when the user changes a setting from defaults. So a
|
||||||
|
# working fresh install is expected to have no config.yaml.
|
||||||
|
# The probe distinguishes:
|
||||||
|
# PASS — file exists and is readable
|
||||||
|
# SKIP — file is absent (informational, not a failure)
|
||||||
|
# FAIL — file exists but the SSH user can't read it (real perm issue)
|
||||||
if [ -r "$H/config.yaml" ]; then
|
if [ -r "$H/config.yaml" ]; then
|
||||||
emit configYAMLReadable PASS ""
|
emit configYAMLReadable PASS ""
|
||||||
else
|
else
|
||||||
if [ -e "$H/config.yaml" ]; then
|
if [ -e "$H/config.yaml" ]; then
|
||||||
emit configYAMLReadable FAIL "exists but not readable by $user"
|
emit configYAMLReadable FAIL "exists but not readable by $user"
|
||||||
else
|
else
|
||||||
emit configYAMLReadable FAIL "file does not exist"
|
emit configYAMLReadable SKIP "not present (Hermes creates it on first config change)"
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
if head -c 1 "$H/config.yaml" > /dev/null 2>&1; then
|
if [ -e "$H/config.yaml" ]; then
|
||||||
size=$(wc -c < "$H/config.yaml" 2>/dev/null | tr -d ' ')
|
if head -c 1 "$H/config.yaml" > /dev/null 2>&1; then
|
||||||
emit configYAMLContents PASS "${size} bytes"
|
size=$(wc -c < "$H/config.yaml" 2>/dev/null | tr -d ' ')
|
||||||
|
emit configYAMLContents PASS "${size} bytes"
|
||||||
|
else
|
||||||
|
emit configYAMLContents FAIL "cannot read file contents"
|
||||||
|
fi
|
||||||
else
|
else
|
||||||
emit configYAMLContents FAIL "cannot read file contents"
|
emit configYAMLContents SKIP "not present (no content to read)"
|
||||||
fi
|
fi
|
||||||
|
|
||||||
if [ -r "$H/state.db" ]; then
|
if [ -r "$H/state.db" ]; then
|
||||||
@@ -319,12 +369,18 @@ final class RemoteDiagnosticsViewModel {
|
|||||||
let parts = line.split(separator: "|", maxSplits: 2, omittingEmptySubsequences: false)
|
let parts = line.split(separator: "|", maxSplits: 2, omittingEmptySubsequences: false)
|
||||||
guard parts.count == 3 else { continue }
|
guard parts.count == 3 else { continue }
|
||||||
let key = String(parts[0]).trimmingCharacters(in: .whitespaces)
|
let key = String(parts[0]).trimmingCharacters(in: .whitespaces)
|
||||||
let status = String(parts[1]).trimmingCharacters(in: .whitespaces)
|
let statusRaw = String(parts[1]).trimmingCharacters(in: .whitespaces)
|
||||||
let detail = String(parts[2]).trimmingCharacters(in: .whitespaces)
|
let detail = String(parts[2]).trimmingCharacters(in: .whitespaces)
|
||||||
guard let probe = ProbeID(rawValue: key) else { continue }
|
guard let probe = ProbeID(rawValue: key) else { continue }
|
||||||
|
let status: ProbeStatus
|
||||||
|
switch statusRaw {
|
||||||
|
case "PASS": status = .pass
|
||||||
|
case "SKIP": status = .skipped
|
||||||
|
default: status = .fail
|
||||||
|
}
|
||||||
results[probe] = Probe(
|
results[probe] = Probe(
|
||||||
id: probe,
|
id: probe,
|
||||||
passed: status == "PASS",
|
status: status,
|
||||||
detail: detail
|
detail: detail
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -342,7 +398,7 @@ final class RemoteDiagnosticsViewModel {
|
|||||||
}
|
}
|
||||||
|
|
||||||
return ProbeID.allCases.map { id in
|
return ProbeID.allCases.map { id in
|
||||||
results[id] ?? Probe(id: id, passed: false, detail: fallbackDetail)
|
results[id] ?? Probe(id: id, status: .fail, detail: fallbackDetail)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -93,8 +93,10 @@ struct RemoteDiagnosticsView: View {
|
|||||||
|
|
||||||
private func probeRow(_ probe: RemoteDiagnosticsViewModel.Probe) -> some View {
|
private func probeRow(_ probe: RemoteDiagnosticsViewModel.Probe) -> some View {
|
||||||
HStack(alignment: .top, spacing: 12) {
|
HStack(alignment: .top, spacing: 12) {
|
||||||
Image(systemName: probe.passed ? "checkmark.circle.fill" : "xmark.circle.fill")
|
// Tri-state icon: green check on pass, red x on fail, grey
|
||||||
.foregroundStyle(probe.passed ? .green : .red)
|
// info-circle on skipped (the optional-and-absent state).
|
||||||
|
Image(systemName: iconName(for: probe.status))
|
||||||
|
.foregroundStyle(iconColor(for: probe.status))
|
||||||
.font(.title3)
|
.font(.title3)
|
||||||
.padding(.top, 2)
|
.padding(.top, 2)
|
||||||
VStack(alignment: .leading, spacing: 4) {
|
VStack(alignment: .leading, spacing: 4) {
|
||||||
@@ -106,7 +108,7 @@ struct RemoteDiagnosticsView: View {
|
|||||||
.foregroundStyle(.secondary)
|
.foregroundStyle(.secondary)
|
||||||
.textSelection(.enabled)
|
.textSelection(.enabled)
|
||||||
}
|
}
|
||||||
if !probe.passed, let hint = probe.id.failureHint {
|
if probe.status == .fail, let hint = probe.id.failureHint {
|
||||||
HStack(alignment: .top, spacing: 6) {
|
HStack(alignment: .top, spacing: 6) {
|
||||||
Image(systemName: "lightbulb")
|
Image(systemName: "lightbulb")
|
||||||
.foregroundStyle(.yellow)
|
.foregroundStyle(.yellow)
|
||||||
@@ -128,6 +130,22 @@ struct RemoteDiagnosticsView: View {
|
|||||||
.padding(.vertical, 10)
|
.padding(.vertical, 10)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private func iconName(for status: RemoteDiagnosticsViewModel.ProbeStatus) -> String {
|
||||||
|
switch status {
|
||||||
|
case .pass: return "checkmark.circle.fill"
|
||||||
|
case .fail: return "xmark.circle.fill"
|
||||||
|
case .skipped: return "info.circle"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private func iconColor(for status: RemoteDiagnosticsViewModel.ProbeStatus) -> Color {
|
||||||
|
switch status {
|
||||||
|
case .pass: return .green
|
||||||
|
case .fail: return .red
|
||||||
|
case .skipped: return .secondary
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private var footer: some View {
|
private var footer: some View {
|
||||||
VStack(alignment: .leading, spacing: 8) {
|
VStack(alignment: .leading, spacing: 8) {
|
||||||
// Raw-output disclosure. Shown whenever anything fails — we need
|
// Raw-output disclosure. Shown whenever anything fails — we need
|
||||||
@@ -189,10 +207,15 @@ struct RemoteDiagnosticsView: View {
|
|||||||
lines.append("Result: \(viewModel.summary)")
|
lines.append("Result: \(viewModel.summary)")
|
||||||
lines.append("")
|
lines.append("")
|
||||||
for probe in viewModel.probes {
|
for probe in viewModel.probes {
|
||||||
let mark = probe.passed ? "PASS" : "FAIL"
|
let mark: String
|
||||||
|
switch probe.status {
|
||||||
|
case .pass: mark = "PASS"
|
||||||
|
case .fail: mark = "FAIL"
|
||||||
|
case .skipped: mark = "SKIP"
|
||||||
|
}
|
||||||
lines.append("[\(mark)] \(probe.id.title)")
|
lines.append("[\(mark)] \(probe.id.title)")
|
||||||
if !probe.detail.isEmpty { lines.append(" \(probe.detail)") }
|
if !probe.detail.isEmpty { lines.append(" \(probe.detail)") }
|
||||||
if !probe.passed, let hint = probe.id.failureHint {
|
if probe.status == .fail, let hint = probe.id.failureHint {
|
||||||
lines.append(" hint: \(hint)")
|
lines.append(" hint: \(hint)")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user