Skip to content

Remove the Donut server integration - #456

Merged
Ekwav merged 1 commit into
Coflnet:mainfrom
ekwav-agent:ekwav-agent/task_egtdlw5zkrnsa3b5uprq
Oct 2, 2026
Merged

Ekwav merged 1 commit into
Coflnet:mainfrom
ekwav-agent:ekwav-agent/task_egtdlw5zkrnsa3b5uprq

Conversation

@ekwav-agent

Copy link
Copy Markdown
Contributor

Summary

I removed the Donut server integration from SkyModCommands. On the base commit the regression test fails with the same error as the production trace; with the patch it passes. The review's only blocking point, the missing regression manifest, is now fixed.

Why: in trace 580d464f255283c1ea07947e09102f2f, /cofl flip failed in SkyBFCS. FlipCommand.Execute calls ModSessionLifesycle.UpdateConnectionTier, which looked up IDonutFlipSubscriptionService on every connection. SkyBFCS never registers that service, so the lookup threw "No service for type … has been registered". The lookup is now gone.

Donut behaviour removed

  • Service: Services/Donut/ is deleted (DonutFlipSubscriptionService, IDonutFlipSubscriptionService, DonutServerContext), along with its registration in Startup.cs.
  • ModSessionLifesycle.UpdateConnectionTier:
    • The donut branch is gone. For donut sessions it removed the connection from the normal flipper service and refreshed a donut subscription instead.
    • The donut unsubscribe call that ran on every connection is also gone.
  • ModSessionLifesycle.GetAuthLink: the https://donut.coflnet.com auth link is gone. The choice between sky-commands.coflnet.com and sky.coflnet.com based on the trusted host is unchanged.
  • MinecraftSocket:
    • The server=donut query parameter is no longer read (NormalizeServerContext is removed).
    • AddNonConnection is no longer skipped for donut sessions.
    • The donut unsubscribe calls in OnClose and RemoveMySelf are removed.
  • Tests: the donut test, the donut auth-link case, the now-unused gameServer parameter and FakeDonutFlipSubscriptionService are removed.
  • Config: there were no donut-only config keys to remove.

Hypixel behaviour is unchanged.

  • The tier handling in UpdateConnectionTier is the same as before; the only calls removed from the Hypixel path were donut calls.
  • GameServer stays on the socket and in SessionInfo, and now always reads "skyblock". It is required by IFlipConnection in SkyBackendForFrontend and still feeds the existing server span tag.
  • Authentication, payments, player data, command permissions and report tracing are not touched, and no other repository was changed.

Regression test: UpdateConnectionTier_AddsConnectionWithoutDonutService (renamed from UpdateConnectionTier_UsesSkyblockFlipperOnSkyblock). No donut service is registered in the test setup. The test calls UpdateConnectionTier(PREMIUM) for an ordinary connection and checks that it is added to FlipperService.

Verification: the manifest command is dotnet test SkyModCommands.csproj --filter "FullyQualifiedName~ModSessionLifesycleTests.UpdateConnectionTier_AddsConnectionWithoutDonutService". I ran it locally with the sibling repos cloned as the Dockerfile does. The full Docker build was not run; that is left to the host gate.

  • Base production code plus the new test: fails with System.InvalidOperationException : No service for type 'Coflnet.Sky.ModCommands.Services.Donut.IDonutFlipSubscriptionService' has been registered.
  • Patched: passes (1/1).
  • Wider focused check (last phase): ModSessionLifesycleTests|MinecraftSocket passes 35/35 with the patch, and there are no new warnings in the changed files.

The temporary .coflnet-regression.json sits uncommitted at the repository root.

Production evidence (bounded, no identifiers):

  • observe trace returned only a summary for sky-commands-mod: counts per operation, including Flip/flip and command, plus one report span flagged as an error without a message.
  • observe logs --contains IDonutFlipSubscriptionService returned no entries.
  • The reviewed diagnostics cover sky-commands-mod, not SkyBFCS, so I could not see the exception text in production logs. The diagnosis rests on the source code (the base version always looks up the donut service) and on the regression test reproducing the exact reported exception.
  • No auction or lastSent data was relevant to this change.

Validation: 6/6 checks passed · reviewed in a separate codex session.

Checks and provenance
  • container_build
  • regression_sibling_clone_dev
  • regression_sibling_clone_SkyFilter
  • regression_sibling_clone_SkyBackendForFrontend
  • regression_sibling_clone_websocket-sharp
  • regression_base_fail_patch_pass

Review notes:

  • Round 1: Production diagnostics returned “diagnostics are not enabled for this task,” so the reported trace could not be independently inspected; source inspection confirms the unconditional service lookup on base.
  • Round 1: Builds and tests were not run in this read-only review; the configured Docker gate remains pending.

Task: task_egtdlw5zkrnsa3b5uprq

Base: main at 7bce2f2f4e793a022346e802d7265a0ec24f39de

@ekwav-agent
ekwav-agent requested a review from Ekwav September 27, 2026 20:56
@Ekwav
Ekwav merged commit bd0f5e8 into Coflnet:main Oct 2, 2026
3 checks passed
@Ekwav Ekwav mentioned this pull request Oct 3, 2026
6 tasks done
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.

2 participants