Skip to content

refactor(mobile): thread outbox reuses the shared serialized queue - #28743

Open
only21mil wants to merge 1 commit into
pingdotgg:mainfrom
only21mil:fix/audit-m162
Open

only21mil wants to merge 1 commit into
pingdotgg:mainfrom
only21mil:fix/audit-m162

Conversation

@only21mil

Copy link
Copy Markdown
Contributor

Problem

The mobile thread outbox keeps its own copy of a promise-tail serializer in thread-outbox-manager.ts. SerializedAsyncQueue in apps/mobile/src/lib/serialized-async-queue.ts already does the same thing: it runs work in order, and a failed operation does not block the next one. Composer drafts, the PR review draft and the share inbox already use it. The outbox is the last hand-rolled copy of this pattern among those queues.

Change

The outbox manager now creates one SerializedAsyncQueue per manager, and serialize calls its run. The deleted code had the same body as run, so ordering, rejection handling and timing do not change. serialize stays in the manager's API, because flushThreadOutbox and the outbox tests call it. Enqueue rollback, load recovery and environment clears are untouched.

This is one file, +3/-9.

Scope and approval

This is a maintenance refactor with no intended behavior change and no screen change, so it needs no prior issue or discussion under CONTRIBUTING. Please confirm that reusing the shared helper here is the direction you want.

Verification

  • vp test run src/state/thread-outbox.test.ts src/state/thread-outbox-removal.test.ts src/state/threadOutboxDelivery.test.ts src/state/threadOutboxDelivery.permissions.test.ts src/lib/serialized-async-queue.test.ts from apps/mobile: 5 files, 81 tests passed.
  • To confirm the outbox tests now guard the shared helper, I broke SerializedAsyncQueue.run temporarily, then reverted it:
    • Running each operation immediately, with no ordering, failed 2 outbox tests, including "holds the mutation queue while persisted messages are loading".
    • Letting a rejected operation block later work failed 8 tests, including "keeps a same-id retry queued when the first attempt's write fails" and "reports structured load failures and permits a retry".
  • vp run check:changed 1d626d02d95f9a2343553f6498037ee82290d304: format, lint, knip, typecheck and react compiler passed.
  • No new test was added. The existing queue and outbox tests already cover failure recovery and ordering through the shared helper.

Not checked:

  • No device run. Nothing on screen changes, and the deleted code was identical to the helper.
  • apps/mobile/src/features/diffs/nativeReviewDiffSource.ts has a similar tail with abort and request-sharing behavior. It is left out to keep this PR to one change.

Prior PRs

Generated with Claude Opus 5.5 and GPT-6.1 Sol in T3 Code Nightly

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 1, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at e0959d4

Macroscope's review found this PR approvable — The mobile thread outbox now reuses the shared queue implementation while preserving operation ordering and recovery after failures. Queue state remains local to each manager, with no changes to storage behavior or product defaults.

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

@coderabbitai

coderabbitai Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 3fd2b85 and e0959d4.

Files selected for processing: 1
  • apps/mobile/src/state/thread-outbox-manager.ts

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

Walkthrough

ThreadOutboxManager now uses SerializedAsyncQueue to serialize mutations. Its serialize wrapper delegates each mutation to run, replacing the manager’s local promise-chain queue.

Changes: Thread outbox mutation serialization

Layer / File(s) Summary
Delegate mutations to SerializedAsyncQueue
apps/mobile/src/state/thread-outbox-manager.ts
ThreadOutboxManager replaces its local promise-chain queue with a SerializedAsyncQueue instance. The serialize wrapper delegates each mutation to run.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Possibly related PRs

  • pingdotgg/t3code#26013: This PR introduced the mobile thread outbox manager and its mutation paths. This change updates that manager’s serialization mechanism.

Suggested reviewers: maria-rcks, juliusmarminge

Merge Risk: Minimal · up to e0959

The serializer refactor preserves outbox mutation behavior, so no actionable merge-blocking risk remains.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: reusing the shared serialized queue for the mobile thread outbox.
Description check Passed The description includes all required sections. It explains the problem, the implementation, scope rationale, verification results, limitations, and agent attribution. The scope section explains why p…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description includes all required sections. It explains the problem, the implementation, scope rationale, verification results, limitations, and agent attribution. The scope section explains why prior approval is not required for this focused maintenance refactor.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

This branch has not been deployed

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

Labels

size:S 10-29 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.

1 participant