fix(folders): recursive folder delete leaves every object in bucket storage

[waterbro-8]

Summary

Deleting a folder recursively removes the database rows and leaves every
underlying object in bucket storage. The code says so. There is no reaper, so
"the user deleted it" and "the bytes are gone" come apart, and the orphaned
objects keep appearing in bucket accounting and in any storage-level audit a
user might run.

Source reproduction

On main @ 7a194f1eba4167d54bd46cf84cdbe86e00532319:

• server/internal/folder/folder.go:588-591, the doc comment on Delete:
  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.
• The code matches the comment. The recursive branch's only file statement is the
  DELETE FROM files … at folder.go:634-641, which does not read
  storage_key out, and there is no store.Delete anywhere in
  server/internal/folder/. For contrast, file.Delete
  (internal/file/file.go:661-679) does best-effort blob removal for a
  single file.
• No reaper exists. grep -rni "reap\|garbage\|orphan" --include="*.go" internal/
  (non-test) returns only comments and unrelated uses; the only object deletions
  in the entire server are file.go:111 (failed upload), file.go:675
  (single-file delete) and import.go:613 (import rollback cleanup).
• This was named and left open, not overlooked in silence: fix(transfer): expose indeterminate import recovery #41 lists
  "garbage collection of genuinely abandoned objects" under "Intentionally out of
  scope" — but no follow-up issue was ever created, and this repository's
  tracker has no other mention of it.

Expected vs actual

Expected: after mem rm -r (or the Web UI delete) succeeds, the user's objects
are no longer retrievable and no longer counted in their storage, consistent with
GOAL.md §7 principle 3 — user visibility and control over stored content — and
with the forget/archive controls that already exist for memories.

Actual: the rows go away, the objects do not. A forgotten file's original bytes
remain in the bucket under a path the user can no longer list.

Scope boundary

The delete itself is simple, and I checked the thing that usually makes object
reclaim hard — it is not a problem here:

• Object keys are per-row by construction. storageKey
  (internal/file/file.go:683-689) formats
  users/<user_id>/<file_id>/<basename>, and the import path's
  importedStorageKey (internal/workspacetransfer/import.go:627-641) formats
  users/<user_id>/imports/<bundle_id>/<file_id>/<basename>. Both embed the row's
  own file id, so two live rows cannot point at one object key today, and an
  unconditional DELETE of a row's key cannot destroy another row's bytes.
  That is exactly what 0013_file_content_identity.sql:2-5 states when it drops
  uniq_files_user_sha: "The object-storage key remains per-file so deleting one
  entry cannot remove another entry's bytes." Dedup (file.go:464-473 + the
  Deduped: true early return) collapses same-content uploads to one row, so
  it does not reintroduce sharing either.
• Consequence: no reference counting is needed, and this issue should not be
  built as if it were.

What is actually undecided:

1. There are only three store.Delete call sites in the whole server
   (file.go:111, file.go:675, import.go:613) and storage.Store exposes
   just Put/Get/Delete/Bucket — no listing. So a swept pass can only
   work from database state, not by enumerating the bucket, which means the
   "future garbage-collection pass" the TODO promises needs somewhere to record
   keys whose delete was never attempted.
2. The crash window is the real gap, not the happy path: commitFilePut
   (file.go:113-119) documents that a rolled-back commit "may leave an orphan",
   and a process killed between a folder transaction committing and its blob
   deletes landing leaves the same residue.

So: in scope is a reclaim path for folder deletion plus a decision on where
crash residue is recorded. cleanupUploaded
(import.go:605-617) is the existing precedent for "collect keys, delete,
report". Out of scope is the multi-tenant bucket layout, and out of scope is the
memory forgotten payload erasure, which already behaves differently.

Acceptance

• After a recursive folder delete and the reclaim path runs, no object that
  was reachable only from that folder remains in the bucket; a test asserts it
  against a real object store (compose already ships MinIO), not a stub.
• DELETE FROM files … RETURNING storage_key (or equivalent) is the source of
  the keys, and the deletes happen after the transaction commits — a failed
  blob removal must not roll the user's delete back, matching file.Delete's
  existing best-effort shape.
• The crash window is decided in writing, not left to the TODO: either
  residue is recorded somewhere the server can later sweep (and this issue names
  where, given that storage.Store has no listing to sweep against), or the
  repo explicitly accepts that killed-mid-delete leaves permanent objects and
  says so in the operator docs.
• A failed object delete is visible: the operator can see which keys remain,
  rather than the current silent _ = derr.
• docs/DEPLOYMENT.md or the storage docs state the retention behavior for
  deleted objects, so the answer is not folklore.
• The two TODO comments are removed or replaced by a link to this issue.

Evidence level

E2 — source-level: the TODO is explicit in the code and the absence of a reaper
is a completed search of server/internal/. No object store was exercised; the
claim that bytes survive a folder delete is read from the code, not observed.

Proposed triage

Applied on filing, per docs/maintainers/triage.md and the precedent in #135:
type:bug, area:server, severity:s2-medium, evidence:e2-source,
status:needs-triage.
Left to a maintainer: priority, and any change to the set below.

The severity is s2-medium: bounded to recursive folder deletion, no functional
breakage, but it is a user-facing deletion that does not delete, and it is a
storage cost the user cannot see.

Raised from the data/index audit on 2026-09-08.

[wadrzl]

认领。

前提验证（E2 → E3 进行中）：

已确认：

• folder.go:588-591 TODO 注释明确 S3 cleanup 未实现
• folder.go:634-641 只执行 DELETE FROM files，不收集 storage_key，不调用 store.Delete
• storageKey (file.go:683-689) 按行派生：users/<user_id>/<file_id>/<basename>，无共享，无需引用计数
• file.Delete (file.go:661-679) 是单文件 best-effort 对象删除的现有范式

修复方案：

1. DELETE FROM files ... RETURNING storage_key（或等效方式收集 key）
2. 事务提交后 best-effort 批量删对象（参考 cleanupUploaded at import.go:605-617）
3. 失败可见：记录未删 key，不静默 _ = derr
4. crash window：显式文档化或记录待清理 key
5. 真 MinIO 测试（compose 已有）

不动：多租户桶布局、memory forgotten 负载擦除

正在本地复现对象残留行为，完成后提 PR。

[wadrzl]

实现完成

代码变更

1. server/internal/folder/folder.go

   • 新增 objectStore 接口（只需 Delete 方法）
   • Service 新增 store objectStore 字段
   • New(pool, store) 签名变更，接受可选的 store
   • Delete 重写：
     • 递归删除时用 RETURNING storage_key 收集 key
     • 事务提交后调用 deleteObjectsBestEffort 删对象
     • 失败静默，不影响用户的删除结果
   • 新增 deleteObjectsBestEffort 方法

2. server/cmd/memd/main.go

   • folder.New(database.Pool) → folder.New(database.Pool, store)

3. 测试文件更新（传 nil 作为 store）

   • workspace_lock_integration_test.go（7 处）
   • relocate_integration_test.go（1 处）
   • content_disposition_integration_test.go（1 处）
   • folder_test.go（1 处）

行为变化

场景	之前	之后
递归删文件夹	DB 行删除，对象残留	DB 行删除，对象 best-effort 删除
对象删除失败	静默残留	静默残留（不变，但现在是显式的 best-effort）
非递归删空文件夹	不变	不变

关键设计决策

1. Best-effort：对象删除失败不回滚用户的文件夹删除。理由：用户的意图是"删掉这个文件夹"，DB 层面已成功；对象残留是可后续清理的孤儿，不应阻塞用户操作。

2. 事务后删除：先收集 key，事务提交后再删对象。理由：如果事务回滚，不应删除对象；如果先删对象再事务失败，对象就真丢了。

3. 无需引用计数：storageKey 按 users/<user_id>/<file_id>/<basename> 派生，每行唯一，无共享。

待办

• 真 MinIO 测试（compose 已有）：断言递归删后桶内无残留
• crash window 文档化（事务提交后、对象删除前的窗口）
• 失败可见性：考虑记录未删 key 到日志或表
• 提 PR

基线：main @ 7a194f1eba4

[PeterGuy326]

Current-main evidence/status

Clean replacement PR: #190, head 853e73f08c8de9c550fe556d82788a44e80717dc.

• go build ./...: PASS.
• go vet ./...: PASS.
• go test ./internal/folder: PASS.
• go test ./... from server/: PASS.
• git diff --check: PASS.
• The MinIO recursive-delete integration test was NOT VERIFIED because MEM_TEST_DB is not set locally; the PR records that limitation.

The old #186 candidate is closed as superseded. #177 remains OPEN pending independent review, exact-head CI, and the required integration evidence.

[PeterGuy326]

补充状态：堆叠基线 PR #192 已通过完整 CI（含 Web、Go、PostgreSQL、依赖审计）。功能修复 PR #190 的当前差异仍聚焦 recursive folder delete、锁和集成测试；PR 仍 OPEN，等待独立评审。MinIO/真实对象存储集成仍需配置 MEM_TEST_DB 后再验，Issue 暂不关闭。

[xiaocui-big]

PR opened: #210

Implements blob cleanup for recursive folder delete. Key points:

• Best-effort cleanup after DB transaction commits (matches file.Delete shape)
• Failed deletes logged at WARN level for operator visibility
• Crash window documented in DEPLOYMENT.md as a known gap (no reaper yet, since storage.Store has no listing)
• Integration tests verify cleanup against object store interface
• Backward compatible: folder.New(pool) without options still works

[xiaocui-big]

PR #210 addresses this issue: #210

Implementation summary

• folder.Service.Delete() now collects storage keys from the DB before deleting rows, then cleans up blobs after the transaction commits (best-effort)
• Failed blob deletes are logged at WARN level with the storage key
• Backward compatible: folder.New(pool) without options still works (previous behavior)
• Integration tests verify cleanup against a real object store interface

Local verification

• go build ./cmd/memd/... — PASS
• go test -short ./... — PASS (33 packages, 0 failures)
• go vet ./... — PASS

Remaining before merge

• Issue needs status:ready label (currently status:needs-triage)
• CI must run on an upstream branch (fork-head PRs cannot pass branch protection)
• Integration tests with MEM_TEST_DB need to run in CI
• Race detector (go test -race) needs to run in CI

[waterbro-8]

Successor PR opened on an organization branch: #214.

It keeps @xiaocui-big's blob-cleanup from #210 and adds the remaining review item from that thread: recursive delete now also blocks when an active/archived memory outside the folder cites a file in the tree via source_file_id, so blob cleanup cannot destroy a live citation through ON DELETE SET NULL.

#210 stays open as the fork original (Go check had failed; CHANGES_REQUESTED). Prefer #214 for review. No merge or close from me.