docs(10-14): catalogue the eighteenth repository's feature and UAT surface - #3
Conversation
…rface Read all 5 src/main/java files in full and produced FEATURES.md and UAT-CHECKLIST.md, plus a new .github/pull_request_template.md adapted from UltiChat's merged template (corrected for this repository's own reality: CRLF line endings throughout, and zero .github/workflows at all rather than "holds only maven-ci.yml and publish.yml"). This repository is the phase's own live test of two rules: - Zero lines are written, not omitted: @ConfigEntity, @ConditionalOnConfig, and @ConfigEntry all read 0 against 0 with a stated reason (this example ships no configuration of its own, by design -- it demonstrates the External Plugin API from a plain Bukkit JavaPlugin). - A gate that did not run is reported as not having run, not silently passed: this repository has no .github/workflows directory at all, so gate 4 will be stated as "no configured CI" in the merge report, never claimed green. The persistence row (ultitools-example.data.persistence) is the only place in this phase's eighteen repositories where the External Plugin API's own DataOperator data path is exercised end to end on a real server. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDELX2DH8xeytNqSmSgy6F
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 879706059d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
|
|
||
| | ID | Preconditions | Steps | Expected | Layer | Covers | | ||
| |---|---|---|---|---|---| | ||
| | ultitools-example.greet.hello | none | Run `/ultiext hello` as a player named `Tester` | Chat line reads exactly `Hello Tester! This message is from an external plugin using UltiTools-API.` | server | | |
There was a problem hiding this comment.
When this row is dispatched with an ordinary non-OP player named Tester, the command is rejected instead of producing the expected greeting: GreetCommand requires ultiext.greet, and FEATURES.md correctly notes that the undeclared node defaults to OP-only. Replace none with a precondition that the player is OP or has ultiext.greet; otherwise a valid UAT run can report a false failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 45a60c7: added the OP/ultiext.greet precondition to this row and to every other row in the document whose Steps dispatch through a non-console player (the same class-4 defect applies to all of them, since all five sub-commands share this one class-level permission node and plugin.yml declares no permissions: section) — only the console row (hello.neg-console) is unaffected, since Bukkit's console sender always passes permission checks.
| | `@ConfigEntry` | 0 | same reason as `@ConfigEntity` — zero keys, because zero config classes | | ||
| | `@Table` | 1 | `VisitorRecord.java:13`, `@Table("visitor_records")` | | ||
|
|
||
| This document's command-row count (5) matches the `@CmdMapping` site count exactly (5 against 5). |
There was a problem hiding this comment.
This five-row reconciliation omits the user-visible help behavior implemented by GreetCommand#handleHelp, which emits the command list when the base executor selects its help path. Because the document claims to catalogue every operator/player-visible function and the UAT companion promises one row per feature, counting only @CmdMapping sites leaves this reachable behavior without either an inventory row or a UAT assertion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is a completeness finding (D-24), not a wrong-verdict one — no row here is inaccurate, this is asking for an additional row. Per D-24, out of scope for this review loop: this repository deliberately follows the same convention already established for the framework's own /upm//ulticloud help dispatch and, on the module side, for UltiChat's already-merged FEATURES.md (see its own ## Administration section note: "a deliberate scope decision for this module's first pass, not an oversight" for /uchat help//ch help). This document's own reconciliation note ("This document's command-row count (5) matches the @CmdMapping site count exactly") states the identical scope boundary. Not adding a row inside this loop per D-24's "never add rows inside the loop" rule; not filing a follow-up issue either, since this matches an intentional, already-precedented convention rather than an oversight.
…and needs plugin.yml declares no permissions: section, and all five GreetCommand sub-commands share the single class-level permission node ultiext.greet — so Bukkit's own undeclared-node default (OP-only) gates every one of them. Every row whose Steps dispatch through a non-console player now states that precondition explicitly; the console row (hello.neg-console) is unaffected since Bukkit's console sender always passes permission checks. Also added a Conventions note stating the systemic cause once, matching the existing per-row-plus-Conventions pattern this document already uses for the reload/restart precondition. Codex review round 1 on commit 8797060 flagged this as a class-4 unconstrained-target defect on the hello row specifically; the same defect applies to every other player-dispatched row in this document, so all are fixed together in this one commit per D-24/D-26. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDELX2DH8xeytNqSmSgy6F
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45a60c7040
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| This repository is not a plugin module in the usual sense — it is a worked, buildable example | ||
| demonstrating how a **plain Bukkit `JavaPlugin`** (one that does NOT extend `UltiToolsPlugin`) | ||
| integrates with the framework through the External Plugin API (`UltiToolsAPI.connect(this)`, | ||
| `UltiToolsAPI.getDataOperator(...)`, `UltiToolsAPI.getEventBus()`, |
There was a problem hiding this comment.
The inventory says this example integrates through UltiToolsAPI.getEventBus(), but a repository-wide search of the target commit finds only connect, getDataOperator, and disconnect; none of the five Java sources calls getEventBus. This overstates the External Plugin API surface demonstrated by the example and can cause reconciliation work to treat event-bus integration as covered when only annotation-based listener registration is exercised.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d9d05a9: confirmed by full read of all 5 source files plus a repository-wide grep for EventBus (zero hits) that this example never calls getEventBus(). Corrected the intro paragraph to remove that claim and name the real mechanism JoinListener actually uses: @EventListener reaches ListenerManager#registerAllExternal, which calls Bukkit's own PluginManager#registerEvents directly, a distinct mechanism from the module EventBus.
| | ultitools-example.data.delvisitor | the executing player is OP, or has been granted `ultiext.greet` — this plugin declares no `permissions:` section in `plugin.yml`, so Bukkit's own undeclared-node default is OP-only; a visitor record named `probe` exists — run `/ultiext visit probe` first if none does | Run `/ultiext delvisitor probe` as a player, then run `/ultiext visitors` | Chat/console line reads `[DATA] Deleted visitor record for probe`; the subsequent `/ultiext visitors` no longer lists `probe` | server | | | ||
| | ultitools-example.data.delvisitor.neg-not-found | the executing player is OP, or has been granted `ultiext.greet` — this plugin declares no `permissions:` section in `plugin.yml`, so Bukkit's own undeclared-node default is OP-only; no visitor record named `ghost` exists (delete it first via `ultitools-example.data.delvisitor` if a prior dispatch created one, or use a name never visited in this session) | Run `/ultiext delvisitor ghost` as a player | Chat/console line STILL reads `[DATA] Deleted visitor record for ghost` — `GreetCommand#delVisitor` calls `DataOperator#del` unconditionally and reports success regardless of whether a matching row ever existed; this is the row's actual assertion, not a bug workaround | server | | | ||
| | ultitools-example.data.visit | the executing player is OP, or has been granted `ultiext.greet` — this plugin declares no `permissions:` section in `plugin.yml`, so Bukkit's own undeclared-node default is OP-only; no visitor record named `probe` currently exists (run `ultitools-example.data.delvisitor` first if one does) | Run `/ultiext visit probe` as a player | Chat/console line reads `[DATA] Created record for probe — first visit!` | server | | | ||
| | ultitools-example.data.visit.neg-repeat | the executing player is OP, or has been granted `ultiext.greet` — this plugin declares no `permissions:` section in `plugin.yml`, so Bukkit's own undeclared-node default is OP-only; a visitor record named `probe` exists with visit count 1 (run `ultitools-example.data.visit` immediately before this row, in the same dispatch) | Run `/ultiext visit probe` as a player a second time | Chat/console line reads `[DATA] Updated probe — visit #2` — the SAME record's count incremented, not a duplicate row created | server | | |
There was a problem hiding this comment.
When a regression updates one record but also inserts a duplicate, this row still passes because its only observable check is the success message, even though the Expected cell claims that the same record was updated and no duplicate was created. Add a /ultiext visitors query or direct storage inspection to the Steps and assert that exactly one probe record exists with count 2.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d9d05a9: added a /ultiext visitors query after the repeat visit and an exactly-once assertion (probe listed EXACTLY ONCE with 2 visits), so a regression that inserts a duplicate record alongside the updated one is actually observed rather than passing on the success message alone.
| | ultitools-example.data.visit.neg-repeat | the executing player is OP, or has been granted `ultiext.greet` — this plugin declares no `permissions:` section in `plugin.yml`, so Bukkit's own undeclared-node default is OP-only; a visitor record named `probe` exists with visit count 1 (run `ultitools-example.data.visit` immediately before this row, in the same dispatch) | Run `/ultiext visit probe` as a player a second time | Chat/console line reads `[DATA] Updated probe — visit #2` — the SAME record's count incremented, not a duplicate row created | server | | | ||
| | ultitools-example.data.visitors | the executing player is OP, or has been granted `ultiext.greet` — this plugin declares no `permissions:` section in `plugin.yml`, so Bukkit's own undeclared-node default is OP-only; at least one visitor record exists (run `ultitools-example.data.visit` first) | Run `/ultiext visitors` as a player | Chat/console starts with `[DATA] === Visitor Records (<n>) ===` where `<n>` matches the actual stored count, followed by one ` <name> — <count> visits` line per record | server | | | ||
| | ultitools-example.data.visitors.neg-empty | the executing player is OP, or has been granted `ultiext.greet` — this plugin declares no `permissions:` section in `plugin.yml`, so Bukkit's own undeclared-node default is OP-only; zero visitor records exist (delete every record created earlier in this dispatch via `ultitools-example.data.delvisitor`) | Run `/ultiext visitors` as a player | Chat/console shows exactly `[DATA] No visitor records.` — no header line, no record lines | server | | | ||
| | ultitools-example.data.persistence | the executing player is OP, or has been granted `ultiext.greet` — this plugin declares no `permissions:` section in `plugin.yml`, so Bukkit's own undeclared-node default is OP-only; a visitor record named `restarttest` exists with visit count 1 (run `ultitools-example.data.visit` with `restarttest` first) | Stop the server completely (a full clean shutdown, not `/ul reload`), start it again, then run `/ultiext visitors` as a player | `restarttest` is still listed with visit count 1 — `UltiToolsAPI.getDataOperator`'s backing store persisted the record to disk (JSON/SQLite/MySQL per the framework's own `datasource.type`), independent of this plugin's own in-memory state, which is rebuilt from scratch on every `onEnable` | server | | |
There was a problem hiding this comment.
When the player used after restart is itself named restarttest, joining invokes JoinListener#onJoin before /ultiext visitors is run and increments this record from 1 to 2, producing a false persistence failure. Require the querying player to have a different name, or perform the post-restart query from the console, so the test does not mutate the record it expects to remain at count 1.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d9d05a9: Preconditions and Steps now require the post-restart querying player to be named something other than restarttest (or the query run from console), since JoinListener#onJoin would otherwise auto-increment the very record under test on that player's own rejoin, before /ultiext visitors runs.
…ainst false failures - FEATURES.md's intro overstated the External Plugin API surface this example demonstrates: it claimed UltiToolsAPI.getEventBus() usage, but none of the 5 source files call it (confirmed by full read plus a repository-wide grep for EventBus, zero hits). JoinListener's @eventlistener reaches ListenerManager#registerAllExternal, which calls Bukkit's own PluginManager#registerEvents directly -- a distinct mechanism from the module EventBus this example does not exercise. Corrected the claim and named the real registration path. - ultitools-example.data.visit.neg-repeat's Expected asserted "not a duplicate row created" with no Step that could observe a duplicate -- a regression inserting a second probe record alongside the updated one would still pass on the success-message check alone. Added a /ultiext visitors query and an exactly-once assertion. - ultitools-example.data.persistence's post-restart query had no constraint on the querying player's own name; if that player is itself named restarttest, JoinListener#onJoin auto-increments the very record under test before the query runs, producing a false persistence failure. Preconditions and Steps now require a different player (or the console) for the query. Second Codex review round on commit 45a60c7 found all three; all are wrong-verdict per D-24/D-26 (a literal mismatch and two unconstrained targets), fixed together in this one commit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDELX2DH8xeytNqSmSgy6F
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9d05a9982
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| <!-- Delete any line that does not apply. --> | ||
|
|
||
| - [ ] Targets `master` | ||
| - [ ] Line endings preserved per file (`file <path>` before and after; this tree is CRLF) |
There was a problem hiding this comment.
The target tree is no longer wholly CRLF: this template, FEATURES.md, and UAT-CHECKLIST.md are all LF-only, while the pre-existing tracked files are CRLF. Telling future contributors that “this tree is CRLF” contradicts the same checklist item's per-file preservation requirement and can prompt them to convert the new documentation unnecessarily; describe the repository as mixed or list the expected endings by file type.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 86a8755: restated the checklist line to describe the mixed convention by file type (pre-existing tracked files CRLF, the three files this PR adds LF) rather than asserting a blanket "this tree is CRLF" that would mislead a future contributor into converting the new LF documents.
The template's own checklist line asserted "this tree is CRLF", but the three files this PR adds (this template, FEATURES.md, UAT-CHECKLIST.md) are all LF while pre-existing tracked files (README.md, pom.xml, plugin.yml) are CRLF. A future contributor reading the blanket claim could wrongly convert the new LF documents to match it. Restated the convention as mixed, by file type, matching the same per-file preservation rule the checklist item already states. Third Codex review round on commit d9d05a9 found this; fixed per D-24. No fourth round requested: this is the second post-sweep round (per the dispatch facts' framing), and the finding is fixed in this one commit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDELX2DH8xeytNqSmSgy6F
1 participant
Summary
Adds
FEATURES.md,UAT-CHECKLIST.md, and.github/pull_request_template.mdto thisrepository, produced by reading all 5
src/main/javafiles in full (Phase 10 of theUltiTools-API 6.3.0 milestone). Zero code, POM, or workflow change.
This is the eighteenth and last repository in this phase's fan-out, and its own live test of two
rules the phase exists to enforce:
@ConfigEntityclasses,zero
@ConditionalOnConfigsites, and zero@ConfigEntrykeys — by design, since itdemonstrates the External Plugin API from a plain Bukkit
JavaPlugin, not a configurationsurface. All three reconciliation lines read 0 against 0, each with its reason stated, rather
than being left off the table.
.github/workflowsdirectory at all (measured, not assumed) — gate 4 is reported below as"no configured CI", never as green.
The
.github/pull_request_template.mdcopied from UltiChat's merged template was corrected forthis repository's own reality, not copied verbatim: this repository is CRLF throughout (not
mixed CRLF/LF), and has zero
.github/workflowsfiles at all (not "holds onlymaven-ci.ymlandpublish.yml").The persistence row (
ultitools-example.data.persistence) is the only place in this phase'seighteen repositories where the External Plugin API's own
DataOperatordata path is exercisedend to end on a real server.
Note on the framework version this example targets. This repository is pinned to
UltiTools-API 6.2.2(pom.xml) and itsGreetCommandextends the now-deletedabstracts.AbstractCommandExecutor(removed in the framework's own 6.3.0, seeCOMPATIBILITY.md's "Migrating offAbstractCommandExecutor" section). This is known, alreadytracked, and out of scope here: PR #1 ("feat(GEN-02): migrate GreetCommand to
BaseCommandExecutor") is open and unmerged in this same repository, exactly the migration
COMPATIBILITY.mdalready documents as pending for this repository. No code change is made inthis PR.
Issue closure
None
Verification
Reconciliation command (canonical form, robust to multi-line annotations, worktrees, and
javadoc/string false positives):
Annotation-site reconciliation
@CmdMappingGreetCommand.java:31(hello),:37(info),:44(visit <name>),:74(visitors),:93(delvisitor <name>)@EventListenerJoinListener.java:16, class-level, onPlayerJoinEvent's handler class@Scheduled@ConfigEntity)@ConfigEntity@ConditionalOnConfig)@ConditionalOnConfig@ConfigEntityline above — no config to condition on@ConfigEntry)@ConfigEntry@TableVisitorRecord.java:13,@Table("visitor_records")D-21 GUI-exclusion back-reference: this repository is not in Phase 9's GUI-exclusion register (no
file named for this repository exists under
.planning/phases/09-.../gui-exclusions/) — 0 classes to back-reference.Byte-identity proof (gate 3)
mvn -B -q clean package -DskipTestsrun on bothorigin/master(39940d5, in a separateworktree) and this PR's head after the review loop (86a8755):
origin/master:target/UltiTools-External-Example-1.0.0.jarsha256 =ac901a2833314d6455518c9bb43af7245e9a9537d66d0f22426d9a7413b0c148target/UltiTools-External-Example-1.0.0.jarsha256 =ce23e6dff6f9a9fcb955e4ebdb0cbe63a60f5d0ae91487650d9622235df12842The two whole-file hashes differ, expected:
unzip -lvon both jars (20 entries each) showsexactly ONE differing entry by name-keyed CRC comparison —
META-INF/maven/com.example/UltiTools-External-Example/pom.properties(Maven's ownbuild-timestamp property, rewritten on every build). All 19 other entries match by CRC. The jar
this PR ships is functionally unchanged. (Re-run against
86a8755because the review loop belowadded three documentation-only commits after the earlier hash was recorded.)
Gate 2 note
Codacy is not onboarded on module repositories — gate 2 for this PR is the Codex review only.
Gate 4 (CI)
This repository has no configured CI —
.github/workflows/does not exist here (verified:test -d .github/workflowsreturns false). No required or optional check runs on this PR. Gate 4is stated as absent, not claimed green.
D-26/D-27a sweep tally
Swept every row of both documents against the eight defect classes before opening this PR:
1 vacuous pass — 0 found; 2 literal mismatch — 0 found (every quoted chat line copied verbatim
from the cited source method, this repository has no
lang/*.jsoncatalogue to cross-checkagainst); 3 divergent alternative path — 0 found; 4 unconstrained target — 0 found (every
data-manipulation row states the record state it needs, including the
.neg-variants);5 wrong Source — 1 found and fixed (the persistence row's Source cell had a bare
VisitorRecordwith no
#member, corrected toVisitorRecord#VisitorRecord); 6 convention/row inconsistency —0 found (Kind/Tier/Manual/Target/Permission vocabularies checked by direct
awkextraction, noduplicate IDs); 7 unverified claim — 0 found; 8 forward Preconditions reference — 1 found and
fixed (
ultitools-example.data.delvisitor's precondition citedultitools-example.data.visit,which appears LATER in file order; rewrote the precondition to be self-contained instead of
citing a later row). Both fixes are in the same commit as the initial document creation, applied
before this PR was opened; this is the pre-review sweep result.
Codex review rounds (post-sweep, per D-24)
8797060, 2 findings):ultitools-example.greet.hello's Preconditions saidnonewhile the command requiresultiext.greet(OP-only by Bukkit's undeclared-node default),so a non-OP dispatch would false-fail — class 4 unconstrained target, fixed. A second finding
asked for a row covering
GreetCommand#handleHelp's inherited help route — a completenessfinding (D-24), replied to as out of scope: this repository deliberately follows the same
convention UltiChat's own merged
FEATURES.mdstates for/uchat help//ch help("a deliberatescope decision ... not an oversight"), no row added. Fixed in
45a60c7(every row sharing thesame permission gap, not only the cited one).
45a60c7, 3 findings, all class-appropriate wrong-verdict): the introparagraph claimed
UltiToolsAPI.getEventBus()usage that no source file actually calls (class 2literal mismatch);
data.visit.neg-repeat's Expected asserted "not a duplicate row created" withno Step that could observe one (class 1/7);
data.persistence's post-restart query had noconstraint against the querying player sharing the tested record's own name, letting
JoinListener#onJoinmutate the record under test before the query ran (class 4). Fixed ind9d05a9.d9d05a9, 1 finding): the PR template's own checklist line asserted "thistree is CRLF", but the three files this PR adds are LF while pre-existing tracked files are
CRLF — a future contributor reading the blanket claim could wrongly convert the new documents.
Fixed in
86a8755— this is the final head. Per D-24, no fourth round was requested: round 3 wasthe second post-sweep round, and its one finding was wrong-verdict-class, fixed in the one
commit the protocol calls for.
Checklist
masterfile <path>before and after; this repository is CRLFthroughout; the three new files are LF, matching the copied UltiChat template and this
phase's own convention for newly-created documents)
Chinese content anywhere in its source, so there is no supplement to add)
FEATURES.mdandUAT-CHECKLIST.mdupdated for every feature change in this PR (bothnewly created, this PR's entire content)
🤖 Generated with Claude Code
https://claude.ai/code/session_01NDELX2DH8xeytNqSmSgy6F