Skip to content

fix(mobile): sent messages no longer stay pinned below newer history - #18081

Closed
saxyguy81 wants to merge 1 commit into
pingdotgg:mainfrom
saxyguy81:fix/mobile-ack-window-pin
Closed

saxyguy81 wants to merge 1 commit into
pingdotgg:mainfrom
saxyguy81:fix/mobile-ack-window-pin

Conversation

@saxyguy81

Copy link
Copy Markdown

Problem

On mobile, a sent message can stay at the bottom of a thread below newer messages until the app restarts. This happens when the thread's loaded history window no longer contains the message's echo. Reproduction and cause: #17940.

Change

settledAcknowledgedMessageIds (new, in acknowledged-thread-messages.ts) decides which retained acknowledged messages to drop for the selected thread:

  • its echo is in the loaded messages (the existing rule), or
  • the thread has more history (history.hasMoreHistory) and the message is older than the oldest loaded message.

use-thread-composer-state.ts uses it instead of the id-only check. The check now runs only for the selected thread's retained messages. The outbox and the Pending row for unsent messages are unchanged.

Scope and approval

Fixes #17940, which has the full failure analysis. I'm submitting this under the very-small-focused-fix exception (CONTRIBUTING, line 40): it fixes one defect with one pure helper, one hook change and a test. There are no contract, server, web or desktop changes.

Verification

  • New apps/mobile/src/state/acknowledged-thread-messages.test.ts covers four cases: echo loaded, message older than a bounded window (the bug), a fully loaded thread without the echo (kept), and a message inside the window (kept).
  • thread-task-permissions.test.ts mocks ./use-thread-detail; I added useSelectedThreadDetailState to that mock because the hook now reads it.
  • vp test run on apps/mobile/src/state: 33 files, 351 tests passed. tsc --noEmit for apps/mobile passes. vp lint on the changed files reports no new warnings.
  • Not checked: I have not run this build on a device or simulator, so there are no before/after screenshots. The bug was seen on App Store 1.4.1.

Claude Opus 5.5 via Claude Code

…tory

A sent message stays in the acknowledged list, appended below the feed,
until the selected thread's loaded messages contain its echo. A bounded
history loads only the newest window, so on a busy thread reopened after
more than one window the echo is in unloaded history and the copy stays
pinned at the bottom until the app restarts.

Treat an acknowledged message as settled when the thread has more history
and the message predates the oldest loaded message.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 11, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 01dcd54

Macroscope's review found this PR approvable — This is a small, localized mobile bug fix that adjusts existing acknowledged-message cleanup and adds focused coverage for bounded history cases. It does not introduce a new capability, alter contracts or defaults, or affect production infrastructure.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d18e655d-f897-4782-89e8-62e3330272f6

📥 Commits

Reviewing files that changed from the base of the PR and between 5ce5a8f and 01dcd54.


📒 Files selected for processing (4)
  • apps/mobile/src/state/acknowledged-thread-messages.test.ts
  • apps/mobile/src/state/acknowledged-thread-messages.ts
  • apps/mobile/src/state/thread-task-permissions.test.ts
  • apps/mobile/src/state/use-thread-composer-state.ts

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



📝 Walkthrough

Walkthrough

The mobile composer now clears acknowledged messages when their echoes appear in loaded history or when they predate the loaded window and more history exists. Tests cover these cases and cases where the acknowledgment remains unsettled.

Changes

Acknowledged message settlement

Layer / File(s) Summary
Settlement rule and test cases
apps/mobile/src/state/acknowledged-thread-messages.ts, apps/mobile/src/state/acknowledged-thread-messages.test.ts
The helper settles an acknowledgment when its ID is loaded, or when its timestamp is earlier than the oldest loaded message and more history exists. Tests cover both settlement conditions and two conditions where the acknowledgment remains unsettled.
Composer cleanup integration
apps/mobile/src/state/use-thread-composer-state.ts, apps/mobile/src/state/thread-task-permissions.test.ts
The cleanup effect uses loaded messages, their timestamps, and history availability to identify settled acknowledgments. The thread-detail test mock now provides history metadata.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge


Merge Risk | ⚪ Minimal · up to 01dcd

Merge Risk: ⚪ Minimal · up to 01dcd

Acknowledged messages now settle once when their echo is outside the loaded history window. The change is narrow and tested, with no identified merge-blocking risk. Device behavior was not tested, which is normal for a change like this.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 01dcd

The change is confined to mobile message-display cleanup. It does not change sending permissions or durable delivery. No introduced security concern was established, but timestamp assumptions across delayed delivery remain partly verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The newly introduced timestamp-based outcome is retirement of local acknowledged display rows for the selected thread. The inspected consumers do not connect this atom to delivery authority, durable message deletion, or retry decisions.

Trust Boundaries and Controls

  • observed — Loaded history and retained acknowledgment timestamps influence display cleanup, not an authorization decision. Timestamp-only cleanup additionally requires explicit more-history metadata and a nonempty loaded window.

Resilience and Maintainability Implications

  • observed — Failed durable cleanup forgets the transient acknowledgment while delivery recovery uses a separate acknowledged-ID set to retry outbox removal. The new history effect does not mutate that recovery state, preserving the existing separation between display cleanup and delivery failure containment.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the mobile bug fix: sent messages no longer remain pinned below newer history.
Description check Passed The description includes the problem, change, scope and approval rationale, verification results, test limitations, and agent details. It clearly explains the issue and the focused fix.
Linked Issues check Passed Issue #17940 requires mobile cleanup of an acknowledged message when its echo is loaded or when bounded history has more entries and the message is older than the oldest loaded message. `settledAcknow…
Out of Scope Changes check Passed The changes are limited to the new settlement helper, its focused unit tests, the selected-thread composer cleanup, and the required test mock update. These changes directly support issue #17940. No u…

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

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

@saxyguy81

Copy link
Copy Markdown
Author

PR babysit: blocked: CI workflows need maintainer approval
Head: 01dcd54. Rounds: 0/3.
Passed: Macroscope Correctness and Approvability, CodeRabbit (no actionable comments), PR Vouch, PR Size.
Not run: CI, Mobile Fingerprint Check, Mobile EAS Preview and Forward to Cursor hygiene are action_required. They're waiting for a maintainer to approve workflow runs from a first-time contributor.
Local verification on main @ 9afde1c: vp test run on apps/mobile/src/state, 33 files, 351 tests passed; tsc --noEmit for apps/mobile passes.
Review comments: none to address.

Copy link
Copy Markdown
Member

Note

This comment is posted by Julius' dot

This changes which sent messages remain visible in the mobile thread, but the verification section says the build was not run on a device or simulator and has no before/after screenshots. The verification rule requires UI evidence. Closing for now: reproduce the bounded-history case in the app, attach before/after screenshots and a short recording of reopening the thread, then request reconsideration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Mobile: a sent message stays pinned at the bottom of a thread when its echo is outside the loaded history window

2 participants