plan widget: show outdated pill if edited (#328864)

* plan widget: show outdated pill if edited

* address comments

src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatPlanReviewPart.ts

@@ -5,6 +5,7 @@
 
 import * as dom from '../../../../../../base/browser/dom.js';
 import { StandardKeyboardEvent } from '../../../../../../base/browser/keyboardEvent.js';
+import { status } from '../../../../../../base/browser/ui/aria/aria.js';
 import { Button, ButtonWithDropdown, IButton } from '../../../../../../base/browser/ui/button/button.js';
 import { DomScrollableElement } from '../../../../../../base/browser/ui/scrollbar/scrollableElement.js';
 import { Action, Separator } from '../../../../../../base/common/actions.js';
@@ -19,9 +20,12 @@ import { basename, isEqual } from '../../../../../../base/common/resources.js';
 import { ThemeIcon } from '../../../../../../base/common/themables.js';
 import { URI } from '../../../../../../base/common/uri.js';
 import { generateUuid } from '../../../../../../base/common/uuid.js';
+import { ITextModel } from '../../../../../../editor/common/model.js';
+import { IModelService } from '../../../../../../editor/common/services/model.js';
 import { localize } from '../../../../../../nls.js';
 import { IContextMenuService } from '../../../../../../platform/contextview/browser/contextView.js';
 import { IDialogService } from '../../../../../../platform/dialogs/common/dialogs.js';
+import { FileChangeType, IFileService } from '../../../../../../platform/files/common/files.js';
 import { IHoverService } from '../../../../../../platform/hover/browser/hover.js';
 import { IMarkdownRendererService } from '../../../../../../platform/markdown/browser/markdownRenderer.js';
 import { defaultButtonStyles } from '../../../../../../platform/theme/browser/defaultStyles.js';
@@ -52,8 +56,10 @@ export class ChatPlanReviewPart extends Disposable implements IChatContentPart {
 	private _submitButton: Button | undefined;
 	private _renderedSubmitInlineCount = -1;
 	private readonly _messageContentDisposables = this._register(new MutableDisposable<DisposableStore>());
+	private readonly _planChangeListeners = this._register(new DisposableStore());
 
 	private readonly _titleActionsEl: HTMLElement;
+	private readonly _outdatedBadgeEl: HTMLElement;
 	private readonly _inlineActionsEl: HTMLElement;
 	private readonly _footerButtonsEl: HTMLElement;
 	private readonly _messageEl: HTMLElement;
@@ -86,6 +92,8 @@ export class ChatPlanReviewPart extends Disposable implements IChatContentPart {
 		@IPlanReviewFeedbackService private readonly _planReviewFeedbackService: IPlanReviewFeedbackService,
 		@IAgentEditorCommentsBridge private readonly _agentEditorCommentsBridge: IAgentEditorCommentsBridge,
 		@ITextFileService private readonly _textFileService: ITextFileService,
+		@IModelService private readonly _modelService: IModelService,
+		@IFileService private readonly _fileService: IFileService,
 	) {
 		super();
 
@@ -137,7 +145,10 @@ export class ChatPlanReviewPart extends Disposable implements IChatContentPart {
 		const elements = dom.h('.chat-confirmation-widget-container.chat-plan-review-container@container', [
 			dom.h('.chat-confirmation-widget2.chat-plan-review@root', [
 				dom.h('.chat-confirmation-widget-title.chat-plan-review-title@title', [
-					dom.h('.chat-plan-review-title-label@titleLabel'),
+					dom.h('.chat-plan-review-title-content', [
+						dom.h('.chat-plan-review-title-label@titleLabel'),
+						dom.h('span.chat-plan-review-outdated@outdatedBadge'),
+					]),
 					dom.h('.chat-plan-review-inline-actions@inlineActions'),
 					dom.h('.chat-plan-review-title-actions@titleActions'),
 				]),
@@ -155,6 +166,7 @@ export class ChatPlanReviewPart extends Disposable implements IChatContentPart {
 		this.domNode.setAttribute('aria-label', localize('chat.planReview.ariaLabel', 'Plan review: {0}', review.title));
 
 		this._titleActionsEl = elements.titleActions;
+		this._outdatedBadgeEl = elements.outdatedBadge;
 		this._inlineActionsEl = elements.inlineActions;
 		this._footerButtonsEl = elements.footerButtons;
 		this._messageEl = elements.message;
@@ -162,6 +174,12 @@ export class ChatPlanReviewPart extends Disposable implements IChatContentPart {
 		// Title label + hover for truncated titles.
 		elements.titleLabel.textContent = review.title;
 		this._register(this._hoverService.setupDelayedHover(elements.titleLabel, { content: review.title }));
+		this._outdatedBadgeEl.textContent = localize('chat.planReview.outdated', 'Outdated');
+		this._outdatedBadgeEl.setAttribute('aria-label', localize('chat.planReview.outdatedAriaLabel', 'Plan summary is outdated'));
+		if (!review.isOutdated) {
+			dom.hide(this._outdatedBadgeEl);
+		}
+		this.watchPlanChanges();
 
 		// Review button — opens the plan file and enters feedback mode.
 		if (review.planUri) {
@@ -241,6 +259,43 @@ export class ChatPlanReviewPart extends Disposable implements IChatContentPart {
 		}
 	}
 
+	private watchPlanChanges(): void {
+		if (!this.review.planUri || this.review.isOutdated) {
+			return;
+		}
+
+		const planUri = URI.revive(this.review.planUri);
+		const modelListener = this._planChangeListeners.add(new MutableDisposable());
+		const watchModel = (model: ITextModel) => {
+			if (isEqual(model.uri, planUri)) {
+				modelListener.value = model.onDidChangeContent(() => this.markOutdated());
+			}
+		};
+
+		const model = this._modelService.getModel(planUri);
+		if (model) {
+			watchModel(model);
+		}
+		this._planChangeListeners.add(this._modelService.onModelAdded(watchModel));
+		const watcher = this._planChangeListeners.add(this._fileService.createWatcher(planUri, { recursive: false, excludes: [] }));
+		this._planChangeListeners.add(watcher.onDidChange(event => {
+			if (event.contains(planUri, FileChangeType.DELETED) || (!this._modelService.getModel(planUri) && event.contains(planUri, FileChangeType.ADDED, FileChangeType.UPDATED))) {
+				this.markOutdated();
+			}
+		}));
+	}
+
+	private markOutdated(): void {
+		if (this.review.isOutdated) {
+			return;
+		}
+
+		this.review.isOutdated = true;
+		dom.show(this._outdatedBadgeEl);
+		this._planChangeListeners.clear();
+		status(localize('chat.planReview.outdatedAnnouncement', 'Plan summary is outdated'));
+	}
+
 	hasSameContent(other: IChatRendererContent, _followingContent: IChatRendererContent[], _element: ChatTreeItem): boolean {
 		if (other.kind !== 'planReview') {
 			return false;

src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatPlanReview.css

@@ -74,8 +74,16 @@
 	flex-shrink: 0;
 }
 
-.interactive-session .chat-plan-review-container .chat-plan-review-title-label {
+.interactive-session .chat-plan-review-container .chat-plan-review-title-content {
 	flex: 1;
+	min-width: 0;
+	display: flex;
+	align-items: center;
+	gap: var(--vscode-spacing-size60);
+	overflow: hidden;
+}
+
+.interactive-session .chat-plan-review-container .chat-plan-review-title-label {
 	min-width: 0;
 	font-weight: var(--vscode-agents-fontWeight-semiBold);
 	font-size: var(--vscode-chat-font-size-body-s);
@@ -84,6 +92,17 @@
 	text-overflow: ellipsis;
 }
 
+.interactive-session .chat-plan-review-container .chat-plan-review-outdated {
+	flex-shrink: 0;
+	padding: 0 var(--vscode-spacing-size60);
+	border: var(--vscode-strokeThickness) solid var(--vscode-chat-requestBorder);
+	border-radius: var(--vscode-cornerRadius-circle);
+	color: var(--vscode-descriptionForeground);
+	font-size: var(--vscode-fontSize-label3);
+	font-weight: var(--vscode-fontWeight-regular);
+	line-height: 14px;
+}
+
 .interactive-session .chat-plan-review-container .chat-plan-review-title-actions {
 	display: flex;
 	align-items: center;

src/vs/workbench/contrib/chat/common/chatService/chatService.ts

@@ -1410,6 +1410,8 @@ export interface IChatPlanReview {
 	data?: IChatPlanReviewResult;
 	/** Whether the widget has been responded to. */
 	isUsed?: boolean;
+	/** Whether the backing plan file changed after this summary was generated. */
+	isOutdated?: boolean;
 	/** Source attribution. */
 	source?: ToolDataSource;
 }

src/vs/workbench/contrib/chat/common/model/chatProgressTypes/chatPlanReviewData.ts

@@ -33,6 +33,7 @@ export class ChatPlanReviewData implements IChatPlanReview {
 		public data?: IChatPlanReviewResult,
 		public isUsed?: boolean,
 		public source?: ToolDataSource,
+		public isOutdated?: boolean,
 	) { }
 
 	/** Dismiss without a user choice (e.g. the response was cancelled). */
@@ -58,6 +59,7 @@ export class ChatPlanReviewData implements IChatPlanReview {
 			resolveId: this.resolveId,
 			data: this.data,
 			isUsed: this.isUsed,
+			isOutdated: this.isOutdated,
 			source: this.source,
 		};
 	}

src/vs/workbench/contrib/chat/common/model/chatSessionOperationLog.ts

@@ -65,6 +65,7 @@ const responsePartSchema = Adapt.v<PersistedResponsePart, SerializedChatResponse
 				case 'multiDiffData':
 				case 'mcpServersStarting':
 				case 'thinking':
+				case 'planReview':
 					return objectsEqual(a, b);
 
 				// Static types that won't change after being pushed can use strict equality.
@@ -81,7 +82,6 @@ const responsePartSchema = Adapt.v<PersistedResponsePart, SerializedChatResponse
 				case 'systemNotification':
 				case 'pullRequest':
 				case 'questionCarousel':
-				case 'planReview':
 				case 'undoStop':
 				case 'warning':
 				case 'info':

src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatPlanReviewPart.test.ts

@@ -7,8 +7,10 @@ import assert from 'assert';
 import { mainWindow } from '../../../../../../../base/browser/window.js';
 import { URI } from '../../../../../../../base/common/uri.js';
 import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../../base/test/common/utils.js';
+import { IModelService } from '../../../../../../../editor/common/services/model.js';
 import { IDialogService } from '../../../../../../../platform/dialogs/common/dialogs.js';
 import { TestDialogService } from '../../../../../../../platform/dialogs/test/common/testDialogService.js';
+import { FileChangesEvent, FileChangeType, IFileService } from '../../../../../../../platform/files/common/files.js';
 import { workbenchInstantiationService } from '../../../../../../test/browser/workbenchTestServices.js';
 import { IPlanReviewFeedbackService, PlanReviewFeedbackService } from '../../../../browser/planReviewFeedback/planReviewFeedbackService.js';
 import { ChatPlanReviewPart, IChatPlanReviewPartOptions } from '../../../../browser/widget/chatContentParts/chatPlanReviewPart.js';
@@ -83,7 +85,9 @@ suite('ChatPlanReviewPart', () => {
 	let lastFeedbackService: IPlanReviewFeedbackService | undefined;
 	let lastEditorService: IEditorService | undefined;
 	let lastTextFileService: ITextFileService | undefined;
+	let lastModelService: IModelService | undefined;
 	let lastCommentsBridge: AgentEditorCommentsBridge | undefined;
+	let fileChangesEmitter: Emitter<FileChangesEvent> | undefined;
 
 	function createWidget(review: IChatPlanReview, dialogService?: TestDialogService, onSubmit?: () => void): ChatPlanReviewPart {
 		const instantiationService = workbenchInstantiationService(undefined, store);
@@ -95,7 +99,14 @@ suite('ChatPlanReviewPart', () => {
 		lastFeedbackService = feedbackService;
 		lastEditorService = instantiationService.get(IEditorService);
 		lastTextFileService = instantiationService.get(ITextFileService);
+		lastModelService = instantiationService.get(IModelService);
 		lastCommentsBridge = commentsBridge;
+		if (fileChangesEmitter) {
+			sinon.stub(instantiationService.get(IFileService), 'createWatcher').returns({
+				onDidChange: fileChangesEmitter.event,
+				dispose: () => { },
+			});
+		}
 		if (dialogService) {
 			instantiationService.stub(IDialogService, dialogService);
 		}
@@ -120,7 +131,9 @@ suite('ChatPlanReviewPart', () => {
 		lastFeedbackService = undefined;
 		lastEditorService = undefined;
 		lastTextFileService = undefined;
+		lastModelService = undefined;
 		lastCommentsBridge = undefined;
+		fileChangesEmitter = undefined;
 		sinon.restore();
 	});
 
@@ -141,6 +154,80 @@ suite('ChatPlanReviewPart', () => {
 			assert.strictEqual(label?.textContent, 'My Plan Title');
 		});
 
+		test('displays the outdated pill only for outdated summaries', () => {
+			createWidget(createMockReviewWithPlan({ isOutdated: true }));
+
+			const badge = widget.domNode.querySelector<HTMLElement>('.chat-plan-review-outdated');
+			assert.deepStrictEqual({
+				text: badge?.textContent,
+				display: badge?.style.display,
+				ariaLabel: badge?.getAttribute('aria-label'),
+			}, {
+				text: 'Outdated',
+				display: '',
+				ariaLabel: 'Plan summary is outdated',
+			});
+		});
+
+		test('hides the outdated pill for current summaries', () => {
+			createWidget(createMockReviewWithPlan());
+
+			assert.strictEqual(widget.domNode.querySelector<HTMLElement>('.chat-plan-review-outdated')?.style.display, 'none');
+		});
+
+		test('marks the summary outdated when the plan model changes', () => {
+			const planUri = URI.parse('file:///outdated-plan.md');
+			const review = new ChatPlanReviewData(
+				'Plan summary',
+				'Generated summary',
+				[{ label: 'Go', default: true }],
+				true,
+				planUri.toJSON(),
+			);
+			createWidget(review);
+			const model = lastModelService!.createModel('# Original plan', null, planUri);
+
+			try {
+				model.setValue('# Edited plan');
+
+				assert.deepStrictEqual({
+					isOutdated: review.isOutdated,
+					persistedIsOutdated: review.toJSON().isOutdated,
+					badgeDisplay: widget.domNode.querySelector<HTMLElement>('.chat-plan-review-outdated')?.style.display,
+					summary: review.content,
+				}, {
+					isOutdated: true,
+					persistedIsOutdated: true,
+					badgeDisplay: '',
+					summary: 'Generated summary',
+				});
+			} finally {
+				model.dispose();
+			}
+		});
+
+		test('marks the summary outdated when an open plan is deleted', () => {
+			const planUri = URI.parse('file:///deleted-plan.md');
+			const review = new ChatPlanReviewData(
+				'Plan summary',
+				'Generated summary',
+				[{ label: 'Go', default: true }],
+				true,
+				planUri.toJSON(),
+			);
+			fileChangesEmitter = store.add(new Emitter<FileChangesEvent>());
+			createWidget(review);
+			const model = lastModelService!.createModel('# Original plan', null, planUri);
+
+			try {
+				fileChangesEmitter.fire(new FileChangesEvent([{ resource: planUri, type: FileChangeType.DELETED }], false));
+
+				assert.strictEqual(review.isOutdated, true);
+			} finally {
+				model.dispose();
+			}
+		});
+
 		test('renders markdown content in the body', () => {
 			createWidget(createMockReview({ content: '**bold text**' }));
 

src/vs/workbench/contrib/chat/test/common/model/chatSessionOperationLog.test.ts

@@ -5,12 +5,33 @@
 
 import assert from 'assert';
 import { VSBuffer } from '../../../../../../base/common/buffer.js';
+import { DisposableStore } from '../../../../../../base/common/lifecycle.js';
+import { hasKey } from '../../../../../../base/common/types.js';
 import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js';
+import { Range } from '../../../../../../editor/common/core/range.js';
+import { OffsetRange } from '../../../../../../editor/common/core/ranges/offsetRange.js';
+import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js';
+import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js';
+import { IContextKeyService } from '../../../../../../platform/contextkey/common/contextkey.js';
+import { TestInstantiationService } from '../../../../../../platform/instantiation/test/common/instantiationServiceMock.js';
+import { MockContextKeyService } from '../../../../../../platform/keybinding/test/common/mockKeybindingService.js';
+import { ILogService, NullLogService } from '../../../../../../platform/log/common/log.js';
+import { IStorageService } from '../../../../../../platform/storage/common/storage.js';
+import { IExtensionService } from '../../../../../services/extensions/common/extensions.js';
+import { TestExtensionService, TestStorageService } from '../../../../../test/common/workbenchTestServices.js';
+import { IChatService } from '../../../common/chatService/chatService.js';
+import { ChatAgentLocation } from '../../../common/constants.js';
+import { ChatModel } from '../../../common/model/chatModel.js';
+import { ChatPlanReviewData } from '../../../common/model/chatProgressTypes/chatPlanReviewData.js';
+import { ChatSessionOperationLog } from '../../../common/model/chatSessionOperationLog.js';
 import * as Adapt from '../../../common/model/objectMutationLog.js';
 import { equals } from '../../../../../../base/common/objects.js';
+import { ChatAgentService, IChatAgentService } from '../../../common/participants/chatAgents.js';
+import { ChatRequestTextPart } from '../../../common/requestParser/chatParserTypes.js';
+import { MockChatService } from '../chatService/mockChatService.js';
 
 suite('ChatSessionOperationLog', () => {
-	ensureNoDisposablesAreLeakedInTestSuite();
+	const testDisposables = ensureNoDisposablesAreLeakedInTestSuite();
 
 	// Test data types
 	interface TestItem {
@@ -59,6 +80,41 @@ suite('ChatSessionOperationLog', () => {
 		return reader.read(fileContent);
 	}
 
+	test('persists plan review changes through the operation log', () => {
+		const store = testDisposables.add(new DisposableStore());
+		const instantiationService = store.add(new TestInstantiationService());
+		instantiationService.stub(IStorageService, store.add(new TestStorageService()));
+		instantiationService.stub(ILogService, new NullLogService());
+		instantiationService.stub(IExtensionService, new TestExtensionService());
+		instantiationService.stub(IContextKeyService, new MockContextKeyService());
+		instantiationService.stub(IChatAgentService, store.add(instantiationService.createInstance(ChatAgentService)));
+		instantiationService.stub(IConfigurationService, new TestConfigurationService());
+		instantiationService.stub(IChatService, new MockChatService());
+
+		const model = store.add(instantiationService.createInstance(ChatModel, undefined, { initialLocation: ChatAgentLocation.Chat, canUseTools: true }));
+		const text = 'Create a plan';
+		const request = model.addRequest({
+			text,
+			parts: [new ChatRequestTextPart(new OffsetRange(0, text.length), new Range(1, 1, 1, text.length + 1), text)],
+		}, { variables: [] }, 0);
+		const review = new ChatPlanReviewData('Plan summary', 'Generated summary', [{ label: 'Approve' }], true);
+		model.acceptResponseProgress(request, review);
+
+		const writer = new ChatSessionOperationLog();
+		const initial = writer.createInitial(model);
+		review.isOutdated = true;
+		const update = writer.write(model);
+		writer.confirmWrite();
+
+		const reader = new ChatSessionOperationLog();
+		const restored = reader.read(VSBuffer.concat([initial, update.data]));
+		const restoredReview = restored.requests[0].response?.[0];
+		if (!restoredReview || !hasKey(restoredReview, { kind: true }) || restoredReview.kind !== 'planReview') {
+			assert.fail('Expected a restored plan review');
+		}
+		assert.strictEqual(restoredReview.isOutdated, true);
+	});
+
 	suite('Transform factories', () => {
 		test('key uses strict equality by default', () => {
 			const transform = Adapt.key<string>();