TODO.md

@@ -25,6 +25,13 @@ Bring the repo into policy compliance in one commit:
 
 # Completed Steps
 
+- 2026-10-04: The `internal/cli` tests are back to about their time
+  before the state directory lock
+  (https://git.eeqj.de/sneak/secret/issues/80). The test that each
+  changing command waits for the lock releases it as soon as it sees the
+  command waiting there, instead of after a fixed 100 ms. The two vaults
+  with passphrase unlockers that the path and move tests start from are
+  made once and copied for each test.
 - 2026-10-04: `secret mv` rejects a move whose destination is the source
   under another name, such as `foo` for `Foo` on a case-insensitive
   filesystem (the macOS default) or a name reached through a symbolic

internal/cli/lock_test.go

@@ -2,9 +2,11 @@
 package cli
 
 import (
+	"bytes"
 	"io"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strconv"
 	"strings"
 	"sync"
@@ -25,11 +27,6 @@ const (
 	// once the lock is free.
 	lockWait = 10 * time.Second
 
-	// heldWait is how long a test watches a command that must wait for the
-	// lock. A command that takes no lock changes the state directory well
-	// within it.
-	heldWait = 100 * time.Millisecond
-
 	// testPassphrase protects the passphrase unlockers the tests create.
 	testPassphrase = "test-passphrase"
 
@@ -323,10 +320,28 @@ func setupEveryCommand(
 	return versions[1], unlockerID
 }
 
+// waitingForLock reports whether a goroutine is stopped in
+// vault.LockStateDir, waiting for the in-memory filesystem's lock. The
+// stack trace of such a goroutine starts with the reason it waits,
+// "[sync.Mutex.Lock]", and names LockStateDir.
+func waitingForLock() bool {
+	stacks := make([]byte, 1<<20)
+	stacks = stacks[:runtime.Stack(stacks, true)]
+
+	for goroutine := range bytes.SplitSeq(stacks, []byte("\n\n")) {
+		if bytes.Contains(goroutine, []byte("[sync.Mutex.Lock")) &&
+			bytes.Contains(goroutine, []byte("vault.LockStateDir(")) {
+			return true
+		}
+	}
+
+	return false
+}
+
 // requireWaitsForLock runs a command, given what setupEveryCommand made,
 // while holding the state directory lock. The command must neither finish
-// nor change anything while the lock is held, and must succeed once it is
+// nor change anything before it waits for the lock, and must succeed once
-// released.
+// the lock is released.
 func requireWaitsForLock(
 	t *testing.T,
 	withUnlocker bool,
@@ -355,14 +370,20 @@ func requireWaitsForLock(
 
 	go func() { done <- run(cli, olderVersion, unlockerID) }()
 
-	select {
+	timeout := time.After(lockWait)
-	case err := <-done:
+
-		t.Fatalf("finished while the lock was held, with error %v", err)
+	for !waitingForLock() {
-	case <-time.After(heldWait):
+		select {
+		case err := <-done:
+			t.Fatalf("finished while the lock was held, with error %v", err)
+		case <-timeout:
+			t.Fatal("never waited for the lock")
+		case <-time.After(time.Millisecond):
+		}
 	}
 
 	assert.Equal(t, before, stateDirModTimes(t, fs),
-		"changed the state directory while the lock was held")
+		"changed the state directory before waiting for the lock")
 
 	release()
 

internal/cli/path_traversal_test.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"slices"
 	"strings"
+	"sync"
 	"testing"
 
 	"git.eeqj.de/sneak/secret/internal/cli"
@@ -32,9 +33,20 @@ const (
 	missingFile = "/no/such/file"
 )
 
+// The state directory newTwoVaultFs copies, recorded by snapshotStateDir.
+// Creating a passphrase unlocker is slow by design, so the vaults are made
+// once, by the first test that needs them.
+//
+//nolint:gochecknoglobals // shared by the tests that use newTwoVaultFs
+var (
+	twoVaultsOnce sync.Once
+	twoVaults     map[string]string
+)
+
 // newTwoVaultFs returns an in-memory filesystem holding the vaults "work"
 // and "default", the current one. Each holds the secret "x" and a
 // passphrase unlocker, so both secrets.d and unlockers.d have contents.
+// Every call returns a new copy of the same vaults.
 //
 //nolint:ireturn // afero.Fs is the filesystem abstraction used throughout
 func newTwoVaultFs(t *testing.T) afero.Fs {
@@ -42,21 +54,27 @@ func newTwoVaultFs(t *testing.T) afero.Fs {
 
 	t.Setenv(secret.EnvMnemonic, testMnemonic)
 
-	fs := afero.NewMemMapFs()
+	twoVaultsOnce.Do(func() {
+		fs := afero.NewMemMapFs()
 
-	for _, name := range []string{"work", "default"} {
+		for _, name := range []string{"work", "default"} {
-		vlt, err := vault.CreateVault(fs, testStateDir, name)
+			vlt, err := vault.CreateVault(fs, testStateDir, name)
-		require.NoError(t, err)
+			require.NoError(t, err)
 
-		err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false)
+			err = vlt.AddSecret("x", memguard.NewBufferFromBytes([]byte("value")), false)
-		require.NoError(t, err)
+			require.NoError(t, err)
 
-		_, err = vlt.CreatePassphraseUnlocker(
+			_, err = vlt.CreatePassphraseUnlocker(
-			memguard.NewBufferFromBytes([]byte(testPassphrase)))
+				memguard.NewBufferFromBytes([]byte(testPassphrase)))
-		require.NoError(t, err)
+			require.NoError(t, err)
-	}
+		}
 
-	return fs
+		twoVaults = snapshotStateDir(t, fs)
+	})
+
+	require.NotNil(t, twoVaults, "making the vaults failed in an earlier test")
+
+	return newFsFromSnapshot(t, twoVaults)
 }
 
 // snapshotStateDir maps every file under the state directory to its