Make an unlocker's ID the name of its directory (closes #98)

Keychain and Secure Enclave unlocker IDs were the creation time to the
minute plus the host name, and passphrase unlocker IDs the time to the
minute, so two created within one minute shared an ID, and `unlocker
select`, `unlocker remove` and the selection after `unlocker add` acted on
the older one. Every unlocker's ID is now its directory name, unique in
its vault. PGP unlocker IDs were `pgp-<fingerprint>`; the check that
refuses a second PGP unlocker for one key now compares the fingerprint in
the other unlockers' metadata.

Model: opus-5-5

internal/cli/confirm_test.go

@@ -101,7 +101,8 @@ func newRemoval(t *testing.T, command string) removal {
 	}
 
 	fs, workDir, older := newConfirmTestVaults(t, unlockers)
-	unlockerID := "pgp-" + listTestGPGKeyID + "A"
+	// The first unlocker's directory name, written by newConfirmTestVaults
+	unlockerID := "pgp-0"
 
 	removeFirstUnlocker := func(cli *Instance, cmd *cobra.Command, force bool) error {
 		return cli.UnlockersRemove(unlockerID, force, cmd)
@@ -142,7 +143,7 @@ func newRemoval(t *testing.T, command string) removal {
 		return removal{
 			fs:      fs,
 			run:     removeFirstUnlocker,
-			removed: filepath.Join(workDir, "unlockers.d", "pgp-0"),
+			removed: filepath.Join(workDir, "unlockers.d", unlockerID),
 			question: "Permanently remove unlocker '" + unlockerID +
 				"' from vault 'work'? It is not the vault's last unlocker.",
 		}
@@ -150,7 +151,7 @@ func newRemoval(t *testing.T, command string) removal {
 		return removal{
 			fs:      fs,
 			run:     removeFirstUnlocker,
-			removed: filepath.Join(workDir, "unlockers.d", "pgp-0"),
+			removed: filepath.Join(workDir, "unlockers.d", unlockerID),
 			question: "Permanently remove unlocker '" + unlockerID +
 				"', the last unlocker of vault 'work', which holds 1 " +
 				"secret(s)? Without an unlocker the vault opens only " +

internal/cli/unlockers.go

@@ -697,9 +697,7 @@ func (cli *Instance) addPGPUnlocker(cmd *cobra.Command) error {
 	}
 
 	// Check if this GPG key is already added
-	expectedID := "pgp-" + fingerprint
-
-	exists, err := cli.checkUnlockerExists(vlt, expectedID)
+	exists, err := cli.pgpUnlockerExists(vlt, fingerprint)
 	if err != nil {
 		return fmt.Errorf(
 			"could not check whether GPG key %s is already an unlocker: %w",
@@ -889,16 +887,16 @@ func (cli *Instance) UnlockerSelect(unlockerID string) error {
 	return vlt.SelectUnlocker(unlockerID)
 }
 
-// checkUnlockerExists reports whether the vault already has an unlocker
-// with the given ID. It returns an error, and no answer, when unlockers.d
-// or an unlocker's metadata file cannot be read; the caller must then not
-// create the unlocker. It reads unlockers.d itself because
-// vault.ListUnlockers skips an unlocker it cannot read, which suits
+// pgpUnlockerExists reports whether the vault already has a PGP unlocker
+// for the GPG key with the given fingerprint. It returns an error, and no
+// answer, when unlockers.d or an unlocker's metadata file cannot be read;
+// the caller must then not create the unlocker. It reads unlockers.d itself
+// because vault.ListUnlockers skips an unlocker it cannot read, which suits
 // `unlocker list` but not this check: the skipped unlocker may be the
 // duplicate. A directory whose metadata file is missing or corrupt is not
 // a working unlocker and is passed over.
-func (cli *Instance) checkUnlockerExists(
-	vlt *vault.Vault, unlockerID string,
+func (cli *Instance) pgpUnlockerExists(
+	vlt *vault.Vault, fingerprint string,
 ) (bool, error) {
 	vaultDir, err := vlt.GetDirectory()
 	if err != nil {
@@ -937,14 +935,14 @@ func (cli *Instance) checkUnlockerExists(
 			)
 		}
 
-		var metadata secret.UnlockerMetadata
+		var metadata secret.PGPUnlockerMetadata
 
 		err = json.Unmarshal(metadataBytes, &metadata)
 		if err != nil {
 			continue
 		}
 
-		if unlockerIDFromDir(cli.fs, unlockerDir, metadata, true) == unlockerID {
+		if metadata.Type == unlockerTypePGP && metadata.GPGKeyID == fingerprint {
 			return true, nil
 		}
 	}

internal/cli/unlockers_corrupt_test.go

@@ -46,7 +46,7 @@ func TestUnlockerSelectSkipsCorruptUnlocker(t *testing.T) {
 	fs := newCorruptUnlockerVault(t)
 	instance, _ := newTestInstance(fs)
 
-	require.NoError(t, instance.UnlockerSelect("pgp-"+listTestGPGKeyID+"B"))
+	require.NoError(t, instance.UnlockerSelect(listTestUnlockerDirTwo))
 
 	current, err := afero.ReadFile(fs,
 		filepath.Join(testVaultDir(listTestVaultName), "current-unlocker"))
@@ -72,7 +72,7 @@ func TestUnlockerRemoveWithCorruptUnlocker(t *testing.T) {
 	}{
 		{
 			name:        "the other unlocker",
-			unlockerID:  "pgp-" + listTestGPGKeyID + "B",
+			unlockerID:  listTestUnlockerDirTwo,
 			wantLast:    true,
 			wantEntries: []string{listTestUnlockerDirOne},
 		},

internal/cli/unlockers_ids_test.go

@@ -0,0 +1,74 @@
+//nolint:testpackage // white-box test of unexported internals
+package cli
+
+import (
+	"encoding/json"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"git.eeqj.de/sneak/secret/internal/secret"
+	"git.eeqj.de/sneak/secret/internal/vault"
+	"github.com/spf13/afero"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestSameMinuteUnlockersHaveTheirOwnIDs writes two passphrase unlockers
+// created in the same minute side by side, as an `unlocker add passphrase`
+// that fails before removing the old one leaves them, and asserts that
+// `unlocker list` gives each its own ID, and that each is selected and
+// removed by its ID alone. Keychain and Secure Enclave unlockers, which
+// only macOS can add, get their IDs the same way.
+func TestSameMinuteUnlockersHaveTheirOwnIDs(t *testing.T) {
+	t.Parallel()
+
+	fs := afero.NewMemMapFs()
+	_, err := vault.CreateVault(fs, listTestStateDir, listTestVaultName,
+		testMnemonicBuffer(t))
+	require.NoError(t, err)
+
+	vaultDir := testVaultDir(listTestVaultName)
+	unlockersDir := filepath.Join(vaultDir, listTestUnlockersDirName)
+	dirNames := []string{
+		"passphrase-2026-10-04.12.30.00.000000000",
+		"passphrase-2026-10-04.12.30.30.000000000",
+	}
+
+	for i, dirName := range dirNames {
+		metadata, err := json.Marshal(secret.UnlockerMetadata{
+			Type:      unlockerTypePassphrase,
+			CreatedAt: time.Date(2026, time.October, 4, 12, 30, 30*i, 0, time.UTC),
+		})
+		require.NoError(t, err)
+
+		dir := filepath.Join(unlockersDir, dirName)
+		require.NoError(t, fs.MkdirAll(dir, listTestDirPerm))
+		require.NoError(t, afero.WriteFile(fs,
+			filepath.Join(dir, listTestMetadataFileName), metadata,
+			listTestFilePerm))
+	}
+
+	listed := listUnlockersJSON(t, fs)
+	require.Len(t, listed, len(dirNames))
+
+	instance, cmd := newTestInstance(fs)
+
+	for i, unlocker := range listed {
+		assert.Equal(t, dirNames[i], unlocker.ID)
+
+		require.NoError(t, instance.UnlockerSelect(unlocker.ID))
+
+		current, err := afero.ReadFile(fs,
+			filepath.Join(vaultDir, "current-unlocker"))
+		require.NoError(t, err)
+		assert.Equal(t, dirNames[i], string(current))
+	}
+
+	// The newer one first: an ID both shared would remove the older one
+	require.NoError(t, instance.UnlockersRemove(listed[1].ID, true, cmd))
+	assertDirEntries(t, fs, unlockersDir, dirNames[0])
+
+	require.NoError(t, instance.UnlockersRemove(listed[0].ID, true, cmd))
+	assertDirEntries(t, fs, unlockersDir)
+}

internal/cli/unlockers_list_test.go

@@ -48,13 +48,12 @@ const (
 	// listTestVaultName is the name of that synthetic vault.
 	listTestVaultName = "default"
 
-	// listTestGPGKeyID is the GPG key ID recorded in the readable PGP
-	// unlocker's metadata. The unlocker's real ID is derived from it, and
-	// differs from the timestamp-derived fallback ID.
+	// listTestGPGKeyID is the GPG key ID recorded, with a letter appended,
+	// in the PGP unlockers' metadata.
 	listTestGPGKeyID = "DEADBEEFDEADBEEF"
 
 	// listTestUnlockerDirOne and listTestUnlockerDirTwo are the unlocker
-	// directory names under unlockers.d.
+	// directory names under unlockers.d, and so the unlockers' IDs.
 	listTestUnlockerDirOne = "host-pgp-2026-08-09"
 	listTestUnlockerDirTwo = "host-pgp-2026-08-10"
 
@@ -142,8 +141,8 @@ func (f *metadataStatFailFs) Stat(name string) (os.FileInfo, error) {
 	return f.Fs.Stat(name)
 }
 
-// writePGPUnlocker writes a PGP unlocker directory with metadata that
-// yields the real ID "pgp-<keyID>".
+// writePGPUnlocker writes a PGP unlocker directory named dirName, with
+// metadata recording the GPG key ID keyID.
 func writePGPUnlocker(
 	t *testing.T, fs afero.Fs, unlockersDir, dirName string,
 	createdAt time.Time, keyID string,
@@ -256,7 +255,7 @@ func TestUnlockersListSkipsOnlyUnreadableEntries(t *testing.T) {
 
 	require.Len(t, unlockers, 1,
 		"only the entry whose directory was readable may be listed")
-	assert.Equal(t, "pgp-"+listTestGPGKeyID+"A", unlockers[0].ID,
+	assert.Equal(t, listTestUnlockerDirOne, unlockers[0].ID,
 		"the surviving row must carry the real unlocker ID")
 	assert.True(t, unlockers[0].IsCurrent,
 		"the current-unlocker marker must survive the skip")
@@ -272,20 +271,21 @@ func TestUnlockersListReadableEntriesAreListed(t *testing.T) {
 	unlockers := listUnlockersJSON(t, base)
 
 	require.Len(t, unlockers, 2)
-	assert.Equal(t, "pgp-"+listTestGPGKeyID+"A", unlockers[0].ID)
-	assert.Equal(t, "pgp-"+listTestGPGKeyID+"B", unlockers[1].ID)
+	assert.Equal(t, listTestUnlockerDirOne, unlockers[0].ID)
+	assert.Equal(t, listTestUnlockerDirTwo, unlockers[1].ID)
 	assert.True(t, unlockers[0].IsCurrent)
 	assert.False(t, unlockers[1].IsCurrent)
 }
 
 // TestUnlockersListToleratesCorruptMetadata asserts that one unlocker with
 // corrupt metadata does not stop the listing. Metadata that is not JSON
-// leaves that unlocker out; PGP metadata without a usable GPG key ID lists
-// it as "pgp-unknown". The healthy unlocker is listed with its real ID.
+// leaves that unlocker out; PGP metadata without a usable GPG key ID is
+// still listed, under its directory name like any other. The healthy
+// unlocker is listed with its real ID.
 func TestUnlockersListToleratesCorruptMetadata(t *testing.T) {
 	t.Parallel()
 
-	healthyID := "pgp-" + listTestGPGKeyID + "A"
+	healthyID := listTestUnlockerDirOne
 
 	tests := []struct {
 		name     string
@@ -300,12 +300,12 @@ func TestUnlockersListToleratesCorruptMetadata(t *testing.T) {
 		{
 			name:     "GPG key ID of the wrong type",
 			metadata: `{"type": "pgp", "gpgKeyId": 42}`,
-			wantIDs:  []string{healthyID, "pgp-unknown"},
+			wantIDs:  []string{healthyID, listTestUnlockerDirTwo},
 		},
 		{
 			name:     "GPG key ID missing",
 			metadata: `{"type": "pgp"}`,
-			wantIDs:  []string{healthyID, "pgp-unknown"},
+			wantIDs:  []string{healthyID, listTestUnlockerDirTwo},
 		},
 	}
 
@@ -371,7 +371,7 @@ func TestUnlockersListSkipsUnreadableMetadata(t *testing.T) {
 
 			require.Len(t, unlockers, 1,
 				"only the unlocker with usable metadata may be listed")
-			assert.Equal(t, "pgp-"+listTestGPGKeyID+"B", unlockers[0].ID,
+			assert.Equal(t, listTestUnlockerDirTwo, unlockers[0].ID,
 				"the listed row must carry the real unlocker ID")
 		})
 	}

internal/cli/unreadable_dir_test.go

@@ -290,7 +290,7 @@ func TestRemoveLastUnlockerAbortsWhenSecretsUnreadable(t *testing.T) {
 			writeTestSecret(t, base, vaultDir)
 			instance, _ := newTestInstance(&statFailFs{Fs: base, path: path})
 
-			_, err := instance.findUnlockerToRemove("pgp-" + listTestGPGKeyID + "A")
+			_, err := instance.findUnlockerToRemove(listTestUnlockerDirOne)
 
 			require.ErrorIs(t, err, errStatFailed)
 			assertDirEntries(t, base, unlockersDir, listTestUnlockerDirOne)