🤖 feat: tab-completion testing and clickable-chat helpers (LK-13, protocol v11)#102
Merged
Conversation
First layer of the S7 tab-completion slice: the wire contract. Adds the TabCompletePlayer namespace (Command carrying the player UUID and raw command-line buffer; Response carrying the ordered completions with a defensive copy and null-to-empty normalization), registers it in the IAgentCommand @JsonSubTypes/permits and IAgentResponse permits, and bumps RuntimeProtocol.VERSION 10 -> 11 so framework and agent stay in lockstep. Unlike ExecutePlayerCommand, the command line is deliberately not stripped or rejected when blank: a trailing space selects argument completion, so the buffer is load-bearing and preserved verbatim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
Second layer: the in-server handler for TAB_COMPLETE_PLAYER. Resolves the synthetic player from the store like its siblings, strips at most one leading slash (mirroring handleExecutePlayerCommand, since the command map matches names without a slash and its args branch does a raw lookup), and calls CommandMap#tabComplete on the main thread, mapping a null result to an empty list. getCommandMap() is not on the org.bukkit.Server interface on either distro (Spigot returns a versioned SimpleCommandMap, Paper an unversioned CommandMap), so it is reached through one cached reflective lookup off the live server; the CommandMap it returns is plain spigot-api, so no per-version NMS module is needed. A missing/renamed accessor fails loudly. Wires the exhaustive dispatcher switch arm. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
Third layer: the test-facing API. Wires tabComplete through the established gateway pattern (IFrameworkGatewayView -> UdsAgentClient -> DefaultLightkeeperFramework) and exposes PlayerHandle.tabComplete(String): the buffer is validated non-blank but never trimmed, so a trailing space still selects argument completion, and a leading slash is accepted. Adds the chat-click convenience with no new protocol command: on ChatComponentSnapshot, clickRunCommand()/clickSuggestedCommand() parse the component tree (Jackson, already a framework dependency) for a click_event of the requested action (findValues walks 'extra' children; per-action matching so run never returns a suggest payload; malformed JSON degrades to empty). On PlayerHandle, clickChatComponent(...) extracts the run_command and reuses executeCommand, throwing when there is no run_command click. Parsing keys on the modern snake_case click_event, never the stale clickEvent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
Chat-component capture was silently dead on Spigot: the outbound ClientboundSystemChatPacket exposes its component through the remapped record accessor `IChatBaseComponent b()`, whose single-letter name matched none of isSafeComponentAccessor's name heuristics (get/is/component/message/ content/title), so the walker never followed it. Paper exposes the same field as content() and worked. The bot therefore captured the message text but not the component JSON, leaving the entire chat-click feature (and hasClickableChatText) non-functional on Spigot. Key the accessor decision on the return type first: any zero-arg accessor returning a net.minecraft.network.chat.* / *Component type is followed regardless of its name, so components are captured identically on Spigot and Paper. The name heuristic is retained only for container (Optional/Collection/array) return types. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
hasClickableChatText keyed on the pre-1.21.5 camelCase "clickEvent" field, but the native component codec renamed it to snake_case "click_event" around 1.21.5. On 1.21.11 the real wire JSON never contains "clickEvent", so the assertion silently never matched a real click-event component. Accept both spellings (click_event first, clickEvent for pre-1.21.5 payload compat) so the assertion works across component-codec versions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
…in support End-to-end coverage on both distros. The test plugin gains /lktestclick, which emits a clickable run_command component via vanilla /tellraw (this Minecraft version's spigot-api ships no component builder, and /tellraw is parsed by the same native codec that serializes click_event), plus a permission-gated no-op /lktestperm command that the tab-complete filtering test needs (lktestgui has no command-level permission, so it could not exercise it). LightkeeperTabCompleteIT asserts a permissionless plugin command is suggested, an unknown command returns empty, and command names are permission-filtered before/after a runtime grant. LightkeeperChatClickIT runs the full capture -> extract -> click -> observe-side-effect flow and locks in that captured JSON uses click_event, not clickEvent. Both are green on Paper and Spigot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
Add the command tab-completion and clickable-chat-component capabilities to the Key Features and Core Features sections, describing the permission filtering, leading-slash/trailing-space handling, and the capture -> extract -> click helper flow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
Goal: fold the Opus review round into the S7 slice. Drops the redundant whole-chat-package disjunct from the component-accessor predicate — the type-suffix match alone covers both distros' real accessors, while the package match would nondeterministically follow MessageSignature/ ChatType.Bound/FilterMask accessors on multi-field chat packets and capture a decoration instead of the message (plus WARNING spam). The widened type-driven capture semantics are documented in place, and the TabCompletePlayer wire javadoc now states that the framework's public API rejects blank input before it reaches the command. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
Owner
Author
|
@coderabbitai full review |
Owner
Author
|
@codex review |
Contributor
✅ Action performedFull review finished. |
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdded end-to-end player tab completion through the protocol, framework, and Spigot agent, alongside clickable chat command extraction and dispatch. Updated component detection, protocol versioning, integration fixtures, tests, and README documentation. ChangesPlayer interaction features
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PlayerHandle
participant Framework
participant UdsAgentClient
participant SpigotAgent
participant CommandMap
PlayerHandle->>Framework: tabComplete(commandLine)
Framework->>UdsAgentClient: send TabCompletePlayer.Command
UdsAgentClient->>SpigotAgent: TAB_COMPLETE_PLAYER request
SpigotAgent->>CommandMap: tabComplete(synthetic player, normalized line)
CommandMap-->>SpigotAgent: completion suggestions
SpigotAgent-->>PlayerHandle: completion response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds new end-to-end testing capabilities to LightKeeper for (1) command tab-completion in a real server/permission context and (2) capturing, extracting, and programmatically “clicking” modern clickable-chat components, alongside a protocol bump to v11 to support the new agent command.
Changes:
- Introduce protocol v11
TAB_COMPLETE_PLAYERand expose it asPlayerHandle.tabComplete(commandLine)with agent-sideCommandMap#tabCompletebacked completions. - Add framework-only clickable chat helpers:
ChatComponentSnapshot.clickRunCommand()/clickSuggestedCommand()plusPlayerHandle.clickChatComponent(...), and update assertions to recognize modernclick_event. - Fix Spigot chat-component capture by following obfuscated record accessors based on component return type; add unit + integration coverage for both features.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents new tab-completion and chat-click testing helpers. |
| lightkeeper-spigot-test-plugin/src/main/java/nl/pim16aap2/lightkeeper/spigot/testplugin/LkTestClickCommandExecutor.java | Adds /lktestclick command emitting a run_command clickable chat component for integration tests. |
| lightkeeper-spigot-test-plugin/src/test/java/nl/pim16aap2/lightkeeper/spigot/testplugin/LkTestClickCommandExecutorTest.java | Unit tests the generated /tellraw payload and click_event wire format. |
| lightkeeper-spigot-test-plugin/src/main/resources/plugin.yml | Registers new test commands and permission node used by tab-completion tests. |
| lightkeeper-spigot-test-plugin/src/main/java/nl/pim16aap2/lightkeeper/spigot/testplugin/LightkeeperSpigotTestPlugin.java | Wires the new /lktestclick command executor at plugin startup. |
| lightkeeper-runtime-core/src/main/java/nl/pim16aap2/lightkeeper/runtime/RuntimeProtocol.java | Bumps runtime protocol version from 10 → 11. |
| lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/TabCompletePlayer.java | Adds typed command/response records for tab-completion, with defensive-copy semantics. |
| lightkeeper-protocol/src/test/java/nl/pim16aap2/lightkeeper/protocol/TabCompletePlayerTest.java | Validates serialization and input/defensive-copy behavior for the new protocol types. |
| lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/IAgentCommand.java | Registers TAB_COMPLETE_PLAYER for polymorphic command decoding and sealing. |
| lightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/IAgentResponse.java | Adds TabCompletePlayer.Response to the sealed response set. |
| lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentRequestDispatcher.java | Routes the new command to player actions. |
| lightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.java | Implements TAB_COMPLETE_PLAYER via cached reflective getCommandMap + CommandMap#tabComplete. |
| lightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.java | Unit tests slash-stripping, null→empty normalization, trailing-space preservation, and unknown-player behavior. |
| lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/PlayerHandle.java | Exposes tabComplete(...) and clickChatComponent(...) to test authors. |
| lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/ChatComponentSnapshot.java | Adds click extraction helpers walking the component JSON tree for modern click_event. |
| lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/assertions/PlayerHandleAssert.java | Updates clickable-chat assertion to recognize both click_event and legacy clickEvent. |
| lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/UdsAgentClient.java | Adds client call to send TAB_COMPLETE_PLAYER and return completions. |
| lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/DefaultLightkeeperFramework.java | Adds framework gateway entrypoint with blank-buffer rejection and pass-through semantics. |
| lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/IFrameworkGatewayView.java | Extends gateway interface with tab-completion API. |
| lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/PlayerHandleTest.java | Unit tests delegation for tabComplete(...) and clickChatComponent(...). |
| lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/ChatComponentSnapshotTest.java | Tests click extraction behavior for root/nested click events and malformed JSON. |
| lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/assertions/HandleAssertionsTest.java | Verifies assertions match modern snake_case click events. |
| lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/internal/UdsAgentClientTest.java | Tests request formatting and trailing-space preservation on the wire. |
| lightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/internal/DefaultLightkeeperFrameworkGatewayTest.java | Tests gateway delegation + validation for tab completion buffers. |
| lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperTabCompleteIT.java | End-to-end coverage for permission-filtered completions and unknown-command behavior. |
| lightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperChatClickIT.java | End-to-end tellraw→capture→extract→click→command-executed validation. |
| lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotPlayerNmsAdapterV1_21_R7.java | Fixes component capture to follow obfuscated record accessors via component return type suffix. |
| lightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/test/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotPlayerNmsAdapterV1_21_R7Test.java | Adds regression test proving obfuscated record accessor component capture works. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@lightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/ChatComponentSnapshot.java`:
- Around line 79-84: Update ChatComponentSnapshot.findClickCommand to inspect
both click_event and legacy clickEvent nodes, accepting command or legacy value
payload fields while preserving action matching and empty results when no
command exists. Update ChatComponentSnapshot JavaDoc to document support for
both field variants, and revise ChatComponentSnapshotTest to expect successful
extraction from legacy payloads at the specified test location.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e313533-7ffa-4d97-a1df-e0b25308eb0e
📒 Files selected for processing (28)
README.mdlightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActions.javalightkeeper-agent-spigot/src/main/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentRequestDispatcher.javalightkeeper-agent-spigot/src/test/java/nl/pim16aap2/lightkeeper/agent/spigot/AgentPlayerActionsTest.javalightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/ChatComponentSnapshot.javalightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/IFrameworkGatewayView.javalightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/PlayerHandle.javalightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/assertions/PlayerHandleAssert.javalightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/DefaultLightkeeperFramework.javalightkeeper-framework-junit/src/main/java/nl/pim16aap2/lightkeeper/framework/internal/UdsAgentClient.javalightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/ChatComponentSnapshotTest.javalightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/PlayerHandleTest.javalightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/assertions/HandleAssertionsTest.javalightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/internal/DefaultLightkeeperFrameworkGatewayTest.javalightkeeper-framework-junit/src/test/java/nl/pim16aap2/lightkeeper/framework/internal/UdsAgentClientTest.javalightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperChatClickIT.javalightkeeper-integration-tests/src/test/java/nl/pim16aap2/lightkeeper/maven/test/LightkeeperTabCompleteIT.javalightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/main/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotPlayerNmsAdapterV1_21_R7.javalightkeeper-nms-parent/lightkeeper-nms-v1_21_R7/src/test/java/nl/pim16aap2/lightkeeper/nms/v121r7/BotPlayerNmsAdapterV1_21_R7Test.javalightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/IAgentCommand.javalightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/IAgentResponse.javalightkeeper-protocol/src/main/java/nl/pim16aap2/lightkeeper/protocol/TabCompletePlayer.javalightkeeper-protocol/src/test/java/nl/pim16aap2/lightkeeper/protocol/TabCompletePlayerTest.javalightkeeper-runtime-core/src/main/java/nl/pim16aap2/lightkeeper/runtime/RuntimeProtocol.javalightkeeper-spigot-test-plugin/src/main/java/nl/pim16aap2/lightkeeper/spigot/testplugin/LightkeeperSpigotTestPlugin.javalightkeeper-spigot-test-plugin/src/main/java/nl/pim16aap2/lightkeeper/spigot/testplugin/LkTestClickCommandExecutor.javalightkeeper-spigot-test-plugin/src/main/resources/plugin.ymllightkeeper-spigot-test-plugin/src/test/java/nl/pim16aap2/lightkeeper/spigot/testplugin/LkTestClickCommandExecutorTest.java
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
PimvanderLoos
added a commit
that referenced
this pull request
Jul 16, 2026
Goal: address CodeRabbit's bot-round finding on PR #102 — extraction rejected the legacy clickEvent/value spelling while hasClickableChatText accepted it, so clickChatComponent would throw on the very payloads the assertion matches. findClickCommand now mines both key spellings (command first, then value) in preference order, with the legacy test flipped to assert successful extraction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
Goal: address CodeRabbit's bot-round finding on PR #102 — extraction rejected the legacy clickEvent/value spelling while hasClickableChatText accepted it, so clickChatComponent would throw on the very payloads the assertion matches. findClickCommand now mines both key spellings (command first, then value) in preference order, with the legacy test flipped to assert successful extraction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P15FjGQzyNmX9T6Qmyq8Dn
PimvanderLoos
force-pushed
the
feat/s7-tab-complete
branch
from
July 16, 2026 08:19
2eaf63b to
6e04d33
Compare
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.
Why
How
TAB_COMPLETE_PLAYERcommand (protocol v11): agent-side completions via a cached reflectivegetCommandMaplookup (works on both distros from spigot-api), leading-/normalized, null→empty; exposed asplayer.tabComplete(commandLine).ChatComponentSnapshot.clickRunCommand()/clickSuggestedCommand()mine the modernclick_eventwire key (whole-tree, action-exact), andplayer.clickChatComponent(snapshot)executes the mined command — semantically what a real client click does.hasClickableChatTextmatched only the pre-1.21.5clickEventkey (could never match modern output); and chat-component capture was entirely dead on Spigot — the accessor-safety heuristic rejected Spigot's obfuscated record accessors, so component capture now keys on the return type (deliberately only the type suffix: a whole-package match would nondeterministically capture chat decorations instead of the message).Tests
mvn --batch-mode -Perrorprone clean verify checkstyle:checkstyle pmd:check→ BUILD SUCCESS (full reactor; newLightkeeperTabCompleteIT(3) andLightkeeperChatClickIT(1, end-to-end tellraw→capture→extract→click→executed) green on both Paper and Spigot lanes).🤖 Generated with Claude Code
Summary by CodeRabbit