fix(sandbox): retry provider updates rejected as modified concurrently

internal/sandbox/sandbox.go

@@ -13,6 +13,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"sort"
 	"strings"
 	"syscall"
@@ -52,8 +53,11 @@ const (
 	retryMaxBackoff          = 15 * time.Second
 
 	// providerRetries is the number of times EnsureProvider retries on
-	// transient "unsupported provider type or profile" errors caused by
-	// concurrent ImportProfile processes deleting and reimporting profiles.
+	// transient errors caused by concurrent fullsend runs sharing a
+	// gateway: "unsupported provider type or profile" (a concurrent
+	// ImportProfile deleting and reimporting a changed profile) and the
+	// gateway's optimistic-concurrency rejection of a provider update
+	// ("provider was modified concurrently").
 	providerRetries      = 3
 	providerRetryBackoff = 500 * time.Millisecond
 )
@@ -309,9 +313,12 @@ var reservedCredentialKeys = map[string]bool{
 // into the child process environment, where openshell reads them directly.
 // See https://docs.nvidia.com/openshell/latest/sandboxes/manage-providers#bare-key-form
 //
-// Transient "unsupported provider type or profile" errors are retried with
-// short backoff. These occur when a concurrent ImportProfile process
-// temporarily removes a profile during its delete+reimport cycle.
+// Transient errors from concurrent runs on the same gateway are retried with
+// short backoff: "unsupported provider type or profile" occurs when a
+// concurrent ImportProfile process temporarily removes a profile during its
+// delete+reimport cycle, and "provider was modified concurrently" when two
+// runs update the same provider at once (the gateway rejects the stale
+// resource_version; the update is idempotent, so retrying is safe).
 func EnsureProvider(ctx context.Context, name, providerType string, credentials, config map[string]string, fromURL bool) error {
 	if fromURL {
 		for k := range credentials {
@@ -329,9 +336,8 @@ func EnsureProvider(ctx context.Context, name, providerType string, credentials,
 		if lastErr == nil {
 			return nil
 		}
-		// Retry only on "unsupported provider type or profile" — this is
-		// the transient error from the ImportProfile race.
-		if !isUnsupportedProviderErr(lastErr) {
+		// Retry only on the transient concurrency errors.
+		if !isTransientProviderErr(lastErr) {
 			return lastErr
 		}
 		if attempt < providerRetries-1 {
@@ -345,17 +351,31 @@ func EnsureProvider(ctx context.Context, name, providerType string, credentials,
 	return fmt.Errorf("retries exhausted after %d attempts: %w", providerRetries, lastErr)
 }
 
-// isUnsupportedProviderErr reports whether err contains the openshell
-// error message indicating a missing or not-yet-imported profile.
+// isTransientProviderErr reports whether err is one of the openshell
+// errors produced by concurrent runs racing on the same gateway: a missing
+// or not-yet-reimported profile, or an optimistic-concurrency rejection of
+// a provider update ("provider was modified concurrently (current
+// resource_version: N)").
 //
 // NOTE: This matches literal text from the openshell CLI's stderr output.
 // If openshell changes its error wording in a future version, this check
 // will silently stop matching and retries will no longer trigger. Update
-// the substring if the upstream message changes.
-func isUnsupportedProviderErr(err error) bool {
-	return err != nil && strings.Contains(strings.ToLower(err.Error()), "unsupported provider type or profile")
+// the substrings if the upstream messages change.
+func isTransientProviderErr(err error) bool {
+	if err == nil {
+		return false
+	}
+	msg := strings.ToLower(err.Error())
+	return strings.Contains(msg, "unsupported provider type or profile") ||
+		providerModifiedConcurrentlyRe.MatchString(msg)
 }
 
+// providerModifiedConcurrentlyRe matches openshell's optimistic-concurrency
+// error. The CLI wraps the message across lines with a box-drawing gutter
+// (`"provider was modified` / `│ concurrently (current resource_version: 2)"`),
+// so the two words are matched across any non-word characters.
+var providerModifiedConcurrentlyRe = regexp.MustCompile(`provider was modified\W+concurrently`)
+
 // tryCreateProvider performs a single attempt to create (or update) a
 // provider via openshell. Extracted from EnsureProvider to support retry.
 func tryCreateProvider(ctx context.Context, name string, args, extraEnv, secrets []string, credentials, config map[string]string, fromURL bool) error {

internal/sandbox/sandbox_test.go

@@ -1617,6 +1617,60 @@ exit 0
 	assert.Len(t, entries, 3, "should have made 3 attempts")
 }
 
+// TestEnsureProvider_RetriesConcurrentUpdateConflict verifies that when the
+// provider already exists and the gateway rejects the update because another
+// run modified it concurrently, EnsureProvider retries the create+update
+// cycle instead of failing the run.
+func TestEnsureProvider_RetriesConcurrentUpdateConflict(t *testing.T) {
+	dir := t.TempDir()
+	markerDir := filepath.Join(dir, "markers")
+	require.NoError(t, os.MkdirAll(markerDir, 0o755))
+
+	// Fake openshell: create always reports the provider exists; update
+	// fails with the optimistic-concurrency error until the 3rd attempt.
+	script := fmt.Sprintf(`#!/bin/sh
+if [ "$2" = "create" ]; then
+  echo "Error: × code: 'Some entity that we attempted to create already exists', message: \"provider already exists\"" >&2
+  exit 1
+fi
+if [ "$2" = "update" ]; then
+  echo x > "%s/attempt.$$"
+  count=0
+  for f in "%s"/attempt.*; do
+    [ -e "$f" ] && count=$((count + 1))
+  done
+  if [ "$count" -lt 3 ]; then
+    echo "Error:   × code: 'The operation was aborted', message: \"provider was modified" >&2
+    echo "  │ concurrently (current resource_version: 2)\"" >&2
+    exit 1
+  fi
+  exit 0
+fi
+exit 0
+`, markerDir, markerDir)
+	fakePath := filepath.Join(dir, "openshell")
+	require.NoError(t, os.WriteFile(fakePath, []byte(script), 0o755))
+	t.Setenv("PATH", dir)
+
+	err := EnsureProvider(context.Background(), "vertex-ai", "vertex-ai", nil, nil, false)
+	assert.NoError(t, err, "should succeed after retrying the conflicting update")
+
+	entries, readErr := os.ReadDir(markerDir)
+	require.NoError(t, readErr)
+	assert.Len(t, entries, 3, "should have retried the update 3 times")
+}
+
+func TestIsTransientProviderErr(t *testing.T) {
+	t.Parallel()
+	wrapped := "provider update \"vertex-ai\" failed: exit status 1 (output: Error:   × code: 'The operation was aborted', message: \"provider was modified\n  │ concurrently (current resource_version: 2)\"\n)"
+	assert.False(t, isTransientProviderErr(nil))
+	assert.True(t, isTransientProviderErr(fmt.Errorf("x: unsupported provider type or profile: p")))
+	assert.True(t, isTransientProviderErr(errors.New(wrapped)), "must match across the CLI's line wrap")
+	assert.True(t, isTransientProviderErr(errors.New("provider was modified concurrently")))
+	assert.False(t, isTransientProviderErr(fmt.Errorf("status: PermissionDenied")))
+	assert.False(t, isTransientProviderErr(fmt.Errorf("provider was modified by an operator")))
+}
+
 // TestEnsureProvider_NoRetryOnOtherErrors verifies that non-transient
 // errors are not retried.
 func TestEnsureProvider_NoRetryOnOtherErrors(t *testing.T) {