Skip to content

Conversation

@netbe
Copy link
Collaborator

@netbe netbe commented Dec 2, 2025

BugWPB-22062 [iOS] don't schedule commitPendingProposal during incrementalSync

Issue

Commit pending proposals should never be executed while performing the incrementalSync. Right now, it's scheduled
via a simple Task.

This PR introduces a generator that will create workItems to commitPendingProposals at the end of incrementalSync and while live sync (push channel open) is running.

CommitPendingProsalsGenerator checks conversations with commitPendingProposalDates and generates CommitPendingProposalItem. This only happens after incrementalSync is done in mainApp.

Then the WorkAgent will run the CommitPendingProposalItem.

Testing

Pending proposals are generated when user leaves a conversation, client is deleted.
Try reproduce these cases and check logs for work-item being generated


Checklist

  • Title contains a reference JIRA issue number like [WPB-XXX].
  • Description is filled and free of optional paragraphs.
  • Adds/updates automated tests.

UI accessibility checklist

If your PR includes UI changes, please utilize this checklist:

  • Make sure you use the API for UI elements that support large fonts.
  • All colors are taken from WireDesign.ColorTheme or constructed using WireDesign.BaseColorPalette.
  • New UI elements have Accessibility strings for VoiceOver.

@netbe netbe force-pushed the fix/WPB-22062-commitPendingProposals branch from acec2b8 to 4780bab Compare December 8, 2025 10:24
).post()
}

syncContext.perform { [weak self] in
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed as we don't need to repair out of sync conv after initial sync, it will be done by MLSRepairGroup during incrementalSync before iterating over the live stream

WireLogger.mls.warn("`qualifiedClientID` is missing for selfClient")
}

if !isRecovering, mlsFeature.isEnabled {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is now replaced by any item generated by the CommitPendingProposalGenerator

@netbe netbe marked this pull request as ready for review December 9, 2025 15:59
@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2025

Test Results

  1 files  131 suites   16s ⏱️
469 tests 468 ✅ 0 💤 1 ❌
470 runs  470 ✅ 0 💤 0 ❌

For more details on these failures, see this check.

Results for commit 21bff1e.

♻️ This comment has been updated with latest results.

@netbe netbe changed the base branch from develop to release/cycle-4.12 December 9, 2025 20:57
@netbe netbe force-pushed the fix/WPB-22062-commitPendingProposals branch 2 times, most recently from 7d1e8a8 to 806c644 Compare December 9, 2025 21:00
@netbe netbe force-pushed the fix/WPB-22062-commitPendingProposals branch from 806c644 to 36c5e90 Compare December 9, 2025 21:01
// otherwise assume that things will sort themselves out through conversation reset.
let feature = await featureRepository.fetchAllowedGlobalOperations()
if feature.status == .disabled || feature.config.mlsConversationReset == false {
// check if this can be removed
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: why do you suspect it can be removed?

}

public func stop() {
fetchedResultsController.delegate = nil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: should we tear down the FRC instead like in the other generator?


let timeIntervalSinceNow = timestamp.timeIntervalSinceNow
if timeIntervalSinceNow > 0 {
try await Task.sleep(for: .seconds(timeIntervalSinceNow))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: this will block the sync agent right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WorkAgent you mean?


// Ensure live does NOT start on this state
live.start_MockMethod = {
Issue.record("Live generator should not start during incrementalSyncing(.createPushChannel)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: what is Issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a type from SwiftTesting you can basically create your error

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants