fix(azure-devops): reviewer votes set review state, leave Reviewing, and show in activity

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

@@ -257,6 +257,120 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
     }),
   );
 
+  it.effect("leaves out of Reviewing what the viewer has voted on and still steps over it", () =>
+    Effect.gen(function* () {
+      const [voted, waiting, groupRequested] = pullRequestRows(3, 1);
+      const viewer = { displayName: "Bilal Hassan", uniqueName: "Bilal@acme.dev" };
+      mockedExecute.mockReturnValueOnce(
+        Effect.succeed(
+          output(
+            JSON.stringify([
+              { ...voted, reviewers: [{ ...viewer, vote: 10 }] },
+              { ...waiting, reviewers: [{ ...viewer, vote: 0 }] },
+              {
+                ...groupRequested,
+                reviewers: [{ displayName: "[web]\\Reviewers", uniqueName: "web-reviewers" }],
+              },
+            ]),
+          ),
+        ),
+      );
+      const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;
+
+      const batch = yield* cli.listPullRequests({
+        cwd: "/w",
+        repository: "web",
+        state: "open",
+        involvement: "reviewing",
+        viewer: "bilal@acme.dev",
+        limit: 2,
+      });
+
+      expect(batch.items.map((item) => item.number)).toEqual([2, 3]);
+      assert.isTrue(batch.truncated);
+      assert.strictEqual(batch.cursorAdvance, 3);
+    }),
+  );
+
+  it.effect("stops reading Reviewing pages of voted pull requests once it has one to show", () =>
+    Effect.gen(function* () {
+      const viewer = { displayName: "Bilal Hassan", uniqueName: "bilal@acme.dev" };
+      const page = (count: number, firstNumber: number, pendingNumbers: ReadonlyArray<number>) =>
+        Effect.succeed(
+          output(
+            JSON.stringify(
+              pullRequestRows(count, firstNumber).map((row) => ({
+                ...row,
+                reviewers: [
+                  { ...viewer, vote: pendingNumbers.includes(Number(row.pullRequestId)) ? 0 : 10 },
+                ],
+              })),
+            ),
+          ),
+        );
+      // Once one row is kept, `--top` asks for one fewer.
+      mockedExecute
+        .mockReturnValueOnce(page(3, 1, [1]))
+        .mockReturnValueOnce(page(2, 4, []))
+        .mockReturnValueOnce(page(2, 6, []))
+        .mockReturnValueOnce(page(2, 8, [8]));
+      const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;
+
+      const batch = yield* cli.listPullRequests({
+        cwd: "/w",
+        repository: "web",
+        state: "all",
+        involvement: "reviewing",
+        viewer: "bilal@acme.dev",
+        limit: 2,
+      });
+
+      assert.strictEqual(mockedExecute.mock.calls.length, 3);
+      expect(batch.items.map((item) => item.number)).toEqual([1]);
+      assert.isTrue(batch.truncated);
+      assert.strictEqual(batch.cursorAdvance, 7);
+    }),
+  );
+
+  it.effect("keeps reading past voted Reviewing pages until it finds one to show", () =>
+    Effect.gen(function* () {
+      const viewer = { displayName: "Bilal Hassan", uniqueName: "bilal@acme.dev" };
+      const page = (count: number, firstNumber: number, pendingNumbers: ReadonlyArray<number>) =>
+        Effect.succeed(
+          output(
+            JSON.stringify(
+              pullRequestRows(count, firstNumber).map((row) => ({
+                ...row,
+                reviewers: [
+                  { ...viewer, vote: pendingNumbers.includes(Number(row.pullRequestId)) ? 0 : 10 },
+                ],
+              })),
+            ),
+          ),
+        );
+      mockedExecute
+        .mockReturnValueOnce(page(3, 1, []))
+        .mockReturnValueOnce(page(3, 4, []))
+        .mockReturnValueOnce(page(3, 7, []))
+        .mockReturnValueOnce(page(3, 10, [10]));
+      const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;
+
+      const batch = yield* cli.listPullRequests({
+        cwd: "/w",
+        repository: "web",
+        state: "all",
+        involvement: "reviewing",
+        viewer: "bilal@acme.dev",
+        limit: 2,
+      });
+
+      assert.strictEqual(mockedExecute.mock.calls.length, 4);
+      expect(batch.items.map((item) => item.number)).toEqual([10]);
+      assert.isTrue(batch.truncated);
+      assert.strictEqual(batch.cursorAdvance, 12);
+    }),
+  );
+
   it.effect("reads the signed-in account, which az reports as a bare value", () =>
     Effect.gen(function* () {
       mockedExecute.mockReturnValueOnce(

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

@@ -24,6 +24,7 @@ import {
   type AzureDevOpsItemContent,
   type AzureDevOpsIteration,
   type AzureDevOpsPullRequest,
+  type AzureDevOpsPullRequestBatch,
   type AzureDevOpsRepositoryLocation,
 } from "./azureDevOpsPullRequestJson.ts";
 import type { ProviderListCursor } from "@t3tools/source-control-core/server/PullRequestProvider";
@@ -104,6 +105,11 @@ export type AzureDevOpsPullRequestCliError =
 
 const REST_API_VERSION = "7.1";
 const PULL_REQUEST_LIST_MAX_OUTPUT_BYTES = 16 * 1024 * 1024;
+/**
+ * Reviewing drops rows the viewer voted on; bound the reads one listing spends skipping them. The
+ * bound waits for one row, since an empty page gives the list service no cursor to resume from.
+ */
+const PULL_REQUEST_LIST_MAX_READS = 3;
 /** A 2,000-file page exceeds the default 1 MiB output budget. */
 const CHANGE_ENTRIES_MAX_OUTPUT_BYTES = 8 * 1024 * 1024;
 /** JSON escaping and base64 can exceed the decoded file's size. */
@@ -161,6 +167,7 @@ export class AzureDevOpsPullRequestCli extends Context.Service<
       readonly cwd: string;
       readonly location: AzureDevOpsRepositoryLocation;
       readonly number: number;
+      readonly reviewersById: AzureDevOpsPullRequest["reviewersById"];
     }) => Effect.Effect<ReadonlyArray<PullRequestComment>, AzureDevOpsPullRequestCliError>;
 
     readonly listIterations: (input: {
@@ -234,6 +241,26 @@ function involvementArgs(input: {
   }
 }
 
+/** `--reviewer` keeps a pull request after the viewer votes on it; only an unvoted one waits on them. */
+function withoutVotedByViewer(
+  batch: AzureDevOpsPullRequestBatch,
+  viewer: string,
+): AzureDevOpsPullRequestBatch {
+  const login = viewer.toLowerCase();
+  const isViewer = (candidate: string) => candidate.toLowerCase() === login;
+  const kept = batch.items.flatMap((item, index) =>
+    item.reviewers.some((reviewer) => isViewer(reviewer.login)) &&
+    !item.reviewRequestLogins.some(isViewer)
+      ? []
+      : [{ item, rawIndex: batch.rawIndexes[index] ?? 0 }],
+  );
+  return {
+    items: kept.map((entry) => entry.item),
+    rawIndexes: kept.map((entry) => entry.rawIndex),
+    rawCount: batch.rawCount,
+  };
+}
+
 function actionArgs(
   action: PullRequestAction,
   mergeMethod: PullRequestMergeMethod | undefined,
@@ -363,6 +390,7 @@ export const make = Effect.gen(function* () {
     readonly limit: number;
     readonly skip: number;
     readonly cursorAdvance: number;
+    readonly reads: number;
     readonly items: ReadonlyArray<AzureDevOpsPullRequest>;
   }): Effect.Effect<
     {
@@ -415,29 +443,37 @@ export const make = Effect.gen(function* () {
             }),
           );
         }
+        const batch =
+          input.involvement === "reviewing"
+            ? withoutVotedByViewer(decoded.success, input.viewer)
+            : decoded.success;
 
-        const lastItemIndex = decoded.success.rawIndexes[remaining - 1];
+        const lastItemIndex = batch.rawIndexes[remaining - 1];
         if (lastItemIndex !== undefined) {
           const consumed = lastItemIndex + 1;
           return Effect.succeed({
-            items: [...input.items, ...decoded.success.items.slice(0, remaining)],
-            truncated: consumed < decoded.success.rawCount || decoded.success.rawCount === top,
+            items: [...input.items, ...batch.items.slice(0, remaining)],
+            truncated: consumed < batch.rawCount || batch.rawCount === top,
             cursorAdvance: input.cursorAdvance + consumed,
           });
         }
 
-        const items = [...input.items, ...decoded.success.items];
-        if (decoded.success.rawCount < top) {
+        const items = [...input.items, ...batch.items];
+        if (
+          batch.rawCount < top ||
+          (items.length > 0 && input.reads + 1 >= PULL_REQUEST_LIST_MAX_READS)
+        ) {
           return Effect.succeed({
             items,
-            truncated: false,
-            cursorAdvance: input.cursorAdvance + decoded.success.rawCount,
+            truncated: batch.rawCount === top,
+            cursorAdvance: input.cursorAdvance + batch.rawCount,
           });
         }
         return listPullRequestPage({
           ...input,
-          skip: input.skip + decoded.success.rawCount,
-          cursorAdvance: input.cursorAdvance + decoded.success.rawCount,
+          skip: input.skip + batch.rawCount,
+          cursorAdvance: input.cursorAdvance + batch.rawCount,
+          reads: input.reads + 1,
           items,
         });
       }),
@@ -509,6 +545,7 @@ export const make = Effect.gen(function* () {
         limit: input.limit,
         skip: input.cursor?.delivered ?? 0,
         cursorAdvance: 0,
+        reads: 0,
         items: [],
       });
     }),
@@ -552,7 +589,7 @@ export const make = Effect.gen(function* () {
         resource: "pullRequestThreads",
         routeParameters: pullRequestRoute(input),
         maxOutputBytes: REVIEW_HISTORY_MAX_OUTPUT_BYTES,
-        decode: decodeThreadsJson,
+        decode: (raw) => decodeThreadsJson(raw, input.reviewersById),
       });
     }),
 

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

@@ -35,6 +35,8 @@ const PULL_REQUEST = {
   body: "",
   reviewRequestLogins: [],
   reviewers: [],
+  reviewersById: new Map(),
+  reviewDecision: null,
   location: { project: "acme", repository: "web" },
   autoMergeEnabled: false,
 };

packages/source-control-azure-devops/src/server/AzureDevOpsPullRequestProvider.ts

@@ -110,6 +110,7 @@ function toChangeRequest(pullRequest: AzureDevOpsPullRequest): ProviderChangeReq
     mergedAt: pullRequest.state === "merged" ? pullRequest.closedAt : null,
     updatedAt: pullRequest.updatedAt,
     reviewRequestLogins: pullRequest.reviewRequestLogins,
+    reviewDecision: pullRequest.reviewDecision,
     // Azure keeps labels on work items rather than on the pull request.
     labels: [],
   };
@@ -129,6 +130,7 @@ function toSummary(pullRequest: AzureDevOpsPullRequest): ProviderChangeRequestSu
     closedAt: pullRequest.state === "closed" ? pullRequest.closedAt : null,
     mergedAt: pullRequest.state === "merged" ? pullRequest.closedAt : null,
     updatedAt: pullRequest.updatedAt,
+    reviewDecision: pullRequest.reviewDecision,
   };
 }
 
@@ -368,6 +370,7 @@ export const make = Effect.gen(function* () {
                   cwd: input.cwd,
                   location: pullRequest.location,
                   number: input.number,
+                  reviewersById: pullRequest.reviewersById,
                 })
                 .pipe(
                   Effect.map((comments) => ({ comments, truncated: false })),