Skip to content
This repository was archived by the owner on Dec 10, 2025. It is now read-only.

Conversation

Copy link
Contributor

Copilot AI commented Nov 8, 2025

Client-side SyncRegistryImpl was only sending SyncValue changes to the server, not SyncSet delta changes. Server-side already implements both directions correctly.

Changes

  • Added afterChange override for SyncSet in client SyncRegistryImpl
    • Calls super.afterChange() for validation
    • Sends SyncSetDeltaPacket via fireAndForget()
    • Mirrors existing SyncValue pattern (simple send, no deduplication)
override fun <T> afterChange(
    syncSet: SyncSetImpl<T>,
    added: Boolean,
    changeId: Long,
    element: T
) {
    super.afterChange(syncSet, added, changeId, element)
    SyncSetDeltaPacket(syncSet, added, changeId, element).fireAndForget()
}

Server-side handleSyncSetDeltaPacket already handles validation and broadcast of incoming client changes.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • repo.slne.dev
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -Xmx4G -Dfile.encoding=UTF-8 -Duser.country -Duser.language=en -Duser.variant -cp /home/REDACTED/.gradle/wrapper/dists/gradle-8.14.3-bin/cv11ve7ro1n3o1j4so8xd9n66/gradle-8.14.3/lib/gradle-daemon-main-8.14.3.jar -javaagent:/home/REDACTED/.gradle/wrapper/dists/gradle-8.14.3-bin/cv11ve7ro1n3o1j4so8xd9n66/gradle-8.14.3/lib/agents/gradle-instrumentation-agent-8.14.3.jar org.gradle.launcher.daemon.bootstrap.GradleDaemon 8.14.3 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

Fixe dieses problem Hä ja ich bin auch dumm xD ich hab das für Sets irgendwie nur von Server zu Client implementiert https://github.com/SLNE-Development/surf-cloud/blob/master/surf-cloud-standalone/src/main/kotlin/dev/slne/surf/cloud/standalone/sync/SyncRegistryImpl.kt#L28
und für Client zu Server habe ich das nur für die SyncValue gemacht https://github.com/SLNE-Development/surf-cloud/blob/master/surf-cloud-core/surf-cloud-core-client/src/main/kotlin/dev/slne/surf/cloud/core/client/sync/SyncRegistryImpl.kt#L20
Du kannst das aber fixen wenn du magst — du musst nur auf dem Client die Methode wie im Server überschreiben, dann die super von der Methode wie im Server callen und das packet schicken. Die Logik ist halt schon für beide Seiten implementiert nur irgendwie hab ich das da vergessen


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Nov 8, 2025
Co-authored-by: twisti-dev <76837088+twisti-dev@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix client-server sync implementation for Sets Add missing SyncSet client-to-server synchronization Nov 8, 2025
Copilot AI requested a review from twisti-dev November 8, 2025 09:49
@twisti-dev twisti-dev marked this pull request as ready for review November 8, 2025 10:02
Copilot AI review requested due to automatic review settings November 8, 2025 10:02
@twisti-dev twisti-dev merged commit aeac183 into master Nov 8, 2025
@github-project-automation github-project-automation bot moved this from Backlog to Done in surf-cloud Nov 8, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for synchronizing SyncSet changes (additions/deletions) from client to server by implementing the afterChange method for SyncSet deltas in the client-side SyncRegistryImpl. This ensures bidirectional synchronization of SyncSet operations across the cloud network.

  • Implements the afterChange method override for SyncSet changes in the client implementation
  • Sends SyncSetDeltaPacket to the server when elements are added or removed from a SyncSet on the client side
  • Maintains consistency with the existing SyncValue synchronization pattern already present in the codebase

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

override fun <T> afterChange(
syncSet: SyncSetImpl<T>,
added: Boolean,
changeId: Long,
element: T
) {
super.afterChange(syncSet, added, changeId, element)
SyncSetDeltaPacket(syncSet, added, changeId, element).fireAndForget()

P1 Badge Avoid treating client change IDs as globally monotonic

The new afterChange override forwards the local changeId to the server without any transformation or namespace. On the server side, SyncRegistryImpl.handleSyncSetDeltaPacket keeps a single lastChangeIds entry per set and drops any packet whose changeId is not strictly increasing. Because each client creates its own SyncSetImpl with an AtomicLong that starts at 0, two different clients will both send changeId = 1 on their first modification. After the first client update, the server records 1 and will reject the second client’s first update as stale, causing all subsequent changes from that client to be ignored. Multi-client set synchronization will therefore fail after this change. The server needs a per-client sequence, a server-issued change ID, or to relax the stale check for inbound client deltas.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants