Add "view changes" action into PR webview header - #8950
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved moderate issues affect missing-head handling, merged-PR diff correctness, and error reporting.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
src/github/pullRequestOverview.ts — This handler has no failure path. If the provider is unavailable, the PR has no head/base commit,… |
What changed in this PR
Adds an agent-session-only “View Changes” action to PR webview headers, opening read-only diffs from local Git or GitHub.
Changes:
- Adds regular and sticky header actions with a diff icon.
- Implements webview messaging and local/GitHub diff construction.
- Extends view state and adds UI/model tests.
| File | Change | Final review notes |
|---|---|---|
webviews/editorWebview/test/overview.test.tsx |
Tests header visibility and clicks. | — |
webviews/editorWebview/test/builder/pullRequest.ts |
Updates PR test fixtures. | — |
webviews/components/stickyHeader.tsx |
Adds the View Changes action. | — |
webviews/components/icon.tsx |
Registers the diff icon. | — |
webviews/components/header.tsx |
Adds the View Changes action. | moderate (1 vote): Hide or disable it when the head repository or ref is unavailable. |
webviews/common/context.tsx |
Sends the view-changes command. | — |
src/test/github/pullRequestModel.test.ts |
Tests diff sources and URI construction. | nit (1 vote): Add mocked GitHub coverage for merged and open-PR fallback paths. |
src/github/views.ts |
Extends PR view state. | — |
src/github/pullRequestOverview.ts |
Handles the view-changes command. | moderate (2 votes): Add failure handling so requests receive _throwError when operations fail. |
src/github/pullRequestModel.ts |
Builds local and GitHub-backed diffs. | moderate (1 vote): Handle deleted PR heads without throwing. moderate (1 vote): Avoid the local fast path for merged PRs unless using the verified original base. |
resources/icons/codicons/diff-multiple.svg |
Adds the diff icon asset. | — |
Suppressed comments (4)
src/github/pullRequestModel.ts:1647
- A deleted PR head can still have
headRefOidwhileheadRepositoryis absent, in which caseparseRefleavesPullRequestModel.headundefined. The header still renders this action for agent workspaces, so this guard throws before the GitHub file-list fallback and makes “View Changes” unusable for those PRs. Preserve/use the raw head OID or suppress the action when no head commit can be resolved.
const headCommit = pullRequestModel.head?.sha;
src/github/pullRequestModel.ts:1657
- The merged-PR branch currently enables the local fast path by diffing
base.shaagainsthead.sha. For a merged PR,base.shacan be the current base-branch tip (including the merge commit), so that range contains the wrong changes; the existinggetRawFileChangesInfodeliberately uses the PR file-list API for merged PRs instead. Skip the local fast path for merged PRs and letgetAllFileChangesInfo()supply the PR's file set/base, or use a verified original-base commit.
let startCommit = pullRequestModel.item.merged
? pullRequestModel.base.sha
: await this.getLocalMergeBase(folderManager.repository, pullRequestModel.base.sha, headCommit);
let localChanges: Change[] | undefined;
if (startCommit && await this.commitsExistLocally(folderManager.repository, startCommit, headCommit)) {
localChanges = await folderManager.repository.diffBetween(startCommit, headCommit);
src/test/github/pullRequestModel.test.ts:157
- The remote fallback test stubs
getAllFileChangesInfo, so the newly added GitHub implementation is never exercised. The mergedpulls.listFilesbranch, open-PRcompareCommits/merge-base wiring, andparseDiffconversion are therefore unverified—the path used when the local commit range is unavailable. Add focused coverage with mocked GitHub responses for the merged and open-PR cases.
const getAllFileChangesInfo = sinon.stub(model, 'getAllFileChangesInfo').resolves({
changes: [new SlimFileChange(mergeBase, '', GitChangeType.RENAME, 'new.ts', 'old.ts')],
mergeBase,
});
webviews/components/header.tsx:154
- This condition exposes the action for PRs whose head repository or ref has been deleted. In that case the serialized
headis empty andopenReadonlyChangesimmediately throws for the missing head commit, so the new control is unusable (and the same condition is duplicated inStickyHeader). Hide or disable the action when no head commit is available.
{!isIssue && isAgentSessionsWorkspace ? <ViewChangesButton /> : null}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Henning Dieterichs (hediet)
approved these changes
Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

No description provided.