docs: catalogue UltiRemoteBag's feature inventory and UAT checklist - #21
Conversation
- Add FEATURES.md: 39 rows (9 commands, 2 GUI pages, 1 event, 1 scheduled, 2 persistence, 24 config keys) read from all 14 src/main/java files - Add UAT-CHECKLIST.md: 26 executable rows including 11 negative cases, covering both Phase 9 GUI-excluded classes (RemoteBagMainGUI, RemoteBagContentGUI) and the bag-survives-restart persistence case - Add .github/pull_request_template.md, copied verbatim from UltiChat - File 8 product defects found while reading (#13-#20): seven declared-but-never-read RemoteBagConfig keys, and hardcoded Simplified Chinese lock-refusal messages ignoring the language setting 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: 15fd57365d
ℹ️ 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".
| | ultiremotebag.bag.content-gui | `language: en`; the sender owns at least one bag page (run `ultiremotebag.bag.open-page` above first, or purchase one via `ultiremotebag.bag.main-gui` above); `sound.enabled: true` (shipped default) | Run `/bag <page>` for an owned, currently-unlocked page | A 6-row inventory opens titled `Bag #<page>` (gold), with any previously placed items visible in slots 0-44; the bottom row (slots 45-53) shows, in order: a back button (yellow, `Back`), two background fillers, a background filler (read-only-only refresh slot, hidden in edit mode), a green save button (`Save`), a green mode indicator (`Edit Mode`), two background fillers, and a red close button (`Close`); the configured `sound.open` sound plays | pixel | RemoteBagContentGUI | | ||
| | ultiremotebag.bag.save | `language: en`; sender owns at least one bag page, currently open in edit mode (run `ultiremotebag.bag.open-page` above first) | Run `/bag save` | Chat line reads `Bag saved manually!` (green, `bag_saved_manually`) — persisted immediately, independent of the 300-second auto-save cadence | server | | | ||
| | ultiremotebag.bag.persistence | `language: en`; a distinguishable item (e.g. a single `DIAMOND`) placed into slot 0 of an owned bag page and saved (run `ultiremotebag.bag.save` above immediately after placing it, or close the content GUI in edit mode) | Restart the server completely (not `/ul reload`), then log back in and run `/bag <page>` for the same page | The `DIAMOND` is present in slot 0, exactly as left before the restart — `RemoteBagService#loadBagIfNeeded` re-queried `remote_bags` and deserialized the stored YAML back into the same slot | server | | | ||
| | ultiremotebag.bag.auto-save | `language: en`; a player has placed at least one item into an owned bag page via the content GUI (open `ultiremotebag.bag.open-page` above first) WITHOUT running `/bag save` and WITHOUT closing the GUI in edit mode (leave it open, or place the item then immediately reopen without closing) — the item must be in the in-memory cache only, not yet flushed to `remote_bags`; DO NOT rely on `auto_save_interval` to shorten the wait — it has no effect (`UltiKits/UltiRemoteBag#13`); the real period is a hardcoded 6000-tick (300s) `@Scheduled` period | Wait 300 seconds with the player online (or offline is fine too — the cache entry persists in server memory regardless of the player's connection state) | The item is present in the `remote_bags` table (or observably present on next `/bag <page>` open after a clean restart) even though the player never ran `/bag save` and never closed the GUI in edit mode — the scheduled task alone persisted it | server | | |
There was a problem hiding this comment.
When this row follows its online path, an item placed in the open inventory is not yet in RemoteBagService's cache: RemoteBagContentGUI#saveCurrentContents copies GUI slots into the cache only on a GUI save or close, while autoSaveTask persists only the existing cache. The item therefore will not be saved by the scheduled task; taking the offered offline path instead triggers the close/quit saves and can make the row pass without testing auto-save at all. Use a genuinely dirty cache-only state and keep the player online so the expected database write can be attributed to the scheduler.
Useful? React with 👍 / 👎.
| | ultiremotebag.config.remotebag.messages.page_locked | Declared as a page-locked message with a `{PAGE}` placeholder; never read anywhere — the actual blocked/read-only messages come from `BagOpenResult`'s own hardcoded strings. Known product defect, `UltiKits/UltiRemoteBag#16` | config | `config/remotebag.yml: messages.page_locked (default: a Chinese-language message meaning "You do not have permission to access page {PAGE}!", has no effect, see UltiKits/UltiRemoteBag#16)` | n/a | n/a | admin | brief | RemoteBagConfig#pageLockedMessage (declared, never read outside this class) | | ||
| | ultiremotebag.config.remotebag.permission_based_pages | Switch the page-limit calculation from the flat `default_pages` value to scanning `permission_prefix.N` nodes downward from `max_pages` for the first one the player holds | config | `config/remotebag.yml: permission_based_pages (default: true)` | n/a | n/a | admin | brief | RemoteBagService#getPlayerMaxPages | | ||
| | ultiremotebag.config.remotebag.permission_prefix | Permission-node prefix scanned (with an appended page-count integer) when `permission_based_pages` is true, e.g. `ultibag.pages.3` grants up to 3 pages | config | `config/remotebag.yml: permission_prefix (default: "ultibag.pages.")` | n/a | n/a | admin | brief | RemoteBagService#getPlayerMaxPages | | ||
| | ultiremotebag.config.remotebag.rows_per_page | Number of inventory rows (1-6) making up one bag page's content area; also fixes each page's slot capacity used for statistics and empty-content sizing | config | `config/remotebag.yml: rows_per_page (default: 6)` | n/a | n/a | admin | brief | RemoteBagMainGUI#createBagIcon, RemoteBagService#deserializeItems | |
There was a problem hiding this comment.
For any configured value below 5, the content GUI still exposes and saves its hardcoded 45 slots, while deserializeItems allocates only rows_per_page * 9 entries and writes YAML slot numbers directly into that array. Saving an item in a higher visible slot and reopening can therefore throw during deserialization and return an empty page, rather than providing the documented reduced-capacity inventory. This key should be documented and tested as the current data-loss hazard, not as a normal content-area size control.
Useful? React with 👍 / 👎.
|
|
||
| | ID | Preconditions | Steps | Expected | Layer | Covers | | ||
| |---|---|---|---|---|---| | ||
| | ultiremotebag.config.remotebag-yml | Fresh `config/remotebag.yml` at its shipped default | Load the file; confirm all 24 keys listed under `FEATURES.md`'s `## Configuration` section are present at their documented defaults; then set `lock.timeout_seconds: 5` (default 300), have the owner open a bag page (acquiring the edit lock) WITHOUT closing it, wait 6 seconds, then have an admin run `ultiremotebag.admin.see` against the same page. Do NOT vary `auto_save_interval`, `gui_title`, `messages.no_permission`, `messages.page_locked`, `messages.bag_saved`, `save_on_close`, or `lock.notify_readonly_viewers` expecting an observable effect — none of the seven has one (see each key's own `FEATURES.md` row and filed issue) | All 24 keys present at their documented defaults before the change; after lowering `lock.timeout_seconds` to 5 and waiting past it, the admin's `/bag see <target>` acquires the EDIT lock directly (owner's now-expired lock is discarded by `BagLockService#adminOpen`'s expiry check) rather than falling back to read-only mode, proving the lowered timeout took effect | server | | |
There was a problem hiding this comment.
This procedure cannot produce the expected six-second expiry: lock.timeout_seconds is constrained by @Range(min = 10, max = 3600), and BagLockService#setLockTimeout is only called from UltiRemoteBag#registerSelf, while the steps neither use a valid value nor restart/re-register after editing the file. As written the service retains its 300-second timeout and the admin receives read-only access after six seconds; use a value of at least 10 and explicitly apply it before acquiring the lock.
Useful? React with 👍 / 👎.
| | ultiremotebag.bag.main-gui | `language: en`; the sender owns exactly one bag page (fresh player, shipped `max_pages: 10` default so the sender is under the page limit); Vault installed, `economy.enabled: true` (shipped default), and the sender's Vault balance is at least `economy.base_price` (10000, shipped default) | Run `/bag` | A 6-row inventory opens titled `<player> 's Remote Bags` (gold), listing exactly one chest icon (item/slot-usage lore) for the owned page, plus a GREEN minecart purchase icon (the balance precondition guarantees the affordable branch of `RemoteBagMainGUI#createPurchaseIcon` — a balance below the price would instead show a RED barrier icon, not exercised by this row); the previous/next navigation buttons plus a red `Close` button appear on the bottom row; the configured `sound.open` sound plays | pixel | RemoteBagMainGUI | | ||
| | ultiremotebag.bag.content-gui | `language: en`; the sender owns at least one bag page (run `ultiremotebag.bag.open-page` above first, or purchase one via `ultiremotebag.bag.main-gui` above); `sound.enabled: true` (shipped default) | Run `/bag <page>` for an owned, currently-unlocked page | A 6-row inventory opens titled `Bag #<page>` (gold), with any previously placed items visible in slots 0-44; the bottom row (slots 45-53) shows, in order: a back button (yellow, `Back`), two background fillers, a background filler (read-only-only refresh slot, hidden in edit mode), a green save button (`Save`), a green mode indicator (`Edit Mode`), two background fillers, and a red close button (`Close`); the configured `sound.open` sound plays | pixel | RemoteBagContentGUI | | ||
| | ultiremotebag.bag.save | `language: en`; sender owns at least one bag page, currently open in edit mode (run `ultiremotebag.bag.open-page` above first) | Run `/bag save` | Chat line reads `Bag saved manually!` (green, `bag_saved_manually`) — persisted immediately, independent of the 300-second auto-save cadence | server | | | ||
| | ultiremotebag.bag.persistence | `language: en`; a distinguishable item (e.g. a single `DIAMOND`) placed into slot 0 of an owned bag page and saved (run `ultiremotebag.bag.save` above immediately after placing it, or close the content GUI in edit mode) | Restart the server completely (not `/ul reload`), then log back in and run `/bag <page>` for the same page | The `DIAMOND` is present in slot 0, exactly as left before the restart — `RemoteBagService#loadBagIfNeeded` re-queried `remote_bags` and deserialized the stored YAML back into the same slot | server | | |
There was a problem hiding this comment.
When the suggested /bag save route is used immediately after placing the DIAMOND in an open GUI, BagCommand#saveBag persists the service cache without first copying the inventory slots into it. That copy occurs only in RemoteBagContentGUI#saveCurrentContents, so the subsequent restart loses the newly placed item even though the command reports success. Require the GUI save button or a close for this persistence row, or add a separate negative check that accurately exposes the command behavior.
Useful? React with 👍 / 👎.
| | ultiremotebag.admin.delete | `language: en`; admin holds `ultibag.use` AND `ultibag.admin.delete`; target player has an unlocked bag page that exists | Run `/bag delete <target> <page>` | Chat line reads `Deleted bag #<page> of player <target>` (green, `admin_bag_deleted`); `/bag list <target>` immediately afterward no longer lists that page, and its `remote_bags` row is gone | server | | | ||
| | ultiremotebag.admin.delete.neg-locked | `language: en`; admin holds `ultibag.use` AND `ultibag.admin.delete`; the target player currently holds the OWNER edit lock on the page | Run `/bag delete <target> <page>` | Chat line reads `Bag is in use, cannot delete` (red, `bag_in_use_cannot_delete`); the page still exists afterward | server | | | ||
| | ultiremotebag.admin.list | `language: en`; admin holds `ultibag.use` AND `ultibag.admin.list`; target player owns at least one bag page with a known item/slot count | Run `/bag list <target>` | Chat starts with `=== <target> 's Bag List ===` (gold), then one line per page reading ` #<N> - <itemCount> items, <stackCount> slots` (yellow `#N`, white separator), then ends with `Total <count> bags` (gold) | server | | | ||
| | ultiremotebag.admin.list.neg-no-bags | `language: en`; admin holds `ultibag.use` AND `ultibag.admin.list`; target player has played but every one of their bag pages has been deleted via `ultiremotebag.admin.delete` first | Run `/bag list <target>` | Chat shows the `=== ... ===` header followed by `No bags` (gray, `no_bags`) and `Total 0 bags` (gold) — no per-page lines | server | | |
There was a problem hiding this comment.
After the final stored page is deleted, RemoteBagService#getPlayerBagPages does not return an empty list; it substitutes Collections.singletonList(1). Consequently this row prints statistics for virtual page 1 and Total 1 bags, and the corresponding see.neg-no-bags row opens page 1 instead of emitting player_no_bags. Both checks need a reachable setup or must document the empty-branch defect rather than expecting these messages.
Useful? React with 👍 / 👎.
| | ID | Feature | Kind | How to reach | Permission | Target | Tier | Manual | Source | | ||
| |---|---|---|---|---|---|---|---|---| | ||
| | ultiremotebag.config.remotebag.auto_save_interval | Declared as the auto-save task's interval in seconds; never read — the real cadence is a hardcoded 6000-tick (300s) `@Scheduled` period regardless of this key's value. Known product defect, `UltiKits/UltiRemoteBag#13` | config | `config/remotebag.yml: auto_save_interval (default: 300, has no effect, see UltiKits/UltiRemoteBag#13)` | n/a | n/a | admin | brief | RemoteBagConfig#autoSaveInterval (declared, never read outside this class) | | ||
| | ultiremotebag.config.remotebag.default_pages | Number of free bag pages a player has before hitting the page limit, used only when `permission_based_pages` is false or no `permission_prefix.N` node matches | config | `config/remotebag.yml: default_pages (default: 1)` | n/a | n/a | admin | brief | RemoteBagService#getPlayerMaxPages | |
There was a problem hiding this comment.
When permission_based_pages is false, RemoteBagService#getPlayerMaxPages immediately returns config.getMaxPages(); default_pages is consulted only when permission-based scanning is enabled and no matching node exists. The current description reverses that behavior and can cause an operator who disables permission-based limits to grant every player max_pages while believing the lower default still applies.
Useful? React with 👍 / 👎.
| | ultiremotebag.config.remotebag.auto_save_interval | Declared as the auto-save task's interval in seconds; never read — the real cadence is a hardcoded 6000-tick (300s) `@Scheduled` period regardless of this key's value. Known product defect, `UltiKits/UltiRemoteBag#13` | config | `config/remotebag.yml: auto_save_interval (default: 300, has no effect, see UltiKits/UltiRemoteBag#13)` | n/a | n/a | admin | brief | RemoteBagConfig#autoSaveInterval (declared, never read outside this class) | | ||
| | ultiremotebag.config.remotebag.default_pages | Number of free bag pages a player has before hitting the page limit, used only when `permission_based_pages` is false or no `permission_prefix.N` node matches | config | `config/remotebag.yml: default_pages (default: 1)` | n/a | n/a | admin | brief | RemoteBagService#getPlayerMaxPages | | ||
| | ultiremotebag.config.remotebag.economy.base_price | Base price (in Vault currency) of the first purchased bag beyond the free allotment | config | `config/remotebag.yml: economy.base_price (default: 10000)` | n/a | n/a | admin | brief | RemoteBagService#calculatePrice | | ||
| | ultiremotebag.config.remotebag.economy.enabled | Enable the purchase-a-new-bag economy feature entirely; when false (or Vault is absent), a new bag page is created for free up to the page limit instead of being purchased | config | `config/remotebag.yml: economy.enabled (default: true)` | n/a | n/a | admin | brief | RemoteBagMainGUI#provideItems, RemoteBagService#purchaseBag | |
There was a problem hiding this comment.
With economy disabled or Vault absent, players cannot reach the free-creation branch described here: the repo-wide call from RemoteBagMainGUI to purchaseBag is attached to the purchase icon, and provideItems omits that icon unless both economy and Vault are available. Opening /bag therefore offers no way to create another page in these configurations, so this inventory should describe the feature as unavailable or record the dead fallback as a defect.
Useful? React with 👍 / 👎.
- Swap the reversed default_pages/max_pages/permission_based_pages logic in FEATURES.md (permission_based_pages=false returns max_pages directly, never falls back to default_pages) - Document rows_per_page below 5 as a data-loss hazard, not a safe smaller-GUI control (#24) - Document economy.enabled=false as removing rather than freeing the purchase path, since the purchase icon is hidden without economy (#25) - Reframe the auto-save scheduled task and the on-quit save as having no observable effect: every cache-write path already saves synchronously, so neither ever catches a genuinely dirty cache entry (#23) - Fix /bag save and the persistence row: the command does not copy a currently-open GUI's live inventory into the cache before saving (#22); route the persistence row through the GUI's own Save button instead, and add a negative row proving the command's blind spot - Document the no-bags empty-state branches on /bag list and /bag see as unreachable, since getPlayerBagPages fabricates a virtual page 1 (#26) - Fix the config-yml checklist row's lock.timeout_seconds test value (10 is the range minimum, not 5) and note the boot-only application Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDELX2DH8xeytNqSmSgy6F
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0414419d8
ℹ️ 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".
| | ultiremotebag.bag.open-page | `language: en`; sender holds `ultibag.use`; the sender owns at least page `1` (any fresh player does, per `RemoteBagService#getPlayerBagPages`'s empty-list fallback to page `1`) | Run `/bag 1` | `RemoteBagContentGUI` opens for page 1 in edit mode (see `ultiremotebag.bag.content-gui` below) — the lock is acquired for the sender as OWNER | pixel | RemoteBagContentGUI | | ||
| | ultiremotebag.bag.open-page.neg-out-of-range | `language: en`; sender's max page count is the shipped default 1 (fresh player, `permission_based_pages: true`, no `ultibag.pages.N` node held) | Run `/bag 2` | Chat line reads `Page 2 out of range, max page is 1` (red) — `BagCommand#openPage`'s range check refuses before attempting to open anything | server | | | ||
| | ultiremotebag.bag.open-page.neg-not-exist | `language: en`; `permission_based_pages: false` and `default_pages: 2` (raises the range to 2 without creating page 2); sender is a fresh player who has never opened a bag GUI (existing pages therefore fall back to the singleton `[1]` per `RemoteBagService#getPlayerBagPages`) | Run `/bag 2` | Chat line reads `Bag #2 does not exist` (red, `bag_not_exist`) — `BagCommand#openPage`'s range check (1..2) passes, but the existence check fails because page 2 was never actually created | server | | | ||
| | ultiremotebag.bag.main-gui | `language: en`; the sender owns exactly one bag page (fresh player, shipped `max_pages: 10` default so the sender is under the page limit); Vault installed, `economy.enabled: true` (shipped default), and the sender's Vault balance is at least `economy.base_price` (10000, shipped default) | Run `/bag` | A 6-row inventory opens titled `<player> 's Remote Bags` (gold), listing exactly one chest icon (item/slot-usage lore) for the owned page, plus a GREEN minecart purchase icon (the balance precondition guarantees the affordable branch of `RemoteBagMainGUI#createPurchaseIcon` — a balance below the price would instead show a RED barrier icon, not exercised by this row); the previous/next navigation buttons plus a red `Close` button appear on the bottom row; the configured `sound.open` sound plays | pixel | RemoteBagMainGUI | |
There was a problem hiding this comment.
With the stated fresh-player setup, permission_based_pages remains true and no ultibag.pages.N permission is granted, so RemoteBagService#getPlayerMaxPages returns default_pages (1), not the configured max_pages (10). Since the virtual page list also has size 1, RemoteBagMainGUI#provideItems evaluates 1 < 1 as false and omits the purchase icon regardless of balance, making this row fail before exercising the affordable branch; grant a higher page permission or disable permission-based limits.
Useful? React with 👍 / 👎.
| | ultiremotebag.bag.main-gui | `language: en`; the sender owns exactly one bag page (fresh player, shipped `max_pages: 10` default so the sender is under the page limit); Vault installed, `economy.enabled: true` (shipped default), and the sender's Vault balance is at least `economy.base_price` (10000, shipped default) | Run `/bag` | A 6-row inventory opens titled `<player> 's Remote Bags` (gold), listing exactly one chest icon (item/slot-usage lore) for the owned page, plus a GREEN minecart purchase icon (the balance precondition guarantees the affordable branch of `RemoteBagMainGUI#createPurchaseIcon` — a balance below the price would instead show a RED barrier icon, not exercised by this row); the previous/next navigation buttons plus a red `Close` button appear on the bottom row; the configured `sound.open` sound plays | pixel | RemoteBagMainGUI | | ||
| | ultiremotebag.bag.content-gui | `language: en`; the sender owns at least one bag page (run `ultiremotebag.bag.open-page` above first, or purchase one via `ultiremotebag.bag.main-gui` above); `sound.enabled: true` (shipped default) | Run `/bag <page>` for an owned, currently-unlocked page | A 6-row inventory opens titled `Bag #<page>` (gold), with any previously placed items visible in slots 0-44; the bottom row (slots 45-53) shows, in order: a back button (yellow, `Back`), two background fillers, a background filler (read-only-only refresh slot, hidden in edit mode), a green save button (`Save`), a green mode indicator (`Edit Mode`), two background fillers, and a red close button (`Close`); the configured `sound.open` sound plays | pixel | RemoteBagContentGUI | | ||
| | ultiremotebag.bag.save | `language: en`; sender owns at least one bag page, NOT currently open in any GUI (close it first, or run this from a fresh login) | Run `/bag save` | Chat line reads `Bag saved manually!` (green, `bag_saved_manually`) — flushes the existing service cache; does NOT copy a currently-open content GUI's live inventory into that cache, `UltiKits/UltiRemoteBag#22` (see `.neg-open-gui-ignored` below) | server | | | ||
| | ultiremotebag.bag.save.neg-open-gui-ignored | `language: en`; sender owns at least one bag page, opened via `ultiremotebag.bag.open-page` above, with a NEW distinguishable item (e.g. a single `DIAMOND`) placed into slot 0, GUI still open, Save button NOT yet clicked | Run `/bag save` (from the chat, while the GUI remains open), then close the GUI WITHOUT clicking its own Save button (e.g. press Escape), then reopen the same page | The `DIAMOND` is GONE — `/bag save` persisted only what was already in the service cache before the item was placed; closing without the GUI's own save discards the placement entirely, despite the earlier `/bag save` reporting success. Known product defect, `UltiKits/UltiRemoteBag#22` | server | | |
There was a problem hiding this comment.
The newly added Escape route is fresh evidence that the earlier save finding remains unresolved: closing an edit-mode RemoteBagContentGUI always invokes onClose, which calls saveCurrentContents() and synchronously persists all 45 GUI slots. Consequently the DIAMOND will be present after the prescribed close and reopen, not gone; this negative check must observe persistence before any close callback or use a harness that can invoke the command without closing the inventory.
Useful? React with 👍 / 👎.
| | ultiremotebag.bag.save | `language: en`; sender owns at least one bag page, NOT currently open in any GUI (close it first, or run this from a fresh login) | Run `/bag save` | Chat line reads `Bag saved manually!` (green, `bag_saved_manually`) — flushes the existing service cache; does NOT copy a currently-open content GUI's live inventory into that cache, `UltiKits/UltiRemoteBag#22` (see `.neg-open-gui-ignored` below) | server | | | ||
| | ultiremotebag.bag.save.neg-open-gui-ignored | `language: en`; sender owns at least one bag page, opened via `ultiremotebag.bag.open-page` above, with a NEW distinguishable item (e.g. a single `DIAMOND`) placed into slot 0, GUI still open, Save button NOT yet clicked | Run `/bag save` (from the chat, while the GUI remains open), then close the GUI WITHOUT clicking its own Save button (e.g. press Escape), then reopen the same page | The `DIAMOND` is GONE — `/bag save` persisted only what was already in the service cache before the item was placed; closing without the GUI's own save discards the placement entirely, despite the earlier `/bag save` reporting success. Known product defect, `UltiKits/UltiRemoteBag#22` | server | | | ||
| | ultiremotebag.bag.persistence | `language: en`; a distinguishable item (e.g. a single `DIAMOND`) placed into slot 0 of an owned bag page, then persisted via the content GUI's OWN Save button (NOT `/bag save` — see `ultiremotebag.bag.save`'s own defect note above) | Click the content GUI's Save button, then restart the server completely (not `/ul reload`), then log back in and run `/bag <page>` for the same page | The `DIAMOND` is present in slot 0, exactly as left before the restart — `RemoteBagContentGUI#saveCurrentContents` copied the GUI's live inventory into the service cache and `RemoteBagService#loadBagIfNeeded` re-queried `remote_bags`, deserializing the stored YAML back into the same slot | server | | | ||
| | ultiremotebag.bag.auto-save | none | Read `RemoteBagService.java`, `RemoteBagContentGUI.java` directly: every call site of `setBagPage` (`RemoteBagContentGUI#saveCurrentContents`, `RemoteBagService#createBagPage`, `RemoteBagService#clearBagPage`) is followed, in the SAME method, by a synchronous `saveBag` call — confirm by reading each of the three call sites in turn | `autoSaveTask`'s own body is correct (it would persist a dirty cache entry if one existed), but no reachable code path through this module's command/GUI surface ever leaves the cache dirty for it to catch — this is a static source confirmation (protocol layer), not a live 300-second wait, because no live setup can produce the dirty-but-unsaved precondition the task is meant to act on. Known product defect, `UltiKits/UltiRemoteBag#23` | protocol | | |
There was a problem hiding this comment.
An absence of dirty item contents does not make autoSaveTask unobservable: every scheduled saveBag call loads each existing record, assigns a fresh lastUpdated value, and calls dataOperator.update, so a cached persisted page's last_updated column advances every 300 seconds. The task can also retry cache contents after a transient update failure, because that failure is logged without removing the cache entry. This static row therefore reports the opposite of actual behavior and leaves the scheduled feature untested; verify the timestamp write or otherwise document the unconditional update.
Useful? React with 👍 / 👎.
| | ultiremotebag.bag.save.neg-open-gui-ignored | `language: en`; sender owns at least one bag page, opened via `ultiremotebag.bag.open-page` above, with a NEW distinguishable item (e.g. a single `DIAMOND`) placed into slot 0, GUI still open, Save button NOT yet clicked | Run `/bag save` (from the chat, while the GUI remains open), then close the GUI WITHOUT clicking its own Save button (e.g. press Escape), then reopen the same page | The `DIAMOND` is GONE — `/bag save` persisted only what was already in the service cache before the item was placed; closing without the GUI's own save discards the placement entirely, despite the earlier `/bag save` reporting success. Known product defect, `UltiKits/UltiRemoteBag#22` | server | | | ||
| | ultiremotebag.bag.persistence | `language: en`; a distinguishable item (e.g. a single `DIAMOND`) placed into slot 0 of an owned bag page, then persisted via the content GUI's OWN Save button (NOT `/bag save` — see `ultiremotebag.bag.save`'s own defect note above) | Click the content GUI's Save button, then restart the server completely (not `/ul reload`), then log back in and run `/bag <page>` for the same page | The `DIAMOND` is present in slot 0, exactly as left before the restart — `RemoteBagContentGUI#saveCurrentContents` copied the GUI's live inventory into the service cache and `RemoteBagService#loadBagIfNeeded` re-queried `remote_bags`, deserializing the stored YAML back into the same slot | server | | | ||
| | ultiremotebag.bag.auto-save | none | Read `RemoteBagService.java`, `RemoteBagContentGUI.java` directly: every call site of `setBagPage` (`RemoteBagContentGUI#saveCurrentContents`, `RemoteBagService#createBagPage`, `RemoteBagService#clearBagPage`) is followed, in the SAME method, by a synchronous `saveBag` call — confirm by reading each of the three call sites in turn | `autoSaveTask`'s own body is correct (it would persist a dirty cache entry if one existed), but no reachable code path through this module's command/GUI surface ever leaves the cache dirty for it to catch — this is a static source confirmation (protocol layer), not a live 300-second wait, because no live setup can produce the dirty-but-unsaved precondition the task is meant to act on. Known product defect, `UltiKits/UltiRemoteBag#23` | protocol | | | ||
| | ultiremotebag.bag.cleanup-on-quit | `language: en`; the player has run `ultiremotebag.bag.open-page` above and is holding the OWNER lock on that page, GUI still open (no item placement needed — only the lock-release effect is independently testable, per `UltiKits/UltiRemoteBag#23`) | Quit the server (disconnect, not kick) while still holding the lock | `BagListener#onPlayerQuit` released every lock the quitting player held — confirmed by a second, OP'd admin running `ultiremotebag.admin.see` above against the same page afterward and receiving EDIT mode (not read-only/blocked) immediately, with no stale lock | server | | |
There was a problem hiding this comment.
The revised lock-only assertion still cannot attribute the result to BagListener#onPlayerQuit: the precondition deliberately leaves RemoteBagContentGUI open, and disconnecting closes that inventory, whose onClose independently calls lockService.release for the same lock. Thus the subsequent admin receiving edit mode passes even if the quit listener's releaseAll is broken; use a lock established without an open content GUI or inspect another effect specific to the listener.
Useful? React with 👍 / 👎.
| | ultiremotebag.admin.clear.neg-locked | `language: en`; admin holds `ultibag.use` AND `ultibag.admin.clear`; the target player currently holds the OWNER edit lock on the page (target has it open in edit mode) | Run `/bag clear <target> <page>` | Chat line reads `Bag is in use, cannot clear` (red, `bag_in_use_cannot_clear`) — `BagLockService#canUpgradeToEdit` refused because the owner's lock has not expired; the page's contents are UNCHANGED | server | | | ||
| | ultiremotebag.admin.create | `language: en`; admin holds `ultibag.use` AND `ultibag.admin.create`; target player has played before | Run `/bag create <target>` | Chat line reads `Created bag #<N> for player <target>` (green, `admin_bag_created`), where `<N>` is one past the target's highest existing page number; `/bag list <target>` immediately afterward shows the new page | server | | | ||
| | ultiremotebag.admin.create.neg-never-played | `language: en`; admin holds `ultibag.use` AND `ultibag.admin.create`; `<name>` has never played on this server | Run `/bag create <name>` | Chat line reads `Player not found: <name>` (red, `player_not_found`) | server | | | ||
| | ultiremotebag.admin.delete | `language: en`; admin holds `ultibag.use` AND `ultibag.admin.delete`; target player has an unlocked bag page that exists | Run `/bag delete <target> <page>` | Chat line reads `Deleted bag #<page> of player <target>` (green, `admin_bag_deleted`); `/bag list <target>` immediately afterward no longer lists that page, and its `remote_bags` row is gone | server | | |
There was a problem hiding this comment.
When the target has only page 1—the simplest state allowed by these preconditions—deleting it removes the database row, but the follow-up /bag list immediately fabricates and displays virtual page 1 through getPlayerBagPages's empty-cache fallback. The expected claim that the deleted page is no longer listed therefore fails in that common scenario; require another real page and delete a non-1 page, or verify deletion solely through the database.
Useful? React with 👍 / 👎.
| | ultiremotebag.config.remotebag.save_on_close | Declared as a toggle for whether closing the content GUI in edit mode saves; `RemoteBagContentGUI#onClose` calls `saveCurrentContents()` unconditionally whenever in edit mode, with no check of this key anywhere. Known product defect, `UltiKits/UltiRemoteBag#18` | config | `config/remotebag.yml: save_on_close (default: true, has no effect, see UltiKits/UltiRemoteBag#18)` | n/a | n/a | admin | brief | RemoteBagConfig#saveOnClose (declared, never read outside this class) | | ||
| | ultiremotebag.config.remotebag.sound.close | Sound effect (XSound-matched name; an unrecognized name is silently ignored) played when the content GUI closes in edit mode | config | `config/remotebag.yml: sound.close (default: BLOCK_CHEST_CLOSE)` | n/a | n/a | admin | brief | SoundUtil#playCloseSound | | ||
| | ultiremotebag.config.remotebag.sound.enabled | Master switch for every sound effect this module plays; when false, none of the other `sound.*` keys have any effect | config | `config/remotebag.yml: sound.enabled (default: true)` | n/a | n/a | admin | brief | SoundUtil#playSound | | ||
| | ultiremotebag.config.remotebag.sound.error | Sound effect played on a refused action (out-of-range page, blocked lock, invalid target, insufficient balance) | config | `config/remotebag.yml: sound.error (default: ENTITY_VILLAGER_NO)` | n/a | n/a | admin | brief | SoundUtil#playErrorSound | |
There was a problem hiding this comment.
sound.error is not played for two refusal cases listed here: BagCommand#openPage returns directly for an out-of-range page, and each never-played target branch sends player_not_found and returns without calling SoundUtil.playErrorSound. An operator testing this documented configuration behavior with either scenario will incorrectly conclude that the sound setting is broken; restrict the examples to refusal paths that actually invoke playErrorSound, such as a blocked open or failed purchase.
Useful? React with 👍 / 👎.
- Fix the main-gui purchase-icon precondition: permission_based_pages must be false, or default_pages (not max_pages) governs the limit - Rewrite the /bag save negative row to query the SQLite database directly rather than closing the GUI, which itself saves via onClose - Reverse the auto-save row's "no effect" claim: the task unconditionally refreshes last_updated on every cached page every run, which is the actual observable effect, even though it never catches a genuinely dirty-but-unsaved entry (comment added to #23 narrowing the claim) - Rewrite the cleanup-on-quit row to test cache eviction (clearCache), the one effect not shared with onClose's own independent lock release - Fix the admin.delete row to delete a second page rather than the target's only page, which the virtual-page-1 fallback would re-fabricate - Narrow the sound.error row to the refusals that actually call playErrorSound; the out-of-range and never-played branches do not Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDELX2DH8xeytNqSmSgy6F
|
Fixed all 6 findings from round 2 (commit 1b8653f). Per this milestone's D-24 review-loop rule, this is the third round; if a further wrong-verdict finding lands I will fix it once more and then stop requesting rounds regardless of outcome, recording anything still open in the SUMMARY. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b8653f5db
ℹ️ 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".
- state the retry-after-a-previously-failed-write path auto-save's own dirty-cache defense actually reaches (cleanup-on-quit and auto-save rows) - point the two database checks at the framework's real internal-plugin path (<core data folder>/sqliteDB/<plugin>.db), not a per-module folder - require economy.price_increase_enabled: false in the main-gui purchase precondition so a base_price balance actually reaches the affordable branch against a fresh player's page-2 price - set max_pages (not default_pages) to raise the range in the open-page.neg-not-exist precondition, since permission_based_pages: false never consults default_pages - add the read-only refresh-button's failed-upgrade case to sound.error's ONLY list Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDELX2DH8xeytNqSmSgy6F
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c85f67149
ℹ️ 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".
| |---|---|---|---|---|---|---|---|---| | ||
| | ultiremotebag.config.remotebag.auto_save_interval | Declared as the auto-save task's interval in seconds; never read — the real cadence is a hardcoded 6000-tick (300s) `@Scheduled` period regardless of this key's value. Known product defect, `UltiKits/UltiRemoteBag#13` | config | `config/remotebag.yml: auto_save_interval (default: 300, has no effect, see UltiKits/UltiRemoteBag#13)` | n/a | n/a | admin | brief | RemoteBagConfig#autoSaveInterval (declared, never read outside this class) | | ||
| | ultiremotebag.config.remotebag.default_pages | Fallback page limit used ONLY when `permission_based_pages` is true AND the player holds no `permission_prefix.N` node at all; consulted only inside that branch, never when `permission_based_pages` is false | config | `config/remotebag.yml: default_pages (default: 1)` | n/a | n/a | admin | brief | RemoteBagService#getPlayerMaxPages | | ||
| | ultiremotebag.config.remotebag.economy.base_price | Base price (in Vault currency) of the first purchased bag beyond the free allotment | config | `config/remotebag.yml: economy.base_price (default: 10000)` | n/a | n/a | admin | brief | RemoteBagService#calculatePrice | |
There was a problem hiding this comment.
With the shipped price escalation enabled, the first reachable purchase is passed to calculatePrice as bag number 2 because the virtual free page 1 is included in existingPages.size(), so it costs ceil(base_price * 1.1) rather than base_price. Calling this key the price of the first purchased bag gives operators the wrong expected charge; describe it as the formula's bag-1 base instead.
Useful? React with 👍 / 👎.
| | ultiremotebag.config.remotebag.economy.price_increase_rate | Per-bag price escalation rate applied as `basePrice * (1 + rate)^(n-1)` for the n-th purchased bag | config | `config/remotebag.yml: economy.price_increase_rate (default: 0.1)` | n/a | n/a | admin | brief | RemoteBagService#calculatePrice | | ||
| | ultiremotebag.config.remotebag.gui_title | Declared as a custom GUI title template with `{PAGE}`/`{MAX}` placeholders; never read — both bag GUI classes build their titles from hardcoded i18n keys (`gui_main_title`, `bag_name`) instead. Known product defect, `UltiKits/UltiRemoteBag#14` | config | `config/remotebag.yml: gui_title (default: a Chinese-language template meaning "Remote Bag, page {PAGE}/{MAX}", has no effect, see UltiKits/UltiRemoteBag#14)` | n/a | n/a | admin | brief | RemoteBagConfig#guiTitle (declared, never read outside this class) | | ||
| | ultiremotebag.config.remotebag.lock.notify_readonly_viewers | Declared as a toggle for whether a read-only viewer is notified when the bag's owner starts using it; `BagLockService#notifyReadOnlyAdmins` runs unconditionally whenever the owner acquires the lock, regardless of this key's value. Known product defect, `UltiKits/UltiRemoteBag#19` | config | `config/remotebag.yml: lock.notify_readonly_viewers (default: true, has no effect, see UltiKits/UltiRemoteBag#19)` | n/a | n/a | admin | brief | RemoteBagConfig#notifyReadonlyViewers (declared, never read outside this class) | | ||
| | ultiremotebag.config.remotebag.lock.timeout_seconds | Seconds an edit or admin lock may sit idle (holder not actively closing the GUI) before it is treated as expired and released to the next opener | config | `config/remotebag.yml: lock.timeout_seconds (default: 300)` | n/a | n/a | admin | brief | UltiRemoteBag#registerSelf (reads at boot via `lockService.setLockTimeout`), BagLockService#setLockTimeout | |
There was a problem hiding this comment.
BagLockInfo#acquiredAt is set only when the lock is created, and no GUI interaction refreshes it, so an actively used bag also becomes available to another editor after timeout_seconds. Describing this as an idle timeout can lead operators to choose a short value expecting activity to preserve exclusivity; state that it is elapsed time since acquisition.
Useful? React with 👍 / 👎.
| | ultiremotebag.config.remotebag.gui_title | Declared as a custom GUI title template with `{PAGE}`/`{MAX}` placeholders; never read — both bag GUI classes build their titles from hardcoded i18n keys (`gui_main_title`, `bag_name`) instead. Known product defect, `UltiKits/UltiRemoteBag#14` | config | `config/remotebag.yml: gui_title (default: a Chinese-language template meaning "Remote Bag, page {PAGE}/{MAX}", has no effect, see UltiKits/UltiRemoteBag#14)` | n/a | n/a | admin | brief | RemoteBagConfig#guiTitle (declared, never read outside this class) | | ||
| | ultiremotebag.config.remotebag.lock.notify_readonly_viewers | Declared as a toggle for whether a read-only viewer is notified when the bag's owner starts using it; `BagLockService#notifyReadOnlyAdmins` runs unconditionally whenever the owner acquires the lock, regardless of this key's value. Known product defect, `UltiKits/UltiRemoteBag#19` | config | `config/remotebag.yml: lock.notify_readonly_viewers (default: true, has no effect, see UltiKits/UltiRemoteBag#19)` | n/a | n/a | admin | brief | RemoteBagConfig#notifyReadonlyViewers (declared, never read outside this class) | | ||
| | ultiremotebag.config.remotebag.lock.timeout_seconds | Seconds an edit or admin lock may sit idle (holder not actively closing the GUI) before it is treated as expired and released to the next opener | config | `config/remotebag.yml: lock.timeout_seconds (default: 300)` | n/a | n/a | admin | brief | UltiRemoteBag#registerSelf (reads at boot via `lockService.setLockTimeout`), BagLockService#setLockTimeout | | ||
| | ultiremotebag.config.remotebag.max_pages | Hard ceiling on bag pages a player may ever hold. When `permission_based_pages` is false this is the player's page limit OUTRIGHT (returned directly, `default_pages` never consulted); when true it is both the top of the `permission_prefix.N` scan range and the ceiling any matched node can return | config | `config/remotebag.yml: max_pages (default: 10)` | n/a | n/a | admin | brief | RemoteBagService#getPlayerMaxPages | |
There was a problem hiding this comment.
An admin can repeatedly invoke BagCommand#createBag, whose RemoteBagService#createBagPage path performs no max_pages check, so a player can hold more than this value. The same inventory already describes admin creation as bypassing the normal limit; document max_pages as the player access/purchase ceiling rather than a hard ceiling on pages the player may ever hold.
Useful? React with 👍 / 👎.
| | ultiremotebag.bag.open-page | Open a specific owned bag page directly by number, acquiring the owner lock; refuses with a range error if the page number is outside `1..maxPages`, or a not-found error if that page does not yet exist for this player | command | `/bag <page>` | ultibag.use | player | player | brief | BagCommand#openPage | | ||
| | ultiremotebag.bag.save | Manually persist the sender's currently CACHED bag pages to the database. Does NOT copy a currently-open content GUI's live inventory into the cache first — only `RemoteBagContentGUI#saveCurrentContents` (the GUI's own Save button, or `onClose` in edit mode) does that copy, so an item placed since the GUI was opened is not persisted by this command until the GUI itself is saved or closed. Known product defect, `UltiKits/UltiRemoteBag#22` | command | `/bag save` | ultibag.use | player | player | detailed | BagCommand#saveBag | | ||
| | ultiremotebag.bag.main-gui | The paginated main page listing every owned bag (chest icon per page showing item/slot counts) plus, when under the page limit and Vault-backed economy is available, a purchase icon whose color and icon (minecart vs. barrier) reflect whether the sender can currently afford the next bag's price | gui | opened by `/bag` (`ultiremotebag.bag.open`) | n/a | n/a | player | brief | RemoteBagMainGUI#provideItems | | ||
| | ultiremotebag.bag.content-gui | The single bag page's content grid (45 slots, rows 1-5) plus a fixed toolbar (back, refresh, save, mode indicator, close) on row 6; item movement is allowed only in edit mode, refused with a message and sound in read-only mode | gui | opened by `/bag <page>` (owner, edit mode) or by an admin's `see`/`create`/`delete`(no)/`clear`(no) flow (owner or read-only, per the current lock state) | n/a | n/a | player | brief | RemoteBagContentGUI#setupContent | |
There was a problem hiding this comment.
Running /bag create <player> only calls createBagPage and sends a chat message; it never constructs or opens RemoteBagContentGUI. Only the owner open command and the two admin see mappings reach this GUI, so listing the create flow here gives UAT dispatchers a nonexistent route.
Useful? React with 👍 / 👎.
| | ultiremotebag.admin.clear.neg-locked | `language: en`; admin holds `ultibag.use` AND `ultibag.admin.clear`; the target player currently holds the OWNER edit lock on the page (target has it open in edit mode) | Run `/bag clear <target> <page>` | Chat line reads `Bag is in use, cannot clear` (red, `bag_in_use_cannot_clear`) — `BagLockService#canUpgradeToEdit` refused because the owner's lock has not expired; the page's contents are UNCHANGED | server | | | ||
| | ultiremotebag.admin.create | `language: en`; admin holds `ultibag.use` AND `ultibag.admin.create`; target player has played before | Run `/bag create <target>` | Chat line reads `Created bag #<N> for player <target>` (green, `admin_bag_created`), where `<N>` is one past the target's highest existing page number; `/bag list <target>` immediately afterward shows the new page | server | | | ||
| | ultiremotebag.admin.create.neg-never-played | `language: en`; admin holds `ultibag.use` AND `ultibag.admin.create`; `<name>` has never played on this server | Run `/bag create <name>` | Chat line reads `Player not found: <name>` (red, `player_not_found`) | server | | | ||
| | ultiremotebag.admin.delete | `language: en`; admin holds `ultibag.use` AND `ultibag.admin.delete`; target player has AT LEAST TWO unlocked bag pages (run `ultiremotebag.admin.create` above first, so the target has page 1 plus a second page), deleting the SECOND (non-1) page — deleting the target's only page (page 1) would not demonstrate removal at all, since `getPlayerBagPages`'s empty-cache fallback re-fabricates a virtual page 1 immediately afterward (`UltiKits/UltiRemoteBag#26`) | Run `/bag delete <target> <page>` naming the SECOND page number | Chat line reads `Deleted bag #<page> of player <target>` (green, `admin_bag_deleted`); `/bag list <target>` immediately afterward lists only page 1 (the second page is gone), and the deleted page's `remote_bags` row is gone | server | | |
There was a problem hiding this comment.
The revised admin.create setup is fresh evidence that the earlier deletion-test issue remains: for a played target with no stored pages, getPlayerBagPages returns virtual [1], but createBagPage then inserts only page 2, leaving no real page 1. Deleting page 2 therefore empties the database and /bag list fabricates page 1 afterward, so the row passes without proving that deletion preserved another page; save/materialize page 1 before creating page 2.
Useful? React with 👍 / 👎.
1 participant
Summary
Catalogues UltiRemoteBag's full operator- and player-visible surface by reading every file under
src/main/javaandsrc/main/resourcesin full (14 Java files, 2 lang files,plugin.yml):FEATURES.md(39 rows: 9 commands, 2 GUI pages, 1 event, 1 scheduled task, 2 persistenceguarantees, 24 configuration keys) and
UAT-CHECKLIST.md(26 executable rows, 11 of themnegative cases exercising a specific handler branch). Adds
.github/pull_request_template.md(new — this repository had none), copied verbatim from the merged
UltiChatworked example, soevery future feature change is required to update both documents. Zero code, POM, or workflow
change (Phase 10, D-05).
Both Phase 9 GUI-excluded classes (
RemoteBagMainGUI,RemoteBagContentGUI) are named in atleast one
Coverscell. The bag-survives-a-restart persistence row is the module's own prioritycase per the plan ("a remote bag that silently loses items is the failure a server owner cares
about") and is ordered after the row that fills the bag.
While reading, 8 product defects were found and filed (not fixed, per D-20) —
UltiKits/UltiRemoteBag#13-#20: sevenRemoteBagConfigkeys that are declared, validated, anddocumented but never read by any production code (
auto_save_interval,gui_title,messages.no_permission,messages.page_locked,messages.bag_saved,save_on_close,lock.notify_readonly_viewers), and one message-hardcoding defect (BagOpenResult'slock-blocked/read-only refusal text ignores the
languagesetting entirely, always SimplifiedChinese).
Issue closure
None — the 8 issues filed above are recorded as found, not closed, by this pull request.
Verification
Reconciliation counts, both from
git -C UltiRemoteBagat this head, using the canonical family:@CmdExecutor= 1,@CmdMapping= 9@EventListener(classes) = 1,@EventHandler(handler methods) = 1@Scheduled= 1@ConditionalOnConfig= 0@ConfigEntity(classes) = 1,@ConfigEntry(keys) = 24@Tableentities = 1 (RemoteBagData)extends BaseInventoryPage/BasePaginationPage) = 2 (RemoteBagMainGUI,RemoteBagContentGUI)grep -rn "registerPlaceholder\|PlaceholderExpansion"returns nothing — the module consumes Vault economy balances, it registers no expansion of its own)Shape checks (all pass on this head):
Mechanical D-26/D-27a sweep (Python script over both documents, re-run on this head):
Annotation-site reconciliation
@CmdExecutor(class-level)permission/target properties are folded into each@CmdMappingrow below it, not a row of its own@CmdMappingcommand)see <player> <page>mapping atBagCommand.java:139, easy to conflate with the one-argumentsee <player>mapping three lines above it (BagCommand.java:115) — confirmed present by name, not merely by count@EventListener(class)BagListeneris the sole listener class; its one handler method is the unit FEATURES.md rows on@EventHandler(handler methods)event)BagListener#onPlayerQuit@Scheduledscheduled)RemoteBagService#autoSaveTask@ConditionalOnConfiggate)@ConfigEntity(classes)RemoteBagConfigis the sole config class; its 24 fields are the unit FEATURES.md rows on@ConfigEntry(keys)config)@Tableentitiesultiremotebag.bag.persistence's Source cell (RemoteBagData#RemoteBagData)gui)RemoteBagMainGUI,RemoteBagContentGUI— both are also this module's two Phase 9 GUI-exclusion entries, each named in aUAT-CHECKLIST.mdCoverscellEconomyUtils); it registers no expansion of its ownByte-identity proof (gate 3)
Two clean builds,
env -u JAVA_TOOL_OPTIONS mvn -B -q clean package -DskipTests, one onorigin/master(via a disposable worktree) and one on this PR's head:origin/master(e85f022):c5e86fbb3a2f12a132a7ead2fd8682714ce4b672090e4be604404ac8eef8a08c9c85f67, after the round-3 sweep fix):bd00a39339531c8b0e2a1dcd963b14eed8ac449624081da73b5249a23ff4f529The two SHA-256 values differ, as expected for a Markdown-only change — a per-entry CRC diff
(
unzip -lv) confirms the only entry that differs between the two archive listings isMETA-INF/maven/.../pom.properties, whose build timestamp (2026-09-06 17:46onorigin/master'sprior build vs.
2026-09-09 15:5xon this head's rebuild) is the usual source of jar-to-jarvariance; every other one of 39 entries (name, size, CRC) is byte-identical, including all 15
compiled classes,
plugin.yml, and bothlang/*.ymlcatalogues. The packaged jar's content isunchanged; the outer hash difference would appear even if it were not, which is why the CRC-level
diff is the actual proof, not the top-level SHA-256 pair. Re-verified after the round-3 fix commit
(
9c85f67), which touched onlyFEATURES.mdandUAT-CHECKLIST.md.Codex review rounds (D-24/D-26 loop)
Three rounds after the pre-open sweep, per the D-26 protocol (fix a swept class in one commit,
request one more round; stop once the head carries no fix-it P2):
b041441): 7 findings fixed, 5 new product defects filed and not fixed(
UltiKits/UltiRemoteBag#22..#26).1b8653f): 6 more findings fixed (siblings of the same swept classes: vacuous-pass onthe no-bags branch, wrong Source, unconstrained precondition, literal-mismatch on the free-page
fallback).
9c85f67, this commit): 5 more findings fixed — a purchase-price precondition(
price_increase_enabled), the auto-save retry-after-failure nuance, the correct internal-pluginSQLite path,
max_pagesvs.default_pagesin the not-exist row, and the refresh-button'sfailed-upgrade case in
sound.error's ONLY list. All 5 threads replied to and resolved.Per D-24/D-26, this is the second confirmation round after the initial sweep — the loop stops here.
No fourth round requested.
Checklist
masterfile/grep -c $'\r'both confirm 0 CR bytes inFEATURES.md,UAT-CHECKLIST.md,.github/pull_request_template.md).github/cjk-allowlist.txt— both new documents are 0-CJK (grep -qP '[\x{4e00}-\x{9fff}]'returns no match), verified after replacing the module's Chinese-language shipped config defaults with English glossesFEATURES.mdandUAT-CHECKLIST.mdupdated for every feature change in this PR — N/A, this PR makes no feature change; it is the first authoring of both documentsGate 2 on this repository is the Codex review only — Codacy is not onboarded on module
repositories (per the monorepo
CLAUDE.md's mandatory pre-merge gates table).D-26/D-27a sweep tally
Swept every row of both documents against the eight defect classes before opening this pull
request:
Expectedfor a positive, non-empty observable; tightenedultiremotebag.bag.main-gui's precondition (balance ≥ price) so the purchase-icon branch is deterministic rather than ambiguous between the affordable/unaffordable outcomes.lang/en.yml's actual (non-MessageFormat) template and substitution order — 0 mismatches found (all matched on first read).Expectedoutcome, so no split was needed.Preconditionsguarantees the state itsStepsact on; 1 fix (ultiremotebag.bag.open-page.neg-not-existoriginally depended on an admin fixture from a later section — rewritten to use a config-only precondition instead).FEATURES.mdSource cell re-read against the cited class; 3 fixes (a bareRemoteBagDataclass reference corrected toRemoteBagData#RemoteBagData; twoSoundUtil#playSound(...)overload-disambiguated signatures simplified to the bare method name after they were found to break comma-delimited parsing).X AND Y, for the six admin sub-commands with cumulative class+method permission checks) is explicitly documented as an extension in this document's own Conventions block, not a silent deviation.Expectedchecked against its ownSteps; 0 found (the one apparent case,ultiremotebag.bag.lock-not-persisted, is a static source-reading row whoseStepsperform the exact grep theExpectedreports on).ultiremotebag.bag.auto-save/.persistencebefore the rows they depend on — reordered to dependency order, documented as a deliberate departure from strict ID-ascending order in the Conventions block) plus 2 further forward references discovered in the same pass (.neg-not-existciting an Admin-section fixture, and a since-deletedcontent-gui.neg-readonly-no-moverow citingadmin.see, merged into the existingadmin.see.neg-readonly-then-owner-blockedrow instead of kept as a separate forward-citing row).🤖 Generated with Claude Code
https://claude.ai/code/session_01NDELX2DH8xeytNqSmSgy6F