fix(agent): validate workdir fallback

Author: zerob13

src/main/presenter/newAgentPresenter/index.ts

@@ -150,7 +150,12 @@ export class NewAgentPresenter {
     if (input.generationSettings) {
       initConfig.generationSettings = input.generationSettings
     }
-    await this.initializeSessionRuntime(agent, sessionId, initConfig)
+    try {
+      await this.initializeSessionRuntime(agent, sessionId, initConfig)
+    } catch (error) {
+      await this.cleanupFailedSessionInitialization(agent, sessionId)
+      throw error
+    }
     console.log(`[NewAgentPresenter] agent.initSession done`)
 
     // Bind to window and emit activated
@@ -224,13 +229,18 @@ export class NewAgentPresenter {
       const sessionId = this.sessionManager.create(agentId, 'New Chat', projectDir, {
         isDraft: true
       })
-      await this.ensureSessionRuntimeInitialized(agent, sessionId, {
-        agentId,
-        providerId: 'acp',
-        modelId: agentId,
-        projectDir,
-        permissionMode
-      })
+      try {
+        await this.ensureSessionRuntimeInitialized(agent, sessionId, {
+          agentId,
+          providerId: 'acp',
+          modelId: agentId,
+          projectDir,
+          permissionMode
+        })
+      } catch (error) {
+        await this.cleanupFailedSessionInitialization(agent, sessionId)
+        throw error
+      }
       record = this.sessionManager.get(sessionId)
       if (!record) {
         throw new Error(`Failed to read created ACP draft session: ${sessionId}`)
@@ -1377,9 +1387,26 @@ export class NewAgentPresenter {
         projectDir: normalizedProjectDir,
         error
       })
+      throw error
     }
   }
 
+  private async cleanupFailedSessionInitialization(
+    agent: IAgentImplementation,
+    sessionId: string
+  ): Promise<void> {
+    try {
+      await agent.destroySession(sessionId)
+    } catch (cleanupError) {
+      console.warn(
+        `[NewAgentPresenter] Failed to cleanup session runtime after initialization error ${sessionId}:`,
+        cleanupError
+      )
+    }
+
+    this.sessionManager.delete(sessionId)
+  }
+
   private buildExportConversation(
     session: SessionRecord,
     providerId: string,

src/main/presenter/skillPresenter/skillExecutionService.ts

@@ -181,7 +181,29 @@ export class SkillExecutionService {
       const resolvedWorkdir = await this.resolveConversationWorkdir(conversationId)
       const normalizedWorkdir = resolvedWorkdir?.trim()
       if (normalizedWorkdir) {
-        return path.resolve(normalizedWorkdir)
+        const resolvedPath = path.resolve(normalizedWorkdir)
+        try {
+          const stat = await fs.promises.stat(resolvedPath)
+          if (stat.isDirectory()) {
+            return resolvedPath
+          }
+          logger.warn(
+            '[SkillExecutionService] Conversation workdir is not a directory, falling back to skill root',
+            {
+              conversationId,
+              invalidWorkdir: resolvedPath
+            }
+          )
+        } catch (error) {
+          logger.warn(
+            '[SkillExecutionService] Conversation workdir is invalid, falling back to skill root',
+            {
+              conversationId,
+              invalidWorkdir: resolvedPath,
+              error
+            }
+          )
+        }
       }
     } catch (error) {
       logger.warn('[SkillExecutionService] Failed to resolve conversation workdir', {

test/main/presenter/llmProviderPresenter/acp/acpProcessManager.test.ts

@@ -1,3 +1,4 @@
+import path from 'path'
 import { describe, expect, it, vi } from 'vitest'
 import {
   AcpProcessManager,
@@ -218,7 +219,7 @@ describe('AcpProcessManager config cache fallback', () => {
       expect.objectContaining({
         sessionId: 'missing-session',
         command: 'pwd',
-        cwd: '/tmp/deepchat-acp/sessions'
+        cwd: expect.stringContaining(path.join('deepchat-acp', 'sessions'))
       })
     )
   })

test/main/presenter/llmProviderPresenter/acp/acpTerminalManager.test.ts

@@ -1,4 +1,5 @@
 import fs from 'fs'
+import path from 'path'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import { AcpTerminalManager } from '@/presenter/llmProviderPresenter/acp/acpTerminalManager'
 import { spawn } from 'node-pty'
@@ -14,6 +15,14 @@ vi.mock('electron', () => ({
 }))
 
 describe('AcpTerminalManager', () => {
+  const getShellExpectation = () =>
+    process.platform === 'win32'
+      ? expect.stringMatching(/powershell/i)
+      : expect.stringMatching(/bash/i)
+
+  const getArgsExpectation = (command: string) =>
+    process.platform === 'win32' ? ['-NoLogo', '-Command', command] : ['-c', command]
+
   const createPty = () => ({
     onData: vi.fn(),
     onExit: vi.fn(),
@@ -36,10 +45,10 @@ describe('AcpTerminalManager', () => {
     })
 
     expect(spawn).toHaveBeenCalledWith(
-      '/bin/bash',
-      ['-c', 'pwd'],
+      getShellExpectation(),
+      getArgsExpectation('pwd'),
       expect.objectContaining({
-        cwd: '/tmp/workspace'
+        cwd: expect.stringContaining(path.normalize('/tmp/workspace'))
       })
     )
   })
@@ -52,14 +61,14 @@ describe('AcpTerminalManager', () => {
       command: 'pwd'
     })
 
-    expect(fs.mkdirSync).toHaveBeenCalledWith('/tmp/deepchat-acp/terminals', {
+    expect(fs.mkdirSync).toHaveBeenCalledWith(path.normalize('/tmp/deepchat-acp/terminals'), {
       recursive: true
     })
     expect(spawn).toHaveBeenCalledWith(
-      '/bin/bash',
-      ['-c', 'pwd'],
+      getShellExpectation(),
+      getArgsExpectation('pwd'),
       expect.objectContaining({
-        cwd: '/tmp/deepchat-acp/terminals'
+        cwd: expect.stringContaining(path.normalize('/tmp/deepchat-acp/terminals'))
       })
     )
   })

test/main/presenter/newAgentPresenter/newAgentPresenter.test.ts

@@ -542,6 +542,36 @@ describe('NewAgentPresenter', () => {
         }
       )
     })
+
+    it('aborts ACP session creation when workdir sync fails', async () => {
+      configPresenter.getAcpAgents.mockResolvedValue([
+        { id: 'acp-coder', name: 'ACP Coder', command: 'acp-coder' }
+      ])
+      deepChatAgent.getSessionState.mockResolvedValue({
+        status: 'idle',
+        providerId: 'acp',
+        modelId: 'acp-coder',
+        permissionMode: 'full_access'
+      })
+      llmProviderPresenter.setAcpWorkdir.mockRejectedValueOnce(new Error('sync failed'))
+
+      await expect(
+        presenter.createSession(
+          {
+            agentId: 'acp-coder',
+            message: 'Hello ACP',
+            projectDir: '/tmp/workspace',
+            providerId: 'acp',
+            modelId: 'acp-coder'
+          },
+          1
+        )
+      ).rejects.toThrow('sync failed')
+
+      expect(deepChatAgent.destroySession).toHaveBeenCalledWith('mock-session-id')
+      expect(sqlitePresenter.newSessionsTable.delete).toHaveBeenCalledWith('mock-session-id')
+      expect(deepChatAgent.processMessage).not.toHaveBeenCalled()
+    })
   })
 
   describe('sendMessage', () => {

test/main/presenter/skillPresenter/skillExecutionService.test.ts

@@ -48,6 +48,9 @@ describe('SkillExecutionService', () => {
   beforeEach(() => {
     vi.clearAllMocks()
     vi.spyOn(fs, 'existsSync').mockReturnValue(false)
+    vi.mocked(fs.promises.stat).mockResolvedValue({
+      isDirectory: () => true
+    } as never)
 
     skillPresenter = {
       getActiveSkills: vi.fn().mockResolvedValue(['ocr']),
@@ -138,6 +141,26 @@ describe('SkillExecutionService', () => {
     expect(plan.cwd).toBe('/skills/ocr')
   })
 
+  it('falls back to skill root cwd when the resolved session workdir is not a directory', async () => {
+    vi.mocked(fs.promises.stat).mockResolvedValueOnce({
+      isDirectory: () => false
+    } as never)
+    vi.spyOn(service as never, 'resolveRuntimeCommand' as never).mockResolvedValue({
+      command: 'uv',
+      mode: 'uv'
+    })
+
+    const plan = await (service as never).buildSpawnPlan(
+      {
+        skill: 'ocr',
+        script: 'scripts/run.py'
+      },
+      'conv-1'
+    )
+
+    expect(plan.cwd).toBe('/skills/ocr')
+  })
+
   it('falls back to bundled uv for python auto runtime', async () => {
     vi.spyOn(service as never, 'hasCommand' as never).mockResolvedValue(false)
     vi.spyOn(service as never, 'getBundledRuntimeCommand' as never).mockImplementation(

test/setup.ts

@@ -53,7 +53,8 @@ vi.mock('fs', () => {
       readFile: vi.fn(),
       writeFile: vi.fn(),
       mkdir: vi.fn(),
-      readdir: vi.fn()
+      readdir: vi.fn(),
+      stat: vi.fn()
     }
   }