feat(gitlab): reviews can request changes

packages/source-control-gitlab/src/server/GitLabPullRequestCli.test.ts

@@ -1421,6 +1421,50 @@ layer("GitLabPullRequestCli.layer", (it) => {
     }),
   );
 
+  it.effect("requests changes as a reviewer and reports what GitLab refused", () =>
+    Effect.gen(function* () {
+      mockedExecute
+        .mockReturnValueOnce(Effect.succeed(output(JSON.stringify({ username: "bilal" }))))
+        .mockReturnValueOnce(
+          Effect.succeed(
+            output(JSON.stringify({ data: { mergeRequestSetReviewers: { errors: [] } } })),
+          ),
+        )
+        .mockReturnValueOnce(
+          Effect.succeed(
+            output(
+              JSON.stringify({
+                data: { mergeRequestRequestChanges: { errors: ["Merge request is locked"] } },
+              }),
+            ),
+          ),
+        );
+      const cli = yield* GitLabPullRequestCli.GitLabPullRequestCli;
+
+      const error = yield* Effect.flip(
+        cli.submitReview({
+          cwd: "/w",
+          repository: "acme/web",
+          number: 7,
+          verdict: "request-changes",
+          body: "Needs work",
+          comments: [{ path: "src/a.ts", position: { kind: "added", newLine: 4 }, body: "nit" }],
+        }),
+      );
+
+      expect(mockedExecute).toHaveBeenCalledTimes(3);
+      const target = { projectPath: "acme/web", iid: "7" };
+      const setReviewers = JSON.parse(callAt(1).stdin ?? "");
+      expect(setReviewers.query).toContain("operationMode: APPEND");
+      expect(setReviewers.variables).toEqual({ ...target, reviewers: ["bilal"] });
+      const requestChanges = JSON.parse(callAt(2).stdin ?? "");
+      expect(requestChanges.query).toContain("mergeRequestRequestChanges");
+      expect(requestChanges.variables).toEqual(target);
+      assert.strictEqual(error._tag, "GitLabMutationRefusedError");
+      expect(error.detail).toBe("GitLab refused it: Merge request is locked");
+    }),
+  );
+
   it.effect("removes an award by listing them and deleting the reader's own id", () =>
     Effect.gen(function* () {
       mockedExecute.mockReturnValueOnce(

packages/source-control-gitlab/src/server/GitLabPullRequestCli.ts

@@ -51,6 +51,7 @@ import {
   decodeMergeRequestListJson,
   decodeMergeRequestFileStatsJson,
   decodeMergeRequestStatsJson,
+  decodeMutationErrorsJson,
   decodeNotesJson,
   decodeOwnAwardIdJson,
   decodePipelineStageChecksJson,
@@ -204,6 +205,34 @@ export class GitLabDiffFileContentsUnavailableError extends Schema.TaggedError<G
   }
 }
 
+export class GitLabMutationRefusedError extends Schema.TaggedError<GitLabMutationRefusedError>()(
+  "GitLabMutationRefusedError",
+  {
+    command: Schema.Literal("glab"),
+    cwd: Schema.String,
+    operation: Schema.String,
+    reasons: Schema.Array(Schema.String),
+  },
+) {
+  get detail(): string {
+    return `GitLab refused it: ${this.reasons.join("; ")}`;
+  }
+
+  override get message(): string {
+    return `GitLab CLI failed in ${this.operation}: ${this.detail}`;
+  }
+}
+
+const SET_SELF_AS_REVIEWER_MUTATION = `mutation($projectPath: ID!, $iid: String!, $reviewers: [String!]!) {
+  mergeRequestSetReviewers(
+    input: { projectPath: $projectPath, iid: $iid, reviewerUsernames: $reviewers, operationMode: APPEND }
+  ) { errors }
+}`;
+
+const REQUEST_CHANGES_MUTATION = `mutation($projectPath: ID!, $iid: String!) {
+  mergeRequestRequestChanges(input: { projectPath: $projectPath, iid: $iid }) { errors }
+}`;
+
 export class GitLabBranchDeletionRefusedError extends Schema.TaggedError<GitLabBranchDeletionRefusedError>()(
   "GitLabBranchDeletionRefusedError",
   {
@@ -266,7 +295,8 @@ export type GitLabPullRequestCliError =
   | GitLabDiffCommitParentUnavailableError
   | GitLabDiffFileContentsUnavailableError
   | GitLabDiffRefsUnavailableError
-  | GitLabViewerUnavailableError;
+  | GitLabViewerUnavailableError
+  | GitLabMutationRefusedError;
 
 /** GitLab's `per_page` limit. */
 const MAX_PAGE_SIZE = 100;
@@ -1452,6 +1482,28 @@ export const make = Effect.gen(function* () {
       }),
     );
 
+  const graphqlMutation = (input: {
+    readonly cwd: string;
+    readonly host?: string | undefined;
+    readonly operation: string;
+    readonly query: string;
+    readonly variables: Record<string, unknown>;
+  }) =>
+    graphqlRead({ ...input, decode: decodeMutationErrorsJson }).pipe(
+      Effect.flatMap((reasons) =>
+        reasons.length === 0
+          ? Effect.void
+          : Effect.fail(
+              new GitLabMutationRefusedError({
+                command: "glab",
+                cwd: input.cwd,
+                operation: input.operation,
+                reasons,
+              }),
+            ),
+      ),
+    );
+
   const graphqlListPage = (input: {
     readonly cwd: string;
     readonly host?: string | undefined;
@@ -2335,10 +2387,29 @@ export const make = Effect.gen(function* () {
     submitReview: Effect.fn("GitLabPullRequestCli.submitReview")(function* (input) {
       const project = projectPath(input.repository);
       const mergeRequest = `projects/${project}/merge_requests/${input.number}`;
-      // GitLab has no pending review to attach comments to, so a review is replayed as the
-      // requests it is made of: the line comments, then the summary, then the verdict. A
-      // failure part-way therefore leaves what was already posted in place, which is why
-      // the verdict goes last — a half-sent review is never an approval.
+      // GitLab has no pending review, so a review is replayed as separate requests and a failure
+      // leaves earlier ones posted. Requesting changes (GitLab 18.9+) goes first so an older
+      // server fails before any comment lands; approval goes last so a half-sent review never
+      // approves.
+      if (input.verdict === "request-changes") {
+        // GitLab keeps the verdict on the reviewer, so the viewer joins the reviewers first.
+        const viewer = yield* viewerUsername(input);
+        const target = { projectPath: input.repository, iid: String(input.number) };
+        yield* graphqlMutation({
+          cwd: input.cwd,
+          host: input.host,
+          operation: "submitReview",
+          query: SET_SELF_AS_REVIEWER_MUTATION,
+          variables: { ...target, reviewers: [viewer] },
+        });
+        yield* graphqlMutation({
+          cwd: input.cwd,
+          host: input.host,
+          operation: "submitReview",
+          query: REQUEST_CHANGES_MUTATION,
+          variables: target,
+        });
+      }
       if (input.comments.length > 0) {
         const refs = yield* getDiffRefs(input);
         yield* Effect.forEach(

packages/source-control-gitlab/src/server/GitLabPullRequestProvider.test.ts

@@ -143,7 +143,7 @@ describe("gitLabViewerPermissions", () => {
       ],
       comment: true,
       resolve: true,
-      verdicts: ["comment", "approve"],
+      verdicts: ["comment", "approve", "request-changes"],
       requestReviewers: true,
       labels: true,
       updateMethods: ["rebase"],
@@ -155,7 +155,7 @@ describe("gitLabViewerPermissions", () => {
       actions: ["ready", "draft", "close", "reopen"],
       comment: true,
       resolve: true,
-      verdicts: ["comment", "approve"],
+      verdicts: ["comment", "approve", "request-changes"],
       requestReviewers: true,
       labels: false,
     });

packages/source-control-gitlab/src/server/GitLabPullRequestProvider.ts

@@ -38,8 +38,7 @@ const CAPABILITIES: PullRequestCapabilities = {
     inlineComment: true,
     reply: true,
     resolve: true,
-    // GitLab has no requested-changes verdict.
-    verdicts: ["comment", "approve"],
+    verdicts: ["comment", "approve", "request-changes"],
   },
   reviewers: { request: true, listCandidates: true },
   labels: true,

packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts

@@ -1712,6 +1712,28 @@ export function decodeGraphqlListSupportJson(raw: string): Result.Result<boolean
   return Result.succeed(major > 17 || (major === 17 && minor >= 9));
 }
 
+const decodeMutationPayloads = decodeJsonResult(
+  Schema.Struct({
+    data: Schema.Record(
+      Schema.String,
+      Schema.NullOr(
+        Schema.Struct({ errors: Schema.optional(Schema.NullOr(Schema.Array(Schema.String))) }),
+      ),
+    ),
+  }),
+);
+
+/** GitLab answers a refused mutation with HTTP 200 and the reasons in each payload's `errors`. */
+export function decodeMutationErrorsJson(
+  raw: string,
+): Result.Result<ReadonlyArray<string>, DecodeFailure> {
+  const decoded = decodeMutationPayloads(raw);
+  if (!Result.isSuccess(decoded)) return Result.fail(decoded.failure);
+  return Result.succeed(
+    Object.values(decoded.success.data).flatMap((payload) => payload?.errors ?? []),
+  );
+}
+
 /**
  * Line counts for merge requests of one project. The REST listing carries no line counts, and
  * reading each diff to count them would cost a request per row, so GraphQL's per-merge-request