[agent] Hardware-testing fixes for the Swift client

swift/Sources/HarkCore/ClientConfig.swift

@@ -0,0 +1,136 @@
+import Foundation
+
+/// Client-side configuration: which server to dictate to, and with what key.
+///
+/// Distinct from `HarkConfig`, which describes how the SERVER deploys itself.
+/// Conflating them is what left the agent hardcoded to
+/// `http://127.0.0.1:\(config.harkPort)/dictate` with the key read from
+/// `~/.config/hark/key` — correct for a single machine, and unusable for the
+/// two-machine setup this project exists to serve, because on the recording
+/// Mac there is no server, no key file, and nothing listening on loopback.
+///
+/// Worth noting the single-machine case is not automatically loopback either:
+/// a server that binds a tailnet address to serve a laptop is NOT reachable at
+/// 127.0.0.1, so the machine running the server needs a real client address
+/// too.
+///
+/// Lives at `~/.config/hark/client.json`:
+///
+///     {
+///       "server": "http://100.64.66.46:8911/dictate",
+///       "key": "...",
+///       "allowPlaintext": true
+///     }
+public struct ClientConfig {
+    public let serverURL: URL
+    public let key: String
+
+    public enum Failure: Error, CustomStringConvertible {
+        case unreadable(String)
+        case malformed(String)
+        case insecure(String)
+
+        public var description: String {
+            switch self {
+            case .unreadable(let m), .malformed(let m), .insecure(let m): return m
+            }
+        }
+    }
+
+    /// Test-only override, mirroring `KeyFile.pathOverride`.
+    public static var pathOverride: URL?
+
+    /// HARK_CLIENT_CONFIG overrides the path, mirroring the server side's
+    /// HARK_CONFIG. Neither FileManager.homeDirectoryForCurrentUser nor
+    /// NSHomeDirectory() reliably honours $HOME on macOS — both resolve through
+    /// the user database — so `HOME=... hark agent` silently reads the real
+    /// config. That produced a round of "passing" policy checks that were all
+    /// reading the same live file. An explicit variable is the only honest way
+    /// to point this somewhere else.
+    public static var path: URL {
+        if let pathOverride { return pathOverride }
+        if let env = ProcessInfo.processInfo.environment["HARK_CLIENT_CONFIG"], !env.isEmpty {
+            return URL(fileURLWithPath: env)
+        }
+        return FileManager.default.homeDirectoryForCurrentUser
+            .appendingPathComponent(".config/hark/client.json")
+    }
+
+    private struct Wire: Decodable {
+        let server: String?
+        let key: String?
+        let allowPlaintext: Bool?
+    }
+
+    /// Reads client.json. Falls back to the single-machine defaults — loopback
+    /// plus `~/.config/hark/key` — when the file is absent, so an existing
+    /// same-machine install keeps working untouched.
+    public static func load(defaultPort: Int) throws -> ClientConfig {
+        guard let data = try? Data(contentsOf: path) else {
+            let url = URL(string: "http://127.0.0.1:\(defaultPort)/dictate")!
+            return ClientConfig(serverURL: url, key: KeyFile.load() ?? "")
+        }
+        guard let wire = try? JSONDecoder().decode(Wire.self, from: data) else {
+            throw Failure.malformed("\(path.path) is not valid JSON")
+        }
+
+        let raw = wire.server ?? "http://127.0.0.1:\(defaultPort)/dictate"
+        guard let url = URL(string: raw), let host = url.host, let scheme = url.scheme else {
+            throw Failure.malformed("\(path.path): \"\(raw)\" is not a usable URL")
+        }
+
+        try validateTransport(url: url, host: host, scheme: scheme,
+                              allowPlaintext: wire.allowPlaintext ?? false)
+
+        let key = wire.key ?? KeyFile.load() ?? ""
+        guard !key.isEmpty else {
+            throw Failure.unreadable(
+                "no shared secret: set \"key\" in \(path.path), or place the server's "
+                + "~/.config/hark/key on this Mac")
+        }
+        return ClientConfig(serverURL: url, key: key)
+    }
+
+    /// The transport policy from the design doc, enforced rather than assumed:
+    ///
+    ///   - plain HTTP to numeric loopback is always fine — nothing leaves the host
+    ///   - plain HTTP to a numeric IP is a defensible choice on a tailnet, but it
+    ///     must be a STATED one, so it needs `allowPlaintext`
+    ///   - plain HTTP to a hostname is refused outright. A name resolves through
+    ///     something, and "the tailnet is trusted" stops being true the moment
+    ///     the name resolves somewhere else. Use the tailnet IP.
+    static func validateTransport(url: URL, host: String, scheme: String,
+                                  allowPlaintext: Bool) throws {
+        guard scheme == "http" else { return } // https needs no argument
+        if isLoopback(host) { return }
+
+        guard isNumericIP(host) else {
+            throw Failure.insecure(
+                "\(path.path): plain HTTP to the hostname \"\(host)\" is not allowed. "
+                + "Use the numeric address (a Tailscale MagicDNS name is a hostname), "
+                + "or https://")
+        }
+        guard allowPlaintext else {
+            throw Failure.insecure(
+                "\(path.path): plain HTTP to \(host) needs \"allowPlaintext\": true. "
+                + "Audio and transcripts cross the network unencrypted; on a tailnet "
+                + "that is defensible, but it should be a decision you made.")
+        }
+    }
+
+    static func isLoopback(_ host: String) -> Bool {
+        host == "127.0.0.1" || host == "::1" || host == "localhost"
+    }
+
+    /// IPv4 dotted-quad or anything bracketed/colon-bearing (IPv6). Deliberately
+    /// narrow: the question is only "did the user give an address or a name".
+    static func isNumericIP(_ host: String) -> Bool {
+        if host.contains(":") { return true }
+        let parts = host.split(separator: ".", omittingEmptySubsequences: false)
+        guard parts.count == 4 else { return false }
+        return parts.allSatisfy { p in
+            guard !p.isEmpty, p.count <= 3, p.allSatisfy(\.isNumber) else { return false }
+            return Int(p).map { (0...255).contains($0) } ?? false
+        }
+    }
+}

swift/Sources/hark/AgentController.swift

@@ -15,6 +15,7 @@ public enum AgentState { case idle, starting, recording, stopping, uploading }
 
 public final class AgentController: NSObject {
     private let config: HarkConfig
+    private let clientConfig: ClientConfig
     private let hotkey = Hotkey()
     private let recorder = Recorder()
     private var dictate: DictateClient
@@ -31,8 +32,24 @@ public final class AgentController: NSObject {
     public init(config: HarkConfig) {
         self.config = config
         self.log = Log()
-        let url = URL(string: "http://127.0.0.1:\(config.harkPort)/dictate")!
-        self.dictate = DictateClient(url: url, key: KeyFile.load() ?? "")
+
+        // Client addressing is NOT the server's deployment config. A recording
+        // Mac has no server, no key file and nothing on loopback; and a server
+        // bound to a tailnet address is unreachable at 127.0.0.1 even on the
+        // machine running it. See ClientConfig.
+        let client: ClientConfig
+        do {
+            client = try ClientConfig.load(defaultPort: config.harkPort)
+        } catch {
+            // Fatal on purpose. Continuing would build a client pointed at
+            // loopback that fails on every utterance with a connection error,
+            // which reads as "the server is down" rather than "your config is
+            // wrong" — and that misdirection is expensive.
+            FileHandle.standardError.write("hark: \(error)\n".data(using: .utf8)!)
+            exit(2)
+        }
+        self.clientConfig = client
+        self.dictate = DictateClient(url: client.serverURL, key: client.key)
         super.init()
     }
 
@@ -45,7 +62,7 @@ public final class AgentController: NSObject {
         hotkey.onPress = { [weak self] in self?.beginCapture() }
         hotkey.onRelease = { [weak self] in self?.endCapture() }
         if !hotkey.register() {
-            alert("hark: Accessibility is NOT granted. The hotkey (Ctrl+Alt+Space) cannot work until you enable it.")
+            alert("hark: could not bind Ctrl+Alt+Space — something else is probably holding it.")
         }
 
         heartbeatTimer = Timer.scheduledTimer(withTimeInterval: 30, repeats: true) { [weak self] _ in
@@ -139,15 +156,29 @@ public final class AgentController: NSObject {
 
     // MARK: - Paste
 
-    private func apply(_ text: String) {
+    private func apply(_ transcript: String) {
+        // A trailing space, so consecutive dictations do not run together.
+        //
+        // The server sanitiser ends with .strip(), and Whisper's own leading
+        // space goes with it — correct for an API, which should return the
+        // transcript and not presentation whitespace. But two dictations into
+        // the same field then paste as "one, two, three.one, two, three." with
+        // nothing between them. Long-standing; the Lua client did the same.
+        //
+        // Trailing rather than leading: a leading space would open an empty
+        // field with whitespace, and there is no reliable way to read what sits
+        // immediately before the cursor to decide. Trailing is occasionally
+        // redundant and never wrong.
+        let text = transcript + " "
+
         // Set the pasteboard and verify the write before synthesising ⌘V.
         let pb = NSPasteboard.general
         pb.clearContents()
         guard pb.setString(text, forType: .string) else {
             alert("hark: could not write to the pasteboard — not pasting.")
             return
         }
-        log.info("pasting \(text.count) chars")
+        log.info("pasting \(transcript.count) chars")
         // Paste-target policy: never type into whatever gained focus since release.
         if frontmostAppName() != frontmostAtRelease {
             brief("transcript is on the clipboard — paste withheld (focus moved)")
@@ -161,14 +192,36 @@ public final class AgentController: NSObject {
     }
 
     private func paste() {
+        // Re-checked here, not just at startup: the grant can be revoked, or
+        // silently invalidated by a rebuild, while the process keeps running,
+        // and this is the moment it matters. Without it every step above still
+        // reports success — the pasteboard write, the log line — and an
+        // untrusted process is indistinguishable from a dropped event.
+        guard accessibilityTrusted() else {
+            alert("hark: Accessibility is not granted, so the transcript cannot be pasted. "
+                + "It IS on the clipboard — press ⌘V. "
+                + "System Settings -> Privacy & Security -> Accessibility -> turn ON hark.")
+            return
+        }
+
         // Synthesise ⌘V. NEVER Return/Enter — auto-submit is a hard non-goal.
         let source = CGEventSource(stateID: .hidSystemState)
         let down = CGEvent(keyboardEventSource: source, virtualKey: 9, keyDown: true) // kVK_ANSI_V
         down?.flags = .maskCommand
         down?.post(tap: .cghidEventTap)
-        let up = CGEvent(keyboardEventSource: source, virtualKey: 9, keyDown: false)
-        up?.flags = .maskCommand
-        up?.post(tap: .cghidEventTap)
+
+        // HOLD THE KEY. Posting key-up immediately after key-down is a
+        // zero-duration keystroke, which some apps silently drop — the events
+        // arrive, nothing acts on them, and the transcript never appears.
+        // hs.eventtap.keyStroke, the implementation being replaced, holds for
+        // 200 ms (`local keyDelay = 200000`, then usleep between down and up);
+        // the tap and the flags are otherwise identical. Async rather than a
+        // sleep so the run loop is not stalled behind it.
+        DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) {
+            let up = CGEvent(keyboardEventSource: source, virtualKey: 9, keyDown: false)
+            up?.flags = .maskCommand
+            up?.post(tap: .cghidEventTap)
+        }
     }
 
     private func clearIfOwned() {
@@ -182,6 +235,14 @@ public final class AgentController: NSObject {
 
     private func showRecordingIndicator(_ on: Bool) {
         statusItem?.button?.title = on ? "●" : "hark"
+        // The menu bar title alone is easy to miss on a crowded bar, and it is
+        // the only signal that the mic is actually open. nil duration: cleared
+        // when capture really ends, never on a timer.
+        if on {
+            Overlay.shared.show("● Recording…", duration: nil)
+        } else {
+            Overlay.shared.hide()
+        }
     }
 
     private func setupMenuBar() {
@@ -197,10 +258,21 @@ public final class AgentController: NSObject {
 
     private func alert(_ message: String) {
         log.info(message)
-        NSWorkspace.shared.notificationCenter.post(name: .init("HarkAlert"), object: message)
+        // Was posted to an NSWorkspace notification nobody observed, which
+        // discarded the whole diagnostic surface. Show it, and beep: if
+        // dictation does nothing the instinct is to try again, and a second
+        // silent failure reads as a broken mic rather than a stale key.
+        Overlay.beep()
+        Overlay.shared.show(message, duration: 7)
         NSSound(named: "Basso")?.play()
     }
-    private func brief(_ message: String) { _ = message }
+    /// Short, non-error feedback — "heard nothing", "paste withheld". Was a
+    /// no-op, so the two states a user is most likely to hit and misread as a
+    /// hang produced no feedback at all. No beep: neither is a failure.
+    private func brief(_ message: String) {
+        log.info(message)
+        Overlay.shared.show(message, duration: 2)
+    }
 
     private func present(_ error: DictateError) -> String {
         switch error {
@@ -222,7 +294,7 @@ public final class AgentController: NSObject {
         return ""
     }
 
-    private var dictateServer: String { "http://127.0.0.1:\(config.harkPort)/dictate" }
+    private var dictateServer: String { clientConfig.serverURL.absoluteString }
 
     // MARK: - Permissions
 
@@ -239,7 +311,22 @@ public final class AgentController: NSObject {
     private func probePermissions() {
         // Raise the microphone dialog if never asked.
         if microphoneStatus() == .notDetermined {
-            AVCaptureDevice.requestAccess(for: .audio) { _ in }
+            // Wait for the answer rather than discarding it. The completion is
+            // delivered on an unspecified queue, so this pumps the run loop
+            // instead of blocking on a semaphore, which would deadlock if that
+            // queue turned out to be main. Continuing without the answer means
+            // recording the substituted silence macOS hands an ungranted
+            // process — full-length buffers of zeros, indistinguishable from a
+            // quiet room until you check the peak.
+            var granted: Bool?
+            AVCaptureDevice.requestAccess(for: .audio) { granted = $0 }
+            let deadline = Date(timeIntervalSinceNow: 60)
+            while granted == nil, Date() < deadline {
+                RunLoop.main.run(until: Date(timeIntervalSinceNow: 0.05))
+            }
+            if granted != true {
+                log.info("microphone access was not granted")
+            }
         }
         // AXIsProcessTrusted alone checks but never prompts; use the option.
         let opts = [kAXTrustedCheckOptionPrompt.takeUnretainedValue() as String: true] as CFDictionary
@@ -251,7 +338,7 @@ public final class AgentController: NSObject {
     private func writeHeartbeat() {
         let heartbeat = Heartbeat.current(microphone: microphoneStatus() == .authorized ? "authorized" : "denied",
                                           accessibility: accessibilityTrusted() ? "trusted" : "not_trusted",
-                                          hotkey: "registered")
+                                          hotkey: hotkey.isRegistered ? "registered" : "not_registered")
         heartbeat.write()
     }
 }