fix(chat): keep notifications when fetching new messages while paused

[ToteMeiSter]

Symptom

A chat notification disappears right after it is shown while the conversation is still open in the back stack (app in the background, or another screen on top). In a log from one device (Huawei DEL-LX9, high performance backend), a chat notification was removed 0.45 s after it was shown: the server sent a delete push, and GET notifications/<id> in NotificationWorker returned 404.

Cause

• OfflineFirstChatRepository.fetchNewMessages() calls ChatMessageSyncer.tryCloseBacklog() without markNotificationsAsRead, so the default true applies and the request carries no markNotificationsAsRead=0.
• On such a request the server marks all chat notifications of the user in that conversation as read before it looks for new messages, and each removal sends a delete push: ChatManager::waitForNewMessages, parameter in ChatController::receiveMessages.
• fetchNewMessages() also runs while the chat is not on screen: the insurance request loop (every 2 minutes), messages from signaling, and the fetch after an upload or share completes. In the log above, the insurance request started when the process was woken by the push, about 0.1 s before the notification was fetched.
• The background catch-up after a push (ChatMessageSyncer.fetchRoomCatchUp, added in 🔄️ Chat message pre-fetching #6454) already passes markNotificationsAsRead = false; fetchNewMessages() was not covered.

Change

• fetchNewMessages() passes markNotificationsAsRead = !itIsPaused, reusing the existing pause flag of the repository (set by handleOnPause/handleOnResume).
  • Chat on screen: unchanged, the request clears notifications.
  • Coming back to the chat: ChatViewModel.onResume calls handleOnResume() before its fetchNewMessages(), so this fetch still clears notifications, as before.
  • Chat paused: the request sends markNotificationsAsRead=0. Notifications are then cleared when the user reads the chat (setReadMarker, ChatController::setReadMarker) or when the chat is resumed.
• itIsPaused is now @Volatile: it is written on the main thread and read from fetch coroutines.
• Servers without the chat-keep-notifications capability ignore the parameter, so nothing changes for them. The fetch is not gated by the capability because the open chat still needs it.

Not covered: a request that already started before onPause can still clear notifications.

How to test

1. Open a conversation on a server with a high performance backend, then press Home (the chat stays in the back stack).
2. Have another user send a message that triggers a notification in that conversation.
3. Wait for an insurance request (up to 2 minutes; it also fires right away when the process is woken).
4. Before: the notification disappears shortly after it is shown, the server log shows GET .../chat/<token>?...lookIntoFuture=1... without markNotificationsAsRead=0. After: the request carries markNotificationsAsRead=0 and the notification stays.

Unit test: OfflineFirstChatRepositoryTest — fetchNewMessages keeps notifications only while the chat is paused (fails without the change).

Not yet tested on a device after the change.

Related

The insurance requests keep running while the chat is paused; skipping them is a separate PR (#6842). That PR alone does not fix this one: signaling and upload paths still call fetchNewMessages() while paused.
Related by area: #6763 (marks prefetch requests next to markNotificationsAsRead in ChatMessageSyncer), #6370 (read marker set by polling).

🖼️ Screenshots

Not applicable.

🚧 TODO

• None

🏁 Checklist

• ⛑️ Tests (unit and/or integration) are included or not needed
• 🔖 Capability is checked or not needed
• 🔙 Backport requests are created or not needed: /backport to stable-xx.x
• 📅 Milestone is set
• 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

🤖 AI (if applicable)

• The content of this PR was partly or fully generated using AI

The analysis, code, test and this description were prepared with Claude Code (claude-opus-5-5) and reviewed by me. The commit carries an Assisted-by trailer.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 44a07ce2-a1e7-42f2-8d9b-48263f694ad7

---

📥 Commits

Reviewing files that changed from the base of the PR and between acc10f9 and 49d2e1d.

---

📒 Files selected for processing (2)

• app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt
• app/src/test/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepositoryTest.kt

---

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

---

📝 Walkthrough

Walkthrough

itIsPaused is now volatile. fetchNewMessages passes the inverse of the pause state as markNotificationsAsRead during backlog synchronization. A test checks that the request field is omitted before pausing, set to 0 while paused, and omitted after resuming.

---

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 49d2e

Paused fetches now send the keep-notifications field, while resumed fetches retain the prior request behavior. The changed request path and regression assertion are verified, with no actionable merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely describes the main change: preserving chat notifications during paused-state message fetches.
Description check		The description explains the symptom, cause, implementation, testing steps, limitations, related work, and checklist status. It is mostly complete; the unchecked milestone and backport items are accep…

---

---

• Fix all pre-merge checks with AI

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}

[rapterjet2004]

LGTM, thanks for the thorough analysis. Resume order is fine (handleOnResume() runs before the resume fetch), and the new test covers all three states.

One thing to fix before merge: check-ai-trailers fails because Assisted-by: Claude-Code:claude-opus-5-5 doesn't match the allowed agent names. Something like Assisted-by: Claude Code (claude-opus-5-5) should pass.

Optional nit: tryCloseBacklog reads the pause state once, so if the chat pauses between rounds of a large backlog, the later rounds still clear notifications. Rare, and similar to the in-flight gap you already mention; passing a () -> Boolean would close it.

[ToteMeiSter]

Thanks! The trailer is detected: the job log shows Found AI-assistant/Assisted-by trailer(s): with this line. The job fails later, at "Label PR as AI assisted": Resource not accessible by integration (addLabelsToLabelable), since the token of a fork PR cannot add labels. The AI assisted label is already set on this PR. I can still change the trailer format if you prefer it.