fix(mac): harden gateway frame decoding

2025-12-12 17:30:21 +00:00
parent 957d7fbe2a
commit 79818f73c0
8 changed files with 153 additions and 131 deletions
--- a/apps/macos/Sources/Clawdis/InstanceIdentity.swift
+++ b/apps/macos/Sources/Clawdis/InstanceIdentity.swift
@@ -7,8 +7,8 @@ enum InstanceIdentity {
    private static var defaults: UserDefaults {
        UserDefaults(suiteName: suiteName) ?? .standard
    }
-
    static let instanceId: String = {
+        let defaults = Self.defaults
        if let existing = defaults.string(forKey: instanceIdKey)?
            .trimmingCharacters(in: .whitespacesAndNewlines),
            !existing.isEmpty
--- a/apps/macos/Sources/Clawdis/Utilities.swift
+++ b/apps/macos/Sources/Clawdis/Utilities.swift
@@ -303,8 +303,12 @@ enum CommandResolver {
        return false
    }

-    static func clawdisNodeCommand(subcommand: String, extraArgs: [String] = []) -> [String] {
-        let settings = self.connectionSettings()
+    static func clawdisNodeCommand(
+        subcommand: String,
+        extraArgs: [String] = [],
+        defaults: UserDefaults = .standard) -> [String]
+    {
+        let settings = self.connectionSettings(defaults: defaults)
        if settings.mode == .remote, let ssh = self.sshNodeCommand(
            subcommand: subcommand,
            extraArgs: extraArgs,
@@ -343,8 +347,12 @@ enum CommandResolver {
        }
    }

-    static func clawdisMacCommand(subcommand: String, extraArgs: [String] = []) -> [String] {
-        let settings = self.connectionSettings()
+    static func clawdisMacCommand(
+        subcommand: String,
+        extraArgs: [String] = [],
+        defaults: UserDefaults = .standard) -> [String]
+    {
+        let settings = self.connectionSettings(defaults: defaults)
        if settings.mode == .remote, let ssh = self.sshMacHelperCommand(
            subcommand: subcommand,
            extraArgs: extraArgs,
@@ -359,8 +367,12 @@ enum CommandResolver {
    }

    // Existing callers still refer to clawdisCommand; keep it as node alias.
-    static func clawdisCommand(subcommand: String, extraArgs: [String] = []) -> [String] {
-        self.clawdisNodeCommand(subcommand: subcommand, extraArgs: extraArgs)
+    static func clawdisCommand(
+        subcommand: String,
+        extraArgs: [String] = [],
+        defaults: UserDefaults = .standard) -> [String]
+    {
+        self.clawdisNodeCommand(subcommand: subcommand, extraArgs: extraArgs, defaults: defaults)
    }

    // MARK: - SSH helpers
@@ -477,12 +489,12 @@ enum CommandResolver {
        let projectRoot: String
    }

-    static func connectionSettings() -> RemoteSettings {
-        let modeRaw = UserDefaults.standard.string(forKey: connectionModeKey) ?? "local"
+    static func connectionSettings(defaults: UserDefaults = .standard) -> RemoteSettings {
+        let modeRaw = defaults.string(forKey: connectionModeKey) ?? "local"
        let mode = AppState.ConnectionMode(rawValue: modeRaw) ?? .local
-        let target = UserDefaults.standard.string(forKey: remoteTargetKey) ?? ""
-        let identity = UserDefaults.standard.string(forKey: remoteIdentityKey) ?? ""
-        let projectRoot = UserDefaults.standard.string(forKey: remoteProjectRootKey) ?? ""
+        let target = defaults.string(forKey: remoteTargetKey) ?? ""
+        let identity = defaults.string(forKey: remoteIdentityKey) ?? ""
+        let projectRoot = defaults.string(forKey: remoteProjectRootKey) ?? ""
        return RemoteSettings(
            mode: mode,
            target: self.sanitizedTarget(target),
@@ -494,8 +506,8 @@ enum CommandResolver {
        UserDefaults.standard.bool(forKey: attachExistingGatewayOnlyKey)
    }

-    static func connectionModeIsRemote() -> Bool {
-        self.connectionSettings().mode == .remote
+    static func connectionModeIsRemote(defaults: UserDefaults = .standard) -> Bool {
+        self.connectionSettings(defaults: defaults).mode == .remote
    }

    private static func sanitizedTarget(_ raw: String) -> String {
--- a/apps/macos/Sources/ClawdisProtocol/GatewayModels.swift
+++ b/apps/macos/Sources/ClawdisProtocol/GatewayModels.swift
@@ -545,26 +545,29 @@ public enum GatewayFrame: Codable {
    case event(EventFrame)
    case unknown(type: String, raw: [String: AnyCodable])

+    private enum CodingKeys: String, CodingKey {
+        case type
+    }
+
    public init(from decoder: Decoder) throws {
-        let container = try decoder.singleValueContainer()
-        let raw = try container.decode([String: AnyCodable].self)
-        guard let type = raw["type"]?.value as? String else {
-            throw DecodingError.dataCorruptedError(in: container, debugDescription: "missing type")
-        }
+        let typeContainer = try decoder.container(keyedBy: CodingKeys.self)
+        let type = try typeContainer.decode(String.self, forKey: .type)
        switch type {
        case "hello":
-            self = .hello(try Self.decodePayload(Hello.self, from: raw))
+            self = .hello(try Hello(from: decoder))
        case "hello-ok":
-            self = .helloOk(try Self.decodePayload(HelloOk.self, from: raw))
+            self = .helloOk(try HelloOk(from: decoder))
        case "hello-error":
-            self = .helloError(try Self.decodePayload(HelloError.self, from: raw))
+            self = .helloError(try HelloError(from: decoder))
        case "req":
-            self = .req(try Self.decodePayload(RequestFrame.self, from: raw))
+            self = .req(try RequestFrame(from: decoder))
        case "res":
-            self = .res(try Self.decodePayload(ResponseFrame.self, from: raw))
+            self = .res(try ResponseFrame(from: decoder))
        case "event":
-            self = .event(try Self.decodePayload(EventFrame.self, from: raw))
+            self = .event(try EventFrame(from: decoder))
        default:
+            let container = try decoder.singleValueContainer()
+            let raw = try container.decode([String: AnyCodable].self)
            self = .unknown(type: type, raw: raw)
        }
    }
@@ -583,13 +586,4 @@ public enum GatewayFrame: Codable {
        }
    }

-
-    private static func decodePayload<T: Decodable>(_ type: T.Type, from raw: [String: AnyCodable]) throws -> T {
-        // raw is [String: AnyCodable] which is not directly JSONSerialization-compatible.
-        // Round-trip through JSONEncoder so AnyCodable can encode itself safely.
-        let data = try JSONEncoder().encode(raw)
-        let decoder = JSONDecoder()
-        return try decoder.decode(T.self, from: data)
-    }
-
 }
--- a/apps/macos/Tests/ClawdisIPCTests/AgentRPCTests.swift
+++ b/apps/macos/Tests/ClawdisIPCTests/AgentRPCTests.swift
@@ -11,7 +11,7 @@ import Testing
    }

    @Test func rejectEmptyMessage() async {
-        let result = await AgentRPC.shared.send(text: "", thinking: nil, session: "main", deliver: false, to: nil)
+        let result = await AgentRPC.shared.send(text: "", thinking: nil, sessionKey: "main", deliver: false, to: nil)
        #expect(result.ok == false)
    }
 }
--- a/apps/macos/Tests/ClawdisIPCTests/CommandResolverTests.swift
+++ b/apps/macos/Tests/ClawdisIPCTests/CommandResolverTests.swift
@@ -4,6 +4,11 @@ import Testing
@testable import Clawdis

@Suite(.serialized) struct CommandResolverTests {
+    private func makeDefaults() -> UserDefaults {
+        // Use a unique suite to avoid cross-suite concurrency on UserDefaults.standard.
+        UserDefaults(suiteName: "CommandResolverTests.\(UUID().uuidString)")!
+    }
+
    private func makeTempDir() throws -> URL {
        let base = URL(fileURLWithPath: NSTemporaryDirectory(), isDirectory: true)
        let dir = base.appendingPathComponent(UUID().uuidString, isDirectory: true)
@@ -20,16 +25,8 @@ import Testing
    }

    @Test func prefersClawdisBinary() async throws {
-        UserDefaults.standard.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey)
-        UserDefaults.standard.removeObject(forKey: remoteTargetKey)
-        UserDefaults.standard.removeObject(forKey: remoteIdentityKey)
-        UserDefaults.standard.removeObject(forKey: remoteProjectRootKey)
-        defer {
-            UserDefaults.standard.removeObject(forKey: connectionModeKey)
-            UserDefaults.standard.removeObject(forKey: remoteTargetKey)
-            UserDefaults.standard.removeObject(forKey: remoteIdentityKey)
-            UserDefaults.standard.removeObject(forKey: remoteProjectRootKey)
-        }
+        let defaults = self.makeDefaults()
+        defaults.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey)

        let tmp = try makeTempDir()
        CommandResolver.setProjectRoot(tmp.path)
@@ -37,21 +34,13 @@ import Testing
        let clawdisPath = tmp.appendingPathComponent("node_modules/.bin/clawdis")
        try self.makeExec(at: clawdisPath)

-        let cmd = CommandResolver.clawdisCommand(subcommand: "gateway")
+        let cmd = CommandResolver.clawdisCommand(subcommand: "gateway", defaults: defaults)
        #expect(cmd.prefix(2).elementsEqual([clawdisPath.path, "gateway"]))
    }

    @Test func fallsBackToNodeAndScript() async throws {
-        UserDefaults.standard.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey)
-        UserDefaults.standard.removeObject(forKey: remoteTargetKey)
-        UserDefaults.standard.removeObject(forKey: remoteIdentityKey)
-        UserDefaults.standard.removeObject(forKey: remoteProjectRootKey)
-        defer {
-            UserDefaults.standard.removeObject(forKey: connectionModeKey)
-            UserDefaults.standard.removeObject(forKey: remoteTargetKey)
-            UserDefaults.standard.removeObject(forKey: remoteIdentityKey)
-            UserDefaults.standard.removeObject(forKey: remoteProjectRootKey)
-        }
+        let defaults = self.makeDefaults()
+        defaults.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey)

        let tmp = try makeTempDir()
        CommandResolver.setProjectRoot(tmp.path)
@@ -63,7 +52,7 @@ import Testing
        try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: nodePath.path)
        try self.makeExec(at: scriptPath)

-        let cmd = CommandResolver.clawdisCommand(subcommand: "rpc")
+        let cmd = CommandResolver.clawdisCommand(subcommand: "rpc", defaults: defaults)

        #expect(cmd.count >= 3)
        #expect(cmd[0] == nodePath.path)
@@ -72,16 +61,8 @@ import Testing
    }

    @Test func fallsBackToPnpm() async throws {
-        UserDefaults.standard.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey)
-        UserDefaults.standard.removeObject(forKey: remoteTargetKey)
-        UserDefaults.standard.removeObject(forKey: remoteIdentityKey)
-        UserDefaults.standard.removeObject(forKey: remoteProjectRootKey)
-        defer {
-            UserDefaults.standard.removeObject(forKey: connectionModeKey)
-            UserDefaults.standard.removeObject(forKey: remoteTargetKey)
-            UserDefaults.standard.removeObject(forKey: remoteIdentityKey)
-            UserDefaults.standard.removeObject(forKey: remoteProjectRootKey)
-        }
+        let defaults = self.makeDefaults()
+        defaults.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey)

        let tmp = try makeTempDir()
        CommandResolver.setProjectRoot(tmp.path)
@@ -89,22 +70,14 @@ import Testing
        let pnpmPath = tmp.appendingPathComponent("node_modules/.bin/pnpm")
        try self.makeExec(at: pnpmPath)

-        let cmd = CommandResolver.clawdisCommand(subcommand: "rpc")
+        let cmd = CommandResolver.clawdisCommand(subcommand: "rpc", defaults: defaults)

        #expect(cmd.prefix(4).elementsEqual([pnpmPath.path, "--silent", "clawdis", "rpc"]))
    }

    @Test func pnpmKeepsExtraArgsAfterSubcommand() async throws {
-        UserDefaults.standard.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey)
-        UserDefaults.standard.removeObject(forKey: remoteTargetKey)
-        UserDefaults.standard.removeObject(forKey: remoteIdentityKey)
-        UserDefaults.standard.removeObject(forKey: remoteProjectRootKey)
-        defer {
-            UserDefaults.standard.removeObject(forKey: connectionModeKey)
-            UserDefaults.standard.removeObject(forKey: remoteTargetKey)
-            UserDefaults.standard.removeObject(forKey: remoteIdentityKey)
-            UserDefaults.standard.removeObject(forKey: remoteProjectRootKey)
-        }
+        let defaults = self.makeDefaults()
+        defaults.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey)

        let tmp = try makeTempDir()
        CommandResolver.setProjectRoot(tmp.path)
@@ -112,24 +85,16 @@ import Testing
        let pnpmPath = tmp.appendingPathComponent("node_modules/.bin/pnpm")
        try self.makeExec(at: pnpmPath)

-        let cmd = CommandResolver.clawdisCommand(subcommand: "health", extraArgs: ["--json", "--timeout", "5"])
+        let cmd = CommandResolver.clawdisCommand(
+            subcommand: "health",
+            extraArgs: ["--json", "--timeout", "5"],
+            defaults: defaults)

        #expect(cmd.prefix(5).elementsEqual([pnpmPath.path, "--silent", "clawdis", "health", "--json"]))
        #expect(cmd.suffix(2).elementsEqual(["--timeout", "5"]))
    }

    @Test func preferredPathsStartWithProjectNodeBins() async throws {
-        UserDefaults.standard.set(AppState.ConnectionMode.local.rawValue, forKey: connectionModeKey)
-        UserDefaults.standard.removeObject(forKey: remoteTargetKey)
-        UserDefaults.standard.removeObject(forKey: remoteIdentityKey)
-        UserDefaults.standard.removeObject(forKey: remoteProjectRootKey)
-        defer {
-            UserDefaults.standard.removeObject(forKey: connectionModeKey)
-            UserDefaults.standard.removeObject(forKey: remoteTargetKey)
-            UserDefaults.standard.removeObject(forKey: remoteIdentityKey)
-            UserDefaults.standard.removeObject(forKey: remoteProjectRootKey)
-        }
-
        let tmp = try makeTempDir()
        CommandResolver.setProjectRoot(tmp.path)

@@ -138,18 +103,13 @@ import Testing
    }

    @Test func buildsSSHCommandForRemoteMode() async throws {
-        UserDefaults.standard.set(AppState.ConnectionMode.remote.rawValue, forKey: connectionModeKey)
-        UserDefaults.standard.set("clawd@example.com:2222", forKey: remoteTargetKey)
-        UserDefaults.standard.set("/tmp/id_ed25519", forKey: remoteIdentityKey)
-        UserDefaults.standard.set("/srv/clawdis", forKey: remoteProjectRootKey)
-        defer {
-            UserDefaults.standard.removeObject(forKey: connectionModeKey)
-            UserDefaults.standard.removeObject(forKey: remoteTargetKey)
-            UserDefaults.standard.removeObject(forKey: remoteIdentityKey)
-            UserDefaults.standard.removeObject(forKey: remoteProjectRootKey)
-        }
+        let defaults = self.makeDefaults()
+        defaults.set(AppState.ConnectionMode.remote.rawValue, forKey: connectionModeKey)
+        defaults.set("clawd@example.com:2222", forKey: remoteTargetKey)
+        defaults.set("/tmp/id_ed25519", forKey: remoteIdentityKey)
+        defaults.set("/srv/clawdis", forKey: remoteProjectRootKey)

-        let cmd = CommandResolver.clawdisCommand(subcommand: "status", extraArgs: ["--json"])
+        let cmd = CommandResolver.clawdisCommand(subcommand: "status", extraArgs: ["--json"], defaults: defaults)

        #expect(cmd.first == "/usr/bin/ssh")
        #expect(cmd.contains("clawd@example.com"))
--- a/apps/macos/Tests/ClawdisIPCTests/GatewayFrameDecodeTests.swift
+++ b/apps/macos/Tests/ClawdisIPCTests/GatewayFrameDecodeTests.swift
@@ -28,4 +28,71 @@ import Testing
        #expect(payload?["count"]?.value as? Int == 1)
        #expect(evt.seq == 7)
    }
+
+    @Test func decodesRequestFrameWithNestedParams() throws {
+        let json = """
+        {
+          "type": "req",
+          "id": "1",
+          "method": "agent.send",
+          "params": {
+            "text": "hi",
+            "items": [1, null, {"ok": true}],
+            "meta": { "count": 2 }
+          }
+        }
+        """
+
+        let frame = try JSONDecoder().decode(GatewayFrame.self, from: Data(json.utf8))
+
+        #expect({
+            if case .req = frame { true } else { false }
+        }(), "expected .req frame")
+
+        guard case let .req(req) = frame else {
+            return
+        }
+
+        let params = req.params?.value as? [String: AnyCodable]
+        #expect(params?["text"]?.value as? String == "hi")
+
+        let items = params?["items"]?.value as? [AnyCodable]
+        #expect(items?.count == 3)
+        #expect(items?[0].value as? Int == 1)
+        #expect(items?[1].value is NSNull)
+
+        let item2 = items?[2].value as? [String: AnyCodable]
+        #expect(item2?["ok"]?.value as? Bool == true)
+
+        let meta = params?["meta"]?.value as? [String: AnyCodable]
+        #expect(meta?["count"]?.value as? Int == 2)
+    }
+
+    @Test func decodesUnknownFrameAndPreservesRaw() throws {
+        let json = """
+        {
+          "type": "made-up",
+          "foo": "bar",
+          "count": 1,
+          "nested": { "ok": true }
+        }
+        """
+
+        let frame = try JSONDecoder().decode(GatewayFrame.self, from: Data(json.utf8))
+
+        #expect({
+            if case .unknown = frame { true } else { false }
+        }(), "expected .unknown frame")
+
+        guard case let .unknown(type, raw) = frame else {
+            return
+        }
+
+        #expect(type == "made-up")
+        #expect(raw["type"]?.value as? String == "made-up")
+        #expect(raw["foo"]?.value as? String == "bar")
+        #expect(raw["count"]?.value as? Int == 1)
+        let nested = raw["nested"]?.value as? [String: AnyCodable]
+        #expect(nested?["ok"]?.value as? Bool == true)
+    }
 }
--- a/apps/macos/Tests/ClawdisIPCTests/UtilitiesTests.swift
+++ b/apps/macos/Tests/ClawdisIPCTests/UtilitiesTests.swift
@@ -2,7 +2,7 @@ import Foundation
 import Testing
@testable import Clawdis

-@Suite struct UtilitiesTests {
+@Suite(.serialized) struct UtilitiesTests {
    @Test func ageStringsCoverCommonWindows() {
        let now = Date(timeIntervalSince1970: 1_000_000)
        #expect(age(from: now, now: now) == "just now")
@@ -33,14 +33,11 @@ import Testing
    }

    @Test func sanitizedTargetStripsLeadingSSHPrefix() {
-        UserDefaults.standard.set(AppState.ConnectionMode.remote.rawValue, forKey: connectionModeKey)
-        UserDefaults.standard.set("ssh  alice@example.com", forKey: remoteTargetKey)
-        defer {
-            UserDefaults.standard.removeObject(forKey: connectionModeKey)
-            UserDefaults.standard.removeObject(forKey: remoteTargetKey)
-        }
+        let defaults = UserDefaults(suiteName: "UtilitiesTests.\(UUID().uuidString)")!
+        defaults.set(AppState.ConnectionMode.remote.rawValue, forKey: connectionModeKey)
+        defaults.set("ssh  alice@example.com", forKey: remoteTargetKey)

-        let settings = CommandResolver.connectionSettings()
+        let settings = CommandResolver.connectionSettings(defaults: defaults)
        #expect(settings.mode == .remote)
        #expect(settings.target == "alice@example.com")
    }
--- a/scripts/protocol-gen-swift.ts
+++ b/scripts/protocol-gen-swift.ts
@@ -157,26 +157,29 @@ function emitGatewayFrame(): string {
  };
  const caseLines = cases.map((c) => `    case ${safeName(c)}(${associated[c]})`);
  const initLines = `
+    private enum CodingKeys: String, CodingKey {
+        case type
+    }
+
    public init(from decoder: Decoder) throws {
-        let container = try decoder.singleValueContainer()
-        let raw = try container.decode([String: AnyCodable].self)
-        guard let type = raw["type"]?.value as? String else {
-            throw DecodingError.dataCorruptedError(in: container, debugDescription: "missing type")
-        }
+        let typeContainer = try decoder.container(keyedBy: CodingKeys.self)
+        let type = try typeContainer.decode(String.self, forKey: .type)
        switch type {
        case "hello":
-            self = .hello(try Self.decodePayload(Hello.self, from: raw))
+            self = .hello(try Hello(from: decoder))
        case "hello-ok":
-            self = .helloOk(try Self.decodePayload(HelloOk.self, from: raw))
+            self = .helloOk(try HelloOk(from: decoder))
        case "hello-error":
-            self = .helloError(try Self.decodePayload(HelloError.self, from: raw))
+            self = .helloError(try HelloError(from: decoder))
        case "req":
-            self = .req(try Self.decodePayload(RequestFrame.self, from: raw))
+            self = .req(try RequestFrame(from: decoder))
        case "res":
-            self = .res(try Self.decodePayload(ResponseFrame.self, from: raw))
+            self = .res(try ResponseFrame(from: decoder))
        case "event":
-            self = .event(try Self.decodePayload(EventFrame.self, from: raw))
+            self = .event(try EventFrame(from: decoder))
        default:
+            let container = try decoder.singleValueContainer()
+            let raw = try container.decode([String: AnyCodable].self)
            self = .unknown(type: type, raw: raw)
        }
    }
@@ -196,22 +199,11 @@ function emitGatewayFrame(): string {
    }
 `;

-  const helper = `
-    private static func decodePayload<T: Decodable>(_ type: T.Type, from raw: [String: AnyCodable]) throws -> T {
-        // raw is [String: AnyCodable] which is not directly JSONSerialization-compatible.
-        // Round-trip through JSONEncoder so AnyCodable can encode itself safely.
-        let data = try JSONEncoder().encode(raw)
-        let decoder = JSONDecoder()
-        return try decoder.decode(T.self, from: data)
-    }
-`;
-
  return [
    "public enum GatewayFrame: Codable {",
    ...caseLines,
    "    case unknown(type: String, raw: [String: AnyCodable])",
    initLines,
-    helper,
    "}",
    "",
  ].join("\n");