Skip to content

Commit be09b4c

Browse files
Connor Peetconnor4312
authored andcommitted
Connor4312/msrc/119 (#38)
* mcp: show environement variables in mcp server editor * tools: ensure apply-patch tool does not change files during healing * mcp: show headers in server editor, too --------- Co-authored-by: Connor Peet <connor@peet.io>
1 parent 4b8f6f1 commit be09b4c

4 files changed

Lines changed: 97 additions & 2 deletions

File tree

extensions/copilot/src/extension/tools/node/applyPatchTool.tsx

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,14 @@ export class ApplyPatchTool implements ICopilotTool<IApplyPatchToolParams> {
551551
}
552552

553553
const patchEnd = fetchResult.value.indexOf(PATCH_SUFFIX, patchStart);
554-
return patchEnd === -1 ? fetchResult.value.slice(patchStart) : fetchResult.value.slice(patchStart, patchEnd + PATCH_SUFFIX.length);
554+
const healed = patchEnd === -1 ? fetchResult.value.slice(patchStart) : fetchResult.value.slice(patchStart, patchEnd + PATCH_SUFFIX.length);
555+
556+
// Reject the healed patch if it doesn't affect exactly the same set of files as the original.
557+
if (!healedPatchAffectsSameFiles(patch, healed)) {
558+
return undefined;
559+
}
560+
561+
return healed;
555562
}
556563

557564
private async buildCommitWithHealing(model: vscode.LanguageModelChat | undefined, patch: string, docText: DocText, explanation: string, token: CancellationToken): Promise<{ commit: Commit; healed?: string }> {
@@ -748,6 +755,25 @@ class HealedError extends Error {
748755
}
749756
}
750757

758+
/**
759+
* Returns true if the healed patch affects exactly the same set of files as
760+
* the original patch. Used to reject heals that hallucinated additional files
761+
* or dropped files that the model was originally trying to edit.
762+
*/
763+
export function healedPatchAffectsSameFiles(original: string, healed: string): boolean {
764+
const originalFiles = new Set(identify_files_affected(original));
765+
const healedFiles = new Set(identify_files_affected(healed));
766+
if (originalFiles.size !== healedFiles.size) {
767+
return false;
768+
}
769+
for (const f of originalFiles) {
770+
if (!healedFiles.has(f)) {
771+
return false;
772+
}
773+
}
774+
return true;
775+
}
776+
751777
ToolRegistry.registerTool(ApplyPatchTool);
752778

753779
const applyPatchExample = `*** Begin Patch

extensions/copilot/src/extension/tools/test/node/applyPatch/applyPatch.spec.tsx

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { ChatResponseTextEditPart } from '../../../../../vscodeTypes';
2020
import { ChatVariablesCollection } from '../../../../prompt/common/chatVariablesCollection';
2121
import { WorkingCopyOriginalDocument } from '../../../../prompts/node/inline/workingCopies';
2222
import { createExtensionUnitTestingServices } from '../../../../test/node/services';
23-
import { ApplyPatchTool, IApplyPatchToolParams } from '../../../node/applyPatchTool';
23+
import { ApplyPatchTool, healedPatchAffectsSameFiles, IApplyPatchToolParams } from '../../../node/applyPatchTool';
2424

2525

2626
suite('ApplyPatch Tool', () => {
@@ -87,4 +87,38 @@ suite('ApplyPatch Tool', () => {
8787
await expect(workingCopyDocument.text).toMatchFileSnapshot('fixtures/4302.ts.txt.expected');
8888

8989
});
90+
91+
suite('healedPatchAffectsSameFiles', () => {
92+
const makePatch = (lines: string[]) => ['*** Begin Patch', ...lines, '*** End Patch'].join('\n');
93+
94+
it('accepts a heal touching the same single file', () => {
95+
const original = makePatch(['*** Update File: /a.ts', '@@', '-x', '+y']);
96+
const healed = makePatch(['*** Update File: /a.ts', '@@', '-x', '+y2']);
97+
expect(healedPatchAffectsSameFiles(original, healed)).toBe(true);
98+
});
99+
100+
it('accepts a heal touching the same set of multiple files in any order', () => {
101+
const original = makePatch(['*** Update File: /a.ts', '@@', '-x', '+y', '*** Delete File: /b.ts']);
102+
const healed = makePatch(['*** Delete File: /b.ts', '*** Update File: /a.ts', '@@', '-x', '+z']);
103+
expect(healedPatchAffectsSameFiles(original, healed)).toBe(true);
104+
});
105+
106+
it('rejects a heal that adds a new file', () => {
107+
const original = makePatch(['*** Update File: /a.ts', '@@', '-x', '+y']);
108+
const healed = makePatch(['*** Update File: /a.ts', '@@', '-x', '+y', '*** Add File: /b.ts', '+hello']);
109+
expect(healedPatchAffectsSameFiles(original, healed)).toBe(false);
110+
});
111+
112+
it('rejects a heal that drops a file', () => {
113+
const original = makePatch(['*** Update File: /a.ts', '@@', '-x', '+y', '*** Delete File: /b.ts']);
114+
const healed = makePatch(['*** Update File: /a.ts', '@@', '-x', '+y']);
115+
expect(healedPatchAffectsSameFiles(original, healed)).toBe(false);
116+
});
117+
118+
it('rejects a heal that targets a different file', () => {
119+
const original = makePatch(['*** Update File: /a.ts', '@@', '-x', '+y']);
120+
const healed = makePatch(['*** Update File: /c.ts', '@@', '-x', '+y']);
121+
expect(healedPatchAffectsSameFiles(original, healed)).toBe(false);
122+
});
123+
});
90124
});

src/vs/workbench/contrib/mcp/browser/mcpServerEditor.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,13 +737,44 @@ export class McpServerEditor extends EditorPane {
737737
const argsValue = append(argsSection, $('code.config-value'));
738738
argsValue.textContent = config.args.join(' ');
739739
}
740+
741+
// Environment variables (if present)
742+
if (config.env && Object.keys(config.env).length > 0) {
743+
const envSection = append(container, $('.config-section'));
744+
const envLabel = append(envSection, $('.config-label'));
745+
envLabel.textContent = localize('environment', "Environment:");
746+
const envValue = append(envSection, $('.config-value'));
747+
for (const [key, value] of Object.entries(config.env)) {
748+
append(envValue, $('code.env-entry', undefined, `${key}=${value ?? ''}`));
749+
}
750+
}
751+
752+
// Env file (if present)
753+
if (config.envFile) {
754+
const envFileSection = append(container, $('.config-section'));
755+
const envFileLabel = append(envFileSection, $('.config-label'));
756+
envFileLabel.textContent = localize('envFile', "Environment File:");
757+
const envFileValue = append(envFileSection, $('code.config-value'));
758+
envFileValue.textContent = config.envFile;
759+
}
740760
} else if (config.type === McpServerType.REMOTE) {
741761
// URL
742762
const urlSection = append(container, $('.config-section'));
743763
const urlLabel = append(urlSection, $('.config-label'));
744764
urlLabel.textContent = localize('url', "URL:");
745765
const urlValue = append(urlSection, $('code.config-value'));
746766
urlValue.textContent = config.url;
767+
768+
// Headers (if present)
769+
if (config.headers && Object.keys(config.headers).length > 0) {
770+
const headersSection = append(container, $('.config-section'));
771+
const headersLabel = append(headersSection, $('.config-label'));
772+
headersLabel.textContent = localize('headers', "Headers:");
773+
const headersValue = append(headersSection, $('.config-value'));
774+
for (const [key, value] of Object.entries(config.headers)) {
775+
append(headersValue, $('code.env-entry', undefined, `${key}: ${value ?? ''}`));
776+
}
777+
}
747778
}
748779
}
749780

src/vs/workbench/contrib/mcp/browser/media/mcpServerEditor.css

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@
2525
word-break: break-all;
2626
}
2727

28+
.config-value .env-entry {
29+
display: block;
30+
}
31+
2832
.no-config {
2933
color: var(--vscode-descriptionForeground);
3034
font-style: italic;

0 commit comments

Comments
 (0)