Skip to content

feat(workspace): add project directory management and drag-and-drop s… - #1483

Merged
zerob13 merged 2 commits into
devfrom
dnd-workspace
Apr 17, 2026
Merged

feat(workspace): add project directory management and drag-and-drop s…#1483
zerob13 merged 2 commits into
devfrom
dnd-workspace

Conversation

@zhangmo8

@zhangmo8 zhangmo8 commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator

…upport

image

Summary by CodeRabbit

  • New Features

    • Drag-and-drop and folder-picker flows to select and persist a workspace directory from the workspace panel; selection updates the UI immediately.
  • Localization

    • Added “no workspace” UI strings for workspace selection in English, Danish, French, Hebrew, Japanese, Korean, Portuguese (BR), Russian, Persian, Chinese (Simplified, Traditional), and Chinese (HK).
  • Tests

    • Added unit tests covering drag-and-drop and non-directory drop behavior.

@coderabbitai

coderabbitai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds UI folder-selection (drag-and-drop and picker) for workspace, a presenter/store flow to persist session projectDir, translation strings for the empty-workspace state, and minor dev-mode detection change in the floating button window. Tests updated to cover drag-and-drop behavior.

Changes

Cohort / File(s) Summary
Agent Session Presenter
src/main/presenter/agentSessionPresenter/index.ts, src/shared/types/presenters/agent-session.presenter.d.ts
Added public `setSessionProjectDir(sessionId, projectDir: string
Renderer — Workspace UI
src/renderer/src/components/sidepanel/WorkspacePanel.vue
Replaced empty placeholder with a drag-and-drop/select folder zone, added drag handlers, uses window.api.getPathForFile, validates via filePresenter.isDirectory, calls sessionStore.setSessionProjectDir, and emits update:workspacePath. Exposed emit typing.
Renderer — Store
src/renderer/src/stores/ui/session.ts
Added setSessionProjectDir(sessionId, projectDir) action which calls presenter and updates sessions state; sets error on failure and re-throws.
Tests — WorkspacePanel
test/renderer/components/WorkspacePanel.test.ts
Refactored usePresenter(name) mock to provide projectPresenter/filePresenter; added useSessionStore mock and window.api.getPathForFile mock. Added tests for directory drop (triggers setSessionProjectDir + emit) and non-directory drop (ignored).
Internationalization
src/renderer/src/i18n/*/chat.json (da-DK, en-US, fa-IR, fr-FR, he-IL, ja-JP, ko-KR, pt-BR, ru-RU, zh-CN, zh-HK, zh-TW)
Added workspace.files.noWorkspace translation block (title, description, button) across 12 locales for the empty workspace UI.
Floating Button Window
src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts
Switched dev-mode detection to use is.dev from @electron-toolkit/utils instead of process.env.NODE_ENV.

Sequence Diagram

sequenceDiagram
    participant User as User
    participant UI as WorkspacePanel
    participant API as window.api
    participant FileP as FilePresenter
    participant ProjectP as ProjectPresenter
    participant Store as SessionStore
    participant Presenter as AgentSessionPresenter
    participant DB as Database

    User->>UI: Drag folder or click "Select folder"
    alt Select via picker
        UI->>ProjectP: selectDirectory()
        ProjectP-->>UI: selected path
    else Drag and drop
        UI->>API: getPathForFile(File)
        API-->>UI: resolved path
    end
    UI->>FileP: isDirectory(path)
    FileP-->>UI: true/false
    alt Directory
        UI->>Store: setSessionProjectDir(sessionId, path)
        Store->>Presenter: setSessionProjectDir(sessionId, path)
        Presenter->>DB: update session.projectDir
        Presenter->>DB: syncPath(projectDir) (optional)
        Presenter-->>Store: SessionWithState
        Store-->>UI: success
        UI->>User: emit update:workspacePath
    else Not a directory
        UI-->>User: ignore / no action
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

A rabbit nudges folders with a hop and grin, 🐰
Dragged or chosen, a workspace now begins,
Strings in many tongues sing out the new way,
Tests keep watch while presenters save the day,
Hopping code and translations — hip hooray! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding project directory management and drag-and-drop support for workspace setup, which aligns with the changeset's core functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dnd-workspace

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Caution

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

⚠️ Outside diff range comments (1)
src/renderer/src/components/sidepanel/WorkspacePanel.vue (1)

1-1: ⚠️ Potential issue | 🟡 Minor

Pipeline: oxfmt --check reported formatting issues.

The PR Check workflow reports that this file has oxfmt formatting violations. Please run oxfmt --write (or the project’s equivalent) on this file before merging.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/src/components/sidepanel/WorkspacePanel.vue` at line 1,
WorkspacePanel.vue has oxfmt formatting violations; run the formatter (e.g.,
oxfmt --write src/renderer/src/components/sidepanel/WorkspacePanel.vue or the
project's equivalent) to reformat the file, stage and commit the changes so the
Pipeline `oxfmt --check` passes; ensure the <template> block and surrounding
markup are reformatted by the tool before pushing.
🧹 Nitpick comments (3)
src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts (1)

88-92: Consider honoring ELECTRON_RENDERER_URL for the dev URL.

The hardcoded http://localhost:5173/floating/ will break if the Vite dev server runs on a different host/port (e.g., when ELECTRON_RENDERER_URL is overridden in the environment). The main window in src/main/presenter/windowPresenter/index.ts guards dev loading with is.dev && process.env['ELECTRON_RENDERER_URL'] and uses that value; applying the same pattern here would keep the floating entry in sync with the rest of the app. Not a blocker since FloatingChatWindow uses the same hardcoded style, but worth aligning eventually.

-      if (isDev) {
-        await this.window.loadURL('http://localhost:5173/floating/')
+      const rendererUrl = process.env['ELECTRON_RENDERER_URL']
+      if (isDev && rendererUrl) {
+        await this.window.loadURL(`${rendererUrl}/floating/`)
       } else {
         await this.window.loadFile(path.join(__dirname, '../renderer/floating/index.html'))
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts` around
lines 88 - 92, The dev URL is hardcoded in FloatingButtonWindow causing mismatch
when VITE/ELECTRON_RENDERER_URL is overridden; update the isDev branch in the
constructor/initializer that calls this.window.loadURL to first check
process.env['ELECTRON_RENDERER_URL'] (same pattern used in WindowPresenter) and,
if present, append the floating route (e.g. '/floating/') to that base URL
before calling this.window.loadURL, otherwise fall back to the existing
hardcoded localhost URL; keep the production path using this.window.loadFile
as-is.
src/renderer/src/components/sidepanel/WorkspacePanel.vue (2)

400-423: Remove or downgrade verbose console.log diagnostics before merge.

handleDrop emits a handful of informational console.log messages (lines 400, 406, 409, 420, 423) that look like debug traces. These will run for every drop in production and pollute users’ DevTools. The console.warn/console.error for actual error conditions are fine; consider removing the console.logs (or gating behind a debug flag).

🧹 Proposed cleanup
   const file = getDroppedFile(event)
   if (!file) {
-    console.log('[WorkspacePanel] No files in drop event')
     return
   }

   const filePath = getDroppedFilePath(file)

-  console.log('[WorkspacePanel] Dropped file:', filePath, file.name)
-
   if (!filePath) {
-    console.log('[WorkspacePanel] No file path available - drag from browser')
     return
   }

   try {
     const isDirectory = await filePresenter.isDirectory(filePath)
     if (!isDirectory) {
       console.warn('[WorkspacePanel] Dropped path is not a directory:', filePath)
       return
     }

-    console.log('[WorkspacePanel] Setting project dir to:', filePath)
     await sessionStore.setSessionProjectDir(props.sessionId, filePath)
     emit('update:workspacePath', filePath)
-    console.log('[WorkspacePanel] Project dir set successfully')
   } catch (e) {
     console.error('[WorkspacePanel] Failed to set workspace from drop:', e)
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/src/components/sidepanel/WorkspacePanel.vue` around lines 400 -
423, The handleDrop handler currently uses multiple verbose console.log calls
that should be removed or gated; remove the non-error console.log statements
(the ones logging '[WorkspacePanel] No files in drop event', 'Dropped file:',
'No file path available - drag from browser', 'Setting project dir to:', and
'Project dir set successfully') inside the drop flow, or replace them with a
conditional debug logger controlled by a config flag; keep the console.warn for
the non-directory case and ensure sessionStore.setSessionProjectDir,
filePresenter.isDirectory, props.sessionId, and emit('update:workspacePath',
...) behavior remains unchanged.

19-27: dragenter sets isDragging unconditionally — can flash the highlight for non-file drags.

@dragenter.prevent="isDragging = true" triggers the highlight for any drag (e.g., text, links), while handleDragOver gates the highlight on hasDroppedFiles(event). Consider using a handler on dragenter that applies the same guard so the empty state doesn't highlight for unsupported payloads.

✏️ Proposed fix
-              `@dragenter.prevent`="isDragging = true"
+              `@dragenter.prevent`="handleDragEnter"
               `@dragover.prevent`="handleDragOver"
               `@dragleave`="handleDragLeave"
               `@drop.prevent`="handleDrop"
+function handleDragEnter(event: DragEvent) {
+  if (hasDroppedFiles(event)) {
+    isDragging.value = true
+  }
+}
+
 function handleDragOver(event: DragEvent) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/src/components/sidepanel/WorkspacePanel.vue` around lines 19 -
27, The dragenter handler sets isDragging unconditionally; replace the inline
`@dragenter` with a method (e.g., handleDragEnter) that calls
hasDroppedFiles(event) and only sets isDragging = true when that returns true,
keeping the existing `@dragover.prevent`="handleDragOver" and
`@dragleave`="handleDragLeave" behavior; update the template to use
`@dragenter.prevent`="handleDragEnter" and add/modify the handleDragEnter(event)
method to guard with hasDroppedFiles before setting isDragging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/presenter/agentSessionPresenter/index.ts`:
- Around line 1552-1575: In setSessionProjectDir, ensure ACP sessions are
treated like other mutation paths by checking and syncing ACP workdir: call
assertAcpSessionHasWorkdir(session) and then syncAcpSessionWorkdir(sessionId,
projectDir) (or the existing sync helper) when the session is ACP-backed and
projectDir is non-null; this mirrors behavior in
sendMessage/queuePendingInput/ensureAcpDraftSession. Also capture the previous
projectDir from the session before updating (const oldPath = session.projectDir)
and after updating call
this.sqlitePresenter.newEnvironmentsTable.syncPath(projectDir) for the new path
and, if oldPath is truthy and different, call syncPath(oldPath) to recompute
aggregates for the old location so its session_count/last_used_at don’t go
stale.
- Around line 1736-1746: The function tryBuildSessionWithState currently returns
null cast to SessionWithState which lies about nullability; change its signature
from Promise<SessionWithState> to Promise<SessionWithState | null> and keep
returning null in the catch branch, then update all callers (notably
getSessionList, getSession, setSessionSubagentEnabled, setSessionProjectDir) to
explicitly handle the nullable result (preserve/restore the existing if
(session) / if (!sessionWithState) checks or add null checks) so null does not
get typed as a SessionWithState anywhere.

In `@src/renderer/src/components/sidepanel/WorkspacePanel.vue`:
- Around line 413-421: There is a TOCTOU race between calling
filePresenter.isDirectory(filePath) and then
sessionStore.setSessionProjectDir(props.sessionId, filePath); to harden this,
replace the two-step check+set with a single atomic RPC or server-side
validation: add or use a presenter method (e.g.,
filePresenter.setSessionProjectDirAtomic or
filePresenter.validateAndSetProjectDir) that verifies the path and persists the
session project dir in one operation, or modify
sessionStore.setSessionProjectDir to perform server-side re-validation (using
filePresenter.isDirectory on the main process) before persisting; update the
call site (currently using filePresenter.isDirectory and
sessionStore.setSessionProjectDir) to call the new/modified atomic method so
deletion/replacement between calls cannot cause inconsistency and rely on
syncPath for further reconciliation.

In `@src/renderer/src/i18n/da-DK/chat.json`:
- Line 187: Update the Danish translation value for the "title" key in the
chat.json entry currently set to "Ingen arbejdsområde" to the grammatically
correct neuter form "Intet arbejdsområde"; locate the JSON object containing
"title": "Ingen arbejdsområde" and replace the string accordingly so the
translation matches the noun gender.

In `@src/renderer/src/i18n/he-IL/chat.json`:
- Line 185: The "section" label currently uses the Hebrew word "מִסְמָך"
(document) which mislabels workspace files; update the "section" key's value in
chat.json (the "section" string) to the correct Hebrew label for workspace
files—for example "קבצים" or "קבצי עבודה"—so the UI correctly reads as workspace
files.

In `@src/renderer/src/i18n/ja-JP/chat.json`:
- Around line 187-188: Update the Japanese empty-state "description" string to
mention both selecting a folder and drag-and-drop so it matches the new setup
flow; edit the "description" key in src/renderer/src/i18n/ja-JP/chat.json (the
JSON object containing "description" and "button") to include language that
references フォルダ選択とドラッグ&ドロップ (or similar natural Japanese phrasing) while keeping
the "button" text ("フォルダを選択") unchanged.

In `@src/renderer/src/i18n/ko-KR/chat.json`:
- Line 185: The value for workspace.files.section is incorrectly set to "문서";
update the JSON entry for the "section" key (currently "문서") to a
files-equivalent Korean label such as "파일" (or "파일들" if pluralization is
preferred) so the workspace.files.section string correctly reflects the Files
section in Korean.

In `@src/renderer/src/i18n/pt-BR/chat.json`:
- Line 185: Replace the incorrect Portuguese value for the workspace files
section: locate the key "workspace.files.section" in the pt-BR locale file
(currently set to "documento") and update its value to the proper translation
"Arquivos" so the label matches the "Files" section semantics in the workspace
sidebar.

In `@src/renderer/src/i18n/ru-RU/chat.json`:
- Line 185: The "section" value in chat.json is incorrectly set to "документ";
update the "section" property for the files/workspace section in
src/renderer/src/i18n/ru-RU/chat.json (the JSON key "section") to the correct
Russian label (e.g., "файлы" or "Файлы" depending on surrounding capitalization)
so the workspace files section is properly labeled.

---

Outside diff comments:
In `@src/renderer/src/components/sidepanel/WorkspacePanel.vue`:
- Line 1: WorkspacePanel.vue has oxfmt formatting violations; run the formatter
(e.g., oxfmt --write src/renderer/src/components/sidepanel/WorkspacePanel.vue or
the project's equivalent) to reformat the file, stage and commit the changes so
the Pipeline `oxfmt --check` passes; ensure the <template> block and surrounding
markup are reformatted by the tool before pushing.

---

Nitpick comments:
In `@src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts`:
- Around line 88-92: The dev URL is hardcoded in FloatingButtonWindow causing
mismatch when VITE/ELECTRON_RENDERER_URL is overridden; update the isDev branch
in the constructor/initializer that calls this.window.loadURL to first check
process.env['ELECTRON_RENDERER_URL'] (same pattern used in WindowPresenter) and,
if present, append the floating route (e.g. '/floating/') to that base URL
before calling this.window.loadURL, otherwise fall back to the existing
hardcoded localhost URL; keep the production path using this.window.loadFile
as-is.

In `@src/renderer/src/components/sidepanel/WorkspacePanel.vue`:
- Around line 400-423: The handleDrop handler currently uses multiple verbose
console.log calls that should be removed or gated; remove the non-error
console.log statements (the ones logging '[WorkspacePanel] No files in drop
event', 'Dropped file:', 'No file path available - drag from browser', 'Setting
project dir to:', and 'Project dir set successfully') inside the drop flow, or
replace them with a conditional debug logger controlled by a config flag; keep
the console.warn for the non-directory case and ensure
sessionStore.setSessionProjectDir, filePresenter.isDirectory, props.sessionId,
and emit('update:workspacePath', ...) behavior remains unchanged.
- Around line 19-27: The dragenter handler sets isDragging unconditionally;
replace the inline `@dragenter` with a method (e.g., handleDragEnter) that calls
hasDroppedFiles(event) and only sets isDragging = true when that returns true,
keeping the existing `@dragover.prevent`="handleDragOver" and
`@dragleave`="handleDragLeave" behavior; update the template to use
`@dragenter.prevent`="handleDragEnter" and add/modify the handleDragEnter(event)
method to guard with hasDroppedFiles before setting isDragging.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8c4c679b-90bd-41b8-ac3b-565796af8145

📥 Commits

Reviewing files that changed from the base of the PR and between d9ea5ef and 1d35a74.

📒 Files selected for processing (18)
  • src/main/presenter/agentSessionPresenter/index.ts
  • src/main/presenter/floatingButtonPresenter/FloatingButtonWindow.ts
  • src/renderer/src/components/sidepanel/WorkspacePanel.vue
  • src/renderer/src/i18n/da-DK/chat.json
  • src/renderer/src/i18n/en-US/chat.json
  • src/renderer/src/i18n/fa-IR/chat.json
  • src/renderer/src/i18n/fr-FR/chat.json
  • src/renderer/src/i18n/he-IL/chat.json
  • src/renderer/src/i18n/ja-JP/chat.json
  • src/renderer/src/i18n/ko-KR/chat.json
  • src/renderer/src/i18n/pt-BR/chat.json
  • src/renderer/src/i18n/ru-RU/chat.json
  • src/renderer/src/i18n/zh-CN/chat.json
  • src/renderer/src/i18n/zh-HK/chat.json
  • src/renderer/src/i18n/zh-TW/chat.json
  • src/renderer/src/stores/ui/session.ts
  • src/shared/types/presenters/agent-session.presenter.d.ts
  • test/renderer/components/WorkspacePanel.test.ts

Comment on lines +1552 to +1575
async setSessionProjectDir(
sessionId: string,
projectDir: string | null
): Promise<SessionWithState> {
const session = this.sessionManager.get(sessionId)
if (!session) {
throw new Error(`Session not found: ${sessionId}`)
}

this.sessionManager.update(sessionId, { projectDir })

// Sync environment for new project dir
if (projectDir) {
this.sqlitePresenter.newEnvironmentsTable.syncPath(projectDir)
}

const updated = this.sessionManager.get(sessionId)
if (!updated) {
throw new Error(`Session not found after update: ${sessionId}`)
}

this.emitSessionListUpdated()
return await this.tryBuildSessionWithState(updated)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

ACP sessions and stale environment aggregates aren't handled.

Two gaps worth addressing:

  1. ACP workdir not synced. Every other mutation path (sendMessage, queuePendingInput, ensureAcpDraftSession) guards ACP sessions with assertAcpSessionHasWorkdir and calls syncAcpSessionWorkdir. Changing projectDir for an ACP-backed session here bypasses both, leaving the ACP runtime pointing at the old workdir and allowing null to silently disarm the invariant enforced elsewhere. Consider asserting + syncing for ACP, or documenting that this entry point is DeepChat-only.

  2. Old path's environment row goes stale. newEnvironmentsTable.syncPath recomputes aggregates for the path you pass in (per src/main/presenter/sqlitePresenter/tables/newEnvironments.ts:122-150). When a session moves from path A → B (or A → null), A's session_count / last_used_at are not re-synced, so the row lingers until another session touches it. Capture the previous session.projectDir and call syncPath on it as well.

🛠️ Suggested shape
     const session = this.sessionManager.get(sessionId)
     if (!session) {
       throw new Error(`Session not found: ${sessionId}`)
     }

+    const previousProjectDir = session.projectDir ?? null
     this.sessionManager.update(sessionId, { projectDir })

-    // Sync environment for new project dir
-    if (projectDir) {
-      this.sqlitePresenter.newEnvironmentsTable.syncPath(projectDir)
+    // Sync environment aggregates for both old and new project dirs
+    if (previousProjectDir && previousProjectDir !== projectDir) {
+      this.sqlitePresenter.newEnvironmentsTable.syncPath(previousProjectDir)
+    }
+    if (projectDir) {
+      this.sqlitePresenter.newEnvironmentsTable.syncPath(projectDir)
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async setSessionProjectDir(
sessionId: string,
projectDir: string | null
): Promise<SessionWithState> {
const session = this.sessionManager.get(sessionId)
if (!session) {
throw new Error(`Session not found: ${sessionId}`)
}
this.sessionManager.update(sessionId, { projectDir })
// Sync environment for new project dir
if (projectDir) {
this.sqlitePresenter.newEnvironmentsTable.syncPath(projectDir)
}
const updated = this.sessionManager.get(sessionId)
if (!updated) {
throw new Error(`Session not found after update: ${sessionId}`)
}
this.emitSessionListUpdated()
return await this.tryBuildSessionWithState(updated)
}
async setSessionProjectDir(
sessionId: string,
projectDir: string | null
): Promise<SessionWithState> {
const session = this.sessionManager.get(sessionId)
if (!session) {
throw new Error(`Session not found: ${sessionId}`)
}
const previousProjectDir = session.projectDir ?? null
this.sessionManager.update(sessionId, { projectDir })
// Sync environment aggregates for both old and new project dirs
if (previousProjectDir && previousProjectDir !== projectDir) {
this.sqlitePresenter.newEnvironmentsTable.syncPath(previousProjectDir)
}
if (projectDir) {
this.sqlitePresenter.newEnvironmentsTable.syncPath(projectDir)
}
const updated = this.sessionManager.get(sessionId)
if (!updated) {
throw new Error(`Session not found after update: ${sessionId}`)
}
this.emitSessionListUpdated()
return await this.tryBuildSessionWithState(updated)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/presenter/agentSessionPresenter/index.ts` around lines 1552 - 1575,
In setSessionProjectDir, ensure ACP sessions are treated like other mutation
paths by checking and syncing ACP workdir: call
assertAcpSessionHasWorkdir(session) and then syncAcpSessionWorkdir(sessionId,
projectDir) (or the existing sync helper) when the session is ACP-backed and
projectDir is non-null; this mirrors behavior in
sendMessage/queuePendingInput/ensureAcpDraftSession. Also capture the previous
projectDir from the session before updating (const oldPath = session.projectDir)
and after updating call
this.sqlitePresenter.newEnvironmentsTable.syncPath(projectDir) for the new path
and, if oldPath is truthy and different, call syncPath(oldPath) to recompute
aggregates for the old location so its session_count/last_used_at don’t go
stale.

Comment on lines +1736 to 1746
private async tryBuildSessionWithState(record: SessionRecord): Promise<SessionWithState> {
try {
return await this.buildSessionWithState(record)
} catch (error) {
console.warn(
`[AgentSessionPresenter] Skipping unavailable session id=${record.id} agent=${record.agentId}:`,
error
)
return null
return null as unknown as SessionWithState
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Return type lies about nullability — null leaks as SessionWithState.

tryBuildSessionWithState now claims Promise<SessionWithState> but the catch branch still returns null as unknown as SessionWithState. This:

  • Makes every caller's type narrowing dead. if (session) in getSessionList (Line 933) and if (!sessionWithState) in setSessionSubagentEnabled (Line 1508) are now typed as always-truthy, so a future refactor may legitimately delete them and start pushing/returning null objects.
  • Silently propagates a real null through the new setSessionProjectDir (Line 1574), whose declared return is Promise<SessionWithState>, so an IPC caller can receive null typed as a session.
  • Contradicts getSession's declared Promise<SessionWithState | null> (Line 941), which now depends on this implicit nullability.

Prefer keeping the original nullable signature and handling null explicitly at the new call site.

🛠️ Proposed fix
-  private async tryBuildSessionWithState(record: SessionRecord): Promise<SessionWithState> {
+  private async tryBuildSessionWithState(
+    record: SessionRecord
+  ): Promise<SessionWithState | null> {
     try {
       return await this.buildSessionWithState(record)
     } catch (error) {
       console.warn(
         `[AgentSessionPresenter] Skipping unavailable session id=${record.id} agent=${record.agentId}:`,
         error
       )
-      return null as unknown as SessionWithState
+      return null
     }
   }

And in setSessionProjectDir (Line 1574):

-    this.emitSessionListUpdated()
-    return await this.tryBuildSessionWithState(updated)
+    this.emitSessionListUpdated()
+    const sessionWithState = await this.tryBuildSessionWithState(updated)
+    if (!sessionWithState) {
+      throw new Error(`Failed to build session state for sessionId: ${sessionId}`)
+    }
+    return sessionWithState
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/main/presenter/agentSessionPresenter/index.ts` around lines 1736 - 1746,
The function tryBuildSessionWithState currently returns null cast to
SessionWithState which lies about nullability; change its signature from
Promise<SessionWithState> to Promise<SessionWithState | null> and keep returning
null in the catch branch, then update all callers (notably getSessionList,
getSession, setSessionSubagentEnabled, setSessionProjectDir) to explicitly
handle the nullable result (preserve/restore the existing if (session) / if
(!sessionWithState) checks or add null checks) so null does not get typed as a
SessionWithState anywhere.

Comment on lines +413 to +421
try {
const isDirectory = await filePresenter.isDirectory(filePath)
if (!isDirectory) {
console.warn('[WorkspacePanel] Dropped path is not a directory:', filePath)
return
}

console.log('[WorkspacePanel] Setting project dir to:', filePath)
await sessionStore.setSessionProjectDir(props.sessionId, filePath)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

TOCTOU between isDirectory check and persistence — acceptable, but worth noting.

filePresenter.isDirectory(filePath) runs in the main process and then the result is sent back before sessionStore.setSessionProjectDir(...) is invoked, so a directory could be deleted/replaced between the two calls. The backend eventually handles any issue via syncPath, so this is not exploitable, just a minor reliability nit. No action required unless you want the presenter to re-validate the path atomically when setting the project dir.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/src/components/sidepanel/WorkspacePanel.vue` around lines 413 -
421, There is a TOCTOU race between calling filePresenter.isDirectory(filePath)
and then sessionStore.setSessionProjectDir(props.sessionId, filePath); to harden
this, replace the two-step check+set with a single atomic RPC or server-side
validation: add or use a presenter method (e.g.,
filePresenter.setSessionProjectDirAtomic or
filePresenter.validateAndSetProjectDir) that verifies the path and persists the
session project dir in one operation, or modify
sessionStore.setSessionProjectDir to perform server-side re-validation (using
filePresenter.isDirectory on the main process) before persisting; update the
call site (currently using filePresenter.isDirectory and
sessionStore.setSessionProjectDir) to call the new/modified atomic method so
deletion/replacement between calls cannot cause inconsistency and rely on
syncPath for further reconciliation.

"section": "dokument"
"section": "dokument",
"noWorkspace": {
"title": "Ingen arbejdsområde",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Minor Danish grammar: "Ingen" → "Intet".

arbejdsområde is a neuter noun (et-word), so the correct negation is Intet arbejdsområde rather than Ingen arbejdsområde.

✏️ Proposed fix
-        "title": "Ingen arbejdsområde",
+        "title": "Intet arbejdsområde",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"title": "Ingen arbejdsområde",
"title": "Intet arbejdsområde",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/src/i18n/da-DK/chat.json` at line 187, Update the Danish
translation value for the "title" key in the chat.json entry currently set to
"Ingen arbejdsområde" to the grammatically correct neuter form "Intet
arbejdsområde"; locate the JSON object containing "title": "Ingen arbejdsområde"
and replace the string accordingly so the translation matches the noun gender.

"empty": "עדיין אין קבצים",
"loading": "טוען קבצים...",
"section": "מִסְמָך"
"section": "מִסְמָך",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Correct Hebrew section label for workspace files.
Line 185 uses "מִסְמָך" ("document"), which appears to mislabel the files section.

💡 Suggested fix
-      "section": "מִסְמָך",
+      "section": "קבצים",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"section": "מִסְמָך",
"section": "קבצים",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/src/i18n/he-IL/chat.json` at line 185, The "section" label
currently uses the Hebrew word "מִסְמָך" (document) which mislabels workspace
files; update the "section" key's value in chat.json (the "section" string) to
the correct Hebrew label for workspace files—for example "קבצים" or "קבצי
עבודה"—so the UI correctly reads as workspace files.

Comment on lines +187 to +188
"description": "フォルダを選択してワークスペースを設定",
"button": "フォルダを選択"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Include drag-and-drop in Japanese empty-state description for feature parity.
Line 188 only mentions folder selection. Since this PR adds drag-and-drop setup, the copy should mention both actions.

💡 Suggested fix
-        "description": "フォルダを選択してワークスペースを設定",
+        "description": "フォルダを選択またはドラッグしてワークスペースを設定",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"description": "フォルダを選択してワークスペースを設定",
"button": "フォルダを選択"
"description": "フォルダを選択またはドラッグしてワークスペースを設定",
"button": "フォルダを選択"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/src/i18n/ja-JP/chat.json` around lines 187 - 188, Update the
Japanese empty-state "description" string to mention both selecting a folder and
drag-and-drop so it matches the new setup flow; edit the "description" key in
src/renderer/src/i18n/ja-JP/chat.json (the JSON object containing "description"
and "button") to include language that references フォルダ選択とドラッグ&ドロップ (or similar
natural Japanese phrasing) while keeping the "button" text ("フォルダを選択")
unchanged.

"empty": "아직 파일이 없습니다",
"loading": "파일 로드 중...",
"section": "문서"
"section": "문서",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use a files-equivalent label for Korean workspace.files.section.
Line 185 changed to "문서" (document), which is inconsistent with the intended files section label.

💡 Suggested fix
-      "section": "문서",
+      "section": "파일",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"section": "문서",
"section": "파일",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/src/i18n/ko-KR/chat.json` at line 185, The value for
workspace.files.section is incorrectly set to "문서"; update the JSON entry for
the "section" key (currently "문서") to a files-equivalent Korean label such as
"파일" (or "파일들" if pluralization is preferred) so the workspace.files.section
string correctly reflects the Files section in Korean.

"empty": "Nenhum arquivo ainda",
"loading": "Carregando arquivos...",
"section": "documento"
"section": "documento",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix section label regression in Portuguese locale.
Line 185 changed workspace.files.section to "documento", which no longer matches the "Files" section semantics and appears incorrect in the workspace sidebar.

💡 Suggested fix
-      "section": "documento",
+      "section": "Arquivos",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"section": "documento",
"section": "Arquivos",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/src/i18n/pt-BR/chat.json` at line 185, Replace the incorrect
Portuguese value for the workspace files section: locate the key
"workspace.files.section" in the pt-BR locale file (currently set to
"documento") and update its value to the proper translation "Arquivos" so the
label matches the "Files" section semantics in the workspace sidebar.

"empty": "Файлов пока нет",
"loading": "Загрузка файлов...",
"section": "документ"
"section": "документ",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Restore correct Russian label for the files section.
Line 185 uses "документ", which mislabels the workspace files section.

💡 Suggested fix
-      "section": "документ",
+      "section": "Файлы",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"section": "документ",
"section": "Файлы",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/src/i18n/ru-RU/chat.json` at line 185, The "section" value in
chat.json is incorrectly set to "документ"; update the "section" property for
the files/workspace section in src/renderer/src/i18n/ru-RU/chat.json (the JSON
key "section") to the correct Russian label (e.g., "файлы" or "Файлы" depending
on surrounding capitalization) so the workspace files section is properly
labeled.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/renderer/src/components/sidepanel/WorkspacePanel.vue (2)

404-427: Remove or gate debug console.log statements.

The drop handler logs user filesystem paths and file names to the console on every drop (Lines 404, 410, 413, 424, 427). This is noisy for production, and the path/filename log at Line 410 leaks user workspace paths to the devtools console. The console.warn/console.error on actual failure paths are fine to keep.

♻️ Proposed cleanup
   const file = getDroppedFile(event)
   if (!file) {
-    console.log('[WorkspacePanel] No files in drop event')
     return
   }

   const filePath = getDroppedFilePath(file)

-  console.log('[WorkspacePanel] Dropped file:', filePath, file.name)
-
   if (!filePath) {
-    console.log('[WorkspacePanel] No file path available - drag from browser')
     return
   }

   try {
     const isDirectory = await filePresenter.isDirectory(filePath)
     if (!isDirectory) {
       console.warn('[WorkspacePanel] Dropped path is not a directory:', filePath)
       return
     }

-    console.log('[WorkspacePanel] Setting project dir to:', filePath)
     await sessionStore.setSessionProjectDir(props.sessionId, filePath)
     emit('update:workspacePath', filePath)
-    console.log('[WorkspacePanel] Project dir set successfully')
   } catch (e) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/src/components/sidepanel/WorkspacePanel.vue` around lines 404 -
427, Remove or gate the noisy debug console.log calls in the drop handler that
emit user file paths/file names: replace or wrap the console.log calls around
getDroppedFilePath, the "Dropped file" message, "No file path available" and the
"Setting project dir" / "Project dir set successfully" messages so they do not
run in production (use an existing isDev flag or app logger debug level) while
keeping the console.warn for non-directory cases and any console.error for real
failures; touch the functions/locals getDroppedFilePath,
filePresenter.isDirectory, sessionStore.setSessionProjectDir and the
emit('update:workspacePath') call to locate the handler to apply this change.

444-467: Only the first dropped item is used; multi-folder drops silently dropped.

getDroppedFile returns only droppedFiles[0] (or the first file-kind item), so if a user drags multiple folders, all but the first are silently ignored without feedback. Since the empty-state explicitly invites "拖拽文件夹", consider either (a) documenting single-folder semantics via the i18n description, or (b) warning/ignoring drops of length > 1 with a user-visible hint. Not a blocker given the empty-workspace single-path model.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/renderer/src/components/sidepanel/WorkspacePanel.vue` around lines 444 -
467, getDroppedFile currently returns only the first file and silently ignores
additional drops; update getDroppedFile to detect multi-item drops (check
droppedFiles.length > 1 and droppedItems.length > 1) and handle them explicitly:
either return null and trigger a user-visible notification/hint (e.g. call the
existing toast/notification helper or emit an event like
"multiple-drop-detected") or update the i18n copy to state single-folder-only
semantics; ensure you reference the getDroppedFile function and the
droppedFiles/droppedItems checks so the UI shows feedback instead of silently
dropping extra folders.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/renderer/src/components/sidepanel/WorkspacePanel.vue`:
- Around line 404-427: Remove or gate the noisy debug console.log calls in the
drop handler that emit user file paths/file names: replace or wrap the
console.log calls around getDroppedFilePath, the "Dropped file" message, "No
file path available" and the "Setting project dir" / "Project dir set
successfully" messages so they do not run in production (use an existing isDev
flag or app logger debug level) while keeping the console.warn for non-directory
cases and any console.error for real failures; touch the functions/locals
getDroppedFilePath, filePresenter.isDirectory, sessionStore.setSessionProjectDir
and the emit('update:workspacePath') call to locate the handler to apply this
change.
- Around line 444-467: getDroppedFile currently returns only the first file and
silently ignores additional drops; update getDroppedFile to detect multi-item
drops (check droppedFiles.length > 1 and droppedItems.length > 1) and handle
them explicitly: either return null and trigger a user-visible notification/hint
(e.g. call the existing toast/notification helper or emit an event like
"multiple-drop-detected") or update the i18n copy to state single-folder-only
semantics; ensure you reference the getDroppedFile function and the
droppedFiles/droppedItems checks so the UI shows feedback instead of silently
dropping extra folders.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8a59d030-3067-4051-b6e6-4673ba87a593

📥 Commits

Reviewing files that changed from the base of the PR and between 1d35a74 and d403710.

📒 Files selected for processing (1)
  • src/renderer/src/components/sidepanel/WorkspacePanel.vue

@zerob13
zerob13 merged commit f0a91b7 into dev Apr 17, 2026
3 checks passed
@zhangmo8
zhangmo8 deleted the dnd-workspace branch April 17, 2026 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants