fix(store): both workspace mints take slug-then-owner locks in one order (BUG-2808 PR A review r2)

CreateWorkspace locked the owner and then inserted; ImportWorkspace inserts
and then locks. On Postgres a create and an import of one name for one owner
formed a cycle: the create held the owner lock while waiting on the import's
uncommitted slug row, and the import then asked for the owner lock. The
create now inserts first and counts with pendingOwn=1, like the import, and a
test builds the interleaving under lock_timeout (red with the old order:
SQLSTATE 55P03). Two doc comments narrowed to what the code guarantees.

Claude-Session: https://claude.ai/code/session_01BLg7wiBeSgXvAhWi23mT4Z

internal/server/handlers_workspaces.go

@@ -420,10 +420,11 @@ func (s *Server) handleCreateWorkspace(w http.ResponseWriter, r *http.Request) {
 		input.OwnerID = mint.OwnerID
 	}
 
-	// Charged to the AUTHENTICATED owner, never to input.OwnerID: for a
-	// caller with no resolved user that field is whatever the body said,
-	// and a body must not be able to spend someone else's plan. The
-	// pre-check in beginWorkspaceMint charges the same identity.
+	// The limit is enabled only for an AUTHENTICATED owner (mint.OwnerID),
+	// which input.OwnerID equals by this point. For a caller with no
+	// resolved user input.OwnerID is whatever the body said, and a body must
+	// not be able to spend someone else's plan. The pre-check in
+	// beginWorkspaceMint makes the same decision.
 	ws, err := s.store.CreateWorkspace(input, s.planLimitMintOpts(mint.OwnerID)...)
 	if err != nil {
 		if writeStorePlanLimitError(w, err, "") {

internal/store/api_tokens.go

@@ -20,7 +20,8 @@ import (
 // CreateAPIToken mints a user-owned token. With WithPlanLimit() the insert runs
 // in a transaction that first takes the owner lock and counts the owner's
 // tokens (enforceUserLimitTx, BUG-2808), refusing with a *PlanLimitError at the
-// limit. Every door that mints through here therefore enforces the same cap.
+// limit. Only callers that pass the option are limited; both server token
+// doors do, in cloud mode.
 func (s *Store) CreateAPIToken(userID string, input models.APITokenCreate, defaultExpiryDays, maxLifetimeDays int, opts ...MintOption) (*models.APITokenWithSecret, error) {
 	// Generate 32 random bytes → 64 hex chars
 	raw := make([]byte, 32)

internal/store/export.go

@@ -1126,7 +1126,9 @@ func (s *Store) ImportWorkspace(data *models.WorkspaceExport, newName string, ow
 	// at the top: the owner lock it takes is held until the commit, and an
 	// import of a large bundle would otherwise hold it for the whole import.
 	// The count therefore includes this transaction's own workspace row,
-	// which is what pendingOwn=1 accounts for.
+	// which is what pendingOwn=1 accounts for. CreateWorkspace takes the same
+	// two locks in the same order (slug row, then owner), which is what keeps
+	// a concurrent create and import of one name from deadlocking.
 	if resolveMintOptions(opts).planLimit {
 		if err := s.enforceUserLimitTx(tx, ownerID, "workspaces", 1); err != nil {
 			return nil, err

internal/store/limits.go

@@ -248,8 +248,9 @@ func resolveMintOptions(opts []MintOption) mintOptions {
 //     committed before the lock was granted.
 //
 // pendingOwn is the number of rows THIS transaction has already inserted that
-// the count will include (1 when the check runs after the insert, as
-// ImportWorkspace does to keep the lock short; 0 when it runs before).
+// the count will include: 1 for the workspace mints, which insert first and
+// then lock (a single lock order for both, and a short hold for the import);
+// 0 for CreateAPIToken, which locks before it inserts.
 //
 // A missing owner is an error, not a pass: the caller asked for a limit and
 // there is no plan to read it from.

internal/store/limits_mint_race_test.go

@@ -229,3 +229,64 @@ func TestLimitedMint_WithoutOption_NotEnforced(t *testing.T) {
 		t.Errorf("CreateAPIToken with the option at cap: err = %v, want *PlanLimitError", err)
 	}
 }
+
+// Both workspace mints take their two locks in ONE order: the workspace row's
+// UNIQUE slug entry, then the owner's users row (review round 2 on PR A). This
+// builds the interleaving the opposite order deadlocks on. An import-shaped
+// transaction has inserted slug "same-name" and not committed. A limited
+// create of the same name is then observed waiting on that row. The import
+// then asks for the owner lock under a short lock_timeout. If the create held
+// the owner lock while it waited (the old lock-then-insert order), that request
+// would wait and time out. With one order the create holds nothing, the lock
+// is granted at once, and the import commits.
+func TestLimitedMint_CreateAndImportOfOneName_NoLockCycle(t *testing.T) {
+	t.Parallel()
+	s := testStore(t)
+	if s.dialect.Driver() != DriverPostgres {
+		t.Skip("PAD_TEST_POSTGRES_URL not set — the cycle needs row locks, which SQLite does not have")
+	}
+	u, err := s.CreateUser(models.UserCreate{Email: "bug2808-cycle@example.com", Name: "C", Password: "pw-bug2808-12345"})
+	if err != nil {
+		t.Fatalf("CreateUser: %v", err)
+	}
+	if err := s.SetUserPlan(u.ID, "free", ""); err != nil {
+		t.Fatalf("SetUserPlan: %v", err)
+	}
+	if err := s.SetUserPlanOverrides(u.ID, `{"workspaces":5}`); err != nil {
+		t.Fatalf("SetUserPlanOverrides: %v", err)
+	}
+
+	imp, err := s.db.Begin()
+	if err != nil {
+		t.Fatalf("begin import-shaped tx: %v", err)
+	}
+	defer imp.Rollback()
+	if _, err := s.createWorkspaceQ(imp, models.WorkspaceCreate{Name: "same-name", OwnerID: u.ID}); err != nil {
+		t.Fatalf("import-shaped insert: %v", err)
+	}
+
+	done := make(chan error, 1)
+	go func() {
+		_, err := s.CreateWorkspace(models.WorkspaceCreate{Name: "same-name", OwnerID: u.ID}, WithPlanLimit())
+		done <- err
+	}()
+	waitForLockWait(t, s, "INSERT INTO workspaces", done)
+
+	if _, err := imp.Exec(`SET LOCAL lock_timeout = '500ms'`); err != nil {
+		t.Fatalf("set lock_timeout: %v", err)
+	}
+	if _, err := imp.Exec(s.q(`SELECT id FROM users WHERE id = ? FOR NO KEY UPDATE`), u.ID); err != nil {
+		t.Fatalf("the import could not take the owner lock while the create waited on its slug: %v "+
+			"(the create is holding the owner lock, which is the order that deadlocks)", err)
+	}
+	if err := imp.Commit(); err != nil {
+		t.Fatalf("commit import-shaped tx: %v", err)
+	}
+	// The create loses the slug race exactly as it did before BUG-2808 (a
+	// unique violation). What matters here is that it finishes.
+	select {
+	case <-done:
+	case <-time.After(10 * time.Second):
+		t.Fatal("the create never finished after the import committed")
+	}
+}

internal/store/workspaces.go

@@ -89,6 +89,14 @@ type execQueryer interface {
 // With WithPlanLimit() the mint runs in its own transaction and is refused with
 // a *PlanLimitError when the owner is already at their workspaces limit,
 // counted under the owner lock (enforceUserLimitTx, BUG-2808).
+//
+// INSERT FIRST, THEN LOCK AND COUNT, the same order ImportWorkspace uses. Both
+// workspace mints must take their two locks in one order: the workspace row's
+// UNIQUE slug entry, then the owner's users row. Review round 2 on PR A found
+// the cycle the opposite order makes on Postgres: a create holding the owner
+// lock while it waits on an import's uncommitted row for the same slug, and
+// the import asking for the owner lock at its end. With one order, the create
+// waits on the slug holding nothing, and the import commits.
 func (s *Store) CreateWorkspace(input models.WorkspaceCreate, opts ...MintOption) (*models.Workspace, error) {
 	if !resolveMintOptions(opts).planLimit {
 		return s.createWorkspaceQ(s.db, input)
@@ -98,13 +106,13 @@ func (s *Store) CreateWorkspace(input models.WorkspaceCreate, opts ...MintOption
 		return nil, fmt.Errorf("create workspace: begin: %w", err)
 	}
 	defer tx.Rollback()
-	if err := s.enforceUserLimitTx(tx, input.OwnerID, "workspaces", 0); err != nil {
-		return nil, err
-	}
 	ws, err := s.createWorkspaceQ(tx, input)
 	if err != nil {
 		return nil, err
 	}
+	if err := s.enforceUserLimitTx(tx, input.OwnerID, "workspaces", 1); err != nil {
+		return nil, err
+	}
 	if err := tx.Commit(); err != nil {
 		return nil, fmt.Errorf("create workspace: commit: %w", err)
 	}