Skip to content

Commit 40c056f

Browse files
Fix wit_work_item_unlink ignoring type when url is provided (#1159)
**Fix type parameter bypass in `wit_work_item_unlink` when URL is provided.** The URL-match branch in the `work_item_unlink` tool only checked `relation.url` without verifying `relation.rel` matched the requested link type. This allowed deletion of relations whose actual type differed from the `type` parameter (e.g., removing an `ArtifactLink` when `type="related"` was specified). The code comment already stated "find relations matching both rel type and url" but the implementation only enforced the URL check. Added `relation.rel === linkType` to the URL-match condition so both type and URL must match before a relation is removed. ## GitHub issue number N/A ## **Associated Risks** - Callers that previously relied on URL-only matching (ignoring type) will now get a "No matching relations found" error if the type doesn't match the relation at that URL. This is the correct behavior per the tool's documented contract. - No breaking changes for callers that already pass consistent type + URL combinations. ## ✅ **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 - [x] 🔭 Telemetry added, updated, or N/A — N/A - [x] 📄 Documentation added, updated, or N/A — N/A (no doc changes needed; tool schema/description unchanged) - [x] 🛡️ Automated tests added, or N/A — Added ## 🧪 **How did you test it?** Added 3 unit tests in work-items.test.ts under `work_item_unlink tool > type + url matching`: | Test case | Scenario | Expected | |---|---|---| | `type=related` + ArtifactLink URL | URL matches but `rel` is `ArtifactLink`, not `System.LinkTypes.Related` | No deletion, returns "No matching relations found" | | `type=child` + ArtifactLink URL | URL matches but `rel` is `ArtifactLink`, not `System.LinkTypes.Hierarchy-Forward` | No deletion, returns "No matching relations found" | | `type=related` + matching Related URL | Both URL and `rel` match | Deletes the correct relation at index 1 | All 3 tests pass after the fix. Before the fix, tests 1 and 2 correctly failed (confirming the bug).
1 parent 5b842c5 commit 40c056f

File tree

2 files changed

+43
-1
lines changed

2 files changed

+43
-1
lines changed

src/tools/work-items.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1225,7 +1225,7 @@ function configureWorkItemTools(server: McpServer, tokenProvider: () => Promise<
12251225

12261226
if (url && url.trim().length > 0) {
12271227
// If url is provided, find relations matching both rel type and url
1228-
relationIndexes = relations.map((relation, idx) => (relation.url === url ? idx : -1)).filter((idx) => idx !== -1);
1228+
relationIndexes = relations.map((relation, idx) => (relation.rel === linkType && relation.url === url ? idx : -1)).filter((idx) => idx !== -1);
12291229
} else {
12301230
// If url is not provided, find all relations matching rel type
12311231
relationIndexes = relations.map((relation, idx) => (relation.rel === linkType ? idx : -1)).filter((idx) => idx !== -1);

test/src/tools/work-items.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2406,6 +2406,48 @@ describe("configureWorkItemTools", () => {
24062406
expect(result.isError).toBe(true);
24072407
expect(result.content[0].text).toBe("Error unlinking work item: Unknown error occurred");
24082408
});
2409+
2410+
describe("type + url matching", () => {
2411+
const artifactUrl = "vstfs:///Git/Ref/project%2Frepo%2FGBmain";
2412+
const relatedUrl = "https://dev.azure.com/contoso/_apis/wit/workItems/2";
2413+
const relations = [
2414+
{ rel: "ArtifactLink", url: artifactUrl, attributes: { name: "Branch" } },
2415+
{ rel: "System.LinkTypes.Related", url: relatedUrl, attributes: { isLocked: false, name: "Related" } },
2416+
];
2417+
2418+
const getUnlinkHandler = () => {
2419+
configureWorkItemTools(server, tokenProvider, connectionProvider, userAgentProvider);
2420+
const call = (server.tool as jest.Mock).mock.calls.find(([toolName]) => toolName === "wit_work_item_unlink");
2421+
if (!call) throw new Error("wit_work_item_unlink tool not registered");
2422+
return call[3];
2423+
};
2424+
2425+
it.each([
2426+
{ name: "type=related + ArtifactLink url (type bypass)", type: "related", url: artifactUrl },
2427+
{ name: "type=child + ArtifactLink url (type bypass)", type: "child", url: artifactUrl },
2428+
])("should NOT remove a relation when url matches but type does not ($name)", async ({ type, url }) => {
2429+
const handler = getUnlinkHandler();
2430+
(mockWorkItemTrackingApi.getWorkItem as jest.Mock).mockResolvedValue({ id: 1, relations });
2431+
2432+
const result = await handler({ project: "TestProject", id: 1, type, url });
2433+
2434+
expect(mockWorkItemTrackingApi.updateWorkItem).not.toHaveBeenCalled();
2435+
expect(result.isError).toBe(true);
2436+
expect(result.content[0].text).toContain("No matching relations found");
2437+
});
2438+
2439+
it("should remove a relation when both url and type match", async () => {
2440+
const handler = getUnlinkHandler();
2441+
(mockWorkItemTrackingApi.getWorkItem as jest.Mock).mockResolvedValue({ id: 1, relations });
2442+
(mockWorkItemTrackingApi.updateWorkItem as jest.Mock).mockResolvedValue({ id: 1, rev: 10 });
2443+
2444+
const result = await handler({ project: "TestProject", id: 1, type: "related", url: relatedUrl });
2445+
2446+
expect(mockWorkItemTrackingApi.updateWorkItem).toHaveBeenCalledWith(null, [{ op: "remove", path: "/relations/1" }], 1, "TestProject");
2447+
expect(result.isError).toBe(false);
2448+
expect(result.content[0].text).toContain("Removed 1 link(s) of type 'related':");
2449+
});
2450+
});
24092451
});
24102452

24112453
// Add error handling tests for existing tools

0 commit comments

Comments
 (0)