From 4ea33ad7a1c4f91ca381d9f7cac4996756f6cb0a Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 13:19:19 -0700 Subject: [PATCH 01/29] feat(gitlab): add hello-world CI/CD template to validate include:remote Adds a minimal gitlab/templates/review.yml job that prints a hello-world message. Used to verify that a GitLab CI pipeline can pull the template over HTTPS from raw.githubusercontent.com before we layer in the Docker image and real review entrypoints. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- gitlab/templates/review.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 gitlab/templates/review.yml diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml new file mode 100644 index 0000000..25dbf1d --- /dev/null +++ b/gitlab/templates/review.yml @@ -0,0 +1,14 @@ +# GitLab CI/CD template for droid-action review. +# Hello-world placeholder to validate `include: remote:` works across GitHub -> GitLab. +# Real implementation lands in subsequent commits on feat/gitlab-support. + +droid-review: + stage: test + image: alpine:3.20 + rules: + - if: $CI_PIPELINE_SOURCE == "merge_request_event" + - if: $CI_PIPELINE_SOURCE == "push" + - if: $CI_PIPELINE_SOURCE == "web" + script: + - echo "hello from droid-action" + - echo "Loaded from Factory-AI/droid-action via include:remote" From 64a787093e53841802f080659392f7f050645075 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 13:28:39 -0700 Subject: [PATCH 02/29] feat(gitlab): scaffold src/gitlab adapter (context, token, api, types) Pure-addition first slice of GitLab support. Mirrors the shape of src/github/ so subsequent entrypoints can swap adapter by PLATFORM env. - src/gitlab/context.ts parses CI_* env vars into a normalized ParsedGitlabContext with MR + commit + user + inputs - src/gitlab/token.ts reads GITLAB_TOKEN with OVERRIDE/CI_JOB_TOKEN fallbacks; throws a clear error when missing - src/gitlab/api/client.ts is a thin fetch wrapper around GitLab v4 exposing getMr/getMrChanges/listNotes/createNote/updateNote/ createDiscussionOnDiff/updateMrDescription - src/gitlab/types.ts defines GitlabMr, GitlabNote, GitlabDiscussion, GitlabPosition - 16 new unit tests; full existing suite (381 -> 397) still passes No GitHub code paths touched. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- src/gitlab/api/client.ts | 156 ++++++++++++++++++++++++++++++ src/gitlab/api/config.ts | 7 ++ src/gitlab/context.ts | 146 ++++++++++++++++++++++++++++ src/gitlab/token.ts | 23 +++++ src/gitlab/types.ts | 81 ++++++++++++++++ test/gitlab/api-client.test.ts | 109 +++++++++++++++++++++ test/gitlab/context.test.ts | 168 +++++++++++++++++++++++++++++++++ test/gitlab/token.test.ts | 50 ++++++++++ 8 files changed, 740 insertions(+) create mode 100644 src/gitlab/api/client.ts create mode 100644 src/gitlab/api/config.ts create mode 100644 src/gitlab/context.ts create mode 100644 src/gitlab/token.ts create mode 100644 src/gitlab/types.ts create mode 100644 test/gitlab/api-client.test.ts create mode 100644 test/gitlab/context.test.ts create mode 100644 test/gitlab/token.test.ts diff --git a/src/gitlab/api/client.ts b/src/gitlab/api/client.ts new file mode 100644 index 0000000..0747d4e --- /dev/null +++ b/src/gitlab/api/client.ts @@ -0,0 +1,156 @@ +import { GITLAB_API_URL } from "./config"; +import type { + GitlabMr, + GitlabMrChanges, + GitlabNote, + GitlabDiscussion, + GitlabPosition, +} from "../types"; + +export class GitlabApiError extends Error { + status: number; + body: unknown; + + constructor(status: number, message: string, body: unknown) { + super(`GitLab API ${status}: ${message}`); + this.name = "GitlabApiError"; + this.status = status; + this.body = body; + } +} + +type RequestInitWithJson = Omit & { json?: unknown }; + +export class GitlabClient { + private readonly baseUrl: string; + private readonly token: string; + + constructor(token: string, baseUrl: string = GITLAB_API_URL) { + this.token = token; + this.baseUrl = baseUrl.replace(/\/+$/, ""); + } + + private async request( + path: string, + init: RequestInitWithJson = {}, + ): Promise { + const { json, headers: rawHeaders, ...rest } = init; + const headers: Record = { + "PRIVATE-TOKEN": this.token, + Accept: "application/json", + ...(rawHeaders as Record | undefined), + }; + + let body: string | undefined; + if (json !== undefined) { + headers["Content-Type"] = "application/json"; + body = JSON.stringify(json); + } + + const url = path.startsWith("http") ? path : `${this.baseUrl}${path}`; + const response = await fetch(url, { + ...rest, + headers, + body, + }); + + if (!response.ok) { + let parsed: unknown = null; + const text = await response.text(); + try { + parsed = text ? JSON.parse(text) : null; + } catch { + parsed = text; + } + throw new GitlabApiError( + response.status, + response.statusText || "Request failed", + parsed, + ); + } + + if (response.status === 204) { + return undefined as unknown as T; + } + + return (await response.json()) as T; + } + + private projectPath(projectId: string | number): string { + return `/projects/${encodeURIComponent(String(projectId))}`; + } + + getMr(projectId: string | number, mrIid: number): Promise { + return this.request( + `${this.projectPath(projectId)}/merge_requests/${mrIid}`, + ); + } + + getMrChanges( + projectId: string | number, + mrIid: number, + ): Promise { + return this.request( + `${this.projectPath(projectId)}/merge_requests/${mrIid}/changes`, + ); + } + + listNotes(projectId: string | number, mrIid: number): Promise { + return this.request( + `${this.projectPath(projectId)}/merge_requests/${mrIid}/notes?per_page=100`, + ); + } + + createNote( + projectId: string | number, + mrIid: number, + body: string, + ): Promise { + return this.request( + `${this.projectPath(projectId)}/merge_requests/${mrIid}/notes`, + { method: "POST", json: { body } }, + ); + } + + updateNote( + projectId: string | number, + mrIid: number, + noteId: number, + body: string, + ): Promise { + return this.request( + `${this.projectPath(projectId)}/merge_requests/${mrIid}/notes/${noteId}`, + { method: "PUT", json: { body } }, + ); + } + + createDiscussionOnDiff( + projectId: string | number, + mrIid: number, + body: string, + position: GitlabPosition, + ): Promise { + return this.request( + `${this.projectPath(projectId)}/merge_requests/${mrIid}/discussions`, + { method: "POST", json: { body, position } }, + ); + } + + updateMrDescription( + projectId: string | number, + mrIid: number, + description: string, + ): Promise { + return this.request( + `${this.projectPath(projectId)}/merge_requests/${mrIid}`, + { method: "PUT", json: { description } }, + ); + } +} + +export function createGitlabClient( + token: string, + baseUrl?: string, +): GitlabClient { + return new GitlabClient(token, baseUrl); +} diff --git a/src/gitlab/api/config.ts b/src/gitlab/api/config.ts new file mode 100644 index 0000000..24c18a4 --- /dev/null +++ b/src/gitlab/api/config.ts @@ -0,0 +1,7 @@ +export const GITLAB_API_URL = ( + process.env.CI_API_V4_URL || "https://gitlab.com/api/v4" +).replace(/\/+$/, ""); + +export const GITLAB_SERVER_URL = ( + process.env.CI_SERVER_URL || "https://gitlab.com" +).replace(/\/+$/, ""); diff --git a/src/gitlab/context.ts b/src/gitlab/context.ts new file mode 100644 index 0000000..87a60f2 --- /dev/null +++ b/src/gitlab/context.ts @@ -0,0 +1,146 @@ +export type ParsedGitlabContext = { + serverUrl: string; + apiUrl: string; + pipelineId: string | null; + pipelineUrl: string | null; + jobId: string | null; + jobUrl: string | null; + pipelineSource: string | null; + project: { + id: string; + path: string; + pathWithNamespace: string; + webUrl: string; + }; + mr: { + iid: number; + sourceBranchSha: string | null; + targetBranchSha: string | null; + diffBaseSha: string | null; + title: string | null; + } | null; + commit: { + sha: string; + branch: string | null; + tag: string | null; + }; + user: { + login: string | null; + name: string | null; + email: string | null; + }; + inputs: { + automaticReview: boolean; + automaticSecurityReview: boolean; + triggerPhrase: string; + reviewDepth: string; + reviewModel: string; + reasoningEffort: string; + fillModel: string; + securityModel: string; + securitySeverityThreshold: string; + securityBlockOnCritical: boolean; + securityBlockOnHigh: boolean; + securityNotifyTeam: string; + securityScanSchedule: boolean; + securityScanDays: number; + }; +}; + +function required(name: string): string { + const value = process.env[name]; + if (!value) { + throw new Error( + `Missing required GitLab CI environment variable: ${name}. Are you running inside a GitLab CI job?`, + ); + } + return value; +} + +function optional(name: string): string | null { + const value = process.env[name]; + return value && value.length > 0 ? value : null; +} + +export function parseGitlabContext(): ParsedGitlabContext { + const serverUrl = (process.env.CI_SERVER_URL || "https://gitlab.com").replace( + /\/+$/, + "", + ); + const apiUrl = (process.env.CI_API_V4_URL || `${serverUrl}/api/v4`).replace( + /\/+$/, + "", + ); + + const projectId = required("CI_PROJECT_ID"); + const projectPath = required("CI_PROJECT_PATH"); + const projectName = + process.env.CI_PROJECT_NAME || projectPath.split("/").pop()!; + const projectUrl = + process.env.CI_PROJECT_URL || `${serverUrl}/${projectPath}`; + + const mrIid = optional("CI_MERGE_REQUEST_IID"); + const mr = mrIid + ? { + iid: parseInt(mrIid, 10), + sourceBranchSha: optional("CI_MERGE_REQUEST_SOURCE_BRANCH_SHA"), + targetBranchSha: optional("CI_MERGE_REQUEST_TARGET_BRANCH_SHA"), + diffBaseSha: optional("CI_MERGE_REQUEST_DIFF_BASE_SHA"), + title: optional("CI_MERGE_REQUEST_TITLE"), + } + : null; + + return { + serverUrl, + apiUrl, + pipelineId: optional("CI_PIPELINE_ID"), + pipelineUrl: optional("CI_PIPELINE_URL"), + jobId: optional("CI_JOB_ID"), + jobUrl: optional("CI_JOB_URL"), + pipelineSource: optional("CI_PIPELINE_SOURCE"), + project: { + id: projectId, + path: projectName, + pathWithNamespace: projectPath, + webUrl: projectUrl, + }, + mr, + commit: { + sha: required("CI_COMMIT_SHA"), + branch: optional("CI_COMMIT_BRANCH"), + tag: optional("CI_COMMIT_TAG"), + }, + user: { + login: optional("GITLAB_USER_LOGIN"), + name: optional("GITLAB_USER_NAME"), + email: optional("GITLAB_USER_EMAIL"), + }, + inputs: { + automaticReview: process.env.AUTOMATIC_REVIEW === "true", + automaticSecurityReview: process.env.AUTOMATIC_SECURITY_REVIEW === "true", + triggerPhrase: process.env.TRIGGER_PHRASE ?? "@droid", + reviewDepth: process.env.REVIEW_DEPTH ?? "deep", + reviewModel: process.env.REVIEW_MODEL ?? "", + reasoningEffort: process.env.REASONING_EFFORT ?? "", + fillModel: process.env.FILL_MODEL ?? "", + securityModel: process.env.SECURITY_MODEL ?? "", + securitySeverityThreshold: + process.env.SECURITY_SEVERITY_THRESHOLD ?? "medium", + securityBlockOnCritical: + process.env.SECURITY_BLOCK_ON_CRITICAL !== "false", + securityBlockOnHigh: process.env.SECURITY_BLOCK_ON_HIGH === "true", + securityNotifyTeam: process.env.SECURITY_NOTIFY_TEAM ?? "", + securityScanSchedule: process.env.SECURITY_SCAN_SCHEDULE === "true", + securityScanDays: Math.max( + 1, + parseInt(process.env.SECURITY_SCAN_DAYS ?? "7", 10) || 7, + ), + }, + }; +} + +export function isMergeRequestContext( + ctx: ParsedGitlabContext, +): ctx is ParsedGitlabContext & { mr: NonNullable } { + return ctx.mr !== null; +} diff --git a/src/gitlab/token.ts b/src/gitlab/token.ts new file mode 100644 index 0000000..cbbb681 --- /dev/null +++ b/src/gitlab/token.ts @@ -0,0 +1,23 @@ +#!/usr/bin/env bun + +export class MissingGitlabTokenError extends Error { + constructor() { + super( + "Missing GITLAB_TOKEN. Set a GitLab Project or Group access token with `api` scope as a masked CI/CD variable named GITLAB_TOKEN.", + ); + this.name = "MissingGitlabTokenError"; + } +} + +export function setupGitlabToken(): string { + const token = + process.env.GITLAB_TOKEN || + process.env.OVERRIDE_GITLAB_TOKEN || + process.env.CI_JOB_TOKEN; + + if (!token) { + throw new MissingGitlabTokenError(); + } + + return token; +} diff --git a/src/gitlab/types.ts b/src/gitlab/types.ts new file mode 100644 index 0000000..15a6449 --- /dev/null +++ b/src/gitlab/types.ts @@ -0,0 +1,81 @@ +export type GitlabUser = { + id: number; + username: string; + name?: string; +}; + +export type GitlabMr = { + id: number; + iid: number; + project_id: number; + title: string; + description: string | null; + state: string; + author: GitlabUser; + source_branch: string; + target_branch: string; + source_project_id: number; + target_project_id: number; + draft: boolean; + work_in_progress: boolean; + diff_refs: { + base_sha: string; + head_sha: string; + start_sha: string; + }; + web_url: string; + created_at: string; + updated_at: string; +}; + +export type GitlabMrDiff = { + old_path: string; + new_path: string; + a_mode: string; + b_mode: string; + diff: string; + new_file: boolean; + renamed_file: boolean; + deleted_file: boolean; +}; + +export type GitlabMrChanges = { + changes: GitlabMrDiff[]; + diff_refs: GitlabMr["diff_refs"]; +}; + +export type GitlabNote = { + id: number; + type: string | null; + body: string; + author: GitlabUser; + created_at: string; + updated_at: string; + system: boolean; + noteable_id: number; + noteable_iid: number; + noteable_type: string; + resolvable: boolean; + resolved?: boolean; +}; + +export type GitlabDiscussion = { + id: string; + individual_note: boolean; + notes: GitlabNote[]; +}; + +export type GitlabPosition = { + base_sha: string; + start_sha: string; + head_sha: string; + position_type: "text"; + new_path: string; + new_line?: number; + old_path?: string; + old_line?: number; + line_range?: { + start: { line_code: string; type: "new" | "old" }; + end: { line_code: string; type: "new" | "old" }; + }; +}; diff --git a/test/gitlab/api-client.test.ts b/test/gitlab/api-client.test.ts new file mode 100644 index 0000000..927084f --- /dev/null +++ b/test/gitlab/api-client.test.ts @@ -0,0 +1,109 @@ +import { describe, expect, it, beforeEach, afterEach, mock } from "bun:test"; +import { GitlabClient, GitlabApiError } from "../../src/gitlab/api/client"; + +const ORIGINAL_FETCH = globalThis.fetch; + +function mockFetch(impl: (url: string, init: RequestInit) => Response) { + const fn = mock(async (url: string, init: RequestInit) => impl(url, init)); + // @ts-expect-error overriding global fetch in tests + globalThis.fetch = fn; + return fn; +} + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +describe("GitlabClient", () => { + beforeEach(() => { + // each test installs its own fetch mock + }); + + afterEach(() => { + globalThis.fetch = ORIGINAL_FETCH; + }); + + it("sends PRIVATE-TOKEN header on GET", async () => { + const fetchMock = mockFetch((url, init) => { + expect(url).toBe( + "https://gitlab.com/api/v4/projects/42/merge_requests/7", + ); + expect(init.method ?? "GET").toBe("GET"); + const headers = init.headers as Record; + expect(headers["PRIVATE-TOKEN"]).toBe("glpat-test"); + return jsonResponse({ iid: 7 }); + }); + + const client = new GitlabClient("glpat-test"); + const mr = await client.getMr(42, 7); + expect(mr).toEqual({ iid: 7 } as never); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("sends JSON body on POST createNote", async () => { + mockFetch((url, init) => { + expect(url).toBe( + "https://gitlab.com/api/v4/projects/42/merge_requests/7/notes", + ); + expect(init.method).toBe("POST"); + expect(init.body).toBe(JSON.stringify({ body: "hello" })); + const headers = init.headers as Record; + expect(headers["Content-Type"]).toBe("application/json"); + return jsonResponse({ id: 1, body: "hello" }); + }); + + const client = new GitlabClient("glpat-test"); + const note = await client.createNote(42, 7, "hello"); + expect((note as { id: number }).id).toBe(1); + }); + + it("URL-encodes string project ids (path-with-namespace)", async () => { + mockFetch((url) => { + expect(url).toBe( + "https://gitlab.com/api/v4/projects/group%2Fsub%2Frepo/merge_requests/7", + ); + return jsonResponse({ iid: 7 }); + }); + const client = new GitlabClient("glpat-test"); + await client.getMr("group/sub/repo", 7); + }); + + it("respects a custom baseUrl (self-hosted)", async () => { + mockFetch((url) => { + expect(url).toBe( + "https://gitlab.example.com/api/v4/projects/1/merge_requests/2", + ); + return jsonResponse({ iid: 2 }); + }); + const client = new GitlabClient( + "glpat-test", + "https://gitlab.example.com/api/v4", + ); + await client.getMr(1, 2); + }); + + it("throws GitlabApiError with parsed body on non-2xx", async () => { + mockFetch( + () => + new Response(JSON.stringify({ message: "404 Not Found" }), { + status: 404, + statusText: "Not Found", + headers: { "Content-Type": "application/json" }, + }), + ); + + const client = new GitlabClient("glpat-test"); + try { + await client.getMr(42, 999); + throw new Error("expected throw"); + } catch (err) { + expect(err).toBeInstanceOf(GitlabApiError); + const e = err as GitlabApiError; + expect(e.status).toBe(404); + expect((e.body as { message: string }).message).toBe("404 Not Found"); + } + }); +}); diff --git a/test/gitlab/context.test.ts b/test/gitlab/context.test.ts new file mode 100644 index 0000000..a605e63 --- /dev/null +++ b/test/gitlab/context.test.ts @@ -0,0 +1,168 @@ +import { describe, expect, it, beforeEach, afterEach } from "bun:test"; +import { + parseGitlabContext, + isMergeRequestContext, +} from "../../src/gitlab/context"; + +const REQUIRED_CI_ENV = [ + "CI_SERVER_URL", + "CI_API_V4_URL", + "CI_PROJECT_ID", + "CI_PROJECT_PATH", + "CI_PROJECT_NAME", + "CI_PROJECT_URL", + "CI_COMMIT_SHA", + "CI_COMMIT_BRANCH", + "CI_COMMIT_TAG", + "CI_PIPELINE_ID", + "CI_PIPELINE_URL", + "CI_PIPELINE_SOURCE", + "CI_JOB_ID", + "CI_JOB_URL", + "CI_MERGE_REQUEST_IID", + "CI_MERGE_REQUEST_SOURCE_BRANCH_SHA", + "CI_MERGE_REQUEST_TARGET_BRANCH_SHA", + "CI_MERGE_REQUEST_DIFF_BASE_SHA", + "CI_MERGE_REQUEST_TITLE", + "GITLAB_USER_LOGIN", + "GITLAB_USER_NAME", + "GITLAB_USER_EMAIL", + "AUTOMATIC_REVIEW", + "AUTOMATIC_SECURITY_REVIEW", + "TRIGGER_PHRASE", + "REVIEW_DEPTH", + "REVIEW_MODEL", + "REASONING_EFFORT", + "FILL_MODEL", + "SECURITY_MODEL", + "SECURITY_SEVERITY_THRESHOLD", + "SECURITY_BLOCK_ON_CRITICAL", + "SECURITY_BLOCK_ON_HIGH", + "SECURITY_NOTIFY_TEAM", + "SECURITY_SCAN_SCHEDULE", + "SECURITY_SCAN_DAYS", +]; + +function clearEnv() { + for (const key of REQUIRED_CI_ENV) { + delete process.env[key]; + } +} + +describe("parseGitlabContext", () => { + const originalEnv: Record = {}; + + beforeEach(() => { + for (const key of REQUIRED_CI_ENV) { + originalEnv[key] = process.env[key]; + } + clearEnv(); + }); + + afterEach(() => { + clearEnv(); + for (const key of Object.keys(originalEnv)) { + if (originalEnv[key] !== undefined) { + process.env[key] = originalEnv[key]; + } + } + }); + + it("parses a typical MR pipeline context", () => { + process.env.CI_SERVER_URL = "https://gitlab.com"; + process.env.CI_API_V4_URL = "https://gitlab.com/api/v4"; + process.env.CI_PROJECT_ID = "12345"; + process.env.CI_PROJECT_PATH = "group/sub/repo-name"; + process.env.CI_PROJECT_NAME = "repo-name"; + process.env.CI_PROJECT_URL = "https://gitlab.com/group/sub/repo-name"; + process.env.CI_COMMIT_SHA = "abc123"; + process.env.CI_COMMIT_BRANCH = "feature/x"; + process.env.CI_PIPELINE_SOURCE = "merge_request_event"; + process.env.CI_PIPELINE_ID = "999"; + process.env.CI_PIPELINE_URL = + "https://gitlab.com/group/sub/repo-name/-/pipelines/999"; + process.env.CI_MERGE_REQUEST_IID = "42"; + process.env.CI_MERGE_REQUEST_SOURCE_BRANCH_SHA = "src-sha"; + process.env.CI_MERGE_REQUEST_TARGET_BRANCH_SHA = "tgt-sha"; + process.env.CI_MERGE_REQUEST_DIFF_BASE_SHA = "base-sha"; + process.env.CI_MERGE_REQUEST_TITLE = "Add feature x"; + process.env.AUTOMATIC_REVIEW = "true"; + + const ctx = parseGitlabContext(); + + expect(ctx.serverUrl).toBe("https://gitlab.com"); + expect(ctx.apiUrl).toBe("https://gitlab.com/api/v4"); + expect(ctx.project.id).toBe("12345"); + expect(ctx.project.pathWithNamespace).toBe("group/sub/repo-name"); + expect(ctx.commit.sha).toBe("abc123"); + expect(ctx.pipelineSource).toBe("merge_request_event"); + expect(isMergeRequestContext(ctx)).toBe(true); + expect(ctx.mr?.iid).toBe(42); + expect(ctx.mr?.diffBaseSha).toBe("base-sha"); + expect(ctx.inputs.automaticReview).toBe(true); + expect(ctx.inputs.reviewDepth).toBe("deep"); + expect(ctx.inputs.securityScanDays).toBe(7); + }); + + it("parses a push-event context with no MR", () => { + process.env.CI_PROJECT_ID = "1"; + process.env.CI_PROJECT_PATH = "g/r"; + process.env.CI_COMMIT_SHA = "deadbeef"; + process.env.CI_PIPELINE_SOURCE = "push"; + + const ctx = parseGitlabContext(); + + expect(ctx.mr).toBeNull(); + expect(isMergeRequestContext(ctx)).toBe(false); + }); + + it("trims trailing slashes from server and api urls", () => { + process.env.CI_SERVER_URL = "https://gitlab.example.com///"; + process.env.CI_API_V4_URL = "https://gitlab.example.com/api/v4///"; + process.env.CI_PROJECT_ID = "1"; + process.env.CI_PROJECT_PATH = "g/r"; + process.env.CI_COMMIT_SHA = "x"; + + const ctx = parseGitlabContext(); + + expect(ctx.serverUrl).toBe("https://gitlab.example.com"); + expect(ctx.apiUrl).toBe("https://gitlab.example.com/api/v4"); + }); + + it("falls back to derived api url when CI_API_V4_URL is unset", () => { + process.env.CI_SERVER_URL = "https://self-hosted.example.com"; + process.env.CI_PROJECT_ID = "1"; + process.env.CI_PROJECT_PATH = "g/r"; + process.env.CI_COMMIT_SHA = "x"; + + const ctx = parseGitlabContext(); + + expect(ctx.apiUrl).toBe("https://self-hosted.example.com/api/v4"); + }); + + it("throws when CI_PROJECT_ID is missing", () => { + process.env.CI_PROJECT_PATH = "g/r"; + process.env.CI_COMMIT_SHA = "x"; + expect(() => parseGitlabContext()).toThrow(/CI_PROJECT_ID/); + }); + + it("clamps negative securityScanDays to at least 1", () => { + process.env.CI_PROJECT_ID = "1"; + process.env.CI_PROJECT_PATH = "g/r"; + process.env.CI_COMMIT_SHA = "x"; + process.env.SECURITY_SCAN_DAYS = "-3"; + + const ctx = parseGitlabContext(); + expect(ctx.inputs.securityScanDays).toBe(1); + }); + + it("falls back to 7 days when securityScanDays is invalid", () => { + process.env.CI_PROJECT_ID = "1"; + process.env.CI_PROJECT_PATH = "g/r"; + process.env.CI_COMMIT_SHA = "x"; + process.env.SECURITY_SCAN_DAYS = "not-a-number"; + + const ctx = parseGitlabContext(); + expect(ctx.inputs.securityScanDays).toBe(7); + }); +}); diff --git a/test/gitlab/token.test.ts b/test/gitlab/token.test.ts new file mode 100644 index 0000000..9cd5a49 --- /dev/null +++ b/test/gitlab/token.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it, beforeEach, afterEach } from "bun:test"; +import { + setupGitlabToken, + MissingGitlabTokenError, +} from "../../src/gitlab/token"; + +const KEYS = ["GITLAB_TOKEN", "OVERRIDE_GITLAB_TOKEN", "CI_JOB_TOKEN"]; + +describe("setupGitlabToken", () => { + const original: Record = {}; + + beforeEach(() => { + for (const k of KEYS) { + original[k] = process.env[k]; + delete process.env[k]; + } + }); + + afterEach(() => { + for (const k of KEYS) { + if (original[k] !== undefined) { + process.env[k] = original[k]; + } else { + delete process.env[k]; + } + } + }); + + it("prefers GITLAB_TOKEN", () => { + process.env.GITLAB_TOKEN = "glpat-primary"; + process.env.OVERRIDE_GITLAB_TOKEN = "glpat-override"; + process.env.CI_JOB_TOKEN = "ci-job-token"; + expect(setupGitlabToken()).toBe("glpat-primary"); + }); + + it("falls back to OVERRIDE_GITLAB_TOKEN", () => { + process.env.OVERRIDE_GITLAB_TOKEN = "glpat-override"; + process.env.CI_JOB_TOKEN = "ci-job-token"; + expect(setupGitlabToken()).toBe("glpat-override"); + }); + + it("falls back to CI_JOB_TOKEN", () => { + process.env.CI_JOB_TOKEN = "ci-job-token"; + expect(setupGitlabToken()).toBe("ci-job-token"); + }); + + it("throws MissingGitlabTokenError when nothing is set", () => { + expect(() => setupGitlabToken()).toThrow(MissingGitlabTokenError); + }); +}); From 8176251fad942bf4ddbd9dddad492746758b8503 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 13:30:54 -0700 Subject: [PATCH 03/29] feat(gitlab): add MCP server exposing MR operations to droid exec src/mcp/gitlab-mr-server.ts mirrors src/mcp/github-pr-server.ts in shape so the review prompt can stay platform-agnostic at the tool layer. Tools registered: - get_mr / get_mr_changes / list_mr_notes - create_mr_note / update_mr_note (sticky tracking comment lifecycle) - update_mr_description (for @droid fill) - submit_review: posts an optional summary note plus N inline discussions anchored to diff positions; per-comment errors are collected so one bad position doesn't abort the batch. Position objects are built from the MR's diff_refs (base/head/start SHA) and switch new_line vs old_line based on RIGHT vs LEFT side, matching the GitHub PR review tool's contract. 5 new unit tests cover summary, inline anchoring, LEFT-side mapping, and partial-failure batching. Full suite: 402/402. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- src/mcp/gitlab-mr-server.ts | 313 ++++++++++++++++++++++++++++++ test/mcp/gitlab-mr-server.test.ts | 154 +++++++++++++++ 2 files changed, 467 insertions(+) create mode 100644 src/mcp/gitlab-mr-server.ts create mode 100644 test/mcp/gitlab-mr-server.test.ts diff --git a/src/mcp/gitlab-mr-server.ts b/src/mcp/gitlab-mr-server.ts new file mode 100644 index 0000000..1adc503 --- /dev/null +++ b/src/mcp/gitlab-mr-server.ts @@ -0,0 +1,313 @@ +#!/usr/bin/env node + +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"; +import { z } from "zod"; +import { GitlabClient } from "../gitlab/api/client"; +import type { GitlabPosition } from "../gitlab/types"; + +export interface GitlabServerDependencies { + projectId: string; + client: GitlabClient; +} + +function textResult(text: string) { + return { content: [{ type: "text" as const, text }] }; +} + +function errorResult(error: unknown) { + const message = error instanceof Error ? error.message : String(error); + return { + content: [{ type: "text" as const, text: `Error: ${message}` }], + error: message, + isError: true, + }; +} + +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)"), +}); + +type ReviewComment = z.infer; + +function buildPosition( + comment: ReviewComment, + diffRefs: { base_sha: string; head_sha: string; start_sha: string }, +): GitlabPosition { + const newPath = comment.path; + const oldPath = comment.old_path ?? comment.path; + + const position: GitlabPosition = { + base_sha: diffRefs.base_sha, + start_sha: diffRefs.start_sha, + head_sha: diffRefs.head_sha, + position_type: "text", + new_path: newPath, + old_path: oldPath, + }; + + if (comment.side === "LEFT") { + if (typeof comment.old_line === "number") { + position.old_line = comment.old_line; + } else if (typeof comment.line === "number") { + position.old_line = comment.line; + } + } else { + if (typeof comment.line === "number") { + position.new_line = comment.line; + } + if (typeof comment.old_line === "number") { + position.old_line = comment.old_line; + } + } + + return position; +} + +export function createGitlabMrServer({ + projectId, + client, +}: GitlabServerDependencies) { + const server = new McpServer({ + name: "GitLab MR Server", + version: "0.0.1", + }); + + server.tool( + "get_mr", + "Fetch merge request metadata including diff_refs (base/head/start SHAs)", + { + mr_iid: z.number().int().describe("Merge request IID to fetch"), + }, + async ({ mr_iid }) => { + try { + const mr = await client.getMr(projectId, mr_iid); + return textResult(JSON.stringify(mr)); + } catch (error) { + return errorResult(error); + } + }, + ); + + server.tool( + "get_mr_changes", + "Fetch the file-level diff for a merge request", + { + mr_iid: z.number().int().describe("Merge request IID"), + }, + async ({ mr_iid }) => { + try { + const changes = await client.getMrChanges(projectId, mr_iid); + return textResult(JSON.stringify(changes)); + } catch (error) { + return errorResult(error); + } + }, + ); + + server.tool( + "list_mr_notes", + "List notes (comments) on a merge request", + { + mr_iid: z.number().int().describe("Merge request IID"), + }, + async ({ mr_iid }) => { + try { + const notes = await client.listNotes(projectId, mr_iid); + return textResult(JSON.stringify(notes)); + } catch (error) { + return errorResult(error); + } + }, + ); + + server.tool( + "create_mr_note", + "Post a top-level note (summary comment) on a merge request", + { + mr_iid: z.number().int().describe("Merge request IID"), + body: z.string().min(1).describe("Note body in markdown"), + }, + async ({ mr_iid, body }) => { + try { + const note = await client.createNote(projectId, mr_iid, body); + return textResult(`Created note ${note.id} on MR !${mr_iid}`); + } catch (error) { + return errorResult(error); + } + }, + ); + + server.tool( + "update_mr_note", + "Edit the body of an existing note (used for the sticky tracking comment)", + { + mr_iid: z.number().int().describe("Merge request IID"), + note_id: z.number().int().describe("Note ID to update"), + body: z.string().min(1).describe("New note body in markdown"), + }, + async ({ mr_iid, note_id, body }) => { + try { + await client.updateNote(projectId, mr_iid, note_id, body); + return textResult(`Updated note ${note_id} on MR !${mr_iid}`); + } catch (error) { + return errorResult(error); + } + }, + ); + + server.tool( + "update_mr_description", + "Replace the description/body of a merge request", + { + mr_iid: z.number().int().describe("Merge request IID"), + description: z.string().describe("New description in markdown"), + }, + async ({ mr_iid, description }) => { + try { + await client.updateMrDescription(projectId, mr_iid, description); + return textResult(`Updated description for MR !${mr_iid}`); + } catch (error) { + return errorResult(error); + } + }, + ); + + server.tool( + "submit_review", + "Post an MR review: optional summary note plus zero or more inline " + + "discussions anchored to diff positions. Inline comments require the " + + "MR's current diff_refs (base/head/start SHAs).", + { + mr_iid: z.number().int().describe("Merge request IID to review"), + body: z + .string() + .optional() + .describe("Optional summary note body in markdown"), + comments: z + .array(ReviewCommentSchema) + .max(30) + .optional() + .describe("Inline review comments (max 30 per call)"), + }, + async ({ mr_iid, body, comments }) => { + try { + const summary = { + summaryNoteId: null as number | null, + discussionsCreated: 0, + discussionErrors: [] as Array<{ index: number; error: string }>, + }; + + if (body && body.trim().length > 0) { + const note = await client.createNote(projectId, mr_iid, body); + summary.summaryNoteId = note.id; + } + + if (comments && comments.length > 0) { + const mr = await client.getMr(projectId, mr_iid); + const diffRefs = mr.diff_refs; + if (!diffRefs) { + throw new Error( + "Merge request is missing diff_refs; cannot anchor inline comments", + ); + } + + for (let i = 0; i < comments.length; i++) { + const c = comments[i]!; + const position = buildPosition(c, diffRefs); + try { + await client.createDiscussionOnDiff( + projectId, + mr_iid, + c.body, + position, + ); + summary.discussionsCreated += 1; + } catch (error) { + const message = + error instanceof Error ? error.message : String(error); + summary.discussionErrors.push({ index: i, error: message }); + } + } + } + + return textResult(JSON.stringify(summary)); + } catch (error) { + return errorResult(error); + } + }, + ); + + return server; +} + +async function runServer() { + const projectId = process.env.CI_PROJECT_ID || process.env.GITLAB_PROJECT_ID; + const token = + process.env.GITLAB_TOKEN || + process.env.OVERRIDE_GITLAB_TOKEN || + process.env.CI_JOB_TOKEN; + const apiUrl = + process.env.CI_API_V4_URL || + process.env.GITLAB_API_URL || + "https://gitlab.com/api/v4"; + + if (!projectId) { + console.error( + "Error: CI_PROJECT_ID (or GITLAB_PROJECT_ID) environment variable is required", + ); + process.exit(1); + } + + if (!token) { + console.error( + "Error: GITLAB_TOKEN (or CI_JOB_TOKEN / OVERRIDE_GITLAB_TOKEN) environment variable is required", + ); + process.exit(1); + } + + const client = new GitlabClient(token, apiUrl); + const server = createGitlabMrServer({ projectId, client }); + + const transport = new StdioServerTransport(); + await server.connect(transport); + + process.on("exit", () => { + server.close(); + }); +} + +if (import.meta.main) { + runServer().catch((error) => { + console.error(error); + process.exit(1); + }); +} diff --git a/test/mcp/gitlab-mr-server.test.ts b/test/mcp/gitlab-mr-server.test.ts new file mode 100644 index 0000000..7d74092 --- /dev/null +++ b/test/mcp/gitlab-mr-server.test.ts @@ -0,0 +1,154 @@ +import { describe, expect, it, mock } from "bun:test"; +import { createGitlabMrServer } from "../../src/mcp/gitlab-mr-server"; +import { GitlabClient } from "../../src/gitlab/api/client"; + +function makeFakeClient(overrides: Partial = {}): GitlabClient { + const fake = { + getMr: mock(async () => ({ + iid: 7, + diff_refs: { + base_sha: "base-sha", + head_sha: "head-sha", + start_sha: "start-sha", + }, + })), + getMrChanges: mock(async () => ({ changes: [], diff_refs: {} })), + listNotes: mock(async () => []), + createNote: mock(async (_p: unknown, _m: unknown, body: string) => ({ + id: 101, + body, + })), + updateNote: mock(async () => ({ id: 101 })), + createDiscussionOnDiff: mock(async () => ({ + id: "disc-1", + individual_note: false, + notes: [], + })), + updateMrDescription: mock(async () => ({ iid: 7 })), + ...overrides, + } as unknown as GitlabClient; + return fake; +} + +function listTools(server: ReturnType) { + // McpServer keeps tools internally; we call _registeredTools via a small probe. + // Instead, exercise the public API by invoking a known tool name via internal handler. + return server; +} + +describe("createGitlabMrServer", () => { + it("registers without throwing and exposes a server instance", () => { + const client = makeFakeClient(); + const server = createGitlabMrServer({ projectId: "42", client }); + expect(server).toBeTruthy(); + listTools(server); + }); + + it("submit_review posts a summary note when body provided", async () => { + const client = makeFakeClient(); + const server = createGitlabMrServer({ projectId: "42", client }); + + // @ts-expect-error - reach into internal tool registry to invoke directly + const tool = server._registeredTools?.submit_review; + expect(tool).toBeTruthy(); + + const result = await tool.callback({ + mr_iid: 7, + body: "Hello reviewers", + comments: [], + }); + + expect(result.isError).toBeUndefined(); + expect( + (client.createNote as ReturnType).mock.calls.length, + ).toBe(1); + const payload = JSON.parse(result.content[0].text); + expect(payload.summaryNoteId).toBe(101); + expect(payload.discussionsCreated).toBe(0); + }); + + it("submit_review anchors inline comments using diff_refs", async () => { + const client = makeFakeClient(); + const server = createGitlabMrServer({ projectId: "42", client }); + // @ts-expect-error - reach into internal tool registry + const tool = server._registeredTools?.submit_review; + + const result = await tool.callback({ + mr_iid: 7, + comments: [ + { path: "src/x.ts", body: "Issue here", line: 12, side: "RIGHT" }, + ], + }); + + expect(result.isError).toBeUndefined(); + const createDisc = client.createDiscussionOnDiff as ReturnType; + expect(createDisc.mock.calls.length).toBe(1); + + const [, , , position] = createDisc.mock.calls[0]; + expect(position).toMatchObject({ + base_sha: "base-sha", + head_sha: "head-sha", + start_sha: "start-sha", + position_type: "text", + new_path: "src/x.ts", + old_path: "src/x.ts", + new_line: 12, + }); + }); + + it("submit_review uses old_line on LEFT side", async () => { + const client = makeFakeClient(); + const server = createGitlabMrServer({ projectId: "42", client }); + // @ts-expect-error + const tool = server._registeredTools?.submit_review; + + await tool.callback({ + mr_iid: 7, + comments: [ + { + path: "src/x.ts", + body: "Removed code issue", + line: 5, + side: "LEFT", + }, + ], + }); + + const createDisc = client.createDiscussionOnDiff as ReturnType; + const [, , , position] = createDisc.mock.calls[0]; + expect(position.old_line).toBe(5); + expect(position.new_line).toBeUndefined(); + }); + + it("submit_review collects per-comment errors without aborting", async () => { + const failingDisc = mock(async (_p, _m, body: string) => { + if (body.includes("bad")) { + throw new Error("position out of range"); + } + return { id: "disc", individual_note: false, notes: [] }; + }); + const client = makeFakeClient({ + createDiscussionOnDiff: failingDisc, + } as Partial); + const server = createGitlabMrServer({ projectId: "42", client }); + // @ts-expect-error + const tool = server._registeredTools?.submit_review; + + const result = await tool.callback({ + mr_iid: 7, + comments: [ + { path: "a.ts", body: "good", line: 1 }, + { path: "a.ts", body: "bad", line: 999 }, + { path: "a.ts", body: "good again", line: 2 }, + ], + }); + + const payload = JSON.parse(result.content[0].text); + expect(payload.discussionsCreated).toBe(2); + expect(payload.discussionErrors).toHaveLength(1); + expect(payload.discussionErrors[0].index).toBe(1); + expect(payload.discussionErrors[0].error).toContain( + "position out of range", + ); + }); +}); From d5e7b348f7458112bd4411bda535506b4ebe44da Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 13:34:12 -0700 Subject: [PATCH 04/29] feat(gitlab): add prepare + update-comment-link entrypoints and sticky-note logic Implements the sticky tracking note lifecycle for GitLab MR pipelines: prepare creates (or reuses) the running-state note before droid exec runs; update-comment-link rewrites it with success/failure + pipeline links after droid exec completes. - src/gitlab/operations/tracking-note.ts builds the note body with a hidden marker so retries find and update the same note instead of creating duplicates. Renders pipeline/job links, security badge (when automatic_security_review is on), and an error-details
block on failure. - src/entrypoints/gitlab-prepare.ts parses CI env, gates on merge_request_event + automatic_review, writes a JSON state file (DROID_STATE_FILE) for downstream steps. - src/entrypoints/gitlab-update-comment-link.ts reads the state file and PUTs the final body. Skips cleanly when no review ran. 7 new tracking-note tests; full suite 402 -> 409. Also tightened two gitlab-mr-server tests to satisfy noUncheckedIndexedAccess. Cross-SCM include:remote was validated end-to-end against gitlab.com pipeline #2568334623 (passed) before this commit. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- src/entrypoints/gitlab-prepare.ts | 141 ++++++++++++++++++ src/entrypoints/gitlab-update-comment-link.ts | 113 ++++++++++++++ src/gitlab/operations/tracking-note.ts | 72 +++++++++ test/gitlab/tracking-note.test.ts | 72 +++++++++ test/mcp/gitlab-mr-server.test.ts | 6 +- 5 files changed, 402 insertions(+), 2 deletions(-) create mode 100644 src/entrypoints/gitlab-prepare.ts create mode 100644 src/entrypoints/gitlab-update-comment-link.ts create mode 100644 src/gitlab/operations/tracking-note.ts create mode 100644 test/gitlab/tracking-note.test.ts diff --git a/src/entrypoints/gitlab-prepare.ts b/src/entrypoints/gitlab-prepare.ts new file mode 100644 index 0000000..9d7d554 --- /dev/null +++ b/src/entrypoints/gitlab-prepare.ts @@ -0,0 +1,141 @@ +#!/usr/bin/env bun + +/** + * Prepare step for the GitLab CI/CD Component. + * + * Responsibilities (v1, automatic review on MR pipelines only): + * 1. Parse GitLab CI env into a normalized context. + * 2. Decide whether this pipeline should run a review (automaticReview + * flag + merge_request_event source). + * 3. Ensure a sticky tracking note exists on the MR; reuse the existing + * one if a prior pipeline already created it. + * 4. Write a small JSON state file so the post-step (and `droid exec`) + * can look up the MR IID, note ID, and other context. + */ + +import * as fs from "fs/promises"; +import * as path from "path"; +import { parseGitlabContext, isMergeRequestContext } from "../gitlab/context"; +import { setupGitlabToken, MissingGitlabTokenError } from "../gitlab/token"; +import { GitlabClient } from "../gitlab/api/client"; +import { + buildTrackingNoteBody, + findExistingTrackingNote, +} from "../gitlab/operations/tracking-note"; + +type PrepareState = { + shouldRunReview: boolean; + projectId: string; + projectPath: string; + mrIid: number | null; + trackingNoteId: number | null; + diffBaseSha: string | null; + sourceBranchSha: string | null; + pipelineUrl: string | null; + jobUrl: string | null; + reason?: string; +}; + +function stateFilePath(): string { + return ( + process.env.DROID_STATE_FILE || + path.join(process.env.CI_PROJECT_DIR || "/tmp", ".droid-state.json") + ); +} + +async function writeState(state: PrepareState): Promise { + const filePath = stateFilePath(); + await fs.mkdir(path.dirname(filePath), { recursive: true }); + await fs.writeFile(filePath, JSON.stringify(state, null, 2)); + console.log(`Wrote droid state to ${filePath}`); +} + +async function run(): Promise { + const context = parseGitlabContext(); + + const baseState: PrepareState = { + shouldRunReview: false, + projectId: context.project.id, + projectPath: context.project.pathWithNamespace, + mrIid: context.mr?.iid ?? null, + trackingNoteId: null, + diffBaseSha: context.mr?.diffBaseSha ?? null, + sourceBranchSha: context.mr?.sourceBranchSha ?? null, + pipelineUrl: context.pipelineUrl, + jobUrl: context.jobUrl, + }; + + if (!isMergeRequestContext(context)) { + console.log( + "Not a merge_request_event pipeline; skipping droid review prepare.", + ); + await writeState({ + ...baseState, + reason: "not-merge-request-event", + }); + return; + } + + if (!context.inputs.automaticReview) { + console.log("automatic_review is disabled; skipping droid review prepare."); + await writeState({ + ...baseState, + reason: "automatic-review-disabled", + }); + return; + } + + let token: string; + try { + token = setupGitlabToken(); + } catch (err) { + if (err instanceof MissingGitlabTokenError) { + console.error(err.message); + } + throw err; + } + + const client = new GitlabClient(token, context.apiUrl); + const mrIid = context.mr.iid; + + const existingNotes = await client.listNotes(context.project.id, mrIid); + const existing = findExistingTrackingNote(existingNotes); + + const noteBody = buildTrackingNoteBody({ + state: "running", + pipelineUrl: context.pipelineUrl, + jobUrl: context.jobUrl, + triggerUsername: context.user.login, + securityReviewRan: context.inputs.automaticSecurityReview, + }); + + let trackingNoteId: number; + if (existing) { + await client.updateNote(context.project.id, mrIid, existing.id, noteBody); + trackingNoteId = existing.id; + console.log(`Reused existing tracking note ${trackingNoteId}`); + } else { + const created = await client.createNote( + context.project.id, + mrIid, + noteBody, + ); + trackingNoteId = created.id; + console.log(`Created tracking note ${trackingNoteId}`); + } + + await writeState({ + ...baseState, + shouldRunReview: true, + trackingNoteId, + }); +} + +if (import.meta.main) { + run().catch((error) => { + console.error("gitlab-prepare failed:", error); + process.exit(1); + }); +} + +export { run }; diff --git a/src/entrypoints/gitlab-update-comment-link.ts b/src/entrypoints/gitlab-update-comment-link.ts new file mode 100644 index 0000000..d4c951c --- /dev/null +++ b/src/entrypoints/gitlab-update-comment-link.ts @@ -0,0 +1,113 @@ +#!/usr/bin/env bun + +/** + * Post-step for the GitLab CI/CD Component: edit the sticky tracking note + * to reflect the final outcome (success/failure) and link to the pipeline. + * + * Inputs (env): + * GITLAB_TOKEN - access token (api scope) + * DROID_STATE_FILE - JSON state written by gitlab-prepare + * DROID_SUCCESS - "true" | "false" set by the CI job + * DROID_ERROR_DETAILS - optional error blob to embed on failure + * AUTOMATIC_SECURITY_REVIEW - "true" to render the security badge + * TRIGGER_USERNAME - optional, e.g. GITLAB_USER_LOGIN + * CI_PIPELINE_URL / CI_JOB_URL - used to keep links fresh + */ + +import * as fs from "fs/promises"; +import * as path from "path"; +import { setupGitlabToken } from "../gitlab/token"; +import { GitlabClient } from "../gitlab/api/client"; +import { buildTrackingNoteBody } from "../gitlab/operations/tracking-note"; + +type PrepareState = { + shouldRunReview: boolean; + projectId: string; + projectPath: string; + mrIid: number | null; + trackingNoteId: number | null; + pipelineUrl: string | null; + jobUrl: string | null; + reason?: string; +}; + +function stateFilePath(): string { + return ( + process.env.DROID_STATE_FILE || + path.join(process.env.CI_PROJECT_DIR || "/tmp", ".droid-state.json") + ); +} + +async function readState(): Promise { + const filePath = stateFilePath(); + try { + const raw = await fs.readFile(filePath, "utf8"); + return JSON.parse(raw) as PrepareState; + } catch (err) { + console.warn(`Could not read droid state file at ${filePath}:`, err); + return null; + } +} + +async function run(): Promise { + const state = await readState(); + if (!state) { + console.log("No droid state available; nothing to update."); + return; + } + + if (!state.shouldRunReview || !state.mrIid || !state.trackingNoteId) { + console.log( + `Skipping note update (shouldRunReview=${state.shouldRunReview}, ` + + `mrIid=${state.mrIid}, trackingNoteId=${state.trackingNoteId}).`, + ); + return; + } + + const token = setupGitlabToken(); + const apiUrl = + process.env.CI_API_V4_URL || + process.env.GITLAB_API_URL || + "https://gitlab.com/api/v4"; + + const client = new GitlabClient(token, apiUrl); + + const droidSuccess = process.env.DROID_SUCCESS !== "false"; + const errorDetails = process.env.DROID_ERROR_DETAILS || null; + const securityReviewRan = process.env.AUTOMATIC_SECURITY_REVIEW === "true"; + const triggerUsername = + process.env.TRIGGER_USERNAME || process.env.GITLAB_USER_LOGIN || null; + + const pipelineUrl = process.env.CI_PIPELINE_URL || state.pipelineUrl; + const jobUrl = process.env.CI_JOB_URL || state.jobUrl; + + const body = buildTrackingNoteBody({ + state: droidSuccess ? "success" : "failure", + pipelineUrl, + jobUrl, + triggerUsername, + errorDetails, + securityReviewRan, + }); + + await client.updateNote( + state.projectId, + state.mrIid, + state.trackingNoteId, + body, + ); + + console.log( + `Updated tracking note ${state.trackingNoteId} on MR !${state.mrIid} ` + + `(state=${droidSuccess ? "success" : "failure"}).`, + ); +} + +if (import.meta.main) { + run().catch((error) => { + console.error("gitlab-update-comment-link failed:", error); + process.exit(1); + }); +} + +export { run }; diff --git a/src/gitlab/operations/tracking-note.ts b/src/gitlab/operations/tracking-note.ts new file mode 100644 index 0000000..5566deb --- /dev/null +++ b/src/gitlab/operations/tracking-note.ts @@ -0,0 +1,72 @@ +/** + * Sticky tracking note helpers for GitLab MR pipelines. + * + * The tracking note carries a hidden HTML marker so we can find and + * update the same note across retries instead of creating duplicates. + */ + +export const DROID_TRACKING_MARKER = ""; +export const DROID_SECURITY_BADGE_MARKER = ""; + +export type TrackingNoteState = "running" | "success" | "failure"; + +export interface TrackingNoteOptions { + state: TrackingNoteState; + pipelineUrl?: string | null; + jobUrl?: string | null; + triggerUsername?: string | null; + errorDetails?: string | null; + securityReviewRan?: boolean; +} + +const SECURITY_BADGE = + "![security](https://img.shields.io/badge/security%20review-enabled-blue?style=flat-square&logo=shield) "; + +const STATE_HEADER: Record = { + running: + "**Droid is reviewing this merge request...** :hourglass_flowing_sand:", + success: "**Droid finished reviewing this merge request** :white_check_mark:", + failure: "**Droid encountered an error reviewing this MR** :x:", +}; + +export function buildTrackingNoteBody(options: TrackingNoteOptions): string { + const lines: string[] = []; + + if (options.securityReviewRan) { + lines.push(`${DROID_SECURITY_BADGE_MARKER}${SECURITY_BADGE}`); + } + + lines.push(DROID_TRACKING_MARKER); + lines.push(""); + lines.push(STATE_HEADER[options.state]); + lines.push(""); + + if (options.triggerUsername) { + lines.push(`Triggered by @${options.triggerUsername}.`); + } + + if (options.pipelineUrl) { + lines.push(`Pipeline: ${options.pipelineUrl}`); + } + if (options.jobUrl) { + lines.push(`Job log: ${options.jobUrl}`); + } + + if (options.state === "failure" && options.errorDetails) { + lines.push(""); + lines.push("
Error details"); + lines.push(""); + lines.push("```"); + lines.push(options.errorDetails.trim()); + lines.push("```"); + lines.push("
"); + } + + return lines.join("\n").trim() + "\n"; +} + +export function findExistingTrackingNote< + T extends { id: number; body: string }, +>(notes: T[]): T | undefined { + return notes.find((n) => n.body && n.body.includes(DROID_TRACKING_MARKER)); +} diff --git a/test/gitlab/tracking-note.test.ts b/test/gitlab/tracking-note.test.ts new file mode 100644 index 0000000..1bbc1cd --- /dev/null +++ b/test/gitlab/tracking-note.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "bun:test"; +import { + DROID_TRACKING_MARKER, + buildTrackingNoteBody, + findExistingTrackingNote, +} from "../../src/gitlab/operations/tracking-note"; + +describe("buildTrackingNoteBody", () => { + it("includes the tracking marker in every state", () => { + for (const state of ["running", "success", "failure"] as const) { + const body = buildTrackingNoteBody({ state }); + expect(body).toContain(DROID_TRACKING_MARKER); + } + }); + + it("renders pipeline + job links when provided", () => { + const body = buildTrackingNoteBody({ + state: "running", + pipelineUrl: "https://gitlab.com/p/-/pipelines/1", + jobUrl: "https://gitlab.com/p/-/jobs/2", + }); + expect(body).toContain("Pipeline: https://gitlab.com/p/-/pipelines/1"); + expect(body).toContain("Job log: https://gitlab.com/p/-/jobs/2"); + }); + + it("renders security badge when securityReviewRan is true", () => { + const body = buildTrackingNoteBody({ + state: "running", + securityReviewRan: true, + }); + expect(body).toContain("security%20review-enabled"); + }); + + it("omits security badge when securityReviewRan is false", () => { + const body = buildTrackingNoteBody({ + state: "running", + securityReviewRan: false, + }); + expect(body).not.toContain("security%20review-enabled"); + }); + + it("embeds error details only on failure state", () => { + const failure = buildTrackingNoteBody({ + state: "failure", + errorDetails: "boom", + }); + expect(failure).toContain("
"); + expect(failure).toContain("boom"); + + const success = buildTrackingNoteBody({ + state: "success", + errorDetails: "boom", + }); + expect(success).not.toContain("
"); + }); +}); + +describe("findExistingTrackingNote", () => { + it("finds the note containing the droid marker", () => { + const notes = [ + { id: 1, body: "regular comment" }, + { id: 2, body: `${DROID_TRACKING_MARKER}\nDroid is reviewing...` }, + { id: 3, body: "another comment" }, + ]; + expect(findExistingTrackingNote(notes)?.id).toBe(2); + }); + + it("returns undefined when no tracking note exists", () => { + const notes = [{ id: 1, body: "regular comment" }]; + expect(findExistingTrackingNote(notes)).toBeUndefined(); + }); +}); diff --git a/test/mcp/gitlab-mr-server.test.ts b/test/mcp/gitlab-mr-server.test.ts index 7d74092..879decd 100644 --- a/test/mcp/gitlab-mr-server.test.ts +++ b/test/mcp/gitlab-mr-server.test.ts @@ -84,7 +84,8 @@ describe("createGitlabMrServer", () => { const createDisc = client.createDiscussionOnDiff as ReturnType; expect(createDisc.mock.calls.length).toBe(1); - const [, , , position] = createDisc.mock.calls[0]; + const call = createDisc.mock.calls[0]!; + const position = call[3]; expect(position).toMatchObject({ base_sha: "base-sha", head_sha: "head-sha", @@ -115,7 +116,8 @@ describe("createGitlabMrServer", () => { }); const createDisc = client.createDiscussionOnDiff as ReturnType; - const [, , , position] = createDisc.mock.calls[0]; + const call = createDisc.mock.calls[0]!; + const position = call[3]; expect(position.old_line).toBe(5); expect(position.new_line).toBeUndefined(); }); From 03ce3175eb8c2fd1de5a8746a2cddb26fad22fe0 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 13:37:45 -0700 Subject: [PATCH 05/29] feat(gitlab): real CI/CD Component template (runtime-clone, no Docker) Replaces the hello-world include-test template with a real droid-review job. No Docker image dependency for v1; instead the job: 1. Uses the public oven/bun:1.2.11 image (Bun preinstalled) 2. apt-get installs git/curl/ca-certificates on top 3. Shallow-clones Factory-AI/droid-action at $DROID_ACTION_REF 4. bun install --frozen-lockfile 5. Runs src/entrypoints/gitlab-prepare.ts (creates sticky tracking note) 6. droid exec placeholder (real prompt + MCP wiring next commit) 7. after_script always runs src/entrypoints/gitlab-update-comment-link.ts so failures still rewrite the note to the failure state Template uses GitLab's spec.inputs so it works equally well via `include: component:` (post-Catalog publish) and `include: remote:` (today). Inputs: automatic_review, review_depth, review_model, reasoning_effort, droid_action_repo, droid_action_ref, stage. Required CI variables on the consumer side: FACTORY_API_KEY, GITLAB_TOKEN. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- gitlab/templates/review.yml | 76 ++++++++++++++++++++++++++++++++----- 1 file changed, 66 insertions(+), 10 deletions(-) diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index 25dbf1d..2aefc32 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -1,14 +1,70 @@ -# GitLab CI/CD template for droid-action review. -# Hello-world placeholder to validate `include: remote:` works across GitHub -> GitLab. -# Real implementation lands in subsequent commits on feat/gitlab-support. +spec: + inputs: + automatic_review: + description: "Run code review automatically on every MR event." + default: "true" + review_depth: + description: "Review depth preset: `shallow` (fast) or `deep` (thorough)." + default: "deep" + review_model: + description: "Override the model used for code review. Empty = use depth preset." + default: "" + reasoning_effort: + description: "Override reasoning effort. Empty = use depth preset." + default: "" + droid_action_repo: + description: | + Git repo (clone URL) of droid-action. Override if you mirror it + privately. Must be reachable from your GitLab runner. + default: "https://github.com/Factory-AI/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. + default: "dev" + stage: + description: "GitLab CI stage to assign the droid-review job to." + default: "test" +--- +stages: + - $[[ inputs.stage ]] droid-review: - stage: test - image: alpine:3.20 + stage: $[[ inputs.stage ]] + image: oven/bun:1.2.11 rules: - - if: $CI_PIPELINE_SOURCE == "merge_request_event" - - if: $CI_PIPELINE_SOURCE == "push" - - if: $CI_PIPELINE_SOURCE == "web" + - if: '$CI_PIPELINE_SOURCE == "merge_request_event"' + variables: + DROID_STATE_FILE: "$CI_PROJECT_DIR/.droid-state.json" + DROID_ACTION_DIR: "/tmp/droid-action" + DROID_ACTION_REPO: "$[[ inputs.droid_action_repo ]]" + DROID_ACTION_REF: "$[[ inputs.droid_action_ref ]]" + AUTOMATIC_REVIEW: "$[[ inputs.automatic_review ]]" + REVIEW_DEPTH: "$[[ inputs.review_depth ]]" + REVIEW_MODEL: "$[[ inputs.review_model ]]" + REASONING_EFFORT: "$[[ inputs.reasoning_effort ]]" + DROID_SUCCESS: "true" + before_script: + - apt-get update -qq + - apt-get install -y -qq --no-install-recommends git ca-certificates curl + - git clone --depth 1 --branch "$DROID_ACTION_REF" "$DROID_ACTION_REPO" "$DROID_ACTION_DIR" + - cd "$DROID_ACTION_DIR" && bun install --frozen-lockfile + - cd "$CI_PROJECT_DIR" script: - - echo "hello from droid-action" - - echo "Loaded from Factory-AI/droid-action via include:remote" + - | + if ! bun run "$DROID_ACTION_DIR/src/entrypoints/gitlab-prepare.ts"; then + export DROID_SUCCESS=false + export DROID_ERROR_DETAILS="gitlab-prepare exited non-zero" + exit 1 + fi + # NOTE: Real `droid exec` invocation lands in a follow-up commit + # once the GitLab review prompt + MCP wiring are in place. For now + # the prepare/post pair alone exercises the full sticky-note lifecycle. + - echo "droid exec placeholder - prompt + MCP wiring lands next." + after_script: + - bun run "$DROID_ACTION_DIR/src/entrypoints/gitlab-update-comment-link.ts" || true + artifacts: + when: always + paths: + - .droid-state.json + expire_in: 1 week From ab7c5f05764b127fea078cd07623b1634ac91c15 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 14:03:16 -0700 Subject: [PATCH 06/29] feat(gitlab): wire up real droid exec with MCP server + inline review The template now installs the Droid CLI, registers the gitlab-mr MCP server with the runtime CI env, and runs `droid exec` against a review prompt that gitlab-prepare writes to /tmp/droid-prompts/droid-prompt.txt. - src/gitlab/review-prompt.ts: minimal v1 review prompt builder embedding the MR diff and instructions to call submit_review with a single batched set of inline comments (max 8). Returns LGTM body when no issues found. - src/entrypoints/gitlab-prepare.ts: after creating the sticky note, fetches MR + diff via the GitLab API and writes the prompt file; exposes promptPath on the state JSON. - gitlab/templates/review.yml: * Installs Droid CLI via curl https://app.factory.ai/cli | sh * Skips droid exec if prepare set shouldRunReview=false * Registers MCP server: droid mcp add gitlab_mr --env ... * Runs: droid exec -f $DROID_PROMPT_FILE --output-format stream-json --skip-permissions-unsafe (plus optional --model/--reasoning-effort) * after_script reads /tmp/droid-error.txt for failure details - 3 new prompt-builder tests; suite at 412 passing. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- gitlab/templates/review.yml | 35 +++++++++++++++--- src/entrypoints/gitlab-prepare.ts | 32 ++++++++++++++++ src/gitlab/review-prompt.ts | 61 +++++++++++++++++++++++++++++++ test/gitlab/review-prompt.test.ts | 45 +++++++++++++++++++++++ 4 files changed, 168 insertions(+), 5 deletions(-) create mode 100644 src/gitlab/review-prompt.ts create mode 100644 test/gitlab/review-prompt.test.ts diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index 2aefc32..a626153 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -36,6 +36,7 @@ droid-review: - if: '$CI_PIPELINE_SOURCE == "merge_request_event"' variables: DROID_STATE_FILE: "$CI_PROJECT_DIR/.droid-state.json" + DROID_PROMPT_FILE: "/tmp/droid-prompts/droid-prompt.txt" DROID_ACTION_DIR: "/tmp/droid-action" DROID_ACTION_REPO: "$[[ inputs.droid_action_repo ]]" DROID_ACTION_REF: "$[[ inputs.droid_action_ref ]]" @@ -50,18 +51,42 @@ droid-review: - git clone --depth 1 --branch "$DROID_ACTION_REF" "$DROID_ACTION_REPO" "$DROID_ACTION_DIR" - cd "$DROID_ACTION_DIR" && bun install --frozen-lockfile - cd "$CI_PROJECT_DIR" + - curl --retry 5 --retry-delay 2 --retry-all-errors -fsSL https://app.factory.ai/cli | sh + - export PATH="$HOME/.local/bin:$PATH" + - droid --version script: + - export PATH="$HOME/.local/bin:$PATH" - | if ! bun run "$DROID_ACTION_DIR/src/entrypoints/gitlab-prepare.ts"; then export DROID_SUCCESS=false - export DROID_ERROR_DETAILS="gitlab-prepare exited non-zero" + echo "gitlab-prepare exited non-zero" > /tmp/droid-error.txt + exit 1 + fi + - | + # Skip droid exec if prepare decided not to review. + if ! grep -q '"shouldRunReview": true' "$DROID_STATE_FILE"; then + echo "shouldRunReview=false in state; skipping droid exec." + exit 0 + fi + - | + droid mcp remove gitlab_mr 2>/dev/null || true + droid mcp add gitlab_mr "bun run $DROID_ACTION_DIR/src/mcp/gitlab-mr-server.ts" \ + --env CI_PROJECT_ID="$CI_PROJECT_ID" \ + --env CI_API_V4_URL="$CI_API_V4_URL" \ + --env GITLAB_TOKEN="$GITLAB_TOKEN" + - | + DROID_ARGS="exec -f $DROID_PROMPT_FILE --output-format stream-json --skip-permissions-unsafe" + if [ -n "$REVIEW_MODEL" ]; then DROID_ARGS="$DROID_ARGS --model $REVIEW_MODEL"; fi + if [ -n "$REASONING_EFFORT" ]; then DROID_ARGS="$DROID_ARGS --reasoning-effort $REASONING_EFFORT"; fi + echo "Running: droid $DROID_ARGS" + if ! droid $DROID_ARGS; then + export DROID_SUCCESS=false + echo "droid exec failed" > /tmp/droid-error.txt exit 1 fi - # NOTE: Real `droid exec` invocation lands in a follow-up commit - # once the GitLab review prompt + MCP wiring are in place. For now - # the prepare/post pair alone exercises the full sticky-note lifecycle. - - echo "droid exec placeholder - prompt + MCP wiring lands next." after_script: + - export PATH="$HOME/.local/bin:$PATH" + - export DROID_ERROR_DETAILS="$(cat /tmp/droid-error.txt 2>/dev/null || true)" - bun run "$DROID_ACTION_DIR/src/entrypoints/gitlab-update-comment-link.ts" || true artifacts: when: always diff --git a/src/entrypoints/gitlab-prepare.ts b/src/entrypoints/gitlab-prepare.ts index 9d7d554..326ed1b 100644 --- a/src/entrypoints/gitlab-prepare.ts +++ b/src/entrypoints/gitlab-prepare.ts @@ -22,6 +22,7 @@ import { buildTrackingNoteBody, findExistingTrackingNote, } from "../gitlab/operations/tracking-note"; +import { buildGitlabReviewPrompt } from "../gitlab/review-prompt"; type PrepareState = { shouldRunReview: boolean; @@ -33,9 +34,14 @@ type PrepareState = { sourceBranchSha: string | null; pipelineUrl: string | null; jobUrl: string | null; + promptPath: string | null; reason?: string; }; +function promptFilePath(): string { + return process.env.DROID_PROMPT_FILE || "/tmp/droid-prompts/droid-prompt.txt"; +} + function stateFilePath(): string { return ( process.env.DROID_STATE_FILE || @@ -63,6 +69,7 @@ async function run(): Promise { sourceBranchSha: context.mr?.sourceBranchSha ?? null, pipelineUrl: context.pipelineUrl, jobUrl: context.jobUrl, + promptPath: null, }; if (!isMergeRequestContext(context)) { @@ -124,10 +131,35 @@ async function run(): Promise { console.log(`Created tracking note ${trackingNoteId}`); } + console.log("Fetching MR changes to build review prompt..."); + const changes = await client.getMrChanges(context.project.id, mrIid); + const mr = await client.getMr(context.project.id, mrIid); + + const diff = (changes.changes || []) + .map((c) => { + const header = `diff --git a/${c.old_path} b/${c.new_path}`; + return `${header}\n${c.diff}`; + }) + .join("\n"); + + const prompt = buildGitlabReviewPrompt({ + projectPath: context.project.pathWithNamespace, + mrIid, + mrTitle: mr.title ?? context.mr.title, + diff, + reviewDepth: context.inputs.reviewDepth, + }); + + const promptPath = promptFilePath(); + await fs.mkdir(path.dirname(promptPath), { recursive: true }); + await fs.writeFile(promptPath, prompt); + console.log(`Wrote review prompt (${prompt.length} bytes) to ${promptPath}`); + await writeState({ ...baseState, shouldRunReview: true, trackingNoteId, + promptPath, }); } diff --git a/src/gitlab/review-prompt.ts b/src/gitlab/review-prompt.ts new file mode 100644 index 0000000..a8ed8ba --- /dev/null +++ b/src/gitlab/review-prompt.ts @@ -0,0 +1,61 @@ +/** + * Minimal v1 review prompt builder for GitLab merge requests. + * + * This is intentionally simple; the cross-platform prompt in + * src/create-prompt/ will replace it in a later step. + */ + +export interface GitlabReviewPromptOptions { + projectPath: string; + mrIid: number; + mrTitle: string | null; + diff: string; + reviewDepth: string; + maxInlineComments?: number; +} + +const DEFAULT_MAX_COMMENTS = 8; + +export function buildGitlabReviewPrompt( + opts: GitlabReviewPromptOptions, +): string { + const max = opts.maxInlineComments ?? DEFAULT_MAX_COMMENTS; + const lines: string[] = []; + + lines.push( + `You are reviewing GitLab merge request !${opts.mrIid} in project "${opts.projectPath}".`, + ); + if (opts.mrTitle) { + lines.push(`Title: ${opts.mrTitle}`); + } + lines.push(""); + lines.push("## Instructions"); + lines.push(""); + lines.push( + "1. Read the diff carefully. Focus on correctness, security, and obvious bugs.", + ); + lines.push( + "2. Only flag high-confidence issues (P0/P1). Do not nit on style.", + ); + lines.push( + `3. Post all findings via a SINGLE call to the \`submit_review\` tool with mr_iid=${opts.mrIid}.`, + ); + lines.push( + `4. Hard cap: at most ${max} inline comments per review. Prioritize the highest-severity issues.`, + ); + lines.push( + "5. For each inline comment, set `path` to the new_path from the diff and `line` to the new-file line number (the line beginning with `+`).", + ); + lines.push( + '6. If you find NO bugs, still call `submit_review` with `body: "LGTM - no issues found."` and an empty comments array.', + ); + lines.push("7. Do not call any other tool; do not edit code."); + lines.push(""); + lines.push("## Diff"); + lines.push(""); + lines.push("```diff"); + lines.push(opts.diff.trim() || "(empty diff)"); + lines.push("```"); + + return lines.join("\n"); +} diff --git a/test/gitlab/review-prompt.test.ts b/test/gitlab/review-prompt.test.ts new file mode 100644 index 0000000..18d497a --- /dev/null +++ b/test/gitlab/review-prompt.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from "bun:test"; +import { buildGitlabReviewPrompt } from "../../src/gitlab/review-prompt"; + +describe("buildGitlabReviewPrompt", () => { + it("renders project path, MR iid, title, and diff", () => { + const prompt = buildGitlabReviewPrompt({ + projectPath: "group/sub/repo", + mrIid: 42, + mrTitle: "Add widget", + diff: "diff --git a/x.ts b/x.ts\n+const a = 1;", + reviewDepth: "deep", + }); + + expect(prompt).toContain('merge request !42 in project "group/sub/repo"'); + expect(prompt).toContain("Title: Add widget"); + expect(prompt).toContain("```diff"); + expect(prompt).toContain("+const a = 1;"); + expect(prompt).toContain("submit_review"); + expect(prompt).toContain("mr_iid=42"); + }); + + it("uses (empty diff) when diff is blank", () => { + const prompt = buildGitlabReviewPrompt({ + projectPath: "g/r", + mrIid: 1, + mrTitle: null, + diff: "", + reviewDepth: "shallow", + }); + expect(prompt).toContain("(empty diff)"); + expect(prompt).not.toContain("Title:"); + }); + + it("respects custom maxInlineComments", () => { + const prompt = buildGitlabReviewPrompt({ + projectPath: "g/r", + mrIid: 1, + mrTitle: null, + diff: "x", + reviewDepth: "deep", + maxInlineComments: 3, + }); + expect(prompt).toContain("at most 3 inline comments"); + }); +}); From 94bb5b5451cb6b1b0cca70215e9170761befb1be Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 14:48:19 -0700 Subject: [PATCH 07/29] fix(gitlab): wire review_depth presets via resolveReviewConfig Previously, review_depth was a declared spec.input that was parsed and threaded through gitlab-prepare but never consumed by the prompt builder or the template -- so customers passing review_depth: shallow silently still got Droid's default model. This wires it end-to-end: - gitlab-prepare now calls resolveReviewConfig({reviewModel, reasoningEffort, reviewDepth}) from src/utils/review-depth.ts (which already had the preset table: shallow -> kimi-k2-0711, deep -> gpt-5.2 + high reasoning), reusing the same logic the GitHub flow uses. - Explicit review_model / reasoning_effort still beat the depth preset (resolveReviewConfig handles priority). - Resolved values are written to /tmp/droid-prompts/resolved-env.sh as shell exports (RESOLVED_MODEL / RESOLVED_REASONING_EFFORT), plus echoed into the state.json for visibility. - Template sources the shim before constructing droid exec args and uses the RESOLVED_* values for --model / --reasoning-effort. Adds 6 unit tests covering: default deep preset, shallow preset, explicit model override, explicit effort override, both overrides simultaneously, unknown depth fallback. Suite at 418 passing. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- gitlab/templates/review.yml | 11 ++++- src/entrypoints/gitlab-prepare.ts | 45 +++++++++++++++++ test/gitlab/review-config-resolution.test.ts | 52 ++++++++++++++++++++ 3 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 test/gitlab/review-config-resolution.test.ts diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index a626153..177977a 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -75,9 +75,16 @@ droid-review: --env CI_API_V4_URL="$CI_API_V4_URL" \ --env GITLAB_TOKEN="$GITLAB_TOKEN" - | + # gitlab-prepare resolved depth+overrides into FINAL_MODEL/FINAL_EFFORT + # via /tmp/droid-prompts/resolved-env.sh. Source it so the template uses + # those resolved values when constructing the droid exec command line. + if [ -f /tmp/droid-prompts/resolved-env.sh ]; then + # shellcheck disable=SC1091 + . /tmp/droid-prompts/resolved-env.sh + fi DROID_ARGS="exec -f $DROID_PROMPT_FILE --output-format stream-json --skip-permissions-unsafe" - if [ -n "$REVIEW_MODEL" ]; then DROID_ARGS="$DROID_ARGS --model $REVIEW_MODEL"; fi - if [ -n "$REASONING_EFFORT" ]; then DROID_ARGS="$DROID_ARGS --reasoning-effort $REASONING_EFFORT"; fi + if [ -n "$RESOLVED_MODEL" ]; then DROID_ARGS="$DROID_ARGS --model $RESOLVED_MODEL"; fi + if [ -n "$RESOLVED_REASONING_EFFORT" ]; then DROID_ARGS="$DROID_ARGS --reasoning-effort $RESOLVED_REASONING_EFFORT"; fi echo "Running: droid $DROID_ARGS" if ! droid $DROID_ARGS; then export DROID_SUCCESS=false diff --git a/src/entrypoints/gitlab-prepare.ts b/src/entrypoints/gitlab-prepare.ts index 326ed1b..eb9cdf9 100644 --- a/src/entrypoints/gitlab-prepare.ts +++ b/src/entrypoints/gitlab-prepare.ts @@ -23,6 +23,7 @@ import { findExistingTrackingNote, } from "../gitlab/operations/tracking-note"; import { buildGitlabReviewPrompt } from "../gitlab/review-prompt"; +import { resolveReviewConfig } from "../utils/review-depth"; type PrepareState = { shouldRunReview: boolean; @@ -35,6 +36,8 @@ type PrepareState = { pipelineUrl: string | null; jobUrl: string | null; promptPath: string | null; + resolvedModel: string | null; + resolvedReasoningEffort: string | null; reason?: string; }; @@ -42,6 +45,31 @@ function promptFilePath(): string { return process.env.DROID_PROMPT_FILE || "/tmp/droid-prompts/droid-prompt.txt"; } +function resolvedEnvShimPath(): string { + return ( + process.env.DROID_RESOLVED_ENV_FILE || "/tmp/droid-prompts/resolved-env.sh" + ); +} + +function shellEscape(value: string): string { + return `'${value.replace(/'/g, `'\\''`)}'`; +} + +async function writeResolvedEnvShim( + model: string | null, + reasoningEffort: string | null, +): Promise { + const filePath = resolvedEnvShimPath(); + await fs.mkdir(path.dirname(filePath), { recursive: true }); + const lines = [ + "# Generated by gitlab-prepare; source this to expose resolved review presets.", + `export RESOLVED_MODEL=${shellEscape(model ?? "")}`, + `export RESOLVED_REASONING_EFFORT=${shellEscape(reasoningEffort ?? "")}`, + ]; + await fs.writeFile(filePath, lines.join("\n") + "\n"); + console.log(`Wrote resolved env shim to ${filePath}`); +} + function stateFilePath(): string { return ( process.env.DROID_STATE_FILE || @@ -70,6 +98,8 @@ async function run(): Promise { pipelineUrl: context.pipelineUrl, jobUrl: context.jobUrl, promptPath: null, + resolvedModel: null, + resolvedReasoningEffort: null, }; if (!isMergeRequestContext(context)) { @@ -142,6 +172,19 @@ async function run(): Promise { }) .join("\n"); + const resolved = resolveReviewConfig({ + reviewModel: context.inputs.reviewModel, + reasoningEffort: context.inputs.reasoningEffort, + reviewDepth: context.inputs.reviewDepth, + }); + console.log( + `Resolved review config: depth=${context.inputs.reviewDepth} ` + + `explicitModel=${context.inputs.reviewModel || "(empty)"} ` + + `explicitEffort=${context.inputs.reasoningEffort || "(empty)"} ` + + `=> model=${resolved.model} effort=${resolved.reasoningEffort ?? "(none)"}`, + ); + await writeResolvedEnvShim(resolved.model, resolved.reasoningEffort ?? null); + const prompt = buildGitlabReviewPrompt({ projectPath: context.project.pathWithNamespace, mrIid, @@ -160,6 +203,8 @@ async function run(): Promise { shouldRunReview: true, trackingNoteId, promptPath, + resolvedModel: resolved.model, + resolvedReasoningEffort: resolved.reasoningEffort ?? null, }); } diff --git a/test/gitlab/review-config-resolution.test.ts b/test/gitlab/review-config-resolution.test.ts new file mode 100644 index 0000000..9541d6b --- /dev/null +++ b/test/gitlab/review-config-resolution.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from "bun:test"; +import { resolveReviewConfig } from "../../src/utils/review-depth"; + +describe("resolveReviewConfig (used by gitlab-prepare)", () => { + it("uses deep preset by default", () => { + expect(resolveReviewConfig()).toEqual({ + model: "gpt-5.2", + reasoningEffort: "high", + }); + }); + + it("returns shallow preset for review_depth=shallow", () => { + expect(resolveReviewConfig({ reviewDepth: "shallow" })).toEqual({ + model: "kimi-k2.6", + reasoningEffort: undefined, + }); + }); + + it("explicit reviewModel beats depth preset", () => { + const out = resolveReviewConfig({ + reviewDepth: "shallow", + reviewModel: "claude-sonnet-4-5-20250929", + }); + expect(out.model).toBe("claude-sonnet-4-5-20250929"); + }); + + it("explicit reasoningEffort beats depth preset", () => { + const out = resolveReviewConfig({ + reviewDepth: "deep", + reasoningEffort: "medium", + }); + expect(out.reasoningEffort).toBe("medium"); + expect(out.model).toBe("gpt-5.2"); + }); + + it("both explicit overrides win simultaneously", () => { + const out = resolveReviewConfig({ + reviewDepth: "shallow", + reviewModel: "claude-opus-4-8", + reasoningEffort: "low", + }); + expect(out).toEqual({ + model: "claude-opus-4-8", + reasoningEffort: "low", + }); + }); + + it("unknown reviewDepth falls back to shallow preset", () => { + const out = resolveReviewConfig({ reviewDepth: "neutron-star" }); + expect(out.model).toBe("kimi-k2.6"); + }); +}); From 08a9b01fc6d23f02bf5b07fec5b59844715e10a2 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 16:13:10 -0700 Subject: [PATCH 08/29] feat(gitlab): two-pass review (candidates + validator) mirroring GitHub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the v1 single-pass bespoke prompt with the same two-pass flow the GitHub Action uses: Pass 1 generates review_candidates.json without posting; Pass 2 validates each candidate, writes review_validated.json, and submits approved findings in a single batched call. The /review skill (loaded by Droid at runtime) drives both passes; the prompts here are the runtime harness that selects which pass + which tools. Why two droid exec calls rather than one: 1. Tool gating — Pass 1 has NO access to gitlab_mr___submit_review so the model can't shortcut by posting raw candidates. --enabled-tools is fixed at process start, so a single exec can't switch mid-run. 2. Fresh-eyes context — Pass 2 re-reads the diff without memory of why each candidate was generated, dropping ~30-60% false positives. 3. Hard output checkpoint on disk between passes (resumable, debuggable). 4. Clean failure semantics — half-baked Pass 1 won't pollute Pass 2. New files: * src/gitlab/data/review-artifacts.ts — fetch + write mr.diff, existing_comments.json, mr_description.txt (mirror of GitHub's precomputed PR artifacts). * src/gitlab/prompts/types.ts — shared prompt-context shape. * src/gitlab/prompts/candidates.ts — Pass 1 prompt, ported from GitHub's review-candidates-prompt.ts with MR/project terminology. * src/gitlab/prompts/validator.ts — Pass 2 prompt, ported from GitHub's review-validator-prompt.ts; uses gitlab_mr___submit_review and the new gitlab_mr___update_tracking_note tool. * src/entrypoints/gitlab-prepare-validator.ts — overwrites the prompt file with Pass 2 between the two droid exec calls. * test/gitlab/prompts.test.ts + test/gitlab/review-artifacts.test.ts — 14 new tests covering the two prompts and artifact computation. MCP additions: * src/mcp/gitlab-mr-server.ts — new update_tracking_note tool that reads DROID_MR_IID + DROID_TRACKING_NOTE_ID from env, symmetric with GitHub's github_comment___update_droid_comment. Template changes (gitlab/templates/review.yml): * Sources resolved-env.sh once up front (now includes DROID_MR_IID + DROID_TRACKING_NOTE_ID). * Registers the MCP server with those env vars exposed. * Pass 1 droid exec: --enabled-tools excludes submit_review. * Runs gitlab-prepare-validator between the two execs. * Pass 2 droid exec: --enabled-tools includes gitlab_mr___submit_review and gitlab_mr___update_tracking_note. * Artifacts archive bumped to include review_candidates.json, review_validated.json, droid-prompt.txt for debugging. Removed: * src/gitlab/review-prompt.ts (v1 single-pass bespoke prompt). * test/gitlab/review-prompt.test.ts. Refactoring src/gitlab/prompts/ + src/create-prompt/ into a shared src/core/review/ tree is queued as a follow-up PR. All 432 tests pass; tsc + prettier clean. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- gitlab/templates/review.yml | 52 +++++-- src/entrypoints/gitlab-prepare-validator.ts | 97 +++++++++++++ src/entrypoints/gitlab-prepare.ts | 152 +++++++++++++++---- src/gitlab/data/review-artifacts.ts | 94 ++++++++++++ src/gitlab/prompts/candidates.ts | 148 +++++++++++++++++++ src/gitlab/prompts/types.ts | 29 ++++ src/gitlab/prompts/validator.ts | 132 +++++++++++++++++ src/gitlab/review-prompt.ts | 61 -------- src/mcp/gitlab-mr-server.ts | 33 +++++ test/gitlab/prompts.test.ts | 129 +++++++++++++++++ test/gitlab/review-artifacts.test.ts | 153 ++++++++++++++++++++ test/gitlab/review-prompt.test.ts | 45 ------ 12 files changed, 978 insertions(+), 147 deletions(-) create mode 100644 src/entrypoints/gitlab-prepare-validator.ts create mode 100644 src/gitlab/data/review-artifacts.ts create mode 100644 src/gitlab/prompts/candidates.ts create mode 100644 src/gitlab/prompts/types.ts create mode 100644 src/gitlab/prompts/validator.ts delete mode 100644 src/gitlab/review-prompt.ts create mode 100644 test/gitlab/prompts.test.ts create mode 100644 test/gitlab/review-artifacts.test.ts delete mode 100644 test/gitlab/review-prompt.test.ts diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index 177977a..bba7a86 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -68,27 +68,56 @@ droid-review: echo "shouldRunReview=false in state; skipping droid exec." exit 0 fi + - | + # Source the resolved env shim from gitlab-prepare to expose + # RESOLVED_MODEL / RESOLVED_REASONING_EFFORT / DROID_MR_IID / + # DROID_TRACKING_NOTE_ID for both droid exec calls and the MCP server. + if [ -f /tmp/droid-prompts/resolved-env.sh ]; then + # shellcheck disable=SC1091 + . /tmp/droid-prompts/resolved-env.sh + fi - | droid mcp remove gitlab_mr 2>/dev/null || true droid mcp add gitlab_mr "bun run $DROID_ACTION_DIR/src/mcp/gitlab-mr-server.ts" \ --env CI_PROJECT_ID="$CI_PROJECT_ID" \ --env CI_API_V4_URL="$CI_API_V4_URL" \ - --env GITLAB_TOKEN="$GITLAB_TOKEN" + --env GITLAB_TOKEN="$GITLAB_TOKEN" \ + --env DROID_MR_IID="$DROID_MR_IID" \ + --env DROID_TRACKING_NOTE_ID="$DROID_TRACKING_NOTE_ID" - | - # gitlab-prepare resolved depth+overrides into FINAL_MODEL/FINAL_EFFORT - # via /tmp/droid-prompts/resolved-env.sh. Source it so the template uses - # those resolved values when constructing the droid exec command line. - if [ -f /tmp/droid-prompts/resolved-env.sh ]; then - # shellcheck disable=SC1091 - . /tmp/droid-prompts/resolved-env.sh + # ---- Pass 1: candidate generation ---- + # Note: gitlab_mr___submit_review and the MR-mutation tools are + # NOT included here. Pass 1 must only WRITE the candidates JSON. + PASS1_TOOLS="Read,Grep,Glob,LS,Execute,Edit,Create,ApplyPatch,Task,Skill,FetchUrl" + DROID_ARGS="exec -f $DROID_PROMPT_FILE --enabled-tools $PASS1_TOOLS --output-format stream-json --skip-permissions-unsafe --tag code-review" + if [ -n "$RESOLVED_MODEL" ]; then DROID_ARGS="$DROID_ARGS --model $RESOLVED_MODEL"; fi + if [ -n "$RESOLVED_REASONING_EFFORT" ]; then DROID_ARGS="$DROID_ARGS --reasoning-effort $RESOLVED_REASONING_EFFORT"; fi + echo "Running Pass 1 (candidates): droid $DROID_ARGS" + if ! droid $DROID_ARGS; then + export DROID_SUCCESS=false + echo "droid exec pass 1 (candidates) failed" > /tmp/droid-error.txt + exit 1 fi - DROID_ARGS="exec -f $DROID_PROMPT_FILE --output-format stream-json --skip-permissions-unsafe" + - | + # ---- Prepare Pass 2 prompt ---- + if ! bun run "$DROID_ACTION_DIR/src/entrypoints/gitlab-prepare-validator.ts"; then + export DROID_SUCCESS=false + echo "gitlab-prepare-validator failed" > /tmp/droid-error.txt + exit 1 + fi + - | + # ---- Pass 2: validator (posts approved comments) ---- + # gitlab_mr___submit_review and gitlab_mr___update_tracking_note are + # the only MR-mutation tools we expose. The model batches all approved + # findings into a single submit_review call. + PASS2_TOOLS="Read,Grep,Glob,LS,Execute,Edit,Create,ApplyPatch,Skill,gitlab_mr___submit_review,gitlab_mr___update_tracking_note" + DROID_ARGS="exec -f $DROID_PROMPT_FILE --enabled-tools $PASS2_TOOLS --output-format stream-json --skip-permissions-unsafe --tag code-review" if [ -n "$RESOLVED_MODEL" ]; then DROID_ARGS="$DROID_ARGS --model $RESOLVED_MODEL"; fi if [ -n "$RESOLVED_REASONING_EFFORT" ]; then DROID_ARGS="$DROID_ARGS --reasoning-effort $RESOLVED_REASONING_EFFORT"; fi - echo "Running: droid $DROID_ARGS" + echo "Running Pass 2 (validator): droid $DROID_ARGS" if ! droid $DROID_ARGS; then export DROID_SUCCESS=false - echo "droid exec failed" > /tmp/droid-error.txt + echo "droid exec pass 2 (validator) failed" > /tmp/droid-error.txt exit 1 fi after_script: @@ -99,4 +128,7 @@ droid-review: when: always paths: - .droid-state.json + - /tmp/droid-prompts/review_candidates.json + - /tmp/droid-prompts/review_validated.json + - /tmp/droid-prompts/droid-prompt.txt expire_in: 1 week diff --git a/src/entrypoints/gitlab-prepare-validator.ts b/src/entrypoints/gitlab-prepare-validator.ts new file mode 100644 index 0000000..d71fb7e --- /dev/null +++ b/src/entrypoints/gitlab-prepare-validator.ts @@ -0,0 +1,97 @@ +#!/usr/bin/env bun + +/** + * Prepare step for Pass 2 (validator) of the GitLab two-pass review flow. + * + * Runs between the two `droid exec` invocations in the CI template: + * + * 1. Reads the state file produced by `gitlab-prepare.ts` (Pass 1). + * Bails out cleanly if Pass 1 decided not to review. + * 2. Reconstructs the GitLab review prompt context from state. + * 3. Generates the Pass-2 validator prompt. + * 4. Overwrites the shared prompt file (the same file Pass 1 used) + * so the next `droid exec -f ` consumes Pass 2. + * + * No GitLab API calls are made here — all the data we need is already + * on disk from Pass 1's artifact precomputation. + */ + +import * as fs from "fs/promises"; +import * as path from "path"; +import { generateGitlabReviewValidatorPrompt } from "../gitlab/prompts/validator"; +import type { GitlabReviewPromptContext } from "../gitlab/prompts/types"; +import type { PrepareState } from "./gitlab-prepare"; + +function stateFilePath(): string { + return ( + process.env.DROID_STATE_FILE || + path.join(process.env.CI_PROJECT_DIR || "/tmp", ".droid-state.json") + ); +} + +async function readState(): Promise { + const filePath = stateFilePath(); + const raw = await fs.readFile(filePath, "utf8"); + return JSON.parse(raw) as PrepareState; +} + +function ensure(value: T | null | undefined, name: string): T { + if (value === null || value === undefined) { + throw new Error( + `gitlab-prepare-validator: missing state.${name}; was gitlab-prepare run successfully?`, + ); + } + return value; +} + +async function run(): Promise { + const state = await readState(); + + if (!state.shouldRunReview) { + console.log( + `Pass 1 was skipped (reason: ${state.reason ?? "unknown"}); skipping validator prepare.`, + ); + return; + } + + const promptPath = ensure(state.promptPath, "promptPath"); + const mrIid = ensure(state.mrIid, "mrIid"); + const candidatesPath = ensure(state.candidatesPath, "candidatesPath"); + const validatedPath = ensure(state.validatedPath, "validatedPath"); + const diffPath = ensure(state.diffPath, "diffPath"); + const commentsPath = ensure(state.commentsPath, "commentsPath"); + const descriptionPath = ensure(state.descriptionPath, "descriptionPath"); + const headSha = ensure(state.headSha, "headSha"); + + const promptCtx: GitlabReviewPromptContext = { + projectPath: state.projectPath, + mrIid, + mrTitle: state.mrTitle ?? "", + sourceBranch: state.sourceBranch ?? "", + targetBranch: state.targetBranch ?? "", + headSha, + diffPath, + commentsPath, + descriptionPath, + candidatesPath, + validatedPath, + includeSuggestions: state.includeSuggestions, + securityReviewEnabled: state.securityReviewEnabled, + }; + + const prompt = generateGitlabReviewValidatorPrompt(promptCtx); + await fs.mkdir(path.dirname(promptPath), { recursive: true }); + await fs.writeFile(promptPath, prompt); + console.log( + `Wrote Pass-2 validator prompt (${prompt.length} bytes) to ${promptPath} (overwrote Pass 1)`, + ); +} + +if (import.meta.main) { + run().catch((error) => { + console.error("gitlab-prepare-validator failed:", error); + process.exit(1); + }); +} + +export { run }; diff --git a/src/entrypoints/gitlab-prepare.ts b/src/entrypoints/gitlab-prepare.ts index eb9cdf9..8fa3c22 100644 --- a/src/entrypoints/gitlab-prepare.ts +++ b/src/entrypoints/gitlab-prepare.ts @@ -1,16 +1,26 @@ #!/usr/bin/env bun /** - * Prepare step for the GitLab CI/CD Component. + * Prepare step for the GitLab CI/CD Component (Pass 1 of the two-pass + * review flow). * - * Responsibilities (v1, automatic review on MR pipelines only): + * Responsibilities: * 1. Parse GitLab CI env into a normalized context. * 2. Decide whether this pipeline should run a review (automaticReview * flag + merge_request_event source). * 3. Ensure a sticky tracking note exists on the MR; reuse the existing * one if a prior pipeline already created it. - * 4. Write a small JSON state file so the post-step (and `droid exec`) - * can look up the MR IID, note ID, and other context. + * 4. Fetch + persist the three review artifacts (mr.diff, + * existing_comments.json, mr_description.txt). + * 5. Resolve review depth presets into concrete model+effort. + * 6. Generate the Pass-1 (candidate-generation) prompt and write it + * to the shared prompt file. + * 7. Write a JSON state file consumed by `gitlab-prepare-validator` + * (Pass 2) and `gitlab-update-comment-link` (post-step). + * + * Pass 2 is generated later by `gitlab-prepare-validator.ts`, which + * reads this state file and overwrites the prompt file with the + * Pass-2 content before the second `droid exec` invocation. */ import * as fs from "fs/promises"; @@ -22,10 +32,12 @@ import { buildTrackingNoteBody, findExistingTrackingNote, } from "../gitlab/operations/tracking-note"; -import { buildGitlabReviewPrompt } from "../gitlab/review-prompt"; +import { computeReviewArtifacts } from "../gitlab/data/review-artifacts"; +import { generateGitlabReviewCandidatesPrompt } from "../gitlab/prompts/candidates"; +import type { GitlabReviewPromptContext } from "../gitlab/prompts/types"; import { resolveReviewConfig } from "../utils/review-depth"; -type PrepareState = { +export type PrepareState = { shouldRunReview: boolean; projectId: string; projectPath: string; @@ -38,6 +50,20 @@ type PrepareState = { promptPath: string | null; resolvedModel: string | null; resolvedReasoningEffort: string | null; + + diffPath: string | null; + commentsPath: string | null; + descriptionPath: string | null; + candidatesPath: string | null; + validatedPath: string | null; + + mrTitle: string | null; + sourceBranch: string | null; + targetBranch: string | null; + headSha: string | null; + includeSuggestions: boolean; + securityReviewEnabled: boolean; + reason?: string; }; @@ -45,6 +71,24 @@ function promptFilePath(): string { return process.env.DROID_PROMPT_FILE || "/tmp/droid-prompts/droid-prompt.txt"; } +function promptsDir(): string { + return path.dirname(promptFilePath()); +} + +function candidatesFilePath(): string { + return ( + process.env.REVIEW_CANDIDATES_PATH || + path.join(promptsDir(), "review_candidates.json") + ); +} + +function validatedFilePath(): string { + return ( + process.env.REVIEW_VALIDATED_PATH || + path.join(promptsDir(), "review_validated.json") + ); +} + function resolvedEnvShimPath(): string { return ( process.env.DROID_RESOLVED_ENV_FILE || "/tmp/droid-prompts/resolved-env.sh" @@ -58,6 +102,7 @@ function shellEscape(value: string): string { async function writeResolvedEnvShim( model: string | null, reasoningEffort: string | null, + extras: Record, ): Promise { const filePath = resolvedEnvShimPath(); await fs.mkdir(path.dirname(filePath), { recursive: true }); @@ -66,6 +111,9 @@ async function writeResolvedEnvShim( `export RESOLVED_MODEL=${shellEscape(model ?? "")}`, `export RESOLVED_REASONING_EFFORT=${shellEscape(reasoningEffort ?? "")}`, ]; + for (const [k, v] of Object.entries(extras)) { + lines.push(`export ${k}=${shellEscape(v ?? "")}`); + } await fs.writeFile(filePath, lines.join("\n") + "\n"); console.log(`Wrote resolved env shim to ${filePath}`); } @@ -100,6 +148,17 @@ async function run(): Promise { promptPath: null, resolvedModel: null, resolvedReasoningEffort: null, + diffPath: null, + commentsPath: null, + descriptionPath: null, + candidatesPath: null, + validatedPath: null, + mrTitle: context.mr?.title ?? null, + sourceBranch: null, + targetBranch: null, + headSha: null, + includeSuggestions: true, + securityReviewEnabled: false, }; if (!isMergeRequestContext(context)) { @@ -134,43 +193,44 @@ async function run(): Promise { const client = new GitlabClient(token, context.apiUrl); const mrIid = context.mr.iid; + const projectId = context.project.id; - const existingNotes = await client.listNotes(context.project.id, mrIid); + const existingNotes = await client.listNotes(projectId, mrIid); const existing = findExistingTrackingNote(existingNotes); + const includeSuggestions = + (process.env.INCLUDE_SUGGESTIONS ?? "true").toLowerCase() !== "false"; + const securityReviewEnabled = context.inputs.automaticSecurityReview; + const noteBody = buildTrackingNoteBody({ state: "running", pipelineUrl: context.pipelineUrl, jobUrl: context.jobUrl, triggerUsername: context.user.login, - securityReviewRan: context.inputs.automaticSecurityReview, + securityReviewRan: securityReviewEnabled, }); let trackingNoteId: number; if (existing) { - await client.updateNote(context.project.id, mrIid, existing.id, noteBody); + await client.updateNote(projectId, mrIid, existing.id, noteBody); trackingNoteId = existing.id; console.log(`Reused existing tracking note ${trackingNoteId}`); } else { - const created = await client.createNote( - context.project.id, - mrIid, - noteBody, - ); + const created = await client.createNote(projectId, mrIid, noteBody); trackingNoteId = created.id; console.log(`Created tracking note ${trackingNoteId}`); } - console.log("Fetching MR changes to build review prompt..."); - const changes = await client.getMrChanges(context.project.id, mrIid); - const mr = await client.getMr(context.project.id, mrIid); - - const diff = (changes.changes || []) - .map((c) => { - const header = `diff --git a/${c.old_path} b/${c.new_path}`; - return `${header}\n${c.diff}`; - }) - .join("\n"); + console.log("Computing review artifacts (diff, notes, description)..."); + const artifacts = await computeReviewArtifacts({ + client, + projectId, + mrIid, + outDir: promptsDir(), + }); + console.log( + `Artifacts written:\n ${artifacts.diffPath}\n ${artifacts.commentsPath}\n ${artifacts.descriptionPath}`, + ); const resolved = resolveReviewConfig({ reviewModel: context.inputs.reviewModel, @@ -183,20 +243,39 @@ async function run(): Promise { `explicitEffort=${context.inputs.reasoningEffort || "(empty)"} ` + `=> model=${resolved.model} effort=${resolved.reasoningEffort ?? "(none)"}`, ); - await writeResolvedEnvShim(resolved.model, resolved.reasoningEffort ?? null); - const prompt = buildGitlabReviewPrompt({ + await writeResolvedEnvShim(resolved.model, resolved.reasoningEffort ?? null, { + DROID_MR_IID: String(mrIid), + DROID_TRACKING_NOTE_ID: String(trackingNoteId), + }); + + const candidatesPath = candidatesFilePath(); + const validatedPath = validatedFilePath(); + + const promptCtx: GitlabReviewPromptContext = { projectPath: context.project.pathWithNamespace, mrIid, - mrTitle: mr.title ?? context.mr.title, - diff, - reviewDepth: context.inputs.reviewDepth, - }); + mrTitle: artifacts.mr.title ?? context.mr.title ?? "", + sourceBranch: artifacts.mr.source_branch ?? "", + targetBranch: artifacts.mr.target_branch ?? "", + headSha: + artifacts.mr.diff_refs?.head_sha ?? context.mr.sourceBranchSha ?? "", + diffPath: artifacts.diffPath, + commentsPath: artifacts.commentsPath, + descriptionPath: artifacts.descriptionPath, + candidatesPath, + validatedPath, + includeSuggestions, + securityReviewEnabled, + }; + const prompt = generateGitlabReviewCandidatesPrompt(promptCtx); const promptPath = promptFilePath(); await fs.mkdir(path.dirname(promptPath), { recursive: true }); await fs.writeFile(promptPath, prompt); - console.log(`Wrote review prompt (${prompt.length} bytes) to ${promptPath}`); + console.log( + `Wrote Pass-1 candidates prompt (${prompt.length} bytes) to ${promptPath}`, + ); await writeState({ ...baseState, @@ -205,6 +284,17 @@ async function run(): Promise { promptPath, resolvedModel: resolved.model, resolvedReasoningEffort: resolved.reasoningEffort ?? null, + diffPath: artifacts.diffPath, + commentsPath: artifacts.commentsPath, + descriptionPath: artifacts.descriptionPath, + candidatesPath, + validatedPath, + mrTitle: promptCtx.mrTitle, + sourceBranch: promptCtx.sourceBranch, + targetBranch: promptCtx.targetBranch, + headSha: promptCtx.headSha, + includeSuggestions, + securityReviewEnabled, }); } diff --git a/src/gitlab/data/review-artifacts.ts b/src/gitlab/data/review-artifacts.ts new file mode 100644 index 0000000..be45775 --- /dev/null +++ b/src/gitlab/data/review-artifacts.ts @@ -0,0 +1,94 @@ +/** + * Compute and persist the three artifact files that the GitLab review + * prompts read from disk: + * + * - mr.diff : concatenated unified diff for every changed file + * - existing_comments.json : array of notes already on the MR (system+human) + * - mr_description.txt : MR title + description + * + * These mirror the artifacts the GitHub Action precomputes + * (`pr.diff`, `existing_comments.json`, `pr_description.txt`) so the GitLab + * Pass-1 / Pass-2 prompts can refer to them by stable paths and the model + * doesn't have to round-trip through MCP tools to fetch them. + */ + +import * as fs from "fs/promises"; +import * as path from "path"; +import type { GitlabClient } from "../api/client"; +import type { GitlabMr, GitlabMrChanges, GitlabNote } from "../types"; + +export type GitlabReviewArtifactPaths = { + diffPath: string; + commentsPath: string; + descriptionPath: string; +}; + +export type GitlabReviewArtifacts = GitlabReviewArtifactPaths & { + mr: GitlabMr; + changes: GitlabMrChanges; + notes: GitlabNote[]; +}; + +export type ComputeReviewArtifactsOptions = { + client: GitlabClient; + projectId: string | number; + mrIid: number; + outDir: string; +}; + +export function buildDiffContent(changes: GitlabMrChanges): string { + const files = changes.changes ?? []; + if (files.length === 0) { + return ""; + } + return files + .map((c) => { + const oldPath = c.old_path || c.new_path; + const newPath = c.new_path || c.old_path; + const header = `diff --git a/${oldPath} b/${newPath}`; + return `${header}\n${c.diff}`; + }) + .join("\n"); +} + +export function buildDescriptionContent(mr: GitlabMr): string { + const title = mr.title ?? ""; + const description = mr.description ?? ""; + return `Title: ${title}\n\n${description}\n`; +} + +export async function computeReviewArtifacts( + opts: ComputeReviewArtifactsOptions, +): Promise { + const { client, projectId, mrIid, outDir } = opts; + + await fs.mkdir(outDir, { recursive: true }); + + const [mr, changes, notes] = await Promise.all([ + client.getMr(projectId, mrIid), + client.getMrChanges(projectId, mrIid), + client.listNotes(projectId, mrIid), + ]); + + const diffPath = path.join(outDir, "mr.diff"); + const commentsPath = path.join(outDir, "existing_comments.json"); + const descriptionPath = path.join(outDir, "mr_description.txt"); + + const diffContent = buildDiffContent(changes); + const descriptionContent = buildDescriptionContent(mr); + + await Promise.all([ + fs.writeFile(diffPath, diffContent), + fs.writeFile(commentsPath, JSON.stringify(notes, null, 2)), + fs.writeFile(descriptionPath, descriptionContent), + ]); + + return { + diffPath, + commentsPath, + descriptionPath, + mr, + changes, + notes, + }; +} diff --git a/src/gitlab/prompts/candidates.ts b/src/gitlab/prompts/candidates.ts new file mode 100644 index 0000000..aa5966a --- /dev/null +++ b/src/gitlab/prompts/candidates.ts @@ -0,0 +1,148 @@ +/** + * GitLab Pass-1 (candidate generation) prompt. + * + * Direct port of the GitHub Action's + * `src/create-prompt/templates/review-candidates-prompt.ts` with PR→MR + * terminology and GitLab-specific entity numbering. The /review skill + * itself is platform-agnostic and contains the two-pass methodology; this + * file is the runtime harness that tells Droid which pass it's in, where + * the input artifacts live on disk, and where to write the candidate JSON. + * + * STRICT contract: this prompt MUST NOT instruct the model to call the + * GitLab posting tool (`gitlab_mr___submit_review`). Posting only happens + * in Pass 2. Tool gating is also enforced at the `droid exec` level by + * excluding `gitlab_mr___submit_review` from `--enabled-tools` during + * Pass 1. + */ + +import type { GitlabReviewPromptContext } from "./types"; + +export function generateGitlabReviewCandidatesPrompt( + ctx: GitlabReviewPromptContext, +): string { + const { + projectPath, + mrIid, + sourceBranch, + targetBranch, + headSha, + diffPath, + commentsPath, + descriptionPath, + candidatesPath, + includeSuggestions, + securityReviewEnabled, + } = ctx; + + const bodyFieldDescription = includeSuggestions + ? " - `body`: Comment text starting with priority tag [P0|P1|P2], then title, then 1 paragraph explanation.\n" + + " Follow the suggestion block rules from the review skill when including suggestions." + : " - `body`: Comment text starting with priority tag [P0|P1|P2], then title, then 1 paragraph explanation"; + + const sideFieldDescription = includeSuggestions + ? ' - `side`: "RIGHT" for new/modified code (default). Use "LEFT" only for removed code **without** suggestions.\n' + + " If you include a suggestion block, choose a RIGHT-side anchor and keep it unchanged so the validator can reuse it." + : ' - `side`: "RIGHT" for new/modified code (default), "LEFT" only for removed code'; + + const skillInstruction = includeSuggestions + ? "Invoke the 'review' skill to load the review methodology, then execute its **Pass 1: Candidate Generation** procedure — including suggestion block rules." + : "Invoke the 'review' skill to load the review methodology, then execute its **Pass 1: Candidate Generation** procedure. Do NOT include code suggestion blocks."; + + const securitySubagentInstruction = securityReviewEnabled + ? ` + +## Security Review (run concurrently) + +In addition to the code review, you MUST also spawn a \`security-reviewer\` subagent via the Task tool. +This subagent runs **concurrently** with the code review subagents during Step 2. + +Spawn it with: +- \`subagent_type\`: "security-reviewer" +- \`description\`: "Security review" +- \`prompt\`: Include the full MR context (project, MR IID, head SHA, target branch) and the paths to precomputed data files (diff, description, existing comments). The security-reviewer will invoke the security-review skill and return a JSON array of security findings. + +**IMPORTANT**: Spawn the security-reviewer in the SAME response as the code review subagents so they all run in parallel. + +After all subagents complete (both code review and security-reviewer), merge the security findings into the \`comments\` array alongside code review findings. Security findings use the same schema but are prefixed with \`[security]\` in their body (e.g., \`[P1] [security] Title\`). +` + : ""; + + return `You are a senior staff software engineer and expert code reviewer. + +Your task: Review MR !${mrIid} in ${projectPath} and generate a JSON file with **high-confidence, actionable** review comments that pinpoint genuine issues. + +${skillInstruction}${securitySubagentInstruction} + + +Project: ${projectPath} +MR IID: ${mrIid} +MR Source Branch: ${sourceBranch} +MR Head SHA: ${headSha} +MR Target Branch: ${targetBranch} + +Precomputed data files: +- MR Description: \`${descriptionPath}\` +- Full MR Diff: \`${diffPath}\` +- Existing Comments: \`${commentsPath}\` + + + +Write output to \`${candidatesPath}\` using this exact schema: + +\`\`\`json +{ + "version": 1, + "meta": { + "project": "group/project", + "mrIid": 123, + "headSha": "", + "targetBranch": "main", + "generatedAt": "" + }, + "comments": [ + { + "path": "src/index.ts", + "body": "[P1] Title\\n\\n1 paragraph.", + "line": 42, + "startLine": null, + "side": "RIGHT", + "commit_id": "" + } + ], + "reviewSummary": { + "body": "1-3 sentence overall assessment" + } +} +\`\`\` + + +- **version**: Always \`1\` + +- **meta**: Metadata object + - \`project\`: "${projectPath}" + - \`mrIid\`: ${mrIid} + - \`headSha\`: "${headSha}" + - \`targetBranch\`: "${targetBranch}" + - \`generatedAt\`: ISO 8601 timestamp (e.g., "2024-01-15T10:30:00Z") + +- **comments**: Array of comment objects + - \`path\`: Relative file path (use the new_path from the diff, e.g., "src/index.ts") +${bodyFieldDescription} + - \`line\`: Target line number in the new file (single-line) or end line number (multi-line). Must be ≥ 0. + - \`startLine\`: \`null\` for single-line comments, or start line number for multi-line comments +${sideFieldDescription} + - \`commit_id\`: "${headSha}" + +- **reviewSummary**: + - \`body\`: 1-3 sentence overall assessment + + + + +**DO NOT** post to GitLab. +**DO NOT** invoke any MR mutation tools (\`gitlab_mr___submit_review\`, \`gitlab_mr___create_mr_note\`, \`gitlab_mr___update_mr_note\`, \`gitlab_mr___update_mr_description\`, \`gitlab_mr___update_tracking_note\`, etc.). +**DO NOT** modify any files other than writing to \`${candidatesPath}\`. +Output ONLY the JSON file—no additional commentary. + +`; +} diff --git a/src/gitlab/prompts/types.ts b/src/gitlab/prompts/types.ts new file mode 100644 index 0000000..9105ed2 --- /dev/null +++ b/src/gitlab/prompts/types.ts @@ -0,0 +1,29 @@ +/** + * Shared shape passed to both the Pass-1 candidate-generation prompt and + * the Pass-2 validator prompt. Mirrors the subset of GitHub's + * `PreparedContext` that the review templates actually consume, but with + * GitLab terminology (MR instead of PR, project path instead of repo + * full_name) and without coupling to the GitHub action's webhook payload + * types. + * + * Kept platform-specific on purpose for v1; a follow-up will extract a + * platform-neutral context into `src/core/review/`. + */ + +export type GitlabReviewPromptContext = { + projectPath: string; + mrIid: number; + mrTitle: string; + sourceBranch: string; + targetBranch: string; + headSha: string; + + diffPath: string; + commentsPath: string; + descriptionPath: string; + candidatesPath: string; + validatedPath: string; + + includeSuggestions: boolean; + securityReviewEnabled: boolean; +}; diff --git a/src/gitlab/prompts/validator.ts b/src/gitlab/prompts/validator.ts new file mode 100644 index 0000000..9606e96 --- /dev/null +++ b/src/gitlab/prompts/validator.ts @@ -0,0 +1,132 @@ +/** + * GitLab Pass-2 (validator) prompt. + * + * Direct port of the GitHub Action's + * `src/create-prompt/templates/review-validator-prompt.ts` with PR→MR + * terminology and the GitLab MCP tool names. The validator reads the + * candidates JSON produced by Pass 1 from disk, validates each one, + * writes a refined JSON, and posts the approved findings as a single + * batched call to `gitlab_mr___submit_review`. + * + * Pass 2 is the only place where the posting tool is exposed (via + * `--enabled-tools` on the second `droid exec` invocation). + */ + +import type { GitlabReviewPromptContext } from "./types"; + +export function generateGitlabReviewValidatorPrompt( + ctx: GitlabReviewPromptContext, +): string { + const { + projectPath, + mrIid, + sourceBranch, + targetBranch, + headSha, + diffPath, + commentsPath, + descriptionPath, + candidatesPath, + validatedPath, + includeSuggestions, + } = ctx; + + const skillInstruction = includeSuggestions + ? "Invoke the 'review' skill to load the review methodology, then execute its **Pass 2: Validation** procedure — including suggestion block rules." + : "Invoke the 'review' skill to load the review methodology, then execute its **Pass 2: Validation** procedure. Do NOT include code suggestion blocks."; + + return `You are validating candidate review comments for MR !${mrIid} in ${projectPath}. + +IMPORTANT: This is Phase 2 (validator) of a two-pass review pipeline. + +${skillInstruction} + +### Context + +* Project: ${projectPath} +* MR IID: ${mrIid} +* MR Source Branch: ${sourceBranch} +* MR Head SHA: ${headSha} +* MR Target Branch: ${targetBranch} + +### Inputs + +Read these files before validating: +* MR Description: \`${descriptionPath}\` +* Candidates: \`${candidatesPath}\` +* Full MR Diff: \`${diffPath}\` +* Existing Comments: \`${commentsPath}\` + +If the diff is large, read in chunks (offset/limit). **Do not proceed until you have read the ENTIRE diff.** + +### Critical Requirements + +1. You MUST read and validate **every** candidate before posting anything. +2. Preserve ordering: keep results in the same order as candidates. +3. **Posting rule (STRICT):** Only post comments where \`status === "approved"\`. Never post rejected items. + +### Output: Write \`${validatedPath}\` + +\`\`\`json +{ + "version": 1, + "meta": { + "project": "${projectPath}", + "mrIid": ${mrIid}, + "headSha": "${headSha}", + "targetBranch": "${targetBranch}", + "validatedAt": "" + }, + "results": [ + { + "status": "approved", + "comment": { + "path": "src/index.ts", + "body": "[P1] Title\\n\\n1 paragraph.", + "line": 42, + "startLine": null, + "side": "RIGHT", + "commit_id": "${headSha}" + } + }, + { + "status": "rejected", + "candidate": { + "path": "src/other.ts", + "body": "[P2] ...", + "line": 10, + "startLine": null, + "side": "RIGHT", + "commit_id": "${headSha}" + }, + "reason": "Not a real bug because ..." + } + ], + "reviewSummary": { + "status": "approved", + "body": "1-3 sentence overall assessment" + } +} +\`\`\` + +Notes: +* Use \`commit_id\` = \`${headSha}\`. +* \`results\` MUST have exactly one entry per candidate, in the same order. + +Tooling note: +* If the tools list includes \`ApplyPatch\` (common for OpenAI models like GPT-5.2), use \`ApplyPatch\` to create/update the file at the exact path. +* Otherwise, use \`Create\` (or \`Edit\` if overwriting) to write the file. + +### Post approved items + +After writing \`${validatedPath}\`, post comments ONLY for \`status === "approved"\`: + +* Collect all approved comments and submit them as a **single batched review** via \`gitlab_mr___submit_review\`, passing them in the \`comments\` array parameter along with \`mr_iid: ${mrIid}\`. +* Do **NOT** post comments individually — batch them all into one \`submit_review\` call. +* Do **NOT** include a \`body\` parameter in \`submit_review\` (we use a separate tracking note for the summary). +* Use \`gitlab_mr___update_tracking_note\` to update the sticky tracking note with the review summary. +* If any approved comments contain \`[security]\` in their body, prepend a security badge to the tracking note: \`![Security Review](https://img.shields.io/badge/security%20review-ran-blue)\`. This indicates that security analysis was performed as part of the review. +* Do **NOT** post the summary as a separate top-level note. +* Do not approve the MR or request changes (GitLab approval rules are handled out-of-band). +`; +} diff --git a/src/gitlab/review-prompt.ts b/src/gitlab/review-prompt.ts deleted file mode 100644 index a8ed8ba..0000000 --- a/src/gitlab/review-prompt.ts +++ /dev/null @@ -1,61 +0,0 @@ -/** - * Minimal v1 review prompt builder for GitLab merge requests. - * - * This is intentionally simple; the cross-platform prompt in - * src/create-prompt/ will replace it in a later step. - */ - -export interface GitlabReviewPromptOptions { - projectPath: string; - mrIid: number; - mrTitle: string | null; - diff: string; - reviewDepth: string; - maxInlineComments?: number; -} - -const DEFAULT_MAX_COMMENTS = 8; - -export function buildGitlabReviewPrompt( - opts: GitlabReviewPromptOptions, -): string { - const max = opts.maxInlineComments ?? DEFAULT_MAX_COMMENTS; - const lines: string[] = []; - - lines.push( - `You are reviewing GitLab merge request !${opts.mrIid} in project "${opts.projectPath}".`, - ); - if (opts.mrTitle) { - lines.push(`Title: ${opts.mrTitle}`); - } - lines.push(""); - lines.push("## Instructions"); - lines.push(""); - lines.push( - "1. Read the diff carefully. Focus on correctness, security, and obvious bugs.", - ); - lines.push( - "2. Only flag high-confidence issues (P0/P1). Do not nit on style.", - ); - lines.push( - `3. Post all findings via a SINGLE call to the \`submit_review\` tool with mr_iid=${opts.mrIid}.`, - ); - lines.push( - `4. Hard cap: at most ${max} inline comments per review. Prioritize the highest-severity issues.`, - ); - lines.push( - "5. For each inline comment, set `path` to the new_path from the diff and `line` to the new-file line number (the line beginning with `+`).", - ); - lines.push( - '6. If you find NO bugs, still call `submit_review` with `body: "LGTM - no issues found."` and an empty comments array.', - ); - lines.push("7. Do not call any other tool; do not edit code."); - lines.push(""); - lines.push("## Diff"); - lines.push(""); - lines.push("```diff"); - lines.push(opts.diff.trim() || "(empty diff)"); - lines.push("```"); - - return lines.join("\n"); -} diff --git a/src/mcp/gitlab-mr-server.ts b/src/mcp/gitlab-mr-server.ts index 1adc503..46c9d29 100644 --- a/src/mcp/gitlab-mr-server.ts +++ b/src/mcp/gitlab-mr-server.ts @@ -184,6 +184,39 @@ export function createGitlabMrServer({ }, ); + server.tool( + "update_tracking_note", + "Update the sticky tracking note that gitlab-prepare created at the " + + "start of the run. Reads the MR IID and note ID from env so the model " + + "doesn't have to thread them through the prompt. Mirrors the GitHub " + + "`update_droid_comment` tool.", + { + body: z + .string() + .min(1) + .describe("New tracking note body in markdown (replaces existing)"), + }, + async ({ body }) => { + try { + const mrIidEnv = + process.env.DROID_MR_IID || process.env.CI_MERGE_REQUEST_IID; + const noteIdEnv = process.env.DROID_TRACKING_NOTE_ID; + const mrIid = mrIidEnv ? Number(mrIidEnv) : NaN; + const noteId = noteIdEnv ? Number(noteIdEnv) : NaN; + if (!Number.isFinite(mrIid) || !Number.isFinite(noteId)) { + throw new Error( + "update_tracking_note requires DROID_MR_IID and " + + "DROID_TRACKING_NOTE_ID environment variables", + ); + } + await client.updateNote(projectId, mrIid, noteId, body); + return textResult(`Updated tracking note ${noteId} on MR !${mrIid}`); + } catch (error) { + return errorResult(error); + } + }, + ); + server.tool( "update_mr_description", "Replace the description/body of a merge request", diff --git a/test/gitlab/prompts.test.ts b/test/gitlab/prompts.test.ts new file mode 100644 index 0000000..83b0c0e --- /dev/null +++ b/test/gitlab/prompts.test.ts @@ -0,0 +1,129 @@ +import { describe, it, expect } from "bun:test"; +import { generateGitlabReviewCandidatesPrompt } from "../../src/gitlab/prompts/candidates"; +import { generateGitlabReviewValidatorPrompt } from "../../src/gitlab/prompts/validator"; +import type { GitlabReviewPromptContext } from "../../src/gitlab/prompts/types"; + +function baseCtx( + overrides: Partial = {}, +): GitlabReviewPromptContext { + return { + projectPath: "top-group/sub/project", + mrIid: 42, + mrTitle: "Add feature X", + sourceBranch: "feat/x", + targetBranch: "main", + headSha: "deadbeef1234567890abcdef", + diffPath: "/tmp/droid-prompts/mr.diff", + commentsPath: "/tmp/droid-prompts/existing_comments.json", + descriptionPath: "/tmp/droid-prompts/mr_description.txt", + candidatesPath: "/tmp/droid-prompts/review_candidates.json", + validatedPath: "/tmp/droid-prompts/review_validated.json", + includeSuggestions: true, + securityReviewEnabled: false, + ...overrides, + }; +} + +describe("generateGitlabReviewCandidatesPrompt", () => { + it("uses MR/project terminology, not PR/repo", () => { + const prompt = generateGitlabReviewCandidatesPrompt(baseCtx()); + expect(prompt).toContain("MR !42"); + expect(prompt).toContain("top-group/sub/project"); + expect(prompt).toContain("MR IID: 42"); + expect(prompt).toContain("MR Source Branch: feat/x"); + expect(prompt).toContain("MR Target Branch: main"); + expect(prompt).toContain("MR Head SHA: deadbeef1234567890abcdef"); + expect(prompt).not.toMatch(/\bPR #\d+\b/); + expect(prompt).not.toContain("PR Head Ref"); + }); + + it("references the three artifact files and candidates output", () => { + const prompt = generateGitlabReviewCandidatesPrompt(baseCtx()); + expect(prompt).toContain("/tmp/droid-prompts/mr.diff"); + expect(prompt).toContain("/tmp/droid-prompts/existing_comments.json"); + expect(prompt).toContain("/tmp/droid-prompts/mr_description.txt"); + expect(prompt).toContain("/tmp/droid-prompts/review_candidates.json"); + }); + + it("instructs the model to invoke the review skill Pass 1 procedure", () => { + const prompt = generateGitlabReviewCandidatesPrompt(baseCtx()); + expect(prompt).toContain("Invoke the 'review' skill"); + expect(prompt).toContain("Pass 1: Candidate Generation"); + }); + + it("explicitly forbids GitLab posting tools", () => { + const prompt = generateGitlabReviewCandidatesPrompt(baseCtx()); + expect(prompt).toContain("post to GitLab"); + expect(prompt).toMatch(/DO NOT\*?\*?\s+post to GitLab/); + expect(prompt).toContain("gitlab_mr___submit_review"); + expect(prompt).toContain("gitlab_mr___update_tracking_note"); + }); + + it("includes the security-reviewer subagent block only when enabled", () => { + const off = generateGitlabReviewCandidatesPrompt( + baseCtx({ securityReviewEnabled: false }), + ); + expect(off).not.toContain("security-reviewer"); + + const on = generateGitlabReviewCandidatesPrompt( + baseCtx({ securityReviewEnabled: true }), + ); + expect(on).toContain("security-reviewer"); + expect(on).toContain("Task tool"); + expect(on).toContain("[security]"); + }); + + it("drops suggestion-block guidance when includeSuggestions is false", () => { + const off = generateGitlabReviewCandidatesPrompt( + baseCtx({ includeSuggestions: false }), + ); + expect(off).toContain("Do NOT include code suggestion blocks"); + expect(off).not.toContain("suggestion block rules"); + }); +}); + +describe("generateGitlabReviewValidatorPrompt", () => { + it("identifies itself as Phase 2 / validator", () => { + const prompt = generateGitlabReviewValidatorPrompt(baseCtx()); + expect(prompt).toContain("Phase 2 (validator)"); + expect(prompt).toContain("Pass 2: Validation"); + }); + + it("uses GitLab MR terminology, not GitHub PR terminology", () => { + const prompt = generateGitlabReviewValidatorPrompt(baseCtx()); + expect(prompt).toContain("MR !42"); + expect(prompt).not.toMatch(/\bPR #\d+\b/); + expect(prompt).not.toContain("github_pr___submit_review"); + }); + + it("instructs the model to batch approved findings via gitlab_mr___submit_review", () => { + const prompt = generateGitlabReviewValidatorPrompt(baseCtx()); + expect(prompt).toContain("gitlab_mr___submit_review"); + expect(prompt).toContain("single batched review"); + expect(prompt).toContain("mr_iid: 42"); + expect(prompt).toContain("gitlab_mr___update_tracking_note"); + }); + + it("enforces the approved/rejected ordering contract", () => { + const prompt = generateGitlabReviewValidatorPrompt(baseCtx()); + expect(prompt).toContain('status === "approved"'); + expect(prompt).toContain("Preserve ordering"); + expect(prompt).toContain("exactly one entry per candidate"); + }); + + it("points at the right artifact paths from state", () => { + const ctx = baseCtx({ + diffPath: "/custom/mr.diff", + commentsPath: "/custom/comments.json", + descriptionPath: "/custom/desc.txt", + candidatesPath: "/custom/candidates.json", + validatedPath: "/custom/validated.json", + }); + const prompt = generateGitlabReviewValidatorPrompt(ctx); + expect(prompt).toContain("/custom/mr.diff"); + expect(prompt).toContain("/custom/comments.json"); + expect(prompt).toContain("/custom/desc.txt"); + expect(prompt).toContain("/custom/candidates.json"); + expect(prompt).toContain("/custom/validated.json"); + }); +}); diff --git a/test/gitlab/review-artifacts.test.ts b/test/gitlab/review-artifacts.test.ts new file mode 100644 index 0000000..047bcc4 --- /dev/null +++ b/test/gitlab/review-artifacts.test.ts @@ -0,0 +1,153 @@ +import { describe, it, expect, beforeEach, afterEach } from "bun:test"; +import * as fs from "fs/promises"; +import * as os from "os"; +import * as path from "path"; +import { + buildDescriptionContent, + buildDiffContent, + computeReviewArtifacts, +} from "../../src/gitlab/data/review-artifacts"; +import type { GitlabClient } from "../../src/gitlab/api/client"; + +function fakeClient(opts: { + mr?: object; + changes?: object; + notes?: object; +}): GitlabClient { + return { + getMr: async () => opts.mr ?? {}, + getMrChanges: async () => opts.changes ?? { changes: [] }, + listNotes: async () => opts.notes ?? [], + } as unknown as GitlabClient; +} + +describe("buildDiffContent", () => { + it("returns empty string when there are no changes", () => { + expect(buildDiffContent({ changes: [] } as never)).toBe(""); + }); + + it("emits a `diff --git` header per file using old/new paths", () => { + const out = buildDiffContent({ + changes: [ + { + old_path: "a/foo.ts", + new_path: "a/foo.ts", + diff: "@@ -1 +1 @@\n-old\n+new\n", + a_mode: "", + b_mode: "", + new_file: false, + renamed_file: false, + deleted_file: false, + }, + { + old_path: "b/old.ts", + new_path: "b/renamed.ts", + diff: "@@ -1 +1 @@\n line\n", + a_mode: "", + b_mode: "", + new_file: false, + renamed_file: true, + deleted_file: false, + }, + ], + } as never); + expect(out).toContain("diff --git a/a/foo.ts b/a/foo.ts"); + expect(out).toContain("diff --git a/b/old.ts b/b/renamed.ts"); + expect(out).toContain("-old"); + expect(out).toContain("+new"); + }); + + it("falls back to new_path when old_path is missing (new file)", () => { + const out = buildDiffContent({ + changes: [ + { + old_path: "", + new_path: "src/new.ts", + diff: "+content\n", + a_mode: "", + b_mode: "", + new_file: true, + renamed_file: false, + deleted_file: false, + }, + ], + } as never); + expect(out).toContain("diff --git a/src/new.ts b/src/new.ts"); + }); +}); + +describe("buildDescriptionContent", () => { + it("includes the title and description", () => { + const out = buildDescriptionContent({ + title: "Add feature X", + description: "Closes #1.", + } as never); + expect(out).toContain("Title: Add feature X"); + expect(out).toContain("Closes #1."); + }); + + it("tolerates a null description", () => { + const out = buildDescriptionContent({ + title: "T", + description: null, + } as never); + expect(out).toContain("Title: T"); + }); +}); + +describe("computeReviewArtifacts", () => { + let tmp: string; + beforeEach(async () => { + tmp = await fs.mkdtemp(path.join(os.tmpdir(), "droid-artifacts-")); + }); + afterEach(async () => { + await fs.rm(tmp, { recursive: true, force: true }); + }); + + it("writes mr.diff, existing_comments.json, mr_description.txt", async () => { + const client = fakeClient({ + mr: { + title: "Hello", + description: "world", + diff_refs: { base_sha: "b", head_sha: "h", start_sha: "s" }, + }, + changes: { + changes: [ + { + old_path: "x.ts", + new_path: "x.ts", + diff: "@@ -1 +1 @@\n+x\n", + a_mode: "", + b_mode: "", + new_file: false, + renamed_file: false, + deleted_file: false, + }, + ], + }, + notes: [{ id: 1, body: "hi", system: false }], + }); + + const result = await computeReviewArtifacts({ + client, + projectId: "42", + mrIid: 7, + outDir: tmp, + }); + + expect(result.diffPath).toBe(path.join(tmp, "mr.diff")); + expect(result.commentsPath).toBe(path.join(tmp, "existing_comments.json")); + expect(result.descriptionPath).toBe(path.join(tmp, "mr_description.txt")); + + const diff = await fs.readFile(result.diffPath, "utf8"); + expect(diff).toContain("diff --git a/x.ts b/x.ts"); + + const comments = JSON.parse(await fs.readFile(result.commentsPath, "utf8")); + expect(comments).toHaveLength(1); + expect(comments[0].body).toBe("hi"); + + const desc = await fs.readFile(result.descriptionPath, "utf8"); + expect(desc).toContain("Title: Hello"); + expect(desc).toContain("world"); + }); +}); diff --git a/test/gitlab/review-prompt.test.ts b/test/gitlab/review-prompt.test.ts deleted file mode 100644 index 18d497a..0000000 --- a/test/gitlab/review-prompt.test.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { describe, expect, it } from "bun:test"; -import { buildGitlabReviewPrompt } from "../../src/gitlab/review-prompt"; - -describe("buildGitlabReviewPrompt", () => { - it("renders project path, MR iid, title, and diff", () => { - const prompt = buildGitlabReviewPrompt({ - projectPath: "group/sub/repo", - mrIid: 42, - mrTitle: "Add widget", - diff: "diff --git a/x.ts b/x.ts\n+const a = 1;", - reviewDepth: "deep", - }); - - expect(prompt).toContain('merge request !42 in project "group/sub/repo"'); - expect(prompt).toContain("Title: Add widget"); - expect(prompt).toContain("```diff"); - expect(prompt).toContain("+const a = 1;"); - expect(prompt).toContain("submit_review"); - expect(prompt).toContain("mr_iid=42"); - }); - - it("uses (empty diff) when diff is blank", () => { - const prompt = buildGitlabReviewPrompt({ - projectPath: "g/r", - mrIid: 1, - mrTitle: null, - diff: "", - reviewDepth: "shallow", - }); - expect(prompt).toContain("(empty diff)"); - expect(prompt).not.toContain("Title:"); - }); - - it("respects custom maxInlineComments", () => { - const prompt = buildGitlabReviewPrompt({ - projectPath: "g/r", - mrIid: 1, - mrTitle: null, - diff: "x", - reviewDepth: "deep", - maxInlineComments: 3, - }); - expect(prompt).toContain("at most 3 inline comments"); - }); -}); From 935519c99b4b080edf95630a2fa2811e80d0f22e Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 16:40:50 -0700 Subject: [PATCH 09/29] feat(gitlab): wire `settings` input pass-through (parity with GitHub) Adds the same `settings` input the GitHub action exposes: * spec.input on the CI/CD Component (JSON string OR path to JSON file). * Plumbed via DROID_SETTINGS env var to gitlab-prepare. * gitlab-prepare reuses base-action/src/setup-droid-settings.ts (zero GitHub deps, fully portable) to write ~/.factory/droid/settings.json before either droid exec call. Always sets enableAllProjectMcpServers=true to match the GitHub flow. * context.inputs.settings surfaces the raw value. * 2 new context tests; 434 tests pass. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- gitlab/templates/review.yml | 7 +++++++ src/entrypoints/gitlab-prepare.ts | 14 ++++++++++++++ src/gitlab/context.ts | 2 ++ test/gitlab/context.test.ts | 20 ++++++++++++++++++++ 4 files changed, 43 insertions(+) diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index bba7a86..a3f5764 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -12,6 +12,12 @@ spec: reasoning_effort: description: "Override reasoning effort. Empty = use depth preset." default: "" + settings: + description: | + Droid Exec settings as a JSON string or a path to a JSON file. + Merged into ~/.factory/droid/settings.json on the runner before + `droid exec` runs. Mirrors the GitHub action's `settings` input. + default: "" droid_action_repo: description: | Git repo (clone URL) of droid-action. Override if you mirror it @@ -44,6 +50,7 @@ droid-review: REVIEW_DEPTH: "$[[ inputs.review_depth ]]" REVIEW_MODEL: "$[[ inputs.review_model ]]" REASONING_EFFORT: "$[[ inputs.reasoning_effort ]]" + DROID_SETTINGS: "$[[ inputs.settings ]]" DROID_SUCCESS: "true" before_script: - apt-get update -qq diff --git a/src/entrypoints/gitlab-prepare.ts b/src/entrypoints/gitlab-prepare.ts index 8fa3c22..5fec141 100644 --- a/src/entrypoints/gitlab-prepare.ts +++ b/src/entrypoints/gitlab-prepare.ts @@ -36,6 +36,7 @@ import { computeReviewArtifacts } from "../gitlab/data/review-artifacts"; import { generateGitlabReviewCandidatesPrompt } from "../gitlab/prompts/candidates"; import type { GitlabReviewPromptContext } from "../gitlab/prompts/types"; import { resolveReviewConfig } from "../utils/review-depth"; +import { setupDroidSettings } from "../../base-action/src/setup-droid-settings"; export type PrepareState = { shouldRunReview: boolean; @@ -191,6 +192,19 @@ async function run(): Promise { throw err; } + // Write ~/.factory/droid/settings.json. Accepts either a raw JSON string + // or a path to a JSON file (matching the GitHub action's `settings` + // input). Always sets enableAllProjectMcpServers=true so project-scope + // MCP servers (if any) are trusted without prompting. + try { + await setupDroidSettings(context.inputs.settings || undefined); + } catch (err) { + console.warn( + "Failed to setup droid settings; continuing with defaults:", + err, + ); + } + const client = new GitlabClient(token, context.apiUrl); const mrIid = context.mr.iid; const projectId = context.project.id; diff --git a/src/gitlab/context.ts b/src/gitlab/context.ts index 87a60f2..1a1576f 100644 --- a/src/gitlab/context.ts +++ b/src/gitlab/context.ts @@ -44,6 +44,7 @@ export type ParsedGitlabContext = { securityNotifyTeam: string; securityScanSchedule: boolean; securityScanDays: number; + settings: string; }; }; @@ -135,6 +136,7 @@ export function parseGitlabContext(): ParsedGitlabContext { 1, parseInt(process.env.SECURITY_SCAN_DAYS ?? "7", 10) || 7, ), + settings: process.env.DROID_SETTINGS ?? "", }, }; } diff --git a/test/gitlab/context.test.ts b/test/gitlab/context.test.ts index a605e63..12fbb49 100644 --- a/test/gitlab/context.test.ts +++ b/test/gitlab/context.test.ts @@ -41,6 +41,7 @@ const REQUIRED_CI_ENV = [ "SECURITY_NOTIFY_TEAM", "SECURITY_SCAN_SCHEDULE", "SECURITY_SCAN_DAYS", + "DROID_SETTINGS", ]; function clearEnv() { @@ -165,4 +166,23 @@ describe("parseGitlabContext", () => { const ctx = parseGitlabContext(); expect(ctx.inputs.securityScanDays).toBe(7); }); + + it("surfaces DROID_SETTINGS as inputs.settings", () => { + process.env.CI_PROJECT_ID = "1"; + process.env.CI_PROJECT_PATH = "g/r"; + process.env.CI_COMMIT_SHA = "x"; + process.env.DROID_SETTINGS = '{"reasoning_effort":"medium"}'; + + const ctx = parseGitlabContext(); + expect(ctx.inputs.settings).toBe('{"reasoning_effort":"medium"}'); + }); + + it("defaults inputs.settings to empty string when unset", () => { + process.env.CI_PROJECT_ID = "1"; + process.env.CI_PROJECT_PATH = "g/r"; + process.env.CI_COMMIT_SHA = "x"; + + const ctx = parseGitlabContext(); + expect(ctx.inputs.settings).toBe(""); + }); }); From 7df3492eac1cb0f074ada98119716ac030c3295b Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 17:09:52 -0700 Subject: [PATCH 10/29] =?UTF-8?q?feat(gitlab):=20GH=20parity=20batch=20?= =?UTF-8?q?=E2=80=94=20custom=20droids,=20debug=20artifacts,=20telemetry,?= =?UTF-8?q?=20draft=20skip,=20stderr=20capture?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 1.6b Copy bundled .factory/droids from droid-action checkout into runner ~/.factory/droids before droid exec, mirroring the GitHub action's Setup Custom Droids step. - 1.6c Stage debug artifacts (/tmp/droid-prompts/*, droid-error.txt) into .droid-debug/ inside CI_PROJECT_DIR in after_script so GitLab can upload them on failure (GitLab artifacts cannot reach outside CI_PROJECT_DIR). - 1.6d Capture droid exec stream-json output to per-pass JSONL log files via tee + pipefail; parse session_id / numTurns / durationMs (+ legacy cost_usd) from the last completion event of each pass; render a summary line (44 turns • 11m 40s • $0.42) and a collapsible session-IDs block in the sticky tracking note. - C7 Skip Draft: / WIP: MRs at the rules: level (GitLab-native equivalent of GitHub's draft == false workflow if:). - C9 On droid exec failure, tail 60 lines of the per-pass log into /tmp/droid-error.txt so the existing error-details block in the sticky note surfaces real failure context instead of a generic message. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- gitlab/templates/review.yml | 48 +++++- src/entrypoints/gitlab-update-comment-link.ts | 15 ++ src/gitlab/data/exec-telemetry.ts | 149 ++++++++++++++++++ src/gitlab/operations/tracking-note.ts | 46 ++++++ test/gitlab/exec-telemetry.test.ts | 116 ++++++++++++++ test/gitlab/tracking-note.test.ts | 40 +++++ 6 files changed, 407 insertions(+), 7 deletions(-) create mode 100644 src/gitlab/data/exec-telemetry.ts create mode 100644 test/gitlab/exec-telemetry.test.ts diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index a3f5764..a62407b 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -39,6 +39,12 @@ droid-review: stage: $[[ inputs.stage ]] image: oven/bun:1.2.11 rules: + # Skip draft / WIP MRs (parity with GitHub's `if: ... draft == false` + # pattern). The legacy "WIP:" prefix is also covered. + - if: '$CI_PIPELINE_SOURCE == "merge_request_event" && $CI_MERGE_REQUEST_TITLE =~ /^Draft:/' + when: never + - if: '$CI_PIPELINE_SOURCE == "merge_request_event" && $CI_MERGE_REQUEST_TITLE =~ /^WIP:/' + when: never - if: '$CI_PIPELINE_SOURCE == "merge_request_event"' variables: DROID_STATE_FILE: "$CI_PROJECT_DIR/.droid-state.json" @@ -61,6 +67,15 @@ droid-review: - curl --retry 5 --retry-delay 2 --retry-all-errors -fsSL https://app.factory.ai/cli | sh - export PATH="$HOME/.local/bin:$PATH" - droid --version + # Copy bundled custom droids (e.g. security-reviewer) into the runner's + # ~/.factory/droids so the Skill / Task subagents can find them. Mirrors + # the GitHub action's "Setup Custom Droids" step. + - mkdir -p ~/.factory/droids + - | + if [ -d "$DROID_ACTION_DIR/.factory/droids" ]; then + cp -r "$DROID_ACTION_DIR/.factory/droids/." ~/.factory/droids/ + echo "Copied custom droids from $DROID_ACTION_DIR/.factory/droids to ~/.factory/droids" + fi script: - export PATH="$HOME/.local/bin:$PATH" - | @@ -95,14 +110,21 @@ droid-review: # ---- Pass 1: candidate generation ---- # Note: gitlab_mr___submit_review and the MR-mutation tools are # NOT included here. Pass 1 must only WRITE the candidates JSON. + set -o pipefail PASS1_TOOLS="Read,Grep,Glob,LS,Execute,Edit,Create,ApplyPatch,Task,Skill,FetchUrl" + PASS1_LOG="/tmp/droid-prompts/pass1-output.jsonl" DROID_ARGS="exec -f $DROID_PROMPT_FILE --enabled-tools $PASS1_TOOLS --output-format stream-json --skip-permissions-unsafe --tag code-review" if [ -n "$RESOLVED_MODEL" ]; then DROID_ARGS="$DROID_ARGS --model $RESOLVED_MODEL"; fi if [ -n "$RESOLVED_REASONING_EFFORT" ]; then DROID_ARGS="$DROID_ARGS --reasoning-effort $RESOLVED_REASONING_EFFORT"; fi echo "Running Pass 1 (candidates): droid $DROID_ARGS" - if ! droid $DROID_ARGS; then + if ! droid $DROID_ARGS 2>&1 | tee "$PASS1_LOG"; then export DROID_SUCCESS=false - echo "droid exec pass 1 (candidates) failed" > /tmp/droid-error.txt + { + echo "droid exec pass 1 (candidates) failed" + echo "" + echo "--- last 60 lines of pass 1 output ---" + tail -n 60 "$PASS1_LOG" 2>/dev/null || true + } > /tmp/droid-error.txt exit 1 fi - | @@ -117,25 +139,37 @@ droid-review: # gitlab_mr___submit_review and gitlab_mr___update_tracking_note are # the only MR-mutation tools we expose. The model batches all approved # findings into a single submit_review call. + set -o pipefail PASS2_TOOLS="Read,Grep,Glob,LS,Execute,Edit,Create,ApplyPatch,Skill,gitlab_mr___submit_review,gitlab_mr___update_tracking_note" + PASS2_LOG="/tmp/droid-prompts/pass2-output.jsonl" DROID_ARGS="exec -f $DROID_PROMPT_FILE --enabled-tools $PASS2_TOOLS --output-format stream-json --skip-permissions-unsafe --tag code-review" if [ -n "$RESOLVED_MODEL" ]; then DROID_ARGS="$DROID_ARGS --model $RESOLVED_MODEL"; fi if [ -n "$RESOLVED_REASONING_EFFORT" ]; then DROID_ARGS="$DROID_ARGS --reasoning-effort $RESOLVED_REASONING_EFFORT"; fi echo "Running Pass 2 (validator): droid $DROID_ARGS" - if ! droid $DROID_ARGS; then + if ! droid $DROID_ARGS 2>&1 | tee "$PASS2_LOG"; then export DROID_SUCCESS=false - echo "droid exec pass 2 (validator) failed" > /tmp/droid-error.txt + { + echo "droid exec pass 2 (validator) failed" + echo "" + echo "--- last 60 lines of pass 2 output ---" + tail -n 60 "$PASS2_LOG" 2>/dev/null || true + } > /tmp/droid-error.txt exit 1 fi after_script: - export PATH="$HOME/.local/bin:$PATH" - export DROID_ERROR_DETAILS="$(cat /tmp/droid-error.txt 2>/dev/null || true)" + - export DROID_PASS1_LOG="/tmp/droid-prompts/pass1-output.jsonl" + - export DROID_PASS2_LOG="/tmp/droid-prompts/pass2-output.jsonl" + # Stage debug artifacts that live outside CI_PROJECT_DIR so GitLab can + # capture them. Failure here must not break the after_script chain. + - mkdir -p "$CI_PROJECT_DIR/.droid-debug/prompts" || true + - cp -r /tmp/droid-prompts/. "$CI_PROJECT_DIR/.droid-debug/prompts/" 2>/dev/null || true + - cp /tmp/droid-error.txt "$CI_PROJECT_DIR/.droid-debug/" 2>/dev/null || true - bun run "$DROID_ACTION_DIR/src/entrypoints/gitlab-update-comment-link.ts" || true artifacts: when: always paths: - .droid-state.json - - /tmp/droid-prompts/review_candidates.json - - /tmp/droid-prompts/review_validated.json - - /tmp/droid-prompts/droid-prompt.txt + - .droid-debug/ expire_in: 1 week diff --git a/src/entrypoints/gitlab-update-comment-link.ts b/src/entrypoints/gitlab-update-comment-link.ts index d4c951c..a190552 100644 --- a/src/entrypoints/gitlab-update-comment-link.ts +++ b/src/entrypoints/gitlab-update-comment-link.ts @@ -19,6 +19,7 @@ import * as path from "path"; import { setupGitlabToken } from "../gitlab/token"; import { GitlabClient } from "../gitlab/api/client"; import { buildTrackingNoteBody } from "../gitlab/operations/tracking-note"; +import { collectExecTelemetry } from "../gitlab/data/exec-telemetry"; type PrepareState = { shouldRunReview: boolean; @@ -81,6 +82,13 @@ async function run(): Promise { const pipelineUrl = process.env.CI_PIPELINE_URL || state.pipelineUrl; const jobUrl = process.env.CI_JOB_URL || state.jobUrl; + const telemetry = await collectExecTelemetry({ + pass1LogPath: + process.env.DROID_PASS1_LOG || "/tmp/droid-prompts/pass1-output.jsonl", + pass2LogPath: + process.env.DROID_PASS2_LOG || "/tmp/droid-prompts/pass2-output.jsonl", + }); + const body = buildTrackingNoteBody({ state: droidSuccess ? "success" : "failure", pipelineUrl, @@ -88,6 +96,13 @@ async function run(): Promise { triggerUsername, errorDetails, securityReviewRan, + telemetry: { + totalNumTurns: telemetry.totalNumTurns, + totalDurationMs: telemetry.totalDurationMs, + totalCostUsd: telemetry.totalCostUsd, + pass1SessionId: telemetry.pass1?.sessionId ?? null, + pass2SessionId: telemetry.pass2?.sessionId ?? null, + }, }); await client.updateNote( diff --git a/src/gitlab/data/exec-telemetry.ts b/src/gitlab/data/exec-telemetry.ts new file mode 100644 index 0000000..76bf30a --- /dev/null +++ b/src/gitlab/data/exec-telemetry.ts @@ -0,0 +1,149 @@ +/** + * Parse `droid exec --output-format stream-json` output captured to a + * JSONL log file by the GitLab CI template, and extract a small summary + * of execution telemetry (session IDs, turn counts, durations, token + * usage). The result is rendered into the sticky tracking note so a + * reviewer can sanity-check what Droid did without opening the full + * job log. + * + * Mirrors the surface area GitHub's `update-comment-link.ts` extracts + * from its array-format result file, parameterized for our stream-json + * one-event-per-line format. + */ + +import * as fs from "fs/promises"; + +export type PassTelemetry = { + sessionId: string | null; + numTurns: number | null; + durationMs: number | null; + inputTokens: number | null; + outputTokens: number | null; + cacheReadInputTokens: number | null; + costUsd: number | null; +}; + +export type ExecTelemetry = { + pass1: PassTelemetry | null; + pass2: PassTelemetry | null; + totalCostUsd: number | null; + totalDurationMs: number | null; + totalNumTurns: number | null; +}; + +const EMPTY_PASS: PassTelemetry = { + sessionId: null, + numTurns: null, + durationMs: null, + inputTokens: null, + outputTokens: null, + cacheReadInputTokens: null, + costUsd: null, +}; + +function readNumber(value: unknown): number | null { + if (typeof value === "number" && Number.isFinite(value)) { + return value; + } + return null; +} + +function readString(value: unknown): string | null { + if (typeof value === "string" && value.length > 0) { + return value; + } + return null; +} + +export function extractPassTelemetry(lines: string[]): PassTelemetry | null { + let result: PassTelemetry | null = null; + + for (let i = lines.length - 1; i >= 0; i--) { + const line = lines[i]?.trim(); + if (!line || !line.startsWith("{")) continue; + let event: Record; + try { + event = JSON.parse(line) as Record; + } catch { + continue; + } + if (event.type !== "completion" && event.type !== "result") continue; + + const usage = (event.usage as Record | undefined) ?? {}; + result = { + sessionId: readString(event.session_id) ?? readString(event.sessionId), + numTurns: readNumber(event.numTurns) ?? readNumber(event.num_turns), + durationMs: readNumber(event.durationMs) ?? readNumber(event.duration_ms), + inputTokens: + readNumber(usage.input_tokens) ?? readNumber(usage.inputTokens), + outputTokens: + readNumber(usage.output_tokens) ?? readNumber(usage.outputTokens), + cacheReadInputTokens: + readNumber(usage.cache_read_input_tokens) ?? + readNumber(usage.cacheReadInputTokens), + costUsd: readNumber(event.cost_usd) ?? readNumber(event.costUsd), + }; + break; + } + + return result; +} + +export async function parseTelemetryFile( + filePath: string, +): Promise { + try { + const raw = await fs.readFile(filePath, "utf8"); + const lines = raw.split(/\r?\n/); + return extractPassTelemetry(lines); + } catch { + return null; + } +} + +export async function collectExecTelemetry(opts: { + pass1LogPath?: string | null; + pass2LogPath?: string | null; +}): Promise { + const pass1 = opts.pass1LogPath + ? await parseTelemetryFile(opts.pass1LogPath) + : null; + const pass2 = opts.pass2LogPath + ? await parseTelemetryFile(opts.pass2LogPath) + : null; + + const sumNullable = (a: number | null, b: number | null): number | null => { + if (a === null && b === null) return null; + return (a ?? 0) + (b ?? 0); + }; + + return { + pass1, + pass2, + totalCostUsd: sumNullable(pass1?.costUsd ?? null, pass2?.costUsd ?? null), + totalDurationMs: sumNullable( + pass1?.durationMs ?? null, + pass2?.durationMs ?? null, + ), + totalNumTurns: sumNullable( + pass1?.numTurns ?? null, + pass2?.numTurns ?? null, + ), + }; +} + +export function isEmptyPass(p: PassTelemetry | null | undefined): boolean { + if (!p) return true; + return ( + p.sessionId === null && + p.numTurns === null && + p.durationMs === null && + p.inputTokens === null && + p.outputTokens === null && + p.costUsd === null + ); +} + +export function emptyPass(): PassTelemetry { + return { ...EMPTY_PASS }; +} diff --git a/src/gitlab/operations/tracking-note.ts b/src/gitlab/operations/tracking-note.ts index 5566deb..286da9d 100644 --- a/src/gitlab/operations/tracking-note.ts +++ b/src/gitlab/operations/tracking-note.ts @@ -10,6 +10,14 @@ export const DROID_SECURITY_BADGE_MARKER = ""; export type TrackingNoteState = "running" | "success" | "failure"; +export type TrackingNoteTelemetry = { + totalNumTurns?: number | null; + totalDurationMs?: number | null; + totalCostUsd?: number | null; + pass1SessionId?: string | null; + pass2SessionId?: string | null; +}; + export interface TrackingNoteOptions { state: TrackingNoteState; pipelineUrl?: string | null; @@ -17,6 +25,21 @@ export interface TrackingNoteOptions { triggerUsername?: string | null; errorDetails?: string | null; securityReviewRan?: boolean; + telemetry?: TrackingNoteTelemetry | null; +} + +function formatDurationMs(ms: number): string { + if (ms < 1000) return `${ms}ms`; + const seconds = ms / 1000; + if (seconds < 60) return `${seconds.toFixed(1)}s`; + const minutes = Math.floor(seconds / 60); + const remSec = Math.round(seconds - minutes * 60); + return `${minutes}m ${remSec}s`; +} + +function formatCostUsd(usd: number): string { + if (usd >= 1) return `$${usd.toFixed(2)}`; + return `$${usd.toFixed(4)}`; } const SECURITY_BADGE = @@ -62,6 +85,29 @@ export function buildTrackingNoteBody(options: TrackingNoteOptions): string { lines.push("
"); } + if (options.telemetry) { + const t = options.telemetry; + const bits: string[] = []; + if (typeof t.totalNumTurns === "number") + bits.push(`${t.totalNumTurns} turns`); + if (typeof t.totalDurationMs === "number") + bits.push(formatDurationMs(t.totalDurationMs)); + if (typeof t.totalCostUsd === "number" && t.totalCostUsd > 0) + bits.push(formatCostUsd(t.totalCostUsd)); + if (bits.length > 0) { + lines.push(""); + lines.push(`${bits.join(" • ")}`); + } + if (t.pass1SessionId || t.pass2SessionId) { + lines.push(""); + lines.push("
Droid session IDs"); + lines.push(""); + if (t.pass1SessionId) lines.push(`- Pass 1: \`${t.pass1SessionId}\``); + if (t.pass2SessionId) lines.push(`- Pass 2: \`${t.pass2SessionId}\``); + lines.push("
"); + } + } + return lines.join("\n").trim() + "\n"; } diff --git a/test/gitlab/exec-telemetry.test.ts b/test/gitlab/exec-telemetry.test.ts new file mode 100644 index 0000000..a9ed21b --- /dev/null +++ b/test/gitlab/exec-telemetry.test.ts @@ -0,0 +1,116 @@ +import { describe, it, expect } from "bun:test"; +import { + collectExecTelemetry, + extractPassTelemetry, + parseTelemetryFile, +} from "../../src/gitlab/data/exec-telemetry"; +import * as fs from "fs/promises"; +import * as os from "os"; +import * as path from "path"; + +describe("extractPassTelemetry", () => { + it("returns null when there are no JSON lines", () => { + expect(extractPassTelemetry([])).toBeNull(); + expect(extractPassTelemetry(["not json", "still not"])).toBeNull(); + }); + + it("picks the last completion event and extracts usage fields", () => { + const lines = [ + '{"type":"system","subtype":"init","session_id":"abc"}', + '{"type":"message","role":"user"}', + '{"type":"completion","finalText":"hi","numTurns":12,"durationMs":42000,"session_id":"final-session","usage":{"input_tokens":1000,"output_tokens":250,"cache_read_input_tokens":50000}}', + ]; + const out = extractPassTelemetry(lines); + expect(out).not.toBeNull(); + expect(out!.sessionId).toBe("final-session"); + expect(out!.numTurns).toBe(12); + expect(out!.durationMs).toBe(42000); + expect(out!.inputTokens).toBe(1000); + expect(out!.outputTokens).toBe(250); + expect(out!.cacheReadInputTokens).toBe(50000); + expect(out!.costUsd).toBeNull(); + }); + + it("also recognizes legacy result events with cost_usd", () => { + const lines = [ + '{"type":"result","cost_usd":0.42,"duration_ms":15000,"num_turns":8,"session_id":"sess"}', + ]; + const out = extractPassTelemetry(lines); + expect(out).not.toBeNull(); + expect(out!.costUsd).toBe(0.42); + expect(out!.durationMs).toBe(15000); + expect(out!.numTurns).toBe(8); + }); + + it("ignores malformed JSON lines and keeps scanning", () => { + const lines = [ + "not json at all", + "{broken", + '{"type":"completion","numTurns":3,"session_id":"x"}', + ]; + const out = extractPassTelemetry(lines); + expect(out!.numTurns).toBe(3); + expect(out!.sessionId).toBe("x"); + }); +}); + +describe("parseTelemetryFile", () => { + it("returns null when file doesn't exist", async () => { + const out = await parseTelemetryFile("/does/not/exist.jsonl"); + expect(out).toBeNull(); + }); + + it("parses a real-looking pass log", async () => { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "droid-tele-")); + const file = path.join(tmp, "pass.jsonl"); + await fs.writeFile( + file, + [ + '{"type":"system","subtype":"init","session_id":"s1"}', + '{"type":"completion","numTurns":15,"durationMs":311617,"session_id":"s1","usage":{"input_tokens":35300,"output_tokens":10341}}', + ].join("\n"), + ); + const out = await parseTelemetryFile(file); + expect(out!.sessionId).toBe("s1"); + expect(out!.numTurns).toBe(15); + expect(out!.durationMs).toBe(311617); + await fs.rm(tmp, { recursive: true, force: true }); + }); +}); + +describe("collectExecTelemetry", () => { + it("returns null passes and null totals when both files missing", async () => { + const out = await collectExecTelemetry({ + pass1LogPath: "/missing/pass1.jsonl", + pass2LogPath: "/missing/pass2.jsonl", + }); + expect(out.pass1).toBeNull(); + expect(out.pass2).toBeNull(); + expect(out.totalCostUsd).toBeNull(); + expect(out.totalDurationMs).toBeNull(); + expect(out.totalNumTurns).toBeNull(); + }); + + it("sums numTurns and durations across both passes", async () => { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "droid-tele-")); + const p1 = path.join(tmp, "p1.jsonl"); + const p2 = path.join(tmp, "p2.jsonl"); + await fs.writeFile( + p1, + '{"type":"completion","numTurns":15,"durationMs":300000,"session_id":"s1"}', + ); + await fs.writeFile( + p2, + '{"type":"completion","numTurns":29,"durationMs":400000,"session_id":"s2"}', + ); + const out = await collectExecTelemetry({ + pass1LogPath: p1, + pass2LogPath: p2, + }); + expect(out.pass1!.sessionId).toBe("s1"); + expect(out.pass2!.sessionId).toBe("s2"); + expect(out.totalNumTurns).toBe(44); + expect(out.totalDurationMs).toBe(700000); + await fs.rm(tmp, { recursive: true, force: true }); + }); +}); diff --git a/test/gitlab/tracking-note.test.ts b/test/gitlab/tracking-note.test.ts index 1bbc1cd..409ad18 100644 --- a/test/gitlab/tracking-note.test.ts +++ b/test/gitlab/tracking-note.test.ts @@ -53,6 +53,46 @@ describe("buildTrackingNoteBody", () => { }); expect(success).not.toContain("
"); }); + + it("renders telemetry summary line when totals are provided", () => { + const body = buildTrackingNoteBody({ + state: "success", + telemetry: { + totalNumTurns: 44, + totalDurationMs: 700000, + totalCostUsd: 0.42, + }, + }); + expect(body).toContain("44 turns"); + expect(body).toContain("11m 40s"); + expect(body).toContain("$0.42"); + }); + + it("includes session IDs in a collapsible block when provided", () => { + const body = buildTrackingNoteBody({ + state: "success", + telemetry: { + pass1SessionId: "sess-1", + pass2SessionId: "sess-2", + }, + }); + expect(body).toContain("Droid session IDs"); + expect(body).toContain("`sess-1`"); + expect(body).toContain("`sess-2`"); + }); + + it("omits telemetry block entirely when telemetry is missing or empty", () => { + const none = buildTrackingNoteBody({ state: "success" }); + expect(none).not.toContain("turns"); + expect(none).not.toContain("Droid session IDs"); + + const empty = buildTrackingNoteBody({ + state: "success", + telemetry: {}, + }); + expect(empty).not.toContain("turns"); + expect(empty).not.toContain("Droid session IDs"); + }); }); describe("findExistingTrackingNote", () => { From efdd18be2868d2bd6989fca822e2040376cbcc4a Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 17:11:29 -0700 Subject: [PATCH 11/29] feat(gitlab): expose include_suggestions input (A5) Adds the include_suggestions spec input to the GitLab review template. Threading was already in place: gitlab-prepare reads the INCLUDE_SUGGESTIONS env var, stores it in the on-disk state, gitlab-prepare-validator restores it, and both candidate + validator prompts already consume includeSuggestions to gate suggestion-block guidance. This commit just exposes the toggle as a customer-facing input. Mirrors the GitHub action's include_suggestions input. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- gitlab/templates/review.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index a62407b..415cca5 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -12,6 +12,12 @@ spec: reasoning_effort: description: "Override reasoning effort. Empty = use depth preset." default: "" + include_suggestions: + description: | + Include code suggestion blocks in review comments when the fix is + high-confidence. Set to "false" to disable suggestion blocks + entirely. Mirrors the GitHub action's `include_suggestions` input. + default: "true" settings: description: | Droid Exec settings as a JSON string or a path to a JSON file. @@ -56,6 +62,7 @@ droid-review: REVIEW_DEPTH: "$[[ inputs.review_depth ]]" REVIEW_MODEL: "$[[ inputs.review_model ]]" REASONING_EFFORT: "$[[ inputs.reasoning_effort ]]" + INCLUDE_SUGGESTIONS: "$[[ inputs.include_suggestions ]]" DROID_SETTINGS: "$[[ inputs.settings ]]" DROID_SUCCESS: "true" before_script: From 3def7bc2868204828ca1bad200e86cbd9b7208ca Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 17:13:15 -0700 Subject: [PATCH 12/29] feat(gitlab): cache bun install cache across pipelines (C8) Adds a cache: block keyed on droid_action_ref so bun's package cache survives across MR pipelines. BUN_INSTALL_CACHE_DIR is pinned to CI_PROJECT_DIR/.bun-cache because GitLab's cache mechanism can only persist paths inside the project directory. policy: pull-push lets every job benefit from prior downloads and contribute newly-fetched packages back to the cache. Mirrors the actions/cache pattern in the GitHub action. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- gitlab/templates/review.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index 415cca5..6369368 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -65,6 +65,19 @@ droid-review: INCLUDE_SUGGESTIONS: "$[[ inputs.include_suggestions ]]" DROID_SETTINGS: "$[[ inputs.settings ]]" DROID_SUCCESS: "true" + # Pin bun's install cache to a stable path inside CI_PROJECT_DIR so + # GitLab's `cache:` rules can persist it across pipelines. Mirrors + # actions/cache usage in the GitHub action. + BUN_INSTALL_CACHE_DIR: "$CI_PROJECT_DIR/.bun-cache" + # Cache the cloned droid-action checkout and bun's package cache to + # avoid re-cloning + re-downloading dependencies on every MR pipeline. + # The `key:` is pinned to the droid_action_ref so a ref change cleanly + # invalidates without manual purging. + cache: + key: "droid-action-$[[ inputs.droid_action_ref ]]" + paths: + - .bun-cache/ + policy: pull-push before_script: - apt-get update -qq - apt-get install -y -qq --no-install-recommends git ca-certificates curl From 94663515f4b4c3fe7afe9fbfea92838e1795aef7 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 17:17:26 -0700 Subject: [PATCH 13/29] feat(gitlab): rate-limit + exponential backoff in GitlabClient (C1) Wraps every GitlabClient.request call with retry-on-transient-failure logic, mirroring the implicit retry behaviour the GitHub action gets from Octokit. Retries are limited to a configurable maxRetries (default 5) for: 408, 429, 500, 502, 503, 504, and raw fetch network errors. On 429 the client honours the Retry-After header when present (seconds or HTTP-date), and otherwise uses exponential backoff with jitter clamped to maxDelayMs. Non-retryable 4xx errors (401, 403, 404, etc.) surface immediately as GitlabApiError. The retry knobs are exposed via a third constructor argument so tests can use sub-millisecond delays without needing fake timers. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- src/gitlab/api/client.ts | 95 +++++++++++++++++++++++++----- test/gitlab/api-client.test.ts | 104 +++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 13 deletions(-) diff --git a/src/gitlab/api/client.ts b/src/gitlab/api/client.ts index 0747d4e..dce14a4 100644 --- a/src/gitlab/api/client.ts +++ b/src/gitlab/api/client.ts @@ -21,13 +21,61 @@ export class GitlabApiError extends Error { type RequestInitWithJson = Omit & { json?: unknown }; +export type RetryOptions = { + maxRetries: number; + baseDelayMs: number; + maxDelayMs: number; +}; + +const DEFAULT_RETRY: RetryOptions = { + maxRetries: 5, + baseDelayMs: 500, + maxDelayMs: 30_000, +}; + +const RETRYABLE_STATUS = new Set([408, 429, 500, 502, 503, 504]); + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +function parseRetryAfter(header: string | null): number | null { + if (!header) return null; + const asInt = Number.parseInt(header, 10); + if (Number.isFinite(asInt) && asInt >= 0) return asInt * 1000; + const asDate = Date.parse(header); + if (Number.isFinite(asDate)) { + return Math.max(0, asDate - Date.now()); + } + return null; +} + +function computeBackoffMs( + attempt: number, + retryAfterMs: number | null, + opts: RetryOptions, +): number { + if (retryAfterMs !== null) { + return Math.min(opts.maxDelayMs, retryAfterMs); + } + const exp = opts.baseDelayMs * Math.pow(2, attempt); + const jitter = Math.random() * opts.baseDelayMs; + return Math.min(opts.maxDelayMs, exp + jitter); +} + export class GitlabClient { private readonly baseUrl: string; private readonly token: string; + private readonly retry: RetryOptions; - constructor(token: string, baseUrl: string = GITLAB_API_URL) { + constructor( + token: string, + baseUrl: string = GITLAB_API_URL, + retry: Partial = {}, + ) { this.token = token; this.baseUrl = baseUrl.replace(/\/+$/, ""); + this.retry = { ...DEFAULT_RETRY, ...retry }; } private async request( @@ -48,13 +96,40 @@ export class GitlabClient { } const url = path.startsWith("http") ? path : `${this.baseUrl}${path}`; - const response = await fetch(url, { - ...rest, - headers, - body, - }); - if (!response.ok) { + let attempt = 0; + // attempt 0 = initial request, then up to maxRetries follow-up tries. + while (true) { + let response: Response; + try { + response = await fetch(url, { ...rest, headers, body }); + } catch (err) { + if (attempt >= this.retry.maxRetries) throw err; + await sleep(computeBackoffMs(attempt, null, this.retry)); + attempt++; + continue; + } + + if (response.ok) { + if (response.status === 204) { + return undefined as unknown as T; + } + return (await response.json()) as T; + } + + const shouldRetry = + RETRYABLE_STATUS.has(response.status) && + attempt < this.retry.maxRetries; + if (shouldRetry) { + const retryAfterMs = parseRetryAfter( + response.headers.get("Retry-After"), + ); + const delay = computeBackoffMs(attempt, retryAfterMs, this.retry); + await sleep(delay); + attempt++; + continue; + } + let parsed: unknown = null; const text = await response.text(); try { @@ -68,12 +143,6 @@ export class GitlabClient { parsed, ); } - - if (response.status === 204) { - return undefined as unknown as T; - } - - return (await response.json()) as T; } private projectPath(projectId: string | number): string { diff --git a/test/gitlab/api-client.test.ts b/test/gitlab/api-client.test.ts index 927084f..4b891c4 100644 --- a/test/gitlab/api-client.test.ts +++ b/test/gitlab/api-client.test.ts @@ -106,4 +106,108 @@ describe("GitlabClient", () => { expect((e.body as { message: string }).message).toBe("404 Not Found"); } }); + + it("does not retry on non-retryable 4xx (404, 401, 403)", async () => { + const fetchMock = mockFetch( + () => + new Response("nope", { + status: 404, + statusText: "Not Found", + }), + ); + const client = new GitlabClient("glpat-test", undefined, { + maxRetries: 5, + baseDelayMs: 1, + maxDelayMs: 10, + }); + try { + await client.getMr(42, 999); + throw new Error("expected throw"); + } catch (err) { + expect(err).toBeInstanceOf(GitlabApiError); + } + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it("retries on 429 with exponential backoff and eventually succeeds", async () => { + let calls = 0; + const fetchMock = mockFetch(() => { + calls++; + if (calls < 3) { + return new Response("rate limited", { + status: 429, + statusText: "Too Many Requests", + }); + } + return jsonResponse({ iid: 7 }); + }); + const client = new GitlabClient("glpat-test", undefined, { + maxRetries: 5, + baseDelayMs: 1, + maxDelayMs: 10, + }); + const mr = await client.getMr(42, 7); + expect((mr as { iid: number }).iid).toBe(7); + expect(fetchMock).toHaveBeenCalledTimes(3); + }); + + it("retries on 500/502/503/504 and surfaces the final error after maxRetries", async () => { + const fetchMock = mockFetch( + () => new Response("upstream down", { status: 503 }), + ); + const client = new GitlabClient("glpat-test", undefined, { + maxRetries: 2, + baseDelayMs: 1, + maxDelayMs: 5, + }); + try { + await client.getMr(42, 7); + throw new Error("expected throw"); + } catch (err) { + expect(err).toBeInstanceOf(GitlabApiError); + expect((err as GitlabApiError).status).toBe(503); + } + // 1 initial + 2 retries = 3 attempts + expect(fetchMock).toHaveBeenCalledTimes(3); + }); + + it("honors Retry-After header (seconds) on 429", async () => { + let calls = 0; + const times: number[] = []; + const fetchMock = mockFetch(() => { + times.push(Date.now()); + calls++; + if (calls < 2) { + return new Response("rate limited", { + status: 429, + headers: { "Retry-After": "0" }, + }); + } + return jsonResponse({ iid: 7 }); + }); + const client = new GitlabClient("glpat-test", undefined, { + maxRetries: 3, + baseDelayMs: 1, + maxDelayMs: 10, + }); + await client.getMr(42, 7); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it("retries on fetch network errors", async () => { + let calls = 0; + const fetchMock = mockFetch(() => { + calls++; + if (calls < 2) throw new Error("ECONNRESET"); + return jsonResponse({ iid: 7 }); + }); + const client = new GitlabClient("glpat-test", undefined, { + maxRetries: 3, + baseDelayMs: 1, + maxDelayMs: 5, + }); + const mr = await client.getMr(42, 7); + expect((mr as { iid: number }).iid).toBe(7); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); }); From 3ff7324a17509c28c4afd2cb37c604f018732276 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 17:19:20 -0700 Subject: [PATCH 14/29] feat(gitlab): paginate listNotes + add listDiscussions (C2) Adds a private paginate() helper that follows GitLab's standard pagination either via the X-Next-Page header (newer GitLab) or Link rel="next" (older GitLab + GitLab.com responses) with per_page=100 to minimise round-trips. listNotes now uses it and a new listDiscussions method is exposed for use cases that need to inspect existing diff discussions. Mirrors Octokit's implicit paginate-everything behaviour that the GitHub action gets for free. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- src/gitlab/api/client.ts | 81 +++++++++++++++++++++++++++++----- test/gitlab/api-client.test.ts | 74 ++++++++++++++++++++++++++++++- 2 files changed, 143 insertions(+), 12 deletions(-) diff --git a/src/gitlab/api/client.ts b/src/gitlab/api/client.ts index dce14a4..debd4b6 100644 --- a/src/gitlab/api/client.ts +++ b/src/gitlab/api/client.ts @@ -50,6 +50,15 @@ function parseRetryAfter(header: string | null): number | null { return null; } +export function parseNextLink(linkHeader: string | null): string | null { + if (!linkHeader) return null; + for (const part of linkHeader.split(",")) { + const m = part.match(/<([^>]+)>\s*;\s*rel="next"/); + if (m) return m[1] ?? null; + } + return null; +} + function computeBackoffMs( attempt: number, retryAfterMs: number | null, @@ -78,10 +87,10 @@ export class GitlabClient { this.retry = { ...DEFAULT_RETRY, ...retry }; } - private async request( + private async requestRaw( path: string, init: RequestInitWithJson = {}, - ): Promise { + ): Promise { const { json, headers: rawHeaders, ...rest } = init; const headers: Record = { "PRIVATE-TOKEN": this.token, @@ -98,7 +107,6 @@ export class GitlabClient { const url = path.startsWith("http") ? path : `${this.baseUrl}${path}`; let attempt = 0; - // attempt 0 = initial request, then up to maxRetries follow-up tries. while (true) { let response: Response; try { @@ -110,12 +118,7 @@ export class GitlabClient { continue; } - if (response.ok) { - if (response.status === 204) { - return undefined as unknown as T; - } - return (await response.json()) as T; - } + if (response.ok) return response; const shouldRetry = RETRYABLE_STATUS.has(response.status) && @@ -145,6 +148,53 @@ export class GitlabClient { } } + private async request( + path: string, + init: RequestInitWithJson = {}, + ): Promise { + const response = await this.requestRaw(path, init); + if (response.status === 204) { + return undefined as unknown as T; + } + return (await response.json()) as T; + } + + private appendQuery(url: string, key: string, value: string): string { + const u = new URL(url); + u.searchParams.set(key, value); + return u.toString(); + } + + private async paginate( + path: string, + init: RequestInitWithJson = {}, + ): Promise { + const absolute = path.startsWith("http") ? path : `${this.baseUrl}${path}`; + const firstUrl = this.appendQuery(absolute, "per_page", "100"); + + const out: T[] = []; + let nextUrl: string | null = firstUrl; + + while (nextUrl !== null) { + const response = await this.requestRaw(nextUrl, init); + const chunk = (await response.json()) as T[]; + if (Array.isArray(chunk)) out.push(...chunk); + + // Prefer X-Next-Page (older GitLab) then Link header (rel=next). + const xNextPage = response.headers.get("X-Next-Page"); + const linkNext = parseNextLink(response.headers.get("Link")); + + if (xNextPage && xNextPage.trim().length > 0) { + nextUrl = this.appendQuery(firstUrl, "page", xNextPage.trim()); + } else if (linkNext) { + nextUrl = linkNext; + } else { + nextUrl = null; + } + } + return out; + } + private projectPath(projectId: string | number): string { return `/projects/${encodeURIComponent(String(projectId))}`; } @@ -165,8 +215,17 @@ export class GitlabClient { } listNotes(projectId: string | number, mrIid: number): Promise { - return this.request( - `${this.projectPath(projectId)}/merge_requests/${mrIid}/notes?per_page=100`, + return this.paginate( + `${this.projectPath(projectId)}/merge_requests/${mrIid}/notes`, + ); + } + + listDiscussions( + projectId: string | number, + mrIid: number, + ): Promise { + return this.paginate( + `${this.projectPath(projectId)}/merge_requests/${mrIid}/discussions`, ); } diff --git a/test/gitlab/api-client.test.ts b/test/gitlab/api-client.test.ts index 4b891c4..5b45be1 100644 --- a/test/gitlab/api-client.test.ts +++ b/test/gitlab/api-client.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it, beforeEach, afterEach, mock } from "bun:test"; -import { GitlabClient, GitlabApiError } from "../../src/gitlab/api/client"; +import { + GitlabClient, + GitlabApiError, + parseNextLink, +} from "../../src/gitlab/api/client"; const ORIGINAL_FETCH = globalThis.fetch; @@ -210,4 +214,72 @@ describe("GitlabClient", () => { expect((mr as { iid: number }).iid).toBe(7); expect(fetchMock).toHaveBeenCalledTimes(2); }); + + it("listNotes paginates via X-Next-Page header", async () => { + let calls = 0; + const fetchMock = mockFetch((url) => { + calls++; + const u = new URL(url); + expect(u.searchParams.get("per_page")).toBe("100"); + if (calls === 1) { + expect(u.searchParams.get("page")).toBeNull(); + return new Response(JSON.stringify([{ id: 1 }, { id: 2 }]), { + status: 200, + headers: { + "Content-Type": "application/json", + "X-Next-Page": "2", + }, + }); + } + if (calls === 2) { + expect(u.searchParams.get("page")).toBe("2"); + return new Response(JSON.stringify([{ id: 3 }]), { + status: 200, + headers: { + "Content-Type": "application/json", + "X-Next-Page": "", + }, + }); + } + throw new Error("unexpected extra call"); + }); + const client = new GitlabClient("glpat-test"); + const notes = await client.listNotes(42, 7); + expect(notes.map((n) => (n as { id: number }).id)).toEqual([1, 2, 3]); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it("listDiscussions paginates via Link rel=next header when X-Next-Page is absent", async () => { + let calls = 0; + mockFetch(() => { + calls++; + if (calls === 1) { + return new Response(JSON.stringify([{ id: "a" }]), { + status: 200, + headers: { + "Content-Type": "application/json", + Link: '; rel="next"', + }, + }); + } + return new Response(JSON.stringify([{ id: "b" }]), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }); + const client = new GitlabClient("glpat-test"); + const disc = await client.listDiscussions(42, 7); + expect(disc.map((d) => (d as { id: string }).id)).toEqual(["a", "b"]); + }); + + it("parseNextLink extracts the next URL from a Link header", () => { + expect(parseNextLink(null)).toBeNull(); + expect(parseNextLink("")).toBeNull(); + expect( + parseNextLink( + '; rel="next", ; rel="last"', + ), + ).toBe("https://x/api/page?page=2"); + expect(parseNextLink('; rel="last"')).toBeNull(); + }); }); From 844768aa3460ce544f47b46d3658dab2a4e8f92e Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 17:22:45 -0700 Subject: [PATCH 15/29] Revert "feat(gitlab): paginate listNotes + add listDiscussions (C2)" This reverts commit 7c721d1f2c7d174f688f9795981ef3d0d19f3266. --- src/gitlab/api/client.ts | 81 +++++----------------------------- test/gitlab/api-client.test.ts | 74 +------------------------------ 2 files changed, 12 insertions(+), 143 deletions(-) diff --git a/src/gitlab/api/client.ts b/src/gitlab/api/client.ts index debd4b6..dce14a4 100644 --- a/src/gitlab/api/client.ts +++ b/src/gitlab/api/client.ts @@ -50,15 +50,6 @@ function parseRetryAfter(header: string | null): number | null { return null; } -export function parseNextLink(linkHeader: string | null): string | null { - if (!linkHeader) return null; - for (const part of linkHeader.split(",")) { - const m = part.match(/<([^>]+)>\s*;\s*rel="next"/); - if (m) return m[1] ?? null; - } - return null; -} - function computeBackoffMs( attempt: number, retryAfterMs: number | null, @@ -87,10 +78,10 @@ export class GitlabClient { this.retry = { ...DEFAULT_RETRY, ...retry }; } - private async requestRaw( + private async request( path: string, init: RequestInitWithJson = {}, - ): Promise { + ): Promise { const { json, headers: rawHeaders, ...rest } = init; const headers: Record = { "PRIVATE-TOKEN": this.token, @@ -107,6 +98,7 @@ export class GitlabClient { const url = path.startsWith("http") ? path : `${this.baseUrl}${path}`; let attempt = 0; + // attempt 0 = initial request, then up to maxRetries follow-up tries. while (true) { let response: Response; try { @@ -118,7 +110,12 @@ export class GitlabClient { continue; } - if (response.ok) return response; + if (response.ok) { + if (response.status === 204) { + return undefined as unknown as T; + } + return (await response.json()) as T; + } const shouldRetry = RETRYABLE_STATUS.has(response.status) && @@ -148,53 +145,6 @@ export class GitlabClient { } } - private async request( - path: string, - init: RequestInitWithJson = {}, - ): Promise { - const response = await this.requestRaw(path, init); - if (response.status === 204) { - return undefined as unknown as T; - } - return (await response.json()) as T; - } - - private appendQuery(url: string, key: string, value: string): string { - const u = new URL(url); - u.searchParams.set(key, value); - return u.toString(); - } - - private async paginate( - path: string, - init: RequestInitWithJson = {}, - ): Promise { - const absolute = path.startsWith("http") ? path : `${this.baseUrl}${path}`; - const firstUrl = this.appendQuery(absolute, "per_page", "100"); - - const out: T[] = []; - let nextUrl: string | null = firstUrl; - - while (nextUrl !== null) { - const response = await this.requestRaw(nextUrl, init); - const chunk = (await response.json()) as T[]; - if (Array.isArray(chunk)) out.push(...chunk); - - // Prefer X-Next-Page (older GitLab) then Link header (rel=next). - const xNextPage = response.headers.get("X-Next-Page"); - const linkNext = parseNextLink(response.headers.get("Link")); - - if (xNextPage && xNextPage.trim().length > 0) { - nextUrl = this.appendQuery(firstUrl, "page", xNextPage.trim()); - } else if (linkNext) { - nextUrl = linkNext; - } else { - nextUrl = null; - } - } - return out; - } - private projectPath(projectId: string | number): string { return `/projects/${encodeURIComponent(String(projectId))}`; } @@ -215,17 +165,8 @@ export class GitlabClient { } listNotes(projectId: string | number, mrIid: number): Promise { - return this.paginate( - `${this.projectPath(projectId)}/merge_requests/${mrIid}/notes`, - ); - } - - listDiscussions( - projectId: string | number, - mrIid: number, - ): Promise { - return this.paginate( - `${this.projectPath(projectId)}/merge_requests/${mrIid}/discussions`, + return this.request( + `${this.projectPath(projectId)}/merge_requests/${mrIid}/notes?per_page=100`, ); } diff --git a/test/gitlab/api-client.test.ts b/test/gitlab/api-client.test.ts index 5b45be1..4b891c4 100644 --- a/test/gitlab/api-client.test.ts +++ b/test/gitlab/api-client.test.ts @@ -1,9 +1,5 @@ import { describe, expect, it, beforeEach, afterEach, mock } from "bun:test"; -import { - GitlabClient, - GitlabApiError, - parseNextLink, -} from "../../src/gitlab/api/client"; +import { GitlabClient, GitlabApiError } from "../../src/gitlab/api/client"; const ORIGINAL_FETCH = globalThis.fetch; @@ -214,72 +210,4 @@ describe("GitlabClient", () => { expect((mr as { iid: number }).iid).toBe(7); expect(fetchMock).toHaveBeenCalledTimes(2); }); - - it("listNotes paginates via X-Next-Page header", async () => { - let calls = 0; - const fetchMock = mockFetch((url) => { - calls++; - const u = new URL(url); - expect(u.searchParams.get("per_page")).toBe("100"); - if (calls === 1) { - expect(u.searchParams.get("page")).toBeNull(); - return new Response(JSON.stringify([{ id: 1 }, { id: 2 }]), { - status: 200, - headers: { - "Content-Type": "application/json", - "X-Next-Page": "2", - }, - }); - } - if (calls === 2) { - expect(u.searchParams.get("page")).toBe("2"); - return new Response(JSON.stringify([{ id: 3 }]), { - status: 200, - headers: { - "Content-Type": "application/json", - "X-Next-Page": "", - }, - }); - } - throw new Error("unexpected extra call"); - }); - const client = new GitlabClient("glpat-test"); - const notes = await client.listNotes(42, 7); - expect(notes.map((n) => (n as { id: number }).id)).toEqual([1, 2, 3]); - expect(fetchMock).toHaveBeenCalledTimes(2); - }); - - it("listDiscussions paginates via Link rel=next header when X-Next-Page is absent", async () => { - let calls = 0; - mockFetch(() => { - calls++; - if (calls === 1) { - return new Response(JSON.stringify([{ id: "a" }]), { - status: 200, - headers: { - "Content-Type": "application/json", - Link: '; rel="next"', - }, - }); - } - return new Response(JSON.stringify([{ id: "b" }]), { - status: 200, - headers: { "Content-Type": "application/json" }, - }); - }); - const client = new GitlabClient("glpat-test"); - const disc = await client.listDiscussions(42, 7); - expect(disc.map((d) => (d as { id: string }).id)).toEqual(["a", "b"]); - }); - - it("parseNextLink extracts the next URL from a Link header", () => { - expect(parseNextLink(null)).toBeNull(); - expect(parseNextLink("")).toBeNull(); - expect( - parseNextLink( - '; rel="next", ; rel="last"', - ), - ).toBe("https://x/api/page?page=2"); - expect(parseNextLink('; rel="last"')).toBeNull(); - }); }); From 18ab39148c8d602c377256945b4ecbe0a8c6c005 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 17:22:45 -0700 Subject: [PATCH 16/29] Revert "feat(gitlab): rate-limit + exponential backoff in GitlabClient (C1)" This reverts commit d70dbd70197c8810c6e55204d3ac0f6dc20686cb. --- src/gitlab/api/client.ts | 95 +++++------------------------- test/gitlab/api-client.test.ts | 104 --------------------------------- 2 files changed, 13 insertions(+), 186 deletions(-) diff --git a/src/gitlab/api/client.ts b/src/gitlab/api/client.ts index dce14a4..0747d4e 100644 --- a/src/gitlab/api/client.ts +++ b/src/gitlab/api/client.ts @@ -21,61 +21,13 @@ export class GitlabApiError extends Error { type RequestInitWithJson = Omit & { json?: unknown }; -export type RetryOptions = { - maxRetries: number; - baseDelayMs: number; - maxDelayMs: number; -}; - -const DEFAULT_RETRY: RetryOptions = { - maxRetries: 5, - baseDelayMs: 500, - maxDelayMs: 30_000, -}; - -const RETRYABLE_STATUS = new Set([408, 429, 500, 502, 503, 504]); - -function sleep(ms: number): Promise { - return new Promise((resolve) => setTimeout(resolve, ms)); -} - -function parseRetryAfter(header: string | null): number | null { - if (!header) return null; - const asInt = Number.parseInt(header, 10); - if (Number.isFinite(asInt) && asInt >= 0) return asInt * 1000; - const asDate = Date.parse(header); - if (Number.isFinite(asDate)) { - return Math.max(0, asDate - Date.now()); - } - return null; -} - -function computeBackoffMs( - attempt: number, - retryAfterMs: number | null, - opts: RetryOptions, -): number { - if (retryAfterMs !== null) { - return Math.min(opts.maxDelayMs, retryAfterMs); - } - const exp = opts.baseDelayMs * Math.pow(2, attempt); - const jitter = Math.random() * opts.baseDelayMs; - return Math.min(opts.maxDelayMs, exp + jitter); -} - export class GitlabClient { private readonly baseUrl: string; private readonly token: string; - private readonly retry: RetryOptions; - constructor( - token: string, - baseUrl: string = GITLAB_API_URL, - retry: Partial = {}, - ) { + constructor(token: string, baseUrl: string = GITLAB_API_URL) { this.token = token; this.baseUrl = baseUrl.replace(/\/+$/, ""); - this.retry = { ...DEFAULT_RETRY, ...retry }; } private async request( @@ -96,40 +48,13 @@ export class GitlabClient { } const url = path.startsWith("http") ? path : `${this.baseUrl}${path}`; + const response = await fetch(url, { + ...rest, + headers, + body, + }); - let attempt = 0; - // attempt 0 = initial request, then up to maxRetries follow-up tries. - while (true) { - let response: Response; - try { - response = await fetch(url, { ...rest, headers, body }); - } catch (err) { - if (attempt >= this.retry.maxRetries) throw err; - await sleep(computeBackoffMs(attempt, null, this.retry)); - attempt++; - continue; - } - - if (response.ok) { - if (response.status === 204) { - return undefined as unknown as T; - } - return (await response.json()) as T; - } - - const shouldRetry = - RETRYABLE_STATUS.has(response.status) && - attempt < this.retry.maxRetries; - if (shouldRetry) { - const retryAfterMs = parseRetryAfter( - response.headers.get("Retry-After"), - ); - const delay = computeBackoffMs(attempt, retryAfterMs, this.retry); - await sleep(delay); - attempt++; - continue; - } - + if (!response.ok) { let parsed: unknown = null; const text = await response.text(); try { @@ -143,6 +68,12 @@ export class GitlabClient { parsed, ); } + + if (response.status === 204) { + return undefined as unknown as T; + } + + return (await response.json()) as T; } private projectPath(projectId: string | number): string { diff --git a/test/gitlab/api-client.test.ts b/test/gitlab/api-client.test.ts index 4b891c4..927084f 100644 --- a/test/gitlab/api-client.test.ts +++ b/test/gitlab/api-client.test.ts @@ -106,108 +106,4 @@ describe("GitlabClient", () => { expect((e.body as { message: string }).message).toBe("404 Not Found"); } }); - - it("does not retry on non-retryable 4xx (404, 401, 403)", async () => { - const fetchMock = mockFetch( - () => - new Response("nope", { - status: 404, - statusText: "Not Found", - }), - ); - const client = new GitlabClient("glpat-test", undefined, { - maxRetries: 5, - baseDelayMs: 1, - maxDelayMs: 10, - }); - try { - await client.getMr(42, 999); - throw new Error("expected throw"); - } catch (err) { - expect(err).toBeInstanceOf(GitlabApiError); - } - expect(fetchMock).toHaveBeenCalledTimes(1); - }); - - it("retries on 429 with exponential backoff and eventually succeeds", async () => { - let calls = 0; - const fetchMock = mockFetch(() => { - calls++; - if (calls < 3) { - return new Response("rate limited", { - status: 429, - statusText: "Too Many Requests", - }); - } - return jsonResponse({ iid: 7 }); - }); - const client = new GitlabClient("glpat-test", undefined, { - maxRetries: 5, - baseDelayMs: 1, - maxDelayMs: 10, - }); - const mr = await client.getMr(42, 7); - expect((mr as { iid: number }).iid).toBe(7); - expect(fetchMock).toHaveBeenCalledTimes(3); - }); - - it("retries on 500/502/503/504 and surfaces the final error after maxRetries", async () => { - const fetchMock = mockFetch( - () => new Response("upstream down", { status: 503 }), - ); - const client = new GitlabClient("glpat-test", undefined, { - maxRetries: 2, - baseDelayMs: 1, - maxDelayMs: 5, - }); - try { - await client.getMr(42, 7); - throw new Error("expected throw"); - } catch (err) { - expect(err).toBeInstanceOf(GitlabApiError); - expect((err as GitlabApiError).status).toBe(503); - } - // 1 initial + 2 retries = 3 attempts - expect(fetchMock).toHaveBeenCalledTimes(3); - }); - - it("honors Retry-After header (seconds) on 429", async () => { - let calls = 0; - const times: number[] = []; - const fetchMock = mockFetch(() => { - times.push(Date.now()); - calls++; - if (calls < 2) { - return new Response("rate limited", { - status: 429, - headers: { "Retry-After": "0" }, - }); - } - return jsonResponse({ iid: 7 }); - }); - const client = new GitlabClient("glpat-test", undefined, { - maxRetries: 3, - baseDelayMs: 1, - maxDelayMs: 10, - }); - await client.getMr(42, 7); - expect(fetchMock).toHaveBeenCalledTimes(2); - }); - - it("retries on fetch network errors", async () => { - let calls = 0; - const fetchMock = mockFetch(() => { - calls++; - if (calls < 2) throw new Error("ECONNRESET"); - return jsonResponse({ iid: 7 }); - }); - const client = new GitlabClient("glpat-test", undefined, { - maxRetries: 3, - baseDelayMs: 1, - maxDelayMs: 5, - }); - const mr = await client.getMr(42, 7); - expect((mr as { iid: number }).iid).toBe(7); - expect(fetchMock).toHaveBeenCalledTimes(2); - }); }); From 46be9f7891cddabf306ac499f7bcbb7d4f9cae72 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 17:22:45 -0700 Subject: [PATCH 17/29] Revert "feat(gitlab): cache bun install cache across pipelines (C8)" This reverts commit 0d6c73b0feae6598590844a3f8739df4aced1f58. --- gitlab/templates/review.yml | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index 6369368..415cca5 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -65,19 +65,6 @@ droid-review: INCLUDE_SUGGESTIONS: "$[[ inputs.include_suggestions ]]" DROID_SETTINGS: "$[[ inputs.settings ]]" DROID_SUCCESS: "true" - # Pin bun's install cache to a stable path inside CI_PROJECT_DIR so - # GitLab's `cache:` rules can persist it across pipelines. Mirrors - # actions/cache usage in the GitHub action. - BUN_INSTALL_CACHE_DIR: "$CI_PROJECT_DIR/.bun-cache" - # Cache the cloned droid-action checkout and bun's package cache to - # avoid re-cloning + re-downloading dependencies on every MR pipeline. - # The `key:` is pinned to the droid_action_ref so a ref change cleanly - # invalidates without manual purging. - cache: - key: "droid-action-$[[ inputs.droid_action_ref ]]" - paths: - - .bun-cache/ - policy: pull-push before_script: - apt-get update -qq - apt-get install -y -qq --no-install-recommends git ca-certificates curl From c71725e94a3e078f0d18eb46e3390c6ecfbe680b Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 17:23:24 -0700 Subject: [PATCH 18/29] revert(gitlab): drop draft-skip rules + stderr tail-60 capture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes two GitLab-only behaviours that don't exist in the GitHub action: - Draft/WIP `rules: when: never` block. GitHub leaves draft handling to the user's workflow `if: ... draft == false` condition and doesn't bake it into the action. Customers who want to skip drafts can add the same condition to their own .gitlab-ci.yml. - Stderr tail-60 capture into droid-error.txt. GitHub uses core.setFailed(error.message) which surfaces in the Actions run summary, not in the PR comment body — the comment just shows an actionFailed flag. Revert to the plain "droid exec pass N failed" message for parity. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- gitlab/templates/review.yml | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index 415cca5..e37325a 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -45,12 +45,6 @@ droid-review: stage: $[[ inputs.stage ]] image: oven/bun:1.2.11 rules: - # Skip draft / WIP MRs (parity with GitHub's `if: ... draft == false` - # pattern). The legacy "WIP:" prefix is also covered. - - if: '$CI_PIPELINE_SOURCE == "merge_request_event" && $CI_MERGE_REQUEST_TITLE =~ /^Draft:/' - when: never - - if: '$CI_PIPELINE_SOURCE == "merge_request_event" && $CI_MERGE_REQUEST_TITLE =~ /^WIP:/' - when: never - if: '$CI_PIPELINE_SOURCE == "merge_request_event"' variables: DROID_STATE_FILE: "$CI_PROJECT_DIR/.droid-state.json" @@ -126,12 +120,7 @@ droid-review: echo "Running Pass 1 (candidates): droid $DROID_ARGS" if ! droid $DROID_ARGS 2>&1 | tee "$PASS1_LOG"; then export DROID_SUCCESS=false - { - echo "droid exec pass 1 (candidates) failed" - echo "" - echo "--- last 60 lines of pass 1 output ---" - tail -n 60 "$PASS1_LOG" 2>/dev/null || true - } > /tmp/droid-error.txt + echo "droid exec pass 1 (candidates) failed" > /tmp/droid-error.txt exit 1 fi - | @@ -155,12 +144,7 @@ droid-review: echo "Running Pass 2 (validator): droid $DROID_ARGS" if ! droid $DROID_ARGS 2>&1 | tee "$PASS2_LOG"; then export DROID_SUCCESS=false - { - echo "droid exec pass 2 (validator) failed" - echo "" - echo "--- last 60 lines of pass 2 output ---" - tail -n 60 "$PASS2_LOG" 2>/dev/null || true - } > /tmp/droid-error.txt + echo "droid exec pass 2 (validator) failed" > /tmp/droid-error.txt exit 1 fi after_script: From 08f5c4c502e877d26d87d1962ee2de572e97030e Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 17:51:56 -0700 Subject: [PATCH 19/29] feat(gitlab): expose automatic_security_review input (A1, per-MR scope) Wires the automatic_security_review toggle through the GitLab CI/CD Component. When set to "true", the security-reviewer subagent already bundled in .factory/droids/security-reviewer.md is spawned in parallel with the code-review subagents during Pass 1 (the conditional block in src/gitlab/prompts/candidates.ts already handles this via the securityReviewEnabled flag). Security findings are prefixed with [security] and flow through the same Pass-2 validator path as code-review findings, ultimately posted via a single batched submit_review call. No plugin install is required: the security-review skill that the subagent invokes as its first action is built into the Droid CLI binary. Mirrors the GitHub action's automatic_security_review input for the per-MR flow only; the scheduled full-repo scan (which additionally uses threat-model-generation / commit-security-scan / vulnerability-validation skills from the security-engineer plugin) is a separate feature. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- gitlab/templates/review.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index e37325a..5f9056d 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -18,6 +18,15 @@ spec: high-confidence. Set to "false" to disable suggestion blocks entirely. Mirrors the GitHub action's `include_suggestions` input. default: "true" + automatic_security_review: + description: | + Automatically run a security-focused review subagent in parallel + with the regular code review on every MR pipeline. Findings are + prefixed with `[security]` and posted alongside code-review + comments. Mirrors the GitHub action's `automatic_security_review` + input. The security-review skill is built into the Droid CLI; no + plugin install is required. + default: "false" settings: description: | Droid Exec settings as a JSON string or a path to a JSON file. @@ -57,6 +66,7 @@ droid-review: REVIEW_MODEL: "$[[ inputs.review_model ]]" REASONING_EFFORT: "$[[ inputs.reasoning_effort ]]" INCLUDE_SUGGESTIONS: "$[[ inputs.include_suggestions ]]" + AUTOMATIC_SECURITY_REVIEW: "$[[ inputs.automatic_security_review ]]" DROID_SETTINGS: "$[[ inputs.settings ]]" DROID_SUCCESS: "true" before_script: From 1c6a2a51dad0838c0edf33f0baf8ec1d44260ee8 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Mon, 1 Jun 2026 19:51:32 -0700 Subject: [PATCH 20/29] feat(gitlab): expose security_block_on_critical/high inputs (A10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Matches GitHub action's input surface for security_block_on_critical (default "true") and security_block_on_high (default "false"). Like the GitHub action, the inputs are parsed into context but not currently consumed by production code — they exist for surface-level parity and forward compatibility when blocking logic lands. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- gitlab/templates/review.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index 5f9056d..3ffaa1a 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -27,6 +27,18 @@ spec: input. The security-review skill is built into the Droid CLI; no plugin install is required. default: "false" + security_block_on_critical: + description: | + When the security reviewer reports CRITICAL findings, block merge + of the MR. Mirrors the GitHub action's `security_block_on_critical` + input. Defaults to `"true"`. + default: "true" + security_block_on_high: + description: | + When the security reviewer reports HIGH findings, block merge of + the MR. Mirrors the GitHub action's `security_block_on_high` input. + Defaults to `"false"`. + default: "false" settings: description: | Droid Exec settings as a JSON string or a path to a JSON file. @@ -67,6 +79,8 @@ droid-review: REASONING_EFFORT: "$[[ inputs.reasoning_effort ]]" INCLUDE_SUGGESTIONS: "$[[ inputs.include_suggestions ]]" AUTOMATIC_SECURITY_REVIEW: "$[[ inputs.automatic_security_review ]]" + SECURITY_BLOCK_ON_CRITICAL: "$[[ inputs.security_block_on_critical ]]" + SECURITY_BLOCK_ON_HIGH: "$[[ inputs.security_block_on_high ]]" DROID_SETTINGS: "$[[ inputs.settings ]]" DROID_SUCCESS: "true" before_script: From 6f05311680dda376bcbcc2a66e5d04d96e40aa8f Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Tue, 2 Jun 2026 10:18:30 -0700 Subject: [PATCH 21/29] docs(gitlab): add gitlab-setup.md and README section for GitLab CI/CD Component Documents the manual installation flow for the new GitLab CI/CD Component (`gitlab/templates/review.yml`): prerequisites, full inputs table, what the pipeline produces, how the two-pass review works, what is intentionally not yet supported (comment triggers, fill mode, scheduled scans), and troubleshooting steps. README now has a short GitLab section that points to the full guide. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- README.md | 22 +++++++ docs/gitlab-setup.md | 149 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 171 insertions(+) create mode 100644 docs/gitlab-setup.md diff --git a/README.md b/README.md index eadf5e9..ac728e0 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,28 @@ The guided flow will: For GitHub-only setups you can also run `/install-github-app`. See the [Automated Code Review guide](https://docs.factory.ai/guides/droid-exec/code-review) and the [GitHub App installation guide](https://docs.factory.ai/cli/features/install-github-app) for full details. +### GitLab + +GitLab support ships as a **GitLab CI/CD Component** under `gitlab/templates/review.yml`. It delivers the same automatic two-pass code review and parallel security-reviewer subagent, with inline MR comments, a sticky tracking note, and `.droid-debug/` artifacts. + +Minimal `.gitlab-ci.yml`: + +```yaml +include: + - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/review.yml" + inputs: + automatic_review: "true" + automatic_security_review: "false" + review_depth: "deep" + +droid-review: + variables: + FACTORY_API_KEY: $FACTORY_API_KEY + GITLAB_TOKEN: $GITLAB_TOKEN +``` + +Full setup, available inputs, and troubleshooting live in [`docs/gitlab-setup.md`](docs/gitlab-setup.md). Comment-triggered modes (`@droid review` / `@droid fill`) are not yet supported on GitLab — they require platform features GitLab does not expose natively. + ### Manual Setup If you prefer to wire things up by hand: diff --git a/docs/gitlab-setup.md b/docs/gitlab-setup.md new file mode 100644 index 0000000..492d4d4 --- /dev/null +++ b/docs/gitlab-setup.md @@ -0,0 +1,149 @@ +# GitLab Setup + +This action ships a **GitLab CI/CD Component** that delivers the same +automated code-review experience as the GitHub action on GitLab merge +requests (MRs). The component runs on every `merge_request_event` pipeline, +posts inline comments on the diff, maintains a sticky tracking note, and +optionally runs a security-focused subagent in parallel. + +## Quick start with `/install-code-review` + +The fastest path is the guided installer built into the Droid CLI: + +```bash +droid +> /install-code-review +``` + +It detects GitLab, optionally provisions a branded `Factory Droid` service +account, asks the configuration questions below, generates the +`.gitlab-ci.yml`, and opens an MR / direct-commits to the target project(s). + +## Manual installation + +### 1. Prerequisites + +| Requirement | How to get it | +| ------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| GitLab Maintainer role on the project | Repo admin grants you Maintainer (40) | +| `FACTORY_API_KEY` CI/CD variable | Generate at ; add as **masked**, **unprotected** variable at the project, subgroup, or top-level group level | +| `GITLAB_TOKEN` CI/CD variable | A personal or service-account access token with the `api` scope. Required so the MCP server can post comments back to the MR. Add as **masked**, **unprotected**. | + +### 2. Add the CI/CD Component + +Create or extend `.gitlab-ci.yml` in your project with: + +```yaml +include: + - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/review.yml" + inputs: + automatic_review: "true" + automatic_security_review: "false" + review_depth: "deep" + include_suggestions: "true" + security_block_on_critical: "true" + security_block_on_high: "false" + droid_action_ref: "main" + +droid-review: + variables: + FACTORY_API_KEY: $FACTORY_API_KEY + GITLAB_TOKEN: $GITLAB_TOKEN +``` + +> Pin both the `include:` URL ref and `droid_action_ref` to a release tag +> (e.g. `v1`) once one is published. They MUST match — the URL pin +> resolves the template at parse time, `droid_action_ref` clones the same +> source at runtime. + +### 3. Push an MR + +Open or push to an MR. The next `merge_request_event` pipeline will run +the `droid-review` job. Expect ~5-10 minutes for a typical change. + +## Inputs + +| Input | Default | Description | +| ---------------------------- | ------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------- | +| `automatic_review` | `"true"` | Run code review automatically on every MR pipeline. | +| `automatic_security_review` | `"false"` | Run a parallel security-focused subagent on every MR pipeline. Findings are prefixed `[security]` and posted alongside code-review comments. | +| `review_depth` | `"deep"` | `"deep"` (thorough) or `"shallow"` (fast). | +| `review_model` | `""` | Override the model. Empty = use depth preset. | +| `reasoning_effort` | `""` | Override reasoning effort. Empty = use depth preset. | +| `include_suggestions` | `"true"` | Include code suggestion blocks in review comments when the fix is high-confidence. | +| `security_block_on_critical` | `"true"` | Block merge on CRITICAL security findings. (Mirrors GitHub action; surface-level parity.) | +| `security_block_on_high` | `"false"` | Block merge on HIGH security findings. (Mirrors GitHub action; surface-level parity.) | +| `settings` | `""` | Droid Exec settings as a JSON string or a path to a JSON file. Merged into `~/.factory/droid/settings.json` before each `droid exec` call. | +| `droid_action_repo` | `https://github.com/Factory-AI/droid-action.git` | Override if you mirror droid-action privately. | +| `droid_action_ref` | `"dev"` | Git ref of droid-action to clone at runtime. Should match the `include: remote:` URL ref. | +| `stage` | `"test"` | GitLab CI stage to assign the `droid-review` job to. | + +## What you get + +Each MR pipeline produces: + +- **Inline review comments** anchored to the relevant diff lines, posted in a + single batched `submit_review` call. Findings are prefixed with priority + tags (`P0`, `P1`, `P2`, `P3`) and `[security]` for security findings. +- **A sticky tracking note** on the MR with pipeline + job links, telemetry + (`N turns • Xm Ys`), session IDs, and a security badge when + `automatic_security_review` is enabled. +- **Debug artifacts** at `.droid-debug/` (prompts, candidate JSON, raw + stream-json logs) retained for 1 week. +- **A custom droid library** copied from + `$DROID_ACTION_DIR/.factory/droids` into `~/.factory/droids` on the + runner, so subagents like `security-reviewer` are reachable. + +## How it works + +The pipeline runs in two passes: + +1. **Pass 1 (candidates)** — Droid Exec reads the MR diff, description, and + existing comments, then writes `/tmp/droid-prompts/review_candidates.json` + with candidate findings. The security-reviewer subagent runs in parallel + if `automatic_security_review` is on. **No MR mutations** happen in this + pass — the MR-write tools are excluded from `--enabled-tools`. +2. **Pass 2 (validator)** — Droid Exec rereads the candidates, filters out + duplicates and low-confidence findings, and batches all approved comments + into a single `gitlab_mr___submit_review` call. The sticky note is + updated via `gitlab_mr___update_tracking_note`. + +A small GitLab MCP server (`src/mcp/gitlab-mr-server.ts`) is launched on +the runner to expose those two write tools. + +## What's not yet supported + +These exist in the GitHub action but require platform features GitLab +does not currently expose, and are not part of Phase 1: + +- **Comment-triggered modes** (`@droid review`, `@droid security`, + `@droid fill`). GitLab does not fire CI pipelines on note events. + Requires a Factory-hosted webhook receiver that turns note webhooks + into `POST /projects/:id/trigger/pipeline` calls. Planned as a follow-up. +- **`@droid fill` (MR description fill).** Depends on the comment trigger + above. +- **Scheduled full-repo security scans.** Tracked separately. + +## Troubleshooting + +| Symptom | Likely cause | Fix | +| --------------------------------------------- | ------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- | +| Pipeline runs but no comments posted | `GITLAB_TOKEN` not set or missing `api` scope | Add a masked `GITLAB_TOKEN` CI/CD variable with `api`-scoped PAT | +| `Could not find file` on include | `droid_action_ref` doesn't exist in `Factory-AI/droid-action` | Verify the ref; pin to a stable tag | +| `factory_api_key is required` error | `FACTORY_API_KEY` not set or scoped wrong | Add as **masked**, **unprotected** variable at the right level (project / subgroup / top-level group) | +| Sticky note posts but inline findings missing | Pass 2 hit a transient API error | Inspect `.droid-debug/prompts/pass2-output.jsonl`; re-run the pipeline | +| Pipeline times out | Very large MR | Set `review_depth: "shallow"` for faster feedback | + +## Self-hosted GitLab + +The component reads `CI_API_V4_URL` and `CI_SERVER_URL` from the standard +GitLab CI environment. Self-managed GitLab works as long as your runner +can: + +- Reach `https://app.factory.ai/cli` to install the Droid CLI +- Reach `https://github.com/Factory-AI/droid-action.git` (or your private + mirror, via `droid_action_repo`) +- Reach `https://raw.githubusercontent.com/...` for the `include: remote:` + +Mirror droid-action internally and set `droid_action_repo` + `droid_action_ref` +accordingly if your runners cannot reach GitHub. From ecd3fbe28f0f44f8bd4e6c06b5ab13825337d290 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Tue, 2 Jun 2026 13:23:04 -0700 Subject: [PATCH 22/29] docs(gitlab): add drop-in .gitlab-ci.yml samples under gitlab/examples/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two consumer-facing sample files plus a README so users have a canonical reference for what a Component-consuming .gitlab-ci.yml looks like: * gitlab/examples/.gitlab-ci.minimal.yml — shortest possible, every default accepted, only the two required CI/CD variables wired. * gitlab/examples/.gitlab-ci.example.yml — annotated, every input spelled out with safe-default guidance, plus an optional @droid fill block (commented out) showing how to add fill. * gitlab/examples/README.md — table mapping each file to its use case and the variable-setup story. Also updates docs/gitlab-setup.md: * Points readers at gitlab/examples/ instead of just inlining the snippet, so customers can clone the project, copy the file, done. * Drops the stale 'service account provisioning' reference now that the install-code-review skill no longer provisions SAs. * Tightens the GITLAB_TOKEN row to make the poster-identity story explicit: the token owner IS the poster, no API impersonation. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- docs/gitlab-setup.md | 24 +++++---- gitlab/examples/.gitlab-ci.example.yml | 70 ++++++++++++++++++++++++++ gitlab/examples/.gitlab-ci.minimal.yml | 12 +++++ gitlab/examples/README.md | 24 +++++++++ 4 files changed, 121 insertions(+), 9 deletions(-) create mode 100644 gitlab/examples/.gitlab-ci.example.yml create mode 100644 gitlab/examples/.gitlab-ci.minimal.yml create mode 100644 gitlab/examples/README.md diff --git a/docs/gitlab-setup.md b/docs/gitlab-setup.md index 492d4d4..518acb5 100644 --- a/docs/gitlab-setup.md +++ b/docs/gitlab-setup.md @@ -15,23 +15,29 @@ droid > /install-code-review ``` -It detects GitLab, optionally provisions a branded `Factory Droid` service -account, asks the configuration questions below, generates the -`.gitlab-ci.yml`, and opens an MR / direct-commits to the target project(s). +It detects GitLab, asks which account should be the poster of review +comments (you supply its PAT as `GITLAB_TOKEN`), asks the configuration +questions below, generates the `.gitlab-ci.yml`, and opens an MR / +direct-commits to the target project(s). ## Manual installation ### 1. Prerequisites -| Requirement | How to get it | -| ------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| GitLab Maintainer role on the project | Repo admin grants you Maintainer (40) | -| `FACTORY_API_KEY` CI/CD variable | Generate at ; add as **masked**, **unprotected** variable at the project, subgroup, or top-level group level | -| `GITLAB_TOKEN` CI/CD variable | A personal or service-account access token with the `api` scope. Required so the MCP server can post comments back to the MR. Add as **masked**, **unprotected**. | +| Requirement | How to get it | +| ------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| GitLab Maintainer role on the project | Repo admin grants you Maintainer (40) | +| `FACTORY_API_KEY` CI/CD variable | Generate at ; add as **masked**, **unprotected** variable at the project, subgroup, or top-level group level | +| `GITLAB_TOKEN` CI/CD variable | A personal access token with the `api` scope, owned by whichever account should post review comments. The token owner is the poster — there is no API impersonation. Add as **masked**, **unprotected**. | ### 2. Add the CI/CD Component -Create or extend `.gitlab-ci.yml` in your project with: +Drop-in samples live in [`gitlab/examples/`](../gitlab/examples/): + +- [`.gitlab-ci.minimal.yml`](../gitlab/examples/.gitlab-ci.minimal.yml) — shortest possible, accepts every default. +- [`.gitlab-ci.example.yml`](../gitlab/examples/.gitlab-ci.example.yml) — annotated with every input. + +The minimal form looks like this; create or extend `.gitlab-ci.yml` in your project with: ```yaml include: diff --git a/gitlab/examples/.gitlab-ci.example.yml b/gitlab/examples/.gitlab-ci.example.yml new file mode 100644 index 0000000..e873075 --- /dev/null +++ b/gitlab/examples/.gitlab-ci.example.yml @@ -0,0 +1,70 @@ +# Example .gitlab-ci.yml showcasing the droid-action GitLab CI/CD Component. +# +# Drop this file (or its snippets) at the root of your GitLab project. +# It defines a single `droid-review` job that runs Factory Droid's +# automated code review on every merge_request_event. +# +# Prerequisites (one-time, configured via the GitLab UI or `glab`): +# +# * FACTORY_API_KEY — masked CI/CD variable. Get one at +# https://app.factory.ai/settings/api-keys +# +# * GITLAB_TOKEN — masked CI/CD variable. A personal access token +# with `api` scope, minted by the GitLab user/account you want +# review comments to be posted from. The poster identity IS the +# owner of this token; there is no API impersonation. To switch +# posters later, replace the variable's value. +# +# Both variables can be set at the project, subgroup, or top-level +# group level, depending on how widely you want the review to roll out. + +include: + # Pin to a release tag once droid-action publishes one. Until then, + # `main` tracks the latest stable cut. `droid_action_ref` below MUST + # match the ref in this URL so the template and the runtime source + # stay consistent. + - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/review.yml" + inputs: + # ---- core review behavior ---- + automatic_review: "true" # run on every MR event; "false" disables + review_depth: "deep" # "deep" (gpt-5.2 / high) or "shallow" (kimi-k2.6 / none) + include_suggestions: "true" # post code-suggestion blocks for high-confidence fixes + + # ---- security review (parallel security-reviewer subagent) ---- + automatic_security_review: "false" # enable to flag vulns alongside the regular review + security_block_on_critical: "true" # block merge on CRITICAL findings (when security review is on) + security_block_on_high: "false" # block merge on HIGH findings (defaults to false) + + # ---- runtime source pin ---- + droid_action_ref: "main" + +droid-review: + variables: + FACTORY_API_KEY: $FACTORY_API_KEY + GITLAB_TOKEN: $GITLAB_TOKEN +# ---- Optional: add @droid fill mode ------------------------------------- +# +# Uncomment the include below to also auto-fill new MRs' descriptions +# from their diffs. The fill job fires on the same `merge_request_event` +# trigger as the review job; together they form Factory Droid's GA GitLab +# experience. +# +# include: +# - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/fill.yml" +# inputs: +# automatic_fill: "false" # true = fill every new MR automatically +# droid_action_ref: "main" +# +# droid-fill: +# variables: +# FACTORY_API_KEY: $FACTORY_API_KEY +# GITLAB_TOKEN: $GITLAB_TOKEN +# +# With automatic_fill="false", trigger fill manually per MR by either: +# - writing "@droid fill" in the MR description, OR +# - writing "@droid fill" in the MR title, OR +# - adding the "droid:fill" label to the MR. +# +# After Droid fills the description, it strips the @droid fill token so +# the next merge_request_event doesn't re-trigger. +# ----------------------------------------------------------------------- diff --git a/gitlab/examples/.gitlab-ci.minimal.yml b/gitlab/examples/.gitlab-ci.minimal.yml new file mode 100644 index 0000000..97643e4 --- /dev/null +++ b/gitlab/examples/.gitlab-ci.minimal.yml @@ -0,0 +1,12 @@ +# Minimal example: every default, just `include:` the Component. +# +# Requires FACTORY_API_KEY and GITLAB_TOKEN to be set as masked CI/CD +# variables. See .gitlab-ci.example.yml for the annotated version. + +include: + - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/review.yml" + +droid-review: + variables: + FACTORY_API_KEY: $FACTORY_API_KEY + GITLAB_TOKEN: $GITLAB_TOKEN diff --git a/gitlab/examples/README.md b/gitlab/examples/README.md new file mode 100644 index 0000000..30a18db --- /dev/null +++ b/gitlab/examples/README.md @@ -0,0 +1,24 @@ +# GitLab examples + +Drop-in `.gitlab-ci.yml` samples for consuming the droid-action GitLab +CI/CD Component. + +| File | When to use | +| ------------------------ | -------------------------------------------------------------------------------------------------------------- | +| `.gitlab-ci.minimal.yml` | Shortest possible — `include:` the review template, accept every default. Good starting point. | +| `.gitlab-ci.example.yml` | Annotated. Every input spelled out with comments explaining what it does and what the safe defaults look like. | + +Both samples wire the same two required CI/CD variables: + +- `FACTORY_API_KEY` — get one at . +- `GITLAB_TOKEN` — a personal access token with `api` scope, owned by + whichever GitLab user/account should be the poster of review comments. + To change the poster later, replace this variable's value. + +Set both as **masked** CI/CD variables at the level you want the review +to apply (project, subgroup, or top-level group). + +For the full input reference (model overrides, security review, +suggestion blocks, custom stage, etc.) see +[`../templates/review.yml`](../templates/review.yml) or the docs at +[`docs/gitlab-setup.md`](../../docs/gitlab-setup.md). From f1252dfc35c496ea1585ef7de3554714cf0f64d9 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Tue, 2 Jun 2026 14:09:20 -0700 Subject: [PATCH 23/29] docs(gitlab): hide internal inputs, trim setup.md, default ref to main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Customer-facing surface cleanup before GA: * gitlab/templates/review.yml: change droid_action_ref default from "dev" to "main" so customers who don't set it track stable. Description annotated 'Internal: most users leave at the default'. * docs/gitlab-setup.md: drop droid_action_ref, droid_action_repo, stage from the Inputs table — they're for self-hosted GitLab mirrors / advanced overrides, not standard customer config. * docs/gitlab-setup.md: drop everything from 'How it works' onward (How it works, What's not yet supported, Troubleshooting, Self-hosted GitLab). Those belong in deeper docs / runbooks; the setup page should be exactly setup. * docs/gitlab-setup.md: drop the 'pin to a release tag (e.g. v1)' advice — droid-action doesn't tag yet, and the @main URL pin is the canonical pattern. * gitlab/examples/.gitlab-ci.example.yml: drop droid_action_ref from the inputs block + drop the same line from the optional fill snippet. Update the comment to just explain the @main pin. * gitlab/examples/README.md: drop the 'custom stage' reference now that stage isn't documented as a user-facing knob. The hidden inputs still exist in the template with sensible defaults, so air-gapped customers mirroring droid-action can still override droid_action_repo or pin droid_action_ref to a private SHA — they just aren't part of the documented surface. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- docs/gitlab-setup.md | 86 ++++---------------------- gitlab/examples/.gitlab-ci.example.yml | 37 +---------- gitlab/examples/README.md | 3 +- gitlab/templates/review.yml | 3 +- 4 files changed, 19 insertions(+), 110 deletions(-) diff --git a/docs/gitlab-setup.md b/docs/gitlab-setup.md index 518acb5..731fc7c 100644 --- a/docs/gitlab-setup.md +++ b/docs/gitlab-setup.md @@ -49,7 +49,6 @@ include: include_suggestions: "true" security_block_on_critical: "true" security_block_on_high: "false" - droid_action_ref: "main" droid-review: variables: @@ -57,10 +56,8 @@ droid-review: GITLAB_TOKEN: $GITLAB_TOKEN ``` -> Pin both the `include:` URL ref and `droid_action_ref` to a release tag -> (e.g. `v1`) once one is published. They MUST match — the URL pin -> resolves the template at parse time, `droid_action_ref` clones the same -> source at runtime. +> The `include:` URL is pinned to `@main`, which tracks the latest stable +> cut of droid-action. ### 3. Push an MR @@ -69,20 +66,17 @@ the `droid-review` job. Expect ~5-10 minutes for a typical change. ## Inputs -| Input | Default | Description | -| ---------------------------- | ------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------- | -| `automatic_review` | `"true"` | Run code review automatically on every MR pipeline. | -| `automatic_security_review` | `"false"` | Run a parallel security-focused subagent on every MR pipeline. Findings are prefixed `[security]` and posted alongside code-review comments. | -| `review_depth` | `"deep"` | `"deep"` (thorough) or `"shallow"` (fast). | -| `review_model` | `""` | Override the model. Empty = use depth preset. | -| `reasoning_effort` | `""` | Override reasoning effort. Empty = use depth preset. | -| `include_suggestions` | `"true"` | Include code suggestion blocks in review comments when the fix is high-confidence. | -| `security_block_on_critical` | `"true"` | Block merge on CRITICAL security findings. (Mirrors GitHub action; surface-level parity.) | -| `security_block_on_high` | `"false"` | Block merge on HIGH security findings. (Mirrors GitHub action; surface-level parity.) | -| `settings` | `""` | Droid Exec settings as a JSON string or a path to a JSON file. Merged into `~/.factory/droid/settings.json` before each `droid exec` call. | -| `droid_action_repo` | `https://github.com/Factory-AI/droid-action.git` | Override if you mirror droid-action privately. | -| `droid_action_ref` | `"dev"` | Git ref of droid-action to clone at runtime. Should match the `include: remote:` URL ref. | -| `stage` | `"test"` | GitLab CI stage to assign the `droid-review` job to. | +| Input | Default | Description | +| ---------------------------- | --------- | -------------------------------------------------------------------------------------------------------------------------------------------- | +| `automatic_review` | `"true"` | Run code review automatically on every MR pipeline. | +| `automatic_security_review` | `"false"` | Run a parallel security-focused subagent on every MR pipeline. Findings are prefixed `[security]` and posted alongside code-review comments. | +| `review_depth` | `"deep"` | `"deep"` (thorough) or `"shallow"` (fast). | +| `review_model` | `""` | Override the model. Empty = use depth preset. | +| `reasoning_effort` | `""` | Override reasoning effort. Empty = use depth preset. | +| `include_suggestions` | `"true"` | Include code suggestion blocks in review comments when the fix is high-confidence. | +| `security_block_on_critical` | `"true"` | Block merge on CRITICAL security findings. (Mirrors GitHub action; surface-level parity.) | +| `security_block_on_high` | `"false"` | Block merge on HIGH security findings. (Mirrors GitHub action; surface-level parity.) | +| `settings` | `""` | Droid Exec settings as a JSON string or a path to a JSON file. Merged into `~/.factory/droid/settings.json` before each `droid exec` call. | ## What you get @@ -99,57 +93,3 @@ Each MR pipeline produces: - **A custom droid library** copied from `$DROID_ACTION_DIR/.factory/droids` into `~/.factory/droids` on the runner, so subagents like `security-reviewer` are reachable. - -## How it works - -The pipeline runs in two passes: - -1. **Pass 1 (candidates)** — Droid Exec reads the MR diff, description, and - existing comments, then writes `/tmp/droid-prompts/review_candidates.json` - with candidate findings. The security-reviewer subagent runs in parallel - if `automatic_security_review` is on. **No MR mutations** happen in this - pass — the MR-write tools are excluded from `--enabled-tools`. -2. **Pass 2 (validator)** — Droid Exec rereads the candidates, filters out - duplicates and low-confidence findings, and batches all approved comments - into a single `gitlab_mr___submit_review` call. The sticky note is - updated via `gitlab_mr___update_tracking_note`. - -A small GitLab MCP server (`src/mcp/gitlab-mr-server.ts`) is launched on -the runner to expose those two write tools. - -## What's not yet supported - -These exist in the GitHub action but require platform features GitLab -does not currently expose, and are not part of Phase 1: - -- **Comment-triggered modes** (`@droid review`, `@droid security`, - `@droid fill`). GitLab does not fire CI pipelines on note events. - Requires a Factory-hosted webhook receiver that turns note webhooks - into `POST /projects/:id/trigger/pipeline` calls. Planned as a follow-up. -- **`@droid fill` (MR description fill).** Depends on the comment trigger - above. -- **Scheduled full-repo security scans.** Tracked separately. - -## Troubleshooting - -| Symptom | Likely cause | Fix | -| --------------------------------------------- | ------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------- | -| Pipeline runs but no comments posted | `GITLAB_TOKEN` not set or missing `api` scope | Add a masked `GITLAB_TOKEN` CI/CD variable with `api`-scoped PAT | -| `Could not find file` on include | `droid_action_ref` doesn't exist in `Factory-AI/droid-action` | Verify the ref; pin to a stable tag | -| `factory_api_key is required` error | `FACTORY_API_KEY` not set or scoped wrong | Add as **masked**, **unprotected** variable at the right level (project / subgroup / top-level group) | -| Sticky note posts but inline findings missing | Pass 2 hit a transient API error | Inspect `.droid-debug/prompts/pass2-output.jsonl`; re-run the pipeline | -| Pipeline times out | Very large MR | Set `review_depth: "shallow"` for faster feedback | - -## Self-hosted GitLab - -The component reads `CI_API_V4_URL` and `CI_SERVER_URL` from the standard -GitLab CI environment. Self-managed GitLab works as long as your runner -can: - -- Reach `https://app.factory.ai/cli` to install the Droid CLI -- Reach `https://github.com/Factory-AI/droid-action.git` (or your private - mirror, via `droid_action_repo`) -- Reach `https://raw.githubusercontent.com/...` for the `include: remote:` - -Mirror droid-action internally and set `droid_action_repo` + `droid_action_ref` -accordingly if your runners cannot reach GitHub. diff --git a/gitlab/examples/.gitlab-ci.example.yml b/gitlab/examples/.gitlab-ci.example.yml index e873075..dfd9203 100644 --- a/gitlab/examples/.gitlab-ci.example.yml +++ b/gitlab/examples/.gitlab-ci.example.yml @@ -19,15 +19,13 @@ # group level, depending on how widely you want the review to roll out. include: - # Pin to a release tag once droid-action publishes one. Until then, - # `main` tracks the latest stable cut. `droid_action_ref` below MUST - # match the ref in this URL so the template and the runtime source - # stay consistent. + # The URL is pinned to @main, which tracks the latest stable cut of + # droid-action. - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/review.yml" inputs: # ---- core review behavior ---- automatic_review: "true" # run on every MR event; "false" disables - review_depth: "deep" # "deep" (gpt-5.2 / high) or "shallow" (kimi-k2.6 / none) + review_depth: "deep" # "deep" (thorough) or "shallow" (fast) include_suggestions: "true" # post code-suggestion blocks for high-confidence fixes # ---- security review (parallel security-reviewer subagent) ---- @@ -35,36 +33,7 @@ include: security_block_on_critical: "true" # block merge on CRITICAL findings (when security review is on) security_block_on_high: "false" # block merge on HIGH findings (defaults to false) - # ---- runtime source pin ---- - droid_action_ref: "main" - droid-review: variables: FACTORY_API_KEY: $FACTORY_API_KEY GITLAB_TOKEN: $GITLAB_TOKEN -# ---- Optional: add @droid fill mode ------------------------------------- -# -# Uncomment the include below to also auto-fill new MRs' descriptions -# from their diffs. The fill job fires on the same `merge_request_event` -# trigger as the review job; together they form Factory Droid's GA GitLab -# experience. -# -# include: -# - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/fill.yml" -# inputs: -# automatic_fill: "false" # true = fill every new MR automatically -# droid_action_ref: "main" -# -# droid-fill: -# variables: -# FACTORY_API_KEY: $FACTORY_API_KEY -# GITLAB_TOKEN: $GITLAB_TOKEN -# -# With automatic_fill="false", trigger fill manually per MR by either: -# - writing "@droid fill" in the MR description, OR -# - writing "@droid fill" in the MR title, OR -# - adding the "droid:fill" label to the MR. -# -# After Droid fills the description, it strips the @droid fill token so -# the next merge_request_event doesn't re-trigger. -# ----------------------------------------------------------------------- diff --git a/gitlab/examples/README.md b/gitlab/examples/README.md index 30a18db..128d28a 100644 --- a/gitlab/examples/README.md +++ b/gitlab/examples/README.md @@ -19,6 +19,5 @@ Set both as **masked** CI/CD variables at the level you want the review to apply (project, subgroup, or top-level group). For the full input reference (model overrides, security review, -suggestion blocks, custom stage, etc.) see -[`../templates/review.yml`](../templates/review.yml) or the docs at +suggestion blocks, etc.) see the docs at [`docs/gitlab-setup.md`](../../docs/gitlab-setup.md). diff --git a/gitlab/templates/review.yml b/gitlab/templates/review.yml index 3ffaa1a..0c56a6a 100644 --- a/gitlab/templates/review.yml +++ b/gitlab/templates/review.yml @@ -54,7 +54,8 @@ spec: description: | Git ref (tag/branch/sha) of droid-action to clone at runtime. The same ref SHOULD match the `include: remote:` URL above. - default: "dev" + Internal: most users leave this at the default. + default: "main" stage: description: "GitLab CI stage to assign the droid-review job to." default: "test" From 4338125df38944136ed204ea2ee10730256d7801 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Tue, 2 Jun 2026 14:17:10 -0700 Subject: [PATCH 24/29] docs(readme): drop GitLab non-support note from quick-start Customer-facing surface should describe what is supported, not what isn't. Caveats belong in deeper docs / runbooks if anywhere, not in the README quick-start. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index ac728e0..acc307e 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ droid-review: GITLAB_TOKEN: $GITLAB_TOKEN ``` -Full setup, available inputs, and troubleshooting live in [`docs/gitlab-setup.md`](docs/gitlab-setup.md). Comment-triggered modes (`@droid review` / `@droid fill`) are not yet supported on GitLab — they require platform features GitLab does not expose natively. +Full setup, available inputs, and troubleshooting live in [`docs/gitlab-setup.md`](docs/gitlab-setup.md). ### Manual Setup From 80e65818a766c5e3a0d18ae40d9abd23f53806c7 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Tue, 2 Jun 2026 14:24:28 -0700 Subject: [PATCH 25/29] docs(readme): tighten GitLab section wording Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index acc307e..4f75ce1 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ For GitHub-only setups you can also run `/install-github-app`. See the [Automate ### GitLab -GitLab support ships as a **GitLab CI/CD Component** under `gitlab/templates/review.yml`. It delivers the same automatic two-pass code review and parallel security-reviewer subagent, with inline MR comments, a sticky tracking note, and `.droid-debug/` artifacts. +GitLab support ships as a **GitLab CI/CD Component** that delivers automated code review — inline MR comments on every merge request, with optional security review. Minimal `.gitlab-ci.yml`: From 448087de88edea6a9ec8b0379bc9c15cf078284f Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Tue, 2 Jun 2026 14:37:17 -0700 Subject: [PATCH 26/29] refactor(gitlab): rename template to droid-review.yml; ship as two-file pattern Component template renamed from gitlab/templates/review.yml to gitlab/templates/droid-review.yml. Customer drops a self-contained factory/droid-review.yml in their project and adds a single `include: - local:` line to their .gitlab-ci.yml, mirroring the GitHub action's `.github/workflows/droid-review.yml` model. Examples restructured to reflect the two-file layout. Docs + README snippets updated accordingly. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- README.md | 13 ++++++- docs/gitlab-setup.md | 27 ++++++++----- gitlab/examples/.gitlab-ci.example.yml | 39 ------------------- gitlab/examples/.gitlab-ci.minimal.yml | 12 ------ gitlab/examples/.gitlab-ci.yml | 9 +++++ gitlab/examples/README.md | 32 +++++++-------- gitlab/examples/factory/droid-review.yml | 29 ++++++++++++++ .../{review.yml => droid-review.yml} | 0 8 files changed, 82 insertions(+), 79 deletions(-) delete mode 100644 gitlab/examples/.gitlab-ci.example.yml delete mode 100644 gitlab/examples/.gitlab-ci.minimal.yml create mode 100644 gitlab/examples/.gitlab-ci.yml create mode 100644 gitlab/examples/factory/droid-review.yml rename gitlab/templates/{review.yml => droid-review.yml} (100%) diff --git a/README.md b/README.md index 4f75ce1..5043a39 100644 --- a/README.md +++ b/README.md @@ -42,11 +42,13 @@ For GitHub-only setups you can also run `/install-github-app`. See the [Automate GitLab support ships as a **GitLab CI/CD Component** that delivers automated code review — inline MR comments on every merge request, with optional security review. -Minimal `.gitlab-ci.yml`: +Two files in your project: + +`factory/droid-review.yml`: ```yaml include: - - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/review.yml" + - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/droid-review.yml" inputs: automatic_review: "true" automatic_security_review: "false" @@ -58,6 +60,13 @@ droid-review: GITLAB_TOKEN: $GITLAB_TOKEN ``` +`.gitlab-ci.yml` (one include line, append to existing if present): + +```yaml +include: + - local: "factory/droid-review.yml" +``` + Full setup, available inputs, and troubleshooting live in [`docs/gitlab-setup.md`](docs/gitlab-setup.md). ### Manual Setup diff --git a/docs/gitlab-setup.md b/docs/gitlab-setup.md index 731fc7c..61f3ba9 100644 --- a/docs/gitlab-setup.md +++ b/docs/gitlab-setup.md @@ -17,8 +17,9 @@ droid It detects GitLab, asks which account should be the poster of review comments (you supply its PAT as `GITLAB_TOKEN`), asks the configuration -questions below, generates the `.gitlab-ci.yml`, and opens an MR / -direct-commits to the target project(s). +questions below, drops `factory/droid-review.yml` in your project, wires +it into `.gitlab-ci.yml`, and opens an MR / direct-commits to the target +project(s). ## Manual installation @@ -32,16 +33,17 @@ direct-commits to the target project(s). ### 2. Add the CI/CD Component -Drop-in samples live in [`gitlab/examples/`](../gitlab/examples/): +Drop-in samples live in [`gitlab/examples/`](../gitlab/examples/). The +layout is two files: -- [`.gitlab-ci.minimal.yml`](../gitlab/examples/.gitlab-ci.minimal.yml) — shortest possible, accepts every default. -- [`.gitlab-ci.example.yml`](../gitlab/examples/.gitlab-ci.example.yml) — annotated with every input. +- [`factory/droid-review.yml`](../gitlab/examples/factory/droid-review.yml) — self-contained config (include + inputs + variables). Drop verbatim. +- [`.gitlab-ci.yml`](../gitlab/examples/.gitlab-ci.yml) — project-root entry point. If you already have one, append the include line below to its `include:` block. -The minimal form looks like this; create or extend `.gitlab-ci.yml` in your project with: +**`factory/droid-review.yml`** (drop into your project): ```yaml include: - - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/review.yml" + - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/droid-review.yml" inputs: automatic_review: "true" automatic_security_review: "false" @@ -56,8 +58,15 @@ droid-review: GITLAB_TOKEN: $GITLAB_TOKEN ``` -> The `include:` URL is pinned to `@main`, which tracks the latest stable -> cut of droid-action. +**`.gitlab-ci.yml`** (project root, just needs the one include line): + +```yaml +include: + - local: "factory/droid-review.yml" +``` + +> The remote `include:` URL is pinned to `@main`, which tracks the +> latest stable cut of droid-action. ### 3. Push an MR diff --git a/gitlab/examples/.gitlab-ci.example.yml b/gitlab/examples/.gitlab-ci.example.yml deleted file mode 100644 index dfd9203..0000000 --- a/gitlab/examples/.gitlab-ci.example.yml +++ /dev/null @@ -1,39 +0,0 @@ -# Example .gitlab-ci.yml showcasing the droid-action GitLab CI/CD Component. -# -# Drop this file (or its snippets) at the root of your GitLab project. -# It defines a single `droid-review` job that runs Factory Droid's -# automated code review on every merge_request_event. -# -# Prerequisites (one-time, configured via the GitLab UI or `glab`): -# -# * FACTORY_API_KEY — masked CI/CD variable. Get one at -# https://app.factory.ai/settings/api-keys -# -# * GITLAB_TOKEN — masked CI/CD variable. A personal access token -# with `api` scope, minted by the GitLab user/account you want -# review comments to be posted from. The poster identity IS the -# owner of this token; there is no API impersonation. To switch -# posters later, replace the variable's value. -# -# Both variables can be set at the project, subgroup, or top-level -# group level, depending on how widely you want the review to roll out. - -include: - # The URL is pinned to @main, which tracks the latest stable cut of - # droid-action. - - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/review.yml" - inputs: - # ---- core review behavior ---- - automatic_review: "true" # run on every MR event; "false" disables - review_depth: "deep" # "deep" (thorough) or "shallow" (fast) - include_suggestions: "true" # post code-suggestion blocks for high-confidence fixes - - # ---- security review (parallel security-reviewer subagent) ---- - automatic_security_review: "false" # enable to flag vulns alongside the regular review - security_block_on_critical: "true" # block merge on CRITICAL findings (when security review is on) - security_block_on_high: "false" # block merge on HIGH findings (defaults to false) - -droid-review: - variables: - FACTORY_API_KEY: $FACTORY_API_KEY - GITLAB_TOKEN: $GITLAB_TOKEN diff --git a/gitlab/examples/.gitlab-ci.minimal.yml b/gitlab/examples/.gitlab-ci.minimal.yml deleted file mode 100644 index 97643e4..0000000 --- a/gitlab/examples/.gitlab-ci.minimal.yml +++ /dev/null @@ -1,12 +0,0 @@ -# Minimal example: every default, just `include:` the Component. -# -# Requires FACTORY_API_KEY and GITLAB_TOKEN to be set as masked CI/CD -# variables. See .gitlab-ci.example.yml for the annotated version. - -include: - - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/review.yml" - -droid-review: - variables: - FACTORY_API_KEY: $FACTORY_API_KEY - GITLAB_TOKEN: $GITLAB_TOKEN diff --git a/gitlab/examples/.gitlab-ci.yml b/gitlab/examples/.gitlab-ci.yml new file mode 100644 index 0000000..36cb454 --- /dev/null +++ b/gitlab/examples/.gitlab-ci.yml @@ -0,0 +1,9 @@ +# Example project-root .gitlab-ci.yml. +# +# If the project doesn't have a .gitlab-ci.yml yet, create one with at +# minimum the include line below. If it already has one, just append the +# `- local: "factory/droid-review.yml"` entry to its existing `include:` +# block (or add a new `include:` block if there isn't one). + +include: + - local: "factory/droid-review.yml" diff --git a/gitlab/examples/README.md b/gitlab/examples/README.md index 128d28a..05fcc72 100644 --- a/gitlab/examples/README.md +++ b/gitlab/examples/README.md @@ -1,23 +1,21 @@ # GitLab examples -Drop-in `.gitlab-ci.yml` samples for consuming the droid-action GitLab -CI/CD Component. +Two-file layout for consuming the droid-action GitLab CI/CD Component: -| File | When to use | -| ------------------------ | -------------------------------------------------------------------------------------------------------------- | -| `.gitlab-ci.minimal.yml` | Shortest possible — `include:` the review template, accept every default. Good starting point. | -| `.gitlab-ci.example.yml` | Annotated. Every input spelled out with comments explaining what it does and what the safe defaults look like. | +``` +your-project/ +├── .gitlab-ci.yml # gains one `include:` line +└── factory/ + └── droid-review.yml # self-contained droid-review config +``` -Both samples wire the same two required CI/CD variables: +| File | Where it lives in your project | Purpose | +| ----------------------------- | -------------------------------------------------- | --------------------------------------------------------------------------------------------- | +| `factory/droid-review.yml` | drop verbatim | Self-contained config: includes the remote Component, sets inputs, wires CI/CD variables. | +| `.gitlab-ci.yml` | append one `include:` line if the file exists | Project-root entry point. Just needs to include `factory/droid-review.yml`. | -- `FACTORY_API_KEY` — get one at . -- `GITLAB_TOKEN` — a personal access token with `api` scope, owned by - whichever GitLab user/account should be the poster of review comments. - To change the poster later, replace this variable's value. +The two required CI/CD variables (`FACTORY_API_KEY`, `GITLAB_TOKEN`) are set +in the GitLab UI under **Project → Settings → CI/CD → Variables** (or at +the group level for org-wide rollout), masked and unprotected. -Set both as **masked** CI/CD variables at the level you want the review -to apply (project, subgroup, or top-level group). - -For the full input reference (model overrides, security review, -suggestion blocks, etc.) see the docs at -[`docs/gitlab-setup.md`](../../docs/gitlab-setup.md). +For the full input reference see the docs at [`docs/gitlab-setup.md`](../../docs/gitlab-setup.md). diff --git a/gitlab/examples/factory/droid-review.yml b/gitlab/examples/factory/droid-review.yml new file mode 100644 index 0000000..baa11dc --- /dev/null +++ b/gitlab/examples/factory/droid-review.yml @@ -0,0 +1,29 @@ +# factory/droid-review.yml +# +# Drop this file at `factory/droid-review.yml` in your project, then +# add a single `include: - local: "factory/droid-review.yml"` line to +# your `.gitlab-ci.yml` (see ../.gitlab-ci.yml for the entry-point +# example). +# +# Prerequisites (one-time, set in Project / Group → Settings → CI/CD): +# +# * FACTORY_API_KEY — masked variable. Get one at +# https://app.factory.ai/settings/api-keys +# * GITLAB_TOKEN — masked variable. A personal access token with +# `api` scope, owned by whichever GitLab account should post review +# comments. The token owner IS the poster. + +include: + - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/droid-review.yml" + inputs: + automatic_review: "true" # run on every MR event; "false" disables + review_depth: "deep" # "deep" (thorough) or "shallow" (fast) + include_suggestions: "true" # post code-suggestion blocks for high-confidence fixes + automatic_security_review: "false" # enable to flag vulns alongside the regular review + security_block_on_critical: "true" # block merge on CRITICAL findings (when security review is on) + security_block_on_high: "false" # block merge on HIGH findings + +droid-review: + variables: + FACTORY_API_KEY: $FACTORY_API_KEY + GITLAB_TOKEN: $GITLAB_TOKEN diff --git a/gitlab/templates/review.yml b/gitlab/templates/droid-review.yml similarity index 100% rename from gitlab/templates/review.yml rename to gitlab/templates/droid-review.yml From 1e3e0aa0889ae909745c6c023254e61cd779010b Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Tue, 2 Jun 2026 15:36:05 -0700 Subject: [PATCH 27/29] refactor(gitlab): move component to top-level templates/ for Catalog compatibility GitLab CI/CD Catalog requires components to live at top-level templates/. Move gitlab/templates/droid-review.yml to templates/droid-review.yml so the repo is ready for Catalog publish (gitlab.com/factory-ai/droid-action) without future renames. Add templates/README.md explaining the directory ownership (mandated by Catalog, parallel to action.yml + .github/workflows/ for GitHub) and a headline comment in droid-review.yml. Update all existing include URLs (README, docs, examples) to the new path. Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- README.md | 2 +- docs/gitlab-setup.md | 2 +- gitlab/examples/factory/droid-review.yml | 2 +- templates/README.md | 31 +++++++++++++++++++ .../templates => templates}/droid-review.yml | 13 ++++++++ 5 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 templates/README.md rename {gitlab/templates => templates}/droid-review.yml (95%) diff --git a/README.md b/README.md index 5043a39..4f92e31 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,7 @@ Two files in your project: ```yaml include: - - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/droid-review.yml" + - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/templates/droid-review.yml" inputs: automatic_review: "true" automatic_security_review: "false" diff --git a/docs/gitlab-setup.md b/docs/gitlab-setup.md index 61f3ba9..e1ef530 100644 --- a/docs/gitlab-setup.md +++ b/docs/gitlab-setup.md @@ -43,7 +43,7 @@ layout is two files: ```yaml include: - - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/droid-review.yml" + - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/templates/droid-review.yml" inputs: automatic_review: "true" automatic_security_review: "false" diff --git a/gitlab/examples/factory/droid-review.yml b/gitlab/examples/factory/droid-review.yml index baa11dc..248aa8b 100644 --- a/gitlab/examples/factory/droid-review.yml +++ b/gitlab/examples/factory/droid-review.yml @@ -14,7 +14,7 @@ # comments. The token owner IS the poster. include: - - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/gitlab/templates/droid-review.yml" + - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/templates/droid-review.yml" inputs: automatic_review: "true" # run on every MR event; "false" disables review_depth: "deep" # "deep" (thorough) or "shallow" (fast) diff --git a/templates/README.md b/templates/README.md new file mode 100644 index 0000000..2c748dd --- /dev/null +++ b/templates/README.md @@ -0,0 +1,31 @@ +# GitLab CI/CD Components + +This directory is **GitLab-specific**. The path is mandated by the +[GitLab CI/CD Catalog](https://docs.gitlab.com/ee/ci/components/) — every +Component project must define its components under top-level `templates/`. + +GitHub action code lives at `action.yml` (root) and `.github/workflows/`. + +## Components in this directory + +| File | Component | Usage | +| ------------------ | -------------- | ---------------------------------------------------------------- | +| `droid-review.yml` | `droid-review` | Automated MR code review (two-pass, optional security subagent). | + +## Consuming a component + +Once the Catalog publishes (`gitlab.com/factory-ai/droid-action`): + +```yaml +include: + - component: gitlab.com/factory-ai/droid-action/droid-review@v1 +``` + +Until then, consume via the raw GitHub URL: + +```yaml +include: + - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/templates/droid-review.yml" +``` + +See [`../docs/gitlab-setup.md`](../docs/gitlab-setup.md) for the full setup guide and [`../gitlab/examples/`](../gitlab/examples/) for drop-in samples. diff --git a/gitlab/templates/droid-review.yml b/templates/droid-review.yml similarity index 95% rename from gitlab/templates/droid-review.yml rename to templates/droid-review.yml index 0c56a6a..45a488b 100644 --- a/gitlab/templates/droid-review.yml +++ b/templates/droid-review.yml @@ -1,3 +1,16 @@ +# GitLab CI/CD Component: droid-review +# +# Automated Droid code review on merge requests. Two-pass pipeline +# (candidate generation + validator), inline MR comments, sticky tracking +# note with telemetry, optional parallel security-reviewer subagent. +# +# Consumed via: +# include: +# - component: gitlab.com/factory-ai/droid-action/droid-review@ +# +# Catalog requires this file to live at `templates/.yml`. +# See ./README.md for the full directory contract. + spec: inputs: automatic_review: From 756198907b21b66cd028db6ae6f5cebf7dc3c454 Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Tue, 2 Jun 2026 16:12:14 -0700 Subject: [PATCH 28/29] feat(gitlab): point at gitlab.com/factory-components/droid-action mirror MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switch the customer-facing include form from a raw GitHub URL to GitLab's native include: project: form pointing at the gitlab.com pull-mirror of this repo. Customers no longer need github.com egress to fetch the template — gitlab.com is sufficient. Also flip the runtime droid_action_repo default from github.com/Factory-AI/droid-action.git to gitlab.com/factory-components/droid-action.git so the at-job clone of the runtime source also goes through gitlab.com. Customers behind a firewall that only permits gitlab.com + app.factory.ai can now run the full flow. Updated: - README.md GitLab quick-start snippet (include: project: form) - docs/gitlab-setup.md Step 2 snippet - gitlab/examples/factory/droid-review.yml - templates/README.md — documents include: project: (default), future include: component: form once Catalog-tagged, and a raw GitHub URL fallback for projects that can't reach gitlab.com - templates/droid-review.yml headline comment + droid_action_repo default Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com> --- README.md | 4 +++- docs/gitlab-setup.md | 4 +++- gitlab/examples/factory/droid-review.yml | 4 +++- templates/README.md | 15 ++++++++++++--- templates/droid-review.yml | 16 ++++++++++++---- 5 files changed, 33 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 4f92e31..c47e1ca 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,9 @@ Two files in your project: ```yaml include: - - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/templates/droid-review.yml" + - project: "factory-components/droid-action" + ref: main + file: "/templates/droid-review.yml" inputs: automatic_review: "true" automatic_security_review: "false" diff --git a/docs/gitlab-setup.md b/docs/gitlab-setup.md index e1ef530..2a5c835 100644 --- a/docs/gitlab-setup.md +++ b/docs/gitlab-setup.md @@ -43,7 +43,9 @@ layout is two files: ```yaml include: - - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/templates/droid-review.yml" + - project: "factory-components/droid-action" + ref: main + file: "/templates/droid-review.yml" inputs: automatic_review: "true" automatic_security_review: "false" diff --git a/gitlab/examples/factory/droid-review.yml b/gitlab/examples/factory/droid-review.yml index 248aa8b..bb64e1f 100644 --- a/gitlab/examples/factory/droid-review.yml +++ b/gitlab/examples/factory/droid-review.yml @@ -14,7 +14,9 @@ # comments. The token owner IS the poster. include: - - remote: "https://raw.githubusercontent.com/Factory-AI/droid-action/main/templates/droid-review.yml" + - project: "factory-components/droid-action" + ref: main + file: "/templates/droid-review.yml" inputs: automatic_review: "true" # run on every MR event; "false" disables review_depth: "deep" # "deep" (thorough) or "shallow" (fast) diff --git a/templates/README.md b/templates/README.md index 2c748dd..790ebf1 100644 --- a/templates/README.md +++ b/templates/README.md @@ -14,14 +14,23 @@ GitHub action code lives at `action.yml` (root) and `.github/workflows/`. ## Consuming a component -Once the Catalog publishes (`gitlab.com/factory-ai/droid-action`): +This repo is mirrored to [`gitlab.com/factory-components/droid-action`](https://gitlab.com/factory-components/droid-action). Customers include the template from the mirror: ```yaml include: - - component: gitlab.com/factory-ai/droid-action/droid-review@v1 + - project: "factory-components/droid-action" + ref: main + file: "/templates/droid-review.yml" ``` -Until then, consume via the raw GitHub URL: +Once the project is marked as a [CI/CD Catalog](https://docs.gitlab.com/ee/ci/components/) project and the first tagged release is published, the shorter `component:` form also becomes available: + +```yaml +include: + - component: gitlab.com/factory-components/droid-action/droid-review@v1 +``` + +Fallback for projects that can't reach gitlab.com (use the raw GitHub URL): ```yaml include: diff --git a/templates/droid-review.yml b/templates/droid-review.yml index 45a488b..6a81862 100644 --- a/templates/droid-review.yml +++ b/templates/droid-review.yml @@ -6,7 +6,13 @@ # # Consumed via: # include: -# - component: gitlab.com/factory-ai/droid-action/droid-review@ +# - project: "factory-components/droid-action" +# ref: main +# file: "/templates/droid-review.yml" +# +# Or, once Catalog-published: +# include: +# - component: gitlab.com/factory-components/droid-action/droid-review@ # # Catalog requires this file to live at `templates/.yml`. # See ./README.md for the full directory contract. @@ -60,9 +66,11 @@ spec: default: "" droid_action_repo: description: | - Git repo (clone URL) of droid-action. Override if you mirror it - privately. Must be reachable from your GitLab runner. - default: "https://github.com/Factory-AI/droid-action.git" + Git repo (clone URL) of droid-action. Defaults to the gitlab.com + mirror so GitLab runners with restricted egress only need + gitlab.com + app.factory.ai access. Override if you mirror it + privately. + 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. From 7ad303f11eb51315865319694e83a96d6a5e748e Mon Sep 17 00:00:00 2001 From: Nizar Alrifai Date: Wed, 3 Jun 2026 10:05:05 -0700 Subject: [PATCH 29/29] 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> --- src/gitlab/token.ts | 5 +-- src/mcp/gitlab-mr-server.ts | 79 +++++++++++++++++++++---------------- templates/droid-review.yml | 13 +++--- test/gitlab/token.test.ts | 4 +- 4 files changed, 56 insertions(+), 45 deletions(-) diff --git a/src/gitlab/token.ts b/src/gitlab/token.ts index cbbb681..0d73ed6 100644 --- a/src/gitlab/token.ts +++ b/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(); diff --git a/src/mcp/gitlab-mr-server.ts b/src/mcp/gitlab-mr-server.ts index 46c9d29..292eaf3 100644 --- a/src/mcp/gitlab-mr-server.ts +++ b/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; @@ -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 { diff --git a/templates/droid-review.yml b/templates/droid-review.yml index 6a81862..26f7b98 100644 --- a/templates/droid-review.yml +++ b/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 diff --git a/test/gitlab/token.test.ts b/test/gitlab/token.test.ts index 9cd5a49..734fdb3 100644 --- a/test/gitlab/token.test.ts +++ b/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", () => {