Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

Inconsistent conflict resolution between production and tests

Open
  • Is something wrong?
#356 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
35/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
sqlite, swift
Domain
databases, distributed-systems, testing-qa

Research direction

Start with SyncEngine.refreshLastKnownServerRecord(_:) and compare how saved records are represented by the mock server. Then read MergeConflictTests, especially clientRecordUpdatedBeforeServerRecord() and serverRecordEditedAfterClientAndProcessedAfterClient(), alongside PR #411. Done means production and test conflict-resolution behavior agree without relying on a polluted last-known server record.

Written by the indexing model from the issue text.

Description

bug
Description

I’ve been trying to wrap my head around the implementation of the built-in “field-wise last edit wins” strategy, and I’m honestly having a hard time. To better understand what’s going on, I started documenting the current behavior in this document. While I still don’t have all the answers, I ran into some unexpected behavior that felt worth reporting, as it might help clarify the intended design.

It traces back to the purpose of the didSet flag in CKRecord.update(with:row:columnNames:parentForeignKey:), which is used during upserts. As I understand it, this logic attempts to restore values from the last-known server record onto an incoming server record. In my mental model, this should be superfluous (and really a no-op), as I was assuming the last-known server record represents the most recent record that was either accepted by or fetched from the server. In the three-way merge terms, this would be the ancestor.

However, while digging deeper, I found two spots in the codebase where a record is saved as the last-known server record before it has been accepted by the server. I went into more detail on these in the warning sections of the linked document. In particular, the premature save during send looked worrying to me, as it seems to break the mental model of the last-known server record.

When testing this with the sample apps, I noticed that the record was not actually being saved during send. Inspecting SyncEngine.refreshLastKnownServerRecord(_:) revealed that this was due to the check of the modificationDate between the saved last-known server record and the given record, which happened to be equal, as the given record was based off of the last-known server record.

In tests, however, the record is being saved. The reason appears to be that the mock server does not populate modificationDate on records it reports back to the sync engine. While CKRecord.modificationDate can’t be set directly, I was able to swizzle it and have the mock server simulate a save timestamp. With that change in place, behavior became closer to production but two MergeConflictTests started producing different results, as they had previously been operating on a polluted last-known server record:

  • clientRecordUpdatedBeforeServerRecord(): the final assertion changed from isCompleted = 1 @ t=30 to isCompleted = 1 @ t=60
  • serverRecordEditedAfterClientAndProcessedAfterClient() (added in #353): the title changed from “Buy milk” to “Get milk”, which is incorrect, but at least aligned with the other test preferring local edits regardless of timestamps (as described in #354)

I might be wrong, but I don’t think the reconciliation from the last-known server record that doesn’t reflect the actual server state is an intended behavior specifically built for tests. Having different results between the production and test environment feels wrong, even though the broader question of the desired semantics still stands.

The most straightforward way to align both environments seems to be to have the mock server populate modificationDate on “saved” records. From there, we could iterate further. Is this how things are meant to work? Do you want me to submit a PR that allows the mock server to set the modification date? PR #411 submitted.

Checklist
  • I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
  • I have determined whether this bug is also reproducible in a vanilla GRDB project.
  • If possible, I've reproduced the issue using the main branch of this package.
  • This issue hasn't been addressed in an existing GitHub issue or discussion.
SQLiteData version information

1.4.1

Sharing version information

2.7.4

GRDB version information

7.8.0

Destination operating system

No response

Xcode version information

Version 26.2 (17C52)

Swift Compiler version information
swift-driver version: 1.127.14.1 Apple Swift version 6.2.3 (swiftlang-6.2.3.3.21 clang-1700.6.3.2)
Target: arm64-apple-macosx15.0
Dominant language
Swift
Stars
1.9k
Forks
149
Avg merge
2d 18h
Merged PRs (30d)
1

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from pointfreeco/sqlite-data

All issues in pointfreeco/sqlite-data

Similar issues

More Swift issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.