Skip to content

Two devices push to each other forever when a save returns to an earlier state (since #42) #71

Open

Description

@ChakraFusion

Summary: since #42, two devices can end up both pushing to each other forever, so a change never arrives. This is what makes TestReportedWAN_AlternatingEditsMustNotConflict fail on Windows CI ("round N: B's edit never reached A"): about 1 in 15 runs on my fork, both on dev-public and with #51. It also failed on dev-public in run #337 and on #55 in run #347.

What happens (from the daemons' logs, captured with #57):

  1. A save returns to bytes identical to an earlier state. The test writes the same "a-turn" / "b-turn" every round; in real use, a setting toggled back can do the same.
  2. One device's base is still a round behind, for example while the report of the other's pull hasn't been processed. A's base is then the previous "b-turn".
  3. B writes "b-turn" again, which hashes identical to A's stale base. With fix(sync): prefer agreed base over misleading file mtimes #42's rule (remoteHash == baseHash && localHash != baseHash → push), A concludes B hasn't changed and pushes. B correctly sees itself as newer and pushes too.
  4. Neither pulls, so nothing moves A's base, and they trigger each other several times a second until the test gives up:
B: local has newer content; triggering "WanAlternate-A" to pull
A: WAN API request: GET /sync/trigger/wanalternate from B
A: local has newer content; triggering "WanAlternate-B" to pull
B: WAN API request: GET /sync/trigger/wanalternate from A
... (repeating)

Deterministic reproduction of the decision (ComputeWithBase, one file slot1.sav, the lineage holding it):

  • A: local "a-turn", remote "b-turn" (newer mtime), base = hash of the earlier "b-turn" → push
  • B: local "b-turn", remote "a-turn", base = hash of "a-turn" → push

With #42 both push; on the commit before #42, A pulls and B pushes, which is correct. I can add this as a test with the fix.

The rule itself is sound: a side whose whole manifest equals the agreed base hasn't changed, except when the same bytes come back after the base was agreed (ABA). Hashes can't tell the two apart, and mtimes can't either: #42 exists precisely because a restored or touched copy of the base has a newer mtime, so a time-based guard would bring that bug back. As I see it, the options are:

This is your design (cc @Seagulls), so I'd rather ask before changing it: which would you prefer? I'm happy to do the second as a PR with the reproduction as its test.

🤖 Generated with Claude Code

Activity

  1. Seagulls commented on Oct 15, 2026

    @Seagulls

    Thanks for investigating this and for isolating the ABA case. The deterministic reproduction is particularly useful.

    My preference would be option 2 as the immediate fix: detect reciprocal push cycles and stop safely rather than weaken the agreed-base precedence introduced in #42.

    I agree that #51 can narrow the stale-base window but cannot eliminate the underlying ambiguity. Returning to timestamp-based decisions risks reintroducing the original rollback problem.

    A few things I'd suggest for the implementation:

    • Detect repeated reciprocal push decisions for the same peer/game pair when neither side's content state has changed or convergence has been confirmed.
    • Stop further reciprocal triggers and preserve both versions rather than guessing which is newer.
    • Surface a conflict or reconciliation-required state so the situation is visible and recoverable.
    • Reset the detection when either side genuinely changes or successful convergence is established.

    I'd also retain the original #42 rollback tests alongside your deterministic ABA regression, to ensure we don't fix the loop by reintroducing silent overwrites.

    Longer term, I think #29's version-vector direction is worth pursuing. Causal version information would address this ambiguity more fundamentally, but that's a considerably larger protocol change and shouldn't block a focused safety fix.

    If the maintainers agree, I'd welcome your proposed PR implementing option 2 with the reproduction test. Happy to help review the implementation and its edge cases.

  2. ChakraFusion commented on Oct 15, 2026

    @ChakraFusion
    Author

    Thanks, that's a clear plan, and I agree on all four points. I'll prepare option 2 that way: detect the reciprocal pushes for a game and peer when neither side's content has changed, stop triggering and keep both versions, surface it as a conflict, and reset once either side really changes or the two agree again. #42's rollback tests stay next to the ABA reproduction. @Omar-VF, if that's fine with you, I'll open it against dev-public.

  3. Omar-VF commented on Oct 16, 2026

    @Omar-VF
    Collaborator

    Agreed, please go ahead with option 2 as you and Seagulls laid out, against dev-public: stop the reciprocal pushes when neither side changed, keep both versions, surface it as a conflict, reset on a real change or agreement, and keep #42's rollback tests alongside the ABA reproduction. Thanks to you both for pinning this down.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions