[myles332]

Summary

Chat attachments (data_dir/chat-attachments/att-<uuid>__<name>.<ext>) were never deleted.

• Source of truth: image parts ({"type":"image","text":<file>}) in user messages' parts_json.
  • They are the only references: queued messages hold base64, retry forks reuse their anchor's still-present message, and restart-recovery prepared_input only names files whose user message is in the same session.
• On chat or project delete: Store::delete_chat_session and delete_local_project_with collect the names their messages show inside the delete transaction.
  • After commit, they remove the files that no remaining message shows.
  • This is best effort. A failure never blocks deletion, and removal is skipped while a data-dir move is claimed.
• Periodic sweep: cleanup_orphaned_chat_attachments runs in the existing reclaim_orphaned_run_artifacts loop, under the same gate, as its own spawn_blocking with its own log line.
  • It removes att-* regular files that no message shows and that are older than 24h. Uploads are written before their message row.
  • It stays within ORPHAN_PASS_BUDGET and skips per-file errors.
• ATTACHMENTS_DIR and is_attachment_name now live next to save_images.

Stacked on #753. Retarget to main after it merges.

Closes OR-574

Testing

• cargo fmt --all --check, cargo clippy --all-targets -- -D warnings, cargo build --locked, cargo test --locked
• New store tests:
  • Chat and project deletes remove only files no remaining message shows; shared files survive.
  • A non-att- name in an image part is never removed.
  • The sweep spares shown, recent and foreign files, and does nothing while a move is claimed.
• Manual: attach an image to a chat, delete the chat, and confirm the file is gone from chat-attachments/.
• Manual: an attachment older than a day with no message is swept on the next pass after orx up starts.

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 4/5

[Medium impact] The changes affect local chat and project cleanup within the app.

Fix the race between attachment cleanup and storage moves before merging.

Findings

1.  Cleanup can break storage moves ▶
2.  Later orphan files stay untouched ▶

Fix with agent prompt

### Issue 1
src/store.rs:2689-2691
`cleanup_orphaned_chat_attachments` checks the move claim but holds no shared lock while deleting files. If another dashboard starts a storage move after that check, cleanup can remove a source file while the move copies it. `copy_tree` then fails on the missing file, or the final size check fails.

Hold a lock that prevents a move from starting until cleanup finishes. Apply the same protection to `remove_unreferenced_chat_attachments`.

### Issue 2
src/store.rs:2698-2702
Every sweep starts `read_dir` from the beginning. If checking retained files in a large folder or on a slow disk consumes the five-second budget, the sweep stops before reaching later orphan files. Each retry checks the same prefix, so those orphans can remain forever while the loop retries every second.

Keep progress between passes or rotate the starting point, and add a test for budget exhaustion.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Adds attachment cleanup after chat and project deletion, plus a periodic sweep for unreferenced files older than 24 hours.

• Chat and project deletion remove attachments no remaining message uses.
• The background sweep removes old attachment files no message uses.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Delete chat or project] --> B[Collect attachment names]
  B --> C[Delete rows and commit]
  C --> D[Read remaining message references]
  D --> E[Remove unreferenced attachments]
  F[Periodic sweep] --> G[Find attachment files older than 24 hours]
  G --> H[Read message references]
  H --> I[Remove unreferenced files within pass budget]

Loading

_{Reviews (1) · Last reviewed commit: "Delete chat attachments with their chats..." · Reviewed by Greptile}

[greptile-apps]

Cleanup can break storage moves

cleanup_orphaned_chat_attachments checks the move claim but holds no shared lock while deleting files. If another dashboard starts a storage move after that check, cleanup can remove a source file while the move copies it. copy_tree then fails on the missing file, or the final size check fails.

Hold a lock that prevents a move from starting until cleanup finishes. Apply the same protection to remove_unreferenced_chat_attachments.

Knowledge Base Used: Workspace state and file synchronization

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/store.rs
Line: 2689-2691

Comment:
**Cleanup can break storage moves**

`cleanup_orphaned_chat_attachments` checks the move claim but holds no shared lock while deleting files. If another dashboard starts a storage move after that check, cleanup can remove a source file while the move copies it. `copy_tree` then fails on the missing file, or the final size check fails.

Hold a lock that prevents a move from starting until cleanup finishes. Apply the same protection to `remove_unreferenced_chat_attachments`.

**Knowledge Base Used:** [Workspace state and file synchronization](https://app.greptile.com/alphaxiv/-/custom-context/knowledge-base/alphaxiv/openresearch/-/docs/workspace-state-and-files.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[myles332]

Fixed in d1cc352: Store::hold_off_data_dir_move clears stale leases, then takes a shared lock on the data-dir move lock (a move takes it exclusively before claiming its lease and keeps it for the whole move) and re-checks the lease. Both the sweep and delete-time removal hold it while removing, so a move from another dashboard fails fast instead of racing the removals. Test: a move cannot acquire its lock while the hold is held.

[greptile-apps]

Later orphan files stay untouched

Every sweep starts read_dir from the beginning. If checking retained files in a large folder or on a slow disk consumes the five-second budget, the sweep stops before reaching later orphan files. Each retry checks the same prefix, so those orphans can remain forever while the loop retries every second.

Keep progress between passes or rotate the starting point, and add a test for budget exhaustion.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/store.rs
Line: 2698-2702

Comment:
**Later orphan files stay untouched**

Every sweep starts `read_dir` from the beginning. If checking retained files in a large folder or on a slow disk consumes the five-second budget, the sweep stops before reaching later orphan files. Each retry checks the same prefix, so those orphans can remain forever while the loop retries every second.

Keep progress between passes or rotate the starting point, and add a test for budget exhaustion.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[myles332]

Fixed in d1cc352: the budget now starts at the first removal, matching the run-artifact sweep (classification unbudgeted). Retained files can no longer exhaust it, and removed files leave the next listing, so every exhausted pass makes progress. Test: with a zero budget each pass removes exactly one orphan until none remain.