* feat(action): expose effort and max_tokens_budget as inputs
- Add validated, case-insensitive effort and base-10 max_tokens_budget
workflow inputs, forwarded to ocr review as --effort and
--max-tokens-budget; empty values omit the flags so CLI defaults apply
- Reject an explicit effort input on ocr older than v1.10.0, where the
flag first shipped, instead of dying on an unknown flag
- Join both axes to the checkpoint config fingerprint
Closes#1147
Signed-off-by: thxCode <thxcode0824@gmail.com>
* feat(action): opt-in live review progress via stream_progress
- add stream_progress input (true|false, case-insensitive, default
'false'), validated and normalized in the Validate inputs step
- the default keeps the original behavior: --audience agent in the
review args, stderr captured to /tmp/ocr-stderr.log with no live tee
- stream_progress=true drops --audience agent so human-audience routing
sends [ocr] progress lines to stderr while the result JSON stays on
stdout; a FIFO feeds a background tee so progress streams live into
the workflow log and is still captured to /tmp/ocr-stderr.log for
artifacts and the posting step
- the tee runs as a real background job and is awaited before the log
file is read, so the capture is fully flushed; OCR_EXIT_CODE is
unaffected in either mode
Task 2 of action-review-controls.
Signed-off-by: thxCode <thxcode0824@gmail.com>
* feat(action): first-class llm_reasoning_effort input
- Merge llm_reasoning_effort into the effective llm.extra_body via node
in the Configure step, riding the existing extra_body merge so any
published CLI supports it; an explicit reasoning_effort key in
llm_extra_body wins
- Reject llm_reasoning_effort on the anthropic protocol, and reject a
malformed or non-object llm_extra_body with an actionable ::error::
- Fingerprint the normalized effort, max_tokens_budget, and
llm_reasoning_effort values instead of the raw inputs, so equivalent
spellings keep the checkpoint
- Keep the last 20000 characters of stderr in the unparseable-result
summary so streamed progress cannot push the error report past
GitHub's comment limit
- Document the new action inputs on the CI/CD docs page (en/zh/ja/ko/ru)
Closes#1149
Signed-off-by: thxCode <thxcode0824@gmail.com>
---------
Signed-off-by: thxCode <thxcode0824@gmail.com>
* chore(claude): drop duplicate local /open-code-review command
The repo-local .claude/commands/open-code-review.md duplicated the
canonical plugin prompt at plugins/open-code-review/claude-code/commands/review.md
verbatim, adding another copy to keep in sync. Docs already point users at
the plugin command file, so remove the redundant local copy.
* ci: add plugin distribution contract guardrails
Two blocking checks over the plugin/skill distribution surface, which had
no automated verification at all.
links: the docs and READMEs embed 146 in-repo path links across 53 files
(raw.githubusercontent /main/ curl commands and blob|tree/main links).
Nothing verified the paths still existed, so moving a referenced file
turned every link into a 404 that only readers hit. A missing path now
fails the build; a blob/tree kind mismatch only warns, since GitHub
redirects between the two views. The scan fails closed below a corpus
floor so a broken walk cannot masquerade as a pass, and reports any
documentation file it had to skip instead of passing over it in silence.
manifests: every path declared by a plugin or marketplace manifest must
resolve to a real, non-empty target, and every SKILL.md and command
prompt must carry the frontmatter its loader requires (a skill's name
must equal its directory, since that is how loaders address it). Without
this a rename yields a plugin that installs cleanly and exposes nothing.
The Cursor manifest resolves `../skills/` from its manifest directory
while its two siblings resolve from the plugin root. That asymmetry could
not be verified against Cursor's published spec, so the declaration
carries an explicit unverified-base warning rather than being silently
certified: if Cursor resolves from the plugin root, `../skills/` points
at a `plugins/skills` directory that does not exist.
Unit tests run against temp fixtures, never the real work tree: pointing
them at the repo would mean a stale link anywhere fails the test step
with a stack trace before the annotation-producing steps run, and would
also fail the unrelated Action Contract workflow, which runs the same
npm script.
* fix(ci): escape all regex metacharacters in repo slug
CodeQL flagged js/incomplete-sanitization (high) on the slug escape in
repoLinkPattern: it escaped `/` but not `\`, so an input backslash could
pair with a following replacement. The slug is a hardcoded constant with
no metacharacters, making it unexploitable today, but the partial escape
is wrong as written and blocks the CodeQL gate.
Escape the full regex metacharacter set with backslash first in the
class. The generated pattern source is byte-identical for the current
slug, and the links/manifests checks still resolve all 146 links.
* fix(agent): scale subtask timeout linearly with effort review rounds
Previously the timeout applied a flat 1.5x multiplier whenever
ReviewRounds > 1, giving both medium (2 rounds) and high (3 rounds)
the same deadline. Replace this with a linear formula:
timeout = base × ReviewRounds(), producing 10/20/30 min defaults
for low/medium/high effort respectively.
* chore: bump default --timeout from 10 to 15 minutes
The base unit of 10 min was too tight for real-world reviews.
Raising it to 15 min gives linear scaling of 15/30/45 min
for low/medium/high effort respectively.
Update CLI flags, action.yml, docs, and skill files.
* test: update action-contract assertions for 15-min default timeout
* docs(ko): complete the Korean docs locale
Final batch of the ko rollout started in #993 and continued in #1072 /
#1080. Translates the last six pages and promotes ko to a complete
locale.
New pages: architecture, mcp, viewer, telemetry, contributing, faq.
Code blocks, flags, config keys, CLI/program output, mermaid sources and
URLs stay verbatim; only prose, table text, and shell/tree comments are
translated. Terminology follows pages/src/i18n/ko.ts and the three
earlier batches (코멘트 · 라운드 · 서브 Agent · 프리셋).
koDocs is now Record<DocSlug, string> instead of a partial map, and the
per-page English-fallback comment is dropped — every slug is translated.
Entries are reordered to match the en/zh/ja key order.
DOCS_LOCALES in check-translation-sync.js now includes "ko", so a future
en docs change asks for its ko counterpart like it does for zh/ja/ru.
The test fixtures that enumerate the counterpart locales are updated to
match.
Also backfills explicit {#en-slug} heading anchors on quickstart,
installation and configuration. Those three predate the anchor
convention adopted in #1072: generateHeadingId() keeps only
[a-z0-9一-鿿], so their Korean headings collapsed to empty and
colliding ids (10 of 18 empty on installation). Deep links into them
were dead — including ../configuration/#timeouts and
../configuration/#reuse-existing-environment-variables, which the new
faq page links to.
Verified: ko heading-id sequence is identical to en on all 16 pages, no
empty ids, every in-page and cross-page fragment link resolves, no {#…}
marker leaks into rendered text. npm run typecheck / lint / test (32) /
build / size (94.68 kB of 150 kB), verify-english-only.go,
verify-license.sh, check-translation-sync tests, plus headless-Chrome
renders with --accept-lang=ko-KR on all nine touched pages.
One deliberate divergence from en: en/faq.md links to #filtering--rules
(two hyphens), which no heading generates — the ko page links to the
working #filtering-rules.
* docs(ko): space before parenthetical qualifiers in h2/h3 headings
The right-hand TOC strips `()` from heading text (extractHeadings.ts),
so `Plan(선택)` renders as `Plan선택` with the words fused. English
headings keep a space before the parenthesis and read fine after the
strip; five Korean headings did not. Adds the space, matching the
spacing already used by `NPM (권장)` and `Homebrew (macOS / Linux)`.
Explicit {#en-slug} anchors are unchanged, so no link breaks.
On every push the reusable Action re-reviews the whole merge-base range, so
later pushes on a large PR cost the same as the first one. This adds an opt-in
`checkpoint_range` input that records the last completely reviewed head in the
sticky summary and narrows the next run to checkpoint..head.
Every gate fails closed to a full review: force-push, base change, config or
rule change, incomplete prior run, or any resolver error.
The config fingerprint hashes one environment variable per axis instead of a
single "|"-joined string. The joined form let a value containing the separator
shift the field boundaries, so `route_severity_below=low|a` with an empty
`route_categories` hashed identically to `low` with `route_categories=a|` —
a checkpoint surviving a config change that should have invalidated it. It also
made the digest depend on how the YAML happened to be wrapped.
`llm_extra_headers`, `llm_auth_header` and `llm_timeout` are axes too: extra
headers can point the same `llm_model` string at a different backend model or
provider, and a timeout change shifts which runs finish. Header values can be
credentials, so that axis goes in as a digest rather than raw.
The author check is pinned to `github-actions` when `github_token` is the
default one, which always belongs to that app. Unpinned, `dependabot[bot]`,
`renovate[bot]` and any linter App all read as ours. A caller-supplied token
keeps the wider bot check, since an installation token cannot ask GitHub which
app it is.
A version `ocr version` does not print yields an empty fingerprint rather than
a `spec:` fallback: `ocr_version` defaults to "latest", so the spec is a
constant that distinguishes nothing. Empty matches no stored fingerprint and
blocks the checkpoint write — don't narrow, don't advance.
The contract suite asserts examples/github_actions/README.md's documented
defaults, so a README change has to be able to trigger it. Expressing that
inside ci.yml meant inverting paths-ignore into a three-stage paths filter,
which also pulled the whole matrix — Go race tests, native Windows,
cross-compile — on a markdown-only change.
Move the job into its own workflow with a narrow positive paths filter and
restore ci.yml's original paths-ignore. The job keeps the self-hosted runner
and the node:24 container it was given in review, so the queueing behaviour
does not change.
The workflow assertions in action-contract.test.js now read the new file;
their regexes are unchanged. Since that file holds a single job, the
checkout SHA-pin and container-image assertions become structurally precise
instead of incidentally so: previously they searched all of ci.yml, so a
floating tag on this job could still pass as long as some pinned action
existed elsewhere in the file.
`echo "$header" | grep -q ...` races. grep -q exits on the first match, so
echo can die of SIGPIPE (141); `set -o pipefail` makes that the pipeline's
status, and verify-license.sh reports a header that is present as missing.
Measured on a 410-byte header under CPU load: 4 spurious failures in 3000
iterations, naming a different file each time.
has_header() in add-license.sh has the same race, and there a false negative
makes add_header() prepend a second copyright block to a file that already
has one.
Feed the header through a here-string instead, and read the year with bash's
regex match rather than a grep | grep | head chain that can take SIGPIPE the
same way.
Replace the plain-text badge line in GitHub review and summary comments
with a single static shields.io badge in the format
img.shields.io/badge/<category>-<severity>-<color>, with the color keyed
off severity (low green, medium orange, high red, critical darkred).
The image renders only when both fields are known enum members; missing
or unknown metadata falls back to the existing plain-text badge so
arbitrary model output is never hotlinked into an image URL. The image
alt text keeps the plain-text badge content for screen readers and
image-load failures, the hidden ocr-id marker stays first, and the CLI's
buildBadge output is unchanged.
* 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.
* 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
* chore: add SPDX license headers to all source files
Add Apache-2.0 SPDX license identifiers and copyright notices to all
tracked .go, .sh, .js, .mjs, .ts, and .tsx source files.
Introduce scripts/verify-license.sh and scripts/add-license.sh for
automated verification and bulk addition of license headers. Integrate
the check into CI (ci.yml) and the Makefile (license-check target as
a prerequisite of the existing check target).
This satisfies the OpenSSF Best Practices Badge requirements for
copyright_per_file and license_per_file.
* fix: restore execute permissions on scripts
* docs: add license header instructions to CONTRIBUTING guides
* docs: add license header instructions to pages contributing guides
* fix(pages): strip unclosed HTML comment markers to satisfy CodeQL
* fix: apply code review suggestions for license scripts
- Fix portability: detect macOS vs Linux stat for permission copy
- Fix has_header: check both SPDX and copyright (match verify logic)
- Fix is_ignored: match on path boundaries to avoid false positives
- Fix year extraction: use consistent pipeline across both scripts
- Fix Bash 3.2 compat: quote array length expansion for set -u
* fix(pages): use loop-until-clean for HTML comment stripping (CodeQL)
* fix(pages): use split/join instead of replace to avoid CodeQL false positive
CodeQL's js/incomplete-multi-character-sanitization rule flags any
.replace() that removes multi-character sequences like '<!--...-->',
regardless of context. The data here comes from readFileSync on the
project's own index.html (no untrusted input), making this a false
positive. Using split(regex).join('') achieves the same result without
triggering the taint-tracking rule.
GitHub rejects an entire createReview batch with HTTP 422 when any one
inline comment points at a line outside the PR diff. The previous fallback
degraded to N separate per-comment reviews, producing N timeline entries
for what is logically one review — the churn described in #624.
The batch path now filters out the comments it can PROVE are unresolvable
against the PR diff and re-sends the survivors as a single review. Dropped
comments are reported in the summary with their original span.
Correctness constraints, each of which can make things worse if relaxed:
- Classification is tri-state (valid/invalid/unknown), not boolean.
"invalid" is a claim that must be proven, because it permanently routes
a finding to the summary without ever attempting to post it. Missing
diff metadata — a patchless binary or oversized file, a truncated file
walk — is "unknown", not "invalid". Treating absent metadata as proof
of out-of-diff would silently void an entire batch with zero posting
attempts, which is strictly worse than the pre-existing behavior.
Only "valid" comments are re-batched; "unknown" keeps the per-comment path.
- The 422 gate is conservative. GitHub documents 422 on this endpoint as
"Validation failed, OR the endpoint has been spammed", so the status
code alone is not evidence of a line-resolution problem. The fallback
activates only on a confirmed line/diff validation error; anything else,
including spam/abuse detection, falls through to the per-comment loop.
- Both the primary and the secondary batch run the same cooldown and
idempotency reconciliation (cooldownAndReconcile). A secondary batch
that fails with a 5xx or network error may still have LANDED with its
response lost; retrying it blindly would repost every comment and
recreate the exact duplication this change removes.
- Diff hunks are parsed as {start,end} ranges, one per hunk, so a
multi-line span must satisfy start_line <= line within a SINGLE hunk.
A flat per-file line set would accept cross-hunk and reversed spans,
which GitHub then rejects with another 422.
The diff inventory is fetched once per run and paginated through the
existing readWithPacing helper, so it shares the repo's read retry and
pacing discipline rather than bypassing it.
Verified against live GitHub, not only against mocks. Driving the real
runPostReviewComments at a real PR with two in-diff and two out-of-diff
comments produced: one 422 on the initial batch, one listFiles read, one
successful secondary batch, and EXACTLY ONE review timeline entry holding
the two surviving comments, with the two unresolvable ones reported in the
summary. The observed 422 body is
{ "message": "Unprocessable Entity",
"errors": ["Line could not be resolved and Line could not be resolved"],
"status": "422" }
which pins two things the implementation silently depended on. First, this
endpoint returns errors[] as plain STRINGS, so the structured entry.field
branch is unreachable here (kept as defensive cover for other endpoints,
now annotated as such). Second, the decisive wording reaches us only through
Octokit's composed error.message; response.data.message alone is
"Unprocessable Entity" and matches no pattern, so narrowing the gate to the
structured field would disable the fallback while leaving every test green.
That footgun is now documented at the call site and pinned by a regression
test built from the verbatim live payload.
Known limitations:
- The 422 gate matches known line-resolution wordings. Live probing found
"Line could not be resolved" (line outside a hunk, past EOF, negative,
LEFT side, or a span straddling hunks), "Start position could not be
resolved" (inverted span) and "Path could not be resolved" (path not in
the PR), all covered by /could not be resolved/i. A future wording
outside these patterns would fall through to the per-comment loop:
never worse than the pre-existing behavior, but it would quietly
disable the grouping.
- Only RIGHT-side comments are modeled. A LEFT-side comment classifies
as "unknown" and takes the per-comment path. The producer emits
RIGHT-side comments today.
npm run test:github-actions passes, covering secondary landed-then-5xx
reconciliation (fully and partially landed), secondary 429 cooldown
followed by the per-comment retry, non-line 422s, patchless files,
cross-hunk and reversed spans, listFiles truncation, and inventory caching.
Refs: #624
Wire ru into the docs site language switcher and i18n strings, add
quickstart/installation translations, README Russian screenshots, and
include ru in the docs translation-sync guard. Remaining docs pages
fall back to English.
Add category/severity-aware, fail-open publication controls to the reusable
GitHub Action: render a CLI-consistent `[category · severity]` badge on every
comment, and add one opt-in routing destination that moves low-severity or
selected-category findings from inline comments to the PR summary.
No finding is ever silently dropped: unknown/malformed metadata on a finding
never matches the policy (routes to its normal inline destination), and a
malformed policy itself degrades to no-routing. A new `routed` accounting
bucket is disjoint from summary/skipped/failed, so destination counts still
sum to the raw input total.
- buildBadge: byte-matches the CLI's buildBadge degeneration
([cat · sev] / [cat] / [sev] / ""), with control-char sanitization that is
intentionally stricter than the CLI (strips \t/\n to defend Markdown layout).
- buildPolicy / routeComment: pure fail-open policy decision. A finding matches
when its severity is at-or-below the threshold OR its category is in the list;
unknown metadata never matches.
- Partition loop: routing is a placement decision (route OUT of reviewComments),
so routed findings never enter any createReview write path (no double-post on
retry) and carry no idempotency id.
- Accounting: new comments_routed output and summary bullet; render order is
counts -> no-line -> routed -> failed.
- action.yml: opt-in route_severity_below and route_categories string inputs
(empty defaults = today's behavior) plus the comments_routed output.
With no routing input set, behavior is byte-equivalent to today except for the
additive badge prefix on comments that carry category/severity metadata.
* feat(actions): chunk inline comments into bounded batches (#479)
Split the single all-in-one createReview call into deterministic, bounded,
sequentially-reconciled batches so a large review (e.g. 71 comments) no
longer fails after partial success against GitHub's practical limits.
- New review_comment_batch_size action input (default 50) plumbed through
OCR_REVIEW_COMMENT_BATCH_SIZE env to a reviewCommentBatchSize param.
- Sort toSend (path -> start_line -> end_line -> original index) before
partitioning so identical reruns reproduce identical batches.
- Factor the per-batch publish + reconcile + per-comment fallback block into
a publishBatch helper and drive it in a sequential for..of loop over the
chunked, sorted toSend. The reconciliation/idempotency machinery
(findExistingBatchReview / getPostedCommentIds / newCommentId fence /
computeRetryDelayMs) is reused unchanged; cross-batch dedup works because
getPostedCommentIds returns a server-global fence-ID set.
- Counts stay exhaustive and mutually exclusive across batches
(success + failed == toSend.length).
- Extend setStatsOutputs with batches_total / batches_attempted /
batches_succeeded / batches_reconciled and a batch_summary JSON output;
the existing five outputs are unchanged.
- Extend the test mock to discriminate batch vs per-comment calls by
body === REVIEW_TAG (callIdx === 0 is unsound under multi-batch) and add
a per-batch error spec.
Adds 11 new tests covering the AS1-AS6 acceptance scenarios and the B1-B7
invariants (partitioning, N=1, N>toSend, partial-success per batch,
exhaustive counts, reconcile-unavailable multi-batch, invalid N fallback,
telemetry outputs, plus pure-helper unit tests).
Closes#479.
* chore(actions): address review feedback on #479
- Remove dead READ_SUCCESS_DELAY / READ_LOW_REMAINING_SPACING declarations
from publishBatch: the read-API pacing is handled internally by
readWithPacing() for the read calls (findExistingBatchReview /
getPostedCommentIds / isCommentAlreadyPosted), so these local consts
were carried over from the original monolithic catch block and never
referenced. Leaves a comment noting where read pacing lives.
- Drop the planning/reasoning notes under thoughts/ from the branch;
they were local development artifacts and added noise to the diff.
* ci: add translation-sync guardrails for READMEs and docs
Add a check-translation-sync script + tests wired into CI: a blocking
check that all five README.<locale>.md files share an identical level-2
heading structure (compared by structure not translated text, and
code-fence-aware), and a non-blocking warning when a docs/en page changes
without its zh/ja counterparts. Addresses #419.
* ci: address review feedback on translation-sync guardrails
- Move the translation-sync job out of ci.yml into its own
.github/workflows/translation-sync.yml, scoped with paths: filters
(README*.md, pages/src/content/docs/**, the checker scripts) so it
only runs on translation changes and never blocks the core pipeline.
- runReadmeCheck: short-circuit with exit 1 when any expected README*.md
is missing, before the structure comparison, so a missing file can no
longer mask a real divergence.
- extractHeadings: add a TODO(commonmark) note that closing fences are
matched by fence char only, not by length.
- test: drop the dead en/zh fixtures and the void statements in
testReorderedHeadingFails; keep the en2/zh2 pair that actually drives
the assertion.
* ci: fix length-unaware code-fence parsing and pin node image
Address the automated review comments beyond the maintainer's minor note:
- extractHeadings: track the opening fence length and only close on a
same-char run that is at least as long (per CommonMark). Previously a
block opened with ```` and containing an inner ``` closed early, so
lines inside the block could be misread as headings (or real headings
dropped), producing false-positive structure-divergence errors. Adds a
regression test covering the inner-shorter-fence case.
- translation-sync.yml: pin node:24.18.0 instead of the mutable node:24
major tag, matching the golang:1.26.5 pin used by the other jobs.
* 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
Replace GitHub releases download with `npm i -g` so platform package
installations also get auto-updates. On permission failure, write a hint
file that bin/ocr.js reads to prompt the user to update manually.
Internal npm registry (anpm) only allows specific scopes like @ali,
not @alibaba-group. Derive scope from OCR_PKG_NAME and apply it to
platform subpackage names during publish. Also make platform.js read
package names from optionalDependencies dynamically instead of relying
solely on the hardcoded scope.
Ship Go binaries inside per-platform npm packages (@alibaba-group/ocr-{os}-{arch})
so npm install resolves the correct binary via optionalDependencies + os/cpu fields.
This removes the need for a postinstall download from GitHub Releases, which is
extremely slow for users behind restricted networks (e.g. China mainland).
The postinstall download is retained as a fallback for --no-optional installs.
Previously, checksum failures in install.js were silently downgraded to
warnings, allowing unverified binaries to be installed. Similarly,
update.js accepted HTTP registries and skipped verification when no
matching platform entry was found.
Now both scripts treat checksum verification as mandatory when configured:
all failure paths (download error, compute error, mismatch, missing
platform entry) abort the operation and clean up downloaded files.
update.js also drops the http module and silently skips version checks
for non-HTTPS registries.
Unify BUILD_PLATFORM macro with optional suffix parameter to eliminate
duplication, centralize IS_WINDOWS/BINARY_NAME exports in install.js,
add Windows-safe rename-then-replace strategy in update.js, and add
PATH guidance in README for Windows users.
* feat(build): add Windows platform support
Add windows/amd64 and windows/arm64 to CI build matrix, Makefile
cross-platform targets, and handle .exe suffix in install/update
scripts and binary wrapper. Skip chmod on Windows where unsupported.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* docs(README): add Windows download instructions
Add Windows x86_64 and ARM64 binary download commands to both
English and Chinese README install sections.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Unify the config folder name to `.opencodereview` across all runtime paths,
tests, docs, and i18n strings. Also make defaultConfigPath() return an error
instead of silently falling back to a current-directory file when $HOME is
unresolvable.
Add generic publish pipeline that supports both internal and external
publishing via environment variables, without leaking any internal URLs
or tool references into the public repository.
Keng Lim