List chat channels in config order rather than query completion order

[dmccoystephenson]

Summary

• /listchatchannels built and sent each channel's line from inside that channel's own listeners.thenAccept callback, so each line was sent whenever its own asynchronous query happened to finish. Completion order is not iteration order, so the list arrived shuffled.
• The per-channel line building has been extracted into a private chatChannelComponents function, and the command now starts every listeners query up front and awaits them together with CompletableFuture.allOf before sending. The queries are still fired off concurrently; only the send is ordered.
• The order followed is RPKChatChannelService.chatChannels, which RPKChatChannelServiceImpl.kt:43 builds from the chat-channels config section's key order — so the listed order is the config order, as the upstream report asked for.
• No line content has been changed. The extracted function body is byte-identical to the old callback body apart from indentation, which git diff -w confirms.
• bukkit/rpk-chat-bukkit/build.gradle had no testImplementation block, so the Kotest + MockK dependencies and test { useJUnitPlatform() } have been copied verbatim from bukkit/rpk-players-bukkit/build.gradle to let the regression test live in the module it covers.

Test plan

• ListChatChannelsCommandTests — three channels are listed in service order even though their listener futures are completed in reverse order, and nothing is sent before the last future completes.
• ListChatChannelsCommandTests — each channel is paired with its own listeners when the queries complete out of order (alpha offers Mute, beta offers Unmute), guarding the index-based pairing introduced here.
• ListChatChannelsCommandTests — a service with no chat channels sends no lines, covering the empty CompletableFuture.allOf() path.
• CI: ./gradlew compileKotlin and ./gradlew test on JDK 17.

Anchor verdict

UNVERIFIED locally, CI-anchored. No JDK 17 is available in the session this branch was prepared in, and Gradle 7.6 cannot run on the JDKs that are, so ./gradlew test was not run locally. The build workflow on this pull request's head is the anchor; its result is recorded in the self-review comment. CI's scope is the whole build rather than a narrowed subset, so it does cover both files changed here.

Closes #9
Upstream: RP-Kit#650

No other issue was open on this fork's tracker at triage time, so no skip reasons apply this cycle.

This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

[dmccoystephenson]

Self-review rubric (scored against the diff and CI output, not recollection):

• Scope: PASS — three files. The command carries the fix; bukkit/rpk-chat-bukkit/build.gradle gains only the testImplementation block and test { useJUnitPlatform() } that must exist before any test can live in this module; the third is the test itself. Nothing else is touched.
• Tests-new: PASS — no new public surface was added; the new private chatChannelComponents is exercised by all three new tests through onCommand.
• Tests-fix: PASS — empirically confirmed FAIL→PASS via the CI revert ladder. Head 41feb554 reverted only ListChatChannelsCommand.kt while keeping the tests: run 31662338983 went red with 3 tests completed, 2 failed, naming exactly ListChatChannelsCommandTests.kt:104 (ordering) and :147 (channel/listeners pairing), while the empty-channel case still passed. Head 81dff394 with the fix restored: run 31662529422 green.
• Sibling structure: PASS — the test sits at bukkit/rpk-chat-bukkit/src/test/kotlin/com/rpkit/chat/bukkit/test/command/listchatchannels/, mirroring rpk-players-bukkit's .../bukkit/test/command/profile/; it uses the plural Tests.kt majority naming, Kotest WordSpec, and the ServicesDelegate injection pattern of ProfileCommandTests.
• Sibling renames: no signal — no identifier was renamed.
• Docs: PASS — bukkit/rpk-chat-bukkit/src/main/resources/plugin.yml:26-29 and :58-60 already declare the listchatchannels command and the rpkit.chat.command.listchatchannels permission the code checks, and neither the command name, usage, aliases, permission, nor any message key changed. messages.yml, the README, and .github/workflows/build.yml are unaffected.
• Issue resolution: PASS — every acceptance criterion on Keep chat channels in a consistent order when listing chat channels #9 is met. All listeners futures are created by chatChannels.map(RPKChatChannel::listeners) before CompletableFuture.allOf is reached, so the queries still run concurrently; ordering follows chatChannelService.chatChannels; and git diff -w on the command shows the extracted body is identical to the old callback body apart from indentation, so line content is unchanged.
• CI: PASS — green on head 81dff394. The workflow's scope is the whole build rather than a narrowed subset, so it does cover both changed files; the failing revert run above is direct evidence that this module's tests are actually being executed.
• License header: PASS — the new test file carries the Apache-2.0 block copied from ListChatChannelsCommand.kt in the same module.
• Permission declared: no signal — no permission node was added or renamed.
• Correct module of the lib/impl pair: PASS — the fix landed in the rpk-chat-bukkit implementation. rpk-chat-lib-bukkit's RPKChatChannel.listeners was read and deliberately left alone: ordering is the caller's concern, and changing the interface would have imposed it on every consumer.
• Result type: no signal — no command path was added. ListChatChannelsCommand is a Bukkit CommandExecutor returning Boolean and predates the rpk-core sealed result hierarchy; converting it is recorded as out of scope on Keep chat channels in a consistent order when listing chat channels #9 rather than smuggled in here.
• Service resolution: PASS — Services[RPKMinecraftProfileService::class.java] and Services[RPKChatChannelService::class.java] and their null branches are untouched; no service is constructed directly and no missing service throws.
• No main-thread I/O: PASS — no I/O was added. listenersFutures[index].join() runs inside the allOf callback, at which point every future is already complete, so it cannot block; the send happens on the completing thread exactly as it did before.
• Messages externalized: PASS — no user-facing string literal was added; the message keys used are unchanged.
• No credential churn: PASS — repo.properties is not in the diff.
• Fidelity to upstream: PASS — both halves of Keep chat channels in a consistent order when listing chat channels RP-Kit/RPKit#650 are implemented as written ("the queries should be fired off asynchronously" and "the eventual order should be determined by the order of the chat channels in the config") and nothing beyond them. The one place where upstream is silent — what should happen when a listeners query completes exceptionally — was recorded under "Assumptions and open questions" on Keep chat channels in a consistent order when listing chat channels #9 rather than resolved by inventing a requirement, and has now been filed separately as A failing listeners query now suppresses the whole /listchatchannels list #11.

Two observations from outside the diff, folded in here rather than posted inline:

• bukkit/rpk-chat-bukkit/src/main/kotlin/com/rpkit/chat/bukkit/chatchannel/RPKChatChannelImpl.kt:74-86 — the listeners getter itself already resolves its per-player mute futures with the same CompletableFuture.allOf(...) then join idiom. The fix therefore follows a pattern this module had already established for this exact problem, one level down.
• bukkit/rpk-chat-bukkit/build.gradle — this path matches the loop's do-not-auto-merge list. The match is disclosed rather than worked around; see the merge note below.

This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

[dmccoystephenson]

Merge note — do-not-auto-merge path check, disclosed rather than skipped.

Two entries on the loop's do-not-auto-merge list are matched by this diff:

• bukkit/rpk-chat-bukkit/build.gradle matches the **/build.gradle entry. The match is real but the change is narrow: five testImplementation lines and a test { useJUnitPlatform() } block, copied verbatim from bukkit/rpk-players-bukkit/build.gradle, in one module of the 71. No runtime dependency, no shaded artifact, and no other module's build is affected. Green CI is direct evidence the additions resolve.
• ListChatChannelsCommand.kt matches the "more than 50 lines deleted from a single file" entry on raw line count, showing 100 deletions. git diff -w reduces those to four genuinely deleted lines — the two closing braces of the old thenAccept callback, the old sender.spigot().sendMessage(...) call, and the old return true — with the remaining 96 being reindentation of a body that is otherwise character-for-character unchanged.

The regression gate is satisfied empirically rather than by reasoning: the CI revert ladder recorded in the self-review shows the two ordering tests failing without the fix and passing with it.

This pull request has been merged under an explicit merge pre-authorization given by the operator for this session, which is what satisfies the hold on the two matched paths. Both matches are stated above so the override is on the record rather than implicit.

The branch has deliberately not been deleted. Contributing this fix to RP-Kit/RPKit is human-gated and has not been done or requested; keeping feature/order-chat-channel-list on this fork preserves the --head Dans-Plugins:feature/order-chat-channel-list option should that authorization ever be given.

This comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).