21 commits

Author	SHA1	Message	Date
xujiejie	d8fc4cd2af	chore(examples): add explicit --audience agent to codeup_ci and action.yml (#1004) ... Since PR #929 made 'human' the default audience and redirected [ocr] progress lines to stderr, the two non-interactive CI call sites that omitted --audience now emit progress noise on stderr. Add --audience agent to match the other four examples. - examples/codeup_ci/post_review.py: add --audience agent to cmd - action.yml: add --audience agent to ARGS (also covers github_actions) - examples/codeup_ci/post_review_test.py: update exact cmd assertion Exit-code handling, JSON parsing, and comment-posting logic are unchanged. Closes #1003	2026-08-19 21:34:16 +08:00
kite	450dd6d1d6	chore(ci): fail CI when unapproved non-English text appears in source files (#876) ... * fix(prompt): replace the fullwidth colon in the file_read tool description tools.json advertised the example output as "File：path/to/example.go" with a fullwidth colon (U+FF1A), while file_read.go actually emits "File: %s". The description is sent to the model on every review, so the example did not match the output it was describing. Also switches action.yml's OCR_LANGUAGE example from 中文 to Chinese, for the same reason as #861: the value is fed to the LLM and Chinese is what the rest of the project uses. * chore(ci): fail CI when CJK characters appear in source files Comments, identifiers and strings in this repository are meant to be English, but nothing enforced it — #861 had to clean up leftovers by hand, and the same drift keeps arriving through generated code and contributions written internally. scripts/verify-cjk.go walks the index plus untracked files and reports Han ideographs, kana, CJK punctuation and fullwidth forms. Written in Go rather than shell so it does not depend on the container's grep having PCRE, and so `unicode.Is` decides what counts as CJK instead of a byte range that would flag the em dashes used throughout the comments. `//go:build ignore` keeps it out of ./..., so it does not affect go vet, go build or the coverage threshold. Untracked files are included (--others --exclude-standard) so a new file is checked before it lands: while writing this, the script's own comment used Chinese punctuation as an example and went unreported until it was staged. Two escape hatches, preferring the narrow one: an `allow-cjk: <reason>` marker comment on a single line, or a prefix in allowedPrefixes for a whole tree. 23 existing lines get markers (UTF-8 encoding fixtures, multibyte truncation fixtures, language-switcher labels, the fullwidth bar used as a terminal cursor). pages/src/i18n/ is allowlisted as translated UI copy; extensions/vscode/ is allowlisted TEMPORARILY — its comments, test names and zh-cn NLS bundle are still Chinese and need a follow-up pass. Wired into CI next to the license and action-pin checks, plus `make cjk-check` and `make check` for local runs. * chore(ci): generalise the CJK check to all non-English text Addresses the review feedback, and widens the rule that the feedback exposed. Review feedback: - exemptMarker requires its colon, so a bare "allow-cjk" can no longer exempt a line without giving a reason. - The script is named for CJK but missed Hangul. - git ls-files gains -z, so paths that are not plain ASCII arrive unquoted, and its stderr is reported rather than a bare exit status. - main discarded run()'s error entirely and only called os.Exit(1), which is what made the lost stderr invisible in the first place. - The CI step and AGENTS.md say "unapproved", since escape hatches exist. The check was skewed by writing system rather than by language. In one array the 'zh' and 'ja' labels each needed a marker while the adjacent 'ru' label passed untouched, and nine lines of Russian sat in the tree unflagged: two language-switcher labels and the heading-ID fixtures. Contributors writing Chinese had to justify every line; contributors writing Russian had nothing to justify. The rule is now "a letter outside ASCII", since written English needs no letter beyond the ASCII 26 -- Cyrillic and Han as much as the diacritics that spell German or Vietnamese. Scripts are not enumerated, so one nobody has contributed in yet is covered when it arrives. Common and Inherited pass, so letterlike symbols (U+2139, U+2113) are not mistaken for prose, and combining accents are caught, so the decomposed spelling of an accented letter cannot slip through. Symbols and emoji stay out of scope by construction: they are not letters. Renamed to scripts/verify-english-only.go and make english-check, and the marker to allow-non-english:. Text spelled entirely in ASCII still takes a dictionary to identify and stays a matter for review. * docs(agents): restate the English-only rule as rule, homes, hatches The rule was one dense bullet that led with the detection mechanism and mentioned the exemptions only in passing, which is the wrong order for the reader: an agent needs to know where a translation may go before it needs to know which Unicode scripts are flagged. Split into three. The homes are now spelled out from what the tree actually holds, rather than left as "<locale> docs or an i18n table": README and CONTRIBUTING in zh-CN, ja-JP, ko-KR and ru-RU; the doc pages under pages/src/content/docs/ in en, zh, ja and ru; the UI copy tables in pages/src/i18n/. Also why the two are exempt for different reasons -- Markdown by extension, the i18n tables by prefix because they are .ts -- since that decides where a new translation can safely go. Drops the enumerated list of what "make check" runs. It duplicated the Makefile, went stale the moment a check was added (this PR had to edit it), and told an agent nothing it would not read in the output anyway. What is worth saying is that the target writes to the tree. * fix(ci): detect U+FE10–FE6F CJK punctuation in english-only check The vertical forms (U+FE10–FE19), CJK compatibility forms (U+FE30–FE4F) and small form variants (U+FE50–FE6F) were not caught, even though their fullwidth counterparts (U+FF00–FFEF) already were. A small question mark (U+FE56 ﹖) or vertical comma (U+FE10 ︐) left in source reads as correct English punctuation and is invisible in review — the same class of typo the fullwidth range already defends against. Skip U+FE20–FE2F (Combining Half Marks) which are used in Latin text.	2026-08-13 14:43:55 +08:00
kite	980f21d6f2	chore: remove leftover Chinese from the Go core, CI examples and pages comments (#861) ... * docs(comments): translate Chinese comments to English Rewrite the remaining Chinese code comments outside the VSCode extension in English, so the Go core and the pages site read consistently. - allowed_ext.go: translate the default_exclude_patterns.json package doc. Quote the wildcards ("*", "**", "{a,b,c}") so gofmt stops reflowing the leading "*" as a markdown bullet, which had swallowed the first entry's wildcard and broken the list alignment in godoc. - HighlightsSection.tsx: translate three comments in parseStatValue and CountUpValue. * docs(examples): use "Chinese" instead of "中文" in OCR_LANGUAGE examples The language config value is fed to the LLM, which understands "Chinese" just as well, and "Chinese" is what the rest of the project already uses (skills/open-code-review/SKILL.md, config_cmd_test.go, ApplyLanguage). Keeps the GitLab CI example's inline docs fully English. * fix(agent): drop the unreachable Chinese branch from planBlockPattern task_template.json ships a single English template ("### Review Plan (Optional)") and is embedded via go:embed with no override path, so the "审查计划" alternative could never match anything. It came from the pre-open-source template and survived the #33 fix as dead defensive code. Drops the two test cases that only exercised that alternative.	2026-08-12 15:58:23 +08:00
Fanzzzd	7e52a4fd55	fix(action): pin nested action references to full commit SHAs (#836) ... * fix(action): pin nested action references to full commit SHAs A consumer who SHA-pins alibaba/open-code-review still ran whatever the floating actions/* tags inside action.yml pointed at, so the outer pin did not actually freeze the workflow. Pin all four nested references to full commit SHAs with a trailing version comment, enforce the invariant with scripts/verify-action-pins.sh in CI, and document the dual pin (action SHA + ocr_version) that reproducible setups need. Refs #816 * fix(scripts): fail the pin check when action.yml is missing	2026-08-11 15:36:13 +08:00
Lei Zhang	84be9d0413	feat(examples/gitlab): align GitLab CI review posting with the GitHub Action (#767) ... * feat(gitlab-ci): align review posting with the GitHub Action behaviors Port the production-hardening of scripts/github-actions/post-review-comments.js into examples/gitlab_ci/post_review.py so the GitLab example matches the GitHub Action's publication semantics. Refactor the transport from a single post() callable into a Poster object (GitLabPoster / DryRunPoster) exposing post_note / post_discussion / list_notes / update_note / list_discussions / get_mr_diffs, so reads (sticky upsert, incremental history, idempotent reconciliation) and the 400-fallback diff inventory can flow through it. Pure-logic ports (Group A): - category/severity badge [category · severity] on every inline discussion and fallback entry (byte-matching the CLI's buildBadge) - publication policy routing findings to the summary by severity (OCR_ROUTE_SEVERITY_BELOW) and category (OCR_ROUTE_CATEGORIES), fail-open on unknown metadata; routed findings never enter the inline write path - deterministic sort (path -> start_line -> end_line -> index) before posting - detailed warning rendering (file (type): message bullets) - backtick-safe fenced code blocks in the fallback Before/After; inline suggestion keeps the fixed suggestion:-0+0 fence so the Apply-suggestion button keeps working Read-API features (Group B): - sticky summary (default ON): the summary note is updated in place across runs via a <!-- ocr-summary --> marker (find-then-PUT), instead of accumulating one note per run; a pre-review anchor pins it above the discussions on the first run - incremental mode (OCR_INCREMENTAL, default off): skip comments whose line range overlaps a prior bot discussion on the same path, using IoU above OCR_INCREMENTAL_OVERLAP_THRESHOLD (default 0.6); single-line vs multi-line never match; bot discussions detected by the <!-- ocr- marker - idempotent retry: each inline discussion carries an invisible <!-- ocr-<pipeline>-<job>-<hex> --> id tag; on a 5xx/408/network failure (maybe-reached-server) the script GETs /discussions and skips the retry when the id is found (no duplicate); when the read API is unavailable it skips the retry and surfaces the reason rather than risk a duplicate C group (minus batched createReview): - 400 line-resolution fallback: when a discussion POST returns 400 with a position error, fetch GET /merge_requests/:iid/diffs, classify the comment (valid/invalid/unknown), and drop provably-out-of-diff findings to the summary instead of blindly retrying; unknown keeps the existing fallback - wait-until-reset: when RateLimit-Remaining == 0, sleep until RateLimit-Reset (defensively handling both epoch-seconds and seconds-until-reset formats), ahead of Retry-After / backoff D group (pipeline alignment): - dotenv stats output (/tmp/ocr-stats.env) with OCR_COMMENTS_{TOTAL,INLINE, SUMMARY,ROUTED,SKIPPED,FAILED} and OCR_SUMMARY_URL, exposed via a reports: dotenv artifact - artifacts (when: always, 1 week): /tmp/ocr-result.json and /tmp/ocr-stderr.log so a failed review is still inspectable - severity fail gating (OCR_FAIL_ON_SEVERITY): non-zero exit when a comment severity is at/above the threshold; the summary note is still posted first Tests: 57 -> 117 (stdlib unittest, no network, no wall-clock). New coverage for badge/policy/sort/warnings/safe-fence, sticky upsert (cold start + reuse), incremental IoU (single/multi/cross-type/read-failure), idempotent reconciliation (posted/unavailable/not-posted/400-no-reconcile), 400 classification (valid/invalid/unknown/truncated patch), wait-until-reset (seconds/epoch/non-zero-remaining), stats file, severity gating. The only behavior change for existing users: the summary note now updates in place by default (sticky_summary default true, matching the GitHub Action); incremental/routing/gating default to off / no-op. * fix(gitlab-ci): harden OCR_INCREMENTAL_OVERLAP_THRESHOLD parsing, drop dead regex - build_config used a bare float() on OCR_INCREMENTAL_OVERLAP_THRESHOLD, which raised an unhandled ValueError and crashed the script when a user set a non-numeric value. Reuse resolve_threshold, which already handles non-numeric / out-of-range values and falls back to the default. - Remove the unused _COMMENT_ID_RE module-level regex; the idempotency check in _is_comment_posted uses a plain substring match, so the regex was dead code. Tests: +2 covering non-numeric and out-of-range env values (117 -> 119). * fix(gitlab-ci): strip control chars in route_comment, drop dead badge init - route_comment read category/severity via str(...).strip().lower() without stripping C0/C1 control characters, while build_badge uses sanitize_metadata for the same fields. If the LLM emits embedded control chars (e.g. "bug\r"), the badge rendered [bug · high] but routing classified it as "unknown" and skipped policy routing. Reuse sanitize_metadata to stay consistent. - format_comment_fallback had a duplicate md/badge initialization block immediately overwritten by an identical block, a merge artifact. Removed the redundant first block. * fix(gitlab-ci): classify line_code 400s, match MR version to OCR-reviewed SHAs - LINE_RESOLUTION_PATTERNS missed the `line_code` substring, so GitLab's `:line_code=>["can't be blank", "must be a valid line code"]` 400 was never classified as a line-resolution failure. The per-comment diff-inventory fallback never fired, and the raw error JSON leaked into the summary note as `⚠️ Could not be posted inline: {"message":"400 Bad request - Note ..."}` instead of a friendly "out of diff" / "line resolution failure" reason. Add `line_code` to the pattern list. - fetch_diff_refs took `versions[0]` blindly. After a force-push or follow-up commit, that version's head_commit_sha no longer matches the diff OCR actually reviewed, so the position's base_sha/head_sha describe the wrong diff and every inline position fails to resolve on GitLab's side — the root cause of the line_code 400 above. Now read resolved_head/resolved_base from the OCR result manifest and pick the matching MR version, falling back to newest-by-created_at with a warning when no version matches. - Migrate artifact paths /tmp -> .ocr (GitLab Runner refuses to upload artifacts outside the build directory) and drop the now-redundant temporary sticky-summary diagnostic block. - Add 7 tests: line_code pattern positive, version matching by head / head+base / no-match-fallback / no-expected-shas-sorts-newest, and _extract_expected_shas with/without manifest. * feat(gitlab-ci): close GitHub Action parity gaps (config, flags, exit gate, pacing, per-run summary) Brings the GitLab CI example to feature parity with the GitHub Action (action.yml + scripts/github-actions/post-review-comments.js). - .gitlab-ci.yml: expose action.yml inputs as CI/CD variables. - OCR_VERSION pins the npm spec (default latest) for reproducible reviews. - Optional LLM config written via `ocr config set` only when the CI var is non-empty (no clobbering): OCR_LANGUAGE, OCR_LLM_AUTH_HEADER, OCR_LLM_EXTRA_HEADERS. OCR_LLM_TIMEOUT is read natively by OCR from env. - Review flags threaded env→flag: OCR_REVIEW_CONCURRENCY, OCR_BACKGROUND, OCR_RULE. - OCR exit-code gate: drop `|| true`, capture OCR_EXIT_CODE, run post_review.py unconditionally (so stderr still posts to MR), then exit non-zero on OCR failure — mirrors the Action's "Fail job on OCR error". - Document `llm.use_anthropic` as a hardcoded (not CI-configurable) value under a separate "Not a CI/CD variable" header; the false listing as a required CI var was misleading. - post_review.py: posting-logic parity. - Read-API pacing: `_read_with_pacing` wraps list_notes/list_discussions/ get_mr_diffs with read_success_delay (0.5s default) and read_low_remaining_spacing (5s default) when RateLimit-Remaining is at/below rate_limit_threshold — mirrors readWithPacing so a large MR does not hammer the read API. - Non-sticky per-run summary matching: `summary_tag_for(run_tag)` / `wrap_summary_body` embed both SUMMARY_MARKER and a per-run `<!-- ocr-summary-run:TAG -->`; find_summary_note/upsert_summary/ ensure_summary_anchor/finalize_summary take a `tag` param. The pre-review anchor now runs in non-sticky mode too (was sticky-only), so non-sticky shows the "⏳ Posting…" live state and finalize updates the same note instead of creating a duplicate. - build_config exposes read_success_delay / read_low_remaining_spacing. - tests (+12, 142 total, 0.01s): - ReadPacingTest: success-delay pacing, low-quota long spacing, no pacing sleep on failed read. - NonStickySummaryTest: cold-start anchor→update same note; same-run reuse; different-run not reused. - SummaryTagPureTest: tag format, wrap embeds both markers, newest-first find by tag/marker, None on no match. - BuildConfigTest: read-pacing defaults & overrides. - Recorder.final_summary_body / summary_call_count helpers adapt existing assertions to the two-phase anchor→finalize flow. - MainAuthHeaderTest: mock urlopen with canned responses — was making real DNS lookups to gitlab.example, amplified 3x by the new always-on anchor (13.7s → 0.01s). - README.md: document the 9 new CI vars in the variables table, add read-pacing vars to the retry/delay table, convert the version/rules/ concurrency/background recipes from "edit the YAML" to "set this CI var". * fix(gitlab-ci): if-guard optional vars, capture post_review exit code, split diff-refs reason - Replace [ -n "$VAR" ] && cmd with if-guards so FF_ENABLE_BASH_EXIT_CODE_CHECK cannot abort on an empty optional variable (|| true would mask real failures) - Capture post_review.py exit code (POST_EXIT_CODE) so the OCR_EXIT_CODE gate runs regardless of severity-gate or posting failures - Split 'not path or not end_line or not diff_refs' into two checks so diff-refs-unavailable comments get an accurate reason in the summary * fix(gitlab-ci): default OCR/POST exit codes to guard against unset gate vars Without a default, an unset OCR_EXIT_CODE/POST_EXIT_CODE makes the gate `[ "" != "0" ]` evaluate true and `exit ""` fail with 'numeric argument required'. Default both to 0 so the gate is always safe.	2026-08-07 15:58:03 +08:00
Seonggwan Ahn	e75601d2e6	feat(examples/gitlab): add fail-open category/severity publication controls (#685) ... * feat(examples/gitlab): add fail-open category/severity publication controls Port the GitHub Action publication policy (#478/#529) to the GitLab CI example so MRs can badge findings and optionally route low-signal categories/severities to summary notes without dropping them. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(examples/gitlab): sanitize metadata in route_comment Align routing with build_badge so control characters in LLM category/severity values do not desync badge labels from policy matching. --------- Co-authored-by: Cursor <cursoragent@cursor.com>	2026-08-03 19:52:44 +08:00
Ritvik K	dc1e346316	Add CodeUp CI integration (#570) ... * Add CodeUp CI integration (issue #517) Adds examples/codeup_ci with: - post_review.py: runs ocr review --format json and posts a GLOBAL_COMMENT summary via CodeUp's CreateChangeRequestComment API - post_review_test.py: unit tests (27 passing) - codeup-flow.yml: example Yunxiao Flow pipeline - README.md: setup docs and known limitations (v1 = summary comment only, inline comments as a follow-up) * chore: ignore Python cache files * Add actual file contents for CodeUp CI integration * Use generic placeholder for OCR_LLM_MODEL example * Address review: remove local .gitignore, add trailing newlines, simplify testing docs	2026-07-29 22:37:29 +08:00
侯雨希	e670b3b3f6	refactor(examples): extract GitLab CI heredoc into testable post_review.py with unit tests (#539) ... * refactor(examples): extract GitLab CI heredoc into post_review.py with unit tests Extracts the ~270-line inline heredoc from .gitlab-ci.yml into a standalone, testable post_review.py module, matching the publish() + make_poster() pattern established by gerrit_ci/ and gitflic_ci/. Key design decisions (from spec issue #1 and wayfinder tickets #3, #4, #5): - publish(result, diff_refs, post, config, sleep) — transport-agnostic - make_poster(api_base, token, auth_header, config) — GitLab REST transport - fetch_diff_refs(api_base, token, auth_header, config) — /versions GET with retry - Single config dict built by main() from env vars; no module-level config state - post() returns {success, rate_limit_remaining, is_rate_limit_exhausted} to preserve failure-pacing behavior (rate-limit vs non-rate-limit delays) - _sleep = time.sleep module-level pattern for testability All existing heredoc behavior preserved 1:1: - GitLab suggestion:-0+0 syntax and <details> fallback format - Retry on 429/403-rate-limit/5xx/408 with exponential backoff + ±25% jitter - Retry-After header honoring, MAX_RETRY_DELAY cap - Proactive RateLimit-Remaining throttling (success path only) - Failure pacing: rate-limit-exhausted → SUCCESS_DELAY, other → FAILURE_DELAY - PRIVATE-TOKEN vs JOB-TOKEN auth selection - Inline → fallback → summary ordering - Parse failure → post stderr as error note - All 6 env vars (OCR_RETRY_BASE_DELAY, OCR_MAX_RETRIES, OCR_MAX_RETRY_DELAY, OCR_SUCCESS_DELAY, OCR_FAILURE_DELAY, OCR_RATE_LIMIT_THRESHOLD) 48 unit tests (stdlib unittest, no network, no real time.sleep): - Seam 1: publish() with Recorder fake poster — inline/fallback/summary flow, proactive throttling, failure pacing - Seam 2: make_poster() with mocked urlopen + _sleep — retry/backoff/jitter, Retry-After, delay cap, auth headers, is_rate_limit_exhausted classification - fetch_diff_refs() with mocked urlopen — success/failure/retry - build_config() defaults and env overrides - Dry-run poster — no HTTP calls Implements #534. * fix(examples): address code review findings on gitlab_ci post_review - Add missing-required check for CI_PROJECT_ID and CI_MERGE_REQUEST_IID in main(), matching gerrit_ci/gitflic_ci pattern. The heredoc used os.environ[...] (KeyError on missing); the extraction silently used env.get(..., "") which constructs a malformed API URL. Now fails fast with a clear error message. - Change transient_base_delay from int 2 to float 2.0 to match spec config-dict type annotation. - Add 5 end-to-end tests for main()'s auth-header env resolution: PRIVATE-TOKEN when GITLAB_API_TOKEN set, JOB-TOKEN when only CI_JOB_TOKEN set, PRIVATE-TOKEN wins when both set, missing CI vars fails fast, missing token fails fast. Addresses review findings: #3 (TP, medium), #4 (Edge), #5 (TP, low). * fix(examples): handle URLError in gitlab_ci post_review retry logic _api_request_with_retry only caught HTTPError, not URLError. Network-layer failures (DNS resolution failure, connection refused, connection reset) raised URLError which propagated uncaught, crashing the script and losing all pending review comments. The original heredoc had the same gap, but the gerrit_ci sibling already handles this correctly (lines 250-259: retry on connection errors, propagate timeouts). This fix follows the gerrit_ci pattern adapted to our return-dict contract: - Add 'except urllib.error.URLError' handler after HTTPError handler - Timeout (socket.timeout/TimeoutError): return failure dict, don't retry (ambiguous — server may have processed the request) - Connection errors (DNS, refused, reset): retry with transient_base_delay backoff + ±25% jitter, same as 5xx/408 handling - Exhaustion: return failure dict with is_rate_limit_exhausted=False 3 new tests: - test_retry_urlerror_then_success: ConnectionRefused → retry → success - test_urlerror_exhausts_retries: 4 ConnectionRefused → failure after 4 attempts - test_urlerror_timeout_not_retried: socket.timeout → immediate failure, no retry Found by OCR (open-code-review) AI code review. * fix(examples): handle non-UTF-8 HTTP error bodies in gitlab_ci post_review e.read().decode('utf-8') raises UnicodeDecodeError when the GitLab server returns a non-UTF-8 error body (e.g., an HTML error page in latin-1 from a misconfigured proxy or load balancer). This exception propagated uncaught, crashing the entire posting loop — no further inline comments, fallback notes, or summary notes would be posted. Both gerrt_ci (line 242: decode('utf-8', 'replace')) and gitflic_ci (line 344: decode('utf-8', 'replace')) siblings already handle this correctly. The original heredoc had the same gap. Fix: add errors='replace' to both decode() calls (success path line 248 + error path line 260). For valid UTF-8 input (the normal case), behavior is identical. The error body is only used for keyword matching and logging, both of which work fine with replacement characters (U+FFFD). 1 new test: - test_non_utf8_error_body_does_not_crash: HTTPError with invalid UTF-8 body → no crash, returns failure dict Found by OCR (open-code-review) AI code review on PR #539.	2026-07-28 10:16:16 +08:00
a1212a11	476f661f26	fix(examples): preserve Bitbucket OCR diagnostics (#486)	2026-07-24 21:03:52 +08:00
Abdul Mohammad	6639298639	docs(examples): add Bitbucket Pipelines CI integration example (#483)	2026-07-24 08:29:18 +08:00
chethanuk	83dacc2725	docs(examples): OCR_LLM_MODEL is required, not optional (#431)	2026-07-22 09:14:39 +08:00
chethanuk	870fc6a4a9	feat(examples): add Gerrit CI integration example (Jenkins + Gerrit Trigger) (#401) ... * feat(examples): add Gerrit CI integration for publishing review results Adds examples/gerrit_ci following the gitflic_ci CI-glue pattern (#316): a stdlib-only post_review.py that reads 'ocr review --format json' and publishes summary, inline comments, suggestion blocks, and file-level findings in ONE batched POST /a/changes/{change}/revisions/{rev}/review, tagged autogenerated:opencodereview with notify=OWNER and omit_duplicate_comments. Decisions (validated against a live Gerrit 3.14 in Docker): - Plain comments, bare line = end_line, no CommentRange: the range form {start,0,end,0} renders lines start..end-1 in the UI (end_character 0 excludes the final line), so correct ranges would need file contents in CI. Verified via UI screenshots during E2E. - Preemptive HTTP Basic auth (urllib's handler does not preempt), XSSI )]}' stripping, HTML-200 detected as config error, 400 batch fold-retry, 409 change-closed tolerated (defensive on modern Gerrit: label-free reviews post fine on closed changes), password scrubbed from all error output. - Jenkins Gerrit Trigger as the reference integration; the script is trigger-agnostic via flags/env (Zuul/hook recipes in the README). Jenkinsfile always passes the injected patchset SHA (revision race). 45 table-driven stdlib-unittest tests, red-to-green TDD; E2E against gerritcodereview/gerrit covering live post, unicode round-trip, dedup re-run, and failure paths (401 exit 2 with scrubbed password). * fix(examples/gerrit_ci): harden post_review.py per review findings - Send Authorization via add_unredirected_header so Basic credentials cannot follow a redirect to another host (urllib forwards ordinary headers cross-host). - Reject non-object JSON input cleanly instead of an AttributeError traceback; validate --timeout > 0 at argparse time. - Warn on stderr when the 400-fallback folded summary is truncated, and report the fold accurately instead of claiming N inline comments. - Pin drafts=KEEP in ReviewInput (depot_tools convention; old servers defaulted to deleting the caller's drafts). - Jenkinsfile: fetch with an explicit dest refspec so origin/$GERRIT_BRANCH materializes under narrow-refspec clones; comment out extra_body thinking (OpenAI rejects unknown fields). - Gitflic parity: optional positional input arg; single-sourced 'current' revision default; scrub() skips sub-4-char passwords. - README: document exit 1 and the defensive 409 branch; add gerrit_ci row to all five root READMEs (parity with the GitFlic example). - Tests: 44 -> 55, covering stdin input, flag-over-env precedence, fold-retry failure, fold truncation, GERRIT_CHANGE_URL wiring, non-dict JSON, timeout validation, positional input. * fix(examples/gerrit_ci): address OCR bot review on #401 - Jenkinsfile: resolve the LLM endpoint from the OCR_LLM_URL/TOKEN/MODEL env triple instead of `ocr config set`, so the auth token stays env-only and is never written to ~/.opencodereview/config.json on a shared agent (OCR_CONFIG_PATH is deliberately ignored by write paths, so it can't redirect the leak). Pin the npm install to a validated version. Document the config-file fallback (and its cleanup) for extra_body, which has no env equivalent. - post_review.py: scrub the base64(user:password) Authorization value from error output too, not just the raw password — a proxy echoing the request header would otherwise leak decodable credentials. +1 test. * feat(examples/gerrit_ci): scoped retry + robustness polish Post-review hardening from an OSS-precedent study (depot_tools, kudu, Gerrit REST docs): - Bounded retry (3 attempts, exp backoff) in make_poster, scoped to the provably-safe failures only: HTTP 5xx and pre-response connection errors (refused/reset/DNS). Read-timeouts are deliberately NOT retried — a timeout is ambiguous (the server may have applied the review) and omit_duplicate_comments dedupes only inline comments, not the summary message, so a blind retry could post a duplicate change message. 4xx (400/401/404/409) propagate unchanged so main() classifies them as before. - Document why plain comments are used, not robot_comments: the latter is deprecated since Gerrit 3.6, disabled-by-default in 3.12, and slated for removal; the tag already marks bot origin. - Fold fallback: strip the '; N posted as inline comment(s).' clause from the reused summary so the folded message doesn't claim inline comments were posted and then explain they couldn't be placed. - README: document the retry scoping and note fix_suggestions / label voting as intentional future options. Tests: 56 -> 61 (5 retry cases: 5xx-then-ok, conn-err-then-ok, 5xx-exhaust, read-timeout-not-retried, 4xx/409-not-retried).	2026-07-21 13:25:30 +08:00
Lei Zhang	d9159276af	feat(action): extract reusable composite PR-review GitHub Action (#337) ... * feat(action): extract reusable OpenCodeReview PR review GitHub Action Consolidate the reusable-action work into one commit: - Add composite action (action.yml at repo root for GitHub Marketplace; helper at scripts/github-actions/post-review-comments.js) porting the sticky summary, incremental posting, and retry idempotency logic. - Add unit tests covering the ported idempotency behavior. - Switch the in-repo CI workflow to use the reusable action. - Add and refine example reusable workflows for consumers. * ci(workflow): point ocr-review at root action.yml and quote boolean inputs - Fix uses: to ./ now that action.yml lives at the repo root. - Quote sticky_summary/incremental/upload_artifacts as strings to match action.yml's input declarations (composite-action inputs are always strings) and silence actionlint. - Enable upload_artifacts for this workflow. * docs(examples): point reusable demo at root action.yml The example workflow referenced alibaba/open-code-review/action@v1, but action.yml now lives at the repo root, so the /action subpath no longer resolves. Use alibaba/open-code-review@v1 and update the stale action/README.md comment to point at the root action.yml. * docs(examples): sync README to root action.yml references The example README still pointed at the relocated/deleted locations: action.yml is now at the repo root, so update all 11 alibaba/open-code-review/action@v1 references to alibaba/open-code-review@v1, and repoint the action/ directory and action/README.md links to the root action.yml. * fix(examples): prevent unrelated PR comments from canceling ocr-review GitHub Actions evaluates concurrency before the job-level if-condition. The flat group mapped every issue_comment event on a PR into the review's group, so any comment (even a skipped conversation reply) canceled any in-progress review. Match the reusable demo's conditional group: PR events and human-authored /open-code-review/@open-code-review comments share a per-PR group, while non-matching comments fall back to a unique noop-<run_id> group that can never collide with a real review. * fix(action): address code-review findings across reusable PR review - post-review-comments: parse retry delays via parseNonNegInt (0/negative fix); paginate findExistingSummaryComment through readAllPages; remove dead rangeOf and hasIssueCommentWithId (plus duplicated comment block) - action.yml: move ${{ }} interpolations into env: (resolve refs, PR_NUM, ocr_version); fail fast on PR head fetch instead of swallowing errors - workflows: add timeout-minutes: 30; gate issue_comment on author_association; tighten pr-context if to == 'issue_comment' * fix(action): harden review posting after code review - pass incremental_overlap_threshold via env to avoid github-script injection - capture ocr review exit code directly instead of &&/|| chain - drop redundant SUMMARY_MARKER prepend in postSummary (callers already add it) - align example job if-condition bot check with its concurrency group * fix(action): always upload review artifacts and capture ocr exit code * fix(action): merge posting statistics into the summary header The PR summary issue comment used to present two overlapping breakdowns: a leading "posted as inline / posted as summary" header and a trailing "📊 Posting Statistics" block. Their definitions overlapped (the header's "summary" count included failures the trailer also listed as failed), and when incremental filtering skipped comments the header counts no longer summed to the total, making the summary hard to interpret. Merge them into a single header whose four counts (inline / summary / skipped / failed) are mutually exclusive and sum to the total, and drop the trailing Posting Statistics section. buildSummaryBody now takes an options object. * fix(action): support local action resolution in container/self-hosted setups - Checkout trusted base + mark workspace safe for pull_request_target so the local `uses: ./` action can be resolved and loaded - Check for git/Node.js and install git when missing, making the composite action resilient across runner images - Move Setup Node.js earlier and make it conditional on availability - Resolve post-review-comments helper at runtime via GITHUB_ACTION_PATH falling back to GITHUB_WORKSPACE, fixing helper lookup for local actions where the action path is a host path invisible inside containers * refactor(examples): consolidate github_actions demo to reusable action Drop the inline-script full-control demo; the renamed ocr-review.yml (from ocr-review-reusable.yml) is now the single demo, invoking alibaba/open-code-review@main. Sync the README to the current implementation: - normalize action refs to @main; point self-hosted-runner users to the repo's own workflow (noting uses: ./ is internal-only) - document config via action inputs (posting modes: sticky/incremental) - update the comment-trigger if with defensive bot/author_association guards and the concurrency mirror - fix Example Output to cover the summary comment + inline comments - replace the non-existent OCR_DEBUG debugging with artifacts/outputs/ACTIONS_STEP_DEBUG - use --replace-all for safe.directory * fix(action): harden withRetry against silent undefined return withRetry's for loop had no terminal return/throw after the loop body. Although the current loop invariant (last attempt always throws, and parseNonNegInt guards against negative MAX_RETRIES) makes fall-through unreachable, an async function that falls through resolves to undefined, which would surface as a confusing downstream TypeError for the read-API callers that rely on it. Capture lastErr in the loop and add an explicit terminal throw so any future break of the invariant fails loudly instead of silently returning undefined. * docs(readme): document the reusable GitHub Action in CI/CD section * fix(action): restore language config via a language input The old inline workflow ran `ocr config set language English`, but the composite action's Configure OCR step only set llm.extra_body, with no language input. Add a language input (default English) and write it via `ocr config set language` so review output language is no longer left to the tool's default. Addresses #337 (discussion_r3550069843). * fix(action): warn when incremental comment listing hits page cap listExistingReviewComments silently dropped comments beyond its 10-page cap, unlike readAllPages which logs when truncation occurs. Add the same max-page-limit warning after the loop so a partial walk during incremental dedup is visible in the logs. Addresses #337 (discussion_r3550069871). * docs(readme): sync GitHub Action section to localized READMEs	2026-07-09 19:08:19 +08:00
Fedor	7497d5ac6e	docs(examples): add GitFlic CI auto-review example (#201) ... * docs(examples): add GitFlic CI auto-review example Add examples/gitflic_ci/, a CI-layer integration that reviews GitFlic merge requests with OpenCodeReview and posts the findings as MR discussions. Like the GitHub and GitLab examples, the posting glue lives outside the core binary. post_review.py (standard library only) reads `ocr review --format json` and posts inline discussions plus a fallback note and a summary. GitFlic's Discussions API requires an old-side line for inline comments, which the new-side-only review output lacks, so the script recomputes it from the same merge-base diff the review ran on. Ships with a stdlib unittest suite whose line-mapping cases are ported from the review's diff logic. * docs(readme): list the GitFlic CI example in the localized READMEs * fix(examples): address GitFlic CI review feedback from PR #201 Apply the five review comments left on the PR: - gitflic-ci.yaml: guard `ocr config set llm.model` behind a non-empty check so the documented-optional OCR_LLM_MODEL no longer breaks the config step when it is unset - gitflic-ci.yaml: skip posting when `ocr review` produced no output (the step ends with `|| true`) instead of feeding empty/partial JSON to post_review.py - post_review.py: redact the token from HTTP error snippets so it cannot leak into CI logs if GitFlic echoes the request back in an error body - post_review.py: read the review-result file via a `with` block so the handle is closed explicitly - examples/README.md: add the missing trailing newline	2026-07-02 10:41:39 +08:00
Lei Zhang	0dac8ac376	fix(ci): prevent duplicate review posts on retry in ocr-review workflow (#250) ... * fix(ci): add idempotency check to prevent duplicate review posts on retry When the batch createReview fails with a 5xx/408/network error, the request may still have landed on the server. Before retrying per-comment, the workflow now: - Tags each review/comment/summary with a per-run HTML comment ID derived from runId + runAttempt + content hash. - Queries existing reviews and review comments to detect whether the batch actually landed, and only retries the comments that are missing. - Before retrying an individual comment whose request may have reached GitHub, cools down (honoring rate-limit headers) then checks whether the comment already exists, treating it as success instead of posting a duplicate. - Skips posting the summary comment when one with the same run tag already exists. - Adds read-API retry/pacing helpers (withRetry/readWithPacing/readAllPages) with shorter spacing than writes (OCR_READ_SUCCESS_DELAY / OCR_READ_LOW_REMAINING_SPACING) since reads are cheaper but still consume the primary rate limit. Degrades gracefully to the original fallback (accepting duplicate risk) when the idempotency read calls themselves fail. * fix(ci): harden idempotency checks in ocr-review workflow Address code review findings on the GitHub Actions PR auto-review workflow (applied to both .github/workflows and examples copies): - readAllPages: cap pagination at maxPages=50 (default) to prevent unbounded loops, and validate the argument is a positive integer. - getPostedCommentIds: anchor the ID regex to the HTML comment wrapper (<!-- ocr-... -->) with a capture group to avoid false positives from user-generated content. - isCommentAlreadyPosted: return null (unknown) instead of false when the read API fails, so callers do not silently risk duplicates; accept a postedIdsCache to reuse a single paginated walk across retries. - hasIssueCommentWithId: return null (unknown) on read API failure, and match the summary tag with an anchored regex for consistency. - Call sites: handle null by skipping retry/posting to avoid duplicates while surfacing the failure in the summary. * fix(ci): validate env config and document intentional behaviors Address code review findings on the ocr-review workflow (applied to both .github/workflows and examples copies): - parseNonNegInt: add a validation helper for env-var parsing so negative or non-numeric values (e.g. OCR_MAX_RETRIES=-5) fall back to defaults instead of bypassing the `|| default` guard (a negative parseInt result is truthy). All seven retry/pacing config values now use it. - readAllPages: document that the 50-page cap is an intentional safety valve against unbounded loops, not a normal mode; callers that depend on completeness already degrade safely to null (unknown), so a truncated walk does not silently produce duplicates. - commentId: document that the 12-hex-char (48-bit) hash collision scope is a single PR (listReviewComments is PR-scoped) and a single run produces only tens to hundreds of comments, making the birthday-bound collision probability negligible (~1e-7 at 10k). * docs(github_actions): sync README with retry/idempotency features in ocr-review.yml - Add OCR_READ_SUCCESS_DELAY and OCR_READ_LOW_REMAINING_SPACING variables for read API pacing used by the idempotency check - Document the three GitHub rate-limit retry strategies (primary reset, retry-after header, secondary no-header backoff) - Add 'Idempotency: avoiding duplicate review comments' section describing how the workflow detects already-landed comments via per-run HTML tags and skips retrying when the read API is unavailable * fix(ci): use full sha256 hash for review comment idempotency IDs Drop the .slice(0, 12) truncation in commentId() and use the full 64-char (256-bit) sha256 hex digest. The truncated 12-char hash carried a tiny but nonzero collision risk whose failure mode was a silently dropped inline comment (the idempotency check would mistake two distinct comments for duplicates). The full hash makes the collision probability effectively zero with no meaningful downside; the ID regex already used [a-f0-9]+ so it accepts the longer IDs unchanged. * fix(ci): use random per-comment IDs and defer body assembly in review workflow Replace the content-derived commentId() (sha256 of path/line/content) with a random per-comment ID (crypto.randomBytes) and restructure the inline- comment flow around an item struct that carries { comment, id, lines }. This fixes two issues in the idempotency check: 1. ID was recomputed on every failure check. Each inline comment is now assigned one random ID up front and carried on the item struct, so the retry/idempotency logic reads item.id directly. The comment body (which embeds the ID) is assembled only at API-call time in toReviewPayload(), eliminating repeated hash computation. 2. Content-derived IDs collided for distinct comments sharing the same path/line/content. A random ID guarantees two such comments get different IDs, so the idempotency check no longer mistakes the second for a duplicate of the first and silently drops it on retry. formatComment/commentId are removed (no callers remain) and replaced with newCommentId/resolveLines/toReviewPayload/buildBody. The matching regex already used [a-f0-9]+ so it accepts the new random tokens unchanged. README ID-format placeholder updated from <hash> to <token>. * docs(ci): correct misleading readAllPages truncation comment The comment claimed 'a truncated walk does not silently produce duplicates' because callers 'degrade safely by returning null on read failures.' That reasoning only holds when the read API THROWS (rate limit, 5xx): isCommentAlreadyPosted/hasIssueCommentWithId then return null (unknown) and the caller skips retrying. A truncated walk does not throw — it returns a partial set silently, so isCommentAlreadyPosted returns false (definitively 'not posted') for comments beyond the cap, and the retry loop reposts them, producing duplicates. Rewrite the comment to state the cap is an intentional safety valve and to explicitly distinguish truncation (partial data, can duplicate) from thrown read failures (null/unknown, safe). No behavior change. * fix(ci): drop stale postedIdsCache to prevent duplicate inline comments isCommentAlreadyPosted reused a single listReviewComments snapshot (postedIdsCache) across all per-comment retries. As comments landed during the loop, the snapshot went stale; a 5xx-landed comment checked against the stale snapshot would be reported as 'not posted' and retried, posting a duplicate. Remove the cache and walk fresh on every check. The extra reads are paced via readAllPages/readWithPacing (with retry honoring retry-after and x-ratelimit-reset) and degrade to null — skip retry — if the read API ultimately fails, so they cannot produce duplicates. The cache provided no real benefit in this path: checked comments are either genuine misses (correctly false) or just-landed (a fresh walk catches them), so hits essentially never occurred.	2026-07-01 19:38:10 +08:00
Zheng Feng	491a2ac6d0	docs(examples): update node.js to 24 in the github action yml (#228)	2026-06-26 20:24:33 +08:00
Lei Zhang	7f22ba867d	fix: actions rate limit (#164) ... * ci: add rate limit handling and version verification to GitHub Actions workflow - Add version check after OCR installation to verify successful setup - Implement exponential backoff retry logic for GitHub API rate limits - Add delays between individual comment posts to avoid secondary rate limits - GitHub enforces ~80 content-generating requests per minute; spacing calls helps stay under that threshold with 2-second base delay and up to 3 retries * ci: refine rate limit handling in GitHub Actions workflow - Add `|| true` to ocr version check for error isolation - Narrow rate limit detection to 429 and 403 with rate-limit message matching, avoiding retries on permission/auth failures - Extract hardcoded delay constants into env-configurable variables (OCR_RETRY_BASE_DELAY, OCR_MAX_RETRIES, OCR_SUCCESS_DELAY, OCR_FAILURE_DELAY) with sensible defaults - Document optional environment variables in workflow header * docs: add environment variable configuration guide for retry and delay settings * ci: add rate-limit resilience and version check to GitLab CI pipeline - Add `ocr version || true` after install for diagnostic logging - Add `api_request_with_retry` function with exponential backoff for 429 and 403 (rate-limit message matching) errors - Respect GitLab `Retry-After` header when present - Extract delay constants into CI/CD-configurable variables (OCR_RETRY_BASE_DELAY, OCR_MAX_RETRIES, OCR_SUCCESS_DELAY, OCR_FAILURE_DELAY) with defaults - Add pacing delays between successful/failed discussion posts - Document optional CI/CD variables in pipeline header comments * docs: add retry/delay settings section to GitLab CI README * ci: fix rate-limit retry delay exhaustion handling in CI workflows - GitHub Actions: distinguish exhausted rate-limit retries from other errors, apply SUCCESS_DELAY (2s) instead of FAILURE_DELAY (1s) when retries exhausted - GitLab CI: return structured result from api_request_with_retry to differentiate failure types, apply context-aware delays based on rate-limit exhaustion status - Both: prevent perpetuating rate-limit failures by using longer delays after retry exhaustion * ci: align GitHub Actions rate-limit retry with header-based strategy Derive wait durations from response headers (retry-after, x-ratelimit-reset) instead of fixed exponential backoff, add proactive throttle when remaining quota is low, honor batch-level rate limits before per-comment retry, and add support for transient 5xx/408 errors. * feat(gitlab-ci): enhance rate-limit handling with jitter, max retry delay, and proactive throttling - Add ±25% jitter on retry delays to avoid thundering herd problems - Add OCR_MAX_RETRY_DELAY (default 60s) to cap per-retry wait time - Add OCR_RATE_LIMIT_THRESHOLD (default 10) for proactive throttling based on GitLab RateLimit-Remaining response header - Parse Retry-After header properly (handle non-numeric values) - Apply retry logic to all API requests (notes, versions, discussions) - Parse and log RateLimit-Remaining/Limit headers for observability - Double pacing delay when remaining quota drops below threshold - Update README with new configuration variables and behavior docs * fix(examples): sync rate-limit docs with script defaults and add missing variables - GitHub Actions README: fix OCR_RETRY_BASE_DELAY default from 2000 to 60000 (matching script code and header comments) - GitHub Actions README: add missing OCR_RETRY_MAX_DELAY, OCR_LOW_REMAINING_THRESHOLD, OCR_LOW_REMAINING_SPACING variables - GitHub Actions README: add GitHub Rate Limits doc reference link - GitHub Actions yml header: add OCR_LLM_USE_ANTHROPIC and llm.extra_body notes - GitLab CI yml header: add llm.extra_body note - GitLab CI README: add GitLab Rate Limits doc reference link * ci(examples): unify header lookup and add transient retry backoff - GitHub Actions: extract inline header closure into a reusable getHeader helper; use it in both computeRetryDelayMs and logRateLimitQuota for consistent case-insensitive header access. - GitHub Actions: use a 2s transientBase for 5xx/408 exponential backoff instead of the 60s rate-limit base, since server hiccups are typically short-lived and the longer base stalled CI jobs unnecessarily. - GitLab CI: add a _get_header helper and route all header access (Retry-After, RateLimit-*) through it, matching the GitHub Actions approach. - GitLab CI: add transient retry logic for 5xx/408 errors with a 2s base delay, so server errors no longer fail immediately.	2026-06-22 10:10:47 +08:00
zhouzhihao	c323c6b40c	fix(actions): preserve failed inline review comments (#81)	2026-06-10 17:50:08 +08:00
Lei Zhang	a32b8c7e30	docs(examples): add concurrency control to CI workflow examples (#60) ... * docs(examples): add concurrency control to CI workflow examples - GitHub Actions: add concurrency group with cancel-in-progress to avoid redundant review runs on rapid pushes - GitLab CI: add interruptible and resource_group to cancel outdated review jobs when new commits are pushed to the same MR * docs(examples): improve GitLab CI example with fork MR and concurrency support - Support forked MR pipelines by using CI_COMMIT_SHA as --to target - Fall back to CI_JOB_TOKEN when GITLAB_API_TOKEN is unavailable - Use appropriate auth header (JOB-TOKEN vs PRIVATE-TOKEN) based on token source - Add --audience agent flag for machine-consumable review output - Make diff_refs required for inline comments, simplify post_discussion signature - Improve summary with inline vs fallback comment breakdown - Add documentation comments for fork MR setup requirements * docs(examples): use pull_request_target and SHA refs for fork PR support - Switch trigger from pull_request to pull_request_target so secrets are available for PRs from forks - Use head SHA instead of branch ref for checkout and ocr --to, since fork branches don't exist on the origin remote - Add explicit fetch step to ensure fork commits are available - Update condition checks and comments to reflect the new event name * docs: sync READMEs with CI script changes for fork PR/MR support	2026-06-08 21:33:20 +08:00
Lei Zhang	55c6bca1f8	chore: update ci pipeline examples (#45) ... * chore: remove --audience agent flag and simplify JSON parsing in CI examples - Remove --audience agent flag from ocr review commands in CI examples - Simplify JSON output parsing by reading directly without skipping first line - Update README docs to reflect the simplified CLI usage * fix: correct typo in .gitlab-ci.yml comment Change 'confuring' to 'setting' in CI variable configuration comment.	2026-06-04 23:57:05 +08:00
Lei Zhang	128787b627	Add CI/CD integration section and examples to documentation (#11) ... * docs: add CI/CD integration section and examples - Add CI/CD Integration section to README.md and README.zh-CN.md - Add GitHub Actions workflow example (examples/github_actions/) - Add GitLab CI pipeline example (examples/gitlab_ci/) - Add examples README with overview of integration options * feat(examples): enhance GitHub Actions demo with comment trigger and improved error handling - Add issue_comment event trigger with /open-code-review and @open-code-review keywords - Add PR context resolution for comment-triggered events via GitHub API - Improve ref handling to support both PR events and comment events - Add individual comment fallback with retry when batch review fails - Add posting statistics (success/failed counts) to summary comment - Update README with comment trigger flow and customization guide * docs(examples): add --background flag usage guide for GitHub Actions and GitLab CI Explain how to pass PR/MR title as background context to help OCR provide more relevant and context-aware review comments. * feat(examples): simplify PR trigger and add skip-existing-review guide for GitLab CI - Reduce GitHub Actions PR trigger to 'opened' only (avoid redundant reviews on synchronize/reopened events) - Add GitLab CI documentation for checking existing OCR comments before running review to save LLM tokens	2026-06-01 16:44:24 +08:00