Replace /tfmc <track> promote|demote with /tfmc promote|demote

src/main/java/net/tfminecraft/tfmccore/tfmc/BukkitTfmcActions.java

@@ -2,6 +2,7 @@
 
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentHashMap;
@@ -25,6 +26,7 @@
 import net.luckperms.api.context.ImmutableContextSet;
 import net.luckperms.api.model.user.User;
 import net.luckperms.api.node.NodeType;
+import net.luckperms.api.node.types.InheritanceNode;
 import net.luckperms.api.track.Track;
 import net.luckperms.api.track.TrackManager;
 import net.tfminecraft.tfmccore.TFMCCore;
@@ -98,6 +100,23 @@ public CompletableFuture<Void> setPermission(Player player, String permission, b
         });
     }
 
+    @Override
+    public Optional<String> highestTrack(Player player, List<String> tracks) {
+        LuckPerms api = luckPerms.get();
+        // The cached user is fine for reading; stepTrack reloads it before changing anything
+        User user = api == null ? null : api.getUserManager().getUser(player.getUniqueId());
+        if (user == null) {
+            return Optional.empty();
+        }
+        for (String name : tracks.reversed()) {
+            Track track = api.getTrackManager().getTrack(name);
+            if (track != null && onTrack(user, track)) {
+                return Optional.of(name);
+            }
+        }
+        return Optional.empty();
+    }
+
     @Override
     public CompletableFuture<TrackStep> stepTrack(Player player, String trackName, boolean promote) {
         LuckPerms api = luckPerms.get();
@@ -112,6 +131,11 @@ public CompletableFuture<TrackStep> stepTrack(Player player, String trackName, b
         // Global context, like "lp user <player> promote|demote <track>"
         ImmutableContextSet global = api.getContextManager().getContextSetFactory().immutableEmpty();
         return withFreshUser(api, player, user -> {
+            // Promoting someone off the track would add them to its first group
+            if (!onTrack(user, track)) {
+                LOGGER.warning(player.getName() + " is not on " + trackName + "; not changing their groups");
+                return CompletableFuture.completedFuture(TrackStep.unchanged());
+            }
             TrackStep step;
             String status;
             if (promote) {
@@ -187,6 +211,19 @@ private <T> CompletableFuture<T> withFreshUser(LuckPerms api, Player player, Fun
         return result;
     }
 
+    /**
+     * Whether the user's own stored global groups, the ones track steps act on, include one on
+     * the track. Transient groups are left out: a step would save them as permanent.
+     */
+    private static boolean onTrack(User user, Track track) {
+        return user.data().toCollection().stream()
+                .filter(NodeType.INHERITANCE::matches)
+                .map(NodeType.INHERITANCE::cast)
+                .filter(node -> node.getValue() && node.getContexts().isEmpty())
+                .map(InheritanceNode::getGroupName)
+                .anyMatch(track::containsGroup);
+    }
+
     private static CompletableFuture<Void> save(LuckPerms api, User user) {
         // The lp command pushes changes to the other servers; the API leaves that to us
         return api.getUserManager().saveUser(user)

src/main/java/net/tfminecraft/tfmccore/tfmc/TfmcActions.java

@@ -39,7 +39,13 @@ public static TrackStep unchanged() {
     /** Sets a LuckPerms permission to true or false for the player in this server's context. */
     CompletableFuture<Void> setPermission(Player player, String permission, boolean granted);
 
-    /** Moves the player one step along a LuckPerms track. */
+    /**
+     * The last of {@code tracks} that holds one of the player's own global groups. Group
+     * permissions cannot answer this, because the {@code *} on some groups matches every track.
+     */
+    Optional<String> highestTrack(Player player, List<String> tracks);
+
+    /** Moves the player one step along a LuckPerms track they are already on. */
     CompletableFuture<TrackStep> stepTrack(Player player, String track, boolean promote);
 
     /** Lets the player fly at {@code speed} and returns their previous flight settings. */

src/main/java/net/tfminecraft/tfmccore/tfmc/TfmcCommand.java

@@ -99,18 +99,12 @@ public LiteralCommandNode<CommandSourceStack> build() {
                         .then(Commands.argument("reason", StringArgumentType.greedyString())
                                 .executes(context -> ban(context, StringArgumentType.getString(context, "reason"))))));
 
-        // The track names shape the tree, so they are fixed until restart; their permissions
-        // are read on every check so /tcore reload tfmc applies them
-        for (String name : config.tracks().keySet()) {
-            Predicate<CommandSourceStack> promote = permission(() -> trackPermission(name, true));
-            Predicate<CommandSourceStack> demote = permission(() -> trackPermission(name, false));
-            root.then(Commands.literal(name)
-                    .requires(promote.or(demote))
-                    .then(Commands.literal("promote").requires(promote)
-                            .executes(player(p -> step(p, name, true))))
-                    .then(Commands.literal("demote").requires(demote)
-                            .executes(player(p -> step(p, name, false)))));
-        }
+        // Promote and demote act on the player's highest track. Permissions are read on every
+        // check, so /tcore reload tfmc applies track changes
+        root.then(Commands.literal("promote").requires(source -> stepTrack(source.getSender(), true).isPresent())
+                .executes(player(p -> step(p, true))));
+        root.then(Commands.literal("demote").requires(source -> stepTrack(source.getSender(), false).isPresent())
+                .executes(player(p -> step(p, false))));
 
         // Content still being built on dev; each section stays hidden until tfmc.yml enables it
         root.then(Commands.literal("tutorial").requires(enabled("tutorials"))
@@ -265,6 +259,11 @@ private int ban(CommandContext<CommandSourceStack> context, String rawReason) {
         return Command.SINGLE_SUCCESS;
     }
 
+    // The node's requires check has already found the track
+    private void step(Player player, boolean promote) {
+        stepTrack(player, promote).ifPresent(track -> step(player, track, promote));
+    }
+
     private void step(Player player, String track, boolean promote) {
         actions.stepTrack(player, track, promote).whenCompleteAsync((step, error) -> {
             if (error != null) {
@@ -398,12 +397,19 @@ private Command<CommandSourceStack> player(Consumer<Player> handler) {
         };
     }
 
-    private String trackPermission(String track, boolean promote) {
-        TfmcConfig.TrackSteps steps = config.tracks().get(track);
-        if (steps == null) {
-            return null;
+    /**
+     * The track a promote or demote moves the sender along: their highest track, the last in
+     * tfmc.yml holding one of their groups, if they hold that step's permission on it.
+     */
+    private Optional<String> stepTrack(CommandSender sender, boolean promote) {
+        if (!(sender instanceof Player player)) {
+            return Optional.empty();
         }
-        return promote ? steps.promotePermission() : steps.demotePermission();
+        Map<String, TfmcConfig.TrackSteps> tracks = config.tracks();
+        return actions.highestTrack(player, List.copyOf(tracks.keySet())).filter(track -> {
+            TfmcConfig.TrackSteps steps = tracks.get(track);
+            return holds(player, promote ? steps.promotePermission() : steps.demotePermission());
+        });
     }
 
     private Predicate<CommandSourceStack> enabled(String section) {
@@ -420,10 +426,11 @@ private Predicate<CommandSourceStack> anyPermission(String path) {
 
     // Restricted nodes fail closed: a missing or blank permission hides the command from everyone
     private static Predicate<CommandSourceStack> permission(Supplier<String> node) {
-        return source -> {
-            String permission = node.get();
-            return permission != null && !permission.isBlank() && source.getSender().hasPermission(permission);
-        };
+        return source -> holds(source.getSender(), node.get());
+    }
+
+    private static boolean holds(CommandSender sender, String permission) {
+        return permission != null && !permission.isBlank() && sender.hasPermission(permission);
     }
 
     private static SuggestionProvider<CommandSourceStack> onlinePlayers() {

src/main/resources/tfmc.yml

@@ -1,8 +1,7 @@
 # /tfmc player commands. Messages use MiniMessage (https://docs.advntr.dev/minimessage/format).
 # A message may be a single string or a list of lines.
 # Commands are run as written, with <player> replaced by the player's name.
-# Changes apply after /tcore reload tfmc. Adding or removing a helper track needs a restart;
-# changing a track's permissions does not.
+# Changes apply after /tcore reload tfmc.
 
 messages:
   players-only: "<red>Only players can use this command."
@@ -149,19 +148,23 @@ ban:
     - "tempban <target> 8h <reason>"
   message: "<green>Tempbanned <red><target></red> for 8 hours."
 
-# /tfmc <track> promote|demote moves the player one step along a LuckPerms track.
+# /tfmc promote|demote moves the player one step along their highest LuckPerms track: the last
+# one listed here that holds one of their global groups. List tracks lowest first.
 helper-tracks:
   helper:
     promote-permission: helper.promote
     demote-permission: helper.demote
   helper+:
     promote-permission: helper+.promote
     demote-permission: helper+.demote
+  staff:
+    promote-permission: staff.promote
+    demote-permission: staff.demote
 track-messages:
   promoted: "<green>You are now <yellow><group></yellow>."
   demoted: "<green>You are now <yellow><group></yellow>."
   removed: "<green>You are no longer on the <yellow><track></yellow> track."
-  failed: "<red>Your helper status could not be changed."
+  failed: "<red>Your status could not be changed."
 
 # /tfmc poster <name> runs the command as the player.
 posters:

src/test/java/net/tfminecraft/tfmccore/tfmc/BukkitTfmcActionsCoverageTest.java

@@ -250,7 +250,7 @@ void globalPermissionChangesRemoveOnlyTheMatchingPermissionAndPushTheSavedUpdate
   @Test
   void aFailedUserLoadReleasesTheQueueAndAnUnsuccessfulDemotionDoesNotSave() {
     LuckPerms api = mock(LuckPerms.class, RETURNS_DEEP_STUBS);
-    User user = mock(User.class);
+    User user = mock(User.class, RETURNS_DEEP_STUBS);
     Player player = mock(Player.class);
     UUID id = UUID.randomUUID();
     when(player.getUniqueId()).thenReturn(id);

src/test/java/net/tfminecraft/tfmccore/tfmc/BukkitTfmcActionsTest.java

@@ -31,7 +31,10 @@
 import net.luckperms.api.model.data.NodeMap;
 import net.luckperms.api.model.user.User;
 import net.luckperms.api.model.user.UserManager;
+import net.luckperms.api.node.Node;
 import net.luckperms.api.node.NodeBuilderRegistry;
+import net.luckperms.api.node.NodeType;
+import net.luckperms.api.node.types.InheritanceNode;
 import net.luckperms.api.node.types.PermissionNode;
 import net.luckperms.api.track.DemotionResult;
 import net.luckperms.api.track.PromotionResult;
@@ -46,6 +49,7 @@ class BukkitTfmcActionsTest {
     private final UserManager users = mock(UserManager.class);
     private final Track track = mock(Track.class);
     private final User user = mock(User.class);
+    private final NodeMap data = mock(NodeMap.class);
     private final Player player = mock(Player.class);
     private BukkitTfmcActions actions;
 
@@ -66,9 +70,11 @@ void setUp() {
         when(api.getNodeBuilderRegistry()).thenReturn(nodes);
         when(nodes.forPermission()).thenReturn(mock(PermissionNode.Builder.class, RETURNS_SELF));
         when(tracks.getTrack("helper+")).thenReturn(track);
+        when(track.containsGroup("helper+_player")).thenReturn(true);
+        groups(group("helper+_player", true, true));
         when(users.getUser(ID)).thenReturn(user);
         when(users.saveUser(user)).thenReturn(CompletableFuture.completedFuture(null));
-        when(user.data()).thenReturn(mock(NodeMap.class));
+        when(user.data()).thenReturn(data);
         when(player.getUniqueId()).thenReturn(ID);
         when(player.getName()).thenReturn("RiverBonnie");
         actions = new BukkitTfmcActions(null, () -> api);
@@ -188,6 +194,64 @@ void callersCannotReleaseTheNextChangeEarly() {
         verify(users, times(2)).loadUser(ID);
     }
 
+    @Test
+    void theHighestTrackIsTheLastHoldingOneOfTheUsersOwnGlobalGroups() {
+        TrackManager tracks = api.getTrackManager();
+        Track staff = mock(Track.class);
+        when(staff.containsGroup("staff_player")).thenReturn(true);
+        when(tracks.getTrack("staff")).thenReturn(staff);
+        List<String> order = List.of("helper", "helper+", "staff");
+
+        assertEquals(Optional.of("helper+"), actions.highestTrack(player, order));
+
+        // Server-only, negated and non-group nodes and absent tracks do not count
+        PermissionNode permission = mock(PermissionNode.class);
+        when(permission.getType()).thenReturn(NodeType.PERMISSION);
+        groups(group("helper+_player", true, true), group("staff_player", true, false), group("staff_player", false, true),
+                permission);
+        assertEquals(Optional.of("helper+"), actions.highestTrack(player, order));
+
+        groups(group("helper+_player", true, true), group("staff_player", true, true));
+        assertEquals(Optional.of("staff"), actions.highestTrack(player, order));
+        assertEquals(Optional.of("helper+"), actions.highestTrack(player, List.of("helper+", "helper")));
+
+        groups();
+        assertEquals(Optional.empty(), actions.highestTrack(player, order));
+    }
+
+    @Test
+    void noTrackWithoutLuckPermsOrALoadedUser() {
+        when(users.getUser(ID)).thenReturn(null);
+        assertEquals(Optional.empty(), actions.highestTrack(player, List.of("helper+")));
+        assertEquals(Optional.empty(), new BukkitTfmcActions(null, () -> null).highestTrack(player, List.of("helper+")));
+    }
+
+    @Test
+    void playersOffTheTrackAreNeverAddedToIt() {
+        when(users.loadUser(ID)).thenReturn(CompletableFuture.completedFuture(user));
+        groups();
+        promoteTo("helper+_player");
+
+        assertFalse(actions.stepTrack(player, "helper+", true).join().changed());
+        verify(track, never()).promote(any(User.class), any(ContextSet.class));
+        verify(users, never()).saveUser(user);
+    }
+
+    private void groups(Node... nodes) {
+        when(data.toCollection()).thenReturn(List.of(nodes));
+    }
+
+    private static InheritanceNode group(String name, boolean value, boolean global) {
+        InheritanceNode node = mock(InheritanceNode.class);
+        ImmutableContextSet contexts = mock(ImmutableContextSet.class);
+        when(contexts.isEmpty()).thenReturn(global);
+        when(node.getGroupName()).thenReturn(name);
+        when(node.getValue()).thenReturn(value);
+        when(node.getContexts()).thenReturn(contexts);
+        when(node.getType()).thenReturn(NodeType.INHERITANCE);
+        return node;
+    }
+
     private void promoteTo(String group) {
         PromotionResult promoted = mock(PromotionResult.class);
         when(promoted.wasSuccessful()).thenReturn(true);