fix(gitlab): address bot review nits on #93

- templates/droid-review.yml: remove top-level `stages:` block so the
  Component doesn't leak its stage list into the consuming project.
  Job-level `stage: $[[ inputs.stage ]]` is unchanged; consumers add
  the stage to their own `stages:` list (or omit it for the default).
- templates/droid-review.yml: tighten droid_action_ref docstring to
  "tag/branch" since the clone uses `git clone --branch` (SHAs would
  silently fail).
- src/gitlab/token.ts: drop CI_JOB_TOKEN fallback. It was always set
  in CI so the MissingGitlabTokenError never fired, producing
  confusing 401s on first API call instead of a clear "set GITLAB_TOKEN"
  message. CI_JOB_TOKEN also lacks scopes for notes/discussions.
- src/mcp/gitlab-mr-server.ts: require a line anchor on ReviewComment
  (line for RIGHT, old_line for LEFT) via .refine(); GitLab rejects
  unanchored diff discussions.
- src/mcp/gitlab-mr-server.ts: remove `.max(30)` cap on comments so
  large MRs don't fail the entire submit_review call.

Tests: 445/445 pass; tsc clean.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

Author: factory-nizar

src/gitlab/token.ts

@@ -10,10 +10,7 @@ export class MissingGitlabTokenError extends Error {
 }
 
 export function setupGitlabToken(): string {
-  const token =
-    process.env.GITLAB_TOKEN ||
-    process.env.OVERRIDE_GITLAB_TOKEN ||
-    process.env.CI_JOB_TOKEN;
+  const token = process.env.GITLAB_TOKEN || process.env.OVERRIDE_GITLAB_TOKEN;
 
   if (!token) {
     throw new MissingGitlabTokenError();

src/mcp/gitlab-mr-server.ts

@@ -24,37 +24,51 @@ function errorResult(error: unknown) {
   };
 }
 
-const ReviewCommentSchema = z.object({
-  path: z
-    .string()
-    .describe(
-      "Path of the file to comment on (use the new_path from the diff)",
-    ),
-  body: z.string().min(1).describe("Comment text (supports markdown)"),
-  line: z
-    .number()
-    .int()
-    .optional()
-    .describe(
-      "Line number in the new file for single-line comments, or end line for multi-line",
-    ),
-  side: z
-    .enum(["LEFT", "RIGHT"])
-    .optional()
-    .default("RIGHT")
-    .describe(
-      "Side of the diff: RIGHT for new/modified code, LEFT for removed code",
-    ),
-  old_path: z
-    .string()
-    .optional()
-    .describe("Path in the old file (defaults to path if unset)"),
-  old_line: z
-    .number()
-    .int()
-    .optional()
-    .describe("Line number in the old file (required when side=LEFT)"),
-});
+const ReviewCommentSchema = z
+  .object({
+    path: z
+      .string()
+      .describe(
+        "Path of the file to comment on (use the new_path from the diff)",
+      ),
+    body: z.string().min(1).describe("Comment text (supports markdown)"),
+    line: z
+      .number()
+      .int()
+      .optional()
+      .describe(
+        "Line number in the new file. Required for side=RIGHT (the default).",
+      ),
+    side: z
+      .enum(["LEFT", "RIGHT"])
+      .optional()
+      .default("RIGHT")
+      .describe(
+        "Side of the diff: RIGHT for new/modified code, LEFT for removed code",
+      ),
+    old_path: z
+      .string()
+      .optional()
+      .describe("Path in the old file (defaults to path if unset)"),
+    old_line: z
+      .number()
+      .int()
+      .optional()
+      .describe(
+        "Line number in the old file. Required for side=LEFT comments.",
+      ),
+  })
+  .refine(
+    (c) => {
+      const side = c.side ?? "RIGHT";
+      if (side === "LEFT") return typeof c.old_line === "number";
+      return typeof c.line === "number";
+    },
+    {
+      message:
+        "Inline diff discussions require a line anchor: provide `line` for side=RIGHT comments, or `old_line` for side=LEFT comments.",
+    },
+  );
 
 type ReviewComment = z.infer<typeof ReviewCommentSchema>;
 
@@ -247,9 +261,8 @@ export function createGitlabMrServer({
         .describe("Optional summary note body in markdown"),
       comments: z
         .array(ReviewCommentSchema)
-        .max(30)
         .optional()
-        .describe("Inline review comments (max 30 per call)"),
+        .describe("Inline review comments"),
     },
     async ({ mr_iid, body, comments }) => {
       try {

templates/droid-review.yml

@@ -73,17 +73,18 @@ spec:
       default: "https://gitlab.com/factory-components/droid-action.git"
     droid_action_ref:
       description: |
-        Git ref (tag/branch/sha) of droid-action to clone at runtime.
-        The same ref SHOULD match the `include: remote:` URL above.
+        Git tag or branch of droid-action to clone at runtime (SHAs are
+        not supported because the clone uses `--branch`). The same ref
+        SHOULD match the `include: remote:` URL above.
         Internal: most users leave this at the default.
       default: "main"
     stage:
-      description: "GitLab CI stage to assign the droid-review job to."
+      description: |
+        GitLab CI stage to assign the droid-review job to. The consuming
+        project's `.gitlab-ci.yml` must include this stage in its
+        `stages:` list (or omit the list to use the default ordering).
       default: "test"
 ---
-stages:
-  - $[[ inputs.stage ]]
-
 droid-review:
   stage: $[[ inputs.stage ]]
   image: oven/bun:1.2.11

test/gitlab/token.test.ts

@@ -39,9 +39,9 @@ describe("setupGitlabToken", () => {
     expect(setupGitlabToken()).toBe("glpat-override");
   });
 
-  it("falls back to CI_JOB_TOKEN", () => {
+  it("does NOT fall back to CI_JOB_TOKEN (its scopes are insufficient for notes/discussions)", () => {
     process.env.CI_JOB_TOKEN = "ci-job-token";
-    expect(setupGitlabToken()).toBe("ci-job-token");
+    expect(() => setupGitlabToken()).toThrow(MissingGitlabTokenError);
   });
 
   it("throws MissingGitlabTokenError when nothing is set", () => {