Skip chat channels whose listeners query fails in /listchatchannels

[dmccoystephenson]

Summary

• ListChatChannelsCommand previously awaited every chat channel's listeners
  future through a single CompletableFuture.allOf. When any one of those
  queries completed exceptionally, the combined future did too, its thenAccept
  never ran, and no channel line was sent — the player was shown only the
  title. That regression was introduced by PR List chat channels in config order rather than query completion order #10 and disclosed on A failing listeners query now suppresses the whole /listchatchannels list #11.
• Each listeners future is now mapped through handle into a null-on-failure
  future before the futures are awaited together, so a chat channel whose
  query fails is skipped and every other channel still prints, still in chat
  channel service order.
• The failure is now logged at SEVERE with the chat channel's name.
  RPKChatChannelImpl.listeners already logs "Failed to get listeners", but
  without naming which channel was being resolved; the command-level log adds
  that context.

Decision recorded

Issue #11 left the expected behaviour open between two readings. Reading 1 —
"a channel whose listeners query fails is skipped, and the rest of the list
still prints in order" — was chosen, because it restores exactly the tolerance
that existed before PR #10 while keeping that PR's ordering fix. Reading 2
(printing the channel with a fallback mute component) would invent a mute state
that was never resolved, and is therefore not taken here. If reading 2 is
preferred instead, this change is a small edit away from it.

Out of scope

• The equivalent all-or-nothing behaviour inside RPKChatChannelImpl.listeners,
  which predates both Keep chat channels in a consistent order when listing chat channels #9 and A failing listeners query now suppresses the whole /listchatchannels list #11.
• Any player-visible message when every channel's query fails. The title alone
  is still sent in that case, which matches what A failing listeners query now suppresses the whole /listchatchannels list #11 describes; nothing beyond
  that was requested.

Test plan

• skip a chat channel whose listeners query fails and still send the rest in order
  — three channels, the middle one completed exceptionally; asserts the
  surviving two lines are sent in service order and that the failure is
  logged with the channel name.
• send nothing but the title when every listeners query fails — pins the
  boundary case.
• The three pre-existing tests in ListChatChannelsCommandTests are
  unchanged, so the ordering guarantees from PR List chat channels in config order rather than query completion order #10 are still asserted.
• Local ./gradlew test — not runnable on the dispatching machine; the
  Gradle wrapper reports JVM: 21.0.10, and Gradle 7.6 with Kotlin 1.7.22
  requires a JDK in the 17-19 range. See the anchor note below.

Anchor

UNVERIFIED locally, deferred to CI. The build workflow
(.github/workflows/build.yml, JDK 17, full-tree compileKotlin + test) is
the anchor for this PR, and its result on this PR's head commit is the verdict
that counts. This PR changes Kotlin source, so it is not eligible for merge on
a red or absent anchor.

Closes #11

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

[dmccoystephenson]

Self-review rubric, scored against the diff and the CI runs on this branch:

• Scope: PASS — only two files are touched, both in rpk-chat-bukkit, and both are named by A failing listeners query now suppresses the whole /listchatchannels list #11. No unrelated file is carried along.
• Tests-new: PASS — no new public method is introduced (the change is confined to the body of ListChatChannelsCommand.onCommand), and that body is exercised by five tests, two of which are added here.
• Tests-fix: PASS — established empirically, not by reasoning, via the CI temporary-revert ladder. Commit b0bbb57 reverted only ListChatChannelsCommand.kt while keeping the new tests; run 31861454777 went red with ListChatChannelsCommandTests > skip a chat channel whose listeners query fails and still send the rest in order FAILED (AssertionFailedError at ListChatChannelsCommandTests.kt:231), 5 tests completed, 1 failed. The revert was then dropped and the branch force-pushed back to ce341b9, where run 31861634070 is green. FAIL to PASS is therefore recorded rather than asserted.
• CI: PASS — build (JDK 17 temurin, full-tree ./gradlew compileKotlin then ./gradlew test) passes on head ce341b9 in 3m52s. The workflow's scope is the whole 71-module tree, so the changed module is genuinely covered; nothing about this green is narrowed. The local Gradle wrapper reports JVM: 21.0.10 on the dispatching machine and cannot serve as an anchor, so CI is the only signal used.
• Sibling structure: PASS — no file is created; both files already existed and their existing conventions are followed.
• Sibling renames: PASS — no identifier is renamed.
• Docs: PASS — Phase 7 was re-run against the implementation. bukkit/rpk-chat-bukkit/src/main/resources/plugin.yml still declares listchatchannels (line 26) and rpkit.chat.command.listchatchannels (line 58), neither of which this change alters; no config key is read or dropped; no README or workflow claim is invalidated.
• Issue resolution: PASS with a disclosure. A failing listeners query now suppresses the whole /listchatchannels list #11 named ListChatChannelsCommand.kt as the surface area and that file is what changed. A failing listeners query now suppresses the whole /listchatchannels list #11 deliberately left the expected behaviour undecided between two readings, so no acceptance-criteria checklist existed to satisfy; reading 1 was selected and the selection is recorded in the PR body rather than left implicit. The alternative reading and the two out-of-scope items from A failing listeners query now suppresses the whole /listchatchannels list #11 are restated there unimplemented.
• License header: PASS — no new .kt file; both edited files retain their existing Apache-2.0 block.
• Permission declared: PASS — no permission node is added or renamed.
• Correct module of the lib/impl pair: PASS — the change lands in the implementation module rpk-chat-bukkit. The API-side alternative was considered and rejected: RPKChatChannel.listeners in rpk-chat-lib-bukkit is a contract that legitimately reports failure, and A failing listeners query now suppresses the whole /listchatchannels list #11 explicitly fences its all-or-nothing behaviour out of scope.
• Result type: PASS — ListChatChannelsCommand is a plain Bukkit CommandExecutor returning Boolean, predating the rpk-core sealed hierarchy; the return path is untouched by this diff, so no ad-hoc result type is introduced.
• Service resolution: PASS — the existing Services[...] lookups and their message-on-null branches are unchanged; no service resolution is added.
• No main-thread I/O: PASS — no query is added. The added handle stage runs on whichever thread completes the already-existing listeners future, and only logs.
• Messages externalized: PASS — the one string added is a server-log line, not player-facing, and matches the plugin.logger.log(Level.SEVERE, ...) convention used throughout this module (for example RPKChatChannelImpl.kt:88). No literal is added to a player-visible path.
• No credential churn: PASS — repo.properties is not in the diff.
• Fidelity to upstream: PASS, not applicable in the strict sense — A failing listeners query now suppresses the whole /listchatchannels list #11 is fork-native rather than mirrored, so no upstream author's request is being translated. Keep chat channels in a consistent order when listing chat channels RP-Kit/RPKit#650, the ancestor of Keep chat channels in a consistent order when listing chat channels #9, is referenced but not modified, commented on, or closed by this work.

One observation folded in here rather than left inline, as it falls outside the diff hunks: the second added test, send nothing but the title when every listeners query fails, passed on the reverted commit as well as the fixed one. That is expected and is not a defect — it pins a boundary that both versions happen to share, and the regression itself is carried by the first added test.

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