Skip to content

Commit 69af407

Browse files
brianmclaude
andcommitted
Add unified logging and improve code quality
- Add os.Logger throughout the codebase, replacing print() statements - Forward broker output to unified logging system with level parsing - Fix epithet binary path resolution when running from Xcode (DEBUG) - Fix Selector("undo:") warnings with #selector(UndoManager.undo) - Add nonEmptyOrNil String extension for cleaner nil coalescing - Extract magic numbers to named constants in InspectWindowController Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent d8dc443 commit 69af407

File tree

7 files changed

+109
-16
lines changed

7 files changed

+109
-16
lines changed

Sources/EpithetAgent/AppDelegate.swift

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
import AppKit
2+
import os
3+
4+
private let logger = Logger(subsystem: "dev.epithet.agent", category: "AppDelegate")
25

36
class AppDelegate: NSObject, NSApplicationDelegate {
47
private var statusBarController: StatusBarController?
58
private let brokerManager = BrokerManager.shared
69
private let sshConfigManager = SSHConfigManager.shared
710

811
func applicationDidFinishLaunching(_ notification: Notification) {
12+
logger.notice("EpithetAgent starting")
13+
914
// Setup main menu with Edit commands (needed for copy/paste in text fields)
1015
setupMainMenu()
1116

@@ -32,8 +37,8 @@ class AppDelegate: NSObject, NSApplicationDelegate {
3237
// Edit menu (for copy/paste support)
3338
let editMenuItem = NSMenuItem()
3439
let editMenu = NSMenu(title: "Edit")
35-
editMenu.addItem(NSMenuItem(title: "Undo", action: Selector("undo:"), keyEquivalent: "z"))
36-
editMenu.addItem(NSMenuItem(title: "Redo", action: Selector("redo:"), keyEquivalent: "Z"))
40+
editMenu.addItem(NSMenuItem(title: "Undo", action: #selector(UndoManager.undo), keyEquivalent: "z"))
41+
editMenu.addItem(NSMenuItem(title: "Redo", action: #selector(UndoManager.redo), keyEquivalent: "Z"))
3742
editMenu.addItem(NSMenuItem.separator())
3843
editMenu.addItem(NSMenuItem(title: "Cut", action: #selector(NSText.cut(_:)), keyEquivalent: "x"))
3944
editMenu.addItem(NSMenuItem(title: "Copy", action: #selector(NSText.copy(_:)), keyEquivalent: "c"))

Sources/EpithetAgent/BrokerConfigStore.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import Foundation
2+
import os
3+
4+
private let logger = Logger(subsystem: "dev.epithet.agent", category: "ConfigStore")
25

36
class BrokerConfigStore {
47
static let shared = BrokerConfigStore()
@@ -36,7 +39,7 @@ class BrokerConfigStore {
3639
let container = try JSONDecoder().decode(ConfigContainer.self, from: data)
3740
brokers = container.brokers
3841
} catch {
39-
print("Failed to load broker configs: \(error)")
42+
logger.error("Failed to load broker configs: \(error)")
4043
brokers = []
4144
}
4245
}
@@ -48,7 +51,7 @@ class BrokerConfigStore {
4851
try data.write(to: configFileURL, options: .atomic)
4952
onChange?()
5053
} catch {
51-
print("Failed to save broker configs: \(error)")
54+
logger.error("Failed to save broker configs: \(error)")
5255
}
5356
}
5457

Sources/EpithetAgent/BrokerManager.swift

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,49 @@
11
import Foundation
22
import AppKit
3+
import os
4+
5+
private let logger = Logger(subsystem: "dev.epithet.agent", category: "BrokerManager")
6+
7+
/// Creates a Logger for a specific broker's output.
8+
private func brokerLogger(name: String) -> Logger {
9+
Logger(subsystem: "dev.epithet.agent", category: "Broker:\(name)")
10+
}
11+
12+
/// Parses a log line from the epithet binary and extracts the level and message.
13+
/// Format: "[ANSI]HH:MM:SS[ANSI] [ANSI]LVL[ANSI] message" where LVL is DBG/INF/WRN/ERR
14+
private func parseLogLine(_ line: String) -> (level: OSLogType, message: String)? {
15+
// Strip ANSI escape codes: \x1B[...m
16+
let stripped = line.replacingOccurrences(
17+
of: "\\x1B\\[[0-9;]*m",
18+
with: "",
19+
options: .regularExpression
20+
)
21+
22+
// Format after stripping: "HH:MM:SS LVL message..."
23+
// Find the level token after the timestamp.
24+
let parts = stripped.split(separator: " ", maxSplits: 2, omittingEmptySubsequences: true)
25+
guard parts.count >= 2 else { return nil }
26+
27+
let levelStr = String(parts[1])
28+
let message = parts.count > 2 ? String(parts[2]) : ""
29+
30+
let level: OSLogType
31+
switch levelStr {
32+
case "DBG":
33+
level = .debug
34+
case "INF":
35+
level = .info
36+
case "WRN":
37+
level = .default // .warning doesn't exist, use .default
38+
case "ERR":
39+
level = .error
40+
default:
41+
// Not a standard log line, treat as info.
42+
return (.info, stripped)
43+
}
44+
45+
return (level, message)
46+
}
347

448
class BrokerManager {
549
static let shared = BrokerManager()
@@ -24,21 +68,34 @@ class BrokerManager {
2468
private init() {}
2569

2670
var epithetBinaryPath: String {
27-
// When running from bundle, use bundled binary
71+
// When running from bundle, use bundled binary.
2872
if let resourcePath = Bundle.main.resourcePath {
2973
let bundledPath = (resourcePath as NSString).appendingPathComponent("epithet")
3074
if FileManager.default.fileExists(atPath: bundledPath) {
3175
return bundledPath
3276
}
3377
}
34-
// Fallback for development: use Resources/epithet relative to executable
78+
// Fallback for development: use Resources/epithet relative to executable.
3579
let executablePath = Bundle.main.executablePath ?? ""
3680
let executableDir = (executablePath as NSString).deletingLastPathComponent
3781
let devPath = (executableDir as NSString).appendingPathComponent("../../Resources/epithet")
3882
if FileManager.default.fileExists(atPath: devPath) {
3983
return (devPath as NSString).standardizingPath
4084
}
41-
// Last resort: check current directory
85+
// When running from Xcode, try the source directory.
86+
// The executable is in DerivedData, but we can find the source via the project structure.
87+
#if DEBUG
88+
let sourceResourcesPath = URL(fileURLWithPath: #file)
89+
.deletingLastPathComponent() // Sources/EpithetAgent
90+
.deletingLastPathComponent() // Sources
91+
.deletingLastPathComponent() // epithet-macos
92+
.appendingPathComponent("Resources/epithet")
93+
.path
94+
if FileManager.default.fileExists(atPath: sourceResourcesPath) {
95+
return sourceResourcesPath
96+
}
97+
#endif
98+
// Last resort: check current directory.
4299
return "./Resources/epithet"
43100
}
44101

@@ -56,7 +113,7 @@ class BrokerManager {
56113

57114
func start(broker: BrokerConfig) {
58115
guard states[broker.name]?.isRunning != true else {
59-
print("Broker \(broker.name) is already running")
116+
logger.debug("Broker \(broker.name) is already running")
60117
return
61118
}
62119

@@ -127,6 +184,17 @@ class BrokerManager {
127184
logs[brokerName] = String(log.suffix(Self.trimmedLogSize))
128185
}
129186

187+
// Forward each line to the unified logger.
188+
let brokerLog = brokerLogger(name: brokerName)
189+
for line in text.split(separator: "\n", omittingEmptySubsequences: true) {
190+
let lineStr = String(line)
191+
if let (level, message) = parseLogLine(lineStr) {
192+
brokerLog.log(level: level, "\(message)")
193+
} else {
194+
brokerLog.info("\(lineStr)")
195+
}
196+
}
197+
130198
onLogUpdate?(brokerName)
131199
}
132200

Sources/EpithetAgent/ConfigurationWindowController.swift

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
import AppKit
22

3+
// Extension to convert empty strings to nil
4+
extension String {
5+
var nonEmptyOrNil: String? {
6+
isEmpty ? nil : self
7+
}
8+
}
9+
310
class ConfigurationWindowController: NSWindowController {
411
private let configStore = BrokerConfigStore.shared
512
private let brokerManager = BrokerManager.shared
@@ -605,10 +612,10 @@ class ConfigurationWindowController: NSWindowController {
605612
broker.name = nameField.stringValue
606613
broker.caURLs = currentCAURLs
607614
broker.authMethod = AuthMethod.allCases[authMethodPopup.indexOfSelectedItem]
608-
broker.oidcIssuer = oidcIssuerField.stringValue.isEmpty ? nil : oidcIssuerField.stringValue
609-
broker.oidcClientID = oidcClientIDField.stringValue.isEmpty ? nil : oidcClientIDField.stringValue
610-
broker.oidcClientSecret = oidcClientSecretField.stringValue.isEmpty ? nil : oidcClientSecretField.stringValue
611-
broker.authCommand = authCommandField.stringValue.isEmpty ? nil : authCommandField.stringValue
615+
broker.oidcIssuer = oidcIssuerField.stringValue.nonEmptyOrNil
616+
broker.oidcClientID = oidcClientIDField.stringValue.nonEmptyOrNil
617+
broker.oidcClientSecret = oidcClientSecretField.stringValue.nonEmptyOrNil
618+
broker.authCommand = authCommandField.stringValue.nonEmptyOrNil
612619
broker.caTimeout = Int(caTimeoutField.stringValue) ?? 15
613620
broker.caCooldown = Int(caCooldownField.stringValue) ?? 600
614621
broker.verbosity = Verbosity.allCases[verbosityPopup.indexOfSelectedItem]

Sources/EpithetAgent/InspectWindowController.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ class InspectWindowController: NSWindowController {
44
private let brokerManager = BrokerManager.shared
55
private let configStore = BrokerConfigStore.shared
66

7+
// Constants
8+
private static let autoRefreshInterval: TimeInterval = 30.0
9+
private static let inspectSplitRatio: CGFloat = 0.55
10+
711
private var tabView: NSTabView!
812
private var noRunningBrokersLabel: NSTextField!
913
private var refreshButton: NSButton!
@@ -141,7 +145,7 @@ class InspectWindowController: NSWindowController {
141145

142146
// Set initial split position (55% inspect, 45% logs)
143147
DispatchQueue.main.async {
144-
splitView.setPosition(splitView.bounds.height * 0.55, ofDividerAt: 0)
148+
splitView.setPosition(splitView.bounds.height * Self.inspectSplitRatio, ofDividerAt: 0)
145149
}
146150

147151
return splitView
@@ -256,7 +260,7 @@ class InspectWindowController: NSWindowController {
256260

257261
@objc private func toggleAutoRefresh(_ sender: NSButton) {
258262
if sender.state == .on {
259-
autoRefreshTimer = Timer.scheduledTimer(withTimeInterval: 30.0, repeats: true) { [weak self] _ in
263+
autoRefreshTimer = Timer.scheduledTimer(withTimeInterval: Self.autoRefreshInterval, repeats: true) { [weak self] _ in
260264
self?.refresh()
261265
}
262266
} else {

Sources/EpithetAgent/SSHConfigManager.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import Foundation
2+
import os
3+
4+
private let logger = Logger(subsystem: "dev.epithet.agent", category: "SSHConfig")
25

36
class SSHConfigManager {
47
static let shared = SSHConfigManager()
@@ -57,7 +60,7 @@ class SSHConfigManager {
5760
// Ensure proper permissions.
5861
try fileManager.setAttributes([.posixPermissions: 0o600], ofItemAtPath: sshConfigPath)
5962
} catch {
60-
print("Failed to update SSH config: \(error)")
63+
logger.error("Failed to update SSH config: \(error)")
6164
}
6265
}
6366
}

Sources/EpithetAgent/StatusBarController.swift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import AppKit
22
import ServiceManagement
3+
import os
4+
5+
private let logger = Logger(subsystem: "dev.epithet.agent", category: "StatusBar")
36

47
class StatusBarController {
58
private var statusItem: NSStatusItem
@@ -108,7 +111,7 @@ class StatusBarController {
108111
launchAtLoginItem.state = .on
109112
}
110113
} catch {
111-
print("Failed to toggle launch at login: \(error)")
114+
logger.error("Failed to toggle launch at login: \(error)")
112115
}
113116
}
114117
}

0 commit comments

Comments
 (0)