* docs(examples): add GitFlic CI auto-review example
Add examples/gitflic_ci/, a CI-layer integration that reviews GitFlic
merge requests with OpenCodeReview and posts the findings as MR
discussions. Like the GitHub and GitLab examples, the posting glue lives
outside the core binary.
post_review.py (standard library only) reads `ocr review --format json`
and posts inline discussions plus a fallback note and a summary. GitFlic's
Discussions API requires an old-side line for inline comments, which the
new-side-only review output lacks, so the script recomputes it from the
same merge-base diff the review ran on. Ships with a stdlib unittest suite
whose line-mapping cases are ported from the review's diff logic.
* docs(readme): list the GitFlic CI example in the localized READMEs
* fix(examples): address GitFlic CI review feedback from PR #201
Apply the five review comments left on the PR:
- gitflic-ci.yaml: guard `ocr config set llm.model` behind a non-empty
check so the documented-optional OCR_LLM_MODEL no longer breaks the
config step when it is unset
- gitflic-ci.yaml: skip posting when `ocr review` produced no output
(the step ends with `|| true`) instead of feeding empty/partial JSON
to post_review.py
- post_review.py: redact the token from HTTP error snippets so it cannot
leak into CI logs if GitFlic echoes the request back in an error body
- post_review.py: read the review-result file via a `with` block so the
handle is closed explicitly
- examples/README.md: add the missing trailing newline
A panic in a single file's review goroutine (dispatchSubtasks) or in a CommentWorkerPool task previously crashed the whole ocr process. Recover in both: the per-file panic is isolated like an error return (counted in subtaskFailed + recorded as a subtask_error warning with stack trace + telemetry, using the parent ctx since fileCtx is already cancelled on unwind), and a panicking comment-pool task is contained so healthy tasks still complete.
Rebased onto current main: the pool moved to internal/llmloop, so the pool-side recover + the panic-isolation test now live in internal/llmloop/pool.go and pool_test.go; the per-file recover stays in internal/agent/agent.go. Also documents CommentWorkerPool.Await's concurrency contract (Submit must not race Await).
Update documentation strings across en/ja/zh to reflect the new
provider-based configuration: add full scan review mode, list built-in
providers (Anthropic, OpenAI, DashScope, DeepSeek, Z.AI), and replace
legacy config keys with provider-scoped keys including extraHeaders.
* 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.
* feat(mcp): add Model Context Protocol server support
Add MCP client and provider packages that allow integrating external
MCP tool servers into the review loop. Includes config commands for
managing MCP servers, stdio subprocess integration tests, and
comprehensive test coverage.
* refactor(mcp): rename loop variable in contentToText to avoid shadowing Client receiver
* fix(mcp): use platform-specific shell for setup command
The MCP server setup command was hardcoded to use `sh -c`, which
fails on Windows. Extract a `shellCommand` helper behind build tags
to use `cmd /c` on Windows and `sh -c` elsewhere.
* docs(mcp): add MCP server documentation to all README locales
Merge the redundant scan --preview standalone section into its existing
card with richer copy, and add a matching Dry-Run Preview card to the
ocr review advanced usage section. Updates en/zh/ja i18n files.
* docs(pages): add ocr scan documentation page
Adds a dedicated Scan page covering when to use ocr scan vs ocr
review, basic usage, --preview dry-run, batching strategies,
--no-plan/--no-dedup/--no-summary toggles, --max-tokens-budget,
and the full flag reference — cross-checked against
cmd/opencodereview/scan_cmd.go. Wires the /scan route into App.tsx,
adds a Scan nav link, and adds scan.* i18n keys for en/zh/ja.
Closes#242
* refactor(pages): move ocr scan docs into the Docs page
Folds the ocr scan documentation into a new section within
DocsPage.tsx instead of a standalone /scan page, matching how
ocr review is structured. Removes ScanPage.tsx, the /scan route,
and the Scan nav link.
Also addresses the automated review findings: reuses DocsPage's
existing Toast/CodeBlock/IconBox (no more duplication), adds alt=""
to IconBox, i18n-izes the 'ocr review'/'ocr scan' comparison labels,
and adds a console.warn when clipboard copy fails.
i18n keys renamed from scan.* to docs.scan* to match the existing
docs.review* naming convention.
Prevent make test from polluting ~/.opencodereview/sessions/ with
thousands of var-folders-* directories by redirecting test session
writes to ~/.opencodereview/test-sessions/.
Introduce a package-level sessionSubDir variable (default "sessions")
and an exported UseTestSessions() function that switches it to
"test-sessions". Each test package that creates sessions calls
UseTestSessions() from init() in an init_test.go file.
* fix: add configurable HTTP timeout for LLM API calls
The LLM HTTP client had no user-configurable request timeout. When the
LLM API becomes unresponsive (e.g., rate limiting, network issues), the
ocr process hangs indefinitely with 0% CPU, holding an ESTABLISHED TCP
connection that never completes.
Changes:
- Add Timeout field to ResolvedEndpoint struct
- Add OCR_LLM_TIMEOUT environment variable (value in seconds)
- Add timeout_sec field to config.json llm and provider sections
- Pass timeout from ResolvedEndpoint to ClientConfig in NewLLMClient
The default timeout remains 5 minutes when not explicitly configured.
Users can now set a shorter timeout via:
- Environment: OCR_LLM_TIMEOUT=120
- Config file: {"llm": {"timeout_sec": 120}}
Fixes#237
* fix: address Copilot review feedback
- Make OCR_LLM_TIMEOUT a global override (works with any endpoint strategy)
- Add proper overflow detection for timeout values (check before multiply)
- Add validateTimeoutSec helper with negative/overflow guards
- Add comprehensive tests for timeout parsing, validation, and forwarding
- Reject negative timeout_sec in config files with explicit error
* fix: address review feedback for timeout configuration
- parseTimeoutEnv now returns errors instead of silent fallback for invalid values (negative, non-integer, overflow)
- Reuse validateTimeoutSec in parseTimeoutEnv to eliminate duplicated validation logic
- Remove redundant comment in tryOCREnv
- Add missing tests:
- TestResolveEndpoint_ProviderConfigTimeoutSec
- TestResolveEndpoint_ProviderConfigNegativeTimeoutSec
- TestResolveEndpoint_EnvTimeoutOverridesConfigTimeout
- TestResolveEndpoint_EnvTimeoutOverridesProviderTimeout
- TestResolveEndpoint_InvalidEnvTimeoutWithConfig
- TestResolveEndpoint_NegativeEnvTimeoutWithConfig
- Reorder terminal lines: move Summary above the separator line
- Change command from range mode (--from/--to) to workspace mode (ocr review)
- Apply semantic color scheme: brand, command, path, success, action, dim
- Remove unused terminal prompt icon and hasIcon field
- Add blinking cursor animation for the last terminal line
- Reduce line number container width after icon removal
Add official website URL to the "What is Open Code Review?" section
across all localized READMEs. Update highlights and benchmark
screenshots for en/zh, add Japanese versions (highlights-ja.png,
benchmark-ja.png), and add id="highlights" to HighlightsSection
for screenshot tool targeting.
Use navigator.languages to detect the user's preferred language and
automatically select the matching locale (zh/ja/en) on first visit,
falling back to English when no match is found.
The ocr viewer command is a session history viewer, not a results browser.
Updated the description in all three locales (en, zh, ja) to accurately
reflect its purpose.
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.
* feat: support inline content and file path resolution for rule field
- rule field auto-detects: .md/.txt/.markdown ending = file path, otherwise inline
- file paths: project-relative first, then as absolute path
- safety: stat before read (512KB cap), extension whitelist, symlink resolution
- tightened heuristic: values with spaces treated as inline to avoid false positives
- guard against empty repoDir to avoid CWD-relative resolution
- 5-language README docs updated with file path usage and first-match-wins behavior
- 15 new unit tests covering all resolution branches
Closes#67
Supersedes #87
* update readme
* feat: update rule resolution logic to clear rules for missing or invalid files
* feat: update rule field description to clarify file path detection criteria
* feat: enhance rule file resolution to block path traversal and improve validation
* fix: clean up tryReadRuleFile and add missing blank line
- Add blank line between matchProjectRuleEntry and allowedRuleExts (Issue 1)
- Remove dead code '|| repoDir == ' in tryReadRuleFile (Issue 2)
- Remove unnecessary warning when repoDir is empty but path is absolute (Issue 3)
Add comprehensive tests for events, metrics, provider, shutdown, span,
and exporter in the telemetry package. Update Makefile to exclude the
extensions directory from test, fmt, vet, and check targets.
The test relied on filesystem ModTime for sorting repos, but files
created in rapid succession can share the same mtime on CI, making
the sort order non-deterministic. Use os.Chtimes to guarantee repo-b
has a strictly later mtime than repo-a.
The testdiff CLI was an early-stage tool for manually testing the
internal/diff package. It has no external references and its role is
fully covered by the existing unit tests in internal/diff/.
Add integration-style tests with fake LLM clients for agent dispatch and
llmloop runner, plus new unit test files for gitcmd, session/history,
tool/code_comment, tool/filereader_read, and viewer/store packages.
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 ASSURANCE_CASE.md covering threat model, secure design principles
(Saltzer & Schroeder), OWASP/CWE countermeasures, and automated
verification. Switch tag command from annotated (-a) to signed (-s) to
match the assurance case's integrity claims.
- 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
Cover planned work (JetBrains plugin, standard MCP integration,
Ultra mode, domain-specific long-term memory) and explicit non-goals.
Satisfies the OpenSSF Best Practices silver badge documentation_roadmap
criterion.
* feat: improve config provider TUI interaction
- Support wrap-around navigation in provider, custom provider, and model lists
(up at first item jumps to last, down at last item jumps to first)
- Add d key to delete custom models in model selection step with confirmation prompt
- Add protocol selection (anthropic/openai) to manual configuration flow
and respect UseAnthropic setting in saved config
- Preserve manual protocol selection when re-entering the form
- Fix manual form input fields losing focus after returning from next step
- Use safe slice removal in removeFromSlice to avoid backing array corruption
* fix: address code review feedback
- Restore Blur() calls in handleManualFormEnter to prevent visual focus artifacts
when transitioning between manual form steps
- Add empty-providers guard in handleUp/handleDown to avoid negative index
- Consolidate redundant protocolIdx checks into single if/else in viewManualTab
and viewCustomProviderForm for clarity
* fix: persist custom model deletions and support masked auth token editing
- Track deleted custom models in TUI and apply them on exit
(both confirm and cancel paths) to keep config.json in sync
- Clear custom provider's active Model field when that model is deleted,
preventing stale "model" reference in the provider list label
- Add masked display for manual config Auth Token to allow easy re-entry,
matching the official provider flow (any key clears and starts fresh)
* test: update manual form tests for masked auth token behavior
Update TestProviderTUI_ManualFormPrefilledValues,
TestProviderTUI_ManualFormEscRestoresOriginalValues, and
TestProviderTUI_ManualFormPrefilledWhenProviderSet to assert
the new masked display state (manualTokenMasked + manualTokenOriginal)
instead of the raw token value.
* feat: refine config provider TUI flows and session persistence
Custom provider interactions:
- Simplify create/edit form to Name → Protocol → URL → API Key → Auth Header
- After create or edit save, jump straight into the model list for that provider
- Support comma-separated model names in the custom model input
- Show masked API key in edit form; empty Auth Header defaults to (Authorization)
- Populate edit form from existingCfg to avoid stale list data after in-session saves
Model selection and highlight:
- Green highlight follows the persisted active model, not the cursor position
- Prefer provider entry.model over global cfg.model when resolving active model
- Deleting a non-active model keeps the current green highlight unchanged
Delete and navigation:
- Delete custom providers (d) and custom models (d) with confirmation prompts
- Persist create/edit/model-select/add/delete changes to disk during the session
- Fix custom provider deletion not surviving Esc exe-entry
Tests:
- Add coverage for model highlight, delete-model behavior, and create→model-list flow
- Update manual/custom form tests for masked token and session save behavior
* refactor: address provider TUI code review feedback
- Remove dead helper removeFromSlice (superseded by removeModels)
- Move misplaced doc comment to applyEditCustomProviderSave
- Drop redundant applyProviderDeletions post-TUI call so provider
deletions rely solely on the in-session save
- Fix brace/indent drift in updateDeleteModelConfirm and cache the
model list instead of recomputing m.models() twice
* feat: refine provider TUI flows and manual config form
Custom provider flow:
- Default protocol to anthropic on the new-provider form
- Single-name model input; reject duplicates with inline error and
preserve the typed value so the user can edit instead of re-typing
- Drop the global green highlight; cursor/blue is the only selection cue
Manual configuration form:
- Add Auth Header step (URL → Protocol → Model → Auth Token → Auth Header)
and persist it to Llm.AuthHeader on confirm
- Reorder so Auth Header is always entered last
- Make Auth Token required; empty Enter stays on the field
- Show every field's label on every render, even when empty, matching
the custom-provider form style
Tests:
- Cover custom model input add / duplicate paths
- Cover manual form prefill of Llm.AuthHeader
- Cover that deleting a non-active model keeps the active model intact
* Improve provider TUI validation, persistence, and code clarity.
Address code review feedback: align custom Auth Header validation with manual mode; fix savedInSession after model deletion; refactor applyEditCustomProviderSave to return error; remove dead applyModelDeletions/deletedModels; document ExtraBody shallow-copy limit. Also fix manual/custom form UX (token skip on edit, k key input, formError scoping, switch indentation).
* Fix provider/model TUI list ordering and add test coverage.
Address review and UX feedback: remove model list sorting in provider and config model TUIs; preserve Models list order when selecting active model (ensureModelInList); add test for duplicate rename on custom provider edit. Includes prior review fixes for savedInSession, applyEditCustomProviderSave error return, and dead code removal.
* Normalize AuthHeader at apply layer and simplify UseAnthropic assignment.
Call NormalizeAuthHeader in applyManualConfig and applyCustomProviderConfig before save; simplify UseAnthropic assignment in manual config; add unit tests.
* Address review: lowercase error strings and newProviderTUI signature.
Use lowercase "failed to save" errors; replace variadic configPath with string and remove configPathFromArgs; pass "" in tests when no path; gofmt provider_tui.go.
