fix(stateio): hold lock across state read-modify-write to stop dropped RecordApplied entries

cmd/delete.go

@@ -28,6 +28,7 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"github.com/a2d2-dev/claudecm/internal/config"
 	"github.com/a2d2-dev/claudecm/internal/storage"
 )
 
@@ -128,16 +129,17 @@ func runDelete(cmd *cobra.Command, args []string) error {
 // name. Returns (true, nil) when the pointer was cleared, (false, nil)
 // otherwise. Any I/O error surfaces as-is.
 func maybeClearActivePointer(store *storage.FileStorage, name string) (bool, error) {
-	state, err := store.LoadState()
+	var cleared bool
+	err := store.UpdateState(func(state *config.State) (bool, error) {
+		if state.CurrentProfile != name {
+			return false, nil
+		}
+		state.CurrentProfile = ""
+		cleared = true
+		return true, nil
+	})
 	if err != nil {
-		return false, fmt.Errorf("load state: %w", err)
-	}
-	if state.CurrentProfile != name {
-		return false, nil
-	}
-	state.CurrentProfile = ""
-	if err := store.SaveState(state); err != nil {
-		return false, fmt.Errorf("save state after delete: %w", err)
+		return false, err
 	}
-	return true, nil
+	return cleared, nil
 }

cmd/rename.go

@@ -26,6 +26,7 @@ import (
 
 	"github.com/spf13/cobra"
 
+	"github.com/a2d2-dev/claudecm/internal/config"
 	"github.com/a2d2-dev/claudecm/internal/storage"
 )
 
@@ -143,16 +144,11 @@ func runRename(cmd *cobra.Command, args []string) error {
 // active" branch — no state I/O has to fire when the pointer already
 // pointed elsewhere.
 func maybeUpdateStateAfterRename(store *storage.FileStorage, oldName, newName string) error {
-	state, err := store.LoadState()
-	if err != nil {
-		return fmt.Errorf("load state: %w", err)
-	}
-	if state.CurrentProfile != oldName {
-		return nil
-	}
-	state.SetCurrentProfile(newName)
-	if err := store.SaveState(state); err != nil {
-		return fmt.Errorf("save state after rename: %w", err)
-	}
-	return nil
+	return store.UpdateState(func(state *config.State) (bool, error) {
+		if state.CurrentProfile != oldName {
+			return false, nil
+		}
+		state.SetCurrentProfile(newName)
+		return true, nil
+	})
 }

cmd/switch.go

@@ -216,7 +216,7 @@ func runSwitch(cmd *cobra.Command, args []string) error {
 	// profile pointer moves — a no-op switch is a legitimate outcome
 	// when the profile matches the current on-disk intent.
 	if len(plans) == 0 {
-		if err := updateStateOnSuccess(resv, store, profileName, nil); err != nil {
+		if err := updateStateOnSuccess(resv, profileName, nil); err != nil {
 			return fmt.Errorf("no plans to commit but state update failed: %w", err)
 		}
 		return renderNoOp(cmd.OutOrStdout(), format, profileName, planErrors)
@@ -308,7 +308,7 @@ func runSwitch(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("commit: %w", commitErr)
 	}
 
-	if err := updateStateOnSuccess(resv, store, profileName, &report); err != nil {
+	if err := updateStateOnSuccess(resv, profileName, &report); err != nil {
 		return fmt.Errorf("commit succeeded but state update failed: %w", err)
 	}
 
@@ -447,33 +447,25 @@ func isNoConfigErr(err error) bool {
 // report != nil, records the (path, sha256, appliedAt) tuple for every
 // committed file so external-drift detection has a fresh anchor. On
 // the empty-plan path (report == nil) only the pointer moves.
-func updateStateOnSuccess(r *storage.Resolver, store *storage.FileStorage, profileName string, report *commit.CommitReport) error {
-	state, err := store.LoadState()
-	if err != nil {
-		return fmt.Errorf("load state: %w", err)
-	}
-	state.SetCurrentProfile(profileName)
-	if err := store.SaveState(state); err != nil {
-		return fmt.Errorf("save state: %w", err)
-	}
-	if report == nil {
-		return nil
-	}
-	for _, pf := range report.PerFile {
-		if pf.Status != commit.StatusCommitted {
-			continue
+func updateStateOnSuccess(r *storage.Resolver, profileName string, report *commit.CommitReport) error {
+	return stateio.UpdateState(r, func(state *config.State) (bool, error) {
+		state.SetCurrentProfile(profileName)
+		if report == nil {
+			return true, nil
 		}
-		if err := stateio.RecordApplied(
-			r,
-			config.ToolID(pf.Report.Tool),
-			pf.Target,
-			pf.Report.PostFingerprint.SHA256,
-			pf.Report.AppliedAt,
-		); err != nil {
-			return fmt.Errorf("record applied for %s: %w", pf.Target, err)
+		for _, pf := range report.PerFile {
+			if pf.Status != commit.StatusCommitted {
+				continue
+			}
+			state.RecordApplied(
+				config.ToolID(pf.Report.Tool),
+				pf.Target,
+				pf.Report.PostFingerprint.SHA256,
+				pf.Report.AppliedAt,
+			)
 		}
-	}
-	return nil
+		return true, nil
+	})
 }
 
 // promptConfirm prints a y/N question and reads a single line from in.

cmd/switch_test.go

@@ -968,7 +968,7 @@ func TestSwitch_UpdateStateOnSuccessRecordsCommittedOnly(t *testing.T) {
 			},
 		},
 	}
-	if err := updateStateOnSuccess(h.resv, h.store, "prod", &report); err != nil {
+	if err := updateStateOnSuccess(h.resv, "prod", &report); err != nil {
 		t.Fatalf("updateStateOnSuccess: %v", err)
 	}
 	state, err := h.store.LoadState()

docs/architecture/coding-standards.md

@@ -48,7 +48,7 @@ These rules encode the locked invariants. Each one is testable.
 
 11. **No `panic` in library code.** `panic` is allowed only in `main()` for unrecoverable startup failures. Every fallible function returns `error`. Wrap with `fmt.Errorf("...: %w", err)` when adding context.
 
-12. **No package-level mutable state.** Pass dependencies explicitly. The single exception is the structured logger configured in `main()`.
+12. **No package-level mutable state.** Pass dependencies explicitly. The only documented exceptions are the structured logger configured in `main()` and the process-local lock registry in `internal/storage/lock.go` (`processLocks`). Same-process flock contenders must be serialized process-wide, so the registry's scope must be the process; per-instance state would not serialize goroutines holding different instances.
 
 13. **Two-phase commit on multi-file writes.** When a single command touches more than one owned file, route through `internal/commit`. Direct sequencing of `writepath.Apply` calls across files is a violation. Maps to FR-16.
 

internal/adapter/stateio/stateio.go

@@ -51,29 +51,13 @@ package stateio
 import (
 	"crypto/sha256"
 	"encoding/hex"
-	"errors"
-	"fmt"
 	"os"
-	"path/filepath"
 	"time"
 
 	"github.com/a2d2-dev/claudecm/internal/config"
 	"github.com/a2d2-dev/claudecm/internal/storage"
 )
 
-// stateLockTimeout is the flock timeout for the state.yaml
-// read-modify-write critical section. Kept short so a stuck adapter
-// surfaces as ErrLockTimeout instead of hanging Apply indefinitely.
-// The state-file write itself is a few KB of YAML; the practical hold
-// time is sub-millisecond, so 5 seconds is generous.
-const stateLockTimeout = 5 * time.Second
-
-// stateLockRelTarget is the HOME-relative path to state.yaml used as
-// the flock target. storage.Acquire refuses absolute paths. The literal
-// mirrors storage.ConfigDirName / storage.StateFileName; kept as a
-// package-level string so any future rename lands in one spot.
-var stateLockRelTarget = filepath.Join(storage.ConfigDirName, storage.StateFileName)
-
 // Sha256Hex returns the lowercase hex-encoded SHA-256 digest of data.
 // Kept in one place so every adapter that hashes a file for drift or
 // state anchoring uses the same algorithm and the same encoding —
@@ -136,25 +120,18 @@ func LoadLastApplied(r *storage.Resolver, tool config.ToolID, filePath string) (
 // condition. Silently swallowing would leave the drift detector in a
 // permanent false-positive state after the next external edit.
 func RecordApplied(r *storage.Resolver, tool config.ToolID, filePath, sha256 string, appliedAt time.Time) error {
-	if r == nil {
-		return errors.New("stateio: RecordApplied: resolver is nil")
-	}
-	fs := storage.NewFileStorage(r)
-	return storage.WithLock(r, stateLockRelTarget, storage.LockOptions{Timeout: stateLockTimeout}, func() error {
-		state, err := fs.LoadState()
-		if err != nil {
-			return fmt.Errorf("stateio: load state: %w", err)
-		}
-		// LoadState returns config.NewState() on a missing file, so
-		// state is never nil when err is nil. No defensive guard here.
+	return UpdateState(r, func(state *config.State) (bool, error) {
 		state.RecordApplied(tool, filePath, sha256, appliedAt)
-		if err := fs.SaveState(state); err != nil {
-			return fmt.Errorf("stateio: save state: %w", err)
-		}
-		return nil
+		return true, nil
 	})
 }
 
+// UpdateState delegates to storage.FileStorage.UpdateState so every state.yaml
+// writer shares one locked read-modify-write implementation.
+func UpdateState(r *storage.Resolver, mutate func(*config.State) (bool, error)) error {
+	return storage.NewFileStorage(r).UpdateState(mutate)
+}
+
 // DriftForFile checks a single owned file for external drift. Returns
 // true iff (a) state.yaml records a prior Apply for this (tool, path)
 // AND (b) the file is present on disk AND (c) the current on-disk

internal/config/manager.go

@@ -14,6 +14,7 @@ type Storage interface {
 	ProfileExists(name string) (bool, error)
 	SaveState(state *State) error
 	LoadState() (*State, error)
+	UpdateState(mutate func(*State) (bool, error)) error
 }
 
 // Manager handles all configuration management operations
@@ -138,18 +139,14 @@ func (m *Manager) DeleteProfile(name string) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
-	// Check if this is the active profile
-	state, err := m.storage.LoadState()
-	if err != nil {
-		return fmt.Errorf("failed to load state: %w", err)
-	}
-
-	if state.CurrentProfile == name {
-		// Clear active profile if deleting it
-		state.CurrentProfile = ""
-		if err := m.storage.SaveState(state); err != nil {
-			return fmt.Errorf("failed to update state: %w", err)
+	if err := m.storage.UpdateState(func(state *State) (bool, error) {
+		if state.CurrentProfile != name {
+			return false, nil
 		}
+		state.CurrentProfile = ""
+		return true, nil
+	}); err != nil {
+		return fmt.Errorf("failed to update state: %w", err)
 	}
 
 	// Delete profile
@@ -178,17 +175,10 @@ func (m *Manager) SetActive(name string) error {
 		return fmt.Errorf("profile %q not found", name)
 	}
 
-	// Load current state
-	state, err := m.storage.LoadState()
-	if err != nil {
-		return fmt.Errorf("failed to load state: %w", err)
-	}
-
-	// Update active profile
-	state.SetCurrentProfile(name)
-
-	// Save state
-	if err := m.storage.SaveState(state); err != nil {
+	if err := m.storage.UpdateState(func(state *State) (bool, error) {
+		state.SetCurrentProfile(name)
+		return true, nil
+	}); err != nil {
 		return fmt.Errorf("failed to save state: %w", err)
 	}
 

internal/storage/filesystem.go

@@ -47,8 +47,15 @@ type Storage interface {
 
 	// LoadState reads the state file
 	LoadState() (*config.State, error)
+
+	// UpdateState performs a locked state read-modify-write.
+	UpdateState(mutate func(*config.State) (bool, error)) error
 }
 
+var (
+	stateLockRelTarget = filepath.Join(ConfigDirName, StateFileName)
+)
+
 // FileStorage implements Storage using the local filesystem. It routes every
 // path through the injected *Resolver — the only source of HOME truth.
 type FileStorage struct {
@@ -238,6 +245,36 @@ func (fs *FileStorage) SaveState(state *config.State) error {
 	return nil
 }
 
+// UpdateState runs mutate against state.yaml and, when mutate reports a change,
+// persists the result while holding the state lock across the full
+// load → mutate → save cycle.
+func (fs *FileStorage) UpdateState(mutate func(*config.State) (bool, error)) error {
+	if fs == nil || fs.r == nil {
+		return errors.New("update state: storage resolver is nil")
+	}
+	if mutate == nil {
+		return errors.New("update state: mutate is nil")
+	}
+
+	return WithLock(fs.r, stateLockRelTarget, LockOptions{}, func() error {
+		state, err := fs.LoadState()
+		if err != nil {
+			return fmt.Errorf("load state: %w", err)
+		}
+		changed, err := mutate(state)
+		if err != nil {
+			return err
+		}
+		if !changed {
+			return nil
+		}
+		if err := fs.SaveState(state); err != nil {
+			return fmt.Errorf("save state: %w", err)
+		}
+		return nil
+	})
+}
+
 // LoadState reads the state file
 func (fs *FileStorage) LoadState() (*config.State, error) {
 	statePath, err := fs.r.StatePath()