Improve code quality: error logging, constants, path validation, safe defaults

- Replace try? with do/catch and [Scarf] error logging in all service-layer
  JSON decoding, file writes, and directory creation
- Extract sqliteTransient constant replacing raw unsafeBitCast(-1, ...) pattern
- Add QueryDefaults and FileSizeUnit enums for all magic numbers
- Guard HOME env var with NSHomeDirectory() fallback instead of force-unwrap
- Add path traversal validation to loadSkillContent()
- Add SessionStats.empty and use it across all initialization sites
- Replace KnownPlatforms array indexing with named .cli constant

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alan Wizemann
2026-04-02 03:15:03 -04:00
parent c7f3ca9be3
commit 563f5a702c
10 changed files with 126 additions and 50 deletions
+31 -3
View File
@@ -1,8 +1,11 @@
import Foundation import Foundation
import SQLite3
enum HermesPaths: Sendable { enum HermesPaths: Sendable {
// Using ProcessInfo to avoid main-actor isolation issues with FileManager/NSHomeDirectory private nonisolated static let userHome: String = ProcessInfo.processInfo.environment["HOME"]
nonisolated static let home: String = ProcessInfo.processInfo.environment["HOME"]! + "/.hermes" ?? NSHomeDirectory()
nonisolated static let home: String = userHome + "/.hermes"
nonisolated static let stateDB: String = home + "/state.db" nonisolated static let stateDB: String = home + "/state.db"
nonisolated static let configYAML: String = home + "/config.yaml" nonisolated static let configYAML: String = home + "/config.yaml"
nonisolated static let memoriesDir: String = home + "/memories" nonisolated static let memoriesDir: String = home + "/memories"
@@ -15,7 +18,32 @@ enum HermesPaths: Sendable {
nonisolated static let skillsDir: String = home + "/skills" nonisolated static let skillsDir: String = home + "/skills"
nonisolated static let errorsLog: String = home + "/logs/errors.log" nonisolated static let errorsLog: String = home + "/logs/errors.log"
nonisolated static let gatewayLog: String = home + "/logs/gateway.log" nonisolated static let gatewayLog: String = home + "/logs/gateway.log"
nonisolated static let hermesBinary: String = ProcessInfo.processInfo.environment["HOME"]! + "/.local/bin/hermes" nonisolated static let hermesBinary: String = userHome + "/.local/bin/hermes"
nonisolated static let scarfDir: String = home + "/scarf" nonisolated static let scarfDir: String = home + "/scarf"
nonisolated static let projectsRegistry: String = scarfDir + "/projects.json" nonisolated static let projectsRegistry: String = scarfDir + "/projects.json"
} }
// MARK: - SQLite Constants
/// SQLITE_TRANSIENT tells SQLite to make its own copy of bound string data.
/// The C macro is defined as ((sqlite3_destructor_type)-1) which can't be imported directly into Swift.
nonisolated let sqliteTransient = unsafeBitCast(-1, to: sqlite3_destructor_type.self)
// MARK: - Query Defaults
enum QueryDefaults: Sendable {
nonisolated static let sessionLimit = 100
nonisolated static let messageSearchLimit = 50
nonisolated static let toolCallLimit = 50
nonisolated static let sessionPreviewLimit = 10
nonisolated static let previewContentLength = 100
nonisolated static let logLineLimit = 200
nonisolated static let defaultSilenceThreshold = 200
}
// MARK: - File Size Formatting
enum FileSizeUnit: Sendable {
nonisolated static let kilobyte = 1_024.0
nonisolated static let megabyte = 1_048_576.0
}
+2 -1
View File
@@ -16,8 +16,9 @@ struct HermesToolPlatform: Identifiable, Sendable {
} }
enum KnownPlatforms { enum KnownPlatforms {
static let cli = HermesToolPlatform(name: "cli", displayName: "CLI", icon: "terminal")
static let all: [HermesToolPlatform] = [ static let all: [HermesToolPlatform] = [
HermesToolPlatform(name: "cli", displayName: "CLI", icon: "terminal"), cli,
HermesToolPlatform(name: "telegram", displayName: "Telegram", icon: "paperplane"), HermesToolPlatform(name: "telegram", displayName: "Telegram", icon: "paperplane"),
HermesToolPlatform(name: "discord", displayName: "Discord", icon: "bubble.left.and.bubble.right"), HermesToolPlatform(name: "discord", displayName: "Discord", icon: "bubble.left.and.bubble.right"),
HermesToolPlatform(name: "slack", displayName: "Slack", icon: "number"), HermesToolPlatform(name: "slack", displayName: "Slack", icon: "number"),
@@ -24,7 +24,7 @@ actor HermesDataService {
db = nil db = nil
} }
func fetchSessions(limit: Int = 100) -> [HermesSession] { func fetchSessions(limit: Int = QueryDefaults.sessionLimit) -> [HermesSession] {
guard let db else { return [] } guard let db else { return [] }
let sql = """ let sql = """
SELECT id, source, user_id, model, title, parent_session_id, SELECT id, source, user_id, model, title, parent_session_id,
@@ -59,7 +59,7 @@ actor HermesDataService {
var stmt: OpaquePointer? var stmt: OpaquePointer?
guard sqlite3_prepare_v2(db, sql, -1, &stmt, nil) == SQLITE_OK else { return [] } guard sqlite3_prepare_v2(db, sql, -1, &stmt, nil) == SQLITE_OK else { return [] }
defer { sqlite3_finalize(stmt) } defer { sqlite3_finalize(stmt) }
sqlite3_bind_text(stmt, 1, sessionId, -1, unsafeBitCast(-1, to: sqlite3_destructor_type.self)) sqlite3_bind_text(stmt, 1, sessionId, -1, sqliteTransient)
var messages: [HermesMessage] = [] var messages: [HermesMessage] = []
while sqlite3_step(stmt) == SQLITE_ROW { while sqlite3_step(stmt) == SQLITE_ROW {
@@ -68,7 +68,7 @@ actor HermesDataService {
return messages return messages
} }
func searchMessages(query: String, limit: Int = 50) -> [HermesMessage] { func searchMessages(query: String, limit: Int = QueryDefaults.messageSearchLimit) -> [HermesMessage] {
guard let db else { return [] } guard let db else { return [] }
let sql = """ let sql = """
SELECT m.id, m.session_id, m.role, m.content, m.tool_call_id, m.tool_calls, SELECT m.id, m.session_id, m.role, m.content, m.tool_call_id, m.tool_calls,
@@ -82,7 +82,7 @@ actor HermesDataService {
var stmt: OpaquePointer? var stmt: OpaquePointer?
guard sqlite3_prepare_v2(db, sql, -1, &stmt, nil) == SQLITE_OK else { return [] } guard sqlite3_prepare_v2(db, sql, -1, &stmt, nil) == SQLITE_OK else { return [] }
defer { sqlite3_finalize(stmt) } defer { sqlite3_finalize(stmt) }
sqlite3_bind_text(stmt, 1, query, -1, unsafeBitCast(-1, to: sqlite3_destructor_type.self)) sqlite3_bind_text(stmt, 1, query, -1, sqliteTransient)
sqlite3_bind_int(stmt, 2, Int32(limit)) sqlite3_bind_int(stmt, 2, Int32(limit))
var messages: [HermesMessage] = [] var messages: [HermesMessage] = []
@@ -92,7 +92,7 @@ actor HermesDataService {
return messages return messages
} }
func fetchRecentToolCalls(limit: Int = 50) -> [HermesMessage] { func fetchRecentToolCalls(limit: Int = QueryDefaults.toolCallLimit) -> [HermesMessage] {
guard let db else { return [] } guard let db else { return [] }
let sql = """ let sql = """
SELECT id, session_id, role, content, tool_call_id, tool_calls, SELECT id, session_id, role, content, tool_call_id, tool_calls,
@@ -114,10 +114,10 @@ actor HermesDataService {
return messages return messages
} }
func fetchSessionPreviews(limit: Int = 10) -> [String: String] { func fetchSessionPreviews(limit: Int = QueryDefaults.sessionPreviewLimit) -> [String: String] {
guard let db else { return [:] } guard let db else { return [:] }
let sql = """ let sql = """
SELECT m.session_id, substr(m.content, 1, 100) SELECT m.session_id, substr(m.content, 1, \(QueryDefaults.previewContentLength))
FROM messages m FROM messages m
INNER JOIN ( INNER JOIN (
SELECT session_id, MIN(id) as min_id SELECT session_id, MIN(id) as min_id
@@ -149,13 +149,15 @@ actor HermesDataService {
let totalInputTokens: Int let totalInputTokens: Int
let totalOutputTokens: Int let totalOutputTokens: Int
let totalCostUSD: Double let totalCostUSD: Double
static let empty = SessionStats(
totalSessions: 0, totalMessages: 0, totalToolCalls: 0,
totalInputTokens: 0, totalOutputTokens: 0, totalCostUSD: 0
)
} }
func fetchStats() -> SessionStats { func fetchStats() -> SessionStats {
guard let db else { guard let db else { return .empty }
return SessionStats(totalSessions: 0, totalMessages: 0, totalToolCalls: 0,
totalInputTokens: 0, totalOutputTokens: 0, totalCostUSD: 0)
}
let sql = """ let sql = """
SELECT COUNT(*), COALESCE(SUM(message_count),0), COALESCE(SUM(tool_call_count),0), SELECT COUNT(*), COALESCE(SUM(message_count),0), COALESCE(SUM(tool_call_count),0),
COALESCE(SUM(input_tokens),0), COALESCE(SUM(output_tokens),0), COALESCE(SUM(input_tokens),0), COALESCE(SUM(output_tokens),0),
@@ -163,16 +165,9 @@ actor HermesDataService {
FROM sessions FROM sessions
""" """
var stmt: OpaquePointer? var stmt: OpaquePointer?
guard sqlite3_prepare_v2(db, sql, -1, &stmt, nil) == SQLITE_OK else { guard sqlite3_prepare_v2(db, sql, -1, &stmt, nil) == SQLITE_OK else { return .empty }
return SessionStats(totalSessions: 0, totalMessages: 0, totalToolCalls: 0,
totalInputTokens: 0, totalOutputTokens: 0, totalCostUSD: 0)
}
defer { sqlite3_finalize(stmt) } defer { sqlite3_finalize(stmt) }
guard sqlite3_step(stmt) == SQLITE_ROW else { return .empty }
guard sqlite3_step(stmt) == SQLITE_ROW else {
return SessionStats(totalSessions: 0, totalMessages: 0, totalToolCalls: 0,
totalInputTokens: 0, totalOutputTokens: 0, totalCostUSD: 0)
}
return SessionStats( return SessionStats(
totalSessions: Int(sqlite3_column_int(stmt, 0)), totalSessions: Int(sqlite3_column_int(stmt, 0)),
totalMessages: Int(sqlite3_column_int(stmt, 1)), totalMessages: Int(sqlite3_column_int(stmt, 1)),
@@ -344,7 +339,12 @@ actor HermesDataService {
private func parseToolCalls(_ json: String?) -> [HermesToolCall] { private func parseToolCalls(_ json: String?) -> [HermesToolCall] {
guard let json, !json.isEmpty, guard let json, !json.isEmpty,
let data = json.data(using: .utf8) else { return [] } let data = json.data(using: .utf8) else { return [] }
return (try? JSONDecoder().decode([HermesToolCall].self, from: data)) ?? [] do {
return try JSONDecoder().decode([HermesToolCall].self, from: data)
} catch {
print("[Scarf] Failed to decode tool calls: \(error.localizedDescription)")
return []
}
} }
private func columnText(_ stmt: OpaquePointer, _ col: Int32) -> String { private func columnText(_ stmt: OpaquePointer, _ col: Int32) -> String {
@@ -44,7 +44,7 @@ struct HermesFileService: Sendable {
showReasoning: values["display.show_reasoning"] == "true", showReasoning: values["display.show_reasoning"] == "true",
verbose: values["agent.verbose"] == "true", verbose: values["agent.verbose"] == "true",
autoTTS: values["voice.auto_tts"] != "false", autoTTS: values["voice.auto_tts"] != "false",
silenceThreshold: Int(values["voice.silence_threshold"] ?? "") ?? 200 silenceThreshold: Int(values["voice.silence_threshold"] ?? "") ?? QueryDefaults.defaultSilenceThreshold
) )
} }
@@ -52,7 +52,12 @@ struct HermesFileService: Sendable {
func loadGatewayState() -> GatewayState? { func loadGatewayState() -> GatewayState? {
guard let data = readFileData(HermesPaths.gatewayStateJSON) else { return nil } guard let data = readFileData(HermesPaths.gatewayStateJSON) else { return nil }
return try? JSONDecoder().decode(GatewayState.self, from: data) do {
return try JSONDecoder().decode(GatewayState.self, from: data)
} catch {
print("[Scarf] Failed to decode gateway state: \(error.localizedDescription)")
return nil
}
} }
// MARK: - Memory // MARK: - Memory
@@ -77,8 +82,13 @@ struct HermesFileService: Sendable {
func loadCronJobs() -> [HermesCronJob] { func loadCronJobs() -> [HermesCronJob] {
guard let data = readFileData(HermesPaths.cronJobsJSON) else { return [] } guard let data = readFileData(HermesPaths.cronJobsJSON) else { return [] }
let file = try? JSONDecoder().decode(CronJobsFile.self, from: data) do {
return file?.jobs ?? [] let file = try JSONDecoder().decode(CronJobsFile.self, from: data)
return file.jobs
} catch {
print("[Scarf] Failed to decode cron jobs: \(error.localizedDescription)")
return []
}
} }
func loadCronOutput(jobId: String) -> String? { func loadCronOutput(jobId: String) -> String? {
@@ -123,7 +133,13 @@ struct HermesFileService: Sendable {
} }
func loadSkillContent(path: String) -> String { func loadSkillContent(path: String) -> String {
readFile(path) ?? "" // Validate path stays within the skills directory to prevent traversal
guard !path.contains(".."),
path.hasPrefix(HermesPaths.skillsDir) else {
print("[Scarf] Rejected skill path outside skills directory: \(path)")
return ""
}
return readFile(path) ?? ""
} }
// MARK: - Hermes Process // MARK: - Hermes Process
@@ -156,6 +172,10 @@ struct HermesFileService: Sendable {
} }
private func writeFile(_ path: String, content: String) { private func writeFile(_ path: String, content: String) {
try? content.write(toFile: path, atomically: true, encoding: .utf8) do {
try content.write(toFile: path, atomically: true, encoding: .utf8)
} catch {
print("[Scarf] Failed to write \(path): \(error.localizedDescription)")
}
} }
} }
@@ -39,12 +39,16 @@ actor HermesLogService {
} }
func closeLog() { func closeLog() {
try? fileHandle?.close() do {
try fileHandle?.close()
} catch {
print("[Scarf] Failed to close log handle: \(error.localizedDescription)")
}
fileHandle = nil fileHandle = nil
currentPath = nil currentPath = nil
} }
func readLastLines(count: Int = 200) -> [LogEntry] { func readLastLines(count: Int = QueryDefaults.logLineLimit) -> [LogEntry] {
guard let path = currentPath, guard let path = currentPath,
let data = FileManager.default.contents(atPath: path) else { return [] } let data = FileManager.default.contents(atPath: path) else { return [] }
let content = String(data: data, encoding: .utf8) ?? "" let content = String(data: data, encoding: .utf8) ?? ""
@@ -8,14 +8,23 @@ struct ProjectDashboardService: Sendable {
guard let data = FileManager.default.contents(atPath: HermesPaths.projectsRegistry) else { guard let data = FileManager.default.contents(atPath: HermesPaths.projectsRegistry) else {
return ProjectRegistry(projects: []) return ProjectRegistry(projects: [])
} }
return (try? JSONDecoder().decode(ProjectRegistry.self, from: data)) do {
?? ProjectRegistry(projects: []) return try JSONDecoder().decode(ProjectRegistry.self, from: data)
} catch {
print("[Scarf] Failed to decode project registry: \(error.localizedDescription)")
return ProjectRegistry(projects: [])
}
} }
func saveRegistry(_ registry: ProjectRegistry) { func saveRegistry(_ registry: ProjectRegistry) {
let dir = HermesPaths.scarfDir let dir = HermesPaths.scarfDir
if !FileManager.default.fileExists(atPath: dir) { if !FileManager.default.fileExists(atPath: dir) {
try? FileManager.default.createDirectory(atPath: dir, withIntermediateDirectories: true) do {
try FileManager.default.createDirectory(atPath: dir, withIntermediateDirectories: true)
} catch {
print("[Scarf] Failed to create scarf directory: \(error.localizedDescription)")
return
}
} }
guard let data = try? JSONEncoder().encode(registry) else { return } guard let data = try? JSONEncoder().encode(registry) else { return }
// Pretty-print for readability (agents may read this file) // Pretty-print for readability (agents may read this file)
@@ -33,7 +42,12 @@ struct ProjectDashboardService: Sendable {
guard let data = FileManager.default.contents(atPath: project.dashboardPath) else { guard let data = FileManager.default.contents(atPath: project.dashboardPath) else {
return nil return nil
} }
return try? JSONDecoder().decode(ProjectDashboard.self, from: data) do {
return try JSONDecoder().decode(ProjectDashboard.self, from: data)
} catch {
print("[Scarf] Failed to decode dashboard for \(project.name): \(error.localizedDescription)")
return nil
}
} }
func dashboardExists(for project: ProjectEntry) -> Bool { func dashboardExists(for project: ProjectEntry) -> Bool {
@@ -5,10 +5,7 @@ final class DashboardViewModel {
private let dataService = HermesDataService() private let dataService = HermesDataService()
private let fileService = HermesFileService() private let fileService = HermesFileService()
var stats = HermesDataService.SessionStats( var stats = HermesDataService.SessionStats.empty
totalSessions: 0, totalMessages: 0, totalToolCalls: 0,
totalInputTokens: 0, totalOutputTokens: 0, totalCostUSD: 0
)
var recentSessions: [HermesSession] = [] var recentSessions: [HermesSession] = []
var sessionPreviews: [String: String] = [:] var sessionPreviews: [String: String] = [:]
var config = HermesConfig.empty var config = HermesConfig.empty
@@ -158,10 +158,10 @@ final class SessionsViewModel {
let fileSize: String let fileSize: String
if let attrs = try? FileManager.default.attributesOfItem(atPath: dbPath), if let attrs = try? FileManager.default.attributesOfItem(atPath: dbPath),
let size = attrs[.size] as? Int { let size = attrs[.size] as? Int {
if size >= 1_048_576 { if Double(size) >= FileSizeUnit.megabyte {
fileSize = String(format: "%.1f MB", Double(size) / 1_048_576) fileSize = String(format: "%.1f MB", Double(size) / FileSizeUnit.megabyte)
} else { } else {
fileSize = String(format: "%.0f KB", Double(size) / 1_024) fileSize = String(format: "%.0f KB", Double(size) / FileSizeUnit.kilobyte)
} }
} else { } else {
fileSize = "unknown" fileSize = "unknown"
@@ -18,7 +18,12 @@ final class SettingsViewModel {
config = fileService.loadConfig() config = fileService.loadConfig()
gatewayState = fileService.loadGatewayState() gatewayState = fileService.loadGatewayState()
hermesRunning = fileService.isHermesRunning() hermesRunning = fileService.isHermesRunning()
rawConfigYAML = (try? String(contentsOfFile: HermesPaths.configYAML, encoding: .utf8)) ?? "" do {
rawConfigYAML = try String(contentsOfFile: HermesPaths.configYAML, encoding: .utf8)
} catch {
print("[Scarf] Failed to read config.yaml: \(error.localizedDescription)")
rawConfigYAML = ""
}
personalities = parsePersonalities() personalities = parsePersonalities()
} }
@@ -2,7 +2,7 @@ import Foundation
@Observable @Observable
final class ToolsViewModel { final class ToolsViewModel {
var selectedPlatform: HermesToolPlatform = KnownPlatforms.all[0] var selectedPlatform: HermesToolPlatform = KnownPlatforms.cli
var toolsets: [HermesToolset] = [] var toolsets: [HermesToolset] = []
var mcpStatus: String = "" var mcpStatus: String = ""
var isLoading = false var isLoading = false
@@ -30,7 +30,13 @@ final class ToolsViewModel {
} }
private func loadPlatforms() { private func loadPlatforms() {
let config = (try? String(contentsOfFile: HermesPaths.configYAML, encoding: .utf8)) ?? "" let config: String
do {
config = try String(contentsOfFile: HermesPaths.configYAML, encoding: .utf8)
} catch {
print("[Scarf] Failed to read config.yaml: \(error.localizedDescription)")
config = ""
}
var platforms: [HermesToolPlatform] = [] var platforms: [HermesToolPlatform] = []
var inSection = false var inSection = false
for line in config.components(separatedBy: "\n") { for line in config.components(separatedBy: "\n") {
@@ -54,9 +60,10 @@ final class ToolsViewModel {
} }
} }
} }
availablePlatforms = platforms.isEmpty ? [KnownPlatforms.all[0]] : platforms availablePlatforms = platforms.isEmpty ? [KnownPlatforms.cli] : platforms
if !availablePlatforms.contains(where: { $0.name == selectedPlatform.name }) { if !availablePlatforms.contains(where: { $0.name == selectedPlatform.name }),
selectedPlatform = availablePlatforms[0] let first = availablePlatforms.first {
selectedPlatform = first
} }
} }