ccr in autopilot

[justschen]

adds ccr in autopilot once it is finished the task.

behind an experimental setting for now

[Copilot]

Pull request overview

This PR adds an optional “code review after autopilot completes” step to the autopilot tool-calling loop, gated behind a new experimental configuration setting. The intent is to automatically run Copilot Code Review on files autopilot edited, and feed any review comments back to the agent for one additional fix cycle.

Changes:

• Introduces github.copilot.chat.autopilot.codeReview.enabled (and corresponding ConfigKey) to gate the feature.
• Extends code review input to accept optional pre-resolved baseContent, and forwards cancellation into the review command.
• Implements autopilot post-completion review logic (snapshotting pre-edit content, running review, and feeding comments back) plus adds unit coverage for the new behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/platform/review/common/reviewCommand.ts	Adds optional baseContent to review file inputs.
src/platform/configuration/common/configurationService.ts	Adds ConfigKey.Advanced.AutopilotCodeReviewEnabled.
src/extension/review/node/doReview.ts	Allows passing a cancellation token and reads optional baseContent.
src/extension/intents/node/toolCallingLoop.ts	Implements pre-edit snapshot capture and autopilot post-task code review loop continuation.
src/extension/intents/test/node/toolCallingLoopAutopilot.spec.ts	Adds tests for edit detection, edited path extraction, and autopilot code review behavior.
package.json	Adds the new experimental setting contribution.
package.nls.json	Adds localized description for the new setting.

Comments suppressed due to low confidence (3)

src/extension/intents/node/toolCallingLoop.ts:547

• capturePreEditSnapshots reads files using URI.file(filePath) where filePath comes directly from the model-provided tool-call JSON arguments. This bypasses the normal tool path validation / external-file confirmation logic (e.g. isFileExternalAndNeedsConfirmation) and could allow unintended reads of arbitrary absolute paths before the tool invocation is validated or even executed. Suggest deriving the set of edited URIs from trusted sources (e.g. actual edit results / editedFileEvents) and/or running the same path resolution + safety/confirmation checks used by edit tools before reading snapshots.

	private async capturePreEditSnapshots(round: IToolCallRound): Promise<void> {
		if (this.options.request.permissionLevel !== 'autopilot') {
			return;
		}
		const filePaths = ToolCallingLoop.extractEditedFilePathsFromRound(round);
		const newPaths = filePaths.filter(p => !this.preEditSnapshots.has(p));
		if (!newPaths.length) {
			return;
		}
		await Promise.all(newPaths.map(async filePath => {
			try {
				const bytes = await this._fileSystemService.readFile(URI.file(filePath));
				if (!this.preEditSnapshots.has(filePath)) {
					this.preEditSnapshots.set(filePath, new TextDecoder().decode(bytes));
				}
			} catch {
				// File doesn't exist yet (e.g. create_file) — record empty string
				if (!this.preEditSnapshots.has(filePath)) {
					this.preEditSnapshots.set(filePath, '');
				}

src/extension/intents/node/toolCallingLoop.ts:548

• capturePreEditSnapshots treats any readFile failure as “file doesn’t exist yet” and stores an empty-string snapshot. This is incorrect for other errors (e.g. the 5MB read limit, permission issues, transient FS errors) and can make the subsequent review think an existing file was created from scratch, generating a huge/incorrect diff. Consider only using the empty-string fallback for a confirmed not-found case, and otherwise skip capturing/reviewing that file (or log + leave the snapshot undefined).

		await Promise.all(newPaths.map(async filePath => {
			try {
				const bytes = await this._fileSystemService.readFile(URI.file(filePath));
				if (!this.preEditSnapshots.has(filePath)) {
					this.preEditSnapshots.set(filePath, new TextDecoder().decode(bytes));
				}
			} catch {
				// File doesn't exist yet (e.g. create_file) — record empty string
				if (!this.preEditSnapshots.has(filePath)) {
					this.preEditSnapshots.set(filePath, '');
				}
			}

src/extension/intents/node/toolCallingLoop.ts:625

• preEditSnapshots is keyed by the raw filePath strings extracted from tool calls, but later lookups use uri.fsPath (this.preEditSnapshots.get(uri.fsPath)). On Windows (or when paths differ only by normalization), this can miss snapshots and cause baseContent to be undefined even when a snapshot was captured. Consider normalizing keys consistently (e.g. store snapshots under URI.file(filePath).fsPath and look up using the same normalization).

			const files: CodeReviewFileInput[] = editedFiles.map(uri => {
				const snapshot = this.preEditSnapshots.get(uri.fsPath);
				return { currentUri: uri, baseContent: snapshot };
			});

[Copilot]

baseContent is documented to take precedence over baseUri, but the current check if (!baseContent && file.baseUri) treats an intentional empty-string base (e.g. an empty file snapshot or “file did not exist” snapshot) as “missing” and will fall back to reading from baseUri. This produces the wrong base for diffing/review. Consider switching this logic to only read from baseUri when file.baseContent is undefined (not when it’s an empty string).

Suggested change

	let baseContent = file.baseContent ?? '';
	if (!baseContent && file.baseUri) {
	const bytes = await fileSystemService.readFile(file.baseUri);
	baseContent = new TextDecoder().decode(bytes);
	}
	let baseContent = file.baseContent;
	if (baseContent === undefined && file.baseUri) {
	const bytes = await fileSystemService.readFile(file.baseUri);
	baseContent = new TextDecoder().decode(bytes);
	}
	if (baseContent === undefined) {
	baseContent = '';
	}

[Copilot]

getEditedFilePaths converts tool-call filePath strings with URI.file(p). This breaks if a tool input uses a non-file URI string (e.g. vscode-vfs://...) and can also mis-handle remote schemes. Since tool inputs are already expected to be in the prompt path representation format, consider resolving them via IPromptPathRepresentationService.resolveFilePath (or a shared helper like resolveToolInputPath) instead of URI.file so the resulting URIs match what the tools actually operate on.

This issue also appears in the following locations of the same file:

• line 528
• line 537
• line 622

[Copilot]

This test class relies on (this as any) to call a private method (performAutopilotCodeReview) and to mutate private fields (taskCompleted, preEditSnapshots, etc.). In this repo’s Vitest guidelines, tests should avoid as any to reach into private internals; prefer exposing the needed behavior via a dedicated test hook API (e.g. make the method protected and call it through a public wrapper on the subclass, or factor the code-review logic into a helper/service that can be unit-tested directly).