govulncheck now fails on golang:1.26.5: the Go vulnerability database
lists fixes in 1.26.6 for seven standard-library issues reachable from
this module (GO-2026-6218, -6091, -6090, -6089, -6088, -5972, -5026),
via net/url, html/template, crypto/tls, net/http, encoding/xml and
encoding/asn1. Nothing in the module's own code changed — the step
turned red when the database was updated, and because Govulncheck runs
before "Test with coverage", every PR is now blocked before a single
test executes.
Bump the pinned image in the test, cross-compile and release jobs, and
refresh the stale version reference in the translation-sync comment so
it keeps matching the other jobs.
Verified locally under go1.26.6: govulncheck reports no vulnerabilities
and exits 0, gofmt -s and go vet are clean, go mod tidy is a no-op, and
the full test suite passes (23 packages).
* 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
* fix(codeql): Workflow does not contain permissions
* 更新 translation-sync.yml
Co-authored-by: Lei Zhang <61303077+stay-foolish-forever@users.noreply.github.com>
---------
Co-authored-by: Lei Zhang <61303077+stay-foolish-forever@users.noreply.github.com>
* ci(pages): add smoke test step to verify build output is servable
Serve dist/ after build and assert key SPA routes return HTML with bundle references so broken templates cannot slip through CI.
Co-authored-by: Cursor <cursoragent@cursor.com>
* ci(pages): harden smoke test route checks
Clarify the SPA fallback limitation, make the curl failure branch work under set -e, and add a /404.html static-file assertion to confirm dist assets are actually served.
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
* test: raise statement coverage to 90% and enforce it in CI
Add unit tests across the cmd and internal packages to bring total
statement coverage above 90%, and gate future regressions.
- Cover CLI helpers, provider TUI handlers, resume/manifest paths, and
error branches in config, llm, llmloop, scan, session, agent, viewer,
mcp, pathutil, and telemetry.
- Raise the coverage threshold from 80% to 90% in the Makefile
(COVERAGE_THRESHOLD) and in the CI "Check coverage threshold" step.
- Ignore generated coverage.out and coverage.html artifacts.
Total statement coverage is now 90.5%, measured consistently by both
`make coverage` and the CI `go test ./...` scope.
* test: widen statement coverage margin with environment-independent unit tests
Add table-driven unit tests for pure, environment-independent functions to
raise the statement-coverage safety margin above the 90% threshold:
- session.ResumeState.ValidateScanOptions (70% -> 100%)
- rules.SystemRule.UnmarshalJSON error branches (71% -> 82%)
- llmloop.stripMarkdownFences no-newline branch (82% -> 100%)
- diff.firstLine empty/blank-input branch
- diff.extractCodeBlock missing-newline and no-closing-fence branches
- main.truncate n<=1 and normalization branches
- agent.Agent nil-receiver accessor guards
* 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.
Pin govulncheck to v1.6.0 instead of @latest. The argument is
reproducibility, not supply chain: golang.org/x/vuln is the Go team's
own module and go install is already checksum-verified via GOSUMDB.
What @latest costs is a new govulncheck release turning CI red on an
unchanged tree. This gate is load-bearing - e6e5da0 bumped the Go image
to fix GO-2026-5856 after govulncheck caught it - so protecting it from
unrelated churn is worth a pin.
Pinning costs nothing in scan freshness: the vulnerability database is
fetched at runtime, independently of the binary version. Verified:
Scanner govulncheck@v1.6.0, DB updated 2026-07-08, no vulnerabilities,
exit 0.
Narrow release.yml's workflow-level permissions from contents: write to
contents: read. Only the build job inherits it, and it only checks out
and uploads artifacts - upload-artifact authenticates with
ACTIONS_RUNTIME_TOKEN, not GITHUB_TOKEN. The release and npm-publish
jobs declare their own job-level permissions, which replace the
inherited set entirely, so both are unaffected.
Verified with actionlint (clean across all workflows) and by confirming
the release.yml still parses as YAML.
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.
package-lock.json is gitignored for the pages directory, so npm ci
always fails in CI (no lockfile present after checkout). Switch to
npm install to match deploy-pages.yml behavior.
The rolling `node:24` tag recently picked up a newer npm version whose
`npm ci` rejects the existing lockfileVersion-3 lockfile. Pin to
24.18.0 (same version used by translation-sync.yml) to restore
deterministic builds.
dependabot.yml declares gomod and github-actions only. The repo has 9
npm manifests, and extensions/vscode/package.json:110-116 is currently
carrying five hand-written CVE resolutions pins - undici, form-data,
js-yaml, minimatch - i.e. someone is patching CVEs by hand in a package
dependabot never looks at.
Scope is deliberately just /extensions/vscode: its yarn.lock is tracked
(Yarn Classic v1, which the npm ecosystem supports). /pages is excluded
because its lockfile is gitignored (.gitignore:13) - a maintainer policy
call, not mine. / and the six npm/*/ stubs have no real dependencies.
@types/vscode is ignored because package.json pins it to ^1.74.0 to
match engines.vscode ^1.74.0; bumping the types alone would type-check
against APIs absent from the declared minimum VS Code. Grouped to
minor/patch so the breaking majors already pending (eslint 8->9,
@typescript-eslint 6->8, @types/node 18->24, vsce ->3) arrive
individually rather than as one unreviewable PR.
* 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.
* ci: add CI job for VS Code extension
Add .github/workflows/vscode-ext.yml to validate the VS Code extension
(extensions/vscode/) on every push/PR that touches that directory.
The workflow runs on the self-hosted node:24 container (matching the
existing CI conventions) and executes:
- yarn install --frozen-lockfile (lockfile integrity)
- yarn lint (ESLint)
- yarn compile (TypeScript + webpack dev build)
- yarn test (Jest, all 10 test suites)
Path filter (extensions/vscode/**) ensures Go-only changes do not
trigger this job. No vsce package step — packaging is a release concern.
Fixes the gap identified in #441: dependency PRs from dependabot (#437)
will now receive real build and test signal before merging.
Closes#441
* ci: pin node image version and add yarn dependency caching
- Pin node container image to node:24.0.0 for reproducible builds
- Add actions/setup-node@v4 with yarn cache for extensions/vscode/yarn.lock
to speed up CI workflow runs
* ci: use node:24 image container tag
* ci: pin node version to 24.4.1
* ci: use actions/cache@v4 for yarn package caching
* ci: remove redundant push trigger on main
Add a Pages CI GitHub Actions workflow that runs on pull requests
touching pages/**, performing npm ci, typecheck, and build. This
provides quality gating for the frontend, which previously had no
PR-level checks (deploy-pages.yml only ran on push to main).
SECURITY.md:16 directs reporters to GitHub Private Vulnerability
Reporting, but the issue chooser had no route to it. With
blank_issues_enabled: false, someone holding a credential-leak bug
lands on the public Bug Report form.
Add the advisory link as the FIRST contact_link. Ordering is the
substance here: the chooser is three rows, and a reporter should not
have to read past two Discussions links to find the private channel.
Verified private vulnerability reporting is enabled on the repo
(GET /repos/alibaba/open-code-review/private-vulnerability-reporting
returns {"enabled":true}), so the link resolves to a live form.
3f41551 introduced a 15-minute timeout on the CI test job. This extends
that same policy to the five jobs that still lack one, so the convention
is applied uniformly across all four workflows.
Values are set from measured durations over three runs each, not guessed:
release build 33-38s -> 15, release 7m03-7m05 -> 20, npm-publish
3m15-3m17 -> 15, pages build 57-58s -> 10, pages deploy 11s -> 15.
pages deploy is 15 rather than 10 deliberately: actions/deploy-pages@v4
declares its own internal polling timeout of 600000ms (10 minutes), so a
10-minute job timeout would fire simultaneously and replace the action's
diagnostic error with an ambiguous job cancellation.
Matters more than usual here because the runners are self-hosted: a hung
job squats a real machine for the ~6h default.
- Change Makefile fmt/check targets from `go fmt` to `gofmt -s -w .`
to match the CI gate introduced in #433, preventing drift where
local `make fmt` passes but CI fails on simplification opportunities.
- Reduce cross-compile job timeout from 20 to 10 minutes, since each
leg completes in ~16 seconds on self-hosted runners.
CI builds one platform (linux/amd64) while release.yml ships six. The
repo has platform-gated source - shell_windows.go, procattr_windows.go
and their _unix twins - that CI never compiles, so a signature change to
shellCommand or configureProcessGroup compiles green on main and breaks
at tag time, after the tag is public.
Add a cross-compile matrix job covering the five targets the test job
does not, mirroring the shape release.yml:11-29 already runs on this
same runner pool. fail-fast: false so a failure names the target.
The job trusts the workspace the same way the test job does. The
container runs as root over a checkout owned by another uid, so git
reports dubious ownership and Go's VCS stamping - which runs when
building main packages - fails the build with "error obtaining VCS
status: exit status 128". Marking the checkout safe keeps stamping on
and consistent with the test job and the release builds, rather than
papering over it with -buildvcs=false.
Also add two cheap gates the repo lacked: gofmt -s drift (make fmt
rewrites and can never fail CI) and go mod tidy drift (make check
mutates, so it cannot gate either). Both print the remedy.
All six targets build clean today - this is a missing guard, not a fix.
The platform-package loop guards itself against an already-published
version (release.yml:209-215), but the meta package's publish was a bare
'npm publish'. Re-running a successful release - or a publish whose
client errored after the registry had already committed the version -
fails permanently at that last step with E403.
Wrap it in the same guard, reusing the identical log strings so both
paths read the same in a release log. Publishing the meta package last
is unchanged and still correct.
Every push to a PR started a new CI run while the previous one kept
running to completion on the self-hosted pool. Only the newest commit
matters, so the earlier runs were holding runners for results nobody
would read.
Adds a top-level concurrency group keyed on the PR number, falling back
to the ref for push-to-main. This follows the pattern OpenSandbox uses
across its CI workflows, and is byte-identical to the block already
running in ocr-review.yml, so it reuses the repo's existing idiom
rather than introducing a second one.
Left the other workflows alone deliberately. deploy-pages.yml already
sets cancel-in-progress: false, and release.yml must never cancel:
aborting mid npm-publish would leave the platform packages published
while the meta package's optionalDependencies reference versions that
were never pushed, and npm publish is not reversible.
Verified with actionlint v1.7.12 (clean across all four workflows) and
by exercising both paths live on a fork: a superseded pull_request run
cancelled in 38s, and a superseded push-to-main run cancelled while a
run from a commit without the block, on the same branch, stayed queued.
Closes#422
* 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
govulncheck flags GO-2026-5856 (Encrypted Client Hello privacy leak in
crypto/tls), present in the Go standard library through go1.26.4 and
fixed in go1.26.5. The CI and release workflows pin the golang:1.26.4
container image, so govulncheck fails with exit code 3 on every run.
Bump both workflow images to golang:1.26.5.
* fix(ci): add idempotency check to prevent duplicate review posts on retry
When the batch createReview fails with a 5xx/408/network error, the
request may still have landed on the server. Before retrying per-comment,
the workflow now:
- Tags each review/comment/summary with a per-run HTML comment ID derived
from runId + runAttempt + content hash.
- Queries existing reviews and review comments to detect whether the batch
actually landed, and only retries the comments that are missing.
- Before retrying an individual comment whose request may have reached
GitHub, cools down (honoring rate-limit headers) then checks whether the
comment already exists, treating it as success instead of posting a
duplicate.
- Skips posting the summary comment when one with the same run tag already
exists.
- Adds read-API retry/pacing helpers (withRetry/readWithPacing/readAllPages)
with shorter spacing than writes (OCR_READ_SUCCESS_DELAY /
OCR_READ_LOW_REMAINING_SPACING) since reads are cheaper but still consume
the primary rate limit.
Degrades gracefully to the original fallback (accepting duplicate risk)
when the idempotency read calls themselves fail.
* fix(ci): harden idempotency checks in ocr-review workflow
Address code review findings on the GitHub Actions PR auto-review
workflow (applied to both .github/workflows and examples copies):
- readAllPages: cap pagination at maxPages=50 (default) to prevent
unbounded loops, and validate the argument is a positive integer.
- getPostedCommentIds: anchor the ID regex to the HTML comment wrapper
(<!-- ocr-... -->) with a capture group to avoid false positives from
user-generated content.
- isCommentAlreadyPosted: return null (unknown) instead of false when
the read API fails, so callers do not silently risk duplicates; accept
a postedIdsCache to reuse a single paginated walk across retries.
- hasIssueCommentWithId: return null (unknown) on read API failure, and
match the summary tag with an anchored regex for consistency.
- Call sites: handle null by skipping retry/posting to avoid duplicates
while surfacing the failure in the summary.
* fix(ci): validate env config and document intentional behaviors
Address code review findings on the ocr-review workflow (applied to
both .github/workflows and examples copies):
- parseNonNegInt: add a validation helper for env-var parsing so
negative or non-numeric values (e.g. OCR_MAX_RETRIES=-5) fall back to
defaults instead of bypassing the `|| default` guard (a negative
parseInt result is truthy). All seven retry/pacing config values now
use it.
- readAllPages: document that the 50-page cap is an intentional safety
valve against unbounded loops, not a normal mode; callers that depend
on completeness already degrade safely to null (unknown), so a
truncated walk does not silently produce duplicates.
- commentId: document that the 12-hex-char (48-bit) hash collision
scope is a single PR (listReviewComments is PR-scoped) and a single
run produces only tens to hundreds of comments, making the
birthday-bound collision probability negligible (~1e-7 at 10k).
* docs(github_actions): sync README with retry/idempotency features in ocr-review.yml
- Add OCR_READ_SUCCESS_DELAY and OCR_READ_LOW_REMAINING_SPACING variables
for read API pacing used by the idempotency check
- Document the three GitHub rate-limit retry strategies (primary reset,
retry-after header, secondary no-header backoff)
- Add 'Idempotency: avoiding duplicate review comments' section describing
how the workflow detects already-landed comments via per-run HTML tags
and skips retrying when the read API is unavailable
* fix(ci): use full sha256 hash for review comment idempotency IDs
Drop the .slice(0, 12) truncation in commentId() and use the full 64-char
(256-bit) sha256 hex digest. The truncated 12-char hash carried a tiny but
nonzero collision risk whose failure mode was a silently dropped inline
comment (the idempotency check would mistake two distinct comments for
duplicates). The full hash makes the collision probability effectively
zero with no meaningful downside; the ID regex already used [a-f0-9]+ so
it accepts the longer IDs unchanged.
* fix(ci): use random per-comment IDs and defer body assembly in review workflow
Replace the content-derived commentId() (sha256 of path/line/content) with
a random per-comment ID (crypto.randomBytes) and restructure the inline-
comment flow around an item struct that carries { comment, id, lines }.
This fixes two issues in the idempotency check:
1. ID was recomputed on every failure check. Each inline comment is now
assigned one random ID up front and carried on the item struct, so the
retry/idempotency logic reads item.id directly. The comment body (which
embeds the ID) is assembled only at API-call time in toReviewPayload(),
eliminating repeated hash computation.
2. Content-derived IDs collided for distinct comments sharing the same
path/line/content. A random ID guarantees two such comments get
different IDs, so the idempotency check no longer mistakes the second
for a duplicate of the first and silently drops it on retry.
formatComment/commentId are removed (no callers remain) and replaced with
newCommentId/resolveLines/toReviewPayload/buildBody. The matching regex
already used [a-f0-9]+ so it accepts the new random tokens unchanged.
README ID-format placeholder updated from <hash> to <token>.
* docs(ci): correct misleading readAllPages truncation comment
The comment claimed 'a truncated walk does not silently produce
duplicates' because callers 'degrade safely by returning null on read
failures.' That reasoning only holds when the read API THROWS (rate
limit, 5xx): isCommentAlreadyPosted/hasIssueCommentWithId then return
null (unknown) and the caller skips retrying. A truncated walk does not
throw — it returns a partial set silently, so isCommentAlreadyPosted
returns false (definitively 'not posted') for comments beyond the cap,
and the retry loop reposts them, producing duplicates.
Rewrite the comment to state the cap is an intentional safety valve and
to explicitly distinguish truncation (partial data, can duplicate) from
thrown read failures (null/unknown, safe). No behavior change.
* fix(ci): drop stale postedIdsCache to prevent duplicate inline comments
isCommentAlreadyPosted reused a single listReviewComments snapshot
(postedIdsCache) across all per-comment retries. As comments landed
during the loop, the snapshot went stale; a 5xx-landed comment checked
against the stale snapshot would be reported as 'not posted' and
retried, posting a duplicate.
Remove the cache and walk fresh on every check. The extra reads are
paced via readAllPages/readWithPacing (with retry honoring retry-after
and x-ratelimit-reset) and degrade to null — skip retry — if the read
API ultimately fails, so they cannot produce duplicates. The cache
provided no real benefit in this path: checked comments are either
genuine misses (correctly false) or just-landed (a fresh walk catches
them), so hits essentially never occurred.
On self-hosted runners, _github_home/.gitconfig persists across jobs.
The ocr-review workflow used --add which accumulated multiple safe.directory
values over time. Once multiple values existed, other workflows using plain
git config (without --add/--replace-all) failed with "cannot overwrite
multiple values with a single value".
Unify all workflows to use --replace-all, which clears previous values and
writes exactly one entry regardless of prior state.
Add build provenance attestation to the release workflow using
actions/attest-build-provenance with OIDC keyless signing.
Document release signature verification in SECURITY.md.
- Add govulncheck step to CI pipeline for vulnerability scanning
- Set 15-minute timeout on CI test job
- Add dependabot config for weekly Go module and GitHub Actions updates
- Split LD_FLAGS so dev builds retain debug symbols while release
builds remain stripped with -s -w
* ci(workflow): add rate-limit-aware retry for PR review comment posting
Add GitHub REST API rate-limit handling to the ocr-review workflow:
- Implement computeRetryDelayMs() following GitHub's documented strategy:
* Honor retry-after header (seconds or HTTP-date)
* Wait until x-ratelimit-reset when primary limit is exhausted (remaining=0)
* Exponential backoff (>=60s base) for secondary limits without a header
* Backoff for transient 5xx/408 errors
- Add per-comment retry loop with configurable attempts and delays
- Add logRateLimitQuota() to log and proactively throttle on low remaining quota
- Honor batch createReview rate-limit headers before per-comment retry
- Cap all waits (including header-derived) to avoid stalling the CI job
- Expose tuning via OCR_* env vars (retries, delays, thresholds)
- Update example workflow docs with retry/delay configuration reference
* fix(workflow): use shorter transient backoff base and dedupe header lookup
- Transient server errors (5xx/408) now use a 2s base delay instead of
the 60s rate-limit base, matching the comment's stated intent and
avoiding unnecessary CI stalls on short-lived server hiccups.
- Extract a shared getHeader() helper for case-insensitive header
lookup, reused by both computeRetryDelayMs and logRateLimitQuota to
eliminate duplicated logic and ensure consistent robustness.
* doc: revert example doc
The release job uses ubuntu:24.04 which lacks git. After adding
commit-based release notes generation, git commands (config/describe/log)
fail with "git: not found". Install git before checkout so the full
history is available for release notes.
Add continuous integration pipeline that runs go vet, tests with
race detection, and a build check on every push to main and PR.
Satisfies the OpenSSF Best Practices test_continuous_integration criterion.
Satisfy the OpenSSF Best Practices "release_notes" criterion by parsing
commits between tags and categorizing them into Features, Bug Fixes,
Refactoring, and Documentation sections.
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.
* ci: use self-hosted runners with container images for build and deploy workflows
- deploy-pages: switch to self-hosted runner with node:20 container, remove redundant setup-node
- release: use self-hosted runner with golang:1.26.4 container for build job, remove redundant setup-go
- release: use self-hosted runner with ubuntu:24.04 container for release job
- release: use self-hosted runner with node:20 container for npm-publish job, remove redundant setup-node
- release: add jq installation step for version injection in npm-publish
* ci: fix dubious ownership in self-hosted container jobs
- Add git safe.directory trust step after checkout in release.yml build and npm-publish jobs
- Add git safe.directory trust step after checkout in deploy-pages.yml build job
- Switch deploy-pages deploy job from ubuntu-latest to self-hosted with node:20 container
Add safe.directory '*' to git config before running git operations.
Self-hosted runners may run jobs under a different OS user than the
one that owns the workspace, causing git to refuse operations with a
'dubious ownership' error.
