From 98b08e401b70be1c20d88a26482794648303d809 Mon Sep 17 00:00:00 2001 From: xcv58 Date: Sun, 23 Aug 2026 09:53:19 -0400 Subject: [PATCH] fix: harden window layout workflows --- Configs/AppVersion.xcconfig | 2 +- .../WindowCustomCommandSettingsView.swift | 100 ++++---- .../Sources/WindowLayoutCalculator.swift | 39 ++- .../Sources/WindowLayoutService.swift | 25 +- .../Sources/WindowLayoutsPlugin.swift | 119 +++++---- .../Sources/WindowLayoutsStore.swift | 6 + .../WindowShortcutPresetSettingsView.swift | 27 ++- .../Tests/WindowLayoutCalculatorTests.swift | 33 +++ .../Tests/WindowLayoutServiceTests.swift | 150 +++++++++++- .../Tests/WindowLayoutsPluginTests.swift | 175 ++++++++++++-- .../Tests/WindowLayoutsStoreTests.swift | 16 ++ Plugins/WindowLayouts/plugin.json | 2 +- Sources/App/MacToolsAppRuntime.swift | 9 +- Sources/App/RunLinkExecutionCoordinator.swift | 3 +- Sources/Core/Plugins/PluginHost.swift | 112 ++++++++- .../ActionShortcutAssignmentStore.swift | 24 +- .../Shortcuts/ShortcutAssignmentService.swift | 91 ++++++- Sources/MacToolsPluginKit/ActionModels.swift | 14 ++ .../RunLinkExecutionCoordinatorTests.swift | 10 + .../PluginHostActionRegistryTests.swift | 227 +++++++++++++++++- .../ShortcutAssignmentServiceTests.swift | 175 +++++++++++++- .../window-layouts-execution-safety.md | 6 + .../window-layouts-host-feedback.md | 2 + scripts/fixtures/plugin-kit-v5/Client.swift | 35 +++ .../plugin-kit-v5/MacToolsPluginKit.swift | 40 +++ .../test_plugin_minimum_host_compatibility.py | 30 ++- 26 files changed, 1272 insertions(+), 200 deletions(-) diff --git a/Configs/AppVersion.xcconfig b/Configs/AppVersion.xcconfig index 88979707..fe40db57 100644 --- a/Configs/AppVersion.xcconfig +++ b/Configs/AppVersion.xcconfig @@ -1,2 +1,2 @@ -MARKETING_VERSION = 1.2.0 +MARKETING_VERSION = 1.2.1 CURRENT_PROJECT_VERSION = 69 diff --git a/Plugins/WindowLayouts/Sources/WindowCustomCommandSettingsView.swift b/Plugins/WindowLayouts/Sources/WindowCustomCommandSettingsView.swift index 78ca9d55..dc5f10da 100644 --- a/Plugins/WindowLayouts/Sources/WindowCustomCommandSettingsView.swift +++ b/Plugins/WindowLayouts/Sources/WindowCustomCommandSettingsView.swift @@ -4,56 +4,33 @@ import MacToolsPluginKit struct WindowCustomCommandPreviewLayout { private static let referenceScreenSize = CGSize(width: 1_440, height: 900) + private static let referenceWindowFrame = CGRect( + x: 288, + y: 180, + width: 864, + height: 540 + ) let command: WindowCustomCommand + let gap: CGFloat func windowFrame(in screenSize: CGSize) -> CGRect { guard screenSize.width > 0, screenSize.height > 0 else { return .zero } - let windowSize = CGSize( - width: screenSize.width * fraction( - for: command.width, - referenceLength: Self.referenceScreenSize.width - ), - height: screenSize.height * fraction( - for: command.height, - referenceLength: Self.referenceScreenSize.height - ) + let referenceFrame = WindowLayoutCalculator().customFrame( + for: command, + windowFrame: Self.referenceWindowFrame, + visibleFrame: CGRect(origin: .zero, size: Self.referenceScreenSize), + gap: gap ) - let factors = anchorFactors(command.anchor) - let origin = CGPoint( - x: (screenSize.width - windowSize.width) * factors.x - + screenSize.width * command.offsetX / Self.referenceScreenSize.width, - y: (screenSize.height - windowSize.height) * factors.y - + screenSize.height * command.offsetY / Self.referenceScreenSize.height + let scaleX = screenSize.width / Self.referenceScreenSize.width + let scaleY = screenSize.height / Self.referenceScreenSize.height + return CGRect( + x: referenceFrame.minX * scaleX, + y: referenceFrame.minY * scaleY, + width: referenceFrame.width * scaleX, + height: referenceFrame.height * scaleY ) - return CGRect(origin: origin, size: windowSize) - } - - private func fraction( - for dimension: WindowLayoutDimension, - referenceLength: CGFloat - ) -> CGFloat { - let value: CGFloat = switch dimension { - case .current: 0.6 - case let .points(points): points / referenceLength - case let .fraction(fraction): fraction - } - return min(max(value, 0.05), 1) - } - - private func anchorFactors(_ anchor: WindowLayoutAnchor) -> CGPoint { - switch anchor { - case .topLeft: CGPoint(x: 0, y: 0) - case .top: CGPoint(x: 0.5, y: 0) - case .topRight: CGPoint(x: 1, y: 0) - case .left: CGPoint(x: 0, y: 0.5) - case .center: CGPoint(x: 0.5, y: 0.5) - case .right: CGPoint(x: 1, y: 0.5) - case .bottomLeft: CGPoint(x: 0, y: 1) - case .bottom: CGPoint(x: 0.5, y: 1) - case .bottomRight: CGPoint(x: 1, y: 1) - } } } @@ -111,6 +88,7 @@ struct WindowCustomCommandSettingsView: View { let commandID: UUID @State private var draft: WindowCustomCommand + @State private var shortcutErrorMessage: String? @FocusState private var isNameFocused: Bool init(plugin: WindowLayoutsPlugin, command: WindowCustomCommand) { @@ -161,7 +139,10 @@ struct WindowCustomCommandSettingsView: View { ) .font(PluginSettingsTheme.Typography.emphasizedRowTitle) - WindowCustomLayoutPreview(command: draft) + WindowCustomLayoutPreview( + command: draft, + gap: plugin.customCommandPreviewGap + ) .frame(width: 184, height: 112) .accessibilityLabel(previewSummary) } @@ -189,7 +170,9 @@ struct WindowCustomCommandSettingsView: View { displayText: shortcutDisplayText, minWidth: PluginSettingsTheme.Size.shortcutRecorderWidth, onRecord: { binding in - plugin.recordCustomCommandShortcut(binding, for: commandID) + let result = plugin.recordCustomCommandShortcut(binding, for: commandID) + updateShortcutError(from: result) + return result } ) .frame(width: PluginSettingsTheme.Size.shortcutRecorderWidth) @@ -199,7 +182,9 @@ struct WindowCustomCommandSettingsView: View { if shortcutBinding != nil { Button { - plugin.clearCustomCommandShortcut(for: commandID) + updateShortcutError( + from: plugin.clearCustomCommandShortcut(for: commandID) + ) } label: { Image(systemName: "xmark.circle.fill") .pluginSettingsRowIconStyle(.secondary) @@ -215,12 +200,31 @@ struct WindowCustomCommandSettingsView: View { )) } } + + if let shortcutErrorMessage { + Label( + shortcutErrorMessage, + systemImage: "exclamationmark.triangle.fill" + ) + .font(PluginSettingsTheme.Typography.rowDescription) + .foregroundStyle(.red) + .fixedSize(horizontal: false, vertical: true) + } } .frame(maxWidth: .infinity, alignment: .leading) } .pluginSettingsListRowPadding(interactive: true) } + private func updateShortcutError(from result: PluginShortcutRecordingResult) { + switch result { + case .accepted: + shortcutErrorMessage = nil + case let .rejected(message): + shortcutErrorMessage = message + } + } + private var nameRow: some View { settingsRow( title: plugin.localizedKey("settings.custom.name", "名称") @@ -518,9 +522,6 @@ struct WindowCustomCommandSettingsView: View { } private func commitDraft() { - guard !draft.name.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { - return - } guard plugin.updateCustomCommand(draft), let stored = plugin.customCommand(id: commandID) else { @@ -543,10 +544,11 @@ struct WindowCustomCommandSettingsView: View { private struct WindowCustomLayoutPreview: View { let command: WindowCustomCommand + let gap: CGFloat var body: some View { GeometryReader { proxy in - let frame = WindowCustomCommandPreviewLayout(command: command) + let frame = WindowCustomCommandPreviewLayout(command: command, gap: gap) .windowFrame(in: proxy.size) ZStack(alignment: .topLeading) { diff --git a/Plugins/WindowLayouts/Sources/WindowLayoutCalculator.swift b/Plugins/WindowLayouts/Sources/WindowLayoutCalculator.swift index 4c44cee7..af253e01 100644 --- a/Plugins/WindowLayouts/Sources/WindowLayoutCalculator.swift +++ b/Plugins/WindowLayouts/Sources/WindowLayoutCalculator.swift @@ -147,7 +147,8 @@ struct WindowLayoutCalculator { func movedFrame( _ windowFrame: CGRect, from sourceVisibleFrame: CGRect, - to destinationVisibleFrame: CGRect + to destinationVisibleFrame: CGRect, + preservingSize: Bool = false ) -> CGRect { let source = sourceVisibleFrame.standardized let destination = destinationVisibleFrame.standardized @@ -159,14 +160,16 @@ struct WindowLayoutCalculator { let widthRatio = max(0, windowFrame.width / source.width) let heightRatio = max(0, windowFrame.height / source.height) - let destinationSize = CGSize( - width: source.width == destination.width - ? min(destination.width, max(0, windowFrame.width)) - : min(destination.width, destination.width * widthRatio), - height: source.height == destination.height - ? min(destination.height, max(0, windowFrame.height)) - : min(destination.height, destination.height * heightRatio) - ) + let destinationSize = preservingSize + ? windowFrame.size + : CGSize( + width: source.width == destination.width + ? min(destination.width, max(0, windowFrame.width)) + : min(destination.width, destination.width * widthRatio), + height: source.height == destination.height + ? min(destination.height, max(0, windowFrame.height)) + : min(destination.height, destination.height * heightRatio) + ) let sourceTravelX = source.width - windowFrame.width let sourceTravelY = source.height - windowFrame.height let relativeX = relativePosition( @@ -185,7 +188,9 @@ struct WindowLayoutCalculator { width: destinationSize.width, height: destinationSize.height ) - return clamp(proposed, inside: destination) + return preservingSize + ? restoreReachableFrame(proposed, inside: destination) + : clamp(proposed, inside: destination) } func clamp(_ frame: CGRect, inside bounds: CGRect) -> CGRect { @@ -202,6 +207,20 @@ struct WindowLayoutCalculator { ) } + /// Keeps the original size while ensuring the window's top edge remains reachable. + func restoreReachableFrame(_ frame: CGRect, inside bounds: CGRect) -> CGRect { + let bounds = bounds.standardized + let minimumX = min(bounds.minX, bounds.maxX - frame.width) + let maximumX = max(bounds.minX, bounds.maxX - frame.width) + let maximumY = max(bounds.minY, bounds.maxY - frame.height) + return CGRect( + x: min(max(frame.minX, minimumX), maximumX), + y: min(max(frame.minY, bounds.minY), maximumY), + width: frame.width, + height: frame.height + ) + } + private func insetFrame(_ frame: CGRect, by requestedGap: CGFloat) -> CGRect { let maximumGap = max(0, min(frame.width, frame.height) / 2 - 0.5) let gap = min(max(0, requestedGap), maximumGap) diff --git a/Plugins/WindowLayouts/Sources/WindowLayoutService.swift b/Plugins/WindowLayouts/Sources/WindowLayoutService.swift index 6a465041..90006ba3 100644 --- a/Plugins/WindowLayouts/Sources/WindowLayoutService.swift +++ b/Plugins/WindowLayouts/Sources/WindowLayoutService.swift @@ -294,7 +294,8 @@ final class WindowLayoutService: WindowLayoutExecuting { targetFrame = calculator.movedFrame( currentFrame, from: effectiveCurrentScreen.visibleFrame, - to: screen(destination, respectingStageManager: options.respectsStageManager).visibleFrame + to: screen(destination, respectingStageManager: options.respectsStageManager).visibleFrame, + preservingSize: !window.canResize ) case .restorePreviousFrame: guard await frameReader.isValid(window) else { @@ -304,22 +305,20 @@ final class WindowLayoutService: WindowLayoutExecuting { guard let previousFrame = history.previousFrame(for: window) else { throw WindowLayoutError.noPreviousFrame } - let safePreviousFrame: CGRect - if screens.contains(where: { - $0.visibleFrame.intersection(previousFrame).area > 0 - }) { - safePreviousFrame = previousFrame - } else if let nearestScreen = screenResolver.screen( + guard let nearestScreen = screenResolver.screen( for: previousFrame, among: screens - ) { - safePreviousFrame = calculator.clamp( - previousFrame, - inside: nearestScreen.visibleFrame - ) - } else { + ) else { throw WindowLayoutError.noDisplay } + let safeVisibleFrame = screen( + nearestScreen, + respectingStageManager: options.respectsStageManager + ).visibleFrame + let safePreviousFrame = calculator.restoreReachableFrame( + previousFrame, + inside: safeVisibleFrame + ) if safePreviousFrame.size != currentFrame.size, !window.canResize { throw WindowLayoutError.windowCannotResize } diff --git a/Plugins/WindowLayouts/Sources/WindowLayoutsPlugin.swift b/Plugins/WindowLayouts/Sources/WindowLayoutsPlugin.swift index 04467715..c0cf89a6 100644 --- a/Plugins/WindowLayouts/Sources/WindowLayoutsPlugin.swift +++ b/Plugins/WindowLayouts/Sources/WindowLayoutsPlugin.swift @@ -24,8 +24,9 @@ private struct WindowLayoutsPluginProvider: PluginProvider { final class WindowLayoutsPlugin: MacToolsPlugin, AccessibilityPermissionRefreshing, PluginActionProviding, PluginActionPermissionProviding, PluginActionExposureProviding, PluginActionExecutionRevisionProviding, + PluginActionSafetyStateChangeProviding, PluginActionShortcutSettingsProviding, PluginRetiredActionShortcutProviding, - PluginActionShortcutPresetApplying, + PluginActionShortcutPresetApplying, PluginActionShortcutReplacementTransactionApplying, PluginActionShortcutAssignmentChangeHandling, ObservableObject, PluginFocusedWindowTargetConsuming { @@ -54,6 +55,7 @@ final class WindowLayoutsPlugin: MacToolsPlugin, AccessibilityPermissionRefreshi let metadata: PluginMetadata var onStateChange: (() -> Void)? + var onActionSafetyStateChange: (() -> Void)? var requestPermissionGuidance: ((String) -> Void)? var shortcutBindingResolver: ((String) -> ShortcutBinding?)? var previewActionShortcutPreset: (( @@ -61,6 +63,12 @@ final class WindowLayoutsPlugin: MacToolsPlugin, AccessibilityPermissionRefreshi [String: ShortcutBinding] ) -> PluginActionShortcutPresetPreview)? var applyActionShortcutPreset: ((Set, [String: ShortcutBinding]) -> String?)? + var currentActionShortcutBindings: ((Set) -> [String: [ShortcutBinding]])? + var performActionShortcutReplacementTransaction: (( + Set, + [String: ShortcutBinding], + () -> String? + ) -> String?)? @Published private(set) var actionShortcutAssignmentRevision: UInt64 = 0 @Published private(set) var customCommandSettingsRevision: UInt64 = 0 @Published private(set) var customCommandDeletionError: String? @@ -130,6 +138,9 @@ final class WindowLayoutsPlugin: MacToolsPlugin, AccessibilityPermissionRefreshi self?.customCommandSettingsRevision &+= 1 self?.onStateChange?() } + self.store.onSafetyPolicyMutation = { [weak self] in + self?.onActionSafetyStateChange?() + } } var permissionRequirements: [PluginPermissionRequirement] { @@ -533,9 +544,18 @@ final class WindowLayoutsPlugin: MacToolsPlugin, AccessibilityPermissionRefreshi @discardableResult func updateCustomCommand(_ command: WindowCustomCommand) -> Bool { - store.updateCustomCommand(command) + var normalized = command + if normalized.name.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { + normalized.name = localizedKey( + "settings.custom.defaultName", + "自定义布局" + ) + } + return store.updateCustomCommand(normalized) } + var customCommandPreviewGap: CGFloat { CGFloat(store.gap) } + func duplicateCustomCommand(_ id: UUID) { _ = store.duplicateCustomCommand( id: id, @@ -547,56 +567,28 @@ final class WindowLayoutsPlugin: MacToolsPlugin, AccessibilityPermissionRefreshi func deleteCustomCommand(_ id: UUID) -> Bool { guard let command = store.customCommand(id: id) else { return false } - var previousBinding: ShortcutBinding? - if applyActionShortcutPreset != nil || previewActionShortcutPreset != nil { - guard let previewActionShortcutPreset else { - publishCustomCommandDeletionError(localizedKey( - "settings.custom.delete.shortcutUnavailable", - "删除前无法读取此布局的快捷键。" - )) - return false - } - let preview = previewActionShortcutPreset([command.actionID], [:]) - guard preview.errorMessage == nil, - let previewItem = preview.items.first(where: { - $0.actionID == command.actionID - }) else { - publishCustomCommandDeletionError(localizedKey( - "settings.custom.delete.shortcutUnavailable", - "删除前无法读取此布局的快捷键。" - )) - return false - } - previousBinding = previewItem.currentBinding - guard let applyActionShortcutPreset else { - publishCustomCommandDeletionError(localizedKey( - "settings.custom.delete.shortcutUnavailable", - "删除前无法读取此布局的快捷键。" - )) - return false - } - if let error = applyActionShortcutPreset([command.actionID], [:]) { - publishCustomCommandDeletionError(error) - return false - } + guard let performActionShortcutReplacementTransaction else { + publishCustomCommandDeletionError(localizedKey( + "settings.custom.delete.shortcutUnavailable", + "删除前无法读取此布局的快捷键。" + )) + return false } - guard store.removeCustomCommand(id: id) else { - if let previousBinding, - let rollbackError = applyActionShortcutPreset?( - [command.actionID], - [command.actionID: previousBinding] - ) { - publishCustomCommandDeletionError(localizedKey( - "settings.custom.delete.rollbackFailed", - "无法删除布局，也无法恢复其快捷键：" - ) + " " + rollbackError) - } else { - publishCustomCommandDeletionError(localizedKey( + let error = performActionShortcutReplacementTransaction( + [command.actionID], + [:] + ) { + guard self.store.removeCustomCommand(id: id) else { + return self.localizedKey( "settings.custom.delete.failed", "无法删除自定义布局；其快捷键已恢复。" - )) + ) } + return nil + } + guard error == nil else { + publishCustomCommandDeletionError(error) return false } @@ -612,13 +604,27 @@ final class WindowLayoutsPlugin: MacToolsPlugin, AccessibilityPermissionRefreshi } func customCommandShortcutBinding(for id: UUID) -> ShortcutBinding? { - guard let command = store.customCommand(id: id), - let preview = previewActionShortcutPreset?([command.actionID], [:]) - else { return nil } + guard let command = store.customCommand(id: id) else { return nil } + if let binding = currentActionShortcutBindings?([command.actionID])[command.actionID]?.first { + return binding + } + guard let preview = previewActionShortcutPreset?([command.actionID], [:]) else { + return nil + } return preview.items.first(where: { $0.actionID == command.actionID })? .currentBinding } + func shortcutPresetCurrentBindings(for actionID: String) -> [ShortcutBinding] { + if let bindings = currentActionShortcutBindings?([actionID])[actionID] { + return bindings + } + guard let preview = previewActionShortcutPreset?([actionID], [:]), + let currentBinding = preview.items.first(where: { $0.actionID == actionID })? + .currentBinding else { return [] } + return [currentBinding] + } + func recordCustomCommandShortcut( _ binding: ShortcutBinding, for id: UUID @@ -637,9 +643,16 @@ final class WindowLayoutsPlugin: MacToolsPlugin, AccessibilityPermissionRefreshi )) } - func clearCustomCommandShortcut(for id: UUID) { - guard let command = store.customCommand(id: id) else { return } - _ = applyActionShortcutPreset?([command.actionID], [:]) + func clearCustomCommandShortcut(for id: UUID) -> PluginShortcutRecordingResult { + guard let command = store.customCommand(id: id), + let applyActionShortcutPreset + else { + return .rejected(localizedKey( + "settings.preset.unavailable", + "当前无法应用快捷键预设。" + )) + } + return .from(errorMessage: applyActionShortcutPreset([command.actionID], [:])) } private func updateCustomNumber(controlID: String, value: Double) { diff --git a/Plugins/WindowLayouts/Sources/WindowLayoutsStore.swift b/Plugins/WindowLayouts/Sources/WindowLayoutsStore.swift index 09ff2aa1..8fad89c2 100644 --- a/Plugins/WindowLayouts/Sources/WindowLayoutsStore.swift +++ b/Plugins/WindowLayouts/Sources/WindowLayoutsStore.swift @@ -36,6 +36,7 @@ final class WindowLayoutsStore { private(set) var customCommands: [WindowCustomCommand] = [] private(set) var revision: UInt64 = 0 var onMutation: (() -> Void)? + var onSafetyPolicyMutation: (() -> Void)? init(storage: PluginStorage) { self.storage = storage @@ -113,11 +114,16 @@ final class WindowLayoutsStore { guard let index = customCommands.firstIndex(where: { $0.id == command.id }) else { return false } guard let normalized = normalizedCommand(command) else { return false } + let externalInvocationPolicyChanged = + customCommands[index].allowExternalInvocation != normalized.allowExternalInvocation var updated = customCommands updated[index] = normalized guard persist(customCommands: updated) else { return false } customCommands = updated recordMutation() + if externalInvocationPolicyChanged { + onSafetyPolicyMutation?() + } return true } diff --git a/Plugins/WindowLayouts/Sources/WindowShortcutPresetSettingsView.swift b/Plugins/WindowLayouts/Sources/WindowShortcutPresetSettingsView.swift index 165ea19f..fb160a83 100644 --- a/Plugins/WindowLayouts/Sources/WindowShortcutPresetSettingsView.swift +++ b/Plugins/WindowLayouts/Sources/WindowShortcutPresetSettingsView.swift @@ -267,7 +267,7 @@ private struct WindowShortcutPresetEditorSheet: View { "settings.preset.currentBinding", "当前" ), - binding: item.currentBinding + bindings: plugin.shortcutPresetCurrentBindings(for: item.actionID) ) Image(systemName: "arrow.right") @@ -330,7 +330,7 @@ private struct WindowShortcutPresetEditorSheet: View { private func currentBindingBlock( label: String, - binding: ShortcutBinding? + bindings: [ShortcutBinding] ) -> some View { VStack( alignment: .leading, @@ -340,12 +340,23 @@ private struct WindowShortcutPresetEditorSheet: View { .font(PluginSettingsTheme.Typography.statusBadge) .foregroundStyle(.secondary) - PluginShortcutRecorderField( - displayText: plugin.shortcutBindingTitle(binding), - isRecording: false, - minWidth: 0 - ) - .frame(maxWidth: .infinity) + if bindings.isEmpty { + PluginShortcutRecorderField( + displayText: plugin.shortcutBindingTitle(nil), + isRecording: false, + minWidth: 0 + ) + .frame(maxWidth: .infinity) + } else { + ForEach(Array(bindings.enumerated()), id: \.offset) { _, binding in + PluginShortcutRecorderField( + displayText: plugin.shortcutBindingTitle(binding), + isRecording: false, + minWidth: 0 + ) + .frame(maxWidth: .infinity) + } + } } .frame(maxWidth: .infinity, alignment: .leading) } diff --git a/Plugins/WindowLayouts/Tests/WindowLayoutCalculatorTests.swift b/Plugins/WindowLayouts/Tests/WindowLayoutCalculatorTests.swift index adc03126..4c1b9968 100644 --- a/Plugins/WindowLayouts/Tests/WindowLayoutCalculatorTests.swift +++ b/Plugins/WindowLayouts/Tests/WindowLayoutCalculatorTests.swift @@ -167,6 +167,39 @@ final class WindowLayoutCalculatorTests: XCTestCase { XCTAssertEqual(moved, CGRect(x: -1280, y: 400, width: 1280, height: 700)) } + func testMovePreservingSizeMapsOnlyPositionAcrossDifferentDisplays() { + let source = CGRect(x: 0, y: 24, width: 1440, height: 876) + let destination = CGRect(x: 1440, y: -276, width: 2560, height: 1416) + let window = CGRect(x: 720, y: 462, width: 720, height: 438) + + let moved = calculator.movedFrame( + window, + from: source, + to: destination, + preservingSize: true + ) + + XCTAssertEqual(moved, CGRect(x: 3280, y: 702, width: 720, height: 438)) + } + + func testMovePreservingSizeDoesNotShrinkOversizedWindow() { + let source = CGRect(x: 0, y: 0, width: 1000, height: 800) + let destination = CGRect(x: 1000, y: -400, width: 600, height: 400) + let oversized = CGRect(x: -500, y: -300, width: 2000, height: 1600) + + let moved = calculator.movedFrame( + oversized, + from: source, + to: destination, + preservingSize: true + ) + + XCTAssertEqual(moved.size, oversized.size) + XCTAssertEqual(moved.minY, destination.minY) + XCTAssertGreaterThan(moved.intersection(destination).width, 0) + XCTAssertEqual(moved.intersection(destination).height, destination.height) + } + func testMoveOversizedWindowClampsInsideDestination() { let source = CGRect(x: 0, y: 0, width: 1000, height: 800) let destination = CGRect(x: 1000, y: -400, width: 600, height: 400) diff --git a/Plugins/WindowLayouts/Tests/WindowLayoutServiceTests.swift b/Plugins/WindowLayouts/Tests/WindowLayoutServiceTests.swift index 5fcf753e..c8c7ffbf 100644 --- a/Plugins/WindowLayouts/Tests/WindowLayoutServiceTests.swift +++ b/Plugins/WindowLayouts/Tests/WindowLayoutServiceTests.swift @@ -94,6 +94,36 @@ final class WindowLayoutServiceTests: XCTestCase { ) } + func testNonResizableWindowMovesBetweenDifferentSizedDisplaysWithoutResizing() async { + let window = makeWindow(canResize: false) + let originalFrame = CGRect(x: 720, y: 462, width: 720, height: 438) + let frameAdapter = MockWindowFrameAdapter(window: window, frame: originalFrame) + let screens = [ + WindowScreen( + id: "main", + frame: CGRect(x: 0, y: 0, width: 1440, height: 900), + visibleFrame: CGRect(x: 0, y: 24, width: 1440, height: 876) + ), + WindowScreen( + id: "right", + frame: CGRect(x: 1440, y: -300, width: 2560, height: 1440), + visibleFrame: CGRect(x: 1440, y: -276, width: 2560, height: 1416) + ), + ] + let service = makeService( + window: window, + frameAdapter: frameAdapter, + screens: screens + ) + + assertSuccess(await service.execute(.moveToNextDisplay, options: options())) + + XCTAssertEqual( + frameAdapter.frames[window.identity], + CGRect(x: 3280, y: 702, width: 720, height: 438) + ) + } + func testNonResizableWindowCanCenterButCannotTile() async { let window = makeWindow(canResize: false) let frameAdapter = MockWindowFrameAdapter( @@ -166,7 +196,106 @@ final class WindowLayoutServiceTests: XCTestCase { XCTAssertEqual( frameAdapter.frames[window.identity], - CGRect(x: 0, y: 24, width: 1440, height: 876) + CGRect(x: -360, y: 24, width: 1800, height: 1200) + ) + } + + func testRestoreKeepsOversizedOffTopFrameReachableWithoutResizing() async { + let window = makeWindow() + let frameAdapter = MockWindowFrameAdapter( + window: window, + frame: CGRect(x: 100, y: 100, width: 600, height: 400) + ) + let history = InMemoryWindowFrameHistory() + history.record( + CGRect(x: -300, y: -200, width: 2000, height: 1200), + for: window + ) + let service = makeService( + window: window, + frameAdapter: frameAdapter, + history: history + ) + + assertSuccess(await service.execute(.restorePreviousFrame, options: options())) + + XCTAssertEqual( + frameAdapter.frames[window.identity], + CGRect(x: -300, y: 24, width: 2000, height: 1200) + ) + } + + func testRestoreKeepsNonResizableOversizedWindowReachable() async { + let historicalFrame = CGRect(x: -300, y: -200, width: 2000, height: 1200) + let window = makeWindow(canResize: false) + let frameAdapter = MockWindowFrameAdapter(window: window, frame: historicalFrame) + let history = InMemoryWindowFrameHistory() + history.record(historicalFrame, for: window) + let service = makeService( + window: window, + frameAdapter: frameAdapter, + history: history + ) + + assertSuccess(await service.execute(.restorePreviousFrame, options: options())) + + XCTAssertEqual( + frameAdapter.frames[window.identity], + CGRect(x: -300, y: 24, width: 2000, height: 1200) + ) + } + + func testRestoreUsesStageManagerSafeVisibleFrame() async { + let window = makeWindow() + let frameAdapter = MockWindowFrameAdapter( + window: window, + frame: CGRect(x: 300, y: 100, width: 600, height: 400) + ) + let history = InMemoryWindowFrameHistory() + history.record(CGRect(x: 0, y: 24, width: 600, height: 400), for: window) + let safeFrame = CGRect(x: 200, y: 24, width: 1240, height: 876) + let service = makeService( + window: window, + frameAdapter: frameAdapter, + history: history, + stageManagerSafeAreaProvider: FixedStageManagerSafeAreaProvider( + safeFrame: safeFrame + ) + ) + + assertSuccess(await service.execute( + .restorePreviousFrame, + options: options(respectsStageManager: true) + )) + + XCTAssertEqual( + frameAdapter.frames[window.identity], + CGRect(x: 200, y: 24, width: 600, height: 400) + ) + } + + func testRestoreClampsFrameWithOnlySliverVisibleOnSurvivingDisplay() async { + let window = makeWindow() + let frameAdapter = MockWindowFrameAdapter( + window: window, + frame: CGRect(x: 100, y: 100, width: 600, height: 400) + ) + let history = InMemoryWindowFrameHistory() + history.record( + CGRect(x: 1439, y: 100, width: 600, height: 400), + for: window + ) + let service = makeService( + window: window, + frameAdapter: frameAdapter, + history: history + ) + + assertSuccess(await service.execute(.restorePreviousFrame, options: options())) + + XCTAssertEqual( + frameAdapter.frames[window.identity], + CGRect(x: 840, y: 100, width: 600, height: 400) ) } @@ -606,6 +735,7 @@ final class WindowLayoutServiceTests: XCTestCase { history: WindowFrameHistory = InMemoryWindowFrameHistory(), fullScreenWriter: WindowFullScreenWriting? = nil, focusedWindowResolver: FocusedWindowResolving? = nil, + stageManagerSafeAreaProvider: StageManagerSafeAreaProviding? = nil, waitForFrameSettlement: @escaping @MainActor @Sendable (Duration) async throws -> Void = { _ in } ) -> WindowLayoutService { WindowLayoutService( @@ -614,6 +744,8 @@ final class WindowLayoutServiceTests: XCTestCase { screenProvider: MockWindowScreenProvider(screens: screens), history: history, fullScreenWriter: fullScreenWriter, + stageManagerSafeAreaProvider: stageManagerSafeAreaProvider + ?? SystemStageManagerSafeAreaProvider(), waitForFrameSettlement: waitForFrameSettlement ) } @@ -640,15 +772,27 @@ final class WindowLayoutServiceTests: XCTestCase { XCTAssertEqual(error, expectedError, file: file, line: line) } - private func options(gap: CGFloat = 0) -> WindowLayoutExecutionOptions { + private func options( + gap: CGFloat = 0, + respectsStageManager: Bool = false + ) -> WindowLayoutExecutionOptions { WindowLayoutExecutionOptions( gap: gap, cyclesHalves: false, - respectsStageManager: false + respectsStageManager: respectsStageManager ) } } +@MainActor +private struct FixedStageManagerSafeAreaProvider: StageManagerSafeAreaProviding { + let safeFrame: CGRect + + func safeVisibleFrame(for screen: WindowScreen) -> CGRect { + safeFrame + } +} + @MainActor private final class MockFullScreenWriter: WindowFullScreenWriting { private(set) var values: [Bool] = [] diff --git a/Plugins/WindowLayouts/Tests/WindowLayoutsPluginTests.swift b/Plugins/WindowLayouts/Tests/WindowLayoutsPluginTests.swift index be679ac3..168bd3df 100644 --- a/Plugins/WindowLayouts/Tests/WindowLayoutsPluginTests.swift +++ b/Plugins/WindowLayouts/Tests/WindowLayoutsPluginTests.swift @@ -275,11 +275,28 @@ final class WindowLayoutsPluginTests: XCTestCase { ) XCTAssertEqual(plugin.customCommandShortcutBinding(for: id), binding) - plugin.clearCustomCommandShortcut(for: id) + XCTAssertEqual(plugin.clearCustomCommandShortcut(for: id), .accepted) XCTAssertNil(plugin.customCommandShortcutBinding(for: id)) } + func testCustomCommandShortcutClearReportsHostFailure() throws { + let plugin = makePlugin() + plugin.handleSettingsAction(.invoke(controlID: "add-custom")) + let definition = try XCTUnwrap(plugin.actionDefinitions.first(where: { + $0.key.actionID.hasPrefix("custom.") + })) + let id = try XCTUnwrap(UUID(uuidString: String( + definition.key.actionID.dropFirst("custom.".count) + ))) + plugin.applyActionShortcutPreset = { _, _ in "Shortcut storage failed" } + + XCTAssertEqual( + plugin.clearCustomCommandShortcut(for: id), + .rejected("Shortcut storage failed") + ) + } + func testDeletingCustomCommandClearsItsShortcutBeforeRemovingAction() throws { let plugin = makePlugin() plugin.handleSettingsAction(.invoke(controlID: "add-custom")) @@ -316,20 +333,9 @@ final class WindowLayoutsPluginTests: XCTestCase { let id = try XCTUnwrap(UUID(uuidString: String( definition.key.actionID.dropFirst("custom.".count) ))) - let binding = ShortcutBinding( - keyCode: UInt16(kVK_ANSI_L), - modifiers: [.control, .option] - ) - plugin.previewActionShortcutPreset = { actionIDs, proposedBindings in - PluginActionShortcutPresetPreview(items: actionIDs.map { actionID in - PluginActionShortcutPresetPreviewItem( - actionID: actionID, - currentBinding: binding, - proposedBinding: proposedBindings[actionID] - ) - }) + plugin.performActionShortcutReplacementTransaction = { _, _, _ in + "Shortcut storage failed" } - plugin.applyActionShortcutPreset = { _, _ in "Shortcut storage failed" } XCTAssertFalse(plugin.deleteCustomCommand(id)) @@ -368,6 +374,46 @@ final class WindowLayoutsPluginTests: XCTestCase { XCTAssertNotNil(plugin.customCommandDeletionError) } + func testCustomCommandDeleteFailureUsesTransactionToRestoreEveryShortcut() throws { + let storage = WindowLayoutsMemoryStorage() + let plugin = makePlugin(storage: storage) + plugin.handleSettingsAction(.invoke(controlID: "add-custom")) + let definition = try XCTUnwrap(plugin.actionDefinitions.first(where: { + $0.key.actionID.hasPrefix("custom.") + })) + let id = try XCTUnwrap(UUID(uuidString: String( + definition.key.actionID.dropFirst("custom.".count) + ))) + let originalBindings = [ + ShortcutBinding( + keyCode: UInt16(kVK_ANSI_L), + modifiers: [.control, .option] + ), + ShortcutBinding( + keyCode: UInt16(kVK_ANSI_M), + modifiers: [.control, .option] + ), + ] + var bindings = originalBindings + plugin.performActionShortcutReplacementTransaction = { _, _, mutation in + let snapshot = bindings + bindings = [] + if let error = mutation() { + bindings = snapshot + return error + } + return nil + } + storage.rejectLibraryWrites = true + + XCTAssertFalse(plugin.deleteCustomCommand(id)) + + XCTAssertEqual(bindings, originalBindings) + XCTAssertTrue(plugin.actionDefinitions.contains(where: { + $0.key.actionID == definition.key.actionID + })) + } + func testCustomCommandShortcutReportsHostValidationConflict() throws { let plugin = makePlugin() plugin.handleSettingsAction(.invoke(controlID: "add-custom")) @@ -398,7 +444,7 @@ final class WindowLayoutsPluginTests: XCTestCase { anchor: .center ) XCTAssertEqual( - WindowCustomCommandPreviewLayout(command: centered) + WindowCustomCommandPreviewLayout(command: centered, gap: 0) .windowFrame(in: CGSize(width: 160, height: 100)), CGRect(x: 32, y: 25, width: 96, height: 50) ) @@ -412,10 +458,73 @@ final class WindowLayoutsPluginTests: XCTestCase { offsetY: 90 ) XCTAssertEqual( - WindowCustomCommandPreviewLayout(command: offsetTopRight) + WindowCustomCommandPreviewLayout(command: offsetTopRight, gap: 0) .windowFrame(in: CGSize(width: 200, height: 100)), CGRect(x: 80, y: 10, width: 100, height: 40) ) + + var clampedBottomRight = offsetTopRight + clampedBottomRight.anchor = .bottomRight + clampedBottomRight.offsetX = 500 + clampedBottomRight.offsetY = 500 + XCTAssertEqual( + WindowCustomCommandPreviewLayout(command: clampedBottomRight, gap: 0) + .windowFrame(in: CGSize(width: 200, height: 100)), + CGRect(x: 100, y: 60, width: 100, height: 40) + ) + } + + func testCustomCommandPreviewLayoutMatchesExecutionGeometryWithGap() { + let command = WindowCustomCommand( + name: "Gap", + width: .fraction(0.5), + height: .fraction(0.4), + anchor: .topLeft, + offsetX: -50, + offsetY: -50 + ) + let referenceSize = CGSize(width: 1440, height: 900) + let expected = WindowLayoutCalculator().customFrame( + for: command, + windowFrame: CGRect(x: 288, y: 180, width: 864, height: 540), + visibleFrame: CGRect(origin: .zero, size: referenceSize), + gap: 17 + ) + + let preview = WindowCustomCommandPreviewLayout(command: command, gap: 17) + .windowFrame(in: referenceSize) + + XCTAssertEqual(preview, expected) + XCTAssertNotEqual( + preview, + WindowCustomCommandPreviewLayout(command: command, gap: 0) + .windowFrame(in: referenceSize) + ) + } + + func testUpdatingCustomCommandNormalizesBlankNameAndPersistsOtherEdits() throws { + let plugin = makePlugin() + plugin.handleSettingsAction(.invoke(controlID: "add-custom")) + let definition = try XCTUnwrap(plugin.actionDefinitions.first(where: { + $0.key.actionID.hasPrefix("custom.") + })) + let id = try XCTUnwrap(UUID(uuidString: String( + definition.key.actionID.dropFirst("custom.".count) + ))) + var command = try XCTUnwrap(plugin.customCommand(id: id)) + command.name = " \n " + command.width = .fraction(0.35) + command.anchor = .bottomRight + + XCTAssertTrue(plugin.updateCustomCommand(command)) + + let stored = try XCTUnwrap(plugin.customCommand(id: command.id)) + XCTAssertEqual( + stored.name, + plugin.localizedKey("settings.custom.defaultName", "自定义布局") + ) + XCTAssertEqual(stored.width, .fraction(0.35)) + XCTAssertEqual(stored.anchor, .bottomRight) } func testActionExecutionUsesCommittedGapAndReset() async throws { @@ -583,6 +692,28 @@ final class WindowLayoutsPluginTests: XCTestCase { XCTAssertEqual(executor.customExecutions.map(\.name), [definition.title]) } + func testCustomRunLinkPolicyChangeRequestsImmediateSafetyRebuild() throws { + let plugin = makePlugin() + plugin.handleSettingsAction(.invoke(controlID: "add-custom")) + let actionID = try XCTUnwrap(plugin.actionDefinitions.first(where: { + $0.key.actionID.hasPrefix("custom.") + })?.key.actionID) + let id = try XCTUnwrap(UUID(uuidString: String(actionID.dropFirst("custom.".count)))) + var customCommand = try XCTUnwrap(plugin.customCommand(id: id)) + var safetyChangeCount = 0 + plugin.onActionSafetyStateChange = { safetyChangeCount += 1 } + + customCommand.allowExternalInvocation = false + XCTAssertTrue(plugin.updateCustomCommand(customCommand)) + + XCTAssertEqual(safetyChangeCount, 1) + XCTAssertEqual( + plugin.actionDefinitions.first(where: { $0.key.actionID == actionID })? + .externalInvocationPolicy, + .unavailable + ) + } + private func makePlugin( executor: MockWindowLayoutExecutor? = nil, storage: PluginStorage? = nil, @@ -618,6 +749,18 @@ final class WindowLayoutsPluginTests: XCTestCase { } return nil } + plugin.performActionShortcutReplacementTransaction = { + actionIDs, bindings, mutation in + let snapshot = state.bindings + for actionID in actionIDs { + state.bindings[actionID] = bindings[actionID] + } + if let error = mutation() { + state.bindings = snapshot + return error + } + return nil + } } } diff --git a/Plugins/WindowLayouts/Tests/WindowLayoutsStoreTests.swift b/Plugins/WindowLayouts/Tests/WindowLayoutsStoreTests.swift index 86dd4b9a..13b06911 100644 --- a/Plugins/WindowLayouts/Tests/WindowLayoutsStoreTests.swift +++ b/Plugins/WindowLayouts/Tests/WindowLayoutsStoreTests.swift @@ -73,6 +73,22 @@ final class WindowLayoutsStoreTests: XCTestCase { XCTAssertEqual(copy.anchor, .top) } + func testCustomRunLinkPolicyNotifiesSafetyRegistryOnlyForPersistedChanges() throws { + let store = WindowLayoutsStore(storage: StoreMemoryStorage()) + var command = try XCTUnwrap(store.addCustomCommand(name: "Safety")) + var safetyMutationCount = 0 + store.onSafetyPolicyMutation = { safetyMutationCount += 1 } + + command.name = "Renamed" + XCTAssertTrue(store.updateCustomCommand(command)) + XCTAssertEqual(safetyMutationCount, 0) + + command.allowExternalInvocation = false + XCTAssertTrue(store.updateCustomCommand(command)) + XCTAssertTrue(store.updateCustomCommand(command)) + XCTAssertEqual(safetyMutationCount, 1) + } + func testDuplicateKeepsCopySuffixAtNameLengthBoundary() throws { let store = WindowLayoutsStore(storage: StoreMemoryStorage()) let source = try XCTUnwrap(store.addCustomCommand( diff --git a/Plugins/WindowLayouts/plugin.json b/Plugins/WindowLayouts/plugin.json index 83aa51fe..b7b4d82e 100644 --- a/Plugins/WindowLayouts/plugin.json +++ b/Plugins/WindowLayouts/plugin.json @@ -49,7 +49,7 @@ } }, "version": "1.0.0", - "minHostVersion": "1.2.0", + "minHostVersion": "1.2.1", "pluginKitVersion": 5, "bundleRelativePath": "WindowLayouts.bundle", "factoryClass": "WindowLayoutsPlugin.WindowLayoutsPluginFactory", diff --git a/Sources/App/MacToolsAppRuntime.swift b/Sources/App/MacToolsAppRuntime.swift index 99deee02..ba0614bc 100644 --- a/Sources/App/MacToolsAppRuntime.swift +++ b/Sources/App/MacToolsAppRuntime.swift @@ -78,10 +78,11 @@ final class MacToolsAppRuntime { pluginHost.installFocusedHostWindowProvider { [weak windowRouter] in windowRouter?.focusedWindowLayoutTarget } - pluginHost.actionExecutionFeedbackHandler = { [weak self] source, reference, outcome in + pluginHost.actionExecutionFeedbackHandler = { [weak self] source, reference, actionTitle, outcome in self?.presentHeadlessActionFeedback( source: source, reference: reference, + actionTitle: actionTitle, outcome: outcome ) } @@ -130,17 +131,17 @@ final class MacToolsAppRuntime { private func presentHeadlessActionFeedback( source: ActionExecutionSource, reference: ActionReference, + actionTitle: String?, outcome: ActionExecutionOutcome ) { guard reference.key.providerID == "window-layouts", - source == .globalShortcut || source == .trackpadGesture, - case let .success(action) = pluginHost.actionRegistry.registeredAction(for: reference) + source == .globalShortcut || source == .trackpadGesture else { return } if let feedback = WindowLayoutActionFeedback.feedback( - actionTitle: action.definition.title, + actionTitle: actionTitle, outcome: outcome ) { runLinkFeedbackPresenter.present(feedback) diff --git a/Sources/App/RunLinkExecutionCoordinator.swift b/Sources/App/RunLinkExecutionCoordinator.swift index 6b5accfc..2cafff96 100644 --- a/Sources/App/RunLinkExecutionCoordinator.swift +++ b/Sources/App/RunLinkExecutionCoordinator.swift @@ -196,9 +196,10 @@ private extension RunLinkExecutionFeedback.Tone { enum WindowLayoutActionFeedback { static func feedback( - actionTitle: String, + actionTitle: String?, outcome: ActionExecutionOutcome ) -> RunLinkExecutionFeedback? { + let actionTitle = actionTitle ?? FeatureL10n.string("窗口布局") switch outcome { case let .completed(.succeeded(message)): guard let message else { return nil } diff --git a/Sources/Core/Plugins/PluginHost.swift b/Sources/Core/Plugins/PluginHost.swift index 59b41148..d81e2319 100644 --- a/Sources/Core/Plugins/PluginHost.swift +++ b/Sources/Core/Plugins/PluginHost.swift @@ -337,6 +337,15 @@ struct PluginAutomaticUpdateVersionStore { @MainActor final class PluginHost: ObservableObject { + private struct ActionShortcutBurstState { + var admittedCount = 0 + var activeTaskCount = 0 + var lastCompletionUptime: TimeInterval? + } + + private static let maximumActionShortcutBurstCount = 9 + private static let actionShortcutBurstResetInterval: TimeInterval = 0.75 + private struct SettingsViewCacheKey: Hashable { enum Content: Hashable { case workspace @@ -420,7 +429,11 @@ final class PluginHost: ObservableObject { [ActionGridPresentationEntry], ActionExecutionSource ) -> Bool)? - private var activeActionShortcutReferences: Set = [] + @TaskLocal private static var actionShortcutAncestry: Set = [] + private var actionShortcutBursts: [ActionReference: ActionShortcutBurstState] = [:] +#if DEBUG + var actionShortcutBurstUptimeProviderForTests: (() -> TimeInterval)? +#endif private var preferencesBackupExportSelection: PreferencesBackupSelection? private var preferencesBackupRestoreContext: PreferencesActionRestoreContext? @@ -499,6 +512,7 @@ final class PluginHost: ObservableObject { var actionExecutionFeedbackHandler: (( ActionExecutionSource, ActionReference, + String?, ActionExecutionOutcome ) -> Void)? @@ -2781,6 +2795,35 @@ final class PluginHost: ObservableObject { } } } + if let transactionApplying = plugin as? + any PluginActionShortcutReplacementTransactionApplying { + transactionApplying.currentActionShortcutBindings = { + [weak self] actionIDs in + self?.shortcutAssignmentService.currentBindings( + providerID: pluginID, + managedActionIDs: actionIDs + ) ?? [:] + } + transactionApplying.performActionShortcutReplacementTransaction = { + [weak self] actionIDs, bindings, mutation in + guard let self else { + return FeatureL10n.string("无法应用快捷键预设。") + } + let previousBindings = self.actionBackedShortcutBindings() + let error = self.shortcutAssignmentService.performReplacementTransaction( + providerID: pluginID, + managedActionIDs: actionIDs, + bindingsByActionID: bindings, + mutation: mutation + ) + self.rebuildDerivedState() + self.syncGlobalShortcuts() + self.notifyChangedActionBackedShortcutBindings( + previous: previousBindings + ) + return error + } + } if let persistentPreferencesSignaling = plugin as? any PluginPersistentPreferencesChangeSignaling { persistentPreferencesSignaling.onPersistentPreferencesChange = { [weak self] in self?.preferencesBackupChangeReporter.didPersist(.plugin(pluginID)) @@ -4129,6 +4172,11 @@ final class PluginHost: ObservableObject { presetApplying.previewActionShortcutPreset = nil presetApplying.applyActionShortcutPreset = nil } + if let transactionApplying = plugin as? + any PluginActionShortcutReplacementTransactionApplying { + transactionApplying.currentActionShortcutBindings = nil + transactionApplying.performActionShortcutReplacementTransaction = nil + } (plugin as? any PluginSettingsPresenting)?.requestSettingsPresentation = nil (plugin as? any ActionGridHostContextConsuming)?.actionGridHostContext = nil (plugin as? any TrackpadActionHostContextConsuming)?.trackpadActionHostContext = nil @@ -5419,16 +5467,18 @@ final class PluginHost: ObservableObject { private func handleShortcutTrigger(shortcutID: String) { if let reference = shortcutAssignmentService.reference(forShortcutID: shortcutID) { - guard activeActionShortcutReferences.insert(reference).inserted else { - return - } + let ancestry = Self.actionShortcutAncestry + guard !ancestry.contains(reference) else { return } + guard admitActionShortcutTrigger(for: reference) else { return } Task { @MainActor [weak self] in guard let self else { return } - defer { activeActionShortcutReferences.remove(reference) } - await executeHeadlessActionNow( - reference: reference, - source: .globalShortcut - ) + defer { finishActionShortcutTrigger(for: reference) } + await Self.$actionShortcutAncestry.withValue(ancestry.union([reference])) { + await executeHeadlessActionNow( + reference: reference, + source: .globalShortcut + ) + } } return } @@ -5453,6 +5503,46 @@ final class PluginHost: ObservableObject { } } + /// Bounds asynchronously delivered synthetic retriggers without dropping ordinary repeats. + /// Carbon re-enters through the main queue, outside task-local ancestry, so the burst remains + /// active briefly after the last invocation completes. A later physical shortcut starts fresh. + private func admitActionShortcutTrigger(for reference: ActionReference) -> Bool { + let now = actionShortcutBurstUptime + var burst = actionShortcutBursts[reference] ?? ActionShortcutBurstState() + if burst.activeTaskCount == 0, + let lastCompletionUptime = burst.lastCompletionUptime, + now - lastCompletionUptime >= Self.actionShortcutBurstResetInterval { + burst = ActionShortcutBurstState() + } + guard burst.admittedCount < Self.maximumActionShortcutBurstCount else { + actionShortcutBursts[reference] = burst + return false + } + burst.admittedCount += 1 + burst.activeTaskCount += 1 + burst.lastCompletionUptime = nil + actionShortcutBursts[reference] = burst + return true + } + + private func finishActionShortcutTrigger(for reference: ActionReference) { + guard var burst = actionShortcutBursts[reference] else { return } + burst.activeTaskCount = max(0, burst.activeTaskCount - 1) + if burst.activeTaskCount == 0 { + burst.lastCompletionUptime = actionShortcutBurstUptime + } + actionShortcutBursts[reference] = burst + } + + private var actionShortcutBurstUptime: TimeInterval { +#if DEBUG + if let actionShortcutBurstUptimeProviderForTests { + return actionShortcutBurstUptimeProviderForTests() + } +#endif + return ProcessInfo.processInfo.systemUptime + } + private func handleShortcutRelease(shortcutID: String) { guard let descriptor = shortcutDescriptor(for: shortcutID), let eventHandler = descriptor.plugin as? any PluginShortcutEventHandling @@ -5480,6 +5570,8 @@ final class PluginHost: ObservableObject { reference: ActionReference, source: ActionExecutionSource ) async { + let actionTitle = try? actionRegistry.registeredAction(for: reference).get() + .definition.title let outcome = await actionExecutor.execute( ActionInvocation( reference: reference, @@ -5487,7 +5579,7 @@ final class PluginHost: ObservableObject { mode: .foreground ) ) - actionExecutionFeedbackHandler?(source, reference, outcome) + actionExecutionFeedbackHandler?(source, reference, actionTitle, outcome) } private func requestPermissionGuidance(forPluginID pluginID: String, permissionID: String) { diff --git a/Sources/Core/Shortcuts/ActionShortcutAssignmentStore.swift b/Sources/Core/Shortcuts/ActionShortcutAssignmentStore.swift index c8f0b5aa..a48073b1 100644 --- a/Sources/Core/Shortcuts/ActionShortcutAssignmentStore.swift +++ b/Sources/Core/Shortcuts/ActionShortcutAssignmentStore.swift @@ -103,22 +103,34 @@ final class ActionShortcutAssignmentStore { } @discardableResult - func replaceAll(_ assignments: [ActionShortcutAssignmentRecord]) -> ActionShortcutStoreWriteResult { + func replaceAll( + _ assignments: [ActionShortcutAssignmentRecord], + reportsCommittedChange: Bool = true + ) -> ActionShortcutStoreWriteResult { _ = self.assignments() guard loadError == nil else { return .rejected(rollbackSucceeded: true) } - return replaceAll(assignments, allowsRecovery: false) + return replaceAll( + assignments, + allowsRecovery: false, + reportsCommittedChange: reportsCommittedChange + ) } @discardableResult func replaceAllForRecovery( _ assignments: [ActionShortcutAssignmentRecord] ) -> ActionShortcutStoreWriteResult { - replaceAll(assignments, allowsRecovery: true) + replaceAll(assignments, allowsRecovery: true, reportsCommittedChange: true) + } + + func reportCommittedAssignmentsChange() { + preferencesBackupChangeReporter?.didPersist(.actionShortcutAssignments) } private func replaceAll( _ assignments: [ActionShortcutAssignmentRecord], - allowsRecovery: Bool + allowsRecovery: Bool, + reportsCommittedChange: Bool ) -> ActionShortcutStoreWriteResult { let previousAssignments = self.assignments() let previousPayloadWasValid = loadError == nil @@ -141,7 +153,9 @@ final class ActionShortcutAssignmentStore { ) } loadError = nil - preferencesBackupChangeReporter?.didPersist(.actionShortcutAssignments) + if reportsCommittedChange { + reportCommittedAssignmentsChange() + } return .committed } diff --git a/Sources/Core/Shortcuts/ShortcutAssignmentService.swift b/Sources/Core/Shortcuts/ShortcutAssignmentService.swift index d438593e..c50a97a3 100644 --- a/Sources/Core/Shortcuts/ShortcutAssignmentService.swift +++ b/Sources/Core/Shortcuts/ShortcutAssignmentService.swift @@ -253,9 +253,12 @@ final class ShortcutAssignmentService { key: ActionKey(providerID: providerID, actionID: actionID) ) let proposedBinding = bindingsByActionID[actionID] - let currentBinding = records.first(where: { - $0.reference.key == reference.key - })?.binding + let currentBindings = records.compactMap { record in + record.reference.key == reference.key ? record.binding : nil + } + let representativeCurrentBinding = currentBindings.first(where: { + $0 != proposedBinding + }) ?? currentBindings.first guard case let .success(action) = registry.registeredAction(for: reference), action.catalogEntry != nil, action.definition.capabilities.contains(.foregroundInteractive) @@ -263,7 +266,7 @@ final class ShortcutAssignmentService { if proposedBinding == nil { items.append(PluginActionShortcutPresetPreviewItem( actionID: actionID, - currentBinding: currentBinding, + currentBinding: representativeCurrentBinding, proposedBinding: nil )) continue @@ -319,7 +322,7 @@ final class ShortcutAssignmentService { items.append(PluginActionShortcutPresetPreviewItem( actionID: actionID, - currentBinding: currentBinding, + currentBinding: representativeCurrentBinding, proposedBinding: proposedBinding, conflictOwnerDescription: conflictOwnerDescription )) @@ -327,12 +330,39 @@ final class ShortcutAssignmentService { return PluginActionShortcutPresetPreview(items: items) } + func currentBindings( + providerID: String, + managedActionIDs: Set + ) -> [String: [ShortcutBinding]] { + guard store.loadError == nil else { return [:] } + return store.assignments().reduce(into: [:]) { result, record in + let key = record.reference.key + guard key.providerID == providerID, + managedActionIDs.contains(key.actionID) else { return } + result[key.actionID, default: []].append(record.binding) + } + } + /// Atomically replaces the assignments managed by a plugin shortcut preset. @discardableResult func replaceAssignments( providerID: String, managedActionIDs: Set, bindingsByActionID: [String: ShortcutBinding] + ) -> ActionShortcutMutationResult { + replaceAssignments( + providerID: providerID, + managedActionIDs: managedActionIDs, + bindingsByActionID: bindingsByActionID, + reportsCommittedChange: true + ) + } + + private func replaceAssignments( + providerID: String, + managedActionIDs: Set, + bindingsByActionID: [String: ShortcutBinding], + reportsCommittedChange: Bool ) -> ActionShortcutMutationResult { guard !providerID.isEmpty, !managedActionIDs.isEmpty, @@ -399,7 +429,7 @@ final class ShortcutAssignmentService { } records.append(contentsOf: requestedRecords) - switch store.replaceAll(records) { + switch store.replaceAll(records, reportsCommittedChange: reportsCommittedChange) { case .committed: synchronize( reservedRegistrations: reservedRegistrations, @@ -417,6 +447,55 @@ final class ShortcutAssignmentService { } } + /// Replaces managed assignments and rolls their exact records back if the paired mutation fails. + /// This keeps assignment IDs and converged records intact across a cross-store operation. + func performReplacementTransaction( + providerID: String, + managedActionIDs: Set, + bindingsByActionID: [String: ShortcutBinding], + mutation: () -> String? + ) -> String? { + let previousRecords = store.assignments() + guard store.loadError == nil else { + return ActionShortcutAssignmentError.recoveryRequired.localizedDescription + } + + switch replaceAssignments( + providerID: providerID, + managedActionIDs: managedActionIDs, + bindingsByActionID: bindingsByActionID, + reportsCommittedChange: false + ) { + case let .failure(error): + return error.localizedDescription + case .success: + break + } + + let assignmentsChanged = store.assignments() != previousRecords + guard let mutationError = mutation() else { + if assignmentsChanged { + store.reportCommittedAssignmentsChange() + } + return nil + } + switch store.replaceAll(previousRecords, reportsCommittedChange: false) { + case .committed: + synchronize( + reservedRegistrations: reservedRegistrations, + reservedOwnerDescriptions: reservedOwnerDescriptions + ) + return mutationError + case .rejected: + synchronize( + reservedRegistrations: reservedRegistrations, + reservedOwnerDescriptions: reservedOwnerDescriptions + ) + return mutationError + " " + + ActionShortcutAssignmentError.persistenceRollbackFailed.localizedDescription + } + } + /// Removes shortcut assignments for actions a loaded plugin has explicitly retired. @discardableResult func removeRetiredAssignments( diff --git a/Sources/MacToolsPluginKit/ActionModels.swift b/Sources/MacToolsPluginKit/ActionModels.swift index da9c66dc..0d682d40 100644 --- a/Sources/MacToolsPluginKit/ActionModels.swift +++ b/Sources/MacToolsPluginKit/ActionModels.swift @@ -646,6 +646,20 @@ public protocol PluginActionShortcutPresetApplying: AnyObject { ) -> String?)? { get set } } +/// Optional host transaction used when a plugin mutation and its managed shortcut replacement +/// must either both persist or restore the exact previous assignment records. +@MainActor +public protocol PluginActionShortcutReplacementTransactionApplying: AnyObject { + var currentActionShortcutBindings: (( + _ managedActionIDs: Set + ) -> [String: [ShortcutBinding]])? { get set } + var performActionShortcutReplacementTransaction: (( + _ managedActionIDs: Set, + _ bindingsByActionID: [String: ShortcutBinding], + _ mutation: () -> String? + ) -> String?)? { get set } +} + /// Optional hook for plugins whose custom settings UI summarizes action shortcut assignments. /// The host invokes it after the shared action-shortcut state changes so cached plugin views can /// publish fresh derived state without polling or owning a parallel shortcut store. diff --git a/Tests/App/RunLinkExecutionCoordinatorTests.swift b/Tests/App/RunLinkExecutionCoordinatorTests.swift index 2660566f..067cedf4 100644 --- a/Tests/App/RunLinkExecutionCoordinatorTests.swift +++ b/Tests/App/RunLinkExecutionCoordinatorTests.swift @@ -186,6 +186,16 @@ final class RunLinkExecutionCoordinatorTests: XCTestCase { message: "No adjacent Desktop." ) ) + + let staleActionFeedback = WindowLayoutActionFeedback.feedback( + actionTitle: nil, + outcome: .rejected(.unknownAction(ActionKey( + providerID: "window-layouts", + actionID: "custom.removed" + ))) + ) + XCTAssertEqual(staleActionFeedback?.title, FeatureL10n.string("窗口布局")) + XCTAssertEqual(staleActionFeedback?.tone, .failure) } func testCancellingSystemConfirmationDismissesPresentationAndResumesOnce() async { diff --git a/Tests/Core/Actions/PluginHostActionRegistryTests.swift b/Tests/Core/Actions/PluginHostActionRegistryTests.swift index 53e34535..e7649afd 100644 --- a/Tests/Core/Actions/PluginHostActionRegistryTests.swift +++ b/Tests/Core/Actions/PluginHostActionRegistryTests.swift @@ -599,7 +599,7 @@ final class PluginHostActionRegistryTests: XCTestCase { $0.binding == binding })?.shortcutID ) - plugin.operation = { + plugin.operation = { _ in shortcutManager.triggerForTests(shortcutID: shortcutID) await Task.yield() return .succeeded() @@ -616,6 +616,198 @@ final class PluginHostActionRegistryTests: XCTestCase { XCTAssertEqual(plugin.beginCount, 1) } + func testActionShortcutAncestrySuppressesIndirectRecursiveRetrigger() async throws { + let registrar = FakeCarbonHotKeyRegistrar() + let shortcutManager = GlobalShortcutManager(registrar: registrar) + let plugin = NativeActionTestPlugin() + let secondDefinition = ActionDefinition( + key: ActionKey(providerID: plugin.metadata.id, actionID: "second"), + title: "Second", + description: "Second test action", + systemImage: "bolt", + capabilities: [.foregroundInteractive] + ) + plugin.additionalDefinitions = [secondDefinition] + let host = makePluginHostForTests( + plugins: [plugin], + globalShortcutManager: shortcutManager + ) + let firstReference = ActionReference(key: plugin.definition.key) + let secondReference = ActionReference(key: secondDefinition.key) + let firstBinding = ShortcutBinding(keyCode: 8, modifiers: [.command, .option]) + let secondBinding = ShortcutBinding(keyCode: 9, modifiers: [.command, .option]) + XCTAssertEqual(host.setActionShortcutBinding(firstBinding, to: firstReference), .success) + XCTAssertEqual(host.setActionShortcutBinding(secondBinding, to: secondReference), .success) + let firstShortcutID = try XCTUnwrap( + shortcutManager.debugRegistrationsForTests.first(where: { + $0.binding == firstBinding + })?.shortcutID + ) + let secondShortcutID = try XCTUnwrap( + shortcutManager.debugRegistrationsForTests.first(where: { + $0.binding == secondBinding + })?.shortcutID + ) + plugin.operation = { invocation in + if invocation.reference == firstReference { + shortcutManager.triggerForTests(shortcutID: secondShortcutID) + } else { + shortcutManager.triggerForTests(shortcutID: firstShortcutID) + } + await Task.yield() + return .succeeded() + } + + shortcutManager.triggerForTests(shortcutID: firstShortcutID) + for _ in 0 ..< 50 where plugin.beginCount < 2 { + await Task.yield() + } + for _ in 0 ..< 20 { await Task.yield() } + + XCTAssertEqual(plugin.beginCount, 2) + } + + func testSerializedActionShortcutAllowsRepeatWhileFirstInvocationRuns() async throws { + let registrar = FakeCarbonHotKeyRegistrar() + let shortcutManager = GlobalShortcutManager(registrar: registrar) + let plugin = NativeActionTestPlugin() + plugin.actionConcurrencyPolicy = .serialize + let host = makePluginHostForTests( + plugins: [plugin], + globalShortcutManager: shortcutManager + ) + let reference = ActionReference(key: plugin.definition.key) + let binding = ShortcutBinding(keyCode: 8, modifiers: [.command, .option]) + XCTAssertEqual(host.setActionShortcutBinding(binding, to: reference), .success) + let shortcutID = try XCTUnwrap( + shortcutManager.debugRegistrationsForTests.first(where: { + $0.binding == binding + })?.shortcutID + ) + var firstContinuation: CheckedContinuation? + var executionOrder: [Int] = [] + plugin.operation = { _ in + let invocationNumber = plugin.beginCount + executionOrder.append(invocationNumber) + if invocationNumber == 1 { + await withCheckedContinuation { continuation in + firstContinuation = continuation + } + } + return .succeeded() + } + + shortcutManager.triggerForTests(shortcutID: shortcutID) + for _ in 0 ..< 50 where plugin.beginCount < 1 { await Task.yield() } + shortcutManager.triggerForTests(shortcutID: shortcutID) + for _ in 0 ..< 20 { await Task.yield() } + XCTAssertEqual(plugin.beginCount, 1) + firstContinuation?.resume() + for _ in 0 ..< 100 where executionOrder.count < 2 { await Task.yield() } + + XCTAssertEqual(plugin.beginCount, 2) + XCTAssertEqual(executionOrder, [1, 2]) + } + + func testActionShortcutBurstGuardStopsAsyncRecursiveRetriggerAndResets() async throws { + let registrar = FakeCarbonHotKeyRegistrar() + let shortcutManager = GlobalShortcutManager(registrar: registrar) + let plugin = NativeActionTestPlugin() + plugin.actionConcurrencyPolicy = .serialize + let host = makePluginHostForTests( + plugins: [plugin], + globalShortcutManager: shortcutManager + ) + var uptime: TimeInterval = 100 + host.actionShortcutBurstUptimeProviderForTests = { uptime } + let reference = ActionReference(key: plugin.definition.key) + let binding = ShortcutBinding(keyCode: 8, modifiers: [.command, .option]) + XCTAssertEqual(host.setActionShortcutBinding(binding, to: reference), .success) + let shortcutID = try XCTUnwrap( + shortcutManager.debugRegistrationsForTests.first(where: { + $0.binding == binding + })?.shortcutID + ) + plugin.operation = { _ in + DispatchQueue.main.async { + shortcutManager.triggerForTests(shortcutID: shortcutID) + } + await Task.yield() + return .succeeded() + } + + shortcutManager.triggerForTests(shortcutID: shortcutID) + for _ in 0 ..< 500 where plugin.beginCount < 9 { await Task.yield() } + for _ in 0 ..< 100 { await Task.yield() } + + XCTAssertEqual(plugin.beginCount, 9) + + uptime += 1 + plugin.operation = { _ in .succeeded() } + shortcutManager.triggerForTests(shortcutID: shortcutID) + for _ in 0 ..< 100 where plugin.beginCount < 10 { await Task.yield() } + + XCTAssertEqual(plugin.beginCount, 10) + } + + func testHeadlessFeedbackKeepsOriginalTitleAfterActionDisappears() async throws { + let registrar = FakeCarbonHotKeyRegistrar() + let shortcutManager = GlobalShortcutManager(registrar: registrar) + let plugin = NativeActionTestPlugin() + let disappearingDefinition = ActionDefinition( + key: ActionKey(providerID: plugin.metadata.id, actionID: "disappearing"), + title: "Original Title", + description: "Removed while running", + systemImage: "rectangle", + externalInvocationPolicy: .allowed, + capabilities: [.foregroundInteractive] + ) + plugin.additionalDefinitions = [disappearingDefinition] + let host = makePluginHostForTests( + plugins: [plugin], + globalShortcutManager: shortcutManager + ) + let reference = ActionReference(key: disappearingDefinition.key) + let binding = ShortcutBinding(keyCode: 9, modifiers: [.command, .option]) + XCTAssertEqual(host.setActionShortcutBinding(binding, to: reference), .success) + let shortcutID = try XCTUnwrap( + shortcutManager.debugRegistrationsForTests.first(where: { + $0.binding == binding + })?.shortcutID + ) + var executionContinuation: CheckedContinuation? + plugin.operation = { _ in + await withCheckedContinuation { executionContinuation = $0 } + return .succeeded() + } + var feedbackTitle: String? + var feedbackOutcome: ActionExecutionOutcome? + host.actionExecutionFeedbackHandler = { _, receivedReference, title, outcome in + guard receivedReference == reference else { return } + feedbackTitle = title + feedbackOutcome = outcome + } + + shortcutManager.triggerForTests(shortcutID: shortcutID) + for _ in 0 ..< 100 where executionContinuation == nil { + await Task.yield() + } + let continuation = try XCTUnwrap(executionContinuation) + + plugin.additionalDefinitions = [] + plugin.onStateChange?() + await host.waitForScheduledPluginStateRebuildForTests() + XCTAssertNil(try? host.actionRegistry.registeredAction(for: reference).get()) + + continuation.resume() + for _ in 0 ..< 100 where feedbackOutcome == nil { + await Task.yield() + } + + XCTAssertEqual(feedbackTitle, disappearingDefinition.title) + XCTAssertEqual(feedbackOutcome, .completed(.succeeded())) + } + func testHostAggregatesActionSurfaceAssignmentSummaries() { let plugin = NativeActionTestPlugin() plugin.summarizedReference = ActionReference(key: plugin.definition.key) @@ -1197,6 +1389,7 @@ private final class NativeActionTestPlugin: PluginActionPermissionProviding, PluginRetiredActionShortcutProviding, PluginActionShortcutPresetApplying, + PluginActionShortcutReplacementTransactionApplying, PluginActionShortcutAssignmentChangeHandling, ActionSurfaceAssignmentSummarizing { @@ -1217,14 +1410,21 @@ private final class NativeActionTestPlugin: var summarizedReference: ActionReference? var permissionTitles = ["测试权限"] var additionalDefinitions: [ActionDefinition] = [] + var actionConcurrencyPolicy: ActionConcurrencyPolicy = .rejectWhileRunning var retiredActionShortcutIDs: Set = [] var previewActionShortcutPreset: (( Set, [String: ShortcutBinding] ) -> PluginActionShortcutPresetPreview)? var applyActionShortcutPreset: ((Set, [String: ShortcutBinding]) -> String?)? + var currentActionShortcutBindings: ((Set) -> [String: [ShortcutBinding]])? + var performActionShortcutReplacementTransaction: (( + Set, + [String: ShortcutBinding], + () -> String? + ) -> String?)? private(set) var actionShortcutAssignmentChangeCount = 0 - var operation: @MainActor @Sendable () async -> ActionExecutionResult = { + var operation: @MainActor @Sendable (ActionInvocation) async -> ActionExecutionResult = { _ in .succeeded(message: "native") } @@ -1239,14 +1439,17 @@ private final class NativeActionTestPlugin: } } - let definition = ActionDefinition( - key: ActionKey(providerID: "action-provider", actionID: "toggle"), - title: "切换", - description: "切换测试状态", - systemImage: "bolt", - externalInvocationPolicy: .allowed, - capabilities: [.background, .foregroundInteractive] - ) + var definition: ActionDefinition { + ActionDefinition( + key: ActionKey(providerID: "action-provider", actionID: "toggle"), + title: "切换", + description: "切换测试状态", + systemImage: "bolt", + externalInvocationPolicy: .allowed, + capabilities: [.background, .foregroundInteractive], + concurrencyPolicy: actionConcurrencyPolicy + ) + } var actionDefinitions: [ActionDefinition] { [definition] + additionalDefinitions @@ -1269,7 +1472,9 @@ private final class NativeActionTestPlugin: func beginAction(_ invocation: ActionInvocation) throws -> ActionExecutionHandle { beginCount += 1 - return ActionExecutionHandle(operation: operation) + return ActionExecutionHandle { + await self.operation(invocation) + } } func actionSurfaceAssignmentSummary( diff --git a/Tests/Core/Shortcuts/ShortcutAssignmentServiceTests.swift b/Tests/Core/Shortcuts/ShortcutAssignmentServiceTests.swift index d4bf4271..92c52f24 100644 --- a/Tests/Core/Shortcuts/ShortcutAssignmentServiceTests.swift +++ b/Tests/Core/Shortcuts/ShortcutAssignmentServiceTests.swift @@ -41,7 +41,7 @@ final class ShortcutAssignmentServiceTests: XCTestCase { reference ) - let reloadedStore = ActionShortcutAssignmentStore(userDefaults: harness.defaults) + let reloadedStore = ActionShortcutAssignmentStore(defaults: harness.defaults) XCTAssertEqual(reloadedStore.assignments(), harness.service.assignments) } @@ -243,6 +243,155 @@ final class ShortcutAssignmentServiceTests: XCTestCase { XCTAssertEqual(harness.service.assignments, assignmentsBeforePreview) } + func testPresetPreviewReportsConvergedAssignmentNormalizationAsAChange() throws { + let harness = try makeHarness() + let records = [ + ActionShortcutAssignmentRecord( + reference: harness.references[0], + binding: harness.bindings[0] + ), + ActionShortcutAssignmentRecord( + reference: harness.references[0], + binding: harness.bindings[1] + ), + ] + XCTAssertEqual( + ActionShortcutAssignmentStore(defaults: harness.defaults).replaceAll(records), + .committed + ) + + let preview = harness.service.replacementPreview( + providerID: "shortcut-tests", + managedActionIDs: ["action-1"], + bindingsByActionID: ["action-1": harness.bindings[0]] + ) + + XCTAssertEqual( + harness.service.currentBindings( + providerID: "shortcut-tests", + managedActionIDs: ["action-1"] + )["action-1"], + harness.bindings + ) + XCTAssertTrue(preview.hasChanges) + XCTAssertEqual( + harness.service.replaceAssignments( + providerID: "shortcut-tests", + managedActionIDs: ["action-1"], + bindingsByActionID: ["action-1": harness.bindings[0]] + ), + .success + ) + XCTAssertEqual(harness.service.assignments.map(\.binding), [harness.bindings[0]]) + } + + func testReplacementTransactionRestoresExactConvergedRecordsWhenMutationFails() throws { + let reporter = PreferencesBackupChangeReporter() + var reportedSources: [PreferencesBackupChangeSource] = [] + reporter.onCommittedChange = { reportedSources.append($0) } + let harness = try makeHarness(preferencesBackupChangeReporter: reporter) + let records = [ + ActionShortcutAssignmentRecord( + reference: harness.references[0], + binding: harness.bindings[0] + ), + ActionShortcutAssignmentRecord( + reference: harness.references[0], + binding: harness.bindings[1] + ), + ] + XCTAssertEqual( + ActionShortcutAssignmentStore(defaults: harness.defaults).replaceAll(records), + .committed + ) + + let error = harness.service.performReplacementTransaction( + providerID: "shortcut-tests", + managedActionIDs: ["action-1"], + bindingsByActionID: [:] + ) { + "Layout storage failed" + } + + XCTAssertEqual(error, "Layout storage failed") + XCTAssertEqual(harness.service.assignments, records) + XCTAssertEqual(reportedSources, []) + } + + func testReplacementTransactionReportsChangedAssignmentsAfterMutationSucceeds() throws { + let reporter = PreferencesBackupChangeReporter() + var reportedSources: [PreferencesBackupChangeSource] = [] + reporter.onCommittedChange = { reportedSources.append($0) } + let harness = try makeHarness(preferencesBackupChangeReporter: reporter) + + let error = harness.service.performReplacementTransaction( + providerID: "shortcut-tests", + managedActionIDs: ["action-1"], + bindingsByActionID: ["action-1": harness.bindings[0]] + ) { + nil + } + + XCTAssertNil(error) + XCTAssertEqual(reportedSources, [.actionShortcutAssignments]) + } + + func testReplacementTransactionDoesNotReportNoOpAssignments() throws { + let reporter = PreferencesBackupChangeReporter() + var reportedSources: [PreferencesBackupChangeSource] = [] + reporter.onCommittedChange = { reportedSources.append($0) } + let harness = try makeHarness(preferencesBackupChangeReporter: reporter) + + let error = harness.service.performReplacementTransaction( + providerID: "shortcut-tests", + managedActionIDs: ["action-1"], + bindingsByActionID: [:] + ) { + nil + } + + XCTAssertNil(error) + XCTAssertEqual(reportedSources, []) + } + + func testReplacementTransactionDoesNotReportWhenRollbackFails() throws { + let defaults = ScriptedActionShortcutDefaults() + let initialRecord = ActionShortcutAssignmentRecord( + reference: ActionReference( + key: ActionKey(providerID: "shortcut-tests", actionID: "action-1") + ), + binding: ShortcutBinding(keyCode: 10, modifiers: [.command, .option]) + ) + XCTAssertEqual( + ActionShortcutAssignmentStore(defaults: defaults).replaceAll([initialRecord]), + .committed + ) + + let reporter = PreferencesBackupChangeReporter() + var reportedSources: [PreferencesBackupChangeSource] = [] + reporter.onCommittedChange = { reportedSources.append($0) } + let harness = try makeHarness( + defaults: defaults, + preferencesBackupChangeReporter: reporter + ) + defaults.payloadWriteBehaviors = [.accept, .corrupt, .ignore] + + let error = harness.service.performReplacementTransaction( + providerID: "shortcut-tests", + managedActionIDs: ["action-1"], + bindingsByActionID: ["action-1": harness.bindings[1]] + ) { + "Layout storage failed" + } + + XCTAssertEqual( + error, + "Layout storage failed " + + ActionShortcutAssignmentError.persistenceRollbackFailed.localizedDescription + ) + XCTAssertEqual(reportedSources, []) + } + func testPresetPreviewReportsConflictOutsideManagedAssignments() throws { let harness = try makeHarness() XCTAssertEqual( @@ -271,7 +420,7 @@ final class ShortcutAssignmentServiceTests: XCTestCase { binding: harness.bindings[0] ) XCTAssertEqual( - ActionShortcutAssignmentStore(userDefaults: harness.defaults) + ActionShortcutAssignmentStore(defaults: harness.defaults) .replaceAll([retiredRecord]), .committed ) @@ -660,9 +809,18 @@ final class ShortcutAssignmentServiceTests: XCTestCase { XCTAssertNotNil(store.loadError) } - private func makeHarness() throws -> ShortcutServiceHarness { - let defaults = try XCTUnwrap(UserDefaults(suiteName: suiteName)) - defaults.removePersistentDomain(forName: suiteName) + private func makeHarness( + defaults suppliedDefaults: (any ActionShortcutAssignmentPersisting)? = nil, + preferencesBackupChangeReporter: PreferencesBackupChangeReporter? = nil + ) throws -> ShortcutServiceHarness { + let defaults: any ActionShortcutAssignmentPersisting + if let suppliedDefaults { + defaults = suppliedDefaults + } else { + let userDefaults = try XCTUnwrap(UserDefaults(suiteName: suiteName)) + userDefaults.removePersistentDomain(forName: suiteName) + defaults = userDefaults + } let registry = ActionRegistry() let provider = ShortcutActionTestProvider() let definitions = (1 ... 2).map { index in @@ -694,7 +852,10 @@ final class ShortcutAssignmentServiceTests: XCTestCase { let manager = GlobalShortcutManager(registrar: registrar) let service = ShortcutAssignmentService( registry: registry, - store: ActionShortcutAssignmentStore(userDefaults: defaults), + store: ActionShortcutAssignmentStore( + defaults: defaults, + preferencesBackupChangeReporter: preferencesBackupChangeReporter + ), shortcutManager: manager ) service.synchronize(reservedRegistrations: [], reservedOwnerDescriptions: [:]) @@ -770,7 +931,7 @@ private final class ScriptedActionShortcutDefaults: ActionShortcutAssignmentPers @MainActor private struct ShortcutServiceHarness { - let defaults: UserDefaults + let defaults: any ActionShortcutAssignmentPersisting let registry: ActionRegistry let registrar: FakeCarbonHotKeyRegistrar let manager: GlobalShortcutManager diff --git a/changes/unreleased/window-layouts-execution-safety.md b/changes/unreleased/window-layouts-execution-safety.md index 6ac34bec..8460878d 100644 --- a/changes/unreleased/window-layouts-execution-safety.md +++ b/changes/unreleased/window-layouts-execution-safety.md @@ -5,3 +5,9 @@ area: Productivity --- Window Layouts now keeps queued commands bound to their original window, stops cancelled work promptly, and removes custom-layout shortcuts together with deleted layouts. + +Disabled custom-layout Run Links now stop immediately, restored windows stay reachable after display changes, and custom previews match the applied edge placement. + +Shortcut preset previews now reveal every existing assignment, failed custom-layout deletion restores them all, inline shortcut errors stay visible, and non-resizable windows keep their size when moved between displays. + +Rapid repeated shortcuts now reach the layout queue, restored windows keep their title bar reachable, and custom-layout previews and blank-name edits stay consistent with saved settings. diff --git a/changes/unreleased/window-layouts-host-feedback.md b/changes/unreleased/window-layouts-host-feedback.md index 80c0e8f4..06753705 100644 --- a/changes/unreleased/window-layouts-host-feedback.md +++ b/changes/unreleased/window-layouts-host-feedback.md @@ -5,3 +5,5 @@ area: Productivity --- Window actions now preserve the intended frontmost window across transient MacTools command surfaces and can show brief headless-action feedback without stealing focus. + +Headless Window Layouts failures remain visible even when their custom action was removed during execution. diff --git a/scripts/fixtures/plugin-kit-v5/Client.swift b/scripts/fixtures/plugin-kit-v5/Client.swift index 185e362f..a41f475e 100644 --- a/scripts/fixtures/plugin-kit-v5/Client.swift +++ b/scripts/fixtures/plugin-kit-v5/Client.swift @@ -1,5 +1,14 @@ import MacToolsPluginKit +@MainActor +private final class LegacyPresetApplying: PluginActionShortcutPresetApplying { + var previewActionShortcutPreset: (( + Set, + [String: ShortcutBinding] + ) -> PluginActionShortcutPresetPreview)? + var applyActionShortcutPreset: ((Set, [String: ShortcutBinding]) -> String?)? +} + @main struct PluginKitV5CompatibilityClient { @MainActor @@ -70,5 +79,31 @@ struct PluginKitV5CompatibilityClient { state.statusText == "Required" else { fatalError("Plugin permission v5 value ABI changed") } + + let previewItem = PluginActionShortcutPresetPreviewItem( + actionID: "run", + currentBinding: binding, + proposedBinding: nil + ) + let previewLabels = Mirror(reflecting: previewItem).children.compactMap(\.label) + guard previewLabels == [ + "actionID", + "currentBinding", + "proposedBinding", + "conflictOwnerDescription", + ] else { + fatalError("PluginActionShortcutPresetPreviewItem v5 stored layout changed: \(previewLabels)") + } + + let legacy = LegacyPresetApplying() + let legacyPresetApplying: any PluginActionShortcutPresetApplying = legacy + legacyPresetApplying.previewActionShortcutPreset = { _, _ in + PluginActionShortcutPresetPreview(items: [previewItem]) + } + legacyPresetApplying.applyActionShortcutPreset = { _, _ in nil } + guard legacyPresetApplying.previewActionShortcutPreset?(["run"], [:]).items.count == 1, + legacyPresetApplying.applyActionShortcutPreset?(["run"], [:]) == nil else { + fatalError("Legacy PluginActionShortcutPresetApplying conformance failed") + } } } diff --git a/scripts/fixtures/plugin-kit-v5/MacToolsPluginKit.swift b/scripts/fixtures/plugin-kit-v5/MacToolsPluginKit.swift index 90d3b244..582d2128 100644 --- a/scripts/fixtures/plugin-kit-v5/MacToolsPluginKit.swift +++ b/scripts/fixtures/plugin-kit-v5/MacToolsPluginKit.swift @@ -103,6 +103,46 @@ public struct PluginPermissionState { } } +public struct PluginActionShortcutPresetPreviewItem: Equatable, Sendable { + public let actionID: String + public let currentBinding: ShortcutBinding? + public let proposedBinding: ShortcutBinding? + public let conflictOwnerDescription: String? + + public init( + actionID: String, + currentBinding: ShortcutBinding?, + proposedBinding: ShortcutBinding?, + conflictOwnerDescription: String? = nil + ) { + fatalError("The compatibility client must link this initializer from the current framework") + } +} + +public struct PluginActionShortcutPresetPreview: Equatable, Sendable { + public let items: [PluginActionShortcutPresetPreviewItem] + public let errorMessage: String? + + public init( + items: [PluginActionShortcutPresetPreviewItem], + errorMessage: String? = nil + ) { + fatalError("The compatibility client must link this initializer from the current framework") + } +} + +@MainActor +public protocol PluginActionShortcutPresetApplying: AnyObject { + var previewActionShortcutPreset: (( + _ managedActionIDs: Set, + _ bindingsByActionID: [String: ShortcutBinding] + ) -> PluginActionShortcutPresetPreview)? { get set } + var applyActionShortcutPreset: (( + _ managedActionIDs: Set, + _ bindingsByActionID: [String: ShortcutBinding] + ) -> String?)? { get set } +} + public enum PluginShortcutRecordingResult: Equatable { case accepted case rejected(String) diff --git a/scripts/tests/test_plugin_minimum_host_compatibility.py b/scripts/tests/test_plugin_minimum_host_compatibility.py index 3e18a19c..634d337e 100644 --- a/scripts/tests/test_plugin_minimum_host_compatibility.py +++ b/scripts/tests/test_plugin_minimum_host_compatibility.py @@ -14,6 +14,7 @@ MAKEFILE = REPO_ROOT / "Makefile" ACTION_MODELS = REPO_ROOT / "Sources/MacToolsPluginKit/ActionModels.swift" COMPONENT_THEME_MODELS = REPO_ROOT / "Sources/MacToolsPluginKit/PluginComponentTheme.swift" +APP_VERSION_CONFIG = REPO_ROOT / "Configs/AppVersion.xcconfig" NEW_API_MINIMUM_HOSTS = { # Canonical action registry, execution, discovery, and surface bridges. "ActionKey": "1.2.0", @@ -40,6 +41,7 @@ "PluginActionShortcutPresetPreviewItem": "1.2.0", "PluginActionShortcutPresetPreview": "1.2.0", "PluginActionShortcutPresetApplying": "1.2.0", + "PluginActionShortcutReplacementTransactionApplying": "1.2.1", "PluginActionShortcutAssignmentChangeHandling": "1.2.0", "PluginActionExecutionRevisionProviding": "1.2.0", "PluginActionExposureProviding": "1.2.0", @@ -102,6 +104,16 @@ def version_tuple(value: str) -> tuple[int, ...]: return tuple(int(component) for component in value.split(".")) +def declared_app_version() -> str: + match = re.search( + r"(?m)^\s*MARKETING_VERSION\s*=\s*([^\s#]+)", + APP_VERSION_CONFIG.read_text(encoding="utf-8"), + ) + if match is None: + raise AssertionError("MARKETING_VERSION is missing") + return match.group(1) + + class PluginMinimumHostCompatibilityTests(unittest.TestCase): def test_legacy_v4_catalog_remains_compatible_with_shipped_1_1_6_verifier(self) -> None: catalog = json.loads(LEGACY_V4_CATALOG.read_text(encoding="utf-8")) @@ -126,14 +138,28 @@ def test_plugin_kit5_release_targets_versioned_host_compatible_catalog(self) -> makefile, ) - def test_every_current_plugin_targets_plugin_kit5_and_mac_tools_1_2(self) -> None: + def test_every_current_plugin_targets_plugin_kit5_and_a_released_host_line(self) -> None: incompatible = [] for manifest_path in sorted(PLUGINS_ROOT.glob("*/plugin.json")): manifest = json.loads(manifest_path.read_text(encoding="utf-8")) - if manifest["pluginKitVersion"] != 5 or manifest["minHostVersion"] != "1.2.0": + if ( + manifest["pluginKitVersion"] != 5 + or version_tuple(manifest["minHostVersion"]) < version_tuple("1.2.0") + or version_tuple(manifest["minHostVersion"]) + > version_tuple(declared_app_version()) + ): incompatible.append(manifest["id"]) self.assertEqual(incompatible, []) + def test_every_new_plugin_kit_api_is_exported_by_the_declared_app_version(self) -> None: + app_version = declared_app_version() + newer_symbols = { + symbol: required + for symbol, required in NEW_API_MINIMUM_HOSTS.items() + if version_tuple(required) > version_tuple(app_version) + } + self.assertEqual(newer_symbols, {}) + def test_new_plugin_kit_api_consumers_require_compatible_host(self) -> None: violations: list[str] = [] for manifest_path in sorted(PLUGINS_ROOT.glob("*/plugin.json")):