Fix combo save lifecycle

[IgorArkhipov]

EN

Problem

Editing a combo could send its partially filled state to firmware immediately. On K:04/RMK, a draft such as MO(0) + B without an output key could leave Entropy blank and unresponsive, while the app could still report that combos were saved.

Fix

• Keep incomplete combo drafts local until they contain 2-4 input keys and an output key; clearing a combo remains a valid deletion.
• Write only changed combo slots on a background HID task, so slow or faulty firmware cannot block egui's update loop.
• Read the slot back after every write and show success only for an exact match. Missing devices, mismatches, task failure, and disconnects keep the edit pending and surface an error.
• Preserve pending edits across in-flight writes and coordinate combo I/O with connection, device-switch, layer-write, and undo lifecycle.

This contains the failure on the Entropy side; it does not replace the upstream RMK firmware fix.

Verification

• cargo test: 198 passed.
• cargo test combo_: 9 passed.
• cargo clippy --all-targets: passed with existing repository warnings.
• cargo build --release: passed on macOS.
• python3 scripts/check_i18n.py: passed.
• Scoped rustfmt --check and git diff --check: passed.
• Physical K:04/RMK validation was not available locally.

Post-Deploy monitoring & validation

• Validate on K:04 with MO(0) + B: incomplete draft must stay editable and show "not saved"; assigning an output must keep UI responsive and finish with verified success.
• Reconnect and confirm saved combo matches firmware state. Also test clearing the slot and unplugging during a write.
• Inspect desktop logs for combo writeback mismatch, Combo write error, or background save task stopped. Repeated errors, a blank UI, or a success message without matching readback are rollback signals.
• Validation window: next 0.2.x prerelease and first 24 hours after release. Owner: Entropy maintainers. Rollback: revert this commit or ship previous Entropy build; retain upstream RMK firmware correction separately.

RU

Проблема

https://t.me/c/1464748383/37027/131783

При редактировании комбо Entropy мог сразу отправить в прошивку незаполненное состояние. На K:04/RMK черновик вроде MO(0) + B без выходной клавиши мог оставить пустое зависшее окно, хотя приложение сообщало об успешном сохранении.

Исправление

• Незавершенный черновик остается локальным, пока не выбраны 2-4 входные клавиши и выходная клавиша; очистка комбо по-прежнему удаляет слот.
• В фоне записывается только измененный слот, поэтому медленный или ошибочный ответ прошивки не блокирует UI.
• После записи слот читается обратно. Успех показывается только при точном совпадении; отсутствие устройства, mismatch, остановка задачи и disconnect сохраняют pending-состояние и показывают ошибку.
• Pending-изменения сохраняются при параллельном редактировании, а combo I/O согласован с reconnect, переключением устройства, layer write и undo.

Это ограничивает сбой на стороне Entropy, но не заменяет upstream-исправление прошивки RMK.

Проверка

• cargo test: прошли 198 тестов.
• cargo test combo_: прошли 9 тестов.
• cargo clippy --all-targets: прошёл с существующими предупреждениями репозитория.
• cargo build --release: прошёл на macOS.
• python3 scripts/check_i18n.py: прошёл.
• Scoped rustfmt --check и git diff --check: прошли.
• Физическая проверка на K:04/RMK локально не выполнялась.

Проверка после билда

• На K:04 создать MO(0) + B: без output черновик должен остаться доступным для редактирования и показать, что он не сохранен; после выбора output UI должен оставаться отзывчивым, а успех появиться только после readback.
• После reconnect проверить сохраненное комбо, очистку слота и отключение клавиатуры во время записи.
• В логах проверить combo writeback mismatch, Combo write error и background save task stopped. Повторяющиеся ошибки, пустое окно или ложный успех требуют rollback.

[kissetfall]

The new background combo writer is currently bypassed by the existing synchronous combo-save block in src/ui/app_lifecycle.rs.

That block still runs first whenever combo_dirty is set and the picker closes. It calls set_combo for every slot, including incomplete drafts, on the UI thread. If those writes succeed it clears combo_dirty, so maybe_start_combo_write never runs. If a write fails, the old block retries synchronously on every frame before the new planner can protect the draft. This preserves both failure modes the PR is intended to fix.

Please make the new lifecycle the single owner of combo device writes:

• remove or replace the old synchronous combo-save path;
• ensure incomplete drafts never reach HID;
• ensure a valid edit is written only through the background writer and only for the changed slot;
• add a lifecycle-level regression test covering both cases. The current planner/helper tests do not exercise the old app_lifecycle path and therefore cannot catch this integration bug.

[IgorArkhipov]

Addressed in 986fadc.

Removed synchronous combo writes from app_lifecycle; background writer is now sole HID path.
Added lifecycle coverage for incomplete drafts and changed-slot-only writes.
Cargo test passes locally (218 tests).

[kissetfall]

Thanks — the original blocker is fixed correctly in 986fadc: the synchronous combo loop is gone, the background writer is now the only combo HID path, incomplete drafts stay off the write path, and only the changed valid slot is scheduled.

There is still a blocking HID-ownership race in the full application lifecycle. maybe_start_combo_write takes self.hid_device for the worker, but the rest of the settings UI remains interactive and the synchronous writers in app_lifecycle.rs continue running on following frames:

• macro save sees no handle and clears macros_dirty, losing the pending change;
• Combo Timeout leaves term_save_ok = true when the handle is absent, clears combo_term_dirty, and reports “Saved” without a device write;
• Tap Dance clears tap_dance_dirty after the unavailable-handle error;
• due Tap Hold numeric writes are removed from pending_tap_hold_numeric_writes before discovering that the handle is unavailable;
• deferred imports can also be processed before the hid_write_task_active scan gate.

A combo HID request can take multiple frames or wait through transport retries, so this is reachable whenever the user edits another setting while the combo worker owns the handle.

Please make HID ownership a shared lifecycle contract: while any task owns the handle, all other HID writers/imports must defer without clearing dirty/pending state (or route through one serialized queue). Once the handle is returned, each deferred operation should execute exactly once. The UI may remain responsive, but writes must not be consumed as failures merely because another in-process owner temporarily holds HID.

Please add lifecycle regression coverage that starts an in-flight combo task, marks Macro / Combo Timeout / Tap Dance / Tap Hold dirty or pending, advances the application lifecycle, and proves:

1. no competing writer runs while the combo task owns HID;
2. no dirty/pending state is cleared and no false “Saved” status is emitted;
3. after the handle returns, each operation is written exactly once.

The current tests validate the pure combo plan, but they do not exercise this cross-writer ownership path.

[IgorArkhipov]

Added a shared HID lifecycle gate so imports and Macro, Combo Timeout, Tap Dance, and Tap Hold writes defer while a background task owns the handle, preserving pending state and preventing false “Saved” statuses.

Added regression coverage too for deferral and exactly-once processing after handle return.

[kissetfall]

Thanks — 4cab506 fixes the previously reported Macro, Combo Timeout, Tap Dance, Tap Hold, and pending-import cases, but the shared HID lifecycle is still incomplete.

Blocking issues:

1. The gate is only applied inside app_lifecycle.rs. Other interactive HID writers remain enabled while combo_write_task owns the handle. For example, Magic toggles update local bits before write_magic_settings returns on hid_device == None; Touchpad controls likewise update local state and skip the firmware write. No dirty or pending value is retained, so the UI and firmware diverge. Please serialize or disable every HID-mutating action through one owning abstraction, and preserve pending values until the handle returns.

2. refresh_current_device_data still checks only layer_write_task. During combo_write_task it clears the current device cache, then start_connect rejects the reconnect through hid_write_task_active and returns silently. Check or disable the full HID lifecycle before deleting cache.

3. in_flight_combo_task_defers_other_hid_writes_until_handle_returns exercises a detached HidLifecycleProbe rather than the production EntropyApp lifecycle. It would still pass if a production guard were removed or miswired. Please add regression coverage through the actual lifecycle path for a competing combo task, a real settings writer, and Refresh Device Data, proving there is no UI/firmware divergence, no cache-only deletion, and exactly-once processing after the handle returns.

Please keep the current fixes, but the PR should remain blocked until the complete gate and production lifecycle coverage are in place.

[IgorArkhipov]

Thank you!
Combo-save ownership now blocks interactive HID mutations, retains picker input until task completion, and prevents Refresh Device Data from clearing cache while any HID write task is active.
Also replaced detached lifecycle probe with EntropyApp coverage for deferred settings input, refresh blocking, and exactly-once post-completion application.

[kissetfall]

Thanks — 65606ec fixes the previously reported interactive-settings and Refresh Device Data blockers: an active combo write now gates the main settings UI, defers picker results, and prevents cache deletion.

Two blocking lifecycle issues remain:

1. The exit path bypasses the shared HID gate. on_exit unconditionally flushes pending Tap Hold numeric writes and applies the Entropy display-preset fallback while the combo worker may still own the HID handle. The Tap Hold flush removes values from the pending map before discovering that no handle is available, so they are lost. The display fallback updates local state and clears host bridges, but cannot send the layout-option fallback to the keyboard. Closing Entropy during a combo write can therefore lose a pending setting and leave a host-dependent display preset active. Please delay/coordinate application exit until the worker has returned the handle, then perform these exit writes without consuming pending state early.

2. combo_task_defers_settings_changes_and_refresh_until_handle_returns fabricates a successful ComboWriteResult with hid_device set to None. That state cannot come from the production worker: a successful set_combo returns the handle, while None is produced only after a disconnect error. After the fabricated result, apply_picker_results changes the local Key Override, but write_key_override sees no HID handle, so the test proves neither a firmware write nor exactly-once processing.

Please replace this with production-lifecycle coverage that returns the handle through the real result path and observes a real/fake HID writer: no competing write while combo owns HID, the deferred setting is written exactly once after handle return, Refresh Device Data does not delete cache, and closing during an active task preserves and flushes pending exit writes safely.

The PR should remain blocked until the exit lifecycle and executable exactly-once coverage are fixed.

[IgorArkhipov]

Confirmed application exits now wait for the active combo HID task, then flush pending tap-hold writes and display fallback before closing. Tray minimization remains unchanged.

Also added executable EntropyApp lifecycle coverage with recorded HID requests, proving no competing key-override or tap-hold write runs while combo owns the handle, Refresh Device Data is deferred, and each deferred write executes once after return.

[kissetfall]

Thanks — the previous fake-handle test has been replaced with a real test HID backend, the handle-return path is now exercised, and failed Tap Hold flushes preserve their pending values.

One exit-path blocker remains:

handle_close_to_tray() only calls defer_exit_until_hid_write_returns() for tray quit and force_close_requested. An ordinary native window close falls through for Close, and also for the default Ask behavior when the text expander is disabled. If the combo worker still owns HID, eframe proceeds to on_exit(), which intentionally skips the pending Tap Hold and display-fallback writes while a HID task is active; the application then exits and those writes are lost. This is the normal window-close path on Linux.

The new lifecycle test does not catch this because it calls defer_exit_until_hid_write_returns() directly instead of exercising close_requested() through handle_close_to_tray().

Please route every real application-close path through the HID-write deferral and add a regression test using the normal close lifecycle. It should prove that close is cancelled while the worker owns HID, the returned handle is used for the pending Tap Hold and display-fallback writes exactly once, and only then is the final close issued.

[IgorArkhipov]

Addressed:

• a92eba2 defers normal window close while a Combo write owns HID, matching tray-quit behavior.
• 6a72ae8 adds regression coverage using an actual ViewportEvent::Close: it verifies CancelClose, no Key Override, Tap Hold, or display-fallback write while HID is busy, then exactly one of each after Combo completion before final close.

[kissetfall]

Re-reviewed head 537f278 against current main after #91. The normal ViewportEvent::Close path now cancels close while Combo owns HID, chains all pending Combo writes before exit, returns the real handle, and flushes Key Override, Tap Hold, and display fallback exactly once before final close. The production-lifecycle regression exercises that path. Local validation passed: 238/238 tests, Clippy, i18n, scoped rustfmt, diff-check, and release build. CI is 4/4 green.

[kissetfall]

Follow-up found during the cross-review of #88 and #89 after this PR was merged: the deferred-exit drain still closes before all pending settings writers run.

In the production update order, poll_combo_write and maybe_start_combo_write are followed immediately by finish_deferred_exit_after_hid_write. Once the Combo handle returns, that function flushes only pending Tap Hold numeric writes and the display fallback, then sends ViewportCommand::Close. Dirty Macro, Combo Timeout, Tap Dance, and a pending Key Override picker result are processed later and can therefore be lost on close.

The regression test does not reproduce this order: it calls apply_picker_results manually before finish_deferred_exit_after_hid_write, so Key Override passes even though the normal frame closes first; it does not assert the other dirty writers.

This needs a follow-up shared exit drain that waits until every queued HID mutation is written exactly once before Close, with a regression test using the real update/ViewportEvent::Close path. Until that lands, avoid treating the merged exit lifecycle as release-safe.