[darbyjack]

/guild create runs a set of checks, shows a confirmation prompt and registers a pending action. When the player answered the prompt, the confirmation trusted the answers the command had collected, so anything that changed in between went through.

What the unfixed code did

The membership check is the NoGuild condition on the command, and it is evaluated when the command runs. A player who joined another guild before confirming was put into the new guild as well, while the guild they had joined kept them on its member list, and the member cache was pointed at the new one. The two guilds then disagreed about where the player was.

The name was checked with GuildInputValidator.isNameTaken at command time only. Another guild claiming the name in between produced a second guild with the same visible name, which is what every command that resolves a guild by name has to disambiguate.

The blacklist, the name and prefix requirements and the balance were checked once, at command time. Only the balance was re-checked at confirmation.

Against the unfixed code the two new E2E cases failed with Expected message matching "You're already in a guild!" not received and the same for This name is already taken!. The plugin's own storage was left holding E2E0009joined, a guild for a player who was already in one, and a second guild named E2E000ctaken next to the one that already held the name.

The fix

CommandCreate now has a single checkCreationAllowed method, which the command path and the confirmation path both call. The confirmation re-runs the same validators and refuses with the same message keys the command path uses, so the player reads the refusal they would have got at command time:

• the player is in a guild by then: error.already-in-guild
• another guild holds the name: create.guild-name-taken
• the name is on the blacklist: error.blacklist
• the name breaks the requirements: create.requirements
• the prefix breaks the requirements: create.prefix-too-long, or create.name-too-long when the name has to serve as the prefix
• the balance no longer covers the price: error.not-enough-money

A refused confirmation drops the pending action. The offer is void once it has been refused, and leaving the action registered meant a later /guild confirm could create the guild and charge for it long after the player had been told it could not be created.

The quoted price is still what gets charged. Reading cost.creation again at confirmation time would charge the player an amount they were never shown, so the price stays as quoted and affordability is what gets re-checked.

How this composes with the economy change

PR #847 takes the creation cost before the guild is added to the handler, so a provider that refuses the withdrawal leaves the player with no guild and no membership. The two changes sit in the same method and neither undoes the other. In the merged result the confirmation runs in this order:

1. the checks are re-run (this PR)
2. the guild is built and GuildCreateEvent is fired
3. the money is taken (fix(bank): read the Vault economy response before committing bank changes #847)
4. the guild is added to the handler
5. the player goes into the member cache and gets the guild and role permissions

Two rules hold in that order. Nothing is charged before the checks pass, because the money moves in step 3 and the checks are step 1. Nothing is added to the guild before the money moved, because the guild is added in step 4.

A failed check throws before step 3, so the player is neither charged nor given a guild. A refused withdrawal returns before step 4, so there is no guild and no membership either. Both paths drop the pending action, so a refused confirmation leaves nothing to confirm a second time.

Each side pins its own rule with a test. The E2E case in this PR asserts that a refused confirmation leaves the balance untouched, which fails if the payment is ever moved in front of the checks. EconomyTransactionTest in #847 asserts that a refused payment adds no guild and no membership, which fails if the guild is ever added before the money moves. The two branches merge cleanly. The merged tree compiles, passes 78 unit tests, and passes the same 146 E2E tests.

Tests

Two cases in lifecycle.spec.ts:

• refuses the confirmation when the player joined a guild in the meantime opens the prompt, has the player join another guild, then confirms. It expects error.already-in-guild, no guild with the proposed name, no guild the player masters, the other guild holding exactly its two members, and an unchanged balance. A second /guild confirm is expected to answer confirm.error, which covers the dropped action.
• refuses the confirmation when another guild took the name in the meantime opens the prompt, has another player claim the name, then confirms. It expects create.guild-name-taken, exactly one guild with that name and the other player as its master, the player in no guild at all, and an unchanged balance.

Results

• ./gradlew test testJava11 spotlessCheck: 68 unit tests pass on Java 21 and the same 68 pass on Java 11, spotless clean.
• ./gradlew check: passes, including the 1.8.8 legacy Material probe.
• ./gradlew plugwrightTest on Paper 1.21.8 with TEST_TIMEOUT=60000: 146 passed, 0 failed, 0 skipped, 7m 26s.
• The merged tree of this branch with fix(bank): read the Vault economy response before committing bank changes #847 compiles, passes 78 unit tests including the 10 cases in EconomyTransactionTest, and passes the 146 E2E tests.

Compatibility

No dependency, API or configuration change. The refusals reuse the Messages keys the command path already used, so the player-visible strings are the ones the E2E suite already asserts on. GuildInputValidator was not touched. The build still targets Java 11 bytecode, and check runs the 1.8.8 legacy Material probe, which passes.

What a failed save can lose

A new guild is not written to disk when it is created. guildHandler.addGuild puts it in the in-memory map, and the file appears with the next save: the autosave task, which runs every storage.save-interval minutes, one minute by default, or the save on shutdown. A server that dies in that window comes back without the guild, and the guild's bank and vault items go with it. The money has already left the player's balance by then, so a player can end up having paid for a guild that no longer exists. Whether the economy plugin has persisted that balance by the time the server comes back is the economy plugin's own business.

A save that fails does no damage on its own. JsonFileUtils.writeAtomically writes a temporary file and moves it into place, so a write that fails leaves the previous file untouched, and saveGuilds logs the guild that failed and carries on with the others. The guild stays in the map, so a later successful save writes it. If the failure persists, the guild is never written and is lost at the next restart, and the player is never told: the creation was reported as successful when it happened.

Nothing in the create path can overwrite an existing record. The new guild gets a fresh UUID, and createGuild returns without writing when a file with that id is already there.

The same shape applies to the leave and kick actions. A member who leaves is taken out of the guild's member list in memory, and the file that still lists them is rewritten at the next save, so a crash in that window brings the player back into the guild after the restart: loadGuilds rebuilds the member cache from that file.

A master who leaves, and a guild that is deleted, are handled differently. The guild's file is removed at once, so the disband lands immediately. The JSON provider ignores the result of file.delete(), though, so a read-only folder or a locked file leaves the file on disk without a word in the log. The guild is no longer in the map, so the autosave never touches it, and the guild comes back on the next restart with its master, its bank and its vault items.

That is the storage layer's business rather than the confirmation's, and #846, #850 and #845 own that code, so this PR reports it and leaves it alone.

Limitations

• Guild capacity is not re-checked. A new guild holds one member, its master, on the tier the command used, and the command path does not check capacity either.
• The other confirmable actions keep their current behaviour. This covers the creation flow only. The leave action still holds the guild it was issued for, so a player who leaves and joins another guild before confirming is removed from the first one.
• A player who was short of money at confirmation time has to run /guild create again, because the refused confirmation drops the action. /guild upgrade keeps its action on a money refusal, and that is unchanged.