A failed secret add --force leaves the secret unchanged (closes #136)

addVersion wrote the new version before it unlocked the vault to set
notAfter on the version being replaced, so a wrong passphrase or mnemonic
left an extra version behind. It now unlocks the vault and loads that
version's metadata first; writing notAfter and switching current still
come after the new version is saved. generate secret --force and
import --force take the same path.

updatePreviousVersion asks for the long-term key again. The vault keeps
it in memory after the first unlock, so this neither prompts nor derives
it a second time.

Model: opus-5-5

internal/vault/secrets.go

@@ -219,6 +219,8 @@ func (v *Vault) addVersion(
 	name, secretDir string, value *memguard.LockedBuffer,
 	previousVersion *secret.Version,
 ) error {
+	now := time.Now()
+
 	// Unlock the vault before writing anything, so that a wrong passphrase
 	// or mnemonic leaves the secret as it was
 	err := v.loadPreviousVersionMetadata(previousVersion)
@@ -226,10 +228,6 @@ func (v *Vault) addVersion(
 		return err
 	}
 
-	// Taken after the unlock, which can wait for the user to type the
-	// passphrase, so that the new version starts when it is created
-	now := time.Now()
-
 	// Create the new version and save the encrypted value
 	versionName, err := v.createAndSaveVersion(
 		name, secretDir, value, previousVersion, &now)

internal/vault/secrets_version_test.go

@@ -10,8 +10,6 @@
 //   version
 // - TestVaultVersionTimestamps: Tests timestamp logic (notBefore/notAfter)
 //   across versions
-// - TestVaultVersionTimestampsAfterUnlock: Tests that those timestamps are
-//   taken after AddSecret unlocks the vault
 // - TestVaultGetNonExistentVersion: Tests error handling for invalid versions
 // - TestUpdateVersionMetadata: Tests metadata update functionality
 //
@@ -297,70 +295,6 @@ func TestVaultVersionTimestamps(t *testing.T) {
 	assert.Nil(t, secondVersion.Metadata.NotAfter) // Current version
 }
 
-// longTermKeyReadFs records when longterm.age, the last file an unlock
-// reads, was last opened.
-type longTermKeyReadFs struct {
-	afero.Fs
-
-	readAt time.Time
-}
-
-//nolint:ireturn // afero.File is the interface required by afero.Fs
-func (f *longTermKeyReadFs) Open(name string) (afero.File, error) {
-	if filepath.Base(name) == "longterm.age" {
-		f.readAt = time.Now()
-	}
-
-	return f.Fs.Open(name)
-}
-
-// TestVaultVersionTimestampsAfterUnlock checks that when AddSecret has to
-// unlock the vault to replace a version, the new version's notBefore and the
-// replaced version's notAfter are taken after the unlock, which can wait for
-// the user to type the passphrase.
-func TestVaultVersionTimestampsAfterUnlock(t *testing.T) {
-	t.Parallel()
-
-	fs := &longTermKeyReadFs{Fs: afero.NewMemMapFs()}
-
-	mnemonic := memguard.NewBufferFromBytes([]byte(testMnemonic))
-	defer mnemonic.Destroy()
-
-	passphrase := memguard.NewBufferFromBytes([]byte("test-passphrase"))
-	defer passphrase.Destroy()
-
-	created, err := CreateVault(fs, testStateDir, "test", mnemonic, passphrase)
-	require.NoError(t, err)
-
-	addTestSecretToVault(t, created, testSecretPath, []byte("version-1"), false)
-
-	// A new instance is locked; AddSecret unlocks it with the passphrase
-	vault := NewVault(fs, testStateDir, "test")
-	vault.SetUnlockPassphrase(passphrase)
-
-	addTestSecretToVault(t, vault, testSecretPath, []byte("version-2"), true)
-	require.False(t, fs.readAt.IsZero(), "AddSecret did not unlock the vault")
-
-	ltIdentity, err := vault.GetOrDeriveLongTermKey()
-	require.NoError(t, err)
-
-	vaultDir, _ := vault.GetDirectory()
-	secretDir := filepath.Join(vaultDir, "secrets.d", "test%secret")
-
-	versions, err := secret.ListVersions(fs, secretDir)
-	require.NoError(t, err)
-	require.Len(t, versions, 2)
-
-	newVersion := secret.NewVersion(vault, testSecretPath, versions[0])
-	require.NoError(t, newVersion.LoadMetadata(ltIdentity))
-
-	replacedVersion := secret.NewVersion(vault, testSecretPath, versions[1])
-	require.NoError(t, replacedVersion.LoadMetadata(ltIdentity))
-
-	assert.False(t, newVersion.Metadata.NotBefore.Before(fs.readAt))
-	assert.False(t, replacedVersion.Metadata.NotAfter.Before(fs.readAt))
-}
-
 func TestVaultGetNonExistentVersion(t *testing.T) {
 	t.Parallel()