* ci(release): publish to npm via trusted publishing
Replace the NPM_TOKEN secret with GitHub OIDC token exchange, and add a
manually triggered job that verifies every package's trusted publisher
configuration without publishing anything.
npm only accepts an OIDC exchange from a cloud-hosted runner, so those two
jobs run on ubuntu-latest while the Go build matrix stays on self-hosted.
* fix(ci): harden npm trusted publishing checks
* fix(test): encode all slashes in release contract URLs
* fix(ci): pin workflow action references to full commit SHAs
Extend the #836 pin discipline from action.yml to every workflow: all 21
external `uses:` references across the 8 workflow files now carry a full
40-hex commit SHA with a strict `# vX.Y.Z` comment, each SHA verified
against the commit its version tag resolves to. Majors are unchanged
(translation-sync stays on checkout v4), so behavior is identical — only
the tags can no longer move. release.yml matters most: it holds the npm
token and the attestation id-token, so a moved upstream tag there had
credential blast radius (#841).
verify-action-pins.sh now covers .github/workflows/*.yml as well. This
also restores the review-requested hardening from #836 that raced the
merge and never landed on main (pushed as 37fc06f eight minutes after
the merge commit): full-line anchoring with a strict version comment, a
directive pre-filter so commented examples are skipped, and the grep -n
prefix strip. The extension surfaced why that matters immediately -
without the pre-filter the check flags codeql.yml's commented
setup-example line.
Fixes#841
* fix(scripts): fail closed on flow-mapping uses entries and cover .yaml workflows
Adversarial review of the extended check found two coverage gaps. A
flow-mapping entry (`- {uses: actions/cache@v6}`) slipped past the
anchored directive filter that block-style lines go through — the old
unanchored grep would have flagged it, so the anchoring traded a comment
false-positive for a narrow fail-open regression. The directive now
matches the flow opener too, and since the pinned pattern never accepts
flow syntax, such entries fail closed as unusual rather than vanishing.
The file glob also only covered *.yml; GitHub executes *.yaml workflows
just the same, so those join the set under nullglob (an absent glob
would otherwise survive as a literal and trip the missing-file guard).
* fix(scripts): scope the pin check back to action.yml
The repo-level SHA-pinning setting covers this repository's own
workflows at job start; the script keeps guarding the layer that
setting cannot see — nested uses: inside the published composite
action.yml, which only execute in consumers' workflows.
Co-authored-by: wu21-web <149216116+wu21-web@users.noreply.github.com>
* fix(ci): pin newly-added workflow action references to full SHAs
Rebasing onto main surfaced floating tags in workflows added after this
branch diverged: idea-ext.yml (checkout, gradle wrapper-validation,
setup-java), ci.yml (windows job checkout, setup-go), and frontend-ext.yml
(checkout, cache). Pin them to the same 40-hex SHAs used elsewhere so the
whole workflow set is SHA-pinned, matching the goal of #841.
* style(ci): use full version in checkout pin comments
action-contract.yml and plugin-contract.yml already pin the full SHA but
their trailing comments read # v7; normalize to # v7.0.1 to match the
rest of the repo. Comment-only change, no behavior difference.
* fix(scripts): catch quoted uses: keys in the pin-check directive
A quoted mapping key ("uses": / 'uses' :) is valid YAML but did not match
the directive filter, so such a line was silently skipped and a floating
tag inside it escaped the check. Broaden the directive to match quoted keys
and space-before-colon; the pinned pattern still rejects those forms, so
they fail closed as unusual syntax rather than slipping through.
Reported by wu21-web in review of #856.
* fix(scripts): extend pin check to workflow files
Scope verify-action-pins.sh back over .github/workflows/*.yml and *.yaml,
not just action.yml. Workflows carry credentials (release.yml holds the
npm token and attestation id-token), and a moved upstream tag there is not
otherwise caught: floating tags reach main today, so runtime enforcement
is not blocking them. The check now fails closed on any future floating
reference in a workflow. All current references already pass.
---------
Co-authored-by: Fanzzzd <fanzzzd@users.noreply.github.com>
Co-authored-by: wu21-web <149216116+wu21-web@users.noreply.github.com>
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
Co-authored-by: kite <lizhengfeng.lzf@alibaba-inc.com>
The Configure OCR step derived llm.protocol from llm_use_anthropic alone,
so it always wrote openai or anthropic. Anyone on the Responses API was
sent to /v1/chat/completions no matter what OCR_LLM_PROTOCOL they set.
Add an optional llm_protocol input (anthropic, openai, openai-responses).
When it is set, or when OCR_LLM_PROTOCOL is inherited from the job
environment, that value is written and llm.use_anthropic is mirrored
from it, the same way `ocr config set llm.protocol` mirrors the boolean.
An unknown value fails the step with the accepted list. The explicit
protocol joins the checkpoint fingerprint, appended only when set so
existing checkpoints stay valid.
Fixes#1134
* chore(idea): translate tests and remove broad language exemption
* fix(ci): retain scoped IDEA translation exemptions
* docs(idea): refine test wording and technical explanations
* docs(idea): simplify test rationale comments
* feat(idea): add IntelliJ IDEA plugin extension
Add a new JetBrains IDE extension under extensions/idea/ that brings
the Open Code Review workflow to IntelliJ-based IDEs.
The implementation mirrors the VSCode extension architecture:
- JCEF-based webview for sidebar and config panel (shared HTML/JS bundle)
- Same CLI integration (ocr review, config, install)
- Git integration via command-line git (branch/commit/workspace modes)
- Comment mounting with RangeHighlighter and inline panels
- i18n support (en/zh-cn), matching the VSCode extension
- Theme-aware CSS variables synced from IDEA's LookAndFeel
195 tests pass across 15 suites. ./gradlew runIde launches successfully.
Closes#499
* fix(idea): harden process/threading/disposal + subdir jump path-basis
Address ocr code-review findings across the host side:
- CliService: probe/runRaw/install process lifecycle (bounded joins,
closeStreamsQuietly, SIGTERM+grace, independent install slot, atomic
current claim), cancel getAndSet, install stale killStaleInstall.
- GitService: stdout UTF-8, reader join on timeout, getState synchronized,
isRelevantTo case-insensitive + os.name hoist, safeRef injection guard,
getCommitFiles dedup.
- ConfigService: atomic temp+rename write, dir 700, readRaw log, setMany
rollback + failing-key log, @Synchronized writers, trySetPosix Windows skip.
- JCEF disposal: EDT-safe browser/dialog teardown, idempotent dispose,
PCE rethrow, init try-catch self-cleanup, @Volatile channel.
- ReviewSession: cancel CAS, run-before-cancel guard, catch Exception+PCE.
- SidebarRouter: CancelReview target capture + identity guard, startReview
cancels stale session.
- CommentService: jump path-basis fix (resolveProjectFile uses repoRoot to
match repo-relative comment paths when project opened at a subdir).
- Misc: HostStrings single-pass param replace, IdeaTheme alpha clamp +
font-name sanitization, CliParse isNotBlank, Providers Locale.ROOT,
ConfigUtils isNotBlank + key normalization, ConfigDraft preset-key
normalization, WebviewHtml BOM strip + IOException fallback.
build.gradle mavenCentral fallback, jest SPDX header, webpack mode,
gradle/settings trailing newline.
* Update extensions/idea/src/main/kotlin/com/alibaba/opencodereview/idea/jcef/JcefConfigPanelHost.kt
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* Update extensions/idea/src/main/kotlin/com/alibaba/opencodereview/idea/messages/HostMessages.kt
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* fix(idea): update HostMessagesTest for explicitNulls=true + trailing newline
- HostMessagesTest: 3 assertions updated (assertFalse→assertTrue + is JsonNull)
to match explicitNulls=true (null fields now present as "field": null)
- HostMessages: comment updated to reflect explicitNulls=true
- CommentService: trailing newline
* fix(idea): parsePorcelain arrow parsing for filenames containing ' -> '
When a filename contains ' -> ' (spaces + arrow), git quotes it. The old
code used indexOf(" -> ") which matched the arrow INSIDE the filename
instead of the separator, producing a corrupted path.
Fix: prioritize '" -> ' (closing quote + separator, 5 chars) which
unambiguously finds the separator for quoted paths. Fall back to ' -> '
for unquoted paths (no spaces → no false match).
Tests: 3 new cases — quoted rename, arrow-in-name rename, arrow-no-space.
* test(idea): add parsePorcelain case for unquoted-old/quoted-new rename
* chore(idea): remove unused JComponent import in OcrToolWindowFactory
* fix(idea): log JSON parse errors inside parseRawConfig
parseRawConfig's internal runCatching swallowed JSON errors silently.
readRaw's outer runCatching only catches IO errors (readText), not JSON
parse errors. Added Logger.warn inside parseRawConfig so JSON errors are
also recorded.
Note: thisLogger() unresolved from top-level function in ConfigDraft.kt
(likely inline-function context issue); used Logger.getInstance instead.
* fix(idea): path traversal guard in readWorkspaceFile + isBinaryFile
File(root, relPath) without validation allows ../../etc/passwd to
escape the repo root. Added canonicalPath.startsWith(root + separator)
check, wrapped in runCatching for graceful degradation on IO errors.
pathExistsAtRef not affected — uses git ref:path syntax which git
itself validates (path stays within repo tree).
* Update extensions/idea/src/main/kotlin/com/alibaba/opencodereview/idea/services/ConfigService.kt
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* fix(idea): @Synchronized on set() to prevent race with deleteCustomProvider
set() goes through cli.runRaw (external CLI process) to write config.json,
bypassing the @Synchronized lock that deleteCustomProvider/writeRaw use.
Adding @Synchronized serializes set with deleteCustomProvider, preventing
interleaving that could silently undo config changes.
* style(idea): extract magic strings + trim CLI args + remove unnecessary toList()
- OpenOcrToolWindowAction: extract tool window ID to companion const
- ReviewSession: extract 'completed_with_errors' to named constant
- CliParse: trim --from/--to/--commit args (consistent with --background)
- CliParse: remove unnecessary .toList() in extractCliError (sequence is enough)
* style(idea): FONT_NAME_INVALID add { symmetry + normalizeLine val expression
- IdeaTheme: add { to FONT_NAME_INVALID char class (symmetric with })
- CommentAnchor: var s → val s with expression body in normalizeLine
* style(idea): locale when-exhaustiveness + Linux shell fallback
- HostStrings: if(locale==ZH_CN) → when(locale) for compiler exhaustiveness
- ShellEnv: /bin/zsh fallback → macOS zsh / Linux bash (portability)
* fix(idea): @Volatile on GitService callback properties
diffDecorator and diffViewerReady are written from ReviewProjectService
(on init/dispose) and read from pooled threads (git operations). Without
@Volatile, the pooled thread may not see the latest value. Added @Volatile
for cross-thread visibility.
* fix(idea): close process streams in ShellEnv.capture via finally
Added closeStreamsQuietly() extension on Process (matching CliService
pattern) and a finally block in capture() to ensure inputStream/
outputStream/errorStream are closed on all three exit paths (timeout/
normal/interrupt). Previously only outputStream was closed (stdin at
start), inputStream fd leaked until GC.
* fix(idea): clear stale maps in show() + guard malformed FAILED + close stale streams + fix leftRefFor fallback
- CommentService: clear jumpBlockReasons/locateNotes in show() first synchronized
block (was only cleared later in clearHighlighters, leaving a window for EDT
jumpTo to read stale entries from previous review)
- SidebarRouter: only post FAILED for Malformed when session.get() != null
(was unconditionally overriding IDLE/DONE state when no review was running)
- CliService: close stale process streams after eviction in runRaw
(killStaleInstall had closeStreamsQuietly, runRaw did not)
- GitService: leftRefFor mergeBase failure returns null instead of falling back
to 'from' (was inconsistent with getBranchDiff's three-dot merge-base diff)
* fix(idea): resolveProjectFile use toRealPath to resolve symlinks
normalize() only resolves .. and . lexically, not symlinks. A symlink
inside the repo pointing outside (e.g. evil_link.txt -> /etc/passwd)
passes the startsWith check but refreshAndFindFileByNioFile resolves
to the symlink target — reading files outside the repo.
Switched normalize() to toRealPath() which resolves all symlinks,
so the startsWith check catches symlink escapes.
* fix(idea): add disposed guard to scheduleFullRepaint
Consistent with forceFullRepaint which already checks disposed.
Prevents restarting stopped timers after dispose().
* fix(idea): register OcrWebview with proper Disposer parent to fix memory leak
OcrWebview's constructor calls messageBus.connect(this), which adds
OcrWebview to the Disposer tree under ROOT_DISPOSABLE (no parent).
Neither JcefReviewPanel nor JcefConfigPanelHost registered OcrWebview
as their child, so on IDE shutdown Disposer found 11+ leaked instances.
Fix: Disposer.register(this, webview) in both JcefReviewPanel and
JcefConfigPanelHost. webview.dispose() is idempotent, so the existing
manual dispose calls in dispose() methods are safe alongside Disposer.
* refactor: extract shared frontend to extensions/frontend/
- Move webview/ and shared/ from vscode/src/ to extensions/frontend/src/
- Split bridge.ts into bridge.vsc.ts and bridge.idea.ts
- Shared webpack.config.js with OCR_TARGET env var (NormalModuleReplacementPlugin)
- VSC webpack.config.js: only extension host config; webview build delegates to extensions/frontend
- IDEA build.gradle.kts: frontendDir points to ../frontend, OCR_TARGET=idea
- VSC extension host imports: ../../shared → @shared (tsconfig paths + webpack alias)
- ts-loader: transpileOnly=true (type checking via tsc separately)
- Delete extensions/idea/frontend/ (no longer needed)
- Delete extensions/vscode/src/webview/ and src/shared/ (moved)
Both VSC and IDEA builds verified: extension.js, webview.js, configPanel.js all produced successfully.
* chore: update READMEs and tests for shared frontend extraction
- idea: update architecture docs, remove stale UPSTREAM.md references
- idea: bump version to 0.3.0
- idea: add MessageContractTest for host↔frontend message type drift
- vscode: update architecture docs for shared frontend at extensions/frontend/
- vscode: fix test imports (../../../shared → @shared) broken by extraction
* chore(idea): use brand plugin icons and expand description for marketplace listing
Replace the template default plugin icon with the project brand icon
(shield+eye, transparent background, aligned with the VS Code extension);
add pluginIcon_dark.svg for dark IDE themes. Expand plugin.xml description
with features/prerequisites HTML for the JetBrains Marketplace listing.
* fix(idea): terminate CLI process tree to prevent orphaned grandchildren
Process.destroy()/destroyForcibly() only signal the direct child. The
globally-installed ocr CLI is a Node launcher that spawns the Go binary
as a grandchild; killing only the launcher orphans the Go process, which
gets reparented to the system root and keeps running after cancellation.
Snapshot descendants before SIGTERM (handles stay valid after the parent
dies, while live enumeration goes blind due to reparenting), then
tree-kill from the snapshot after the grace period, parent first so
supervisor-like parents cannot respawn children. Applied to all six kill
sites: cancel, runRaw stale/finally, install finally, killStaleInstall,
and probe timeout. Fallback sites drop the isAlive guard to shrink the
check-then-enumerate TOCTOU window, and kill helpers never throw so
stream cleanup in finally blocks is always reached.
Adds CliServiceTreeKillTest: spawns a fake CLI with a grandchild,
cancels, and asserts the grandchild dies (POSIX only).
* build(idea): enable local Plugin Verifier and fix deprecated JCEF API
Configure pluginVerification so ./gradlew verifyPlugin has an IDE to
check against (2.x requires an explicit list); this only wires the
verification task — the packaged zip is byte-identical before/after.
Switch OcrWebview to the JBCefBrowserBase overload of JBCefJSQuery.create:
the JBCefBrowser overload is scheduled for removal and was flagged by
the verifier. Verifier verdict is now clean ("Compatible").
* fix: add .gitignore for extensions/frontend, remove node_modules from tracking
* fix(extensions): satisfy license and english-only CI checks
Add missing SPDX headers to shared config files, translate the
remaining Chinese comments in extensions/frontend, exempt the webview
i18n copy table in english-check, and untrack extensions/.idea.
* fix(idea): validate review results and isolate cancellation
* chore(idea): drop internal handoff notes
* ci(extensions): cover frontend and idea, restore webview type checking
- bridge.ts facade lets tsc resolve ./bridge, which only webpack's
NormalModuleReplacementPlugin could map before (typecheck was broken)
- build:webview installs frontend deps first so clean clones build
- add frontend-ext and idea-ext workflows; extend vscode-ext paths filter
* ci(idea): drop gradle cache upload on self-hosted host
The idea job runs on the self-hosted host without a container, so
~/.gradle (including the ~1GB IDE distribution) already persists
between runs. setup-java's post-job cache save tried to upload a
2.4GB archive and stalled the runner, leaving the job red despite
green tests.
* chore(extensions): add SPDX headers to kt, kts and css sources
Upstream #1407 extends the license gate to kt/kts/css; the IDEA plugin
and the shared frontend predated that requirement. Headers generated
with scripts/add-license.sh from upstream/main.
* chore(scripts): temporarily exempt extensions/idea from english-only gate
The plugin's comments and zh-cn UI strings are still Chinese. Mirror
the extensions/vscode/ precedent with a TEMPORARY allowedPrefixes
entry, dropped once the sources are translated. Takes effect together
with upstream #1407, which adds .kt/.kts to scannedExts.
* chore(vscode): drop accidentally committed package-lock.json
The frontend-extraction refactor ran npm install inside the vscode
extension package and committed the resulting npm lockfile. That
package is yarn-managed (yarn.lock, yarn install --frozen-lockfile
in CI), so the npm lockfile is redundant and risks drifting out of
sync with yarn.lock. Nothing references it.
* chore(extensions): gitignore the foreign lockfile in each subpackage
extensions/vscode is yarn-managed and extensions/frontend is
npm-managed; ignore the other manager's lockfile in each so an
accidental npm/yarn install cannot sneak a second lockfile into the
repo again.
---------
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
The updater runs detached and has no console, so both of its spawnSync calls
made Windows allocate one: an empty window that also blocked the call until a
terminal attached, past the probe's 3s timeout. windowsHide maps to
CREATE_NO_WINDOW.
The license gate listed only go/sh/js/mjs/ts/tsx, so eight Python and six
CSS source files carried no SPDX header while still passing CI. Backfill
those headers, and cover kt/kts before any Kotlin code lands.
* feat(action): opt-in resolution of the bot's own outdated review threads (#567)
Adds a resolve_outdated input, default off, that resolves review threads
GitHub has already marked outdated. No model is involved in the decision:
the signal is GitHub's server-computed isOutdated, and five vetoes sit on
top of it, each of which can only refuse to resolve a thread.
Three modes: 'false' (no GraphQL calls at all), 'report' (log the verdict
for every thread, mutate nothing), 'true'.
* fix(action): address review on resolve_outdated
- Veto by path instead of disabling the whole run: a finding with a path but
no line is ordinary output and now costs only its own path, while a finding
with no path at all still disables the run.
- Normalize paths before the overlap veto, so ./src/a.js and src/a.js no
longer compare unequal and resolve a thread over a live finding.
- Resolve the authenticated login once per run rather than once per caller.
- Validate resolve_outdated in the Validate inputs step, so a typo fails
before the LLM runs rather than silently falling back to off.
- Correct the permission documentation from measurement, and show the
contents: write scope in a job that never checks out the pull request.
- Split the overloaded unverified reason, fix the documented preview count,
list the missing outputs, and replace the nested ternary with a Map.
The #1156 test posted inline comments on a workflow_run event without a
manifest, tripping the input.resolved_head requirement that landed on main
in parallel. Provide a manifest so the test exercises the intended path.
* feat(action): add optional pr_number input and workflow_run fallback
The pull request number was read from the event payload in two places: the
fetch step's `github.event.pull_request.number || github.event.issue.number`,
and `context.issue.number` in the range and posting steps. Neither resolves on
`workflow_run`, whose payload names the pull request only under
`workflow_run.pull_requests[0]`. The review therefore ran to completion and the
posting step then requested /repos/OWNER/REPO/issues//comments, got a 404, and
the findings were lost.
"Resolve PR refs" now resolves the number once — the new optional `pr_number`
input, then the event payload, then `workflow_run.pull_requests[0].number` —
publishes it as $PR_NUMBER, and fails there when nothing resolves or the value
is not a PR number, before OCR is installed or any LLM quota is spent. The head
fetch, the checkpoint resolver and the posting step all read that one value;
runPostReviewComments takes it as an option and still falls back to
context.issue.number when a caller omits it.
Behavior is unchanged for pull_request, pull_request_target and issue_comment:
an unset input falls through to the existing resolution.
Closes#1150
* style(action): use strict equality for the pr_number override
The review guidelines rule out loose equality, so spell out the null and undefined checks that `!= null` covered.
* docs: refresh landing screenshots and add Korean
Regenerate the highlights and benchmark images for en/ja/ru/zh and add the Korean (ko) variants now that the site ships a Korean locale.
* docs: move localized README and CONTRIBUTING into docs/i18n/
The repo root listed five READMEs and five CONTRIBUTING files. Move the four translations of each into docs/i18n/ so only the English entry points remain in the root, and update every reference: the language switchers in the English files, the documentation-file table in CONTRIBUTING.md, the README sync rule in AGENTS.md, the README_FILES paths in the translation-sync checker, and the docs/i18n/** path trigger in the workflow. Relative links inside the moved files are rewritten to resolve from their new location.
* chore: move SECURITY.md into .github/
GitHub still surfaces the Security policy when SECURITY.md lives under .github/, matching projects like kubernetes and vite. Update the two links in GOVERNANCE.md to the new path.
* fix(pages): keep highlights stat labels aligned across locales
Long stat labels wrapped to two lines on the fixed-width column, left-aligning the wrapped text and dropping that column's caption below the others. The label style now centers text and reserves two lines (lineHeight 16px, minHeight 32px) so every column's caption shares a baseline regardless of label length or locale. Also shorten highlights.stat2Label from 'NPM COMMUNITY DOWNLOADS' to 'NPM DOWNLOADS' across all five locales so it reads directly and fits one line, and regenerate the highlights screenshots to match.
* docs: realign CONTRIBUTING documentation-file tables
The docs/i18n/ path prefix widened the first column and knocked the table pipes out of alignment. Re-pad the columns (display-width aware, so the CJK and Cyrillic rows line up too).
* docs: revert no-op benchmark screenshot re-renders
Generating the Korean benchmark image re-rendered en/ja/ru too, producing byte-different but visually identical PNGs. Restore those three to their main versions; only benchmark-ko.png (the new Korean locale) is a real addition.
* 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.
Kite