[5hseok]

Summary

Closes #9876.

Adds a Delete action to AI Vault session rows that removes a provider's session for real — moving it to the trash — behind a confirmation dialog.

AI Vault accumulates every local agent session across 16 providers, with no way to retire a finished one; they pile up and slow every scan. Deleting is the natural way to retire a session, but only if it's complete: an Orca that says "deleted" while the conversation is still on disk (or reappears on the next scan) is worse than no delete at all. So Delete is offered only where Orca can remove the session completely, and shown disabled (not hidden) elsewhere so the row's absence is never mistaken for a bug.

Supported (12 of 16 providers) — everything the session wrote lives at paths derived from the one path the scanner surfaced, and nothing there is shared with another session:

• Single file = session (9): gemini, copilot, cursor, hermes, devin, openclaw, droid, pi, omp
• Directory = session (3): claude, rovo, grok — claude keeps Task subagent transcripts in a sibling <uuid>/, rovo/grok keep everything under <sessionId>/, so a directory-aware delete is still complete.

Not supported (4): antigravity, kimi (dangling registry entry), codex (registry + hardlink aliases), opencode (a SQLite row, not a file). These show a disabled Delete item whose tooltip states the limit without exposing the provider's storage internals.

Design points:

• Real removal, to the trash (shell.trashItem) — deleting another app's data must be reversible; ENOENT is treated as success so an external delete stays idempotent.
• Renderer path is never trusted — the main process re-validates against each agent's known session roots (WSL-home expansion included) and the provider's own discovery predicate before removing anything, then invalidates the three caches that could otherwise resurrect the row.
• Ordered removal plan — companions first, the session's own transcript last, since the transcript is what puts the row on screen; a part-way failure leaves the row to retry from rather than dropping it and stranding data on disk.
• Claude sidecars — session-env/<uuid>/ (generated shell exports) is removed with the session; file-history/<uuid>/ (the rewind buffer holding earlier versions of the user's own files) is deliberately kept.

Screenshots

No visual change beyond the new Delete menu item, its confirmation dialog, and the disabled state for unsupported providers. (Reviewer: happy to add a short recording — the interaction is menu → Delete → confirm.)

Testing

• pnpm lint
• pnpm typecheck
• pnpm test (ai-vault suites green; a few unrelated suites fail only when git/electron env vars leak into the runner — GIT_ASKPASS, GIT_CONFIG_*, ELECTRON_RENDERER_URL — and pass in a clean env)
• pnpm build
• Added high-quality tests that would catch regressions (unit + an e2e spec that deletes real files in the isolated HOME)

Test coverage:

• Validator: root membership, .. escape, extension/predicate mismatch, non-local host, synthetic path, directory-shaped removal plans, degenerate stems.
• Executor: removal ordering, absent-companion idempotency, kind-mismatch rejection.
• Renderer: row stays collapsed when a menu action is chosen (a real bug found and fixed during manual testing).
• Manually exercised end-to-end against an isolated disposable HOME (never touching real provider data): single-file deletes, claude's 3-path delete with file-history preserved, rovo/grok directory deletes, the disabled codex tooltip, consecutive deletes, and cancel.

AI Review Report

Reviewed with an AI coding agent across the dimensions below.

• Cross-platform (macOS/Linux/Windows): paths built with path.join/resolve; no separator assumptions. WSL-home roots are expanded for validation. The one platform gap is the trash itself — WSL UNC paths have no recycle bin, handled by the existing wsl-unc-delete path (rm -f), attempted before the lstat/realpath guards.
• SSH/remote/local: local sessions only. SSH/runtime sessions live on the remote host and are shown disabled (non-local-host), matching the existing gating for Open/Reveal Log.
• Agent/integration compatibility: deletability is centralized in one shared table so main and renderer can't disagree; each supported agent's removal target reuses the scanner's own root/predicate derivation, so a delete target can't drift from what discovery surfaced. The 4 unsupported providers are explicitly excluded with recorded reasons.
• Performance: delete is a user-triggered, one-off fs op; no hot-path impact. The merge with main preserved upstream's memory-bounding of the parse-cache/list-cache (fix(memory): bound OOM-prone accumulators across Orca #10179) while keeping this branch's cache-generation guard.
• UI quality: disabled item carries an accessible reason (tooltip + aria-label); destructive styling on the item and dialog.
• Security (see below).

Fixed during review/testing: a leftover empty <uuid>/ directory after claude deletes (now removes the session directory itself, with a guard against a degenerate stem resolving to the project root), and rows expanding behind the confirm dialog due to React bubbling portalled menu clicks.

Security Audit

• Untrusted IPC input: the delete payload (agent, filePath, executionHostId) is untyped at runtime and never trusted. The main process re-derives the resolved path, checks membership in the agent's known roots after resolve() collapses .., matches the provider's discovery predicate, and re-checks each removal on disk (lstat kind + realpath still inside roots) before trashing — catching a regular file reached through a symlinked parent that escapes the roots.
• Path handling: no shell interpolation; removal goes through shell.trashItem (and the existing WSL rm -f helper for UNC paths, which refuses a bare directory and never follows a symlink).
• Blast radius: directory-shaped deletes reject a file sitting directly in the sessions root (which would trash every session) and a degenerate stem (which would resolve to the project root).
• No new secrets, auth, or network paths. No new dependencies.

Notes

• Platform-specific: WSL UNC deletes fall to rm -f (no recycle bin) rather than the trash — flagged in the confirm copy.
• Follow-up (out of scope, recorded in the design): extend deletion to SSH sessions via the ssh filesystem provider's deletePath.

Platform verification (honest scope)

• macOS: exercised end-to-end by hand against an isolated disposable HOME — single-file deletes, claude's directory delete with file-history preserved, rovo/grok directory deletes, the disabled codex tooltip, consecutive deletes, cancel.
• Linux: the new e2e spec (tests/e2e/ai-vault-session-delete.spec.ts) runs on CI and proves the real on-disk removal (single-file + claude directory + companions, file-history kept).
• Windows / WSL: covered by unit tests and code review only, not a live runtime. The one place this bit — WSL-UNC directory removals falling through to shell.trashItem — was found in review and fixed (route directory removals through the WSL rm -rf branch). Reviewers may want to give the Windows/WSL path a closer look.

ELI5

AI Vault session rows gain a Delete action that really moves the provider session to trash after confirm. Finished sessions no longer pile up forever and slow every scan.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds AI Vault session deletion with shared contracts, agent/path validation, ordered filesystem trash operations, IPC and preload APIs, cache invalidation, and stale-scan guards. The renderer gains deletion eligibility checks, action menus, confirmation dialogs, toast handling, web fallbacks, and localized strings. Scanner root helpers, OpenClaw path predicates, DROID root construction, subagent listing validation, and parse-cache entry invalidation are also added or shared. Extensive tests cover validation, deletion execution, cache behavior, IPC orchestration, and renderer interactions.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes implement the requested per-session delete flow, provider-aware validation, cache invalidation, and remote-session exclusion.
Out of Scope Changes check	✅ Passed	The diff appears focused on the delete feature, related cache/path helpers, UI, IPC wiring, tests, and locale updates.
Title check	✅ Passed	The title is concise and accurately summarizes the main change: deleting AI Vault provider sessions.
Description check	✅ Passed	The description matches the template with all required sections and covers summary, screenshots, testing, review, security, and notes.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/main/ai-vault/cached-session-list.ts (1)

57-79: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Keep a replacement same-key scan from being cleared by its predecessor.

After invalidation, a new request can start scan B with the same key as scan A. When A settles, the existing finally key comparison still matches and clears B’s in-flight tracking, allowing a third caller to launch another expensive scan. Guard cleanup by request identity or startGeneration, and add an A → invalidate → B → resolve A regression case.

Proposed fix

-      if (inflightKey === key) {
+      if (inflightKey === key && startGeneration === cacheGeneration) {
         inflightKey = null
         inflightList = null
       }

src/main/ipc/ai-vault.ts (1)

116-123: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Guard the multi-host in-flight cleanup with startGeneration.

invalidateMultiHostAiVaultListCache() now bumps and checks cacheGeneration, but the .finally() cleanup at line 124 still clears inflightKey/inflightList for any same-key scan. If an older scan resolves after a delete-invalidation, its cleanup can still clobber a newer same-key scan’s dedup tracking, causing an extra scan.

Proposed guard

     .finally(() => {
       // Only clear tracking if it still refers to this request: a concurrent
       // different-scope scan may have replaced it and must stay dedupable.
-      if (inflightKey === key) {
+      if (startGeneration === cacheGeneration && inflightKey === key) {
         inflightKey = null
         inflightList = null
       }
     })

🧹 Nitpick comments (2)

src/preload/index.ts (1)

3965-3966: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Tighten the aiVault preload return types.

Each aiVault IPC method is annotated as Promise<unknown>, while src/preload/api-types.ts declares specific result types like AiVaultDeleteSessionResult. This bypasses compile-time checking that the handler return shape matches the exposed API contract.

♻️ Suggested fix

-    deleteSession: (args: AiVaultDeleteSessionArgs): Promise<unknown> =>
+    deleteSession: (args: AiVaultDeleteSessionArgs): Promise<AiVaultDeleteSessionResult> =>
       ipcRenderer.invoke('aiVault:deleteSession', args),

src/renderer/src/components/right-sidebar/AiVaultSessionDeleteDialog.tsx (1)

1-1: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Ref mutated during render — prefer the conditional-setState "latest value" pattern.

Static analysis (React Doctor, no-ref-current-in-render) flags lastSessionRef.current = session as a render-time mutation. It's the common "keep last non-null value" idiom and is low-risk today, but writing to a ref unconditionally on every render (rather than only lazily/once) can misbehave with discarded/duplicated renders in Concurrent React. React's own docs recommend the equivalent pattern using state instead of a ref for exactly this "remember info from a previous render" case, which stays render-pure while producing the same flash-free result.

♻️ Proposed fix using conditional setState instead of ref mutation

-import { useRef } from 'react'
+import { useState } from 'react'

-  const lastSessionRef = useRef<AiVaultSession | null>(session)
-  if (session) {
-    lastSessionRef.current = session
-  }
-  const displayedSession = session ?? lastSessionRef.current
+  const [lastSession, setLastSession] = useState<AiVaultSession | null>(session)
+  if (session && session !== lastSession) {
+    setLastSession(session)
+  }
+  const displayedSession = session ?? lastSession

Also applies to: 31-35

Source: Linters/SAST tools

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e512fe41-57b3-48fc-ae55-4e3e23e95987

📥 Commits

Reviewing files that changed from the base of the PR and between 43f6265 and cb52731.

📒 Files selected for processing (39)

• src/main/ai-vault/cached-session-list.test.ts
• src/main/ai-vault/cached-session-list.ts
• src/main/ai-vault/session-delete-target.test.ts
• src/main/ai-vault/session-delete-target.ts
• src/main/ai-vault/session-delete.test.ts
• src/main/ai-vault/session-delete.ts
• src/main/ai-vault/session-scanner-discovery.ts
• src/main/ai-vault/session-scanner-droid-kimi-sources.ts
• src/main/ai-vault/session-scanner-parse-cache.ts
• src/main/ai-vault/session-scanner-root-dirs.ts
• src/main/ai-vault/session-scanner-source-discovery.ts
• src/main/ipc/ai-vault-delete.ts
• src/main/ipc/ai-vault-subagent-list.ts
• src/main/ipc/ai-vault.test.ts
• src/main/ipc/ai-vault.ts
• src/preload/api-types.ts
• src/preload/index.ts
• src/renderer/src/components/right-sidebar/AiVaultPanel.tsx
• src/renderer/src/components/right-sidebar/AiVaultSessionActionMenuItems.tsx
• src/renderer/src/components/right-sidebar/AiVaultSessionDeleteDialog.tsx
• src/renderer/src/components/right-sidebar/AiVaultSessionRow.test.tsx
• src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx
• src/renderer/src/components/right-sidebar/AiVaultSessionVirtualList.tsx
• src/renderer/src/components/right-sidebar/SessionRowTrailingActions.test.tsx
• src/renderer/src/components/right-sidebar/SessionRowTrailingActions.tsx
• src/renderer/src/components/right-sidebar/ai-vault-session-deletability.test.ts
• src/renderer/src/components/right-sidebar/ai-vault-session-deletability.ts
• src/renderer/src/components/right-sidebar/ai-vault-session-delete-action.test.tsx
• src/renderer/src/components/right-sidebar/ai-vault-session-delete-action.ts
• src/renderer/src/components/right-sidebar/ai-vault-session-delete-reason.test.ts
• src/renderer/src/components/right-sidebar/ai-vault-session-delete-reason.ts
• src/renderer/src/i18n/locales/en.json
• src/renderer/src/i18n/locales/es.json
• src/renderer/src/i18n/locales/ja.json
• src/renderer/src/i18n/locales/ko.json
• src/renderer/src/i18n/locales/zh.json
• src/renderer/src/web/web-preload-api.ts
• src/shared/ai-vault-session-deletion.ts
• src/shared/ai-vault-types.ts

[5hseok]

Thanks @coderabbitai — both addressed in 1834a80:

1. Unhandled IPC rejection: handleConfirmDelete now wraps the await in a catch that shows the same generic failure toast. The main handler resolves with a failed/rejected outcome rather than throwing, but a transport/serialization reject can still escape — and the caller fires it with void, so that would have been an unhandled rejection with no toast.
2. Mid-delete dismissal: handleDialogOpenChange now ignores close requests while deletingSession is true, so Radix's Escape/outside-click/X can't dismiss an in-flight delete out from under itself.

Both are covered by regression tests (verified failing without the fix).

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@5hseok Thanks for addressing both issues and adding regression coverage. The defensive IPC rejection handling and the in-flight dialog-dismissal guard resolve the concerns.

🐇✨

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

[AmethystLiang]

reviewing now!

[greptile-apps]

Greptile Summary

This PR adds a Delete action to AI Vault session rows, moving sessions to the OS trash after a confirmation dialog. It supports 12 of 16 providers (single-file and directory-shaped sessions), shows a disabled item with a tooltip for the 4 unsupported providers, and gates on live-session state, execution host, and path type before exposing the action.

• Main-process executor (session-delete-target.ts, session-delete.ts) re-validates every IPC payload against per-agent known roots using resolve() + realpath() guards, checks disk kind (lstat), and removes companions before the transcript so a partial failure leaves the session row visible for retry.
• Cache coherence is maintained via a cacheGeneration counter in both the multi-host cache (ai-vault.ts) and the shared local cache (cached-session-list.ts); an in-flight scan started before an invalidation cannot write its stale result back.
• Renderer layer computes deletability once per row (shared between dropdown and context menus) and gates on host, synthetic path, agent support, and live-session state in that order, deliberately ordering the session-live check last so a supported-but-running session shows an actionable "wait for it to finish" reason rather than a permanent one.

Confidence Score: 4/5

Safe to merge; the implementation is well-guarded on all code paths exercised by tests, with the one honest gap being Windows/WSL runtime validation, which is acknowledged and scoped to unit-only coverage.

The delete path is thoroughly tested (unit + e2e on Linux CI) and follows defense-in-depth: renderer UI gate, main-process path re-validation, disk-level kind + realpath checks, and cache-generation guards to prevent deleted sessions from resurfacing. The code path exercised least at runtime is Windows/WSL — specifically the WSL UNC directory-removal branch and the interaction of WSL home expansions with the realpath root checks. The author acknowledges this gap and recommends reviewers give that path a closer look before merging on Windows.

Files Needing Attention: The WSL UNC directory removal path in src/main/ai-vault/session-delete.ts (the tryDeleteWslUncPath branch with recursive: true) and the WSL home expansion in src/main/ai-vault/session-delete-target.ts would benefit from a runtime check on Windows before merging if WSL support is a near-term priority.

Important Files Changed

Filename	Overview
src/main/ai-vault/session-delete-target.ts	Pure validator that re-derives per-agent session roots from the same constants the scanner uses, checks path membership and file predicates, and builds an ordered removal plan. Degenerate-stem guard for claude prevents trashing the whole project directory.
src/main/ai-vault/session-delete.ts	Executor that runs the removal plan: tries WSL UNC fast path first, then lstat (kind check, symlink-safe), then realpath (roots re-realpath'd to handle symlinked-parent escape), then shell.trashItem. ENOENT treated as success throughout for idempotency.
src/main/ipc/ai-vault-delete.ts	IPC adapter: fetches WSL homes, delegates to executor, and on success invalidates all three caches. Parse-cache invalidation deliberately uses the raw renderer-echoed filePath (not resolve()-d) to match the scanner's own key.
src/shared/ai-vault-session-deletion.ts	Shared type and constant module. AI_VAULT_DELETABLE_AGENTS is the single source of truth used by both renderer and main, preventing disagreement on what is deletable.
src/main/ai-vault/cached-session-list.ts	Adds cacheGeneration counter to prevent a stale in-flight scan from overwriting the cache after a delete invalidation. Logic is correct.
src/main/ipc/ai-vault.ts	Registers the delete handler; adds the same cacheGeneration guard for the multi-host list cache. The aiVaultDeleteDeps const references a hoisted function declaration — valid JS, correctly noted in a comment.
src/renderer/src/components/right-sidebar/ai-vault-session-deletability.ts	Renderer-side UI gate. Correctly checks session-live state last. The renderer gate is a subset of the main-side gate, as required.
src/renderer/src/components/right-sidebar/ai-vault-session-delete-action.ts	Custom hook owning the confirm-delete flow: in-flight guard, IPC call, toast on settle, force-refresh. Error handling covers both rejected/failed outcomes and IPC transport errors.
src/renderer/src/components/right-sidebar/AiVaultSessionRow.tsx	Computes deletability once and passes it to both dropdown and context menus. Adds the portalled-menu click bubble-cancellation fix.
src/renderer/src/components/right-sidebar/AiVaultSessionDeleteDialog.tsx	Confirmation dialog with lastSessionRef pattern to prevent blank flash during Radix close animation. Disabled buttons during in-flight delete.
tests/e2e/ai-vault-session-delete.spec.ts	E2E spec against isolated HOME: covers single-file delete (gemini) and directory-shaped delete with companions (claude) including the file-history preservation assertion.
src/renderer/src/components/right-sidebar/AiVaultSessionActionMenuItems.tsx	Adds Delete item with accessible aria-label, wrapping div for tooltip hover over disabled item, and tooltip content explaining the limit without leaking storage internals.
src/renderer/src/web/web-preload-api.ts	Web stub for deleteSession correctly returns a non-local-host rejection rather than throwing or pretending to delete.

_{Reviews (2): Last reviewed commit: "fix(ai-vault): address review findings o..." | Re-trigger Greptile}

[AmethystLiang]

reviewing now