Skip to content

Commit f9ebf27

Browse files
committed
Refresh non-destructively on save and harden session lifecycle
1 parent 422db86 commit f9ebf27

4 files changed

Lines changed: 54 additions & 11 deletions

File tree

src/views/solution-outline/commands/merge-command.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ describe('MergeCommand', () => {
8383
mergeSessionCoordinator = {
8484
activate: jest.fn().mockResolvedValue(),
8585
startSession: jest.fn(),
86+
cancelSession: jest.fn(),
8687
onMergeProcessExit: jest.fn().mockResolvedValue(),
8788
};
8889
command = new MergeCommand(commandsProvider, mergeSessionCoordinator);
@@ -361,6 +362,7 @@ describe('MergeCommand', () => {
361362
expect(warningSpy).toHaveBeenCalledWith('Merge exited with code 1. Conflicts may exist.');
362363
expect(mergeSessionCoordinator.startSession).toHaveBeenCalledTimes(1);
363364
expect(mergeSessionCoordinator.onMergeProcessExit).toHaveBeenCalledWith(1);
365+
expect(mergeSessionCoordinator.cancelSession).not.toHaveBeenCalled();
364366
});
365367

366368
it('handles merge errors gracefully', async () => {
@@ -377,6 +379,7 @@ describe('MergeCommand', () => {
377379
await command['runVSCodeMerge'](fileNode);
378380
expect(errorSpy).toHaveBeenCalledWith('Merge operations failed:', expect.any(Error));
379381
expect(showErrorMessageSpy).toHaveBeenCalledWith('Merge operation failed: unexpected');
382+
expect(mergeSessionCoordinator.cancelSession).toHaveBeenCalledTimes(1);
380383
});
381384

382385
it('shows a merge failure message when merge command validation throws', async () => {
@@ -402,6 +405,7 @@ describe('MergeCommand', () => {
402405
await command['runVSCodeMerge'](node);
403406

404407
expect(showErrorMessageSpy).toHaveBeenCalledWith('Merge operation failed: Invalid update file: contains unsupported shell-sensitive characters.');
408+
expect(mergeSessionCoordinator.cancelSession).toHaveBeenCalledTimes(1);
405409
});
406410

407411
it('starts merge session and notifies coordinator when merge exits successfully', async () => {
@@ -435,6 +439,7 @@ describe('MergeCommand', () => {
435439
mergedMTimeBefore: 1000,
436440
});
437441
expect(mergeSessionCoordinator.onMergeProcessExit).toHaveBeenCalledWith(0);
442+
expect(mergeSessionCoordinator.cancelSession).not.toHaveBeenCalled();
438443
});
439444
});
440445
});

src/views/solution-outline/commands/merge-command.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ export class MergeCommand {
108108

109109
// get the modification time of the merged file before merge
110110
const mergedMTimeBefore = fsUtils.getFileModificationTime(merged);
111+
let exitHandled = false;
111112

112113
try {
113114
this.mergeSessionCoordinator.startSession({
@@ -120,6 +121,7 @@ export class MergeCommand {
120121
const command = this.buildMergeCommand(codePath, local, update, base, merged);
121122
const exitCode = await this.doOpen3WayMerge(command);
122123
await this.mergeSessionCoordinator.onMergeProcessExit(exitCode);
124+
exitHandled = true;
123125

124126
if (exitCode !== 0) {
125127
console.warn(`Merge exited with code ${exitCode}. Conflicts may exist.`);
@@ -129,6 +131,10 @@ export class MergeCommand {
129131
console.error('Merge operations failed:', err);
130132
const details = err instanceof Error ? err.message : String(err);
131133
vscode.window.showErrorMessage(`Merge operation failed: ${details}`);
134+
} finally {
135+
if (!exitHandled) {
136+
this.mergeSessionCoordinator.cancelSession();
137+
}
132138
}
133139
}
134140

src/views/solution-outline/commands/merge-session-coordinator.test.ts

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ describe('MergeSessionCoordinator', () => {
5252
testDataHandler.dispose();
5353
});
5454

55-
it('finalizes merge on save and refreshes solution', async () => {
55+
it('refreshes on save without destructive file operations', async () => {
5656
const local = path.join(tmpDir, 'component.c');
5757
const update = path.join(tmpDir, 'component.c.update@1.0.0');
5858
const base = path.join(tmpDir, 'component.c.base@1.0.0');
@@ -69,12 +69,12 @@ describe('MergeSessionCoordinator', () => {
6969
handleDidSaveTextDocument: (document: vscode.TextDocument) => Promise<void>
7070
}).handleDidSaveTextDocument({ uri: { fsPath: merged } } as vscode.TextDocument);
7171

72-
const newBase = path.join(tmpDir, 'component.c.base@1.0.0');
73-
expect(fsUtils.fileExists(`${local}.bak`)).toBeTruthy();
7472
expect(fsUtils.fileExists(local)).toBeTruthy();
75-
expect(fsUtils.readTextFile(local)).toContain('// merged');
76-
expect(fsUtils.fileExists(merged)).toBeFalsy();
77-
expect(fsUtils.fileExists(newBase)).toBeTruthy();
73+
expect(fsUtils.readTextFile(local)).toContain('// local');
74+
expect(fsUtils.fileExists(merged)).toBeTruthy();
75+
expect(fsUtils.fileExists(base)).toBeTruthy();
76+
expect(fsUtils.fileExists(update)).toBeTruthy();
77+
expect(fsUtils.fileExists(`${local}.bak`)).toBeFalsy();
7878

7979
expect(solutionManager.refresh).toHaveBeenCalledTimes(1);
8080
});
@@ -100,6 +100,28 @@ describe('MergeSessionCoordinator', () => {
100100
expect(fsUtils.fileExists(merged)).toBeTruthy();
101101
});
102102

103+
it('does not refresh on save after session is canceled', async () => {
104+
const local = path.join(tmpDir, 'component.c');
105+
const update = path.join(tmpDir, 'component.c.update@1.0.0');
106+
const base = path.join(tmpDir, 'component.c.base@1.0.0');
107+
const merged = `${local}.merged`;
108+
109+
fsUtils.writeTextFile(local, '// local\n');
110+
fsUtils.writeTextFile(update, '// update\n');
111+
fsUtils.writeTextFile(base, '// base\n');
112+
fsUtils.writeTextFile(merged, '// merged\n');
113+
114+
coordinator.startSession({ local, update, base, merged, mergedMTimeBefore: 0 });
115+
coordinator.cancelSession();
116+
117+
await (coordinator as unknown as {
118+
handleDidSaveTextDocument: (document: vscode.TextDocument) => Promise<void>
119+
}).handleDidSaveTextDocument({ uri: { fsPath: merged } } as vscode.TextDocument);
120+
121+
expect(solutionManager.refresh).not.toHaveBeenCalled();
122+
expect(fsUtils.fileExists(merged)).toBeTruthy();
123+
});
124+
103125
it('finalizes on merge process exit when save hook did not run', async () => {
104126
const local = path.join(tmpDir, 'component.c');
105127
const update = path.join(tmpDir, 'component.c.update@1.0.0');

src/views/solution-outline/commands/merge-session-coordinator.ts

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export interface MergeSessionFiles {
3131
export interface MergeSessionCoordinator {
3232
activate(context: Pick<vscode.ExtensionContext, 'subscriptions'>): Promise<void>;
3333
startSession(files: MergeSessionFiles): void;
34+
cancelSession(): void;
3435
onMergeProcessExit(exitCode: number): Promise<void>;
3536
}
3637

@@ -53,11 +54,18 @@ export class MergeSessionCoordinatorImpl implements MergeSessionCoordinator {
5354
this.activeSession = files;
5455
}
5556

57+
public cancelSession(): void {
58+
this.activeSession = undefined;
59+
}
60+
5661
public async onMergeProcessExit(exitCode: number): Promise<void> {
57-
if (exitCode === 0) {
58-
await this.tryFinalize();
62+
try {
63+
if (exitCode === 0) {
64+
await this.tryFinalizeOnExit();
65+
}
66+
} finally {
67+
this.activeSession = undefined;
5968
}
60-
this.activeSession = undefined;
6169
}
6270

6371
private async handleDidSaveTextDocument(document: vscode.TextDocument): Promise<void> {
@@ -67,10 +75,12 @@ export class MergeSessionCoordinatorImpl implements MergeSessionCoordinator {
6775
if (!pathsEqual(document.uri.fsPath, this.activeSession.merged)) {
6876
return;
6977
}
70-
await this.tryFinalize();
78+
// Keep save handling non-destructive while the merge editor is still open.
79+
// The actual file rename/delete operations are performed on merge process exit.
80+
await this.solutionManager.refresh();
7181
}
7282

73-
private async tryFinalize(): Promise<void> {
83+
private async tryFinalizeOnExit(): Promise<void> {
7484
if (!this.activeSession || this.finalizing) {
7585
return;
7686
}

0 commit comments

Comments
 (0)