diff --git a/README.md b/README.md index eadf5e9..c47e1ca 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,39 @@ 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** that delivers automated code review — inline MR comments on every merge request, with optional security review. + +Two files in your project: + +`factory/droid-review.yml`: + +```yaml +include: + - project: "factory-components/droid-action" + ref: main + file: "/templates/droid-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 +``` + +`.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 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..2a5c835 --- /dev/null +++ b/docs/gitlab-setup.md @@ -0,0 +1,106 @@ +# 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, asks which account should be the poster of review +comments (you supply its PAT as `GITLAB_TOKEN`), asks the configuration +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 + +### 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 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 + +Drop-in samples live in [`gitlab/examples/`](../gitlab/examples/). The +layout is two files: + +- [`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. + +**`factory/droid-review.yml`** (drop into your project): + +```yaml +include: + - project: "factory-components/droid-action" + ref: main + file: "/templates/droid-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-review: + variables: + FACTORY_API_KEY: $FACTORY_API_KEY + GITLAB_TOKEN: $GITLAB_TOKEN +``` + +**`.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 + +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. | + +## 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. 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 new file mode 100644 index 0000000..05fcc72 --- /dev/null +++ b/gitlab/examples/README.md @@ -0,0 +1,21 @@ +# GitLab examples + +Two-file layout for consuming the droid-action GitLab CI/CD Component: + +``` +your-project/ +├── .gitlab-ci.yml # gains one `include:` line +└── factory/ + └── droid-review.yml # self-contained droid-review config +``` + +| 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`. | + +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. + +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..bb64e1f --- /dev/null +++ b/gitlab/examples/factory/droid-review.yml @@ -0,0 +1,31 @@ +# 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: + - 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) + 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/src/core/review/artifacts/types.ts b/src/core/review/artifacts/types.ts new file mode 100644 index 0000000..62843d0 --- /dev/null +++ b/src/core/review/artifacts/types.ts @@ -0,0 +1,25 @@ +/** + * Shared shape for the three on-disk review artifacts. Both GitHub and + * GitLab pipelines pre-compute the same trio (diff, existing comments, + * description) and write them under `${tempDir}/droid-prompts/`, then + * pass the resulting paths into the Pass 1 / Pass 2 prompts. + * + * The fetch mechanics differ substantially per platform (git+gh CLI vs + * REST API), so we don't try to share the fetchers — only the path + * shape and the disk-write helper. + */ +export type ReviewArtifactPaths = { + diffPath: string; + commentsPath: string; + descriptionPath: string; +}; + +/** + * Raw content for each of the three artifacts, before they're written + * to disk by `writeReviewArtifacts`. + */ +export type ReviewArtifactContents = { + diff: string; + comments: unknown; // JSON-serializable + description: string; +}; diff --git a/src/core/review/artifacts/write.ts b/src/core/review/artifacts/write.ts new file mode 100644 index 0000000..4befa76 --- /dev/null +++ b/src/core/review/artifacts/write.ts @@ -0,0 +1,41 @@ +import * as fs from "fs/promises"; +import * as path from "path"; +import type { ReviewArtifactContents, ReviewArtifactPaths } from "./types"; + +/** + * Naming convention for review-artifact files on disk. Each platform + * gets its own basename for the diff and description (pr.diff / + * mr.diff, pr_description.txt / mr_description.txt) but the existing + * comments file is platform-neutral. + */ +export type ReviewArtifactNames = { + diff: string; + comments: string; + description: string; +}; + +/** + * Write the three review-artifact files into `outDir` in parallel, + * creating `outDir` if it doesn't yet exist. Returns the resolved + * on-disk paths so the caller can hand them straight to the prompt + * builder. + */ +export async function writeReviewArtifacts( + outDir: string, + contents: ReviewArtifactContents, + names: ReviewArtifactNames, +): Promise { + await fs.mkdir(outDir, { recursive: true }); + + const diffPath = path.join(outDir, names.diff); + const commentsPath = path.join(outDir, names.comments); + const descriptionPath = path.join(outDir, names.description); + + await Promise.all([ + fs.writeFile(diffPath, contents.diff), + fs.writeFile(commentsPath, JSON.stringify(contents.comments, null, 2)), + fs.writeFile(descriptionPath, contents.description), + ]); + + return { diffPath, commentsPath, descriptionPath }; +} diff --git a/src/core/review/prompts/candidates.ts b/src/core/review/prompts/candidates.ts new file mode 100644 index 0000000..d5fb278 --- /dev/null +++ b/src/core/review/prompts/candidates.ts @@ -0,0 +1,146 @@ +/** + * Platform-agnostic Pass 1 (candidate generation) prompt. + * + * Both `src/create-prompt/templates/review-candidates-prompt.ts` (GitHub) + * and `src/gitlab/prompts/candidates.ts` (GitLab) delegate to this builder + * via thin adapters that supply a `ReviewTerminology` object plus the + * runtime context (entity number, SHAs, artifact paths). The /review skill + * itself is platform-agnostic; this builder produces the runtime harness + * (where to read inputs, where to write the candidates JSON, what NOT to + * call) around it. + * + * STRICT contract: this prompt MUST NOT instruct the model to call any + * posting tool. Posting is Pass 2's responsibility, and tool gating is + * additionally enforced at the `droid exec` level via `--enabled-tools`. + */ + +import type { ReviewPromptContext } from "./types"; + +export function generateCandidatesPrompt(ctx: ReviewPromptContext): string { + const { + terminology: t, + entityNumber, + repoOrProject, + headRef, + headSha, + baseRef, + 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 ${t.entityNoun} context (${t.repoLabel.toLowerCase()}, ${t.entityNoun} ${t.metaEntityNumberKey === "prNumber" ? "number" : "IID"}, head SHA, ${t.baseRefShortLabel}) 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 ${t.entityNoun} ${t.entityNumberSigil}${entityNumber} in ${repoOrProject} and generate a JSON file with **high-confidence, actionable** review comments that pinpoint genuine issues. + +${skillInstruction}${securitySubagentInstruction} + + +${t.repoLabel}: ${repoOrProject} +${t.entityNumberLabel}: ${entityNumber} +${t.headRefLabel}: ${headRef} +${t.headShaLabel}: ${headSha} +${t.baseRefLabel}: ${baseRef} + +Precomputed data files: +- ${t.descriptionLabel}: \`${descriptionPath}\` +- ${t.diffLabel}: \`${diffPath}\` +- Existing Comments: \`${commentsPath}\` + + + +Write output to \`${candidatesPath}\` using this exact schema: + +\`\`\`json +{ + "version": 1, + "meta": { + "${t.metaRepoKey}": "${t.repoExample}", + "${t.metaEntityNumberKey}": 123, + "headSha": "", + "${t.metaBaseRefKey}": "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 + - \`${t.metaRepoKey}\`: "${repoOrProject}" + - \`${t.metaEntityNumberKey}\`: ${entityNumber} + - \`headSha\`: "${headSha}" + - \`${t.metaBaseRefKey}\`: "${baseRef}" + - \`generatedAt\`: ISO 8601 timestamp (e.g., "2024-01-15T10:30:00Z") + +- **comments**: Array of comment objects + - \`path\`: ${t.pathFieldDescription} +${bodyFieldDescription} + - \`line\`: ${t.lineFieldDescription} + - \`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 ${t.platformName}. +**DO NOT** invoke any ${t.entityNoun} mutation tools ${t.mutationToolForbiddance}. +**DO NOT** modify any files other than writing to \`${candidatesPath}\`. +Output ONLY the JSON file—no additional commentary. + +`; +} diff --git a/src/core/review/prompts/types.ts b/src/core/review/prompts/types.ts new file mode 100644 index 0000000..58313c7 --- /dev/null +++ b/src/core/review/prompts/types.ts @@ -0,0 +1,93 @@ +/** + * Shared types for the platform-agnostic review prompts. + * + * The candidate-generation (Pass 1) and validator (Pass 2) prompts are + * structurally identical between GitHub and GitLab. The only things that + * vary are labels, identifier names, MCP tool names, and a small number + * of platform-specific instruction lines. Those variations are captured + * here so the shared templates can stay platform-agnostic. + */ + +export interface ReviewTerminology { + /** "PR" or "MR" */ + entityNoun: string; + /** "#" for GitHub PRs, "!" for GitLab MRs */ + entityNumberSigil: string; + /** "GitHub" or "GitLab" */ + platformName: string; + /** Label for the repo/project value (e.g. "Repo" or "Project") */ + repoLabel: string; + /** Label for the entity number row in (e.g. "PR Number" or "MR IID") */ + entityNumberLabel: string; + /** Label for the source branch row (e.g. "PR Head Ref" or "MR Source Branch") */ + headRefLabel: string; + /** Label for the head SHA row (e.g. "PR Head SHA" or "MR Head SHA") */ + headShaLabel: string; + /** Label for the target branch row (e.g. "PR Base Ref" or "MR Target Branch") */ + baseRefLabel: string; + /** Short form of the target-branch concept, used in narrative prose (e.g. "base ref" or "target branch") */ + baseRefShortLabel: string; + /** Label for the description artifact (e.g. "PR Description" or "MR Description") */ + descriptionLabel: string; + /** Label for the diff artifact (e.g. "Full PR Diff" or "Full MR Diff") */ + diffLabel: string; + /** JSON meta key for the repo identifier (e.g. "repo" or "project") */ + metaRepoKey: string; + /** JSON meta key for the entity number (e.g. "prNumber" or "mrIid") */ + metaEntityNumberKey: string; + /** JSON meta key for the base ref (e.g. "baseRef" or "targetBranch") */ + metaBaseRefKey: string; + /** Example identifier in schema doc (e.g. "owner/repo" or "group/project") */ + repoExample: string; + /** Free-form description for the `path` field (small wording variation) */ + pathFieldDescription: string; + /** Free-form description for the `line` field (small wording variation) */ + lineFieldDescription: string; + /** Free-form blurb listing mutation tools the candidates pass MUST NOT use */ + mutationToolForbiddance: string; + /** MCP tool name for posting a batched review (Pass 2) */ + submitReviewToolName: string; + /** Extra args appended to the submit_review instruction (e.g. " along with `mr_iid: 5`") */ + submitReviewExtraArg: string; + /** Trailing clause on the "Do NOT include a body parameter" line */ + submitReviewBodyExclusionTrailer: string; + /** MCP tool name for updating the sticky tracking comment/note */ + updateTrackingToolName: string; + /** Human-readable name for the sticky comment (e.g. "tracking comment" or "sticky tracking note") */ + trackingCommentName: string; + /** "comment" (GH) or "top-level note" (GL), used in "post the summary as a separate X" */ + summaryEntityName: string; + /** Trailing clause on the summary-forbiddance line (e.g. " or as the body of `submit_review`" on GH; "" on GL) */ + summaryPostingExtraExclusion: string; + /** Approval/changes line at the end of validator instructions */ + approvalChangesNote: string; + /** Optional security-badge instruction line — GL only */ + securityBadgeInstruction?: string; +} + +export interface ReviewPromptContext { + terminology: ReviewTerminology; + /** PR number / MR IID */ + entityNumber: number | string; + /** Full repo/project identifier */ + repoOrProject: string; + /** Source branch name */ + headRef: string; + /** Head SHA */ + headSha: string; + /** Target branch name */ + baseRef: string; + /** On-disk path to the diff artifact */ + diffPath: string; + /** On-disk path to existing-comments JSON */ + commentsPath: string; + /** On-disk path to description text */ + descriptionPath: string; + /** On-disk path where Pass 1 writes the candidates JSON */ + candidatesPath: string; + /** On-disk path where Pass 2 writes the validated JSON (validator only) */ + validatedPath?: string; + includeSuggestions: boolean; + /** Spawn security-reviewer subagent during Pass 1 (candidates only) */ + securityReviewEnabled: boolean; +} diff --git a/src/core/review/prompts/validator.ts b/src/core/review/prompts/validator.ts new file mode 100644 index 0000000..50c2c40 --- /dev/null +++ b/src/core/review/prompts/validator.ts @@ -0,0 +1,138 @@ +/** + * Platform-agnostic Pass 2 (validator) prompt. + * + * The validator reads the candidates JSON produced by Pass 1, validates + * each one, writes a refined JSON to disk, and posts the approved + * findings as a single batched call to the platform's submit-review tool. + * Both `src/create-prompt/templates/review-validator-prompt.ts` (GitHub) + * and `src/gitlab/prompts/validator.ts` (GitLab) delegate to this builder + * via thin adapters that supply a `ReviewTerminology` shape. + * + * Pass 2 is the only place where the posting tool is exposed (via + * `--enabled-tools` on the second `droid exec` invocation). + */ + +import type { ReviewPromptContext } from "./types"; + +export function generateValidatorPrompt(ctx: ReviewPromptContext): string { + const { + terminology: t, + entityNumber, + repoOrProject, + headRef, + headSha, + baseRef, + diffPath, + commentsPath, + descriptionPath, + candidatesPath, + validatedPath, + includeSuggestions, + } = ctx; + + if (!validatedPath) { + throw new Error("validator prompt requires validatedPath in context"); + } + + 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."; + + const securityBadgeLine = t.securityBadgeInstruction + ? `\n* ${t.securityBadgeInstruction}` + : ""; + + return `You are validating candidate review comments for ${t.entityNoun} ${t.entityNumberSigil}${entityNumber} in ${repoOrProject}. + +IMPORTANT: This is Phase 2 (validator) of a two-pass review pipeline. + +${skillInstruction} + +### Context + +* ${t.repoLabel}: ${repoOrProject} +* ${t.entityNumberLabel}: ${entityNumber} +* ${t.headRefLabel}: ${headRef} +* ${t.headShaLabel}: ${headSha} +* ${t.baseRefLabel}: ${baseRef} + +### Inputs + +Read these files before validating: +* ${t.descriptionLabel}: \`${descriptionPath}\` +* Candidates: \`${candidatesPath}\` +* ${t.diffLabel}: \`${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": { + "${t.metaRepoKey}": "${repoOrProject}", + "${t.metaEntityNumberKey}": ${entityNumber}, + "headSha": "${headSha}", + "${t.metaBaseRefKey}": "${baseRef}", + "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 \`${t.submitReviewToolName}\`, passing them in the \`comments\` array parameter${t.submitReviewExtraArg}. +* Do **NOT** post comments individually — batch them all into one \`submit_review\` call. +* Do **NOT** include a \`body\` parameter in \`submit_review\`${t.submitReviewBodyExclusionTrailer}. +* Use \`${t.updateTrackingToolName}\` to update the ${t.trackingCommentName} with the review summary.${securityBadgeLine} +* Do **NOT** post the summary as a separate ${t.summaryEntityName}${t.summaryPostingExtraExclusion}. +* ${t.approvalChangesNote} +`; +} diff --git a/src/core/review/tracking/format.ts b/src/core/review/tracking/format.ts new file mode 100644 index 0000000..b008cb1 --- /dev/null +++ b/src/core/review/tracking/format.ts @@ -0,0 +1,25 @@ +/** + * Small formatting helpers for sticky review-tracking + * comments/notes. Currently consumed by the GitLab tracking-note + * builder (`src/gitlab/operations/tracking-note.ts`); kept platform- + * agnostic in core so the GitHub MCP comment server can adopt the same + * "1m 23s • $0.0042" rendering when its telemetry path is unified. + */ + +export function formatDurationMs(ms: number): string { + if (ms < 1000) return `${ms}ms`; + const seconds = ms / 1000; + if (seconds < 60) return `${seconds.toFixed(1)}s`; + // Round to whole seconds first, then split into minutes+seconds, so + // that a remainder rounding up to 60 carries cleanly into the next + // minute (e.g. 119600ms → "2m 0s", not "1m 60s"). + const totalSeconds = Math.round(seconds); + const minutes = Math.floor(totalSeconds / 60); + const remSec = totalSeconds - minutes * 60; + return `${minutes}m ${remSec}s`; +} + +export function formatCostUsd(usd: number): string { + if (usd >= 1) return `$${usd.toFixed(2)}`; + return `$${usd.toFixed(4)}`; +} diff --git a/src/core/review/tracking/types.ts b/src/core/review/tracking/types.ts new file mode 100644 index 0000000..5a24b64 --- /dev/null +++ b/src/core/review/tracking/types.ts @@ -0,0 +1,31 @@ +/** + * Shared shapes for sticky review-tracking comments/notes. + * + * Both GitHub and GitLab keep a single "sticky" comment per PR/MR that + * carries the live status of the Droid review pipeline (running → + * success/failure) plus telemetry. The body-building mechanics differ + * between platforms today — GitHub updates it from inside the agent via + * the github-comment-server MCP tool, while GitLab builds the whole body + * in TypeScript and writes it from CI — so we only share the contract + * (state machine + telemetry shape) here, not the renderer itself. + */ + +export type ReviewTrackingState = "running" | "success" | "failure"; + +export type ReviewTrackingTelemetry = { + totalNumTurns?: number | null; + totalDurationMs?: number | null; + totalCostUsd?: number | null; + pass1SessionId?: string | null; + pass2SessionId?: string | null; +}; + +export interface ReviewTrackingFields { + state: ReviewTrackingState; + pipelineUrl?: string | null; + jobUrl?: string | null; + triggerUsername?: string | null; + errorDetails?: string | null; + securityReviewRan?: boolean; + telemetry?: ReviewTrackingTelemetry | null; +} diff --git a/src/core/review/triggers/parse-command.ts b/src/core/review/triggers/parse-command.ts new file mode 100644 index 0000000..725f4a5 --- /dev/null +++ b/src/core/review/triggers/parse-command.ts @@ -0,0 +1,74 @@ +/** + * Platform-agnostic Droid command parser. + * + * Both GitHub and GitLab look for the same `@droid ` mentions + * in PR/MR bodies and comments. This parser only operates on a raw + * string; the per-platform context extraction (which fields of which + * webhook payload to scan) stays in the platform-specific module + * because the payload shapes diverge. + */ + +export type DroidCommand = + | "fill" + | "review" + | "security" + | "security-full" + | "default"; + +export interface ParsedCommand { + command: DroidCommand; + raw: string; + location: "body" | "comment"; + timestamp?: string | null; +} + +/** + * Parses text to detect specific @droid commands. + * + * Returns `null` when no `@droid` mention is present, the generic + * `default` command for a bare `@droid`, or the specific subcommand + * when matched. `location` defaults to "body" and is overridden by + * the caller when the text came from a comment. + */ +export function parseDroidCommand(text: string): ParsedCommand | null { + if (!text) { + return null; + } + + const fillMatch = text.match(/@droid\s+fill/i); + if (fillMatch) { + return { command: "fill", raw: fillMatch[0], location: "body" }; + } + + // Note: `@droid review security` will match as just `@droid review`. + const reviewMatch = text.match(/@droid\s+review/i); + if (reviewMatch) { + return { command: "review", raw: reviewMatch[0], location: "body" }; + } + + const securityFullMatch = text.match(/@droid\s+security\s+--full/i); + if (securityFullMatch) { + return { + command: "security-full", + raw: securityFullMatch[0], + location: "body", + }; + } + + // Check after security-full to avoid false matches. + const securityMatch = text.match(/@droid\s+security(?:\s|$|[^-\w])/i); + if (securityMatch) { + return { + command: "security", + raw: securityMatch[0].trim(), + location: "body", + }; + } + + const droidMatch = text.match(/@droid/i); + if (droidMatch) { + return { command: "default", raw: droidMatch[0], location: "body" }; + } + + return null; +} diff --git a/src/create-prompt/templates/review-candidates-prompt.ts b/src/create-prompt/templates/review-candidates-prompt.ts index a596041..310d551 100644 --- a/src/create-prompt/templates/review-candidates-prompt.ts +++ b/src/create-prompt/templates/review-candidates-prompt.ts @@ -1,3 +1,14 @@ +/** + * GitHub Pass-1 (candidate generation) prompt — thin adapter. + * + * Delegates to the platform-agnostic builder in + * `src/core/review/prompts/candidates.ts`, mapping the GitHub + * `PreparedContext` shape onto the shared `ReviewPromptContext` and + * supplying `GITHUB_TERMINOLOGY` (PR labels, github_* MCP tool names). + */ + +import { generateCandidatesPrompt } from "../../core/review/prompts/candidates"; +import { GITHUB_TERMINOLOGY } from "../terminology"; import type { PreparedContext } from "../types"; export function generateReviewCandidatesPrompt( @@ -9,137 +20,25 @@ export function generateReviewCandidatesPrompt( ? String(context.githubContext.entityNumber) : "unknown"; - const repoFullName = context.repository; - const prHeadRef = context.prBranchData?.headRefName ?? "unknown"; - const prHeadSha = context.prBranchData?.headRefOid ?? "unknown"; - const prBaseRef = context.eventData.baseBranch ?? "unknown"; - - const diffPath = - context.reviewArtifacts?.diffPath ?? "$RUNNER_TEMP/droid-prompts/pr.diff"; - const commentsPath = - context.reviewArtifacts?.commentsPath ?? - "$RUNNER_TEMP/droid-prompts/existing_comments.json"; - const descriptionPath = - context.reviewArtifacts?.descriptionPath ?? - "$RUNNER_TEMP/droid-prompts/pr_description.txt"; - - const reviewCandidatesPath = - process.env.REVIEW_CANDIDATES_PATH ?? - "$RUNNER_TEMP/droid-prompts/review_candidates.json"; - - const includeSuggestions = context.includeSuggestions !== false; - - 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 securityReviewEnabled = process.env.SECURITY_REVIEW_ENABLED === "true"; - - 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 PR context (repo, PR number, head SHA, base ref) 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 PR #${prNumber} in ${repoFullName} and generate a JSON file with **high-confidence, actionable** review comments that pinpoint genuine issues. - -${skillInstruction}${securitySubagentInstruction} - - -Repo: ${repoFullName} -PR Number: ${prNumber} -PR Head Ref: ${prHeadRef} -PR Head SHA: ${prHeadSha} -PR Base Ref: ${prBaseRef} - -Precomputed data files: -- PR Description: \`${descriptionPath}\` -- Full PR Diff: \`${diffPath}\` -- Existing Comments: \`${commentsPath}\` - - - -Write output to \`${reviewCandidatesPath}\` using this exact schema: - -\`\`\`json -{ - "version": 1, - "meta": { - "repo": "owner/repo", - "prNumber": 123, - "headSha": "", - "baseRef": "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 - - \`repo\`: "${repoFullName}" - - \`prNumber\`: ${prNumber} - - \`headSha\`: "${prHeadSha}" - - \`baseRef\`: "${prBaseRef}" - - \`generatedAt\`: ISO 8601 timestamp (e.g., "2024-01-15T10:30:00Z") - -- **comments**: Array of comment objects - - \`path\`: Relative file path (e.g., "src/index.ts") -${bodyFieldDescription} - - \`line\`: Target line number (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\`: "${prHeadSha}" - -- **reviewSummary**: - - \`body\`: 1-3 sentence overall assessment - - - - -**DO NOT** post to GitHub. -**DO NOT** invoke any PR mutation tools (inline comments, submit review, delete/minimize/reply/resolve, etc.). -**DO NOT** modify any files other than writing to \`${reviewCandidatesPath}\`. -Output ONLY the JSON file—no additional commentary. - -`; + return generateCandidatesPrompt({ + terminology: GITHUB_TERMINOLOGY, + entityNumber: prNumber, + repoOrProject: context.repository, + headRef: context.prBranchData?.headRefName ?? "unknown", + headSha: context.prBranchData?.headRefOid ?? "unknown", + baseRef: context.eventData.baseBranch ?? "unknown", + diffPath: + context.reviewArtifacts?.diffPath ?? "$RUNNER_TEMP/droid-prompts/pr.diff", + commentsPath: + context.reviewArtifacts?.commentsPath ?? + "$RUNNER_TEMP/droid-prompts/existing_comments.json", + descriptionPath: + context.reviewArtifacts?.descriptionPath ?? + "$RUNNER_TEMP/droid-prompts/pr_description.txt", + candidatesPath: + process.env.REVIEW_CANDIDATES_PATH ?? + "$RUNNER_TEMP/droid-prompts/review_candidates.json", + includeSuggestions: context.includeSuggestions !== false, + securityReviewEnabled: process.env.SECURITY_REVIEW_ENABLED === "true", + }); } diff --git a/src/create-prompt/templates/review-validator-prompt.ts b/src/create-prompt/templates/review-validator-prompt.ts index 866ceb0..ded234f 100644 --- a/src/create-prompt/templates/review-validator-prompt.ts +++ b/src/create-prompt/templates/review-validator-prompt.ts @@ -1,3 +1,14 @@ +/** + * GitHub Pass-2 (validator) prompt — thin adapter. + * + * Delegates to the platform-agnostic builder in + * `src/core/review/prompts/validator.ts`, mapping the GitHub + * `PreparedContext` shape onto the shared `ReviewPromptContext` and + * supplying `GITHUB_TERMINOLOGY`. + */ + +import { generateValidatorPrompt } from "../../core/review/prompts/validator"; +import { GITHUB_TERMINOLOGY } from "../terminology"; import type { PreparedContext } from "../types"; export function generateReviewValidatorPrompt( @@ -9,124 +20,28 @@ export function generateReviewValidatorPrompt( ? String(context.githubContext.entityNumber) : "unknown"; - const repoFullName = context.repository; - const prHeadRef = context.prBranchData?.headRefName ?? "unknown"; - const prHeadSha = context.prBranchData?.headRefOid ?? "unknown"; - const prBaseRef = context.eventData.baseBranch ?? "unknown"; - - const diffPath = - context.reviewArtifacts?.diffPath ?? "$RUNNER_TEMP/droid-prompts/pr.diff"; - const commentsPath = - context.reviewArtifacts?.commentsPath ?? - "$RUNNER_TEMP/droid-prompts/existing_comments.json"; - const descriptionPath = - context.reviewArtifacts?.descriptionPath ?? - "$RUNNER_TEMP/droid-prompts/pr_description.txt"; - - const reviewCandidatesPath = - process.env.REVIEW_CANDIDATES_PATH ?? - "$RUNNER_TEMP/droid-prompts/review_candidates.json"; - const reviewValidatedPath = - process.env.REVIEW_VALIDATED_PATH ?? - "$RUNNER_TEMP/droid-prompts/review_validated.json"; - - const includeSuggestions = context.includeSuggestions !== false; - - 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 PR #${prNumber} in ${repoFullName}. - -IMPORTANT: This is Phase 2 (validator) of a two-pass review pipeline. - -${skillInstruction} - -### Context - -* Repo: ${repoFullName} -* PR Number: ${prNumber} -* PR Head Ref: ${prHeadRef} -* PR Head SHA: ${prHeadSha} -* PR Base Ref: ${prBaseRef} - -### Inputs - -Read these files before validating: -* PR Description: \`${descriptionPath}\` -* Candidates: \`${reviewCandidatesPath}\` -* Full PR 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 \`${reviewValidatedPath}\` - -\`\`\`json -{ - "version": 1, - "meta": { - "repo": "${repoFullName}", - "prNumber": ${prNumber}, - "headSha": "${prHeadSha}", - "baseRef": "${prBaseRef}", - "validatedAt": "" - }, - "results": [ - { - "status": "approved", - "comment": { - "path": "src/index.ts", - "body": "[P1] Title\\n\\n1 paragraph.", - "line": 42, - "startLine": null, - "side": "RIGHT", - "commit_id": "${prHeadSha}" - } - }, - { - "status": "rejected", - "candidate": { - "path": "src/other.ts", - "body": "[P2] ...", - "line": 10, - "startLine": null, - "side": "RIGHT", - "commit_id": "${prHeadSha}" - }, - "reason": "Not a real bug because ..." - } - ], - "reviewSummary": { - "status": "approved", - "body": "1-3 sentence overall assessment" - } -} -\`\`\` - -Notes: -* Use \`commit_id\` = \`${prHeadSha}\`. -* \`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 \`${reviewValidatedPath}\`, post comments ONLY for \`status === "approved"\`: - -* Collect all approved comments and submit them as a **single batched review** via \`github_pr___submit_review\`, passing them in the \`comments\` array parameter. -* Do **NOT** post comments individually — batch them all into one \`submit_review\` call. -* Do **NOT** include a \`body\` parameter in \`submit_review\`. -* Use \`github_comment___update_droid_comment\` to update the tracking comment with the review summary. -* Do **NOT** post the summary as a separate comment or as the body of \`submit_review\`. -* Do not approve or request changes. -`; + return generateValidatorPrompt({ + terminology: GITHUB_TERMINOLOGY, + entityNumber: prNumber, + repoOrProject: context.repository, + headRef: context.prBranchData?.headRefName ?? "unknown", + headSha: context.prBranchData?.headRefOid ?? "unknown", + baseRef: context.eventData.baseBranch ?? "unknown", + diffPath: + context.reviewArtifacts?.diffPath ?? "$RUNNER_TEMP/droid-prompts/pr.diff", + commentsPath: + context.reviewArtifacts?.commentsPath ?? + "$RUNNER_TEMP/droid-prompts/existing_comments.json", + descriptionPath: + context.reviewArtifacts?.descriptionPath ?? + "$RUNNER_TEMP/droid-prompts/pr_description.txt", + candidatesPath: + process.env.REVIEW_CANDIDATES_PATH ?? + "$RUNNER_TEMP/droid-prompts/review_candidates.json", + validatedPath: + process.env.REVIEW_VALIDATED_PATH ?? + "$RUNNER_TEMP/droid-prompts/review_validated.json", + includeSuggestions: context.includeSuggestions !== false, + securityReviewEnabled: process.env.SECURITY_REVIEW_ENABLED === "true", + }); } diff --git a/src/create-prompt/terminology.ts b/src/create-prompt/terminology.ts new file mode 100644 index 0000000..d3b2ef2 --- /dev/null +++ b/src/create-prompt/terminology.ts @@ -0,0 +1,33 @@ +import type { ReviewTerminology } from "../core/review/prompts/types"; + +export const GITHUB_TERMINOLOGY: ReviewTerminology = { + entityNoun: "PR", + entityNumberSigil: "#", + platformName: "GitHub", + repoLabel: "Repo", + entityNumberLabel: "PR Number", + headRefLabel: "PR Head Ref", + headShaLabel: "PR Head SHA", + baseRefLabel: "PR Base Ref", + baseRefShortLabel: "base ref", + descriptionLabel: "PR Description", + diffLabel: "Full PR Diff", + metaRepoKey: "repo", + metaEntityNumberKey: "prNumber", + metaBaseRefKey: "baseRef", + repoExample: "owner/repo", + pathFieldDescription: 'Relative file path (e.g., "src/index.ts")', + lineFieldDescription: + "Target line number (single-line) or end line number (multi-line). Must be ≥ 0.", + mutationToolForbiddance: + "(inline comments, submit review, delete/minimize/reply/resolve, etc.)", + submitReviewToolName: "github_pr___submit_review", + submitReviewExtraArg: "", + submitReviewBodyExclusionTrailer: "", + updateTrackingToolName: "github_comment___update_droid_comment", + trackingCommentName: "tracking comment", + summaryEntityName: "comment", + summaryPostingExtraExclusion: " or as the body of `submit_review`", + approvalChangesNote: "Do not approve or request changes.", + // No security-badge instruction on GitHub today; left undefined intentionally. +}; diff --git a/src/create-prompt/types.ts b/src/create-prompt/types.ts index f9945e0..8ec8238 100644 --- a/src/create-prompt/types.ts +++ b/src/create-prompt/types.ts @@ -1,4 +1,5 @@ import type { GitHubContext } from "../github/context"; +import type { ReviewArtifactPaths } from "../core/review/artifacts/types"; export type CommonFields = { repository: string; @@ -103,11 +104,7 @@ export type EventData = | PullRequestEvent | PullRequestTargetEvent; -export type ReviewArtifacts = { - diffPath: string; - commentsPath: string; - descriptionPath: string; -}; +export type ReviewArtifacts = ReviewArtifactPaths; export type PreparedContext = CommonFields & { eventData: EventData; 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 new file mode 100644 index 0000000..5fec141 --- /dev/null +++ b/src/entrypoints/gitlab-prepare.ts @@ -0,0 +1,322 @@ +#!/usr/bin/env bun + +/** + * Prepare step for the GitLab CI/CD Component (Pass 1 of the two-pass + * review flow). + * + * 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. 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"; +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"; +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; + projectId: string; + projectPath: string; + mrIid: number | null; + trackingNoteId: number | null; + diffBaseSha: string | null; + sourceBranchSha: string | null; + pipelineUrl: string | null; + jobUrl: string | null; + 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; +}; + +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" + ); +} + +function shellEscape(value: string): string { + return `'${value.replace(/'/g, `'\\''`)}'`; +} + +async function writeResolvedEnvShim( + model: string | null, + reasoningEffort: string | null, + extras: Record, +): 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 ?? "")}`, + ]; + 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}`); +} + +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, + 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)) { + 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; + } + + // 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; + + 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: securityReviewEnabled, + }); + + let trackingNoteId: number; + if (existing) { + await client.updateNote(projectId, mrIid, existing.id, noteBody); + trackingNoteId = existing.id; + console.log(`Reused existing tracking note ${trackingNoteId}`); + } else { + const created = await client.createNote(projectId, mrIid, noteBody); + trackingNoteId = created.id; + console.log(`Created tracking note ${trackingNoteId}`); + } + + 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, + 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, { + 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: 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 Pass-1 candidates prompt (${prompt.length} bytes) to ${promptPath}`, + ); + + await writeState({ + ...baseState, + shouldRunReview: true, + trackingNoteId, + 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, + }); +} + +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..a190552 --- /dev/null +++ b/src/entrypoints/gitlab-update-comment-link.ts @@ -0,0 +1,128 @@ +#!/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"; +import { collectExecTelemetry } from "../gitlab/data/exec-telemetry"; + +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 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, + jobUrl, + 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( + 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/github/utils/command-parser.ts b/src/github/utils/command-parser.ts index 5a9ca27..9903038 100644 --- a/src/github/utils/command-parser.ts +++ b/src/github/utils/command-parser.ts @@ -1,87 +1,19 @@ /** - * Command parser for detecting specific @droid commands in GitHub comments and PR bodies + * Command parser for detecting specific @droid commands in GitHub + * comments and PR bodies. The pure string-parsing portion is platform- + * agnostic and lives in `src/core/review/triggers/parse-command.ts`; + * this file owns the GitHub-payload-aware context extraction. */ import type { GitHubContext } from "../context"; +import { + parseDroidCommand, + type DroidCommand, + type ParsedCommand, +} from "../../core/review/triggers/parse-command"; -export type DroidCommand = - | "fill" - | "review" - | "security" - | "security-full" - | "default"; - -export interface ParsedCommand { - command: DroidCommand; - raw: string; - location: "body" | "comment"; - timestamp?: string | null; -} - -/** - * Parses text to detect specific @droid commands - * @param text The text to parse (comment body or PR description) - * @returns ParsedCommand if a command is found, null otherwise - */ -export function parseDroidCommand(text: string): ParsedCommand | null { - if (!text) { - return null; - } - - // Check for @droid fill command (case insensitive) - const fillMatch = text.match(/@droid\s+fill/i); - if (fillMatch) { - return { - command: "fill", - raw: fillMatch[0], - location: "body", // Will be set by caller - }; - } - - // Check for @droid review command (case insensitive) - // Note: @droid review security will match as just @droid review - const reviewMatch = text.match(/@droid\s+review/i); - if (reviewMatch) { - return { - command: "review", - raw: reviewMatch[0], - location: "body", // Will be set by caller - }; - } - - // Check for @droid security --full command (case insensitive) - const securityFullMatch = text.match(/@droid\s+security\s+--full/i); - if (securityFullMatch) { - return { - command: "security-full", - raw: securityFullMatch[0], - location: "body", // Will be set by caller - }; - } - - // Check for @droid security command (case insensitive) - // Must check after security-full to avoid false matches - const securityMatch = text.match(/@droid\s+security(?:\s|$|[^-\w])/i); - if (securityMatch) { - return { - command: "security", - raw: securityMatch[0].trim(), - location: "body", // Will be set by caller - }; - } - - // Check for generic @droid mention (default behavior) - const droidMatch = text.match(/@droid/i); - if (droidMatch) { - return { - command: "default", - raw: droidMatch[0], - location: "body", // Will be set by caller - }; - } - - return null; -} +export type { DroidCommand, ParsedCommand }; +export { parseDroidCommand }; /** * Extracts a droid command from the GitHub context @@ -91,12 +23,10 @@ export function parseDroidCommand(text: string): ParsedCommand | null { export function extractCommandFromContext( context: GitHubContext, ): ParsedCommand | null { - // Handle missing payload if (!context.payload) { return null; } - // Check PR body for commands (pull_request events) if ( context.eventName === "pull_request" && "pull_request" in context.payload @@ -110,7 +40,6 @@ export function extractCommandFromContext( } } - // Check issue body for commands (issues events) if (context.eventName === "issues" && "issue" in context.payload) { const body = context.payload.issue.body; if (body) { @@ -121,7 +50,6 @@ export function extractCommandFromContext( } } - // Check comment body for commands (issue_comment events) if (context.eventName === "issue_comment" && "comment" in context.payload) { const comment = context.payload.comment; if (comment.body) { @@ -136,7 +64,6 @@ export function extractCommandFromContext( } } - // Check review comment body (pull_request_review_comment events) if ( context.eventName === "pull_request_review_comment" && "comment" in context.payload @@ -154,7 +81,6 @@ export function extractCommandFromContext( } } - // Check review body (pull_request_review events) if ( context.eventName === "pull_request_review" && "review" in context.payload 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..1a1576f --- /dev/null +++ b/src/gitlab/context.ts @@ -0,0 +1,148 @@ +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; + settings: string; + }; +}; + +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, + ), + settings: process.env.DROID_SETTINGS ?? "", + }, + }; +} + +export function isMergeRequestContext( + ctx: ParsedGitlabContext, +): ctx is ParsedGitlabContext & { mr: NonNullable } { + return ctx.mr !== null; +} 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/data/review-artifacts.ts b/src/gitlab/data/review-artifacts.ts new file mode 100644 index 0000000..6dcd706 --- /dev/null +++ b/src/gitlab/data/review-artifacts.ts @@ -0,0 +1,84 @@ +/** + * 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. The disk + * write itself uses the shared `writeReviewArtifacts` helper in + * `src/core/review/artifacts/write.ts`. + */ + +import { writeReviewArtifacts } from "../../core/review/artifacts/write"; +import type { ReviewArtifactPaths } from "../../core/review/artifacts/types"; +import type { GitlabClient } from "../api/client"; +import type { GitlabMr, GitlabMrChanges, GitlabNote } from "../types"; + +export type GitlabReviewArtifactPaths = ReviewArtifactPaths; + +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; + + const [mr, changes, notes] = await Promise.all([ + client.getMr(projectId, mrIid), + client.getMrChanges(projectId, mrIid), + client.listNotes(projectId, mrIid), + ]); + + const paths = await writeReviewArtifacts( + outDir, + { + diff: buildDiffContent(changes), + comments: notes, + description: buildDescriptionContent(mr), + }, + { + diff: "mr.diff", + comments: "existing_comments.json", + description: "mr_description.txt", + }, + ); + + return { ...paths, mr, changes, notes }; +} diff --git a/src/gitlab/operations/tracking-note.ts b/src/gitlab/operations/tracking-note.ts new file mode 100644 index 0000000..f33ccae --- /dev/null +++ b/src/gitlab/operations/tracking-note.ts @@ -0,0 +1,105 @@ +/** + * 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. + * + * The state machine (running/success/failure), telemetry shape, and + * formatting helpers are platform-agnostic and live in + * `src/core/review/tracking/`. This file owns the GitLab-specific body + * rendering, including the markdown layout, the security badge, the + * error-details accordion, and the hidden marker conventions. + */ + +import { + formatCostUsd, + formatDurationMs, +} from "../../core/review/tracking/format"; +import type { + ReviewTrackingFields, + ReviewTrackingState, +} from "../../core/review/tracking/types"; + +export const DROID_TRACKING_MARKER = ""; +export const DROID_SECURITY_BADGE_MARKER = ""; + +export type TrackingNoteState = ReviewTrackingState; +export type TrackingNoteTelemetry = NonNullable< + ReviewTrackingFields["telemetry"] +>; +export type TrackingNoteOptions = ReviewTrackingFields; + +export 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("
"); + } + + 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"; +} + +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/src/gitlab/prompts/candidates.ts b/src/gitlab/prompts/candidates.ts new file mode 100644 index 0000000..413c393 --- /dev/null +++ b/src/gitlab/prompts/candidates.ts @@ -0,0 +1,35 @@ +/** + * GitLab Pass-1 (candidate generation) prompt — thin adapter. + * + * Delegates to the platform-agnostic builder in + * `src/core/review/prompts/candidates.ts` after mapping the GitLab + * context shape onto the shared `ReviewPromptContext` and supplying + * `GITLAB_TERMINOLOGY` (PR→MR labels, GitLab MCP tool names, etc.). + * + * The GitLab posting tool is exposed only in Pass 2; this Pass 1 prompt + * MUST NOT instruct the model to call it. The shared builder enforces + * that, plus tool gating at the `droid exec` level via `--enabled-tools`. + */ + +import { generateCandidatesPrompt } from "../../core/review/prompts/candidates"; +import { GITLAB_TERMINOLOGY } from "./terminology"; +import type { GitlabReviewPromptContext } from "./types"; + +export function generateGitlabReviewCandidatesPrompt( + ctx: GitlabReviewPromptContext, +): string { + return generateCandidatesPrompt({ + terminology: GITLAB_TERMINOLOGY, + entityNumber: ctx.mrIid, + repoOrProject: ctx.projectPath, + headRef: ctx.sourceBranch, + headSha: ctx.headSha, + baseRef: ctx.targetBranch, + diffPath: ctx.diffPath, + commentsPath: ctx.commentsPath, + descriptionPath: ctx.descriptionPath, + candidatesPath: ctx.candidatesPath, + includeSuggestions: ctx.includeSuggestions, + securityReviewEnabled: ctx.securityReviewEnabled, + }); +} diff --git a/src/gitlab/prompts/terminology.ts b/src/gitlab/prompts/terminology.ts new file mode 100644 index 0000000..c2616b3 --- /dev/null +++ b/src/gitlab/prompts/terminology.ts @@ -0,0 +1,49 @@ +import type { ReviewTerminology } from "../../core/review/prompts/types"; +import { SECURITY_BADGE } from "../operations/tracking-note"; + +export const GITLAB_TERMINOLOGY: ReviewTerminology = { + entityNoun: "MR", + entityNumberSigil: "!", + platformName: "GitLab", + repoLabel: "Project", + entityNumberLabel: "MR IID", + headRefLabel: "MR Source Branch", + headShaLabel: "MR Head SHA", + baseRefLabel: "MR Target Branch", + baseRefShortLabel: "target branch", + descriptionLabel: "MR Description", + diffLabel: "Full MR Diff", + metaRepoKey: "project", + metaEntityNumberKey: "mrIid", + metaBaseRefKey: "targetBranch", + repoExample: "group/project", + pathFieldDescription: + 'Relative file path (use the new_path from the diff, e.g., "src/index.ts")', + lineFieldDescription: + "Target line number in the new file (single-line) or end line number (multi-line). Must be ≥ 0.", + mutationToolForbiddance: + "(`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.)", + submitReviewToolName: "gitlab_mr___submit_review", + submitReviewExtraArg: "", // populated dynamically below via factory + submitReviewBodyExclusionTrailer: + " (we use a separate tracking note for the summary)", + updateTrackingToolName: "gitlab_mr___update_tracking_note", + trackingCommentName: "sticky tracking note", + summaryEntityName: "top-level note", + summaryPostingExtraExclusion: "", + approvalChangesNote: + "Do not approve the MR or request changes (GitLab approval rules are handled out-of-band).", + securityBadgeInstruction: `If any approved comments contain \`[security]\` in their body, ensure a security badge is prepended to the tracking note body using exactly this markdown image (no surrounding backticks or code fences): ${SECURITY_BADGE.trim()} — this indicates that security analysis was performed as part of the review.`, +}; + +/** + * Build the GitLab terminology with the runtime MR IID baked into the + * "passing them in the comments array parameter along with mr_iid: N" arg. + * GitLab's submit_review tool needs the IID re-asserted in the call. + */ +export function gitlabTerminologyFor(mrIid: number): ReviewTerminology { + return { + ...GITLAB_TERMINOLOGY, + submitReviewExtraArg: ` along with \`mr_iid: ${mrIid}\``, + }; +} 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..602a9a6 --- /dev/null +++ b/src/gitlab/prompts/validator.ts @@ -0,0 +1,37 @@ +/** + * GitLab Pass-2 (validator) prompt — thin adapter. + * + * Delegates to the platform-agnostic builder in + * `src/core/review/prompts/validator.ts` after mapping the GitLab + * context shape onto the shared `ReviewPromptContext` and supplying + * `gitlabTerminologyFor(mrIid)` (which bakes the MR IID into the + * submit_review call hint, since GitLab's tool requires `mr_iid` to be + * re-asserted in the invocation). + * + * Pass 2 is the only place the GitLab posting tool is exposed (via + * `--enabled-tools` on the second `droid exec` invocation). + */ + +import { generateValidatorPrompt } from "../../core/review/prompts/validator"; +import { gitlabTerminologyFor } from "./terminology"; +import type { GitlabReviewPromptContext } from "./types"; + +export function generateGitlabReviewValidatorPrompt( + ctx: GitlabReviewPromptContext, +): string { + return generateValidatorPrompt({ + terminology: gitlabTerminologyFor(ctx.mrIid), + entityNumber: ctx.mrIid, + repoOrProject: ctx.projectPath, + headRef: ctx.sourceBranch, + headSha: ctx.headSha, + baseRef: ctx.targetBranch, + diffPath: ctx.diffPath, + commentsPath: ctx.commentsPath, + descriptionPath: ctx.descriptionPath, + candidatesPath: ctx.candidatesPath, + validatedPath: ctx.validatedPath, + includeSuggestions: ctx.includeSuggestions, + securityReviewEnabled: ctx.securityReviewEnabled, + }); +} diff --git a/src/gitlab/token.ts b/src/gitlab/token.ts new file mode 100644 index 0000000..0d73ed6 --- /dev/null +++ b/src/gitlab/token.ts @@ -0,0 +1,20 @@ +#!/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; + + 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/src/mcp/gitlab-mr-server.ts b/src/mcp/gitlab-mr-server.ts new file mode 100644 index 0000000..292eaf3 --- /dev/null +++ b/src/mcp/gitlab-mr-server.ts @@ -0,0 +1,359 @@ +#!/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. 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; + +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_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", + { + 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) + .optional() + .describe("Inline review comments"), + }, + 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/templates/README.md b/templates/README.md new file mode 100644 index 0000000..790ebf1 --- /dev/null +++ b/templates/README.md @@ -0,0 +1,40 @@ +# 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 + +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: + - project: "factory-components/droid-action" + ref: main + file: "/templates/droid-review.yml" +``` + +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: + - 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/templates/droid-review.yml b/templates/droid-review.yml new file mode 100644 index 0000000..26f7b98 --- /dev/null +++ b/templates/droid-review.yml @@ -0,0 +1,213 @@ +# 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: +# - 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. + +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: "" + 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" + 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" + 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. + 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. 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 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. 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" +--- +droid-review: + stage: $[[ inputs.stage ]] + image: oven/bun:1.2.11 + rules: + - 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 ]]" + AUTOMATIC_REVIEW: "$[[ inputs.automatic_review ]]" + REVIEW_DEPTH: "$[[ inputs.review_depth ]]" + REVIEW_MODEL: "$[[ inputs.review_model ]]" + 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: + - 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" + - 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" + - | + if ! bun run "$DROID_ACTION_DIR/src/entrypoints/gitlab-prepare.ts"; then + export DROID_SUCCESS=false + 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 + - | + # 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 DROID_MR_IID="$DROID_MR_IID" \ + --env DROID_TRACKING_NOTE_ID="$DROID_TRACKING_NOTE_ID" + - | + # ---- 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 2>&1 | tee "$PASS1_LOG"; then + export DROID_SUCCESS=false + echo "droid exec pass 1 (candidates) failed" > /tmp/droid-error.txt + exit 1 + fi + - | + # ---- 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. + 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 2>&1 | tee "$PASS2_LOG"; then + export DROID_SUCCESS=false + echo "droid exec pass 2 (validator) failed" > /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 + - .droid-debug/ + expire_in: 1 week diff --git a/test/core/review/tracking/format.test.ts b/test/core/review/tracking/format.test.ts new file mode 100644 index 0000000..faf9aea --- /dev/null +++ b/test/core/review/tracking/format.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from "bun:test"; +import { + formatCostUsd, + formatDurationMs, +} from "../../../../src/core/review/tracking/format"; + +describe("formatDurationMs", () => { + it("renders sub-second durations in ms", () => { + expect(formatDurationMs(0)).toBe("0ms"); + expect(formatDurationMs(750)).toBe("750ms"); + }); + + it("renders sub-minute durations to one decimal in seconds", () => { + expect(formatDurationMs(1500)).toBe("1.5s"); + expect(formatDurationMs(59999)).toBe("60.0s"); + }); + + it("renders multi-minute durations as `Xm Ys`", () => { + expect(formatDurationMs(60000)).toBe("1m 0s"); + expect(formatDurationMs(90000)).toBe("1m 30s"); + expect(formatDurationMs(125000)).toBe("2m 5s"); + }); + + it("carries the remainder cleanly when seconds round up to 60", () => { + // Pre-fix bug: 119600ms produced "1m 60s" because remSec rounded to 60. + expect(formatDurationMs(119600)).toBe("2m 0s"); + expect(formatDurationMs(179600)).toBe("3m 0s"); + expect(formatDurationMs(239600)).toBe("4m 0s"); + }); +}); + +describe("formatCostUsd", () => { + it("uses 4 decimals under $1", () => { + expect(formatCostUsd(0.0042)).toBe("$0.0042"); + expect(formatCostUsd(0.5)).toBe("$0.5000"); + }); + + it("uses 2 decimals at $1 or above", () => { + expect(formatCostUsd(1)).toBe("$1.00"); + expect(formatCostUsd(12.345)).toBe("$12.35"); + }); +}); 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..12fbb49 --- /dev/null +++ b/test/gitlab/context.test.ts @@ -0,0 +1,188 @@ +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", + "DROID_SETTINGS", +]; + +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); + }); + + 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(""); + }); +}); 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/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-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"); + }); +}); diff --git a/test/gitlab/token.test.ts b/test/gitlab/token.test.ts new file mode 100644 index 0000000..734fdb3 --- /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("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()).toThrow(MissingGitlabTokenError); + }); + + it("throws MissingGitlabTokenError when nothing is set", () => { + expect(() => setupGitlabToken()).toThrow(MissingGitlabTokenError); + }); +}); diff --git a/test/gitlab/tracking-note.test.ts b/test/gitlab/tracking-note.test.ts new file mode 100644 index 0000000..409ad18 --- /dev/null +++ b/test/gitlab/tracking-note.test.ts @@ -0,0 +1,112 @@ +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("
"); + }); + + 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", () => { + 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 new file mode 100644 index 0000000..879decd --- /dev/null +++ b/test/mcp/gitlab-mr-server.test.ts @@ -0,0 +1,156 @@ +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 call = createDisc.mock.calls[0]!; + const position = call[3]; + 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 call = createDisc.mock.calls[0]!; + const position = call[3]; + 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", + ); + }); +});