fix(shim): preserve configuration created while waiting for the lock

CHANGELOG.md

@@ -9,6 +9,9 @@ contract may still change while cfs is pre-1.0.
 
 ### Fixed
 
+- The `cf` shim now checks for an existing default workspace configuration while
+  it holds the context lock. Automatic login copying no longer overwrites
+  configuration created by a concurrent command.
 - `cfs doctor` no longer reports that a saved local target proves current
   authentication.
 - Application tests no longer inherit or read the caller's global Cloud Foundry

README.md

@@ -91,18 +91,18 @@ names fail without creating state.
 
 An ordinary managed `cf` command can copy your saved global API and authentication
 configuration into the workspace's fresh default context without prompting.
-This happens only when its managed `.cf/config.json` is absent at command start
-and the global configuration has a nonblank API `Target`. The copy clears the
-organization and space, so choose them in the workspace:
+This happens only when its managed `.cf/config.json` is absent while the shim
+holds the context lock and the global configuration has a nonblank API `Target`.
+The copy clears the organization and space, so choose them in the workspace:
 
 ```sh
 cd ~/work/orders
 cf target -o commerce -s development
 ```
 
 The shim skips automatic copying if it finds an existing workspace configuration
-at command start, even if the configuration has no API target. Named invocations
-such as `cfs -c prod ...` require explicit import or login.
+while holding the context lock, even if the configuration has no API target.
+Named invocations such as `cfs -c prod ...` require explicit import or login.
 
 To copy the global organization and space as well, run `cfs import` in the
 workspace. To import into a named context, create the context first:

SECURITY.md

@@ -36,12 +36,12 @@ from other local users, but the files are not encrypted by `cfs`; backups and
 host administrators may still access them.
 
 An ordinary managed `cf` command automatically copies `$HOME/.cf/config.json`
-when the default workspace configuration is absent at command start and the
-source has a nonblank API `Target`. This copy requires no confirmation,
-preserves the saved API and authentication configuration, and clears the
-organization and space. The shim skips this copy if it finds an existing
-workspace configuration at command start. Named invocations require explicit
-import or login.
+when the default workspace configuration is absent while the shim holds the
+context lock and the source has a nonblank API `Target`. This copy requires no
+confirmation, preserves the saved API and authentication configuration, and
+clears the organization and space. The shim skips this copy if it finds an
+existing workspace configuration while holding the context lock. Named
+invocations require explicit import or login.
 
 `cfs import` copies the global configuration into the default context or an
 existing named context, including the saved organization and space. It requires

docs/design.md

@@ -106,19 +106,15 @@ There is no setup step for the default context and no mutable `cfs use`
 selection.
 
 On the first managed `cf` invocation in a workspace, `cfs` creates a private CF
-home. If its `.cf/config.json` is absent at command start and the global
-configuration has a nonblank API `Target`, `cfs` attempts to copy the saved API
-and authentication configuration without confirmation. It clears
+home. If its `.cf/config.json` is absent while the shim holds the context lock
+and the global configuration has a nonblank API `Target`, `cfs` attempts to copy
+the saved API and authentication configuration without confirmation. It clears
 `OrganizationFields` and `SpaceFields`, so the user chooses the workspace's
 organization and space.
-The shim skips copying if it finds an existing configuration at command start,
-even without an API target. If no usable source exists, the official CLI starts
-with the workspace's fresh home. Future shells and coding tools in that worktree
-resolve the same home.
-
-The absence check currently runs before acquiring the context lock. A command
-waiting for that lock can overwrite configuration created while it waits.
-Initialize a fresh default context before starting concurrent commands there.
+The shim skips copying if it finds an existing configuration while holding the
+context lock, even without an API target. If no usable source exists, the
+official CLI starts with the workspace's fresh home. Future shells and coding
+tools in that worktree resolve the same home.
 
 An existing global context can be copied explicitly:
 
@@ -289,9 +285,10 @@ For each invocation of `cf <arguments>`:
    context ID from both workspace identity and name.
 6. Validate and create the private state directory.
 7. Acquire the selected context's process lock.
-8. Construct the child environment.
-9. For the ordinary shim path, attempt automatic copying if the workspace
-   configuration was absent and a saved global API target is available.
+8. For the ordinary shim path, inspect the workspace configuration while holding
+   the lock.
+9. Construct the child environment. Attempt automatic copying if the
+   configuration is absent and a saved global API target is available.
 10. Start the official CLI with the original argument vector.
 11. Forward standard input, output, error, terminal behavior, and signals.
 12. Release the lock and return the official CLI exit status.
@@ -303,10 +300,10 @@ func runCF(args []string) int {
     realCF := config.RealCFPath
     scope := resolveScope(os.Getwd(), os.Environ())
     context := store.Open(scope)
-    configMissing := !context.HasCFConfig()
 
     lock := context.Lock()
     defer lock.Release()
+    configMissing := !context.HasCFConfig()
 
     env := cloneEnvironment()
     env.Set("CF_HOME", context.CFHome())
@@ -591,8 +588,8 @@ The MVP is complete only when all of the following are demonstrated:
 13. Unknown or malformed context names cannot create state or fall back to the
     default or global CF home.
 14. A fresh default workspace can copy saved global API and authentication
-    configuration without inheriting the organization or space. Configuration
-    present at command start and the global source remain unchanged.
+    configuration without inheriting the organization or space. Configuration that
+    exists while the context lock is held and the global source remain unchanged.
 15. Named invocations do not copy global configuration automatically.
 
 The automated coverage for these criteria is documented in

internal/app/app.go

@@ -99,12 +99,6 @@ func runShim(options Options, args []string) int {
 		reportWorkspaceError(options, err)
 		return exitUnavailable
 	}
-	workspaceWasEmpty, err := workspaceConfigMissing(managed.Context.CFHome)
-	if err != nil {
-		fprintf(options.Stderr, "cfs: inspect workspace CF state: %v\n", err)
-		return exitError
-	}
-
 	timeout, err := lockTimeout()
 	if err != nil {
 		fprintf(options.Stderr, "cfs: %v\n", err)
@@ -121,13 +115,14 @@ func runShim(options Options, args []string) int {
 		return exitError
 	}
 
+	workspaceWasEmpty, err := workspaceConfigMissing(managed.Context.CFHome)
+	if err != nil {
+		fprintf(options.Stderr, "cfs: inspect workspace CF state: %v\n", errors.Join(err, workspaceLock.Release()))
+		return exitError
+	}
+
 	env := managed.environment(os.Environ())
 
-	// Share one login across workspaces: a brand-new workspace inherits the
-	// global CF login automatically instead of forcing a fresh `cf login` (or a
-	// manual `cfs import`) in every worktree. Only an empty workspace is seeded,
-	// so a target the caller already set here is never overwritten. Auth is
-	// shared; org/space stay per-workspace.
 	seeded := false
 	if workspaceWasEmpty {
 		if home, available, gerr := globalTargetAvailable(); gerr == nil && available {

internal/app/app_linux_test.go

@@ -0,0 +1,115 @@
+package app
+
+import (
+	"bytes"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/zongqichen/cloud-foundry-cli-contexts/internal/lock"
+	"github.com/zongqichen/cloud-foundry-cli-contexts/internal/store"
+	"github.com/zongqichen/cloud-foundry-cli-contexts/internal/workspace"
+	"golang.org/x/sys/unix"
+)
+
+func TestShimPreservesConfigurationCreatedWhileWaitingForLock(t *testing.T) {
+	fakeCF := writeFakeCF(t)
+	stateRoot := canonicalTestPath(t, t.TempDir())
+	configureTestEnvironment(t, fakeCF, stateRoot)
+	t.Setenv("CFS_LOCK_TIMEOUT", "5s")
+	globalHome := t.TempDir()
+	t.Setenv("HOME", globalHome)
+	writeCFConfig(t, globalHome, []byte(`{"Target":"https://synthetic-global.invalid"}`))
+	root := markerWorkspace(t)
+
+	ws, err := workspace.Resolve(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	stateStore := store.New(stateRoot)
+	ctx, err := stateStore.ContextFor(ws)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := stateStore.Prepare(ctx); err != nil {
+		t.Fatal(err)
+	}
+	held, err := lock.Acquire(ctx.LockPath, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	lockHeld := true
+	t.Cleanup(func() {
+		if lockHeld {
+			_ = held.Release()
+		}
+	})
+
+	watch, err := unix.InotifyInit1(unix.IN_CLOEXEC | unix.IN_NONBLOCK)
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = unix.Close(watch) })
+	if _, err := unix.InotifyAddWatch(watch, ctx.LockPath, unix.IN_OPEN); err != nil {
+		t.Fatal(err)
+	}
+
+	previous, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Chdir(root); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.Chdir(previous) })
+
+	result := make(chan commandResult, 1)
+	go func() {
+		var stdout bytes.Buffer
+		var stderr bytes.Buffer
+		code := Run(Options{Args: []string{"cf", "version"}, Stdout: &stdout, Stderr: &stderr})
+		result <- commandResult{code: code, stdout: stdout.String(), stderr: stderr.String()}
+	}()
+
+	events := []unix.PollFd{{Fd: int32(watch), Events: unix.POLLIN}}
+	ready, err := unix.Poll(events, 5_000)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if ready != 1 || events[0].Revents&unix.POLLIN == 0 {
+		t.Fatal("shim did not reach the held context lock")
+	}
+	buffer := make([]byte, unix.SizeofInotifyEvent+unix.NAME_MAX+1)
+	if _, err := unix.Read(watch, buffer); err != nil {
+		t.Fatal(err)
+	}
+
+	want := []byte(`{"ConfigVersion":4}`)
+	writeCFConfig(t, ctx.CFHome, want)
+	if err := held.Release(); err != nil {
+		t.Fatal(err)
+	}
+	lockHeld = false
+
+	var got commandResult
+	select {
+	case got = <-result:
+	case <-time.After(10 * time.Second):
+		t.Fatal("shim did not finish after the context lock was released")
+	}
+	if got.code != exitOK {
+		t.Fatalf("result = %#v", got)
+	}
+	actual, err := os.ReadFile(filepath.Join(ctx.CFHome, ".cf", "config.json"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !bytes.Equal(actual, want) {
+		t.Fatalf("workspace configuration = %s, want %s", actual, want)
+	}
+	if strings.Contains(got.stderr, "seeded this workspace") {
+		t.Fatalf("stderr = %q", got.stderr)
+	}
+}

internal/app/app_unix_test.go

@@ -179,6 +179,42 @@ func TestShimRejectsConcurrentCommandInSameWorkspace(t *testing.T) {
 	}
 }
 
+func TestShimReleasesLockWhenWorkspaceStateInspectionFails(t *testing.T) {
+	fakeCF := writeFakeCF(t)
+	stateRoot := canonicalTestPath(t, t.TempDir())
+	configureTestEnvironment(t, fakeCF, stateRoot)
+	root := markerWorkspace(t)
+
+	ws, err := workspace.Resolve(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	stateStore := store.New(stateRoot)
+	ctx, err := stateStore.ContextFor(ws)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := stateStore.Prepare(ctx); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.MkdirAll(filepath.Join(ctx.CFHome, ".cf", "config.json"), 0o700); err != nil {
+		t.Fatal(err)
+	}
+
+	result := runFromDirectory(t, root, []string{"cf", "apps"})
+	if result.code != exitError || !strings.Contains(result.stderr, "inspect workspace CF state") {
+		t.Fatalf("result = %#v", result)
+	}
+
+	acquired, err := lock.Acquire(ctx.LockPath, 0)
+	if err != nil {
+		t.Fatalf("context lock remained held: %v", err)
+	}
+	if err := acquired.Release(); err != nil {
+		t.Fatal(err)
+	}
+}
+
 func TestShimForwardsInteractiveInputAndArguments(t *testing.T) {
 	fakeCF := writeFakeCF(t)
 	configureTestEnvironment(t, fakeCF, t.TempDir())