Skip to content

Commit a97e477

Browse files
authored
fix(agent): honor session workdir defaults (#1380)
* fix(agent): honor session workdir defaults * fix(agent): validate workdir fallback
1 parent 98450f5 commit a97e477

File tree

14 files changed

+526
-25
lines changed

14 files changed

+526
-25
lines changed

docs/specs/skill-runtime-hardening/spec.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ Add runtime-aware skill extensions without changing external `SKILL.md` formats.
1717
- Acceptance:
1818
- Only scripts under `<skillRoot>/scripts/` can be executed.
1919
- The runtime picks system `uv`/`node` first, then falls back to DeepChat bundled runtimes.
20-
- Python scripts run from the skill root and honor `pyproject.toml` via `uv run --project`.
20+
- Skill scripts execute from the current session workdir when available, while Python still honors `pyproject.toml` via `uv run --project <skillRoot>`.
2121

2222
### US-3: Prevent prompt pollution from binary file reads
2323
- As an agent, reading an image or binary file through text-file APIs does not inject raw bytes into prompt context.
@@ -28,7 +28,7 @@ Add runtime-aware skill extensions without changing external `SKILL.md` formats.
2828
### US-4: Guide the model toward stable skill execution
2929
- As an agent, active skill instructions clearly include absolute paths, script inventory, and the preferred execution tool.
3030
- Acceptance:
31-
- Active skill prompt includes `skillRoot`, recommended `base_directory`, runnable scripts, and explicit guardrails against inline `python -c` / `node -e`.
31+
- Active skill prompt includes `skillRoot`, skill root env vars, recommended `base_directory`, runnable scripts, and explicit guardrails against inline `python -c` / `node -e`.
3232

3333
### US-5: Keep process output after process exit
3434
- As an agent, command output is still available when a child process writes a large payload right before exiting.

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,25 @@ export class AcpProcessManager implements AgentProcessManager<AcpProcessHandle,
166166
return handler
167167
}
168168

169+
private resolveTerminalCwd(sessionId: string, requestedCwd?: string | null): string {
170+
const explicitCwd = requestedCwd?.trim()
171+
if (explicitCwd) {
172+
return explicitCwd
173+
}
174+
175+
const sessionWorkdir = this.sessionWorkdirs.get(sessionId)?.trim()
176+
if (sessionWorkdir) {
177+
return sessionWorkdir
178+
}
179+
180+
const fallbackWorkdir = this.getFallbackWorkdir()
181+
const conversationId = this.sessionConversations.get(sessionId)
182+
console.warn(
183+
`[ACP] Missing session workdir for terminal session ${sessionId}${conversationId ? ` (conversation ${conversationId})` : ''}, using fallback workdir: ${fallbackWorkdir}`
184+
)
185+
return fallbackWorkdir
186+
}
187+
169188
/**
170189
* Provide a fallback workspace for sessions that haven't registered a workdir.
171190
* Keeps file access constrained to a temp directory rather than the entire filesystem.
@@ -954,7 +973,10 @@ export class AcpProcessManager implements AgentProcessManager<AcpProcessHandle,
954973
},
955974
// Terminal operations
956975
createTerminal: async (params) => {
957-
return this.terminalManager.createTerminal(params)
976+
return this.terminalManager.createTerminal({
977+
...params,
978+
cwd: this.resolveTerminalCwd(params.sessionId, params.cwd)
979+
})
958980
},
959981
terminalOutput: async (params) => {
960982
return this.terminalManager.terminalOutput(params)

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

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
import fs from 'fs'
2+
import path from 'path'
13
import { spawn } from 'node-pty'
24
import type { IPty } from 'node-pty'
35
import { nanoid } from 'nanoid'
6+
import { app } from 'electron'
47
import { RequestError } from '@agentclientprotocol/sdk'
58
import type * as schema from '@agentclientprotocol/sdk/dist/schema/index.js'
69

@@ -34,6 +37,27 @@ export class AcpTerminalManager {
3437
private readonly terminals = new Map<string, TerminalState>()
3538
private readonly defaultMaxOutputBytes = 1024 * 1024 // 1MB default
3639

40+
private resolveTerminalCwd(cwd?: string | null): string {
41+
const normalized = cwd?.trim()
42+
if (normalized) {
43+
return path.resolve(normalized)
44+
}
45+
46+
const fallbackDir = path.join(app.getPath('temp'), 'deepchat-acp', 'terminals')
47+
try {
48+
fs.mkdirSync(fallbackDir, { recursive: true })
49+
console.warn(`[ACP Terminal] Missing cwd, using fallback directory: ${fallbackDir}`)
50+
return fallbackDir
51+
} catch (error) {
52+
const tempDir = app.getPath('temp')
53+
console.warn(
54+
`[ACP Terminal] Failed to create fallback directory, using temp path instead: ${tempDir}`,
55+
error
56+
)
57+
return tempDir
58+
}
59+
}
60+
3761
/**
3862
* Create a new terminal to execute a command.
3963
*/
@@ -42,6 +66,7 @@ export class AcpTerminalManager {
4266
): Promise<schema.CreateTerminalResponse> {
4367
const id = `term_${nanoid(12)}`
4468
const maxOutputBytes = params.outputByteLimit ?? this.defaultMaxOutputBytes
69+
const cwd = this.resolveTerminalCwd(params.cwd)
4570

4671
let exitResolve!: (status: { exitCode?: number | null; signal?: string | null }) => void
4772
const exitPromise = new Promise<{ exitCode?: number | null; signal?: string | null }>(
@@ -75,7 +100,7 @@ export class AcpTerminalManager {
75100
name: 'xterm-256color',
76101
cols: 120,
77102
rows: 30,
78-
cwd: params.cwd ?? process.cwd(),
103+
cwd,
79104
env
80105
})
81106

src/main/presenter/newAgentPresenter/index.ts

Lines changed: 108 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,12 @@ export class NewAgentPresenter {
150150
if (input.generationSettings) {
151151
initConfig.generationSettings = input.generationSettings
152152
}
153-
await agent.initSession(sessionId, initConfig)
153+
try {
154+
await this.initializeSessionRuntime(agent, sessionId, initConfig)
155+
} catch (error) {
156+
await this.cleanupFailedSessionInitialization(agent, sessionId)
157+
throw error
158+
}
154159
console.log(`[NewAgentPresenter] agent.initSession done`)
155160

156161
// Bind to window and emit activated
@@ -224,13 +229,18 @@ export class NewAgentPresenter {
224229
const sessionId = this.sessionManager.create(agentId, 'New Chat', projectDir, {
225230
isDraft: true
226231
})
227-
await this.ensureSessionRuntimeInitialized(agent, sessionId, {
228-
agentId,
229-
providerId: 'acp',
230-
modelId: agentId,
231-
projectDir,
232-
permissionMode
233-
})
232+
try {
233+
await this.ensureSessionRuntimeInitialized(agent, sessionId, {
234+
agentId,
235+
providerId: 'acp',
236+
modelId: agentId,
237+
projectDir,
238+
permissionMode
239+
})
240+
} catch (error) {
241+
await this.cleanupFailedSessionInitialization(agent, sessionId)
242+
throw error
243+
}
234244
record = this.sessionManager.get(sessionId)
235245
if (!record) {
236246
throw new Error(`Failed to read created ACP draft session: ${sessionId}`)
@@ -280,6 +290,12 @@ export class NewAgentPresenter {
280290
}
281291
}
282292
this.assertAcpSessionHasWorkdir(providerId, session.projectDir ?? null)
293+
await this.syncAcpSessionWorkdir(
294+
providerId,
295+
sessionId,
296+
session.agentId,
297+
session.projectDir ?? null
298+
)
283299
if (agent.queuePendingInput) {
284300
await agent.queuePendingInput(sessionId, normalizedInput)
285301
return
@@ -329,6 +345,12 @@ export class NewAgentPresenter {
329345
}
330346
}
331347
this.assertAcpSessionHasWorkdir(providerId, currentSession.projectDir ?? null)
348+
await this.syncAcpSessionWorkdir(
349+
providerId,
350+
sessionId,
351+
currentSession.agentId,
352+
currentSession.projectDir ?? null
353+
)
332354
return await agent.queuePendingInput(sessionId, normalizedInput)
333355
}
334356

@@ -465,7 +487,7 @@ export class NewAgentPresenter {
465487
)
466488

467489
try {
468-
await agent.initSession(targetSessionId, {
490+
await this.initializeSessionRuntime(agent, targetSessionId, {
469491
agentId: sourceSession.agentId,
470492
providerId: sourceState.providerId,
471493
modelId: sourceState.modelId,
@@ -1296,7 +1318,7 @@ export class NewAgentPresenter {
12961318
): Promise<void> {
12971319
const state = await agent.getSessionState(sessionId)
12981320
if (!state) {
1299-
await agent.initSession(sessionId, config)
1321+
await this.initializeSessionRuntime(agent, sessionId, config)
13001322
return
13011323
}
13021324

@@ -1307,6 +1329,82 @@ export class NewAgentPresenter {
13071329
) {
13081330
await agent.setPermissionMode(sessionId, config.permissionMode)
13091331
}
1332+
1333+
await this.syncAcpSessionWorkdir(
1334+
config.providerId,
1335+
sessionId,
1336+
config.agentId ?? config.modelId,
1337+
config.projectDir
1338+
)
1339+
}
1340+
1341+
private async initializeSessionRuntime(
1342+
agent: IAgentImplementation,
1343+
sessionId: string,
1344+
config: {
1345+
agentId?: string
1346+
providerId: string
1347+
modelId: string
1348+
projectDir?: string | null
1349+
permissionMode: PermissionMode
1350+
generationSettings?: Partial<SessionGenerationSettings>
1351+
}
1352+
): Promise<void> {
1353+
await agent.initSession(sessionId, config)
1354+
await this.syncAcpSessionWorkdir(
1355+
config.providerId,
1356+
sessionId,
1357+
config.agentId ?? config.modelId,
1358+
config.projectDir ?? null
1359+
)
1360+
}
1361+
1362+
private async syncAcpSessionWorkdir(
1363+
providerId: string,
1364+
conversationId: string,
1365+
agentId: string,
1366+
projectDir?: string | null
1367+
): Promise<void> {
1368+
if (providerId !== 'acp') {
1369+
return
1370+
}
1371+
1372+
const normalizedProjectDir = projectDir?.trim()
1373+
if (!normalizedProjectDir) {
1374+
return
1375+
}
1376+
1377+
try {
1378+
await this.llmProviderPresenter.setAcpWorkdir(
1379+
conversationId,
1380+
resolveAcpAgentAlias(agentId),
1381+
normalizedProjectDir
1382+
)
1383+
} catch (error) {
1384+
console.warn('[NewAgentPresenter] Failed to sync ACP workdir for session:', {
1385+
conversationId,
1386+
agentId,
1387+
projectDir: normalizedProjectDir,
1388+
error
1389+
})
1390+
throw error
1391+
}
1392+
}
1393+
1394+
private async cleanupFailedSessionInitialization(
1395+
agent: IAgentImplementation,
1396+
sessionId: string
1397+
): Promise<void> {
1398+
try {
1399+
await agent.destroySession(sessionId)
1400+
} catch (cleanupError) {
1401+
console.warn(
1402+
`[NewAgentPresenter] Failed to cleanup session runtime after initialization error ${sessionId}:`,
1403+
cleanupError
1404+
)
1405+
}
1406+
1407+
this.sessionManager.delete(sessionId)
13101408
}
13111409

13121410
private buildExportConversation(

src/main/presenter/skillPresenter/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,8 @@ export class SkillPresenter implements ISkillPresenter {
382382
const scripts = (await this.listSkillScripts(metadata.name)).filter((script) => script.enabled)
383383
const lines = [
384384
'## DeepChat Runtime Context',
385-
'- Skill root: resolved server-side by `skill_run`.',
385+
'- Skill root: available via `SKILL_ROOT` and `DEEPCHAT_SKILL_ROOT`.',
386+
'- `skill_run` executes from the current session workdir when available.',
386387
'- Recommended base_directory: `<skill_root>`'
387388
]
388389

src/main/presenter/skillPresenter/skillExecutionService.ts

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ export interface SkillRunOptions {
3434
conversationId: string
3535
}
3636

37+
interface SkillExecutionServiceOptions {
38+
resolveConversationWorkdir?: (conversationId: string) => Promise<string | null>
39+
}
40+
3741
interface SkillExecutionResult {
3842
output: string | { status: 'running'; sessionId: string }
3943
rtkApplied: boolean
@@ -66,12 +70,15 @@ function toStringEnv(input: NodeJS.ProcessEnv | Record<string, string>): Record<
6670
export class SkillExecutionService {
6771
private readonly runtimeHelper = RuntimeHelper.getInstance()
6872
private readonly configPresenter?: Pick<IConfigPresenter, 'getSetting'>
73+
private readonly resolveConversationWorkdir?: (conversationId: string) => Promise<string | null>
6974

7075
constructor(
7176
private readonly skillPresenter: ISkillPresenter,
72-
configPresenter: IConfigPresenter
77+
configPresenter: IConfigPresenter,
78+
options: SkillExecutionServiceOptions = {}
7379
) {
7480
this.configPresenter = configPresenter
81+
this.resolveConversationWorkdir = options.resolveConversationWorkdir
7582
this.runtimeHelper.initializeRuntimes()
7683
}
7784

@@ -136,10 +143,13 @@ export class SkillExecutionService {
136143

137144
const extension = await this.skillPresenter.getSkillExtension(input.skill)
138145
const shellEnv = await getShellEnvironment()
146+
const executionCwd = await this.resolveExecutionCwd(conversationId, metadata.skillRoot)
139147
const mergedEnv = {
140148
...toStringEnv(process.env),
141149
...shellEnv,
142-
...extension.env
150+
...extension.env,
151+
SKILL_ROOT: metadata.skillRoot,
152+
DEEPCHAT_SKILL_ROOT: metadata.skillRoot
143153
}
144154

145155
const runtime = await this.resolveRuntimeCommand(
@@ -153,14 +163,65 @@ export class SkillExecutionService {
153163
return {
154164
command: runtime.command,
155165
args,
156-
cwd: metadata.skillRoot,
166+
cwd: executionCwd,
157167
env: mergedEnv,
158168
shellCommand: this.buildShellCommand(runtime.command, args),
159169
outputPrefix: `skillrun_${input.skill.replace(/[^a-zA-Z0-9_-]/g, '_')}`,
160170
spawnMode: 'direct'
161171
}
162172
}
163173

174+
private async resolveExecutionCwd(conversationId: string, skillRoot: string): Promise<string> {
175+
const normalizedSkillRoot = path.resolve(skillRoot)
176+
if (!this.resolveConversationWorkdir) {
177+
return normalizedSkillRoot
178+
}
179+
180+
try {
181+
const resolvedWorkdir = await this.resolveConversationWorkdir(conversationId)
182+
const normalizedWorkdir = resolvedWorkdir?.trim()
183+
if (normalizedWorkdir) {
184+
const resolvedPath = path.resolve(normalizedWorkdir)
185+
try {
186+
const stat = await fs.promises.stat(resolvedPath)
187+
if (stat.isDirectory()) {
188+
return resolvedPath
189+
}
190+
logger.warn(
191+
'[SkillExecutionService] Conversation workdir is not a directory, falling back to skill root',
192+
{
193+
conversationId,
194+
invalidWorkdir: resolvedPath
195+
}
196+
)
197+
} catch (error) {
198+
logger.warn(
199+
'[SkillExecutionService] Conversation workdir is invalid, falling back to skill root',
200+
{
201+
conversationId,
202+
invalidWorkdir: resolvedPath,
203+
error
204+
}
205+
)
206+
}
207+
}
208+
} catch (error) {
209+
logger.warn('[SkillExecutionService] Failed to resolve conversation workdir', {
210+
conversationId,
211+
error
212+
})
213+
}
214+
215+
logger.warn(
216+
'[SkillExecutionService] Missing conversation workdir, falling back to skill root',
217+
{
218+
conversationId,
219+
skillRoot: normalizedSkillRoot
220+
}
221+
)
222+
return normalizedSkillRoot
223+
}
224+
164225
private resolveRequestedScript(
165226
requestedScript: string,
166227
scripts: SkillScriptDescriptor[]

0 commit comments

Comments
 (0)