Error on duplicate profile ID instead of appending number suffix

When creating a profile without an explicit ID, return an error if the
derived ID already exists rather than silently appending _2, _3, etc.
Removes the now-unused FindWorkingSetsByIDPrefix DB method.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: cmrigney

pkg/db/workingset.go

@@ -11,7 +11,6 @@ import (
 
 type WorkingSetDAO interface {
 	GetWorkingSet(ctx context.Context, id string) (*WorkingSet, error)
-	FindWorkingSetsByIDPrefix(ctx context.Context, prefix string) ([]WorkingSet, error)
 	ListWorkingSets(ctx context.Context) ([]WorkingSet, error)
 	CreateWorkingSet(ctx context.Context, workingSet WorkingSet) error
 	UpdateWorkingSet(ctx context.Context, workingSet WorkingSet) error
@@ -145,17 +144,6 @@ func (d *dao) UpdateWorkingSet(ctx context.Context, workingSet WorkingSet) error
 	return nil
 }
 
-func (d *dao) FindWorkingSetsByIDPrefix(ctx context.Context, prefix string) ([]WorkingSet, error) {
-	const query = `SELECT id, name, servers, secrets FROM working_set WHERE id LIKE $1`
-
-	var workingSets []WorkingSet
-	err := d.db.SelectContext(ctx, &workingSets, query, prefix+"%")
-	if err != nil {
-		return nil, err
-	}
-	return workingSets, nil
-}
-
 func (d *dao) ListWorkingSets(ctx context.Context) ([]WorkingSet, error) {
 	const query = `SELECT id, name, servers, secrets FROM working_set`
 

pkg/db/workingset_test.go

@@ -257,88 +257,6 @@ func TestListWorkingSetsEmpty(t *testing.T) {
 	assert.Empty(t, retrieved)
 }
 
-func TestFindWorkingSetsByIDPrefix(t *testing.T) {
-	dao := setupTestDB(t)
-	ctx := t.Context()
-
-	// Create working sets with different prefixes
-	workingSets := []WorkingSet{
-		{ID: "dev-frontend", Name: "Dev Frontend", Servers: ServerList{}, Secrets: SecretMap{}},
-		{ID: "dev-backend", Name: "Dev Backend", Servers: ServerList{}, Secrets: SecretMap{}},
-		{ID: "prod-frontend", Name: "Prod Frontend", Servers: ServerList{}, Secrets: SecretMap{}},
-		{ID: "prod-backend", Name: "Prod Backend", Servers: ServerList{}, Secrets: SecretMap{}},
-		{ID: "test-env", Name: "Test Env", Servers: ServerList{}, Secrets: SecretMap{}},
-	}
-
-	for _, ws := range workingSets {
-		err := dao.CreateWorkingSet(ctx, ws)
-		require.NoError(t, err)
-	}
-
-	tests := []struct {
-		name          string
-		prefix        string
-		expectedCount int
-		expectedIDs   []string
-	}{
-		{
-			name:          "dev prefix",
-			prefix:        "dev",
-			expectedCount: 2,
-			expectedIDs:   []string{"dev-frontend", "dev-backend"},
-		},
-		{
-			name:          "prod prefix",
-			prefix:        "prod",
-			expectedCount: 2,
-			expectedIDs:   []string{"prod-frontend", "prod-backend"},
-		},
-		{
-			name:          "test prefix",
-			prefix:        "test",
-			expectedCount: 1,
-			expectedIDs:   []string{"test-env"},
-		},
-		{
-			name:          "dev-f prefix (more specific)",
-			prefix:        "dev-f",
-			expectedCount: 1,
-			expectedIDs:   []string{"dev-frontend"},
-		},
-		{
-			name:          "nonexistent prefix",
-			prefix:        "staging",
-			expectedCount: 0,
-			expectedIDs:   []string{},
-		},
-		{
-			name:          "empty prefix",
-			prefix:        "",
-			expectedCount: 5,
-			expectedIDs:   []string{"dev-frontend", "dev-backend", "prod-frontend", "prod-backend", "test-env"},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			retrieved, err := dao.FindWorkingSetsByIDPrefix(ctx, tt.prefix)
-			require.NoError(t, err)
-			expectedCount := len(tt.expectedIDs)
-			assert.Len(t, retrieved, expectedCount)
-
-			if expectedCount > 0 {
-				ids := make(map[string]bool)
-				for _, ws := range retrieved {
-					ids[ws.ID] = true
-				}
-				for _, expectedID := range tt.expectedIDs {
-					assert.True(t, ids[expectedID], "Expected ID %s not found", expectedID)
-				}
-			}
-		})
-	}
-}
-
 func TestServerListJSONMarshaling(t *testing.T) {
 	dao := setupTestDB(t)
 	ctx := t.Context()

pkg/workingset/create_test.go

@@ -196,7 +196,7 @@ func TestCreateWithExistingId(t *testing.T) {
 	assert.Contains(t, err.Error(), "already exists")
 }
 
-func TestCreateGeneratesUniqueIds(t *testing.T) {
+func TestCreateErrorsOnDuplicateDerivedID(t *testing.T) {
 	dao := setupTestDB(t)
 	ctx := t.Context()
 
@@ -206,34 +206,12 @@ func TestCreateGeneratesUniqueIds(t *testing.T) {
 	}, []string{})
 	require.NoError(t, err)
 
-	// Create second with same name
+	// Create second with same name should fail
 	err = Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "", "Test Set", []string{
 		"docker://anotherimage:v1.0",
 	}, []string{})
-	require.NoError(t, err)
-
-	// Create third with same name
-	err = Create(ctx, dao, getMockRegistryClient(), getMockOciService(), "", "Test Set", []string{
-		"docker://anotherimage:v1.0",
-	}, []string{})
-	require.NoError(t, err)
-
-	// List all working sets
-	sets, err := dao.ListWorkingSets(ctx)
-	require.NoError(t, err)
-	assert.Len(t, sets, 3)
-
-	// Verify IDs are unique
-	ids := make(map[string]bool)
-	for _, set := range sets {
-		assert.False(t, ids[set.ID], "ID %s should be unique", set.ID)
-		ids[set.ID] = true
-	}
-
-	// Verify ID pattern
-	assert.Contains(t, ids, "test_set")
-	assert.Contains(t, ids, "test_set_2")
-	assert.Contains(t, ids, "test_set_3")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "already exists")
 }
 
 func TestCreateWithInvalidServerFormat(t *testing.T) {

pkg/workingset/workingset.go

@@ -360,30 +360,15 @@ func createWorkingSetID(ctx context.Context, name string, dao db.DAO) (string, e
 	cleaned := re.ReplaceAllString(name, "_")
 	baseName := strings.ToLower(cleaned)
 
-	existingSets, err := dao.FindWorkingSetsByIDPrefix(ctx, baseName)
-	if err != nil {
-		return "", fmt.Errorf("failed to find profiles by name prefix: %w", err)
-	}
-
-	if len(existingSets) == 0 {
-		return baseName, nil
+	_, err := dao.GetWorkingSet(ctx, baseName)
+	if err == nil {
+		return "", fmt.Errorf("a profile with id %q already exists", baseName)
 	}
-
-	takenIDs := make(map[string]bool)
-	for _, set := range existingSets {
-		takenIDs[set.ID] = true
-	}
-
-	// TODO(cody): there are better ways to do this, but this is a simple brute force for now
-	// Append a number to the base name
-	for i := 2; i <= 100; i++ {
-		newName := fmt.Sprintf("%s_%d", baseName, i)
-		if !takenIDs[newName] {
-			return newName, nil
-		}
+	if !errors.Is(err, sql.ErrNoRows) {
+		return "", fmt.Errorf("failed to look for existing profile: %w", err)
 	}
 
-	return "", fmt.Errorf("failed to create profile id")
+	return baseName, nil
 }
 
 func ResolveServersFromString(ctx context.Context, registryClient registryapi.Client, ociService oci.Service, dao db.DAO, value string) ([]Server, error) {

pkg/workingset/workingset_test.go

@@ -902,6 +902,7 @@ func TestCreateWorkingSetID(t *testing.T) {
 		inputName   string
 		existingIDs []string
 		expectedID  string
+		expectError bool
 	}{
 		{
 			name:       "simple name",
@@ -919,16 +920,10 @@ func TestCreateWorkingSetID(t *testing.T) {
 			expectedID: "my_working_set_",
 		},
 		{
-			name:        "name with collision",
+			name:        "name with collision returns error",
 			inputName:   "test",
 			existingIDs: []string{"test"},
-			expectedID:  "test_2",
-		},
-		{
-			name:        "name with multiple collisions",
-			inputName:   "test",
-			existingIDs: []string{"test", "test_2", "test_3"},
-			expectedID:  "test_4",
+			expectError: true,
 		},
 	}
 
@@ -950,8 +945,12 @@ func TestCreateWorkingSetID(t *testing.T) {
 			}
 
 			id, err := createWorkingSetID(ctx, tt.inputName, dao)
-			require.NoError(t, err)
-			assert.Equal(t, tt.expectedID, id)
+			if tt.expectError {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, tt.expectedID, id)
+			}
 		})
 	}
 }