fix(server): read Azure DevOps pull requests from the app

apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts

@@ -97,6 +97,49 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
     }),
   );
 
+  it.effect("names the repository the way az does, not the way the remote does", () =>
+    Effect.gen(function* () {
+      mockedExecute.mockReturnValueOnce(Effect.succeed(output(pullRequests(1, 1))));
+      const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;
+
+      yield* cli.listPullRequests({
+        cwd: "/w",
+        // What the service calls a repository below an Azure host: the whole remote path.
+        repository: "acme/platform/_git/web",
+        state: "open",
+        involvement: "all",
+        viewer: "bilal@acme.dev",
+        limit: 10,
+      });
+
+      // az puts `--repository` straight into the REST route it builds, so the path would address
+      // a route that does not exist and Azure would answer 404 rather than an empty listing.
+      const args = argsOfCall(0);
+      assert.strictEqual(args[args.indexOf("--repository") + 1], "web");
+      expect(args).not.toContain("acme/platform/_git/web");
+    }),
+  );
+
+  it.effect("unescapes the repository name, which the remote path carries escaped", () =>
+    Effect.gen(function* () {
+      mockedExecute.mockReturnValueOnce(Effect.succeed(output(pullRequests(1, 1))));
+      const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;
+
+      yield* cli.listPullRequests({
+        cwd: "/w",
+        // A project and a repository both named with a space, as the remote spells them.
+        repository: "acme/shared%20platform/_git/web%20client",
+        state: "open",
+        involvement: "all",
+        viewer: "bilal@acme.dev",
+        limit: 10,
+      });
+
+      const args = argsOfCall(0);
+      assert.strictEqual(args[args.indexOf("--repository") + 1], "web client");
+    }),
+  );
+
   it.effect("reads the page unnarrowed when asked to search, having nothing to search with", () =>
     Effect.gen(function* () {
       mockedExecute.mockReturnValueOnce(Effect.succeed(output(pullRequests(3, 1))));
@@ -401,6 +444,13 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
       expect(argsOfCall(0)).toContain(
         "https://dev.azure.com/acme/platform/_apis/git/r/web/pullRequests/42/threads?api-version=7.1",
       );
+      // Azure DevOps is not the resource az asks for a token for by default, and it answers the
+      // sign-in page rather than refusing the wrong one, so the call has to name it.
+      const args = argsOfCall(0);
+      assert.strictEqual(
+        args[args.indexOf("--resource") + 1],
+        "499b84ac-1321-427f-aa17-267ca6975798",
+      );
     }),
   );
 

apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts

@@ -112,6 +112,14 @@ export type AzureDevOpsPullRequestCliError =
 /** The version every REST call below is pinned to, so a new default cannot reshape a response. */
 const REST_API_VERSION = "7.1";
 
+/**
+ * The application `az rest` has to ask for a token for. Azure DevOps is not the resource az
+ * defaults to, and it does not refuse the wrong token: it answers the sign-in page, as HTML, with
+ * a status the CLI reports as success. So a call without this reads as a decode failure on a
+ * response nobody asked for rather than as the unauthenticated call it is.
+ */
+const AZURE_DEVOPS_RESOURCE_ID = "499b84ac-1321-427f-aa17-267ca6975798";
+
 export class AzureDevOpsPullRequestCli extends Context.Service<
   AzureDevOpsPullRequestCli,
   {
@@ -173,6 +181,28 @@ export class AzureDevOpsPullRequestCli extends Context.Service<
   }
 >()("t3/pullRequest/AzureDevOpsPullRequestCli") {}
 
+/**
+ * The repository name `az` wants. The service names a repository the way its remote does — below
+ * an Azure host that is the whole path, `{organization}/{project}/_git/{repository}` — but
+ * `--repository` takes a name or an id, and az interpolates whatever it is given straight into the
+ * REST route it builds. A path there does not narrow the listing to the wrong repository; it
+ * addresses a route that does not exist, and Azure answers 404.
+ *
+ * The last segment is the name. It is unescaped because the path carries it as the remote spells
+ * it: a project or a repository named with a space reaches this as `%20`, which Azure would look
+ * up literally. A name recorded without a path is already a name and survives both steps.
+ */
+function repositoryNameOf(repository: string): string {
+  const name = repository.split("/").findLast((segment) => segment.length > 0);
+  if (name === undefined) return repository.trim();
+  try {
+    return decodeURIComponent(name);
+  } catch {
+    // A stray `%` is not an escape, so the name is whatever the remote spelled.
+    return name;
+  }
+}
+
 function statusArgs(state: PullRequestListState): ReadonlyArray<string> {
   switch (state) {
     case "open":
@@ -365,7 +395,8 @@ export const make = Effect.gen(function* () {
     listPullRequests: (input) =>
       listPullRequestPage({
         cwd: input.cwd,
-        repository: input.repository,
+        // Translated once, here, so every page of the walk asks for the same repository.
+        repository: repositoryNameOf(input.repository),
         state: input.state,
         involvement: input.involvement,
         viewer: input.viewer,
@@ -419,6 +450,8 @@ export const make = Effect.gen(function* () {
           "rest",
           "--method",
           "get",
+          "--resource",
+          AZURE_DEVOPS_RESOURCE_ID,
           "--url",
           `${input.threadsUrl}?api-version=${REST_API_VERSION}`,
         ],