From a652dfd77a39a7b8e4296a173ecc2cd0e4776475 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Thu, 16 Jul 2026 18:31:58 +0200 Subject: [PATCH 1/2] Cancel in-flight cover action before starting a new one Tapping Stop (or the icon) while a cover is opening/closing appeared to stop it, but ~2s later it snapped fully open/closed anyway. The VM assigned actionTask = Task { ... } for each action without cancelling the previous task, so the still-running demo open/close bypassed its `guard !Task.isCancelled` guard and overwrote the state Stop had just set. Cancel the outstanding actionTask before starting each new action (open/close/stop/setPosition/toggle) so the in-flight demo travel bails. Add a Swift Testing case with a slow spy CoverControlling that mirrors the demo guard, verifying stop during open cancels the open so it never completes. Co-Authored-By: Claude Opus 4.8 --- .../Cover/UI/CoverCardViewModel.swift | 5 ++ .../Entities/CoverCardViewModelTests.swift | 50 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/Hemera/Entities/Cover/UI/CoverCardViewModel.swift b/Hemera/Entities/Cover/UI/CoverCardViewModel.swift index a596f51..2faa555 100644 --- a/Hemera/Entities/Cover/UI/CoverCardViewModel.swift +++ b/Hemera/Entities/Cover/UI/CoverCardViewModel.swift @@ -134,6 +134,7 @@ final class CoverCardViewModel: Identifiable { func setPosition(to position: Int) { guard cover.isAvailable else { return } + actionTask?.cancel() pendingPosition = position cooldown.commit() actionTask = Task { @@ -171,6 +172,7 @@ final class CoverCardViewModel: Identifiable { func open() { guard cover.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.openCover(id) } @@ -178,6 +180,7 @@ final class CoverCardViewModel: Identifiable { func close() { guard cover.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.closeCover(id) } @@ -185,12 +188,14 @@ final class CoverCardViewModel: Identifiable { func stop() { guard cover.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.stopCover(id) } } private func toggle() { + actionTask?.cancel() actionTask = Task { await controller.toggleCover(id) } diff --git a/HemeraTests/Entities/CoverCardViewModelTests.swift b/HemeraTests/Entities/CoverCardViewModelTests.swift index b3d96b5..436accc 100644 --- a/HemeraTests/Entities/CoverCardViewModelTests.swift +++ b/HemeraTests/Entities/CoverCardViewModelTests.swift @@ -131,6 +131,30 @@ struct CoverCardViewModelTests { #expect(spy.openedIds.isEmpty) } + // MARK: - Action Cancellation + + /** + Stopping mid-open must cancel the in-flight open Task. Otherwise the demo + controller's post-sleep `guard !Task.isCancelled` never fires and the still-running + open overwrites the state Stop just set (cover snaps fully open ~2s after Stop). + */ + @Test + func stop_duringOpen_cancelsInFlightOpenSoItDoesNotOverwriteState() async { + let controller = SlowCoverControlling() + let vm = makeViewModel(features: [.open, .stop], controller: controller) + + vm.open() + let openTask = vm.actionTask + vm.stop() + + await openTask?.value + await vm.actionTask?.value + + #expect(openTask?.isCancelled == true) + #expect(controller.didCompleteOpen == false) + #expect(controller.stoppedIds == ["cover.test"]) + } + // MARK: - Simple State Description @Test @@ -317,3 +341,29 @@ private final class SpyCoverControlling: CoverControlling { toggledIds.append(id) } } + +/** + Mirrors the demo controller: `openCover` simulates travel, then only completes if + its Task was not cancelled — the same `guard !Task.isCancelled` the real bug hinges on. + */ +@MainActor +private final class SlowCoverControlling: CoverControlling { + var didCompleteOpen = false + var stoppedIds: [String] = [] + + func setPosition(of id: String, to position: Int) async {} + + func openCover(_ id: String) async { + try? await Task.sleep(for: .seconds(2)) + guard !Task.isCancelled else { return } + didCompleteOpen = true + } + + func closeCover(_ id: String) async {} + + func stopCover(_ id: String) async { + stoppedIds.append(id) + } + + func toggleCover(_ id: String) async {} +} From 7047c2781e671244c342ff5f77ae177cd8afab36 Mon Sep 17 00:00:00 2001 From: Adam Borbas Date: Thu, 16 Jul 2026 22:20:09 +0200 Subject: [PATCH 2/2] Cancel in-flight climate action and cover-action test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same fire-and-forget `actionTask = Task { ... }` pattern in ClimateCardViewModel replaced the task reference without cancelling the prior task, so a still-running demo action (post-sleep `guard !Task.isCancelled`) could overwrite the state a newer action just applied — the same defect just fixed for Cover. Cancel the outstanding actionTask before starting each new climate action. Also extend cover cancellation coverage to close(), setPosition(), and toggle() (via iconTapped), which the original fix changed but left untested. Co-Authored-By: Claude Opus 4.8 --- .../Climate/UI/ClimateCardViewModel.swift | 8 +++ .../Entities/ClimateCardViewModelTests.swift | 51 ++++++++++++++ .../Entities/CoverCardViewModelTests.swift | 67 ++++++++++++++++++- 3 files changed, 123 insertions(+), 3 deletions(-) diff --git a/Hemera/Entities/Climate/UI/ClimateCardViewModel.swift b/Hemera/Entities/Climate/UI/ClimateCardViewModel.swift index 5322d93..1add9b7 100644 --- a/Hemera/Entities/Climate/UI/ClimateCardViewModel.swift +++ b/Hemera/Entities/Climate/UI/ClimateCardViewModel.swift @@ -200,6 +200,7 @@ final class ClimateCardViewModel: Identifiable { func togglePower() { guard climate.isAvailable else { return } + actionTask?.cancel() if climate.state == .off { actionTask = Task { await controller.turnOnClimate(id) } } else { @@ -209,6 +210,7 @@ final class ClimateCardViewModel: Identifiable { func setHVACMode(_ mode: ClimateEntity.HVACMode) { guard climate.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.setHVACMode(id, mode: mode.rawValue) } } @@ -217,6 +219,7 @@ final class ClimateCardViewModel: Identifiable { resetPending() pendingTargetTemp = temperature cooldown.commit() + actionTask?.cancel() actionTask = Task { await controller.setTemperature(id, temperature: temperature) } } @@ -226,6 +229,7 @@ final class ClimateCardViewModel: Identifiable { pendingTargetTempLow = low pendingTargetTempHigh = high cooldown.commit() + actionTask?.cancel() actionTask = Task { await controller.setTemperatureRange(id, low: low, high: high) } } @@ -243,21 +247,25 @@ final class ClimateCardViewModel: Identifiable { func setFanMode(_ mode: String) { guard climate.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.setFanMode(id, mode: mode) } } func setSwingMode(_ mode: String) { guard climate.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.setSwingMode(id, mode: mode) } } func setPresetMode(_ mode: String) { guard climate.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.setPresetMode(id, mode: mode) } } func setHumidity(_ humidity: Double) { guard climate.isAvailable else { return } + actionTask?.cancel() actionTask = Task { await controller.setHumidity(id, humidity: humidity) } } diff --git a/HemeraTests/Entities/ClimateCardViewModelTests.swift b/HemeraTests/Entities/ClimateCardViewModelTests.swift index 446d1db..125f001 100644 --- a/HemeraTests/Entities/ClimateCardViewModelTests.swift +++ b/HemeraTests/Entities/ClimateCardViewModelTests.swift @@ -282,6 +282,29 @@ struct ClimateCardViewModelTests { #expect(spy.setTemperatureCalls.isEmpty) } + // MARK: - Action Cancellation + + /** + A second action must cancel the in-flight one. Otherwise the demo controller's + post-sleep `guard !Task.isCancelled` never fires and the still-running earlier + action overwrites the state the newer action just applied. + */ + @Test func setTemperature_whileActionInFlight_cancelsPriorAction() async { + let controller = SlowClimateControlling() + let vm = makeViewModel(state: .heat, controller: controller) + + vm.setTemperature(20) + let firstTask = vm.actionTask + vm.setHVACMode(.cool) + + await firstTask?.value + await vm.actionTask?.value + + #expect(firstTask?.isCancelled == true) + #expect(controller.didCompleteSetTemperature == false) + #expect(controller.setHVACModeCalls == ["cool"]) + } + // MARK: - Available HVAC Modes @Test func availableHVACModes_parsesFromRaw() { @@ -408,3 +431,31 @@ private final class SpyClimateControlling: ClimateControlling { turnOffCalls.append(id) } } + +/** + Mirrors the demo controller: `setTemperature` simulates a delayed apply, then only + completes if its Task was not cancelled — the same `guard !Task.isCancelled` the bug hinges on. + */ +@MainActor +private final class SlowClimateControlling: ClimateControlling { + var didCompleteSetTemperature = false + var setHVACModeCalls: [String] = [] + + func setHVACMode(_ id: String, mode: String) async { + setHVACModeCalls.append(mode) + } + + func setTemperature(_ id: String, temperature: Double) async { + try? await Task.sleep(for: .seconds(2)) + guard !Task.isCancelled else { return } + didCompleteSetTemperature = true + } + + func setTemperatureRange(_ id: String, low: Double, high: Double) async {} + func setFanMode(_ id: String, mode: String) async {} + func setSwingMode(_ id: String, mode: String) async {} + func setPresetMode(_ id: String, mode: String) async {} + func setHumidity(_ id: String, humidity: Double) async {} + func turnOnClimate(_ id: String) async {} + func turnOffClimate(_ id: String) async {} +} diff --git a/HemeraTests/Entities/CoverCardViewModelTests.swift b/HemeraTests/Entities/CoverCardViewModelTests.swift index 436accc..6f10f6e 100644 --- a/HemeraTests/Entities/CoverCardViewModelTests.swift +++ b/HemeraTests/Entities/CoverCardViewModelTests.swift @@ -155,6 +155,58 @@ struct CoverCardViewModelTests { #expect(controller.stoppedIds == ["cover.test"]) } + @Test + func close_duringOpen_cancelsInFlightOpen() async { + let controller = SlowCoverControlling() + let vm = makeViewModel(features: [.open, .close], controller: controller) + + vm.open() + let openTask = vm.actionTask + vm.close() + + await openTask?.value + await vm.actionTask?.value + + #expect(openTask?.isCancelled == true) + #expect(controller.didCompleteOpen == false) + #expect(controller.closedIds == ["cover.test"]) + } + + @Test + func setPosition_duringOpen_cancelsInFlightOpen() async { + let controller = SlowCoverControlling() + let vm = makeViewModel(features: [.open, .setPosition], controller: controller) + + vm.open() + let openTask = vm.actionTask + vm.setPosition(to: 50) + + await openTask?.value + await vm.actionTask?.value + + #expect(openTask?.isCancelled == true) + #expect(controller.didCompleteOpen == false) + #expect(controller.positionCalls == [50]) + } + + @Test + func toggleViaIconTapped_duringOpen_cancelsInFlightOpen() async { + let controller = SlowCoverControlling() + // No .open feature → iconTapped's .closed case falls back to toggle(). + let vm = makeViewModel(state: .closed, features: [], controller: controller) + + vm.open() + let openTask = vm.actionTask + vm.iconTapped() + + await openTask?.value + await vm.actionTask?.value + + #expect(openTask?.isCancelled == true) + #expect(controller.didCompleteOpen == false) + #expect(controller.toggledIds == ["cover.test"]) + } + // MARK: - Simple State Description @Test @@ -350,8 +402,13 @@ private final class SpyCoverControlling: CoverControlling { private final class SlowCoverControlling: CoverControlling { var didCompleteOpen = false var stoppedIds: [String] = [] + var positionCalls: [Int] = [] + var closedIds: [String] = [] + var toggledIds: [String] = [] - func setPosition(of id: String, to position: Int) async {} + func setPosition(of id: String, to position: Int) async { + positionCalls.append(position) + } func openCover(_ id: String) async { try? await Task.sleep(for: .seconds(2)) @@ -359,11 +416,15 @@ private final class SlowCoverControlling: CoverControlling { didCompleteOpen = true } - func closeCover(_ id: String) async {} + func closeCover(_ id: String) async { + closedIds.append(id) + } func stopCover(_ id: String) async { stoppedIds.append(id) } - func toggleCover(_ id: String) async {} + func toggleCover(_ id: String) async { + toggledIds.append(id) + } }