Skip to content

Commit cd02548

Browse files
lgpearson1771Copilotdanhellem
authored
Add includeChangedFiles parameter to get_pull_request_by_id (#1117)
Adds an optional `includeChangedFiles` parameter to the existing `repo_get_pull_request_by_id` tool. When set to `true`, it fetches the latest PR iteration and returns file-level change metadata (paths and change types) — no diffs, no file content. ## GitHub Issue Closes #958 ## Approach Per @danhellem's [feedback](#958 (comment)), this avoids tool bloat by extending the existing tool with a parameter rather than adding a new standalone tool. **Note:** I was on vacation when the issue was closed — this PR implements the approved approach from the discussion. ## Changes - **`src/tools/repositories.ts`** — Added `includeChangedFiles` param; fetches latest iteration and returns `changedFilesSummary` with change entries, file count, and pagination info - **`test/src/tools/repositories.test.ts`** — 7 new test cases: happy path, disabled/default, empty iterations, null iteration ID, API errors, and combo with `includeLabels` - **`docs/TOOLSET.md`** — Updated parameter docs ## Associated Risks Low risk — additive change behind an opt-in flag (`default: false`), no change to existing behavior ## ✅ PR Checklist - [x] I have read the [contribution guidelines](https://github.com/microsoft/azure-devops-mcp/blob/main/CONTRIBUTING.md) - [x] I have read the [code of conduct guidelines](https://github.com/microsoft/azure-devops-mcp/blob/main/CODE_OF_CONDUCT.md) - [x] Title of the pull request is clear and informative - [x] 👌 Code hygiene - [ ] 🔭 Telemetry added, updated, or N/A - [x] 📄 Documentation added, updated, or N/A - [x] 🛡️ Automated tests added, or N/A ## 🧪 How did you test it? - All 226 repository tests pass (`npx jest --testPathPatterns="repositories"`) - 7 new tests cover: happy path, opt-out, default behavior, empty iterations, null iteration IDs, API error handling, and combined use with `includeLabels` Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Dan Hellem <dahellem@microsoft.com>
1 parent bf6d04a commit cd02548

File tree

3 files changed

+211
-14
lines changed

3 files changed

+211
-14
lines changed

‎docs/TOOLSET.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ Lists pull requests by commit IDs to find which pull requests contain specific c
301301
Get a pull request by its ID.
302302

303303
- **Required**: `repositoryId`, `pullRequestId`
304-
- **Optional**: `includeWorkItemRefs`
304+
- **Optional**: `project`, `includeWorkItemRefs`, `includeLabels`, `includeChangedFiles`
305305

306306
### mcp_ado_repo_get_pull_request_changes
307307

‎src/tools/repositories.ts‎

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,13 +1010,16 @@ function configureRepoTools(server: McpServer, tokenProvider: () => Promise<stri
10101010
project: z.string().optional().describe("Project ID or project name. Required when repositoryId is a repository name instead of a GUID."),
10111011
includeWorkItemRefs: z.boolean().optional().default(false).describe("Whether to reference work items associated with the pull request."),
10121012
includeLabels: z.boolean().optional().default(false).describe("Whether to include a summary of labels in the response."),
1013+
includeChangedFiles: z.boolean().optional().default(false).describe("Whether to include the list of files changed in the pull request."),
10131014
},
1014-
async ({ repositoryId, pullRequestId, project, includeWorkItemRefs, includeLabels }) => {
1015+
async ({ repositoryId, pullRequestId, project, includeWorkItemRefs, includeLabels, includeChangedFiles }) => {
10151016
try {
10161017
const connection = await connectionProvider();
10171018
const gitApi = await connection.getGitApi();
10181019
const pullRequest = await gitApi.getPullRequest(repositoryId, pullRequestId, project, undefined, undefined, undefined, undefined, includeWorkItemRefs);
10191020

1021+
let enhancedResponse: Record<string, unknown> = { ...pullRequest };
1022+
10201023
if (includeLabels) {
10211024
try {
10221025
const projectId = pullRequest.repository?.project?.id;
@@ -1025,32 +1028,64 @@ function configureRepoTools(server: McpServer, tokenProvider: () => Promise<stri
10251028

10261029
const labelNames = labels.map((label) => label.name).filter((name) => name !== undefined);
10271030

1028-
const enhancedResponse = {
1029-
...pullRequest,
1031+
enhancedResponse = {
1032+
...enhancedResponse,
10301033
labelSummary: {
10311034
labels: labelNames,
10321035
labelCount: labelNames.length,
10331036
},
10341037
};
1035-
1036-
return {
1037-
content: [{ type: "text", text: JSON.stringify(enhancedResponse, null, 2) }],
1038-
};
10391038
} catch (error) {
10401039
console.warn(`Error fetching PR labels: ${error instanceof Error ? error.message : "Unknown error"}`);
1041-
// Fall back to the original response without labels
1042-
const enhancedResponse = {
1043-
...pullRequest,
1040+
enhancedResponse = {
1041+
...enhancedResponse,
10441042
labelSummary: {},
10451043
};
1044+
}
1045+
}
10461046

1047-
return {
1048-
content: [{ type: "text", text: JSON.stringify(enhancedResponse, null, 2) }],
1047+
if (includeChangedFiles) {
1048+
try {
1049+
const iterations = await gitApi.getPullRequestIterations(repositoryId, pullRequestId, project);
1050+
1051+
if (iterations?.length) {
1052+
const latestIteration = iterations[iterations.length - 1];
1053+
1054+
if (latestIteration.id != null) {
1055+
const changes = await gitApi.getPullRequestIterationChanges(repositoryId, pullRequestId, latestIteration.id, project);
1056+
1057+
enhancedResponse = {
1058+
...enhancedResponse,
1059+
changedFilesSummary: {
1060+
changeEntries: changes?.changeEntries ?? [],
1061+
fileCount: changes?.changeEntries?.length ?? 0,
1062+
nextSkip: changes?.nextSkip,
1063+
nextTop: changes?.nextTop,
1064+
},
1065+
};
1066+
} else {
1067+
enhancedResponse = {
1068+
...enhancedResponse,
1069+
changedFilesSummary: { changeEntries: [], fileCount: 0 },
1070+
};
1071+
}
1072+
} else {
1073+
enhancedResponse = {
1074+
...enhancedResponse,
1075+
changedFilesSummary: { changeEntries: [], fileCount: 0 },
1076+
};
1077+
}
1078+
} catch (error) {
1079+
console.warn(`Error fetching PR changed files: ${error instanceof Error ? error.message : "Unknown error"}`);
1080+
enhancedResponse = {
1081+
...enhancedResponse,
1082+
changedFilesSummary: {},
10491083
};
10501084
}
10511085
}
1086+
10521087
return {
1053-
content: [{ type: "text", text: JSON.stringify(pullRequest, null, 2) }],
1088+
content: [{ type: "text", text: JSON.stringify(enhancedResponse, null, 2) }],
10541089
};
10551090
} catch (error) {
10561091
const errorMessage = error instanceof Error ? error.message : "Unknown error occurred";

‎test/src/tools/repositories.test.ts‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3862,6 +3862,168 @@ describe("repos tools", () => {
38623862

38633863
expect(result.content[0].text).toBe(JSON.stringify(expectedResponse, null, 2));
38643864
});
3865+
3866+
it("should include changed files when includeChangedFiles is true", async () => {
3867+
configureRepoTools(server, tokenProvider, connectionProvider, userAgentProvider);
3868+
3869+
const call = (server.tool as jest.Mock).mock.calls.find(([toolName]) => toolName === REPO_TOOLS.get_pull_request_by_id);
3870+
if (!call) throw new Error("repo_get_pull_request_by_id tool not registered");
3871+
const [, , , handler] = call;
3872+
3873+
const mockPR = {
3874+
pullRequestId: 123,
3875+
title: "Test PR",
3876+
repository: { project: { id: "project123", name: "testproject" } },
3877+
};
3878+
mockGitApi.getPullRequest.mockResolvedValue(mockPR);
3879+
3880+
const mockChangeEntries = [
3881+
{ changeTrackingId: 1, item: { path: "/src/file1.ts" }, changeType: 2 },
3882+
{ changeTrackingId: 2, item: { path: "/src/file2.ts" }, changeType: 1 },
3883+
];
3884+
mockGitApi.getPullRequestIterations.mockResolvedValue([{ id: 1 }, { id: 2 }]);
3885+
mockGitApi.getPullRequestIterationChanges.mockResolvedValue({ changeEntries: mockChangeEntries });
3886+
3887+
const params = {
3888+
repositoryId: "repo123",
3889+
pullRequestId: 123,
3890+
includeChangedFiles: true,
3891+
};
3892+
3893+
const result = await handler(params);
3894+
3895+
expect(mockGitApi.getPullRequestIterations).toHaveBeenCalledWith("repo123", 123, undefined);
3896+
expect(mockGitApi.getPullRequestIterationChanges).toHaveBeenCalledWith("repo123", 123, 2, undefined);
3897+
3898+
const resultData = JSON.parse(result.content[0].text);
3899+
expect(resultData.changedFilesSummary).toEqual({
3900+
changeEntries: mockChangeEntries,
3901+
fileCount: 2,
3902+
});
3903+
});
3904+
3905+
it("should not fetch changed files when includeChangedFiles is false", async () => {
3906+
configureRepoTools(server, tokenProvider, connectionProvider, userAgentProvider);
3907+
3908+
const call = (server.tool as jest.Mock).mock.calls.find(([toolName]) => toolName === REPO_TOOLS.get_pull_request_by_id);
3909+
const [, , , handler] = call;
3910+
3911+
const mockPR = { pullRequestId: 123, title: "Test PR" };
3912+
mockGitApi.getPullRequest.mockResolvedValue(mockPR);
3913+
3914+
const result = await handler({ repositoryId: "repo123", pullRequestId: 123, includeChangedFiles: false });
3915+
3916+
expect(mockGitApi.getPullRequestIterations).not.toHaveBeenCalled();
3917+
expect(result.content[0].text).toBe(JSON.stringify(mockPR, null, 2));
3918+
});
3919+
3920+
it("should not fetch changed files when includeChangedFiles is not specified", async () => {
3921+
configureRepoTools(server, tokenProvider, connectionProvider, userAgentProvider);
3922+
3923+
const call = (server.tool as jest.Mock).mock.calls.find(([toolName]) => toolName === REPO_TOOLS.get_pull_request_by_id);
3924+
const [, , , handler] = call;
3925+
3926+
const mockPR = { pullRequestId: 123, title: "Test PR" };
3927+
mockGitApi.getPullRequest.mockResolvedValue(mockPR);
3928+
3929+
const result = await handler({ repositoryId: "repo123", pullRequestId: 123 });
3930+
3931+
expect(mockGitApi.getPullRequestIterations).not.toHaveBeenCalled();
3932+
expect(result.content[0].text).toBe(JSON.stringify(mockPR, null, 2));
3933+
});
3934+
3935+
it("should handle empty iterations when includeChangedFiles is true", async () => {
3936+
configureRepoTools(server, tokenProvider, connectionProvider, userAgentProvider);
3937+
3938+
const call = (server.tool as jest.Mock).mock.calls.find(([toolName]) => toolName === REPO_TOOLS.get_pull_request_by_id);
3939+
const [, , , handler] = call;
3940+
3941+
const mockPR = { pullRequestId: 123, title: "Test PR" };
3942+
mockGitApi.getPullRequest.mockResolvedValue(mockPR);
3943+
mockGitApi.getPullRequestIterations.mockResolvedValue([]);
3944+
3945+
const result = await handler({ repositoryId: "repo123", pullRequestId: 123, includeChangedFiles: true });
3946+
3947+
const resultData = JSON.parse(result.content[0].text);
3948+
expect(resultData.changedFilesSummary).toEqual({ changeEntries: [], fileCount: 0 });
3949+
expect(mockGitApi.getPullRequestIterationChanges).not.toHaveBeenCalled();
3950+
});
3951+
3952+
it("should handle getPullRequestIterationChanges API error gracefully", async () => {
3953+
configureRepoTools(server, tokenProvider, connectionProvider, userAgentProvider);
3954+
3955+
const call = (server.tool as jest.Mock).mock.calls.find(([toolName]) => toolName === REPO_TOOLS.get_pull_request_by_id);
3956+
const [, , , handler] = call;
3957+
3958+
const mockPR = { pullRequestId: 123, title: "Test PR" };
3959+
mockGitApi.getPullRequest.mockResolvedValue(mockPR);
3960+
mockGitApi.getPullRequestIterations.mockResolvedValue([{ id: 1 }]);
3961+
mockGitApi.getPullRequestIterationChanges.mockRejectedValue(new Error("API Error: Changes not accessible"));
3962+
3963+
const consoleSpy = jest.spyOn(console, "warn").mockImplementation();
3964+
3965+
const result = await handler({ repositoryId: "repo123", pullRequestId: 123, includeChangedFiles: true });
3966+
3967+
expect(consoleSpy).toHaveBeenCalledWith("Error fetching PR changed files: API Error: Changes not accessible");
3968+
3969+
const resultData = JSON.parse(result.content[0].text);
3970+
expect(resultData.pullRequestId).toBe(123);
3971+
expect(resultData.changedFilesSummary).toEqual({});
3972+
3973+
consoleSpy.mockRestore();
3974+
});
3975+
3976+
it("should handle iteration with null id when includeChangedFiles is true", async () => {
3977+
configureRepoTools(server, tokenProvider, connectionProvider, userAgentProvider);
3978+
3979+
const call = (server.tool as jest.Mock).mock.calls.find(([toolName]) => toolName === REPO_TOOLS.get_pull_request_by_id);
3980+
const [, , , handler] = call;
3981+
3982+
const mockPR = { pullRequestId: 123, title: "Test PR" };
3983+
mockGitApi.getPullRequest.mockResolvedValue(mockPR);
3984+
mockGitApi.getPullRequestIterations.mockResolvedValue([{ id: null }]);
3985+
3986+
const result = await handler({ repositoryId: "repo123", pullRequestId: 123, includeChangedFiles: true });
3987+
3988+
const resultData = JSON.parse(result.content[0].text);
3989+
expect(resultData.changedFilesSummary).toEqual({ changeEntries: [], fileCount: 0 });
3990+
expect(mockGitApi.getPullRequestIterationChanges).not.toHaveBeenCalled();
3991+
});
3992+
3993+
it("should work with both includeLabels and includeChangedFiles enabled", async () => {
3994+
configureRepoTools(server, tokenProvider, connectionProvider, userAgentProvider);
3995+
3996+
const call = (server.tool as jest.Mock).mock.calls.find(([toolName]) => toolName === REPO_TOOLS.get_pull_request_by_id);
3997+
const [, , , handler] = call;
3998+
3999+
const mockPR = {
4000+
pullRequestId: 123,
4001+
title: "Test PR",
4002+
repository: { project: { id: "project123", name: "testproject" } },
4003+
};
4004+
mockGitApi.getPullRequest.mockResolvedValue(mockPR);
4005+
4006+
const mockLabels = [{ name: "bug", id: "label1" }];
4007+
mockGitApi.getPullRequestLabels.mockResolvedValue(mockLabels);
4008+
4009+
const mockChangeEntries = [{ changeTrackingId: 1, item: { path: "/src/app.ts" }, changeType: 2 }];
4010+
mockGitApi.getPullRequestIterations.mockResolvedValue([{ id: 1 }]);
4011+
mockGitApi.getPullRequestIterationChanges.mockResolvedValue({ changeEntries: mockChangeEntries });
4012+
4013+
const result = await handler({
4014+
repositoryId: "repo123",
4015+
pullRequestId: 123,
4016+
includeLabels: true,
4017+
includeChangedFiles: true,
4018+
});
4019+
4020+
expect(mockGitApi.getPullRequestLabels).toHaveBeenCalled();
4021+
expect(mockGitApi.getPullRequestIterations).toHaveBeenCalled();
4022+
4023+
const resultData = JSON.parse(result.content[0].text);
4024+
expect(resultData.labelSummary).toEqual({ labels: ["bug"], labelCount: 1 });
4025+
expect(resultData.changedFilesSummary).toEqual({ changeEntries: mockChangeEntries, fileCount: 1 });
4026+
});
38654027
});
38664028

38674029
describe("repo_get_pull_request_changes", () => {

0 commit comments

Comments
 (0)