From 0f4a7d0803b971e34796e699a7f3ac1b94e5e308 Mon Sep 17 00:00:00 2001
From: Chris Lu <chrislusf@users.noreply.github.com>
Date: Mon, 31 Aug 2026 11:36:23 -0700
Subject: [PATCH] shell: keep noLock to the command that set it (#11052)

noLock says "this invocation changes nothing" -- volume.balance, volume.move,
volume.copy, volume.merge and volume.fix.replication all set it for a dry run,
and none clears it. The CommandEnv is created once and reused by both
dispatchers, the interactive shell and the master's maintenance script runner,
so a simulation left every later command unlocked:

    volume.balance -noLock          # changes nothing
    volume.move ...                 # mutates, and skips its lock

Reset before dispatch in both, where the invocation begins. forceNoLock is
untouched: that is set once, deliberately, for a trusted path.
---
 weed/server/master_server.go          |  5 +++
 weed/shell/shell_liner.go             |  6 ++++
 weed/shell/shell_nolock_scope_test.go | 48 +++++++++++++++++++++++++++
 3 files changed, 59 insertions(+)
 create mode 100644 weed/shell/shell_nolock_scope_test.go

diff --git a/weed/server/master_server.go b/weed/server/master_server.go
index 401e2ec37..d921203c3 100644
--- a/weed/server/master_server.go
+++ b/weed/server/master_server.go
@@ -473,6 +473,11 @@ func processEachCmd(reg *regexp.Regexp, line string, commandEnv *shell.CommandEn
 				continue
 			}
 			glog.V(0).Infof("executing: %s %v", cmd, args)
+			// noLock belongs to the invocation that set it, not to the
+			// CommandEnv this loop reuses for every line of the script. Without
+			// this, a dry run earlier in the script leaves the mutations after
+			// it unlocked.
+			commandEnv.SetNoLock(false)
 			if err := c.Do(args, commandEnv, os.Stdout); err != nil {
 				glog.V(0).Infof("error: %v", err)
 			}
diff --git a/weed/shell/shell_liner.go b/weed/shell/shell_liner.go
index 1ae4ce5c5..9ea8ca873 100644
--- a/weed/shell/shell_liner.go
+++ b/weed/shell/shell_liner.go
@@ -141,6 +141,12 @@ func processEachCmd(cmd string, commandEnv *CommandEnv) bool {
 			foundCommand := false
 			for _, c := range Commands {
 				if c.Name() == cmd || c.Name() == "fs."+cmd {
+					// noLock says "this invocation changes nothing", which is a
+					// property of the invocation and not of the session. A
+					// command that set it for a dry run would otherwise leave
+					// every later command in the session unlocked, so a real
+					// mutation right after a simulation would skip its lock.
+					commandEnv.SetNoLock(false)
 					if err := c.Do(args, commandEnv, os.Stdout); err != nil {
 						fmt.Fprintf(os.Stderr, "error: %v\n", err)
 					}
diff --git a/weed/shell/shell_nolock_scope_test.go b/weed/shell/shell_nolock_scope_test.go
new file mode 100644
index 000000000..c8c0d85e5
--- /dev/null
+++ b/weed/shell/shell_nolock_scope_test.go
@@ -0,0 +1,48 @@
+package shell
+
+import (
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/cluster"
+	"github.com/seaweedfs/seaweedfs/weed/wdclient/exclusive_locks"
+)
+
+// noLock says "this invocation changes nothing" -- volume.balance,
+// volume.fix.replication and others set it for a dry run. It is a property of
+// the invocation, not of the session: the CommandEnv is reused across every
+// command, so without a reset a simulation leaves later commands unlocked and a
+// real mutation run straight after one skips its lock silently.
+func TestNoLockDoesNotOutliveTheCommandThatSetIt(t *testing.T) {
+	env := &CommandEnv{locker: exclusive_locks.NewExclusiveLocker(nil, cluster.AdminShellLockName)}
+
+	env.SetNoLock(true)
+	if err := env.confirmIsLocked([]string{"volume.balance"}); err != nil {
+		t.Fatalf("a dry run needs no lock: %v", err)
+	}
+
+	env.SetNoLock(false)
+
+	if err := env.confirmIsLocked([]string{"volume.move"}); err == nil {
+		t.Error("the next command mutates and must require the lock again")
+	}
+}
+
+// Both dispatchers reuse one CommandEnv: the interactive shell across the lines
+// an operator types, and the master's maintenance script runner across the lines
+// of a script. Resetting has to restore the lock requirement however many
+// invocations share the environment.
+func TestNoLockResetRestoresTheRequirementForEveryInvocation(t *testing.T) {
+	env := &CommandEnv{locker: exclusive_locks.NewExclusiveLocker(nil, cluster.AdminShellLockName)}
+
+	for i := 0; i < 3; i++ {
+		env.SetNoLock(true)
+		if err := env.confirmIsLocked([]string{"dry run"}); err != nil {
+			t.Fatalf("run %d: a dry run needs no lock: %v", i, err)
+		}
+
+		env.SetNoLock(false)
+		if err := env.confirmIsLocked([]string{"mutation"}); err == nil {
+			t.Fatalf("run %d: the mutation after it must require the lock again", i)
+		}
+	}
+}