Skip to content

Commit 6fe0fdf

Browse files
aotradovecAdam OtradovecYash-Singh1
authored
fix(azure-devops): existing pull requests are found for branches containing # (#31928)
Co-authored-by: Adam Otradovec <du239@creditas.cz> Co-authored-by: Yash Singh <saiansh2525@gmail.com>
1 parent c09060b commit 6fe0fdf

4 files changed

Lines changed: 57 additions & 2 deletions

File tree

‎packages/source-control-azure-devops/src/server/AzureDevOpsCli.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,38 @@ describe("AzureDevOpsCli.layer", () => {
233233
}).pipe(Effect.provide(layer)),
234234
);
235235

236+
it.effect("percent-encodes the source branch while preserving recognized ref prefixes", () =>
237+
Effect.gen(function* () {
238+
mockRun.mockReturnValueOnce(Effect.succeed(processOutput("[]")));
239+
240+
const az = yield* AzureDevOpsCli.AzureDevOpsCli;
241+
yield* az.listPullRequests({
242+
cwd: "/somewhere",
243+
headSelector: "user/feat/1-(#2)-slug",
244+
state: "open",
245+
});
246+
247+
const args = mockRun.mock.calls.at(-1)?.[0].args ?? [];
248+
assert.strictEqual(args[args.indexOf("--source-branch") + 1], "user%2Ffeat%2F1-(%232)-slug");
249+
250+
for (const prefix of ["refs/heads/", "refs/pull/", "refs/tags/"]) {
251+
mockRun.mockReturnValueOnce(Effect.succeed(processOutput("[]")));
252+
yield* az.listPullRequests({
253+
cwd: "/somewhere",
254+
headSelector: "fallback",
255+
source: { refName: `${prefix}user/feat/1-(#2)-slug` },
256+
state: "open",
257+
});
258+
259+
const refArgs = mockRun.mock.calls.at(-1)?.[0].args ?? [];
260+
assert.strictEqual(
261+
refArgs[refArgs.indexOf("--source-branch") + 1],
262+
`${prefix}user%2Ffeat%2F1-(%232)-slug`,
263+
);
264+
}
265+
}).pipe(Effect.provide(layer)),
266+
);
267+
236268
it.effect("preserves VCS causes without copying upstream details into messages", () =>
237269
Effect.gen(function* () {
238270
const cause = new VcsProcessExitError({

‎packages/source-control-azure-devops/src/server/AzureDevOpsCli.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,11 @@ export const make = Effect.gen(function* () {
401401
return AzureDevOpsCli.of({
402402
execute,
403403
listPullRequests: Effect.fn("AzureDevOpsCli.listPullRequests")(function (input) {
404+
const branch = SourceControlProvider.sourceBranch(input);
405+
// az checks literal ref prefixes before constructing the request URL.
406+
const refPrefix =
407+
["refs/heads/", "refs/pull/", "refs/tags/"].find((prefix) => branch.startsWith(prefix)) ??
408+
"";
404409
return executeJson({
405410
cwd: input.cwd,
406411
args: [
@@ -410,7 +415,8 @@ export const make = Effect.gen(function* () {
410415
"--detect",
411416
"true",
412417
"--source-branch",
413-
SourceControlProvider.sourceBranch(input),
418+
// az puts this in the query string unescaped; a `#` would cut the request.
419+
refPrefix + encodeURIComponent(branch.slice(refPrefix.length)),
414420
"--status",
415421
toAzureStatus(input.state),
416422
"--top",

‎packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -850,6 +850,22 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
850850
}),
851851
);
852852

853+
it.effect("escapes characters in a file path that would cut the query string", () =>
854+
Effect.gen(function* () {
855+
mockedExecute.mockReturnValueOnce(Effect.succeed(output(json({ content: "x" }))));
856+
const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;
857+
858+
yield* cli.readItemContent({
859+
cwd: "/w",
860+
location: { project: "platform", repository: "web" },
861+
path: "docs/C#/a&b.md",
862+
commit: "a".repeat(40),
863+
});
864+
865+
expect(mockedExecute.mock.calls.at(-1)?.[0].args).toContain("path=%2Fdocs%2FC%23%2Fa%26b.md");
866+
}),
867+
);
868+
853869
it.effect("reports a pull request it cannot place as its own outcome", () =>
854870
Effect.gen(function* () {
855871
mockedExecute.mockReturnValueOnce(

‎packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestCli.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,8 @@ export const make = Effect.gen(function* () {
603603
resource: "items",
604604
routeParameters: repositoryRoute(input.location),
605605
queryParameters: [
606-
`path=${toItemPath(input.path)}`,
606+
// `az devops invoke` does not escape query values, so `#` or `&` in a path would truncate it.
607+
`path=${encodeURIComponent(toItemPath(input.path))}`,
607608
"versionDescriptor.versionType=commit",
608609
`versionDescriptor.version=${input.commit}`,
609610
"includeContent=true",

0 commit comments

Comments
 (0)