fix: improve error message for locking failures

cmd/dbc/add.go

@@ -27,7 +27,6 @@ import (
 	"charm.land/lipgloss/v2"
 	"github.com/Masterminds/semver/v3"
 	"github.com/columnar-tech/dbc/config"
-	"github.com/columnar-tech/dbc/internal/fslock"
 	"github.com/columnar-tech/dbc/internal/jsonschema"
 	"github.com/pelletier/go-toml/v2"
 )
@@ -136,9 +135,9 @@ func (m addModel) Init() tea.Cmd {
 		// read, a concurrent writer using os.Create could expose partial
 		// contents to this decode path.
 		lockPath := filepath.Join(filepath.Dir(p), ".dbc.project.lock")
-		readLock, err := fslock.Acquire(lockPath, 10*time.Second)
+		readLock, err := acquireLock(lockPath, 10*time.Second)
 		if err != nil {
-			return fmt.Errorf("another dbc operation is in progress: %w", err)
+			return err
 		}
 		f, err := os.Open(p)
 		if err != nil {
@@ -241,9 +240,9 @@ func (m addModel) Init() tea.Cmd {
 		// Re-read the file under the lock so a concurrent
 		// `dbc add`/`dbc remove` that landed while we were doing the
 		// registry lookup above doesn't get clobbered.
-		lock, err := fslock.Acquire(lockPath, 10*time.Second)
+		lock, err := acquireLock(lockPath, 10*time.Second)
 		if err != nil {
-			return fmt.Errorf("another dbc operation is in progress: %w", err)
+			return err
 		}
 		defer lock.Release()
 

cmd/dbc/helpers.go

@@ -18,10 +18,38 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"path/filepath"
+	"runtime"
+	"time"
 
+	"github.com/columnar-tech/dbc/internal/fslock"
 	"github.com/pelletier/go-toml/v2"
 )
 
+// elevationHint names the privilege-escalation mechanism for the host platform.
+func elevationHint() string {
+	if runtime.GOOS == "windows" {
+		return "re-running from an Administrator terminal"
+	}
+	return "re-running with sudo"
+}
+
+func acquireLock(lockPath string, timeout time.Duration) (fslock.Lock, error) {
+	lock, err := fslock.Acquire(lockPath, timeout)
+	if err == nil {
+		return lock, nil
+	}
+	if errors.Is(err, fslock.ErrLockContended) {
+		return fslock.Lock{}, fmt.Errorf("another dbc operation is in progress: %w", err)
+	}
+	if errors.Is(err, os.ErrPermission) {
+		return fslock.Lock{}, fmt.Errorf(
+			"cannot write to %s: permission denied.\nThis command requires elevated privileges; try %s.",
+			filepath.Dir(lockPath), elevationHint())
+	}
+	return fslock.Lock{}, fmt.Errorf("could not acquire lock in %s: %w", filepath.Dir(lockPath), err)
+}
+
 func wrapWithRegistryContext(err, registryErr error) error {
 	if registryErr != nil {
 		return fmt.Errorf("%w\n\nNote: Some driver registries were unavailable:\n%s", err, registryErr.Error())

cmd/dbc/install.go

@@ -32,7 +32,6 @@ import (
 	"github.com/Masterminds/semver/v3"
 	"github.com/columnar-tech/dbc"
 	"github.com/columnar-tech/dbc/config"
-	"github.com/columnar-tech/dbc/internal/fslock"
 	"github.com/columnar-tech/dbc/internal/jsonschema"
 )
 
@@ -274,9 +273,9 @@ func (m progressiveInstallModel) Init() tea.Cmd {
 			lockDir = parent
 		}
 		lockPath := filepath.Join(lockDir, ".dbc.install.lock")
-		lock, err := fslock.Acquire(lockPath, 10*time.Second)
+		lock, err := acquireLock(lockPath, 10*time.Second)
 		if err != nil {
-			return fmt.Errorf("another dbc operation is in progress: %w", err)
+			return err
 		}
 		defer lock.Release()
 

cmd/dbc/lock_error_test.go

@@ -0,0 +1,161 @@
+// Copyright 2026 Columnar Technologies Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+	"time"
+
+	tea "charm.land/bubbletea/v2"
+	"github.com/columnar-tech/dbc/internal/fslock"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// makeLockPathUnwritable seeds a read-only file at the lock path so that
+// opening it with O_RDWR fails with a permission error. This reproduces the
+// open() failure a user hits when the install directory requires elevation
+// (e.g. C:\Program Files\ADBC\Drivers for --level system) without the test
+// needing to be elevated or to manipulate ACLs.
+//
+// Returns false if the platform/filesystem/user does not enforce the
+// restriction, so callers can skip rather than assert on a non-reproduction.
+func makeLockPathUnwritable(t *testing.T, lockPath string) bool {
+	t.Helper()
+
+	if os.Geteuid() == 0 {
+		t.Log("running as root/elevated: mode bits do not deny access")
+		return false
+	}
+	require.NoError(t, os.WriteFile(lockPath, nil, 0o400))
+	require.NoError(t, os.Chmod(lockPath, 0o400))
+	t.Cleanup(func() { _ = os.Chmod(lockPath, 0o600) })
+
+	// Confirm the restriction actually bites before relying on it.
+	f, err := os.OpenFile(lockPath, os.O_RDWR, 0o600)
+	if err == nil {
+		f.Close()
+		t.Log("filesystem does not enforce the read-only bit for this user")
+		return false
+	}
+	if !errors.Is(err, os.ErrPermission) {
+		t.Logf("expected a permission error, got: %v", err)
+		return false
+	}
+	return true
+}
+
+// TestAcquireLockPermissionErrorIsNotReportedAsContention is the regression
+// test for the reported bug:
+//
+//	PS C:\Users\Bryce> dbc uninstall --level system foo
+//	Error: another dbc operation is in progress: fslock: open
+//	  C:\Program Files\ADBC\Drivers\.dbc.install.lock: ... Access is denied.
+//
+// No other dbc process was running. The install directory required
+// Administrator rights, so creating the lock file failed with
+// ERROR_ACCESS_DENIED — and every lock call site blanket-wrapped *any*
+// Acquire failure as "another dbc operation is in progress". That sent the
+// user hunting for a phantom concurrent process instead of telling them to
+// elevate.
+//
+// Before the fix this test fails on the first assertion: the message contains
+// "another dbc operation is in progress".
+func TestAcquireLockPermissionErrorIsNotReportedAsContention(t *testing.T) {
+	dir := t.TempDir()
+	lockPath := filepath.Join(dir, ".dbc.install.lock")
+	if !makeLockPathUnwritable(t, lockPath) {
+		t.Skip("cannot make lock path unwritable on this platform/user")
+	}
+
+	_, err := acquireLock(lockPath, 100*time.Millisecond)
+	require.Error(t, err, "acquiring a lock in an unwritable location must fail")
+
+	assert.NotContains(t, err.Error(), "another dbc operation is in progress",
+		"a permission failure must NOT be reported as a concurrent dbc operation; "+
+			"nothing was holding the lock")
+
+	// The message must instead point at the real cause and the real remedy.
+	assert.Contains(t, err.Error(), "permission denied",
+		"error should name the actual cause")
+	assert.Contains(t, err.Error(), dir,
+		"error should name the directory that could not be written")
+	assert.Contains(t, err.Error(), "elevated privileges",
+		"error should explain that elevation is required")
+
+	if runtime.GOOS == "windows" {
+		assert.Contains(t, err.Error(), "Administrator",
+			"on Windows the remedy is an Administrator terminal, not sudo")
+		assert.NotContains(t, err.Error(), "sudo",
+			"must not suggest sudo on Windows")
+	} else {
+		assert.Contains(t, err.Error(), "sudo",
+			"on Unix the remedy is sudo")
+	}
+}
+
+// TestAcquireLockGenuineContentionStillReported guards the other side of the
+// fix: a real concurrent holder must still produce the "another dbc operation
+// is in progress" message. A fix that simply dropped that message would make
+// the test above pass while destroying the diagnostic it exists to provide.
+func TestAcquireLockGenuineContentionStillReported(t *testing.T) {
+	lockPath := filepath.Join(t.TempDir(), ".dbc.install.lock")
+
+	held, err := fslock.Acquire(lockPath, 5*time.Second)
+	require.NoError(t, err)
+	defer held.Release() //nolint:errcheck
+
+	_, err = acquireLock(lockPath, 100*time.Millisecond)
+	require.Error(t, err, "lock is held, so acquisition must fail")
+	assert.Contains(t, err.Error(), "another dbc operation is in progress",
+		"genuine contention must still be reported as a concurrent operation")
+}
+
+// TestUninstallReportsElevationNotContention drives the actual `dbc uninstall`
+// command path — the exact invocation from the bug report — rather than just
+// the helper, so a regression at the call site is caught even if the helper
+// stays correct. install/add/remove/sync share the same helper.
+func TestUninstallReportsElevationNotContention(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("ADBC_DRIVER_PATH", dir)
+
+	lockPath := filepath.Join(dir, ".dbc.install.lock")
+	if !makeLockPathUnwritable(t, lockPath) {
+		t.Skip("cannot make lock path unwritable on this platform/user")
+	}
+
+	m := UninstallCmd{Driver: "foo"}.GetModelCustom(testBaseModel())
+	init, ok := m.(interface{ Init() tea.Cmd })
+	require.True(t, ok)
+
+	cmd := init.Init()
+	require.NotNil(t, cmd)
+	msg := cmd()
+
+	err, isErr := msg.(error)
+	require.True(t, isErr, "uninstall must surface an error, got %T: %v", msg, msg)
+
+	assert.NotContains(t, err.Error(), "another dbc operation is in progress",
+		"uninstall must not blame a phantom concurrent dbc process for a "+
+			"permission failure")
+	assert.True(t,
+		strings.Contains(err.Error(), "elevated privileges"),
+		"uninstall should tell the user to elevate; got: %v", err)
+}

cmd/dbc/remove.go

@@ -23,7 +23,6 @@ import (
 	"time"
 
 	tea "charm.land/bubbletea/v2"
-	"github.com/columnar-tech/dbc/internal/fslock"
 	"github.com/columnar-tech/dbc/internal/jsonschema"
 	"github.com/pelletier/go-toml/v2"
 )
@@ -77,9 +76,9 @@ func (m removeModel) Init() tea.Cmd {
 		}
 
 		lockPath := filepath.Join(filepath.Dir(p), ".dbc.project.lock")
-		lock, err := fslock.Acquire(lockPath, 10*time.Second)
+		lock, err := acquireLock(lockPath, 10*time.Second)
 		if err != nil {
-			return fmt.Errorf("another dbc operation is in progress: %w", err)
+			return err
 		}
 		defer lock.Release()
 

cmd/dbc/sync.go

@@ -31,7 +31,6 @@ import (
 	"charm.land/lipgloss/v2"
 	"github.com/columnar-tech/dbc"
 	"github.com/columnar-tech/dbc/config"
-	"github.com/columnar-tech/dbc/internal/fslock"
 	"github.com/columnar-tech/dbc/internal/jsonschema"
 	"github.com/pelletier/go-toml/v2"
 )
@@ -157,9 +156,9 @@ func (s syncModel) Init() tea.Cmd {
 		}
 
 		lockPath := filepath.Join(filepath.Dir(p), ".dbc.project.lock")
-		lock, err := fslock.Acquire(lockPath, 10*time.Second)
+		lock, err := acquireLock(lockPath, 10*time.Second)
 		if err != nil {
-			return fmt.Errorf("another dbc operation is in progress: %w", err)
+			return err
 		}
 		defer lock.Release()
 

cmd/dbc/uninstall.go

@@ -23,7 +23,6 @@ import (
 
 	tea "charm.land/bubbletea/v2"
 	"github.com/columnar-tech/dbc/config"
-	"github.com/columnar-tech/dbc/internal/fslock"
 	"github.com/columnar-tech/dbc/internal/jsonschema"
 )
 
@@ -80,9 +79,9 @@ func (m uninstallModel) Init() tea.Cmd {
 			lockDir = parent
 		}
 		lockPath := filepath.Join(lockDir, ".dbc.install.lock")
-		lock, err := fslock.Acquire(lockPath, 10*time.Second)
+		lock, err := acquireLock(lockPath, 10*time.Second)
 		if err != nil {
-			return fmt.Errorf("another dbc operation is in progress: %w", err)
+			return err
 		}
 		defer lock.Release()
 		return m.startUninstall()

internal/fslock/fslock.go

@@ -16,10 +16,15 @@
 // access to shared resources across processes.
 package fslock
 
-import "os"
+import (
+	"errors"
+	"os"
+)
 
 // Lock represents an acquired advisory file lock.
 type Lock struct {
 	f    *os.File
 	path string
 }
+
+var ErrLockContended = errors.New("lock is held by another process")

internal/fslock/fslock_test.go

@@ -15,6 +15,7 @@
 package fslock_test
 
 import (
+	"errors"
 	"os"
 	"path/filepath"
 	"sync"
@@ -116,4 +117,37 @@ func TestAcquireTimeout(t *testing.T) {
 	if err == nil {
 		t.Fatal("expected timeout error, got nil")
 	}
+	// Verify the error type is ErrLockContended
+	if !errors.Is(err, fslock.ErrLockContended) {
+		t.Fatalf("timeout error must wrap ErrLockContended, got: %v", err)
+	}
+}
+
+func TestAcquireUnwritableDirIsNotContention(t *testing.T) {
+	// Simulate a lock failure that's due to permissions instead of actual lock
+	// contention by creating a read-only file and later trying to lock on it
+	path := filepath.Join(t.TempDir(), "test.lock")
+	if err := os.WriteFile(path, nil, 0o400); err != nil {
+		t.Fatalf("seed lock file: %v", err)
+	}
+	if err := os.Chmod(path, 0o400); err != nil {
+		t.Fatalf("chmod: %v", err)
+	}
+	t.Cleanup(func() { os.Chmod(path, 0o600) })
+
+	// Skip if running with elevated privs
+	if os.Geteuid() == 0 {
+		t.Skip("running as root/elevated: mode bits do not deny access")
+	}
+
+	_, err := fslock.Acquire(path, 100*time.Millisecond)
+	if err == nil {
+		t.Skip("filesystem does not enforce the read-only bit for this user")
+	}
+	if !errors.Is(err, os.ErrPermission) {
+		t.Skipf("expected a permission error from open, got: %v", err)
+	}
+	if errors.Is(err, fslock.ErrLockContended) {
+		t.Fatalf("permission failure must not be classified as contention: %v", err)
+	}
 }

internal/fslock/fslock_unix.go

@@ -46,7 +46,8 @@ func Acquire(path string, timeout time.Duration) (Lock, error) {
 				// Reopen and try again within the remaining budget.
 				continue
 			}
-			return Lock{}, fmt.Errorf("fslock: could not acquire lock on %s within %s: %w", path, timeout, err)
+			return Lock{}, fmt.Errorf("fslock: could not acquire lock on %s within %s (%v): %w",
+				path, timeout, err, ErrLockContended)
 		}
 		return Lock{}, err
 	}
@@ -73,7 +74,8 @@ func lockFile(f *os.File, path string, deadline time.Time) (Lock, error) {
 			return Lock{f: f, path: path}, nil
 		}
 		if time.Now().After(deadline) {
-			return Lock{}, fmt.Errorf("fslock: could not acquire lock on %s: %w", path, err)
+			return Lock{}, fmt.Errorf("fslock: could not acquire lock on %s (%v): %w",
+				path, err, ErrLockContended)
 		}
 		time.Sleep(50 * time.Millisecond)
 	}