Microsoft/vscode
1
0
Fork 0
mirror of https://github.com/microsoft/vscode.git synced 2026-05-08 17:19:48 +01:00
Code Issues Packages Projects Releases Wiki Activity

check output before appending (#291658)

Browse Source
This commit is contained in:
Megan Rogge
2026-01-29 13:45:54 -05:00
committed by GitHub
parent 424641585a
commit 9081bdd491
2 changed files with 229 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/vs/workbench/contrib/terminal/browser/chatTerminalCommandMirror.ts
+48 -3
View File
@@ -68,6 +68,28 @@ export function computeMaxBufferColumnWidth(buffer: { readonly length: number; g
return maxWidth;
}
/**
* Checks if two VT strings match around a boundary where we would slice.
* This is an efficient O(1) check that verifies a small window of characters
* before the slice point to detect if the VT sequences have diverged (common on Windows).
*
* @param newVT The new VT text to compare.
* @param oldVT The old VT text to compare against.
* @param slicePoint The point where we would slice. Must be <= both string lengths.
* @param windowSize The number of characters before slicePoint to check (default 50).
* @returns True if the boundary matches, false if VT sequences have diverged.
*/
export function vtBoundaryMatches(newVT: string, oldVT: string, slicePoint: number, windowSize: number = 50): boolean {
const start = Math.max(0, slicePoint - windowSize);
const end = slicePoint;
for (let i = start; i < end; i++) {
if (newVT.charCodeAt(i) !== oldVT.charCodeAt(i)) {
return false;
}
}
return true;
}
export interface IDetachedTerminalCommandMirrorRenderResult {
lineCount?: number;
maxColumnWidth?: number;
@@ -280,7 +302,16 @@ export class DetachedTerminalCommandMirror extends Disposable implements IDetach
}
await new Promise<void>(resolve => {
if (!this._lastVT) {
// Only append if the boundary around the slice point matches; otherwise rewrite.
// This is an efficient constant-time check (checking up to 50 characters) instead of comparing the entire prefix.
// On Windows, VT sequences can differ even for equivalent content, causing corruption
// if we blindly append.
const canAppend = !!this._lastVT && vt.text.length >= this._lastVT.length && this._vtBoundaryMatches(vt.text, this._lastVT.length);
if (!canAppend) {
// Reset the terminal if we had previous content (can't append, need full rewrite)
if (this._lastVT) {
detached.xterm.clearBuffer();
}
if (vt.text) {
detached.xterm.write(vt.text, resolve);
} else {
@@ -505,9 +536,16 @@ export class DetachedTerminalCommandMirror extends Disposable implements IDetach
return;
}
const canAppend = !!this._lastVT && startLine >= previousCursor;
// Only append if: (1) cursor hasn't moved backwards, and (2) boundary around slice point matches.
// This is an efficient O(1) check instead of comparing the entire prefix.
// On Windows, VT sequences can differ even for equivalent content, so we must verify.
const canAppend = !!this._lastVT && startLine >= previousCursor && vt.text.length >= this._lastVT.length && this._vtBoundaryMatches(vt.text, this._lastVT.length);
await new Promise<void>(resolve => {
if (!this._lastVT || !canAppend) {
if (!canAppend) {
// Reset the terminal if we had previous content (can't append, need full rewrite)
if (this._lastVT) {
detachedRaw.clearBuffer();
}
if (vt.text) {
detachedRaw.write(vt.text, resolve);
} else {
@@ -542,6 +580,13 @@ export class DetachedTerminalCommandMirror extends Disposable implements IDetach
private _getAbsoluteCursorY(raw: RawXtermTerminal): number {
return raw.buffer.active.baseY + raw.buffer.active.cursorY;
}
/**
* Checks if the new VT text matches the old VT around the boundary where we would slice.
*/
private _vtBoundaryMatches(newVT: string, slicePoint: number): boolean {
return vtBoundaryMatches(newVT, this._lastVT, slicePoint);
}
}
/**
src/vs/workbench/contrib/terminal/test/browser/chatTerminalCommandMirror.test.ts
+181 -1
View File
@@ -14,7 +14,7 @@ import { TerminalCapabilityStore } from '../../../../../platform/terminal/common
import { XtermTerminal } from '../../browser/xterm/xtermTerminal.js';
import { workbenchInstantiationService } from '../../../../test/browser/workbenchTestServices.js';
import { TestXtermAddonImporter } from './xterm/xtermTestUtils.js';
import { computeMaxBufferColumnWidth } from '../../browser/chatTerminalCommandMirror.js';
import { computeMaxBufferColumnWidth, vtBoundaryMatches } from '../../browser/chatTerminalCommandMirror.js';
const defaultTerminalConfig = {
fontFamily: 'monospace',
@@ -231,6 +231,123 @@ suite('Workbench - ChatTerminalCommandMirror', () => {
// Incremental mirror should match fresh mirror
strictEqual(getBufferText(mirror), getBufferText(freshMirror));
});
test('VT divergence detection prevents corruption (Windows scenario)', async () => {
// This test simulates the Windows issue where VT sequences can differ
// between calls even for equivalent visual content. On Windows, the
// serializer can produce different escape sequences (e.g., different
// line endings or cursor positioning) causing the prefix to diverge.
//
// Without boundary checking, blindly slicing would corrupt output:
// - vt1: "Line1\r\nLine2" (length 13)
// - vt2: "Line1\nLine2\nLine3" (different format, but starts similarly)
// - slice(13) on vt2 would give "ine3" instead of the full new content
const mirror = await createXterm();
// Simulate first VT snapshot
const vt1 = 'Line1\r\nLine2';
await write(mirror, vt1);
strictEqual(getBufferText(mirror), 'Line1\nLine2');
// Simulate divergent VT snapshot (different escape sequences for same content)
// This mimics what can happen on Windows where the VT serializer
// produces different output between calls
const vt2 = 'DifferentPrefix' + 'Line3';
// Use the actual utility function to test boundary checking
const boundaryMatches = vtBoundaryMatches(vt2, vt1, vt1.length);
// Boundary should NOT match because the prefix diverged
strictEqual(boundaryMatches, false, 'Boundary check should detect divergence');
// When boundary doesn't match, the fix does a full reset + rewrite
// instead of corrupting the output by blind slicing
mirror.raw.reset();
await write(mirror, vt2);
// Final content should be the complete new VT, not corrupted
strictEqual(getBufferText(mirror), 'DifferentPrefixLine3');
});
test('boundary check allows append when VT prefix matches', async () => {
const mirror = await createXterm();
// First VT snapshot
const vt1 = 'Line1\r\nLine2\r\n';
await write(mirror, vt1);
// Second VT snapshot that properly extends the first
const vt2 = vt1 + 'Line3\r\n';
// Use the actual utility function to test boundary checking
const boundaryMatches = vtBoundaryMatches(vt2, vt1, vt1.length);
strictEqual(boundaryMatches, true, 'Boundary check should pass when prefix matches');
// Append should work correctly
const appended = vt2.slice(vt1.length);
await write(mirror, appended);
strictEqual(getBufferText(mirror), 'Line1\nLine2\nLine3');
});
test('incremental updates use append path (not full rewrite) in normal operation', async () => {
// This test verifies that in normal operation (VT prefix matches),
// we use the efficient append path rather than full rewrite.
const source = await createXterm();
const marker = source.raw.registerMarker(0)!;
// Build up content incrementally, simulating streaming output
const writes: string[] = [];
// Step 1: Initial content
await write(source, 'output line 1\r\n');
const vt1 = await source.getRangeAsVT(marker, undefined, true) ?? '';
const mirror = await createXterm();
await write(mirror, vt1);
writes.push(vt1);
// Step 2: Add more content - should use append path
await write(source, 'output line 2\r\n');
const vt2 = await source.getRangeAsVT(marker, undefined, true) ?? '';
// Verify VT extends properly (prefix matches)
strictEqual(vt2.startsWith(vt1), true, 'VT2 should start with VT1');
// Append only the new part (this is what the append path does)
const appended2 = vt2.slice(vt1.length);
strictEqual(appended2.length > 0, true, 'Should have new content to append');
strictEqual(appended2.length < vt2.length, true, 'Append should be smaller than full rewrite');
await write(mirror, appended2);
writes.push(appended2);
// Step 3: Add more content - should continue using append path
await write(source, 'output line 3\r\n');
const vt3 = await source.getRangeAsVT(marker, undefined, true) ?? '';
strictEqual(vt3.startsWith(vt2), true, 'VT3 should start with VT2');
const appended3 = vt3.slice(vt2.length);
strictEqual(appended3.length > 0, true, 'Should have new content to append');
strictEqual(appended3.length < vt3.length, true, 'Append should be smaller than full rewrite');
await write(mirror, appended3);
writes.push(appended3);
marker.dispose();
// Verify final content is correct
strictEqual(getBufferText(mirror), 'output line 1\noutput line 2\noutput line 3');
// Verify we used the append path (total bytes written should be roughly
// equal to total VT, not 3x the total due to full rewrites)
const totalWritten = writes.reduce((sum, w) => sum + w.length, 0);
const fullRewriteWouldBe = vt1.length + vt2.length + vt3.length;
strictEqual(totalWritten < fullRewriteWouldBe, true,
`Append path should write less (${totalWritten}) than full rewrites would (${fullRewriteWouldBe})`);
});
});
suite('computeMaxBufferColumnWidth', () => {
@@ -375,4 +492,67 @@ suite('Workbench - ChatTerminalCommandMirror', () => {
strictEqual(computeMaxBufferColumnWidth(buffer, 150), 120);
});
});
suite('vtBoundaryMatches', () => {
test('returns true when strings match at boundary', () => {
const oldVT = 'Line1\r\nLine2\r\n';
const newVT = oldVT + 'Line3\r\n';
strictEqual(vtBoundaryMatches(newVT, oldVT, oldVT.length), true);
});
test('returns false when strings diverge at boundary', () => {
const oldVT = 'Line1\r\nLine2';
const newVT = 'DifferentPrefixLine3';
strictEqual(vtBoundaryMatches(newVT, oldVT, oldVT.length), false);
});
test('returns false when single character differs in window', () => {
const oldVT = 'AAAAAAAAAA';
const newVT = 'AAAAABAAAA' + 'NewContent';
strictEqual(vtBoundaryMatches(newVT, oldVT, oldVT.length), false);
});
test('returns true for empty strings', () => {
strictEqual(vtBoundaryMatches('', '', 0), true);
});
test('returns true when slicePoint is 0', () => {
const oldVT = '';
const newVT = 'SomeContent';
strictEqual(vtBoundaryMatches(newVT, oldVT, 0), true);
});
test('handles strings shorter than window size', () => {
const oldVT = 'Short';
const newVT = 'Short' + 'Added';
strictEqual(vtBoundaryMatches(newVT, oldVT, oldVT.length), true);
});
test('respects custom window size parameter', () => {
// With default window (50), this would match since the diff is at position 70
const prefix = 'A'.repeat(80);
const oldVT = prefix;
const newVT = 'X' + 'A'.repeat(79) + 'NewContent'; // differs at position 0
// With window of 50, only checks chars 30-80, which would match
strictEqual(vtBoundaryMatches(newVT, oldVT, oldVT.length, 50), true);
// With window of 100, would check chars 0-80, which would NOT match
strictEqual(vtBoundaryMatches(newVT, oldVT, oldVT.length, 100), false);
});
test('detects divergence in escape sequences (Windows scenario)', () => {
// Simulates Windows issue where VT escape sequences differ
const oldVT = '\x1b[0m\x1b[1mBold\x1b[0m\r\n';
const newVT = '\x1b[0m\x1b[22mBold\x1b[0m\r\nMore'; // Different escape code for bold
strictEqual(vtBoundaryMatches(newVT, oldVT, oldVT.length), false);
});
test('handles matching escape sequences', () => {
const oldVT = '\x1b[31mRed\x1b[0m\r\n';
const newVT = '\x1b[31mRed\x1b[0m\r\nGreen';
strictEqual(vtBoundaryMatches(newVT, oldVT, oldVT.length), true);
});
});
});