From 853e73f08c8de9c550fe556d82788a44e80717dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BF=AE=E9=9B=A8?= <47820304+PeterGuy326@users.noreply.github.com> Date: Thu, 10 Sep 2026 09:42:06 +0800 Subject: [PATCH 1/5] fix(folder): compile recursive blob cleanup --- CHANGELOG.md | 7 + docs/DEPLOYMENT.md | 11 ++ server/cmd/memd/main.go | 2 +- .../content_disposition_integration_test.go | 2 +- .../internal/api/relocate_integration_test.go | 2 +- .../file/workspace_lock_integration_test.go | 14 +- server/internal/folder/folder.go | 75 +++++++-- .../folder/folder_delete_integration_test.go | 154 ++++++++++++++++++ server/internal/folder/folder_test.go | 2 +- .../folder/workspace_lock_integration_test.go | 12 +- 10 files changed, 246 insertions(+), 35 deletions(-) create mode 100644 server/internal/folder/folder_delete_integration_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 104b02f..12723a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,13 @@ The project publishes 0.x prerelease versions; a stable release line is not yet to the canonical `bytefolk` organization while retaining the published npm scope, MCP identity, and existing cache paths. +### Fixed + +- Recursive folder delete now removes associated objects from bucket storage + after the database transaction commits. Previously only DB rows were deleted, + leaving orphan blobs in the bucket. Blob deletion is best-effort and logged + on failure, matching the existing single-file delete behavior. + ### Security - Normalize the client-declared MIME type of a stored file before deciding how diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 801e41d..ddfbf7f 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -225,6 +225,17 @@ Redis AOF protects normal restarts but is not in the portable backup. A restore therefore starts with an empty queue/replay window. Requeue or reindex any file whose processing did not reach a terminal state before the backup. +### Object retention + +Deleting a file or recursively deleting a folder removes the database rows and +then deletes the associated objects from the bucket. Object deletion is +best-effort and happens after the database transaction commits, so a process +crash between commit and blob removal may leave orphan objects in the bucket. +These orphans are unreferenced — no live database row points at them — and are +safe to leave in place. They are not automatically reclaimed; operators may +remove them manually if bucket accounting matters. This crash window also +applies to single-file deletion. + ### Restore drill Restore only into an empty installation. The script verifies every checksum diff --git a/server/cmd/memd/main.go b/server/cmd/memd/main.go index 37cb47d..3681230 100644 --- a/server/cmd/memd/main.go +++ b/server/cmd/memd/main.go @@ -117,7 +117,7 @@ func run() error { "providers", cfg.ManagedEmbeddingProviders, ) } - folderSvc := folder.New(database.Pool) + folderSvc := folder.New(database.Pool, store, logger) fileSvc := file.New(database.Pool, store, folderSvc) memorySvc := memory.New(database.Pool) durableContextSvc := durablecontext.New(database.Pool, memorySvc) diff --git a/server/internal/api/content_disposition_integration_test.go b/server/internal/api/content_disposition_integration_test.go index 29baedf..cb2e0a7 100644 --- a/server/internal/api/content_disposition_integration_test.go +++ b/server/internal/api/content_disposition_integration_test.go @@ -105,7 +105,7 @@ func TestContentDispositionHTTPPostgres(t *testing.T) { authService := auth.New(database.Pool) workspaceService := workspace.New(database.Pool) - folderService := folder.New(database.Pool) + folderService := folder.New(database.Pool, nil, nil) user, err := authService.CreateUser( ctx, "content-disposition-"+uuid.NewString()+"@example.test", diff --git a/server/internal/api/relocate_integration_test.go b/server/internal/api/relocate_integration_test.go index 855a449..b18c5e7 100644 --- a/server/internal/api/relocate_integration_test.go +++ b/server/internal/api/relocate_integration_test.go @@ -103,7 +103,7 @@ func TestRelocateHTTPPostgres(t *testing.T) { t.Fatalf("create restricted relocation token: %v", err) } - folderService := folder.New(database.Pool) + folderService := folder.New(database.Pool, nil, nil) fileService := file.New(database.Pool, nil, folderService) server := httptest.NewServer((&Server{ Auth: authService, diff --git a/server/internal/file/workspace_lock_integration_test.go b/server/internal/file/workspace_lock_integration_test.go index 49d0b9c..f47a02b 100644 --- a/server/internal/file/workspace_lock_integration_test.go +++ b/server/internal/file/workspace_lock_integration_test.go @@ -50,15 +50,15 @@ func TestFilePathLockingIntegration(t *testing.T) { t.Run("Put then Rename has one path", func(t *testing.T) { userID, _ := createFileLockTenant(t, ctx, database.Pool, "put-rename") - folders := folder.New(database.Pool) + folders := folder.New(database.Pool, nil, nil) if _, err := folders.Create(ctx, userID, "/A"); err != nil { t.Fatalf("create destination: %v", err) } putBackend := pglockwait.NewBackend(t, ctx, dsn, "file-put") renameBackend := pglockwait.NewBackend(t, ctx, dsn, "file-put-rename") store := newBlockingObjectStore() - service := New(putBackend.Pool, store, folder.New(putBackend.Pool)) - renameFolders := folder.New(renameBackend.Pool) + service := New(putBackend.Pool, store, folder.New(putBackend.Pool, nil, nil)) + renameFolders := folder.New(renameBackend.Pool, nil, nil) type putOutcome struct { result *PutResult @@ -130,7 +130,7 @@ func TestFilePathLockingIntegration(t *testing.T) { t.Run("Move then Rename has one path", func(t *testing.T) { userID, _ := createFileLockTenant(t, ctx, database.Pool, "move-rename") - folders := folder.New(database.Pool) + folders := folder.New(database.Pool, nil, nil) source, err := folders.Create(ctx, userID, "/Source") if err != nil { t.Fatalf("create source: %v", err) @@ -161,9 +161,9 @@ func TestFilePathLockingIntegration(t *testing.T) { service := New( moveBackend.Pool, &recordingObjectStore{}, - folder.New(moveBackend.Pool), + folder.New(moveBackend.Pool, nil, nil), ) - renameFolders := folder.New(renameBackend.Pool) + renameFolders := folder.New(renameBackend.Pool, nil, nil) fileGate, fileGatePID := lockFilesTable(t, ctx, database.Pool) moveDone := make(chan error, 1) go func() { @@ -206,7 +206,7 @@ func TestFilePathLockingIntegration(t *testing.T) { t.Run("Relocate validates and commits path with name", func(t *testing.T) { userID, _ := createFileLockTenant(t, ctx, database.Pool, "atomic-relocate") - folders := folder.New(database.Pool) + folders := folder.New(database.Pool, nil, nil) source, err := folders.Create(ctx, userID, "/RelocateSource") if err != nil { t.Fatalf("create relocate source: %v", err) diff --git a/server/internal/folder/folder.go b/server/internal/folder/folder.go index 0dd0469..88e805c 100644 --- a/server/internal/folder/folder.go +++ b/server/internal/folder/folder.go @@ -13,6 +13,7 @@ import ( "context" "errors" "fmt" + "log/slog" "strings" "time" @@ -58,13 +59,29 @@ type Node struct { Children []*Node `json:"children,omitempty"` } +// blobDeleter is the narrow contract for best-effort object removal. The folder +// package depends on this interface rather than the concrete storage.Store so +// that tests and callers that do not need blob cleanup can pass nil. +type blobDeleter interface { + Delete(ctx context.Context, key string) error +} + // Service is the folder service. type Service struct { - pool *pgxpool.Pool + pool *pgxpool.Pool + store blobDeleter + log *slog.Logger } -// New constructs a folder Service. -func New(pool *pgxpool.Pool) *Service { return &Service{pool: pool} } +// New constructs a folder Service. A nil store skips object-storage cleanup on +// recursive delete (useful for tests that have no bucket). A nil logger falls +// back to slog.Default. +func New(pool *pgxpool.Pool, store blobDeleter, log *slog.Logger) *Service { + if log == nil { + log = slog.Default() + } + return &Service{pool: pool, store: store, log: log} +} // Sentinel errors. var ( @@ -586,9 +603,10 @@ func rewritePrefixTx(ctx context.Context, tx pgx.Tx, userID, srcID uuid.UUID, ol // // - recursive=false (default): folder must be empty (no subfolders, no files) // or ErrNotEmpty is returned. -// - recursive=true: subfolders + files are deleted from the DB. S3 cleanup -// is TODO — for now we only purge the DB rows; orphan blobs will be -// reaped by a future garbage-collection pass. +// - recursive=true: subfolders + files are deleted from the DB and their +// objects are removed from bucket storage. Blob deletion is best-effort +// and happens after the database transaction commits, so a crash between +// commit and blob removal may leave orphan objects (see docs/DEPLOYMENT.md). func (s *Service) Delete(ctx context.Context, userID uuid.UUID, path string, recursive bool) error { norm, err := pathx.Normalize(path) if err != nil { @@ -597,7 +615,8 @@ func (s *Service) Delete(ctx context.Context, userID uuid.UUID, path string, rec if norm == pathx.Root { return ErrRootOp } - return s.withPathMutationTx(ctx, userID, func(tx pgx.Tx) error { + var orphanKeys []string + txErr := s.withPathMutationTx(ctx, userID, func(tx pgx.Tx) error { src, err := selectFolderByPathTx(ctx, tx, userID, norm) if err != nil { return err @@ -623,24 +642,33 @@ func (s *Service) Delete(ctx context.Context, userID uuid.UUID, path string, rec return fmt.Errorf("check recursive delete memories: %w", err) } if containsMemories { - // A folder operation must never become an implicit memory - // deletion. The caller has to use the memory lifecycle's - // explicit forget operation first. return ErrContainsMemories } - // Hard delete: remove all descendant files first (FKs cascade - // from folders → files would only NULL out folder_id, so we have - // to delete files explicitly). - if _, err := tx.Exec(ctx, + rows, err := tx.Query(ctx, `DELETE FROM files WHERE user_id = $1 AND (folder_id = $2 OR path = $3 - OR left(path, length($3) + 1) = $3 || '/')`, - userID, src.ID, src.Path); err != nil { + OR left(path, length($3) + 1) = $3 || '/') + RETURNING storage_key`, + userID, src.ID, src.Path) + if err != nil { return fmt.Errorf("recursive delete files: %w", err) } - // Subfolder rows cascade via the FK ON DELETE CASCADE when we - // drop the parent below. + for rows.Next() { + var key string + if err := rows.Scan(&key); err != nil { + rows.Close() + return fmt.Errorf("scan storage_key: %w", err) + } + if key != "" { + orphanKeys = append(orphanKeys, key) + } + } + if err := rows.Err(); err != nil { + rows.Close() + return fmt.Errorf("iterate deleted storage_keys: %w", err) + } + rows.Close() } if _, err := tx.Exec(ctx, `DELETE FROM folders WHERE id = $1 AND user_id = $2`, src.ID, userID); err != nil { @@ -648,6 +676,17 @@ func (s *Service) Delete(ctx context.Context, userID uuid.UUID, path string, rec } return nil }) + if txErr != nil { + return txErr + } + if s.store != nil { + for _, key := range orphanKeys { + if derr := s.store.Delete(ctx, key); derr != nil { + s.log.Warn("folder.blob_delete_failed", "storage_key", key, "err", derr) + } + } + } + return nil } func isEmptyTx(ctx context.Context, tx pgx.Tx, userID, folderID uuid.UUID, path string) (bool, error) { diff --git a/server/internal/folder/folder_delete_integration_test.go b/server/internal/folder/folder_delete_integration_test.go new file mode 100644 index 0000000..9ef2b7f --- /dev/null +++ b/server/internal/folder/folder_delete_integration_test.go @@ -0,0 +1,154 @@ +package folder + +import ( + "bytes" + "context" + "fmt" + "os" + "strings" + "testing" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/PeterGuy326/mem/server/internal/storage" +) + +// TestRecursiveDeleteCleansBlobs verifies that a recursive folder delete +// removes objects from bucket storage, not just DB rows. It requires a live +// PostgreSQL instance (MEM_TEST_DB) and a MinIO endpoint (MEM_TEST_S3_ENDPOINT). +func TestRecursiveDeleteCleansBlobs(t *testing.T) { + dsn := os.Getenv("MEM_TEST_DB") + if dsn == "" { + t.Skip("MEM_TEST_DB not set; skipping") + } + s3Endpoint := os.Getenv("MEM_TEST_S3_ENDPOINT") + if s3Endpoint == "" { + t.Skip("MEM_TEST_S3_ENDPOINT not set; skipping") + } + + s3AccessKey := os.Getenv("MEM_TEST_S3_ACCESS_KEY") + if s3AccessKey == "" { + s3AccessKey = "mem" + } + s3SecretKey := os.Getenv("MEM_TEST_S3_SECRET_KEY") + if s3SecretKey == "" { + s3SecretKey = "mem-minio-password" + } + s3Bucket := os.Getenv("MEM_TEST_S3_BUCKET") + if s3Bucket == "" { + s3Bucket = "mem" + } + + ctx := context.Background() + + cfg, err := pgxpool.ParseConfig(dsn) + if err != nil { + t.Fatalf("parse MEM_TEST_DB: %v", err) + } + if !strings.HasSuffix(cfg.ConnConfig.Database, "_test") { + t.Fatalf("refusing non-test database %q", cfg.ConnConfig.Database) + } + pool, err := pgxpool.NewWithConfig(ctx, cfg) + if err != nil { + t.Fatalf("pgxpool: %v", err) + } + defer pool.Close() + + store, err := storage.New(ctx, s3Endpoint, s3AccessKey, s3SecretKey, s3Bucket, "", false) + if err != nil { + t.Fatalf("storage.New: %v", err) + } + + userID := folderTestUUID(t, ctx, pool, + `INSERT INTO users (email, password_hash) VALUES ($1, 'x') RETURNING id`, + "folder-blob-cleanup-"+uuid.NewString()+"@example.com") + defer func() { + _, _ = pool.Exec(ctx, `DELETE FROM users WHERE id = $1`, userID) + }() + + svc := New(pool, store, nil) + + if _, err := svc.Create(ctx, userID, "/CleanupParent/Child"); err != nil { + t.Fatalf("create folders: %v", err) + } + + childFolder, err := svc.Get(ctx, userID, "/CleanupParent/Child") + if err != nil { + t.Fatalf("get child folder: %v", err) + } + + type testFile struct { + id uuid.UUID + name string + path string + folderID uuid.UUID + storageKey string + } + files := []testFile{ + { + id: uuid.New(), + name: "a.txt", + path: "/CleanupParent", + folderID: mustFolderID(t, ctx, pool, userID, "/CleanupParent"), + storageKey: fmt.Sprintf("users/%s/%s/a.txt", userID, uuid.New()), + }, + { + id: uuid.New(), + name: "b.txt", + path: "/CleanupParent/Child", + folderID: childFolder.ID, + storageKey: fmt.Sprintf("users/%s/%s/b.txt", userID, uuid.New()), + }, + } + + for _, f := range files { + if err := store.Put(ctx, f.storageKey, bytes.NewReader([]byte("content-"+f.name)), int64(len("content-"+f.name)), "text/plain"); err != nil { + t.Fatalf("put object %s: %v", f.storageKey, err) + } + if _, err := pool.Exec(ctx, + `INSERT INTO files (id, user_id, name, path, folder_id, size, sha256, mime, storage_key, tags, index_status) + VALUES ($1, $2, $3, $4, $5, $6, $7, 'text/plain', $8, '{}', 'pending')`, + f.id, userID, f.name, f.path, f.folderID, len("content-"+f.name), + "sha256-"+f.id.String(), f.storageKey, + ); err != nil { + t.Fatalf("insert file row %s: %v", f.name, err) + } + } + + for _, f := range files { + if _, err := store.Get(ctx, f.storageKey); err != nil { + t.Fatalf("precondition: object %s should exist before delete: %v", f.storageKey, err) + } + } + + if err := svc.Delete(ctx, userID, "/CleanupParent", true); err != nil { + t.Fatalf("recursive delete: %v", err) + } + + var remaining int + if err := pool.QueryRow(ctx, + `SELECT COUNT(*) FROM files WHERE user_id = $1 AND path LIKE '/CleanupParent%'`, + userID).Scan(&remaining); err != nil { + t.Fatalf("count remaining files: %v", err) + } + if remaining != 0 { + t.Fatalf("expected 0 file rows after recursive delete, got %d", remaining) + } + + for _, f := range files { + if _, err := store.Get(ctx, f.storageKey); err == nil { + t.Errorf("object %s still exists in bucket after recursive delete", f.storageKey) + } + } +} + +func mustFolderID(t *testing.T, ctx context.Context, pool *pgxpool.Pool, userID uuid.UUID, path string) uuid.UUID { + t.Helper() + var id uuid.UUID + if err := pool.QueryRow(ctx, + `SELECT id FROM folders WHERE user_id = $1 AND path = $2`, userID, path).Scan(&id); err != nil { + t.Fatalf("lookup folder %q: %v", path, err) + } + return id +} diff --git a/server/internal/folder/folder_test.go b/server/internal/folder/folder_test.go index dcb9fc9..21fd464 100644 --- a/server/internal/folder/folder_test.go +++ b/server/internal/folder/folder_test.go @@ -228,7 +228,7 @@ func TestMemoryPathLifecycleIntegration(t *testing.T) { VALUES ('folder memory test', $1) RETURNING id`, userID) - svc := New(pool) + svc := New(pool, nil, nil) for _, p := range []string{ "/A%_/Child", "/AtomicSource/original/Child", diff --git a/server/internal/folder/workspace_lock_integration_test.go b/server/internal/folder/workspace_lock_integration_test.go index 53d3eee..caf433d 100644 --- a/server/internal/folder/workspace_lock_integration_test.go +++ b/server/internal/folder/workspace_lock_integration_test.go @@ -53,12 +53,12 @@ func TestWorkspacePathLockingIntegration(t *testing.T) { t.Run("content lock blocks folder rewrite", func(t *testing.T) { userID, _ := createWorkspaceLockTenant(t, ctx, database.Pool, "content-blocks-folder") - service := New(database.Pool) + service := New(database.Pool, nil, nil) if _, err := service.Create(ctx, userID, "/Shared/Child"); err != nil { t.Fatalf("create source: %v", err) } renameBackend := pglockwait.NewBackend(t, ctx, dsn, "folder-rewrite") - renameService := New(renameBackend.Pool) + renameService := New(renameBackend.Pool, nil, nil) writerTx, err := database.Pool.BeginTx(ctx, pgx.TxOptions{}) if err != nil { @@ -105,7 +105,7 @@ func TestWorkspacePathLockingIntegration(t *testing.T) { createBackend := pglockwait.NewBackend(t, ctx, dsn, "folder-create") memoryBackend := pglockwait.NewBackend(t, ctx, dsn, "memory-remember") checkpointBackend := pglockwait.NewBackend(t, ctx, dsn, "handoff-checkpoint") - folderService := New(createBackend.Pool) + folderService := New(createBackend.Pool, nil, nil) memoryService := memory.New(memoryBackend.Pool) handoffService := handoff.New(checkpointBackend.Pool) @@ -177,7 +177,7 @@ func TestWorkspacePathLockingIntegration(t *testing.T) { t.Run("concurrent renames cannot split a subtree", func(t *testing.T) { userID, _ := createWorkspaceLockTenant(t, ctx, database.Pool, "rename-race") - service := New(database.Pool) + service := New(database.Pool, nil, nil) child, err := service.Create(ctx, userID, "/Race/Child") if err != nil { t.Fatalf("create source subtree: %v", err) @@ -207,8 +207,8 @@ func TestWorkspacePathLockingIntegration(t *testing.T) { service *Service } attempts := []renameAttempt{ - {name: "RaceB", backend: renameBBackend, service: New(renameBBackend.Pool)}, - {name: "RaceC", backend: renameCBackend, service: New(renameCBackend.Pool)}, + {name: "RaceB", backend: renameBBackend, service: New(renameBBackend.Pool, nil, nil)}, + {name: "RaceC", backend: renameCBackend, service: New(renameCBackend.Pool, nil, nil)}, } gateTx, err := database.Pool.BeginTx(ctx, pgx.TxOptions{}) From d21c23cef6fb832171c511c7b62ad7274eb39c07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BF=AE=E9=9B=A8?= <47820304+PeterGuy326@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:12:01 +0800 Subject: [PATCH 2/5] chore(web): carry audited lockfile baseline from #192 Copy the reviewed baseline lockfile from 11e02e21ef2c3dbd2dae26e4376872e54e78ecb5 without changing application code or dependency ranges. Refs #192. --- web/package-lock.json | 100 +++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/web/package-lock.json b/web/package-lock.json index 103ae42..28a95f0 100644 --- a/web/package-lock.json +++ b/web/package-lock.json @@ -2161,16 +2161,16 @@ } }, "node_modules/@vitest/expect": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/expect/-/expect-4.1.10.tgz", - "integrity": "sha512-YsCn+qAk1GWjQOWFEsEcL2gNQ0zmVmQu3T03qP6UyjhtmdtwtbuI+DASn/7iQB3HGTXkdBwGddzxPlmiql5vlA==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/expect/-/expect-4.1.11.tgz", + "integrity": "sha512-VX2x5vNJXET47KAFzwERI+KRMtTTCSWTfSMKsW7JsUsXV4psq++e3DvZpuTDOpHcxytiDs6p2nhVb2tVDiiUYw==", "dev": true, "license": "MIT", "dependencies": { "@standard-schema/spec": "^1.1.0", "@types/chai": "^5.2.2", - "@vitest/spy": "4.1.10", - "@vitest/utils": "4.1.10", + "@vitest/spy": "4.1.11", + "@vitest/utils": "4.1.11", "chai": "^6.2.2", "tinyrainbow": "^3.1.0" }, @@ -2179,13 +2179,13 @@ } }, "node_modules/@vitest/mocker": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/mocker/-/mocker-4.1.10.tgz", - "integrity": "sha512-v0xaezt+DKEmKfaxg133ldzADrwLGd7Ze1MfQQTYfvs8OqZIwbxyxaYURivwV7sWy5fqn3rH5uOrSp07bp44Ow==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/mocker/-/mocker-4.1.11.tgz", + "integrity": "sha512-2XJVD55d1o5AZous5CCGKS74g/riOj9odEt2bQpCVZeblHyHdnMeFl4jl0XjU21stf4mbjUkew2eXQZt65g5CQ==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/spy": "4.1.10", + "@vitest/spy": "4.1.11", "estree-walker": "^3.0.3", "magic-string": "^0.30.21" }, @@ -2206,9 +2206,9 @@ } }, "node_modules/@vitest/pretty-format": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/pretty-format/-/pretty-format-4.1.10.tgz", - "integrity": "sha512-W1HsjSH4MXQ9YfmmhLAoIYf1HRfekQCGngeIgcei6MP5QQGWUe0gkopdZQaVCFO+JDJMrAJGwa5pRpNpvy4P8Q==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/pretty-format/-/pretty-format-4.1.11.tgz", + "integrity": "sha512-yiZzPbGTS9Sr/JpFl8zHrcIkAofNbFV6k21vIgQN/cY/oxZeXhJv5sc/MBJ5jFKWmWs+oJHw0UXLZjmf931+Vw==", "dev": true, "license": "MIT", "dependencies": { @@ -2219,13 +2219,13 @@ } }, "node_modules/@vitest/runner": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/runner/-/runner-4.1.10.tgz", - "integrity": "sha512-IKI6kpIH+LmpROplyLwBBaCfMgOZOMsygVa6BARD6ahA04VRuJSa6OaVG7kRvSEMD870Vd91rSSw0eegtWyLGg==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/runner/-/runner-4.1.11.tgz", + "integrity": "sha512-LztvUgdwMNJMIkj3hQnnxiC2Xy1zNxq928W/xhjCLaNCzqTZOudjwbQf6v9IntZGPw132i2Lq2rgTRZHD3JHNw==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/utils": "4.1.10", + "@vitest/utils": "4.1.11", "pathe": "^2.0.3" }, "funding": { @@ -2233,14 +2233,14 @@ } }, "node_modules/@vitest/snapshot": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/snapshot/-/snapshot-4.1.10.tgz", - "integrity": "sha512-xRkfOT1qpTAi/Ti4Y1LtfRc3kEuqxGw59eN2jN9pRWMtS/XDevekhcFSqvQqjUNGksfjMJu3Y+oJ+4Ypn2OaJw==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/snapshot/-/snapshot-4.1.11.tgz", + "integrity": "sha512-pN7ikn1ON7h8ee4gIAp4AzyK+zBtJPzVbqOgu5LCEh4VaJVbPQcgYQYJIMGQPXVeJJq1fnfazis7a5pFNPahog==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/pretty-format": "4.1.10", - "@vitest/utils": "4.1.10", + "@vitest/pretty-format": "4.1.11", + "@vitest/utils": "4.1.11", "magic-string": "^0.30.21", "pathe": "^2.0.3" }, @@ -2249,9 +2249,9 @@ } }, "node_modules/@vitest/spy": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/spy/-/spy-4.1.10.tgz", - "integrity": "sha512-PLf/Ugvoq5wO/b4rwYCR1h2PSIdXz7wnkQFMiUpLdtM7l6pqVFcQIBEHyT1+l+cj7mNwAfZHzqXqDyjvOuwbDw==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/spy/-/spy-4.1.11.tgz", + "integrity": "sha512-apNa/prQy2qCeywhnixOHPRCgGNhvg7T4Dapfl1GahLp/R+uhBm5cPyFoNVyqsNd2h1nJxL6BqqdIjiABL60YA==", "dev": true, "license": "MIT", "funding": { @@ -2259,13 +2259,13 @@ } }, "node_modules/@vitest/utils": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/@vitest/utils/-/utils-4.1.10.tgz", - "integrity": "sha512-fy9am/HWxbaGt/Sawrp90vt6Y6jQwf1RX77cz3uwoJwJVMli/e1IEwRPnMNJ7vKfPTwo0diXifkpPvwH9v7nGA==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/@vitest/utils/-/utils-4.1.11.tgz", + "integrity": "sha512-zTCVGpyFsGWBhllOyKlTw/vnr6D9qxsfSDyfbyZmTyjHw5N/VuvzHpHoQjm2ZJzn4RJgx5w4r7V0er69CmLgPQ==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/pretty-format": "4.1.10", + "@vitest/pretty-format": "4.1.11", "convert-source-map": "^2.0.0", "tinyrainbow": "^3.1.0" }, @@ -3784,9 +3784,9 @@ "peer": true }, "node_modules/js-yaml": { - "version": "4.3.1", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.1.tgz", - "integrity": "sha512-CY6crGq313MX8GkwvB7tzgp99vjQxY1++5y10/BKN/GUfHqWaOGQMNZkBvqSzsZKWk/ijwHlWzzkLulsGHhjWQ==", + "version": "4.3.2", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.2.tgz", + "integrity": "sha512-SFNOvSJ+Dgf/9An904Yx+CgSlIPCkIpao4qo51lpee25TIRejdH3rhR4EZMGoNx3/TP3O+wzWuiTFl4sqbltzA==", "dev": true, "funding": [ { @@ -4847,9 +4847,9 @@ } }, "node_modules/postcss-selector-parser": { - "version": "6.1.2", - "resolved": "https://registry.npmjs.org/postcss-selector-parser/-/postcss-selector-parser-6.1.2.tgz", - "integrity": "sha512-Q8qQfPiZ+THO/3ZrOrO0cJJKfpYCagtMUkXbnEfmgUjwXg6z/WBeOyS9APBBPCTSiDV+s4SwQGu8yFsiMRIudg==", + "version": "6.1.4", + "resolved": "https://registry.npmjs.org/postcss-selector-parser/-/postcss-selector-parser-6.1.4.tgz", + "integrity": "sha512-bIoJLOmjCO1S9XdY/DcnR5hJxvrDir1PbGChrzXG3vw0/FOliy/fA3dmdhQ441kah4gKv+TwckGzex6wNS5cnQ==", "dev": true, "license": "MIT", "dependencies": { @@ -6009,19 +6009,19 @@ } }, "node_modules/vitest": { - "version": "4.1.10", - "resolved": "https://registry.npmjs.org/vitest/-/vitest-4.1.10.tgz", - "integrity": "sha512-R9jUTe5S4Qb0HCd4TNqpC7oGcrMssMRGXLW80ubjWsW9VH5GF8y1Y0SFLY9AbqSk6nt0PnOx4H4WNJYZ13GUPw==", + "version": "4.1.11", + "resolved": "https://registry.npmjs.org/vitest/-/vitest-4.1.11.tgz", + "integrity": "sha512-fhACrNXUidIbGSBr5FlbuBkO7VWC1ZyLl0DO4CU2DrQoAPxX84Ysxs+HeGQpii5lZWV1Q4gBZTTu49mF+A6Edw==", "dev": true, "license": "MIT", "dependencies": { - "@vitest/expect": "4.1.10", - "@vitest/mocker": "4.1.10", - "@vitest/pretty-format": "4.1.10", - "@vitest/runner": "4.1.10", - "@vitest/snapshot": "4.1.10", - "@vitest/spy": "4.1.10", - "@vitest/utils": "4.1.10", + "@vitest/expect": "4.1.11", + "@vitest/mocker": "4.1.11", + "@vitest/pretty-format": "4.1.11", + "@vitest/runner": "4.1.11", + "@vitest/snapshot": "4.1.11", + "@vitest/spy": "4.1.11", + "@vitest/utils": "4.1.11", "es-module-lexer": "^2.0.0", "expect-type": "^1.3.0", "magic-string": "^0.30.21", @@ -6049,12 +6049,12 @@ "@edge-runtime/vm": "*", "@opentelemetry/api": "^1.9.0", "@types/node": "^20.0.0 || ^22.0.0 || >=24.0.0", - "@vitest/browser-playwright": "4.1.10", - "@vitest/browser-preview": "4.1.10", - "@vitest/browser-webdriverio": "4.1.10", - "@vitest/coverage-istanbul": "4.1.10", - "@vitest/coverage-v8": "4.1.10", - "@vitest/ui": "4.1.10", + "@vitest/browser-playwright": "4.1.11", + "@vitest/browser-preview": "4.1.11", + "@vitest/browser-webdriverio": "4.1.11", + "@vitest/coverage-istanbul": "4.1.11", + "@vitest/coverage-v8": "4.1.11", + "@vitest/ui": "4.1.11", "happy-dom": "*", "jsdom": "*", "vite": "^6.0.0 || ^7.0.0 || ^8.0.0" From 7ed5a6b09889a4e7a44657d7d9148901a05c3769 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BF=AE=E9=9B=A8?= <47820304+PeterGuy326@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:38:18 +0800 Subject: [PATCH 3/5] test(folder): require live blob cleanup acceptance --- docs/TESTING.md | 6 ++++++ scripts/acceptance_agent_memory.sh | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/docs/TESTING.md b/docs/TESTING.md index a3204af..0aaa0cb 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -312,6 +312,12 @@ real `tools/call → HTTP → PostgreSQL` remember, checkpoint inspection, feedback, archive, restore and logical forget. Archive must remove the memory from recall, restore must make it recallable again, and forget must redact the live PostgreSQL projection and event actors before the database is destroyed. +The same required lifecycle gate runs `TestRecursiveDeleteCleansBlobs` against +its migrated PostgreSQL database and live MinIO endpoint. It requires the +named test's structured PASS event; a missing test, skipped service dependency, +or failed object cleanup fails the gate. The test uses only the disposable +stack's data, and the existing cleanup trap removes that stack. + The script never reads or writes the user's CLI configuration. Set `MEM_ACCEPTANCE_HTTP_PORT` only when an explicit fixed port is required. diff --git a/scripts/acceptance_agent_memory.sh b/scripts/acceptance_agent_memory.sh index f913556..afaba72 100755 --- a/scripts/acceptance_agent_memory.sh +++ b/scripts/acceptance_agent_memory.sh @@ -891,4 +891,19 @@ printf '%s' "$redaction_json" | .marker_absent_from_events == true ' >/dev/null +log "Verifying recursive folder deletion against real PostgreSQL and MinIO" +( + cd "${REPO_ROOT}/server" + MEM_TEST_DB="$DB_URL" \ + MEM_TEST_S3_ENDPOINT="127.0.0.1:${S3_PORT}" \ + MEM_TEST_S3_ACCESS_KEY="mem" \ + MEM_TEST_S3_SECRET_KEY="mem-minio-password" \ + MEM_TEST_S3_BUCKET="mem" \ + go test -json -count=1 -timeout 2m \ + -run '^TestRecursiveDeleteCleansBlobs$' ./internal/folder +) | tee "${E2E_DIR}/folder-blob-cleanup.json" +jq -s -e 'any(.[]; .Action == "pass" and .Test == "TestRecursiveDeleteCleansBlobs")' \ + "${E2E_DIR}/folder-blob-cleanup.json" >/dev/null \ + || die "TestRecursiveDeleteCleansBlobs did not execute and pass" + log "PASS: isolated HTTP, CLI and MCP Agent-memory lifecycle acceptance" From e76d4b03a4d3681db868bcd2587e54a56cbceda1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BF=AE=E9=9B=A8?= <47820304+PeterGuy326@users.noreply.github.com> Date: Thu, 10 Sep 2026 12:41:23 +0800 Subject: [PATCH 4/5] test(folder): exercise real blob cleanup with workspace fixture --- .../folder/folder_delete_integration_test.go | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/server/internal/folder/folder_delete_integration_test.go b/server/internal/folder/folder_delete_integration_test.go index 9ef2b7f..12386c8 100644 --- a/server/internal/folder/folder_delete_integration_test.go +++ b/server/internal/folder/folder_delete_integration_test.go @@ -3,13 +3,16 @@ package folder import ( "bytes" "context" + "errors" "fmt" + "io" "os" "strings" "testing" "github.com/google/uuid" "github.com/jackc/pgx/v5/pgxpool" + "github.com/minio/minio-go/v7" "github.com/PeterGuy326/mem/server/internal/storage" ) @@ -66,6 +69,9 @@ func TestRecursiveDeleteCleansBlobs(t *testing.T) { defer func() { _, _ = pool.Exec(ctx, `DELETE FROM users WHERE id = $1`, userID) }() + folderTestUUID(t, ctx, pool, + `INSERT INTO workspaces (name, resource_owner_user_id) + VALUES ('folder blob cleanup test', $1) RETURNING id`, userID) svc := New(pool, store, nil) @@ -117,9 +123,15 @@ func TestRecursiveDeleteCleansBlobs(t *testing.T) { } for _, f := range files { - if _, err := store.Get(ctx, f.storageKey); err != nil { + reader, err := store.Get(ctx, f.storageKey) + if err != nil { t.Fatalf("precondition: object %s should exist before delete: %v", f.storageKey, err) } + content, readErr := io.ReadAll(reader) + closeErr := reader.Close() + if readErr != nil || closeErr != nil || string(content) != "content-"+f.name { + t.Fatalf("precondition: object %s content=%q read=%v close=%v", f.storageKey, content, readErr, closeErr) + } } if err := svc.Delete(ctx, userID, "/CleanupParent", true); err != nil { @@ -137,8 +149,15 @@ func TestRecursiveDeleteCleansBlobs(t *testing.T) { } for _, f := range files { - if _, err := store.Get(ctx, f.storageKey); err == nil { + reader, err := store.Get(ctx, f.storageKey) + if err == nil { + _ = reader.Close() t.Errorf("object %s still exists in bucket after recursive delete", f.storageKey) + continue + } + var response minio.ErrorResponse + if !errors.As(err, &response) || response.Code != "NoSuchKey" { + t.Errorf("object %s: expected S3 NoSuchKey after recursive delete, got %v", f.storageKey, err) } } } From a9f7238e8fcfbb031c84912a2923a6e6af0a6578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BF=AE=E9=9B=A8?= <47820304+PeterGuy326@users.noreply.github.com> Date: Thu, 10 Sep 2026 15:07:55 +0800 Subject: [PATCH 5/5] fix(folder): preserve deletion rationale and changelog structure --- CHANGELOG.md | 11 ++++------- scripts/test_release_guards.sh | 22 ++++++++++++++++++++++ server/internal/folder/folder.go | 8 ++++++++ 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12723a4..91ee969 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,13 +13,6 @@ The project publishes 0.x prerelease versions; a stable release line is not yet to the canonical `bytefolk` organization while retaining the published npm scope, MCP identity, and existing cache paths. -### Fixed - -- Recursive folder delete now removes associated objects from bucket storage - after the database transaction commits. Previously only DB rows were deleted, - leaving orphan blobs in the bucket. Blob deletion is best-effort and logged - on failure, matching the existing single-file delete behavior. - ### Security - Normalize the client-declared MIME type of a stored file before deciding how @@ -58,6 +51,10 @@ The project publishes 0.x prerelease versions; a stable release line is not yet path now proves a lock can be inspected before treating those Windows errors as contention, preserves prompt failure for unrelated permission errors, and observes its deadline when a competing lock disappears during inspection. +- Recursive folder delete now removes associated objects from bucket storage + after the database transaction commits. Previously only DB rows were deleted, + leaving orphan blobs in the bucket. Blob deletion is best-effort and logged + on failure, matching the existing single-file delete behavior. ## [0.1.1] - 2026-08-31 diff --git a/scripts/test_release_guards.sh b/scripts/test_release_guards.sh index 4366a82..9c166c3 100755 --- a/scripts/test_release_guards.sh +++ b/scripts/test_release_guards.sh @@ -22,6 +22,28 @@ expect_failure() { fi } +check_changelog_subsections() { + awk ' + /^## \[/ { + for (heading in seen) delete seen[heading] + } + /^### / && seen[$0]++ { + printf "ERROR: duplicate changelog subsection at line %d: %s\n", NR, $0 > "/dev/stderr" + exit 1 + } + ' "$1" +} + +# A category may recur in another release, but not twice in the same release. +changelog_fixture="${tmp_dir}/changelog.md" +printf '%s\n' '## [Unreleased]' '### Fixed' '- Current fix.' \ + '## [0.1.1] - 2026-08-31' '### Fixed' '- Earlier fix.' >"${changelog_fixture}" +check_changelog_subsections "${changelog_fixture}" +printf '%s\n' '## [Unreleased]' '### Fixed' '- First fix.' \ + '### Security' '- Security fix.' '### Fixed' '- Second fix.' >"${changelog_fixture}" +expect_failure "duplicate Unreleased subsection" check_changelog_subsections "${changelog_fixture}" +check_changelog_subsections "${repo_root}/CHANGELOG.md" + verify_manifest() { local directory="$1" ( diff --git a/server/internal/folder/folder.go b/server/internal/folder/folder.go index 88e805c..60f736a 100644 --- a/server/internal/folder/folder.go +++ b/server/internal/folder/folder.go @@ -642,8 +642,14 @@ func (s *Service) Delete(ctx context.Context, userID uuid.UUID, path string, rec return fmt.Errorf("check recursive delete memories: %w", err) } if containsMemories { + // A folder operation must never become an implicit memory + // deletion. The caller has to use the memory lifecycle's + // explicit forget operation first. return ErrContainsMemories } + // Delete descendant files explicitly: the folder FK would only + // NULL out folder_id. RETURNING retains their storage keys for + // best-effort object removal after this transaction commits. rows, err := tx.Query(ctx, `DELETE FROM files WHERE user_id = $1 @@ -669,6 +675,8 @@ func (s *Service) Delete(ctx context.Context, userID uuid.UUID, path string, rec return fmt.Errorf("iterate deleted storage_keys: %w", err) } rows.Close() + // Subfolder rows cascade via the FK ON DELETE CASCADE when we + // drop the parent below. } if _, err := tx.Exec(ctx, `DELETE FROM folders WHERE id = $1 AND user_id = $2`, src.ID, userID); err != nil {