Skip to content

Commit ca0d33d

Browse files
TyriarTyriar
authored
Merge pull request microsoft#256716 from microsoft/tyriar/toolSpecific
Recreate toolSpecificData when needed
2 parents 56271c2 + 25b0272 commit ca0d33d

File tree

2 files changed

+70
-52
lines changed

2 files changed

+70
-52
lines changed

‎src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/runInTerminalTool.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
186186
}
187187

188188
const instance = context.chatSessionId ? this._sessionTerminalAssociations.get(context.chatSessionId)?.instance : undefined;
189-
let toolEditedCommand: string | undefined = await this._rewriteCommandIfNeeded(context, args, instance, shell);
189+
let toolEditedCommand: string | undefined = await this._rewriteCommandIfNeeded(args, instance, shell);
190190
if (toolEditedCommand === args.command) {
191191
toolEditedCommand = undefined;
192192
}
@@ -211,9 +211,30 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
211211
}
212212

213213
const args = invocation.parameters as IRunInTerminalInputParams;
214-
const toolSpecificData = invocation.toolSpecificData as IChatTerminalToolInvocationData | IChatTerminalToolInvocationData2 | undefined;
214+
215+
// Tool specific data is not provided when the invocation is auto-approved. Re-calculate it
216+
// if needed
217+
let toolSpecificData = invocation.toolSpecificData as IChatTerminalToolInvocationData | IChatTerminalToolInvocationData2 | undefined;
215218
if (toolSpecificData === undefined) {
216-
throw new Error('Tool specific data must be provided');
219+
const os = await this._osBackend;
220+
const shell = await this._terminalProfileResolverService.getDefaultShell({
221+
os,
222+
remoteAuthority: this._remoteAgentService.getConnection()?.remoteAuthority
223+
});
224+
const language = os === OperatingSystem.Windows ? 'pwsh' : 'sh';
225+
const instance = invocation.context?.sessionId ? this._sessionTerminalAssociations.get(invocation.context!.sessionId)?.instance : undefined;
226+
let toolEditedCommand: string | undefined = await this._rewriteCommandIfNeeded(args, instance, shell);
227+
if (toolEditedCommand === args.command) {
228+
toolEditedCommand = undefined;
229+
}
230+
toolSpecificData = {
231+
kind: 'terminal2',
232+
commandLine: {
233+
original: args.command,
234+
toolEdited: toolEditedCommand
235+
},
236+
language
237+
};
217238
}
218239

219240
this._logService.debug(`RunInTerminalTool: Invoking with options ${JSON.stringify(args)}`);
@@ -397,7 +418,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl {
397418
}
398419
}
399420

400-
protected async _rewriteCommandIfNeeded(context: IToolInvocationPreparationContext, args: IRunInTerminalInputParams, instance: Pick<ITerminalInstance, 'getCwdResource'> | undefined, shell: string): Promise<string> {
421+
protected async _rewriteCommandIfNeeded(args: IRunInTerminalInputParams, instance: Pick<ITerminalInstance, 'getCwdResource'> | undefined, shell: string): Promise<string> {
401422
const commandLine = args.command;
402423
const os = await this._osBackend;
403424

‎src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/runInTerminalTool.test.ts

Lines changed: 45 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ class TestRunInTerminalTool extends RunInTerminalTool {
2525

2626
get commandLineAutoApprover(): CommandLineAutoApprover { return this._commandLineAutoApprover; }
2727

28-
async rewriteCommandIfNeeded(options: IToolInvocationPreparationContext, args: IRunInTerminalInputParams, instance: Pick<ITerminalInstance, 'getCwdResource'> | undefined, shell: string): Promise<string> {
29-
return this._rewriteCommandIfNeeded(options, args, instance, shell);
28+
async rewriteCommandIfNeeded(args: IRunInTerminalInputParams, instance: Pick<ITerminalInstance, 'getCwdResource'> | undefined, shell: string): Promise<string> {
29+
return this._rewriteCommandIfNeeded(args, instance, shell);
3030
}
3131

3232
setBackendOs(os: OperatingSystem) {
@@ -222,15 +222,12 @@ suite('RunInTerminalTool', () => {
222222
});
223223

224224
suite('command re-writing', () => {
225-
function createRewriteOptions(command: string, chatSessionId?: string): IToolInvocationPreparationContext {
225+
function createRewriteParams(command: string, chatSessionId?: string): IRunInTerminalInputParams {
226226
return {
227-
parameters: {
228-
command,
229-
explanation: 'Test command',
230-
isBackground: false
231-
} as IRunInTerminalInputParams,
232-
chatSessionId
233-
} as IToolInvocationPreparationContext;
227+
command,
228+
explanation: 'Test command',
229+
isBackground: false
230+
};
234231
}
235232

236233
suite('cd <cwd> && <suffix> -> <suffix>', () => {
@@ -240,52 +237,52 @@ suite('RunInTerminalTool', () => {
240237
});
241238

242239
test('should return original command when no cd prefix pattern matches', async () => {
243-
const options = createRewriteOptions('echo hello world');
244-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, undefined, 'pwsh');
240+
const parameters = createRewriteParams('echo hello world');
241+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, undefined, 'pwsh');
245242

246243
strictEqual(result, 'echo hello world');
247244
});
248245

249246
test('should return original command when cd pattern does not have suffix', async () => {
250247
runInTerminalTool.setBackendOs(OperatingSystem.Linux);
251-
const options = createRewriteOptions('cd /some/path');
252-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, undefined, 'pwsh');
248+
const parameters = createRewriteParams('cd /some/path');
249+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, undefined, 'pwsh');
253250

254251
strictEqual(result, 'cd /some/path');
255252
});
256253

257254
test('should rewrite command with ; separator when directory matches cwd', async () => {
258255
const testDir = '/test/workspace';
259-
const options = createRewriteOptions(`cd ${testDir}; npm test`, 'session-1');
256+
const parameters = createRewriteParams(`cd ${testDir}; npm test`, 'session-1');
260257
workspaceService.setWorkspace({
261258
folders: [{ uri: { fsPath: testDir } }]
262259
} as any);
263260

264-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, undefined, 'pwsh');
261+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, undefined, 'pwsh');
265262

266263
strictEqual(result, 'npm test');
267264
});
268265

269266
test('should rewrite command with && separator when directory matches cwd', async () => {
270267
const testDir = '/test/workspace';
271-
const options = createRewriteOptions(`cd ${testDir} && npm install`, 'session-1');
268+
const parameters = createRewriteParams(`cd ${testDir} && npm install`, 'session-1');
272269
workspaceService.setWorkspace({
273270
folders: [{ uri: { fsPath: testDir } }]
274271
} as any);
275272

276-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, undefined, 'bash');
273+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, undefined, 'bash');
277274

278275
strictEqual(result, 'npm install');
279276
});
280277

281278
test('should rewrite command when the path is wrapped in double quotes', async () => {
282279
const testDir = '/test/workspace';
283-
const options = createRewriteOptions(`cd "${testDir}" && npm install`, 'session-1');
280+
const parameters = createRewriteParams(`cd "${testDir}" && npm install`, 'session-1');
284281
workspaceService.setWorkspace({
285282
folders: [{ uri: { fsPath: testDir } }]
286283
} as any);
287284

288-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, undefined, 'bash');
285+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, undefined, 'bash');
289286

290287
strictEqual(result, 'npm install');
291288
});
@@ -294,76 +291,76 @@ suite('RunInTerminalTool', () => {
294291
const testDir = '/test/workspace';
295292
const differentDir = '/different/path';
296293
const command = `cd ${differentDir} && npm install`;
297-
const options = createRewriteOptions(command, 'session-1');
294+
const parameters = createRewriteParams(command, 'session-1');
298295
workspaceService.setWorkspace({
299296
folders: [{ uri: { fsPath: testDir } }]
300297
} as any);
301298

302-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, undefined, 'bash');
299+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, undefined, 'bash');
303300

304301
strictEqual(result, command);
305302
});
306303

307304
test('should return original command when no workspace folders available', async () => {
308305
const command = 'cd /some/path && npm install';
309-
const options = createRewriteOptions(command, 'session-1');
306+
const parameters = createRewriteParams(command, 'session-1');
310307
workspaceService.setWorkspace({
311308
folders: []
312309
} as any);
313310

314-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, undefined, 'bash');
311+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, undefined, 'bash');
315312

316313
strictEqual(result, command);
317314
});
318315

319316
test('should return original command when multiple workspace folders available', async () => {
320317
const command = 'cd /some/path && npm install';
321-
const options = createRewriteOptions(command, 'session-1');
318+
const parameters = createRewriteParams(command, 'session-1');
322319
workspaceService.setWorkspace({
323320
folders: [
324321
{ uri: { fsPath: '/workspace1' } },
325322
{ uri: { fsPath: '/workspace2' } }
326323
]
327324
} as any);
328325

329-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, undefined, 'bash');
326+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, undefined, 'bash');
330327

331328
strictEqual(result, command);
332329
});
333330

334331
test('should handle commands with complex suffixes', async () => {
335332
const testDir = '/test/workspace';
336333
const command = `cd ${testDir} && npm install && npm test && echo "done"`;
337-
const options = createRewriteOptions(command, 'session-1');
334+
const parameters = createRewriteParams(command, 'session-1');
338335
workspaceService.setWorkspace({
339336
folders: [{ uri: { fsPath: testDir } }]
340337
} as any);
341338

342-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, undefined, 'bash');
339+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, undefined, 'bash');
343340

344341
strictEqual(result, 'npm install && npm test && echo "done"');
345342
});
346343

347344
test('should handle session without chatSessionId', async () => {
348345
const command = 'cd /some/path && npm install';
349-
const options = createRewriteOptions(command);
346+
const parameters = createRewriteParams(command);
350347
workspaceService.setWorkspace({
351348
folders: [{ uri: { fsPath: '/some/path' } }]
352349
} as any);
353350

354-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, undefined, 'bash');
351+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, undefined, 'bash');
355352

356353
strictEqual(result, 'npm install');
357354
});
358355

359356
test('should ignore any trailing forward slash', async () => {
360357
const testDir = '/test/workspace';
361-
const options = createRewriteOptions(`cd ${testDir}/ && npm install`, 'session-1');
358+
const parameters = createRewriteParams(`cd ${testDir}/ && npm install`, 'session-1');
362359
workspaceService.setWorkspace({
363360
folders: [{ uri: { fsPath: testDir } }]
364361
} as any);
365362

366-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, undefined, 'bash');
363+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, undefined, 'bash');
367364

368365
strictEqual(result, 'npm install');
369366
});
@@ -376,12 +373,12 @@ suite('RunInTerminalTool', () => {
376373

377374
test('should ignore any trailing back slash', async () => {
378375
const testDir = 'c:\\test\\workspace';
379-
const options = createRewriteOptions(`cd ${testDir}\\ && npm install`, 'session-1');
376+
const parameters = createRewriteParams(`cd ${testDir}\\ && npm install`, 'session-1');
380377
workspaceService.setWorkspace({
381378
folders: [{ uri: { fsPath: testDir } }]
382379
} as any);
383380

384-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, undefined, 'cmd');
381+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, undefined, 'cmd');
385382

386383
strictEqual(result, 'npm install');
387384
});
@@ -390,14 +387,14 @@ suite('RunInTerminalTool', () => {
390387
const instanceDir = 'C:\\instance\\workspace';
391388
const workspaceDir = 'C:\\workspace\\service';
392389
const command = `cd ${instanceDir} && npm test`;
393-
const options = createRewriteOptions(command, 'session-1');
390+
const parameters = createRewriteParams(command, 'session-1');
394391

395392
workspaceService.setWorkspace({
396393
folders: [{ uri: { fsPath: workspaceDir } }]
397394
} as any);
398395
const instance = createInstanceWithCwd({ fsPath: instanceDir } as any);
399396

400-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, instance, 'cmd');
397+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, instance, 'cmd');
401398

402399
strictEqual(result, 'npm test');
403400
});
@@ -406,14 +403,14 @@ suite('RunInTerminalTool', () => {
406403
const instanceDir = 'C:\\instance\\workspace';
407404
const workspaceDir = 'C:\\workspace\\service';
408405
const command = `cd ${instanceDir}; npm test`;
409-
const options = createRewriteOptions(command, 'session-1');
406+
const parameters = createRewriteParams(command, 'session-1');
410407

411408
workspaceService.setWorkspace({
412409
folders: [{ uri: { fsPath: workspaceDir } }]
413410
} as any);
414411
const instance = createInstanceWithCwd({ fsPath: instanceDir } as any);
415412

416-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, instance, 'pwsh');
413+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, instance, 'pwsh');
417414

418415
strictEqual(result, 'npm test');
419416
});
@@ -423,14 +420,14 @@ suite('RunInTerminalTool', () => {
423420
const cdDir = 'C:\\different\\path';
424421
const workspaceDir = 'C:\\workspace\\service';
425422
const command = `cd ${cdDir} && npm test`;
426-
const options = createRewriteOptions(command, 'session-1');
423+
const parameters = createRewriteParams(command, 'session-1');
427424

428425
workspaceService.setWorkspace({
429426
folders: [{ uri: { fsPath: workspaceDir } }]
430427
} as any);
431428
const instance = createInstanceWithCwd({ fsPath: instanceDir } as any);
432429

433-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, instance, 'cmd');
430+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, instance, 'cmd');
434431

435432
// Should not rewrite since instance cwd doesn't match cd path
436433
strictEqual(result, command);
@@ -439,29 +436,29 @@ suite('RunInTerminalTool', () => {
439436
test('should fallback to workspace service when instance getCwdResource returns undefined', async () => {
440437
const workspaceDir = 'C:\\workspace\\service';
441438
const command = `cd ${workspaceDir} && npm test`;
442-
const options = createRewriteOptions(command, 'session-1');
439+
const parameters = createRewriteParams(command, 'session-1');
443440

444441
workspaceService.setWorkspace({
445442
folders: [{ uri: { fsPath: workspaceDir } }]
446443
} as any);
447444
const instance = createInstanceWithCwd(undefined);
448445

449-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, instance, 'cmd');
446+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, instance, 'cmd');
450447

451448
strictEqual(result, 'npm test');
452449
});
453450

454451
test('should prioritize instance cwd over workspace service even when both match cd path', async () => {
455452
const sharedDir = 'C:\\shared\\workspace';
456453
const command = `cd ${sharedDir} && npm build`;
457-
const options = createRewriteOptions(command, 'session-1');
454+
const parameters = createRewriteParams(command, 'session-1');
458455

459456
workspaceService.setWorkspace({
460457
folders: [{ uri: { fsPath: sharedDir } }]
461458
} as any);
462459
const instance = createInstanceWithCwd({ fsPath: sharedDir } as any);
463460

464-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, instance, 'cmd');
461+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, instance, 'cmd');
465462

466463
strictEqual(result, 'npm build');
467464
});
@@ -470,26 +467,26 @@ suite('RunInTerminalTool', () => {
470467
const instanceDir = 'C:\\Instance\\Workspace';
471468
const cdDir = 'c:\\instance\\workspace'; // Different case
472469
const command = `cd ${cdDir} && npm test`;
473-
const options = createRewriteOptions(command, 'session-1');
470+
const parameters = createRewriteParams(command, 'session-1');
474471

475472
const instance = createInstanceWithCwd({ fsPath: instanceDir } as any);
476473

477-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, instance, 'cmd');
474+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, instance, 'cmd');
478475

479476
strictEqual(result, 'npm test');
480477
});
481478

482479
test('should handle quoted paths with instance priority', async () => {
483480
const instanceDir = 'C:\\instance\\workspace';
484481
const command = 'cd "C:\\instance\\workspace" && npm test';
485-
const options = createRewriteOptions(command, 'session-1');
482+
const parameters = createRewriteParams(command, 'session-1');
486483

487484
workspaceService.setWorkspace({
488485
folders: [{ uri: { fsPath: 'C:\\different\\workspace' } }]
489486
} as any);
490487
const instance = createInstanceWithCwd({ fsPath: instanceDir } as any);
491488

492-
const result = await runInTerminalTool.rewriteCommandIfNeeded(options, options.parameters as IRunInTerminalInputParams, instance, 'cmd');
489+
const result = await runInTerminalTool.rewriteCommandIfNeeded(parameters, instance, 'cmd');
493490

494491
strictEqual(result, 'npm test');
495492
});

0 commit comments

Comments
 (0)