diff --git a/README.ja-JP.md b/README.ja-JP.md index d2cc1a54..fd4dafd4 100644 --- a/README.ja-JP.md +++ b/README.ja-JP.md @@ -331,6 +331,7 @@ ocr review \ - [`github_actions/`](./examples/github_actions/) — GitHub Actions統合の例 - [`gitlab_ci/`](./examples/gitlab_ci/) — GitLab CI統合の例 +- [`gitflic_ci/`](./examples/gitflic_ci/) — GitFlic CI統合の例 ## コマンド diff --git a/README.ko-KR.md b/README.ko-KR.md index 28e8323d..99535e85 100644 --- a/README.ko-KR.md +++ b/README.ko-KR.md @@ -331,6 +331,7 @@ ocr review \ - [`github_actions/`](./examples/github_actions/): GitHub Actions 통합 예시 - [`gitlab_ci/`](./examples/gitlab_ci/): GitLab CI 통합 예시 +- [`gitflic_ci/`](./examples/gitflic_ci/): GitFlic CI 통합 예시 ## Commands diff --git a/README.md b/README.md index 574def5b..9ac7c2b4 100644 --- a/README.md +++ b/README.md @@ -333,6 +333,7 @@ See the [`examples/`](./examples/) directory for integration examples: - [`github_actions/`](./examples/github_actions/) — GitHub Actions integration example - [`gitlab_ci/`](./examples/gitlab_ci/) — GitLab CI integration example +- [`gitflic_ci/`](./examples/gitflic_ci/) — GitFlic CI integration example ## Commands diff --git a/README.ru-RU.md b/README.ru-RU.md index 2c27f0ea..54a8d1ff 100644 --- a/README.ru-RU.md +++ b/README.ru-RU.md @@ -333,6 +333,7 @@ ocr review \ - [`github_actions/`](./examples/github_actions/) — пример интеграции с GitHub Actions - [`gitlab_ci/`](./examples/gitlab_ci/) — пример интеграции с GitLab CI +- [`gitflic_ci/`](./examples/gitflic_ci/) — пример интеграции с GitFlic CI ## Команды diff --git a/README.zh-CN.md b/README.zh-CN.md index 6a430ef8..1b30f472 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -331,6 +331,7 @@ ocr review \ - [`github_actions/`](./examples/github_actions/) — GitHub Actions 集成示例 - [`gitlab_ci/`](./examples/gitlab_ci/) — GitLab CI 集成示例 +- [`gitflic_ci/`](./examples/gitflic_ci/) — GitFlic CI 集成示例 ## 命令 diff --git a/examples/README.md b/examples/README.md index 8a4caea1..2ffc3ffe 100644 --- a/examples/README.md +++ b/examples/README.md @@ -6,5 +6,6 @@ This directory contains examples for integrating OpenCodeReview (OCR) into vario - **[github_actions/](./github_actions/)** - GitHub Actions integration example - **[gitlab_ci/](./gitlab_ci/)** - GitLab CI integration example +- **[gitflic_ci/](./gitflic_ci/)** - GitFlic CI integration example -Each subdirectory contains its own README with detailed setup instructions. \ No newline at end of file +Each subdirectory contains its own README with detailed setup instructions. diff --git a/examples/gitflic_ci/README.md b/examples/gitflic_ci/README.md new file mode 100644 index 00000000..9f36ce73 --- /dev/null +++ b/examples/gitflic_ci/README.md @@ -0,0 +1,80 @@ +# OpenCodeReview - GitFlic CI Demo + +This demo shows how to integrate OpenCodeReview into a [GitFlic](https://gitflic.ru) CI/CD pipeline to automatically review Merge Requests and post the findings as MR discussions — inline on the changed lines where possible. + +Like the GitHub Actions and GitLab CI examples, the posting glue lives in the CI layer rather than in the `ocr` binary. Here it is a small, dependency-free Python script — [`post_review.py`](post_review.py) — that reads `ocr review --format json` and posts to the GitFlic Discussions API. The only GitFlic-specific wrinkle it handles is the **old-side line**: GitFlic requires it even for a comment on the new side of the diff, and `ocr review` reports new-side positions only, so the script recomputes it from the same merge-base diff the review ran on. + +## How It Works + +``` +MR Created/Updated → Merge Request Pipeline → ocr review → post_review.py → Discussions on MR +``` + +1. A Merge Request Pipeline triggers the `code-review` job +2. It installs OCR via npm in a `node:20` image (which also ships `python3` and `git`) +3. Runs `ocr review --from origin/ --to $CI_COMMIT_SHA --format json --audience agent` +4. Runs `python3 post_review.py`, which reads the JSON and posts: + - **Inline discussions** on the changed lines (`POST .../discussions/create` with `newLine`/`oldLine`/`newPath`/`oldPath`) + - **A fallback note** collecting comments that could not be placed inline + - **A summary note** with the totals + +The MR context (owner, project, MR id, branch refs) is picked up automatically from the predefined GitFlic CI variables (`CI_PROJECT_NAMESPACE`, `CI_PROJECT_NAME`, `CI_MERGE_REQUEST_LOCAL_ID`, `CI_MERGE_REQUEST_TARGET_BRANCH_NAME`, `CI_COMMIT_SHA`), so `post_review.py` needs no arguments in CI. Outside CI every value can be passed via flags — run `python3 post_review.py -h`. + +## Setup + +### 1. Enable Merge Request Pipelines + +Go to **Project Settings → CI/CD Settings** and enable **Merge Request Pipeline**. New merge requests will then trigger the pipeline automatically. + +### 2. Copy the pipeline files + +Copy **both** `gitflic-ci.yaml` (GitFlic expects this exact file name at the repository root) and `post_review.py` into your repository. If you keep `post_review.py` somewhere other than the repo root, adjust the `python3 post_review.py` path in `gitflic-ci.yaml` accordingly. + +### 3. Configure CI/CD Variables + +Go to **Settings → CI/CD → Variables** and add: + +| Variable | Required | Description | +|----------|----------|-------------| +| `OCR_LLM_URL` | Yes | LLM API endpoint URL | +| `OCR_LLM_AUTH_TOKEN` | Yes | LLM API authentication token | +| `GITFLIC_TOKEN` | Yes | GitFlic access token used to post discussions | +| `OCR_LLM_MODEL` | No | Model name (e.g., `gpt-4o`) | +| `GITFLIC_API_URL` | No | REST API base URL for self-hosted GitFlic (default: `https://api.gitflic.ru`) | + +> **Note:** GitFlic CI/CD does not accept variables with values shorter than 8 characters, so `use_anthropic` cannot be set as a CI variable. The pipeline sets it to `false`; to use Anthropic Claude models, edit `gitflic-ci.yaml` directly. + +### 4. Create a GitFlic Access Token + +Create a token in **User Settings → Access Tokens** (or a dedicated service account — its name becomes the bot name shown in discussions) and store it in the `GITFLIC_TOKEN` variable. The token owner must have access to the project sufficient for commenting on merge requests. + +## Notes & Limitations + +- **Inline positioning** — GitFlic requires all four of `newLine`/`oldLine`/`newPath`/`oldPath` for a code comment; if any is missing it silently creates a general comment. `post_review.py` computes the old-side position from the same merge-base diff the review ran on (`git diff merge-base(from, to)..to`), and anchors added lines to the closest preceding old line. +- **Rate limit** — the GitFlic cloud API allows 500 requests/hour per token. One review posts `comments + 2` requests at most, which fits comfortably. +- **Self-hosted GitFlic** — set `GITFLIC_API_URL` to your instance's REST API base URL. +- **Re-reviews** — every push to the MR triggers a new pipeline and a new review. To skip already-reviewed MRs, check existing discussions for the `OpenCodeReview` marker before running the review step. + +## Tests + +`post_review.py` ships with [`post_review_test.py`](post_review_test.py) — standard-library `unittest`, no network or git required: + +```bash +cd examples/gitflic_ci +python3 post_review_test.py +``` + +The line-mapping cases are ported from the upstream Go tests so the script keeps proven parity with the binary. + +## Debugging + +Test the posting step locally without touching the MR: + +```bash +ocr review --from origin/main --to HEAD --format json > /tmp/r.json +python3 post_review.py /tmp/r.json \ + --owner --project --mr \ + --from origin/main --to HEAD --dry-run +``` + +`--dry-run` prints every discussion (with the computed positions) instead of posting, and does not require `GITFLIC_TOKEN`. diff --git a/examples/gitflic_ci/gitflic-ci.yaml b/examples/gitflic_ci/gitflic-ci.yaml new file mode 100644 index 00000000..2a2b9bd1 --- /dev/null +++ b/examples/gitflic_ci/gitflic-ci.yaml @@ -0,0 +1,84 @@ +# OpenCodeReview - GitFlic CI Merge Request Auto-Review Demo +# +# Reviews Merge Requests with OpenCodeReview and posts the findings onto the +# MR as discussions (inline where possible). The posting glue lives in the CI +# layer, in post_review.py next to this file -- consistent with the GitHub and +# GitLab examples, which keep platform-specific publishing out of the `ocr` +# binary. +# +# Setup: +# - Commit BOTH this file and post_review.py into your repository (adjust the +# `python3 post_review.py` path below if you place the script elsewhere). +# - Enable "Merge Request Pipeline" in Project Settings -> CI/CD Settings. +# - Use a runner able to run the node:20 image (it ships node, python3 and git), +# or any shell runner with node 20+, python3 and git available. +# +# Required CI/CD Variables (Settings -> CI/CD -> Variables): +# OCR_LLM_URL - LLM API endpoint (e.g., https://api.openai.com/v1/chat/completions) +# OCR_LLM_AUTH_TOKEN - Authentication token for the LLM API +# GITFLIC_TOKEN - GitFlic access token used to post MR discussions +# +# Optional CI/CD Variables: +# OCR_LLM_MODEL - Model name (e.g., gpt-4o) +# GITFLIC_API_URL - GitFlic REST API base URL; only needed for self-hosted +# instances (defaults to https://api.gitflic.ru) +# +# post_review.py picks up the MR context automatically from the predefined +# GitFlic CI variables: CI_PROJECT_NAMESPACE, CI_PROJECT_NAME, +# CI_MERGE_REQUEST_LOCAL_ID, CI_MERGE_REQUEST_TARGET_BRANCH_NAME, CI_COMMIT_SHA. + +stages: + - review + +code-review: + stage: review + image: node:20 + script: + # Run only in merge request pipelines + - | + if [ -z "$CI_MERGE_REQUEST_LOCAL_ID" ]; then + echo "Not a merge request pipeline, skipping review." + exit 0 + fi + + # Install OpenCodeReview + - npm install -g @alibaba-group/open-code-review + + # Configure OCR + - | + ocr config set llm.url $OCR_LLM_URL + ocr config set llm.auth_token $OCR_LLM_AUTH_TOKEN + if [ -n "$OCR_LLM_MODEL" ]; then + ocr config set llm.model "$OCR_LLM_MODEL" + fi + ocr config set llm.use_anthropic false + ocr config set llm.extra_body '{"thinking": {"type": "disabled"}}' + + # Make sure the target branch and full history are available for merge-base diff + - git fetch --unshallow 2>/dev/null || true + - git fetch origin "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" + + # Run OCR review (CI_COMMIT_SHA as head supports forked MRs as well) + - | + ocr review \ + --from "origin/${CI_MERGE_REQUEST_TARGET_BRANCH_NAME}" \ + --to "${CI_COMMIT_SHA}" \ + --format json \ + --audience agent \ + > /tmp/ocr-result.json || true + echo "OCR review completed." + + # Post review comments onto the MR (inline discussions + summary note). + # post_review.py recomputes the old-side line for each comment from the + # merge-base diff, which the GitFlic Discussions API requires for inline + # (code) comments. + # + # The review step above ends with `|| true`, so a failed `ocr review` (bad + # token, network error) leaves an empty or partial file. Skip posting in + # that case instead of feeding invalid JSON to post_review.py. + - | + if [ ! -s /tmp/ocr-result.json ]; then + echo "OCR review produced no output, skipping post." + exit 0 + fi + python3 post_review.py /tmp/ocr-result.json diff --git a/examples/gitflic_ci/post_review.py b/examples/gitflic_ci/post_review.py new file mode 100644 index 00000000..e8b05556 --- /dev/null +++ b/examples/gitflic_ci/post_review.py @@ -0,0 +1,480 @@ +#!/usr/bin/env python3 +"""Post an OpenCodeReview result onto a GitFlic merge request. + +This is the CI-layer "glue" for GitFlic, mirroring examples/gitlab_ci: it keeps +platform-specific publishing out of the `ocr` binary and lives entirely in the +pipeline. It reads the JSON emitted by `ocr review --format json` and posts it +onto the merge request as discussions: + + - one inline discussion per comment that maps onto the diff, + - a single fallback note collecting the comments that do not, + - a final summary note. + +GitFlic's Discussions API needs an *old-side* line even for a comment on the new +side of the diff: an inline (code) discussion requires all four of +newLine/oldLine/newPath/oldPath, otherwise GitFlic silently records a plain +comment. `ocr review` only reports new-side positions, so this script computes +the old-side line itself by parsing the same merge-base diff the review ran on +(`git diff merge-base(from, to)..to`). + +Standard library only (json, urllib, subprocess) so it runs on the stock +node:20 / python image used by the pipeline. +""" + +import argparse +import json +import os +import re +import subprocess +import sys +import urllib.error +import urllib.request +from urllib.parse import quote + +# GitFlic SaaS REST API endpoint; override with --api-url / $GITFLIC_API_URL for +# self-hosted instances (e.g. http://gitflic.example/rest-api). +DEFAULT_API_URL = "https://api.gitflic.ru" + +# Context lines around each hunk; must match what `ocr review` diffs with so the +# new-side line numbers in the comments align with the hunks parsed here. +DIFF_CONTEXT_LINES = 3 + + +def log(msg): + print(msg, file=sys.stderr) + + +# --------------------------------------------------------------------------- # +# Diff parsing +# --------------------------------------------------------------------------- # + +HUNK_CONTEXT, HUNK_ADDED, HUNK_DELETED = range(3) + +_HUNK_HEADER_RE = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@") +_DIFF_HEADER_RE = re.compile(r"^diff --git a/(.+?) b/(.+)$") + + +class Hunk: + """One @@ ... @@ block of a unified diff.""" + + __slots__ = ("old_start", "old_count", "new_start", "new_count", "lines") + + def __init__(self, old_start, old_count, new_start, new_count): + self.old_start = old_start + self.old_count = old_count + self.new_start = new_start + self.new_count = new_count + self.lines = [] # list of (type, content) + + +class FileDiff: + """A single file's section of a unified diff.""" + + __slots__ = ("old_path", "new_path", "is_new", "is_deleted", "is_binary", "text") + + def __init__(self, old_path="", new_path=""): + self.old_path = old_path + self.new_path = new_path + self.is_new = False + self.is_deleted = False + self.is_binary = False + self.text = "" # raw diff body, fed to parse_hunks() on demand + + +def parse_hunks(raw): + """Parse one file's unified diff text into a list of Hunks. + + Lines before the first @@ header (diff --git, ---, +++) are ignored. + """ + hunks = [] + current = None + for line in raw.split("\n"): + m = _HUNK_HEADER_RE.match(line) + if m: + if current is not None: + hunks.append(current) + old_start = int(m.group(1)) + old_count = int(m.group(2)) if m.group(2) else 1 + new_start = int(m.group(3)) + new_count = int(m.group(4)) if m.group(4) else 1 + current = Hunk(old_start, old_count, new_start, new_count) + continue + if current is None: + continue + if line.startswith("\\ No newline at end of file"): + continue + if line.startswith("diff --git "): + break + if line.startswith("+"): + current.lines.append((HUNK_ADDED, line[1:])) + elif line.startswith("-"): + current.lines.append((HUNK_DELETED, line[1:])) + else: + content = line[1:] if line[:1] == " " else line + current.lines.append((HUNK_CONTEXT, content)) + if current is not None: + hunks.append(current) + return hunks + + +def parse_diff(diff_text): + """Split combined unified diff text into per-file FileDiff sections.""" + files = [] + current = None + buf = [] + + def flush(): + if current is not None: + current.text = "\n".join(buf) + files.append(current) + + for line in diff_text.split("\n"): + m = _DIFF_HEADER_RE.match(line) + if m: + flush() + buf = [] + current = FileDiff(old_path=m.group(1), new_path=m.group(2)) + if current is None: + continue + if line.startswith("Binary files ") or line.startswith("GIT binary patch"): + current.is_binary = True + elif line.startswith("new file mode"): + current.is_new = True + elif line.startswith("deleted file mode"): + current.is_deleted = True + elif line.startswith("--- "): + path = line[4:] + if path == "/dev/null": + current.is_new = True + current.old_path = "/dev/null" + elif path.startswith("a/"): + current.old_path = path[2:] + elif line.startswith("+++ "): + path = line[4:] + if path == "/dev/null": + current.is_deleted = True + current.new_path = "/dev/null" + elif path.startswith("b/"): + current.new_path = path[2:] + buf.append(line) + + flush() + return files + + +# --------------------------------------------------------------------------- # +# Line mapping (new file side -> old file side) +# --------------------------------------------------------------------------- # + + +def clamp_line(n): + return 1 if n < 1 else n + + +def old_line_for(hunks, new_line): + """Map a new-side line number to the corresponding old-side line. + + Lines added by the diff have no old counterpart, so they are anchored to the + closest preceding old line -- GitFlic only needs a plausible old-side + position to render the code comment next to the insertion point. The result + is always >= 1. + """ + delta = 0 # cumulative (new - old) line-count shift from preceding hunks + for h in hunks: + if new_line < h.new_start: + break + if new_line < h.new_start + h.new_count: + return _old_line_in_hunk(h, new_line) + delta += h.new_count - h.old_count + return clamp_line(new_line - delta) + + +def _old_line_in_hunk(h, new_line): + """Walk a hunk's lines tracking both counters until reaching new_line.""" + old_ln, new_ln = h.old_start, h.new_start + last_old = h.old_start - 1 # last old line seen before the current position + for line_type, _content in h.lines: + if line_type == HUNK_CONTEXT: + if new_ln == new_line: + return clamp_line(old_ln) + last_old = old_ln + old_ln += 1 + new_ln += 1 + elif line_type == HUNK_DELETED: + last_old = old_ln + old_ln += 1 + elif line_type == HUNK_ADDED: + if new_ln == new_line: + return clamp_line(last_old) + new_ln += 1 + return clamp_line(last_old) + + +# --------------------------------------------------------------------------- # +# Comment formatting +# --------------------------------------------------------------------------- # + + +def format_comment(c): + """Render an inline discussion body.""" + body = c.get("content", "") + suggestion = c.get("suggestion_code", "") + existing = c.get("existing_code", "") + if suggestion and existing: + body += "\n\n**Suggestion:**\n```\n" + suggestion + "\n```" + return body + + +def format_comment_fallback(c): + """Render a comment for the fallback (non-inline) note.""" + md = "### 📄 `%s`" % c.get("path", "") + start_line = c.get("start_line", 0) + end_line = c.get("end_line", 0) + if start_line and end_line: + md += " (L%d-L%d)" % (start_line, end_line) + md += "\n\n" + c.get("content", "") + suggestion = c.get("suggestion_code", "") + existing = c.get("existing_code", "") + if suggestion and existing: + md += "\n\n**Before:**\n```\n" + existing + "\n```\n\n**After:**\n```\n" + suggestion + "\n```" + return md + + +# --------------------------------------------------------------------------- # +# Publishing (transport-agnostic; `post` does the actual API call) +# --------------------------------------------------------------------------- # + + +def publish(result, diffs_by_path, post): + """Post the review result via the `post(discussion)` callable. + + `post` receives a discussion dict and must raise on failure. A general + comment carries only "message"; an inline comment also carries + newLine/oldLine/newPath/oldPath. Returns {"inline": int, "fallback": int}. + """ + comments = result.get("comments") or [] + if not comments: + message = result.get("message") or "No comments generated. Looks good to me." + post({"message": "✅ **OpenCodeReview**: " + message}) + return {"inline": 0, "fallback": 0} + + inline = 0 + failed = [] + hunks_cache = {} + + for c in comments: + path = c.get("path", "") + end_line = c.get("end_line", 0) or 0 + fd = diffs_by_path.get(path) + if fd is None: + log("no diff for %s; folding comment into the summary note" % path) + failed.append(c) + continue + if fd.is_binary or fd.is_deleted or end_line <= 0: + failed.append(c) + continue + + old_path = fd.old_path + old_line = 1 + if fd.is_new or old_path == "" or old_path == "/dev/null": + # GitFlic has no old side for a new file; anchor to the new path. + old_path = fd.new_path + else: + hunks = hunks_cache.get(path) + if hunks is None: + hunks = parse_hunks(fd.text) + hunks_cache[path] = hunks + old_line = old_line_for(hunks, end_line) + + discussion = { + "message": format_comment(c), + "newLine": end_line, + "oldLine": old_line, + "newPath": path, + "oldPath": old_path, + } + try: + post(discussion) + except Exception as e: # noqa: BLE001 - any transport error falls back + log("inline comment failed for %s:%d: %s" % (path, end_line, e)) + failed.append(c) + continue + inline += 1 + + if failed: + note = "🔍 **OpenCodeReview** found issues that could not be posted inline:\n\n---\n\n" + for c in failed: + note += format_comment_fallback(c) + "\n\n---\n\n" + post({"message": note}) + + summary = "🔍 **OpenCodeReview** found **%d** issue(s) in this MR." % len(comments) + summary += "\n- ✅ %d posted as inline comment(s)" % inline + summary += "\n- 📝 %d posted as summary (could not be placed inline)" % len(failed) + warnings = result.get("warnings") or [] + if warnings: + summary += "\n\n⚠️ %d warning(s) occurred during review." % len(warnings) + post({"message": summary}) + + return {"inline": inline, "fallback": len(failed)} + + +# --------------------------------------------------------------------------- # +# GitFlic REST transport +# --------------------------------------------------------------------------- # + + +def make_poster(api_url, token, owner, project, mr): + """Return a post(discussion) that POSTs to the GitFlic Discussions API.""" + endpoint = "%s/project/%s/%s/merge-request/%s/discussions/create" % ( + api_url.rstrip("/"), + quote(owner, safe=""), + quote(project, safe=""), + quote(mr, safe=""), + ) + + def post(discussion): + body = json.dumps(discussion).encode("utf-8") + req = urllib.request.Request(endpoint, data=body, method="POST") + req.add_header("Authorization", "token " + token) + req.add_header("Content-Type", "application/json") + try: + with urllib.request.urlopen(req) as resp: + resp.read() + except urllib.error.HTTPError as e: + snippet = e.read(512).decode("utf-8", "replace").strip() + # Some APIs echo request details back in error bodies; never let the + # token reach the CI log if GitFlic does that. + if token: + snippet = snippet.replace(token, "***") + raise RuntimeError("gitflic API %s %s: %s" % (e.code, e.reason, snippet)) + + return post + + +def make_dry_run_poster(): + """Return a post(discussion) that prints instead of calling the API.""" + + def post(discussion): + if discussion.get("newPath") and "newLine" in discussion and "oldLine" in discussion: + position = "%s:%d (old %s:%d)" % ( + discussion["newPath"], + discussion["newLine"], + discussion.get("oldPath", ""), + discussion["oldLine"], + ) + else: + position = "general" + print("--- dry-run discussion [%s] ---\n%s\n" % (position, discussion["message"])) + + return post + + +# --------------------------------------------------------------------------- # +# git / IO +# --------------------------------------------------------------------------- # + + +def _git(repo, *args): + return subprocess.run( + ["git", *args], cwd=repo, check=True, capture_output=True, text=True + ).stdout + + +def load_diffs_by_path(repo, from_ref, to_ref): + """Build {new_path: FileDiff} for the merge-base diff `ocr review` ran on.""" + base = _git(repo, "merge-base", from_ref, to_ref).strip() + out = _git( + repo, "diff", "--no-ext-diff", "--no-textconv", + "--src-prefix=a/", "--dst-prefix=b/", "--no-color", + "-U%d" % DIFF_CONTEXT_LINES, base, to_ref, "--", + ) + return {fd.new_path: fd for fd in parse_diff(out)} + + +def load_review_result(path): + """Read the JSON produced by `ocr review --format json` (path '-' = stdin).""" + if path == "-": + data = sys.stdin.read() + else: + with open(path, encoding="utf-8") as f: + data = f.read() + return json.loads(data) + + +# --------------------------------------------------------------------------- # +# CLI +# --------------------------------------------------------------------------- # + + +def parse_args(argv): + target = os.environ.get("CI_MERGE_REQUEST_TARGET_BRANCH_NAME", "") + default_from = "origin/" + target if target else "" + + p = argparse.ArgumentParser( + description="Post `ocr review --format json` output onto a GitFlic merge request." + ) + p.add_argument("file", nargs="?", default="-", + help="review result JSON ('-' = stdin, default)") + p.add_argument("--owner", default=os.environ.get("CI_PROJECT_NAMESPACE", ""), + help="project owner alias (default: $CI_PROJECT_NAMESPACE)") + p.add_argument("--project", default=os.environ.get("CI_PROJECT_NAME", ""), + help="project alias (default: $CI_PROJECT_NAME)") + p.add_argument("--mr", default=os.environ.get("CI_MERGE_REQUEST_LOCAL_ID", ""), + help="merge request local id (default: $CI_MERGE_REQUEST_LOCAL_ID)") + p.add_argument("--api-url", default=os.environ.get("GITFLIC_API_URL", "") or DEFAULT_API_URL, + help="GitFlic REST API base URL (default: $GITFLIC_API_URL or %s)" % DEFAULT_API_URL) + p.add_argument("--from", dest="from_ref", default=default_from, + help="base ref of the reviewed range (default: origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME)") + p.add_argument("--to", dest="to_ref", default=os.environ.get("CI_COMMIT_SHA", ""), + help="head ref of the reviewed range (default: $CI_COMMIT_SHA)") + p.add_argument("--repo", default=".", help="git repository root (default: .)") + p.add_argument("--dry-run", action="store_true", + help="print discussions instead of posting them") + return p.parse_args(argv) + + +def main(argv=None): + args = parse_args(sys.argv[1:] if argv is None else argv) + + missing = [name for name, value in ( + ("--owner", args.owner), ("--project", args.project), ("--mr", args.mr), + ("--from", args.from_ref), ("--to", args.to_ref), + ) if not value] + if missing: + log("error: %s required (set via flag or CI environment)" % ", ".join(missing)) + return 2 + + token = os.environ.get("GITFLIC_TOKEN", "") + if not token and not args.dry_run: + log("error: GITFLIC_TOKEN environment variable is required") + return 2 + + try: + result = load_review_result(args.file) + except (OSError, ValueError) as e: + log("error: cannot read review result %s: %s" % (args.file, e)) + return 1 + + try: + diffs_by_path = load_diffs_by_path(args.repo, args.from_ref, args.to_ref) + except (subprocess.CalledProcessError, OSError) as e: + # Without the diff, inline positions cannot be computed; comments still + # go out via the fallback note. + log("warning: cannot read diff %s..%s, posting all comments as fallback: %s" + % (args.from_ref, args.to_ref, e)) + diffs_by_path = {} + + if args.dry_run: + post = make_dry_run_poster() + else: + post = make_poster(args.api_url, token, args.owner, args.project, args.mr) + + stats = publish(result, diffs_by_path, post) + total = len(result.get("comments") or []) + print("Posted %d inline comment(s), %d via fallback note (%d total)." + % (stats["inline"], stats["fallback"], total)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/examples/gitflic_ci/post_review_test.py b/examples/gitflic_ci/post_review_test.py new file mode 100644 index 00000000..be438289 --- /dev/null +++ b/examples/gitflic_ci/post_review_test.py @@ -0,0 +1,240 @@ +#!/usr/bin/env python3 +"""Tests for post_review.py. + +Standard-library unittest only (no pytest, no network, no git): run with + + python3 post_review_test.py # from examples/gitflic_ci/ + python3 -m unittest discover examples/gitflic_ci + +The line-mapping cases are ported 1:1 from the upstream Go test +internal/publish/gitflic/linemap_test.go and publisher_test.go, so the script +keeps proven parity with the binary it replaces. +""" + +import unittest + +import post_review as pr + + +# old file lines 1..10; line 3 modified, a line inserted after old line 5, +# old line 8 deleted. (from linemap_test.go) +SAMPLE_DIFF = """diff --git a/main.go b/main.go +--- a/main.go ++++ b/main.go +@@ -1,10 +1,10 @@ + line1 + line2 +-line3 old ++line3 new + line4 + line5 ++inserted after5 + line6 + line7 +-line8 + line9 + line10 +""" + +NEW_FILE_DIFF = """diff --git a/added.go b/added.go +new file mode 100644 +--- /dev/null ++++ b/added.go +@@ -0,0 +1,3 @@ ++package main ++ ++func main() {} +""" + +DELETED_FILE_DIFF = """diff --git a/gone.go b/gone.go +deleted file mode 100644 +--- a/gone.go ++++ /dev/null +@@ -1,2 +0,0 @@ +-package main +- +""" + +BINARY_DIFF = """diff --git a/logo.png b/logo.png +index 1111111..2222222 100644 +Binary files a/logo.png and b/logo.png differ +""" + + +class OldLineForTest(unittest.TestCase): + def setUp(self): + self.hunks = pr.parse_hunks(SAMPLE_DIFF) + + def test_single_hunk_positions(self): + self.assertEqual(len(self.hunks), 1) + cases = [ + ("context before changes", 1, 1), + ("modified line maps to deleted position anchor", 3, 3), + ("context after modification", 4, 4), + ("added line anchors to preceding old line", 6, 5), + ("context shifted by insertion", 7, 6), + ("context after deletion", 9, 9), + ("last context line", 10, 10), + ] + for name, new_line, want in cases: + with self.subTest(name): + self.assertEqual(pr.old_line_for(self.hunks, new_line), want) + + def test_outside_hunks(self): + # one line added and one deleted -> cumulative delta 0 + self.assertEqual(pr.old_line_for(self.hunks, 42), 42) + + def test_multiple_hunks(self): + multi = ( + "@@ -1,2 +1,4 @@\n" + " line1\n" + "+added2\n" + "+added3\n" + " line2\n" + "@@ -10,3 +12,3 @@\n" + " line10\n" + "-line11 old\n" + "+line11 new\n" + " line12\n" + ) + hunks = pr.parse_hunks(multi) + self.assertEqual(len(hunks), 2) + # between hunks: new 8 = old 6 (two lines added by hunk 1) + self.assertEqual(pr.old_line_for(hunks, 8), 6) + # inside second hunk: modified new 13 anchors to old 11 + self.assertEqual(pr.old_line_for(hunks, 13), 11) + + def test_pure_addition_at_top(self): + hunks = pr.parse_hunks("@@ -0,0 +1,2 @@\n+first\n+second\n") + self.assertEqual(pr.old_line_for(hunks, 1), 1) + + +class ParseDiffTest(unittest.TestCase): + def test_modified_file(self): + fd = pr.parse_diff(SAMPLE_DIFF)[0] + self.assertEqual((fd.old_path, fd.new_path), ("main.go", "main.go")) + self.assertFalse(fd.is_new or fd.is_deleted or fd.is_binary) + + def test_new_file(self): + fd = pr.parse_diff(NEW_FILE_DIFF)[0] + self.assertTrue(fd.is_new) + self.assertEqual(fd.new_path, "added.go") + + def test_deleted_file(self): + fd = pr.parse_diff(DELETED_FILE_DIFF)[0] + self.assertTrue(fd.is_deleted) + self.assertEqual(fd.new_path, "/dev/null") + + def test_binary_file(self): + fd = pr.parse_diff(BINARY_DIFF)[0] + self.assertTrue(fd.is_binary) + + def test_multiple_files(self): + files = pr.parse_diff(SAMPLE_DIFF + NEW_FILE_DIFF) + self.assertEqual([f.new_path for f in files], ["main.go", "added.go"]) + + +class Recorder: + """A post() that records discussions; optionally fails the first inline.""" + + def __init__(self, fail_first_inline=False): + self.calls = [] + self.fail_first_inline = fail_first_inline + self._inline_seen = 0 + + def __call__(self, discussion): + if self.fail_first_inline and "newPath" in discussion: + self._inline_seen += 1 + if self._inline_seen == 1: + raise RuntimeError("simulated 403") + self.calls.append(discussion) + + +def diffs_from(diff_text): + return {fd.new_path: fd for fd in pr.parse_diff(diff_text)} + + +class PublishTest(unittest.TestCase): + def test_inline_and_summary(self): + result = { + "comments": [{ + "path": "main.go", "content": "possible nil dereference", + "start_line": 6, "end_line": 6, + "existing_code": "x := y.Field", + "suggestion_code": "if y != nil { x = y.Field }", + }], + } + rec = Recorder() + stats = pr.publish(result, diffs_from(SAMPLE_DIFF), rec) + + self.assertEqual(stats, {"inline": 1, "fallback": 0}) + self.assertEqual(len(rec.calls), 2) # inline + summary + + inline = rec.calls[0] + self.assertEqual(inline["newLine"], 6) + self.assertEqual(inline["oldLine"], 5) + self.assertEqual((inline["newPath"], inline["oldPath"]), ("main.go", "main.go")) + self.assertIn("possible nil dereference", inline["message"]) + self.assertIn("**Suggestion:**", inline["message"]) + + summary = rec.calls[1] + self.assertNotIn("newPath", summary) + self.assertIn("**1** issue(s)", summary["message"]) + + def test_fallback_for_unmapped_comment(self): + result = { + "comments": [{ + "path": "missing.go", "content": "issue in file absent from diff", + "start_line": 1, "end_line": 1, + }], + "warnings": [{"file": "a.go", "message": "skipped", "type": "subtask_error"}], + } + rec = Recorder() + stats = pr.publish(result, {}, rec) + + self.assertEqual(stats, {"inline": 0, "fallback": 1}) + self.assertEqual(len(rec.calls), 2) # fallback + summary + self.assertIn("could not be posted inline", rec.calls[0]["message"]) + self.assertIn("`missing.go`", rec.calls[0]["message"]) + self.assertIn("1 warning(s)", rec.calls[1]["message"]) + + def test_inline_error_falls_back(self): + result = { + "comments": [{ + "path": "main.go", "content": "finding", + "start_line": 1, "end_line": 1, + }], + } + rec = Recorder(fail_first_inline=True) + stats = pr.publish(result, diffs_from(SAMPLE_DIFF), rec) + + self.assertEqual(stats, {"inline": 0, "fallback": 1}) + self.assertEqual(len(rec.calls), 2) # fallback + summary after inline failure + + def test_no_comments(self): + rec = Recorder() + stats = pr.publish({"message": "No comments generated. Looks good to me."}, {}, rec) + + self.assertEqual(stats, {"inline": 0, "fallback": 0}) + self.assertEqual(len(rec.calls), 1) + self.assertIn("Looks good to me", rec.calls[0]["message"]) + + def test_new_file_anchors_to_new_path(self): + result = { + "comments": [{ + "path": "added.go", "content": "empty main", + "start_line": 3, "end_line": 3, + }], + } + rec = Recorder() + stats = pr.publish(result, diffs_from(NEW_FILE_DIFF), rec) + + self.assertEqual(stats["inline"], 1) + inline = rec.calls[0] + self.assertEqual(inline["oldPath"], "added.go") + self.assertEqual(inline["oldLine"], 1) + self.assertEqual(inline["newLine"], 3) + + +if __name__ == "__main__": + unittest.main()