fix(service): detach the skill space cleanup from the request context (#21648)

## Summary

- `SkillSpaceService.DeleteSpace` passes the gin request context to `go
s.asyncDeleteSpace(ctx, ...)`. net/http cancels that context when
`ServeHTTP` returns, so the cleanup's first DB call fails with `context
canceled` and the space stays at status `2` ("deleting") forever — the
delete endpoint then answers the idempotent "already deleting" success
and the name can no longer be reused.
- `asyncDeleteSpace` now derives its own context with
`context.WithoutCancel`, matching the other detached-after-response
paths in this tree (`internal/admin/user_delete.go`,
`internal/agent/sandbox/manager_client.go`,
`internal/ingestion/service/ingestion_service.go`).
- Drops the stale comment above the folder step that described the
requirement the code did not meet.


Fixes #21647

internal/service/skill_space.go

@@ -18,6 +18,7 @@ package service
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"ragflow/internal/common"
 	"ragflow/internal/dao"
@@ -442,9 +443,12 @@ func (s *SkillSpaceService) DeleteSpace(ctx context.Context, userID, spaceID str
 		return code, normalized
 	}
 
-	// If already deleting, return success (idempotent)
+	// A space still at "2" was left behind by an earlier cleanup that could not
+	// finish, so a repeat delete is how it gets retried: re-drive the cleanup
+	// instead of answering with a no-op success that would strand it for good.
 	if space.Status == entity.SpaceStatusDeleting {
-		common.Info("Space is already being deleted", zap.String("spaceID", spaceID))
+		common.Info("Space is already deleting, restarting the cleanup", zap.String("spaceID", spaceID))
+		go s.asyncDeleteSpace(ctx, spaceID, space.FolderID, tenantID, docEngine)
 		return common.CodeSuccess, nil
 	}
 
@@ -475,13 +479,16 @@ func (s *SkillSpaceService) DeleteSpace(ctx context.Context, userID, spaceID str
 // asyncDeleteSpace performs the actual deletion work in the background.
 // It deletes the search index, removes files via Go FileService, and soft-deletes the space record.
 func (s *SkillSpaceService) asyncDeleteSpace(ctx context.Context, spaceID, folderID, tenantID string, docEngine engine.DocEngine) {
-	bgCtx, bgCancel := context.WithTimeout(ctx, 120*time.Second)
-	defer bgCancel()
+	// Detach from the caller's cancellation: the request context is canceled once
+	// the handler returns, which is before this goroutine reaches the database.
+	ctx = context.WithoutCancel(ctx)
 
 	defer func() {
 		if r := recover(); r != nil {
 			common.Warn("Panic in asyncDeleteSpace, marking space as deleted", zap.Any("recover", r), zap.String("spaceID", spaceID))
-			_, _ = s.spaceDAO.CASStatus(bgCtx, dao.DB, spaceID, entity.SpaceStatusDeleting, entity.SpaceStatusDeleted)
+			finalCtx, finalCancel := context.WithTimeout(ctx, 30*time.Second)
+			defer finalCancel()
+			_, _ = s.spaceDAO.CASStatus(finalCtx, dao.DB, spaceID, entity.SpaceStatusDeleting, entity.SpaceStatusDeleted)
 		}
 	}()
 
@@ -500,36 +507,53 @@ func (s *SkillSpaceService) asyncDeleteSpace(ctx context.Context, spaceID, folde
 	}
 
 	// Step 2: Delete folder and storage via Go FileService
-	// Use a fresh background context with timeout, NOT the incoming ctx (which
-	// is the HTTP request context canceled when the handler returns and the
-	// goroutine starts executing).
 	common.Info("Async deleting space folder via Go FileService", zap.String("folderID", folderID), zap.String("spaceID", spaceID))
-	ctxFS, cancelFS := context.WithTimeout(ctx, 60*time.Second)
-	defer cancelFS()
-	err := s.fileService.DeleteFiles(ctxFS, tenantID, []string{folderID})
-	if err != nil {
-		common.Error(fmt.Sprintf("Failed to delete space folder via Go FileService during async delete, spaceID=%s, error=%v", spaceID, err), nil)
-		// Retry once with a delay (same ctxFS, still valid)
-		time.Sleep(5 * time.Second)
-		if retryErr := s.fileService.DeleteFiles(ctxFS, tenantID, []string{folderID}); retryErr != nil {
-			common.Error(fmt.Sprintf("Retry failed to delete space folder, marking space as deleted anyway, spaceID=%s, error=%v", spaceID, retryErr), nil)
-			// Mark as deleted even if folder deletion fails - orphaned folders can be cleaned up later
+	// DeleteFiles authorizes the whole tree against the folder creator's grant,
+	// and a tenant member can create the space while only the tenant owner may
+	// delete it, so tenantID is not the principal that owns the folder.
+	actingUserID, folderGone, ownerErr := s.resolveSpaceFolderOwner(ctx, folderID)
+	switch {
+	case folderGone:
+		common.Info("Space folder is already gone, skipping the folder cleanup", zap.String("folderID", folderID))
+	case actingUserID == "":
+		// Any attempt now would be authorized against a principal that does not
+		// own the folder, so DeleteFiles would refuse it and the space would be
+		// reported deleted while its folder rows and objects survive. Keep the
+		// space at "2"; a repeat delete request re-drives this cleanup.
+		common.Error(fmt.Sprintf("Failed to resolve the space folder creator, keeping the space in the deleting state, spaceID=%s, folderID=%s, error=%v", spaceID, folderID, ownerErr), nil)
+		return
+	default:
+		ctxFS, cancelFS := context.WithTimeout(ctx, 60*time.Second)
+		defer cancelFS()
+		err := s.fileService.DeleteFiles(ctxFS, actingUserID, []string{folderID})
+		if err != nil {
+			common.Error(fmt.Sprintf("Failed to delete space folder via Go FileService during async delete, spaceID=%s, error=%v", spaceID, err), nil)
+			// Retry once with a delay (same ctxFS, still valid)
+			time.Sleep(5 * time.Second)
+			if retryErr := s.fileService.DeleteFiles(ctxFS, actingUserID, []string{folderID}); retryErr != nil {
+				common.Error(fmt.Sprintf("Retry failed to delete space folder, marking space as deleted anyway, spaceID=%s, error=%v", spaceID, retryErr), nil)
+				// Mark as deleted even if folder deletion fails - orphaned folders can be cleaned up later
+			} else {
+				common.Info("Successfully deleted space folder on retry via Go FileService", zap.String("folderID", folderID))
+			}
 		} else {
-			common.Info("Successfully deleted space folder on retry via Go FileService", zap.String("folderID", folderID))
+			common.Info("Successfully deleted space folder via Go FileService", zap.String("folderID", folderID))
 		}
-	} else {
-		common.Info("Successfully deleted space folder via Go FileService", zap.String("folderID", folderID))
 	}
 
 	// Step 3: Soft delete the space record (status "2" → "0")
 	// First, permanently remove any previously deleted spaces with the same tenant+name
 	// to avoid UNIQUE INDEX constraint violation when changing status from "2" to "0"
-	space, err := s.spaceDAO.GetByIDAnyStatus(bgCtx, dao.DB, spaceID)
+	// The index and folder steps can outlast one cleanup window, so the final status
+	// update gets its own budget; an expired context would strand the space at "2".
+	finalCtx, finalCancel := context.WithTimeout(ctx, 30*time.Second)
+	defer finalCancel()
+	space, err := s.spaceDAO.GetByIDAnyStatus(finalCtx, dao.DB, spaceID)
 	if err == nil && space != nil {
-		_ = s.spaceDAO.DeletePermanentByName(bgCtx, dao.DB, space.TenantID, space.Name)
+		_ = s.spaceDAO.DeletePermanentByName(finalCtx, dao.DB, space.TenantID, space.Name)
 	}
 
-	swapped, err := s.spaceDAO.CASStatus(bgCtx, dao.DB, spaceID, entity.SpaceStatusDeleting, entity.SpaceStatusDeleted)
+	swapped, err := s.spaceDAO.CASStatus(finalCtx, dao.DB, spaceID, entity.SpaceStatusDeleting, entity.SpaceStatusDeleted)
 	if err != nil {
 		common.Error(fmt.Sprintf("Failed to update space status to deleted, spaceID=%s", spaceID), err)
 		return
@@ -542,6 +566,33 @@ func (s *SkillSpaceService) asyncDeleteSpace(ctx context.Context, spaceID, folde
 	common.Info("Successfully completed async space deletion", zap.String("spaceID", spaceID))
 }
 
+// resolveSpaceFolderOwner returns the user whose grant authorizes deleting the
+// space folder. folderGone reports a row that no longer exists, which is not a
+// failure. An empty user without an error means the row carries no creator, so
+// the principal cannot be derived from it.
+//
+// The detached cleanup context has no deadline, so every attempt bounds its own
+// read; a stalled lookup would otherwise block the cleanup before the bounded
+// delete. Transient lookup errors are retried because guessing the principal
+// makes the guarded delete fail every attempt.
+func (s *SkillSpaceService) resolveSpaceFolderOwner(ctx context.Context, folderID string) (string, bool, error) {
+	const attempts = 2
+	var lastErr error
+	for attempt := 0; attempt < attempts; attempt++ {
+		lookupCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
+		folder, err := s.fileDAO.GetByID(lookupCtx, dao.DB, folderID)
+		cancel()
+		if err == nil {
+			return folder.CreatedBy, false, nil
+		}
+		if errors.Is(err, gorm.ErrRecordNotFound) {
+			return "", true, nil
+		}
+		lastErr = err
+	}
+	return "", false, lastErr
+}
+
 // deleteFolderRecursive recursively deletes a folder and all its contents
 func (s *SkillSpaceService) deleteFolderRecursive(ctx context.Context, folderID string) error {
 	// Get all children

internal/service/skill_space_test.go

@@ -0,0 +1,334 @@
+//
+//  Copyright 2026 The InfiniFlow Authors. All Rights Reserved.
+//
+//  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 service
+
+import (
+	"context"
+	"errors"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"ragflow/internal/common"
+	"ragflow/internal/dao"
+	"ragflow/internal/entity"
+	"ragflow/internal/permission"
+	"ragflow/internal/storage"
+
+	"gorm.io/gorm"
+)
+
+// TestAsyncDeleteSpaceOutlivesTheRequestContext pins the cleanup goroutine to a
+// context that survives the request. Gin cancels c.Request.Context() as soon as
+// the handler returns, so inheriting it left the space at status "2" forever:
+// the delete endpoint answers with the idempotent "already deleting" success on
+// every retry and space creation refuses the same name.
+func TestAsyncDeleteSpaceOutlivesTheRequestContext(t *testing.T) {
+	db := setupServiceTestDB(t)
+	if err := db.AutoMigrate(&entity.SkillSpace{}); err != nil {
+		t.Fatalf("failed to migrate skill space: %v", err)
+	}
+	pushServiceDB(t, db)
+
+	space := &entity.SkillSpace{
+		ID:       "space-1",
+		TenantID: "tenant-1",
+		Name:     "skills",
+		FolderID: "folder-1",
+		Status:   entity.SpaceStatusDeleting,
+	}
+	if err := db.Create(space).Error; err != nil {
+		t.Fatalf("create space: %v", err)
+	}
+
+	requestCtx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	NewSkillSpaceService(nil).asyncDeleteSpace(requestCtx, space.ID, space.FolderID, space.TenantID, nil)
+
+	stored, err := dao.NewSkillSpaceDAO().GetByIDAnyStatus(context.Background(), db, space.ID)
+	if err != nil {
+		t.Fatalf("reload space: %v", err)
+	}
+	if stored.Status != entity.SpaceStatusDeleted {
+		t.Fatalf("space status = %q, want %q: cleanup still inherits the canceled request context", stored.Status, entity.SpaceStatusDeleted)
+	}
+}
+
+// TestAsyncDeleteSpaceDeletesTheFolderAsItsCreator pins the acting user of the
+// folder cleanup to the folder creator. DeleteFiles authorizes every entry in
+// the tree against the creator's grant, so a tenant member can create the space
+// while the tenant owner deletes it; passing the tenant ID left the folder rows
+// and storage objects behind even though the space was marked deleted.
+func TestAsyncDeleteSpaceDeletesTheFolderAsItsCreator(t *testing.T) {
+	db := setupServiceTestDB(t)
+	if err := db.AutoMigrate(&entity.SkillSpace{}); err != nil {
+		t.Fatalf("failed to migrate skill space: %v", err)
+	}
+	pushServiceDB(t, db)
+
+	factory := storage.GetStorageFactory()
+	previous := factory.GetStorage()
+	factory.SetStorage(storage.NewMemoryStorage())
+	t.Cleanup(func() { factory.SetStorage(previous) })
+
+	for _, file := range []*entity.File{
+		{ID: "skills-root", ParentID: "skills-root", TenantID: "tenant-1", CreatedBy: "owner-1", Name: "/", Type: "folder"},
+		{ID: "folder-1", ParentID: "skills-root", TenantID: "tenant-1", CreatedBy: "member-1", Name: "skills", Type: "folder"},
+		{ID: "file-1", ParentID: "folder-1", TenantID: "tenant-1", CreatedBy: "member-1", Name: "skill.md", Type: "doc"},
+	} {
+		if err := db.Create(file).Error; err != nil {
+			t.Fatalf("create file %s: %v", file.ID, err)
+		}
+	}
+
+	space := &entity.SkillSpace{
+		ID:       "space-1",
+		TenantID: "tenant-1",
+		Name:     "skills",
+		FolderID: "folder-1",
+		Status:   entity.SpaceStatusDeleting,
+	}
+	if err := db.Create(space).Error; err != nil {
+		t.Fatalf("create space: %v", err)
+	}
+
+	NewSkillSpaceService(nil).asyncDeleteSpace(context.Background(), space.ID, space.FolderID, space.TenantID, nil)
+
+	fileDAO := dao.NewFileDAO()
+	for _, id := range []string{"folder-1", "file-1"} {
+		if _, err := fileDAO.GetByID(context.Background(), db, id); err == nil {
+			t.Fatalf("file %q survived the space cleanup: DeleteFiles ran as a non-creator", id)
+		}
+	}
+	stored, err := dao.NewSkillSpaceDAO().GetByIDAnyStatus(context.Background(), db, space.ID)
+	if err != nil {
+		t.Fatalf("reload space: %v", err)
+	}
+	if stored.Status != entity.SpaceStatusDeleted {
+		t.Fatalf("space status = %q, want %q", stored.Status, entity.SpaceStatusDeleted)
+	}
+}
+
+// failFolderLookupsOnce fails the first budget reads of the file table with a
+// non-not-found error, standing in for a transient database failure that leaves
+// the folder row intact.
+func failFolderLookupsOnce(t *testing.T, db *gorm.DB, budget int32) {
+	t.Helper()
+	var calls int32
+	err := db.Callback().Query().Before("gorm:query").Register("test:fail_folder_lookup", func(tx *gorm.DB) {
+		if tx.Statement.Table != "file" {
+			return
+		}
+		if atomic.AddInt32(&calls, 1) <= budget {
+			tx.AddError(errors.New("transient lookup failure"))
+		}
+	})
+	if err != nil {
+		t.Fatalf("register lookup failure hook: %v", err)
+	}
+}
+
+// TestAsyncDeleteSpaceRetriesTheFolderLookup pins the creator lookup to a retry.
+// Falling back to the tenant ID on a transient read error made DeleteFiles
+// authorize against a principal that does not own a member-created folder, so
+// the retry is what keeps the space and its folder in agreement.
+func TestAsyncDeleteSpaceRetriesTheFolderLookup(t *testing.T) {
+	db := setupServiceTestDB(t)
+	if err := db.AutoMigrate(&entity.SkillSpace{}); err != nil {
+		t.Fatalf("failed to migrate skill space: %v", err)
+	}
+	pushServiceDB(t, db)
+
+	factory := storage.GetStorageFactory()
+	previous := factory.GetStorage()
+	factory.SetStorage(storage.NewMemoryStorage())
+	t.Cleanup(func() { factory.SetStorage(previous) })
+
+	for _, file := range []*entity.File{
+		{ID: "skills-root", ParentID: "skills-root", TenantID: "tenant-1", CreatedBy: "owner-1", Name: "/", Type: "folder"},
+		{ID: "folder-1", ParentID: "skills-root", TenantID: "tenant-1", CreatedBy: "member-1", Name: "skills", Type: "folder"},
+		{ID: "file-1", ParentID: "folder-1", TenantID: "tenant-1", CreatedBy: "member-1", Name: "skill.md", Type: "doc"},
+	} {
+		if err := db.Create(file).Error; err != nil {
+			t.Fatalf("create file %s: %v", file.ID, err)
+		}
+	}
+
+	space := &entity.SkillSpace{
+		ID:       "space-1",
+		TenantID: "tenant-1",
+		Name:     "skills",
+		FolderID: "folder-1",
+		Status:   entity.SpaceStatusDeleting,
+	}
+	if err := db.Create(space).Error; err != nil {
+		t.Fatalf("create space: %v", err)
+	}
+
+	failFolderLookupsOnce(t, db, 1)
+
+	NewSkillSpaceService(nil).asyncDeleteSpace(context.Background(), space.ID, space.FolderID, space.TenantID, nil)
+
+	fileDAO := dao.NewFileDAO()
+	for _, id := range []string{"folder-1", "file-1"} {
+		if _, err := fileDAO.GetByID(context.Background(), db, id); err == nil {
+			t.Fatalf("file %q survived the space cleanup: the lookup retry did not run", id)
+		}
+	}
+	stored, err := dao.NewSkillSpaceDAO().GetByIDAnyStatus(context.Background(), db, space.ID)
+	if err != nil {
+		t.Fatalf("reload space: %v", err)
+	}
+	if stored.Status != entity.SpaceStatusDeleted {
+		t.Fatalf("space status = %q, want %q", stored.Status, entity.SpaceStatusDeleted)
+	}
+}
+
+// TestAsyncDeleteSpaceKeepsTheDeletingStatusWhenTheFolderCreatorCannotBeRead
+// pins the outcome of an unreadable folder creator: the space must stay at "2"
+// with its folder intact rather than be reported deleted while the rows and
+// storage objects survive.
+func TestAsyncDeleteSpaceKeepsTheDeletingStatusWhenTheFolderCreatorCannotBeRead(t *testing.T) {
+	db := setupServiceTestDB(t)
+	if err := db.AutoMigrate(&entity.SkillSpace{}); err != nil {
+		t.Fatalf("failed to migrate skill space: %v", err)
+	}
+	pushServiceDB(t, db)
+
+	for _, file := range []*entity.File{
+		{ID: "skills-root", ParentID: "skills-root", TenantID: "tenant-1", CreatedBy: "owner-1", Name: "/", Type: "folder"},
+		{ID: "folder-1", ParentID: "skills-root", TenantID: "tenant-1", CreatedBy: "member-1", Name: "skills", Type: "folder"},
+	} {
+		if err := db.Create(file).Error; err != nil {
+			t.Fatalf("create file %s: %v", file.ID, err)
+		}
+	}
+
+	space := &entity.SkillSpace{
+		ID:       "space-1",
+		TenantID: "tenant-1",
+		Name:     "skills",
+		FolderID: "folder-1",
+		Status:   entity.SpaceStatusDeleting,
+	}
+	if err := db.Create(space).Error; err != nil {
+		t.Fatalf("create space: %v", err)
+	}
+
+	// Both lookup attempts fail with a non-not-found error.
+	failFolderLookupsOnce(t, db, 2)
+
+	NewSkillSpaceService(nil).asyncDeleteSpace(context.Background(), space.ID, space.FolderID, space.TenantID, nil)
+
+	if _, err := dao.NewFileDAO().GetByID(context.Background(), db, "folder-1"); err != nil {
+		t.Fatalf("folder was deleted without knowing its creator: %v", err)
+	}
+	stored, err := dao.NewSkillSpaceDAO().GetByIDAnyStatus(context.Background(), db, space.ID)
+	if err != nil {
+		t.Fatalf("reload space: %v", err)
+	}
+	if stored.Status != entity.SpaceStatusDeleting {
+		t.Fatalf("space status = %q, want %q: the space was reported deleted while its folder survived", stored.Status, entity.SpaceStatusDeleting)
+	}
+}
+
+// TestDeleteSpaceRestartsCleanupLeftInTheDeletingState pins the retry path for a
+// space an earlier cleanup could not finish. Answering the repeat delete with an
+// idempotent success left the space at "2" for good: cleanup never ran again and
+// space creation kept refusing the name.
+func TestDeleteSpaceRestartsCleanupLeftInTheDeletingState(t *testing.T) {
+	db := setupServiceTestDB(t)
+	if err := db.AutoMigrate(&entity.SkillSpace{}); err != nil {
+		t.Fatalf("failed to migrate skill space: %v", err)
+	}
+	pushServiceDB(t, db)
+
+	// The cleanup goroutine and the poll below hit the database at the same
+	// time, and an in-memory SQLite database only carries its schema on the
+	// connection that created it.
+	sqlDB, err := db.DB()
+	if err != nil {
+		t.Fatalf("unwrap test database: %v", err)
+	}
+	sqlDB.SetMaxOpenConns(1)
+
+	valid := string(entity.StatusValid)
+	if err := db.Create(&entity.UserTenant{
+		ID:       "owner-membership",
+		UserID:   "owner-1",
+		TenantID: "tenant-1",
+		Role:     string(permission.RoleOwner),
+		Status:   &valid,
+	}).Error; err != nil {
+		t.Fatalf("create membership: %v", err)
+	}
+
+	factory := storage.GetStorageFactory()
+	previous := factory.GetStorage()
+	factory.SetStorage(storage.NewMemoryStorage())
+	t.Cleanup(func() { factory.SetStorage(previous) })
+
+	for _, file := range []*entity.File{
+		{ID: "skills-root", ParentID: "skills-root", TenantID: "tenant-1", CreatedBy: "owner-1", Name: "/", Type: "folder"},
+		{ID: "folder-1", ParentID: "skills-root", TenantID: "tenant-1", CreatedBy: "member-1", Name: "skills", Type: "folder"},
+		{ID: "file-1", ParentID: "folder-1", TenantID: "tenant-1", CreatedBy: "member-1", Name: "skill.md", Type: "doc"},
+	} {
+		if err := db.Create(file).Error; err != nil {
+			t.Fatalf("create file %s: %v", file.ID, err)
+		}
+	}
+
+	space := &entity.SkillSpace{
+		ID:       "space-1",
+		TenantID: "tenant-1",
+		Name:     "skills",
+		FolderID: "folder-1",
+		Status:   entity.SpaceStatusDeleting,
+	}
+	if err := db.Create(space).Error; err != nil {
+		t.Fatalf("create space: %v", err)
+	}
+
+	code, err := NewSkillSpaceService(nil).DeleteSpace(context.Background(), "owner-1", space.ID, nil)
+	if err != nil || code != common.CodeSuccess {
+		t.Fatalf("DeleteSpace = (%v, %v), want success", code, err)
+	}
+
+	spaceDAO := dao.NewSkillSpaceDAO()
+	deadline := time.Now().Add(3 * time.Second)
+	for {
+		stored, reloadErr := spaceDAO.GetByIDAnyStatus(context.Background(), db, space.ID)
+		if reloadErr != nil {
+			t.Fatalf("reload space: %v", reloadErr)
+		}
+		if stored.Status == entity.SpaceStatusDeleted {
+			break
+		}
+		if time.Now().After(deadline) {
+			t.Fatalf("space status = %q, want %q: the repeat delete did not restart the cleanup", stored.Status, entity.SpaceStatusDeleted)
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+
+	fileDAO := dao.NewFileDAO()
+	for _, id := range []string{"folder-1", "file-1"} {
+		if _, err := fileDAO.GetByID(context.Background(), db, id); err == nil {
+			t.Fatalf("file %q survived the restarted cleanup", id)
+		}
+	}
+}