diff --git a/lib/tdeck_ui/UI/LXMF/ChatScreen.cpp b/lib/tdeck_ui/UI/LXMF/ChatScreen.cpp index a72b2905..ad0ecf53 100644 --- a/lib/tdeck_ui/UI/LXMF/ChatScreen.cpp +++ b/lib/tdeck_ui/UI/LXMF/ChatScreen.cpp @@ -768,32 +768,42 @@ void ChatScreen::on_send_clicked(lv_event_t* event) { String message(text); if (message.length() > 0 && screen->_send_message_callback) { - // Publish to the main loop. On acceptance, record the exact composer - // state that was submitted; the composer is cleared only after - // persistence and queue admission succeed (clear_composer() from - // UIManager::apply_outbound_result) AND only if the composer still - // holds that same text. A rejected send (busy router, full queue, - // storage error) keeps the text for a normal re-send, and anything - // typed into the composer after submission is never wiped by the - // later completion commit. - if (screen->_send_message_callback(message)) { - screen->_pending_submitted_text = message.c_str(); - } + // Publish to the main loop. On acceptance the callback records the + // exact composer state that was submitted (inside the callback, so + // the marker is set under this same LVGL lock section, BEFORE the + // main loop can observe the mailbox entry). The composer is cleared + // only after persistence and queue admission succeed AND only when + // it still holds that same text. A rejected send keeps the text for + // a normal re-send, and anything typed after submission is never + // wiped by the later completion commit. + screen->_send_message_callback(message); } } +void ChatScreen::set_pending_submitted_text(const std::string& text) { + // Recursive lock: the send callback runs on the LVGL task under the + // LVGL lock (on_send_message_from_chat holds it via the click handler's + // event context); this keeps the marker consistent with the composer. + LVGL_LOCK(); + _pending_submitted_text = text; +} + void ChatScreen::clear_composer() { // Recursive lock: apply_outbound_result() calls this while already // holding the LVGL lock. LVGL_LOCK(); - // Only clear if the composer still holds the exact text we submitted. - // Persistence + router admission run on the main loop off the LVGL - // lock, so the user may have started typing the next message before the - // commit lands; wiping an edited composer here would erase fresh input. - // An empty _pending_submitted_text means nothing is pending, in which - // case a clear is a no-op-safe reset. + // Only clear when a submission is pending (non-empty marker) and the + // composer still holds exactly the submitted text. Persistence + router + // admission run on the main loop off the LVGL lock, so the user may + // have started typing the next message before the commit lands; wiping + // an edited composer here would erase fresh input. An empty marker + // (rejected/retained send, or no submission) clears nothing — the + // retained text is kept deliberately for a normal re-send. + if (_pending_submitted_text.empty()) { + return; + } const char* current = lv_textarea_get_text(_text_area); - if (current != nullptr && _pending_submitted_text != current) { + if (current == nullptr || _pending_submitted_text != current) { return; } _pending_submitted_text.clear(); diff --git a/lib/tdeck_ui/UI/LXMF/ChatScreen.h b/lib/tdeck_ui/UI/LXMF/ChatScreen.h index 46b056cb..e245df3c 100644 --- a/lib/tdeck_ui/UI/LXMF/ChatScreen.h +++ b/lib/tdeck_ui/UI/LXMF/ChatScreen.h @@ -155,10 +155,24 @@ public: /** * Set callback for sending messages - * @param callback Function to call when send button is pressed + * @param callback Function that sends the message; return true when the + * send was accepted into the main-loop mailbox. The callback + * (UIManager::on_send_message_from_chat) records the submitted + * text via set_pending_submitted_text() in the same LVGL lock + * section as the publish, so the main loop can never observe + * the mailbox entry before the marker is set. */ void set_send_message_callback(SendMessageCallback callback); + /** + * Record the composer text that was just published to the main-loop + * send mailbox. Must be called by the send callback in the same LVGL + * lock section as the publish so the completion commit (main loop) can + * match its clear to the exact submission. See ChatScreen.h field + * _pending_submitted_text. + */ + void set_pending_submitted_text(const std::string& text); + /** * Set callback for voice call button * @param callback Function to call when call button is pressed @@ -198,9 +212,15 @@ private: std::deque _messages; // Composer text captured when a send was accepted into the main-loop - // mailbox. apply_outbound_result() only clears the composer if it still - // holds exactly this text, so input typed into the composer while - // persistence/admission was in flight is never erased by a later commit. + // mailbox. apply_outbound_result() only clears the composer when a + // non-empty marker matches the current composer text exactly, so input + // typed into the composer while persistence/admission was in flight is + // never erased by a later commit. Empty means "no pending submission", + // which clears nothing (safe for retry/rejected sends, where the text + // is retained deliberately). The marker is assigned by + // UIManager::on_send_message_from_chat() in the same LVGL lock section + // as the mailbox publish, so the main loop can never observe the + // mailbox entry before the marker is set. std::string _pending_submitted_text; // Map message hash to bubble row for targeted updates diff --git a/lib/tdeck_ui/UI/LXMF/UIManager.cpp b/lib/tdeck_ui/UI/LXMF/UIManager.cpp index ae0b07a9..e7d44467 100644 --- a/lib/tdeck_ui/UI/LXMF/UIManager.cpp +++ b/lib/tdeck_ui/UI/LXMF/UIManager.cpp @@ -1431,7 +1431,19 @@ void UIManager::on_back_to_conversation_list() { } bool UIManager::on_send_message_from_chat(const String& content) { - return send_message(_current_peer_hash, content); + const bool accepted = send_message(_current_peer_hash, content); + if (accepted && _chat_screen) { + // Race-critical: set the submitted-text marker in the SAME LVGL lock + // section as the mailbox publish above. If the marker were assigned + // later (on the ChatScreen side, after this callback returned), the + // main loop could take() + admit the send and run clear_composer() + // while the marker was still empty, neither clearing the submitted + // text nor matching the later commit to its submission. This handler + // runs on the LVGL task (click event, lock held), so the marker is + // visible to the main loop only after the mailbox entry is. + _chat_screen->set_pending_submitted_text(content.c_str()); + } + return accepted; } void UIManager::on_call_from_chat() { diff --git a/tests/build_scripts/test_send_draft_and_stale_history_contract.py b/tests/build_scripts/test_send_draft_and_stale_history_contract.py index bb286a99..cdea9447 100644 --- a/tests/build_scripts/test_send_draft_and_stale_history_contract.py +++ b/tests/build_scripts/test_send_draft_and_stale_history_contract.py @@ -51,29 +51,47 @@ def test_clear_composer_is_generation_checked(): "void ChatScreen::clear_composer()", "lv_group_focus_obj(_text_area);", ) - # The clear must compare the current composer text against the - # captured submitted text and bail when they differ. - assert "_pending_submitted_text" in body, ( - "clear_composer() no longer consults the submitted-text generation" + # The clear must bail when no submission is pending (empty marker) and + # compare the current composer text against the captured submitted text. + assert "_pending_submitted_text.empty()" in body, ( + "clear_composer() must no-op when no submission is pending" ) + assert "_pending_submitted_text" in body assert "lv_textarea_get_text(_text_area)" in body assert "return;" in body, ( "clear_composer() must skip the clear when the composer was edited" ) -def test_send_click_records_submitted_text(): +def test_send_path_records_submitted_text_in_lock_section(): source = chat_source() + ui_source = (ROOT / "lib/tdeck_ui/UI/LXMF/UIManager.cpp").read_text() + # The ChatScreen click handler must not assign the marker itself — the + # marker has to be set inside the callback (UIManager), in the same LVGL + # lock section as the mailbox publish, or the main loop can observe the + # mailbox entry before the marker exists (Greptile P1: completion race). body = function_body( source, "void ChatScreen::on_send_clicked(", - "void ChatScreen::clear_composer()", + "void ChatScreen::set_pending_submitted_text(", ) - assert "_pending_submitted_text" in body, ( - "on_send_clicked() must capture the submitted composer text on " - "acceptance so the later completion can identify it" + assert "_pending_submitted_text" not in body, ( + "on_send_clicked() must not assign the marker outside the callback " + "lock section (race with the main-loop completion)" ) assert "_send_message_callback(message)" in body + # The callback-side handler sets the marker right after acceptance, + # under the same LVGL lock the click handler already holds. + ui_body = function_body( + ui_source, + "bool UIManager::on_send_message_from_chat(const String& content)", + "void UIManager::on_call_from_chat()", + ) + assert "set_pending_submitted_text" in ui_body, ( + "on_send_message_from_chat must record the submitted text in the " + "same LVGL lock section as the mailbox publish" + ) + assert "send_message(_current_peer_hash, content)" in ui_body def test_same_peer_reopen_checks_live_message_count():