Remove the duplicate kimi provider entry introduced by overlapping PRs
(#130 and #134). Keep the Kimi Moonshot API entry with MOONSHOT_API_KEY.
Set LC_ALL=C in Makefile test target to ensure git outputs English
messages regardless of system locale.
When a file was renamed on the target branch, ocr review emitted
'[ocr] WARNING: cannot read file <old path> at ref <to>: exit status 128'.
Two compounding bugs:
1. The parser required 'a/'/'b/' prefixes when matching '--- /dev/null' /
'+++ /dev/null', but git emits these lines without prefixes, so
IsNew/IsDeleted were never set and deleted files fell through to a
doomed 'git show ref:<old path>'.
2. 'rename from' / 'rename to' extended headers were never parsed, and
git diff/show call sites did not force rename detection, so renames
degraded to delete+add whenever the user had diff.renames=false.
Fixes:
- Parse 'rename from'/'rename to', 'new file mode', 'deleted file mode'
and unprefixed /dev/null markers in ParseDiffText.
- Pass --find-renames to all git diff/show invocations so rename
detection no longer depends on user config.
- Add IsRenamed to model.Diff (json: is_renamed) and prefer it in
diffStatus.
- Add parser unit tests and a range-mode rename regression test.
Fixes#99
- Add REVIEW_FILTER_TASK template and LLM conversation config
- Implement executeReviewFilter to post-process comments per file
- Add CommentsForPath and RemoveByPathAndIndices to CommentCollector
- Apply timeout from template config to review filter LLM call
- Await CommentWorkerPool before running filter to prevent race condition
- Include raw response preview in filter JSON parse error logs
Add --src-prefix=a/ --dst-prefix=b/ to all git diff/show calls so that
user config (diff.noprefix, diff.mnemonicPrefix) cannot alter the prefix
format the parser depends on. Also add missing ModeCommit test coverage.
When a user has configured a global external diff tool (diff.external /
GIT_EXTERNAL_DIFF) or a textconv filter, git diff/show emit the tool's
output instead of unified diff text. The provider's parser keys off
`diff --git` headers, so it parses zero diffs and the review silently
reports "No files changed".
Add --no-ext-diff --no-textconv to all four git diff/show call sites in
internal/diff/git.go (ModeRange diff, ModeCommit show, and both
workspaceTrackedDiff calls). merge-base and ls-files are left untouched
since they don't run the diff machinery.
Adds an integration test that initializes a real git repo, activates a
garbage GIT_EXTERNAL_DIFF script, and asserts the provider still parses
a non-empty diff (fails before this change, passes after).
Closes#82
AI-assisted contribution.
- Propagate json.Unmarshal error in buildAnthropicParams when parsing
tool call arguments, preventing silent empty-map fallback on malformed JSON
- Return explicit error message in ParseComments when comments JSON
string fails to unmarshal, instead of silently ignoring
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Export NormalizeAuthHeader from the llm package and remove the duplicate
normalizeConfigAuthHeader in config_cmd.go. Invalid auth_header values
now return an error at configuration time instead of being silently
accepted and causing opaque 401s at runtime.
Anthropic models with extended thinking return type:"thinking" content
blocks which were silently dropped, causing empty assistant content in
conversation history. Extract thinking blocks into ReasoningContent so
the existing Content() fallback works correctly.
The session page was showing significantly lower token counts than the console
summary because it used local tiktoken estimation while the console used actual
API-reported usage data. Now SetResponse prefers resp.Usage when available
(falling back to tiktoken), persists cache token fields to JSONL, and the
viewer displays cache read/write stats alongside prompt/completion totals.
Replace os.ReadFile + strings.Split with bufio.Reader streaming in
FileReadProvider, so only the requested window (up to 500 lines) is kept
in memory instead of the entire file. Adds Runner.Stream for semaphore-
aware stdout piping, FileReader.ReadLines for line-windowed reading, and
validates invalid line ranges. Reduces per-goroutine memory from ~8MB to
~44KB for large files under concurrent workloads.
Introduce gitcmd.Runner with channel-based semaphore to cap concurrent
git subprocesses (default 16, configurable via --max-git-procs). Route
all tool-layer and diff-layer git calls through the shared runner.
Also fix diff.Provider.runGit lacking context.Context — now the full
chain (GetDiff → MergeBase → ParseDiffText → finalizeDiff) propagates
the caller's context for proper cancellation and timeout support.
Registry was a plain map alias and DiffMap a shared map[string]string,
both relying on implicit write-before-read ordering with no compile-time
safety. Refactor Registry into an encapsulated struct with Freeze() that
panics on post-freeze writes, and wrap DiffMap as an immutable value type
with defensive copy on construction. Move DiffMap injection to before
filterDiffs to preserve the original behavior of exposing all file diffs.
Add cache_control ephemeral breakpoints on system prompt and tool
definitions to activate Anthropic prompt caching for multi-turn
agent loops. Fix token double-counting by removing the redundant
totalTokensUsed accumulator and computing it from sub-items. Surface
cache read/write statistics in both text and JSON summary output.
Add ctx parameter to Provider.Execute interface and propagate it through
FileReader.Read, code_search.gitGrep/runGitGrep, and file_find.listGitFiles.
This ensures parent task cancellation/timeout properly terminates running
git subprocesses (grep, show, ls-files, ls-tree). Also adds 10s timeout
to file_find and fixes context.Canceled handling in code_search.
Rule doc files were translated from Chinese to English but tests still
expected Chinese substrings. Also added HOME isolation to prevent the
user's ~/.opencodereview/rule.json from interfering with system-default
rule assertions.
Migrated internal/llm from manual net/http implementation to anthropic-sdk-go v1.47.0
and openai-go/v3 v3.39.0, reducing ~400 lines of retry, request-building, and streaming
code. Simplified LLMClient interface to single CompletionsWithCtx method. Fixed Anthropic
auth to use WithAuthToken (Bearer) instead of WithAPIKey (X-Api-Key) for proxy compatibility.
Fixed OpenAI message builder to use ExtractText() for proper []ContentBlock handling.
StreamCompletion used http.NewRequest without context and retry with
context.Background(), making streaming calls uncancellable. Memory
compression called Completions() without context, leaving async
goroutines unable to be cancelled or timed out.
- Add StreamCompletionWithCtx to LLMClient interface and both
OpenAI/Anthropic implementations
- Propagate context through runCompression, addNextMessage, and
triggerAsyncCompression call chain
- Add 5-minute timeout cap on async compression goroutines
- Cancel in-flight compression HTTP requests when job is superseded
The code_search tool used -E (extended regex) by default, causing
unmatched parentheses in search patterns (e.g. "func Hello()") to
trigger "Unmatched ( or \(" fatal errors on Linux (GNU regex is strict),
while macOS (BSD regex) silently tolerated them. Since the tool
definition states the default mode is "literal text", switch to -F
(fixed string). Also capture stderr separately so real errors are
surfaced instead of being silently reported as "No matches found".
The searchText was placed after '--', causing git to interpret it as a
pathspec instead of a pattern. Use '-e' to explicitly mark the search
pattern and place the ref after it. Also replace CombinedOutput with
Output to prevent stderr from polluting result parsing.
* feat: add .ets file extension support for HarmonyOS development
* feat: add complete HarmonyOS review support with ArkTS rules, .json5 config, and oh_modules exclusion
- Add arkts.md rule doc with ArkTS-specific and TypeScript general checks
- Add .json5 to supported file types for HarmonyOS config files
- Map .json5 to json.md rules in system_rules.json
- Exclude oh_modules/** (HarmonyOS dependency dir) from review
- Exclude *.test.ets (HarmonyOS test files) from review
- Add unit tests for all new extensions and patterns
* refactor: merge json and json5 glob patterns into single brace expansion
Store ReviewMode, DiffFrom, DiffTo, and DiffCommit in SessionHistory
and persist them to JSONL (conditionally, only when non-empty). Display
mode in viewer list page and show version details (from/to or commit)
on the session detail page based on review mode.
code_search used SplitN(":", 3) to parse git grep output, but in
range/commit mode git grep prefixes each line with the ref
(ref:file:line:content). All matches silently failed Atoi on the
filepath field, returning an empty string that caused the review
session to terminate immediately.
Fix code_search to detect the ref prefix and adjust split/index
accordingly. Also replace the hard stop on empty tool results with
a retry loop (up to 3 consecutive empty rounds) so the LLM can
recover or call task_done gracefully.
file_find and finalizeDiff always read from the working tree, even in
range/commit mode where the review targets a specific git ref. This
caused inconsistent file versions compared to file_read and code_search
which correctly used git show. Fix file_find to use git ls-tree and
finalizeDiff to use git show when a ref is specified.
Replace the hard-coded Chinese-only empty-plan branch with a compiled
regex + helper that strips the "### Review Plan …" wrapper (any language
variant) when the plan phase produces no output.
TWO BUGS fixed:
- The wrapper was only stripped for the Chinese literal `### 审查计划`,
not the actual English `### Review Plan (Optional)` shipped in the
template, so `{{plan_guidance}}` leaked into the rendered prompt on
every review where the plan phase was skipped or failed.
- The strip ran AFTER `ReplaceAll("{{plan_guidance}}", "")`, so the
token was already gone and the regex couldn't match the wrapper.
Fixed ordering: strip BEFORE replace.
Also simplifies the regex alternation per Copilot review feedback:
`Review Plan` and `审查计划` already subsume the `(Optional)`/`（可选）`
suffixes because the trailing `[^\n]*` consumes them.
Adds unit tests for English, Chinese, and integration (production flow).
Co-authored-by: Copilot <copilot-pull-request-reviewer@github.com>
Make encodeRepoPath cross-platform by properly handling volume names, backslash separators, and drive letter colons. Add unit tests covering Unix, Windows, and edge-case inputs.
* fix(agent): skip binary files during code review
Previously, binary files were passed through the review pipeline because
shouldReview only checked path/extension rules. This caused the agent to
incorrectly attempt reviewing binary content.
- Add ExcludeBinary reason to the exclusion filter
- Change shouldReview/whyExcluded to accept model.Diff instead of path
string, enabling access to the IsBinary flag
- Check IsBinary as the first gate in whyExcluded
- Differentiate skip log messages for binary vs path-filtered files
* test: add unit tests for preview.go
* fix: normalize dash to em-dash in binary file skip log message
* test(agent): strengthen UserIncludePattern tests to cover additive semantics
- Document that include patterns are additive, not exclusive
- Add cases verifying include bypasses default-path exclusion
- Add cases for extension filter precedence over include logic
- Add TestWhyExcluded_IncludeBypassesDefaultPath for rescue semantics
- Add TestWhyExcluded_IncludeAndExcludeInteraction for priority order
The viewer's HTTP server (StartServer in internal/viewer/server.go) had no
Host-header validation, so a web page the user visits could DNS-rebind its
own origin to 127.0.0.1:5483 and read every session JSONL — which contains
LLM request messages (= source code being reviewed) and the LLM's analysis.
Adds a default-deny host guard that always allows loopback names plus the
concrete bind host. Wildcard binds and extra hostnames go through the new
OCR_VIEWER_ALLOWED_HOSTS env var, so an operator who binds the viewer on a
LAN interface has to acknowledge the exposure explicitly.
Detected by Aeon + manual review.
Severity: high
CWE-346 (Origin Validation Error), CWE-200 (Exposure of Sensitive Info)
Co-authored-by: aeonframework <aeonframework@users.noreply.github.com>
