From 15866acef522681530563d27545fca3d35fcad44 Mon Sep 17 00:00:00 2001 From: "Bryan Tegomoh, MD, MPH" Date: Tue, 28 Jul 2026 06:10:45 -0700 Subject: [PATCH] Refresh macOS skills after node reconnect (#107660) * Refresh macOS skills after node reconnect * fix(macos): queue skills refresh during reconnect * chore(i18n): refresh native skills inventory * fix(macos): refresh skills from gateway invalidations Co-authored-by: Bryan Tegomoh * fix(android): update gateway event catalog Co-authored-by: Bryan Tegomoh * refactor(skills): keep remote eligibility helper bounded Co-authored-by: Bryan Tegomoh * ci(test): pin model shard worker budget Co-authored-by: Bryan Tegomoh * ci(test): isolate network-sensitive agent suites Co-authored-by: Bryan Tegomoh * test: align isolated prefix routing expectation Co-authored-by: Bryan Tegomoh * test(macos): isolate gateway process test port Co-authored-by: Bryan Tegomoh * fix(macos): recover skills after failed refresh Co-authored-by: Bryan Tegomoh * fix(ci): avoid duplicate agent test routing Co-authored-by: Bryan Tegomoh --------- Co-authored-by: Peter Steinberger --- apps/.i18n/native-source.json | 96 ++++---- .../openclaw/app/gateway/GatewayProtocol.kt | 1 + .../Sources/OpenClaw/SkillsSettings.swift | 78 ++++++- .../GatewayProcessManagerTests.swift | 48 +++- .../SkillsSettingsSmokeTests.swift | 208 ++++++++++++++++++ docs/gateway/protocol.md | 5 + docs/platforms/mac/skills.md | 4 + .../protocol-event-coverage.allowlist.json | 2 + src/gateway/server-broadcast.board.test.ts | 20 ++ src/gateway/server-broadcast.ts | 1 + src/gateway/server-methods-list.test.ts | 4 + src/gateway/server-methods-list.ts | 1 + src/gateway/server-startup-early.test.ts | 61 +++++ src/gateway/server-startup-early.ts | 15 +- src/skills/runtime/remote-probe-utils.ts | 17 ++ src/skills/runtime/remote.test.ts | 18 ++ src/skills/runtime/remote.ts | 7 + 17 files changed, 525 insertions(+), 61 deletions(-) diff --git a/apps/.i18n/native-source.json b/apps/.i18n/native-source.json index bc2e1bb1654f..2de7930ca3a6 100644 --- a/apps/.i18n/native-source.json +++ b/apps/.i18n/native-source.json @@ -37875,7 +37875,7 @@ }, { "kind": "ui-named-argument", - "line": 24, + "line": 23, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Skills", "surface": "apple", @@ -37883,7 +37883,7 @@ }, { "kind": "ui-named-argument", - "line": 25, + "line": 24, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Manage installed capabilities or discover verified releases on ClawHub.", "surface": "apple", @@ -37891,7 +37891,7 @@ }, { "kind": "ui-call", - "line": 62, + "line": 66, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Skills section", "surface": "apple", @@ -37899,7 +37899,7 @@ }, { "kind": "conditional-branch", - "line": 87, + "line": 91, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Loading skills", "surface": "apple", @@ -37907,7 +37907,7 @@ }, { "kind": "conditional-branch", - "line": 87, + "line": 91, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "\\(ready) ready · \\(needsSetup) need setup", "surface": "apple", @@ -37915,7 +37915,7 @@ }, { "kind": "ui-call", - "line": 89, + "line": 93, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Enable ready skills, or install missing tools on the Gateway or this Mac.", "surface": "apple", @@ -37923,7 +37923,7 @@ }, { "kind": "ui-call", - "line": 98, + "line": 102, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "\\(total)", "surface": "apple", @@ -37931,7 +37931,7 @@ }, { "kind": "ui-call", - "line": 113, + "line": 117, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Controls", "surface": "apple", @@ -37939,7 +37939,7 @@ }, { "kind": "ui-named-argument", - "line": 115, + "line": 119, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Skill catalog", "surface": "apple", @@ -37947,7 +37947,7 @@ }, { "kind": "ui-named-argument", - "line": 116, + "line": 120, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Refresh after changing binaries, environment variables, or skill config.", "surface": "apple", @@ -37955,7 +37955,7 @@ }, { "kind": "ui-call", - "line": 126, + "line": 130, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Refresh", "surface": "apple", @@ -37963,7 +37963,7 @@ }, { "kind": "ui-call", - "line": 131, + "line": 135, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Search installed skills", "surface": "apple", @@ -37971,7 +37971,7 @@ }, { "kind": "conditional-branch", - "line": 158, + "line": 162, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Loading…", "surface": "apple", @@ -37979,7 +37979,7 @@ }, { "kind": "conditional-branch", - "line": 158, + "line": 162, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "No skills reported yet", "surface": "apple", @@ -37987,7 +37987,7 @@ }, { "kind": "conditional-branch", - "line": 159, + "line": 163, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Reading the Gateway skill catalog.", "surface": "apple", @@ -37995,7 +37995,7 @@ }, { "kind": "ui-named-argument", - "line": 194, + "line": 198, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "No skills match this filter.", "surface": "apple", @@ -38003,7 +38003,7 @@ }, { "kind": "ui-call", - "line": 206, + "line": 210, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Filter", "surface": "apple", @@ -38011,7 +38011,7 @@ }, { "kind": "conditional-branch", - "line": 249, + "line": 253, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Installed", "surface": "apple", @@ -38019,7 +38019,7 @@ }, { "kind": "conditional-branch", - "line": 250, + "line": 254, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Browse", "surface": "apple", @@ -38027,7 +38027,7 @@ }, { "kind": "conditional-branch", - "line": 268, + "line": 272, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "All", "surface": "apple", @@ -38035,7 +38035,7 @@ }, { "kind": "conditional-branch", - "line": 270, + "line": 274, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Ready", "surface": "apple", @@ -38043,7 +38043,7 @@ }, { "kind": "conditional-branch", - "line": 272, + "line": 276, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Needs Setup", "surface": "apple", @@ -38051,7 +38051,7 @@ }, { "kind": "conditional-branch", - "line": 274, + "line": 278, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Disabled", "surface": "apple", @@ -38059,7 +38059,7 @@ }, { "kind": "ui-call", - "line": 359, + "line": 363, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Website", "surface": "apple", @@ -38067,7 +38067,7 @@ }, { "kind": "ui-call", - "line": 393, + "line": 397, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Details", "surface": "apple", @@ -38075,7 +38075,7 @@ }, { "kind": "conditional-branch", - "line": 419, + "line": 423, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Needs setup", "surface": "apple", @@ -38083,7 +38083,7 @@ }, { "kind": "conditional-branch", - "line": 434, + "line": 438, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Managed", "surface": "apple", @@ -38091,7 +38091,7 @@ }, { "kind": "conditional-branch", - "line": 436, + "line": 440, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Workspace", "surface": "apple", @@ -38099,7 +38099,7 @@ }, { "kind": "conditional-branch", - "line": 438, + "line": 442, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Extra", "surface": "apple", @@ -38107,7 +38107,7 @@ }, { "kind": "conditional-branch", - "line": 440, + "line": 444, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Plugin", "surface": "apple", @@ -38115,7 +38115,7 @@ }, { "kind": "ui-call", - "line": 507, + "line": 511, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Missing binaries: \\(self.missingBins.joined(separator: \", \"))", "surface": "apple", @@ -38123,7 +38123,7 @@ }, { "kind": "ui-call", - "line": 512, + "line": 516, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Needs any binary: \\(self.missingAnyBins.joined(separator: \", \"))", "surface": "apple", @@ -38131,7 +38131,7 @@ }, { "kind": "ui-call", - "line": 517, + "line": 521, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Missing env: \\(self.missingEnv.joined(separator: \", \"))", "surface": "apple", @@ -38139,7 +38139,7 @@ }, { "kind": "ui-call", - "line": 522, + "line": 526, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Requires config: \\(self.missingConfig.joined(separator: \", \"))", "surface": "apple", @@ -38147,7 +38147,7 @@ }, { "kind": "ui-call", - "line": 527, + "line": 531, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Requires OS: \\(self.missingOS.joined(separator: \", \"))", "surface": "apple", @@ -38155,7 +38155,7 @@ }, { "kind": "conditional-branch", - "line": 554, + "line": 558, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Set \\(envKey)", "surface": "apple", @@ -38163,7 +38163,7 @@ }, { "kind": "ui-call", - "line": 570, + "line": 574, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Install on Gateway", "surface": "apple", @@ -38171,7 +38171,7 @@ }, { "kind": "ui-call", - "line": 583, + "line": 587, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Install on This Mac", "surface": "apple", @@ -38179,7 +38179,7 @@ }, { "kind": "conditional-branch", - "line": 588, + "line": 592, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Switches to Local mode to install on this Mac.", "surface": "apple", @@ -38187,7 +38187,7 @@ }, { "kind": "ui-call", - "line": 693, + "line": 697, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Get your key →", "surface": "apple", @@ -38195,7 +38195,7 @@ }, { "kind": "ui-call", - "line": 698, + "line": 702, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Saved to openclaw.json under skills.entries.\\(self.editor.skillKey)", "surface": "apple", @@ -38203,7 +38203,7 @@ }, { "kind": "ui-call", - "line": 702, + "line": 706, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Cancel", "surface": "apple", @@ -38211,7 +38211,7 @@ }, { "kind": "ui-call", - "line": 704, + "line": 708, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Save", "surface": "apple", @@ -38219,7 +38219,7 @@ }, { "kind": "conditional-branch", - "line": 732, + "line": 736, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Set API Key", "surface": "apple", @@ -38227,7 +38227,7 @@ }, { "kind": "conditional-branch", - "line": 732, + "line": 736, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Set Environment Variable", "surface": "apple", @@ -38235,7 +38235,7 @@ }, { "kind": "conditional-branch", - "line": 807, + "line": 869, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Skill disabled", "surface": "apple", @@ -38243,7 +38243,7 @@ }, { "kind": "conditional-branch", - "line": 807, + "line": 869, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Skill enabled", "surface": "apple", @@ -38251,7 +38251,7 @@ }, { "kind": "ui-named-argument", - "line": 885, + "line": 947, "path": "apps/macos/Sources/OpenClaw/SkillsSettings.swift", "source": "Bundled", "surface": "apple", diff --git a/apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewayProtocol.kt b/apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewayProtocol.kt index b330669bc337..6128cbc6420e 100644 --- a/apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewayProtocol.kt +++ b/apps/android/app/src/main/java/ai/openclaw/app/gateway/GatewayProtocol.kt @@ -535,6 +535,7 @@ enum class GatewayEvent( NodeInvokeRequest("node.invoke.request"), DevicePairRequested("device.pair.requested"), DevicePairResolved("device.pair.resolved"), + SkillsChanged("skills.changed"), VoicewakeChanged("voicewake.changed"), VoicewakeRoutingChanged("voicewake.routing.changed"), ExecApprovalRequested("exec.approval.requested"), diff --git a/apps/macos/Sources/OpenClaw/SkillsSettings.swift b/apps/macos/Sources/OpenClaw/SkillsSettings.swift index 840668f7d178..e274ea0f90f9 100644 --- a/apps/macos/Sources/OpenClaw/SkillsSettings.swift +++ b/apps/macos/Sources/OpenClaw/SkillsSettings.swift @@ -10,7 +10,6 @@ struct SkillsSettings: View { @State private var section: SkillsSection = .installed @State private var searchText = "" @State private var filter: SkillsFilter = .all - @State private var didScheduleInitialRefresh = false init(state: AppState = AppStateStore.shared, model: SkillsSettingsModel = SkillsSettingsModel()) { self.state = state @@ -40,10 +39,15 @@ struct SkillsSettings: View { .settingsDetailContent() } .task { - guard !self.didScheduleInitialRefresh else { return } - self.didScheduleInitialRefresh = true - await Task.yield() - await self.model.refreshIfNeeded() + // Subscribe before the first request so a remote-node invalidation cannot + // overtake the response and leave stale eligibility visible. + let pushes = await GatewayConnection.shared.subscribe() + let initialRefresh = Task { await self.model.refreshIfNeeded() } + defer { initialRefresh.cancel() } + for await push in pushes { + if Task.isCancelled { return } + self.model.handleGatewayPush(push) + } } .sheet(item: self.$envEditor) { editor in EnvEditorView(editor: editor) { value in @@ -745,7 +749,23 @@ final class SkillsSettingsModel { var error: String? var statusMessage: String? private var hasLoaded = false + private var hasAttemptedLoad = false private var busySkills: Set = [] + private var pendingForcedRefresh = false + private var gatewayRefreshTask: Task? + private let loadSkillsStatus: () async throws -> SkillsStatusReport + + init() { + self.loadSkillsStatus = Self.defaultLoadSkillsStatus + } + + init(loadSkillsStatus: @escaping () async throws -> SkillsStatusReport) { + self.loadSkillsStatus = loadSkillsStatus + } + + private static func defaultLoadSkillsStatus() async throws -> SkillsStatusReport { + try await GatewayConnection.shared.skillsStatus() + } func isBusy(skill: SkillStatus) -> Bool { self.busySkills.contains(skill.skillKey) @@ -756,21 +776,63 @@ final class SkillsSettingsModel { await self.refresh() } + func handleGatewayPush(_ push: GatewayPush) { + switch push { + case let .event(event) where event.event == "skills.changed": + break + case .seqGap: + break + default: + return + } + // The view subscribes before launching its initial request. If that request + // has not started yet, it will observe this invalidation without a duplicate. + guard self.hasAttemptedLoad || self.isLoading else { return } + self.pendingForcedRefresh = true + self.startPendingGatewayRefreshIfNeeded() + } + + private func startPendingGatewayRefreshIfNeeded() { + guard self.pendingForcedRefresh, !self.isLoading, self.gatewayRefreshTask == nil else { return } + self.gatewayRefreshTask = Task { [weak self] in + guard let self else { return } + await self.refresh(force: true) + self.gatewayRefreshTask = nil + // An invalidation can arrive after the drain ends but before this task + // releases its lease; start a successor instead of losing that signal. + self.startPendingGatewayRefreshIfNeeded() + } + } + func refresh(force: Bool = false) async { - guard !self.isLoading else { return } + if self.isLoading { + if force { + self.pendingForcedRefresh = true + } + return + } if self.hasLoaded, !force { return } + self.hasAttemptedLoad = true self.isLoading = true + defer { self.isLoading = false } + + repeat { + self.pendingForcedRefresh = false + await self.runRefresh() + } while self.pendingForcedRefresh + } + + private func runRefresh() async { self.error = nil do { - let report = try await GatewayConnection.shared.skillsStatus() + let report = try await self.loadSkillsStatus() self.skills = report.skills.sorted { $0.name < $1.name } self.hasLoaded = true } catch { self.error = error.localizedDescription } - self.isLoading = false } func acceptInstalledSkills(_ skills: [SkillStatus]) { diff --git a/apps/macos/Tests/OpenClawIPCTests/GatewayProcessManagerTests.swift b/apps/macos/Tests/OpenClawIPCTests/GatewayProcessManagerTests.swift index 0f9142138dcb..301c39a98f7d 100644 --- a/apps/macos/Tests/OpenClawIPCTests/GatewayProcessManagerTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/GatewayProcessManagerTests.swift @@ -1,3 +1,4 @@ +import Darwin import Foundation import Testing @testable import OpenClaw @@ -6,21 +7,59 @@ import Testing @Suite(.serialized) @MainActor struct GatewayProcessManagerTests { + private func availableGatewayPort() throws -> Int { + let fd = socket(AF_INET, SOCK_STREAM, 0) + guard fd >= 0 else { + throw NSError(domain: NSPOSIXErrorDomain, code: Int(errno)) + } + defer { _ = Darwin.close(fd) } + + var address = sockaddr_in() + address.sin_len = UInt8(MemoryLayout.size) + address.sin_family = sa_family_t(AF_INET) + address.sin_port = 0 + address.sin_addr = in_addr(s_addr: inet_addr("127.0.0.1")) + let bound = withUnsafePointer(to: &address) { pointer in + pointer.withMemoryRebound(to: sockaddr.self, capacity: 1) { socketAddress in + Darwin.bind(fd, socketAddress, socklen_t(MemoryLayout.size)) + } + } + guard bound == 0 else { + throw NSError(domain: NSPOSIXErrorDomain, code: Int(errno)) + } + + var assigned = sockaddr_in() + var assignedLength = socklen_t(MemoryLayout.size) + let resolved = withUnsafeMutablePointer(to: &assigned) { pointer in + pointer.withMemoryRebound(to: sockaddr.self, capacity: 1) { socketAddress in + getsockname(fd, socketAddress, &assignedLength) + } + } + guard resolved == 0 else { + throw NSError(domain: NSPOSIXErrorDomain, code: Int(errno)) + } + return Int(UInt16(bigEndian: assigned.sin_port)) + } + private func withGatewayConfig( mode: String, + port: Int? = nil, _ body: () async throws -> T) async throws -> T { let configPath = TestIsolation.tempConfigPath() - try Data(#"{"gateway":{"mode":"\#(mode)"}}"#.utf8) + let portFragment = port.map { ",\"port\":\($0)" } ?? "" + let config = #"{"gateway":{"mode":"\#(mode)""# + portFragment + "}}" + try Data(config.utf8) .write(to: URL(fileURLWithPath: configPath)) defer { try? FileManager.default.removeItem(atPath: configPath) } return try await TestIsolation.withEnvValues(["OPENCLAW_CONFIG_PATH": configPath], body) } private func withLocalGatewayConfig( + port: Int? = nil, _ body: () async throws -> T) async throws -> T { - try await self.withGatewayConfig(mode: "local", body) + try await self.withGatewayConfig(mode: "local", port: port, body) } @Test func `coalesces concurrent launch agent enable requests`() async throws { @@ -1002,6 +1041,7 @@ struct GatewayProcessManagerTests { } @Test func `startup install forces the recovered control channel refresh`() async throws { + let port = try self.availableGatewayPort() let marker = FileManager.default.temporaryDirectory .appendingPathComponent("openclaw-launchagent-marker-\(UUID().uuidString)") let session = GatewayTestWebSocketSession( @@ -1018,8 +1058,8 @@ struct GatewayProcessManagerTests { configProvider: { (url: url, token: nil, password: nil) }, sessionBox: WebSocketSessionBox(session: session)) - try await self.withLocalGatewayConfig { - let port = GatewayEnvironment.gatewayPort() + try await self.withLocalGatewayConfig(port: port) { + #expect(GatewayEnvironment.gatewayPort() == port) GatewayLaunchAgentManager.setTestingDisableLaunchAgentMarkerURL(marker) GatewayLaunchAgentManager.setTestingInterceptDaemonCommands(true) GatewayLaunchAgentManager.setTestingDaemonStatusPayload( diff --git a/apps/macos/Tests/OpenClawIPCTests/SkillsSettingsSmokeTests.swift b/apps/macos/Tests/OpenClawIPCTests/SkillsSettingsSmokeTests.swift index bcc20ce0f2b1..5e1663f01b75 100644 --- a/apps/macos/Tests/OpenClawIPCTests/SkillsSettingsSmokeTests.swift +++ b/apps/macos/Tests/OpenClawIPCTests/SkillsSettingsSmokeTests.swift @@ -1,3 +1,4 @@ +import OpenClawKit import OpenClawProtocol import Testing @testable import OpenClaw @@ -38,6 +39,21 @@ private func makeSkillStatus( install: install) } +private func makeSkillsStatusReport(_ skills: [SkillStatus]) -> SkillsStatusReport { + SkillsStatusReport( + workspaceDir: "/tmp/workspace", + managedSkillsDir: "/tmp/skills", + skills: skills) +} + +private func makeSkillsChangedPush() -> GatewayPush { + .event(EventFrame(type: "event", event: "skills.changed")) +} + +private enum SkillsStatusLoadError: Error { + case unavailable +} + @Suite(.serialized) @MainActor struct SkillsSettingsSmokeTests { @@ -146,6 +162,198 @@ struct SkillsSettingsSmokeTests { _ = view.body } + @Test func `skills changed event forces a loaded status refresh`() async { + var reports = [ + makeSkillsStatusReport([ + makeSkillStatus( + name: "Paprika", + description: "Missing local binary", + source: "openclaw-workspace", + filePath: "/tmp/skills/paprika", + skillKey: "paprika", + emoji: "P", + eligible: false, + requirements: SkillRequirements(bins: ["paprika"], env: [], config: []), + missing: SkillMissing(bins: ["paprika"], env: [], config: [])), + ]), + makeSkillsStatusReport([ + makeSkillStatus( + name: "Paprika", + description: "Ready after node reconnect", + source: "openclaw-workspace", + filePath: "/tmp/skills/paprika", + skillKey: "paprika", + emoji: "P", + eligible: true, + requirements: SkillRequirements(bins: ["paprika"], env: [], config: []), + missing: SkillMissing(bins: [], env: [], config: [])), + ]), + ] + var loadCount = 0 + let model = SkillsSettingsModel { + loadCount += 1 + return reports.removeFirst() + } + + await model.refreshIfNeeded() + #expect(loadCount == 1) + #expect(model.skills.first?.eligible == false) + + model.handleGatewayPush(makeSkillsChangedPush()) + while loadCount < 2 || model.isLoading { + await Task.yield() + } + + #expect(loadCount == 2) + #expect(model.skills.first?.eligible == true) + #expect(model.skills.first?.missing.bins == []) + } + + @Test func `skills changed event queues refresh under one loading lease`() async { + var pendingLoads: [CheckedContinuation] = [] + let model = SkillsSettingsModel { + await withCheckedContinuation { continuation in + pendingLoads.append(continuation) + } + } + + let initialRefresh = Task { await model.refreshIfNeeded() } + while pendingLoads.count < 1 { + await Task.yield() + } + + model.handleGatewayPush(makeSkillsChangedPush()) + + #expect(pendingLoads.count == 1) + #expect(model.isLoading) + + pendingLoads.removeFirst().resume(returning: makeSkillsStatusReport([ + makeSkillStatus( + name: "Paprika", + description: "Missing local binary", + source: "openclaw-workspace", + filePath: "/tmp/skills/paprika", + skillKey: "paprika", + emoji: "P", + eligible: false, + requirements: SkillRequirements(bins: ["paprika"], env: [], config: []), + missing: SkillMissing(bins: ["paprika"], env: [], config: [])), + ])) + while pendingLoads.count < 1 { + await Task.yield() + } + + #expect(model.isLoading) + pendingLoads.removeFirst().resume(returning: makeSkillsStatusReport([ + makeSkillStatus( + name: "Paprika", + description: "Ready after node reconnect", + source: "openclaw-workspace", + filePath: "/tmp/skills/paprika", + skillKey: "paprika", + emoji: "P", + eligible: true, + requirements: SkillRequirements(bins: ["paprika"], env: [], config: []), + missing: SkillMissing(bins: [], env: [], config: [])), + ])) + await initialRefresh.value + + #expect(!model.isLoading) + #expect(model.skills.first?.eligible == true) + #expect(model.skills.first?.missing.bins == []) + } + + @Test func `unrelated gateway event does not load skills`() { + var loadCount = 0 + let model = SkillsSettingsModel { + loadCount += 1 + return makeSkillsStatusReport([]) + } + + model.handleGatewayPush(.event(EventFrame(type: "event", event: "health"))) + + #expect(loadCount == 0) + #expect(model.skills.isEmpty) + } + + @Test func `skills changed before initial load does not duplicate it`() async { + var loadCount = 0 + let model = SkillsSettingsModel { + loadCount += 1 + return makeSkillsStatusReport([]) + } + + model.handleGatewayPush(makeSkillsChangedPush()) + #expect(loadCount == 0) + + await model.refreshIfNeeded() + #expect(loadCount == 1) + } + + @Test func `skills changed retries a failed initial load`() async { + var loadCount = 0 + let model = SkillsSettingsModel { + loadCount += 1 + if loadCount == 1 { + throw SkillsStatusLoadError.unavailable + } + return makeSkillsStatusReport([]) + } + + await model.refreshIfNeeded() + #expect(model.error != nil) + + model.handleGatewayPush(makeSkillsChangedPush()) + while loadCount < 2 || model.isLoading { + await Task.yield() + } + + #expect(loadCount == 2) + #expect(model.error == nil) + } + + @Test func `queued success clears the failed refresh error`() async { + var firstLoad: CheckedContinuation? + var loadCount = 0 + let model = SkillsSettingsModel { + loadCount += 1 + if loadCount == 1 { + return try await withCheckedThrowingContinuation { continuation in + firstLoad = continuation + } + } + return makeSkillsStatusReport([]) + } + + let initialRefresh = Task { await model.refreshIfNeeded() } + while firstLoad == nil { + await Task.yield() + } + model.handleGatewayPush(makeSkillsChangedPush()) + firstLoad?.resume(throwing: SkillsStatusLoadError.unavailable) + await initialRefresh.value + + #expect(loadCount == 2) + #expect(!model.isLoading) + #expect(model.error == nil) + } + + @Test func `gateway sequence gap refreshes loaded skills`() async { + var loadCount = 0 + let model = SkillsSettingsModel { + loadCount += 1 + return makeSkillsStatusReport([]) + } + await model.refreshIfNeeded() + + model.handleGatewayPush(.seqGap(expected: 2, received: 3)) + while loadCount < 2 || model.isLoading { + await Task.yield() + } + + #expect(loadCount == 2) + } + @Test func `skills settings exercises private views`() { SkillsSettings.exerciseForTesting() } diff --git a/docs/gateway/protocol.md b/docs/gateway/protocol.md index 42a328385d18..f8e69d859176 100644 --- a/docs/gateway/protocol.md +++ b/docs/gateway/protocol.md @@ -671,6 +671,11 @@ methods. Treat this as feature discovery, not a full enumeration of - `config.changed`: a config write persisted (payload carries the config path, the new snapshot hash, and a timestamp — never config content). Operator-read scoped; clients refresh via `config.get`. +- `skills.changed`: connectivity, the skill catalog, config, or eligibility + changed after the gateway invalidated its skills snapshot. The payload's + `reason` is `watch`, `watch-targets`, `manual`, `remote-node`, + `config-change`, or `workshop`. Operator-read scoped; clients refresh via + `skills.status`. - `exec.approval.requested` / `exec.approval.resolved`: exec approval lifecycle. - `plugin.approval.requested` / `plugin.approval.resolved`: plugin approval diff --git a/docs/platforms/mac/skills.md b/docs/platforms/mac/skills.md index dbd428d0ca08..aae9c551bef0 100644 --- a/docs/platforms/mac/skills.md +++ b/docs/platforms/mac/skills.md @@ -30,6 +30,10 @@ The macOS app surfaces OpenClaw skills via the gateway; it does not parse skills ## Remote mode - Install and config updates happen on the gateway host, not the local Mac. +- When skill files, config, Mac-node connectivity, its catalog, or its executable + inventory changes, the gateway emits `skills.changed` after invalidating its + authoritative snapshot. An open Skills pane then refetches `skills.status`, + including changes that finish while an earlier request is still in flight. ## Related diff --git a/scripts/protocol-event-coverage.allowlist.json b/scripts/protocol-event-coverage.allowlist.json index 0a6ac9914feb..77fd2898efb8 100644 --- a/scripts/protocol-event-coverage.allowlist.json +++ b/scripts/protocol-event-coverage.allowlist.json @@ -15,6 +15,7 @@ "node.presence": "Node activity is reported by macOS nodes and consumed by gateway routing; iOS has no presence reporter yet.", "device.pair.requested": "Device pairing flows poll via device.pair.* methods on iOS.", "device.pair.resolved": "Device pairing flows poll via device.pair.* methods on iOS.", + "skills.changed": "Skills settings is a macOS operator surface; iOS does not expose skill management yet.", "voicewake.routing.changed": "iOS only consumes voicewake.changed trigger updates; routing changes are not surfaced.", "plugin.approval.requested": "Plugin approval prompts are not implemented on iOS.", "plugin.approval.resolved": "Plugin approval prompts are not implemented on iOS.", @@ -44,6 +45,7 @@ "node.presence": "Node activity is reported by macOS nodes and consumed by gateway routing; Android has no presence reporter yet.", "device.pair.requested": "Device pairing flows poll via device.pair.* methods on Android.", "device.pair.resolved": "Device pairing flows poll via device.pair.* methods on Android.", + "skills.changed": "Skills settings is a macOS operator surface; Android does not expose skill management yet.", "voicewake.routing.changed": "Android reads voicewake state on demand via voicewake.get; no push consumer yet.", "plugin.approval.requested": "Plugin approval prompts are not implemented on Android.", "plugin.approval.resolved": "Plugin approval prompts are not implemented on Android.", diff --git a/src/gateway/server-broadcast.board.test.ts b/src/gateway/server-broadcast.board.test.ts index 8338b2d930e3..31c29b9b72bb 100644 --- a/src/gateway/server-broadcast.board.test.ts +++ b/src/gateway/server-broadcast.board.test.ts @@ -36,6 +36,26 @@ function makeClient( }; } +describe("skills event scope guards", () => { + it("delivers skill invalidations only to read-capable operators", () => { + const pairing = makeClient("pairing", "operator", ["operator.pairing"]); + const node = makeClient("node", "node", ["operator.read"]); + const read = makeClient("read", "operator", ["operator.read"]); + const write = makeClient("write", "operator", ["operator.write"]); + const admin = makeClient("admin", "operator", ["operator.admin"]); + const clients = new Set([pairing, node, read, write, admin].map((entry) => entry.client)); + const { broadcast } = createGatewayBroadcaster({ clients }); + + broadcast("skills.changed", { reason: "remote-node" }); + + expect(pairing.socket.events).toEqual([]); + expect(node.socket.events).toEqual([]); + expect(read.socket.events).toEqual(["skills.changed"]); + expect(write.socket.events).toEqual(["skills.changed"]); + expect(admin.socket.events).toEqual(["skills.changed"]); + }); +}); + describe("board event scope guards", () => { it("delivers board events only to read-capable operators", () => { const pairing = makeClient("pairing", "operator", ["operator.pairing"]); diff --git a/src/gateway/server-broadcast.ts b/src/gateway/server-broadcast.ts index 194a850bdec8..812eb23145fa 100644 --- a/src/gateway/server-broadcast.ts +++ b/src/gateway/server-broadcast.ts @@ -61,6 +61,7 @@ const EVENT_SCOPE_GUARDS: Record = { // Hash-only change notice after a persisted config write; content stays // behind the operator-scoped config.get. "config.changed": [READ_SCOPE], + "skills.changed": [READ_SCOPE], "voicewake.changed": [READ_SCOPE], "voicewake.routing.changed": [READ_SCOPE], "device.pair.requested": [PAIRING_SCOPE], diff --git a/src/gateway/server-methods-list.test.ts b/src/gateway/server-methods-list.test.ts index a87f37c8728a..29456b288630 100644 --- a/src/gateway/server-methods-list.test.ts +++ b/src/gateway/server-methods-list.test.ts @@ -22,6 +22,10 @@ describe("GATEWAY_EVENTS", () => { expect(GATEWAY_EVENTS).toContain("node.presence"); }); + it("advertises skill invalidation updates", () => { + expect(GATEWAY_EVENTS).toContain("skills.changed"); + }); + it("advertises session observer digests", () => { expect(GATEWAY_EVENTS).toContain("session.observer"); }); diff --git a/src/gateway/server-methods-list.ts b/src/gateway/server-methods-list.ts index 86deb702f857..2b9562a394ed 100644 --- a/src/gateway/server-methods-list.ts +++ b/src/gateway/server-methods-list.ts @@ -68,6 +68,7 @@ export const GATEWAY_EVENTS = [ "node.invoke.request", "device.pair.requested", "device.pair.resolved", + "skills.changed", "voicewake.changed", "voicewake.routing.changed", "exec.approval.requested", diff --git a/src/gateway/server-startup-early.test.ts b/src/gateway/server-startup-early.test.ts index 6720514ec865..528f7012d2e9 100644 --- a/src/gateway/server-startup-early.test.ts +++ b/src/gateway/server-startup-early.test.ts @@ -146,6 +146,67 @@ describe("startGatewayEarlyRuntime", () => { expect(mocks.skillsChangeUnsub).toHaveBeenCalledTimes(1); }); + it("broadcasts remote-node skill invalidations to operator clients", async () => { + const broadcast = vi.fn(); + + await startGatewayEarlyRuntime( + earlyRuntimeInput({ + minimalTestGateway: false, + broadcast, + }), + ); + + const listener = mocks.registerSkillsChangeListener.mock.calls.at(-1)?.[0] as + | ((event: { reason: "remote-node" }) => void) + | undefined; + expect(listener).toBeDefined(); + + listener?.({ reason: "remote-node" }); + + expect(broadcast).toHaveBeenCalledWith("skills.changed", { reason: "remote-node" }); + expect(mocks.refreshRemoteBinsForConnectedNodes).not.toHaveBeenCalled(); + }); + + it("broadcasts local skill changes after the coalesced remote-bin refresh", async () => { + vi.useFakeTimers(); + const broadcast = vi.fn(); + let refreshTimer: ReturnType | null = null; + let finishRefresh: (() => void) | undefined; + mocks.refreshRemoteBinsForConnectedNodes.mockImplementationOnce( + () => + new Promise((resolve) => { + finishRefresh = resolve; + }), + ); + try { + await startGatewayEarlyRuntime( + earlyRuntimeInput({ + minimalTestGateway: false, + broadcast, + getSkillsRefreshTimer: () => refreshTimer, + setSkillsRefreshTimer: (timer) => { + refreshTimer = timer; + }, + }), + ); + + const listener = mocks.registerSkillsChangeListener.mock.calls.at(-1)?.[0] as + | ((event: { reason: "watch" }) => void) + | undefined; + listener?.({ reason: "watch" }); + await vi.advanceTimersByTimeAsync(30_000); + + expect(mocks.refreshRemoteBinsForConnectedNodes).toHaveBeenCalledWith({}); + expect(broadcast).not.toHaveBeenCalled(); + + finishRefresh?.(); + await Promise.resolve(); + expect(broadcast).toHaveBeenCalledWith("skills.changed", { reason: "watch" }); + } finally { + vi.useRealTimers(); + } + }); + it("fails before discovery and task maintenance when task state cannot restore", async () => { mocks.ensureTaskRuntimeStateReady.mockImplementationOnce(() => { throw new Error("task-flow registry restore failed"); diff --git a/src/gateway/server-startup-early.ts b/src/gateway/server-startup-early.ts index cf31f80c55fc..060ff522d39b 100644 --- a/src/gateway/server-startup-early.ts +++ b/src/gateway/server-startup-early.ts @@ -134,6 +134,9 @@ export async function startGatewayEarlyRuntime(params: { ]); return registerSkillsChangeListener((event) => { if (event.reason === "remote-node") { + // The snapshot invalidation runs after remote descriptors/bins change; + // clients can now refetch authoritative skills.status without racing the probe. + params.broadcast("skills.changed", { reason: event.reason }); return; } // Coalesce local skill changes before refreshing connected remote @@ -144,7 +147,17 @@ export async function startGatewayEarlyRuntime(params: { } const nextTimer = setTimeout(() => { params.setSkillsRefreshTimer(null); - void refreshRemoteBinsForConnectedNodes(params.getRuntimeConfig()); + void refreshRemoteBinsForConnectedNodes(params.getRuntimeConfig()).then( + () => { + params.broadcast("skills.changed", { reason: event.reason }); + }, + (error: unknown) => { + params.log.warn( + `failed to refresh remote bins after skills change: ${String(error)}`, + ); + params.broadcast("skills.changed", { reason: event.reason }); + }, + ); }, params.skillsRefreshDelayMs); params.setSkillsRefreshTimer(nextTimer); }); diff --git a/src/skills/runtime/remote-probe-utils.ts b/src/skills/runtime/remote-probe-utils.ts index a2ac67113125..bc19606c0751 100644 --- a/src/skills/runtime/remote-probe-utils.ts +++ b/src/skills/runtime/remote-probe-utils.ts @@ -43,6 +43,23 @@ export function supportsSystemRun(commands?: string[]): boolean { return Array.isArray(commands) && commands.includes("system.run"); } +export function isRemoteSkillEligibilityNode( + node: + | { + connected?: boolean; + platform?: string; + deviceFamily?: string; + commands?: string[]; + } + | undefined, +): boolean { + return Boolean( + node?.connected && + isMacPlatform(node.platform, node.deviceFamily) && + supportsSystemRun(node.commands), + ); +} + export function supportsSystemWhich(commands?: string[]): boolean { return Array.isArray(commands) && commands.includes("system.which"); } diff --git a/src/skills/runtime/remote.test.ts b/src/skills/runtime/remote.test.ts index 7e1b67313a5d..58f68943e680 100644 --- a/src/skills/runtime/remote.test.ts +++ b/src/skills/runtime/remote.test.ts @@ -211,6 +211,24 @@ describe("skills-remote", () => { expect(after).toBeGreaterThan(before); }); + it("bumps the skills snapshot version when an eligible remote node connects", async () => { + await resetSkillsRefreshForTest(); + const workspaceDir = `/tmp/ws-${randomUUID()}`; + const nodeId = `node-${randomUUID()}`; + + const before = getSkillsSnapshotVersion(workspaceDir); + recordRemoteNodeInfo({ + nodeId, + displayName: "Remote Mac", + platform: "darwin", + commands: ["system.run"], + }); + const after = getSkillsSnapshotVersion(workspaceDir); + + expect(after).toBeGreaterThan(before); + removeRemoteNodeInfo(nodeId); + }); + it("ignores non-mac and non-system.run nodes for eligibility", () => { const linuxNodeId = `node-${randomUUID()}`; const noRunNodeId = `node-${randomUUID()}`; diff --git a/src/skills/runtime/remote.ts b/src/skills/runtime/remote.ts index 934c2c314727..dd102d907d24 100644 --- a/src/skills/runtime/remote.ts +++ b/src/skills/runtime/remote.ts @@ -14,6 +14,7 @@ import { collectRequiredBins, extractErrorMessage, isMacPlatform, + isRemoteSkillEligibilityNode, parseBinProbePayload, supportsSystemRun, supportsSystemWhich, @@ -352,6 +353,7 @@ export function recordRemoteNodeInfo(node: { pairingGeneration?: string; }) { const existing = remoteNodes.get(node.nodeId); + const wasEligible = isRemoteSkillEligibilityNode(existing); const pairingGenerationChanged = Boolean( existing && existing.pairingGeneration !== node.pairingGeneration, ); @@ -364,6 +366,11 @@ export function recordRemoteNodeInfo(node: { remoteNodeProbeStates.delete(node.nodeId); } upsertNode({ ...node, connected: true }, { pairingGenerationAuthoritative: true }); + if (wasEligible !== isRemoteSkillEligibilityNode(remoteNodes.get(node.nodeId))) { + // Connectivity and command-surface changes affect OS-only skills even before + // the delayed bin probe; the probe emits a second invalidation if bins change. + bumpSkillsSnapshotVersion({ reason: "remote-node" }); + } recordRemoteSkillNodeInfo({ nodeId: node.nodeId, connId: node.connId,