diff --git a/README.ja-JP.md b/README.ja-JP.md index 573741d2..1cf8e6c2 100644 --- a/README.ja-JP.md +++ b/README.ja-JP.md @@ -288,12 +288,30 @@ CI統合のコアコマンド： ```bash ocr review \ --from "origin/main" \ - --to "origin/feature-branch" \ + --to "" \ --format json ``` +`--from`フラグはベースとしてブランチref（例：`origin/main`）またはコミットSHAを受け取り、`--to`はheadとしてコミットSHAまたはブランチrefを受け取ります。CI環境では、fork PR/MRのようにsource branchが`origin` remoteに存在しないケースに対応するため、`--to`にはコミットSHAを使うことを推奨します。 + `--format json`フラグは、CIスクリプトでのパースに適した機械可読な結果を出力します。 +WebUIや下流サービスで最終レビュー結果を参照する場合は、`--save-result`を追加します。デフォルトでは`~/.opencodereview/reviews`に保存されます。GitLab RunnerやJenkins agentなどのコンテナCIで実行する場合は、`--result-dir`または`OCR_REVIEWS_DIR`を使い、そのディレクトリを永続ストレージにマウントしてください。 + +```bash +ocr review \ + --from "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" \ + --to "$CI_COMMIT_SHA" \ + --format json \ + --save-result \ + --result-dir /ocr-data/reviews \ + --result-project "$CI_PROJECT_PATH" \ + --result-source-branch "$CI_MERGE_REQUEST_SOURCE_BRANCH_NAME" \ + --result-target-branch "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" +``` + +CIで企業共有ルールを使う場合は、ルールディレクトリをマウントし、`--rules-dir /ocr-data/rules`を渡すか、`OCR_RULES_DIR=/ocr-data/rules`を設定します。 + 統合例は[`examples/`](./examples/)ディレクトリを参照してください： - [`github_actions/`](./examples/github_actions/) — GitHub Actions統合の例 @@ -329,6 +347,12 @@ ocr review \ | `--background` | `-b` | — | レビューのための任意の要件/ビジネスコンテキスト。`--commit`使用時に未指定の場合、コミットメッセージから自動取得 | | `--model` | — | — | このレビューでLLMモデルを選択または上書き | | `--rule` | — | — | カスタムJSONレビュールールへのパス | +| `--rules-dir` | — | `OCR_RULES_DIR` | 企業/プロジェクト共有レビュールールのディレクトリ | +| `--save-result` | — | `false` | WebUI/APIで参照するために最終レビュー結果を保存 | +| `--result-dir` | — | `OCR_REVIEWS_DIR`または`~/.opencodereview/reviews` | レビュー結果の保存ルート | +| `--result-project` | — | GitLab `CI_PROJECT_PATH`またはrepo名 | 保存結果のプロジェクト名/パス | +| `--result-source-branch` | — | GitLab MR source branch | 保存結果のsource branchメタデータ | +| `--result-target-branch` | — | GitLab MR target branch | 保存結果のtarget branchメタデータ | | `--max-tools` | — | 組み込み値 | ファイルごとのツール呼び出しラウンドの上限。テンプレートのデフォルトより大きい場合のみ有効 | | `--max-git-procs` | — | 組み込み値 | gitサブプロセスの最大同時実行数 | | `--tools` | — | — | カスタムJSONツール設定へのパス | @@ -363,19 +387,25 @@ ocr review --background "ログインAPIにレート制限を追加" # カスタムレビュールールを使用 ocr review --rule /path/to/my-rules.json +ocr review --rules-dir /ocr-data/rules + +# WebUI/APIで参照するために最終レビュー結果を保存 +ocr review --from main --to my-feature --save-result --result-dir /ocr-data/reviews # ファイルに適用されるルールをプレビュー ocr rules check src/main/java/com/example/Foo.java ocr rules check --rule custom.json src/main/resources/mapper/UserMapper.xml +ocr rules check --rules-dir /ocr-data/rules src/main/java/com/example/Foo.java -# ブラウザでレビューセッション履歴を表示 +# ブラウザでレビューセッション履歴と保存済みレビュー結果を表示 ocr viewer ocr viewer --addr :3000 +ocr viewer --addr :5483 --reviews-dir /ocr-data/reviews ``` ### ビューアーのセキュリティ -ビューアーはセッションのJSONLコンテンツ（LLMリクエストメッセージとレスポンス）をHTTPで配信します。すべてのリクエストに対してHostヘッダーの許可リストを強制します：ループバック名（`localhost`、`127.0.0.0/8`、`::1`）と実際のバインドホストは常に許可されます。ワイルドカードバインド（`--addr :3000`、`--addr 0.0.0.0:3000`）やその他の非ループバックのホスト名は、環境変数`OCR_VIEWER_ALLOWED_HOSTS`（カンマ区切り）で追加する必要があります： +ビューアーはセッションのJSONLコンテンツ（LLMリクエストメッセージとレスポンス）と保存済みレビュー結果をHTTPで配信します。すべてのリクエストに対してHostヘッダーの許可リストを強制します：ループバック名（`localhost`、`127.0.0.0/8`、`::1`）と実際のバインドホストは常に許可されます。ワイルドカードバインド（`--addr :3000`、`--addr 0.0.0.0:3000`）やその他の非ループバックのホスト名は、環境変数`OCR_VIEWER_ALLOWED_HOSTS`（カンマ区切り）で追加する必要があります： ```bash OCR_VIEWER_ALLOWED_HOSTS=review.internal,ocr.lan ocr viewer --addr :3000 @@ -383,20 +413,53 @@ OCR_VIEWER_ALLOWED_HOSTS=review.internal,ocr.lan ocr viewer --addr :3000 これにより、ローカルビューアーに対するDNSリバインディング攻撃をブロックします。 +### 保存済みレビュー結果 + +`ocr review --save-result`は最終レビュー結果をJSONファイルとして保存します。結果には、プロジェクトメタデータ、利用可能なGitLabメタデータ、source/target branch、MR IID、pipeline/job ID、モデルとtoken使用量、warnings、review commentsが含まれます。 + +デフォルトの保存先： + +```text +~/.opencodereview/reviews//.json +``` + +CIコンテナでは、マウントされたパスを使うことを推奨します： + +```bash +export OCR_REVIEWS_DIR=/ocr-data/reviews +ocr review --save-result --from origin/main --to "$CI_COMMIT_SHA" +ocr viewer --addr :5483 --reviews-dir /ocr-data/reviews +``` + +viewerはHTMLページとJSON APIの両方を提供します： + +| Endpoint | 説明 | +|----------|------| +| `/reviews` | 保存済みレビュー結果を持つプロジェクト一覧 | +| `/reviews/{project}?source=&target=` | branch filter付きで単一プロジェクトのレビューを表示 | +| `/reviews/{project}/{reviewID}` | レビュー詳細を表示 | +| `/api/reviews?project=&source=&target=` | project/source/targetでfilterできるJSON一覧 | +| `/api/reviews/{project}?source=&target=` | encoded projectごとのJSON一覧 | +| `/api/reviews/{project}/{reviewID}` | JSONレビュー詳細 | + ## レビュールール -OCRは4層の優先度チェーンを使ってレビュールールを解決します。各層はファーストマッチ優先です：ファイルパスがパターンにマッチすればそのルールが使われ、マッチしなければ次の層にフォールスルーします。 +OCRは階層化された優先度チェーンを使ってレビュールールを解決します。各層はファーストマッチ優先です：ファイルパスがパターンにマッチすればそのルールが使われ、マッチしなければ次の層にフォールスルーします。 | 優先度 | ソース | パス | 説明 | |----------|--------|------|-------------| | 1（最高） | `--rule`フラグ | ユーザー指定パス | CLIによる明示的なオーバーライド | | 2 | プロジェクト設定 | `/.opencodereview/rule.json` | プロジェクトごとのルール。gitにコミット可能 | -| 3 | グローバル設定 | `~/.opencodereview/rule.json` | ユーザー全体の個人設定 | -| 4（最低） | システムデフォルト | 組み込みの`system_rules.json` | 一般的な言語とファイルタイプをカバーする組み込みルール | +| 3 | 企業プロジェクト設定 | `/projects//rule.json` | `--rules-dir`または`OCR_RULES_DIR`からのプロジェクト/チーム共有ルール | +| 4 | 企業グローバル設定 | `/global.json` | `--rules-dir`または`OCR_RULES_DIR`からの組織全体ルール | +| 5 | グローバル設定 | `~/.opencodereview/rule.json` | ユーザー全体の個人設定 | +| 6（最低） | システムデフォルト | 組み込みの`system_rules.json` | 一般的な言語とファイルタイプをカバーする組み込みルール | + +企業プロジェクトルールの検索では、`OCR_PROJECT`、GitLab `CI_PROJECT_PATH`、リポジトリディレクトリ名の順にproject keyを使用します。例：`CI_PROJECT_PATH=payments/order-service`は`/projects/payments/order-service/rule.json`にマップされます。 ### ルールファイルの形式 -第1〜3層は同じJSON形式を共有します： +すべてのカスタムルール層は同じJSON形式を共有します： ```json { @@ -417,6 +480,17 @@ OCRは4層の優先度チェーンを使ってレビュールールを解決し - 各層の中では、ルールは宣言順に評価されます — 最初にマッチしたものが採用されます。 - ルールファイルが存在しない場合は、何も出力せずスキップされます。 +企業ルールディレクトリの例： + +```text +/ocr-data/rules/ + global.json + projects/ + payments/ + order-service/ + rule.json +``` + ### パスフィルタリング ルールファイルでは `include` と `exclude` フィールドも使用でき、どのファイルをレビュー対象にするかを制御できます： @@ -444,7 +518,7 @@ OCRは4層の優先度チェーンを使ってレビュールールを解決し **動作ロジック：** -- `include`と`exclude`はレビュールールと同じ優先度チェーン（`--rule` > プロジェクト設定 > グローバル設定）に従います。**include/excludeが設定されている最も高い優先度の層**が一括で適用され、層を跨いだマージは行われません。 +- `include`と`exclude`はレビュールールと同じ優先度チェーン（`--rule` > プロジェクト設定 > 企業プロジェクト設定 > 企業グローバル設定 > グローバル設定）に従います。**include/excludeが設定されている最も高い優先度の層**が一括で適用され、層を跨いだマージは行われません。 - `exclude`は常に`include`より優先されます — 両方にマッチするファイルは除外されます。 - `include`は**組み込みデフォルト除外パターンをバイパスする**ためのものであり（例：テストファイル）、排他的な許可リストではありません — `include`パターンにマッチしないファイルも通常通りデフォルトフィルタチェックに進みます。 - パターン構文：`**`再帰マッチ、`*`単一セグメントマッチ、`{a,b}`ブレース展開をサポート。マッチングは大文字小文字を区別しません。 @@ -495,6 +569,9 @@ OCRは4層の優先度チェーンを使ってレビュールールを解決し | `OCR_LLM_AUTH_HEADER` | Anthropic認証ヘッダー（`x-api-key`または`authorization`） | | `OCR_LLM_MODEL` | モデル名 | | `OCR_USE_ANTHROPIC` | `true` = Anthropic、`false` = OpenAI | +| `OCR_REVIEWS_DIR` | `ocr review --save-result`と`ocr viewer --reviews-dir`のデフォルト保存ルート | +| `OCR_RULES_DIR` | `ocr review --rules-dir`と`ocr rules check --rules-dir`のデフォルト企業ルールディレクトリ | +| `OCR_PROJECT` | `/projects//rule.json`を解決するためのproject key | ## テレメトリー diff --git a/README.ko-KR.md b/README.ko-KR.md index 54f9a64b..cd9c519a 100644 --- a/README.ko-KR.md +++ b/README.ko-KR.md @@ -288,12 +288,30 @@ CI 통합의 핵심 명령: ```bash ocr review \ --from "origin/main" \ - --to "origin/feature-branch" \ + --to "" \ --format json ``` +`--from` flag는 base로 branch ref(예: `origin/main`) 또는 commit SHA를 받을 수 있고, `--to`는 head로 commit SHA 또는 branch ref를 받을 수 있습니다. CI 환경에서는 fork PR/MR처럼 source branch가 `origin` remote에 없을 수 있으므로 `--to`에 commit SHA를 사용하는 것을 권장합니다. + `--format json` flag는 CI script에서 파싱하기 좋은 machine-readable 결과를 출력합니다. +WebUI나 downstream service에서 최종 review 결과를 조회해야 한다면 `--save-result`를 추가하세요. 기본 저장 위치는 `~/.opencodereview/reviews`입니다. GitLab Runner나 Jenkins agent처럼 containerized CI에서 실행하는 경우 `--result-dir` 또는 `OCR_REVIEWS_DIR`를 사용하고 해당 directory를 persistent storage로 mount하세요. + +```bash +ocr review \ + --from "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" \ + --to "$CI_COMMIT_SHA" \ + --format json \ + --save-result \ + --result-dir /ocr-data/reviews \ + --result-project "$CI_PROJECT_PATH" \ + --result-source-branch "$CI_MERGE_REQUEST_SOURCE_BRANCH_NAME" \ + --result-target-branch "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" +``` + +CI에서 enterprise shared rules를 사용하려면 rules directory를 mount하고 `--rules-dir /ocr-data/rules`를 전달하거나 `OCR_RULES_DIR=/ocr-data/rules`를 설정하세요. + 통합 예시는 [`examples/`](./examples/) 디렉터리를 참고하세요. - [`github_actions/`](./examples/github_actions/): GitHub Actions 통합 예시 @@ -329,6 +347,12 @@ ocr review \ | `--background` | `-b` | - | 리뷰를 위한 선택적 요구사항/비즈니스 컨텍스트. `--commit` 사용 시 미지정이면 commit message에서 자동 추출 | | `--model` | - | - | 이번 리뷰에서 LLM model 선택 또는 override | | `--rule` | - | - | custom JSON review rules 경로 | +| `--rules-dir` | - | `OCR_RULES_DIR` | enterprise/project shared review rules directory | +| `--save-result` | - | `false` | WebUI/API 조회를 위해 최종 review result 저장 | +| `--result-dir` | - | `OCR_REVIEWS_DIR` 또는 `~/.opencodereview/reviews` | review result storage root | +| `--result-project` | - | GitLab `CI_PROJECT_PATH` 또는 repo 이름 | 저장된 result의 project name/path | +| `--result-source-branch` | - | GitLab MR source branch | 저장된 result의 source branch metadata | +| `--result-target-branch` | - | GitLab MR target branch | 저장된 result의 target branch metadata | | `--max-tools` | - | built-in | 파일별 최대 tool call round. template default보다 클 때만 적용 | | `--max-git-procs` | - | built-in | 최대 동시 git subprocess 수 | | `--tools` | - | - | custom JSON tools config 경로 | @@ -363,19 +387,25 @@ ocr review --background "로그인 API에 rate limiting 추가" # custom review rules 사용 ocr review --rule /path/to/my-rules.json +ocr review --rules-dir /ocr-data/rules + +# WebUI/API 조회를 위해 최종 review result 저장 +ocr review --from main --to my-feature --save-result --result-dir /ocr-data/reviews # 파일에 적용될 rule 미리보기 ocr rules check src/main/java/com/example/Foo.java ocr rules check --rule custom.json src/main/resources/mapper/UserMapper.xml +ocr rules check --rules-dir /ocr-data/rules src/main/java/com/example/Foo.java -# browser에서 review session history 보기 +# browser에서 review session history와 저장된 review result 보기 ocr viewer ocr viewer --addr :3000 +ocr viewer --addr :5483 --reviews-dir /ocr-data/reviews ``` ### Viewer 보안 -viewer는 session JSONL 내용(LLM request messages와 responses)을 HTTP로 제공합니다. 모든 request에 대해 Host header allowlist를 적용합니다. loopback 이름(`localhost`, `127.0.0.0/8`, `::1`)과 실제 bind host는 항상 허용됩니다. wildcard bind(`--addr :3000`, `--addr 0.0.0.0:3000`)와 다른 non-loopback hostname은 `OCR_VIEWER_ALLOWED_HOSTS` 환경 변수에 comma-separated 값으로 추가해야 합니다. +viewer는 session JSONL 내용(LLM request messages와 responses)과 저장된 review result를 HTTP로 제공합니다. 모든 request에 대해 Host header allowlist를 적용합니다. loopback 이름(`localhost`, `127.0.0.0/8`, `::1`)과 실제 bind host는 항상 허용됩니다. wildcard bind(`--addr :3000`, `--addr 0.0.0.0:3000`)와 다른 non-loopback hostname은 `OCR_VIEWER_ALLOWED_HOSTS` 환경 변수에 comma-separated 값으로 추가해야 합니다. ```bash OCR_VIEWER_ALLOWED_HOSTS=review.internal,ocr.lan ocr viewer --addr :3000 @@ -383,20 +413,53 @@ OCR_VIEWER_ALLOWED_HOSTS=review.internal,ocr.lan ocr viewer --addr :3000 이 설정은 local viewer를 대상으로 하는 DNS rebinding 공격을 차단합니다. +### 저장된 review result + +`ocr review --save-result`는 최종 review result를 JSON file로 저장합니다. result에는 project metadata, 사용 가능한 GitLab metadata, source/target branch, MR IID, pipeline/job ID, model 및 token usage, warnings, review comments가 포함됩니다. + +기본 저장 위치: + +```text +~/.opencodereview/reviews//.json +``` + +CI container에서는 mount된 path를 권장합니다. + +```bash +export OCR_REVIEWS_DIR=/ocr-data/reviews +ocr review --save-result --from origin/main --to "$CI_COMMIT_SHA" +ocr viewer --addr :5483 --reviews-dir /ocr-data/reviews +``` + +viewer는 HTML page와 JSON API를 함께 제공합니다. + +| Endpoint | Description | +|----------|-------------| +| `/reviews` | 저장된 review result가 있는 project 목록 | +| `/reviews/{project}?source=&target=` | branch filter로 단일 project review 탐색 | +| `/reviews/{project}/{reviewID}` | review detail 표시 | +| `/api/reviews?project=&source=&target=` | project/source/target으로 filter 가능한 JSON list | +| `/api/reviews/{project}?source=&target=` | encoded project별 JSON list | +| `/api/reviews/{project}/{reviewID}` | JSON review detail | + ## Review Rules -OCR은 네 계층의 priority chain으로 review rule을 해석합니다. 각 계층은 first-match-wins 방식입니다. 파일 경로가 pattern에 match되면 해당 rule을 사용하고, 아니면 다음 계층으로 넘어갑니다. +OCR은 layered priority chain으로 review rule을 해석합니다. 각 계층은 first-match-wins 방식입니다. 파일 경로가 pattern에 match되면 해당 rule을 사용하고, 아니면 다음 계층으로 넘어갑니다. | Priority | Source | Path | Description | |----------|--------|------|-------------| | 1 (highest) | `--rule` flag | User-specified path | CLI explicit override | | 2 | Project config | `/.opencodereview/rule.json` | project별 rule, git commit 가능 | -| 3 | Global config | `~/.opencodereview/rule.json` | user-wide 개인 선호 | -| 4 (lowest) | System default | Embedded `system_rules.json` | 일반 language와 file type을 다루는 built-in rule | +| 3 | Enterprise project config | `/projects//rule.json` | `--rules-dir` 또는 `OCR_RULES_DIR`의 project/team shared rules | +| 4 | Enterprise global config | `/global.json` | `--rules-dir` 또는 `OCR_RULES_DIR`의 organization-wide rules | +| 5 | Global config | `~/.opencodereview/rule.json` | user-wide 개인 선호 | +| 6 (lowest) | System default | Embedded `system_rules.json` | 일반 language와 file type을 다루는 built-in rule | + +Enterprise project lookup은 `OCR_PROJECT`, GitLab `CI_PROJECT_PATH`, repository directory name 순서로 project key를 사용합니다. 예: `CI_PROJECT_PATH=payments/order-service`는 `/projects/payments/order-service/rule.json`으로 매핑됩니다. ### Rule File Format -모든 계층은 같은 JSON format을 공유합니다. +모든 custom rule 계층은 같은 JSON format을 공유합니다. ```json { @@ -417,6 +480,17 @@ OCR은 네 계층의 priority chain으로 review rule을 해석합니다. 각 - 각 계층 안에서는 rule이 선언 순서대로 평가되며 첫 번째 match가 선택됩니다. - rule file이 없으면 조용히 건너뜁니다. +Enterprise rules directory 예시: + +```text +/ocr-data/rules/ + global.json + projects/ + payments/ + order-service/ + rule.json +``` + ## Configuration Reference Config file: `~/.opencodereview/config.json` @@ -453,6 +527,9 @@ Config file: `~/.opencodereview/config.json` | `OCR_LLM_AUTH_HEADER` | Anthropic auth header (`x-api-key` 또는 `authorization`) | | `OCR_LLM_MODEL` | Model name | | `OCR_USE_ANTHROPIC` | `true` = Anthropic, `false` = OpenAI | +| `OCR_REVIEWS_DIR` | `ocr review --save-result`와 `ocr viewer --reviews-dir`의 default storage root | +| `OCR_RULES_DIR` | `ocr review --rules-dir`와 `ocr rules check --rules-dir`의 default enterprise rules directory | +| `OCR_PROJECT` | `/projects//rule.json` resolving에 사용할 project key | ## Telemetry diff --git a/README.md b/README.md index c13e2e6e..14d8c47d 100644 --- a/README.md +++ b/README.md @@ -296,6 +296,22 @@ The `--from` flag accepts a branch ref (e.g., `origin/main`) or commit SHA as th The `--format json` flag outputs machine-readable results suitable for parsing in CI scripts. +To persist final review results for a WebUI or downstream service, add `--save-result`. By default results are written under `~/.opencodereview/reviews`; in containerized CI such as GitLab Runner or Jenkins agents, use `--result-dir` or `OCR_REVIEWS_DIR` and mount that directory to persistent storage: + +```bash +ocr review \ + --from "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" \ + --to "$CI_COMMIT_SHA" \ + --format json \ + --save-result \ + --result-dir /ocr-data/reviews \ + --result-project "$CI_PROJECT_PATH" \ + --result-source-branch "$CI_MERGE_REQUEST_SOURCE_BRANCH_NAME" \ + --result-target-branch "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" +``` + +For shared enterprise rules in CI, mount a rules directory and pass `--rules-dir /ocr-data/rules` or set `OCR_RULES_DIR=/ocr-data/rules`. + See the [`examples/`](./examples/) directory for integration examples: - [`github_actions/`](./examples/github_actions/) — GitHub Actions integration example @@ -331,6 +347,12 @@ See the [`examples/`](./examples/) directory for integration examples: | `--background` | `-b` | — | Optional requirement/business context for the review; auto-filled from commit message when using `--commit` | | `--model` | — | — | Select or override the LLM model for this review | | `--rule` | — | — | Path to custom JSON review rules | +| `--rules-dir` | — | `OCR_RULES_DIR` | Directory with enterprise/project review rules | +| `--save-result` | — | `false` | Persist final review result for the WebUI review viewer | +| `--result-dir` | — | `OCR_REVIEWS_DIR` or `~/.opencodereview/reviews` | Review result storage root | +| `--result-project` | — | GitLab `CI_PROJECT_PATH` or repo name | Project name/path for persisted results | +| `--result-source-branch` | — | GitLab MR source branch | Source branch metadata for persisted results | +| `--result-target-branch` | — | GitLab MR target branch | Target branch metadata for persisted results | | `--max-tools` | — | built-in | Max tool call rounds per file; only takes effect when greater than template default | | `--max-git-procs` | — | built-in | Max concurrent git subprocesses | | `--tools` | — | — | Path to custom JSON tools config | @@ -365,19 +387,25 @@ ocr review --background "Adding rate limiting to the login API" # Use custom review rules ocr review --rule /path/to/my-rules.json +ocr review --rules-dir /ocr-data/rules + +# Persist final review results for the WebUI/API +ocr review --from main --to my-feature --save-result --result-dir /ocr-data/reviews # Preview which rule applies to a file ocr rules check src/main/java/com/example/Foo.java ocr rules check --rule custom.json src/main/resources/mapper/UserMapper.xml +ocr rules check --rules-dir /ocr-data/rules src/main/java/com/example/Foo.java -# View review session history in browser +# View review session history and saved review results in browser ocr viewer ocr viewer --addr :3000 +ocr viewer --addr :5483 --reviews-dir /ocr-data/reviews ``` ### Viewer security -The viewer serves session JSONL contents (LLM request messages and responses) over HTTP. It enforces a Host-header allowlist on every request: loopback names (`localhost`, `127.0.0.0/8`, `::1`) and the concrete bind host are always allowed. Wildcard binds (`--addr :3000`, `--addr 0.0.0.0:3000`) and other non-loopback Hostnames must be added via the `OCR_VIEWER_ALLOWED_HOSTS` environment variable (comma-separated): +The viewer serves session JSONL contents (LLM request messages and responses) and saved review results over HTTP. It enforces a Host-header allowlist on every request: loopback names (`localhost`, `127.0.0.0/8`, `::1`) and the concrete bind host are always allowed. Wildcard binds (`--addr :3000`, `--addr 0.0.0.0:3000`) and other non-loopback Hostnames must be added via the `OCR_VIEWER_ALLOWED_HOSTS` environment variable (comma-separated): ```bash OCR_VIEWER_ALLOWED_HOSTS=review.internal,ocr.lan ocr viewer --addr :3000 @@ -385,20 +413,53 @@ OCR_VIEWER_ALLOWED_HOSTS=review.internal,ocr.lan ocr viewer --addr :3000 This blocks DNS-rebinding attacks against the local viewer. +### Saved review results + +`ocr review --save-result` writes final review results as JSON files. The result includes project metadata, GitLab metadata when available, source/target branches, MR IID, pipeline/job IDs, model and token usage, warnings, and review comments. + +Default storage: + +```text +~/.opencodereview/reviews//.json +``` + +In CI containers, prefer a mounted path: + +```bash +export OCR_REVIEWS_DIR=/ocr-data/reviews +ocr review --save-result --from origin/main --to "$CI_COMMIT_SHA" +ocr viewer --addr :5483 --reviews-dir /ocr-data/reviews +``` + +The viewer exposes both HTML pages and JSON APIs: + +| Endpoint | Description | +|----------|-------------| +| `/reviews` | List projects with saved review results | +| `/reviews/{project}?source=&target=` | Browse one project's saved reviews with branch filters | +| `/reviews/{project}/{reviewID}` | Show review details | +| `/api/reviews?project=&source=&target=` | JSON list across projects, filtered by project/source/target | +| `/api/reviews/{project}?source=&target=` | JSON list for one encoded project | +| `/api/reviews/{project}/{reviewID}` | JSON review detail | + ## Review Rules -OCR resolves review rules using a four-layer priority chain. Each layer uses first-match-wins: if a file path matches a pattern, that rule is used; otherwise it falls through to the next layer. +OCR resolves review rules using a layered priority chain. Each layer uses first-match-wins: if a file path matches a pattern, that rule is used; otherwise it falls through to the next layer. | Priority | Source | Path | Description | |----------|--------|------|-------------| | 1 (highest) | `--rule` flag | User-specified path | CLI explicit override | | 2 | Project config | `/.opencodereview/rule.json` | Per-project rules, can be committed to git | -| 3 | Global config | `~/.opencodereview/rule.json` | User-wide personal preferences | -| 4 (lowest) | System default | Embedded `system_rules.json` | Built-in rules covering common languages and file types | +| 3 | Enterprise project config | `/projects//rule.json` | Shared per-project/team rules from `--rules-dir` or `OCR_RULES_DIR` | +| 4 | Enterprise global config | `/global.json` | Shared organization-wide rules from `--rules-dir` or `OCR_RULES_DIR` | +| 5 | Global config | `~/.opencodereview/rule.json` | User-wide personal preferences | +| 6 (lowest) | System default | Embedded `system_rules.json` | Built-in rules covering common languages and file types | + +Enterprise project lookup uses the first available project key from `OCR_PROJECT`, GitLab `CI_PROJECT_PATH`, then the repository directory name. For example, `CI_PROJECT_PATH=payments/order-service` maps to `/projects/payments/order-service/rule.json`. ### Rule File Format -Layers 1–3 share the same JSON format: +All custom rule layers share the same JSON format: ```json { @@ -419,6 +480,17 @@ Layers 1–3 share the same JSON format: - Within each layer, rules are evaluated in declaration order — the first match wins. - If a rule file does not exist, it is silently skipped. +Example enterprise rules directory: + +```text +/ocr-data/rules/ + global.json + projects/ + payments/ + order-service/ + rule.json +``` + ### Path Filtering Rule files also support `include` and `exclude` fields to control which files enter the review scope: @@ -446,7 +518,7 @@ Rule files also support `include` and `exclude` fields to control which files en **How it works:** -- `include` and `exclude` follow the same priority chain as review rules (`--rule` > project config > global config). The **highest-priority layer that has include/exclude configured** takes effect as a whole — patterns are not merged across layers. +- `include` and `exclude` follow the same priority chain as review rules (`--rule` > project config > enterprise project config > enterprise global config > global config). The **highest-priority layer that has include/exclude configured** takes effect as a whole — patterns are not merged across layers. - `exclude` always wins over `include` — a file matching both is excluded. - `include` acts as a **bypass for built-in default exclude patterns** (e.g., test files), not as an exclusive allowlist — files not matching any `include` pattern still proceed through the default filter checks normally. - Pattern syntax: supports `**` recursive matching, `*` single-segment matching, and `{a,b}` brace expansion. Matching is case-insensitive. @@ -497,6 +569,9 @@ Environment variables take precedence over the config file. | `OCR_LLM_AUTH_HEADER` | Anthropic auth header (`x-api-key` or `authorization`) | | `OCR_LLM_MODEL` | Model name | | `OCR_USE_ANTHROPIC` | `true` = Anthropic, `false` = OpenAI | +| `OCR_REVIEWS_DIR` | Default storage root for `ocr review --save-result` and `ocr viewer --reviews-dir` | +| `OCR_RULES_DIR` | Default enterprise rules directory for `ocr review --rules-dir` and `ocr rules check --rules-dir` | +| `OCR_PROJECT` | Project key used to resolve `/projects//rule.json` | ## Telemetry diff --git a/README.ru-RU.md b/README.ru-RU.md index 146c868c..c42f09a5 100644 --- a/README.ru-RU.md +++ b/README.ru-RU.md @@ -296,6 +296,22 @@ ocr review \ Флаг `--format json` выводит машиночитаемый результат, удобный для разбора в CI-скриптах. +Чтобы сохранять финальные результаты ревью для WebUI или внешнего сервиса, добавьте `--save-result`. По умолчанию результаты пишутся в `~/.opencodereview/reviews`; в контейнерном CI, например GitLab Runner или Jenkins agents, используйте `--result-dir` или `OCR_REVIEWS_DIR` и примонтируйте этот каталог к постоянному хранилищу: + +```bash +ocr review \ + --from "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" \ + --to "$CI_COMMIT_SHA" \ + --format json \ + --save-result \ + --result-dir /ocr-data/reviews \ + --result-project "$CI_PROJECT_PATH" \ + --result-source-branch "$CI_MERGE_REQUEST_SOURCE_BRANCH_NAME" \ + --result-target-branch "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" +``` + +Для общих корпоративных правил в CI примонтируйте каталог правил и передайте `--rules-dir /ocr-data/rules` либо задайте `OCR_RULES_DIR=/ocr-data/rules`. + Примеры интеграции — в каталоге [`examples/`](./examples/): - [`github_actions/`](./examples/github_actions/) — пример интеграции с GitHub Actions @@ -331,6 +347,12 @@ ocr review \ | `--background` | `-b` | — | Необязательный контекст требований/бизнес-логики для ревью; при `--commit` автоматически заполняется из сообщения коммита | | `--model` | — | — | Выбрать или переопределить LLM-модель для этого ревью | | `--rule` | — | — | Путь к пользовательским JSON-правилам ревью | +| `--rules-dir` | — | `OCR_RULES_DIR` | Каталог корпоративных/проектных правил ревью | +| `--save-result` | — | `false` | Сохранить финальный результат ревью для WebUI/API | +| `--result-dir` | — | `OCR_REVIEWS_DIR` или `~/.opencodereview/reviews` | Корневой каталог хранения результатов ревью | +| `--result-project` | — | GitLab `CI_PROJECT_PATH` или имя repo | Имя/путь проекта в сохранённых результатах | +| `--result-source-branch` | — | Source branch GitLab MR | Метаданные исходной ветки | +| `--result-target-branch` | — | Target branch GitLab MR | Метаданные целевой ветки | | `--max-tools` | — | встроенное | Максимум раундов вызова инструментов на файл; действует, только если больше значения шаблона по умолчанию | | `--max-git-procs` | — | встроенное | Максимум одновременных git-подпроцессов | | `--tools` | — | — | Путь к пользовательскому JSON-конфигу инструментов | @@ -365,19 +387,25 @@ ocr review --background "Добавляем rate limiting в API логина" # Использовать собственные правила ревью ocr review --rule /path/to/my-rules.json +ocr review --rules-dir /ocr-data/rules + +# Сохранить финальные результаты ревью для WebUI/API +ocr review --from main --to my-feature --save-result --result-dir /ocr-data/reviews # Посмотреть, какое правило применяется к файлу ocr rules check src/main/java/com/example/Foo.java ocr rules check --rule custom.json src/main/resources/mapper/UserMapper.xml +ocr rules check --rules-dir /ocr-data/rules src/main/java/com/example/Foo.java -# Открыть историю сессий ревью в браузере +# Открыть историю сессий и сохранённые результаты ревью в браузере ocr viewer ocr viewer --addr :3000 +ocr viewer --addr :5483 --reviews-dir /ocr-data/reviews ``` ### Безопасность viewer'а -Viewer отдаёт содержимое сессионных JSONL-файлов (сообщения запросов к LLM и ответы) по HTTP. На каждый запрос применяется allowlist по заголовку Host: loopback-имена (`localhost`, `127.0.0.0/8`, `::1`) и конкретный хост привязки разрешены всегда. Wildcard-привязки (`--addr :3000`, `--addr 0.0.0.0:3000`) и прочие не-loopback имена хостов нужно добавлять через переменную окружения `OCR_VIEWER_ALLOWED_HOSTS` (через запятую): +Viewer отдаёт содержимое сессионных JSONL-файлов (сообщения запросов к LLM и ответы) и сохранённые результаты ревью по HTTP. На каждый запрос применяется allowlist по заголовку Host: loopback-имена (`localhost`, `127.0.0.0/8`, `::1`) и конкретный хост привязки разрешены всегда. Wildcard-привязки (`--addr :3000`, `--addr 0.0.0.0:3000`) и прочие не-loopback имена хостов нужно добавлять через переменную окружения `OCR_VIEWER_ALLOWED_HOSTS` (через запятую): ```bash OCR_VIEWER_ALLOWED_HOSTS=review.internal,ocr.lan ocr viewer --addr :3000 @@ -385,20 +413,53 @@ OCR_VIEWER_ALLOWED_HOSTS=review.internal,ocr.lan ocr viewer --addr :3000 Это блокирует атаки DNS rebinding на локальный viewer. +### Сохранённые результаты ревью + +`ocr review --save-result` сохраняет финальный результат ревью в JSON-файл. Результат включает метаданные проекта, доступные метаданные GitLab, source/target branches, MR IID, pipeline/job ID, модель и расход токенов, warnings и review comments. + +Путь хранения по умолчанию: + +```text +~/.opencodereview/reviews//.json +``` + +В CI-контейнерах лучше использовать примонтированный путь: + +```bash +export OCR_REVIEWS_DIR=/ocr-data/reviews +ocr review --save-result --from origin/main --to "$CI_COMMIT_SHA" +ocr viewer --addr :5483 --reviews-dir /ocr-data/reviews +``` + +Viewer предоставляет HTML-страницы и JSON API: + +| Endpoint | Описание | +|----------|----------| +| `/reviews` | Список проектов с сохранёнными результатами ревью | +| `/reviews/{project}?source=&target=` | Просмотр ревью одного проекта с фильтрами веток | +| `/reviews/{project}/{reviewID}` | Детали ревью | +| `/api/reviews?project=&source=&target=` | JSON-список по всем проектам с фильтрами project/source/target | +| `/api/reviews/{project}?source=&target=` | JSON-список для одного encoded project | +| `/api/reviews/{project}/{reviewID}` | JSON-детали ревью | + ## Правила ревью -OCR разрешает правила ревью по цепочке приоритетов из четырёх уровней. На каждом уровне действует принцип «первое совпадение побеждает»: если путь файла совпал с паттерном, используется это правило; иначе поиск продолжается на следующем уровне. +OCR разрешает правила ревью по многоуровневой цепочке приоритетов. На каждом уровне действует принцип «первое совпадение побеждает»: если путь файла совпал с паттерном, используется это правило; иначе поиск продолжается на следующем уровне. | Приоритет | Источник | Путь | Описание | |-----------|----------|------|----------| | 1 (высший) | Флаг `--rule` | Путь, указанный пользователем | Явное переопределение из CLI | | 2 | Конфиг проекта | `/.opencodereview/rule.json` | Правила уровня проекта, можно коммитить в git | -| 3 | Глобальный конфиг | `~/.opencodereview/rule.json` | Личные настройки пользователя | -| 4 (низший) | Системные по умолчанию | Встроенный `system_rules.json` | Встроенные правила для распространённых языков и типов файлов | +| 3 | Корпоративный конфиг проекта | `/projects//rule.json` | Общие правила проекта/команды из `--rules-dir` или `OCR_RULES_DIR` | +| 4 | Корпоративный глобальный конфиг | `/global.json` | Общие правила организации из `--rules-dir` или `OCR_RULES_DIR` | +| 5 | Глобальный конфиг | `~/.opencodereview/rule.json` | Личные настройки пользователя | +| 6 (низший) | Системные по умолчанию | Встроенный `system_rules.json` | Встроенные правила для распространённых языков и типов файлов | + +Поиск корпоративных правил проекта использует первый доступный ключ проекта из `OCR_PROJECT`, GitLab `CI_PROJECT_PATH`, затем имени каталога репозитория. Например, `CI_PROJECT_PATH=payments/order-service` соответствует `/projects/payments/order-service/rule.json`. ### Формат файла правил -Уровни 1–3 используют один и тот же JSON-формат: +Все пользовательские уровни правил используют один и тот же JSON-формат: ```json { @@ -419,6 +480,17 @@ OCR разрешает правила ревью по цепочке приор - Внутри каждого уровня правила проверяются в порядке объявления — побеждает первое совпадение. - Если файл правил не существует, он молча пропускается. +Пример каталога корпоративных правил: + +```text +/ocr-data/rules/ + global.json + projects/ + payments/ + order-service/ + rule.json +``` + ### Фильтрация путей Файлы правил также поддерживают поля `include` и `exclude`, управляющие тем, какие файлы попадают в область ревью: @@ -446,7 +518,7 @@ OCR разрешает правила ревью по цепочке приор **Как это работает:** -- `include` и `exclude` следуют той же цепочке приоритетов, что и правила ревью (`--rule` > конфиг проекта > глобальный конфиг). Действует **целиком самый приоритетный уровень, на котором include/exclude настроены** — паттерны разных уровней не объединяются. +- `include` и `exclude` следуют той же цепочке приоритетов, что и правила ревью (`--rule` > конфиг проекта > корпоративный конфиг проекта > корпоративный глобальный конфиг > глобальный конфиг). Действует **целиком самый приоритетный уровень, на котором include/exclude настроены** — паттерны разных уровней не объединяются. - `exclude` всегда сильнее `include` — файл, совпавший с обоими, исключается. - `include` работает как **обход встроенных паттернов исключения по умолчанию** (например, тестовых файлов), а не как эксклюзивный allowlist: файлы, не совпавшие ни с одним паттерном `include`, всё равно обычным образом проходят проверки фильтра по умолчанию. - Синтаксис паттернов: поддерживаются рекурсивное сопоставление `**`, односегментное `*` и расширение фигурных скобок `{a,b}`. Сопоставление регистронезависимое. @@ -497,6 +569,9 @@ OCR разрешает правила ревью по цепочке приор | `OCR_LLM_AUTH_HEADER` | Заголовок авторизации Anthropic (`x-api-key` или `authorization`) | | `OCR_LLM_MODEL` | Имя модели | | `OCR_USE_ANTHROPIC` | `true` = Anthropic, `false` = OpenAI | +| `OCR_REVIEWS_DIR` | Корневой каталог хранения по умолчанию для `ocr review --save-result` и `ocr viewer --reviews-dir` | +| `OCR_RULES_DIR` | Корпоративный каталог правил по умолчанию для `ocr review --rules-dir` и `ocr rules check --rules-dir` | +| `OCR_PROJECT` | Ключ проекта для разрешения `/projects//rule.json` | ## Телеметрия diff --git a/README.zh-CN.md b/README.zh-CN.md index 812b5516..d56f7d55 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -288,12 +288,30 @@ CI 集成的核心命令： ```bash ocr review \ --from "origin/main" \ - --to "origin/feature-branch" \ + --to "" \ --format json ``` +`--from` 参数接受分支引用（如 `origin/main`）或提交 SHA 作为基准，`--to` 接受提交 SHA 或分支引用作为目标。在 CI 环境中，推荐将 `--to` 设置为提交 SHA，以正确处理 fork PR/MR 中源分支不在 `origin` remote 的情况。 + `--format json` 参数输出适合 CI 脚本解析的机器可读结果。 +如果需要为 WebUI 或下游服务持久化最终评审结果，添加 `--save-result`。默认结果写入 `~/.opencodereview/reviews`；在 GitLab Runner、Jenkins agent 等容器化 CI 中，应使用 `--result-dir` 或 `OCR_REVIEWS_DIR`，并将该目录挂载到持久化存储： + +```bash +ocr review \ + --from "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" \ + --to "$CI_COMMIT_SHA" \ + --format json \ + --save-result \ + --result-dir /ocr-data/reviews \ + --result-project "$CI_PROJECT_PATH" \ + --result-source-branch "$CI_MERGE_REQUEST_SOURCE_BRANCH_NAME" \ + --result-target-branch "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" +``` + +如果需要在 CI 中使用企业共享规则，挂载规则目录并传入 `--rules-dir /ocr-data/rules`，或设置 `OCR_RULES_DIR=/ocr-data/rules`。 + 集成示例请参见 [`examples/`](./examples/) 目录： - [`github_actions/`](./examples/github_actions/) — GitHub Actions 集成示例 @@ -329,6 +347,12 @@ ocr review \ | `--background` | `-b` | — | 可选的需求/业务背景信息；使用 `--commit` 时如未指定则自动从 commit message 中提取 | | `--model` | — | — | 为本次审查选择或覆盖 LLM 模型 | | `--rule` | — | — | 自定义 JSON 审查规则路径 | +| `--rules-dir` | — | `OCR_RULES_DIR` | 企业/项目共享审查规则目录 | +| `--save-result` | — | `false` | 持久化最终评审结果，供 WebUI/API 查看 | +| `--result-dir` | — | `OCR_REVIEWS_DIR` 或 `~/.opencodereview/reviews` | 评审结果存储根目录 | +| `--result-project` | — | GitLab `CI_PROJECT_PATH` 或仓库名 | 持久化结果中的项目名称/路径 | +| `--result-source-branch` | — | GitLab MR 源分支 | 持久化结果中的来源分支元数据 | +| `--result-target-branch` | — | GitLab MR 目标分支 | 持久化结果中的目标分支元数据 | | `--max-tools` | — | 内置默认 | 每个文件的最大工具调用轮次；仅在大于模板默认值时生效 | | `--max-git-procs` | — | 内置默认 | 最大并发 git 子进程数 | | `--tools` | — | — | 自定义 JSON 工具配置路径 | @@ -363,30 +387,79 @@ ocr review --background "为登录 API 添加限流" # 使用自定义审查规则 ocr review --rule /path/to/my-rules.json +ocr review --rules-dir /ocr-data/rules + +# 持久化最终评审结果，供 WebUI/API 查看 +ocr review --from main --to my-feature --save-result --result-dir /ocr-data/reviews # 预览某个文件路径生效的规则 ocr rules check src/main/java/com/example/Foo.java ocr rules check --rule custom.json src/main/resources/mapper/UserMapper.xml +ocr rules check --rules-dir /ocr-data/rules src/main/java/com/example/Foo.java -# 在浏览器中查看审查会话历史 +# 在浏览器中查看审查会话历史和已保存的评审结果 ocr viewer ocr viewer --addr :3000 +ocr viewer --addr :5483 --reviews-dir /ocr-data/reviews ``` +### Viewer 安全性 + +viewer 会通过 HTTP 暴露 session JSONL 内容（LLM 请求消息和响应）以及已保存的评审结果。它会对每个请求强制校验 Host header：loopback 名称（`localhost`、`127.0.0.0/8`、`::1`）和实际绑定 host 总是允许；通配绑定（`--addr :3000`、`--addr 0.0.0.0:3000`）以及其他非 loopback hostname 需要通过 `OCR_VIEWER_ALLOWED_HOSTS` 环境变量以逗号分隔添加： + +```bash +OCR_VIEWER_ALLOWED_HOSTS=review.internal,ocr.lan ocr viewer --addr :3000 +``` + +这可以阻止针对本地 viewer 的 DNS rebinding 攻击。 + +### 已保存的评审结果 + +`ocr review --save-result` 会将最终评审结果写成 JSON 文件。结果包含项目元数据、可用的 GitLab 元数据、来源/目标分支、MR IID、pipeline/job ID、模型和 token 使用量、warnings 以及 review comments。 + +默认存储位置： + +```text +~/.opencodereview/reviews//.json +``` + +在 CI 容器中，建议使用挂载目录： + +```bash +export OCR_REVIEWS_DIR=/ocr-data/reviews +ocr review --save-result --from origin/main --to "$CI_COMMIT_SHA" +ocr viewer --addr :5483 --reviews-dir /ocr-data/reviews +``` + +viewer 同时提供 HTML 页面和 JSON API： + +| Endpoint | 描述 | +|----------|------| +| `/reviews` | 列出有已保存评审结果的项目 | +| `/reviews/{project}?source=&target=` | 按来源/目标分支筛选并浏览单个项目的评审 | +| `/reviews/{project}/{reviewID}` | 查看评审详情 | +| `/api/reviews?project=&source=&target=` | 跨项目 JSON 列表，支持项目/source/target 筛选 | +| `/api/reviews/{project}?source=&target=` | 单个 encoded project 的 JSON 列表 | +| `/api/reviews/{project}/{reviewID}` | JSON 评审详情 | + ## 评审规则 -OCR 通过四层优先级链解析评审规则。每层采用首次匹配原则：如果文件路径匹配到某个模式，则使用该规则；否则穿透到下一层。 +OCR 通过分层优先级链解析评审规则。每层采用首次匹配原则：如果文件路径匹配到某个模式，则使用该规则；否则穿透到下一层。 | 优先级 | 来源 | 路径 | 描述 | |--------|------|------|------| | 1（最高） | `--rule` 参数 | 用户指定路径 | CLI 显式覆盖 | | 2 | 项目配置 | `/.opencodereview/rule.json` | 项目级规则，可提交到 git | -| 3 | 全局配置 | `~/.opencodereview/rule.json` | 用户级个人偏好 | -| 4（最低） | 系统默认 | 内嵌 `system_rules.json` | 覆盖常见语言和文件类型的内置规则 | +| 3 | 企业项目配置 | `/projects//rule.json` | 来自 `--rules-dir` 或 `OCR_RULES_DIR` 的项目/团队共享规则 | +| 4 | 企业全局配置 | `/global.json` | 来自 `--rules-dir` 或 `OCR_RULES_DIR` 的组织级共享规则 | +| 5 | 全局配置 | `~/.opencodereview/rule.json` | 用户级个人偏好 | +| 6（最低） | 系统默认 | 内嵌 `system_rules.json` | 覆盖常见语言和文件类型的内置规则 | + +企业项目规则查找会依次使用 `OCR_PROJECT`、GitLab `CI_PROJECT_PATH`、仓库目录名。比如 `CI_PROJECT_PATH=payments/order-service` 会映射到 `/projects/payments/order-service/rule.json`。 ### 规则文件格式 -第 1–3 层使用相同的 JSON 格式： +所有自定义规则层使用相同的 JSON 格式： ```json { @@ -407,6 +480,17 @@ OCR 通过四层优先级链解析评审规则。每层采用首次匹配原则 - 在每一层内，规则按声明顺序评估 —— 首次匹配生效。 - 如果规则文件不存在，将被静默跳过。 +企业规则目录示例： + +```text +/ocr-data/rules/ + global.json + projects/ + payments/ + order-service/ + rule.json +``` + ### 路径过滤 规则文件同时支持 `include` 和 `exclude` 字段，用于控制哪些文件进入审查范围： @@ -434,7 +518,7 @@ OCR 通过四层优先级链解析评审规则。每层采用首次匹配原则 **生效逻辑：** -- `include` 和 `exclude` 遵循与评审规则相同的优先级链（`--rule` > 项目配置 > 全局配置），取**最高优先级中配置了 include/exclude 的那一层**整体生效，不会跨层合并。 +- `include` 和 `exclude` 遵循与评审规则相同的优先级链（`--rule` > 项目配置 > 企业项目配置 > 企业全局配置 > 全局配置），取**最高优先级中配置了 include/exclude 的那一层**整体生效，不会跨层合并。 - `exclude` 始终优先于 `include` —— 同时匹配两者的文件会被排除。 - `include` 的作用是**绕过内置默认排除模式**（如测试文件），而非限制审查范围 —— 未匹配 `include` 的文件仍会正常进入后续的默认过滤判断。 - 模式语法：支持 `**` 递归匹配、`*` 单级匹配和 `{a,b}` 大括号展开，匹配时不区分大小写。 @@ -485,6 +569,9 @@ OCR 通过四层优先级链解析评审规则。每层采用首次匹配原则 | `OCR_LLM_AUTH_HEADER` | Anthropic 认证头（`x-api-key` 或 `authorization`） | | `OCR_LLM_MODEL` | 模型名称 | | `OCR_USE_ANTHROPIC` | `true` = Anthropic，`false` = OpenAI | +| `OCR_REVIEWS_DIR` | `ocr review --save-result` 和 `ocr viewer --reviews-dir` 的默认评审结果存储根目录 | +| `OCR_RULES_DIR` | `ocr review --rules-dir` 和 `ocr rules check --rules-dir` 的默认企业规则目录 | +| `OCR_PROJECT` | 用于解析 `/projects//rule.json` 的项目 key | ## 遥测 diff --git a/cmd/opencodereview/flags.go b/cmd/opencodereview/flags.go index 6c3d4f00..7be633a1 100644 --- a/cmd/opencodereview/flags.go +++ b/cmd/opencodereview/flags.go @@ -105,6 +105,12 @@ type reviewOptions struct { audience string // --audience: "human" (default) or "agent" background string // --background: optional requirement context model string // --model: override resolved LLM model for this review + rulesDir string // --rules-dir: directory with enterprise/project rules + saveResult bool // --save-result: persist final review result for viewer + resultDir string // --result-dir: root directory for persisted review results + resultProject string // --result-project: display project name/path for persisted results + resultSource string // --result-source-branch: source branch metadata for persisted results + resultTarget string // --result-target-branch: target branch metadata for persisted results concurrency int perFileTimeout int maxTools int @@ -130,6 +136,12 @@ func parseReviewFlags(args []string) (reviewOptions, error) { a.StringVar(&opts.audience, "audience", "human", "output audience: human (show progress) or agent (summary only)") a.StringVarP(&opts.background, "background", "b", "", "optional requirement/business context for the review") a.StringVar(&opts.model, "model", "", "override LLM model for this review (e.g., claude-opus-4-6)") + a.StringVar(&opts.rulesDir, "rules-dir", "", "directory with enterprise/project review rules") + a.BoolVar(&opts.saveResult, "save-result", false, "persist final review result for the WebUI review viewer") + a.StringVar(&opts.resultDir, "result-dir", "", "review result storage root (env: OCR_REVIEWS_DIR, default: ~/.opencodereview/reviews)") + a.StringVar(&opts.resultProject, "result-project", "", "project name/path for persisted review results (default: GitLab CI_PROJECT_PATH or repo basename)") + a.StringVar(&opts.resultSource, "result-source-branch", "", "source branch metadata for persisted review results") + a.StringVar(&opts.resultTarget, "result-target-branch", "", "target branch metadata for persisted review results") a.IntVar(&opts.maxTools, "max-tools", 0, "max tool call rounds per file (0 = template default; min 10)") a.IntVar(&opts.maxGitProcs, "max-git-procs", 16, "max concurrent git subprocesses") a.BoolVarP(&opts.preview, "preview", "p", false, "preview which files will be reviewed without running the LLM") @@ -142,6 +154,12 @@ func parseReviewFlags(args []string) (reviewOptions, error) { if opts.showHelp { return opts, nil } + if opts.resultDir == "" { + opts.resultDir = os.Getenv("OCR_REVIEWS_DIR") + } + if opts.rulesDir == "" { + opts.rulesDir = os.Getenv("OCR_RULES_DIR") + } modeCount := 0 if opts.from != "" || opts.to != "" { @@ -212,6 +230,9 @@ Examples: ocr review --preview ocr review -c abc123 -p + # Persist final review result for WebUI lookup + ocr review --from master --to dev-ref --save-result + Flags: --audience string output audience: human (show progress) or agent (summary only) (default "human") -b, --background string optional requirement/business context for the review @@ -224,7 +245,15 @@ Flags: --model string override LLM model for this review (e.g., claude-opus-4-6) -p, --preview preview which files will be reviewed without running the LLM --repo string root directory of the git repository (default: current dir) + --result-dir string review result storage root (env: OCR_REVIEWS_DIR, default: ~/.opencodereview/reviews) + --result-project string project name/path for persisted review results + --result-source-branch string + source branch metadata for persisted review results + --result-target-branch string + target branch metadata for persisted review results --rule string path to JSON file with system review rules + --rules-dir string directory with enterprise/project review rules + --save-result persist final review result for the WebUI review viewer --timeout int concurrent task timeout in minutes (default 10) --to string target ref to end diff at (e.g., 'feature-branch') --tools string path to JSON tools config file (default: embedded)`) diff --git a/cmd/opencodereview/flags_test.go b/cmd/opencodereview/flags_test.go index 617dfa44..03830a9b 100644 --- a/cmd/opencodereview/flags_test.go +++ b/cmd/opencodereview/flags_test.go @@ -18,3 +18,46 @@ func TestParseReviewFlagsModelOverride(t *testing.T) { t.Errorf("audience = %q, want %q", opts.audience, "human") } } + +func TestParseReviewFlagsSaveResult(t *testing.T) { + opts, err := parseReviewFlags([]string{ + "--save-result", + "--result-dir", "/tmp/ocr-results", + "--result-project", "group/project", + "--result-source-branch", "feature/a", + "--result-target-branch", "main", + }) + if err != nil { + t.Fatalf("parseReviewFlags: %v", err) + } + + if !opts.saveResult { + t.Fatal("saveResult = false, want true") + } + if opts.resultDir != "/tmp/ocr-results" { + t.Errorf("resultDir = %q", opts.resultDir) + } + if opts.resultProject != "group/project" { + t.Errorf("resultProject = %q", opts.resultProject) + } + if opts.resultSource != "feature/a" { + t.Errorf("resultSource = %q", opts.resultSource) + } + if opts.resultTarget != "main" { + t.Errorf("resultTarget = %q", opts.resultTarget) + } +} + +func TestParseReviewFlagsRulesDirAndEnvResultDir(t *testing.T) { + t.Setenv("OCR_REVIEWS_DIR", "/mnt/ocr/reviews") + opts, err := parseReviewFlags([]string{"--rules-dir", "/mnt/ocr/rules"}) + if err != nil { + t.Fatalf("parseReviewFlags: %v", err) + } + if opts.rulesDir != "/mnt/ocr/rules" { + t.Errorf("rulesDir = %q", opts.rulesDir) + } + if opts.resultDir != "/mnt/ocr/reviews" { + t.Errorf("resultDir = %q", opts.resultDir) + } +} diff --git a/cmd/opencodereview/review_cmd.go b/cmd/opencodereview/review_cmd.go index be68b32c..dd4d3fab 100644 --- a/cmd/opencodereview/review_cmd.go +++ b/cmd/opencodereview/review_cmd.go @@ -59,7 +59,10 @@ func runReview(args []string) error { } } - resolver, fileFilter, err := rules.NewResolver(repoDir, opts.rulePath) + resolver, fileFilter, err := rules.NewResolverWithOptions(repoDir, rules.ResolverOptions{ + CustomRulePath: opts.rulePath, + RulesDir: opts.rulesDir, + }) if err != nil { return fmt.Errorf("load rules: %w", err) } @@ -162,6 +165,16 @@ func runReview(args []string) error { if len(comments) > 0 { telemetry.RecordCommentsGenerated(ctx, int64(len(comments))) } + warnings := ag.Warnings() + + if opts.saveResult { + path, err := saveReviewResult(repoDir, opts, ag, comments, warnings, duration) + if err != nil { + fmt.Fprintf(os.Stderr, "[ocr] warning: failed to save review result: %v\n", err) + } else { + warnReviewResultSaved(path) + } + } // If no files were reviewed (e.g. workspace has no changes), inform the caller in JSON mode. if opts.outputFormat == "json" && len(comments) == 0 && ag.FilesReviewed() == 0 { @@ -179,13 +192,13 @@ func runReview(args []string) error { } if opts.outputFormat == "json" { - return outputJSONWithWarnings(comments, ag.Warnings(), ag.FilesReviewed(), ag.TotalInputTokens(), ag.TotalOutputTokens(), ag.TotalTokensUsed(), ag.TotalCacheReadTokens(), ag.TotalCacheWriteTokens(), duration) + return outputJSONWithWarnings(comments, warnings, ag.FilesReviewed(), ag.TotalInputTokens(), ag.TotalOutputTokens(), ag.TotalTokensUsed(), ag.TotalCacheReadTokens(), ag.TotalCacheWriteTokens(), duration) } if opts.audience == "agent" { - outputTextWithWarnings(comments, ag.Warnings()) + outputTextWithWarnings(comments, warnings) return nil } - outputTextWithWarnings(comments, ag.Warnings()) + outputTextWithWarnings(comments, warnings) return nil } diff --git a/cmd/opencodereview/review_result.go b/cmd/opencodereview/review_result.go new file mode 100644 index 00000000..33f000f7 --- /dev/null +++ b/cmd/opencodereview/review_result.go @@ -0,0 +1,97 @@ +package main + +import ( + "fmt" + "os" + "path/filepath" + "time" + + "github.com/open-code-review/open-code-review/internal/agent" + "github.com/open-code-review/open-code-review/internal/model" + "github.com/open-code-review/open-code-review/internal/reviewstore" +) + +func saveReviewResult(repoDir string, opts reviewOptions, ag *agent.Agent, comments []model.LlmComment, warnings []agent.AgentWarning, duration time.Duration) (string, error) { + sourceBranch := firstNonEmpty(opts.resultSource, os.Getenv("CI_MERGE_REQUEST_SOURCE_BRANCH_NAME")) + targetBranch := firstNonEmpty(opts.resultTarget, os.Getenv("CI_MERGE_REQUEST_TARGET_BRANCH_NAME")) + projectName := firstNonEmpty(opts.resultProject, os.Getenv("CI_PROJECT_PATH"), filepath.Base(repoDir)) + projectID := os.Getenv("CI_PROJECT_ID") + + reviewMode := "workspace" + if opts.commit != "" { + reviewMode = "commit" + } else if opts.from != "" && opts.to != "" { + reviewMode = "range" + } + + result := reviewstore.Result{ + Project: reviewstore.ProjectInfo{ + ID: projectID, + Name: projectName, + RepoDir: repoDir, + WebURL: os.Getenv("CI_PROJECT_URL"), + }, + GitLab: reviewstore.GitLabInfo{ + ServerURL: os.Getenv("CI_SERVER_URL"), + ProjectID: projectID, + MergeRequestIID: os.Getenv("CI_MERGE_REQUEST_IID"), + PipelineID: os.Getenv("CI_PIPELINE_ID"), + JobID: os.Getenv("CI_JOB_ID"), + }, + Review: reviewstore.ReviewInfo{ + Mode: reviewMode, + SourceBranch: sourceBranch, + TargetBranch: targetBranch, + From: opts.from, + To: opts.to, + Commit: opts.commit, + Model: ag.Session().Model, + FilesReviewed: ag.FilesReviewed(), + CommentCount: int64(len(comments)), + TotalTokens: ag.TotalTokensUsed(), + InputTokens: ag.TotalInputTokens(), + OutputTokens: ag.TotalOutputTokens(), + CacheReadTokens: ag.TotalCacheReadTokens(), + CacheWriteTokens: ag.TotalCacheWriteTokens(), + Duration: duration.String(), + DurationSeconds: int64(duration.Seconds()), + SessionID: ag.Session().SessionID, + }, + Comments: comments, + Warnings: mapReviewWarnings(warnings), + } + + path, err := reviewstore.Save(opts.resultDir, result) + if err != nil { + return "", err + } + return path, nil +} + +func mapReviewWarnings(warnings []agent.AgentWarning) []reviewstore.Warning { + if len(warnings) == 0 { + return nil + } + out := make([]reviewstore.Warning, 0, len(warnings)) + for _, warning := range warnings { + out = append(out, reviewstore.Warning{ + File: warning.File, + Message: warning.Message, + Type: warning.Type, + }) + } + return out +} + +func firstNonEmpty(values ...string) string { + for _, value := range values { + if value != "" { + return value + } + } + return "" +} + +func warnReviewResultSaved(path string) { + fmt.Fprintf(os.Stderr, "[ocr] review result saved: %s\n", path) +} diff --git a/cmd/opencodereview/rules_cmd.go b/cmd/opencodereview/rules_cmd.go index cfb9b5bd..aac4fa0c 100644 --- a/cmd/opencodereview/rules_cmd.go +++ b/cmd/opencodereview/rules_cmd.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "os" "strings" "github.com/open-code-review/open-code-review/internal/config/rules" @@ -25,9 +26,10 @@ func runRules(args []string) error { func runRulesCheck(args []string) error { a := newOcrFlagSet("ocr rules check") - var repoDir, rulePath string + var repoDir, rulePath, rulesDir string a.StringVar(&repoDir, "repo", "", "root directory of the git repository (default: current dir)") a.StringVar(&rulePath, "rule", "", "path to JSON file with custom review rules") + a.StringVar(&rulesDir, "rules-dir", "", "directory with enterprise/project review rules") if err := a.Parse(args); err != nil { return err } @@ -35,6 +37,9 @@ func runRulesCheck(args []string) error { printRulesCheckUsage() return nil } + if rulesDir == "" { + rulesDir = os.Getenv("OCR_RULES_DIR") + } rest := a.fs.Args() if len(rest) == 0 { @@ -48,7 +53,10 @@ func runRulesCheck(args []string) error { return err } - resolver, _, err := rules.NewResolver(resolvedRepo, rulePath) + resolver, _, err := rules.NewResolverWithOptions(resolvedRepo, rules.ResolverOptions{ + CustomRulePath: rulePath, + RulesDir: rulesDir, + }) if err != nil { return fmt.Errorf("load rules: %w", err) } @@ -61,10 +69,12 @@ func runRulesCheck(args []string) error { detail := dr.ResolveDetail(strings.ToLower(filePath)) sourceLabel := map[string]string{ - "custom": "Custom (--rule)", - "project": "Project (.opencodereview/rule.json)", - "global": "Global (~/.opencodereview/rule.json)", - "system": "System built-in", + "custom": "Custom (--rule)", + "project": "Project (.opencodereview/rule.json)", + "enterprise_project": "Enterprise project (--rules-dir/projects//rule.json)", + "enterprise_global": "Enterprise global (--rules-dir/global.json)", + "global": "Global (~/.opencodereview/rule.json)", + "system": "System built-in", } fmt.Printf("File: %s\n", filePath) @@ -97,8 +107,11 @@ Show which review rule applies to the given file path, including its source laye Flags: --repo Root directory of the git repository (default: current dir) --rule Path to a custom rule JSON file + --rules-dir + Directory with enterprise/project review rules Examples: ocr rules check src/main/java/com/example/Foo.java - ocr rules check --rule custom.json src/main/resources/mapper/UserMapper.xml`) + ocr rules check --rule custom.json src/main/resources/mapper/UserMapper.xml + ocr rules check --rules-dir /ocr-data/rules src/main/java/com/example/Foo.java`) } diff --git a/cmd/opencodereview/viewer_cmd.go b/cmd/opencodereview/viewer_cmd.go index d2ee060e..6c273477 100644 --- a/cmd/opencodereview/viewer_cmd.go +++ b/cmd/opencodereview/viewer_cmd.go @@ -2,13 +2,15 @@ package main import ( "fmt" + "os" "github.com/open-code-review/open-code-review/internal/viewer" ) type viewerOptions struct { - addr string - showHelp bool + addr string + reviewsDir string + showHelp bool } func parseViewerFlags(args []string) (viewerOptions, error) { @@ -16,12 +18,16 @@ func parseViewerFlags(args []string) (viewerOptions, error) { opts := viewerOptions{} a.StringVar(&opts.addr, "addr", "localhost:5483", "listen address") + a.StringVar(&opts.reviewsDir, "reviews-dir", "", "review result storage root (env: OCR_REVIEWS_DIR, default: ~/.opencodereview/reviews)") if err := a.Parse(args); err != nil { return opts, fmt.Errorf("parse flags: %w", err) } opts.showHelp = a.showHelp + if opts.reviewsDir == "" { + opts.reviewsDir = os.Getenv("OCR_REVIEWS_DIR") + } return opts, nil } @@ -36,7 +42,10 @@ func runViewer(args []string) error { } fmt.Printf("Open Code Review Viewer starting on http://%s\n", opts.addr) - return viewer.StartServer(opts.addr) + return viewer.StartServerWithOptions(viewer.ServerOptions{ + Addr: opts.addr, + ReviewsDir: opts.reviewsDir, + }) } func printViewerUsage() { @@ -48,8 +57,10 @@ Usage: Flags: --addr
listen address (default: localhost:5483) + --reviews-dir review result storage root (env: OCR_REVIEWS_DIR, default: ~/.opencodereview/reviews) Examples: ocr viewer # start on default port - ocr viewer --addr :3000 # bind to all interfaces on port 3000`) + ocr viewer --addr :3000 # bind to all interfaces on port 3000 + ocr viewer --reviews-dir /ocr-data/reviews`) } diff --git a/cmd/opencodereview/viewer_cmd_test.go b/cmd/opencodereview/viewer_cmd_test.go new file mode 100644 index 00000000..ae89b6ff --- /dev/null +++ b/cmd/opencodereview/viewer_cmd_test.go @@ -0,0 +1,27 @@ +package main + +import "testing" + +func TestParseViewerFlagsReviewsDir(t *testing.T) { + opts, err := parseViewerFlags([]string{"--addr", ":3000", "--reviews-dir", "/mnt/ocr/reviews"}) + if err != nil { + t.Fatalf("parseViewerFlags: %v", err) + } + if opts.addr != ":3000" { + t.Errorf("addr = %q", opts.addr) + } + if opts.reviewsDir != "/mnt/ocr/reviews" { + t.Errorf("reviewsDir = %q", opts.reviewsDir) + } +} + +func TestParseViewerFlagsReviewsDirFromEnv(t *testing.T) { + t.Setenv("OCR_REVIEWS_DIR", "/mnt/ocr/reviews") + opts, err := parseViewerFlags(nil) + if err != nil { + t.Fatalf("parseViewerFlags: %v", err) + } + if opts.reviewsDir != "/mnt/ocr/reviews" { + t.Errorf("reviewsDir = %q", opts.reviewsDir) + } +} diff --git a/examples/README.md b/examples/README.md index 8a4caea1..d48cf3e3 100644 --- a/examples/README.md +++ b/examples/README.md @@ -7,4 +7,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 -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. + +The CI examples also show how to persist `ocr review --save-result` output, mount a shared `OCR_REVIEWS_DIR` for a long-running `ocr viewer`, and provide enterprise rules through `OCR_RULES_DIR` / `--rules-dir`. diff --git a/examples/github_actions/README.md b/examples/github_actions/README.md index 272f788b..6aa704e6 100644 --- a/examples/github_actions/README.md +++ b/examples/github_actions/README.md @@ -91,6 +91,52 @@ Use the `--rule` flag to pass a custom rules JSON file: run: ocr review --rule ./my-rules.json --from origin/${{ github.base_ref }} --to origin/${{ github.head_ref }} ``` +For shared enterprise rules, check out or mount a rules directory and pass `--rules-dir`: + +```text +ocr-rules/ + global.json + projects/ + owner/ + repo/ + rule.json +``` + +```yaml +- name: Run OCR review + env: + OCR_PROJECT: owner/repo + run: ocr review --rules-dir ./ocr-rules --from origin/${{ github.base_ref }} --to ${{ github.event.pull_request.head.sha }} +``` + +OCR looks up project rules using `OCR_PROJECT`, then CI provider project variables when available, then the repository directory name. + +### Persist review results + +Use `--save-result` to write final review results as JSON files. In GitHub Actions, upload the result directory as an artifact or sync it to external storage for a long-running `ocr viewer` service. + +```yaml +- name: Run OCR review + env: + OCR_REVIEWS_DIR: /tmp/ocr-reviews + run: | + ocr review \ + --from origin/${{ github.base_ref }} \ + --to ${{ github.event.pull_request.head.sha }} \ + --format json \ + --save-result \ + --result-dir "$OCR_REVIEWS_DIR" \ + --result-project "${{ github.repository }}" \ + --result-source-branch "${{ github.head_ref }}" \ + --result-target-branch "${{ github.base_ref }}" + +- name: Upload OCR review results + uses: actions/upload-artifact@v4 + with: + name: ocr-review-results + path: /tmp/ocr-reviews +``` + ### Limit concurrency Adjust the `--concurrency` flag for large PRs to control the number of concurrent LLM requests: diff --git a/examples/gitlab_ci/README.md b/examples/gitlab_ci/README.md index 6ac630bc..7f1e2fb5 100644 --- a/examples/gitlab_ci/README.md +++ b/examples/gitlab_ci/README.md @@ -13,6 +13,8 @@ MR Created/Updated → GitLab Pipeline Triggered → OCR Reviews Diff → Discus 3. Runs `ocr review --from origin/ --to --format json --audience agent` to analyze the diff (uses commit SHA to support fork MRs) 4. Parses the JSON output and posts inline discussions on the MR using GitLab's Discussions API +To persist review results for the WebUI/API viewer, add `--save-result` to the review command. OCR will automatically read GitLab CI metadata such as project path, MR IID, source branch, target branch, pipeline ID, and job ID from the environment. In containerized runners, use `--result-dir` or `OCR_REVIEWS_DIR` and mount that directory to persistent storage. + ## Setup ### 1. Copy the pipeline file @@ -40,6 +42,8 @@ Go to your project's **Settings → CI/CD → Variables** and add: | `OCR_LLM_AUTH_TOKEN` | Yes | Yes | API authentication token | | `OCR_LLM_MODEL` | No | No | Model name (defaults to `gpt-4o`) | | `GITLAB_API_TOKEN` | No | Yes | GitLab access token with `api` scope (falls back to `CI_JOB_TOKEN` if not set) | +| `OCR_REVIEWS_DIR` | No | No | Mounted directory for saved review results, e.g. `/ocr-data/reviews` | +| `OCR_RULES_DIR` | No | No | Mounted directory for enterprise rules, e.g. `/ocr-data/rules` | > **Note:** GitLab CI/CD does not support variables with values shorter than 8 characters, so `use_anthropic` cannot be set as a CI variable. The pipeline sets it to `false` by default. If you need to use Anthropic Claude models, you'll need to modify the `.gitlab-ci.yml` script directly. > @@ -98,6 +102,61 @@ script: - ocr review --rule ./my-rules.json --from origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME --to $CI_COMMIT_SHA ``` +For shared enterprise rules, mount a rules directory and pass `--rules-dir` or set `OCR_RULES_DIR`: + +```text +/ocr-data/rules/ + global.json + projects/ + group/ + project/ + rule.json +``` + +```yaml +script: + - ocr review --rules-dir /ocr-data/rules --from origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME --to $CI_COMMIT_SHA +``` + +OCR maps GitLab `CI_PROJECT_PATH=group/project` to `/ocr-data/rules/projects/group/project/rule.json`. + +### Persist review results for viewer/API + +If your runner uses containers, save results to a mounted path. The same path should be mounted into a long-running `ocr viewer` process. + +```yaml +variables: + OCR_REVIEWS_DIR: /ocr-data/reviews + OCR_RULES_DIR: /ocr-data/rules + +script: + - | + ocr review \ + --from origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME \ + --to $CI_COMMIT_SHA \ + --format json \ + --audience agent \ + --save-result \ + --result-dir "$OCR_REVIEWS_DIR" \ + --result-project "$CI_PROJECT_PATH" \ + --result-source-branch "$CI_MERGE_REQUEST_SOURCE_BRANCH_NAME" \ + --result-target-branch "$CI_MERGE_REQUEST_TARGET_BRANCH_NAME" +``` + +Run the viewer as a separate service or VM/container with the same storage mounted: + +```bash +ocr viewer --addr :5483 --reviews-dir /ocr-data/reviews +``` + +Useful JSON APIs: + +```text +GET /api/reviews?project=group/project&source=feature/a&target=main +GET /api/reviews/{encodedProject}?source=feature/a&target=main +GET /api/reviews/{encodedProject}/{reviewID} +``` + ### Limit concurrency Adjust the `--concurrency` flag for large MRs to control the number of concurrent LLM requests: @@ -169,13 +228,22 @@ script: # No existing review found - run OCR print("🔍 No existing OCR review found. Running review...") COMMIT_SHA = os.environ["CI_COMMIT_SHA"] - result = subprocess.run([ + cmd = [ "ocr", "review", "--from", f"origin/{TARGET_BRANCH}", "--to", COMMIT_SHA, "--format", "json", - "--audience", "agent" - ], capture_output=True, text=True) + "--audience", "agent", + "--save-result", + "--result-project", os.environ.get("CI_PROJECT_PATH", ""), + "--result-source-branch", SOURCE_BRANCH, + "--result-target-branch", TARGET_BRANCH + ] + if os.environ.get("OCR_REVIEWS_DIR"): + cmd.extend(["--result-dir", os.environ["OCR_REVIEWS_DIR"]]) + if os.environ.get("OCR_RULES_DIR"): + cmd.extend(["--rules-dir", os.environ["OCR_RULES_DIR"]]) + result = subprocess.run(cmd, capture_output=True, text=True) # Save output for the posting script with open("/tmp/ocr-result.json", "w") as f: @@ -265,5 +333,3 @@ script: - cat /tmp/ocr-result.json - cat /tmp/ocr-stderr.log ``` - - diff --git a/internal/config/rules/system_rules.go b/internal/config/rules/system_rules.go index fe0780ba..24f05fd8 100644 --- a/internal/config/rules/system_rules.go +++ b/internal/config/rules/system_rules.go @@ -189,6 +189,11 @@ type ProjectRule struct { Exclude []string `json:"exclude,omitempty"` } +type ResolverOptions struct { + CustomRulePath string + RulesDir string +} + // FileFilter holds the merged user-configured include/exclude glob patterns // collected from all rule.json layers (custom, project, global). type FileFilter struct { @@ -235,28 +240,36 @@ func (f *FileFilter) IsUserIncluded(path string) bool { // composedResolver implements Resolver with layered priority. type composedResolver struct { - custom *ProjectRule // highest: --rule flag - project *ProjectRule // high: .opencodereview/rule.json - global *ProjectRule // low: ~/.opencodereview/rule.json - system *SystemRule // lowest: embedded default + custom *ProjectRule // highest: --rule flag + project *ProjectRule // high: .opencodereview/rule.json + enterpriseProject *ProjectRule // shared: /projects//rule.json + enterpriseGlobal *ProjectRule // shared: /global.json + global *ProjectRule // low: ~/.opencodereview/rule.json + system *SystemRule // lowest: embedded default } // NewResolver builds a Resolver with the following priority: // 1. Custom rule file specified via --rule flag (first match wins) // 2. Project-local .opencodereview/rule.json (first match wins) -// 3. Global ~/.opencodereview/rule.json (first match wins) -// 4. Embedded system default rules +// 3. Enterprise project rule from /projects//rule.json +// 4. Enterprise global rule from /global.json +// 5. Global ~/.opencodereview/rule.json (first match wins) +// 6. Embedded system default rules // // It also returns a FileFilter with the merged include/exclude patterns from all layers. func NewResolver(repoDir, customRulePath string) (Resolver, *FileFilter, error) { + return NewResolverWithOptions(repoDir, ResolverOptions{CustomRulePath: customRulePath}) +} + +func NewResolverWithOptions(repoDir string, opts ResolverOptions) (Resolver, *FileFilter, error) { sysRule, err := LoadDefault() if err != nil { return nil, nil, err } var customRule *ProjectRule - if customRulePath != "" { - cr, err := loadRuleFile(customRulePath) + if opts.CustomRulePath != "" { + cr, err := loadRuleFile(opts.CustomRulePath) if err != nil { return nil, nil, err } @@ -272,18 +285,34 @@ func NewResolver(repoDir, customRulePath string) (Resolver, *FileFilter, error) projectRule = pr } + var enterpriseProjectRule, enterpriseGlobalRule *ProjectRule + if opts.RulesDir != "" { + enterpriseProjectRule, enterpriseGlobalRule, err = loadRulesDir(opts.RulesDir, repoDir) + if err != nil { + return nil, nil, err + } + } + globalRule, err := loadGlobalRule() if err != nil { return nil, nil, err } - filter := buildFileFilter(customRule, projectRule, globalRule) + filter := buildFileFilter(customRule, projectRule, enterpriseProjectRule, enterpriseGlobalRule, globalRule) - return &composedResolver{custom: customRule, project: projectRule, global: globalRule, system: sysRule}, filter, nil + return &composedResolver{ + custom: customRule, + project: projectRule, + enterpriseProject: enterpriseProjectRule, + enterpriseGlobal: enterpriseGlobalRule, + global: globalRule, + system: sysRule, + }, filter, nil } // buildFileFilter picks the highest-priority layer that has any include/exclude -// configured. Priority order: custom (--rule) > project > global. +// configured. Priority order: custom > project > enterprise project > +// enterprise global > global. func buildFileFilter(layers ...*ProjectRule) *FileFilter { for _, pr := range layers { if pr == nil { @@ -324,6 +353,90 @@ func loadGlobalRule() (*ProjectRule, error) { return &pr, nil } +func loadRulesDir(dir, repoDir string) (*ProjectRule, *ProjectRule, error) { + global, err := loadOptionalRuleFile(filepath.Join(dir, "global.json"), "enterprise global rule") + if err != nil { + return nil, nil, err + } + + var project *ProjectRule + for _, path := range enterpriseProjectRulePaths(dir, repoDir) { + pr, err := loadOptionalRuleFile(path, "enterprise project rule") + if err != nil { + return nil, nil, err + } + if pr != nil { + project = pr + break + } + } + return project, global, nil +} + +func enterpriseProjectRulePaths(dir, repoDir string) []string { + seen := map[string]bool{} + var paths []string + add := func(project string) { + project = strings.Trim(project, "/") + if project == "" || seen[project] { + return + } + path, ok := enterpriseProjectRulePath(dir, project) + if !ok { + return + } + seen[project] = true + paths = append(paths, path) + } + add(os.Getenv("OCR_PROJECT")) + add(os.Getenv("CI_PROJECT_PATH")) + if repoDir != "" { + add(filepath.Base(repoDir)) + } + return paths +} + +func enterpriseProjectRulePath(dir, project string) (string, bool) { + parts := strings.Split(project, "/") + for _, part := range parts { + if !isSafeRulePathSegment(part) { + return "", false + } + } + base := filepath.Clean(filepath.Join(dir, "projects")) + path := filepath.Clean(filepath.Join(append([]string{base}, append(parts, "rule.json")...)...)) + rel, err := filepath.Rel(base, path) + if err != nil || rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) || filepath.IsAbs(rel) { + return "", false + } + return path, true +} + +func isSafeRulePathSegment(segment string) bool { + if segment == "" || segment == "." || segment == ".." { + return false + } + if strings.Contains(segment, `\`) || filepath.IsAbs(segment) || filepath.Base(segment) != segment { + return false + } + return true +} + +func loadOptionalRuleFile(path, label string) (*ProjectRule, error) { + data, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, fmt.Errorf("read %s %s: %w", label, path, err) + } + var pr ProjectRule + if err := json.Unmarshal(data, &pr); err != nil { + return nil, fmt.Errorf("unmarshal %s %s: %w", label, path, err) + } + return &pr, nil +} + func loadRuleFile(path string) (*ProjectRule, error) { data, err := os.ReadFile(path) if err != nil { @@ -360,6 +473,12 @@ func (c *composedResolver) Resolve(path string) string { if rule := matchProjectRule(c.project, path); rule != "" { return rule } + if rule := matchProjectRule(c.enterpriseProject, path); rule != "" { + return rule + } + if rule := matchProjectRule(c.enterpriseGlobal, path); rule != "" { + return rule + } if rule := matchProjectRule(c.global, path); rule != "" { return rule } @@ -374,6 +493,12 @@ func (c *composedResolver) ResolveDetail(path string) RuleDetail { if detail := matchProjectRuleDetail(c.project, path, "project"); detail != nil { return *detail } + if detail := matchProjectRuleDetail(c.enterpriseProject, path, "enterprise_project"); detail != nil { + return *detail + } + if detail := matchProjectRuleDetail(c.enterpriseGlobal, path, "enterprise_global"); detail != nil { + return *detail + } if detail := matchProjectRuleDetail(c.global, path, "global"); detail != nil { return *detail } diff --git a/internal/config/rules/system_rules_test.go b/internal/config/rules/system_rules_test.go index e99391ab..edc761d6 100644 --- a/internal/config/rules/system_rules_test.go +++ b/internal/config/rules/system_rules_test.go @@ -339,6 +339,96 @@ func TestNewResolver_CustomOverridesProject(t *testing.T) { } } +func TestNewResolver_RulesDirProjectAndGlobal(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("CI_PROJECT_PATH", "group/project") + + repoDir := t.TempDir() + rulesDir := t.TempDir() + projectDir := filepath.Join(rulesDir, "projects", "group", "project") + if err := os.MkdirAll(projectDir, 0o755); err != nil { + t.Fatalf("mkdir project rules: %v", err) + } + if err := os.WriteFile(filepath.Join(projectDir, "rule.json"), []byte(`{"rules":[{"path":"src/**/*.go","rule":"enterprise-project-go"}]}`), 0o644); err != nil { + t.Fatalf("write project rule: %v", err) + } + if err := os.WriteFile(filepath.Join(rulesDir, "global.json"), []byte(`{"rules":[{"path":"**/*.go","rule":"enterprise-global-go"},{"path":"**/*.md","rule":"enterprise-global-md"}]}`), 0o644); err != nil { + t.Fatalf("write global rule: %v", err) + } + + resolver, _, err := NewResolverWithOptions(repoDir, ResolverOptions{RulesDir: rulesDir}) + if err != nil { + t.Fatalf("NewResolverWithOptions: %v", err) + } + + if got := resolver.Resolve("src/main.go"); got != "enterprise-project-go" { + t.Errorf("project rule = %q", got) + } + if got := resolver.Resolve("cmd/main.go"); got != "enterprise-global-go" { + t.Errorf("global rule = %q", got) + } + if got := resolver.Resolve("README.md"); got != "enterprise-global-md" { + t.Errorf("global md rule = %q", got) + } +} + +func TestEnterpriseProjectRulePathsRejectTraversal(t *testing.T) { + t.Setenv("OCR_PROJECT", "../../outside") + t.Setenv("CI_PROJECT_PATH", `group\project`) + + rulesDir := t.TempDir() + paths := enterpriseProjectRulePaths(rulesDir, "") + if len(paths) != 0 { + t.Fatalf("paths = %v, want none", paths) + } +} + +func TestEnterpriseProjectRulePathsKeepsPathUnderProjectsDir(t *testing.T) { + t.Setenv("OCR_PROJECT", "group/project") + t.Setenv("CI_PROJECT_PATH", "") + + rulesDir := t.TempDir() + paths := enterpriseProjectRulePaths(rulesDir, "") + if len(paths) != 1 { + t.Fatalf("paths len = %d, want 1: %v", len(paths), paths) + } + want := filepath.Join(rulesDir, "projects", "group", "project", "rule.json") + if paths[0] != want { + t.Fatalf("path = %q, want %q", paths[0], want) + } +} + +func TestNewResolver_ProjectRuleOverridesRulesDir(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("CI_PROJECT_PATH", "group/project") + + repoDir := t.TempDir() + ocrDir := filepath.Join(repoDir, ".opencodereview") + if err := os.MkdirAll(ocrDir, 0o755); err != nil { + t.Fatalf("mkdir project rule dir: %v", err) + } + if err := os.WriteFile(filepath.Join(ocrDir, "rule.json"), []byte(`{"rules":[{"path":"**/*.go","rule":"repo-go"}]}`), 0o644); err != nil { + t.Fatalf("write project rule: %v", err) + } + + rulesDir := t.TempDir() + enterpriseDir := filepath.Join(rulesDir, "projects", "group", "project") + if err := os.MkdirAll(enterpriseDir, 0o755); err != nil { + t.Fatalf("mkdir enterprise rule dir: %v", err) + } + if err := os.WriteFile(filepath.Join(enterpriseDir, "rule.json"), []byte(`{"rules":[{"path":"**/*.go","rule":"enterprise-go"}]}`), 0o644); err != nil { + t.Fatalf("write enterprise rule: %v", err) + } + + resolver, _, err := NewResolverWithOptions(repoDir, ResolverOptions{RulesDir: rulesDir}) + if err != nil { + t.Fatalf("NewResolverWithOptions: %v", err) + } + if got := resolver.Resolve("main.go"); got != "repo-go" { + t.Errorf("expected repo-go, got %q", got) + } +} + func TestNewResolver_ProjectFileMalformed(t *testing.T) { dir := t.TempDir() ocrDir := filepath.Join(dir, ".opencodereview") diff --git a/internal/reviewstore/store.go b/internal/reviewstore/store.go new file mode 100644 index 00000000..c8c5b5e1 --- /dev/null +++ b/internal/reviewstore/store.go @@ -0,0 +1,365 @@ +package reviewstore + +import ( + "crypto/rand" + "encoding/json" + "fmt" + "io" + "log" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/open-code-review/open-code-review/internal/model" +) + +type ProjectInfo struct { + ID string `json:"id,omitempty"` + Name string `json:"name,omitempty"` + RepoDir string `json:"repo_dir,omitempty"` + WebURL string `json:"web_url,omitempty"` +} + +type GitLabInfo struct { + ServerURL string `json:"server_url,omitempty"` + ProjectID string `json:"project_id,omitempty"` + MergeRequestIID string `json:"merge_request_iid,omitempty"` + PipelineID string `json:"pipeline_id,omitempty"` + JobID string `json:"job_id,omitempty"` +} + +type ReviewInfo struct { + Mode string `json:"mode,omitempty"` + SourceBranch string `json:"source_branch,omitempty"` + TargetBranch string `json:"target_branch,omitempty"` + From string `json:"from,omitempty"` + To string `json:"to,omitempty"` + Commit string `json:"commit,omitempty"` + Model string `json:"model,omitempty"` + FilesReviewed int64 `json:"files_reviewed"` + CommentCount int64 `json:"comments"` + TotalTokens int64 `json:"total_tokens"` + InputTokens int64 `json:"input_tokens"` + OutputTokens int64 `json:"output_tokens"` + CacheReadTokens int64 `json:"cache_read_tokens,omitempty"` + CacheWriteTokens int64 `json:"cache_write_tokens,omitempty"` + Duration string `json:"duration,omitempty"` + DurationSeconds int64 `json:"duration_seconds,omitempty"` + SessionID string `json:"session_id,omitempty"` +} + +type Warning struct { + File string `json:"file,omitempty"` + Message string `json:"message,omitempty"` + Type string `json:"type,omitempty"` +} + +type Result struct { + ID string `json:"id"` + CreatedAt time.Time `json:"created_at"` + Project ProjectInfo `json:"project"` + GitLab GitLabInfo `json:"gitlab,omitempty"` + Review ReviewInfo `json:"review"` + Comments []model.LlmComment `json:"comments"` + Warnings []Warning `json:"warnings,omitempty"` +} + +type ProjectSummary struct { + EncodedKey string + Project ProjectInfo + ReviewCount int + LastReview time.Time +} + +type ReviewSummary struct { + ID string + CreatedAt time.Time + Project ProjectInfo + GitLab GitLabInfo + Review ReviewInfo +} + +type reviewSummaryFile struct { + ID string `json:"id"` + CreatedAt time.Time `json:"created_at"` + Project ProjectInfo `json:"project"` + GitLab GitLabInfo `json:"gitlab,omitempty"` + Review ReviewInfo `json:"review"` +} + +type ReviewFilter struct { + Project string + SourceBranch string + TargetBranch string +} + +func DefaultRoot() (string, error) { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("resolve home dir: %w", err) + } + return filepath.Join(home, ".opencodereview", "reviews"), nil +} + +func Save(root string, result Result) (string, error) { + if root == "" { + var err error + root, err = DefaultRoot() + if err != nil { + return "", err + } + } + if result.ID == "" { + id, err := generateID() + if err != nil { + return "", err + } + result.ID = id + } + if result.CreatedAt.IsZero() { + result.CreatedAt = time.Now().UTC() + } + if result.Project.Name == "" { + result.Project.Name = result.Project.RepoDir + } + projectKey := ProjectKey(result.Project) + dir := filepath.Join(root, projectKey) + if err := os.MkdirAll(dir, 0700); err != nil { + return "", fmt.Errorf("create review result dir: %w", err) + } + + path := filepath.Join(dir, result.ID+".json") + data, err := json.MarshalIndent(result, "", " ") + if err != nil { + return "", fmt.Errorf("marshal review result: %w", err) + } + if err := os.WriteFile(path, append(data, '\n'), 0600); err != nil { + return "", fmt.Errorf("write review result: %w", err) + } + return path, nil +} + +func ListProjects(root string) ([]ProjectSummary, error) { + entries, err := os.ReadDir(root) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, fmt.Errorf("read reviews dir: %w", err) + } + + var projects []ProjectSummary + for _, entry := range entries { + if !entry.IsDir() { + continue + } + reviews, err := ListReviews(root, entry.Name(), ReviewFilter{}) + if err != nil || len(reviews) == 0 { + continue + } + summary := ProjectSummary{ + EncodedKey: entry.Name(), + Project: reviews[0].Project, + ReviewCount: len(reviews), + LastReview: reviews[0].CreatedAt, + } + projects = append(projects, summary) + } + + sort.Slice(projects, func(i, j int) bool { + return projects[i].LastReview.After(projects[j].LastReview) + }) + return projects, nil +} + +func ListReviews(root, encodedProject string, filter ReviewFilter) ([]ReviewSummary, error) { + if !isSafePathSegment(encodedProject) { + return nil, fmt.Errorf("invalid review path") + } + dir := filepath.Join(root, encodedProject) + entries, err := os.ReadDir(dir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, fmt.Errorf("read project review dir: %w", err) + } + + var reviews []ReviewSummary + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".json") { + continue + } + review, err := loadSummary(root, encodedProject, strings.TrimSuffix(entry.Name(), ".json")) + if err != nil { + log.Printf("[ocr] warning: skip review result %s/%s: %v", encodedProject, entry.Name(), err) + continue + } + if filter.SourceBranch != "" && review.Review.SourceBranch != filter.SourceBranch { + continue + } + if filter.TargetBranch != "" && review.Review.TargetBranch != filter.TargetBranch { + continue + } + reviews = append(reviews, review) + } + + sort.Slice(reviews, func(i, j int) bool { + return reviews[i].CreatedAt.After(reviews[j].CreatedAt) + }) + return reviews, nil +} + +func ListAllReviews(root string, filter ReviewFilter) ([]ReviewSummary, error) { + entries, err := os.ReadDir(root) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, fmt.Errorf("read reviews dir: %w", err) + } + + var reviews []ReviewSummary + for _, entry := range entries { + if !entry.IsDir() { + continue + } + projectReviews, err := ListReviews(root, entry.Name(), ReviewFilter{ + SourceBranch: filter.SourceBranch, + TargetBranch: filter.TargetBranch, + }) + if err != nil { + return nil, err + } + for _, review := range projectReviews { + if filter.Project != "" && !matchesProjectFilter(entry.Name(), review.Project, filter.Project) { + continue + } + reviews = append(reviews, review) + } + } + + sort.Slice(reviews, func(i, j int) bool { + return reviews[i].CreatedAt.After(reviews[j].CreatedAt) + }) + return reviews, nil +} + +func Load(root, encodedProject, reviewID string) (*Result, error) { + path, err := reviewResultPath(root, encodedProject, reviewID) + if err != nil { + return nil, fmt.Errorf("invalid review path") + } + f, err := os.Open(path) + if err != nil { + return nil, fmt.Errorf("open review result: %w", err) + } + defer f.Close() + + var result Result + dec := json.NewDecoder(f) + if err := dec.Decode(&result); err != nil { + return nil, fmt.Errorf("decode review result: %w", err) + } + return &result, nil +} + +func loadSummary(root, encodedProject, reviewID string) (ReviewSummary, error) { + path, err := reviewResultPath(root, encodedProject, reviewID) + if err != nil { + return ReviewSummary{}, fmt.Errorf("invalid review path") + } + f, err := os.Open(path) + if err != nil { + return ReviewSummary{}, fmt.Errorf("open review result: %w", err) + } + defer f.Close() + + var result reviewSummaryFile + dec := json.NewDecoder(f) + if err := dec.Decode(&result); err != nil { + return ReviewSummary{}, fmt.Errorf("decode review result summary: %w", err) + } + return ReviewSummary{ + ID: result.ID, + CreatedAt: result.CreatedAt, + Project: result.Project, + GitLab: result.GitLab, + Review: result.Review, + }, nil +} + +func ProjectKey(project ProjectInfo) string { + key := project.ID + if key == "" { + key = project.Name + } + if key == "" { + key = project.RepoDir + } + if key == "" { + key = "unknown" + } + return encodeProjectKey(key) +} + +func encodeProjectKey(key string) string { + if key == "" { + return "empty" + } + vol := filepath.VolumeName(key) + key = key[len(vol):] + key = strings.TrimLeft(key, "/\\") + key = strings.ReplaceAll(key, "/", "-") + key = strings.ReplaceAll(key, "\\", "-") + vol = strings.ReplaceAll(vol, ":", "_") + result := vol + key + if result == "" { + return "empty" + } + return result +} + +func matchesProjectFilter(encodedProject string, project ProjectInfo, filter string) bool { + return filter == encodedProject || filter == project.ID || filter == project.Name || filter == project.RepoDir +} + +func reviewResultPath(root, encodedProject, reviewID string) (string, error) { + if !isSafePathSegment(encodedProject) || !isSafePathSegment(reviewID) { + return "", fmt.Errorf("invalid review path") + } + root = filepath.Clean(root) + path := filepath.Clean(filepath.Join(root, encodedProject, reviewID+".json")) + rel, err := filepath.Rel(root, path) + if err != nil || rel == "." || rel == ".." || strings.HasPrefix(rel, ".."+string(os.PathSeparator)) || filepath.IsAbs(rel) { + return "", fmt.Errorf("invalid review path") + } + return path, nil +} + +func isSafePathSegment(segment string) bool { + if segment == "" || segment == "." || segment == ".." { + return false + } + if strings.Contains(segment, "/") || strings.Contains(segment, `\`) { + return false + } + return !filepath.IsAbs(segment) && filepath.Base(segment) == segment +} + +func generateID() (string, error) { + return generateIDFromReader(rand.Reader) +} + +func generateIDFromReader(reader io.Reader) (string, error) { + b := make([]byte, 16) + if _, err := io.ReadFull(reader, b); err != nil { + return "", fmt.Errorf("generate review id: %w", err) + } + b[6] = (b[6] & 0x0f) | 0x40 + b[8] = (b[8] & 0x3f) | 0x80 + return fmt.Sprintf("%08x-%04x-%04x-%04x-%012x", + b[0:4], b[4:6], b[6:8], b[8:10], b[10:]), nil +} diff --git a/internal/reviewstore/store_test.go b/internal/reviewstore/store_test.go new file mode 100644 index 00000000..162b16b1 --- /dev/null +++ b/internal/reviewstore/store_test.go @@ -0,0 +1,220 @@ +package reviewstore + +import ( + "strings" + "testing" + "time" + + "github.com/open-code-review/open-code-review/internal/model" +) + +func TestSaveListLoadResult(t *testing.T) { + root := t.TempDir() + result := Result{ + Project: ProjectInfo{ + ID: "42", + Name: "group/project", + RepoDir: "/repo/project", + }, + Review: ReviewInfo{ + Mode: "range", + SourceBranch: "feature/a", + TargetBranch: "main", + CommentCount: 1, + FilesReviewed: 2, + }, + Comments: []model.LlmComment{{ + Path: "main.go", + Content: "check this", + StartLine: 10, + EndLine: 10, + }}, + } + + if _, err := Save(root, result); err != nil { + t.Fatalf("Save: %v", err) + } + + projects, err := ListProjects(root) + if err != nil { + t.Fatalf("ListProjects: %v", err) + } + if len(projects) != 1 { + t.Fatalf("projects len = %d, want 1", len(projects)) + } + if projects[0].Project.Name != "group/project" { + t.Fatalf("project name = %q", projects[0].Project.Name) + } + + reviews, err := ListReviews(root, projects[0].EncodedKey, ReviewFilter{SourceBranch: "feature/a", TargetBranch: "main"}) + if err != nil { + t.Fatalf("ListReviews: %v", err) + } + if len(reviews) != 1 { + t.Fatalf("reviews len = %d, want 1", len(reviews)) + } + if reviews[0].Review.CommentCount != 1 { + t.Fatalf("comment count = %d, want 1", reviews[0].Review.CommentCount) + } + + loaded, err := Load(root, projects[0].EncodedKey, reviews[0].ID) + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(loaded.Comments) != 1 || loaded.Comments[0].Path != "main.go" { + t.Fatalf("unexpected comments: %#v", loaded.Comments) + } +} + +func TestListReviewsFiltersBranches(t *testing.T) { + root := t.TempDir() + project := ProjectInfo{ID: "p1", Name: "p1"} + for _, source := range []string{"feature/a", "feature/b"} { + _, err := Save(root, Result{ + CreatedAt: time.Now().UTC(), + Project: project, + Review: ReviewInfo{ + SourceBranch: source, + TargetBranch: "main", + }, + }) + if err != nil { + t.Fatalf("Save: %v", err) + } + } + + reviews, err := ListReviews(root, ProjectKey(project), ReviewFilter{SourceBranch: "feature/a"}) + if err != nil { + t.Fatalf("ListReviews: %v", err) + } + if len(reviews) != 1 { + t.Fatalf("reviews len = %d, want 1", len(reviews)) + } + if reviews[0].Review.SourceBranch != "feature/a" { + t.Fatalf("source = %q", reviews[0].Review.SourceBranch) + } +} + +func TestListAllReviewsFiltersProjectAndBranches(t *testing.T) { + root := t.TempDir() + results := []Result{ + { + Project: ProjectInfo{ID: "42", Name: "group/project"}, + Review: ReviewInfo{SourceBranch: "feature/a", TargetBranch: "main"}, + }, + { + Project: ProjectInfo{ID: "43", Name: "group/other"}, + Review: ReviewInfo{SourceBranch: "feature/a", TargetBranch: "main"}, + }, + { + Project: ProjectInfo{ID: "42", Name: "group/project"}, + Review: ReviewInfo{SourceBranch: "feature/b", TargetBranch: "main"}, + }, + } + for _, result := range results { + if _, err := Save(root, result); err != nil { + t.Fatalf("Save: %v", err) + } + } + + reviews, err := ListAllReviews(root, ReviewFilter{ + Project: "group/project", + SourceBranch: "feature/a", + TargetBranch: "main", + }) + if err != nil { + t.Fatalf("ListAllReviews: %v", err) + } + if len(reviews) != 1 { + t.Fatalf("reviews len = %d, want 1", len(reviews)) + } + if reviews[0].Project.Name != "group/project" || reviews[0].Review.SourceBranch != "feature/a" { + t.Fatalf("unexpected review: %#v", reviews[0]) + } +} + +func TestLoadRejectsUnsafePathSegments(t *testing.T) { + root := t.TempDir() + tests := []struct { + name string + project string + review string + }{ + {name: "project traversal", project: "..", review: "review-id"}, + {name: "review traversal", project: "project", review: "../review-id"}, + {name: "project slash", project: "project/child", review: "review-id"}, + {name: "review slash", project: "project", review: "dir/review-id"}, + {name: "project backslash", project: `project\child`, review: "review-id"}, + {name: "review backslash", project: "project", review: `dir\review-id`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, err := Load(root, tt.project, tt.review); err == nil { + t.Fatal("Load succeeded, want invalid path error") + } + }) + } +} + +func TestListReviewsRejectsUnsafeProjectSegment(t *testing.T) { + root := t.TempDir() + for _, project := range []string{"..", "../project", "project/child", `project\child`} { + t.Run(project, func(t *testing.T) { + if _, err := ListReviews(root, project, ReviewFilter{}); err == nil { + t.Fatal("ListReviews succeeded, want invalid path error") + } + }) + } +} + +func TestProjectKeyUsesReadablePathEncoding(t *testing.T) { + tests := []struct { + name string + project ProjectInfo + want string + }{ + { + name: "project name path", + project: ProjectInfo{Name: "group/project"}, + want: "group-project", + }, + { + name: "repo dir path", + project: ProjectInfo{RepoDir: "/Users/kite/Desktop/my-project"}, + want: "Users-kite-Desktop-my-project", + }, + { + name: "mixed separators", + project: ProjectInfo{Name: `group\project/service`}, + want: "group-project-service", + }, + { + name: "dotted name", + project: ProjectInfo{Name: "my..project"}, + want: "my..project", + }, + { + name: "fallback unknown", + project: ProjectInfo{}, + want: "unknown", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ProjectKey(tt.project); got != tt.want { + t.Fatalf("ProjectKey() = %q, want %q", got, tt.want) + } + if !isSafePathSegment(ProjectKey(tt.project)) { + t.Fatalf("ProjectKey() produced unsafe segment %q", ProjectKey(tt.project)) + } + }) + } +} + +func TestGenerateIDPropagatesRandomReadError(t *testing.T) { + if _, err := generateIDFromReader(strings.NewReader("short")); err == nil { + t.Fatal("generateIDFromReader succeeded, want error") + } +} diff --git a/internal/viewer/review_handler.go b/internal/viewer/review_handler.go new file mode 100644 index 00000000..8d4477c8 --- /dev/null +++ b/internal/viewer/review_handler.go @@ -0,0 +1,149 @@ +package viewer + +import ( + "encoding/json" + "fmt" + "net/http" + + "github.com/open-code-review/open-code-review/internal/reviewstore" +) + +func ReviewsRoot() (string, error) { + return reviewstore.DefaultRoot() +} + +type reviewProjectsData struct { + Projects []reviewstore.ProjectSummary +} + +func handleReviewProjects(w http.ResponseWriter, r *http.Request, root string) { + if r.URL.Path != "/reviews" { + http.NotFound(w, r) + return + } + projects, err := reviewstore.ListProjects(root) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + renderTemplate(w, "review_projects.html", reviewProjectsData{Projects: projects}) +} + +type projectReviewsData struct { + EncodedProject string + ProjectName string + SourceBranch string + TargetBranch string + Reviews []reviewstore.ReviewSummary +} + +func handleProjectReviews(w http.ResponseWriter, r *http.Request, root, project string) { + source := r.URL.Query().Get("source") + target := r.URL.Query().Get("target") + reviews, err := reviewstore.ListReviews(root, project, reviewstore.ReviewFilter{ + SourceBranch: source, + TargetBranch: target, + }) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + name := project + for _, review := range reviews { + if review.Project.Name != "" { + name = review.Project.Name + break + } + } + + renderTemplate(w, "project_reviews.html", projectReviewsData{ + EncodedProject: project, + ProjectName: name, + SourceBranch: source, + TargetBranch: target, + Reviews: reviews, + }) +} + +type reviewDetailData struct { + EncodedProject string + ProjectName string + Result *reviewstore.Result +} + +func handleReviewDetail(w http.ResponseWriter, r *http.Request, root, project, reviewID string) { + result, err := reviewstore.Load(root, project, reviewID) + if err != nil { + http.Error(w, fmt.Sprintf("Failed to load review result: %v", err), http.StatusNotFound) + return + } + name := result.Project.Name + if name == "" { + name = project + } + renderTemplate(w, "review_detail.html", reviewDetailData{ + EncodedProject: project, + ProjectName: name, + Result: result, + }) +} + +func handleAPIReviews(w http.ResponseWriter, r *http.Request, root string) { + if r.URL.Path != "/api/reviews" { + http.NotFound(w, r) + return + } + reviews, err := reviewstore.ListAllReviews(root, reviewstore.ReviewFilter{ + Project: r.URL.Query().Get("project"), + SourceBranch: r.URL.Query().Get("source"), + TargetBranch: r.URL.Query().Get("target"), + }) + if err != nil { + writeJSONError(w, http.StatusInternalServerError, err) + return + } + writeJSON(w, http.StatusOK, map[string]any{ + "reviews": reviews, + }) +} + +func handleAPIProjectReviews(w http.ResponseWriter, r *http.Request, root, project string) { + reviews, err := reviewstore.ListReviews(root, project, reviewstore.ReviewFilter{ + SourceBranch: r.URL.Query().Get("source"), + TargetBranch: r.URL.Query().Get("target"), + }) + if err != nil { + writeJSONError(w, http.StatusInternalServerError, err) + return + } + writeJSON(w, http.StatusOK, map[string]any{ + "project": project, + "reviews": reviews, + }) +} + +func handleAPIReviewDetail(w http.ResponseWriter, r *http.Request, root, project, reviewID string) { + result, err := reviewstore.Load(root, project, reviewID) + if err != nil { + writeJSONError(w, http.StatusNotFound, err) + return + } + writeJSON(w, http.StatusOK, result) +} + +func writeJSON(w http.ResponseWriter, status int, value any) { + w.Header().Set("Content-Type", "application/json; charset=utf-8") + w.WriteHeader(status) + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + if err := enc.Encode(value); err != nil { + fmt.Printf("[viewer] json encode error: %v\n", err) + } +} + +func writeJSONError(w http.ResponseWriter, status int, err error) { + writeJSON(w, status, map[string]string{ + "error": err.Error(), + }) +} diff --git a/internal/viewer/server.go b/internal/viewer/server.go index 38038601..7a694463 100644 --- a/internal/viewer/server.go +++ b/internal/viewer/server.go @@ -6,6 +6,7 @@ import ( "html/template" "io/fs" "net/http" + "path/filepath" "strings" "time" ) @@ -14,10 +15,30 @@ import ( var assets embed.FS func StartServer(addr string) error { + return StartServerWithOptions(ServerOptions{Addr: addr}) +} + +type ServerOptions struct { + Addr string + ReviewsDir string +} + +func StartServerWithOptions(opts ServerOptions) error { + addr := opts.Addr + if addr == "" { + addr = "localhost:5483" + } root, err := SessionsRoot() if err != nil { return fmt.Errorf("resolve sessions root: %w", err) } + reviewsRoot := opts.ReviewsDir + if reviewsRoot == "" { + reviewsRoot, err = ReviewsRoot() + if err != nil { + return fmt.Errorf("resolve reviews root: %w", err) + } + } mux := http.NewServeMux() @@ -25,12 +46,52 @@ func StartServer(addr string) error { mux.Handle("/static/", http.StripPrefix("/static/", http.FileServer(http.FS(staticFS())))) // Routes + mux.HandleFunc("/reviews", func(w http.ResponseWriter, r *http.Request) { + handleReviewProjects(w, r, reviewsRoot) + }) + mux.HandleFunc("/reviews/{project}", func(w http.ResponseWriter, r *http.Request) { + project := r.PathValue("project") + if !isValidPathSegment(project) { + http.Error(w, "invalid project path", http.StatusBadRequest) + return + } + handleProjectReviews(w, r, reviewsRoot, project) + }) + mux.HandleFunc("/reviews/{project}/{reviewID}", func(w http.ResponseWriter, r *http.Request) { + project := r.PathValue("project") + reviewID := r.PathValue("reviewID") + if !isValidPathSegment(project) || !isValidPathSegment(reviewID) { + http.Error(w, "invalid path", http.StatusBadRequest) + return + } + handleReviewDetail(w, r, reviewsRoot, project, reviewID) + }) + mux.HandleFunc("/api/reviews", func(w http.ResponseWriter, r *http.Request) { + handleAPIReviews(w, r, reviewsRoot) + }) + mux.HandleFunc("/api/reviews/{project}", func(w http.ResponseWriter, r *http.Request) { + project := r.PathValue("project") + if !isValidPathSegment(project) { + http.Error(w, "invalid project path", http.StatusBadRequest) + return + } + handleAPIProjectReviews(w, r, reviewsRoot, project) + }) + mux.HandleFunc("/api/reviews/{project}/{reviewID}", func(w http.ResponseWriter, r *http.Request) { + project := r.PathValue("project") + reviewID := r.PathValue("reviewID") + if !isValidPathSegment(project) || !isValidPathSegment(reviewID) { + http.Error(w, "invalid path", http.StatusBadRequest) + return + } + handleAPIReviewDetail(w, r, reviewsRoot, project, reviewID) + }) mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { handleRepos(w, r, root) }) mux.HandleFunc("/r/{repo}", func(w http.ResponseWriter, r *http.Request) { repo := r.PathValue("repo") - if strings.Contains(repo, "..") || strings.Contains(repo, "/") { + if !isValidPathSegment(repo) { http.Error(w, "invalid repo path", http.StatusBadRequest) return } @@ -39,7 +100,7 @@ func StartServer(addr string) error { mux.HandleFunc("/r/{repo}/{sessionID}", func(w http.ResponseWriter, r *http.Request) { repo := r.PathValue("repo") sid := r.PathValue("sessionID") - if strings.Contains(repo, "..") || strings.Contains(sid, "..") { + if !isValidPathSegment(repo) || !isValidPathSegment(sid) { http.Error(w, "invalid path", http.StatusBadRequest) return } @@ -62,6 +123,16 @@ func StartServer(addr string) error { return srv.ListenAndServe() } +func isValidPathSegment(segment string) bool { + return segment != "" && + segment != "." && + segment != ".." && + !strings.Contains(segment, "/") && + !strings.Contains(segment, `\`) && + !filepath.IsAbs(segment) && + filepath.Base(segment) == segment +} + var cstZone = func() *time.Location { loc, err := time.LoadLocation("Asia/Shanghai") if err != nil { diff --git a/internal/viewer/static/style.css b/internal/viewer/static/style.css index e008eafc..e3e7e2f4 100644 --- a/internal/viewer/static/style.css +++ b/internal/viewer/static/style.css @@ -25,6 +25,53 @@ main { max-width: 1200px; margin: 1.5rem auto; padding: 0 1rem; } h2 { margin-bottom: 1rem; color: #24292e; } h3 { margin: 1.5rem 0 0.75rem; color: #24292e; } +.page-actions { + margin-bottom: 1rem; +} +.page-actions a { + color: #0366d6; + text-decoration: none; + font-weight: 600; +} +.page-actions a:hover { text-decoration: underline; } + +.filter-bar { + background: #fff; + border: 1px solid #e1e4e8; + border-radius: 6px; + padding: 0.85rem 1rem; + margin-bottom: 1rem; + display: flex; + flex-wrap: wrap; + gap: 0.75rem; + align-items: center; + box-shadow: 0 1px 3px rgba(0,0,0,0.08); +} +.filter-bar label { + color: #586069; + font-size: 0.85rem; +} +.filter-bar input { + margin-left: 0.35rem; + padding: 0.35rem 0.5rem; + border: 1px solid #d0d7de; + border-radius: 4px; + min-width: 180px; +} +.filter-bar button { + border: 1px solid #1f6feb; + background: #0969da; + color: #fff; + border-radius: 4px; + padding: 0.38rem 0.8rem; + cursor: pointer; +} +.filter-bar a { + color: #0366d6; + text-decoration: none; + font-size: 0.9rem; +} + .table { width: 100%; border-collapse: collapse; @@ -130,6 +177,45 @@ h3 { margin: 1.5rem 0 0.75rem; color: #24292e; } /* ── Conversations Section ── */ .conversations { margin-top: 0.5rem; } +.review-comment { + background: #fff; + border: 1px solid #e1e4e8; + border-radius: 8px; + margin-bottom: 0.75rem; + overflow: hidden; + box-shadow: 0 1px 3px rgba(0,0,0,0.08); +} +.review-comment-header { + background: #f6f8fa; + border-bottom: 1px solid #eaecef; + padding: 0.75rem 1rem; + display: flex; + gap: 0.75rem; + align-items: center; +} +.review-comment-body { + padding: 1rem; +} +.review-comment-body p { + margin-bottom: 0.85rem; + white-space: pre-wrap; +} +.code-pair { + display: grid; + grid-template-columns: repeat(auto-fit, minmax(280px, 1fr)); + gap: 0.75rem; +} +.code-pair pre { + background: #f6f8fa; + border: 1px solid #e1e4e8; + border-radius: 6px; + padding: 0.75rem; + overflow-x: auto; + overflow-y: auto; + max-height: 400px; + font-size: 0.82rem; +} + /* File Accordion */ .file-accordion { background: #fff; diff --git a/internal/viewer/templates/project_reviews.html b/internal/viewer/templates/project_reviews.html new file mode 100644 index 00000000..7aaccc17 --- /dev/null +++ b/internal/viewer/templates/project_reviews.html @@ -0,0 +1,44 @@ + + + + + + Project Reviews - Open Code Review Viewer + + + + +
+

Reviews: {{.ProjectName}}

+ +
+ + + + Reset +
+ +{{if .Reviews}} + + + + {{range .Reviews}} + + + + + + + + + + + {{end}} + +
ReviewSourceTargetModeCommentsFilesModelCreated At
{{.ID | truncate 8}}…{{if .Review.SourceBranch}}{{.Review.SourceBranch}}{{else}}-{{end}}{{if .Review.TargetBranch}}{{.Review.TargetBranch}}{{else}}-{{end}}{{if .Review.Mode}}{{.Review.Mode}}{{else}}-{{end}}{{.Review.CommentCount}}{{.Review.FilesReviewed}}{{if .Review.Model}}{{.Review.Model}}{{else}}-{{end}}{{formatTime .CreatedAt}}
+{{else}} +

No review results matched this filter.

+{{end}} +
+ + diff --git a/internal/viewer/templates/repos.html b/internal/viewer/templates/repos.html index 4a37f24a..7b7d6a1a 100644 --- a/internal/viewer/templates/repos.html +++ b/internal/viewer/templates/repos.html @@ -9,6 +9,7 @@
+

Review Results

Repositories

{{if .Repos}} diff --git a/internal/viewer/templates/review_detail.html b/internal/viewer/templates/review_detail.html new file mode 100644 index 00000000..40325036 --- /dev/null +++ b/internal/viewer/templates/review_detail.html @@ -0,0 +1,101 @@ + + + + + + Review Detail - Open Code Review Viewer + + + + +
+
+

Review: {{.Result.ID}}

+
+ Project: {{.ProjectName}} + Mode: {{if .Result.Review.Mode}}{{.Result.Review.Mode}}{{else}}-{{end}} + {{if .Result.Review.SourceBranch}}Source: {{.Result.Review.SourceBranch}}{{end}} + {{if .Result.Review.TargetBranch}}Target: {{.Result.Review.TargetBranch}}{{end}} + {{if .Result.Review.From}}From: {{.Result.Review.From}}{{end}} + {{if .Result.Review.To}}To: {{.Result.Review.To}}{{end}} + {{if .Result.Review.Commit}}Commit: {{.Result.Review.Commit}}{{end}} + Model: {{.Result.Review.Model}} + Created: {{formatTime .Result.CreatedAt}} +
+
+ +
+

Summary

+
+
+
{{.Result.Review.CommentCount}}
+
Comments
+
+
+
{{.Result.Review.FilesReviewed}}
+
Files
+
+
+
{{.Result.Review.TotalTokens}}
+
Tokens
+
+
+
{{.Result.Review.Duration}}
+
Duration
+
+
+
+ {{if .Result.GitLab.MergeRequestIID}}MR: !{{.Result.GitLab.MergeRequestIID}}{{end}} + {{if .Result.GitLab.PipelineID}}Pipeline: {{.Result.GitLab.PipelineID}}{{end}} + {{if .Result.GitLab.JobID}}Job: {{.Result.GitLab.JobID}}{{end}} + {{if .Result.Review.SessionID}}Session: {{.Result.Review.SessionID}}{{end}} +
+
+ +{{if .Result.Comments}} +

Comments

+
+{{range .Result.Comments}} +
+
+ {{.Path}} + L{{.StartLine}}-{{.EndLine}} +
+
+

{{.Content}}

+ {{if .ExistingCode}} +
+
+
Existing
+ 
{{.ExistingCode}}
+
+ {{if .SuggestionCode}} +
+
Suggestion
+ 
{{.SuggestionCode}}
+
+ {{end}} +
+ {{end}} +
+
+{{end}} +
+{{else}} +

No comments generated. Looks good to me.

+{{end}} + +{{if .Result.Warnings}} +

Warnings

+
+ + + {{range .Result.Warnings}} + + {{end}} + +
TypeFileMessage
{{.Type}}{{.File}}{{.Message}}
+{{end}} +
+ + diff --git a/internal/viewer/templates/review_projects.html b/internal/viewer/templates/review_projects.html new file mode 100644 index 00000000..8dc60772 --- /dev/null +++ b/internal/viewer/templates/review_projects.html @@ -0,0 +1,31 @@ + + + + + + Review Results - Open Code Review Viewer + + + + +
+

Review Results

+{{if .Projects}} + + + + {{range .Projects}} + + + + + + {{end}} + +
ProjectReviewsLast Review
{{if .Project.Name}}{{.Project.Name}}{{else}}{{.EncodedKey}}{{end}}{{.ReviewCount}}{{formatTime .LastReview}}
+{{else}} +

No review result data found. Run ocr review --save-result first.

+{{end}} +
+ +