Fix external editor diff drift (#12846)

This commit is contained in:
cornmander
2025-11-10 16:19:32 -05:00
committed by GitHub
parent 8f4b1b582d
commit 37ca643a64
5 changed files with 252 additions and 378 deletions
@@ -235,6 +235,64 @@ describe('modifyWithEditor', () => {
expect(result.updatedDiff).toBe('mock diff content');
});
it('should honor override content values when provided', async () => {
const overrideCurrent = 'override current content';
const overrideProposed = 'override proposed content';
mockModifyContext.getCurrentContent = vi.fn();
mockModifyContext.getProposedContent = vi.fn();
await modifyWithEditor(
mockParams,
mockModifyContext,
'vscode' as EditorType,
abortSignal,
vi.fn(),
{
currentContent: overrideCurrent,
proposedContent: overrideProposed,
},
);
expect(mockModifyContext.getCurrentContent).not.toHaveBeenCalled();
expect(mockModifyContext.getProposedContent).not.toHaveBeenCalled();
expect(mockCreatePatch).toHaveBeenCalledWith(
path.basename(mockParams.filePath),
overrideCurrent,
modifiedContent,
'Current',
'Proposed',
expect.any(Object),
);
});
it('should treat null override as explicit empty content', async () => {
mockModifyContext.getCurrentContent = vi.fn();
mockModifyContext.getProposedContent = vi.fn();
await modifyWithEditor(
mockParams,
mockModifyContext,
'vscode' as EditorType,
abortSignal,
vi.fn(),
{
currentContent: null,
proposedContent: 'override proposed content',
},
);
expect(mockModifyContext.getCurrentContent).not.toHaveBeenCalled();
expect(mockModifyContext.getProposedContent).not.toHaveBeenCalled();
expect(mockCreatePatch).toHaveBeenCalledWith(
path.basename(mockParams.filePath),
'',
modifiedContent,
'Current',
'Proposed',
expect.any(Object),
);
});
it('should clean up temp files even if editor fails', async () => {
const editorError = new Error('Editor failed to open');
mockOpenDiff.mockRejectedValue(editorError);
+20 -5
View File
@@ -46,6 +46,11 @@ export interface ModifyResult<ToolParams> {
updatedDiff: string;
}
export interface ModifyContentOverrides {
currentContent?: string | null;
proposedContent?: string;
}
/**
* Type guard to check if a declarative tool is modifiable.
*/
@@ -172,14 +177,24 @@ export async function modifyWithEditor<ToolParams>(
editorType: EditorType,
_abortSignal: AbortSignal,
onEditorClose: () => void,
overrides?: ModifyContentOverrides,
): Promise<ModifyResult<ToolParams>> {
const currentContent = await modifyContext.getCurrentContent(originalParams);
const proposedContent =
await modifyContext.getProposedContent(originalParams);
const hasCurrentOverride =
overrides !== undefined && 'currentContent' in overrides;
const hasProposedOverride =
overrides !== undefined && 'proposedContent' in overrides;
const currentContent = hasCurrentOverride
? (overrides!.currentContent ?? '')
: await modifyContext.getCurrentContent(originalParams);
const proposedContent = hasProposedOverride
? (overrides!.proposedContent ?? '')
: await modifyContext.getProposedContent(originalParams);
const { oldPath, newPath, dirPath } = createTempFilesForModify(
currentContent,
proposedContent,
currentContent ?? '',
proposedContent ?? '',
modifyContext.getFilePath(originalParams),
);