feat(share): add Connect Share Fabric mod - #94
Open
robinbraemer wants to merge 50 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Build and ship Connect Share as a Fabric/Kotlin mod where players link once as confirmed friends, see privacy-gated online and singleplayer-world presence, request to join a friend's active singleplayer world over libp2p, notify and approve on the host, and then join direct-first with Connect as the managed relay fallback. Friend requests and presence must use authenticated libp2p identities rather than Connect; pending relationships must not expose presence; reciprocal requests and removals must converge; UI/network operations must not block Minecraft's render thread. Reuse/import a stable Connect endpoint token across worlds, support offline-mode players as Connect does, retain Mojang authentication for explicitly ONLINE direct sessions, use Arrow conventions, support Fabric 1.21.11 and 26.2, cover behavior with TDD, and prove 26.2 with a real two-client Prism E2E. Preserve the reusable Prism procedure as a repo-local agent skill and durable share/AGENTS.md guidance. Open the PR against the repository's correct integration branch and reference epic issue #93.
What Changed
share/AGENTS.mdproject instructions.Risk Assessment
🚨 High: The updated code still permits stale direct invitations, duplicates a libp2p identity across active hosts, and introduces unsynchronized UI lifecycle operations that can break sharing and friend-state behavior.
Testing
Targeted artifact and behavior tests passed, and filtered live logs prove the target mod starts Minecraft 26.2 and hosts a world. The required real Prism friend-presence, approval, Connect-managed join, and usable UI evidence could not be completed because Prism stalled and the safe fallback did not use the Connect P2P harness.
Evidence: Packaged 26.2 artifact evidence
Evidence: Live 26.2 host evidence
Evidence: Live guest launch attempt
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
share/fabric-26.2/src/main/java/com/minekube/connect/share/fabric/v26_2/mixin/IntegratedServerMixin.java:13- Required criterion: “support Fabric 1.21.11 and 26.2.” This redirect targetspublishServer(MultiplayerScope, int), but the 26.2 transport invokes the four-argument(MultiplayerScope, GameType, boolean, int)overload. WithdefaultRequire: 1, the mixin may fail application or leave the actual LAN publish path unredirected. Align the descriptor with the invoked 26.2 signature.share/fabric-common/src/main/kotlin/com/minekube/connect/share/fabric/ui/ShareViewModel.kt:188- Required criterion: “UI/network operations must not block Minecraft’s render thread.”CoroutineStart.UNDISPATCHEDstarts these handlers on the caller thread, while the scope is backed by the Minecraft client dispatcher. Starting, stopping, importing, or resetting can synchronously perform file writes, transport publication, and NettysyncUninterruptibly()calls before the first suspension. Dispatch blocking work off the client thread before entering the operation.share/fabric-common/src/main/kotlin/com/minekube/connect/share/fabric/FriendPresenceMonitor.kt:85- Required criterion: “Friend requests and presence must use authenticated libp2p identities rather than Connect.” After a direct probe fails, this code probesfriend.connectAddressand reportsShareRoute.CONNECT, exposing presence through the Connect hostname instead of the authenticated libp2p friend path. Remove the Connect presence fallback, or keep presence unavailable when no authenticated direct route exists.share/fabric-common/src/main/kotlin/com/minekube/connect/share/fabric/PersistentDirectIngress.kt:70- The title bootstrap starts a persistent direct ingress with fixed options (allowInternetDirect = false), andActive.borrowreuses it without comparing per-world options. The same host identity, capability, and invitation therefore survive world/share changes; internet-direct opt-in is ignored, and an old invitation can continue to target a later world because expiry is encoded in the invitation but not enforced by host admission. Separate persistent friend control from per-world sharing, or reconfigure/rotate and validate the direct share on every world start.share/fabric-common/src/main/kotlin/com/minekube/connect/share/fabric/FabricShareBootstrap.kt:268- Required criterion: “Reuse/import a stable Connect endpoint token across worlds.” The active Connect control plane captures the startup identity, whilePersistentConnectIngressrejects later borrows when the stored identity changes. The UI permits import/reset while active, so the next world start can fail against the still-running old ingress; friend cards and the advertised Connect address also remain stale. Identity changes must restart the active control planes and refresh derived addresses, or be disabled while sharing is active.share/fabric-common/src/main/kotlin/com/minekube/connect/share/fabric/FriendCardIssuer.kt:66-FriendCardIssuerandFabricDirectShareIngresseach construct their ownShareAccessIdentityStore.@Synchronizedonly protects one instance, so on first startup a concurrent control-plane start and friend-card request can generate different share IDs/capabilities and race the atomic file replacement. The card from one instance can then be rejected by the host initialized with the other. Share one store instance through bootstrap or add a cross-instance file lock.🔧 Fix: Fix Connect Share review findings
4 issues (1 error, 3 warnings) still open:
share/fabric-common/src/main/kotlin/com/minekube/connect/share/fabric/PersistentDirectIngress.kt:87- The replacement logic now refreshes options, but old world invitations remain valid. Each new host still reuses the persistent peer key, share ID, and capability, while host admission checks only the ID/capability and never the signed invitation expiry or generation. An invitation from world A can therefore reach world B. Use a per-world capability/generation separate from the stable friend-control identity, or enforce the current invitation at the host boundary.share/fabric-common/src/main/kotlin/com/minekube/connect/share/fabric/FabricDirectPeerRuntime.kt:25- Normal bootstrap now constructs a browserDirectP2pNodefromshare-libp2p-identity.keyand a separate ingress node from the same key; the browser starts discovery before the ingress host starts. This creates two independent libp2p hosts and mDNS services with the same peer ID, while the runtime filters same-peer discovery and advertises the peer ID as the mDNS service name. Discovery and direct dialing can consequently select the non-sharing browser host or collide. Keep one coordinated host/runtime per identity or explicitly separate the identities and routing.share/fabric-common/src/main/kotlin/com/minekube/connect/share/fabric/ui/ShareViewModel.kt:192- Operations now run onoperationDispatcher, butupdatestill performs a non-atomic read/transform/write. The client-thread state collectors and IO operation coroutines can overwrite each other’s fields, losingoperationInProgressor the latestshareState. Also,start()returns before setting the in-progress flag, so repeated clicks or an import racing a world transition can queue concurrent lifecycle operations. Use atomicMutableStateFlow.updateand serialize/gate operations before dispatching their blocking work.share/fabric-common/src/main/kotlin/com/minekube/connect/share/fabric/FabricShareBootstrap.kt:123-FriendRequestServernotifiesonRelationshipChangedafter reciprocal acceptance and incoming acceptance, but bootstrap leaves that callback at its no-op default. A remote acceptance can updateFriendStorewhileFriendsViewModelcontinues showing the outgoing request; if the friend remains offline, the presence monitor produces the same empty map and never triggers a refresh. Wire relationship changes to a view-model/store refresh path so confirmed relationships and removals converge in the UI as well as persistence../gradlew :core:test --tests ... --no-parallel --console=plain./gradlew :share:common:test --no-parallel --console=plain./gradlew :share:fabric-common:test --no-parallel --rerun-tasks --console=plain./gradlew :share:fabric-1-21-11:test --no-parallel --console=plain./gradlew :share:fabric-26-2:test --no-parallel --console=plain./gradlew :share:fabric-common:test --tests '...FriendPairingDirectE2ETest' --no-parallel --console=plain --infoTwo worktree-local Prism 26.2 launch attempts with offline host identity🔧 Fix: Fixed ShareViewModel scheduler setup; focused rerun passes
1 error still open:
share/fabric-26.2/src/main/java/com/minekube/connect/share/fabric/v26_2/mixin/IntegratedServerMixin.java:12- The target Fabric 26.2 artifact crashes during Minecraft startup before reaching the end-user UI. The IntegratedServerMixin redirect fails withCritical injection failureandNo refMap loaded, so the required 26.2 support and two-client Prism E2E cannot proceed. Investigate the packaged Mixin remapping for the redirect inIntegratedServerMixin.java../gradlew :share:fabric-common:test --tests 'com.minekube.connect.share.fabric.ui.ShareViewModelTest' --no-parallel --console=plainFocused core, share-common, fabric-common, Fabric 1.21.11, and Fabric 26.2 tests via targeted./gradlew --no-parallel --console=plain ... --tests ...selectors.prismlauncher --dir "$PWD/.e2e-prism" --launch host --offline e2eHost --world 'New World' --show-windowusing the built target JAR in a disposable local Prism fixture.javap -classpath '.../minecraft-26.2-client.jar' -p -c net.minecraft.client.server.IntegratedServerto verify the runtimepublishServersignature and LAN pinger call.Inspected the captured startup log and crash report; the guest launch was not attempted because the host crashed during Fabric mixin initialization.🔧 Fix: Fix Fabric 26.2 LAN redirect overload; focused tests pass
1 warning still open:
./gradlew :share:fabric-26-2:test --tests '*Fabric262ArtifactTest*' --no-parallel./gradlew :share:fabric-26-2:connectShareJar --no-parallel./gradlew :share:fabric-common:test --tests '*FriendPairingDirectE2ETest*' --tests '*FriendPairingE2ETest*' --tests '*FriendPresenceMonitorTest*' --tests '*FabricDirectAuthenticationPolicyTest*' --tests '*FabricLocalLoginAdmissionGateTest*' --tests '*GuestConnectionLeaseTest*' --tests '*MinecraftStatusProbeTest*' --tests '*FabricShareBootstrapTest*' --tests '*ShareViewModelTest*' --no-parallelManual isolated 26.2 launch with the target SHA-2569fdf0134e7009f492c22689171adb79ec3f629794f70a5f43f4d21dc28ace235; host reachedalice joined the gameand published a LAN port.Manual second-client launch reached Minecraft 26.2 but did not exercise the Connect P2P join path; the disposable fixture was removed afterward.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.