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

@@ -221,6 +221,13 @@ func (v *Vault) addVersion(
 ) 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)
+	if err != nil {
+		return err
+	}
+
 	// Create the new version and save the encrypted value
 	versionName, err := v.createAndSaveVersion(
 		name, secretDir, value, previousVersion, &now)
@@ -632,8 +639,35 @@ func (v *Vault) checkExistingSecret(
 	return true, previousVersion, nil
 }
 
+// loadPreviousVersionMetadata unlocks the vault and loads the metadata of
+// the version being superseded. It is a no-op when previousVersion is nil.
+func (v *Vault) loadPreviousVersionMetadata(
+	previousVersion *secret.Version,
+) error {
+	if previousVersion == nil {
+		return nil
+	}
+
+	ltIdentity, err := v.GetOrDeriveLongTermKey()
+	if err != nil {
+		secret.Debug("Failed to get long-term key for previous version", "error", err)
+
+		return fmt.Errorf("failed to get long-term key: %w", err)
+	}
+
+	err = previousVersion.LoadMetadata(ltIdentity)
+	if err != nil {
+		secret.Debug("Failed to load previous version metadata", "error", err)
+
+		return fmt.Errorf("failed to load previous version metadata: %w", err)
+	}
+
+	return nil
+}
+
 // updatePreviousVersion sets the notAfter timestamp on the version being
-// superseded. It is a no-op when previousVersion is nil.
+// superseded, whose metadata loadPreviousVersionMetadata has loaded. It is
+// a no-op when previousVersion is nil.
 func (v *Vault) updatePreviousVersion(
 	previousVersion *secret.Version, now *time.Time,
 ) error {
@@ -641,22 +675,13 @@ func (v *Vault) updatePreviousVersion(
 		return nil
 	}
 
-	// Get long-term key to decrypt/encrypt metadata
+	// loadPreviousVersionMetadata unlocked the vault, so this returns the
+	// key it keeps in memory
 	ltIdentity, err := v.GetOrDeriveLongTermKey()
 	if err != nil {
-		secret.Debug("Failed to get long-term key for metadata update", "error", err)
-
 		return fmt.Errorf("failed to get long-term key: %w", err)
 	}
 
-	// Load previous version metadata
-	err = previousVersion.LoadMetadata(ltIdentity)
-	if err != nil {
-		secret.Debug("Failed to load previous version metadata", "error", err)
-
-		return fmt.Errorf("failed to load previous version metadata: %w", err)
-	}
-
 	// Update notAfter timestamp
 	previousVersion.Metadata.NotAfter = now